Hi Robin, On 16.01.2019 19:55, Robin Murphy wrote: > On 16/01/2019 16:17, Laurentiu Tudor wrote: >> This is an attempt to fix an iommu exception when doing dma to the >> i2c controller with EDMA. Without these mappings the smmu raises a >> context fault [1] exactly with the address of the i2c data i/o reg. >> This was seen on an NXP LS1043A chip while working on enabling SMMU. > > Rather than gradually adding much the same code to potentially every > possible client driver, can it not be implemented once in the edma > driver as was done for pl330 and rcar-dmac? That also sidesteps any of > the nastiness of smuggling a dma_addr_t via a phys_addr_t variable. Thanks for the pointer. I was actually unsure where this should be tackled: either i2c or dma side. Plus I somehow managed to completely miss the support added in the dma drivers you mention. I'll start looking into stealing some of your code [1]. :-) [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=4d6d74e22096543cb3b35e717cf1b9aea3655f37 --- Best Regards, Laurentiu > >> [1] arm-smmu 9000000.iommu: Unhandled context fault: fsr=0x402, >> iova=0x02180004, fsynr=0x150021, cb=7 >> >> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@xxxxxxx> >> --- >> drivers/i2c/busses/i2c-imx.c | 57 +++++++++++++++++++++++++++++------- >> 1 file changed, 47 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c >> index 4e34b1572756..07cc8f4b45b9 100644 >> --- a/drivers/i2c/busses/i2c-imx.c >> +++ b/drivers/i2c/busses/i2c-imx.c >> @@ -202,6 +202,9 @@ struct imx_i2c_struct { >> struct pinctrl_state *pinctrl_pins_gpio; >> struct imx_i2c_dma *dma; >> + >> + dma_addr_t dma_tx_addr; >> + dma_addr_t dma_rx_addr; >> }; >> static const struct imx_i2c_hwdata imx1_i2c_hwdata = { >> @@ -274,17 +277,20 @@ static inline unsigned char >> imx_i2c_read_reg(struct imx_i2c_struct *i2c_imx, >> /* Functions for DMA support */ >> static int i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx, >> - dma_addr_t phy_addr) >> + phys_addr_t phy_addr) >> { >> struct imx_i2c_dma *dma; >> struct dma_slave_config dma_sconfig; >> struct device *dev = &i2c_imx->adapter.dev; >> int ret; >> + phys_addr_t i2dr_pa; >> dma = devm_kzalloc(dev, sizeof(*dma), GFP_KERNEL); >> if (!dma) >> return -ENOMEM; >> + i2dr_pa = phy_addr + (IMX_I2C_I2DR << i2c_imx->hwdata->regshift); >> + >> dma->chan_tx = dma_request_chan(dev, "tx"); >> if (IS_ERR(dma->chan_tx)) { >> ret = PTR_ERR(dma->chan_tx); >> @@ -293,15 +299,25 @@ static int i2c_imx_dma_request(struct >> imx_i2c_struct *i2c_imx, >> goto fail_al; >> } >> - dma_sconfig.dst_addr = phy_addr + >> - (IMX_I2C_I2DR << i2c_imx->hwdata->regshift); >> + i2c_imx->dma_tx_addr = dma_map_resource(dma->chan_tx->device->dev, >> + i2dr_pa, >> + DMA_SLAVE_BUSWIDTH_1_BYTE, >> + DMA_MEM_TO_DEV, 0); >> + ret = dma_mapping_error(dma->chan_tx->device->dev, >> + i2c_imx->dma_tx_addr); >> + if (ret) { >> + dev_err(dev, "can't dma map tx destination (%d)\n", ret); >> + goto fail_tx; >> + } >> + >> + dma_sconfig.dst_addr = i2c_imx->dma_tx_addr; >> dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE; >> dma_sconfig.dst_maxburst = 1; >> dma_sconfig.direction = DMA_MEM_TO_DEV; >> ret = dmaengine_slave_config(dma->chan_tx, &dma_sconfig); >> if (ret < 0) { >> dev_err(dev, "can't configure tx channel (%d)\n", ret); >> - goto fail_tx; >> + goto fail_tx_dma; >> } >> dma->chan_rx = dma_request_chan(dev, "rx"); >> @@ -309,18 +325,28 @@ static int i2c_imx_dma_request(struct >> imx_i2c_struct *i2c_imx, >> ret = PTR_ERR(dma->chan_rx); >> if (ret != -ENODEV && ret != -EPROBE_DEFER) >> dev_err(dev, "can't request DMA rx channel (%d)\n", ret); >> - goto fail_tx; >> + goto fail_tx_dma; >> } >> - dma_sconfig.src_addr = phy_addr + >> - (IMX_I2C_I2DR << i2c_imx->hwdata->regshift); >> + i2c_imx->dma_rx_addr = dma_map_resource(dma->chan_rx->device->dev, >> + i2dr_pa, >> + DMA_SLAVE_BUSWIDTH_1_BYTE, >> + DMA_DEV_TO_MEM, 0); >> + ret = dma_mapping_error(dma->chan_rx->device->dev, >> + i2c_imx->dma_rx_addr); >> + if (ret) { >> + dev_err(dev, "can't dma map rx source (%d)\n", ret); >> + goto fail_rx; >> + } >> + >> + dma_sconfig.src_addr = i2c_imx->dma_rx_addr; >> dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE; >> dma_sconfig.src_maxburst = 1; >> dma_sconfig.direction = DMA_DEV_TO_MEM; >> ret = dmaengine_slave_config(dma->chan_rx, &dma_sconfig); >> if (ret < 0) { >> dev_err(dev, "can't configure rx channel (%d)\n", ret); >> - goto fail_rx; >> + goto fail_rx_dma; >> } >> i2c_imx->dma = dma; >> @@ -330,8 +356,14 @@ static int i2c_imx_dma_request(struct >> imx_i2c_struct *i2c_imx, >> return 0; >> +fail_rx_dma: >> + dma_unmap_resource(dma->chan_rx->device->dev, i2c_imx->dma_rx_addr, >> + DMA_SLAVE_BUSWIDTH_1_BYTE, DMA_DEV_TO_MEM, 0); >> fail_rx: >> dma_release_channel(dma->chan_rx); >> +fail_tx_dma: >> + dma_unmap_resource(dma->chan_tx->device->dev, i2c_imx->dma_tx_addr, >> + DMA_SLAVE_BUSWIDTH_1_BYTE, DMA_MEM_TO_DEV, 0); >> fail_tx: >> dma_release_channel(dma->chan_tx); >> fail_al: >> @@ -1057,7 +1089,7 @@ static int i2c_imx_probe(struct platform_device >> *pdev) >> struct imxi2c_platform_data *pdata = dev_get_platdata(&pdev->dev); >> void __iomem *base; >> int irq, ret; >> - dma_addr_t phy_addr; >> + phys_addr_t phy_addr; >> dev_dbg(&pdev->dev, "<%s>\n", __func__); >> @@ -1072,7 +1104,7 @@ static int i2c_imx_probe(struct platform_device >> *pdev) >> if (IS_ERR(base)) >> return PTR_ERR(base); >> - phy_addr = (dma_addr_t)res->start; >> + phy_addr = res->start; >> i2c_imx = devm_kzalloc(&pdev->dev, sizeof(*i2c_imx), GFP_KERNEL); >> if (!i2c_imx) >> return -ENOMEM; >> @@ -1220,6 +1252,11 @@ static int i2c_imx_remove(struct >> platform_device *pdev) >> pm_runtime_put_noidle(&pdev->dev); >> pm_runtime_disable(&pdev->dev); >> + dma_unmap_resource(&pdev->dev, i2c_imx->dma_tx_addr, >> + DMA_SLAVE_BUSWIDTH_1_BYTE, DMA_MEM_TO_DEV, 0); >> + dma_unmap_resource(&pdev->dev, i2c_imx->dma_rx_addr, >> + DMA_SLAVE_BUSWIDTH_1_BYTE, DMA_DEV_TO_MEM, 0); >> + >> return 0; >> } >>
