Re: [PATCH 1/6] media: qcom: camss: Program CSIPHY common control registers

[Bryan O'Donoghue <bryan.odonoghue@linaro.org>]

On 20/07/2026 08:56, Loic Poulain wrote:
> On Sat, Jul 18, 2026 at 1:13 AM Anusha Arun Nandi
> <anusha.nandi@oss.qualcomm.com> wrote:
>>
>> From: Jigarkumar Zala <jigarkumar.zala@oss.qualcomm.com>
>>
>> Program the CSIPHY common-control registers during lane enable so
>> SA8775P uses the required 3-phase 1.5 Gsps reset-release values for
>> both C-PHY and D-PHY. Keep the existing reset-release value for the
>> other CAMSS variants.
>>
>> Co-developed-by: Anusha Arun Nandi <anusha.nandi@oss.qualcomm.com>
>> Signed-off-by: Anusha Arun Nandi <anusha.nandi@oss.qualcomm.com>
>> Signed-off-by: Jigarkumar Zala <jigarkumar.zala@oss.qualcomm.com>
>> ---
>>   .../qcom/camss/camss-csiphy-3ph-1-0.c         | 36 ++++++++++++++++---
>>   1 file changed, 31 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
>> index 25a3554ef201..11b7a236e607 100644
>> --- a/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
>> +++ b/drivers/media/platform/qcom/camss/camss-csiphy-3ph-1-0.c
>> @@ -1031,8 +1031,6 @@ static void csiphy_reset(struct csiphy_device *csiphy)
>>          writel_relaxed(0x1, csiphy->base +
>>                        CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->offset, 0));
>>          usleep_range(5000, 8000);
>> -       writel_relaxed(0x0, csiphy->base +
>> -                      CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->offset, 0));
>>   }
>>
>>   static irqreturn_t csiphy_isr(int irq, void *dev)
>> @@ -1343,9 +1341,37 @@ static void csiphy_lanes_enable(struct csiphy_device *csiphy,
>>          writel_relaxed(val, csiphy->base +
>>                         CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->offset, 6));
>>
>> -       val = 0x02;
>> -       writel_relaxed(val, csiphy->base +
>> -                      CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->offset, 7));
>> +       switch (csiphy->camss->res->version) {
>> +       case CAMSS_8300:
>> +       case CAMSS_8775P:
>> +               if (c->phy_cfg == V4L2_MBUS_CSI2_CPHY) {
>> +                       val = 0x5A;
>> +                       writel_relaxed(val, csiphy->base +
>> +                                      CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->offset, 7));
>> +                       val = 0xE;
>> +                       writel_relaxed(val, csiphy->base +
>> +                                      CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->offset, 0));
>> +               } else {
>> +                       val = 0x02;
>> +                       writel_relaxed(val, csiphy->base +
>> +                                      CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->offset, 7));
>> +                       writel_relaxed(val, csiphy->base +
>> +                                      CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->offset, 0));
>> +               }
>> +               break;
>> +       case CAMSS_7280:
>> +       case CAMSS_8250:
>> +       case CAMSS_8280XP:
>> +       case CAMSS_845:
>> +       default:
>> +               val = 0x02;
>> +               writel_relaxed(val, csiphy->base +
>> +                              CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->offset, 7));
>> +               val = 0x0;
>> +               writel_relaxed(val, csiphy->base +
>> +                              CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->offset, 0));
>> +               break;
>> +       }
> 
> I don't really like this big camss-version switch case with magic,
> which also introduces more magic values. Maybe we should have a
> different version of csiphy_lanes_enable, based on the phy compatible
> or revision.
No.

Just move all of this logic into the new CSIPHY driver. I won't be 
merging more technical debt here.

---
bod