From: Heikki Krogerus <heikki.krogerus@linux.intel.com> Date: 2015-01-26 13:17:43
This extends the unified device property interface by adding
"Generic Property" to it for cases where device tree or ACPI
are not being used.
That makes the unified device property interface cover also
most of the cases where information is extracted from custom
platform_data in the drivers. So if before we had to check
separately is there custom platform_data for a driver:
if (pdata)
bar = pdata->bar;
else
device_property_read_u32(dev, "foo", &bar);
we can now drop that check and simply always use the unified
device property interface.
That makes it possible to drop a lot of boiler plate from
the drivers, plus quite a few header files under
include/linux/ describing those driver specific platform
data structures can be removed.
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
drivers/base/property.c | 135 ++++++++++++++++++++++++++++++++++++++++++-----
include/linux/device.h | 3 ++
include/linux/property.h | 17 ++++++
3 files changed, 143 insertions(+), 12 deletions(-)
From: Heikki Krogerus <heikki.krogerus@linux.intel.com> Date: 2015-03-09 14:45:30
On Mon, Jan 26, 2015 at 03:17:40PM +0200, Heikki Krogerus wrote:
This extends the unified device property interface by adding
"Generic Property" to it for cases where device tree or ACPI
are not being used.
That makes the unified device property interface cover also
most of the cases where information is extracted from custom
platform_data in the drivers. So if before we had to check
separately is there custom platform_data for a driver:
if (pdata)
bar = pdata->bar;
else
device_property_read_u32(dev, "foo", &bar);
we can now drop that check and simply always use the unified
device property interface.
That makes it possible to drop a lot of boiler plate from
the drivers, plus quite a few header files under
include/linux/ describing those driver specific platform
data structures can be removed.
From: Rafael J. Wysocki <hidden> Date: 2015-03-12 01:17:48
On Monday, January 26, 2015 03:17:40 PM Heikki Krogerus wrote:
quoted hunk
This extends the unified device property interface by adding
"Generic Property" to it for cases where device tree or ACPI
are not being used.
That makes the unified device property interface cover also
most of the cases where information is extracted from custom
platform_data in the drivers. So if before we had to check
separately is there custom platform_data for a driver:
if (pdata)
bar = pdata->bar;
else
device_property_read_u32(dev, "foo", &bar);
we can now drop that check and simply always use the unified
device property interface.
That makes it possible to drop a lot of boiler plate from
the drivers, plus quite a few header files under
include/linux/ describing those driver specific platform
data structures can be removed.
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
drivers/base/property.c | 135 ++++++++++++++++++++++++++++++++++++++++++-----
include/linux/device.h | 3 ++
include/linux/property.h | 17 ++++++
3 files changed, 143 insertions(+), 12 deletions(-)
Use memcpy() perhaps? And below too? And then you may not need these
helpers any more.
quoted hunk
+ return 0;+}++static int dev_prop_copy_array_u16(u16 *src, u16 *val, size_t nval)+{+ int i;++ for (i = 0; i < nval; i++)+ val[i] = src[i];+ return 0;+}++static int dev_prop_copy_array_u32(u32 *src, u32 *val, size_t nval)+{+ int i;++ for (i = 0; i < nval; i++)+ val[i] = src[i];+ return 0;+}++static int dev_prop_copy_array_u64(u64 *src, u64 *val, size_t nval)+{+ int i;++ for (i = 0; i < nval; i++)+ val[i] = src[i];+ return 0;+}++static int dev_prop_copy_array_string(const char **src, const char **val,+ size_t nval)+{+ int i;++ for (i = 0; i < nval; i++)+ val[i] = src[i];+ return 0;+}++static int dev_prop_read_array(struct device *dev, const char *name,+ enum dev_prop_type type, void *val, size_t nval)+{+ struct dev_gen_prop *prop;+ int ret = 0;++ prop = dev_prop_get(dev, name);+ if (!prop)+ return -ENODATA;++ if (prop->type != type)+ return -EPROTO;++ if (prop->nval < nval)+ return -EOVERFLOW;++ switch (prop->type) {+ case DEV_PROP_U8:+ ret = dev_prop_copy_array_u8((u8 *)prop->num, (u8 *)val,+ nval);+ break;+ case DEV_PROP_U16:+ ret = dev_prop_copy_array_u16((u16 *)prop->num, (u16 *)val,+ nval);+ break;+ case DEV_PROP_U32:+ ret = dev_prop_copy_array_u32((u32 *)prop->num, (u32 *)val,+ nval);+ break;+ case DEV_PROP_U64:+ ret = dev_prop_copy_array_u64(prop->num, (u64 *)val, nval);+ break;+ case DEV_PROP_STRING:+ ret = dev_prop_copy_array_string(prop->str,+ (const char **)val, nval);+ break;+ default:+ ret = -EINVAL;+ break;+ }+ return ret;+}+ /** * device_property_present - check if a property of a device is present * @dev: Device whose property is being checked
That doesn't seem to go in the right direction to be honest.
Actually, having introduced struct fwnode_handle, we should perhaps try to
replace both of_node and acpi_node with a single struct fwnode_handle pointer
and then add a new fwnode_type for the "pdata" stuff.
If you don't want to deal with of_node, which I can understand easily, it
may be worth trying with acpi_node alone at this point and once you have
the fwnode_handle pointer, you might use it for both ACPI and "pdata"?
Grant, Arnd, I wonder what you think?
From: Grant Likely <hidden> Date: 2015-03-13 15:11:05
On Thu, Mar 12, 2015 at 1:41 AM, Rafael J. Wysocki [off-list ref] wrote:
On Monday, January 26, 2015 03:17:40 PM Heikki Krogerus wrote:
quoted
This extends the unified device property interface by adding
"Generic Property" to it for cases where device tree or ACPI
are not being used.
That makes the unified device property interface cover also
most of the cases where information is extracted from custom
platform_data in the drivers. So if before we had to check
separately is there custom platform_data for a driver:
if (pdata)
bar = pdata->bar;
else
device_property_read_u32(dev, "foo", &bar);
we can now drop that check and simply always use the unified
device property interface.
That makes it possible to drop a lot of boiler plate from
the drivers, plus quite a few header files under
include/linux/ describing those driver specific platform
data structures can be removed.
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
[...]
quoted
@@ -704,6 +705,7 @@ struct acpi_dev_node { * @archdata: For arch-specific additions. * @of_node: Associated device tree node. * @acpi_node: Associated ACPI device node.+ * @gen_prop: Generic device property * @devt: For creating the sysfs "dev". * @id: device instance * @devres_lock: Spinlock to protect the resource of the device.
That doesn't seem to go in the right direction to be honest.
Actually, having introduced struct fwnode_handle, we should perhaps try to
replace both of_node and acpi_node with a single struct fwnode_handle pointer
and then add a new fwnode_type for the "pdata" stuff.
I agree on this, but it will be a lot of work to convert...
If you don't want to deal with of_node, which I can understand easily, it
may be worth trying with acpi_node alone at this point and once you have
the fwnode_handle pointer, you might use it for both ACPI and "pdata"?
Grant, Arnd, I wonder what you think?
That makes sense, and we can populate the fwnode_handle even when
using DT. That will allow a transition period to move everyone over to
the fwnode_handle.
g.
On Friday 13 March 2015 15:10:38 Grant Likely wrote:
On Thu, Mar 12, 2015 at 1:41 AM, Rafael J. Wysocki [off-list ref] wrote:
quoted
On Monday, January 26, 2015 03:17:40 PM Heikki Krogerus wrote:
That doesn't seem to go in the right direction to be honest.
Actually, having introduced struct fwnode_handle, we should perhaps try to
replace both of_node and acpi_node with a single struct fwnode_handle pointer
and then add a new fwnode_type for the "pdata" stuff.
I agree on this, but it will be a lot of work to convert...
quoted
If you don't want to deal with of_node, which I can understand easily, it
may be worth trying with acpi_node alone at this point and once you have
the fwnode_handle pointer, you might use it for both ACPI and "pdata"?
Grant, Arnd, I wonder what you think?
That makes sense, and we can populate the fwnode_handle even when
using DT. That will allow a transition period to move everyone over to
the fwnode_handle.
From: Rafael J. Wysocki <hidden> Date: 2015-03-14 00:45:21
On Friday, March 13, 2015 04:24:11 PM Arnd Bergmann wrote:
On Friday 13 March 2015 15:10:38 Grant Likely wrote:
quoted
On Thu, Mar 12, 2015 at 1:41 AM, Rafael J. Wysocki [off-list ref] wrote:
quoted
On Monday, January 26, 2015 03:17:40 PM Heikki Krogerus wrote:
That doesn't seem to go in the right direction to be honest.
Actually, having introduced struct fwnode_handle, we should perhaps try to
replace both of_node and acpi_node with a single struct fwnode_handle pointer
and then add a new fwnode_type for the "pdata" stuff.
I agree on this, but it will be a lot of work to convert...
quoted
If you don't want to deal with of_node, which I can understand easily, it
may be worth trying with acpi_node alone at this point and once you have
the fwnode_handle pointer, you might use it for both ACPI and "pdata"?
Grant, Arnd, I wonder what you think?
That makes sense, and we can populate the fwnode_handle even when
using DT. That will allow a transition period to move everyone over to
the fwnode_handle.
Agreed on both as well.
OK
So below is what it takes to use struct fwnode_handle to represent ACPI
companions.
Rafael
---
From: Rafael J. Wysocki <redacted>
Subject: driver core / ACPI: Represent ACPI companions using fwnode_handle
Now that we have struct fwnode_handle, we can use that to point to
ACPI companions from struct device objects instead of pointing to
struct acpi_device directly.
There are two benefits from that. First, the somewhat ugly and
hackish struct acpi_dev_node can be dropped and, second, the same
struct fwnode_handle pointer can be used in the future to point
to other (non-ACPI) firmware device node types.
Signed-off-by: Rafael J. Wysocki <redacted>
---
drivers/acpi/acpi_platform.c | 2 +-
drivers/acpi/dock.c | 2 +-
drivers/base/platform.c | 2 +-
drivers/i2c/i2c-core.c | 4 ++--
include/acpi/acpi_bus.h | 3 ++-
include/linux/acpi.h | 7 ++++---
include/linux/device.h | 13 +++----------
include/linux/fwnode.h | 25 +++++++++++++++++++++++++
include/linux/i2c.h | 4 ++--
include/linux/platform_device.h | 2 +-
include/linux/property.h | 11 +----------
11 files changed, 43 insertions(+), 32 deletions(-)
Index: linux-pm/include/linux/device.h
===================================================================
On Sat, Mar 14, 2015 at 02:09:06AM +0100, Rafael J. Wysocki wrote:
On Friday, March 13, 2015 04:24:11 PM Arnd Bergmann wrote:
quoted
On Friday 13 March 2015 15:10:38 Grant Likely wrote:
quoted
On Thu, Mar 12, 2015 at 1:41 AM, Rafael J. Wysocki [off-list ref] wrote:
quoted
On Monday, January 26, 2015 03:17:40 PM Heikki Krogerus wrote:
That doesn't seem to go in the right direction to be honest.
Actually, having introduced struct fwnode_handle, we should perhaps try to
replace both of_node and acpi_node with a single struct fwnode_handle pointer
and then add a new fwnode_type for the "pdata" stuff.
I agree on this, but it will be a lot of work to convert...
quoted
If you don't want to deal with of_node, which I can understand easily, it
may be worth trying with acpi_node alone at this point and once you have
the fwnode_handle pointer, you might use it for both ACPI and "pdata"?
Grant, Arnd, I wonder what you think?
That makes sense, and we can populate the fwnode_handle even when
using DT. That will allow a transition period to move everyone over to
the fwnode_handle.
Agreed on both as well.
OK
So below is what it takes to use struct fwnode_handle to represent ACPI
companions.
Rafael
---
From: Rafael J. Wysocki <redacted>
Subject: driver core / ACPI: Represent ACPI companions using fwnode_handle
Now that we have struct fwnode_handle, we can use that to point to
ACPI companions from struct device objects instead of pointing to
struct acpi_device directly.
There are two benefits from that. First, the somewhat ugly and
hackish struct acpi_dev_node can be dropped and, second, the same
struct fwnode_handle pointer can be used in the future to point
to other (non-ACPI) firmware device node types.
Signed-off-by: Rafael J. Wysocki <redacted>
---
drivers/acpi/acpi_platform.c | 2 +-
drivers/acpi/dock.c | 2 +-
drivers/base/platform.c | 2 +-
drivers/i2c/i2c-core.c | 4 ++--
include/acpi/acpi_bus.h | 3 ++-
include/linux/acpi.h | 7 ++++---
include/linux/device.h | 13 +++----------
include/linux/fwnode.h | 25 +++++++++++++++++++++++++
include/linux/i2c.h | 4 ++--
include/linux/platform_device.h | 2 +-
include/linux/property.h | 11 +----------
11 files changed, 43 insertions(+), 32 deletions(-)
Nice, I like the driver core changes:
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
That doesn't seem to go in the right direction to be honest.
Actually, having introduced struct fwnode_handle, we should perhaps try to
replace both of_node and acpi_node with a single struct fwnode_handle pointer
and then add a new fwnode_type for the "pdata" stuff.
If you don't want to deal with of_node, which I can understand easily, it
may be worth trying with acpi_node alone at this point and once you have
the fwnode_handle pointer, you might use it for both ACPI and "pdata"?
It sounds like a better Idea to me too.
Thanks,
--
heikki
From: Grant Likely <hidden> Date: 2015-03-16 09:47:43
On Sat, Mar 14, 2015 at 9:42 AM, Greg Kroah-Hartman
[off-list ref] wrote:
On Sat, Mar 14, 2015 at 02:09:06AM +0100, Rafael J. Wysocki wrote:
quoted
On Friday, March 13, 2015 04:24:11 PM Arnd Bergmann wrote:
quoted
On Friday 13 March 2015 15:10:38 Grant Likely wrote:
quoted
On Thu, Mar 12, 2015 at 1:41 AM, Rafael J. Wysocki [off-list ref] wrote:
quoted
On Monday, January 26, 2015 03:17:40 PM Heikki Krogerus wrote:
That doesn't seem to go in the right direction to be honest.
Actually, having introduced struct fwnode_handle, we should perhaps try to
replace both of_node and acpi_node with a single struct fwnode_handle pointer
and then add a new fwnode_type for the "pdata" stuff.
I agree on this, but it will be a lot of work to convert...
quoted
If you don't want to deal with of_node, which I can understand easily, it
may be worth trying with acpi_node alone at this point and once you have
the fwnode_handle pointer, you might use it for both ACPI and "pdata"?
Grant, Arnd, I wonder what you think?
That makes sense, and we can populate the fwnode_handle even when
using DT. That will allow a transition period to move everyone over to
the fwnode_handle.
Agreed on both as well.
OK
So below is what it takes to use struct fwnode_handle to represent ACPI
companions.
Rafael
---
From: Rafael J. Wysocki <redacted>
Subject: driver core / ACPI: Represent ACPI companions using fwnode_handle
Now that we have struct fwnode_handle, we can use that to point to
ACPI companions from struct device objects instead of pointing to
struct acpi_device directly.
There are two benefits from that. First, the somewhat ugly and
hackish struct acpi_dev_node can be dropped and, second, the same
struct fwnode_handle pointer can be used in the future to point
to other (non-ACPI) firmware device node types.
Signed-off-by: Rafael J. Wysocki <redacted>
---
drivers/acpi/acpi_platform.c | 2 +-
drivers/acpi/dock.c | 2 +-
drivers/base/platform.c | 2 +-
drivers/i2c/i2c-core.c | 4 ++--
include/acpi/acpi_bus.h | 3 ++-
include/linux/acpi.h | 7 ++++---
include/linux/device.h | 13 +++----------
include/linux/fwnode.h | 25 +++++++++++++++++++++++++
include/linux/i2c.h | 4 ++--
include/linux/platform_device.h | 2 +-
include/linux/property.h | 11 +----------
11 files changed, 43 insertions(+), 32 deletions(-)
Nice, I like the driver core changes:
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
I like it too. The only comment I have is I would use a static inline
helper for getting/setting the fwnode_handle pointer. The reason being
that there are situations where we're going to want to attach both a
DT node and static data to the same node. Using a helper isolates the
rest of the kernel from any future changes here.
However, despite that comment:
Acked-by: Grant Likely <redacted>
My comment isn't significant enough to hold up this change.
From: Rafael J. Wysocki <hidden> Date: 2015-03-16 22:36:01
On Monday, March 16, 2015 09:47:22 AM Grant Likely wrote:
On Sat, Mar 14, 2015 at 9:42 AM, Greg Kroah-Hartman
[off-list ref] wrote:
quoted
On Sat, Mar 14, 2015 at 02:09:06AM +0100, Rafael J. Wysocki wrote:
quoted
On Friday, March 13, 2015 04:24:11 PM Arnd Bergmann wrote:
quoted
On Friday 13 March 2015 15:10:38 Grant Likely wrote:
quoted
On Thu, Mar 12, 2015 at 1:41 AM, Rafael J. Wysocki [off-list ref] wrote:
quoted
On Monday, January 26, 2015 03:17:40 PM Heikki Krogerus wrote:
That doesn't seem to go in the right direction to be honest.
Actually, having introduced struct fwnode_handle, we should perhaps try to
replace both of_node and acpi_node with a single struct fwnode_handle pointer
and then add a new fwnode_type for the "pdata" stuff.
I agree on this, but it will be a lot of work to convert...
quoted
If you don't want to deal with of_node, which I can understand easily, it
may be worth trying with acpi_node alone at this point and once you have
the fwnode_handle pointer, you might use it for both ACPI and "pdata"?
Grant, Arnd, I wonder what you think?
That makes sense, and we can populate the fwnode_handle even when
using DT. That will allow a transition period to move everyone over to
the fwnode_handle.
Agreed on both as well.
OK
So below is what it takes to use struct fwnode_handle to represent ACPI
companions.
Rafael
---
From: Rafael J. Wysocki <redacted>
Subject: driver core / ACPI: Represent ACPI companions using fwnode_handle
Now that we have struct fwnode_handle, we can use that to point to
ACPI companions from struct device objects instead of pointing to
struct acpi_device directly.
There are two benefits from that. First, the somewhat ugly and
hackish struct acpi_dev_node can be dropped and, second, the same
struct fwnode_handle pointer can be used in the future to point
to other (non-ACPI) firmware device node types.
Signed-off-by: Rafael J. Wysocki <redacted>
---
drivers/acpi/acpi_platform.c | 2 +-
drivers/acpi/dock.c | 2 +-
drivers/base/platform.c | 2 +-
drivers/i2c/i2c-core.c | 4 ++--
include/acpi/acpi_bus.h | 3 ++-
include/linux/acpi.h | 7 ++++---
include/linux/device.h | 13 +++----------
include/linux/fwnode.h | 25 +++++++++++++++++++++++++
include/linux/i2c.h | 4 ++--
include/linux/platform_device.h | 2 +-
include/linux/property.h | 11 +----------
11 files changed, 43 insertions(+), 32 deletions(-)
Nice, I like the driver core changes:
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
I like it too. The only comment I have is I would use a static inline
helper for getting/setting the fwnode_handle pointer.
Yes, that's the next step. :-)
The reason being
that there are situations where we're going to want to attach both a
DT node and static data to the same node. Using a helper isolates the
rest of the kernel from any future changes here.
Right.
I'm going to send a couple more patches on top of this one going in that
direction.
However, despite that comment:
Acked-by: Grant Likely <redacted>
My comment isn't significant enough to hold up this change.
OK then, I'm queuing this up for 4.1. Thanks!
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
From: Rafael J. Wysocki <hidden> Date: 2015-03-17 13:54:01
From: Rafael J. Wysocki <redacted>
Now that the ACPI companions of devices are represented by pointers
to struct fwnode_handle, it is not quite efficient to check whether
or not an ACPI companion of a device is present by evaluating the
ACPI_COMPANION() macro.
For this reason, introduce a special static inline routine for that,
has_acpi_companion(), and update the code to use it where applicable.
Signed-off-by: Rafael J. Wysocki <redacted>
---
On top of https://patchwork.kernel.org/patch/6010351/
---
drivers/acpi/glue.c | 4 ++--
drivers/i2c/busses/i2c-designware-platdrv.c | 4 ++--
drivers/iommu/intel-iommu.c | 2 +-
include/linux/acpi.h | 10 ++++++++++
4 files changed, 15 insertions(+), 5 deletions(-)
Index: linux-pm/include/linux/acpi.h
===================================================================
@@ -166,7 +166,7 @@ static int dw_i2c_probe(struct platform_/* fast mode by default because of legacy reasons */clk_freq=400000;-if(ACPI_COMPANION(&pdev->dev)){+if(has_acpi_companion(&pdev->dev)){dw_i2c_acpi_configure(pdev);}elseif(pdev->dev.of_node){of_property_read_u32(pdev->dev.of_node,
@@ -286,7 +286,7 @@ static int dw_i2c_remove(struct platformpm_runtime_put(&pdev->dev);pm_runtime_disable(&pdev->dev);-if(ACPI_COMPANION(&pdev->dev))+if(has_acpi_companion(&pdev->dev))dw_i2c_acpi_unconfigure(pdev);return0;
From: Rafael J. Wysocki <hidden> Date: 2015-03-22 22:46:34
From: Rafael J. Wysocki <redacted>
Now that the ACPI companions of devices are pointed to by the fwnode
field in struct device, the device_property_*() accessor functions
can be modified to use their fwnode_property_*() counterparts
internally with minimum extra overhead in the IS_ENABLED(CONFIG_OF)
case, so make those changes.
This allows us to get rid of the rather ugly DEV_PROP_READ_ARRAY()
macro among other things.
Signed-off-by: Rafael J. Wysocki <redacted>
---
On top of
http://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=ce793486e23e0162a732c605189c8028e0910e86
---
drivers/base/property.c | 44 +++++++++++++++++---------------------------
1 file changed, 17 insertions(+), 27 deletions(-)
Index: linux-pm/drivers/base/property.c
===================================================================
On Mon, Mar 23, 2015 at 12:10:30AM +0100, Rafael J. Wysocki wrote:
From: Rafael J. Wysocki <redacted>
Now that the ACPI companions of devices are pointed to by the fwnode
field in struct device, the device_property_*() accessor functions
can be modified to use their fwnode_property_*() counterparts
internally with minimum extra overhead in the IS_ENABLED(CONFIG_OF)
case, so make those changes.
This allows us to get rid of the rather ugly DEV_PROP_READ_ARRAY()
macro among other things.
Signed-off-by: Rafael J. Wysocki <redacted>
---
On top of
http://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/commit/?h=linux-next&id=ce793486e23e0162a732c605189c8028e0910e86
From: Rafael J. Wysocki <hidden> Date: 2015-03-28 01:03:07
From: Rafael J. Wysocki <redacted>
Introduce data structures and code allowing "built-in" properties
to be associated with devices in such a way that they will be used
by the device_property_* API if no proper firmware node (neither DT
nor ACPI) is present for the given device.
Each property is to be represented by a property_entry structure.
An array of property_entry structures (terminated with a null
entry) can be pointed to by the properties field of struct
property_set that can be added as a firmware node to a struct
device using device_add_property_set(). That will cause the
device_property_* API to use that property_set as the source
of properties if the given device does not have a DT node or
an ACPI companion device object associated with it.
Signed-off-by: Rafael J. Wysocki <redacted>
---
drivers/base/property.c | 98 ++++++++++++++++++++++++++++++++++++++++++++---
include/linux/fwnode.h | 1
include/linux/property.h | 33 +++++++++++++++
3 files changed, 127 insertions(+), 5 deletions(-)
Index: linux-pm/include/linux/fwnode.h
===================================================================
From: Rafael J. Wysocki <hidden> Date: 2015-03-28 01:03:07
From: Rafael J. Wysocki <redacted>
Modify the fwnode_property_* accessors so as to make them fall back
to using "built-in" device properties from a property_set structure
associated with the given device (as the secondary firmware node)
if the property in question cannot be retrieved from the primary
firmware node (DT or ACPI). That will make the device_properties_*
functions do the same thing as they use fwnode_property_* internally.
That should make it easier to provide default values for device
properties used by device drivers to be used in case the properties
in question are not provided by the platform firmare at all or
they cannot be retireved from the platform firmware due to errors.
Signed-off-by: Rafael J. Wysocki <redacted>
---
First of all, this should show why I thought it would be better to use
the primary/secondary scheme for firmware nodes instead of just having a
list of them with the head pointed to by the struct device's fwnode field.
Namely, within the primary/secondary scheme the ordering of property lookup
is always well defined and there's no overhead when, for example, somebody
specifically wants to access the ACPI companion firmware node etc.
Second, as I said in the intro message, this is not for immediate application
(apart from any review comments on [1-2/3]), because it would introduce an
artificial difference between DT and ACPI that I'd prefer to avoid.
Namely, dev_fwnode() (introduced by one of the patches in linux-next) returns
the address of the dev->of_node's fwnode field if dev->of_node is present and
that would not have the secondary pointer set. On the other hand, since ACPI
is now using set_primary_fwnode() to add its firmware node companions to
devices, the secondary pointer in its firmware node handle will be set if the
"built-in" properties are present. So, effectively, with this patch applied
ACPI would always fall back to the "built-in" properties (if present), while
DT would not do that (unless the DT node is missing entirely).
That difference could be worked around, though, if dev->fwnode is always set
whenever dev->of_node is set in essentially this way:
dev->of_node = dn;
set_primary_fwnode(dev, &dn->fwnode);
(that, of course, can be defined as a macro or static inline). Now, I'm not
sure about that, but my somewhat educated guess is that while dev->of_node is
read in many many places, the number of places in which it is set is actually
much smaller and making this change in one go should be a workable thing.
Please let me know what you think.
---
drivers/base/property.c | 61 ++++++++++++++++++++++++------------------------
1 file changed, 31 insertions(+), 30 deletions(-)
Index: linux-pm/drivers/base/property.c
===================================================================
From: Rafael J. Wysocki <hidden> Date: 2015-03-28 01:03:09
Hi,
Patches [1-2/3] add support for properties provided in a "pdata way"
roughly along the lines of the Heikki's patch at
https://patchwork.kernel.org/patch/5709461/
but using the fwnode field in struct device introduced by one of my
previous patches (currently in linux-next).
Patch [3/3] does one more interesting thing on top of that, but is mostly
for discussion. Namely, it changes the behavior of the unified device
properties API to fall back to the "built-in" properties when it cannot
find the requested data in the stuff provided by the platform firmware.
Currently, that patch would introduce an artificial difference between ACPI
and DT, because DT uses the of_node pointer in struct device won't use the
set_primary_fwnode() thing introduced buy the [1/3]. However, I have an
idea about how that can be worked around, but more on that in the changelog
of patch [3/3].
Kind regards,
Rafael
From: Rafael J. Wysocki <hidden> Date: 2015-03-28 01:03:09
From: Rafael J. Wysocki <redacted>
Add a secondary pointer to struct fwnode_handle so as to make it
possible for a device to have two firmware nodes associated with
it at the same time, for example, an ACPI node and a node with
a set of properties provided by platform initialization code.
In the future that will allow device property lookup to fall back
from the primary firmware node to the secondary one if the given
property is not present there to make it easier to provide defaults
for device properties used by device drivers.
Introduce two helper routines, set_primary_fwnode() and
set_secondary_fwnode() allowing callers to add a primary/secondary
firmware node to the given device in such a way that
(1) If there's only one firmware node for that device, it will be
pointed to by the device's firmware node pointer.
(2) If both the primary and secondary firmware nodes are present,
the primary one will be pointed to by the device's firmware
node pointer, while the secondary one will be pointed to by the
primary node's secondary pointer.
(3) If one of these nodes is removed (by calling one of the new
nelpers with NULL as the second argument), the other one will
be preserved.
Make ACPI use set_primary_fwnode() for attaching its firmware nodes
to devices.
Signed-off-by: Rafael J. Wysocki <redacted>
---
drivers/base/core.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/acpi.h | 4 +--
include/linux/device.h | 3 ++
include/linux/fwnode.h | 1
4 files changed, 57 insertions(+), 2 deletions(-)
Index: linux-pm/include/linux/fwnode.h
===================================================================
From: Heikki Krogerus <heikki.krogerus@linux.intel.com> Date: 2015-04-02 14:35:10
Hi,
On Sat, Mar 28, 2015 at 02:26:35AM +0100, Rafael J. Wysocki wrote:
From: Rafael J. Wysocki <redacted>
Modify the fwnode_property_* accessors so as to make them fall back
to using "built-in" device properties from a property_set structure
associated with the given device (as the secondary firmware node)
if the property in question cannot be retrieved from the primary
firmware node (DT or ACPI). That will make the device_properties_*
functions do the same thing as they use fwnode_property_* internally.
That should make it easier to provide default values for device
properties used by device drivers to be used in case the properties
in question are not provided by the platform firmare at all or
they cannot be retireved from the platform firmware due to errors.
Signed-off-by: Rafael J. Wysocki <redacted>
FWIW for the whole series:
Tested-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
I'm attaching the little patch I used to test this with Synopsys
Designware USB Device Controller. Everything worked nicely.
First of all, this should show why I thought it would be better to use
the primary/secondary scheme for firmware nodes instead of just having a
list of them with the head pointed to by the struct device's fwnode field.
Namely, within the primary/secondary scheme the ordering of property lookup
is always well defined and there's no overhead when, for example, somebody
specifically wants to access the ACPI companion firmware node etc.
Second, as I said in the intro message, this is not for immediate application
(apart from any review comments on [1-2/3]), because it would introduce an
artificial difference between DT and ACPI that I'd prefer to avoid.
Namely, dev_fwnode() (introduced by one of the patches in linux-next) returns
the address of the dev->of_node's fwnode field if dev->of_node is present and
that would not have the secondary pointer set. On the other hand, since ACPI
is now using set_primary_fwnode() to add its firmware node companions to
devices, the secondary pointer in its firmware node handle will be set if the
"built-in" properties are present. So, effectively, with this patch applied
ACPI would always fall back to the "built-in" properties (if present), while
DT would not do that (unless the DT node is missing entirely).
That difference could be worked around, though, if dev->fwnode is always set
whenever dev->of_node is set in essentially this way:
dev->of_node = dn;
set_primary_fwnode(dev, &dn->fwnode);
(that, of course, can be defined as a macro or static inline). Now, I'm not
sure about that, but my somewhat educated guess is that while dev->of_node is
read in many many places, the number of places in which it is set is actually
much smaller and making this change in one go should be a workable thing.
Please let me know what you think.
To me falling back to the "built-in" device properties is the most
interesting thing. I have already a set of mfd and probing drivers,
such as dwc3-pci.c in my example, in my mind, where I have the ACPI
companion but could still really use the build-in properties.
Thanks,
--
heikki
From: Rafael J. Wysocki <hidden> Date: 2015-04-03 13:35:23
On Thursday, April 02, 2015 05:35:06 PM Heikki Krogerus wrote:
--AqsLC8rIMeq19msA
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Hi,
On Sat, Mar 28, 2015 at 02:26:35AM +0100, Rafael J. Wysocki wrote:
quoted
From: Rafael J. Wysocki <redacted>
Modify the fwnode_property_* accessors so as to make them fall back
to using "built-in" device properties from a property_set structure
associated with the given device (as the secondary firmware node)
if the property in question cannot be retrieved from the primary
firmware node (DT or ACPI). That will make the device_properties_*
functions do the same thing as they use fwnode_property_* internally.
That should make it easier to provide default values for device
properties used by device drivers to be used in case the properties
in question are not provided by the platform firmare at all or
they cannot be retireved from the platform firmware due to errors.
Signed-off-by: Rafael J. Wysocki <redacted>
FWIW for the whole series:
Tested-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
I'm attaching the little patch I used to test this with Synopsys
Designware USB Device Controller. Everything worked nicely.
Thanks a lot for testing this!
quoted
First of all, this should show why I thought it would be better to use
the primary/secondary scheme for firmware nodes instead of just having a
list of them with the head pointed to by the struct device's fwnode field.
Namely, within the primary/secondary scheme the ordering of property lookup
is always well defined and there's no overhead when, for example, somebody
specifically wants to access the ACPI companion firmware node etc.
Second, as I said in the intro message, this is not for immediate application
(apart from any review comments on [1-2/3]), because it would introduce an
artificial difference between DT and ACPI that I'd prefer to avoid.
Namely, dev_fwnode() (introduced by one of the patches in linux-next) returns
the address of the dev->of_node's fwnode field if dev->of_node is present and
that would not have the secondary pointer set. On the other hand, since ACPI
is now using set_primary_fwnode() to add its firmware node companions to
devices, the secondary pointer in its firmware node handle will be set if the
"built-in" properties are present. So, effectively, with this patch applied
ACPI would always fall back to the "built-in" properties (if present), while
DT would not do that (unless the DT node is missing entirely).
That difference could be worked around, though, if dev->fwnode is always set
whenever dev->of_node is set in essentially this way:
dev->of_node = dn;
set_primary_fwnode(dev, &dn->fwnode);
(that, of course, can be defined as a macro or static inline). Now, I'm not
sure about that, but my somewhat educated guess is that while dev->of_node is
read in many many places, the number of places in which it is set is actually
much smaller and making this change in one go should be a workable thing.
Please let me know what you think.
To me falling back to the "built-in" device properties is the most
interesting thing. I have already a set of mfd and probing drivers,
such as dwc3-pci.c in my example, in my mind, where I have the ACPI
companion but could still really use the build-in properties.
Yes, this is the most interesting part to me too, but as I said, I wouldn't like
to introduce functional differences between DT and ACPI due to this and we need
response from the DT party to make changes along the lines of patch [3/3].
For now, let me resend patches [1-2/3] with your Tested-by: tag. :-)
Rafael
From: Rafael J. Wysocki <hidden> Date: 2015-04-03 13:41:18
From: Rafael J. Wysocki <redacted>
Add a "secondary" pointer to struct fwnode_handle so as to make it
possible for a device to have two firmware nodes associated with
it at the same time, for example, an ACPI node and a node with
a set of properties provided by platform initialization code.
In the future that will allow device property lookup to fall back
from the primary firmware node to the secondary one if the given
property is not present there to make it easier to provide defaults
for device properties used by device drivers.
Introduce two helper routines, set_primary_fwnode() and
set_secondary_fwnode() allowing callers to add a primary/secondary
firmware node to the given device in such a way that
(1) If there's only one firmware node for that device, it will be
pointed to by the device's firmware node pointer.
(2) If both the primary and secondary firmware nodes are present,
the primary one will be pointed to by the device's firmware
node pointer, while the secondary one will be pointed to by the
primary node's secondary pointer.
(3) If one of these nodes is removed (by calling one of the new
nelpers with NULL as the second argument), the other one will
be preserved.
Make ACPI use set_primary_fwnode() for attaching its firmware nodes
to devices.
Signed-off-by: Rafael J. Wysocki <redacted>
Tested-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
Greg, any objections here?
---
drivers/base/core.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/acpi.h | 4 +--
include/linux/device.h | 3 ++
include/linux/fwnode.h | 1
4 files changed, 57 insertions(+), 2 deletions(-)
Index: linux-pm/include/linux/fwnode.h
===================================================================
From: Rafael J. Wysocki <hidden> Date: 2015-04-03 13:41:18
From: Rafael J. Wysocki <redacted>
Introduce data structures and code allowing "built-in" properties
to be associated with devices in such a way that they will be used
by the device_property_* API if no proper firmware node (neither DT
nor ACPI) is present for the given device.
Each property is to be represented by a property_entry structure.
An array of property_entry structures (terminated with a null
entry) can be pointed to by the properties field of struct
property_set that can be added as a firmware node to a struct
device using device_add_property_set(). That will cause the
device_property_* API to use that property_set as the source
of properties if the given device does not have a DT node or
an ACPI companion device object associated with it.
Signed-off-by: Rafael J. Wysocki <redacted>
Tested-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
Greg, any objections?
---
drivers/base/property.c | 98 ++++++++++++++++++++++++++++++++++++++++++++---
include/linux/fwnode.h | 1
include/linux/property.h | 33 +++++++++++++++
3 files changed, 127 insertions(+), 5 deletions(-)
Index: linux-pm/include/linux/fwnode.h
===================================================================
From: Rafael J. Wysocki <hidden> Date: 2015-04-03 13:41:19
On Saturday, March 28, 2015 02:05:19 AM Rafael J. Wysocki wrote:
Hi,
Patches [1-2/3] add support for properties provided in a "pdata way"
roughly along the lines of the Heikki's patch at
https://patchwork.kernel.org/patch/5709461/
but using the fwnode field in struct device introduced by one of my
previous patches (currently in linux-next).
Patch [3/3] does one more interesting thing on top of that, but is mostly
for discussion. Namely, it changes the behavior of the unified device
properties API to fall back to the "built-in" properties when it cannot
find the requested data in the stuff provided by the platform firmware.
Currently, that patch would introduce an artificial difference between ACPI
and DT, because DT uses the of_node pointer in struct device won't use the
set_primary_fwnode() thing introduced buy the [1/3]. However, I have an
idea about how that can be worked around, but more on that in the changelog
of patch [3/3].
So to make some progress here I'd like to apply patches [1-2/3] (which I'm going
to resend shortly) if there are no objections.
Thanks,
Rafael
On Fri, Apr 03, 2015 at 04:03:27PM +0200, Rafael J. Wysocki wrote:
From: Rafael J. Wysocki <redacted>
Add a "secondary" pointer to struct fwnode_handle so as to make it
possible for a device to have two firmware nodes associated with
it at the same time, for example, an ACPI node and a node with
a set of properties provided by platform initialization code.
In the future that will allow device property lookup to fall back
from the primary firmware node to the secondary one if the given
property is not present there to make it easier to provide defaults
for device properties used by device drivers.
Introduce two helper routines, set_primary_fwnode() and
set_secondary_fwnode() allowing callers to add a primary/secondary
firmware node to the given device in such a way that
(1) If there's only one firmware node for that device, it will be
pointed to by the device's firmware node pointer.
(2) If both the primary and secondary firmware nodes are present,
the primary one will be pointed to by the device's firmware
node pointer, while the secondary one will be pointed to by the
primary node's secondary pointer.
(3) If one of these nodes is removed (by calling one of the new
nelpers with NULL as the second argument), the other one will
be preserved.
Make ACPI use set_primary_fwnode() for attaching its firmware nodes
to devices.
Signed-off-by: Rafael J. Wysocki <redacted>
Tested-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
Greg, any objections here?
On Fri, Apr 03, 2015 at 04:05:11PM +0200, Rafael J. Wysocki wrote:
From: Rafael J. Wysocki <redacted>
Introduce data structures and code allowing "built-in" properties
to be associated with devices in such a way that they will be used
by the device_property_* API if no proper firmware node (neither DT
nor ACPI) is present for the given device.
Each property is to be represented by a property_entry structure.
An array of property_entry structures (terminated with a null
entry) can be pointed to by the properties field of struct
property_set that can be added as a firmware node to a struct
device using device_add_property_set(). That will cause the
device_property_* API to use that property_set as the source
of properties if the given device does not have a DT node or
an ACPI companion device object associated with it.
Signed-off-by: Rafael J. Wysocki <redacted>
Tested-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
---
Greg, any objections?