| Message ID | 20241016-topic-sm8x50-qcom-pmic-re-add-quirk-v1-1-6d4917d9063f@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | gpio: qcom_pmic: add again the quirk to skip GPIO configuration on PM8550 | expand |
Hi Caleb, On 16/10/2024 11:16, Neil Armstrong wrote: > The qcom_pmic code is broken for new PMICs and should be fixed, > without the QUIRK the code is broken and the GPIOs don't work > anymore on SM8550 and SM8650 platforms. > > Partially revert the revert and only add the quirk on the PM8550 > PMIC, making the buttons and MMC detect gpio work again. > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > --- > drivers/gpio/qcom_pmic_gpio.c | 21 +++++++++++++++++++-- > 1 file changed, 19 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpio/qcom_pmic_gpio.c b/drivers/gpio/qcom_pmic_gpio.c > index f2ef4e5ce14..cd9f3926ac4 100644 > --- a/drivers/gpio/qcom_pmic_gpio.c > +++ b/drivers/gpio/qcom_pmic_gpio.c > @@ -69,6 +69,17 @@ > #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) */ > @@ -117,8 +128,13 @@ static int qcom_gpio_set_direction(struct udevice *dev, unsigned int offset, > { > 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); > @@ -262,6 +278,7 @@ static int qcom_gpio_bind(struct udevice *dev) > { Gentle ping, without this PMICs gpio are unusable... Neil > > 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; > @@ -275,7 +292,7 @@ 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, > - 0, dev_ofnode(dev), &child); > + quirks, dev_ofnode(dev), &child); > if (ret) > return log_msg_ret("bind", ret); > > @@ -348,7 +365,7 @@ static const struct udevice_id qcom_gpio_ids[] = { > { .compatible = "qcom,pms405-gpio" }, > { .compatible = "qcom,pm6125-gpio" }, > { .compatible = "qcom,pm8150-gpio" }, > - { .compatible = "qcom,pm8550-gpio" }, > + { .compatible = "qcom,pm8550-gpio", .data = QCOM_PMIC_QUIRK_READONLY }, > { } > }; > > > --- > base-commit: d5cab0d6adc26ec1bbd45c2fed101184d04454ae > change-id: 20241016-topic-sm8x50-qcom-pmic-re-add-quirk-75b24dd86849 > > Best regards,
On 16/10/2024 11:16, Neil Armstrong wrote: > The qcom_pmic code is broken for new PMICs and should be fixed, > without the QUIRK the code is broken and the GPIOs don't work > anymore on SM8550 and SM8650 platforms. > > Partially revert the revert and only add the quirk on the PM8550 > PMIC, making the buttons and MMC detect gpio work again. > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> Reviewed-by: Caleb Connolly <caleb.connolly@linaro.org> sorry, this slipped by somehow. > --- > drivers/gpio/qcom_pmic_gpio.c | 21 +++++++++++++++++++-- > 1 file changed, 19 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpio/qcom_pmic_gpio.c b/drivers/gpio/qcom_pmic_gpio.c > index f2ef4e5ce14..cd9f3926ac4 100644 > --- a/drivers/gpio/qcom_pmic_gpio.c > +++ b/drivers/gpio/qcom_pmic_gpio.c > @@ -69,6 +69,17 @@ > #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) */ > @@ -117,8 +128,13 @@ static int qcom_gpio_set_direction(struct udevice *dev, unsigned int offset, > { > 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); > @@ -262,6 +278,7 @@ 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; > @@ -275,7 +292,7 @@ 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, > - 0, dev_ofnode(dev), &child); > + quirks, dev_ofnode(dev), &child); > if (ret) > return log_msg_ret("bind", ret); > > @@ -348,7 +365,7 @@ static const struct udevice_id qcom_gpio_ids[] = { > { .compatible = "qcom,pms405-gpio" }, > { .compatible = "qcom,pm6125-gpio" }, > { .compatible = "qcom,pm8150-gpio" }, > - { .compatible = "qcom,pm8550-gpio" }, > + { .compatible = "qcom,pm8550-gpio", .data = QCOM_PMIC_QUIRK_READONLY }, > { } > }; > > > --- > base-commit: d5cab0d6adc26ec1bbd45c2fed101184d04454ae > change-id: 20241016-topic-sm8x50-qcom-pmic-re-add-quirk-75b24dd86849 > > Best regards,
On Wed, 16 Oct 2024 11:16:18 +0200, Neil Armstrong wrote: > The qcom_pmic code is broken for new PMICs and should be fixed, > without the QUIRK the code is broken and the GPIOs don't work > anymore on SM8550 and SM8650 platforms. > > Partially revert the revert and only add the quirk on the PM8550 > PMIC, making the buttons and MMC detect gpio work again. > > [...] Applied, thanks! [1/1] gpio: qcom_pmic: add again the quirk to skip GPIO configuration on PM8550 commit: 1729a1988193eabcf6ac350923bbdfd32643a0c4 Best regards,
diff --git a/drivers/gpio/qcom_pmic_gpio.c b/drivers/gpio/qcom_pmic_gpio.c index f2ef4e5ce14..cd9f3926ac4 100644 --- a/drivers/gpio/qcom_pmic_gpio.c +++ b/drivers/gpio/qcom_pmic_gpio.c @@ -69,6 +69,17 @@ #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) */ @@ -117,8 +128,13 @@ static int qcom_gpio_set_direction(struct udevice *dev, unsigned int offset, { 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); @@ -262,6 +278,7 @@ 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; @@ -275,7 +292,7 @@ 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, - 0, dev_ofnode(dev), &child); + quirks, dev_ofnode(dev), &child); if (ret) return log_msg_ret("bind", ret); @@ -348,7 +365,7 @@ static const struct udevice_id qcom_gpio_ids[] = { { .compatible = "qcom,pms405-gpio" }, { .compatible = "qcom,pm6125-gpio" }, { .compatible = "qcom,pm8150-gpio" }, - { .compatible = "qcom,pm8550-gpio" }, + { .compatible = "qcom,pm8550-gpio", .data = QCOM_PMIC_QUIRK_READONLY }, { } };
The qcom_pmic code is broken for new PMICs and should be fixed, without the QUIRK the code is broken and the GPIOs don't work anymore on SM8550 and SM8650 platforms. Partially revert the revert and only add the quirk on the PM8550 PMIC, making the buttons and MMC detect gpio work again. Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> --- drivers/gpio/qcom_pmic_gpio.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) --- base-commit: d5cab0d6adc26ec1bbd45c2fed101184d04454ae change-id: 20241016-topic-sm8x50-qcom-pmic-re-add-quirk-75b24dd86849 Best regards,