linux-kernel - Re: [PATCH v5] mtd: rawnand: brcmnand: legacy exec

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: <87wma74ceh.fsf@bootlin.com>
Date: Fri, 23 May 2025 16:41:58 +0200
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: Álvaro Fernández Rojas <noltari@...il.com>
Cc: linux-mtd@...ts.infradead.org,  dregan@...adcom.com,
  bcm-kernel-feedback-list@...adcom.com,  florian.fainelli@...adcom.com,
  rafal@...ecki.pl,  computersforpeace@...il.com,  kamal.dasu@...adcom.com,
  dan.beygelman@...adcom.com,  william.zhang@...adcom.com,
  frieder.schrempf@...tron.de,  linux-kernel@...r.kernel.org,
  vigneshr@...com,  richard@....at,  bbrezillon@...nel.org,
  kdasu.kdev@...il.com,  jaimeliao.tw@...il.com,  kilobyte@...band.pl,
  jonas.gorski@...il.com,  dgcbueu@...il.com
Subject: Re: [PATCH v5] mtd: rawnand: brcmnand: legacy exec_op implementation

On 21/05/2025 at 10:03:25 +02, Álvaro Fernández Rojas <noltari@...il.com> wrote:

> Commit 3c8260ce7663 ("mtd: rawnand: brcmnand: exec_op implementation")
> removed legacy interface functions, breaking < v5.0 controllers support.
> In order to fix older controllers we need to add an alternative exec_op
> implementation which doesn't rely on low level registers.
>
> Fixes: 3c8260ce7663 ("mtd: rawnand: brcmnand: exec_op implementation")
> Signed-off-by: Álvaro Fernández Rojas <noltari@...il.com>
> Reviewed-by: David Regan <dregan@...adcom.com>
> ---
>  drivers/mtd/nand/raw/brcmnand/brcmnand.c | 222 ++++++++++++++++++++++-
>  1 file changed, 215 insertions(+), 7 deletions(-)
>
>  v5: add changes requested by Miquèl Raynal:
>   - Mention and explain legacy in native_cmd_conv.
>   - EOPNOTSUPP instead of EINVAL for instr->type else.
>   - Implement missing check_only functionality.
>
>  v4: add changes requested by Jonas Gorski:
>   - Add missing breaks in brcmnand_exec_instructions_legacy.
>   - Restore missing ret assignment in brcmnand_exec_op.
>
>  v3: add changes requested by Florian and other improvements:
>   - Add associative array for native command conversion.
>   - Add function pointer to brcmnand_controller for exec_instr
>     functionality.
>   - Fix CMD_BLOCK_ERASE address.
>   - Drop NAND_CMD_READOOB support.
>
>  v2: multiple improvements:
>   - Use proper native commands for checks.
>   - Fix NAND_CMD_PARAM/NAND_CMD_RNDOUT addr calculation.
>   - Remove host->last_addr usage.
>   - Remove sector_size_1k since it only applies to v5.0+ controllers.
>   - Remove brcmnand_wp since it doesn't exist for < v5.0 controllers.
>   - Use j instead of i for flash_cache loop.
>

...

> +static int brcmnand_check_instructions_legacy(struct nand_chip *chip,
> +					      const struct nand_operation *op)
> +{
> +	const struct nand_op_instr *instr;
> +	unsigned int i;
> +	u8 cmd;
> +
> +	for (i = 0; i < op->ninstrs; i++) {
> +		instr = &op->instrs[i];
> +
> +		switch (instr->type) {
> +		case NAND_OP_CMD_INSTR:
> +			cmd = native_cmd_conv[instr->ctx.cmd.opcode];
> +			if (cmd == CMD_NOT_SUPPORTED)
> +				return -EOPNOTSUPP;
> +			break;
> +		case NAND_OP_ADDR_INSTR:
> +		case NAND_OP_DATA_IN_INSTR:

No NAND_OP_DATA_OUT_INSTR?

> +		case NAND_OP_WAITRDY_INSTR:
> +			break;
> +		default:
> +			return -EOPNOTSUPP;
> +		}
> +	}
> +
> +	return 0;
> +}

Rest lgtm.

Thanks,
Miquèl