* [PATCH 0/3] i2c: omap: add support for DMA
@ 2026-11-05 20:05 Antoni Pokusinski
2026-11-05 20:05 ` [PATCH 1/3] dt-bindings: i2c: ti,omap4-i2c: Add DMA properties Antoni Pokusinski
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Antoni Pokusinski @ 2026-11-05 20:05 UTC (permalink / raw)
To: vigneshr, andi.shyti, robh, krzk+dt, conor+dt, aaro.koskinen,
andreas, khilman, rogerq, tony, jmkrzyszt, sumit.semwal,
christian.koenig, phasta
Cc: linux-omap, linux-i2c, devicetree, linux-kernel, linux-media,
dri-devel, linaro-mm-sig, Antoni Pokusinski
Hello,
This patchset introduces support for the TX & RX DMA transfers in the
am335x/omap I2C controller. I tested this implementation on AM335x
(Beaglebone Black) and things seem to work fine.
Kind regards,
Antoni Pokusinski
Antoni Pokusinski (3):
dt-bindings: i2c: ti,omap4-i2c: Add DMA properties
ARM: dts: am33xx-l4: Add missing DMAs for I2C controllers
i2c: omap: add DMA support
.../devicetree/bindings/i2c/ti,omap4-i2c.yaml | 12 +
arch/arm/boot/dts/ti/omap/am33xx-l4.dtsi | 4 +
drivers/i2c/busses/i2c-omap.c | 291 +++++++++++++++++-
3 files changed, 303 insertions(+), 4 deletions(-)
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
--
2.56.0
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/3] dt-bindings: i2c: ti,omap4-i2c: Add DMA properties 2026-11-05 20:05 [PATCH 0/3] i2c: omap: add support for DMA Antoni Pokusinski @ 2026-11-05 20:05 ` Antoni Pokusinski 2026-11-09 13:21 ` Krzysztof Kozlowski 2026-11-05 20:05 ` [PATCH 2/3] ARM: dts: am33xx-l4: Add missing DMAs for I2C controllers Antoni Pokusinski 2026-11-05 20:05 ` [PATCH 3/3] i2c: omap: add DMA support Antoni Pokusinski 2 siblings, 1 reply; 6+ messages in thread From: Antoni Pokusinski @ 2026-11-05 20:05 UTC (permalink / raw) To: vigneshr, andi.shyti, robh, krzk+dt, conor+dt, aaro.koskinen, andreas, khilman, rogerq, tony, jmkrzyszt, sumit.semwal, christian.koenig, phasta Cc: linux-omap, linux-i2c, devicetree, linux-kernel, linux-media, dri-devel, linaro-mm-sig, Antoni Pokusinski Document the TX and RX DMA request channels featured by the OMAP I2C controller. Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com> --- .../devicetree/bindings/i2c/ti,omap4-i2c.yaml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Documentation/devicetree/bindings/i2c/ti,omap4-i2c.yaml b/Documentation/devicetree/bindings/i2c/ti,omap4-i2c.yaml index 58d32ceeacfc..ff946fdfaad3 100644 --- a/Documentation/devicetree/bindings/i2c/ti,omap4-i2c.yaml +++ b/Documentation/devicetree/bindings/i2c/ti,omap4-i2c.yaml @@ -37,6 +37,16 @@ properties: clock-names: const: fck + dmas: + items: + - description: TX DMA channel phandle + - description: RX DMA channel phandle + + dma-names: + items: + - const: tx + - const: rx + power-domains: true ti,hwmods: @@ -93,4 +103,6 @@ examples: #address-cells = <1>; #size-cells = <0>; mux-states = <&i2c_mux 1>; + dmas = <&edma 58 0>, <&edma 59 0>; + dma-names = "tx", "rx"; }; -- 2.56.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] dt-bindings: i2c: ti,omap4-i2c: Add DMA properties 2026-11-05 20:05 ` [PATCH 1/3] dt-bindings: i2c: ti,omap4-i2c: Add DMA properties Antoni Pokusinski @ 2026-11-09 13:21 ` Krzysztof Kozlowski 0 siblings, 0 replies; 6+ messages in thread From: Krzysztof Kozlowski @ 2026-11-09 13:21 UTC (permalink / raw) To: Antoni Pokusinski Cc: vigneshr, andi.shyti, robh, krzk+dt, conor+dt, aaro.koskinen, andreas, khilman, rogerq, tony, jmkrzyszt, sumit.semwal, christian.koenig, phasta, linux-omap, linux-i2c, devicetree, linux-kernel, linux-media, dri-devel, linaro-mm-sig On Thu, Nov 05, 2026 at 09:05:52PM +0100, Antoni Pokusinski wrote: > Document the TX and RX DMA request channels featured > by the OMAP I2C controller. > > Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com> > --- > .../devicetree/bindings/i2c/ti,omap4-i2c.yaml | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/Documentation/devicetree/bindings/i2c/ti,omap4-i2c.yaml b/Documentation/devicetree/bindings/i2c/ti,omap4-i2c.yaml > index 58d32ceeacfc..ff946fdfaad3 100644 > --- a/Documentation/devicetree/bindings/i2c/ti,omap4-i2c.yaml > +++ b/Documentation/devicetree/bindings/i2c/ti,omap4-i2c.yaml > @@ -37,6 +37,16 @@ properties: > clock-names: > const: fck > > + dmas: > + items: > + - description: TX DMA channel phandle > + - description: RX DMA channel phandle > + > + dma-names: > + items: > + - const: tx > + - const: rx Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com> Best regards, Krzysztof ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/3] ARM: dts: am33xx-l4: Add missing DMAs for I2C controllers 2026-11-05 20:05 [PATCH 0/3] i2c: omap: add support for DMA Antoni Pokusinski 2026-11-05 20:05 ` [PATCH 1/3] dt-bindings: i2c: ti,omap4-i2c: Add DMA properties Antoni Pokusinski @ 2026-11-05 20:05 ` Antoni Pokusinski 2026-11-05 20:05 ` [PATCH 3/3] i2c: omap: add DMA support Antoni Pokusinski 2 siblings, 0 replies; 6+ messages in thread From: Antoni Pokusinski @ 2026-11-05 20:05 UTC (permalink / raw) To: vigneshr, andi.shyti, robh, krzk+dt, conor+dt, aaro.koskinen, andreas, khilman, rogerq, tony, jmkrzyszt, sumit.semwal, christian.koenig, phasta Cc: linux-omap, linux-i2c, devicetree, linux-kernel, linux-media, dri-devel, linaro-mm-sig, Antoni Pokusinski The i2c0 and i2c1 controllers on AM33xx support TX and RX DMA requests. Add the EDMA request mappings and DMA names for both controllers. Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com> --- arch/arm/boot/dts/ti/omap/am33xx-l4.dtsi | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/arch/arm/boot/dts/ti/omap/am33xx-l4.dtsi b/arch/arm/boot/dts/ti/omap/am33xx-l4.dtsi index 57a45609f9f5..89fb1d1eb049 100644 --- a/arch/arm/boot/dts/ti/omap/am33xx-l4.dtsi +++ b/arch/arm/boot/dts/ti/omap/am33xx-l4.dtsi @@ -239,6 +239,8 @@ i2c0: i2c@0 { reg = <0x0 0x1000>; interrupts = <70>; status = "disabled"; + dmas = <&edma 58 0>, <&edma 59 0>; + dma-names = "tx", "rx"; }; }; @@ -1189,6 +1191,8 @@ i2c1: i2c@0 { reg = <0x0 0x1000>; interrupts = <71>; status = "disabled"; + dmas = <&edma 60 0>, <&edma 61 0>; + dma-names = "tx", "rx"; }; }; -- 2.56.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] i2c: omap: add DMA support 2026-11-05 20:05 [PATCH 0/3] i2c: omap: add support for DMA Antoni Pokusinski 2026-11-05 20:05 ` [PATCH 1/3] dt-bindings: i2c: ti,omap4-i2c: Add DMA properties Antoni Pokusinski 2026-11-05 20:05 ` [PATCH 2/3] ARM: dts: am33xx-l4: Add missing DMAs for I2C controllers Antoni Pokusinski @ 2026-11-05 20:05 ` Antoni Pokusinski 2026-11-05 20:32 ` sashiko-bot 2 siblings, 1 reply; 6+ messages in thread From: Antoni Pokusinski @ 2026-11-05 20:05 UTC (permalink / raw) To: vigneshr, andi.shyti, robh, krzk+dt, conor+dt, aaro.koskinen, andreas, khilman, rogerq, tony, jmkrzyszt, sumit.semwal, christian.koenig, phasta Cc: linux-omap, linux-i2c, devicetree, linux-kernel, linux-media, dri-devel, linaro-mm-sig, Antoni Pokusinski Add support for the TX and RX DMA requests of the OMAP I2C controller. The DMA transactions are dispatched only if the I2C message size is a multiple of the configured FIFO threshold and is greater than a predefined threshold of 8 bytes. Support for DMA transactions that are not a multiple of the FIFO threshold can be added in the future using the draining feature. Signed-off-by: Antoni Pokusinski <apokusinski01@gmail.com> --- drivers/i2c/busses/i2c-omap.c | 291 +++++++++++++++++++++++++++++++++- 1 file changed, 287 insertions(+), 4 deletions(-) diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index f02d294db42a..59e633c80236 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -5,6 +5,7 @@ * Copyright (C) 2003 MontaVista Software, Inc. * Copyright (C) 2005 Nokia Corporation * Copyright (C) 2004 - 2007 Texas Instruments. + * Copyright (C) 2026 Antoni Pokusiński * * Originally written by MontaVista Software, Inc. * Additional contributions by: @@ -13,10 +14,13 @@ * Juha Yrjölä <juha.yrjola@solidboot.com> * Syed Khasim <x0khasim@ti.com> * Nishant Menon <nm@ti.com> + * Antoni Pokusiński <apokusinski01@gmail.com> */ #include <linux/module.h> #include <linux/delay.h> +#include <linux/dma-mapping.h> +#include <linux/dmaengine.h> #include <linux/i2c.h> #include <linux/err.h> #include <linux/interrupt.h> @@ -50,6 +54,8 @@ /* timeout for making decision on bus free status */ #define OMAP_I2C_BUS_FREE_TIMEOUT (msecs_to_jiffies(10)) +#define OMAP_I2C_DMA_THRESHOLD 8 + /* For OMAP3 I2C_IV has changed to I2C_WE (wakeup enable) */ enum { OMAP_I2C_REV_REG = 0, @@ -57,6 +63,10 @@ enum { OMAP_I2C_STAT_REG, OMAP_I2C_IV_REG, OMAP_I2C_WE_REG, + OMAP_I2C_DMARXENABLE_SET, + OMAP_I2C_DMATXENABLE_SET, + OMAP_I2C_DMARXENABLE_CLR, + OMAP_I2C_DMATXENABLE_CLR, OMAP_I2C_SYSS_REG, OMAP_I2C_BUF_REG, OMAP_I2C_CNT_REG, @@ -176,12 +186,27 @@ enum { #define OMAP_I2C_IP_V2_INTERRUPTS_MASK 0x6FFF +struct omap_i2c_dma { + struct dma_chan *chan_rx; + struct dma_chan *chan_tx; + struct dma_chan *chan_using; + struct device *dma_dev; + u8 *curr_msg_buf; + struct i2c_msg *curr_msg; + dma_addr_t dma_buf; + unsigned int dma_len; + enum dma_data_direction dma_data_dir; + struct completion complete; + dma_addr_t phy_addr; +}; + struct omap_i2c_dev { struct device *dev; void __iomem *base; /* virtual */ int irq; int reg_shift; /* bit shift for I2C register addresses */ struct completion cmd_complete; + struct omap_i2c_dma *dma; struct resource *ioarea; u32 latency; /* maximum mpu wkup latency */ void (*set_mpu_wkup_lat)(struct device *dev, @@ -242,6 +267,10 @@ static const u8 reg_map_ip_v2[] = { [OMAP_I2C_STAT_REG] = 0x28, [OMAP_I2C_IV_REG] = 0x34, [OMAP_I2C_WE_REG] = 0x34, + [OMAP_I2C_DMARXENABLE_SET] = 0x38, + [OMAP_I2C_DMATXENABLE_SET] = 0x3c, + [OMAP_I2C_DMARXENABLE_CLR] = 0x40, + [OMAP_I2C_DMATXENABLE_CLR] = 0x44, [OMAP_I2C_SYSS_REG] = 0x90, [OMAP_I2C_BUF_REG] = 0x94, [OMAP_I2C_CNT_REG] = 0x98, @@ -655,6 +684,144 @@ static void omap_i2c_wait(struct omap_i2c_dev *omap) } while (!(stat & mask) && count < 5); } +static void omap_i2c_dma_callback(void *arg) +{ + struct omap_i2c_dev *omap = (struct omap_i2c_dev *)arg; + + complete(&omap->dma->complete); +} + +static int omap_i2c_dma_xfer(struct omap_i2c_dev *omap, struct i2c_msg *msg) +{ + int ret; + struct dma_slave_config dma_sconfig = {0}; + struct omap_i2c_dma *dma = omap->dma; + struct dma_async_tx_descriptor *txdesc; + struct device *dev = &omap->adapter.dev; + + dma->curr_msg = msg; + + dma->curr_msg_buf = i2c_get_dma_safe_msg_buf(msg, 1); + if (!dma->curr_msg_buf) + return -ENOMEM; + + if (omap->receiver) { + dma->chan_using = dma->chan_rx; + + dma_sconfig.src_addr = dma->phy_addr + + (omap->regs[OMAP_I2C_DATA_REG] + << omap->reg_shift); + dma_sconfig.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE; + dma_sconfig.src_maxburst = omap->threshold; + dma_sconfig.direction = DMA_DEV_TO_MEM; + } else { + dma->chan_using = dma->chan_tx; + + dma_sconfig.dst_addr = dma->phy_addr + + (omap->regs[OMAP_I2C_DATA_REG] + << omap->reg_shift); + dma_sconfig.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE; + dma_sconfig.dst_maxburst = omap->threshold; + dma_sconfig.direction = DMA_MEM_TO_DEV; + } + + ret = dmaengine_slave_config(dma->chan_using, &dma_sconfig); + if (ret < 0) { + dev_err(dev, "DMA channel config failed: %d\n", ret); + goto err_map; + } + + dma->dma_len = msg->len; + dma->dma_data_dir = omap->receiver ? DMA_FROM_DEVICE : DMA_TO_DEVICE; + + dma->dma_dev = dmaengine_get_dma_device(dma->chan_using); + + dma->dma_buf = dma_map_single(dma->dma_dev, dma->curr_msg_buf, + dma->dma_len, dma->dma_data_dir); + if (dma_mapping_error(dma->dma_dev, dma->dma_buf)) { + dev_err(dev, "DMA mapping failed\n"); + ret = -EINVAL; + goto err_map; + } + + txdesc = dmaengine_prep_slave_single(dma->chan_using, dma->dma_buf, + dma->dma_len, + omap->receiver ? DMA_DEV_TO_MEM : + DMA_MEM_TO_DEV, + DMA_PREP_INTERRUPT | DMA_CTRL_ACK); + if (!txdesc) { + dev_err(dev, "Not able to get desc for DMA xfer\n"); + ret = -EINVAL; + goto err_desc; + } + + reinit_completion(&dma->complete); + + dmaengine_desc_set_callback(txdesc, omap_i2c_dma_callback, NULL, omap); + ret = dma_submit_error(dmaengine_submit(txdesc)); + if (ret < 0) { + dev_err(dev, "DMA submit failed\n"); + goto err_submit; + } + + dma_async_issue_pending(dma->chan_using); + return 0; + +err_submit: + dmaengine_terminate_sync(dma->chan_using); +err_desc: + dma_unmap_single(dma->dma_dev, dma->dma_buf, dma->dma_len, + dma->dma_data_dir); +err_map: + i2c_put_dma_safe_msg_buf(dma->curr_msg_buf, msg, false); + return ret; +} + +static void omap_i2c_dma_cleanup(struct omap_i2c_dev *omap, bool xferred) +{ + struct omap_i2c_dma *dma = omap->dma; + + dma_unmap_single(dma->dma_dev, dma->dma_buf, dma->dma_len, + dma->dma_data_dir); + i2c_put_dma_safe_msg_buf(dma->curr_msg_buf, dma->curr_msg, xferred); +} + +static void omap_i2c_dma_enable_req(struct omap_i2c_dev *omap) +{ + u16 w; + + w = omap_i2c_read_reg(omap, OMAP_I2C_BUF_REG); + if (omap->receiver) { + w |= OMAP_I2C_BUF_RDMA_EN; + omap_i2c_write_reg(omap, OMAP_I2C_BUF_REG, w); + + omap_i2c_write_reg(omap, OMAP_I2C_DMARXENABLE_SET, 1); + } else { + w |= OMAP_I2C_BUF_XDMA_EN; + omap_i2c_write_reg(omap, OMAP_I2C_BUF_REG, w); + + omap_i2c_write_reg(omap, OMAP_I2C_DMATXENABLE_SET, 1); + } +} + +static void omap_i2c_dma_disable_req(struct omap_i2c_dev *omap) +{ + u16 w; + + w = omap_i2c_read_reg(omap, OMAP_I2C_BUF_REG); + if (omap->receiver) { + w &= ~OMAP_I2C_BUF_RDMA_EN; + omap_i2c_write_reg(omap, OMAP_I2C_BUF_REG, w); + + omap_i2c_write_reg(omap, OMAP_I2C_DMARXENABLE_CLR, 1); + } else { + w &= ~OMAP_I2C_BUF_XDMA_EN; + omap_i2c_write_reg(omap, OMAP_I2C_BUF_REG, w); + + omap_i2c_write_reg(omap, OMAP_I2C_DMATXENABLE_CLR, 1); + } +} + /* * Low level master read/write transaction. */ @@ -664,7 +831,7 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap, struct omap_i2c_dev *omap = i2c_get_adapdata(adap); unsigned long time_left; u16 w; - int ret; + int ret, use_dma; dev_dbg(omap->dev, "addr: 0x%04x, len: %d, flags: 0x%x, stop: %d\n", msg->addr, msg->len, msg->flags, stop); @@ -672,6 +839,14 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap, omap->receiver = !!(msg->flags & I2C_M_RD); omap_i2c_resize_fifo(omap, msg->len, omap->receiver); + /** + * REVISIT: If the message size is not a multiple of the threshold, + * the draining feature must be used. + */ + use_dma = !polling && omap->dma && msg->len >= OMAP_I2C_DMA_THRESHOLD && + !(omap->flags & OMAP_I2C_FLAG_NO_FIFO) && + msg->len % omap->threshold == 0; + omap_i2c_write_reg(omap, OMAP_I2C_SA_REG, msg->addr); /* REVISIT: Could the STB bit of I2C_CON be used with probing? */ @@ -692,6 +867,17 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap, reinit_completion(&omap->cmd_complete); omap->cmd_err = 0; + if (use_dma) { + ret = omap_i2c_dma_xfer(omap, msg); + if (ret) { + omap_i2c_reset(omap); + __omap_i2c_init(omap); + return ret; + } + + omap_i2c_dma_enable_req(omap); + } + w = OMAP_I2C_CON_EN | OMAP_I2C_CON_MST | OMAP_I2C_CON_STT; /* High speed configuration */ @@ -742,8 +928,17 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap, * into arbitration and we're currently unable to recover from it. */ if (!polling) { - time_left = wait_for_completion_timeout(&omap->cmd_complete, - OMAP_I2C_TIMEOUT); + time_left = OMAP_I2C_TIMEOUT; + + if (use_dma) + time_left = wait_for_completion_timeout( + &omap->dma->complete, + time_left); + + if (time_left) + time_left = wait_for_completion_timeout( + &omap->cmd_complete, + time_left); } else { do { omap_i2c_wait(omap); @@ -753,6 +948,15 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap, time_left = !ret; } + if (use_dma) { + omap_i2c_dma_disable_req(omap); + + if (!time_left || omap->cmd_err) + dmaengine_terminate_sync(omap->dma->chan_using); + + omap_i2c_dma_cleanup(omap, time_left && !omap->cmd_err); + } + if (time_left == 0) { omap_i2c_reset(omap); __omap_i2c_init(omap); @@ -1338,10 +1542,70 @@ static struct i2c_bus_recovery_info omap_i2c_bus_recovery_info = { .recover_bus = i2c_generic_scl_recovery, }; +static struct omap_i2c_dma *omap_i2c_dma_init(struct omap_i2c_dev *omap, + dma_addr_t phy_addr) +{ + struct device *dev = omap->dev; + struct omap_i2c_dma *dma; + int ret; + + dma = devm_kzalloc(dev, sizeof(*dma), GFP_KERNEL); + if (!dma) + return ERR_PTR(-ENOMEM); + + dma->phy_addr = phy_addr; + + init_completion(&dma->complete); + + dma->chan_tx = dma_request_chan(dev, "tx"); + if (IS_ERR(dma->chan_tx)) { + ret = PTR_ERR(dma->chan_tx); + if (ret != -ENODEV) + dev_err_probe(dev, ret, + "can't request DMA tx channel\n"); + goto dma_free; + } + + dma->chan_rx = dma_request_chan(dev, "rx"); + if (IS_ERR(dma->chan_rx)) { + ret = PTR_ERR(dma->chan_rx); + if (ret != -ENODEV) + dev_err_probe(dev, ret, + "can't request DMA rx channel\n"); + goto dma_tx_fail; + } + + return dma; + +dma_tx_fail: + dma_release_channel(dma->chan_tx); +dma_free: + devm_kfree(dev, dma); + + return ERR_PTR(ret); +} + +static void omap_i2c_dma_free(struct omap_i2c_dev *omap) +{ + struct omap_i2c_dma *dma = omap->dma; + + dma->dma_buf = 0; + dma->dma_len = 0; + + dma_release_channel(dma->chan_tx); + dma->chan_tx = NULL; + + dma_release_channel(dma->chan_rx); + dma->chan_rx = NULL; + + dma->chan_using = NULL; +} + static int omap_i2c_probe(struct platform_device *pdev) { struct omap_i2c_dev *omap; + struct resource *res; struct i2c_adapter *adap; const struct omap_i2c_bus_platform_data *pdata = dev_get_platdata(&pdev->dev); @@ -1359,7 +1623,7 @@ omap_i2c_probe(struct platform_device *pdev) if (!omap) return -ENOMEM; - omap->base = devm_platform_ioremap_resource(pdev, 0); + omap->base = devm_platform_get_and_ioremap_resource(pdev, 0, &res); if (IS_ERR(omap->base)) return PTR_ERR(omap->base); @@ -1478,6 +1742,22 @@ omap_i2c_probe(struct platform_device *pdev) goto err_unuse_clocks; } + if (omap->scheme != OMAP_I2C_SCHEME_0) { + omap->dma = omap_i2c_dma_init(omap, (dma_addr_t)res->start); + if (PTR_ERR(omap->dma) == -EPROBE_DEFER) { + r = -EPROBE_DEFER; + dev_err_probe(&pdev->dev, r, + "can't get DMA channels\n"); + goto err_unuse_clocks; + } else if (PTR_ERR(omap->dma) == -ENODEV) { + dev_dbg(omap->dev, "DMA setup failed\n"); + omap->dma = NULL; + } else if (IS_ERR(omap->dma)) { + r = PTR_ERR(omap->dma); + goto err_unuse_clocks; + } + } + adap = &omap->adapter; i2c_set_adapdata(adap, omap); adap->owner = THIS_MODULE; @@ -1523,6 +1803,9 @@ static void omap_i2c_remove(struct platform_device *pdev) if (omap->mux_state) mux_state_deselect(omap->mux_state); + if (omap->dma) + omap_i2c_dma_free(omap); + ret = pm_runtime_get_sync(&pdev->dev); if (ret < 0) dev_err(omap->dev, "Failed to resume hardware, skip disable\n"); -- 2.56.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] i2c: omap: add DMA support 2026-11-05 20:05 ` [PATCH 3/3] i2c: omap: add DMA support Antoni Pokusinski @ 2026-11-05 20:32 ` sashiko-bot 0 siblings, 0 replies; 6+ messages in thread From: sashiko-bot @ 2026-11-05 20:32 UTC (permalink / raw) To: Antoni Pokusinski; +Cc: devicetree, dri-devel, robh, conor+dt, media-ci Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] i2c: omap: stall and -ETIMEDOUT on bus error in omap_i2c_xfer_msg() - [Medium] i2c: omap: DMA channel leak in omap_i2c_probe() on adapter add error - [Medium] i2c: omap: active PIO FIFO interrupts during DMA in omap_i2c_xfer_msg() -- commit 4941414830d919c66f3a4ea17fab3a4dcf5a305e Author: Antoni Pokusinski <apokusinski01@gmail.com> i2c: omap: add DMA support This commit adds TX and RX DMA support to the OMAP I2C controller driver for messages whose length is at least 8 bytes and a multiple of the configured FIFO threshold. > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c > index f02d294db42a6..59e633c80236b 100644 > --- a/drivers/i2c/busses/i2c-omap.c > +++ b/drivers/i2c/busses/i2c-omap.c [ ... ] > @@ -742,8 +928,17 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap, > * into arbitration and we're currently unable to recover from it. > */ > if (!polling) { > - time_left = wait_for_completion_timeout(&omap->cmd_complete, > - OMAP_I2C_TIMEOUT); > + time_left = OMAP_I2C_TIMEOUT; > + > + if (use_dma) > + time_left = wait_for_completion_timeout( > + &omap->dma->complete, > + time_left); > + > + if (time_left) > + time_left = wait_for_completion_timeout( > + &omap->cmd_complete, > + time_left); [Severity: High] What happens in omap_i2c_xfer_msg() in drivers/i2c/busses/i2c-omap.c if an I2C bus error such as a NACK (OMAP_I2C_STAT_NACK) or arbitration loss (OMAP_I2C_STAT_AL) occurs during a DMA transfer? When the target device NACKs or arbitration is lost before all msg->len bytes are transferred, the controller stops generating FIFO DMA requests, so omap_i2c_dma_callback() will not be invoked to complete omap->dma->complete. Meanwhile, the interrupt handler path: omap_i2c_isr_thread() -> omap_i2c_xfer_data() -> omap_i2c_complete_cmd() records the error in omap->cmd_err and signals omap->cmd_complete, but does not signal omap->dma->complete: static inline void omap_i2c_complete_cmd(struct omap_i2c_dev *omap, u16 err) { omap->cmd_err |= err; complete(&omap->cmd_complete); } Won't waiting on omap->dma->complete first cause omap_i2c_xfer_msg() to stall for the full OMAP_I2C_TIMEOUT, reset the controller, and return -ETIMEDOUT instead of handling omap->cmd_err and returning -EREMOTEIO, -EAGAIN, or 0 (for I2C_M_IGNORE_NAK)? > } else { > do { > omap_i2c_wait(omap); > @@ -753,6 +948,15 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap, > time_left = !ret; > } > > + if (use_dma) { > + omap_i2c_dma_disable_req(omap); > + > + if (!time_left || omap->cmd_err) > + dmaengine_terminate_sync(omap->dma->chan_using); > + > + omap_i2c_dma_cleanup(omap, time_left && !omap->cmd_err); > + } [Severity: Medium] Could disabling DMA requests here race with the PIO interrupt handler in drivers/i2c/busses/i2c-omap.c? During a DMA transfer, omap_i2c_xfer_msg() still sets omap->buf = msg->buf and omap->buf_len = msg->len, and OMAP_I2C_IE_XRDY, OMAP_I2C_IE_RRDY, OMAP_I2C_IE_XDR, and OMAP_I2C_IE_RDR remain enabled in OMAP_I2C_IE_REG. If a DMA transfer times out or fails while the controller is still enabled (prior to omap_i2c_reset()), omap_i2c_dma_disable_req() clears OMAP_I2C_BUF_RDMA_EN or OMAP_I2C_BUF_XDMA_EN in OMAP_I2C_BUF_REG. Once the DMA request bit is cleared in OMAP_I2C_BUF_REG, can pending FIFO threshold conditions trigger XRDY/RRDY/XDR/RDR interrupts and cause omap_i2c_isr_thread() -> omap_i2c_xfer_data() to read or write OMAP_I2C_DATA_REG and omap->buf concurrently with omap_i2c_dma_cleanup() and omap_i2c_reset()? [ ... ] > @@ -1478,6 +1742,22 @@ omap_i2c_probe(struct platform_device *pdev) > goto err_unuse_clocks; > } > > + if (omap->scheme != OMAP_I2C_SCHEME_0) { > + omap->dma = omap_i2c_dma_init(omap, (dma_addr_t)res->start); > + if (PTR_ERR(omap->dma) == -EPROBE_DEFER) { > + r = -EPROBE_DEFER; > + dev_err_probe(&pdev->dev, r, > + "can't get DMA channels\n"); > + goto err_unuse_clocks; > + } else if (PTR_ERR(omap->dma) == -ENODEV) { > + dev_dbg(omap->dev, "DMA setup failed\n"); > + omap->dma = NULL; > + } else if (IS_ERR(omap->dma)) { > + r = PTR_ERR(omap->dma); > + goto err_unuse_clocks; > + } > + } > + > adap = &omap->adapter; > i2c_set_adapdata(adap, omap); > adap->owner = THIS_MODULE; [Severity: Medium] Does omap_i2c_probe() in drivers/i2c/busses/i2c-omap.c leak dma->chan_tx and dma->chan_rx if i2c_add_numbered_adapter() fails? omap_i2c_dma_init() acquires dma->chan_tx and dma->chan_rx using dma_request_chan(). Later in omap_i2c_probe(): r = i2c_add_numbered_adapter(adap); if (r) goto err_unuse_clocks; The err_unuse_clocks error path does not call omap_i2c_dma_free(omap) when omap->dma is set: err_unuse_clocks: omap_i2c_write_reg(omap, OMAP_I2C_CON_REG, 0); err_put_pm: pm_runtime_put_sync(omap->dev); err_disable_pm: pm_runtime_dont_use_autosuspend(omap->dev); pm_runtime_disable(&pdev->dev); return r; Should the error unwind path release the DMA channels with omap_i2c_dma_free(omap) when omap->dma is non-NULL? -- Sashiko AI review · https://sashiko.dev/#/patchset/20261105200556.34240-2-apokusinski01@gmail.com?part=3 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-11-09 13:21 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-11-05 20:05 [PATCH 0/3] i2c: omap: add support for DMA Antoni Pokusinski 2026-11-05 20:05 ` [PATCH 1/3] dt-bindings: i2c: ti,omap4-i2c: Add DMA properties Antoni Pokusinski 2026-11-09 13:21 ` Krzysztof Kozlowski 2026-11-05 20:05 ` [PATCH 2/3] ARM: dts: am33xx-l4: Add missing DMAs for I2C controllers Antoni Pokusinski 2026-11-05 20:05 ` [PATCH 3/3] i2c: omap: add DMA support Antoni Pokusinski 2026-11-05 20:32 ` sashiko-bot
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.