diff mbox series

[5/6] mtd: spi-nor: Add s3an_post_sfdp_fixups()

Message ID 20190731091145.27374-6-tudor.ambarus@microchip.com
State Changes Requested
Delegated to: Ambarus Tudor
Headers show
Series mtd: spi-nor: move manuf out of the core - batch 2 | expand

Commit Message

Tudor Ambarus July 31, 2019, 9:12 a.m. UTC 
From: Tudor Ambarus <tudor.ambarus@microchip.com>

s3an_nor_scan() was overriding the opcode selection done in
spi_nor_default_setup(). Set nor->setup() method in order to
avoid unnecessary call to spi_nor_default_setup().

Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 35 +++++++++++++++++++++++++----------
 1 file changed, 25 insertions(+), 10 deletions(-)

Comments

Naga Sureshkumar Relli July 31, 2019, 12:31 p.m. UTC | #1 
Hi Tudor,

Thanks for the updates. With these kind of updates, we can add Vendor specific
Code easily, like Xilinx Dual parallel and stacked modes.
In these configurations we need to tweak the nor parameters like page_size, sectors etc.
So with the help of these patches. we can easily update these parameters.

> -----Original Message-----
> From: linux-mtd <linux-mtd-bounces@lists.infradead.org> On Behalf Of
> Tudor.Ambarus@microchip.com
> Sent: Wednesday, July 31, 2019 2:42 PM
> To: boris.brezillon@collabora.com; marek.vasut@gmail.com; vigneshr@ti.com
> Cc: Tudor.Ambarus@microchip.com; richard@nod.at; linux-kernel@vger.kernel.org; linux-
> mtd@lists.infradead.org; miquel.raynal@bootlin.com; computersforpeace@gmail.com;
> dwmw2@infradead.org
> Subject: [PATCH 5/6] mtd: spi-nor: Add s3an_post_sfdp_fixups()
> 
> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> s3an_nor_scan() was overriding the opcode selection done in spi_nor_default_setup(). Set nor-
> >setup() method in order to avoid unnecessary call to spi_nor_default_setup().
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 35 +++++++++++++++++++++++++----------
>  1 file changed, 25 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c index
> 0ff474e5e4f5..5fea5d7ce2cb 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2795,7 +2795,9 @@ static int spi_nor_check(struct spi_nor *nor)
>  	return 0;
>  }
> 
> -static int s3an_nor_scan(struct spi_nor *nor)
> +static int s3an_nor_setup(struct spi_nor *nor,
> +			  const struct spi_nor_flash_parameter *params,
> +			  const struct spi_nor_hwcaps *hwcaps)
>  {
>  	int ret;
>  	u8 val;
> @@ -4393,6 +4395,11 @@ static void spansion_post_sfdp_fixups(struct spi_nor *nor)
>  	nor->mtd.erasesize = nor->info->sector_size;  }
> 
> +static void s3an_post_sfdp_fixups(struct spi_nor *nor) {
> +	nor->setup = s3an_nor_setup;
> +}
> +
>  static void
>  spi_nor_manufacturer_post_sfdp_fixups(struct spi_nor *nor,
>  				      struct spi_nor_flash_parameter *params) @@ -4405,6
> +4412,9 @@ spi_nor_manufacturer_post_sfdp_fixups(struct spi_nor *nor,
>  	default:
>  		break;
>  	}
> +
> +	if (nor->info->flags & SPI_S3AN)
> +		s3an_post_sfdp_fixups(nor);
>  }
>
Instead of checking the flags, why can't we call directly the nor_fixups?
like Boris implementation nor->info->fixups->post_sfdp()
https://patchwork.ozlabs.org/patch/1009291/

Thanks,
Naga Sureshkumar Relli

>  static void spi_nor_post_sfdp_fixups(struct spi_nor *nor, @@ -4582,9 +4592,9 @@ static
> int spi_nor_select_erase(struct spi_nor *nor, u32 wanted_size)
>  	return 0;
>  }
> 
> -static int spi_nor_setup(struct spi_nor *nor,
> -			 const struct spi_nor_flash_parameter *params,
> -			 const struct spi_nor_hwcaps *hwcaps)
> +static int spi_nor_default_setup(struct spi_nor *nor,
> +				 const struct spi_nor_flash_parameter *params,
> +				 const struct spi_nor_hwcaps *hwcaps)
>  {
>  	u32 ignored_mask, shared_mask;
>  	int err;
> @@ -4641,6 +4651,16 @@ static int spi_nor_setup(struct spi_nor *nor,
>  	return err;
>  }
> 
> +static int spi_nor_setup(struct spi_nor *nor,
> +			 const struct spi_nor_flash_parameter *params,
> +			 const struct spi_nor_hwcaps *hwcaps) {
> +	if (!nor->setup)
> +		return 0;
> +
> +	return nor->setup(nor, params, hwcaps); }
> +
>  static int spi_nor_disable_write_protection(struct spi_nor *nor)  {
>  	if (!nor->disable_write_protection)
> @@ -4804,6 +4824,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>  	/* Kept only for backward compatibility purpose. */
>  	nor->quad_enable = spansion_quad_enable;
>  	nor->set_4byte = spansion_set_4byte;
> +	nor->setup = spi_nor_default_setup;
> 
>  	/* Default locking operations. */
>  	if (info->flags & SPI_NOR_HAS_LOCK) {
> @@ -4905,12 +4926,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>  		return -EINVAL;
>  	}
> 
> -	if (info->flags & SPI_S3AN) {
> -		ret = s3an_nor_scan(nor);
> -		if (ret)
> -			return ret;
> -	}
> -
>  	/* Send all the required SPI flash commands to initialize device */
>  	ret = spi_nor_init(nor);
>  	if (ret)
> --
> 2.9.5
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
Boris Brezillon July 31, 2019, 12:38 p.m. UTC | #2 
On Wed, 31 Jul 2019 12:31:19 +0000
Naga Sureshkumar Relli <nagasure@xilinx.com> wrote:

> Hi Tudor,
> 
> Thanks for the updates. With these kind of updates, we can add Vendor specific
> Code easily, like Xilinx Dual parallel and stacked modes.
> In these configurations we need to tweak the nor parameters like page_size, sectors etc.
> So with the help of these patches. we can easily update these parameters.

Absolutely not. This is just a solution to recover from broken SFDP
tables.
Tudor Ambarus July 31, 2019, 1:06 p.m. UTC | #3 
Hi, Naga,

On 07/31/2019 03:31 PM, Naga Sureshkumar Relli wrote:
>> +	if (nor->info->flags & SPI_S3AN)
>> +		s3an_post_sfdp_fixups(nor);
>>  }
>>
> Instead of checking the flags, why can't we call directly the nor_fixups?
> like Boris implementation nor->info->fixups->post_sfdp()
> https://patchwork.ozlabs.org/patch/1009291/

This check will vanish and nor->info->fixups->post_sfdp() will be called
directly once I'll respin the manufacturer driver part. post_sfdp() will set
just flash parameters. Check Boris' patch at
https://patchwork.ozlabs.org/patch/1009295/

I'll try to respin the rest of Boris' patches sometime at the beginning of the
next week.

Cheers,
ta
Naga Sureshkumar Relli Aug. 1, 2019, 4:47 a.m. UTC | #4 
> -----Original Message-----
> From: Boris Brezillon <boris.brezillon@collabora.com>
> Sent: Wednesday, July 31, 2019 6:08 PM
> To: Naga Sureshkumar Relli <nagasure@xilinx.com>
> Cc: Tudor.Ambarus@microchip.com; marek.vasut@gmail.com; vigneshr@ti.com;
> richard@nod.at; linux-kernel@vger.kernel.org; linux-mtd@lists.infradead.org;
> miquel.raynal@bootlin.com; computersforpeace@gmail.com; dwmw2@infradead.org
> Subject: Re: [PATCH 5/6] mtd: spi-nor: Add s3an_post_sfdp_fixups()
> 
> On Wed, 31 Jul 2019 12:31:19 +0000
> Naga Sureshkumar Relli <nagasure@xilinx.com> wrote:
> 
> > Hi Tudor,
> >
> > Thanks for the updates. With these kind of updates, we can add Vendor
> > specific Code easily, like Xilinx Dual parallel and stacked modes.
> > In these configurations we need to tweak the nor parameters like page_size, sectors etc.
> > So with the help of these patches. we can easily update these parameters.
> 
> Absolutely not. This is just a solution to recover from broken SFDP tables.
Ok.

Thanks,
Naga Sureshkumar Relli
Naga Sureshkumar Relli Aug. 1, 2019, 4:47 a.m. UTC | #5 
Hi Tudor,

> -----Original Message-----
> From: Tudor.Ambarus@microchip.com <Tudor.Ambarus@microchip.com>
> Sent: Wednesday, July 31, 2019 6:37 PM
> To: Naga Sureshkumar Relli <nagasure@xilinx.com>; boris.brezillon@collabora.com;
> marek.vasut@gmail.com; vigneshr@ti.com
> Cc: richard@nod.at; linux-kernel@vger.kernel.org; linux-mtd@lists.infradead.org;
> miquel.raynal@bootlin.com; computersforpeace@gmail.com; dwmw2@infradead.org
> Subject: Re: [PATCH 5/6] mtd: spi-nor: Add s3an_post_sfdp_fixups()
> 
> Hi, Naga,
> 
> On 07/31/2019 03:31 PM, Naga Sureshkumar Relli wrote:
> >> +	if (nor->info->flags & SPI_S3AN)
> >> +		s3an_post_sfdp_fixups(nor);
> >>  }
> >>
> > Instead of checking the flags, why can't we call directly the nor_fixups?
> > like Boris implementation nor->info->fixups->post_sfdp()
> > https://patchwork.ozlabs.org/patch/1009291/
> 
> This check will vanish and nor->info->fixups->post_sfdp() will be called directly once I'll
> respin the manufacturer driver part. post_sfdp() will set just flash parameters. Check Boris'
> patch at https://patchwork.ozlabs.org/patch/1009295/
> 
> I'll try to respin the rest of Boris' patches sometime at the beginning of the next week.
Ok, Thanks.

Regards,
Naga Sureshkumar Relli
> 
> Cheers,
> ta
Boris Brezillon Aug. 1, 2019, 6:42 a.m. UTC | #6 
On Wed, 31 Jul 2019 09:12:16 +0000
<Tudor.Ambarus@microchip.com> wrote:

> From: Tudor Ambarus <tudor.ambarus@microchip.com>
> 
> s3an_nor_scan() was overriding the opcode selection done in
> spi_nor_default_setup(). Set nor->setup() method in order to
> avoid unnecessary call to spi_nor_default_setup().
> 
> Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 35 +++++++++++++++++++++++++----------
>  1 file changed, 25 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 0ff474e5e4f5..5fea5d7ce2cb 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2795,7 +2795,9 @@ static int spi_nor_check(struct spi_nor *nor)
>  	return 0;
>  }
>  
> -static int s3an_nor_scan(struct spi_nor *nor)
> +static int s3an_nor_setup(struct spi_nor *nor,
> +			  const struct spi_nor_flash_parameter *params,
> +			  const struct spi_nor_hwcaps *hwcaps)
>  {
>  	int ret;
>  	u8 val;
> @@ -4393,6 +4395,11 @@ static void spansion_post_sfdp_fixups(struct spi_nor *nor)
>  	nor->mtd.erasesize = nor->info->sector_size;
>  }
>  
> +static void s3an_post_sfdp_fixups(struct spi_nor *nor)
> +{
> +	nor->setup = s3an_nor_setup;
> +}
> +
>  static void
>  spi_nor_manufacturer_post_sfdp_fixups(struct spi_nor *nor,
>  				      struct spi_nor_flash_parameter *params)
> @@ -4405,6 +4412,9 @@ spi_nor_manufacturer_post_sfdp_fixups(struct spi_nor *nor,
>  	default:
>  		break;
>  	}
> +
> +	if (nor->info->flags & SPI_S3AN)
> +		s3an_post_sfdp_fixups(nor);
>  }
>  
>  static void spi_nor_post_sfdp_fixups(struct spi_nor *nor,
> @@ -4582,9 +4592,9 @@ static int spi_nor_select_erase(struct spi_nor *nor, u32 wanted_size)
>  	return 0;
>  }
>  
> -static int spi_nor_setup(struct spi_nor *nor,
> -			 const struct spi_nor_flash_parameter *params,
> -			 const struct spi_nor_hwcaps *hwcaps)
> +static int spi_nor_default_setup(struct spi_nor *nor,
> +				 const struct spi_nor_flash_parameter *params,
> +				 const struct spi_nor_hwcaps *hwcaps)
>  {
>  	u32 ignored_mask, shared_mask;
>  	int err;
> @@ -4641,6 +4651,16 @@ static int spi_nor_setup(struct spi_nor *nor,
>  	return err;
>  }
>  
> +static int spi_nor_setup(struct spi_nor *nor,
> +			 const struct spi_nor_flash_parameter *params,
> +			 const struct spi_nor_hwcaps *hwcaps)
> +{
> +	if (!nor->setup)
> +		return 0;
> +
> +	return nor->setup(nor, params, hwcaps);
> +}
> +
>  static int spi_nor_disable_write_protection(struct spi_nor *nor)
>  {
>  	if (!nor->disable_write_protection)
> @@ -4804,6 +4824,7 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>  	/* Kept only for backward compatibility purpose. */
>  	nor->quad_enable = spansion_quad_enable;
>  	nor->set_4byte = spansion_set_4byte;
> +	nor->setup = spi_nor_default_setup;
>  
>  	/* Default locking operations. */
>  	if (info->flags & SPI_NOR_HAS_LOCK) {
> @@ -4905,12 +4926,6 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
>  		return -EINVAL;
>  	}
>  
> -	if (info->flags & SPI_S3AN) {
> -		ret = s3an_nor_scan(nor);
> -		if (ret)
> -			return ret;
> -	}
> -
>  	/* Send all the required SPI flash commands to initialize device */
>  	ret = spi_nor_init(nor);
>  	if (ret)

Almost all of this (except the s3an specific bits) should be done in
the previous patch. So I'll put a condition on the R-b I placed on patch
4: some of this code should be moved there.
Tudor Ambarus Aug. 5, 2019, 6:40 a.m. UTC | #7 
On 08/01/2019 09:42 AM, Boris Brezillon wrote:
> Almost all of this (except the s3an specific bits) should be done in
> the previous patch. So I'll put a condition on the R-b I placed on patch
> 4: some of this code should be moved there.

You're right, will do.

Cheers,
ta
diff mbox series

Patch

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index 0ff474e5e4f5..5fea5d7ce2cb 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -2795,7 +2795,9 @@  static int spi_nor_check(struct spi_nor *nor)
 	return 0;
 }
 
-static int s3an_nor_scan(struct spi_nor *nor)
+static int s3an_nor_setup(struct spi_nor *nor,
+			  const struct spi_nor_flash_parameter *params,
+			  const struct spi_nor_hwcaps *hwcaps)
 {
 	int ret;
 	u8 val;
@@ -4393,6 +4395,11 @@  static void spansion_post_sfdp_fixups(struct spi_nor *nor)
 	nor->mtd.erasesize = nor->info->sector_size;
 }
 
+static void s3an_post_sfdp_fixups(struct spi_nor *nor)
+{
+	nor->setup = s3an_nor_setup;
+}
+
 static void
 spi_nor_manufacturer_post_sfdp_fixups(struct spi_nor *nor,
 				      struct spi_nor_flash_parameter *params)
@@ -4405,6 +4412,9 @@  spi_nor_manufacturer_post_sfdp_fixups(struct spi_nor *nor,
 	default:
 		break;
 	}
+
+	if (nor->info->flags & SPI_S3AN)
+		s3an_post_sfdp_fixups(nor);
 }
 
 static void spi_nor_post_sfdp_fixups(struct spi_nor *nor,
@@ -4582,9 +4592,9 @@  static int spi_nor_select_erase(struct spi_nor *nor, u32 wanted_size)
 	return 0;
 }
 
-static int spi_nor_setup(struct spi_nor *nor,
-			 const struct spi_nor_flash_parameter *params,
-			 const struct spi_nor_hwcaps *hwcaps)
+static int spi_nor_default_setup(struct spi_nor *nor,
+				 const struct spi_nor_flash_parameter *params,
+				 const struct spi_nor_hwcaps *hwcaps)
 {
 	u32 ignored_mask, shared_mask;
 	int err;
@@ -4641,6 +4651,16 @@  static int spi_nor_setup(struct spi_nor *nor,
 	return err;
 }
 
+static int spi_nor_setup(struct spi_nor *nor,
+			 const struct spi_nor_flash_parameter *params,
+			 const struct spi_nor_hwcaps *hwcaps)
+{
+	if (!nor->setup)
+		return 0;
+
+	return nor->setup(nor, params, hwcaps);
+}
+
 static int spi_nor_disable_write_protection(struct spi_nor *nor)
 {
 	if (!nor->disable_write_protection)
@@ -4804,6 +4824,7 @@  int spi_nor_scan(struct spi_nor *nor, const char *name,
 	/* Kept only for backward compatibility purpose. */
 	nor->quad_enable = spansion_quad_enable;
 	nor->set_4byte = spansion_set_4byte;
+	nor->setup = spi_nor_default_setup;
 
 	/* Default locking operations. */
 	if (info->flags & SPI_NOR_HAS_LOCK) {
@@ -4905,12 +4926,6 @@  int spi_nor_scan(struct spi_nor *nor, const char *name,
 		return -EINVAL;
 	}
 
-	if (info->flags & SPI_S3AN) {
-		ret = s3an_nor_scan(nor);
-		if (ret)
-			return ret;
-	}
-
 	/* Send all the required SPI flash commands to initialize device */
 	ret = spi_nor_init(nor);
 	if (ret)