Re: [PATCH v2 04/10] mtd: rawnand: denali: switch over to ->exec_op() from legacy hooks

[Miquel Raynal]

Hi Masahiro,

Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx> wrote on Tue, 12 Feb
2019 16:12:56 +0900:

> Implement ->exec_op(), and remove the deprecated hooks.
> 
> Signed-off-by: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
> ---

Thanks for working on this, I like it very much!

> 
> Changes in v2: None
> 
>  drivers/mtd/nand/raw/denali.c | 234 +++++++++++++++++++++++-------------------
>  1 file changed, 126 insertions(+), 108 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/denali.c b/drivers/mtd/nand/raw/denali.c
> index a2fe2ff..bd7df25 100644
> --- a/drivers/mtd/nand/raw/denali.c
> +++ b/drivers/mtd/nand/raw/denali.c

[...]

> +
> +static int denali_exec_instr(struct nand_chip *chip,
> +			     const struct nand_op_instr *instr)
> +{
> +	struct denali_nand_info *denali = to_denali(chip);
> +	bool width16 = chip->options & NAND_BUSWIDTH_16;
> +
> +	switch (instr->type) {
> +	case NAND_OP_CMD_INSTR:
> +		denali_exec_out8(denali, DENALI_MAP11_CMD,
> +				 &instr->ctx.cmd.opcode, 1);
> +		return 0;
> +	case NAND_OP_ADDR_INSTR:
> +		denali_exec_out8(denali, DENALI_MAP11_ADDR,
> +				 instr->ctx.addr.addrs,
> +				 instr->ctx.addr.naddrs);
> +		return 0;
> +	case NAND_OP_DATA_IN_INSTR:
> +		(!instr->ctx.data.force_8bit && width16 ?
> +		 denali_exec_in16 :
> +		 denali_exec_in8)(denali, DENALI_MAP11_DATA,
> +				  instr->ctx.data.buf.in,
> +				  instr->ctx.data.len);

I think this is abusing the ternary operator, can you please find
another way for writing this? Otherwise it is not easily readable... If
it is really too complicated within 80 chars, then never mind.

> +		return 0;
> +	case NAND_OP_DATA_OUT_INSTR:
> +		(!instr->ctx.data.force_8bit && width16 ?
> +		 denali_exec_out16 :
> +		 denali_exec_out8)(denali, DENALI_MAP11_DATA,
> +				   instr->ctx.data.buf.out,
> +				   instr->ctx.data.len);
> +		return 0;
> +	case NAND_OP_WAITRDY_INSTR:
> +		return denali_exec_waitrdy(denali);
> +	default:
> +		WARN_ONCE(1, "unsupported NAND instruction type: %d\n",
> +			  instr->type);
> +
> +		return -EINVAL;
> +	}
> +}
> +


Thanks,
MiquÃl