Re: [PATCH v19 06/12] media: qcom: camss: csiphy: Add support for v2.4.0 two-phase CSIPHY

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] media: camss: NULL dereference in msm_csiphy_subdev_init_legacy on Kaanapali
- [Medium] doc: commit message claims to modify DPHY driver but patch diff only modifies camss.c
--

commit ee4f5ef31347740ffd64c4c9ec08e7e1f2398bfc
Author: Hangxiang Ma <hangxiang.ma@oss.qualcomm.com>

media: qcom: camss: csiphy: Add support for v2.4.0 two-phase CSIPHY

This commit adds support for the v2.4.0 two-phase CSIPHY found on
Kaanapali, defining a new resource structure for its configuration.

>     Add support for the v2.4.0 two-phase CSIPHY found on Kaanapali, where
>     the PHY is driven by the standalone phy-qcom-mipi-csi2 DPHY driver
>     rather than by CAMSS. Add the mipi_csi2_dphy_3nm_kaanapali configuration,
>     which reuses the x1e80100 lane sequence, clocks and supplies but selects
>     the v2.4.0 common status offset, adds Kaanapali power domain and register
>     the "qcom,kaanapali-csi2-phy" compatible.

[Severity: Medium]
Does this patch omit some files, or does the commit message incorrectly
attribute changes that belong to another patch in the series?

The commit message explicitly claims to add the DPHY configuration and
register the compatible, but the actual patch diff only modifies
drivers/media/platform/qcom/camss/camss.c.

> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index fca3de214a922..eeceb79c1e863 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -38,6 +38,51 @@
>  
>  static const struct parent_dev_ops vfe_parent_dev_ops;
>  
> +static const struct camss_subdev_resources csiphy_res_kaanapali[] = {
> +	/* CSIPHY0 */
> +	{
> +		.csiphy = {

[Severity: High]
Does this code cause a NULL pointer dereference on probe if the device tree
lacks endpoints for the camss node?

If a malformed or empty device tree is provided, camss_detect_legacy_phy()
leaves camss->legacy_phy as true.

During camss_init_subdevices(), this triggers a fallback to the legacy
initialization:

camss_init_subdevices() -> msm_csiphy_subdev_init_legacy()

The legacy initialization unconditionally dereferences hw_ops:

ret = csiphy->res->hw_ops->init(csiphy);

Since csiphy_res_kaanapali omits hw_ops here, wouldn't this result in an
immediate kernel panic instead of a clean error during probe?

> +			.id = 0,
> +			.formats = &csiphy_formats_sdm845
> +		},
> +	},
> +	/* CSIPHY1 */
> +	{
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008-kaanapali-camss-v19-0-b2c583ef50e0@oss.qualcomm.com?part=6