| Message ID | 20220907201528.9351-3-quic_amelende@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add Support for Qualcomm SPMI GPIOs | expand |
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
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
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
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