| Message ID | 20230601054549.10843-10-nikita.shubin@maquefel.me (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ep93xx device tree conversion | expand |
Hi Nikita,
kernel test robot noticed the following build errors:
[auto build test ERROR on clk/clk-next]
[also build test ERROR on linusw-pinctrl/devel linusw-pinctrl/for-next linus/master v6.4-rc4 next-20230601]
[cannot apply to soc/for-next robh/for-next]
[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#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Nikita-Shubin/dt-bindings-soc-Add-Cirrus-EP93xx/20230601-143415
base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
patch link: https://lore.kernel.org/r/20230601054549.10843-10-nikita.shubin%40maquefel.me
patch subject: [PATCH v1 28/43] input: keypad: ep93xx: add DT support for Cirrus EP93xx
config: arm-randconfig-r046-20230531 (https://download.01.org/0day-ci/archive/20230601/202306012336.68kZBdn0-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 12.3.0
reproduce (this is a W=1 build):
mkdir -p ~/bin
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/79136093fef692a2db3c48c2d30e37310599131f
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Nikita-Shubin/dt-bindings-soc-Add-Cirrus-EP93xx/20230601-143415
git checkout 79136093fef692a2db3c48c2d30e37310599131f
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross W=1 O=build_dir ARCH=arm olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash drivers/input/keyboard/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306012336.68kZBdn0-lkp@intel.com/
All errors (new ones prefixed by >>):
drivers/input/keyboard/ep93xx_keypad.c: In function 'ep93xx_keypad_probe':
>> drivers/input/keyboard/ep93xx_keypad.c:262:9: error: implicit declaration of function 'of_property_read_u32' [-Werror=implicit-function-declaration]
262 | of_property_read_u32(np, "cirrus,debounce-delay-ms", &keypad->debounce);
| ^~~~~~~~~~~~~~~~~~~~
cc1: some warnings being treated as errors
vim +/of_property_read_u32 +262 drivers/input/keyboard/ep93xx_keypad.c
233
234 static DEFINE_SIMPLE_DEV_PM_OPS(ep93xx_keypad_pm_ops,
235 ep93xx_keypad_suspend, ep93xx_keypad_resume);
236
237 static int ep93xx_keypad_probe(struct platform_device *pdev)
238 {
239 struct device_node *np = pdev->dev.of_node;
240 struct ep93xx_keypad *keypad;
241 struct input_dev *input_dev;
242 int err;
243
244 keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad), GFP_KERNEL);
245 if (!keypad)
246 return -ENOMEM;
247
248 keypad->irq = platform_get_irq(pdev, 0);
249 if (keypad->irq < 0)
250 return keypad->irq;
251
252 keypad->mmio_base = devm_platform_ioremap_resource(pdev, 0);
253 if (IS_ERR(keypad->mmio_base))
254 return PTR_ERR(keypad->mmio_base);
255
256 keypad->clk = devm_clk_get(&pdev->dev, NULL);
257 if (IS_ERR(keypad->clk))
258 return PTR_ERR(keypad->clk);
259
260 keypad->flags = ep93xx_keypad_flags;
261
> 262 of_property_read_u32(np, "cirrus,debounce-delay-ms", &keypad->debounce);
263 of_property_read_u32(np, "cirrus,prescale", &keypad->prescale);
264
265 input_dev = devm_input_allocate_device(&pdev->dev);
266 if (!input_dev)
267 return -ENOMEM;
268
269 keypad->input_dev = input_dev;
270
271 input_dev->name = pdev->name;
272 input_dev->id.bustype = BUS_HOST;
273 input_dev->open = ep93xx_keypad_open;
274 input_dev->close = ep93xx_keypad_close;
275
276 err = matrix_keypad_build_keymap(NULL, NULL,
277 EP93XX_MATRIX_ROWS, EP93XX_MATRIX_COLS,
278 keypad->keycodes, input_dev);
279 if (err)
280 return err;
281
282 if (keypad->flags & EP93XX_KEYPAD_AUTOREPEAT)
283 __set_bit(EV_REP, input_dev->evbit);
284 input_set_drvdata(input_dev, keypad);
285
286 err = devm_request_irq(&pdev->dev, keypad->irq,
287 ep93xx_keypad_irq_handler,
288 0, pdev->name, keypad);
289 if (err)
290 return err;
291
292 err = input_register_device(input_dev);
293 if (err)
294 return err;
295
296 platform_set_drvdata(pdev, keypad);
297
298 device_init_wakeup(&pdev->dev, 1);
299 err = dev_pm_set_wake_irq(&pdev->dev, keypad->irq);
300 if (err)
301 dev_warn(&pdev->dev, "failed to set up wakeup irq: %d\n", err);
302
303 return 0;
304 }
305
Hi Nikita,
kernel test robot noticed the following build errors:
[auto build test ERROR on clk/clk-next]
[also build test ERROR on linusw-pinctrl/devel linusw-pinctrl/for-next linus/master v6.4-rc4 next-20230601]
[cannot apply to soc/for-next robh/for-next]
[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#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Nikita-Shubin/dt-bindings-soc-Add-Cirrus-EP93xx/20230601-143415
base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
patch link: https://lore.kernel.org/r/20230601054549.10843-10-nikita.shubin%40maquefel.me
patch subject: [PATCH v1 28/43] input: keypad: ep93xx: add DT support for Cirrus EP93xx
config: hexagon-randconfig-r045-20230531 (https://download.01.org/0day-ci/archive/20230601/202306012327.f8AIwhqv-lkp@intel.com/config)
compiler: clang version 15.0.4 (https://github.com/llvm/llvm-project 5c68a1cb123161b54b72ce90e7975d95a8eaf2a4)
reproduce (this is a W=1 build):
mkdir -p ~/bin
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/79136093fef692a2db3c48c2d30e37310599131f
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Nikita-Shubin/dt-bindings-soc-Add-Cirrus-EP93xx/20230601-143415
git checkout 79136093fef692a2db3c48c2d30e37310599131f
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=hexagon olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=hexagon SHELL=/bin/bash drivers/input/keyboard/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306012327.f8AIwhqv-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from drivers/input/keyboard/ep93xx_keypad.c:24:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:334:
include/asm-generic/io.h:547:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __raw_readb(PCI_IOBASE + addr);
~~~~~~~~~~ ^
include/asm-generic/io.h:560:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
#define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
^
In file included from drivers/input/keyboard/ep93xx_keypad.c:24:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:334:
include/asm-generic/io.h:573: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/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
#define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
^
In file included from drivers/input/keyboard/ep93xx_keypad.c:24:
In file included from include/linux/interrupt.h:11:
In file included from include/linux/hardirq.h:11:
In file included from ./arch/hexagon/include/generated/asm/hardirq.h:1:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:13:
In file included from arch/hexagon/include/asm/io.h:334:
include/asm-generic/io.h:584: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:594: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:604: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);
~~~~~~~~~~ ^
>> drivers/input/keyboard/ep93xx_keypad.c:262:2: error: call to undeclared function 'of_property_read_u32'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
of_property_read_u32(np, "cirrus,debounce-delay-ms", &keypad->debounce);
^
6 warnings and 1 error generated.
vim +/of_property_read_u32 +262 drivers/input/keyboard/ep93xx_keypad.c
233
234 static DEFINE_SIMPLE_DEV_PM_OPS(ep93xx_keypad_pm_ops,
235 ep93xx_keypad_suspend, ep93xx_keypad_resume);
236
237 static int ep93xx_keypad_probe(struct platform_device *pdev)
238 {
239 struct device_node *np = pdev->dev.of_node;
240 struct ep93xx_keypad *keypad;
241 struct input_dev *input_dev;
242 int err;
243
244 keypad = devm_kzalloc(&pdev->dev, sizeof(*keypad), GFP_KERNEL);
245 if (!keypad)
246 return -ENOMEM;
247
248 keypad->irq = platform_get_irq(pdev, 0);
249 if (keypad->irq < 0)
250 return keypad->irq;
251
252 keypad->mmio_base = devm_platform_ioremap_resource(pdev, 0);
253 if (IS_ERR(keypad->mmio_base))
254 return PTR_ERR(keypad->mmio_base);
255
256 keypad->clk = devm_clk_get(&pdev->dev, NULL);
257 if (IS_ERR(keypad->clk))
258 return PTR_ERR(keypad->clk);
259
260 keypad->flags = ep93xx_keypad_flags;
261
> 262 of_property_read_u32(np, "cirrus,debounce-delay-ms", &keypad->debounce);
263 of_property_read_u32(np, "cirrus,prescale", &keypad->prescale);
264
265 input_dev = devm_input_allocate_device(&pdev->dev);
266 if (!input_dev)
267 return -ENOMEM;
268
269 keypad->input_dev = input_dev;
270
271 input_dev->name = pdev->name;
272 input_dev->id.bustype = BUS_HOST;
273 input_dev->open = ep93xx_keypad_open;
274 input_dev->close = ep93xx_keypad_close;
275
276 err = matrix_keypad_build_keymap(NULL, NULL,
277 EP93XX_MATRIX_ROWS, EP93XX_MATRIX_COLS,
278 keypad->keycodes, input_dev);
279 if (err)
280 return err;
281
282 if (keypad->flags & EP93XX_KEYPAD_AUTOREPEAT)
283 __set_bit(EV_REP, input_dev->evbit);
284 input_set_drvdata(input_dev, keypad);
285
286 err = devm_request_irq(&pdev->dev, keypad->irq,
287 ep93xx_keypad_irq_handler,
288 0, pdev->name, keypad);
289 if (err)
290 return err;
291
292 err = input_register_device(input_dev);
293 if (err)
294 return err;
295
296 platform_set_drvdata(pdev, keypad);
297
298 device_init_wakeup(&pdev->dev, 1);
299 err = dev_pm_set_wake_irq(&pdev->dev, keypad->irq);
300 if (err)
301 dev_warn(&pdev->dev, "failed to set up wakeup irq: %d\n", err);
302
303 return 0;
304 }
305
On Thu, Jun 01, 2023 at 08:45:33AM +0300, Nikita Shubin wrote: > - get keymap from the device tree > - find register range from the device tree > - get interrupts from device tree ... > +/* flags for the ep93xx_keypad driver */ > +#define EP93XX_KEYPAD_DISABLE_3_KEY (1<<0) /* disable 3-key reset */ > +#define EP93XX_KEYPAD_DIAG_MODE (1<<1) /* diagnostic mode */ > +#define EP93XX_KEYPAD_BACK_DRIVE (1<<2) /* back driving mode */ > +#define EP93XX_KEYPAD_TEST_MODE (1<<3) /* scan only column 0 */ > +#define EP93XX_KEYPAD_AUTOREPEAT (1<<4) /* enable key autorepeat */ > +static int ep93xx_keypad_flags; > +module_param(ep93xx_keypad_flags, int, 0); > +MODULE_PARM_DESC(ep93xx_keypad_flags, "EP93XX keypad flags."); Why? This pretty much looks like missing DT description. Please, write your commit message better, so we can understand the point of such decisions w/o asking.
Hello Andy! On Thu, 2023-06-01 at 19:54 +0300, Andy Shevchenko wrote: > On Thu, Jun 01, 2023 at 08:45:33AM +0300, Nikita Shubin wrote: > > - get keymap from the device tree > > - find register range from the device tree > > - get interrupts from device tree > > ... > > > +/* flags for the ep93xx_keypad driver */ > > +#define EP93XX_KEYPAD_DISABLE_3_KEY (1<<0) /* disable 3-key > > reset */ > > +#define EP93XX_KEYPAD_DIAG_MODE (1<<1) /* > > diagnostic mode */ > > +#define EP93XX_KEYPAD_BACK_DRIVE (1<<2) /* back driving > > mode */ > > +#define EP93XX_KEYPAD_TEST_MODE (1<<3) /* scan > > only column 0 */ > > +#define EP93XX_KEYPAD_AUTOREPEAT (1<<4) /* enable key > > autorepeat */ > > > +static int ep93xx_keypad_flags; > > +module_param(ep93xx_keypad_flags, int, 0); > > +MODULE_PARM_DESC(ep93xx_keypad_flags, "EP93XX keypad flags."); > > Why? This pretty much looks like missing DT description. From other patches from this series, i learned NOT to add new DT entities, not even with vendor prefix, no way! May be i missing something of course... Either way https://elixir.bootlin.com/linux/v6.4-rc4/source/arch/arm/mach-ep93xx/core.c#L577 static struct ep93xx_keypad_platform_data ep93xx_keypad_data; Was never used in different ways than initializing all to zeroes including flags since 2.6 (didn't look before through), so i would prefer to drop this completely than moving it into device tree. May we should drop ep93xx_keypad entirely, i don't have hardware to test it anyway, neither does Alexander. > > Please, write your commit message better, so we can understand the > point of > such decisions w/o asking. >
On Sun, Jun 04, 2023 at 10:14:52PM +0300, Nikita Shubin wrote: > On Thu, 2023-06-01 at 19:54 +0300, Andy Shevchenko wrote: > > On Thu, Jun 01, 2023 at 08:45:33AM +0300, Nikita Shubin wrote: ... > > > +static int ep93xx_keypad_flags; > > > +module_param(ep93xx_keypad_flags, int, 0); > > > +MODULE_PARM_DESC(ep93xx_keypad_flags, "EP93XX keypad flags."); > > > > Why? This pretty much looks like missing DT description. > > From other patches from this series, i learned NOT to add new DT > entities, not even with vendor prefix, no way! > > May be i missing something of course... We do not add module parameters to a new code either. So this will be a dead end. ... > Was never used in different ways than initializing all to zeroes > including flags since 2.6 (didn't look before through), so i would > prefer to drop this completely than moving it into device tree. This sounds the best!
On Thu, Jun 01, 2023 at 08:45:33AM +0300, Nikita Shubin wrote: > - get keymap from the device tree > - find register range from the device tree > - get interrupts from device tree > > Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me> > --- > > Notes: > v0 -> v1: > > - fixed header > - dropped coma in id table > - take debounce, prescale from dt > - remove ep93xx_keypad_platform_data > - move flags to module params > - drop setting clock rate, it's useless, as was never used, > it seems we are okay with default clk rate used > - move usefull defines from platform file here > - drop platform header > > drivers/input/keyboard/ep93xx_keypad.c | 78 +++++++++++++------------- > 1 file changed, 40 insertions(+), 38 deletions(-) > > diff --git a/drivers/input/keyboard/ep93xx_keypad.c b/drivers/input/keyboard/ep93xx_keypad.c > index 55075addcac2..8b0e73f56216 100644 > --- a/drivers/input/keyboard/ep93xx_keypad.c > +++ b/drivers/input/keyboard/ep93xx_keypad.c > @@ -20,6 +20,7 @@ > #include <linux/bits.h> > #include <linux/module.h> > #include <linux/platform_device.h> > +#include <linux/mod_devicetable.h> > #include <linux/interrupt.h> > #include <linux/clk.h> > #include <linux/io.h> > @@ -27,7 +28,6 @@ > #include <linux/input/matrix_keypad.h> > #include <linux/slab.h> > #include <linux/soc/cirrus/ep93xx.h> > -#include <linux/platform_data/keypad-ep93xx.h> > #include <linux/pm_wakeirq.h> > > /* > @@ -61,12 +61,18 @@ > #define KEY_REG_KEY1_MASK GENMASK(5, 0) > #define KEY_REG_KEY1_SHIFT 0 > > +#define EP93XX_MATRIX_ROWS (8) > +#define EP93XX_MATRIX_COLS (8) > + > #define EP93XX_MATRIX_SIZE (EP93XX_MATRIX_ROWS * EP93XX_MATRIX_COLS) > > struct ep93xx_keypad { > - struct ep93xx_keypad_platform_data *pdata; > struct input_dev *input_dev; > struct clk *clk; > + unsigned int debounce; > + unsigned int prescale; > + unsigned int flags; > + unsigned int clk_rate; > > void __iomem *mmio_base; > > @@ -80,6 +86,17 @@ struct ep93xx_keypad { > bool enabled; > }; > > +/* flags for the ep93xx_keypad driver */ > +#define EP93XX_KEYPAD_DISABLE_3_KEY (1<<0) /* disable 3-key reset */ > +#define EP93XX_KEYPAD_DIAG_MODE (1<<1) /* diagnostic mode */ > +#define EP93XX_KEYPAD_BACK_DRIVE (1<<2) /* back driving mode */ > +#define EP93XX_KEYPAD_TEST_MODE (1<<3) /* scan only column 0 */ > +#define EP93XX_KEYPAD_AUTOREPEAT (1<<4) /* enable key autorepeat */ > + > +static int ep93xx_keypad_flags; > +module_param(ep93xx_keypad_flags, int, 0); > +MODULE_PARM_DESC(ep93xx_keypad_flags, "EP93XX keypad flags."); > + > static irqreturn_t ep93xx_keypad_irq_handler(int irq, void *dev_id) > { > struct ep93xx_keypad *keypad = dev_id; > @@ -133,23 +150,20 @@ static irqreturn_t ep93xx_keypad_irq_handler(int irq, void *dev_id) > > static void ep93xx_keypad_config(struct ep93xx_keypad *keypad) > { > - struct ep93xx_keypad_platform_data *pdata = keypad->pdata; > unsigned int val = 0; > > - clk_set_rate(keypad->clk, pdata->clk_rate); > - > - if (pdata->flags & EP93XX_KEYPAD_DISABLE_3_KEY) > + if (keypad->flags & EP93XX_KEYPAD_DISABLE_3_KEY) > val |= KEY_INIT_DIS3KY; > - if (pdata->flags & EP93XX_KEYPAD_DIAG_MODE) > + if (keypad->flags & EP93XX_KEYPAD_DIAG_MODE) > val |= KEY_INIT_DIAG; > - if (pdata->flags & EP93XX_KEYPAD_BACK_DRIVE) > + if (keypad->flags & EP93XX_KEYPAD_BACK_DRIVE) > val |= KEY_INIT_BACK; > - if (pdata->flags & EP93XX_KEYPAD_TEST_MODE) > + if (keypad->flags & EP93XX_KEYPAD_TEST_MODE) > val |= KEY_INIT_T2; > > - val |= ((pdata->debounce << KEY_INIT_DBNC_SHIFT) & KEY_INIT_DBNC_MASK); > + val |= ((keypad->debounce << KEY_INIT_DBNC_SHIFT) & KEY_INIT_DBNC_MASK); > > - val |= ((pdata->prescale << KEY_INIT_PRSCL_SHIFT) & KEY_INIT_PRSCL_MASK); > + val |= ((keypad->prescale << KEY_INIT_PRSCL_SHIFT) & KEY_INIT_PRSCL_MASK); > > __raw_writel(val, keypad->mmio_base + KEY_INIT); > } > @@ -220,17 +234,10 @@ static int ep93xx_keypad_resume(struct device *dev) > static DEFINE_SIMPLE_DEV_PM_OPS(ep93xx_keypad_pm_ops, > ep93xx_keypad_suspend, ep93xx_keypad_resume); > > -static void ep93xx_keypad_release_gpio_action(void *_pdev) > -{ > - struct platform_device *pdev = _pdev; > - > - ep93xx_keypad_release_gpio(pdev); > -} > - > static int ep93xx_keypad_probe(struct platform_device *pdev) > { > + struct device_node *np = pdev->dev.of_node; > struct ep93xx_keypad *keypad; > - const struct matrix_keymap_data *keymap_data; > struct input_dev *input_dev; > int err; > > @@ -238,14 +245,6 @@ static int ep93xx_keypad_probe(struct platform_device *pdev) > if (!keypad) > return -ENOMEM; > > - keypad->pdata = dev_get_platdata(&pdev->dev); > - if (!keypad->pdata) > - return -EINVAL; > - > - keymap_data = keypad->pdata->keymap_data; > - if (!keymap_data) > - return -EINVAL; > - > keypad->irq = platform_get_irq(pdev, 0); > if (keypad->irq < 0) > return keypad->irq; > @@ -254,19 +253,15 @@ static int ep93xx_keypad_probe(struct platform_device *pdev) > if (IS_ERR(keypad->mmio_base)) > return PTR_ERR(keypad->mmio_base); > > - err = ep93xx_keypad_acquire_gpio(pdev); > - if (err) > - return err; > - > - err = devm_add_action_or_reset(&pdev->dev, > - ep93xx_keypad_release_gpio_action, pdev); > - if (err) > - return err; > - > keypad->clk = devm_clk_get(&pdev->dev, NULL); > if (IS_ERR(keypad->clk)) > return PTR_ERR(keypad->clk); > > + keypad->flags = ep93xx_keypad_flags; > + > + of_property_read_u32(np, "cirrus,debounce-delay-ms", &keypad->debounce); > + of_property_read_u32(np, "cirrus,prescale", &keypad->prescale); Please use device_property_read_*() API for this. > + > input_dev = devm_input_allocate_device(&pdev->dev); > if (!input_dev) > return -ENOMEM; > @@ -278,13 +273,13 @@ static int ep93xx_keypad_probe(struct platform_device *pdev) > input_dev->open = ep93xx_keypad_open; > input_dev->close = ep93xx_keypad_close; > > - err = matrix_keypad_build_keymap(keymap_data, NULL, > + err = matrix_keypad_build_keymap(NULL, NULL, > EP93XX_MATRIX_ROWS, EP93XX_MATRIX_COLS, > keypad->keycodes, input_dev); > if (err) > return err; > > - if (keypad->pdata->flags & EP93XX_KEYPAD_AUTOREPEAT) > + if (keypad->flags & EP93XX_KEYPAD_AUTOREPEAT) > __set_bit(EV_REP, input_dev->evbit); I think this should be controlled by "autorepeat" device property. > input_set_drvdata(input_dev, keypad); > > @@ -315,10 +310,17 @@ static int ep93xx_keypad_remove(struct platform_device *pdev) > return 0; > } > > +static const struct of_device_id ep93xx_keypad_of_ids[] = { > + { .compatible = "cirrus,ep9307-keypad" }, > + { /* sentinel */ } > +}; > +MODULE_DEVICE_TABLE(of, ep93xx_keypad_of_ids); > + > static struct platform_driver ep93xx_keypad_driver = { > .driver = { > .name = "ep93xx-keypad", > .pm = pm_sleep_ptr(&ep93xx_keypad_pm_ops), > + .of_match_table = ep93xx_keypad_of_ids, > }, > .probe = ep93xx_keypad_probe, > .remove = ep93xx_keypad_remove, > -- > 2.37.4 > Thanks.
diff --git a/drivers/input/keyboard/ep93xx_keypad.c b/drivers/input/keyboard/ep93xx_keypad.c index 55075addcac2..8b0e73f56216 100644 --- a/drivers/input/keyboard/ep93xx_keypad.c +++ b/drivers/input/keyboard/ep93xx_keypad.c @@ -20,6 +20,7 @@ #include <linux/bits.h> #include <linux/module.h> #include <linux/platform_device.h> +#include <linux/mod_devicetable.h> #include <linux/interrupt.h> #include <linux/clk.h> #include <linux/io.h> @@ -27,7 +28,6 @@ #include <linux/input/matrix_keypad.h> #include <linux/slab.h> #include <linux/soc/cirrus/ep93xx.h> -#include <linux/platform_data/keypad-ep93xx.h> #include <linux/pm_wakeirq.h> /* @@ -61,12 +61,18 @@ #define KEY_REG_KEY1_MASK GENMASK(5, 0) #define KEY_REG_KEY1_SHIFT 0 +#define EP93XX_MATRIX_ROWS (8) +#define EP93XX_MATRIX_COLS (8) + #define EP93XX_MATRIX_SIZE (EP93XX_MATRIX_ROWS * EP93XX_MATRIX_COLS) struct ep93xx_keypad { - struct ep93xx_keypad_platform_data *pdata; struct input_dev *input_dev; struct clk *clk; + unsigned int debounce; + unsigned int prescale; + unsigned int flags; + unsigned int clk_rate; void __iomem *mmio_base; @@ -80,6 +86,17 @@ struct ep93xx_keypad { bool enabled; }; +/* flags for the ep93xx_keypad driver */ +#define EP93XX_KEYPAD_DISABLE_3_KEY (1<<0) /* disable 3-key reset */ +#define EP93XX_KEYPAD_DIAG_MODE (1<<1) /* diagnostic mode */ +#define EP93XX_KEYPAD_BACK_DRIVE (1<<2) /* back driving mode */ +#define EP93XX_KEYPAD_TEST_MODE (1<<3) /* scan only column 0 */ +#define EP93XX_KEYPAD_AUTOREPEAT (1<<4) /* enable key autorepeat */ + +static int ep93xx_keypad_flags; +module_param(ep93xx_keypad_flags, int, 0); +MODULE_PARM_DESC(ep93xx_keypad_flags, "EP93XX keypad flags."); + static irqreturn_t ep93xx_keypad_irq_handler(int irq, void *dev_id) { struct ep93xx_keypad *keypad = dev_id; @@ -133,23 +150,20 @@ static irqreturn_t ep93xx_keypad_irq_handler(int irq, void *dev_id) static void ep93xx_keypad_config(struct ep93xx_keypad *keypad) { - struct ep93xx_keypad_platform_data *pdata = keypad->pdata; unsigned int val = 0; - clk_set_rate(keypad->clk, pdata->clk_rate); - - if (pdata->flags & EP93XX_KEYPAD_DISABLE_3_KEY) + if (keypad->flags & EP93XX_KEYPAD_DISABLE_3_KEY) val |= KEY_INIT_DIS3KY; - if (pdata->flags & EP93XX_KEYPAD_DIAG_MODE) + if (keypad->flags & EP93XX_KEYPAD_DIAG_MODE) val |= KEY_INIT_DIAG; - if (pdata->flags & EP93XX_KEYPAD_BACK_DRIVE) + if (keypad->flags & EP93XX_KEYPAD_BACK_DRIVE) val |= KEY_INIT_BACK; - if (pdata->flags & EP93XX_KEYPAD_TEST_MODE) + if (keypad->flags & EP93XX_KEYPAD_TEST_MODE) val |= KEY_INIT_T2; - val |= ((pdata->debounce << KEY_INIT_DBNC_SHIFT) & KEY_INIT_DBNC_MASK); + val |= ((keypad->debounce << KEY_INIT_DBNC_SHIFT) & KEY_INIT_DBNC_MASK); - val |= ((pdata->prescale << KEY_INIT_PRSCL_SHIFT) & KEY_INIT_PRSCL_MASK); + val |= ((keypad->prescale << KEY_INIT_PRSCL_SHIFT) & KEY_INIT_PRSCL_MASK); __raw_writel(val, keypad->mmio_base + KEY_INIT); } @@ -220,17 +234,10 @@ static int ep93xx_keypad_resume(struct device *dev) static DEFINE_SIMPLE_DEV_PM_OPS(ep93xx_keypad_pm_ops, ep93xx_keypad_suspend, ep93xx_keypad_resume); -static void ep93xx_keypad_release_gpio_action(void *_pdev) -{ - struct platform_device *pdev = _pdev; - - ep93xx_keypad_release_gpio(pdev); -} - static int ep93xx_keypad_probe(struct platform_device *pdev) { + struct device_node *np = pdev->dev.of_node; struct ep93xx_keypad *keypad; - const struct matrix_keymap_data *keymap_data; struct input_dev *input_dev; int err; @@ -238,14 +245,6 @@ static int ep93xx_keypad_probe(struct platform_device *pdev) if (!keypad) return -ENOMEM; - keypad->pdata = dev_get_platdata(&pdev->dev); - if (!keypad->pdata) - return -EINVAL; - - keymap_data = keypad->pdata->keymap_data; - if (!keymap_data) - return -EINVAL; - keypad->irq = platform_get_irq(pdev, 0); if (keypad->irq < 0) return keypad->irq; @@ -254,19 +253,15 @@ static int ep93xx_keypad_probe(struct platform_device *pdev) if (IS_ERR(keypad->mmio_base)) return PTR_ERR(keypad->mmio_base); - err = ep93xx_keypad_acquire_gpio(pdev); - if (err) - return err; - - err = devm_add_action_or_reset(&pdev->dev, - ep93xx_keypad_release_gpio_action, pdev); - if (err) - return err; - keypad->clk = devm_clk_get(&pdev->dev, NULL); if (IS_ERR(keypad->clk)) return PTR_ERR(keypad->clk); + keypad->flags = ep93xx_keypad_flags; + + of_property_read_u32(np, "cirrus,debounce-delay-ms", &keypad->debounce); + of_property_read_u32(np, "cirrus,prescale", &keypad->prescale); + input_dev = devm_input_allocate_device(&pdev->dev); if (!input_dev) return -ENOMEM; @@ -278,13 +273,13 @@ static int ep93xx_keypad_probe(struct platform_device *pdev) input_dev->open = ep93xx_keypad_open; input_dev->close = ep93xx_keypad_close; - err = matrix_keypad_build_keymap(keymap_data, NULL, + err = matrix_keypad_build_keymap(NULL, NULL, EP93XX_MATRIX_ROWS, EP93XX_MATRIX_COLS, keypad->keycodes, input_dev); if (err) return err; - if (keypad->pdata->flags & EP93XX_KEYPAD_AUTOREPEAT) + if (keypad->flags & EP93XX_KEYPAD_AUTOREPEAT) __set_bit(EV_REP, input_dev->evbit); input_set_drvdata(input_dev, keypad); @@ -315,10 +310,17 @@ static int ep93xx_keypad_remove(struct platform_device *pdev) return 0; } +static const struct of_device_id ep93xx_keypad_of_ids[] = { + { .compatible = "cirrus,ep9307-keypad" }, + { /* sentinel */ } +}; +MODULE_DEVICE_TABLE(of, ep93xx_keypad_of_ids); + static struct platform_driver ep93xx_keypad_driver = { .driver = { .name = "ep93xx-keypad", .pm = pm_sleep_ptr(&ep93xx_keypad_pm_ops), + .of_match_table = ep93xx_keypad_of_ids, }, .probe = ep93xx_keypad_probe, .remove = ep93xx_keypad_remove,
- get keymap from the device tree - find register range from the device tree - get interrupts from device tree Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me> --- Notes: v0 -> v1: - fixed header - dropped coma in id table - take debounce, prescale from dt - remove ep93xx_keypad_platform_data - move flags to module params - drop setting clock rate, it's useless, as was never used, it seems we are okay with default clk rate used - move usefull defines from platform file here - drop platform header drivers/input/keyboard/ep93xx_keypad.c | 78 +++++++++++++------------- 1 file changed, 40 insertions(+), 38 deletions(-)