linux-kernel - Re: [PATCH v2 04/10] mtd: rawnand: denali: switch over to ->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: <20190304103052.0eba71a6@xps13>
Date:   Mon, 4 Mar 2019 10:30:52 +0100
From:   Miquel Raynal <miquel.raynal@...tlin.com>
To:     Masahiro Yamada <yamada.masahiro@...ionext.com>
Cc:     linux-mtd@...ts.infradead.org,
        Boris Brezillon <bbrezillon@...nel.org>,
        Brian Norris <computersforpeace@...il.com>,
        linux-kernel@...r.kernel.org, Marek Vasut <marek.vasut@...il.com>,
        Richard Weinberger <richard@....at>,
        David Woodhouse <dwmw2@...radead.org>
Subject: Re: [PATCH v2 04/10] mtd: rawnand: denali: switch over to
 ->exec_op() from legacy hooks

Hi Masahiro,

Masahiro Yamada <yamada.masahiro@...ionext.com> 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@...ionext.com>
> ---

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