diff mbox

[v2,4/5] mmc: mxcmmc: enable DMA support on mpc512x

Message ID 1364768585-5177-5-git-send-email-agust@denx.de (mailing list archive)
State New, archived
Headers show

Commit Message

Anatolij Gustschin March 31, 2013, 10:23 p.m. UTC
Add SDHC DMA channel description to the mpc512x device
tree to enable slave channel requesting in the mxcmmc
driver.

mpc512x DMA engine doesn't support endianness conversion
when reading/writing data from peripheral's FIFO, so we
have to swap data buffers before each DMA write and after
each DMA read transfer manually.

Since chained SDHC DMA transfers are not supported on
mpc512x, limit 'max_segs' tunable parameter to one and
initialise it to 64 only when running on i.MX platforms.

Signed-off-by: Anatolij Gustschin <agust@denx.de>
---
v2:
 - only rebased

 arch/powerpc/boot/dts/mpc5121.dtsi |    2 ++
 drivers/mmc/host/mxcmmc.c          |   35 +++++++++++++++++++++++++++++++++--
 2 files changed, 35 insertions(+), 2 deletions(-)

Comments

Sascha Hauer April 2, 2013, 6:56 a.m. UTC | #1
On Mon, Apr 01, 2013 at 12:23:04AM +0200, Anatolij Gustschin wrote:
> Add SDHC DMA channel description to the mpc512x device
> tree to enable slave channel requesting in the mxcmmc
> driver.
> 
> mpc512x DMA engine doesn't support endianness conversion
> when reading/writing data from peripheral's FIFO, so we
> have to swap data buffers before each DMA write and after
> each DMA read transfer manually.
> 
> Since chained SDHC DMA transfers are not supported on
> mpc512x, limit 'max_segs' tunable parameter to one and
> initialise it to 64 only when running on i.MX platforms.
> 
> Signed-off-by: Anatolij Gustschin <agust@denx.de>
> ---
> v2:
>  - only rebased
> 
>  arch/powerpc/boot/dts/mpc5121.dtsi |    2 ++
>  drivers/mmc/host/mxcmmc.c          |   35 +++++++++++++++++++++++++++++++++--
>  2 files changed, 35 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/boot/dts/mpc5121.dtsi b/arch/powerpc/boot/dts/mpc5121.dtsi
> index d1fe070..712b15a 100644
> --- a/arch/powerpc/boot/dts/mpc5121.dtsi
> +++ b/arch/powerpc/boot/dts/mpc5121.dtsi
> @@ -152,6 +152,8 @@
>  			compatible = "fsl,mpc5121-sdhc";
>  			reg = <0x1500 0x100>;
>  			interrupts = <8 0x8>;
> +			dmas = <&dma0 30>;
> +			dma-names = "rx-tx";
>  		};
>  
>  		i2c@1700 {
> diff --git a/drivers/mmc/host/mxcmmc.c b/drivers/mmc/host/mxcmmc.c
> index dbacbde..2861e0d 100644
> --- a/drivers/mmc/host/mxcmmc.c
> +++ b/drivers/mmc/host/mxcmmc.c
> @@ -301,6 +301,29 @@ static void mxcmci_softreset(struct mxcmci_host *host)
>  }
>  static int mxcmci_setup_dma(struct mmc_host *mmc);
>  
> +#if IS_ENABLED(CONFIG_PPC_MPC512x)
> +static inline void buffer_swap32(u32 *buf, int len)
> +{
> +	int i;
> +
> +	for (i = 0; i < ((len + 3) / 4); i++) {
> +		st_le32(buf, *buf);
> +		buf++;
> +	}
> +}
> +
> +static void mxcmci_fixup_buffers(struct mmc_data *data)
> +{
> +	struct scatterlist *sg;
> +	int i;
> +
> +	for_each_sg(data->sg, sg, data->sg_len, i)
> +		buffer_swap32(sg_virt(sg), sg->length);
> +}
> +#else
> +static inline void mxcmci_fixup_buffers(struct mmc_data *data) {}
> +#endif
> +
>  static int mxcmci_setup_data(struct mxcmci_host *host, struct mmc_data *data)
>  {
>  	unsigned int nob = data->blocks;
> @@ -336,6 +359,8 @@ static int mxcmci_setup_data(struct mxcmci_host *host, struct mmc_data *data)
>  	} else {
>  		host->dma_dir = DMA_TO_DEVICE;
>  		slave_dirn = DMA_MEM_TO_DEV;
> +
> +		mxcmci_fixup_buffers(data);

For the register accesses it's fine the way you did in this version, but
here I think a

	if (is_mpc512x_mmc())
		swap_buffer()

would look better.

Sascha
Anatolij Gustschin April 2, 2013, 7:29 a.m. UTC | #2
On Tue, 2 Apr 2013 08:56:54 +0200
Sascha Hauer <s.hauer@pengutronix.de> wrote:
...
> > @@ -336,6 +359,8 @@ static int mxcmci_setup_data(struct mxcmci_host *host, struct mmc_data *data)
> >  	} else {
> >  		host->dma_dir = DMA_TO_DEVICE;
> >  		slave_dirn = DMA_MEM_TO_DEV;
> > +
> > +		mxcmci_fixup_buffers(data);
> 
> For the register accesses it's fine the way you did in this version, but
> here I think a
> 
> 	if (is_mpc512x_mmc())
> 		swap_buffer()
> 
> would look better.

the swapping function uses optimised st_le32() which is not available
on ARM, that's the reason why this is build time selected. Maybe the
"fixup" in the function name is not pretty, I can change it to "swap"
then.

Thanks,

Anatolij
--
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
Sascha Hauer April 2, 2013, 7:48 a.m. UTC | #3
On Tue, Apr 02, 2013 at 09:29:44AM +0200, Anatolij Gustschin wrote:
> On Tue, 2 Apr 2013 08:56:54 +0200
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> ...
> > > @@ -336,6 +359,8 @@ static int mxcmci_setup_data(struct mxcmci_host *host, struct mmc_data *data)
> > >  	} else {
> > >  		host->dma_dir = DMA_TO_DEVICE;
> > >  		slave_dirn = DMA_MEM_TO_DEV;
> > > +
> > > +		mxcmci_fixup_buffers(data);
> > 
> > For the register accesses it's fine the way you did in this version, but
> > here I think a
> > 
> > 	if (is_mpc512x_mmc())
> > 		swap_buffer()
> > 
> > would look better.
> 
> the swapping function uses optimised st_le32() which is not available
> on ARM, that's the reason why this is build time selected.

Ah, ok. I think it's fine then.

> Maybe the
> "fixup" in the function name is not pretty, I can change it to "swap"
> then.

Yes, that would be better.

Sascha
diff mbox

Patch

diff --git a/arch/powerpc/boot/dts/mpc5121.dtsi b/arch/powerpc/boot/dts/mpc5121.dtsi
index d1fe070..712b15a 100644
--- a/arch/powerpc/boot/dts/mpc5121.dtsi
+++ b/arch/powerpc/boot/dts/mpc5121.dtsi
@@ -152,6 +152,8 @@ 
 			compatible = "fsl,mpc5121-sdhc";
 			reg = <0x1500 0x100>;
 			interrupts = <8 0x8>;
+			dmas = <&dma0 30>;
+			dma-names = "rx-tx";
 		};
 
 		i2c@1700 {
diff --git a/drivers/mmc/host/mxcmmc.c b/drivers/mmc/host/mxcmmc.c
index dbacbde..2861e0d 100644
--- a/drivers/mmc/host/mxcmmc.c
+++ b/drivers/mmc/host/mxcmmc.c
@@ -301,6 +301,29 @@  static void mxcmci_softreset(struct mxcmci_host *host)
 }
 static int mxcmci_setup_dma(struct mmc_host *mmc);
 
+#if IS_ENABLED(CONFIG_PPC_MPC512x)
+static inline void buffer_swap32(u32 *buf, int len)
+{
+	int i;
+
+	for (i = 0; i < ((len + 3) / 4); i++) {
+		st_le32(buf, *buf);
+		buf++;
+	}
+}
+
+static void mxcmci_fixup_buffers(struct mmc_data *data)
+{
+	struct scatterlist *sg;
+	int i;
+
+	for_each_sg(data->sg, sg, data->sg_len, i)
+		buffer_swap32(sg_virt(sg), sg->length);
+}
+#else
+static inline void mxcmci_fixup_buffers(struct mmc_data *data) {}
+#endif
+
 static int mxcmci_setup_data(struct mxcmci_host *host, struct mmc_data *data)
 {
 	unsigned int nob = data->blocks;
@@ -336,6 +359,8 @@  static int mxcmci_setup_data(struct mxcmci_host *host, struct mmc_data *data)
 	} else {
 		host->dma_dir = DMA_TO_DEVICE;
 		slave_dirn = DMA_MEM_TO_DEV;
+
+		mxcmci_fixup_buffers(data);
 	}
 
 	nents = dma_map_sg(host->dma->device->dev, data->sg,
@@ -461,9 +486,11 @@  static int mxcmci_finish_data(struct mxcmci_host *host, unsigned int stat)
 	struct mmc_data *data = host->data;
 	int data_error;
 
-	if (mxcmci_use_dma(host))
+	if (mxcmci_use_dma(host)) {
 		dma_unmap_sg(host->dma->device->dev, data->sg, data->sg_len,
 				host->dma_dir);
+		mxcmci_fixup_buffers(data);
+	}
 
 	if (stat & STATUS_ERR_MASK) {
 		dev_dbg(mmc_dev(host->mmc), "request failed. status: 0x%08x\n",
@@ -1033,7 +1060,6 @@  static int mxcmci_probe(struct platform_device *pdev)
 		mmc->caps |= MMC_CAP_SDIO_IRQ;
 
 	/* MMC core transfer sizes tunable parameters */
-	mmc->max_segs = 64;
 	mmc->max_blk_size = 2048;
 	mmc->max_blk_count = 65535;
 	mmc->max_req_size = mmc->max_blk_size * mmc->max_blk_count;
@@ -1052,6 +1078,11 @@  static int mxcmci_probe(struct platform_device *pdev)
 	} else {
 		host->devtype = pdev->id_entry->driver_data;
 	}
+
+	/* adjust max_segs after devtype detection */
+	if (!is_mpc512x_mmc(host))
+		mmc->max_segs = 64;
+
 	host->mmc = mmc;
 	host->pdata = pdata;
 	spin_lock_init(&host->lock);