* [PATCH 1/5] ALSA: hda: cs35l41: Avoid overwriting register patch @ 2022-01-13 17:07 ` Lucas Tanure 0 siblings, 0 replies; 28+ messages in thread From: Lucas Tanure @ 2022-01-13 17:07 UTC (permalink / raw) To: Rafael J . Wysocki, Len Brown, Hans de Goede, Mark Gross, Jaroslav Kysela, Takashi Iwai Cc: alsa-devel, linux-acpi, patches, platform-driver-x86, linux-kernel, Charles Keepax, Lucas Tanure From: Charles Keepax <ckeepax@opensource.cirrus.com> regmap_register_patch can't be used to apply the probe sequence as a patch is already registers with the regmap by cs35l41_register_errata_patch and only a single patch can be attached to a single regmap. The driver doesn't currently rely on a cache sync to re-apply this probe sequence so simply switch it to a multi write. Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com> Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> --- sound/pci/hda/cs35l41_hda.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c index 30b40d865863..c47c5f0b4e59 100644 --- a/sound/pci/hda/cs35l41_hda.c +++ b/sound/pci/hda/cs35l41_hda.c @@ -480,7 +480,7 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i acpi_hw_cfg = NULL; if (cs35l41->reg_seq->probe) { - ret = regmap_register_patch(cs35l41->regmap, cs35l41->reg_seq->probe, + ret = regmap_multi_reg_write(cs35l41->regmap, cs35l41->reg_seq->probe, cs35l41->reg_seq->num_probe); if (ret) { dev_err(cs35l41->dev, "Fail to apply probe reg patch: %d\n", ret); -- 2.34.1 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 1/5] ALSA: hda: cs35l41: Avoid overwriting register patch @ 2022-01-13 17:07 ` Lucas Tanure 0 siblings, 0 replies; 28+ messages in thread From: Lucas Tanure @ 2022-01-13 17:07 UTC (permalink / raw) To: Rafael J . Wysocki, Len Brown, Hans de Goede, Mark Gross, Jaroslav Kysela, Takashi Iwai Cc: alsa-devel, Charles Keepax, Lucas Tanure, patches, linux-kernel, platform-driver-x86, linux-acpi From: Charles Keepax <ckeepax@opensource.cirrus.com> regmap_register_patch can't be used to apply the probe sequence as a patch is already registers with the regmap by cs35l41_register_errata_patch and only a single patch can be attached to a single regmap. The driver doesn't currently rely on a cache sync to re-apply this probe sequence so simply switch it to a multi write. Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com> Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> --- sound/pci/hda/cs35l41_hda.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c index 30b40d865863..c47c5f0b4e59 100644 --- a/sound/pci/hda/cs35l41_hda.c +++ b/sound/pci/hda/cs35l41_hda.c @@ -480,7 +480,7 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i acpi_hw_cfg = NULL; if (cs35l41->reg_seq->probe) { - ret = regmap_register_patch(cs35l41->regmap, cs35l41->reg_seq->probe, + ret = regmap_multi_reg_write(cs35l41->regmap, cs35l41->reg_seq->probe, cs35l41->reg_seq->num_probe); if (ret) { dev_err(cs35l41->dev, "Fail to apply probe reg patch: %d\n", ret); -- 2.34.1 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 2/5] ALSA: hda: cs35l41: Add calls to newly added test key function 2022-01-13 17:07 ` Lucas Tanure @ 2022-01-13 17:07 ` Lucas Tanure -1 siblings, 0 replies; 28+ messages in thread From: Lucas Tanure @ 2022-01-13 17:07 UTC (permalink / raw) To: Rafael J . Wysocki, Len Brown, Hans de Goede, Mark Gross, Jaroslav Kysela, Takashi Iwai Cc: alsa-devel, linux-acpi, patches, platform-driver-x86, linux-kernel, Charles Keepax, Lucas Tanure From: Charles Keepax <ckeepax@opensource.cirrus.com> The test key now needs to be manually held when calling cs35l41_register_errata_patch, after patch: 'commit f517ba4924ad ("ASoC: cs35l41: Add support for hibernate memory retention mode")' Add the missing function calls to this driver. Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com> Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> --- sound/pci/hda/cs35l41_hda.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c index c47c5f0b4e59..509a380f9be7 100644 --- a/sound/pci/hda/cs35l41_hda.c +++ b/sound/pci/hda/cs35l41_hda.c @@ -463,6 +463,10 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i goto err; } + ret = cs35l41_test_key_unlock(cs35l41->dev, cs35l41->regmap); + if (ret) + goto err; + ret = cs35l41_register_errata_patch(cs35l41->dev, cs35l41->regmap, reg_revid); if (ret) goto err; @@ -473,6 +477,10 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i goto err; } + ret = cs35l41_test_key_lock(cs35l41->dev, cs35l41->regmap); + if (ret) + goto err; + ret = cs35l41_hda_apply_properties(cs35l41, acpi_hw_cfg); if (ret) goto err; -- 2.34.1 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 2/5] ALSA: hda: cs35l41: Add calls to newly added test key function @ 2022-01-13 17:07 ` Lucas Tanure 0 siblings, 0 replies; 28+ messages in thread From: Lucas Tanure @ 2022-01-13 17:07 UTC (permalink / raw) To: Rafael J . Wysocki, Len Brown, Hans de Goede, Mark Gross, Jaroslav Kysela, Takashi Iwai Cc: alsa-devel, Charles Keepax, Lucas Tanure, patches, linux-kernel, platform-driver-x86, linux-acpi From: Charles Keepax <ckeepax@opensource.cirrus.com> The test key now needs to be manually held when calling cs35l41_register_errata_patch, after patch: 'commit f517ba4924ad ("ASoC: cs35l41: Add support for hibernate memory retention mode")' Add the missing function calls to this driver. Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com> Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> --- sound/pci/hda/cs35l41_hda.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c index c47c5f0b4e59..509a380f9be7 100644 --- a/sound/pci/hda/cs35l41_hda.c +++ b/sound/pci/hda/cs35l41_hda.c @@ -463,6 +463,10 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i goto err; } + ret = cs35l41_test_key_unlock(cs35l41->dev, cs35l41->regmap); + if (ret) + goto err; + ret = cs35l41_register_errata_patch(cs35l41->dev, cs35l41->regmap, reg_revid); if (ret) goto err; @@ -473,6 +477,10 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i goto err; } + ret = cs35l41_test_key_lock(cs35l41->dev, cs35l41->regmap); + if (ret) + goto err; + ret = cs35l41_hda_apply_properties(cs35l41, acpi_hw_cfg); if (ret) goto err; -- 2.34.1 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 2/5] ALSA: hda: cs35l41: Add calls to newly added test key function 2022-01-13 17:07 ` Lucas Tanure @ 2022-01-14 16:14 ` Cezary Rojewski -1 siblings, 0 replies; 28+ messages in thread From: Cezary Rojewski @ 2022-01-14 16:14 UTC (permalink / raw) To: Lucas Tanure, Rafael J . Wysocki, Len Brown, Hans de Goede, Mark Gross, Jaroslav Kysela, Takashi Iwai Cc: alsa-devel, linux-acpi, patches, platform-driver-x86, linux-kernel, Charles Keepax On 2022-01-13 6:07 PM, Lucas Tanure wrote: > From: Charles Keepax <ckeepax@opensource.cirrus.com> > > The test key now needs to be manually held when calling > cs35l41_register_errata_patch, after patch: > > 'commit f517ba4924ad ("ASoC: cs35l41: Add support for > hibernate memory retention mode")' > > Add the missing function calls to this driver. > > Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com> > Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> Judging by the commit's message, this patch is a fix for a previously added commit so appropriate 'Fixes' tag could prove helpful here. > sound/pci/hda/cs35l41_hda.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c > index c47c5f0b4e59..509a380f9be7 100644 > --- a/sound/pci/hda/cs35l41_hda.c > +++ b/sound/pci/hda/cs35l41_hda.c > @@ -463,6 +463,10 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i > goto err; > } > > + ret = cs35l41_test_key_unlock(cs35l41->dev, cs35l41->regmap); > + if (ret) > + goto err; > + > ret = cs35l41_register_errata_patch(cs35l41->dev, cs35l41->regmap, reg_revid); > if (ret) > goto err; > @@ -473,6 +477,10 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i > goto err; > } > > + ret = cs35l41_test_key_lock(cs35l41->dev, cs35l41->regmap); > + if (ret) > + goto err; > + > ret = cs35l41_hda_apply_properties(cs35l41, acpi_hw_cfg); > if (ret) > goto err; > ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 2/5] ALSA: hda: cs35l41: Add calls to newly added test key function @ 2022-01-14 16:14 ` Cezary Rojewski 0 siblings, 0 replies; 28+ messages in thread From: Cezary Rojewski @ 2022-01-14 16:14 UTC (permalink / raw) To: Lucas Tanure, Rafael J . Wysocki, Len Brown, Hans de Goede, Mark Gross, Jaroslav Kysela, Takashi Iwai Cc: alsa-devel, Charles Keepax, patches, linux-kernel, platform-driver-x86, linux-acpi On 2022-01-13 6:07 PM, Lucas Tanure wrote: > From: Charles Keepax <ckeepax@opensource.cirrus.com> > > The test key now needs to be manually held when calling > cs35l41_register_errata_patch, after patch: > > 'commit f517ba4924ad ("ASoC: cs35l41: Add support for > hibernate memory retention mode")' > > Add the missing function calls to this driver. > > Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com> > Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> Judging by the commit's message, this patch is a fix for a previously added commit so appropriate 'Fixes' tag could prove helpful here. > sound/pci/hda/cs35l41_hda.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c > index c47c5f0b4e59..509a380f9be7 100644 > --- a/sound/pci/hda/cs35l41_hda.c > +++ b/sound/pci/hda/cs35l41_hda.c > @@ -463,6 +463,10 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i > goto err; > } > > + ret = cs35l41_test_key_unlock(cs35l41->dev, cs35l41->regmap); > + if (ret) > + goto err; > + > ret = cs35l41_register_errata_patch(cs35l41->dev, cs35l41->regmap, reg_revid); > if (ret) > goto err; > @@ -473,6 +477,10 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i > goto err; > } > > + ret = cs35l41_test_key_lock(cs35l41->dev, cs35l41->regmap); > + if (ret) > + goto err; > + > ret = cs35l41_hda_apply_properties(cs35l41, acpi_hw_cfg); > if (ret) > goto err; > ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 3/5] ALSA: hda: cs35l41: Move cs35l41* calls to its own symbol namespace 2022-01-13 17:07 ` Lucas Tanure @ 2022-01-13 17:07 ` Lucas Tanure -1 siblings, 0 replies; 28+ messages in thread From: Lucas Tanure @ 2022-01-13 17:07 UTC (permalink / raw) To: Rafael J . Wysocki, Len Brown, Hans de Goede, Mark Gross, Jaroslav Kysela, Takashi Iwai Cc: alsa-devel, linux-acpi, patches, platform-driver-x86, linux-kernel, Lucas Tanure Create own namespace and avoid polluting the global namespace Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> --- sound/pci/hda/cs35l41_hda.c | 5 ++--- sound/pci/hda/cs35l41_hda_i2c.c | 1 + sound/pci/hda/cs35l41_hda_spi.c | 1 + 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c index 509a380f9be7..c4f25e48dcc0 100644 --- a/sound/pci/hda/cs35l41_hda.c +++ b/sound/pci/hda/cs35l41_hda.c @@ -514,7 +514,7 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i return ret; } -EXPORT_SYMBOL_GPL(cs35l41_hda_probe); +EXPORT_SYMBOL_NS_GPL(cs35l41_hda_probe, SND_HDA_SCODEC_CS35L41); int cs35l41_hda_remove(struct device *dev) { @@ -528,8 +528,7 @@ int cs35l41_hda_remove(struct device *dev) return 0; } -EXPORT_SYMBOL_GPL(cs35l41_hda_remove); - +EXPORT_SYMBOL_NS_GPL(cs35l41_hda_remove, SND_HDA_SCODEC_CS35L41); MODULE_DESCRIPTION("CS35L41 HDA Driver"); MODULE_AUTHOR("Lucas Tanure, Cirrus Logic Inc, <tanureal@opensource.cirrus.com>"); diff --git a/sound/pci/hda/cs35l41_hda_i2c.c b/sound/pci/hda/cs35l41_hda_i2c.c index 4a9462fb5c14..eeb387853ee3 100644 --- a/sound/pci/hda/cs35l41_hda_i2c.c +++ b/sound/pci/hda/cs35l41_hda_i2c.c @@ -62,5 +62,6 @@ static struct i2c_driver cs35l41_i2c_driver = { module_i2c_driver(cs35l41_i2c_driver); MODULE_DESCRIPTION("HDA CS35L41 driver"); +MODULE_IMPORT_NS(SND_HDA_SCODEC_CS35L41); MODULE_AUTHOR("Lucas Tanure <tanureal@opensource.cirrus.com>"); MODULE_LICENSE("GPL"); diff --git a/sound/pci/hda/cs35l41_hda_spi.c b/sound/pci/hda/cs35l41_hda_spi.c index 77426e96c58f..15345a72b9d1 100644 --- a/sound/pci/hda/cs35l41_hda_spi.c +++ b/sound/pci/hda/cs35l41_hda_spi.c @@ -59,5 +59,6 @@ static struct spi_driver cs35l41_spi_driver = { module_spi_driver(cs35l41_spi_driver); MODULE_DESCRIPTION("HDA CS35L41 driver"); +MODULE_IMPORT_NS(SND_HDA_SCODEC_CS35L41); MODULE_AUTHOR("Lucas Tanure <tanureal@opensource.cirrus.com>"); MODULE_LICENSE("GPL"); -- 2.34.1 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 3/5] ALSA: hda: cs35l41: Move cs35l41* calls to its own symbol namespace @ 2022-01-13 17:07 ` Lucas Tanure 0 siblings, 0 replies; 28+ messages in thread From: Lucas Tanure @ 2022-01-13 17:07 UTC (permalink / raw) To: Rafael J . Wysocki, Len Brown, Hans de Goede, Mark Gross, Jaroslav Kysela, Takashi Iwai Cc: alsa-devel, Lucas Tanure, patches, linux-kernel, platform-driver-x86, linux-acpi Create own namespace and avoid polluting the global namespace Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> --- sound/pci/hda/cs35l41_hda.c | 5 ++--- sound/pci/hda/cs35l41_hda_i2c.c | 1 + sound/pci/hda/cs35l41_hda_spi.c | 1 + 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c index 509a380f9be7..c4f25e48dcc0 100644 --- a/sound/pci/hda/cs35l41_hda.c +++ b/sound/pci/hda/cs35l41_hda.c @@ -514,7 +514,7 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i return ret; } -EXPORT_SYMBOL_GPL(cs35l41_hda_probe); +EXPORT_SYMBOL_NS_GPL(cs35l41_hda_probe, SND_HDA_SCODEC_CS35L41); int cs35l41_hda_remove(struct device *dev) { @@ -528,8 +528,7 @@ int cs35l41_hda_remove(struct device *dev) return 0; } -EXPORT_SYMBOL_GPL(cs35l41_hda_remove); - +EXPORT_SYMBOL_NS_GPL(cs35l41_hda_remove, SND_HDA_SCODEC_CS35L41); MODULE_DESCRIPTION("CS35L41 HDA Driver"); MODULE_AUTHOR("Lucas Tanure, Cirrus Logic Inc, <tanureal@opensource.cirrus.com>"); diff --git a/sound/pci/hda/cs35l41_hda_i2c.c b/sound/pci/hda/cs35l41_hda_i2c.c index 4a9462fb5c14..eeb387853ee3 100644 --- a/sound/pci/hda/cs35l41_hda_i2c.c +++ b/sound/pci/hda/cs35l41_hda_i2c.c @@ -62,5 +62,6 @@ static struct i2c_driver cs35l41_i2c_driver = { module_i2c_driver(cs35l41_i2c_driver); MODULE_DESCRIPTION("HDA CS35L41 driver"); +MODULE_IMPORT_NS(SND_HDA_SCODEC_CS35L41); MODULE_AUTHOR("Lucas Tanure <tanureal@opensource.cirrus.com>"); MODULE_LICENSE("GPL"); diff --git a/sound/pci/hda/cs35l41_hda_spi.c b/sound/pci/hda/cs35l41_hda_spi.c index 77426e96c58f..15345a72b9d1 100644 --- a/sound/pci/hda/cs35l41_hda_spi.c +++ b/sound/pci/hda/cs35l41_hda_spi.c @@ -59,5 +59,6 @@ static struct spi_driver cs35l41_spi_driver = { module_spi_driver(cs35l41_spi_driver); MODULE_DESCRIPTION("HDA CS35L41 driver"); +MODULE_IMPORT_NS(SND_HDA_SCODEC_CS35L41); MODULE_AUTHOR("Lucas Tanure <tanureal@opensource.cirrus.com>"); MODULE_LICENSE("GPL"); -- 2.34.1 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 4/5] ALSA: hda: cs35l41: Tidyup code and add default cases 2022-01-13 17:07 ` Lucas Tanure @ 2022-01-13 17:07 ` Lucas Tanure -1 siblings, 0 replies; 28+ messages in thread From: Lucas Tanure @ 2022-01-13 17:07 UTC (permalink / raw) To: Rafael J . Wysocki, Len Brown, Hans de Goede, Mark Gross, Jaroslav Kysela, Takashi Iwai Cc: alsa-devel, linux-acpi, patches, platform-driver-x86, linux-kernel, Lucas Tanure Clean up the code, plus adding default cases for switch and dev_err_probe use. Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> --- sound/pci/hda/cs35l41_hda.c | 109 ++++++++++++++++---------------- sound/pci/hda/cs35l41_hda.h | 2 +- sound/pci/hda/cs35l41_hda_i2c.c | 1 - sound/pci/hda/cs35l41_hda_spi.c | 1 - 4 files changed, 57 insertions(+), 56 deletions(-) diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c index c4f25e48dcc0..cb8331587c17 100644 --- a/sound/pci/hda/cs35l41_hda.c +++ b/sound/pci/hda/cs35l41_hda.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 // -// cs35l41.c -- CS35l41 ALSA HDA audio driver +// CS35l41 ALSA HDA audio driver // // Copyright 2021 Cirrus Logic, Inc. // @@ -17,19 +17,19 @@ #include "cs35l41_hda.h" static const struct reg_sequence cs35l41_hda_config[] = { - { CS35L41_PLL_CLK_CTRL, 0x00000430 }, //3200000Hz, BCLK Input, PLL_REFCLK_EN = 1 - { CS35L41_GLOBAL_CLK_CTRL, 0x00000003 }, //GLOBAL_FS = 48 kHz - { CS35L41_SP_ENABLES, 0x00010000 }, //ASP_RX1_EN = 1 - { CS35L41_SP_RATE_CTRL, 0x00000021 }, //ASP_BCLK_FREQ = 3.072 MHz - { CS35L41_SP_FORMAT, 0x20200200 }, //24 bits, I2S, BCLK Slave, FSYNC Slave - { CS35L41_DAC_PCM1_SRC, 0x00000008 }, //DACPCM1_SRC = ASPRX1 - { CS35L41_AMP_DIG_VOL_CTRL, 0x00000000 }, //AMP_VOL_PCM 0.0 dB - { CS35L41_AMP_GAIN_CTRL, 0x00000084 }, //AMP_GAIN_PCM 4.5 dB - { CS35L41_PWR_CTRL2, 0x00000001 }, //AMP_EN = 1 + { CS35L41_PLL_CLK_CTRL, 0x00000430 }, // 3200000Hz, BCLK Input, PLL_REFCLK_EN = 1 + { CS35L41_GLOBAL_CLK_CTRL, 0x00000003 }, // GLOBAL_FS = 48 kHz + { CS35L41_SP_ENABLES, 0x00010000 }, // ASP_RX1_EN = 1 + { CS35L41_SP_RATE_CTRL, 0x00000021 }, // ASP_BCLK_FREQ = 3.072 MHz + { CS35L41_SP_FORMAT, 0x20200200 }, // 24 bits, I2S, BCLK Slave, FSYNC Slave + { CS35L41_DAC_PCM1_SRC, 0x00000008 }, // DACPCM1_SRC = ASPRX1 + { CS35L41_AMP_DIG_VOL_CTRL, 0x00000000 }, // AMP_VOL_PCM 0.0 dB + { CS35L41_AMP_GAIN_CTRL, 0x00000084 }, // AMP_GAIN_PCM 4.5 dB + { CS35L41_PWR_CTRL2, 0x00000001 }, // AMP_EN = 1 }; static const struct reg_sequence cs35l41_hda_start_bst[] = { - { CS35L41_PWR_CTRL2, 0x00000021 }, //BST_EN = 10, AMP_EN = 1 + { CS35L41_PWR_CTRL2, 0x00000021 }, // BST_EN = 10, AMP_EN = 1 { CS35L41_PWR_CTRL1, 0x00000001, 3000}, // set GLOBAL_EN = 1 }; @@ -60,7 +60,7 @@ static const struct reg_sequence cs35l41_stop_ext_vspk[] = { { 0x00000040, 0x00000055 }, { 0x00000040, 0x000000AA }, { 0x00007438, 0x00585941 }, - { 0x00002014, 0x00000000, 3000}, //set GLOBAL_EN = 0 + { 0x00002014, 0x00000000, 3000}, // set GLOBAL_EN = 0 { 0x0000742C, 0x00000009 }, { 0x00007438, 0x00580941 }, { 0x00011008, 0x00000001 }, @@ -78,7 +78,7 @@ static const struct reg_sequence cs35l41_safe_to_active[] = { { 0x0000742C, 0x0000000F }, { 0x0000742C, 0x00000079 }, { 0x00007438, 0x00585941 }, - { CS35L41_PWR_CTRL1, 0x00000001, 2000 }, //GLOBAL_EN = 1 + { CS35L41_PWR_CTRL1, 0x00000001, 2000 }, // GLOBAL_EN = 1 { 0x0000742C, 0x000000F9 }, { 0x00007438, 0x00580941 }, { 0x00000040, 0x000000CC }, @@ -89,8 +89,8 @@ static const struct reg_sequence cs35l41_active_to_safe[] = { { 0x00000040, 0x00000055 }, { 0x00000040, 0x000000AA }, { 0x00007438, 0x00585941 }, - { CS35L41_AMP_DIG_VOL_CTRL, 0x0000A678 }, //AMP_VOL_PCM Mute - { CS35L41_PWR_CTRL2, 0x00000000 }, //AMP_EN = 0 + { CS35L41_AMP_DIG_VOL_CTRL, 0x0000A678 }, // AMP_VOL_PCM Mute + { CS35L41_PWR_CTRL2, 0x00000000 }, // AMP_EN = 0 { CS35L41_PWR_CTRL1, 0x00000000 }, { 0x0000742C, 0x00000009, 2000 }, { 0x00007438, 0x00580941 }, @@ -161,11 +161,13 @@ static void cs35l41_hda_playback_hook(struct device *dev, int action) if (reg_seq->close) ret = regmap_multi_reg_write(reg, reg_seq->close, reg_seq->num_close); break; + default: + ret = -EINVAL; + break; } if (ret) dev_warn(cs35l41->dev, "Failed to apply multi reg write: %d\n", ret); - } static int cs35l41_hda_channel_map(struct device *dev, unsigned int tx_num, unsigned int *tx_slot, @@ -182,20 +184,19 @@ static int cs35l41_hda_bind(struct device *dev, struct device *master, void *mas struct cs35l41_hda *cs35l41 = dev_get_drvdata(dev); struct hda_component *comps = master_data; - if (comps && cs35l41->index >= 0 && cs35l41->index < HDA_MAX_COMPONENTS) - comps = &comps[cs35l41->index]; - else + if (!comps || cs35l41->index < 0 || cs35l41->index >= HDA_MAX_COMPONENTS) return -EINVAL; - if (!comps->dev) { - comps->dev = dev; - strscpy(comps->name, dev_name(dev), sizeof(comps->name)); - comps->playback_hook = cs35l41_hda_playback_hook; - comps->set_channel_map = cs35l41_hda_channel_map; - return 0; - } + comps = &comps[cs35l41->index]; + if (comps->dev) + return -EBUSY; + + comps->dev = dev; + strscpy(comps->name, dev_name(dev), sizeof(comps->name)); + comps->playback_hook = cs35l41_hda_playback_hook; + comps->set_channel_map = cs35l41_hda_channel_map; - return -EBUSY; + return 0; } static void cs35l41_hda_unbind(struct device *dev, struct device *master, void *master_data) @@ -235,6 +236,8 @@ static int cs35l41_hda_apply_properties(struct cs35l41_hda *cs35l41, regmap_update_bits(cs35l41->regmap, CS35L41_GPIO_PAD_CONTROL, CS35L41_GPIO1_CTRL_MASK, 2 << CS35L41_GPIO1_CTRL_SHIFT); break; + default: + return -EINVAL; } switch (hw_cfg->gpio2_func) { @@ -242,6 +245,8 @@ static int cs35l41_hda_apply_properties(struct cs35l41_hda *cs35l41, regmap_update_bits(cs35l41->regmap, CS35L41_GPIO_PAD_CONTROL, CS35L41_GPIO2_CTRL_MASK, 2 << CS35L41_GPIO2_CTRL_SHIFT); break; + default: + return -EINVAL; } if (internal_boost) { @@ -256,11 +261,7 @@ static int cs35l41_hda_apply_properties(struct cs35l41_hda *cs35l41, cs35l41->reg_seq = &cs35l41_hda_reg_seq_ext_bst; } - ret = cs35l41_hda_channel_map(cs35l41->dev, 0, NULL, 1, (unsigned int *)&hw_cfg->spk_pos); - if (ret) - return ret; - - return 0; + return cs35l41_hda_channel_map(cs35l41->dev, 0, NULL, 1, (unsigned int *)&hw_cfg->spk_pos); } static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *cs35l41, @@ -269,7 +270,7 @@ static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *c struct cs35l41_hda_hw_config *hw_cfg; u32 values[HDA_MAX_COMPONENTS]; struct acpi_device *adev; - struct device *acpi_dev; + struct device *physdev; char *property; size_t nval; int i, ret; @@ -280,11 +281,11 @@ static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *c return ERR_PTR(-ENODEV); } - acpi_dev = get_device(acpi_get_first_physical_node(adev)); + physdev = get_device(acpi_get_first_physical_node(adev)); acpi_dev_put(adev); property = "cirrus,dev-index"; - ret = device_property_count_u32(acpi_dev, property); + ret = device_property_count_u32(physdev, property); if (ret <= 0) goto no_acpi_dsd; @@ -294,7 +295,7 @@ static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *c } nval = ret; - ret = device_property_read_u32_array(acpi_dev, property, values, nval); + ret = device_property_read_u32_array(physdev, property, values, nval); if (ret) goto err; @@ -311,7 +312,9 @@ static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *c goto err; } - /* No devm_ version as CLSA0100, in no_acpi_dsd case, can't use devm version */ + /* To use the same release code for all laptop variants we can't use devm_ version of + * gpiod_get here, as CLSA010* don't have a fully functional bios with an _DSD node + */ cs35l41->reset_gpio = fwnode_gpiod_get_index(&adev->fwnode, "reset", cs35l41->index, GPIOD_OUT_LOW, "cs35l41-reset"); @@ -322,46 +325,46 @@ static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *c } property = "cirrus,speaker-position"; - ret = device_property_read_u32_array(acpi_dev, property, values, nval); + ret = device_property_read_u32_array(physdev, property, values, nval); if (ret) goto err_free; hw_cfg->spk_pos = values[cs35l41->index]; property = "cirrus,gpio1-func"; - ret = device_property_read_u32_array(acpi_dev, property, values, nval); + ret = device_property_read_u32_array(physdev, property, values, nval); if (ret) goto err_free; hw_cfg->gpio1_func = values[cs35l41->index]; property = "cirrus,gpio2-func"; - ret = device_property_read_u32_array(acpi_dev, property, values, nval); + ret = device_property_read_u32_array(physdev, property, values, nval); if (ret) goto err_free; hw_cfg->gpio2_func = values[cs35l41->index]; property = "cirrus,boost-peak-milliamp"; - ret = device_property_read_u32_array(acpi_dev, property, values, nval); + ret = device_property_read_u32_array(physdev, property, values, nval); if (ret == 0) hw_cfg->bst_ipk = values[cs35l41->index]; property = "cirrus,boost-ind-nanohenry"; - ret = device_property_read_u32_array(acpi_dev, property, values, nval); + ret = device_property_read_u32_array(physdev, property, values, nval); if (ret == 0) hw_cfg->bst_ind = values[cs35l41->index]; property = "cirrus,boost-cap-microfarad"; - ret = device_property_read_u32_array(acpi_dev, property, values, nval); + ret = device_property_read_u32_array(physdev, property, values, nval); if (ret == 0) hw_cfg->bst_cap = values[cs35l41->index]; - put_device(acpi_dev); + put_device(physdev); return hw_cfg; err_free: kfree(hw_cfg); err: - put_device(acpi_dev); + put_device(physdev); dev_err(cs35l41->dev, "Failed property %s: %d\n", property, ret); return ERR_PTR(ret); @@ -370,18 +373,18 @@ static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *c /* * Device CLSA0100 doesn't have _DSD so a gpiod_get by the label reset won't work. * And devices created by i2c-multi-instantiate don't have their device struct pointing to - * the correct fwnode, so acpi_dev must be used here + * the correct fwnode, so acpi_dev must be used here. * And devm functions expect that the device requesting the resource has the correct - * fwnode + * fwnode. */ if (strncmp(hid, "CLSA0100", 8) != 0) return ERR_PTR(-EINVAL); /* check I2C address to assign the index */ cs35l41->index = id == 0x40 ? 0 : 1; - cs35l41->reset_gpio = gpiod_get_index(acpi_dev, NULL, 0, GPIOD_OUT_HIGH); + cs35l41->reset_gpio = gpiod_get_index(physdev, NULL, 0, GPIOD_OUT_HIGH); cs35l41->vspk_always_on = true; - put_device(acpi_dev); + put_device(physdev); return NULL; } @@ -416,8 +419,7 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i if (ret == -EBUSY) { dev_info(cs35l41->dev, "Reset line busy, assuming shared reset\n"); } else { - if (ret != -EPROBE_DEFER) - dev_err(cs35l41->dev, "Failed to get reset GPIO: %d\n", ret); + dev_err_probe(cs35l41->dev, ret, "Failed to get reset GPIO: %d\n", ret); goto err; } } @@ -437,7 +439,8 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i ret = regmap_read(cs35l41->regmap, CS35L41_IRQ1_STATUS3, &int_sts); if (ret || (int_sts & CS35L41_OTP_BOOT_ERR)) { - dev_err(cs35l41->dev, "OTP Boot error\n"); + dev_err(cs35l41->dev, "OTP Boot status %x error: %d\n", + int_sts & CS35L41_OTP_BOOT_ERR, ret); ret = -EIO; goto err; } @@ -489,7 +492,7 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i if (cs35l41->reg_seq->probe) { ret = regmap_multi_reg_write(cs35l41->regmap, cs35l41->reg_seq->probe, - cs35l41->reg_seq->num_probe); + cs35l41->reg_seq->num_probe); if (ret) { dev_err(cs35l41->dev, "Fail to apply probe reg patch: %d\n", ret); goto err; diff --git a/sound/pci/hda/cs35l41_hda.h b/sound/pci/hda/cs35l41_hda.h index 76c69a8a22f6..640afc98b686 100644 --- a/sound/pci/hda/cs35l41_hda.h +++ b/sound/pci/hda/cs35l41_hda.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0 * - * cs35l41_hda.h -- CS35L41 ALSA HDA audio driver + * CS35L41 ALSA HDA audio driver * * Copyright 2021 Cirrus Logic, Inc. * diff --git a/sound/pci/hda/cs35l41_hda_i2c.c b/sound/pci/hda/cs35l41_hda_i2c.c index eeb387853ee3..c2397dc53e78 100644 --- a/sound/pci/hda/cs35l41_hda_i2c.c +++ b/sound/pci/hda/cs35l41_hda_i2c.c @@ -58,7 +58,6 @@ static struct i2c_driver cs35l41_i2c_driver = { .probe = cs35l41_hda_i2c_probe, .remove = cs35l41_hda_i2c_remove, }; - module_i2c_driver(cs35l41_i2c_driver); MODULE_DESCRIPTION("HDA CS35L41 driver"); diff --git a/sound/pci/hda/cs35l41_hda_spi.c b/sound/pci/hda/cs35l41_hda_spi.c index 15345a72b9d1..36815ab4e461 100644 --- a/sound/pci/hda/cs35l41_hda_spi.c +++ b/sound/pci/hda/cs35l41_hda_spi.c @@ -55,7 +55,6 @@ static struct spi_driver cs35l41_spi_driver = { .probe = cs35l41_hda_spi_probe, .remove = cs35l41_hda_spi_remove, }; - module_spi_driver(cs35l41_spi_driver); MODULE_DESCRIPTION("HDA CS35L41 driver"); -- 2.34.1 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 4/5] ALSA: hda: cs35l41: Tidyup code and add default cases @ 2022-01-13 17:07 ` Lucas Tanure 0 siblings, 0 replies; 28+ messages in thread From: Lucas Tanure @ 2022-01-13 17:07 UTC (permalink / raw) To: Rafael J . Wysocki, Len Brown, Hans de Goede, Mark Gross, Jaroslav Kysela, Takashi Iwai Cc: alsa-devel, Lucas Tanure, patches, linux-kernel, platform-driver-x86, linux-acpi Clean up the code, plus adding default cases for switch and dev_err_probe use. Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> --- sound/pci/hda/cs35l41_hda.c | 109 ++++++++++++++++---------------- sound/pci/hda/cs35l41_hda.h | 2 +- sound/pci/hda/cs35l41_hda_i2c.c | 1 - sound/pci/hda/cs35l41_hda_spi.c | 1 - 4 files changed, 57 insertions(+), 56 deletions(-) diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c index c4f25e48dcc0..cb8331587c17 100644 --- a/sound/pci/hda/cs35l41_hda.c +++ b/sound/pci/hda/cs35l41_hda.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0 // -// cs35l41.c -- CS35l41 ALSA HDA audio driver +// CS35l41 ALSA HDA audio driver // // Copyright 2021 Cirrus Logic, Inc. // @@ -17,19 +17,19 @@ #include "cs35l41_hda.h" static const struct reg_sequence cs35l41_hda_config[] = { - { CS35L41_PLL_CLK_CTRL, 0x00000430 }, //3200000Hz, BCLK Input, PLL_REFCLK_EN = 1 - { CS35L41_GLOBAL_CLK_CTRL, 0x00000003 }, //GLOBAL_FS = 48 kHz - { CS35L41_SP_ENABLES, 0x00010000 }, //ASP_RX1_EN = 1 - { CS35L41_SP_RATE_CTRL, 0x00000021 }, //ASP_BCLK_FREQ = 3.072 MHz - { CS35L41_SP_FORMAT, 0x20200200 }, //24 bits, I2S, BCLK Slave, FSYNC Slave - { CS35L41_DAC_PCM1_SRC, 0x00000008 }, //DACPCM1_SRC = ASPRX1 - { CS35L41_AMP_DIG_VOL_CTRL, 0x00000000 }, //AMP_VOL_PCM 0.0 dB - { CS35L41_AMP_GAIN_CTRL, 0x00000084 }, //AMP_GAIN_PCM 4.5 dB - { CS35L41_PWR_CTRL2, 0x00000001 }, //AMP_EN = 1 + { CS35L41_PLL_CLK_CTRL, 0x00000430 }, // 3200000Hz, BCLK Input, PLL_REFCLK_EN = 1 + { CS35L41_GLOBAL_CLK_CTRL, 0x00000003 }, // GLOBAL_FS = 48 kHz + { CS35L41_SP_ENABLES, 0x00010000 }, // ASP_RX1_EN = 1 + { CS35L41_SP_RATE_CTRL, 0x00000021 }, // ASP_BCLK_FREQ = 3.072 MHz + { CS35L41_SP_FORMAT, 0x20200200 }, // 24 bits, I2S, BCLK Slave, FSYNC Slave + { CS35L41_DAC_PCM1_SRC, 0x00000008 }, // DACPCM1_SRC = ASPRX1 + { CS35L41_AMP_DIG_VOL_CTRL, 0x00000000 }, // AMP_VOL_PCM 0.0 dB + { CS35L41_AMP_GAIN_CTRL, 0x00000084 }, // AMP_GAIN_PCM 4.5 dB + { CS35L41_PWR_CTRL2, 0x00000001 }, // AMP_EN = 1 }; static const struct reg_sequence cs35l41_hda_start_bst[] = { - { CS35L41_PWR_CTRL2, 0x00000021 }, //BST_EN = 10, AMP_EN = 1 + { CS35L41_PWR_CTRL2, 0x00000021 }, // BST_EN = 10, AMP_EN = 1 { CS35L41_PWR_CTRL1, 0x00000001, 3000}, // set GLOBAL_EN = 1 }; @@ -60,7 +60,7 @@ static const struct reg_sequence cs35l41_stop_ext_vspk[] = { { 0x00000040, 0x00000055 }, { 0x00000040, 0x000000AA }, { 0x00007438, 0x00585941 }, - { 0x00002014, 0x00000000, 3000}, //set GLOBAL_EN = 0 + { 0x00002014, 0x00000000, 3000}, // set GLOBAL_EN = 0 { 0x0000742C, 0x00000009 }, { 0x00007438, 0x00580941 }, { 0x00011008, 0x00000001 }, @@ -78,7 +78,7 @@ static const struct reg_sequence cs35l41_safe_to_active[] = { { 0x0000742C, 0x0000000F }, { 0x0000742C, 0x00000079 }, { 0x00007438, 0x00585941 }, - { CS35L41_PWR_CTRL1, 0x00000001, 2000 }, //GLOBAL_EN = 1 + { CS35L41_PWR_CTRL1, 0x00000001, 2000 }, // GLOBAL_EN = 1 { 0x0000742C, 0x000000F9 }, { 0x00007438, 0x00580941 }, { 0x00000040, 0x000000CC }, @@ -89,8 +89,8 @@ static const struct reg_sequence cs35l41_active_to_safe[] = { { 0x00000040, 0x00000055 }, { 0x00000040, 0x000000AA }, { 0x00007438, 0x00585941 }, - { CS35L41_AMP_DIG_VOL_CTRL, 0x0000A678 }, //AMP_VOL_PCM Mute - { CS35L41_PWR_CTRL2, 0x00000000 }, //AMP_EN = 0 + { CS35L41_AMP_DIG_VOL_CTRL, 0x0000A678 }, // AMP_VOL_PCM Mute + { CS35L41_PWR_CTRL2, 0x00000000 }, // AMP_EN = 0 { CS35L41_PWR_CTRL1, 0x00000000 }, { 0x0000742C, 0x00000009, 2000 }, { 0x00007438, 0x00580941 }, @@ -161,11 +161,13 @@ static void cs35l41_hda_playback_hook(struct device *dev, int action) if (reg_seq->close) ret = regmap_multi_reg_write(reg, reg_seq->close, reg_seq->num_close); break; + default: + ret = -EINVAL; + break; } if (ret) dev_warn(cs35l41->dev, "Failed to apply multi reg write: %d\n", ret); - } static int cs35l41_hda_channel_map(struct device *dev, unsigned int tx_num, unsigned int *tx_slot, @@ -182,20 +184,19 @@ static int cs35l41_hda_bind(struct device *dev, struct device *master, void *mas struct cs35l41_hda *cs35l41 = dev_get_drvdata(dev); struct hda_component *comps = master_data; - if (comps && cs35l41->index >= 0 && cs35l41->index < HDA_MAX_COMPONENTS) - comps = &comps[cs35l41->index]; - else + if (!comps || cs35l41->index < 0 || cs35l41->index >= HDA_MAX_COMPONENTS) return -EINVAL; - if (!comps->dev) { - comps->dev = dev; - strscpy(comps->name, dev_name(dev), sizeof(comps->name)); - comps->playback_hook = cs35l41_hda_playback_hook; - comps->set_channel_map = cs35l41_hda_channel_map; - return 0; - } + comps = &comps[cs35l41->index]; + if (comps->dev) + return -EBUSY; + + comps->dev = dev; + strscpy(comps->name, dev_name(dev), sizeof(comps->name)); + comps->playback_hook = cs35l41_hda_playback_hook; + comps->set_channel_map = cs35l41_hda_channel_map; - return -EBUSY; + return 0; } static void cs35l41_hda_unbind(struct device *dev, struct device *master, void *master_data) @@ -235,6 +236,8 @@ static int cs35l41_hda_apply_properties(struct cs35l41_hda *cs35l41, regmap_update_bits(cs35l41->regmap, CS35L41_GPIO_PAD_CONTROL, CS35L41_GPIO1_CTRL_MASK, 2 << CS35L41_GPIO1_CTRL_SHIFT); break; + default: + return -EINVAL; } switch (hw_cfg->gpio2_func) { @@ -242,6 +245,8 @@ static int cs35l41_hda_apply_properties(struct cs35l41_hda *cs35l41, regmap_update_bits(cs35l41->regmap, CS35L41_GPIO_PAD_CONTROL, CS35L41_GPIO2_CTRL_MASK, 2 << CS35L41_GPIO2_CTRL_SHIFT); break; + default: + return -EINVAL; } if (internal_boost) { @@ -256,11 +261,7 @@ static int cs35l41_hda_apply_properties(struct cs35l41_hda *cs35l41, cs35l41->reg_seq = &cs35l41_hda_reg_seq_ext_bst; } - ret = cs35l41_hda_channel_map(cs35l41->dev, 0, NULL, 1, (unsigned int *)&hw_cfg->spk_pos); - if (ret) - return ret; - - return 0; + return cs35l41_hda_channel_map(cs35l41->dev, 0, NULL, 1, (unsigned int *)&hw_cfg->spk_pos); } static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *cs35l41, @@ -269,7 +270,7 @@ static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *c struct cs35l41_hda_hw_config *hw_cfg; u32 values[HDA_MAX_COMPONENTS]; struct acpi_device *adev; - struct device *acpi_dev; + struct device *physdev; char *property; size_t nval; int i, ret; @@ -280,11 +281,11 @@ static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *c return ERR_PTR(-ENODEV); } - acpi_dev = get_device(acpi_get_first_physical_node(adev)); + physdev = get_device(acpi_get_first_physical_node(adev)); acpi_dev_put(adev); property = "cirrus,dev-index"; - ret = device_property_count_u32(acpi_dev, property); + ret = device_property_count_u32(physdev, property); if (ret <= 0) goto no_acpi_dsd; @@ -294,7 +295,7 @@ static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *c } nval = ret; - ret = device_property_read_u32_array(acpi_dev, property, values, nval); + ret = device_property_read_u32_array(physdev, property, values, nval); if (ret) goto err; @@ -311,7 +312,9 @@ static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *c goto err; } - /* No devm_ version as CLSA0100, in no_acpi_dsd case, can't use devm version */ + /* To use the same release code for all laptop variants we can't use devm_ version of + * gpiod_get here, as CLSA010* don't have a fully functional bios with an _DSD node + */ cs35l41->reset_gpio = fwnode_gpiod_get_index(&adev->fwnode, "reset", cs35l41->index, GPIOD_OUT_LOW, "cs35l41-reset"); @@ -322,46 +325,46 @@ static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *c } property = "cirrus,speaker-position"; - ret = device_property_read_u32_array(acpi_dev, property, values, nval); + ret = device_property_read_u32_array(physdev, property, values, nval); if (ret) goto err_free; hw_cfg->spk_pos = values[cs35l41->index]; property = "cirrus,gpio1-func"; - ret = device_property_read_u32_array(acpi_dev, property, values, nval); + ret = device_property_read_u32_array(physdev, property, values, nval); if (ret) goto err_free; hw_cfg->gpio1_func = values[cs35l41->index]; property = "cirrus,gpio2-func"; - ret = device_property_read_u32_array(acpi_dev, property, values, nval); + ret = device_property_read_u32_array(physdev, property, values, nval); if (ret) goto err_free; hw_cfg->gpio2_func = values[cs35l41->index]; property = "cirrus,boost-peak-milliamp"; - ret = device_property_read_u32_array(acpi_dev, property, values, nval); + ret = device_property_read_u32_array(physdev, property, values, nval); if (ret == 0) hw_cfg->bst_ipk = values[cs35l41->index]; property = "cirrus,boost-ind-nanohenry"; - ret = device_property_read_u32_array(acpi_dev, property, values, nval); + ret = device_property_read_u32_array(physdev, property, values, nval); if (ret == 0) hw_cfg->bst_ind = values[cs35l41->index]; property = "cirrus,boost-cap-microfarad"; - ret = device_property_read_u32_array(acpi_dev, property, values, nval); + ret = device_property_read_u32_array(physdev, property, values, nval); if (ret == 0) hw_cfg->bst_cap = values[cs35l41->index]; - put_device(acpi_dev); + put_device(physdev); return hw_cfg; err_free: kfree(hw_cfg); err: - put_device(acpi_dev); + put_device(physdev); dev_err(cs35l41->dev, "Failed property %s: %d\n", property, ret); return ERR_PTR(ret); @@ -370,18 +373,18 @@ static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *c /* * Device CLSA0100 doesn't have _DSD so a gpiod_get by the label reset won't work. * And devices created by i2c-multi-instantiate don't have their device struct pointing to - * the correct fwnode, so acpi_dev must be used here + * the correct fwnode, so acpi_dev must be used here. * And devm functions expect that the device requesting the resource has the correct - * fwnode + * fwnode. */ if (strncmp(hid, "CLSA0100", 8) != 0) return ERR_PTR(-EINVAL); /* check I2C address to assign the index */ cs35l41->index = id == 0x40 ? 0 : 1; - cs35l41->reset_gpio = gpiod_get_index(acpi_dev, NULL, 0, GPIOD_OUT_HIGH); + cs35l41->reset_gpio = gpiod_get_index(physdev, NULL, 0, GPIOD_OUT_HIGH); cs35l41->vspk_always_on = true; - put_device(acpi_dev); + put_device(physdev); return NULL; } @@ -416,8 +419,7 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i if (ret == -EBUSY) { dev_info(cs35l41->dev, "Reset line busy, assuming shared reset\n"); } else { - if (ret != -EPROBE_DEFER) - dev_err(cs35l41->dev, "Failed to get reset GPIO: %d\n", ret); + dev_err_probe(cs35l41->dev, ret, "Failed to get reset GPIO: %d\n", ret); goto err; } } @@ -437,7 +439,8 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i ret = regmap_read(cs35l41->regmap, CS35L41_IRQ1_STATUS3, &int_sts); if (ret || (int_sts & CS35L41_OTP_BOOT_ERR)) { - dev_err(cs35l41->dev, "OTP Boot error\n"); + dev_err(cs35l41->dev, "OTP Boot status %x error: %d\n", + int_sts & CS35L41_OTP_BOOT_ERR, ret); ret = -EIO; goto err; } @@ -489,7 +492,7 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i if (cs35l41->reg_seq->probe) { ret = regmap_multi_reg_write(cs35l41->regmap, cs35l41->reg_seq->probe, - cs35l41->reg_seq->num_probe); + cs35l41->reg_seq->num_probe); if (ret) { dev_err(cs35l41->dev, "Fail to apply probe reg patch: %d\n", ret); goto err; diff --git a/sound/pci/hda/cs35l41_hda.h b/sound/pci/hda/cs35l41_hda.h index 76c69a8a22f6..640afc98b686 100644 --- a/sound/pci/hda/cs35l41_hda.h +++ b/sound/pci/hda/cs35l41_hda.h @@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0 * - * cs35l41_hda.h -- CS35L41 ALSA HDA audio driver + * CS35L41 ALSA HDA audio driver * * Copyright 2021 Cirrus Logic, Inc. * diff --git a/sound/pci/hda/cs35l41_hda_i2c.c b/sound/pci/hda/cs35l41_hda_i2c.c index eeb387853ee3..c2397dc53e78 100644 --- a/sound/pci/hda/cs35l41_hda_i2c.c +++ b/sound/pci/hda/cs35l41_hda_i2c.c @@ -58,7 +58,6 @@ static struct i2c_driver cs35l41_i2c_driver = { .probe = cs35l41_hda_i2c_probe, .remove = cs35l41_hda_i2c_remove, }; - module_i2c_driver(cs35l41_i2c_driver); MODULE_DESCRIPTION("HDA CS35L41 driver"); diff --git a/sound/pci/hda/cs35l41_hda_spi.c b/sound/pci/hda/cs35l41_hda_spi.c index 15345a72b9d1..36815ab4e461 100644 --- a/sound/pci/hda/cs35l41_hda_spi.c +++ b/sound/pci/hda/cs35l41_hda_spi.c @@ -55,7 +55,6 @@ static struct spi_driver cs35l41_spi_driver = { .probe = cs35l41_hda_spi_probe, .remove = cs35l41_hda_spi_remove, }; - module_spi_driver(cs35l41_spi_driver); MODULE_DESCRIPTION("HDA CS35L41 driver"); -- 2.34.1 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 4/5] ALSA: hda: cs35l41: Tidyup code and add default cases 2022-01-13 17:07 ` Lucas Tanure @ 2022-01-14 13:04 ` Lucas tanure -1 siblings, 0 replies; 28+ messages in thread From: Lucas tanure @ 2022-01-14 13:04 UTC (permalink / raw) To: Rafael J . Wysocki, Len Brown, Hans de Goede, Mark Gross, Jaroslav Kysela, Takashi Iwai Cc: alsa-devel, linux-acpi, patches, platform-driver-x86, linux-kernel On 1/13/22 17:07, Lucas Tanure wrote: > Clean up the code, plus adding default cases for switch > and dev_err_probe use. > > Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> > --- > sound/pci/hda/cs35l41_hda.c | 109 ++++++++++++++++---------------- > sound/pci/hda/cs35l41_hda.h | 2 +- > sound/pci/hda/cs35l41_hda_i2c.c | 1 - > sound/pci/hda/cs35l41_hda_spi.c | 1 - > 4 files changed, 57 insertions(+), 56 deletions(-) > > diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c > index c4f25e48dcc0..cb8331587c17 100644 > --- a/sound/pci/hda/cs35l41_hda.c > +++ b/sound/pci/hda/cs35l41_hda.c > @@ -1,6 +1,6 @@ > // SPDX-License-Identifier: GPL-2.0 > // > -// cs35l41.c -- CS35l41 ALSA HDA audio driver > +// CS35l41 ALSA HDA audio driver > // > // Copyright 2021 Cirrus Logic, Inc. > // > @@ -17,19 +17,19 @@ > #include "cs35l41_hda.h" > > static const struct reg_sequence cs35l41_hda_config[] = { > - { CS35L41_PLL_CLK_CTRL, 0x00000430 }, //3200000Hz, BCLK Input, PLL_REFCLK_EN = 1 > - { CS35L41_GLOBAL_CLK_CTRL, 0x00000003 }, //GLOBAL_FS = 48 kHz > - { CS35L41_SP_ENABLES, 0x00010000 }, //ASP_RX1_EN = 1 > - { CS35L41_SP_RATE_CTRL, 0x00000021 }, //ASP_BCLK_FREQ = 3.072 MHz > - { CS35L41_SP_FORMAT, 0x20200200 }, //24 bits, I2S, BCLK Slave, FSYNC Slave > - { CS35L41_DAC_PCM1_SRC, 0x00000008 }, //DACPCM1_SRC = ASPRX1 > - { CS35L41_AMP_DIG_VOL_CTRL, 0x00000000 }, //AMP_VOL_PCM 0.0 dB > - { CS35L41_AMP_GAIN_CTRL, 0x00000084 }, //AMP_GAIN_PCM 4.5 dB > - { CS35L41_PWR_CTRL2, 0x00000001 }, //AMP_EN = 1 > + { CS35L41_PLL_CLK_CTRL, 0x00000430 }, // 3200000Hz, BCLK Input, PLL_REFCLK_EN = 1 > + { CS35L41_GLOBAL_CLK_CTRL, 0x00000003 }, // GLOBAL_FS = 48 kHz > + { CS35L41_SP_ENABLES, 0x00010000 }, // ASP_RX1_EN = 1 > + { CS35L41_SP_RATE_CTRL, 0x00000021 }, // ASP_BCLK_FREQ = 3.072 MHz > + { CS35L41_SP_FORMAT, 0x20200200 }, // 24 bits, I2S, BCLK Slave, FSYNC Slave > + { CS35L41_DAC_PCM1_SRC, 0x00000008 }, // DACPCM1_SRC = ASPRX1 > + { CS35L41_AMP_DIG_VOL_CTRL, 0x00000000 }, // AMP_VOL_PCM 0.0 dB > + { CS35L41_AMP_GAIN_CTRL, 0x00000084 }, // AMP_GAIN_PCM 4.5 dB > + { CS35L41_PWR_CTRL2, 0x00000001 }, // AMP_EN = 1 > }; > > static const struct reg_sequence cs35l41_hda_start_bst[] = { > - { CS35L41_PWR_CTRL2, 0x00000021 }, //BST_EN = 10, AMP_EN = 1 > + { CS35L41_PWR_CTRL2, 0x00000021 }, // BST_EN = 10, AMP_EN = 1 > { CS35L41_PWR_CTRL1, 0x00000001, 3000}, // set GLOBAL_EN = 1 > }; > > @@ -60,7 +60,7 @@ static const struct reg_sequence cs35l41_stop_ext_vspk[] = { > { 0x00000040, 0x00000055 }, > { 0x00000040, 0x000000AA }, > { 0x00007438, 0x00585941 }, > - { 0x00002014, 0x00000000, 3000}, //set GLOBAL_EN = 0 > + { 0x00002014, 0x00000000, 3000}, // set GLOBAL_EN = 0 > { 0x0000742C, 0x00000009 }, > { 0x00007438, 0x00580941 }, > { 0x00011008, 0x00000001 }, > @@ -78,7 +78,7 @@ static const struct reg_sequence cs35l41_safe_to_active[] = { > { 0x0000742C, 0x0000000F }, > { 0x0000742C, 0x00000079 }, > { 0x00007438, 0x00585941 }, > - { CS35L41_PWR_CTRL1, 0x00000001, 2000 }, //GLOBAL_EN = 1 > + { CS35L41_PWR_CTRL1, 0x00000001, 2000 }, // GLOBAL_EN = 1 > { 0x0000742C, 0x000000F9 }, > { 0x00007438, 0x00580941 }, > { 0x00000040, 0x000000CC }, > @@ -89,8 +89,8 @@ static const struct reg_sequence cs35l41_active_to_safe[] = { > { 0x00000040, 0x00000055 }, > { 0x00000040, 0x000000AA }, > { 0x00007438, 0x00585941 }, > - { CS35L41_AMP_DIG_VOL_CTRL, 0x0000A678 }, //AMP_VOL_PCM Mute > - { CS35L41_PWR_CTRL2, 0x00000000 }, //AMP_EN = 0 > + { CS35L41_AMP_DIG_VOL_CTRL, 0x0000A678 }, // AMP_VOL_PCM Mute > + { CS35L41_PWR_CTRL2, 0x00000000 }, // AMP_EN = 0 > { CS35L41_PWR_CTRL1, 0x00000000 }, > { 0x0000742C, 0x00000009, 2000 }, > { 0x00007438, 0x00580941 }, > @@ -161,11 +161,13 @@ static void cs35l41_hda_playback_hook(struct device *dev, int action) > if (reg_seq->close) > ret = regmap_multi_reg_write(reg, reg_seq->close, reg_seq->num_close); > break; > + default: > + ret = -EINVAL; > + break; > } > > if (ret) > dev_warn(cs35l41->dev, "Failed to apply multi reg write: %d\n", ret); > - > } > > static int cs35l41_hda_channel_map(struct device *dev, unsigned int tx_num, unsigned int *tx_slot, > @@ -182,20 +184,19 @@ static int cs35l41_hda_bind(struct device *dev, struct device *master, void *mas > struct cs35l41_hda *cs35l41 = dev_get_drvdata(dev); > struct hda_component *comps = master_data; > > - if (comps && cs35l41->index >= 0 && cs35l41->index < HDA_MAX_COMPONENTS) > - comps = &comps[cs35l41->index]; > - else > + if (!comps || cs35l41->index < 0 || cs35l41->index >= HDA_MAX_COMPONENTS) > return -EINVAL; > > - if (!comps->dev) { > - comps->dev = dev; > - strscpy(comps->name, dev_name(dev), sizeof(comps->name)); > - comps->playback_hook = cs35l41_hda_playback_hook; > - comps->set_channel_map = cs35l41_hda_channel_map; > - return 0; > - } > + comps = &comps[cs35l41->index]; > + if (comps->dev) > + return -EBUSY; > + > + comps->dev = dev; > + strscpy(comps->name, dev_name(dev), sizeof(comps->name)); > + comps->playback_hook = cs35l41_hda_playback_hook; > + comps->set_channel_map = cs35l41_hda_channel_map; > > - return -EBUSY; > + return 0; > } > > static void cs35l41_hda_unbind(struct device *dev, struct device *master, void *master_data) > @@ -235,6 +236,8 @@ static int cs35l41_hda_apply_properties(struct cs35l41_hda *cs35l41, > regmap_update_bits(cs35l41->regmap, CS35L41_GPIO_PAD_CONTROL, > CS35L41_GPIO1_CTRL_MASK, 2 << CS35L41_GPIO1_CTRL_SHIFT); > break; > + default: > + return -EINVAL; > } > > switch (hw_cfg->gpio2_func) { > @@ -242,6 +245,8 @@ static int cs35l41_hda_apply_properties(struct cs35l41_hda *cs35l41, > regmap_update_bits(cs35l41->regmap, CS35L41_GPIO_PAD_CONTROL, > CS35L41_GPIO2_CTRL_MASK, 2 << CS35L41_GPIO2_CTRL_SHIFT); > break; > + default: > + return -EINVAL; This default option introduces issues some laptops. A new version of this series will be sent. > } > > if (internal_boost) { > @@ -256,11 +261,7 @@ static int cs35l41_hda_apply_properties(struct cs35l41_hda *cs35l41, > cs35l41->reg_seq = &cs35l41_hda_reg_seq_ext_bst; > } > > - ret = cs35l41_hda_channel_map(cs35l41->dev, 0, NULL, 1, (unsigned int *)&hw_cfg->spk_pos); > - if (ret) > - return ret; > - > - return 0; > + return cs35l41_hda_channel_map(cs35l41->dev, 0, NULL, 1, (unsigned int *)&hw_cfg->spk_pos); > } > > static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *cs35l41, > @@ -269,7 +270,7 @@ static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *c > struct cs35l41_hda_hw_config *hw_cfg; > u32 values[HDA_MAX_COMPONENTS]; > struct acpi_device *adev; > - struct device *acpi_dev; > + struct device *physdev; > char *property; > size_t nval; > int i, ret; > @@ -280,11 +281,11 @@ static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *c > return ERR_PTR(-ENODEV); > } > > - acpi_dev = get_device(acpi_get_first_physical_node(adev)); > + physdev = get_device(acpi_get_first_physical_node(adev)); > acpi_dev_put(adev); > > property = "cirrus,dev-index"; > - ret = device_property_count_u32(acpi_dev, property); > + ret = device_property_count_u32(physdev, property); > if (ret <= 0) > goto no_acpi_dsd; > > @@ -294,7 +295,7 @@ static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *c > } > nval = ret; > > - ret = device_property_read_u32_array(acpi_dev, property, values, nval); > + ret = device_property_read_u32_array(physdev, property, values, nval); > if (ret) > goto err; > > @@ -311,7 +312,9 @@ static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *c > goto err; > } > > - /* No devm_ version as CLSA0100, in no_acpi_dsd case, can't use devm version */ > + /* To use the same release code for all laptop variants we can't use devm_ version of > + * gpiod_get here, as CLSA010* don't have a fully functional bios with an _DSD node > + */ > cs35l41->reset_gpio = fwnode_gpiod_get_index(&adev->fwnode, "reset", cs35l41->index, > GPIOD_OUT_LOW, "cs35l41-reset"); > > @@ -322,46 +325,46 @@ static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *c > } > > property = "cirrus,speaker-position"; > - ret = device_property_read_u32_array(acpi_dev, property, values, nval); > + ret = device_property_read_u32_array(physdev, property, values, nval); > if (ret) > goto err_free; > hw_cfg->spk_pos = values[cs35l41->index]; > > property = "cirrus,gpio1-func"; > - ret = device_property_read_u32_array(acpi_dev, property, values, nval); > + ret = device_property_read_u32_array(physdev, property, values, nval); > if (ret) > goto err_free; > hw_cfg->gpio1_func = values[cs35l41->index]; > > property = "cirrus,gpio2-func"; > - ret = device_property_read_u32_array(acpi_dev, property, values, nval); > + ret = device_property_read_u32_array(physdev, property, values, nval); > if (ret) > goto err_free; > hw_cfg->gpio2_func = values[cs35l41->index]; > > property = "cirrus,boost-peak-milliamp"; > - ret = device_property_read_u32_array(acpi_dev, property, values, nval); > + ret = device_property_read_u32_array(physdev, property, values, nval); > if (ret == 0) > hw_cfg->bst_ipk = values[cs35l41->index]; > > property = "cirrus,boost-ind-nanohenry"; > - ret = device_property_read_u32_array(acpi_dev, property, values, nval); > + ret = device_property_read_u32_array(physdev, property, values, nval); > if (ret == 0) > hw_cfg->bst_ind = values[cs35l41->index]; > > property = "cirrus,boost-cap-microfarad"; > - ret = device_property_read_u32_array(acpi_dev, property, values, nval); > + ret = device_property_read_u32_array(physdev, property, values, nval); > if (ret == 0) > hw_cfg->bst_cap = values[cs35l41->index]; > > - put_device(acpi_dev); > + put_device(physdev); > > return hw_cfg; > > err_free: > kfree(hw_cfg); > err: > - put_device(acpi_dev); > + put_device(physdev); > dev_err(cs35l41->dev, "Failed property %s: %d\n", property, ret); > > return ERR_PTR(ret); > @@ -370,18 +373,18 @@ static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *c > /* > * Device CLSA0100 doesn't have _DSD so a gpiod_get by the label reset won't work. > * And devices created by i2c-multi-instantiate don't have their device struct pointing to > - * the correct fwnode, so acpi_dev must be used here > + * the correct fwnode, so acpi_dev must be used here. > * And devm functions expect that the device requesting the resource has the correct > - * fwnode > + * fwnode. > */ > if (strncmp(hid, "CLSA0100", 8) != 0) > return ERR_PTR(-EINVAL); > > /* check I2C address to assign the index */ > cs35l41->index = id == 0x40 ? 0 : 1; > - cs35l41->reset_gpio = gpiod_get_index(acpi_dev, NULL, 0, GPIOD_OUT_HIGH); > + cs35l41->reset_gpio = gpiod_get_index(physdev, NULL, 0, GPIOD_OUT_HIGH); > cs35l41->vspk_always_on = true; > - put_device(acpi_dev); > + put_device(physdev); > > return NULL; > } > @@ -416,8 +419,7 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i > if (ret == -EBUSY) { > dev_info(cs35l41->dev, "Reset line busy, assuming shared reset\n"); > } else { > - if (ret != -EPROBE_DEFER) > - dev_err(cs35l41->dev, "Failed to get reset GPIO: %d\n", ret); > + dev_err_probe(cs35l41->dev, ret, "Failed to get reset GPIO: %d\n", ret); > goto err; > } > } > @@ -437,7 +439,8 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i > > ret = regmap_read(cs35l41->regmap, CS35L41_IRQ1_STATUS3, &int_sts); > if (ret || (int_sts & CS35L41_OTP_BOOT_ERR)) { > - dev_err(cs35l41->dev, "OTP Boot error\n"); > + dev_err(cs35l41->dev, "OTP Boot status %x error: %d\n", > + int_sts & CS35L41_OTP_BOOT_ERR, ret); > ret = -EIO; > goto err; > } > @@ -489,7 +492,7 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i > > if (cs35l41->reg_seq->probe) { > ret = regmap_multi_reg_write(cs35l41->regmap, cs35l41->reg_seq->probe, > - cs35l41->reg_seq->num_probe); > + cs35l41->reg_seq->num_probe); > if (ret) { > dev_err(cs35l41->dev, "Fail to apply probe reg patch: %d\n", ret); > goto err; > diff --git a/sound/pci/hda/cs35l41_hda.h b/sound/pci/hda/cs35l41_hda.h > index 76c69a8a22f6..640afc98b686 100644 > --- a/sound/pci/hda/cs35l41_hda.h > +++ b/sound/pci/hda/cs35l41_hda.h > @@ -1,6 +1,6 @@ > /* SPDX-License-Identifier: GPL-2.0 > * > - * cs35l41_hda.h -- CS35L41 ALSA HDA audio driver > + * CS35L41 ALSA HDA audio driver > * > * Copyright 2021 Cirrus Logic, Inc. > * > diff --git a/sound/pci/hda/cs35l41_hda_i2c.c b/sound/pci/hda/cs35l41_hda_i2c.c > index eeb387853ee3..c2397dc53e78 100644 > --- a/sound/pci/hda/cs35l41_hda_i2c.c > +++ b/sound/pci/hda/cs35l41_hda_i2c.c > @@ -58,7 +58,6 @@ static struct i2c_driver cs35l41_i2c_driver = { > .probe = cs35l41_hda_i2c_probe, > .remove = cs35l41_hda_i2c_remove, > }; > - > module_i2c_driver(cs35l41_i2c_driver); > > MODULE_DESCRIPTION("HDA CS35L41 driver"); > diff --git a/sound/pci/hda/cs35l41_hda_spi.c b/sound/pci/hda/cs35l41_hda_spi.c > index 15345a72b9d1..36815ab4e461 100644 > --- a/sound/pci/hda/cs35l41_hda_spi.c > +++ b/sound/pci/hda/cs35l41_hda_spi.c > @@ -55,7 +55,6 @@ static struct spi_driver cs35l41_spi_driver = { > .probe = cs35l41_hda_spi_probe, > .remove = cs35l41_hda_spi_remove, > }; > - > module_spi_driver(cs35l41_spi_driver); > > MODULE_DESCRIPTION("HDA CS35L41 driver"); ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/5] ALSA: hda: cs35l41: Tidyup code and add default cases @ 2022-01-14 13:04 ` Lucas tanure 0 siblings, 0 replies; 28+ messages in thread From: Lucas tanure @ 2022-01-14 13:04 UTC (permalink / raw) To: Rafael J . Wysocki, Len Brown, Hans de Goede, Mark Gross, Jaroslav Kysela, Takashi Iwai Cc: linux-acpi, alsa-devel, platform-driver-x86, linux-kernel, patches On 1/13/22 17:07, Lucas Tanure wrote: > Clean up the code, plus adding default cases for switch > and dev_err_probe use. > > Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> > --- > sound/pci/hda/cs35l41_hda.c | 109 ++++++++++++++++---------------- > sound/pci/hda/cs35l41_hda.h | 2 +- > sound/pci/hda/cs35l41_hda_i2c.c | 1 - > sound/pci/hda/cs35l41_hda_spi.c | 1 - > 4 files changed, 57 insertions(+), 56 deletions(-) > > diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c > index c4f25e48dcc0..cb8331587c17 100644 > --- a/sound/pci/hda/cs35l41_hda.c > +++ b/sound/pci/hda/cs35l41_hda.c > @@ -1,6 +1,6 @@ > // SPDX-License-Identifier: GPL-2.0 > // > -// cs35l41.c -- CS35l41 ALSA HDA audio driver > +// CS35l41 ALSA HDA audio driver > // > // Copyright 2021 Cirrus Logic, Inc. > // > @@ -17,19 +17,19 @@ > #include "cs35l41_hda.h" > > static const struct reg_sequence cs35l41_hda_config[] = { > - { CS35L41_PLL_CLK_CTRL, 0x00000430 }, //3200000Hz, BCLK Input, PLL_REFCLK_EN = 1 > - { CS35L41_GLOBAL_CLK_CTRL, 0x00000003 }, //GLOBAL_FS = 48 kHz > - { CS35L41_SP_ENABLES, 0x00010000 }, //ASP_RX1_EN = 1 > - { CS35L41_SP_RATE_CTRL, 0x00000021 }, //ASP_BCLK_FREQ = 3.072 MHz > - { CS35L41_SP_FORMAT, 0x20200200 }, //24 bits, I2S, BCLK Slave, FSYNC Slave > - { CS35L41_DAC_PCM1_SRC, 0x00000008 }, //DACPCM1_SRC = ASPRX1 > - { CS35L41_AMP_DIG_VOL_CTRL, 0x00000000 }, //AMP_VOL_PCM 0.0 dB > - { CS35L41_AMP_GAIN_CTRL, 0x00000084 }, //AMP_GAIN_PCM 4.5 dB > - { CS35L41_PWR_CTRL2, 0x00000001 }, //AMP_EN = 1 > + { CS35L41_PLL_CLK_CTRL, 0x00000430 }, // 3200000Hz, BCLK Input, PLL_REFCLK_EN = 1 > + { CS35L41_GLOBAL_CLK_CTRL, 0x00000003 }, // GLOBAL_FS = 48 kHz > + { CS35L41_SP_ENABLES, 0x00010000 }, // ASP_RX1_EN = 1 > + { CS35L41_SP_RATE_CTRL, 0x00000021 }, // ASP_BCLK_FREQ = 3.072 MHz > + { CS35L41_SP_FORMAT, 0x20200200 }, // 24 bits, I2S, BCLK Slave, FSYNC Slave > + { CS35L41_DAC_PCM1_SRC, 0x00000008 }, // DACPCM1_SRC = ASPRX1 > + { CS35L41_AMP_DIG_VOL_CTRL, 0x00000000 }, // AMP_VOL_PCM 0.0 dB > + { CS35L41_AMP_GAIN_CTRL, 0x00000084 }, // AMP_GAIN_PCM 4.5 dB > + { CS35L41_PWR_CTRL2, 0x00000001 }, // AMP_EN = 1 > }; > > static const struct reg_sequence cs35l41_hda_start_bst[] = { > - { CS35L41_PWR_CTRL2, 0x00000021 }, //BST_EN = 10, AMP_EN = 1 > + { CS35L41_PWR_CTRL2, 0x00000021 }, // BST_EN = 10, AMP_EN = 1 > { CS35L41_PWR_CTRL1, 0x00000001, 3000}, // set GLOBAL_EN = 1 > }; > > @@ -60,7 +60,7 @@ static const struct reg_sequence cs35l41_stop_ext_vspk[] = { > { 0x00000040, 0x00000055 }, > { 0x00000040, 0x000000AA }, > { 0x00007438, 0x00585941 }, > - { 0x00002014, 0x00000000, 3000}, //set GLOBAL_EN = 0 > + { 0x00002014, 0x00000000, 3000}, // set GLOBAL_EN = 0 > { 0x0000742C, 0x00000009 }, > { 0x00007438, 0x00580941 }, > { 0x00011008, 0x00000001 }, > @@ -78,7 +78,7 @@ static const struct reg_sequence cs35l41_safe_to_active[] = { > { 0x0000742C, 0x0000000F }, > { 0x0000742C, 0x00000079 }, > { 0x00007438, 0x00585941 }, > - { CS35L41_PWR_CTRL1, 0x00000001, 2000 }, //GLOBAL_EN = 1 > + { CS35L41_PWR_CTRL1, 0x00000001, 2000 }, // GLOBAL_EN = 1 > { 0x0000742C, 0x000000F9 }, > { 0x00007438, 0x00580941 }, > { 0x00000040, 0x000000CC }, > @@ -89,8 +89,8 @@ static const struct reg_sequence cs35l41_active_to_safe[] = { > { 0x00000040, 0x00000055 }, > { 0x00000040, 0x000000AA }, > { 0x00007438, 0x00585941 }, > - { CS35L41_AMP_DIG_VOL_CTRL, 0x0000A678 }, //AMP_VOL_PCM Mute > - { CS35L41_PWR_CTRL2, 0x00000000 }, //AMP_EN = 0 > + { CS35L41_AMP_DIG_VOL_CTRL, 0x0000A678 }, // AMP_VOL_PCM Mute > + { CS35L41_PWR_CTRL2, 0x00000000 }, // AMP_EN = 0 > { CS35L41_PWR_CTRL1, 0x00000000 }, > { 0x0000742C, 0x00000009, 2000 }, > { 0x00007438, 0x00580941 }, > @@ -161,11 +161,13 @@ static void cs35l41_hda_playback_hook(struct device *dev, int action) > if (reg_seq->close) > ret = regmap_multi_reg_write(reg, reg_seq->close, reg_seq->num_close); > break; > + default: > + ret = -EINVAL; > + break; > } > > if (ret) > dev_warn(cs35l41->dev, "Failed to apply multi reg write: %d\n", ret); > - > } > > static int cs35l41_hda_channel_map(struct device *dev, unsigned int tx_num, unsigned int *tx_slot, > @@ -182,20 +184,19 @@ static int cs35l41_hda_bind(struct device *dev, struct device *master, void *mas > struct cs35l41_hda *cs35l41 = dev_get_drvdata(dev); > struct hda_component *comps = master_data; > > - if (comps && cs35l41->index >= 0 && cs35l41->index < HDA_MAX_COMPONENTS) > - comps = &comps[cs35l41->index]; > - else > + if (!comps || cs35l41->index < 0 || cs35l41->index >= HDA_MAX_COMPONENTS) > return -EINVAL; > > - if (!comps->dev) { > - comps->dev = dev; > - strscpy(comps->name, dev_name(dev), sizeof(comps->name)); > - comps->playback_hook = cs35l41_hda_playback_hook; > - comps->set_channel_map = cs35l41_hda_channel_map; > - return 0; > - } > + comps = &comps[cs35l41->index]; > + if (comps->dev) > + return -EBUSY; > + > + comps->dev = dev; > + strscpy(comps->name, dev_name(dev), sizeof(comps->name)); > + comps->playback_hook = cs35l41_hda_playback_hook; > + comps->set_channel_map = cs35l41_hda_channel_map; > > - return -EBUSY; > + return 0; > } > > static void cs35l41_hda_unbind(struct device *dev, struct device *master, void *master_data) > @@ -235,6 +236,8 @@ static int cs35l41_hda_apply_properties(struct cs35l41_hda *cs35l41, > regmap_update_bits(cs35l41->regmap, CS35L41_GPIO_PAD_CONTROL, > CS35L41_GPIO1_CTRL_MASK, 2 << CS35L41_GPIO1_CTRL_SHIFT); > break; > + default: > + return -EINVAL; > } > > switch (hw_cfg->gpio2_func) { > @@ -242,6 +245,8 @@ static int cs35l41_hda_apply_properties(struct cs35l41_hda *cs35l41, > regmap_update_bits(cs35l41->regmap, CS35L41_GPIO_PAD_CONTROL, > CS35L41_GPIO2_CTRL_MASK, 2 << CS35L41_GPIO2_CTRL_SHIFT); > break; > + default: > + return -EINVAL; This default option introduces issues some laptops. A new version of this series will be sent. > } > > if (internal_boost) { > @@ -256,11 +261,7 @@ static int cs35l41_hda_apply_properties(struct cs35l41_hda *cs35l41, > cs35l41->reg_seq = &cs35l41_hda_reg_seq_ext_bst; > } > > - ret = cs35l41_hda_channel_map(cs35l41->dev, 0, NULL, 1, (unsigned int *)&hw_cfg->spk_pos); > - if (ret) > - return ret; > - > - return 0; > + return cs35l41_hda_channel_map(cs35l41->dev, 0, NULL, 1, (unsigned int *)&hw_cfg->spk_pos); > } > > static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *cs35l41, > @@ -269,7 +270,7 @@ static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *c > struct cs35l41_hda_hw_config *hw_cfg; > u32 values[HDA_MAX_COMPONENTS]; > struct acpi_device *adev; > - struct device *acpi_dev; > + struct device *physdev; > char *property; > size_t nval; > int i, ret; > @@ -280,11 +281,11 @@ static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *c > return ERR_PTR(-ENODEV); > } > > - acpi_dev = get_device(acpi_get_first_physical_node(adev)); > + physdev = get_device(acpi_get_first_physical_node(adev)); > acpi_dev_put(adev); > > property = "cirrus,dev-index"; > - ret = device_property_count_u32(acpi_dev, property); > + ret = device_property_count_u32(physdev, property); > if (ret <= 0) > goto no_acpi_dsd; > > @@ -294,7 +295,7 @@ static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *c > } > nval = ret; > > - ret = device_property_read_u32_array(acpi_dev, property, values, nval); > + ret = device_property_read_u32_array(physdev, property, values, nval); > if (ret) > goto err; > > @@ -311,7 +312,9 @@ static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *c > goto err; > } > > - /* No devm_ version as CLSA0100, in no_acpi_dsd case, can't use devm version */ > + /* To use the same release code for all laptop variants we can't use devm_ version of > + * gpiod_get here, as CLSA010* don't have a fully functional bios with an _DSD node > + */ > cs35l41->reset_gpio = fwnode_gpiod_get_index(&adev->fwnode, "reset", cs35l41->index, > GPIOD_OUT_LOW, "cs35l41-reset"); > > @@ -322,46 +325,46 @@ static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *c > } > > property = "cirrus,speaker-position"; > - ret = device_property_read_u32_array(acpi_dev, property, values, nval); > + ret = device_property_read_u32_array(physdev, property, values, nval); > if (ret) > goto err_free; > hw_cfg->spk_pos = values[cs35l41->index]; > > property = "cirrus,gpio1-func"; > - ret = device_property_read_u32_array(acpi_dev, property, values, nval); > + ret = device_property_read_u32_array(physdev, property, values, nval); > if (ret) > goto err_free; > hw_cfg->gpio1_func = values[cs35l41->index]; > > property = "cirrus,gpio2-func"; > - ret = device_property_read_u32_array(acpi_dev, property, values, nval); > + ret = device_property_read_u32_array(physdev, property, values, nval); > if (ret) > goto err_free; > hw_cfg->gpio2_func = values[cs35l41->index]; > > property = "cirrus,boost-peak-milliamp"; > - ret = device_property_read_u32_array(acpi_dev, property, values, nval); > + ret = device_property_read_u32_array(physdev, property, values, nval); > if (ret == 0) > hw_cfg->bst_ipk = values[cs35l41->index]; > > property = "cirrus,boost-ind-nanohenry"; > - ret = device_property_read_u32_array(acpi_dev, property, values, nval); > + ret = device_property_read_u32_array(physdev, property, values, nval); > if (ret == 0) > hw_cfg->bst_ind = values[cs35l41->index]; > > property = "cirrus,boost-cap-microfarad"; > - ret = device_property_read_u32_array(acpi_dev, property, values, nval); > + ret = device_property_read_u32_array(physdev, property, values, nval); > if (ret == 0) > hw_cfg->bst_cap = values[cs35l41->index]; > > - put_device(acpi_dev); > + put_device(physdev); > > return hw_cfg; > > err_free: > kfree(hw_cfg); > err: > - put_device(acpi_dev); > + put_device(physdev); > dev_err(cs35l41->dev, "Failed property %s: %d\n", property, ret); > > return ERR_PTR(ret); > @@ -370,18 +373,18 @@ static struct cs35l41_hda_hw_config *cs35l41_hda_read_acpi(struct cs35l41_hda *c > /* > * Device CLSA0100 doesn't have _DSD so a gpiod_get by the label reset won't work. > * And devices created by i2c-multi-instantiate don't have their device struct pointing to > - * the correct fwnode, so acpi_dev must be used here > + * the correct fwnode, so acpi_dev must be used here. > * And devm functions expect that the device requesting the resource has the correct > - * fwnode > + * fwnode. > */ > if (strncmp(hid, "CLSA0100", 8) != 0) > return ERR_PTR(-EINVAL); > > /* check I2C address to assign the index */ > cs35l41->index = id == 0x40 ? 0 : 1; > - cs35l41->reset_gpio = gpiod_get_index(acpi_dev, NULL, 0, GPIOD_OUT_HIGH); > + cs35l41->reset_gpio = gpiod_get_index(physdev, NULL, 0, GPIOD_OUT_HIGH); > cs35l41->vspk_always_on = true; > - put_device(acpi_dev); > + put_device(physdev); > > return NULL; > } > @@ -416,8 +419,7 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i > if (ret == -EBUSY) { > dev_info(cs35l41->dev, "Reset line busy, assuming shared reset\n"); > } else { > - if (ret != -EPROBE_DEFER) > - dev_err(cs35l41->dev, "Failed to get reset GPIO: %d\n", ret); > + dev_err_probe(cs35l41->dev, ret, "Failed to get reset GPIO: %d\n", ret); > goto err; > } > } > @@ -437,7 +439,8 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i > > ret = regmap_read(cs35l41->regmap, CS35L41_IRQ1_STATUS3, &int_sts); > if (ret || (int_sts & CS35L41_OTP_BOOT_ERR)) { > - dev_err(cs35l41->dev, "OTP Boot error\n"); > + dev_err(cs35l41->dev, "OTP Boot status %x error: %d\n", > + int_sts & CS35L41_OTP_BOOT_ERR, ret); > ret = -EIO; > goto err; > } > @@ -489,7 +492,7 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i > > if (cs35l41->reg_seq->probe) { > ret = regmap_multi_reg_write(cs35l41->regmap, cs35l41->reg_seq->probe, > - cs35l41->reg_seq->num_probe); > + cs35l41->reg_seq->num_probe); > if (ret) { > dev_err(cs35l41->dev, "Fail to apply probe reg patch: %d\n", ret); > goto err; > diff --git a/sound/pci/hda/cs35l41_hda.h b/sound/pci/hda/cs35l41_hda.h > index 76c69a8a22f6..640afc98b686 100644 > --- a/sound/pci/hda/cs35l41_hda.h > +++ b/sound/pci/hda/cs35l41_hda.h > @@ -1,6 +1,6 @@ > /* SPDX-License-Identifier: GPL-2.0 > * > - * cs35l41_hda.h -- CS35L41 ALSA HDA audio driver > + * CS35L41 ALSA HDA audio driver > * > * Copyright 2021 Cirrus Logic, Inc. > * > diff --git a/sound/pci/hda/cs35l41_hda_i2c.c b/sound/pci/hda/cs35l41_hda_i2c.c > index eeb387853ee3..c2397dc53e78 100644 > --- a/sound/pci/hda/cs35l41_hda_i2c.c > +++ b/sound/pci/hda/cs35l41_hda_i2c.c > @@ -58,7 +58,6 @@ static struct i2c_driver cs35l41_i2c_driver = { > .probe = cs35l41_hda_i2c_probe, > .remove = cs35l41_hda_i2c_remove, > }; > - > module_i2c_driver(cs35l41_i2c_driver); > > MODULE_DESCRIPTION("HDA CS35L41 driver"); > diff --git a/sound/pci/hda/cs35l41_hda_spi.c b/sound/pci/hda/cs35l41_hda_spi.c > index 15345a72b9d1..36815ab4e461 100644 > --- a/sound/pci/hda/cs35l41_hda_spi.c > +++ b/sound/pci/hda/cs35l41_hda_spi.c > @@ -55,7 +55,6 @@ static struct spi_driver cs35l41_spi_driver = { > .probe = cs35l41_hda_spi_probe, > .remove = cs35l41_hda_spi_remove, > }; > - > module_spi_driver(cs35l41_spi_driver); > > MODULE_DESCRIPTION("HDA CS35L41 driver"); ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/5] ALSA: hda: cs35l41: Tidyup code and add default cases 2022-01-13 17:07 ` Lucas Tanure @ 2022-01-14 16:15 ` Takashi Iwai -1 siblings, 0 replies; 28+ messages in thread From: Takashi Iwai @ 2022-01-14 16:15 UTC (permalink / raw) To: Lucas Tanure Cc: Rafael J . Wysocki, Len Brown, Hans de Goede, Mark Gross, Jaroslav Kysela, Takashi Iwai, alsa-devel, linux-acpi, patches, platform-driver-x86, linux-kernel On Thu, 13 Jan 2022 18:07:27 +0100, Lucas Tanure wrote: > > Clean up the code, plus adding default cases for switch > and dev_err_probe use. Please split to patches. There are too much mixed up of changes for different goals. There is difference between a white-space cleanup or such (that is, no significant change of the resultant binary), a cleanup with a better API usage, and a code refactoring. And, the addition of the missing default case is rather the fix, not a clean up... thanks, Takashi ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 4/5] ALSA: hda: cs35l41: Tidyup code and add default cases @ 2022-01-14 16:15 ` Takashi Iwai 0 siblings, 0 replies; 28+ messages in thread From: Takashi Iwai @ 2022-01-14 16:15 UTC (permalink / raw) To: Lucas Tanure Cc: alsa-devel, linux-acpi, Rafael J . Wysocki, patches, Takashi Iwai, Mark Gross, Hans de Goede, platform-driver-x86, linux-kernel, Len Brown On Thu, 13 Jan 2022 18:07:27 +0100, Lucas Tanure wrote: > > Clean up the code, plus adding default cases for switch > and dev_err_probe use. Please split to patches. There are too much mixed up of changes for different goals. There is difference between a white-space cleanup or such (that is, no significant change of the resultant binary), a cleanup with a better API usage, and a code refactoring. And, the addition of the missing default case is rather the fix, not a clean up... thanks, Takashi ^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 5/5] ACPI / scan: Create platform device for CLSA0100 2022-01-13 17:07 ` Lucas Tanure @ 2022-01-13 17:07 ` Lucas Tanure -1 siblings, 0 replies; 28+ messages in thread From: Lucas Tanure @ 2022-01-13 17:07 UTC (permalink / raw) To: Rafael J . Wysocki, Len Brown, Hans de Goede, Mark Gross, Jaroslav Kysela, Takashi Iwai Cc: alsa-devel, linux-acpi, patches, platform-driver-x86, linux-kernel, Lucas Tanure The ACPI device with CLSA0100 is a sound card with multiple instances of CS35L41 connected by I2C to the main CPU. We add an ID to the i2c_multi_instantiate_idsi list to enumerate all I2C slaves correctly. Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> --- drivers/acpi/scan.c | 2 ++ drivers/platform/x86/i2c-multi-instantiate.c | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index c215bc8723d0..2a68031d953e 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -1753,6 +1753,8 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device) */ {"BCM4752", }, {"LNV4752", }, + /* Non-conforming _HID for Cirrus Logic already released */ + {"CLSA0100", }, {} }; diff --git a/drivers/platform/x86/i2c-multi-instantiate.c b/drivers/platform/x86/i2c-multi-instantiate.c index 4956a1df5b90..a51a74933fa9 100644 --- a/drivers/platform/x86/i2c-multi-instantiate.c +++ b/drivers/platform/x86/i2c-multi-instantiate.c @@ -147,6 +147,12 @@ static const struct i2c_inst_data int3515_data[] = { {} }; +static const struct i2c_inst_data cs35l41_hda[] = { + { "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 }, + { "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 }, + {} +}; + /* * Note new device-ids must also be added to i2c_multi_instantiate_ids in * drivers/acpi/scan.c: acpi_device_enumeration_by_parent(). @@ -155,6 +161,8 @@ static const struct acpi_device_id i2c_multi_inst_acpi_ids[] = { { "BSG1160", (unsigned long)bsg1160_data }, { "BSG2150", (unsigned long)bsg2150_data }, { "INT3515", (unsigned long)int3515_data }, + /* Non-conforming _HID for Cirrus Logic already released */ + { "CLSA0100", (unsigned long)cs35l41_hda }, { } }; MODULE_DEVICE_TABLE(acpi, i2c_multi_inst_acpi_ids); -- 2.34.1 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 5/5] ACPI / scan: Create platform device for CLSA0100 @ 2022-01-13 17:07 ` Lucas Tanure 0 siblings, 0 replies; 28+ messages in thread From: Lucas Tanure @ 2022-01-13 17:07 UTC (permalink / raw) To: Rafael J . Wysocki, Len Brown, Hans de Goede, Mark Gross, Jaroslav Kysela, Takashi Iwai Cc: alsa-devel, Lucas Tanure, patches, linux-kernel, platform-driver-x86, linux-acpi The ACPI device with CLSA0100 is a sound card with multiple instances of CS35L41 connected by I2C to the main CPU. We add an ID to the i2c_multi_instantiate_idsi list to enumerate all I2C slaves correctly. Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> --- drivers/acpi/scan.c | 2 ++ drivers/platform/x86/i2c-multi-instantiate.c | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index c215bc8723d0..2a68031d953e 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -1753,6 +1753,8 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device) */ {"BCM4752", }, {"LNV4752", }, + /* Non-conforming _HID for Cirrus Logic already released */ + {"CLSA0100", }, {} }; diff --git a/drivers/platform/x86/i2c-multi-instantiate.c b/drivers/platform/x86/i2c-multi-instantiate.c index 4956a1df5b90..a51a74933fa9 100644 --- a/drivers/platform/x86/i2c-multi-instantiate.c +++ b/drivers/platform/x86/i2c-multi-instantiate.c @@ -147,6 +147,12 @@ static const struct i2c_inst_data int3515_data[] = { {} }; +static const struct i2c_inst_data cs35l41_hda[] = { + { "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 }, + { "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 }, + {} +}; + /* * Note new device-ids must also be added to i2c_multi_instantiate_ids in * drivers/acpi/scan.c: acpi_device_enumeration_by_parent(). @@ -155,6 +161,8 @@ static const struct acpi_device_id i2c_multi_inst_acpi_ids[] = { { "BSG1160", (unsigned long)bsg1160_data }, { "BSG2150", (unsigned long)bsg2150_data }, { "INT3515", (unsigned long)int3515_data }, + /* Non-conforming _HID for Cirrus Logic already released */ + { "CLSA0100", (unsigned long)cs35l41_hda }, { } }; MODULE_DEVICE_TABLE(acpi, i2c_multi_inst_acpi_ids); -- 2.34.1 ^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 5/5] ACPI / scan: Create platform device for CLSA0100 2022-01-13 17:07 ` Lucas Tanure @ 2022-01-14 16:19 ` Takashi Iwai -1 siblings, 0 replies; 28+ messages in thread From: Takashi Iwai @ 2022-01-14 16:19 UTC (permalink / raw) To: Lucas Tanure Cc: Rafael J . Wysocki, Len Brown, Hans de Goede, Mark Gross, Jaroslav Kysela, Takashi Iwai, alsa-devel, linux-acpi, patches, platform-driver-x86, linux-kernel On Thu, 13 Jan 2022 18:07:28 +0100, Lucas Tanure wrote: > > The ACPI device with CLSA0100 is a sound card with > multiple instances of CS35L41 connected by I2C to > the main CPU. > > We add an ID to the i2c_multi_instantiate_idsi list > to enumerate all I2C slaves correctly. > > Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> I think it's better to merge this from sound git tree together with others in the patch set, presumably for rc1. It'd be great if ACPI people can take a review and give an ack/nack. Thanks! Takashi > --- > drivers/acpi/scan.c | 2 ++ > drivers/platform/x86/i2c-multi-instantiate.c | 8 ++++++++ > 2 files changed, 10 insertions(+) > > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c > index c215bc8723d0..2a68031d953e 100644 > --- a/drivers/acpi/scan.c > +++ b/drivers/acpi/scan.c > @@ -1753,6 +1753,8 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device) > */ > {"BCM4752", }, > {"LNV4752", }, > + /* Non-conforming _HID for Cirrus Logic already released */ > + {"CLSA0100", }, > {} > }; > > diff --git a/drivers/platform/x86/i2c-multi-instantiate.c b/drivers/platform/x86/i2c-multi-instantiate.c > index 4956a1df5b90..a51a74933fa9 100644 > --- a/drivers/platform/x86/i2c-multi-instantiate.c > +++ b/drivers/platform/x86/i2c-multi-instantiate.c > @@ -147,6 +147,12 @@ static const struct i2c_inst_data int3515_data[] = { > {} > }; > > +static const struct i2c_inst_data cs35l41_hda[] = { > + { "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 }, > + { "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 }, > + {} > +}; > + > /* > * Note new device-ids must also be added to i2c_multi_instantiate_ids in > * drivers/acpi/scan.c: acpi_device_enumeration_by_parent(). > @@ -155,6 +161,8 @@ static const struct acpi_device_id i2c_multi_inst_acpi_ids[] = { > { "BSG1160", (unsigned long)bsg1160_data }, > { "BSG2150", (unsigned long)bsg2150_data }, > { "INT3515", (unsigned long)int3515_data }, > + /* Non-conforming _HID for Cirrus Logic already released */ > + { "CLSA0100", (unsigned long)cs35l41_hda }, > { } > }; > MODULE_DEVICE_TABLE(acpi, i2c_multi_inst_acpi_ids); > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 5/5] ACPI / scan: Create platform device for CLSA0100 @ 2022-01-14 16:19 ` Takashi Iwai 0 siblings, 0 replies; 28+ messages in thread From: Takashi Iwai @ 2022-01-14 16:19 UTC (permalink / raw) To: Lucas Tanure Cc: alsa-devel, linux-acpi, Rafael J . Wysocki, patches, Takashi Iwai, Mark Gross, Hans de Goede, platform-driver-x86, linux-kernel, Len Brown On Thu, 13 Jan 2022 18:07:28 +0100, Lucas Tanure wrote: > > The ACPI device with CLSA0100 is a sound card with > multiple instances of CS35L41 connected by I2C to > the main CPU. > > We add an ID to the i2c_multi_instantiate_idsi list > to enumerate all I2C slaves correctly. > > Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> I think it's better to merge this from sound git tree together with others in the patch set, presumably for rc1. It'd be great if ACPI people can take a review and give an ack/nack. Thanks! Takashi > --- > drivers/acpi/scan.c | 2 ++ > drivers/platform/x86/i2c-multi-instantiate.c | 8 ++++++++ > 2 files changed, 10 insertions(+) > > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c > index c215bc8723d0..2a68031d953e 100644 > --- a/drivers/acpi/scan.c > +++ b/drivers/acpi/scan.c > @@ -1753,6 +1753,8 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device) > */ > {"BCM4752", }, > {"LNV4752", }, > + /* Non-conforming _HID for Cirrus Logic already released */ > + {"CLSA0100", }, > {} > }; > > diff --git a/drivers/platform/x86/i2c-multi-instantiate.c b/drivers/platform/x86/i2c-multi-instantiate.c > index 4956a1df5b90..a51a74933fa9 100644 > --- a/drivers/platform/x86/i2c-multi-instantiate.c > +++ b/drivers/platform/x86/i2c-multi-instantiate.c > @@ -147,6 +147,12 @@ static const struct i2c_inst_data int3515_data[] = { > {} > }; > > +static const struct i2c_inst_data cs35l41_hda[] = { > + { "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 }, > + { "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 }, > + {} > +}; > + > /* > * Note new device-ids must also be added to i2c_multi_instantiate_ids in > * drivers/acpi/scan.c: acpi_device_enumeration_by_parent(). > @@ -155,6 +161,8 @@ static const struct acpi_device_id i2c_multi_inst_acpi_ids[] = { > { "BSG1160", (unsigned long)bsg1160_data }, > { "BSG2150", (unsigned long)bsg2150_data }, > { "INT3515", (unsigned long)int3515_data }, > + /* Non-conforming _HID for Cirrus Logic already released */ > + { "CLSA0100", (unsigned long)cs35l41_hda }, > { } > }; > MODULE_DEVICE_TABLE(acpi, i2c_multi_inst_acpi_ids); > -- > 2.34.1 > ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 5/5] ACPI / scan: Create platform device for CLSA0100 2022-01-14 16:19 ` Takashi Iwai @ 2022-01-14 17:51 ` Rafael J. Wysocki -1 siblings, 0 replies; 28+ messages in thread From: Rafael J. Wysocki @ 2022-01-14 17:51 UTC (permalink / raw) To: Takashi Iwai, Hans de Goede Cc: Lucas Tanure, Rafael J . Wysocki, Len Brown, Mark Gross, Jaroslav Kysela, Takashi Iwai, moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM..., ACPI Devel Maling List, patches, Platform Driver, Linux Kernel Mailing List On Fri, Jan 14, 2022 at 5:19 PM Takashi Iwai <tiwai@suse.de> wrote: > > On Thu, 13 Jan 2022 18:07:28 +0100, > Lucas Tanure wrote: > > > > The ACPI device with CLSA0100 is a sound card with > > multiple instances of CS35L41 connected by I2C to > > the main CPU. > > > > We add an ID to the i2c_multi_instantiate_idsi list > > to enumerate all I2C slaves correctly. > > > > Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> > > I think it's better to merge this from sound git tree together with > others in the patch set, presumably for rc1. > > It'd be great if ACPI people can take a review and give an ack/nack. Hans, what do you think? > > --- > > drivers/acpi/scan.c | 2 ++ > > drivers/platform/x86/i2c-multi-instantiate.c | 8 ++++++++ > > 2 files changed, 10 insertions(+) > > > > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c > > index c215bc8723d0..2a68031d953e 100644 > > --- a/drivers/acpi/scan.c > > +++ b/drivers/acpi/scan.c > > @@ -1753,6 +1753,8 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device) > > */ > > {"BCM4752", }, > > {"LNV4752", }, > > + /* Non-conforming _HID for Cirrus Logic already released */ > > + {"CLSA0100", }, > > {} > > }; > > > > diff --git a/drivers/platform/x86/i2c-multi-instantiate.c b/drivers/platform/x86/i2c-multi-instantiate.c > > index 4956a1df5b90..a51a74933fa9 100644 > > --- a/drivers/platform/x86/i2c-multi-instantiate.c > > +++ b/drivers/platform/x86/i2c-multi-instantiate.c > > @@ -147,6 +147,12 @@ static const struct i2c_inst_data int3515_data[] = { > > {} > > }; > > > > +static const struct i2c_inst_data cs35l41_hda[] = { > > + { "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 }, > > + { "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 }, > > + {} > > +}; > > + > > /* > > * Note new device-ids must also be added to i2c_multi_instantiate_ids in > > * drivers/acpi/scan.c: acpi_device_enumeration_by_parent(). > > @@ -155,6 +161,8 @@ static const struct acpi_device_id i2c_multi_inst_acpi_ids[] = { > > { "BSG1160", (unsigned long)bsg1160_data }, > > { "BSG2150", (unsigned long)bsg2150_data }, > > { "INT3515", (unsigned long)int3515_data }, > > + /* Non-conforming _HID for Cirrus Logic already released */ > > + { "CLSA0100", (unsigned long)cs35l41_hda }, > > { } > > }; > > MODULE_DEVICE_TABLE(acpi, i2c_multi_inst_acpi_ids); > > -- > > 2.34.1 > > ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 5/5] ACPI / scan: Create platform device for CLSA0100 @ 2022-01-14 17:51 ` Rafael J. Wysocki 0 siblings, 0 replies; 28+ messages in thread From: Rafael J. Wysocki @ 2022-01-14 17:51 UTC (permalink / raw) To: Takashi Iwai, Hans de Goede Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM..., Lucas Tanure, Rafael J . Wysocki, patches, Takashi Iwai, Mark Gross, ACPI Devel Maling List, Platform Driver, Linux Kernel Mailing List, Len Brown On Fri, Jan 14, 2022 at 5:19 PM Takashi Iwai <tiwai@suse.de> wrote: > > On Thu, 13 Jan 2022 18:07:28 +0100, > Lucas Tanure wrote: > > > > The ACPI device with CLSA0100 is a sound card with > > multiple instances of CS35L41 connected by I2C to > > the main CPU. > > > > We add an ID to the i2c_multi_instantiate_idsi list > > to enumerate all I2C slaves correctly. > > > > Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> > > I think it's better to merge this from sound git tree together with > others in the patch set, presumably for rc1. > > It'd be great if ACPI people can take a review and give an ack/nack. Hans, what do you think? > > --- > > drivers/acpi/scan.c | 2 ++ > > drivers/platform/x86/i2c-multi-instantiate.c | 8 ++++++++ > > 2 files changed, 10 insertions(+) > > > > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c > > index c215bc8723d0..2a68031d953e 100644 > > --- a/drivers/acpi/scan.c > > +++ b/drivers/acpi/scan.c > > @@ -1753,6 +1753,8 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device) > > */ > > {"BCM4752", }, > > {"LNV4752", }, > > + /* Non-conforming _HID for Cirrus Logic already released */ > > + {"CLSA0100", }, > > {} > > }; > > > > diff --git a/drivers/platform/x86/i2c-multi-instantiate.c b/drivers/platform/x86/i2c-multi-instantiate.c > > index 4956a1df5b90..a51a74933fa9 100644 > > --- a/drivers/platform/x86/i2c-multi-instantiate.c > > +++ b/drivers/platform/x86/i2c-multi-instantiate.c > > @@ -147,6 +147,12 @@ static const struct i2c_inst_data int3515_data[] = { > > {} > > }; > > > > +static const struct i2c_inst_data cs35l41_hda[] = { > > + { "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 }, > > + { "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 }, > > + {} > > +}; > > + > > /* > > * Note new device-ids must also be added to i2c_multi_instantiate_ids in > > * drivers/acpi/scan.c: acpi_device_enumeration_by_parent(). > > @@ -155,6 +161,8 @@ static const struct acpi_device_id i2c_multi_inst_acpi_ids[] = { > > { "BSG1160", (unsigned long)bsg1160_data }, > > { "BSG2150", (unsigned long)bsg2150_data }, > > { "INT3515", (unsigned long)int3515_data }, > > + /* Non-conforming _HID for Cirrus Logic already released */ > > + { "CLSA0100", (unsigned long)cs35l41_hda }, > > { } > > }; > > MODULE_DEVICE_TABLE(acpi, i2c_multi_inst_acpi_ids); > > -- > > 2.34.1 > > ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 5/5] ACPI / scan: Create platform device for CLSA0100 2022-01-14 17:51 ` Rafael J. Wysocki @ 2022-01-14 18:56 ` Hans de Goede -1 siblings, 0 replies; 28+ messages in thread From: Hans de Goede @ 2022-01-14 18:56 UTC (permalink / raw) To: Rafael J. Wysocki, Takashi Iwai Cc: Lucas Tanure, Len Brown, Mark Gross, Jaroslav Kysela, Takashi Iwai, moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM..., ACPI Devel Maling List, patches, Platform Driver, Linux Kernel Mailing List Hi, On 1/14/22 18:51, Rafael J. Wysocki wrote: > On Fri, Jan 14, 2022 at 5:19 PM Takashi Iwai <tiwai@suse.de> wrote: >> >> On Thu, 13 Jan 2022 18:07:28 +0100, >> Lucas Tanure wrote: >>> >>> The ACPI device with CLSA0100 is a sound card with >>> multiple instances of CS35L41 connected by I2C to >>> the main CPU. >>> >>> We add an ID to the i2c_multi_instantiate_idsi list >>> to enumerate all I2C slaves correctly. >>> >>> Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> >> >> I think it's better to merge this from sound git tree together with >> others in the patch set, presumably for rc1. >> >> It'd be great if ACPI people can take a review and give an ack/nack. > > Hans, what do you think? This patch (5/5) applies on top of: https://lore.kernel.org/linux-acpi/20211210154050.3713-1-sbinding@opensource.cirrus.com/ Which still needs some work and which really should be merged through the ACPI tree. IMHO it would be best to simply drop this (5/5) from this series and move it to the v3 of the series which I've linked to above. 1-4 can be merged through the alsa tree independently of 5/5 AFAIK. Regards, Hans > >>> --- >>> drivers/acpi/scan.c | 2 ++ >>> drivers/platform/x86/i2c-multi-instantiate.c | 8 ++++++++ >>> 2 files changed, 10 insertions(+) >>> >>> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c >>> index c215bc8723d0..2a68031d953e 100644 >>> --- a/drivers/acpi/scan.c >>> +++ b/drivers/acpi/scan.c >>> @@ -1753,6 +1753,8 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device) >>> */ >>> {"BCM4752", }, >>> {"LNV4752", }, >>> + /* Non-conforming _HID for Cirrus Logic already released */ >>> + {"CLSA0100", }, >>> {} >>> }; >>> >>> diff --git a/drivers/platform/x86/i2c-multi-instantiate.c b/drivers/platform/x86/i2c-multi-instantiate.c >>> index 4956a1df5b90..a51a74933fa9 100644 >>> --- a/drivers/platform/x86/i2c-multi-instantiate.c >>> +++ b/drivers/platform/x86/i2c-multi-instantiate.c >>> @@ -147,6 +147,12 @@ static const struct i2c_inst_data int3515_data[] = { >>> {} >>> }; >>> >>> +static const struct i2c_inst_data cs35l41_hda[] = { >>> + { "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 }, >>> + { "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 }, >>> + {} >>> +}; >>> + >>> /* >>> * Note new device-ids must also be added to i2c_multi_instantiate_ids in >>> * drivers/acpi/scan.c: acpi_device_enumeration_by_parent(). >>> @@ -155,6 +161,8 @@ static const struct acpi_device_id i2c_multi_inst_acpi_ids[] = { >>> { "BSG1160", (unsigned long)bsg1160_data }, >>> { "BSG2150", (unsigned long)bsg2150_data }, >>> { "INT3515", (unsigned long)int3515_data }, >>> + /* Non-conforming _HID for Cirrus Logic already released */ >>> + { "CLSA0100", (unsigned long)cs35l41_hda }, >>> { } >>> }; >>> MODULE_DEVICE_TABLE(acpi, i2c_multi_inst_acpi_ids); >>> -- >>> 2.34.1 >>> > ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 5/5] ACPI / scan: Create platform device for CLSA0100 @ 2022-01-14 18:56 ` Hans de Goede 0 siblings, 0 replies; 28+ messages in thread From: Hans de Goede @ 2022-01-14 18:56 UTC (permalink / raw) To: Rafael J. Wysocki, Takashi Iwai Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM..., Lucas Tanure, patches, Takashi Iwai, Mark Gross, ACPI Devel Maling List, Platform Driver, Linux Kernel Mailing List, Len Brown Hi, On 1/14/22 18:51, Rafael J. Wysocki wrote: > On Fri, Jan 14, 2022 at 5:19 PM Takashi Iwai <tiwai@suse.de> wrote: >> >> On Thu, 13 Jan 2022 18:07:28 +0100, >> Lucas Tanure wrote: >>> >>> The ACPI device with CLSA0100 is a sound card with >>> multiple instances of CS35L41 connected by I2C to >>> the main CPU. >>> >>> We add an ID to the i2c_multi_instantiate_idsi list >>> to enumerate all I2C slaves correctly. >>> >>> Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> >> >> I think it's better to merge this from sound git tree together with >> others in the patch set, presumably for rc1. >> >> It'd be great if ACPI people can take a review and give an ack/nack. > > Hans, what do you think? This patch (5/5) applies on top of: https://lore.kernel.org/linux-acpi/20211210154050.3713-1-sbinding@opensource.cirrus.com/ Which still needs some work and which really should be merged through the ACPI tree. IMHO it would be best to simply drop this (5/5) from this series and move it to the v3 of the series which I've linked to above. 1-4 can be merged through the alsa tree independently of 5/5 AFAIK. Regards, Hans > >>> --- >>> drivers/acpi/scan.c | 2 ++ >>> drivers/platform/x86/i2c-multi-instantiate.c | 8 ++++++++ >>> 2 files changed, 10 insertions(+) >>> >>> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c >>> index c215bc8723d0..2a68031d953e 100644 >>> --- a/drivers/acpi/scan.c >>> +++ b/drivers/acpi/scan.c >>> @@ -1753,6 +1753,8 @@ static bool acpi_device_enumeration_by_parent(struct acpi_device *device) >>> */ >>> {"BCM4752", }, >>> {"LNV4752", }, >>> + /* Non-conforming _HID for Cirrus Logic already released */ >>> + {"CLSA0100", }, >>> {} >>> }; >>> >>> diff --git a/drivers/platform/x86/i2c-multi-instantiate.c b/drivers/platform/x86/i2c-multi-instantiate.c >>> index 4956a1df5b90..a51a74933fa9 100644 >>> --- a/drivers/platform/x86/i2c-multi-instantiate.c >>> +++ b/drivers/platform/x86/i2c-multi-instantiate.c >>> @@ -147,6 +147,12 @@ static const struct i2c_inst_data int3515_data[] = { >>> {} >>> }; >>> >>> +static const struct i2c_inst_data cs35l41_hda[] = { >>> + { "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 }, >>> + { "cs35l41-hda", IRQ_RESOURCE_GPIO, 0 }, >>> + {} >>> +}; >>> + >>> /* >>> * Note new device-ids must also be added to i2c_multi_instantiate_ids in >>> * drivers/acpi/scan.c: acpi_device_enumeration_by_parent(). >>> @@ -155,6 +161,8 @@ static const struct acpi_device_id i2c_multi_inst_acpi_ids[] = { >>> { "BSG1160", (unsigned long)bsg1160_data }, >>> { "BSG2150", (unsigned long)bsg2150_data }, >>> { "INT3515", (unsigned long)int3515_data }, >>> + /* Non-conforming _HID for Cirrus Logic already released */ >>> + { "CLSA0100", (unsigned long)cs35l41_hda }, >>> { } >>> }; >>> MODULE_DEVICE_TABLE(acpi, i2c_multi_inst_acpi_ids); >>> -- >>> 2.34.1 >>> > ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 5/5] ACPI / scan: Create platform device for CLSA0100 2022-01-14 18:56 ` Hans de Goede @ 2022-01-15 6:59 ` Takashi Iwai -1 siblings, 0 replies; 28+ messages in thread From: Takashi Iwai @ 2022-01-15 6:59 UTC (permalink / raw) To: Hans de Goede Cc: Rafael J. Wysocki, Lucas Tanure, Len Brown, Mark Gross, Jaroslav Kysela, Takashi Iwai, moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM..., ACPI Devel Maling List, patches, Platform Driver, Linux Kernel Mailing List On Fri, 14 Jan 2022 19:56:04 +0100, Hans de Goede wrote: > > Hi, > > On 1/14/22 18:51, Rafael J. Wysocki wrote: > > On Fri, Jan 14, 2022 at 5:19 PM Takashi Iwai <tiwai@suse.de> wrote: > >> > >> On Thu, 13 Jan 2022 18:07:28 +0100, > >> Lucas Tanure wrote: > >>> > >>> The ACPI device with CLSA0100 is a sound card with > >>> multiple instances of CS35L41 connected by I2C to > >>> the main CPU. > >>> > >>> We add an ID to the i2c_multi_instantiate_idsi list > >>> to enumerate all I2C slaves correctly. > >>> > >>> Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> > >> > >> I think it's better to merge this from sound git tree together with > >> others in the patch set, presumably for rc1. > >> > >> It'd be great if ACPI people can take a review and give an ack/nack. > > > > Hans, what do you think? > > This patch (5/5) applies on top of: > > https://lore.kernel.org/linux-acpi/20211210154050.3713-1-sbinding@opensource.cirrus.com/ > > Which still needs some work and which really should be merged > through the ACPI tree. IMHO it would be best to simply drop > this (5/5) from this series and move it to the v3 of the > series which I've linked to above. > > 1-4 can be merged through the alsa tree independently of 5/5 AFAIK. OK, that's fine. Lucas, could you submit v3 patches in the suggested way? thanks, Takashi ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 5/5] ACPI / scan: Create platform device for CLSA0100 @ 2022-01-15 6:59 ` Takashi Iwai 0 siblings, 0 replies; 28+ messages in thread From: Takashi Iwai @ 2022-01-15 6:59 UTC (permalink / raw) To: Hans de Goede Cc: moderated list:SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM..., Lucas Tanure, Rafael J. Wysocki, patches, Takashi Iwai, Mark Gross, ACPI Devel Maling List, Platform Driver, Linux Kernel Mailing List, Len Brown On Fri, 14 Jan 2022 19:56:04 +0100, Hans de Goede wrote: > > Hi, > > On 1/14/22 18:51, Rafael J. Wysocki wrote: > > On Fri, Jan 14, 2022 at 5:19 PM Takashi Iwai <tiwai@suse.de> wrote: > >> > >> On Thu, 13 Jan 2022 18:07:28 +0100, > >> Lucas Tanure wrote: > >>> > >>> The ACPI device with CLSA0100 is a sound card with > >>> multiple instances of CS35L41 connected by I2C to > >>> the main CPU. > >>> > >>> We add an ID to the i2c_multi_instantiate_idsi list > >>> to enumerate all I2C slaves correctly. > >>> > >>> Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> > >> > >> I think it's better to merge this from sound git tree together with > >> others in the patch set, presumably for rc1. > >> > >> It'd be great if ACPI people can take a review and give an ack/nack. > > > > Hans, what do you think? > > This patch (5/5) applies on top of: > > https://lore.kernel.org/linux-acpi/20211210154050.3713-1-sbinding@opensource.cirrus.com/ > > Which still needs some work and which really should be merged > through the ACPI tree. IMHO it would be best to simply drop > this (5/5) from this series and move it to the v3 of the > series which I've linked to above. > > 1-4 can be merged through the alsa tree independently of 5/5 AFAIK. OK, that's fine. Lucas, could you submit v3 patches in the suggested way? thanks, Takashi ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 5/5] ACPI / scan: Create platform device for CLSA0100 2022-01-15 6:59 ` Takashi Iwai @ 2022-01-17 10:47 ` tanureal -1 siblings, 0 replies; 28+ messages in thread From: tanureal @ 2022-01-17 10:47 UTC (permalink / raw) To: Takashi Iwai, Hans de Goede, Rafael J. Wysocki, Len Brown, Mark Gross, Jaroslav Kysela, Takashi Iwai, SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM..., ACPI Devel Maling List, patches, Platform Driver, Linux Kernel Mailing List, On 1/15/22 6:59 AM, Takashi Iwai <tiwai@suse.de> wrote: > On Fri, 14 Jan 2022 19:56:04 +0100, > Hans de Goede wrote: > > > > Hi, > > > > On 1/14/22 18:51, Rafael J. Wysocki wrote: > >> On Fri, Jan 14, 2022 at 5:19 PM Takashi Iwai <tiwai@suse.de> wrote: > >>> > >>> On Thu, 13 Jan 2022 18:07:28 +0100, > >>> Lucas Tanure wrote: > >>>> > >>>> The ACPI device with CLSA0100 is a sound card with > >>>> multiple instances of CS35L41 connected by I2C to > >>>> the main CPU. > >>>> > >>>> We add an ID to the i2c_multi_instantiate_idsi list > >>>> to enumerate all I2C slaves correctly. > >>>> > >>>> Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> > >>> > >>> I think it's better to merge this from sound git tree together with > >>> others in the patch set, presumably for rc1. > >>> > >>> It'd be great if ACPI people can take a review and give an ack/nack. > >> > >> Hans, what do you think? > > > > This patch (5/5) applies on top of: > > > > https://lore.kernel.org/linux-acpi/20211210154050.3713-1-sbinding@opensource.cirrus.com/ > > > > Which still needs some work and which really should be merged > > through the ACPI tree. IMHO it would be best to simply drop > > this (5/5) from this series and move it to the v3 of the > > series which I've linked to above. > > > > 1-4 can be merged through the alsa tree independently of 5/5 AFAIK. > > OK, that's fine. > > Lucas, could you submit v3 patches in the suggested way? Yes, we will do that. Thanks > > > thanks, > > Takashi > ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 5/5] ACPI / scan: Create platform device for CLSA0100 @ 2022-01-17 10:47 ` tanureal 0 siblings, 0 replies; 28+ messages in thread From: tanureal @ 2022-01-17 10:47 UTC (permalink / raw) To: Takashi Iwai, Hans de Goede, Rafael J. Wysocki, Len Brown, Mark Gross, Jaroslav Kysela, Takashi Iwai, SOUND - SOC LAYER / DYNAMIC AUDIO POWER MANAGEM..., ACPI Devel Maling List, patches, Platform Driver, Linux Kernel Mailing List On 1/15/22 6:59 AM, Takashi Iwai <tiwai@suse.de> wrote: > On Fri, 14 Jan 2022 19:56:04 +0100, > Hans de Goede wrote: > > > > Hi, > > > > On 1/14/22 18:51, Rafael J. Wysocki wrote: > >> On Fri, Jan 14, 2022 at 5:19 PM Takashi Iwai <tiwai@suse.de> wrote: > >>> > >>> On Thu, 13 Jan 2022 18:07:28 +0100, > >>> Lucas Tanure wrote: > >>>> > >>>> The ACPI device with CLSA0100 is a sound card with > >>>> multiple instances of CS35L41 connected by I2C to > >>>> the main CPU. > >>>> > >>>> We add an ID to the i2c_multi_instantiate_idsi list > >>>> to enumerate all I2C slaves correctly. > >>>> > >>>> Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> > >>> > >>> I think it's better to merge this from sound git tree together with > >>> others in the patch set, presumably for rc1. > >>> > >>> It'd be great if ACPI people can take a review and give an ack/nack. > >> > >> Hans, what do you think? > > > > This patch (5/5) applies on top of: > > > > https://lore.kernel.org/linux-acpi/20211210154050.3713-1-sbinding@opensource.cirrus.com/ > > > > Which still needs some work and which really should be merged > > through the ACPI tree. IMHO it would be best to simply drop > > this (5/5) from this series and move it to the v3 of the > > series which I've linked to above. > > > > 1-4 can be merged through the alsa tree independently of 5/5 AFAIK. > > OK, that's fine. > > Lucas, could you submit v3 patches in the suggested way? Yes, we will do that. Thanks > > > thanks, > > Takashi > ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/5] ALSA: hda: cs35l41: Avoid overwriting register patch 2022-01-13 17:07 ` Lucas Tanure @ 2022-01-14 16:06 ` Cezary Rojewski -1 siblings, 0 replies; 28+ messages in thread From: Cezary Rojewski @ 2022-01-14 16:06 UTC (permalink / raw) To: Lucas Tanure, Rafael J . Wysocki, Len Brown, Hans de Goede, Mark Gross, Jaroslav Kysela, Takashi Iwai Cc: alsa-devel, linux-acpi, patches, platform-driver-x86, linux-kernel, Charles Keepax On 2022-01-13 6:07 PM, Lucas Tanure wrote: > From: Charles Keepax <ckeepax@opensource.cirrus.com> > > regmap_register_patch can't be used to apply the probe sequence as a > patch is already registers with the regmap by > cs35l41_register_errata_patch and only a single patch can be attached to > a single regmap. The driver doesn't currently rely on a cache sync to > re-apply this probe sequence so simply switch it to a multi write. > > Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com> > Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> Please correct me if I'm wrong, but this change looks like a fix for the previously added commit: ALSA: hda: cs35l41: Add support for CS35L41 in HDA systems and so it would be good to append appropriate 'Fixes' tag here. > --- > sound/pci/hda/cs35l41_hda.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c > index 30b40d865863..c47c5f0b4e59 100644 > --- a/sound/pci/hda/cs35l41_hda.c > +++ b/sound/pci/hda/cs35l41_hda.c > @@ -480,7 +480,7 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i > acpi_hw_cfg = NULL; > > if (cs35l41->reg_seq->probe) { > - ret = regmap_register_patch(cs35l41->regmap, cs35l41->reg_seq->probe, > + ret = regmap_multi_reg_write(cs35l41->regmap, cs35l41->reg_seq->probe, > cs35l41->reg_seq->num_probe); > if (ret) { > dev_err(cs35l41->dev, "Fail to apply probe reg patch: %d\n", ret); > ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/5] ALSA: hda: cs35l41: Avoid overwriting register patch @ 2022-01-14 16:06 ` Cezary Rojewski 0 siblings, 0 replies; 28+ messages in thread From: Cezary Rojewski @ 2022-01-14 16:06 UTC (permalink / raw) To: Lucas Tanure, Rafael J . Wysocki, Len Brown, Hans de Goede, Mark Gross, Jaroslav Kysela, Takashi Iwai Cc: alsa-devel, Charles Keepax, patches, linux-kernel, platform-driver-x86, linux-acpi On 2022-01-13 6:07 PM, Lucas Tanure wrote: > From: Charles Keepax <ckeepax@opensource.cirrus.com> > > regmap_register_patch can't be used to apply the probe sequence as a > patch is already registers with the regmap by > cs35l41_register_errata_patch and only a single patch can be attached to > a single regmap. The driver doesn't currently rely on a cache sync to > re-apply this probe sequence so simply switch it to a multi write. > > Signed-off-by: Charles Keepax <ckeepax@opensource.cirrus.com> > Signed-off-by: Lucas Tanure <tanureal@opensource.cirrus.com> Please correct me if I'm wrong, but this change looks like a fix for the previously added commit: ALSA: hda: cs35l41: Add support for CS35L41 in HDA systems and so it would be good to append appropriate 'Fixes' tag here. > --- > sound/pci/hda/cs35l41_hda.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/sound/pci/hda/cs35l41_hda.c b/sound/pci/hda/cs35l41_hda.c > index 30b40d865863..c47c5f0b4e59 100644 > --- a/sound/pci/hda/cs35l41_hda.c > +++ b/sound/pci/hda/cs35l41_hda.c > @@ -480,7 +480,7 @@ int cs35l41_hda_probe(struct device *dev, const char *device_name, int id, int i > acpi_hw_cfg = NULL; > > if (cs35l41->reg_seq->probe) { > - ret = regmap_register_patch(cs35l41->regmap, cs35l41->reg_seq->probe, > + ret = regmap_multi_reg_write(cs35l41->regmap, cs35l41->reg_seq->probe, > cs35l41->reg_seq->num_probe); > if (ret) { > dev_err(cs35l41->dev, "Fail to apply probe reg patch: %d\n", ret); > ^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2022-01-17 10:48 UTC | newest] Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-01-13 17:07 [PATCH 1/5] ALSA: hda: cs35l41: Avoid overwriting register patch Lucas Tanure 2022-01-13 17:07 ` Lucas Tanure 2022-01-13 17:07 ` [PATCH 2/5] ALSA: hda: cs35l41: Add calls to newly added test key function Lucas Tanure 2022-01-13 17:07 ` Lucas Tanure 2022-01-14 16:14 ` Cezary Rojewski 2022-01-14 16:14 ` Cezary Rojewski 2022-01-13 17:07 ` [PATCH 3/5] ALSA: hda: cs35l41: Move cs35l41* calls to its own symbol namespace Lucas Tanure 2022-01-13 17:07 ` Lucas Tanure 2022-01-13 17:07 ` [PATCH 4/5] ALSA: hda: cs35l41: Tidyup code and add default cases Lucas Tanure 2022-01-13 17:07 ` Lucas Tanure 2022-01-14 13:04 ` Lucas tanure 2022-01-14 13:04 ` Lucas tanure 2022-01-14 16:15 ` Takashi Iwai 2022-01-14 16:15 ` Takashi Iwai 2022-01-13 17:07 ` [PATCH 5/5] ACPI / scan: Create platform device for CLSA0100 Lucas Tanure 2022-01-13 17:07 ` Lucas Tanure 2022-01-14 16:19 ` Takashi Iwai 2022-01-14 16:19 ` Takashi Iwai 2022-01-14 17:51 ` Rafael J. Wysocki 2022-01-14 17:51 ` Rafael J. Wysocki 2022-01-14 18:56 ` Hans de Goede 2022-01-14 18:56 ` Hans de Goede 2022-01-15 6:59 ` Takashi Iwai 2022-01-15 6:59 ` Takashi Iwai 2022-01-17 10:47 ` tanureal 2022-01-17 10:47 ` tanureal 2022-01-14 16:06 ` [PATCH 1/5] ALSA: hda: cs35l41: Avoid overwriting register patch Cezary Rojewski 2022-01-14 16:06 ` Cezary Rojewski
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.