[v7,06/14] spi/spi-atmel: add flag to controller data for lock operations

Commit Message

Wenyou Yang March 19, 2013, 7:46 a.m. UTC

From: Nicolas Ferre <[e-mail]>

Will allow to drop the lock during DMA operations.

Signed-off-by: Nicolas Ferre <[e-mail]>
Cc: [e-mail]
Cc: [e-mail]
[[e-mail]: submit the patch]
Signed-off-by: Wenyou Yang <[e-mail]>
---
 drivers/spi/spi-atmel.c |   31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

Comments

Mark Brown April 1, 2013, 1:46 p.m. UTC | #1

On Tue, Mar 19, 2013 at 03:46:03PM +0800, Wenyou Yang wrote:

> +static void atmel_spi_lock(struct atmel_spi *as)
> +{
> +		spin_lock_irqsave(&as->lock, as->flags);
> +}

Coding style.

> -	spin_unlock(&as->lock);
> +	atmel_spi_unlock(as);
>  	msg->complete(msg->context);
> -	spin_lock(&as->lock);
> +	atmel_spi_lock(as);

This replaces non-irqsave versions with irqsave versions of the lock.
Is this safe/correct?  If it is I'd have expected some sort of mention
of it in the commit log.

Wenyou Yang April 3, 2013, 5:09 a.m. UTC | #2

Hi, Mark,

> -----Original Message-----

> From: Mark Brown [mailto:[e-mail]]

> Sent: 2013?4?1? 21:46

> To: Yang, Wenyou

> Cc: [e-mail]; [e-mail];

> [e-mail]; [e-mail]; Ferre, Nicolas; Lin, JM;

> [e-mail]; [e-mail]

> Subject: Re: [PATCH v7 06/14] spi/spi-atmel: add flag to controller data for lock

> operations

> 

> On Tue, Mar 19, 2013 at 03:46:03PM +0800, Wenyou Yang wrote:

> 

> > +static void atmel_spi_lock(struct atmel_spi *as) {

> > +		spin_lock_irqsave(&as->lock, as->flags); }

> 

> Coding style.

I will change it in next version.

> 

> > -	spin_unlock(&as->lock);

> > +	atmel_spi_unlock(as);

> >  	msg->complete(msg->context);

> > -	spin_lock(&as->lock);

> > +	atmel_spi_lock(as);

> 

> This replaces non-irqsave versions with irqsave versions of the lock.

> Is this safe/correct?  If it is I'd have expected some sort of mention of it in the

> commit log.

I will add some commit log.

Best Regards,
Wenyou Yang

Patch

diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index 4692f1f..f28ce22 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -195,6 +195,7 @@  struct atmel_spi_caps {
  */
 struct atmel_spi {
 	spinlock_t		lock;
+	unsigned long		flags;
 
 	resource_size_t		phybase;
 	void __iomem		*regs;
@@ -333,6 +334,16 @@  static void cs_deactivate(struct atmel_spi *as, struct spi_device *spi)
 		gpio_set_value(asd->npcs_pin, !active);
 }
 
+static void atmel_spi_lock(struct atmel_spi *as)
+{
+		spin_lock_irqsave(&as->lock, as->flags);
+}
+
+static void atmel_spi_unlock(struct atmel_spi *as)
+{
+		spin_unlock_irqrestore(&as->lock, as->flags);
+}
+
 static inline int atmel_spi_xfer_is_last(struct spi_message *msg,
 					struct spi_transfer *xfer)
 {
@@ -569,9 +580,9 @@  atmel_spi_msg_done(struct spi_master *master, struct atmel_spi *as,
 		"xfer complete: %u bytes transferred\n",
 		msg->actual_length);
 
-	spin_unlock(&as->lock);
+	atmel_spi_unlock(as);
 	msg->complete(msg->context);
-	spin_lock(&as->lock);
+	atmel_spi_lock(as);
 
 	as->current_transfer = NULL;
 	as->next_transfer = NULL;
@@ -802,13 +813,11 @@  static int atmel_spi_setup(struct spi_device *spi)
 		spi->controller_state = asd;
 		gpio_direction_output(npcs_pin, !(spi->mode & SPI_CS_HIGH));
 	} else {
-		unsigned long		flags;
-
-		spin_lock_irqsave(&as->lock, flags);
+		atmel_spi_lock(as);
 		if (as->stay == spi)
 			as->stay = NULL;
 		cs_deactivate(as, spi);
-		spin_unlock_irqrestore(&as->lock, flags);
+		atmel_spi_unlock(as);
 	}
 
 	asd->csr = csr;
@@ -827,7 +836,6 @@  static int atmel_spi_transfer(struct spi_device *spi, struct spi_message *msg)
 {
 	struct atmel_spi	*as;
 	struct spi_transfer	*xfer;
-	unsigned long		flags;
 	struct device		*controller = spi->master->dev.parent;
 	u8			bits;
 	struct atmel_spi_device	*asd;
@@ -892,11 +900,11 @@  static int atmel_spi_transfer(struct spi_device *spi, struct spi_message *msg)
 	msg->status = -EINPROGRESS;
 	msg->actual_length = 0;
 
-	spin_lock_irqsave(&as->lock, flags);
+	atmel_spi_lock(as);
 	list_add_tail(&msg->queue, &as->queue);
 	if (!as->current_transfer)
 		atmel_spi_next_message(spi->master);
-	spin_unlock_irqrestore(&as->lock, flags);
+	atmel_spi_unlock(as);
 
 	return 0;
 }
@@ -906,17 +914,16 @@  static void atmel_spi_cleanup(struct spi_device *spi)
 	struct atmel_spi	*as = spi_master_get_devdata(spi->master);
 	struct atmel_spi_device	*asd = spi->controller_state;
 	unsigned		gpio = (unsigned) spi->controller_data;
-	unsigned long		flags;
 
 	if (!asd)
 		return;
 
-	spin_lock_irqsave(&as->lock, flags);
+	atmel_spi_lock(as);
 	if (as->stay == spi) {
 		as->stay = NULL;
 		cs_deactivate(as, spi);
 	}
-	spin_unlock_irqrestore(&as->lock, flags);
+	atmel_spi_unlock(as);
 
 	spi->controller_state = NULL;
 	gpio_free(gpio);