[PATCH RESEND 0/2] Add reset-gpios handling for max98927
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 <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
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
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.
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-h... 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/4cc91cc4ff05ef061e2247a49c8b7cf9084f... 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
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
participants (4)
-
Alejandro Tafalla -
Andy Shevchenko -
kernel test robot -
Mark Brown