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
Drive the reset gpio if defined in the DTS node.
Signed-off-by: Alejandro Tafalla <[email protected]>
---
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
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 <[email protected]>
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/[email protected]
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
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.