* [PATCH 1/4] pinctrl: qcom: spmi-gpio: add support for LV_VIN2 and MV_VIN3 subtypes
2022-09-07 20:15 [PATCH 0/4] Add Support for Qualcomm SPMI GPIOs Anjelique Melendez
@ 2022-09-07 20:15 ` Anjelique Melendez
2022-09-07 20:15 ` [PATCH 2/4] pinctrl: qcom: spmi-gpio: Fix the GPIO strength mapping Anjelique Melendez
` (2 subsequent siblings)
3 siblings, 0 replies; 13+ messages in thread
From: Anjelique Melendez @ 2022-09-07 20:15 UTC (permalink / raw)
To: agross, bjorn.andersson, linus.walleij, robh+dt,
krzysztof.kozlowski+dt
Cc: konrad.dybcio, linux-arm-msm, linux-gpio, devicetree,
linux-kernel, David Collins, Anjelique Melendez
From: David Collins <quic_collinsd@quicinc.com>
Add support for SPMI PMIC GPIO subtypes GPIO_LV_VIN2 and
GPIO_MV_VIN3.
GPIO_LV_VIN2 GPIOs support two input reference voltages: VIN0 and
VIN1. These are typically connected to 1.8 V and 1.2 V supplies
respectively.
GPIO_MV_VIN3 GPIOs support three input reference voltages: VIN0,
VIN1, and VIN2. These are typically connected to Vph, 1.8 V, and
1.2 V supplies respectively.
Signed-off-by: David Collins <quic_collinsd@quicinc.com>
Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com>
---
drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index ccaf40a9c0e6..cf6b6047de8d 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
* Copyright (c) 2012-2014, 2016-2021 The Linux Foundation. All rights reserved.
+ * Copyright (c) 2021-2022 Qualcomm Innovation Center, Inc. All rights reserved.
*/
#include <linux/gpio/driver.h>
@@ -36,6 +37,8 @@
#define PMIC_GPIO_SUBTYPE_GPIOC_8CH 0xd
#define PMIC_GPIO_SUBTYPE_GPIO_LV 0x10
#define PMIC_GPIO_SUBTYPE_GPIO_MV 0x11
+#define PMIC_GPIO_SUBTYPE_GPIO_LV_VIN2 0x12
+#define PMIC_GPIO_SUBTYPE_GPIO_MV_VIN3 0x13
#define PMIC_MPP_REG_RT_STS 0x10
#define PMIC_MPP_REG_RT_STS_VAL_MASK 0x1
@@ -823,6 +826,16 @@ static int pmic_gpio_populate(struct pmic_gpio_state *state,
pad->have_buffer = true;
pad->lv_mv_type = true;
break;
+ case PMIC_GPIO_SUBTYPE_GPIO_LV_VIN2:
+ pad->num_sources = 2;
+ pad->have_buffer = true;
+ pad->lv_mv_type = true;
+ break;
+ case PMIC_GPIO_SUBTYPE_GPIO_MV_VIN3:
+ pad->num_sources = 3;
+ pad->have_buffer = true;
+ pad->lv_mv_type = true;
+ break;
default:
dev_err(state->dev, "unknown GPIO type 0x%x\n", subtype);
return -ENODEV;
--
2.35.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 2/4] pinctrl: qcom: spmi-gpio: Fix the GPIO strength mapping
2022-09-07 20:15 [PATCH 0/4] Add Support for Qualcomm SPMI GPIOs Anjelique Melendez
2022-09-07 20:15 ` [PATCH 1/4] pinctrl: qcom: spmi-gpio: add support for LV_VIN2 and MV_VIN3 subtypes Anjelique Melendez
@ 2022-09-07 20:15 ` Anjelique Melendez
2022-09-08 11:14 ` Krzysztof Kozlowski
2022-09-07 20:15 ` [PATCH 3/4] pinctrl: qcom: spmi-gpio: Add compatible for PM7250B Anjelique Melendez
2022-09-07 20:15 ` [PATCH 4/4] dt-bindings: qcom-pmic-gpio: Add PM7250B and PM8450 bindings Anjelique Melendez
3 siblings, 1 reply; 13+ messages in thread
From: Anjelique Melendez @ 2022-09-07 20:15 UTC (permalink / raw)
To: agross, bjorn.andersson, linus.walleij, robh+dt,
krzysztof.kozlowski+dt
Cc: konrad.dybcio, linux-arm-msm, linux-gpio, devicetree,
linux-kernel, Anirudh Ghayal, Anjelique Melendez
From: Anirudh Ghayal <quic_aghayal@quicinc.com>
The SPMI based PMICs have the HIGH and LOW GPIO output
strength mappings interchanged, fix them.
Keep the mapping same for older SSBI based PMICs.
CRs-Fixed: 2246473
Signed-off-by: Anirudh Ghayal <quic_aghayal@quicinc.com>
Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com>
---
drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 2 +-
drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c | 4 ++--
include/dt-bindings/pinctrl/qcom,pmic-gpio.h | 9 +++++++--
3 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index cf6b6047de8d..fceccf1ec099 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -525,7 +525,7 @@ static int pmic_gpio_config_set(struct pinctrl_dev *pctldev, unsigned int pin,
pad->pullup = arg;
break;
case PMIC_GPIO_CONF_STRENGTH:
- if (arg > PMIC_GPIO_STRENGTH_LOW)
+ if (arg > PMIC_GPIO_STRENGTH_HIGH)
return -EINVAL;
pad->strength = arg;
break;
diff --git a/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c b/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c
index 1b41adda8129..0f96d130813b 100644
--- a/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c
@@ -1,7 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-only
/*
* Copyright (c) 2015, Sony Mobile Communications AB.
- * Copyright (c) 2013, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2013, 2018 The Linux Foundation. All rights reserved.
*/
#include <linux/module.h>
@@ -371,7 +371,7 @@ static int pm8xxx_pin_config_set(struct pinctrl_dev *pctldev,
banks |= BIT(0);
break;
case PM8XXX_QCOM_DRIVE_STRENGH:
- if (arg > PMIC_GPIO_STRENGTH_LOW) {
+ if (arg > PM8921_GPIO_STRENGTH_LOW) {
dev_err(pctrl->dev, "invalid drive strength\n");
return -EINVAL;
}
diff --git a/include/dt-bindings/pinctrl/qcom,pmic-gpio.h b/include/dt-bindings/pinctrl/qcom,pmic-gpio.h
index e5df5ce45a0f..950be952ad3e 100644
--- a/include/dt-bindings/pinctrl/qcom,pmic-gpio.h
+++ b/include/dt-bindings/pinctrl/qcom,pmic-gpio.h
@@ -12,9 +12,14 @@
#define PMIC_GPIO_PULL_UP_1P5_30 3
#define PMIC_GPIO_STRENGTH_NO 0
-#define PMIC_GPIO_STRENGTH_HIGH 1
+#define PMIC_GPIO_STRENGTH_LOW 1
#define PMIC_GPIO_STRENGTH_MED 2
-#define PMIC_GPIO_STRENGTH_LOW 3
+#define PMIC_GPIO_STRENGTH_HIGH 3
+
+#define PM8921_GPIO_STRENGTH_NO 0
+#define PM8921_GPIO_STRENGTH_HIGH 1
+#define PM8921_GPIO_STRENGTH_MED 2
+#define PM8921_GPIO_STRENGTH_LOW 3
/*
* Note: PM8018 GPIO3 and GPIO4 are supporting
--
2.35.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 2/4] pinctrl: qcom: spmi-gpio: Fix the GPIO strength mapping
2022-09-07 20:15 ` [PATCH 2/4] pinctrl: qcom: spmi-gpio: Fix the GPIO strength mapping Anjelique Melendez
@ 2022-09-08 11:14 ` Krzysztof Kozlowski
2022-09-08 23:52 ` Anjelique Melendez
2022-09-09 0:25 ` David Collins
0 siblings, 2 replies; 13+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-08 11:14 UTC (permalink / raw)
To: Anjelique Melendez, agross, bjorn.andersson, linus.walleij,
robh+dt, krzysztof.kozlowski+dt
Cc: konrad.dybcio, linux-arm-msm, linux-gpio, devicetree,
linux-kernel, Anirudh Ghayal
On 07/09/2022 22:15, Anjelique Melendez wrote:
> From: Anirudh Ghayal <quic_aghayal@quicinc.com>
>
> The SPMI based PMICs have the HIGH and LOW GPIO output
> strength mappings interchanged, fix them.
>
> Keep the mapping same for older SSBI based PMICs.
>
> CRs-Fixed: 2246473
What is this tag about?
> Signed-off-by: Anirudh Ghayal <quic_aghayal@quicinc.com>
> Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com>
> ---
> drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 2 +-
> drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c | 4 ++--
> include/dt-bindings/pinctrl/qcom,pmic-gpio.h | 9 +++++++--
> 3 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> index cf6b6047de8d..fceccf1ec099 100644
> --- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> +++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
> @@ -525,7 +525,7 @@ static int pmic_gpio_config_set(struct pinctrl_dev *pctldev, unsigned int pin,
> pad->pullup = arg;
> break;
> case PMIC_GPIO_CONF_STRENGTH:
> - if (arg > PMIC_GPIO_STRENGTH_LOW)
> + if (arg > PMIC_GPIO_STRENGTH_HIGH)
> return -EINVAL;
> pad->strength = arg;
> break;
> diff --git a/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c b/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c
> index 1b41adda8129..0f96d130813b 100644
> --- a/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c
> +++ b/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c
> @@ -1,7 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0-only
> /*
> * Copyright (c) 2015, Sony Mobile Communications AB.
> - * Copyright (c) 2013, The Linux Foundation. All rights reserved.
> + * Copyright (c) 2013, 2018 The Linux Foundation. All rights reserved.
> */
>
> #include <linux/module.h>
> @@ -371,7 +371,7 @@ static int pm8xxx_pin_config_set(struct pinctrl_dev *pctldev,
> banks |= BIT(0);
> break;
> case PM8XXX_QCOM_DRIVE_STRENGH:
> - if (arg > PMIC_GPIO_STRENGTH_LOW) {
> + if (arg > PM8921_GPIO_STRENGTH_LOW) {
> dev_err(pctrl->dev, "invalid drive strength\n");
> return -EINVAL;
> }
> diff --git a/include/dt-bindings/pinctrl/qcom,pmic-gpio.h b/include/dt-bindings/pinctrl/qcom,pmic-gpio.h
> index e5df5ce45a0f..950be952ad3e 100644
> --- a/include/dt-bindings/pinctrl/qcom,pmic-gpio.h
> +++ b/include/dt-bindings/pinctrl/qcom,pmic-gpio.h
You cannot mix bindings with driver. This is an ABI break.
> @@ -12,9 +12,14 @@
> #define PMIC_GPIO_PULL_UP_1P5_30 3
>
> #define PMIC_GPIO_STRENGTH_NO 0
> -#define PMIC_GPIO_STRENGTH_HIGH 1
> +#define PMIC_GPIO_STRENGTH_LOW 1
> #define PMIC_GPIO_STRENGTH_MED 2
> -#define PMIC_GPIO_STRENGTH_LOW 3
> +#define PMIC_GPIO_STRENGTH_HIGH 3
Didn't you just break all DTSes in the world?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 2/4] pinctrl: qcom: spmi-gpio: Fix the GPIO strength mapping
2022-09-08 11:14 ` Krzysztof Kozlowski
@ 2022-09-08 23:52 ` Anjelique Melendez
2022-09-09 7:35 ` Krzysztof Kozlowski
2022-09-09 0:25 ` David Collins
1 sibling, 1 reply; 13+ messages in thread
From: Anjelique Melendez @ 2022-09-08 23:52 UTC (permalink / raw)
To: Krzysztof Kozlowski, agross, bjorn.andersson, linus.walleij,
robh+dt, krzysztof.kozlowski+dt
Cc: konrad.dybcio, linux-arm-msm, linux-gpio, devicetree,
linux-kernel, Anirudh Ghayal
On 9/8/2022 4:14 AM, Krzysztof Kozlowski wrote:
> On 07/09/2022 22:15, Anjelique Melendez wrote:
>> From: Anirudh Ghayal <quic_aghayal@quicinc.com>
>>
>> The SPMI based PMICs have the HIGH and LOW GPIO output
>> strength mappings interchanged, fix them.
>>
>> Keep the mapping same for older SSBI based PMICs.
>>
>> CRs-Fixed: 2246473
>
> What is this tag about?
Forgot to remove this tag before up streamed. Will remove for next version.
>
>> Signed-off-by: Anirudh Ghayal <quic_aghayal@quicinc.com>
>> Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com>
>> ---
>> drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 2 +-
>> drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c | 4 ++--
>> include/dt-bindings/pinctrl/qcom,pmic-gpio.h | 9 +++++++--
>> 3 files changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
>> index cf6b6047de8d..fceccf1ec099 100644
>> --- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
>> +++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
>> @@ -525,7 +525,7 @@ static int pmic_gpio_config_set(struct pinctrl_dev *pctldev, unsigned int pin,
>> pad->pullup = arg;
>> break;
>> case PMIC_GPIO_CONF_STRENGTH:
>> - if (arg > PMIC_GPIO_STRENGTH_LOW)
>> + if (arg > PMIC_GPIO_STRENGTH_HIGH)
>> return -EINVAL;
>> pad->strength = arg;
>> break;
>> diff --git a/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c b/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c
>> index 1b41adda8129..0f96d130813b 100644
>> --- a/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c
>> +++ b/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c
>> @@ -1,7 +1,7 @@
>> // SPDX-License-Identifier: GPL-2.0-only
>> /*
>> * Copyright (c) 2015, Sony Mobile Communications AB.
>> - * Copyright (c) 2013, The Linux Foundation. All rights reserved.
>> + * Copyright (c) 2013, 2018 The Linux Foundation. All rights reserved.
>> */
>>
>> #include <linux/module.h>
>> @@ -371,7 +371,7 @@ static int pm8xxx_pin_config_set(struct pinctrl_dev *pctldev,
>> banks |= BIT(0);
>> break;
>> case PM8XXX_QCOM_DRIVE_STRENGH:
>> - if (arg > PMIC_GPIO_STRENGTH_LOW) {
>> + if (arg > PM8921_GPIO_STRENGTH_LOW) {
>> dev_err(pctrl->dev, "invalid drive strength\n");
>> return -EINVAL;
>> }
>> diff --git a/include/dt-bindings/pinctrl/qcom,pmic-gpio.h b/include/dt-bindings/pinctrl/qcom,pmic-gpio.h
>> index e5df5ce45a0f..950be952ad3e 100644
>> --- a/include/dt-bindings/pinctrl/qcom,pmic-gpio.h
>> +++ b/include/dt-bindings/pinctrl/qcom,pmic-gpio.h
>
> You cannot mix bindings with driver. This is an ABI break.
Ack - Will separate into two changes.
>> @@ -12,9 +12,14 @@
>> #define PMIC_GPIO_PULL_UP_1P5_30 3
>>
>> #define PMIC_GPIO_STRENGTH_NO 0
>> -#define PMIC_GPIO_STRENGTH_HIGH 1
>> +#define PMIC_GPIO_STRENGTH_LOW 1
>> #define PMIC_GPIO_STRENGTH_MED 2
>> -#define PMIC_GPIO_STRENGTH_LOW 3
>> +#define PMIC_GPIO_STRENGTH_HIGH 3
>
> Didn't you just break all DTSes in the world?
Ack - lol. Next version will include changes to update any DTS
that uses PMIC_GPIO_STRENGTH_
>
> Best regards,
> Krzysztof
Thanks,
Anjelique
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 2/4] pinctrl: qcom: spmi-gpio: Fix the GPIO strength mapping
2022-09-08 23:52 ` Anjelique Melendez
@ 2022-09-09 7:35 ` Krzysztof Kozlowski
0 siblings, 0 replies; 13+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-09 7:35 UTC (permalink / raw)
To: Anjelique Melendez, agross, bjorn.andersson, linus.walleij,
robh+dt, krzysztof.kozlowski+dt
Cc: konrad.dybcio, linux-arm-msm, linux-gpio, devicetree,
linux-kernel, Anirudh Ghayal
On 09/09/2022 01:52, Anjelique Melendez wrote:
pio.h b/include/dt-bindings/pinctrl/qcom,pmic-gpio.h
>>> index e5df5ce45a0f..950be952ad3e 100644
>>> --- a/include/dt-bindings/pinctrl/qcom,pmic-gpio.h
>>> +++ b/include/dt-bindings/pinctrl/qcom,pmic-gpio.h
>>
>> You cannot mix bindings with driver. This is an ABI break.
> Ack - Will separate into two changes.
>>> @@ -12,9 +12,14 @@
>>> #define PMIC_GPIO_PULL_UP_1P5_30 3
>>>
>>> #define PMIC_GPIO_STRENGTH_NO 0
>>> -#define PMIC_GPIO_STRENGTH_HIGH 1
>>> +#define PMIC_GPIO_STRENGTH_LOW 1
>>> #define PMIC_GPIO_STRENGTH_MED 2
>>> -#define PMIC_GPIO_STRENGTH_LOW 3
>>> +#define PMIC_GPIO_STRENGTH_HIGH 3
>>
>> Didn't you just break all DTSes in the world?
> Ack - lol. Next version will include changes to update any DTS
> that uses PMIC_GPIO_STRENGTH_
There is discussion with David, so please wait till consensus is
reached. It seems you want to change DT binding constant to match
register value which is not appropriate. Constants are not change'able.
Constants are abstractions which might or might not match register value.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 2/4] pinctrl: qcom: spmi-gpio: Fix the GPIO strength mapping
2022-09-08 11:14 ` Krzysztof Kozlowski
2022-09-08 23:52 ` Anjelique Melendez
@ 2022-09-09 0:25 ` David Collins
2022-09-09 7:34 ` Krzysztof Kozlowski
1 sibling, 1 reply; 13+ messages in thread
From: David Collins @ 2022-09-09 0:25 UTC (permalink / raw)
To: Krzysztof Kozlowski, Anjelique Melendez, agross, bjorn.andersson,
linus.walleij, robh+dt, krzysztof.kozlowski+dt
Cc: konrad.dybcio, linux-arm-msm, linux-gpio, devicetree,
linux-kernel, Anirudh Ghayal
On 9/8/22 04:14, Krzysztof Kozlowski wrote:
> On 07/09/2022 22:15, Anjelique Melendez wrote:
>> From: Anirudh Ghayal <quic_aghayal@quicinc.com>
>>
>> The SPMI based PMICs have the HIGH and LOW GPIO output
>> strength mappings interchanged, fix them.
>>
>> Keep the mapping same for older SSBI based PMICs.
>>
>> CRs-Fixed: 2246473
>
> What is this tag about?
This is for internal tracking. It will be removed in the next version
of this patch series.
>> drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 2 +-
>> drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c | 4 ++--
>> include/dt-bindings/pinctrl/qcom,pmic-gpio.h | 9 +++++++--
>> 3 files changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
>> index cf6b6047de8d..fceccf1ec099 100644
>> --- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
>> +++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
>> @@ -525,7 +525,7 @@ static int pmic_gpio_config_set(struct pinctrl_dev *pctldev, unsigned int pin,
>> pad->pullup = arg;
>> break;
>> case PMIC_GPIO_CONF_STRENGTH:
>> - if (arg > PMIC_GPIO_STRENGTH_LOW)
>> + if (arg > PMIC_GPIO_STRENGTH_HIGH)
>> return -EINVAL;
>> pad->strength = arg;
>> break;
>> diff --git a/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c b/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c
>> index 1b41adda8129..0f96d130813b 100644
>> --- a/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c
>> +++ b/drivers/pinctrl/qcom/pinctrl-ssbi-gpio.c
>> @@ -1,7 +1,7 @@
>> // SPDX-License-Identifier: GPL-2.0-only
>> /*
>> * Copyright (c) 2015, Sony Mobile Communications AB.
>> - * Copyright (c) 2013, The Linux Foundation. All rights reserved.
>> + * Copyright (c) 2013, 2018 The Linux Foundation. All rights reserved.
>> */
>>
>> #include <linux/module.h>
>> @@ -371,7 +371,7 @@ static int pm8xxx_pin_config_set(struct pinctrl_dev *pctldev,
>> banks |= BIT(0);
>> break;
>> case PM8XXX_QCOM_DRIVE_STRENGH:
>> - if (arg > PMIC_GPIO_STRENGTH_LOW) {
>> + if (arg > PM8921_GPIO_STRENGTH_LOW) {
>> dev_err(pctrl->dev, "invalid drive strength\n");
>> return -EINVAL;
>> }
>> diff --git a/include/dt-bindings/pinctrl/qcom,pmic-gpio.h b/include/dt-bindings/pinctrl/qcom,pmic-gpio.h
>> index e5df5ce45a0f..950be952ad3e 100644
>> --- a/include/dt-bindings/pinctrl/qcom,pmic-gpio.h
>> +++ b/include/dt-bindings/pinctrl/qcom,pmic-gpio.h
>
> You cannot mix bindings with driver. This is an ABI break.
This could be split into two patches. However, both would need to make
it into any given build to avoid runtime regressions when
pinctrl-spmi-gpio.c rejects GPIO strength configurations larger than 1.
I suppose that this kind of bi-directional dependency could be avoided
by using one of these checks instead in the driver:
if (arg > 3) {
or
if (arg > max(PMIC_GPIO_STRENGTH_LOW, PMIC_GPIO_STRENGTH_HIGH))
Going this route would only require that the driver patch is picked up
before the DT header patch.
>> @@ -12,9 +12,14 @@
>> #define PMIC_GPIO_PULL_UP_1P5_30 3
>>
>> #define PMIC_GPIO_STRENGTH_NO 0
>> -#define PMIC_GPIO_STRENGTH_HIGH 1
>> +#define PMIC_GPIO_STRENGTH_LOW 1
>> #define PMIC_GPIO_STRENGTH_MED 2
>> -#define PMIC_GPIO_STRENGTH_LOW 3
>> +#define PMIC_GPIO_STRENGTH_HIGH 3
>
> Didn't you just break all DTSes in the world?
Currently, all PMIC GPIO peripherals managed by the pinctrl-spmi-gpio
driver are having their drive strength control register programmed
incorrectly at runtime for the constant name used in DT (i.e.
PMIC_GPIO_STRENGTH_LOW vs PMIC_GPIO_STRENGTH_HIGH). Changing the values
of those constants as done in this patch fixes that incorrect behavior.
The qcom,drive-strength DT property is taking a raw drive strength
control register value instead of some logical strength abstraction.
I'm not sure of a better way to handle the situation than fixing the
incorrect drive strength constant to register value mapping as defined
in qcom,pmic-gpio.h.
Changing the mapping in qcom,pmic-gpio.h without updating any dtsi files
could cause a problem for very old targets that use SSBI instead of SPMI
for PMIC communication. However, for there to actually be a problem,
PMIC_GPIO_STRENGTH_LOW or PMIC_GPIO_STRENGTH_HIGH would need to be
specified for the SSBI PMIC. That would be GPIO devices with compatible
strings: "qcom,pm8018-gpio", "qcom,pm8038-gpio", "qcom,pm8058-gpio",
"qcom,pm8917-gpio", or "qcom,pm8921-gpio". I could find no instances of
this situation in the kernel source tree.
The PMIC_GPIO_STRENGTH_LOW or PMIC_GPIO_STRENGTH_HIGH usage in dtsi
files for SPMI PMICs does not need to be modified. The DT header patch
fixes configurations that are currently broken for them.
Note that the drive strength misconfiguration issue doesn't present a
problem for commercial products as this patch has been cherry-picked
downstream for several years.
Take care,
David
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 2/4] pinctrl: qcom: spmi-gpio: Fix the GPIO strength mapping
2022-09-09 0:25 ` David Collins
@ 2022-09-09 7:34 ` Krzysztof Kozlowski
0 siblings, 0 replies; 13+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-09 7:34 UTC (permalink / raw)
To: David Collins, Anjelique Melendez, agross, bjorn.andersson,
linus.walleij, robh+dt, krzysztof.kozlowski+dt
Cc: konrad.dybcio, linux-arm-msm, linux-gpio, devicetree,
linux-kernel, Anirudh Ghayal
On 09/09/2022 02:25, David Collins wrote:
>>> diff --git a/include/dt-bindings/pinctrl/qcom,pmic-gpio.h b/include/dt-bindings/pinctrl/qcom,pmic-gpio.h
>>> index e5df5ce45a0f..950be952ad3e 100644
>>> --- a/include/dt-bindings/pinctrl/qcom,pmic-gpio.h
>>> +++ b/include/dt-bindings/pinctrl/qcom,pmic-gpio.h
>>
>> You cannot mix bindings with driver. This is an ABI break.
>
> This could be split into two patches. However, both would need to make
> it into any given build to avoid runtime regressions when
> pinctrl-spmi-gpio.c rejects GPIO strength configurations larger than 1.
Which proves this is an ABI break. You need to gracefully handle in the
driver.
>
> I suppose that this kind of bi-directional dependency could be avoided
> by using one of these checks instead in the driver:
>
> if (arg > 3) {
>
> or
>
> if (arg > max(PMIC_GPIO_STRENGTH_LOW, PMIC_GPIO_STRENGTH_HIGH))
>
> Going this route would only require that the driver patch is picked up
> before the DT header patch.
You cannot change constants in the DT bindings header. Regardless
whether now or in the future - the constants are frozen. Otherwise it is
an ABI break. It would be acceptable only if existing feature was
completely broken and never worked.
>
>
>
>>> @@ -12,9 +12,14 @@
>>> #define PMIC_GPIO_PULL_UP_1P5_30 3
>>>
>>> #define PMIC_GPIO_STRENGTH_NO 0
>>> -#define PMIC_GPIO_STRENGTH_HIGH 1
>>> +#define PMIC_GPIO_STRENGTH_LOW 1
>>> #define PMIC_GPIO_STRENGTH_MED 2
>>> -#define PMIC_GPIO_STRENGTH_LOW 3
>>> +#define PMIC_GPIO_STRENGTH_HIGH 3
>>
>> Didn't you just break all DTSes in the world?
>
> Currently, all PMIC GPIO peripherals managed by the pinctrl-spmi-gpio
> driver are having their drive strength control register programmed
> incorrectly at runtime for the constant name used in DT (i.e.
> PMIC_GPIO_STRENGTH_LOW vs PMIC_GPIO_STRENGTH_HIGH). Changing the values
> of those constants as done in this patch fixes that incorrect behavior.
Wait. The values in the bindings should be only, *only* abstract ID
numbers. Not register values. How is it related to the value being
programmed in the driver? This is just an enum. If you have DTS with
PMIC_GPIO_STRENGTH_LOW you program 0xwhatever-you-wish. Not exactly
current value of "PMIC_GPIO_STRENGTH_LOW".
You need to fix the driver, not the bindings.
>
> The qcom,drive-strength DT property is taking a raw drive strength
> control register value instead of some logical strength abstraction.
> I'm not sure of a better way to handle the situation than fixing the
> incorrect drive strength constant to register value mapping as defined
> in qcom,pmic-gpio.h.
Bindings are not for defining register values, but to define the DTS.
Feel free to use binding constants for register values if they fit
you... but if they don't fit, fix the driver. Not the bindings.
>
> Changing the mapping in qcom,pmic-gpio.h without updating any dtsi files
> could cause a problem for very old targets that use SSBI instead of SPMI
> for PMIC communication. However, for there to actually be a problem,
> PMIC_GPIO_STRENGTH_LOW or PMIC_GPIO_STRENGTH_HIGH would need to be
> specified for the SSBI PMIC. That would be GPIO devices with compatible
> strings: "qcom,pm8018-gpio", "qcom,pm8038-gpio", "qcom,pm8058-gpio",
> "qcom,pm8917-gpio", or "qcom,pm8921-gpio". I could find no instances of
> this situation in the kernel source tree.
>
> The PMIC_GPIO_STRENGTH_LOW or PMIC_GPIO_STRENGTH_HIGH usage in dtsi
> files for SPMI PMICs does not need to be modified. The DT header patch
> fixes configurations that are currently broken for them.
>
> Note that the drive strength misconfiguration issue doesn't present a
> problem for commercial products as this patch has been cherry-picked
> downstream for several years.
It will affect several other out-of-tree users and other projects. Don't
think only about Qualcomm tree, but about entire Qualcomm ecosystem and
its users.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 3/4] pinctrl: qcom: spmi-gpio: Add compatible for PM7250B
2022-09-07 20:15 [PATCH 0/4] Add Support for Qualcomm SPMI GPIOs Anjelique Melendez
2022-09-07 20:15 ` [PATCH 1/4] pinctrl: qcom: spmi-gpio: add support for LV_VIN2 and MV_VIN3 subtypes Anjelique Melendez
2022-09-07 20:15 ` [PATCH 2/4] pinctrl: qcom: spmi-gpio: Fix the GPIO strength mapping Anjelique Melendez
@ 2022-09-07 20:15 ` Anjelique Melendez
2022-09-08 11:14 ` Krzysztof Kozlowski
2022-09-07 20:15 ` [PATCH 4/4] dt-bindings: qcom-pmic-gpio: Add PM7250B and PM8450 bindings Anjelique Melendez
3 siblings, 1 reply; 13+ messages in thread
From: Anjelique Melendez @ 2022-09-07 20:15 UTC (permalink / raw)
To: agross, bjorn.andersson, linus.walleij, robh+dt,
krzysztof.kozlowski+dt
Cc: konrad.dybcio, linux-arm-msm, linux-gpio, devicetree,
linux-kernel, Jishnu Prakash, David Collins, Anjelique Melendez
From: Jishnu Prakash <quic_jprakash@quicinc.org>
Add support for qcom,pm7250b-gpio variant.
Signed-off-by: Jishnu Prakash <quic_jprakash@quicinc.org>
Signed-off-by: David Collins <quic_collinsd@quicinc.org>
Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com>
---
drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index fceccf1ec099..1f40aacc49b9 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -1160,6 +1160,7 @@ static const struct of_device_id pmic_gpio_of_match[] = {
{ .compatible = "qcom,pm6150-gpio", .data = (void *) 10 },
{ .compatible = "qcom,pm6150l-gpio", .data = (void *) 12 },
{ .compatible = "qcom,pm6350-gpio", .data = (void *) 9 },
+ { .compatible = "qcom,pm7250b-gpio", .data = (void *) 12 },
{ .compatible = "qcom,pm7325-gpio", .data = (void *) 10 },
{ .compatible = "qcom,pm8005-gpio", .data = (void *) 4 },
{ .compatible = "qcom,pm8008-gpio", .data = (void *) 2 },
--
2.35.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 3/4] pinctrl: qcom: spmi-gpio: Add compatible for PM7250B
2022-09-07 20:15 ` [PATCH 3/4] pinctrl: qcom: spmi-gpio: Add compatible for PM7250B Anjelique Melendez
@ 2022-09-08 11:14 ` Krzysztof Kozlowski
0 siblings, 0 replies; 13+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-08 11:14 UTC (permalink / raw)
To: Anjelique Melendez, agross, bjorn.andersson, linus.walleij,
robh+dt, krzysztof.kozlowski+dt
Cc: konrad.dybcio, linux-arm-msm, linux-gpio, devicetree,
linux-kernel, Jishnu Prakash, David Collins
On 07/09/2022 22:15, Anjelique Melendez wrote:
> From: Jishnu Prakash <quic_jprakash@quicinc.org>
>
> Add support for qcom,pm7250b-gpio variant.
>
> Signed-off-by: Jishnu Prakash <quic_jprakash@quicinc.org>
> Signed-off-by: David Collins <quic_collinsd@quicinc.org>
> Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com>
> ---
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/4] dt-bindings: qcom-pmic-gpio: Add PM7250B and PM8450 bindings
2022-09-07 20:15 [PATCH 0/4] Add Support for Qualcomm SPMI GPIOs Anjelique Melendez
` (2 preceding siblings ...)
2022-09-07 20:15 ` [PATCH 3/4] pinctrl: qcom: spmi-gpio: Add compatible for PM7250B Anjelique Melendez
@ 2022-09-07 20:15 ` Anjelique Melendez
2022-09-08 11:13 ` Krzysztof Kozlowski
3 siblings, 1 reply; 13+ messages in thread
From: Anjelique Melendez @ 2022-09-07 20:15 UTC (permalink / raw)
To: agross, bjorn.andersson, linus.walleij, robh+dt,
krzysztof.kozlowski+dt
Cc: konrad.dybcio, linux-arm-msm, linux-gpio, devicetree,
linux-kernel, Anjelique Melendez
Update the Qualcomm Technologies, Inc. PMIC GPIO binding documentation
to include compatible strings for PM7250B and PM8450 PMICs.
Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com>
---
Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml
index 694898f382be..a548323e54f1 100644
--- a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml
@@ -24,6 +24,7 @@ properties:
- qcom,pm6150-gpio
- qcom,pm6150l-gpio
- qcom,pm6350-gpio
+ - qcom,pm7250b-gpio
- qcom,pm7325-gpio
- qcom,pm8005-gpio
- qcom,pm8008-gpio
@@ -392,6 +393,7 @@ $defs:
- gpio1-gpio10 for pm6150
- gpio1-gpio12 for pm6150l
- gpio1-gpio9 for pm6350
+ - gpio1-gpio12 for pm7250b
- gpio1-gpio10 for pm7325
- gpio1-gpio4 for pm8005
- gpio1-gpio2 for pm8008
@@ -407,6 +409,7 @@ $defs:
- gpio1-gpio10 for pm8350
- gpio1-gpio8 for pm8350b
- gpio1-gpio9 for pm8350c
+ - gpio1-gpio4 for pm8450
- gpio1-gpio38 for pm8917
- gpio1-gpio44 for pm8921
- gpio1-gpio36 for pm8941
--
2.35.1
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 4/4] dt-bindings: qcom-pmic-gpio: Add PM7250B and PM8450 bindings
2022-09-07 20:15 ` [PATCH 4/4] dt-bindings: qcom-pmic-gpio: Add PM7250B and PM8450 bindings Anjelique Melendez
@ 2022-09-08 11:13 ` Krzysztof Kozlowski
2022-09-08 16:55 ` Anjelique Melendez
0 siblings, 1 reply; 13+ messages in thread
From: Krzysztof Kozlowski @ 2022-09-08 11:13 UTC (permalink / raw)
To: Anjelique Melendez, agross, bjorn.andersson, linus.walleij,
robh+dt, krzysztof.kozlowski+dt
Cc: konrad.dybcio, linux-arm-msm, linux-gpio, devicetree,
linux-kernel
On 07/09/2022 22:15, Anjelique Melendez wrote:
> Update the Qualcomm Technologies, Inc. PMIC GPIO binding documentation
> to include compatible strings for PM7250B and PM8450 PMICs.
>
> Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com>
> ---
> Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml
> index 694898f382be..a548323e54f1 100644
> --- a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml
> @@ -24,6 +24,7 @@ properties:
> - qcom,pm6150-gpio
> - qcom,pm6150l-gpio
> - qcom,pm6350-gpio
> + - qcom,pm7250b-gpio
> - qcom,pm7325-gpio
> - qcom,pm8005-gpio
> - qcom,pm8008-gpio
This is incomplete. You need to update allOf.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 4/4] dt-bindings: qcom-pmic-gpio: Add PM7250B and PM8450 bindings
2022-09-08 11:13 ` Krzysztof Kozlowski
@ 2022-09-08 16:55 ` Anjelique Melendez
0 siblings, 0 replies; 13+ messages in thread
From: Anjelique Melendez @ 2022-09-08 16:55 UTC (permalink / raw)
To: Krzysztof Kozlowski, agross, bjorn.andersson, linus.walleij,
robh+dt, krzysztof.kozlowski+dt
Cc: konrad.dybcio, linux-arm-msm, linux-gpio, devicetree,
linux-kernel
On 9/8/2022 4:13 AM, Krzysztof Kozlowski wrote:
> On 07/09/2022 22:15, Anjelique Melendez wrote:
>> Update the Qualcomm Technologies, Inc. PMIC GPIO binding documentation
>> to include compatible strings for PM7250B and PM8450 PMICs.
>>
>> Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com>
>> ---
>> Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml
>> index 694898f382be..a548323e54f1 100644
>> --- a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml
>> +++ b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml
>> @@ -24,6 +24,7 @@ properties:
>> - qcom,pm6150-gpio
>> - qcom,pm6150l-gpio
>> - qcom,pm6350-gpio
>> + - qcom,pm7250b-gpio
>> - qcom,pm7325-gpio
>> - qcom,pm8005-gpio
>> - qcom,pm8008-gpio
>
> This is incomplete. You need to update allOf.
ACK - will add for next version
>
> Best regards,
> Krzysztof
^ permalink raw reply [flat|nested] 13+ messages in thread