[RFC PATCH 1/2] mtd: nand: vf610_nfc: make use of ->exec_op()
stefan at agner.ch
Fri Jan 12 04:58:11 PST 2018
On 2018-01-12 11:59, Boris Brezillon wrote:
> Hi Stefan,
> On Fri, 12 Jan 2018 00:50:36 +0100
> Stefan Agner <stefan at agner.ch> wrote:
>> This reworks the driver to make use of ->exec_op() callback. The
>> command sequencer of the VF610 NFC aligns well with the new ops
> That's great news!
>> Currently we handle reset and read id pattern separately. The
>> read id pattern uses registers for the returned data, using a
>> dedicated function helps to special case the data in op.
>> The parameter page handling is rather ad-hoc currently: The
>> stack calls exec_op twice, once to read the paramter page and
>> a second time to tranfer data. The code/controller can not
>> handle this steps separately, hence just transfer data already
>> in the first call and just memcpy from the buffer in the second
>> call. This needs a small change in the nand_base.c file.
> Hm, I think it's using change_column after the read_param_page command,
> so it's not exactly a 'data xfer' only step. Can't your controller send
> only a command or address cycle without any associated data xfers on
> the bus?
Hm, I was looking at ONFI, where it does not do the column change:
I am pretty sure that the controller can handle command and address
separately, but not sure about the successive data transfer...
> Anyway, if you really have to do the READ_PARAM_PAGE in a single step,
> you'll have to patch the core to pass a valid buffer and len != 0 when
> calling nand_read_param_page_op().
>> The current state seems to pass MTD tests on a Colibri VF61.
>> Signed-off-by: Stefan Agner <stefan at agner.ch>
>> Hi Boris, Hi Miquel,
>> Thanks for your work on the new NAND interface. The VF610 NFC
>> definitly matches better the ->exec_op() interface.
> And thanks for transitioning to this new interface.
>> This is still in early stage but seems to boot a Linux as rootfs
>> successfully. Before working on further simplification I wanted
>> to get some feedback on the general idea.
>> If needed, the data sheet of the controller can be optained
>> publicly (Vybrid Reference Manual, Chapter 10.3).
> I'll have a look, thanks for the pointer.
>> Some questions from my side:
>> - Parameter page: Why is exec_op called twice currently? This
>> seems to be problematic for this controller. Are there plans
>> to integrate the two calls in a single op sequence?
> I don't know, but nothing is set in stone.
>> - Row/Column addresses: The controller seems to separate those
>> two, hence the driver "guesses" which address bytes need to
>> go where.. Maybe it would be better to separate that on ops
> Miquel had a similar need for the new marvell NAND driver, so maybe we
> can store this information at the nand_chip level:
> unsigned int row_cycles;
> unsigned int column_cycles;
> I still want to keep the address data in an array of bytes, but thanks
> to the [row,column]_cycles information you'll be able to easily convert
> this array of bytes into row and column addresses.
That sounds good enough for my case.
>> - Separation is often needed by command. One can make an educated
>> guess what kind of command is going to be passed by filtering
>> the appropriate ops. Maybe it would be good if the ops parser
>> can filter by command?
> I'll have to check the datasheet you pointed out. We're not closed to
> the idea of filtering things by particular opcodes, but most of the time
> the controller is not hardcoded this way, and what they refer as READID
> or READSTATUS is not about sending a NAND_READ_ID or a NAND_READ_STATUS
> opcode, but following the sequence imposed by these operations (READID
> is CMD+ADDR+DATA_IN, READSTATUS is CMD+DATA_IN). Which means, if you
> have another operation which uses the same sequence but a different
> opcode, the controller is able to handle it.
The chapter 10.3.4.7 Flash Command Code Description is probably most
interesting in that regard. With the old interface, I used predefined
command codes, with the new interface I can dynamically generate the
command code from the operations provided by the stack.
However, some seem to be handled specially: There is single bit for
"Read ID". And the bytes returned by the controller are stored in
register instead of the regular data area. These are the reasons I do
use special handling right now...
>> miquel.raynal at free-electrons.com
>> drivers/mtd/nand/vf610_nfc.c | 543 +++++++++++++++++++++++++++----------------
>> 1 file changed, 349 insertions(+), 194 deletions(-)
> I'll have a look at the code once I have a better understanding of how
> the controller actually work.
> Once again, thanks a lot for working on this topic.
More information about the linux-mtd