Re: [PATCH RFC 2/3] mmc: sdhci-omap: Add using external dma
On 9 November 2018 at 06:27, Kishon Vijay Abraham I wrote: > Hi Arnd, > > On 06/11/18 6:21 PM, Arnd Bergmann wrote: >> On 11/5/18, Kishon Vijay Abraham I wrote: >>> On 05/11/18 8:46 AM, Chunyan Zhang wrote: + sdhci_switch_extdma(host, true); >>> >>> A number of devices using sdhci-omap supports ADMA. So switching to >>> external >>> DMA shouldn't be unconditional. >>> >>> IMHO sdhci.c should see if the device supports ADMA or SDMA. If not it >>> should >>> try switching to external DMA and if external DMA too is not supported, it >>> should use PIO. >> >> What's the reasoning for preferring ADMA/SDMA over external DMA if >> both are supported? > > Generally from our experiments we've found ADMA gives better throughput than > DMA. I have to ascertain the actual reasons for that. >> >> I'd expect that if the external DMA for some reason is worse than >> ADMA, we just wouldn't list it in the DT at all, but if it's listed and >> ADMA also works, then the driver should try to use it before falling >> back to ADMA. > > I would agree that for newer dtbs. However if you consider omap_hsmmc, > external > DMA bindings are already added in dt. If we try to switch to sdhci-omap with > the same bindings, systems with older dtbs will use external DMA and give > lesser throughput. I was under the impression that the internal DMA (for old versions) is rather poor. However, in the end this is a software policy decision of what to use. I have no strong opinion, so feel free to pick what you think makes best sense. Maybe Adrian also have some thoughts? Kind regards Uffe
Re: [PATCH RFC 2/3] mmc: sdhci-omap: Add using external dma
On 9 November 2018 at 06:27, Kishon Vijay Abraham I wrote: > Hi Arnd, > > On 06/11/18 6:21 PM, Arnd Bergmann wrote: >> On 11/5/18, Kishon Vijay Abraham I wrote: >>> On 05/11/18 8:46 AM, Chunyan Zhang wrote: + sdhci_switch_extdma(host, true); >>> >>> A number of devices using sdhci-omap supports ADMA. So switching to >>> external >>> DMA shouldn't be unconditional. >>> >>> IMHO sdhci.c should see if the device supports ADMA or SDMA. If not it >>> should >>> try switching to external DMA and if external DMA too is not supported, it >>> should use PIO. >> >> What's the reasoning for preferring ADMA/SDMA over external DMA if >> both are supported? > > Generally from our experiments we've found ADMA gives better throughput than > DMA. I have to ascertain the actual reasons for that. >> >> I'd expect that if the external DMA for some reason is worse than >> ADMA, we just wouldn't list it in the DT at all, but if it's listed and >> ADMA also works, then the driver should try to use it before falling >> back to ADMA. > > I would agree that for newer dtbs. However if you consider omap_hsmmc, > external > DMA bindings are already added in dt. If we try to switch to sdhci-omap with > the same bindings, systems with older dtbs will use external DMA and give > lesser throughput. I was under the impression that the internal DMA (for old versions) is rather poor. However, in the end this is a software policy decision of what to use. I have no strong opinion, so feel free to pick what you think makes best sense. Maybe Adrian also have some thoughts? Kind regards Uffe
Re: [PATCH RFC 2/3] mmc: sdhci-omap: Add using external dma
Hi Arnd, On 06/11/18 6:21 PM, Arnd Bergmann wrote: > On 11/5/18, Kishon Vijay Abraham I wrote: >> On 05/11/18 8:46 AM, Chunyan Zhang wrote: >>> >>> + sdhci_switch_extdma(host, true); >> >> A number of devices using sdhci-omap supports ADMA. So switching to >> external >> DMA shouldn't be unconditional. >> >> IMHO sdhci.c should see if the device supports ADMA or SDMA. If not it >> should >> try switching to external DMA and if external DMA too is not supported, it >> should use PIO. > > What's the reasoning for preferring ADMA/SDMA over external DMA if > both are supported? Generally from our experiments we've found ADMA gives better throughput than DMA. I have to ascertain the actual reasons for that. > > I'd expect that if the external DMA for some reason is worse than > ADMA, we just wouldn't list it in the DT at all, but if it's listed and > ADMA also works, then the driver should try to use it before falling > back to ADMA. I would agree that for newer dtbs. However if you consider omap_hsmmc, external DMA bindings are already added in dt. If we try to switch to sdhci-omap with the same bindings, systems with older dtbs will use external DMA and give lesser throughput. Thanks Kishon
Re: [PATCH RFC 2/3] mmc: sdhci-omap: Add using external dma
Hi Arnd, On 06/11/18 6:21 PM, Arnd Bergmann wrote: > On 11/5/18, Kishon Vijay Abraham I wrote: >> On 05/11/18 8:46 AM, Chunyan Zhang wrote: >>> >>> + sdhci_switch_extdma(host, true); >> >> A number of devices using sdhci-omap supports ADMA. So switching to >> external >> DMA shouldn't be unconditional. >> >> IMHO sdhci.c should see if the device supports ADMA or SDMA. If not it >> should >> try switching to external DMA and if external DMA too is not supported, it >> should use PIO. > > What's the reasoning for preferring ADMA/SDMA over external DMA if > both are supported? Generally from our experiments we've found ADMA gives better throughput than DMA. I have to ascertain the actual reasons for that. > > I'd expect that if the external DMA for some reason is worse than > ADMA, we just wouldn't list it in the DT at all, but if it's listed and > ADMA also works, then the driver should try to use it before falling > back to ADMA. I would agree that for newer dtbs. However if you consider omap_hsmmc, external DMA bindings are already added in dt. If we try to switch to sdhci-omap with the same bindings, systems with older dtbs will use external DMA and give lesser throughput. Thanks Kishon
Re: [PATCH RFC 2/3] mmc: sdhci-omap: Add using external dma
On Tue, 6 Nov 2018 at 20:51, Arnd Bergmann wrote: > > On 11/5/18, Kishon Vijay Abraham I wrote: > > On 05/11/18 8:46 AM, Chunyan Zhang wrote: > >> > >> +sdhci_switch_extdma(host, true); > > > > A number of devices using sdhci-omap supports ADMA. So switching to > > external > > DMA shouldn't be unconditional. > > > > IMHO sdhci.c should see if the device supports ADMA or SDMA. If not it > > should > > try switching to external DMA and if external DMA too is not supported, it > > should use PIO. > > What's the reasoning for preferring ADMA/SDMA over external DMA if > both are supported? > > I'd expect that if the external DMA for some reason is worse than > ADMA, we just wouldn't list it in the DT at all, but if it's listed and > ADMA also works, then the driver should try to use it before falling > back to ADMA. Thanks Arnd and Kishon, let's see what Andrian and Ulf would suggest, and then I will address in next version of patchset. Thanks again, Chunyan > >Arnd
Re: [PATCH RFC 2/3] mmc: sdhci-omap: Add using external dma
On Tue, 6 Nov 2018 at 20:51, Arnd Bergmann wrote: > > On 11/5/18, Kishon Vijay Abraham I wrote: > > On 05/11/18 8:46 AM, Chunyan Zhang wrote: > >> > >> +sdhci_switch_extdma(host, true); > > > > A number of devices using sdhci-omap supports ADMA. So switching to > > external > > DMA shouldn't be unconditional. > > > > IMHO sdhci.c should see if the device supports ADMA or SDMA. If not it > > should > > try switching to external DMA and if external DMA too is not supported, it > > should use PIO. > > What's the reasoning for preferring ADMA/SDMA over external DMA if > both are supported? > > I'd expect that if the external DMA for some reason is worse than > ADMA, we just wouldn't list it in the DT at all, but if it's listed and > ADMA also works, then the driver should try to use it before falling > back to ADMA. Thanks Arnd and Kishon, let's see what Andrian and Ulf would suggest, and then I will address in next version of patchset. Thanks again, Chunyan > >Arnd
Re: [PATCH RFC 2/3] mmc: sdhci-omap: Add using external dma
On 11/5/18, Kishon Vijay Abraham I wrote: > On 05/11/18 8:46 AM, Chunyan Zhang wrote: >> >> +sdhci_switch_extdma(host, true); > > A number of devices using sdhci-omap supports ADMA. So switching to > external > DMA shouldn't be unconditional. > > IMHO sdhci.c should see if the device supports ADMA or SDMA. If not it > should > try switching to external DMA and if external DMA too is not supported, it > should use PIO. What's the reasoning for preferring ADMA/SDMA over external DMA if both are supported? I'd expect that if the external DMA for some reason is worse than ADMA, we just wouldn't list it in the DT at all, but if it's listed and ADMA also works, then the driver should try to use it before falling back to ADMA. Arnd
Re: [PATCH RFC 2/3] mmc: sdhci-omap: Add using external dma
On 11/5/18, Kishon Vijay Abraham I wrote: > On 05/11/18 8:46 AM, Chunyan Zhang wrote: >> >> +sdhci_switch_extdma(host, true); > > A number of devices using sdhci-omap supports ADMA. So switching to > external > DMA shouldn't be unconditional. > > IMHO sdhci.c should see if the device supports ADMA or SDMA. If not it > should > try switching to external DMA and if external DMA too is not supported, it > should use PIO. What's the reasoning for preferring ADMA/SDMA over external DMA if both are supported? I'd expect that if the external DMA for some reason is worse than ADMA, we just wouldn't list it in the DT at all, but if it's listed and ADMA also works, then the driver should try to use it before falling back to ADMA. Arnd
Re: [PATCH RFC 2/3] mmc: sdhci-omap: Add using external dma
Hi,
On 05/11/18 8:46 AM, Chunyan Zhang wrote:
> sdhci-omap can support both external dma controllers via dmaengine
> framework as well as ADMA in which the controller acts as DMA master.
>
> Signed-off-by: Chunyan Zhang
> ---
> drivers/mmc/host/sdhci-omap.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
> index 88347ce..0a8162c 100644
> --- a/drivers/mmc/host/sdhci-omap.c
> +++ b/drivers/mmc/host/sdhci-omap.c
> @@ -896,6 +896,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
> const struct of_device_id *match;
> struct sdhci_omap_data *data;
> const struct soc_device_attribute *soc;
> + struct resource *regs;
>
> match = of_match_device(omap_sdhci_match, dev);
> if (!match)
> @@ -908,6 +909,10 @@ static int sdhci_omap_probe(struct platform_device *pdev)
> }
> offset = data->offset;
>
> + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!regs)
> + return -ENXIO;
> +
> host = sdhci_pltfm_init(pdev, _omap_pdata,
> sizeof(*omap_host));
> if (IS_ERR(host)) {
> @@ -924,6 +929,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
> omap_host->timing = MMC_TIMING_LEGACY;
> omap_host->flags = data->flags;
> host->ioaddr += offset;
> + host->mapbase = regs->start;
>
> mmc = host->mmc;
> sdhci_get_of_property(pdev);
> @@ -991,6 +997,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
> host->mmc_host_ops.execute_tuning = sdhci_omap_execute_tuning;
> host->mmc_host_ops.enable_sdio_irq = sdhci_omap_enable_sdio_irq;
>
> + sdhci_switch_extdma(host, true);
A number of devices using sdhci-omap supports ADMA. So switching to external
DMA shouldn't be unconditional.
IMHO sdhci.c should see if the device supports ADMA or SDMA. If not it should
try switching to external DMA and if external DMA too is not supported, it
should use PIO.
Thanks
Kishon
Re: [PATCH RFC 2/3] mmc: sdhci-omap: Add using external dma
Hi,
On 05/11/18 8:46 AM, Chunyan Zhang wrote:
> sdhci-omap can support both external dma controllers via dmaengine
> framework as well as ADMA in which the controller acts as DMA master.
>
> Signed-off-by: Chunyan Zhang
> ---
> drivers/mmc/host/sdhci-omap.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
> index 88347ce..0a8162c 100644
> --- a/drivers/mmc/host/sdhci-omap.c
> +++ b/drivers/mmc/host/sdhci-omap.c
> @@ -896,6 +896,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
> const struct of_device_id *match;
> struct sdhci_omap_data *data;
> const struct soc_device_attribute *soc;
> + struct resource *regs;
>
> match = of_match_device(omap_sdhci_match, dev);
> if (!match)
> @@ -908,6 +909,10 @@ static int sdhci_omap_probe(struct platform_device *pdev)
> }
> offset = data->offset;
>
> + regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!regs)
> + return -ENXIO;
> +
> host = sdhci_pltfm_init(pdev, _omap_pdata,
> sizeof(*omap_host));
> if (IS_ERR(host)) {
> @@ -924,6 +929,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
> omap_host->timing = MMC_TIMING_LEGACY;
> omap_host->flags = data->flags;
> host->ioaddr += offset;
> + host->mapbase = regs->start;
>
> mmc = host->mmc;
> sdhci_get_of_property(pdev);
> @@ -991,6 +997,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
> host->mmc_host_ops.execute_tuning = sdhci_omap_execute_tuning;
> host->mmc_host_ops.enable_sdio_irq = sdhci_omap_enable_sdio_irq;
>
> + sdhci_switch_extdma(host, true);
A number of devices using sdhci-omap supports ADMA. So switching to external
DMA shouldn't be unconditional.
IMHO sdhci.c should see if the device supports ADMA or SDMA. If not it should
try switching to external DMA and if external DMA too is not supported, it
should use PIO.
Thanks
Kishon
[PATCH RFC 2/3] mmc: sdhci-omap: Add using external dma
sdhci-omap can support both external dma controllers via dmaengine
framework as well as ADMA in which the controller acts as DMA master.
Signed-off-by: Chunyan Zhang
---
drivers/mmc/host/sdhci-omap.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
index 88347ce..0a8162c 100644
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -896,6 +896,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
const struct of_device_id *match;
struct sdhci_omap_data *data;
const struct soc_device_attribute *soc;
+ struct resource *regs;
match = of_match_device(omap_sdhci_match, dev);
if (!match)
@@ -908,6 +909,10 @@ static int sdhci_omap_probe(struct platform_device *pdev)
}
offset = data->offset;
+ regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!regs)
+ return -ENXIO;
+
host = sdhci_pltfm_init(pdev, _omap_pdata,
sizeof(*omap_host));
if (IS_ERR(host)) {
@@ -924,6 +929,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
omap_host->timing = MMC_TIMING_LEGACY;
omap_host->flags = data->flags;
host->ioaddr += offset;
+ host->mapbase = regs->start;
mmc = host->mmc;
sdhci_get_of_property(pdev);
@@ -991,6 +997,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
host->mmc_host_ops.execute_tuning = sdhci_omap_execute_tuning;
host->mmc_host_ops.enable_sdio_irq = sdhci_omap_enable_sdio_irq;
+ sdhci_switch_extdma(host, true);
ret = sdhci_setup_host(host);
if (ret)
goto err_put_sync;
--
2.7.4
[PATCH RFC 2/3] mmc: sdhci-omap: Add using external dma
sdhci-omap can support both external dma controllers via dmaengine
framework as well as ADMA in which the controller acts as DMA master.
Signed-off-by: Chunyan Zhang
---
drivers/mmc/host/sdhci-omap.c | 7 +++
1 file changed, 7 insertions(+)
diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
index 88347ce..0a8162c 100644
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -896,6 +896,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
const struct of_device_id *match;
struct sdhci_omap_data *data;
const struct soc_device_attribute *soc;
+ struct resource *regs;
match = of_match_device(omap_sdhci_match, dev);
if (!match)
@@ -908,6 +909,10 @@ static int sdhci_omap_probe(struct platform_device *pdev)
}
offset = data->offset;
+ regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!regs)
+ return -ENXIO;
+
host = sdhci_pltfm_init(pdev, _omap_pdata,
sizeof(*omap_host));
if (IS_ERR(host)) {
@@ -924,6 +929,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
omap_host->timing = MMC_TIMING_LEGACY;
omap_host->flags = data->flags;
host->ioaddr += offset;
+ host->mapbase = regs->start;
mmc = host->mmc;
sdhci_get_of_property(pdev);
@@ -991,6 +997,7 @@ static int sdhci_omap_probe(struct platform_device *pdev)
host->mmc_host_ops.execute_tuning = sdhci_omap_execute_tuning;
host->mmc_host_ops.enable_sdio_irq = sdhci_omap_enable_sdio_irq;
+ sdhci_switch_extdma(host, true);
ret = sdhci_setup_host(host);
if (ret)
goto err_put_sync;
--
2.7.4
