| Message ID | 20201227211232.117801-4-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | MFD/extcon/ASoC: Add support for Intel Bay Trail boards with WM5102 codec | expand |
Hi Hans,
I love your patch! Perhaps something to improve:
[auto build test WARNING on lee-mfd/for-mfd-next]
[also build test WARNING on chanwoo-extcon/extcon-next asoc/for-next v5.10 next-20201223]
[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/Hans-de-Goede/MFD-extcon-ASoC-Add-support-for-Intel-Bay-Trail-boards-with-WM5102-codec/20201228-051806
base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: i386-randconfig-s002-20201227 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.3-184-g1b896707-dirty
# https://github.com/0day-ci/linux/commit/bc64046ad088b340f127f9eefdb8f941c03dcb53
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Hans-de-Goede/MFD-extcon-ASoC-Add-support-for-Intel-Bay-Trail-boards-with-WM5102-codec/20201228-051806
git checkout bc64046ad088b340f127f9eefdb8f941c03dcb53
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
"sparse warnings: (new ones prefixed by >>)"
>> drivers/mfd/arizona-spi.c:29:31: sparse: sparse: symbol 'ldoena_gpios' was not declared. Should it be static?
Please review and possibly fold the followup patch.
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Sun, Dec 27, 2020 at 11:16 PM Hans de Goede <hdegoede@redhat.com> wrote: > > The Intel Bay Trail (x86/ACPI) based Lenovo Yoga Tablet 2 series use > a WM5102 codec connected over SPI. > > Add support for ACPI enumeration to arizona-spi so that arizona-spi can > bind to the codec on these tablets. > > This is loosely based on an earlier attempt (for Android-x86) at this by > Christian Hartmann, combined with insights in things like the speaker GPIO > from the android-x86 android port for the Lenovo Yoga Tablet 2 1051F/L [1]. Few nitpicks here and there, but the most important bit that hits me is device_get_match_data(). > [1] https://github.com/Kitsune2222/Android_Yoga_Tablet_2-1051F_Kernel > > Cc: Christian Hartmann <cornogle@googlemail.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/mfd/arizona-spi.c | 120 ++++++++++++++++++++++++++++++++++++++ > 1 file changed, 120 insertions(+) > > diff --git a/drivers/mfd/arizona-spi.c b/drivers/mfd/arizona-spi.c > index 704f214d2614..bcdbd72fefb5 100644 > --- a/drivers/mfd/arizona-spi.c > +++ b/drivers/mfd/arizona-spi.c > @@ -7,7 +7,10 @@ > * Author: Mark Brown <broonie@opensource.wolfsonmicro.com> > */ > > +#include <linux/acpi.h> > #include <linux/err.h> > +#include <linux/gpio/consumer.h> > +#include <linux/gpio/machine.h> > #include <linux/module.h> > #include <linux/pm_runtime.h> > #include <linux/regmap.h> > @@ -15,11 +18,119 @@ > #include <linux/slab.h> > #include <linux/spi/spi.h> > #include <linux/of.h> > +#include <uapi/linux/input-event-codes.h> > > #include <linux/mfd/arizona/core.h> > > #include "arizona.h" > > +#ifdef CONFIG_ACPI > +const struct acpi_gpio_params reset_gpios = { 1, 0, false }; > +const struct acpi_gpio_params ldoena_gpios = { 2, 0, false }; > + > +static const struct acpi_gpio_mapping arizona_acpi_gpios[] = { > + { "reset-gpios", &reset_gpios, 1, }, > + { "wlf,ldoena-gpios", &ldoena_gpios, 1 }, > + { } > +}; > + > +/* > + * The ACPI resources for the device only describe external GPIO-s. They do > + * not provide mappings for the GPIO-s coming from the Arizona codec itself. > + */ > +static const struct gpiod_lookup arizona_soc_gpios[] = { > + { "arizona", 2, "wlf,spkvdd-ena", 0, GPIO_ACTIVE_HIGH }, > + { "arizona", 4, "wlf,micd-pol", 0, GPIO_ACTIVE_LOW }, > +}; > + > +/* > + * The AOSP 3.5 mm Headset: Accessory Specification gives the following values: > + * Function A Play/Pause: 0 ohm > + * Function D Voice assistant: 135 ohm > + * Function B Volume Up 240 ohm > + * Function C Volume Down 470 ohm > + * Minimum Mic DC resistance 1000 ohm > + * Minimum Ear speaker impedance 16 ohm > + * Note the first max value below must be less then the min. speaker impedance, > + * to allow CTIA/OMTP detection to work. The other max values are the closest > + * value from extcon-arizona.c:arizona_micd_levels halfway 2 button resistances. > + */ > +static const struct arizona_micd_range arizona_micd_aosp_ranges[] = { > + { .max = 11, .key = KEY_PLAYPAUSE }, > + { .max = 186, .key = KEY_VOICECOMMAND }, > + { .max = 348, .key = KEY_VOLUMEUP }, > + { .max = 752, .key = KEY_VOLUMEDOWN }, > +}; > + > +static void arizona_spi_acpi_remove_lookup(void *lookup) > +{ > + gpiod_remove_lookup_table(lookup); > +} > + > +static int arizona_spi_acpi_probe(struct arizona *arizona) > +{ > + struct gpiod_lookup_table *lookup; > + int i, ret; > + > + /* Add mappings for the 2 ACPI declared GPIOs used for reset and ldo-ena */ > + devm_acpi_dev_add_driver_gpios(arizona->dev, arizona_acpi_gpios); > + > + /* Add lookups for the SoCs own GPIOs used for micdet-polarity and spkVDD-enable */ > + lookup = devm_kzalloc(arizona->dev, > + struct_size(lookup, table, ARRAY_SIZE(arizona_soc_gpios) + 1), > + GFP_KERNEL); > + if (!lookup) > + return -ENOMEM; > + > + lookup->dev_id = dev_name(arizona->dev); > + for (i = 0; i < ARRAY_SIZE(arizona_soc_gpios); i++) > + lookup->table[i] = arizona_soc_gpios[i]; Would memcpy() do the same at one pass? > + gpiod_add_lookup_table(lookup); > + ret = devm_add_action_or_reset(arizona->dev, arizona_spi_acpi_remove_lookup, lookup); > + if (ret) > + return ret; > + /* Enable 32KHz clock from SoC to codec for jack-detect */ > + acpi_evaluate_object(ACPI_HANDLE(arizona->dev), "CLKE", NULL, NULL); No error check? > + /* > + * Some DSDTs wrongly declare the IRQ trigger-type as IRQF_TRIGGER_FALLING > + * The IRQ line will stay low when a new IRQ event happens between reading > + * the IRQ status flags and acknowledging them. When the IRQ line stays > + * low like this the IRQ will never trigger again when its type is set > + * to IRQF_TRIGGER_FALLING. Correct the IRQ trigger-type to fix this. > + */ > + arizona->pdata.irq_flags = IRQF_TRIGGER_LOW; > + > + /* Wait 200 ms after jack insertion */ > + arizona->pdata.micd_detect_debounce = 200; > + > + /* Use standard AOSP values for headset-button mappings */ > + arizona->pdata.micd_ranges = arizona_micd_aosp_ranges; > + arizona->pdata.num_micd_ranges = ARRAY_SIZE(arizona_micd_aosp_ranges); > + > + return 0; > +} > + > +static const struct acpi_device_id arizona_acpi_match[] = { > + { > + .id = "WM510204", > + .driver_data = WM5102, > + }, > + { > + .id = "WM510205", > + .driver_data = WM5102, > + }, > + { }, No need for comma here. > +}; > +MODULE_DEVICE_TABLE(acpi, arizona_acpi_match); > +#else > +static void arizona_spi_acpi_probe(struct arizona *arizona) > +{ > +} Can be one line? > +#endif > + > static int arizona_spi_probe(struct spi_device *spi) > { > const struct spi_device_id *id = spi_get_device_id(spi); > @@ -30,6 +141,8 @@ static int arizona_spi_probe(struct spi_device *spi) > > if (spi->dev.of_node) > type = arizona_of_get_type(&spi->dev); > + else if (ACPI_COMPANION(&spi->dev)) > + type = (unsigned long)acpi_device_get_match_data(&spi->dev); Can we rather get rid of these and use device_get_match_data() directly? > else > type = id->driver_data; > > @@ -75,6 +188,12 @@ static int arizona_spi_probe(struct spi_device *spi) > arizona->dev = &spi->dev; > arizona->irq = spi->irq; > > + if (ACPI_COMPANION(&spi->dev)) { has_acpi_companion() ? > + ret = arizona_spi_acpi_probe(arizona); > + if (ret) > + return ret; > + } > + > return arizona_dev_init(arizona); > } > > @@ -102,6 +221,7 @@ static struct spi_driver arizona_spi_driver = { > .name = "arizona", > .pm = &arizona_pm_ops, > .of_match_table = of_match_ptr(arizona_of_match), > + .acpi_match_table = ACPI_PTR(arizona_acpi_match), > }, > .probe = arizona_spi_probe, > .remove = arizona_spi_remove, > -- > 2.28.0 >
Hi Hans,
I love your patch! Yet something to improve:
[auto build test ERROR on lee-mfd/for-mfd-next]
[also build test ERROR on chanwoo-extcon/extcon-next asoc/for-next v5.11-rc1 next-20201223]
[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/Hans-de-Goede/MFD-extcon-ASoC-Add-support-for-Intel-Bay-Trail-boards-with-WM5102-codec/20201228-051806
base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: s390-randconfig-r034-20201228 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project cee1e7d14f4628d6174b33640d502bff3b54ae45)
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
# install s390 cross compiling tool for clang build
# apt-get install binutils-s390x-linux-gnu
# https://github.com/0day-ci/linux/commit/bc64046ad088b340f127f9eefdb8f941c03dcb53
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Hans-de-Goede/MFD-extcon-ASoC-Add-support-for-Intel-Bay-Trail-boards-with-WM5102-codec/20201228-051806
git checkout bc64046ad088b340f127f9eefdb8f941c03dcb53
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=s390
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 >>):
include/uapi/linux/swab.h:19:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0x000000ffUL) << 24) | \
^
In file included from drivers/mfd/arizona-spi.c:16:
In file included from include/linux/regmap.h:20:
In file included from include/linux/iopoll.h:14:
In file included from include/linux/io.h:13:
In file included from arch/s390/include/asm/io.h:80:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : \
^
include/uapi/linux/swab.h:20:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0x0000ff00UL) << 8) | \
^
In file included from drivers/mfd/arizona-spi.c:16:
In file included from include/linux/regmap.h:20:
In file included from include/linux/iopoll.h:14:
In file included from include/linux/io.h:13:
In file included from arch/s390/include/asm/io.h:80:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : \
^
include/uapi/linux/swab.h:21:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0x00ff0000UL) >> 8) | \
^
In file included from drivers/mfd/arizona-spi.c:16:
In file included from include/linux/regmap.h:20:
In file included from include/linux/iopoll.h:14:
In file included from include/linux/io.h:13:
In file included from arch/s390/include/asm/io.h:80:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:119:21: note: expanded from macro '__swab32'
___constant_swab32(x) : \
^
include/uapi/linux/swab.h:22:12: note: expanded from macro '___constant_swab32'
(((__u32)(x) & (__u32)0xff000000UL) >> 24)))
^
In file included from drivers/mfd/arizona-spi.c:16:
In file included from include/linux/regmap.h:20:
In file included from include/linux/iopoll.h:14:
In file included from include/linux/io.h:13:
In file included from arch/s390/include/asm/io.h:80:
include/asm-generic/io.h:490:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/big_endian.h:34:59: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
^
include/uapi/linux/swab.h:120:12: note: expanded from macro '__swab32'
__fswab32(x))
^
In file included from drivers/mfd/arizona-spi.c:16:
In file included from include/linux/regmap.h:20:
In file included from include/linux/iopoll.h:14:
In file included from include/linux/io.h:13:
In file included from arch/s390/include/asm/io.h:80:
include/asm-generic/io.h:501:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writeb(value, PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:511:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:521:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
__raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:609:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:617:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:625:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
readsl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:634:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesb(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:643:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesw(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
include/asm-generic/io.h:652:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
writesl(PCI_IOBASE + addr, buffer, count);
~~~~~~~~~~ ^
>> drivers/mfd/arizona-spi.c:192:7: error: assigning to 'int' from incompatible type 'void'
ret = arizona_spi_acpi_probe(arizona);
^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
20 warnings and 1 error generated.
vim +192 drivers/mfd/arizona-spi.c
133
134 static int arizona_spi_probe(struct spi_device *spi)
135 {
136 const struct spi_device_id *id = spi_get_device_id(spi);
137 struct arizona *arizona;
138 const struct regmap_config *regmap_config = NULL;
139 unsigned long type;
140 int ret;
141
142 if (spi->dev.of_node)
143 type = arizona_of_get_type(&spi->dev);
144 else if (ACPI_COMPANION(&spi->dev))
145 type = (unsigned long)acpi_device_get_match_data(&spi->dev);
146 else
147 type = id->driver_data;
148
149 switch (type) {
150 case WM5102:
151 if (IS_ENABLED(CONFIG_MFD_WM5102))
152 regmap_config = &wm5102_spi_regmap;
153 break;
154 case WM5110:
155 case WM8280:
156 if (IS_ENABLED(CONFIG_MFD_WM5110))
157 regmap_config = &wm5110_spi_regmap;
158 break;
159 case WM1831:
160 case CS47L24:
161 if (IS_ENABLED(CONFIG_MFD_CS47L24))
162 regmap_config = &cs47l24_spi_regmap;
163 break;
164 default:
165 dev_err(&spi->dev, "Unknown device type %ld\n", type);
166 return -EINVAL;
167 }
168
169 if (!regmap_config) {
170 dev_err(&spi->dev,
171 "No kernel support for device type %ld\n", type);
172 return -EINVAL;
173 }
174
175 arizona = devm_kzalloc(&spi->dev, sizeof(*arizona), GFP_KERNEL);
176 if (arizona == NULL)
177 return -ENOMEM;
178
179 arizona->regmap = devm_regmap_init_spi(spi, regmap_config);
180 if (IS_ERR(arizona->regmap)) {
181 ret = PTR_ERR(arizona->regmap);
182 dev_err(&spi->dev, "Failed to allocate register map: %d\n",
183 ret);
184 return ret;
185 }
186
187 arizona->type = type;
188 arizona->dev = &spi->dev;
189 arizona->irq = spi->irq;
190
191 if (ACPI_COMPANION(&spi->dev)) {
> 192 ret = arizona_spi_acpi_probe(arizona);
193 if (ret)
194 return ret;
195 }
196
197 return arizona_dev_init(arizona);
198 }
199
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Hans,
I love your patch! Yet something to improve:
[auto build test ERROR on lee-mfd/for-mfd-next]
[also build test ERROR on chanwoo-extcon/extcon-next asoc/for-next v5.11-rc1 next-20201223]
[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/Hans-de-Goede/MFD-extcon-ASoC-Add-support-for-Intel-Bay-Trail-boards-with-WM5102-codec/20201228-051806
base: https://git.kernel.org/pub/scm/linux/kernel/git/lee/mfd.git for-mfd-next
config: alpha-randconfig-r035-20201228 (attached as .config)
compiler: alpha-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/bc64046ad088b340f127f9eefdb8f941c03dcb53
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Hans-de-Goede/MFD-extcon-ASoC-Add-support-for-Intel-Bay-Trail-boards-with-WM5102-codec/20201228-051806
git checkout bc64046ad088b340f127f9eefdb8f941c03dcb53
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=alpha
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 >>):
drivers/mfd/arizona-spi.c: In function 'arizona_spi_probe':
>> drivers/mfd/arizona-spi.c:192:7: error: void value not ignored as it ought to be
192 | ret = arizona_spi_acpi_probe(arizona);
| ^
vim +192 drivers/mfd/arizona-spi.c
133
134 static int arizona_spi_probe(struct spi_device *spi)
135 {
136 const struct spi_device_id *id = spi_get_device_id(spi);
137 struct arizona *arizona;
138 const struct regmap_config *regmap_config = NULL;
139 unsigned long type;
140 int ret;
141
142 if (spi->dev.of_node)
143 type = arizona_of_get_type(&spi->dev);
144 else if (ACPI_COMPANION(&spi->dev))
145 type = (unsigned long)acpi_device_get_match_data(&spi->dev);
146 else
147 type = id->driver_data;
148
149 switch (type) {
150 case WM5102:
151 if (IS_ENABLED(CONFIG_MFD_WM5102))
152 regmap_config = &wm5102_spi_regmap;
153 break;
154 case WM5110:
155 case WM8280:
156 if (IS_ENABLED(CONFIG_MFD_WM5110))
157 regmap_config = &wm5110_spi_regmap;
158 break;
159 case WM1831:
160 case CS47L24:
161 if (IS_ENABLED(CONFIG_MFD_CS47L24))
162 regmap_config = &cs47l24_spi_regmap;
163 break;
164 default:
165 dev_err(&spi->dev, "Unknown device type %ld\n", type);
166 return -EINVAL;
167 }
168
169 if (!regmap_config) {
170 dev_err(&spi->dev,
171 "No kernel support for device type %ld\n", type);
172 return -EINVAL;
173 }
174
175 arizona = devm_kzalloc(&spi->dev, sizeof(*arizona), GFP_KERNEL);
176 if (arizona == NULL)
177 return -ENOMEM;
178
179 arizona->regmap = devm_regmap_init_spi(spi, regmap_config);
180 if (IS_ERR(arizona->regmap)) {
181 ret = PTR_ERR(arizona->regmap);
182 dev_err(&spi->dev, "Failed to allocate register map: %d\n",
183 ret);
184 return ret;
185 }
186
187 arizona->type = type;
188 arizona->dev = &spi->dev;
189 arizona->irq = spi->irq;
190
191 if (ACPI_COMPANION(&spi->dev)) {
> 192 ret = arizona_spi_acpi_probe(arizona);
193 if (ret)
194 return ret;
195 }
196
197 return arizona_dev_init(arizona);
198 }
199
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi, Thank you for the review. On 12/28/20 3:14 PM, Andy Shevchenko wrote: > On Sun, Dec 27, 2020 at 11:16 PM Hans de Goede <hdegoede@redhat.com> wrote: >> >> The Intel Bay Trail (x86/ACPI) based Lenovo Yoga Tablet 2 series use >> a WM5102 codec connected over SPI. >> >> Add support for ACPI enumeration to arizona-spi so that arizona-spi can >> bind to the codec on these tablets. >> >> This is loosely based on an earlier attempt (for Android-x86) at this by >> Christian Hartmann, combined with insights in things like the speaker GPIO >> from the android-x86 android port for the Lenovo Yoga Tablet 2 1051F/L [1]. > > Few nitpicks here and there, but the most important bit that hits me > is device_get_match_data(). > >> [1] https://github.com/Kitsune2222/Android_Yoga_Tablet_2-1051F_Kernel >> >> Cc: Christian Hartmann <cornogle@googlemail.com> >> Signed-off-by: Hans de Goede <hdegoede@redhat.com> >> --- >> drivers/mfd/arizona-spi.c | 120 ++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 120 insertions(+) >> >> diff --git a/drivers/mfd/arizona-spi.c b/drivers/mfd/arizona-spi.c >> index 704f214d2614..bcdbd72fefb5 100644 >> --- a/drivers/mfd/arizona-spi.c >> +++ b/drivers/mfd/arizona-spi.c >> @@ -7,7 +7,10 @@ >> * Author: Mark Brown <broonie@opensource.wolfsonmicro.com> >> */ >> >> +#include <linux/acpi.h> >> #include <linux/err.h> >> +#include <linux/gpio/consumer.h> >> +#include <linux/gpio/machine.h> >> #include <linux/module.h> >> #include <linux/pm_runtime.h> >> #include <linux/regmap.h> >> @@ -15,11 +18,119 @@ >> #include <linux/slab.h> >> #include <linux/spi/spi.h> >> #include <linux/of.h> >> +#include <uapi/linux/input-event-codes.h> >> >> #include <linux/mfd/arizona/core.h> >> >> #include "arizona.h" >> >> +#ifdef CONFIG_ACPI >> +const struct acpi_gpio_params reset_gpios = { 1, 0, false }; >> +const struct acpi_gpio_params ldoena_gpios = { 2, 0, false }; >> + >> +static const struct acpi_gpio_mapping arizona_acpi_gpios[] = { >> + { "reset-gpios", &reset_gpios, 1, }, >> + { "wlf,ldoena-gpios", &ldoena_gpios, 1 }, >> + { } >> +}; >> + >> +/* >> + * The ACPI resources for the device only describe external GPIO-s. They do >> + * not provide mappings for the GPIO-s coming from the Arizona codec itself. >> + */ >> +static const struct gpiod_lookup arizona_soc_gpios[] = { >> + { "arizona", 2, "wlf,spkvdd-ena", 0, GPIO_ACTIVE_HIGH }, >> + { "arizona", 4, "wlf,micd-pol", 0, GPIO_ACTIVE_LOW }, >> +}; >> + >> +/* >> + * The AOSP 3.5 mm Headset: Accessory Specification gives the following values: >> + * Function A Play/Pause: 0 ohm >> + * Function D Voice assistant: 135 ohm >> + * Function B Volume Up 240 ohm >> + * Function C Volume Down 470 ohm >> + * Minimum Mic DC resistance 1000 ohm >> + * Minimum Ear speaker impedance 16 ohm >> + * Note the first max value below must be less then the min. speaker impedance, >> + * to allow CTIA/OMTP detection to work. The other max values are the closest >> + * value from extcon-arizona.c:arizona_micd_levels halfway 2 button resistances. >> + */ >> +static const struct arizona_micd_range arizona_micd_aosp_ranges[] = { >> + { .max = 11, .key = KEY_PLAYPAUSE }, >> + { .max = 186, .key = KEY_VOICECOMMAND }, >> + { .max = 348, .key = KEY_VOLUMEUP }, >> + { .max = 752, .key = KEY_VOLUMEDOWN }, >> +}; >> + >> +static void arizona_spi_acpi_remove_lookup(void *lookup) >> +{ >> + gpiod_remove_lookup_table(lookup); >> +} >> + >> +static int arizona_spi_acpi_probe(struct arizona *arizona) >> +{ >> + struct gpiod_lookup_table *lookup; >> + int i, ret; >> + >> + /* Add mappings for the 2 ACPI declared GPIOs used for reset and ldo-ena */ >> + devm_acpi_dev_add_driver_gpios(arizona->dev, arizona_acpi_gpios); >> + >> + /* Add lookups for the SoCs own GPIOs used for micdet-polarity and spkVDD-enable */ >> + lookup = devm_kzalloc(arizona->dev, >> + struct_size(lookup, table, ARRAY_SIZE(arizona_soc_gpios) + 1), >> + GFP_KERNEL); >> + if (!lookup) >> + return -ENOMEM; >> + >> + lookup->dev_id = dev_name(arizona->dev); > >> + for (i = 0; i < ARRAY_SIZE(arizona_soc_gpios); i++) >> + lookup->table[i] = arizona_soc_gpios[i]; > > Would memcpy() do the same at one pass? True, fixed for v2. >> + gpiod_add_lookup_table(lookup); >> + ret = devm_add_action_or_reset(arizona->dev, arizona_spi_acpi_remove_lookup, lookup); >> + if (ret) >> + return ret; > >> + /* Enable 32KHz clock from SoC to codec for jack-detect */ >> + acpi_evaluate_object(ACPI_HANDLE(arizona->dev), "CLKE", NULL, NULL); > > No error check? The codec will still work without the 32KHz clk, but we will loose jack-detect functionality. I expect the ACPI call to already print some error, but to make sure something is logged and to clarify what any previous logged ACPI errors are about I've added code to log a warning when this fails for v2. > >> + /* >> + * Some DSDTs wrongly declare the IRQ trigger-type as IRQF_TRIGGER_FALLING >> + * The IRQ line will stay low when a new IRQ event happens between reading >> + * the IRQ status flags and acknowledging them. When the IRQ line stays >> + * low like this the IRQ will never trigger again when its type is set >> + * to IRQF_TRIGGER_FALLING. Correct the IRQ trigger-type to fix this. >> + */ >> + arizona->pdata.irq_flags = IRQF_TRIGGER_LOW; >> + >> + /* Wait 200 ms after jack insertion */ >> + arizona->pdata.micd_detect_debounce = 200; >> + >> + /* Use standard AOSP values for headset-button mappings */ >> + arizona->pdata.micd_ranges = arizona_micd_aosp_ranges; >> + arizona->pdata.num_micd_ranges = ARRAY_SIZE(arizona_micd_aosp_ranges); >> + >> + return 0; >> +} >> + >> +static const struct acpi_device_id arizona_acpi_match[] = { >> + { >> + .id = "WM510204", >> + .driver_data = WM5102, >> + }, >> + { >> + .id = "WM510205", >> + .driver_data = WM5102, >> + }, > >> + { }, > > No need for comma here. Fixed for v2. > >> +}; >> +MODULE_DEVICE_TABLE(acpi, arizona_acpi_match); >> +#else > >> +static void arizona_spi_acpi_probe(struct arizona *arizona) >> +{ >> +} > > Can be one line? I find it more readable as is. > >> +#endif >> + >> static int arizona_spi_probe(struct spi_device *spi) >> { >> const struct spi_device_id *id = spi_get_device_id(spi); >> @@ -30,6 +141,8 @@ static int arizona_spi_probe(struct spi_device *spi) >> >> if (spi->dev.of_node) >> type = arizona_of_get_type(&spi->dev); >> + else if (ACPI_COMPANION(&spi->dev)) >> + type = (unsigned long)acpi_device_get_match_data(&spi->dev); > > Can we rather get rid of these and use device_get_match_data() directly? That is a good idea, I'll add a nw preparation patch to v2 replacing the custom arizona_of_get_type() helper with device_get_match_data(). >> else >> type = id->driver_data; >> >> @@ -75,6 +188,12 @@ static int arizona_spi_probe(struct spi_device *spi) >> arizona->dev = &spi->dev; >> arizona->irq = spi->irq; >> >> + if (ACPI_COMPANION(&spi->dev)) { > > has_acpi_companion() ? Fixed for v2. >> + ret = arizona_spi_acpi_probe(arizona); >> + if (ret) >> + return ret; >> + } >> + >> return arizona_dev_init(arizona); >> } >> >> @@ -102,6 +221,7 @@ static struct spi_driver arizona_spi_driver = { >> .name = "arizona", >> .pm = &arizona_pm_ops, >> .of_match_table = of_match_ptr(arizona_of_match), >> + .acpi_match_table = ACPI_PTR(arizona_acpi_match), >> }, >> .probe = arizona_spi_probe, >> .remove = arizona_spi_remove, >> -- >> 2.28.0 >> > > Regards, Hans
diff --git a/drivers/mfd/arizona-spi.c b/drivers/mfd/arizona-spi.c index 704f214d2614..bcdbd72fefb5 100644 --- a/drivers/mfd/arizona-spi.c +++ b/drivers/mfd/arizona-spi.c @@ -7,7 +7,10 @@ * Author: Mark Brown <broonie@opensource.wolfsonmicro.com> */ +#include <linux/acpi.h> #include <linux/err.h> +#include <linux/gpio/consumer.h> +#include <linux/gpio/machine.h> #include <linux/module.h> #include <linux/pm_runtime.h> #include <linux/regmap.h> @@ -15,11 +18,119 @@ #include <linux/slab.h> #include <linux/spi/spi.h> #include <linux/of.h> +#include <uapi/linux/input-event-codes.h> #include <linux/mfd/arizona/core.h> #include "arizona.h" +#ifdef CONFIG_ACPI +const struct acpi_gpio_params reset_gpios = { 1, 0, false }; +const struct acpi_gpio_params ldoena_gpios = { 2, 0, false }; + +static const struct acpi_gpio_mapping arizona_acpi_gpios[] = { + { "reset-gpios", &reset_gpios, 1, }, + { "wlf,ldoena-gpios", &ldoena_gpios, 1 }, + { } +}; + +/* + * The ACPI resources for the device only describe external GPIO-s. They do + * not provide mappings for the GPIO-s coming from the Arizona codec itself. + */ +static const struct gpiod_lookup arizona_soc_gpios[] = { + { "arizona", 2, "wlf,spkvdd-ena", 0, GPIO_ACTIVE_HIGH }, + { "arizona", 4, "wlf,micd-pol", 0, GPIO_ACTIVE_LOW }, +}; + +/* + * The AOSP 3.5 mm Headset: Accessory Specification gives the following values: + * Function A Play/Pause: 0 ohm + * Function D Voice assistant: 135 ohm + * Function B Volume Up 240 ohm + * Function C Volume Down 470 ohm + * Minimum Mic DC resistance 1000 ohm + * Minimum Ear speaker impedance 16 ohm + * Note the first max value below must be less then the min. speaker impedance, + * to allow CTIA/OMTP detection to work. The other max values are the closest + * value from extcon-arizona.c:arizona_micd_levels halfway 2 button resistances. + */ +static const struct arizona_micd_range arizona_micd_aosp_ranges[] = { + { .max = 11, .key = KEY_PLAYPAUSE }, + { .max = 186, .key = KEY_VOICECOMMAND }, + { .max = 348, .key = KEY_VOLUMEUP }, + { .max = 752, .key = KEY_VOLUMEDOWN }, +}; + +static void arizona_spi_acpi_remove_lookup(void *lookup) +{ + gpiod_remove_lookup_table(lookup); +} + +static int arizona_spi_acpi_probe(struct arizona *arizona) +{ + struct gpiod_lookup_table *lookup; + int i, ret; + + /* Add mappings for the 2 ACPI declared GPIOs used for reset and ldo-ena */ + devm_acpi_dev_add_driver_gpios(arizona->dev, arizona_acpi_gpios); + + /* Add lookups for the SoCs own GPIOs used for micdet-polarity and spkVDD-enable */ + lookup = devm_kzalloc(arizona->dev, + struct_size(lookup, table, ARRAY_SIZE(arizona_soc_gpios) + 1), + GFP_KERNEL); + if (!lookup) + return -ENOMEM; + + lookup->dev_id = dev_name(arizona->dev); + for (i = 0; i < ARRAY_SIZE(arizona_soc_gpios); i++) + lookup->table[i] = arizona_soc_gpios[i]; + + gpiod_add_lookup_table(lookup); + ret = devm_add_action_or_reset(arizona->dev, arizona_spi_acpi_remove_lookup, lookup); + if (ret) + return ret; + + /* Enable 32KHz clock from SoC to codec for jack-detect */ + acpi_evaluate_object(ACPI_HANDLE(arizona->dev), "CLKE", NULL, NULL); + + /* + * Some DSDTs wrongly declare the IRQ trigger-type as IRQF_TRIGGER_FALLING + * The IRQ line will stay low when a new IRQ event happens between reading + * the IRQ status flags and acknowledging them. When the IRQ line stays + * low like this the IRQ will never trigger again when its type is set + * to IRQF_TRIGGER_FALLING. Correct the IRQ trigger-type to fix this. + */ + arizona->pdata.irq_flags = IRQF_TRIGGER_LOW; + + /* Wait 200 ms after jack insertion */ + arizona->pdata.micd_detect_debounce = 200; + + /* Use standard AOSP values for headset-button mappings */ + arizona->pdata.micd_ranges = arizona_micd_aosp_ranges; + arizona->pdata.num_micd_ranges = ARRAY_SIZE(arizona_micd_aosp_ranges); + + return 0; +} + +static const struct acpi_device_id arizona_acpi_match[] = { + { + .id = "WM510204", + .driver_data = WM5102, + }, + { + .id = "WM510205", + .driver_data = WM5102, + }, + { }, +}; +MODULE_DEVICE_TABLE(acpi, arizona_acpi_match); +#else +static void arizona_spi_acpi_probe(struct arizona *arizona) +{ +} +#endif + static int arizona_spi_probe(struct spi_device *spi) { const struct spi_device_id *id = spi_get_device_id(spi); @@ -30,6 +141,8 @@ static int arizona_spi_probe(struct spi_device *spi) if (spi->dev.of_node) type = arizona_of_get_type(&spi->dev); + else if (ACPI_COMPANION(&spi->dev)) + type = (unsigned long)acpi_device_get_match_data(&spi->dev); else type = id->driver_data; @@ -75,6 +188,12 @@ static int arizona_spi_probe(struct spi_device *spi) arizona->dev = &spi->dev; arizona->irq = spi->irq; + if (ACPI_COMPANION(&spi->dev)) { + ret = arizona_spi_acpi_probe(arizona); + if (ret) + return ret; + } + return arizona_dev_init(arizona); } @@ -102,6 +221,7 @@ static struct spi_driver arizona_spi_driver = { .name = "arizona", .pm = &arizona_pm_ops, .of_match_table = of_match_ptr(arizona_of_match), + .acpi_match_table = ACPI_PTR(arizona_acpi_match), }, .probe = arizona_spi_probe, .remove = arizona_spi_remove,
The Intel Bay Trail (x86/ACPI) based Lenovo Yoga Tablet 2 series use a WM5102 codec connected over SPI. Add support for ACPI enumeration to arizona-spi so that arizona-spi can bind to the codec on these tablets. This is loosely based on an earlier attempt (for Android-x86) at this by Christian Hartmann, combined with insights in things like the speaker GPIO from the android-x86 android port for the Lenovo Yoga Tablet 2 1051F/L [1]. [1] https://github.com/Kitsune2222/Android_Yoga_Tablet_2-1051F_Kernel Cc: Christian Hartmann <cornogle@googlemail.com> Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/mfd/arizona-spi.c | 120 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 120 insertions(+)