Hi Prabhakar,
Thanks for your patch!
On Tue, Jan 7, 2025 at 11:59 AM Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxx>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> --- 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@xxxxxxxxxxxxxx
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
