Rob Herring <robh@xxxxxxxxxx> wrote on Tue [2019-Oct-29 08:18:55 -0500]:
> 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@xxxxxx>
> > ---
> > 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.
Ok I'll change that.
>
> > + 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.
Yeah, I wanted to make a distinction between the node being present and
any other kind of errors, so we can have a little more precise error
message.
>
> > + 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);
>
