[PATCH linux dev-5.3 2/4] pinctrl: aspeed-g6: Fix LPC/eSPI mux configuration

[Jae Hyun Yoo]

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.

Cheers,

Jae