[PATCH] ASoC: soc-dai: don't use dev_{set/get}_drvdata() on DAI

[PATCH] ASoC: soc-dai: don't use dev_{set/get}_drvdata() on DAI

[Kuninori Morimoto]

We have 2 type of driver data set functions.

(A)	snd_soc_component_set_drvdata()
(B)	snd_soc_dai_set_drvdata()

Both are using dev_set_drvdata() with own dev

(A)	static inline void snd_soc_component_set_drvdata(...)
	{
		dev_set_drvdata(component->dev, data);
	}			^^^^^^^^^^^^^^

(B)	static inline void snd_soc_dai_set_drvdata(...)
	{
		dev_set_drvdata(dai->dev, data);
	}			^^^^^^^^

But, these are same dev.

	struct snd_soc_dai *snd_soc_register_dai(...)
	{
(A)		struct device *dev = component->dev;
		...
(B)		dai->dev = dev;
		...
	}

This means, if (A) and (B) were called in the same time, private data
will be overwritten.

But, in the same time, commit 5f86d41d0410 ("ASoC: soc-dai: Add private
data to snd_soc_dai") added dai->priv for private data, and sdca_asoc.c
is only user of it.

We can re-use this dai->priv for snd_soc_dai_set_drvdata() (B) instead,
and avoid the issue.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/soc-dai.h    | 4 ++--
 sound/soc/sdca/sdca_asoc.c | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
index 224396927aef3..2b006d3de777a 100644
--- a/include/sound/soc-dai.h
+++ b/include/sound/soc-dai.h
@@ -536,12 +536,12 @@ static inline unsigned int snd_soc_dai_stream_active(const struct snd_soc_dai *d
 static inline void snd_soc_dai_set_drvdata(struct snd_soc_dai *dai,
 		void *data)
 {
-	dev_set_drvdata(dai->dev, data);
+	dai->priv = data;
 }
 
 static inline void *snd_soc_dai_get_drvdata(struct snd_soc_dai *dai)
 {
-	return dev_get_drvdata(dai->dev);
+	return dai->priv;
 }
 
 /**
diff --git a/sound/soc/sdca/sdca_asoc.c b/sound/soc/sdca/sdca_asoc.c
index a342a4e56717a..c53c74952069f 100644
--- a/sound/soc/sdca/sdca_asoc.c
+++ b/sound/soc/sdca/sdca_asoc.c
@@ -1456,7 +1456,7 @@ int sdca_asoc_set_constraints(struct device *dev, struct regmap *regmap,
 		return ret;
 	}
 
-	dai->priv = constraint;
+	snd_soc_dai_set_drvdata(dai, constraint);
 
 	return 0;
 }
@@ -1472,7 +1472,7 @@ EXPORT_SYMBOL_NS(sdca_asoc_set_constraints, "SND_SOC_SDCA");
 void sdca_asoc_free_constraints(struct snd_pcm_substream *substream,
 				struct snd_soc_dai *dai)
 {
-	struct snd_pcm_hw_constraint_list *constraint = dai->priv;
+	struct snd_pcm_hw_constraint_list *constraint = snd_soc_dai_get_drvdata(dai);
 
 	kfree(constraint);
 }
-- 
2.43.0

Re: [PATCH] ASoC: soc-dai: don't use dev_{set/get}_drvdata() on DAI

[Charles Keepax]

On Thu, Mar 05, 2026 at 01:56:44AM +0000, Kuninori Morimoto wrote:
> We have 2 type of driver data set functions.
> 
> (A)	snd_soc_component_set_drvdata()
> (B)	snd_soc_dai_set_drvdata()
> 
> Both are using dev_set_drvdata() with own dev
> 
> (A)	static inline void snd_soc_component_set_drvdata(...)
> 	{
> 		dev_set_drvdata(component->dev, data);
> 	}			^^^^^^^^^^^^^^
> 
> (B)	static inline void snd_soc_dai_set_drvdata(...)
> 	{
> 		dev_set_drvdata(dai->dev, data);
> 	}			^^^^^^^^
> 
> But, these are same dev.
> 
> 	struct snd_soc_dai *snd_soc_register_dai(...)
> 	{
> (A)		struct device *dev = component->dev;
> 		...
> (B)		dai->dev = dev;
> 		...
> 	}
> 
> This means, if (A) and (B) were called in the same time, private data
> will be overwritten.
> 
> But, in the same time, commit 5f86d41d0410 ("ASoC: soc-dai: Add private
> data to snd_soc_dai") added dai->priv for private data, and sdca_asoc.c
> is only user of it.
> 
> We can re-use this dai->priv for snd_soc_dai_set_drvdata() (B) instead,
> and avoid the issue.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---

Hm... the patch looks pretty reasonable to me but does seem to
cause issues on my system. I will try to find some time to
investigate further:

Mar 05 13:06:17.645064 ck-upx-mtl kernel: i915 0000:00:02.0: [drm] *ERROR* DC state mismatch (0x0 -> 0x2)
Mar 05 13:06:17.647247 ck-upx-mtl kernel: BUG: kernel NULL pointer dereference, address: 0000000000000000
Mar 05 13:06:17.647257 ck-upx-mtl kernel: #PF: supervisor read access in kernel mode
Mar 05 13:06:17.647265 ck-upx-mtl kernel: #PF: error_code(0x0000) - not-present page
Mar 05 13:06:17.647270 ck-upx-mtl kernel: PGD 0 P4D 0 
Mar 05 13:06:17.647276 ck-upx-mtl kernel: Oops: Oops: 0000 [#1] SMP NOPTI
Mar 05 13:06:17.647281 ck-upx-mtl kernel: CPU: 0 UID: 0 PID: 98 Comm: (udev-worker) Tainted: G            E 7.0.0-rc2-mykernel #1 PREEMPT(lazy) Mar 05 13:06:17.647286 ck-upx-mtl kernel: Tainted: [E]=UNSIGNED_MODULE
Mar 05 13:06:17.647291 ck-upx-mtl kernel: Hardware name: AAEON UPX-MTL01/UPX-MTL01, BIOS UXMTAM13 02/13/2025
Mar 05 13:06:17.647295 ck-upx-mtl kernel: RIP: 0010:intel_dmc_update_dc6_allowed_count+0x17/0xb0 [i915]

I wonder if maybe something is mixing these functions and
expecting them to set the same thing?

Thanks,
Charles

Re: [PATCH] ASoC: soc-dai: don't use dev_{set/get}_drvdata() on DAI

[Charles Keepax]

On Thu, Mar 05, 2026 at 01:14:30PM +0000, Charles Keepax wrote:
> On Thu, Mar 05, 2026 at 01:56:44AM +0000, Kuninori Morimoto wrote:
> > We can re-use this dai->priv for snd_soc_dai_set_drvdata() (B) instead,
> > and avoid the issue.
> 
> Hm... the patch looks pretty reasonable to me but does seem to
> cause issues on my system. I will try to find some time to
> investigate further:
> 
> Mar 05 13:06:17.645064 ck-upx-mtl kernel: i915 0000:00:02.0: [drm] *ERROR* DC state mismatch (0x0 -> 0x2)
> Mar 05 13:06:17.647247 ck-upx-mtl kernel: BUG: kernel NULL pointer dereference, address: 0000000000000000
> Mar 05 13:06:17.647257 ck-upx-mtl kernel: #PF: supervisor read access in kernel mode
> Mar 05 13:06:17.647265 ck-upx-mtl kernel: #PF: error_code(0x0000) - not-present page
> Mar 05 13:06:17.647270 ck-upx-mtl kernel: PGD 0 P4D 0 
> Mar 05 13:06:17.647276 ck-upx-mtl kernel: Oops: Oops: 0000 [#1] SMP NOPTI
> Mar 05 13:06:17.647281 ck-upx-mtl kernel: CPU: 0 UID: 0 PID: 98 Comm: (udev-worker) Tainted: G            E 7.0.0-rc2-mykernel #1 PREEMPT(lazy) Mar 05 13:06:17.647286 ck-upx-mtl kernel: Tainted: [E]=UNSIGNED_MODULE
> Mar 05 13:06:17.647291 ck-upx-mtl kernel: Hardware name: AAEON UPX-MTL01/UPX-MTL01, BIOS UXMTAM13 02/13/2025
> Mar 05 13:06:17.647295 ck-upx-mtl kernel: RIP: 0010:intel_dmc_update_dc6_allowed_count+0x17/0xb0 [i915]
> 
> I wonder if maybe something is mixing these functions and
> expecting them to set the same thing?

Ah ok yeah looking a little further it seems an awful lot of
machine drivers use snd_soc_dai_get_drvdata but very few use
snd_soc_dai_set_drvdata, so I think a large amount of the world
is relying on these two functions accessing the same stuff.

Thanks,
Charles

Re: [PATCH] ASoC: soc-dai: don't use dev_{set/get}_drvdata() on DAI

[Kuninori Morimoto]

Hi Charles
Cc Mark

Thank you for testing

> Ah ok yeah looking a little further it seems an awful lot of
> machine drivers use snd_soc_dai_get_drvdata but very few use
> snd_soc_dai_set_drvdata, so I think a large amount of the world
> is relying on these two functions accessing the same stuff.

Grr.. indeed...

Actually, I'm thinking this drvdata (= using dev_set_drvdata()) will be
issue in the future.
For example, below code *looks* very normal. It looks set each private
data to each struct. But all are setting private data to same dev.
It is difficult to notice.

	static int probe(struct platform_device *pdev)
	{
		...
		// set priv to pdev->dev;
		dev_set_drvdata(pdev->dev, priv);

		...

		// set component data to component
		for (...)
			snd_soc_component_set_drvdata(component + i, component_data + i);

		// set dai data to dai
		for (...)
			snd_soc_dai_set_drvdata(dai + i, dai_data + i);

	}

And now, we are already using unbalanced set/get...

	dev_set_drvdata(dai->dev, data);
	...
	data = snd_soc_dai_get_drvdata(dai);

I think it would be better to remove
snd_soc_{dai/component}_{set/get}_drvdata() ?
At least we can regain the balance.

	dev_set_drvdata(dai->dev, data);
	...
-	data = snd_soc_dai_get_drvdata(dai);
+	data = dev_get_drvdata(dai->dev);

But what do you think ?

Thank you for your help !!

Best regards
---
Kuninori Morimoto

Re: [PATCH] ASoC: soc-dai: don't use dev_{set/get}_drvdata() on DAI

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 840 bytes --]

On Thu, Mar 05, 2026 at 11:59:28PM +0000, Kuninori Morimoto wrote:

> I think it would be better to remove
> snd_soc_{dai/component}_{set/get}_drvdata() ?
> At least we can regain the balance.

> 	dev_set_drvdata(dai->dev, data);
> 	...
> -	data = snd_soc_dai_get_drvdata(dai);
> +	data = dev_get_drvdata(dai->dev);

> But what do you think ?

IIRC the original demand for those functions was that drivers were doing
a chain of lookups from whatever API they were working with to get back
to their driver data and that this was before there was a convenient
struct device around in all those structs.  With a struct device right
there it's less clear that's buying us much, and now you mention it I
can see the potential for surprises if people think each struct has it's
own driver data.  Don't know what other people's opinion is though?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: soc-dai: don't use dev_{set/get}_drvdata() on DAI

[Kuninori Morimoto]

Hi

> > 	dev_set_drvdata(dai->dev, data);
> > 	...
> > -	data = snd_soc_dai_get_drvdata(dai);
> > +	data = dev_get_drvdata(dai->dev);
> 
> > But what do you think ?
> 
> IIRC the original demand for those functions was that drivers were doing
> a chain of lookups from whatever API they were working with to get back
> to their driver data and that this was before there was a convenient
> struct device around in all those structs.  With a struct device right
> there it's less clear that's buying us much, and now you mention it I
> can see the potential for surprises if people think each struct has it's
> own driver data.  Don't know what other people's opinion is though?

In my quick check, rather than setting DAI-specific data, it seems the driver's
private data is set to dev during probe(), and is used in each functions.

Because there is no diff between dai->dev / component->dev, I'm thinking
that we need to remove dai->dev (= replace to dai->component->dev) to
avoid confusion.

I will do like below.
	- replace snd_soc_{dai/component}_get_drvdata() to dev_get_drvdata()
	- replace dai->dev -> dai->component->dev
	- remove dai->dev

Thank you for your help !!

Best regards
---
Kuninori Morimoto