| Message ID | ab1acd836e990c536ff3a8c715ce57363d3ff8cb.1623315732.git.geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | arm64: renesas: Add support for R Car H3e 2G-and M3e-2G | expand |
Hi Geert, Thank you for the patch. 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@glider.be> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> I wonder if this means we could drop the second entry in the quirks array, in sh_pfc_quirk_match(). > --- > drivers/pinctrl/renesas/core.c | 14 +++++++++----- > 1 file changed, 9 insertions(+), 5 deletions(-) > > diff --git a/drivers/pinctrl/renesas/core.c b/drivers/pinctrl/renesas/core.c > index 5ccc49b387f17eb9..9adb97065704270d 100644 > --- 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
Hi Laurent, On Mon, Jun 14, 2021 at 8:38 PM Laurent Pinchart <laurent.pinchart@ideasonboard.com> 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@glider.be> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> 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
Hi Geert-san, > From: Geert Uytterhoeven, Sent: Thursday, June 10, 2021 6:37 PM > > 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. Thank you for the patch! After that, IIUC, we can remove an entry of r8a77951 from quirks[] in the sh_pfc_quirk_match(). > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com> Best regards, Yoshihiro Shimoda
diff --git a/drivers/pinctrl/renesas/core.c b/drivers/pinctrl/renesas/core.c index 5ccc49b387f17eb9..9adb97065704270d 100644 --- 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
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@glider.be> --- drivers/pinctrl/renesas/core.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)