Re: [RFC PATCH] i2c: imx: dma map the i2c data i/o register

From: Laurentiu Tudor
Date: Thu Jan 17 2019 - 04:58:17 EST

Next message: Dmitry Vyukov: "Re: kernel BUG at mm/page_alloc.c:LINE!"
Previous message: Michael Ellerman: "Re: [PATCH V2] mm: Introduce GFP_PGTABLE"
In reply to: Robin Murphy: "Re: [RFC PATCH] i2c: imx: dma map the i2c data i/o register"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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;
>> Â }
>>

Next message: Dmitry Vyukov: "Re: kernel BUG at mm/page_alloc.c:LINE!"
Previous message: Michael Ellerman: "Re: [PATCH V2] mm: Introduce GFP_PGTABLE"
In reply to: Robin Murphy: "Re: [RFC PATCH] i2c: imx: dma map the i2c data i/o register"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]