[PATCH v4 55/58] mtd: nand: add helpers to access ->priv

On Sat, Dec 19, 2015 at 04:01:24AM +0100, Boris Brezillon wrote:

Actually the nand_{get,set}_controller_data() helpers are not about
assigning NAND controller private data (as you pointed those can
already be retrieved thanks to the ->controller field using the
container_of() trick), but per-chip private data instantiated by the
NAND controller and attached to a specific chip. For example, some
controllers pre-compute some register values or a clk rate to set when
a specific chip is selected. This is what per-chip controller data is
meant for.

Sure. Really, it's just anything the controller driver needs to store on
a per-chip basis. All I'm suggesting is picking a name that doesn't
imply it's a per-controller instance, when it's actually a
per-flash-chip instance.

Now, the reason I explicitly specified the data usage instead of using
a generic name like nand_{get,set}_data() is because I plan to define

I never suggested just "_data"; I said "_drvdata".

other helpers to allow NAND manufacturer code to manipulate its own
private data. This is required if we want to support read-retry on some
chips who are requiring a read OTP area step to retrieve some register
values which will later be used to change from one read-retry mode to
another.
The plan was to define the nand_{set,get}_manufacturer_data() helpers,
and create or reuse an existing priv field (mtd->priv?) to store this
private data.

That's interesting. Sounds like an OK idea. (Personally, I wouldn't
try to use mtd->priv for this, but otherwise looks OK.)

Also note that the spi framework provides the same kind of helpers [1].

Hmm, OK. FWIW, they have both "driver data" and "controller state". It's
not perfectly clear to me why both exist.

This being said, I'm perfectly fine changing the function names, but
I'd like to replace it by something explicitly telling the user that
this field should only be set by NAND controller drivers. 

Sure. I though a "driver data"-based name did this. But I'll leave it to
you. I could even be OK with "controller data", if you still think this
fits your overall controller refactoring plan, and communicates its
purpose best.

[1]http://lxr.free-electrons.com/source/include/linux/spi/spi.h#L189

Regards,
Brian