LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v2] fix wrong usage of dmaengine_unmap_put in async_xxx
From: Dan Williams @ 2014-04-10  4:01 UTC (permalink / raw)
  To: xuelin.shi; +Cc: Koul, Vinod, dmaengine@vger.kernel.org, linuxppc-dev
In-Reply-To: <1395303385-29461-1-git-send-email-xuelin.shi@freescale.com>

On Thu, Mar 20, 2014 at 1:16 AM,  <xuelin.shi@freescale.com> wrote:
> From: Xuelin Shi <xuelin.shi@freescale.com>
>
> dmaengine_unmap_put does below two things:
> a) unmap pages for srcs and dests
> b) free unmap struct
>
> The unmap struct data is generated but only initialized while
> other some dma contions are met, like dma alignment etc.
> If the unmap data is not initialized, call dmaengine_unmap_put
> will unmap some random data in unmap->addr[...]

Actually, this should be fixed by your other patch:

https://patchwork.kernel.org/patch/3863711/

Because the to_cnt, from_cnt, are properly initialized to zero.  The
only issue was that the unmap pool was not specified.

Can you verify that the problem still exists with that patch applied?
I'll mark it for -stable.

> Also call dmaengine_get_unmap_data immediatally after generating tx
> is not correct. Maybe the tx has not been finished by DMA hardware
> yet but the srcs and dests are dma unmapped.

I disagree.  It is correct because the unmap data is reference
counted.  If the DMA hardware is still using the buffers then it must
hold a reference on the unmap data.  The dmaengine_put_unmap_data()
instances your are removing are the ones that drop the initial
reference count set in dmaengine_get_unmap_data().

^ permalink raw reply

* Re: [PATCH v2] fix dmaengine_unmap failure
From: Dan Williams @ 2014-04-10  4:03 UTC (permalink / raw)
  To: xuelin.shi; +Cc: Koul, Vinod, dmaengine@vger.kernel.org, linuxppc-dev
In-Reply-To: <1395297230-8195-1-git-send-email-xuelin.shi@freescale.com>

On Wed, Mar 19, 2014 at 11:33 PM,  <xuelin.shi@freescale.com> wrote:
> From: Xuelin Shi <xuelin.shi@freescale.com>
>
> The count which is used to get_unmap_data maybe not the same as the
> count computed in dmaengine_unmap which causes to free data in a
> wrong pool.
>
> This patch fixes this issue by keeping the map count with unmap_data
> structure and use this count to get the pool.
>
> Signed-off-by: Xuelin Shi <xuelin.shi@freescale.com>
> ---
> change history:
>         v1: keep mempool pointer with unmap struct
>         v2: keep u8 map_cnt instead of mempool pointer to save mem.
>

Looks good, but please next time remember to prefix your subject line.  I.e.

"dmaengine: fix dmaengine_unmap failure"

I'll fix this up.

--
Dan

^ permalink raw reply

* [PATCH] Fix 3bc95598 'powerpc/PCI: Use list_for_each_entry() for bus traversal'
From: Mike Qiu @ 2014-04-10  6:51 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: wangyijing, Mike Qiu, anton, bhelgaas, paulus

Unable to handle kernel paging request for data at address 0x00000000
Faulting instruction address: 0xc000000000041d78
Oops: Kernel access of bad area, sig: 11 [#1]
...
NIP [c000000000041d78] .sys_pciconfig_iobase+0x68/0x1f0
LR [c000000000041e0c] .sys_pciconfig_iobase+0xfc/0x1f0
Call Trace:
[c0000003b4787db0] [c000000000041e0c] .sys_pciconfig_iobase+0xfc/0x1f0 (unreliable)
[c0000003b4787e30] [c000000000009ed8] syscall_exit+0x0/0x98

This bug was introduced by commit 3bc955987fb377f3c95bc29deb498e96819b8451
The root cause was the 'bus' has been set to null while try to access
bus->next.

Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/pci_64.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
index 2a47790..7b6c1ae 100644
--- a/arch/powerpc/kernel/pci_64.c
+++ b/arch/powerpc/kernel/pci_64.c
@@ -209,6 +209,7 @@ long sys_pciconfig_iobase(long which, unsigned long in_bus,
 {
 	struct pci_controller* hose;
 	struct pci_bus *bus = NULL;
+	struct pci_bus *tmp_bus = NULL;
 	struct device_node *hose_node;
 
 	/* Argh ! Please forgive me for that hack, but that's the
@@ -229,10 +230,12 @@ long sys_pciconfig_iobase(long which, unsigned long in_bus,
 	 * used on pre-domains setup. We return the first match
 	 */
 
-	list_for_each_entry(bus, &pci_root_buses, node) {
-		if (in_bus >= bus->number && in_bus <= bus->busn_res.end)
+	list_for_each_entry(tmp_bus, &pci_root_buses, node) {
+		if (in_bus >= tmp_bus->number &&
+		    in_bus <= tmp_bus->busn_res.end) {
+			bus = tmp_bus;
 			break;
-		bus = NULL;
+		}
 	}
 	if (bus == NULL || bus->dev.of_node == NULL)
 		return -ENODEV;
-- 
1.8.0.1

^ permalink raw reply related

* Re: [PATCH v2 8/8] DMA: Freescale: add suspend resume functions for DMA driver
From: Hongbo Zhang @ 2014-04-10  6:55 UTC (permalink / raw)
  To: vkoul, dan.j.williams, dmaengine; +Cc: scottwood, linuxppc-dev, linux-kernel
In-Reply-To: <1396582037-23065-9-git-send-email-hongbo.zhang@freescale.com>


On 04/04/2014 11:27 AM, hongbo.zhang@freescale.com wrote:
> From: Hongbo Zhang <hongbo.zhang@freescale.com>
>
> This patch adds suspend resume functions for Freescale DMA driver.
> .prepare callback is used to stop further descriptors from being added into the
> pending queue, and also issue pending queues into execution if there is any.
> .suspend callback makes sure all the pending jobs are cleaned up and all the
> channels are idle, and save the mode registers.
> .resume callback re-initializes the channels by restore the mode registers.
>
> Signed-off-by: Hongbo Zhang <hongbo.zhang@freescale.com>
> ---
>   drivers/dma/fsldma.c |   99 ++++++++++++++++++++++++++++++++++++++++++++++++++
>   drivers/dma/fsldma.h |   16 ++++++++
>   2 files changed, 115 insertions(+)
>
> diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
> index c9bf54a..91482d2 100644
> --- a/drivers/dma/fsldma.c
> +++ b/drivers/dma/fsldma.c
> @@ -400,6 +400,14 @@ static dma_cookie_t fsl_dma_tx_submit(struct dma_async_tx_descriptor *tx)
>   
>   	spin_lock_bh(&chan->desc_lock);
>   
> +#ifdef CONFIG_PM
> +	if (unlikely(chan->pm_state != RUNNING)) {
> +		chan_dbg(chan, "cannot submit due to suspend\n");
> +		spin_unlock_bh(&chan->desc_lock);
> +		return -1;
> +	}
> +#endif
> +
>   	/*
>   	 * assign cookies to all of the software descriptors
>   	 * that make up this transaction
> @@ -1311,6 +1319,9 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
>   	INIT_LIST_HEAD(&chan->ld_running);
>   	INIT_LIST_HEAD(&chan->ld_completed);
>   	chan->idle = true;
> +#ifdef CONFIG_PM
> +	chan->pm_state = RUNNING;
> +#endif
>   
>   	chan->common.device = &fdev->common;
>   	dma_cookie_init(&chan->common);
> @@ -1450,6 +1461,91 @@ static int fsldma_of_remove(struct platform_device *op)
>   	return 0;
>   }
>   
> +#ifdef CONFIG_PM
> +static int fsldma_prepare(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct fsldma_device *fdev = platform_get_drvdata(pdev);
> +	struct fsldma_chan *chan;
> +	int i;
> +
> +	for (i = 0; i < FSL_DMA_MAX_CHANS_PER_DEVICE; i++) {
> +		chan = fdev->chan[i];
> +		if (!chan)
> +			continue;
> +
> +		spin_lock_bh(&chan->desc_lock);
> +		chan->pm_state = SUSPENDING;
> +		if (!list_empty(&chan->ld_pending))
> +			fsl_chan_xfer_ld_queue(chan);
> +		spin_unlock_bh(&chan->desc_lock);
> +	}
> +
> +	return 0;
> +}
> +
> +static int fsldma_suspend(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct fsldma_device *fdev = platform_get_drvdata(pdev);
> +	struct fsldma_chan *chan;
> +	int i;
> +
> +	for (i = 0; i < FSL_DMA_MAX_CHANS_PER_DEVICE; i++) {
> +		chan = fdev->chan[i];
> +		if (!chan)
> +			continue;
> +
> +		spin_lock_bh(&chan->desc_lock);
> +		if (!chan->idle)
> +			goto out;
> +		chan->regs_save.mr = DMA_IN(chan, &chan->regs->mr, 32);
> +		chan->pm_state = SUSPENDED;
> +		spin_unlock_bh(&chan->desc_lock);
> +	}
> +	return 0;
> +
> +out:
> +	for (; i >= 0; i--) {
> +		chan = fdev->chan[i];
> +		if (!chan)
> +			continue;

the pm_state should be restored too, e.g.
     chan->pm_state = RUNNING;
I will send new iteration for adding this.

> +		spin_unlock_bh(&chan->desc_lock);
> +	}
> +	return -EBUSY;
> +}
> +
> +static int fsldma_resume(struct device *dev)
> +{
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct fsldma_device *fdev = platform_get_drvdata(pdev);
> +	struct fsldma_chan *chan;
> +	u32 mode;
> +	int i;
> +
> +	for (i = 0; i < FSL_DMA_MAX_CHANS_PER_DEVICE; i++) {
> +		chan = fdev->chan[i];
> +		if (!chan)
> +			continue;
> +
> +		spin_lock_bh(&chan->desc_lock);
> +		mode = chan->regs_save.mr
> +			& ~FSL_DMA_MR_CS & ~FSL_DMA_MR_CC & ~FSL_DMA_MR_CA;
> +		DMA_OUT(chan, &chan->regs->mr, mode, 32);
> +		chan->pm_state = RUNNING;
> +		spin_unlock_bh(&chan->desc_lock);
> +	}
> +
> +	return 0;
> +}
> +
> +static const struct dev_pm_ops fsldma_pm_ops = {
> +	.prepare	= fsldma_prepare,
> +	.suspend	= fsldma_suspend,
> +	.resume		= fsldma_resume,
> +};
> +#endif
> +
>   static const struct of_device_id fsldma_of_ids[] = {
>   	{ .compatible = "fsl,elo3-dma", },
>   	{ .compatible = "fsl,eloplus-dma", },
> @@ -1462,6 +1558,9 @@ static struct platform_driver fsldma_of_driver = {
>   		.name = "fsl-elo-dma",
>   		.owner = THIS_MODULE,
>   		.of_match_table = fsldma_of_ids,
> +#ifdef CONFIG_PM
> +		.pm = &fsldma_pm_ops,
> +#endif
>   	},
>   	.probe = fsldma_of_probe,
>   	.remove = fsldma_of_remove,
> diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h
> index ec19517..eecaf9e 100644
> --- a/drivers/dma/fsldma.h
> +++ b/drivers/dma/fsldma.h
> @@ -134,6 +134,18 @@ struct fsldma_device {
>   #define FSL_DMA_CHAN_PAUSE_EXT	0x00001000
>   #define FSL_DMA_CHAN_START_EXT	0x00002000
>   
> +#ifdef CONFIG_PM
> +struct fsldma_chan_regs_save {
> +	u32 mr;
> +};
> +
> +enum fsldma_pm_state {
> +	RUNNING = 0,
> +	SUSPENDING,
> +	SUSPENDED,
> +};
> +#endif
> +
>   struct fsldma_chan {
>   	char name[8];			/* Channel name */
>   	struct fsldma_chan_regs __iomem *regs;
> @@ -161,6 +173,10 @@ struct fsldma_chan {
>   	struct tasklet_struct tasklet;
>   	u32 feature;
>   	bool idle;			/* DMA controller is idle */
> +#ifdef CONFIG_PM
> +	struct fsldma_chan_regs_save regs_save;
> +	enum fsldma_pm_state pm_state;
> +#endif
>   
>   	void (*toggle_ext_pause)(struct fsldma_chan *fsl_chan, int enable);
>   	void (*toggle_ext_start)(struct fsldma_chan *fsl_chan, int enable);

^ permalink raw reply

* Re: [PATCH 01/60] powerpc/powernv: Return secondary CPUs to firmware before FW update
From: Vasant Hegde @ 2014-04-10  7:04 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <1397063935-30478-1-git-send-email-hegdevasant@linux.vnet.ibm.com>

On 04/09/2014 10:48 PM, Vasant Hegde wrote:
> Firmware update on PowerNV platform takes several minutes. During
> this time one CPU is stuck in FW and the kernel complains about "soft
> lockups".
>

Ben,

Sorry for the confusion in subject line.. Its just 1 patch.. not 1/60 .

-Vasant

> This patch returns all secondary CPUs to firmware before starting
> firmware update process.
>
> [ Reworked a bit and cleaned up -- BenH ]
>
> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com>
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> ---
>   arch/powerpc/include/asm/opal.h             |  1 +
>   arch/powerpc/platforms/powernv/opal-flash.c | 47 ++++++++++++++++++++++++++---
>   arch/powerpc/platforms/powernv/setup.c      | 25 +++++++++++++--
>   3 files changed, 66 insertions(+), 7 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
> index 05a23d0..5c34170 100644
> --- a/arch/powerpc/include/asm/opal.h
> +++ b/arch/powerpc/include/asm/opal.h
> @@ -922,6 +922,7 @@ extern unsigned long opal_get_boot_time(void);
>   extern void opal_nvram_init(void);
>   extern int opal_elog_register_init(void);
>   extern void opal_flash_init(void);
> +extern void opal_flash_term_callback(void);
>   extern int opal_elog_init(void);
>   extern void opal_platform_dump_init(void);
>   extern void opal_sys_param_init(void);
> diff --git a/arch/powerpc/platforms/powernv/opal-flash.c b/arch/powerpc/platforms/powernv/opal-flash.c
> index 16e571b..11ab43f 100644
> --- a/arch/powerpc/platforms/powernv/opal-flash.c
> +++ b/arch/powerpc/platforms/powernv/opal-flash.c
> @@ -20,6 +20,7 @@
>   #include <linux/mm.h>
>   #include <linux/vmalloc.h>
>   #include <linux/pagemap.h>
> +#include <linux/delay.h>
>
>   #include <asm/opal.h>
>
> @@ -388,11 +389,6 @@ static int opal_flash_update(int op)
>   			(sg->num_entries * sizeof(struct opal_sg_entry) + 16);
>   	}
>
> -	pr_alert("FLASH: Image is %u bytes\n", image_data.size);
> -	pr_alert("FLASH: Image update requested\n");
> -	pr_alert("FLASH: Image will be updated during system reboot\n");
> -	pr_alert("FLASH: This will take several minutes. Do not power off!\n");
> -
>   flash:
>   	rc = opal_update_flash(addr);
>
> @@ -400,6 +396,47 @@ invalid_img:
>   	return rc;
>   }
>
> +/* Return CPUs to OPAL before starting FW update */
> +static void flash_return_cpu(void *info)
> +{
> +	int cpu = smp_processor_id();
> +
> +	if (!cpu_online(cpu))
> +		return;
> +
> +	/* Disable IRQ */
> +	hard_irq_disable();
> +
> +	/* Return the CPU to OPAL */
> +	opal_return_cpu();
> +}
> +
> +/* This gets called just before system reboots */
> +void opal_flash_term_callback(void)
> +{
> +	struct cpumask mask;
> +
> +	if (update_flash_data.status != FLASH_IMG_READY)
> +		return;
> +
> +	pr_alert("FLASH: Flashing new firmware\n");
> +	pr_alert("FLASH: Image is %u bytes\n", image_data.size);
> +	pr_alert("FLASH: Performing flash and reboot/shutdown\n");
> +	pr_alert("FLASH: This will take several minutes. Do not power off!\n");
> +
> +	/* Small delay to help getting the above message out */
> +	msleep(500);
> +
> +	/* Return secondary CPUs to firmware */
> +	cpumask_copy(&mask, cpu_online_mask);
> +	cpumask_clear_cpu(smp_processor_id(), &mask);
> +	if (!cpumask_empty(&mask))
> +		smp_call_function_many(&mask,
> +				       flash_return_cpu, NULL, false);
> +	/* Hard disable interrupts */
> +	hard_irq_disable();
> +}
> +
>   /*
>    * Show candidate image status
>    */
> diff --git a/arch/powerpc/platforms/powernv/setup.c b/arch/powerpc/platforms/powernv/setup.c
> index 1735678..42c16a6 100644
> --- a/arch/powerpc/platforms/powernv/setup.c
> +++ b/arch/powerpc/platforms/powernv/setup.c
> @@ -98,11 +98,32 @@ static void pnv_show_cpuinfo(struct seq_file *m)
>   	of_node_put(root);
>   }
>
> +static void pnv_prepare_going_down(void)
> +{
> +	/*
> +	 * Disable all notifiers from OPAL, we can't
> +	 * service interrupts anymore anyway
> +	 */
> +	opal_notifier_disable();
> +
> +	/* Soft disable interrupts */
> +	local_irq_disable();
> +
> +	/*
> +	 * Return secondary CPUs to firwmare if a flash update
> +	 * is pending otherwise we will get all sort of error
> +	 * messages about CPU being stuck etc.. This will also
> +	 * have the side effect of hard disabling interrupts so
> +	 * past this point, the kernel is effectively dead.
> +	 */
> +	opal_flash_term_callback();
> +}
> +
>   static void  __noreturn pnv_restart(char *cmd)
>   {
>   	long rc = OPAL_BUSY;
>
> -	opal_notifier_disable();
> +	pnv_prepare_going_down();
>
>   	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
>   		rc = opal_cec_reboot();
> @@ -119,7 +140,7 @@ static void __noreturn pnv_power_off(void)
>   {
>   	long rc = OPAL_BUSY;
>
> -	opal_notifier_disable();
> +	pnv_prepare_going_down();
>
>   	while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
>   		rc = opal_cec_power_down(0);
>

^ permalink raw reply

* [PATCH v3 0/8] DMA: Freescale: driver cleanups and enhancements
From: hongbo.zhang @ 2014-04-10  7:09 UTC (permalink / raw)
  To: vkoul, dan.j.williams, dmaengine
  Cc: scottwood, Hongbo Zhang, linuxppc-dev, linux-kernel, leo.li

From: Hongbo Zhang <hongbo.zhang@linaro.org>

Hi Vinod Koul,
Please have a look at the v3 patch set.

v2 -> v3 change:
Only add "chan->pm_state = RUNNING" for patch[8/8].

v1 -> v2 change:
The only one change is introducing a new patch[1/7] to remove the unnecessary
macro FSL_DMA_LD_DEBUG, thus the total patches number is 8 now (was 7)

Hongbo Zhang (8):
  DMA: Freescale: remove the unnecessary FSL_DMA_LD_DEBUG
  DMA: Freescale: unify register access methods
  DMA: Freescale: remove attribute DMA_INTERRUPT of dmaengine
  DMA: Freescale: add fsl_dma_free_descriptor() to reduce code
    duplication
  DMA: Freescale: move functions to avoid forward declarations
  DMA: Freescale: change descriptor release process for supporting
    async_tx
  DMA: Freescale: use spin_lock_bh instead of spin_lock_irqsave
  DMA: Freescale: add suspend resume functions for DMA driver

 drivers/dma/fsldma.c |  591 ++++++++++++++++++++++++++++++++------------------
 drivers/dma/fsldma.h |   33 ++-
 2 files changed, 409 insertions(+), 215 deletions(-)

-- 
1.7.9.5

^ permalink raw reply

* [PATCH v3 1/8] DMA: Freescale: remove the unnecessary FSL_DMA_LD_DEBUG
From: hongbo.zhang @ 2014-04-10  7:09 UTC (permalink / raw)
  To: vkoul, dan.j.williams, dmaengine
  Cc: scottwood, Hongbo Zhang, linuxppc-dev, linux-kernel, leo.li
In-Reply-To: <1397113805-24171-1-git-send-email-hongbo.zhang@freescale.com>

From: Hongbo Zhang <hongbo.zhang@freescale.com>

Some codes are calling chan_dbg with FSL_DMA_LD_DEBUG surrounded, it is really
unnecessary to use such a macro because chan_dbg is a wrapper of dev_dbg, we do
have corresponding DEBUG macro to switch on/off dev_dbg, and most of the other
codes are also calling chan_dbg directly without using FSL_DMA_LD_DEBUG.

Signed-off-by: Hongbo Zhang <hongbo.zhang@freescale.com>
---
 drivers/dma/fsldma.c |   10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index f157c6f..ec50420 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -426,9 +426,7 @@ static struct fsl_desc_sw *fsl_dma_alloc_descriptor(struct fsldma_chan *chan)
 	desc->async_tx.tx_submit = fsl_dma_tx_submit;
 	desc->async_tx.phys = pdesc;
 
-#ifdef FSL_DMA_LD_DEBUG
 	chan_dbg(chan, "LD %p allocated\n", desc);
-#endif
 
 	return desc;
 }
@@ -479,9 +477,7 @@ static void fsldma_free_desc_list(struct fsldma_chan *chan,
 
 	list_for_each_entry_safe(desc, _desc, list, node) {
 		list_del(&desc->node);
-#ifdef FSL_DMA_LD_DEBUG
 		chan_dbg(chan, "LD %p free\n", desc);
-#endif
 		dma_pool_free(chan->desc_pool, desc, desc->async_tx.phys);
 	}
 }
@@ -493,9 +489,7 @@ static void fsldma_free_desc_list_reverse(struct fsldma_chan *chan,
 
 	list_for_each_entry_safe_reverse(desc, _desc, list, node) {
 		list_del(&desc->node);
-#ifdef FSL_DMA_LD_DEBUG
 		chan_dbg(chan, "LD %p free\n", desc);
-#endif
 		dma_pool_free(chan->desc_pool, desc, desc->async_tx.phys);
 	}
 }
@@ -832,9 +826,7 @@ static void fsldma_cleanup_descriptor(struct fsldma_chan *chan,
 
 	/* Run the link descriptor callback function */
 	if (txd->callback) {
-#ifdef FSL_DMA_LD_DEBUG
 		chan_dbg(chan, "LD %p callback\n", desc);
-#endif
 		txd->callback(txd->callback_param);
 	}
 
@@ -842,9 +834,7 @@ static void fsldma_cleanup_descriptor(struct fsldma_chan *chan,
 	dma_run_dependencies(txd);
 
 	dma_descriptor_unmap(txd);
-#ifdef FSL_DMA_LD_DEBUG
 	chan_dbg(chan, "LD %p free\n", desc);
-#endif
 	dma_pool_free(chan->desc_pool, desc, txd->phys);
 }
 
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v3 3/8] DMA: Freescale: remove attribute DMA_INTERRUPT of dmaengine
From: hongbo.zhang @ 2014-04-10  7:10 UTC (permalink / raw)
  To: vkoul, dan.j.williams, dmaengine
  Cc: scottwood, Hongbo Zhang, linuxppc-dev, linux-kernel, leo.li
In-Reply-To: <1397113805-24171-1-git-send-email-hongbo.zhang@freescale.com>

From: Hongbo Zhang <hongbo.zhang@freescale.com>

Delete attribute DMA_INTERRUPT because fsldma doesn't support this function,
exception will be thrown if talitos is used to offload xor at the same time.

Signed-off-by: Hongbo Zhang <hongbo.zhang@freescale.com>
Signed-off-by: Qiang Liu <qiang.liu@freescale.com>
---
 drivers/dma/fsldma.c |   31 -------------------------------
 1 file changed, 31 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 5f32cb8..b71cc04 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -528,35 +528,6 @@ static void fsl_dma_free_chan_resources(struct dma_chan *dchan)
 }
 
 static struct dma_async_tx_descriptor *
-fsl_dma_prep_interrupt(struct dma_chan *dchan, unsigned long flags)
-{
-	struct fsldma_chan *chan;
-	struct fsl_desc_sw *new;
-
-	if (!dchan)
-		return NULL;
-
-	chan = to_fsl_chan(dchan);
-
-	new = fsl_dma_alloc_descriptor(chan);
-	if (!new) {
-		chan_err(chan, "%s\n", msg_ld_oom);
-		return NULL;
-	}
-
-	new->async_tx.cookie = -EBUSY;
-	new->async_tx.flags = flags;
-
-	/* Insert the link descriptor to the LD ring */
-	list_add_tail(&new->node, &new->tx_list);
-
-	/* Set End-of-link to the last link descriptor of new list */
-	set_ld_eol(chan, new);
-
-	return &new->async_tx;
-}
-
-static struct dma_async_tx_descriptor *
 fsl_dma_prep_memcpy(struct dma_chan *dchan,
 	dma_addr_t dma_dst, dma_addr_t dma_src,
 	size_t len, unsigned long flags)
@@ -1308,12 +1279,10 @@ static int fsldma_of_probe(struct platform_device *op)
 	fdev->irq = irq_of_parse_and_map(op->dev.of_node, 0);
 
 	dma_cap_set(DMA_MEMCPY, fdev->common.cap_mask);
-	dma_cap_set(DMA_INTERRUPT, fdev->common.cap_mask);
 	dma_cap_set(DMA_SG, fdev->common.cap_mask);
 	dma_cap_set(DMA_SLAVE, fdev->common.cap_mask);
 	fdev->common.device_alloc_chan_resources = fsl_dma_alloc_chan_resources;
 	fdev->common.device_free_chan_resources = fsl_dma_free_chan_resources;
-	fdev->common.device_prep_dma_interrupt = fsl_dma_prep_interrupt;
 	fdev->common.device_prep_dma_memcpy = fsl_dma_prep_memcpy;
 	fdev->common.device_prep_dma_sg = fsl_dma_prep_sg;
 	fdev->common.device_tx_status = fsl_tx_status;
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v3 2/8] DMA: Freescale: unify register access methods
From: hongbo.zhang @ 2014-04-10  7:09 UTC (permalink / raw)
  To: vkoul, dan.j.williams, dmaengine
  Cc: scottwood, Hongbo Zhang, linuxppc-dev, linux-kernel, leo.li
In-Reply-To: <1397113805-24171-1-git-send-email-hongbo.zhang@freescale.com>

From: Hongbo Zhang <hongbo.zhang@freescale.com>

Methods of accessing DMA contorller registers are inconsistent, some registers
are accessed by DMA_IN/OUT directly, while others are accessed by functions
get/set_* which are wrappers of DMA_IN/OUT, and even for the BCR register, it
is read by get_bcr but written by DMA_OUT.
This patch unifies the inconsistent methods, all registers are accessed by
get/set_* now.

Signed-off-by: Hongbo Zhang <hongbo.zhang@freescale.com>
---
 drivers/dma/fsldma.c |   52 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 33 insertions(+), 19 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index ec50420..5f32cb8 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -61,6 +61,16 @@ static u32 get_sr(struct fsldma_chan *chan)
 	return DMA_IN(chan, &chan->regs->sr, 32);
 }
 
+static void set_mr(struct fsldma_chan *chan, u32 val)
+{
+	DMA_OUT(chan, &chan->regs->mr, val, 32);
+}
+
+static u32 get_mr(struct fsldma_chan *chan)
+{
+	return DMA_IN(chan, &chan->regs->mr, 32);
+}
+
 static void set_cdar(struct fsldma_chan *chan, dma_addr_t addr)
 {
 	DMA_OUT(chan, &chan->regs->cdar, addr | FSL_DMA_SNEN, 64);
@@ -71,6 +81,11 @@ static dma_addr_t get_cdar(struct fsldma_chan *chan)
 	return DMA_IN(chan, &chan->regs->cdar, 64) & ~FSL_DMA_SNEN;
 }
 
+static void set_bcr(struct fsldma_chan *chan, u32 val)
+{
+	DMA_OUT(chan, &chan->regs->bcr, val, 32);
+}
+
 static u32 get_bcr(struct fsldma_chan *chan)
 {
 	return DMA_IN(chan, &chan->regs->bcr, 32);
@@ -135,7 +150,7 @@ static void set_ld_eol(struct fsldma_chan *chan, struct fsl_desc_sw *desc)
 static void dma_init(struct fsldma_chan *chan)
 {
 	/* Reset the channel */
-	DMA_OUT(chan, &chan->regs->mr, 0, 32);
+	set_mr(chan, 0);
 
 	switch (chan->feature & FSL_DMA_IP_MASK) {
 	case FSL_DMA_IP_85XX:
@@ -144,16 +159,15 @@ static void dma_init(struct fsldma_chan *chan)
 		 * EOLNIE - End of links interrupt enable
 		 * BWC - Bandwidth sharing among channels
 		 */
-		DMA_OUT(chan, &chan->regs->mr, FSL_DMA_MR_BWC
-				| FSL_DMA_MR_EIE | FSL_DMA_MR_EOLNIE, 32);
+		set_mr(chan, FSL_DMA_MR_BWC | FSL_DMA_MR_EIE
+			| FSL_DMA_MR_EOLNIE);
 		break;
 	case FSL_DMA_IP_83XX:
 		/* Set the channel to below modes:
 		 * EOTIE - End-of-transfer interrupt enable
 		 * PRC_RM - PCI read multiple
 		 */
-		DMA_OUT(chan, &chan->regs->mr, FSL_DMA_MR_EOTIE
-				| FSL_DMA_MR_PRC_RM, 32);
+		set_mr(chan, FSL_DMA_MR_EOTIE | FSL_DMA_MR_PRC_RM);
 		break;
 	}
 }
@@ -175,10 +189,10 @@ static void dma_start(struct fsldma_chan *chan)
 {
 	u32 mode;
 
-	mode = DMA_IN(chan, &chan->regs->mr, 32);
+	mode = get_mr(chan);
 
 	if (chan->feature & FSL_DMA_CHAN_PAUSE_EXT) {
-		DMA_OUT(chan, &chan->regs->bcr, 0, 32);
+		set_bcr(chan, 0);
 		mode |= FSL_DMA_MR_EMP_EN;
 	} else {
 		mode &= ~FSL_DMA_MR_EMP_EN;
@@ -191,7 +205,7 @@ static void dma_start(struct fsldma_chan *chan)
 		mode |= FSL_DMA_MR_CS;
 	}
 
-	DMA_OUT(chan, &chan->regs->mr, mode, 32);
+	set_mr(chan, mode);
 }
 
 static void dma_halt(struct fsldma_chan *chan)
@@ -200,7 +214,7 @@ static void dma_halt(struct fsldma_chan *chan)
 	int i;
 
 	/* read the mode register */
-	mode = DMA_IN(chan, &chan->regs->mr, 32);
+	mode = get_mr(chan);
 
 	/*
 	 * The 85xx controller supports channel abort, which will stop
@@ -209,14 +223,14 @@ static void dma_halt(struct fsldma_chan *chan)
 	 */
 	if ((chan->feature & FSL_DMA_IP_MASK) == FSL_DMA_IP_85XX) {
 		mode |= FSL_DMA_MR_CA;
-		DMA_OUT(chan, &chan->regs->mr, mode, 32);
+		set_mr(chan, mode);
 
 		mode &= ~FSL_DMA_MR_CA;
 	}
 
 	/* stop the DMA controller */
 	mode &= ~(FSL_DMA_MR_CS | FSL_DMA_MR_EMS_EN);
-	DMA_OUT(chan, &chan->regs->mr, mode, 32);
+	set_mr(chan, mode);
 
 	/* wait for the DMA controller to become idle */
 	for (i = 0; i < 100; i++) {
@@ -245,7 +259,7 @@ static void fsl_chan_set_src_loop_size(struct fsldma_chan *chan, int size)
 {
 	u32 mode;
 
-	mode = DMA_IN(chan, &chan->regs->mr, 32);
+	mode = get_mr(chan);
 
 	switch (size) {
 	case 0:
@@ -259,7 +273,7 @@ static void fsl_chan_set_src_loop_size(struct fsldma_chan *chan, int size)
 		break;
 	}
 
-	DMA_OUT(chan, &chan->regs->mr, mode, 32);
+	set_mr(chan, mode);
 }
 
 /**
@@ -277,7 +291,7 @@ static void fsl_chan_set_dst_loop_size(struct fsldma_chan *chan, int size)
 {
 	u32 mode;
 
-	mode = DMA_IN(chan, &chan->regs->mr, 32);
+	mode = get_mr(chan);
 
 	switch (size) {
 	case 0:
@@ -291,7 +305,7 @@ static void fsl_chan_set_dst_loop_size(struct fsldma_chan *chan, int size)
 		break;
 	}
 
-	DMA_OUT(chan, &chan->regs->mr, mode, 32);
+	set_mr(chan, mode);
 }
 
 /**
@@ -312,10 +326,10 @@ static void fsl_chan_set_request_count(struct fsldma_chan *chan, int size)
 
 	BUG_ON(size > 1024);
 
-	mode = DMA_IN(chan, &chan->regs->mr, 32);
+	mode = get_mr(chan);
 	mode |= (__ilog2(size) << 24) & 0x0f000000;
 
-	DMA_OUT(chan, &chan->regs->mr, mode, 32);
+	set_mr(chan, mode);
 }
 
 /**
@@ -889,9 +903,9 @@ static void fsl_chan_xfer_ld_queue(struct fsldma_chan *chan)
 	if ((chan->feature & FSL_DMA_IP_MASK) == FSL_DMA_IP_85XX) {
 		u32 mode;
 
-		mode = DMA_IN(chan, &chan->regs->mr, 32);
+		mode = get_mr(chan);
 		mode &= ~FSL_DMA_MR_CS;
-		DMA_OUT(chan, &chan->regs->mr, mode, 32);
+		set_mr(chan, mode);
 	}
 
 	/*
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v3 4/8] DMA: Freescale: add fsl_dma_free_descriptor() to reduce code duplication
From: hongbo.zhang @ 2014-04-10  7:10 UTC (permalink / raw)
  To: vkoul, dan.j.williams, dmaengine
  Cc: scottwood, Hongbo Zhang, linuxppc-dev, linux-kernel, leo.li
In-Reply-To: <1397113805-24171-1-git-send-email-hongbo.zhang@freescale.com>

From: Hongbo Zhang <hongbo.zhang@freescale.com>

There are several places where descriptors are freed using identical code.
This patch puts this code into a function to reduce code duplication.

Signed-off-by: Hongbo Zhang <hongbo.zhang@freescale.com>
Signed-off-by: Qiang Liu <qiang.liu@freescale.com>
---
 drivers/dma/fsldma.c |   30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index b71cc04..b5a0ffa 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -418,6 +418,19 @@ static dma_cookie_t fsl_dma_tx_submit(struct dma_async_tx_descriptor *tx)
 }
 
 /**
+ * fsl_dma_free_descriptor - Free descriptor from channel's DMA pool.
+ * @chan : Freescale DMA channel
+ * @desc: descriptor to be freed
+ */
+static void fsl_dma_free_descriptor(struct fsldma_chan *chan,
+		struct fsl_desc_sw *desc)
+{
+	list_del(&desc->node);
+	chan_dbg(chan, "LD %p free\n", desc);
+	dma_pool_free(chan->desc_pool, desc, desc->async_tx.phys);
+}
+
+/**
  * fsl_dma_alloc_descriptor - Allocate descriptor from channel's DMA pool.
  * @chan : Freescale DMA channel
  *
@@ -489,11 +502,8 @@ static void fsldma_free_desc_list(struct fsldma_chan *chan,
 {
 	struct fsl_desc_sw *desc, *_desc;
 
-	list_for_each_entry_safe(desc, _desc, list, node) {
-		list_del(&desc->node);
-		chan_dbg(chan, "LD %p free\n", desc);
-		dma_pool_free(chan->desc_pool, desc, desc->async_tx.phys);
-	}
+	list_for_each_entry_safe(desc, _desc, list, node)
+		fsl_dma_free_descriptor(chan, desc);
 }
 
 static void fsldma_free_desc_list_reverse(struct fsldma_chan *chan,
@@ -501,11 +511,8 @@ static void fsldma_free_desc_list_reverse(struct fsldma_chan *chan,
 {
 	struct fsl_desc_sw *desc, *_desc;
 
-	list_for_each_entry_safe_reverse(desc, _desc, list, node) {
-		list_del(&desc->node);
-		chan_dbg(chan, "LD %p free\n", desc);
-		dma_pool_free(chan->desc_pool, desc, desc->async_tx.phys);
-	}
+	list_for_each_entry_safe_reverse(desc, _desc, list, node)
+		fsl_dma_free_descriptor(chan, desc);
 }
 
 /**
@@ -819,8 +826,7 @@ static void fsldma_cleanup_descriptor(struct fsldma_chan *chan,
 	dma_run_dependencies(txd);
 
 	dma_descriptor_unmap(txd);
-	chan_dbg(chan, "LD %p free\n", desc);
-	dma_pool_free(chan->desc_pool, desc, txd->phys);
+	fsl_dma_free_descriptor(chan, desc);
 }
 
 /**
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v3 5/8] DMA: Freescale: move functions to avoid forward declarations
From: hongbo.zhang @ 2014-04-10  7:10 UTC (permalink / raw)
  To: vkoul, dan.j.williams, dmaengine
  Cc: scottwood, Hongbo Zhang, linuxppc-dev, linux-kernel, leo.li
In-Reply-To: <1397113805-24171-1-git-send-email-hongbo.zhang@freescale.com>

From: Hongbo Zhang <hongbo.zhang@freescale.com>

These functions will be modified in the next patch in the series. By moving the
function in a patch separate from the changes, it will make review easier.

Signed-off-by: Hongbo Zhang <hongbo.zhang@freescale.com>
Signed-off-by: Qiang Liu <qiang.liu@freescale.com>
---
 drivers/dma/fsldma.c |  188 +++++++++++++++++++++++++-------------------------
 1 file changed, 94 insertions(+), 94 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index b5a0ffa..968877f 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -459,6 +459,100 @@ static struct fsl_desc_sw *fsl_dma_alloc_descriptor(struct fsldma_chan *chan)
 }
 
 /**
+ * fsl_chan_xfer_ld_queue - transfer any pending transactions
+ * @chan : Freescale DMA channel
+ *
+ * HARDWARE STATE: idle
+ * LOCKING: must hold chan->desc_lock
+ */
+static void fsl_chan_xfer_ld_queue(struct fsldma_chan *chan)
+{
+	struct fsl_desc_sw *desc;
+
+	/*
+	 * If the list of pending descriptors is empty, then we
+	 * don't need to do any work at all
+	 */
+	if (list_empty(&chan->ld_pending)) {
+		chan_dbg(chan, "no pending LDs\n");
+		return;
+	}
+
+	/*
+	 * The DMA controller is not idle, which means that the interrupt
+	 * handler will start any queued transactions when it runs after
+	 * this transaction finishes
+	 */
+	if (!chan->idle) {
+		chan_dbg(chan, "DMA controller still busy\n");
+		return;
+	}
+
+	/*
+	 * If there are some link descriptors which have not been
+	 * transferred, we need to start the controller
+	 */
+
+	/*
+	 * Move all elements from the queue of pending transactions
+	 * onto the list of running transactions
+	 */
+	chan_dbg(chan, "idle, starting controller\n");
+	desc = list_first_entry(&chan->ld_pending, struct fsl_desc_sw, node);
+	list_splice_tail_init(&chan->ld_pending, &chan->ld_running);
+
+	/*
+	 * The 85xx DMA controller doesn't clear the channel start bit
+	 * automatically at the end of a transfer. Therefore we must clear
+	 * it in software before starting the transfer.
+	 */
+	if ((chan->feature & FSL_DMA_IP_MASK) == FSL_DMA_IP_85XX) {
+		u32 mode;
+
+		mode = get_mr(chan);
+		mode &= ~FSL_DMA_MR_CS;
+		set_mr(chan, mode);
+	}
+
+	/*
+	 * Program the descriptor's address into the DMA controller,
+	 * then start the DMA transaction
+	 */
+	set_cdar(chan, desc->async_tx.phys);
+	get_cdar(chan);
+
+	dma_start(chan);
+	chan->idle = false;
+}
+
+/**
+ * fsldma_cleanup_descriptor - cleanup and free a single link descriptor
+ * @chan: Freescale DMA channel
+ * @desc: descriptor to cleanup and free
+ *
+ * This function is used on a descriptor which has been executed by the DMA
+ * controller. It will run any callbacks, submit any dependencies, and then
+ * free the descriptor.
+ */
+static void fsldma_cleanup_descriptor(struct fsldma_chan *chan,
+				      struct fsl_desc_sw *desc)
+{
+	struct dma_async_tx_descriptor *txd = &desc->async_tx;
+
+	/* Run the link descriptor callback function */
+	if (txd->callback) {
+		chan_dbg(chan, "LD %p callback\n", desc);
+		txd->callback(txd->callback_param);
+	}
+
+	/* Run any dependencies */
+	dma_run_dependencies(txd);
+
+	dma_descriptor_unmap(txd);
+	fsl_dma_free_descriptor(chan, desc);
+}
+
+/**
  * fsl_dma_alloc_chan_resources - Allocate resources for DMA channel.
  * @chan : Freescale DMA channel
  *
@@ -803,100 +897,6 @@ static int fsl_dma_device_control(struct dma_chan *dchan,
 }
 
 /**
- * fsldma_cleanup_descriptor - cleanup and free a single link descriptor
- * @chan: Freescale DMA channel
- * @desc: descriptor to cleanup and free
- *
- * This function is used on a descriptor which has been executed by the DMA
- * controller. It will run any callbacks, submit any dependencies, and then
- * free the descriptor.
- */
-static void fsldma_cleanup_descriptor(struct fsldma_chan *chan,
-				      struct fsl_desc_sw *desc)
-{
-	struct dma_async_tx_descriptor *txd = &desc->async_tx;
-
-	/* Run the link descriptor callback function */
-	if (txd->callback) {
-		chan_dbg(chan, "LD %p callback\n", desc);
-		txd->callback(txd->callback_param);
-	}
-
-	/* Run any dependencies */
-	dma_run_dependencies(txd);
-
-	dma_descriptor_unmap(txd);
-	fsl_dma_free_descriptor(chan, desc);
-}
-
-/**
- * fsl_chan_xfer_ld_queue - transfer any pending transactions
- * @chan : Freescale DMA channel
- *
- * HARDWARE STATE: idle
- * LOCKING: must hold chan->desc_lock
- */
-static void fsl_chan_xfer_ld_queue(struct fsldma_chan *chan)
-{
-	struct fsl_desc_sw *desc;
-
-	/*
-	 * If the list of pending descriptors is empty, then we
-	 * don't need to do any work at all
-	 */
-	if (list_empty(&chan->ld_pending)) {
-		chan_dbg(chan, "no pending LDs\n");
-		return;
-	}
-
-	/*
-	 * The DMA controller is not idle, which means that the interrupt
-	 * handler will start any queued transactions when it runs after
-	 * this transaction finishes
-	 */
-	if (!chan->idle) {
-		chan_dbg(chan, "DMA controller still busy\n");
-		return;
-	}
-
-	/*
-	 * If there are some link descriptors which have not been
-	 * transferred, we need to start the controller
-	 */
-
-	/*
-	 * Move all elements from the queue of pending transactions
-	 * onto the list of running transactions
-	 */
-	chan_dbg(chan, "idle, starting controller\n");
-	desc = list_first_entry(&chan->ld_pending, struct fsl_desc_sw, node);
-	list_splice_tail_init(&chan->ld_pending, &chan->ld_running);
-
-	/*
-	 * The 85xx DMA controller doesn't clear the channel start bit
-	 * automatically at the end of a transfer. Therefore we must clear
-	 * it in software before starting the transfer.
-	 */
-	if ((chan->feature & FSL_DMA_IP_MASK) == FSL_DMA_IP_85XX) {
-		u32 mode;
-
-		mode = get_mr(chan);
-		mode &= ~FSL_DMA_MR_CS;
-		set_mr(chan, mode);
-	}
-
-	/*
-	 * Program the descriptor's address into the DMA controller,
-	 * then start the DMA transaction
-	 */
-	set_cdar(chan, desc->async_tx.phys);
-	get_cdar(chan);
-
-	dma_start(chan);
-	chan->idle = false;
-}
-
-/**
  * fsl_dma_memcpy_issue_pending - Issue the DMA start command
  * @chan : Freescale DMA channel
  */
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v3 6/8] DMA: Freescale: change descriptor release process for supporting async_tx
From: hongbo.zhang @ 2014-04-10  7:10 UTC (permalink / raw)
  To: vkoul, dan.j.williams, dmaengine
  Cc: scottwood, Hongbo Zhang, linuxppc-dev, linux-kernel, leo.li
In-Reply-To: <1397113805-24171-1-git-send-email-hongbo.zhang@freescale.com>

From: Hongbo Zhang <hongbo.zhang@freescale.com>

Fix the potential risk when enable config NET_DMA and ASYNC_TX. Async_tx is
lack of support in current release process of dma descriptor, all descriptors
will be released whatever is acked or no-acked by async_tx, so there is a
potential race condition when dma engine is uesd by others clients (e.g. when
enable NET_DMA to offload TCP).

In our case, a race condition which is raised when use both of talitos and
dmaengine to offload xor is because napi scheduler will sync all pending
requests in dma channels, it affects the process of raid operations due to
ack_tx is not checked in fsl dma. The no-acked descriptor is freed which is
submitted just now, as a dependent tx, this freed descriptor trigger
BUG_ON(async_tx_test_ack(depend_tx)) in async_tx_submit().

TASK = ee1a94a0[1390] 'md0_raid5' THREAD: ecf40000 CPU: 0
GPR00: 00000001 ecf41ca0 ee44/921a94a0 0000003f 00000001 c00593e4 00000000 00000001
GPR08: 00000000 a7a7a7a7 00000001 045/920000002 42028042 100a38d4 ed576d98 00000000
GPR16: ed5a11b0 00000000 2b162000 00000200 046/920000000 2d555000 ed3015e8 c15a7aa0
GPR24: 00000000 c155fc40 00000000 ecb63220 ecf41d28 e47/92f640bb0 ef640c30 ecf41ca0
NIP [c02b048c] async_tx_submit+0x6c/0x2b4
LR [c02b068c] async_tx_submit+0x26c/0x2b4
Call Trace:
[ecf41ca0] [c02b068c] async_tx_submit+0x26c/0x2b448/92 (unreliable)
[ecf41cd0] [c02b0a4c] async_memcpy+0x240/0x25c
[ecf41d20] [c0421064] async_copy_data+0xa0/0x17c
[ecf41d70] [c0421cf4] __raid_run_ops+0x874/0xe10
[ecf41df0] [c0426ee4] handle_stripe+0x820/0x25e8
[ecf41e90] [c0429080] raid5d+0x3d4/0x5b4
[ecf41f40] [c04329b8] md_thread+0x138/0x16c
[ecf41f90] [c008277c] kthread+0x8c/0x90
[ecf41ff0] [c0011630] kernel_thread+0x4c/0x68

Another modification in this patch is the change of completed descriptors,
there is a potential risk which caused by exception interrupt, all descriptors
in ld_running list are seemed completed when an interrupt raised, it works fine
under normal condition, but if there is an exception occured, it cannot work as
our excepted. Hardware should not be depend on s/w list, the right way is to
read current descriptor address register to find the last completed descriptor.
If an interrupt is raised by an error, all descriptors in ld_running should not
be seemed finished, or these unfinished descriptors in ld_running will be
released wrongly.

A simple way to reproduce:
Enable dmatest first, then insert some bad descriptors which can trigger
Programming Error interrupts before the good descriptors. Last, the good
descriptors will be freed before they are processsed because of the exception
intrerrupt.

Note: the bad descriptors are only for simulating an exception interrupt.  This
case can illustrate the potential risk in current fsl-dma very well.

Signed-off-by: Hongbo Zhang <hongbo.zhang@freescale.com>
Signed-off-by: Qiang Liu <qiang.liu@freescale.com>
Signed-off-by: Ira W. Snyder <iws@ovro.caltech.edu>
---
 drivers/dma/fsldma.c |  195 ++++++++++++++++++++++++++++++++++++--------------
 drivers/dma/fsldma.h |   17 ++++-
 2 files changed, 158 insertions(+), 54 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 968877f..f8eee60 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -459,6 +459,87 @@ static struct fsl_desc_sw *fsl_dma_alloc_descriptor(struct fsldma_chan *chan)
 }
 
 /**
+ * fsldma_clean_completed_descriptor - free all descriptors which
+ * has been completed and acked
+ * @chan: Freescale DMA channel
+ *
+ * This function is used on all completed and acked descriptors.
+ * All descriptors should only be freed in this function.
+ */
+static void fsldma_clean_completed_descriptor(struct fsldma_chan *chan)
+{
+	struct fsl_desc_sw *desc, *_desc;
+
+	/* Run the callback for each descriptor, in order */
+	list_for_each_entry_safe(desc, _desc, &chan->ld_completed, node)
+		if (async_tx_test_ack(&desc->async_tx))
+			fsl_dma_free_descriptor(chan, desc);
+}
+
+/**
+ * fsldma_run_tx_complete_actions - cleanup a single link descriptor
+ * @chan: Freescale DMA channel
+ * @desc: descriptor to cleanup and free
+ * @cookie: Freescale DMA transaction identifier
+ *
+ * This function is used on a descriptor which has been executed by the DMA
+ * controller. It will run any callbacks, submit any dependencies.
+ */
+static dma_cookie_t fsldma_run_tx_complete_actions(struct fsldma_chan *chan,
+		struct fsl_desc_sw *desc, dma_cookie_t cookie)
+{
+	struct dma_async_tx_descriptor *txd = &desc->async_tx;
+
+	BUG_ON(txd->cookie < 0);
+
+	if (txd->cookie > 0) {
+		cookie = txd->cookie;
+
+		/* Run the link descriptor callback function */
+		if (txd->callback) {
+			chan_dbg(chan, "LD %p callback\n", desc);
+			txd->callback(txd->callback_param);
+		}
+	}
+
+	/* Run any dependencies */
+	dma_run_dependencies(txd);
+
+	return cookie;
+}
+
+/**
+ * fsldma_clean_running_descriptor - move the completed descriptor from
+ * ld_running to ld_completed
+ * @chan: Freescale DMA channel
+ * @desc: the descriptor which is completed
+ *
+ * Free the descriptor directly if acked by async_tx api, or move it to
+ * queue ld_completed.
+ */
+static void fsldma_clean_running_descriptor(struct fsldma_chan *chan,
+		struct fsl_desc_sw *desc)
+{
+	/* Remove from the list of transactions */
+	list_del(&desc->node);
+
+	/*
+	 * the client is allowed to attach dependent operations
+	 * until 'ack' is set
+	 */
+	if (!async_tx_test_ack(&desc->async_tx)) {
+		/*
+		 * Move this descriptor to the list of descriptors which is
+		 * completed, but still awaiting the 'ack' bit to be set.
+		 */
+		list_add_tail(&desc->node, &chan->ld_completed);
+		return;
+	}
+
+	dma_pool_free(chan->desc_pool, desc, desc->async_tx.phys);
+}
+
+/**
  * fsl_chan_xfer_ld_queue - transfer any pending transactions
  * @chan : Freescale DMA channel
  *
@@ -526,30 +607,58 @@ static void fsl_chan_xfer_ld_queue(struct fsldma_chan *chan)
 }
 
 /**
- * fsldma_cleanup_descriptor - cleanup and free a single link descriptor
+ * fsldma_cleanup_descriptors - cleanup link descriptors which are completed
+ * and move them to ld_completed to free until flag 'ack' is set
  * @chan: Freescale DMA channel
- * @desc: descriptor to cleanup and free
  *
- * This function is used on a descriptor which has been executed by the DMA
- * controller. It will run any callbacks, submit any dependencies, and then
- * free the descriptor.
+ * This function is used on descriptors which have been executed by the DMA
+ * controller. It will run any callbacks, submit any dependencies, then
+ * free these descriptors if flag 'ack' is set.
  */
-static void fsldma_cleanup_descriptor(struct fsldma_chan *chan,
-				      struct fsl_desc_sw *desc)
+static void fsldma_cleanup_descriptors(struct fsldma_chan *chan)
 {
-	struct dma_async_tx_descriptor *txd = &desc->async_tx;
+	struct fsl_desc_sw *desc, *_desc;
+	dma_cookie_t cookie = 0;
+	dma_addr_t curr_phys = get_cdar(chan);
+	int seen_current = 0;
+
+	fsldma_clean_completed_descriptor(chan);
+
+	/* Run the callback for each descriptor, in order */
+	list_for_each_entry_safe(desc, _desc, &chan->ld_running, node) {
+		/*
+		 * do not advance past the current descriptor loaded into the
+		 * hardware channel, subsequent descriptors are either in
+		 * process or have not been submitted
+		 */
+		if (seen_current)
+			break;
+
+		/*
+		 * stop the search if we reach the current descriptor and the
+		 * channel is busy
+		 */
+		if (desc->async_tx.phys == curr_phys) {
+			seen_current = 1;
+			if (!dma_is_idle(chan))
+				break;
+		}
+
+		cookie = fsldma_run_tx_complete_actions(chan, desc, cookie);
 
-	/* Run the link descriptor callback function */
-	if (txd->callback) {
-		chan_dbg(chan, "LD %p callback\n", desc);
-		txd->callback(txd->callback_param);
+		fsldma_clean_running_descriptor(chan, desc);
 	}
 
-	/* Run any dependencies */
-	dma_run_dependencies(txd);
+	/*
+	 * Start any pending transactions automatically
+	 *
+	 * In the ideal case, we keep the DMA controller busy while we go
+	 * ahead and free the descriptors below.
+	 */
+	fsl_chan_xfer_ld_queue(chan);
 
-	dma_descriptor_unmap(txd);
-	fsl_dma_free_descriptor(chan, desc);
+	if (cookie > 0)
+		chan->common.completed_cookie = cookie;
 }
 
 /**
@@ -620,8 +729,10 @@ static void fsl_dma_free_chan_resources(struct dma_chan *dchan)
 
 	chan_dbg(chan, "free all channel resources\n");
 	spin_lock_irqsave(&chan->desc_lock, flags);
+	fsldma_cleanup_descriptors(chan);
 	fsldma_free_desc_list(chan, &chan->ld_pending);
 	fsldma_free_desc_list(chan, &chan->ld_running);
+	fsldma_free_desc_list(chan, &chan->ld_completed);
 	spin_unlock_irqrestore(&chan->desc_lock, flags);
 
 	dma_pool_destroy(chan->desc_pool);
@@ -859,6 +970,7 @@ static int fsl_dma_device_control(struct dma_chan *dchan,
 		/* Remove and free all of the descriptors in the LD queue */
 		fsldma_free_desc_list(chan, &chan->ld_pending);
 		fsldma_free_desc_list(chan, &chan->ld_running);
+		fsldma_free_desc_list(chan, &chan->ld_completed);
 		chan->idle = true;
 
 		spin_unlock_irqrestore(&chan->desc_lock, flags);
@@ -918,6 +1030,17 @@ static enum dma_status fsl_tx_status(struct dma_chan *dchan,
 					dma_cookie_t cookie,
 					struct dma_tx_state *txstate)
 {
+	struct fsldma_chan *chan = to_fsl_chan(dchan);
+	enum dma_status ret;
+
+	ret = dma_cookie_status(dchan, cookie, txstate);
+	if (ret == DMA_COMPLETE)
+		return ret;
+
+	spin_lock_bh(&chan->desc_lock);
+	fsldma_cleanup_descriptors(chan);
+	spin_unlock_bh(&chan->desc_lock);
+
 	return dma_cookie_status(dchan, cookie, txstate);
 }
 
@@ -995,52 +1118,19 @@ static irqreturn_t fsldma_chan_irq(int irq, void *data)
 static void dma_do_tasklet(unsigned long data)
 {
 	struct fsldma_chan *chan = (struct fsldma_chan *)data;
-	struct fsl_desc_sw *desc, *_desc;
-	LIST_HEAD(ld_cleanup);
 	unsigned long flags;
 
 	chan_dbg(chan, "tasklet entry\n");
 
 	spin_lock_irqsave(&chan->desc_lock, flags);
 
-	/* update the cookie if we have some descriptors to cleanup */
-	if (!list_empty(&chan->ld_running)) {
-		dma_cookie_t cookie;
-
-		desc = to_fsl_desc(chan->ld_running.prev);
-		cookie = desc->async_tx.cookie;
-		dma_cookie_complete(&desc->async_tx);
-
-		chan_dbg(chan, "completed_cookie=%d\n", cookie);
-	}
-
-	/*
-	 * move the descriptors to a temporary list so we can drop the lock
-	 * during the entire cleanup operation
-	 */
-	list_splice_tail_init(&chan->ld_running, &ld_cleanup);
-
 	/* the hardware is now idle and ready for more */
 	chan->idle = true;
 
-	/*
-	 * Start any pending transactions automatically
-	 *
-	 * In the ideal case, we keep the DMA controller busy while we go
-	 * ahead and free the descriptors below.
-	 */
-	fsl_chan_xfer_ld_queue(chan);
-	spin_unlock_irqrestore(&chan->desc_lock, flags);
+	/* Run all cleanup for descriptors which have been completed */
+	fsldma_cleanup_descriptors(chan);
 
-	/* Run the callback for each descriptor, in order */
-	list_for_each_entry_safe(desc, _desc, &ld_cleanup, node) {
-
-		/* Remove from the list of transactions */
-		list_del(&desc->node);
-
-		/* Run all cleanup for this descriptor */
-		fsldma_cleanup_descriptor(chan, desc);
-	}
+	spin_unlock_irqrestore(&chan->desc_lock, flags);
 
 	chan_dbg(chan, "tasklet exit\n");
 }
@@ -1224,6 +1314,7 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
 	spin_lock_init(&chan->desc_lock);
 	INIT_LIST_HEAD(&chan->ld_pending);
 	INIT_LIST_HEAD(&chan->ld_running);
+	INIT_LIST_HEAD(&chan->ld_completed);
 	chan->idle = true;
 
 	chan->common.device = &fdev->common;
diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h
index d56e835..ec19517 100644
--- a/drivers/dma/fsldma.h
+++ b/drivers/dma/fsldma.h
@@ -138,8 +138,21 @@ struct fsldma_chan {
 	char name[8];			/* Channel name */
 	struct fsldma_chan_regs __iomem *regs;
 	spinlock_t desc_lock;		/* Descriptor operation lock */
-	struct list_head ld_pending;	/* Link descriptors queue */
-	struct list_head ld_running;	/* Link descriptors queue */
+	/*
+	 * Descriptors which are queued to run, but have not yet been
+	 * submitted to the hardware for execution
+	 */
+	struct list_head ld_pending;
+	/*
+	 * Descriptors which are currently being executed by the hardware
+	 */
+	struct list_head ld_running;
+	/*
+	 * Descriptors which have finished execution by the hardware. These
+	 * descriptors have already had their cleanup actions run. They are
+	 * waiting for the ACK bit to be set by the async_tx API.
+	 */
+	struct list_head ld_completed;	/* Link descriptors queue */
 	struct dma_chan common;		/* DMA common channel */
 	struct dma_pool *desc_pool;	/* Descriptors pool */
 	struct device *dev;		/* Channel device */
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v3 7/8] DMA: Freescale: use spin_lock_bh instead of spin_lock_irqsave
From: hongbo.zhang @ 2014-04-10  7:10 UTC (permalink / raw)
  To: vkoul, dan.j.williams, dmaengine
  Cc: scottwood, Hongbo Zhang, linuxppc-dev, linux-kernel, leo.li
In-Reply-To: <1397113805-24171-1-git-send-email-hongbo.zhang@freescale.com>

From: Hongbo Zhang <hongbo.zhang@freescale.com>

The usage of spin_lock_irqsave() is a stronger locking mechanism than is
required throughout the driver. The minimum locking required should be used
instead. Interrupts will be turned off and context will be saved, it is
unnecessary to use irqsave.

This patch changes all instances of spin_lock_irqsave() to spin_lock_bh(). All
manipulation of protected fields is done using tasklet context or weaker, which
makes spin_lock_bh() the correct choice.

Signed-off-by: Hongbo Zhang <hongbo.zhang@freescale.com>
Signed-off-by: Qiang Liu <qiang.liu@freescale.com>
---
 drivers/dma/fsldma.c |   25 ++++++++++---------------
 1 file changed, 10 insertions(+), 15 deletions(-)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index f8eee60..c9bf54a 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -396,10 +396,9 @@ static dma_cookie_t fsl_dma_tx_submit(struct dma_async_tx_descriptor *tx)
 	struct fsldma_chan *chan = to_fsl_chan(tx->chan);
 	struct fsl_desc_sw *desc = tx_to_fsl_desc(tx);
 	struct fsl_desc_sw *child;
-	unsigned long flags;
 	dma_cookie_t cookie = -EINVAL;
 
-	spin_lock_irqsave(&chan->desc_lock, flags);
+	spin_lock_bh(&chan->desc_lock);
 
 	/*
 	 * assign cookies to all of the software descriptors
@@ -412,7 +411,7 @@ static dma_cookie_t fsl_dma_tx_submit(struct dma_async_tx_descriptor *tx)
 	/* put this transaction onto the tail of the pending queue */
 	append_ld_queue(chan, desc);
 
-	spin_unlock_irqrestore(&chan->desc_lock, flags);
+	spin_unlock_bh(&chan->desc_lock);
 
 	return cookie;
 }
@@ -725,15 +724,14 @@ static void fsldma_free_desc_list_reverse(struct fsldma_chan *chan,
 static void fsl_dma_free_chan_resources(struct dma_chan *dchan)
 {
 	struct fsldma_chan *chan = to_fsl_chan(dchan);
-	unsigned long flags;
 
 	chan_dbg(chan, "free all channel resources\n");
-	spin_lock_irqsave(&chan->desc_lock, flags);
+	spin_lock_bh(&chan->desc_lock);
 	fsldma_cleanup_descriptors(chan);
 	fsldma_free_desc_list(chan, &chan->ld_pending);
 	fsldma_free_desc_list(chan, &chan->ld_running);
 	fsldma_free_desc_list(chan, &chan->ld_completed);
-	spin_unlock_irqrestore(&chan->desc_lock, flags);
+	spin_unlock_bh(&chan->desc_lock);
 
 	dma_pool_destroy(chan->desc_pool);
 	chan->desc_pool = NULL;
@@ -952,7 +950,6 @@ static int fsl_dma_device_control(struct dma_chan *dchan,
 {
 	struct dma_slave_config *config;
 	struct fsldma_chan *chan;
-	unsigned long flags;
 	int size;
 
 	if (!dchan)
@@ -962,7 +959,7 @@ static int fsl_dma_device_control(struct dma_chan *dchan,
 
 	switch (cmd) {
 	case DMA_TERMINATE_ALL:
-		spin_lock_irqsave(&chan->desc_lock, flags);
+		spin_lock_bh(&chan->desc_lock);
 
 		/* Halt the DMA engine */
 		dma_halt(chan);
@@ -973,7 +970,7 @@ static int fsl_dma_device_control(struct dma_chan *dchan,
 		fsldma_free_desc_list(chan, &chan->ld_completed);
 		chan->idle = true;
 
-		spin_unlock_irqrestore(&chan->desc_lock, flags);
+		spin_unlock_bh(&chan->desc_lock);
 		return 0;
 
 	case DMA_SLAVE_CONFIG:
@@ -1015,11 +1012,10 @@ static int fsl_dma_device_control(struct dma_chan *dchan,
 static void fsl_dma_memcpy_issue_pending(struct dma_chan *dchan)
 {
 	struct fsldma_chan *chan = to_fsl_chan(dchan);
-	unsigned long flags;
 
-	spin_lock_irqsave(&chan->desc_lock, flags);
+	spin_lock_bh(&chan->desc_lock);
 	fsl_chan_xfer_ld_queue(chan);
-	spin_unlock_irqrestore(&chan->desc_lock, flags);
+	spin_unlock_bh(&chan->desc_lock);
 }
 
 /**
@@ -1118,11 +1114,10 @@ static irqreturn_t fsldma_chan_irq(int irq, void *data)
 static void dma_do_tasklet(unsigned long data)
 {
 	struct fsldma_chan *chan = (struct fsldma_chan *)data;
-	unsigned long flags;
 
 	chan_dbg(chan, "tasklet entry\n");
 
-	spin_lock_irqsave(&chan->desc_lock, flags);
+	spin_lock_bh(&chan->desc_lock);
 
 	/* the hardware is now idle and ready for more */
 	chan->idle = true;
@@ -1130,7 +1125,7 @@ static void dma_do_tasklet(unsigned long data)
 	/* Run all cleanup for descriptors which have been completed */
 	fsldma_cleanup_descriptors(chan);
 
-	spin_unlock_irqrestore(&chan->desc_lock, flags);
+	spin_unlock_bh(&chan->desc_lock);
 
 	chan_dbg(chan, "tasklet exit\n");
 }
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v3 8/8] DMA: Freescale: add suspend resume functions for DMA driver
From: hongbo.zhang @ 2014-04-10  7:10 UTC (permalink / raw)
  To: vkoul, dan.j.williams, dmaengine
  Cc: scottwood, Hongbo Zhang, linuxppc-dev, linux-kernel, leo.li
In-Reply-To: <1397113805-24171-1-git-send-email-hongbo.zhang@freescale.com>

From: Hongbo Zhang <hongbo.zhang@freescale.com>

This patch adds suspend resume functions for Freescale DMA driver.
.prepare callback is used to stop further descriptors from being added into the
pending queue, and also issue pending queues into execution if there is any.
.suspend callback makes sure all the pending jobs are cleaned up and all the
channels are idle, and save the mode registers.
.resume callback re-initializes the channels by restore the mode registers.

Signed-off-by: Hongbo Zhang <hongbo.zhang@freescale.com>
---
 drivers/dma/fsldma.c |  100 ++++++++++++++++++++++++++++++++++++++++++++++++++
 drivers/dma/fsldma.h |   16 ++++++++
 2 files changed, 116 insertions(+)

diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index c9bf54a..d6da222 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -400,6 +400,14 @@ static dma_cookie_t fsl_dma_tx_submit(struct dma_async_tx_descriptor *tx)
 
 	spin_lock_bh(&chan->desc_lock);
 
+#ifdef CONFIG_PM
+	if (unlikely(chan->pm_state != RUNNING)) {
+		chan_dbg(chan, "cannot submit due to suspend\n");
+		spin_unlock_bh(&chan->desc_lock);
+		return -1;
+	}
+#endif
+
 	/*
 	 * assign cookies to all of the software descriptors
 	 * that make up this transaction
@@ -1311,6 +1319,9 @@ static int fsl_dma_chan_probe(struct fsldma_device *fdev,
 	INIT_LIST_HEAD(&chan->ld_running);
 	INIT_LIST_HEAD(&chan->ld_completed);
 	chan->idle = true;
+#ifdef CONFIG_PM
+	chan->pm_state = RUNNING;
+#endif
 
 	chan->common.device = &fdev->common;
 	dma_cookie_init(&chan->common);
@@ -1450,6 +1461,92 @@ static int fsldma_of_remove(struct platform_device *op)
 	return 0;
 }
 
+#ifdef CONFIG_PM
+static int fsldma_prepare(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct fsldma_device *fdev = platform_get_drvdata(pdev);
+	struct fsldma_chan *chan;
+	int i;
+
+	for (i = 0; i < FSL_DMA_MAX_CHANS_PER_DEVICE; i++) {
+		chan = fdev->chan[i];
+		if (!chan)
+			continue;
+
+		spin_lock_bh(&chan->desc_lock);
+		chan->pm_state = SUSPENDING;
+		if (!list_empty(&chan->ld_pending))
+			fsl_chan_xfer_ld_queue(chan);
+		spin_unlock_bh(&chan->desc_lock);
+	}
+
+	return 0;
+}
+
+static int fsldma_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct fsldma_device *fdev = platform_get_drvdata(pdev);
+	struct fsldma_chan *chan;
+	int i;
+
+	for (i = 0; i < FSL_DMA_MAX_CHANS_PER_DEVICE; i++) {
+		chan = fdev->chan[i];
+		if (!chan)
+			continue;
+
+		spin_lock_bh(&chan->desc_lock);
+		if (!chan->idle)
+			goto out;
+		chan->regs_save.mr = DMA_IN(chan, &chan->regs->mr, 32);
+		chan->pm_state = SUSPENDED;
+		spin_unlock_bh(&chan->desc_lock);
+	}
+	return 0;
+
+out:
+	for (; i >= 0; i--) {
+		chan = fdev->chan[i];
+		if (!chan)
+			continue;
+		chan->pm_state = RUNNING;
+		spin_unlock_bh(&chan->desc_lock);
+	}
+	return -EBUSY;
+}
+
+static int fsldma_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct fsldma_device *fdev = platform_get_drvdata(pdev);
+	struct fsldma_chan *chan;
+	u32 mode;
+	int i;
+
+	for (i = 0; i < FSL_DMA_MAX_CHANS_PER_DEVICE; i++) {
+		chan = fdev->chan[i];
+		if (!chan)
+			continue;
+
+		spin_lock_bh(&chan->desc_lock);
+		mode = chan->regs_save.mr
+			& ~FSL_DMA_MR_CS & ~FSL_DMA_MR_CC & ~FSL_DMA_MR_CA;
+		DMA_OUT(chan, &chan->regs->mr, mode, 32);
+		chan->pm_state = RUNNING;
+		spin_unlock_bh(&chan->desc_lock);
+	}
+
+	return 0;
+}
+
+static const struct dev_pm_ops fsldma_pm_ops = {
+	.prepare	= fsldma_prepare,
+	.suspend	= fsldma_suspend,
+	.resume		= fsldma_resume,
+};
+#endif
+
 static const struct of_device_id fsldma_of_ids[] = {
 	{ .compatible = "fsl,elo3-dma", },
 	{ .compatible = "fsl,eloplus-dma", },
@@ -1462,6 +1559,9 @@ static struct platform_driver fsldma_of_driver = {
 		.name = "fsl-elo-dma",
 		.owner = THIS_MODULE,
 		.of_match_table = fsldma_of_ids,
+#ifdef CONFIG_PM
+		.pm = &fsldma_pm_ops,
+#endif
 	},
 	.probe = fsldma_of_probe,
 	.remove = fsldma_of_remove,
diff --git a/drivers/dma/fsldma.h b/drivers/dma/fsldma.h
index ec19517..eecaf9e 100644
--- a/drivers/dma/fsldma.h
+++ b/drivers/dma/fsldma.h
@@ -134,6 +134,18 @@ struct fsldma_device {
 #define FSL_DMA_CHAN_PAUSE_EXT	0x00001000
 #define FSL_DMA_CHAN_START_EXT	0x00002000
 
+#ifdef CONFIG_PM
+struct fsldma_chan_regs_save {
+	u32 mr;
+};
+
+enum fsldma_pm_state {
+	RUNNING = 0,
+	SUSPENDING,
+	SUSPENDED,
+};
+#endif
+
 struct fsldma_chan {
 	char name[8];			/* Channel name */
 	struct fsldma_chan_regs __iomem *regs;
@@ -161,6 +173,10 @@ struct fsldma_chan {
 	struct tasklet_struct tasklet;
 	u32 feature;
 	bool idle;			/* DMA controller is idle */
+#ifdef CONFIG_PM
+	struct fsldma_chan_regs_save regs_save;
+	enum fsldma_pm_state pm_state;
+#endif
 
 	void (*toggle_ext_pause)(struct fsldma_chan *fsl_chan, int enable);
 	void (*toggle_ext_start)(struct fsldma_chan *fsl_chan, int enable);
-- 
1.7.9.5

^ permalink raw reply related

* [PATCH v3 0/8] DMA: Freescale: driver cleanups and enhancements
From: Hongbo Zhang @ 2014-04-10  7:20 UTC (permalink / raw)
  To: vkoul, dan.j.williams, dmaengine
  Cc: scottwood, linuxppc-dev, linux-kernel, leo.li
In-Reply-To: <1397113805-24171-2-git-send-email-hongbo.zhang@freescale.com>

Sorry, forgot the cover letter, plus it here.


From: Hongbo Zhang <hongbo.zhang@freescale.com>
Date: Thu, 10 Apr 2014 15:16:31 +0800
Subject: [PATCH v3 0/8] DMA: Freescale: driver cleanups and enhancements

Hi Vinod Koul,
Please have a look at the v3 patch set.

v2 -> v3 change:
Only add "chan->pm_state = RUNNING" for patch[8/8].

v1 -> v2 change:
The only one change is introducing a new patch[1/7] to remove the 
unnecessary
macro FSL_DMA_LD_DEBUG, thus the total patches number is 8 now (was 7)

Hongbo Zhang (8):
   DMA: Freescale: remove the unnecessary FSL_DMA_LD_DEBUG
   DMA: Freescale: unify register access methods
   DMA: Freescale: remove attribute DMA_INTERRUPT of dmaengine
   DMA: Freescale: add fsl_dma_free_descriptor() to reduce code
     duplication
   DMA: Freescale: move functions to avoid forward declarations
   DMA: Freescale: change descriptor release process for supporting
     async_tx
   DMA: Freescale: use spin_lock_bh instead of spin_lock_irqsave
   DMA: Freescale: add suspend resume functions for DMA driver

  drivers/dma/fsldma.c |  591 
++++++++++++++++++++++++++++++++------------------
  drivers/dma/fsldma.h |   33 ++-
  2 files changed, 409 insertions(+), 215 deletions(-)

^ permalink raw reply

* RE: [PATCH v4] ASoC: fsl_sai: Add clock controls for SAI
From: Li.Xiubo @ 2014-04-10  7:39 UTC (permalink / raw)
  To: guangyu.chen@freescale.com, broonie@kernel.org
  Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
	alsa-devel@alsa-project.org, pawel.moll@arm.com,
	linux-doc@vger.kernel.org, ijc+devicetree@hellion.org.uk,
	linux-kernel@vger.kernel.org, timur@tabi.org, robh+dt@kernel.org,
	rob@landley.net, galak@codeaurora.org, shawn.guo@linaro.org,
	linuxppc-dev@lists.ozlabs.org
In-Reply-To: <1397041577-9580-1-git-send-email-Guangyu.Chen@freescale.com>


> Subject: [PATCH v4] ASoC: fsl_sai: Add clock controls for SAI
>=20
> The SAI mainly has the following clocks:
>   bus clock
>     control and configure registers and to generate synchronous
>     interrupts and DMA requests.
>=20
>   mclk1, mclk2, mclk3
>     to generate the bit clock when the receiver or transmitter is
>     configured for an internally generated bit clock.
>=20
> So this patch adds these clocks and their clock controls to the driver,
> meanwhile, corrects the existing DTS accordingly so those platforms can
> benifit from the further feature with different clock sources.
>=20
> Signed-off-by: Nicolin Chen <Guangyu.Chen@freescale.com>
> CC: Xiubo Li <Li.Xiubo@freescale.com>
> Acked-by: Shawn Guo <shawn.guo@linaro.org>
> ---
>=20
> @Xiubo
> Even though you've tested it before, I'd still like to wait for your test
> result to this newer version.
>=20
> Changelog
> v4:
>  * Merged into single patch.
>  * Fixed bus clock ID on vf610.
> v3:
>  * Use int type for ret instead of u32.
>  * Added Acked-by and Tested-by from Xiubo Li.
> v2:
>  * Appended two extra mclks to the driver since SAI actually has three.
>  * Renamed clock name to 'bus' and 'mclk' according to the reference manu=
al.
>=20
>  .../devicetree/bindings/sound/fsl-sai.txt          |  9 ++--
>  arch/arm/boot/dts/vf610.dtsi                       |  6 ++-
>  sound/soc/fsl/fsl_sai.c                            | 51 ++++++++++++++++=
++++-
> -
>  sound/soc/fsl/fsl_sai.h                            |  4 ++
>  4 files changed, 61 insertions(+), 9 deletions(-)
>=20
> diff --git a/Documentation/devicetree/bindings/sound/fsl-sai.txt
> b/Documentation/devicetree/bindings/sound/fsl-sai.txt
> index 35c09fe..0f4e238 100644
> --- a/Documentation/devicetree/bindings/sound/fsl-sai.txt
> +++ b/Documentation/devicetree/bindings/sound/fsl-sai.txt
> @@ -10,7 +10,8 @@ Required properties:
>  - compatible: Compatible list, contains "fsl,vf610-sai" or "fsl,imx6sx-s=
ai".
>  - reg: Offset and length of the register set for the device.
>  - clocks: Must contain an entry for each entry in clock-names.
> -- clock-names : Must include the "sai" entry.
> +- clock-names : Must include the "bus" for register access and "mclk1"
> "mclk2"
> +  "mclk3" for bit clock and frame clock providing.
>  - dmas : Generic dma devicetree binding as described in
>    Documentation/devicetree/bindings/dma/dma.txt.
>  - dma-names : Two dmas have to be defined, "tx" and "rx".
> @@ -30,8 +31,10 @@ sai2: sai@40031000 {
>  	      reg =3D <0x40031000 0x1000>;
>  	      pinctrl-names =3D "default";
>  	      pinctrl-0 =3D <&pinctrl_sai2_1>;
> -	      clocks =3D <&clks VF610_CLK_SAI2>;
> -	      clock-names =3D "sai";
> +	      clocks =3D <&clks VF610_CLK_PLATFORM_BUS>,
> +		     <&clks VF610_CLK_SAI2>,
> +		     <&clks 0>, <&clks 0>;
> +	      clock-names =3D "bus", "mclk1", "mclk2", "mclk3";
>  	      dma-names =3D "tx", "rx";
>  	      dmas =3D <&edma0 0 VF610_EDMA_MUXID0_SAI2_TX>,
>  		   <&edma0 0 VF610_EDMA_MUXID0_SAI2_RX>;
> diff --git a/arch/arm/boot/dts/vf610.dtsi b/arch/arm/boot/dts/vf610.dtsi
> index d31ce1b..4c3cd59 100644
> --- a/arch/arm/boot/dts/vf610.dtsi
> +++ b/arch/arm/boot/dts/vf610.dtsi
> @@ -139,8 +139,10 @@
>  				compatible =3D "fsl,vf610-sai";
>  				reg =3D <0x40031000 0x1000>;
>  				interrupts =3D <0 86 0x04>;
> -				clocks =3D <&clks VF610_CLK_SAI2>;
> -				clock-names =3D "sai";
> +				clocks =3D <&clks VF610_CLK_PLATFORM_BUS>,
> +				       <&clks VF610_CLK_SAI2>,
> +				       <&clks 0>, <&clks 0>;
> +				clock-names =3D "bus", "mclk1", "mclk2", "mclk3";

Yes, this okey for Vybrid.

On Vybrid, only the 'bus' and 'mclk1' are present, and the 'bus' clock is
Enable by default.


>  				status =3D "disabled";
>  			};
>=20
> diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> index db9f75e..7cd4af9 100644
> --- a/sound/soc/fsl/fsl_sai.c
> +++ b/sound/soc/fsl/fsl_sai.c
> @@ -401,7 +401,23 @@ static int fsl_sai_startup(struct snd_pcm_substream
> *substream,
>  		struct snd_soc_dai *cpu_dai)
>  {
>  	struct fsl_sai *sai =3D snd_soc_dai_get_drvdata(cpu_dai);
> -	u32 reg;
> +	struct device *dev =3D &sai->pdev->dev;
> +	u32 reg, i;
> +	int ret;
> +
> +	ret =3D clk_prepare_enable(sai->bus_clk);
> +	if (ret) {
> +		dev_err(dev, "failed to enable bus clock\n");
> +		return ret;
> +	}
> +
> +	for (i =3D 0; i < FSL_SAI_MCLK_MAX; i++) {
> +		ret =3D clk_prepare_enable(sai->mclk_clk[i]);
> +		if (ret) {
> +			dev_err(dev, "failed to enable mclk%d clock\n", i + 1);
> +			goto err;
> +		}
> +	}
>=20

Why prepare and enable all the mclks here ?
And at last only one of 'bus', 'mclk1', 'mclk2' and 'mclk3' will be selecte=
d
To generate the bit clock. How about just prepare and enable the selected
one ?




>  	if (substream->stream =3D=3D SNDRV_PCM_STREAM_PLAYBACK)
>  		reg =3D FSL_SAI_TCR3;
> @@ -412,13 +428,20 @@ static int fsl_sai_startup(struct snd_pcm_substream
> *substream,
>  			   FSL_SAI_CR3_TRCE);
>=20
>  	return 0;
> +
> +err:
> +	for (; i > 0; i--)
> +		clk_disable_unprepare(sai->mclk_clk[i - 1]);
> +	clk_disable_unprepare(sai->bus_clk);
> +
> +	return ret;
>  }
>=20
>  static void fsl_sai_shutdown(struct snd_pcm_substream *substream,
>  		struct snd_soc_dai *cpu_dai)
>  {
>  	struct fsl_sai *sai =3D snd_soc_dai_get_drvdata(cpu_dai);
> -	u32 reg;
> +	u32 reg, i;
>=20
>  	if (substream->stream =3D=3D SNDRV_PCM_STREAM_PLAYBACK)
>  		reg =3D FSL_SAI_TCR3;
> @@ -427,6 +450,10 @@ static void fsl_sai_shutdown(struct snd_pcm_substrea=
m
> *substream,
>=20
>  	regmap_update_bits(sai->regmap, reg, FSL_SAI_CR3_TRCE,
>  			   ~FSL_SAI_CR3_TRCE);
> +
> +	for (i =3D 0; i < FSL_SAI_MCLK_MAX; i++)
> +		clk_disable_unprepare(sai->mclk_clk[i]);
> +	clk_disable_unprepare(sai->bus_clk);
>  }
>=20
>  static const struct snd_soc_dai_ops fsl_sai_pcm_dai_ops =3D {
> @@ -559,7 +586,8 @@ static int fsl_sai_probe(struct platform_device *pdev=
)
>  	struct fsl_sai *sai;
>  	struct resource *res;
>  	void __iomem *base;
> -	int irq, ret;
> +	char tmp[8];
> +	int irq, ret, i;
>=20
>  	sai =3D devm_kzalloc(&pdev->dev, sizeof(*sai), GFP_KERNEL);
>  	if (!sai)
> @@ -582,12 +610,27 @@ static int fsl_sai_probe(struct platform_device *pd=
ev)
>  		return PTR_ERR(base);
>=20
>  	sai->regmap =3D devm_regmap_init_mmio_clk(&pdev->dev,
> -			"sai", base, &fsl_sai_regmap_config);
> +			"bus", base, &fsl_sai_regmap_config);
>  	if (IS_ERR(sai->regmap)) {
>  		dev_err(&pdev->dev, "regmap init failed\n");
>  		return PTR_ERR(sai->regmap);
>  	}
>=20
> +	sai->bus_clk =3D devm_clk_get(&pdev->dev, "bus");
> +	if (IS_ERR(sai->bus_clk)) {
> +		dev_err(&pdev->dev, "failed to get bus clock\n");
> +		return PTR_ERR(sai->bus_clk);
> +	}
> +
> +	for (i =3D 0; i < FSL_SAI_MCLK_MAX; i++) {
> +		sprintf(tmp, "mclk%d", i + 1);
> +		sai->mclk_clk[i] =3D devm_clk_get(&pdev->dev, tmp);
> +		if (IS_ERR(sai->mclk_clk[i])) {
> +			dev_err(&pdev->dev, "failed to get mclk%d clock\n", i + 1);
> +			return PTR_ERR(sai->mclk_clk[i]);
> +		}
> +	}
> +
>  	irq =3D platform_get_irq(pdev, 0);
>  	if (irq < 0) {
>  		dev_err(&pdev->dev, "no irq for node %s\n", np->full_name);
> diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
> index 677670d..0e6c9f5 100644
> --- a/sound/soc/fsl/fsl_sai.h
> +++ b/sound/soc/fsl/fsl_sai.h
> @@ -119,6 +119,8 @@
>  #define FSL_SAI_CLK_MAST2	2
>  #define FSL_SAI_CLK_MAST3	3
>=20
> +#define FSL_SAI_MCLK_MAX	3
> +
>  /* SAI data transfer numbers per DMA request */
>  #define FSL_SAI_MAXBURST_TX 6
>  #define FSL_SAI_MAXBURST_RX 6
> @@ -126,6 +128,8 @@
>  struct fsl_sai {
>  	struct platform_device *pdev;
>  	struct regmap *regmap;
> +	struct clk *bus_clk;
> +	struct clk *mclk_clk[FSL_SAI_MCLK_MAX];
>=20
>  	bool big_endian_regs;
>  	bool big_endian_data;
> --
> 1.8.4
>=20

^ permalink raw reply

* Re: [PATCH v4] ASoC: fsl_sai: Add clock controls for SAI
From: Nicolin Chen @ 2014-04-10  7:46 UTC (permalink / raw)
  To: Xiubo Li-B47053
  Cc: mark.rutland@arm.com, devicetree@vger.kernel.org,
	alsa-devel@alsa-project.org, pawel.moll@arm.com,
	linux-doc@vger.kernel.org, ijc+devicetree@hellion.org.uk,
	linux-kernel@vger.kernel.org, robh+dt@kernel.org, timur@tabi.org,
	broonie@kernel.org, rob@landley.net, galak@codeaurora.org,
	shawn.guo@linaro.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <fe26bf436e1846fca85415fc4c8bb67e@BY2PR03MB505.namprd03.prod.outlook.com>

On Thu, Apr 10, 2014 at 03:39:51PM +0800, Xiubo Li-B47053 wrote:
> > diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
> > index db9f75e..7cd4af9 100644
> > --- a/sound/soc/fsl/fsl_sai.c
> > +++ b/sound/soc/fsl/fsl_sai.c
> > @@ -401,7 +401,23 @@ static int fsl_sai_startup(struct snd_pcm_substream
> > *substream,
> >  		struct snd_soc_dai *cpu_dai)
> >  {
> >  	struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
> > -	u32 reg;
> > +	struct device *dev = &sai->pdev->dev;
> > +	u32 reg, i;
> > +	int ret;
> > +
> > +	ret = clk_prepare_enable(sai->bus_clk);
> > +	if (ret) {
> > +		dev_err(dev, "failed to enable bus clock\n");
> > +		return ret;
> > +	}
> > +
> > +	for (i = 0; i < FSL_SAI_MCLK_MAX; i++) {
> > +		ret = clk_prepare_enable(sai->mclk_clk[i]);
> > +		if (ret) {
> > +			dev_err(dev, "failed to enable mclk%d clock\n", i + 1);
> > +			goto err;
> > +		}
> > +	}
> > 
> 
> Why prepare and enable all the mclks here ?
> And at last only one of 'bus', 'mclk1', 'mclk2' and 'mclk3' will be selected
> To generate the bit clock. How about just prepare and enable the selected
> one ?
 
That's a fair suggestion. I'll do the revise.

But in this way. We can provisionally drop the clock enabling part and add
them later after my clock dividing patch is ready.

Thank you,
Nicolin

^ permalink raw reply

* [RFC PATCH v2 powerpc] Protect remove_memory() with device hotplug lock
From: Li Zhong @ 2014-04-10  8:25 UTC (permalink / raw)
  To: Nathan Fontenot; +Cc: PowerPC email list, Paul Mackerras
In-Reply-To: <534571D0.2030700@linux.vnet.ibm.com>

While testing memory hot-remove, I found following dead lock:

Process #1141 is drmgr, trying to remove some memory, i.e. memory499. 
It holds the memory_hotplug_mutex, and blocks when trying to remove file
"online" under dir memory499, in kernfs_drain(), at 
        wait_event(root->deactivate_waitq,
                   atomic_read(&kn->active) == KN_DEACTIVATED_BIAS);

Process #1120 is trying to online memory499 by 
   echo 1 > memory499/online

In .kernfs_fop_write, it uses kernfs_get_active() to increase
&kn->active, thus blocking process #1141. While itself is blocked later
when trying to acquire memory_hotplug_mutex, which is held by process
#1141. 

The backtrace of both processes are shown below:

# cat /proc/1120/stack 
[<c000000001b18600>] 0xc000000001b18600
[<c000000000015044>] .__switch_to+0x144/0x200
[<c000000000263ca4>] .online_pages+0x74/0x7b0
[<c00000000055b40c>] .memory_subsys_online+0x9c/0x150
[<c00000000053cbe8>] .device_online+0xb8/0x120
[<c00000000053cd04>] .online_store+0xb4/0xc0
[<c000000000538ce4>] .dev_attr_store+0x64/0xa0
[<c00000000030f4ec>] .sysfs_kf_write+0x7c/0xb0
[<c00000000030e574>] .kernfs_fop_write+0x154/0x1e0
[<c000000000268450>] .vfs_write+0xe0/0x260
[<c000000000269144>] .SyS_write+0x64/0x110
[<c000000000009ffc>] syscall_exit+0x0/0x7c


# cat /proc/1141/stack 
[<c000000001b18600>] 0xc000000001b18600
[<c000000000015044>] .__switch_to+0x144/0x200
[<c00000000030be14>] .__kernfs_remove+0x204/0x300
[<c00000000030d428>] .kernfs_remove_by_name_ns+0x68/0xf0
[<c00000000030fb38>] .sysfs_remove_file_ns+0x38/0x60
[<c000000000539354>] .device_remove_attrs+0x54/0xc0
[<c000000000539fd8>] .device_del+0x158/0x250
[<c00000000053a104>] .device_unregister+0x34/0xa0
[<c00000000055bc14>] .unregister_memory_section+0x164/0x170
[<c00000000024ee18>] .__remove_pages+0x108/0x4c0
[<c00000000004b590>] .arch_remove_memory+0x60/0xc0
[<c00000000026446c>] .remove_memory+0x8c/0xe0
[<c00000000007f9f4>] .pseries_remove_memblock+0xd4/0x160
[<c00000000007fcfc>] .pseries_memory_notifier+0x27c/0x290
[<c0000000008ae6cc>] .notifier_call_chain+0x8c/0x100
[<c0000000000d858c>] .__blocking_notifier_call_chain+0x6c/0xe0
[<c00000000071ddec>] .of_property_notify+0x7c/0xc0
[<c00000000071ed3c>] .of_update_property+0x3c/0x1b0
[<c0000000000756cc>] .ofdt_write+0x3dc/0x740
[<c0000000002f60fc>] .proc_reg_write+0xac/0x110
[<c000000000268450>] .vfs_write+0xe0/0x260
[<c000000000269144>] .SyS_write+0x64/0x110
[<c000000000009ffc>] syscall_exit+0x0/0x7c

This patch uses lock_device_hotplug() to protect remove_memory() called
in pseries_remove_memblock(), which is also stated before function
remove_memory():

 * NOTE: The caller must call lock_device_hotplug() to serialize hotplug
 * and online/offline operations before this call, as required by
 * try_offline_node().
 */     
void __ref remove_memory(int nid, u64 start, u64 size)

With this lock held, the other process(#1120 above) trying to online the
memory block will retry the system call when calling
lock_device_hotplug_sysfs(), and finally find No such device error. 

Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/pseries/hotplug-memory.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index 573b488..7f75c94 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -100,10 +100,10 @@ static int pseries_remove_memblock(unsigned long base, unsigned int memblock_siz
 
 	start_pfn = base >> PAGE_SHIFT;
 
-	if (!pfn_valid(start_pfn)) {
-		memblock_remove(base, memblock_size);
-		return 0;
-	}
+	lock_device_hotplug();
+
+	if (!pfn_valid(start_pfn))
+		goto out;
 
 	block_sz = memory_block_size_bytes();
 	sections_per_block = block_sz / MIN_MEMORY_BLOCK_SIZE;
@@ -114,8 +114,10 @@ static int pseries_remove_memblock(unsigned long base, unsigned int memblock_siz
 		base += MIN_MEMORY_BLOCK_SIZE;
 	}
 
+out:
 	/* Update memory regions for memory remove */
 	memblock_remove(base, memblock_size);
+	unlock_device_hotplug();
 	return 0;
 }
 

^ permalink raw reply related

* Re: [PATCH V2 1/2] mm: move FAULT_AROUND_ORDER to arch/
From: Madhavan Srinivasan @ 2014-04-10  8:29 UTC (permalink / raw)
  To: Dave Hansen, Peter Zijlstra
  Cc: linux-arch, riel, rusty, x86, linux-kernel, linux-mm, ak, paulus,
	mgorman, akpm, linuxppc-dev, mingo, kirill.shutemov
In-Reply-To: <53456BE2.90905@intel.com>

On Wednesday 09 April 2014 09:18 PM, Dave Hansen wrote:
> On 04/09/2014 01:20 AM, Peter Zijlstra wrote:
>> This still misses out on Ben's objection that its impossible to get this
>> right at compile time for many kernels, since they can boot and run on
>> many different subarchs.
> 
> Completely agree.  The Kconfig-time stuff should probably just be a knob
> to turn it off completely, if anything.
> 

ok. Here is my thought. So to address Ben's concern, it would be better
to have this as a variable with a default value (and the platform can
override ride it). And a mm/Kconfig to disable it?
Kindly let me know whether this will work.

Thanks for review comments.
With regards
Maddy

^ permalink raw reply

* RE: [PATCH v3 2/8] DMA: Freescale: unify register access methods
From: David Laight @ 2014-04-10  8:46 UTC (permalink / raw)
  To: 'hongbo.zhang@freescale.com', vkoul@infradead.org,
	dan.j.williams@intel.com, dmaengine@vger.kernel.org
  Cc: scottwood@freescale.com, linuxppc-dev@lists.ozlabs.org,
	linux-kernel@vger.kernel.org, leo.li@freescale.com
In-Reply-To: <1397113805-24171-3-git-send-email-hongbo.zhang@freescale.com>

RnJvbTogaG9uZ2JvLnpoYW5nQGZyZWVzY2FsZS5jb20NCj4gTWV0aG9kcyBvZiBhY2Nlc3Npbmcg
RE1BIGNvbnRvcmxsZXIgcmVnaXN0ZXJzIGFyZSBpbmNvbnNpc3RlbnQsIHNvbWUgcmVnaXN0ZXJz
DQogICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgXl4NCj4gYXJlIGFjY2Vzc2VkIGJ5IERN
QV9JTi9PVVQgZGlyZWN0bHksIHdoaWxlIG90aGVycyBhcmUgYWNjZXNzZWQgYnkgZnVuY3Rpb25z
DQo+IGdldC9zZXRfKiB3aGljaCBhcmUgd3JhcHBlcnMgb2YgRE1BX0lOL09VVCwgYW5kIGV2ZW4g
Zm9yIHRoZSBCQ1IgcmVnaXN0ZXIsIGl0DQo+IGlzIHJlYWQgYnkgZ2V0X2JjciBidXQgd3JpdHRl
biBieSBETUFfT1VULg0KPiBUaGlzIHBhdGNoIHVuaWZpZXMgdGhlIGluY29uc2lzdGVudCBtZXRo
b2RzLCBhbGwgcmVnaXN0ZXJzIGFyZSBhY2Nlc3NlZCBieQ0KPiBnZXQvc2V0Xyogbm93Lg0KDQoJ
RGF2aWQNCg0K

^ permalink raw reply

* Re: [PATCH] Fix 3bc95598 'powerpc/PCI: Use list_for_each_entry() for bus traversal'
From: Benjamin Herrenschmidt @ 2014-04-10  7:54 UTC (permalink / raw)
  To: Mike Qiu; +Cc: bhelgaas, paulus, wangyijing, linuxppc-dev, anton
In-Reply-To: <1397112695-3945-1-git-send-email-qiudayu@linux.vnet.ibm.com>

On Thu, 2014-04-10 at 02:51 -0400, Mike Qiu wrote:
> Unable to handle kernel paging request for data at address 0x00000000
> Faulting instruction address: 0xc000000000041d78
> Oops: Kernel access of bad area, sig: 11 [#1]
> ...
> NIP [c000000000041d78] .sys_pciconfig_iobase+0x68/0x1f0
> LR [c000000000041e0c] .sys_pciconfig_iobase+0xfc/0x1f0
> Call Trace:
> [c0000003b4787db0] [c000000000041e0c] .sys_pciconfig_iobase+0xfc/0x1f0 (unreliable)
> [c0000003b4787e30] [c000000000009ed8] syscall_exit+0x0/0x98
> 
> This bug was introduced by commit 3bc955987fb377f3c95bc29deb498e96819b8451
> The root cause was the 'bus' has been set to null while try to access
> bus->next.

Good catch. Out of curiosity, what is using that syscall nowadays ? It's
been long buggy in all sort of ways and is pretty much deprecated...

Cheers,
Ben.

> Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
> ---
>  arch/powerpc/kernel/pci_64.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
> index 2a47790..7b6c1ae 100644
> --- a/arch/powerpc/kernel/pci_64.c
> +++ b/arch/powerpc/kernel/pci_64.c
> @@ -209,6 +209,7 @@ long sys_pciconfig_iobase(long which, unsigned long in_bus,
>  {
>  	struct pci_controller* hose;
>  	struct pci_bus *bus = NULL;
> +	struct pci_bus *tmp_bus = NULL;
>  	struct device_node *hose_node;
>  
>  	/* Argh ! Please forgive me for that hack, but that's the
> @@ -229,10 +230,12 @@ long sys_pciconfig_iobase(long which, unsigned long in_bus,
>  	 * used on pre-domains setup. We return the first match
>  	 */
>  
> -	list_for_each_entry(bus, &pci_root_buses, node) {
> -		if (in_bus >= bus->number && in_bus <= bus->busn_res.end)
> +	list_for_each_entry(tmp_bus, &pci_root_buses, node) {
> +		if (in_bus >= tmp_bus->number &&
> +		    in_bus <= tmp_bus->busn_res.end) {
> +			bus = tmp_bus;
>  			break;
> -		bus = NULL;
> +		}
>  	}
>  	if (bus == NULL || bus->dev.of_node == NULL)
>  		return -ENODEV;

^ permalink raw reply

* Re: [PATCH v3 2/8] DMA: Freescale: unify register access methods
From: Hongbo Zhang @ 2014-04-10  9:33 UTC (permalink / raw)
  To: David Laight, vkoul@infradead.org, dan.j.williams@intel.com,
	dmaengine@vger.kernel.org
  Cc: scottwood@freescale.com, linuxppc-dev@lists.ozlabs.org,
	linux-kernel@vger.kernel.org, leo.li@freescale.com
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D0F6F4317@AcuExch.aculab.com>


On 04/10/2014 04:46 PM, David Laight wrote:
> From: hongbo.zhang@freescale.com
>> Methods of accessing DMA contorller registers are inconsistent, some registers
>                                 ^^

Thanks.
sorry, that it a typo.
I would wait to see if there are other defects I have to correct, if yes 
I can send a new iteration including this update, if no I would like to 
know if the maintainer can do me the favor to correct it when merging 
this patch, if still no, I will send a new iteration for this then.

>> are accessed by DMA_IN/OUT directly, while others are accessed by functions
>> get/set_* which are wrappers of DMA_IN/OUT, and even for the BCR register, it
>> is read by get_bcr but written by DMA_OUT.
>> This patch unifies the inconsistent methods, all registers are accessed by
>> get/set_* now.
> 	David
>

^ permalink raw reply

* Re: [PATCH v2] powernv: kvm: make _PAGE_NUMA take effect
From: Alexander Graf @ 2014-04-10 10:02 UTC (permalink / raw)
  To: Liu ping fan
  Cc: kvm-devel, kvm-ppc, Paul Mackerras, Aneesh Kumar K.V,
	linuxppc-dev, Alexander Graf
In-Reply-To: <CAFgQCTtbxRYMBVQL5Fw0254sFAZ_HSzkAfiH0ppNXacRSmxu-w@mail.gmail.com>


On 10.04.14 05:28, Liu ping fan wrote:
> On Mon, Apr 7, 2014 at 4:36 PM, Alexander Graf <agraf@suse.de> wrote:
>> On 07.04.14 09:42, Aneesh Kumar K.V wrote:
>>> Alexander Graf <agraf@suse.com> writes:
>>>
>>>> On 03.04.14 04:36, Liu ping fan wrote:
>>>>> Hi Alex, could you help to pick up this patch? since  v3.14 kernel can
>>>>> enable numa fault for powerpc.
>>>> What bad happens without this patch? We map a page even though it was
>>>> declared to get NUMA migrated? What happens next?
>>> Nothing much, we won't be properly accounting the numa access in the
>>> host.  What we want to achieve is to convert a guest access of the page to
>>> a host fault so that we can do proper numa access accounting in the
>>> host. This would enable us to migrate the page to the correct numa
>>> node.
>>
>> Ok, so no breakages, just less performance. I wouldn't consider it stable
>> material then :).
>>
> Sorry to reply late, since I am half out of office during this period.
> I am puzzling about you reply.    Without this patch, the guest can
> NOT sense the numa changes and ask host to put the pages in right
> place.  So the pages which is used by guest will be always misplaced.
> The numa-fault method is inspired by real requirement to improve
> performance, so we should also consider the performance drop of guest.
> Right?

The patch will get into Linux, I just consider a non-working new feature 
not a regression that warrants us to CC stable@vger :). After all 
performance shouldn't be worse than without the numa migration feature, 
correct?


Alex

^ permalink raw reply

* Re: [PATCH] Fix 3bc95598 'powerpc/PCI: Use list_for_each_entry() for bus traversal'
From: Mike Qiu @ 2014-04-10 10:03 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <1397116463.3671.181.camel@pasglop>

On 04/10/2014 03:54 PM, Benjamin Herrenschmidt wrote:
> On Thu, 2014-04-10 at 02:51 -0400, Mike Qiu wrote:
>> Unable to handle kernel paging request for data at address 0x00000000
>> Faulting instruction address: 0xc000000000041d78
>> Oops: Kernel access of bad area, sig: 11 [#1]
>> ...
>> NIP [c000000000041d78] .sys_pciconfig_iobase+0x68/0x1f0
>> LR [c000000000041e0c] .sys_pciconfig_iobase+0xfc/0x1f0
>> Call Trace:
>> [c0000003b4787db0] [c000000000041e0c] .sys_pciconfig_iobase+0xfc/0x1f0 (unreliable)
>> [c0000003b4787e30] [c000000000009ed8] syscall_exit+0x0/0x98
>>
>> This bug was introduced by commit 3bc955987fb377f3c95bc29deb498e96819b8451
>> The root cause was the 'bus' has been set to null while try to access
>> bus->next.
> Good catch. Out of curiosity, what is using that syscall nowadays ? It's
> been long buggy in all sort of ways and is pretty much deprecated...
>

I just boot my Power7 machine with newest mainline kernel, it happens 
and block the system.

I really do not know which software use this syscall, need to do some 
research on it.

Thanks
Mike
> Cheers,
> Ben.
>
>> Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
>> ---
>>   arch/powerpc/kernel/pci_64.c | 9 ++++++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/pci_64.c b/arch/powerpc/kernel/pci_64.c
>> index 2a47790..7b6c1ae 100644
>> --- a/arch/powerpc/kernel/pci_64.c
>> +++ b/arch/powerpc/kernel/pci_64.c
>> @@ -209,6 +209,7 @@ long sys_pciconfig_iobase(long which, unsigned long in_bus,
>>   {
>>   	struct pci_controller* hose;
>>   	struct pci_bus *bus = NULL;
>> +	struct pci_bus *tmp_bus = NULL;
>>   	struct device_node *hose_node;
>>   
>>   	/* Argh ! Please forgive me for that hack, but that's the
>> @@ -229,10 +230,12 @@ long sys_pciconfig_iobase(long which, unsigned long in_bus,
>>   	 * used on pre-domains setup. We return the first match
>>   	 */
>>   
>> -	list_for_each_entry(bus, &pci_root_buses, node) {
>> -		if (in_bus >= bus->number && in_bus <= bus->busn_res.end)
>> +	list_for_each_entry(tmp_bus, &pci_root_buses, node) {
>> +		if (in_bus >= tmp_bus->number &&
>> +		    in_bus <= tmp_bus->busn_res.end) {
>> +			bus = tmp_bus;
>>   			break;
>> -		bus = NULL;
>> +		}
>>   	}
>>   	if (bus == NULL || bus->dev.of_node == NULL)
>>   		return -ENODEV;
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

^ permalink raw reply

* [PATCH v5] ASoC: fsl_sai: Add clock controls for SAI
From: Nicolin Chen @ 2014-04-10 11:23 UTC (permalink / raw)
  To: broonie, shawn.guo
  Cc: mark.rutland, devicetree, alsa-devel, pawel.moll, linux-doc,
	ijc+devicetree, linux-kernel, robh+dt, timur, Li.Xiubo, rob,
	galak, linuxppc-dev

The SAI mainly has the following clocks:
  bus clock
    control and configure registers and to generate synchronous
    interrupts and DMA requests.

  mclk1, mclk2, mclk3
    to generate the bit clock when the receiver or transmitter is
    configured for an internally generated bit clock.

So this patch adds these clocks and their clock controls to the driver,
meanwhile, corrects the existing DTS accordingly so those platforms can
benifit from the further feature with different clock sources.

[ To concern the old DTB cases, I've added a bit of extra code to make
  the driver compatible with them. And by marking clock NULL if failed
  to get, the clk_prepare() or clk_get_rate() would easily return 0
  so no further path should be broken. -- by Nicolin ]

Signed-off-by: Nicolin Chen <Guangyu.Chen@freescale.com>
Acked-by: Xiubo Li <Li.Xiubo@freescale.com>
Acked-by: Shawn Guo <shawn.guo@linaro.org>
---

Changelog
v5:
 * Dropped mclk preparing and enabling since we are not using them currecntly.
 * Made the change compatible to the old DTB.
 * Added returned error value print.
v4:
 * Merged into single patch.
 * Fixed bus clock ID on vf610.
v3:
 * Use int type for ret instead of u32.
 * Added Acked-by and Tested-by from Xiubo Li.
v2:
 * Appended two extra mclks to the driver since SAI actually has three.
 * Renamed clock name to 'bus' and 'mclk' according to the reference manual.

 .../devicetree/bindings/sound/fsl-sai.txt          |  9 +++--
 arch/arm/boot/dts/vf610.dtsi                       |  6 ++--
 sound/soc/fsl/fsl_sai.c                            | 38 ++++++++++++++++++++--
 sound/soc/fsl/fsl_sai.h                            |  4 +++
 4 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/fsl-sai.txt b/Documentation/devicetree/bindings/sound/fsl-sai.txt
index 35c09fe..0f4e238 100644
--- a/Documentation/devicetree/bindings/sound/fsl-sai.txt
+++ b/Documentation/devicetree/bindings/sound/fsl-sai.txt
@@ -10,7 +10,8 @@ Required properties:
 - compatible: Compatible list, contains "fsl,vf610-sai" or "fsl,imx6sx-sai".
 - reg: Offset and length of the register set for the device.
 - clocks: Must contain an entry for each entry in clock-names.
-- clock-names : Must include the "sai" entry.
+- clock-names : Must include the "bus" for register access and "mclk1" "mclk2"
+  "mclk3" for bit clock and frame clock providing.
 - dmas : Generic dma devicetree binding as described in
   Documentation/devicetree/bindings/dma/dma.txt.
 - dma-names : Two dmas have to be defined, "tx" and "rx".
@@ -30,8 +31,10 @@ sai2: sai@40031000 {
 	      reg = <0x40031000 0x1000>;
 	      pinctrl-names = "default";
 	      pinctrl-0 = <&pinctrl_sai2_1>;
-	      clocks = <&clks VF610_CLK_SAI2>;
-	      clock-names = "sai";
+	      clocks = <&clks VF610_CLK_PLATFORM_BUS>,
+		     <&clks VF610_CLK_SAI2>,
+		     <&clks 0>, <&clks 0>;
+	      clock-names = "bus", "mclk1", "mclk2", "mclk3";
 	      dma-names = "tx", "rx";
 	      dmas = <&edma0 0 VF610_EDMA_MUXID0_SAI2_TX>,
 		   <&edma0 0 VF610_EDMA_MUXID0_SAI2_RX>;
diff --git a/arch/arm/boot/dts/vf610.dtsi b/arch/arm/boot/dts/vf610.dtsi
index d31ce1b..4c3cd59 100644
--- a/arch/arm/boot/dts/vf610.dtsi
+++ b/arch/arm/boot/dts/vf610.dtsi
@@ -139,8 +139,10 @@
 				compatible = "fsl,vf610-sai";
 				reg = <0x40031000 0x1000>;
 				interrupts = <0 86 0x04>;
-				clocks = <&clks VF610_CLK_SAI2>;
-				clock-names = "sai";
+				clocks = <&clks VF610_CLK_PLATFORM_BUS>,
+				       <&clks VF610_CLK_SAI2>,
+				       <&clks 0>, <&clks 0>;
+				clock-names = "bus", "mclk1", "mclk2", "mclk3";
 				status = "disabled";
 			};
 
diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c
index db9f75e..5fff2e1 100644
--- a/sound/soc/fsl/fsl_sai.c
+++ b/sound/soc/fsl/fsl_sai.c
@@ -401,7 +401,15 @@ static int fsl_sai_startup(struct snd_pcm_substream *substream,
 		struct snd_soc_dai *cpu_dai)
 {
 	struct fsl_sai *sai = snd_soc_dai_get_drvdata(cpu_dai);
+	struct device *dev = &sai->pdev->dev;
 	u32 reg;
+	int ret;
+
+	ret = clk_prepare_enable(sai->bus_clk);
+	if (ret) {
+		dev_err(dev, "failed to enable bus clock: %d\n", ret);
+		return ret;
+	}
 
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
 		reg = FSL_SAI_TCR3;
@@ -427,6 +435,8 @@ static void fsl_sai_shutdown(struct snd_pcm_substream *substream,
 
 	regmap_update_bits(sai->regmap, reg, FSL_SAI_CR3_TRCE,
 			   ~FSL_SAI_CR3_TRCE);
+
+	clk_disable_unprepare(sai->bus_clk);
 }
 
 static const struct snd_soc_dai_ops fsl_sai_pcm_dai_ops = {
@@ -559,7 +569,8 @@ static int fsl_sai_probe(struct platform_device *pdev)
 	struct fsl_sai *sai;
 	struct resource *res;
 	void __iomem *base;
-	int irq, ret;
+	char tmp[8];
+	int irq, ret, i;
 
 	sai = devm_kzalloc(&pdev->dev, sizeof(*sai), GFP_KERNEL);
 	if (!sai)
@@ -582,12 +593,35 @@ static int fsl_sai_probe(struct platform_device *pdev)
 		return PTR_ERR(base);
 
 	sai->regmap = devm_regmap_init_mmio_clk(&pdev->dev,
-			"sai", base, &fsl_sai_regmap_config);
+			"bus", base, &fsl_sai_regmap_config);
+
+	/* Compatible with old DTB cases */
+	if (IS_ERR(sai->regmap))
+		sai->regmap = devm_regmap_init_mmio_clk(&pdev->dev,
+				"sai", base, &fsl_sai_regmap_config);
 	if (IS_ERR(sai->regmap)) {
 		dev_err(&pdev->dev, "regmap init failed\n");
 		return PTR_ERR(sai->regmap);
 	}
 
+	/* No error out for old DTB cases but only mark the clock NULL */
+	sai->bus_clk = devm_clk_get(&pdev->dev, "bus");
+	if (IS_ERR(sai->bus_clk)) {
+		dev_err(&pdev->dev, "failed to get bus clock: %ld\n",
+				PTR_ERR(sai->bus_clk));
+		sai->bus_clk = NULL;
+	}
+
+	for (i = 0; i < FSL_SAI_MCLK_MAX; i++) {
+		sprintf(tmp, "mclk%d", i + 1);
+		sai->mclk_clk[i] = devm_clk_get(&pdev->dev, tmp);
+		if (IS_ERR(sai->mclk_clk[i])) {
+			dev_err(&pdev->dev, "failed to get mclk%d clock: %ld\n",
+					i + 1, PTR_ERR(sai->mclk_clk[i]));
+			sai->mclk_clk[i] = NULL;
+		}
+	}
+
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {
 		dev_err(&pdev->dev, "no irq for node %s\n", np->full_name);
diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h
index 677670d..0e6c9f5 100644
--- a/sound/soc/fsl/fsl_sai.h
+++ b/sound/soc/fsl/fsl_sai.h
@@ -119,6 +119,8 @@
 #define FSL_SAI_CLK_MAST2	2
 #define FSL_SAI_CLK_MAST3	3
 
+#define FSL_SAI_MCLK_MAX	3
+
 /* SAI data transfer numbers per DMA request */
 #define FSL_SAI_MAXBURST_TX 6
 #define FSL_SAI_MAXBURST_RX 6
@@ -126,6 +128,8 @@
 struct fsl_sai {
 	struct platform_device *pdev;
 	struct regmap *regmap;
+	struct clk *bus_clk;
+	struct clk *mclk_clk[FSL_SAI_MCLK_MAX];
 
 	bool big_endian_regs;
 	bool big_endian_data;
-- 
1.8.4

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox