diff mbox

ARM: dts: imx23-olinuxino: Add i2c support

Message ID 1428515100-1496-1-git-send-email-harald@ccbib.org (mailing list archive)
State New, archived
Headers show

Commit Message

Harald Geyer April 8, 2015, 5:45 p.m. UTC
The imx23-olinuxino board has an i2c interface exposed on UEXT connector.
This patch provides the necessary devicetree code.
Tested with MOD-LCD1x9 from Olimex.

This patch is based on work by Fadil Berisha with his permission. However
all bugs are mine.

Signed-off-by: Harald Geyer <harald@ccbib.org>
---
 arch/arm/boot/dts/imx23-olinuxino.dts |    6 +++++
 arch/arm/boot/dts/imx23.dtsi          |   40 ++++++++++++++++++++++++++++++++-
 2 files changed, 45 insertions(+), 1 deletion(-)

Comments

Stefan Wahren April 8, 2015, 10:12 p.m. UTC | #1
Hi Harald,

[ add devicetree mailinglist ]

> Harald Geyer <harald@ccbib.org> hat am 8. April 2015 um 19:45 geschrieben:
>
>
> The imx23-olinuxino board has an i2c interface exposed on UEXT connector.

AFAIK the iMX233-OLinuXino-MICRO don't have a UEXT connector. Maybe we reached
the point 
to make dts files for every Olinuxino board. In that case we could also handle
the USB Host / Peripherial issue.

> This patch provides the necessary devicetree code.
> Tested with MOD-LCD1x9 from Olimex.
>
> This patch is based on work by Fadil Berisha with his permission. However
> all bugs are mine.
>
> Signed-off-by: Harald Geyer <harald@ccbib.org>
> ---
> arch/arm/boot/dts/imx23-olinuxino.dts | 6 +++++
> arch/arm/boot/dts/imx23.dtsi | 40 ++++++++++++++++++++++++++++++++-
> 2 files changed, 45 insertions(+), 1 deletion(-)
>
> [...]
> --- a/arch/arm/boot/dts/imx23.dtsi
> +++ b/arch/arm/boot/dts/imx23.dtsi
> @@ -308,6 +308,39 @@
> fsl,voltage = <MXS_VOLTAGE_HIGH>;
> fsl,pull-up = <MXS_PULL_ENABLE>;
> };
> +
> + i2c_pins_a: i2c@0 {
> + reg = <0>;
> + fsl,pinmux-ids = <
> + MX23_PAD_I2C_SCL__I2C_SCL
> + MX23_PAD_I2C_SDA__I2C_SDA
> + >;
> + fsl,drive-strength = <MXS_DRIVE_8mA>;
> + fsl,voltage = <MXS_VOLTAGE_HIGH>;
> + fsl,pull-up = <MXS_PULL_ENABLE>;
> + };
> +
> + i2c_pins_b: i2c@1 {
> + reg = <1>;
> + fsl,pinmux-ids = <
> + MX23_PAD_LCD_ENABLE__I2C_SCL
> + MX23_PAD_LCD_HSYNC__I2C_SDA
> + >;
> + fsl,drive-strength = <MXS_DRIVE_8mA>;
> + fsl,voltage = <MXS_VOLTAGE_HIGH>;
> + fsl,pull-up = <MXS_PULL_ENABLE>;
> + };
> +
> + i2c_pins_c: i2c@2 {
> + reg = <2>;
> + fsl,pinmux-ids = <
> + MX23_PAD_SSP1_DATA1__I2C_SCL
> + MX23_PAD_SSP1_DATA2__I2C_SDA
> + >;
> + fsl,drive-strength = <MXS_DRIVE_8mA>;
> + fsl,voltage = <MXS_VOLTAGE_HIGH>;
> + fsl,pull-up = <MXS_PULL_ENABLE>;
> + };

Please add only i2c_pins_b because this pin muxing is the only used one.

Thanks Stefan
Harald Geyer April 8, 2015, 10:43 p.m. UTC | #2
Hi Stefan!

On Thu, 9 Apr 2015 00:12:55 +0200 (CEST), Stefan Wahren
<stefan.wahren@i2se.com> wrote:
> Hi Harald,
> 
> [ add devicetree mailinglist ]
> 
>> Harald Geyer <harald@ccbib.org> hat am 8. April 2015 um 19:45
>> geschrieben:
>>
>>
>> The imx23-olinuxino board has an i2c interface exposed on UEXT
connector.
> 
> AFAIK the iMX233-OLinuXino-MICRO don't have a UEXT connector.

Yes, but I don't think having default i2c pins there would be a bad thing.

> Maybe we reached the point to make dts files for every Olinuxino board.
> In that case we could also handle the USB Host / Peripherial issue.

Yes, I'm thinking in this direction. But even if peripherial mode works
on micro/nano (which I haven't been able to confirm yet) that doesn't mean
that it should be set in the default device tree. After all Olimex
themself
advertise the port as host. 

>> This patch provides the necessary devicetree code.
>> Tested with MOD-LCD1x9 from Olimex.
>>
>> This patch is based on work by Fadil Berisha with his permission.
However
>> all bugs are mine.
>>
>> Signed-off-by: Harald Geyer <harald@ccbib.org>
>> ---
>> arch/arm/boot/dts/imx23-olinuxino.dts | 6 +++++
>> arch/arm/boot/dts/imx23.dtsi | 40 ++++++++++++++++++++++++++++++++-
>> 2 files changed, 45 insertions(+), 1 deletion(-)
>>
>> [...]
>> --- a/arch/arm/boot/dts/imx23.dtsi
>> +++ b/arch/arm/boot/dts/imx23.dtsi
>> @@ -308,6 +308,39 @@
>> fsl,voltage = <MXS_VOLTAGE_HIGH>;
>> fsl,pull-up = <MXS_PULL_ENABLE>;
>> };
>> +
>> + i2c_pins_a: i2c@0 {
>> + reg = <0>;
>> + fsl,pinmux-ids = <
>> + MX23_PAD_I2C_SCL__I2C_SCL
>> + MX23_PAD_I2C_SDA__I2C_SDA
>> + >;
>> + fsl,drive-strength = <MXS_DRIVE_8mA>;
>> + fsl,voltage = <MXS_VOLTAGE_HIGH>;
>> + fsl,pull-up = <MXS_PULL_ENABLE>;
>> + };
>> +
>> + i2c_pins_b: i2c@1 {
>> + reg = <1>;
>> + fsl,pinmux-ids = <
>> + MX23_PAD_LCD_ENABLE__I2C_SCL
>> + MX23_PAD_LCD_HSYNC__I2C_SDA
>> + >;
>> + fsl,drive-strength = <MXS_DRIVE_8mA>;
>> + fsl,voltage = <MXS_VOLTAGE_HIGH>;
>> + fsl,pull-up = <MXS_PULL_ENABLE>;
>> + };
>> +
>> + i2c_pins_c: i2c@2 {
>> + reg = <2>;
>> + fsl,pinmux-ids = <
>> + MX23_PAD_SSP1_DATA1__I2C_SCL
>> + MX23_PAD_SSP1_DATA2__I2C_SDA
>> + >;
>> + fsl,drive-strength = <MXS_DRIVE_8mA>;
>> + fsl,voltage = <MXS_VOLTAGE_HIGH>;
>> + fsl,pull-up = <MXS_PULL_ENABLE>;
>> + };
> 
> Please add only i2c_pins_b because this pin muxing is the only used one.

I disagree: Having all possible i2c pin groups documented in devicetree is
a good thing. Also imx23.dtsi is more general then olinuxino - somebody
might produce a imx23 based board where one of the other pin groups makes
more sense to use. Well, even on olinuxino somebody might use i2c_pins_a
if they don't need an UART, but use an LCD.

Thanks,
Harald
Fabio Estevam April 8, 2015, 10:52 p.m. UTC | #3
Hi Stefan,

On Wed, Apr 8, 2015 at 7:12 PM, Stefan Wahren <stefan.wahren@i2se.com> wrote:

> AFAIK the iMX233-OLinuXino-MICRO don't have a UEXT connector. Maybe we reached
> the point
> to make dts files for every Olinuxino board. In that case we could also handle
> the USB Host / Peripherial issue.

Yes, I agree. it would be better to split the dts files for the
olinuxino variants.

Regards,

Fabio Estevam
Stefan Wahren April 9, 2015, 9:20 a.m. UTC | #4
Hi Harald,

>>> This patch provides the necessary devicetree code.
>>> Tested with MOD-LCD1x9 from Olimex.
>>>
>>> This patch is based on work by Fadil Berisha with his permission.
> However
>>> all bugs are mine.
>>>
>>> Signed-off-by: Harald Geyer <harald@ccbib.org>
>>> ---
>>> arch/arm/boot/dts/imx23-olinuxino.dts | 6 +++++
>>> arch/arm/boot/dts/imx23.dtsi | 40 ++++++++++++++++++++++++++++++++-
>>> 2 files changed, 45 insertions(+), 1 deletion(-)
>>>
>>> [...]
>>> --- a/arch/arm/boot/dts/imx23.dtsi
>>> +++ b/arch/arm/boot/dts/imx23.dtsi
>>> @@ -308,6 +308,39 @@
>>> fsl,voltage = <MXS_VOLTAGE_HIGH>;
>>> fsl,pull-up = <MXS_PULL_ENABLE>;
>>> };
>>> +
>>> + i2c_pins_a: i2c@0 {
>>> + reg = <0>;
>>> + fsl,pinmux-ids = <
>>> + MX23_PAD_I2C_SCL__I2C_SCL
>>> + MX23_PAD_I2C_SDA__I2C_SDA
>>> + >;
>>> + fsl,drive-strength = <MXS_DRIVE_8mA>;
>>> + fsl,voltage = <MXS_VOLTAGE_HIGH>;
>>> + fsl,pull-up = <MXS_PULL_ENABLE>;
>>> + };
>>> +
>>> + i2c_pins_b: i2c@1 {
>>> + reg = <1>;
>>> + fsl,pinmux-ids = <
>>> + MX23_PAD_LCD_ENABLE__I2C_SCL
>>> + MX23_PAD_LCD_HSYNC__I2C_SDA
>>> + >;
>>> + fsl,drive-strength = <MXS_DRIVE_8mA>;
>>> + fsl,voltage = <MXS_VOLTAGE_HIGH>;
>>> + fsl,pull-up = <MXS_PULL_ENABLE>;
>>> + };
>>> +
>>> + i2c_pins_c: i2c@2 {
>>> + reg = <2>;
>>> + fsl,pinmux-ids = <
>>> + MX23_PAD_SSP1_DATA1__I2C_SCL
>>> + MX23_PAD_SSP1_DATA2__I2C_SDA
>>> + >;
>>> + fsl,drive-strength = <MXS_DRIVE_8mA>;
>>> + fsl,voltage = <MXS_VOLTAGE_HIGH>;
>>> + fsl,pull-up = <MXS_PULL_ENABLE>;
>>> + };
>> Please add only i2c_pins_b because this pin muxing is the only used one.
> I disagree: Having all possible i2c pin groups documented in devicetree is
> a good thing. Also imx23.dtsi is more general then olinuxino - somebody
> might produce a imx23 based board where one of the other pin groups makes
> more sense to use. Well, even on olinuxino somebody might use i2c_pins_a
> if they don't need an UART, but use an LCD.

Sure. But this 2 additional muxes have nothing to do with adding i2c
support to olinuxino and should be a separate patch. In case of a revert
of this patch the other i.MX23 boards are also affected. So i think it
should be a separate patch.

Btw i read that Shawn only want used muxes [1].

[1] -
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-August/115779.html

> Thanks,
> Harald
Harald Geyer April 9, 2015, 1:03 p.m. UTC | #5
Hi Stefan!

On Thu, 09 Apr 2015 11:20:10 +0200, Stefan Wahren <stefan.wahren@i2se.com>
wrote:
> Hi Harald,
> 
>>>> This patch provides the necessary devicetree code.
>>>> Tested with MOD-LCD1x9 from Olimex.
>>>>
>>>> This patch is based on work by Fadil Berisha with his permission.
>> However
>>>> all bugs are mine.
>>>>
>>>> Signed-off-by: Harald Geyer <harald@ccbib.org>
>>>> ---
>>>> arch/arm/boot/dts/imx23-olinuxino.dts | 6 +++++
>>>> arch/arm/boot/dts/imx23.dtsi | 40 ++++++++++++++++++++++++++++++++-
>>>> 2 files changed, 45 insertions(+), 1 deletion(-)
>>>>
>>>> [...]
>>>> --- a/arch/arm/boot/dts/imx23.dtsi
>>>> +++ b/arch/arm/boot/dts/imx23.dtsi
>>>> @@ -308,6 +308,39 @@
>>>> fsl,voltage = <MXS_VOLTAGE_HIGH>;
>>>> fsl,pull-up = <MXS_PULL_ENABLE>;
>>>> };
>>>> +
>>>> + i2c_pins_a: i2c@0 {
>>>> + reg = <0>;
>>>> + fsl,pinmux-ids = <
>>>> + MX23_PAD_I2C_SCL__I2C_SCL
>>>> + MX23_PAD_I2C_SDA__I2C_SDA
>>>> + >;
>>>> + fsl,drive-strength = <MXS_DRIVE_8mA>;
>>>> + fsl,voltage = <MXS_VOLTAGE_HIGH>;
>>>> + fsl,pull-up = <MXS_PULL_ENABLE>;
>>>> + };
>>>> +
>>>> + i2c_pins_b: i2c@1 {
>>>> + reg = <1>;
>>>> + fsl,pinmux-ids = <
>>>> + MX23_PAD_LCD_ENABLE__I2C_SCL
>>>> + MX23_PAD_LCD_HSYNC__I2C_SDA
>>>> + >;
>>>> + fsl,drive-strength = <MXS_DRIVE_8mA>;
>>>> + fsl,voltage = <MXS_VOLTAGE_HIGH>;
>>>> + fsl,pull-up = <MXS_PULL_ENABLE>;
>>>> + };
>>>> +
>>>> + i2c_pins_c: i2c@2 {
>>>> + reg = <2>;
>>>> + fsl,pinmux-ids = <
>>>> + MX23_PAD_SSP1_DATA1__I2C_SCL
>>>> + MX23_PAD_SSP1_DATA2__I2C_SDA
>>>> + >;
>>>> + fsl,drive-strength = <MXS_DRIVE_8mA>;
>>>> + fsl,voltage = <MXS_VOLTAGE_HIGH>;
>>>> + fsl,pull-up = <MXS_PULL_ENABLE>;
>>>> + };
>>> Please add only i2c_pins_b because this pin muxing is the only used
one.
>> I disagree: Having all possible i2c pin groups documented in devicetree
>> is
>> a good thing. Also imx23.dtsi is more general then olinuxino - somebody
>> might produce a imx23 based board where one of the other pin groups
makes
>> more sense to use. Well, even on olinuxino somebody might use
i2c_pins_a
>> if they don't need an UART, but use an LCD.
> 
> Sure. But this 2 additional muxes have nothing to do with adding i2c
> support to olinuxino and should be a separate patch. In case of a revert
> of this patch the other i.MX23 boards are also affected. So i think it
> should be a separate patch.

If it is prefered, I will split the changes to imx23.dtsi and
imx23-olinuxino.dts into two patches.

> Btw i read that Shawn only want used muxes [1].
> 
> [1] -
>
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-August/115779.html

If that is an issue, then I'll just test all three pin groups on my
olinuxino
board. After all it would be really awkward to have only one pin group and
that
one is labeled "i2c_pins_b" ...

Thanks,
Harald
Stefan Wahren April 9, 2015, 4:34 p.m. UTC | #6
Hi Harald,

> harald@ccbib.org hat am 9. April 2015 um 15:03 geschrieben:
>
>
> Hi Stefan!
>
> On Thu, 09 Apr 2015 11:20:10 +0200, Stefan Wahren <stefan.wahren@i2se.com>
> wrote:
>
> > Btw i read that Shawn only want used muxes [1].
> >
> > [1] -
> >
> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-August/115779.html
>
> If that is an issue, then I'll just test all three pin groups on my
> olinuxino
> board. After all it would be really awkward to have only one pin group and
> that
> one is labeled "i2c_pins_b" ...

from my understanding you are free about the labels. So you could rename it to
"i2c_pins_a" and change the other pin groups.

>
> Thanks,
> Harald

Stefan
Harald Geyer April 10, 2015, 5:37 p.m. UTC | #7
Stefan Wahren writes:
> > On Thu, 09 Apr 2015 11:20:10 +0200, Stefan Wahren <stefan.wahren@i2se.com>
> > wrote:
> >
> > > Btw i read that Shawn only want used muxes [1].
> > >
> > > [1] -
> > >
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2012-August/115779.html
> >
> > If that is an issue, then I'll just test all three pin groups on my
> > olinuxino
> > board. After all it would be really awkward to have only one pin group and
> > that
> > one is labeled "i2c_pins_b" ...
> 
> from my understanding you are free about the labels. So you could
> rename it to "i2c_pins_a" and change the other pin groups.

Theoretically yes, but there are two reasons not to do that:
1) When Fadil Berisha chose the labels he didn't do so arbitrarily. The
   labels make sense the way the pinmux table is organised in the datasheet.
   (otherwise he sure would have chosen "i2c_pins_a" for the default pin
   group on olinuxino).
2) The labels are in use for about two years now by a lot of people. (Yes
   it really takes that long to have this merged.) I would hate to break
   their setups for no strong reason.

Harald
diff mbox

Patch

diff --git a/arch/arm/boot/dts/imx23-olinuxino.dts b/arch/arm/boot/dts/imx23-olinuxino.dts
index 7e6eef2..85718df 100644
--- a/arch/arm/boot/dts/imx23-olinuxino.dts
+++ b/arch/arm/boot/dts/imx23-olinuxino.dts
@@ -73,6 +73,12 @@ 
 				status = "okay";
 			};
 
+			i2c: i2c@80058000 {
+				pinctrl-names = "default";
+				pinctrl-0 = <&i2c_pins_b>;
+				status = "okay";
+			};
+
 			duart: serial@80070000 {
 				pinctrl-names = "default";
 				pinctrl-0 = <&duart_pins_a>;
diff --git a/arch/arm/boot/dts/imx23.dtsi b/arch/arm/boot/dts/imx23.dtsi
index bbcfb5a..c892d58 100644
--- a/arch/arm/boot/dts/imx23.dtsi
+++ b/arch/arm/boot/dts/imx23.dtsi
@@ -308,6 +308,39 @@ 
 					fsl,voltage = <MXS_VOLTAGE_HIGH>;
 					fsl,pull-up = <MXS_PULL_ENABLE>;
 				};
+
+				i2c_pins_a: i2c@0 {
+					reg = <0>;
+					fsl,pinmux-ids = <
+						MX23_PAD_I2C_SCL__I2C_SCL
+						MX23_PAD_I2C_SDA__I2C_SDA
+					>;
+					fsl,drive-strength = <MXS_DRIVE_8mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_ENABLE>;
+				};
+
+				i2c_pins_b: i2c@1 {
+					reg = <1>;
+					fsl,pinmux-ids = <
+						MX23_PAD_LCD_ENABLE__I2C_SCL
+						MX23_PAD_LCD_HSYNC__I2C_SDA
+					>;
+					fsl,drive-strength = <MXS_DRIVE_8mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_ENABLE>;
+				};
+
+				i2c_pins_c: i2c@2 {
+					reg = <2>;
+					fsl,pinmux-ids = <
+						MX23_PAD_SSP1_DATA1__I2C_SCL
+						MX23_PAD_SSP1_DATA2__I2C_SDA
+					>;
+					fsl,drive-strength = <MXS_DRIVE_8mA>;
+					fsl,voltage = <MXS_VOLTAGE_HIGH>;
+					fsl,pull-up = <MXS_PULL_ENABLE>;
+				};
 			};
 
 			digctl@8001c000 {
@@ -444,8 +477,13 @@ 
 				status = "disabled";
 			};
 
-			i2c@80058000 {
+			i2c: i2c@80058000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "fsl,imx23-i2c";
 				reg = <0x80058000 0x2000>;
+				interrupts = <27>;
+				clock-frequency = <100000>;
 				dmas = <&dma_apbx 3>;
 				dma-names = "rx-tx";
 				status = "disabled";