Re: [PATCH] ARM: dts: imx23-olinuxino: Add i2c support

Re: [PATCH] ARM: dts: imx23-olinuxino: Add i2c support

[Stefan Wahren]

Hi Harald,

[ add devicetree mailinglist ]

> Harald Geyer <harald-95f8Dae0BrPYtjvyW6yDsg@public.gmane.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-95f8Dae0BrPYtjvyW6yDsg@public.gmane.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
--
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] ARM: dts: imx23-olinuxino: Add i2c support

[harald]

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

Re: [PATCH] ARM: dts: imx23-olinuxino: Add i2c support

[Stefan Wahren]

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-95f8Dae0BrPYtjvyW6yDsg@public.gmane.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

--
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] ARM: dts: imx23-olinuxino: Add i2c support

[harald-95f8Dae0BrPYtjvyW6yDsg]

Hi Stefan!

On Thu, 09 Apr 2015 11:20:10 +0200, Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
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-95f8Dae0BrPYtjvyW6yDsg@public.gmane.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
--
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] ARM: dts: imx23-olinuxino: Add i2c support

[Stefan Wahren]

Hi Harald,

> harald-95f8Dae0BrPYtjvyW6yDsg@public.gmane.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-eS4NqCHxEME@public.gmane.org>
> 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
--
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] ARM: dts: imx23-olinuxino: Add i2c support

[Harald Geyer]

Stefan Wahren writes:
> > On Thu, 09 Apr 2015 11:20:10 +0200, Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> > 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
--
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] ARM: dts: imx23-olinuxino: Add i2c support

[Fabio Estevam]

Hi Stefan,

On Wed, Apr 8, 2015 at 7:12 PM, Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> 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
--
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