Re: [PATCH V7 3/5] i2c: tegra: Add DMA Support

From: Dmitry Osipenko
Date: Thu Jan 31 2019 - 13:09:06 EST


31.01.2019 19:55, Dmitry Osipenko ÐÐÑÐÑ:
> config I2C_TEGRA
> tristate "NVIDIA Tegra internal I2C controller"
> depends on ARCH_TEGRA
> select TEGRA20_APB_DMA if (I2C_TEGRA_DMA_SUPPORT && ARCH_TEGRA_2x_SOC)
> select TEGRA20_APB_DMA if (I2C_TEGRA_DMA_SUPPORT && ARCH_TEGRA_3x_SOC)
> select TEGRA20_APB_DMA if (I2C_TEGRA_DMA_SUPPORT && ARCH_TEGRA_114_SOC)
> select TEGRA20_APB_DMA if (I2C_TEGRA_DMA_SUPPORT && ARCH_TEGRA_124_SOC)
> select TEGRA20_APB_DMA if (I2C_TEGRA_DMA_SUPPORT && ARCH_TEGRA_210_SOC)
> help
> If you say yes to this option, support will be included for the
> I2C controller embedded in NVIDIA Tegra SOCs
>
> config I2C_TEGRA_DMA_SUPPORT
> bool "NVIDIA Tegra internal I2C controller DMA support"
> depends on I2C_TEGRA
> help
> If you say yes to this option, DMA engine integration support will
> be included for the I2C controller embedded in NVIDIA Tegra SOCs

Thinking a bit more on it, perhaps this will be an ideal variant:

On top of V8:
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 046aeb92a467..520ead24fb51 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1016,11 +1016,23 @@ config I2C_SYNQUACER

config I2C_TEGRA
tristate "NVIDIA Tegra internal I2C controller"
- depends on (ARCH_TEGRA && TEGRA20_APB_DMA)
+ depends on ARCH_TEGRA
help
If you say yes to this option, support will be included for the
I2C controller embedded in NVIDIA Tegra SOCs

+config I2C_TEGRA_DMA_SUPPORT
+ bool "NVIDIA Tegra internal I2C controller DMA support"
+ depends on I2C_TEGRA
+ depends on TEGRA20_APB_DMA && ARCH_TEGRA_2x_SOC
+ depends on TEGRA20_APB_DMA && ARCH_TEGRA_3x_SOC
+ depends on TEGRA20_APB_DMA && ARCH_TEGRA_114_SOC
+ depends on TEGRA20_APB_DMA && ARCH_TEGRA_124_SOC
+ depends on TEGRA20_APB_DMA && ARCH_TEGRA_210_SOC
+ help
+ If you say yes to this option, DMA engine integration support will
+ be included for the I2C controller embedded in NVIDIA Tegra SOCs
+
config I2C_TEGRA_BPMP
tristate "NVIDIA Tegra BPMP I2C controller"
depends on TEGRA_BPMP
diff --git a/drivers/i2c/busses/i2c-tegra.c b/drivers/i2c/busses/i2c-tegra.c
index fe5b89abc576..bce7283b027b 100644
--- a/drivers/i2c/busses/i2c-tegra.c
+++ b/drivers/i2c/busses/i2c-tegra.c
@@ -225,6 +225,7 @@ struct tegra_i2c_hw_feature {
u32 setup_hold_time_std_mode;
u32 setup_hold_time_fast_fast_plus_mode;
u32 setup_hold_time_hs_mode;
+ bool has_gpc_dma;
};

/**
@@ -389,51 +390,51 @@ static int tegra_i2c_dma_submit(struct tegra_i2c_dev *i2c_dev, size_t len)
return 0;
}

-static int tegra_i2c_init_dma_param(struct tegra_i2c_dev *i2c_dev)
+static int tegra_i2c_init_dma(struct tegra_i2c_dev *i2c_dev)
{
- struct dma_chan *dma_chan;
- u32 *dma_buf;
- dma_addr_t dma_phys;
+ struct device *dev = i2c_dev->dev;
+ int err;

if (!i2c_dev->has_dma)
- return -EINVAL;
-
- if (!i2c_dev->rx_dma_chan) {
- dma_chan = dma_request_slave_channel_reason(i2c_dev->dev, "rx");
- if (IS_ERR(dma_chan))
- return PTR_ERR(dma_chan);
+ return 0;

- i2c_dev->rx_dma_chan = dma_chan;
+ i2c_dev->rx_dma_chan = dma_request_slave_channel_reason(dev, "rx");
+ if (IS_ERR(i2c_dev->rx_dma_chan)) {
+ err = PTR_ERR(i2c_dev->rx_dma_chan);
+ goto err_out;
}

- if (!i2c_dev->tx_dma_chan) {
- dma_chan = dma_request_slave_channel_reason(i2c_dev->dev, "tx");
- if (IS_ERR(dma_chan))
- return PTR_ERR(dma_chan);
-
- i2c_dev->tx_dma_chan = dma_chan;
+ i2c_dev->tx_dma_chan = dma_request_slave_channel_reason(dev, "tx");
+ if (IS_ERR(i2c_dev->tx_dma_chan)) {
+ err = PTR_ERR(i2c_dev->tx_dma_chan);
+ goto err_release_rx;
}

- if (!i2c_dev->dma_buf && i2c_dev->msg_buf_remaining) {
- dma_buf = dma_alloc_coherent(i2c_dev->dev,
- i2c_dev->dma_buf_size,
- &dma_phys, GFP_KERNEL);
+ i2c_dev->dma_buf = dma_alloc_coherent(dev, i2c_dev->dma_buf_size,
+ &i2c_dev->dma_phys,
+ GFP_KERNEL | __GFP_NOWARN);
+ if (!i2c_dev->dma_buf) {
+ dev_err(dev, "failed to allocate the DMA buffer\n");
+ err = -ENOMEM;
+ goto err_release_tx;
+ }

- if (!dma_buf) {
- dev_err(i2c_dev->dev,
- "failed to allocate the DMA buffer\n");
- dma_release_channel(i2c_dev->tx_dma_chan);
- dma_release_channel(i2c_dev->rx_dma_chan);
- i2c_dev->tx_dma_chan = NULL;
- i2c_dev->rx_dma_chan = NULL;
- return -ENOMEM;
- }
+ return 0;

- i2c_dev->dma_buf = dma_buf;
- i2c_dev->dma_phys = dma_phys;
+err_release_tx:
+ dma_release_channel(i2c_dev->tx_dma_chan);
+err_release_rx:
+ dma_release_channel(i2c_dev->rx_dma_chan);
+err_out:
+ if (err == -ENODEV) {
+ i2c_dev->has_dma = false;
+ i2c_dev->tx_dma_chan = NULL;
+ i2c_dev->rx_dma_chan = NULL;
+ i2c_dev->dma_buf = NULL;
+ return 0;
}

- return 0;
+ return err;
}

static int tegra_i2c_flush_fifos(struct tegra_i2c_dev *i2c_dev)
@@ -991,8 +992,8 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
size_t xfer_size;
u32 *buffer = 0;
int err = 0;
- bool dma = false;
- struct dma_chan *chan;
+ bool dma;
+ struct dma_chan *chan = NULL;
u16 xfer_time = 100;

tegra_i2c_flush_fifos(i2c_dev);
@@ -1016,14 +1017,7 @@ static int tegra_i2c_xfer_msg(struct tegra_i2c_dev *i2c_dev,
xfer_time += DIV_ROUND_CLOSEST(((xfer_size * 9) + 2) * 1000,
i2c_dev->bus_clk_rate);

- dma = (xfer_size > I2C_PIO_MODE_MAX_LEN);
- if (dma) {
- err = tegra_i2c_init_dma_param(i2c_dev);
- if (err < 0) {
- dev_dbg(i2c_dev->dev, "switching to PIO transfer\n");
- dma = false;
- }
- }
+ dma = (xfer_size > I2C_PIO_MODE_MAX_LEN) && i2c_dev->has_dma;

i2c_dev->is_curr_dma_xfer = dma;
spin_lock_irqsave(&i2c_dev->xfer_lock, flags);
@@ -1245,7 +1239,10 @@ static void tegra_i2c_parse_dt(struct tegra_i2c_dev *i2c_dev)
i2c_dev->is_multimaster_mode = of_property_read_bool(np,
"multi-master");

- i2c_dev->has_dma = of_property_read_bool(np, "dmas");
+ if (IS_ENABLED(CONFIG_I2C_TEGRA_DMA_SUPPORT) &&
+ IS_ENABLED(CONFIG_TEGRA20_APB_DMA) &&
+ !i2c_dev->hw->has_gpc_dma)
+ i2c_dev->has_dma = true;
}

static const struct i2c_algorithm tegra_i2c_algo = {
@@ -1280,6 +1277,7 @@ static const struct tegra_i2c_hw_feature tegra20_i2c_hw = {
.setup_hold_time_std_mode = 0x0,
.setup_hold_time_fast_fast_plus_mode = 0x0,
.setup_hold_time_hs_mode = 0x0,
+ .has_gpc_dma = false,
};

static const struct tegra_i2c_hw_feature tegra30_i2c_hw = {
@@ -1302,6 +1300,7 @@ static const struct tegra_i2c_hw_feature tegra30_i2c_hw = {
.setup_hold_time_std_mode = 0x0,
.setup_hold_time_fast_fast_plus_mode = 0x0,
.setup_hold_time_hs_mode = 0x0,
+ .has_gpc_dma = false,
};

static const struct tegra_i2c_hw_feature tegra114_i2c_hw = {
@@ -1324,6 +1323,7 @@ static const struct tegra_i2c_hw_feature tegra114_i2c_hw = {
.setup_hold_time_std_mode = 0x0,
.setup_hold_time_fast_fast_plus_mode = 0x0,
.setup_hold_time_hs_mode = 0x0,
+ .has_gpc_dma = false,
};

static const struct tegra_i2c_hw_feature tegra124_i2c_hw = {
@@ -1346,6 +1346,7 @@ static const struct tegra_i2c_hw_feature tegra124_i2c_hw = {
.setup_hold_time_std_mode = 0x0,
.setup_hold_time_fast_fast_plus_mode = 0x0,
.setup_hold_time_hs_mode = 0x0,
+ .has_gpc_dma = false,
};

static const struct tegra_i2c_hw_feature tegra210_i2c_hw = {
@@ -1368,6 +1369,7 @@ static const struct tegra_i2c_hw_feature tegra210_i2c_hw = {
.setup_hold_time_std_mode = 0,
.setup_hold_time_fast_fast_plus_mode = 0,
.setup_hold_time_hs_mode = 0,
+ .has_gpc_dma = false,
};

static const struct tegra_i2c_hw_feature tegra186_i2c_hw = {
@@ -1390,6 +1392,7 @@ static const struct tegra_i2c_hw_feature tegra186_i2c_hw = {
.setup_hold_time_std_mode = 0,
.setup_hold_time_fast_fast_plus_mode = 0,
.setup_hold_time_hs_mode = 0,
+ .has_gpc_dma = true,
};

static const struct tegra_i2c_hw_feature tegra194_i2c_hw = {
@@ -1412,6 +1415,7 @@ static const struct tegra_i2c_hw_feature tegra194_i2c_hw = {
.setup_hold_time_std_mode = 0x08080808,
.setup_hold_time_fast_fast_plus_mode = 0x02020202,
.setup_hold_time_hs_mode = 0x090909,
+ .has_gpc_dma = true,
};

/* Match table for of_platform binding */
@@ -1479,8 +1483,6 @@ static int tegra_i2c_probe(struct platform_device *pdev)
return PTR_ERR(i2c_dev->rst);
}

- tegra_i2c_parse_dt(i2c_dev);
-
i2c_dev->hw = of_device_get_match_data(&pdev->dev);
i2c_dev->is_dvc = of_device_is_compatible(pdev->dev.of_node,
"nvidia,tegra20-i2c-dvc");
@@ -1488,6 +1490,8 @@ static int tegra_i2c_probe(struct platform_device *pdev)
init_completion(&i2c_dev->dma_complete);
spin_lock_init(&i2c_dev->xfer_lock);

+ tegra_i2c_parse_dt(i2c_dev);
+
if (!i2c_dev->hw->has_single_clk_source) {
fast_clk = devm_clk_get(&pdev->dev, "fast-clk");
if (IS_ERR(fast_clk)) {
@@ -1543,7 +1547,7 @@ static int tegra_i2c_probe(struct platform_device *pdev)
}
}

- ret = tegra_i2c_init_dma_param(i2c_dev);
+ ret = tegra_i2c_init_dma(i2c_dev);
if (ret == -EPROBE_DEFER)
goto disable_div_clk;

@@ -1611,18 +1615,11 @@ static int tegra_i2c_remove(struct platform_device *pdev)
if (!i2c_dev->hw->has_single_clk_source)
clk_unprepare(i2c_dev->fast_clk);

- if (i2c_dev->dma_buf)
+ if (i2c_dev->has_dma) {
dma_free_coherent(i2c_dev->dev, i2c_dev->dma_buf_size,
i2c_dev->dma_buf, i2c_dev->dma_phys);
-
- if (i2c_dev->tx_dma_chan) {
dma_release_channel(i2c_dev->tx_dma_chan);
- i2c_dev->tx_dma_chan = NULL;
- }
-
- if (i2c_dev->rx_dma_chan) {
dma_release_channel(i2c_dev->rx_dma_chan);
- i2c_dev->rx_dma_chan = NULL;
}

return 0;