Hi Shimoda-san,
On Wed, Dec 25, 2019 at 10:46 AM Yoshihiro Shimoda
<yoshihiro.shimoda.uh@xxxxxxxxxxx> wrote:
> > From: Geert Uytterhoeven, Sent: Wednesday, December 18, 2019 3:43 AM
> >
> > Despite using the same compatible values ("r8a7795"-based) because of
> > historical reasons, R-Car H3 ES1.x (R8A77950) and R-Car H3 ES2.0+
> > (R8A77951) are really different SoCs, with different part numbers, and
> > with different Pin Function Controller blocks.
> >
> > Reflect this in the pinctrl configuration, by replacing the existing
> > CONFIG_PINCTRL_PFC_R8A7795 symbol by two new config symbols:
> > CONFIG_PINCTRL_PFC_R8A77950 and CONFIG_PINCTRL_PFC_R8A77951. The latter
> > are selected automatically, depending on the soon-to-be-introduced
> > corresponding SoC-specific config options, and on the current common
> > config option, to relax dependencies.
> >
> > Rename the individual pin control driver source files from
> > pfc-r8a7795-es1.c to pfc-r8a77950.c, and from pfc-r8a7795.c to
> > pfc-r8a77951.c, and make them truly independent.
> > As both SoCs share the same compatible value, special care must be taken
> > to match them to the correct pin control driver, if support for it is
> > included in the running kernel.
> >
> > This will allow making support for early R-Car H3 revisions optional,
> > the largest share of which is taken by the pin control driver.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> > ---
> > Suggestions for simplifying sh_pfc_quirk_match(), or for alternative
> > solutions are welcome!
>
> I wondered if using weak attribute on both info variables could
> simplify sh_pfc_quirk_match(), but such a code [1] doesn't seem better
> than using #ifdef. Also, using weak attributes waste data size
> if R8A77950=n and R8A77951=y for instance.
Thanks for the great suggestion!
The trick is to add __weak to the existing extern declarations in sh_pfc.h,
instead of adding weak empty structs.
When the structs don't exist, their addresses just become zero.
So I came up with the following (whitespace-damaged) patch, which I intend
to fold into the original, if no one objects.
diff --git a/drivers/pinctrl/sh-pfc/core.c b/drivers/pinctrl/sh-pfc/core.c
index 8da95bf22735fd7b..92f8d5f5b6930849 100644
--- a/drivers/pinctrl/sh-pfc/core.c
+++ b/drivers/pinctrl/sh-pfc/core.c
@@ -1120,19 +1120,11 @@ static const void *sh_pfc_quirk_match(void)
static const struct soc_device_attribute quirks[] = {
{
.soc_id = "r8a7795", .revision = "ES1.*",
-#ifdef CONFIG_PINCTRL_PFC_R8A77950
.data = &r8a77950_pinmux_info,
-#else
- .data = (void *)-ENODEV,
-#endif
},
{
.soc_id = "r8a7795",
-#ifdef CONFIG_PINCTRL_PFC_R8A77951
.data = &r8a77951_pinmux_info,
-#else
- .data = (void *)-ENODEV,
-#endif
},
{ /* sentinel */ }
@@ -1140,7 +1132,7 @@ static const void *sh_pfc_quirk_match(void)
match = soc_device_match(quirks);
if (match)
- return match->data;
+ return match->data ?: ERR_PTR(-ENODEV);
#endif /* CONFIG_PINCTRL_PFC_R8A77950 || CONFIG_PINCTRL_PFC_R8A77951 */
return NULL;
diff --git a/drivers/pinctrl/sh-pfc/sh_pfc.h b/drivers/pinctrl/sh-pfc/sh_pfc.h
index aa298332f00f1a8e..d57e633e99c0ce66 100644
--- a/drivers/pinctrl/sh-pfc/sh_pfc.h
+++ b/drivers/pinctrl/sh-pfc/sh_pfc.h
@@ -318,8 +318,8 @@ extern const struct sh_pfc_soc_info r8a7791_pinmux_info;
extern const struct sh_pfc_soc_info r8a7792_pinmux_info;
extern const struct sh_pfc_soc_info r8a7793_pinmux_info;
extern const struct sh_pfc_soc_info r8a7794_pinmux_info;
-extern const struct sh_pfc_soc_info r8a77950_pinmux_info;
-extern const struct sh_pfc_soc_info r8a77951_pinmux_info;
+extern const struct sh_pfc_soc_info r8a77950_pinmux_info __weak;
+extern const struct sh_pfc_soc_info r8a77951_pinmux_info __weak;
extern const struct sh_pfc_soc_info r8a77960_pinmux_info;
extern const struct sh_pfc_soc_info r8a77961_pinmux_info;
extern const struct sh_pfc_soc_info r8a77965_pinmux_info;
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
