Message-ID: <AANLkTi=ypose5day4=ypV8_SsiFq4vf9vqZh-OWret0x@mail.gmail.com>
Date:	Thu, 16 Dec 2010 15:38:52 +0000
From:	Daniel Drake <dsd@...top.org>
To:	Samuel Ortiz <sameo@...ux.intel.com>
Cc:	Paul Fox <pgf@...top.org>, Andres Salomon <dilinger@...ued.net>,
	linux-kernel@...r.kernel.org
Subject: Re: MFD cell structure and sharing of resources

Hi Samuel,

Thanks for your valuable input here!

On 16 December 2010 10:35, Samuel Ortiz <sameo@...ux.intel.com> wrote:
>> 2. Continue with Andres' olpc-xo1-pm patch that makes that driver act
>> as a driver for both acpi & pms. When both resources are available, it
>> could probe an olpc-xo1-sci platform device as a child of itself,
>> having the ACPI resource shared on the platform_device level (similar
>> to how MFD shares resources via cells).
> This sounds like an acceptable solution to me.

This is also the one that stuck out to me.

There are complications though.


Firstly, passing down the resources from mfd to olpc-xo1-pm, then from
olpc-xo1-pm using "struct resource" doesn't work - the 2nd passing
down fails because the platform layer checks that the resources are
free.

This can be worked around by using platform_data to pass the required
info from olpc-xo1-pm to olpc-xo1-sci (the base register addresses).


Secondly, the olpc-xo1-pm driver is going to have a couple of sysfs
nodes that will be used by userspace.

Under the current design they appear as e.g.:
/sys/devices/pci0000:00/0000:00:0f.0/cs5535-acpi/wakeup_reason

However, it only appears under cs5535-acpi because that's the device
that appeared second (out of acpi + pms). If the probe order ever
changed, the path would change too.
This could be worked around by storing both pointers (acpi + pms) and
choosing one that the olpc-xo1-pm parts will always live under. But as
this detail represents an interface to userspace we should be careful
and try and get it right first time. That wakeup_reason node is not
really related to cs5535, it's an OLPC-specific thing (since the
wakeups can be caused by things totally separate from the geode hw).
So I'd feel a lot more comfortable if that path was less related to
cs5535.

This problem extends to olpc-xo1-sci which becomes a child of
olpc-xo1-pm, and creates another bunch of sysfs nodes e.g.
/sys/devices/pci0000:00/0000:00:0f.0/cs5535-acpi/olpc-xo1-sci/lid_wake_mode



I have one solution in mind but I'm not sure if it goes beyond your
definition of what a mfd cell should be:

cs5535-mfd creates and registers a MFD cell specifically for
olpc-xo1-pm if it finds itself running on an XO laptop. The cell has
the 2 resources that are needed by that driver, and has name
"olpc-xo1-pm". Therefore our sysfs paths start with:
/sys/devices/pci0000:00/0000:00:0f.0/olpc-xo1-pm/

olpc-xo1-pm is then simplified and doesn't have to pretend to be a
driver for 2 devices. It just waits for that mfd cell to cause it to
be probed, then immediately has both resources available to it.

The first problem is also solved (clean sharing of resources between
olpc-xo1-pm and olpc-xo1-sci). olpc-xo1-pm creates olpc-xo1-sci as a
child device, therefore olpc-xo1-sci can access the resources of its
parent as follows:
    platform_get_resource(to_platform_device(pdev->dev.parent),
IORESOURCE_IO, 0);

Attaching a cs5535-mfd patch to illustrate what I mean a bit clearer
(compile tested only). The OLPC-specific code in cs5536-mfd compiles
away to nothing on CONFIG_OLPC=n

Thoughts?

Daniel

View attachment "mfd-scratch.txt" of type "text/plain" (4798 bytes)

Powered by blists - more mailing lists