Re: [PATCH 1/3] ata: sata_dwc_460ex: use "dmas" DT property to find dma channel

[Andy Shevchenko]

On Tue, 2015-12-15 at 23:25 +0000, Mans Rullgard wrote:
> Currently this driver only works with a DesignWare DMA engine which
> it
> registers manually using the second "reg" address range and interrupt
> number from the DT node.
> 
> This patch makes the driver instead use the "dmas" property if
> present,
> otherwise optionally falling back on the old way so existing device
> trees can continue to work.
> 
> With this change, there is no longer any reason to depend on the
> 460EX
> machine type so drop that from Kconfig.

Looks good for me (from dw_dmac usage prospective).

> 
> Signed-off-by: Mans Rullgard <mans@xxxxxxxxx>
> ---
> Âdrivers/ata/KconfigÂÂÂÂÂÂÂÂÂÂ|ÂÂ10 ++-
> Âdrivers/ata/sata_dwc_460ex.c | 192 +++++++++++++++++++++++++++----
> ------------
> Â2 files changed, 131 insertions(+), 71 deletions(-)
> 
> diff --git a/drivers/ata/Kconfig b/drivers/ata/Kconfig
> index 3fc2a56..193c673 100644
> --- a/drivers/ata/Kconfig
> +++ b/drivers/ata/Kconfig
> @@ -296,14 +296,20 @@ config ATA_PIIX
> Â
> Âconfig SATA_DWC
> Â	tristate "DesignWare Cores SATA support"
> -	depends on 460EX
> -	select DW_DMAC
> Â	help
> Â	ÂÂThis option enables support for the on-chip SATA
> controller of the
> Â	ÂÂAppliedMicro processor 460EX.
> Â
> Â	ÂÂIf unsure, say N.
> Â
> +config SATA_DWC_OLD_DMA
> +	bool "Support old device trees"
> +	depends on SATA_DWC && 460EX
> +	select DW_DMAC
> +	help
> +	ÂÂThis option enables support for old device trees without
> the
> +	ÂÂ"dmas" property.
> +
> Âconfig SATA_DWC_DEBUG
> Â	bool "Debugging driver version"
> Â	depends on SATA_DWC
> diff --git a/drivers/ata/sata_dwc_460ex.c
> b/drivers/ata/sata_dwc_460ex.c
> index 9020349..9985749 100644
> --- a/drivers/ata/sata_dwc_460ex.c
> +++ b/drivers/ata/sata_dwc_460ex.c
> @@ -30,6 +30,7 @@
> Â#include <linux/kernel.h>
> Â#include <linux/module.h>
> Â#include <linux/device.h>
> +#include <linux/dmaengine.h>
> Â#include <linux/of_address.h>
> Â#include <linux/of_irq.h>
> Â#include <linux/of_platform.h>
> @@ -42,10 +43,6 @@
> Â#include <scsi/scsi_host.h>
> Â#include <scsi/scsi_cmnd.h>
> Â
> -/* Supported DMA engine drivers */
> -#include <linux/platform_data/dma-dw.h>
> -#include <linux/dma/dw.h>
> -
> Â/* These two are defined in "libata.h" */
> Â#undef	DRV_NAME
> Â#undef	DRV_VERSION
> @@ -148,7 +145,9 @@ struct sata_dwc_device {
> Â	struct ata_host		*host;
> Â	u8 __iomem		*reg_base;
> Â	struct sata_dwc_regs	*sata_dwc_regs;	/* DW
> Synopsys SATA specific */
> +#ifdef CONFIG_SATA_DWC_OLD_DMA
> Â	struct dw_dma_chip	*dma;
> +#endif
> Â};
> Â
> Â#define SATA_DWC_QCMD_MAX	32
> @@ -159,7 +158,6 @@ struct sata_dwc_device_port {
> Â	int			dma_pending[SATA_DWC_QCMD_MAX];
> Â
> Â	/* DMA info */
> -	struct dw_dma_slave		*dws;
> Â	struct dma_chan			*chan;
> Â	struct dma_async_tx_descriptor	*desc[SATA_DWC_QCMD_MA
> X];
> Â	u32				dma_interrupt_count;
> @@ -198,13 +196,6 @@ struct sata_dwc_host_priv {
> Â
> Âstatic struct sata_dwc_host_priv host_pvt;
> Â
> -static struct dw_dma_slave sata_dwc_dma_dws = {
> -	.src_id = 0,
> -	.dst_id = 0,
> -	.src_master = 0,
> -	.dst_master = 1,
> -};
> -
> Â/*
> Â * Prototypes
> Â */
> @@ -215,6 +206,90 @@ static void sata_dwc_dma_xfer_complete(struct
> ata_port *ap, u32 check_status);
> Âstatic void sata_dwc_port_stop(struct ata_port *ap);
> Âstatic void sata_dwc_clear_dmacr(struct sata_dwc_device_port
> *hsdevp, u8 tag);
> Â
> +#ifdef CONFIG_SATA_DWC_OLD_DMA
> +
> +#include <linux/platform_data/dma-dw.h>
> +#include <linux/dma/dw.h>
> +
> +static struct dw_dma_slave sata_dwc_dma_dws = {
> +	.src_id = 0,
> +	.dst_id = 0,
> +	.src_master = 0,
> +	.dst_master = 1,
> +};
> +
> +static bool sata_dwc_dma_filter(struct dma_chan *chan, void *param)
> +{
> +	struct dw_dma_slave *dws = &sata_dwc_dma_dws;
> +
> +	if (dws->dma_dev != chan->device->dev)
> +		return false;
> +
> +	chan->private = dws;
> +	return true;
> +}
> +
> +static int sata_dwc_dma_get_channel_old(struct sata_dwc_device_port
> *hsdevp)
> +{
> +	struct sata_dwc_device *hsdev = hsdevp->hsdev;
> +	struct dw_dma_slave *dws = &sata_dwc_dma_dws;
> +	dma_cap_mask_t mask;
> +
> +	dws->dma_dev = hsdev->dev;
> +
> +	dma_cap_zero(mask);
> +	dma_cap_set(DMA_SLAVE, mask);
> +
> +	/* Acquire DMA channel */
> +	hsdevp->chan = dma_request_channel(mask,
> sata_dwc_dma_filter, hsdevp);
> +	if (!hsdevp->chan) {
> +		dev_err(hsdev->dev, "%s: dma channel unavailable\n",
> +			Â__func__);
> +		return -EAGAIN;
> +	}
> +
> +	return 0;
> +}
> +
> +static int sata_dwc_dma_init_old(struct platform_device *pdev,
> +				Âstruct sata_dwc_device *hsdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	int err;
> +
> +	hsdev->dma = devm_kzalloc(&pdev->dev, sizeof(*hsdev->dma),
> GFP_KERNEL);
> +	if (!hsdev->dma)
> +		return -ENOMEM;
> +
> +	hsdev->dma->dev = &pdev->dev;
> +
> +	/* Get SATA DMA interrupt number */
> +	hsdev->dma->irq = irq_of_parse_and_map(np, 1);
> +	if (hsdev->dma->irq == NO_IRQ) {
> +		dev_err(&pdev->dev, "no SATA DMA irq\n");
> +		return -ENODEV;
> +	}
> +
> +	/* Get physical SATA DMA register base address */
> +	hsdev->dma->regs = of_iomap(np, 1);
> +	if (!hsdev->dma->regs) {
> +		dev_err(&pdev->dev,
> +			"ioremap failed for AHBDMA register
> address\n");
> +		return -ENODEV;
> +	}
> +
> +	/* Initialize AHB DMAC */
> +	err = dw_dma_probe(hsdev->dma, NULL);
> +	if (err) {
> +		iounmap(hsdev->dma->regs);
> +		return err;
> +	}
> +
> +	return 0;
> +}
> +
> +#endif
> +
> Âstatic const char *get_prot_descript(u8 protocol)
> Â{
> Â	switch ((enum ata_tf_protocols)protocol) {
> @@ -783,18 +858,6 @@ static void sata_dwc_enable_interrupts(struct
> sata_dwc_device *hsdev)
> Â		in_le32(&hsdev->sata_dwc_regs->errmr));
> Â}
> Â
> -static bool sata_dwc_dma_filter(struct dma_chan *chan, void *param)
> -{
> -	struct sata_dwc_device_port *hsdevp = param;
> -	struct dw_dma_slave *dws = hsdevp->dws;
> -
> -	if (dws->dma_dev != chan->device->dev)
> -		return false;
> -
> -	chan->private = dws;
> -	return true;
> -}
> -
> Âstatic void sata_dwc_setup_port(struct ata_ioports *port, unsigned
> long base)
> Â{
> Â	port->cmd_addr = (void __iomem *)base + 0x00;
> @@ -817,6 +880,26 @@ static void sata_dwc_setup_port(struct
> ata_ioports *port, unsigned long base)
> Â	port->ctl_addr = (void __iomem *)base + 0x20;
> Â}
> Â
> +static int sata_dwc_dma_get_channel(struct sata_dwc_device_port
> *hsdevp)
> +{
> +	struct sata_dwc_device *hsdev = hsdevp->hsdev;
> +	struct device *dev = hsdev->dev;
> +
> +#ifdef CONFIG_SATA_DWC_OLD_DMA
> +	if (!of_find_property(dev->of_node, "dmas", NULL))
> +		return sata_dwc_dma_get_channel_old(hsdevp);
> +#endif
> +
> +	hsdevp->chan = dma_request_slave_channel(dev, "sata-dma");
> +	if (IS_ERR(hsdevp->chan)) {
> +		dev_err(dev, "failed to allocate dma channel:
> %ld\n",
> +			PTR_ERR(hsdevp->chan));
> +		return PTR_ERR(hsdevp->chan);
> +	}
> +
> +	return 0;
> +}
> +
> Â/*
> Â * Function : sata_dwc_port_start
> Â * arguments : struct ata_ioports *port
> @@ -829,7 +912,6 @@ static int sata_dwc_port_start(struct ata_port
> *ap)
> Â	struct sata_dwc_device *hsdev;
> Â	struct sata_dwc_device_port *hsdevp = NULL;
> Â	struct device *pdev;
> -	dma_cap_mask_t mask;
> Â	int i;
> Â
> Â	hsdev = HSDEV_FROM_AP(ap);
> @@ -853,20 +935,9 @@ static int sata_dwc_port_start(struct ata_port
> *ap)
> Â	}
> Â	hsdevp->hsdev = hsdev;
> Â
> -	hsdevp->dws = &sata_dwc_dma_dws;
> -	hsdevp->dws->dma_dev = hsdev->dev;
> -
> -	dma_cap_zero(mask);
> -	dma_cap_set(DMA_SLAVE, mask);
> -
> -	/* Acquire DMA channel */
> -	hsdevp->chan = dma_request_channel(mask,
> sata_dwc_dma_filter, hsdevp);
> -	if (!hsdevp->chan) {
> -		dev_err(hsdev->dev, "%s: dma channel unavailable\n",
> -			Â__func__);
> -		err = -EAGAIN;
> +	err = sata_dwc_dma_get_channel(hsdevp);
> +	if (err)
> Â		goto CLEANUP_ALLOC;
> -	}
> Â
> Â	for (i = 0; i < SATA_DWC_QCMD_MAX; i++)
> Â		hsdevp->cmd_issued[i] = SATA_DWC_CMD_ISSUED_NOT;
> @@ -1225,33 +1296,9 @@ static int sata_dwc_probe(struct
> platform_device *ofdev)
> Â	dev_notice(&ofdev->dev, "id %d, controller version
> %c.%c%c\n",
> Â		ÂÂÂidr, ver[0], ver[1], ver[2]);
> Â
> -	/* Get SATA DMA interrupt number */
> -	hsdev->dma->irq = irq_of_parse_and_map(np, 1);
> -	if (hsdev->dma->irq == NO_IRQ) {
> -		dev_err(&ofdev->dev, "no SATA DMA irq\n");
> -		err = -ENODEV;
> -		goto error_iomap;
> -	}
> -
> -	/* Get physical SATA DMA register base address */
> -	hsdev->dma->regs = of_iomap(np, 1);
> -	if (!hsdev->dma->regs) {
> -		dev_err(&ofdev->dev,
> -			"ioremap failed for AHBDMA register
> address\n");
> -		err = -ENODEV;
> -		goto error_iomap;
> -	}
> -
> Â	/* Save dev for later use in dev_xxx() routines */
> Â	hsdev->dev = &ofdev->dev;
> Â
> -	hsdev->dma->dev = &ofdev->dev;
> -
> -	/* Initialize AHB DMAC */
> -	err = dw_dma_probe(hsdev->dma, NULL);
> -	if (err)
> -		goto error_dma_iomap;
> -
> Â	/* Enable SATA Interrupts */
> Â	sata_dwc_enable_interrupts(hsdev);
> Â
> @@ -1263,6 +1310,14 @@ static int sata_dwc_probe(struct
> platform_device *ofdev)
> Â		goto error_out;
> Â	}
> Â
> +#ifdef CONFIG_SATA_DWC_OLD_DMA
> +	if (!of_find_property(np, "dmas", NULL)) {
> +		err = sata_dwc_dma_init_old(ofdev, hsdev);
> +		if (err)
> +			goto error_out;
> +	}
> +#endif
> +
> Â	/*
> Â	Â* Now, register with libATA core, this will also initiate
> the
> Â	Â* device discovery process, invoking our port_start()
> handler &
> @@ -1276,11 +1331,6 @@ static int sata_dwc_probe(struct
> platform_device *ofdev)
> Â	return 0;
> Â
> Âerror_out:
> -	/* Free SATA DMA resources */
> -	dw_dma_remove(hsdev->dma);
> -error_dma_iomap:
> -	iounmap(hsdev->dma->regs);
> -error_iomap:
> Â	iounmap(base);
> Â	return err;
> Â}
> @@ -1293,10 +1343,14 @@ static int sata_dwc_remove(struct
> platform_device *ofdev)
> Â
> Â	ata_host_detach(host);
> Â
> +#ifdef CONFIG_SATA_DWC_OLD_DMA
> Â	/* Free SATA DMA resources */
> -	dw_dma_remove(hsdev->dma);
> +	if (hsdev->dma) {
> +		dw_dma_remove(hsdev->dma);
> +		iounmap(hsdev->dma->regs);
> +	}
> +#endif
> Â
> -	iounmap(hsdev->dma->regs);
> Â	iounmap(hsdev->reg_base);
> Â	dev_dbg(&ofdev->dev, "done\n");
> Â	return 0;

-- 
Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
Intel Finland Oy

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/