.../bindings/input/gpio-matrix-keypad.txt | 2 + drivers/input/keyboard/matrix_keypad.c | 44 ++++++++++++++++--- 2 files changed, 39 insertions(+), 7 deletions(-)
This series is needed to avoid key-loss if the GPIOs are connected via a I2C GPIO Mux
that introduces additonal latencies between key change and matrix scan.
Patch 1, 2 and 6 in this series introduce a performance boost on its own
and can be merged separately.
Patch 3, 4 and 5 depend on each other.
Markus Burri (6):
Input: matrix_keypad - move gpio-row init to the init part
Input: matrix_keypad - use fsleep for variable delay duration
Input: matrix_keypad - add function for reading row state
dt-bindings: input: matrix_keypad - add settle time after enable all
columns
Input: matrix_keypad - add settle time after enable all columns
Input: matrix_keypad - detect change during scan
.../bindings/input/gpio-matrix-keypad.txt | 2 +
drivers/input/keyboard/matrix_keypad.c | 44 ++++++++++++++++---
2 files changed, 39 insertions(+), 7 deletions(-)
--
2.39.5
This series is needed to avoid key-loss if the GPIOs are connected via a I2C GPIO Mux
that introduces additonal latencies between key change and matrix scan.
This series applies on 6.12-rc6
Changes in V2:
* added patch to convert dt-bindings to YAML
* added missing dt-bindings properties description
* removed [PATCH 1/6] Input: matrix_keypad - move gpio-row init to the init
it would revert 01c84b03d80aab9f04c4e3e1f9085f4202ff7c29
* removed internally given Tested-by and Reviewed-by tags
V1: https://lore.kernel.org/lkml/20241031063004.69956-1-markus.burri@mt.com/
Markus Burri (7):
Input: matrix_keypad - use fsleep for variable delay duration
Input: matrix_keypad - add function for reading row state
dt-bindings: input: matrix_keypad - convert to YAML
dt-bindings: input: matrix_keypad - add missing property
dt-bindings: input: matrix_keypad - add settle time after enable all
columns
Input: matrix_keypad - add settle time after enable all columns
Input: matrix_keypad - detect change during scan
.../bindings/input/gpio-matrix-keypad.txt | 49 --------
.../bindings/input/gpio-matrix-keypad.yaml | 110 ++++++++++++++++++
drivers/input/keyboard/matrix_keypad.c | 39 ++++++-
3 files changed, 145 insertions(+), 53 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt
create mode 100644 Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
--
2.39.5
The delay is retrieved from a device-tree property, so the duration is
variable. fsleep guesses the best delay function based on duration.
Link: https://www.kernel.org/doc/html/latest/timers/timers-howto.html
Signed-off-by: Markus Burri <markus.burri@mt.com>
---
drivers/input/keyboard/matrix_keypad.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
index 3c38bae..9eb6808 100644
--- a/drivers/input/keyboard/matrix_keypad.c
+++ b/drivers/input/keyboard/matrix_keypad.c
@@ -69,7 +69,7 @@ static void activate_col(struct matrix_keypad *keypad, int col, bool on)
__activate_col(keypad, col, on);
if (on && keypad->col_scan_delay_us)
- udelay(keypad->col_scan_delay_us);
+ fsleep(keypad->col_scan_delay_us);
}
static void activate_all_cols(struct matrix_keypad *keypad, bool on)
--
2.39.5On Tue, Nov 05, 2024 at 02:03:16PM +0100, Markus Burri wrote: > The delay is retrieved from a device-tree property, so the duration is > variable. fsleep guesses the best delay function based on duration. activate_col() may be called in atomic context, and if fsleep() turns into usleep_range() or msleep() we are going to have a bad time. We should either stop using request_any_context_irq() or figure out if interrupt handler can sleep or not and adjust behavior properly. > > Link: https://www.kernel.org/doc/html/latest/timers/timers-howto.html > > Signed-off-by: Markus Burri <markus.burri@mt.com> > --- > drivers/input/keyboard/matrix_keypad.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c > index 3c38bae..9eb6808 100644 > --- a/drivers/input/keyboard/matrix_keypad.c > +++ b/drivers/input/keyboard/matrix_keypad.c > @@ -69,7 +69,7 @@ static void activate_col(struct matrix_keypad *keypad, int col, bool on) > __activate_col(keypad, col, on); > > if (on && keypad->col_scan_delay_us) > - udelay(keypad->col_scan_delay_us); > + fsleep(keypad->col_scan_delay_us); > } > > static void activate_all_cols(struct matrix_keypad *keypad, bool on) > -- > 2.39.5 > Thanks. -- Dmitry
Move the evaluation of a row state into separate function.
It will be also used by a change later in this series.
Signed-off-by: Markus Burri <markus.burri@mt.com>
---
drivers/input/keyboard/matrix_keypad.c | 14 +++++++++++---
1 file changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
index 9eb6808..2818915 100644
--- a/drivers/input/keyboard/matrix_keypad.c
+++ b/drivers/input/keyboard/matrix_keypad.c
@@ -101,6 +101,16 @@ static void disable_row_irqs(struct matrix_keypad *keypad)
disable_irq_nosync(keypad->row_irqs[i]);
}
+static uint32_t read_row_state(struct matrix_keypad *keypad)
+{
+ int row;
+ u32 row_state = 0;
+
+ for (row = 0; row < keypad->num_row_gpios; row++)
+ row_state |= row_asserted(keypad, row) ? BIT(row) : 0;
+ return row_state;
+}
+
/*
* This gets the keys from keyboard and reports it to input subsystem
*/
@@ -126,9 +136,7 @@ static void matrix_keypad_scan(struct work_struct *work)
activate_col(keypad, col, true);
- for (row = 0; row < keypad->num_row_gpios; row++)
- new_state[col] |=
- row_asserted(keypad, row) ? BIT(row) : 0;
+ new_state[col] = read_row_state(keypad);
activate_col(keypad, col, false);
}
--
2.39.5Convert the gpio-matrix-keypad bindings from text to DT schema.
Signed-off-by: Markus Burri <markus.burri@mt.com>
---
.../bindings/input/gpio-matrix-keypad.txt | 49 ---------
.../bindings/input/gpio-matrix-keypad.yaml | 99 +++++++++++++++++++
2 files changed, 99 insertions(+), 49 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt
create mode 100644 Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt
deleted file mode 100644
index 570dc10..0000000
--- a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt
+++ /dev/null
@@ -1,49 +0,0 @@
-* GPIO driven matrix keypad device tree bindings
-
-GPIO driven matrix keypad is used to interface a SoC with a matrix keypad.
-The matrix keypad supports multiple row and column lines, a key can be
-placed at each intersection of a unique row and a unique column. The matrix
-keypad can sense a key-press and key-release by means of GPIO lines and
-report the event using GPIO interrupts to the cpu.
-
-Required Properties:
-- compatible: Should be "gpio-matrix-keypad"
-- row-gpios: List of gpios used as row lines. The gpio specifier
- for this property depends on the gpio controller to
- which these row lines are connected.
-- col-gpios: List of gpios used as column lines. The gpio specifier
- for this property depends on the gpio controller to
- which these column lines are connected.
-- linux,keymap: The definition can be found at
- bindings/input/matrix-keymap.txt
-
-Optional Properties:
-- linux,no-autorepeat: do no enable autorepeat feature.
-- wakeup-source: use any event on keypad as wakeup event.
- (Legacy property supported: "linux,wakeup")
-- debounce-delay-ms: debounce interval in milliseconds
-- col-scan-delay-us: delay, measured in microseconds, that is needed
- before we can scan keypad after activating column gpio
-- drive-inactive-cols: drive inactive columns during scan,
- default is to turn inactive columns into inputs.
-
-Example:
- matrix-keypad {
- compatible = "gpio-matrix-keypad";
- debounce-delay-ms = <5>;
- col-scan-delay-us = <2>;
-
- row-gpios = <&gpio2 25 0
- &gpio2 26 0
- &gpio2 27 0>;
-
- col-gpios = <&gpio2 21 0
- &gpio2 22 0>;
-
- linux,keymap = <0x0000008B
- 0x0100009E
- 0x02000069
- 0x0001006A
- 0x0101001C
- 0x0201006C>;
- };
diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
new file mode 100644
index 0000000..745652b
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
@@ -0,0 +1,99 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+
+$id: http://devicetree.org/schemas/input/gpio-matrix-keypad.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GPIO matrix keypad
+
+description: |
+ GPIO driven matrix keypad is used to interface a SoC with a matrix keypad.
+ The matrix keypad supports multiple row and column lines, a key can be
+ placed at each intersection of a unique row and a unique column. The matrix
+ keypad can sense a key-press and key-release by means of GPIO lines and
+ report the event using GPIO interrupts to the cpu.
+
+maintainers:
+ - Marek Vasut <marek.vasut@gmail.com>
+
+properties:
+ compatible:
+ const: gpio-matrix-keypad
+
+ row-gpios:
+ description: |
+ List of GPIOs used as row lines. The gpio specifier
+ for this property depends on the gpio controller to
+ which these row lines are connected.
+
+ col-gpios:
+ description: |
+ List of GPIOs used as column lines. The gpio specifier
+ for this property depends on the gpio controller to
+ which these column lines are connected.
+
+ linux,keymap:
+ $ref: /schemas/types.yaml#/definitions/uint32-array
+ description: |
+ An array of packed 1-cell entries containing the equivalent of row,
+ column and linux key-code. The 32-bit big endian cell is packed as:
+ row << 24 | column << 16 | key-code
+
+ linux,no-autorepeat:
+ type: boolean
+ description: Do not enable autorepeat feature.
+ default: false
+
+ wakeup-source:
+ description: |
+ Use any event on keypad as wakeup event.
+ (Legacy property supported: "linux,wakeup")
+ default: false
+
+ debounce-delay-ms:
+ description: Debounce interval in milliseconds.
+ default: 0
+
+ col-scan-delay-us:
+ description: |
+ Delay, measured in microseconds, that is needed
+ before we can scan keypad after activating column gpio.
+ default: 0
+
+ drive-inactive-cols:
+ type: boolean
+ description: |
+ Drive inactive columns during scan,
+ default is to turn inactive columns into inputs.
+ default: false
+
+required:
+ - compatible
+ - row-gpios
+ - col-gpios
+ - linux,keymap
+
+additionalProperties: false
+
+examples:
+ - |
+ matrix-keypad {
+ compatible = "gpio-matrix-keypad";
+ debounce-delay-ms = <5>;
+ col-scan-delay-us = <2>;
+
+ row-gpios = <&gpio2 25 0
+ &gpio2 26 0
+ &gpio2 27 0>;
+
+ col-gpios = <&gpio2 21 0
+ &gpio2 22 0>;
+
+ linux,keymap = <0x0000008B
+ 0x0100009E
+ 0x02000069
+ 0x0001006A
+ 0x0101001C
+ 0x0201006C>;
+ };
--
2.39.5On Tue, Nov 05, 2024 at 02:03:18PM +0100, Markus Burri wrote:
> Convert the gpio-matrix-keypad bindings from text to DT schema.
Please don't thread new versions to previous thread.
>
> Signed-off-by: Markus Burri <markus.burri@mt.com>
> ---
> .../bindings/input/gpio-matrix-keypad.txt | 49 ---------
> .../bindings/input/gpio-matrix-keypad.yaml | 99 +++++++++++++++++++
> 2 files changed, 99 insertions(+), 49 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt
> create mode 100644 Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
>
> diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt
> deleted file mode 100644
> index 570dc10..0000000
> --- a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt
> +++ /dev/null
> @@ -1,49 +0,0 @@
> -* GPIO driven matrix keypad device tree bindings
> -
> -GPIO driven matrix keypad is used to interface a SoC with a matrix keypad.
> -The matrix keypad supports multiple row and column lines, a key can be
> -placed at each intersection of a unique row and a unique column. The matrix
> -keypad can sense a key-press and key-release by means of GPIO lines and
> -report the event using GPIO interrupts to the cpu.
> -
> -Required Properties:
> -- compatible: Should be "gpio-matrix-keypad"
> -- row-gpios: List of gpios used as row lines. The gpio specifier
> - for this property depends on the gpio controller to
> - which these row lines are connected.
> -- col-gpios: List of gpios used as column lines. The gpio specifier
> - for this property depends on the gpio controller to
> - which these column lines are connected.
> -- linux,keymap: The definition can be found at
> - bindings/input/matrix-keymap.txt
> -
> -Optional Properties:
> -- linux,no-autorepeat: do no enable autorepeat feature.
> -- wakeup-source: use any event on keypad as wakeup event.
> - (Legacy property supported: "linux,wakeup")
> -- debounce-delay-ms: debounce interval in milliseconds
> -- col-scan-delay-us: delay, measured in microseconds, that is needed
> - before we can scan keypad after activating column gpio
> -- drive-inactive-cols: drive inactive columns during scan,
> - default is to turn inactive columns into inputs.
> -
> -Example:
> - matrix-keypad {
> - compatible = "gpio-matrix-keypad";
> - debounce-delay-ms = <5>;
> - col-scan-delay-us = <2>;
> -
> - row-gpios = <&gpio2 25 0
> - &gpio2 26 0
> - &gpio2 27 0>;
> -
> - col-gpios = <&gpio2 21 0
> - &gpio2 22 0>;
> -
> - linux,keymap = <0x0000008B
> - 0x0100009E
> - 0x02000069
> - 0x0001006A
> - 0x0101001C
> - 0x0201006C>;
> - };
> diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> new file mode 100644
> index 0000000..745652b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
> @@ -0,0 +1,99 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +
> +$id: http://devicetree.org/schemas/input/gpio-matrix-keypad.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: GPIO matrix keypad
> +
> +description: |
Don't need '|' if no formatting.
> + GPIO driven matrix keypad is used to interface a SoC with a matrix keypad.
> + The matrix keypad supports multiple row and column lines, a key can be
> + placed at each intersection of a unique row and a unique column. The matrix
> + keypad can sense a key-press and key-release by means of GPIO lines and
> + report the event using GPIO interrupts to the cpu.
> +
> +maintainers:
> + - Marek Vasut <marek.vasut@gmail.com>
> +
> +properties:
> + compatible:
> + const: gpio-matrix-keypad
> +
> + row-gpios:
> + description: |
Ditto.
> + List of GPIOs used as row lines. The gpio specifier
> + for this property depends on the gpio controller to
> + which these row lines are connected.
Why the odd line length? Wrap at 80 char unless there's some exceptional
reason to go to 100.
> +
> + col-gpios:
> + description: |
> + List of GPIOs used as column lines. The gpio specifier
> + for this property depends on the gpio controller to
> + which these column lines are connected.
Is there some reasonable range of GPIOs? 1xN is in theory possible, but
surely no one would use N+1 GPIOs for what could be done with N GPIOs?
So 'minItems: 2'. A max is not so clear. Eventually, I guess you
couldn't scan them fast enough. 64 or 128 seems like way more than
anyone would do.
> +
> + linux,keymap:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + description: |
> + An array of packed 1-cell entries containing the equivalent of row,
> + column and linux key-code. The 32-bit big endian cell is packed as:
> + row << 24 | column << 16 | key-code
If you are copying another schema, you are doing it wrong...
You need a $ref to matrix-keymap.yaml.
> +
> + linux,no-autorepeat:
> + type: boolean
> + description: Do not enable autorepeat feature.
> + default: false
Drop 'default'. Doesn't apply to booleans.
> +
> + wakeup-source:
> + description: |
> + Use any event on keypad as wakeup event.
> + (Legacy property supported: "linux,wakeup")
Not supported if it is not in the schema. I imagine that's been
deprecated long enough, you can just drop mentioning it.
> + default: false
Drop.
> +
> + debounce-delay-ms:
> + description: Debounce interval in milliseconds.
> + default: 0
> +
> + col-scan-delay-us:
> + description: |
> + Delay, measured in microseconds, that is needed
> + before we can scan keypad after activating column gpio.
> + default: 0
> +
> + drive-inactive-cols:
> + type: boolean
> + description: |
> + Drive inactive columns during scan,
> + default is to turn inactive columns into inputs.
> + default: false
Drop
> +
> +required:
> + - compatible
> + - row-gpios
> + - col-gpios
> + - linux,keymap
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + matrix-keypad {
> + compatible = "gpio-matrix-keypad";
> + debounce-delay-ms = <5>;
> + col-scan-delay-us = <2>;
> +
> + row-gpios = <&gpio2 25 0
> + &gpio2 26 0
> + &gpio2 27 0>;
> +
> + col-gpios = <&gpio2 21 0
> + &gpio2 22 0>;
> +
> + linux,keymap = <0x0000008B
> + 0x0100009E
> + 0x02000069
> + 0x0001006A
> + 0x0101001C
> + 0x0201006C>;
> + };
> --
> 2.39.5
>
Hi Markus, kernel test robot noticed the following build warnings: [auto build test WARNING on dtor-input/for-linus] [also build test WARNING on robh/for-next linus/master v6.12-rc6 next-20241105] [cannot apply to dtor-input/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/Markus-Burri/Input-matrix_keypad-use-fsleep-for-variable-delay-duration/20241105-211348 base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus patch link: https://lore.kernel.org/r/20241105130322.213623-4-markus.burri%40mt.com patch subject: [PATCH v2 3/7] dt-bindings: input: matrix_keypad - convert to YAML reproduce: (https://download.01.org/0day-ci/archive/20241106/202411060143.8tNHeidk-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202411060143.8tNHeidk-lkp@intel.com/ All warnings (new ones prefixed by >>): >> Warning: Documentation/devicetree/bindings/power/wakeup-source.txt references a file that doesn't exist: Documentation/devicetree/bindings/input/gpio-matrix-keypad.txt Warning: Documentation/devicetree/bindings/power/wakeup-source.txt references a file that doesn't exist: Documentation/devicetree/bindings/input/qcom,pm8xxx-keypad.txt Warning: Documentation/devicetree/bindings/regulator/siliconmitus,sm5703-regulator.yaml references a file that doesn't exist: Documentation/devicetree/bindings/mfd/siliconmitus,sm5703.yaml Warning: Documentation/hwmon/g762.rst references a file that doesn't exist: Documentation/devicetree/bindings/hwmon/g762.txt Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/reserved-memory/qcom Warning: MAINTAINERS references a file that doesn't exist: Documentation/devicetree/bindings/display/exynos/ -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki
Add missing property 'gpio-activelow' to DT schema.
Signed-off-by: Markus Burri <markus.burri@mt.com>
---
.../devicetree/bindings/input/gpio-matrix-keypad.yaml | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
index 745652b..9ea66b3 100644
--- a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
+++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
@@ -51,6 +51,11 @@ properties:
(Legacy property supported: "linux,wakeup")
default: false
+ gpio-activelow:
+ type: boolean
+ description: The GPIOs are low active.
+ default: false
+
debounce-delay-ms:
description: Debounce interval in milliseconds.
default: 0
--
2.39.5On Tue, Nov 05, 2024 at 02:03:19PM +0100, Markus Burri wrote: > Add missing property 'gpio-activelow' to DT schema. What do you mean "missing property"? Why isn't it sufficient to mark the GPIOs as GPIO_ACTIVE_LOW in the various -gpios properties? > > Signed-off-by: Markus Burri <markus.burri@mt.com> > --- > .../devicetree/bindings/input/gpio-matrix-keypad.yaml | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml > index 745652b..9ea66b3 100644 > --- a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml > +++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml > @@ -51,6 +51,11 @@ properties: > (Legacy property supported: "linux,wakeup") > default: false > > + gpio-activelow: > + type: boolean > + description: The GPIOs are low active. > + default: false What you want is a flag, not a boolean here btw. Flags you can check for the presence of, booleans you cannot. > + > debounce-delay-ms: > description: Debounce interval in milliseconds. > default: 0 > -- > 2.39.5 >
Hi Conor, On Tue, Nov 05, 2024 at 06:22:36PM +0000, Conor Dooley wrote: > On Tue, Nov 05, 2024 at 02:03:19PM +0100, Markus Burri wrote: > > Add missing property 'gpio-activelow' to DT schema. > > What do you mean "missing property"? Why isn't it sufficient to mark the > GPIOs as GPIO_ACTIVE_LOW in the various -gpios properties? Unfortunately we do have "gpio-activelow" property already used in the driver since 2012 when DT support was added to the driver. This patch merely acknowledges that it is there. If DT maintainers wish to adjust known DTSes and switch to proper polarity annotation through gpio property I am all for it. > > > > > Signed-off-by: Markus Burri <markus.burri@mt.com> > > --- > > .../devicetree/bindings/input/gpio-matrix-keypad.yaml | 5 +++++ > > 1 file changed, 5 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml > > index 745652b..9ea66b3 100644 > > --- a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml > > +++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml > > @@ -51,6 +51,11 @@ properties: > > (Legacy property supported: "linux,wakeup") > > default: false > > > > + gpio-activelow: > > + type: boolean > > + description: The GPIOs are low active. > > + default: false > > What you want is a flag, not a boolean here btw. Flags you can check for > the presence of, booleans you cannot. The behavior is fixed. If the flag is true GPIO is assumed to be active low, otherwise (and in the absence of the flag) GPIO is assumed to be active high. > > > + > > debounce-delay-ms: > > description: Debounce interval in milliseconds. > > default: 0 > > -- > > 2.39.5 > > Thanks. -- Dmitry
On Tue, Nov 05, 2024 at 11:14:32AM -0800, Dmitry Torokhov wrote: > Hi Conor, > > On Tue, Nov 05, 2024 at 06:22:36PM +0000, Conor Dooley wrote: > > On Tue, Nov 05, 2024 at 02:03:19PM +0100, Markus Burri wrote: > > > Add missing property 'gpio-activelow' to DT schema. > > > > What do you mean "missing property"? Why isn't it sufficient to mark the > > GPIOs as GPIO_ACTIVE_LOW in the various -gpios properties? > > Unfortunately we do have "gpio-activelow" property already used in the > driver since 2012 when DT support was added to the driver. This patch > merely acknowledges that it is there. > > If DT maintainers wish to adjust known DTSes and switch to proper > polarity annotation through gpio property I am all for it. If that's the rationale, it should be in the commit message. It's not my job to figure out people's intentions :) > > > Signed-off-by: Markus Burri <markus.burri@mt.com> > > > --- > > > .../devicetree/bindings/input/gpio-matrix-keypad.yaml | 5 +++++ > > > 1 file changed, 5 insertions(+) > > > > > > diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml > > > index 745652b..9ea66b3 100644 > > > --- a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml > > > +++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml > > > @@ -51,6 +51,11 @@ properties: > > > (Legacy property supported: "linux,wakeup") > > > default: false > > > > > > + gpio-activelow: > > > + type: boolean > > > + description: The GPIOs are low active. > > > + default: false > > > > What you want is a flag, not a boolean here btw. Flags you can check for > > the presence of, booleans you cannot. > > The behavior is fixed. What do you mean by "fixed"? Corrected, unchangeable or something else? > If the flag is true GPIO is assumed to be active > low, otherwise (and in the absence of the flag) GPIO is assumed to be > active high. Flags cannot be false only true or absent, what you have described here is a boolean. > > > > > > + > > > debounce-delay-ms: > > > description: Debounce interval in milliseconds. > > > default: 0 > > > -- > > > 2.39.5 > > > > > Thanks. > > -- > Dmitry
matrix_keypad with high capacity need a longer settle time after enable
all columns.
Add optional property to specify the settle time
Signed-off-by: Markus Burri <markus.burri@mt.com>
---
.../devicetree/bindings/input/gpio-matrix-keypad.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
index 9ea66b3..0ce89a6 100644
--- a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
+++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml
@@ -66,6 +66,12 @@ properties:
before we can scan keypad after activating column gpio.
default: 0
+ all-cols-on-delay-us:
+ description: |
+ Delay, measured in microseconds, that is needed
+ after activating all column gpios.
+ default: 0
+
drive-inactive-cols:
type: boolean
description: |
--
2.39.5On Tue, Nov 05, 2024 at 02:03:20PM +0100, Markus Burri wrote: > matrix_keypad with high capacity need a longer settle time after enable > all columns. > Add optional property to specify the settle time > > Signed-off-by: Markus Burri <markus.burri@mt.com> > --- > .../devicetree/bindings/input/gpio-matrix-keypad.yaml | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml > index 9ea66b3..0ce89a6 100644 > --- a/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml > +++ b/Documentation/devicetree/bindings/input/gpio-matrix-keypad.yaml > @@ -66,6 +66,12 @@ properties: > before we can scan keypad after activating column gpio. > default: 0 > > + all-cols-on-delay-us: > + description: | Don't need '|'. > + Delay, measured in microseconds, that is needed > + after activating all column gpios. Wrap at 80 char. > + default: 0 > + > drive-inactive-cols: > type: boolean > description: | > -- > 2.39.5 >
Matrix_keypad with high capacity need a longer settle time after enable
all columns and re-enabling interrupts.
This to give time stable the system and not generate interrupts.
Add a new optional device-tree property to configure the time before
enabling interrupts after disable all columns.
The default is no delay.
Signed-off-by: Markus Burri <markus.burri@mt.com>
---
drivers/input/keyboard/matrix_keypad.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
index 2818915..46873b8 100644
--- a/drivers/input/keyboard/matrix_keypad.c
+++ b/drivers/input/keyboard/matrix_keypad.c
@@ -27,6 +27,7 @@ struct matrix_keypad {
unsigned int row_shift;
unsigned int col_scan_delay_us;
+ unsigned int all_cols_on_delay_us;
/* key debounce interval in milli-second */
unsigned int debounce_ms;
bool drive_inactive_cols;
@@ -78,6 +79,9 @@ static void activate_all_cols(struct matrix_keypad *keypad, bool on)
for (col = 0; col < keypad->num_col_gpios; col++)
__activate_col(keypad, col, on);
+
+ if (on && keypad->all_cols_on_delay_us)
+ fsleep(keypad->all_cols_on_delay_us);
}
static bool row_asserted(struct matrix_keypad *keypad, int row)
@@ -403,6 +407,8 @@ static int matrix_keypad_probe(struct platform_device *pdev)
&keypad->debounce_ms);
device_property_read_u32(&pdev->dev, "col-scan-delay-us",
&keypad->col_scan_delay_us);
+ device_property_read_u32(&pdev->dev, "all-cols-on-delay-us",
+ &keypad->all_cols_on_delay_us);
err = matrix_keypad_init_gpio(pdev, keypad);
if (err)
--
2.39.5For a setup where the matrix keypad is connected over a slow interface
(e.g. a gpio-expansion over i2c), the scan can take a longer time to read.
Interrupts need to be disabled during scan. And therefore changes in this
period are not detected.
To improve this situation, scan the matrix again if the row state changed
during interrupts disabled.
Signed-off-by: Markus Burri <markus.burri@mt.com>
---
drivers/input/keyboard/matrix_keypad.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
index 46873b8..93c8404 100644
--- a/drivers/input/keyboard/matrix_keypad.c
+++ b/drivers/input/keyboard/matrix_keypad.c
@@ -126,6 +126,10 @@ static void matrix_keypad_scan(struct work_struct *work)
const unsigned short *keycodes = input_dev->keycode;
uint32_t new_state[MATRIX_MAX_COLS];
int row, col, code;
+ u32 init_row_state, new_row_state;
+
+ /* read initial row state to detect changes between scan */
+ init_row_state = read_row_state(keypad);
/* de-activate all columns for scanning */
activate_all_cols(keypad, false);
@@ -174,6 +178,19 @@ static void matrix_keypad_scan(struct work_struct *work)
keypad->scan_pending = false;
enable_row_irqs(keypad);
spin_unlock_irq(&keypad->lock);
+
+ /* read new row state and detect if value has changed */
+ new_row_state = read_row_state(keypad);
+ if (init_row_state != new_row_state) {
+ spin_lock_irq(&keypad->lock);
+ if (likely(!keypad->scan_pending && !keypad->stopped)) {
+ disable_row_irqs(keypad);
+ keypad->scan_pending = true;
+ schedule_delayed_work(&keypad->work,
+ msecs_to_jiffies(keypad->debounce_ms));
+ }
+ spin_unlock_irq(&keypad->lock);
+ }
}
static irqreturn_t matrix_keypad_interrupt(int irq, void *id)
--
2.39.5On Tue, Nov 05, 2024 at 02:03:22PM +0100, Markus Burri wrote:
> For a setup where the matrix keypad is connected over a slow interface
> (e.g. a gpio-expansion over i2c), the scan can take a longer time to read.
>
> Interrupts need to be disabled during scan. And therefore changes in this
> period are not detected.
> To improve this situation, scan the matrix again if the row state changed
> during interrupts disabled.
And what if the state changes yet again during the 2nd scan attempt?
>
> Signed-off-by: Markus Burri <markus.burri@mt.com>
> ---
> drivers/input/keyboard/matrix_keypad.c | 17 +++++++++++++++++
> 1 file changed, 17 insertions(+)
>
> diff --git a/drivers/input/keyboard/matrix_keypad.c b/drivers/input/keyboard/matrix_keypad.c
> index 46873b8..93c8404 100644
> --- a/drivers/input/keyboard/matrix_keypad.c
> +++ b/drivers/input/keyboard/matrix_keypad.c
> @@ -126,6 +126,10 @@ static void matrix_keypad_scan(struct work_struct *work)
> const unsigned short *keycodes = input_dev->keycode;
> uint32_t new_state[MATRIX_MAX_COLS];
> int row, col, code;
> + u32 init_row_state, new_row_state;
> +
> + /* read initial row state to detect changes between scan */
> + init_row_state = read_row_state(keypad);
>
> /* de-activate all columns for scanning */
> activate_all_cols(keypad, false);
> @@ -174,6 +178,19 @@ static void matrix_keypad_scan(struct work_struct *work)
> keypad->scan_pending = false;
> enable_row_irqs(keypad);
> spin_unlock_irq(&keypad->lock);
> +
> + /* read new row state and detect if value has changed */
> + new_row_state = read_row_state(keypad);
> + if (init_row_state != new_row_state) {
> + spin_lock_irq(&keypad->lock);
> + if (likely(!keypad->scan_pending && !keypad->stopped)) {
> + disable_row_irqs(keypad);
> + keypad->scan_pending = true;
> + schedule_delayed_work(&keypad->work,
> + msecs_to_jiffies(keypad->debounce_ms));
> + }
> + spin_unlock_irq(&keypad->lock);
> + }
> }
>
> static irqreturn_t matrix_keypad_interrupt(int irq, void *id)
> --
> 2.39.5
>
--
Dmitry
© 2016 - 2024 Red Hat, Inc.