Re: [PATCH v7 9/9] iio: adc: adi-axi-adc: move to backend framework

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, Jan 27, 2024 at 9:21 AM Jonathan Cameron <jic23@xxxxxxxxxx> wrote:
>
> On Tue, 23 Jan 2024 16:14:30 +0100
> Nuno Sa via B4 Relay <devnull+nuno.sa.analog.com@xxxxxxxxxx> wrote:
>
> > From: Nuno Sa <nuno.sa@xxxxxxxxxx>
> >
> > Move to the IIO backend framework. Devices supported by adi-axi-adc now
> > register themselves as backend devices.
> >
> > Signed-off-by: Nuno Sa <nuno.sa@xxxxxxxxxx>
> I'm still not getting the %d vs %c change..
>
> Otherwise LGTM
>
> > -     if (cl->info->version > ver) {
> > +     if (*expected_ver > ver) {
> >               dev_err(&pdev->dev,
> > -                     "IP core version is too old. Expected %d.%.2d.%c, Reported %d.%.2d.%c\n",
> > -                     ADI_AXI_PCORE_VER_MAJOR(cl->info->version),
> > -                     ADI_AXI_PCORE_VER_MINOR(cl->info->version),
> > -                     ADI_AXI_PCORE_VER_PATCH(cl->info->version),
> > +                     "IP core version is too old. Expected %d.%.2d.%d, Reported %d.%.2d.%c\n",
>
> If it's a valid change fine, but then I'd expect both %c to change.
> I'd also expect it to be in a separate patch with an explanation of why.

I was noticing this same pattern in other "AXI" drivers. I think the
reason for the %c is to match the version format in the devicetree
compatible string which looks like "1.00.a". So to fix this we should
probably keep %c and change the value line to
`ADI_AXI_PCORE_VER_PATCH(cl->info->version) + 'a'` to convert it to
the appropriate ascii value.

(But agree that this should be done in a separate patch/)

>
> > +                     ADI_AXI_PCORE_VER_MAJOR(*expected_ver),
> > +                     ADI_AXI_PCORE_VER_MINOR(*expected_ver),
> > +                     ADI_AXI_PCORE_VER_PATCH(*expected_ver),
> >                       ADI_AXI_PCORE_VER_MAJOR(ver),
> >                       ADI_AXI_PCORE_VER_MINOR(ver),
> >                       ADI_AXI_PCORE_VER_PATCH(ver));
> >               return -ENODEV;
> >       }
> >
> > -     indio_dev->info = &adi_axi_adc_info;
> > -     indio_dev->name = "adi-axi-adc";
> > -     indio_dev->modes = INDIO_DIRECT_MODE;
> > -     indio_dev->num_channels = conv->chip_info->num_channels;
> > -     indio_dev->channels = conv->chip_info->channels;
> > -
> > -     ret = adi_axi_adc_config_dma_buffer(&pdev->dev, indio_dev);
> > +     ret = devm_iio_backend_register(&pdev->dev, &adi_axi_adc_generic, st);
> >       if (ret)
> >               return ret;
> >
> > -     ret = adi_axi_adc_setup_channels(&pdev->dev, st);
> > -     if (ret)
> > -             return ret;
> > -
> > -     ret = devm_iio_device_register(&pdev->dev, indio_dev);
> > -     if (ret)
> > -             return ret;
> > -
> > -     dev_info(&pdev->dev, "AXI ADC IP core (%d.%.2d.%c) probed\n",
> > +     dev_info(&pdev->dev, "AXI ADC IP core (%d.%.2d.%d) probed\n",
>
> This should also be in that separate patch fixing up this formatting.
>
> >                ADI_AXI_PCORE_VER_MAJOR(ver),
> >                ADI_AXI_PCORE_VER_MINOR(ver),
> >                ADI_AXI_PCORE_VER_PATCH(ver));
> > @@ -428,6 +229,8 @@ static int adi_axi_adc_probe(struct platform_device *pdev)
> >       return 0;
> >  }
>
>





[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux