Messages in this thread | | | Date | Wed, 9 Jul 2008 15:15:47 +0400 | From | Dmitry <> | Subject | Re: [patch 4/4] MFD: Change mfd platform device usage to wrapper platform_device |
| |
2008/7/9 Ben Dooks <ben-linux@fluff.org>: > This patch changes the mfd core behaviour to wrapper the platform_device > it creates in an struct mfd_device which contains the information > about the cell that was created. > > 1) The creation of the resource list and then passing it to the > platform_device_add_resources() causes the allocation of a > large array on the stack as well as copying the source data > twice (it is copied from the mfd_cell to the temporary array > and then copied into the newly allocated array) > > 2) We can wrapper the platform_device into an mfd_device and use > that to do the platform_device and resource allocation in one > go to reduce the failiure. > > Note, is there actually any reason to pass the sub devices any > information about the cell they are created from? The mfd core > already makes the appropriate resource adjustments and anything > else like clocks should be exported by the clock drivers? > > Signed-off-by: Ben Dooks <ben-linux@fluff.org>
NAK. 0) It was discussed yesterday on the list and the decision was to go in a different way. I've provided a bit cleaner patch with the same idea, but then we decided to go in a bit different way. 1) I prefer patch by Mike Rapoport which is more clear and goes in a more correct way. 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).
> > Index: linux-2.6.26-rc9-next20080709/include/linux/mfd/core.h > =================================================================== > --- linux-2.6.26-rc9-next20080709.orig/include/linux/mfd/core.h 2008-07-09 10:46:23.000000000 +0100 > +++ linux-2.6.26-rc9-next20080709/include/linux/mfd/core.h 2008-07-09 11:14:55.000000000 +0100 > @@ -18,8 +18,6 @@ > > /* > * This struct describes the MFD part ("cell"). > - * After registration the copy of this structure will become the platform data > - * of the resulting platform_device > */ > struct mfd_cell { > const char *name; > @@ -33,9 +31,21 @@ struct mfd_cell { > const struct resource *resources; > }; > > +struct mfd_device { > + struct platform_device pdev; > + struct mfd_cell *cell; > + struct resource resources[0]; > +}; > + > +static inline struct mfd_device *to_mfd_device(struct device *dev) > +{ > + struct platform_device *pdev = to_platform_device(dev); > + return container_of(pdev, struct mfd_device, pdev); > +} > + > static inline struct mfd_cell *mfd_get_cell(struct platform_device *pdev) > { > - return (struct mfd_cell *)pdev->dev.platform_data; > + return container_of(pdev, struct mfd_device, pdev)->cell; > } > > extern int mfd_add_devices(struct platform_device *parent, > Index: linux-2.6.26-rc9-next20080709/drivers/mfd/mfd-core.c > =================================================================== > --- linux-2.6.26-rc9-next20080709.orig/drivers/mfd/mfd-core.c 2008-07-09 10:59:54.000000000 +0100 > +++ linux-2.6.26-rc9-next20080709/drivers/mfd/mfd-core.c 2008-07-09 11:09:59.000000000 +0100 > @@ -15,28 +15,41 @@ > #include <linux/platform_device.h> > #include <linux/mfd/core.h> > > +static void mfd_dev_release(struct device *dev) > +{ > + struct mfd_device *mdev = to_mfd_device(dev); > + > + kfree(mdev); > +} > + > static int mfd_add_device(struct platform_device *parent, > const struct mfd_cell *cell, > struct resource *mem_base, > int irq_base) > { > - struct resource res[cell->num_resources]; > + struct resource *res; > + struct mfd_device *mdev; > struct platform_device *pdev; > int ret = -ENOMEM; > int r; > > - pdev = platform_device_alloc(cell->name, parent->id); > - if (!pdev) > + mdev = kzalloc(sizeof(struct mfd_device) + > + sizeof(struct resource) * cell->num_resources, > + GFP_KERNEL); > + if (!mdev) > goto fail_alloc; > > - pdev->dev.parent = &parent->dev; > + mdev->cell = cell; > + mdev->pdev.dev.parent = &parent->dev; > > - ret = platform_device_add_data(pdev, > - cell, sizeof(struct mfd_cell)); > - if (ret) > - goto fail_device; > + pdev = &mdev->pdev; > + res = &mdev->resources; > + > + device_initialise(&pdev->dev); > + pdev->id = parent->id; > + pdev->name = cell->name; > + pdev->release = mfd_device_release; > > - memzero(res, sizeof(res)); > for (r = 0; r < cell->num_resources; r++) { > res[r].name = cell->resources[r].name; > res[r].flags = cell->resources[r].flags; > > -- >
-- With best wishes Dmitry
| |