Hi Wolfram,
Thank you for the patch.
On Friday 31 October 2014 11:51:16 Wolfram Sang wrote:
> From: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
>
> Make it possible to transfer i2c message buffers via DMA.
> Start/Stop/Sending_Slave_Adress is still handled using the old state
> machine, it is sending the actual data that is done via DMA. This is
> least intrusive and allows us to work with the message buffers directly
> instead of preparing a custom buffer which involves copying the data
> around.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
> ---
> .../devicetree/bindings/i2c/i2c-sh_mobile.txt | 5 +
> drivers/i2c/busses/i2c-sh_mobile.c | 203 ++++++++++++++++--
> 2 files changed, 189 insertions(+), 19 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/i2c/i2c-sh_mobile.txt
> b/Documentation/devicetree/bindings/i2c/i2c-sh_mobile.txt index
> d2153ce36fa8..58dcf7d71e9c 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c-sh_mobile.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c-sh_mobile.txt
> @@ -10,6 +10,11 @@ Required properties:
>
> Optional properties:
> - clock-frequency : frequency of bus clock in Hz. Default 100kHz if unset.
> +- dmas : Must contain a list of two references to DMA
> + specifiers, one for transmission, and one for
> + reception.
> +- dma-names : Must contain a list of two DMA names, "tx" and "rx".
> +
>
> Pinctrl properties might be needed, too. See there.
>
> diff --git a/drivers/i2c/busses/i2c-sh_mobile.c
> b/drivers/i2c/busses/i2c-sh_mobile.c index 8b5e79cb4468..23664b21ab37
> 100644
> --- a/drivers/i2c/busses/i2c-sh_mobile.c
> +++ b/drivers/i2c/busses/i2c-sh_mobile.c
> @@ -1,6 +1,8 @@
> /*
> * SuperH Mobile I2C Controller
> *
> + * Copyright (C) 2014 Wolfram Sang <wsa@xxxxxxxxxxxxxxxxxxxx>
> + *
> * Copyright (C) 2008 Magnus Damm
> *
> * Portions of the code based on out-of-tree driver i2c-sh7343.c
> @@ -33,6 +35,9 @@
> #include <linux/io.h>
> #include <linux/slab.h>
> #include <linux/of_device.h>
> +#include <linux/dmaengine.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/sh_dma.h>
I would have tried to keep the headers alphabetically sorted, if they had been
sorted in the first place :-)
> #include <linux/i2c/i2c-sh_mobile.h>
>
> /* Transmit operation:
> */ @@ -114,6 +119,7 @@ enum sh_mobile_i2c_op {
> OP_TX_FIRST,
> OP_TX,
> OP_TX_STOP,
> + OP_TX_STOP_DATA,
> OP_TX_TO_RX,
> OP_RX,
> OP_RX_STOP,
> @@ -138,6 +144,12 @@ struct sh_mobile_i2c_data {
> int pos;
> int sr;
> bool send_stop;
> +
> + struct dma_chan *dma_tx;
> + struct dma_chan *dma_rx;
> + struct scatterlist sg;
> + enum dma_data_direction dma_direction;
> + bool buf_mapped;
> };
>
> struct sh_mobile_dt_config {
> @@ -175,6 +187,8 @@ struct sh_mobile_dt_config {
>
> #define ICIC_ICCLB8 0x80
> #define ICIC_ICCHB8 0x40
> +#define ICIC_TDMAE 0x20
> +#define ICIC_RDMAE 0x10
> #define ICIC_ALE 0x08
> #define ICIC_TACKE 0x04
> #define ICIC_WAITE 0x02
> @@ -336,8 +350,10 @@ static unsigned char i2c_op(struct sh_mobile_i2c_data
> *pd, case OP_TX: /* write data */
> iic_wr(pd, ICDR, data);
> break;
> - case OP_TX_STOP: /* write data and issue a stop afterwards */
> + case OP_TX_STOP_DATA: /* write data and issue a stop afterwards */
> iic_wr(pd, ICDR, data);
> + /* fallthrough */
> + case OP_TX_STOP: /* issue a stop */
> iic_wr(pd, ICCR, pd->send_stop ? ICCR_ICE | ICCR_TRS
>
> : ICCR_ICE | ICCR_TRS | ICCR_BBSY);
>
> break;
> @@ -393,13 +409,17 @@ static int sh_mobile_i2c_isr_tx(struct
> sh_mobile_i2c_data *pd) {
> unsigned char data;
>
> - if (pd->pos == pd->msg->len)
> + if (pd->pos == pd->msg->len) {
> + /* Send stop if we haven't yet (DMA case) */
> + if (pd->send_stop && (iic_rd(pd, ICCR) & ICCR_BBSY))
> + i2c_op(pd, OP_TX_STOP, data);
> return 1;
> + }
>
> sh_mobile_i2c_get_data(pd, &data);
>
> if (sh_mobile_i2c_is_last_byte(pd))
> - i2c_op(pd, OP_TX_STOP, data);
> + i2c_op(pd, OP_TX_STOP_DATA, data);
> else if (sh_mobile_i2c_is_first_byte(pd))
> i2c_op(pd, OP_TX_FIRST, data);
> else
> @@ -454,7 +474,7 @@ static irqreturn_t sh_mobile_i2c_isr(int irq, void
> *dev_id) struct platform_device *dev = dev_id;
> struct sh_mobile_i2c_data *pd = platform_get_drvdata(dev);
> unsigned char sr;
> - int wakeup;
> + int wakeup = 0;
>
> sr = iic_rd(pd, ICSR);
> pd->sr |= sr; /* remember state */
> @@ -463,15 +483,21 @@ static irqreturn_t sh_mobile_i2c_isr(int irq, void
> *dev_id) (pd->msg->flags & I2C_M_RD) ? "read" : "write",
> pd->pos, pd->msg->len);
>
> - if (sr & (ICSR_AL | ICSR_TACK)) {
> + /* Kick off TxDMA after preface was done */
> + if (pd->dma_direction == DMA_TO_DEVICE && pd->pos == 0)
> + iic_set_clr(pd, ICIC, ICIC_TDMAE, 0);
> + else if (sr & (ICSR_AL | ICSR_TACK))
> /* don't interrupt transaction - continue to issue stop */
> iic_wr(pd, ICSR, sr & ~(ICSR_AL | ICSR_TACK));
> - wakeup = 0;
> - } else if (pd->msg->flags & I2C_M_RD)
> + else if (pd->msg->flags & I2C_M_RD)
> wakeup = sh_mobile_i2c_isr_rx(pd);
> else
> wakeup = sh_mobile_i2c_isr_tx(pd);
>
> + /* Kick off RxDMA after preface was done */
> + if (pd->dma_direction == DMA_FROM_DEVICE && pd->pos == 1)
> + iic_set_clr(pd, ICIC, ICIC_RDMAE, 0);
> +
> if (sr & ICSR_WAIT) /* TODO: add delay here to support slow acks */
> iic_wr(pd, ICSR, sr & ~ICSR_WAIT);
>
> @@ -486,6 +512,82 @@ static irqreturn_t sh_mobile_i2c_isr(int irq, void
> *dev_id) return IRQ_HANDLED;
> }
>
> +static void sh_mobile_i2c_cleanup_dma(struct sh_mobile_i2c_data *pd)
> +{
> + if (pd->dma_direction == DMA_FROM_DEVICE)
> + dmaengine_terminate_all(pd->dma_rx);
> + if (pd->dma_direction == DMA_TO_DEVICE)
else ?
> + dmaengine_terminate_all(pd->dma_tx);
> +
> + if (pd->buf_mapped) {
> + dma_unmap_single(pd->dev, sg_dma_address(&pd->sg),
> + pd->msg->len, pd->dma_direction);
> + pd->buf_mapped = false;
> + }
> +
> + pd->dma_direction = DMA_NONE;
> +}
> +
> +static void sh_mobile_i2c_dma_callback(void *data)
> +{
> + struct sh_mobile_i2c_data *pd = data;
> +
> + dma_unmap_single(pd->dev, sg_dma_address(&pd->sg),
> + pd->msg->len, pd->dma_direction);
> +
> + pd->buf_mapped = false;
> + pd->dma_direction = DMA_NONE;
> + pd->pos = pd->msg->len;
> +
> + iic_set_clr(pd, ICIC, 0, ICIC_TDMAE | ICIC_RDMAE);
> +}
> +
> +static void sh_mobile_i2c_xfer_dma(struct sh_mobile_i2c_data *pd)
> +{
> + bool read = pd->msg->flags & I2C_M_RD;
> + enum dma_data_direction dir = read ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
> + struct dma_chan *chan = read ? pd->dma_rx : pd->dma_tx;
> + struct dma_async_tx_descriptor *txdesc;
> + dma_addr_t dma_addr;
> + dma_cookie_t cookie;
> +
> + if (!chan)
> + return;
> +
> + dma_addr = dma_map_single(pd->dev, pd->msg->buf, pd->msg->len, dir);
> + if (dma_mapping_error(pd->dev, dma_addr)) {
> + dev_warn(pd->dev, "dma map failed, using PIO\n");
> + return;
> + }
> +
> + sg_dma_len(&pd->sg) = pd->msg->len;
> + sg_dma_address(&pd->sg) = dma_addr;
> +
> + pd->buf_mapped = true;
Can't you use dma_direction != DMA_NONE to detect whether the buffer has been
mapped, instead of adding a new field to struct sh_mobile_i2c_data ? You could
then simplify sh_mobile_i2c_cleanup_dma() by returning immediately at the
beginning of the function if dma_direction == DMA_NONE.
> + pd->dma_direction = dir;
> +
> + txdesc = dmaengine_prep_slave_sg(chan, &pd->sg, 1,
> + read ? DMA_DEV_TO_MEM : DMA_MEM_TO_DEV,
> + DMA_PREP_INTERRUPT | DMA_CTRL_ACK);
> + if (!txdesc) {
> + dev_warn(pd->dev, "dma prep slave sg failed, using PIO\n");
> + sh_mobile_i2c_cleanup_dma(pd);
> + return;
> + }
> +
> + txdesc->callback = sh_mobile_i2c_dma_callback;
> + txdesc->callback_param = pd;
> +
> + cookie = dmaengine_submit(txdesc);
> + if (dma_submit_error(cookie)) {
> + dev_warn(pd->dev, "submitting dma failed, using PIO\n");
> + sh_mobile_i2c_cleanup_dma(pd);
> + return;
> + }
> +
> + dma_async_issue_pending(chan);
> +}
> +
> static int start_ch(struct sh_mobile_i2c_data *pd, struct i2c_msg *usr_msg,
> bool do_init)
> {
> @@ -510,6 +612,9 @@ static int start_ch(struct sh_mobile_i2c_data *pd,
> struct i2c_msg *usr_msg, pd->pos = -1;
> pd->sr = 0;
>
> + if (pd->msg->len > 4)
> + sh_mobile_i2c_xfer_dma(pd);
> +
> /* Enable all interrupts to begin with */
> iic_wr(pd, ICIC, ICIC_DTEE | ICIC_WAITE | ICIC_ALE | ICIC_TACKE);
> return 0;
> @@ -593,6 +698,9 @@ static int sh_mobile_i2c_xfer(struct i2c_adapter
> *adapter, 5 * HZ);
> if (!k) {
> dev_err(pd->dev, "Transfer request timed out\n");
> + if (pd->dma_direction != DMA_NONE)
> + sh_mobile_i2c_cleanup_dma(pd);
> +
> err = -ETIMEDOUT;
> break;
> }
> @@ -641,6 +749,70 @@ static const struct of_device_id sh_mobile_i2c_dt_ids[]
> = { };
> MODULE_DEVICE_TABLE(of, sh_mobile_i2c_dt_ids);
>
> +static struct dma_chan *sh_mobile_i2c_request_dma_chan(struct device *dev,
> + enum dma_transfer_direction dir,
> + dma_addr_t port_addr)
> +{
> + dma_cap_mask_t mask;
> + struct dma_chan *chan;
> + struct dma_slave_config cfg;
> + int ret;
> +
> + dma_cap_zero(mask);
> + dma_cap_set(DMA_SLAVE, mask);
> +
> + chan = dma_request_slave_channel_compat(mask, shdma_chan_filter,
> + (void *)0UL, dev, dir == DMA_MEM_TO_DEV ? "tx" : "rx");
> +
> + if (!chan)
> + return NULL;
> +
> + memset(&cfg, 0, sizeof(cfg));
> + cfg.direction = dir;
> + if (dir == DMA_MEM_TO_DEV) {
> + cfg.dst_addr = port_addr;
> + cfg.dst_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
> + } else {
> + cfg.src_addr = port_addr;
> + cfg.src_addr_width = DMA_SLAVE_BUSWIDTH_1_BYTE;
> + }
> +
> + ret = dmaengine_slave_config(chan, &cfg);
> + if (ret) {
> + dev_warn(dev, "dmaengine_slave_config failed %d\n", ret);
> + dma_release_channel(chan);
> + return NULL;
> + }
> +
> + return chan;
> +}
> +
> +static void sh_mobile_i2c_request_dma(struct sh_mobile_i2c_data *pd, const
> struct resource *res)
> +{
> + pd->buf_mapped = false;
> + pd->dma_direction = DMA_NONE;
> + sg_init_table(&pd->sg, 1);
> +
> + pd->dma_rx = sh_mobile_i2c_request_dma_chan(pd->dev, DMA_DEV_TO_MEM,
> + res->start + ICDR);
> +
> + pd->dma_tx = sh_mobile_i2c_request_dma_chan(pd->dev, DMA_MEM_TO_DEV,
> + res->start + ICDR);
> +}
> +
> +static void sh_mobile_i2c_release_dma(struct sh_mobile_i2c_data *pd)
> +{
> + if (pd->dma_tx) {
> + dma_release_channel(pd->dma_tx);
> + pd->dma_tx = NULL;
> + }
> +
> + if (pd->dma_rx) {
> + dma_release_channel(pd->dma_rx);
> + pd->dma_rx = NULL;
> + }
> +}
> +
> static int sh_mobile_i2c_hook_irqs(struct platform_device *dev)
> {
> struct resource *res;
> @@ -727,6 +899,8 @@ static int sh_mobile_i2c_probe(struct platform_device
> *dev) if (ret)
> return ret;
>
> + sh_mobile_i2c_request_dma(pd, res);
> +
> /* Enable Runtime PM for this device.
> *
> * Also tell the Runtime PM core to ignore children
> @@ -758,6 +932,7 @@ static int sh_mobile_i2c_probe(struct platform_device
> *dev)
>
> ret = i2c_add_numbered_adapter(adap);
> if (ret < 0) {
> + sh_mobile_i2c_release_dma(pd);
> dev_err(&dev->dev, "cannot add numbered adapter\n");
> return ret;
> }
> @@ -774,6 +949,7 @@ static int sh_mobile_i2c_remove(struct platform_device
> *dev) struct sh_mobile_i2c_data *pd = platform_get_drvdata(dev);
>
> i2c_del_adapter(&pd->adap);
> + sh_mobile_i2c_release_dma(pd);
> pm_runtime_disable(&dev->dev);
> return 0;
> }
> @@ -805,19 +981,8 @@ static struct platform_driver sh_mobile_i2c_driver = {
> .probe = sh_mobile_i2c_probe,
> .remove = sh_mobile_i2c_remove,
> };
> +module_platform_driver(sh_mobile_i2c_driver);
>
> -static int __init sh_mobile_i2c_adap_init(void)
> -{
> - return platform_driver_register(&sh_mobile_i2c_driver);
> -}
> -
> -static void __exit sh_mobile_i2c_adap_exit(void)
> -{
> - platform_driver_unregister(&sh_mobile_i2c_driver);
> -}
> -
> -subsys_initcall(sh_mobile_i2c_adap_init);
> -module_exit(sh_mobile_i2c_adap_exit);
>
Extra blank line here.
> MODULE_DESCRIPTION("SuperH Mobile I2C Bus Controller driver");
> MODULE_AUTHOR("Magnus Damm");
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
