Re: [RFCv2: PATCH 2/2] mtd: mediatek: driver for MTK Smart Device Gen1 NAND

[Jorge Ramirez-Ortiz <jorge.ramirez-ortiz@linaro.org>]

On 03/22/2016 12:58 PM, Boris Brezillon wrote:
>> +typedef void (*release)(struct sdg1_ecc_if *);
>> > +typedef int (*decode)(struct sdg1_ecc_if *);
>> > +typedef void (*init) (struct sdg1_ecc_if *);
>> > +
>> > +struct sdg1_ecc_if {
>> > +	release	release;
>> > +	control control;
>> > +	config config;
>> > +	encode encode;
>> > +	decode decode;
>> > +	check check;
>> > +	init init;
>> > +};
>> > +
>> > +struct sdg1_ecc_if *of_sdg1_ecc_get(struct device_node *);
> I think you should directly return a pointer to the sdg1_ecc instance
> here.
> AFAICS, the "interface" appraoch is useless, all you need is a set of
> exported functions, which all takes the sdg1_ecc pointer as a first
> argument and a set of extra arguments depending on the function.
> See what's done in the jz4780_bch.c driver.
>

yes I am of the contrary opinion (why export a number of functions when you can
just export an interface and keep a clear encapsulation).
as a matter of fact the interface exposes the dependency with the engine while a
random number of function calls does not (with the interface approach eventually
we might be able to have a common ecc interface some day, maybe)

anyway, working on v3 now. thanks for the detailed review as always (and sorry
for the clear fuck ups like the size 0 array in the wrong place...argh!)