Re: [PATCH v2 5/6] media: qcom: camss: csid: Add support for CSID 1080

Vijay Kumar Tumati
2025-10-16T22:03:44Z

On 10/16/2025 3:18 AM, Bryan O'Donoghue wrote:
> On 16/10/2025 11:04, Bryan O'Donoghue wrote:
>> drivers/media/platform/qcom/camss/camss-csid-gen3.c: csid-
>>  >reg_update &= ~CSID_RUP_AUP_RDI(port_id);
>> drivers/media/platform/qcom/camss/camss-csid-gen3.c: csid-
>>  >reg_update |= CSID_RUP_AUP_RDI(port_id);
>>
>> and this in your code
>>
>>
>> λ ~/Development/qualcomm/qlt-kernel/ linux-stable/master-reviews-
>> oct15-25* grep aup_update drivers/media/platform/qcom/camss/*
>>
>> drivers/media/platform/qcom/camss/camss-csid-1080.c:static void
>> __csid_aup_update(struct csid_device *csid, int port_id)
>> drivers/media/platform/qcom/camss/camss-csid-1080.c: csid->aup_update
>> |= AUP_RDIN << port_id;
>
> And now that I see the code side-by-side - also please use the
> established macros and/or write a new macro to follow the established
> pattern.
>
> There's virtually no good argument to replicate a bit shift or twiddle
> - that can be functionally decomposed and encapsulated in one place
> and subsequently reused.
>
> ---
> bod
>
Hi @Bryan, sure. Both are essentially shift + twiddle, just that in this
patch, both are happening in one place. Where as in gen3, the shift is
happening inside the macro. The other difference is that on Kaanapali,
RUP and AUP update registers are separated and hence need to be handled
separately. But I understand your point about the consistency. We will
modify the 1080 macros to be consistent with gen3. OR we can add two
macros commonly in csid.h that takes both the base bit (RDI0) offset
within those registers and also the port ID to return a value with the
bit set, just that gen3 file will have to call them separately from
within "csid_subdev_reg_update". Please let us know if you would like
this. Thanks.