[PATCH] ASoc: cs42xx8-i2c.c: add module device table for of
When trying to connect the device with the driver through of it is not working. The of_device_id is defined in cs42xx8.c but is not correctly included in cs42xx8-i2c.c. Also add the matching table for of in the i2c file.
Signed-off-by: Peter Bergin peter@berginkonsult.se --- sound/soc/codecs/cs42xx8-i2c.c | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/sound/soc/codecs/cs42xx8-i2c.c b/sound/soc/codecs/cs42xx8-i2c.c index cb06a06d48b0..6e8ee28d01f8 100644 --- a/sound/soc/codecs/cs42xx8-i2c.c +++ b/sound/soc/codecs/cs42xx8-i2c.c @@ -37,6 +37,13 @@ static int cs42xx8_i2c_remove(struct i2c_client *i2c) return 0; }
+const struct of_device_id cs42xx8_of_match[] = { + { .compatible = "cirrus,cs42448", .data = &cs42448_data, }, + { .compatible = "cirrus,cs42888", .data = &cs42888_data, }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, cs42xx8_of_match); + static struct i2c_device_id cs42xx8_i2c_id[] = { {"cs42448", (kernel_ulong_t)&cs42448_data}, {"cs42888", (kernel_ulong_t)&cs42888_data},
On Thu, Oct 27, 2022 at 01:50:56PM +0200, Peter Bergin wrote:
When trying to connect the device with the driver through of it is not working. The of_device_id is defined in cs42xx8.c but is not correctly included in cs42xx8-i2c.c. Also add the matching table for of in the i2c file.
Signed-off-by: Peter Bergin peter@berginkonsult.se
sound/soc/codecs/cs42xx8-i2c.c | 7 +++++++ 1 file changed, 7 insertions(+)
This should move the ID table out of cs42xx8.c, not just duplicate it.
Hi Peter,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on v6.0] [also build test ERROR on linus/master next-20221027] [cannot apply to broonie-sound/for-next v6.1-rc2 v6.1-rc1] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Peter-Bergin/ASoc-cs42xx8-i2c... patch link: https://lore.kernel.org/r/20221027115057.442925-1-peter%40berginkonsult.se patch subject: [PATCH] ASoc: cs42xx8-i2c.c: add module device table for of config: s390-allyesconfig compiler: s390-linux-gcc (GCC) 12.1.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/a09fe8d3c2a7a2270c0ec1582ed182... git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Peter-Bergin/ASoc-cs42xx8-i2c-c-add-module-device-table-for-of/20221027-195337 git checkout a09fe8d3c2a7a2270c0ec1582ed182d747f0d9b8 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=s390 SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot lkp@intel.com
All errors (new ones prefixed by >>):
s390-linux-ld: sound/soc/codecs/cs42xx8-i2c.o:(.data.rel.ro+0x0): multiple definition of `cs42xx8_of_match'; sound/soc/codecs/cs42xx8.o:(.data.rel.ro.local+0x540): first defined here
Hi Peter,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on v6.0] [also build test ERROR on linus/master next-20221027] [cannot apply to broonie-sound/for-next v6.1-rc2 v6.1-rc1] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Peter-Bergin/ASoc-cs42xx8-i2c... patch link: https://lore.kernel.org/r/20221027115057.442925-1-peter%40berginkonsult.se patch subject: [PATCH] ASoc: cs42xx8-i2c.c: add module device table for of config: mips-allyesconfig compiler: mips-linux-gcc (GCC) 12.1.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/a09fe8d3c2a7a2270c0ec1582ed182... git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Peter-Bergin/ASoc-cs42xx8-i2c-c-add-module-device-table-for-of/20221027-195337 git checkout a09fe8d3c2a7a2270c0ec1582ed182d747f0d9b8 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=mips SHELL=/bin/bash
If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot lkp@intel.com
All errors (new ones prefixed by >>):
mips-linux-ld: sound/soc/codecs/cs42xx8-i2c.o:(.rodata.cs42xx8_of_match+0x0): multiple definition of `cs42xx8_of_match'; sound/soc/codecs/cs42xx8.o:(.rodata.cs42xx8_of_match+0x0): first defined here
When trying to connect the device with the driver through device-tree it is not working. The of_device_id is defined in cs42xx8.c but is not correctly included in cs42xx8-i2c.c.
Move of_device_id table to cs42xx8-i2c.c. Get cs42xx8_driver_data in cs42xx8_i2c_probe() and pass as argument to cs42xx8_probe(). Move error check if no driver data found to cs42xx8_i2c_probe().
Matching of device in cs42xx8_i2c_probe() is coded with inspiration from tlv320aic32x4-i2c.c.
Signed-off-by: Peter Bergin peter@berginkonsult.se --- v2: reworked and removed duplication of cs42xx8_of_match
sound/soc/codecs/cs42xx8-i2c.c | 49 +++++++++++++++++++++++++++++++--- sound/soc/codecs/cs42xx8.c | 33 +++-------------------- sound/soc/codecs/cs42xx8.h | 5 +--- 3 files changed, 50 insertions(+), 37 deletions(-)
diff --git a/sound/soc/codecs/cs42xx8-i2c.c b/sound/soc/codecs/cs42xx8-i2c.c index bd80e9fc907f..9f9e57398272 100644 --- a/sound/soc/codecs/cs42xx8-i2c.c +++ b/sound/soc/codecs/cs42xx8-i2c.c @@ -17,10 +17,32 @@
#include "cs42xx8.h"
+static const struct i2c_device_id cs42xx8_i2c_id[]; +static const struct of_device_id cs42xx8_of_match[]; + static int cs42xx8_i2c_probe(struct i2c_client *i2c) { - int ret = cs42xx8_probe(&i2c->dev, - devm_regmap_init_i2c(i2c, &cs42xx8_regmap_config)); + int ret; + struct cs42xx8_driver_data *drvdata; + + if (i2c->dev.of_node) { + const struct of_device_id *oid; + + oid = of_match_node(cs42xx8_of_match, i2c->dev.of_node); + if (!oid) + goto err_not_found; + drvdata = (struct cs42xx8_driver_data *)oid->data; + } else { + const struct i2c_device_id *id; + + id = i2c_match_id(cs42xx8_i2c_id, i2c); + if (!id) + goto err_not_found; + drvdata = (struct cs42xx8_driver_data *)id->driver_data; + } + + ret = cs42xx8_probe(&i2c->dev, + devm_regmap_init_i2c(i2c, &cs42xx8_regmap_config), drvdata); if (ret) return ret;
@@ -28,6 +50,10 @@ static int cs42xx8_i2c_probe(struct i2c_client *i2c) pm_request_idle(&i2c->dev);
return 0; + +err_not_found: + dev_err(&i2c->dev, "failed to find driver data\n"); + return -EINVAL; }
static void cs42xx8_i2c_remove(struct i2c_client *i2c) @@ -35,7 +61,24 @@ static void cs42xx8_i2c_remove(struct i2c_client *i2c) pm_runtime_disable(&i2c->dev); }
-static struct i2c_device_id cs42xx8_i2c_id[] = { +static const struct cs42xx8_driver_data cs42448_data = { + .name = "cs42448", + .num_adcs = 3, +}; + +static const struct cs42xx8_driver_data cs42888_data = { + .name = "cs42888", + .num_adcs = 2, +}; + +static const struct of_device_id cs42xx8_of_match[] = { + { .compatible = "cirrus,cs42448", .data = &cs42448_data, }, + { .compatible = "cirrus,cs42888", .data = &cs42888_data, }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, cs42xx8_of_match); + +static const struct i2c_device_id cs42xx8_i2c_id[] = { {"cs42448", (kernel_ulong_t)&cs42448_data}, {"cs42888", (kernel_ulong_t)&cs42888_data}, {} diff --git a/sound/soc/codecs/cs42xx8.c b/sound/soc/codecs/cs42xx8.c index d14eb2f6e1dd..957ae08bcf7c 100644 --- a/sound/soc/codecs/cs42xx8.c +++ b/sound/soc/codecs/cs42xx8.c @@ -499,29 +499,8 @@ static const struct snd_soc_component_driver cs42xx8_driver = { .endianness = 1, };
-const struct cs42xx8_driver_data cs42448_data = { - .name = "cs42448", - .num_adcs = 3, -}; -EXPORT_SYMBOL_GPL(cs42448_data); - -const struct cs42xx8_driver_data cs42888_data = { - .name = "cs42888", - .num_adcs = 2, -}; -EXPORT_SYMBOL_GPL(cs42888_data); - -const struct of_device_id cs42xx8_of_match[] = { - { .compatible = "cirrus,cs42448", .data = &cs42448_data, }, - { .compatible = "cirrus,cs42888", .data = &cs42888_data, }, - { /* sentinel */ } -}; -MODULE_DEVICE_TABLE(of, cs42xx8_of_match); -EXPORT_SYMBOL_GPL(cs42xx8_of_match); - -int cs42xx8_probe(struct device *dev, struct regmap *regmap) +int cs42xx8_probe(struct device *dev, struct regmap *regmap, struct cs42xx8_driver_data *drvdata) { - const struct of_device_id *of_id; struct cs42xx8_priv *cs42xx8; int ret, val, i;
@@ -535,17 +514,11 @@ int cs42xx8_probe(struct device *dev, struct regmap *regmap) if (cs42xx8 == NULL) return -ENOMEM;
- cs42xx8->regmap = regmap; dev_set_drvdata(dev, cs42xx8);
- of_id = of_match_device(cs42xx8_of_match, dev); - if (of_id) - cs42xx8->drvdata = of_id->data; + cs42xx8->regmap = regmap;
- if (!cs42xx8->drvdata) { - dev_err(dev, "failed to find driver data\n"); - return -EINVAL; - } + cs42xx8->drvdata = drvdata;
cs42xx8->gpiod_reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); diff --git a/sound/soc/codecs/cs42xx8.h b/sound/soc/codecs/cs42xx8.h index d36c61b6df74..938e21d92ac2 100644 --- a/sound/soc/codecs/cs42xx8.h +++ b/sound/soc/codecs/cs42xx8.h @@ -19,11 +19,8 @@ struct cs42xx8_driver_data { };
extern const struct dev_pm_ops cs42xx8_pm; -extern const struct cs42xx8_driver_data cs42448_data; -extern const struct cs42xx8_driver_data cs42888_data; extern const struct regmap_config cs42xx8_regmap_config; -extern const struct of_device_id cs42xx8_of_match[]; -int cs42xx8_probe(struct device *dev, struct regmap *regmap); +int cs42xx8_probe(struct device *dev, struct regmap *regmap, struct cs42xx8_driver_data *drvdata);
/* CS42888 register map */ #define CS42XX8_CHIPID 0x01 /* Chip ID */
On Fri, Oct 28, 2022 at 09:50:44AM +0200, Peter Bergin wrote:
Matching of device in cs42xx8_i2c_probe() is coded with inspiration from tlv320aic32x4-i2c.c.
- if (i2c->dev.of_node) {
const struct of_device_id *oid;oid = of_match_node(cs42xx8_of_match, i2c->dev.of_node);if (!oid)goto err_not_found;drvdata = (struct cs42xx8_driver_data *)oid->data;- } else {
const struct i2c_device_id *id;id = i2c_match_id(cs42xx8_i2c_id, i2c);if (!id)goto err_not_found;drvdata = (struct cs42xx8_driver_data *)id->driver_data;- }
Be worth noting a little more explicitly in the commit message you updated this logic as part of the move. I would even be tempted to put it as a separate patch personally.
+static const struct cs42xx8_driver_data cs42448_data = {
- .name = "cs42448",
- .num_adcs = 3,
+};
+static const struct cs42xx8_driver_data cs42888_data = {
- .name = "cs42888",
- .num_adcs = 2,
+};
It is probably better to leave these two structures exported from the primary module. These devices could technically have SPI support added in the future and it might as well reuse these same data structures.
Thanks, Charles
On Fri, Oct 28, 2022 at 09:50:44AM +0200, Peter Bergin wrote:
When trying to connect the device with the driver through device-tree it is not working. The of_device_id is defined in cs42xx8.c but is not correctly included in cs42xx8-i2c.c.
Please don't send new patches in reply to old patches or serieses, this makes it harder for both people and tools to understand what is going on - it can bury things in mailboxes and make it difficult to keep track of what current patches are, both for the new patches and the old ones.
On 2022-10-28 12:55, Mark Brown wrote:
On Fri, Oct 28, 2022 at 09:50:44AM +0200, Peter Bergin wrote:
When trying to connect the device with the driver through device-tree it is not working. The of_device_id is defined in cs42xx8.c but is not correctly included in cs42xx8-i2c.c.
Please don't send new patches in reply to old patches or serieses, this makes it harder for both people and tools to understand what is going on - it can bury things in mailboxes and make it difficult to keep track of what current patches are, both for the new patches and the old ones.
Thanks for the instructions! Will follow on the next update. I'm new to this and trying to learn, pointers like this are very welcome. :-)
Best regards, /Peter
participants (4)
-
Charles Keepax -
kernel test robot -
Mark Brown -
Peter Bergin