diff mbox

[RFC/PATCH,11/14] dt: omap3: add soc file for handling i2c controllers

Message ID 1312897232-4792-12-git-send-email-manjugk@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

manjugk manjugk Aug. 9, 2011, 2:10 p.m. UTC
Add omap3 soc file for handling omap3 soc i2c controllers existing
on l4-core bus.

Signed-off-by: G, Manjunath Kondaiah <manjugk@ti.com>
---
 arch/arm/boot/dts/omap3-soc.dtsi |   62 ++++++++++++++++++++++++++++++++++++++
 1 files changed, 62 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/boot/dts/omap3-soc.dtsi

Comments

Benoit Cousson Aug. 10, 2011, 12:36 p.m. UTC | #1
On 8/9/2011 4:10 PM, G, Manjunath Kondaiah wrote:
>
> Add omap3 soc file for handling omap3 soc i2c controllers existing
> on l4-core bus.
>
> Signed-off-by: G, Manjunath Kondaiah<manjugk@ti.com>
> ---
>   arch/arm/boot/dts/omap3-soc.dtsi |   62 ++++++++++++++++++++++++++++++++++++++
>   1 files changed, 62 insertions(+), 0 deletions(-)
>   create mode 100644 arch/arm/boot/dts/omap3-soc.dtsi
>
> diff --git a/arch/arm/boot/dts/omap3-soc.dtsi b/arch/arm/boot/dts/omap3-soc.dtsi
> new file mode 100644
> index 0000000..85de92f
> --- /dev/null
> +++ b/arch/arm/boot/dts/omap3-soc.dtsi
> @@ -0,0 +1,62 @@
> +/*
> + * Device Tree Source for OMAP3 SoC
> + *
> + * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/
> + *
> + * This file is licensed under the terms of the GNU General Public License
> + * version 2.  This program is licensed "as is" without any warranty of any
> + * kind, whether express or implied.
> + */
> +
> +/dts-v1/;
> +/include/ "skeleton.dtsi"
> +
> +/ {
> +	#address-cells =<1>;
> +	#size-cells =<1>;
> +	model = "ti,omap3";
> +
> +	aliases {
> +		i2c1 =&i2c1;
> +		i2c2 =&i2c2;
> +		i2c3 =&i2c3;
> +	};
> +
> +	l4-core {

That comment is probably subject to discussion, but even if this 
interconnect is there physically, I'm not sure of the added value to add it.
It will add an extra level of indentation and that all. Moreover, it 
will mess up the physical address that are expressed using absolute 
value in the TRM with a less readable offset value.
In fact, most DTS files in the ARM directory are using a purely flat 
representation of the interconnect.

> +		compatible = "ti,omap3-l4-core";

Assuming we will keep that, you should probably add a more generic 
compatible name after that one. Like "ti,s3220" or even "sonics,s3220", 
assuming that this IP is generic enough for other SoC.

> +		#address-cells =<1>;
> +		#size-cells =<1>;
> +		ranges =<0 0x48000000 0x1000000>;
> +
> +		i2c1: i2c@70000 {
> +			#address-cells =<1>;
> +			#size-cells =<0>;
> +			compatible = "ti,omap3-i2c";

The I2C IP and thus the driver is generic across OMAP generations and is 
even potentially used by other non-OMAP TI chips like DSP or Davinci. So 
having an extra compatible entry with "ti,i2c" or "ti, omap-i2c" seems 
mandatory.

Regards,
Benoit
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
manjugk manjugk Aug. 10, 2011, 4:57 p.m. UTC | #2
On Wed, Aug 10, 2011 at 02:36:50PM +0200, Cousson, Benoit wrote:
> On 8/9/2011 4:10 PM, G, Manjunath Kondaiah wrote:
> >
> >Add omap3 soc file for handling omap3 soc i2c controllers existing
> >on l4-core bus.
> >
> >Signed-off-by: G, Manjunath Kondaiah<manjugk@ti.com>
> >---
> >  arch/arm/boot/dts/omap3-soc.dtsi |   62 ++++++++++++++++++++++++++++++++++++++
> >  1 files changed, 62 insertions(+), 0 deletions(-)
> >  create mode 100644 arch/arm/boot/dts/omap3-soc.dtsi
> >
> >diff --git a/arch/arm/boot/dts/omap3-soc.dtsi b/arch/arm/boot/dts/omap3-soc.dtsi
> >new file mode 100644
> >index 0000000..85de92f
> >--- /dev/null
> >+++ b/arch/arm/boot/dts/omap3-soc.dtsi
> >@@ -0,0 +1,62 @@
> >+/*
> >+ * Device Tree Source for OMAP3 SoC
> >+ *
> >+ * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/
> >+ *
> >+ * This file is licensed under the terms of the GNU General Public License
> >+ * version 2.  This program is licensed "as is" without any warranty of any
> >+ * kind, whether express or implied.
> >+ */
> >+
> >+/dts-v1/;
> >+/include/ "skeleton.dtsi"
> >+
> >+/ {
> >+	#address-cells =<1>;
> >+	#size-cells =<1>;
> >+	model = "ti,omap3";
> >+
> >+	aliases {
> >+		i2c1 =&i2c1;
> >+		i2c2 =&i2c2;
> >+		i2c3 =&i2c3;
> >+	};
> >+
> >+	l4-core {
> 
> That comment is probably subject to discussion, but even if this
> interconnect is there physically, I'm not sure of the added value to
> add it.
> It will add an extra level of indentation and that all. Moreover, it
> will mess up the physical address that are expressed using absolute
> value in the TRM with a less readable offset value.
> In fact, most DTS files in the ARM directory are using a purely flat
> representation of the interconnect.
This is as per alignment with Tony and Grant:
http://permalink.gmane.org/gmane.linux.ports.arm.omap/60391

> 
> >+		compatible = "ti,omap3-l4-core";
> 
> Assuming we will keep that, you should probably add a more generic
> compatible name after that one. Like "ti,s3220" or even
> "sonics,s3220", assuming that this IP is generic enough for other
> SoC.
will check this. I don't remember any generic names.
> 
> >+		#address-cells =<1>;
> >+		#size-cells =<1>;
> >+		ranges =<0 0x48000000 0x1000000>;
> >+
> >+		i2c1: i2c@70000 {
> >+			#address-cells =<1>;
> >+			#size-cells =<0>;
> >+			compatible = "ti,omap3-i2c";
> 
> The I2C IP and thus the driver is generic across OMAP generations
> and is even potentially used by other non-OMAP TI chips like DSP or
> Davinci. So having an extra compatible entry with "ti,i2c" or "ti,
> omap-i2c" seems mandatory.
This can be updated as and when new soc/board adaptations are done. 
As of now, it is omap3 and when we have omap4 it will be appended with
"ti,omap4-i2c" etc

-M
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Benoit Cousson Aug. 10, 2011, 5:45 p.m. UTC | #3
+ Tony

On 8/10/2011 6:57 PM, G, Manjunath Kondaiah wrote:
> On Wed, Aug 10, 2011 at 02:36:50PM +0200, Cousson, Benoit wrote:
>> On 8/9/2011 4:10 PM, G, Manjunath Kondaiah wrote:
>>>
>>> Add omap3 soc file for handling omap3 soc i2c controllers existing
>>> on l4-core bus.
>>>
>>> Signed-off-by: G, Manjunath Kondaiah<manjugk@ti.com>
>>> ---
>>>   arch/arm/boot/dts/omap3-soc.dtsi |   62 ++++++++++++++++++++++++++++++++++++++
>>>   1 files changed, 62 insertions(+), 0 deletions(-)
>>>   create mode 100644 arch/arm/boot/dts/omap3-soc.dtsi
>>>
>>> diff --git a/arch/arm/boot/dts/omap3-soc.dtsi b/arch/arm/boot/dts/omap3-soc.dtsi
>>> new file mode 100644
>>> index 0000000..85de92f
>>> --- /dev/null
>>> +++ b/arch/arm/boot/dts/omap3-soc.dtsi
>>> @@ -0,0 +1,62 @@
>>> +/*
>>> + * Device Tree Source for OMAP3 SoC
>>> + *
>>> + * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/
>>> + *
>>> + * This file is licensed under the terms of the GNU General Public License
>>> + * version 2.  This program is licensed "as is" without any warranty of any
>>> + * kind, whether express or implied.
>>> + */
>>> +
>>> +/dts-v1/;
>>> +/include/ "skeleton.dtsi"
>>> +
>>> +/ {
>>> +	#address-cells =<1>;
>>> +	#size-cells =<1>;
>>> +	model = "ti,omap3";
>>> +
>>> +	aliases {
>>> +		i2c1 =&i2c1;
>>> +		i2c2 =&i2c2;
>>> +		i2c3 =&i2c3;
>>> +	};
>>> +
>>> +	l4-core {
>>
>> That comment is probably subject to discussion, but even if this
>> interconnect is there physically, I'm not sure of the added value to
>> add it.
>> It will add an extra level of indentation and that all. Moreover, it
>> will mess up the physical address that are expressed using absolute
>> value in the TRM with a less readable offset value.
>> In fact, most DTS files in the ARM directory are using a purely flat
>> representation of the interconnect.
> This is as per alignment with Tony and Grant:
> http://permalink.gmane.org/gmane.linux.ports.arm.omap/60391

So I'm asking the same question to Grant and Tony then:-)

>>> +		compatible = "ti,omap3-l4-core";
>>
>> Assuming we will keep that, you should probably add a more generic
>> compatible name after that one. Like "ti,s3220" or even
>> "sonics,s3220", assuming that this IP is generic enough for other
>> SoC.
> will check this. I don't remember any generic names.
>>
>>> +		#address-cells =<1>;
>>> +		#size-cells =<1>;
>>> +		ranges =<0 0x48000000 0x1000000>;
>>> +
>>> +		i2c1: i2c@70000 {
>>> +			#address-cells =<1>;
>>> +			#size-cells =<0>;
>>> +			compatible = "ti,omap3-i2c";
>>
>> The I2C IP and thus the driver is generic across OMAP generations
>> and is even potentially used by other non-OMAP TI chips like DSP or
>> Davinci. So having an extra compatible entry with "ti,i2c" or "ti,
>> omap-i2c" seems mandatory.
> This can be updated as and when new soc/board adaptations are done.
> As of now, it is omap3 and when we have omap4 it will be appended with
> "ti,omap4-i2c" etc

To infinity and beyond?

There is no need, and we should absolutely not update the driver each 
time we introduce a new SoC version/revision.
The driver should match with the generic device name in that case. Until 
we change the IP and potentially introduce a new driver.

BTW, even omap3 should not be in the name in theory. It should be 
potentially "ti,i2c-v3" or whatever HW version the IP is using.
But for some reason, most devices are using such convention in existing 
DTS:-(
This is probably due to the lack of well identified IP version in most 
SoC... including OMAP:-)


Benoit
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
manjugk manjugk Aug. 16, 2011, 6:32 a.m. UTC | #4
On Wed, Aug 10, 2011 at 07:45:42PM +0200, Cousson, Benoit wrote:
> + Tony
> 
> On 8/10/2011 6:57 PM, G, Manjunath Kondaiah wrote:
> >On Wed, Aug 10, 2011 at 02:36:50PM +0200, Cousson, Benoit wrote:
> >>On 8/9/2011 4:10 PM, G, Manjunath Kondaiah wrote:
> >>>
> >>>Add omap3 soc file for handling omap3 soc i2c controllers existing
> >>>on l4-core bus.
> >>>
> >>>Signed-off-by: G, Manjunath Kondaiah<manjugk@ti.com>
> >>>---
> >>>  arch/arm/boot/dts/omap3-soc.dtsi |   62 ++++++++++++++++++++++++++++++++++++++
> >>>  1 files changed, 62 insertions(+), 0 deletions(-)
> >>>  create mode 100644 arch/arm/boot/dts/omap3-soc.dtsi
> >>>
> >>>diff --git a/arch/arm/boot/dts/omap3-soc.dtsi b/arch/arm/boot/dts/omap3-soc.dtsi
> >>>new file mode 100644
> >>>index 0000000..85de92f
> >>>--- /dev/null
> >>>+++ b/arch/arm/boot/dts/omap3-soc.dtsi
> >>>@@ -0,0 +1,62 @@
> >>>+/*
> >>>+ * Device Tree Source for OMAP3 SoC
> >>>+ *
> >>>+ * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/
> >>>+ *
> >>>+ * This file is licensed under the terms of the GNU General Public License
> >>>+ * version 2.  This program is licensed "as is" without any warranty of any
> >>>+ * kind, whether express or implied.
> >>>+ */
> >>>+
> >>>+/dts-v1/;
> >>>+/include/ "skeleton.dtsi"
> >>>+
> >>>+/ {
> >>>+	#address-cells =<1>;
> >>>+	#size-cells =<1>;
> >>>+	model = "ti,omap3";
> >>>+
> >>>+	aliases {
> >>>+		i2c1 =&i2c1;
> >>>+		i2c2 =&i2c2;
> >>>+		i2c3 =&i2c3;
> >>>+	};
> >>>+
> >>>+	l4-core {
> >>
> >>That comment is probably subject to discussion, but even if this
> >>interconnect is there physically, I'm not sure of the added value to
> >>add it.
> >>It will add an extra level of indentation and that all. Moreover, it
> >>will mess up the physical address that are expressed using absolute
> >>value in the TRM with a less readable offset value.
> >>In fact, most DTS files in the ARM directory are using a purely flat
> >>representation of the interconnect.
> >This is as per alignment with Tony and Grant:
> >http://permalink.gmane.org/gmane.linux.ports.arm.omap/60391
> 
> So I'm asking the same question to Grant and Tony then:-)

What is the conclusion here? I will go ahead with current approach since
tony and grant has voted for it.

-M

> 
> >>>+		compatible = "ti,omap3-l4-core";
> >>
> >>Assuming we will keep that, you should probably add a more generic
> >>compatible name after that one. Like "ti,s3220" or even
> >>"sonics,s3220", assuming that this IP is generic enough for other
> >>SoC.
> >will check this. I don't remember any generic names.
> >>
> >>>+		#address-cells =<1>;
> >>>+		#size-cells =<1>;
> >>>+		ranges =<0 0x48000000 0x1000000>;
> >>>+
> >>>+		i2c1: i2c@70000 {
> >>>+			#address-cells =<1>;
> >>>+			#size-cells =<0>;
> >>>+			compatible = "ti,omap3-i2c";
> >>
> >>The I2C IP and thus the driver is generic across OMAP generations
> >>and is even potentially used by other non-OMAP TI chips like DSP or
> >>Davinci. So having an extra compatible entry with "ti,i2c" or "ti,
> >>omap-i2c" seems mandatory.
> >This can be updated as and when new soc/board adaptations are done.
> >As of now, it is omap3 and when we have omap4 it will be appended with
> >"ti,omap4-i2c" etc
> 
> To infinity and beyond?
> 
> There is no need, and we should absolutely not update the driver
> each time we introduce a new SoC version/revision.
> The driver should match with the generic device name in that case.
> Until we change the IP and potentially introduce a new driver.
> 
> BTW, even omap3 should not be in the name in theory. It should be
> potentially "ti,i2c-v3" or whatever HW version the IP is using.
> But for some reason, most devices are using such convention in
> existing DTS:-(
> This is probably due to the lack of well identified IP version in
> most SoC... including OMAP:-)
> 
> 
> Benoit
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" 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/omap3-soc.dtsi b/arch/arm/boot/dts/omap3-soc.dtsi
new file mode 100644
index 0000000..85de92f
--- /dev/null
+++ b/arch/arm/boot/dts/omap3-soc.dtsi
@@ -0,0 +1,62 @@ 
+/*
+ * Device Tree Source for OMAP3 SoC
+ *
+ * Copyright (C) 2011 Texas Instruments Incorporated - http://www.ti.com/
+ *
+ * This file is licensed under the terms of the GNU General Public License
+ * version 2.  This program is licensed "as is" without any warranty of any
+ * kind, whether express or implied.
+ */
+
+/dts-v1/;
+/include/ "skeleton.dtsi"
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+	model = "ti,omap3";
+
+	aliases {
+		i2c1 = &i2c1;
+		i2c2 = &i2c2;
+		i2c3 = &i2c3;
+	};
+
+	l4-core {
+		compatible = "ti,omap3-l4-core";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0 0x48000000 0x1000000>;
+
+		i2c1: i2c@70000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "ti,omap3-i2c";
+			reg = <0x70000 0x100>;
+			interrupts = < 88 >;
+		};
+
+		i2c2: i2c@72000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "ti,omap3-i2c";
+			reg = <0x72000 0x100>;
+			interrupts = < 89 >;
+		};
+
+		i2c3: i2c@60000 {
+			#address-cells = <1>;
+			#size-cells = <0>;
+			compatible = "ti,omap3-i2c";
+			reg = <0x60000 0x100>;
+			interrupts = < 93 >;
+		};
+	};
+
+	l4-per {
+		compatible = "ti,l4-per";
+		#address-cells = <1>;
+		#size-cells = <1>;
+		ranges = <0 0x49000000 0x100000>;
+	};
+};