linux-kernel - Re: [PATCH] pinctrl: renesas: rzg2l: Use configurable PFC mask

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 linux-cve-announce 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]

Message-ID: <CA+V-a8szu6-XeCQU0_6EM64YB_rsZOqpjimfRfmGT02enEToKg@mail.gmail.com>
Date: Fri, 10 Jan 2025 11:52:12 +0000
From: "Lad, Prabhakar" <prabhakar.csengg@...il.com>
To: Geert Uytterhoeven <geert@...ux-m68k.org>
Cc: Linus Walleij <linus.walleij@...aro.org>, linux-renesas-soc@...r.kernel.org, 
	linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Biju Das <biju.das.jz@...renesas.com>, 
	Fabrizio Castro <fabrizio.castro.jz@...esas.com>, 
	Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
Subject: Re: [PATCH] pinctrl: renesas: rzg2l: Use configurable PFC mask

Hi Geert,

Thank you for the review.

On Fri, Jan 10, 2025 at 11:30 AM Geert Uytterhoeven
<geert@...ux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> Thanks for your patch!
>
> On Tue, Jan 7, 2025 at 11:59 AM Prabhakar <prabhakar.csengg@...il.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> >
> > Make the PFC mask for the PFC_mx register configurable to address
> > differences between SoCs in the RZ/G2L family and RZ/V2H.
>
> "and the RZ/V2h family", as RZ/G3E is also affected.
>
> > On RZ/G2L family SoCs, the PFC_mx mask is `0x7` (3 bits), while on RZ/V2H
> > it is `0xf` (4 bits). The previous implementation hardcoded the PFC mask
> > as `0x7`, which caused incorrect behavior when configuring PFC registers
> > on RZ/V2H.
> >
> > To resolve this, introduce a `pfcmask` field in the `rzg2l_hwcfg` structure
> > to make the mask value configurable based on the SoC. Update PFC-related
> > logic to use `hwcfg->pfcmask` instead of a hardcoded value. Additionally,
> > update hardware configuration definitions to include the appropriate mask
> > values (`0x7` for RZ/G2L family and `0xf` for RZ/V2H).
>
> i understand this means that SD1_CD has always been broken:
>
Yea, but since the TF-A configured it we didnt see an issue.

>     arch/arm64/boot/dts/renesas/r9a09g057h44-rzv2h-evk.dts:
>         pinmux = <RZV2H_PORT_PINMUX(9, 4, 14)>; /* SD1_CD */
>
> and this should be queued as a fix, and backported to v6.12?
>
Agreed.

> > Fixes: 9bd95ac86e70 ("pinctrl: renesas: rzg2l: Add support for RZ/V2H SoC")
> > Reported-by: Hien Huynh <hien.huynh.px@...esas.com>
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
>
> > --- a/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > +++ b/drivers/pinctrl/renesas/pinctrl-rzg2l.c
> > @@ -159,7 +159,6 @@
> >  #define PWPR_REGWE_B           BIT(5)  /* OEN Register Write Enable, known only in RZ/V2H(P) */
> >
> >  #define PM_MASK                        0x03
> > -#define PFC_MASK               0x07
>
> What about just changing this to 0x0f instead? The bitfields are
> 4-bit wide anyway, and reserved bits on RZ/G2L are documented to be
> read as zero, and ignored when written.  Sole impact would be that
> specifying invalid functions 8-15 in an RZ/G2L DTS would no longer be
> masked by software.
>
agreed let's make PFC_MASK to 0x0f and keep the changes simple.

> >  #define IEN_MASK               0x01
> >  #define IOLH_MASK              0x03
> >  #define SR_MASK                        0x01
>
> > @@ -505,7 +507,7 @@ static void rzg2l_pinctrl_set_pfc_mode(struct rzg2l_pinctrl *pctrl,
> >
> >         /* Set pin to 'Non-use (Hi-Z input protection)'  */
> >         reg = readw(pctrl->base + PM(off));
> > -       reg &= ~(PM_MASK << (pin * 2));
> > +       reg &= ~(hwcfg->pfcmask << (pin * 2));
>
> Oops, this should not be replaced?
>
Arghh!

Cheers,
Prabhakar