| Message ID | 1461935197-28664-1-git-send-email-peter.ujfalusi@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 29 April 2016 at 15:06, Peter Ujfalusi <peter.ujfalusi@ti.com> wrote: > With the new dma_request_chan() the client driver does not need to look for > the DMA resource and it does not need to pass filter_fn anymore. > By switching to the new API the driver can now support deferred probing > against DMA. > > Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> > CC: Ulf Hansson <ulf.hansson@linaro.org> > CC: Jarkko Nikula <jarkko.nikula@bitmer.com> > --- > drivers/mmc/host/omap.c | 45 ++++++++++++++++++++++----------------------- > 1 file changed, 22 insertions(+), 23 deletions(-) > > diff --git a/drivers/mmc/host/omap.c b/drivers/mmc/host/omap.c > index b9958a123594..a8d9228657b2 100644 > --- a/drivers/mmc/host/omap.c > +++ b/drivers/mmc/host/omap.c > @@ -23,7 +23,6 @@ > #include <linux/spinlock.h> > #include <linux/timer.h> > #include <linux/of.h> > -#include <linux/omap-dma.h> > #include <linux/mmc/host.h> > #include <linux/mmc/card.h> > #include <linux/mmc/mmc.h> > @@ -1321,8 +1320,6 @@ static int mmc_omap_probe(struct platform_device *pdev) > struct omap_mmc_platform_data *pdata = pdev->dev.platform_data; > struct mmc_omap_host *host = NULL; > struct resource *res; > - dma_cap_mask_t mask; > - unsigned sig = 0; > int i, ret = 0; > int irq; > > @@ -1382,29 +1379,31 @@ static int mmc_omap_probe(struct platform_device *pdev) > goto err_free_iclk; > } > > - dma_cap_zero(mask); > - dma_cap_set(DMA_SLAVE, mask); > - > host->dma_tx_burst = -1; > host->dma_rx_burst = -1; > > - res = platform_get_resource_byname(pdev, IORESOURCE_DMA, "tx"); > - if (res) > - sig = res->start; > - host->dma_tx = dma_request_slave_channel_compat(mask, > - omap_dma_filter_fn, &sig, &pdev->dev, "tx"); > - if (!host->dma_tx) > - dev_warn(host->dev, "unable to obtain TX DMA engine channel %u\n", > - sig); > - > - res = platform_get_resource_byname(pdev, IORESOURCE_DMA, "rx"); > - if (res) > - sig = res->start; > - host->dma_rx = dma_request_slave_channel_compat(mask, > - omap_dma_filter_fn, &sig, &pdev->dev, "rx"); > - if (!host->dma_rx) > - dev_warn(host->dev, "unable to obtain RX DMA engine channel %u\n", > - sig); > + host->dma_tx = dma_request_chan(&pdev->dev, "tx"); > + if (IS_ERR(host->dma_tx)) { > + ret = PTR_ERR(host->dma_tx); > + if (ret == -EPROBE_DEFER) > + goto err_free_iclk; > + > + host->dma_tx = NULL; Instead of setting this to NULL, let's keep its value and later check it with IS_ERR() when needed. > + dev_warn(host->dev, "TX DMA channel request failed\n"); > + } > + > + host->dma_rx = dma_request_chan(&pdev->dev, "rx"); > + if (IS_ERR(host->dma_rx)) { > + ret = PTR_ERR(host->dma_rx); > + if (ret == -EPROBE_DEFER) { > + dma_release_channel(host->dma_tx); host->dma_tx can be NULL here, so this isn't safe. > + clk_put(host->fclk); > + goto err_free_iclk; > + } > + > + host->dma_rx = NULL; Instead of setting this to NULL, let's keep its value and later check it with IS_ERR() when needed. > + dev_warn(host->dev, "RX DMA channel request failed\n"); > + } > > ret = request_irq(host->irq, mmc_omap_irq, 0, DRIVER_NAME, host); > if (ret) > -- > 2.8.1 > Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/03/16 11:46, Ulf Hansson wrote: > On 29 April 2016 at 15:06, Peter Ujfalusi <peter.ujfalusi@ti.com> wrote: >> With the new dma_request_chan() the client driver does not need to look for >> the DMA resource and it does not need to pass filter_fn anymore. >> By switching to the new API the driver can now support deferred probing >> against DMA. >> >> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> >> CC: Ulf Hansson <ulf.hansson@linaro.org> >> CC: Jarkko Nikula <jarkko.nikula@bitmer.com> >> --- >> drivers/mmc/host/omap.c | 45 ++++++++++++++++++++++----------------------- >> 1 file changed, 22 insertions(+), 23 deletions(-) >> >> diff --git a/drivers/mmc/host/omap.c b/drivers/mmc/host/omap.c >> index b9958a123594..a8d9228657b2 100644 >> --- a/drivers/mmc/host/omap.c >> +++ b/drivers/mmc/host/omap.c >> @@ -23,7 +23,6 @@ >> #include <linux/spinlock.h> >> #include <linux/timer.h> >> #include <linux/of.h> >> -#include <linux/omap-dma.h> >> #include <linux/mmc/host.h> >> #include <linux/mmc/card.h> >> #include <linux/mmc/mmc.h> >> @@ -1321,8 +1320,6 @@ static int mmc_omap_probe(struct platform_device *pdev) >> struct omap_mmc_platform_data *pdata = pdev->dev.platform_data; >> struct mmc_omap_host *host = NULL; >> struct resource *res; >> - dma_cap_mask_t mask; >> - unsigned sig = 0; >> int i, ret = 0; >> int irq; >> >> @@ -1382,29 +1379,31 @@ static int mmc_omap_probe(struct platform_device *pdev) >> goto err_free_iclk; >> } >> >> - dma_cap_zero(mask); >> - dma_cap_set(DMA_SLAVE, mask); >> - >> host->dma_tx_burst = -1; >> host->dma_rx_burst = -1; >> >> - res = platform_get_resource_byname(pdev, IORESOURCE_DMA, "tx"); >> - if (res) >> - sig = res->start; >> - host->dma_tx = dma_request_slave_channel_compat(mask, >> - omap_dma_filter_fn, &sig, &pdev->dev, "tx"); >> - if (!host->dma_tx) >> - dev_warn(host->dev, "unable to obtain TX DMA engine channel %u\n", >> - sig); >> - >> - res = platform_get_resource_byname(pdev, IORESOURCE_DMA, "rx"); >> - if (res) >> - sig = res->start; >> - host->dma_rx = dma_request_slave_channel_compat(mask, >> - omap_dma_filter_fn, &sig, &pdev->dev, "rx"); >> - if (!host->dma_rx) >> - dev_warn(host->dev, "unable to obtain RX DMA engine channel %u\n", >> - sig); >> + host->dma_tx = dma_request_chan(&pdev->dev, "tx"); >> + if (IS_ERR(host->dma_tx)) { >> + ret = PTR_ERR(host->dma_tx); >> + if (ret == -EPROBE_DEFER) >> + goto err_free_iclk; and one clk_put(host->fclk) is missing from here. >> + >> + host->dma_tx = NULL; > > Instead of setting this to NULL, let's keep its value and later check > it with IS_ERR() when needed. I thought about it, but decided against without need to have changes in other places in the driver. > >> + dev_warn(host->dev, "TX DMA channel request failed\n"); >> + } >> + >> + host->dma_rx = dma_request_chan(&pdev->dev, "rx"); >> + if (IS_ERR(host->dma_rx)) { >> + ret = PTR_ERR(host->dma_rx); >> + if (ret == -EPROBE_DEFER) { >> + dma_release_channel(host->dma_tx); > > host->dma_tx can be NULL here, so this isn't safe. Oh, true. > >> + clk_put(host->fclk); >> + goto err_free_iclk; >> + } >> + >> + host->dma_rx = NULL; > > Instead of setting this to NULL, let's keep its value and later check > it with IS_ERR() when needed. > >> + dev_warn(host->dev, "RX DMA channel request failed\n"); >> + } >> >> ret = request_irq(host->irq, mmc_omap_irq, 0, DRIVER_NAME, host); >> if (ret) >> -- >> 2.8.1 >> > > Kind regards > Uffe >
On 05/03/2016 11:00 PM, Peter Ujfalusi wrote: >>> @@ -1382,29 +1379,31 @@ static int mmc_omap_probe(struct platform_device *pdev) >>> goto err_free_iclk; >>> } >>> >>> - dma_cap_zero(mask); >>> - dma_cap_set(DMA_SLAVE, mask); >>> - >>> host->dma_tx_burst = -1; >>> host->dma_rx_burst = -1; >>> >>> - res = platform_get_resource_byname(pdev, IORESOURCE_DMA, "tx"); >>> - if (res) >>> - sig = res->start; >>> - host->dma_tx = dma_request_slave_channel_compat(mask, >>> - omap_dma_filter_fn, &sig, &pdev->dev, "tx"); >>> - if (!host->dma_tx) >>> - dev_warn(host->dev, "unable to obtain TX DMA engine channel %u\n", >>> - sig); >>> - >>> - res = platform_get_resource_byname(pdev, IORESOURCE_DMA, "rx"); >>> - if (res) >>> - sig = res->start; >>> - host->dma_rx = dma_request_slave_channel_compat(mask, >>> - omap_dma_filter_fn, &sig, &pdev->dev, "rx"); >>> - if (!host->dma_rx) >>> - dev_warn(host->dev, "unable to obtain RX DMA engine channel %u\n", >>> - sig); >>> + host->dma_tx = dma_request_chan(&pdev->dev, "tx"); >>> + if (IS_ERR(host->dma_tx)) { >>> + ret = PTR_ERR(host->dma_tx); >>> + if (ret == -EPROBE_DEFER) >>> + goto err_free_iclk; > > and one clk_put(host->fclk) is missing from here. > >>> + >>> + host->dma_tx = NULL; >> >> Instead of setting this to NULL, let's keep its value and later check >> it with IS_ERR() when needed. > > I thought about it, but decided against without need to have changes in other > places in the driver. This sentence does not make too much sense ;) Let's try it again: So, I have considered to use PTR_ERR in the driver for the DMA channel, but I wanted to avoid too broad changes in the driver. Keeping the ERR_PTR would bring no runtime benefit since we only care if we have channel or not.
On 4 May 2016 at 05:59, Peter Ujfalusi <peter.ujfalusi@ti.com> wrote: > On 05/03/2016 11:00 PM, Peter Ujfalusi wrote: >>>> @@ -1382,29 +1379,31 @@ static int mmc_omap_probe(struct platform_device *pdev) >>>> goto err_free_iclk; >>>> } >>>> >>>> - dma_cap_zero(mask); >>>> - dma_cap_set(DMA_SLAVE, mask); >>>> - >>>> host->dma_tx_burst = -1; >>>> host->dma_rx_burst = -1; >>>> >>>> - res = platform_get_resource_byname(pdev, IORESOURCE_DMA, "tx"); >>>> - if (res) >>>> - sig = res->start; >>>> - host->dma_tx = dma_request_slave_channel_compat(mask, >>>> - omap_dma_filter_fn, &sig, &pdev->dev, "tx"); >>>> - if (!host->dma_tx) >>>> - dev_warn(host->dev, "unable to obtain TX DMA engine channel %u\n", >>>> - sig); >>>> - >>>> - res = platform_get_resource_byname(pdev, IORESOURCE_DMA, "rx"); >>>> - if (res) >>>> - sig = res->start; >>>> - host->dma_rx = dma_request_slave_channel_compat(mask, >>>> - omap_dma_filter_fn, &sig, &pdev->dev, "rx"); >>>> - if (!host->dma_rx) >>>> - dev_warn(host->dev, "unable to obtain RX DMA engine channel %u\n", >>>> - sig); >>>> + host->dma_tx = dma_request_chan(&pdev->dev, "tx"); >>>> + if (IS_ERR(host->dma_tx)) { >>>> + ret = PTR_ERR(host->dma_tx); >>>> + if (ret == -EPROBE_DEFER) >>>> + goto err_free_iclk; >> >> and one clk_put(host->fclk) is missing from here. >> >>>> + >>>> + host->dma_tx = NULL; >>> >>> Instead of setting this to NULL, let's keep its value and later check >>> it with IS_ERR() when needed. >> >> I thought about it, but decided against without need to have changes in other >> places in the driver. > > This sentence does not make too much sense ;) Let's try it again: > So, I have considered to use PTR_ERR in the driver for the DMA channel, but I > wanted to avoid too broad changes in the driver. Keeping the ERR_PTR would > bring no runtime benefit since we only care if we have channel or not. Okay, so let's deal with that separately instead then. Kind regards Uffe -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/mmc/host/omap.c b/drivers/mmc/host/omap.c index b9958a123594..a8d9228657b2 100644 --- a/drivers/mmc/host/omap.c +++ b/drivers/mmc/host/omap.c @@ -23,7 +23,6 @@ #include <linux/spinlock.h> #include <linux/timer.h> #include <linux/of.h> -#include <linux/omap-dma.h> #include <linux/mmc/host.h> #include <linux/mmc/card.h> #include <linux/mmc/mmc.h> @@ -1321,8 +1320,6 @@ static int mmc_omap_probe(struct platform_device *pdev) struct omap_mmc_platform_data *pdata = pdev->dev.platform_data; struct mmc_omap_host *host = NULL; struct resource *res; - dma_cap_mask_t mask; - unsigned sig = 0; int i, ret = 0; int irq; @@ -1382,29 +1379,31 @@ static int mmc_omap_probe(struct platform_device *pdev) goto err_free_iclk; } - dma_cap_zero(mask); - dma_cap_set(DMA_SLAVE, mask); - host->dma_tx_burst = -1; host->dma_rx_burst = -1; - res = platform_get_resource_byname(pdev, IORESOURCE_DMA, "tx"); - if (res) - sig = res->start; - host->dma_tx = dma_request_slave_channel_compat(mask, - omap_dma_filter_fn, &sig, &pdev->dev, "tx"); - if (!host->dma_tx) - dev_warn(host->dev, "unable to obtain TX DMA engine channel %u\n", - sig); - - res = platform_get_resource_byname(pdev, IORESOURCE_DMA, "rx"); - if (res) - sig = res->start; - host->dma_rx = dma_request_slave_channel_compat(mask, - omap_dma_filter_fn, &sig, &pdev->dev, "rx"); - if (!host->dma_rx) - dev_warn(host->dev, "unable to obtain RX DMA engine channel %u\n", - sig); + host->dma_tx = dma_request_chan(&pdev->dev, "tx"); + if (IS_ERR(host->dma_tx)) { + ret = PTR_ERR(host->dma_tx); + if (ret == -EPROBE_DEFER) + goto err_free_iclk; + + host->dma_tx = NULL; + dev_warn(host->dev, "TX DMA channel request failed\n"); + } + + host->dma_rx = dma_request_chan(&pdev->dev, "rx"); + if (IS_ERR(host->dma_rx)) { + ret = PTR_ERR(host->dma_rx); + if (ret == -EPROBE_DEFER) { + dma_release_channel(host->dma_tx); + clk_put(host->fclk); + goto err_free_iclk; + } + + host->dma_rx = NULL; + dev_warn(host->dev, "RX DMA channel request failed\n"); + } ret = request_irq(host->irq, mmc_omap_irq, 0, DRIVER_NAME, host); if (ret)
With the new dma_request_chan() the client driver does not need to look for the DMA resource and it does not need to pass filter_fn anymore. By switching to the new API the driver can now support deferred probing against DMA. Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com> CC: Ulf Hansson <ulf.hansson@linaro.org> CC: Jarkko Nikula <jarkko.nikula@bitmer.com> --- drivers/mmc/host/omap.c | 45 ++++++++++++++++++++++----------------------- 1 file changed, 22 insertions(+), 23 deletions(-)