[PATCH 0/6] spi/spi-pl022 fixes

[PATCH 0/6] spi/spi-pl022 fixes

[Viresh Kumar]

Hi Linus,

This patchset mainly covers following fixes:
- formatting related issues
- Passing GFP_NOWAIT for sg allocation from tasklet
- Fixing calculate_effective_freq() routine
- Allocate/free DMA channels as and when required.

I have rebased them on linux-next/master over following patch:

commit 9be355da3bae9feb09cdaf80c3ab560f1f0172cb
Author: Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org>
Date:   Tue Aug 9 13:30:19 2011 +1000

    Add linux-next specific files for 20110809
    
    Signed-off-by: Stephen Rothwell <sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org>

Looking forward to see your view on the patchset.

Viresh Kumar (6):
  spi/spi-pl022: Resolve formatting issues
  spi/spi-pl022: Use GFP_NOWAIT for allocation from tasklet
  spi/spi-pl022: Don't allocate more sg than required.
  spi/spi-pl022: calculate_effective_freq() must set rate <= requested
    rate
  spi/spi-pl022: Call pl022_dma_remove(pl022) only if enable_dma is
    true
  spi/spi-pl022: Request/free DMA channels as and when required.

 drivers/spi/spi-pl022.c |  265 ++++++++++++++++++++++++++---------------------
 1 files changed, 147 insertions(+), 118 deletions(-)

-- 
1.7.2.2


------------------------------------------------------------------------------
uberSVN's rich system and user administration capabilities and model 
configuration take the hassle out of deploying and managing Subversion and 
the tools developers use with it. Learn more about uberSVN and get a free 
download at:  http://p.sf.net/sfu/wandisco-dev2dev

[PATCH 1/6] spi/spi-pl022: Resolve formatting issues

[Viresh Kumar]

There were few formatting related issues in code. This patch fixes them.
Fixes include:
- Remove extra blank lines
- align code to 80 cols
- combine several lines to one line
- Replace multiple spaces with tabs
- Remove spaces before labels

Signed-off-by: Viresh Kumar <viresh.kumar-qxv4g6HH51o@public.gmane.org>
---
 drivers/spi/spi-pl022.c |   54 +++++++++++++++++-----------------------------
 1 files changed, 20 insertions(+), 34 deletions(-)

diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
index 730b4a3..f600d00 100644
--- a/drivers/spi/spi-pl022.c
+++ b/drivers/spi/spi-pl022.c
@@ -113,7 +113,6 @@
 #define SSP_CR0_MASK_CSS_ST	(0x1FUL << 16)
 #define SSP_CR0_MASK_FRF_ST	(0x3UL << 21)
 
-
 /*
  * SSP Control Register 0  - SSP_CR1
  */
@@ -283,7 +282,6 @@
 
 #define SPI_POLLING_TIMEOUT 1000
 
-
 /*
  * The type of reading going on on this chip
  */
@@ -752,7 +750,6 @@ static void readwriter(struct pl022 *pl022)
 	 */
 }
 
-
 /**
  * next_transfer - Move to the Next transfer in the current spi message
  * @pl022: SSP driver private data structure
@@ -1534,8 +1531,7 @@ static void pump_messages(struct work_struct *work)
 	/* Initial message state */
 	pl022->cur_msg->state = STATE_START;
 	pl022->cur_transfer = list_entry(pl022->cur_msg->transfers.next,
-					    struct spi_transfer,
-					    transfer_list);
+					    struct spi_transfer, transfer_list);
 
 	/* Setup the SPI using the per chip configuration */
 	pl022->cur_chip = spi_get_ctldata(pl022->cur_msg->spi);
@@ -1557,7 +1553,6 @@ static void pump_messages(struct work_struct *work)
 		do_interrupt_dma_transfer(pl022);
 }
 
-
 static int __init init_queue(struct pl022 *pl022)
 {
 	INIT_LIST_HEAD(&pl022->queue);
@@ -1566,8 +1561,8 @@ static int __init init_queue(struct pl022 *pl022)
 	pl022->running = false;
 	pl022->busy = false;
 
-	tasklet_init(&pl022->pump_transfers,
-			pump_transfers,	(unsigned long)pl022);
+	tasklet_init(&pl022->pump_transfers, pump_transfers,
+			(unsigned long)pl022);
 
 	INIT_WORK(&pl022->pump_messages, pump_messages);
 	pl022->workqueue = create_singlethread_workqueue(
@@ -1578,7 +1573,6 @@ static int __init init_queue(struct pl022 *pl022)
 	return 0;
 }
 
-
 static int start_queue(struct pl022 *pl022)
 {
 	unsigned long flags;
@@ -1601,7 +1595,6 @@ static int start_queue(struct pl022 *pl022)
 	return 0;
 }
 
-
 static int stop_queue(struct pl022 *pl022)
 {
 	unsigned long flags;
@@ -1861,7 +1854,6 @@ static int calculate_effective_freq(struct pl022 *pl022,
 	return 0;
 }
 
-
 /*
  * A piece of default chip info unless the platform
  * supplies it.
@@ -1879,7 +1871,6 @@ static const struct pl022_config_chip pl022_default_chip_info = {
 	.cs_control = null_cs_control,
 };
 
-
 /**
  * pl022_setup - setup function registered to SPI master framework
  * @spi: spi device which is requesting setup
@@ -1956,7 +1947,6 @@ static int pl022_setup(struct spi_device *spi)
 		goto err_config_params;
 	}
 
-
 	status = verify_controller_parameters(pl022, chip_info);
 	if (status) {
 		dev_err(&spi->dev, "controller data is incorrect");
@@ -2096,12 +2086,13 @@ static int pl022_setup(struct spi_device *spi)
 	}
 	SSP_WRITE_BITS(chip->cr1, SSP_DISABLED, SSP_CR1_MASK_SSE, 1);
 	SSP_WRITE_BITS(chip->cr1, chip_info->hierarchy, SSP_CR1_MASK_MS, 2);
-	SSP_WRITE_BITS(chip->cr1, chip_info->slave_tx_disable, SSP_CR1_MASK_SOD, 3);
+	SSP_WRITE_BITS(chip->cr1, chip_info->slave_tx_disable, SSP_CR1_MASK_SOD,
+		3);
 
 	/* Save controller_state */
 	spi_set_ctldata(spi, chip);
 	return status;
- err_config_params:
+err_config_params:
 	spi_set_ctldata(spi, NULL);
 	kfree(chip);
 	return status;
@@ -2122,7 +2113,6 @@ static void pl022_cleanup(struct spi_device *spi)
 	kfree(chip);
 }
 
-
 static int __devinit
 pl022_probe(struct amba_device *adev, const struct amba_id *id)
 {
@@ -2243,23 +2233,23 @@ pl022_probe(struct amba_device *adev, const struct amba_id *id)
 	amba_vcore_disable(adev);
 	return 0;
 
- err_spi_register:
- err_start_queue:
- err_init_queue:
+err_spi_register:
+err_start_queue:
+err_init_queue:
 	destroy_queue(pl022);
 	pl022_dma_remove(pl022);
 	free_irq(adev->irq[0], pl022);
 	pm_runtime_disable(&adev->dev);
- err_no_irq:
+err_no_irq:
 	clk_put(pl022->clk);
- err_no_clk:
+err_no_clk:
 	iounmap(pl022->virtbase);
- err_no_ioremap:
+err_no_ioremap:
 	amba_release_regions(adev);
- err_no_ioregion:
+err_no_ioregion:
 	spi_master_put(master);
- err_no_master:
- err_no_pdata:
+err_no_master:
+err_no_pdata:
 	return status;
 }
 
@@ -2337,7 +2327,6 @@ static struct vendor_data vendor_arm = {
 	.loopback = true,
 };
 
-
 static struct vendor_data vendor_st = {
 	.fifodepth = 32,
 	.max_bpw = 32,
@@ -2392,9 +2381,9 @@ static struct amba_id pl022_ids[] = {
 		 * and 32 locations deep TX/RX FIFO but no extended
 		 * CR0/CR1 register
 		 */
-		.id     = 0x00080023,
-		.mask   = 0xffffffff,
-		.data   = &vendor_st_pl023,
+		.id	= 0x00080023,
+		.mask	= 0xffffffff,
+		.data	= &vendor_st_pl023,
 	},
 	{
 		.id	= 0x10080023,
@@ -2411,23 +2400,20 @@ static struct amba_driver pl022_driver = {
 	.id_table	= pl022_ids,
 	.probe		= pl022_probe,
 	.remove		= __devexit_p(pl022_remove),
-	.suspend        = pl022_suspend,
-	.resume         = pl022_resume,
+	.suspend	= pl022_suspend,
+	.resume		= pl022_resume,
 };
 
-
 static int __init pl022_init(void)
 {
 	return amba_driver_register(&pl022_driver);
 }
-
 subsys_initcall(pl022_init);
 
 static void __exit pl022_exit(void)
 {
 	amba_driver_unregister(&pl022_driver);
 }
-
 module_exit(pl022_exit);
 
 MODULE_AUTHOR("Linus Walleij <linus.walleij-0IS4wlFg1OjSUeElwK9/Pw@public.gmane.org>");
-- 
1.7.2.2


------------------------------------------------------------------------------
uberSVN's rich system and user administration capabilities and model 
configuration take the hassle out of deploying and managing Subversion and 
the tools developers use with it. Learn more about uberSVN and get a free 
download at:  http://p.sf.net/sfu/wandisco-dev2dev

[PATCH 2/6] spi/spi-pl022: Use GFP_NOWAIT for allocation from tasklet

[Viresh Kumar]

tasklets don't allow invocation to sleeping routines. In configure_dma()
routine, sg_alloc_table() was called with GFP_KERNEL flag and so this causes
crash when called from tasklet.

Replace GFP_KERNEL with GFP_NOWAIT to get this fixed.

Signed-off-by: Viresh Kumar <viresh.kumar-qxv4g6HH51o@public.gmane.org>
---
 drivers/spi/spi-pl022.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
index f600d00..cbd9afe 100644
--- a/drivers/spi/spi-pl022.c
+++ b/drivers/spi/spi-pl022.c
@@ -1019,11 +1019,11 @@ static int configure_dma(struct pl022 *pl022)
 	pages = (pl022->cur_transfer->len >> PAGE_SHIFT) + 1;
 	dev_dbg(&pl022->adev->dev, "using %d pages for transfer\n", pages);
 
-	ret = sg_alloc_table(&pl022->sgt_rx, pages, GFP_KERNEL);
+	ret = sg_alloc_table(&pl022->sgt_rx, pages, GFP_NOWAIT);
 	if (ret)
 		goto err_alloc_rx_sg;
 
-	ret = sg_alloc_table(&pl022->sgt_tx, pages, GFP_KERNEL);
+	ret = sg_alloc_table(&pl022->sgt_tx, pages, GFP_NOWAIT);
 	if (ret)
 		goto err_alloc_tx_sg;
 
-- 
1.7.2.2


------------------------------------------------------------------------------
uberSVN's rich system and user administration capabilities and model 
configuration take the hassle out of deploying and managing Subversion and 
the tools developers use with it. Learn more about uberSVN and get a free 
download at:  http://p.sf.net/sfu/wandisco-dev2dev

Re: [PATCH 2/6] spi/spi-pl022: Use GFP_NOWAIT for allocation from tasklet

[Jassi Brar]

On Tue, Aug 9, 2011 at 4:35 PM, Viresh Kumar <viresh.kumar@st.com> wrote:
> tasklets don't allow invocation to sleeping routines. In configure_dma()
> routine, sg_alloc_table() was called with GFP_KERNEL flag and so this causes
> crash when called from tasklet.
>
> Replace GFP_KERNEL with GFP_NOWAIT to get this fixed.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
> ---
>  drivers/spi/spi-pl022.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
> index f600d00..cbd9afe 100644
> --- a/drivers/spi/spi-pl022.c
> +++ b/drivers/spi/spi-pl022.c
> @@ -1019,11 +1019,11 @@ static int configure_dma(struct pl022 *pl022)
>        pages = (pl022->cur_transfer->len >> PAGE_SHIFT) + 1;
>        dev_dbg(&pl022->adev->dev, "using %d pages for transfer\n", pages);
>
> -       ret = sg_alloc_table(&pl022->sgt_rx, pages, GFP_KERNEL);
> +       ret = sg_alloc_table(&pl022->sgt_rx, pages, GFP_NOWAIT);
>        if (ret)
>                goto err_alloc_rx_sg;
>
> -       ret = sg_alloc_table(&pl022->sgt_tx, pages, GFP_KERNEL);
> +       ret = sg_alloc_table(&pl022->sgt_tx, pages, GFP_NOWAIT);
>        if (ret)
>                goto err_alloc_tx_sg;

Since this could be called from when we actually need the transfer start,
maybe we could try harder using GFP_ATOMIC instead ?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/6] spi/spi-pl022: Use GFP_NOWAIT for allocation from tasklet

[viresh kumar]

On 08/09/2011 04:56 PM, Jassi Brar wrote:
> Since this could be called from when we actually need the transfer start,
> maybe we could try harder using GFP_ATOMIC instead ?

Yes. Will do it.

-- 
viresh

------------------------------------------------------------------------------
uberSVN's rich system and user administration capabilities and model 
configuration take the hassle out of deploying and managing Subversion and 
the tools developers use with it. Learn more about uberSVN and get a free 
download at:  http://p.sf.net/sfu/wandisco-dev2dev

[PATCH 3/6] spi/spi-pl022: Don't allocate more sg than required.

[Viresh Kumar]

In routine configure_dma(), if transfer->len = PAGE_SIZE, then pages is one more
than required. While leads to one more sg getting allocated.

This is wrong. Correct this to allocate correct number of sg.

Signed-off-by: Viresh Kumar <viresh.kumar-qxv4g6HH51o@public.gmane.org>
---
 drivers/spi/spi-pl022.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
index cbd9afe..452952b 100644
--- a/drivers/spi/spi-pl022.c
+++ b/drivers/spi/spi-pl022.c
@@ -1016,7 +1016,8 @@ static int configure_dma(struct pl022 *pl022)
 	dmaengine_slave_config(txchan, &tx_conf);
 
 	/* Create sglists for the transfers */
-	pages = (pl022->cur_transfer->len >> PAGE_SHIFT) + 1;
+	pages = ((pl022->cur_transfer->len + (1 << PAGE_SHIFT) - 1)
+			>> PAGE_SHIFT);
 	dev_dbg(&pl022->adev->dev, "using %d pages for transfer\n", pages);
 
 	ret = sg_alloc_table(&pl022->sgt_rx, pages, GFP_NOWAIT);
-- 
1.7.2.2


------------------------------------------------------------------------------
uberSVN's rich system and user administration capabilities and model 
configuration take the hassle out of deploying and managing Subversion and 
the tools developers use with it. Learn more about uberSVN and get a free 
download at:  http://p.sf.net/sfu/wandisco-dev2dev

[PATCH 4/6] spi/spi-pl022: calculate_effective_freq() must set rate <= requested rate

[Viresh Kumar]

There were few issues with calculate_effective_freq() routine:
- It was returning first rate found >= requested rate. Now, if system have spi's
  rate as 83 MHz, with possible prescaled rates as 83, 41.5, 20.75, 13.83 (as we
  can prescale with multiples of 2). If user has given rate to be programmed as
  22 MHz, then driver programmes it to 41.5 MHz. This looks to be incorrect, as
  user might have given the upper limit of the device, and we are programming it
  above it.
- Driver finds the first satisfying rate and programmes it, but with other
  values of scr & cpsdvsr, it is possible to get more closer rate.

This patch fixes these two issues, with some reformatting inside the code.
This also creates a macro to calculate prescaled rate based on spi's rate,
cpsdvsr and scr.

Signed-off-by: Viresh Kumar <viresh.kumar-qxv4g6HH51o@public.gmane.org>
---
 drivers/spi/spi-pl022.c |   98 ++++++++++++++++++++++------------------------
 1 files changed, 47 insertions(+), 51 deletions(-)

diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
index 452952b..e0cfb8c 100644
--- a/drivers/spi/spi-pl022.c
+++ b/drivers/spi/spi-pl022.c
@@ -1791,67 +1791,63 @@ static int pl022_transfer(struct spi_device *spi, struct spi_message *msg)
 	return 0;
 }
 
-static int calculate_effective_freq(struct pl022 *pl022,
-				    int freq,
-				    struct ssp_clock_params *clk_freq)
+#define SPI_RATE(rate, cpsdvsr, scr)	(rate / (cpsdvsr * (1 + scr)))
+static int calculate_effective_freq(struct pl022 *pl022, int freq, struct
+				    ssp_clock_params * clk_freq)
 {
 	/* Lets calculate the frequency parameters */
-	u16 cpsdvsr = 2;
-	u16 scr = 0;
-	bool freq_found = false;
-	u32 rate;
-	u32 max_tclk;
-	u32 min_tclk;
+	u16 cpsdvsr = CPSDVR_MIN, scr = SCR_MIN;
+	u32 rate, max_tclk, min_tclk, best_freq = 0, best_cpsdvsr = 0,
+		best_scr = 0, tmp, found = 0;
 
 	rate = clk_get_rate(pl022->clk);
 	/* cpsdvscr = 2 & scr 0 */
-	max_tclk = (rate / (CPSDVR_MIN * (1 + SCR_MIN)));
+	max_tclk = SPI_RATE(rate, CPSDVR_MIN, SCR_MIN);
 	/* cpsdvsr = 254 & scr = 255 */
-	min_tclk = (rate / (CPSDVR_MAX * (1 + SCR_MAX)));
-
-	if ((freq <= max_tclk) && (freq >= min_tclk)) {
-		while (cpsdvsr <= CPSDVR_MAX && !freq_found) {
-			while (scr <= SCR_MAX && !freq_found) {
-				if ((rate /
-				     (cpsdvsr * (1 + scr))) > freq)
-					scr += 1;
-				else {
-					/*
-					 * This bool is made true when
-					 * effective frequency >=
-					 * target frequency is found
-					 */
-					freq_found = true;
-					if ((rate /
-					     (cpsdvsr * (1 + scr))) != freq) {
-						if (scr == SCR_MIN) {
-							cpsdvsr -= 2;
-							scr = SCR_MAX;
-						} else
-							scr -= 1;
-					}
-				}
-			}
-			if (!freq_found) {
-				cpsdvsr += 2;
-				scr = SCR_MIN;
-			}
-		}
-		if (cpsdvsr != 0) {
-			dev_dbg(&pl022->adev->dev,
-				"SSP Effective Frequency is %u\n",
-				(rate / (cpsdvsr * (1 + scr))));
-			clk_freq->cpsdvsr = (u8) (cpsdvsr & 0xFF);
-			clk_freq->scr = (u8) (scr & 0xFF);
-			dev_dbg(&pl022->adev->dev,
-				"SSP cpsdvsr = %d, scr = %d\n",
-				clk_freq->cpsdvsr, clk_freq->scr);
-		}
-	} else {
+	min_tclk = SPI_RATE(rate, CPSDVR_MAX, SCR_MAX);
+
+	if (!((freq <= max_tclk) && (freq >= min_tclk))) {
 		dev_err(&pl022->adev->dev,
 			"controller data is incorrect: out of range frequency");
 		return -EINVAL;
 	}
+
+	/*
+	 * best_freq will give closest possible available rate (<= requested
+	 * freq) for all values of scr & cpsdvsr.
+	 */
+	while ((cpsdvsr <= CPSDVR_MAX) && !found) {
+		while (scr <= SCR_MAX) {
+			tmp = SPI_RATE(rate, cpsdvsr, scr);
+
+			if (tmp > freq)
+				scr++;
+			/*
+			 * If found exact value, update and break.
+			 * If found more closer value, update and continue.
+			 */
+			else if ((tmp == freq) || (tmp > best_freq)) {
+				best_freq = tmp;
+				best_cpsdvsr = cpsdvsr;
+				best_scr = scr;
+
+				if (tmp == freq)
+					break;
+			}
+			scr++;
+		}
+		cpsdvsr += 2;
+		scr = SCR_MIN;
+	}
+
+	clk_freq->cpsdvsr = (u8) (best_cpsdvsr & 0xFF);
+	clk_freq->scr = (u8) (best_scr & 0xFF);
+	dev_dbg(&pl022->adev->dev,
+		"SSP Target Frequency is: %u, Effective Frequency is %u\n",
+		freq, best_freq);
+	dev_dbg(&pl022->adev->dev, "SSP cpsdvsr = %d, scr = %d\n",
+		clk_freq->cpsdvsr, clk_freq->scr);
+
 	return 0;
 }
 
-- 
1.7.2.2


------------------------------------------------------------------------------
uberSVN's rich system and user administration capabilities and model 
configuration take the hassle out of deploying and managing Subversion and 
the tools developers use with it. Learn more about uberSVN and get a free 
download at:  http://p.sf.net/sfu/wandisco-dev2dev

Re: [PATCH 4/6] spi/spi-pl022: calculate_effective_freq() must set rate <= requested rate

[viresh kumar]

[Probably you missed reply-all by mistake, so adding them again]

On 08/10/2011 01:34 AM, Linus Walleij wrote:
> On Tue, Aug 9, 2011 at 1:05 PM, Viresh Kumar <viresh.kumar@st.com> wrote:
> 
>> There were few issues with calculate_effective_freq() routine:
>> - It was returning first rate found >= requested rate. Now, if system have spi's
>>  rate as 83 MHz, with possible prescaled rates as 83, 41.5, 20.75, 13.83 (as we
>>  can prescale with multiples of 2). If user has given rate to be programmed as
>>  22 MHz, then driver programmes it to 41.5 MHz. This looks to be incorrect, as
>>  user might have given the upper limit of the device, and we are programming it
>>  above it.
>> - Driver finds the first satisfying rate and programmes it, but with other
>>  values of scr & cpsdvsr, it is possible to get more closer rate.
> 
> Good that you found this bug!
> 
>> +#define SPI_RATE(rate, cpsdvsr, scr)   (rate / (cpsdvsr * (1 + scr)))
> 
> Can you use a static inline instead of a macro? It is often preferred.
> If you do, this is Acked-by.
> 

Not a problem. Will surely do that.

-- 
viresh

[PATCH 5/6] spi/spi-pl022: Call pl022_dma_remove(pl022) only if enable_dma is true

[Viresh Kumar]

pl022_dma_remove() should be called only if enable_dma is true. There is no
point calling it when pl022_dma_probe() is not called, which again depends on
enable_dma.

Signed-off-by: Viresh Kumar <viresh.kumar-qxv4g6HH51o@public.gmane.org>
---
 drivers/spi/spi-pl022.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
index e0cfb8c..e2333e7 100644
--- a/drivers/spi/spi-pl022.c
+++ b/drivers/spi/spi-pl022.c
@@ -2234,7 +2234,9 @@ err_spi_register:
 err_start_queue:
 err_init_queue:
 	destroy_queue(pl022);
-	pl022_dma_remove(pl022);
+	if (platform_info->enable_dma)
+		pl022_dma_remove(pl022);
+
 	free_irq(adev->irq[0], pl022);
 	pm_runtime_disable(&adev->dev);
 err_no_irq:
@@ -2262,7 +2264,9 @@ pl022_remove(struct amba_device *adev)
 	if (destroy_queue(pl022) != 0)
 		dev_err(&adev->dev, "queue remove failed\n");
 	load_ssp_default_config(pl022);
-	pl022_dma_remove(pl022);
+	if (pl022->master_info->enable_dma)
+		pl022_dma_remove(pl022);
+
 	free_irq(adev->irq[0], pl022);
 	clk_disable(pl022->clk);
 	clk_put(pl022->clk);
-- 
1.7.2.2


------------------------------------------------------------------------------
uberSVN's rich system and user administration capabilities and model 
configuration take the hassle out of deploying and managing Subversion and 
the tools developers use with it. Learn more about uberSVN and get a free 
download at:  http://p.sf.net/sfu/wandisco-dev2dev

[PATCH 6/6] spi/spi-pl022: Request/free DMA channels as and when required.

[Viresh Kumar]

Currently we request DMA channels at probe time and free them at remove. They
are always occupied, irrespective of their usage.

They must be allocated when they are required and must be freed after we are
done with transfers. So that they can be used by other users.

Signed-off-by: Viresh Kumar <viresh.kumar-qxv4g6HH51o@public.gmane.org>
---
 drivers/spi/spi-pl022.c |   98 +++++++++++++++++++++++++++++++++-------------
 1 files changed, 70 insertions(+), 28 deletions(-)

diff --git a/drivers/spi/spi-pl022.c b/drivers/spi/spi-pl022.c
index e2333e7..3653f00 100644
--- a/drivers/spi/spi-pl022.c
+++ b/drivers/spi/spi-pl022.c
@@ -389,6 +389,7 @@ struct pl022 {
 	struct sg_table			sgt_rx;
 	struct sg_table			sgt_tx;
 	char				*dummypage;
+	dma_cap_mask_t			mask;
 #endif
 };
 
@@ -1093,16 +1094,33 @@ err_alloc_rx_sg:
 
 static int __init pl022_dma_probe(struct pl022 *pl022)
 {
-	dma_cap_mask_t mask;
+	/* set dma mask */
+	dma_cap_zero(pl022->mask);
+	dma_cap_set(DMA_SLAVE, pl022->mask);
 
-	/* Try to acquire a generic DMA engine slave channel */
-	dma_cap_zero(mask);
-	dma_cap_set(DMA_SLAVE, mask);
+	pl022->dummypage = kmalloc(PAGE_SIZE, GFP_KERNEL);
+	if (!pl022->dummypage) {
+		dev_err(&pl022->adev->dev,
+			"Failed to work in dma mode, work without dma!\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
+static int pl022_alloc_dmachan(struct pl022 *pl022)
+{
 	/*
-	 * We need both RX and TX channels to do DMA, else do none
-	 * of them.
+	 * Both channels should be allocated or not allocated. It is wrong if
+	 * only one allocated
 	 */
-	pl022->dma_rx_channel = dma_request_channel(mask,
+	if (pl022->dma_rx_channel && pl022->dma_tx_channel)
+		return 0;
+
+	BUG_ON(pl022->dma_rx_channel || pl022->dma_tx_channel);
+
+	/* We need both RX and TX channels to do DMA, else do none of them. */
+	pl022->dma_rx_channel = dma_request_channel(pl022->mask,
 					    pl022->master_info->dma_filter,
 					    pl022->master_info->dma_rx_param);
 	if (!pl022->dma_rx_channel) {
@@ -1110,7 +1128,7 @@ static int __init pl022_dma_probe(struct pl022 *pl022)
 		goto err_no_rxchan;
 	}
 
-	pl022->dma_tx_channel = dma_request_channel(mask,
+	pl022->dma_tx_channel = dma_request_channel(pl022->mask,
 					    pl022->master_info->dma_filter,
 					    pl022->master_info->dma_tx_param);
 	if (!pl022->dma_tx_channel) {
@@ -1118,20 +1136,12 @@ static int __init pl022_dma_probe(struct pl022 *pl022)
 		goto err_no_txchan;
 	}
 
-	pl022->dummypage = kmalloc(PAGE_SIZE, GFP_KERNEL);
-	if (!pl022->dummypage) {
-		dev_dbg(&pl022->adev->dev, "no DMA dummypage!\n");
-		goto err_no_dummypage;
-	}
-
-	dev_info(&pl022->adev->dev, "setup for DMA on RX %s, TX %s\n",
+	dev_dbg(&pl022->adev->dev, "setup for DMA on RX %s, TX %s\n",
 		 dma_chan_name(pl022->dma_rx_channel),
 		 dma_chan_name(pl022->dma_tx_channel));
 
 	return 0;
 
-err_no_dummypage:
-	dma_release_channel(pl022->dma_tx_channel);
 err_no_txchan:
 	dma_release_channel(pl022->dma_rx_channel);
 	pl022->dma_rx_channel = NULL;
@@ -1143,22 +1153,30 @@ err_no_rxchan:
 
 static void terminate_dma(struct pl022 *pl022)
 {
-	struct dma_chan *rxchan = pl022->dma_rx_channel;
-	struct dma_chan *txchan = pl022->dma_tx_channel;
-
-	dmaengine_terminate_all(rxchan);
-	dmaengine_terminate_all(txchan);
-	unmap_free_dma_scatter(pl022);
+	if (pl022->dma_rx_channel)
+		dmaengine_terminate_all(pl022->dma_rx_channel);
+	if (pl022->dma_tx_channel)
+		dmaengine_terminate_all(pl022->dma_tx_channel);
 }
 
-static void pl022_dma_remove(struct pl022 *pl022)
+static void pl022_free_dmachan(struct pl022 *pl022)
 {
-	if (pl022->busy)
-		terminate_dma(pl022);
 	if (pl022->dma_tx_channel)
 		dma_release_channel(pl022->dma_tx_channel);
 	if (pl022->dma_rx_channel)
 		dma_release_channel(pl022->dma_rx_channel);
+
+	pl022->dma_rx_channel = pl022->dma_tx_channel = NULL;
+}
+
+static void pl022_dma_remove(struct pl022 *pl022)
+{
+	if (pl022->busy) {
+		terminate_dma(pl022);
+		unmap_free_dma_scatter(pl022);
+	}
+
+	pl022_free_dmachan(pl022);
 	kfree(pl022->dummypage);
 }
 
@@ -1176,6 +1194,19 @@ static inline int pl022_dma_probe(struct pl022 *pl022)
 static inline void pl022_dma_remove(struct pl022 *pl022)
 {
 }
+
+static inline int pl022_alloc_dmachan(struct pl022 *pl022)
+{
+	return 0;
+}
+
+static inline void terminate_dma(struct pl022 *pl022)
+{
+}
+
+static void pl022_free_dmachan(struct pl022 *pl022)
+{
+}
 #endif
 
 /**
@@ -1401,16 +1432,20 @@ static void do_interrupt_dma_transfer(struct pl022 *pl022)
 	}
 	/* If we're using DMA, set up DMA here */
 	if (pl022->cur_chip->enable_dma) {
+		int status = pl022_alloc_dmachan(pl022);
+		if (status)
+			goto switch_to_irq_mode;
+
 		/* Configure DMA transfer */
 		if (configure_dma(pl022)) {
 			dev_dbg(&pl022->adev->dev,
 				"configuration of DMA failed, fall back to interrupt mode\n");
-			goto err_config_dma;
+			goto switch_to_irq_mode;
 		}
 		/* Disable interrupts in DMA mode, IRQ from DMA controller */
 		irqflags = DISABLE_ALL_INTERRUPTS;
 	}
-err_config_dma:
+switch_to_irq_mode:
 	/* Enable SSP, turn on interrupts */
 	writew((readw(SSP_CR1(pl022->virtbase)) | SSP_CR1_MASK_SSE),
 	       SSP_CR1(pl022->virtbase));
@@ -1513,7 +1548,14 @@ static void pump_messages(struct work_struct *work)
 	spin_lock_irqsave(&pl022->queue_lock, flags);
 	if (list_empty(&pl022->queue) || !pl022->running) {
 		pl022->busy = false;
+
 		spin_unlock_irqrestore(&pl022->queue_lock, flags);
+
+		/* free dma channels */
+		if (pl022->master_info->enable_dma) {
+			terminate_dma(pl022);
+			pl022_free_dmachan(pl022);
+		}
 		return;
 	}
 	/* Make sure we are not already running a message */
-- 
1.7.2.2


------------------------------------------------------------------------------
uberSVN's rich system and user administration capabilities and model 
configuration take the hassle out of deploying and managing Subversion and 
the tools developers use with it. Learn more about uberSVN and get a free 
download at:  http://p.sf.net/sfu/wandisco-dev2dev

Re: [PATCH 0/6] spi/spi-pl022 fixes

[Linus Walleij]

On Tue, Aug 9, 2011 at 1:05 PM, Viresh Kumar <viresh.kumar@st.com> wrote:

> This patchset mainly covers following fixes:
> - formatting related issues
> - Passing GFP_NOWAIT for sg allocation from tasklet
> - Fixing calculate_effective_freq() routine
> - Allocate/free DMA channels as and when required.

Excellent work Viresh, apart from the small comment on 4/6
they all look good.

I also tested them all on the U300, works perfectly.

Please fixup 4/6 and respin with my:
Tested-by: Linus Walleij <linus.walleij@linaro.org>

So that Grant can pick them up.

> I have rebased them on linux-next/master over following patch:
>
> commit 9be355da3bae9feb09cdaf80c3ab560f1f0172cb
> Author: Stephen Rothwell <sfr@canb.auug.org.au>
> Date:   Tue Aug 9 13:30:19 2011 +1000
>
>    Add linux-next specific files for 20110809

They all work perfectly well on top of v3.1-rc1, please use
mainline kernel tags as patch base if you can.

Yours,
Linus Walleij

Re: [PATCH 0/6] spi/spi-pl022 fixes

[viresh kumar]

On 08/10/2011 01:40 AM, Linus Walleij wrote:
> On Tue, Aug 9, 2011 at 1:05 PM, Viresh Kumar <viresh.kumar@st.com> wrote:
> 
>> This patchset mainly covers following fixes:
>> - formatting related issues
>> - Passing GFP_NOWAIT for sg allocation from tasklet
>> - Fixing calculate_effective_freq() routine
>> - Allocate/free DMA channels as and when required.
> 
> Excellent work Viresh, apart from the small comment on 4/6
> they all look good.
> 
> I also tested them all on the U300, works perfectly.
> 
> Please fixup 4/6 and respin with my:
> Tested-by: Linus Walleij <linus.walleij@linaro.org>

Thanks for testing.

> 
> So that Grant can pick them up.
> 
>> I have rebased them on linux-next/master over following patch:
>>
>> commit 9be355da3bae9feb09cdaf80c3ab560f1f0172cb
>> Author: Stephen Rothwell <sfr@canb.auug.org.au>
>> Date:   Tue Aug 9 13:30:19 2011 +1000
>>
>>    Add linux-next specific files for 20110809
> 
> They all work perfectly well on top of v3.1-rc1, please use
> mainline kernel tags as patch base if you can.
> 

Sure, can take care of this. But i have a doubt here,
What if a patch on the same driver is applied after kernel tag.
Shouldn't i rebase my patches on the latest commit available.

-- 
viresh

Re: [PATCH 0/6] spi/spi-pl022 fixes

[Linus Walleij]

2011/8/10 viresh kumar <viresh.kumar@st.com>:
> On 08/10/2011 01:40 AM, Linus Walleij wrote:
>>
>> They all work perfectly well on top of v3.1-rc1, please use
>> mainline kernel tags as patch base if you can.
>
> Sure, can take care of this. But i have a doubt here,
> What if a patch on the same driver is applied after kernel tag.
> Shouldn't i rebase my patches on the latest commit available.

The basic assumption is that patches are orthogonal and apply
without conflicts, i.e. patch different parts of the kernel tree or
different lines in the same file.

Second assumption is that if patches do collide, the subsystem
maintainer can solve most conflicts, and is expected to do
so. Torvalds sets an example by requesting that subsystem
maintainers do not fixup conflicts between subsystems so he
can see them himself.

Then there are special cases when you get a strong
coupling between patches.

In these special cases, ask the subsystem maintainer what to
base the patches on. Usually it's some commit on the subsystem
tree rather than -next.

This is my understanding of how we work, maybe I should
patch Documentation/development-process/*, Arnd do you have
anything to add?

Thanks,
Linus Walleij

Re: [PATCH 0/6] spi/spi-pl022 fixes

[Arnd Bergmann]

On Wednesday 10 August 2011, Linus Walleij wrote:
> The basic assumption is that patches are orthogonal and apply
> without conflicts, i.e. patch different parts of the kernel tree or
> different lines in the same file.
> 
> Second assumption is that if patches do collide, the subsystem
> maintainer can solve most conflicts, and is expected to do
> so. Torvalds sets an example by requesting that subsystem
> maintainers do not fixup conflicts between subsystems so he
> can see them himself.
> 
> Then there are special cases when you get a strong
> coupling between patches.
> 
> In these special cases, ask the subsystem maintainer what to
> base the patches on. Usually it's some commit on the subsystem
> tree rather than -next.
> 
> This is my understanding of how we work, maybe I should
> patch Documentation/development-process/*, Arnd do you have
> anything to add?

No, your explanations are pretty much what I would have said.

	Arnd

------------------------------------------------------------------------------
uberSVN's rich system and user administration capabilities and model 
configuration take the hassle out of deploying and managing Subversion and 
the tools developers use with it. Learn more about uberSVN and get a free 
download at:  http://p.sf.net/sfu/wandisco-dev2dev