[PATCH v5] mtd: rawnand: brcmnand: legacy exec_op implementation

[Miquel Raynal]

On 21/05/2025 at 10:03:25 +02, Álvaro Fernández Rojas <noltari at gmail.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 at gmail.com>
> Reviewed-by: David Regan <dregan at broadcom.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