linux-kernel - Re: [PATCH 1/3] ASoC: cs4271: Fix cs4271 I2C and SPI drivers automatic module loading

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: <20251021192527.2aea3b89@bootlin.com>
Date: Tue, 21 Oct 2025 19:25:27 +0200
From: Herve Codina <herve.codina@...tlin.com>
To: Alexander Sverdlin <alexander.sverdlin@...il.com>
Cc: David Rhodes <david.rhodes@...rus.com>, Richard Fitzgerald	
 <rf@...nsource.cirrus.com>, Liam Girdwood <lgirdwood@...il.com>, Mark Brown
  <broonie@...nel.org>, Rob Herring <robh@...nel.org>, Krzysztof Kozlowski	
 <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, Jaroslav Kysela	
 <perex@...ex.cz>, Takashi Iwai <tiwai@...e.com>, Nikita Shubin	
 <nikita.shubin@...uefel.me>, Axel Lin <axel.lin@...ics.com>, Brian Austin	
 <brian.austin@...rus.com>, linux-sound@...r.kernel.org,
 patches@...nsource.cirrus.com, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, Thomas Petazzoni
 <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH 1/3] ASoC: cs4271: Fix cs4271 I2C and SPI drivers
 automatic module loading

On Fri, 17 Oct 2025 18:03:13 +0200
Herve Codina <herve.codina@...tlin.com> wrote:

> Hi Alexander,
> 
> On Fri, 17 Oct 2025 17:35:56 +0200
> Alexander Sverdlin <alexander.sverdlin@...il.com> wrote:
> 
> > Hi Herve,
> > 
> > On Fri, 2025-10-17 at 17:10 +0200, Herve Codina wrote:  
> > > > > > > In order to have the I2C or the SPI module loaded automatically, move
> > > > > > > the MODULE_DEVICE_TABLE(of, ...) the core to I2C and SPI parts.
> > > > > > > Also move cs4271_dt_ids itself from the core part to I2C and SPI parts
> > > > > > > as both the call to MODULE_DEVICE_TABLE(of, ...) and the cs4271_dt_ids
> > > > > > > table itself need to be in the same file.        
> > > > > > 
> > > > > > I'm a bit confused by this change.
> > > > > > What do you have in SYSFS "uevent" entry for the real device?      
> > > > > 
> > > > > Here is my uevent content:
> > > > > --- 8<---
> > > > > # cat /sys/bus/i2c/devices/3-0010/uevent 
> > > > > DRIVER=cs4271
> > > > > OF_NAME=cs4271
> > > > > OF_FULLNAME=/i2c@...30000/cs4271@10
> > > > > OF_COMPATIBLE_0=cirrus,cs4271
> > > > > OF_COMPATIBLE_N=1
> > > > > MODALIAS=of:Ncs4271T(null)Ccirrus,cs4271
> > > > > # 
> > > > > --- 8< ---      
> > > > 
> > > > that's what I get with SPI-connected CS4271, and this is actually what I'd
> > > > expect (linux-next as of 2433b8476165):
> > > > 
> > > > # cat /sys/bus/spi/devices/spi0.0/uevent
> > > > DRIVER=cs4271
> > > > OF_NAME=codec
> > > > OF_FULLNAME=/soc/spi@...a0000/codec@0
> > > > OF_COMPATIBLE_0=cirrus,cs4271
> > > > OF_COMPATIBLE_N=1
> > > > MODALIAS=spi:cs4271    
> > > 
> > > So, this is without my patch applied.    
> > 
> > this is the modalias of the device, it doesn't depend on your patch series.
> > 
> > I'd say that modalias for SPI device is correct but commit c973b8a7dc50
> > lacks MODULE_DEVICE_TABLE(spi, ...) in the driver.
> > 
> > I'd argue that I2C modalias is correct in the driver:
> > 
> > # modinfo snd-soc-cs4271-i2c
> > ...
> > alias:          i2c:cs4271
> > 
> > But I still have to understand what happened to I2C core.
> >   
> > > I don't have any CS4271 connected on SPI bus to perform the same test
> > > with my patch applied.    
> >   
> 
> In my next iteration, I will fix the MODULE_DEVICE_TABLE() in cs4271-spi.c
> replacing
>   MODULE_DEVICE_TABLE(of, cs4271_dt_ids);
> by
>   MODULE_DEVICE_TABLE(spi, cs4271_dt_ids);

I have the feeling that this was not correct.

Looking at several SPI driver, both
MODULE_DEVICE_TABLE(of, ...) and MODULE_DEVICE_TABLE(spi, ...) are present.

I should keep the MODULE_DEVICE_TABLE(of, cs4271_dt_ids) in the cs4271-spi.c
but a MODULE_DEVICE_TABLE(spi, ...) is missing.

I plan to add the following for the cs4271-spi.c file:
--- 8< ---
static const struct spi_device_id cs4271_spi_id[] = {
	{ "cs4271" },
	{ }
};
MODULE_DEVICE_TABLE(spi, cs4271_spi_id);

static struct spi_driver cs4271_spi_driver = {
	...
	.id_table = cs4271_spi_id,     /* <---- This was also missing */
};

--- 8< ---

Also no change on cs4271-i2c.c file as both MODULE_DEVICE_TABLE(i2c, ...)
and MODULE_DEVICE_TABLE(of, ...) are already present.

Looking at other drivers in the kernel for devices that can be available on
SPI or I2C busses, a lot of them have the same MODULE_DEVICE_TABLE(of, ...)
for the SPI and the I2C parts and a specific MODULE_DEVICE_TABLE(spi, ...)
for the SPI part or MODULE_DEVICE_TABLE(i2c, ...) for the I2C part.

My next iteration will be consistent with this pattern.
- cs4271-spi.c:
    The cs4271_spi_id table contains { "cs4271" }
    The cs4271_dt_ids table contains { .compatible = "cirrus,cs4271" }
    
    MODULE_DEVICE_TABLE(spi, cs4271_spi_id);
    MODULE_DEVICE_TABLE(of, cs4271_dt_ids);

- cs4271-i2c.c
    The cs4271_i2c_id table contains { "cs4271" }
    The cs4271_dt_ids table contains { .compatible = "cirrus,cs4271" }

    MODULE_DEVICE_TABLE(i2c, cs4271_i2c_id);
    MODULE_DEVICE_TABLE(of, cs4271_dt_ids);

Best regards,
Hervé