lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7aca2dd2-611d-4af2-b4a8-265528de2534@linaro.org>
Date: Wed, 2 Apr 2025 13:10:43 +0100
From: Tudor Ambarus <tudor.ambarus@...aro.org>
To: Cheng Ming Lin <linchengming884@...il.com>, pratyush@...nel.org,
 mwalle@...nel.org, miquel.raynal@...tlin.com, richard@....at,
 vigneshr@...com, linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org
Cc: alvinzhou@...c.com.tw, leoyu@...c.com.tw,
 Cheng Ming Lin <chengminglin@...c.com.tw>
Subject: Re: [PATCH 3/3] mtd: spi-nor: macronix: Move
 macronix_nor_default_init logic to macronix_nor_late_init



On 4/2/25 9:51 AM, Cheng Ming Lin wrote:
> From: Cheng Ming Lin <chengminglin@...c.com.tw>
> 
> Remove macronix_nor_default_init and move its functionality to
> macronix_nor_late_init to ensure proper quad_enable initialization.
> 
> For MX25L3255E, SFDP follows JESD216, which does not include the Quad
> Enable bit Requirements field in its version. When the size field is
> removed, manufacturer->fixups->default_init hook is not executed, causing
> params->quad_enable not being overwritten with the intended function.
> Consequently, it remains as the default spi_nor_sr2_bit1_quad_enable.
> 
> By moving quad_enable setup from default_init to late_init, quad_enable
> is correctly assigned after spi_nor_init_params, regardless of the size
> field removal.
> 
> Additionally, according to spi-nor/core.h, quad_enable is more
> appropriately placed in late_init, as older SFDP versions did not define
> the Quad Enable bit Requirements. This change removes default_init and
> moves quad_enable handling to late_init accordingly.
> 
> Signed-off-by: Cheng Ming Lin <chengminglin@...c.com.tw>
> ---
>  drivers/mtd/spi-nor/macronix.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/mtd/spi-nor/macronix.c b/drivers/mtd/spi-nor/macronix.c
> index 07e0bd0b70a0..216c02b92bfe 100644
> --- a/drivers/mtd/spi-nor/macronix.c
> +++ b/drivers/mtd/spi-nor/macronix.c
> @@ -282,22 +282,17 @@ static int macronix_nor_set_octal_dtr(struct spi_nor *nor, bool enable)
>  	return enable ? macronix_nor_octal_dtr_en(nor) : macronix_nor_octal_dtr_dis(nor);
>  }
>  
> -static void macronix_nor_default_init(struct spi_nor *nor)
> -{
> -	nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;
> -}
> -
>  static int macronix_nor_late_init(struct spi_nor *nor)
>  {
>  	if (!nor->params->set_4byte_addr_mode)
>  		nor->params->set_4byte_addr_mode = spi_nor_set_4byte_addr_mode_en4b_ex4b;
> +	nor->params->quad_enable = spi_nor_sr1_bit6_quad_enable;

Not at manufacturer level, please. You change the behavior and overwrite
whatever is retrieved from SFDP for all the flashes. Instead you should
introduce late init just for MX25L3255E, because set_4byte_addr_mode is
not covered in rev A of JESD216 I assume. Then if other flashes get this
param wrong from BFPT, amend them with post_bfpt hooks.


>  	nor->params->set_octal_dtr = macronix_nor_set_octal_dtr;
>  
>  	return 0;
>  }
>  
>  static const struct spi_nor_fixups macronix_nor_fixups = {
> -	.default_init = macronix_nor_default_init,
>  	.late_init = macronix_nor_late_init,
>  };
>  


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ