Re: [PATCH v5 2/9] button: qcom-pmic: introduce Qualcomm PMIC button driver

2023-12-01 Thread Neil Armstrong

On 30/11/2023 21:22, Caleb Connolly wrote:

Qualcomm PMICs include a "pon" function which handles two buttons, the
power button and "resin" button (usually volume down). Introduce a new
driver following upstream Linux DT to enable these and map them to Enter
and Down respectively to enable use in boot menus.

Signed-off-by: Caleb Connolly 
---
  MAINTAINERS   |   1 +
  drivers/button/Kconfig|   9 +++
  drivers/button/Makefile   |   1 +
  drivers/button/button-qcom-pmic.c | 165 ++
  4 files changed, 176 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index f6d63c8ab563..8cd102eaa070 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -572,6 +572,7 @@ M:  Neil Armstrong 
  R:Sumit Garg 
  S:Maintained
  F:arch/arm/mach-snapdragon/
+F: drivers/button/button-qcom-pmic.c
  F:drivers/clk/qcom/
  F:drivers/gpio/msm_gpio.c
  F:drivers/mmc/msm_sdhci.c
diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig
index 8ce2de37d62a..097b05f822e7 100644
--- a/drivers/button/Kconfig
+++ b/drivers/button/Kconfig
@@ -27,4 +27,13 @@ config BUTTON_GPIO
  The GPIO driver must used driver model. Buttons are configured using
  the device tree.
  
+config BUTTON_QCOM_PMIC

+   bool "Qualcomm power button"
+   depends on BUTTON
+   depends on PMIC_QCOM
+   help
+ Enable support for the power and "resin" (usually volume down) buttons
+ on Qualcomm SoCs. These will be configured as the Enter and Down keys
+ respectively, allowing navigation of bootmenu with buttons on device.
+
  endmenu
diff --git a/drivers/button/Makefile b/drivers/button/Makefile
index bbd18af14940..68555081a47a 100644
--- a/drivers/button/Makefile
+++ b/drivers/button/Makefile
@@ -5,3 +5,4 @@
  obj-$(CONFIG_BUTTON) += button-uclass.o
  obj-$(CONFIG_BUTTON_ADC) += button-adc.o
  obj-$(CONFIG_BUTTON_GPIO) += button-gpio.o
+obj-$(CONFIG_BUTTON_QCOM_PMIC) += button-qcom-pmic.o
\ No newline at end of file
diff --git a/drivers/button/button-qcom-pmic.c 
b/drivers/button/button-qcom-pmic.c
new file mode 100644
index ..34a976d1e6c6
--- /dev/null
+++ b/drivers/button/button-qcom-pmic.c
@@ -0,0 +1,165 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Qualcomm generic pmic gpio driver
+ *
+ * (C) Copyright 2015 Mateusz Kulikowski 
+ * (C) Copyright 2023 Linaro Ltd.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define REG_TYPE   0x4
+#define REG_SUBTYPE0x5
+
+struct qcom_pmic_btn_priv {
+   u32 base;
+   u32 status_bit;
+   int code;
+   struct udevice *pmic;
+};
+
+#define PON_INT_RT_STS0x10
+#define KPDPWR_ON_INT_BIT 0
+#define RESIN_ON_INT_BIT  1
+
+#define NODE_IS_PWRKEY(node) (!strncmp(ofnode_get_name(node), "pwrkey", 
strlen("pwrkey")))
+#define NODE_IS_RESIN(node) (!strncmp(ofnode_get_name(node), "resin", 
strlen("resin")))


This is not very pretty, but I don't see better alternative except perhaps
defining a struct with the node name and the properties and simply
match over it, but since there's only 2 buttons it makes it generic for nothing.


+
+static enum button_state_t qcom_pwrkey_get_state(struct udevice *dev)
+{
+   struct qcom_pmic_btn_priv *priv = dev_get_priv(dev);
+
+   int reg = pmic_reg_read(priv->pmic, priv->base + PON_INT_RT_STS);
+
+   if (reg < 0)
+   return 0;
+
+   return (reg & BIT(priv->status_bit)) != 0;
+}
+
+static int qcom_pwrkey_get_code(struct udevice *dev)
+{
+   struct qcom_pmic_btn_priv *priv = dev_get_priv(dev);
+
+   return priv->code;
+}
+
+static int qcom_pwrkey_probe(struct udevice *dev)
+{
+   struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev);
+   struct qcom_pmic_btn_priv *priv = dev_get_priv(dev);
+   ofnode node = dev_ofnode(dev);
+   int ret;
+   u64 base;
+
+   /* Ignore the top-level pon node */
+   if (!uc_plat->label)
+   return 0;
+
+   /* the pwrkey and resin nodes are children of the "pon" node, get the
+* PMIC device to use in pmic_reg_* calls.
+*/ > +  priv->pmic = dev->parent->parent;
+
+   /* Get the address of the parent pon node */
+   base = dev_read_addr(dev->parent);
+   if (base == FDT_ADDR_T_NONE) {
+   printf("%s: Can't find address\n", dev->name);
+   return -EINVAL;
+   }
+
+   priv->base = base;
+
+   /* Do a sanity check */
+   ret = pmic_reg_read(priv->pmic, priv->base + REG_TYPE);
+   if (ret != 0x1 && ret != 0xb) {
+   printf("%s: unexpected PMIC function type %d\n", dev->name, 
ret);
+   return -ENXIO;
+   }
+
+   ret = pmic_reg_read(priv->pmic, priv->base + REG_SUBTYPE);
+   if ((ret & 0x7) == 0) {
+   printf("%s: unexpected PMCI function subtype %d\n", 

[PATCH v5 2/9] button: qcom-pmic: introduce Qualcomm PMIC button driver

2023-11-30 Thread Caleb Connolly
Qualcomm PMICs include a "pon" function which handles two buttons, the
power button and "resin" button (usually volume down). Introduce a new
driver following upstream Linux DT to enable these and map them to Enter
and Down respectively to enable use in boot menus.

Signed-off-by: Caleb Connolly 
---
 MAINTAINERS   |   1 +
 drivers/button/Kconfig|   9 +++
 drivers/button/Makefile   |   1 +
 drivers/button/button-qcom-pmic.c | 165 ++
 4 files changed, 176 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index f6d63c8ab563..8cd102eaa070 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -572,6 +572,7 @@ M:  Neil Armstrong 
 R: Sumit Garg 
 S: Maintained
 F: arch/arm/mach-snapdragon/
+F: drivers/button/button-qcom-pmic.c
 F: drivers/clk/qcom/
 F: drivers/gpio/msm_gpio.c
 F: drivers/mmc/msm_sdhci.c
diff --git a/drivers/button/Kconfig b/drivers/button/Kconfig
index 8ce2de37d62a..097b05f822e7 100644
--- a/drivers/button/Kconfig
+++ b/drivers/button/Kconfig
@@ -27,4 +27,13 @@ config BUTTON_GPIO
  The GPIO driver must used driver model. Buttons are configured using
  the device tree.
 
+config BUTTON_QCOM_PMIC
+   bool "Qualcomm power button"
+   depends on BUTTON
+   depends on PMIC_QCOM
+   help
+ Enable support for the power and "resin" (usually volume down) buttons
+ on Qualcomm SoCs. These will be configured as the Enter and Down keys
+ respectively, allowing navigation of bootmenu with buttons on device.
+
 endmenu
diff --git a/drivers/button/Makefile b/drivers/button/Makefile
index bbd18af14940..68555081a47a 100644
--- a/drivers/button/Makefile
+++ b/drivers/button/Makefile
@@ -5,3 +5,4 @@
 obj-$(CONFIG_BUTTON) += button-uclass.o
 obj-$(CONFIG_BUTTON_ADC) += button-adc.o
 obj-$(CONFIG_BUTTON_GPIO) += button-gpio.o
+obj-$(CONFIG_BUTTON_QCOM_PMIC) += button-qcom-pmic.o
\ No newline at end of file
diff --git a/drivers/button/button-qcom-pmic.c 
b/drivers/button/button-qcom-pmic.c
new file mode 100644
index ..34a976d1e6c6
--- /dev/null
+++ b/drivers/button/button-qcom-pmic.c
@@ -0,0 +1,165 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Qualcomm generic pmic gpio driver
+ *
+ * (C) Copyright 2015 Mateusz Kulikowski 
+ * (C) Copyright 2023 Linaro Ltd.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define REG_TYPE   0x4
+#define REG_SUBTYPE0x5
+
+struct qcom_pmic_btn_priv {
+   u32 base;
+   u32 status_bit;
+   int code;
+   struct udevice *pmic;
+};
+
+#define PON_INT_RT_STS0x10
+#define KPDPWR_ON_INT_BIT 0
+#define RESIN_ON_INT_BIT  1
+
+#define NODE_IS_PWRKEY(node) (!strncmp(ofnode_get_name(node), "pwrkey", 
strlen("pwrkey")))
+#define NODE_IS_RESIN(node) (!strncmp(ofnode_get_name(node), "resin", 
strlen("resin")))
+
+static enum button_state_t qcom_pwrkey_get_state(struct udevice *dev)
+{
+   struct qcom_pmic_btn_priv *priv = dev_get_priv(dev);
+
+   int reg = pmic_reg_read(priv->pmic, priv->base + PON_INT_RT_STS);
+
+   if (reg < 0)
+   return 0;
+
+   return (reg & BIT(priv->status_bit)) != 0;
+}
+
+static int qcom_pwrkey_get_code(struct udevice *dev)
+{
+   struct qcom_pmic_btn_priv *priv = dev_get_priv(dev);
+
+   return priv->code;
+}
+
+static int qcom_pwrkey_probe(struct udevice *dev)
+{
+   struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev);
+   struct qcom_pmic_btn_priv *priv = dev_get_priv(dev);
+   ofnode node = dev_ofnode(dev);
+   int ret;
+   u64 base;
+
+   /* Ignore the top-level pon node */
+   if (!uc_plat->label)
+   return 0;
+
+   /* the pwrkey and resin nodes are children of the "pon" node, get the
+* PMIC device to use in pmic_reg_* calls.
+*/
+   priv->pmic = dev->parent->parent;
+
+   /* Get the address of the parent pon node */
+   base = dev_read_addr(dev->parent);
+   if (base == FDT_ADDR_T_NONE) {
+   printf("%s: Can't find address\n", dev->name);
+   return -EINVAL;
+   }
+
+   priv->base = base;
+
+   /* Do a sanity check */
+   ret = pmic_reg_read(priv->pmic, priv->base + REG_TYPE);
+   if (ret != 0x1 && ret != 0xb) {
+   printf("%s: unexpected PMIC function type %d\n", dev->name, 
ret);
+   return -ENXIO;
+   }
+
+   ret = pmic_reg_read(priv->pmic, priv->base + REG_SUBTYPE);
+   if ((ret & 0x7) == 0) {
+   printf("%s: unexpected PMCI function subtype %d\n", dev->name, 
ret);
+   return -ENXIO;
+   }
+
+   if (NODE_IS_PWRKEY(node)) {
+   priv->status_bit = 0;
+   priv->code = KEY_ENTER;
+   } else if (NODE_IS_RESIN(node)) {
+   priv->status_bit = 1;
+   priv->code =