diff mbox

[V2,2/2] ARM: shmobile: r8a7791: add i2c2 bus to koelsch dt

Message ID 1392633882-12142-2-git-send-email-wsa@the-dreams.de (mailing list archive)
State New, archived
Headers show

Commit Message

Wolfram Sang Feb. 17, 2014, 10:44 a.m. UTC
From: Wolfram Sang <wsa@sang-engineering.com>

Signed-off-by: Wolfram Sang <wsa@sang-engineering.com>
---
 arch/arm/boot/dts/r8a7791-koelsch.dts | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

Comments

Magnus Damm Feb. 17, 2014, 12:07 p.m. UTC | #1
On Mon, Feb 17, 2014 at 7:44 PM, Wolfram Sang <wsa@the-dreams.de> wrote:
> From: Wolfram Sang <wsa@sang-engineering.com>
>
> Signed-off-by: Wolfram Sang <wsa@sang-engineering.com>
> ---
>  arch/arm/boot/dts/r8a7791-koelsch.dts | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)

Acked-by: Magnus Damm <damm@opensource.se>

Thanks,

/ magnus
Sergei Shtylyov May 8, 2014, 11:54 p.m. UTC | #2
Hello.

On 02/17/2014 01:44 PM, Wolfram Sang wrote:

> From: Wolfram Sang <wsa@sang-engineering.com>

> Signed-off-by: Wolfram Sang <wsa@sang-engineering.com>
> ---
>   arch/arm/boot/dts/r8a7791-koelsch.dts | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)

> diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
> index d4b9bba..b3f75d7 100644
> --- a/arch/arm/boot/dts/r8a7791-koelsch.dts
> +++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
> @@ -108,10 +108,29 @@
>   	clock-frequency = <20000000>;
>   };
>
> +&i2c2 {
> +	pinctrl-0 = <&i2c2_pins>;
> +	pinctrl-names = "default";
> +
> +	status = "okay";
> +	clock-frequency = <400000>;
> +
> +	eeprom@50 {
> +		compatible = "renesas,24c02";

    This is not quite right: Renesas' part is called differently, and you're 
carrying the other vendor's naming onto Renesas parts. I'd just say "24c02" 
(which I'll do in the Henninger board patch).

WBR, Sergei
Wolfram Sang May 9, 2014, 5:06 a.m. UTC | #3
> >+	eeprom@50 {
> >+		compatible = "renesas,24c02";
> 
>    This is not quite right: Renesas' part is called differently, and
> you're carrying the other vendor's naming onto Renesas parts. I'd
> just say "24c02" (which I'll do in the Henninger board patch).

What about "generic"?
Sergei Shtylyov May 9, 2014, 10:59 p.m. UTC | #4
Hello.

On 05/09/2014 09:06 AM, Wolfram Sang wrote:

>>> +	eeprom@50 {
>>> +		compatible = "renesas,24c02";

>>     This is not quite right: Renesas' part is called differently, and
>> you're carrying the other vendor's naming onto Renesas parts. I'd
>> just say "24c02" (which I'll do in the Henninger board patch).

> What about "generic"?

    I don't know, really. However, you're right in that there should be some 
vendor prefix -- I've just seen Documentation/devicetree/bindings/eeprom.txt.

WBR, Sergei
Simon Horman May 11, 2014, 12:08 a.m. UTC | #5
On Sat, May 10, 2014 at 02:59:15AM +0400, Sergei Shtylyov wrote:
> Hello.
> 
> On 05/09/2014 09:06 AM, Wolfram Sang wrote:
> 
> >>>+	eeprom@50 {
> >>>+		compatible = "renesas,24c02";
> 
> >>    This is not quite right: Renesas' part is called differently, and
> >>you're carrying the other vendor's naming onto Renesas parts. I'd
> >>just say "24c02" (which I'll do in the Henninger board patch).
> 
> >What about "generic"?
> 
>    I don't know, really. However, you're right in that there should
> be some vendor prefix -- I've just seen
> Documentation/devicetree/bindings/eeprom.txt.

A quick survey of arch/arm/boot/dts/ yields:

"atmel,24c02" (22 times)
"microchip,24c02" (once)
"mcp,24c02" (once)

So it seems to me that precedence is in favour of using "renesas,24c02".
Geert Uytterhoeven May 12, 2014, 7:35 a.m. UTC | #6
On Sun, May 11, 2014 at 2:08 AM, Simon Horman <horms@verge.net.au> wrote:
> On Sat, May 10, 2014 at 02:59:15AM +0400, Sergei Shtylyov wrote:
>> Hello.
>>
>> On 05/09/2014 09:06 AM, Wolfram Sang wrote:
>>
>> >>>+  eeprom@50 {
>> >>>+          compatible = "renesas,24c02";
>>
>> >>    This is not quite right: Renesas' part is called differently, and
>> >>you're carrying the other vendor's naming onto Renesas parts. I'd
>> >>just say "24c02" (which I'll do in the Henninger board patch).
>>
>> >What about "generic"?
>>
>>    I don't know, really. However, you're right in that there should
>> be some vendor prefix -- I've just seen
>> Documentation/devicetree/bindings/eeprom.txt.
>
> A quick survey of arch/arm/boot/dts/ yields:
>
> "atmel,24c02" (22 times)
> "microchip,24c02" (once)
> "mcp,24c02" (once)
>
> So it seems to me that precedence is in favour of using "renesas,24c02".

Except that the Renesas part is called differently (as Sergei already pointed
out), unlike the Atmel and Microchip parts.

So it should be:

        compatible = "renesas,r1ex24002", "<something>,24c02";

Unfortunately the datasheet doesn't mention compatibility with parts from
other manufacturers.

So let's <something> be "atmel"?

BTW, we have a similar issue with Genmai, which has an R1EX24128.

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
Sergei Shtylyov May 12, 2014, 12:10 p.m. UTC | #7
Hello.

On 05/12/2014 11:35 AM, Geert Uytterhoeven wrote:

>>>>>> +  eeprom@50 {
>>>>>> +          compatible = "renesas,24c02";

>>>>>     This is not quite right: Renesas' part is called differently, and
>>>>> you're carrying the other vendor's naming onto Renesas parts. I'd
>>>>> just say "24c02" (which I'll do in the Henninger board patch).

>>>> What about "generic"?

>>>     I don't know, really. However, you're right in that there should
>>> be some vendor prefix -- I've just seen
>>> Documentation/devicetree/bindings/eeprom.txt.

>> A quick survey of arch/arm/boot/dts/ yields:

>> "atmel,24c02" (22 times)
>> "microchip,24c02" (once)
>> "mcp,24c02" (once)

>> So it seems to me that precedence is in favour of using "renesas,24c02".

> Except that the Renesas part is called differently (as Sergei already pointed
> out), unlike the Atmel and Microchip parts.

> So it should be:

>          compatible = "renesas,r1ex24002", "<something>,24c02";

    Unfortunately, that way the legacy I2C driver matching used now won't work 
anymore. :-(

> Unfortunately the datasheet doesn't mention compatibility with parts from
> other manufacturers.

> So let's <something> be "atmel"?

    Yes, probably. But the "renesas,r1ex24002" part have to be dropped.

> Gr{oetje,eeting}s,
>                          Geert

WBR, Sergei
Simon Horman May 13, 2014, 12:07 a.m. UTC | #8
On Mon, May 12, 2014 at 09:35:46AM +0200, Geert Uytterhoeven wrote:
> On Sun, May 11, 2014 at 2:08 AM, Simon Horman <horms@verge.net.au> wrote:
> > On Sat, May 10, 2014 at 02:59:15AM +0400, Sergei Shtylyov wrote:
> >> Hello.
> >>
> >> On 05/09/2014 09:06 AM, Wolfram Sang wrote:
> >>
> >> >>>+  eeprom@50 {
> >> >>>+          compatible = "renesas,24c02";
> >>
> >> >>    This is not quite right: Renesas' part is called differently, and
> >> >>you're carrying the other vendor's naming onto Renesas parts. I'd
> >> >>just say "24c02" (which I'll do in the Henninger board patch).
> >>
> >> >What about "generic"?
> >>
> >>    I don't know, really. However, you're right in that there should
> >> be some vendor prefix -- I've just seen
> >> Documentation/devicetree/bindings/eeprom.txt.
> >
> > A quick survey of arch/arm/boot/dts/ yields:
> >
> > "atmel,24c02" (22 times)
> > "microchip,24c02" (once)
> > "mcp,24c02" (once)
> >
> > So it seems to me that precedence is in favour of using "renesas,24c02".
> 
> Except that the Renesas part is called differently (as Sergei already pointed
> out), unlike the Atmel and Microchip parts.
> 
> So it should be:
> 
>         compatible = "renesas,r1ex24002", "<something>,24c02";
> 
> Unfortunately the datasheet doesn't mention compatibility with parts from
> other manufacturers.
> 
> So let's <something> be "atmel"?

If <something> isn't documented as being compatible then
I feel that we should not add it to the list.

> BTW, we have a similar issue with Genmai, which has an R1EX24128.
> 
> 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
> --
> 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
>
diff mbox

Patch

diff --git a/arch/arm/boot/dts/r8a7791-koelsch.dts b/arch/arm/boot/dts/r8a7791-koelsch.dts
index d4b9bba..b3f75d7 100644
--- a/arch/arm/boot/dts/r8a7791-koelsch.dts
+++ b/arch/arm/boot/dts/r8a7791-koelsch.dts
@@ -108,10 +108,29 @@ 
 	clock-frequency = <20000000>;
 };
 
+&i2c2 {
+	pinctrl-0 = <&i2c2_pins>;
+	pinctrl-names = "default";
+
+	status = "okay";
+	clock-frequency = <400000>;
+
+	eeprom@50 {
+		compatible = "renesas,24c02";
+		reg = <0x50>;
+		pagesize = <16>;
+	};
+};
+
 &pfc {
 	pinctrl-0 = <&scif0_pins &scif1_pins>;
 	pinctrl-names = "default";
 
+	i2c2_pins: i2c {
+		renesas,groups = "i2c2";
+		renesas,function = "i2c2";
+	};
+
 	scif0_pins: serial0 {
 		renesas,groups = "scif0_data_d";
 		renesas,function = "scif0";