Re: [PATCH v2 3/7] media: qcom: camss: Allow CSI2 PHY devices without CSIPHY resources

[Vladimir Zapolskiy <vz@kernel.org>]

On 10/27/26 12:52, Loic Poulain wrote:
> Hi Vlad,
> 
> On Mon, Oct 26, 2026 at 11:50 PM Vladimir Zapolskiy <vz@kernel.org> wrote:
>>
>> If CAMSS CSIPHYs are supported by the stand-alone CSI2 PHY driver only,
>> there is no need to provide description of legacy CSIPHY resources in
>> the CAMSS driver, therefore if CSIPHY resources are not given for some
>> supported platform, use CAMSS CSIPHY port numbers as CSIPHY identifiers.
>>
>> Signed-off-by: Vladimir Zapolskiy <vz@kernel.org>
>> ---
>>   drivers/media/platform/qcom/camss/camss-csiphy.c | 16 +++++++++++++---
>>   drivers/media/platform/qcom/camss/camss.c        |  8 ++++++--
>>   2 files changed, 19 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
>> index f99eea2ef228..8ecd514451d2 100644
>> --- a/drivers/media/platform/qcom/camss/camss-csiphy.c
>> +++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
>> @@ -799,6 +799,11 @@ int msm_csiphy_subdev_init_legacy(struct camss *camss,
>>          return ret;
>>   }
>>
>> +/* Fallback to generic SDM845 compatible formats for non-legacy CSIPHYs */
>> +static const struct csiphy_subdev_resources csiphy_res_phy = {
>> +       .formats = &csiphy_formats_sdm845,
>> +};
>> +
>>   /*
>>    * msm_csiphy_subdev_init - Initialize CSIPHY device structure and resources
>>    * @camss: CAMSS structure
>> @@ -808,7 +813,7 @@ int msm_csiphy_subdev_init_legacy(struct camss *camss,
>>    */
>>   int msm_csiphy_subdev_init(struct camss *camss, u8 port)
>>   {
>> -       const struct camss_subdev_resources *res = &camss->res->csiphy_res[port];
>> +       const struct camss_subdev_resources *res = camss->res->csiphy_res;
>>          struct csiphy_device *csiphy = &camss->csiphy[port];
>>          struct device *dev = camss->dev;
>>          struct device_node *ep, *remote;
>> @@ -836,8 +841,13 @@ int msm_csiphy_subdev_init(struct camss *camss, u8 port)
>>          }
>>
>>          csiphy->camss = camss;
>> -       csiphy->id = res->csiphy.id;
>> -       csiphy->res = &res->csiphy;
>> +       if (res) {
>> +               csiphy->id = res[port].csiphy.id;
>> +               csiphy->res = &res[port].csiphy;
> 
> So, can't we simply drop the above assignment and keep only the
> port/csiphy_res_phy below? That would provide the same behavior for
> all non-legacy PHY initializations, regardless of whether legacy
> CSIPHY resources are described or not.
> 

Please note that 'csiphy_res_phy' was introduced a few lines above
by this change, it's only purpose is to provide the needed supported
media formats, and this is unavoidable.

This change follows the assumption that pixel formats from all
non-legacy PHYs are SDM845 compatible, that's true for this platform,
it is expected to be true for the most of approaching non-legacy
platforms, and on demand it may be changed later on.

The current form likely is the most compact and the least invasive
change, which gets to the point of supporting CAMSS on platforms
with no CSIPHY resources given in the CAMSS driver.

For instance, note, the recently proposed Glymur CAMSS [1] references
X1E CSIPHY resources, which is actually incorrect, since there is
no "csiphyX" I/O resources anymore for Glymur. This change can be
reused for Glymur as well, and its reference to X1E CSIPHYs can be
dropped, but any better proposal or improvement are welcome.

>> +       } else {
>> +               csiphy->id = port;
>> +               csiphy->res = &csiphy_res_phy;
>> +       }
>>
>>          snprintf(csiphy->name, ARRAY_SIZE(csiphy->name), "csi%d", csiphy->id);
>>
>> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
>> index c54185c93a8a..b0d60e70e4bb 100644
>> --- a/drivers/media/platform/qcom/camss/camss.c
>> +++ b/drivers/media/platform/qcom/camss/camss.c
>> @@ -4796,7 +4796,10 @@ static void camss_detect_legacy_phy(struct camss *camss)
>>          struct device_node *remote;
>>          struct device_node *ep;
>>
>> -       camss->legacy_phy = true;
>> +       /* Legacy CSIPHY description implies the provided CSIPHY resources */
>> +       camss->legacy_phy = !!camss->res->csiphy_res;
>> +       if (!camss->legacy_phy)
>> +               goto done;
>>
>>          /* Find first remote-endpoint and determine if its a PHY */
>>          for_each_endpoint_of_node(camss->dev->of_node, ep) {
>> @@ -4810,6 +4813,7 @@ static void camss_detect_legacy_phy(struct camss *camss)
>>                  break;
>>          }
>>
>> +done:
>>          dev_dbg(camss->dev, "legacy phy mode %s\n",
>>                  camss->legacy_phy ? "true" : "false");
>>   }
>> @@ -4841,7 +4845,7 @@ static int camss_init_subdevices(struct camss *camss)
>>
>>                  if (ret < 0) {
>>                          dev_err(camss->dev, "csiphy %d init fail\n",
>> -                               res->csiphy_res[i].csiphy.id);
>> +                               res->csiphy_res ? res->csiphy_res[i].csiphy.id : i);
>>                          return ret;
>>                  }
>>          }
>> --
>> 2.51.0
>>

[1] https://lore.kernel.org/linux-arm-msm/20261015-glymur_camss_dts-v2-0-e7b1f403146f@oss.qualcomm.com/

-- 
Best wishes,
Vladimir