Re: [PATCH 1/3] mfd: allow mfd_cell association with device tree node

2011-10-03 Thread Daniel Drake 
On Wed, Sep 28, 2011 at 1:31 PM, Mark Brown
broo...@opensource.wolfsonmicro.com wrote:
 Right, but your modification was to the MFD core so it's going to affect
 other devices...

But only when they start using the newly added functionality. At this
point, it affects nobody except vx855.

Grant, any chance I could have your comments on the latest patches?
http://marc.info/?l=linux-kernelm=131720353308737w=2
http://marc.info/?l=linux-kernelm=131720341808574w=2
http://marc.info/?l=linux-kernelm=131720093305636w=2

Thanks,
Daniel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 1/3] mfd: allow mfd_cell association with device tree node

2011-10-03 Thread Mark Brown 
On Mon, Oct 03, 2011 at 09:40:10AM +0100, Daniel Drake wrote:
 On Wed, Sep 28, 2011 at 1:31 PM, Mark Brown
  Right, but your modification was to the MFD core so it's going to affect
  other devices...

 But only when they start using the newly added functionality. At this
 point, it affects nobody except vx855.

I'd really expect that if we're adding stuff to the framework then it
should be suitable for random drivers to use.  To be honest I don't
really understand why you're changing the framework at all here, the
child driver is entirely specific to the parent as far as I can see.
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 1/3] mfd: allow mfd_cell association with device tree node

2011-10-03 Thread Mark Brown 
On Mon, Oct 03, 2011 at 11:39:47AM +0100, Daniel Drake wrote:
 On Mon, Oct 3, 2011 at 11:28 AM, Mark Brown

  I'd really expect that if we're adding stuff to the framework then it
  should be suitable for random drivers to use.

 It is suitable. If other drivers would otherwise run into the data
 reuse problem you have described, they can use the kmemdup solution
 you have described.

This seems the wrong way round - we're working

  To be honest I don't
  really understand why you're changing the framework at all here, the
  child driver is entirely specific to the parent as far as I can see.

 Because I'm trying to get devicetree-driven HDD LED support driven,
 and Grant stated that doing it this way is a hard rule:

 What is a hard rule is that the code creating the children needs to
 know what the binding is and populate the child's of_node
 appropriately.

 I also confirmed that extending the mfd_cell layer is exactly what
 Grant was suggesting:
 http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-September/008480.html

That's in the context of adding a binding for the vx855 GPIO module
that's distinct from the binding for the rest of the chip.  It's not
massively clear to me that this is a good idea.

 So I seem to be stuck between two people giving me conflicting
 requirements for merge of my work (which commenced in July, and has
 already seen several discussions and rounds of patches). Any help
 appreciated - I'm just trying to make a HDD LED blink.

It seems to me like either the IP block is heavily dependant on the core
and shouldn't be split out in the device tree at all (but should instead
be part of the core node) or the IP block is very isolated from the core
(in which case we should just be able to instantiate the device from the
device tree without using explict code in the core driver).

This all feels like there's some abstraction violation going on.
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 1/3] mfd: allow mfd_cell association with device tree node

2011-10-03 Thread Daniel Drake 
On Mon, Oct 3, 2011 at 1:16 PM, Mark Brown
broo...@opensource.wolfsonmicro.com wrote:
 It seems to me like either the IP block is heavily dependant on the core
 and shouldn't be split out in the device tree at all (but should instead
 be part of the core node) or the IP block is very isolated from the core
 (in which case we should just be able to instantiate the device from the
 device tree without using explict code in the core driver).

 This all feels like there's some abstraction violation going on.

I guess it is a matter of opinion. To me, the abstraction is sensible
and representative of the hardware.

The gpio controller (and several other components) are abstracted at
the hardware level behind an ISA bridge.
In the device tree, we have the ISA bridge represented as /pci/isa.

Then we have various child nodes of that ISA bridge, such as the gpio
controller at /pci/isa/gpios or the timer controller at /pci/isa/timer

This also matches the way that the hardware is described by VIA in the
specs, and it matches the way that Linux has its own device hierachy
laid out (i.e. the ISA bridge driven by the mfd driver, which then
spawns off a child device for the GPIO controller).

It would not be possible to fold all of the isa bridge child
components into the same device tree node without dealing with various
namespace collisions.

Grant, could we have your opinion on whether this is a good
abstraction or not? and generally how we go forward.

Thanks,
Daniel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 1/3] mfd: allow mfd_cell association with device tree node

2011-10-03 Thread Mark Brown 
On Mon, Oct 03, 2011 at 01:30:15PM +0100, Daniel Drake wrote:
 On Mon, Oct 3, 2011 at 1:16 PM, Mark Brown

  It seems to me like either the IP block is heavily dependant on the core
  and shouldn't be split out in the device tree at all (but should instead
  be part of the core node) or the IP block is very isolated from the core
  (in which case we should just be able to instantiate the device from the
  device tree without using explict code in the core driver).

  This all feels like there's some abstraction violation going on.

 I guess it is a matter of opinion. To me, the abstraction is sensible
 and representative of the hardware.

 This also matches the way that the hardware is described by VIA in the
 specs, and it matches the way that Linux has its own device hierachy
 laid out (i.e. the ISA bridge driven by the mfd driver, which then
 spawns off a child device for the GPIO controller).

 It would not be possible to fold all of the isa bridge child
 components into the same device tree node without dealing with various
 namespace collisions.

So, I made two suggestions above and it sounds like you want the second
one but you've only responded to the first one without commenting on the
second.  My second suggestion was that if the block is sufficiently
isoltated from the core we should be able instantiate it from the device
tree without requiring explicit code in the core driver.
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 1/3] mfd: allow mfd_cell association with device tree node

2011-10-03 Thread Daniel Drake 
On Mon, Oct 3, 2011 at 1:40 PM, Mark Brown
broo...@opensource.wolfsonmicro.com wrote:
 So, I made two suggestions above and it sounds like you want the second
 one but you've only responded to the first one without commenting on the
 second.  My second suggestion was that if the block is sufficiently
 isoltated from the core we should be able instantiate it from the device
 tree without requiring explicit code in the core driver.

Sorry, hadn't quite grasped what you meant there. I understand now.

It is an interesting idea but leaves me with some questions/problems:

The GPIO controller needs to find its register space by looking at the
PCI device (the ISA bridge). So probing it independently could maybe
be viewed by some as a hierarchy violation as it would have to then
hunt around for its PCI dev.

According to Grant's hard rule, the parent device needs to be the
thing that passes the of_node to the child.
So we would still need a driver for the parent ISA bridge
instantiating the child GPIO controller. Wouldn't that bring us
straight back to the same problem (that the core needs code to
instantiate the child)?

Also, not an argument against the direction, but an outstanding
problem that would need to be resolved: the x86 device tree
implementation doesn't seem to follow Grant's design for how things
should work (or maybe I misunderstood something), so work would be
needed there first. See the unfinished discussion at
http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-July/006853.html

Thanks,
Daniel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 1/3] mfd: allow mfd_cell association with device tree node

2011-09-28 Thread Daniel Drake 
On Tue, Sep 27, 2011 at 7:38 PM, Mark Brown
broo...@opensource.wolfsonmicro.com wrote:
 You're modifying global data which should really be const and is shared
 between multiple devices in place.  Probably you'll not notice any
 practical effects, especially if you don't happen to have multiple
 devices in the same system.

I see. In this case, it would be impossible to have two VX855s in the
same system, and if you did, you would have bigger problems. In this
context what we're doing is safe.

Thanks,
Daniel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

[PATCH 1/3] mfd: allow mfd_cell association with device tree node

2011-09-28 Thread Daniel Drake 
This allows a mfd_cell to be linked with a device tree node, which
then allows child drivers to have easy access to that handle.

Signed-off-by: Daniel Drake d...@laptop.org
---
 drivers/mfd/mfd-core.c   |1 +
 include/linux/mfd/core.h |4 
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
index 0902523..7d22dcd 100644
--- a/drivers/mfd/mfd-core.c
+++ b/drivers/mfd/mfd-core.c
@@ -87,6 +87,7 @@ static int mfd_add_device(struct device *parent, int id,
goto fail_device;
 
pdev-dev.parent = parent;
+   pdev-dev.of_node = cell-of_node;
 
if (cell-pdata_size) {
ret = platform_device_add_data(pdev,
diff --git a/include/linux/mfd/core.h b/include/linux/mfd/core.h
index 4e76163..9b836f9 100644
--- a/include/linux/mfd/core.h
+++ b/include/linux/mfd/core.h
@@ -15,6 +15,7 @@
 #define MFD_CORE_H
 
 #include linux/platform_device.h
+#include linux/of.h
 
 /*
  * This struct describes the MFD part (cell).
@@ -37,6 +38,9 @@ struct mfd_cell {
void*platform_data;
size_t  pdata_size;
 
+   /* association with device tree node (optional) */
+   struct device_node  *of_node;
+
/*
 * These resources can be specified relative to the parent device.
 * For accessing hardware you should use resources from the platform dev
-- 
1.7.6.2

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 1/3] mfd: allow mfd_cell association with device tree node

2011-09-28 Thread Mark Brown 
On Wed, Sep 28, 2011 at 10:07:43AM +0100, Daniel Drake wrote:
 On Tue, Sep 27, 2011 at 7:38 PM, Mark Brown

  You're modifying global data which should really be const and is shared
  between multiple devices in place.  Probably you'll not notice any
  practical effects, especially if you don't happen to have multiple
  devices in the same system.

 I see. In this case, it would be impossible to have two VX855s in the
 same system, and if you did, you would have bigger problems. In this
 context what we're doing is safe.

Right, but your modification was to the MFD core so it's going to affect
other devices...
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 1/3] mfd: allow mfd_cell association with device tree node

2011-09-27 Thread Daniel Drake 
On Wed, Sep 21, 2011 at 2:16 PM, Mark Brown
broo...@opensource.wolfsonmicro.com wrote:
 Not sure how the MFD cells could get at the OF node by using the
 parent device - if the parent device had a OF node, wouldn't this
 correspond to the parent instead of the child? Also, as far as I can

 Well, obviously.  But then with a lot of MFDs (including this one, the
 GPIO driver is entirely specific to the parent) it's not clear that we
 should be splitting the device up in the device tree in the first place
 - our use of MFDs is a Linux implementation detail.  If the child is
 specific to the parent it can cooperate with the parent device happily.

 My suspicion is that for device tree in cases where the MFD really is
 totally independent of the parent we shouldn't need explicit MFD code to
 instantiate the child at all any more in the same way that we should be
 avoiding this for the SoCs.

The VX855 is somewhat a SoC if you ignore the fact that the processor
is separate.
The software-visible architecture is somewhat messy and may hide this however.

The fact that it exposes some things as individual PCI devices and
some as behind the ISA bridge (which the mfd driver latches onto) is
probably just there for legacy reasons. Once you get behind that
cover, you see that the VX855 register space is really quite
disorganised with individual bits within the same byte of
configuration space used for vastly different system components (e.g.
gpio bits mixed with acpi and audio stuff in the same byte) and you
get the feeling that this really is a lot of hardware combined. So the
discussion of independence is particularly tricky in this case.

Anyway, I think I have come up with an approach on these lines. The
mfd driver latches onto the ISA bridge, and the documented
architecture suggests that a whole bunch of stuff comes off this:
8042, interrupt controller, dma controller, rtc, serial, timer, gpio,
spi, ...

We already have this accurately laid out in the device tree hierarchy:
/pci/isa/ has a child node for each of the above mentioned things
(e.g. /pci/isa/timer , /pci/isa/rtc and so on)

So, the /pci/isa node nicely matches the vx855-mfd driver, so we can
assign its of_node to that. Then, the vx855-gpio driver can consult
the parent and then look at its children in order to find the of_node
for the gpio controller. I think this nicely crosses with Mark's
request that the ability to have constant mfd definitions isn't broken
any more than it is already, and with Grant's preference that the
parent resource has some contribution in helping the child gpio
controller driver find its of_node. How does the attached patch look?

Grant, what do you think of this, and the rest of the discussion so far?

Daniel (just trying to make our gpio-based HDD LED go blinky blink...)
diff --git a/drivers/gpio/gpio-vx855.c b/drivers/gpio/gpio-vx855.c
index ef5aabd..fd24213 100644
--- a/drivers/gpio/gpio-vx855.c
+++ b/drivers/gpio/gpio-vx855.c
@@ -31,6 +31,7 @@
 #include linux/platform_device.h
 #include linux/pci.h
 #include linux/io.h
+#include linux/of.h
 
 #define MODULE_NAME vx855_gpio
 
@@ -201,7 +202,27 @@ static const char *vx855gpio_names[NR_VX855_GP] = {
VX855_GPIO12, VX855_GPIO13, VX855_GPIO14
 };
 
-static void vx855gpio_gpio_setup(struct vx855_gpio *vg)
+static void vx855gpio_set_of_node(struct platform_device *pdev,
+ struct gpio_chip *c)
+{
+#ifdef CONFIG_OF
+   struct device_node *parent_node = pdev-dev.parent-of_node;
+   struct device_node *child;
+
+   if (!parent_node)
+   return;
+
+   for_each_child_of_node(parent_node, child) {
+   if (of_device_is_compatible(child, via,vx855-gpio)) {
+   c-of_node = child;
+   break;
+   }
+   }
+#endif
+}
+
+static void vx855gpio_gpio_setup(struct vx855_gpio *vg,
+struct platform_device *pdev)
 {
struct gpio_chip *c = vg-gpio;
 
@@ -216,6 +237,7 @@ static void vx855gpio_gpio_setup(struct vx855_gpio *vg)
c-ngpio = NR_VX855_GP;
c-can_sleep = 0;
c-names = vx855gpio_names;
+   vx855gpio_set_of_node(pdev, c);
 }
 
 /* This platform device is ordinarily registered by the vx855 mfd driver */
@@ -264,7 +286,7 @@ static __devinit int vx855gpio_probe(struct platform_device 
*pdev)
else
vg-gpo_reserved = true;
 
-   vx855gpio_gpio_setup(vg);
+   vx855gpio_gpio_setup(vg, pdev);
 
ret = gpiochip_add(vg-gpio);
if (ret) {
diff --git a/drivers/mfd/vx855.c b/drivers/mfd/vx855.c
index d698703..aee9a5f 100644
--- a/drivers/mfd/vx855.c
+++ b/drivers/mfd/vx855.c
@@ -30,6 +30,7 @@
 #include linux/platform_device.h
 #include linux/pci.h
 #include linux/mfd/core.h
+#include linux/of.h
 
 /* offset into pci config space indicating the 16bit register containing
  * the power management IO space base */
@@ -90,6 +91,10 @@ static __devinit int

Re: [PATCH 1/3] mfd: allow mfd_cell association with device tree node

2011-09-27 Thread Grant Likely 
On Tue, Sep 27, 2011 at 03:44:56PM +0100, Daniel Drake wrote:
 On Wed, Sep 21, 2011 at 2:16 PM, Mark Brown
 broo...@opensource.wolfsonmicro.com wrote:
  Not sure how the MFD cells could get at the OF node by using the
  parent device - if the parent device had a OF node, wouldn't this
  correspond to the parent instead of the child? Also, as far as I can
 
  Well, obviously.  But then with a lot of MFDs (including this one, the
  GPIO driver is entirely specific to the parent) it's not clear that we
  should be splitting the device up in the device tree in the first place
  - our use of MFDs is a Linux implementation detail.  If the child is
  specific to the parent it can cooperate with the parent device happily.
 
  My suspicion is that for device tree in cases where the MFD really is
  totally independent of the parent we shouldn't need explicit MFD code to
  instantiate the child at all any more in the same way that we should be
  avoiding this for the SoCs.

Right. MFD seems to be most useful when IP blocks are used in multiple
places and can be instantiated by multiple parents.  Sometimes a
driver really should just register the interfaces that the device
provides without the MFD framework.

 The VX855 is somewhat a SoC if you ignore the fact that the processor
 is separate.
 The software-visible architecture is somewhat messy and may hide this however.
 
 The fact that it exposes some things as individual PCI devices and
 some as behind the ISA bridge (which the mfd driver latches onto) is
 probably just there for legacy reasons. Once you get behind that
 cover, you see that the VX855 register space is really quite
 disorganised with individual bits within the same byte of
 configuration space used for vastly different system components (e.g.
 gpio bits mixed with acpi and audio stuff in the same byte) and you
 get the feeling that this really is a lot of hardware combined. So the
 discussion of independence is particularly tricky in this case.
 
 Anyway, I think I have come up with an approach on these lines. The
 mfd driver latches onto the ISA bridge, and the documented
 architecture suggests that a whole bunch of stuff comes off this:
 8042, interrupt controller, dma controller, rtc, serial, timer, gpio,
 spi, ...
 
 We already have this accurately laid out in the device tree hierarchy:
 /pci/isa/ has a child node for each of the above mentioned things
 (e.g. /pci/isa/timer , /pci/isa/rtc and so on)
 
 So, the /pci/isa node nicely matches the vx855-mfd driver, so we can
 assign its of_node to that. Then, the vx855-gpio driver can consult
 the parent and then look at its children in order to find the of_node
 for the gpio controller. I think this nicely crosses with Mark's
 request that the ability to have constant mfd definitions isn't broken
 any more than it is already, and with Grant's preference that the
 parent resource has some contribution in helping the child gpio
 controller driver find its of_node. How does the attached patch look?
 
 Grant, what do you think of this, and the rest of the discussion so far?

MFD bindings really need to be designed on a device-by-device basis.
Sometimes it will make more sense to have a single node, sometimes it
needs to be split up.  There is no hard rule here.

What is a hard rule is that the code creating the children needs to
know what the binding is and populate the child's of_node
appropriately.  Similarly, the child driver needs to know something
about the kinds of nodes it will be passed (can be tested by the
compatible property).

One warning however, the current platform_bus_type DT code won't deal
well with multiple platform_devices pointing to the same of_node.  It
may result in the wrong driver getting bound since the probe code
first looks at .compatible to decide which driver to probe.  If 3
separate devices are intended to bind to 3 different drivers, you'll
probably end up with all three matching against the first one unless
the drivers have some additional explicit test.

g.

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 1/3] mfd: allow mfd_cell association with device tree node

2011-09-27 Thread Mark Brown 
On Tue, Sep 27, 2011 at 09:05:55AM -0600, Grant Likely wrote:
 On Tue, Sep 27, 2011 at 03:44:56PM +0100, Daniel Drake wrote:
  On Wed, Sep 21, 2011 at 2:16 PM, Mark Brown

   My suspicion is that for device tree in cases where the MFD really is
   totally independent of the parent we shouldn't need explicit MFD code to
   instantiate the child at all any more in the same way that we should be
   avoiding this for the SoCs.

 Right. MFD seems to be most useful when IP blocks are used in multiple
 places and can be instantiated by multiple parents.  Sometimes a
 driver really should just register the interfaces that the device
 provides without the MFD framework.

Well, if you need a bunch of platform devices it's a good way of
creating them especially in the current world.  There's also generally
some core logic, for example routing interrupt lines, that can usefully
be provided by the MFD part of the driver.
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 1/3] mfd: allow mfd_cell association with device tree node

2011-09-27 Thread Daniel Drake 
On Tue, Sep 27, 2011 at 4:05 PM, Grant Likely grant.lik...@secretlab.ca wrote:
 What is a hard rule is that the code creating the children needs to
 know what the binding is and populate the child's of_node
 appropriately.  Similarly, the child driver needs to know something
 about the kinds of nodes it will be passed (can be tested by the
 compatible property).

Thanks for your input.
Here is an updated patch which takes account of this hard rule.

It uses the dynamic mfd cell creation like before. Mark isn't keen on
this, but its how the driver works already, so I don't think it should
be a blocker. I don't know how else we can make the mfd framework meet
Grant's hard rule.

However, it does take Mark's suggestion into account that the mfd
should have some clear representation in the device tree. For us it
already did have (naturally), but this wasn't reflected in my earlier
patch. It is now - the location of the vx855-gpio node is based on
finding the mfd node and going from there.

Comments?
It obviously needs to be tested and split up into individual patches -
I'll do that shortly if this doesn't get shot down.

cheers
Daniel
diff --git a/drivers/gpio/gpio-vx855.c b/drivers/gpio/gpio-vx855.c
index ef5aabd..2c300c9 100644
--- a/drivers/gpio/gpio-vx855.c
+++ b/drivers/gpio/gpio-vx855.c
@@ -201,7 +201,8 @@ static const char *vx855gpio_names[NR_VX855_GP] = {
VX855_GPIO12, VX855_GPIO13, VX855_GPIO14
 };
 
-static void vx855gpio_gpio_setup(struct vx855_gpio *vg)
+static void vx855gpio_gpio_setup(struct vx855_gpio *vg,
+struct platform_device *pdev)
 {
struct gpio_chip *c = vg-gpio;
 
@@ -216,6 +217,10 @@ static void vx855gpio_gpio_setup(struct vx855_gpio *vg)
c-ngpio = NR_VX855_GP;
c-can_sleep = 0;
c-names = vx855gpio_names;
+
+#ifdef CONFIG_OF_GPIO
+   c-of_node = pdev-dev.of_node;
+#endif
 }
 
 /* This platform device is ordinarily registered by the vx855 mfd driver */
@@ -264,7 +269,7 @@ static __devinit int vx855gpio_probe(struct platform_device 
*pdev)
else
vg-gpo_reserved = true;
 
-   vx855gpio_gpio_setup(vg);
+   vx855gpio_gpio_setup(vg, pdev);
 
ret = gpiochip_add(vg-gpio);
if (ret) {
diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
index 0902523..7d22dcd 100644
--- a/drivers/mfd/mfd-core.c
+++ b/drivers/mfd/mfd-core.c
@@ -87,6 +87,7 @@ static int mfd_add_device(struct device *parent, int id,
goto fail_device;
 
pdev-dev.parent = parent;
+   pdev-dev.of_node = cell-of_node;
 
if (cell-pdata_size) {
ret = platform_device_add_data(pdev,
diff --git a/drivers/mfd/vx855.c b/drivers/mfd/vx855.c
index d698703..d223840 100644
--- a/drivers/mfd/vx855.c
+++ b/drivers/mfd/vx855.c
@@ -30,6 +30,7 @@
 #include linux/platform_device.h
 #include linux/pci.h
 #include linux/mfd/core.h
+#include linux/of.h
 
 /* offset into pci config space indicating the 16bit register containing
  * the power management IO space base */
@@ -72,6 +73,27 @@ static struct mfd_cell vx855_cells[] = {
},
 };
 
+static __devinit void vx855_set_of_nodes(struct pci_dev *pdev)
+{
+#ifdef CONFIG_OF
+   struct device_node *isa_node;
+   struct device_node *child;
+
+   isa_node = of_find_compatible_node(NULL, NULL, via,vx855-isa);
+   pdev-dev.of_node = isa_node;
+
+   if (!isa_node)
+   return;
+
+   for_each_child_of_node(isa_node, child) {
+   if (of_device_is_compatible(child, via,vx855-gpio)) {
+   vx855_cells[0].of_node = child;
+   break;
+   }
+   }
+#endif
+}
+
 static __devinit int vx855_probe(struct pci_dev *pdev,
 const struct pci_device_id *id)
 {
@@ -90,6 +112,8 @@ static __devinit int vx855_probe(struct pci_dev *pdev,
goto out;
}
 
+   vx855_set_of_nodes(pdev);
+
/* mask out the lowest seven bits, as they are always zero, but
 * hardware returns them as 0x01 */
gpio_io_offset = 0xff80;
diff --git a/include/linux/mfd/core.h b/include/linux/mfd/core.h
index 4e76163..9b836f9 100644
--- a/include/linux/mfd/core.h
+++ b/include/linux/mfd/core.h
@@ -15,6 +15,7 @@
 #define MFD_CORE_H
 
 #include linux/platform_device.h
+#include linux/of.h
 
 /*
  * This struct describes the MFD part (cell).
@@ -37,6 +38,9 @@ struct mfd_cell {
void*platform_data;
size_t  pdata_size;
 
+   /* association with device tree node (optional) */
+   struct device_node  *of_node;
+
/*
 * These resources can be specified relative to the parent device.
 * For accessing hardware you should use resources from the platform dev
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 1/3] mfd: allow mfd_cell association with device tree node

2011-09-27 Thread Mark Brown 
On Tue, Sep 27, 2011 at 05:44:04PM +0100, Daniel Drake wrote:

 However, it does take Mark's suggestion into account that the mfd
 should have some clear representation in the device tree. For us it
 already did have (naturally), but this wasn't reflected in my earlier
 patch. It is now - the location of the vx855-gpio node is based on
 finding the mfd node and going from there.

Why not just kmemdup() the template you're using rather than modifying
it in place?
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 1/3] mfd: allow mfd_cell association with device tree node

2011-09-27 Thread Daniel Drake 
On Tue, Sep 27, 2011 at 7:14 PM, Mark Brown
broo...@opensource.wolfsonmicro.com wrote:
 On Tue, Sep 27, 2011 at 05:44:04PM +0100, Daniel Drake wrote:

 However, it does take Mark's suggestion into account that the mfd
 should have some clear representation in the device tree. For us it
 already did have (naturally), but this wasn't reflected in my earlier
 patch. It is now - the location of the vx855-gpio node is based on
 finding the mfd node and going from there.

 Why not just kmemdup() the template you're using rather than modifying
 it in place?

Could do, but is there any point in this case? I'm not seeing it. The
template is already being modified in this way in the current driver
as well.

Daniel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 1/3] mfd: allow mfd_cell association with device tree node

2011-09-27 Thread Mark Brown 
On Tue, Sep 27, 2011 at 07:25:03PM +0100, Daniel Drake wrote:
 On Tue, Sep 27, 2011 at 7:14 PM, Mark Brown

  Why not just kmemdup() the template you're using rather than modifying
  it in place?

 Could do, but is there any point in this case? I'm not seeing it. The
 template is already being modified in this way in the current driver
 as well.

That seems like a bug that just happened to get noticed while reviewing
this change, though...
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 1/3] mfd: allow mfd_cell association with device tree node

2011-09-27 Thread Daniel Drake 
On Tue, Sep 27, 2011 at 7:26 PM, Mark Brown
broo...@opensource.wolfsonmicro.com wrote:
 That seems like a bug that just happened to get noticed while reviewing
 this change, though...

Sorry if I'm missing something obvious but I'm not seeing the issue.
What exactly is this bug and what effects could it cause?

Thanks,
Daniel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 1/3] mfd: allow mfd_cell association with device tree node

2011-09-27 Thread Mark Brown 
On Tue, Sep 27, 2011 at 07:28:40PM +0100, Daniel Drake wrote:
 On Tue, Sep 27, 2011 at 7:26 PM, Mark Brown

  That seems like a bug that just happened to get noticed while reviewing
  this change, though...

 Sorry if I'm missing something obvious but I'm not seeing the issue.
 What exactly is this bug and what effects could it cause?

You're modifying global data which should really be const and is shared
between multiple devices in place.  Probably you'll not notice any
practical effects, especially if you don't happen to have multiple
devices in the same system.
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

[PATCH 1/3] mfd: allow mfd_cell association with device tree node

2011-09-21 Thread Daniel Drake 
This allows a mfd_cell to be linked with a device tree node, which
then allows child drivers to have easy access to that handle.

Signed-off-by: Daniel Drake d...@laptop.org
---
 drivers/mfd/mfd-core.c   |1 +
 include/linux/mfd/core.h |4 
 2 files changed, 5 insertions(+), 0 deletions(-)

I think this is what is being suggested at 
http://lists.ozlabs.org/pipermail/devicetree-discuss/2011-September/008235.html

diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
index 0902523..7d22dcd 100644
--- a/drivers/mfd/mfd-core.c
+++ b/drivers/mfd/mfd-core.c
@@ -87,6 +87,7 @@ static int mfd_add_device(struct device *parent, int id,
goto fail_device;
 
pdev-dev.parent = parent;
+   pdev-dev.of_node = cell-of_node;
 
if (cell-pdata_size) {
ret = platform_device_add_data(pdev,
diff --git a/include/linux/mfd/core.h b/include/linux/mfd/core.h
index 4e76163..9b836f9 100644
--- a/include/linux/mfd/core.h
+++ b/include/linux/mfd/core.h
@@ -15,6 +15,7 @@
 #define MFD_CORE_H
 
 #include linux/platform_device.h
+#include linux/of.h
 
 /*
  * This struct describes the MFD part (cell).
@@ -37,6 +38,9 @@ struct mfd_cell {
void*platform_data;
size_t  pdata_size;
 
+   /* association with device tree node (optional) */
+   struct device_node  *of_node;
+
/*
 * These resources can be specified relative to the parent device.
 * For accessing hardware you should use resources from the platform dev
-- 
1.7.6.2

___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 1/3] mfd: allow mfd_cell association with device tree node

2011-09-21 Thread Mark Brown 
On Wed, Sep 21, 2011 at 01:01:48PM +0100, Daniel Drake wrote:

 @@ -37,6 +38,9 @@ struct mfd_cell {
   void*platform_data;
   size_t  pdata_size;
  
 + /* association with device tree node (optional) */
 + struct device_node  *of_node;
 +

This doesn't seem great as it means that the mfd_cells can't be static
data, they have to be allocated per device.  Why can't the MFD cells get
at the OF node by using the parent device in the same way as they
(mostly) get platform data at the minute?
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 1/3] mfd: allow mfd_cell association with device tree node

2011-09-21 Thread Daniel Drake 
On Wed, Sep 21, 2011 at 1:49 PM, Mark Brown
broo...@opensource.wolfsonmicro.com wrote:
 On Wed, Sep 21, 2011 at 01:01:48PM +0100, Daniel Drake wrote:

 @@ -37,6 +38,9 @@ struct mfd_cell {
       void                    *platform_data;
       size_t                  pdata_size;

 +     /* association with device tree node (optional) */
 +     struct device_node      *of_node;
 +

 This doesn't seem great as it means that the mfd_cells can't be static
 data, they have to be allocated per device.  Why can't the MFD cells get
 at the OF node by using the parent device in the same way as they
 (mostly) get platform data at the minute?

Thanks for reviewing.
The data can still be static, not needing allocation, it just has to
be modified at runtime. See patch 2.

Not sure how the MFD cells could get at the OF node by using the
parent device - if the parent device had a OF node, wouldn't this
correspond to the parent instead of the child? Also, as far as I can
see, platform data is passed to the child in exactly the same way - by
including it in the mfd_cell definition - see mfd_add_device():

if (cell-pdata_size) {
ret = platform_device_add_data(pdev,
cell-platform_data, cell-pdata_size);
if (ret)
goto fail_res;
}

Daniel
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: [PATCH 1/3] mfd: allow mfd_cell association with device tree node

2011-09-21 Thread Mark Brown 
On Wed, Sep 21, 2011 at 02:02:43PM +0100, Daniel Drake wrote:

 Thanks for reviewing.
 The data can still be static, not needing allocation, it just has to
 be modified at runtime. See patch 2.

Right, but that means that you need to take a copy before using it
otherwise two devices of the same type might get into a fight with each
other.

 Not sure how the MFD cells could get at the OF node by using the
 parent device - if the parent device had a OF node, wouldn't this
 correspond to the parent instead of the child? Also, as far as I can

Well, obviously.  But then with a lot of MFDs (including this one, the
GPIO driver is entirely specific to the parent) it's not clear that we
should be splitting the device up in the device tree in the first place
- our use of MFDs is a Linux implementation detail.  If the child is
specific to the parent it can cooperate with the parent device happily.

My suspicion is that for device tree in cases where the MFD really is
totally independent of the parent we shouldn't need explicit MFD code to
instantiate the child at all any more in the same way that we should be
avoiding this for the SoCs.

 see, platform data is passed to the child in exactly the same way - by
 including it in the mfd_cell definition - see mfd_add_device():

Indeed, I have the same concerns there.  
___
devicetree-discuss mailing list
devicetree-discuss@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss