diff mbox

[1/3] ASoC: alc5632: add an of_match table

Message ID 1396291098-13796-1-git-send-email-swarren@wwwdotorg.org (mailing list archive)
State Accepted
Commit c31b0cb1f1a19bc551875e07e9dd7c531ac3580e
Headers show

Commit Message

Stephen Warren March 31, 2014, 6:38 p.m. UTC
From: Stephen Warren <swarren@nvidia.com>

Add a device tree match table. This serves to make the driver's support
of device tree more explicit. Perhaps the fallback for DT matching to
using the i2c_device_id table will go away one day, since it fails in
face of devices from different vendors with the same name.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
 sound/soc/codecs/alc5632.c | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Daniel Mack March 31, 2014, 6:53 p.m. UTC | #1
On 03/31/2014 08:38 PM, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> Add a device tree match table. This serves to make the driver's support
> of device tree more explicit. Perhaps the fallback for DT matching to
> using the i2c_device_id table will go away one day, since it fails in
> face of devices from different vendors with the same name.
> 
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
>  sound/soc/codecs/alc5632.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/sound/soc/codecs/alc5632.c b/sound/soc/codecs/alc5632.c
> index 3ee8d4e41a99..85942ca36cbf 100644
> --- a/sound/soc/codecs/alc5632.c
> +++ b/sound/soc/codecs/alc5632.c
> @@ -1190,11 +1190,18 @@ static const struct i2c_device_id alc5632_i2c_table[] = {
>  };
>  MODULE_DEVICE_TABLE(i2c, alc5632_i2c_table);
>  
> +static const struct of_device_id alc5632_of_match[] = {
> +	{ .compatible = "realtek,alc5632", },

In order to make them usable with the simple-card machine driver, all
codecs that have DT bindings should be visible in Kconfig. Maybe this
can be done in a subsequent patch though.


Daniel
Stephen Warren March 31, 2014, 6:58 p.m. UTC | #2
On 03/31/2014 12:53 PM, Daniel Mack wrote:
> On 03/31/2014 08:38 PM, Stephen Warren wrote:
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> Add a device tree match table. This serves to make the driver's support
>> of device tree more explicit. Perhaps the fallback for DT matching to
>> using the i2c_device_id table will go away one day, since it fails in
>> face of devices from different vendors with the same name.
>>
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>> ---
>>  sound/soc/codecs/alc5632.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/sound/soc/codecs/alc5632.c b/sound/soc/codecs/alc5632.c
>> index 3ee8d4e41a99..85942ca36cbf 100644
>> --- a/sound/soc/codecs/alc5632.c
>> +++ b/sound/soc/codecs/alc5632.c
>> @@ -1190,11 +1190,18 @@ static const struct i2c_device_id alc5632_i2c_table[] = {
>>  };
>>  MODULE_DEVICE_TABLE(i2c, alc5632_i2c_table);
>>  
>> +static const struct of_device_id alc5632_of_match[] = {
>> +	{ .compatible = "realtek,alc5632", },
> 
> In order to make them usable with the simple-card machine driver, all
> codecs that have DT bindings should be visible in Kconfig. Maybe this
> can be done in a subsequent patch though.

I personally don't need to use that machine driver; the drivers patched
in this series are all already in use on Tegra using DT via other
machine drivers.
Thierry Reding March 31, 2014, 7:23 p.m. UTC | #3
On Mon, Mar 31, 2014 at 12:38:16PM -0600, Stephen Warren wrote:
[...]
> diff --git a/sound/soc/codecs/alc5632.c b/sound/soc/codecs/alc5632.c
[...]
> +static const struct of_device_id alc5632_of_match[] = {
> +	{ .compatible = "realtek,alc5632", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, alc5632_of_match);

Doesn't this need #ifdef protection to prevent warnings about this being
unused for !OF?

Thierry
Stephen Warren March 31, 2014, 7:47 p.m. UTC | #4
On 03/31/2014 01:23 PM, Thierry Reding wrote:
> On Mon, Mar 31, 2014 at 12:38:16PM -0600, Stephen Warren wrote:
> [...]
>> diff --git a/sound/soc/codecs/alc5632.c b/sound/soc/codecs/alc5632.c
> [...]
>> +static const struct of_device_id alc5632_of_match[] = {
>> +	{ .compatible = "realtek,alc5632", },
>> +	{ }
>> +};
>> +MODULE_DEVICE_TABLE(of, alc5632_of_match);
> 
> Doesn't this need #ifdef protection to prevent warnings about this being
> unused for !OF?

What I really meant to do was reference these tables directly rather
than using of_match_ptr().

There seems to be a mix of ifdef'ing the table and using of_match_ptr()
vs. the other way around in ASoC. Mark, do you have a preference which
way to go?
Mark Brown March 31, 2014, 10:45 p.m. UTC | #5
On Mon, Mar 31, 2014 at 01:47:04PM -0600, Stephen Warren wrote:

> There seems to be a mix of ifdef'ing the table and using of_match_ptr()
> vs. the other way around in ASoC. Mark, do you have a preference which
> way to go?

of_match_ptr() is supposed to avoid the warnings that the ifdefs fixed
more prettily - I suspect you'll find that the ones that don't use it
either predate of_match_ptr() or copied something that did.
Stephen Warren March 31, 2014, 10:53 p.m. UTC | #6
On 03/31/2014 04:45 PM, Mark Brown wrote:
> On Mon, Mar 31, 2014 at 01:47:04PM -0600, Stephen Warren wrote:
> 
>> There seems to be a mix of ifdef'ing the table and using of_match_ptr()
>> vs. the other way around in ASoC. Mark, do you have a preference which
>> way to go?
> 
> of_match_ptr() is supposed to avoid the warnings that the ifdefs fixed
> more prettily - I suspect you'll find that the ones that don't use it
> either predate of_match_ptr() or copied something that did.

I believe you either have the ifdef and the of_match_ptr(), or you have
neither. The use of of_match_ptr() is required when you wrap the
struct/array in an ifdef, so that the struct/array is only referenced
when it's declared.

I guess from your response you want the struct/array wrapped in an
ifdef, and to use of_match_ptr() when referencing it. I'll go ahead with
that; no need to reply if that's what you meant.
Mark Brown March 31, 2014, 11:05 p.m. UTC | #7
On Mon, Mar 31, 2014 at 04:53:57PM -0600, Stephen Warren wrote:
> On 03/31/2014 04:45 PM, Mark Brown wrote:

> > of_match_ptr() is supposed to avoid the warnings that the ifdefs fixed
> > more prettily - I suspect you'll find that the ones that don't use it
> > either predate of_match_ptr() or copied something that did.

> I believe you either have the ifdef and the of_match_ptr(), or you have
> neither. The use of of_match_ptr() is required when you wrap the
> struct/array in an ifdef, so that the struct/array is only referenced
> when it's declared.

> I guess from your response you want the struct/array wrapped in an
> ifdef, and to use of_match_ptr() when referencing it. I'll go ahead with
> that; no need to reply if that's what you meant.

No, what I'm saying is you shouldn't need the ifdef any more if you use
of_match_ptr() and that's the more modern way to do things.
Mark Brown April 1, 2014, 11:40 a.m. UTC | #8
On Mon, Mar 31, 2014 at 12:38:16PM -0600, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
> 
> Add a device tree match table. This serves to make the driver's support
> of device tree more explicit. Perhaps the fallback for DT matching to
> using the i2c_device_id table will go away one day, since it fails in
> face of devices from different vendors with the same name.

Applied, thanks.
Thierry Reding April 4, 2014, 9:06 a.m. UTC | #9
On Tue, Apr 01, 2014 at 12:05:17AM +0100, Mark Brown wrote:
> On Mon, Mar 31, 2014 at 04:53:57PM -0600, Stephen Warren wrote:
> > On 03/31/2014 04:45 PM, Mark Brown wrote:
> 
> > > of_match_ptr() is supposed to avoid the warnings that the ifdefs fixed
> > > more prettily - I suspect you'll find that the ones that don't use it
> > > either predate of_match_ptr() or copied something that did.
> 
> > I believe you either have the ifdef and the of_match_ptr(), or you have
> > neither. The use of of_match_ptr() is required when you wrap the
> > struct/array in an ifdef, so that the struct/array is only referenced
> > when it's declared.
> 
> > I guess from your response you want the struct/array wrapped in an
> > ifdef, and to use of_match_ptr() when referencing it. I'll go ahead with
> > that; no need to reply if that's what you meant.
> 
> No, what I'm saying is you shouldn't need the ifdef any more if you use
> of_match_ptr() and that's the more modern way to do things.

But I don't think that's the way it works. If you drop the #ifdef around
the struct of_device_id table and use of_match_ptr() when referencing it
then you'll get a warning from the compiler because the table is in fact
unreferenced. So all that of_match_ptr() really gives you is a way to
omit the:

	#else
	#  define of_match_table NULL

So it doesn't really give you all that much in the end.

Thierry
diff mbox

Patch

diff --git a/sound/soc/codecs/alc5632.c b/sound/soc/codecs/alc5632.c
index 3ee8d4e41a99..85942ca36cbf 100644
--- a/sound/soc/codecs/alc5632.c
+++ b/sound/soc/codecs/alc5632.c
@@ -1190,11 +1190,18 @@  static const struct i2c_device_id alc5632_i2c_table[] = {
 };
 MODULE_DEVICE_TABLE(i2c, alc5632_i2c_table);
 
+static const struct of_device_id alc5632_of_match[] = {
+	{ .compatible = "realtek,alc5632", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, alc5632_of_match);
+
 /* i2c codec control layer */
 static struct i2c_driver alc5632_i2c_driver = {
 	.driver = {
 		.name = "alc5632",
 		.owner = THIS_MODULE,
+		.of_match_table = of_match_ptr(alc5632_of_match),
 	},
 	.probe = alc5632_i2c_probe,
 	.remove =  alc5632_i2c_remove,