* [PATCH 1/5] of/device: Centralize checking of 'status' properties [not found] <cover.1291799069.git.deepak_saxena@mentor.com> @ 2010-12-08 19:29 ` Deepak Saxena 2010-12-31 7:43 ` Grant Likely 2010-12-08 19:29 ` [PATCH 4/5] of/device: Create _of_get_next_child() Deepak Saxena ` (3 subsequent siblings) 4 siblings, 1 reply; 13+ messages in thread From: Deepak Saxena @ 2010-12-08 19:29 UTC (permalink / raw) To: devicetree-discuss; +Cc: linuxppc-dev Don't call any of_platform_driver's probe() function if the device node is not accessible (as denoted in the status property). Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com> Signed-off-by: Deepak Saxena <deepak_saxena@mentor.com> --- arch/powerpc/platforms/83xx/suspend.c | 3 --- drivers/mmc/host/sdhci-of-core.c | 3 --- drivers/net/gianfar.c | 2 +- drivers/net/ibm_newemac/core.c | 2 +- drivers/of/platform.c | 7 +++++-- 5 files changed, 7 insertions(+), 10 deletions(-) diff --git a/arch/powerpc/platforms/83xx/suspend.c b/arch/powerpc/platforms/83xx/suspend.c index 75ae77f..a5a54aa 100644 --- a/arch/powerpc/platforms/83xx/suspend.c +++ b/arch/powerpc/platforms/83xx/suspend.c @@ -326,9 +326,6 @@ static int pmc_probe(struct platform_device *ofdev, struct pmc_type *type = match->data; int ret = 0; - if (!of_device_is_available(np)) - return -ENODEV; - has_deep_sleep = type->has_deep_sleep; immrbase = get_immrbase(); pmc_dev = ofdev; diff --git a/drivers/mmc/host/sdhci-of-core.c b/drivers/mmc/host/sdhci-of-core.c index c51b711..51218d4 100644 --- a/drivers/mmc/host/sdhci-of-core.c +++ b/drivers/mmc/host/sdhci-of-core.c @@ -126,9 +126,6 @@ static int __devinit sdhci_of_probe(struct platform_device *ofdev, int size; int ret; - if (!of_device_is_available(np)) - return -ENODEV; - host = sdhci_alloc_host(&ofdev->dev, sizeof(*of_host)); if (IS_ERR(host)) return -ENOMEM; diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c index d1bec62..9918914 100644 --- a/drivers/net/gianfar.c +++ b/drivers/net/gianfar.c @@ -620,7 +620,7 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev) unsigned int num_tx_qs, num_rx_qs; u32 *tx_queues, *rx_queues; - if (!np || !of_device_is_available(np)) + if (!np) return -ENODEV; /* parse the num of tx and rx queues */ diff --git a/drivers/net/ibm_newemac/core.c b/drivers/net/ibm_newemac/core.c index 06bb9b7..290fff2 100644 --- a/drivers/net/ibm_newemac/core.c +++ b/drivers/net/ibm_newemac/core.c @@ -2732,7 +2732,7 @@ static int __devinit emac_probe(struct platform_device *ofdev, * property here for now, but new flat device trees should set a * status property to "disabled" instead. */ - if (of_get_property(np, "unused", NULL) || !of_device_is_available(np)) + if (of_get_property(np, "unused", NULL)) return -ENODEV; /* Find ourselves in the bootlist if we are there */ diff --git a/drivers/of/platform.c b/drivers/of/platform.c index 5b4a07f..14370a0 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -56,7 +56,10 @@ static int platform_driver_probe_shim(struct platform_device *pdev) * come up empty. Return -EINVAL in this case so other drivers get * the chance to bind. */ match = of_match_device(pdev->dev.driver->of_match_table, &pdev->dev); - return match ? ofpdrv->probe(pdev, match) : -EINVAL; + if (match && of_device_is_available(pdev->dev.of_node)) + return ofpdrv->probe(pdev, match); + + return -EINVAL; } static void platform_driver_shutdown_shim(struct platform_device *pdev) @@ -142,7 +145,7 @@ static int of_platform_device_probe(struct device *dev) of_dev_get(of_dev); match = of_match_device(drv->driver.of_match_table, dev); - if (match) + if (match && of_device_is_available(dev->of_node)) error = drv->probe(of_dev, match); if (error) of_dev_put(of_dev); -- 1.6.3.3 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/5] of/device: Centralize checking of 'status' properties 2010-12-08 19:29 ` [PATCH 1/5] of/device: Centralize checking of 'status' properties Deepak Saxena @ 2010-12-31 7:43 ` Grant Likely 0 siblings, 0 replies; 13+ messages in thread From: Grant Likely @ 2010-12-31 7:43 UTC (permalink / raw) To: Deepak Saxena; +Cc: devicetree-discuss, linuxppc-dev On Wed, Dec 08, 2010 at 11:29:13AM -0800, Deepak Saxena wrote: > Don't call any of_platform_driver's probe() function if the device node is > not accessible (as denoted in the status property). > > Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com> > Signed-off-by: Deepak Saxena <deepak_saxena@mentor.com> > --- > arch/powerpc/platforms/83xx/suspend.c | 3 --- > drivers/mmc/host/sdhci-of-core.c | 3 --- > drivers/net/gianfar.c | 2 +- > drivers/net/ibm_newemac/core.c | 2 +- > drivers/of/platform.c | 7 +++++-- > 5 files changed, 7 insertions(+), 10 deletions(-) > > diff --git a/arch/powerpc/platforms/83xx/suspend.c b/arch/powerpc/platforms/83xx/suspend.c > index 75ae77f..a5a54aa 100644 > --- a/arch/powerpc/platforms/83xx/suspend.c > +++ b/arch/powerpc/platforms/83xx/suspend.c > @@ -326,9 +326,6 @@ static int pmc_probe(struct platform_device *ofdev, > struct pmc_type *type = match->data; > int ret = 0; > > - if (!of_device_is_available(np)) > - return -ENODEV; > - > has_deep_sleep = type->has_deep_sleep; > immrbase = get_immrbase(); > pmc_dev = ofdev; > diff --git a/drivers/mmc/host/sdhci-of-core.c b/drivers/mmc/host/sdhci-of-core.c > index c51b711..51218d4 100644 > --- a/drivers/mmc/host/sdhci-of-core.c > +++ b/drivers/mmc/host/sdhci-of-core.c > @@ -126,9 +126,6 @@ static int __devinit sdhci_of_probe(struct platform_device *ofdev, > int size; > int ret; > > - if (!of_device_is_available(np)) > - return -ENODEV; > - > host = sdhci_alloc_host(&ofdev->dev, sizeof(*of_host)); > if (IS_ERR(host)) > return -ENOMEM; > diff --git a/drivers/net/gianfar.c b/drivers/net/gianfar.c > index d1bec62..9918914 100644 > --- a/drivers/net/gianfar.c > +++ b/drivers/net/gianfar.c > @@ -620,7 +620,7 @@ static int gfar_of_init(struct platform_device *ofdev, struct net_device **pdev) > unsigned int num_tx_qs, num_rx_qs; > u32 *tx_queues, *rx_queues; > > - if (!np || !of_device_is_available(np)) > + if (!np) > return -ENODEV; > > /* parse the num of tx and rx queues */ > diff --git a/drivers/net/ibm_newemac/core.c b/drivers/net/ibm_newemac/core.c > index 06bb9b7..290fff2 100644 > --- a/drivers/net/ibm_newemac/core.c > +++ b/drivers/net/ibm_newemac/core.c > @@ -2732,7 +2732,7 @@ static int __devinit emac_probe(struct platform_device *ofdev, > * property here for now, but new flat device trees should set a > * status property to "disabled" instead. > */ > - if (of_get_property(np, "unused", NULL) || !of_device_is_available(np)) > + if (of_get_property(np, "unused", NULL)) > return -ENODEV; > > /* Find ourselves in the bootlist if we are there */ > diff --git a/drivers/of/platform.c b/drivers/of/platform.c > index 5b4a07f..14370a0 100644 > --- a/drivers/of/platform.c > +++ b/drivers/of/platform.c > @@ -56,7 +56,10 @@ static int platform_driver_probe_shim(struct platform_device *pdev) > * come up empty. Return -EINVAL in this case so other drivers get > * the chance to bind. */ > match = of_match_device(pdev->dev.driver->of_match_table, &pdev->dev); > - return match ? ofpdrv->probe(pdev, match) : -EINVAL; > + if (match && of_device_is_available(pdev->dev.of_node)) > + return ofpdrv->probe(pdev, match); > + > + return -EINVAL; Hi Deepak, This works for drivers still using the deprecated of_platform_driver structure, but all of those users are being converted into platform_drivers which do not use this shim. Fixing it here will not work after conversion. I think it would be better to inhibit the devices from getting registered in the first place instead of short circuiting the probe path. g. > } > > static void platform_driver_shutdown_shim(struct platform_device *pdev) > @@ -142,7 +145,7 @@ static int of_platform_device_probe(struct device *dev) > of_dev_get(of_dev); > > match = of_match_device(drv->driver.of_match_table, dev); > - if (match) > + if (match && of_device_is_available(dev->of_node)) > error = drv->probe(of_dev, match); > if (error) > of_dev_put(of_dev); > -- > 1.6.3.3 > > _______________________________________________ > devicetree-discuss mailing list > devicetree-discuss@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/devicetree-discuss ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 4/5] of/device: Create _of_get_next_child() [not found] <cover.1291799069.git.deepak_saxena@mentor.com> 2010-12-08 19:29 ` [PATCH 1/5] of/device: Centralize checking of 'status' properties Deepak Saxena @ 2010-12-08 19:29 ` Deepak Saxena 2010-12-08 19:29 ` [PATCH 5/5] of/device: Show even unavailable nodes in procfs Deepak Saxena ` (2 subsequent siblings) 4 siblings, 0 replies; 13+ messages in thread From: Deepak Saxena @ 2010-12-08 19:29 UTC (permalink / raw) To: devicetree-discuss; +Cc: linuxppc-dev There aren't many, but some of_get_next_child() callers do need an unabridged view of the device tree so we provide a wrapper that can be used by them. Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com> Signed-off-by: Deepak Saxena <deepak_saxena@mentor.com> --- drivers/of/base.c | 26 ++++++++++++++++++++++++++ include/linux/of.h | 2 ++ 2 files changed, 28 insertions(+), 0 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index 81b2601..7bc1398 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -315,6 +315,32 @@ struct device_node *of_get_next_parent(struct device_node *node) } /** + * _of_get_next_child - Iterate a node childs + * @node: parent node + * @prev: previous child of the parent node, or NULL to get first + * + * Returns a node pointer with refcount incremented, use + * of_node_put() on it when done. + * + * Unlike most other node accessors, ignores status properties. + */ +struct device_node *_of_get_next_child(const struct device_node *node, + struct device_node *prev) +{ + struct device_node *next; + + read_lock(&devtree_lock); + next = prev ? prev->sibling : node->child; + for (; next; next = next->sibling) + if (of_device_is_available(next) && of_node_get(next)) + break; + of_node_put(prev); + read_unlock(&devtree_lock); + return next; +} +EXPORT_SYMBOL(_of_get_next_child); + +/** * of_get_next_child - Iterate a node childs * @node: parent node * @prev: previous child of the parent node, or NULL to get first diff --git a/include/linux/of.h b/include/linux/of.h index cad7cf0..1fea01d 100644 --- a/include/linux/of.h +++ b/include/linux/of.h @@ -175,6 +175,8 @@ extern struct device_node *of_find_node_by_path(const char *path); extern struct device_node *of_find_node_by_phandle(phandle handle); extern struct device_node *of_get_parent(const struct device_node *node); extern struct device_node *of_get_next_parent(struct device_node *node); +extern struct device_node *_of_get_next_child(const struct device_node *node, + struct device_node *prev); extern struct device_node *of_get_next_child(const struct device_node *node, struct device_node *prev); #define for_each_child_of_node(parent, child) \ -- 1.6.3.3 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 5/5] of/device: Show even unavailable nodes in procfs [not found] <cover.1291799069.git.deepak_saxena@mentor.com> 2010-12-08 19:29 ` [PATCH 1/5] of/device: Centralize checking of 'status' properties Deepak Saxena 2010-12-08 19:29 ` [PATCH 4/5] of/device: Create _of_get_next_child() Deepak Saxena @ 2010-12-08 19:29 ` Deepak Saxena 2010-12-08 19:29 ` [PATCH 3/5] of/device: Make of_get_next_child() check status properties Deepak Saxena 2010-12-08 19:29 ` [PATCH 2/5] of/device: make for_each_node* " Deepak Saxena 4 siblings, 0 replies; 13+ messages in thread From: Deepak Saxena @ 2010-12-08 19:29 UTC (permalink / raw) To: devicetree-discuss; +Cc: linuxppc-dev Use the new "raw" of_get_next_child() variant so all device tree nodes will appear in procfs. Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com> Signed-off-by: Deepak Saxena <deepak_saxena@mentor.com> --- fs/proc/proc_devtree.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/fs/proc/proc_devtree.c b/fs/proc/proc_devtree.c index d9396a4..aa914eb 100644 --- a/fs/proc/proc_devtree.c +++ b/fs/proc/proc_devtree.c @@ -188,7 +188,7 @@ void proc_device_tree_add_node(struct device_node *np, const char *p; set_node_proc_entry(np, de); - for (child = NULL; (child = of_get_next_child(np, child));) { + for (child = NULL; (child = _of_get_next_child(np, child));) { /* Use everything after the last slash, or the full name */ p = strrchr(child->full_name, '/'); if (!p) -- 1.6.3.3 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/5] of/device: Make of_get_next_child() check status properties [not found] <cover.1291799069.git.deepak_saxena@mentor.com> ` (2 preceding siblings ...) 2010-12-08 19:29 ` [PATCH 5/5] of/device: Show even unavailable nodes in procfs Deepak Saxena @ 2010-12-08 19:29 ` Deepak Saxena 2010-12-08 21:01 ` Scott Wood 2010-12-08 19:29 ` [PATCH 2/5] of/device: make for_each_node* " Deepak Saxena 4 siblings, 1 reply; 13+ messages in thread From: Deepak Saxena @ 2010-12-08 19:29 UTC (permalink / raw) To: devicetree-discuss; +Cc: linuxppc-dev We only return the next child if the device is available. Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com> Signed-off-by: Deepak Saxena <deepak_saxena@mentor.com> --- drivers/of/base.c | 4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/drivers/of/base.c b/drivers/of/base.c index 5d269a4..81b2601 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -321,6 +321,8 @@ struct device_node *of_get_next_parent(struct device_node *node) * * Returns a node pointer with refcount incremented, use * of_node_put() on it when done. + * + * Does not return nodes marked unavailable by a status property. */ struct device_node *of_get_next_child(const struct device_node *node, struct device_node *prev) @@ -330,7 +332,7 @@ struct device_node *of_get_next_child(const struct device_node *node, read_lock(&devtree_lock); next = prev ? prev->sibling : node->child; for (; next; next = next->sibling) - if (of_node_get(next)) + if (of_device_is_available(next) && of_node_get(next)) break; of_node_put(prev); read_unlock(&devtree_lock); -- 1.6.3.3 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 3/5] of/device: Make of_get_next_child() check status properties 2010-12-08 19:29 ` [PATCH 3/5] of/device: Make of_get_next_child() check status properties Deepak Saxena @ 2010-12-08 21:01 ` Scott Wood 2010-12-09 1:33 ` Michael Ellerman 0 siblings, 1 reply; 13+ messages in thread From: Scott Wood @ 2010-12-08 21:01 UTC (permalink / raw) To: Deepak Saxena; +Cc: devicetree-discuss, linuxppc-dev On Wed, 8 Dec 2010 11:29:44 -0800 Deepak Saxena <deepak_saxena@mentor.com> wrote: > We only return the next child if the device is available. > > Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com> > Signed-off-by: Deepak Saxena <deepak_saxena@mentor.com> > --- > drivers/of/base.c | 4 +++- > 1 files changed, 3 insertions(+), 1 deletions(-) > > diff --git a/drivers/of/base.c b/drivers/of/base.c > index 5d269a4..81b2601 100644 > --- a/drivers/of/base.c > +++ b/drivers/of/base.c > @@ -321,6 +321,8 @@ struct device_node *of_get_next_parent(struct device_node *node) > * > * Returns a node pointer with refcount incremented, use > * of_node_put() on it when done. > + * > + * Does not return nodes marked unavailable by a status property. > */ > struct device_node *of_get_next_child(const struct device_node *node, > struct device_node *prev) > @@ -330,7 +332,7 @@ struct device_node *of_get_next_child(const struct device_node *node, > read_lock(&devtree_lock); > next = prev ? prev->sibling : node->child; > for (; next; next = next->sibling) > - if (of_node_get(next)) > + if (of_device_is_available(next) && of_node_get(next)) > break; > of_node_put(prev); > read_unlock(&devtree_lock); This seems like too low-level a place to put this. Some code may know how to un-disable a device in certain situations, or it may be part of debug code trying to dump the whole device tree, etc. Looking further[1], I see a raw version of this function, but not other things like of_find_compatible_node. Could this be done more othogonally, so that the caller specifies in a parameter whether to skip based on status? -Scott [1] For some reason I received some of these patches from linuxppc-dev, and others from devicetree-discuss. I wish lists wouldn't try to be "smart" about discarding duplicates -- it messes with filters. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/5] of/device: Make of_get_next_child() check status properties 2010-12-08 21:01 ` Scott Wood @ 2010-12-09 1:33 ` Michael Ellerman 2010-12-09 3:09 ` David Gibson 0 siblings, 1 reply; 13+ messages in thread From: Michael Ellerman @ 2010-12-09 1:33 UTC (permalink / raw) To: Scott Wood; +Cc: devicetree-discuss, linuxppc-dev, Deepak Saxena [-- Attachment #1: Type: text/plain, Size: 2048 bytes --] On Wed, 2010-12-08 at 15:01 -0600, Scott Wood wrote: > On Wed, 8 Dec 2010 11:29:44 -0800 > Deepak Saxena <deepak_saxena@mentor.com> wrote: > > > We only return the next child if the device is available. > > > > Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com> > > Signed-off-by: Deepak Saxena <deepak_saxena@mentor.com> > > --- > > drivers/of/base.c | 4 +++- > > 1 files changed, 3 insertions(+), 1 deletions(-) > > > > diff --git a/drivers/of/base.c b/drivers/of/base.c > > index 5d269a4..81b2601 100644 > > --- a/drivers/of/base.c > > +++ b/drivers/of/base.c > > @@ -321,6 +321,8 @@ struct device_node *of_get_next_parent(struct device_node *node) > > * > > * Returns a node pointer with refcount incremented, use > > * of_node_put() on it when done. > > + * > > + * Does not return nodes marked unavailable by a status property. > > */ > > struct device_node *of_get_next_child(const struct device_node *node, > > struct device_node *prev) > > @@ -330,7 +332,7 @@ struct device_node *of_get_next_child(const struct device_node *node, > > read_lock(&devtree_lock); > > next = prev ? prev->sibling : node->child; > > for (; next; next = next->sibling) > > - if (of_node_get(next)) > > + if (of_device_is_available(next) && of_node_get(next)) > > break; > > of_node_put(prev); > > read_unlock(&devtree_lock); > > This seems like too low-level a place to put this. Some code may know > how to un-disable a device in certain situations, or it may be part of > debug code trying to dump the whole device tree, etc. Looking > further[1], I see a raw version of this function, but not other things > like of_find_compatible_node. Yeah I agree. I think we'll eventually end up with __ versions of all or lots of them. Not to mention there might be cases you've missed where code expects to see unavailable nodes. The right approach is to add _new_ routines that don't return unavailable nodes, and convert code that you know wants to use them. cheers [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/5] of/device: Make of_get_next_child() check status properties 2010-12-09 1:33 ` Michael Ellerman @ 2010-12-09 3:09 ` David Gibson 2010-12-15 18:35 ` Hollis Blanchard 0 siblings, 1 reply; 13+ messages in thread From: David Gibson @ 2010-12-09 3:09 UTC (permalink / raw) To: Michael Ellerman; +Cc: Scott Wood, devicetree-discuss, linuxppc-dev [-- Attachment #1: Type: text/plain, Size: 2729 bytes --] On Thu, Dec 09, 2010 at 12:33:22PM +1100, Michael Ellerman wrote: > On Wed, 2010-12-08 at 15:01 -0600, Scott Wood wrote: > > On Wed, 8 Dec 2010 11:29:44 -0800 > > Deepak Saxena <deepak_saxena@mentor.com> wrote: > > > > > We only return the next child if the device is available. > > > > > > Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com> > > > Signed-off-by: Deepak Saxena <deepak_saxena@mentor.com> > > > --- > > > drivers/of/base.c | 4 +++- > > > 1 files changed, 3 insertions(+), 1 deletions(-) > > > > > > diff --git a/drivers/of/base.c b/drivers/of/base.c > > > index 5d269a4..81b2601 100644 > > > --- a/drivers/of/base.c > > > +++ b/drivers/of/base.c > > > @@ -321,6 +321,8 @@ struct device_node *of_get_next_parent(struct device_node *node) > > > * > > > * Returns a node pointer with refcount incremented, use > > > * of_node_put() on it when done. > > > + * > > > + * Does not return nodes marked unavailable by a status property. > > > */ > > > struct device_node *of_get_next_child(const struct device_node *node, > > > struct device_node *prev) > > > @@ -330,7 +332,7 @@ struct device_node *of_get_next_child(const struct device_node *node, > > > read_lock(&devtree_lock); > > > next = prev ? prev->sibling : node->child; > > > for (; next; next = next->sibling) > > > - if (of_node_get(next)) > > > + if (of_device_is_available(next) && of_node_get(next)) > > > break; > > > of_node_put(prev); > > > read_unlock(&devtree_lock); > > > > This seems like too low-level a place to put this. Some code may know > > how to un-disable a device in certain situations, or it may be part of > > debug code trying to dump the whole device tree, etc. Looking > > further[1], I see a raw version of this function, but not other things > > like of_find_compatible_node. > > Yeah I agree. I think we'll eventually end up with __ versions of all or > lots of them. Not to mention there might be cases you've missed where > code expects to see unavailable nodes. The right approach is to add > _new_ routines that don't return unavailable nodes, and convert code > that you know wants to use them. Actually, I don't think we really want these status-skipping iterators at all. The device tree iterators should give us the device tree, as it is. Those old-style drivers which seach for a node rather than using the bus probing logic can keep individual checks of the status property until they're converted to the new scheme. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/5] of/device: Make of_get_next_child() check status properties 2010-12-09 3:09 ` David Gibson @ 2010-12-15 18:35 ` Hollis Blanchard 2010-12-15 22:40 ` Michael Ellerman 0 siblings, 1 reply; 13+ messages in thread From: Hollis Blanchard @ 2010-12-15 18:35 UTC (permalink / raw) To: Michael Ellerman, Scott Wood, devicetree-discuss, linuxppc-dev On Wed, Dec 8, 2010 at 7:09 PM, David Gibson <david@gibson.dropbear.id.au> wrote: > On Thu, Dec 09, 2010 at 12:33:22PM +1100, Michael Ellerman wrote: >> On Wed, 2010-12-08 at 15:01 -0600, Scott Wood wrote: >> > On Wed, 8 Dec 2010 11:29:44 -0800 >> > Deepak Saxena <deepak_saxena@mentor.com> wrote: >> > >> > > We only return the next child if the device is available. >> > > >> > > Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com> >> > > Signed-off-by: Deepak Saxena <deepak_saxena@mentor.com> >> > > --- >> > > =A0drivers/of/base.c | =A0 =A04 +++- >> > > =A01 files changed, 3 insertions(+), 1 deletions(-) >> > > >> > > diff --git a/drivers/of/base.c b/drivers/of/base.c >> > > index 5d269a4..81b2601 100644 >> > > --- a/drivers/of/base.c >> > > +++ b/drivers/of/base.c >> > > @@ -321,6 +321,8 @@ struct device_node *of_get_next_parent(struct de= vice_node *node) >> > > =A0 * >> > > =A0 * =A0 =A0 =A0 Returns a node pointer with refcount incremented, = use >> > > =A0 * =A0 =A0 =A0 of_node_put() on it when done. >> > > + * >> > > + * =A0 =A0 =A0 Does not return nodes marked unavailable by a status= property. >> > > =A0 */ >> > > =A0struct device_node *of_get_next_child(const struct device_node *n= ode, >> > > =A0 struct device_node *prev) >> > > @@ -330,7 +332,7 @@ struct device_node *of_get_next_child(const stru= ct device_node *node, >> > > =A0 read_lock(&devtree_lock); >> > > =A0 next =3D prev ? prev->sibling : node->child; >> > > =A0 for (; next; next =3D next->sibling) >> > > - =A0 =A0 =A0 =A0 if (of_node_get(next)) >> > > + =A0 =A0 =A0 =A0 if (of_device_is_available(next) && of_node_get(ne= xt)) >> > > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break; >> > > =A0 of_node_put(prev); >> > > =A0 read_unlock(&devtree_lock); >> > >> > This seems like too low-level a place to put this. =A0Some code may kn= ow >> > how to un-disable a device in certain situations, or it may be part of >> > debug code trying to dump the whole device tree, etc. =A0Looking >> > further[1], I see a raw version of this function, but not other things >> > like of_find_compatible_node. >> >> Yeah I agree. I think we'll eventually end up with __ versions of all or >> lots of them. Not to mention there might be cases you've missed where >> code expects to see unavailable nodes. The right approach is to add >> _new_ routines that don't return unavailable nodes, and convert code >> that you know wants to use them. > > Actually, I don't think we really want these status-skipping > iterators at all. =A0The device tree iterators should give us the device > tree, as it is. =A0Those old-style drivers which seach for a node rather > than using the bus probing logic can keep individual checks of the > status property until they're converted to the new scheme. So the patch should look something like this? @@ -321,6 +321,8 @@ struct device_node *of_get_next_parent(struct device_node *node) * * Returns a node pointer with refcount incremented, use * of_node_put() on it when done. + * + * Do not use this function. */ struct device_node *of_get_next_child(const struct device_node *node, struct device_node *prev) ... + struct device_node *of_get_next_available_child(const struct device_node *node, + struct device_node *prev) + ... + } And then (almost) all the of_get_next_child() sites should be changed to call the new function? -Hollis ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/5] of/device: Make of_get_next_child() check status properties 2010-12-15 18:35 ` Hollis Blanchard @ 2010-12-15 22:40 ` Michael Ellerman 2010-12-31 7:39 ` Grant Likely 0 siblings, 1 reply; 13+ messages in thread From: Michael Ellerman @ 2010-12-15 22:40 UTC (permalink / raw) To: Hollis Blanchard; +Cc: Scott Wood, devicetree-discuss, linuxppc-dev [-- Attachment #1: Type: text/plain, Size: 3511 bytes --] On Wed, 2010-12-15 at 10:35 -0800, Hollis Blanchard wrote: > On Wed, Dec 8, 2010 at 7:09 PM, David Gibson > <david@gibson.dropbear.id.au> wrote: > > On Thu, Dec 09, 2010 at 12:33:22PM +1100, Michael Ellerman wrote: > >> On Wed, 2010-12-08 at 15:01 -0600, Scott Wood wrote: > >> > On Wed, 8 Dec 2010 11:29:44 -0800 > >> > Deepak Saxena <deepak_saxena@mentor.com> wrote: > >> > > >> > > We only return the next child if the device is available. > >> > > > >> > > Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com> > >> > > Signed-off-by: Deepak Saxena <deepak_saxena@mentor.com> > >> > > --- > >> > > drivers/of/base.c | 4 +++- > >> > > 1 files changed, 3 insertions(+), 1 deletions(-) > >> > > > >> > > diff --git a/drivers/of/base.c b/drivers/of/base.c > >> > > index 5d269a4..81b2601 100644 > >> > > --- a/drivers/of/base.c > >> > > +++ b/drivers/of/base.c > >> > > @@ -321,6 +321,8 @@ struct device_node *of_get_next_parent(struct device_node *node) > >> > > * > >> > > * Returns a node pointer with refcount incremented, use > >> > > * of_node_put() on it when done. > >> > > + * > >> > > + * Does not return nodes marked unavailable by a status property. > >> > > */ > >> > > struct device_node *of_get_next_child(const struct device_node *node, > >> > > struct device_node *prev) > >> > > @@ -330,7 +332,7 @@ struct device_node *of_get_next_child(const struct device_node *node, > >> > > read_lock(&devtree_lock); > >> > > next = prev ? prev->sibling : node->child; > >> > > for (; next; next = next->sibling) > >> > > - if (of_node_get(next)) > >> > > + if (of_device_is_available(next) && of_node_get(next)) > >> > > break; > >> > > of_node_put(prev); > >> > > read_unlock(&devtree_lock); > >> > > >> > This seems like too low-level a place to put this. Some code may know > >> > how to un-disable a device in certain situations, or it may be part of > >> > debug code trying to dump the whole device tree, etc. Looking > >> > further[1], I see a raw version of this function, but not other things > >> > like of_find_compatible_node. > >> > >> Yeah I agree. I think we'll eventually end up with __ versions of all or > >> lots of them. Not to mention there might be cases you've missed where > >> code expects to see unavailable nodes. The right approach is to add > >> _new_ routines that don't return unavailable nodes, and convert code > >> that you know wants to use them. > > > > Actually, I don't think we really want these status-skipping > > iterators at all. The device tree iterators should give us the device > > tree, as it is. Those old-style drivers which seach for a node rather > > than using the bus probing logic can keep individual checks of the > > status property until they're converted to the new scheme. > > So the patch should look something like this? > > @@ -321,6 +321,8 @@ struct device_node *of_get_next_parent(struct > device_node *node) > * > * Returns a node pointer with refcount incremented, use > * of_node_put() on it when done. > + * > + * Do not use this function. > */ > struct device_node *of_get_next_child(const struct device_node *node, > struct device_node *prev) Haha. No it should say "this function doesn't lie to you". And the patch should say "this patch _doesn't_ subtly change all callers of of_get_next_child() without carefully auditing them". cheers [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 198 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 3/5] of/device: Make of_get_next_child() check status properties 2010-12-15 22:40 ` Michael Ellerman @ 2010-12-31 7:39 ` Grant Likely 0 siblings, 0 replies; 13+ messages in thread From: Grant Likely @ 2010-12-31 7:39 UTC (permalink / raw) To: Michael Ellerman Cc: Scott Wood, devicetree-discuss, linuxppc-dev, Hollis Blanchard On Thu, Dec 16, 2010 at 09:40:55AM +1100, Michael Ellerman wrote: > On Wed, 2010-12-15 at 10:35 -0800, Hollis Blanchard wrote: > > On Wed, Dec 8, 2010 at 7:09 PM, David Gibson > > <david@gibson.dropbear.id.au> wrote: > > > On Thu, Dec 09, 2010 at 12:33:22PM +1100, Michael Ellerman wrote: > > >> On Wed, 2010-12-08 at 15:01 -0600, Scott Wood wrote: > > >> > On Wed, 8 Dec 2010 11:29:44 -0800 > > >> > Deepak Saxena <deepak_saxena@mentor.com> wrote: > > >> > > > >> > > We only return the next child if the device is available. > > >> > > > > >> > > Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com> > > >> > > Signed-off-by: Deepak Saxena <deepak_saxena@mentor.com> > > >> > > --- > > >> > > drivers/of/base.c | 4 +++- > > >> > > 1 files changed, 3 insertions(+), 1 deletions(-) > > >> > > > > >> > > diff --git a/drivers/of/base.c b/drivers/of/base.c > > >> > > index 5d269a4..81b2601 100644 > > >> > > --- a/drivers/of/base.c > > >> > > +++ b/drivers/of/base.c > > >> > > @@ -321,6 +321,8 @@ struct device_node *of_get_next_parent(struct device_node *node) > > >> > > * > > >> > > * Returns a node pointer with refcount incremented, use > > >> > > * of_node_put() on it when done. > > >> > > + * > > >> > > + * Does not return nodes marked unavailable by a status property. > > >> > > */ > > >> > > struct device_node *of_get_next_child(const struct device_node *node, > > >> > > struct device_node *prev) > > >> > > @@ -330,7 +332,7 @@ struct device_node *of_get_next_child(const struct device_node *node, > > >> > > read_lock(&devtree_lock); > > >> > > next = prev ? prev->sibling : node->child; > > >> > > for (; next; next = next->sibling) > > >> > > - if (of_node_get(next)) > > >> > > + if (of_device_is_available(next) && of_node_get(next)) > > >> > > break; > > >> > > of_node_put(prev); > > >> > > read_unlock(&devtree_lock); > > >> > > > >> > This seems like too low-level a place to put this. Some code may know > > >> > how to un-disable a device in certain situations, or it may be part of > > >> > debug code trying to dump the whole device tree, etc. Looking > > >> > further[1], I see a raw version of this function, but not other things > > >> > like of_find_compatible_node. > > >> > > >> Yeah I agree. I think we'll eventually end up with __ versions of all or > > >> lots of them. Not to mention there might be cases you've missed where > > >> code expects to see unavailable nodes. The right approach is to add > > >> _new_ routines that don't return unavailable nodes, and convert code > > >> that you know wants to use them. > > > > > > Actually, I don't think we really want these status-skipping > > > iterators at all. The device tree iterators should give us the device > > > tree, as it is. Those old-style drivers which seach for a node rather > > > than using the bus probing logic can keep individual checks of the > > > status property until they're converted to the new scheme. > > > > So the patch should look something like this? > > > > @@ -321,6 +321,8 @@ struct device_node *of_get_next_parent(struct > > device_node *node) > > * > > * Returns a node pointer with refcount incremented, use > > * of_node_put() on it when done. > > + * > > + * Do not use this function. > > */ > > struct device_node *of_get_next_child(const struct device_node *node, > > struct device_node *prev) > > Haha. No it should say "this function doesn't lie to you". > > And the patch should say "this patch _doesn't_ subtly change all callers > of of_get_next_child() without carefully auditing them". Heh, Yes. The comments made on this patch are totally on-base. Not all nodes are devices, and not all callers will want to skip nodes; regardless of the reason for skipping. Case in point: the /proc/device-tree support code. If a caller needs a version of the function that skips unavailable nodes, then that behaviour should be explicitly asked for. In this case it should be a new function with a new name. Don't change the behaviour out from under the existing users. g. ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/5] of/device: make for_each_node* check status properties [not found] <cover.1291799069.git.deepak_saxena@mentor.com> ` (3 preceding siblings ...) 2010-12-08 19:29 ` [PATCH 3/5] of/device: Make of_get_next_child() check status properties Deepak Saxena @ 2010-12-08 19:29 ` Deepak Saxena 2010-12-31 7:46 ` Grant Likely 4 siblings, 1 reply; 13+ messages in thread From: Deepak Saxena @ 2010-12-08 19:29 UTC (permalink / raw) To: devicetree-discuss; +Cc: linuxppc-dev Some early device drivers don't actually implement a struct of_platform_driver, and instead use the for_each_node* iterators to search for matching device nodes. Disabled device nodes will no longer be returned by these iterators. Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com> Signed-off-by: Deepak Saxena <deepak_saxena@mentor.com> --- arch/powerpc/kernel/pci_of_scan.c | 2 -- arch/powerpc/platforms/83xx/usb.c | 2 +- arch/powerpc/platforms/85xx/mpc85xx_mds.c | 13 ++----------- arch/powerpc/sysdev/fsl_pci.c | 5 ----- arch/powerpc/sysdev/ppc4xx_pci.c | 15 --------------- drivers/of/base.c | 25 +++++++++++++++++++++++-- 6 files changed, 26 insertions(+), 36 deletions(-) diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c index e751506..fa4f719 100644 --- a/arch/powerpc/kernel/pci_of_scan.c +++ b/arch/powerpc/kernel/pci_of_scan.c @@ -310,8 +310,6 @@ static void __devinit __of_scan_bus(struct device_node *node, /* Scan direct children */ for_each_child_of_node(node, child) { pr_debug(" * %s\n", child->full_name); - if (!of_device_is_available(child)) - continue; reg = of_get_property(child, "reg", ®len); if (reg == NULL || reglen < 20) continue; diff --git a/arch/powerpc/platforms/83xx/usb.c b/arch/powerpc/platforms/83xx/usb.c index 3ba4bb7..749c70b 100644 --- a/arch/powerpc/platforms/83xx/usb.c +++ b/arch/powerpc/platforms/83xx/usb.c @@ -211,7 +211,7 @@ int mpc837x_usb_cfg(void) int ret = 0; np = of_find_compatible_node(NULL, NULL, "fsl-usb2-dr"); - if (!np || !of_device_is_available(np)) + if (!np) return -ENODEV; prop = of_get_property(np, "phy_type", NULL); diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c b/arch/powerpc/platforms/85xx/mpc85xx_mds.c index aa34cac..5d1a33f 100644 --- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c +++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c @@ -170,10 +170,8 @@ static void __init mpc85xx_publish_qe_devices(void) struct device_node *np; np = of_find_compatible_node(NULL, NULL, "fsl,qe"); - if (!of_device_is_available(np)) { - of_node_put(np); + if (!np) return; - } of_platform_bus_probe(NULL, mpc85xx_qe_ids, NULL); } @@ -267,11 +265,6 @@ static void __init mpc85xx_mds_qe_init(void) return; } - if (!of_device_is_available(np)) { - of_node_put(np); - return; - } - qe_reset(); of_node_put(np); @@ -328,10 +321,8 @@ static void __init mpc85xx_mds_qeic_init(void) struct device_node *np; np = of_find_compatible_node(NULL, NULL, "fsl,qe"); - if (!of_device_is_available(np)) { - of_node_put(np); + if (!np) return; - } np = of_find_compatible_node(NULL, NULL, "fsl,qe-ic"); if (!np) { diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c index 818f7c6..fba4ec7 100644 --- a/arch/powerpc/sysdev/fsl_pci.c +++ b/arch/powerpc/sysdev/fsl_pci.c @@ -623,11 +623,6 @@ int __init mpc83xx_add_bridge(struct device_node *dev) is_mpc83xx_pci = 1; - if (!of_device_is_available(dev)) { - pr_warning("%s: disabled by the firmware.\n", - dev->full_name); - return -ENODEV; - } pr_debug("Adding PCI host bridge %s\n", dev->full_name); /* Fetch host bridge registers address */ diff --git a/arch/powerpc/sysdev/ppc4xx_pci.c b/arch/powerpc/sysdev/ppc4xx_pci.c index 156aa7d..cf2adec 100644 --- a/arch/powerpc/sysdev/ppc4xx_pci.c +++ b/arch/powerpc/sysdev/ppc4xx_pci.c @@ -321,13 +321,6 @@ static void __init ppc4xx_probe_pci_bridge(struct device_node *np) const int *bus_range; int primary = 0; - /* Check if device is enabled */ - if (!of_device_is_available(np)) { - printk(KERN_INFO "%s: Port disabled via device-tree\n", - np->full_name); - return; - } - /* Fetch config space registers address */ if (of_address_to_resource(np, 0, &rsrc_cfg)) { printk(KERN_ERR "%s: Can't get PCI config register base !", @@ -1890,14 +1883,6 @@ static void __init ppc4xx_probe_pciex_bridge(struct device_node *np) port = &ppc4xx_pciex_ports[portno]; port->index = portno; - /* - * Check if device is enabled - */ - if (!of_device_is_available(np)) { - printk(KERN_INFO "PCIE%d: Port disabled via device-tree\n", port->index); - return; - } - port->node = of_node_get(np); pval = of_get_property(np, "sdr-base", NULL); if (pval == NULL) { diff --git a/drivers/of/base.c b/drivers/of/base.c index 710b53b..5d269a4 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -344,6 +344,8 @@ EXPORT_SYMBOL(of_get_next_child); * * Returns a node pointer with refcount incremented, use * of_node_put() on it when done. + * + * Does not return nodes marked unavailable by a status property. */ struct device_node *of_find_node_by_path(const char *path) { @@ -352,6 +354,7 @@ struct device_node *of_find_node_by_path(const char *path) read_lock(&devtree_lock); for (; np; np = np->allnext) { if (np->full_name && (of_node_cmp(np->full_name, path) == 0) + && of_device_is_available(np) && of_node_get(np)) break; } @@ -370,6 +373,8 @@ EXPORT_SYMBOL(of_find_node_by_path); * * Returns a node pointer with refcount incremented, use * of_node_put() on it when done. + * + * Does not return nodes marked unavailable by a status property. */ struct device_node *of_find_node_by_name(struct device_node *from, const char *name) @@ -380,6 +385,7 @@ struct device_node *of_find_node_by_name(struct device_node *from, np = from ? from->allnext : allnodes; for (; np; np = np->allnext) if (np->name && (of_node_cmp(np->name, name) == 0) + && of_device_is_available(np) && of_node_get(np)) break; of_node_put(from); @@ -399,6 +405,8 @@ EXPORT_SYMBOL(of_find_node_by_name); * * Returns a node pointer with refcount incremented, use * of_node_put() on it when done. + * + * Does not return nodes marked unavailable by a status property. */ struct device_node *of_find_node_by_type(struct device_node *from, const char *type) @@ -409,6 +417,7 @@ struct device_node *of_find_node_by_type(struct device_node *from, np = from ? from->allnext : allnodes; for (; np; np = np->allnext) if (np->type && (of_node_cmp(np->type, type) == 0) + && of_device_is_available(np) && of_node_get(np)) break; of_node_put(from); @@ -430,6 +439,8 @@ EXPORT_SYMBOL(of_find_node_by_type); * * Returns a node pointer with refcount incremented, use * of_node_put() on it when done. + * + * Does not return nodes marked unavailable by a status property. */ struct device_node *of_find_compatible_node(struct device_node *from, const char *type, const char *compatible) @@ -442,7 +453,9 @@ struct device_node *of_find_compatible_node(struct device_node *from, if (type && !(np->type && (of_node_cmp(np->type, type) == 0))) continue; - if (of_device_is_compatible(np, compatible) && of_node_get(np)) + if (of_device_is_compatible(np, compatible) + && of_device_is_available(np) + && of_node_get(np)) break; } of_node_put(from); @@ -462,6 +475,8 @@ EXPORT_SYMBOL(of_find_compatible_node); * * Returns a node pointer with refcount incremented, use * of_node_put() on it when done. + * + * Does not return nodes marked unavailable by a status property. */ struct device_node *of_find_node_with_property(struct device_node *from, const char *prop_name) @@ -472,6 +487,8 @@ struct device_node *of_find_node_with_property(struct device_node *from, read_lock(&devtree_lock); np = from ? from->allnext : allnodes; for (; np; np = np->allnext) { + if (!of_device_is_available(np)) + continue; for (pp = np->properties; pp != 0; pp = pp->next) { if (of_prop_cmp(pp->name, prop_name) == 0) { of_node_get(np); @@ -526,6 +543,8 @@ EXPORT_SYMBOL(of_match_node); * * Returns a node pointer with refcount incremented, use * of_node_put() on it when done. + * + * Does not return nodes marked unavailable by a status property. */ struct device_node *of_find_matching_node(struct device_node *from, const struct of_device_id *matches) @@ -535,7 +554,9 @@ struct device_node *of_find_matching_node(struct device_node *from, read_lock(&devtree_lock); np = from ? from->allnext : allnodes; for (; np; np = np->allnext) { - if (of_match_node(matches, np) && of_node_get(np)) + if (of_match_node(matches, np) + && of_device_is_available(np) + && of_node_get(np)) break; } of_node_put(from); -- 1.6.3.3 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 2/5] of/device: make for_each_node* check status properties 2010-12-08 19:29 ` [PATCH 2/5] of/device: make for_each_node* " Deepak Saxena @ 2010-12-31 7:46 ` Grant Likely 0 siblings, 0 replies; 13+ messages in thread From: Grant Likely @ 2010-12-31 7:46 UTC (permalink / raw) To: Deepak Saxena; +Cc: devicetree-discuss, linuxppc-dev On Wed, Dec 08, 2010 at 11:29:57AM -0800, Deepak Saxena wrote: > Some early device drivers don't actually implement a struct of_platform_driver, > and instead use the for_each_node* iterators to search for matching device > nodes. Disabled device nodes will no longer be returned by these iterators. > > Signed-off-by: Hollis Blanchard <hollis_blanchard@mentor.com> > Signed-off-by: Deepak Saxena <deepak_saxena@mentor.com> As already discussed, the generic device tree iterator and search functions should not be filtering the results as done here. g. > --- > arch/powerpc/kernel/pci_of_scan.c | 2 -- > arch/powerpc/platforms/83xx/usb.c | 2 +- > arch/powerpc/platforms/85xx/mpc85xx_mds.c | 13 ++----------- > arch/powerpc/sysdev/fsl_pci.c | 5 ----- > arch/powerpc/sysdev/ppc4xx_pci.c | 15 --------------- > drivers/of/base.c | 25 +++++++++++++++++++++++-- > 6 files changed, 26 insertions(+), 36 deletions(-) > > diff --git a/arch/powerpc/kernel/pci_of_scan.c b/arch/powerpc/kernel/pci_of_scan.c > index e751506..fa4f719 100644 > --- a/arch/powerpc/kernel/pci_of_scan.c > +++ b/arch/powerpc/kernel/pci_of_scan.c > @@ -310,8 +310,6 @@ static void __devinit __of_scan_bus(struct device_node *node, > /* Scan direct children */ > for_each_child_of_node(node, child) { > pr_debug(" * %s\n", child->full_name); > - if (!of_device_is_available(child)) > - continue; > reg = of_get_property(child, "reg", ®len); > if (reg == NULL || reglen < 20) > continue; > diff --git a/arch/powerpc/platforms/83xx/usb.c b/arch/powerpc/platforms/83xx/usb.c > index 3ba4bb7..749c70b 100644 > --- a/arch/powerpc/platforms/83xx/usb.c > +++ b/arch/powerpc/platforms/83xx/usb.c > @@ -211,7 +211,7 @@ int mpc837x_usb_cfg(void) > int ret = 0; > > np = of_find_compatible_node(NULL, NULL, "fsl-usb2-dr"); > - if (!np || !of_device_is_available(np)) > + if (!np) > return -ENODEV; > prop = of_get_property(np, "phy_type", NULL); > > diff --git a/arch/powerpc/platforms/85xx/mpc85xx_mds.c b/arch/powerpc/platforms/85xx/mpc85xx_mds.c > index aa34cac..5d1a33f 100644 > --- a/arch/powerpc/platforms/85xx/mpc85xx_mds.c > +++ b/arch/powerpc/platforms/85xx/mpc85xx_mds.c > @@ -170,10 +170,8 @@ static void __init mpc85xx_publish_qe_devices(void) > struct device_node *np; > > np = of_find_compatible_node(NULL, NULL, "fsl,qe"); > - if (!of_device_is_available(np)) { > - of_node_put(np); > + if (!np) > return; > - } > > of_platform_bus_probe(NULL, mpc85xx_qe_ids, NULL); > } > @@ -267,11 +265,6 @@ static void __init mpc85xx_mds_qe_init(void) > return; > } > > - if (!of_device_is_available(np)) { > - of_node_put(np); > - return; > - } > - > qe_reset(); > of_node_put(np); > > @@ -328,10 +321,8 @@ static void __init mpc85xx_mds_qeic_init(void) > struct device_node *np; > > np = of_find_compatible_node(NULL, NULL, "fsl,qe"); > - if (!of_device_is_available(np)) { > - of_node_put(np); > + if (!np) > return; > - } > > np = of_find_compatible_node(NULL, NULL, "fsl,qe-ic"); > if (!np) { > diff --git a/arch/powerpc/sysdev/fsl_pci.c b/arch/powerpc/sysdev/fsl_pci.c > index 818f7c6..fba4ec7 100644 > --- a/arch/powerpc/sysdev/fsl_pci.c > +++ b/arch/powerpc/sysdev/fsl_pci.c > @@ -623,11 +623,6 @@ int __init mpc83xx_add_bridge(struct device_node *dev) > > is_mpc83xx_pci = 1; > > - if (!of_device_is_available(dev)) { > - pr_warning("%s: disabled by the firmware.\n", > - dev->full_name); > - return -ENODEV; > - } > pr_debug("Adding PCI host bridge %s\n", dev->full_name); > > /* Fetch host bridge registers address */ > diff --git a/arch/powerpc/sysdev/ppc4xx_pci.c b/arch/powerpc/sysdev/ppc4xx_pci.c > index 156aa7d..cf2adec 100644 > --- a/arch/powerpc/sysdev/ppc4xx_pci.c > +++ b/arch/powerpc/sysdev/ppc4xx_pci.c > @@ -321,13 +321,6 @@ static void __init ppc4xx_probe_pci_bridge(struct device_node *np) > const int *bus_range; > int primary = 0; > > - /* Check if device is enabled */ > - if (!of_device_is_available(np)) { > - printk(KERN_INFO "%s: Port disabled via device-tree\n", > - np->full_name); > - return; > - } > - > /* Fetch config space registers address */ > if (of_address_to_resource(np, 0, &rsrc_cfg)) { > printk(KERN_ERR "%s: Can't get PCI config register base !", > @@ -1890,14 +1883,6 @@ static void __init ppc4xx_probe_pciex_bridge(struct device_node *np) > port = &ppc4xx_pciex_ports[portno]; > port->index = portno; > > - /* > - * Check if device is enabled > - */ > - if (!of_device_is_available(np)) { > - printk(KERN_INFO "PCIE%d: Port disabled via device-tree\n", port->index); > - return; > - } > - > port->node = of_node_get(np); > pval = of_get_property(np, "sdr-base", NULL); > if (pval == NULL) { > diff --git a/drivers/of/base.c b/drivers/of/base.c > index 710b53b..5d269a4 100644 > --- a/drivers/of/base.c > +++ b/drivers/of/base.c > @@ -344,6 +344,8 @@ EXPORT_SYMBOL(of_get_next_child); > * > * Returns a node pointer with refcount incremented, use > * of_node_put() on it when done. > + * > + * Does not return nodes marked unavailable by a status property. > */ > struct device_node *of_find_node_by_path(const char *path) > { > @@ -352,6 +354,7 @@ struct device_node *of_find_node_by_path(const char *path) > read_lock(&devtree_lock); > for (; np; np = np->allnext) { > if (np->full_name && (of_node_cmp(np->full_name, path) == 0) > + && of_device_is_available(np) > && of_node_get(np)) > break; > } > @@ -370,6 +373,8 @@ EXPORT_SYMBOL(of_find_node_by_path); > * > * Returns a node pointer with refcount incremented, use > * of_node_put() on it when done. > + * > + * Does not return nodes marked unavailable by a status property. > */ > struct device_node *of_find_node_by_name(struct device_node *from, > const char *name) > @@ -380,6 +385,7 @@ struct device_node *of_find_node_by_name(struct device_node *from, > np = from ? from->allnext : allnodes; > for (; np; np = np->allnext) > if (np->name && (of_node_cmp(np->name, name) == 0) > + && of_device_is_available(np) > && of_node_get(np)) > break; > of_node_put(from); > @@ -399,6 +405,8 @@ EXPORT_SYMBOL(of_find_node_by_name); > * > * Returns a node pointer with refcount incremented, use > * of_node_put() on it when done. > + * > + * Does not return nodes marked unavailable by a status property. > */ > struct device_node *of_find_node_by_type(struct device_node *from, > const char *type) > @@ -409,6 +417,7 @@ struct device_node *of_find_node_by_type(struct device_node *from, > np = from ? from->allnext : allnodes; > for (; np; np = np->allnext) > if (np->type && (of_node_cmp(np->type, type) == 0) > + && of_device_is_available(np) > && of_node_get(np)) > break; > of_node_put(from); > @@ -430,6 +439,8 @@ EXPORT_SYMBOL(of_find_node_by_type); > * > * Returns a node pointer with refcount incremented, use > * of_node_put() on it when done. > + * > + * Does not return nodes marked unavailable by a status property. > */ > struct device_node *of_find_compatible_node(struct device_node *from, > const char *type, const char *compatible) > @@ -442,7 +453,9 @@ struct device_node *of_find_compatible_node(struct device_node *from, > if (type > && !(np->type && (of_node_cmp(np->type, type) == 0))) > continue; > - if (of_device_is_compatible(np, compatible) && of_node_get(np)) > + if (of_device_is_compatible(np, compatible) > + && of_device_is_available(np) > + && of_node_get(np)) > break; > } > of_node_put(from); > @@ -462,6 +475,8 @@ EXPORT_SYMBOL(of_find_compatible_node); > * > * Returns a node pointer with refcount incremented, use > * of_node_put() on it when done. > + * > + * Does not return nodes marked unavailable by a status property. > */ > struct device_node *of_find_node_with_property(struct device_node *from, > const char *prop_name) > @@ -472,6 +487,8 @@ struct device_node *of_find_node_with_property(struct device_node *from, > read_lock(&devtree_lock); > np = from ? from->allnext : allnodes; > for (; np; np = np->allnext) { > + if (!of_device_is_available(np)) > + continue; > for (pp = np->properties; pp != 0; pp = pp->next) { > if (of_prop_cmp(pp->name, prop_name) == 0) { > of_node_get(np); > @@ -526,6 +543,8 @@ EXPORT_SYMBOL(of_match_node); > * > * Returns a node pointer with refcount incremented, use > * of_node_put() on it when done. > + * > + * Does not return nodes marked unavailable by a status property. > */ > struct device_node *of_find_matching_node(struct device_node *from, > const struct of_device_id *matches) > @@ -535,7 +554,9 @@ struct device_node *of_find_matching_node(struct device_node *from, > read_lock(&devtree_lock); > np = from ? from->allnext : allnodes; > for (; np; np = np->allnext) { > - if (of_match_node(matches, np) && of_node_get(np)) > + if (of_match_node(matches, np) > + && of_device_is_available(np) > + && of_node_get(np)) > break; > } > of_node_put(from); > -- > 1.6.3.3 > > _______________________________________________ > devicetree-discuss mailing list > devicetree-discuss@lists.ozlabs.org > https://lists.ozlabs.org/listinfo/devicetree-discuss ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2010-12-31 7:46 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <cover.1291799069.git.deepak_saxena@mentor.com> 2010-12-08 19:29 ` [PATCH 1/5] of/device: Centralize checking of 'status' properties Deepak Saxena 2010-12-31 7:43 ` Grant Likely 2010-12-08 19:29 ` [PATCH 4/5] of/device: Create _of_get_next_child() Deepak Saxena 2010-12-08 19:29 ` [PATCH 5/5] of/device: Show even unavailable nodes in procfs Deepak Saxena 2010-12-08 19:29 ` [PATCH 3/5] of/device: Make of_get_next_child() check status properties Deepak Saxena 2010-12-08 21:01 ` Scott Wood 2010-12-09 1:33 ` Michael Ellerman 2010-12-09 3:09 ` David Gibson 2010-12-15 18:35 ` Hollis Blanchard 2010-12-15 22:40 ` Michael Ellerman 2010-12-31 7:39 ` Grant Likely 2010-12-08 19:29 ` [PATCH 2/5] of/device: make for_each_node* " Deepak Saxena 2010-12-31 7:46 ` Grant Likely
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).