Re: [PATCH v2 1/3] iio: adc: Add Allwinner D1/T113s/R329/T507 SoCs GPADC

[Maxim Kiselev]

пт, 2 июн. 2023 г. в 17:49, Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>:

...

> The locks should be explained (what are they for? what do they protect?).
I added the explanation comment in the next version.

...

> > +static const struct regmap_config sun20i_gpadc_regmap_config = {
> > +     .reg_bits = 32,
> > +     .val_bits = 32,
> > +     .reg_stride = 4,
> > +     .fast_io = true,
>
> I forgot if I asked about regmap lock do you need it?

I think we could drop the regmap altogether. As Andy suggested in
previous series.

...

> > +     if (num_channels > SUN20I_GPADC_MAX_CHANNELS) {
> > +             dev_err(dev, "num of channel children out of range");
> > +             return -EINVAL;
> > +     }
>
> Is it really critical error?

Yes, as Jonathan already noted, this may lead to out of range error.

вс, 4 июн. 2023 г. в 13:46, Jonathan Cameron <jic23@xxxxxxxxxx>:

...

> We try to make this name identify the chip in question.
> If the driver name is sufficient for these platforms then fair enough.
> It should certainly be enough to distinguish this from other ADCs on the
> platform.

I believe the driver name should be enough. All listed SoCs have the
same GPADC register
layout and differ only in the number of channels.