Re: [PATCH linux dev-5.3 2/4] pinctrl: aspeed-g6: Fix LPC/eSPI mux configuration
Andrew Jeffery
andrew at aj.id.au
Wed Oct 23 11:00:43 AEDT 2019
On Wed, 23 Oct 2019, at 05:42, Jae Hyun Yoo wrote:
> On 10/22/2019 11:34 AM, Jae Hyun Yoo wrote:
> > Hi Andrew,
> >
> > On 10/21/2019 9:47 PM, Andrew Jeffery wrote:
> >> Early revisions of the AST2600 datasheet are conflicted about the state
> >> of the LPC/eSPI strapping bit (SCU510[6]). Conversations with ASPEED
> >> determined that the reference pinmux configuration tables were in error
> >> and the SCU documentation contained the correct configuration. Update
> >> the driver to reflect the state described in the SCU documentation.
> >>
> >> Fixes: 2eda1cdec49f ("pinctrl: aspeed: Add AST2600 pinmux support")
> >> Signed-off-by: Andrew Jeffery <andrew at aj.id.au>
> >> ---
> >> drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c | 56 ++++++++++------------
> >> 1 file changed, 24 insertions(+), 32 deletions(-)
> >>
> >> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> >> b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> >> index eb0c11a9fbf2..fb96e8d2e6c8 100644
> >> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> >> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
> >> @@ -1098,61 +1098,53 @@ SSSF_PIN_DECL(AD15, GPIOV6, LPCPME,
> >> SIG_DESC_SET(SCU434, 14));
> >> SSSF_PIN_DECL(AF15, GPIOV7, LPCSMI, SIG_DESC_SET(SCU434, 15));
> >> #define AB7 176
> >> -SIG_EXPR_LIST_DECL_SESG(AB7, LAD0, LPC, SIG_DESC_SET(SCU434, 16),
> >> - SIG_DESC_CLEAR(SCU510, 6));
> >> -SIG_EXPR_LIST_DECL_SESG(AB7, ESPID0, ESPI, SIG_DESC_SET(SCU434, 16),
> >> - SIG_DESC_SET(SCU510, 6));
> >> +SIG_EXPR_LIST_DECL_SESG(AB7, LAD0, LPC, SIG_DESC_SET(SCU510, 6),
> >> + SIG_DESC_SET(SCU434, 16));
> >> +SIG_EXPR_LIST_DECL_SESG(AB7, ESPID0, ESPI, SIG_DESC_SET(SCU434, 16));
> >> PIN_DECL_2(AB7, GPIOW0, LAD0, ESPID0);
> >> #define AB8 177
> >> -SIG_EXPR_LIST_DECL_SESG(AB8, LAD1, LPC, SIG_DESC_SET(SCU434, 17),
> >> - SIG_DESC_CLEAR(SCU510, 6));
> >> -SIG_EXPR_LIST_DECL_SESG(AB8, ESPID1, ESPI, SIG_DESC_SET(SCU434, 17),
> >> - SIG_DESC_SET(SCU510, 6));
> >> +SIG_EXPR_LIST_DECL_SESG(AB8, LAD1, LPC, SIG_DESC_SET(SCU510, 6),
> >> + SIG_DESC_SET(SCU434, 17));
> >> +SIG_EXPR_LIST_DECL_SESG(AB8, ESPID1, ESPI, SIG_DESC_SET(SCU434, 17));
> >> PIN_DECL_2(AB8, GPIOW1, LAD1, ESPID1);
> >> #define AC8 178
> >> -SIG_EXPR_LIST_DECL_SESG(AC8, LAD2, LPC, SIG_DESC_SET(SCU434, 18),
> >> - SIG_DESC_CLEAR(SCU510, 6));
> >> -SIG_EXPR_LIST_DECL_SESG(AC8, ESPID2, ESPI, SIG_DESC_SET(SCU434, 18),
> >> - SIG_DESC_SET(SCU510, 6));
> >> +SIG_EXPR_LIST_DECL_SESG(AC8, LAD2, LPC, SIG_DESC_SET(SCU510, 6),
> >> + SIG_DESC_SET(SCU434, 18));
> >> +SIG_EXPR_LIST_DECL_SESG(AC8, ESPID2, ESPI, SIG_DESC_SET(SCU434, 18));
> >> PIN_DECL_2(AC8, GPIOW2, LAD2, ESPID2);
> >> #define AC7 179
> >> -SIG_EXPR_LIST_DECL_SESG(AC7, LAD3, LPC, SIG_DESC_SET(SCU434, 19),
> >> - SIG_DESC_CLEAR(SCU510, 6));
> >> -SIG_EXPR_LIST_DECL_SESG(AC7, ESPID3, ESPI, SIG_DESC_SET(SCU434, 19),
> >> - SIG_DESC_SET(SCU510, 6));
> >> +SIG_EXPR_LIST_DECL_SESG(AC7, LAD3, LPC, SIG_DESC_SET(SCU510, 6),
> >> + SIG_DESC_SET(SCU434, 19));
> >> +SIG_EXPR_LIST_DECL_SESG(AC7, ESPID3, ESPI, SIG_DESC_SET(SCU434, 19));
> >> PIN_DECL_2(AC7, GPIOW3, LAD3, ESPID3);
> >> #define AE7 180
> >> -SIG_EXPR_LIST_DECL_SESG(AE7, LCLK, LPC, SIG_DESC_SET(SCU434, 20),
> >> - SIG_DESC_CLEAR(SCU510, 6));
> >> -SIG_EXPR_LIST_DECL_SESG(AE7, ESPICK, ESPI, SIG_DESC_SET(SCU434, 20),
> >> - SIG_DESC_SET(SCU510, 6));
> >> +SIG_EXPR_LIST_DECL_SESG(AE7, LCLK, LPC, SIG_DESC_SET(SCU510, 6),
> >> + SIG_DESC_SET(SCU434, 20));
> >> +SIG_EXPR_LIST_DECL_SESG(AE7, ESPICK, ESPI, SIG_DESC_SET(SCU434, 20));
> >> PIN_DECL_2(AE7, GPIOW4, LCLK, ESPICK);
> >> #define AF7 181
> >> -SIG_EXPR_LIST_DECL_SESG(AF7, LFRAME, LPC, SIG_DESC_SET(SCU434, 21),
> >> - SIG_DESC_CLEAR(SCU510, 6));
> >> -SIG_EXPR_LIST_DECL_SESG(AF7, ESPICS, ESPI, SIG_DESC_SET(SCU434, 21),
> >> - SIG_DESC_SET(SCU510, 6));
> >> +SIG_EXPR_LIST_DECL_SESG(AF7, LFRAME, LPC, SIG_DESC_SET(SCU510, 6),
> >> + SIG_DESC_SET(SCU434, 21));
> >> +SIG_EXPR_LIST_DECL_SESG(AF7, ESPICS, ESPI, SIG_DESC_SET(SCU434, 21));
> >> PIN_DECL_2(AF7, GPIOW5, LFRAME, ESPICS);
> >> #define AD7 182
> >> -SIG_EXPR_LIST_DECL_SESG(AD7, LSIRQ, LSIRQ, SIG_DESC_SET(SCU434, 22),
> >> - SIG_DESC_CLEAR(SCU510, 6));
> >> -SIG_EXPR_LIST_DECL_SESG(AD7, ESPIALT, ESPIALT, SIG_DESC_SET(SCU434, 22),
> >> - SIG_DESC_SET(SCU510, 6));
> >> +SIG_EXPR_LIST_DECL_SESG(AD7, LSIRQ, LSIRQ, SIG_DESC_SET(SCU510, 6),
> >> + SIG_DESC_SET(SCU434, 22));
> >> +SIG_EXPR_LIST_DECL_SESG(AD7, ESPIALT, ESPIALT, SIG_DESC_SET(SCU434,
> >> 22));
> >> PIN_DECL_2(AD7, GPIOW6, LSIRQ, ESPIALT);
> >> FUNC_GROUP_DECL(LSIRQ, AD7);
> >> FUNC_GROUP_DECL(ESPIALT, AD7);
> >> #define AD8 183
> >> -SIG_EXPR_LIST_DECL_SESG(AD8, LPCRST, LPC, SIG_DESC_SET(SCU434, 23),
> >> - SIG_DESC_CLEAR(SCU510, 6));
> >> -SIG_EXPR_LIST_DECL_SESG(AD8, ESPIRST, ESPI, SIG_DESC_SET(SCU434, 23),
> >> - SIG_DESC_SET(SCU510, 6));
> >> +SIG_EXPR_LIST_DECL_SESG(AD8, LPCRST, LPC, SIG_DESC_SET(SCU510, 6),
> >> + SIG_DESC_SET(SCU434, 23));
> >> +SIG_EXPR_LIST_DECL_SESG(AD8, ESPIRST, ESPI, SIG_DESC_SET(SCU434, 23));
> >> PIN_DECL_2(AD8, GPIOW7, LPCRST, ESPIRST);
> >> FUNC_GROUP_DECL(LPC, AB7, AB8, AC8, AC7, AE7, AF7, AD8);
> >
> > Does it need AD7 too in this group?
> >
> > I think it should be:
> > FUNC_GROUP_DECL(LPC, AB7, AB8, AC8, AC7, AE7, AF7, AD7, AD8);
> > FUNC_GROUP_DECL(ESPI, AB7, AB8, AC8, AC7, AE7, AF7, AD7, AD8);
>
> Ah, I realized that there are seperate group definitions for LSIRQ and
> ESPIALT. Ignore my comment.
Yeah, I grouped them based on required/optional signals in the LPC/eSPI specs.
I suspect we'll always want to mux e.g. LSIRQ if we're using LPC, but who knows.
I'm trying not to constrain what lines can be used for GPIO where possible.
Thanks for taking a look :)
Andrew
More information about the openbmc
mailing list