* Re: [PATCH 00/43] ep93xx device tree conversion
2023-04-24 12:34 [PATCH 00/43] ep93xx device tree conversion Nikita Shubin
@ 2023-04-24 11:31 ` Arnd Bergmann
[not found] ` <20230424152933.48b2ede1@kernel.org>
2023-04-24 12:34 ` [PATCH 26/43] dt-bindings: input: Add DT bindings ep93xx keypad Nikita Shubin
` (6 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Arnd Bergmann @ 2023-04-24 11:31 UTC (permalink / raw)
To: Nikita Shubin
Cc: Arnd Bergmann, Linus Walleij, Alexander Sverdlin,
David S . Miller, Jonathan Neuschäfer, Russell King,
Uwe Kleine-König, Alessandro Zummo, Alexander Gordeev,
Alexandre Belloni, Andy Shevchenko, Bartosz Golaszewski,
Brian Norris, Chuanhong Guo, Conor.Dooley, Damien Le Moal,
Daniel Lezcano, Dmitry Torokhov, Emil Renner Berthing,
Eric Dumazet, Florian Fainelli, Guenter Roeck, Hartley Sweeten,
Heiko Stübner, Hitomi Hasegawa, Jakub Kicinski,
Jaroslav Kysela, Jean Delvare, Joel Stanley, Jonathan Cameron,
Krzysztof Kozlowski, Damien Le Moal, Liam Girdwood, Liang Yang,
Linus Walleij, Lukasz Majewski, Lv Ruyi, Mark Brown,
Masahiro Yamada, Michael Turquette, Miquel Raynal,
Nathan Chancellor, Nick Desaulniers, Nicolas Saenz Julienne,
Olof Johansson, Paolo Abeni, Qin Jian, Richard Weinberger,
Rob Herring, Robert Jarzmik, Russell King, Sebastian Reichel,
Sergey Shtylyov, Stephen Boyd, Sumanth Korikkar, Sven Peter,
Takashi Iwai, Thierry Reding, Thomas Gleixner, Ulf Hansson,
Vasily Gorbik, Vignesh Raghavendra, Vinod Koul, Walker Chen,
Wim Van Sebroeck, Yinbo Zhu, alsa-devel, devicetree, dmaengine,
linux-arm-kernel, linux-clk, open list:GPIO SUBSYSTEM, linux-ide,
linux-input, linux-kernel, linux-mtd, linux-pm, linux-pwm,
linux-rtc, linux-spi, linux-watchdog, Netdev, soc
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
^ permalink raw reply [flat|nested] 23+ messages in thread* [PATCH 26/43] dt-bindings: input: Add DT bindings ep93xx keypad
2023-04-24 12:34 [PATCH 00/43] ep93xx device tree conversion Nikita Shubin
2023-04-24 11:31 ` Arnd Bergmann
@ 2023-04-24 12:34 ` Nikita Shubin
2023-04-24 16:21 ` Rob Herring
2023-04-24 12:34 ` [PATCH 27/43] input: keypad: ep93xx: add DT support for Cirrus EP93xx Nikita Shubin
` (5 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Nikita Shubin @ 2023-04-24 12:34 UTC (permalink / raw)
Cc: Arnd Bergmann, Linus Walleij, Alexander Sverdlin, Dmitry Torokhov,
Rob Herring, Krzysztof Kozlowski, linux-input, devicetree,
linux-kernel
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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 26/43] dt-bindings: input: Add DT bindings ep93xx keypad
2023-04-24 12:34 ` [PATCH 26/43] dt-bindings: input: Add DT bindings ep93xx keypad Nikita Shubin
@ 2023-04-24 16:21 ` Rob Herring
0 siblings, 0 replies; 23+ messages in thread
From: Rob Herring @ 2023-04-24 16:21 UTC (permalink / raw)
To: Nikita Shubin
Cc: Arnd Bergmann, Linus Walleij, Alexander Sverdlin, Dmitry Torokhov,
Krzysztof Kozlowski, linux-input, devicetree, linux-kernel
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
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 27/43] input: keypad: ep93xx: add DT support for Cirrus EP93xx
2023-04-24 12:34 [PATCH 00/43] ep93xx device tree conversion Nikita Shubin
2023-04-24 11:31 ` Arnd Bergmann
2023-04-24 12:34 ` [PATCH 26/43] dt-bindings: input: Add DT bindings ep93xx keypad Nikita Shubin
@ 2023-04-24 12:34 ` Nikita Shubin
2023-04-24 14:45 ` Andy Shevchenko
2023-04-24 12:34 ` [PATCH 37/43] input: keypad: ep93xx: drop legacy pinctrl Nikita Shubin
` (4 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Nikita Shubin @ 2023-04-24 12:34 UTC (permalink / raw)
Cc: Arnd Bergmann, Linus Walleij, Alexander Sverdlin, Dmitry Torokhov,
Jonathan Cameron, Andy Shevchenko, Lv Ruyi, linux-input,
linux-kernel
- 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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 27/43] input: keypad: ep93xx: add DT support for Cirrus EP93xx
2023-04-24 12:34 ` [PATCH 27/43] input: keypad: ep93xx: add DT support for Cirrus EP93xx Nikita Shubin
@ 2023-04-24 14:45 ` Andy Shevchenko
0 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2023-04-24 14:45 UTC (permalink / raw)
To: Nikita Shubin
Cc: Arnd Bergmann, Linus Walleij, Alexander Sverdlin, Dmitry Torokhov,
Jonathan Cameron, Lv Ruyi, linux-input, linux-kernel
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 37/43] input: keypad: ep93xx: drop legacy pinctrl
2023-04-24 12:34 [PATCH 00/43] ep93xx device tree conversion Nikita Shubin
` (2 preceding siblings ...)
2023-04-24 12:34 ` [PATCH 27/43] input: keypad: ep93xx: add DT support for Cirrus EP93xx Nikita Shubin
@ 2023-04-24 12:34 ` Nikita Shubin
2023-04-26 20:56 ` [PATCH 00/43] ep93xx device tree conversion Linus Walleij
` (3 subsequent siblings)
7 siblings, 0 replies; 23+ messages in thread
From: Nikita Shubin @ 2023-04-24 12:34 UTC (permalink / raw)
Cc: Arnd Bergmann, Linus Walleij, Alexander Sverdlin, Dmitry Torokhov,
Andy Shevchenko, Jonathan Cameron, Lv Ruyi, linux-input,
linux-kernel
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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH 00/43] ep93xx device tree conversion
2023-04-24 12:34 [PATCH 00/43] ep93xx device tree conversion Nikita Shubin
` (3 preceding siblings ...)
2023-04-24 12:34 ` [PATCH 37/43] input: keypad: ep93xx: drop legacy pinctrl Nikita Shubin
@ 2023-04-26 20:56 ` Linus Walleij
[not found] ` <b5396ef5-3fed-4e98-8f37-a9cd4473bddc@sirena.org.uk>
2023-05-16 3:47 ` Florian Fainelli
` (2 subsequent siblings)
7 siblings, 1 reply; 23+ messages in thread
From: Linus Walleij @ 2023-04-26 20:56 UTC (permalink / raw)
To: Nikita Shubin
Cc: Arnd Bergmann, Linus Walleij, Alexander Sverdlin, David S. Miller,
Jonathan Neuschäfer, Russell King (Oracle),
Uwe Kleine-König, Alessandro Zummo, Alexander Gordeev,
Alexandre Belloni, Andy Shevchenko, Arnd Bergmann,
Bartosz Golaszewski, Brian Norris, Chuanhong Guo, Conor Dooley,
Damien Le Moal, Daniel Lezcano, Dmitry Torokhov,
Emil Renner Berthing, Eric Dumazet, Florian Fainelli,
Guenter Roeck, Hartley Sweeten, Heiko Stuebner, Hitomi Hasegawa,
Jakub Kicinski, Jaroslav Kysela, Jean Delvare, Joel Stanley,
Jonathan Cameron, Krzysztof Kozlowski, Le Moal, Liam Girdwood,
Liang Yang, Lukasz Majewski, Lv Ruyi, Mark Brown, Masahiro Yamada,
Michael Turquette, Miquel Raynal, Nathan Chancellor,
Nick Desaulniers, Nicolas Saenz Julienne, Olof Johansson,
Paolo Abeni, Qin Jian, Richard Weinberger, Rob Herring,
Robert Jarzmik, Russell King, Sebastian Reichel, Sergey Shtylyov,
Stephen Boyd, Sumanth Korikkar, Sven Peter, Takashi Iwai,
Thierry Reding, Thomas Gleixner, Ulf Hansson, Vasily Gorbik,
Vignesh Raghavendra, Vinod Koul, Walker Chen, Wim Van Sebroeck,
Yinbo Zhu, alsa-devel, devicetree, dmaengine, linux-arm-kernel,
linux-clk, linux-gpio, linux-ide, linux-input, linux-kernel,
linux-mtd, linux-pm, linux-pwm, linux-rtc, linux-spi,
linux-watchdog, netdev, soc
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
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 00/43] ep93xx device tree conversion
2023-04-24 12:34 [PATCH 00/43] ep93xx device tree conversion Nikita Shubin
` (4 preceding siblings ...)
2023-04-26 20:56 ` [PATCH 00/43] ep93xx device tree conversion Linus Walleij
@ 2023-05-16 3:47 ` Florian Fainelli
2023-05-16 10:37 ` Nikita Shubin
2023-06-01 5:45 ` [PATCH v1 27/43] dt-bindings: input: Add Cirrus EP93xx keypad Nikita Shubin
2023-06-01 5:45 ` [PATCH v1 28/43] input: keypad: ep93xx: add DT support for Cirrus EP93xx Nikita Shubin
7 siblings, 1 reply; 23+ messages in thread
From: Florian Fainelli @ 2023-05-16 3:47 UTC (permalink / raw)
To: Nikita Shubin
Cc: Arnd Bergmann, Linus Walleij, Alexander Sverdlin, David S. Miller,
Jonathan Neuschäfer, Russell King (Oracle),
Uwe Kleine-König, Alessandro Zummo, Alexander Gordeev,
Alexandre Belloni, Andy Shevchenko, Arnd Bergmann,
Bartosz Golaszewski, Brian Norris, Chuanhong Guo, Conor Dooley,
Damien Le Moal, Daniel Lezcano, Dmitry Torokhov,
Emil Renner Berthing, Eric Dumazet, Guenter Roeck,
Hartley Sweeten, Heiko Stuebner, Hitomi Hasegawa, Jakub Kicinski,
Jaroslav Kysela, Jean Delvare, Joel Stanley, Jonathan Cameron,
Krzysztof Kozlowski, Le Moal, Liam Girdwood, Liang Yang,
Linus Walleij, Lukasz Majewski, Lv Ruyi, Mark Brown,
Masahiro Yamada, Michael Turquette, Miquel Raynal,
Nathan Chancellor, Nick Desaulniers, Nicolas Saenz Julienne,
Olof Johansson, Paolo Abeni, Qin Jian, Richard Weinberger,
Rob Herring, Robert Jarzmik, Russell King, Sebastian Reichel,
Sergey Shtylyov, Stephen Boyd, Sumanth Korikkar, Sven Peter,
Takashi Iwai, Thierry Reding, Thomas Gleixner, Ulf Hansson,
Vasily Gorbik, Vignesh Raghavendra, Vinod Koul, Walker Chen,
Wim Van Sebroeck, Yinbo Zhu, alsa-devel, devicetree, dmaengine,
linux-arm-kernel, linux-clk, linux-gpio, linux-ide, linux-input,
linux-kernel, linux-mtd, linux-pm, linux-pwm, linux-rtc,
linux-spi, linux-watchdog, netdev, soc
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
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH 00/43] ep93xx device tree conversion
2023-05-16 3:47 ` Florian Fainelli
@ 2023-05-16 10:37 ` Nikita Shubin
0 siblings, 0 replies; 23+ messages in thread
From: Nikita Shubin @ 2023-05-16 10:37 UTC (permalink / raw)
To: Florian Fainelli
Cc: Arnd Bergmann, Linus Walleij, Alexander Sverdlin, David S. Miller,
Jonathan Neuschäfer, Russell King (Oracle),
Uwe Kleine-König, Alessandro Zummo, Alexander Gordeev,
Alexandre Belloni, Andy Shevchenko, Arnd Bergmann,
Bartosz Golaszewski, Brian Norris, Chuanhong Guo, Conor Dooley,
Damien Le Moal, Daniel Lezcano, Dmitry Torokhov,
Emil Renner Berthing, Eric Dumazet, Guenter Roeck,
Hartley Sweeten, Heiko Stuebner, Hitomi Hasegawa, Jakub Kicinski,
Jaroslav Kysela, Jean Delvare, Joel Stanley, Jonathan Cameron,
Krzysztof Kozlowski, Le Moal, Liam Girdwood, Liang Yang,
Linus Walleij, Lukasz Majewski, Lv Ruyi, Mark Brown,
Masahiro Yamada, Michael Turquette, Miquel Raynal,
Nathan Chancellor, Nick Desaulniers, Nicolas Saenz Julienne,
Olof Johansson, Paolo Abeni, Qin Jian, Richard Weinberger,
Rob Herring, Robert Jarzmik, Russell King, Sebastian Reichel,
Sergey Shtylyov, Stephen Boyd, Sumanth Korikkar, Sven Peter,
Takashi Iwai, Thierry Reding, Thomas Gleixner, Ulf Hansson,
Vasily Gorbik, Vignesh Raghavendra, Vinod Koul, Walker Chen,
Wim Van Sebroeck, Yinbo Zhu, alsa-devel, devicetree, dmaengine,
linux-arm-kernel, linux-clk, linux-gpio, linux-ide, linux-input,
linux-kernel, linux-mtd, linux-pm, linux-pwm, linux-rtc,
linux-spi, linux-watchdog, netdev, soc
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v1 27/43] dt-bindings: input: Add Cirrus EP93xx keypad
2023-04-24 12:34 [PATCH 00/43] ep93xx device tree conversion Nikita Shubin
` (5 preceding siblings ...)
2023-05-16 3:47 ` Florian Fainelli
@ 2023-06-01 5:45 ` Nikita Shubin
2023-06-01 8:24 ` Rob Herring
2023-06-08 15:01 ` Rob Herring
2023-06-01 5:45 ` [PATCH v1 28/43] input: keypad: ep93xx: add DT support for Cirrus EP93xx Nikita Shubin
7 siblings, 2 replies; 23+ messages in thread
From: Nikita Shubin @ 2023-06-01 5:45 UTC (permalink / raw)
To: Alexander Sverdlin, Arnd Bergmann, Linus Walleij, Dmitry Torokhov,
Rob Herring, Krzysztof Kozlowski
Cc: Nikita Shubin, Michael Peters, Kris Bahnsen, linux-input,
devicetree, linux-kernel
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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v1 27/43] dt-bindings: input: Add Cirrus EP93xx keypad
2023-06-01 5:45 ` [PATCH v1 27/43] dt-bindings: input: Add Cirrus EP93xx keypad Nikita Shubin
@ 2023-06-01 8:24 ` Rob Herring
2023-06-08 15:01 ` Rob Herring
1 sibling, 0 replies; 23+ messages in thread
From: Rob Herring @ 2023-06-01 8:24 UTC (permalink / raw)
To: Nikita Shubin
Cc: linux-input, devicetree, Rob Herring, Krzysztof Kozlowski,
linux-kernel, Michael Peters, Linus Walleij, Kris Bahnsen,
Arnd Bergmann, Alexander Sverdlin, Dmitry Torokhov
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.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1 27/43] dt-bindings: input: Add Cirrus EP93xx keypad
2023-06-01 5:45 ` [PATCH v1 27/43] dt-bindings: input: Add Cirrus EP93xx keypad Nikita Shubin
2023-06-01 8:24 ` Rob Herring
@ 2023-06-08 15:01 ` Rob Herring
1 sibling, 0 replies; 23+ messages in thread
From: Rob Herring @ 2023-06-08 15:01 UTC (permalink / raw)
To: Nikita Shubin
Cc: Alexander Sverdlin, Arnd Bergmann, Linus Walleij, Dmitry Torokhov,
Krzysztof Kozlowski, Michael Peters, Kris Bahnsen, linux-input,
devicetree, linux-kernel
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
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH v1 28/43] input: keypad: ep93xx: add DT support for Cirrus EP93xx
2023-04-24 12:34 [PATCH 00/43] ep93xx device tree conversion Nikita Shubin
` (6 preceding siblings ...)
2023-06-01 5:45 ` [PATCH v1 27/43] dt-bindings: input: Add Cirrus EP93xx keypad Nikita Shubin
@ 2023-06-01 5:45 ` Nikita Shubin
2023-06-01 15:20 ` kernel test robot
` (3 more replies)
7 siblings, 4 replies; 23+ messages in thread
From: Nikita Shubin @ 2023-06-01 5:45 UTC (permalink / raw)
To: Alexander Sverdlin, Arnd Bergmann, Linus Walleij, Dmitry Torokhov,
Andy Shevchenko, Nikita Shubin, Jonathan Cameron
Cc: Michael Peters, Kris Bahnsen, linux-input, linux-kernel
- 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
^ permalink raw reply related [flat|nested] 23+ messages in thread* Re: [PATCH v1 28/43] input: keypad: ep93xx: add DT support for Cirrus EP93xx
2023-06-01 5:45 ` [PATCH v1 28/43] input: keypad: ep93xx: add DT support for Cirrus EP93xx Nikita Shubin
@ 2023-06-01 15:20 ` kernel test robot
2023-06-01 15:31 ` kernel test robot
` (2 subsequent siblings)
3 siblings, 0 replies; 23+ messages in thread
From: kernel test robot @ 2023-06-01 15:20 UTC (permalink / raw)
To: Nikita Shubin, Alexander Sverdlin, Arnd Bergmann, Linus Walleij,
Dmitry Torokhov, Andy Shevchenko, Jonathan Cameron
Cc: oe-kbuild-all, Michael Peters, Kris Bahnsen, linux-input,
linux-kernel
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
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v1 28/43] input: keypad: ep93xx: add DT support for Cirrus EP93xx
2023-06-01 5:45 ` [PATCH v1 28/43] input: keypad: ep93xx: add DT support for Cirrus EP93xx Nikita Shubin
2023-06-01 15:20 ` kernel test robot
@ 2023-06-01 15:31 ` kernel test robot
2023-06-01 16:54 ` Andy Shevchenko
2023-06-06 18:57 ` Dmitry Torokhov
3 siblings, 0 replies; 23+ messages in thread
From: kernel test robot @ 2023-06-01 15:31 UTC (permalink / raw)
To: Nikita Shubin, Alexander Sverdlin, Arnd Bergmann, Linus Walleij,
Dmitry Torokhov, Andy Shevchenko, Jonathan Cameron
Cc: llvm, oe-kbuild-all, Michael Peters, Kris Bahnsen, linux-input,
linux-kernel
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
^ permalink raw reply [flat|nested] 23+ messages in thread* Re: [PATCH v1 28/43] input: keypad: ep93xx: add DT support for Cirrus EP93xx
2023-06-01 5:45 ` [PATCH v1 28/43] input: keypad: ep93xx: add DT support for Cirrus EP93xx Nikita Shubin
2023-06-01 15:20 ` kernel test robot
2023-06-01 15:31 ` kernel test robot
@ 2023-06-01 16:54 ` Andy Shevchenko
2023-06-04 19:14 ` Nikita Shubin
2023-06-06 18:57 ` Dmitry Torokhov
3 siblings, 1 reply; 23+ messages in thread
From: Andy Shevchenko @ 2023-06-01 16:54 UTC (permalink / raw)
To: Nikita Shubin
Cc: Alexander Sverdlin, Arnd Bergmann, Linus Walleij, Dmitry Torokhov,
Jonathan Cameron, Michael Peters, Kris Bahnsen, linux-input,
linux-kernel
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1 28/43] input: keypad: ep93xx: add DT support for Cirrus EP93xx
2023-06-01 16:54 ` Andy Shevchenko
@ 2023-06-04 19:14 ` Nikita Shubin
2023-06-05 11:26 ` Andy Shevchenko
0 siblings, 1 reply; 23+ messages in thread
From: Nikita Shubin @ 2023-06-04 19:14 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Alexander Sverdlin, Arnd Bergmann, Linus Walleij, Dmitry Torokhov,
Jonathan Cameron, Michael Peters, Kris Bahnsen, linux-input,
linux-kernel
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.
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1 28/43] input: keypad: ep93xx: add DT support for Cirrus EP93xx
2023-06-04 19:14 ` Nikita Shubin
@ 2023-06-05 11:26 ` Andy Shevchenko
0 siblings, 0 replies; 23+ messages in thread
From: Andy Shevchenko @ 2023-06-05 11:26 UTC (permalink / raw)
To: Nikita Shubin
Cc: Alexander Sverdlin, Arnd Bergmann, Linus Walleij, Dmitry Torokhov,
Jonathan Cameron, Michael Peters, Kris Bahnsen, linux-input,
linux-kernel
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
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v1 28/43] input: keypad: ep93xx: add DT support for Cirrus EP93xx
2023-06-01 5:45 ` [PATCH v1 28/43] input: keypad: ep93xx: add DT support for Cirrus EP93xx Nikita Shubin
` (2 preceding siblings ...)
2023-06-01 16:54 ` Andy Shevchenko
@ 2023-06-06 18:57 ` Dmitry Torokhov
3 siblings, 0 replies; 23+ messages in thread
From: Dmitry Torokhov @ 2023-06-06 18:57 UTC (permalink / raw)
To: Nikita Shubin
Cc: Alexander Sverdlin, Arnd Bergmann, Linus Walleij, Andy Shevchenko,
Jonathan Cameron, Michael Peters, Kris Bahnsen, linux-input,
linux-kernel
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
^ permalink raw reply [flat|nested] 23+ messages in thread