Re: [PATCH v3] pinctrl: aspeed: Fix ast2500 strap register write logic

[Yong Li]

Hi Andrew,

Thanks for your review. I checked the patch before I sent out, but the
tool did not report any problems. Could you help to share your
checking commands?

scripts/checkpatch.pl
0001-pinctrl-aspeed-Fix-ast2500-strap-register-write-logi.patch
total: 0 errors, 0 warnings, 38 lines checked

Thanks,
Yong

2017-08-16 6:45 GMT-07:00 Andrew Jeffery <andrew@xxxxxxxx>:
> Hi Yong,
>
> On Wed, 2017-08-16 at 00:21 +0800, Yong Li wrote:
>> On AST2500, the hardware strap register(SCU70) only accepts write â1â,
>> to clear it to â0â, must set bits(write  â1â) to SCU7C
>>
>> Signed-off-by: Yong Li <sdliyong@xxxxxxxxx>
>
> ./scripts/checkpatch.pl complains about DOS line-endings thoughout -
> you should fix your editor to use Unix line endings (at least for
> kernel work). Maybe if Linus is feeling charitable he can fix it up for
> you. Regarding the meat of the change:
>
> Reviewed-by: Andrew Jeffery <andrew@xxxxxxxx>
> Tested-by: Andrew Jeffery <andrew@xxxxxxxx>
>
> Thanks for the patch!
>
> Andrew
>
>> ---
>>  drivers/pinctrl/aspeed/pinctrl-aspeed.c | 19 +++++++++++++++++--
>>  drivers/pinctrl/aspeed/pinctrl-aspeed.h |  1 +
>>  2 files changed, 18 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.c b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
>> index a86a4d6..f2d5133 100644
>> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed.c
>> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.c
>> @@ -183,6 +183,7 @@ static int aspeed_sig_expr_set(const struct aspeed_sig_expr *expr,
>>  {
>> >     int ret;
>> >     int i;
>> > +   unsigned int rev_id;
>>
>> >     for (i = 0; i < expr->ndescs; i++) {
>> >             const struct aspeed_sig_desc *desc = &expr->descs[i];
>> @@ -213,8 +214,22 @@ static int aspeed_sig_expr_set(const struct aspeed_sig_expr *expr,
>> >             if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP2)
>> >                     continue;
>>
>> > -           ret = regmap_update_bits(maps[desc->ip], desc->reg,
>> > -                                    desc->mask, val);
>> > +           /* On AST2500, Set bits in SCU7C are cleared from SCU70 */
>> > +           if (desc->ip == ASPEED_IP_SCU && desc->reg == HW_STRAP1) {
>> > +                   ret = regmap_read(maps[ASPEED_IP_SCU],
>> > +                           HW_REVISION_ID, &rev_id);
>> > +                   if (ret < 0)
>> > +                           return ret;
>> +
>> > +                   if (0x04 == ((rev_id >> 24) & 0xff))
>> > +                           ret = regmap_write(maps[desc->ip],
>> > +                                   HW_REVISION_ID, (~val & desc->mask));
>> > +                   else
>> > +                           ret = regmap_update_bits(maps[desc->ip],
>> > +                                   desc->reg, desc->mask, val);
>> > +           } else
>> > +                   ret = regmap_update_bits(maps[desc->ip], desc->reg,
>> > +                           desc->mask, val);
>>
>> >             if (ret)
>> >                     return ret;
>> diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed.h b/drivers/pinctrl/aspeed/pinctrl-aspeed.h
>> index fa125db..d4d7f03 100644
>> --- a/drivers/pinctrl/aspeed/pinctrl-aspeed.h
>> +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed.h
>> @@ -251,6 +251,7 @@
>>  #define SCU3C           0x3C /* System Reset Control/Status Register */
>>  #define SCU48           0x48 /* MAC Interface Clock Delay Setting */
>>  #define HW_STRAP1       0x70 /* AST2400 strapping is 33 bits, is split */
>> +#define HW_REVISION_ID  0x7C /* Silicon revision ID register */
>>  #define SCU80           0x80 /* Multi-function Pin Control #1 */
>>  #define SCU84           0x84 /* Multi-function Pin Control #2 */
>>  #define SCU88           0x88 /* Multi-function Pin Control #3 */