Re: [PATCH 2/2] iio: adc: ad7476: Support ROHM BU79100G

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 14 May 2025 12:21:30 +0300
Matti Vaittinen <mazziesaccount@gmail.com> wrote:

> On 14/05/2025 10:38, Matti Vaittinen wrote:
> > On 13/05/2025 11:26, Matti Vaittinen wrote:  
> >> ROHM BU79100G is a 12-bit, single channel ADC. Support reading ADC
> >> measurements using the ad7476.c
> >>
> >> Signed-off-by: Matti Vaittinen <mazziesaccount@gmail.com>
> >> ---
> >>   drivers/iio/adc/ad7476.c | 8 ++++++++
> >>   1 file changed, 8 insertions(+)  
> > 
> > For anyone who might hit this mail thread later:
> > 
> > Conor made me realize that, for now, the BU79100G looks identical to the 
> > ads7866. Thus, these code-changes aren't needed at the moment, and this 
> > patch can be dropped. For those who wish to use BU79100G, please 
> > introduce it as
> > 
> > compatible = "rohm,bu79100g", "ti,ads7866";
> >   
> 
> I was too hasty.
> 
> It seems to me that the fallback won't work with the current driver 
> because the driver is not populating the of_match_table, but is relying 
> solely on the spi_device_id table.
> 
> Judging a quick code reading, the spi_driver_id table entries are 
> matched to the modalias:
> https://elixir.bootlin.com/linux/v6.15-rc6/source/drivers/spi/spi.c#L393
> 
> Which is (as far as I understand), generated from the first compatible:
> https://elixir.bootlin.com/linux/v6.15-rc6/source/drivers/of/base.c#L1170
> 
> and not from the fallback one.
> 
> I suppose this means that we would need to add the of_match_table entry 
> for the ti,ads7866 to make the fallback entry to match the driver.
> 
> But...
> 
> The __spi_register_driver() has following comment:
> 	/*
> 	 * For Really Good Reasons we use spi: modaliases not of:
> 	 * modaliases for DT so module autoloading won't work if we
> 	 * don't have a spi_device_id as well as a compatible string.
> 	 */
> https://elixir.bootlin.com/linux/v6.15-rc6/source/drivers/spi/spi.c#L487
> 
> So, having the of_match_table for would not be sufficient for the 
> autoloading, which would still require the bu79100g to be in the 
> spi_device_id table.
> 
> Am I missing something? I don't see how the Linux SPI drivers benefit 
> from the fallback entries in the dt? (Not saying fallbacks wouldn't be 
> The Right Thing To Do. Ideally DTs aren't for Linux only, maybe some 
> other systems can utilize them). To me it seems I still need to add the 
> spi_device_id entry for the BU79100G, and of_match_table has no 
> additional benefit? If this is right, then this patch is still relevant, 
> even though the binding should be done as in v2.
+CC Mark Brown and linux-spi.


> 
> Yours,
> 	-- Matti
> 
> 
>