[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <bc64b4640807090415w781507fetfe332271ad158112@mail.gmail.com>
Date: Wed, 9 Jul 2008 15:15:47 +0400
From: Dmitry <dbaryshkov@...il.com>
To: "Ben Dooks" <ben-linux@...ff.org>
Cc: linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.arm.linux.org.uk, sameo@...nedhand.com
Subject: Re: [patch 4/4] MFD: Change mfd platform device usage to wrapper platform_device
2008/7/9 Ben Dooks <ben-linux@...ff.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@...ff.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists