Re: [PATCH v3 1/2] of: Rename poweroff-source property to system-power-controller
On Thu, 6 Nov 2014 14:23:02 +
, Romain Perier romain.per...@gmail.com
wrote:
It reverts commit a4b4e0461ec5 (of: Add standard property for poweroff
capability).
As discussed on the mailing list, it makes more sense to rename back to the
old established property name, without the vendor prefix. Problem being that
the word source usually tends to be used for inputs and that is out of
control
of the OS. The poweroff capability is an output which simply turns the
system-power off. Also, this property might be used by drivers which power-off
the system and power back on subsequent RTC alarms. This seems to suggest to
remove poweroff from the property name and to choose
system-power-controller
as the more generic name. This patchs adds the required renaming changes and
defines an helper function which is compatible with both properties, the old
one
which was only used by tps65910 and the new one without vendor-prefix.
Signed-off-by: Romain Perier romain.per...@gmail.com
---
.../bindings/power/{poweroff.txt = power-controller.txt} | 0
Documentation/devicetree/bindings/regulator/act8865-regulator.txt | 4 ++--
drivers/mfd/tps65910.c| 2 +-
drivers/regulator/act8865-regulator.c | 2 +-
include/linux/of.h| 8
+---
5 files changed, 9 insertions(+), 7 deletions(-)
rename Documentation/devicetree/bindings/power/{poweroff.txt =
power-controller.txt} (100%)
diff --git a/Documentation/devicetree/bindings/power/poweroff.txt
b/Documentation/devicetree/bindings/power/power-controller.txt
similarity index 100%
rename from Documentation/devicetree/bindings/power/poweroff.txt
rename to Documentation/devicetree/bindings/power/power-controller.txt
diff --git
a/Documentation/devicetree/bindings/regulator/act8865-regulator.txt
b/Documentation/devicetree/bindings/regulator/act8865-regulator.txt
index 01a5b07..dad6358 100644
--- a/Documentation/devicetree/bindings/regulator/act8865-regulator.txt
+++ b/Documentation/devicetree/bindings/regulator/act8865-regulator.txt
@@ -6,8 +6,8 @@ Required properties:
- reg: I2C slave address
Optional properties:
-- poweroff-source: Telling whether or not this pmic is controlling
- the system power. See Documentation/devicetree/bindings/power/poweroff.txt
.
+- system-power-controller: Telling whether or not this pmic is controlling
+ the system power. See
Documentation/devicetree/bindings/power/power-controller.txt .
Any standard regulator properties can be used to configure the single
regulator.
diff --git a/drivers/mfd/tps65910.c b/drivers/mfd/tps65910.c
index b8dca8a..77a7f78 100644
--- a/drivers/mfd/tps65910.c
+++ b/drivers/mfd/tps65910.c
@@ -423,7 +423,7 @@ static struct tps65910_board *tps65910_parse_dt(struct
i2c_client *client,
board_info-irq = client-irq;
board_info-irq_base = -1;
- board_info-pm_off = of_system_has_poweroff_source(np);
+ board_info-pm_off = of_is_system_power_controller(np);
return board_info;
}
diff --git a/drivers/regulator/act8865-regulator.c
b/drivers/regulator/act8865-regulator.c
index 76301ed..435aba1 100644
--- a/drivers/regulator/act8865-regulator.c
+++ b/drivers/regulator/act8865-regulator.c
@@ -365,7 +365,7 @@ static int act8865_pmic_probe(struct i2c_client *client,
return ret;
}
- if (of_system_has_poweroff_source(dev-of_node)) {
+ if (of_is_system_power_controller(dev-of_node)) {
if (!pm_power_off) {
act8865_i2c_client = client;
act8865-off_reg = off_reg;
diff --git a/include/linux/of.h b/include/linux/of.h
index 868fdad..f3595ba 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -911,14 +911,16 @@ static inline int of_changeset_update_property(struct
of_changeset *ocs,
extern int of_resolve_phandles(struct device_node *tree);
/**
- * of_system_has_poweroff_source - Tells if poweroff-source is found for
device_node
+ * of_is_system_power_controller - Tells if system-power-controller is found
for device_node
* @np: Pointer to the given device_node
*
* return true if present false otherwise
*/
-static inline bool of_system_has_poweroff_source(const struct device_node
*np)
+static inline bool of_is_system_power_controller(const struct device_node
*np)
{
- return of_property_read_bool(np, poweroff-source);
+ if (of_property_read_bool(np, system-power-controller))
+ return true;
+ return of_property_read_bool(np, ti,system-power-controller);
So, to be a little pendantic, the 'ti,' prefix is basically just for
backwards compatibility with the old binding, and that old binding is
only used on palmas.c, tps65*.c and twl4030-power.c, correct?
We don't actually want a ti comparison in the generic code. Normally
what you'd do here
[GIT PULL] Devicetree bug fix for v3.18
[resending; forgot to cc the mailing lists] Hi Linus. Please pull the following branch for a device tree bug fix. Thanks, g. The following changes since commit 0df1f2487d2f0d04703f142813d53615d62a1da4: Linux 3.18-rc3 (2014-11-02 15:01:51 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/glikely/linux devicetree/merge for you to fetch changes up to a87fa1d81a9fb5e9adca9820e16008c40ad09f33: of: Fix overflow bug in string property parsing functions (2014-11-04 10:19:48 +) Devicetree bugfix for v3.18 One buffer overflow bug that shouldn't be left around. Grant Likely (1): of: Fix overflow bug in string property parsing functions drivers/of/base.c | 88 - drivers/of/selftest.c | 66 -- drivers/of/testcase-data/tests-phandle.dtsi | 2 + include/linux/of.h | 84 ++- 4 files changed, 154 insertions(+), 86 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[GIT PULL] Devicetree bug fix for v3.18
[resending; forgot to cc the mailing lists] Hi Linus. Please pull the following branch for a device tree bug fix. Thanks, g. The following changes since commit 0df1f2487d2f0d04703f142813d53615d62a1da4: Linux 3.18-rc3 (2014-11-02 15:01:51 -0800) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/glikely/linux devicetree/merge for you to fetch changes up to a87fa1d81a9fb5e9adca9820e16008c40ad09f33: of: Fix overflow bug in string property parsing functions (2014-11-04 10:19:48 +) Devicetree bugfix for v3.18 One buffer overflow bug that shouldn't be left around. Grant Likely (1): of: Fix overflow bug in string property parsing functions drivers/of/base.c | 88 - drivers/of/selftest.c | 66 -- drivers/of/testcase-data/tests-phandle.dtsi | 2 + include/linux/of.h | 84 ++- 4 files changed, 154 insertions(+), 86 deletions(-) -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 0/2] pinctrl: Intel Cherryview/Braswell support
On Wed, Nov 5, 2014 at 10:15 PM, Rafael J. Wysocki wrote: > On Wednesday, November 05, 2014 09:46:14 PM Grant Likely wrote: >> On Tue, Nov 4, 2014 at 5:54 PM, Timur Tabi wrote: >> > On 11/04/2014 02:20 AM, Mika Westerberg wrote: >> >> >> >> There is nothing like that yet in ACPI world but with the ACPI _DSD >> >> patches we are getting properties similar to DT which means that we can >> >> provide pinctrl bindings from ACPI systems as well. >> > >> > >> > Do you know if anyone has signed up to actually do the work of defining how >> > pinctrl will look in ACPI? I'm guessing the simplest approach would be to: >> >> Actually, for the most part we /don't/ want to try and put pinctrl >> into an ACPI binding. The assumption is that on an ACPI platform it >> would be the combination of ACPI and firmware responsible for the pin >> mappings, not the operating system. >> >> Before even attempting to put pinctrl mappings into your driver, there >> should instead be a conversation at the ACPI spec level about whether >> or not it makes sense and what such a binding should look like. > > We may need to support pinctrl on ACPI platforms in the meantime, though, > for reasons that Mika mentioned at one point (there are systems where the > firmware mangles things etc., which quite frankly is to be expected). > > So while I'm all for discussing this in the ASWG, what's the interim approach > we should be using, in your opinion? Context would be useful at this point. I know (well, I /strongly/ suspect) that Timur is working on ARM server support, where as most of the work you, Darren and Mika are doing is focused on getting into embedded. The reason I'm pushing for a pinctrl discussion at the ASWG level is because it affects OS portability. The OS should not not be required to have a driver for the pinctrl hardware in order to boot into a usable state (at least far enough to obtain further drivers). The same goes for clock controllers and power regulators. In x86 world this is the default assumption, but in the ARM world it needs to be re-enforced over and over to break out of the embedded mindset that a lot of us have. So, to answer your question, what's the interim approach? I think it is two things: 1) The base assumption must be that firmware sets up the pinctrl hardware into a usable state at boot and ACPI is used to adjust it as part of the normal OSPM runtime PM operations on devices. 2) Where direct control over the pinctrl hardware is required by the OS, build it into the GPIO driver functionality (which from what I understand is exactly what Mika has done). If any data for the pinctrl is required from ACPI, it can be driver specific data, but any attempt to create a generic binding that the OS is expected to understand should be avoided. This is so we don't end up with the OS needing to understand more than is in the ACPI spec in order to boot. For a more generic solution (building into OSPM the understanding of pin groups and how to control them when power managing devices), there needs to be work at the ASWG level to figure out what it is going to look like. g. > > -- > I speak only for myself. > Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/5] of: Only call notifiers when node is attached
On Thu, Nov 6, 2014 at 12:33 PM, Pantelis Antoniou
wrote:
> Hi Grant,
>
>> On Nov 5, 2014, at 23:39 , Grant Likely wrote:
>>
>> On Tue, 28 Oct 2014 22:33:51 +0200
>> , Pantelis Antoniou
>> wrote:
>>> Make sure we call notifier only when the node is attached.
>>> When a detatched tree is being constructed we do not want the
>>> notifiers to fire at all.
>>
>> The description does not match what the patch does. The patch moves the
>> test into of_{add,remove,update}_property() and out of
>> of_property_notify() itself. That leaves one other caller of
>> of_property_notify(); __of_changeset_entry_notify(). The effect of this
>> patch is that applying a changeset will cause notifiers to be fired for
>> each property modified in a changeset. The comment says nothing about
>> the change in behaviour and it sounds like it is a bug fix when it
>> doesn't actually change the behaviour at all for the
>> of_{add,remove,update}_property() paths.
>>
>> This needs a better changelog. It needs to describe what the effects of
>> the patch are and why the change is being made. When someone is
>> bisecting a problem and they land on this change, the changelog needs to
>> give them a good idea about what is going on and why.
>>
>
> Valid points. In fact I performed some tests and with this reverted things
> still work.
>
> The rationale behind this is for when nodes/properties are removed in the
> overlay,
> but since we don't support this for now, we never hit the case where it was
> needed.
>
> Please remove from the patch series, I'll revisit this when I add the removal
> functionality.
Cool, thanks.
g.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/5] of: Add old prop argument on OF_RECONFIG_UPDATE_PROPERTY
On Wed, Nov 5, 2014 at 8:08 PM, Pantelis Antoniou wrote: > Hi Grant, > >> On Nov 5, 2014, at 22:01 , Grant Likely wrote: >> >> On Tue, 28 Oct 2014 22:33:52 +0200 >> , Pantelis Antoniou >> wrote: >>> The notifier now includes the old_prop argument when updating >>> properties, propagate this API to changeset internals while >>> also retaining the old behaviour of retrieving the old_property >>> when NULL is passed. >>> >>> Signed-off-by: Pantelis Antoniou >> >> I'm still fuzzy on the need for this patch. Is this just an optimization >> so that the property doesn't get looked up twice? Or is there a reason >> when the oldprop really needs to be passed in explicitly? >> > > In the case of overlay's applying a property change the old property has > been already retrieved. Passing it as an argument saves us a traversal of the > property list. > > On the original series were removal was supported, the old property was > required > since you can't find the old property anymore on the node (it was on another > list). > > When we go back to revisit the removal case, that API is useful. Since we're not doing removal anymore, lets drop this patch. g. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 0/2] pinctrl: Intel Cherryview/Braswell support
On Wed, Nov 5, 2014 at 10:15 PM, Rafael J. Wysocki r...@rjwysocki.net wrote: On Wednesday, November 05, 2014 09:46:14 PM Grant Likely wrote: On Tue, Nov 4, 2014 at 5:54 PM, Timur Tabi ti...@codeaurora.org wrote: On 11/04/2014 02:20 AM, Mika Westerberg wrote: There is nothing like that yet in ACPI world but with the ACPI _DSD patches we are getting properties similar to DT which means that we can provide pinctrl bindings from ACPI systems as well. Do you know if anyone has signed up to actually do the work of defining how pinctrl will look in ACPI? I'm guessing the simplest approach would be to: Actually, for the most part we /don't/ want to try and put pinctrl into an ACPI binding. The assumption is that on an ACPI platform it would be the combination of ACPI and firmware responsible for the pin mappings, not the operating system. Before even attempting to put pinctrl mappings into your driver, there should instead be a conversation at the ACPI spec level about whether or not it makes sense and what such a binding should look like. We may need to support pinctrl on ACPI platforms in the meantime, though, for reasons that Mika mentioned at one point (there are systems where the firmware mangles things etc., which quite frankly is to be expected). So while I'm all for discussing this in the ASWG, what's the interim approach we should be using, in your opinion? Context would be useful at this point. I know (well, I /strongly/ suspect) that Timur is working on ARM server support, where as most of the work you, Darren and Mika are doing is focused on getting into embedded. The reason I'm pushing for a pinctrl discussion at the ASWG level is because it affects OS portability. The OS should not not be required to have a driver for the pinctrl hardware in order to boot into a usable state (at least far enough to obtain further drivers). The same goes for clock controllers and power regulators. In x86 world this is the default assumption, but in the ARM world it needs to be re-enforced over and over to break out of the embedded mindset that a lot of us have. So, to answer your question, what's the interim approach? I think it is two things: 1) The base assumption must be that firmware sets up the pinctrl hardware into a usable state at boot and ACPI is used to adjust it as part of the normal OSPM runtime PM operations on devices. 2) Where direct control over the pinctrl hardware is required by the OS, build it into the GPIO driver functionality (which from what I understand is exactly what Mika has done). If any data for the pinctrl is required from ACPI, it can be driver specific data, but any attempt to create a generic binding that the OS is expected to understand should be avoided. This is so we don't end up with the OS needing to understand more than is in the ACPI spec in order to boot. For a more generic solution (building into OSPM the understanding of pin groups and how to control them when power managing devices), there needs to be work at the ASWG level to figure out what it is going to look like. g. -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/5] of: Add old prop argument on OF_RECONFIG_UPDATE_PROPERTY
On Wed, Nov 5, 2014 at 8:08 PM, Pantelis Antoniou pantelis.anton...@konsulko.com wrote: Hi Grant, On Nov 5, 2014, at 22:01 , Grant Likely grant.lik...@secretlab.ca wrote: On Tue, 28 Oct 2014 22:33:52 +0200 , Pantelis Antoniou pantelis.anton...@konsulko.com wrote: The notifier now includes the old_prop argument when updating properties, propagate this API to changeset internals while also retaining the old behaviour of retrieving the old_property when NULL is passed. Signed-off-by: Pantelis Antoniou pantelis.anton...@konsulko.com I'm still fuzzy on the need for this patch. Is this just an optimization so that the property doesn't get looked up twice? Or is there a reason when the oldprop really needs to be passed in explicitly? In the case of overlay's applying a property change the old property has been already retrieved. Passing it as an argument saves us a traversal of the property list. On the original series were removal was supported, the old property was required since you can't find the old property anymore on the node (it was on another list). When we go back to revisit the removal case, that API is useful. Since we're not doing removal anymore, lets drop this patch. g. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/5] of: Only call notifiers when node is attached
On Thu, Nov 6, 2014 at 12:33 PM, Pantelis Antoniou
pantelis.anton...@konsulko.com wrote:
Hi Grant,
On Nov 5, 2014, at 23:39 , Grant Likely grant.lik...@secretlab.ca wrote:
On Tue, 28 Oct 2014 22:33:51 +0200
, Pantelis Antoniou pantelis.anton...@konsulko.com
wrote:
Make sure we call notifier only when the node is attached.
When a detatched tree is being constructed we do not want the
notifiers to fire at all.
The description does not match what the patch does. The patch moves the
test into of_{add,remove,update}_property() and out of
of_property_notify() itself. That leaves one other caller of
of_property_notify(); __of_changeset_entry_notify(). The effect of this
patch is that applying a changeset will cause notifiers to be fired for
each property modified in a changeset. The comment says nothing about
the change in behaviour and it sounds like it is a bug fix when it
doesn't actually change the behaviour at all for the
of_{add,remove,update}_property() paths.
This needs a better changelog. It needs to describe what the effects of
the patch are and why the change is being made. When someone is
bisecting a problem and they land on this change, the changelog needs to
give them a good idea about what is going on and why.
Valid points. In fact I performed some tests and with this reverted things
still work.
The rationale behind this is for when nodes/properties are removed in the
overlay,
but since we don't support this for now, we never hit the case where it was
needed.
Please remove from the patch series, I'll revisit this when I add the removal
functionality.
Cool, thanks.
g.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 0/2] pinctrl: Intel Cherryview/Braswell support
On Tue, Nov 4, 2014 at 5:54 PM, Timur Tabi wrote: > On 11/04/2014 02:20 AM, Mika Westerberg wrote: >> >> There is nothing like that yet in ACPI world but with the ACPI _DSD >> patches we are getting properties similar to DT which means that we can >> provide pinctrl bindings from ACPI systems as well. > > > Do you know if anyone has signed up to actually do the work of defining how > pinctrl will look in ACPI? I'm guessing the simplest approach would be to: Actually, for the most part we /don't/ want to try and put pinctrl into an ACPI binding. The assumption is that on an ACPI platform it would be the combination of ACPI and firmware responsible for the pin mappings, not the operating system. Before even attempting to put pinctrl mappings into your driver, there should instead be a conversation at the ACPI spec level about whether or not it makes sense and what such a binding should look like. g. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/5] of: Only call notifiers when node is attached
On Tue, 28 Oct 2014 22:33:51 +0200
, Pantelis Antoniou
wrote:
> Make sure we call notifier only when the node is attached.
> When a detatched tree is being constructed we do not want the
> notifiers to fire at all.
The description does not match what the patch does. The patch moves the
test into of_{add,remove,update}_property() and out of
of_property_notify() itself. That leaves one other caller of
of_property_notify(); __of_changeset_entry_notify(). The effect of this
patch is that applying a changeset will cause notifiers to be fired for
each property modified in a changeset. The comment says nothing about
the change in behaviour and it sounds like it is a bug fix when it
doesn't actually change the behaviour at all for the
of_{add,remove,update}_property() paths.
This needs a better changelog. It needs to describe what the effects of
the patch are and why the change is being made. When someone is
bisecting a problem and they land on this change, the changelog needs to
give them a good idea about what is going on and why.
g.
>
> Signed-off-by: Pantelis Antoniou
> ---
> drivers/of/base.c| 9 ++---
> drivers/of/dynamic.c | 5 +
> 2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index 2305dc0..a79d4ee 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1695,7 +1695,8 @@ int of_add_property(struct device_node *np, struct
> property *prop)
>
> mutex_unlock(_mutex);
>
> - if (!rc)
> + /* only call notifiers if the node is attached and no error occurred */
> + if (of_node_is_attached(np) && !rc)
> of_property_notify(OF_RECONFIG_ADD_PROPERTY, np, prop, NULL);
>
> return rc;
> @@ -1754,7 +1755,8 @@ int of_remove_property(struct device_node *np, struct
> property *prop)
>
> mutex_unlock(_mutex);
>
> - if (!rc)
> + /* only call notifiers if the node is attached and no error occurred */
> + if (of_node_is_attached(np) && !rc)
> of_property_notify(OF_RECONFIG_REMOVE_PROPERTY, np, prop, NULL);
>
> return rc;
> @@ -1830,7 +1832,8 @@ int of_update_property(struct device_node *np, struct
> property *newprop)
>
> mutex_unlock(_mutex);
>
> - if (!rc)
> + /* only call notifiers if the node is attached and no error occurred */
> + if (of_node_is_attached(np) && !rc)
> of_property_notify(OF_RECONFIG_UPDATE_PROPERTY, np, newprop,
> oldprop);
>
> return rc;
> diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
> index f297891..8e8b614 100644
> --- a/drivers/of/dynamic.c
> +++ b/drivers/of/dynamic.c
> @@ -90,10 +90,6 @@ int of_property_notify(int action, struct device_node *np,
> {
> struct of_prop_reconfig pr;
>
> - /* only call notifiers if the node is attached */
> - if (!of_node_is_attached(np))
> - return 0;
> -
> pr.dn = np;
> pr.prop = prop;
> pr.old_prop = oldprop;
> @@ -138,6 +134,7 @@ int of_attach_node(struct device_node *np)
> __of_attach_node_sysfs(np);
> mutex_unlock(_mutex);
>
> + /* node is guaranteed to be attached at this point */
> of_reconfig_notify(OF_RECONFIG_ATTACH_NODE, np);
>
> return 0;
> --
> 1.7.12
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 5/5] of: of_reconfig_get_state_change() of notifier helper.
On Tue, 28 Oct 2014 22:33:53 +0200
, Pantelis Antoniou
wrote:
> Introduce of_reconfig_get_state_change() which allows an of notifier
> to query about device state changes.
>
> Signed-off-by: Pantelis Antoniou
Applied, thanks.
g.
> ---
> drivers/of/dynamic.c | 96
>
> include/linux/of.h | 1 +
> 2 files changed, 97 insertions(+)
>
> diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
> index 9eb8528..303a59b 100644
> --- a/drivers/of/dynamic.c
> +++ b/drivers/of/dynamic.c
> @@ -85,6 +85,102 @@ int of_reconfig_notify(unsigned long action, void *p)
> return notifier_to_errno(rc);
> }
>
> +/*
> + * of_reconfig_get_state_change()- Returns new state of device
> + * @action - action of the of notifier
> + * @arg - argument of the of notifier
> + *
> + * Returns the new state of a device based on the notifier used.
> + * Returns 0 on device going from enabled to disabled, 1 on device
> + * going from disabled to enabled and -1 on no change.
> + */
> +int of_reconfig_get_state_change(unsigned long action, void *arg)
> +{
> + struct device_node *dn;
> + struct property *prop, *old_prop;
> + struct of_prop_reconfig *pr;
> + int is_status, status_state, old_status_state, prev_state, new_state;
> +
> + /* figure out if a device should be created or destroyed */
> + dn = NULL;
> + prop = old_prop = NULL;
> + switch (action) {
> + case OF_RECONFIG_ATTACH_NODE:
> + case OF_RECONFIG_DETACH_NODE:
> + dn = arg;
> + prop = of_find_property(dn, "status", NULL);
> + break;
> + case OF_RECONFIG_ADD_PROPERTY:
> + case OF_RECONFIG_REMOVE_PROPERTY:
> + pr = arg;
> + dn = pr->dn;
> + prop = pr->prop;
> + break;
> + case OF_RECONFIG_UPDATE_PROPERTY:
> + pr = arg;
> + dn = pr->dn;
> + prop = pr->prop;
> + old_prop = pr->old_prop;
> + break;
> + default:
> + return -1; /* don't care */
> + }
> +
> + is_status = 0;
> + status_state = -1;
> + old_status_state = -1;
> + prev_state = -1;
> + new_state = -1;
> +
> + if (prop && !strcmp(prop->name, "status")) {
> + is_status = 1;
> + status_state = !strcmp(prop->value, "okay") ||
> +!strcmp(prop->value, "ok");
> + if (old_prop)
> + old_status_state = !strcmp(old_prop->value, "okay") ||
> +!strcmp(old_prop->value, "ok");
> + }
> +
> + switch (action) {
> + case OF_RECONFIG_ATTACH_NODE:
> + prev_state = 0;
> + /* -1 & 0 status either missing or okay */
> + new_state = status_state != 0;
> + break;
> + case OF_RECONFIG_DETACH_NODE:
> + /* -1 & 0 status either missing or okay */
> + prev_state = status_state != 0;
> + new_state = 0;
> + break;
> + case OF_RECONFIG_ADD_PROPERTY:
> + if (is_status) {
> + /* no status property -> enabled (legacy) */
> + prev_state = 1;
> + new_state = status_state;
> + }
> + break;
> + case OF_RECONFIG_REMOVE_PROPERTY:
> + if (is_status) {
> + prev_state = status_state;
> + /* no status property -> enabled (legacy) */
> + new_state = 1;
> + }
> + break;
> + case OF_RECONFIG_UPDATE_PROPERTY:
> + if (is_status) {
> + prev_state = old_status_state != 0;
> + new_state = status_state != 0;
> + }
> + break;
> + }
> +
> + if (prev_state == new_state)
> + return -1;
> +
> + return new_state;
> +}
> +EXPORT_SYMBOL_GPL(of_reconfig_get_state_change);
> +
> int of_property_notify(int action, struct device_node *np,
> struct property *prop, struct property *oldprop)
> {
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 71d25aa..9ff1ec5 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -328,6 +328,7 @@ struct of_prop_reconfig {
> extern int of_reconfig_notifier_register(struct notifier_block *);
> extern int of_reconfig_notifier_unregister(struct notifier_block *);
> extern int of_reconfig_notify(unsigned long, void *);
> +extern int of_reconfig_get_state_change(unsigned long action, void *arg);
>
> extern int of_attach_node(struct device_node *);
> extern int of_detach_node(struct device_node *);
> --
> 1.7.12
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at
Re: [PATCH 4/5] of: Add old prop argument on OF_RECONFIG_UPDATE_PROPERTY
On Tue, 28 Oct 2014 22:33:52 +0200
, Pantelis Antoniou
wrote:
> The notifier now includes the old_prop argument when updating
> properties, propagate this API to changeset internals while
> also retaining the old behaviour of retrieving the old_property
> when NULL is passed.
>
> Signed-off-by: Pantelis Antoniou
I'm still fuzzy on the need for this patch. Is this just an optimization
so that the property doesn't get looked up twice? Or is there a reason
when the oldprop really needs to be passed in explicitly?
g.
> ---
> drivers/of/dynamic.c | 18 ++
> drivers/of/selftest.c | 2 +-
> include/linux/of.h| 20 +---
> 3 files changed, 28 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
> index 8e8b614..9eb8528 100644
> --- a/drivers/of/dynamic.c
> +++ b/drivers/of/dynamic.c
> @@ -627,6 +627,7 @@ int of_changeset_revert(struct of_changeset *ocs)
> * @action: action to perform
> * @np: Pointer to device node
> * @prop:Pointer to property
> + * @old_prop:Pointer to old property (if updating)
> *
> * On action being one of:
> * + OF_RECONFIG_ATTACH_NODE
> @@ -637,10 +638,21 @@ int of_changeset_revert(struct of_changeset *ocs)
> * Returns 0 on success, a negative error value in case of an error.
> */
> int of_changeset_action(struct of_changeset *ocs, unsigned long action,
> - struct device_node *np, struct property *prop)
> + struct device_node *np, struct property *prop,
> + struct property *oldprop)
> {
> struct of_changeset_entry *ce;
>
> + /* in case someone passed NULL as old_prop, find it */
> + if (action == OF_RECONFIG_UPDATE_PROPERTY && prop && !oldprop) {
> + oldprop = of_find_property(np, prop->name, NULL);
> + if (!oldprop) {
> + pr_err("%s: Failed to find old_prop for \"%s/%s\"\n",
> + __func__, np->full_name, prop->name);
> + return -EINVAL;
> + }
> + }
> +
> ce = kzalloc(sizeof(*ce), GFP_KERNEL);
> if (!ce) {
> pr_err("%s: Failed to allocate\n", __func__);
> @@ -650,9 +662,7 @@ int of_changeset_action(struct of_changeset *ocs,
> unsigned long action,
> ce->action = action;
> ce->np = of_node_get(np);
> ce->prop = prop;
> -
> - if (action == OF_RECONFIG_UPDATE_PROPERTY && prop)
> - ce->old_prop = of_find_property(np, prop->name, NULL);
> + ce->old_prop = oldprop;
>
> /* add it to the list */
> list_add_tail(>node, >entries);
> diff --git a/drivers/of/selftest.c b/drivers/of/selftest.c
> index 7800127..16102fd 100644
> --- a/drivers/of/selftest.c
> +++ b/drivers/of/selftest.c
> @@ -427,7 +427,7 @@ static void __init of_selftest_changeset(void)
> selftest(!of_changeset_detach_node(, nremove), "fail remove
> node\n");
> selftest(!of_changeset_attach_node(, n21), "fail attach n21\n");
> selftest(!of_changeset_add_property(, parent, ppadd), "fail add
> prop\n");
> - selftest(!of_changeset_update_property(, parent, ppupdate),
> "fail update prop\n");
> + selftest(!of_changeset_update_property(, parent, ppupdate,
> NULL), "fail update prop\n");
> selftest(!of_changeset_remove_property(, parent, ppremove),
> "fail remove prop\n");
> mutex_lock(_mutex);
> selftest(!of_changeset_apply(), "apply failed\n");
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 6545e7a..71d25aa 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -830,36 +830,42 @@ extern int of_changeset_apply(struct of_changeset *ocs);
> extern int of_changeset_revert(struct of_changeset *ocs);
> extern int of_changeset_action(struct of_changeset *ocs,
> unsigned long action, struct device_node *np,
> - struct property *prop);
> + struct property *prop, struct property *old_prop);
>
> static inline int of_changeset_attach_node(struct of_changeset *ocs,
> struct device_node *np)
> {
> - return of_changeset_action(ocs, OF_RECONFIG_ATTACH_NODE, np, NULL);
> + return of_changeset_action(ocs, OF_RECONFIG_ATTACH_NODE, np, NULL,
> + NULL);
> }
>
> static inline int of_changeset_detach_node(struct of_changeset *ocs,
> struct device_node *np)
> {
> - return of_changeset_action(ocs, OF_RECONFIG_DETACH_NODE, np, NULL);
> + return of_changeset_action(ocs, OF_RECONFIG_DETACH_NODE, np, NULL,
> + NULL);
> }
>
> static inline int of_changeset_add_property(struct of_changeset *ocs,
> struct device_node *np, struct property *prop)
> {
> - return of_changeset_action(ocs, OF_RECONFIG_ADD_PROPERTY, np, prop);
> + return of_changeset_action(ocs, OF_RECONFIG_ADD_PROPERTY, np, prop,
> + NULL);
> }
>
> static inline int
Re: [PATCH 1/1] of/fdt: simplify early_init_dt_verify
On Wed, Nov 5, 2014 at 12:49 AM, leizhen wrote: > On 2014/11/5 1:03, Bjorn Helgaas wrote: >> On Tue, Nov 4, 2014 at 9:20 AM, Grant Likely wrote: >>> On Tue, 28 Oct 2014 19:51:05 +0800 >>> , Zhen Lei >>> wrote: >>>> put off initial_boot_params = params; until all checks finished. If any >>>> check >>>> failed, initial_boot_params is equal NULL, base upon bss segment initial >>>> value. >>>> >>>> Signed-off-by: Zhen Lei >>> >>> Interesting. I received exactly the same patch from Bjorn Helgaas. Were >>> the two of you working together, or is this merely a coincedence? >> >> LOL, that's really funny. We're not working together, so it's a total >> coincidence :) >> > Haha!!! > > Hi, Bjorn Helgaas. Nice to meet you. I've applied Bjorn's version, but since the diffstat is *identical*, I've put both of your s-o-b lines into it. BTW Lei, what prompted you to write this patch? Was there a bug you were fixing, or did you just notice the inconsistency and decide to fix it? g. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] of/fdt: simplify early_init_dt_verify
On Wed, Nov 5, 2014 at 12:49 AM, leizhen thunder.leiz...@huawei.com wrote: On 2014/11/5 1:03, Bjorn Helgaas wrote: On Tue, Nov 4, 2014 at 9:20 AM, Grant Likely grant.lik...@linaro.org wrote: On Tue, 28 Oct 2014 19:51:05 +0800 , Zhen Lei thunder.leiz...@huawei.com wrote: put off initial_boot_params = params; until all checks finished. If any check failed, initial_boot_params is equal NULL, base upon bss segment initial value. Signed-off-by: Zhen Lei thunder.leiz...@huawei.com Interesting. I received exactly the same patch from Bjorn Helgaas. Were the two of you working together, or is this merely a coincedence? LOL, that's really funny. We're not working together, so it's a total coincidence :) Haha!!! Hi, Bjorn Helgaas. Nice to meet you. I've applied Bjorn's version, but since the diffstat is *identical*, I've put both of your s-o-b lines into it. BTW Lei, what prompted you to write this patch? Was there a bug you were fixing, or did you just notice the inconsistency and decide to fix it? g. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/5] of: Add old prop argument on OF_RECONFIG_UPDATE_PROPERTY
On Tue, 28 Oct 2014 22:33:52 +0200
, Pantelis Antoniou pantelis.anton...@konsulko.com
wrote:
The notifier now includes the old_prop argument when updating
properties, propagate this API to changeset internals while
also retaining the old behaviour of retrieving the old_property
when NULL is passed.
Signed-off-by: Pantelis Antoniou pantelis.anton...@konsulko.com
I'm still fuzzy on the need for this patch. Is this just an optimization
so that the property doesn't get looked up twice? Or is there a reason
when the oldprop really needs to be passed in explicitly?
g.
---
drivers/of/dynamic.c | 18 ++
drivers/of/selftest.c | 2 +-
include/linux/of.h| 20 +---
3 files changed, 28 insertions(+), 12 deletions(-)
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 8e8b614..9eb8528 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -627,6 +627,7 @@ int of_changeset_revert(struct of_changeset *ocs)
* @action: action to perform
* @np: Pointer to device node
* @prop:Pointer to property
+ * @old_prop:Pointer to old property (if updating)
*
* On action being one of:
* + OF_RECONFIG_ATTACH_NODE
@@ -637,10 +638,21 @@ int of_changeset_revert(struct of_changeset *ocs)
* Returns 0 on success, a negative error value in case of an error.
*/
int of_changeset_action(struct of_changeset *ocs, unsigned long action,
- struct device_node *np, struct property *prop)
+ struct device_node *np, struct property *prop,
+ struct property *oldprop)
{
struct of_changeset_entry *ce;
+ /* in case someone passed NULL as old_prop, find it */
+ if (action == OF_RECONFIG_UPDATE_PROPERTY prop !oldprop) {
+ oldprop = of_find_property(np, prop-name, NULL);
+ if (!oldprop) {
+ pr_err(%s: Failed to find old_prop for \%s/%s\\n,
+ __func__, np-full_name, prop-name);
+ return -EINVAL;
+ }
+ }
+
ce = kzalloc(sizeof(*ce), GFP_KERNEL);
if (!ce) {
pr_err(%s: Failed to allocate\n, __func__);
@@ -650,9 +662,7 @@ int of_changeset_action(struct of_changeset *ocs,
unsigned long action,
ce-action = action;
ce-np = of_node_get(np);
ce-prop = prop;
-
- if (action == OF_RECONFIG_UPDATE_PROPERTY prop)
- ce-old_prop = of_find_property(np, prop-name, NULL);
+ ce-old_prop = oldprop;
/* add it to the list */
list_add_tail(ce-node, ocs-entries);
diff --git a/drivers/of/selftest.c b/drivers/of/selftest.c
index 7800127..16102fd 100644
--- a/drivers/of/selftest.c
+++ b/drivers/of/selftest.c
@@ -427,7 +427,7 @@ static void __init of_selftest_changeset(void)
selftest(!of_changeset_detach_node(chgset, nremove), fail remove
node\n);
selftest(!of_changeset_attach_node(chgset, n21), fail attach n21\n);
selftest(!of_changeset_add_property(chgset, parent, ppadd), fail add
prop\n);
- selftest(!of_changeset_update_property(chgset, parent, ppupdate),
fail update prop\n);
+ selftest(!of_changeset_update_property(chgset, parent, ppupdate,
NULL), fail update prop\n);
selftest(!of_changeset_remove_property(chgset, parent, ppremove),
fail remove prop\n);
mutex_lock(of_mutex);
selftest(!of_changeset_apply(chgset), apply failed\n);
diff --git a/include/linux/of.h b/include/linux/of.h
index 6545e7a..71d25aa 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -830,36 +830,42 @@ extern int of_changeset_apply(struct of_changeset *ocs);
extern int of_changeset_revert(struct of_changeset *ocs);
extern int of_changeset_action(struct of_changeset *ocs,
unsigned long action, struct device_node *np,
- struct property *prop);
+ struct property *prop, struct property *old_prop);
static inline int of_changeset_attach_node(struct of_changeset *ocs,
struct device_node *np)
{
- return of_changeset_action(ocs, OF_RECONFIG_ATTACH_NODE, np, NULL);
+ return of_changeset_action(ocs, OF_RECONFIG_ATTACH_NODE, np, NULL,
+ NULL);
}
static inline int of_changeset_detach_node(struct of_changeset *ocs,
struct device_node *np)
{
- return of_changeset_action(ocs, OF_RECONFIG_DETACH_NODE, np, NULL);
+ return of_changeset_action(ocs, OF_RECONFIG_DETACH_NODE, np, NULL,
+ NULL);
}
static inline int of_changeset_add_property(struct of_changeset *ocs,
struct device_node *np, struct property *prop)
{
- return of_changeset_action(ocs, OF_RECONFIG_ADD_PROPERTY, np, prop);
+ return of_changeset_action(ocs, OF_RECONFIG_ADD_PROPERTY, np, prop,
+ NULL);
}
static inline int of_changeset_remove_property(struct of_changeset *ocs,
Re: [PATCH 5/5] of: of_reconfig_get_state_change() of notifier helper.
On Tue, 28 Oct 2014 22:33:53 +0200
, Pantelis Antoniou pantelis.anton...@konsulko.com
wrote:
Introduce of_reconfig_get_state_change() which allows an of notifier
to query about device state changes.
Signed-off-by: Pantelis Antoniou pantelis.anton...@konsulko.com
Applied, thanks.
g.
---
drivers/of/dynamic.c | 96
include/linux/of.h | 1 +
2 files changed, 97 insertions(+)
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index 9eb8528..303a59b 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -85,6 +85,102 @@ int of_reconfig_notify(unsigned long action, void *p)
return notifier_to_errno(rc);
}
+/*
+ * of_reconfig_get_state_change()- Returns new state of device
+ * @action - action of the of notifier
+ * @arg - argument of the of notifier
+ *
+ * Returns the new state of a device based on the notifier used.
+ * Returns 0 on device going from enabled to disabled, 1 on device
+ * going from disabled to enabled and -1 on no change.
+ */
+int of_reconfig_get_state_change(unsigned long action, void *arg)
+{
+ struct device_node *dn;
+ struct property *prop, *old_prop;
+ struct of_prop_reconfig *pr;
+ int is_status, status_state, old_status_state, prev_state, new_state;
+
+ /* figure out if a device should be created or destroyed */
+ dn = NULL;
+ prop = old_prop = NULL;
+ switch (action) {
+ case OF_RECONFIG_ATTACH_NODE:
+ case OF_RECONFIG_DETACH_NODE:
+ dn = arg;
+ prop = of_find_property(dn, status, NULL);
+ break;
+ case OF_RECONFIG_ADD_PROPERTY:
+ case OF_RECONFIG_REMOVE_PROPERTY:
+ pr = arg;
+ dn = pr-dn;
+ prop = pr-prop;
+ break;
+ case OF_RECONFIG_UPDATE_PROPERTY:
+ pr = arg;
+ dn = pr-dn;
+ prop = pr-prop;
+ old_prop = pr-old_prop;
+ break;
+ default:
+ return -1; /* don't care */
+ }
+
+ is_status = 0;
+ status_state = -1;
+ old_status_state = -1;
+ prev_state = -1;
+ new_state = -1;
+
+ if (prop !strcmp(prop-name, status)) {
+ is_status = 1;
+ status_state = !strcmp(prop-value, okay) ||
+!strcmp(prop-value, ok);
+ if (old_prop)
+ old_status_state = !strcmp(old_prop-value, okay) ||
+!strcmp(old_prop-value, ok);
+ }
+
+ switch (action) {
+ case OF_RECONFIG_ATTACH_NODE:
+ prev_state = 0;
+ /* -1 0 status either missing or okay */
+ new_state = status_state != 0;
+ break;
+ case OF_RECONFIG_DETACH_NODE:
+ /* -1 0 status either missing or okay */
+ prev_state = status_state != 0;
+ new_state = 0;
+ break;
+ case OF_RECONFIG_ADD_PROPERTY:
+ if (is_status) {
+ /* no status property - enabled (legacy) */
+ prev_state = 1;
+ new_state = status_state;
+ }
+ break;
+ case OF_RECONFIG_REMOVE_PROPERTY:
+ if (is_status) {
+ prev_state = status_state;
+ /* no status property - enabled (legacy) */
+ new_state = 1;
+ }
+ break;
+ case OF_RECONFIG_UPDATE_PROPERTY:
+ if (is_status) {
+ prev_state = old_status_state != 0;
+ new_state = status_state != 0;
+ }
+ break;
+ }
+
+ if (prev_state == new_state)
+ return -1;
+
+ return new_state;
+}
+EXPORT_SYMBOL_GPL(of_reconfig_get_state_change);
+
int of_property_notify(int action, struct device_node *np,
struct property *prop, struct property *oldprop)
{
diff --git a/include/linux/of.h b/include/linux/of.h
index 71d25aa..9ff1ec5 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -328,6 +328,7 @@ struct of_prop_reconfig {
extern int of_reconfig_notifier_register(struct notifier_block *);
extern int of_reconfig_notifier_unregister(struct notifier_block *);
extern int of_reconfig_notify(unsigned long, void *);
+extern int of_reconfig_get_state_change(unsigned long action, void *arg);
extern int of_attach_node(struct device_node *);
extern int of_detach_node(struct device_node *);
--
1.7.12
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 3/5] of: Only call notifiers when node is attached
On Tue, 28 Oct 2014 22:33:51 +0200
, Pantelis Antoniou pantelis.anton...@konsulko.com
wrote:
Make sure we call notifier only when the node is attached.
When a detatched tree is being constructed we do not want the
notifiers to fire at all.
The description does not match what the patch does. The patch moves the
test into of_{add,remove,update}_property() and out of
of_property_notify() itself. That leaves one other caller of
of_property_notify(); __of_changeset_entry_notify(). The effect of this
patch is that applying a changeset will cause notifiers to be fired for
each property modified in a changeset. The comment says nothing about
the change in behaviour and it sounds like it is a bug fix when it
doesn't actually change the behaviour at all for the
of_{add,remove,update}_property() paths.
This needs a better changelog. It needs to describe what the effects of
the patch are and why the change is being made. When someone is
bisecting a problem and they land on this change, the changelog needs to
give them a good idea about what is going on and why.
g.
Signed-off-by: Pantelis Antoniou pantelis.anton...@konsulko.com
---
drivers/of/base.c| 9 ++---
drivers/of/dynamic.c | 5 +
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 2305dc0..a79d4ee 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1695,7 +1695,8 @@ int of_add_property(struct device_node *np, struct
property *prop)
mutex_unlock(of_mutex);
- if (!rc)
+ /* only call notifiers if the node is attached and no error occurred */
+ if (of_node_is_attached(np) !rc)
of_property_notify(OF_RECONFIG_ADD_PROPERTY, np, prop, NULL);
return rc;
@@ -1754,7 +1755,8 @@ int of_remove_property(struct device_node *np, struct
property *prop)
mutex_unlock(of_mutex);
- if (!rc)
+ /* only call notifiers if the node is attached and no error occurred */
+ if (of_node_is_attached(np) !rc)
of_property_notify(OF_RECONFIG_REMOVE_PROPERTY, np, prop, NULL);
return rc;
@@ -1830,7 +1832,8 @@ int of_update_property(struct device_node *np, struct
property *newprop)
mutex_unlock(of_mutex);
- if (!rc)
+ /* only call notifiers if the node is attached and no error occurred */
+ if (of_node_is_attached(np) !rc)
of_property_notify(OF_RECONFIG_UPDATE_PROPERTY, np, newprop,
oldprop);
return rc;
diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
index f297891..8e8b614 100644
--- a/drivers/of/dynamic.c
+++ b/drivers/of/dynamic.c
@@ -90,10 +90,6 @@ int of_property_notify(int action, struct device_node *np,
{
struct of_prop_reconfig pr;
- /* only call notifiers if the node is attached */
- if (!of_node_is_attached(np))
- return 0;
-
pr.dn = np;
pr.prop = prop;
pr.old_prop = oldprop;
@@ -138,6 +134,7 @@ int of_attach_node(struct device_node *np)
__of_attach_node_sysfs(np);
mutex_unlock(of_mutex);
+ /* node is guaranteed to be attached at this point */
of_reconfig_notify(OF_RECONFIG_ATTACH_NODE, np);
return 0;
--
1.7.12
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2 0/2] pinctrl: Intel Cherryview/Braswell support
On Tue, Nov 4, 2014 at 5:54 PM, Timur Tabi ti...@codeaurora.org wrote: On 11/04/2014 02:20 AM, Mika Westerberg wrote: There is nothing like that yet in ACPI world but with the ACPI _DSD patches we are getting properties similar to DT which means that we can provide pinctrl bindings from ACPI systems as well. Do you know if anyone has signed up to actually do the work of defining how pinctrl will look in ACPI? I'm guessing the simplest approach would be to: Actually, for the most part we /don't/ want to try and put pinctrl into an ACPI binding. The assumption is that on an ACPI platform it would be the combination of ACPI and firmware responsible for the pin mappings, not the operating system. Before even attempting to put pinctrl mappings into your driver, there should instead be a conversation at the ACPI spec level about whether or not it makes sense and what such a binding should look like. g. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Tue, Nov 4, 2014 at 10:54 PM, Rafael J. Wysocki wrote:
> On Tuesday, November 04, 2014 05:06:40 PM Rafael J. Wysocki wrote:
>> On Tuesday, November 04, 2014 02:48:40 PM Grant Likely wrote:
>> > On Mon, Nov 3, 2014 at 9:52 PM, Rafael J. Wysocki
>> > wrote:
>> > > On Monday, November 03, 2014 04:25:08 PM Rafael J. Wysocki wrote:
>> > >> On Sunday, November 02, 2014 08:49:37 PM Darren Hart wrote:
>> > >> >
>> > >> > On 11/1/14 4:11, Grant Likely wrote:
>> > >> > > On Tue, 28 Oct 2014 22:59:57 +0100
>> > >> > > , "Rafael J. Wysocki"
>> > >> > > wrote:
>> > >> > >> On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
>> > >> > >>> acpi_dev_add_driver_gpios() makes it possible to set up mapping
>> > >> > >>> between
>> > >> > >>> properties and ACPI GpioIo resources in a driver, so we can take
>> > >> > >>> index
>> > >> > >>> parameter in acpi_find_gpio() into use with _DSD device
>> > >> > >>> properties now.
>> > >> > >>>
>> > >> > >>> This index can be used to select a GPIO from a property with
>> > >> > >>> multiple
>> > >> > >>> GPIOs:
>> > >> > >>>
>> > >> > >>> Package () {
>> > >> > >>> "data-gpios",
>> > >> > >>> Package () {
>> > >> > >>> \_SB.GPIO, 0, 0, 0,
>> > >> > >>> \_SB.GPIO, 1, 0, 0,
>> > >> > >>> \_SB.GPIO, 2, 0, 1,
>> > >> > >>> }
>> > >> > >>> }
>> > >> > >>>
>> > >> > >>> In order to retrieve the last GPIO from a driver we can simply do:
>> > >> > >>>
>> > >> > >>> desc = devm_gpiod_get_index(dev, "data", 2);
>> > >> > >>>
>> > >> > >>> and so on.
>> > >> > >>>
>> > >> > >>> Signed-off-by: Mika Westerberg
>> > >> > >>
>> > >> > >> Cool. :-)
>> > >> > >>
>> > >> > >> Any objections anyone?
>> > >> > >
>> > >> > > Actually, I do. Not in the idea, but in the implementation. The way
>> > >> > > this gets encoded:
>> > >> > >
>> > >> > > Package () {
>> > >> > > \_SB.GPIO, 0, 0, 0,
>> > >> > > \_SB.GPIO, 1, 0, 0,
>> > >> > > \_SB.GPIO, 2, 0, 1,
>> > >> > > }
>> > >> > >
>> > >> > > Means that decoding each GPIO tuple requires the length of a tuple
>> > >> > > to be
>> > >> > > fixed, or to implement a DT-like #gpio-cells. If it is fixed, then
>> > >> > > there
>> > >> > > is no way to expand the binding later. Can this be done in the
>> > >> > > following
>> > >> > > way instead?
>> > >> > >
>> > >> > > Package () {
>> > >> > > Package () { \_SB.GPIO, 0, 0, 0 },
>> > >> > > Package () { \_SB.GPIO, 1, 0, 0 },
>> > >> > > Package () { \_SB.GPIO, 2, 0, 1 },
>> > >> > > }
>> > >> > >
>> > >> > > This is one of the biggest pains in device tree. We don't have any
>> > >> > > way
>> > >> > > to group tuples so it requires looking up stuff across the tree to
>> > >> > > figure out how to parse each multi-item property.
>> > >> > >
>> > >> > > I know that last year we talked about how bios vendors would get
>> > >> > > complicated properties wrong, but I think there is little risk in
>> > >> > > this
>> > >> > > case. If the property is encoded wrong, the driver simply won't
>> > >> > > work and
>> > >> > > it is unlikely to get shipped before being fixed.
>> > >> >
>>
Re: [PATCH v6 02/12] Driver core: Unified device properties interface for platform firmware
On Mon, Nov 3, 2014 at 10:04 PM, Rafael J. Wysocki wrote: >> I also found that this parser code doesn't correctly handle malformed >> (unterminated) string properties. It will overflow. The existing >> functions have the same problem, so it isn't something that you've >> added. I've got a fix, and as a side effect the fix creates the _array >> version basically for free as part of reworking >> of_property_count_strings() and of_property_read_string_index() > > OK > > So can you please point me to a git branch containing the fix? I'll rebase > the > patch on top of that then and everything should merge just fine. git://git.kernel.org/pub/scm/linux/kernel/git/glikely/linux.git devicetree/merge I'm going to let the patch sit in there for a few days to get linux-next exposure before I ask Linus to pull. The pull req will go out before the end of the week. g. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: Fix padding in _OF_DECLARE macro definition
On Thu, 2 Oct 2014 16:01:10 +0200
, Thierry Reding
wrote:
> From: Thierry Reding
>
> Signed-off-by: Thierry Reding
Applied, thanks.
g.
> ---
> include/linux/of.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 6c4363b8ddc3..951e9ee7d3e0 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -772,7 +772,7 @@ static inline int of_get_available_child_count(const
> struct device_node *np)
>= { .compatible = compat, \
>.data = (fn == (fn_type)NULL) ? fn : fn }
> #else
> -#define _OF_DECLARE(table, name, compat, fn, fn_type)
> \
> +#define _OF_DECLARE(table, name, compat, fn, fn_type)
> \
> static const struct of_device_id __of_table_##name \
> __attribute__((unused)) \
>= { .compatible = compat, \
> --
> 2.1.0
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] of: Add a function to read 64-bit arrays
On Fri, 3 Oct 2014 00:59:23 +0300
, Sakari Ailus
wrote:
> Implement of_property_read_u64_array() for reading 64-bit arrays.
>
> This is needed for e.g. reading the valid link frequencies in the smiapp
> driver.
>
> Signed-off-by: Sakari Ailus
A patch that adds this function is alread part of the device properties
API patch series that will be merged for v3.19.
g.
> ---
> Hi,
>
> While the smiapp (found in drivers/media/i2c/smiapp/) OF support which needs
> this isn't in yet, other drivers such as mt9v032 which would be reading the
> valid link frequency control values will need reading arrays. This might
> make it to v4l2-of.c in the end.
>
> drivers/of/base.c | 44
> include/linux/of.h |3 +++
> 2 files changed, 39 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index d8574ad..35e24f4 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -1214,6 +1214,41 @@ int of_property_read_u32_array(const struct
> device_node *np,
> EXPORT_SYMBOL_GPL(of_property_read_u32_array);
>
> /**
> + * of_property_read_u64_array - Find and read an array of 64 bit integers
> + * from a property.
> + *
> + * @np: device node from which the property value is to be read.
> + * @propname:name of the property to be searched.
> + * @out_values: pointer to return value, modified only if return value
> is 0.
> + * @sz: number of array elements to read
> + *
> + * Search for a property in a device node and read 64-bit value(s) from
> + * it. Returns 0 on success, -EINVAL if the property does not exist,
> + * -ENODATA if property does not have a value, and -EOVERFLOW if the
> + * property data isn't large enough.
> + *
> + * The out_values is modified only if a valid u32 value can be decoded.
> + */
> +int of_property_read_u64_array(const struct device_node *np,
> +const char *propname, u64 *out_value, size_t sz)
> +{
> + const __be32 *val = of_find_property_value_of_size(
> + np, propname, sz * sizeof(*out_value));
> +
> + if (IS_ERR(val))
> + return PTR_ERR(val);
> +
> + while (sz--) {
> + *out_value = of_read_number(val, 2);
> + out_value++;
> + val += 2;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(of_property_read_u64_array);
> +
> +/**
> * of_property_read_u64 - Find and read a 64 bit integer from a property
> * @np: device node from which the property value is to be read.
> * @propname:name of the property to be searched.
> @@ -1229,14 +1264,7 @@ EXPORT_SYMBOL_GPL(of_property_read_u32_array);
> int of_property_read_u64(const struct device_node *np, const char *propname,
>u64 *out_value)
> {
> - const __be32 *val = of_find_property_value_of_size(np, propname,
> - sizeof(*out_value));
> -
> - if (IS_ERR(val))
> - return PTR_ERR(val);
> -
> - *out_value = of_read_number(val, 2);
> - return 0;
> + return of_property_read_u64_array(np, propname, out_value, 1);
> }
> EXPORT_SYMBOL_GPL(of_property_read_u64);
>
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 6c4363b..e84533f 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -263,6 +263,9 @@ extern int of_property_read_u32_array(const struct
> device_node *np,
> size_t sz);
> extern int of_property_read_u64(const struct device_node *np,
> const char *propname, u64 *out_value);
> +extern int of_property_read_u64_array(const struct device_node *np,
> + const char *propname, u64 *out_value,
> + size_t sz);
>
> extern int of_property_read_string(struct device_node *np,
> const char *propname,
> --
> 1.7.10.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] of: Remove spaces before tabs
On Wed, 22 Oct 2014 11:44:55 +0200
, Geert Uytterhoeven
wrote:
> Signed-off-by: Geert Uytterhoeven
Applied all 4 patches. Thanks.
g.
> ---
> drivers/of/base.c | 14 +++---
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index cff3afcff1951faf..19075e770d8b6306 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -577,9 +577,9 @@ EXPORT_SYMBOL(of_get_parent);
> * of_get_next_parent - Iterate to a node's parent
> * @node: Node to get parent of
> *
> - * This is like of_get_parent() except that it drops the
> - * refcount on the passed node, making it suitable for iterating
> - * through a node's parents.
> + * This is like of_get_parent() except that it drops the
> + * refcount on the passed node, making it suitable for iterating
> + * through a node's parents.
> *
> * Returns a node pointer with refcount incremented, use
> * of_node_put() on it when done.
> @@ -1566,15 +1566,15 @@ EXPORT_SYMBOL(of_parse_phandle);
> * Example:
> *
> * phandle1: node1 {
> - * #list-cells = <2>;
> + * #list-cells = <2>;
> * }
> *
> * phandle2: node2 {
> - * #list-cells = <1>;
> + * #list-cells = <1>;
> * }
> *
> * node3 {
> - * list = < 1 2 3>;
> + * list = < 1 2 3>;
> * }
> *
> * To get a device_node of the `node2' node you may call this:
> @@ -1615,7 +1615,7 @@ EXPORT_SYMBOL(of_parse_phandle_with_args);
> * }
> *
> * node3 {
> - * list = < 0 2 2 3>;
> + * list = < 0 2 2 3>;
> * }
> *
> * To get a device_node of the `node2' node you may call this:
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] of/fdt: simplify early_init_dt_verify
On Tue, 28 Oct 2014 19:51:05 +0800
, Zhen Lei
wrote:
> put off initial_boot_params = params; until all checks finished. If any check
> failed, initial_boot_params is equal NULL, base upon bss segment initial
> value.
>
> Signed-off-by: Zhen Lei
Interesting. I received exactly the same patch from Bjorn Helgaas. Were
the two of you working together, or is this merely a coincedence?
g.
> ---
> drivers/of/fdt.c | 10 --
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index d1ffca8..325c638 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -994,14 +994,12 @@ bool __init early_init_dt_verify(void *params)
> if (!params)
> return false;
>
> - /* Setup flat device-tree pointer */
> - initial_boot_params = params;
> -
> /* check device tree validity */
> - if (fdt_check_header(params)) {
> - initial_boot_params = NULL;
> + if (fdt_check_header(params))
> return false;
> - }
> +
> + /* Setup flat device-tree pointer */
> + initial_boot_params = params;
>
> return true;
> }
> --
> 1.8.0
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of/fdt: Don't clear initial_boot_params if fdt_check_header() fails
On Wed, 29 Oct 2014 12:15:00 -0600
, Bjorn Helgaas
wrote:
> If the device tree pointer is NULL, early_init_dt_verify() fails, leaving
> initial_boot_params unchanged. If the device tree pointer is non-NULL but
> invalid, early_init_dt_verify() again fails but this time it also clears
> initial_boot_params.
>
> Leave initial_boot_params unchanged if the device tree pointer is invalid.
> This doesn't fix a bug, but it makes the behavior more consistent and
> easier to analyze.
>
> Signed-off-by: Bjorn Helgaas
Looks correct. Applied.
g.
> ---
> drivers/of/fdt.c |9 +++--
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index d1ffca8b34ea..55a566a53f36 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -994,15 +994,12 @@ bool __init early_init_dt_verify(void *params)
> if (!params)
> return false;
>
> - /* Setup flat device-tree pointer */
> - initial_boot_params = params;
> -
> /* check device tree validity */
> - if (fdt_check_header(params)) {
> - initial_boot_params = NULL;
> + if (fdt_check_header(params))
> return false;
> - }
>
> + /* Setup flat device-tree pointer */
> + initial_boot_params = params;
> return true;
> }
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/1] Compact interface for Device-Tree
On Mon, 03 Nov 2014 16:06:03 +0100
, Arnd Bergmann
wrote:
> On Friday 31 October 2014 23:53:28 Rafael J. Wysocki wrote:
> > On Saturday, November 01, 2014 05:13:45 AM Rob Herring wrote:
> > > On Fri, Oct 31, 2014 at 6:59 AM, Gilad Avidov
> > > wrote:
> > > >
> > > > Device-Tree compact API
> > > >
> > > >
> > > > Common code seen in driverâs probe reads device tree values and
> > > > handling
> > > > erroneous return codes from all those of_property_read_xxx() APIs. This
> > > > common code is factored out by the of_property_map module which allows
> > > > driverâs probe to replace that (often lengthy) code with a concise
> > > > table:
> > > >
> > > > struct of_prop_map map[] = {
> > > > {"i2c",>id,OF_REQ, OF_ID, -1},
> > > > {"qcom,clk-freq-out",>clk_freq_out,OF_REQ, OF_U32,
> > > > 0},
> > > > {"qcom,clk-freq-in",>clk_freq_in,OF_REQ, OF_U32, 0},
> > > > {"qcom,disable-dma",>disable_dma,OF_OPT, OF_BOOL, 0},
> > > > {"qcom,master-id",>mstr_id,OF_SGST, OF_U32, 0},
> > > > {NULL,NULL,0, 0, 0},
> > > > };
> > > >
> > > > Then call populate to read the values into the deviceâs variables:
> > > >
> > > > ret = of_prop_populate(dev, dev->of_node, map);
> > >
> > > Interesting idea. The main concern I have with this is there has been
> > > on-going discussions about how to generalize property handling across
> > > DT and ACPI to make drivers more agnostic, so I'm copying a few folks
> > > involved in that. That may be a bit orthogonal to what this is doing,
> > > but we may want some coordination here.
> >
> > Agreed.
> >
> > We actually have a patchset adding a unified device property API in
> > linux-next
> > (http://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/log/?h=device-properties)
> > and I'd prefer to see the "compactization" to happen at that level, if
> > possible,
> > rather that for of_ only.
>
> Agreed, this should definitely use the new generalized API.
> I have prototyped a similar concept last year, which actually went much
> further and also abstracted high-level properties such as interrupts,
> gpios, pwm, dma-engine, etc. I still think we should do something
> like that, but I've never had the time to follow up and nobody else
> picked up my work from back then.
>
> Would others like to see that?
Absolutely. I also tried to do the same thing and didn't get very far.
And, yes, it should be done at the level of the device properties API.
g.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: Request and map make argument name constant
On Tue, 21 Oct 2014 18:27:25 +0200
, Matthias Brugger
wrote:
> This patch makes the name argument from of_io_request_and_map constant.
>
> Signed-off-by: Matthias Brugger
Applied, thanks.
g.
> ---
> drivers/of/address.c | 2 +-
> include/linux/of_address.h | 4 ++--
> 2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index afdb782..e02828f 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -871,7 +871,7 @@ EXPORT_SYMBOL(of_iomap);
> * return PTR_ERR(base);
> */
> void __iomem *of_io_request_and_map(struct device_node *np, int index,
> - char *name)
> + const char *name)
> {
> struct resource res;
> void __iomem *mem;
> diff --git a/include/linux/of_address.h b/include/linux/of_address.h
> index 8cb14eb..d88e81b 100644
> --- a/include/linux/of_address.h
> +++ b/include/linux/of_address.h
> @@ -106,7 +106,7 @@ extern int of_address_to_resource(struct device_node
> *dev, int index,
> struct resource *r);
> void __iomem *of_iomap(struct device_node *node, int index);
> void __iomem *of_io_request_and_map(struct device_node *device,
> - int index, char *name);
> + int index, const char *name);
> #else
>
> #include
> @@ -123,7 +123,7 @@ static inline void __iomem *of_iomap(struct device_node
> *device, int index)
> }
>
> static inline void __iomem *of_io_request_and_map(struct device_node *device,
> - int index, char *name)
> + int index, const char *name)
> {
> return IOMEM_ERR_PTR(-EINVAL);
> }
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] of/irq: Export of_irq_get()
On Thu, 30 Oct 2014 15:16:44 +0100 , Wolfram Sang wrote: > On Thu, Oct 30, 2014 at 03:59:36PM +0200, Laurent Pinchart wrote: > > The function will be used by the I2C core which can be compiled as a > > module. > > > > Signed-off-by: Laurent Pinchart > > I think it makes sense if I take this via I2C to get the dependencies > for the later patches right? Agreed. Acked-by: Grant Likely > > > --- > > drivers/of/irq.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/of/irq.c b/drivers/of/irq.c > > index 1471e0a223a5..0d7765807f49 100644 > > --- a/drivers/of/irq.c > > +++ b/drivers/of/irq.c > > @@ -405,6 +405,7 @@ int of_irq_get(struct device_node *dev, int index) > > > > return irq_create_of_mapping(); > > } > > +EXPORT_SYMBOL_GPL(of_irq_get); > > > > /** > > * of_irq_get_byname - Decode a node's IRQ and return it as a Linux irq > > number > > -- > > 2.0.4 > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-i2c" in > > the body of a message to majord...@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 02/12] Driver core: Unified device properties interface for platform firmware
On Tue, 21 Oct 2014 23:15:50 +0200
, "Rafael J. Wysocki"
wrote:
> From: "Rafael J. Wysocki"
>
> Add a uniform interface by which device drivers can request device
> properties from the platform firmware by providing a property name
> and the corresponding data type. The purpose of it is to help to
> write portable code that won't depend on any particular platform
> firmware interface.
>
> The following general helper functions are added:
>
> device_property_present()
> device_property_read_u8()
> device_property_read_u16()
> device_property_read_u32()
> device_property_read_u64()
> device_property_read_string()
> device_property_read_u8_array()
> device_property_read_u16_array()
> device_property_read_u32_array()
> device_property_read_u64_array()
> device_property_read_string_array()
>
> The first one allows the caller to check if the given property is
> present. The next 5 of them allow single-valued properties of
> various types to be retrieved in a uniform way. The remaining 5 are
> for reading properties with multiple values (arrays of either numbers
> or strings).
>
> The interface covers both ACPI and Device Trees.
>
> This change set includes material from Mika Westerberg and Aaron Lu.
>
> Signed-off-by: Aaron Lu
> Signed-off-by: Mika Westerberg
> Acked-by: Greg Kroah-Hartman
> Signed-off-by: Rafael J. Wysocki
> ---
>
> Changes from v5:
> - acpi_dev_prop_read() can now handle both list (package) and single-value
> properties (the latter are tried first for the last argument equal to 1).
> - There is a new macro to generate the bodies of
> device_property_read_u*_array().
> - device_property_read_u*() are implemented using
> device_property_read_u*_array().
> - device_property_read_bool() is a new static inline wrapper around
> device_property_present().
>
> ---
> drivers/acpi/property.c | 178 +++
> drivers/base/Makefile|2
> drivers/base/property.c | 233
> +++
> drivers/of/base.c| 106 ++---
> include/linux/acpi.h | 32 ++
> include/linux/of.h | 22
> include/linux/property.h | 53 ++
> 7 files changed, 609 insertions(+), 17 deletions(-)
> create mode 100644 drivers/base/property.c
> create mode 100644 include/linux/property.h
>
> Index: linux-pm/include/linux/property.h
> ===
> --- /dev/null
> +++ linux-pm/include/linux/property.h
> @@ -0,0 +1,53 @@
> +/*
> + * property.h - Unified device property interface.
> + *
> + * Copyright (C) 2014, Intel Corporation
> + * Authors: Rafael J. Wysocki
> + * Mika Westerberg
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef _LINUX_PROPERTY_H_
> +#define _LINUX_PROPERTY_H_
> +
> +#include
> +
> +struct device;
> +
> +enum dev_prop_type {
> + DEV_PROP_U8,
> + DEV_PROP_U16,
> + DEV_PROP_U32,
> + DEV_PROP_U64,
> + DEV_PROP_STRING,
> + DEV_PROP_MAX,
> +};
> +
> +bool device_property_present(struct device *dev, const char *propname);
> +int device_property_read_u8_array(struct device *dev, const char *propname,
> + u8 *val, size_t nval);
> +int device_property_read_u16_array(struct device *dev, const char *propname,
> +u16 *val, size_t nval);
> +int device_property_read_u32_array(struct device *dev, const char *propname,
> +u32 *val, size_t nval);
> +int device_property_read_u64_array(struct device *dev, const char *propname,
> +u64 *val, size_t nval);
> +int device_property_read_string_array(struct device *dev, const char
> *propname,
> + char **val, size_t nval);
I'm not sure if I asked this elsewhere. Can 'val' be made a const char **?
g.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 00/12] Add ACPI _DSD and unified device properties support
On Sat, 25 Oct 2014 00:10:20 +0200
, "Rafael J. Wysocki"
wrote:
> On Tuesday, October 21, 2014 11:08:59 PM Rafael J. Wysocki wrote:
> > Hi Everyone,
> >
> > This is version 6 of the unified device properties interface patchset.
> >
> > The original cover letter from Mika is here:
> >
> > http://marc.info/?l=devicetree=141087052200600=4
> >
> > and my cover letters for previous iterations are at:
> >
> > http://marc.info/?l=linux-acpi=141212903816560=4
> > http://marc.info/?l=linux-kernel=141354745011569=4
> >
> > There are a few changes with respect to v5 and the affected patches are
> > [02-03/12] and [09-12/12]. The remaining ones have not been modified.
> >
> > Most importantly, requesting the first element of a list (package) property
> > from _DSD is now equivalent to accessing a single-value property of the
> > same type, so device_property_read_u8(dev, pname, val) will now be
> > equivalent
> > to device_property_read_u8_array(dev, pname, val, 1), for example.
> > Consequently, this _DSD definition:
> >
> > Name (_DSD, Package () {
> > ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > Package () {
> > Package () {"blah", "A string"},
> > }
> > })
> >
> > can be used instead of
> >
> > Name (_DSD, Package () {
> > ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> > Package () {
> > Package () {"blah", Package () {"A string"}},
> > }
> > })
> >
> > and the code will be able to retrieve the property value from the both of
> > them just fine.
> >
> > This means, among other things, that accessors for single-value properties
> > can be implemented in terms of the analogous "array" property accessors
> > which allows the code size to be reduced somewhat.
> >
> > Patches [02/12] and [09/12] have been modified to achieve that and patch
> > [03/12] have been modified accordingly for the "compatible" property in
> > _DSD to behave in an analogous way. Additionally, the bodies of the
> > numerical property accessors in patches [02/12] and [09/12] are now
> > generated using macros (string property accessors have slightly different
> > rules and are simply open coded for that reason).
> >
> > Patch [10/12] has been modified to drop function arguments that happened to
> > have the same values for both of the current users of those functions and
> > patches [11-12/12] have been modified to take that change into account. If
> > the code in question needs to be made more complex in the future, there
> > should not be any problems with that.
> >
> > Due to the nature of the changes I have retained all ACKs except for the
> > Grant's Reviewed-by on patch [03/12] (if that had been Acked-by, I would
> > have
> > retained it too, but that didn't feel appropriate for the "reviewed by"
> > thing
> > to me). If any of you think that the ACKs are not applicable any more,
> > please
> > let me know and I'll drop them.
> >
> > Finally, many thanks to Mika for testing the series on MinnowBoard 1 and
> > MinnowBoard Max. In case anybody else would like to test it, it is
> > available
> > from the device-properties branch of the linux-pm.git tree:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git
> > device-properties
> >
> > Thanks!
>
> Crickets ...
>
> OK, so I'm taking the lack of comments as the lack of objections and I'm
> already
> getting merge conflicts for this series. Moreover, we already have done some
> work on top of it.
>
> So, if there still are no comments by Sunday evening, I'll add this series to
> my linux-next branch with 3.19-rc1 as the target.
Aside from the comments I've made elsewhere, you can add my acked by for the
whole series.
Acked-by: Grant Likely
g.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 04/12] misc: at25: Make use of device property API
On Tue, Nov 4, 2014 at 2:38 PM, Mika Westerberg
wrote:
> On Tue, Nov 04, 2014 at 02:18:26PM +0000, Grant Likely wrote:
>> > - strncpy(chip->name, np->name, sizeof(chip->name));
>> > + strncpy(chip->name, "at25", sizeof(chip->name));
>>
>> This line changes behaviour of the driver. It's possibly not a problem,
>> but it should be commented on and whether any due diligance has been
>> done to make sure it there isn't anything that depends on it.
>
> I found only one user for "atmel,at25" in any of the DT sample files.
>
> arch/arm/boot/dts/phy3250.dts:
>
> eeprom: at25@0 {
> ...
> at25,byte-len = <0x8000>;
> at25,addr-mode = <2>;
> at25,page-size = <64>;
>
> compatible = "atmel,at25";
> reg = <0>;
> spi-max-frequency = <500>;
> };
>
> I think np->name is "at25" in this case? The binding file
> Documentation/devicetree/bindings/misc/at25.txt also has the same name.
>
> Are you OK, if we add something like below to the changelog?
>
> In addition we hard-code the name of the chip to be "at25" for the
> reason that there is no common mechanism to fetch name of the firmware
> node. The only existing user (arch/arm/boot/dts/phy3250.dts) uses the
> same name so it should continue to work.
Yes. If somebody complains, then we can reinstate the previous
behaviour, but assume it isn't necessary for now.
g.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Mon, Nov 3, 2014 at 9:52 PM, Rafael J. Wysocki wrote:
> On Monday, November 03, 2014 04:25:08 PM Rafael J. Wysocki wrote:
>> On Sunday, November 02, 2014 08:49:37 PM Darren Hart wrote:
>> >
>> > On 11/1/14 4:11, Grant Likely wrote:
>> > > On Tue, 28 Oct 2014 22:59:57 +0100
>> > > , "Rafael J. Wysocki"
>> > > wrote:
>> > >> On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
>> > >>> acpi_dev_add_driver_gpios() makes it possible to set up mapping between
>> > >>> properties and ACPI GpioIo resources in a driver, so we can take index
>> > >>> parameter in acpi_find_gpio() into use with _DSD device properties now.
>> > >>>
>> > >>> This index can be used to select a GPIO from a property with multiple
>> > >>> GPIOs:
>> > >>>
>> > >>> Package () {
>> > >>> "data-gpios",
>> > >>> Package () {
>> > >>> \_SB.GPIO, 0, 0, 0,
>> > >>> \_SB.GPIO, 1, 0, 0,
>> > >>> \_SB.GPIO, 2, 0, 1,
>> > >>> }
>> > >>> }
>> > >>>
>> > >>> In order to retrieve the last GPIO from a driver we can simply do:
>> > >>>
>> > >>> desc = devm_gpiod_get_index(dev, "data", 2);
>> > >>>
>> > >>> and so on.
>> > >>>
>> > >>> Signed-off-by: Mika Westerberg
>> > >>
>> > >> Cool. :-)
>> > >>
>> > >> Any objections anyone?
>> > >
>> > > Actually, I do. Not in the idea, but in the implementation. The way this
>> > > gets encoded:
>> > >
>> > > Package () {
>> > > \_SB.GPIO, 0, 0, 0,
>> > > \_SB.GPIO, 1, 0, 0,
>> > > \_SB.GPIO, 2, 0, 1,
>> > > }
>> > >
>> > > Means that decoding each GPIO tuple requires the length of a tuple to be
>> > > fixed, or to implement a DT-like #gpio-cells. If it is fixed, then there
>> > > is no way to expand the binding later. Can this be done in the following
>> > > way instead?
>> > >
>> > > Package () {
>> > > Package () { \_SB.GPIO, 0, 0, 0 },
>> > > Package () { \_SB.GPIO, 1, 0, 0 },
>> > > Package () { \_SB.GPIO, 2, 0, 1 },
>> > > }
>> > >
>> > > This is one of the biggest pains in device tree. We don't have any way
>> > > to group tuples so it requires looking up stuff across the tree to
>> > > figure out how to parse each multi-item property.
>> > >
>> > > I know that last year we talked about how bios vendors would get
>> > > complicated properties wrong, but I think there is little risk in this
>> > > case. If the property is encoded wrong, the driver simply won't work and
>> > > it is unlikely to get shipped before being fixed.
>> >
>> > This particular nesting of Packages is expressly prohibited by the
>> > Device Properties UUID for the reasons you mention.
>> >
>> > http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
>>
>> Also we don't use properties where single name is assigned to multiple GPIOs
>> anywhere in the current device-properties patchset, so this is not relevant
>> at
>> the moment.
>>
>> Moreover, even if we were to use them, we would need to ensure that this:
>>
>> Package () {
>> \_SB.GPIO, 0, 0, 0
>> }
>>
>> was equivalent to
>>
>> Package () {
>> Package () { \_SB.GPIO, 0, 0, 0 }
>> }
>>
>> This is not impossible to do and I suppose we could even explain that in the
>> implementation guide document in a sensible way, but that would require the
>> document linked above to be changed first and *then* we can think about
>> writing
>> kernel code to it. Not the other way around, please.
>>
>> So Grant, do you want us to proceed with that?
>
> Before you reply, one more observation that seems to be relevant.
>
> In ACPI, both this:
>
> Package () {
> \_SB.GPIO, 0, 0, 0,
> \_SB.GPIO, 1, 0, 0,
> \_SB.GPIO, 2, 0, 1,
> }
>
> and this:
>
>
Re: [PATCH v6 04/12] misc: at25: Make use of device property API
On Tue, 21 Oct 2014 23:19:56 +0200
, "Rafael J. Wysocki"
wrote:
> From: Mika Westerberg
>
> Make use of device property API in this driver so that both DT and ACPI
> based systems can use this driver.
>
> Signed-off-by: Mika Westerberg
> Signed-off-by: Rafael J. Wysocki
> ---
> drivers/misc/eeprom/at25.c | 34 +-
> 1 file changed, 13 insertions(+), 21 deletions(-)
>
> Index: linux-pm/drivers/misc/eeprom/at25.c
> ===
> --- linux-pm.orig/drivers/misc/eeprom/at25.c
> +++ linux-pm/drivers/misc/eeprom/at25.c
> @@ -18,7 +18,7 @@
>
> #include
> #include
> -#include
> +#include
>
> /*
> * NOTE: this is an *EEPROM* driver. The vagaries of product naming
> @@ -301,35 +301,33 @@ static ssize_t at25_mem_write(struct mem
>
> /*-*/
>
> -static int at25_np_to_chip(struct device *dev,
> -struct device_node *np,
> -struct spi_eeprom *chip)
> +static int at25_fw_to_chip(struct device *dev, struct spi_eeprom *chip)
> {
> u32 val;
>
> memset(chip, 0, sizeof(*chip));
> - strncpy(chip->name, np->name, sizeof(chip->name));
> + strncpy(chip->name, "at25", sizeof(chip->name));
This line changes behaviour of the driver. It's possibly not a problem,
but it should be commented on and whether any due diligance has been
done to make sure it there isn't anything that depends on it.
Otherwise:
Acked-by: Grant Likely
>
> - if (of_property_read_u32(np, "size", ) == 0 ||
> - of_property_read_u32(np, "at25,byte-len", ) == 0) {
> + if (device_property_read_u32(dev, "size", ) == 0 ||
> + device_property_read_u32(dev, "at25,byte-len", ) == 0) {
> chip->byte_len = val;
> } else {
> dev_err(dev, "Error: missing \"size\" property\n");
> return -ENODEV;
> }
>
> - if (of_property_read_u32(np, "pagesize", ) == 0 ||
> - of_property_read_u32(np, "at25,page-size", ) == 0) {
> + if (device_property_read_u32(dev, "pagesize", ) == 0 ||
> + device_property_read_u32(dev, "at25,page-size", ) == 0) {
> chip->page_size = (u16)val;
> } else {
> dev_err(dev, "Error: missing \"pagesize\" property\n");
> return -ENODEV;
> }
>
> - if (of_property_read_u32(np, "at25,addr-mode", ) == 0) {
> + if (device_property_read_u32(dev, "at25,addr-mode", ) == 0) {
> chip->flags = (u16)val;
> } else {
> - if (of_property_read_u32(np, "address-width", )) {
> + if (device_property_read_u32(dev, "address-width", )) {
> dev_err(dev,
> "Error: missing \"address-width\" property\n");
> return -ENODEV;
> @@ -350,7 +348,7 @@ static int at25_np_to_chip(struct device
> val);
> return -ENODEV;
> }
> - if (of_find_property(np, "read-only", NULL))
> + if (device_property_present(dev, "read-only"))
> chip->flags |= EE_READONLY;
> }
> return 0;
> @@ -360,21 +358,15 @@ static int at25_probe(struct spi_device
> {
> struct at25_data*at25 = NULL;
> struct spi_eeprom chip;
> - struct device_node *np = spi->dev.of_node;
> int err;
> int sr;
> int addrlen;
>
> /* Chip description */
> if (!spi->dev.platform_data) {
> - if (np) {
> - err = at25_np_to_chip(>dev, np, );
> - if (err)
> - return err;
> - } else {
> - dev_err(>dev, "Error: no chip description\n");
> - return -ENODEV;
> - }
> + err = at25_fw_to_chip(>dev, );
> + if (err)
> + return err;
> } else
> chip = *(struct spi_eeprom *)spi->dev.platform_data;
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 1/2] of/unittest: Remove test devices after adding them
The of_platform_populate() test cases don't remove the test devices
after they are added. Fix this by adding tests for
of_platform_depopulate().
At the same time rework the selftest() macro to return the test result
value. This makes it easy to use the macro inside an if() condition.
Signed-off-by: Grant Likely
---
drivers/of/selftest.c | 39 ---
1 file changed, 28 insertions(+), 11 deletions(-)
diff --git a/drivers/of/selftest.c b/drivers/of/selftest.c
index bf7d99317a94..082bb2b6a5ad 100644
--- a/drivers/of/selftest.c
+++ b/drivers/of/selftest.c
@@ -30,15 +30,17 @@ static struct device_node *nodes[NO_OF_NODES];
static int last_node_index;
static bool selftest_live_tree;
-#define selftest(result, fmt, ...) { \
- if (!(result)) { \
+#define selftest(result, fmt, ...) ({ \
+ bool failed = !(result); \
+ if (failed) { \
selftest_results.failed++; \
pr_err("FAIL %s():%i " fmt, __func__, __LINE__, ##__VA_ARGS__);
\
} else { \
selftest_results.passed++; \
pr_debug("pass %s():%i\n", __func__, __LINE__); \
} \
-}
+ failed; \
+})
static void __init of_selftest_find_node_by_name(void)
{
@@ -694,10 +696,13 @@ static void __init of_selftest_match_node(void)
}
}
+struct device test_bus = {
+ .init_name = "unittest-bus",
+};
static void __init of_selftest_platform_populate(void)
{
- int irq;
- struct device_node *np, *child;
+ int irq, rc;
+ struct device_node *np, *child, *grandchild;
struct platform_device *pdev;
struct of_device_id match[] = {
{ .compatible = "test-device", },
@@ -722,20 +727,32 @@ static void __init of_selftest_platform_populate(void)
irq = platform_get_irq(pdev, 0);
selftest(irq < 0 && irq != -EPROBE_DEFER, "device parsing error failed
- %d\n", irq);
- np = of_find_node_by_path("/testcase-data/platform-tests");
- if (!np) {
- pr_err("No testcase data in device tree\n");
+ if (selftest(np = of_find_node_by_path("/testcase-data/platform-tests"),
+"No testcase data in device tree\n"));
+ return;
+
+ if (selftest(!(rc = device_register(_bus)),
+"testbus registration failed; rc=%i\n", rc));
return;
- }
for_each_child_of_node(np, child) {
- struct device_node *grandchild;
- of_platform_populate(child, match, NULL, NULL);
+ of_platform_populate(child, match, NULL, _bus);
for_each_child_of_node(child, grandchild)
selftest(of_find_device_by_node(grandchild),
"Could not create device for node '%s'\n",
grandchild->name);
}
+
+ of_platform_depopulate(_bus);
+ for_each_child_of_node(np, child) {
+ for_each_child_of_node(child, grandchild)
+ selftest(!of_find_device_by_node(grandchild),
+"device didn't get destroyed '%s'\n",
+grandchild->name);
+ }
+
+ device_unregister(_bus);
+ of_node_put(np);
}
/**
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] of/unittest: Rename selftest.c to unittest.c
This is unit testing code. It should use that name because it makes more
sense than 'selftest'. Rename the files to match and rename the config
variable.
Signed-off-by: Grant Likely
---
drivers/of/Kconfig | 4 +-
drivers/of/Makefile| 4 +-
drivers/of/selftest.c | 969 -
drivers/of/testcase-data/testcases.dts | 50 --
drivers/of/testcase-data/tests-interrupts.dtsi | 71 --
drivers/of/testcase-data/tests-match.dtsi | 19 -
drivers/of/testcase-data/tests-phandle.dtsi| 48 --
drivers/of/testcase-data/tests-platform.dtsi | 35 -
drivers/of/unittest-data/testcases.dts | 50 ++
drivers/of/unittest-data/tests-interrupts.dtsi | 71 ++
drivers/of/unittest-data/tests-match.dtsi | 19 +
drivers/of/unittest-data/tests-phandle.dtsi| 48 ++
drivers/of/unittest-data/tests-platform.dtsi | 35 +
drivers/of/unittest.c | 969 +
14 files changed, 1196 insertions(+), 1196 deletions(-)
delete mode 100644 drivers/of/selftest.c
delete mode 100644 drivers/of/testcase-data/testcases.dts
delete mode 100644 drivers/of/testcase-data/tests-interrupts.dtsi
delete mode 100644 drivers/of/testcase-data/tests-match.dtsi
delete mode 100644 drivers/of/testcase-data/tests-phandle.dtsi
delete mode 100644 drivers/of/testcase-data/tests-platform.dtsi
create mode 100644 drivers/of/unittest-data/testcases.dts
create mode 100644 drivers/of/unittest-data/tests-interrupts.dtsi
create mode 100644 drivers/of/unittest-data/tests-match.dtsi
create mode 100644 drivers/of/unittest-data/tests-phandle.dtsi
create mode 100644 drivers/of/unittest-data/tests-platform.dtsi
create mode 100644 drivers/of/unittest.c
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 1a13f5b722c5..be16ce2ffd69 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -7,8 +7,8 @@ config OF
menu "Device Tree and Open Firmware support"
depends on OF
-config OF_SELFTEST
- bool "Device Tree Runtime self tests"
+config OF_UNITTEST
+ bool "Device Tree runtime unit tests"
depends on OF_IRQ && OF_EARLY_FLATTREE
select OF_DYNAMIC
select OF_RESOLVE
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index ca9209ce50cd..d90553fcd37f 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -6,8 +6,8 @@ obj-$(CONFIG_OF_PROMTREE) += pdt.o
obj-$(CONFIG_OF_ADDRESS) += address.o
obj-$(CONFIG_OF_IRQ)+= irq.o
obj-$(CONFIG_OF_NET) += of_net.o
-obj-$(CONFIG_OF_SELFTEST) += of_selftest.o
-of_selftest-objs := selftest.o testcase-data/testcases.dtb.o
+obj-$(CONFIG_OF_UNITTEST) += of_unittest.o
+of_unittest-objs := unittest.o unittest-data/testcases.dtb.o
obj-$(CONFIG_OF_MDIO) += of_mdio.o
obj-$(CONFIG_OF_PCI) += of_pci.o
obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o
diff --git a/drivers/of/selftest.c b/drivers/of/selftest.c
deleted file mode 100644
index 082bb2b6a5ad..
--- a/drivers/of/selftest.c
+++ /dev/null
@@ -1,969 +0,0 @@
-/*
- * Self tests for device tree subsystem
- */
-
-#define pr_fmt(fmt) "### dt-test ### " fmt
-
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-#include
-
-#include "of_private.h"
-
-static struct selftest_results {
- int passed;
- int failed;
-} selftest_results;
-
-#define NO_OF_NODES 3
-static struct device_node *nodes[NO_OF_NODES];
-static int last_node_index;
-static bool selftest_live_tree;
-
-#define selftest(result, fmt, ...) ({ \
- bool failed = !(result); \
- if (failed) { \
- selftest_results.failed++; \
- pr_err("FAIL %s():%i " fmt, __func__, __LINE__, ##__VA_ARGS__);
\
- } else { \
- selftest_results.passed++; \
- pr_debug("pass %s():%i\n", __func__, __LINE__); \
- } \
- failed; \
-})
-
-static void __init of_selftest_find_node_by_name(void)
-{
- struct device_node *np;
-
- np = of_find_node_by_path("/testcase-data");
- selftest(np && !strcmp("/testcase-data", np->full_name),
- "find /testcase-data failed\n");
- of_node_put(np);
-
- /* Test if trailing '/' works */
- np = of_find_node_by_path("/testcase-data/");
- selftest(!np, "trailing '/' on /testcase-data/ should fail\n");
-
- np = of_find_node_by_path("/testcase-data/phandle-tests/consumer-a");
- selftest(np && !strcmp("/testcase-data/phandle-tests/consumer-a",
np->full_name),
- "find /testcase-data/phandle-tests/consumer-a failed\n");
- of_node_put(np);
-
- np = of_find_node_by_path("testcase-alias");
- selftest(np && !strcmp("/
Re: [PATCH] of: Fix overflow bug in string property parsing functions
On Tue, 04 Nov 2014 01:43:24 +0100
, "Rafael J. Wysocki"
wrote:
> On Monday, November 03, 2014 11:38:24 PM Grant Likely wrote:
>
> [cut]
>
> > diff --git a/include/linux/of.h b/include/linux/of.h
> > index 6545e7aec7bb..3b3c6e849ae8 100644
> > --- a/include/linux/of.h
> > +++ b/include/linux/of.h
> > @@ -267,14 +267,12 @@ extern int of_property_read_u64(const struct
> > device_node *np,
> > extern int of_property_read_string(struct device_node *np,
> >const char *propname,
> >const char **out_string);
> > -extern int of_property_read_string_index(struct device_node *np,
> > -const char *propname,
> > -int index, const char **output);
> > extern int of_property_match_string(struct device_node *np,
> > const char *propname,
> > const char *string);
> > -extern int of_property_count_strings(struct device_node *np,
> > -const char *propname);
> > +extern int of_property_read_string_helper(struct device_node *np,
> > + const char *propname,
> > + const char **out_strs, size_t sz,
> > int index);
> > extern int of_device_is_compatible(const struct device_node *device,
> >const char *);
> > extern int of_device_is_available(const struct device_node *device);
> > @@ -486,15 +484,9 @@ static inline int of_property_read_string(struct
> > device_node *np,
> > return -ENOSYS;
> > }
> >
> > -static inline int of_property_read_string_index(struct device_node *np,
> > - const char *propname, int index,
> > - const char **out_string)
> > -{
> > - return -ENOSYS;
> > -}
> > -
> > -static inline int of_property_count_strings(struct device_node *np,
> > - const char *propname)
> > +static inline int of_property_read_string_helper(struct device_node *np,
> > +const char *propname,
> > +char **out_strs, size_t sz,
> > int index)
>
> const char **out_strs, size_t sz, int index)
>
> > {
> > return -ENOSYS;
> > }
>
> Other than that, looks good to me.
Fixed, thanks.
g.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] of/platform: Move platform devices under /sys/devices/platform
Currently the devices created by drivers/of/platform.c get created at the root of /sys/devices. This goes against the typical pattern for sysfs where the top level /sys/devices structure contains categories of devices, and the structure of devices is placed below that. To fix this, make the code in drivers/of/platform.c follow the drivers/base/platform.c behaviour, and use _bus as the default parent for all new platform_devices and amba_devices. This change has been discussed for a long time, but nobody has actually acted on it. Userspace code that expects to find devices under a fixed /sys/devices/... path will be affected. It isn't /supposed/ to do that, but if anyone complains then I'll add a default-off workaround option to put them back into the root. Signed-off-by: Grant Likely Cc: Rob Herring Cc: Arnd Bergmann Cc: Greg Kroah-Hartman Cc: Benjamin Herrenschmidt --- drivers/of/platform.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/of/platform.c b/drivers/of/platform.c index 3b64d0bf5bba..7c6771986c06 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -138,7 +138,7 @@ struct platform_device *of_device_alloc(struct device_node *np, } dev->dev.of_node = of_node_get(np); - dev->dev.parent = parent; + dev->dev.parent = parent ? : _bus; if (bus_id) dev_set_name(>dev, "%s", bus_id); @@ -291,7 +291,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.parent = parent; + dev->dev.parent = parent ? : _bus; dev->dev.platform_data = platform_data; if (bus_id) dev_set_name(>dev, "%s", bus_id); -- 1.9.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] of/platform: Move platform devices under /sys/devices/platform
Currently the devices created by drivers/of/platform.c get created at the root of /sys/devices. This goes against the typical pattern for sysfs where the top level /sys/devices structure contains categories of devices, and the structure of devices is placed below that. To fix this, make the code in drivers/of/platform.c follow the drivers/base/platform.c behaviour, and use platform_bus as the default parent for all new platform_devices and amba_devices. This change has been discussed for a long time, but nobody has actually acted on it. Userspace code that expects to find devices under a fixed /sys/devices/... path will be affected. It isn't /supposed/ to do that, but if anyone complains then I'll add a default-off workaround option to put them back into the root. Signed-off-by: Grant Likely grant.lik...@linaro.org Cc: Rob Herring robh...@kernel.org Cc: Arnd Bergmann a...@arndb.de Cc: Greg Kroah-Hartman gre...@linuxfoundation.org Cc: Benjamin Herrenschmidt b...@kernel.crashing.org --- drivers/of/platform.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/of/platform.c b/drivers/of/platform.c index 3b64d0bf5bba..7c6771986c06 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -138,7 +138,7 @@ struct platform_device *of_device_alloc(struct device_node *np, } dev-dev.of_node = of_node_get(np); - dev-dev.parent = parent; + dev-dev.parent = parent ? : platform_bus; if (bus_id) dev_set_name(dev-dev, %s, bus_id); @@ -291,7 +291,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.parent = parent; + dev-dev.parent = parent ? : platform_bus; dev-dev.platform_data = platform_data; if (bus_id) dev_set_name(dev-dev, %s, bus_id); -- 1.9.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: Fix overflow bug in string property parsing functions
On Tue, 04 Nov 2014 01:43:24 +0100
, Rafael J. Wysocki r...@rjwysocki.net
wrote:
On Monday, November 03, 2014 11:38:24 PM Grant Likely wrote:
[cut]
diff --git a/include/linux/of.h b/include/linux/of.h
index 6545e7aec7bb..3b3c6e849ae8 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -267,14 +267,12 @@ extern int of_property_read_u64(const struct
device_node *np,
extern int of_property_read_string(struct device_node *np,
const char *propname,
const char **out_string);
-extern int of_property_read_string_index(struct device_node *np,
-const char *propname,
-int index, const char **output);
extern int of_property_match_string(struct device_node *np,
const char *propname,
const char *string);
-extern int of_property_count_strings(struct device_node *np,
-const char *propname);
+extern int of_property_read_string_helper(struct device_node *np,
+ const char *propname,
+ const char **out_strs, size_t sz,
int index);
extern int of_device_is_compatible(const struct device_node *device,
const char *);
extern int of_device_is_available(const struct device_node *device);
@@ -486,15 +484,9 @@ static inline int of_property_read_string(struct
device_node *np,
return -ENOSYS;
}
-static inline int of_property_read_string_index(struct device_node *np,
- const char *propname, int index,
- const char **out_string)
-{
- return -ENOSYS;
-}
-
-static inline int of_property_count_strings(struct device_node *np,
- const char *propname)
+static inline int of_property_read_string_helper(struct device_node *np,
+const char *propname,
+char **out_strs, size_t sz,
int index)
const char **out_strs, size_t sz, int index)
{
return -ENOSYS;
}
Other than that, looks good to me.
Fixed, thanks.
g.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH 2/2] of/unittest: Rename selftest.c to unittest.c
This is unit testing code. It should use that name because it makes more
sense than 'selftest'. Rename the files to match and rename the config
variable.
Signed-off-by: Grant Likely grant.lik...@linaro.org
---
drivers/of/Kconfig | 4 +-
drivers/of/Makefile| 4 +-
drivers/of/selftest.c | 969 -
drivers/of/testcase-data/testcases.dts | 50 --
drivers/of/testcase-data/tests-interrupts.dtsi | 71 --
drivers/of/testcase-data/tests-match.dtsi | 19 -
drivers/of/testcase-data/tests-phandle.dtsi| 48 --
drivers/of/testcase-data/tests-platform.dtsi | 35 -
drivers/of/unittest-data/testcases.dts | 50 ++
drivers/of/unittest-data/tests-interrupts.dtsi | 71 ++
drivers/of/unittest-data/tests-match.dtsi | 19 +
drivers/of/unittest-data/tests-phandle.dtsi| 48 ++
drivers/of/unittest-data/tests-platform.dtsi | 35 +
drivers/of/unittest.c | 969 +
14 files changed, 1196 insertions(+), 1196 deletions(-)
delete mode 100644 drivers/of/selftest.c
delete mode 100644 drivers/of/testcase-data/testcases.dts
delete mode 100644 drivers/of/testcase-data/tests-interrupts.dtsi
delete mode 100644 drivers/of/testcase-data/tests-match.dtsi
delete mode 100644 drivers/of/testcase-data/tests-phandle.dtsi
delete mode 100644 drivers/of/testcase-data/tests-platform.dtsi
create mode 100644 drivers/of/unittest-data/testcases.dts
create mode 100644 drivers/of/unittest-data/tests-interrupts.dtsi
create mode 100644 drivers/of/unittest-data/tests-match.dtsi
create mode 100644 drivers/of/unittest-data/tests-phandle.dtsi
create mode 100644 drivers/of/unittest-data/tests-platform.dtsi
create mode 100644 drivers/of/unittest.c
diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index 1a13f5b722c5..be16ce2ffd69 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -7,8 +7,8 @@ config OF
menu Device Tree and Open Firmware support
depends on OF
-config OF_SELFTEST
- bool Device Tree Runtime self tests
+config OF_UNITTEST
+ bool Device Tree runtime unit tests
depends on OF_IRQ OF_EARLY_FLATTREE
select OF_DYNAMIC
select OF_RESOLVE
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index ca9209ce50cd..d90553fcd37f 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -6,8 +6,8 @@ obj-$(CONFIG_OF_PROMTREE) += pdt.o
obj-$(CONFIG_OF_ADDRESS) += address.o
obj-$(CONFIG_OF_IRQ)+= irq.o
obj-$(CONFIG_OF_NET) += of_net.o
-obj-$(CONFIG_OF_SELFTEST) += of_selftest.o
-of_selftest-objs := selftest.o testcase-data/testcases.dtb.o
+obj-$(CONFIG_OF_UNITTEST) += of_unittest.o
+of_unittest-objs := unittest.o unittest-data/testcases.dtb.o
obj-$(CONFIG_OF_MDIO) += of_mdio.o
obj-$(CONFIG_OF_PCI) += of_pci.o
obj-$(CONFIG_OF_PCI_IRQ) += of_pci_irq.o
diff --git a/drivers/of/selftest.c b/drivers/of/selftest.c
deleted file mode 100644
index 082bb2b6a5ad..
--- a/drivers/of/selftest.c
+++ /dev/null
@@ -1,969 +0,0 @@
-/*
- * Self tests for device tree subsystem
- */
-
-#define pr_fmt(fmt) ### dt-test ### fmt
-
-#include linux/clk.h
-#include linux/err.h
-#include linux/errno.h
-#include linux/hashtable.h
-#include linux/module.h
-#include linux/of.h
-#include linux/of_fdt.h
-#include linux/of_irq.h
-#include linux/of_platform.h
-#include linux/list.h
-#include linux/mutex.h
-#include linux/slab.h
-#include linux/device.h
-
-#include of_private.h
-
-static struct selftest_results {
- int passed;
- int failed;
-} selftest_results;
-
-#define NO_OF_NODES 3
-static struct device_node *nodes[NO_OF_NODES];
-static int last_node_index;
-static bool selftest_live_tree;
-
-#define selftest(result, fmt, ...) ({ \
- bool failed = !(result); \
- if (failed) { \
- selftest_results.failed++; \
- pr_err(FAIL %s():%i fmt, __func__, __LINE__, ##__VA_ARGS__);
\
- } else { \
- selftest_results.passed++; \
- pr_debug(pass %s():%i\n, __func__, __LINE__); \
- } \
- failed; \
-})
-
-static void __init of_selftest_find_node_by_name(void)
-{
- struct device_node *np;
-
- np = of_find_node_by_path(/testcase-data);
- selftest(np !strcmp(/testcase-data, np-full_name),
- find /testcase-data failed\n);
- of_node_put(np);
-
- /* Test if trailing '/' works */
- np = of_find_node_by_path(/testcase-data/);
- selftest(!np, trailing '/' on /testcase-data/ should fail\n);
-
- np = of_find_node_by_path(/testcase-data/phandle-tests/consumer-a);
- selftest(np !strcmp(/testcase-data/phandle-tests/consumer-a,
np-full_name),
- find /testcase-data/phandle-tests/consumer-a failed\n);
- of_node_put(np);
-
- np = of_find_node_by_path(testcase-alias);
- selftest(np !strcmp(/testcase-data, np-full_name
[PATCH 1/2] of/unittest: Remove test devices after adding them
The of_platform_populate() test cases don't remove the test devices
after they are added. Fix this by adding tests for
of_platform_depopulate().
At the same time rework the selftest() macro to return the test result
value. This makes it easy to use the macro inside an if() condition.
Signed-off-by: Grant Likely grant.lik...@linaro.org
---
drivers/of/selftest.c | 39 ---
1 file changed, 28 insertions(+), 11 deletions(-)
diff --git a/drivers/of/selftest.c b/drivers/of/selftest.c
index bf7d99317a94..082bb2b6a5ad 100644
--- a/drivers/of/selftest.c
+++ b/drivers/of/selftest.c
@@ -30,15 +30,17 @@ static struct device_node *nodes[NO_OF_NODES];
static int last_node_index;
static bool selftest_live_tree;
-#define selftest(result, fmt, ...) { \
- if (!(result)) { \
+#define selftest(result, fmt, ...) ({ \
+ bool failed = !(result); \
+ if (failed) { \
selftest_results.failed++; \
pr_err(FAIL %s():%i fmt, __func__, __LINE__, ##__VA_ARGS__);
\
} else { \
selftest_results.passed++; \
pr_debug(pass %s():%i\n, __func__, __LINE__); \
} \
-}
+ failed; \
+})
static void __init of_selftest_find_node_by_name(void)
{
@@ -694,10 +696,13 @@ static void __init of_selftest_match_node(void)
}
}
+struct device test_bus = {
+ .init_name = unittest-bus,
+};
static void __init of_selftest_platform_populate(void)
{
- int irq;
- struct device_node *np, *child;
+ int irq, rc;
+ struct device_node *np, *child, *grandchild;
struct platform_device *pdev;
struct of_device_id match[] = {
{ .compatible = test-device, },
@@ -722,20 +727,32 @@ static void __init of_selftest_platform_populate(void)
irq = platform_get_irq(pdev, 0);
selftest(irq 0 irq != -EPROBE_DEFER, device parsing error failed
- %d\n, irq);
- np = of_find_node_by_path(/testcase-data/platform-tests);
- if (!np) {
- pr_err(No testcase data in device tree\n);
+ if (selftest(np = of_find_node_by_path(/testcase-data/platform-tests),
+No testcase data in device tree\n));
+ return;
+
+ if (selftest(!(rc = device_register(test_bus)),
+testbus registration failed; rc=%i\n, rc));
return;
- }
for_each_child_of_node(np, child) {
- struct device_node *grandchild;
- of_platform_populate(child, match, NULL, NULL);
+ of_platform_populate(child, match, NULL, test_bus);
for_each_child_of_node(child, grandchild)
selftest(of_find_device_by_node(grandchild),
Could not create device for node '%s'\n,
grandchild-name);
}
+
+ of_platform_depopulate(test_bus);
+ for_each_child_of_node(np, child) {
+ for_each_child_of_node(child, grandchild)
+ selftest(!of_find_device_by_node(grandchild),
+device didn't get destroyed '%s'\n,
+grandchild-name);
+ }
+
+ device_unregister(test_bus);
+ of_node_put(np);
}
/**
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 04/12] misc: at25: Make use of device property API
On Tue, 21 Oct 2014 23:19:56 +0200
, Rafael J. Wysocki r...@rjwysocki.net
wrote:
From: Mika Westerberg mika.westerb...@linux.intel.com
Make use of device property API in this driver so that both DT and ACPI
based systems can use this driver.
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
drivers/misc/eeprom/at25.c | 34 +-
1 file changed, 13 insertions(+), 21 deletions(-)
Index: linux-pm/drivers/misc/eeprom/at25.c
===
--- linux-pm.orig/drivers/misc/eeprom/at25.c
+++ linux-pm/drivers/misc/eeprom/at25.c
@@ -18,7 +18,7 @@
#include linux/spi/spi.h
#include linux/spi/eeprom.h
-#include linux/of.h
+#include linux/property.h
/*
* NOTE: this is an *EEPROM* driver. The vagaries of product naming
@@ -301,35 +301,33 @@ static ssize_t at25_mem_write(struct mem
/*-*/
-static int at25_np_to_chip(struct device *dev,
-struct device_node *np,
-struct spi_eeprom *chip)
+static int at25_fw_to_chip(struct device *dev, struct spi_eeprom *chip)
{
u32 val;
memset(chip, 0, sizeof(*chip));
- strncpy(chip-name, np-name, sizeof(chip-name));
+ strncpy(chip-name, at25, sizeof(chip-name));
This line changes behaviour of the driver. It's possibly not a problem,
but it should be commented on and whether any due diligance has been
done to make sure it there isn't anything that depends on it.
Otherwise:
Acked-by: Grant Likely grant.lik...@linaro.org
- if (of_property_read_u32(np, size, val) == 0 ||
- of_property_read_u32(np, at25,byte-len, val) == 0) {
+ if (device_property_read_u32(dev, size, val) == 0 ||
+ device_property_read_u32(dev, at25,byte-len, val) == 0) {
chip-byte_len = val;
} else {
dev_err(dev, Error: missing \size\ property\n);
return -ENODEV;
}
- if (of_property_read_u32(np, pagesize, val) == 0 ||
- of_property_read_u32(np, at25,page-size, val) == 0) {
+ if (device_property_read_u32(dev, pagesize, val) == 0 ||
+ device_property_read_u32(dev, at25,page-size, val) == 0) {
chip-page_size = (u16)val;
} else {
dev_err(dev, Error: missing \pagesize\ property\n);
return -ENODEV;
}
- if (of_property_read_u32(np, at25,addr-mode, val) == 0) {
+ if (device_property_read_u32(dev, at25,addr-mode, val) == 0) {
chip-flags = (u16)val;
} else {
- if (of_property_read_u32(np, address-width, val)) {
+ if (device_property_read_u32(dev, address-width, val)) {
dev_err(dev,
Error: missing \address-width\ property\n);
return -ENODEV;
@@ -350,7 +348,7 @@ static int at25_np_to_chip(struct device
val);
return -ENODEV;
}
- if (of_find_property(np, read-only, NULL))
+ if (device_property_present(dev, read-only))
chip-flags |= EE_READONLY;
}
return 0;
@@ -360,21 +358,15 @@ static int at25_probe(struct spi_device
{
struct at25_data*at25 = NULL;
struct spi_eeprom chip;
- struct device_node *np = spi-dev.of_node;
int err;
int sr;
int addrlen;
/* Chip description */
if (!spi-dev.platform_data) {
- if (np) {
- err = at25_np_to_chip(spi-dev, np, chip);
- if (err)
- return err;
- } else {
- dev_err(spi-dev, Error: no chip description\n);
- return -ENODEV;
- }
+ err = at25_fw_to_chip(spi-dev, chip);
+ if (err)
+ return err;
} else
chip = *(struct spi_eeprom *)spi-dev.platform_data;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Mon, Nov 3, 2014 at 9:52 PM, Rafael J. Wysocki r...@rjwysocki.net wrote:
On Monday, November 03, 2014 04:25:08 PM Rafael J. Wysocki wrote:
On Sunday, November 02, 2014 08:49:37 PM Darren Hart wrote:
On 11/1/14 4:11, Grant Likely wrote:
On Tue, 28 Oct 2014 22:59:57 +0100
, Rafael J. Wysocki r...@rjwysocki.net
wrote:
On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
acpi_dev_add_driver_gpios() makes it possible to set up mapping between
properties and ACPI GpioIo resources in a driver, so we can take index
parameter in acpi_find_gpio() into use with _DSD device properties now.
This index can be used to select a GPIO from a property with multiple
GPIOs:
Package () {
data-gpios,
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
}
In order to retrieve the last GPIO from a driver we can simply do:
desc = devm_gpiod_get_index(dev, data, 2);
and so on.
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Cool. :-)
Any objections anyone?
Actually, I do. Not in the idea, but in the implementation. The way this
gets encoded:
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
Means that decoding each GPIO tuple requires the length of a tuple to be
fixed, or to implement a DT-like #gpio-cells. If it is fixed, then there
is no way to expand the binding later. Can this be done in the following
way instead?
Package () {
Package () { \_SB.GPIO, 0, 0, 0 },
Package () { \_SB.GPIO, 1, 0, 0 },
Package () { \_SB.GPIO, 2, 0, 1 },
}
This is one of the biggest pains in device tree. We don't have any way
to group tuples so it requires looking up stuff across the tree to
figure out how to parse each multi-item property.
I know that last year we talked about how bios vendors would get
complicated properties wrong, but I think there is little risk in this
case. If the property is encoded wrong, the driver simply won't work and
it is unlikely to get shipped before being fixed.
This particular nesting of Packages is expressly prohibited by the
Device Properties UUID for the reasons you mention.
http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
Also we don't use properties where single name is assigned to multiple GPIOs
anywhere in the current device-properties patchset, so this is not relevant
at
the moment.
Moreover, even if we were to use them, we would need to ensure that this:
Package () {
\_SB.GPIO, 0, 0, 0
}
was equivalent to
Package () {
Package () { \_SB.GPIO, 0, 0, 0 }
}
This is not impossible to do and I suppose we could even explain that in the
implementation guide document in a sensible way, but that would require the
document linked above to be changed first and *then* we can think about
writing
kernel code to it. Not the other way around, please.
So Grant, do you want us to proceed with that?
Before you reply, one more observation that seems to be relevant.
In ACPI, both this:
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
and this:
Package () {
Package () { \_SB.GPIO, 0, 0, 0 },
Package () { \_SB.GPIO, 1, 0, 0 },
Package () { \_SB.GPIO, 2, 0, 1 },
}
carry the same information, because every element of a package has a type,
so there is no danger of confusing an ACPI_TYPE_LOCAL_REFERENCE with
ACPI_TYPE_INTEGER. Thus one can easily count the number of GPIOs represented
by the first package by counting the number of reference elements in it.
The second one has more structure which in this particular case is arguably
redundant.
Okay, this make sense. I'm okay with this approach, and I would
recommend making that the only valid method for parsing in
acpi_dev_get_property_reference(). Get rid of the *size_prop argument
so that it always behaves the same way and users aren't tempted to do
something clever.
Of course, that's not the case for list properties where each item consists
of a bunch of integers, like
Package () {
Package () { 0, 0, 0 },
Package () { 1, 0, 0 },
Package () { 2, 0, 1 },
}
but I'm not sure if this is relevant at all.
Probably not. With a pure list it isn't implicitly referencing data in
another node. In the ref+args pattern the length of each tuple can
vary based on which node it references, but on a simple list the
parsing is going to be a lot simpler.
g.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel
Re: [PATCH v6 04/12] misc: at25: Make use of device property API
On Tue, Nov 4, 2014 at 2:38 PM, Mika Westerberg
mika.westerb...@linux.intel.com wrote:
On Tue, Nov 04, 2014 at 02:18:26PM +, Grant Likely wrote:
- strncpy(chip-name, np-name, sizeof(chip-name));
+ strncpy(chip-name, at25, sizeof(chip-name));
This line changes behaviour of the driver. It's possibly not a problem,
but it should be commented on and whether any due diligance has been
done to make sure it there isn't anything that depends on it.
I found only one user for atmel,at25 in any of the DT sample files.
arch/arm/boot/dts/phy3250.dts:
eeprom: at25@0 {
...
at25,byte-len = 0x8000;
at25,addr-mode = 2;
at25,page-size = 64;
compatible = atmel,at25;
reg = 0;
spi-max-frequency = 500;
};
I think np-name is at25 in this case? The binding file
Documentation/devicetree/bindings/misc/at25.txt also has the same name.
Are you OK, if we add something like below to the changelog?
In addition we hard-code the name of the chip to be at25 for the
reason that there is no common mechanism to fetch name of the firmware
node. The only existing user (arch/arm/boot/dts/phy3250.dts) uses the
same name so it should continue to work.
Yes. If somebody complains, then we can reinstate the previous
behaviour, but assume it isn't necessary for now.
g.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 00/12] Add ACPI _DSD and unified device properties support
On Sat, 25 Oct 2014 00:10:20 +0200
, Rafael J. Wysocki r...@rjwysocki.net
wrote:
On Tuesday, October 21, 2014 11:08:59 PM Rafael J. Wysocki wrote:
Hi Everyone,
This is version 6 of the unified device properties interface patchset.
The original cover letter from Mika is here:
http://marc.info/?l=devicetreem=141087052200600w=4
and my cover letters for previous iterations are at:
http://marc.info/?l=linux-acpim=141212903816560w=4
http://marc.info/?l=linux-kernelm=141354745011569w=4
There are a few changes with respect to v5 and the affected patches are
[02-03/12] and [09-12/12]. The remaining ones have not been modified.
Most importantly, requesting the first element of a list (package) property
from _DSD is now equivalent to accessing a single-value property of the
same type, so device_property_read_u8(dev, pname, val) will now be
equivalent
to device_property_read_u8_array(dev, pname, val, 1), for example.
Consequently, this _DSD definition:
Name (_DSD, Package () {
ToUUID(daffd814-6eba-4d8c-8a91-bc9bbf4aa301),
Package () {
Package () {blah, A string},
}
})
can be used instead of
Name (_DSD, Package () {
ToUUID(daffd814-6eba-4d8c-8a91-bc9bbf4aa301),
Package () {
Package () {blah, Package () {A string}},
}
})
and the code will be able to retrieve the property value from the both of
them just fine.
This means, among other things, that accessors for single-value properties
can be implemented in terms of the analogous array property accessors
which allows the code size to be reduced somewhat.
Patches [02/12] and [09/12] have been modified to achieve that and patch
[03/12] have been modified accordingly for the compatible property in
_DSD to behave in an analogous way. Additionally, the bodies of the
numerical property accessors in patches [02/12] and [09/12] are now
generated using macros (string property accessors have slightly different
rules and are simply open coded for that reason).
Patch [10/12] has been modified to drop function arguments that happened to
have the same values for both of the current users of those functions and
patches [11-12/12] have been modified to take that change into account. If
the code in question needs to be made more complex in the future, there
should not be any problems with that.
Due to the nature of the changes I have retained all ACKs except for the
Grant's Reviewed-by on patch [03/12] (if that had been Acked-by, I would
have
retained it too, but that didn't feel appropriate for the reviewed by
thing
to me). If any of you think that the ACKs are not applicable any more,
please
let me know and I'll drop them.
Finally, many thanks to Mika for testing the series on MinnowBoard 1 and
MinnowBoard Max. In case anybody else would like to test it, it is
available
from the device-properties branch of the linux-pm.git tree:
git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git
device-properties
Thanks!
Crickets ...
OK, so I'm taking the lack of comments as the lack of objections and I'm
already
getting merge conflicts for this series. Moreover, we already have done some
work on top of it.
So, if there still are no comments by Sunday evening, I'll add this series to
my linux-next branch with 3.19-rc1 as the target.
Aside from the comments I've made elsewhere, you can add my acked by for the
whole series.
Acked-by: Grant Likely grant.lik...@linaro.org
g.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 02/12] Driver core: Unified device properties interface for platform firmware
On Tue, 21 Oct 2014 23:15:50 +0200
, Rafael J. Wysocki r...@rjwysocki.net
wrote:
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
Add a uniform interface by which device drivers can request device
properties from the platform firmware by providing a property name
and the corresponding data type. The purpose of it is to help to
write portable code that won't depend on any particular platform
firmware interface.
The following general helper functions are added:
device_property_present()
device_property_read_u8()
device_property_read_u16()
device_property_read_u32()
device_property_read_u64()
device_property_read_string()
device_property_read_u8_array()
device_property_read_u16_array()
device_property_read_u32_array()
device_property_read_u64_array()
device_property_read_string_array()
The first one allows the caller to check if the given property is
present. The next 5 of them allow single-valued properties of
various types to be retrieved in a uniform way. The remaining 5 are
for reading properties with multiple values (arrays of either numbers
or strings).
The interface covers both ACPI and Device Trees.
This change set includes material from Mika Westerberg and Aaron Lu.
Signed-off-by: Aaron Lu aaron...@intel.com
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Acked-by: Greg Kroah-Hartman gre...@linuxfoundation.org
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
Changes from v5:
- acpi_dev_prop_read() can now handle both list (package) and single-value
properties (the latter are tried first for the last argument equal to 1).
- There is a new macro to generate the bodies of
device_property_read_u*_array().
- device_property_read_u*() are implemented using
device_property_read_u*_array().
- device_property_read_bool() is a new static inline wrapper around
device_property_present().
---
drivers/acpi/property.c | 178 +++
drivers/base/Makefile|2
drivers/base/property.c | 233
+++
drivers/of/base.c| 106 ++---
include/linux/acpi.h | 32 ++
include/linux/of.h | 22
include/linux/property.h | 53 ++
7 files changed, 609 insertions(+), 17 deletions(-)
create mode 100644 drivers/base/property.c
create mode 100644 include/linux/property.h
Index: linux-pm/include/linux/property.h
===
--- /dev/null
+++ linux-pm/include/linux/property.h
@@ -0,0 +1,53 @@
+/*
+ * property.h - Unified device property interface.
+ *
+ * Copyright (C) 2014, Intel Corporation
+ * Authors: Rafael J. Wysocki rafael.j.wyso...@intel.com
+ * Mika Westerberg mika.westerb...@linux.intel.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef _LINUX_PROPERTY_H_
+#define _LINUX_PROPERTY_H_
+
+#include linux/types.h
+
+struct device;
+
+enum dev_prop_type {
+ DEV_PROP_U8,
+ DEV_PROP_U16,
+ DEV_PROP_U32,
+ DEV_PROP_U64,
+ DEV_PROP_STRING,
+ DEV_PROP_MAX,
+};
+
+bool device_property_present(struct device *dev, const char *propname);
+int device_property_read_u8_array(struct device *dev, const char *propname,
+ u8 *val, size_t nval);
+int device_property_read_u16_array(struct device *dev, const char *propname,
+u16 *val, size_t nval);
+int device_property_read_u32_array(struct device *dev, const char *propname,
+u32 *val, size_t nval);
+int device_property_read_u64_array(struct device *dev, const char *propname,
+u64 *val, size_t nval);
+int device_property_read_string_array(struct device *dev, const char
*propname,
+ char **val, size_t nval);
I'm not sure if I asked this elsewhere. Can 'val' be made a const char **?
g.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/3] of/irq: Export of_irq_get()
On Thu, 30 Oct 2014 15:16:44 +0100 , Wolfram Sang w...@the-dreams.de wrote: On Thu, Oct 30, 2014 at 03:59:36PM +0200, Laurent Pinchart wrote: The function will be used by the I2C core which can be compiled as a module. Signed-off-by: Laurent Pinchart laurent.pinchart+rene...@ideasonboard.com I think it makes sense if I take this via I2C to get the dependencies for the later patches right? Agreed. Acked-by: Grant Likely grant.lik...@linaro.org --- drivers/of/irq.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/of/irq.c b/drivers/of/irq.c index 1471e0a223a5..0d7765807f49 100644 --- a/drivers/of/irq.c +++ b/drivers/of/irq.c @@ -405,6 +405,7 @@ int of_irq_get(struct device_node *dev, int index) return irq_create_of_mapping(oirq); } +EXPORT_SYMBOL_GPL(of_irq_get); /** * of_irq_get_byname - Decode a node's IRQ and return it as a Linux irq number -- 2.0.4 -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: Request and map make argument name constant
On Tue, 21 Oct 2014 18:27:25 +0200
, Matthias Brugger matthias@gmail.com
wrote:
This patch makes the name argument from of_io_request_and_map constant.
Signed-off-by: Matthias Brugger matthias@gmail.com
Applied, thanks.
g.
---
drivers/of/address.c | 2 +-
include/linux/of_address.h | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/of/address.c b/drivers/of/address.c
index afdb782..e02828f 100644
--- a/drivers/of/address.c
+++ b/drivers/of/address.c
@@ -871,7 +871,7 @@ EXPORT_SYMBOL(of_iomap);
* return PTR_ERR(base);
*/
void __iomem *of_io_request_and_map(struct device_node *np, int index,
- char *name)
+ const char *name)
{
struct resource res;
void __iomem *mem;
diff --git a/include/linux/of_address.h b/include/linux/of_address.h
index 8cb14eb..d88e81b 100644
--- a/include/linux/of_address.h
+++ b/include/linux/of_address.h
@@ -106,7 +106,7 @@ extern int of_address_to_resource(struct device_node
*dev, int index,
struct resource *r);
void __iomem *of_iomap(struct device_node *node, int index);
void __iomem *of_io_request_and_map(struct device_node *device,
- int index, char *name);
+ int index, const char *name);
#else
#include linux/io.h
@@ -123,7 +123,7 @@ static inline void __iomem *of_iomap(struct device_node
*device, int index)
}
static inline void __iomem *of_io_request_and_map(struct device_node *device,
- int index, char *name)
+ int index, const char *name)
{
return IOMEM_ERR_PTR(-EINVAL);
}
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 0/1] Compact interface for Device-Tree
On Mon, 03 Nov 2014 16:06:03 +0100
, Arnd Bergmann a...@arndb.de
wrote:
On Friday 31 October 2014 23:53:28 Rafael J. Wysocki wrote:
On Saturday, November 01, 2014 05:13:45 AM Rob Herring wrote:
On Fri, Oct 31, 2014 at 6:59 AM, Gilad Avidov gavi...@codeaurora.org
wrote:
Device-Tree compact API
Common code seen in driverâs probe reads device tree values and
handling
erroneous return codes from all those of_property_read_xxx() APIs. This
common code is factored out by the of_property_map module which allows
driverâs probe to replace that (often lengthy) code with a concise
table:
struct of_prop_map map[] = {
{i2c,dev-id,OF_REQ, OF_ID, -1},
{qcom,clk-freq-out,dev-clk_freq_out,OF_REQ, OF_U32,
0},
{qcom,clk-freq-in,dev-clk_freq_in,OF_REQ, OF_U32, 0},
{qcom,disable-dma,dev-disable_dma,OF_OPT, OF_BOOL, 0},
{qcom,master-id,dev-mstr_id,OF_SGST, OF_U32, 0},
{NULL,NULL,0, 0, 0},
};
Then call populate to read the values into the deviceâs variables:
ret = of_prop_populate(dev, dev-of_node, map);
Interesting idea. The main concern I have with this is there has been
on-going discussions about how to generalize property handling across
DT and ACPI to make drivers more agnostic, so I'm copying a few folks
involved in that. That may be a bit orthogonal to what this is doing,
but we may want some coordination here.
Agreed.
We actually have a patchset adding a unified device property API in
linux-next
(http://git.kernel.org/cgit/linux/kernel/git/rafael/linux-pm.git/log/?h=device-properties)
and I'd prefer to see the compactization to happen at that level, if
possible,
rather that for of_ only.
Agreed, this should definitely use the new generalized API.
I have prototyped a similar concept last year, which actually went much
further and also abstracted high-level properties such as interrupts,
gpios, pwm, dma-engine, etc. I still think we should do something
like that, but I've never had the time to follow up and nobody else
picked up my work from back then.
Would others like to see that?
Absolutely. I also tried to do the same thing and didn't get very far.
And, yes, it should be done at the level of the device properties API.
g.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of/fdt: Don't clear initial_boot_params if fdt_check_header() fails
On Wed, 29 Oct 2014 12:15:00 -0600
, Bjorn Helgaas bhelg...@google.com
wrote:
If the device tree pointer is NULL, early_init_dt_verify() fails, leaving
initial_boot_params unchanged. If the device tree pointer is non-NULL but
invalid, early_init_dt_verify() again fails but this time it also clears
initial_boot_params.
Leave initial_boot_params unchanged if the device tree pointer is invalid.
This doesn't fix a bug, but it makes the behavior more consistent and
easier to analyze.
Signed-off-by: Bjorn Helgaas bhelg...@google.com
Looks correct. Applied.
g.
---
drivers/of/fdt.c |9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index d1ffca8b34ea..55a566a53f36 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -994,15 +994,12 @@ bool __init early_init_dt_verify(void *params)
if (!params)
return false;
- /* Setup flat device-tree pointer */
- initial_boot_params = params;
-
/* check device tree validity */
- if (fdt_check_header(params)) {
- initial_boot_params = NULL;
+ if (fdt_check_header(params))
return false;
- }
+ /* Setup flat device-tree pointer */
+ initial_boot_params = params;
return true;
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] of/fdt: simplify early_init_dt_verify
On Tue, 28 Oct 2014 19:51:05 +0800
, Zhen Lei thunder.leiz...@huawei.com
wrote:
put off initial_boot_params = params; until all checks finished. If any check
failed, initial_boot_params is equal NULL, base upon bss segment initial
value.
Signed-off-by: Zhen Lei thunder.leiz...@huawei.com
Interesting. I received exactly the same patch from Bjorn Helgaas. Were
the two of you working together, or is this merely a coincedence?
g.
---
drivers/of/fdt.c | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
index d1ffca8..325c638 100644
--- a/drivers/of/fdt.c
+++ b/drivers/of/fdt.c
@@ -994,14 +994,12 @@ bool __init early_init_dt_verify(void *params)
if (!params)
return false;
- /* Setup flat device-tree pointer */
- initial_boot_params = params;
-
/* check device tree validity */
- if (fdt_check_header(params)) {
- initial_boot_params = NULL;
+ if (fdt_check_header(params))
return false;
- }
+
+ /* Setup flat device-tree pointer */
+ initial_boot_params = params;
return true;
}
--
1.8.0
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 4/4] of: Remove spaces before tabs
On Wed, 22 Oct 2014 11:44:55 +0200
, Geert Uytterhoeven geert+rene...@glider.be
wrote:
Signed-off-by: Geert Uytterhoeven geert+rene...@glider.be
Applied all 4 patches. Thanks.
g.
---
drivers/of/base.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index cff3afcff1951faf..19075e770d8b6306 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -577,9 +577,9 @@ EXPORT_SYMBOL(of_get_parent);
* of_get_next_parent - Iterate to a node's parent
* @node: Node to get parent of
*
- * This is like of_get_parent() except that it drops the
- * refcount on the passed node, making it suitable for iterating
- * through a node's parents.
+ * This is like of_get_parent() except that it drops the
+ * refcount on the passed node, making it suitable for iterating
+ * through a node's parents.
*
* Returns a node pointer with refcount incremented, use
* of_node_put() on it when done.
@@ -1566,15 +1566,15 @@ EXPORT_SYMBOL(of_parse_phandle);
* Example:
*
* phandle1: node1 {
- * #list-cells = 2;
+ * #list-cells = 2;
* }
*
* phandle2: node2 {
- * #list-cells = 1;
+ * #list-cells = 1;
* }
*
* node3 {
- * list = phandle1 1 2 phandle2 3;
+ * list = phandle1 1 2 phandle2 3;
* }
*
* To get a device_node of the `node2' node you may call this:
@@ -1615,7 +1615,7 @@ EXPORT_SYMBOL(of_parse_phandle_with_args);
* }
*
* node3 {
- * list = phandle1 0 2 phandle2 2 3;
+ * list = phandle1 0 2 phandle2 2 3;
* }
*
* To get a device_node of the `node2' node you may call this:
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 1/1] of: Add a function to read 64-bit arrays
On Fri, 3 Oct 2014 00:59:23 +0300
, Sakari Ailus sakari.ai...@iki.fi
wrote:
Implement of_property_read_u64_array() for reading 64-bit arrays.
This is needed for e.g. reading the valid link frequencies in the smiapp
driver.
Signed-off-by: Sakari Ailus sakari.ai...@iki.fi
A patch that adds this function is alread part of the device properties
API patch series that will be merged for v3.19.
g.
---
Hi,
While the smiapp (found in drivers/media/i2c/smiapp/) OF support which needs
this isn't in yet, other drivers such as mt9v032 which would be reading the
valid link frequency control values will need reading arrays. This might
make it to v4l2-of.c in the end.
drivers/of/base.c | 44
include/linux/of.h |3 +++
2 files changed, 39 insertions(+), 8 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index d8574ad..35e24f4 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1214,6 +1214,41 @@ int of_property_read_u32_array(const struct
device_node *np,
EXPORT_SYMBOL_GPL(of_property_read_u32_array);
/**
+ * of_property_read_u64_array - Find and read an array of 64 bit integers
+ * from a property.
+ *
+ * @np: device node from which the property value is to be read.
+ * @propname:name of the property to be searched.
+ * @out_values: pointer to return value, modified only if return value
is 0.
+ * @sz: number of array elements to read
+ *
+ * Search for a property in a device node and read 64-bit value(s) from
+ * it. Returns 0 on success, -EINVAL if the property does not exist,
+ * -ENODATA if property does not have a value, and -EOVERFLOW if the
+ * property data isn't large enough.
+ *
+ * The out_values is modified only if a valid u32 value can be decoded.
+ */
+int of_property_read_u64_array(const struct device_node *np,
+const char *propname, u64 *out_value, size_t sz)
+{
+ const __be32 *val = of_find_property_value_of_size(
+ np, propname, sz * sizeof(*out_value));
+
+ if (IS_ERR(val))
+ return PTR_ERR(val);
+
+ while (sz--) {
+ *out_value = of_read_number(val, 2);
+ out_value++;
+ val += 2;
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(of_property_read_u64_array);
+
+/**
* of_property_read_u64 - Find and read a 64 bit integer from a property
* @np: device node from which the property value is to be read.
* @propname:name of the property to be searched.
@@ -1229,14 +1264,7 @@ EXPORT_SYMBOL_GPL(of_property_read_u32_array);
int of_property_read_u64(const struct device_node *np, const char *propname,
u64 *out_value)
{
- const __be32 *val = of_find_property_value_of_size(np, propname,
- sizeof(*out_value));
-
- if (IS_ERR(val))
- return PTR_ERR(val);
-
- *out_value = of_read_number(val, 2);
- return 0;
+ return of_property_read_u64_array(np, propname, out_value, 1);
}
EXPORT_SYMBOL_GPL(of_property_read_u64);
diff --git a/include/linux/of.h b/include/linux/of.h
index 6c4363b..e84533f 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -263,6 +263,9 @@ extern int of_property_read_u32_array(const struct
device_node *np,
size_t sz);
extern int of_property_read_u64(const struct device_node *np,
const char *propname, u64 *out_value);
+extern int of_property_read_u64_array(const struct device_node *np,
+ const char *propname, u64 *out_value,
+ size_t sz);
extern int of_property_read_string(struct device_node *np,
const char *propname,
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: Fix padding in _OF_DECLARE macro definition
On Thu, 2 Oct 2014 16:01:10 +0200
, Thierry Reding thierry.red...@gmail.com
wrote:
From: Thierry Reding tred...@nvidia.com
Signed-off-by: Thierry Reding tred...@nvidia.com
Applied, thanks.
g.
---
include/linux/of.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/of.h b/include/linux/of.h
index 6c4363b8ddc3..951e9ee7d3e0 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -772,7 +772,7 @@ static inline int of_get_available_child_count(const
struct device_node *np)
= { .compatible = compat, \
.data = (fn == (fn_type)NULL) ? fn : fn }
#else
-#define _OF_DECLARE(table, name, compat, fn, fn_type)
\
+#define _OF_DECLARE(table, name, compat, fn, fn_type)
\
static const struct of_device_id __of_table_##name \
__attribute__((unused)) \
= { .compatible = compat, \
--
2.1.0
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v6 02/12] Driver core: Unified device properties interface for platform firmware
On Mon, Nov 3, 2014 at 10:04 PM, Rafael J. Wysocki r...@rjwysocki.net wrote: I also found that this parser code doesn't correctly handle malformed (unterminated) string properties. It will overflow. The existing functions have the same problem, so it isn't something that you've added. I've got a fix, and as a side effect the fix creates the _array version basically for free as part of reworking of_property_count_strings() and of_property_read_string_index() OK So can you please point me to a git branch containing the fix? I'll rebase the patch on top of that then and everything should merge just fine. git://git.kernel.org/pub/scm/linux/kernel/git/glikely/linux.git devicetree/merge I'm going to let the patch sit in there for a few days to get linux-next exposure before I ask Linus to pull. The pull req will go out before the end of the week. g. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Tue, Nov 4, 2014 at 10:54 PM, Rafael J. Wysocki r...@rjwysocki.net wrote:
On Tuesday, November 04, 2014 05:06:40 PM Rafael J. Wysocki wrote:
On Tuesday, November 04, 2014 02:48:40 PM Grant Likely wrote:
On Mon, Nov 3, 2014 at 9:52 PM, Rafael J. Wysocki r...@rjwysocki.net
wrote:
On Monday, November 03, 2014 04:25:08 PM Rafael J. Wysocki wrote:
On Sunday, November 02, 2014 08:49:37 PM Darren Hart wrote:
On 11/1/14 4:11, Grant Likely wrote:
On Tue, 28 Oct 2014 22:59:57 +0100
, Rafael J. Wysocki r...@rjwysocki.net
wrote:
On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
acpi_dev_add_driver_gpios() makes it possible to set up mapping
between
properties and ACPI GpioIo resources in a driver, so we can take
index
parameter in acpi_find_gpio() into use with _DSD device
properties now.
This index can be used to select a GPIO from a property with
multiple
GPIOs:
Package () {
data-gpios,
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
}
In order to retrieve the last GPIO from a driver we can simply do:
desc = devm_gpiod_get_index(dev, data, 2);
and so on.
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Cool. :-)
Any objections anyone?
Actually, I do. Not in the idea, but in the implementation. The way
this gets encoded:
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
Means that decoding each GPIO tuple requires the length of a tuple
to be
fixed, or to implement a DT-like #gpio-cells. If it is fixed, then
there
is no way to expand the binding later. Can this be done in the
following
way instead?
Package () {
Package () { \_SB.GPIO, 0, 0, 0 },
Package () { \_SB.GPIO, 1, 0, 0 },
Package () { \_SB.GPIO, 2, 0, 1 },
}
This is one of the biggest pains in device tree. We don't have any
way
to group tuples so it requires looking up stuff across the tree to
figure out how to parse each multi-item property.
I know that last year we talked about how bios vendors would get
complicated properties wrong, but I think there is little risk in
this
case. If the property is encoded wrong, the driver simply won't
work and
it is unlikely to get shipped before being fixed.
This particular nesting of Packages is expressly prohibited by the
Device Properties UUID for the reasons you mention.
http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
Also we don't use properties where single name is assigned to multiple
GPIOs
anywhere in the current device-properties patchset, so this is not
relevant at
the moment.
Moreover, even if we were to use them, we would need to ensure that
this:
Package () {
\_SB.GPIO, 0, 0, 0
}
was equivalent to
Package () {
Package () { \_SB.GPIO, 0, 0, 0 }
}
This is not impossible to do and I suppose we could even explain that
in the
implementation guide document in a sensible way, but that would require
the
document linked above to be changed first and *then* we can think about
writing
kernel code to it. Not the other way around, please.
So Grant, do you want us to proceed with that?
Before you reply, one more observation that seems to be relevant.
In ACPI, both this:
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
and this:
Package () {
Package () { \_SB.GPIO, 0, 0, 0 },
Package () { \_SB.GPIO, 1, 0, 0 },
Package () { \_SB.GPIO, 2, 0, 1 },
}
carry the same information, because every element of a package has a
type,
so there is no danger of confusing an ACPI_TYPE_LOCAL_REFERENCE with
ACPI_TYPE_INTEGER. Thus one can easily count the number of GPIOs
represented
by the first package by counting the number of reference elements in it.
The second one has more structure which in this particular case is
arguably
redundant.
Okay, this make sense. I'm okay with this approach, and I would
recommend making that the only valid method for parsing in
acpi_dev_get_property_reference(). Get rid of the *size_prop argument
so that it always behaves the same way and users aren't tempted to do
something clever.
OK, I'll send a followup patch to remove the size_prop arg from
[PATCH] of: Fix overflow bug in string property parsing functions
The string property read helpers will run off the end of the buffer if
it is handed a malformed string property. Rework the parsers to make
sure that doesn't happen. At the same time add new test cases to make
sure the functions behave themselves.
The original implementations of of_property_read_string_index() and
of_property_count_strings() both open-coded the same block of parsing
code, each with it's own subtly different bugs. The fix here merges
functions into a single helper and makes the original functions static
inline wrappers around the helper.
One non-bugfix aspect of this patch is the addition of a new wrapper,
of_property_read_string_array(). The new wrapper is needed by the
device_properties feature that Rafael is working on and planning to
merge for v3.19. The implementation is identical both with and without
the new static inline wrapper, so it just got left in to reduce the
churn on the header file.
Signed-off-by: Grant Likely
Cc: Rafael J. Wysocki
Cc: Mika Westerberg
Cc: Rob Herring
Cc: Arnd Bergmann
Cc: Darren Hart
Cc: # v3.3+: Drop selftest hunks that don't apply
---
I'd like to get this into v3.18 in the next week. Please shout if you
have any objections.
drivers/of/base.c | 88 -
drivers/of/selftest.c | 66 --
drivers/of/testcase-data/tests-phandle.dtsi | 2 +
include/linux/of.h | 84 ++-
4 files changed, 154 insertions(+), 86 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 2305dc0382bc..3823edf2d012 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1280,52 +1280,6 @@ int of_property_read_string(struct device_node *np,
const char *propname,
EXPORT_SYMBOL_GPL(of_property_read_string);
/**
- * of_property_read_string_index - Find and read a string from a multiple
- * strings property.
- * @np:device node from which the property value is to be read.
- * @propname: name of the property to be searched.
- * @index: index of the string in the list of strings
- * @out_string:pointer to null terminated return string, modified only
if
- * return value is 0.
- *
- * Search for a property in a device tree node and retrieve a null
- * terminated string value (pointer to data, not a copy) in the list of strings
- * contained in that property.
- * Returns 0 on success, -EINVAL if the property does not exist, -ENODATA if
- * property does not have a value, and -EILSEQ if the string is not
- * null-terminated within the length of the property data.
- *
- * The out_string pointer is modified only if a valid string can be decoded.
- */
-int of_property_read_string_index(struct device_node *np, const char *propname,
- int index, const char **output)
-{
- struct property *prop = of_find_property(np, propname, NULL);
- int i = 0;
- size_t l = 0, total = 0;
- const char *p;
-
- if (!prop)
- return -EINVAL;
- if (!prop->value)
- return -ENODATA;
- if (strnlen(prop->value, prop->length) >= prop->length)
- return -EILSEQ;
-
- p = prop->value;
-
- for (i = 0; total < prop->length; total += l, p += l) {
- l = strlen(p) + 1;
- if (i++ == index) {
- *output = p;
- return 0;
- }
- }
- return -ENODATA;
-}
-EXPORT_SYMBOL_GPL(of_property_read_string_index);
-
-/**
* of_property_match_string() - Find string in a list and return index
* @np: pointer to node containing string list property
* @propname: string list property name
@@ -1351,7 +1305,7 @@ int of_property_match_string(struct device_node *np,
const char *propname,
end = p + prop->length;
for (i = 0; p < end; i++, p += l) {
- l = strlen(p) + 1;
+ l = strnlen(p, end - p) + 1;
if (p + l > end)
return -EILSEQ;
pr_debug("comparing %s with %s\n", string, p);
@@ -1363,39 +1317,41 @@ int of_property_match_string(struct device_node *np,
const char *propname,
EXPORT_SYMBOL_GPL(of_property_match_string);
/**
- * of_property_count_strings - Find and return the number of strings from a
- * multiple strings property.
+ * of_property_read_string_util() - Utility helper for parsing string
properties
* @np:device node from which the property value is to be read.
* @propname: name of the property to be searched.
+ * @out_strs: output array of string pointers.
+ * @sz:number of array elements to read.
+ * @skip: Number of strings to skip over at beginning of list.
*
- * Search for a property in a device tree node and retrieve the number of null
- * terminated string contain in it. Returns the number of strings on
- * succ
Re: [PATCH v6 02/12] Driver core: Unified device properties interface for platform firmware
Hi Rafael,
While reviewing and testing these patches I ran into serious bugs in
the string parsers (in both the existing code and the new functions
here). It took me a number of days, but I've got a fix now which I'll
be posting shortly and I want to get into mainline right away. I'll cc
you when I post it.
The fix will conflict with this patch. Mostly as a side effect of the
fix, the of_property_*string* function changes will no longer be
needed in this patch. It will either need to be respun, or we'll need
to give Linus some guidance on resolving the conflicts when merging.
Other comments below...
On Tue, Oct 21, 2014 at 10:15 PM, Rafael J. Wysocki wrote:
> From: "Rafael J. Wysocki"
>
> Add a uniform interface by which device drivers can request device
> properties from the platform firmware by providing a property name
> and the corresponding data type. The purpose of it is to help to
> write portable code that won't depend on any particular platform
> firmware interface.
>
> The following general helper functions are added:
[...]
> +/**
> + * device_property_read_u8_array - return a u8 array property of a device
> + * @dev: Device to get the property of
> + * @propname: Name of the property
> + * @val: The values are stored here
> + * @nval: Size of the @val array
> + *
> + * Function reads an array of u8 properties with @propname from the device
> + * firmware description and stores them to @val if found.
> + *
> + * Return: %0 if the property was found (success),
> + *%-EINVAL if given arguments are not valid,
> + *%-ENODATA if the property does not have a value,
> + *%-EPROTO if the property is not an array of numbers,
> + *%-EOVERFLOW if the size of the property is not as expected.
> + */
> +int device_property_read_u8_array(struct device *dev, const char *propname,
> + u8 *val, size_t nval)
> +{
> + return DEV_PROP_READ_ARRAY(dev, propname, u8, DEV_PROP_U8, val, nval);
> +}
> +EXPORT_SYMBOL_GPL(device_property_read_u8_array);
Yup, I'm a lot happier with this approach. :-)
> +
> +/**
> + * device_property_read_u16_array - return a u16 array property of a device
> + * @dev: Device to get the property of
> + * @propname: Name of the property
> + * @val: The values are stored here
> + * @nval: Size of the @val array
> + *
> + * Function reads an array of u16 properties with @propname from the device
> + * firmware description and stores them to @val if found.
> + *
> + * Return: %0 if the property was found (success),
> + *%-EINVAL if given arguments are not valid,
> + *%-ENODATA if the property does not have a value,
> + *%-EPROTO if the property is not an array of numbers,
> + *%-EOVERFLOW if the size of the property is not as expected.
> + */
> +int device_property_read_u16_array(struct device *dev, const char *propname,
> + u16 *val, size_t nval)
> +{
> + return DEV_PROP_READ_ARRAY(dev, propname, u16, DEV_PROP_U16, val,
> nval);
> +}
> +EXPORT_SYMBOL_GPL(device_property_read_u16_array);
> +
> +/**
> + * device_property_read_u32_array - return a u32 array property of a device
> + * @dev: Device to get the property of
> + * @propname: Name of the property
> + * @val: The values are stored here
> + * @nval: Size of the @val array
> + *
> + * Function reads an array of u32 properties with @propname from the device
> + * firmware description and stores them to @val if found.
> + *
> + * Return: %0 if the property was found (success),
> + *%-EINVAL if given arguments are not valid,
> + *%-ENODATA if the property does not have a value,
> + *%-EPROTO if the property is not an array of numbers,
> + *%-EOVERFLOW if the size of the property is not as expected.
> + */
> +int device_property_read_u32_array(struct device *dev, const char *propname,
> + u32 *val, size_t nval)
> +{
> + return DEV_PROP_READ_ARRAY(dev, propname, u32, DEV_PROP_U32, val,
> nval);
> +}
> +EXPORT_SYMBOL_GPL(device_property_read_u32_array);
> +
> +/**
> + * device_property_read_u64_array - return a u64 array property of a device
> + * @dev: Device to get the property of
> + * @propname: Name of the property
> + * @val: The values are stored here
> + * @nval: Size of the @val array
> + *
> + * Function reads an array of u64 properties with @propname from the device
> + * firmware description and stores them to @val if found.
> + *
> + * Return: %0 if the property was found (success),
> + *%-EINVAL if given arguments are not valid,
> + *%-ENODATA if the property does not have a value,
> + *%-EPROTO if the property is not an array of numbers,
> + *%-EOVERFLOW if the size of the property is not as expected.
> + */
> +int device_property_read_u64_array(struct device *dev, const char *propname,
> + u64 *val, size_t nval)
> +{
> + return
Re: [PATCH v6 02/12] Driver core: Unified device properties interface for platform firmware
Hi Rafael,
While reviewing and testing these patches I ran into serious bugs in
the string parsers (in both the existing code and the new functions
here). It took me a number of days, but I've got a fix now which I'll
be posting shortly and I want to get into mainline right away. I'll cc
you when I post it.
The fix will conflict with this patch. Mostly as a side effect of the
fix, the of_property_*string* function changes will no longer be
needed in this patch. It will either need to be respun, or we'll need
to give Linus some guidance on resolving the conflicts when merging.
Other comments below...
On Tue, Oct 21, 2014 at 10:15 PM, Rafael J. Wysocki r...@rjwysocki.net wrote:
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
Add a uniform interface by which device drivers can request device
properties from the platform firmware by providing a property name
and the corresponding data type. The purpose of it is to help to
write portable code that won't depend on any particular platform
firmware interface.
The following general helper functions are added:
[...]
+/**
+ * device_property_read_u8_array - return a u8 array property of a device
+ * @dev: Device to get the property of
+ * @propname: Name of the property
+ * @val: The values are stored here
+ * @nval: Size of the @val array
+ *
+ * Function reads an array of u8 properties with @propname from the device
+ * firmware description and stores them to @val if found.
+ *
+ * Return: %0 if the property was found (success),
+ *%-EINVAL if given arguments are not valid,
+ *%-ENODATA if the property does not have a value,
+ *%-EPROTO if the property is not an array of numbers,
+ *%-EOVERFLOW if the size of the property is not as expected.
+ */
+int device_property_read_u8_array(struct device *dev, const char *propname,
+ u8 *val, size_t nval)
+{
+ return DEV_PROP_READ_ARRAY(dev, propname, u8, DEV_PROP_U8, val, nval);
+}
+EXPORT_SYMBOL_GPL(device_property_read_u8_array);
Yup, I'm a lot happier with this approach. :-)
+
+/**
+ * device_property_read_u16_array - return a u16 array property of a device
+ * @dev: Device to get the property of
+ * @propname: Name of the property
+ * @val: The values are stored here
+ * @nval: Size of the @val array
+ *
+ * Function reads an array of u16 properties with @propname from the device
+ * firmware description and stores them to @val if found.
+ *
+ * Return: %0 if the property was found (success),
+ *%-EINVAL if given arguments are not valid,
+ *%-ENODATA if the property does not have a value,
+ *%-EPROTO if the property is not an array of numbers,
+ *%-EOVERFLOW if the size of the property is not as expected.
+ */
+int device_property_read_u16_array(struct device *dev, const char *propname,
+ u16 *val, size_t nval)
+{
+ return DEV_PROP_READ_ARRAY(dev, propname, u16, DEV_PROP_U16, val,
nval);
+}
+EXPORT_SYMBOL_GPL(device_property_read_u16_array);
+
+/**
+ * device_property_read_u32_array - return a u32 array property of a device
+ * @dev: Device to get the property of
+ * @propname: Name of the property
+ * @val: The values are stored here
+ * @nval: Size of the @val array
+ *
+ * Function reads an array of u32 properties with @propname from the device
+ * firmware description and stores them to @val if found.
+ *
+ * Return: %0 if the property was found (success),
+ *%-EINVAL if given arguments are not valid,
+ *%-ENODATA if the property does not have a value,
+ *%-EPROTO if the property is not an array of numbers,
+ *%-EOVERFLOW if the size of the property is not as expected.
+ */
+int device_property_read_u32_array(struct device *dev, const char *propname,
+ u32 *val, size_t nval)
+{
+ return DEV_PROP_READ_ARRAY(dev, propname, u32, DEV_PROP_U32, val,
nval);
+}
+EXPORT_SYMBOL_GPL(device_property_read_u32_array);
+
+/**
+ * device_property_read_u64_array - return a u64 array property of a device
+ * @dev: Device to get the property of
+ * @propname: Name of the property
+ * @val: The values are stored here
+ * @nval: Size of the @val array
+ *
+ * Function reads an array of u64 properties with @propname from the device
+ * firmware description and stores them to @val if found.
+ *
+ * Return: %0 if the property was found (success),
+ *%-EINVAL if given arguments are not valid,
+ *%-ENODATA if the property does not have a value,
+ *%-EPROTO if the property is not an array of numbers,
+ *%-EOVERFLOW if the size of the property is not as expected.
+ */
+int device_property_read_u64_array(struct device *dev, const char *propname,
+ u64 *val, size_t nval)
+{
+ return DEV_PROP_READ_ARRAY(dev, propname, u64, DEV_PROP_U64, val,
nval);
+}
[PATCH] of: Fix overflow bug in string property parsing functions
The string property read helpers will run off the end of the buffer if
it is handed a malformed string property. Rework the parsers to make
sure that doesn't happen. At the same time add new test cases to make
sure the functions behave themselves.
The original implementations of of_property_read_string_index() and
of_property_count_strings() both open-coded the same block of parsing
code, each with it's own subtly different bugs. The fix here merges
functions into a single helper and makes the original functions static
inline wrappers around the helper.
One non-bugfix aspect of this patch is the addition of a new wrapper,
of_property_read_string_array(). The new wrapper is needed by the
device_properties feature that Rafael is working on and planning to
merge for v3.19. The implementation is identical both with and without
the new static inline wrapper, so it just got left in to reduce the
churn on the header file.
Signed-off-by: Grant Likely grant.lik...@linaro.org
Cc: Rafael J. Wysocki rafael.j.wyso...@intel.com
Cc: Mika Westerberg mika.westerb...@linux.intel.com
Cc: Rob Herring robh...@kernel.org
Cc: Arnd Bergmann a...@arndb.de
Cc: Darren Hart darren.h...@intel.com
Cc: sta...@vger.kernel.org # v3.3+: Drop selftest hunks that don't apply
---
I'd like to get this into v3.18 in the next week. Please shout if you
have any objections.
drivers/of/base.c | 88 -
drivers/of/selftest.c | 66 --
drivers/of/testcase-data/tests-phandle.dtsi | 2 +
include/linux/of.h | 84 ++-
4 files changed, 154 insertions(+), 86 deletions(-)
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 2305dc0382bc..3823edf2d012 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -1280,52 +1280,6 @@ int of_property_read_string(struct device_node *np,
const char *propname,
EXPORT_SYMBOL_GPL(of_property_read_string);
/**
- * of_property_read_string_index - Find and read a string from a multiple
- * strings property.
- * @np:device node from which the property value is to be read.
- * @propname: name of the property to be searched.
- * @index: index of the string in the list of strings
- * @out_string:pointer to null terminated return string, modified only
if
- * return value is 0.
- *
- * Search for a property in a device tree node and retrieve a null
- * terminated string value (pointer to data, not a copy) in the list of strings
- * contained in that property.
- * Returns 0 on success, -EINVAL if the property does not exist, -ENODATA if
- * property does not have a value, and -EILSEQ if the string is not
- * null-terminated within the length of the property data.
- *
- * The out_string pointer is modified only if a valid string can be decoded.
- */
-int of_property_read_string_index(struct device_node *np, const char *propname,
- int index, const char **output)
-{
- struct property *prop = of_find_property(np, propname, NULL);
- int i = 0;
- size_t l = 0, total = 0;
- const char *p;
-
- if (!prop)
- return -EINVAL;
- if (!prop-value)
- return -ENODATA;
- if (strnlen(prop-value, prop-length) = prop-length)
- return -EILSEQ;
-
- p = prop-value;
-
- for (i = 0; total prop-length; total += l, p += l) {
- l = strlen(p) + 1;
- if (i++ == index) {
- *output = p;
- return 0;
- }
- }
- return -ENODATA;
-}
-EXPORT_SYMBOL_GPL(of_property_read_string_index);
-
-/**
* of_property_match_string() - Find string in a list and return index
* @np: pointer to node containing string list property
* @propname: string list property name
@@ -1351,7 +1305,7 @@ int of_property_match_string(struct device_node *np,
const char *propname,
end = p + prop-length;
for (i = 0; p end; i++, p += l) {
- l = strlen(p) + 1;
+ l = strnlen(p, end - p) + 1;
if (p + l end)
return -EILSEQ;
pr_debug(comparing %s with %s\n, string, p);
@@ -1363,39 +1317,41 @@ int of_property_match_string(struct device_node *np,
const char *propname,
EXPORT_SYMBOL_GPL(of_property_match_string);
/**
- * of_property_count_strings - Find and return the number of strings from a
- * multiple strings property.
+ * of_property_read_string_util() - Utility helper for parsing string
properties
* @np:device node from which the property value is to be read.
* @propname: name of the property to be searched.
+ * @out_strs: output array of string pointers.
+ * @sz:number of array elements to read.
+ * @skip: Number of strings to skip over at beginning of list.
*
- * Search for a property in a device tree node and retrieve
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Tue, 28 Oct 2014 22:59:57 +0100
, "Rafael J. Wysocki"
wrote:
> On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
> > acpi_dev_add_driver_gpios() makes it possible to set up mapping between
> > properties and ACPI GpioIo resources in a driver, so we can take index
> > parameter in acpi_find_gpio() into use with _DSD device properties now.
> >
> > This index can be used to select a GPIO from a property with multiple
> > GPIOs:
> >
> > Package () {
> > "data-gpios",
> > Package () {
> > \_SB.GPIO, 0, 0, 0,
> > \_SB.GPIO, 1, 0, 0,
> > \_SB.GPIO, 2, 0, 1,
> > }
> > }
> >
> > In order to retrieve the last GPIO from a driver we can simply do:
> >
> > desc = devm_gpiod_get_index(dev, "data", 2);
> >
> > and so on.
> >
> > Signed-off-by: Mika Westerberg
>
> Cool. :-)
>
> Any objections anyone?
Actually, I do. Not in the idea, but in the implementation. The way this gets
encoded:
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
Means that decoding each GPIO tuple requires the length of a tuple to be
fixed, or to implement a DT-like #gpio-cells. If it is fixed, then there
is no way to expand the binding later. Can this be done in the following
way instead?
Package () {
Package () { \_SB.GPIO, 0, 0, 0 },
Package () { \_SB.GPIO, 1, 0, 0 },
Package () { \_SB.GPIO, 2, 0, 1 },
}
This is one of the biggest pains in device tree. We don't have any way
to group tuples so it requires looking up stuff across the tree to
figure out how to parse each multi-item property.
I know that last year we talked about how bios vendors would get
complicated properties wrong, but I think there is little risk in this
case. If the property is encoded wrong, the driver simply won't work and
it is unlikely to get shipped before being fixed.
g.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI / GPIO: Pass index to acpi_get_gpiod_by_index() when using properties
On Tue, 28 Oct 2014 22:59:57 +0100
, Rafael J. Wysocki r...@rjwysocki.net
wrote:
On Tuesday, October 28, 2014 01:15:27 PM Mika Westerberg wrote:
acpi_dev_add_driver_gpios() makes it possible to set up mapping between
properties and ACPI GpioIo resources in a driver, so we can take index
parameter in acpi_find_gpio() into use with _DSD device properties now.
This index can be used to select a GPIO from a property with multiple
GPIOs:
Package () {
data-gpios,
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
}
In order to retrieve the last GPIO from a driver we can simply do:
desc = devm_gpiod_get_index(dev, data, 2);
and so on.
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Cool. :-)
Any objections anyone?
Actually, I do. Not in the idea, but in the implementation. The way this gets
encoded:
Package () {
\_SB.GPIO, 0, 0, 0,
\_SB.GPIO, 1, 0, 0,
\_SB.GPIO, 2, 0, 1,
}
Means that decoding each GPIO tuple requires the length of a tuple to be
fixed, or to implement a DT-like #gpio-cells. If it is fixed, then there
is no way to expand the binding later. Can this be done in the following
way instead?
Package () {
Package () { \_SB.GPIO, 0, 0, 0 },
Package () { \_SB.GPIO, 1, 0, 0 },
Package () { \_SB.GPIO, 2, 0, 1 },
}
This is one of the biggest pains in device tree. We don't have any way
to group tuples so it requires looking up stuff across the tree to
figure out how to parse each multi-item property.
I know that last year we talked about how bios vendors would get
complicated properties wrong, but I think there is little risk in this
case. If the property is encoded wrong, the driver simply won't work and
it is unlikely to get shipped before being fixed.
g.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 09/12] Driver core: Unified interface for firmware node properties
On Mon, Oct 20, 2014 at 4:19 PM, Arnd Bergmann wrote:
> On Monday 20 October 2014 01:46:00 Rafael J. Wysocki wrote:
>>
>> > Something like:
>> >
>> > #define define_fwnode_accessors(__type, __devprop_type) \
>> > int device_property_read_##__type(struct device *dev, \
>> > const char *propname, __type *val) \
>> > { \
>> > if (IS_ENABLED(CONFIG_OF) && dev->of_node) \
>> > return of_property_read_##__type(dev->of_node, propname,
>> > val); \
>> > return acpi_dev_prop_read(ACPI_COMPANION(dev), propname, \
>> > __devprop_type, val); \
>> > } \
>> > int fwnode_property_read_##__type(struct fwnode_handle *fwnode, \
>> > const char *propname, __type *val) \
>> > { \
>> > if (IS_ENABLED(CONFIG_OF) && is_of_node(fwnode)) \
>> > return of_property_read_##__type(of_node(fwnode), propname,
>> > val); \
>> > else if (IS_ENABLED(CONFIG_ACPI) && is_acpi_node(fwnode)) \
>> > return acpi_dev_prop_read(acpi_node(fwnode), propname, \
>> > __devprop_type, val); \
>> > return -ENXIO; \
>> > }
>> >
>> > define_fwnode_accessors(u8, DEV_PROP_U8);
>> > define_fwnode_accessors(u16, DEV_PROP_U16);
>> > define_fwnode_accessors(u32, DEV_PROP_U32);
>> > define_fwnode_accessors(u64, DEV_PROP_U64);
>> >
>> > That significantly reduces the code size for these things.
>>
>> So I was considering to do that, but eventually decided not to, because (1)
>> adding kerneldoc comments to such things looks odd and (2) (which IMO is
>> more important) this breaks LXR (for example, the thing at
>> lxr.free-electrons.com
>> that some people, including me in particular, occasionally use to check how
>> things
>> are defined). And even if you used the old good grep to look for a
>> definition of
>> fwnode_property_read_u8, say, this wouldn't work exactly as expected I'm
>> afraid.
>
> Agreed, I'd also prefer your proposed code over Grant's macros.
>
>> I would very much like to retain the headers at least for this reason, if
>> that's
>> not a big deal.
>>
>> What I can do, however, is to use macros for generating the bodies of those
>> functions.
>
> Yes, just don't do any concatenation to generate the names of the called
> functions, i.e.
>
> return fwnode_call(of_property_read_u32, acpi_dev_prop_read,
> DEV_PROP_U32, node, propname, val);
>
> is better than
>
> return fwnode_call(u32, DEV_PROP_U32, node, propname, val);
>
> because it's easier to understand the call chain.
Sounds reasonable. I've got no problem with that.
g.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 09/12] Driver core: Unified interface for firmware node properties
On Mon, 20 Oct 2014 01:46 +0200
, "Rafael J. Wysocki"
wrote:
> On Saturday, October 18, 2014 04:55:20 PM Grant Likely wrote:
> > On Fri, 17 Oct 2014 14:14:53 +0200
> > , "Rafael J. Wysocki"
> > wrote:
> > > From: Rafael J. Wysocki
> > >
> > > Add new generic routines are provided for retrieving properties from
> > > device description objects in the platform firmware in case there are
> > > no struct device objects for them (either those objects have not been
> > > created yet or they do not exist at all).
> > >
> > > The following functions are provided:
> > >
> > > fwnode_property_present()
> > > fwnode_property_read_u8()
> > > fwnode_property_read_u16()
> > > fwnode_property_read_u32()
> > > fwnode_property_read_u64()
> > > fwnode_property_read_string()
> > > fwnode_property_read_u8_array()
> > > fwnode_property_read_u16_array()
> > > fwnode_property_read_u32_array()
> > > fwnode_property_read_u64_array()
> > > fwnode_property_read_string_array()
> > >
> > > in analogy with the corresponding functions for struct device added
> > > previously. For all of them, the first argument is a pointer to struct
> > > fwnode_handle (new type) that allows a device description object
> > > (depending on what platform firmware interface is in use) to be
> > > obtained.
> > >
> > > Add a new macro device_for_each_child_node() for iterating over the
> > > children of the device description object associated with a given
> > > device and a new function device_get_child_node_count() returning the
> > > number of a given device's child nodes.
> > >
> > > The interface covers both ACPI and Device Trees.
> >
> > This is all *so much* better. I'm a lot happier.
> >
> > I was about to make the comment that the implementation for
> > device_property_read_*() should merely be wrappers around
> > fwnode_property_read_*(), but when when I actually looked at it, I saw
> > this:
> >
> > In patch 2:
> > int device_property_read_u8(struct device *dev, const char *propname, u8
> > *val)
> > {
> > if (IS_ENABLED(CONFIG_OF) && dev->of_node)
> > return of_property_read_u8(dev->of_node, propname, val);
> >
> > return acpi_dev_prop_read(ACPI_COMPANION(dev), propname,
> > DEV_PROP_U8, val);
> > }
> >
> > And in this patch:
> > int fwnode_property_read_u8(struct fwnode_handle *fwnode, const char
> > *propname,
> > u8 *val)
> > {
> > if (is_of_node(fwnode))
> > return of_property_read_u8(of_node(fwnode), propname, val);
> > else if (is_acpi_node(fwnode))
> > return acpi_dev_prop_read(acpi_node(fwnode), propname,
> > DEV_PROP_U8, val);
> >
> > return -ENXIO;
> > }
> >
> > Making the device_property functions wrappers around fwnode_property_*
> > wouldn't actually be great since it would need to decode the fwnode
> > pointer twice.
>
> Indeed.
>
> > I do still think the functions above should be macro generated, just in
> > terms of keeping the line count down, and I would suggest merging patches #2
> > and #9.
>
> Well, the changes in those patches are almost completely independent and patch
> #9 is only actually needed for #11 and #12, so I'm not sure if that would be
> better. I certainly prefer splitting longer patches into pieces if that makes
> sense and it does make sense to do so in this particular case IMHO.
I'm not going to make a big deal about. Do what you think is best.
> > Something like:
> >
> > #define define_fwnode_accessors(__type, __devprop_type) \
> > int device_property_read_##__type(struct device *dev, \
> > const char *propname, __type *val) \
> > { \
> > if (IS_ENABLED(CONFIG_OF) && dev->of_node) \
> > return of_property_read_##__type(dev->of_node, propname, val); \
> > return acpi_dev_prop_read(ACPI_COMPANION(dev), propname, \
> > __devprop_type, val); \
> > } \
> > int fwnode_property_read_##__type(struct fwnode_handle *fwnode, \
> > const char *propname, __type *val) \
> > { \
> > if (IS_ENABLED(CONFIG_OF) && is_of_node(fwnode)) \
> > return of_property_read_##__type(of_node(fwnode), propname,
> > val
Re: [PATCH v5 03/12] ACPI: Allow drivers to match using Device Tree compatible property
On Fri, 17 Oct 2014 14:05:59 +0200
, "Rafael J. Wysocki"
wrote:
> From: Mika Westerberg
>
> We have lots of existing Device Tree enabled drivers and allocating
> separate _HID for each is not feasible. Instead we allocate special _HID
> "PRP0001" that means that the match should be done using Device Tree
> compatible property using driver's .of_match_table instead if the driver
> is missing .acpi_match_table.
(Not a critique of this patch, but merely a helpful comment; my previous
ack remains intact) :-)
It would be useful for the code to point at some document that describes
the semantics of the PRP0001 binding.
g.
>
> If there is a need to distinguish from where the device is enumerated
> (DT/ACPI) driver can check dev->of_node or ACPI_COMPATION(dev).
>
> Signed-off-by: Mika Westerberg
> Reviewed-by: Grant Likely
> Signed-off-by: Rafael J. Wysocki
> ---
> drivers/acpi/property.c | 34 +
> drivers/acpi/scan.c | 97
> +++--
> include/acpi/acpi_bus.h | 1 +
> include/linux/acpi.h| 8 +---
> 4 files changed, 123 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> index 680f7f1..ff53eb8 100644
> --- a/drivers/acpi/property.c
> +++ b/drivers/acpi/property.c
> @@ -76,6 +76,37 @@ static bool acpi_properties_format_valid(const union
> acpi_object *properties)
> return true;
> }
>
> +static void acpi_init_of_compatible(struct acpi_device *adev)
> +{
> + const union acpi_object *of_compatible;
> + struct acpi_hardware_id *hwid;
> + bool acpi_of = false;
> +
> + /*
> + * Check if the special PRP0001 ACPI ID is present and in that
> + * case we fill in Device Tree compatible properties for this
> + * device.
> + */
> + list_for_each_entry(hwid, >pnp.ids, list) {
> + if (!strcmp(hwid->id, "PRP0001")) {
> + acpi_of = true;
> + break;
> + }
> + }
> +
> + if (!acpi_of)
> + return;
> +
> + if (acpi_dev_get_property_array(adev, "compatible", ACPI_TYPE_STRING,
> + _compatible)) {
> + acpi_handle_warn(adev->handle,
> + "PRP0001 requires compatible property\n");
> + return;
> + }
> +
> + adev->data.of_compatible = of_compatible;
> +}
> +
> void acpi_init_properties(struct acpi_device *adev)
> {
> struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER };
> @@ -119,6 +150,8 @@ void acpi_init_properties(struct acpi_device *adev)
>
> adev->data.pointer = buf.pointer;
> adev->data.properties = properties;
> +
> + acpi_init_of_compatible(adev);
> return;
> }
>
> @@ -130,6 +163,7 @@ void acpi_init_properties(struct acpi_device *adev)
> void acpi_free_properties(struct acpi_device *adev)
> {
> ACPI_FREE((void *)adev->data.pointer);
> + adev->data.of_compatible = NULL;
> adev->data.pointer = NULL;
> adev->data.properties = NULL;
> }
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 1979ab3..bc999f1 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -124,17 +124,51 @@ static int create_modalias(struct acpi_device
> *acpi_dev, char *modalias,
> if (list_empty(_dev->pnp.ids))
> return 0;
>
> - len = snprintf(modalias, size, "acpi:");
> - size -= len;
> -
> - list_for_each_entry(id, _dev->pnp.ids, list) {
> - count = snprintf([len], size, "%s:", id->id);
> - if (count < 0)
> - return -EINVAL;
> - if (count >= size)
> - return -ENOMEM;
> - len += count;
> - size -= count;
> + /*
> + * If the device has PRP0001 we expose DT compatible modalias
> + * instead in form of of:NnameTCcompatible.
> + */
> + if (acpi_dev->data.of_compatible) {
> + struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER };
> + const union acpi_object *of_compatible, *obj;
> + char *c;
> + int i;
> +
> + acpi_get_name(acpi_dev->handle, ACPI_SINGLE_NAME, );
> + /* DT strings are all in lower case */
> + for (c = buf.pointer; *c != '\0'; c++)
> + *c = tolower(*c);
> +
> + len = snprintf(modalias, size, "of:N%sT", (char *)buf.pointer);
> +
Re: [PATCH v5 03/12] ACPI: Allow drivers to match using Device Tree compatible property
On Fri, 17 Oct 2014 14:05:59 +0200
, Rafael J. Wysocki r...@rjwysocki.net
wrote:
From: Mika Westerberg mika.westerb...@linux.intel.com
We have lots of existing Device Tree enabled drivers and allocating
separate _HID for each is not feasible. Instead we allocate special _HID
PRP0001 that means that the match should be done using Device Tree
compatible property using driver's .of_match_table instead if the driver
is missing .acpi_match_table.
(Not a critique of this patch, but merely a helpful comment; my previous
ack remains intact) :-)
It would be useful for the code to point at some document that describes
the semantics of the PRP0001 binding.
g.
If there is a need to distinguish from where the device is enumerated
(DT/ACPI) driver can check dev-of_node or ACPI_COMPATION(dev).
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Reviewed-by: Grant Likely grant.lik...@linaro.org
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
drivers/acpi/property.c | 34 +
drivers/acpi/scan.c | 97
+++--
include/acpi/acpi_bus.h | 1 +
include/linux/acpi.h| 8 +---
4 files changed, 123 insertions(+), 17 deletions(-)
diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 680f7f1..ff53eb8 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -76,6 +76,37 @@ static bool acpi_properties_format_valid(const union
acpi_object *properties)
return true;
}
+static void acpi_init_of_compatible(struct acpi_device *adev)
+{
+ const union acpi_object *of_compatible;
+ struct acpi_hardware_id *hwid;
+ bool acpi_of = false;
+
+ /*
+ * Check if the special PRP0001 ACPI ID is present and in that
+ * case we fill in Device Tree compatible properties for this
+ * device.
+ */
+ list_for_each_entry(hwid, adev-pnp.ids, list) {
+ if (!strcmp(hwid-id, PRP0001)) {
+ acpi_of = true;
+ break;
+ }
+ }
+
+ if (!acpi_of)
+ return;
+
+ if (acpi_dev_get_property_array(adev, compatible, ACPI_TYPE_STRING,
+ of_compatible)) {
+ acpi_handle_warn(adev-handle,
+ PRP0001 requires compatible property\n);
+ return;
+ }
+
+ adev-data.of_compatible = of_compatible;
+}
+
void acpi_init_properties(struct acpi_device *adev)
{
struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER };
@@ -119,6 +150,8 @@ void acpi_init_properties(struct acpi_device *adev)
adev-data.pointer = buf.pointer;
adev-data.properties = properties;
+
+ acpi_init_of_compatible(adev);
return;
}
@@ -130,6 +163,7 @@ void acpi_init_properties(struct acpi_device *adev)
void acpi_free_properties(struct acpi_device *adev)
{
ACPI_FREE((void *)adev-data.pointer);
+ adev-data.of_compatible = NULL;
adev-data.pointer = NULL;
adev-data.properties = NULL;
}
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 1979ab3..bc999f1 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -124,17 +124,51 @@ static int create_modalias(struct acpi_device
*acpi_dev, char *modalias,
if (list_empty(acpi_dev-pnp.ids))
return 0;
- len = snprintf(modalias, size, acpi:);
- size -= len;
-
- list_for_each_entry(id, acpi_dev-pnp.ids, list) {
- count = snprintf(modalias[len], size, %s:, id-id);
- if (count 0)
- return -EINVAL;
- if (count = size)
- return -ENOMEM;
- len += count;
- size -= count;
+ /*
+ * If the device has PRP0001 we expose DT compatible modalias
+ * instead in form of of:NnameTCcompatible.
+ */
+ if (acpi_dev-data.of_compatible) {
+ struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER };
+ const union acpi_object *of_compatible, *obj;
+ char *c;
+ int i;
+
+ acpi_get_name(acpi_dev-handle, ACPI_SINGLE_NAME, buf);
+ /* DT strings are all in lower case */
+ for (c = buf.pointer; *c != '\0'; c++)
+ *c = tolower(*c);
+
+ len = snprintf(modalias, size, of:N%sT, (char *)buf.pointer);
+ ACPI_FREE(buf.pointer);
+
+ of_compatible = acpi_dev-data.of_compatible;
+ for (i = 0; i of_compatible-package.count; i++) {
+ obj = of_compatible-package.elements[i];
+
+ count = snprintf(modalias[len], size, C%s,
+ obj-string.pointer);
+ if (count 0)
+ return -EINVAL;
+ if (count = size
Re: [PATCH v5 09/12] Driver core: Unified interface for firmware node properties
On Mon, 20 Oct 2014 01:46 +0200
, Rafael J. Wysocki r...@rjwysocki.net
wrote:
On Saturday, October 18, 2014 04:55:20 PM Grant Likely wrote:
On Fri, 17 Oct 2014 14:14:53 +0200
, Rafael J. Wysocki r...@rjwysocki.net
wrote:
From: Rafael J. Wysocki rafael.j.wyso...@intel.com
Add new generic routines are provided for retrieving properties from
device description objects in the platform firmware in case there are
no struct device objects for them (either those objects have not been
created yet or they do not exist at all).
The following functions are provided:
fwnode_property_present()
fwnode_property_read_u8()
fwnode_property_read_u16()
fwnode_property_read_u32()
fwnode_property_read_u64()
fwnode_property_read_string()
fwnode_property_read_u8_array()
fwnode_property_read_u16_array()
fwnode_property_read_u32_array()
fwnode_property_read_u64_array()
fwnode_property_read_string_array()
in analogy with the corresponding functions for struct device added
previously. For all of them, the first argument is a pointer to struct
fwnode_handle (new type) that allows a device description object
(depending on what platform firmware interface is in use) to be
obtained.
Add a new macro device_for_each_child_node() for iterating over the
children of the device description object associated with a given
device and a new function device_get_child_node_count() returning the
number of a given device's child nodes.
The interface covers both ACPI and Device Trees.
This is all *so much* better. I'm a lot happier.
I was about to make the comment that the implementation for
device_property_read_*() should merely be wrappers around
fwnode_property_read_*(), but when when I actually looked at it, I saw
this:
In patch 2:
int device_property_read_u8(struct device *dev, const char *propname, u8
*val)
{
if (IS_ENABLED(CONFIG_OF) dev-of_node)
return of_property_read_u8(dev-of_node, propname, val);
return acpi_dev_prop_read(ACPI_COMPANION(dev), propname,
DEV_PROP_U8, val);
}
And in this patch:
int fwnode_property_read_u8(struct fwnode_handle *fwnode, const char
*propname,
u8 *val)
{
if (is_of_node(fwnode))
return of_property_read_u8(of_node(fwnode), propname, val);
else if (is_acpi_node(fwnode))
return acpi_dev_prop_read(acpi_node(fwnode), propname,
DEV_PROP_U8, val);
return -ENXIO;
}
Making the device_property functions wrappers around fwnode_property_*
wouldn't actually be great since it would need to decode the fwnode
pointer twice.
Indeed.
I do still think the functions above should be macro generated, just in
terms of keeping the line count down, and I would suggest merging patches #2
and #9.
Well, the changes in those patches are almost completely independent and patch
#9 is only actually needed for #11 and #12, so I'm not sure if that would be
better. I certainly prefer splitting longer patches into pieces if that makes
sense and it does make sense to do so in this particular case IMHO.
I'm not going to make a big deal about. Do what you think is best.
Something like:
#define define_fwnode_accessors(__type, __devprop_type) \
int device_property_read_##__type(struct device *dev, \
const char *propname, __type *val) \
{ \
if (IS_ENABLED(CONFIG_OF) dev-of_node) \
return of_property_read_##__type(dev-of_node, propname, val); \
return acpi_dev_prop_read(ACPI_COMPANION(dev), propname, \
__devprop_type, val); \
} \
int fwnode_property_read_##__type(struct fwnode_handle *fwnode, \
const char *propname, __type *val) \
{ \
if (IS_ENABLED(CONFIG_OF) is_of_node(fwnode)) \
return of_property_read_##__type(of_node(fwnode), propname,
val); \
else if (IS_ENABLED(CONFIG_ACPI) is_acpi_node(fwnode)) \
return acpi_dev_prop_read(acpi_node(fwnode), propname, \
__devprop_type, val); \
return -ENXIO; \
}
define_fwnode_accessors(u8, DEV_PROP_U8);
define_fwnode_accessors(u16, DEV_PROP_U16);
define_fwnode_accessors(u32, DEV_PROP_U32);
define_fwnode_accessors(u64, DEV_PROP_U64);
That significantly reduces the code size for these things.
So I was considering to do that, but eventually decided not to, because (1)
adding kerneldoc comments to such things looks odd and (2) (which IMO is
more important) this breaks LXR (for example, the thing at
lxr.free-electrons.com
that some people, including me in particular, occasionally use to check how
things
are defined). And even if you used the old good grep to look for a
definition of
fwnode_property_read_u8
Re: [PATCH v5 09/12] Driver core: Unified interface for firmware node properties
On Mon, Oct 20, 2014 at 4:19 PM, Arnd Bergmann a...@arndb.de wrote:
On Monday 20 October 2014 01:46:00 Rafael J. Wysocki wrote:
Something like:
#define define_fwnode_accessors(__type, __devprop_type) \
int device_property_read_##__type(struct device *dev, \
const char *propname, __type *val) \
{ \
if (IS_ENABLED(CONFIG_OF) dev-of_node) \
return of_property_read_##__type(dev-of_node, propname,
val); \
return acpi_dev_prop_read(ACPI_COMPANION(dev), propname, \
__devprop_type, val); \
} \
int fwnode_property_read_##__type(struct fwnode_handle *fwnode, \
const char *propname, __type *val) \
{ \
if (IS_ENABLED(CONFIG_OF) is_of_node(fwnode)) \
return of_property_read_##__type(of_node(fwnode), propname,
val); \
else if (IS_ENABLED(CONFIG_ACPI) is_acpi_node(fwnode)) \
return acpi_dev_prop_read(acpi_node(fwnode), propname, \
__devprop_type, val); \
return -ENXIO; \
}
define_fwnode_accessors(u8, DEV_PROP_U8);
define_fwnode_accessors(u16, DEV_PROP_U16);
define_fwnode_accessors(u32, DEV_PROP_U32);
define_fwnode_accessors(u64, DEV_PROP_U64);
That significantly reduces the code size for these things.
So I was considering to do that, but eventually decided not to, because (1)
adding kerneldoc comments to such things looks odd and (2) (which IMO is
more important) this breaks LXR (for example, the thing at
lxr.free-electrons.com
that some people, including me in particular, occasionally use to check how
things
are defined). And even if you used the old good grep to look for a
definition of
fwnode_property_read_u8, say, this wouldn't work exactly as expected I'm
afraid.
Agreed, I'd also prefer your proposed code over Grant's macros.
I would very much like to retain the headers at least for this reason, if
that's
not a big deal.
What I can do, however, is to use macros for generating the bodies of those
functions.
Yes, just don't do any concatenation to generate the names of the called
functions, i.e.
return fwnode_call(of_property_read_u32, acpi_dev_prop_read,
DEV_PROP_U32, node, propname, val);
is better than
return fwnode_call(u32, DEV_PROP_U32, node, propname, val);
because it's easier to understand the call chain.
Sounds reasonable. I've got no problem with that.
g.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 09/12] Driver core: Unified interface for firmware node properties
ev, propname, val, 1) \
} \
int fwnode_property_read_##__type##_array(struct fwnode_handle *fwnode, \
const char *propname, __type *val, \
size_t nval) \
{ \
if (IS_ENABLED(CONFIG_OF) && is_of_node(fwnode)) \
return of_property_read_##__type(of_node(fwnode), propname,
val, nval); \
else if (IS_ENABLED(CONFIG_ACPI) && is_acpi_node(fwnode)) \
return acpi_dev_prop_read(acpi_node(fwnode), propname, \
__devprop_type, val, nval); \
return -ENXIO; \
} \
static inline int fwnode_property_read_##__type(struct fwnode_handle *fwnode, \
const char *propname, __type *val) \
{ \
return fwnode_property_read_##__type##_array(fwnode, propname, val, 1) \
}
define_fwnode_accessors(u8, DEV_PROP_U8);
define_fwnode_accessors(u16, DEV_PROP_U16);
define_fwnode_accessors(u32, DEV_PROP_U32);
define_fwnode_accessors(u64, DEV_PROP_U64);
g.
>
> Suggested-by: Grant Likely
> Signed-off-by: Rafael J. Wysocki
> ---
> drivers/acpi/scan.c | 21 ++
> drivers/base/property.c | 373
> +++
> include/acpi/acpi_bus.h | 12 +
> include/linux/acpi.h | 21 ++
> include/linux/of.h | 22 ++
> include/linux/property.h | 48 ++
> 6 files changed, 497 insertions(+)
>
> Index: linux-pm/include/linux/property.h
> ===
> --- linux-pm.orig/include/linux/property.h
> +++ linux-pm/include/linux/property.h
> @@ -44,4 +44,52 @@ int device_property_read_u64_array(struc
> int device_property_read_string_array(struct device *dev, const char
> *propname,
> char **val, size_t nval);
>
> +enum fwnode_type {
> + FWNODE_INVALID = 0,
> + FWNODE_OF,
> + FWNODE_ACPI,
> +};
> +
> +struct fwnode_handle {
> + enum fwnode_type type;
> +};
> +
> +bool fwnode_property_present(struct fwnode_handle *fwnode, const char
> *propname);
> +int fwnode_property_read_u8(struct fwnode_handle *fwnode, const char
> *propname,
> + u8 *val);
> +int fwnode_property_read_u16(struct fwnode_handle *fwnode, const char
> *propname,
> + u16 *val);
> +int fwnode_property_read_u32(struct fwnode_handle *fwnode, const char
> *propname,
> + u32 *val);
> +int fwnode_property_read_u64(struct fwnode_handle *fwnode, const char
> *propname,
> + u64 *val);
> +int fwnode_property_read_string(struct fwnode_handle *fwnode,
> + const char *propname, const char **val);
> +int fwnode_property_read_u8_array(struct fwnode_handle *fwnode,
> + const char *propname, u8 *val,
> + size_t nval);
> +int fwnode_property_read_u16_array(struct fwnode_handle *fwnode,
> +const char *propname, u16 *val,
> +size_t nval);
> +int fwnode_property_read_u32_array(struct fwnode_handle *fwnode,
> +const char *propname, u32 *val,
> +size_t nval);
> +int fwnode_property_read_u64_array(struct fwnode_handle *fwnode,
> +const char *propname, u64 *val,
> +size_t nval);
> +int fwnode_property_read_string_array(struct fwnode_handle *fwnode,
> + const char *propname, char **val,
> + size_t nval);
> +
> +struct fwnode_handle *device_get_next_child_node(struct device *dev,
> + struct fwnode_handle *child);
> +
> +#define device_for_each_child_node(dev, child) \
> + for (child = device_get_next_child_node(dev, NULL); child; \
> + child = device_get_next_child_node(dev, child))
> +
> +void fwnode_handle_put(struct fwnode_handle *fwnode);
> +
> +unsigned int device_get_child_node_count(struct device *dev);
> +
> #endif /* _LINUX_PROPERTY_H_ */
> Index: linux-pm/include/acpi/acpi_bus.h
> ===
> --- linux-pm.orig/include/acpi/acpi_bus.h
> +++ linux-pm/include/acpi/acpi_bus.h
> @@ -27,6 +27,7 @@
> #define __ACPI_BUS_H__
>
> #include
> +#include
>
> /* TBD: Make dynamic */
> #define ACPI_MAX_HANDLES 10
> @@ -348,6 +349,7 @@ struct acpi_device_data {
> struct acpi_device {
> int device_type;
> acpi_handle handle; /* no handle for fixed hardware */
> + struc
Re: [PATCH v4 00/13] Add ACPI _DSD and unified device properties support
On Thu, 16 Oct 2014 16:55:56 +0200 , David Woodhouse wrote: > On Wed, 2014-10-15 at 17:43 +0200, Darren Hart wrote: > > > > So my objection here is that by keeping the of_* terms in the driver we > > are required to include of, although it does safely convert to returning > > NULL if !CONFIG_OF I suppose. > > New version removes everything but the of_match_id bits which we need to > match ACPI devices too. Perhaps they ought to be renamed, but I'm not > sure it's worth it. > > This also removes the call to platform_get_resource(IORESOURCE_MEM) and > fall back to platform_get_resource(IORESOURCE_IO) as discussed IRL with > Rafael. I'm not sure it's much of an improvement, mind you :) > > Still untested. I think it's OK to switch to platform_get_irq() and then > drop the irq_dispose_mapping() call, right? The platform_device takes > care of all of that for us? Well, the irq management code is all messed up, but what you are doing is indeed okay. Unfortunately for platform devices we can never free an IRQ once we've claimed it. That's a completely separate problem and you don't need to worry about it for this patch. g. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 00/12] Add ACPI _DSD and unified device properties support
On Sat, 18 Oct 2014 00:50 +0200 , "Rafael J. Wysocki" wrote: > On Friday, October 17, 2014 08:04:52 PM Arnd Bergmann wrote: > > > > On October 17, 2014 2:01:33 PM CEST, "Rafael J. Wysocki" > > wrote: > > >Hi Everyone, > > > > > >Hving had a couple of chats with Grant and Arnd during LinuxCon EU/LPC, > > >we > > >now have version 5 taking all feedback into account (hopefully). > > > > Awesome, that was really fast. I'm currently on my way his me in > > the train, replying from my phone, but it looks good now. I'll have a more > > detailed look next week but I'm definitely happy to see this go in (to next > > and 3.19) now, any details we still find can be fixed on top. > > > > > In > > >short, if > > >we are passed a struct fwnode_handle pointer, we can get from it to the > > >appropriate device node pointer (either struct acpi_device or struct > > >device_node) > > >using container_of() after we've checked the type. This is needed for > > >the code > > >that needs to access child nodes of a device in case when they don't > > >have > > >struct device representations (whatever the reason). This has been > > >suggested > > >by Grant and pretty much everyone involved agrees that it's better that > > >the > > >alternatives presented so far. > > > > Yes, it's nice enough that I now take back all the objections I had for the > > child accessory API. > > Cool, thanks! > > I've just refreshed the device-properties branch of the linux-pm.git tree > and it contains the current material now (including a couple of build > fixes reported by the autobuild robot in patches [02/12] and [09/12]). > > My plan is to merge this into the linux-next branch when 3.18-rc1 is out. Wait for my ack please, but I should be able to review it this weekend or on Monday. I'm actually reviewing now, but I'm on a plane near the end of the flight. As soon as I get on the ground SIGFAMILY will interrupt me for a bit. :-) g. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 00/13] Add ACPI _DSD and unified device properties support
On Wed, 15 Oct 2014 17:43:01 +0200 , Darren Hart wrote: > > > On 10/15/14 17:17, Mark Rutland wrote: > > On Wed, Oct 15, 2014 at 03:46:39PM +0100, Darren Hart wrote: > > >> Mark, what would you propose we do differently to enable this driver to > >> be firmware-type agnostic? > > > > For this particular driver, all I'm asking for is that the > > "used-by-rtas" property is not moved over from of_find_property to > > device_get_property. It is irrelevant for all ACPI systems. Evidently my > > comment was unclear; I apologise for that. > > So my objection here is that by keeping the of_* terms in the driver we > are required to include of, although it does safely convert to returning > NULL if !CONFIG_OF I suppose. This shouldn't be that controversial. There will be things that only make sense for DT or only ACPI. Allowing the property to be processed when the other interface is being used may tempt firmware authors to use the property because it just happens to have a side effect that looks right to them. I don't see any problem with factoring out those bits into a function that is only called (or built) when the associated firmware interface is used. In these situations, the driver isn't 100% generic, so having small per-firmware hooks is absolutely okay and not a burden to maintain. g. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 00/13] Add ACPI _DSD and unified device properties support
On Thu, 16 Oct 2014 16:55:56 +0200
, David Woodhouse
wrote:
> On Wed, 2014-10-15 at 17:43 +0200, Darren Hart wrote:
> >
> > So my objection here is that by keeping the of_* terms in the driver we
> > are required to include of, although it does safely convert to returning
> > NULL if !CONFIG_OF I suppose.
>
> New version removes everything but the of_match_id bits which we need to
> match ACPI devices too. Perhaps they ought to be renamed, but I'm not
> sure it's worth it.
>
> This also removes the call to platform_get_resource(IORESOURCE_MEM) and
> fall back to platform_get_resource(IORESOURCE_IO) as discussed IRL with
> Rafael. I'm not sure it's much of an improvement, mind you :)
>
> Still untested. I think it's OK to switch to platform_get_irq() and then
> drop the irq_dispose_mapping() call, right? The platform_device takes
> care of all of that for us?
>
> diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
> index 26cec64..be95a4c 100644
> --- a/drivers/tty/serial/Kconfig
> +++ b/drivers/tty/serial/Kconfig
> @@ -1094,14 +1094,14 @@ config SERIAL_NETX_CONSOLE
> you can make it the console by answering Y to this option.
>
> config SERIAL_OF_PLATFORM
> - tristate "Serial port on Open Firmware platform bus"
> - depends on OF
> + tristate "Serial port on firmware platform bus"
> + depends on OF || ACPI
> depends on SERIAL_8250 || SERIAL_OF_PLATFORM_NWPSERIAL
> help
> - If you have a PowerPC based system that has serial ports
> - on a platform specific bus, you should enable this option.
> - Currently, only 8250 compatible ports are supported, but
> - others can easily be added.
> + If you have a system which advertises its serial ports through
> + devicetree or ACPI, you should enable this option. Currently
> + only 8250 compatible and NWP ports and are supported, but others
> + can easily be added.
>
> config SERIAL_OMAP
> tristate "OMAP serial port support"
> diff --git a/drivers/tty/serial/of_serial.c b/drivers/tty/serial/of_serial.c
> index 68d4455..cc6c99b 100644
> --- a/drivers/tty/serial/of_serial.c
> +++ b/drivers/tty/serial/of_serial.c
> @@ -14,8 +14,7 @@
> #include
> #include
> #include
> -#include
> -#include
> +#include
> #include
> #include
> #include
> @@ -53,22 +52,22 @@ static inline void tegra_serial_handle_break(struct
> uart_port *port)
> /*
> * Fill a struct uart_port for a given device node
> */
> -static int of_platform_serial_setup(struct platform_device *ofdev,
> +static int of_platform_serial_setup(struct platform_device *pdev,
> int type, struct uart_port *port,
> struct of_serial_info *info)
> {
> - struct resource resource;
> - struct device_node *np = ofdev->dev.of_node;
> u32 clk, spd, prop;
> - int ret;
> + int iotype = -1;
> + u32 res_start;
> + int ret, i;
>
> memset(port, 0, sizeof *port);
> - if (of_property_read_u32(np, "clock-frequency", )) {
> + if (device_property_read_u32(>dev, "clock-frequency", )) {
>
> /* Get clk rate through clk driver if present */
> - info->clk = clk_get(>dev, NULL);
> + info->clk = clk_get(>dev, NULL);
> if (IS_ERR(info->clk)) {
> - dev_warn(>dev,
> + dev_warn(>dev,
> "clk or clock-frequency not defined\n");
> return PTR_ERR(info->clk);
> }
> @@ -77,57 +76,63 @@ static int of_platform_serial_setup(struct
> platform_device *ofdev,
> clk = clk_get_rate(info->clk);
> }
> /* If current-speed was set, then try not to change it. */
> - if (of_property_read_u32(np, "current-speed", ) == 0)
> + if (device_property_read_u32(>dev, "current-speed", ) == 0)
> port->custom_divisor = clk / (16 * spd);
>
> - ret = of_address_to_resource(np, 0, );
> - if (ret) {
> - dev_warn(>dev, "invalid address\n");
> + /* Check for shifted address mapping */
> + if (device_property_read_u32(>dev, "reg-offset", ) != 0)
> + prop = 0;
> +
> + for (i = 0; iotype == -1 && i < pdev->num_resources; i++) {
> + struct resource *resource = >resource[i];
> + if (resource_type(resource) == IORESOURCE_MEM) {
> + iotype = UPIO_MEM;
> + port->mapbase = res_start + prop;
> + } else if (resource_type(resource) == IORESOURCE_IO) {
> + iotype = UPIO_PORT;
> + port->iobase = res_start + prop;
> + }
> +
> + res_start = resource->start;
> + }
> + if (iotype == -1) {
> + dev_warn(>dev, "invalid address\n");
> goto out;
> }
>
> spin_lock_init(>lock);
> - port->mapbase = resource.start;
> -
> - /* Check for shifted address
Re: [PATCH v4 00/13] Add ACPI _DSD and unified device properties support
On Thu, 16 Oct 2014 16:55:56 +0200
, David Woodhouse dw...@infradead.org
wrote:
On Wed, 2014-10-15 at 17:43 +0200, Darren Hart wrote:
So my objection here is that by keeping the of_* terms in the driver we
are required to include of, although it does safely convert to returning
NULL if !CONFIG_OF I suppose.
New version removes everything but the of_match_id bits which we need to
match ACPI devices too. Perhaps they ought to be renamed, but I'm not
sure it's worth it.
This also removes the call to platform_get_resource(IORESOURCE_MEM) and
fall back to platform_get_resource(IORESOURCE_IO) as discussed IRL with
Rafael. I'm not sure it's much of an improvement, mind you :)
Still untested. I think it's OK to switch to platform_get_irq() and then
drop the irq_dispose_mapping() call, right? The platform_device takes
care of all of that for us?
diff --git a/drivers/tty/serial/Kconfig b/drivers/tty/serial/Kconfig
index 26cec64..be95a4c 100644
--- a/drivers/tty/serial/Kconfig
+++ b/drivers/tty/serial/Kconfig
@@ -1094,14 +1094,14 @@ config SERIAL_NETX_CONSOLE
you can make it the console by answering Y to this option.
config SERIAL_OF_PLATFORM
- tristate Serial port on Open Firmware platform bus
- depends on OF
+ tristate Serial port on firmware platform bus
+ depends on OF || ACPI
depends on SERIAL_8250 || SERIAL_OF_PLATFORM_NWPSERIAL
help
- If you have a PowerPC based system that has serial ports
- on a platform specific bus, you should enable this option.
- Currently, only 8250 compatible ports are supported, but
- others can easily be added.
+ If you have a system which advertises its serial ports through
+ devicetree or ACPI, you should enable this option. Currently
+ only 8250 compatible and NWP ports and are supported, but others
+ can easily be added.
config SERIAL_OMAP
tristate OMAP serial port support
diff --git a/drivers/tty/serial/of_serial.c b/drivers/tty/serial/of_serial.c
index 68d4455..cc6c99b 100644
--- a/drivers/tty/serial/of_serial.c
+++ b/drivers/tty/serial/of_serial.c
@@ -14,8 +14,7 @@
#include linux/delay.h
#include linux/serial_core.h
#include linux/serial_reg.h
-#include linux/of_address.h
-#include linux/of_irq.h
+#include linux/property.h
#include linux/of_platform.h
#include linux/nwpserial.h
#include linux/clk.h
@@ -53,22 +52,22 @@ static inline void tegra_serial_handle_break(struct
uart_port *port)
/*
* Fill a struct uart_port for a given device node
*/
-static int of_platform_serial_setup(struct platform_device *ofdev,
+static int of_platform_serial_setup(struct platform_device *pdev,
int type, struct uart_port *port,
struct of_serial_info *info)
{
- struct resource resource;
- struct device_node *np = ofdev-dev.of_node;
u32 clk, spd, prop;
- int ret;
+ int iotype = -1;
+ u32 res_start;
+ int ret, i;
memset(port, 0, sizeof *port);
- if (of_property_read_u32(np, clock-frequency, clk)) {
+ if (device_property_read_u32(pdev-dev, clock-frequency, clk)) {
/* Get clk rate through clk driver if present */
- info-clk = clk_get(ofdev-dev, NULL);
+ info-clk = clk_get(pdev-dev, NULL);
if (IS_ERR(info-clk)) {
- dev_warn(ofdev-dev,
+ dev_warn(pdev-dev,
clk or clock-frequency not defined\n);
return PTR_ERR(info-clk);
}
@@ -77,57 +76,63 @@ static int of_platform_serial_setup(struct
platform_device *ofdev,
clk = clk_get_rate(info-clk);
}
/* If current-speed was set, then try not to change it. */
- if (of_property_read_u32(np, current-speed, spd) == 0)
+ if (device_property_read_u32(pdev-dev, current-speed, spd) == 0)
port-custom_divisor = clk / (16 * spd);
- ret = of_address_to_resource(np, 0, resource);
- if (ret) {
- dev_warn(ofdev-dev, invalid address\n);
+ /* Check for shifted address mapping */
+ if (device_property_read_u32(pdev-dev, reg-offset, prop) != 0)
+ prop = 0;
+
+ for (i = 0; iotype == -1 i pdev-num_resources; i++) {
+ struct resource *resource = pdev-resource[i];
+ if (resource_type(resource) == IORESOURCE_MEM) {
+ iotype = UPIO_MEM;
+ port-mapbase = res_start + prop;
+ } else if (resource_type(resource) == IORESOURCE_IO) {
+ iotype = UPIO_PORT;
+ port-iobase = res_start + prop;
+ }
+
+ res_start = resource-start;
+ }
+ if (iotype == -1) {
+ dev_warn(pdev-dev, invalid address\n);
goto out;
}
spin_lock_init(port-lock);
-
Re: [PATCH v5 00/12] Add ACPI _DSD and unified device properties support
On Sat, 18 Oct 2014 00:50 +0200 , Rafael J. Wysocki r...@rjwysocki.net wrote: On Friday, October 17, 2014 08:04:52 PM Arnd Bergmann wrote: On October 17, 2014 2:01:33 PM CEST, Rafael J. Wysocki r...@rjwysocki.net wrote: Hi Everyone, Hving had a couple of chats with Grant and Arnd during LinuxCon EU/LPC, we now have version 5 taking all feedback into account (hopefully). Awesome, that was really fast. I'm currently on my way his me in the train, replying from my phone, but it looks good now. I'll have a more detailed look next week but I'm definitely happy to see this go in (to next and 3.19) now, any details we still find can be fixed on top. In short, if we are passed a struct fwnode_handle pointer, we can get from it to the appropriate device node pointer (either struct acpi_device or struct device_node) using container_of() after we've checked the type. This is needed for the code that needs to access child nodes of a device in case when they don't have struct device representations (whatever the reason). This has been suggested by Grant and pretty much everyone involved agrees that it's better that the alternatives presented so far. Yes, it's nice enough that I now take back all the objections I had for the child accessory API. Cool, thanks! I've just refreshed the device-properties branch of the linux-pm.git tree and it contains the current material now (including a couple of build fixes reported by the autobuild robot in patches [02/12] and [09/12]). My plan is to merge this into the linux-next branch when 3.18-rc1 is out. Wait for my ack please, but I should be able to review it this weekend or on Monday. I'm actually reviewing now, but I'm on a plane near the end of the flight. As soon as I get on the ground SIGFAMILY will interrupt me for a bit. :-) g. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 00/13] Add ACPI _DSD and unified device properties support
On Wed, 15 Oct 2014 17:43:01 +0200 , Darren Hart dvh...@linux.intel.com wrote: On 10/15/14 17:17, Mark Rutland wrote: On Wed, Oct 15, 2014 at 03:46:39PM +0100, Darren Hart wrote: Mark, what would you propose we do differently to enable this driver to be firmware-type agnostic? For this particular driver, all I'm asking for is that the used-by-rtas property is not moved over from of_find_property to device_get_property. It is irrelevant for all ACPI systems. Evidently my comment was unclear; I apologise for that. So my objection here is that by keeping the of_* terms in the driver we are required to include of, although it does safely convert to returning NULL if !CONFIG_OF I suppose. This shouldn't be that controversial. There will be things that only make sense for DT or only ACPI. Allowing the property to be processed when the other interface is being used may tempt firmware authors to use the property because it just happens to have a side effect that looks right to them. I don't see any problem with factoring out those bits into a function that is only called (or built) when the associated firmware interface is used. In these situations, the driver isn't 100% generic, so having small per-firmware hooks is absolutely okay and not a burden to maintain. g. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 00/13] Add ACPI _DSD and unified device properties support
On Thu, 16 Oct 2014 16:55:56 +0200 , David Woodhouse dw...@infradead.org wrote: On Wed, 2014-10-15 at 17:43 +0200, Darren Hart wrote: So my objection here is that by keeping the of_* terms in the driver we are required to include of, although it does safely convert to returning NULL if !CONFIG_OF I suppose. New version removes everything but the of_match_id bits which we need to match ACPI devices too. Perhaps they ought to be renamed, but I'm not sure it's worth it. This also removes the call to platform_get_resource(IORESOURCE_MEM) and fall back to platform_get_resource(IORESOURCE_IO) as discussed IRL with Rafael. I'm not sure it's much of an improvement, mind you :) Still untested. I think it's OK to switch to platform_get_irq() and then drop the irq_dispose_mapping() call, right? The platform_device takes care of all of that for us? Well, the irq management code is all messed up, but what you are doing is indeed okay. Unfortunately for platform devices we can never free an IRQ once we've claimed it. That's a completely separate problem and you don't need to worry about it for this patch. g. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v5 09/12] Driver core: Unified interface for firmware node properties
, \
size_t nval) \
{ \
if (IS_ENABLED(CONFIG_OF) is_of_node(fwnode)) \
return of_property_read_##__type(of_node(fwnode), propname,
val, nval); \
else if (IS_ENABLED(CONFIG_ACPI) is_acpi_node(fwnode)) \
return acpi_dev_prop_read(acpi_node(fwnode), propname, \
__devprop_type, val, nval); \
return -ENXIO; \
} \
static inline int fwnode_property_read_##__type(struct fwnode_handle *fwnode, \
const char *propname, __type *val) \
{ \
return fwnode_property_read_##__type##_array(fwnode, propname, val, 1) \
}
define_fwnode_accessors(u8, DEV_PROP_U8);
define_fwnode_accessors(u16, DEV_PROP_U16);
define_fwnode_accessors(u32, DEV_PROP_U32);
define_fwnode_accessors(u64, DEV_PROP_U64);
g.
Suggested-by: Grant Likely grant.lik...@linaro.org
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
drivers/acpi/scan.c | 21 ++
drivers/base/property.c | 373
+++
include/acpi/acpi_bus.h | 12 +
include/linux/acpi.h | 21 ++
include/linux/of.h | 22 ++
include/linux/property.h | 48 ++
6 files changed, 497 insertions(+)
Index: linux-pm/include/linux/property.h
===
--- linux-pm.orig/include/linux/property.h
+++ linux-pm/include/linux/property.h
@@ -44,4 +44,52 @@ int device_property_read_u64_array(struc
int device_property_read_string_array(struct device *dev, const char
*propname,
char **val, size_t nval);
+enum fwnode_type {
+ FWNODE_INVALID = 0,
+ FWNODE_OF,
+ FWNODE_ACPI,
+};
+
+struct fwnode_handle {
+ enum fwnode_type type;
+};
+
+bool fwnode_property_present(struct fwnode_handle *fwnode, const char
*propname);
+int fwnode_property_read_u8(struct fwnode_handle *fwnode, const char
*propname,
+ u8 *val);
+int fwnode_property_read_u16(struct fwnode_handle *fwnode, const char
*propname,
+ u16 *val);
+int fwnode_property_read_u32(struct fwnode_handle *fwnode, const char
*propname,
+ u32 *val);
+int fwnode_property_read_u64(struct fwnode_handle *fwnode, const char
*propname,
+ u64 *val);
+int fwnode_property_read_string(struct fwnode_handle *fwnode,
+ const char *propname, const char **val);
+int fwnode_property_read_u8_array(struct fwnode_handle *fwnode,
+ const char *propname, u8 *val,
+ size_t nval);
+int fwnode_property_read_u16_array(struct fwnode_handle *fwnode,
+const char *propname, u16 *val,
+size_t nval);
+int fwnode_property_read_u32_array(struct fwnode_handle *fwnode,
+const char *propname, u32 *val,
+size_t nval);
+int fwnode_property_read_u64_array(struct fwnode_handle *fwnode,
+const char *propname, u64 *val,
+size_t nval);
+int fwnode_property_read_string_array(struct fwnode_handle *fwnode,
+ const char *propname, char **val,
+ size_t nval);
+
+struct fwnode_handle *device_get_next_child_node(struct device *dev,
+ struct fwnode_handle *child);
+
+#define device_for_each_child_node(dev, child) \
+ for (child = device_get_next_child_node(dev, NULL); child; \
+ child = device_get_next_child_node(dev, child))
+
+void fwnode_handle_put(struct fwnode_handle *fwnode);
+
+unsigned int device_get_child_node_count(struct device *dev);
+
#endif /* _LINUX_PROPERTY_H_ */
Index: linux-pm/include/acpi/acpi_bus.h
===
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -27,6 +27,7 @@
#define __ACPI_BUS_H__
#include linux/device.h
+#include linux/property.h
/* TBD: Make dynamic */
#define ACPI_MAX_HANDLES 10
@@ -348,6 +349,7 @@ struct acpi_device_data {
struct acpi_device {
int device_type;
acpi_handle handle; /* no handle for fixed hardware */
+ struct fwnode_handle fwnode;
struct acpi_device *parent;
struct list_head children;
struct list_head node;
@@ -372,6 +374,16 @@ struct acpi_device {
void (*remove)(struct acpi_device *);
};
+static inline bool is_acpi_node(struct fwnode_handle *fwnode)
+{
+ return fwnode fwnode-type == FWNODE_ACPI;
+}
+
+static inline struct acpi_device *acpi_node(struct fwnode_handle *fwnode)
+{
+ return fwnode ? container_of(fwnode, struct acpi_device, fwnode
Re: [RFC v3 PATCH 1/5] of: Add standard property for poweroff capability
On Tue, 14 Oct 2014 06:31:09 +
, Romain Perier
wrote:
> Several drivers create their own devicetree property when they register
> poweroff capabilities. This is for example the case for mfd, regulator
> or power drivers which define "vendor,system-power-controller" property.
> This patch adds support for a standard property "poweroff-source"
> which marks the device as able to shutdown the system.
>
> Signed-off-by: Romain Perier
Acked-by: Grant Likely
And same for the documentation patches.
g.
> ---
> include/linux/of.h | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 6545e7a..27b3ba1 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -866,4 +866,15 @@ static inline int of_changeset_update_property(struct
> of_changeset *ocs,
> /* CONFIG_OF_RESOLVE api */
> extern int of_resolve_phandles(struct device_node *tree);
>
> +/**
> + * of_system_has_poweroff_source - Tells if poweroff-source is found for
> device_node
> + * @np: Pointer to the given device_node
> + *
> + * return true if present false otherwise
> + */
> +static inline bool of_system_has_poweroff_source(const struct device_node
> *np)
> +{
> + return of_property_read_bool(np, "poweroff-source");
> +}
> +
> #endif /* _LINUX_OF_H */
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC v3 PATCH 1/5] of: Add standard property for poweroff capability
On Tue, 14 Oct 2014 06:31:09 +
, Romain Perier romain.per...@gmail.com
wrote:
Several drivers create their own devicetree property when they register
poweroff capabilities. This is for example the case for mfd, regulator
or power drivers which define vendor,system-power-controller property.
This patch adds support for a standard property poweroff-source
which marks the device as able to shutdown the system.
Signed-off-by: Romain Perier romain.per...@gmail.com
Acked-by: Grant Likely grant.lik...@linaro.org
And same for the documentation patches.
g.
---
include/linux/of.h | 11 +++
1 file changed, 11 insertions(+)
diff --git a/include/linux/of.h b/include/linux/of.h
index 6545e7a..27b3ba1 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -866,4 +866,15 @@ static inline int of_changeset_update_property(struct
of_changeset *ocs,
/* CONFIG_OF_RESOLVE api */
extern int of_resolve_phandles(struct device_node *tree);
+/**
+ * of_system_has_poweroff_source - Tells if poweroff-source is found for
device_node
+ * @np: Pointer to the given device_node
+ *
+ * return true if present false otherwise
+ */
+static inline bool of_system_has_poweroff_source(const struct device_node
*np)
+{
+ return of_property_read_bool(np, poweroff-source);
+}
+
#endif /* _LINUX_OF_H */
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 06/13] gpio / ACPI: Add support for _DSD device properties
On Tue, 07 Oct 2014 02:15:18 +0200
, "Rafael J. Wysocki"
wrote:
> From: Mika Westerberg
>
> With release of ACPI 5.1 and _DSD method we can finally name GPIOs (and
> other things as well) returned by _CRS. Previously we were only able to
> use integer index to find the corresponding GPIO, which is pretty error
> prone if the order changes.
>
> With _DSD we can now query GPIOs using name instead of an integer index,
> like the below example shows:
>
> // Bluetooth device with reset and shutdown GPIOs
> Device (BTH)
> {
> Name (_HID, ...)
>
> Name (_CRS, ResourceTemplate ()
> {
> GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
> "\\_SB.GPO0", 0, ResourceConsumer) {15}
> GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
> "\\_SB.GPO0", 0, ResourceConsumer) {27, 31}
> })
>
> Name (_DSD, Package ()
> {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package ()
> {
> Package () {"reset-gpio", Package() {^BTH, 1, 1, 0 }},
> Package () {"shutdown-gpio", Package() {^BTH, 0, 0, 0 }},
> }
> })
> }
>
> The format of the supported GPIO property is:
>
> Package () { "name", Package () { ref, index, pin, active_low }}
>
> ref - The device that has _CRS containing GpioIo()/GpioInt() resources,
> typically this is the device itself (BTH in our case).
> index - Index of the GpioIo()/GpioInt() resource in _CRS starting from zero.
> pin - Pin in the GpioIo()/GpioInt() resource. Typically this is zero.
> active_low - If 1 the GPIO is marked as active_low.
>
> Since ACPI GpioIo() resource does not have field saying whether it is
> active low or high, the "active_low" argument can be used here. Setting
> it to 1 marks the GPIO as active low.
>
> In our Bluetooth example the "reset-gpio" refers to the second GpioIo()
> resource, second pin in that resource with the GPIO number of 31.
>
> This patch implements necessary support to gpiolib for extracting GPIOs
> using _DSD device properties.
Patch looks good, but please put the above description into
/Documentation until we've got a better place to document extra bindings.
g.
>
> Signed-off-by: Mika Westerberg
> Acked-by: Linus Walleij
> Signed-off-by: Rafael J. Wysocki
> ---
> drivers/gpio/gpiolib-acpi.c | 78
> +
> drivers/gpio/gpiolib.c | 30 ++---
> drivers/gpio/gpiolib.h | 7 ++--
> 3 files changed, 94 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> index 687476f..b14c045 100644
> --- a/drivers/gpio/gpiolib-acpi.c
> +++ b/drivers/gpio/gpiolib-acpi.c
> @@ -293,6 +293,7 @@ void acpi_gpiochip_free_interrupts(struct gpio_chip *chip)
> struct acpi_gpio_lookup {
> struct acpi_gpio_info info;
> int index;
> + int pin_index;
> struct gpio_desc *desc;
> int n;
> };
> @@ -306,13 +307,24 @@ static int acpi_find_gpio(struct acpi_resource *ares,
> void *data)
>
> if (lookup->n++ == lookup->index && !lookup->desc) {
> const struct acpi_resource_gpio *agpio = >data.gpio;
> + int pin_index = lookup->pin_index;
> +
> + if (pin_index >= agpio->pin_table_length)
> + return 1;
>
> lookup->desc = acpi_get_gpiod(agpio->resource_source.string_ptr,
> - agpio->pin_table[0]);
> + agpio->pin_table[pin_index]);
> lookup->info.gpioint =
> agpio->connection_type == ACPI_RESOURCE_GPIO_TYPE_INT;
> - lookup->info.active_low =
> - agpio->polarity == ACPI_ACTIVE_LOW;
> +
> + /*
> + * ActiveLow is only specified for GpioInt resource. If
> + * GpioIo is used then the only way to set the flag is
> + * to use _DSD "gpios" property.
> + */
> + if (lookup->info.gpioint)
> + lookup->info.active_low =
> + agpio->polarity == ACPI_ACTIVE_LOW;
> }
>
> return 1;
> @@ -320,40 +332,75 @@ static int acpi_find_gpio(struct acpi_resource *ares,
> void *data)
>
> /**
> * acpi_get_gpiod_by_index() - get a GPIO descriptor from device resources
> - * @dev: pointer to a device to get GPIO from
> + * @adev: pointer to a ACPI device to get GPIO from
> + * @propname: Property name of the GPIO (optional)
> * @index: index of GpioIo/GpioInt resource (starting from %0)
> * @info: info pointer to fill in (optional)
> *
> - * Function goes through ACPI resources for @dev and based on @index looks
> + * Function goes through ACPI resources for @adev and based on @index looks
> * up a GpioIo/GpioInt resource, translates it to the Linux GPIO descriptor,
> * and returns it.
Re: [PATCH 03/13] ACPI: Allow drivers to match using Device Tree compatible property
On Tue, 07 Oct 2014 02:13:29 +0200
, "Rafael J. Wysocki"
wrote:
> From: Mika Westerberg
>
> We have lots of existing Device Tree enabled drivers and allocating
> separate _HID for each is not feasible. Instead we allocate special _HID
> "PRP0001" that means that the match should be done using Device Tree
> compatible property using driver's .of_match_table instead if the driver
> is missing .acpi_match_table.
>
> If there is a need to distinguish from where the device is enumerated
> (DT/ACPI) driver can check dev->of_node or ACPI_COMPATION(dev).
>
> Signed-off-by: Mika Westerberg
> Signed-off-by: Rafael J. Wysocki
Looks okay to me.
Reviewed-by: Grant Likely
> ---
> drivers/acpi/property.c | 34 +
> drivers/acpi/scan.c | 97
> +++--
> include/acpi/acpi_bus.h | 1 +
> include/linux/acpi.h| 8 +---
> 4 files changed, 123 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> index 680f7f1..ff53eb8 100644
> --- a/drivers/acpi/property.c
> +++ b/drivers/acpi/property.c
> @@ -76,6 +76,37 @@ static bool acpi_properties_format_valid(const union
> acpi_object *properties)
> return true;
> }
>
> +static void acpi_init_of_compatible(struct acpi_device *adev)
> +{
> + const union acpi_object *of_compatible;
> + struct acpi_hardware_id *hwid;
> + bool acpi_of = false;
> +
> + /*
> + * Check if the special PRP0001 ACPI ID is present and in that
> + * case we fill in Device Tree compatible properties for this
> + * device.
> + */
> + list_for_each_entry(hwid, >pnp.ids, list) {
> + if (!strcmp(hwid->id, "PRP0001")) {
> + acpi_of = true;
> + break;
> + }
> + }
> +
> + if (!acpi_of)
> + return;
> +
> + if (acpi_dev_get_property_array(adev, "compatible", ACPI_TYPE_STRING,
> + _compatible)) {
> + acpi_handle_warn(adev->handle,
> + "PRP0001 requires compatible property\n");
> + return;
> + }
> +
> + adev->data.of_compatible = of_compatible;
> +}
> +
> void acpi_init_properties(struct acpi_device *adev)
> {
> struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER };
> @@ -119,6 +150,8 @@ void acpi_init_properties(struct acpi_device *adev)
>
> adev->data.pointer = buf.pointer;
> adev->data.properties = properties;
> +
> + acpi_init_of_compatible(adev);
> return;
> }
>
> @@ -130,6 +163,7 @@ void acpi_init_properties(struct acpi_device *adev)
> void acpi_free_properties(struct acpi_device *adev)
> {
> ACPI_FREE((void *)adev->data.pointer);
> + adev->data.of_compatible = NULL;
> adev->data.pointer = NULL;
> adev->data.properties = NULL;
> }
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 1979ab3..bc999f1 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -124,17 +124,51 @@ static int create_modalias(struct acpi_device
> *acpi_dev, char *modalias,
> if (list_empty(_dev->pnp.ids))
> return 0;
>
> - len = snprintf(modalias, size, "acpi:");
> - size -= len;
> -
> - list_for_each_entry(id, _dev->pnp.ids, list) {
> - count = snprintf([len], size, "%s:", id->id);
> - if (count < 0)
> - return -EINVAL;
> - if (count >= size)
> - return -ENOMEM;
> - len += count;
> - size -= count;
> + /*
> + * If the device has PRP0001 we expose DT compatible modalias
> + * instead in form of of:NnameTCcompatible.
> + */
> + if (acpi_dev->data.of_compatible) {
> + struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER };
> + const union acpi_object *of_compatible, *obj;
> + char *c;
> + int i;
> +
> + acpi_get_name(acpi_dev->handle, ACPI_SINGLE_NAME, );
> + /* DT strings are all in lower case */
> + for (c = buf.pointer; *c != '\0'; c++)
> + *c = tolower(*c);
> +
> + len = snprintf(modalias, size, "of:N%sT", (char *)buf.pointer);
> + ACPI_FREE(buf.pointer);
> +
> + of_compatible = acpi_dev->data.of_compatible;
> + for (i = 0; i < of_compatible->package.count; i++) {
> +
Re: [PATCH 03/13] ACPI: Allow drivers to match using Device Tree compatible property
On Tue, 07 Oct 2014 02:13:29 +0200
, Rafael J. Wysocki r...@rjwysocki.net
wrote:
From: Mika Westerberg mika.westerb...@linux.intel.com
We have lots of existing Device Tree enabled drivers and allocating
separate _HID for each is not feasible. Instead we allocate special _HID
PRP0001 that means that the match should be done using Device Tree
compatible property using driver's .of_match_table instead if the driver
is missing .acpi_match_table.
If there is a need to distinguish from where the device is enumerated
(DT/ACPI) driver can check dev-of_node or ACPI_COMPATION(dev).
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
Looks okay to me.
Reviewed-by: Grant Likely grant.lik...@linaro.org
---
drivers/acpi/property.c | 34 +
drivers/acpi/scan.c | 97
+++--
include/acpi/acpi_bus.h | 1 +
include/linux/acpi.h| 8 +---
4 files changed, 123 insertions(+), 17 deletions(-)
diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 680f7f1..ff53eb8 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -76,6 +76,37 @@ static bool acpi_properties_format_valid(const union
acpi_object *properties)
return true;
}
+static void acpi_init_of_compatible(struct acpi_device *adev)
+{
+ const union acpi_object *of_compatible;
+ struct acpi_hardware_id *hwid;
+ bool acpi_of = false;
+
+ /*
+ * Check if the special PRP0001 ACPI ID is present and in that
+ * case we fill in Device Tree compatible properties for this
+ * device.
+ */
+ list_for_each_entry(hwid, adev-pnp.ids, list) {
+ if (!strcmp(hwid-id, PRP0001)) {
+ acpi_of = true;
+ break;
+ }
+ }
+
+ if (!acpi_of)
+ return;
+
+ if (acpi_dev_get_property_array(adev, compatible, ACPI_TYPE_STRING,
+ of_compatible)) {
+ acpi_handle_warn(adev-handle,
+ PRP0001 requires compatible property\n);
+ return;
+ }
+
+ adev-data.of_compatible = of_compatible;
+}
+
void acpi_init_properties(struct acpi_device *adev)
{
struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER };
@@ -119,6 +150,8 @@ void acpi_init_properties(struct acpi_device *adev)
adev-data.pointer = buf.pointer;
adev-data.properties = properties;
+
+ acpi_init_of_compatible(adev);
return;
}
@@ -130,6 +163,7 @@ void acpi_init_properties(struct acpi_device *adev)
void acpi_free_properties(struct acpi_device *adev)
{
ACPI_FREE((void *)adev-data.pointer);
+ adev-data.of_compatible = NULL;
adev-data.pointer = NULL;
adev-data.properties = NULL;
}
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 1979ab3..bc999f1 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -124,17 +124,51 @@ static int create_modalias(struct acpi_device
*acpi_dev, char *modalias,
if (list_empty(acpi_dev-pnp.ids))
return 0;
- len = snprintf(modalias, size, acpi:);
- size -= len;
-
- list_for_each_entry(id, acpi_dev-pnp.ids, list) {
- count = snprintf(modalias[len], size, %s:, id-id);
- if (count 0)
- return -EINVAL;
- if (count = size)
- return -ENOMEM;
- len += count;
- size -= count;
+ /*
+ * If the device has PRP0001 we expose DT compatible modalias
+ * instead in form of of:NnameTCcompatible.
+ */
+ if (acpi_dev-data.of_compatible) {
+ struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER };
+ const union acpi_object *of_compatible, *obj;
+ char *c;
+ int i;
+
+ acpi_get_name(acpi_dev-handle, ACPI_SINGLE_NAME, buf);
+ /* DT strings are all in lower case */
+ for (c = buf.pointer; *c != '\0'; c++)
+ *c = tolower(*c);
+
+ len = snprintf(modalias, size, of:N%sT, (char *)buf.pointer);
+ ACPI_FREE(buf.pointer);
+
+ of_compatible = acpi_dev-data.of_compatible;
+ for (i = 0; i of_compatible-package.count; i++) {
+ obj = of_compatible-package.elements[i];
+
+ count = snprintf(modalias[len], size, C%s,
+ obj-string.pointer);
+ if (count 0)
+ return -EINVAL;
+ if (count = size)
+ return -ENOMEM;
+
+ len += count;
+ size -= count;
+ }
+ } else {
+ len = snprintf(modalias, size
Re: [PATCH 06/13] gpio / ACPI: Add support for _DSD device properties
On Tue, 07 Oct 2014 02:15:18 +0200
, Rafael J. Wysocki r...@rjwysocki.net
wrote:
From: Mika Westerberg mika.westerb...@linux.intel.com
With release of ACPI 5.1 and _DSD method we can finally name GPIOs (and
other things as well) returned by _CRS. Previously we were only able to
use integer index to find the corresponding GPIO, which is pretty error
prone if the order changes.
With _DSD we can now query GPIOs using name instead of an integer index,
like the below example shows:
// Bluetooth device with reset and shutdown GPIOs
Device (BTH)
{
Name (_HID, ...)
Name (_CRS, ResourceTemplate ()
{
GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
\\_SB.GPO0, 0, ResourceConsumer) {15}
GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
\\_SB.GPO0, 0, ResourceConsumer) {27, 31}
})
Name (_DSD, Package ()
{
ToUUID(daffd814-6eba-4d8c-8a91-bc9bbf4aa301),
Package ()
{
Package () {reset-gpio, Package() {^BTH, 1, 1, 0 }},
Package () {shutdown-gpio, Package() {^BTH, 0, 0, 0 }},
}
})
}
The format of the supported GPIO property is:
Package () { name, Package () { ref, index, pin, active_low }}
ref - The device that has _CRS containing GpioIo()/GpioInt() resources,
typically this is the device itself (BTH in our case).
index - Index of the GpioIo()/GpioInt() resource in _CRS starting from zero.
pin - Pin in the GpioIo()/GpioInt() resource. Typically this is zero.
active_low - If 1 the GPIO is marked as active_low.
Since ACPI GpioIo() resource does not have field saying whether it is
active low or high, the active_low argument can be used here. Setting
it to 1 marks the GPIO as active low.
In our Bluetooth example the reset-gpio refers to the second GpioIo()
resource, second pin in that resource with the GPIO number of 31.
This patch implements necessary support to gpiolib for extracting GPIOs
using _DSD device properties.
Patch looks good, but please put the above description into
/Documentation until we've got a better place to document extra bindings.
g.
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Acked-by: Linus Walleij linus.wall...@linaro.org
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
drivers/gpio/gpiolib-acpi.c | 78
+
drivers/gpio/gpiolib.c | 30 ++---
drivers/gpio/gpiolib.h | 7 ++--
3 files changed, 94 insertions(+), 21 deletions(-)
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 687476f..b14c045 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -293,6 +293,7 @@ void acpi_gpiochip_free_interrupts(struct gpio_chip *chip)
struct acpi_gpio_lookup {
struct acpi_gpio_info info;
int index;
+ int pin_index;
struct gpio_desc *desc;
int n;
};
@@ -306,13 +307,24 @@ static int acpi_find_gpio(struct acpi_resource *ares,
void *data)
if (lookup-n++ == lookup-index !lookup-desc) {
const struct acpi_resource_gpio *agpio = ares-data.gpio;
+ int pin_index = lookup-pin_index;
+
+ if (pin_index = agpio-pin_table_length)
+ return 1;
lookup-desc = acpi_get_gpiod(agpio-resource_source.string_ptr,
- agpio-pin_table[0]);
+ agpio-pin_table[pin_index]);
lookup-info.gpioint =
agpio-connection_type == ACPI_RESOURCE_GPIO_TYPE_INT;
- lookup-info.active_low =
- agpio-polarity == ACPI_ACTIVE_LOW;
+
+ /*
+ * ActiveLow is only specified for GpioInt resource. If
+ * GpioIo is used then the only way to set the flag is
+ * to use _DSD gpios property.
+ */
+ if (lookup-info.gpioint)
+ lookup-info.active_low =
+ agpio-polarity == ACPI_ACTIVE_LOW;
}
return 1;
@@ -320,40 +332,75 @@ static int acpi_find_gpio(struct acpi_resource *ares,
void *data)
/**
* acpi_get_gpiod_by_index() - get a GPIO descriptor from device resources
- * @dev: pointer to a device to get GPIO from
+ * @adev: pointer to a ACPI device to get GPIO from
+ * @propname: Property name of the GPIO (optional)
* @index: index of GpioIo/GpioInt resource (starting from %0)
* @info: info pointer to fill in (optional)
*
- * Function goes through ACPI resources for @dev and based on @index looks
+ * Function goes through ACPI resources for @adev and based on @index looks
* up a GpioIo/GpioInt resource, translates it to the Linux GPIO descriptor,
* and returns it. @index matches GpioIo/GpioInt
Re: [PATCH 01/13] ACPI: Add support for device specific properties
On Tue, 07 Oct 2014 02:12:23 +0200
, "Rafael J. Wysocki"
wrote:
> From: Mika Westerberg
>
> Device Tree is used in many embedded systems to describe the system
> configuration to the OS. It supports attaching properties or name-value
> pairs to the devices it describe. With these properties one can pass
> additional information to the drivers that would not be available
> otherwise.
>
> ACPI is another configuration mechanism (among other things) typically
> seen, but not limited to, x86 machines. ACPI allows passing arbitrary
> data from methods but there has not been mechanism equivalent to Device
> Tree until the introduction of _DSD in the recent publication of the
> ACPI 5.1 specification.
>
> In order to facilitate ACPI usage in systems where Device Tree is
> typically used, it would be beneficial to standardize a way to retrieve
> Device Tree style properties from ACPI devices, which is what we do in
> this patch.
>
> If a given device described in ACPI namespace wants to export properties it
> must implement _DSD method (Device Specific Data, introduced with ACPI 5.1)
> that returns the properties in a package of packages. For example:
>
> Name (_DSD, Package () {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package () {
> Package () {"name1", },
> Package () {"name2", },
> ...
> }
> })
>
> The UUID reserved for properties is daffd814-6eba-4d8c-8a91-bc9bbf4aa301
> and is documented in the ACPI 5.1 companion document called "_DSD
> Implementation Guide" [1], [2].
>
> We add several helper functions that can be used to extract these
> properties and convert them to different Linux data types.
>
> The ultimate goal is that we only have one device property API that
> retrieves the requested properties from Device Tree or from ACPI
> transparent to the caller.
>
> [1]
> http://www.uefi.org/sites/default/files/resources/_DSD-implementation-guide-toplevel.htm
> [2]
> http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
>
> Reviewed-by: Hanjun Guo
> Reviewed-by: Josh Triplett
> Signed-off-by: Darren Hart
> Signed-off-by: Rafael J. Wysocki
> Signed-off-by: Mika Westerberg
> Signed-off-by: Rafael J. Wysocki
This patch seems fine.
Reviewed-by: Grant Likely
> ---
> drivers/acpi/Makefile |1
> drivers/acpi/internal.h |6
> drivers/acpi/property.c | 364
>
> drivers/acpi/scan.c |2
> include/acpi/acpi_bus.h |7
> include/linux/acpi.h| 40 +
> 6 files changed, 420 insertions(+)
> create mode 100644 drivers/acpi/property.c
>
> Index: linux-pm/drivers/acpi/Makefile
> ===
> --- linux-pm.orig/drivers/acpi/Makefile
> +++ linux-pm/drivers/acpi/Makefile
> @@ -46,6 +46,7 @@ acpi-y += acpi_pnp.o
> acpi-y += power.o
> acpi-y += event.o
> acpi-y += sysfs.o
> +acpi-y += property.o
> acpi-$(CONFIG_X86) += acpi_cmos_rtc.o
> acpi-$(CONFIG_DEBUG_FS) += debugfs.o
> acpi-$(CONFIG_ACPI_NUMA) += numa.o
> Index: linux-pm/drivers/acpi/internal.h
> ===
> --- linux-pm.orig/drivers/acpi/internal.h
> +++ linux-pm/drivers/acpi/internal.h
> @@ -181,4 +181,10 @@ struct platform_device *acpi_create_plat
> bool acpi_osi_is_win8(void);
> #endif
>
> +/*--
> + Device properties
> + --
> */
> +void acpi_init_properties(struct acpi_device *adev);
> +void acpi_free_properties(struct acpi_device *adev);
> +
> #endif /* _ACPI_INTERNAL_H_ */
> Index: linux-pm/drivers/acpi/property.c
> ===
> --- /dev/null
> +++ linux-pm/drivers/acpi/property.c
> @@ -0,0 +1,364 @@
> +/*
> + * ACPI device specific properties support.
> + *
> + * Copyright (C) 2014, Intel Corporation
> + * All rights reserved.
> + *
> + * Authors: Mika Westerberg
> + * Darren Hart
> + * Rafael J. Wysocki
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free S
Re: [PATCH 04/13] ACPI: Document ACPI device specific properties
On Tue, 07 Oct 2014 02:14:06 +0200
, "Rafael J. Wysocki"
wrote:
> From: Mika Westerberg
>
> This document describes the data format and interfaces of ACPI device
> specific properties.
>
> Signed-off-by: Mika Westerberg
> Signed-off-by: Darren Hart
> Signed-off-by: Rafael J. Wysocki
> ---
> Documentation/acpi/properties.txt | 376
> ++
> 1 file changed, 376 insertions(+)
> create mode 100644 Documentation/acpi/properties.txt
>
> Index: linux-pm/Documentation/acpi/properties.txt
> ===
> --- /dev/null
> +++ linux-pm/Documentation/acpi/properties.txt
> @@ -0,0 +1,376 @@
> +ACPI device properties
> +==
> +This document describes the format and interfaces of ACPI device
> +properties as specified in "Device Properties UUID For _DSD" available
> +here:
> +
> +http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
> +
> +1. Introduction
> +---
> +In systems that use ACPI and want to take advantage of device specific
> +properties, there needs to be a standard way to return and extract
> +name-value pairs for a given ACPI device.
> +
> +An ACPI device that wants to export its properties must implement a
> +static name called _DSD that takes no arguments and returns a package of
> +packages:
> +
> + Name (_DSD, Package () {
> + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> + Package () {
> + Package () {"name1", },
> + Package () {"name2", }
> + }
> + })
> +
> +The UUID identifies contents of the following package. In case of ACPI
> +device properties it is daffd814-6eba-4d8c-8a91-bc9bbf4aa301.
> +
> +In each returned package, the first item is the name and must be a string.
> +The corresponding value can be a string, integer, reference, or package. If
> +a package it may only contain strings, integers, and references.
> +
> +An example device where we might need properties is a device that uses
> +GPIOs. In addition to the GpioIo/GpioInt resources the driver needs to
> +know which GPIO is used for which purpose.
> +
> +To solve this we add the following ACPI device properties to the device:
> +
> + Device (DEV0)
> + {
> + Name (_CRS, ResourceTemplate () {
> + GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
> + "\\_SB.PCI0.LPC", 0, ResourceConsumer) {0}
> + GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
> + "\\_SB.PCI0.LPC", 0, ResourceConsumer) {1}
> + ...
> + })
> +
> + Name (_DSD, Package () {
> + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> + Package () {
> + Package () {"reset-gpio", {^DEV0, 0, 0, 0}},
> + Package () {"shutdown-gpio", {^DEV0, 1, 0, 0}},
> + }
> + })
> + }
> +
> +Now the device driver can reference the GPIOs using names instead of
> +using indexes.
Ummm... so this is kind of odd. Why would arguments be needed for the
gpio reference, or a device trying to reference itself (^DEV0) within
it's own device.
It looks like it is trying to shoehorn DT design patterns into the ACPI
tables when ACPI already has a native method for doing the same thing.
That concerns me.
[...]
> +In addition to simple object references it is also possible to have object
> +references with arguments. These are represented in ASL as follows:
> +
> + Device (\_SB.PCI0.PWM)
> + {
> + Name (_DSD, Package () {
> + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> + Package () {
> + Package () {"#pwm-cells", 2}
> + }
> + })
> + }
> +
> + Device (\_SB.PCI0.BL)
> + {
> + Name (_DSD, Package () {
> + ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> + Package () {
> + Package () {
> + "pwms",
> + Package () {
> + \_SB.PCI0.PWM, 0, 500,
> + \_SB.PCI0.PWM, 1, 450,
> + }
> + }
> + }
> + })
> + }
This worries me even more. #x-cells is a very devicetree oriented
pattern that looks wrong for ACPI data. I would expect PWM resources in
ACPI to look much like ACPI GPIO resources or Serial bus resources.
g.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/13] ACPI: Document ACPI device specific properties
On Tue, 07 Oct 2014 02:14:06 +0200
, Rafael J. Wysocki r...@rjwysocki.net
wrote:
From: Mika Westerberg mika.westerb...@linux.intel.com
This document describes the data format and interfaces of ACPI device
specific properties.
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Signed-off-by: Darren Hart dvh...@linux.intel.com
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
---
Documentation/acpi/properties.txt | 376
++
1 file changed, 376 insertions(+)
create mode 100644 Documentation/acpi/properties.txt
Index: linux-pm/Documentation/acpi/properties.txt
===
--- /dev/null
+++ linux-pm/Documentation/acpi/properties.txt
@@ -0,0 +1,376 @@
+ACPI device properties
+==
+This document describes the format and interfaces of ACPI device
+properties as specified in Device Properties UUID For _DSD available
+here:
+
+http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
+
+1. Introduction
+---
+In systems that use ACPI and want to take advantage of device specific
+properties, there needs to be a standard way to return and extract
+name-value pairs for a given ACPI device.
+
+An ACPI device that wants to export its properties must implement a
+static name called _DSD that takes no arguments and returns a package of
+packages:
+
+ Name (_DSD, Package () {
+ ToUUID(daffd814-6eba-4d8c-8a91-bc9bbf4aa301),
+ Package () {
+ Package () {name1, VALUE1},
+ Package () {name2, VALUE2}
+ }
+ })
+
+The UUID identifies contents of the following package. In case of ACPI
+device properties it is daffd814-6eba-4d8c-8a91-bc9bbf4aa301.
+
+In each returned package, the first item is the name and must be a string.
+The corresponding value can be a string, integer, reference, or package. If
+a package it may only contain strings, integers, and references.
+
+An example device where we might need properties is a device that uses
+GPIOs. In addition to the GpioIo/GpioInt resources the driver needs to
+know which GPIO is used for which purpose.
+
+To solve this we add the following ACPI device properties to the device:
+
+ Device (DEV0)
+ {
+ Name (_CRS, ResourceTemplate () {
+ GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
+ \\_SB.PCI0.LPC, 0, ResourceConsumer) {0}
+ GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
+ \\_SB.PCI0.LPC, 0, ResourceConsumer) {1}
+ ...
+ })
+
+ Name (_DSD, Package () {
+ ToUUID(daffd814-6eba-4d8c-8a91-bc9bbf4aa301),
+ Package () {
+ Package () {reset-gpio, {^DEV0, 0, 0, 0}},
+ Package () {shutdown-gpio, {^DEV0, 1, 0, 0}},
+ }
+ })
+ }
+
+Now the device driver can reference the GPIOs using names instead of
+using indexes.
Ummm... so this is kind of odd. Why would arguments be needed for the
gpio reference, or a device trying to reference itself (^DEV0) within
it's own device.
It looks like it is trying to shoehorn DT design patterns into the ACPI
tables when ACPI already has a native method for doing the same thing.
That concerns me.
[...]
+In addition to simple object references it is also possible to have object
+references with arguments. These are represented in ASL as follows:
+
+ Device (\_SB.PCI0.PWM)
+ {
+ Name (_DSD, Package () {
+ ToUUID(daffd814-6eba-4d8c-8a91-bc9bbf4aa301),
+ Package () {
+ Package () {#pwm-cells, 2}
+ }
+ })
+ }
+
+ Device (\_SB.PCI0.BL)
+ {
+ Name (_DSD, Package () {
+ ToUUID(daffd814-6eba-4d8c-8a91-bc9bbf4aa301),
+ Package () {
+ Package () {
+ pwms,
+ Package () {
+ \_SB.PCI0.PWM, 0, 500,
+ \_SB.PCI0.PWM, 1, 450,
+ }
+ }
+ }
+ })
+ }
This worries me even more. #x-cells is a very devicetree oriented
pattern that looks wrong for ACPI data. I would expect PWM resources in
ACPI to look much like ACPI GPIO resources or Serial bus resources.
g.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at
Re: [PATCH 01/13] ACPI: Add support for device specific properties
On Tue, 07 Oct 2014 02:12:23 +0200
, Rafael J. Wysocki r...@rjwysocki.net
wrote:
From: Mika Westerberg mika.westerb...@linux.intel.com
Device Tree is used in many embedded systems to describe the system
configuration to the OS. It supports attaching properties or name-value
pairs to the devices it describe. With these properties one can pass
additional information to the drivers that would not be available
otherwise.
ACPI is another configuration mechanism (among other things) typically
seen, but not limited to, x86 machines. ACPI allows passing arbitrary
data from methods but there has not been mechanism equivalent to Device
Tree until the introduction of _DSD in the recent publication of the
ACPI 5.1 specification.
In order to facilitate ACPI usage in systems where Device Tree is
typically used, it would be beneficial to standardize a way to retrieve
Device Tree style properties from ACPI devices, which is what we do in
this patch.
If a given device described in ACPI namespace wants to export properties it
must implement _DSD method (Device Specific Data, introduced with ACPI 5.1)
that returns the properties in a package of packages. For example:
Name (_DSD, Package () {
ToUUID(daffd814-6eba-4d8c-8a91-bc9bbf4aa301),
Package () {
Package () {name1, VALUE1},
Package () {name2, VALUE2},
...
}
})
The UUID reserved for properties is daffd814-6eba-4d8c-8a91-bc9bbf4aa301
and is documented in the ACPI 5.1 companion document called _DSD
Implementation Guide [1], [2].
We add several helper functions that can be used to extract these
properties and convert them to different Linux data types.
The ultimate goal is that we only have one device property API that
retrieves the requested properties from Device Tree or from ACPI
transparent to the caller.
[1]
http://www.uefi.org/sites/default/files/resources/_DSD-implementation-guide-toplevel.htm
[2]
http://www.uefi.org/sites/default/files/resources/_DSD-device-properties-UUID.pdf
Reviewed-by: Hanjun Guo hanjun@linaro.org
Reviewed-by: Josh Triplett j...@joshtriplett.org
Signed-off-by: Darren Hart dvh...@linux.intel.com
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
Signed-off-by: Mika Westerberg mika.westerb...@linux.intel.com
Signed-off-by: Rafael J. Wysocki rafael.j.wyso...@intel.com
This patch seems fine.
Reviewed-by: Grant Likely grant.lik...@linaro.org
---
drivers/acpi/Makefile |1
drivers/acpi/internal.h |6
drivers/acpi/property.c | 364
drivers/acpi/scan.c |2
include/acpi/acpi_bus.h |7
include/linux/acpi.h| 40 +
6 files changed, 420 insertions(+)
create mode 100644 drivers/acpi/property.c
Index: linux-pm/drivers/acpi/Makefile
===
--- linux-pm.orig/drivers/acpi/Makefile
+++ linux-pm/drivers/acpi/Makefile
@@ -46,6 +46,7 @@ acpi-y += acpi_pnp.o
acpi-y += power.o
acpi-y += event.o
acpi-y += sysfs.o
+acpi-y += property.o
acpi-$(CONFIG_X86) += acpi_cmos_rtc.o
acpi-$(CONFIG_DEBUG_FS) += debugfs.o
acpi-$(CONFIG_ACPI_NUMA) += numa.o
Index: linux-pm/drivers/acpi/internal.h
===
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -181,4 +181,10 @@ struct platform_device *acpi_create_plat
bool acpi_osi_is_win8(void);
#endif
+/*--
+ Device properties
+ --
*/
+void acpi_init_properties(struct acpi_device *adev);
+void acpi_free_properties(struct acpi_device *adev);
+
#endif /* _ACPI_INTERNAL_H_ */
Index: linux-pm/drivers/acpi/property.c
===
--- /dev/null
+++ linux-pm/drivers/acpi/property.c
@@ -0,0 +1,364 @@
+/*
+ * ACPI device specific properties support.
+ *
+ * Copyright (C) 2014, Intel Corporation
+ * All rights reserved.
+ *
+ * Authors: Mika Westerberg mika.westerb...@linux.intel.com
+ * Darren Hart dvh...@linux.intel.com
+ * Rafael J. Wysocki rafael.j.wyso...@intel.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include linux/acpi.h
+#include linux/device.h
+#include linux/export.h
+
+#include internal.h
+
+/* ACPI _DSD device properties UUID: daffd814-6eba-4d8c-8a91
Re: [RFC PATCH v2 1/4] regulator: Add helper function to get "poweroff-source" property
On Fri, Oct 10, 2014 at 1:29 PM, PERIER Romain wrote: >> >> What I'm more concerned about is the semantics of the property. What >> do the generic code paths gain by standardizing this property? Is it >> expected that > > We really need to come up with a standard property for this and document > it rather than continuing to add individual device specific properties > each time a driver adds poweroff capability, > all doing the same thing (a lot of regulators driver, mfd drivers, soc > specific drivers, power drivers already do that, that's very redudant) > . This is a simple unification logic. Yes, it's fine. I accidentally left that paragraph in when I sent my reply. I started writing that concern, and then thought better of it. The property is fine. g. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH v2 1/4] regulator: Add helper function to get poweroff-source property
On Fri, Oct 10, 2014 at 1:29 PM, PERIER Romain romain.per...@gmail.com wrote: What I'm more concerned about is the semantics of the property. What do the generic code paths gain by standardizing this property? Is it expected that We really need to come up with a standard property for this and document it rather than continuing to add individual device specific properties each time a driver adds poweroff capability, all doing the same thing (a lot of regulators driver, mfd drivers, soc specific drivers, power drivers already do that, that's very redudant) . This is a simple unification logic. Yes, it's fine. I accidentally left that paragraph in when I sent my reply. I started writing that concern, and then thought better of it. The property is fine. g. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[GIT PULL] Devicetree changes for v3.18
Hi Linus, Here are the device tree changes I have queued up for v3.18. Pretty small set this time. Please pull. g. The following changes since commit fe82dcec644244676d55a1384c958d5f67979adb: Linux 3.17-rc7 (2014-09-28 14:29:07 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/glikely/linux tags/devicetree-for-linus for you to fetch changes up to 2118f4b8dfc666c3e4a9e262beca79636a0852fe: of/selftest: Move hash table off stack to fix large frame size (2014-10-07 11:30:31 +0100) Devicetree changes for v3.18 This branch contains bug fixes and new features for the devicetree code. Most of the changes are either new testcases for the selftest code or documentation changes. The most notable change is the addition of a phandle resolver for use when grafting in a second device tree blob into the core tree. The resolver isn't currently used by anything other than the selftest module, but it will be used to support device tree overlays; probably in the v3.19 timeframe. Also note that I've moved my normal tree from git.secretlab.ca to git.kernel.org. Gaurav Minocha (1): To remove non-ascii characters in of_selftest.txt Geert Uytterhoeven (1): of: Add vendor prefix for Sitronix Grant Likely (7): of: Fix NULL dereference in selftest removal code of/selftest: Test structure of device tree of: Don't try to search when phandle == 0 of/selftest: Add a test for duplicate phandles of/selftest: Use the resolver to fixup phandles Merge branch 'for-next' of git://git.kernel.org/.../robh/linux into devicetree/next of/selftest: Move hash table off stack to fix large frame size Pantelis Antoniou (1): of: Introduce Device Tree resolve support. Peter Chen (1): of: add vendor prefix for Chipidea Santosh Shilimkar (1): dt/documentation: add specification of dma bus information Thierry Reding (1): of: Add vendor prefix for Innolux Corporation Tim Harvey (1): devicetree: bindings: Document Gateworks vendor prefix Uwe Kleine-König (1): of: Add vendor prefix for Energy Micro .../devicetree/bindings/vendor-prefixes.txt| 5 + Documentation/devicetree/booting-without-of.txt| 53 .../devicetree/dynamic-resolution-notes.txt| 25 ++ Documentation/devicetree/of_selftest.txt | 30 +- drivers/of/Kconfig | 4 + drivers/of/Makefile| 1 + drivers/of/base.c | 3 + drivers/of/resolver.c | 336 + drivers/of/selftest.c | 120 +++- drivers/of/testcase-data/testcases.dts | 35 +++ include/linux/of.h | 3 + 11 files changed, 591 insertions(+), 24 deletions(-) create mode 100644 Documentation/devicetree/dynamic-resolution-notes.txt create mode 100644 drivers/of/resolver.c -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: error fetching the devicetree tree
Hi Stephen, Yes, the server is down and being worked on. I've moved my tree to kernel.org. Here is the new URL: git://git.kernel.org/pub/scm/linux/kernel/git/glikely/linux.git g. On Tue, Oct 7, 2014 at 11:48 PM, Stephen Rothwell wrote: > Hi Grant, > > Fetching the devicetree tree this morning produced this error: > > fatal: unable to connect to git.secretlab.ca: > git.secretlab.ca[0: 208.123.74.12]: errno=Connection timed out > > -- > Cheers, > Stephen Rothwells...@canb.auug.org.au -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH v2 1/4] regulator: Add helper function to get "poweroff-source" property
On Tue, Oct 7, 2014 at 8:43 PM, PERIER Romain wrote:
> This is not the final location, I have no idea exactly where I might
> put this helper function. This is why I ask you your opinion (and
> also, this is why that's a proposal)
>
> 2014-10-07 21:45 GMT+02:00 Romain Perier :
>> Several drivers create their own devicetree property when they register
>> poweroff capabilities. This is for example the case for mfd, regulator
>> or power drivers which define "vendor,system-power-controller" property.
>> This patch adds support for a standard property "poweroff-source"
>> which marks the device as able to shutdown the system.
>>
>> Signed-off-by: Romain Perier
>> ---
>> drivers/regulator/of_regulator.c | 12
>> include/linux/regulator/of_regulator.h | 6 ++
>> 2 files changed, 18 insertions(+)
>>
>> diff --git a/drivers/regulator/of_regulator.c
>> b/drivers/regulator/of_regulator.c
>> index 7a51814..8b898e6 100644
>> --- a/drivers/regulator/of_regulator.c
>> +++ b/drivers/regulator/of_regulator.c
>> @@ -240,3 +240,15 @@ struct regulator_init_data
>> *regulator_of_get_init_data(struct device *dev,
>>
>> return init_data;
>> }
>> +
>> +/**
>> + * is_system_poweroff_source - Tells if poweroff-source is found for
>> device_node
>> + * @np: Pointer to the given device_node
>> + *
>> + * return true if present false otherwise
>> + */
>> +bool is_system_poweroff_source(const struct device_node *np)
>> +{
>> + return of_property_read_bool(np, "poweroff-source");
>> +}
>> +EXPORT_SYMBOL_GPL(is_system_poweroff_source);
Hi Romain,
This is an extremely simple wrapper. It may be best to simply make it
a static inline. It is also generic enough that I don't have a problem
with it living in include/linux/of.h; but of_regulator.h would be fine
too.
Since this is a device tree specific function (it doesn't work with
ACPI or platform_data), you should prefix the function name with of_
g.
What I'm more concerned about is the semantics of the property. What
do the generic code paths gain by standardizing this property? Is it
expected that
>> diff --git a/include/linux/regulator/of_regulator.h
>> b/include/linux/regulator/of_regulator.h
>> index f921796..9d8fbb2 100644
>> --- a/include/linux/regulator/of_regulator.h
>> +++ b/include/linux/regulator/of_regulator.h
>> @@ -20,6 +20,7 @@ extern struct regulator_init_data
>> extern int of_regulator_match(struct device *dev, struct device_node *node,
>> struct of_regulator_match *matches,
>> unsigned int num_matches);
>> +extern bool is_system_poweroff_source(const struct device_node *np);
>> #else
>> static inline struct regulator_init_data
>> *of_get_regulator_init_data(struct device *dev,
>> @@ -35,6 +36,11 @@ static inline int of_regulator_match(struct device *dev,
>> {
>> return 0;
>> }
>> +
>> +static inline bool is_system_poweroff_source(const struct device_node *np)
>> +{
>> + return false;
>> +}
>> #endif /* CONFIG_OF */
>>
>> #endif /* __LINUX_OF_REG_H */
>> --
>> 1.9.1
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC PATCH v2 1/4] regulator: Add helper function to get poweroff-source property
On Tue, Oct 7, 2014 at 8:43 PM, PERIER Romain romain.per...@gmail.com wrote:
This is not the final location, I have no idea exactly where I might
put this helper function. This is why I ask you your opinion (and
also, this is why that's a proposal)
2014-10-07 21:45 GMT+02:00 Romain Perier romain.per...@gmail.com:
Several drivers create their own devicetree property when they register
poweroff capabilities. This is for example the case for mfd, regulator
or power drivers which define vendor,system-power-controller property.
This patch adds support for a standard property poweroff-source
which marks the device as able to shutdown the system.
Signed-off-by: Romain Perier romain.per...@gmail.com
---
drivers/regulator/of_regulator.c | 12
include/linux/regulator/of_regulator.h | 6 ++
2 files changed, 18 insertions(+)
diff --git a/drivers/regulator/of_regulator.c
b/drivers/regulator/of_regulator.c
index 7a51814..8b898e6 100644
--- a/drivers/regulator/of_regulator.c
+++ b/drivers/regulator/of_regulator.c
@@ -240,3 +240,15 @@ struct regulator_init_data
*regulator_of_get_init_data(struct device *dev,
return init_data;
}
+
+/**
+ * is_system_poweroff_source - Tells if poweroff-source is found for
device_node
+ * @np: Pointer to the given device_node
+ *
+ * return true if present false otherwise
+ */
+bool is_system_poweroff_source(const struct device_node *np)
+{
+ return of_property_read_bool(np, poweroff-source);
+}
+EXPORT_SYMBOL_GPL(is_system_poweroff_source);
Hi Romain,
This is an extremely simple wrapper. It may be best to simply make it
a static inline. It is also generic enough that I don't have a problem
with it living in include/linux/of.h; but of_regulator.h would be fine
too.
Since this is a device tree specific function (it doesn't work with
ACPI or platform_data), you should prefix the function name with of_
g.
What I'm more concerned about is the semantics of the property. What
do the generic code paths gain by standardizing this property? Is it
expected that
diff --git a/include/linux/regulator/of_regulator.h
b/include/linux/regulator/of_regulator.h
index f921796..9d8fbb2 100644
--- a/include/linux/regulator/of_regulator.h
+++ b/include/linux/regulator/of_regulator.h
@@ -20,6 +20,7 @@ extern struct regulator_init_data
extern int of_regulator_match(struct device *dev, struct device_node *node,
struct of_regulator_match *matches,
unsigned int num_matches);
+extern bool is_system_poweroff_source(const struct device_node *np);
#else
static inline struct regulator_init_data
*of_get_regulator_init_data(struct device *dev,
@@ -35,6 +36,11 @@ static inline int of_regulator_match(struct device *dev,
{
return 0;
}
+
+static inline bool is_system_poweroff_source(const struct device_node *np)
+{
+ return false;
+}
#endif /* CONFIG_OF */
#endif /* __LINUX_OF_REG_H */
--
1.9.1
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: linux-next: error fetching the devicetree tree
Hi Stephen, Yes, the server is down and being worked on. I've moved my tree to kernel.org. Here is the new URL: git://git.kernel.org/pub/scm/linux/kernel/git/glikely/linux.git g. On Tue, Oct 7, 2014 at 11:48 PM, Stephen Rothwell s...@canb.auug.org.au wrote: Hi Grant, Fetching the devicetree tree this morning produced this error: fatal: unable to connect to git.secretlab.ca: git.secretlab.ca[0: 208.123.74.12]: errno=Connection timed out -- Cheers, Stephen Rothwells...@canb.auug.org.au -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[GIT PULL] Devicetree changes for v3.18
Hi Linus, Here are the device tree changes I have queued up for v3.18. Pretty small set this time. Please pull. g. The following changes since commit fe82dcec644244676d55a1384c958d5f67979adb: Linux 3.17-rc7 (2014-09-28 14:29:07 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/glikely/linux tags/devicetree-for-linus for you to fetch changes up to 2118f4b8dfc666c3e4a9e262beca79636a0852fe: of/selftest: Move hash table off stack to fix large frame size (2014-10-07 11:30:31 +0100) Devicetree changes for v3.18 This branch contains bug fixes and new features for the devicetree code. Most of the changes are either new testcases for the selftest code or documentation changes. The most notable change is the addition of a phandle resolver for use when grafting in a second device tree blob into the core tree. The resolver isn't currently used by anything other than the selftest module, but it will be used to support device tree overlays; probably in the v3.19 timeframe. Also note that I've moved my normal tree from git.secretlab.ca to git.kernel.org. Gaurav Minocha (1): To remove non-ascii characters in of_selftest.txt Geert Uytterhoeven (1): of: Add vendor prefix for Sitronix Grant Likely (7): of: Fix NULL dereference in selftest removal code of/selftest: Test structure of device tree of: Don't try to search when phandle == 0 of/selftest: Add a test for duplicate phandles of/selftest: Use the resolver to fixup phandles Merge branch 'for-next' of git://git.kernel.org/.../robh/linux into devicetree/next of/selftest: Move hash table off stack to fix large frame size Pantelis Antoniou (1): of: Introduce Device Tree resolve support. Peter Chen (1): of: add vendor prefix for Chipidea Santosh Shilimkar (1): dt/documentation: add specification of dma bus information Thierry Reding (1): of: Add vendor prefix for Innolux Corporation Tim Harvey (1): devicetree: bindings: Document Gateworks vendor prefix Uwe Kleine-König (1): of: Add vendor prefix for Energy Micro .../devicetree/bindings/vendor-prefixes.txt| 5 + Documentation/devicetree/booting-without-of.txt| 53 .../devicetree/dynamic-resolution-notes.txt| 25 ++ Documentation/devicetree/of_selftest.txt | 30 +- drivers/of/Kconfig | 4 + drivers/of/Makefile| 1 + drivers/of/base.c | 3 + drivers/of/resolver.c | 336 + drivers/of/selftest.c | 120 +++- drivers/of/testcase-data/testcases.dts | 35 +++ include/linux/of.h | 3 + 11 files changed, 591 insertions(+), 24 deletions(-) create mode 100644 Documentation/devicetree/dynamic-resolution-notes.txt create mode 100644 drivers/of/resolver.c -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] of/selftest: Move hash table off stack to fix large frame size
The new testcase that checks phandle consistency was using a hash table
on the stack which made the frame size much large than it should be. Fix
the problem by moving the hash table into the file scope.
Signed-off-by: Grant Likely
---
This patch applies on my devicetree/next branch, which I'm going to ask
Linux to pull later this week.
g.
drivers/of/selftest.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/of/selftest.c b/drivers/of/selftest.c
index 4fed34bff5cf..78001270a598 100644
--- a/drivers/of/selftest.c
+++ b/drivers/of/selftest.c
@@ -198,20 +198,19 @@ struct node_hash {
struct device_node *np;
};
+static DEFINE_HASHTABLE(phandle_ht, 8);
static void __init of_selftest_check_phandles(void)
{
struct device_node *np;
struct node_hash *nh;
struct hlist_node *tmp;
int i, dup_count = 0, phandle_count = 0;
- DECLARE_HASHTABLE(ht, 8);
- hash_init(ht);
for_each_of_allnodes(np) {
if (!np->phandle)
continue;
- hash_for_each_possible(ht, nh, node, np->phandle) {
+ hash_for_each_possible(phandle_ht, nh, node, np->phandle) {
if (nh->np->phandle == np->phandle) {
pr_info("Duplicate phandle! %i used by %s and
%s\n",
np->phandle, nh->np->full_name,
np->full_name);
@@ -225,14 +224,14 @@ static void __init of_selftest_check_phandles(void)
return;
nh->np = np;
- hash_add(ht, >node, np->phandle);
+ hash_add(phandle_ht, >node, np->phandle);
phandle_count++;
}
selftest(dup_count == 0, "Found %i duplicates in %i phandles\n",
dup_count, phandle_count);
/* Clean up */
- hash_for_each_safe(ht, i, tmp, nh, node) {
+ hash_for_each_safe(phandle_ht, i, tmp, nh, node) {
hash_del(>node);
kfree(nh);
}
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: Eliminate of_allnodes list
On Mon, Oct 6, 2014 at 2:56 PM, Rob Herring wrote:
> On Mon, Oct 6, 2014 at 3:59 AM, Grant Likely wrote:
>> The device tree structure is composed of two lists; the 'allnodes' list
>> which is a singly linked list containing every node in the tree, and the
>> child->parent structure where each parent node has a singly linked list
>> of children. All of the data in the allnodes list can be easily
>> reproduced with the parent-child lists, so of_allnodes is actually
>> unnecessary. Remove it entirely which saves a bit of memory and
>> simplifies the data structure quite a lot.
>>
>> Signed-off-by: Grant Likely
>> Cc: Rob Herring
>> Cc: Gaurav Minocha
>> ---
>>
>> This applies against my current devicetree/next branch on
>> git.secretlab.ca/git/linux
>>
>> I'm not going to try and merge this in v3.18. It's too risky a patch and
>> I only just wrote it. I'll try to get it in for v3.19.
>
> Looks good. A few minor things below.
>
> [...]
>
>> diff --git a/drivers/mfd/vexpress-sysreg.c b/drivers/mfd/vexpress-sysreg.c
>> index 9e21e4fc9599..8f43ab8fd2d6 100644
>> --- a/drivers/mfd/vexpress-sysreg.c
>> +++ b/drivers/mfd/vexpress-sysreg.c
>> @@ -223,7 +223,7 @@ static int vexpress_sysreg_probe(struct platform_device
>> *pdev)
>> vexpress_config_set_master(vexpress_sysreg_get_master());
>>
>> /* Confirm board type against DT property, if available */
>> - if (of_property_read_u32(of_allnodes, "arm,hbi", _hbi) == 0) {
>> + if (of_property_read_u32(of_root, "arm,hbi", _hbi) == 0) {
>
> Given this and selftests are the only users in the whole tree, perhaps
> this should just use of_find_node_by_path("/") rather than expose this
> global.
I'll look at doing that with a separate patch. The
of_have_populated_dt() static inline in of.h uses it, so it needs to
be exported with the current code. I'd need to create an exported
version of that function to unexport of_root.
>
>> u32 id = vexpress_get_procid(VEXPRESS_SITE_MASTER);
>> u32 hbi = (id >> SYS_PROCIDx_HBI_SHIFT) & SYS_HBI_MASK;
>>
>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index 2305dc0382bc..b86beff7b167 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
>> @@ -32,8 +32,8 @@
>>
>> LIST_HEAD(aliases_lookup);
>>
>> -struct device_node *of_allnodes;
>> -EXPORT_SYMBOL(of_allnodes);
>> +struct device_node *of_root;
>> +EXPORT_SYMBOL(of_root);
>> struct device_node *of_chosen;
>> struct device_node *of_aliases;
>> struct device_node *of_stdout;
>> @@ -48,7 +48,7 @@ struct kset *of_kset;
>> */
>> DEFINE_MUTEX(of_mutex);
>>
>> -/* use when traversing tree through the allnext, child, sibling,
>> +/* use when traversing tree through the child, sibling,
>> * or parent members of struct device_node.
>> */
>> DEFINE_RAW_SPINLOCK(devtree_lock);
>> @@ -204,7 +204,7 @@ static int __init of_init(void)
>> mutex_unlock(_mutex);
>>
>> /* Symlink in /proc as required by userspace ABI */
>> - if (of_allnodes)
>> + if (of_root)
>> proc_symlink("device-tree", NULL,
>> "/sys/firmware/devicetree/base");
>>
>> return 0;
>> @@ -245,6 +245,23 @@ struct property *of_find_property(const struct
>> device_node *np,
>> }
>> EXPORT_SYMBOL(of_find_property);
>>
>> +struct device_node *__of_find_all_nodes(struct device_node *prev)
>> +{
>> + struct device_node *np;
>> + if (!prev)
>> + np = of_root;
>> + else if (prev->child) {
>> + np = prev->child;
>> + } else {
>
> Your braces are inconsistent.
Fixed.
>> @@ -180,12 +172,12 @@ static void __init of_selftest_check_tree_linkage(void)
>> struct device_node *np;
>> int allnode_count = 0, child_count;
>>
>> - if (!of_allnodes)
>> + if (!of_root)
>
> This could be !of_have_populated_dt() instead.
For internal dt code, I don't mind referencing of_root directly.
Thanks for the review.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] of: Eliminate of_allnodes list
On Mon, Oct 6, 2014 at 2:56 PM, Rob Herring r...@kernel.org wrote:
On Mon, Oct 6, 2014 at 3:59 AM, Grant Likely grant.lik...@linaro.org wrote:
The device tree structure is composed of two lists; the 'allnodes' list
which is a singly linked list containing every node in the tree, and the
child-parent structure where each parent node has a singly linked list
of children. All of the data in the allnodes list can be easily
reproduced with the parent-child lists, so of_allnodes is actually
unnecessary. Remove it entirely which saves a bit of memory and
simplifies the data structure quite a lot.
Signed-off-by: Grant Likely grant.lik...@linaro.org
Cc: Rob Herring r...@kernel.org
Cc: Gaurav Minocha gaurav.minocha...@gmail.com
---
This applies against my current devicetree/next branch on
git.secretlab.ca/git/linux
I'm not going to try and merge this in v3.18. It's too risky a patch and
I only just wrote it. I'll try to get it in for v3.19.
Looks good. A few minor things below.
[...]
diff --git a/drivers/mfd/vexpress-sysreg.c b/drivers/mfd/vexpress-sysreg.c
index 9e21e4fc9599..8f43ab8fd2d6 100644
--- a/drivers/mfd/vexpress-sysreg.c
+++ b/drivers/mfd/vexpress-sysreg.c
@@ -223,7 +223,7 @@ static int vexpress_sysreg_probe(struct platform_device
*pdev)
vexpress_config_set_master(vexpress_sysreg_get_master());
/* Confirm board type against DT property, if available */
- if (of_property_read_u32(of_allnodes, arm,hbi, dt_hbi) == 0) {
+ if (of_property_read_u32(of_root, arm,hbi, dt_hbi) == 0) {
Given this and selftests are the only users in the whole tree, perhaps
this should just use of_find_node_by_path(/) rather than expose this
global.
I'll look at doing that with a separate patch. The
of_have_populated_dt() static inline in of.h uses it, so it needs to
be exported with the current code. I'd need to create an exported
version of that function to unexport of_root.
u32 id = vexpress_get_procid(VEXPRESS_SITE_MASTER);
u32 hbi = (id SYS_PROCIDx_HBI_SHIFT) SYS_HBI_MASK;
diff --git a/drivers/of/base.c b/drivers/of/base.c
index 2305dc0382bc..b86beff7b167 100644
--- a/drivers/of/base.c
+++ b/drivers/of/base.c
@@ -32,8 +32,8 @@
LIST_HEAD(aliases_lookup);
-struct device_node *of_allnodes;
-EXPORT_SYMBOL(of_allnodes);
+struct device_node *of_root;
+EXPORT_SYMBOL(of_root);
struct device_node *of_chosen;
struct device_node *of_aliases;
struct device_node *of_stdout;
@@ -48,7 +48,7 @@ struct kset *of_kset;
*/
DEFINE_MUTEX(of_mutex);
-/* use when traversing tree through the allnext, child, sibling,
+/* use when traversing tree through the child, sibling,
* or parent members of struct device_node.
*/
DEFINE_RAW_SPINLOCK(devtree_lock);
@@ -204,7 +204,7 @@ static int __init of_init(void)
mutex_unlock(of_mutex);
/* Symlink in /proc as required by userspace ABI */
- if (of_allnodes)
+ if (of_root)
proc_symlink(device-tree, NULL,
/sys/firmware/devicetree/base);
return 0;
@@ -245,6 +245,23 @@ struct property *of_find_property(const struct
device_node *np,
}
EXPORT_SYMBOL(of_find_property);
+struct device_node *__of_find_all_nodes(struct device_node *prev)
+{
+ struct device_node *np;
+ if (!prev)
+ np = of_root;
+ else if (prev-child) {
+ np = prev-child;
+ } else {
Your braces are inconsistent.
Fixed.
@@ -180,12 +172,12 @@ static void __init of_selftest_check_tree_linkage(void)
struct device_node *np;
int allnode_count = 0, child_count;
- if (!of_allnodes)
+ if (!of_root)
This could be !of_have_populated_dt() instead.
For internal dt code, I don't mind referencing of_root directly.
Thanks for the review.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
