i2c: at91: introduce probe deferring
Subsystems (2)
PATCH 1/1 i2c: at91: introduce probe deferring
Return probe defer if requesting a dma channel without a dma controller probed.
--- drivers/i2c/busses/i2c-at91.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-)
drivers/i2c/busses/i2c-at91.c +16 -6
@@ -679,14 +679,21 @@ static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr)dma_cap_zero (mask);dma_cap_set (DMA_SLAVE , mask); - dma->chan_tx =dma_request_slave_channel_compat (mask, filter, pdata, - dev->dev,"tx" ); -if (!dma->chan_tx) { + dma->chan_tx =dma_request_slave_channel_reason (dev->dev,"tx" ); +if (IS_ERR (dma->chan_tx)) { + ret =PTR_ERR (dma->chan_tx); +if (ret == -EPROBE_DEFER ) { +dev_warn (dev->dev,"no DMA channel available at the moment\n" ); +return ret; + }dev_err (dev->dev,"can't get a DMA channel for tx\n" ); - ret = -EBUSY ; goto error; } +/* +* No reason to check EPROBE_DEFER here since we have already request +* tx channel. If it fails here, it's for another reason. +*/ dma->chan_rx =dma_request_slave_channel_compat (mask, filter, pdata, dev->dev,"rx" );if (!dma->chan_rx) {
@@ -722,7 +729,7 @@ error:dev_info (dev->dev,"can't use DMA\n" );if (dma->chan_rx)dma_release_channel (dma->chan_rx); -if (dma->chan_tx) +if (!IS_ERR (dma->chan_tx))dma_release_channel (dma->chan_tx);return ret; }
@@ -788,8 +795,11 @@ static int at91_twi_probe(struct platform_device *pdev)clk_prepare_enable (dev->clk);if (dev->pdata->has_dma_support) { -if (at91_twi_configure_dma (dev, phy_addr) ==0 ) + rc =at91_twi_configure_dma (dev, phy_addr); +if (rc ==0 ) dev->use_dma = true; +else if (rc == -EPROBE_DEFER ) +return rc; } rc = of_property_read_u32(dev->dev->of_node, "clock-frequency",
Signature · 1 line
2.0.3
PATCH 1/1 i2c: at91: introduce probe deferring
Quoted 12 lines · Ludovic Desroches
Return probe defer if requesting a dma channel without a dma controller probed.
Signed-off-by Ludovic Desroches <[email protected]>--- drivers/i2c/busses/i2c-at91.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-)drivers/i2c/busses/i2c-at91.c +0
-0
drivers/i2c/busses/i2c-at91.c static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr) · line 682
Quoted 7 lines
drivers/i2c/busses/i2c-at91.c +1
-3@@ -679,14 +679,21 @@ static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr)dma_cap_zero (mask);dma_cap_set (DMA_SLAVE , mask); - dma->chan_tx =dma_request_slave_channel_compat (mask, filter, pdata, - dev->dev,"tx" ); -if (!dma->chan_tx) { + dma->chan_tx =dma_request_slave_channel_reason (dev->dev,"tx" );
Will it cause regressions if you drop the compat-version of requesting a channel?
drivers/i2c/busses/i2c-at91.c static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr) · line 686
Quoted 4 lines
drivers/i2c/busses/i2c-at91.c +4
-0+if (IS_ERR (dma->chan_tx)) { + ret =PTR_ERR (dma->chan_tx); +if (ret == -EPROBE_DEFER ) { +dev_warn (dev->dev,"no DMA channel available at the moment\n" );
I'd say drop this warning. The core usually prints when deferred probing takes place.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141119/068461e4/attachment.sig>
PATCH 1/1 i2c: at91: introduce probe deferring
Quoted 16 lines · On Wednesday 19 November 2014 10:16:47 Wolfram Sang wrote:
Quoted 15 lines · Ludovic Desroches
Return probe defer if requesting a dma channel without a dma controller probed.
Signed-off-by Ludovic Desroches <[email protected]>--- drivers/i2c/busses/i2c-at91.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-)drivers/i2c/busses/i2c-at91.c +0
-0@@ -679,14 +679,21 @@ static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr)dma_cap_zero (mask);dma_cap_set (DMA_SLAVE , mask);
Quoted 7 lines
drivers/i2c/busses/i2c-at91.c static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr) · line 682
Quoted 4 lines
drivers/i2c/busses/i2c-at91.c +1
-3- dma->chan_tx =dma_request_slave_channel_compat (mask, filter, pdata, - dev->dev,"tx" ); -if (!dma->chan_tx) { + dma->chan_tx =dma_request_slave_channel_reason (dev->dev,"tx" );Will it cause regressions if you drop the compat-version of requesting a channel?
I got curious about this, since the patch looks obviously wrong, but actually it's ok. However the entire DMA support for non-DT platforms can be dropped in this driver, see patch below
Quoted 7 lines
drivers/i2c/busses/i2c-at91.c static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr) · line 686
Quoted 4 lines
drivers/i2c/busses/i2c-at91.c +4
-0+if (IS_ERR (dma->chan_tx)) { + ret =PTR_ERR (dma->chan_tx); +if (ret == -EPROBE_DEFER ) { +dev_warn (dev->dev,"no DMA channel available at the moment\n" );I'd say drop this warning. The core usually prints when deferred probing takes place.
Definitely yes.
Arnd
8<---
[PATCH] i2c: at91: remove legacy DMA supoprt
Since at91sam9g45 is now DT-only, all DMA capable users of this driver are using the DT case, and the legacy support can be removed. While at it, fix the deferred probe case.
drivers/i2c/busses/i2c-at91.c +13 -45
@@ -72,8 +72,6 @@ struct at91_twi_pdata {unsigned clk_max_div;unsigned clk_offset;bool has_unre_flag; -bool has_dma_support; -struct at_dma_slave dma_slave; };struct at91_twi_dma {
@@ -541,35 +539,30 @@ static struct at91_twi_pdata at91rm9200_config = { .clk_max_div = 5, .clk_offset = 3, .has_unre_flag = true, - .has_dma_support = false, };static struct at91_twi_pdata at91sam9261_config = { .clk_max_div =5 , .clk_offset =4 , .has_unre_flag = false, - .has_dma_support = false, };static struct at91_twi_pdata at91sam9260_config = { .clk_max_div =7 , .clk_offset =4 , .has_unre_flag = false, - .has_dma_support = false, };static struct at91_twi_pdata at91sam9g20_config = { .clk_max_div =7 , .clk_offset =4 , .has_unre_flag = false, - .has_dma_support = false, };static struct at91_twi_pdata at91sam9g10_config = { .clk_max_div =7 , .clk_offset =4 , .has_unre_flag = false, - .has_dma_support = false, };static const struct platform_device_id at91_twi_devtypes[] = {
@@ -598,7 +591,6 @@ static struct at91_twi_pdata at91sam9x5_config = { .clk_max_div = 7, .clk_offset = 4, .has_unre_flag = false, - .has_dma_support = true, };static const struct of_device_id atmel_twi_dt_ids[] = {
@@ -627,30 +619,11 @@ static const struct of_device_id atmel_twi_dt_ids[] = {MODULE_DEVICE_TABLE (of, atmel_twi_dt_ids);#endif -static bool filter (struct dma_chan *chan,void *pdata) -{ -struct at91_twi_pdata *sl_pdata = pdata; -struct at_dma_slave *sl; - -if (!sl_pdata) -return false; - - sl = &sl_pdata->dma_slave; -if (sl && (sl->dma_dev == chan->device->dev)) { - chan->private = sl; -return true; - }else { -return false; - } -} -static int at91_twi_configure_dma (struct at91_twi_dev *dev,u32 phy_addr) {int ret =0 ; -struct at91_twi_pdata *pdata = dev->pdata;struct dma_slave_config slave_config;struct at91_twi_dma *dma = &dev->dma; -dma_cap_mask_t mask;memset (&slave_config,0 ,sizeof (slave_config)); slave_config.src_addr = (dma_addr_t )phy_addr +AT91_TWI_RHR ;
@@ -661,22 +634,17 @@ static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr) slave_config.dst_maxburst =1 ; slave_config.device_fc = false; -dma_cap_zero (mask); -dma_cap_set (DMA_SLAVE , mask); - - dma->chan_tx =dma_request_slave_channel_compat (mask, filter, pdata, - dev->dev,"tx" ); -if (!dma->chan_tx) { -dev_err (dev->dev,"can't get a DMA channel for tx\n" ); - ret = -EBUSY ; + dma->chan_tx =dma_request_slave_channel_reason (dev->dev,"tx" ); +if (IS_ERR (dma->chan_tx)) { + ret =PTR_ERR (dma->chan_tx); + dma->chan_tx =NULL ; goto error; } - dma->chan_rx =dma_request_slave_channel_compat (mask, filter, pdata, - dev->dev,"rx" ); -if (!dma->chan_rx) { -dev_err (dev->dev,"can't get a DMA channel for rx\n" ); - ret = -EBUSY ; + dma->chan_rx =dma_request_slave_channel_reason (dev->dev,"rx" ); +if (IS_ERR (dma->chan_rx)) { + ret =PTR_ERR (dma->chan_rx); + dma->chan_rx =NULL ; goto error; }
@@ -697,6 +665,7 @@ static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr)sg_init_table (&dma->sg,1 ); dma->buf_mapped = false; dma->xfer_in_progress = false; + dev->use_dma = true;dev_info (dev->dev,"using %s (tx) and %s (rx) for DMA transfers\n" ,dma_chan_name (dma->chan_tx),dma_chan_name (dma->chan_rx));
@@ -704,7 +673,8 @@ static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr)return ret; error: -dev_info (dev->dev,"can't use DMA\n" ); +if (ret != -EPROBE_DEFER ) +dev_info (dev->dev,"can't use DMA, error %d\n" , ret);if (dma->chan_rx)dma_release_channel (dma->chan_rx);if (dma->chan_tx)
@@ -772,10 +742,8 @@ static int at91_twi_probe(struct platform_device *pdev) }clk_prepare_enable (dev->clk); -if (dev->pdata->has_dma_support) { -if (at91_twi_configure_dma (dev, phy_addr) ==0 ) - dev->use_dma = true; - } +if (dev->of_node) +at91_twi_configure_dma (dev, phy_addr); rc =of_property_read_u32 (dev->dev->of_node,"clock-frequency" , &bus_clk_rate);
PATCH 1/1 i2c: at91: introduce probe deferring
Quoted 43 lines · Arnd Bergmann
Quoted 16 lines · On Wednesday 19 November 2014 10:16:47 Wolfram Sang wrote:
Quoted 15 lines · Ludovic Desroches
Return probe defer if requesting a dma channel without a dma controller probed.
Signed-off-by Ludovic Desroches <[email protected]>--- drivers/i2c/busses/i2c-at91.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-)drivers/i2c/busses/i2c-at91.c +0
-0@@ -679,14 +679,21 @@ static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr)dma_cap_zero (mask);dma_cap_set (DMA_SLAVE , mask);Quoted 7 lines
drivers/i2c/busses/i2c-at91.c static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr) · line 637
Quoted 4 lines
drivers/i2c/busses/i2c-at91.c +1
-3- dma->chan_tx =dma_request_slave_channel_compat (mask, filter, pdata, - dev->dev,"tx" ); -if (!dma->chan_tx) { + dma->chan_tx =dma_request_slave_channel_reason (dev->dev,"tx" );Will it cause regressions if you drop the compat-version of requesting a channel?
I got curious about this, since the patch looks obviously wrong, but actually it's ok. However the entire DMA support for non-DT platforms can be dropped in this driver, see patch below
Quoted 7 lines
drivers/i2c/busses/i2c-at91.c static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr)
Quoted 4 lines
drivers/i2c/busses/i2c-at91.c +4
-0+if (IS_ERR (dma->chan_tx)) { + ret =PTR_ERR (dma->chan_tx); +if (ret == -EPROBE_DEFER ) { +dev_warn (dev->dev,"no DMA channel available at the moment\n" );I'd say drop this warning. The core usually prints when deferred probing takes place.
Definitely yes.
Arnd
8<---
[PATCH] i2c: at91: remove legacy DMA supoprt
Quoted 5 lines
Since at91sam9g45 is now DT-only, all DMA capable users of this driver are using the DT case, and the legacy support can be removed. While at it, fix the deferred probe case.
Signed-off-by Arnd Bergmann <[email protected]>
Thanks
Quoted 152 lines
drivers/i2c/busses/i2c-at91.c +13
-45@@ -72,8 +72,6 @@ struct at91_twi_pdata {unsigned clk_max_div;unsigned clk_offset;bool has_unre_flag; -bool has_dma_support; -struct at_dma_slave dma_slave; };struct at91_twi_dma {@@ -541,35 +539,30 @@ static struct at91_twi_pdata at91rm9200_config = { .clk_max_div = 5, .clk_offset = 3, .has_unre_flag = true, - .has_dma_support = false, };static struct at91_twi_pdata at91sam9261_config = { .clk_max_div =5 , .clk_offset =4 , .has_unre_flag = false, - .has_dma_support = false, };static struct at91_twi_pdata at91sam9260_config = { .clk_max_div =7 , .clk_offset =4 , .has_unre_flag = false, - .has_dma_support = false, };static struct at91_twi_pdata at91sam9g20_config = { .clk_max_div =7 , .clk_offset =4 , .has_unre_flag = false, - .has_dma_support = false, };static struct at91_twi_pdata at91sam9g10_config = { .clk_max_div =7 , .clk_offset =4 , .has_unre_flag = false, - .has_dma_support = false, };static const struct platform_device_id at91_twi_devtypes[] = {@@ -598,7 +591,6 @@ static struct at91_twi_pdata at91sam9x5_config = { .clk_max_div = 7, .clk_offset = 4, .has_unre_flag = false, - .has_dma_support = true, };static const struct of_device_id atmel_twi_dt_ids[] = {@@ -627,30 +619,11 @@ static const struct of_device_id atmel_twi_dt_ids[] = {MODULE_DEVICE_TABLE (of, atmel_twi_dt_ids);#endif -static bool filter (struct dma_chan *chan,void *pdata) -{ -struct at91_twi_pdata *sl_pdata = pdata; -struct at_dma_slave *sl; - -if (!sl_pdata) -return false; - - sl = &sl_pdata->dma_slave; -if (sl && (sl->dma_dev == chan->device->dev)) { - chan->private = sl; -return true; - }else { -return false; - } -} -static int at91_twi_configure_dma (struct at91_twi_dev *dev,u32 phy_addr) {int ret =0 ; -struct at91_twi_pdata *pdata = dev->pdata;struct dma_slave_config slave_config;struct at91_twi_dma *dma = &dev->dma; -dma_cap_mask_t mask;memset (&slave_config,0 ,sizeof (slave_config)); slave_config.src_addr = (dma_addr_t )phy_addr +AT91_TWI_RHR ;@@ -661,22 +634,17 @@ static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr) slave_config.dst_maxburst =1 ; slave_config.device_fc = false; -dma_cap_zero (mask); -dma_cap_set (DMA_SLAVE , mask); - - dma->chan_tx =dma_request_slave_channel_compat (mask, filter, pdata, - dev->dev,"tx" ); -if (!dma->chan_tx) { -dev_err (dev->dev,"can't get a DMA channel for tx\n" ); - ret = -EBUSY ; + dma->chan_tx =dma_request_slave_channel_reason (dev->dev,"tx" ); +if (IS_ERR (dma->chan_tx)) { + ret =PTR_ERR (dma->chan_tx); + dma->chan_tx =NULL ; goto error; } - dma->chan_rx =dma_request_slave_channel_compat (mask, filter, pdata, - dev->dev,"rx" ); -if (!dma->chan_rx) { -dev_err (dev->dev,"can't get a DMA channel for rx\n" ); - ret = -EBUSY ; + dma->chan_rx =dma_request_slave_channel_reason (dev->dev,"rx" ); +if (IS_ERR (dma->chan_rx)) { + ret =PTR_ERR (dma->chan_rx); + dma->chan_rx =NULL ; goto error; }@@ -697,6 +665,7 @@ static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr)sg_init_table (&dma->sg,1 ); dma->buf_mapped = false; dma->xfer_in_progress = false; + dev->use_dma = true;dev_info (dev->dev,"using %s (tx) and %s (rx) for DMA transfers\n" ,dma_chan_name (dma->chan_tx),dma_chan_name (dma->chan_rx));@@ -704,7 +673,8 @@ static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr)return ret; error: -dev_info (dev->dev,"can't use DMA\n" ); +if (ret != -EPROBE_DEFER ) +dev_info (dev->dev,"can't use DMA, error %d\n" , ret);if (dma->chan_rx)dma_release_channel (dma->chan_rx);if (dma->chan_tx)@@ -772,10 +742,8 @@ static int at91_twi_probe(struct platform_device *pdev) }clk_prepare_enable (dev->clk); -if (dev->pdata->has_dma_support) { -if (at91_twi_configure_dma (dev, phy_addr) ==0 ) - dev->use_dma = true; - } +if (dev->of_node) +at91_twi_configure_dma (dev, phy_addr); rc =of_property_read_u32 (dev->dev->of_node,"clock-frequency" , &bus_clk_rate);
PATCH 1/1 i2c: at91: introduce probe deferring
Quoted 44 lines · Ludovic Desroches
Quoted 43 lines · Arnd Bergmann
Quoted 16 lines · On Wednesday 19 November 2014 10:16:47 Wolfram Sang wrote:
Quoted 15 lines · Ludovic Desroches
Return probe defer if requesting a dma channel without a dma controller probed.
Signed-off-by Ludovic Desroches <[email protected]>--- drivers/i2c/busses/i2c-at91.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-)drivers/i2c/busses/i2c-at91.c +0
-0@@ -679,14 +679,21 @@ static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr)dma_cap_zero (mask);dma_cap_set (DMA_SLAVE , mask);Quoted 7 lines
drivers/i2c/busses/i2c-at91.c static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr) · line 637
Quoted 4 lines
drivers/i2c/busses/i2c-at91.c +1
-3- dma->chan_tx =dma_request_slave_channel_compat (mask, filter, pdata, - dev->dev,"tx" ); -if (!dma->chan_tx) { + dma->chan_tx =dma_request_slave_channel_reason (dev->dev,"tx" );Will it cause regressions if you drop the compat-version of requesting a channel?
I got curious about this, since the patch looks obviously wrong, but actually it's ok. However the entire DMA support for non-DT platforms can be dropped in this driver, see patch below
Quoted 7 lines
drivers/i2c/busses/i2c-at91.c static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr)
Quoted 4 lines
drivers/i2c/busses/i2c-at91.c +4
-0+if (IS_ERR (dma->chan_tx)) { + ret =PTR_ERR (dma->chan_tx); +if (ret == -EPROBE_DEFER ) { +dev_warn (dev->dev,"no DMA channel available at the moment\n" );I'd say drop this warning. The core usually prints when deferred probing takes place.
Definitely yes.
Arnd
8<---
[PATCH] i2c: at91: remove legacy DMA supoprt
Quoted 5 lines
Quoted 5 lines
Since at91sam9g45 is now DT-only, all DMA capable users of this driver are using the DT case, and the legacy support can be removed. While at it, fix the deferred probe case.
Signed-off-by Arnd Bergmann <[email protected]>
Thanks for this cleanup, yet I'd really appreciate a build test at least.
Quoted 3 lines
From you, I had even hoped for a quick runtime test. Would that be
possible for future patches?
Thanks,
Wolfram
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141121/3afced22/attachment.sig>
PATCH 1/1 i2c: at91: introduce probe deferring
In reply to Wolfram SangQuoted 16 lines · Wolfram Sang
Quoted 15 lines · Ludovic Desroches
Return probe defer if requesting a dma channel without a dma controller probed.
Signed-off-by Ludovic Desroches <[email protected]>--- drivers/i2c/busses/i2c-at91.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-)drivers/i2c/busses/i2c-at91.c +0
-0@@ -679,14 +679,21 @@ static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr)dma_cap_zero (mask);dma_cap_set (DMA_SLAVE , mask);
Quoted 7 lines
drivers/i2c/busses/i2c-at91.c static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr) · line 682
Quoted 4 lines
drivers/i2c/busses/i2c-at91.c +1
-3- dma->chan_tx =dma_request_slave_channel_compat (mask, filter, pdata, - dev->dev,"tx" ); -if (!dma->chan_tx) { + dma->chan_tx =dma_request_slave_channel_reason (dev->dev,"tx" );Will it cause regressions if you drop the compat-version of requesting a channel?
All our devices with a DMA controller have been converted to device tree and the legacy board support will be removed in 3.19 so it won't cause regression.
Quoted 9 lines
drivers/i2c/busses/i2c-at91.c static int at91_twi_configure_dma(struct at91_twi_dev *dev, u32 phy_addr) · line 686
Quoted 5 lines
drivers/i2c/busses/i2c-at91.c +4
-0+if (IS_ERR (dma->chan_tx)) { + ret =PTR_ERR (dma->chan_tx); +if (ret == -EPROBE_DEFER ) { +dev_warn (dev->dev,"no DMA channel available at the moment\n" );I'd say drop this warning. The core usually prints when deferred probing takes place.
Ok, I'll remove it.
Ludovic