Re: [PATCH v2 2/4] iio: adc: meson-saradc: add support for Meson S4

[Jonathan Cameron <jic23@kernel.org>]

On Tue, 24 Mar 2026 08:07:15 +0100
Krzysztof Kozlowski <krzk@kernel.org> wrote:

> On 23/03/2026 21:05, Jonathan Cameron wrote:
> > On Mon, 23 Mar 2026 08:54:21 +0100
> > Krzysztof Kozlowski <krzk@kernel.org> wrote:
> >   
> >> On Mon, Mar 23, 2026 at 09:34:06AM +0800, Nick Xie wrote:  
> >>> Add support for the SARADC found on the Amlogic Meson S4 SoC.
> >>> According to the documentation and current testing, it is fully
> >>> compatible with the G12A parameter set, so we reuse
> >>> `meson_sar_adc_g12a_data` for this new compatible string.
> >>>
> >>> Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> >>> Signed-off-by: Nick Xie <nick@khadas.com>
> >>> ---
> >>>  drivers/iio/adc/meson_saradc.c | 8 ++++++++
> >>>  1 file changed, 8 insertions(+)
> >>>
> >>> diff --git a/drivers/iio/adc/meson_saradc.c b/drivers/iio/adc/meson_saradc.c
> >>> index 47cd350498a0d..3ac48b7842c4f 100644
> >>> --- a/drivers/iio/adc/meson_saradc.c
> >>> +++ b/drivers/iio/adc/meson_saradc.c
> >>> @@ -1313,6 +1313,11 @@ static const struct meson_sar_adc_data meson_sar_adc_g12a_data = {
> >>>  	.name = "meson-g12a-saradc",
> >>>  };
> >>>  
> >>> +static const struct meson_sar_adc_data meson_sar_adc_s4_data = {
> >>> +	.param = &meson_sar_adc_g12a_param,
> >>> +	.name = "meson-s4-saradc",
> >>> +};
> >>> +
> >>>  static const struct of_device_id meson_sar_adc_of_match[] = {
> >>>  	{
> >>>  		.compatible = "amlogic,meson8-saradc",
> >>> @@ -1341,6 +1346,9 @@ static const struct of_device_id meson_sar_adc_of_match[] = {
> >>>  	}, {
> >>>  		.compatible = "amlogic,meson-g12a-saradc",
> >>>  		.data = &meson_sar_adc_g12a_data,
> >>> +	}, {
> >>> +		.compatible = "amlogic,meson-s4-saradc",    
> >>
> >> The point of compatible devices is to not add such entries. Drop.  
> > It's used for naming in the userspace ABI which is supposed to reflect the part number.  
> 
> Indeed, I saw this pattern in more places. Does userspace need it? There
> is no "compatible" entry shown?

Yes. Userspace uses this (+ a label) to identify which of multiple devices
it is talking to.  Given some of these parts are very specific rather than
covering a general purpose, people tend to be looking at datasheets if there
are multiple parts and they want to know which is which.  Knowing whether
it's compatible with something else doesn't help with that identification
part.  

> 
> If there is no, then probably this could be automated by taking the name
> from compatible after ',', but that would be out of scope for this set,
> so here it is fine.

For DT that works but not for other firmware types.  We had a bunch of bugs
where people ended up putting out ACPI IDs (some of which we've had to leave
in place to avoid regressions).  Hence I've always been nervous about not
encoding the string explicitly in the driver.  Obviously this is a bit
paranoid for DT where it's reasonably tightly defined.

Jonathan

> 
> 
> Best regards,
> Krzysztof