Re: [PATCH] drivers/core/of: Add symlink to device-tree from devices with an OF node
On Wed, 2014-11-26 at 19:39 -0800, Greg KH wrote: > Are you going to resend a changed version of this? Yes, I've been distracted by a few other things, but I suppose I should do it :-) Possibly tomorrow. Arnd, are you doing that helper you suggested to get to the of_node or should I ? Cheers, Ben. > thanks, > > greg k-h > -- > 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] drivers/core/of: Add symlink to device-tree from devices with an OF node
On Thu, Nov 13, 2014 at 12:10:47PM +1100, Benjamin Herrenschmidt wrote:
> So I've been annoyed lately with having a bunch of devices such as i2c
> eeproms (for use by VPDs, server world !) and other bits and pieces that
> I want to be able to identify from userspace, and possibly provide
> additional data about from FW.
>
> Basically, it boils down to correlating the sysfs device with the OF
> tree device node, so that user space can use device-tree info such as
> additional "location" or "label" (or whatever else we can come up with)
> propreties to identify a given device, or get some attributes of use
> about it, etc...
>
> Now, so far, we've done that in some subsystem in a fairly ad-hoc basis
> using "devspec" properties. For example, PCI creates them if it can
> correlate the probed device with a DT node. Some powerpc specific busses
> do that too.
>
> However, i2c doesn't and it would be nice to have something more generic
> since technically any device can have a corresponding device tree node.
>
> This patch adds an "of_node" symlink to devices that have a non-NULL
> dev->of_node pointer, the patch is pretty trivial and seems to work just
> fine for me.
>
> Signed-off-by: Benjamin Herrenschmidt
> ---
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 20da3ad..8c7b607 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -493,6 +493,15 @@ static int device_add_attrs(struct device *dev)
> goto err_remove_dev_groups;
> }
>
> +#ifdef CONFIG_OF
> + if (dev->of_node) {
> + error = sysfs_create_link(&dev->kobj, &dev->of_node->kobj,
> + "of_node");
> + if (error)
> + dev_warn(dev, "Error %d creating of_node link\n",
> error);
> + }
> +#endif /* CONFIG_OF */
> +
> return 0;
>
> err_remove_dev_groups:
>
Are you going to resend a changed version of this?
thanks,
greg k-h
--
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] drivers/core/of: Add symlink to device-tree from devices with an OF node
On Wed, Nov 19, 2014 at 3:39 PM, Rob Herring wrote:
> On Wed, Nov 19, 2014 at 8:49 AM, Arnd Bergmann wrote:
>> On Wednesday 19 November 2014 08:45:58 Rob Herring wrote:
>>> > static inline struct device_node *dev_of_node(struct device *of_node)
>>> > {
>>> > if (!IS_ENABLED(CONFIG_OF))
>>> > return NULL;
>>> >
>>> > return dev->of_node;
>>> > }
>>> >
>>> > Adding the IS_ENABLED() in a lot of drivers isn't horrible, but we seem
>>> > to be doing it a lot.
>>>
>>> I think you misread things. of_node is always present now, so it
>>> should always be NULL for !CONFIG_OF.
>>>
>>
>> No, I didn't misread it but I should have been clearer with the intention:
>> The idea is to tell the compiler that we know it will be NULL when CONFIG_OF
>> is unset, so it can optimize out all code that does
>>
>> struct device_node *dn = dev_of_node(dev);
>>
>> if (dn) {
>> ...
>> /* complex code */
>> ...
>> }
>>
>> and we can avoid using an #ifdef or if(IS_ENABLED()) in the source to
>> compile out the DT-only sections of a driver.
>
> Oh, right. That would definitely be worthwhile to do.
Agreed.
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] drivers/core/of: Add symlink to device-tree from devices with an OF node
On Wed, Nov 19, 2014 at 8:49 AM, Arnd Bergmann wrote:
> On Wednesday 19 November 2014 08:45:58 Rob Herring wrote:
>> > static inline struct device_node *dev_of_node(struct device *of_node)
>> > {
>> > if (!IS_ENABLED(CONFIG_OF))
>> > return NULL;
>> >
>> > return dev->of_node;
>> > }
>> >
>> > Adding the IS_ENABLED() in a lot of drivers isn't horrible, but we seem
>> > to be doing it a lot.
>>
>> I think you misread things. of_node is always present now, so it
>> should always be NULL for !CONFIG_OF.
>>
>
> No, I didn't misread it but I should have been clearer with the intention:
> The idea is to tell the compiler that we know it will be NULL when CONFIG_OF
> is unset, so it can optimize out all code that does
>
> struct device_node *dn = dev_of_node(dev);
>
> if (dn) {
> ...
> /* complex code */
> ...
> }
>
> and we can avoid using an #ifdef or if(IS_ENABLED()) in the source to
> compile out the DT-only sections of a driver.
Oh, right. That would definitely be worthwhile to do.
Rob
--
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] drivers/core/of: Add symlink to device-tree from devices with an OF node
On Wednesday 19 November 2014 08:45:58 Rob Herring wrote:
> > static inline struct device_node *dev_of_node(struct device *of_node)
> > {
> > if (!IS_ENABLED(CONFIG_OF))
> > return NULL;
> >
> > return dev->of_node;
> > }
> >
> > Adding the IS_ENABLED() in a lot of drivers isn't horrible, but we seem
> > to be doing it a lot.
>
> I think you misread things. of_node is always present now, so it
> should always be NULL for !CONFIG_OF.
>
No, I didn't misread it but I should have been clearer with the intention:
The idea is to tell the compiler that we know it will be NULL when CONFIG_OF
is unset, so it can optimize out all code that does
struct device_node *dn = dev_of_node(dev);
if (dn) {
...
/* complex code */
...
}
and we can avoid using an #ifdef or if(IS_ENABLED()) in the source to
compile out the DT-only sections of a driver.
Arnd
--
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] drivers/core/of: Add symlink to device-tree from devices with an OF node
On Wed, Nov 19, 2014 at 2:38 AM, Arnd Bergmann wrote:
> On Wednesday 19 November 2014 13:35:44 Benjamin Herrenschmidt wrote:
>> On Wed, 2014-11-19 at 10:39 +1100, Jeremy Kerr wrote:
>> > Hi Rob,
>> >
>> > >> diff --git a/drivers/base/core.c b/drivers/base/core.c
>> > >> index 20da3ad..8c7b607 100644
>> > >> --- a/drivers/base/core.c
>> > >> +++ b/drivers/base/core.c
>> > >> @@ -493,6 +493,15 @@ static int device_add_attrs(struct device *dev)
>> > >> goto err_remove_dev_groups;
>> > >> }
>> > >>
>> > >> +#ifdef CONFIG_OF
>> > >> + if (dev->of_node) {
>> > >
>> > > if (IS_ENABLED(CONFIG_OF) && dev->of_node)
>> >
>> > struct device doesn't have an of_node member if !CONFIG_OF, so we'll
>> > need to disable this block in the preprocessor.
>>
>> Actually that's no longer the case since 2.6.39 afaik
>
> I wonder if we should create a small helper for this, like
>
> static inline struct device_node *dev_of_node(struct device *of_node)
> {
> if (!IS_ENABLED(CONFIG_OF))
> return NULL;
>
> return dev->of_node;
> }
>
> Adding the IS_ENABLED() in a lot of drivers isn't horrible, but we seem
> to be doing it a lot.
I think you misread things. of_node is always present now, so it
should always be NULL for !CONFIG_OF.
Rob
--
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] drivers/core/of: Add symlink to device-tree from devices with an OF node
On Wednesday 19 November 2014 13:35:44 Benjamin Herrenschmidt wrote:
> On Wed, 2014-11-19 at 10:39 +1100, Jeremy Kerr wrote:
> > Hi Rob,
> >
> > >> diff --git a/drivers/base/core.c b/drivers/base/core.c
> > >> index 20da3ad..8c7b607 100644
> > >> --- a/drivers/base/core.c
> > >> +++ b/drivers/base/core.c
> > >> @@ -493,6 +493,15 @@ static int device_add_attrs(struct device *dev)
> > >> goto err_remove_dev_groups;
> > >> }
> > >>
> > >> +#ifdef CONFIG_OF
> > >> + if (dev->of_node) {
> > >
> > > if (IS_ENABLED(CONFIG_OF) && dev->of_node)
> >
> > struct device doesn't have an of_node member if !CONFIG_OF, so we'll
> > need to disable this block in the preprocessor.
>
> Actually that's no longer the case since 2.6.39 afaik
I wonder if we should create a small helper for this, like
static inline struct device_node *dev_of_node(struct device *of_node)
{
if (!IS_ENABLED(CONFIG_OF))
return NULL;
return dev->of_node;
}
Adding the IS_ENABLED() in a lot of drivers isn't horrible, but we seem
to be doing it a lot.
Arnd
--
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] drivers/core/of: Add symlink to device-tree from devices with an OF node
On Wed, 2014-11-19 at 10:39 +1100, Jeremy Kerr wrote:
> Hi Rob,
>
> >> diff --git a/drivers/base/core.c b/drivers/base/core.c
> >> index 20da3ad..8c7b607 100644
> >> --- a/drivers/base/core.c
> >> +++ b/drivers/base/core.c
> >> @@ -493,6 +493,15 @@ static int device_add_attrs(struct device *dev)
> >> goto err_remove_dev_groups;
> >> }
> >>
> >> +#ifdef CONFIG_OF
> >> + if (dev->of_node) {
> >
> > if (IS_ENABLED(CONFIG_OF) && dev->of_node)
>
> struct device doesn't have an of_node member if !CONFIG_OF, so we'll
> need to disable this block in the preprocessor.
Actually that's no longer the case since 2.6.39 afaik :-)
Cheers,
Ben.
--
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] drivers/core/of: Add symlink to device-tree from devices with an OF node
On Tue, 2014-11-18 at 10:37 -0600, Rob Herring wrote:
> On Wed, Nov 12, 2014 at 7:10 PM, Benjamin Herrenschmidt
> wrote:
> > So I've been annoyed lately with having a bunch of devices such as i2c
> > eeproms (for use by VPDs, server world !) and other bits and pieces that
> > I want to be able to identify from userspace, and possibly provide
> > additional data about from FW.
> >
> > Basically, it boils down to correlating the sysfs device with the OF
> > tree device node, so that user space can use device-tree info such as
> > additional "location" or "label" (or whatever else we can come up with)
> > propreties to identify a given device, or get some attributes of use
> > about it, etc...
> >
> > Now, so far, we've done that in some subsystem in a fairly ad-hoc basis
> > using "devspec" properties. For example, PCI creates them if it can
> > correlate the probed device with a DT node. Some powerpc specific busses
> > do that too.
> >
> > However, i2c doesn't and it would be nice to have something more generic
> > since technically any device can have a corresponding device tree node.
> >
> > This patch adds an "of_node" symlink to devices that have a non-NULL
> > dev->of_node pointer, the patch is pretty trivial and seems to work just
> > fine for me.
> >
> > Signed-off-by: Benjamin Herrenschmidt
> > ---
> > diff --git a/drivers/base/core.c b/drivers/base/core.c
> > index 20da3ad..8c7b607 100644
> > --- a/drivers/base/core.c
> > +++ b/drivers/base/core.c
> > @@ -493,6 +493,15 @@ static int device_add_attrs(struct device *dev)
> > goto err_remove_dev_groups;
> > }
> >
> > +#ifdef CONFIG_OF
> > + if (dev->of_node) {
>
> if (IS_ENABLED(CONFIG_OF) && dev->of_node)
Ok, I didn't realize the of_node field existed in struct device even
without CONFIG_OF (otherwise that wouldn't have compiled). Grant, Rob,
do you want to take this patch (with the above fixed) or should I not
bother based on the fact that the info is in uevent ? I prefer still
doing the symlink but you tell me.
> > + error = sysfs_create_link(&dev->kobj, &dev->of_node->kobj,
> > + "of_node");
> > + if (error)
> > + dev_warn(dev, "Error %d creating of_node link\n",
> > error);
> > + }
> > +#endif /* CONFIG_OF */
> > +
> > return 0;
> >
> > err_remove_dev_groups:
> >
> >
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" 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] drivers/core/of: Add symlink to device-tree from devices with an OF node
Hi Rob, > struct device doesn't have an of_node member if !CONFIG_OF, so we'll > need to disable this block in the preprocessor. Scratch that, I was looking at the wrong header - we do indeed have the of_node available independently of CONFIG_OF, and this makes the logic a little cleaner. Cheers, Jeremy -- 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] drivers/core/of: Add symlink to device-tree from devices with an OF node
Hi Rob,
>> diff --git a/drivers/base/core.c b/drivers/base/core.c
>> index 20da3ad..8c7b607 100644
>> --- a/drivers/base/core.c
>> +++ b/drivers/base/core.c
>> @@ -493,6 +493,15 @@ static int device_add_attrs(struct device *dev)
>> goto err_remove_dev_groups;
>> }
>>
>> +#ifdef CONFIG_OF
>> + if (dev->of_node) {
>
> if (IS_ENABLED(CONFIG_OF) && dev->of_node)
struct device doesn't have an of_node member if !CONFIG_OF, so we'll
need to disable this block in the preprocessor.
Cheers,
Jeremy
--
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] drivers/core/of: Add symlink to device-tree from devices with an OF node
On Wed, Nov 12, 2014 at 7:10 PM, Benjamin Herrenschmidt
wrote:
> So I've been annoyed lately with having a bunch of devices such as i2c
> eeproms (for use by VPDs, server world !) and other bits and pieces that
> I want to be able to identify from userspace, and possibly provide
> additional data about from FW.
>
> Basically, it boils down to correlating the sysfs device with the OF
> tree device node, so that user space can use device-tree info such as
> additional "location" or "label" (or whatever else we can come up with)
> propreties to identify a given device, or get some attributes of use
> about it, etc...
>
> Now, so far, we've done that in some subsystem in a fairly ad-hoc basis
> using "devspec" properties. For example, PCI creates them if it can
> correlate the probed device with a DT node. Some powerpc specific busses
> do that too.
>
> However, i2c doesn't and it would be nice to have something more generic
> since technically any device can have a corresponding device tree node.
>
> This patch adds an "of_node" symlink to devices that have a non-NULL
> dev->of_node pointer, the patch is pretty trivial and seems to work just
> fine for me.
>
> Signed-off-by: Benjamin Herrenschmidt
> ---
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 20da3ad..8c7b607 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -493,6 +493,15 @@ static int device_add_attrs(struct device *dev)
> goto err_remove_dev_groups;
> }
>
> +#ifdef CONFIG_OF
> + if (dev->of_node) {
if (IS_ENABLED(CONFIG_OF) && dev->of_node)
> + error = sysfs_create_link(&dev->kobj, &dev->of_node->kobj,
> + "of_node");
> + if (error)
> + dev_warn(dev, "Error %d creating of_node link\n",
> error);
> + }
> +#endif /* CONFIG_OF */
> +
> return 0;
>
> err_remove_dev_groups:
>
>
--
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] drivers/core/of: Add symlink to device-tree from devices with an OF node
So I've been annoyed lately with having a bunch of devices such as i2c
eeproms (for use by VPDs, server world !) and other bits and pieces that
I want to be able to identify from userspace, and possibly provide
additional data about from FW.
Basically, it boils down to correlating the sysfs device with the OF
tree device node, so that user space can use device-tree info such as
additional "location" or "label" (or whatever else we can come up with)
propreties to identify a given device, or get some attributes of use
about it, etc...
Now, so far, we've done that in some subsystem in a fairly ad-hoc basis
using "devspec" properties. For example, PCI creates them if it can
correlate the probed device with a DT node. Some powerpc specific busses
do that too.
However, i2c doesn't and it would be nice to have something more generic
since technically any device can have a corresponding device tree node.
This patch adds an "of_node" symlink to devices that have a non-NULL
dev->of_node pointer, the patch is pretty trivial and seems to work just
fine for me.
Signed-off-by: Benjamin Herrenschmidt
---
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 20da3ad..8c7b607 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -493,6 +493,15 @@ static int device_add_attrs(struct device *dev)
goto err_remove_dev_groups;
}
+#ifdef CONFIG_OF
+ if (dev->of_node) {
+ error = sysfs_create_link(&dev->kobj, &dev->of_node->kobj,
+ "of_node");
+ if (error)
+ dev_warn(dev, "Error %d creating of_node link\n",
error);
+ }
+#endif /* CONFIG_OF */
+
return 0;
err_remove_dev_groups:
--
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/
