Hi Laurent, On Mon, Jun 14, 2021 at 8:38 PM Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> wrote: > On Thu, Jun 10, 2021 at 11:37:16AM +0200, Geert Uytterhoeven wrote: > > As R-Car H3 ES1.x (R8A77950) and R-Car ES2.0+ (R8A77951) use the same > > compatible value, the pin control driver relies on soc_device_match() > > with soc_id = "r8a7795" and the (non)matching of revision = "ES1.*" to > > match with and distinguish between the two SoC variants. The > > corresponding entries in the normal of_match_table are present only to > > make the optional sanity checks work. > > > > The R-Car H3e-2G (R8A779M1) SoC is a different grading of the R-Car H3 > > ES3.0 (R8A77951) SoC. It uses the same compatible values for individual > > devices, but has an additional compatible value for the root node. > > When running on an R-Car H3e-2G SoC, soc_device_match() with soc_id = > > "r8a7795" does not return a match. Hence the pin control driver falls > > back to the normal of_match_table, and, as the R8A77950 entry is listed > > first, incorrectly uses the sub-driver for R-Car H3 ES1.x. > > > > Fix this by moving the entry for R8A77951 before the entry for R8A77950. > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> Thanks! > I wonder if this means we could drop the second entry in the quirks > array, in sh_pfc_quirk_match(). As both R-Car H3 ES1.x and ES2.0+ use the same compatible value, that function is designed (with the help of the __weak r8a7795[01]_pinmux_info symbols) to fail, when booting a kernel that lacks the right pin control driver. It's less likely to happen nowadays, since we gained separate Kconfig symbols. Note that if you enable CONFIG_ARCH_R8A77950 but not CONFIG_ARCH_R8A77951, you can still trick a kernel running on R-Car H3e-2G into using the wrong pin control driver, which will usually cause something to fail during boot. Perhaps the time is ripe to drop the safety net; need to thing about that with a fresh mind, after a morning coffee... > > --- a/drivers/pinctrl/renesas/core.c > > +++ b/drivers/pinctrl/renesas/core.c > > @@ -571,17 +571,21 @@ static const struct of_device_id sh_pfc_of_table[] = { > > .data = &r8a7794_pinmux_info, > > }, > > #endif > > -/* Both r8a7795 entries must be present to make sanity checks work */ > > -#ifdef CONFIG_PINCTRL_PFC_R8A77950 > > +/* > > + * Both r8a7795 entries must be present to make sanity checks work, but only > > + * the first entry is actually used, for R-Car H3e. > > + * R-Car H3 ES1.x and ES2.0+ are matched using soc_device_match() instead. > > + */ > > +#ifdef CONFIG_PINCTRL_PFC_R8A77951 > > { > > .compatible = "renesas,pfc-r8a7795", > > - .data = &r8a77950_pinmux_info, > > + .data = &r8a77951_pinmux_info, > > }, > > #endif > > -#ifdef CONFIG_PINCTRL_PFC_R8A77951 > > +#ifdef CONFIG_PINCTRL_PFC_R8A77950 > > { > > .compatible = "renesas,pfc-r8a7795", > > - .data = &r8a77951_pinmux_info, > > + .data = &r8a77950_pinmux_info, > > }, > > #endif > > #ifdef CONFIG_PINCTRL_PFC_R8A77960 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