| Message ID | 1583839922-22699-1-git-send-email-yibin.gong@nxp.com |
|---|---|
| Headers | show |
| Series | add ecspi ERR009165 for i.mx6/7 soc family | expand |
On Tue, Mar 10, 2020 at 07:31:55PM +0800, Robin Gong wrote: > Change to XCH mode even in dma mode, please refer to the below > errata: > https://www.nxp.com/docs/en/errata/IMX6DQCE.pdf > > Signed-off-by: Robin Gong <yibin.gong@nxp.com> > Acked-by: Mark Brown <broonie@kernel.org> > --- > drivers/spi/spi-imx.c | 17 ++++++++++------- > 1 file changed, 10 insertions(+), 7 deletions(-) > > diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c > index f4f28a4..842a86e 100644 > --- a/drivers/spi/spi-imx.c > +++ b/drivers/spi/spi-imx.c > @@ -585,8 +585,9 @@ static int mx51_ecspi_prepare_transfer(struct spi_imx_data *spi_imx, > ctrl |= mx51_ecspi_clkdiv(spi_imx, t->speed_hz, &clk); > spi_imx->spi_bus_clk = clk; > > + /* ERR009165: work in XHC mode as PIO */ > if (spi_imx->usedma) > - ctrl |= MX51_ECSPI_CTRL_SMC; > + ctrl &= ~MX51_ECSPI_CTRL_SMC; 'ctrl' was read from the hardware. In the dma case it was set explicitly, but it was never cleared for a PIO transfer. This looked wrong before this patch. Now with this patch it looks even more wrong: We clear a bit that has never been set and we only do this for DMA, when for the PIO case it definitly must be cleared. Drop the if clause. > > writel(ctrl, spi_imx->base + MX51_ECSPI_CTRL); > > @@ -612,12 +613,14 @@ static int mx51_ecspi_prepare_transfer(struct spi_imx_data *spi_imx, > > static void mx51_setup_wml(struct spi_imx_data *spi_imx) > { > + u32 tx_wml = 0; > + > /* > * Configure the DMA register: setup the watermark > * and enable DMA request. > */ > writel(MX51_ECSPI_DMA_RX_WML(spi_imx->wml - 1) | > - MX51_ECSPI_DMA_TX_WML(spi_imx->wml) | > + MX51_ECSPI_DMA_TX_WML(tx_wml) | tx_wml is never assigned any other value than 0. Drop the variable. > MX51_ECSPI_DMA_RXT_WML(spi_imx->wml) | > MX51_ECSPI_DMA_TEDEN | MX51_ECSPI_DMA_RXDEN | > MX51_ECSPI_DMA_RXTDEN, spi_imx->base + MX51_ECSPI_DMA); > @@ -1171,7 +1174,11 @@ static int spi_imx_dma_configure(struct spi_master *master) > tx.direction = DMA_MEM_TO_DEV; > tx.dst_addr = spi_imx->base_phys + MXC_CSPITXDATA; > tx.dst_addr_width = buswidth; > - tx.dst_maxburst = spi_imx->wml; > + /* > + * For ERR009165 with tx_wml = 0 could enlarge burst size to fifo size > + * to speed up fifo filling as possible. > + */ > + tx.dst_maxburst = spi_imx->devtype_data->fifo_size; > ret = dmaengine_slave_config(master->dma_tx, &tx); > if (ret) { > dev_err(spi_imx->dev, "TX dma configuration failed with %d\n", ret); > @@ -1265,10 +1272,6 @@ static int spi_imx_sdma_init(struct device *dev, struct spi_imx_data *spi_imx, > { > int ret; > > - /* use pio mode for i.mx6dl chip TKT238285 */ > - if (of_machine_is_compatible("fsl,imx6dl")) > - return 0; So with this patch it becomes possible to do DMA on i.MX6dl, but it is mentioned nowhere. Sascha
On Tue, Mar 10, 2020 at 07:31:56PM +0800, Robin Gong wrote: > ERR009165 fixed on i.mx6ul/6ull/6sll. All other i.mx6/7 and > i.mx8m/8mm still need this errata. Please refer to nxp official > errata document from https://www.nxp.com/ . > > For removing workaround on those chips. Add new i.mx6ul type. > > Signed-off-by: Robin Gong <yibin.gong@nxp.com> > Acked-by: Mark Brown <broonie@kernel.org> > --- > drivers/spi/spi-imx.c | 50 +++++++++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 45 insertions(+), 5 deletions(-) > > diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c > index 842a86e..f7ee2ec 100644 > --- a/drivers/spi/spi-imx.c > +++ b/drivers/spi/spi-imx.c > @@ -57,6 +57,7 @@ enum spi_imx_devtype { > IMX35_CSPI, /* CSPI on all i.mx except above */ > IMX51_ECSPI, /* ECSPI on i.mx51 */ > IMX53_ECSPI, /* ECSPI on i.mx53 and later */ > + IMX6UL_ECSPI, /* ERR009165 fix from i.mx6ul */ > }; > > struct spi_imx_data; > @@ -75,6 +76,11 @@ struct spi_imx_devtype_data { > bool has_slavemode; > unsigned int fifo_size; > bool dynamic_burst; > + /* > + * ERR009165 fixed or not: > + * https://www.nxp.com/docs/en/errata/IMX6DQCE.pdf > + */ > + bool tx_glitch_fixed; > enum spi_imx_devtype devtype; > }; > > @@ -128,7 +134,8 @@ static inline int is_imx35_cspi(struct spi_imx_data *d) > > static inline int is_imx51_ecspi(struct spi_imx_data *d) > { > - return d->devtype_data->devtype == IMX51_ECSPI; > + return d->devtype_data->devtype == IMX51_ECSPI || > + d->devtype_data->devtype == IMX6UL_ECSPI; > } Erm, no. A i.MX51 ECSPI is a i.MX51 ECSPI and not a i.MX6UL ECSPI. If you want to handle them equally somewhere then explicitly test for i.MX6ul *and* i.MX51 there. > > static inline int is_imx53_ecspi(struct spi_imx_data *d) > @@ -585,9 +592,16 @@ static int mx51_ecspi_prepare_transfer(struct spi_imx_data *spi_imx, > ctrl |= mx51_ecspi_clkdiv(spi_imx, t->speed_hz, &clk); > spi_imx->spi_bus_clk = clk; > > - /* ERR009165: work in XHC mode as PIO */ > - if (spi_imx->usedma) > - ctrl &= ~MX51_ECSPI_CTRL_SMC; > + /* > + * ERR009165: work in XHC mode instead of SMC as PIO on the chips > + * before i.mx6ul. > + */ > + if (spi_imx->usedma) { > + if (spi_imx->devtype_data->tx_glitch_fixed) > + ctrl |= MX51_ECSPI_CTRL_SMC; > + else > + ctrl &= ~MX51_ECSPI_CTRL_SMC; > + } Changed again, but the PIO case still not honoured. This should look like if (spi_imx->usedma && spi_imx->devtype_data->tx_glitch_fixed) ctrl |= MX51_ECSPI_CTRL_SMC; else ctrl &= ~MX51_ECSPI_CTRL_SMC; > > writel(ctrl, spi_imx->base + MX51_ECSPI_CTRL); > > @@ -615,6 +629,8 @@ static void mx51_setup_wml(struct spi_imx_data *spi_imx) > { > u32 tx_wml = 0; > > + if (spi_imx->devtype_data->tx_glitch_fixed) > + tx_wml = spi_imx->wml; That explains the variable introduced in the last patch, ok. I have the impression that splitting up 06/13 and 07/13 into two patches doesn't make it easier to review. Sascha
On Tue, Mar 10, 2020 at 07:31:58PM +0800, Robin Gong wrote: > ECSPI issue fixed from i.mx6ul at hardware level, no need > ERR009165 anymore on those chips such as i.mx8mq. Add i.mx6sx > from where i.mx6ul source. > > Signed-off-by: Robin Gong <yibin.gong@nxp.com> > Acked-by: Vinod Koul <vkoul@kernel.org> > --- > drivers/dma/imx-sdma.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 50 insertions(+), 1 deletion(-) > > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c > index 56288d8..5ae7237 100644 > --- a/drivers/dma/imx-sdma.c > +++ b/drivers/dma/imx-sdma.c > @@ -419,6 +419,13 @@ struct sdma_driver_data { > int num_events; > struct sdma_script_start_addrs *script_addrs; > bool check_ratio; > + /* > + * ecspi ERR009165 fixed should be done in sdma script > + * and it has been fixed in soc from i.mx6ul. > + * please get more information from the below link: > + * https://www.nxp.com/docs/en/errata/IMX6DQCE.pdf > + */ > + bool ecspi_fixed; > }; > > struct sdma_engine { > @@ -539,6 +546,31 @@ static struct sdma_driver_data sdma_imx6q = { > .script_addrs = &sdma_script_imx6q, > }; > > +static struct sdma_script_start_addrs sdma_script_imx6sx = { > + .ap_2_ap_addr = 642, > + .uart_2_mcu_addr = 817, > + .mcu_2_app_addr = 747, > + .uartsh_2_mcu_addr = 1032, > + .mcu_2_shp_addr = 960, > + .app_2_mcu_addr = 683, > + .shp_2_mcu_addr = 891, > + .spdif_2_mcu_addr = 1100, > + .mcu_2_spdif_addr = 1134, > +}; > + > +static struct sdma_driver_data sdma_imx6sx = { > + .chnenbl0 = SDMA_CHNENBL0_IMX35, > + .num_events = 48, > + .script_addrs = &sdma_script_imx6sx, > +}; > + > +static struct sdma_driver_data sdma_imx6ul = { > + .chnenbl0 = SDMA_CHNENBL0_IMX35, > + .num_events = 48, > + .script_addrs = &sdma_script_imx6sx, > + .ecspi_fixed = true, > +}; > + > static struct sdma_script_start_addrs sdma_script_imx7d = { > .ap_2_ap_addr = 644, > .uart_2_mcu_addr = 819, > @@ -584,9 +616,15 @@ static const struct platform_device_id sdma_devtypes[] = { > .name = "imx6q-sdma", > .driver_data = (unsigned long)&sdma_imx6q, > }, { > + .name = "imx6sx-sdma", > + .driver_data = (unsigned long)&sdma_imx6sx, > + }, { Now the i.MX6sx uses a new sdma_script_start_addrs entry which is the same as the i.MX6q one we used before with one exception: it lacks the per_2_per_addr = 6331 entry. This is only used for IMX_DMATYPE_ASRC and IMX_DMATYPE_ASRC_SP, both are entirely unused in the mainline kernel. So why must the i.MX6sx changed here and what has this to do with ECSPI? Sascha > .name = "imx7d-sdma", > .driver_data = (unsigned long)&sdma_imx7d, > }, { > + .name = "imx6ul-sdma", > + .driver_data = (unsigned long)&sdma_imx6ul, > + }, { > .name = "imx8mq-sdma", > .driver_data = (unsigned long)&sdma_imx8mq, > }, { > @@ -602,7 +640,9 @@ static const struct of_device_id sdma_dt_ids[] = { > { .compatible = "fsl,imx35-sdma", .data = &sdma_imx35, }, > { .compatible = "fsl,imx31-sdma", .data = &sdma_imx31, }, > { .compatible = "fsl,imx25-sdma", .data = &sdma_imx25, }, > + { .compatible = "fsl,imx6sx-sdma", .data = &sdma_imx6sx, }, > { .compatible = "fsl,imx7d-sdma", .data = &sdma_imx7d, }, > + { .compatible = "fsl,imx6ul-sdma", .data = &sdma_imx6ul, }, > { .compatible = "fsl,imx8mq-sdma", .data = &sdma_imx8mq, }, > { /* sentinel */ } > }; > @@ -1169,8 +1209,17 @@ static int sdma_config_channel(struct dma_chan *chan) > if (sdmac->peripheral_type == IMX_DMATYPE_ASRC_SP || > sdmac->peripheral_type == IMX_DMATYPE_ASRC) > sdma_set_watermarklevel_for_p2p(sdmac); > - } else > + } else { > + /* > + * ERR009165 fixed from i.mx6ul, no errata need, > + * set bit31 to let sdma script skip the errata. > + */ > + if (sdmac->peripheral_type == IMX_DMATYPE_CSPI && > + sdmac->direction == DMA_MEM_TO_DEV && > + sdmac->sdma->drvdata->ecspi_fixed) > + __set_bit(31, &sdmac->watermark_level); > __set_bit(sdmac->event_id0, sdmac->event_mask); > + } > > /* Address */ > sdmac->shp_addr = sdmac->per_address; > -- > 2.7.4 > >
On 2020/03/10 Sascha Hauer <s.hauer@pengutronix.de> wrote: > On Tue, Mar 10, 2020 at 07:31:55PM +0800, Robin Gong wrote: > > Change to XCH mode even in dma mode, please refer to the below > > errata: > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww. > > > nxp.com%2Fdocs%2Fen%2Ferrata%2FIMX6DQCE.pdf&data=02%7C01%7C > yibin.g > > > ong%40nxp.com%7Ccbabce268dfd4b0a0e2a08d7c4c62ff6%7C686ea1d3bc2b4c > 6fa92 > > > cd99c5c301635%7C0%7C1%7C637194227793913712&sdata=Q5N49T4jgX > TcdTzsB > > 3D0saK2%2Fzj0R4gnJcGR%2Bd70Fm4%3D&reserved=0 > > > > Signed-off-by: Robin Gong <yibin.gong@nxp.com> > > Acked-by: Mark Brown <broonie@kernel.org> > > --- > > drivers/spi/spi-imx.c | 17 ++++++++++------- > > 1 file changed, 10 insertions(+), 7 deletions(-) > > > > diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index > > f4f28a4..842a86e 100644 > > --- a/drivers/spi/spi-imx.c > > +++ b/drivers/spi/spi-imx.c > > @@ -585,8 +585,9 @@ static int mx51_ecspi_prepare_transfer(struct > spi_imx_data *spi_imx, > > ctrl |= mx51_ecspi_clkdiv(spi_imx, t->speed_hz, &clk); > > spi_imx->spi_bus_clk = clk; > > > > + /* ERR009165: work in XHC mode as PIO */ > > if (spi_imx->usedma) > > - ctrl |= MX51_ECSPI_CTRL_SMC; > > + ctrl &= ~MX51_ECSPI_CTRL_SMC; > > 'ctrl' was read from the hardware. In the dma case it was set explicitly, but it > was never cleared for a PIO transfer. This looked wrong before this patch. Now > with this patch it looks even more wrong: > We clear a bit that has never been set and we only do this for DMA, when for > the PIO case it definitly must be cleared. Drop the if clause. Good point, ACK. > > > > > writel(ctrl, spi_imx->base + MX51_ECSPI_CTRL); > > > > @@ -612,12 +613,14 @@ static int mx51_ecspi_prepare_transfer(struct > > spi_imx_data *spi_imx, > > > > static void mx51_setup_wml(struct spi_imx_data *spi_imx) { > > + u32 tx_wml = 0; > > + > > /* > > * Configure the DMA register: setup the watermark > > * and enable DMA request. > > */ > > writel(MX51_ECSPI_DMA_RX_WML(spi_imx->wml - 1) | > > - MX51_ECSPI_DMA_TX_WML(spi_imx->wml) | > > + MX51_ECSPI_DMA_TX_WML(tx_wml) | > > tx_wml is never assigned any other value than 0. Drop the variable. That's prepared for 07/13 patch which may assign spi_imx->wml to tx_wml. > > > MX51_ECSPI_DMA_RXT_WML(spi_imx->wml) | > > MX51_ECSPI_DMA_TEDEN | MX51_ECSPI_DMA_RXDEN | > > MX51_ECSPI_DMA_RXTDEN, spi_imx->base + MX51_ECSPI_DMA); > @@ -1171,7 > > +1174,11 @@ static int spi_imx_dma_configure(struct spi_master *master) > > tx.direction = DMA_MEM_TO_DEV; > > tx.dst_addr = spi_imx->base_phys + MXC_CSPITXDATA; > > tx.dst_addr_width = buswidth; > > - tx.dst_maxburst = spi_imx->wml; > > + /* > > + * For ERR009165 with tx_wml = 0 could enlarge burst size to fifo size > > + * to speed up fifo filling as possible. > > + */ > > + tx.dst_maxburst = spi_imx->devtype_data->fifo_size; > > ret = dmaengine_slave_config(master->dma_tx, &tx); > > if (ret) { > > dev_err(spi_imx->dev, "TX dma configuration failed with %d\n", > > ret); @@ -1265,10 +1272,6 @@ static int spi_imx_sdma_init(struct > > device *dev, struct spi_imx_data *spi_imx, { > > int ret; > > > > - /* use pio mode for i.mx6dl chip TKT238285 */ > > - if (of_machine_is_compatible("fsl,imx6dl")) > > - return 0; > > So with this patch it becomes possible to do DMA on i.MX6dl, but it is > mentioned nowhere. That's a common IP issue but caught on i.mx6dl at that time, so this time I didn't mention i.mx6dl. > > Sascha > > -- > Pengutronix e.K. | > | > Steuerwalder Str. 21 | > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pe > ngutronix.de%2F&data=02%7C01%7Cyibin.gong%40nxp.com%7Ccbabce2 > 68dfd4b0a0e2a08d7c4c62ff6%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0% > 7C1%7C637194227793913712&sdata=TL%2BheiNsYVPld3qyzWjF6yQZgH2 > HLdVFxzFeK3MupTc%3D&reserved=0 | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 > | > Amtsgericht Hildesheim, HRA 2686 | Fax: > +49-5121-206917-5555 |
Hello, On Tue, Mar 10, 2020 at 08:27:41AM +0000, Robin Gong wrote: > On 2020/03/10 Sascha Hauer <s.hauer@pengutronix.de> wrote: > > On Tue, Mar 10, 2020 at 07:31:55PM +0800, Robin Gong wrote: > > > writel(ctrl, spi_imx->base + MX51_ECSPI_CTRL); > > > > > > @@ -612,12 +613,14 @@ static int mx51_ecspi_prepare_transfer(struct > > > spi_imx_data *spi_imx, > > > > > > static void mx51_setup_wml(struct spi_imx_data *spi_imx) { > > > + u32 tx_wml = 0; > > > + > > > /* > > > * Configure the DMA register: setup the watermark > > > * and enable DMA request. > > > */ > > > writel(MX51_ECSPI_DMA_RX_WML(spi_imx->wml - 1) | > > > - MX51_ECSPI_DMA_TX_WML(spi_imx->wml) | > > > + MX51_ECSPI_DMA_TX_WML(tx_wml) | > > > > tx_wml is never assigned any other value than 0. Drop the variable. > That's prepared for 07/13 patch which may assign spi_imx->wml to tx_wml. Then this belongs into patch 7, right? Best regards Uwe
On 2020/03/10 Sascha Hauer <s.hauer@pengutronix.de> wrote: > On Tue, Mar 10, 2020 at 07:31:56PM +0800, Robin Gong wrote: > > ERR009165 fixed on i.mx6ul/6ull/6sll. All other i.mx6/7 and i.mx8m/8mm > > still need this errata. Please refer to nxp official errata document > > from > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.nx > p.com%2F&data=02%7C01%7Cyibin.gong%40nxp.com%7Cf73bfc11a68c4 > 2f5f6d308d7c4c96efa%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C > 637194241755109112&sdata=xzIUP8qZkrlDXX0yjTcUNZB6zDrevTdHFg1o4 > PZZd8E%3D&reserved=0 . > > > > For removing workaround on those chips. Add new i.mx6ul type. > > > > Signed-off-by: Robin Gong <yibin.gong@nxp.com> > > Acked-by: Mark Brown <broonie@kernel.org> > > --- > > drivers/spi/spi-imx.c | 50 > > +++++++++++++++++++++++++++++++++++++++++++++----- > > 1 file changed, 45 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index > > 842a86e..f7ee2ec 100644 > > --- a/drivers/spi/spi-imx.c > > +++ b/drivers/spi/spi-imx.c > > @@ -57,6 +57,7 @@ enum spi_imx_devtype { > > IMX35_CSPI, /* CSPI on all i.mx except above */ > > IMX51_ECSPI, /* ECSPI on i.mx51 */ > > IMX53_ECSPI, /* ECSPI on i.mx53 and later */ > > + IMX6UL_ECSPI, /* ERR009165 fix from i.mx6ul */ > > }; > > > > struct spi_imx_data; > > @@ -75,6 +76,11 @@ struct spi_imx_devtype_data { > > bool has_slavemode; > > unsigned int fifo_size; > > bool dynamic_burst; > > + /* > > + * ERR009165 fixed or not: > > + * > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.nx > p.com%2Fdocs%2Fen%2Ferrata%2FIMX6DQCE.pdf&data=02%7C01%7Cyi > bin.gong%40nxp.com%7Cf73bfc11a68c42f5f6d308d7c4c96efa%7C686ea1d3bc > 2b4c6fa92cd99c5c301635%7C0%7C1%7C637194241755109112&sdata=m > uw4HL5nMDjREJwVd885Wrxka0moMaaZ%2BhJgsAgY3eo%3D&reserved= > 0 > > + */ > > + bool tx_glitch_fixed; > > enum spi_imx_devtype devtype; > > }; > > > > @@ -128,7 +134,8 @@ static inline int is_imx35_cspi(struct > > spi_imx_data *d) > > > > static inline int is_imx51_ecspi(struct spi_imx_data *d) { > > - return d->devtype_data->devtype == IMX51_ECSPI; > > + return d->devtype_data->devtype == IMX51_ECSPI || > > + d->devtype_data->devtype == IMX6UL_ECSPI; > > } > > Erm, no. A i.MX51 ECSPI is a i.MX51 ECSPI and not a i.MX6UL ECSPI. If you want > to handle them equally somewhere then explicitly test for i.MX6ul *and* > i.MX51 there. But all i.mx6 chips including i.MX53 ECSPI are almost same as i.MX51 ECSPI, and ERR00915 is fixed from i.mx6ul.... > > > > > static inline int is_imx53_ecspi(struct spi_imx_data *d) @@ -585,9 > > +592,16 @@ static int mx51_ecspi_prepare_transfer(struct spi_imx_data > *spi_imx, > > ctrl |= mx51_ecspi_clkdiv(spi_imx, t->speed_hz, &clk); > > spi_imx->spi_bus_clk = clk; > > > > - /* ERR009165: work in XHC mode as PIO */ > > - if (spi_imx->usedma) > > - ctrl &= ~MX51_ECSPI_CTRL_SMC; > > + /* > > + * ERR009165: work in XHC mode instead of SMC as PIO on the chips > > + * before i.mx6ul. > > + */ > > + if (spi_imx->usedma) { > > + if (spi_imx->devtype_data->tx_glitch_fixed) > > + ctrl |= MX51_ECSPI_CTRL_SMC; > > + else > > + ctrl &= ~MX51_ECSPI_CTRL_SMC; > > + } > > Changed again, but the PIO case still not honoured. This should look like > if (spi_imx->usedma && spi_imx->devtype_data->tx_glitch_fixed) > ctrl |= MX51_ECSPI_CTRL_SMC; > else > ctrl &= ~MX51_ECSPI_CTRL_SMC; > Okay, will fix in v7. > > > > writel(ctrl, spi_imx->base + MX51_ECSPI_CTRL); > > > > @@ -615,6 +629,8 @@ static void mx51_setup_wml(struct spi_imx_data > > *spi_imx) { > > u32 tx_wml = 0; > > > > + if (spi_imx->devtype_data->tx_glitch_fixed) > > + tx_wml = spi_imx->wml; > > That explains the variable introduced in the last patch, ok. > > I have the impression that splitting up 06/13 and 07/13 into two patches > doesn't make it easier to review. But 06 is a errata for all i.mx6 legacy chips, while 07 is for i.mx6ul and newer chips which have been already fixed the HW issue. I think two patches are better. > > Sascha > > > -- > Pengutronix e.K. | > | > Steuerwalder Str. 21 | > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pe > ngutronix.de%2F&data=02%7C01%7Cyibin.gong%40nxp.com%7Cf73bfc11 > a68c42f5f6d308d7c4c96efa%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7 > C1%7C637194241755114106&sdata=DKe%2B2SynMv%2Be3rMBrO79ou6 > 5ADTwO03KRT%2FqsDbCWjc%3D&reserved=0 | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 > | > Amtsgericht Hildesheim, HRA 2686 | Fax: > +49-5121-206917-5555 |
On 2020/03/10 Sascha Hauer <s.hauer@pengutronix.de> wrote: > On Tue, Mar 10, 2020 at 07:31:58PM +0800, Robin Gong wrote: > > ECSPI issue fixed from i.mx6ul at hardware level, no need > > ERR009165 anymore on those chips such as i.mx8mq. Add i.mx6sx from > > where i.mx6ul source. > > > > Signed-off-by: Robin Gong <yibin.gong@nxp.com> > > Acked-by: Vinod Koul <vkoul@kernel.org> > > --- > > drivers/dma/imx-sdma.c | 51 > > +++++++++++++++++++++++++++++++++++++++++++++++++- > > 1 file changed, 50 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c index > > 56288d8..5ae7237 100644 > > --- a/drivers/dma/imx-sdma.c > > +++ b/drivers/dma/imx-sdma.c > > @@ -419,6 +419,13 @@ struct sdma_driver_data { > > int num_events; > > struct sdma_script_start_addrs *script_addrs; > > bool check_ratio; > > + /* > > + * ecspi ERR009165 fixed should be done in sdma script > > + * and it has been fixed in soc from i.mx6ul. > > + * please get more information from the below link: > > + * > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.nx > p.com%2Fdocs%2Fen%2Ferrata%2FIMX6DQCE.pdf&data=02%7C01%7Cyi > bin.gong%40nxp.com%7C91d42046e6894501d48508d7c4cbcae2%7C686ea1d3 > bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637194251876090170&sdata= > T6LA4xz9CUFlNpnyjHSThEQb8i1rhbY9e1nUyxIGD5Q%3D&reserved=0 > > + */ > > + bool ecspi_fixed; > > }; > > > > struct sdma_engine { > > @@ -539,6 +546,31 @@ static struct sdma_driver_data sdma_imx6q = { > > .script_addrs = &sdma_script_imx6q, > > }; > > > > +static struct sdma_script_start_addrs sdma_script_imx6sx = { > > + .ap_2_ap_addr = 642, > > + .uart_2_mcu_addr = 817, > > + .mcu_2_app_addr = 747, > > + .uartsh_2_mcu_addr = 1032, > > + .mcu_2_shp_addr = 960, > > + .app_2_mcu_addr = 683, > > + .shp_2_mcu_addr = 891, > > + .spdif_2_mcu_addr = 1100, > > + .mcu_2_spdif_addr = 1134, > > +}; > > + > > +static struct sdma_driver_data sdma_imx6sx = { > > + .chnenbl0 = SDMA_CHNENBL0_IMX35, > > + .num_events = 48, > > + .script_addrs = &sdma_script_imx6sx, }; > > + > > +static struct sdma_driver_data sdma_imx6ul = { > > + .chnenbl0 = SDMA_CHNENBL0_IMX35, > > + .num_events = 48, > > + .script_addrs = &sdma_script_imx6sx, > > + .ecspi_fixed = true, > > +}; > > + > > static struct sdma_script_start_addrs sdma_script_imx7d = { > > .ap_2_ap_addr = 644, > > .uart_2_mcu_addr = 819, > > @@ -584,9 +616,15 @@ static const struct platform_device_id > sdma_devtypes[] = { > > .name = "imx6q-sdma", > > .driver_data = (unsigned long)&sdma_imx6q, > > }, { > > + .name = "imx6sx-sdma", > > + .driver_data = (unsigned long)&sdma_imx6sx, > > + }, { > > Now the i.MX6sx uses a new sdma_script_start_addrs entry which is the same > as the i.MX6q one we used before with one exception: it lacks the > per_2_per_addr = 6331 entry. This is only used for IMX_DMATYPE_ASRC and Totally same script for i.mx6 chips whatever i.MX6sx, i.MX6q or i.MX6ul. > IMX_DMATYPE_ASRC_SP, both are entirely unused in the mainline kernel. So > why must the i.MX6sx changed here and what has this to do with ECSPI? i.MX6ul is based on i.MX6sx, so adding i.MX6sx could keep good shape on our i.MX family evolution. > > Sascha > > > .name = "imx7d-sdma", > > .driver_data = (unsigned long)&sdma_imx7d, > > }, { > > + .name = "imx6ul-sdma", > > + .driver_data = (unsigned long)&sdma_imx6ul, > > + }, { > > .name = "imx8mq-sdma", > > .driver_data = (unsigned long)&sdma_imx8mq, > > }, { > > @@ -602,7 +640,9 @@ static const struct of_device_id sdma_dt_ids[] = { > > { .compatible = "fsl,imx35-sdma", .data = &sdma_imx35, }, > > { .compatible = "fsl,imx31-sdma", .data = &sdma_imx31, }, > > { .compatible = "fsl,imx25-sdma", .data = &sdma_imx25, }, > > + { .compatible = "fsl,imx6sx-sdma", .data = &sdma_imx6sx, }, > > { .compatible = "fsl,imx7d-sdma", .data = &sdma_imx7d, }, > > + { .compatible = "fsl,imx6ul-sdma", .data = &sdma_imx6ul, }, > > { .compatible = "fsl,imx8mq-sdma", .data = &sdma_imx8mq, }, > > { /* sentinel */ } > > }; > > @@ -1169,8 +1209,17 @@ static int sdma_config_channel(struct dma_chan > *chan) > > if (sdmac->peripheral_type == IMX_DMATYPE_ASRC_SP || > > sdmac->peripheral_type == IMX_DMATYPE_ASRC) > > sdma_set_watermarklevel_for_p2p(sdmac); > > - } else > > + } else { > > + /* > > + * ERR009165 fixed from i.mx6ul, no errata need, > > + * set bit31 to let sdma script skip the errata. > > + */ > > + if (sdmac->peripheral_type == IMX_DMATYPE_CSPI && > > + sdmac->direction == DMA_MEM_TO_DEV && > > + sdmac->sdma->drvdata->ecspi_fixed) > > + __set_bit(31, &sdmac->watermark_level); > > __set_bit(sdmac->event_id0, sdmac->event_mask); > > + } > > > > /* Address */ > > sdmac->shp_addr = sdmac->per_address; > > -- > > 2.7.4 > > > > > > -- > Pengutronix e.K. | > | > Steuerwalder Str. 21 | > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pe > ngutronix.de%2F&data=02%7C01%7Cyibin.gong%40nxp.com%7C91d4204 > 6e6894501d48508d7c4cbcae2%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0 > %7C1%7C637194251876090170&sdata=d0X1DBm%2Fc0lwcHVUtTW0ITw > o5qW3SKtVd5Va0EVVw3E%3D&reserved=0 | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 > | > Amtsgericht Hildesheim, HRA 2686 | Fax: > +49-5121-206917-5555 |
On 2020/03/10 Uwe Kleine-König <u.kleine-koenig@pengutronix.de> wrote: > > Hello, > > On Tue, Mar 10, 2020 at 08:27:41AM +0000, Robin Gong wrote: > > On 2020/03/10 Sascha Hauer <s.hauer@pengutronix.de> wrote: > > > On Tue, Mar 10, 2020 at 07:31:55PM +0800, Robin Gong wrote: > > > > writel(ctrl, spi_imx->base + MX51_ECSPI_CTRL); > > > > > > > > @@ -612,12 +613,14 @@ static int mx51_ecspi_prepare_transfer(struct > > > > spi_imx_data *spi_imx, > > > > > > > > static void mx51_setup_wml(struct spi_imx_data *spi_imx) { > > > > + u32 tx_wml = 0; > > > > + > > > > /* > > > > * Configure the DMA register: setup the watermark > > > > * and enable DMA request. > > > > */ > > > > writel(MX51_ECSPI_DMA_RX_WML(spi_imx->wml - 1) | > > > > - MX51_ECSPI_DMA_TX_WML(spi_imx->wml) | > > > > + MX51_ECSPI_DMA_TX_WML(tx_wml) | > > > > > > tx_wml is never assigned any other value than 0. Drop the variable. > > That's prepared for 07/13 patch which may assign spi_imx->wml to tx_wml. > > Then this belongs into patch 7, right? Okay, understood your concern. Then I'll drop tx_wml to make it clear. > > Best regards > Uwe > > -- > Pengutronix e.K. | Uwe Kleine-König > | > Industrial Linux Solutions | > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.p > engutronix.de%2F&data=02%7C01%7Cyibin.gong%40nxp.com%7Cca6f14 > 28f4224fa79f1d08d7c4ce4a41%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0 > %7C0%7C637194262606339591&sdata=kI9HAyC%2FG3qmrIrLmfUDJ7dac > %2FOEDBtDm4oeRyak1xE%3D&reserved=0 |
On Tue, Mar 10, 2020 at 08:43:10AM +0000, Robin Gong wrote: > On 2020/03/10 Sascha Hauer <s.hauer@pengutronix.de> wrote: > > On Tue, Mar 10, 2020 at 07:31:56PM +0800, Robin Gong wrote: > > > ERR009165 fixed on i.mx6ul/6ull/6sll. All other i.mx6/7 and i.mx8m/8mm > > > still need this errata. Please refer to nxp official errata document > > > from > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.nx > > p.com%2F&data=02%7C01%7Cyibin.gong%40nxp.com%7Cf73bfc11a68c4 > > 2f5f6d308d7c4c96efa%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C > > 637194241755109112&sdata=xzIUP8qZkrlDXX0yjTcUNZB6zDrevTdHFg1o4 > > PZZd8E%3D&reserved=0 . > > > > > > For removing workaround on those chips. Add new i.mx6ul type. > > > > > > Signed-off-by: Robin Gong <yibin.gong@nxp.com> > > > Acked-by: Mark Brown <broonie@kernel.org> > > > --- > > > drivers/spi/spi-imx.c | 50 > > > +++++++++++++++++++++++++++++++++++++++++++++----- > > > 1 file changed, 45 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index > > > 842a86e..f7ee2ec 100644 > > > --- a/drivers/spi/spi-imx.c > > > +++ b/drivers/spi/spi-imx.c > > > @@ -57,6 +57,7 @@ enum spi_imx_devtype { > > > IMX35_CSPI, /* CSPI on all i.mx except above */ > > > IMX51_ECSPI, /* ECSPI on i.mx51 */ > > > IMX53_ECSPI, /* ECSPI on i.mx53 and later */ > > > + IMX6UL_ECSPI, /* ERR009165 fix from i.mx6ul */ > > > }; > > > > > > struct spi_imx_data; > > > @@ -75,6 +76,11 @@ struct spi_imx_devtype_data { > > > bool has_slavemode; > > > unsigned int fifo_size; > > > bool dynamic_burst; > > > + /* > > > + * ERR009165 fixed or not: > > > + * > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.nx > > p.com%2Fdocs%2Fen%2Ferrata%2FIMX6DQCE.pdf&data=02%7C01%7Cyi > > bin.gong%40nxp.com%7Cf73bfc11a68c42f5f6d308d7c4c96efa%7C686ea1d3bc > > 2b4c6fa92cd99c5c301635%7C0%7C1%7C637194241755109112&sdata=m > > uw4HL5nMDjREJwVd885Wrxka0moMaaZ%2BhJgsAgY3eo%3D&reserved= > > 0 > > > + */ > > > + bool tx_glitch_fixed; > > > enum spi_imx_devtype devtype; > > > }; > > > > > > @@ -128,7 +134,8 @@ static inline int is_imx35_cspi(struct > > > spi_imx_data *d) > > > > > > static inline int is_imx51_ecspi(struct spi_imx_data *d) { > > > - return d->devtype_data->devtype == IMX51_ECSPI; > > > + return d->devtype_data->devtype == IMX51_ECSPI || > > > + d->devtype_data->devtype == IMX6UL_ECSPI; > > > } > > > > Erm, no. A i.MX51 ECSPI is a i.MX51 ECSPI and not a i.MX6UL ECSPI. If you want > > to handle them equally somewhere then explicitly test for i.MX6ul *and* > > i.MX51 there. > But all i.mx6 chips including i.MX53 ECSPI are almost same as i.MX51 ECSPI, and ERR00915 is fixed from i.mx6ul.... You introduce .devtype = IMX6UL_ECSPI in this series, so apparently it is *not* the same as IMX51_ECSPI, then please also don't introduce a function which claims they are the same. Sascha
On Tue, Mar 10, 2020 at 08:59:03AM +0000, Robin Gong wrote: > On 2020/03/10 Sascha Hauer <s.hauer@pengutronix.de> wrote: > > On Tue, Mar 10, 2020 at 07:31:58PM +0800, Robin Gong wrote: > > > ECSPI issue fixed from i.mx6ul at hardware level, no need > > > ERR009165 anymore on those chips such as i.mx8mq. Add i.mx6sx from > > > where i.mx6ul source. > > > > > > Signed-off-by: Robin Gong <yibin.gong@nxp.com> > > > Acked-by: Vinod Koul <vkoul@kernel.org> > > > --- > > > drivers/dma/imx-sdma.c | 51 > > > +++++++++++++++++++++++++++++++++++++++++++++++++- > > > 1 file changed, 50 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c index > > > 56288d8..5ae7237 100644 > > > --- a/drivers/dma/imx-sdma.c > > > +++ b/drivers/dma/imx-sdma.c > > > @@ -419,6 +419,13 @@ struct sdma_driver_data { > > > int num_events; > > > struct sdma_script_start_addrs *script_addrs; > > > bool check_ratio; > > > + /* > > > + * ecspi ERR009165 fixed should be done in sdma script > > > + * and it has been fixed in soc from i.mx6ul. > > > + * please get more information from the below link: > > > + * > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.nx > > p.com%2Fdocs%2Fen%2Ferrata%2FIMX6DQCE.pdf&data=02%7C01%7Cyi > > bin.gong%40nxp.com%7C91d42046e6894501d48508d7c4cbcae2%7C686ea1d3 > > bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637194251876090170&sdata= > > T6LA4xz9CUFlNpnyjHSThEQb8i1rhbY9e1nUyxIGD5Q%3D&reserved=0 > > > + */ > > > + bool ecspi_fixed; > > > }; > > > > > > struct sdma_engine { > > > @@ -539,6 +546,31 @@ static struct sdma_driver_data sdma_imx6q = { > > > .script_addrs = &sdma_script_imx6q, > > > }; > > > > > > +static struct sdma_script_start_addrs sdma_script_imx6sx = { > > > + .ap_2_ap_addr = 642, > > > + .uart_2_mcu_addr = 817, > > > + .mcu_2_app_addr = 747, > > > + .uartsh_2_mcu_addr = 1032, > > > + .mcu_2_shp_addr = 960, > > > + .app_2_mcu_addr = 683, > > > + .shp_2_mcu_addr = 891, > > > + .spdif_2_mcu_addr = 1100, > > > + .mcu_2_spdif_addr = 1134, > > > +}; > > > + > > > +static struct sdma_driver_data sdma_imx6sx = { > > > + .chnenbl0 = SDMA_CHNENBL0_IMX35, > > > + .num_events = 48, > > > + .script_addrs = &sdma_script_imx6sx, }; > > > + > > > +static struct sdma_driver_data sdma_imx6ul = { > > > + .chnenbl0 = SDMA_CHNENBL0_IMX35, > > > + .num_events = 48, > > > + .script_addrs = &sdma_script_imx6sx, > > > + .ecspi_fixed = true, > > > +}; > > > + > > > static struct sdma_script_start_addrs sdma_script_imx7d = { > > > .ap_2_ap_addr = 644, > > > .uart_2_mcu_addr = 819, > > > @@ -584,9 +616,15 @@ static const struct platform_device_id > > sdma_devtypes[] = { > > > .name = "imx6q-sdma", > > > .driver_data = (unsigned long)&sdma_imx6q, > > > }, { > > > + .name = "imx6sx-sdma", > > > + .driver_data = (unsigned long)&sdma_imx6sx, > > > + }, { > > > > Now the i.MX6sx uses a new sdma_script_start_addrs entry which is the same > > as the i.MX6q one we used before with one exception: it lacks the > > per_2_per_addr = 6331 entry. This is only used for IMX_DMATYPE_ASRC and > Totally same script for i.mx6 chips whatever i.MX6sx, i.MX6q or i.MX6ul. When it's the same then use it. > > IMX_DMATYPE_ASRC_SP, both are entirely unused in the mainline kernel. So > > why must the i.MX6sx changed here and what has this to do with ECSPI? > i.MX6ul is based on i.MX6sx, so adding i.MX6sx could keep good shape on our i.MX family evolution. My point is that there is no difference between i.MX6q and i.MX6sx here, so do not artificially introduce i.MX6sx support when all you do is copying the i.MX6q support. Sascha
On 2020/03/10 Sascha Hauer <s.hauer@pengutronix.de> wrote: > On Tue, Mar 10, 2020 at 08:43:10AM +0000, Robin Gong wrote: > > On 2020/03/10 Sascha Hauer <s.hauer@pengutronix.de> wrote: > > > On Tue, Mar 10, 2020 at 07:31:56PM +0800, Robin Gong wrote: > > > > ERR009165 fixed on i.mx6ul/6ull/6sll. All other i.mx6/7 and > > > > i.mx8m/8mm still need this errata. Please refer to nxp official > > > > errata document from > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fww > > > w.nx > > > > p.com%2F&data=02%7C01%7Cyibin.gong%40nxp.com%7Cf73bfc11a68c4 > > > > 2f5f6d308d7c4c96efa%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C > > > > 637194241755109112&sdata=xzIUP8qZkrlDXX0yjTcUNZB6zDrevTdHFg1o4 > > > PZZd8E%3D&reserved=0 . > > > > > > > > For removing workaround on those chips. Add new i.mx6ul type. > > > > > > > > Signed-off-by: Robin Gong <yibin.gong@nxp.com> > > > > Acked-by: Mark Brown <broonie@kernel.org> > > > > --- > > > > drivers/spi/spi-imx.c | 50 > > > > +++++++++++++++++++++++++++++++++++++++++++++----- > > > > 1 file changed, 45 insertions(+), 5 deletions(-) > > > > > > > > diff --git a/drivers/spi/spi-imx.c b/drivers/spi/spi-imx.c index > > > > 842a86e..f7ee2ec 100644 > > > > --- a/drivers/spi/spi-imx.c > > > > +++ b/drivers/spi/spi-imx.c > > > > @@ -57,6 +57,7 @@ enum spi_imx_devtype { > > > > IMX35_CSPI, /* CSPI on all i.mx except above */ > > > > IMX51_ECSPI, /* ECSPI on i.mx51 */ > > > > IMX53_ECSPI, /* ECSPI on i.mx53 and later */ > > > > + IMX6UL_ECSPI, /* ERR009165 fix from i.mx6ul */ > > > > }; > > > > > > > > struct spi_imx_data; > > > > @@ -75,6 +76,11 @@ struct spi_imx_devtype_data { > > > > bool has_slavemode; > > > > unsigned int fifo_size; > > > > bool dynamic_burst; > > > > + /* > > > > + * ERR009165 fixed or not: > > > > + * > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fww > > > w.nx > p.com%2Fdocs%2Fen%2Ferrata%2FIMX6DQCE.pdf&data=02%7C01%7Cyi > > > > bin.gong%40nxp.com%7Cf73bfc11a68c42f5f6d308d7c4c96efa%7C686ea1d3bc > > > > 2b4c6fa92cd99c5c301635%7C0%7C1%7C637194241755109112&sdata=m > > > > uw4HL5nMDjREJwVd885Wrxka0moMaaZ%2BhJgsAgY3eo%3D&reserved= > > > 0 > > > > + */ > > > > + bool tx_glitch_fixed; > > > > enum spi_imx_devtype devtype; > > > > }; > > > > > > > > @@ -128,7 +134,8 @@ static inline int is_imx35_cspi(struct > > > > spi_imx_data *d) > > > > > > > > static inline int is_imx51_ecspi(struct spi_imx_data *d) { > > > > - return d->devtype_data->devtype == IMX51_ECSPI; > > > > + return d->devtype_data->devtype == IMX51_ECSPI || > > > > + d->devtype_data->devtype == IMX6UL_ECSPI; > > > > } > > > > > > Erm, no. A i.MX51 ECSPI is a i.MX51 ECSPI and not a i.MX6UL ECSPI. > > > If you want to handle them equally somewhere then explicitly test > > > for i.MX6ul *and* > > > i.MX51 there. > > But all i.mx6 chips including i.MX53 ECSPI are almost same as i.MX51 ECSPI, > and ERR00915 is fixed from i.mx6ul.... > > You introduce .devtype = IMX6UL_ECSPI in this series, so apparently it is *not* > the same as IMX51_ECSPI, then please also don't introduce a function which > claims they are the same. Okay, I'll add is_imx6ul_ecspi explicitly. > > Sascha > > -- > Pengutronix e.K. | > | > Steuerwalder Str. 21 | > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pe > ngutronix.de%2F&data=02%7C01%7Cyibin.gong%40nxp.com%7Ca10f3fdc > 28084c1fe89a08d7c4d50531%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7 > C0%7C637194291506592966&sdata=aqMqER0j0fVbAiKqM1tzBuEVlhiVcVL > GnrSRhJvhDk8%3D&reserved=0 | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 > | > Amtsgericht Hildesheim, HRA 2686 | Fax: > +49-5121-206917-5555 |
On 2020/03/10 Sascha Hauer <s.hauer@pengutronix.de> wrote: > On Tue, Mar 10, 2020 at 08:59:03AM +0000, Robin Gong wrote: > > On 2020/03/10 Sascha Hauer <s.hauer@pengutronix.de> wrote: > > > On Tue, Mar 10, 2020 at 07:31:58PM +0800, Robin Gong wrote: > > > > ECSPI issue fixed from i.mx6ul at hardware level, no need > > > > ERR009165 anymore on those chips such as i.mx8mq. Add i.mx6sx from > > > > where i.mx6ul source. > > > > > > > > Signed-off-by: Robin Gong <yibin.gong@nxp.com> > > > > Acked-by: Vinod Koul <vkoul@kernel.org> > > > > --- > > > > drivers/dma/imx-sdma.c | 51 > > > > +++++++++++++++++++++++++++++++++++++++++++++++++- > > > > 1 file changed, 50 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c index > > > > 56288d8..5ae7237 100644 > > > > --- a/drivers/dma/imx-sdma.c > > > > +++ b/drivers/dma/imx-sdma.c > > > > @@ -419,6 +419,13 @@ struct sdma_driver_data { > > > > int num_events; > > > > struct sdma_script_start_addrs *script_addrs; > > > > bool check_ratio; > > > > + /* > > > > + * ecspi ERR009165 fixed should be done in sdma script > > > > + * and it has been fixed in soc from i.mx6ul. > > > > + * please get more information from the below link: > > > > + * > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fww > > > w.nx > p.com%2Fdocs%2Fen%2Ferrata%2FIMX6DQCE.pdf&data=02%7C01%7Cyi > > > > bin.gong%40nxp.com%7C91d42046e6894501d48508d7c4cbcae2%7C686ea1d3 > > > > bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637194251876090170&sdata= > > > T6LA4xz9CUFlNpnyjHSThEQb8i1rhbY9e1nUyxIGD5Q%3D&reserved=0 > > > > + */ > > > > + bool ecspi_fixed; > > > > }; > > > > > > > > struct sdma_engine { > > > > @@ -539,6 +546,31 @@ static struct sdma_driver_data sdma_imx6q = { > > > > .script_addrs = &sdma_script_imx6q, }; > > > > > > > > +static struct sdma_script_start_addrs sdma_script_imx6sx = { > > > > + .ap_2_ap_addr = 642, > > > > + .uart_2_mcu_addr = 817, > > > > + .mcu_2_app_addr = 747, > > > > + .uartsh_2_mcu_addr = 1032, > > > > + .mcu_2_shp_addr = 960, > > > > + .app_2_mcu_addr = 683, > > > > + .shp_2_mcu_addr = 891, > > > > + .spdif_2_mcu_addr = 1100, > > > > + .mcu_2_spdif_addr = 1134, > > > > +}; > > > > + > > > > +static struct sdma_driver_data sdma_imx6sx = { > > > > + .chnenbl0 = SDMA_CHNENBL0_IMX35, > > > > + .num_events = 48, > > > > + .script_addrs = &sdma_script_imx6sx, }; > > > > + > > > > +static struct sdma_driver_data sdma_imx6ul = { > > > > + .chnenbl0 = SDMA_CHNENBL0_IMX35, > > > > + .num_events = 48, > > > > + .script_addrs = &sdma_script_imx6sx, > > > > + .ecspi_fixed = true, > > > > +}; > > > > + > > > > static struct sdma_script_start_addrs sdma_script_imx7d = { > > > > .ap_2_ap_addr = 644, > > > > .uart_2_mcu_addr = 819, > > > > @@ -584,9 +616,15 @@ static const struct platform_device_id > > > sdma_devtypes[] = { > > > > .name = "imx6q-sdma", > > > > .driver_data = (unsigned long)&sdma_imx6q, > > > > }, { > > > > + .name = "imx6sx-sdma", > > > > + .driver_data = (unsigned long)&sdma_imx6sx, > > > > + }, { > > > > > > Now the i.MX6sx uses a new sdma_script_start_addrs entry which is > > > the same as the i.MX6q one we used before with one exception: it > > > lacks the per_2_per_addr = 6331 entry. This is only used for > > > IMX_DMATYPE_ASRC and > > Totally same script for i.mx6 chips whatever i.MX6sx, i.MX6q or i.MX6ul. > > When it's the same then use it. > > > > IMX_DMATYPE_ASRC_SP, both are entirely unused in the mainline > > > kernel. So why must the i.MX6sx changed here and what has this to do with > ECSPI? > > i.MX6ul is based on i.MX6sx, so adding i.MX6sx could keep good shape on our > i.MX family evolution. > > My point is that there is no difference between i.MX6q and i.MX6sx here, so do > not artificially introduce i.MX6sx support when all you do is copying the i.MX6q > support. Okay, will remove i.MX6sx now. > -- > Pengutronix e.K. | > | > Steuerwalder Str. 21 | > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.pe > ngutronix.de%2F&data=02%7C01%7Cyibin.gong%40nxp.com%7C02af95d > 81bf745b7b2cc08d7c4d55ed2%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0 > %7C0%7C637194293001547060&sdata=cN13LuC6Bgs1m9W6oKc2q03j5rf > KvsMbonpd1JALA%2Fk%3D&reserved=0 | > 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 > | > Amtsgericht Hildesheim, HRA 2686 | Fax: > +49-5121-206917-5555 |
There is ecspi ERR009165 on i.mx6/7 soc family, which cause FIFO transfer to be send twice in DMA mode. Please get more information from: https://www.nxp.com/docs/en/errata/IMX6DQCE.pdf. The workaround is adding new sdma ram script which works in XCH mode as PIO inside sdma instead of SMC mode, meanwhile, 'TX_THRESHOLD' should be 0. The issue should be exist on all legacy i.mx6/7 soc family before i.mx6ul. NXP fix this design issue from i.mx6ul, so newer chips including i.mx6ul/ 6ull/6sll do not need this workaroud anymore. All other i.mx6/7/8 chips still need this workaroud. This patch set add new 'fsl,imx6ul-ecspi' for ecspi driver and 'ecspi_fixed' in sdma driver to choose if need errata or not. The first two reverted patches should be the same issue, though, it seems 'fixed' by changing to other shp script. Hope Sean or Sascha could have the chance to test this patch set if could fix their issues. Besides, enable sdma support for i.mx8mm/8mq and fix ecspi1 not work on i.mx8mm because the event id is zero. PS: Please get sdma firmware from below linux-firmware and copy it to your local rootfs /lib/firmware/imx/sdma. https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/tree/imx/sdma v2: 1.Add commit log for reverted patches. 2.Add comment for 'ecspi_fixed' in sdma driver. 3.Add 'fsl,imx6sll-ecspi' compatible instead of 'fsl,imx6ul-ecspi' rather than remove. v3: 1.Confirm with design team make sure ERR009165 fixed on i.mx6ul/i.mx6ull /i.mx6sll, not fixed on i.mx8m/8mm and other i.mx6/7 legacy chips. Correct dts related dts patch in v2. 2.Clean eratta information in binding doc and new 'tx_glitch_fixed' flag in spi-imx driver to state ERR009165 fixed or not. 3.Enlarge burst size to fifo size for tx since tx_wml set to 0 in the errata workaroud, thus improve performance as possible. v4: 1.Add Ack tag from Mark and Vinod 2.Remove checking 'event_id1' zero as 'event_id0'. v5: 1.Add the last patch for compatible with the current uart driver which using rom script, so both uart ram script and rom script supported in latest firmware, by default uart rom script used. UART driver will be broken without this patch. v6: 1.Resend after rebase the latest next branch. 2.Remove below No.13~No.15 patches of v5 because they were mergered. ARM: dts: imx6ul: add dma support on ecspi ARM: dts: imx6sll: correct sdma compatible arm64: defconfig: Enable SDMA on i.mx8mq/8mm 3.Revert "dmaengine: imx-sdma: fix context cache" since 'context_loaded' removed. Robin Gong (13): Revert "ARM: dts: imx6q: Use correct SDMA script for SPI5 core" Revert "ARM: dts: imx6: Use correct SDMA script for SPI cores" Revert "dmaengine: imx-sdma: refine to load context only once" dmaengine: imx-sdma: remove dupilicated sdma_load_context dmaengine: imx-sdma: add mcu_2_ecspi script spi: imx: fix ERR009165 spi: imx: remove ERR009165 workaround on i.mx6ul spi: imx: add new i.mx6ul compatible name in binding doc dmaengine: imx-sdma: remove ERR009165 on i.mx6ul dma: imx-sdma: add i.mx6ul/6sx compatible name dmaengine: imx-sdma: fix ecspi1 rx dma not work on i.mx8mm dmaengine: imx-sdma: add uart rom script Revert "dmaengine: imx-sdma: fix context cache" .../devicetree/bindings/dma/fsl-imx-sdma.txt | 2 + .../devicetree/bindings/spi/fsl-imx-cspi.txt | 1 + arch/arm/boot/dts/imx6q.dtsi | 2 +- arch/arm/boot/dts/imx6qdl.dtsi | 8 +- drivers/dma/imx-sdma.c | 89 ++++++++++++++++------ drivers/spi/spi-imx.c | 61 ++++++++++++--- include/linux/platform_data/dma-imx-sdma.h | 8 +- 7 files changed, 130 insertions(+), 41 deletions(-)