[PATCH 0/5] i2c: rcar: add renesas,i2c-rcar-gen1/gen2 in DT compatible

[PATCH 0/5] i2c: rcar: add renesas,i2c-rcar-gen1/gen2 in DT compatible

[Kuninori Morimoto]

Hi Wolfram, Simon

These patches add R-Car generatoin level
DT compatible on i2c-rcar driver and platform.

Kuninori Morimoto (5):
  i2c: rcar: add renesas,i2c-rcar-gen1/gen2 in DT compatible
  ARM: shmobile: r8a7778: add generation level compatible for i2c
  ARM: shmobile: r8a7779: add generation level compatible for i2c
  ARM: shmobile: r8a7790: add generation level compatible for i2c
  ARM: shmobile: r8a7791: add generation level compatible for i2c

 Documentation/devicetree/bindings/i2c/i2c-rcar.txt |    4 +++-
 arch/arm/boot/dts/r8a7778.dtsi                     |    8 ++++----
 arch/arm/boot/dts/r8a7779.dtsi                     |    8 ++++----
 arch/arm/boot/dts/r8a7790.dtsi                     |    8 ++++----
 arch/arm/boot/dts/r8a7791.dtsi                     |   12 ++++++------
 drivers/i2c/busses/i2c-rcar.c                      |    2 ++
 6 files changed, 23 insertions(+), 19 deletions(-)

-- 
1.7.9.5

[PATCH 3/5] ARM: shmobile: r8a7779: add generation level compatible for i2c

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 arch/arm/boot/dts/r8a7779.dtsi |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7779.dtsi b/arch/arm/boot/dts/r8a7779.dtsi
index 463e3fd..ac89e42 100644
--- a/arch/arm/boot/dts/r8a7779.dtsi
+++ b/arch/arm/boot/dts/r8a7779.dtsi
@@ -159,7 +159,7 @@
 	i2c0: i2c@ffc70000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
-		compatible = "renesas,i2c-r8a7779";
+		compatible = "renesas,i2c-r8a7779", "renesas,i2c-rcar-gen1";
 		reg = <0xffc70000 0x1000>;
 		interrupts = <0 79 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp0_clks R8A7779_CLK_I2C0>;
@@ -169,7 +169,7 @@
 	i2c1: i2c@ffc71000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
-		compatible = "renesas,i2c-r8a7779";
+		compatible = "renesas,i2c-r8a7779", "renesas,i2c-rcar-gen1";
 		reg = <0xffc71000 0x1000>;
 		interrupts = <0 82 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp0_clks R8A7779_CLK_I2C1>;
@@ -179,7 +179,7 @@
 	i2c2: i2c@ffc72000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
-		compatible = "renesas,i2c-r8a7779";
+		compatible = "renesas,i2c-r8a7779", "renesas,i2c-rcar-gen1";
 		reg = <0xffc72000 0x1000>;
 		interrupts = <0 80 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp0_clks R8A7779_CLK_I2C2>;
@@ -189,7 +189,7 @@
 	i2c3: i2c@ffc73000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
-		compatible = "renesas,i2c-r8a7779";
+		compatible = "renesas,i2c-r8a7779", "renesas,i2c-rcar-gen1";
 		reg = <0xffc73000 0x1000>;
 		interrupts = <0 81 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp0_clks R8A7779_CLK_I2C3>;
-- 
1.7.9.5

[PATCH 4/5] ARM: shmobile: r8a7790: add generation level compatible for i2c

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 arch/arm/boot/dts/r8a7790.dtsi |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7790.dtsi b/arch/arm/boot/dts/r8a7790.dtsi
index 7218bff..02d07b7 100644
--- a/arch/arm/boot/dts/r8a7790.dtsi
+++ b/arch/arm/boot/dts/r8a7790.dtsi
@@ -252,7 +252,7 @@
 	i2c0: i2c@e6508000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
-		compatible = "renesas,i2c-r8a7790";
+		compatible = "renesas,i2c-r8a7790", "renesas,i2c-rcar-gen2";
 		reg = <0 0xe6508000 0 0x40>;
 		interrupts = <0 287 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp9_clks R8A7790_CLK_I2C0>;
@@ -262,7 +262,7 @@
 	i2c1: i2c@e6518000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
-		compatible = "renesas,i2c-r8a7790";
+		compatible = "renesas,i2c-r8a7790", "renesas,i2c-rcar-gen2";
 		reg = <0 0xe6518000 0 0x40>;
 		interrupts = <0 288 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp9_clks R8A7790_CLK_I2C1>;
@@ -272,7 +272,7 @@
 	i2c2: i2c@e6530000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
-		compatible = "renesas,i2c-r8a7790";
+		compatible = "renesas,i2c-r8a7790", "renesas,i2c-rcar-gen2";
 		reg = <0 0xe6530000 0 0x40>;
 		interrupts = <0 286 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp9_clks R8A7790_CLK_I2C2>;
@@ -282,7 +282,7 @@
 	i2c3: i2c@e6540000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
-		compatible = "renesas,i2c-r8a7790";
+		compatible = "renesas,i2c-r8a7790", "renesas,i2c-rcar-gen2";
 		reg = <0 0xe6540000 0 0x40>;
 		interrupts = <0 290 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp9_clks R8A7790_CLK_I2C3>;
-- 
1.7.9.5

[PATCH 5/5] ARM: shmobile: r8a7791: add generation level compatible for i2c

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 arch/arm/boot/dts/r8a7791.dtsi |   12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi
index 1e97113..854eca2 100644
--- a/arch/arm/boot/dts/r8a7791.dtsi
+++ b/arch/arm/boot/dts/r8a7791.dtsi
@@ -245,7 +245,7 @@
 	i2c0: i2c@e6508000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
-		compatible = "renesas,i2c-r8a7791";
+		compatible = "renesas,i2c-r8a7791", "renesas,i2c-rcar-gen2";
 		reg = <0 0xe6508000 0 0x40>;
 		interrupts = <0 287 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp9_clks R8A7791_CLK_I2C0>;
@@ -255,7 +255,7 @@
 	i2c1: i2c@e6518000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
-		compatible = "renesas,i2c-r8a7791";
+		compatible = "renesas,i2c-r8a7791", "renesas,i2c-rcar-gen2";
 		reg = <0 0xe6518000 0 0x40>;
 		interrupts = <0 288 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp9_clks R8A7791_CLK_I2C1>;
@@ -265,7 +265,7 @@
 	i2c2: i2c@e6530000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
-		compatible = "renesas,i2c-r8a7791";
+		compatible = "renesas,i2c-r8a7791", "renesas,i2c-rcar-gen2";
 		reg = <0 0xe6530000 0 0x40>;
 		interrupts = <0 286 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp9_clks R8A7791_CLK_I2C2>;
@@ -275,7 +275,7 @@
 	i2c3: i2c@e6540000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
-		compatible = "renesas,i2c-r8a7791";
+		compatible = "renesas,i2c-r8a7791", "renesas,i2c-rcar-gen2";
 		reg = <0 0xe6540000 0 0x40>;
 		interrupts = <0 290 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp9_clks R8A7791_CLK_I2C3>;
@@ -285,7 +285,7 @@
 	i2c4: i2c@e6520000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
-		compatible = "renesas,i2c-r8a7791";
+		compatible = "renesas,i2c-r8a7791", "renesas,i2c-rcar-gen2";
 		reg = <0 0xe6520000 0 0x40>;
 		interrupts = <0 19 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp9_clks R8A7791_CLK_I2C4>;
@@ -296,7 +296,7 @@
 		/* doesn't need pinmux */
 		#address-cells = <1>;
 		#size-cells = <0>;
-		compatible = "renesas,i2c-r8a7791";
+		compatible = "renesas,i2c-r8a7791", "renesas,i2c-rcar-gen2";
 		reg = <0 0xe6528000 0 0x40>;
 		interrupts = <0 20 IRQ_TYPE_LEVEL_HIGH>;
 		clocks = <&mstp9_clks R8A7791_CLK_I2C5>;
-- 
1.7.9.5

[PATCH 1/5] i2c: rcar: add renesas,i2c-rcar-gen1/gen2 in DT compatible

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

This patch adds DT compatible for Renesas R-Car Gen1/Gen2.
Current driver has SoC level .compatible
(r8a7778/r8a7779/r8a7790/r8a7791/r8a7792/r8a7793/r8a7794),
but these can be match as generation level.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 Documentation/devicetree/bindings/i2c/i2c-rcar.txt |    4 +++-
 drivers/i2c/busses/i2c-rcar.c                      |    2 ++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-rcar.txt b/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
index 16b3e07..0f9e812 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
@@ -3,6 +3,8 @@ I2C for R-Car platforms
 Required properties:
 - compatible: Must be one of
 	"renesas,i2c-rcar"
+	"renesas,i2c-rcar-gen1"
+	"renesas,i2c-rcar-gen2"
 	"renesas,i2c-r8a7778"
 	"renesas,i2c-r8a7779"
 	"renesas,i2c-r8a7790"
@@ -24,7 +26,7 @@ Examples :
 i2c0: i2c@e6508000 {
 	#address-cells = <1>;
 	#size-cells = <0>;
-	compatible = "renesas,i2c-r8a7791";
+	compatible = "renesas,i2c-r8a7791", "renesas,i2c-rcar-gen2";
 	reg = <0 0xe6508000 0 0x40>;
 	interrupts = <0 287 IRQ_TYPE_LEVEL_HIGH>;
 	clocks = <&mstp9_clks R8A7791_CLK_I2C0>;
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
index f3c7139..6f805f8 100644
--- a/drivers/i2c/busses/i2c-rcar.c
+++ b/drivers/i2c/busses/i2c-rcar.c
@@ -487,6 +487,8 @@ static const struct i2c_algorithm rcar_i2c_algo = {
 
 static const struct of_device_id rcar_i2c_dt_ids[] = {
 	{ .compatible = "renesas,i2c-rcar", .data = (void *)I2C_RCAR_GEN1 },
+	{ .compatible = "renesas,i2c-rcar-gen1", .data = (void *)I2C_RCAR_GEN1 },
+	{ .compatible = "renesas,i2c-rcar-gen2", .data = (void *)I2C_RCAR_GEN2 },
 	{ .compatible = "renesas,i2c-r8a7778", .data = (void *)I2C_RCAR_GEN1 },
 	{ .compatible = "renesas,i2c-r8a7779", .data = (void *)I2C_RCAR_GEN1 },
 	{ .compatible = "renesas,i2c-r8a7790", .data = (void *)I2C_RCAR_GEN2 },
-- 
1.7.9.5

Re: [PATCH 1/5] i2c: rcar: add renesas,i2c-rcar-gen1/gen2 in DT compatible

[Simon Horman]

On Tue, Aug 05, 2014 at 09:40:12PM -0700, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> This patch adds DT compatible for Renesas R-Car Gen1/Gen2.
> Current driver has SoC level .compatible
> (r8a7778/r8a7779/r8a7790/r8a7791/r8a7792/r8a7793/r8a7794),
> but these can be match as generation level.

Hi Morimoto-san,

is this compatibility explicitly documented somewhere?

> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  Documentation/devicetree/bindings/i2c/i2c-rcar.txt |    4 +++-
>  drivers/i2c/busses/i2c-rcar.c                      |    2 ++
>  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-rcar.txt b/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
> index 16b3e07..0f9e812 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
> @@ -3,6 +3,8 @@ I2C for R-Car platforms
>  Required properties:
>  - compatible: Must be one of
>  	"renesas,i2c-rcar"
> +	"renesas,i2c-rcar-gen1"
> +	"renesas,i2c-rcar-gen2"
>  	"renesas,i2c-r8a7778"
>  	"renesas,i2c-r8a7779"
>  	"renesas,i2c-r8a7790"
> @@ -24,7 +26,7 @@ Examples :
>  i2c0: i2c@e6508000 {
>  	#address-cells = <1>;
>  	#size-cells = <0>;
> -	compatible = "renesas,i2c-r8a7791";
> +	compatible = "renesas,i2c-r8a7791", "renesas,i2c-rcar-gen2";
>  	reg = <0 0xe6508000 0 0x40>;
>  	interrupts = <0 287 IRQ_TYPE_LEVEL_HIGH>;
>  	clocks = <&mstp9_clks R8A7791_CLK_I2C0>;
> diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
> index f3c7139..6f805f8 100644
> --- a/drivers/i2c/busses/i2c-rcar.c
> +++ b/drivers/i2c/busses/i2c-rcar.c
> @@ -487,6 +487,8 @@ static const struct i2c_algorithm rcar_i2c_algo = {
>  
>  static const struct of_device_id rcar_i2c_dt_ids[] = {
>  	{ .compatible = "renesas,i2c-rcar", .data = (void *)I2C_RCAR_GEN1 },
> +	{ .compatible = "renesas,i2c-rcar-gen1", .data = (void *)I2C_RCAR_GEN1 },
> +	{ .compatible = "renesas,i2c-rcar-gen2", .data = (void *)I2C_RCAR_GEN2 },
>  	{ .compatible = "renesas,i2c-r8a7778", .data = (void *)I2C_RCAR_GEN1 },
>  	{ .compatible = "renesas,i2c-r8a7779", .data = (void *)I2C_RCAR_GEN1 },
>  	{ .compatible = "renesas,i2c-r8a7790", .data = (void *)I2C_RCAR_GEN2 },
> -- 
> 1.7.9.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 1/5] i2c: rcar: add renesas,i2c-rcar-gen1/gen2 in DT compatible

[Kuninori Morimoto]

Hi Simon

> > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > 
> > This patch adds DT compatible for Renesas R-Car Gen1/Gen2.
> > Current driver has SoC level .compatible
> > (r8a7778/r8a7779/r8a7790/r8a7791/r8a7792/r8a7793/r8a7794),
> > but these can be match as generation level.
> 
> Hi Morimoto-san,
> 
> is this compatibility explicitly documented somewhere?

?
Do you mean ${LINUX}/Documentation/devicetree/bindings/xxx ?
[1/5] patch have it, but is it not enough ?

> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > ---
> >  Documentation/devicetree/bindings/i2c/i2c-rcar.txt |    4 +++-
> >  drivers/i2c/busses/i2c-rcar.c                      |    2 ++
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c-rcar.txt b/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
> > index 16b3e07..0f9e812 100644
> > --- a/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
> > +++ b/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
> > @@ -3,6 +3,8 @@ I2C for R-Car platforms
> >  Required properties:
> >  - compatible: Must be one of
> >  	"renesas,i2c-rcar"
> > +	"renesas,i2c-rcar-gen1"
> > +	"renesas,i2c-rcar-gen2"
> >  	"renesas,i2c-r8a7778"
> >  	"renesas,i2c-r8a7779"
> >  	"renesas,i2c-r8a7790"
> > @@ -24,7 +26,7 @@ Examples :
> >  i2c0: i2c@e6508000 {
> >  	#address-cells = <1>;
> >  	#size-cells = <0>;
> > -	compatible = "renesas,i2c-r8a7791";
> > +	compatible = "renesas,i2c-r8a7791", "renesas,i2c-rcar-gen2";
> >  	reg = <0 0xe6508000 0 0x40>;
> >  	interrupts = <0 287 IRQ_TYPE_LEVEL_HIGH>;
> >  	clocks = <&mstp9_clks R8A7791_CLK_I2C0>;
> > diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
> > index f3c7139..6f805f8 100644
> > --- a/drivers/i2c/busses/i2c-rcar.c
> > +++ b/drivers/i2c/busses/i2c-rcar.c
> > @@ -487,6 +487,8 @@ static const struct i2c_algorithm rcar_i2c_algo = {
> >  
> >  static const struct of_device_id rcar_i2c_dt_ids[] = {
> >  	{ .compatible = "renesas,i2c-rcar", .data = (void *)I2C_RCAR_GEN1 },
> > +	{ .compatible = "renesas,i2c-rcar-gen1", .data = (void *)I2C_RCAR_GEN1 },
> > +	{ .compatible = "renesas,i2c-rcar-gen2", .data = (void *)I2C_RCAR_GEN2 },
> >  	{ .compatible = "renesas,i2c-r8a7778", .data = (void *)I2C_RCAR_GEN1 },
> >  	{ .compatible = "renesas,i2c-r8a7779", .data = (void *)I2C_RCAR_GEN1 },
> >  	{ .compatible = "renesas,i2c-r8a7790", .data = (void *)I2C_RCAR_GEN2 },
> > -- 
> > 1.7.9.5
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/5] i2c: rcar: add renesas,i2c-rcar-gen1/gen2 in DT compatible

[Simon Horman]

On Wed, Aug 06, 2014 at 05:36:37PM -0700, Kuninori Morimoto wrote:
> 
> Hi Simon
> 
> > > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > > 
> > > This patch adds DT compatible for Renesas R-Car Gen1/Gen2.
> > > Current driver has SoC level .compatible
> > > (r8a7778/r8a7779/r8a7790/r8a7791/r8a7792/r8a7793/r8a7794),
> > > but these can be match as generation level.
> > 
> > Hi Morimoto-san,
> > 
> > is this compatibility explicitly documented somewhere?
> 
> ?
> Do you mean ${LINUX}/Documentation/devicetree/bindings/xxx ?
> [1/5] patch have it, but is it not enough ?

Sorry for not being clearer.

At our face-to-face meeting in Montpellier we discussed the
idea of generation bindings. And my recollection is that Magnus
and I had strong reservations about declaring what generation compatibility
is without it being explicitly stated in hardware documentation.

From observation we can say that the i2c controllers on r8a7778 and r8a7779
appear to be compatible. But can we say that in fact they are in fact
compatible hardware. That they are different hardware instances of a common
i2c controller whose name may or may not be is known to us? If not
I do not think that using a common binding describes the hardware,
which is the intention of DT bindings.

A second problem is the using the generation as a name.  Assuming the
answer to the above question is yes can we further say with certainty
that all variants of Gen1 SoCs that currently exist or will exist in the
future are compatible?  If not then using Gen1 as the name does not seem
to accurately describe the hardware.

> > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > > ---
> > >  Documentation/devicetree/bindings/i2c/i2c-rcar.txt |    4 +++-
> > >  drivers/i2c/busses/i2c-rcar.c                      |    2 ++
> > >  2 files changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/i2c/i2c-rcar.txt b/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
> > > index 16b3e07..0f9e812 100644
> > > --- a/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
> > > +++ b/Documentation/devicetree/bindings/i2c/i2c-rcar.txt
> > > @@ -3,6 +3,8 @@ I2C for R-Car platforms
> > >  Required properties:
> > >  - compatible: Must be one of
> > >  	"renesas,i2c-rcar"
> > > +	"renesas,i2c-rcar-gen1"
> > > +	"renesas,i2c-rcar-gen2"
> > >  	"renesas,i2c-r8a7778"
> > >  	"renesas,i2c-r8a7779"
> > >  	"renesas,i2c-r8a7790"
> > > @@ -24,7 +26,7 @@ Examples :
> > >  i2c0: i2c@e6508000 {
> > >  	#address-cells = <1>;
> > >  	#size-cells = <0>;
> > > -	compatible = "renesas,i2c-r8a7791";
> > > +	compatible = "renesas,i2c-r8a7791", "renesas,i2c-rcar-gen2";
> > >  	reg = <0 0xe6508000 0 0x40>;
> > >  	interrupts = <0 287 IRQ_TYPE_LEVEL_HIGH>;
> > >  	clocks = <&mstp9_clks R8A7791_CLK_I2C0>;
> > > diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c
> > > index f3c7139..6f805f8 100644
> > > --- a/drivers/i2c/busses/i2c-rcar.c
> > > +++ b/drivers/i2c/busses/i2c-rcar.c
> > > @@ -487,6 +487,8 @@ static const struct i2c_algorithm rcar_i2c_algo = {
> > >  
> > >  static const struct of_device_id rcar_i2c_dt_ids[] = {
> > >  	{ .compatible = "renesas,i2c-rcar", .data = (void *)I2C_RCAR_GEN1 },
> > > +	{ .compatible = "renesas,i2c-rcar-gen1", .data = (void *)I2C_RCAR_GEN1 },
> > > +	{ .compatible = "renesas,i2c-rcar-gen2", .data = (void *)I2C_RCAR_GEN2 },
> > >  	{ .compatible = "renesas,i2c-r8a7778", .data = (void *)I2C_RCAR_GEN1 },
> > >  	{ .compatible = "renesas,i2c-r8a7779", .data = (void *)I2C_RCAR_GEN1 },
> > >  	{ .compatible = "renesas,i2c-r8a7790", .data = (void *)I2C_RCAR_GEN2 },
> > > -- 
> > > 1.7.9.5
> > > 
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 1/5] i2c: rcar: add renesas,i2c-rcar-gen1/gen2 in DT compatible

[Kuninori Morimoto]

Hi Simon

> At our face-to-face meeting in Montpellier we discussed the
> idea of generation bindings. And my recollection is that Magnus
> and I had strong reservations about declaring what generation compatibility
> is without it being explicitly stated in hardware documentation.
> 
> From observation we can say that the i2c controllers on r8a7778 and r8a7779
> appear to be compatible. But can we say that in fact they are in fact
> compatible hardware. That they are different hardware instances of a common
> i2c controller whose name may or may not be is known to us? If not
> I do not think that using a common binding describes the hardware,
> which is the intention of DT bindings.
> 
> A second problem is the using the generation as a name.  Assuming the
> answer to the above question is yes can we further say with certainty
> that all variants of Gen1 SoCs that currently exist or will exist in the
> future are compatible?  If not then using Gen1 as the name does not seem
> to accurately describe the hardware.

This is the reason why SoC side have multi compatible name ?
My understanding is that, generation compatibility support
common feature, SoC compatibility support specific feature.

       compatible = "renesas,i2c-r8a7790", renesas,i2c-rcar-gen2"

If r8a7790 has special feature, then, it match to first compatible name
(or it can have special property)
If it is same as other Gen2, then, it can match to 2nd name.

HW doesn't have IP specific version, but latest datasheet
is indicating all H2/M2/E2/V2.
And, from HW team point of view, they don't want to create
same name but multi feature IP in same generation.
Creating and Testing new IP needs too much time / money / paper work.

Best regards
---
Kuninori Morimoto

Re: [PATCH 1/5] i2c: rcar: add renesas,i2c-rcar-gen1/gen2 in DT compatible

[Magnus Damm]

Hi Morimoto-san,

On Thu, Aug 7, 2014 at 11:43 AM, Kuninori Morimoto
<kuninori.morimoto.gx@gmail.com> wrote:
>
> Hi Simon
>
>> At our face-to-face meeting in Montpellier we discussed the
>> idea of generation bindings. And my recollection is that Magnus
>> and I had strong reservations about declaring what generation compatibility
>> is without it being explicitly stated in hardware documentation.
>>
>> From observation we can say that the i2c controllers on r8a7778 and r8a7779
>> appear to be compatible. But can we say that in fact they are in fact
>> compatible hardware. That they are different hardware instances of a common
>> i2c controller whose name may or may not be is known to us? If not
>> I do not think that using a common binding describes the hardware,
>> which is the intention of DT bindings.
>>
>> A second problem is the using the generation as a name.  Assuming the
>> answer to the above question is yes can we further say with certainty
>> that all variants of Gen1 SoCs that currently exist or will exist in the
>> future are compatible?  If not then using Gen1 as the name does not seem
>> to accurately describe the hardware.
>
> This is the reason why SoC side have multi compatible name ?
> My understanding is that, generation compatibility support
> common feature, SoC compatibility support specific feature.
>
>        compatible = "renesas,i2c-r8a7790", renesas,i2c-rcar-gen2"
>
> If r8a7790 has special feature, then, it match to first compatible name
> (or it can have special property)
> If it is same as other Gen2, then, it can match to 2nd name.

The problem is that "gen2" is not something that is pre-defined. As
you may have noticed earlier, new SoCs keep on coming and even though
they may be part of "gen2" they may or may not be compatible with the
"gen2" compatible string. So based on that, if we use the SoC part
number in the compatible string we at least know what the support
status is.

> HW doesn't have IP specific version, but latest datasheet
> is indicating all H2/M2/E2/V2.
> And, from HW team point of view, they don't want to create
> same name but multi feature IP in same generation.
> Creating and Testing new IP needs too much time / money / paper work.

So I agree that sharing hardware makes sense from a resource saving
point of view. However in reality there may be smaller differences
between devices used for each version within the generation though.

Now, if we could get the hardware version number from the device team
somehow....

Thanks,

/ magnus

Re: [PATCH 1/5] i2c: rcar: add renesas,i2c-rcar-gen1/gen2 in DT compatible

[Kuninori Morimoto]

Hi Magnus

> The problem is that "gen2" is not something that is pre-defined. As
> you may have noticed earlier, new SoCs keep on coming and even though
> they may be part of "gen2" they may or may not be compatible with the
> "gen2" compatible string. So based on that, if we use the SoC part
> number in the compatible string we at least know what the support
> status is.

Do you mean "driver" table ?
This patch doesn't remove SoC level compatible from driver.

And additionally,
we can check support status in Document/devicetree/...txt
and Geert is doing it now ?
(Which SoC is working/supporting)

> So I agree that sharing hardware makes sense from a resource saving
> point of view. However in reality there may be smaller differences
> between devices used for each version within the generation though.

Yes, "maybe" it has smaller difference, but what is the problem ?
Driver is already have SoC level compatible.
Why we can't update it ?

Or, "maybe" we can add new property for it ?
It can keep compatible anyway, because current code is already working.

In my opinion, we can use SoC level compatible name if it was special,
otherwise, we can use generation level compatible.
Platform side DTS file should include SoC and generation level compatible
from first support patch. If possible, series compatible too.
like this

	compatible = "renesas,i2c-r8a7790", "renesas,i2c-rcar-gen2", "renesas,i2c-rcar"

But, it is too late for Gen1/Gen2.
I don't want to have more pointless string in driver in Gen3 or later.

But yes, we want to know which SoC is supported.
We can use Document/devicetree/...txt for this purpose

Best regards
---
Kuninori Morimoto

Re: [PATCH 1/5] i2c: rcar: add renesas,i2c-rcar-gen1/gen2 in DT compatible

[Magnus Damm]

Hi Morimoto-san,

Thanks for your comments!

On Thu, Aug 7, 2014 at 2:19 PM, Kuninori Morimoto
<kuninori.morimoto.gx@gmail.com> wrote:
>
> Hi Magnus
>
>> The problem is that "gen2" is not something that is pre-defined. As
>> you may have noticed earlier, new SoCs keep on coming and even though
>> they may be part of "gen2" they may or may not be compatible with the
>> "gen2" compatible string. So based on that, if we use the SoC part
>> number in the compatible string we at least know what the support
>> status is.
>
> Do you mean "driver" table ?
> This patch doesn't remove SoC level compatible from driver.

Yes, this applies to the compatible string table in each driver.
Because "gen2" is an ongoing project we have no idea what kind of new
products that will come out - that may or may not be compatible with
the "gen2" current compatible strings... And these strings are only
really specifying if our current version of the driver is compatible
or not. We should instead describe the hardware dependency IMO.

> And additionally,
> we can check support status in Document/devicetree/...txt
> and Geert is doing it now ?
> (Which SoC is working/supporting)

Right, that is good!

My main concern is not the support status though. It is more about how
to handle the "gen2" compatible string over time. Please see below for
some example of difficulties introduced by this approach.

>> So I agree that sharing hardware makes sense from a resource saving
>> point of view. However in reality there may be smaller differences
>> between devices used for each version within the generation though.
>
> Yes, "maybe" it has smaller difference, but what is the problem ?
> Driver is already have SoC level compatible.
> Why we can't update it ?

We can of course update anything, the question is just why we should. =)

> Or, "maybe" we can add new property for it ?
> It can keep compatible anyway, because current code is already working.

In my opinion having a "gen2" compatible string in the driver does not
help anything. For the existing SoCs we already have compatible
strings. For new SoCs we can simply add the new part number to the
driver.

Perhaps you are thinking about the case when new "compatible" SoCs
come out, the idea that we want to not modify the driver but just add
DTS and then everything should work as-is. This is a great idea, and
for this to work we want to have "hardware block version number" from
the device team. Until we have that I believe it is best to not use
"gen2" compatible strings. Let me explain why.

1) We don't know if the hardware is compatible unless it is documented to be so
2) We can test using the same compatible string, but it just means the
current driver implementation happens to work
3) If all registers and features are used by the driver then we can
perhaps assume that we have tested the hardware
4) The common case with a driver is that not all hardware features are
used, so some are untested and unknown compatibility wise
5) If we use "gen2" compatible string then future driver feature
extensions may break some hardware
6) To avoid breakage in 5) we have test on all known platforms which
is quite difficult
7) People will ship out of tree code using the generic binding so we
cannot perform testing in 6) on all platforms
8) The meaning of "gen2" is changing over time, so it is not a fixed
definition of anything
9) In the end, this discussion is matter of describing compatibility
of hardware or software implementation.
10) DT bindings should in theory work with multiple operating systems.
So we have to describe hardware, not software.

With this in mind I think the best way forward is as follows:
A) Keep on using per-SoC compatible string unless it is documented somewhere
and in parallel
B) Work on getting per-device hardware version number from hardware device team

> In my opinion, we can use SoC level compatible name if it was special,
> otherwise, we can use generation level compatible.
> Platform side DTS file should include SoC and generation level compatible
> from first support patch. If possible, series compatible too.
> like this
>
>         compatible = "renesas,i2c-r8a7790", "renesas,i2c-rcar-gen2", "renesas,i2c-rcar"
>
> But, it is too late for Gen1/Gen2.
> I don't want to have more pointless string in driver in Gen3 or later.

I understand your opinion. I don't want to have pointless strings in
new SoCs either.

From my point of view your proposal adds problematic dependencies for
each device without too much upside. The problem will happen over time
when adding features. It is in my opinion easier to just add the
compatible string entry for a new SoC together with the rest of the
SoC support activity. There is no easy way out when the hardware is
changing - we have to modify the code anyway! Of course, if the
hardware is not changing then it is a different story. But in such
case we need to have documented compatibility somewhere...

What is your opinion about "B) per-device hardware version number"?

> But yes, we want to know which SoC is supported.
> We can use Document/devicetree/...txt for this purpose

I agree about that portion. =)

Cheers,

/ magnus

Re: [PATCH 1/5] i2c: rcar: add renesas,i2c-rcar-gen1/gen2 in DT compatible

[Kuninori Morimoto]

Hi Magnus

> With this in mind I think the best way forward is as follows:
> A) Keep on using per-SoC compatible string unless it is documented somewhere
> and in parallel
> B) Work on getting per-device hardware version number from hardware device team
(snip)
> From my point of view your proposal adds problematic dependencies for
> each device without too much upside. The problem will happen over time
> when adding features. It is in my opinion easier to just add the
> compatible string entry for a new SoC together with the rest of the
> SoC support activity. There is no easy way out when the hardware is
> changing - we have to modify the code anyway! Of course, if the
> hardware is not changing then it is a different story. But in such
> case we need to have documented compatibility somewhere...
> 
> What is your opinion about "B) per-device hardware version number"?

According to HW team, they are creating 2 type of datasheet now.
One is "R-Car Gen2 common datasheet",
and the other is "R-Car SoC specify datasheet".
Not, HW version though.

I could understand about your concern.
But, it is always happen on all code
which is used from many platform/driver/framework.
It is not only DT compatible issue.

I still can't give up adding generation level compatible.
So how about this idea ?
Now, our driver / platform are like this.
SoC level matching only.

driver
	{ .compatible = "renesas,xxx-r8a7790", .data = xxx_gen2_dt_config },
	{ .compatible = "renesas,xxx-r8a7791", .data = xxx_gen2_dt_config },
	{ .compatible = "renesas,xxx-r8a7792", .data = xxx_gen2_dt_config },

platform
	compatible = "renesas,xxx-r8a7790";


Now, we add generation level compatible to platform side only.
We don't know what is "gen2" mean at this point.
But, this is no problem, because driver matching is happen on SoC level.

driver
	{ .compatible = "renesas,xxx-r8a7790", .data = xxx_gen2_dt_config },
	{ .compatible = "renesas,xxx-r8a7791", .data = xxx_gen2_dt_config },
	{ .compatible = "renesas,xxx-r8a7792", .data = xxx_gen2_dt_config },

platform
-	compatible = "renesas,xxx-r8a7790";
+	compatible = "renesas,xxx-r8a7790", "renesas,xxx-gen2";

In the future, if our driver are growup enough,
and we could confirm there is no difference between r8a7790/1/2 in HW,
and we could confirm there is no drfference between r8a7790/1/2 in SW,
then, we can add generation level compatible,
and remove SoC level compatible in driver side,

driver
+	{ .compatible = "renesas,xxx-gen2",    .data = xxx_gen2_dt_config },
-	{ .compatible = "renesas,xxx-r8a7790", .data = xxx_gen2_dt_config },
-	{ .compatible = "renesas,xxx-r8a7791", .data = xxx_gen2_dt_config },
-	{ .compatible = "renesas,xxx-r8a7792", .data = xxx_gen2_dt_config },

platform
	compatible = "renesas,xxx-r8a7790", "renesas,xxx-gen2";

If there is non-compatible feature, we can keep it.
like this

driver
+	{ .compatible = "renesas,xxx-gen2",    .data = xxx_gen2_dt_config },
-	{ .compatible = "renesas,xxx-r8a7790", .data = xxx_gen2_dt_config },
-	{ .compatible = "renesas,xxx-r8a7791", .data = xxx_gen2_dt_config },
-	{ .compatible = "renesas,xxx-r8a7792", .data = xxx_gen2_dt_config },
+	{ .compatible = "renesas,xxx-r8a7792", .data = xxx_r8a7792_dt_config },

platform
	compatible = "renesas,xxx-r8a7790", "renesas,xxx-gen2";

This approach has no effect to current feature,
and can keep compatible for current and future,
and can reduce / cleanup driver in the future.

>> But yes, we want to know which SoC is supported.
>> We can use Document/devicetree/...txt for this purpose
>
> I agree about that portion. =)

Thank you :)

Best regards
---
Kuninori Morimoto

Re: [PATCH 1/5] i2c: rcar: add renesas,i2c-rcar-gen1/gen2 in DT compatible

[Wolfram Sang]

> This approach has no effect to current feature,
> and can keep compatible for current and future,
> and can reduce / cleanup driver in the future.

I defer the patch series for now. Maybe we should talk about it at our
next meeting?

Re: [PATCH 1/5] i2c: rcar: add renesas,i2c-rcar-gen1/gen2 in DT compatible

[Simon Horman]

On Fri, Sep 19, 2014 at 07:18:51PM +0200, Wolfram Sang wrote:
> 
> > This approach has no effect to current feature,
> > and can keep compatible for current and future,
> > and can reduce / cleanup driver in the future.
> 
> I defer the patch series for now. Maybe we should talk about it at our
> next meeting?

That is fine by me.

[PATCH 2/5] ARM: shmobile: r8a7778: add generation level compatible for i2c

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 arch/arm/boot/dts/r8a7778.dtsi |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/r8a7778.dtsi b/arch/arm/boot/dts/r8a7778.dtsi
index ecfdf4b..1379a59 100644
--- a/arch/arm/boot/dts/r8a7778.dtsi
+++ b/arch/arm/boot/dts/r8a7778.dtsi
@@ -123,7 +123,7 @@
 	i2c0: i2c@ffc70000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
-		compatible = "renesas,i2c-r8a7778";
+		compatible = "renesas,i2c-r8a7778", "renesas,i2c-rcar-gen1";
 		reg = <0xffc70000 0x1000>;
 		interrupts = <0 67 IRQ_TYPE_LEVEL_HIGH>;
 		status = "disabled";
@@ -132,7 +132,7 @@
 	i2c1: i2c@ffc71000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
-		compatible = "renesas,i2c-r8a7778";
+		compatible = "renesas,i2c-r8a7778", "renesas,i2c-rcar-gen1";
 		reg = <0xffc71000 0x1000>;
 		interrupts = <0 78 IRQ_TYPE_LEVEL_HIGH>;
 		status = "disabled";
@@ -141,7 +141,7 @@
 	i2c2: i2c@ffc72000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
-		compatible = "renesas,i2c-r8a7778";
+		compatible = "renesas,i2c-r8a7778", "renesas,i2c-rcar-gen1";
 		reg = <0xffc72000 0x1000>;
 		interrupts = <0 76 IRQ_TYPE_LEVEL_HIGH>;
 		status = "disabled";
@@ -150,7 +150,7 @@
 	i2c3: i2c@ffc73000 {
 		#address-cells = <1>;
 		#size-cells = <0>;
-		compatible = "renesas,i2c-r8a7778";
+		compatible = "renesas,i2c-r8a7778", "renesas,i2c-rcar-gen1";
 		reg = <0xffc73000 0x1000>;
 		interrupts = <0 77 IRQ_TYPE_LEVEL_HIGH>;
 		status = "disabled";
-- 
1.7.9.5

Re: [PATCH 0/5] i2c: rcar: add renesas,i2c-rcar-gen1/gen2 in DT compatible

[Wolfram Sang]

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

On Tue, Aug 05, 2014 at 09:39:27PM -0700, Kuninori Morimoto wrote:
> 
> Hi Wolfram, Simon
> 
> These patches add R-Car generatoin level
> DT compatible on i2c-rcar driver and platform.
> 
> Kuninori Morimoto (5):
>   i2c: rcar: add renesas,i2c-rcar-gen1/gen2 in DT compatible
>   ARM: shmobile: r8a7778: add generation level compatible for i2c
>   ARM: shmobile: r8a7779: add generation level compatible for i2c
>   ARM: shmobile: r8a7790: add generation level compatible for i2c
>   ARM: shmobile: r8a7791: add generation level compatible for i2c

All patches:

Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Simon: Shall I take patch 1?


[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]