lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Thu, 17 Dec 2020 02:38:26 +0000
From:   Billy Tsai <billy_tsai@...eedtech.com>
To:     Andrew Jeffery <andrew@...id.au>
CC:     BMC-SW <BMC-SW@...eedtech.com>, "joel@....id.au" <joel@....id.au>,
        "linus.walleij@...aro.org" <linus.walleij@...aro.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "linux-aspeed@...ts.ozlabs.org" <linux-aspeed@...ts.ozlabs.org>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "openbmc@...ts.ozlabs.org" <openbmc@...ts.ozlabs.org>
Subject: Re: [PATCH v2] driver: aspeed: g6: Fix PWMG0 pinctrl setting

Hi Andrew,

Best Regards,
Billy Tsai

´╗┐On 2020/12/17, 8:38 AM, Andrew Jeffery wrote:

    > The SCU offset for signal PWM8 in group PWM8G0 is wrong, fix it from
    > SCU414 to SCU4B4.
    > Besides that, When PWM8~15 of PWMG0 set it needs to clear SCU414 bits
    > at the same time.
    
    FYI, we don't need to explicitly clear SCU414[...] as part of the PWM mux 
    configuration as the these bits are cleared as part of disabling the SD1* 
    signal state on each pin[1]. You should be able to confirm this by compiling 
    with CONFIG_DEBUG_PINCTRL=y and "debug" on the kernel commandline.
    
    That said, it would be neat if we had some kunit tests to exercise all this, 
    but it's not something I've thought deeply about.

Thanks for your remainder. I will send v3 to just fix the copy/paste error.

    [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pinctrl/aspeed/pinctrl-aspeed.c?h=v5.10#n248
    
    >
    > Fixes: 2eda1cdec49f ("pinctrl: aspeed: Add AST2600 pinmux support")
    >
    > Signed-off-by: Billy Tsai <billy_tsai@...eedtech.com>
    > ---
    >  drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c | 24 ++++++++++++++--------
    >  1 file changed, 16 insertions(+), 8 deletions(-)
    >
    > diff --git a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
    > index b673a44ffa3b..1dfb12a5b2ce 100644
    > --- a/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
    > +++ b/drivers/pinctrl/aspeed/pinctrl-aspeed-g6.c
    > @@ -367,49 +367,57 @@ FUNC_GROUP_DECL(RMII4, F24, E23, E24, E25, C25, C24, B26, B25, B24);
    >
    >  #define D22 40
    >  SIG_EXPR_LIST_DECL_SESG(D22, SD1CLK, SD1, SIG_DESC_SET(SCU414, 8));
    > -SIG_EXPR_LIST_DECL_SEMG(D22, PWM8, PWM8G0, PWM8, SIG_DESC_SET(SCU414, 8));
    > +SIG_EXPR_LIST_DECL_SEMG(D22, PWM8, PWM8G0, PWM8, SIG_DESC_SET(SCU4B4, 8),
    
    Good catch, looks like a copy/paste fail on my part :)
    
    > +SIG_DESC_CLEAR(SCU414, 8));
    
    As above, this should be unnecessary.
    
    Can you confirm and remove the CLEAR()s for v3?
    
    Cheers,
    
    Andrew
    
    >  PIN_DECL_2(D22, GPIOF0, SD1CLK, PWM8);
    >  GROUP_DECL(PWM8G0, D22);
    >
    >  #define E22 41
    >  SIG_EXPR_LIST_DECL_SESG(E22, SD1CMD, SD1, SIG_DESC_SET(SCU414, 9));
    > -SIG_EXPR_LIST_DECL_SEMG(E22, PWM9, PWM9G0, PWM9, SIG_DESC_SET(SCU4B4, 9));
    > +SIG_EXPR_LIST_DECL_SEMG(E22, PWM9, PWM9G0, PWM9, SIG_DESC_SET(SCU4B4, 9),
    > +SIG_DESC_CLEAR(SCU414, 9));
    >  PIN_DECL_2(E22, GPIOF1, SD1CMD, PWM9);
    >  GROUP_DECL(PWM9G0, E22);
    >
    >  #define D23 42
    >  SIG_EXPR_LIST_DECL_SESG(D23, SD1DAT0, SD1, SIG_DESC_SET(SCU414, 10));
    > -SIG_EXPR_LIST_DECL_SEMG(D23, PWM10, PWM10G0, PWM10, SIG_DESC_SET(SCU4B4, 10));
    > +SIG_EXPR_LIST_DECL_SEMG(D23, PWM10, PWM10G0, PWM10, SIG_DESC_SET(SCU4B4, 10),
    > +SIG_DESC_CLEAR(SCU414, 10));
    >  PIN_DECL_2(D23, GPIOF2, SD1DAT0, PWM10);
    >  GROUP_DECL(PWM10G0, D23);
    >
    >  #define C23 43
    >  SIG_EXPR_LIST_DECL_SESG(C23, SD1DAT1, SD1, SIG_DESC_SET(SCU414, 11));
    > -SIG_EXPR_LIST_DECL_SEMG(C23, PWM11, PWM11G0, PWM11, SIG_DESC_SET(SCU4B4, 11));
    > +SIG_EXPR_LIST_DECL_SEMG(C23, PWM11, PWM11G0, PWM11, SIG_DESC_SET(SCU4B4, 11),
    > +SIG_DESC_CLEAR(SCU414, 11));
    >  PIN_DECL_2(C23, GPIOF3, SD1DAT1, PWM11);
    >  GROUP_DECL(PWM11G0, C23);
    >
    >  #define C22 44
    >  SIG_EXPR_LIST_DECL_SESG(C22, SD1DAT2, SD1, SIG_DESC_SET(SCU414, 12));
    > -SIG_EXPR_LIST_DECL_SEMG(C22, PWM12, PWM12G0, PWM12, SIG_DESC_SET(SCU4B4, 12));
    > +SIG_EXPR_LIST_DECL_SEMG(C22, PWM12, PWM12G0, PWM12, SIG_DESC_SET(SCU4B4, 12),
    > +SIG_DESC_CLEAR(SCU414, 12));
    >  PIN_DECL_2(C22, GPIOF4, SD1DAT2, PWM12);
    >  GROUP_DECL(PWM12G0, C22);
    >
    >  #define A25 45
    >  SIG_EXPR_LIST_DECL_SESG(A25, SD1DAT3, SD1, SIG_DESC_SET(SCU414, 13));
    > -SIG_EXPR_LIST_DECL_SEMG(A25, PWM13, PWM13G0, PWM13, SIG_DESC_SET(SCU4B4, 13));
    > +SIG_EXPR_LIST_DECL_SEMG(A25, PWM13, PWM13G0, PWM13, SIG_DESC_SET(SCU4B4, 13),
    > +SIG_DESC_CLEAR(SCU414, 13));
    >  PIN_DECL_2(A25, GPIOF5, SD1DAT3, PWM13);
    >  GROUP_DECL(PWM13G0, A25);
    >
    >  #define A24 46
    >  SIG_EXPR_LIST_DECL_SESG(A24, SD1CD, SD1, SIG_DESC_SET(SCU414, 14));
    > -SIG_EXPR_LIST_DECL_SEMG(A24, PWM14, PWM14G0, PWM14, SIG_DESC_SET(SCU4B4, 14));
    > +SIG_EXPR_LIST_DECL_SEMG(A24, PWM14, PWM14G0, PWM14, SIG_DESC_SET(SCU4B4, 14),
    > +SIG_DESC_CLEAR(SCU414, 14));
    >  PIN_DECL_2(A24, GPIOF6, SD1CD, PWM14);
    >  GROUP_DECL(PWM14G0, A24);
    >
    >  #define A23 47
    >  SIG_EXPR_LIST_DECL_SESG(A23, SD1WP, SD1, SIG_DESC_SET(SCU414, 15));
    > -SIG_EXPR_LIST_DECL_SEMG(A23, PWM15, PWM15G0, PWM15, SIG_DESC_SET(SCU4B4, 15));
    > +SIG_EXPR_LIST_DECL_SEMG(A23, PWM15, PWM15G0, PWM15, SIG_DESC_SET(SCU4B4, 15),
    > +SIG_DESC_CLEAR(SCU414, 15));
    >  PIN_DECL_2(A23, GPIOF7, SD1WP, PWM15);
    >  GROUP_DECL(PWM15G0, A23);
    >
    > --
    > 2.17.1
    

Powered by blists - more mailing lists