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: <87ecn1hovq.fsf@bootlin.com>
Date: Tue, 03 Feb 2026 15:40:09 +0100
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: Cheng Ming Lin <linchengming884@...il.com>
Cc: Vignesh Raghavendra <vigneshr@...com>,  Richard Weinberger
 <richard@....at>,  Tudor Ambarus <tudor.ambarus@...aro.org>,  Martin
 Kurbanov <mmkurbanov@...utedevices.com>,  Mikhail Kshevetskiy
 <mikhail.kshevetskiy@...sys.eu>,  Pratyush Yadav <pratyush@...nel.org>,
  linux-mtd@...ts.infradead.org,  linux-kernel@...r.kernel.org,
  alvinzhou@...c.com.tw,  Cheng Ming Lin <chengminglin@...c.com.tw>
Subject: Re: [PATCH v4 1/2] mtd: spi-nand: Add support for randomizer

Hello Cheng Ming,

On 03/02/2026 at 10:13:57 +08, Cheng Ming Lin <linchengming884@...il.com> wrote:

> From: Cheng Ming Lin <chengminglin@...c.com.tw>
>
> This patch adds support for the randomizer feature.
>
> It introduces a 'set_randomizer' callback in 'struct spinand_info' and
> 'struct spinand_device'.
>
> If a driver implements this callback, the core will call it during
> device initialization (spinand_init) to enable the randomizer.

I recently had a request regarding the sunxi controller driver
randomizer. I would like to get back to having a nand wide randomizer DT
property, because a Macronix specific value honestly no longer makes
sense. Randomizers have been there for ages, they are not so often used
because scrambling data is not convenient for developers but they may
have a real interest depending on the NAND chip or the external
conditions.

We will need two mutually exclusive optional properties in
nand-chip.yaml:
nand-enable-randomizer;
nand-disable-randomizer;

BTW, where is the patch adding the property to the bindings? Did we
already merge it? I do not find it anymore.

Looking at the current implementation, I really don't like the core
always calling in the "init" and the manufacturer driver reading the DT
and deciding whether or not it enables it.

> Signed-off-by: Cheng Ming Lin <chengminglin@...c.com.tw>
> ---
>  drivers/mtd/nand/spi/core.c | 23 +++++++++++++++++++++++
>  include/linux/mtd/spinand.h |  9 +++++++++
>  2 files changed, 32 insertions(+)
>
> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> index d207286572d8..4f8ff94f5ccc 100644
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
> @@ -206,6 +206,12 @@ static int spinand_cont_read_enable(struct spinand_device *spinand,
>  	return spinand->set_cont_read(spinand, enable);
>  }
>  
> +static int spinand_randomizer_enable(struct spinand_device *spinand,
> +				     bool enable)
> +{
> +	return spinand->set_randomizer(spinand, enable);

This helper seems mostly useless.

> +}
> +
>  static int spinand_check_ecc_status(struct spinand_device *spinand, u8 status)
>  {
>  	struct nand_device *nand = spinand_to_nand(spinand);
> @@ -1218,6 +1224,19 @@ static int spinand_create_dirmaps(struct spinand_device *spinand)
>  	return 0;
>  }
>  
> +static int spinand_randomizer_init(struct spinand_device *spinand)
> +{
> +	int ret;
> +
> +	if (spinand->set_randomizer) {
> +		ret = spinand_randomizer_enable(spinand, true);

Please directly call the hook.

Given the previous feedback, you should change "true" to be based on the
presence of the DT property.

> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}

Rest LGTM.

Thanks,
Miquèl

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ