checking status semantics with compatible functions

checking status semantics with compatible functions

[Kumar Gala]

Grant,

We have a few helper functions like:

drivers/of/base.c:EXPORT_SYMBOL(of_device_is_compatible);
drivers/of/base.c:EXPORT_SYMBOL(of_find_compatible_node);

That we use in places like arch/powerpc/platform/85xx/mpc85xx_ds.c:

        for_each_node_by_type(np, "pci") {
                if (of_device_is_compatible(np, "fsl,mpc8540-pci") ||
                    of_device_is_compatible(np, "fsl,mpc8548-pcie") ||
                    of_device_is_compatible(np, "fsl,p2020-pcie")) {

It seems like we should check 'status' in these cases with 'of_device_is_available'.  I'm thinking that we should have versions of the helpers that also do the 'of_device_is_available' checking.  Or should we just change the helpers to call of_device_is_available?

- k

Re: checking status semantics with compatible functions

[Kumar Gala]

[Adding proper lists on CC]

- k

On Mar 30, 2011, at 3:07 AM, Kumar Gala wrote:

> Grant,
> 
> We have a few helper functions like:
> 
> drivers/of/base.c:EXPORT_SYMBOL(of_device_is_compatible);
> drivers/of/base.c:EXPORT_SYMBOL(of_find_compatible_node);
> 
> That we use in places like arch/powerpc/platform/85xx/mpc85xx_ds.c:
> 
>        for_each_node_by_type(np, "pci") {
>                if (of_device_is_compatible(np, "fsl,mpc8540-pci") ||
>                    of_device_is_compatible(np, "fsl,mpc8548-pcie") ||
>                    of_device_is_compatible(np, "fsl,p2020-pcie")) {
> 
> It seems like we should check 'status' in these cases with 'of_device_is_available'.  I'm thinking that we should have versions of the helpers that also do the 'of_device_is_available' checking.  Or should we just change the helpers to call of_device_is_available?
> 
> - k
> _______________________________________________
> devicetree-discuss mailing list
> devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
> https://lists.ozlabs.org/listinfo/devicetree-discuss

Re: checking status semantics with compatible functions

[Grant Likely]

On Wed, Mar 30, 2011 at 03:26:29AM -0500, Kumar Gala wrote:
> [Adding proper lists on CC]
> 
> - k
> 
> On Mar 30, 2011, at 3:07 AM, Kumar Gala wrote:
> 
> > Grant,
> > 
> > We have a few helper functions like:
> > 
> > drivers/of/base.c:EXPORT_SYMBOL(of_device_is_compatible);
> > drivers/of/base.c:EXPORT_SYMBOL(of_find_compatible_node);
> > 
> > That we use in places like arch/powerpc/platform/85xx/mpc85xx_ds.c:
> > 
> >        for_each_node_by_type(np, "pci") {
> >                if (of_device_is_compatible(np, "fsl,mpc8540-pci") ||
> >                    of_device_is_compatible(np, "fsl,mpc8548-pcie") ||
> >                    of_device_is_compatible(np, "fsl,p2020-pcie")) {
> > 
> > It seems like we should check 'status' in these cases with
> > 'of_device_is_available'.  I'm thinking that we should have
> > versions of the helpers that also do the 'of_device_is_available'
> > checking.  Or should we just change the helpers to call
> > of_device_is_available?

Yes, of_device_is_available() should be checked, but it should not be
added directly to of_device_is_compatible().  I'm okay with adding
a helper variant that does the of_device_is_compatible() check.

In that particular case, I'd also suggest using for_each_matching_node().

g.

Re: checking status semantics with compatible functions

[Benjamin Herrenschmidt]

On Wed, 2011-03-30 at 08:31 -0600, Grant Likely wrote:
> Yes, of_device_is_available() should be checked, but it should not be
> added directly to of_device_is_compatible().  I'm okay with adding
> a helper variant that does the of_device_is_compatible() check.
> 
> In that particular case, I'd also suggest using
> for_each_matching_node().

Agreed. A device might be unavailable due to how FW configured the
machine, but made later on available by the kernel, that shouldn't
impact the interface compatibility test.

Cheers,
Ben.