[PATCH V3] mtd: fsl-quadspi: Access multiple chips simultaneously

[PATCH V3] mtd: fsl-quadspi: Access multiple chips simultaneously

[Han Xu]

Add supports for simultaneous access to multiple chips. Need to lock
the mutex before any quad spi operations and unlock the mutex after
operations complete.

Signed-off-by: Han Xu <b45815@freescale.com>
---
v2 -- > v3
1. Unlock the mutex if errors occur.
2. Destroy the mutex when module remove or probe fail.

v1 -- > v2
1. Rebase to latest l2-mtd code and resend.
---


 drivers/mtd/spi-nor/fsl-quadspi.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 52a872f..e6fc064 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -26,6 +26,7 @@
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
 #include <linux/mtd/spi-nor.h>
+#include <linux/mutex.h>
 
 /* The registers */
 #define QUADSPI_MCR			0x00
@@ -233,6 +234,7 @@ struct fsl_qspi {
 	u32 clk_rate;
 	unsigned int chip_base_addr; /* We may support two chips. */
 	bool has_second_chip;
+	struct mutex lock;
 };
 
 static inline int is_vybrid_qspi(struct fsl_qspi *q)
@@ -761,13 +763,17 @@ static int fsl_qspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
 	struct fsl_qspi *q = nor->priv;
 	int ret;
 
+	mutex_lock(&q->lock);
 	ret = clk_enable(q->clk_en);
-	if (ret)
+	if (ret) {
+		mutex_unlock(&q->lock);
 		return ret;
+	}
 
 	ret = clk_enable(q->clk);
 	if (ret) {
 		clk_disable(q->clk_en);
+		mutex_unlock(&q->lock);
 		return ret;
 	}
 
@@ -781,6 +787,7 @@ static void fsl_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
 
 	clk_disable(q->clk);
 	clk_disable(q->clk_en);
+	mutex_unlock(&q->lock);
 }
 
 static int fsl_qspi_probe(struct platform_device *pdev)
@@ -864,6 +871,8 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 	if (of_get_property(np, "fsl,qspi-has-second-chip", NULL))
 		q->has_second_chip = true;
 
+	mutex_init(&q->lock);
+
 	/* iterate the subnodes. */
 	for_each_available_child_of_node(dev->of_node, np) {
 		char modalias[40];
@@ -892,24 +901,24 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 
 		ret = of_modalias_node(np, modalias, sizeof(modalias));
 		if (ret < 0)
-			goto irq_failed;
+			goto mutex_failed;
 
 		ret = of_property_read_u32(np, "spi-max-frequency",
 				&q->clk_rate);
 		if (ret < 0)
-			goto irq_failed;
+			goto mutex_failed;
 
 		/* set the chip address for READID */
 		fsl_qspi_set_base_addr(q, nor);
 
 		ret = spi_nor_scan(nor, modalias, SPI_NOR_QUAD);
 		if (ret)
-			goto irq_failed;
+			goto mutex_failed;
 
 		ppdata.of_node = np;
 		ret = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
 		if (ret)
-			goto irq_failed;
+			goto mutex_failed;
 
 		/* Set the correct NOR size now. */
 		if (q->nor_size == 0) {
@@ -950,6 +959,8 @@ last_init_failed:
 			i *= 2;
 		mtd_device_unregister(&q->mtd[i]);
 	}
+mutex_failed:
+	mutex_destroy(&q->lock);
 irq_failed:
 	clk_disable_unprepare(q->clk);
 clk_failed:
@@ -973,6 +984,7 @@ static int fsl_qspi_remove(struct platform_device *pdev)
 	writel(QUADSPI_MCR_MDIS_MASK, q->iobase + QUADSPI_MCR);
 	writel(0x0, q->iobase + QUADSPI_RSER);
 
+	mutex_destroy(&q->lock);
 	clk_unprepare(q->clk);
 	clk_unprepare(q->clk_en);
 	return 0;
-- 
1.9.1

Re: [PATCH V3] mtd: fsl-quadspi: Access multiple chips simultaneously

[Brian Norris]

Hi Han,

On Wed, May 13, 2015 at 02:40:57PM -0500, Han Xu wrote:
> Add supports for simultaneous access to multiple chips. Need to lock
> the mutex before any quad spi operations and unlock the mutex after
> operations complete.
> 
> Signed-off-by: Han Xu <b45815@freescale.com>
> ---
> v2 -- > v3
> 1. Unlock the mutex if errors occur.
> 2. Destroy the mutex when module remove or probe fail.
> 
> v1 -- > v2
> 1. Rebase to latest l2-mtd code and resend.
> ---
> 
> 
>  drivers/mtd/spi-nor/fsl-quadspi.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index 52a872f..e6fc064 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
...
> @@ -761,13 +763,17 @@ static int fsl_qspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
>  	struct fsl_qspi *q = nor->priv;
>  	int ret;
>  
> +	mutex_lock(&q->lock);
>  	ret = clk_enable(q->clk_en);
> -	if (ret)
> +	if (ret) {
> +		mutex_unlock(&q->lock);
>  		return ret;
> +	}
>  
>  	ret = clk_enable(q->clk);
>  	if (ret) {
>  		clk_disable(q->clk_en);
> +		mutex_unlock(&q->lock);
>  		return ret;
>  	}
>  

I think it'd be better to have a single error path in this function now.

Otherwise, I think this is OK. I can push the appended patch instead, if
that's OK with you.

Signed-off-by: Han Xu <b45815@freescale.com>
[Brian: reworked err path in fsl_qspi_prep()]
Signed-off-by: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/spi-nor/fsl-quadspi.c | 32 +++++++++++++++++++++++---------
 1 file changed, 23 insertions(+), 9 deletions(-)

diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
index 52a872fa1b6e..4fe13dd535f8 100644
--- a/drivers/mtd/spi-nor/fsl-quadspi.c
+++ b/drivers/mtd/spi-nor/fsl-quadspi.c
@@ -26,6 +26,7 @@
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/partitions.h>
 #include <linux/mtd/spi-nor.h>
+#include <linux/mutex.h>
 
 /* The registers */
 #define QUADSPI_MCR			0x00
@@ -233,6 +234,7 @@ struct fsl_qspi {
 	u32 clk_rate;
 	unsigned int chip_base_addr; /* We may support two chips. */
 	bool has_second_chip;
+	struct mutex lock;
 };
 
 static inline int is_vybrid_qspi(struct fsl_qspi *q)
@@ -761,18 +763,24 @@ static int fsl_qspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
 	struct fsl_qspi *q = nor->priv;
 	int ret;
 
+	mutex_lock(&q->lock);
 	ret = clk_enable(q->clk_en);
 	if (ret)
-		return ret;
+		goto err_mutex;
 
 	ret = clk_enable(q->clk);
-	if (ret) {
-		clk_disable(q->clk_en);
-		return ret;
-	}
+	if (ret)
+		goto err_clk;
 
 	fsl_qspi_set_base_addr(q, nor);
 	return 0;
+
+err_clk:
+	clk_disable(q->clk_en);
+err_mutex:
+	mutex_unlock(&q->lock);
+
+	return ret;
 }
 
 static void fsl_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
@@ -781,6 +789,7 @@ static void fsl_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
 
 	clk_disable(q->clk);
 	clk_disable(q->clk_en);
+	mutex_unlock(&q->lock);
 }
 
 static int fsl_qspi_probe(struct platform_device *pdev)
@@ -864,6 +873,8 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 	if (of_get_property(np, "fsl,qspi-has-second-chip", NULL))
 		q->has_second_chip = true;
 
+	mutex_init(&q->lock);
+
 	/* iterate the subnodes. */
 	for_each_available_child_of_node(dev->of_node, np) {
 		char modalias[40];
@@ -892,24 +903,24 @@ static int fsl_qspi_probe(struct platform_device *pdev)
 
 		ret = of_modalias_node(np, modalias, sizeof(modalias));
 		if (ret < 0)
-			goto irq_failed;
+			goto mutex_failed;
 
 		ret = of_property_read_u32(np, "spi-max-frequency",
 				&q->clk_rate);
 		if (ret < 0)
-			goto irq_failed;
+			goto mutex_failed;
 
 		/* set the chip address for READID */
 		fsl_qspi_set_base_addr(q, nor);
 
 		ret = spi_nor_scan(nor, modalias, SPI_NOR_QUAD);
 		if (ret)
-			goto irq_failed;
+			goto mutex_failed;
 
 		ppdata.of_node = np;
 		ret = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
 		if (ret)
-			goto irq_failed;
+			goto mutex_failed;
 
 		/* Set the correct NOR size now. */
 		if (q->nor_size == 0) {
@@ -950,6 +961,8 @@ last_init_failed:
 			i *= 2;
 		mtd_device_unregister(&q->mtd[i]);
 	}
+mutex_failed:
+	mutex_destroy(&q->lock);
 irq_failed:
 	clk_disable_unprepare(q->clk);
 clk_failed:
@@ -973,6 +986,7 @@ static int fsl_qspi_remove(struct platform_device *pdev)
 	writel(QUADSPI_MCR_MDIS_MASK, q->iobase + QUADSPI_MCR);
 	writel(0x0, q->iobase + QUADSPI_RSER);
 
+	mutex_destroy(&q->lock);
 	clk_unprepare(q->clk);
 	clk_unprepare(q->clk_en);
 	return 0;
-- 
1.9.1

Re: [PATCH V3] mtd: fsl-quadspi: Access multiple chips simultaneously

[Han Xu]

On Wed, May 20, 2015 at 4:52 PM, Brian Norris
<computersforpeace@gmail.com> wrote:
> Hi Han,
>
> On Wed, May 13, 2015 at 02:40:57PM -0500, Han Xu wrote:
>> Add supports for simultaneous access to multiple chips. Need to lock
>> the mutex before any quad spi operations and unlock the mutex after
>> operations complete.
>>
>> Signed-off-by: Han Xu <b45815@freescale.com>
>> ---
>> v2 -- > v3
>> 1. Unlock the mutex if errors occur.
>> 2. Destroy the mutex when module remove or probe fail.
>>
>> v1 -- > v2
>> 1. Rebase to latest l2-mtd code and resend.
>> ---
>>
>>
>>  drivers/mtd/spi-nor/fsl-quadspi.c | 22 +++++++++++++++++-----
>>  1 file changed, 17 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
>> index 52a872f..e6fc064 100644
>> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
>> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> ...
>> @@ -761,13 +763,17 @@ static int fsl_qspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
>>       struct fsl_qspi *q = nor->priv;
>>       int ret;
>>
>> +     mutex_lock(&q->lock);
>>       ret = clk_enable(q->clk_en);
>> -     if (ret)
>> +     if (ret) {
>> +             mutex_unlock(&q->lock);
>>               return ret;
>> +     }
>>
>>       ret = clk_enable(q->clk);
>>       if (ret) {
>>               clk_disable(q->clk_en);
>> +             mutex_unlock(&q->lock);
>>               return ret;
>>       }
>>
>
> I think it'd be better to have a single error path in this function now.
>
> Otherwise, I think this is OK. I can push the appended patch instead, if
> that's OK with you.

Hi Brian, I think that's fine. I will submit another patch to improve it.

>
> Signed-off-by: Han Xu <b45815@freescale.com>
> [Brian: reworked err path in fsl_qspi_prep()]
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 32 +++++++++++++++++++++++---------
>  1 file changed, 23 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/fsl-quadspi.c b/drivers/mtd/spi-nor/fsl-quadspi.c
> index 52a872fa1b6e..4fe13dd535f8 100644
> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> @@ -26,6 +26,7 @@
>  #include <linux/mtd/mtd.h>
>  #include <linux/mtd/partitions.h>
>  #include <linux/mtd/spi-nor.h>
> +#include <linux/mutex.h>
>
>  /* The registers */
>  #define QUADSPI_MCR                    0x00
> @@ -233,6 +234,7 @@ struct fsl_qspi {
>         u32 clk_rate;
>         unsigned int chip_base_addr; /* We may support two chips. */
>         bool has_second_chip;
> +       struct mutex lock;
>  };
>
>  static inline int is_vybrid_qspi(struct fsl_qspi *q)
> @@ -761,18 +763,24 @@ static int fsl_qspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
>         struct fsl_qspi *q = nor->priv;
>         int ret;
>
> +       mutex_lock(&q->lock);
>         ret = clk_enable(q->clk_en);
>         if (ret)
> -               return ret;
> +               goto err_mutex;
>
>         ret = clk_enable(q->clk);
> -       if (ret) {
> -               clk_disable(q->clk_en);
> -               return ret;
> -       }
> +       if (ret)
> +               goto err_clk;
>
>         fsl_qspi_set_base_addr(q, nor);
>         return 0;
> +
> +err_clk:
> +       clk_disable(q->clk_en);
> +err_mutex:
> +       mutex_unlock(&q->lock);
> +
> +       return ret;
>  }
>
>  static void fsl_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
> @@ -781,6 +789,7 @@ static void fsl_qspi_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
>
>         clk_disable(q->clk);
>         clk_disable(q->clk_en);
> +       mutex_unlock(&q->lock);
>  }
>
>  static int fsl_qspi_probe(struct platform_device *pdev)
> @@ -864,6 +873,8 @@ static int fsl_qspi_probe(struct platform_device *pdev)
>         if (of_get_property(np, "fsl,qspi-has-second-chip", NULL))
>                 q->has_second_chip = true;
>
> +       mutex_init(&q->lock);
> +
>         /* iterate the subnodes. */
>         for_each_available_child_of_node(dev->of_node, np) {
>                 char modalias[40];
> @@ -892,24 +903,24 @@ static int fsl_qspi_probe(struct platform_device *pdev)
>
>                 ret = of_modalias_node(np, modalias, sizeof(modalias));
>                 if (ret < 0)
> -                       goto irq_failed;
> +                       goto mutex_failed;
>
>                 ret = of_property_read_u32(np, "spi-max-frequency",
>                                 &q->clk_rate);
>                 if (ret < 0)
> -                       goto irq_failed;
> +                       goto mutex_failed;
>
>                 /* set the chip address for READID */
>                 fsl_qspi_set_base_addr(q, nor);
>
>                 ret = spi_nor_scan(nor, modalias, SPI_NOR_QUAD);
>                 if (ret)
> -                       goto irq_failed;
> +                       goto mutex_failed;
>
>                 ppdata.of_node = np;
>                 ret = mtd_device_parse_register(mtd, NULL, &ppdata, NULL, 0);
>                 if (ret)
> -                       goto irq_failed;
> +                       goto mutex_failed;
>
>                 /* Set the correct NOR size now. */
>                 if (q->nor_size == 0) {
> @@ -950,6 +961,8 @@ last_init_failed:
>                         i *= 2;
>                 mtd_device_unregister(&q->mtd[i]);
>         }
> +mutex_failed:
> +       mutex_destroy(&q->lock);
>  irq_failed:
>         clk_disable_unprepare(q->clk);
>  clk_failed:
> @@ -973,6 +986,7 @@ static int fsl_qspi_remove(struct platform_device *pdev)
>         writel(QUADSPI_MCR_MDIS_MASK, q->iobase + QUADSPI_MCR);
>         writel(0x0, q->iobase + QUADSPI_RSER);
>
> +       mutex_destroy(&q->lock);
>         clk_unprepare(q->clk);
>         clk_unprepare(q->clk_en);
>         return 0;
> --
> 1.9.1
>
>
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH V3] mtd: fsl-quadspi: Access multiple chips simultaneously

[Brian Norris]

Hi Han,

On Wed, May 20, 2015 at 05:28:20PM -0500, Han Xu wrote:
> On Wed, May 20, 2015 at 4:52 PM, Brian Norris
> <computersforpeace@gmail.com> wrote:
> > On Wed, May 13, 2015 at 02:40:57PM -0500, Han Xu wrote:
> >> --- a/drivers/mtd/spi-nor/fsl-quadspi.c
> >> +++ b/drivers/mtd/spi-nor/fsl-quadspi.c
> > ...
> >> @@ -761,13 +763,17 @@ static int fsl_qspi_prep(struct spi_nor *nor, enum spi_nor_ops ops)
> >>       struct fsl_qspi *q = nor->priv;
> >>       int ret;
> >>
> >> +     mutex_lock(&q->lock);
> >>       ret = clk_enable(q->clk_en);
> >> -     if (ret)
> >> +     if (ret) {
> >> +             mutex_unlock(&q->lock);
> >>               return ret;
> >> +     }
> >>
> >>       ret = clk_enable(q->clk);
> >>       if (ret) {
> >>               clk_disable(q->clk_en);
> >> +             mutex_unlock(&q->lock);
> >>               return ret;
> >>       }
> >>
> >
> > I think it'd be better to have a single error path in this function now.
> >
> > Otherwise, I think this is OK. I can push the appended patch instead, if
> > that's OK with you.
> 
> Hi Brian, I think that's fine. I will submit another patch to improve it.

OK. Resubmit if you'd like, or just ack my version. I'd like your
Tested-by if possible.

> >
> > Signed-off-by: Han Xu <b45815@freescale.com>
> > [Brian: reworked err path in fsl_qspi_prep()]
> > Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> > ---
> >  drivers/mtd/spi-nor/fsl-quadspi.c | 32 +++++++++++++++++++++++---------
> >  1 file changed, 23 insertions(+), 9 deletions(-)
[...]

Brian

Re: [PATCH V3] mtd: fsl-quadspi: Access multiple chips simultaneously

[Brian Norris]

On Wed, May 20, 2015 at 02:52:50PM -0700, Brian Norris wrote:
> On Wed, May 13, 2015 at 02:40:57PM -0500, Han Xu wrote:
> I think it'd be better to have a single error path in this function now.
> 
> Otherwise, I think this is OK. I can push the appended patch instead, if
> that's OK with you.
> 
> Signed-off-by: Han Xu <b45815@freescale.com>
> [Brian: reworked err path in fsl_qspi_prep()]
> Signed-off-by: Brian Norris <computersforpeace@gmail.com>
> ---
>  drivers/mtd/spi-nor/fsl-quadspi.c | 32 +++++++++++++++++++++++---------
>  1 file changed, 23 insertions(+), 9 deletions(-)
...

Pushed this version to l2-mtd.git/next.