Devicetree
 help / color / mirror / Atom feed
* [PATCH RFC 0/4] Add regulator support to gpio-keys
@ 2026-05-08 12:53 Griffin Kroah-Hartman
  2026-05-08 12:53 ` [PATCH RFC 1/4] regulator: add devm_fwnode family of functions Griffin Kroah-Hartman
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Griffin Kroah-Hartman @ 2026-05-08 12:53 UTC (permalink / raw)
  To: Dmitry Torokhov, Liam Girdwood, Mark Brown, Bjorn Andersson,
	Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Luca Weiss
  Cc: linux-input, linux-kernel, linux-arm-msm, devicetree,
	Griffin Kroah-Hartman

This patchset adds regulator support to gpio-keys to ensure that the
regulator powering the Hall Effect sensor IC ("flip cover sensor") is on
when the input device is used.

Marked as RFC because we may want to represent a Hall Effect sensor with
a separate compatible + driver, and not use gpio-keys.

There was also a comment around the usage of of_regulator_get() in the
follow thread:
https://lore.kernel.org/all/DI5HOWHCCKKD.1SQFAA3L4QFDI@fairphone.com/

Signed-off-by: Griffin Kroah-Hartman <griffin.kroah@fairphone.com>
---
Griffin Kroah-Hartman (4):
      regulator: add devm_fwnode family of functions
      dt-bindings: input: gpio-keys: Add vdd-supply
      Input: gpio-keys - add regulator to gpio_keys
      arm64: dts: qcom: milos-fairphone-fp6: add supply for Hall Effect sensor

 .../devicetree/bindings/input/gpio-keys.yaml       |  4 ++
 arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts   |  4 +-
 drivers/input/keyboard/gpio_keys.c                 | 41 ++++++++++++++
 drivers/regulator/devres.c                         | 66 ++++++++++++++++++++++
 include/linux/gpio_keys.h                          |  2 +
 include/linux/regulator/consumer.h                 | 30 ++++++++++
 6 files changed, 144 insertions(+), 3 deletions(-)
---
base-commit: 17c7841d09ee7d33557fd075562d9289b6018c90
change-id: 20260410-gpiokeys-vdd-supply-855d6ca8534b

Best regards,
-- 
Griffin Kroah-Hartman <griffin.kroah@fairphone.com>


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH RFC 1/4] regulator: add devm_fwnode family of functions
  2026-05-08 12:53 [PATCH RFC 0/4] Add regulator support to gpio-keys Griffin Kroah-Hartman
@ 2026-05-08 12:53 ` Griffin Kroah-Hartman
  2026-05-08 13:31   ` Mark Brown
  2026-05-08 21:08   ` sashiko-bot
  2026-05-08 12:53 ` [PATCH RFC 2/4] dt-bindings: input: gpio-keys: Add vdd-supply Griffin Kroah-Hartman
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 10+ messages in thread
From: Griffin Kroah-Hartman @ 2026-05-08 12:53 UTC (permalink / raw)
  To: Dmitry Torokhov, Liam Girdwood, Mark Brown, Bjorn Andersson,
	Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Luca Weiss
  Cc: linux-input, linux-kernel, linux-arm-msm, devicetree,
	Griffin Kroah-Hartman

Add devm_fwnode_regulator_get and variants.

These function wrappers allow regulators to be accessed from the fwnode
struct without any casts.

Signed-off-by: Griffin Kroah-Hartman <griffin.kroah@fairphone.com>
---
 drivers/regulator/devres.c         | 66 ++++++++++++++++++++++++++++++++++++++
 include/linux/regulator/consumer.h | 30 +++++++++++++++++
 2 files changed, 96 insertions(+)

diff --git a/drivers/regulator/devres.c b/drivers/regulator/devres.c
index 615deba5d22c8ce190c69081c94651d8df93d002..f525b1f48bcf18593798f6fa520ccdfc656f6b6c 100644
--- a/drivers/regulator/devres.c
+++ b/drivers/regulator/devres.c
@@ -11,6 +11,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/regulator/driver.h>
 #include <linux/module.h>
+#include <linux/of.h>
 
 #include "internal.h"
 
@@ -803,4 +804,69 @@ struct regulator *devm_of_regulator_get_optional(struct device *dev, struct devi
 	return _devm_of_regulator_get(dev, node, id, OPTIONAL_GET);
 }
 EXPORT_SYMBOL_GPL(devm_of_regulator_get_optional);
+
+static struct regulator *_devm_fwnode_regulator_get(struct device *dev,
+						    struct fwnode_handle *fwnode,
+						    const char *id,
+						    enum regulator_get_type get_type)
+{
+	if (is_of_node(fwnode))
+		return _devm_of_regulator_get(dev, to_of_node(fwnode), id, get_type);
+
+	return ERR_PTR(-ENODEV);
+}
+
+/**
+ * devm_fwnode_regulator_get() - get resource managed regulator from firmware node
+ * @dev: device to supply
+ * @fwnode: firmware node to get regulator from
+ * @id: supply name or regulator ID.
+ *
+ * Managed of_regulator_get(). Regulators returned from this
+ * function are automatically regulator_put() on driver detach. See
+ * of_regulator_get() for more information.
+ */
+struct regulator *devm_fwnode_regulator_get(struct device *dev,
+					    struct fwnode_handle *fwnode,
+					    const char *id)
+{
+	return _devm_fwnode_regulator_get(dev, fwnode, id, NORMAL_GET);
+}
+EXPORT_SYMBOL_GPL(devm_fwnode_regulator_get);
+
+/**
+ * devm_fwnode_regulator_get_exclusive() - get resource managed regulator from firmware node
+ * @dev: device to supply
+ * @fwnode: firmware node to get regulator from
+ * @id: supply name or regulator ID.
+ *
+ * Managed of_regulator_get_exclusive(). Regulators returned from this
+ * function are automatically regulator_put() on driver detach. See
+ * of_regulator_get_exclusive() for more information.
+ */
+struct regulator *devm_fwnode_regulator_get_exclusive(struct device *dev,
+						      struct fwnode_handle *fwnode,
+						      const char *id)
+{
+	return _devm_fwnode_regulator_get(dev, fwnode, id, EXCLUSIVE_GET);
+}
+EXPORT_SYMBOL_GPL(devm_fwnode_regulator_get_exclusive);
+
+/**
+ * devm_fwnode_regulator_get_optional() - get resource managed regulator from firmware node
+ * @dev: device to supply
+ * @fwnode: firmware node to get regulator from
+ * @id: supply name or regulator ID.
+ *
+ * Managed of_regulator_get_optional(). Regulators returned from this
+ * function are automatically regulator_put() on driver detach. See
+ * of_regulator_get_optional() for more information.
+ */
+struct regulator *devm_fwnode_regulator_get_optional(struct device *dev,
+						     struct fwnode_handle *fwnode,
+						     const char *id)
+{
+	return _devm_fwnode_regulator_get(dev, fwnode, id, OPTIONAL_GET);
+}
+EXPORT_SYMBOL_GPL(devm_fwnode_regulator_get_optional);
 #endif
diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
index 56fe2693d9b2284d04ebae50165f9aa7b1b3fee4..7c3aedf7f4902c4e3e8e2f5ecf6e4323b77658f3 100644
--- a/include/linux/regulator/consumer.h
+++ b/include/linux/regulator/consumer.h
@@ -691,6 +691,15 @@ struct regulator *__must_check devm_of_regulator_get_optional(struct device *dev
 							      const char *id);
 int __must_check of_regulator_bulk_get_all(struct device *dev, struct device_node *np,
 					   struct regulator_bulk_data **consumers);
+struct regulator *__must_check devm_fwnode_regulator_get(struct device *dev,
+							 struct fwnode_handle *fwnode,
+							 const char *id);
+struct regulator *__must_check devm_fwnode_regulator_get_exclusive(struct device *dev,
+								   struct fwnode_handle *fwnode,
+								   const char *id);
+struct regulator *__must_check devm_fwnode_regulator_get_optional(struct device *dev,
+								  struct fwnode_handle *fwnode,
+								  const char *id);
 #else
 static inline struct regulator *__must_check of_regulator_get_optional(struct device *dev,
 								       struct device_node *node,
@@ -712,6 +721,27 @@ static inline int of_regulator_bulk_get_all(struct device *dev, struct device_no
 	return 0;
 }
 
+static inline struct regulator *__must_check
+devm_fwnode_regulator_get(struct device *dev, struct fwnode_handle *fwnode,
+	const char *id)
+{
+	return NULL;
+}
+
+static inline struct regulator *__must_check
+devm_fwnode_regulator_get_exclusive(struct device *dev, struct fwnode_handle *fwnode,
+	const char *id)
+{
+	return NULL;
+}
+
+static inline struct regulator *__must_check
+devm_fwnode_regulator_get_optional(struct device *dev, struct fwnode_handle *fwnode,
+	const char *id)
+{
+	return NULL;
+}
+
 #endif
 
 static inline int regulator_set_voltage_triplet(struct regulator *regulator,

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH RFC 2/4] dt-bindings: input: gpio-keys: Add vdd-supply
  2026-05-08 12:53 [PATCH RFC 0/4] Add regulator support to gpio-keys Griffin Kroah-Hartman
  2026-05-08 12:53 ` [PATCH RFC 1/4] regulator: add devm_fwnode family of functions Griffin Kroah-Hartman
@ 2026-05-08 12:53 ` Griffin Kroah-Hartman
  2026-05-08 12:53 ` [PATCH RFC 3/4] Input: gpio-keys - add regulator to gpio_keys Griffin Kroah-Hartman
  2026-05-08 12:53 ` [PATCH RFC 4/4] arm64: dts: qcom: milos-fairphone-fp6: add supply for Hall Effect sensor Griffin Kroah-Hartman
  3 siblings, 0 replies; 10+ messages in thread
From: Griffin Kroah-Hartman @ 2026-05-08 12:53 UTC (permalink / raw)
  To: Dmitry Torokhov, Liam Girdwood, Mark Brown, Bjorn Andersson,
	Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Luca Weiss
  Cc: linux-input, linux-kernel, linux-arm-msm, devicetree,
	Griffin Kroah-Hartman

Allow the use of vdd-supply regulators for gpio-keys. For example, the
Fairphone Gen. 6 Hall effect sensor requires this functionality.

Signed-off-by: Griffin Kroah-Hartman <griffin.kroah@fairphone.com>
---
 Documentation/devicetree/bindings/input/gpio-keys.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/input/gpio-keys.yaml b/Documentation/devicetree/bindings/input/gpio-keys.yaml
index cc78c2152921308fe0cad3e29ca78a5fad08f066..9c5ec1e7d0fc7ea236c161c7bd5a041aafdd07a1 100644
--- a/Documentation/devicetree/bindings/input/gpio-keys.yaml
+++ b/Documentation/devicetree/bindings/input/gpio-keys.yaml
@@ -102,6 +102,10 @@ patternProperties:
           which can be disabled to suppress events from the button.
         type: boolean
 
+      vdd-supply:
+        description:
+          Regulator that provides a VDD power supply.
+
     required:
       - linux,code
 

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH RFC 3/4] Input: gpio-keys - add regulator to gpio_keys
  2026-05-08 12:53 [PATCH RFC 0/4] Add regulator support to gpio-keys Griffin Kroah-Hartman
  2026-05-08 12:53 ` [PATCH RFC 1/4] regulator: add devm_fwnode family of functions Griffin Kroah-Hartman
  2026-05-08 12:53 ` [PATCH RFC 2/4] dt-bindings: input: gpio-keys: Add vdd-supply Griffin Kroah-Hartman
@ 2026-05-08 12:53 ` Griffin Kroah-Hartman
  2026-05-08 13:44   ` Mark Brown
  2026-05-08 21:36   ` sashiko-bot
  2026-05-08 12:53 ` [PATCH RFC 4/4] arm64: dts: qcom: milos-fairphone-fp6: add supply for Hall Effect sensor Griffin Kroah-Hartman
  3 siblings, 2 replies; 10+ messages in thread
From: Griffin Kroah-Hartman @ 2026-05-08 12:53 UTC (permalink / raw)
  To: Dmitry Torokhov, Liam Girdwood, Mark Brown, Bjorn Andersson,
	Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Luca Weiss
  Cc: linux-input, linux-kernel, linux-arm-msm, devicetree,
	Griffin Kroah-Hartman

Allow gpio-keys to have vdd power suppy through regulators.

Signed-off-by: Griffin Kroah-Hartman <griffin.kroah@fairphone.com>
---
 drivers/input/keyboard/gpio_keys.c | 41 ++++++++++++++++++++++++++++++++++++++
 include/linux/gpio_keys.h          |  2 ++
 2 files changed, 43 insertions(+)

diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index e196174856796b391f14c31da9b2fee5ff742172..2199ce7455cfc763569e173d4d18da92507f38d3 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -28,6 +28,7 @@
 #include <linux/of.h>
 #include <linux/of_irq.h>
 #include <linux/spinlock.h>
+#include <linux/regulator/consumer.h>
 #include <dt-bindings/input/gpio-keys.h>
 
 struct gpio_button_data {
@@ -729,6 +730,7 @@ static int gpio_keys_open(struct input_dev *input)
 	struct gpio_keys_drvdata *ddata = input_get_drvdata(input);
 	const struct gpio_keys_platform_data *pdata = ddata->pdata;
 	int error;
+	int i;
 
 	if (pdata->enable) {
 		error = pdata->enable(input->dev.parent);
@@ -736,19 +738,48 @@ static int gpio_keys_open(struct input_dev *input)
 			return error;
 	}
 
+	for (i = 0; i < pdata->nbuttons; i++) {
+		const struct gpio_keys_button *button = &pdata->buttons[i];
+
+		if (!button->regulator)
+			continue;
+		error = regulator_enable(button->regulator);
+		if (error)
+			goto reg_err;
+	}
+
 	/* Report current state of buttons that are connected to GPIOs */
 	gpio_keys_report_state(ddata);
 
 	return 0;
+
+reg_err:
+	for (--i; i >= 0; i--) {
+		const struct gpio_keys_button *button = &pdata->buttons[i];
+
+		if (!button->regulator)
+			continue;
+		regulator_disable(button->regulator);
+	}
+	return error;
 }
 
 static void gpio_keys_close(struct input_dev *input)
 {
 	struct gpio_keys_drvdata *ddata = input_get_drvdata(input);
 	const struct gpio_keys_platform_data *pdata = ddata->pdata;
+	int i;
 
 	if (pdata->disable)
 		pdata->disable(input->dev.parent);
+
+	for (i = 0; i < pdata->nbuttons; i++) {
+		const struct gpio_keys_button *button = &pdata->buttons[i];
+
+		if (!button->regulator)
+			continue;
+		regulator_disable(button->regulator);
+	}
 }
 
 /*
@@ -829,6 +860,16 @@ gpio_keys_get_devtree_pdata(struct device *dev)
 					 &button->debounce_interval))
 			button->debounce_interval = 5;
 
+		if (fwnode_property_present(child, "vdd-supply")) {
+			button->regulator = devm_fwnode_regulator_get_optional(dev, child, "vdd");
+			if (IS_ERR(button->regulator)) {
+				if (PTR_ERR(button->regulator) != -ENODEV)
+					return dev_err_ptr_probe(dev, PTR_ERR(button->regulator),
+								 "Failed to get regulator\n");
+				button->regulator = NULL;
+			}
+		}
+
 		button++;
 	}
 
diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
index 80fa930b04c6795eb7c6143a79655a6f918446eb..66e786ad7eba407dbea2545941f5b6e464a4f61c 100644
--- a/include/linux/gpio_keys.h
+++ b/include/linux/gpio_keys.h
@@ -3,6 +3,7 @@
 #define _GPIO_KEYS_H
 
 #include <linux/types.h>
+#include <linux/regulator/consumer.h>
 
 struct device;
 
@@ -36,6 +37,7 @@ struct gpio_keys_button {
 	int value;
 	unsigned int irq;
 	unsigned int wakeirq;
+	struct regulator *regulator;
 };
 
 /**

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH RFC 4/4] arm64: dts: qcom: milos-fairphone-fp6: add supply for Hall Effect sensor
  2026-05-08 12:53 [PATCH RFC 0/4] Add regulator support to gpio-keys Griffin Kroah-Hartman
                   ` (2 preceding siblings ...)
  2026-05-08 12:53 ` [PATCH RFC 3/4] Input: gpio-keys - add regulator to gpio_keys Griffin Kroah-Hartman
@ 2026-05-08 12:53 ` Griffin Kroah-Hartman
  2026-05-08 21:56   ` sashiko-bot
  3 siblings, 1 reply; 10+ messages in thread
From: Griffin Kroah-Hartman @ 2026-05-08 12:53 UTC (permalink / raw)
  To: Dmitry Torokhov, Liam Girdwood, Mark Brown, Bjorn Andersson,
	Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Luca Weiss
  Cc: linux-input, linux-kernel, linux-arm-msm, devicetree,
	Griffin Kroah-Hartman

Add vdd-supply for the Hall Effect sensor in gpio-keys so that power for
the sensor will be enabled when it's in use.

With this, we can drop the regulator-always-on for vreg_l10b.

Signed-off-by: Griffin Kroah-Hartman <griffin.kroah@fairphone.com>
---
 arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts b/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts
index c1899db46e714137d7849b3b043062fe8b05cc42..ae6900c3f75c64ea5b4feadf38df101abb43c1ea 100644
--- a/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts
+++ b/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts
@@ -32,13 +32,13 @@ gpio-keys {
 		pinctrl-0 = <&volume_up_default>, <&hall_sensor_default>;
 		pinctrl-names = "default";
 
-		/* Powered by the always-on vreg_l10b */
 		event-hall-sensor {
 			label = "Hall Effect Sensor";
 			gpios = <&tlmm 70 GPIO_ACTIVE_LOW>;
 			linux,input-type = <EV_SW>;
 			linux,code = <SW_LID>;
 			linux,can-disable;
+			vdd-supply = <&vreg_l10b>;
 			wakeup-source;
 		};
 
@@ -326,8 +326,6 @@ vreg_l10b: ldo10 {
 			regulator-min-microvolt = <1800000>;
 			regulator-max-microvolt = <1800000>;
 			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
-			/* Hall sensor VDD */
-			regulator-always-on;
 		};
 
 		vreg_l11b: ldo11 {

-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH RFC 1/4] regulator: add devm_fwnode family of functions
  2026-05-08 12:53 ` [PATCH RFC 1/4] regulator: add devm_fwnode family of functions Griffin Kroah-Hartman
@ 2026-05-08 13:31   ` Mark Brown
  2026-05-08 21:08   ` sashiko-bot
  1 sibling, 0 replies; 10+ messages in thread
From: Mark Brown @ 2026-05-08 13:31 UTC (permalink / raw)
  To: Griffin Kroah-Hartman
  Cc: Dmitry Torokhov, Liam Girdwood, Bjorn Andersson, Konrad Dybcio,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Luca Weiss,
	linux-input, linux-kernel, linux-arm-msm, devicetree

[-- Attachment #1: Type: text/plain, Size: 235 bytes --]

On Fri, May 08, 2026 at 02:53:13PM +0200, Griffin Kroah-Hartman wrote:
> Add devm_fwnode_regulator_get and variants.
> 
> These function wrappers allow regulators to be accessed from the fwnode
> struct without any casts.

Why?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH RFC 3/4] Input: gpio-keys - add regulator to gpio_keys
  2026-05-08 12:53 ` [PATCH RFC 3/4] Input: gpio-keys - add regulator to gpio_keys Griffin Kroah-Hartman
@ 2026-05-08 13:44   ` Mark Brown
  2026-05-08 21:36   ` sashiko-bot
  1 sibling, 0 replies; 10+ messages in thread
From: Mark Brown @ 2026-05-08 13:44 UTC (permalink / raw)
  To: Griffin Kroah-Hartman
  Cc: Dmitry Torokhov, Liam Girdwood, Bjorn Andersson, Konrad Dybcio,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Luca Weiss,
	linux-input, linux-kernel, linux-arm-msm, devicetree

[-- Attachment #1: Type: text/plain, Size: 892 bytes --]

On Fri, May 08, 2026 at 02:53:15PM +0200, Griffin Kroah-Hartman wrote:

> +		if (fwnode_property_present(child, "vdd-supply")) {
> +			button->regulator = devm_fwnode_regulator_get_optional(dev, child, "vdd");
> +			if (IS_ERR(button->regulator)) {

As well as the issue I mentioned on a prior thread with this assigning a
non-physical "vdd" name to the single supply that these components can
have (which has had issues in the past, AHCI being one of the more
painful) the fact that this is fwnode means that this opens up support
for using this with ACPI which is very problematic given that ACPI has a
strong model of how regulators should work which is that they should not
be OS visible at all.  That probably needs more addressing in the prior
regulator patch, that needs a bit more motivation and discussion about
the issues with trying to do a regulator interface firmware neutrally.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH RFC 1/4] regulator: add devm_fwnode family of functions
  2026-05-08 12:53 ` [PATCH RFC 1/4] regulator: add devm_fwnode family of functions Griffin Kroah-Hartman
  2026-05-08 13:31   ` Mark Brown
@ 2026-05-08 21:08   ` sashiko-bot
  1 sibling, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-05-08 21:08 UTC (permalink / raw)
  To: Griffin Kroah-Hartman
  Cc: robh, devicetree, dmitry.torokhov, krzk+dt, conor+dt, linux-input

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The fallback stubs for `devm_fwnode_regulator_get` and its variants incorrectly return `NULL` instead of `ERR_PTR(-ENODEV)` when `CONFIG_OF` is disabled but `CONFIG_REGULATOR` is enabled, leading to a guaranteed kernel panic on non-OF systems.
--

commit f8899ceec7e07e935c3ae44032450cd2b8f68352
Author: Griffin Kroah-Hartman <griffin.kroah@fairphone.com>

regulator: add devm_fwnode family of functions

Add devm_fwnode_regulator_get and variants.

These function wrappers allow regulators to be accessed from the fwnode
struct without any casts.

> diff --git a/include/linux/regulator/consumer.h b/include/linux/regulator/consumer.h
> --- a/include/linux/regulator/consumer.h
> +++ b/include/linux/regulator/consumer.h
[ ... ]
> +static inline struct regulator *__must_check
> +devm_fwnode_regulator_get(struct device *dev, struct fwnode_handle *fwnode,
> +	const char *id)
> +{
> +	return NULL;
> +}

Will returning NULL here instead of ERR_PTR(-ENODEV) cause a panic on non-OF
systems?

If CONFIG_REGULATOR is enabled but CONFIG_OF is disabled, a driver calling
this will get NULL. Since drivers typically check the return value with
IS_ERR(), IS_ERR(NULL) evaluates to false. The driver would assume it
successfully acquired a valid regulator.

Later, when the driver calls regulator_enable(), it will pass this NULL
pointer. Since CONFIG_REGULATOR is enabled, regulator_enable() is not stubbed
out and will unconditionally dereference the pointer, leading to a NULL
pointer dereference.

Should these stubs return ERR_PTR(-ENODEV) instead to safely signal the
absence of a regulator and match the expected regulator API semantics?

> +
> +static inline struct regulator *__must_check
> +devm_fwnode_regulator_get_exclusive(struct device *dev, struct fwnode_handle *fwnode,
> +	const char *id)
> +{
> +	return NULL;
> +}
> +
> +static inline struct regulator *__must_check
> +devm_fwnode_regulator_get_optional(struct device *dev, struct fwnode_handle *fwnode,
> +	const char *id)
> +{
> +	return NULL;
> +}

Does returning NULL for the optional and exclusive variants violate the
regulator API semantics which require returning ERR_PTR(-ENODEV) to safely
signal the absence of a regulator?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508-gpiokeys-vdd-supply-v1-0-0bb32e8e6428@fairphone.com?part=1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH RFC 3/4] Input: gpio-keys - add regulator to gpio_keys
  2026-05-08 12:53 ` [PATCH RFC 3/4] Input: gpio-keys - add regulator to gpio_keys Griffin Kroah-Hartman
  2026-05-08 13:44   ` Mark Brown
@ 2026-05-08 21:36   ` sashiko-bot
  1 sibling, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-05-08 21:36 UTC (permalink / raw)
  To: Griffin Kroah-Hartman
  Cc: linux-input, robh, krzk+dt, dmitry.torokhov, conor+dt, devicetree

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Tying the regulator's power state to the input device's open()/close() lifecycle leaves interrupts active while the hardware is unpowered, causing an interrupt storm.
- [Medium] System wakeup functionality is broken because regulators are disabled if the input device is closed.
- [Medium] Unsafe hardware access during system resume causes spurious key events.
- [Medium] Missing pdata->disable() in the gpio_keys_open() error path leads to a resource leak.
- [Low] Incorrect teardown order in gpio_keys_close() violates LIFO symmetry.
--

commit d47e6012a488ac21d0005ba6dd4aa95a285831d3
Author: Griffin Kroah-Hartman <griffin.kroah@fairphone.com>

Input: gpio-keys - add regulator to gpio_keys

This commit allows gpio-keys to have a vdd power supply managed through
regulators.

> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> index e196174856796..2199ce7455cfc 100644
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c

[ ... ]

> @@ -729,6 +730,7 @@ static int gpio_keys_open(struct input_dev *input)
>  	struct gpio_keys_drvdata *ddata = input_get_drvdata(input);
>  	const struct gpio_keys_platform_data *pdata = ddata->pdata;
>  	int error;
> +	int i;
>  
>  	if (pdata->enable) {
>  		error = pdata->enable(input->dev.parent);
> @@ -736,19 +738,48 @@ static int gpio_keys_open(struct input_dev *input)
>  			return error;
>  	}
>  
> +	for (i = 0; i < pdata->nbuttons; i++) {
> +		const struct gpio_keys_button *button = &pdata->buttons[i];
> +
> +		if (!button->regulator)
> +			continue;
> +		error = regulator_enable(button->regulator);
> +		if (error)
> +			goto reg_err;
> +	}

Because interrupts are requested unconditionally in gpio_keys_probe() and
remain unmasked, tying the regulator power state to the input device's
open()/close() lifecycle leaves the physical button unpowered when the device
is closed. Could this allow the GPIO pin to float and trigger an interrupt
storm?

Also, if the input device is closed before suspend, the regulator remains
disabled. Does this prevent an unpowered physical button from generating the
electrical signal required to trigger a wakeup interrupt?

Additionally, gpio_keys_resume() unconditionally calls
gpio_keys_report_state(), which reads the current state of all GPIOs. If the
device is closed, the regulators are powered off. Can reading unpowered GPIOs
return floating states that inject spurious phantom key press events on every
resume?

> +
>  	/* Report current state of buttons that are connected to GPIOs */
>  	gpio_keys_report_state(ddata);
>  
>  	return 0;
> +
> +reg_err:
> +	for (--i; i >= 0; i--) {
> +		const struct gpio_keys_button *button = &pdata->buttons[i];
> +
> +		if (!button->regulator)
> +			continue;
> +		regulator_disable(button->regulator);
> +	}
> +	return error;

If regulator_enable() fails and we jump to reg_err, we return the error but
fail to call pdata->disable(). Does this leave the platform state permanently
active and leak resources since pdata->enable() was called earlier?

>  }
>  
>  static void gpio_keys_close(struct input_dev *input)
>  {
>  	struct gpio_keys_drvdata *ddata = input_get_drvdata(input);
>  	const struct gpio_keys_platform_data *pdata = ddata->pdata;
> +	int i;
>  
>  	if (pdata->disable)
>  		pdata->disable(input->dev.parent);
> +
> +	for (i = 0; i < pdata->nbuttons; i++) {
> +		const struct gpio_keys_button *button = &pdata->buttons[i];
> +
> +		if (!button->regulator)
> +			continue;
> +		regulator_disable(button->regulator);
> +	}
>  }

Is the teardown order correct here? In gpio_keys_open(), pdata->enable() is
called before the regulators are enabled. Should gpio_keys_close() reverse
this sequence by disabling the regulators before calling pdata->disable() to
maintain LIFO symmetry?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508-gpiokeys-vdd-supply-v1-0-0bb32e8e6428@fairphone.com?part=3

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH RFC 4/4] arm64: dts: qcom: milos-fairphone-fp6: add supply for Hall Effect sensor
  2026-05-08 12:53 ` [PATCH RFC 4/4] arm64: dts: qcom: milos-fairphone-fp6: add supply for Hall Effect sensor Griffin Kroah-Hartman
@ 2026-05-08 21:56   ` sashiko-bot
  0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-05-08 21:56 UTC (permalink / raw)
  To: Griffin Kroah-Hartman
  Cc: robh, conor+dt, linux-input, krzk+dt, devicetree, dmitry.torokhov

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing `regulator-always-on` from the Hall sensor's supply causes interrupt storms, spurious events, and broken system wakeup when the input device is closed.
--

commit 35bf0d08eef4da2139153f5425e351a93d9d591d
Author: Griffin Kroah-Hartman <griffin.kroah@fairphone.com>

arm64: dts: qcom: milos-fairphone-fp6: add supply for Hall Effect sensor

This commit adds a vdd-supply property to the Hall Effect sensor and removes
the regulator-always-on property from vreg_l10b, relying on the gpio-keys
driver to manage power.

> diff --git a/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts b/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts
> index c1899db46e714..ae6900c3f75c6 100644
> --- a/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts
> +++ b/arch/arm64/boot/dts/qcom/milos-fairphone-fp6.dts
> @@ -32,13 +32,13 @@ gpio-keys {
>  		pinctrl-0 = <&volume_up_default>, <&hall_sensor_default>;
>  		pinctrl-names = "default";
>  
> -		/* Powered by the always-on vreg_l10b */
>  		event-hall-sensor {
>  			label = "Hall Effect Sensor";
>  			gpios = <&tlmm 70 GPIO_ACTIVE_LOW>;
>  			linux,input-type = <EV_SW>;
>  			linux,code = <SW_LID>;
>  			linux,can-disable;
> +			vdd-supply = <&vreg_l10b>;

When the device is opened, the driver reads the GPIO state immediately after
enabling the regulator. Without a startup-delay-us property on the regulator
to allow the voltage to stabilize, could this read an uninitialized state and
generate spurious lid events?

>  			wakeup-source;
>  		};
>  
[ ... ]
> @@ -326,8 +326,6 @@ vreg_l10b: ldo10 {
>  			regulator-min-microvolt = <1800000>;
>  			regulator-max-microvolt = <1800000>;
>  			regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> -			/* Hall sensor VDD */
> -			regulator-always-on;
>  		};

Could removing this property cause an interrupt storm?

The gpio-keys driver requests and unmasks the GPIO interrupt during probe, but
only enables the vdd-supply regulator when the input device is explicitly
opened by userspace.

If the device is closed, the regulator is disabled. Cutting power to the Hall
sensor while its edge-triggered interrupt remains active might cause the GPIO
line to float. A floating line can pick up electrical noise and trigger a
continuous interrupt storm.

Additionally, does this break the system's ability to wake up?

If the system suspends while the device is closed, the sensor remains
unpowered. Even though the wake IRQ is armed due to the wakeup-source
property, an unpowered sensor wouldn't be able to detect lid events to wake
the system.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508-gpiokeys-vdd-supply-v1-0-0bb32e8e6428@fairphone.com?part=4

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-05-08 21:56 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-08 12:53 [PATCH RFC 0/4] Add regulator support to gpio-keys Griffin Kroah-Hartman
2026-05-08 12:53 ` [PATCH RFC 1/4] regulator: add devm_fwnode family of functions Griffin Kroah-Hartman
2026-05-08 13:31   ` Mark Brown
2026-05-08 21:08   ` sashiko-bot
2026-05-08 12:53 ` [PATCH RFC 2/4] dt-bindings: input: gpio-keys: Add vdd-supply Griffin Kroah-Hartman
2026-05-08 12:53 ` [PATCH RFC 3/4] Input: gpio-keys - add regulator to gpio_keys Griffin Kroah-Hartman
2026-05-08 13:44   ` Mark Brown
2026-05-08 21:36   ` sashiko-bot
2026-05-08 12:53 ` [PATCH RFC 4/4] arm64: dts: qcom: milos-fairphone-fp6: add supply for Hall Effect sensor Griffin Kroah-Hartman
2026-05-08 21:56   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox