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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250515180616.23ca96fd@jic23-huawei>
Date: Thu, 15 May 2025 18:06:16 +0100
From: Jonathan Cameron <jic23@...nel.org>
To: Matti Vaittinen <mazziesaccount@...il.com>
Cc: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>, Lars-Peter Clausen
 <lars@...afoo.de>, Michael Hennerich <Michael.Hennerich@...log.com>, Rob
 Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor
 Dooley <conor+dt@...nel.org>, linux-iio@...r.kernel.org,
 devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, Mark Brown
 <broonie@...nel.org>, linux-spi@...r.kernel.org
Subject: Re: [PATCH 2/2] iio: adc: ad7476: Support ROHM BU79100G

On Wed, 14 May 2025 12:21:30 +0300
Matti Vaittinen <mazziesaccount@...il.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@...il.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
> 
> 
>