[PATCH] mtd: spi-nor: gigadevice: gd25q256: replace gd25q256_default_init with gd25q256_post_bfpt

[PATCH] mtd: spi-nor: gigadevice: gd25q256: replace gd25q256_default_init with gd25q256_post_bfpt

[yaliang.wang]

From: Yaliang Wang <Yaliang.Wang@windriver.com>

When utilizing PARSE_SFDP to initialize the flash parameter, the
deprecated initializing method spi_nor_init_params_deprecated() and the
function spi_nor_manufacturer_init_params() within it will never be
executed, which results in the default_init hook function will also never
be executed. As we do have quad enable function defined in BFPT, the
post_bfpt hook will be the right place to tweak the function.

Cc: stable@vger.kernel.org
Fixes: 047275f7de18 ("mtd: spi-nor: gigadevice: gd25q256: Init flash based on SFDP")
Signed-off-by: Yaliang Wang <Yaliang.Wang@windriver.com>
---
 drivers/mtd/spi-nor/gigadevice.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/spi-nor/gigadevice.c b/drivers/mtd/spi-nor/gigadevice.c
index 119b38e6fc2a..d9b2e36971ea 100644
--- a/drivers/mtd/spi-nor/gigadevice.c
+++ b/drivers/mtd/spi-nor/gigadevice.c
@@ -8,19 +8,22 @@
 
 #include "core.h"
 
-static void gd25q256_default_init(struct spi_nor *nor)
+static int
+gd25q256_post_bfpt(struct spi_nor *nor,
+			    const struct sfdp_parameter_header *bfpt_header,
+			    const struct sfdp_bfpt *bfpt)
 {
 	/*
 	 * Some manufacturer like GigaDevice may use different
 	 * bit to set QE on different memories, so the MFR can't
 	 * indicate the quad_enable method for this case, we need
-	 * to set it in the default_init fixup hook.
+	 * to set it in the post_bfpt fixup hook.
 	 */
 	nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
 }
 
 static const struct spi_nor_fixups gd25q256_fixups = {
-	.default_init = gd25q256_default_init,
+	.post_bfpt = gd25q256_post_bfpt,
 };
 
 static const struct flash_info gigadevice_nor_parts[] = {
-- 
2.34.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: gigadevice: gd25q256: replace gd25q256_default_init with gd25q256_post_bfpt

[kernel test robot]

Hi,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.0-rc5 next-20220912]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/yaliang-wang-windriver-com/mtd-spi-nor-gigadevice-gd25q256-replace-gd25q256_default_init-with-gd25q256_post_bfpt/20220912-223028
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 80e78fcce86de0288793a0ef0f6acf37656ee4cf
config: i386-randconfig-a016-20220912 (https://download.01.org/0day-ci/archive/20220913/202209130732.cQpPG34i-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/0238f8172a76ee5a84dda79b45911a2b63d59721
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review yaliang-wang-windriver-com/mtd-spi-nor-gigadevice-gd25q256-replace-gd25q256_default_init-with-gd25q256_post_bfpt/20220912-223028
        git checkout 0238f8172a76ee5a84dda79b45911a2b63d59721
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/mtd/spi-nor/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/mtd/spi-nor/gigadevice.c:23:1: error: non-void function does not return a value [-Werror,-Wreturn-type]
   }
   ^
   1 error generated.


vim +23 drivers/mtd/spi-nor/gigadevice.c

acb96ecd59f7fd Boris Brezillon 2020-03-13  10  
0238f8172a76ee Yaliang Wang    2022-09-12  11  static int
0238f8172a76ee Yaliang Wang    2022-09-12  12  gd25q256_post_bfpt(struct spi_nor *nor,
0238f8172a76ee Yaliang Wang    2022-09-12  13  			    const struct sfdp_parameter_header *bfpt_header,
0238f8172a76ee Yaliang Wang    2022-09-12  14  			    const struct sfdp_bfpt *bfpt)
acb96ecd59f7fd Boris Brezillon 2020-03-13  15  {
acb96ecd59f7fd Boris Brezillon 2020-03-13  16  	/*
acb96ecd59f7fd Boris Brezillon 2020-03-13  17  	 * Some manufacturer like GigaDevice may use different
acb96ecd59f7fd Boris Brezillon 2020-03-13  18  	 * bit to set QE on different memories, so the MFR can't
acb96ecd59f7fd Boris Brezillon 2020-03-13  19  	 * indicate the quad_enable method for this case, we need
0238f8172a76ee Yaliang Wang    2022-09-12  20  	 * to set it in the post_bfpt fixup hook.
acb96ecd59f7fd Boris Brezillon 2020-03-13  21  	 */
829ec6408dc58d Tudor Ambarus   2020-03-13  22  	nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
acb96ecd59f7fd Boris Brezillon 2020-03-13 @23  }
acb96ecd59f7fd Boris Brezillon 2020-03-13  24  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: gigadevice: gd25q256: replace gd25q256_default_init with gd25q256_post_bfpt

[Yaliang Wang]

Sorry for the error, I'll make another patch to fix the error.

Regards, Yaliang.

On 9/13/22 07:18, kernel test robot wrote:
> [Please note: This e-mail is from an EXTERNAL e-mail address]
>
> Hi,
>
> Thank you for the patch! Yet something to improve:
>
> [auto build test ERROR on linus/master]
> [also build test ERROR on v6.0-rc5 next-20220912]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url:    https://github.com/intel-lab-lkp/linux/commits/yaliang-wang-windriver-com/mtd-spi-nor-gigadevice-gd25q256-replace-gd25q256_default_init-with-gd25q256_post_bfpt/20220912-223028
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 80e78fcce86de0288793a0ef0f6acf37656ee4cf
> config: i386-randconfig-a016-20220912 (https://download.01.org/0day-ci/archive/20220913/202209130732.cQpPG34i-lkp@intel.com/config)
> compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
> reproduce (this is a W=1 build):
>          wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>          chmod +x ~/bin/make.cross
>          # https://github.com/intel-lab-lkp/linux/commit/0238f8172a76ee5a84dda79b45911a2b63d59721
>          git remote add linux-review https://github.com/intel-lab-lkp/linux
>          git fetch --no-tags linux-review yaliang-wang-windriver-com/mtd-spi-nor-gigadevice-gd25q256-replace-gd25q256_default_init-with-gd25q256_post_bfpt/20220912-223028
>          git checkout 0238f8172a76ee5a84dda79b45911a2b63d59721
>          # save the config file
>          mkdir build_dir && cp config build_dir/.config
>          COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/mtd/spi-nor/
>
> If you fix the issue, kindly add following tag where applicable
> Reported-by: kernel test robot <lkp@intel.com>
>
> All errors (new ones prefixed by >>):
>
>>> drivers/mtd/spi-nor/gigadevice.c:23:1: error: non-void function does not return a value [-Werror,-Wreturn-type]
>     }
>     ^
>     1 error generated.
>
>
> vim +23 drivers/mtd/spi-nor/gigadevice.c
>
> acb96ecd59f7fd Boris Brezillon 2020-03-13  10
> 0238f8172a76ee Yaliang Wang    2022-09-12  11  static int
> 0238f8172a76ee Yaliang Wang    2022-09-12  12  gd25q256_post_bfpt(struct spi_nor *nor,
> 0238f8172a76ee Yaliang Wang    2022-09-12  13                       const struct sfdp_parameter_header *bfpt_header,
> 0238f8172a76ee Yaliang Wang    2022-09-12  14                       const struct sfdp_bfpt *bfpt)
> acb96ecd59f7fd Boris Brezillon 2020-03-13  15  {
> acb96ecd59f7fd Boris Brezillon 2020-03-13  16   /*
> acb96ecd59f7fd Boris Brezillon 2020-03-13  17    * Some manufacturer like GigaDevice may use different
> acb96ecd59f7fd Boris Brezillon 2020-03-13  18    * bit to set QE on different memories, so the MFR can't
> acb96ecd59f7fd Boris Brezillon 2020-03-13  19    * indicate the quad_enable method for this case, we need
> 0238f8172a76ee Yaliang Wang    2022-09-12  20    * to set it in the post_bfpt fixup hook.
> acb96ecd59f7fd Boris Brezillon 2020-03-13  21    */
> 829ec6408dc58d Tudor Ambarus   2020-03-13  22   nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
> acb96ecd59f7fd Boris Brezillon 2020-03-13 @23  }
> acb96ecd59f7fd Boris Brezillon 2020-03-13  24
>
> --
> 0-DAY CI Kernel Test Service
> https://01.org/lkp


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH] mtd: spi-nor: gigadevice: gd25q256: replace gd25q256_default_init with gd25q256_post_bfpt

[yaliang.wang]

From: Yaliang Wang <Yaliang.Wang@windriver.com>

When utilizing PARSE_SFDP to initialize the flash parameter, the
deprecated initializing method spi_nor_init_params_deprecated() and the
function spi_nor_manufacturer_init_params() within it will never be
executed, which results in the default_init hook function will also never
be executed.

This is okay for 'D' generation of GD25Q256, because 'D' generation is
implementing the JESD216B standards, it has QER field defined in BFPT,
parsing the SFDP can properly set the quad_enable function. The 'E'
generation also implements the JESD216B standards, and it has the same
status register definitions as 'D' generation, parsing the SFDP to set
the quad_enable function should also work for 'E' generation.

However, the same thing can't apply to 'C' generation. 'C' generation
'GD25Q256C' implements the JESD216 standards, and it doesn't have the
QER field defined in BFPT, since it does have QE bit in status register
1, the quad_enable hook needs to be tweaked to properly set the
quad_enable function, this can be done in post_bfpt fixup hook.

Cc: stable@vger.kernel.org
Fixes: 047275f7de18 ("mtd: spi-nor: gigadevice: gd25q256: Init flash based on SFDP")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Yaliang Wang <Yaliang.Wang@windriver.com>
---
 drivers/mtd/spi-nor/gigadevice.c | 28 +++++++++++++++++++++-------
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/mtd/spi-nor/gigadevice.c b/drivers/mtd/spi-nor/gigadevice.c
index 119b38e6fc2a..5fc5d2b2d15e 100644
--- a/drivers/mtd/spi-nor/gigadevice.c
+++ b/drivers/mtd/spi-nor/gigadevice.c
@@ -8,19 +8,33 @@
 
 #include "core.h"
 
-static void gd25q256_default_init(struct spi_nor *nor)
+static int
+gd25q256_post_bfpt(struct spi_nor *nor,
+		   const struct sfdp_parameter_header *bfpt_header,
+		   const struct sfdp_bfpt *bfpt)
 {
 	/*
-	 * Some manufacturer like GigaDevice may use different
-	 * bit to set QE on different memories, so the MFR can't
-	 * indicate the quad_enable method for this case, we need
-	 * to set it in the default_init fixup hook.
+	 * GD25Q256 'C' generation 'GD25Q256C' implements the JESD216
+	 * standards, JESD216 doesn't define QER field in BFPT, but
+	 * the 'GD25Q256C' does have QE bit defined in status register
+	 * 1, this means parsing the BFPT can't properly set the
+	 * quad_enable function, so we need to tweak the quad_enable
+	 * function manually.
+	 *
+	 * GD25Q256 GENERATION|SFDP MAJOR VERSION|SFDP MINOR VERSION
+	 *      GD25Q256C     |SFDP_JESD216_MAJOR|SFDP_JESD216_MINOR
+	 *      GD25Q256D     |SFDP_JESD216_MAJOR|SFDP_JESD216B_MINOR
+	 *      GD25Q256E     |SFDP_JESD216_MAJOR|SFDP_JESD216B_MINOR
 	 */
-	nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
+	if (bfpt_header->major == SFDP_JESD216_MAJOR &&
+	    bfpt_header->minor == SFDP_JESD216_MINOR)
+		nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
+
+	return 0;
 }
 
 static const struct spi_nor_fixups gd25q256_fixups = {
-	.default_init = gd25q256_default_init,
+	.post_bfpt = gd25q256_post_bfpt,
 };
 
 static const struct flash_info gigadevice_nor_parts[] = {
-- 
2.34.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: gigadevice: gd25q256: replace gd25q256_default_init with gd25q256_post_bfpt

[Yaliang Wang]

On 10/3/22 13:35, Tudor.Ambarus@microchip.com wrote:
> [Please note: This e-mail is from an EXTERNAL e-mail address]
> 
> On 9/13/22 11:40, yaliang.wang@windriver.com wrote:
>> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>>
>> From: Yaliang Wang <Yaliang.Wang@windriver.com>
>>
>> When utilizing PARSE_SFDP to initialize the flash parameter, the
>> deprecated initializing method spi_nor_init_params_deprecated() and the
>> function spi_nor_manufacturer_init_params() within it will never be
>> executed, which results in the default_init hook function will also never
>> be executed. As we do have quad enable function defined in BFPT, the
>> post_bfpt hook will be the right place to tweak the function.
>>
>> Cc: stable@vger.kernel.org
>> Fixes: 047275f7de18 ("mtd: spi-nor: gigadevice: gd25q256: Init flash based on SFDP")
>> Reported-by: kernel test robot <lkp@intel.com>
>> Signed-off-by: Yaliang Wang <Yaliang.Wang@windriver.com>
>> ---
>>   drivers/mtd/spi-nor/gigadevice.c | 11 ++++++++---
>>   1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mtd/spi-nor/gigadevice.c b/drivers/mtd/spi-nor/gigadevice.c
>> index 119b38e6fc2a..bdc4d73424af 100644
>> --- a/drivers/mtd/spi-nor/gigadevice.c
>> +++ b/drivers/mtd/spi-nor/gigadevice.c
>> @@ -8,19 +8,24 @@
>>
>>   #include "core.h"
>>
>> -static void gd25q256_default_init(struct spi_nor *nor)
>> +static int
>> +gd25q256_post_bfpt(struct spi_nor *nor,
>> +                  const struct sfdp_parameter_header *bfpt_header,
>> +                  const struct sfdp_bfpt *bfpt)
>>   {
>>          /*
>>           * Some manufacturer like GigaDevice may use different
>>           * bit to set QE on different memories, so the MFR can't
>>           * indicate the quad_enable method for this case, we need
>> -        * to set it in the default_init fixup hook.
>> +        * to set it in the post_bfpt fixup hook.
>>           */
>>          nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
>> +
>> +       return 0;
>>   }
> 
> Maybe we can get rid of this fixup hook entirely. If it was a default_init(), then it
> was set before the SFDP was parsed, so the quad_enable method was overwritten anyway.
> Would you please check why this method was introduced?

I googled the gd25q256 datasheet, and found 'C' generation 
datasheet(GD25Q256C[1]) implements the JESD216 standards, it doesn't 
have the QER(Quad Enable Requirements) field in BFPT, but it does have 
the QE bit in SR1 bit6, this may be the reason why the method was 
introduced.

[1]https://datasheetspdf.com/pdf-file/1295936/GigaDevice/GD25Q256C/1
> 
> What Quad Enable method do you get from SFDP? I expect that spi_nor_sr1_bit6_quad_enable,
> and the fixup hook to be in vain.
As above mentioned, 'C' generation doesn't have the QER field in BFPT, 
so no corresponding quad_enable method can be found in SFDP.

Regrading to 'D' generation [2], it implements the JESD216B standards, 
the corresponding quad_enable method is spi_nor_sr2_bit1_quad_enable, 
parsing the BFPT can get the right method.

As for 'E' generation[3], it also implements the JESD216B standards and 
has the same status registers as defined in 'D' generation, but its 
datasheet doesn't contain SFDP and BFPT definations, so there may be 
some uncertain.

So, in summary, I think 'C' generation devices still need the post_bfpt 
to add the correct quad_enable method, this can do some benefits to the 
outdated devices. For sure, the post_bfpt method should distinguish the 
JESD216 standards the first and then apply the correct quad_enable 
method, I can cook another patch to do this.

[2]https://www.gigadevice.com/datasheet/gd25q256d/
[3]https://www.gigadevice.com/datasheet/gd25q256e/
> 
>>
>>   static const struct spi_nor_fixups gd25q256_fixups = {
>> -       .default_init = gd25q256_default_init,
>> +       .post_bfpt = gd25q256_post_bfpt,
>>   };
>>
>>   static const struct flash_info gigadevice_nor_parts[] = {
>> --
>> 2.34.1
>>
> 
> 
> --
> Cheers,
> ta

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: gigadevice: gd25q256: replace gd25q256_default_init with gd25q256_post_bfpt

[Tudor.Ambarus]

On 9/13/22 11:40, yaliang.wang@windriver.com wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> From: Yaliang Wang <Yaliang.Wang@windriver.com>
> 
> When utilizing PARSE_SFDP to initialize the flash parameter, the
> deprecated initializing method spi_nor_init_params_deprecated() and the
> function spi_nor_manufacturer_init_params() within it will never be
> executed, which results in the default_init hook function will also never
> be executed. As we do have quad enable function defined in BFPT, the
> post_bfpt hook will be the right place to tweak the function.
> 
> Cc: stable@vger.kernel.org
> Fixes: 047275f7de18 ("mtd: spi-nor: gigadevice: gd25q256: Init flash based on SFDP")
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Yaliang Wang <Yaliang.Wang@windriver.com>
> ---
>  drivers/mtd/spi-nor/gigadevice.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/gigadevice.c b/drivers/mtd/spi-nor/gigadevice.c
> index 119b38e6fc2a..bdc4d73424af 100644
> --- a/drivers/mtd/spi-nor/gigadevice.c
> +++ b/drivers/mtd/spi-nor/gigadevice.c
> @@ -8,19 +8,24 @@
> 
>  #include "core.h"
> 
> -static void gd25q256_default_init(struct spi_nor *nor)
> +static int
> +gd25q256_post_bfpt(struct spi_nor *nor,
> +                  const struct sfdp_parameter_header *bfpt_header,
> +                  const struct sfdp_bfpt *bfpt)
>  {
>         /*
>          * Some manufacturer like GigaDevice may use different
>          * bit to set QE on different memories, so the MFR can't
>          * indicate the quad_enable method for this case, we need
> -        * to set it in the default_init fixup hook.
> +        * to set it in the post_bfpt fixup hook.
>          */
>         nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
> +
> +       return 0;
>  }

Maybe we can get rid of this fixup hook entirely. If it was a default_init(), then it
was set before the SFDP was parsed, so the quad_enable method was overwritten anyway.
Would you please check why this method was introduced?

What Quad Enable method do you get from SFDP? I expect that spi_nor_sr1_bit6_quad_enable,
and the fixup hook to be in vain.

> 
>  static const struct spi_nor_fixups gd25q256_fixups = {
> -       .default_init = gd25q256_default_init,
> +       .post_bfpt = gd25q256_post_bfpt,
>  };
> 
>  static const struct flash_info gigadevice_nor_parts[] = {
> --
> 2.34.1
> 


-- 
Cheers,
ta
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: gigadevice: gd25q256: replace gd25q256_default_init with gd25q256_post_bfpt

[Michael Walle]

Am 2022-09-14 05:05, schrieb Yaliang Wang:
> In case the mail list can't receive the email, I'd like to send the 
> email again.
> 
> Sorry, I didn't make it clear.
> I'm doing this because the flash info member parse_sfdp is initialized
> to "true" by PARSE_SFDP macro, as you can see below
>         { "gd25q256", INFO(0xc84019, 0, 64 * 1024, 512)
>                 PARSE_SFDP
>                 FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | 
> SPI_NOR_TB_SR_BIT6)
>                 FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
>                 .fixups = &gd25q256_fixups },
> 
> And when parse_sfdp is true, the function spi_nor_init_params() will
> call spi_nor_parse_sfdp() to initialize the parameters, and I can't
> see any other place to call the original default_init() hook in the
> fixups other than in spi_nor_init_params_deprecated()  ->
> spi_nor_manufacturer_init_params().
> 
> After checking the history of why we are adding this, that is to say
> the commit 48e4d973aefe ("mtd: spi-nor: Add a default_init() fixup
> hook for gd25q256") and the corresponding disscusions, I believe it
> was added for some reason, and we need to add back this function.
> 
> On 9/13/22 18:46, Vanessa Page wrote:
>> **[Please note: This e-mail is from an EXTERNAL e-mail address]
>> 
>> Why are you doing this?

Vanessa Page is a bot/troll.

-michael

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH] mtd: spi-nor: gigadevice: gd25q256: replace gd25q256_default_init with gd25q256_post_bfpt

[Yaliang Wang]

In case the mail list can't receive the email, I'd like to send the 
email again.

Sorry, I didn't make it clear.
I'm doing this because the flash info member parse_sfdp is initialized 
to "true" by PARSE_SFDP macro, as you can see below
         { "gd25q256", INFO(0xc84019, 0, 64 * 1024, 512)
                 PARSE_SFDP
                 FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | 
SPI_NOR_TB_SR_BIT6)
                 FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
                 .fixups = &gd25q256_fixups },

And when parse_sfdp is true, the function spi_nor_init_params() will 
call spi_nor_parse_sfdp() to initialize the parameters, and I can't see 
any other place to call the original default_init() hook in the fixups 
other than in spi_nor_init_params_deprecated()  -> 
spi_nor_manufacturer_init_params().

After checking the history of why we are adding this, that is to say the 
commit 48e4d973aefe ("mtd: spi-nor: Add a default_init() fixup hook for 
gd25q256") and the corresponding disscusions, I believe it was added for 
some reason, and we need to add back this function.

On 9/13/22 18:46, Vanessa Page wrote:
> **[Please note: This e-mail is from an EXTERNAL e-mail address]
> 
> Why are you doing this?
> ------------------------------------------------------------------------
> *From:* linux-mtd <linux-mtd-bounces@lists.infradead.org> on behalf of 
> yaliang.wang@windriver.com <yaliang.wang@windriver.com>
> *Sent:* Tuesday, September 13, 2022 4:40 AM
> *To:* tudor.ambarus@microchip.com <tudor.ambarus@microchip.com>; 
> pratyush@kernel.org <pratyush@kernel.org>; michael@walle.cc 
> <michael@walle.cc>; miquel.raynal@bootlin.com 
> <miquel.raynal@bootlin.com>; richard@nod.at <richard@nod.at>; 
> vigneshr@ti.com <vigneshr@ti.com>
> *Cc:* linux-mtd@lists.infradead.org <linux-mtd@lists.infradead.org>; 
> linux-kernel@vger.kernel.org <linux-kernel@vger.kernel.org>
> *Subject:* [PATCH] mtd: spi-nor: gigadevice: gd25q256: replace 
> gd25q256_default_init with gd25q256_post_bfpt
> From: Yaliang Wang <Yaliang.Wang@windriver.com>
> 
> When utilizing PARSE_SFDP to initialize the flash parameter, the
> deprecated initializing method spi_nor_init_params_deprecated() and the
> function spi_nor_manufacturer_init_params() within it will never be
> executed, which results in the default_init hook function will also never
> be executed. As we do have quad enable function defined in BFPT, the
> post_bfpt hook will be the right place to tweak the function.
> 
> Cc: stable@vger.kernel.org
> Fixes: 047275f7de18 ("mtd: spi-nor: gigadevice: gd25q256: Init flash 
> based on SFDP")
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Yaliang Wang <Yaliang.Wang@windriver.com>
> ---
>   drivers/mtd/spi-nor/gigadevice.c | 11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/gigadevice.c 
> b/drivers/mtd/spi-nor/gigadevice.c
> index 119b38e6fc2a..bdc4d73424af 100644
> --- a/drivers/mtd/spi-nor/gigadevice.c
> +++ b/drivers/mtd/spi-nor/gigadevice.c
> @@ -8,19 +8,24 @@
> 
>   #include "core.h"
> 
> -static void gd25q256_default_init(struct spi_nor *nor)
> +static int
> +gd25q256_post_bfpt(struct spi_nor *nor,
> +                  const struct sfdp_parameter_header *bfpt_header,
> +                  const struct sfdp_bfpt *bfpt)
>   {
>           /*
>            * Some manufacturer like GigaDevice may use different
>            * bit to set QE on different memories, so the MFR can't
>            * indicate the quad_enable method for this case, we need
> -        * to set it in the default_init fixup hook.
> +        * to set it in the post_bfpt fixup hook.
>            */
>           nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
> +
> +       return 0;
>   }
> 
>   static const struct spi_nor_fixups gd25q256_fixups = {
> -       .default_init = gd25q256_default_init,
> +       .post_bfpt = gd25q256_post_bfpt,
>   };
> 
>   static const struct flash_info gigadevice_nor_parts[] = {
> -- 
> 2.34.1
> 
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/ 
> <https://urldefense.com/v3/__http://lists.infradead.org/mailman/listinfo/linux-mtd/__;!!AjveYdw8EvQ!cFcIju_6rdasA67JAVB5Ww727YTj7uzWhJ8HjkRjEJmn-BFGWsga9mPTqCo_m8WFT4Jcu8Uj0iME34B3OOc$>

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

[PATCH] mtd: spi-nor: gigadevice: gd25q256: replace gd25q256_default_init with gd25q256_post_bfpt

[yaliang.wang]

From: Yaliang Wang <Yaliang.Wang@windriver.com>

When utilizing PARSE_SFDP to initialize the flash parameter, the
deprecated initializing method spi_nor_init_params_deprecated() and the
function spi_nor_manufacturer_init_params() within it will never be
executed, which results in the default_init hook function will also never
be executed. As we do have quad enable function defined in BFPT, the
post_bfpt hook will be the right place to tweak the function.

Cc: stable@vger.kernel.org
Fixes: 047275f7de18 ("mtd: spi-nor: gigadevice: gd25q256: Init flash based on SFDP")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Yaliang Wang <Yaliang.Wang@windriver.com>
---
 drivers/mtd/spi-nor/gigadevice.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/mtd/spi-nor/gigadevice.c b/drivers/mtd/spi-nor/gigadevice.c
index 119b38e6fc2a..bdc4d73424af 100644
--- a/drivers/mtd/spi-nor/gigadevice.c
+++ b/drivers/mtd/spi-nor/gigadevice.c
@@ -8,19 +8,24 @@
 
 #include "core.h"
 
-static void gd25q256_default_init(struct spi_nor *nor)
+static int
+gd25q256_post_bfpt(struct spi_nor *nor,
+		   const struct sfdp_parameter_header *bfpt_header,
+		   const struct sfdp_bfpt *bfpt)
 {
 	/*
 	 * Some manufacturer like GigaDevice may use different
 	 * bit to set QE on different memories, so the MFR can't
 	 * indicate the quad_enable method for this case, we need
-	 * to set it in the default_init fixup hook.
+	 * to set it in the post_bfpt fixup hook.
 	 */
 	nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
+
+	return 0;
 }
 
 static const struct spi_nor_fixups gd25q256_fixups = {
-	.default_init = gd25q256_default_init,
+	.post_bfpt = gd25q256_post_bfpt,
 };
 
 static const struct flash_info gigadevice_nor_parts[] = {
-- 
2.34.1


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/