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

[Bryan O'Donoghue <bryan.odonoghue@linaro.org>]

On 17/07/2026 15:20, 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)
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>   drivers/media/platform/qcom/camss/camss.c | 34 +++++++++++++++++++++++++++++++
>   drivers/media/platform/qcom/camss/camss.h |  1 +
>   2 files changed, 35 insertions(+)
> 
> diff --git a/drivers/media/platform/qcom/camss/camss.c b/drivers/media/platform/qcom/camss/camss.c
> index 1e97b0b7ec463f1fa2615704d09ce465a8964176..2caa253fc4ad48f7f04580417d15553a710cbc6f 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,25 @@ void camss_pm_domain_off(struct camss *camss, int id)
>   	}
>   }
>   
> +static void camss_read_version(struct camss *camss)
> +{
> +	u32 hw_version;
> +
> +	if (!camss->top_base || !camss->res->pm_clks[0])
> +		return;
> +
> +	if (pm_runtime_resume_and_get(camss->dev))
> +		return;

If this fails, you're in real trouble.

Surely this wants to be a return -EBADSTUFF;

> +
> +	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;
> +}
> +
>   static int vfe_parent_dev_ops_get(struct camss *camss, int id)
>   {
>   	int ret = -EINVAL;
> @@ -4866,6 +4888,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;
> +	}
> +
>   	for (i = 0; i < camss->res->csid_num; i++) {
>   		ret = msm_csid_subdev_init(camss, &camss->csid[i],
>   					   &res->csid_res[i], i);
> @@ -5454,6 +5486,8 @@ static int camss_probe(struct platform_device *pdev)
>   	if (ret)
>   		goto err_v4l2_device_unregister;
>   
> +	camss_read_version(camss);
> +

Make it an int a bug out if it fails.

>   	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;
>