[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20181013080542.2ae9b199@bbrezillon>
Date: Sat, 13 Oct 2018 08:05:42 +0200
From: Boris Brezillon <boris.brezillon@...tlin.com>
To: Janusz Krzysztofik <jmkrzyszt@...il.com>
Cc: Miquel Raynal <miquel.raynal@...tlin.com>,
Richard Weinberger <richard@....at>,
David Woodhouse <dwmw2@...radead.org>,
Brian Norris <computersforpeace@...il.com>,
Marek Vasut <marek.vasut@...il.com>,
linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] mtd: rawnand: ams-delta: Use ->exec_op()
Hi Janusz,
On Fri, 12 Oct 2018 22:41:01 +0200
Janusz Krzysztofik <jmkrzyszt@...il.com> wrote:
> Replace legacy callbacks with ->select_chip() and ->exec_op().
>
> In order to remove any references to legacy structure members, use of
> .IO_ADDR_R/W has been replaced wit runtime calculations based on
^ with
> priv->io_base.
Can we do that in 2 steps?
1/ Stop using .IO_ADDR_R/W
2/ Convert the driver to ->exec_op()
>
> Suggested-by: Boris Brezillon <boris.brezillon@...tlin.com>
> Signed-off-by: Janusz Krzysztofik <jmkrzyszt@...il.com>
> ---
[...]
> -static int ams_delta_nand_ready(struct nand_chip *this)
> +static int ams_delta_exec_op(struct nand_chip *this,
> + const struct nand_operation *op, bool check_only)
> {
> struct ams_delta_nand *priv = nand_get_controller_data(this);
> + const struct nand_op_instr *instr;
> + int ret = 0;
> +
You should have:
if (check_only)
return 0;
Other than that, the conversion looks good, so you can add
Reviewed-by: Boris Brezillon <boris.brezillon@...tlin.com> once you've
addressed my comments.
Regards,
Boris
> + for (instr = op->instrs; instr < op->instrs + op->ninstrs; instr++) {
> +
> + switch (instr->type) {
> + case NAND_OP_CMD_INSTR:
> + gpiod_set_value(priv->gpiod_cle, 1);
> + ams_delta_write_buf(priv, &instr->ctx.cmd.opcode, 1);
> + gpiod_set_value(priv->gpiod_cle, 0);
> + break;
> +
> + case NAND_OP_ADDR_INSTR:
> + gpiod_set_value(priv->gpiod_ale, 1);
> + ams_delta_write_buf(priv, instr->ctx.addr.addrs,
> + instr->ctx.addr.naddrs);
> + gpiod_set_value(priv->gpiod_ale, 0);
> + break;
> +
> + case NAND_OP_DATA_IN_INSTR:
> + ams_delta_read_buf(priv, instr->ctx.data.buf.in,
> + instr->ctx.data.len);
> + break;
> +
> + case NAND_OP_DATA_OUT_INSTR:
> + ams_delta_write_buf(priv, instr->ctx.data.buf.out,
> + instr->ctx.data.len);
> + break;
> +
> + case NAND_OP_WAITRDY_INSTR:
> + ret = priv->gpiod_rdy ?
> + nand_gpio_waitrdy(this, priv->gpiod_rdy,
> + instr->ctx.waitrdy.timeout_ms) :
> + nand_soft_waitrdy(this,
> + instr->ctx.waitrdy.timeout_ms);
> + break;
> + }
> +
> + if (ret)
> + break;
> + }
>
> - return gpiod_get_value(priv->gpiod_rdy);
> + return ret;
> }
Powered by blists - more mailing lists