diff mbox series

[v2,09/32] gpio: qcom_pmic: add a quirk to skip GPIO configuration

Message ID 20231219-b4-qcom-common-target-v2-9-b6dd9704219e@linaro.org
State Superseded
Headers show
Series Qualcomm generic board support | expand

Commit Message

Caleb Connolly Dec. 19, 2023, 4:04 p.m. UTC
Some platforms hard reset when attempting to configure PMIC GPIOs. Add
support for quirks specified in match data with a single quirk to skip
this configuration. We rely on the GPIO already be configured correctly,
which is always the case for volume up (the only current user of these
GPIOs).

Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
 drivers/gpio/qcom_pmic_gpio.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

Comments

Neil Armstrong Dec. 21, 2023, 4:19 p.m. UTC | #1
On 19/12/2023 17:04, Caleb Connolly wrote:
> Some platforms hard reset when attempting to configure PMIC GPIOs. Add
> support for quirks specified in match data with a single quirk to skip
> this configuration. We rely on the GPIO already be configured correctly,
> which is always the case for volume up (the only current user of these
> GPIOs).

Do you know why this happens in u-boot and not in Linux ?

> 
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
>   drivers/gpio/qcom_pmic_gpio.c | 18 ++++++++++++++++--
>   1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpio/qcom_pmic_gpio.c b/drivers/gpio/qcom_pmic_gpio.c
> index 2a4fef8d28cb..198cd84bc31e 100644
> --- a/drivers/gpio/qcom_pmic_gpio.c
> +++ b/drivers/gpio/qcom_pmic_gpio.c
> @@ -64,6 +64,15 @@
>   #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
> + */
> +enum pmic_gpio_quirks {
> +	QCOM_PMIC_QUIRK_READONLY = (1 << 0),
> +};
> +
>   struct qcom_gpio_bank {
>   	uint32_t pid; /* Peripheral ID on SPMI bus */
>   	bool     lv_mv_type; /* If subtype is GPIO_LV(0x10) or GPIO_MV(0x11) */
> @@ -75,7 +84,12 @@ static int qcom_gpio_set_direction(struct udevice *dev, unsigned offset,
>   	struct qcom_gpio_bank *priv = dev_get_priv(dev);
>   	uint32_t gpio_base = priv->pid + REG_OFFSET(offset);
>   	uint32_t reg_ctl_val;
> -	int ret;
> +	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,
> @@ -304,7 +318,7 @@ static int qcom_gpio_of_to_plat(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" },
> +	{ .compatible = "qcom,pm8998-gpio", .data = QCOM_PMIC_QUIRK_READONLY },
>   	{ .compatible = "qcom,pms405-gpio" },
>   	{ }
>   };
>
Caleb Connolly Dec. 21, 2023, 4:26 p.m. UTC | #2
On 21/12/2023 16:19, Neil Armstrong wrote:
> On 19/12/2023 17:04, Caleb Connolly wrote:
>> Some platforms hard reset when attempting to configure PMIC GPIOs. Add
>> support for quirks specified in match data with a single quirk to skip
>> this configuration. We rely on the GPIO already be configured correctly,
>> which is always the case for volume up (the only current user of these
>> GPIOs).
> 
> Do you know why this happens in u-boot and not in Linux ?

I don't, I did some preliminary debugging but couldn't root-cause this.
Writing to any of the GPIO registers seem to cause a crash. It could
potentially be a bug in the SPMI arb driver - I'm not sure if there are
any SPMI writes done on this platform by U-Boot
> 
>>
>> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
>> ---
>>   drivers/gpio/qcom_pmic_gpio.c | 18 ++++++++++++++++--
>>   1 file changed, 16 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpio/qcom_pmic_gpio.c
>> b/drivers/gpio/qcom_pmic_gpio.c
>> index 2a4fef8d28cb..198cd84bc31e 100644
>> --- a/drivers/gpio/qcom_pmic_gpio.c
>> +++ b/drivers/gpio/qcom_pmic_gpio.c
>> @@ -64,6 +64,15 @@
>>   #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
>> + */
>> +enum pmic_gpio_quirks {
>> +    QCOM_PMIC_QUIRK_READONLY = (1 << 0),
>> +};
>> +
>>   struct qcom_gpio_bank {
>>       uint32_t pid; /* Peripheral ID on SPMI bus */
>>       bool     lv_mv_type; /* If subtype is GPIO_LV(0x10) or
>> GPIO_MV(0x11) */
>> @@ -75,7 +84,12 @@ static int qcom_gpio_set_direction(struct udevice
>> *dev, unsigned offset,
>>       struct qcom_gpio_bank *priv = dev_get_priv(dev);
>>       uint32_t gpio_base = priv->pid + REG_OFFSET(offset);
>>       uint32_t reg_ctl_val;
>> -    int ret;
>> +    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,
>> @@ -304,7 +318,7 @@ static int qcom_gpio_of_to_plat(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" },
>> +    { .compatible = "qcom,pm8998-gpio", .data =
>> QCOM_PMIC_QUIRK_READONLY },
>>       { .compatible = "qcom,pms405-gpio" },
>>       { }
>>   };
>>
>
diff mbox series

Patch

diff --git a/drivers/gpio/qcom_pmic_gpio.c b/drivers/gpio/qcom_pmic_gpio.c
index 2a4fef8d28cb..198cd84bc31e 100644
--- a/drivers/gpio/qcom_pmic_gpio.c
+++ b/drivers/gpio/qcom_pmic_gpio.c
@@ -64,6 +64,15 @@ 
 #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
+ */
+enum pmic_gpio_quirks {
+	QCOM_PMIC_QUIRK_READONLY = (1 << 0),
+};
+
 struct qcom_gpio_bank {
 	uint32_t pid; /* Peripheral ID on SPMI bus */
 	bool     lv_mv_type; /* If subtype is GPIO_LV(0x10) or GPIO_MV(0x11) */
@@ -75,7 +84,12 @@  static int qcom_gpio_set_direction(struct udevice *dev, unsigned offset,
 	struct qcom_gpio_bank *priv = dev_get_priv(dev);
 	uint32_t gpio_base = priv->pid + REG_OFFSET(offset);
 	uint32_t reg_ctl_val;
-	int ret;
+	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,
@@ -304,7 +318,7 @@  static int qcom_gpio_of_to_plat(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" },
+	{ .compatible = "qcom,pm8998-gpio", .data = QCOM_PMIC_QUIRK_READONLY },
 	{ .compatible = "qcom,pms405-gpio" },
 	{ }
 };