[PATCH 0/2] spi: imx: don't loop endlessly

[PATCH 0/2] spi: imx: don't loop endlessly

[Uwe Kleine-König]

Hello,

during bringup of an i.MX7 board I am faced with cspi_2_3_xchg_single not
returning. I don't know yet why this happens, but with this patch set it at least
doesn't block barebox.

I didn't test on a working board, maybe the timeout (10 µs) I chose is too tight?

Best regards
Uwe

Uwe Kleine-König (2):
  spi: imx: add error checking
  spi: imx: add timeout to xchg_single

 drivers/spi/imx_spi.c | 105 ++++++++++++++++++++++++++++++++++----------------
 1 file changed, 72 insertions(+), 33 deletions(-)

-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 1/2] spi: imx: add error checking

[Uwe Kleine-König]

This makes it possible that the xchg_single callbacks return an error code that
is then passed to the caller of spi_transfer. There is no change in behaviour
intended as up to now the callbacks always return 0.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/spi/imx_spi.c | 77 ++++++++++++++++++++++++++++++++-------------------
 1 file changed, 48 insertions(+), 29 deletions(-)

diff --git a/drivers/spi/imx_spi.c b/drivers/spi/imx_spi.c
index 876699a02da1..97a2091969dd 100644
--- a/drivers/spi/imx_spi.c
+++ b/drivers/spi/imx_spi.c
@@ -38,8 +38,8 @@ struct imx_spi {
 	void __iomem		*regs;
 	struct clk		*clk;
 
-	unsigned int		(*xchg_single)(struct imx_spi *imx, u32 data);
-	void			(*do_transfer)(struct spi_device *spi);
+	int			(*xchg_single)(struct imx_spi *imx, u32 txdata, u32 *rxdata);
+	int			(*do_transfer)(struct spi_device *spi);
 	void			(*chipselect)(struct spi_device *spi, int active);
 
 	const void *tx_buf;
@@ -49,8 +49,8 @@ struct imx_spi {
 };
 
 struct spi_imx_devtype_data {
-	unsigned int		(*xchg_single)(struct imx_spi *imx, u32 data);
-	void			(*do_transfer)(struct spi_device *spi);
+	int			(*xchg_single)(struct imx_spi *imx, u32 txdata, u32 *rxdata);
+	int			(*do_transfer)(struct spi_device *spi);
 	void			(*chipselect)(struct spi_device *spi, int active);
 	void			(*init)(struct imx_spi *imx);
 };
@@ -86,13 +86,13 @@ static unsigned int imx_spi_maybe_reverse_bits(struct spi_device *spi, unsigned
 	return result;
 }
 
-static unsigned int cspi_0_0_xchg_single(struct imx_spi *imx, unsigned int data)
+static int cspi_0_0_xchg_single(struct imx_spi *imx, u32 txdata, u32 *rxdata)
 {
 	void __iomem *base = imx->regs;
 
 	unsigned int cfg_reg = readl(base + CSPI_0_0_CTRL);
 
-	writel(data, base + CSPI_0_0_TXDATA);
+	writel(txdata, base + CSPI_0_0_TXDATA);
 
 	cfg_reg |= CSPI_0_0_CTRL_XCH;
 
@@ -100,7 +100,9 @@ static unsigned int cspi_0_0_xchg_single(struct imx_spi *imx, unsigned int data)
 
 	while (!(readl(base + CSPI_0_0_INT) & CSPI_0_0_STAT_RR));
 
-	return readl(base + CSPI_0_0_RXDATA);
+	*rxdata = readl(base + CSPI_0_0_RXDATA);
+
+	return 0;
 }
 
 static void cspi_0_0_chipselect(struct spi_device *spi, int is_active)
@@ -152,13 +154,13 @@ static void cspi_0_0_init(struct imx_spi *imx)
 	} while (readl(base + CSPI_0_0_RESET) & CSPI_0_0_RESET_START);
 }
 
-static unsigned int cspi_0_7_xchg_single(struct imx_spi *imx, unsigned int data)
+static int cspi_0_7_xchg_single(struct imx_spi *imx, u32 txdata, u32 *rxdata)
 {
 	void __iomem *base = imx->regs;
 
 	unsigned int cfg_reg = readl(base + CSPI_0_7_CTRL);
 
-	writel(data, base + CSPI_0_7_TXDATA);
+	writel(txdata, base + CSPI_0_7_TXDATA);
 
 	cfg_reg |= CSPI_0_7_CTRL_XCH;
 
@@ -167,7 +169,8 @@ static unsigned int cspi_0_7_xchg_single(struct imx_spi *imx, unsigned int data)
 	while (!(readl(base + CSPI_0_7_STAT) & CSPI_0_7_STAT_RR))
 		;
 
-	return readl(base + CSPI_0_7_RXDATA);
+	*rxdata = readl(base + CSPI_0_7_RXDATA);
+	return 0;
 }
 
 /* MX1, MX31, MX35, MX51 CSPI */
@@ -242,15 +245,17 @@ static void cspi_0_7_init(struct imx_spi *imx)
 		readl(base + CSPI_0_7_RXDATA);
 }
 
-static unsigned int cspi_2_3_xchg_single(struct imx_spi *imx, unsigned int data)
+static int cspi_2_3_xchg_single(struct imx_spi *imx, u32 txdata, u32 *rxdata)
 {
 	void __iomem *base = imx->regs;
 
-	writel(data, base + CSPI_2_3_TXDATA);
+	writel(txdata, base + CSPI_2_3_TXDATA);
 
 	while (!(readl(base + CSPI_2_3_STAT) & CSPI_2_3_STAT_RR));
 
-	return readl(base + CSPI_2_3_RXDATA);
+	*rxdata = readl(base + CSPI_2_3_RXDATA);
+
+	return 0;
 }
 
 static unsigned int cspi_2_3_clkdiv(unsigned int fin, unsigned int fspi)
@@ -336,16 +341,20 @@ static void cspi_2_3_chipselect(struct spi_device *spi, int is_active)
 		gpio_set_value(gpio, gpio_cs);
 }
 
-static u32 imx_xchg_single(struct spi_device *spi, u32 tx_val)
+static int imx_xchg_single(struct spi_device *spi, u32 tx_val, u32* rx_val)
 {
-	u32 rx_val;
 	struct imx_spi *imx = container_of(spi->master, struct imx_spi, master);
-
+	u32 local_rx_val;
+	int ret;
 
 	tx_val = imx_spi_maybe_reverse_bits(spi, tx_val);
-	rx_val = imx->xchg_single(imx, tx_val);
+	ret = imx->xchg_single(imx, tx_val, &local_rx_val);
+	if (ret)
+		return ret;
 
-	return imx_spi_maybe_reverse_bits(spi, rx_val);
+	*rx_val = imx_spi_maybe_reverse_bits(spi, local_rx_val);
+
+	return 0;
 }
 
 static void cspi_2_3_init(struct imx_spi *imx)
@@ -355,18 +364,21 @@ static void cspi_2_3_init(struct imx_spi *imx)
 	writel(0, base + CSPI_2_3_CTRL);
 }
 
-static void imx_spi_do_transfer(struct spi_device *spi)
+static int imx_spi_do_transfer(struct spi_device *spi)
 {
 	struct imx_spi *imx = container_of(spi->master, struct imx_spi, master);
 	unsigned i;
+	u32 rx_val;
+	int ret;
 
 	if (imx->bits_per_word <= 8) {
 		const u8	*tx_buf = imx->tx_buf;
 		u8		*rx_buf = imx->rx_buf;
-		u8		rx_val;
 
 		for (i = 0; i < imx->xfer_len; i++) {
-			rx_val = imx_xchg_single(spi, tx_buf ? tx_buf[i] : 0);
+			ret = imx_xchg_single(spi, tx_buf ? tx_buf[i] : 0, &rx_val);
+			if (ret)
+				return ret;
 
 			if (rx_buf)
 				rx_buf[i] = rx_val;
@@ -374,10 +386,11 @@ static void imx_spi_do_transfer(struct spi_device *spi)
 	} else if (imx->bits_per_word <= 16) {
 		const u16	*tx_buf = imx->tx_buf;
 		u16		*rx_buf = imx->rx_buf;
-		u16		rx_val;
 
 		for (i = 0; i < imx->xfer_len >> 1; i++) {
-			rx_val = imx_xchg_single(spi, tx_buf ? tx_buf[i] : 0);
+			ret = imx_xchg_single(spi, tx_buf ? tx_buf[i] : 0, &rx_val);
+			if (ret)
+				return ret;
 
 			if (rx_buf)
 				rx_buf[i] = rx_val;
@@ -385,15 +398,18 @@ static void imx_spi_do_transfer(struct spi_device *spi)
 	} else if (imx->bits_per_word <= 32) {
 		const u32	*tx_buf = imx->tx_buf;
 		u32		*rx_buf = imx->rx_buf;
-		u32		rx_val;
 
 		for (i = 0; i < imx->xfer_len >> 2; i++) {
-			rx_val = imx_xchg_single(spi, tx_buf ? tx_buf[i] : 0);
+			ret = imx_xchg_single(spi, tx_buf ? tx_buf[i] : 0, &rx_val);
+			if (ret)
+				return ret;
 
 			if (rx_buf)
 				rx_buf[i] = rx_val;
 		}
 	}
+
+	return 0;
 }
 
 static int cspi_2_3_xchg_burst(struct spi_device *spi)
@@ -448,7 +464,7 @@ static int cspi_2_3_xchg_burst(struct spi_device *spi)
 	return now;
 }
 
-static void cspi_2_3_do_transfer(struct spi_device *spi)
+static int cspi_2_3_do_transfer(struct spi_device *spi)
 {
 	struct imx_spi *imx = container_of(spi->master, struct imx_spi, master);
 	u32 ctrl;
@@ -457,14 +473,14 @@ static void cspi_2_3_do_transfer(struct spi_device *spi)
 		while (cspi_2_3_xchg_burst(spi) > 0);
 
 	if (!imx->xfer_len)
-		return;
+		return 0;
 
 	ctrl = readl(imx->regs + CSPI_2_3_CTRL);
 	ctrl &= ~(0xfff << CSPI_2_3_CTRL_BL_OFFSET);
 	ctrl |= (spi->bits_per_word - 1) << CSPI_2_3_CTRL_BL_OFFSET;
 	writel(ctrl, imx->regs + CSPI_2_3_CTRL);
 
-	imx_spi_do_transfer(spi);
+	return imx_spi_do_transfer(spi);
 }
 
 static int imx_spi_transfer(struct spi_device *spi, struct spi_message *mesg)
@@ -473,6 +489,7 @@ static int imx_spi_transfer(struct spi_device *spi, struct spi_message *mesg)
 	struct spi_transfer *t;
 	unsigned int cs_change;
 	const int nsecs = 50;
+	int ret;
 
 	imx->chipselect(spi, 1);
 
@@ -494,7 +511,9 @@ static int imx_spi_transfer(struct spi_device *spi, struct spi_message *mesg)
 		imx->rx_buf = t->rx_buf;
 		imx->xfer_len = t->len;
 		imx->bits_per_word = spi->bits_per_word;
-		imx->do_transfer(spi);
+		ret = imx->do_transfer(spi);
+		if (ret)
+			return ret;
 
 		mesg->actual_length += t->len;
 
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/2] spi: imx: add timeout to xchg_single

[Uwe Kleine-König]

If there is a problem STAT_RR might never be set which results in an endless loop.
Break out after 10 µs with -ETIMEOUT instead.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/spi/imx_spi.c | 28 ++++++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/imx_spi.c b/drivers/spi/imx_spi.c
index 97a2091969dd..78198798a57e 100644
--- a/drivers/spi/imx_spi.c
+++ b/drivers/spi/imx_spi.c
@@ -32,6 +32,9 @@
 #include <linux/err.h>
 #include <clock.h>
 
+/* time to wait for STAT_RR getting set */
+#define IMX_SPI_RR_TIMEOUT	10000 /* ns */
+
 struct imx_spi {
 	struct spi_master	master;
 	int			*cs_array;
@@ -89,6 +92,7 @@ static unsigned int imx_spi_maybe_reverse_bits(struct spi_device *spi, unsigned
 static int cspi_0_0_xchg_single(struct imx_spi *imx, u32 txdata, u32 *rxdata)
 {
 	void __iomem *base = imx->regs;
+	int ret;
 
 	unsigned int cfg_reg = readl(base + CSPI_0_0_CTRL);
 
@@ -98,7 +102,12 @@ static int cspi_0_0_xchg_single(struct imx_spi *imx, u32 txdata, u32 *rxdata)
 
 	writel(cfg_reg, base + CSPI_0_0_CTRL);
 
-	while (!(readl(base + CSPI_0_0_INT) & CSPI_0_0_STAT_RR));
+	ret = wait_on_timeout(IMX_SPI_RR_TIMEOUT,
+			      readl(base + CSPI_0_0_INT) & CSPI_0_0_STAT_RR);
+	if (ret) {
+		dev_err(imx->master.dev, "Timeout waiting for received data\n");
+		return ret;
+	}
 
 	*rxdata = readl(base + CSPI_0_0_RXDATA);
 
@@ -157,6 +166,7 @@ static void cspi_0_0_init(struct imx_spi *imx)
 static int cspi_0_7_xchg_single(struct imx_spi *imx, u32 txdata, u32 *rxdata)
 {
 	void __iomem *base = imx->regs;
+	int ret;
 
 	unsigned int cfg_reg = readl(base + CSPI_0_7_CTRL);
 
@@ -166,8 +176,12 @@ static int cspi_0_7_xchg_single(struct imx_spi *imx, u32 txdata, u32 *rxdata)
 
 	writel(cfg_reg, base + CSPI_0_7_CTRL);
 
-	while (!(readl(base + CSPI_0_7_STAT) & CSPI_0_7_STAT_RR))
-		;
+	ret = wait_on_timeout(IMX_SPI_RR_TIMEOUT,
+			      readl(base + CSPI_0_7_STAT) & CSPI_0_7_STAT_RR);
+	if (ret) {
+		dev_err(imx->master.dev, "Timeout waiting for received data\n");
+		return ret;
+	}
 
 	*rxdata = readl(base + CSPI_0_7_RXDATA);
 	return 0;
@@ -248,10 +262,16 @@ static void cspi_0_7_init(struct imx_spi *imx)
 static int cspi_2_3_xchg_single(struct imx_spi *imx, u32 txdata, u32 *rxdata)
 {
 	void __iomem *base = imx->regs;
+	int ret;
 
 	writel(txdata, base + CSPI_2_3_TXDATA);
 
-	while (!(readl(base + CSPI_2_3_STAT) & CSPI_2_3_STAT_RR));
+	ret = wait_on_timeout(IMX_SPI_RR_TIMEOUT,
+			      readl(base + CSPI_2_3_STAT) & CSPI_2_3_STAT_RR);
+	if (ret) {
+		dev_err(imx->master.dev, "Timeout waiting for received data\n");
+		return ret;
+	}
 
 	*rxdata = readl(base + CSPI_2_3_RXDATA);
 
-- 
2.11.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 0/2] spi: imx: don't loop endlessly

[Sam Ravnborg]

Hi Uwe.

On Wed, Aug 02, 2017 at 09:39:05PM +0200, Uwe Kleine-König wrote:
> Hello,
> 
> during bringup of an i.MX7 board I am faced with cspi_2_3_xchg_single not
> returning. I don't know yet why this happens, but with this patch set it at least
> doesn't block barebox.

Your description reminded me of a similar issue we hit with SPI transfer.
Back then Sascha fixed it but with some open question marks.

See 9da7e18573c725eaa6123c401df57d6a4f6a0ea2
("spi: i.MX: reset controller on init")

But as this is the latest patch to the imx_spi.c file you likely
have already looked at it.
But mentioned just in case..

	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 0/2] spi: imx: don't loop endlessly

[Uwe Kleine-König]

On Wed, Aug 02, 2017 at 09:54:40PM +0200, Sam Ravnborg wrote:
> Hi Uwe.
> 
> On Wed, Aug 02, 2017 at 09:39:05PM +0200, Uwe Kleine-König wrote:
> > Hello,
> > 
> > during bringup of an i.MX7 board I am faced with cspi_2_3_xchg_single not
> > returning. I don't know yet why this happens, but with this patch set it at least
> > doesn't block barebox.
> 
> Your description reminded me of a similar issue we hit with SPI transfer.
> Back then Sascha fixed it but with some open question marks.
> 
> See 9da7e18573c725eaa6123c401df57d6a4f6a0ea2
> ("spi: i.MX: reset controller on init")
> 
> But as this is the latest patch to the imx_spi.c file you likely
> have already looked at it.

I havn't looked at it, but it is included in my barebox as I'm using
2017.08.0 + some patches.

Reading through the patch description, it doesn't match my situation.
I'm doing USB-Booting because when I made eMMC work all my available
remote hands already called it a day. So unless the boot ROM does
something strange (impossible!) barebox is the first spi user.

What seems to be similar is that TESTREG.TXCNT is != 0. For me this
smells clk-related. On i.MX6 I recently identified a problem (but didn't
come around yet to mainline a fix) that might match at least your
problem.

Can you still reproduce? How does your clk-tree look like (clk_dump)?

Thanks for your attention
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 0/2] spi: imx: don't loop endlessly

[Sam Ravnborg]

Hi Uwe.
> I havn't looked at it, but it is included in my barebox as I'm using
> 2017.08.0 + some patches.
And I did not expect it to solve your problem, only to give you
inspiration what could be the issue.

> 
> Reading through the patch description, it doesn't match my situation.
OK

> I'm doing USB-Booting because when I made eMMC work all my available
> remote hands already called it a day. So unless the boot ROM does
> something strange (impossible!) barebox is the first spi user.
> 
> What seems to be similar is that TESTREG.TXCNT is != 0. For me this
> smells clk-related. On i.MX6 I recently identified a problem (but didn't
> come around yet to mainline a fix) that might match at least your
> problem.
> 
> Can you still reproduce?
We have used the patch from Sascha for a long time now (from before it
hit barebox-next, and on our proprietary IMX6 target we never
saw the SPI related error since.

	Sam

> How does your clk-tree look like (clk_dump)?

This is with:
$ version
barebox 2017.03.0-1 #1 Sun May 14 16:17:24 CEST 2017
+ some of our own patches (nothing clk related).

$ clk_dump
lvds2_sel (rate 0, enabled)
    lvds2_gate (rate 0, disabled)
osc (rate 24000000, enabled)
    pll1_sys (rate 792000000, disabled)
        pll1_sw (rate 792000000, disabled)
            arm (rate 792000000, disabled)
                twd (rate 396000000, disabled)
                cko2_sel (rate 792000000, disabled)
                    cko2_podf (rate 792000000, disabled)
                        cko2 (rate 792000000, disabled)
    pll2_bus (rate 528000000, enabled)
        pll2_pfd0_352m (rate 306580645, disabled)
        pll2_pfd1_594m (rate 528000000, disabled)
        pll2_pfd2_396m (rate 396000000, enabled)
            pll2_198m (rate 198000000, enabled)
            step (rate 396000000, enabled)
            periph_pre (rate 396000000, enabled)
                periph (rate 396000000, enabled)
                    axi_sel (rate 396000000, enabled)
                        axi (rate 198000000, enabled)
                            eim_sel (rate 198000000, enabled)
                                eim_podf (rate 99000000, enabled)
                            eim_slow_sel (rate 198000000, enabled)
                                eim_slow_podf (rate 99000000, enabled)
                                    eim_slow (rate 99000000, enabled)
                            vdo_axi_sel (rate 198000000, enabled)
                            pcie_axi_sel (rate 198000000, enabled)
                                pcie_axi (rate 198000000, disabled)
                    mmdc_ch0_axi_podf (rate 396000000, enabled)
                        ipu1_sel (rate 396000000, enabled)
                            ipu1_podf (rate 49500000, enabled)
                                ipu1 (rate 49500000, enabled)
                                    2400000.ipu_di0_pixel (rate 33000000, enabled)
                                    2400000.ipu_di1_pixel (rate 49500000, enabled)
                        ipu2_sel (rate 396000000, enabled)
                            ipu2_podf (rate 198000000, enabled)
                                ipu2 (rate 198000000, enabled)
                    ahb (rate 132000000, enabled)
                        ipg (rate 66000000, enabled)
                            ipg_per (rate 66000000, enabled)
                                gpt_ipg_per (rate 66000000, enabled)
                                i2c1 (rate 66000000, enabled)
                                i2c2 (rate 66000000, enabled)
                                i2c3 (rate 66000000, enabled)
                                pwm1 (rate 66000000, enabled)
                                pwm2 (rate 66000000, enabled)
                                pwm3 (rate 66000000, enabled)
                                pwm4 (rate 66000000, enabled)
                            caam_ipg (rate 66000000, enabled)
                            enet (rate 66000000, enabled)
                            gpt_ipg (rate 66000000, enabled)
                            iim (rate 66000000, enabled)
                            sata (rate 66000000, enabled)
                            uart_ipg (rate 66000000, enabled)
                            usboh3 (rate 66000000, enabled)
                        caam_mem (rate 132000000, enabled)
                        caam_aclk (rate 132000000, enabled)
            periph2_pre (rate 396000000, enabled)
                periph2 (rate 396000000, enabled)
                    mmdc_ch1_axi_podf (rate 396000000, enabled)
            usdhc1_sel (rate 396000000, enabled)
                usdhc1_podf (rate 198000000, enabled)
                    usdhc1 (rate 198000000, enabled)
            usdhc2_sel (rate 396000000, enabled)
                usdhc2_podf (rate 198000000, enabled)
                    usdhc2 (rate 198000000, enabled)
            usdhc3_sel (rate 396000000, enabled)
                usdhc3_podf (rate 198000000, enabled)
                    usdhc3 (rate 198000000, enabled)
                        apbh_dma (rate 198000000, enabled)
                        per1_bch (rate 198000000, enabled)
                        gpmi_bch_apb (rate 198000000, enabled)
                        gpmi_apb (rate 198000000, enabled)
            usdhc4_sel (rate 396000000, enabled)
                usdhc4_podf (rate 198000000, enabled)
                    usdhc4 (rate 198000000, enabled)
                        gpmi_bch (rate 198000000, enabled)
        cko1_sel (rate 528000000, enabled)
            cko1_podf (rate 528000000, enabled)
                cko1 (rate 528000000, disabled)
                    cko (rate 528000000, disabled)
    pll3_usb_otg (rate 480000000, enabled)
        usbphy1 (rate 480000000, disabled)
        pll3_pfd0_720m (rate 720000000, disabled)
        pll3_pfd1_540m (rate 540000000, disabled)
        pll3_pfd2_508m (rate 508235294, disabled)
        pll3_pfd3_454m (rate 454736842, disabled)
        pll3_120m (rate 120000000, enabled)
        pll3_80m (rate 80000000, enabled)
            uart_serial_podf (rate 80000000, enabled)
                uart_serial (rate 80000000, enabled)
        pll3_60m (rate 60000000, enabled)
            ecspi_root (rate 60000000, enabled)
                ecspi1 (rate 60000000, enabled)
                ecspi2 (rate 60000000, enabled)
                ecspi3 (rate 60000000, enabled)
                ecspi4 (rate 60000000, enabled)
                ecspi5 (rate 60000000, enabled)
        periph2_clk2_sel (rate 480000000, enabled)
            periph2_clk2 (rate 480000000, enabled)
        enfc_sel (rate 480000000, enabled)
            enfc_pred (rate 96000000, enabled)
                enfc_podf (rate 96000000, enabled)
                    enfc (rate 96000000, enabled)
                        gpmi_io (rate 96000000, enabled)
        can_root (rate 240000000, enabled)
    pll4_audio (rate 144000000, disabled)
    pll5_video (rate 288000000, disabled)
        pll5_post_div (rate 72000000, disabled)
            pll5_video_div (rate 72000000, disabled)
                ldb_di0_sel (rate 72000000, disabled)
                    ldb_di0_div_3_5 (rate 20571428, disabled)
                        ldb_di0_podf (rate 10285714, disabled)
                            ldb_di0 (rate 10285714, disabled)
                ldb_di1_sel (rate 72000000, disabled)
                    ldb_di1_div_3_5 (rate 20571428, disabled)
                        ldb_di1_podf (rate 10285714, disabled)
                            ldb_di1 (rate 10285714, disabled)
                ipu1_di0_pre_sel (rate 72000000, disabled)
                    ipu1_di0_pre (rate 24000000, disabled)
                        ipu1_di0_sel (rate 24000000, disabled)
                            ipu1_di0 (rate 24000000, disabled)
                ipu1_di1_pre_sel (rate 72000000, disabled)
                    ipu1_di1_pre (rate 24000000, disabled)
                        ipu1_di1_sel (rate 24000000, disabled)
                            ipu1_di1 (rate 24000000, disabled)
                ipu2_di0_pre_sel (rate 72000000, disabled)
                    ipu2_di0_pre (rate 24000000, disabled)
                        ipu2_di0_sel (rate 24000000, disabled)
                            ipu2_di0 (rate 24000000, disabled)
                ipu2_di1_pre_sel (rate 72000000, disabled)
                    ipu2_di1_pre (rate 36000000, disabled)
                        ipu2_di1_sel (rate 36000000, disabled)
                            ipu2_di1 (rate 36000000, disabled)
    pll8_mlb (rate 24000000, disabled)
    pll7_usb_host (rate 480000000, disabled)
        usbphy2 (rate 480000000, disabled)
    pll6_enet (rate 500000000, enabled)
        sata_ref (rate 100000000, enabled)
            sata_ref_100m (rate 100000000, enabled)
                lvds1_sel (rate 100000000, enabled)
                    lvds1_gate (rate 100000000, disabled)
        pcie_ref (rate 125000000, enabled)
            pcie_ref_125m (rate 125000000, disabled)
        enet_ref (rate 50000000, enabled)
    periph_clk2_sel (rate 24000000, enabled)
        periph_clk2 (rate 24000000, enabled)
ckih1 (rate 0, enabled)
ckil (rate 32768, enabled)



_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 0/2] spi: imx: don't loop endlessly

[Uwe Kleine-König]

On Wed, Aug 02, 2017 at 10:32:40PM +0200, Sam Ravnborg wrote:
> Hi Uwe.
> > I havn't looked at it, but it is included in my barebox as I'm using
> > 2017.08.0 + some patches.
> And I did not expect it to solve your problem, only to give you
> inspiration what could be the issue.
> 
> > 
> > Reading through the patch description, it doesn't match my situation.
> OK
> 
> > I'm doing USB-Booting because when I made eMMC work all my available
> > remote hands already called it a day. So unless the boot ROM does
> > something strange (impossible!) barebox is the first spi user.
> > 
> > What seems to be similar is that TESTREG.TXCNT is != 0. For me this
> > smells clk-related. On i.MX6 I recently identified a problem (but didn't
> > come around yet to mainline a fix) that might match at least your
> > problem.
> > 
> > Can you still reproduce?
> We have used the patch from Sascha for a long time now (from before it
> hit barebox-next, and on our proprietary IMX6 target we never
> saw the SPI related error since.
> 
> 	Sam
> 
> > How does your clk-tree look like (clk_dump)?
> 
> This is with:
> $ version
> barebox 2017.03.0-1 #1 Sun May 14 16:17:24 CEST 2017
> + some of our own patches (nothing clk related).

Which SoC are you using? Doesn't seem to match my problem. I don't know
what Sascha did, I'd route ecspi_root to cko2 and check if there is a
clk in the failing situation. (Assuming that pin is accessible on your
machine of course.)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 0/2] spi: imx: don't loop endlessly

[Sam Ravnborg]

Hi Uwe.

> Which SoC are you using? Doesn't seem to match my problem. I don't know
> what Sascha did, I'd route ecspi_root to cko2 and check if there is a
> clk in the failing situation. (Assuming that pin is accessible on your
> machine of course.)

The CPU is:
NXP i.MX 6 Single Core Cortex-A9 Industrial processor
MCIMX6S5EVM10AC
We also ahev a quadcore variant.
Both are a proprietary design, which have roots from a freescale reference design.

I can send you the schematics (in private mail) if you think it is usefull for you.
But as you already wrote, this is likely a different problem you are fighting with.

	Sam

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 0/2] spi: imx: don't loop endlessly

[Uwe Kleine-König]

On Wed, Aug 02, 2017 at 10:32:40PM +0200, Sam Ravnborg wrote:
> Hi Uwe.
> > I havn't looked at it, but it is included in my barebox as I'm using
> > 2017.08.0 + some patches.
> And I did not expect it to solve your problem, only to give you
> inspiration what could be the issue.
> 
> > 
> > Reading through the patch description, it doesn't match my situation.
> OK
> 
> > I'm doing USB-Booting because when I made eMMC work all my available
> > remote hands already called it a day. So unless the boot ROM does
> > something strange (impossible!) barebox is the first spi user.
> > 
> > What seems to be similar is that TESTREG.TXCNT is != 0. For me this
> > smells clk-related. On i.MX6 I recently identified a problem (but didn't
> > come around yet to mainline a fix) that might match at least your
> > problem.
> > 
> > Can you still reproduce?
> We have used the patch from Sascha for a long time now (from before it
> hit barebox-next, and on our proprietary IMX6 target we never
> saw the SPI related error since.
> 
> 	Sam
> 
> > How does your clk-tree look like (clk_dump)?
> 
> This is with:
> $ version
> barebox 2017.03.0-1 #1 Sun May 14 16:17:24 CEST 2017
> + some of our own patches (nothing clk related).
> 
> $ clk_dump
> [...]
>             ecspi_root (rate 60000000, enabled)
>                 ecspi1 (rate 60000000, enabled)
>                 ecspi2 (rate 60000000, enabled)
>                 ecspi3 (rate 60000000, enabled)
>                 ecspi4 (rate 60000000, enabled)
>                 ecspi5 (rate 60000000, enabled)

ecspi_root is defined as:

	clks[IMX6QDL_CLK_ECSPI_ROOT] = imx_clk_divider("ecspi_root", "pll3_60m", base + 0x38, 19, 6);

The RM (Reference Manual) describes this register as follows:

	CCM_CSCDR2, bits 24–19: ... NOTE: Divider should be updated when
	output clock is gated.

It seems the BOOTROM in some older revisions of the chips didn't respect
this requirement (for a different clk though) which resulted in
ERR007117 (described in the ChipErrata document for the i.XM6DQ):

	For raw NAND boot, ROM switches the source of enfc_clk_root from
	PLL2_PFD2 to PLL3. The root clock is required to be gated before
	switching the source clock. If the root clock is not gated, clock
	glitches might be passed to the divider that follows the clock mux, and
	the divider might behave unpredictably. This can cause the clock
	generation to fail and the chip will not boot successfully.
	
	This problem can also occur elsewhere in application code if the
	root clock is not properly gated when the clock configuration is
	changed.

IMHO this fits nicely to your error description. i.MX6UL is affected for
sure as it does

        clk_set_parent(clks[IMX6UL_CLK_ENFC_SEL], clks[IMX6UL_CLK_PLL2_PFD2]);

without making sure that some affected clks are off. This resulted in
failures to probe the NAND occasionally on a customer machine.

For i.MX6SDL or i.MX6DQ I don't see a matching change for the ecspi_root
clk, but maybe this is triggered by setting the frequency on one of the
child clocks?!

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 0/2] spi: imx: don't loop endlessly

[Alexander Kurz]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1211 bytes --]

Hi,
during xchg_single 32 bits will be sent and received:
2x32 bits / 10 Microseconds = 6.4MHz Clock.

Hence, a 10 Microseconds timeout will break SPI communication for
boards with SPI frequencies less then 6.4MHz.
On some boards spi-max-frequency is limited due to improper communication
at higher frequencies, e.g. for the kindle4 it is 1MHz and there 
also exists one board with 100kHz.

Before sending a patch calculating the timeout from spi-max-frequency,
is 640 Microseconds (to fit imx28-cfa10049.dts 100kHz) acceptable?

Regards, Alexander



On Wed, 2 Aug 2017, Uwe Kleine-König wrote:

> Hello,
> 
> during bringup of an i.MX7 board I am faced with cspi_2_3_xchg_single not
> returning. I don't know yet why this happens, but with this patch set it at least
> doesn't block barebox.
> 
> I didn't test on a working board, maybe the timeout (10 ?s) I chose is too tight?
> 
> Best regards
> Uwe
> 
> Uwe Kleine-K?nig (2):
>   spi: imx: add error checking
>   spi: imx: add timeout to xchg_single
> 
>  drivers/spi/imx_spi.c | 105 ++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 72 insertions(+), 33 deletions(-)
> 
> -- 
> 2.11.0
> 
> 
> 

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 0/2] spi: imx: don't loop endlessly

[Uwe Kleine-König]

On Fri, Sep 29, 2017 at 08:08:28PM +0200, Alexander Kurz wrote:
> Hi,
> during xchg_single 32 bits will be sent and received:
> 2x32 bits / 10 Microseconds = 6.4MHz Clock.

why 2x?

> Hence, a 10 Microseconds timeout will break SPI communication for
> boards with SPI frequencies less then 6.4MHz.
> On some boards spi-max-frequency is limited due to improper communication
> at higher frequencies, e.g. for the kindle4 it is 1MHz and there 
> also exists one board with 100kHz.
> 
> Before sending a patch calculating the timeout from spi-max-frequency,
> is 640 Microseconds (to fit imx28-cfa10049.dts 100kHz) acceptable?

I would expect that the hardware and software add some latency, so maybe
round it up to 1 ms?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox