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: <CAMuHMdWHYDV=R6pVCXc-Upwi=egW1UYO0XJePE7JC4WGbaW=1A@mail.gmail.com>
Date: Fri, 10 Jan 2025 12:30:40 +0100
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Prabhakar <prabhakar.csengg@...il.com>
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 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:

    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?

> 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.

>  #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?

>         writew(reg, pctrl->base + PM(off));
>
>         pctrl->data->pwpr_pfc_lock_unlock(pctrl, false);

> @@ -3110,7 +3113,7 @@ static void rzg2l_pinctrl_pm_setup_pfc(struct rzg2l_pinctrl *pctrl)
>                                 continue;
>
>                         /* Set pin to 'Non-use (Hi-Z input protection)' */
> -                       pm &= ~(PM_MASK << (pin * 2));
> +                       pm &= ~(hwcfg->pfcmask << (pin * 2));

Likewise.

>                         writew(pm, pctrl->base + PM(off));
>
>                         /* Temporarily switch to GPIO mode with PMC register */

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds