Re: [PATCH v1 2/3] i2c: imx: separate atomic, dma and non-dma use case

From: Frank Li
Date: Mon Jul 15 2024 - 12:45:05 EST

On Mon, Jul 15, 2024 at 05:17:52PM +0200, Stefan Eichenberger wrote:
> From: Stefan Eichenberger <stefan.eichenberger@xxxxxxxxxxx>
>
> Separate the atomic, dma and non-dma use case as a preparation step for
> moving the non-dma use case to the isr to avoid rescheduling while a
> transfer is in progress.
>

Reviewed-by: Frank Li <Frank.Li@xxxxxxx>
> Signed-off-by: Stefan Eichenberger <stefan.eichenberger@xxxxxxxxxxx>
> ---
> drivers/i2c/busses/i2c-imx.c | 107 +++++++++++++++++++++++------------
> 1 file changed, 70 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c
> index 1add946e3bc20..e242166cb6638 100644
> --- a/drivers/i2c/busses/i2c-imx.c
> +++ b/drivers/i2c/busses/i2c-imx.c
> @@ -1011,6 +1011,43 @@ static int i2c_imx_dma_write(struct imx_i2c_struct *i2c_imx,
> return i2c_imx_acked(i2c_imx);
> }
>
> +static int i2c_imx_start_read(struct imx_i2c_struct *i2c_imx,
> + struct i2c_msg *msgs, bool atomic,
> + bool use_dma)
> +{
> + int result;
> + unsigned int temp = 0;
> +
> + /* write slave address */
> + imx_i2c_write_reg(i2c_8bit_addr_from_msg(msgs), i2c_imx, IMX_I2C_I2DR);
> + result = i2c_imx_trx_complete(i2c_imx, atomic);
> + if (result)
> + return result;
> + result = i2c_imx_acked(i2c_imx);
> + if (result)
> + return result;
> +
> + dev_dbg(&i2c_imx->adapter.dev, "<%s> setup bus\n", __func__);
> +
> + /* setup bus to read data */
> + temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> + temp &= ~I2CR_MTX;
> +
> + /*
> + * Reset the I2CR_TXAK flag initially for SMBus block read since the
> + * length is unknown
> + */
> + if (msgs->len - 1)
> + temp &= ~I2CR_TXAK;
> + if (use_dma)
> + temp |= I2CR_DMAEN;
> +
> + imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> + imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR); /* dummy read */
> +
> + return 0;
> +}
> +
> static int i2c_imx_dma_read(struct imx_i2c_struct *i2c_imx,
> struct i2c_msg *msgs, bool is_lastmsg)
> {
> @@ -1021,6 +1058,11 @@ static int i2c_imx_dma_read(struct imx_i2c_struct *i2c_imx,
> struct imx_i2c_dma *dma = i2c_imx->dma;
> struct device *dev = &i2c_imx->adapter.dev;
>
> + result = i2c_imx_start_read(i2c_imx, msgs, false, true);
> + if (result)
> + return result;
> +
> + dev_dbg(&i2c_imx->adapter.dev, "<%s> read data\n", __func__);
>
> dma->chan_using = dma->chan_rx;
> dma->dma_transfer_dir = DMA_DEV_TO_MEM;
> @@ -1131,50 +1173,24 @@ static int i2c_imx_write(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs,
> return 0;
> }
>
> +static int i2c_imx_atomic_write(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs)
> +{
> + return i2c_imx_write(i2c_imx, msgs, true);
> +}
> +
> static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs,
> bool is_lastmsg, bool atomic)
> {
> int i, result;
> unsigned int temp;
> int block_data = msgs->flags & I2C_M_RECV_LEN;
> - int use_dma = i2c_imx->dma && msgs->flags & I2C_M_DMA_SAFE &&
> - msgs->len >= DMA_THRESHOLD && !block_data;
>
> - dev_dbg(&i2c_imx->adapter.dev,
> - "<%s> write slave address: addr=0x%x\n",
> - __func__, i2c_8bit_addr_from_msg(msgs));
> -
> - /* write slave address */
> - imx_i2c_write_reg(i2c_8bit_addr_from_msg(msgs), i2c_imx, IMX_I2C_I2DR);
> - result = i2c_imx_trx_complete(i2c_imx, atomic);
> + result = i2c_imx_start_read(i2c_imx, msgs, atomic, false);
> if (result)
> return result;
> - result = i2c_imx_acked(i2c_imx);
> - if (result)
> - return result;
> -
> - dev_dbg(&i2c_imx->adapter.dev, "<%s> setup bus\n", __func__);
> -
> - /* setup bus to read data */
> - temp = imx_i2c_read_reg(i2c_imx, IMX_I2C_I2CR);
> - temp &= ~I2CR_MTX;
> -
> - /*
> - * Reset the I2CR_TXAK flag initially for SMBus block read since the
> - * length is unknown
> - */
> - if ((msgs->len - 1) || block_data)
> - temp &= ~I2CR_TXAK;
> - if (use_dma)
> - temp |= I2CR_DMAEN;
> - imx_i2c_write_reg(temp, i2c_imx, IMX_I2C_I2CR);
> - imx_i2c_read_reg(i2c_imx, IMX_I2C_I2DR); /* dummy read */
>
> dev_dbg(&i2c_imx->adapter.dev, "<%s> read data\n", __func__);
>
> - if (use_dma)
> - return i2c_imx_dma_read(i2c_imx, msgs, is_lastmsg);
> -
> /* read data */
> for (i = 0; i < msgs->len; i++) {
> u8 len = 0;
> @@ -1241,6 +1257,12 @@ static int i2c_imx_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs,
> return 0;
> }
>
> +static int i2c_imx_atomic_read(struct imx_i2c_struct *i2c_imx, struct i2c_msg *msgs,
> + bool is_lastmsg)
> +{
> + return i2c_imx_read(i2c_imx, msgs, is_lastmsg, true);
> +}
> +
> static int i2c_imx_xfer_common(struct i2c_adapter *adapter,
> struct i2c_msg *msgs, int num, bool atomic)
> {
> @@ -1248,6 +1270,7 @@ static int i2c_imx_xfer_common(struct i2c_adapter *adapter,
> int result;
> bool is_lastmsg = false;
> struct imx_i2c_struct *i2c_imx = i2c_get_adapdata(adapter);
> + int use_dma = 0;
>
> /* Start I2C transfer */
> result = i2c_imx_start(i2c_imx, atomic);
> @@ -1300,15 +1323,25 @@ static int i2c_imx_xfer_common(struct i2c_adapter *adapter,
> (temp & I2SR_SRW ? 1 : 0), (temp & I2SR_IIF ? 1 : 0),
> (temp & I2SR_RXAK ? 1 : 0));
> #endif
> +
> + use_dma = i2c_imx->dma && msgs[i].len >= DMA_THRESHOLD &&
> + msgs[i].flags & I2C_M_DMA_SAFE;
> if (msgs[i].flags & I2C_M_RD) {
> - result = i2c_imx_read(i2c_imx, &msgs[i], is_lastmsg, atomic);
> + int block_data = msgs->flags & I2C_M_RECV_LEN;
> +
> + if (atomic)
> + result = i2c_imx_atomic_read(i2c_imx, &msgs[i], is_lastmsg);
> + else if (use_dma && !block_data)
> + result = i2c_imx_dma_read(i2c_imx, &msgs[i], is_lastmsg);
> + else
> + result = i2c_imx_read(i2c_imx, &msgs[i], is_lastmsg, false);
> } else {
> - if (!atomic &&
> - i2c_imx->dma && msgs[i].len >= DMA_THRESHOLD &&
> - msgs[i].flags & I2C_M_DMA_SAFE)
> + if (atomic)
> + result = i2c_imx_atomic_write(i2c_imx, &msgs[i]);
> + else if (use_dma)
> result = i2c_imx_dma_write(i2c_imx, &msgs[i]);
> else
> - result = i2c_imx_write(i2c_imx, &msgs[i], atomic);
> + result = i2c_imx_write(i2c_imx, &msgs[i], false);
> }
> if (result)
> goto fail0;
> --
> 2.43.0
>