Re: [PATCH v20 5/6] media: qcom: camss: Add support for PHY API devices

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] media: camss: incorrect data rate passed to PHY config
- [Medium] kconfig: select of non-existent symbol PHY_QCOM_MIPI_CSI2
--

commit 53c91247e8179ba522478e54bb98836c106cd0ee
Author: Bryan O'Donoghue <bryan.odonoghue@linaro.org>

media: qcom: camss: Add support for PHY API devices

Add the ability to use a PHY pointer which interacts with the standard PHY
API.

> diff --git a/drivers/media/platform/qcom/camss/Kconfig b/drivers/media/platform/qcom/camss/Kconfig
> index 4eda48cb1adf0..1edc5e5a1829e 100644
> --- a/drivers/media/platform/qcom/camss/Kconfig
> +++ b/drivers/media/platform/qcom/camss/Kconfig
> @@ -7,3 +7,4 @@ config VIDEO_QCOM_CAMSS
>  	select VIDEO_V4L2_SUBDEV_API
>  	select VIDEOBUF2_DMA_SG
>  	select V4L2_FWNODE
> +	select PHY_QCOM_MIPI_CSI2

[Severity: Medium]
Does this Kconfig symbol exist? I am unable to find PHY_QCOM_MIPI_CSI2 anywhere
in the kernel tree. 

Selecting a non-existent symbol has no effect. This could lead to a silent
failure where the required PHY driver is not built, potentially leaving the
hardware non-functional when relying on this driver.

> diff --git a/drivers/media/platform/qcom/camss/camss-csiphy.c b/drivers/media/platform/qcom/camss/camss-csiphy.c
> index 539ac4888b608..aaed7d0309dd6 100644
> --- a/drivers/media/platform/qcom/camss/camss-csiphy.c
> +++ b/drivers/media/platform/qcom/camss/camss-csiphy.c
[ ... ]
> +static int csiphy_stream_on(struct csiphy_device *csiphy)
> +{
> +	u8 bpp = csiphy_get_bpp(csiphy->res->formats->formats, csiphy->res->formats->nformats,
> +				csiphy->fmt[MSM_CSIPHY_PAD_SINK].code);
> +	struct csiphy_lanes_cfg *lncfg = &csiphy->cfg.csi2->lane_cfg;
> +	struct phy_configure_opts_mipi_dphy *dphy_cfg;
> +	union phy_configure_opts dphy_opts = { 0 };
> +	struct device *dev = csiphy->camss->dev;
> +	u8 num_lanes = lncfg->num_data;
> +	s64 link_freq;
> +	int ret;
> +
> +	dphy_cfg = &dphy_opts.mipi_dphy;
> +
> +	link_freq = camss_get_link_freq(&csiphy->subdev.entity, bpp, num_lanes);
> +
> +	if (link_freq <= 0) {
> +		dev_err(dev,
> +			"Cannot get CSI2 transmitter's link frequency\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = phy_mipi_dphy_get_default_config_for_hsclk(link_freq, num_lanes,
> +							 dphy_cfg);

[Severity: High]
Are we passing the correct data rate here? The V4L2 camss_get_link_freq()
helper returns the MIPI D-PHY link frequency (the clock lane frequency in Hz).

However, the generic PHY framework's phy_mipi_dphy_get_default_config_for_hsclk()
function expects its hs_clk_rate argument to be the data rate per lane in bits
per second.

Since MIPI D-PHY uses Double Data Rate, the data rate is exactly twice the
clock frequency. Passing the clock frequency directly during csiphy_stream_on()
causes the calculated Unit Interval (UI) to be twice its correct value. This
will result in completely incorrect MIPI D-PHY timing parameters (hs_prepare,
hs_settle) and could cause physical layer synchronization failures with the
sensor.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918-b4-linux-next-25-03-13-dtsi-x1e80100-camss-v20-0-dc244e124e71@linaro.org?part=5

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