| Message ID | 1372094699-3832-3-git-send-email-alexandre.belloni@free-electrons.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Alexandre, On Mon, Jun 24, 2013 at 2:24 PM, Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote: > From: Maxime Ripard <maxime.ripard@free-electrons.com> > > The ADCs connected to this bus have been experiencing some timeout > issues when using the iMX28 i2c controller. Switching back to bitbanging > solves this. Are you able to use the mxs i2c controller instead of bitbang if you use this patch? http://www.spinics.net/lists/stable/msg13202.html Regards, Fabio Estevam
Hi, On 02/07/2013 04:45, Fabio Estevam wrote: > Are you able to use the mxs i2c controller instead of bitbang if you > use this patch? > http://www.spinics.net/lists/stable/msg13202.html > Actually, it gets worse. I'm probably doing something wrong but I get: mxs-i2c 8005a000.i2c: Failed to get PIO reg. write descriptor. That is the one line 243. I'm open to any suggestion. BTW, I also tested PIO mode with and without touchscreen support as that seemed to toggle the issue on your side. It is also worse than what it was in 3.9. Trying to read the nau7802 ADC never returns. The excerpt from my DT: i2c1: i2c@8005a000 { pinctrl-names = "default"; pinctrl-0 = <&i2c1_pins_a>; status = "okay"; }; i2cmux { compatible = "i2c-mux-gpio"; #address-cells = <1>; #size-cells = <0>; pinctrl-names = "default"; pinctrl-0 = <&i2cmux_pins_cfa10049>; mux-gpios = <&gpio1 22 0 &gpio1 23 0>; i2c-parent = <&i2c1>; i2c@0 { reg = <0>; #address-cells = <1>; #size-cells = <0>; adc0: nau7802@2a { compatible = "nuvoton,nau7802"; reg = <0x2a>; nuvoton,vldo = <3000>; }; }; [...]
On Tue, Jul 2, 2013 at 8:35 AM, Alexandre Belloni <alexandre.belloni@free-electrons.com> wrote: > mxs-i2c 8005a000.i2c: Failed to get PIO reg. write descriptor. > > That is the one line 243. I'm open to any suggestion. > > BTW, I also tested PIO mode with and without touchscreen support as that > seemed to toggle the issue on your side. It is also worse than what it > was in 3.9. Trying to read the nau7802 ADC never returns. > > The excerpt from my DT: > > i2c1: i2c@8005a000 { > pinctrl-names = "default"; > pinctrl-0 = <&i2c1_pins_a>; > status = "okay"; > }; > > i2cmux { > compatible = "i2c-mux-gpio"; > #address-cells = <1>; > #size-cells = <0>; > pinctrl-names = "default"; > pinctrl-0 = <&i2cmux_pins_cfa10049>; > mux-gpios = <&gpio1 22 0 &gpio1 23 0>; > i2c-parent = <&i2c1>; > > i2c@0 { > reg = <0>; Shouldn't this be i2c@1 { reg = <1>; ?
On 02/07/2013 13:45, Fabio Estevam wrote: > Shouldn't this be > > i2c@1 { > reg = <1>; ? > No, we have 4 devices on that mux and 2 pins to select the muxing.
On 02/07/2013 13:50, Alexandre Belloni wrote: > On 02/07/2013 13:45, Fabio Estevam wrote: > >> Shouldn't this be >> >> i2c@1 { >> reg = <1>; ? >> > > No, we have 4 devices on that mux and 2 pins to select the muxing. > OK, got it working. So, the results: bitbanging: # time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw 2637 real 0m 0.09s user 0m 0.01s sys 0m 0.01s i2c-mxs PIO mode: # time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw [ 35.007650] [sched_delayed] sched: RT throttling activated 2627 real 0m 7.14s user 0m 0.02s sys 0m 0.01s i2c-mxs PIO mode without LRADC: # time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw [ 18.007432] [sched_delayed] sched: RT throttling activated 2629 real 0m 7.09s user 0m 0.00s sys 0m 0.03s i2c-mxs DMA mode: # time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw 2631 real 0m 0.12s user 0m 0.01s sys 0m 0.01s It seems fine for me.
Dear Alexandre Belloni, > On 02/07/2013 13:50, Alexandre Belloni wrote: > > On 02/07/2013 13:45, Fabio Estevam wrote: > >> Shouldn't this be > >> > >> i2c@1 { > >> > >> reg = <1>; ? > > > > No, we have 4 devices on that mux and 2 pins to select the muxing. > > OK, got it working. > > So, the results: > > bitbanging: > > # time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw > 2637 > real 0m 0.09s > user 0m 0.01s > sys 0m 0.01s > > > i2c-mxs PIO mode: > > # time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw > [ 35.007650] [sched_delayed] sched: RT throttling activated > 2627 > real 0m 7.14s > user 0m 0.02s > sys 0m 0.01s > > > i2c-mxs PIO mode without LRADC: > > # time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw > [ 18.007432] [sched_delayed] sched: RT throttling activated > 2629 > real 0m 7.09s > user 0m 0.00s > sys 0m 0.03s > > > i2c-mxs DMA mode: > > # time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw > 2631 > real 0m 0.12s > user 0m 0.01s > sys 0m 0.01s > > > It seems fine for me. I think I'm getting a little lost in these gazilions of i2c and lradc threads. Can we not create one thread and keep the related stuff in there instead of discussing it all around !? Only one question comes to mind with this email -- what do LRADC and I2C have to do with each other here ? It'd be nice if someone could summarize on what I should focus and possibly prepare a testcase. Best regards, Marek Vasut
On 02/07/2013 18:33, Marek Vasut wrote: > Dear Alexandre Belloni, > >> On 02/07/2013 13:50, Alexandre Belloni wrote: >>> On 02/07/2013 13:45, Fabio Estevam wrote: >>>> Shouldn't this be >>>> >>>> i2c@1 { >>>> >>>> reg = <1>; ? >>> >>> No, we have 4 devices on that mux and 2 pins to select the muxing. >> >> OK, got it working. >> >> So, the results: >> >> bitbanging: >> >> # time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw >> 2637 >> real 0m 0.09s >> user 0m 0.01s >> sys 0m 0.01s >> >> >> i2c-mxs PIO mode: >> >> # time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw >> [ 35.007650] [sched_delayed] sched: RT throttling activated >> 2627 >> real 0m 7.14s >> user 0m 0.02s >> sys 0m 0.01s >> >> >> i2c-mxs PIO mode without LRADC: >> >> # time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw >> [ 18.007432] [sched_delayed] sched: RT throttling activated >> 2629 >> real 0m 7.09s >> user 0m 0.00s >> sys 0m 0.03s >> >> >> i2c-mxs DMA mode: >> >> # time cat /sys/bus/iio/devices/iio\:device1/in_voltage0_raw >> 2631 >> real 0m 0.12s >> user 0m 0.01s >> sys 0m 0.01s >> >> >> It seems fine for me. > > I think I'm getting a little lost in these gazilions of i2c and lradc threads. > Can we not create one thread and keep the related stuff in there instead of > discussing it all around !? > > Only one question comes to mind with this email -- what do LRADC and I2C have to > do with each other here ? > Yeah, sorry, I meant the lradc touchscreen support. This seemed to trigger the issue for Fabio but as my testing shows, this is not the case for me, I get the issue with PIO, whether the lradc touchscreen support is activated or not. I think Torsten is the one that investigated it the most : http://www.spinics.net/lists/linux-i2c/msg12619.html > It'd be nice if someone could summarize on what I should focus and possibly > prepare a testcase. > On my setup, it happens on every i2c read that are done in PIO mode. But, my setup may be a bit unconventional as we are using a i2c gpio muxer. Regards,
On 06/24/2013 06:24 PM, Alexandre Belloni wrote: > From: Maxime Ripard <maxime.ripard@free-electrons.com> > > The ADCs connected to this bus have been experiencing some timeout > issues when using the iMX28 i2c controller. Switching back to bitbanging > solves this. > > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> > Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> As there are no disadvantages in taking the driver through IIO and these changes through the appropriate arch trees, I'd not propose to take these through IIO (even when the discussion is done) unless specifically asked to. Jonathan > --- > arch/arm/boot/dts/imx28-cfa10049.dts | 108 +++++++++++++++++++++-------------- > 1 file changed, 65 insertions(+), 43 deletions(-) > > diff --git a/arch/arm/boot/dts/imx28-cfa10049.dts b/arch/arm/boot/dts/imx28-cfa10049.dts > index 05ae549..d3758c2 100644 > --- a/arch/arm/boot/dts/imx28-cfa10049.dts > +++ b/arch/arm/boot/dts/imx28-cfa10049.dts > @@ -139,6 +139,17 @@ > fsl,pull-up = <0>; /* 0 will enable the keeper */ > }; > > + i2c1_pins_cfa10049: i2c1@0 { > + reg = <0>; > + fsl,pinmux-ids = < > + 0x3103 /* MX28_PAD_PWM0__GPIO */ > + 0x3113 /* MX28_PAD_PWM1__I2C1_SDA */ > + >; > + fsl,drive-strength = <1>; > + fsl,voltage = <1>; > + fsl,pull-up = <1>; > + }; > + > fiq_pins_cfa10049: fiq@0 { > reg = <0>; > fsl,pinmux-ids = < > @@ -199,49 +210,6 @@ > status = "okay"; > }; > > - i2c1: i2c@8005a000 { > - pinctrl-names = "default"; > - pinctrl-0 = <&i2c1_pins_a>; > - status = "okay"; > - }; > - > - i2cmux { > - compatible = "i2c-mux-gpio"; > - #address-cells = <1>; > - #size-cells = <0>; > - mux-gpios = <&gpio1 22 0 &gpio1 23 0>; > - i2c-parent = <&i2c1>; > - > - i2c@0 { > - reg = <0>; > - }; > - > - i2c@1 { > - reg = <1>; > - }; > - > - i2c@2 { > - reg = <2>; > - }; > - > - i2c@3 { > - reg = <3>; > - #address-cells = <1>; > - #size-cells = <0>; > - > - pca9555: pca9555@20 { > - compatible = "nxp,pca9555"; > - interrupt-parent = <&gpio2>; > - interrupts = <19 0x2>; > - gpio-controller; > - #gpio-cells = <2>; > - interrupt-controller; > - #interrupt-cells = <2>; > - reg = <0x20>; > - }; > - }; > - }; > - > usbphy1: usbphy@8007e000 { > status = "okay"; > }; > @@ -366,6 +334,60 @@ > rotary-encoder,relative-axis; > }; > > + i2c1gpio: i2c@0 { > + compatible = "i2c-gpio"; > + pinctrl-0 = <&i2c1_pins_cfa10049>; > + pinctrl-names = "default"; > + gpios = < > + &gpio3 17 0 /* sda */ > + &gpio3 16 0 /* scl */ > + >; > + i2c-gpio,delay-us = <2>; /* ~100 kHz */ > + }; > + > + i2cmux { > + compatible = "i2c-mux-gpio"; > + #address-cells = <1>; > + #size-cells = <0>; > + mux-gpios = <&gpio1 22 0 &gpio1 23 0>; > + i2c-parent = <&i2c1gpio>; > + > + i2c@0 { > + reg = <0>; > + #address-cells = <1>; > + #size-cells = <0>; > + }; > + > + i2c@1 { > + reg = <1>; > + #address-cells = <1>; > + #size-cells = <0>; > + }; > + > + i2c@2 { > + reg = <2>; > + #address-cells = <1>; > + #size-cells = <0>; > + }; > + > + i2c@3 { > + reg = <3>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + pca9555: pca9555@20 { > + compatible = "nxp,pca9555"; > + interrupt-parent = <&gpio2>; > + interrupts = <19 0x2>; > + gpio-controller; > + #gpio-cells = <2>; > + interrupt-controller; > + #interrupt-cells = <2>; > + reg = <0x20>; > + }; > + }; > + }; > + > backlight { > compatible = "pwm-backlight"; > pwms = <&pwm 3 5000000>; >
On 06/07/2013 12:26, Jonathan Cameron wrote: > On 06/24/2013 06:24 PM, Alexandre Belloni wrote: >> From: Maxime Ripard <maxime.ripard@free-electrons.com> >> >> The ADCs connected to this bus have been experiencing some timeout >> issues when using the iMX28 i2c controller. Switching back to bitbanging >> solves this. >> >> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com> >> Signed-off-by: Alexandre Belloni <alexandre.belloni@free-electrons.com> > > As there are no disadvantages in taking the driver through IIO and these changes > through the appropriate arch trees, I'd not propose to take these through IIO > (even when the discussion is done) unless specifically asked to. > Sure, especially since, we may finally not move to i2c bitbanging. I'll probably resend a new version of those patches separately. Shawn, please wait for the i2c-mxs PIO/DMA mode discussion to end before applying. If possible, I would prefer not using bitbanging. Regards,
diff --git a/arch/arm/boot/dts/imx28-cfa10049.dts b/arch/arm/boot/dts/imx28-cfa10049.dts index 05ae549..d3758c2 100644 --- a/arch/arm/boot/dts/imx28-cfa10049.dts +++ b/arch/arm/boot/dts/imx28-cfa10049.dts @@ -139,6 +139,17 @@ fsl,pull-up = <0>; /* 0 will enable the keeper */ }; + i2c1_pins_cfa10049: i2c1@0 { + reg = <0>; + fsl,pinmux-ids = < + 0x3103 /* MX28_PAD_PWM0__GPIO */ + 0x3113 /* MX28_PAD_PWM1__I2C1_SDA */ + >; + fsl,drive-strength = <1>; + fsl,voltage = <1>; + fsl,pull-up = <1>; + }; + fiq_pins_cfa10049: fiq@0 { reg = <0>; fsl,pinmux-ids = < @@ -199,49 +210,6 @@ status = "okay"; }; - i2c1: i2c@8005a000 { - pinctrl-names = "default"; - pinctrl-0 = <&i2c1_pins_a>; - status = "okay"; - }; - - i2cmux { - compatible = "i2c-mux-gpio"; - #address-cells = <1>; - #size-cells = <0>; - mux-gpios = <&gpio1 22 0 &gpio1 23 0>; - i2c-parent = <&i2c1>; - - i2c@0 { - reg = <0>; - }; - - i2c@1 { - reg = <1>; - }; - - i2c@2 { - reg = <2>; - }; - - i2c@3 { - reg = <3>; - #address-cells = <1>; - #size-cells = <0>; - - pca9555: pca9555@20 { - compatible = "nxp,pca9555"; - interrupt-parent = <&gpio2>; - interrupts = <19 0x2>; - gpio-controller; - #gpio-cells = <2>; - interrupt-controller; - #interrupt-cells = <2>; - reg = <0x20>; - }; - }; - }; - usbphy1: usbphy@8007e000 { status = "okay"; }; @@ -366,6 +334,60 @@ rotary-encoder,relative-axis; }; + i2c1gpio: i2c@0 { + compatible = "i2c-gpio"; + pinctrl-0 = <&i2c1_pins_cfa10049>; + pinctrl-names = "default"; + gpios = < + &gpio3 17 0 /* sda */ + &gpio3 16 0 /* scl */ + >; + i2c-gpio,delay-us = <2>; /* ~100 kHz */ + }; + + i2cmux { + compatible = "i2c-mux-gpio"; + #address-cells = <1>; + #size-cells = <0>; + mux-gpios = <&gpio1 22 0 &gpio1 23 0>; + i2c-parent = <&i2c1gpio>; + + i2c@0 { + reg = <0>; + #address-cells = <1>; + #size-cells = <0>; + }; + + i2c@1 { + reg = <1>; + #address-cells = <1>; + #size-cells = <0>; + }; + + i2c@2 { + reg = <2>; + #address-cells = <1>; + #size-cells = <0>; + }; + + i2c@3 { + reg = <3>; + #address-cells = <1>; + #size-cells = <0>; + + pca9555: pca9555@20 { + compatible = "nxp,pca9555"; + interrupt-parent = <&gpio2>; + interrupts = <19 0x2>; + gpio-controller; + #gpio-cells = <2>; + interrupt-controller; + #interrupt-cells = <2>; + reg = <0x20>; + }; + }; + }; + backlight { compatible = "pwm-backlight"; pwms = <&pwm 3 5000000>;