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 |
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/
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.
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
> -----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
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
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.
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 --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)