[PATCH 1/2] ARM: dts: imx: Remove custom dma-channel bindings

[PATCH 1/2] ARM: dts: imx: Remove custom dma-channel bindings

[Fabio Estevam]

Commit f30fb03 (ARM: dts: add generic DMA device tree binding for mxs-dma) added
the generic DMA dt bindings, but missed to remove the old ones.

Remove the the old dma bindings.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 arch/arm/boot/dts/imx23.dtsi   | 3 ---
 arch/arm/boot/dts/imx28.dtsi   | 5 -----
 arch/arm/boot/dts/imx6qdl.dtsi | 1 -
 3 files changed, 9 deletions(-)

diff --git a/arch/arm/boot/dts/imx23.dtsi b/arch/arm/boot/dts/imx23.dtsi
index 73fd7d0..be93494 100644
--- a/arch/arm/boot/dts/imx23.dtsi
+++ b/arch/arm/boot/dts/imx23.dtsi
@@ -78,7 +78,6 @@
 				clock-names = "gpmi_io";
 				dmas = <&dma_apbh 4>;
 				dma-names = "rx-tx";
-				fsl,gpmi-dma-channel = <4>;
 				status = "disabled";
 			};
 
@@ -88,7 +87,6 @@
 				clocks = <&clks 33>;
 				dmas = <&dma_apbh 1>;
 				dma-names = "rx-tx";
-				fsl,ssp-dma-channel = <1>;
 				status = "disabled";
 			};
 
@@ -366,7 +364,6 @@
 				clocks = <&clks 33>;
 				dmas = <&dma_apbh 2>;
 				dma-names = "rx-tx";
-				fsl,ssp-dma-channel = <2>;
 				status = "disabled";
 			};
 
diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index e401dee..33389b8 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -100,7 +100,6 @@
 				clock-names = "gpmi_io";
 				dmas = <&dma_apbh 4>;
 				dma-names = "rx-tx";
-				fsl,gpmi-dma-channel = <4>;
 				status = "disabled";
 			};
 
@@ -112,7 +111,6 @@
 				clocks = <&clks 46>;
 				dmas = <&dma_apbh 0>;
 				dma-names = "rx-tx";
-				fsl,ssp-dma-channel = <0>;
 				status = "disabled";
 			};
 
@@ -124,7 +122,6 @@
 				clocks = <&clks 47>;
 				dmas = <&dma_apbh 1>;
 				dma-names = "rx-tx";
-				fsl,ssp-dma-channel = <1>;
 				status = "disabled";
 			};
 
@@ -136,7 +133,6 @@
 				clocks = <&clks 48>;
 				dmas = <&dma_apbh 2>;
 				dma-names = "rx-tx";
-				fsl,ssp-dma-channel = <2>;
 				status = "disabled";
 			};
 
@@ -148,7 +144,6 @@
 				clocks = <&clks 49>;
 				dmas = <&dma_apbh 3>;
 				dma-names = "rx-tx";
-				fsl,ssp-dma-channel = <3>;
 				status = "disabled";
 			};
 
diff --git a/arch/arm/boot/dts/imx6qdl.dtsi b/arch/arm/boot/dts/imx6qdl.dtsi
index 42e461c..5cc2855 100644
--- a/arch/arm/boot/dts/imx6qdl.dtsi
+++ b/arch/arm/boot/dts/imx6qdl.dtsi
@@ -89,7 +89,6 @@
 				      "gpmi_bch_apb", "per1_bch";
 			dmas = <&dma_apbh 0>;
 			dma-names = "rx-tx";
-			fsl,gpmi-dma-channel = <0>;
 			status = "disabled";
 		};
 
-- 
1.8.1.2

[PATCH 2/2] ARM: dts: imx28: Adjust i2c interrupt bindings

[Fabio Estevam]

Commit e5aba13 (i2c: i2c-mxs: move to use generic DMA helper) changed
i2c-mxs.txt and drivers/i2c/busses/i2c-mxs.c to only use one I2C interrupt.

Adapt the dtsi bindings with such change.

Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
---
 arch/arm/boot/dts/imx28.dtsi | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
index 33389b8..2e1ba4b 100644
--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -878,7 +878,7 @@
 				#size-cells = <0>;
 				compatible = "fsl,imx28-i2c";
 				reg = <0x80058000 0x2000>;
-				interrupts = <111 68>;
+				interrupts = <111>;
 				clock-frequency = <100000>;
 				dmas = <&dma_apbx 6>;
 				dma-names = "rx-tx";
@@ -891,7 +891,7 @@
 				#size-cells = <0>;
 				compatible = "fsl,imx28-i2c";
 				reg = <0x8005a000 0x2000>;
-				interrupts = <110 69>;
+				interrupts = <110>;
 				clock-frequency = <100000>;
 				dmas = <&dma_apbx 7>;
 				dma-names = "rx-tx";
-- 
1.8.1.2

[PATCH 2/2] ARM: dts: imx28: Adjust i2c interrupt bindings

[Fabio Estevam]

On Thu, May 16, 2013 at 11:13 AM, Fabio Estevam
<fabio.estevam@freescale.com> wrote:
> Commit e5aba13 (i2c: i2c-mxs: move to use generic DMA helper) changed
> i2c-mxs.txt and drivers/i2c/busses/i2c-mxs.c to only use one I2C interrupt.
>
> Adapt the dtsi bindings with such change.
>
> Signed-off-by: Fabio Estevam <fabio.estevam@freescale.com>
> ---
>  arch/arm/boot/dts/imx28.dtsi | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/boot/dts/imx28.dtsi b/arch/arm/boot/dts/imx28.dtsi
> index 33389b8..2e1ba4b 100644
> --- a/arch/arm/boot/dts/imx28.dtsi
> +++ b/arch/arm/boot/dts/imx28.dtsi
> @@ -878,7 +878,7 @@
>                                 #size-cells = <0>;
>                                 compatible = "fsl,imx28-i2c";
>                                 reg = <0x80058000 0x2000>;
> -                               interrupts = <111 68>;
> +                               interrupts = <111>;
>                                 clock-frequency = <100000>;
>                                 dmas = <&dma_apbx 6>;
>                                 dma-names = "rx-tx";
> @@ -891,7 +891,7 @@
>                                 #size-cells = <0>;
>                                 compatible = "fsl,imx28-i2c";
>                                 reg = <0x8005a000 0x2000>;
> -                               interrupts = <110 69>;
> +                               interrupts = <110>;
>                                 clock-frequency = <100000>;
>                                 dmas = <&dma_apbx 7>;
>                                 dma-names = "rx-tx";

Actually the correct is:

--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -878,7 +878,7 @@
 				#size-cells = <0>;
 				compatible = "fsl,imx28-i2c";
 				reg = <0x80058000 0x2000>;
-				interrupts = <111 68>;
+				interrupts = <68>;
 				clock-frequency = <100000>;
 				dmas = <&dma_apbx 6>;
 				dma-names = "rx-tx";
@@ -891,7 +891,7 @@
 				#size-cells = <0>;
 				compatible = "fsl,imx28-i2c";
 				reg = <0x8005a000 0x2000>;
-				interrupts = <110 69>;
+				interrupts = <69>;
 				clock-frequency = <100000>;
 				dmas = <&dma_apbx 7>;
 				dma-names = "rx-tx";

,but then i2c fails to probe.

Has anyone tried mxs i2c dma recently?

Here is what I do on linux-next-20130516:

- Revert  b871f1ad3c8a1ac2 (will send this later to alsa-devel)
- Booting mx28evk will show:

[    6.378298]  mmcblk0: p1 p2 p3
[   29.547791] sgtl5000 0-000a: Failed to get supply 'VDDD': -19
[   29.556308] 0-000a: 1200 mV normal
[   29.562225] sgtl5000 0-000a: Using internal LDO instead of VDDD
[   29.573156] sgtl5000 0-000a: sgtl5000 revision 0x11

,ie, i2c is too slow as it takes 13 seconds to read the sgtl5000 codec
registers. Looks like i2c dma is disabled

Then I apply

--- a/arch/arm/boot/dts/imx28.dtsi
+++ b/arch/arm/boot/dts/imx28.dtsi
@@ -878,7 +878,7 @@
 				#size-cells = <0>;
 				compatible = "fsl,imx28-i2c";
 				reg = <0x80058000 0x2000>;
-				interrupts = <111 68>;
+				interrupts = <68>;
 				clock-frequency = <100000>;
 				dmas = <&dma_apbx 6>;
 				dma-names = "rx-tx";
@@ -891,7 +891,7 @@
 				#size-cells = <0>;
 				compatible = "fsl,imx28-i2c";
 				reg = <0x8005a000 0x2000>;
-				interrupts = <110 69>;
+				interrupts = <69>;
 				clock-frequency = <100000>;
 				dmas = <&dma_apbx 7>;
 				dma-names = "rx-tx";

and the sgtl5000 probe fails.

[PATCH 2/2] ARM: dts: imx28: Adjust i2c interrupt bindings

[Fabio Estevam]

On Thu, May 16, 2013 at 12:50 PM, Fabio Estevam <festevam@gmail.com> wrote:

> Actually the correct is:
>
> --- a/arch/arm/boot/dts/imx28.dtsi
> +++ b/arch/arm/boot/dts/imx28.dtsi
> @@ -878,7 +878,7 @@
>                                 #size-cells = <0>;
>                                 compatible = "fsl,imx28-i2c";
>                                 reg = <0x80058000 0x2000>;
> -                               interrupts = <111 68>;
> +                               interrupts = <68>;
>                                 clock-frequency = <100000>;
>                                 dmas = <&dma_apbx 6>;
>                                 dma-names = "rx-tx";
> @@ -891,7 +891,7 @@
>                                 #size-cells = <0>;
>                                 compatible = "fsl,imx28-i2c";
>                                 reg = <0x8005a000 0x2000>;
> -                               interrupts = <110 69>;
> +                               interrupts = <69>;
>                                 clock-frequency = <100000>;
>                                 dmas = <&dma_apbx 7>;
>                                 dma-names = "rx-tx";

Sorry, the original patch is correct.

i2c slowness is a different issue.

[PATCH 2/2] ARM: dts: imx28: Adjust i2c interrupt bindings

[Alexandre Belloni]

Hi Fabio,

On 16/05/2013 19:25, Fabio Estevam wrote:
> Sorry, the original patch is correct.
>
> i2c slowness is a different issue.

Would that be related to the timeouts I'm observing ?

http://www.spinics.net/lists/linux-i2c/msg11985.html

I didn't have the time to have a look at it yet. I was planning to hook
up a logic analyzer someday in june to debug it. It seems that there is
a 1 second timeout with each i2c message that are sent.

Don't hesitate to contact me if you need more info.

Regards,

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH 2/2] ARM: dts: imx28: Adjust i2c interrupt bindings

[Fabio Estevam]

Hi Alexandre,

On Thu, May 16, 2013 at 4:33 PM, Alexandre Belloni
<alexandre.belloni@free-electrons.com> wrote:

> Would that be related to the timeouts I'm observing ?
>
> http://www.spinics.net/lists/linux-i2c/msg11985.html
>
> I didn't have the time to have a look at it yet. I was planning to hook
> up a logic analyzer someday in june to debug it. It seems that there is
> a 1 second timeout with each i2c message that are sent.

Yes, I see the same one second timeout for each i2c transaction here.

Regards,

Fabio Estevam

[PATCH 1/2] ARM: dts: imx: Remove custom dma-channel bindings

[Shawn Guo]

On Thu, May 16, 2013 at 11:13:58AM -0300, Fabio Estevam wrote:
> Commit f30fb03 (ARM: dts: add generic DMA device tree binding for mxs-dma) added
> the generic DMA dt bindings, but missed to remove the old ones.
> 
> Remove the the old dma bindings.

I already had a patch [1] to remove those old DMA binding data (both
channel and interrupt number) from client device nodes.  And I'm waiting
for mxs-saif/pcm to be converted to generic DMA binding helper to apply
the patch.  Mark has queued mxs-saif conversion for 3.11, and I have to
wait for 3.11-rc1 to apply the dts cleanup.

Shawn

[1] http://thread.gmane.org/gmane.linux.alsa.devel/106027/focus=220815

[PATCH 2/2] ARM: dts: imx28: Adjust i2c interrupt bindings

[Juergen Beisert]

Hi Fabio,

> i2c slowness is a different issue.

Same happens here for my i.M23 based platform. It seems the PIO mode does not
work, or at least not like it works on a i.MX28. Each short transfer needs
about one second (without an error message) but does not send anything on the
I2C lines.

I need the following patches to make I2C master work within a 3.10-rc2 kernel:

Subject: [PATCH] I2C/MXS: distinguish i.MX23 and i.MX28 based I2C controller
Signed-off-by: Juergen Beisert <jbe@pengutronix.de>

---
 drivers/i2c/busses/i2c-mxs.c | 39 +++++++++++++++++++++++++++++++++------
 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
index 2039f23..5e69d8c 100644
--- a/drivers/i2c/busses/i2c-mxs.c
+++ b/drivers/i2c/busses/i2c-mxs.c
@@ -97,10 +97,17 @@
 #define MXS_CMD_I2C_READ	(MXS_I2C_CTRL0_SEND_NAK_ON_LAST | \
 				 MXS_I2C_CTRL0_MASTER_MODE)
 
+enum mxs_i2c_devtype {
+	MXS_I2C_UNKNOWN = 0,
+	MXS_I2C_V1,
+	MXS_I2C_V2,
+};
+
 /**
  * struct mxs_i2c_dev - per device, private MXS-I2C data
  *
  * @dev: driver model device node
+ * @dev_type: distinguish i.MX23/i.MX28 features
  * @regs: IO registers pointer
  * @cmd_complete: completion object for transaction wait
  * @cmd_err: error code for last transaction
@@ -108,6 +115,7 @@
  */
 struct mxs_i2c_dev {
 	struct device *dev;
+	enum mxs_i2c_devtype dev_type;
 	void __iomem *regs;
 	struct completion cmd_complete;
 	int cmd_err;
@@ -633,8 +641,28 @@ static int mxs_i2c_get_ofdata(struct mxs_i2c_dev *i2c)
 	return 0;
 }
 
+static struct platform_device_id mxs_i2c_devtype[] = {
+	{
+		.name = "imx23-i2c",
+		.driver_data = MXS_I2C_V1,
+	}, {
+		.name = "imx28-i2c",
+		.driver_data = MXS_I2C_V2,
+	}, { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(platform, mxs_i2c_devtype);
+
+static const struct of_device_id mxs_i2c_dt_ids[] = {
+	{ .compatible = "fsl,imx23-i2c", .data = &mxs_i2c_devtype[0], },
+	{ .compatible = "fsl,imx28-i2c", .data = &mxs_i2c_devtype[1], },
+	{ /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, mxs_i2c_dt_ids);
+
 static int mxs_i2c_probe(struct platform_device *pdev)
 {
+	const struct of_device_id *of_id =
+				of_match_device(mxs_i2c_dt_ids, &pdev->dev);
 	struct device *dev = &pdev->dev;
 	struct mxs_i2c_dev *i2c;
 	struct i2c_adapter *adap;
@@ -651,6 +679,11 @@ static int mxs_i2c_probe(struct platform_device *pdev)
 	if (!i2c)
 		return -ENOMEM;
 
+	if (of_id) {
+		const struct platform_device_id *device_id = of_id->data;
+		i2c->dev_type = device_id->driver_data;
+	}
+
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	irq = platform_get_irq(pdev, 0);
 
@@ -726,12 +759,6 @@ static int mxs_i2c_remove(struct platform_device *pdev)
 	return 0;
 }
 
-static const struct of_device_id mxs_i2c_dt_ids[] = {
-	{ .compatible = "fsl,imx28-i2c", },
-	{ /* sentinel */ }
-};
-MODULE_DEVICE_TABLE(of, mxs_i2c_dt_ids);
-
 static struct platform_driver mxs_i2c_driver = {
 	.driver = {
 		   .name = DRIVER_NAME,
---------------------------------------------------------------------------------------------

Subject: [PATCH] I2C/MXS: on i.MX23 only do DMA based I2C transfers
Signed-off-by: Juergen Beisert <jbe@pengutronix.de>

---
 drivers/i2c/busses/i2c-mxs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c
index 5e69d8c..90cbac5 100644
--- a/drivers/i2c/busses/i2c-mxs.c
+++ b/drivers/i2c/busses/i2c-mxs.c
@@ -500,9 +500,10 @@ static int mxs_i2c_xfer_msg(struct i2c_adapter *adap, struct i2c_msg *msg,
 	 * is set to 8 bytes, transfers shorter than 8 bytes are transfered
 	 * using PIO mode while longer transfers use DMA. The 8 byte border is
 	 * based on this empirical measurement and a lot of previous frobbing.
+	 * Note: this special feature only works on i.MX28 SoC
 	 */
 	i2c->cmd_err = 0;
-	if (msg->len < 8) {
+	if (i2c->dev_type == MXS_I2C_V2 && msg->len < 8) {
 		ret = mxs_i2c_pio_setup_xfer(adap, msg, flags);
 		if (ret)
 			mxs_i2c_reset(i2c);

Regards,
Juergen

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | Phone: +49-5121-206917-5128 |
Peiner Str. 6-8, 31137 Hildesheim, Germany    | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686              | http://www.pengutronix.de/  |

[PATCH 2/2] ARM: dts: imx28: Adjust i2c interrupt bindings

[Marek Vasut]

Dear Juergen Beisert,

> Hi Fabio,
> 
> > i2c slowness is a different issue.
> 
> Same happens here for my i.M23 based platform. It seems the PIO mode does
> not work, or at least not like it works on a i.MX28. Each short transfer
> needs about one second (without an error message) but does not send
> anything on the I2C lines.
> 
> I need the following patches to make I2C master work within a 3.10-rc2
> kernel:
> 
> Subject: [PATCH] I2C/MXS: distinguish i.MX23 and i.MX28 based I2C

I'm all for it, but then ... won't it be better if you actually fixed the PIO 
and mixed-mode on MX23 instead of implementing such hack? It might actually turn 
out to be beneficial for MX28 too.

Best regards,
Marek Vasut

[PATCH 2/2] ARM: dts: imx28: Adjust i2c interrupt bindings

[Fabio Estevam]

Hi Juergen,

On Wed, May 22, 2013 at 7:19 AM, Juergen Beisert <jbe@pengutronix.de> wrote:
> Hi Fabio,
>
>> i2c slowness is a different issue.
>
> Same happens here for my i.M23 based platform. It seems the PIO mode does not
> work, or at least not like it works on a i.MX28. Each short transfer needs
> about one second (without an error message) but does not send anything on the
> I2C lines.
>
> I need the following patches to make I2C master work within a 3.10-rc2 kernel:

Thanks, I tried forcing dma mode and I got the following probe issue
on a mx28evk running today's linux-next:

[    6.403193] mxs-sgtl5000 sound.12: ASoC: CODEC (null) not registered
[    6.412175] mxs-sgtl5000 sound.12: snd_soc_register_card failed (-517)
[    6.425582] platform sound.12: Driver mxs-sgtl5000 requests probe deferral
[    6.434043]  mmcblk0: p1 p2 p3
[    6.439586] stmp3xxx-rtc 80056000.rtc: setting system clock to
2013-04-23 21:00:42 UTC (1366750842)
[    6.458141] mxs-sgtl5000 sound.12: ASoC: CODEC (null) not registered
[    6.464773] mxs-sgtl5000 sound.12: snd_soc_register_card failed (-517)

If I run linux-next with 2f769432d8554 reverted then I get a 23
seconds delay in the probe (1 second for each sgtl5000 register
write).

Regards,

Fabio Estevam

[PATCH 2/2] ARM: dts: imx28: Adjust i2c interrupt bindings

[Juergen Beisert]

Hi Marek,

Marek Vasut wrote:
> > > i2c slowness is a different issue.
> >
> > Same happens here for my i.M23 based platform. It seems the PIO mode does
> > not work, or at least not like it works on a i.MX28. Each short transfer
> > needs about one second (without an error message) but does not send
> > anything on the I2C lines.
> >
> > I need the following patches to make I2C master work within a 3.10-rc2
> > kernel:
> >
> > Subject: [PATCH] I2C/MXS: distinguish i.MX23 and i.MX28 based I2C
>
> I'm all for it, but then ... won't it be better if you actually fixed the
> PIO and mixed-mode on MX23 instead of implementing such hack?

If the PIO mode or my patch is a hack depends on the point of view: Lucas told 
me the PIO mode is *mentioned* but *not specified* in the i.MX23/i.MX28 
datasheets.
So, the PIO mode seems to depend on some undocumented status bits in the 
i.MX28 I2C controller implementation.

Regards,
Juergen

-- 
Pengutronix e.K.                              | Juergen Beisert             |
Linux Solutions for Science and Industry      | Phone: +49-5121-206917-5128 |
Peiner Str. 6-8, 31137 Hildesheim, Germany    | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686              | http://www.pengutronix.de/  |

[PATCH 2/2] ARM: dts: imx28: Adjust i2c interrupt bindings

[Marek Vasut]

Dear Juergen Beisert,

> Hi Marek,
> 
> Marek Vasut wrote:
> > > > i2c slowness is a different issue.
> > > 
> > > Same happens here for my i.M23 based platform. It seems the PIO mode
> > > does not work, or at least not like it works on a i.MX28. Each short
> > > transfer needs about one second (without an error message) but does
> > > not send anything on the I2C lines.
> > > 
> > > I need the following patches to make I2C master work within a 3.10-rc2
> > > kernel:
> > > 
> > > Subject: [PATCH] I2C/MXS: distinguish i.MX23 and i.MX28 based I2C
> > 
> > I'm all for it, but then ... won't it be better if you actually fixed the
> > PIO and mixed-mode on MX23 instead of implementing such hack?
> 
> If the PIO mode or my patch is a hack depends on the point of view: Lucas
> told me the PIO mode is *mentioned* but *not specified* in the
> i.MX23/i.MX28 datasheets.

The PIO works the same way DMA does -- set up bits and then pump data into the 
DATA register. 

> So, the PIO mode seems to depend on some undocumented status bits in the
> i.MX28 I2C controller implementation.

How would DMA work then if it used undocumented registers ? It's in the 
documentation, just read it or ask FSL ;-)

Best regards,
Marek Vasut

[PATCH 2/2] ARM: dts: imx28: Adjust i2c interrupt bindings

[Lucas Stach]

Am Donnerstag, den 23.05.2013, 16:48 +0200 schrieb Marek Vasut:
> Dear Juergen Beisert,
> 
> > Hi Marek,
> > 
> > Marek Vasut wrote:
> > > > > i2c slowness is a different issue.
> > > > 
> > > > Same happens here for my i.M23 based platform. It seems the PIO mode
> > > > does not work, or at least not like it works on a i.MX28. Each short
> > > > transfer needs about one second (without an error message) but does
> > > > not send anything on the I2C lines.
> > > > 
> > > > I need the following patches to make I2C master work within a 3.10-rc2
> > > > kernel:
> > > > 
> > > > Subject: [PATCH] I2C/MXS: distinguish i.MX23 and i.MX28 based I2C
> > > 
> > > I'm all for it, but then ... won't it be better if you actually fixed the
> > > PIO and mixed-mode on MX23 instead of implementing such hack?
> > 
> > If the PIO mode or my patch is a hack depends on the point of view: Lucas
> > told me the PIO mode is *mentioned* but *not specified* in the
> > i.MX23/i.MX28 datasheets.
> 
> The PIO works the same way DMA does -- set up bits and then pump data into the 
> DATA register. 

> > So, the PIO mode seems to depend on some undocumented status bits in the
> > i.MX28 I2C controller implementation.
> 
> How would DMA work then if it used undocumented registers ? It's in the 
> documentation, just read it or ask FSL ;-)
> 
While the PIO mode might use the same controller mechanisms as the DMA
mode, PIO mode is _not_ a documented mode of operation for the i.MX23.

To quote the i.MX28 RM: "The I2C block on the i.MX28 supports a new PIO
mode or soft-DMA mode.", which implies the PIO mode to be a new mode of
operation not found on earlier i.MX SoCs.
The doc is slightly fuzzy here as the i.MX23 RM in contrary states:
"Short transmission (up to three bytes plus address) can be easily
triggered using only PIO operations, i.e., no DMA setup required." But
again it's not a documented mode of operation, i.MX23 doc only describes
the DMA mode.

So while we _might_ be able to get the PIO mode to work on the i.MX23
there is nothing in the doc stating that it's even meant to work. Even
while PIO and DMA mode use the same internal mechanisms, there's still
plenty of opportunities of fail in there. After all PIO mode relies on
reading a debug register in the course of normal operation.

Only more extensive experimentation could show if we are in fact able to
make it work, a first shot of using PIO mode on MX23 failed, so it might
as well be that Juergens quick fix is correct and we have to disable PIO
mode on MX23 altogether. That said please stop slapping the word "hack"
over this patch until proven otherwise.

Regards,
Lucas
-- 
Pengutronix e.K.                           | Lucas Stach                 |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 2/2] ARM: dts: imx28: Adjust i2c interrupt bindings

[Alexandre Belloni]

Hi,

On 23/05/2013 17:28, Lucas Stach wrote:
> Am Donnerstag, den 23.05.2013, 16:48 +0200 schrieb Marek Vasut:
>> Dear Juergen Beisert,
>>
>>> Hi Marek,
>>>
>>> Marek Vasut wrote:
>>>>>> i2c slowness is a different issue.
>>>>> Same happens here for my i.M23 based platform. It seems the PIO mode
>>>>> does not work, or at least not like it works on a i.MX28. Each short
>>>>> transfer needs about one second (without an error message) but does
>>>>> not send anything on the I2C lines.
>>>>>
>>>>> I need the following patches to make I2C master work within a 3.10-rc2
>>>>> kernel:
>>>>>
>>>>> Subject: [PATCH] I2C/MXS: distinguish i.MX23 and i.MX28 based I2C
>>>> I'm all for it, but then ... won't it be better if you actually fixed the
>>>> PIO and mixed-mode on MX23 instead of implementing such hack?
>>> If the PIO mode or my patch is a hack depends on the point of view: Lucas
>>> told me the PIO mode is *mentioned* but *not specified* in the
>>> i.MX23/i.MX28 datasheets.
>> The PIO works the same way DMA does -- set up bits and then pump data into the 
>> DATA register. 
>>> So, the PIO mode seems to depend on some undocumented status bits in the
>>> i.MX28 I2C controller implementation.
>> How would DMA work then if it used undocumented registers ? It's in the 
>> documentation, just read it or ask FSL ;-)
>>
> While the PIO mode might use the same controller mechanisms as the DMA
> mode, PIO mode is _not_ a documented mode of operation for the i.MX23.
>
> To quote the i.MX28 RM: "The I2C block on the i.MX28 supports a new PIO
> mode or soft-DMA mode.", which implies the PIO mode to be a new mode of
> operation not found on earlier i.MX SoCs.
> The doc is slightly fuzzy here as the i.MX23 RM in contrary states:
> "Short transmission (up to three bytes plus address) can be easily
> triggered using only PIO operations, i.e., no DMA setup required." But
> again it's not a documented mode of operation, i.MX23 doc only describes
> the DMA mode.
>
> So while we _might_ be able to get the PIO mode to work on the i.MX23
> there is nothing in the doc stating that it's even meant to work. Even
> while PIO and DMA mode use the same internal mechanisms, there's still
> plenty of opportunities of fail in there. After all PIO mode relies on
> reading a debug register in the course of normal operation.
>
> Only more extensive experimentation could show if we are in fact able to
> make it work, a first shot of using PIO mode on MX23 failed, so it might
> as well be that Juergens quick fix is correct and we have to disable PIO
> mode on MX23 altogether. That said please stop slapping the word "hack"
> over this patch until proven otherwise.

Still, Fabio and I have been experiencing the one second delay on
i.mx28. From my experience, it appeared between 3.7 and 3.9, see:
http://www.spinics.net/lists/linux-i2c/msg11985.html. I still didn't
take the time to investigate more yet but it confirms that PIO mode is
working on i.mx28 though with an annoying delay.

Regards,

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[PATCH 2/2] ARM: dts: imx28: Adjust i2c interrupt bindings

[Marek Vasut]

Dear Lucas Stach,

> Am Donnerstag, den 23.05.2013, 16:48 +0200 schrieb Marek Vasut:
> > Dear Juergen Beisert,
> > 
> > > Hi Marek,
> > > 
> > > Marek Vasut wrote:
> > > > > > i2c slowness is a different issue.
> > > > > 
> > > > > Same happens here for my i.M23 based platform. It seems the PIO
> > > > > mode does not work, or at least not like it works on a i.MX28.
> > > > > Each short transfer needs about one second (without an error
> > > > > message) but does not send anything on the I2C lines.
> > > > > 
> > > > > I need the following patches to make I2C master work within a
> > > > > 3.10-rc2 kernel:
> > > > > 
> > > > > Subject: [PATCH] I2C/MXS: distinguish i.MX23 and i.MX28 based I2C
> > > > 
> > > > I'm all for it, but then ... won't it be better if you actually fixed
> > > > the PIO and mixed-mode on MX23 instead of implementing such hack?
> > > 
> > > If the PIO mode or my patch is a hack depends on the point of view:
> > > Lucas told me the PIO mode is *mentioned* but *not specified* in the
> > > i.MX23/i.MX28 datasheets.
> > 
> > The PIO works the same way DMA does -- set up bits and then pump data
> > into the DATA register.
> > 
> > > So, the PIO mode seems to depend on some undocumented status bits in
> > > the i.MX28 I2C controller implementation.
> > 
> > How would DMA work then if it used undocumented registers ? It's in the
> > documentation, just read it or ask FSL ;-)
> 
> While the PIO mode might use the same controller mechanisms as the DMA
> mode, PIO mode is _not_ a documented mode of operation for the i.MX23.
> 
> To quote the i.MX28 RM: "The I2C block on the i.MX28 supports a new PIO
> mode or soft-DMA mode.", which implies the PIO mode to be a new mode of
> operation not found on earlier i.MX SoCs.
> The doc is slightly fuzzy here as the i.MX23 RM in contrary states:
> "Short transmission (up to three bytes plus address) can be easily
> triggered using only PIO operations, i.e., no DMA setup required." But
> again it's not a documented mode of operation, i.MX23 doc only describes
> the DMA mode.

I'm CCing a Amaury Pouly so he can comment on this stuff. He did some hardware 
digging.

> So while we _might_ be able to get the PIO mode to work on the i.MX23
> there is nothing in the doc stating that it's even meant to work. Even
> while PIO and DMA mode use the same internal mechanisms, there's still
> plenty of opportunities of fail in there. After all PIO mode relies on
> reading a debug register in the course of normal operation.

What's the problem with that ? The debug register contains status bits.

> Only more extensive experimentation could show if we are in fact able to
> make it work, a first shot of using PIO mode on MX23 failed, so it might
> as well be that Juergens quick fix is correct and we have to disable PIO
> mode on MX23 altogether. That said please stop slapping the word "hack"
> over this patch until proven otherwise.
> 
> Regards,
> Lucas

Best regards,
Marek Vasut

[PATCH 2/2] ARM: dts: imx28: Adjust i2c interrupt bindings

[Lucas Stach]

Am Donnerstag, den 23.05.2013, 19:51 +0200 schrieb Alexandre Belloni:
> Hi,
> 
[...]
> >
> > So while we _might_ be able to get the PIO mode to work on the i.MX23
> > there is nothing in the doc stating that it's even meant to work. Even
> > while PIO and DMA mode use the same internal mechanisms, there's still
> > plenty of opportunities of fail in there. After all PIO mode relies on
> > reading a debug register in the course of normal operation.
> >
> > Only more extensive experimentation could show if we are in fact able to
> > make it work, a first shot of using PIO mode on MX23 failed, so it might
> > as well be that Juergens quick fix is correct and we have to disable PIO
> > mode on MX23 altogether. That said please stop slapping the word "hack"
> > over this patch until proven otherwise.
> 
> Still, Fabio and I have been experiencing the one second delay on
> i.mx28. From my experience, it appeared between 3.7 and 3.9, see:
> http://www.spinics.net/lists/linux-i2c/msg11985.html. I still didn't
> take the time to investigate more yet but it confirms that PIO mode is
> working on i.mx28 though with an annoying delay.
> 

Just for the record: I know that PIO mode is functional on i.MX28
because I have it working on one of our customer boards. I am rather
much surprised it's still not working well for you, but haven't had a
chance to investigate yet. The i.MX28 RM explicitly describes how to
work with the PIO mode, so if it's not working this is bug somewhere.

This thread/patch is about a whole different problem, namely PIO mode
being non-functional on i.MX23. As the i.MX23 RM doesn't describe the
PIO mode, there is no indication that it's even meant to work on this
chip.

Regards,
Lucas
-- 
Pengutronix e.K.                           | Lucas Stach                 |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

[PATCH 2/2] ARM: dts: imx28: Adjust i2c interrupt bindings

[Marek Vasut]

Dear Lucas Stach,

> Am Donnerstag, den 23.05.2013, 19:51 +0200 schrieb Alexandre Belloni:
> > Hi,
> 
> [...]
> 
> > > So while we _might_ be able to get the PIO mode to work on the i.MX23
> > > there is nothing in the doc stating that it's even meant to work. Even
> > > while PIO and DMA mode use the same internal mechanisms, there's still
> > > plenty of opportunities of fail in there. After all PIO mode relies on
> > > reading a debug register in the course of normal operation.
> > > 
> > > Only more extensive experimentation could show if we are in fact able
> > > to make it work, a first shot of using PIO mode on MX23 failed, so it
> > > might as well be that Juergens quick fix is correct and we have to
> > > disable PIO mode on MX23 altogether. That said please stop slapping
> > > the word "hack" over this patch until proven otherwise.
> > 
> > Still, Fabio and I have been experiencing the one second delay on
> > i.mx28. From my experience, it appeared between 3.7 and 3.9, see:
> > http://www.spinics.net/lists/linux-i2c/msg11985.html. I still didn't
> > take the time to investigate more yet but it confirms that PIO mode is
> > working on i.mx28 though with an annoying delay.
> 
> Just for the record: I know that PIO mode is functional on i.MX28
> because I have it working on one of our customer boards. I am rather
> much surprised it's still not working well for you, but haven't had a
> chance to investigate yet. The i.MX28 RM explicitly describes how to
> work with the PIO mode, so if it's not working this is bug somewhere.

I won't be surprised if there was more than one. The RM describes something , 
but has many flaws. I'm sure you noticed that yourself.

> This thread/patch is about a whole different problem, namely PIO mode
> being non-functional on i.MX23. As the i.MX23 RM doesn't describe the
> PIO mode, there is no indication that it's even meant to work on this
> chip.

I'm not against the patch per-se, but can we not get the best of this and 
instead try some more to fix the PIO on MX23 so the MX23 users won't suffer 
performance degradation on already weak CPU?

Best regards,
Marek Vasut

[PATCH 2/2] ARM: dts: imx28: Adjust i2c interrupt bindings

[Marek Vasut]

Dear Juergen Beisert,

> Hi Fabio,
> 
> > i2c slowness is a different issue.
> 
> Same happens here for my i.M23 based platform. It seems the PIO mode does
> not work, or at least not like it works on a i.MX28. Each short transfer
> needs about one second (without an error message) but does not send
> anything on the I2C lines.
> 
> I need the following patches to make I2C master work within a 3.10-rc2
> kernel:
> 
> Subject: [PATCH] I2C/MXS: distinguish i.MX23 and i.MX28 based I2C
> controller Signed-off-by: Juergen Beisert <jbe@pengutronix.de>

Can you please properly submit this patch ?

Best regards,
Marek Vasut