diff mbox series

[v1,28/43] input: keypad: ep93xx: add DT support for Cirrus EP93xx

Message ID 20230601054549.10843-10-nikita.shubin@maquefel.me
State New
Headers show
Series None | expand

Commit Message

Nikita Shubin June 1, 2023, 5:45 a.m. UTC
- 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(-)

Comments

kernel test robot June 1, 2023, 3:31 p.m. UTC | #1
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
Andy Shevchenko June 1, 2023, 4:54 p.m. UTC | #2
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.
Nikita Shubin June 4, 2023, 7:14 p.m. UTC | #3
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.
diff mbox series

Patch

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,