mbox series

[00/14] arm64: renesas: Add support for R Car H3e 2G-and M3e-2G

Message ID cover.1623315732.git.geert+renesas@glider.be
Headers show
Series arm64: renesas: Add support for R Car H3e 2G-and M3e-2G | expand

Message

Geert Uytterhoeven June 10, 2021, 9:37 a.m. UTC
Hi all,

The new R-Car Gen3e SoCs are different gradings of the existing R-Car
Gen3 SoCs.  This series adds support for the first two members of the
family: R-Car H3e-2G (R8A779M1) and R-Car M3e-2G (R8A779M3), on the
Salvator-XS and ULCB (with and without Kingfisher) development boards.

This series depends on "[PATCH 0/4] arm64: dts: renesas: salvator-x(s):
Factor out common optional parts".

It has been tested on Salvator-X with R-Car H3 ES3.0 and Salvator-XS
with R-Car M3-W+.  The last two patches have been tested by overclocking
R-Car H3 ES2.0 and M3-W ES1.0.

For testing, a branch with dependencies and fixups to prototype R-Car
Gen3e development on older Gen3 SoCs can be found at[2].

For this initial submission, I'm restricting the audience to the Renesas
SoC and Device Tree people.

Thanks for your comments!

[1] https://lore.kernel.org/linux-renesas-soc/cover.1623087028.git.geert+renesas@glider.be
[2] https://git.kernel.org/pub/scm/linux/kernel/git/geert/renesas-drivers.git/log/?h=topic/rcar-gen3e-v1

Geert Uytterhoeven (14):
  dt-bindings: arm: renesas: Document R-Car H3e-2G and M3e-2G SoCs and
    boards
  soc: renesas: Identify R-Car H3e-2G and M3e-2G
  pinctrl: renesas: Fix pin control matching on R-Car H3e-2G
  mmc: renesas_sdhi: Add support for R-Car H3e-2G and M3e-2G
  arm64: dts: renesas: Add Renesas R8A779M1 SoC support
  arm64: dts: renesas: Add Renesas R8A779M3 SoC support
  arm64: dts: renesas: Add support for Salvator-XS with R-Car H3e-2G
  arm64: dts: renesas: Add support for H3ULCB with R-Car H3e-2G
  arm64: dts: renesas: Add support for H3ULCB+Kingfisher with R-Car
    H3e-2G
  arm64: dts: renesas: Add support for Salvator-XS with R-Car M3e-2G
  arm64: dts: renesas: Add support for M3ULCB with R-Car M3e-2G
  arm64: dts: renesas: Add support for M3ULCB+Kingfisher with R-Car
    M3e-2G
  [RFC] arm64: dts: renesas: r8a779m1: Add Cortex-A57 2 GHz opp
  [RFC] arm64: dts: renesas: r8a779m3: Add Cortex-A57 2 GHz opp

 .../devicetree/bindings/arm/renesas.yaml      | 50 +++++++++++++----
 arch/arm64/boot/dts/renesas/Makefile          |  8 +++
 .../boot/dts/renesas/r8a779m1-salvator-xs.dts | 53 ++++++++++++++++++
 .../boot/dts/renesas/r8a779m1-ulcb-kf.dts     | 19 +++++++
 arch/arm64/boot/dts/renesas/r8a779m1-ulcb.dts | 54 +++++++++++++++++++
 arch/arm64/boot/dts/renesas/r8a779m1.dtsi     | 21 ++++++++
 .../boot/dts/renesas/r8a779m3-salvator-xs.dts | 46 ++++++++++++++++
 .../boot/dts/renesas/r8a779m3-ulcb-kf.dts     | 18 +++++++
 arch/arm64/boot/dts/renesas/r8a779m3-ulcb.dts | 45 ++++++++++++++++
 arch/arm64/boot/dts/renesas/r8a779m3.dtsi     | 21 ++++++++
 drivers/mmc/host/renesas_sdhi_core.c          |  2 +
 drivers/pinctrl/renesas/core.c                | 14 +++--
 drivers/soc/renesas/Kconfig                   |  2 +
 drivers/soc/renesas/renesas-soc.c             | 19 +++++++
 14 files changed, 356 insertions(+), 16 deletions(-)
 create mode 100644 arch/arm64/boot/dts/renesas/r8a779m1-salvator-xs.dts
 create mode 100644 arch/arm64/boot/dts/renesas/r8a779m1-ulcb-kf.dts
 create mode 100644 arch/arm64/boot/dts/renesas/r8a779m1-ulcb.dts
 create mode 100644 arch/arm64/boot/dts/renesas/r8a779m1.dtsi
 create mode 100644 arch/arm64/boot/dts/renesas/r8a779m3-salvator-xs.dts
 create mode 100644 arch/arm64/boot/dts/renesas/r8a779m3-ulcb-kf.dts
 create mode 100644 arch/arm64/boot/dts/renesas/r8a779m3-ulcb.dts
 create mode 100644 arch/arm64/boot/dts/renesas/r8a779m3.dtsi

Comments

Laurent Pinchart June 14, 2021, 6:31 p.m. UTC | #1
Hi Geert,

Thank you for the patch.

On Thu, Jun 10, 2021 at 11:37:15AM +0200, Geert Uytterhoeven wrote:
> Add support for identifying the R-Car H3e-2G (R8A779M1) and R-Car M3e-2G
> (R8A779M3) SoCs.
> 
> As these are different gradings of the already supported R-Car H3 ES3.0
> (R8A77951) and M3-W+ (R8A77961) SoCs, support for them is enabled
> through the existing ARCH_R8A77951 and ARCH_R8A77961 configuration
> symbols.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
> Question: Should we drop fam_rcar_gen3e and soc_rcar_[hm]3e, and just
> 	  use the existing soc_rcar_h3 and soc_rcar_m3_w?

I'd vote for that, as there's no functional difference in the code below
between fam_rcar_gen3e and fam_rcar_gen3.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/soc/renesas/Kconfig       |  2 ++
>  drivers/soc/renesas/renesas-soc.c | 19 +++++++++++++++++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/drivers/soc/renesas/Kconfig b/drivers/soc/renesas/Kconfig
> index 4fe0247189a615b0..089c7c50e3ca4c3b 100644
> --- a/drivers/soc/renesas/Kconfig
> +++ b/drivers/soc/renesas/Kconfig
> @@ -213,6 +213,7 @@ config ARCH_R8A77951
>  	help
>  	  This enables support for the Renesas R-Car H3 SoC (revisions 2.0 and
>  	  later).
> +	  This includes different gradings like R-Car H3e-2G.
>  
>  config ARCH_R8A77965
>  	bool "ARM64 Platform support for R-Car M3-N"
> @@ -234,6 +235,7 @@ config ARCH_R8A77961
>  	select SYSC_R8A77961
>  	help
>  	  This enables support for the Renesas R-Car M3-W+ SoC.
> +	  This includes different gradings like R-Car M3e-2G.
>  
>  config ARCH_R8A77980
>  	bool "ARM64 Platform support for R-Car V3H"
> diff --git a/drivers/soc/renesas/renesas-soc.c b/drivers/soc/renesas/renesas-soc.c
> index 0f8eff4a641a21b7..2851fd9c44ce8e3f 100644
> --- a/drivers/soc/renesas/renesas-soc.c
> +++ b/drivers/soc/renesas/renesas-soc.c
> @@ -33,6 +33,11 @@ static const struct renesas_family fam_rcar_gen3 __initconst __maybe_unused = {
>  	.reg	= 0xfff00044,		/* PRR (Product Register) */
>  };
>  
> +static const struct renesas_family fam_rcar_gen3e __initconst __maybe_unused = {
> +	.name	= "R-Car Gen3e",
> +	.reg	= 0xfff00044,		/* PRR (Product Register) */
> +};
> +
>  static const struct renesas_family fam_rmobile __initconst __maybe_unused = {
>  	.name	= "R-Mobile",
>  	.reg	= 0xe600101c,		/* CCCR (Common Chip Code Register) */
> @@ -205,6 +210,16 @@ static const struct renesas_soc soc_rcar_v3u __initconst __maybe_unused = {
>  	.id	= 0x59,
>  };
>  
> +static const struct renesas_soc soc_rcar_h3e __initconst __maybe_unused = {
> +	.family	= &fam_rcar_gen3e,
> +	.id	= 0x4f,
> +};
> +
> +static const struct renesas_soc soc_rcar_m3e __initconst __maybe_unused = {
> +	.family	= &fam_rcar_gen3e,
> +	.id	= 0x52,
> +};
> +
>  static const struct renesas_soc soc_shmobile_ag5 __initconst __maybe_unused = {
>  	.family	= &fam_shmobile,
>  	.id	= 0x37,
> @@ -275,11 +290,15 @@ static const struct of_device_id renesas_socs[] __initconst = {
>  #if defined(CONFIG_ARCH_R8A77950) || defined(CONFIG_ARCH_R8A77951)
>  	{ .compatible = "renesas,r8a7795",	.data = &soc_rcar_h3 },
>  #endif
> +#ifdef CONFIG_ARCH_R8A77951
> +	{ .compatible = "renesas,r8a779m1",	.data = &soc_rcar_h3e },
> +#endif
>  #ifdef CONFIG_ARCH_R8A77960
>  	{ .compatible = "renesas,r8a7796",	.data = &soc_rcar_m3_w },
>  #endif
>  #ifdef CONFIG_ARCH_R8A77961
>  	{ .compatible = "renesas,r8a77961",	.data = &soc_rcar_m3_w },
> +	{ .compatible = "renesas,r8a779m3",	.data = &soc_rcar_m3e },
>  #endif
>  #ifdef CONFIG_ARCH_R8A77965
>  	{ .compatible = "renesas,r8a77965",	.data = &soc_rcar_m3_n },
Laurent Pinchart June 14, 2021, 6:38 p.m. UTC | #2
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
Laurent Pinchart June 14, 2021, 6:42 p.m. UTC | #3
Hi Geert,

Thank you for the patch.

On Thu, Jun 10, 2021 at 11:37:17AM +0200, Geert Uytterhoeven wrote:
> Add support for the R-Car H3e-2G (R8A779M1) and M3e-2G (R8A779M3) SoCs.
> These are different gradings of the R-Car H3 ES3.0 (R8A77951) and M3-W+
> (R8A77961) SoCs, and thus subject to the same quirks.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  drivers/mmc/host/renesas_sdhi_core.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c
> index b719eda6b8619453..0478c9d58b965392 100644
> --- a/drivers/mmc/host/renesas_sdhi_core.c
> +++ b/drivers/mmc/host/renesas_sdhi_core.c
> @@ -943,6 +943,8 @@ static const struct soc_device_attribute sdhi_quirks_match[]  = {
>  	{ .soc_id = "r8a77965", .data = &sdhi_quirks_r8a77965 },
>  	{ .soc_id = "r8a77980", .data = &sdhi_quirks_nohs400 },
>  	{ .soc_id = "r8a77990", .data = &sdhi_quirks_r8a77990 },
> +	{ .soc_id = "r8a779m1", .data = &sdhi_quirks_bad_taps2367 },
> +	{ .soc_id = "r8a779m3", .data = &sdhi_quirks_bad_taps1357 },

Could we reuse the entries for H3 and M3 instead, by dropping the
"ES3.*" revision ?

>  	{ /* Sentinel. */ },
>  };
>
Laurent Pinchart June 14, 2021, 6:43 p.m. UTC | #4
Hi Geert,

Thank you for the patch.

On Thu, Jun 10, 2021 at 11:37:18AM +0200, Geert Uytterhoeven wrote:
> Add support for the Renesas R-Car H3e-2G (R8A779M1) SoC, which is a
> different grading of the R-Car H3 ES3.0 (R8A77951) SoC.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  arch/arm64/boot/dts/renesas/r8a779m1.dtsi | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/renesas/r8a779m1.dtsi
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a779m1.dtsi b/arch/arm64/boot/dts/renesas/r8a779m1.dtsi
> new file mode 100644
> index 0000000000000000..0e9b04469b83c661
> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/r8a779m1.dtsi
> @@ -0,0 +1,12 @@
> +// SPDX-License-Identifier: (GPL-2.0 or MIT)
> +/*
> + * Device Tree Source for the R-Car H3e-2G (R8A779M1) SoC
> + *
> + * Copyright (C) 2021 Glider bv
> + */
> +
> +#include "r8a77951.dtsi"
> +
> +/ {
> +	compatible = "renesas,r8a779m1", "renesas,r8a7795";

As commented in a separate patch this will be overridden by the .dts
that includes this file, so we could drop it altogether. I don't care
much though.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

The same applies to 06/14.

> +};
Laurent Pinchart June 14, 2021, 6:45 p.m. UTC | #5
Hi Geert,

Thank you for the patch.

On Thu, Jun 10, 2021 at 11:37:20AM +0200, Geert Uytterhoeven wrote:
> Add support for the Renesas Salvator-X 2nd version development
> board equipped with an R-Car H3e-2G SiP.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  arch/arm64/boot/dts/renesas/Makefile          |  2 +
>  .../boot/dts/renesas/r8a779m1-salvator-xs.dts | 53 +++++++++++++++++++
>  2 files changed, 55 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/renesas/r8a779m1-salvator-xs.dts
> 
> diff --git a/arch/arm64/boot/dts/renesas/Makefile b/arch/arm64/boot/dts/renesas/Makefile
> index f2de2fa0c8b890fb..5a689a1d10821f1d 100644
> --- a/arch/arm64/boot/dts/renesas/Makefile
> +++ b/arch/arm64/boot/dts/renesas/Makefile
> @@ -62,3 +62,5 @@ dtb-$(CONFIG_ARCH_R8A77990) += r8a77990-ebisu.dtb
>  dtb-$(CONFIG_ARCH_R8A77995) += r8a77995-draak.dtb
>  
>  dtb-$(CONFIG_ARCH_R8A779A0) += r8a779a0-falcon.dtb
> +
> +dtb-$(CONFIG_ARCH_R8A77951) += r8a779m1-salvator-xs.dtb

How about preserving the alphabetical order of the Kconfig symbols ?

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> diff --git a/arch/arm64/boot/dts/renesas/r8a779m1-salvator-xs.dts b/arch/arm64/boot/dts/renesas/r8a779m1-salvator-xs.dts
> new file mode 100644
> index 0000000000000000..084b75b046802339
> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/r8a779m1-salvator-xs.dts
> @@ -0,0 +1,53 @@
> +// SPDX-License-Identifier: (GPL-2.0 or MIT)
> +/*
> + * Device Tree Source for the Salvator-X 2nd version board with R-Car H3e-2G
> + *
> + * Copyright (C) 2021 Glider bv
> + *
> + * Based on r8a77951-salvator-xs.dts
> + * Copyright (C) 2015-2017 Renesas Electronics Corp.
> + */
> +
> +/dts-v1/;
> +#include "r8a779m1.dtsi"
> +#include "salvator-xs.dtsi"
> +
> +/ {
> +	model = "Renesas Salvator-X 2nd version board based on r8a779m1";
> +	compatible = "renesas,salvator-xs", "renesas,r8a779m1",
> +		     "renesas,r8a7795";
> +
> +	memory@48000000 {
> +		device_type = "memory";
> +		/* first 128MB is reserved for secure area. */
> +		reg = <0x0 0x48000000 0x0 0x38000000>;
> +	};
> +
> +	memory@500000000 {
> +		device_type = "memory";
> +		reg = <0x5 0x00000000 0x0 0x40000000>;
> +	};
> +
> +	memory@600000000 {
> +		device_type = "memory";
> +		reg = <0x6 0x00000000 0x0 0x40000000>;
> +	};
> +
> +	memory@700000000 {
> +		device_type = "memory";
> +		reg = <0x7 0x00000000 0x0 0x40000000>;
> +	};
> +};
> +
> +&du {
> +	clocks = <&cpg CPG_MOD 724>,
> +		 <&cpg CPG_MOD 723>,
> +		 <&cpg CPG_MOD 722>,
> +		 <&cpg CPG_MOD 721>,
> +		 <&versaclock6 1>,
> +		 <&x21_clk>,
> +		 <&x22_clk>,
> +		 <&versaclock6 2>;
> +	clock-names = "du.0", "du.1", "du.2", "du.3",
> +		      "dclkin.0", "dclkin.1", "dclkin.2", "dclkin.3";
> +};
Laurent Pinchart June 14, 2021, 6:46 p.m. UTC | #6
Hi Geert,

Thank you for the patch.

On Thu, Jun 10, 2021 at 11:37:21AM +0200, Geert Uytterhoeven wrote:
> Add support for the Renesas R-Car Starter Kit Premier equipped with an
> R-Car H3e-2G SiP.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  arch/arm64/boot/dts/renesas/Makefile          |  1 +
>  arch/arm64/boot/dts/renesas/r8a779m1-ulcb.dts | 54 +++++++++++++++++++
>  2 files changed, 55 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/renesas/r8a779m1-ulcb.dts
> 
> diff --git a/arch/arm64/boot/dts/renesas/Makefile b/arch/arm64/boot/dts/renesas/Makefile
> index 5a689a1d10821f1d..2a9119c8651815eb 100644
> --- a/arch/arm64/boot/dts/renesas/Makefile
> +++ b/arch/arm64/boot/dts/renesas/Makefile
> @@ -64,3 +64,4 @@ dtb-$(CONFIG_ARCH_R8A77995) += r8a77995-draak.dtb
>  dtb-$(CONFIG_ARCH_R8A779A0) += r8a779a0-falcon.dtb
>  
>  dtb-$(CONFIG_ARCH_R8A77951) += r8a779m1-salvator-xs.dtb
> +dtb-$(CONFIG_ARCH_R8A77951) += r8a779m1-ulcb.dtb
> diff --git a/arch/arm64/boot/dts/renesas/r8a779m1-ulcb.dts b/arch/arm64/boot/dts/renesas/r8a779m1-ulcb.dts
> new file mode 100644
> index 0000000000000000..e294b6bda28c68c8
> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/r8a779m1-ulcb.dts
> @@ -0,0 +1,54 @@
> +// SPDX-License-Identifier: (GPL-2.0 or MIT)
> +/*
> + * Device Tree Source for the H3ULCB (R-Car Starter Kit Premier) with R-Car H3e-2G
> + *
> + * Copyright (C) 2021 Glider bv
> + *
> + * Based on r8a77951-ulcb.dts
> + *
> + * Copyright (C) 2016 Renesas Electronics Corp.
> + * Copyright (C) 2016 Cogent Embedded, Inc.
> + */
> +
> +/dts-v1/;
> +#include "r8a779m1.dtsi"
> +#include "ulcb.dtsi"
> +
> +/ {
> +	model = "Renesas H3ULCB board based on r8a779m1";
> +	compatible = "renesas,h3ulcb", "renesas,r8a779m1", "renesas,r8a7795";
> +
> +	memory@48000000 {
> +		device_type = "memory";
> +		/* first 128MB is reserved for secure area. */
> +		reg = <0x0 0x48000000 0x0 0x38000000>;
> +	};
> +
> +	memory@500000000 {
> +		device_type = "memory";
> +		reg = <0x5 0x00000000 0x0 0x40000000>;
> +	};
> +
> +	memory@600000000 {
> +		device_type = "memory";
> +		reg = <0x6 0x00000000 0x0 0x40000000>;
> +	};
> +
> +	memory@700000000 {
> +		device_type = "memory";
> +		reg = <0x7 0x00000000 0x0 0x40000000>;
> +	};
> +};
> +
> +&du {
> +	clocks = <&cpg CPG_MOD 724>,
> +		 <&cpg CPG_MOD 723>,
> +		 <&cpg CPG_MOD 722>,
> +		 <&cpg CPG_MOD 721>,
> +		 <&versaclock5 1>,
> +		 <&versaclock5 3>,
> +		 <&versaclock5 4>,
> +		 <&versaclock5 2>;
> +	clock-names = "du.0", "du.1", "du.2", "du.3",
> +		      "dclkin.0", "dclkin.1", "dclkin.2", "dclkin.3";
> +};
> -- 
> 2.25.1
>
Laurent Pinchart June 14, 2021, 6:46 p.m. UTC | #7
Hi Geert,

Thank you for the patch.

On Thu, Jun 10, 2021 at 11:37:22AM +0200, Geert Uytterhoeven wrote:
> Add support for the Renesas R-Car Starter Kit Premier and Kingfisher
> combo equipped with an R-Car H3e-2G SiP.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  arch/arm64/boot/dts/renesas/Makefile          |  1 +
>  .../boot/dts/renesas/r8a779m1-ulcb-kf.dts     | 19 +++++++++++++++++++
>  2 files changed, 20 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/renesas/r8a779m1-ulcb-kf.dts
> 
> diff --git a/arch/arm64/boot/dts/renesas/Makefile b/arch/arm64/boot/dts/renesas/Makefile
> index 2a9119c8651815eb..4fa0d56602684faf 100644
> --- a/arch/arm64/boot/dts/renesas/Makefile
> +++ b/arch/arm64/boot/dts/renesas/Makefile
> @@ -65,3 +65,4 @@ dtb-$(CONFIG_ARCH_R8A779A0) += r8a779a0-falcon.dtb
>  
>  dtb-$(CONFIG_ARCH_R8A77951) += r8a779m1-salvator-xs.dtb
>  dtb-$(CONFIG_ARCH_R8A77951) += r8a779m1-ulcb.dtb
> +dtb-$(CONFIG_ARCH_R8A77951) += r8a779m1-ulcb-kf.dtb
> diff --git a/arch/arm64/boot/dts/renesas/r8a779m1-ulcb-kf.dts b/arch/arm64/boot/dts/renesas/r8a779m1-ulcb-kf.dts
> new file mode 100644
> index 0000000000000000..0baebc5c58b06a34
> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/r8a779m1-ulcb-kf.dts
> @@ -0,0 +1,19 @@
> +// SPDX-License-Identifier: (GPL-2.0 or MIT)
> +/*
> + * Device Tree Source for the H3ULCB Kingfisher board with R-Car H3e-2G
> + *
> + * Copyright (C) 2021 Glider bv
> + *
> + * Based on r8a77951-ulcb-kf.dts
> + * Copyright (C) 2017 Renesas Electronics Corp.
> + * Copyright (C) 2017 Cogent Embedded, Inc.
> + */
> +
> +#include "r8a779m1-ulcb.dts"
> +#include "ulcb-kf.dtsi"
> +
> +/ {
> +	model = "Renesas H3ULCB Kingfisher board based on r8a779m1";
> +	compatible = "shimafuji,kingfisher", "renesas,h3ulcb",
> +		     "renesas,r8a779m1", "renesas,r8a7795";
> +};
Laurent Pinchart June 14, 2021, 6:48 p.m. UTC | #8
Hi Geert,

Thank you for the patch.

On Thu, Jun 10, 2021 at 11:37:23AM +0200, Geert Uytterhoeven wrote:
> Add support for the Renesas Salvator-X 2nd version development
> board equipped with an R-Car M3e-2G SiP.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  arch/arm64/boot/dts/renesas/Makefile          |  2 +
>  .../boot/dts/renesas/r8a779m3-salvator-xs.dts | 46 +++++++++++++++++++
>  2 files changed, 48 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/renesas/r8a779m3-salvator-xs.dts
> 
> diff --git a/arch/arm64/boot/dts/renesas/Makefile b/arch/arm64/boot/dts/renesas/Makefile
> index 4fa0d56602684faf..6a1061824a9f29f0 100644
> --- a/arch/arm64/boot/dts/renesas/Makefile
> +++ b/arch/arm64/boot/dts/renesas/Makefile
> @@ -66,3 +66,5 @@ dtb-$(CONFIG_ARCH_R8A779A0) += r8a779a0-falcon.dtb
>  dtb-$(CONFIG_ARCH_R8A77951) += r8a779m1-salvator-xs.dtb
>  dtb-$(CONFIG_ARCH_R8A77951) += r8a779m1-ulcb.dtb
>  dtb-$(CONFIG_ARCH_R8A77951) += r8a779m1-ulcb-kf.dtb
> +
> +dtb-$(CONFIG_ARCH_R8A77961) += r8a779m3-salvator-xs.dtb

Same comment as or 07/14 regarding alphabetical ordering.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>


> diff --git a/arch/arm64/boot/dts/renesas/r8a779m3-salvator-xs.dts b/arch/arm64/boot/dts/renesas/r8a779m3-salvator-xs.dts
> new file mode 100644
> index 0000000000000000..4ab26fd7233d62fc
> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/r8a779m3-salvator-xs.dts
> @@ -0,0 +1,46 @@
> +// SPDX-License-Identifier: (GPL-2.0 or MIT)
> +/*
> + * Device Tree Source for the Salvator-X 2nd version board with R-Car M3e-2G
> + *
> + * Copyright (C) 2021 Glider bv
> + *
> + * Based on r8a77961-salvator-xs.dts
> + * Copyright (C) 2018 Renesas Electronics Corp.
> + */
> +
> +/dts-v1/;
> +#include "r8a779m3.dtsi"
> +#include "salvator-xs.dtsi"
> +
> +/ {
> +	model = "Renesas Salvator-X 2nd version board based on r8a779m3";
> +	compatible = "renesas,salvator-xs", "renesas,r8a779m3",
> +		     "renesas,r8a77961";
> +
> +	memory@48000000 {
> +		device_type = "memory";
> +		/* first 128MB is reserved for secure area. */
> +		reg = <0x0 0x48000000 0x0 0x78000000>;
> +	};
> +
> +	memory@480000000 {
> +		device_type = "memory";
> +		reg = <0x4 0x80000000 0x0 0x80000000>;
> +	};
> +
> +	memory@600000000 {
> +		device_type = "memory";
> +		reg = <0x6 0x00000000 0x1 0x00000000>;
> +	};
> +};
> +
> +&du {
> +	clocks = <&cpg CPG_MOD 724>,
> +		 <&cpg CPG_MOD 723>,
> +		 <&cpg CPG_MOD 722>,
> +		 <&versaclock6 1>,
> +		 <&x21_clk>,
> +		 <&versaclock6 2>;
> +	clock-names = "du.0", "du.1", "du.2",
> +		      "dclkin.0", "dclkin.1", "dclkin.2";
> +};
Laurent Pinchart June 14, 2021, 6:48 p.m. UTC | #9
Hi Geert,

Thank you for the patch.

On Thu, Jun 10, 2021 at 11:37:24AM +0200, Geert Uytterhoeven wrote:
> Add support for the Renesas R-Car Starter Kit Pro equipped with an R-Car
> M3e-2G SiP.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  arch/arm64/boot/dts/renesas/Makefile          |  1 +
>  arch/arm64/boot/dts/renesas/r8a779m3-ulcb.dts | 45 +++++++++++++++++++
>  2 files changed, 46 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/renesas/r8a779m3-ulcb.dts
> 
> diff --git a/arch/arm64/boot/dts/renesas/Makefile b/arch/arm64/boot/dts/renesas/Makefile
> index 6a1061824a9f29f0..f29dd27692ba4a66 100644
> --- a/arch/arm64/boot/dts/renesas/Makefile
> +++ b/arch/arm64/boot/dts/renesas/Makefile
> @@ -68,3 +68,4 @@ dtb-$(CONFIG_ARCH_R8A77951) += r8a779m1-ulcb.dtb
>  dtb-$(CONFIG_ARCH_R8A77951) += r8a779m1-ulcb-kf.dtb
>  
>  dtb-$(CONFIG_ARCH_R8A77961) += r8a779m3-salvator-xs.dtb
> +dtb-$(CONFIG_ARCH_R8A77961) += r8a779m3-ulcb.dtb
> diff --git a/arch/arm64/boot/dts/renesas/r8a779m3-ulcb.dts b/arch/arm64/boot/dts/renesas/r8a779m3-ulcb.dts
> new file mode 100644
> index 0000000000000000..8f215a0b771b7c30
> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/r8a779m3-ulcb.dts
> @@ -0,0 +1,45 @@
> +// SPDX-License-Identifier: (GPL-2.0 or MIT)
> +/*
> + * Device Tree Source for the M3ULCB (R-Car Starter Kit Pro) with R-Car M3e-2G
> + *
> + * Copyright (C) 2021 Glider bv
> + *
> + * Based on r8a77961-ulcb.dts
> + * Copyright (C) 2020 Renesas Electronics Corp.
> + */
> +
> +/dts-v1/;
> +#include "r8a779m3.dtsi"
> +#include "ulcb.dtsi"
> +
> +/ {
> +	model = "Renesas M3ULCB board based on r8a779m3";
> +	compatible = "renesas,m3ulcb", "renesas,r8a779m3", "renesas,r8a77961";
> +
> +	memory@48000000 {
> +		device_type = "memory";
> +		/* first 128MB is reserved for secure area. */
> +		reg = <0x0 0x48000000 0x0 0x78000000>;
> +	};
> +
> +	memory@480000000 {
> +		device_type = "memory";
> +		reg = <0x4 0x80000000 0x0 0x80000000>;
> +	};
> +
> +	memory@600000000 {
> +		device_type = "memory";
> +		reg = <0x6 0x00000000 0x1 0x00000000>;
> +	};
> +};
> +
> +&du {
> +	clocks = <&cpg CPG_MOD 724>,
> +		 <&cpg CPG_MOD 723>,
> +		 <&cpg CPG_MOD 722>,
> +		 <&versaclock5 1>,
> +		 <&versaclock5 3>,
> +		 <&versaclock5 2>;
> +	clock-names = "du.0", "du.1", "du.2",
> +		      "dclkin.0", "dclkin.1", "dclkin.2";
> +};
Laurent Pinchart June 14, 2021, 6:48 p.m. UTC | #10
Hi Geert,

Thank you for the patch.

On Thu, Jun 10, 2021 at 11:37:25AM +0200, Geert Uytterhoeven wrote:
> Add support for the Renesas R-Car Starter Kit Pro and Kingfisher combo
> equipped with an R-Car M3e-2G SiP.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  arch/arm64/boot/dts/renesas/Makefile           |  1 +
>  .../boot/dts/renesas/r8a779m3-ulcb-kf.dts      | 18 ++++++++++++++++++
>  2 files changed, 19 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/renesas/r8a779m3-ulcb-kf.dts
> 
> diff --git a/arch/arm64/boot/dts/renesas/Makefile b/arch/arm64/boot/dts/renesas/Makefile
> index f29dd27692ba4a66..5c73eb7baeadc8c4 100644
> --- a/arch/arm64/boot/dts/renesas/Makefile
> +++ b/arch/arm64/boot/dts/renesas/Makefile
> @@ -69,3 +69,4 @@ dtb-$(CONFIG_ARCH_R8A77951) += r8a779m1-ulcb-kf.dtb
>  
>  dtb-$(CONFIG_ARCH_R8A77961) += r8a779m3-salvator-xs.dtb
>  dtb-$(CONFIG_ARCH_R8A77961) += r8a779m3-ulcb.dtb
> +dtb-$(CONFIG_ARCH_R8A77961) += r8a779m3-ulcb-kf.dtb
> diff --git a/arch/arm64/boot/dts/renesas/r8a779m3-ulcb-kf.dts b/arch/arm64/boot/dts/renesas/r8a779m3-ulcb-kf.dts
> new file mode 100644
> index 0000000000000000..6bacee1d2ef5c857
> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/r8a779m3-ulcb-kf.dts
> @@ -0,0 +1,18 @@
> +// SPDX-License-Identifier: (GPL-2.0 or MIT)
> +/*
> + * Device Tree Source for the M3ULCB Kingfisher board with R-Car M3e-2G
> + *
> + * Copyright (C) 2021 Glider bv
> + *
> + * Based on r8a77961-ulcb-kf.dts
> + * Copyright (C) 2020 Eugeniu Rosca <rosca.eugeniu@gmail.com>
> + */
> +
> +#include "r8a779m3-ulcb.dts"
> +#include "ulcb-kf.dtsi"
> +
> +/ {
> +	model = "Renesas M3ULCB Kingfisher board based on r8a779m3";
> +	compatible = "shimafuji,kingfisher", "renesas,m3ulcb",
> +		     "renesas,r8a779m3", "renesas,r8a77961";
> +};
Geert Uytterhoeven June 14, 2021, 7:16 p.m. UTC | #11
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
Geert Uytterhoeven June 14, 2021, 7:32 p.m. UTC | #12
Hi Laurent,

On Mon, Jun 14, 2021 at 8:42 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Thu, Jun 10, 2021 at 11:37:17AM +0200, Geert Uytterhoeven wrote:
> > Add support for the R-Car H3e-2G (R8A779M1) and M3e-2G (R8A779M3) SoCs.
> > These are different gradings of the R-Car H3 ES3.0 (R8A77951) and M3-W+
> > (R8A77961) SoCs, and thus subject to the same quirks.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

> > --- a/drivers/mmc/host/renesas_sdhi_core.c
> > +++ b/drivers/mmc/host/renesas_sdhi_core.c
> > @@ -943,6 +943,8 @@ static const struct soc_device_attribute sdhi_quirks_match[]  = {
> >       { .soc_id = "r8a77965", .data = &sdhi_quirks_r8a77965 },
> >       { .soc_id = "r8a77980", .data = &sdhi_quirks_nohs400 },
> >       { .soc_id = "r8a77990", .data = &sdhi_quirks_r8a77990 },
> > +     { .soc_id = "r8a779m1", .data = &sdhi_quirks_bad_taps2367 },
> > +     { .soc_id = "r8a779m3", .data = &sdhi_quirks_bad_taps1357 },
>
> Could we reuse the entries for H3 and M3 instead, by dropping the
> "ES3.*" revision ?

We cannot reuse the H3 ES3.0 entry, as soc_device_match()
works differently than of_machine_is_compatible(): the former doesn't
consider "r8a779m1" and "r8a7795" equivalent, the latter does.
Same for M3-W+ (no explicit ES3.0 there) and M3e-2G.

It's a pity we still don't have a "quirk-free" SDHI version on H3
and M3-W class SoCs (waiting for ES4.0?), as that would allow us to
just match on "renesas,sdhi-r8a7795" resp. "renesas,sdhi-r8a77961"
through the driver's .of_match_table[] instead, which would work for
H3e-2G and M3e-2G, too.

Gr{oetje,eeting}s,

                        Geert
Geert Uytterhoeven June 14, 2021, 7:34 p.m. UTC | #13
Hi Laurent,

On Mon, Jun 14, 2021 at 8:46 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> On Thu, Jun 10, 2021 at 11:37:20AM +0200, Geert Uytterhoeven wrote:
> > Add support for the Renesas Salvator-X 2nd version development
> > board equipped with an R-Car H3e-2G SiP.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

> > --- a/arch/arm64/boot/dts/renesas/Makefile
> > +++ b/arch/arm64/boot/dts/renesas/Makefile
> > @@ -62,3 +62,5 @@ dtb-$(CONFIG_ARCH_R8A77990) += r8a77990-ebisu.dtb
> >  dtb-$(CONFIG_ARCH_R8A77995) += r8a77995-draak.dtb
> >
> >  dtb-$(CONFIG_ARCH_R8A779A0) += r8a779a0-falcon.dtb
> > +
> > +dtb-$(CONFIG_ARCH_R8A77951) += r8a779m1-salvator-xs.dtb
>
> How about preserving the alphabetical order of the Kconfig symbols ?

At the expense of breaking alphabetical order of the DTB file names?

I agree both make sense. Do we need a vote? ;-)

> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks!

Gr{oetje,eeting}s,

                        Geert
Yoshihiro Shimoda June 16, 2021, 11:25 a.m. UTC | #14
Hi Geert-san,

Thank you for the patch!

> From: Geert Uytterhoeven, Sent: Thursday, June 10, 2021 6:37 PM
> 
> Add a preliminary operating point for running the Cortex-A57 CPU cores
> on R-Car H3e-2G at 2 GHz.
> 
> The opp-microvolt value depends on a future update of the Electrical
> Characteristics for R-Car H3e-2G.

Yes, we have to confirm the official document in the future, but...

> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  arch/arm64/boot/dts/renesas/r8a779m1.dtsi | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a779m1.dtsi b/arch/arm64/boot/dts/renesas/r8a779m1.dtsi
> index 0e9b04469b83c661..01b4e787e5749219 100644
> --- a/arch/arm64/boot/dts/renesas/r8a779m1.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a779m1.dtsi
> @@ -10,3 +10,12 @@
>  / {
>  	compatible = "renesas,r8a779m1", "renesas,r8a7795";
>  };
> +
> +&cluster0_opp {
> +	opp-2000000000 {
> +		opp-hz = /bits/ 64 <2000000000>;
> +		opp-microvolt = <1020000>;	// FIXME TBC

I heard this voltage is the same as 1.7GHz's one (0.96V).

Best regards,
Yoshihiro Shimoda

> +		clock-latency-ns = <300000>;
> +		turbo-mode;
> +	};
> +};
> --
> 2.25.1
Yoshihiro Shimoda June 16, 2021, 11:27 a.m. UTC | #15
Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Thursday, June 10, 2021 6:37 PM
> 
> Add a preliminary operating point for running the Cortex-A57 CPU cores
> on R-Car M3e-2G at 2 GHz.
> 
> The opp-microvolt value depends on a future update of the Electrical
> Characteristics for R-Car M3e-2G.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> ---
>  arch/arm64/boot/dts/renesas/r8a779m3.dtsi | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a779m3.dtsi b/arch/arm64/boot/dts/renesas/r8a779m3.dtsi
> index 65bb6188ccf5470a..fa5e8ffdf7343739 100644
> --- a/arch/arm64/boot/dts/renesas/r8a779m3.dtsi
> +++ b/arch/arm64/boot/dts/renesas/r8a779m3.dtsi
> @@ -10,3 +10,12 @@
>  / {
>  	compatible = "renesas,r8a779m3", "renesas,r8a77961";
>  };
> +
> +&cluster0_opp {
> +	opp-2000000000 {
> +		opp-hz = /bits/ 64 <2000000000>;
> +		opp-microvolt = <1020000>;	// FIXME TBC

Like r8a779m1, I heard this is also <960000>.

Best regards,
Yoshihiro Shimoda

> +		clock-latency-ns = <300000>;
> +		turbo-mode;
> +	};
> +};
> --
> 2.25.1
Yoshihiro Shimoda June 23, 2021, 10:13 a.m. UTC | #16
Hi Geert-san,

Thank you for the patch!

> From: Laurent Pinchart, Sent: Tuesday, June 15, 2021 3:31 AM
> 
> Hi Geert,
> 
> Thank you for the patch.
> 
> On Thu, Jun 10, 2021 at 11:37:15AM +0200, Geert Uytterhoeven wrote:
> > Add support for identifying the R-Car H3e-2G (R8A779M1) and R-Car M3e-2G
> > (R8A779M3) SoCs.
> >
> > As these are different gradings of the already supported R-Car H3 ES3.0
> > (R8A77951) and M3-W+ (R8A77961) SoCs, support for them is enabled
> > through the existing ARCH_R8A77951 and ARCH_R8A77961 configuration
> > symbols.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> > Question: Should we drop fam_rcar_gen3e and soc_rcar_[hm]3e, and just
> > 	  use the existing soc_rcar_h3 and soc_rcar_m3_w?
> 
> I'd vote for that, as there's no functional difference in the code below
> between fam_rcar_gen3e and fam_rcar_gen3.

+1

If we add fam_rcar_gen3e, we need update some drivers [1] which
are using .family.

[1] drivers/iommu/ipmmu-vmsa.c and drivers/usb/host/ehci-platform.c.

Best regards,
Yoshihiro Shimoda
Yoshihiro Shimoda June 23, 2021, 10:34 a.m. UTC | #17
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
Yoshihiro Shimoda June 24, 2021, 2:25 a.m. UTC | #18
Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Tuesday, June 15, 2021 4:32 AM
> 
> Hi Laurent,
> 
> On Mon, Jun 14, 2021 at 8:42 PM Laurent Pinchart wrote:
> > On Thu, Jun 10, 2021 at 11:37:17AM +0200, Geert Uytterhoeven wrote:
> > > Add support for the R-Car H3e-2G (R8A779M1) and M3e-2G (R8A779M3) SoCs.
> > > These are different gradings of the R-Car H3 ES3.0 (R8A77951) and M3-W+
> > > (R8A77961) SoCs, and thus subject to the same quirks.
> > >
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> > > --- a/drivers/mmc/host/renesas_sdhi_core.c
> > > +++ b/drivers/mmc/host/renesas_sdhi_core.c
> > > @@ -943,6 +943,8 @@ static const struct soc_device_attribute sdhi_quirks_match[]  = {
> > >       { .soc_id = "r8a77965", .data = &sdhi_quirks_r8a77965 },
> > >       { .soc_id = "r8a77980", .data = &sdhi_quirks_nohs400 },
> > >       { .soc_id = "r8a77990", .data = &sdhi_quirks_r8a77990 },
> > > +     { .soc_id = "r8a779m1", .data = &sdhi_quirks_bad_taps2367 },
> > > +     { .soc_id = "r8a779m3", .data = &sdhi_quirks_bad_taps1357 },
> >
> > Could we reuse the entries for H3 and M3 instead, by dropping the
> > "ES3.*" revision ?
> 
> We cannot reuse the H3 ES3.0 entry, as soc_device_match()
> works differently than of_machine_is_compatible(): the former doesn't
> consider "r8a779m1" and "r8a7795" equivalent, the latter does.
> Same for M3-W+ (no explicit ES3.0 there) and M3e-2G.
> 
> It's a pity we still don't have a "quirk-free" SDHI version on H3
> and M3-W class SoCs (waiting for ES4.0?), as that would allow us to
> just match on "renesas,sdhi-r8a7795" resp. "renesas,sdhi-r8a77961"
> through the driver's .of_match_table[] instead, which would work for
> H3e-2G and M3e-2G, too.

Perhaps, ES4.0 will not be released. So, we can refactor the driver's
.of_match_table[] now. I investigated this a little, and it seems
we need many renesas_sdhi_of_data for each SoC instead of
of_rcar_gen3_compatible. But, I guess such modification is better
than adding sdhi_quirks_match entries.

Wolfram-san, what do you thinks?

Best regards,
Yoshihiro Shimoda
Yoshihiro Shimoda June 24, 2021, 4:56 a.m. UTC | #19
Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Thursday, June 10, 2021 6:37 PM
> 
> Add support for the Renesas R-Car H3e-2G (R8A779M1) SoC, which is a
> different grading of the R-Car H3 ES3.0 (R8A77951) SoC.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thank you for the patch!

Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Best regards,
Yoshihiro Shimoda
Yoshihiro Shimoda June 24, 2021, 5:08 a.m. UTC | #20
Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Thursday, June 10, 2021 6:37 PM
> 
> Add support for the Renesas R-Car M3e-2G (R8A779M3) SoC, which is a
> different grading of the R-Car M3-W+ (R8A77961) SoC.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Thank you for the patch!

Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Best regards,
Yoshihiro Shimoda

> ---
>  arch/arm64/boot/dts/renesas/r8a779m3.dtsi | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>  create mode 100644 arch/arm64/boot/dts/renesas/r8a779m3.dtsi
> 
> diff --git a/arch/arm64/boot/dts/renesas/r8a779m3.dtsi b/arch/arm64/boot/dts/renesas/r8a779m3.dtsi
> new file mode 100644
> index 0000000000000000..65bb6188ccf5470a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/renesas/r8a779m3.dtsi
> @@ -0,0 +1,12 @@
> +// SPDX-License-Identifier: (GPL-2.0 or MIT)
> +/*
> + * Device Tree Source for the R-Car M3e-2G (R8A779M3) SoC
> + *
> + * Copyright (C) 2021 Glider bv
> + */
> +
> +#include "r8a77961.dtsi"
> +
> +/ {
> +	compatible = "renesas,r8a779m3", "renesas,r8a77961";
> +};
> --
> 2.25.1
Yoshihiro Shimoda June 24, 2021, 5:27 a.m. UTC | #21
Hi Geert-san,

> From: Geert Uytterhoeven, Sent: Tuesday, June 15, 2021 4:34 AM
> 
> Hi Laurent,
> 
> On Mon, Jun 14, 2021 at 8:46 PM Laurent Pinchart wrote:
> > On Thu, Jun 10, 2021 at 11:37:20AM +0200, Geert Uytterhoeven wrote:
> > > Add support for the Renesas Salvator-X 2nd version development
> > > board equipped with an R-Car H3e-2G SiP.
> > >
> > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> > > --- a/arch/arm64/boot/dts/renesas/Makefile
> > > +++ b/arch/arm64/boot/dts/renesas/Makefile
> > > @@ -62,3 +62,5 @@ dtb-$(CONFIG_ARCH_R8A77990) += r8a77990-ebisu.dtb
> > >  dtb-$(CONFIG_ARCH_R8A77995) += r8a77995-draak.dtb
> > >
> > >  dtb-$(CONFIG_ARCH_R8A779A0) += r8a779a0-falcon.dtb
> > > +
> > > +dtb-$(CONFIG_ARCH_R8A77951) += r8a779m1-salvator-xs.dtb
> >
> > How about preserving the alphabetical order of the Kconfig symbols ?
> 
> At the expense of breaking alphabetical order of the DTB file names?
> 
> I agree both make sense. Do we need a vote? ;-)

I prefer the alphabetical order of the DTB file names :)

Reviewed-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>

Best regards,
Yoshihiro Shimoda
Wolfram Sang June 24, 2021, 6:14 a.m. UTC | #22
Hi all,

> > > > --- a/drivers/mmc/host/renesas_sdhi_core.c
> > > > +++ b/drivers/mmc/host/renesas_sdhi_core.c
> > > > @@ -943,6 +943,8 @@ static const struct soc_device_attribute sdhi_quirks_match[]  = {
> > > >       { .soc_id = "r8a77965", .data = &sdhi_quirks_r8a77965 },
> > > >       { .soc_id = "r8a77980", .data = &sdhi_quirks_nohs400 },
> > > >       { .soc_id = "r8a77990", .data = &sdhi_quirks_r8a77990 },
> > > > +     { .soc_id = "r8a779m1", .data = &sdhi_quirks_bad_taps2367 },
> > > > +     { .soc_id = "r8a779m3", .data = &sdhi_quirks_bad_taps1357 },
> > >
> > > Could we reuse the entries for H3 and M3 instead, by dropping the
> > > "ES3.*" revision ?
> > 
> > We cannot reuse the H3 ES3.0 entry, as soc_device_match()
> > works differently than of_machine_is_compatible(): the former doesn't
> > consider "r8a779m1" and "r8a7795" equivalent, the latter does.
> > Same for M3-W+ (no explicit ES3.0 there) and M3e-2G.
> > 
> > It's a pity we still don't have a "quirk-free" SDHI version on H3
> > and M3-W class SoCs (waiting for ES4.0?), as that would allow us to
> > just match on "renesas,sdhi-r8a7795" resp. "renesas,sdhi-r8a77961"
> > through the driver's .of_match_table[] instead, which would work for
> > H3e-2G and M3e-2G, too.
> 
> Perhaps, ES4.0 will not be released. So, we can refactor the driver's
> .of_match_table[] now. I investigated this a little, and it seems
> we need many renesas_sdhi_of_data for each SoC instead of
> of_rcar_gen3_compatible. But, I guess such modification is better
> than adding sdhi_quirks_match entries.
> 
> Wolfram-san, what do you thinks?

I don't fully understand how the refactoring should look like? Is it
moving 'struct renesas_sdhi_quirks' to renesas_sdhi_internal_dmac.c and
merge it there with renesas_sdhi_of_data? Is it really better to copy
this struct per SoC? Most of the data is the same.

Thanks,

   Wolfram
Yoshihiro Shimoda June 24, 2021, 10:54 a.m. UTC | #23
Hi Wolfram-san,

> From: Wolfram Sang, Sent: Thursday, June 24, 2021 3:15 PM
> 
> Hi all,
> 
> > > > > --- a/drivers/mmc/host/renesas_sdhi_core.c
> > > > > +++ b/drivers/mmc/host/renesas_sdhi_core.c
> > > > > @@ -943,6 +943,8 @@ static const struct soc_device_attribute sdhi_quirks_match[]  = {
> > > > >       { .soc_id = "r8a77965", .data = &sdhi_quirks_r8a77965 },
> > > > >       { .soc_id = "r8a77980", .data = &sdhi_quirks_nohs400 },
> > > > >       { .soc_id = "r8a77990", .data = &sdhi_quirks_r8a77990 },
> > > > > +     { .soc_id = "r8a779m1", .data = &sdhi_quirks_bad_taps2367 },
> > > > > +     { .soc_id = "r8a779m3", .data = &sdhi_quirks_bad_taps1357 },
> > > >
> > > > Could we reuse the entries for H3 and M3 instead, by dropping the
> > > > "ES3.*" revision ?
> > >
> > > We cannot reuse the H3 ES3.0 entry, as soc_device_match()
> > > works differently than of_machine_is_compatible(): the former doesn't
> > > consider "r8a779m1" and "r8a7795" equivalent, the latter does.
> > > Same for M3-W+ (no explicit ES3.0 there) and M3e-2G.
> > >
> > > It's a pity we still don't have a "quirk-free" SDHI version on H3
> > > and M3-W class SoCs (waiting for ES4.0?), as that would allow us to
> > > just match on "renesas,sdhi-r8a7795" resp. "renesas,sdhi-r8a77961"
> > > through the driver's .of_match_table[] instead, which would work for
> > > H3e-2G and M3e-2G, too.
> >
> > Perhaps, ES4.0 will not be released. So, we can refactor the driver's
> > .of_match_table[] now. I investigated this a little, and it seems
> > we need many renesas_sdhi_of_data for each SoC instead of
> > of_rcar_gen3_compatible. But, I guess such modification is better
> > than adding sdhi_quirks_match entries.
> >
> > Wolfram-san, what do you thinks?
> 
> I don't fully understand how the refactoring should look like? Is it
> moving 'struct renesas_sdhi_quirks' to renesas_sdhi_internal_dmac.c and
> merge it there with renesas_sdhi_of_data? Is it really better to copy
> this struct per SoC? Most of the data is the same.

I also have the same concern. But, I guess we can refactor
the renesas_sdhi_of_data like below to avoid increasing data size:

struct renesas_sdhi_of_data_with_quirks {
	const struct renesas_sdhi_of_data *of_data;
	const struct renesas_sdhi_quirks *quirks;
};

And then, we can keep of_rcar_gen3_compatible and
we can add each SoC's renesas_sdhi_of_data_with_quirks
and set it to the .data.

Best regards,
Yoshihiro Shimoda

> Thanks,
> 
>    Wolfram
Wolfram Sang June 24, 2021, 3:19 p.m. UTC | #24
> > I don't fully understand how the refactoring should look like? Is it
> > moving 'struct renesas_sdhi_quirks' to renesas_sdhi_internal_dmac.c and
> > merge it there with renesas_sdhi_of_data? Is it really better to copy
> > this struct per SoC? Most of the data is the same.
> 
> I also have the same concern. But, I guess we can refactor
> the renesas_sdhi_of_data like below to avoid increasing data size:
> 
> struct renesas_sdhi_of_data_with_quirks {
> 	const struct renesas_sdhi_of_data *of_data;
> 	const struct renesas_sdhi_quirks *quirks;
> };
> 
> And then, we can keep of_rcar_gen3_compatible and
> we can add each SoC's renesas_sdhi_of_data_with_quirks
> and set it to the .data.

That sounds like a reasonable approach to me. This would also allow us
to merge the quirks from sdhi_core with the quirks from
sdhi_internal_dmac.
Yoshihiro Shimoda June 25, 2021, 4:54 a.m. UTC | #25
Hi Wolfram-san,

> From: Wolfram Sang, Sent: Friday, June 25, 2021 12:20 AM
> 
> > > I don't fully understand how the refactoring should look like? Is it
> > > moving 'struct renesas_sdhi_quirks' to renesas_sdhi_internal_dmac.c and
> > > merge it there with renesas_sdhi_of_data? Is it really better to copy
> > > this struct per SoC? Most of the data is the same.
> >
> > I also have the same concern. But, I guess we can refactor
> > the renesas_sdhi_of_data like below to avoid increasing data size:
> >
> > struct renesas_sdhi_of_data_with_quirks {
> > 	const struct renesas_sdhi_of_data *of_data;
> > 	const struct renesas_sdhi_quirks *quirks;
> > };
> >
> > And then, we can keep of_rcar_gen3_compatible and
> > we can add each SoC's renesas_sdhi_of_data_with_quirks
> > and set it to the .data.
> 
> That sounds like a reasonable approach to me. This would also allow us
> to merge the quirks from sdhi_core with the quirks from
> sdhi_internal_dmac.

Thank you for your reply! So, I'll try to make such a patch.

Best regards,
Yoshihiro Shimoda