* [PATCH 0/3] pinctrl: aspeed: AST2600 pinconf fixes
@ 2020-09-09 11:43 Andrew Jeffery
2020-09-09 11:43 ` [PATCH 1/3] pinctrl: aspeed: Format pinconf debug consistent with pinmux Andrew Jeffery
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Andrew Jeffery @ 2020-09-09 11:43 UTC (permalink / raw)
To: linux-gpio
Cc: linus.walleij, joel, johnny_huang, linux-aspeed, openbmc,
linux-arm-kernel, linux-kernel
Hello,
The AST2600 pinctrl driver was missing support for bias control on the 1.8V
GPIO pins, and in the process of resolving that I discovered a couple of other
bugs that are fixed in the first two patches of the series.
Please review!
Andrew
Andrew Jeffery (3):
pinctrl: aspeed: Format pinconf debug consistent with pinmux
pinctrl: aspeed: Use the right pinconf mask
pinctrl: aspeed-g6: Add bias controls for 1.8V GPIO banks
drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c | 17 +++++++++++++++++
drivers/pinctrl/aspeed/pinctrl-aspeed.c | 8 ++++----
2 files changed, 21 insertions(+), 4 deletions(-)
--
2.25.1
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/3] pinctrl: aspeed: Format pinconf debug consistent with pinmux 2020-09-09 11:43 [PATCH 0/3] pinctrl: aspeed: AST2600 pinconf fixes Andrew Jeffery @ 2020-09-09 11:43 ` Andrew Jeffery 2020-09-10 1:52 ` Joel Stanley 2020-09-09 11:43 ` [PATCH 2/3] pinctrl: aspeed: Use the right pinconf mask Andrew Jeffery ` (2 subsequent siblings) 3 siblings, 1 reply; 9+ messages in thread From: Andrew Jeffery @ 2020-09-09 11:43 UTC (permalink / raw) To: linux-gpio Cc: linus.walleij, joel, johnny_huang, linux-aspeed, openbmc, linux-arm-kernel, linux-kernel When displaying which pinconf register and field is being touched, format the field mask so that it's consistent with the way the pinmux portion formats the mask. Signed-off-by: Andrew Jeffery <andrew@aj.id.au> --- drivers/pinctrl/aspeed/pinctrl-aspeed.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c b/drivers/pinctrl/aspeed/pinctrl-aspeed.c index 53f3f8aec695..d8972911d505 100644 --- a/drivers/pinctrl/aspeed/pinctrl-aspeed.c +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.c @@ -539,9 +539,9 @@ int aspeed_pin_config_set(struct pinctrl_dev *pctldev, unsigned int offset, if (rc < 0) return rc; - pr_debug("%s: Set SCU%02X[%lu]=%d for param %d(=%d) on pin %d\n", - __func__, pconf->reg, __ffs(pconf->mask), - pmap->val, param, arg, offset); + pr_debug("%s: Set SCU%02X[0x%08X]=%d for param %d(=%d) on pin %d\n", + __func__, pconf->reg, pconf->mask, + val, param, arg, offset); } return 0; -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] pinctrl: aspeed: Format pinconf debug consistent with pinmux 2020-09-09 11:43 ` [PATCH 1/3] pinctrl: aspeed: Format pinconf debug consistent with pinmux Andrew Jeffery @ 2020-09-10 1:52 ` Joel Stanley 2020-09-10 2:15 ` Andrew Jeffery 0 siblings, 1 reply; 9+ messages in thread From: Joel Stanley @ 2020-09-10 1:52 UTC (permalink / raw) To: Andrew Jeffery Cc: open list:GPIO SUBSYSTEM, Linus Walleij, johnny_huang, linux-aspeed, OpenBMC Maillist, Linux ARM, Linux Kernel Mailing List On Wed, 9 Sep 2020 at 11:43, Andrew Jeffery <andrew@aj.id.au> wrote: > > When displaying which pinconf register and field is being touched, format the > field mask so that it's consistent with the way the pinmux portion > formats the mask. > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > --- > drivers/pinctrl/aspeed/pinctrl-aspeed.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c b/drivers/pinctrl/aspeed/pinctrl-aspeed.c > index 53f3f8aec695..d8972911d505 100644 > --- a/drivers/pinctrl/aspeed/pinctrl-aspeed.c > +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.c > @@ -539,9 +539,9 @@ int aspeed_pin_config_set(struct pinctrl_dev *pctldev, unsigned int offset, > if (rc < 0) > return rc; > > - pr_debug("%s: Set SCU%02X[%lu]=%d for param %d(=%d) on pin %d\n", > - __func__, pconf->reg, __ffs(pconf->mask), > - pmap->val, param, arg, offset); > + pr_debug("%s: Set SCU%02X[0x%08X]=%d for param %d(=%d) on pin %d\n", The pr_debug in pinmux-aspeed.c prints val as 0x%X. Did you want to do that here? > + __func__, pconf->reg, pconf->mask, > + val, param, arg, offset); > } > > return 0; > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] pinctrl: aspeed: Format pinconf debug consistent with pinmux 2020-09-10 1:52 ` Joel Stanley @ 2020-09-10 2:15 ` Andrew Jeffery 0 siblings, 0 replies; 9+ messages in thread From: Andrew Jeffery @ 2020-09-10 2:15 UTC (permalink / raw) To: Joel Stanley Cc: open list:GPIO SUBSYSTEM, Linus Walleij, Johnny Huang, linux-aspeed, OpenBMC Maillist, Linux ARM, Linux Kernel Mailing List On Thu, 10 Sep 2020, at 11:22, Joel Stanley wrote: > On Wed, 9 Sep 2020 at 11:43, Andrew Jeffery <andrew@aj.id.au> wrote: > > > > When displaying which pinconf register and field is being touched, format the > > field mask so that it's consistent with the way the pinmux portion > > formats the mask. > > > > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > > --- > > drivers/pinctrl/aspeed/pinctrl-aspeed.c | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c b/drivers/pinctrl/aspeed/pinctrl-aspeed.c > > index 53f3f8aec695..d8972911d505 100644 > > --- a/drivers/pinctrl/aspeed/pinctrl-aspeed.c > > +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.c > > @@ -539,9 +539,9 @@ int aspeed_pin_config_set(struct pinctrl_dev *pctldev, unsigned int offset, > > if (rc < 0) > > return rc; > > > > - pr_debug("%s: Set SCU%02X[%lu]=%d for param %d(=%d) on pin %d\n", > > - __func__, pconf->reg, __ffs(pconf->mask), > > - pmap->val, param, arg, offset); > > + pr_debug("%s: Set SCU%02X[0x%08X]=%d for param %d(=%d) on pin %d\n", > > > The pr_debug in pinmux-aspeed.c prints val as 0x%X. Did you want to do > that here? Fair point, I'll do a v2. Andrew ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/3] pinctrl: aspeed: Use the right pinconf mask 2020-09-09 11:43 [PATCH 0/3] pinctrl: aspeed: AST2600 pinconf fixes Andrew Jeffery 2020-09-09 11:43 ` [PATCH 1/3] pinctrl: aspeed: Format pinconf debug consistent with pinmux Andrew Jeffery @ 2020-09-09 11:43 ` Andrew Jeffery 2020-09-10 1:52 ` Joel Stanley 2020-09-09 11:43 ` [PATCH 3/3] pinctrl: aspeed-g6: Add bias controls for 1.8V GPIO banks Andrew Jeffery 2020-09-29 12:25 ` [PATCH 0/3] pinctrl: aspeed: AST2600 pinconf fixes Linus Walleij 3 siblings, 1 reply; 9+ messages in thread From: Andrew Jeffery @ 2020-09-09 11:43 UTC (permalink / raw) To: linux-gpio Cc: linus.walleij, joel, johnny_huang, linux-aspeed, openbmc, linux-arm-kernel, linux-kernel The Aspeed pinconf data structures are split into 'conf' and 'map' types, where the 'conf' struct defines which register and bitfield to manipulate, while the 'map' struct defines what value to write to the register and bitfield. Both structs have a mask member, and the wrong mask was being used to tell the regmap which bits to update. A todo is to look at whether we can remove the mask from the 'map' struct. Cc: Johnny Huang <johnny_huang@aspeedtech.com> Fixes: 5f52c853847f ("pinctrl: aspeed: Use masks to describe pinconf bitfields") Signed-off-by: Andrew Jeffery <andrew@aj.id.au> --- drivers/pinctrl/aspeed/pinctrl-aspeed.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c b/drivers/pinctrl/aspeed/pinctrl-aspeed.c index d8972911d505..e03ee78b2434 100644 --- a/drivers/pinctrl/aspeed/pinctrl-aspeed.c +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.c @@ -534,7 +534,7 @@ int aspeed_pin_config_set(struct pinctrl_dev *pctldev, unsigned int offset, val = pmap->val << __ffs(pconf->mask); rc = regmap_update_bits(pdata->scu, pconf->reg, - pmap->mask, val); + pconf->mask, val); if (rc < 0) return rc; -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/3] pinctrl: aspeed: Use the right pinconf mask 2020-09-09 11:43 ` [PATCH 2/3] pinctrl: aspeed: Use the right pinconf mask Andrew Jeffery @ 2020-09-10 1:52 ` Joel Stanley 0 siblings, 0 replies; 9+ messages in thread From: Joel Stanley @ 2020-09-10 1:52 UTC (permalink / raw) To: Andrew Jeffery Cc: open list:GPIO SUBSYSTEM, Linus Walleij, johnny_huang, linux-aspeed, OpenBMC Maillist, Linux ARM, Linux Kernel Mailing List On Wed, 9 Sep 2020 at 11:43, Andrew Jeffery <andrew@aj.id.au> wrote: > > The Aspeed pinconf data structures are split into 'conf' and 'map' > types, where the 'conf' struct defines which register and bitfield to > manipulate, while the 'map' struct defines what value to write to > the register and bitfield. > > Both structs have a mask member, and the wrong mask was being used to > tell the regmap which bits to update. > > A todo is to look at whether we can remove the mask from the 'map' > struct. > > Cc: Johnny Huang <johnny_huang@aspeedtech.com> > Fixes: 5f52c853847f ("pinctrl: aspeed: Use masks to describe pinconf bitfields") > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Owch. Reviewed-by: Joel Stanley <joel@jms.id.au> > --- > drivers/pinctrl/aspeed/pinctrl-aspeed.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c b/drivers/pinctrl/aspeed/pinctrl-aspeed.c > index d8972911d505..e03ee78b2434 100644 > --- a/drivers/pinctrl/aspeed/pinctrl-aspeed.c > +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.c > @@ -534,7 +534,7 @@ int aspeed_pin_config_set(struct pinctrl_dev *pctldev, unsigned int offset, > val = pmap->val << __ffs(pconf->mask); > > rc = regmap_update_bits(pdata->scu, pconf->reg, > - pmap->mask, val); > + pconf->mask, val); > > if (rc < 0) > return rc; > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 3/3] pinctrl: aspeed-g6: Add bias controls for 1.8V GPIO banks 2020-09-09 11:43 [PATCH 0/3] pinctrl: aspeed: AST2600 pinconf fixes Andrew Jeffery 2020-09-09 11:43 ` [PATCH 1/3] pinctrl: aspeed: Format pinconf debug consistent with pinmux Andrew Jeffery 2020-09-09 11:43 ` [PATCH 2/3] pinctrl: aspeed: Use the right pinconf mask Andrew Jeffery @ 2020-09-09 11:43 ` Andrew Jeffery 2020-09-10 2:02 ` Joel Stanley 2020-09-29 12:25 ` [PATCH 0/3] pinctrl: aspeed: AST2600 pinconf fixes Linus Walleij 3 siblings, 1 reply; 9+ messages in thread From: Andrew Jeffery @ 2020-09-09 11:43 UTC (permalink / raw) To: linux-gpio Cc: linus.walleij, joel, johnny_huang, linux-aspeed, openbmc, linux-arm-kernel, linux-kernel These were skipped in the original patches adding pinconf support for the AST2600. Cc: Johnny Huang <johnny_huang@aspeedtech.com> Signed-off-by: Andrew Jeffery <andrew@aj.id.au> --- drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c index 7efe6dbe4398..34803a6c7664 100644 --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c @@ -19,6 +19,7 @@ #define SCU400 0x400 /* Multi-function Pin Control #1 */ #define SCU404 0x404 /* Multi-function Pin Control #2 */ +#define SCU40C 0x40C /* Multi-function Pin Control #3 */ #define SCU410 0x410 /* Multi-function Pin Control #4 */ #define SCU414 0x414 /* Multi-function Pin Control #5 */ #define SCU418 0x418 /* Multi-function Pin Control #6 */ @@ -2591,6 +2592,22 @@ static struct aspeed_pin_config aspeed_g6_configs[] = { /* MAC4 */ { PIN_CONFIG_POWER_SOURCE, { F24, B24 }, SCU458, BIT_MASK(5)}, { PIN_CONFIG_DRIVE_STRENGTH, { F24, B24 }, SCU458, GENMASK(3, 2)}, + + /* GPIO18E */ + ASPEED_SB_PINCONF(PIN_CONFIG_BIAS_PULL_DOWN, Y1, Y4, SCU40C, 4), + ASPEED_SB_PINCONF(PIN_CONFIG_BIAS_DISABLE, Y1, Y4, SCU40C, 4), + /* GPIO18D */ + ASPEED_SB_PINCONF(PIN_CONFIG_BIAS_PULL_DOWN, AB4, AC5, SCU40C, 3), + ASPEED_SB_PINCONF(PIN_CONFIG_BIAS_DISABLE, AB4, AC5, SCU40C, 3), + /* GPIO18C */ + ASPEED_SB_PINCONF(PIN_CONFIG_BIAS_PULL_DOWN, E4, E1, SCU40C, 2), + ASPEED_SB_PINCONF(PIN_CONFIG_BIAS_DISABLE, E4, E1, SCU40C, 2), + /* GPIO18B */ + ASPEED_SB_PINCONF(PIN_CONFIG_BIAS_PULL_DOWN, B2, D3, SCU40C, 1), + ASPEED_SB_PINCONF(PIN_CONFIG_BIAS_DISABLE, B2, D3, SCU40C, 1), + /* GPIO18A */ + ASPEED_SB_PINCONF(PIN_CONFIG_BIAS_PULL_DOWN, C6, A2, SCU40C, 0), + ASPEED_SB_PINCONF(PIN_CONFIG_BIAS_DISABLE, C6, A2, SCU40C, 0), }; /** -- 2.25.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] pinctrl: aspeed-g6: Add bias controls for 1.8V GPIO banks 2020-09-09 11:43 ` [PATCH 3/3] pinctrl: aspeed-g6: Add bias controls for 1.8V GPIO banks Andrew Jeffery @ 2020-09-10 2:02 ` Joel Stanley 0 siblings, 0 replies; 9+ messages in thread From: Joel Stanley @ 2020-09-10 2:02 UTC (permalink / raw) To: Andrew Jeffery Cc: open list:GPIO SUBSYSTEM, Linus Walleij, johnny_huang, linux-aspeed, OpenBMC Maillist, Linux ARM, Linux Kernel Mailing List On Wed, 9 Sep 2020 at 11:43, Andrew Jeffery <andrew@aj.id.au> wrote: > > These were skipped in the original patches adding pinconf support for > the AST2600. > > Cc: Johnny Huang <johnny_huang@aspeedtech.com> > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> Reviewed-by: Joel Stanley <joel@jms.id.au> > --- > drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c | 17 +++++++++++++++++ > 1 file changed, 17 insertions(+) > > diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c > index 7efe6dbe4398..34803a6c7664 100644 > --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c > +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c > @@ -19,6 +19,7 @@ > > #define SCU400 0x400 /* Multi-function Pin Control #1 */ > #define SCU404 0x404 /* Multi-function Pin Control #2 */ > +#define SCU40C 0x40C /* Multi-function Pin Control #3 */ > #define SCU410 0x410 /* Multi-function Pin Control #4 */ > #define SCU414 0x414 /* Multi-function Pin Control #5 */ > #define SCU418 0x418 /* Multi-function Pin Control #6 */ > @@ -2591,6 +2592,22 @@ static struct aspeed_pin_config aspeed_g6_configs[] = { > /* MAC4 */ > { PIN_CONFIG_POWER_SOURCE, { F24, B24 }, SCU458, BIT_MASK(5)}, > { PIN_CONFIG_DRIVE_STRENGTH, { F24, B24 }, SCU458, GENMASK(3, 2)}, > + > + /* GPIO18E */ > + ASPEED_SB_PINCONF(PIN_CONFIG_BIAS_PULL_DOWN, Y1, Y4, SCU40C, 4), > + ASPEED_SB_PINCONF(PIN_CONFIG_BIAS_DISABLE, Y1, Y4, SCU40C, 4), > + /* GPIO18D */ > + ASPEED_SB_PINCONF(PIN_CONFIG_BIAS_PULL_DOWN, AB4, AC5, SCU40C, 3), > + ASPEED_SB_PINCONF(PIN_CONFIG_BIAS_DISABLE, AB4, AC5, SCU40C, 3), > + /* GPIO18C */ > + ASPEED_SB_PINCONF(PIN_CONFIG_BIAS_PULL_DOWN, E4, E1, SCU40C, 2), > + ASPEED_SB_PINCONF(PIN_CONFIG_BIAS_DISABLE, E4, E1, SCU40C, 2), > + /* GPIO18B */ > + ASPEED_SB_PINCONF(PIN_CONFIG_BIAS_PULL_DOWN, B2, D3, SCU40C, 1), > + ASPEED_SB_PINCONF(PIN_CONFIG_BIAS_DISABLE, B2, D3, SCU40C, 1), > + /* GPIO18A */ > + ASPEED_SB_PINCONF(PIN_CONFIG_BIAS_PULL_DOWN, C6, A2, SCU40C, 0), > + ASPEED_SB_PINCONF(PIN_CONFIG_BIAS_DISABLE, C6, A2, SCU40C, 0), > }; > > /** > -- > 2.25.1 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] pinctrl: aspeed: AST2600 pinconf fixes 2020-09-09 11:43 [PATCH 0/3] pinctrl: aspeed: AST2600 pinconf fixes Andrew Jeffery ` (2 preceding siblings ...) 2020-09-09 11:43 ` [PATCH 3/3] pinctrl: aspeed-g6: Add bias controls for 1.8V GPIO banks Andrew Jeffery @ 2020-09-29 12:25 ` Linus Walleij 3 siblings, 0 replies; 9+ messages in thread From: Linus Walleij @ 2020-09-29 12:25 UTC (permalink / raw) To: Andrew Jeffery Cc: open list:GPIO SUBSYSTEM, Joel Stanley, Johnny Huang, linux-aspeed, OpenBMC Maillist, Linux ARM, linux-kernel On Wed, Sep 9, 2020 at 1:43 PM Andrew Jeffery <andrew@aj.id.au> wrote: > The AST2600 pinctrl driver was missing support for bias control on the 1.8V > GPIO pins, and in the process of resolving that I discovered a couple of other > bugs that are fixed in the first two patches of the series. All 3 patches applied. Yours, Linus Walleij ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-09-29 12:26 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-09 11:43 [PATCH 0/3] pinctrl: aspeed: AST2600 pinconf fixes Andrew Jeffery 2020-09-09 11:43 ` [PATCH 1/3] pinctrl: aspeed: Format pinconf debug consistent with pinmux Andrew Jeffery 2020-09-10 1:52 ` Joel Stanley 2020-09-10 2:15 ` Andrew Jeffery 2020-09-09 11:43 ` [PATCH 2/3] pinctrl: aspeed: Use the right pinconf mask Andrew Jeffery 2020-09-10 1:52 ` Joel Stanley 2020-09-09 11:43 ` [PATCH 3/3] pinctrl: aspeed-g6: Add bias controls for 1.8V GPIO banks Andrew Jeffery 2020-09-10 2:02 ` Joel Stanley 2020-09-29 12:25 ` [PATCH 0/3] pinctrl: aspeed: AST2600 pinconf fixes Linus Walleij
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).