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

[Daniel Drake]

On Tue, Sep 27, 2011 at 4:05 PM, Grant Likely <grant.likely@xxxxxxxxxxxx> 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