| Message ID | 1396291098-13796-1-git-send-email-swarren@wwwdotorg.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | c31b0cb1f1a19bc551875e07e9dd7c531ac3580e |
| Headers | show |
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
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.
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
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?
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.
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.
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.
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.
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 --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,