Re: [PATCH v2 4/4] media: qcom: camss: Report hardware version via media controller

[Frank Li]

On Tue, Jul 21, 2026 at 03:21:11PM +0200, Loic Poulain wrote:
> Populate media_dev.hw_revision with the top-level CAMSS hardware version
> so that user space can query the exact hardware variant through the media
> controller (e.g. via media-ctl or MEDIA_IOC_DEVICE_INFO). This can help
> identify the platform variant and adapt behaviour accordingly, for example
> allowing libcamera to apply quirks or enable features that cannot be
> discovered through standard V4L2 or media-controller APIs.
>
> The version register is exposed by some platforms through the top-level
> "top" reg region (offset 0). When present, map it and read the version
> once at probe. This is a no-op on platforms not mapping the region (yet).
>
> Reported media info on Agatti/CM2290 (Spectra 520):
> Media Driver Info:
>         Driver name      : qcom-camss
>         Model            : Qualcomm Camera Subsystem
>         Bus info         : platform:5c11000.camss
>         Hardware revision: 0x00050200 (328192)
>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxxxx>
> Signed-off-by: Loic Poulain <loic.poulain@xxxxxxxxxxxxxxxx>
> ---
>  drivers/media/platform/qcom/camss/camss.c | 40 +++++++++++++++++++++++++++++++
>  drivers/media/platform/qcom/camss/camss.h |  1 +
>  2 files changed, 41 insertions(+)
>
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index 1f691b8940a9b3d8a9c03cb3159ed711d0d18e77..caf11ea21a60b5e10644d3daebbde7371d549ff5 100644
> --- a/drivers/media/platform/qcom/camss/camss.c
> +++ b/drivers/media/platform/qcom/camss/camss.c
> @@ -33,6 +33,9 @@
>  #define CAMSS_CLOCK_MARGIN_NUMERATOR 105
>  #define CAMSS_CLOCK_MARGIN_DENOMINATOR 100
>
> +/* Top-level CAMSS version register */
> +#define CAMSS_HW_VERSION		0x0
> +
>  static const struct parent_dev_ops vfe_parent_dev_ops;
>
>  static const struct camss_subdev_resources csiphy_res_8x16[] = {
> @@ -4677,6 +4680,29 @@ void camss_pm_domain_off(struct camss *camss, int id)
>  	}
>  }
>
> +static int camss_read_version(struct camss *camss)
> +{
> +	u32 hw_version;
> +	int ret;
> +
> +	if (!camss->top_base || !camss->res->pm_clks[0])
> +		return 0;
> +
> +	ret = pm_runtime_resume_and_get(camss->dev);
> +	if (ret < 0)
> +		return ret;
> +
> +	hw_version = readl_relaxed(camss->top_base + CAMSS_HW_VERSION);
> +
> +	pm_runtime_put_sync(camss->dev);
> +
> +	dev_dbg(camss->dev, "CAMSS HW Version = 0x%08x\n", hw_version);
> +
> +	camss->media_dev.hw_revision = hw_version;
> +
> +	return 0;
> +}
> +
>  static int vfe_parent_dev_ops_get(struct camss *camss, int id)
>  {
>  	int ret = -EINVAL;
> @@ -4866,6 +4892,16 @@ static int camss_init_subdevices(struct camss *camss)
>  		camss->csid_wrapper_base = base;
>  	}
>
> +	/* Optional top register for hardware version info */
> +	if (platform_get_resource_byname(pdev, IORESOURCE_MEM, "top")) {
> +		void __iomem *base;
> +
> +		base = devm_platform_ioremap_resource_byname(pdev, "top");
> +		if (IS_ERR(base))
> +			return PTR_ERR(base);
> +		camss->top_base = base;


The read_version() only do once, if move to here and pass down base, needn't
save it to cmass

		ret = camss_read_version(camss, base);
		if (ret)
			return ret;

reduce one goto branch, which is quite easy to make mistake.

Frank


> +	}
> +
>  	for (i = 0; i < camss->res->csid_num; i++) {
>  		ret = msm_csid_subdev_init(camss, &camss->csid[i],
>  					   &res->csid_res[i], i);
> @@ -5456,6 +5492,10 @@ static int camss_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err_v4l2_device_unregister;
>
> +	ret = camss_read_version(camss);
> +	if (ret)
> +		goto err_v4l2_device_unregister;
> +
>  	ret = camss_parse_ports(camss);
>  	if (ret < 0)
>  		goto err_v4l2_device_unregister;
> diff --git a/drivers/media/platform/qcom/camss/camss.h b/drivers/media/platform/qcom/camss/camss.h
> index fe5fe25d5f18d8a3ce35b48077a975f1453c341f..cce79e56f58c8bdc0097f4a1702df22c4248f2fa 100644
> --- a/drivers/media/platform/qcom/camss/camss.h
> +++ b/drivers/media/platform/qcom/camss/camss.h
> @@ -133,6 +133,7 @@ struct camss {
>  	struct ispif_device *ispif;
>  	struct vfe_device *vfe;
>  	void __iomem *csid_wrapper_base;
> +	void __iomem *top_base;
>  	atomic_t ref_count;
>  	int genpd_num;
>  	struct device *genpd;
>
> --
> 2.34.1
>