Re: [PATCH] media: qcom: camss: csid: Consolidate CSI2_RX_CFG0_PHY_SEL_BASE_IDX definition

[Vijay Kumar Tumati <vijay.tumati@oss.qualcomm.com>]

Hi Hangxiang,

On 6/1/2026 8:13 AM, hangxiang.ma@oss.qualcomm.com wrote:
> On 6/1/26 11:04 PM, Loic Poulain <loic.poulain@oss.qualcomm.com> wrote:
>> On Mon, Jun 1, 2026 at 4:44 PM Hangxiang Ma
>> <hangxiang.ma@oss.qualcomm.com> wrote:
>> >
>> > Move the duplicate CSI2_RX_CFG0_PHY_SEL_BASE_IDX definition from
>> > camss-csid-680.c and camss-csid-gen3.c into the shared camss-csid.h
>> > header. This eliminates redundancy and makes the constant available
>> > to future CSID implementations.
>>
>> Taking that direction, I don’t think this is the only instance of
>> redundancy, so why single out this one in particular? Should we
>> consider one-line cleanups across all similar cases? Also, other CSID
>> drivers follow the same pattern but use different identifiers for that
>> define (e.g. csid-340).
>>
>> Also, introducing such low-level, register-aligned naming
>> (CSI2_RX_CFG0_PHY...)  in what is supposed to be a generic
>> CSID header doesn’t seem appropriate.
>>
>> Regards,
>> Loic
>>
>>
>>
>> >
>> > Signed-off-by: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
>> > ---
>> > Move the duplicate CSI2_RX_CFG0_PHY_SEL_BASE_IDX definition from
>> > camss-csid-680.c and camss-csid-gen3.c into the shared camss-csid.h
>> > header. This eliminates redundancy and makes the constant available
>> > to future CSID implementations.
>> > ---
>> >   drivers/media/platform/qcom/camss/camss-csid-680.c  | 1 -
>> >   drivers/media/platform/qcom/camss/camss-csid-gen3.c | 1 -
>> >   drivers/media/platform/qcom/camss/camss-csid.h      | 2 ++
>> >   3 files changed, 2 insertions(+), 2 deletions(-)
>> >
>> > diff --git a/drivers/media/platform/qcom/camss/camss-csid-680.c b/ 
>> drivers/media/platform/qcom/camss/camss-csid-680.c
>> > index 345a67c8fb94..bf7164085ddb 100644
>> > --- a/drivers/media/platform/qcom/camss/camss-csid-680.c
>> > +++ b/drivers/media/platform/qcom/camss/camss-csid-680.c
>> > @@ -101,7 +101,6 @@
>> >   #define                
>> CSI2_RX_CFG0_DL2_INPUT_SEL                      12
>> >   #define                
>> CSI2_RX_CFG0_DL3_INPUT_SEL                      16
>> >   #define                
>> CSI2_RX_CFG0_PHY_NUM_SEL                        20
>> > -#define                
>> CSI2_RX_CFG0_PHY_SEL_BASE_IDX                   1
>> >   #define                
>> CSI2_RX_CFG0_PHY_TYPE_SEL                       24
>> >   #define                
>> CSI2_RX_CFG0_TPG_MUX_EN                         BIT(27)
>> >   #define                
>> CSI2_RX_CFG0_TPG_MUX_SEL                        GENMASK(29, 28)
>> > diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen3.c b/ 
>> drivers/media/platform/qcom/camss/camss-csid-gen3.c
>> > index 0fdbf75fb27d..da9458cd178b 100644
>> > --- a/drivers/media/platform/qcom/camss/camss-csid-gen3.c
>> > +++ b/drivers/media/platform/qcom/camss/camss-csid-gen3.c
>> > @@ -105,7 +105,6 @@
>> >   #define CSID_RDI_IRQ_SUBSAMPLE_PERIOD(rdi)     (csid_is_lite(csid) 
>> && IS_CSID_690(csid) ?\
>> >                                                          (0x34C + 
>> 0x100 * (rdi)) :\
>> >                                                          (0x54C + 
>> 0x100 * (rdi)))
>> > -#define CSI2_RX_CFG0_PHY_SEL_BASE_IDX  1
>> >
>> >   static void __csid_configure_rx(struct csid_device *csid,
>> >                                  struct csid_phy_config *phy, int vc)
>> > diff --git a/drivers/media/platform/qcom/camss/camss-csid.h b/ 
>> drivers/media/platform/qcom/camss/camss-csid.h
>> > index 5296b10f6bac..059ac94ad1be 100644
>> > --- a/drivers/media/platform/qcom/camss/camss-csid.h
>> > +++ b/drivers/media/platform/qcom/camss/camss-csid.h
>> > @@ -27,6 +27,8 @@
>> >   /* CSID hardware can demultiplex up to 4 outputs */
>> >   #define MSM_CSID_MAX_SRC_STREAMS       4
>> >
>> > +/* CSIPHY to hardware PHY selector mapping */
>> > +#define CSI2_RX_CFG0_PHY_SEL_BASE_IDX 1
>> >   #define CSID_RESET_TIMEOUT_MS 500
>> >
>> >   enum csid_testgen_mode {
>> >
>> > ---
>> > base-commit: 697a0e31ee66f5ddb929c09895139779fff33f20
>> > change-id: 20260601-camss-macro-3d40c4d4e90d
>> >
>> > Best regards,
>> > --
>> > Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>
>> >
>>
> Thanks Loic, Bryan pointed this out in last review cycle and suggested 
> to split it as a standalone series. This idea comes from KNP series as I 
> was once suggested to move this macro into one common header to remove 
> redundancy. I think your are correct after fully consideration. I will 
> make changes only for KNP and put it in driver.
> 
> Best regards,
> Hangxiang
> 
So this patch is not necessary any more, is it? If there is anything 
specifically to be done to withdraw this, just for it to be clear to 
Bryan, can you please?

Thanks,
Vijay.