| Message ID | 20181018112615.29362-3-o.rempel@pengutronix.de |
|---|---|
| State | Rejected |
| Headers | show |
| Series | i2c: imx: DMA related fixes | expand |
Hello Oleksij, On Thu, Oct 18, 2018 at 01:26:14PM +0200, Oleksij Rempel wrote: > The DMA support for I2C was introduced on i.MX50. So, > avoid of DMA probing on not supported versions. What is the obvious downside of trying to use DMA on i.MX21? If the purpose is just to suppress the message can't request DMA tx channel , that goes away with patch 3, too. Note that if we agree that i.MX50 (and later) isn't compatible to i.MX21, all device trees should be fixed accordingly. IMHO the difference "There is a DMA engine connected only on some implementations" doesn't give enough incentive to claim that i.MX21 and i.MX50 must not share the compatible. Best regards Uwe
On Fri, Oct 19, 2018 at 09:12:43AM +0200, Uwe Kleine-König wrote: > Hello Oleksij, > > On Thu, Oct 18, 2018 at 01:26:14PM +0200, Oleksij Rempel wrote: > > The DMA support for I2C was introduced on i.MX50. So, > > avoid of DMA probing on not supported versions. > > What is the obvious downside of trying to use DMA on i.MX21? If the > purpose is just to suppress the message > > can't request DMA tx channel > > , that goes away with patch 3, too. > > Note that if we agree that i.MX50 (and later) isn't compatible to > i.MX21, all device trees should be fixed accordingly. IMHO the > difference "There is a DMA engine connected only on some > implementations" doesn't give enough incentive to claim that i.MX21 and > i.MX50 must not share the compatible. I don't see any sense to do allocation and add extra probes just to find what we already know from compatible.
On Fri, Oct 19, 2018 at 11:01:00AM +0200, Oleksij Rempel wrote: > On Fri, Oct 19, 2018 at 09:12:43AM +0200, Uwe Kleine-König wrote: > > Hello Oleksij, > > > > On Thu, Oct 18, 2018 at 01:26:14PM +0200, Oleksij Rempel wrote: > > > The DMA support for I2C was introduced on i.MX50. So, > > > avoid of DMA probing on not supported versions. > > > > What is the obvious downside of trying to use DMA on i.MX21? If the > > purpose is just to suppress the message > > > > can't request DMA tx channel > > > > , that goes away with patch 3, too. > > > > Note that if we agree that i.MX50 (and later) isn't compatible to > > i.MX21, all device trees should be fixed accordingly. IMHO the > > difference "There is a DMA engine connected only on some > > implementations" doesn't give enough incentive to claim that i.MX21 and > > i.MX50 must not share the compatible. > > I don't see any sense to do allocation and add extra probes just to find > what we already know from compatible. I think it's perfectly fine to only notice that there is no DMA support when trying to set it up. It has slight runtime overhead, yes, but being able to handle imx21 and imx50 in the same way is a nice advantage that I don't want to throw away easily. If you care about the allocation, you can do do the calls to dma_request_chan first and assign to local variables before the allocation of *dma. Best regards Uwe
On Fri, Oct 19, 2018 at 11:21:45AM +0200, Uwe Kleine-König wrote: > On Fri, Oct 19, 2018 at 11:01:00AM +0200, Oleksij Rempel wrote: > > On Fri, Oct 19, 2018 at 09:12:43AM +0200, Uwe Kleine-König wrote: > > > Hello Oleksij, > > > > > > On Thu, Oct 18, 2018 at 01:26:14PM +0200, Oleksij Rempel wrote: > > > > The DMA support for I2C was introduced on i.MX50. So, > > > > avoid of DMA probing on not supported versions. > > > > > > What is the obvious downside of trying to use DMA on i.MX21? If the > > > purpose is just to suppress the message > > > > > > can't request DMA tx channel > > > > > > , that goes away with patch 3, too. > > > > > > Note that if we agree that i.MX50 (and later) isn't compatible to > > > i.MX21, all device trees should be fixed accordingly. IMHO the > > > difference "There is a DMA engine connected only on some > > > implementations" doesn't give enough incentive to claim that i.MX21 and > > > i.MX50 must not share the compatible. > > > > I don't see any sense to do allocation and add extra probes just to find > > what we already know from compatible. > > I think it's perfectly fine to only notice that there is no DMA support > when trying to set it up. It has slight runtime overhead, yes, but being > able to handle imx21 and imx50 in the same way is a nice advantage that > I don't want to throw away easily. > > If you care about the allocation, you can do do the calls to > dma_request_chan first and assign to local variables before the > allocation of *dma. Ok, then we can drop this patch.
diff --git a/drivers/i2c/busses/i2c-imx.c b/drivers/i2c/busses/i2c-imx.c index c406700789e1..df013b1b6fa4 100644 --- a/drivers/i2c/busses/i2c-imx.c +++ b/drivers/i2c/busses/i2c-imx.c @@ -169,6 +169,7 @@ struct imx_i2c_hwdata { unsigned ndivs; unsigned i2sr_clr_opcode; unsigned i2cr_ien_opcode; + bool has_dmamode; }; struct imx_i2c_dma { @@ -211,7 +212,7 @@ static const struct imx_i2c_hwdata imx1_i2c_hwdata = { .ndivs = ARRAY_SIZE(imx_i2c_clk_div), .i2sr_clr_opcode = I2SR_CLR_OPCODE_W0C, .i2cr_ien_opcode = I2CR_IEN_OPCODE_1, - + .has_dmamode = false, }; static const struct imx_i2c_hwdata imx21_i2c_hwdata = { @@ -221,7 +222,17 @@ static const struct imx_i2c_hwdata imx21_i2c_hwdata = { .ndivs = ARRAY_SIZE(imx_i2c_clk_div), .i2sr_clr_opcode = I2SR_CLR_OPCODE_W0C, .i2cr_ien_opcode = I2CR_IEN_OPCODE_1, + .has_dmamode = false, +}; +static const struct imx_i2c_hwdata imx50_i2c_hwdata = { + .devtype = IMX21_I2C, + .regshift = IMX_I2C_REGSHIFT, + .clk_div = imx_i2c_clk_div, + .ndivs = ARRAY_SIZE(imx_i2c_clk_div), + .i2sr_clr_opcode = I2SR_CLR_OPCODE_W0C, + .i2cr_ien_opcode = I2CR_IEN_OPCODE_1, + .has_dmamode = true, }; static struct imx_i2c_hwdata vf610_i2c_hwdata = { @@ -231,7 +242,7 @@ static struct imx_i2c_hwdata vf610_i2c_hwdata = { .ndivs = ARRAY_SIZE(vf610_i2c_clk_div), .i2sr_clr_opcode = I2SR_CLR_OPCODE_W1C, .i2cr_ien_opcode = I2CR_IEN_OPCODE_0, - + .has_dmamode = true, }; static const struct platform_device_id imx_i2c_devtype[] = { @@ -241,6 +252,33 @@ static const struct platform_device_id imx_i2c_devtype[] = { }, { .name = "imx21-i2c", .driver_data = (kernel_ulong_t)&imx21_i2c_hwdata, + }, { + .name = "imx50-i2c", + .driver_data = (kernel_ulong_t)&imx50_i2c_hwdata, + }, { + .name = "imx51-i2c", + .driver_data = (kernel_ulong_t)&imx50_i2c_hwdata, + }, { + .name = "imx53-i2c", + .driver_data = (kernel_ulong_t)&imx50_i2c_hwdata, + }, { + .name = "imx6q-i2c", + .driver_data = (kernel_ulong_t)&imx50_i2c_hwdata, + }, { + .name = "imx6sl-i2c", + .driver_data = (kernel_ulong_t)&imx50_i2c_hwdata, + }, { + .name = "imx6sll-i2c", + .driver_data = (kernel_ulong_t)&imx50_i2c_hwdata, + }, { + .name = "imx6sx-i2c", + .driver_data = (kernel_ulong_t)&imx50_i2c_hwdata, + }, { + .name = "imx6ul-i2c", + .driver_data = (kernel_ulong_t)&imx50_i2c_hwdata, + }, { + .name = "imx7d-i2c", + .driver_data = (kernel_ulong_t)&imx50_i2c_hwdata, }, { /* sentinel */ } @@ -250,6 +288,14 @@ MODULE_DEVICE_TABLE(platform, imx_i2c_devtype); static const struct of_device_id i2c_imx_dt_ids[] = { { .compatible = "fsl,imx1-i2c", .data = &imx1_i2c_hwdata, }, { .compatible = "fsl,imx21-i2c", .data = &imx21_i2c_hwdata, }, + { .compatible = "fsl,imx50-i2c", .data = &imx50_i2c_hwdata, }, + { .compatible = "fsl,imx51-i2c", .data = &imx50_i2c_hwdata, }, + { .compatible = "fsl,imx53-i2c", .data = &imx50_i2c_hwdata, }, + { .compatible = "fsl,imx6q-i2c", .data = &imx50_i2c_hwdata, }, + { .compatible = "fsl,imx6sl-i2c", .data = &imx50_i2c_hwdata, }, + { .compatible = "fsl,imx6sll-i2c", .data = &imx50_i2c_hwdata, }, + { .compatible = "fsl,imx6sx-i2c", .data = &imx50_i2c_hwdata, }, + { .compatible = "fsl,imx6ul-i2c", .data = &imx50_i2c_hwdata, }, { .compatible = "fsl,vf610-i2c", .data = &vf610_i2c_hwdata, }, { /* sentinel */ } }; @@ -281,6 +327,9 @@ static void i2c_imx_dma_request(struct imx_i2c_struct *i2c_imx, struct device *dev = &i2c_imx->adapter.dev; int ret; + if (!i2c_imx->hwdata->has_dmamode) + return; + dma = devm_kzalloc(dev, sizeof(*dma), GFP_KERNEL); if (!dma) return;
The DMA support for I2C was introduced on i.MX50. So, avoid of DMA probing on not supported versions. Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de> --- drivers/i2c/busses/i2c-imx.c | 53 ++++++++++++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 2 deletions(-)