[v6] i2c: qcom-geni: Add support for GPI DMA

QUP Serial engines supports data transfers thru FIFO mode, SE DMA mode
and lastly GPI DMA mode. Former two are already supported and this adds
supports for the last mode.

In GPI DMA mode, the firmware is issued commands by driver to perform
DMA and setup the serial port.

Signed-off-by: Vinod Koul <vkoul@kernel.org>
---

Changes since v5:
 - Fix comments by Bjorn, Dmitry and Jordan

Changes since v4:
 - Fix buildbot warn
 - Fix flase warn reported by Alexey
 - Fix feedback from Bjorn and cleanup the probe code and add more details
   in changelog

Changes since v3:
 - remove separate tx and rx function for gsi dma and make a common one
 - remove global structs and use local variables instead


 drivers/i2c/busses/i2c-qcom-geni.c | 303 ++++++++++++++++++++++++++---
 1 file changed, 275 insertions(+), 28 deletions(-)

On Fri 18 Feb 10:55 PST 2022, Vinod Koul wrote:

> QUP Serial engines supports data transfers thru FIFO mode, SE DMA mode
> and lastly GPI DMA mode. Former two are already supported and this adds
> supports for the last mode.
> 
> In GPI DMA mode, the firmware is issued commands by driver to perform
> DMA and setup the serial port.
> 
> Signed-off-by: Vinod Koul <vkoul@kernel.org>
> ---
> 
> Changes since v5:
>  - Fix comments by Bjorn, Dmitry and Jordan

Thanks, but it's useful to know what those things where. Please list the
actual changes.

> 
> Changes since v4:
>  - Fix buildbot warn
>  - Fix flase warn reported by Alexey
>  - Fix feedback from Bjorn and cleanup the probe code and add more details
>    in changelog
> 
> Changes since v3:
>  - remove separate tx and rx function for gsi dma and make a common one
>  - remove global structs and use local variables instead
> 
> 
>  drivers/i2c/busses/i2c-qcom-geni.c | 303 ++++++++++++++++++++++++++---
>  1 file changed, 275 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index 6d635a7c104c..e03544ba827c 100644
> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
> @@ -3,7 +3,9 @@
>  
>  #include <linux/acpi.h>
>  #include <linux/clk.h>
> +#include <linux/dmaengine.h>
>  #include <linux/dma-mapping.h>
> +#include <linux/dma/qcom-gpi-dma.h>
>  #include <linux/err.h>
>  #include <linux/i2c.h>
>  #include <linux/interrupt.h>
> @@ -48,6 +50,9 @@
>  #define LOW_COUNTER_SHFT	10
>  #define CYCLE_COUNTER_MSK	GENMASK(9, 0)
>  
> +#define I2C_PACK_TX		BIT(0)
> +#define I2C_PACK_RX		BIT(1)
> +
>  enum geni_i2c_err_code {
>  	GP_IRQ0,
>  	NACK,
> @@ -89,6 +94,9 @@ struct geni_i2c_dev {
>  	void *dma_buf;
>  	size_t xfer_len;
>  	dma_addr_t dma_addr;
> +	struct dma_chan *tx_c;
> +	struct dma_chan *rx_c;
> +	bool gpi_mode;
>  };
>  
>  struct geni_i2c_err_log {
> @@ -456,12 +464,202 @@ static int geni_i2c_tx_one_msg(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
>  	return gi2c->err;
>  }
>  
> +static void i2c_gpi_cb_result(void *cb, const struct dmaengine_result *result)
> +{
> +	struct geni_i2c_dev *gi2c = cb;
> +
> +	if (result->result != DMA_TRANS_NOERROR) {
> +		dev_err(gi2c->se.dev, "DMA txn failed:%d\n", result->result);
> +		goto err_trans;

Maybe I'm just missing it, but aren't you completing done now to avoid
the timeout, which causes the xfer to return success?

Note that, as I mentioned in previous review, we're under the adaptor
lock, so I don't think you need any additional locking to pass a value
back to xfer().

> +	}
> +
> +	if (result->residue)

You can avoid the goto, and make it clear from the overall structure that
only one (or none) of these paths will be taken.

	if (NOERROR) {

	} else if (result->residue) {

	}

Regards,
Bjorn

> +		dev_dbg(gi2c->se.dev, "DMA xfer has pending: %d\n", result->residue);
> +
> +err_trans:
> +	complete(&gi2c->done);
> +}
> +
> +static void geni_i2c_gpi_unmap(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
> +			       void *tx_buf, dma_addr_t tx_addr,
> +			       void *rx_buf, dma_addr_t rx_addr)
> +{
> +	if (tx_buf) {
> +		dma_unmap_single(gi2c->se.dev->parent, tx_addr, msg->len, DMA_TO_DEVICE);
> +		i2c_put_dma_safe_msg_buf(tx_buf, msg, false);
> +	}
> +
> +	if (rx_buf) {
> +		dma_unmap_single(gi2c->se.dev->parent, rx_addr, msg->len, DMA_FROM_DEVICE);
> +		i2c_put_dma_safe_msg_buf(rx_buf, msg, false);
> +	}
> +}
> +
> +static int geni_i2c_gpi(struct geni_i2c_dev *gi2c, struct i2c_msg *msg,
> +			   struct dma_slave_config *config, dma_addr_t *dma_addr_p,
> +			   void **buf, unsigned int op, struct dma_chan *dma_chan)
> +{
> +	struct gpi_i2c_config *peripheral;
> +	unsigned int flags;
> +	void *dma_buf;
> +	dma_addr_t addr;
> +	enum dma_data_direction map_dirn;
> +	enum dma_transfer_direction dma_dirn;
> +	struct dma_async_tx_descriptor *desc;
> +	int ret;
> +
> +	peripheral = config->peripheral_config;
> +
> +	dma_buf = i2c_get_dma_safe_msg_buf(msg, 1);
> +	if (!dma_buf)
> +		return -ENOMEM;
> +
> +	if (op == I2C_WRITE)
> +		map_dirn = DMA_TO_DEVICE;
> +	else
> +		map_dirn = DMA_FROM_DEVICE;
> +
> +	addr = dma_map_single(gi2c->se.dev->parent, dma_buf, msg->len, map_dirn);
> +	if (dma_mapping_error(gi2c->se.dev->parent, addr)) {
> +		i2c_put_dma_safe_msg_buf(dma_buf, msg, false);
> +		return -ENOMEM;
> +	}
> +
> +	/* set the length as message for rx txn */
> +	peripheral->rx_len = msg->len;
> +	peripheral->op = op;
> +
> +	ret = dmaengine_slave_config(dma_chan, config);
> +	if (ret) {
> +		dev_err(gi2c->se.dev, "dma config error: %d for op:%d\n", ret, op);
> +		goto err_config;
> +	}
> +
> +	peripheral->set_config = 0;
> +	peripheral->multi_msg = true;
> +	flags = DMA_PREP_INTERRUPT | DMA_CTRL_ACK;
> +
> +	if (op == I2C_WRITE)
> +		dma_dirn = DMA_MEM_TO_DEV;
> +	else
> +		dma_dirn = DMA_DEV_TO_MEM;
> +
> +	desc = dmaengine_prep_slave_single(dma_chan, addr, msg->len, dma_dirn, flags);
> +	if (!desc) {
> +		dev_err(gi2c->se.dev, "prep_slave_sg failed\n");
> +		ret = -EIO;
> +		goto err_config;
> +	}
> +
> +	desc->callback_result = i2c_gpi_cb_result;
> +	desc->callback_param = gi2c;
> +
> +	dmaengine_submit(desc);
> +	*dma_addr_p = addr;
> +
> +	return 0;
> +
> +err_config:
> +	dma_unmap_single(gi2c->se.dev->parent, addr, msg->len, map_dirn);
> +	i2c_put_dma_safe_msg_buf(dma_buf, msg, false);
> +	return ret;
> +}
> +
> +static int geni_i2c_gpi_xfer(struct geni_i2c_dev *gi2c, struct i2c_msg msgs[], int num)
> +{
> +	struct dma_slave_config config = {};
> +	struct gpi_i2c_config peripheral = {};
> +	int i, ret = 0, timeout;
> +	dma_addr_t tx_addr, rx_addr;
> +	void *tx_buf = NULL, *rx_buf = NULL;
> +	const struct geni_i2c_clk_fld *itr = gi2c->clk_fld;
> +
> +	config.peripheral_config = &peripheral;
> +	config.peripheral_size = sizeof(peripheral);
> +
> +	peripheral.pack_enable = I2C_PACK_TX | I2C_PACK_RX;
> +	peripheral.cycle_count = itr->t_cycle_cnt;
> +	peripheral.high_count = itr->t_high_cnt;
> +	peripheral.low_count = itr->t_low_cnt;
> +	peripheral.clk_div = itr->clk_div;
> +	peripheral.set_config = 1;
> +	peripheral.multi_msg = false;
> +
> +	for (i = 0; i < num; i++) {
> +		gi2c->cur = &msgs[i];
> +		dev_dbg(gi2c->se.dev, "msg[%d].len:%d\n", i, gi2c->cur->len);
> +
> +		peripheral.stretch = 0;
> +		if (i < num - 1)
> +			peripheral.stretch = 1;
> +
> +		peripheral.addr = msgs[i].addr;
> +
> +		if (msgs[i].flags & I2C_M_RD) {
> +			ret =  geni_i2c_gpi(gi2c, &msgs[i], &config,
> +					    &rx_addr, &rx_buf, I2C_READ, gi2c->rx_c);
> +			if (ret)
> +				goto err;
> +		}
> +
> +		ret =  geni_i2c_gpi(gi2c, &msgs[i], &config,
> +				    &tx_addr, &tx_buf, I2C_WRITE, gi2c->tx_c);
> +		if (ret)
> +			goto err;
> +
> +		if (msgs[i].flags & I2C_M_RD)
> +			dma_async_issue_pending(gi2c->rx_c);
> +		dma_async_issue_pending(gi2c->tx_c);
> +
> +		timeout = wait_for_completion_timeout(&gi2c->done, XFER_TIMEOUT);
> +		if (!timeout) {
> +			dev_err(gi2c->se.dev, "I2C timeout gpi flags:%d addr:0x%x\n",
> +				gi2c->cur->flags, gi2c->cur->addr);
> +			ret = gi2c->err = -ETIMEDOUT;
> +			goto err;
> +		}
> +
> +		geni_i2c_gpi_unmap(gi2c, &msgs[i], tx_buf, tx_addr, rx_buf, rx_addr);
> +	}
> +
> +	return num;
> +
> +err:
> +	dmaengine_terminate_sync(gi2c->rx_c);
> +	dmaengine_terminate_sync(gi2c->tx_c);
> +	geni_i2c_gpi_unmap(gi2c, &msgs[i], tx_buf, tx_addr, rx_buf, rx_addr);
> +	return ret;
> +}
> +
> +static int geni_i2c_fifo_xfer(struct geni_i2c_dev *gi2c,
> +			      struct i2c_msg msgs[], int num)
> +{
> +	int i, ret = 0;
> +
> +	for (i = 0; i < num; i++) {
> +		u32 m_param = i < (num - 1) ? STOP_STRETCH : 0;
> +
> +		m_param |= ((msgs[i].addr << SLV_ADDR_SHFT) & SLV_ADDR_MSK);
> +
> +		gi2c->cur = &msgs[i];
> +		if (msgs[i].flags & I2C_M_RD)
> +			ret = geni_i2c_rx_one_msg(gi2c, &msgs[i], m_param);
> +		else
> +			ret = geni_i2c_tx_one_msg(gi2c, &msgs[i], m_param);
> +
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return num;
> +}
> +
>  static int geni_i2c_xfer(struct i2c_adapter *adap,
>  			 struct i2c_msg msgs[],
>  			 int num)
>  {
>  	struct geni_i2c_dev *gi2c = i2c_get_adapdata(adap);
> -	int i, ret;
> +	int ret;
>  
>  	gi2c->err = 0;
>  	reinit_completion(&gi2c->done);
> @@ -475,28 +673,17 @@ static int geni_i2c_xfer(struct i2c_adapter *adap,
>  	}
>  
>  	qcom_geni_i2c_conf(gi2c);
> -	for (i = 0; i < num; i++) {
> -		u32 m_param = i < (num - 1) ? STOP_STRETCH : 0;
> -
> -		m_param |= ((msgs[i].addr << SLV_ADDR_SHFT) & SLV_ADDR_MSK);
> -
> -		gi2c->cur = &msgs[i];
> -		if (msgs[i].flags & I2C_M_RD)
> -			ret = geni_i2c_rx_one_msg(gi2c, &msgs[i], m_param);
> -		else
> -			ret = geni_i2c_tx_one_msg(gi2c, &msgs[i], m_param);
>  
> -		if (ret)
> -			break;
> -	}
> -	if (ret == 0)
> -		ret = num;
> +	if (gi2c->gpi_mode)
> +		ret = geni_i2c_gpi_xfer(gi2c, msgs, num);
> +	else
> +		ret = geni_i2c_fifo_xfer(gi2c, msgs, num);
>  
>  	pm_runtime_mark_last_busy(gi2c->se.dev);
>  	pm_runtime_put_autosuspend(gi2c->se.dev);
>  	gi2c->cur = NULL;
>  	gi2c->err = 0;
> -	return ret;
> +	return num;
>  }
>  
>  static u32 geni_i2c_func(struct i2c_adapter *adap)
> @@ -517,11 +704,50 @@ static const struct acpi_device_id geni_i2c_acpi_match[] = {
>  MODULE_DEVICE_TABLE(acpi, geni_i2c_acpi_match);
>  #endif
>  
> +static void release_gpi_dma(struct geni_i2c_dev *gi2c)
> +{
> +	if (gi2c->rx_c)
> +		dma_release_channel(gi2c->rx_c);
> +
> +	if (gi2c->tx_c)
> +		dma_release_channel(gi2c->tx_c);
> +}
> +
> +static int setup_gpi_dma(struct geni_i2c_dev *gi2c)
> +{
> +	int ret;
> +
> +	geni_se_select_mode(&gi2c->se, GENI_GPI_DMA);
> +	gi2c->tx_c = dma_request_chan(gi2c->se.dev, "tx");
> +	if (IS_ERR(gi2c->tx_c)) {
> +		ret = dev_err_probe(gi2c->se.dev, PTR_ERR(gi2c->tx_c),
> +				    "Failed to get tx DMA ch\n");
> +		if (ret < 0)
> +			goto err_tx;
> +	}
> +
> +	gi2c->rx_c = dma_request_chan(gi2c->se.dev, "rx");
> +	if (IS_ERR(gi2c->rx_c)) {
> +		ret = dev_err_probe(gi2c->se.dev, PTR_ERR(gi2c->rx_c),
> +				    "Failed to get rx DMA ch\n");
> +		if (ret < 0)
> +			goto err_rx;
> +	}
> +
> +	dev_dbg(gi2c->se.dev, "Grabbed GPI dma channels\n");
> +	return 0;
> +
> +err_rx:
> +	dma_release_channel(gi2c->tx_c);
> +err_tx:
> +	return ret;
> +}
> +
>  static int geni_i2c_probe(struct platform_device *pdev)
>  {
>  	struct geni_i2c_dev *gi2c;
>  	struct resource *res;
> -	u32 proto, tx_depth;
> +	u32 proto, tx_depth, fifo_disable;
>  	int ret;
>  	struct device *dev = &pdev->dev;
>  
> @@ -601,27 +827,43 @@ static int geni_i2c_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  	proto = geni_se_read_proto(&gi2c->se);
> -	tx_depth = geni_se_get_tx_fifo_depth(&gi2c->se);
>  	if (proto != GENI_SE_I2C) {
>  		dev_err(dev, "Invalid proto %d\n", proto);
>  		geni_se_resources_off(&gi2c->se);
>  		return -ENXIO;
>  	}
> -	gi2c->tx_wm = tx_depth - 1;
> -	geni_se_init(&gi2c->se, gi2c->tx_wm, tx_depth);
> -	geni_se_config_packing(&gi2c->se, BITS_PER_BYTE, PACKING_BYTES_PW,
> -							true, true, true);
> +
> +	fifo_disable = readl_relaxed(gi2c->se.base + GENI_IF_DISABLE_RO) & FIFO_IF_DISABLE;
> +	if (fifo_disable) {
> +		/* FIFO is disabled, so we can only use GPI DMA */
> +		gi2c->gpi_mode = true;
> +		ret = setup_gpi_dma(gi2c);
> +		if (ret) {
> +			dev_err(dev, "Failed to setup GPI DMA mode:%d ret\n", ret);
> +			return ret;
> +		}
> +
> +		dev_dbg(dev, "Using GPI DMA mode for I2C\n");
> +	} else {
> +		gi2c->gpi_mode = false;
> +		tx_depth = geni_se_get_tx_fifo_depth(&gi2c->se);
> +		gi2c->tx_wm = tx_depth - 1;
> +		geni_se_init(&gi2c->se, gi2c->tx_wm, tx_depth);
> +		geni_se_config_packing(&gi2c->se, BITS_PER_BYTE,
> +				       PACKING_BYTES_PW, true, true, true);
> +
> +		dev_dbg(dev, "i2c fifo/se-dma mode. fifo depth:%d\n", tx_depth);
> +	}
> +
>  	ret = geni_se_resources_off(&gi2c->se);
>  	if (ret) {
>  		dev_err(dev, "Error turning off resources %d\n", ret);
> -		return ret;
> +		goto err_dma;
>  	}
>  
>  	ret = geni_icc_disable(&gi2c->se);
>  	if (ret)
> -		return ret;
> -
> -	dev_dbg(dev, "i2c fifo/se-dma mode. fifo depth:%d\n", tx_depth);
> +		goto err_dma;
>  
>  	gi2c->suspended = 1;
>  	pm_runtime_set_suspended(gi2c->se.dev);
> @@ -633,12 +875,16 @@ static int geni_i2c_probe(struct platform_device *pdev)
>  	if (ret) {
>  		dev_err(dev, "Error adding i2c adapter %d\n", ret);
>  		pm_runtime_disable(gi2c->se.dev);
> -		return ret;
> +		goto err_dma;
>  	}
>  
>  	dev_dbg(dev, "Geni-I2C adaptor successfully added\n");
>  
>  	return 0;
> +
> +err_dma:
> +	release_gpi_dma(gi2c);
> +	return ret;
>  }
>  
>  static int geni_i2c_remove(struct platform_device *pdev)
> @@ -646,6 +892,7 @@ static int geni_i2c_remove(struct platform_device *pdev)
>  	struct geni_i2c_dev *gi2c = platform_get_drvdata(pdev);
>  
>  	i2c_del_adapter(&gi2c->adap);
> +	release_gpi_dma(gi2c);
>  	pm_runtime_disable(gi2c->se.dev);
>  	return 0;
>  }
> -- 
> 2.31.1
>