On Fri, 10 Jun 2016, Boris Brezillon wrote: > The goal here is to retrieve the controller attached to a given chip in > order to avoid the global nfc_info variable (and abusing > nand_get/set_controller_data() to store a pointer to the controller is > not a good idea either: it's supposed to be used to store per-chip > private data). > Then, let's say I really care about this clear separation between NAND > controllers and NAND chips (even if the controller is only supporting a > single device), because it makes things clearer, and because it brings > some consistency in the NAND controller drivers. > That's something I've asked to other contributors, and I'm asking it to > you too. Certainly. As I said, I'm not trying to get away from doing it if there's any motivation for it which there clearly is. > You'll see that implementing this separation is not much more > complicated than having this global variable, and I must admit global > variable make me scream (especially when they can be avoided). Yes, I see now. With struct nand_hw_control in the proper place it allows a translation from a struct nand_chip * to a future non-global struct nfc_info * . Yeah, I would probably have stumbled upon this when rewriting it. > > > But this is not there yet, and in the meantime, if possible, I'd prefer > > > seeing drivers implementing the ->cmd_ctrl() function instead of > > > overloading the default ->cmdfunc() implementation. > > > > I see, I suppose that's because during the course of this the ->cmdfunc() > > logic will be significantly changed, requiring corresponding changes in > > drivers that do overload that function? Fair enough, that's a pretty good > > reason, probably more so than the alleged simplicity of the ->cmd_ctrl() > > interface. > > There's another reason actually. We have chip specific functions (like > ->setup_read_retry()) which might want to use > non-standard/vendor-specific operations, and this implies patching all > ->cmdfunc() implementations, or at least making sure they will work > fine with these new commands. I see. > So yes, I'm clearly trying to avoid specific ->cmdfunc() (especially > when they are not generic enough to support new commands). Yes, makes sense. > Again, ->cmd_ctrl() does not have to be used in your internal > ecc->read/write_page() implementations (all you'll have to do is avoid > using the ->cmdfunc() method and create your own NAND controller > specific commands instead), but it should at least be used for basic > operations that do not require high performances (i.e. NAND detection, > NAND RESET, read-retry, ...). Ok, good. /Ricard -- Ricard Wolf Wanderlöf ricardw(at)axis.com Axis Communications AB, Lund, Sweden www.axis.com Phone +46 46 272 2016 Fax +46 46 13 61 30 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
