Re: [PATCH 00/43] ep93xx device tree conversion

Re: [PATCH 00/43] ep93xx device tree conversion

[Arnd Bergmann]

On Mon, Apr 24, 2023, at 14:34, Nikita Shubin wrote:
> This series aims to convert ep93xx from platform to full device tree support.
>
> Tested on ts7250 64 RAM/128 MiB Nand flash, edb9302.
>
> Thank you Linus and Arnd for your support, review and comments, sorry 
> if i missed something -
> these series are quite big for me.
>
> Big thanks to Alexander Sverdlin for his testing, support, review, 
> fixes and patches.

Thanks a lot for your continued work. I can't merge any of this at
the moment since the upstream merge window just opened, but I'm
happy to take this all through the soc tree for 6.5, provided we
get the sufficient Acks from the subsystem maintainers. Merging
it through each individual tree would take a lot longer, so I
hope we can avoid that.

      Arnd

[PATCH 00/43] ep93xx device tree conversion

[Nikita Shubin]

This series aims to convert ep93xx from platform to full device tree support.

Tested on ts7250 64 RAM/128 MiB Nand flash, edb9302.

Thank you Linus and Arnd for your support, review and comments, sorry if i missed something -
these series are quite big for me.

Big thanks to Alexander Sverdlin for his testing, support, review, fixes and patches.

Alexander Sverdlin (4):
  ARM: dts: ep93xx: Add ADC node
  ARM: dts: ep93xx: Add I2S and AC97 nodes
  ARM: dts: ep93xx: Add EDB9302 DT
  ASoC: cirrus: edb93xx: Delete driver

Nikita Shubin (39):
  gpio: ep93xx: split device in multiple
  soc: Add SoC driver for Cirrus ep93xx
  dt-bindings: pinctrl: Add DT bindings ep93xx pinctrl
  pinctrl: add a Cirrus ep93xx SoC pin controller
  dt-bindings: timers: add DT bindings for Cirrus EP93xx
  clocksource: ep93xx: Add driver for Cirrus Logic EP93xx
  dt-bindings: rtc: add DT bindings for Cirrus EP93xx
  rtc: ep93xx: add DT support for Cirrus EP93xx
  dt-bindings: watchdog: add DT bindings for Cirrus EP93x
  watchdog: ep93xx: add DT support for Cirrus EP93xx
  dt-bindings: clock: add DT bindings for Cirrus EP93xx
  clk: ep93xx: add DT support for Cirrus EP93xx
  power: reset: Add a driver for the ep93xx reset
  dt-bindings: pwm: Add DT bindings ep93xx PWM
  pwm: ep93xx: add DT support for Cirrus EP93xx
  dt-bindings: spi: Add DT bindings ep93xx spi
  spi: ep93xx: add DT support for Cirrus EP93xx
  dt-bindings: net: Add DT bindings ep93xx eth
  net: cirrus: add DT support for Cirrus EP93xx
  dt-bindings: dma: Add DT bindings ep93xx dma
  dma: cirrus: add DT support for Cirrus EP93xx
  dt-bindings: mtd: add DT bindings for ts7250 nand
  mtd: ts72xx_nand: add platform helper
  dt-bindings: ata: Add DT bindings ep93xx pata
  pata: cirrus: add DT support for Cirrus EP93xx
  dt-bindings: input: Add DT bindings ep93xx keypad
  input: keypad: ep93xx: add DT support for Cirrus EP93xx
  dt-bindings: rtc: Add DT binding m48t86 rtc
  rtc: m48t86: add DT support for m48t86
  dt-bindings: wdt: Add DT binding ts72xx wdt
  wdt: ts72xx: add DT support for ts72xx
  dt-bindings: gpio: Add DT bindings ep93xx gpio
  gpio: ep93xx: add DT support for gpio-ep93xx
  ARM: dts: add device tree for ep93xx Soc
  ARM: ep93xx: DT for the Cirrus ep93xx SoC platforms
  pwm: ep93xx: drop legacy pinctrl
  input: keypad: ep93xx: drop legacy pinctrl
  ARM: ep93xx: soc: drop defines
  ARM: ep93xx: delete all boardfiles

 .../devicetree/bindings/arm/ep93xx.yaml       |   99 +
 .../bindings/ata/cirrus,ep93xx-pata.yaml      |   40 +
 .../bindings/dma/cirrus,ep93xx-dma-m2m.yaml   |   66 +
 .../bindings/dma/cirrus,ep93xx-dma-m2p.yaml   |  102 +
 .../devicetree/bindings/gpio/gpio-ep93xx.yaml |  161 ++
 .../bindings/input/cirrus,ep93xx-keypad.yaml  |  123 ++
 .../bindings/mtd/technologic,nand.yaml        |   56 +
 .../bindings/net/cirrus,ep93xx_eth.yaml       |   51 +
 .../pinctrl/cirrus,ep93xx-pinctrl.yaml        |   66 +
 .../bindings/pwm/cirrus,ep93xx-pwm.yaml       |   45 +
 .../bindings/rtc/cirrus,ep93xx-rtc.yaml       |   32 +
 .../bindings/rtc/dallas,rtc-m48t86.yaml       |   33 +
 .../devicetree/bindings/spi/spi-ep93xx.yaml   |   68 +
 .../bindings/timer/cirrus,ep93xx-timer.yaml   |   41 +
 .../bindings/watchdog/cirrus,ep93xx-wdt.yaml  |   38 +
 .../watchdog/technologic,ts72xx-wdt.yaml      |   39 +
 arch/arm/Makefile                             |    1 -
 arch/arm/boot/dts/Makefile                    |    1 +
 arch/arm/boot/dts/ep93xx-bk3.dts              |   96 +
 arch/arm/boot/dts/ep93xx-edb9302.dts          |  150 ++
 arch/arm/boot/dts/ep93xx-ts7250.dts           |  113 ++
 arch/arm/boot/dts/ep93xx.dtsi                 |  466 +++++
 arch/arm/mach-ep93xx/Kconfig                  |   20 +-
 arch/arm/mach-ep93xx/Makefile                 |   11 -
 arch/arm/mach-ep93xx/core.c                   | 1017 ----------
 arch/arm/mach-ep93xx/dma.c                    |  114 --
 arch/arm/mach-ep93xx/edb93xx.c                |  344 ----
 arch/arm/mach-ep93xx/ep93xx-regs.h            |   38 -
 arch/arm/mach-ep93xx/gpio-ep93xx.h            |  111 --
 arch/arm/mach-ep93xx/hardware.h               |   25 -
 arch/arm/mach-ep93xx/irqs.h                   |   76 -
 arch/arm/mach-ep93xx/platform.h               |   42 -
 arch/arm/mach-ep93xx/soc.h                    |  212 --
 arch/arm/mach-ep93xx/ts72xx.c                 |  422 ----
 arch/arm/mach-ep93xx/ts72xx.h                 |   94 -
 arch/arm/mach-ep93xx/vision_ep9307.c          |  311 ---
 drivers/ata/pata_ep93xx.c                     |    9 +
 drivers/clk/Kconfig                           |    8 +
 drivers/clk/Makefile                          |    1 +
 .../clock.c => drivers/clk/clk-ep93xx.c       |  491 +++--
 drivers/clocksource/Kconfig                   |   11 +
 drivers/clocksource/Makefile                  |    1 +
 .../clocksource}/timer-ep93xx.c               |  143 +-
 drivers/dma/ep93xx_dma.c                      |  119 +-
 drivers/gpio/gpio-ep93xx.c                    |  329 ++--
 drivers/input/keyboard/ep93xx_keypad.c        |   25 +-
 drivers/mtd/nand/raw/Kconfig                  |    8 +
 drivers/mtd/nand/raw/Makefile                 |    1 +
 drivers/mtd/nand/raw/ts72xx_nand.c            |   94 +
 drivers/net/ethernet/cirrus/ep93xx_eth.c      |   49 +-
 drivers/pinctrl/Kconfig                       |    7 +
 drivers/pinctrl/Makefile                      |    1 +
 drivers/pinctrl/pinctrl-ep93xx.c              | 1698 +++++++++++++++++
 drivers/power/reset/Kconfig                   |   10 +
 drivers/power/reset/Makefile                  |    1 +
 drivers/power/reset/ep93xx-restart.c          |   65 +
 drivers/pwm/pwm-ep93xx.c                      |   24 +-
 drivers/rtc/rtc-ep93xx.c                      |    8 +
 drivers/rtc/rtc-m48t86.c                      |   10 +
 drivers/soc/Kconfig                           |    1 +
 drivers/soc/Makefile                          |    1 +
 drivers/soc/cirrus/Kconfig                    |   11 +
 drivers/soc/cirrus/Makefile                   |    2 +
 drivers/soc/cirrus/soc-ep93xx.c               |  134 ++
 drivers/spi/spi-ep93xx.c                      |   31 +-
 drivers/watchdog/ep93xx_wdt.c                 |    8 +
 drivers/watchdog/ts72xx_wdt.c                 |    8 +
 .../dt-bindings/clock/cirrus,ep93xx-clock.h   |   53 +
 include/linux/platform_data/dma-ep93xx.h      |    3 +
 include/linux/soc/cirrus/ep93xx.h             |   28 +-
 sound/soc/cirrus/Kconfig                      |    9 -
 sound/soc/cirrus/Makefile                     |    4 -
 sound/soc/cirrus/edb93xx.c                    |  119 --
 73 files changed, 4796 insertions(+), 3453 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/ep93xx.yaml
 create mode 100644 Documentation/devicetree/bindings/ata/cirrus,ep93xx-pata.yaml
 create mode 100644 Documentation/devicetree/bindings/dma/cirrus,ep93xx-dma-m2m.yaml
 create mode 100644 Documentation/devicetree/bindings/dma/cirrus,ep93xx-dma-m2p.yaml
 create mode 100644 Documentation/devicetree/bindings/gpio/gpio-ep93xx.yaml
 create mode 100644 Documentation/devicetree/bindings/input/cirrus,ep93xx-keypad.yaml
 create mode 100644 Documentation/devicetree/bindings/mtd/technologic,nand.yaml
 create mode 100644 Documentation/devicetree/bindings/net/cirrus,ep93xx_eth.yaml
 create mode 100644 Documentation/devicetree/bindings/pinctrl/cirrus,ep93xx-pinctrl.yaml
 create mode 100644 Documentation/devicetree/bindings/pwm/cirrus,ep93xx-pwm.yaml
 create mode 100644 Documentation/devicetree/bindings/rtc/cirrus,ep93xx-rtc.yaml
 create mode 100644 Documentation/devicetree/bindings/rtc/dallas,rtc-m48t86.yaml
 create mode 100644 Documentation/devicetree/bindings/spi/spi-ep93xx.yaml
 create mode 100644 Documentation/devicetree/bindings/timer/cirrus,ep93xx-timer.yaml
 create mode 100644 Documentation/devicetree/bindings/watchdog/cirrus,ep93xx-wdt.yaml
 create mode 100644 Documentation/devicetree/bindings/watchdog/technologic,ts72xx-wdt.yaml
 create mode 100644 arch/arm/boot/dts/ep93xx-bk3.dts
 create mode 100644 arch/arm/boot/dts/ep93xx-edb9302.dts
 create mode 100644 arch/arm/boot/dts/ep93xx-ts7250.dts
 create mode 100644 arch/arm/boot/dts/ep93xx.dtsi
 delete mode 100644 arch/arm/mach-ep93xx/Makefile
 delete mode 100644 arch/arm/mach-ep93xx/core.c
 delete mode 100644 arch/arm/mach-ep93xx/dma.c
 delete mode 100644 arch/arm/mach-ep93xx/edb93xx.c
 delete mode 100644 arch/arm/mach-ep93xx/ep93xx-regs.h
 delete mode 100644 arch/arm/mach-ep93xx/gpio-ep93xx.h
 delete mode 100644 arch/arm/mach-ep93xx/hardware.h
 delete mode 100644 arch/arm/mach-ep93xx/irqs.h
 delete mode 100644 arch/arm/mach-ep93xx/platform.h
 delete mode 100644 arch/arm/mach-ep93xx/soc.h
 delete mode 100644 arch/arm/mach-ep93xx/ts72xx.c
 delete mode 100644 arch/arm/mach-ep93xx/ts72xx.h
 delete mode 100644 arch/arm/mach-ep93xx/vision_ep9307.c
 rename arch/arm/mach-ep93xx/clock.c => drivers/clk/clk-ep93xx.c (60%)
 rename {arch/arm/mach-ep93xx => drivers/clocksource}/timer-ep93xx.c (51%)
 create mode 100644 drivers/mtd/nand/raw/ts72xx_nand.c
 create mode 100644 drivers/pinctrl/pinctrl-ep93xx.c
 create mode 100644 drivers/power/reset/ep93xx-restart.c
 create mode 100644 drivers/soc/cirrus/Kconfig
 create mode 100644 drivers/soc/cirrus/Makefile
 create mode 100644 drivers/soc/cirrus/soc-ep93xx.c
 create mode 100644 include/dt-bindings/clock/cirrus,ep93xx-clock.h
 delete mode 100644 sound/soc/cirrus/edb93xx.c

-- 
2.39.2

[PATCH 26/43] dt-bindings: input: Add DT bindings ep93xx keypad

[Nikita Shubin]

Add YAML bindings ep93xx SoC.

Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---

Notes:
    Linus Walleij:
    - replaced hex with proper <KEY_UP>, etc

 .../bindings/input/cirrus,ep93xx-keypad.yaml  | 123 ++++++++++++++++++
 1 file changed, 123 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/cirrus,ep93xx-keypad.yaml

diff --git a/Documentation/devicetree/bindings/input/cirrus,ep93xx-keypad.yaml b/Documentation/devicetree/bindings/input/cirrus,ep93xx-keypad.yaml
new file mode 100644
index 000000000000..0310114de22e
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/cirrus,ep93xx-keypad.yaml
@@ -0,0 +1,123 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/cirrus,ep93xx-keypad.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cirrus ep93xx keypad
+
+maintainers:
+  - Dmitry Torokhov <dmitry.torokhov@gmail.com>
+  - Alexander Sverdlin <alexander.sverdlin@gmail.com>
+
+allOf:
+  - $ref: "/schemas/input/matrix-keymap.yaml#"
+
+description: |
+  The KPP is designed to interface with a keypad matrix with 2-point contact
+  or 3-point contact keys. The KPP is designed to simplify the software task
+  of scanning a keypad matrix. The KPP is capable of detecting, debouncing,
+  and decoding one or multiple keys pressed simultaneously on a keypad.
+
+properties:
+  compatible:
+    enum:
+      - cirrus,ep9301-keypad
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    enum:
+      - ep93xx-keypad
+
+  debounce:
+    description: |
+          Time in microseconds that key must be pressed or
+          released for state change interrupt to trigger.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  prescale:
+    description: row/column counter pre-scaler load value
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  clk-rate:
+    description: clock rate setting
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  disable-3-key:
+    type: boolean
+    description:
+      Disable 3 Key reset. Setting this disables the three key reset
+      output to the watchdog reset block.
+
+  diag-mode:
+    type: boolean
+    description:
+      Key scan diagnostic mode. Setting this allows key scanning to be
+      directly controlled through the key register by writes from the
+      ARM Core.
+
+  back-drive:
+    type: boolean
+    description:
+      Key scan back driving enable. Setting this enables the key
+      scanning logic to back drive the row and column pins of the
+      chip high during the first two column counts in the
+      row/column counter.
+
+  test-mode:
+    type: boolean
+    description:
+      Test mode. When this is set, the counter RC_COUNT is advanced
+      by 8 counts when EN is active. The effect is that only column 0
+      is checked in each row. This test mode allows a faster test
+      of the ROW pins.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - linux,keymap
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/cirrus,ep93xx-clock.h>
+    #include <dt-bindings/input/input.h>
+    keypad@800f0000 {
+        compatible = "cirrus,ep9301-keypad";
+        reg = <0x800f0000 0x0c>;
+        interrupt-parent = <&vic0>;
+        interrupts = <29>;
+        clocks = <&syscon EP93XX_CLK_KEYPAD>;
+        clock-names = "ep93xx-keypad";
+        pinctrl-names = "default";
+        pinctrl-0 = <&keypad_default_pins>;
+        linux,keymap = <KEY_UP>,
+                       <KEY_DOWN>,
+                       <KEY_VOLUMEDOWN>,
+                       <KEY_HOME>,
+                       <KEY_RIGHT>,
+                       <KEY_LEFT>,
+                       <KEY_ENTER>,
+                       <KEY_VOLUMEUP>,
+                       <KEY_F6>,
+                       <KEY_F8>,
+                       <KEY_F9>,
+                       <KEY_F10>,
+                       <KEY_F1>,
+                       <KEY_F2>,
+                       <KEY_F3>,
+                       <KEY_POWER>;
+    };
+
-- 
2.39.2

[PATCH 27/43] input: keypad: ep93xx: add DT support for Cirrus EP93xx

[Nikita Shubin]

- 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:
    Arnd Bergmann:
    - wildcards ep93xx to something meaningful, i.e. ep9301
    - drop wrappers

 drivers/input/keyboard/ep93xx_keypad.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/input/keyboard/ep93xx_keypad.c b/drivers/input/keyboard/ep93xx_keypad.c
index 55075addcac2..bf77754fa4c7 100644
--- a/drivers/input/keyboard/ep93xx_keypad.c
+++ b/drivers/input/keyboard/ep93xx_keypad.c
@@ -20,6 +20,8 @@
 #include <linux/bits.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
 #include <linux/interrupt.h>
 #include <linux/clk.h>
 #include <linux/io.h>
@@ -315,10 +317,17 @@ static int ep93xx_keypad_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct of_device_id ep93xx_keypad_of_ids[] = {
+	{ .compatible = "cirrus,ep9301-keypad" },
+	{},
+};
+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.39.2

[PATCH 37/43] input: keypad: ep93xx: drop legacy pinctrl

[Nikita Shubin]

Drop legacy acquire/release since we are using
pinctrl for this now.

Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---
 drivers/input/keyboard/ep93xx_keypad.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/drivers/input/keyboard/ep93xx_keypad.c b/drivers/input/keyboard/ep93xx_keypad.c
index bf77754fa4c7..a5f5d7d453e7 100644
--- a/drivers/input/keyboard/ep93xx_keypad.c
+++ b/drivers/input/keyboard/ep93xx_keypad.c
@@ -222,13 +222,6 @@ 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 ep93xx_keypad *keypad;
@@ -256,15 +249,6 @@ 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);
-- 
2.39.2

Re: [PATCH 27/43] input: keypad: ep93xx: add DT support for Cirrus EP93xx

[Andy Shevchenko]

On Mon, Apr 24, 2023 at 03:34:43PM +0300, Nikita Shubin wrote:
> - get keymap from the device tree
> - find register range from the device tree
> - get interrupts from device tree

Note, the below comments may be applied to the whole series where it makes sense.

...

> +#include <linux/of.h>
> +#include <linux/of_platform.h>

Why?

You do not use them. Please, check all your patches to follow the rule: include
only the headers you are really using (or their up level wrappers if required).

(Hint: in this case you have to include mod_devicetable.h)

...

> +static const struct of_device_id ep93xx_keypad_of_ids[] = {
> +	{ .compatible = "cirrus,ep9301-keypad" },
> +	{},

No comma for the terminator entry.

> +};

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 26/43] dt-bindings: input: Add DT bindings ep93xx keypad

[Rob Herring]

On Mon, Apr 24, 2023 at 03:34:42PM +0300, Nikita Shubin wrote:
> Add YAML bindings ep93xx SoC.
> 
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> ---
> 
> Notes:
>     Linus Walleij:
>     - replaced hex with proper <KEY_UP>, etc
> 
>  .../bindings/input/cirrus,ep93xx-keypad.yaml  | 123 ++++++++++++++++++
>  1 file changed, 123 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/cirrus,ep93xx-keypad.yaml
> 
> diff --git a/Documentation/devicetree/bindings/input/cirrus,ep93xx-keypad.yaml b/Documentation/devicetree/bindings/input/cirrus,ep93xx-keypad.yaml
> new file mode 100644
> index 000000000000..0310114de22e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/cirrus,ep93xx-keypad.yaml
> @@ -0,0 +1,123 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/cirrus,ep93xx-keypad.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Cirrus ep93xx keypad
> +
> +maintainers:
> +  - Dmitry Torokhov <dmitry.torokhov@gmail.com>
> +  - Alexander Sverdlin <alexander.sverdlin@gmail.com>
> +
> +allOf:
> +  - $ref: "/schemas/input/matrix-keymap.yaml#"

Drop quotes.

> +
> +description: |
> +  The KPP is designed to interface with a keypad matrix with 2-point contact
> +  or 3-point contact keys. The KPP is designed to simplify the software task
> +  of scanning a keypad matrix. The KPP is capable of detecting, debouncing,
> +  and decoding one or multiple keys pressed simultaneously on a keypad.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - cirrus,ep9301-keypad
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    enum:
> +      - ep93xx-keypad
> +
> +  debounce:

I think we have a common property for this.

> +    description: |
> +          Time in microseconds that key must be pressed or
> +          released for state change interrupt to trigger.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  prescale:
> +    description: row/column counter pre-scaler load value
> +    $ref: /schemas/types.yaml#/definitions/uint32

constraints? Or 0-2^32 is valid?

> +
> +  clk-rate:

No, have standard bindings for this.

> +    description: clock rate setting
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  disable-3-key:

Needs a vendor prefix.

> +    type: boolean
> +    description:
> +      Disable 3 Key reset. Setting this disables the three key reset
> +      output to the watchdog reset block.
> +
> +  diag-mode:

Why do we need this in DT. Shouldn't this be a runtime setting.

> +    type: boolean
> +    description:
> +      Key scan diagnostic mode. Setting this allows key scanning to be
> +      directly controlled through the key register by writes from the
> +      ARM Core.
> +
> +  back-drive:

Needs a vendor prefix.

> +    type: boolean
> +    description:
> +      Key scan back driving enable. Setting this enables the key
> +      scanning logic to back drive the row and column pins of the
> +      chip high during the first two column counts in the
> +      row/column counter.
> +
> +  test-mode:

Why do we need this in DT. Shouldn't this be a runtime setting.

> +    type: boolean
> +    description:
> +      Test mode. When this is set, the counter RC_COUNT is advanced
> +      by 8 counts when EN is active. The effect is that only column 0
> +      is checked in each row. This test mode allows a faster test
> +      of the ROW pins.
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - linux,keymap
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/cirrus,ep93xx-clock.h>
> +    #include <dt-bindings/input/input.h>
> +    keypad@800f0000 {
> +        compatible = "cirrus,ep9301-keypad";
> +        reg = <0x800f0000 0x0c>;
> +        interrupt-parent = <&vic0>;
> +        interrupts = <29>;
> +        clocks = <&syscon EP93XX_CLK_KEYPAD>;
> +        clock-names = "ep93xx-keypad";
> +        pinctrl-names = "default";
> +        pinctrl-0 = <&keypad_default_pins>;
> +        linux,keymap = <KEY_UP>,
> +                       <KEY_DOWN>,
> +                       <KEY_VOLUMEDOWN>,
> +                       <KEY_HOME>,
> +                       <KEY_RIGHT>,
> +                       <KEY_LEFT>,
> +                       <KEY_ENTER>,
> +                       <KEY_VOLUMEUP>,
> +                       <KEY_F6>,
> +                       <KEY_F8>,
> +                       <KEY_F9>,
> +                       <KEY_F10>,
> +                       <KEY_F1>,
> +                       <KEY_F2>,
> +                       <KEY_F3>,
> +                       <KEY_POWER>;
> +    };
> +
> -- 
> 2.39.2
> 

Re: [PATCH 00/43] ep93xx device tree conversion

[Krzysztof Kozlowski]

On 25/04/2023 00:29, Jakub Kicinski wrote:
> On Mon, 24 Apr 2023 13:31:25 +0200 Arnd Bergmann wrote:
>> Thanks a lot for your continued work. I can't merge any of this at
>> the moment since the upstream merge window just opened, but I'm
>> happy to take this all through the soc tree for 6.5, provided we
>> get the sufficient Acks from the subsystem maintainers. Merging
>> it through each individual tree would take a lot longer, so I
>> hope we can avoid that.
> 
> Is there a dependency between the patches?

I didn't get entire patchset and cover letter does not mention
dependencies, but usually there shouldn't be such. Maybe for the next
versions this should be split per subsystem?

Best regards,
Krzysztof

Re: [PATCH 00/43] ep93xx device tree conversion

[Arnd Bergmann]

On Tue, Apr 25, 2023, at 10:20, Krzysztof Kozlowski wrote:
> On 25/04/2023 00:29, Jakub Kicinski wrote:
>> On Mon, 24 Apr 2023 13:31:25 +0200 Arnd Bergmann wrote:
>>> Thanks a lot for your continued work. I can't merge any of this at
>>> the moment since the upstream merge window just opened, but I'm
>>> happy to take this all through the soc tree for 6.5, provided we
>>> get the sufficient Acks from the subsystem maintainers. Merging
>>> it through each individual tree would take a lot longer, so I
>>> hope we can avoid that.
>> 
>> Is there a dependency between the patches?
>
> I didn't get entire patchset and cover letter does not mention
> dependencies, but usually there shouldn't be such. Maybe for the next
> versions this should be split per subsystem?

Clearly the last patch that removes the board files depends on
all the previous patches, but I assume that the other ones
are all independent.

We don't do complete conversions from boardfiles to DT that often
any more, but in the past we tended to do this through a cross-
subsystem branch in the soc tree, which helps do it more quickly
and is less work for Nikita. In this case, I would make it a
separate top-level branch in the soc tree.

If anyone strongly feels that the patches should go through
the subsystem trees here, we'll take the longer path and
do the changes separately, with the boardfile removal
coming a release later.

     Arnd

Re: [PATCH 00/43] ep93xx device tree conversion

[Linus Walleij]

On Mon, Apr 24, 2023 at 11:35 AM Nikita Shubin
<nikita.shubin@maquefel.me> wrote:

> This series aims to convert ep93xx from platform to full device tree support.
>
> Tested on ts7250 64 RAM/128 MiB Nand flash, edb9302.

Neat, I'd say let's merge this for 6.5 once the final rough edges are
off. The DT bindings should be easy to fix.

This is a big patch set and the improvement to the ARM kernel it
brings is great, so I am a bit worried about over-review stalling the
merged. If there start to be nitpicky comments I would prefer that
we merge it and let minor comments and "nice-to-haves" be
addressed in-tree during the development cycle.

I encourage you to use b4 to manage the patch series if you
have time to learn it, it could help you:
https://people.kernel.org/monsieuricon/sending-a-kernel-patch-with-b4-part-1

Yours,
Linus Walleij

Re: [PATCH 00/43] ep93xx device tree conversion

[Linus Walleij]

On Wed, Apr 26, 2023 at 11:02 PM Mark Brown <broonie@kernel.org> wrote:
> On Wed, Apr 26, 2023 at 10:56:53PM +0200, Linus Walleij wrote:
>
> > This is a big patch set and the improvement to the ARM kernel it
> > brings is great, so I am a bit worried about over-review stalling the
> > merged. If there start to be nitpicky comments I would prefer that
> > we merge it and let minor comments and "nice-to-haves" be
> > addressed in-tree during the development cycle.
>
> I'm really not enthusiastic about the SPI bindings being merged as-is.

Agree, the bindings are more important than the code IMO,
they tend to get written in stone.

Yours,
Linus Walleij

Re: [PATCH 00/43] ep93xx device tree conversion

[Florian Fainelli]

On 4/24/2023 5:34 AM, Nikita Shubin wrote:
> This series aims to convert ep93xx from platform to full device tree support.
> 
> Tested on ts7250 64 RAM/128 MiB Nand flash, edb9302.
> 
> Thank you Linus and Arnd for your support, review and comments, sorry if i missed something -
> these series are quite big for me.
> 
> Big thanks to Alexander Sverdlin for his testing, support, review, fixes and patches.

If anyone is interested I still have a TS-7300 board [1] that is fully 
functional and could be sent out to a new home.

https://www.embeddedts.com/products/TS-7300
-- 
Florian

Re: [PATCH 00/43] ep93xx device tree conversion

[Nikita Shubin]

Hello Florian!

On Mon, 2023-05-15 at 20:47 -0700, Florian Fainelli wrote:
> 
> 
> On 4/24/2023 5:34 AM, Nikita Shubin wrote:
> > This series aims to convert ep93xx from platform to full device
> > tree support.
> > 
> > Tested on ts7250 64 RAM/128 MiB Nand flash, edb9302.
> > 
> > Thank you Linus and Arnd for your support, review and comments,
> > sorry if i missed something -
> > these series are quite big for me.
> > 
> > Big thanks to Alexander Sverdlin for his testing, support, review,
> > fixes and patches.
> 
> If anyone is interested I still have a TS-7300 board [1] that is
> fully 
> functional and could be sent out to a new home.

Thank you kindly, i'll keep this in mind !

> 
> https://www.embeddedts.com/products/TS-7300

[PATCH v1 27/43] dt-bindings: input: Add Cirrus EP93xx keypad

[Nikita Shubin]

Add YAML bindings for ep93xx SoC keypad.

Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
---

Notes:
    v0 -> v1:
    
    - remove almost all but debounce-delay-ms and prescale
    - s/ep9301-keypad/ep9307-keypad/ it's actually only for
      ep9307, ep9312, ep9315
    
    Krzysztof Kozlowski:
    - renamed file
    - changed maintainers
    - dropped quotes
    - dropped clock-names
    - use fallback compatible and list all possible compatibles
    - fix ident

 .../bindings/input/cirrus,ep9307-keypad.yaml  | 86 +++++++++++++++++++
 1 file changed, 86 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml

diff --git a/Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml b/Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml
new file mode 100644
index 000000000000..c7eb10a84a6b
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml
@@ -0,0 +1,86 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/cirrus,ep9307-keypad.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cirrus ep93xx keypad
+
+maintainers:
+  - Alexander Sverdlin <alexander.sverdlin@gmail.com>
+
+allOf:
+  - $ref: /schemas/input/matrix-keymap.yaml#
+
+description: |
+  The KPP is designed to interface with a keypad matrix with 2-point contact
+  or 3-point contact keys. The KPP is designed to simplify the software task
+  of scanning a keypad matrix. The KPP is capable of detecting, debouncing,
+  and decoding one or multiple keys pressed simultaneously on a keypad.
+
+properties:
+  compatible:
+    oneOf:
+      - const: cirrus,ep9307-keypad
+      - items:
+          - enum:
+              - cirrus,ep9312-keypad
+              - cirrus,ep9315-keypad
+          - const: cirrus,ep9307-keypad
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+  cirrus,debounce-delay-ms:
+    description: |
+          Time in microseconds that key must be pressed or
+          released for state change interrupt to trigger.
+
+  cirrus,prescale:
+    description: row/column counter pre-scaler load value
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - linux,keymap
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/cirrus,ep93xx-clock.h>
+    #include <dt-bindings/input/input.h>
+    keypad@800f0000 {
+      compatible = "cirrus,ep9301-keypad";
+      reg = <0x800f0000 0x0c>;
+      interrupt-parent = <&vic0>;
+      interrupts = <29>;
+      clocks = <&syscon EP93XX_CLK_KEYPAD>;
+      pinctrl-names = "default";
+      pinctrl-0 = <&keypad_default_pins>;
+      linux,keymap = <KEY_UP>,
+                     <KEY_DOWN>,
+                     <KEY_VOLUMEDOWN>,
+                     <KEY_HOME>,
+                     <KEY_RIGHT>,
+                     <KEY_LEFT>,
+                     <KEY_ENTER>,
+                     <KEY_VOLUMEUP>,
+                     <KEY_F6>,
+                     <KEY_F8>,
+                     <KEY_F9>,
+                     <KEY_F10>,
+                     <KEY_F1>,
+                     <KEY_F2>,
+                     <KEY_F3>,
+                     <KEY_POWER>;
+    };
-- 
2.37.4

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

[Nikita Shubin]

- 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);
+
 	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,
-- 
2.37.4

Re: [PATCH v1 27/43] dt-bindings: input: Add Cirrus EP93xx keypad

[Rob Herring]

On Thu, 01 Jun 2023 08:45:32 +0300, Nikita Shubin wrote:
> Add YAML bindings for ep93xx SoC keypad.
> 
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> ---
> 
> Notes:
>     v0 -> v1:
> 
>     - remove almost all but debounce-delay-ms and prescale
>     - s/ep9301-keypad/ep9307-keypad/ it's actually only for
>       ep9307, ep9312, ep9315
> 
>     Krzysztof Kozlowski:
>     - renamed file
>     - changed maintainers
>     - dropped quotes
>     - dropped clock-names
>     - use fallback compatible and list all possible compatibles
>     - fix ident
> 
>  .../bindings/input/cirrus,ep9307-keypad.yaml  | 86 +++++++++++++++++++
>  1 file changed, 86 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.example.dtb: /example-0/keypad@800f0000: failed to match any schema with compatible: ['cirrus,ep9301-keypad']

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230601054549.10843-9-nikita.shubin@maquefel.me

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

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

[kernel test robot]

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	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[kernel test robot]

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	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

[Andy Shevchenko]

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.

-- 
With Best Regards,
Andy Shevchenko

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

[Nikita Shubin]

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.
> 

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

[Andy Shevchenko]

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!

-- 
With Best Regards,
Andy Shevchenko

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

[Dmitry Torokhov]

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.

-- 
Dmitry

Re: [PATCH v1 27/43] dt-bindings: input: Add Cirrus EP93xx keypad

[Rob Herring]

On Thu, Jun 01, 2023 at 08:45:32AM +0300, Nikita Shubin wrote:
> Add YAML bindings for ep93xx SoC keypad.
> 
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> ---
> 
> Notes:
>     v0 -> v1:
>     
>     - remove almost all but debounce-delay-ms and prescale
>     - s/ep9301-keypad/ep9307-keypad/ it's actually only for
>       ep9307, ep9312, ep9315
>     
>     Krzysztof Kozlowski:
>     - renamed file
>     - changed maintainers
>     - dropped quotes
>     - dropped clock-names
>     - use fallback compatible and list all possible compatibles
>     - fix ident
> 
>  .../bindings/input/cirrus,ep9307-keypad.yaml  | 86 +++++++++++++++++++
>  1 file changed, 86 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml
> 
> diff --git a/Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml b/Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml
> new file mode 100644
> index 000000000000..c7eb10a84a6b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml
> @@ -0,0 +1,86 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/cirrus,ep9307-keypad.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Cirrus ep93xx keypad
> +
> +maintainers:
> +  - Alexander Sverdlin <alexander.sverdlin@gmail.com>
> +
> +allOf:
> +  - $ref: /schemas/input/matrix-keymap.yaml#
> +
> +description: |

Don't need '|'.

> +  The KPP is designed to interface with a keypad matrix with 2-point contact
> +  or 3-point contact keys. The KPP is designed to simplify the software task
> +  of scanning a keypad matrix. The KPP is capable of detecting, debouncing,
> +  and decoding one or multiple keys pressed simultaneously on a keypad.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: cirrus,ep9307-keypad
> +      - items:
> +          - enum:
> +              - cirrus,ep9312-keypad
> +              - cirrus,ep9315-keypad
> +          - const: cirrus,ep9307-keypad
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +  cirrus,debounce-delay-ms:

Use the somewhat standard 'debounce-delay-ms'.

> +    description: |
> +          Time in microseconds that key must be pressed or
> +          released for state change interrupt to trigger.
> +
> +  cirrus,prescale:
> +    description: row/column counter pre-scaler load value
> +    $ref: /schemas/types.yaml#/definitions/uint32

Constraints?

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - linux,keymap
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/cirrus,ep93xx-clock.h>
> +    #include <dt-bindings/input/input.h>
> +    keypad@800f0000 {
> +      compatible = "cirrus,ep9301-keypad";
> +      reg = <0x800f0000 0x0c>;
> +      interrupt-parent = <&vic0>;
> +      interrupts = <29>;
> +      clocks = <&syscon EP93XX_CLK_KEYPAD>;
> +      pinctrl-names = "default";
> +      pinctrl-0 = <&keypad_default_pins>;
> +      linux,keymap = <KEY_UP>,
> +                     <KEY_DOWN>,
> +                     <KEY_VOLUMEDOWN>,
> +                     <KEY_HOME>,
> +                     <KEY_RIGHT>,
> +                     <KEY_LEFT>,
> +                     <KEY_ENTER>,
> +                     <KEY_VOLUMEUP>,
> +                     <KEY_F6>,
> +                     <KEY_F8>,
> +                     <KEY_F9>,
> +                     <KEY_F10>,
> +                     <KEY_F1>,
> +                     <KEY_F2>,
> +                     <KEY_F3>,
> +                     <KEY_POWER>;
> +    };
> -- 
> 2.37.4
>