Re: [PATCH v3 5/7] media: qcom: camss: Add support for SM8150

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] media: qcom: camss: NULL deref and dead code in
  csiphy_res_sm8150
- [Medium] media: qcom: camss: CAMSS_8150 used in parent commit before
  definition
--

commit debc7cc0585d6af1f184ab66cdb3fb1863bba5d6
Author: Andrew Gigena <mail@andrewgigena.dev>

media: qcom: camss: Add support for SM8150

This commit adds CAMSS platform data, resource definitions, and version
handling to support the Qualcomm SM8150 SoC.

> 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 dac8d2ecf7995..5e7c2e67ae887 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
> @@ -1133,6 +1133,7 @@ static bool csiphy_is_gen2(u32 version)
>  	case CAMSS_6150:
>  	case CAMSS_6350:
>  	case CAMSS_7280:
> +	case CAMSS_8150:
>  	case CAMSS_8250:
>  	case CAMSS_8280XP:
>  	case CAMSS_8300:
> @@ -1235,6 +1236,7 @@ static int csiphy_init(struct csiphy_device *csiphy)
>  		regs->lane_regs = &lane_regs_sm8250[0];
>  		regs->lane_array_size = ARRAY_SIZE(lane_regs_sm8250);
>  		break;
> +	case CAMSS_8150:
>  	case CAMSS_8280XP:
>  		regs->lane_regs = &lane_regs_sc8280xp[0];
>  		regs->lane_array_size = ARRAY_SIZE(lane_regs_sc8280xp);
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index c54185c93a8a9..da4d2213fd3dc 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -1940,6 +1940,223 @@ static const struct resources_icc icc_res_sm6350[] = {
>  	},
>  };
>  
> +static const struct camss_subdev_resources csiphy_res_sm8150[] = {
> +	/* CSIPHY0 - clocks/regs/supplies owned by PHY driver */
> +	{
> +		.csiphy = {
> +			.id = 0,
> +			.hw_ops = &csiphy_ops_3ph_1_0,
> +			.formats = &csiphy_formats_sdm845
> +		}
> +	},

[Severity: Medium]
Can csiphy_res_sm8150 cause a NULL pointer dereference in
platform_get_resource_byname() when no remote PHY endpoints are connected in
Device Tree?

camss_detect_legacy_phy() defaults camss->legacy_phy to true and only clears
it when it finds an endpoint whose remote parent node is named "phy". If the
CSIPHY ports are omitted in DT, camss_init_subdevices() takes the legacy path
and calls msm_csiphy_subdev_init_legacy(), which passes res->reg[0] (NULL
here) to devm_platform_ioremap_resource_byname().

Conversely, when camss->legacy_phy is false, msm_csiphy_subdev_init() and
csiphy_v4l2_ops never use csiphy->res->hw_ops, making .hw_ops =
&csiphy_ops_3ph_1_0 and the CAMSS_8150 additions in
camss-csiphy-3ph-1-0.c unreachable dead code.

[ ... ]

> diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
> index 7dc2fd9f03e0d..74d8cc2921be9 100644
> --- a/drivers/media/platform/qcom/camss/camss.h
> +++ b/drivers/media/platform/qcom/camss/camss.h
> @@ -89,6 +89,7 @@ enum camss_version {
>  	CAMSS_8x39,
>  	CAMSS_8x53,
>  	CAMSS_8x96,
> +	CAMSS_8150,
>  	CAMSS_8250,
>  	CAMSS_8280XP,
>  	CAMSS_8300,

[Severity: Medium]
Does defining CAMSS_8150 here break the build on the preceding commit
"media: qcom: camss: Match VFE lite clocks by name prefix"?

That earlier commit already adds case CAMSS_8150: in vfe_src_pad_code() and
vfe_bpl_align_rdi() in drivers/media/platform/qcom/camss/camss-vfe.c before
CAMSS_8150 is added to enum camss_version, causing an undeclared identifier
error during git bisect. Moving those two switch cases into this patch keeps
both commits buildable.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106-sm8150-v7.4-rc1-camss-v3-0-0103e8a0bbcd@andrewgigena.dev?part=5

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy