* [PATCH RESEND 0/2] Add reset-gpios handling for max98927
@ 2021-08-29 17:00 ` Alejandro Tafalla
0 siblings, 0 replies; 14+ messages in thread
From: Alejandro Tafalla @ 2021-08-29 17:00 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, alsa-devel
Cc: Alejandro Tafalla, Rob Herring, Jaroslav Kysela, Takashi Iwai,
devicetree, linux-kernel
The max98927 codec on some devices (i.e. Xiaomi Mi A2 Lite phone) require
hardware-resetting the codec by driving a reset-gpio. This series add
support for it through an optional reset-gpios property.
Alejandro Tafalla (2):
ASoC: max98927: Handle reset gpio when probing i2c
dt-bindings: sound: max98927: Add reset-gpios optional property
.../devicetree/bindings/sound/max9892x.txt | 3 +++
sound/soc/codecs/max98927.c | 16 ++++++++++++++++
sound/soc/codecs/max98927.h | 1 +
3 files changed, 20 insertions(+)
--
2.32.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH RESEND 0/2] Add reset-gpios handling for max98927
@ 2021-08-29 17:00 ` Alejandro Tafalla
0 siblings, 0 replies; 14+ messages in thread
From: Alejandro Tafalla @ 2021-08-29 17:00 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, alsa-devel
Cc: devicetree, linux-kernel, Takashi Iwai, Rob Herring, Alejandro Tafalla
The max98927 codec on some devices (i.e. Xiaomi Mi A2 Lite phone) require
hardware-resetting the codec by driving a reset-gpio. This series add
support for it through an optional reset-gpios property.
Alejandro Tafalla (2):
ASoC: max98927: Handle reset gpio when probing i2c
dt-bindings: sound: max98927: Add reset-gpios optional property
.../devicetree/bindings/sound/max9892x.txt | 3 +++
sound/soc/codecs/max98927.c | 16 ++++++++++++++++
sound/soc/codecs/max98927.h | 1 +
3 files changed, 20 insertions(+)
--
2.32.0
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH RESEND 1/2] ASoC: max98927: Handle reset gpio when probing i2c
2021-08-29 17:00 ` Alejandro Tafalla
@ 2021-08-29 17:00 ` Alejandro Tafalla
-1 siblings, 0 replies; 14+ messages in thread
From: Alejandro Tafalla @ 2021-08-29 17:00 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, alsa-devel
Cc: Alejandro Tafalla, Rob Herring, Jaroslav Kysela, Takashi Iwai,
devicetree, linux-kernel
Drive the reset gpio if defined in the DTS node.
Signed-off-by: Alejandro Tafalla <atafalla@dnyon.com>
---
sound/soc/codecs/max98927.c | 16 ++++++++++++++++
sound/soc/codecs/max98927.h | 1 +
2 files changed, 17 insertions(+)
diff --git a/sound/soc/codecs/max98927.c b/sound/soc/codecs/max98927.c
index 8b206ee77709..dacf64c4cdf7 100644
--- a/sound/soc/codecs/max98927.c
+++ b/sound/soc/codecs/max98927.c
@@ -898,6 +898,22 @@ static int max98927_i2c_probe(struct i2c_client *i2c,
return ret;
}
+ max98927->reset_gpio
+ = devm_gpiod_get_optional(&i2c->dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(max98927->reset_gpio)) {
+ ret = PTR_ERR(max98927->reset_gpio);
+ dev_err(&i2c->dev,
+ "Failed to request GPIO reset pin, error %d\n", ret);
+ return ret;
+ }
+
+ if (max98927->reset_gpio) {
+ gpiod_set_value_cansleep(max98927->reset_gpio, 0);
+ usleep_range(5, 10)
+ gpiod_set_value_cansleep(max98927->reset_gpio, 1);
+ usleep_range(1, 5)
+ }
+
/* Check Revision ID */
ret = regmap_read(max98927->regmap,
MAX98927_R01FF_REV_ID, ®);
diff --git a/sound/soc/codecs/max98927.h b/sound/soc/codecs/max98927.h
index 05f495db914d..5c04bf38e24a 100644
--- a/sound/soc/codecs/max98927.h
+++ b/sound/soc/codecs/max98927.h
@@ -255,6 +255,7 @@ struct max98927_priv {
struct regmap *regmap;
struct snd_soc_component *component;
struct max98927_pdata *pdata;
+ struct gpio_desc *reset_gpio;
unsigned int spk_gain;
unsigned int sysclk;
unsigned int v_l_slot;
--
2.32.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH RESEND 1/2] ASoC: max98927: Handle reset gpio when probing i2c
@ 2021-08-29 17:00 ` Alejandro Tafalla
0 siblings, 0 replies; 14+ messages in thread
From: Alejandro Tafalla @ 2021-08-29 17:00 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, alsa-devel
Cc: devicetree, linux-kernel, Takashi Iwai, Rob Herring, Alejandro Tafalla
Drive the reset gpio if defined in the DTS node.
Signed-off-by: Alejandro Tafalla <atafalla@dnyon.com>
---
sound/soc/codecs/max98927.c | 16 ++++++++++++++++
sound/soc/codecs/max98927.h | 1 +
2 files changed, 17 insertions(+)
diff --git a/sound/soc/codecs/max98927.c b/sound/soc/codecs/max98927.c
index 8b206ee77709..dacf64c4cdf7 100644
--- a/sound/soc/codecs/max98927.c
+++ b/sound/soc/codecs/max98927.c
@@ -898,6 +898,22 @@ static int max98927_i2c_probe(struct i2c_client *i2c,
return ret;
}
+ max98927->reset_gpio
+ = devm_gpiod_get_optional(&i2c->dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(max98927->reset_gpio)) {
+ ret = PTR_ERR(max98927->reset_gpio);
+ dev_err(&i2c->dev,
+ "Failed to request GPIO reset pin, error %d\n", ret);
+ return ret;
+ }
+
+ if (max98927->reset_gpio) {
+ gpiod_set_value_cansleep(max98927->reset_gpio, 0);
+ usleep_range(5, 10)
+ gpiod_set_value_cansleep(max98927->reset_gpio, 1);
+ usleep_range(1, 5)
+ }
+
/* Check Revision ID */
ret = regmap_read(max98927->regmap,
MAX98927_R01FF_REV_ID, ®);
diff --git a/sound/soc/codecs/max98927.h b/sound/soc/codecs/max98927.h
index 05f495db914d..5c04bf38e24a 100644
--- a/sound/soc/codecs/max98927.h
+++ b/sound/soc/codecs/max98927.h
@@ -255,6 +255,7 @@ struct max98927_priv {
struct regmap *regmap;
struct snd_soc_component *component;
struct max98927_pdata *pdata;
+ struct gpio_desc *reset_gpio;
unsigned int spk_gain;
unsigned int sysclk;
unsigned int v_l_slot;
--
2.32.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH RESEND 2/2] dt-bindings: sound: max98927: Add reset-gpios optional property
2021-08-29 17:00 ` Alejandro Tafalla
@ 2021-08-29 17:00 ` Alejandro Tafalla
-1 siblings, 0 replies; 14+ messages in thread
From: Alejandro Tafalla @ 2021-08-29 17:00 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, alsa-devel
Cc: Alejandro Tafalla, Rob Herring, Jaroslav Kysela, Takashi Iwai,
devicetree, linux-kernel
Add the reset-gpios as an optional property because some devices might
not need it to work properly.
Signed-off-by: Alejandro Tafalla <atafalla@dnyon.com>
---
Documentation/devicetree/bindings/sound/max9892x.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/max9892x.txt b/Documentation/devicetree/bindings/sound/max9892x.txt
index f6171591ddc6..73733fb60136 100644
--- a/Documentation/devicetree/bindings/sound/max9892x.txt
+++ b/Documentation/devicetree/bindings/sound/max9892x.txt
@@ -30,6 +30,9 @@ Required properties:
- reg : the I2C address of the device for I2C
+Optional Properties:
+ - reset-gpios : reference to the GPIO connected to the reset pin, if any.
+
Example:
codec: max98927@3a {
--
2.32.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH RESEND 2/2] dt-bindings: sound: max98927: Add reset-gpios optional property
@ 2021-08-29 17:00 ` Alejandro Tafalla
0 siblings, 0 replies; 14+ messages in thread
From: Alejandro Tafalla @ 2021-08-29 17:00 UTC (permalink / raw)
To: Liam Girdwood, Mark Brown, alsa-devel
Cc: devicetree, linux-kernel, Takashi Iwai, Rob Herring, Alejandro Tafalla
Add the reset-gpios as an optional property because some devices might
not need it to work properly.
Signed-off-by: Alejandro Tafalla <atafalla@dnyon.com>
---
Documentation/devicetree/bindings/sound/max9892x.txt | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/max9892x.txt b/Documentation/devicetree/bindings/sound/max9892x.txt
index f6171591ddc6..73733fb60136 100644
--- a/Documentation/devicetree/bindings/sound/max9892x.txt
+++ b/Documentation/devicetree/bindings/sound/max9892x.txt
@@ -30,6 +30,9 @@ Required properties:
- reg : the I2C address of the device for I2C
+Optional Properties:
+ - reset-gpios : reference to the GPIO connected to the reset pin, if any.
+
Example:
codec: max98927@3a {
--
2.32.0
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH RESEND 1/2] ASoC: max98927: Handle reset gpio when probing i2c
2021-08-29 17:00 ` Alejandro Tafalla
(?)
@ 2021-08-29 20:22 ` Andy Shevchenko
2021-08-29 22:13 ` Alejandro Tafalla
-1 siblings, 1 reply; 14+ messages in thread
From: Andy Shevchenko @ 2021-08-29 20:22 UTC (permalink / raw)
To: Alejandro Tafalla
Cc: devicetree, alsa-devel, linux-kernel, Takashi Iwai, Rob Herring,
Liam Girdwood, Mark Brown
On Sunday, August 29, 2021, Alejandro Tafalla <atafalla@dnyon.com> wrote:
> Drive the reset gpio if defined in the DTS node.
>
> Signed-off-by: Alejandro Tafalla <atafalla@dnyon.com>
> ---
> sound/soc/codecs/max98927.c | 16 ++++++++++++++++
> sound/soc/codecs/max98927.h | 1 +
> 2 files changed, 17 insertions(+)
>
> diff --git a/sound/soc/codecs/max98927.c b/sound/soc/codecs/max98927.c
> index 8b206ee77709..dacf64c4cdf7 100644
> --- a/sound/soc/codecs/max98927.c
> +++ b/sound/soc/codecs/max98927.c
> @@ -898,6 +898,22 @@ static int max98927_i2c_probe(struct i2c_client *i2c,
> return ret;
> }
>
> + max98927->reset_gpio
> + = devm_gpiod_get_optional(&i2c->dev, "reset",
> GPIOD_OUT_HIGH);
> + if (IS_ERR(max98927->reset_gpio)) {
> + ret = PTR_ERR(max98927->reset_gpio);
> + dev_err(&i2c->dev,
> + "Failed to request GPIO reset pin, error %d\n",
> ret);
> + return ret;
Spamming logs is not good. Use
return dev_err_probe(...);
> + }
> +
> + if (max98927->reset_gpio) {
> + gpiod_set_value_cansleep(max98927->reset_gpio, 0);
You may request the pin in a proper state, also with current code you seems
mishandle the conception of the logical pin level vs. physical one.
> + usleep_range(5, 10)
> + gpiod_set_value_cansleep(max98927->reset_gpio, 1);
> + usleep_range(1, 5)
> + }
> +
> /* Check Revision ID */
> ret = regmap_read(max98927->regmap,
> MAX98927_R01FF_REV_ID, ®);
> diff --git a/sound/soc/codecs/max98927.h b/sound/soc/codecs/max98927.h
> index 05f495db914d..5c04bf38e24a 100644
> --- a/sound/soc/codecs/max98927.h
> +++ b/sound/soc/codecs/max98927.h
> @@ -255,6 +255,7 @@ struct max98927_priv {
> struct regmap *regmap;
> struct snd_soc_component *component;
> struct max98927_pdata *pdata;
> + struct gpio_desc *reset_gpio;
Why? Are you using it outside of ->probe()?
> unsigned int spk_gain;
> unsigned int sysclk;
> unsigned int v_l_slot;
> --
> 2.32.0
>
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RESEND 1/2] ASoC: max98927: Handle reset gpio when probing i2c
2021-08-29 17:00 ` Alejandro Tafalla
(?)
@ 2021-08-29 21:24 ` kernel test robot
-1 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2021-08-29 21:24 UTC (permalink / raw)
To: Alejandro Tafalla, Liam Girdwood, Mark Brown, alsa-devel
Cc: llvm, kbuild-all, Alejandro Tafalla, Rob Herring,
Jaroslav Kysela, Takashi Iwai, devicetree, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 4437 bytes --]
Hi Alejandro,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on asoc/for-next]
[also build test ERROR on v5.14-rc7 next-20210827]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Alejandro-Tafalla/Add-reset-gpios-handling-for-max98927/20210830-010941
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: hexagon-randconfig-r013-20210829 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 510e106fa8635e7f9c51c896180b971de6309b2f)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/4cc91cc4ff05ef061e2247a49c8b7cf9084fe6fb
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Alejandro-Tafalla/Add-reset-gpios-handling-for-max98927/20210830-010941
git checkout 4cc91cc4ff05ef061e2247a49c8b7cf9084fe6fb
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross O=build_dir ARCH=hexagon SHELL=/bin/bash sound/soc/codecs/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> sound/soc/codecs/max98927.c:912:22: error: expected ';' after expression
usleep_range(5, 10)
^
;
sound/soc/codecs/max98927.c:914:21: error: expected ';' after expression
usleep_range(1, 5)
^
;
2 errors generated.
vim +912 sound/soc/codecs/max98927.c
867
868 int ret = 0, value;
869 int reg = 0;
870 struct max98927_priv *max98927 = NULL;
871
872 max98927 = devm_kzalloc(&i2c->dev,
873 sizeof(*max98927), GFP_KERNEL);
874
875 if (!max98927) {
876 ret = -ENOMEM;
877 return ret;
878 }
879 i2c_set_clientdata(i2c, max98927);
880
881 /* update interleave mode info */
882 if (!of_property_read_u32(i2c->dev.of_node,
883 "interleave_mode", &value)) {
884 if (value > 0)
885 max98927->interleave_mode = true;
886 else
887 max98927->interleave_mode = false;
888 } else
889 max98927->interleave_mode = false;
890
891 /* regmap initialization */
892 max98927->regmap
893 = devm_regmap_init_i2c(i2c, &max98927_regmap);
894 if (IS_ERR(max98927->regmap)) {
895 ret = PTR_ERR(max98927->regmap);
896 dev_err(&i2c->dev,
897 "Failed to allocate regmap: %d\n", ret);
898 return ret;
899 }
900
901 max98927->reset_gpio
902 = devm_gpiod_get_optional(&i2c->dev, "reset", GPIOD_OUT_HIGH);
903 if (IS_ERR(max98927->reset_gpio)) {
904 ret = PTR_ERR(max98927->reset_gpio);
905 dev_err(&i2c->dev,
906 "Failed to request GPIO reset pin, error %d\n", ret);
907 return ret;
908 }
909
910 if (max98927->reset_gpio) {
911 gpiod_set_value_cansleep(max98927->reset_gpio, 0);
> 912 usleep_range(5, 10)
913 gpiod_set_value_cansleep(max98927->reset_gpio, 1);
914 usleep_range(1, 5)
915 }
916
917 /* Check Revision ID */
918 ret = regmap_read(max98927->regmap,
919 MAX98927_R01FF_REV_ID, ®);
920 if (ret < 0) {
921 dev_err(&i2c->dev,
922 "Failed to read: 0x%02X\n", MAX98927_R01FF_REV_ID);
923 return ret;
924 }
925 dev_info(&i2c->dev, "MAX98927 revisionID: 0x%02X\n", reg);
926
927 /* voltage/current slot configuration */
928 max98927_slot_config(i2c, max98927);
929
930 /* codec registeration */
931 ret = devm_snd_soc_register_component(&i2c->dev,
932 &soc_component_dev_max98927,
933 max98927_dai, ARRAY_SIZE(max98927_dai));
934 if (ret < 0)
935 dev_err(&i2c->dev, "Failed to register component: %d\n", ret);
936
937 return ret;
938 }
939
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30377 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RESEND 1/2] ASoC: max98927: Handle reset gpio when probing i2c
@ 2021-08-29 21:24 ` kernel test robot
0 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2021-08-29 21:24 UTC (permalink / raw)
To: Alejandro Tafalla, Liam Girdwood, Mark Brown, alsa-devel
Cc: devicetree, kbuild-all, linux-kernel, llvm, Takashi Iwai,
Rob Herring, Alejandro Tafalla
[-- Attachment #1: Type: text/plain, Size: 4437 bytes --]
Hi Alejandro,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on asoc/for-next]
[also build test ERROR on v5.14-rc7 next-20210827]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Alejandro-Tafalla/Add-reset-gpios-handling-for-max98927/20210830-010941
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: hexagon-randconfig-r013-20210829 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 510e106fa8635e7f9c51c896180b971de6309b2f)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/4cc91cc4ff05ef061e2247a49c8b7cf9084fe6fb
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Alejandro-Tafalla/Add-reset-gpios-handling-for-max98927/20210830-010941
git checkout 4cc91cc4ff05ef061e2247a49c8b7cf9084fe6fb
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross O=build_dir ARCH=hexagon SHELL=/bin/bash sound/soc/codecs/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> sound/soc/codecs/max98927.c:912:22: error: expected ';' after expression
usleep_range(5, 10)
^
;
sound/soc/codecs/max98927.c:914:21: error: expected ';' after expression
usleep_range(1, 5)
^
;
2 errors generated.
vim +912 sound/soc/codecs/max98927.c
867
868 int ret = 0, value;
869 int reg = 0;
870 struct max98927_priv *max98927 = NULL;
871
872 max98927 = devm_kzalloc(&i2c->dev,
873 sizeof(*max98927), GFP_KERNEL);
874
875 if (!max98927) {
876 ret = -ENOMEM;
877 return ret;
878 }
879 i2c_set_clientdata(i2c, max98927);
880
881 /* update interleave mode info */
882 if (!of_property_read_u32(i2c->dev.of_node,
883 "interleave_mode", &value)) {
884 if (value > 0)
885 max98927->interleave_mode = true;
886 else
887 max98927->interleave_mode = false;
888 } else
889 max98927->interleave_mode = false;
890
891 /* regmap initialization */
892 max98927->regmap
893 = devm_regmap_init_i2c(i2c, &max98927_regmap);
894 if (IS_ERR(max98927->regmap)) {
895 ret = PTR_ERR(max98927->regmap);
896 dev_err(&i2c->dev,
897 "Failed to allocate regmap: %d\n", ret);
898 return ret;
899 }
900
901 max98927->reset_gpio
902 = devm_gpiod_get_optional(&i2c->dev, "reset", GPIOD_OUT_HIGH);
903 if (IS_ERR(max98927->reset_gpio)) {
904 ret = PTR_ERR(max98927->reset_gpio);
905 dev_err(&i2c->dev,
906 "Failed to request GPIO reset pin, error %d\n", ret);
907 return ret;
908 }
909
910 if (max98927->reset_gpio) {
911 gpiod_set_value_cansleep(max98927->reset_gpio, 0);
> 912 usleep_range(5, 10)
913 gpiod_set_value_cansleep(max98927->reset_gpio, 1);
914 usleep_range(1, 5)
915 }
916
917 /* Check Revision ID */
918 ret = regmap_read(max98927->regmap,
919 MAX98927_R01FF_REV_ID, ®);
920 if (ret < 0) {
921 dev_err(&i2c->dev,
922 "Failed to read: 0x%02X\n", MAX98927_R01FF_REV_ID);
923 return ret;
924 }
925 dev_info(&i2c->dev, "MAX98927 revisionID: 0x%02X\n", reg);
926
927 /* voltage/current slot configuration */
928 max98927_slot_config(i2c, max98927);
929
930 /* codec registeration */
931 ret = devm_snd_soc_register_component(&i2c->dev,
932 &soc_component_dev_max98927,
933 max98927_dai, ARRAY_SIZE(max98927_dai));
934 if (ret < 0)
935 dev_err(&i2c->dev, "Failed to register component: %d\n", ret);
936
937 return ret;
938 }
939
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 30377 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RESEND 1/2] ASoC: max98927: Handle reset gpio when probing i2c
@ 2021-08-29 21:24 ` kernel test robot
0 siblings, 0 replies; 14+ messages in thread
From: kernel test robot @ 2021-08-29 21:24 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4559 bytes --]
Hi Alejandro,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on asoc/for-next]
[also build test ERROR on v5.14-rc7 next-20210827]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Alejandro-Tafalla/Add-reset-gpios-handling-for-max98927/20210830-010941
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
config: hexagon-randconfig-r013-20210829 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 510e106fa8635e7f9c51c896180b971de6309b2f)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/4cc91cc4ff05ef061e2247a49c8b7cf9084fe6fb
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Alejandro-Tafalla/Add-reset-gpios-handling-for-max98927/20210830-010941
git checkout 4cc91cc4ff05ef061e2247a49c8b7cf9084fe6fb
# save the attached .config to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross O=build_dir ARCH=hexagon SHELL=/bin/bash sound/soc/codecs/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
>> sound/soc/codecs/max98927.c:912:22: error: expected ';' after expression
usleep_range(5, 10)
^
;
sound/soc/codecs/max98927.c:914:21: error: expected ';' after expression
usleep_range(1, 5)
^
;
2 errors generated.
vim +912 sound/soc/codecs/max98927.c
867
868 int ret = 0, value;
869 int reg = 0;
870 struct max98927_priv *max98927 = NULL;
871
872 max98927 = devm_kzalloc(&i2c->dev,
873 sizeof(*max98927), GFP_KERNEL);
874
875 if (!max98927) {
876 ret = -ENOMEM;
877 return ret;
878 }
879 i2c_set_clientdata(i2c, max98927);
880
881 /* update interleave mode info */
882 if (!of_property_read_u32(i2c->dev.of_node,
883 "interleave_mode", &value)) {
884 if (value > 0)
885 max98927->interleave_mode = true;
886 else
887 max98927->interleave_mode = false;
888 } else
889 max98927->interleave_mode = false;
890
891 /* regmap initialization */
892 max98927->regmap
893 = devm_regmap_init_i2c(i2c, &max98927_regmap);
894 if (IS_ERR(max98927->regmap)) {
895 ret = PTR_ERR(max98927->regmap);
896 dev_err(&i2c->dev,
897 "Failed to allocate regmap: %d\n", ret);
898 return ret;
899 }
900
901 max98927->reset_gpio
902 = devm_gpiod_get_optional(&i2c->dev, "reset", GPIOD_OUT_HIGH);
903 if (IS_ERR(max98927->reset_gpio)) {
904 ret = PTR_ERR(max98927->reset_gpio);
905 dev_err(&i2c->dev,
906 "Failed to request GPIO reset pin, error %d\n", ret);
907 return ret;
908 }
909
910 if (max98927->reset_gpio) {
911 gpiod_set_value_cansleep(max98927->reset_gpio, 0);
> 912 usleep_range(5, 10)
913 gpiod_set_value_cansleep(max98927->reset_gpio, 1);
914 usleep_range(1, 5)
915 }
916
917 /* Check Revision ID */
918 ret = regmap_read(max98927->regmap,
919 MAX98927_R01FF_REV_ID, ®);
920 if (ret < 0) {
921 dev_err(&i2c->dev,
922 "Failed to read: 0x%02X\n", MAX98927_R01FF_REV_ID);
923 return ret;
924 }
925 dev_info(&i2c->dev, "MAX98927 revisionID: 0x%02X\n", reg);
926
927 /* voltage/current slot configuration */
928 max98927_slot_config(i2c, max98927);
929
930 /* codec registeration */
931 ret = devm_snd_soc_register_component(&i2c->dev,
932 &soc_component_dev_max98927,
933 max98927_dai, ARRAY_SIZE(max98927_dai));
934 if (ret < 0)
935 dev_err(&i2c->dev, "Failed to register component: %d\n", ret);
936
937 return ret;
938 }
939
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 30377 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RESEND 1/2] ASoC: max98927: Handle reset gpio when probing i2c
2021-08-29 20:22 ` Andy Shevchenko
@ 2021-08-29 22:13 ` Alejandro Tafalla
0 siblings, 0 replies; 14+ messages in thread
From: Alejandro Tafalla @ 2021-08-29 22:13 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Liam Girdwood, Mark Brown, alsa-devel, Rob Herring,
Jaroslav Kysela, Takashi Iwai, devicetree, linux-kernel
[-- Attachment #1: mensaje --]
[-- Type: text/plain, Size: 1553 bytes --]
Hello Andy,
On Sun, Aug 29, 2021 at 11:22:35PM +0300, Andy Shevchenko wrote:
> > + max98927->reset_gpio
> > + = devm_gpiod_get_optional(&i2c->dev, "reset",
> > GPIOD_OUT_HIGH);
> > + if (IS_ERR(max98927->reset_gpio)) {
> > + ret = PTR_ERR(max98927->reset_gpio);
> > + dev_err(&i2c->dev,
> > + "Failed to request GPIO reset pin, error %d\n",
> > ret);
> > + return ret;
>
>
>
> Spamming logs is not good. Use
>
> return dev_err_probe(...);
Okay.
> > + }
> > +
> > + if (max98927->reset_gpio) {
> > + gpiod_set_value_cansleep(max98927->reset_gpio, 0);
>
>
>
> You may request the pin in a proper state, also with current code you seems
> mishandle the conception of the logical pin level vs. physical one.
Right, i made the mistake of basing off an old driver that use legacy
functions.
> > diff --git a/sound/soc/codecs/max98927.h b/sound/soc/codecs/max98927.h
> > index 05f495db914d..5c04bf38e24a 100644
> > --- a/sound/soc/codecs/max98927.h
> > +++ b/sound/soc/codecs/max98927.h
> > @@ -255,6 +255,7 @@ struct max98927_priv {
> > struct regmap *regmap;
> > struct snd_soc_component *component;
> > struct max98927_pdata *pdata;
>
>
>
> > + struct gpio_desc *reset_gpio;
>
>
> Why? Are you using it outside of ->probe()?
No, I'll delete it and use a local variable.
> With Best Regards,
> Andy Shevchenko
Thank you for the feedback, I'll address all the issues in a V2.
Alejandro Tafalla
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RESEND 1/2] ASoC: max98927: Handle reset gpio when probing i2c
@ 2021-08-29 22:13 ` Alejandro Tafalla
0 siblings, 0 replies; 14+ messages in thread
From: Alejandro Tafalla @ 2021-08-29 22:13 UTC (permalink / raw)
To: Andy Shevchenko
Cc: devicetree, alsa-devel, linux-kernel, Takashi Iwai, Rob Herring,
Liam Girdwood, Mark Brown
[-- Attachment #1: mensaje --]
[-- Type: text/plain, Size: 1553 bytes --]
Hello Andy,
On Sun, Aug 29, 2021 at 11:22:35PM +0300, Andy Shevchenko wrote:
> > + max98927->reset_gpio
> > + = devm_gpiod_get_optional(&i2c->dev, "reset",
> > GPIOD_OUT_HIGH);
> > + if (IS_ERR(max98927->reset_gpio)) {
> > + ret = PTR_ERR(max98927->reset_gpio);
> > + dev_err(&i2c->dev,
> > + "Failed to request GPIO reset pin, error %d\n",
> > ret);
> > + return ret;
>
>
>
> Spamming logs is not good. Use
>
> return dev_err_probe(...);
Okay.
> > + }
> > +
> > + if (max98927->reset_gpio) {
> > + gpiod_set_value_cansleep(max98927->reset_gpio, 0);
>
>
>
> You may request the pin in a proper state, also with current code you seems
> mishandle the conception of the logical pin level vs. physical one.
Right, i made the mistake of basing off an old driver that use legacy
functions.
> > diff --git a/sound/soc/codecs/max98927.h b/sound/soc/codecs/max98927.h
> > index 05f495db914d..5c04bf38e24a 100644
> > --- a/sound/soc/codecs/max98927.h
> > +++ b/sound/soc/codecs/max98927.h
> > @@ -255,6 +255,7 @@ struct max98927_priv {
> > struct regmap *regmap;
> > struct snd_soc_component *component;
> > struct max98927_pdata *pdata;
>
>
>
> > + struct gpio_desc *reset_gpio;
>
>
> Why? Are you using it outside of ->probe()?
No, I'll delete it and use a local variable.
> With Best Regards,
> Andy Shevchenko
Thank you for the feedback, I'll address all the issues in a V2.
Alejandro Tafalla
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RESEND 1/2] ASoC: max98927: Handle reset gpio when probing i2c
2021-08-29 22:13 ` Alejandro Tafalla
@ 2021-09-02 14:43 ` Mark Brown
-1 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2021-09-02 14:43 UTC (permalink / raw)
To: Alejandro Tafalla
Cc: Andy Shevchenko, Liam Girdwood, alsa-devel, Rob Herring,
Jaroslav Kysela, Takashi Iwai, devicetree, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 419 bytes --]
On Mon, Aug 30, 2021 at 12:13:41AM +0200, Alejandro Tafalla wrote:
> On Sun, Aug 29, 2021 at 11:22:35PM +0300, Andy Shevchenko wrote:
> > > + struct gpio_desc *reset_gpio;
> > Why? Are you using it outside of ->probe()?
> No, I'll delete it and use a local variable.
It can be good to reassert reset when unloading the driver in order to
ensure that the device isn't active. It doesn't really matter though.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH RESEND 1/2] ASoC: max98927: Handle reset gpio when probing i2c
@ 2021-09-02 14:43 ` Mark Brown
0 siblings, 0 replies; 14+ messages in thread
From: Mark Brown @ 2021-09-02 14:43 UTC (permalink / raw)
To: Alejandro Tafalla
Cc: devicetree, alsa-devel, linux-kernel, Takashi Iwai,
Liam Girdwood, Andy Shevchenko, Rob Herring
[-- Attachment #1: Type: text/plain, Size: 419 bytes --]
On Mon, Aug 30, 2021 at 12:13:41AM +0200, Alejandro Tafalla wrote:
> On Sun, Aug 29, 2021 at 11:22:35PM +0300, Andy Shevchenko wrote:
> > > + struct gpio_desc *reset_gpio;
> > Why? Are you using it outside of ->probe()?
> No, I'll delete it and use a local variable.
It can be good to reassert reset when unloading the driver in order to
ensure that the device isn't active. It doesn't really matter though.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-09-02 14:45 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-29 17:00 [PATCH RESEND 0/2] Add reset-gpios handling for max98927 Alejandro Tafalla
2021-08-29 17:00 ` Alejandro Tafalla
2021-08-29 17:00 ` [PATCH RESEND 1/2] ASoC: max98927: Handle reset gpio when probing i2c Alejandro Tafalla
2021-08-29 17:00 ` Alejandro Tafalla
2021-08-29 20:22 ` Andy Shevchenko
2021-08-29 22:13 ` Alejandro Tafalla
2021-08-29 22:13 ` Alejandro Tafalla
2021-09-02 14:43 ` Mark Brown
2021-09-02 14:43 ` Mark Brown
2021-08-29 21:24 ` kernel test robot
2021-08-29 21:24 ` kernel test robot
2021-08-29 21:24 ` kernel test robot
2021-08-29 17:00 ` [PATCH RESEND 2/2] dt-bindings: sound: max98927: Add reset-gpios optional property Alejandro Tafalla
2021-08-29 17:00 ` Alejandro Tafalla
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.