Messages in this thread | | | Date | Fri, 11 Jul 2008 23:37:08 +0200 | From | Samuel Ortiz <> | Subject | Re: [patch 4/4] MFD: Change mfd platform device usage to wrapper platform_device |
| |
On Wed, Jul 09, 2008 at 03:56:54PM +0400, Dmitry wrote: > >> Basically we have two choises for the subdevice driver: > >> 1) it doesn't know about cells at all (e.g. generic-bl, IIRC). Then we are safe > >> to loose that "cell" information > >> 2) If it does use cell information (to get access to hooks), we pass it > >> via platform_data pointer in the mfd_cell and we are ok with it. > > > > Erm, that is complete non-answer. The driver model and various other > > parts of the kernel are littered with examples of embedding one > > structure within another to gain an C++ like object inheritance. > > > > I've supplied an reasonable example of doing this to create an mfd_cell > > device from an platform_device without creating an large amount of code > > and improving the efficiency and code-lineage in the process. I do not > > see how this isn't "correct" or in any way breaing the current linux > > model of doing things. > > It isn't breaking it. OK. I'm leaving the decision to the MFD or ARM > maintainers. > And BTW, nearly the same patch was sent yesterday by me[1]. Is it an independant > work, or did you miss my sign-off? > > [1]: http://permalink.gmane.org/gmane.linux.ports.arm.kernel/44142 As Russel pointed out, both patches are broken as long as we support a modular mfd-core (and there's no reason why we shouldnt). So, so far, Mike's patch is the best candidate, even though the idea of wrapping the platform device into an mfd_device sounded neater at first.
And again, this is definitely post merge window material.
Cheers, Samuel.
> > > >> > >> > The current driver is being inefficent in the way it creates resources > >> > on the stack and then calls a routine that does an kalloc/memcpy on > >> > the resources. > >> > >> I don't see any inefficiency ATM. > >> > >> >> 2) Please examine the tmio-nand driver (was here on the list and on > >> >> linux-mtd). It uses the mfd_cell > >> >> to call hooks from the "host" driver (tc6393xb, more to be added soon). > >> > > >> > The one posted in [1] does not call these hooks at-all, can ou please > >> > explain why these hooks are needed in addition to the ones already > >> > available in the platform device driver? > >> > > >> > [1] http://lists.infradead.org/pipermail/linux-mtd/2008-June/022137.html > >> > >> + > >> +static int tmio_hw_init(struct platform_device *dev, struct tmio_nand *tmio) > >> +{ > >> + struct mfd_cell *cell = mfd_get_cell(dev); > >> + const struct resource *nfcr = NULL; > >> + unsigned long base; > >> + int i; > >> + > >> + for (i = 0; i < cell->num_resources; i++) > >> + if (!strcmp((cell->resources+i)->name, TMIO_NAND_CONTROL)) > >> + nfcr = &cell->resources[i]; > >> + > >> + if (nfcr == NULL) > >> + return -ENOMEM; > >> + > >> + if (cell->enable) { > >> + int rc = cell->enable(dev); > >> + if (rc) > >> + return rc; > >> + } > >> > >> That cell->enable() is necessary to set up the host (in the tc6393xb > >> case to enable buffers) > >> to enable access to the nand. > > > > So, the enable/disable calls might be useful, however is there any > > reason this could not be handled by the clock framework? The suspend/resume > > entries where not used, and I belive should not be in here. > > They should be here for exactly the same reason. They are used by the drivers > that will be submitted later. E.g. OHCI driver needs such > suspend/resume handling. > > > As noted before, mfd_get_cell() got dropped by [2] > > > > [2] http://lists.arm.linux.org.uk/lurker/message/20080708.153450.bb33046d.en.html > > Yes, and as I said before it will need some small modifications. > > -- > With best wishes > Dmitry > > ------------------------------------------------------------------- > List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel > FAQ: http://www.arm.linux.org.uk/mailinglists/faq.php > Etiquette: http://www.arm.linux.org.uk/mailinglists/etiquette.php
| |