Re: [PATCH v2 2/6] nand: spi: add basic operations support

[Arnaud Mouiche <arnaud.mouiche@gmail.com>]

On 09/03/2017 07:02, Boris Brezillon wrote:
> [...]
>>>   
>>>> diff --git a/include/linux/mtd/spinand.h b/include/linux/mtd/spinand.h
>>>> index f3d0351..ee447c1 100644
>>>> --- a/include/linux/mtd/spinand.h
>>>> +++ b/include/linux/mtd/spinand.h
>>>> @@ -135,6 +135,8 @@ struct spinand_manufacturer_ops {
>>>>        bool(*detect)(struct spinand_device *chip);
>>>>        int (*init)(struct spinand_device *chip);
>>>>        void (*cleanup)(struct spinand_device *chip);
>>>> +     void (*build_column_addr)(struct spinand_device *chip,
>>>> +                               struct spinand_op *op, u32 page, u32 column);
>>> Okay, I think Arnaud was right, maybe we should have vendor specific
>>> ops for basic operations like ->prepare_read/write_op(), instead of
>>> having these ->get_dummy() and ->build_column_addr() hooks.
>>> Or maybe just a ->prepare_op() hook that can prepare things for any
>>> basic operation (read, write, ...).
>> I prefer ->prepare_read_op() and ->prepare_write_op. Fix this in v3
> I'd like to have Arnaud's feedback on this. Can you wait a bit before
> sending a new version?

->prepare_read_op() and ->prepare_write_op
looks fine to me.

FYI: I have prepared a imx6sl board which can run vanilla mtd/master kernels.
On this board, I can swap the various spinand samples I have:
- Micron (the one supported by those patches)
- winbond
- macronix
- ESMT

Looking forward to test v3 ;)
Arnaud