Message-ID: <bc64b4640807090431g5b1960edua2508337973615da@mail.gmail.com>
Date:	Wed, 9 Jul 2008 15:31:04 +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>:
> On Wed, Jul 09, 2008 at 03:15:47PM +0400, Dmitry wrote:
>> 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.
>
> How "more correct", whilst the patch by Mike makes the platform data
> be passed from the cell, there is no longer any way to get from the
> platform device to the mfd_cell...

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.


> 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.

-- 
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