[PATCH 0/2] spi: sh-msiof: Add r8a774[35] bindings

[PATCH 0/2] spi: sh-msiof: Add r8a774[35] bindings

[Fabrizio Castro]

This series adds compatible strings and device tree bindings in order to enable
msiof support for r8a7743 and r8a7745.

Fabrizio Castro (2):
  spi: sh-msiof: Add compatible strings for r8a774[35]
  dt-bindings: spi: sh-msiof: Add r8a774[35] to the compatible list

 Documentation/devicetree/bindings/spi/sh-msiof.txt | 6 ++++--
 drivers/spi/spi-sh-msiof.c                         | 2 ++
 2 files changed, 6 insertions(+), 2 deletions(-)

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] spi: sh-msiof: Add compatible strings for r8a774[35]

[Fabrizio Castro]

Signed-off-by: Fabrizio Castro <fabrizio.castro-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org>
---
 drivers/spi/spi-sh-msiof.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 0eb1e95..0080844 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -1021,6 +1021,8 @@ static const struct sh_msiof_chipdata rcar_gen3_data = {
 
 static const struct of_device_id sh_msiof_match[] = {
 	{ .compatible = "renesas,sh-mobile-msiof", .data = &sh_data },
+	{ .compatible = "renesas,msiof-r8a7743",   .data = &rcar_gen2_data },
+	{ .compatible = "renesas,msiof-r8a7745",   .data = &rcar_gen2_data },
 	{ .compatible = "renesas,msiof-r8a7790",   .data = &rcar_gen2_data },
 	{ .compatible = "renesas,msiof-r8a7791",   .data = &rcar_gen2_data },
 	{ .compatible = "renesas,msiof-r8a7792",   .data = &rcar_gen2_data },
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] dt-bindings: spi: sh-msiof: Add r8a774[35] to the compatible list

[Fabrizio Castro]

Signed-off-by: Fabrizio Castro <fabrizio.castro-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org>
---
 Documentation/devicetree/bindings/spi/sh-msiof.txt | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/sh-msiof.txt b/Documentation/devicetree/bindings/spi/sh-msiof.txt
index e865855..bdd8395 100644
--- a/Documentation/devicetree/bindings/spi/sh-msiof.txt
+++ b/Documentation/devicetree/bindings/spi/sh-msiof.txt
@@ -1,7 +1,9 @@
 Renesas MSIOF spi controller
 
 Required properties:
-- compatible           : "renesas,msiof-r8a7790" (R-Car H2)
+- compatible           : "renesas,msiof-r8a7743" (RZ/G1M)
+			 "renesas,msiof-r8a7745" (RZ/G1E)
+			 "renesas,msiof-r8a7790" (R-Car H2)
 			 "renesas,msiof-r8a7791" (R-Car M2-W)
 			 "renesas,msiof-r8a7792" (R-Car V2H)
 			 "renesas,msiof-r8a7793" (R-Car M2-N)
@@ -10,7 +12,7 @@ Required properties:
 			 "renesas,msiof-r8a7796" (R-Car M3-W)
 			 "renesas,msiof-sh73a0" (SH-Mobile AG5)
 			 "renesas,sh-mobile-msiof" (generic SH-Mobile compatibile device)
-			 "renesas,rcar-gen2-msiof" (generic R-Car Gen2 compatible device)
+			 "renesas,rcar-gen2-msiof" (generic R-Car Gen2 and RZ/G1 compatible device)
 			 "renesas,rcar-gen3-msiof" (generic R-Car Gen3 compatible device)
 			 "renesas,sh-msiof"      (deprecated)
 
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] dt-bindings: spi: sh-msiof: Add r8a774[35] to the compatible list

[Geert Uytterhoeven]

On Mon, Sep 25, 2017 at 10:54 AM, Fabrizio Castro
<fabrizio.castro-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org> wrote:
> Signed-off-by: Fabrizio Castro <fabrizio.castro-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/spi/sh-msiof.txt | 6 ++++--

Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] spi: sh-msiof: Add compatible strings for r8a774[35]

[Geert Uytterhoeven]

Hi Fabrizio,

On Mon, Sep 25, 2017 at 10:54 AM, Fabrizio Castro
<fabrizio.castro-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org> wrote:
> Signed-off-by: Fabrizio Castro <fabrizio.castro-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org>

This patch is not needed...

> ---
>  drivers/spi/spi-sh-msiof.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
> index 0eb1e95..0080844 100644
> --- a/drivers/spi/spi-sh-msiof.c
> +++ b/drivers/spi/spi-sh-msiof.c
> @@ -1021,6 +1021,8 @@ static const struct sh_msiof_chipdata rcar_gen3_data = {
>
>  static const struct of_device_id sh_msiof_match[] = {
>         { .compatible = "renesas,sh-mobile-msiof", .data = &sh_data },
> +       { .compatible = "renesas,msiof-r8a7743",   .data = &rcar_gen2_data },
> +       { .compatible = "renesas,msiof-r8a7745",   .data = &rcar_gen2_data },
>         { .compatible = "renesas,msiof-r8a7790",   .data = &rcar_gen2_data },
>         { .compatible = "renesas,msiof-r8a7791",   .data = &rcar_gen2_data },
>         { .compatible = "renesas,msiof-r8a7792",   .data = &rcar_gen2_data },

... as the driver already matches against the family-specific compatible value
below:

        { .compatible = "renesas,rcar-gen2-msiof", .data = &rcar_gen2_data },

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

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
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/2] spi: sh-msiof: Add compatible strings for r8a774[35]

[Fabrizio Castro]

Hello Geert,

Thank you for getting back to me.

> -----Original Message-----
> From: geert.uytterhoeven@gmail.com [mailto:geert.uytterhoeven@gmail.com] On Behalf Of Geert Uytterhoeven
> Sent: 25 September 2017 12:22
> To: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> Cc: Mark Brown <broonie@kernel.org>; Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; linux-spi
> <linux-spi@vger.kernel.org>; devicetree@vger.kernel.org; Linux-Renesas <linux-renesas-soc@vger.kernel.org>; Chris Paterson
> <Chris.Paterson2@renesas.com>; Biju Das <biju.das@bp.renesas.com>
> Subject: Re: [PATCH 1/2] spi: sh-msiof: Add compatible strings for r8a774[35]
>
> Hi Fabrizio,
>
> On Mon, Sep 25, 2017 at 10:54 AM, Fabrizio Castro
> <fabrizio.castro@bp.renesas.com> wrote:
> > Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
>
> This patch is not needed...
>
> > ---
> >  drivers/spi/spi-sh-msiof.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
> > index 0eb1e95..0080844 100644
> > --- a/drivers/spi/spi-sh-msiof.c
> > +++ b/drivers/spi/spi-sh-msiof.c
> > @@ -1021,6 +1021,8 @@ static const struct sh_msiof_chipdata rcar_gen3_data = {
> >
> >  static const struct of_device_id sh_msiof_match[] = {
> >         { .compatible = "renesas,sh-mobile-msiof", .data = &sh_data },
> > +       { .compatible = "renesas,msiof-r8a7743",   .data = &rcar_gen2_data },
> > +       { .compatible = "renesas,msiof-r8a7745",   .data = &rcar_gen2_data },
> >         { .compatible = "renesas,msiof-r8a7790",   .data = &rcar_gen2_data },
> >         { .compatible = "renesas,msiof-r8a7791",   .data = &rcar_gen2_data },
> >         { .compatible = "renesas,msiof-r8a7792",   .data = &rcar_gen2_data },
>
> ... as the driver already matches against the family-specific compatible value
> below:
>
>         { .compatible = "renesas,rcar-gen2-msiof", .data = &rcar_gen2_data },

We suspected this was the case, but we weren't sure.
I apologize for wasting your time.

Cheers,
Fabrizio

>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> 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



Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.

Applied "spi: sh-msiof: Add r8a774[35] to the compatible list" to the spi tree

[Mark Brown]

The patch

   spi: sh-msiof: Add r8a774[35] to the compatible list

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 4702f4b23a2fc6196abacf515a959e69176da40e Mon Sep 17 00:00:00 2001
From: Fabrizio Castro <fabrizio.castro-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org>
Date: Mon, 25 Sep 2017 09:54:20 +0100
Subject: [PATCH] spi: sh-msiof: Add r8a774[35] to the compatible list

Signed-off-by: Fabrizio Castro <fabrizio.castro-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org>
Reviewed-by: Geert Uytterhoeven <geert+renesas-gXvu3+zWzMSzQB+pC5nmwQ@public.gmane.org>
Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 Documentation/devicetree/bindings/spi/sh-msiof.txt | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/spi/sh-msiof.txt b/Documentation/devicetree/bindings/spi/sh-msiof.txt
index e865855726a2..bdd83959019c 100644
--- a/Documentation/devicetree/bindings/spi/sh-msiof.txt
+++ b/Documentation/devicetree/bindings/spi/sh-msiof.txt
@@ -1,7 +1,9 @@
 Renesas MSIOF spi controller
 
 Required properties:
-- compatible           : "renesas,msiof-r8a7790" (R-Car H2)
+- compatible           : "renesas,msiof-r8a7743" (RZ/G1M)
+			 "renesas,msiof-r8a7745" (RZ/G1E)
+			 "renesas,msiof-r8a7790" (R-Car H2)
 			 "renesas,msiof-r8a7791" (R-Car M2-W)
 			 "renesas,msiof-r8a7792" (R-Car V2H)
 			 "renesas,msiof-r8a7793" (R-Car M2-N)
@@ -10,7 +12,7 @@ Required properties:
 			 "renesas,msiof-r8a7796" (R-Car M3-W)
 			 "renesas,msiof-sh73a0" (SH-Mobile AG5)
 			 "renesas,sh-mobile-msiof" (generic SH-Mobile compatibile device)
-			 "renesas,rcar-gen2-msiof" (generic R-Car Gen2 compatible device)
+			 "renesas,rcar-gen2-msiof" (generic R-Car Gen2 and RZ/G1 compatible device)
 			 "renesas,rcar-gen3-msiof" (generic R-Car Gen3 compatible device)
 			 "renesas,sh-msiof"      (deprecated)
 
-- 
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Applied "spi: sh-msiof: Add compatible strings for r8a774[35]" to the spi tree

[Mark Brown]

The patch

   spi: sh-msiof: Add compatible strings for r8a774[35]

has been applied to the spi tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From bdacfc7b6216dd30d07c10732fd4c0a660c62853 Mon Sep 17 00:00:00 2001
From: Fabrizio Castro <fabrizio.castro-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org>
Date: Mon, 25 Sep 2017 09:54:19 +0100
Subject: [PATCH] spi: sh-msiof: Add compatible strings for r8a774[35]

Signed-off-by: Fabrizio Castro <fabrizio.castro-kTT6dE0pTRh9uiUsa/gSgQ@public.gmane.org>
Signed-off-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/spi/spi-sh-msiof.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
index 0eb1e9583485..00808448cf35 100644
--- a/drivers/spi/spi-sh-msiof.c
+++ b/drivers/spi/spi-sh-msiof.c
@@ -1021,6 +1021,8 @@ static const struct sh_msiof_chipdata rcar_gen3_data = {
 
 static const struct of_device_id sh_msiof_match[] = {
 	{ .compatible = "renesas,sh-mobile-msiof", .data = &sh_data },
+	{ .compatible = "renesas,msiof-r8a7743",   .data = &rcar_gen2_data },
+	{ .compatible = "renesas,msiof-r8a7745",   .data = &rcar_gen2_data },
 	{ .compatible = "renesas,msiof-r8a7790",   .data = &rcar_gen2_data },
 	{ .compatible = "renesas,msiof-r8a7791",   .data = &rcar_gen2_data },
 	{ .compatible = "renesas,msiof-r8a7792",   .data = &rcar_gen2_data },
-- 
2.14.1

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Applied "spi: sh-msiof: Add compatible strings for r8a774[35]" to the spi tree

[Geert Uytterhoeven]

Hi Mark,

On Mon, Sep 25, 2017 at 6:16 PM, Mark Brown <broonie@kernel.org> wrote:
> The patch
>
>    spi: sh-msiof: Add compatible strings for r8a774[35]
>
> has been applied to the spi tree at
>
>    git://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git
>
> All being well this means that it will be integrated into the linux-next
> tree (usually sometime in the next 24 hours) and sent to Linus during
> the next merge window (or sooner if it is a bug fix), however if
> problems are discovered then the patch may be dropped or reverted.
>
> You may get further e-mails resulting from automated or manual testing
> and review of the tree, please engage with people reporting problems and
> send followup patches addressing any issues that are reported if needed.
>
> If any updates are required or you are submitting further changes they
> should be sent as incremental updates against current git, existing
> patches will not be replaced.
>
> Please add any relevant lists and maintainers to the CCs when replying
> to this mail.

Please drop this patch, as there's no need to add explicit matching for these
compatible values. The family-specific compatible values (which the driver
already matches against) are sufficient.

Thanks!

> From bdacfc7b6216dd30d07c10732fd4c0a660c62853 Mon Sep 17 00:00:00 2001
> From: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> Date: Mon, 25 Sep 2017 09:54:19 +0100
> Subject: [PATCH] spi: sh-msiof: Add compatible strings for r8a774[35]
>
> Signed-off-by: Fabrizio Castro <fabrizio.castro@bp.renesas.com>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  drivers/spi/spi-sh-msiof.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/spi/spi-sh-msiof.c b/drivers/spi/spi-sh-msiof.c
> index 0eb1e9583485..00808448cf35 100644
> --- a/drivers/spi/spi-sh-msiof.c
> +++ b/drivers/spi/spi-sh-msiof.c
> @@ -1021,6 +1021,8 @@ static const struct sh_msiof_chipdata rcar_gen3_data = {
>
>  static const struct of_device_id sh_msiof_match[] = {
>         { .compatible = "renesas,sh-mobile-msiof", .data = &sh_data },
> +       { .compatible = "renesas,msiof-r8a7743",   .data = &rcar_gen2_data },
> +       { .compatible = "renesas,msiof-r8a7745",   .data = &rcar_gen2_data },
>         { .compatible = "renesas,msiof-r8a7790",   .data = &rcar_gen2_data },
>         { .compatible = "renesas,msiof-r8a7791",   .data = &rcar_gen2_data },
>         { .compatible = "renesas,msiof-r8a7792",   .data = &rcar_gen2_data },

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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

Re: Applied "spi: sh-msiof: Add compatible strings for r8a774[35]" to the spi tree

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 503 bytes --]

On Mon, Sep 25, 2017 at 07:48:44PM +0200, Geert Uytterhoeven wrote:

> Please drop this patch, as there's no need to add explicit matching for these
> compatible values. The family-specific compatible values (which the driver
> already matches against) are sufficient.

While the patch is not needed if people list the fallback property it
also does no harm and provides a marginal documentation benefit in
saying that someone has considered if any special handling is useful and
decided that it isn't.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: Applied "spi: sh-msiof: Add compatible strings for r8a774[35]" to the spi tree

[Geert Uytterhoeven]

Hi Mark,

On Mon, Sep 25, 2017 at 8:45 PM, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Mon, Sep 25, 2017 at 07:48:44PM +0200, Geert Uytterhoeven wrote:
>> Please drop this patch, as there's no need to add explicit matching for these
>> compatible values. The family-specific compatible values (which the driver
>> already matches against) are sufficient.
>
> While the patch is not needed if people list the fallback property it
> also does no harm and provides a marginal documentation benefit in
> saying that someone has considered if any special handling is useful and
> decided that it isn't.

All true.

My rebuttal is threefold:
  1. Listing the fallback property is mandatory for new SoCs. We only keep
     the per-SoC compatible values in the driver for older SoCs that predate
     the introduction of fallback properties.
  2. Some harm is involved, in the form of increased kernel image size.
  3. When updating DT bindings for new SoCs, we usually add "No driver
     update is needed" to the patch description to clarify. Unfortunately
     that was missed here.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org

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
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: Applied "spi: sh-msiof: Add compatible strings for r8a774[35]" to the spi tree

[Fabrizio Castro]

Hi Mark,

I am very sorry about the confusion. Geert has a good point so please drop this patch.

Thanks,
Fabrizio

> -----Original Message-----
> From: geert.uytterhoeven@gmail.com [mailto:geert.uytterhoeven@gmail.com] On Behalf Of Geert Uytterhoeven
> Sent: 25 September 2017 20:16
> To: Mark Brown <broonie@kernel.org>
> Cc: Fabrizio Castro <fabrizio.castro@bp.renesas.com>; Rob Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>;
> linux-spi <linux-spi@vger.kernel.org>; devicetree@vger.kernel.org; Linux-Renesas <linux-renesas-soc@vger.kernel.org>; Chris
> Paterson <Chris.Paterson2@renesas.com>; Biju Das <biju.das@bp.renesas.com>
> Subject: Re: Applied "spi: sh-msiof: Add compatible strings for r8a774[35]" to the spi tree
>
> Hi Mark,
>
> On Mon, Sep 25, 2017 at 8:45 PM, Mark Brown <broonie@kernel.org> wrote:
> > On Mon, Sep 25, 2017 at 07:48:44PM +0200, Geert Uytterhoeven wrote:
> >> Please drop this patch, as there's no need to add explicit matching for these
> >> compatible values. The family-specific compatible values (which the driver
> >> already matches against) are sufficient.
> >
> > While the patch is not needed if people list the fallback property it
> > also does no harm and provides a marginal documentation benefit in
> > saying that someone has considered if any special handling is useful and
> > decided that it isn't.
>
> All true.
>
> My rebuttal is threefold:
>   1. Listing the fallback property is mandatory for new SoCs. We only keep
>      the per-SoC compatible values in the driver for older SoCs that predate
>      the introduction of fallback properties.
>   2. Some harm is involved, in the form of increased kernel image size.
>   3. When updating DT bindings for new SoCs, we usually add "No driver
>      update is needed" to the patch description to clarify. Unfortunately
>      that was missed here.
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> 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



Renesas Electronics Europe Ltd, Dukes Meadow, Millboard Road, Bourne End, Buckinghamshire, SL8 5FH, UK. Registered in England & Wales under Registered No. 04586709.

Re: Applied "spi: sh-msiof: Add compatible strings for r8a774[35]" to the spi tree

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 986 bytes --]

On Mon, Sep 25, 2017 at 09:15:53PM +0200, Geert Uytterhoeven wrote:

> My rebuttal is threefold:
>   1. Listing the fallback property is mandatory for new SoCs. We only keep
>      the per-SoC compatible values in the driver for older SoCs that predate
>      the introduction of fallback properties.

So good practice is always good practice, but I'd argue that it's not
bad practice to also explicitly enumerate all the documented compatibles
in the driver.  Being liberal in what you accept and all that.

>   2. Some harm is involved, in the form of increased kernel image size.

Is this really a meaningful impact?

>   3. When updating DT bindings for new SoCs, we usually add "No driver
>      update is needed" to the patch description to clarify. Unfortunately
>      that was missed here.

That's basically the same good practice thing, it's just documenting
what you're trying to do here with not putting things in code but
writing things in changelogs doesn't make them so!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: Applied "spi: sh-msiof: Add compatible strings for r8a774[35]" to the spi tree

[Magnus Damm]

Hi Mark, Geert, everyone,

On Wed, Sep 27, 2017 at 2:30 AM, Mark Brown <broonie@kernel.org> wrote:
> On Mon, Sep 25, 2017 at 09:15:53PM +0200, Geert Uytterhoeven wrote:
>
>> My rebuttal is threefold:
>>   1. Listing the fallback property is mandatory for new SoCs. We only keep
>>      the per-SoC compatible values in the driver for older SoCs that predate
>>      the introduction of fallback properties.
>
> So good practice is always good practice, but I'd argue that it's not
> bad practice to also explicitly enumerate all the documented compatibles
> in the driver.  Being liberal in what you accept and all that.
>
>>   2. Some harm is involved, in the form of increased kernel image size.
>
> Is this really a meaningful impact?
>
>>   3. When updating DT bindings for new SoCs, we usually add "No driver
>>      update is needed" to the patch description to clarify. Unfortunately
>>      that was missed here.
>
> That's basically the same good practice thing, it's just documenting
> what you're trying to do here with not putting things in code but
> writing things in changelogs doesn't make them so!

My opinion that good practice is to document the per-driver supported
hardware by explicitly listing the per-SoC compat strings in the DT
binding document.

Then exactly which compat strings the driver matches on is really part
of the software implementation and it will most likely vary over time.

So my view is that only updating the DT binding document should be
enough in most cases when fall-back compat strings are used. I guess
other people see it differently?

Is it too much detail to let the MAINTAINERS file point out both
driver files and the DT binding files?

Cheers,

/ magnus

Re: Applied "spi: sh-msiof: Add compatible strings for r8a774[35]" to the spi tree

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1423 bytes --]

On Wed, Sep 27, 2017 at 02:36:01PM +0900, Magnus Damm wrote:
> On Wed, Sep 27, 2017 at 2:30 AM, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > On Mon, Sep 25, 2017 at 09:15:53PM +0200, Geert Uytterhoeven wrote:

> >>   3. When updating DT bindings for new SoCs, we usually add "No driver
> >>      update is needed" to the patch description to clarify. Unfortunately
> >>      that was missed here.

> > That's basically the same good practice thing, it's just documenting
> > what you're trying to do here with not putting things in code but
> > writing things in changelogs doesn't make them so!

> My opinion that good practice is to document the per-driver supported
> hardware by explicitly listing the per-SoC compat strings in the DT
> binding document.

> Then exactly which compat strings the driver matches on is really part
> of the software implementation and it will most likely vary over time.

> So my view is that only updating the DT binding document should be
> enough in most cases when fall-back compat strings are used. I guess
> other people see it differently?

> Is it too much detail to let the MAINTAINERS file point out both
> driver files and the DT binding files?

It's supposed to do that already.  All I'm saying here is that the patch
doesn't seem like something that needs actively reverting since it's a
perfectly valid and even potentially useful change to make.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: Applied "spi: sh-msiof: Add compatible strings for r8a774[35]" to the spi tree

[Magnus Damm]

Hi Mark,

On Thu, Sep 28, 2017 at 2:01 AM, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Wed, Sep 27, 2017 at 02:36:01PM +0900, Magnus Damm wrote:
>> On Wed, Sep 27, 2017 at 2:30 AM, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> > On Mon, Sep 25, 2017 at 09:15:53PM +0200, Geert Uytterhoeven wrote:
>
>> >>   3. When updating DT bindings for new SoCs, we usually add "No driver
>> >>      update is needed" to the patch description to clarify. Unfortunately
>> >>      that was missed here.
>
>> > That's basically the same good practice thing, it's just documenting
>> > what you're trying to do here with not putting things in code but
>> > writing things in changelogs doesn't make them so!
>
>> My opinion that good practice is to document the per-driver supported
>> hardware by explicitly listing the per-SoC compat strings in the DT
>> binding document.
>
>> Then exactly which compat strings the driver matches on is really part
>> of the software implementation and it will most likely vary over time.
>
>> So my view is that only updating the DT binding document should be
>> enough in most cases when fall-back compat strings are used. I guess
>> other people see it differently?
>
>> Is it too much detail to let the MAINTAINERS file point out both
>> driver files and the DT binding files?
>
> It's supposed to do that already.  All I'm saying here is that the patch
> doesn't seem like something that needs actively reverting since it's a
> perfectly valid and even potentially useful change to make.

Thanks for clarifying the inclusion of DT bindings in the MAINTAINER
file. To escape any sort of conflict I will happily let you and Geert
discuss what is potentially useful or not! =)

Nevertheless I agree that reverting this rather harmless change seems
a bit overly aggressive.

Cheers,

/ magnus
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html