Re: [Patch 03/19] media: ti-vpe: cal: Add per platform data support

Rob Herring
2019-10-29T13:19:02Z

On Fri, Oct 18, 2019 at 10:34:21AM -0500, Benoit Parrot wrote:
> First this patch adds a method to access the CTRL_CORE_CAMERRX_CONTROL
> register to use the syscon mechanism. For backward compatibility we also
> handle using the existing camerrx_control "reg" entry if a syscon node
> is not found.
>
> In addition the register bit layout for the CTRL_CORE_CAMERRX_CONTROL
> changes depending on the device. In order to support this we need to use
> a register access scheme based on data configuration instead of using
> static macro.
>
> In this case we make use of the regmap facility and create data set
> based on the various device and phy available.
>
> Signed-off-by: Benoit Parrot <bparrot@ti.com>
> ---
> drivers/media/platform/ti-vpe/cal.c | 281 +++++++++++++++++++++-------
> 1 file changed, 212 insertions(+), 69 deletions(-)

> @@ -1816,6 +1911,18 @@ static int cal_probe(struct platform_device *pdev)
> if (!dev)
> return -ENOMEM;
>
> + match = of_match_device(of_match_ptr(cal_of_match), &pdev->dev);

Use of_device_get_match_data() instead.

> + if (!match)
> + return -ENODEV;
> +
> + if (match->data) {
> + dev->data = (struct cal_data *)match->data;
> + dev->flags = dev->data->flags;
> + } else {
> + dev_err(&pdev->dev, "Could not get feature data based on compatible version\n");
> + return -ENODEV;
> + }
> +
> /* set pseudo v4l2 device name so we can use v4l2_printk */
> strscpy(dev->v4l2_dev.name, CAL_MODULE_NAME,
> sizeof(dev->v4l2_dev.name));
> @@ -1823,6 +1930,43 @@ static int cal_probe(struct platform_device *pdev)
> /* save pdev pointer */
> dev->pdev = pdev;
>
> + if (parent && of_property_read_bool(parent, "syscon-camerrx")) {
> + syscon_camerrx =
> + syscon_regmap_lookup_by_phandle(parent,
> + "syscon-camerrx");
> + if (IS_ERR(syscon_camerrx)) {
> + dev_err(&pdev->dev, "failed to get syscon-camerrx regmap\n");
> + return PTR_ERR(syscon_camerrx);
> + }
> +
> + if (of_property_read_u32_index(parent, "syscon-camerrx", 1,
> + &syscon_camerrx_offset)) {

Kind of odd to read the property twice and using functions that don't
match the type. We have functions to retrieve phandle and args.

> + dev_err(&pdev->dev, "failed to get syscon-camerrx offset\n");
> + return -EINVAL;
> + }
> + } else {
> + /*
> + * Backward DTS compatibility.
> + * If syscon entry is not present then check if the
> + * camerrx_control resource is present.
> + */
> + syscon_camerrx = cal_get_camerarx_regmap(dev);
> + if (IS_ERR(syscon_camerrx)) {
> + dev_err(&pdev->dev, "failed to get camerrx_control regmap\n");
> + return PTR_ERR(syscon_camerrx);
> + }
> + /* In this case the base already point to the direct
> + * CM register so no need for an offset
> + */
> + syscon_camerrx_offset = 0;
> + }
> +
> + dev->syscon_camerrx = syscon_camerrx;
> + dev->syscon_camerrx_offset = syscon_camerrx_offset;
> + ret = cal_camerarx_regmap_init(dev);
> + if (ret)
> + return ret;
> +
> dev->res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "cal_top");
> dev->base = devm_ioremap_resource(&pdev->dev, dev->res);