diff mbox series

Revert "gpio: qcom_pmic: add a quirk to skip GPIO configuration"

Message ID 20240909120641.1396544-1-caleb.connolly@linaro.org
State Accepted
Commit dc554a07ea295b5178d7632a7129583481451654
Headers show
Series Revert "gpio: qcom_pmic: add a quirk to skip GPIO configuration" | expand

Commit Message

Caleb Connolly Sept. 9, 2024, 12:06 p.m. UTC
This reverts commit 19f000b72b2fa7e4540f7cdb91287aff594239bd.

The bug in writing was caused by a long-standing error in the SPMI
driver which has since been fixed - c2de620d64d4 ("spmi: msm: fix
version 5 support"). We can safely enable writing GPIO configuration
now.

Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
Tested on SDM845/pm8998 (OnePlus 6)
---
 drivers/gpio/qcom_pmic_gpio.c | 27 +++++----------------------
 1 file changed, 5 insertions(+), 22 deletions(-)

Comments

Neil Armstrong Sept. 9, 2024, 12:52 p.m. UTC | #1
On 09/09/2024 14:06, Caleb Connolly wrote:
> This reverts commit 19f000b72b2fa7e4540f7cdb91287aff594239bd.
> 
> The bug in writing was caused by a long-standing error in the SPMI
> driver which has since been fixed - c2de620d64d4 ("spmi: msm: fix
> version 5 support"). We can safely enable writing GPIO configuration
> now.
> 
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
> Tested on SDM845/pm8998 (OnePlus 6)
> ---
>   drivers/gpio/qcom_pmic_gpio.c | 27 +++++----------------------
>   1 file changed, 5 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpio/qcom_pmic_gpio.c b/drivers/gpio/qcom_pmic_gpio.c
> index 80fee841ee3f..f2ef4e5ce144 100644
> --- a/drivers/gpio/qcom_pmic_gpio.c
> +++ b/drivers/gpio/qcom_pmic_gpio.c
> @@ -68,19 +68,8 @@
>   
>   #define REG_EN_CTL             0x46
>   #define REG_EN_CTL_ENABLE      (1 << 7)
>   
> -/**
> - * pmic_gpio_match_data - platform specific configuration
> - *
> - * @PMIC_MATCH_READONLY: treat all GPIOs as readonly, don't attempt to configure them.
> - * This is a workaround for an unknown bug on some platforms where trying to write the
> - * GPIO configuration registers causes the board to hang.
> - */
> -enum pmic_gpio_quirks {
> -	QCOM_PMIC_QUIRK_READONLY = (1 << 0),
> -};
> -
>   struct qcom_pmic_gpio_data {
>   	uint32_t pid; /* Peripheral ID on SPMI bus */
>   	bool     lv_mv_type; /* If subtype is GPIO_LV(0x10) or GPIO_MV(0x11) */
>   	u32 pin_count;
> @@ -127,15 +116,10 @@ static int qcom_gpio_set_direction(struct udevice *dev, unsigned int offset,
>   				   bool input, int value)
>   {
>   	struct qcom_pmic_gpio_data *plat = dev_get_plat(dev);
>   	uint32_t gpio_base = plat->pid + REG_OFFSET(offset);
> -	ulong quirks = dev_get_driver_data(dev);
>   	int ret = 0;
>   
> -	/* Some PMICs don't like their GPIOs being configured */
> -	if (quirks & QCOM_PMIC_QUIRK_READONLY)
> -		return 0;
> -
>   	/* Disable the GPIO */
>   	ret = pmic_clrsetbits(dev->parent, gpio_base + REG_EN_CTL,
>   			      REG_EN_CTL_ENABLE, 0);
>   	if (ret < 0)
> @@ -277,9 +261,8 @@ static const struct dm_gpio_ops qcom_gpio_ops = {
>   static int qcom_gpio_bind(struct udevice *dev)
>   {
>   
>   	struct qcom_pmic_gpio_data *plat = dev_get_plat(dev);
> -	ulong quirks = dev_get_driver_data(dev);
>   	struct udevice *child;
>   	struct driver *drv;
>   	int ret;
>   
> @@ -291,9 +274,9 @@ static int qcom_gpio_bind(struct udevice *dev)
>   
>   	/* Bind the GPIO driver as a child of the PMIC. */
>   	ret = device_bind_with_driver_data(dev, drv,
>   					   dev->name,
> -					   quirks, dev_ofnode(dev), &child);
> +					   0, dev_ofnode(dev), &child);
>   	if (ret)
>   		return log_msg_ret("bind", ret);
>   
>   	dev_set_plat(child, plat);
> @@ -360,13 +343,13 @@ static int qcom_gpio_probe(struct udevice *dev)
>   
>   static const struct udevice_id qcom_gpio_ids[] = {
>   	{ .compatible = "qcom,pm8916-gpio" },
>   	{ .compatible = "qcom,pm8994-gpio" },	/* 22 GPIO's */
> -	{ .compatible = "qcom,pm8998-gpio", .data = QCOM_PMIC_QUIRK_READONLY },
> +	{ .compatible = "qcom,pm8998-gpio" },
>   	{ .compatible = "qcom,pms405-gpio" },
> -	{ .compatible = "qcom,pm6125-gpio", .data = QCOM_PMIC_QUIRK_READONLY },
> -	{ .compatible = "qcom,pm8150-gpio", .data = QCOM_PMIC_QUIRK_READONLY },
> -	{ .compatible = "qcom,pm8550-gpio", .data = QCOM_PMIC_QUIRK_READONLY },
> +	{ .compatible = "qcom,pm6125-gpio" },
> +	{ .compatible = "qcom,pm8150-gpio" },
> +	{ .compatible = "qcom,pm8550-gpio" },
>   	{ }
>   };
>   
>   U_BOOT_DRIVER(qcom_pmic_gpio) = {

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Caleb Connolly Oct. 2, 2024, 2:52 p.m. UTC | #2
On Mon, 09 Sep 2024 14:06:10 +0200, Caleb Connolly wrote:
> This reverts commit 19f000b72b2fa7e4540f7cdb91287aff594239bd.
> 
> The bug in writing was caused by a long-standing error in the SPMI
> driver which has since been fixed - c2de620d64d4 ("spmi: msm: fix
> version 5 support"). We can safely enable writing GPIO configuration
> now.
> 
> [...]

Applied, thanks!

[1/1] Revert "gpio: qcom_pmic: add a quirk to skip GPIO configuration"
      https://source.denx.de/u-boot/custodians/u-boot-snapdragon/-/commit/25595a48555c

Best regards,
diff mbox series

Patch

diff --git a/drivers/gpio/qcom_pmic_gpio.c b/drivers/gpio/qcom_pmic_gpio.c
index 80fee841ee3f..f2ef4e5ce144 100644
--- a/drivers/gpio/qcom_pmic_gpio.c
+++ b/drivers/gpio/qcom_pmic_gpio.c
@@ -68,19 +68,8 @@ 
 
 #define REG_EN_CTL             0x46
 #define REG_EN_CTL_ENABLE      (1 << 7)
 
-/**
- * pmic_gpio_match_data - platform specific configuration
- *
- * @PMIC_MATCH_READONLY: treat all GPIOs as readonly, don't attempt to configure them.
- * This is a workaround for an unknown bug on some platforms where trying to write the
- * GPIO configuration registers causes the board to hang.
- */
-enum pmic_gpio_quirks {
-	QCOM_PMIC_QUIRK_READONLY = (1 << 0),
-};
-
 struct qcom_pmic_gpio_data {
 	uint32_t pid; /* Peripheral ID on SPMI bus */
 	bool     lv_mv_type; /* If subtype is GPIO_LV(0x10) or GPIO_MV(0x11) */
 	u32 pin_count;
@@ -127,15 +116,10 @@  static int qcom_gpio_set_direction(struct udevice *dev, unsigned int offset,
 				   bool input, int value)
 {
 	struct qcom_pmic_gpio_data *plat = dev_get_plat(dev);
 	uint32_t gpio_base = plat->pid + REG_OFFSET(offset);
-	ulong quirks = dev_get_driver_data(dev);
 	int ret = 0;
 
-	/* Some PMICs don't like their GPIOs being configured */
-	if (quirks & QCOM_PMIC_QUIRK_READONLY)
-		return 0;
-
 	/* Disable the GPIO */
 	ret = pmic_clrsetbits(dev->parent, gpio_base + REG_EN_CTL,
 			      REG_EN_CTL_ENABLE, 0);
 	if (ret < 0)
@@ -277,9 +261,8 @@  static const struct dm_gpio_ops qcom_gpio_ops = {
 static int qcom_gpio_bind(struct udevice *dev)
 {
 
 	struct qcom_pmic_gpio_data *plat = dev_get_plat(dev);
-	ulong quirks = dev_get_driver_data(dev);
 	struct udevice *child;
 	struct driver *drv;
 	int ret;
 
@@ -291,9 +274,9 @@  static int qcom_gpio_bind(struct udevice *dev)
 
 	/* Bind the GPIO driver as a child of the PMIC. */
 	ret = device_bind_with_driver_data(dev, drv,
 					   dev->name,
-					   quirks, dev_ofnode(dev), &child);
+					   0, dev_ofnode(dev), &child);
 	if (ret)
 		return log_msg_ret("bind", ret);
 
 	dev_set_plat(child, plat);
@@ -360,13 +343,13 @@  static int qcom_gpio_probe(struct udevice *dev)
 
 static const struct udevice_id qcom_gpio_ids[] = {
 	{ .compatible = "qcom,pm8916-gpio" },
 	{ .compatible = "qcom,pm8994-gpio" },	/* 22 GPIO's */
-	{ .compatible = "qcom,pm8998-gpio", .data = QCOM_PMIC_QUIRK_READONLY },
+	{ .compatible = "qcom,pm8998-gpio" },
 	{ .compatible = "qcom,pms405-gpio" },
-	{ .compatible = "qcom,pm6125-gpio", .data = QCOM_PMIC_QUIRK_READONLY },
-	{ .compatible = "qcom,pm8150-gpio", .data = QCOM_PMIC_QUIRK_READONLY },
-	{ .compatible = "qcom,pm8550-gpio", .data = QCOM_PMIC_QUIRK_READONLY },
+	{ .compatible = "qcom,pm6125-gpio" },
+	{ .compatible = "qcom,pm8150-gpio" },
+	{ .compatible = "qcom,pm8550-gpio" },
 	{ }
 };
 
 U_BOOT_DRIVER(qcom_pmic_gpio) = {