devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexander Sverdlin <alexander.sverdlin@gmail.com>
To: Herve Codina <herve.codina@bootlin.com>
Cc: David Rhodes <david.rhodes@cirrus.com>,
	Richard Fitzgerald	 <rf@opensource.cirrus.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>, Rob Herring <robh@kernel.org>,
	Krzysztof Kozlowski	 <krzk+dt@kernel.org>,
	Conor Dooley <conor+dt@kernel.org>,
	Jaroslav Kysela	 <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>,
	Nikita Shubin	 <nikita.shubin@maquefel.me>,
	Axel Lin <axel.lin@ingics.com>,
	Brian Austin	 <brian.austin@cirrus.com>,
	linux-sound@vger.kernel.org,  patches@opensource.cirrus.com,
	devicetree@vger.kernel.org,  linux-kernel@vger.kernel.org,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Subject: Re: [PATCH 1/3] ASoC: cs4271: Fix cs4271 I2C and SPI drivers automatic module loading
Date: Fri, 17 Oct 2025 16:41:53 +0200	[thread overview]
Message-ID: <4b851d47bf1d03988a27671ae21208cdeed76837.camel@gmail.com> (raw)
In-Reply-To: <d6bd466a5d11b016183db0ac3c25185fad3036fc.camel@gmail.com>

Hi Herve,

On Fri, 2025-10-17 at 15:25 +0200, Alexander Sverdlin 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@ff130000/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@808a0000/codec@0
> OF_COMPATIBLE_0=cirrus,cs4271
> OF_COMPATIBLE_N=1
> MODALIAS=spi:cs4271
> 
> > > If you consider spi_uevent() and i2c_device_uevent(), "MODALIAS=" in the
> > > "uevent" should be prefixed with either "spi:" or "i2c:".
> > > And this isn't what you adress in your patch.
> > > 
> > > You provide [identical] "of:" prefixed modalias to two different modules
> > > (not sure, how this should work), but cs4271 is not an MMIO device,
> > > so it should not generate an "of:" prefixed uevent.
> > > 
> > > Could you please show the relevant DT snippet for the affected HW?
> > 
> > And this is the related DT part:
> > --- 8< ---
> > &i2c3 {
> >   status = "okay";
> > 
> >   cs4271@10 {
> >   compatible = "cirrus,cs4271";
> >   reg = <0x10>;
> >   clocks = <&cru SCLK_I2S_8CH_OUT>;
> >   clock-names = "mclk";
> > 
> >   ...
> >   };
> > };
> > --- 8< ---
> > 
> > i2c3 is the following node:
> > https://elixir.bootlin.com/linux/v6.17.1/source/arch/arm64/boot/dts/rockchip/rk3399-base.dtsi#L732
> 
> the above looks OK to me on the first glance, I'm really puzzled what
> is the reason for "of:" prefixed MODALIAS in the uevent for an i2c device.
> 
> I still believe, that the culprit is the creation of a platform device
> from the DT for an i2c device.
> 
> I don't have any real I2C-connected CS4271, but I think I could fake one
> in any DT just to verify how uevents would look like on my side.

indeed, that's what I've got for a fake I2C device:

# cat /sys/bus/i2c/devices/0-0010/uevent 
OF_NAME=cs4271
OF_FULLNAME=/soc/i2c@4000000/cs4271@10
OF_COMPATIBLE_0=cirrus,cs4271
OF_COMPATIBLE_N=1
MODALIAS=of:Ncs4271T(null)Ccirrus,cs4271

to me it looks like a bug somewhere in I2C core...

-- 
Alexander Sverdlin.

  reply	other threads:[~2025-10-17 14:41 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-16 13:03 [PATCH 0/3] Add support for an external Master Clock in the Cirrus CS4271 codec Herve Codina
2025-10-16 13:03 ` [PATCH 1/3] ASoC: cs4271: Fix cs4271 I2C and SPI drivers automatic module loading Herve Codina
2025-10-16 18:40   ` Alexander Sverdlin
2025-10-17  6:32     ` Herve Codina
2025-10-17 13:25       ` Alexander Sverdlin
2025-10-17 14:41         ` Alexander Sverdlin [this message]
2025-10-17 15:01           ` Mark Brown
2025-10-17 18:14             ` Alexander Sverdlin
2025-10-21 19:00               ` Mark Brown
2025-10-21 19:12                 ` Alexander Sverdlin
2025-10-22 14:56                   ` Mark Brown
2025-10-22 17:46                     ` Alexander Sverdlin
2025-10-22 18:01                       ` Mark Brown
2025-10-23 11:40                       ` Javier Martinez Canillas
2025-10-23 12:32                         ` Alexander Sverdlin
2025-10-17 15:10         ` Herve Codina
2025-10-17 15:35           ` Alexander Sverdlin
2025-10-17 16:03             ` Herve Codina
2025-10-21 17:25               ` Herve Codina
2025-10-16 13:03 ` [PATCH 2/3] ASoC: dt-bindings: cirrus,cs4271: Document mclk clock Herve Codina
2025-10-22  7:57   ` Krzysztof Kozlowski
2025-10-16 13:03 ` [PATCH 3/3] ASoC: cs4271: Add support for the external mclk Herve Codina
2025-10-16 19:14   ` Alexander Sverdlin
2025-10-16 19:17   ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4b851d47bf1d03988a27671ae21208cdeed76837.camel@gmail.com \
    --to=alexander.sverdlin@gmail.com \
    --cc=axel.lin@ingics.com \
    --cc=brian.austin@cirrus.com \
    --cc=broonie@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=david.rhodes@cirrus.com \
    --cc=devicetree@vger.kernel.org \
    --cc=herve.codina@bootlin.com \
    --cc=krzk+dt@kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=nikita.shubin@maquefel.me \
    --cc=patches@opensource.cirrus.com \
    --cc=perex@perex.cz \
    --cc=rf@opensource.cirrus.com \
    --cc=robh@kernel.org \
    --cc=thomas.petazzoni@bootlin.com \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).