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 <[email protected]>
---
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},
--
2.34.1
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 <[email protected]>
> ---
> 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-c-add-module-device-table-for-of/20221027-195337
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/a09fe8d3c2a7a2270c0ec1582ed182d747f0d9b8
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 <[email protected]>
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
--
0-DAY CI Kernel Test Service
https://01.org/lkp
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-c-add-module-device-table-for-of/20221027-195337
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/a09fe8d3c2a7a2270c0ec1582ed182d747f0d9b8
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 <[email protected]>
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
--
0-DAY CI Kernel Test Service
https://01.org/lkp
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 <[email protected]>
---
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 */
--
2.34.1
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 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
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.