Re: [PATCH 2/2] mtd: nand: nand_do_{read,write}_ops - pass OOB buffer through

[Shmulik Ladkani <shmulik.ladkani@gmail.com>]

Hi Brian,

On Wed, 18 Apr 2012 09:13:38 -0700 Brian Norris <computersforpeace@gmail.com> wrote:
> > So the 'oob' parameter is more of a boolean than an actual buffer to be
> > used by the various ecc.{read,write}_page implementors.
> 
> Yes, I suppose so. I naturally used 'oob' as a buffer, since that's very 
> straightforward and logical from a 'layers' perspective and because my 
> driver doesn't need any buffer when oob is not required. But I see that 
> it essentially would become a boolean flag for many of the other 
> interfaces, and so a boolean can work just as well.
> 
> > Any reason not to pass a boolean instead?
> 
> Only reason I'm thinking of: a cleaner interface.
> 
> To me, the interface is rather non-obvious and ugly when data is 
> constantly shuttled back and forth behind the scenes (i.e., not via 
> function arguments or ret values) by using chip->oob_poi.
> 
> However, this sense of "ugliness" competes with the ugliness of needing 
> a buffer even when the interface might otherwise say "no OOB." Many 
> {read,write}_page functions would need something like:
> 
>      uint8_t *oobbuf = oob ? oob : chip->oob_poi;
> 
> which is not pretty.
> 
> I'm open to either way, I guess, but I'm now leaning a little toward 
> 'oob' as a boolean.

Yes, both approaches aren't perfect.

However a 'int has_user_oob' (or 'has_oob') has a precise, consistent
meaning: the mtd user explicitly provided an OOB buffer (or in the read
case, the mtd user expects the OOB to be returned).
(need help with the boolean's name, though)

Also, I think it will result in less changes (makes the whole
s/oob/oobbuf/ collateral damage of 1st patch irrelevant).

So IMO a boolean is better.

Regards,
Shmulik