Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1483424 > unrolled thread

[PATCH] of/platform: Initialise dev->fwnode appropriately

Back to article view | Back to linux.kernel

Contents

  [PATCH] of/platform: Initialise dev->fwnode appropriately Robin Murphy <robin.murphy@arm.com> - 2016-09-14 17:10 +0200
    Re: [PATCH] of/platform: Initialise dev->fwnode appropriately Rob Herring <robh@kernel.org> - 2016-09-15 15:40 +0200
      Re: [PATCH] of/platform: Initialise dev->fwnode appropriately Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> - 2016-09-16 13:20 +0200
      Re: [PATCH] of/platform: Initialise dev->fwnode appropriately Robin Murphy <robin.murphy@arm.com> - 2016-09-16 14:10 +0200

#1483424 — [PATCH] of/platform: Initialise dev->fwnode appropriately

Whilst we're some of the way towards a universal firmware property
interface, drivers which deal with both OF and ACPI probing end up
having to do things like this:

    dev->of_node ? &dev->of_node->fwnode : dev->fwnode

This seems unnecessary, when the OF code could instead simply fill in
the device's fwnode when binding the of_node, and let the drivers use
dev->fwnode either way. Let's give it a go and see what falls out.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 drivers/of/platform.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index f39ccd5aa701..f811d2796437 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -142,6 +142,7 @@ struct platform_device *of_device_alloc(struct device_node *np,
 	}
 
 	dev->dev.of_node = of_node_get(np);
+	dev->dev.fwnode = &np->fwnode;
 	dev->dev.parent = parent ? : &platform_bus;
 
 	if (bus_id)
@@ -241,6 +242,7 @@ static struct amba_device *of_amba_device_create(struct device_node *node,
 
 	/* setup generic device info */
 	dev->dev.of_node = of_node_get(node);
+	dev->dev.fwnode = &node->fwnode;
 	dev->dev.parent = parent ? : &platform_bus;
 	dev->dev.platform_data = platform_data;
 	if (bus_id)
-- 
2.8.1.dirty

[toc] | [next] | [standalone]

#1484177

On Wed, Sep 14, 2016 at 04:01:24PM +0100, Robin Murphy wrote:
> Whilst we're some of the way towards a universal firmware property
> interface, drivers which deal with both OF and ACPI probing end up
> having to do things like this:
> 
>     dev->of_node ? &dev->of_node->fwnode : dev->fwnode
> 
> This seems unnecessary, when the OF code could instead simply fill in
> the device's fwnode when binding the of_node, and let the drivers use
> dev->fwnode either way. Let's give it a go and see what falls out.
> 
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>  drivers/of/platform.c | 2 ++
>  1 file changed, 2 insertions(+)

I've applied this, but what about non-platform devices such as i2c?

Rob

[toc] | [prev] | [next] | [standalone]

#1484894

On Thu, Sep 15, 2016 at 08:36:57AM -0500, Rob Herring wrote:
> On Wed, Sep 14, 2016 at 04:01:24PM +0100, Robin Murphy wrote:
> > Whilst we're some of the way towards a universal firmware property
> > interface, drivers which deal with both OF and ACPI probing end up
> > having to do things like this:
> > 
> >     dev->of_node ? &dev->of_node->fwnode : dev->fwnode
> > 
> > This seems unnecessary, when the OF code could instead simply fill in
> > the device's fwnode when binding the of_node, and let the drivers use
> > dev->fwnode either way. Let's give it a go and see what falls out.
> > 
> > Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> > ---
> >  drivers/of/platform.c | 2 ++
> >  1 file changed, 2 insertions(+)
> 
> I've applied this, but what about non-platform devices such as i2c?

Thanks ! Patch below should do for mfd and i2c (to be confirmed) but
I am pretty certain it is still missing some devices, are we going
to convert them on a case-by-case policy (ie when/if needed) ?

Lorenzo

-- >8 --
diff --git a/drivers/i2c/i2c-core.c b/drivers/i2c/i2c-core.c
index da3a02e..667a393 100644
--- a/drivers/i2c/i2c-core.c
+++ b/drivers/i2c/i2c-core.c
@@ -1574,6 +1574,7 @@ static struct i2c_client *of_i2c_register_device(struct i2c_adapter *adap,
 
 	info.addr = addr;
 	info.of_node = of_node_get(node);
+	info.fwnode = &node->fwnode;
 	info.archdata = &dev_ad;
 
 	if (of_get_property(node, "wakeup-source", NULL))
diff --git a/drivers/mfd/mfd-core.c b/drivers/mfd/mfd-core.c
index 3ac486a..c264bf5 100644
--- a/drivers/mfd/mfd-core.c
+++ b/drivers/mfd/mfd-core.c
@@ -179,6 +179,7 @@ static int mfd_add_device(struct device *parent, int id,
 		for_each_child_of_node(parent->of_node, np) {
 			if (of_device_is_compatible(np, cell->of_compatible)) {
 				pdev->dev.of_node = np;
+				pdev->dev.fwnode = &np->fwnode;
 				break;
 			}
 		}

[toc] | [prev] | [next] | [standalone]

#1484929

On 15/09/16 14:36, Rob Herring wrote:
> On Wed, Sep 14, 2016 at 04:01:24PM +0100, Robin Murphy wrote:
>> Whilst we're some of the way towards a universal firmware property
>> interface, drivers which deal with both OF and ACPI probing end up
>> having to do things like this:
>>
>>     dev->of_node ? &dev->of_node->fwnode : dev->fwnode
>>
>> This seems unnecessary, when the OF code could instead simply fill in
>> the device's fwnode when binding the of_node, and let the drivers use
>> dev->fwnode either way. Let's give it a go and see what falls out.
>>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>> ---
>>  drivers/of/platform.c | 2 ++
>>  1 file changed, 2 insertions(+)
> 
> I've applied this, but what about non-platform devices such as i2c?

I was indeed wondering that, coming from the perspective of DMA/IOMMU
configuration which doesn't really apply beyond the PCI and platform
buses. More generally, at this point it's largely for the benefit of
subsystems which need to chuck around firmware data for other devices
that don't exist yet (or at all), which maybe is just the DMA and IRQ
layers.

I guess beyond that it comes down to auditing the intersection between
callers of the property API and OF-probed buses, then killing the
dev_fwnode() helper in property.c and making sure nothing else breaks.
The possible alternative would be pushing this right down into
device_initialize(), but that seems a bit wrong.

Thanks,
Robin.

> 
> Rob
> 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web