Re: #size-cells = <0> in a bus node, and kernel messages complaining about this

On 6/28/2012 8:49 AM, Mitch Bradley wrote:

On 6/28/2012 8:21 AM, Mitch Bradley wrote:

quoted

On 6/28/2012 7:50 AM, Stephen Warren wrote:

quoted

On 06/27/2012 06:57 PM, Mitch Bradley wrote:

quoted

On 6/27/2012 11:26 AM, Stephen Warren wrote:

quoted

I believe I've seen the following construct bandied about as the
correct
way of representing a bunch of nodes that have the same name (since
they
represent the same type of object) within device-tree.

     regulators {
         compatible = "simple-bus";
         #address-cells = <1>;
         #size-cells = <0>;

         regulator@0 {
             compatible = "regulator-fixed";
             reg = <0>;
...
         };

         regulator@1 {
             compatible = "regulator-fixed";
             reg = <1>;
...
         };
     };

However, when the kernel parses that, it issues messages such as:

prom_parse: Bad cell count for /regulators/regulator@0
prom_parse: Bad cell count for /regulators/regulator@1

The message comes from __of_translate_address(), which has the comment:

  * Note: We consider that crossing any level with #size-cells == 0
to mean
  * that translation is impossible (that is we are not dealing with a
value
  * that can be mapped to a cpu physical address). This is not really
specified
  * that way, but this is traditionally the way IBM at least do things

So it seems that the problem only occurs if something tries to
translate
the regulator's "reg" address to a CPU address.  Is that
possible/meaningful
in your case?

That's quite likely.

Note that the regulators node is compatible = "simple-bus", and I'm
doing that so that the child regulator nodes are automatically recursed
into, and a platform device created for each. Part of creating those
platform devices is to convert the reg and interrupts properties to
platform device resources, which is almost certainly what's calling
__of_translate_address(). This is a compatibility thing on ARM; I guess
pure OF-style drivers call something like of_get_address()/of_iomap()
themselves in their probe() function if appropriate, rather than relying
on calling platform_get_resource(), and hence forcing the DT parsing
code to convert resources beforehand in all cases, even if not used.

Perhaps simple-bus could be enhanced to detect when size-cells==0 and
know that this means the bus is really just a container/grouping of
non-addressed objects, and hence not provide the memory resources. Or,
perhaps we should introduce a new compatible value that only triggers
child instantiation but not resource setup, or something like that?

ISTM that if you want different semantics, a different compatible value
is the best approach.  My mental model is that "simple-bus" has
memory-mapped children. Perhaps something like "enumerated-bus" for
non-memory-mapped ?

I just looked at the code in platform.c that calls
of_translate_address() (in of_device_make_bus_id()).  It appears that
the reason for translating the address is to create a unique device name
via:

     dev_set_name(dev, "%llx.%s", addr, node->name);

Failure of_translate_address() is not treated as an error, rather the
code falls back to naming the device with an incrementing number:

     dev_set_name(dev, "%s.%d", node->name, magic - 1);

Perhaps the solution is add an additional step to the heuristic.  If
of_translate_address() fails, try to use the untranslated reg property
value to make the name unique, generating a name like:

     name.N,M

where N,M is a list of #address-cells numbers from the first entry of
the reg property value.

That would be better than the existing heuristic (which includes the
phrase "(and pray ...)" in its commentary), and fully consistent with
device tree dogma.

One could still add a new compatible value to distinguish that from a
"simple-bus", as platform.c has a match table that includes simple-bus
and others.  The heuristic for making the unique name is not contingent
upon a specific bus name.

Hmmm.

1) The "KERN_ERR "prom_parse: Bad cell count ..." message is 
presumptuous, if one treats failure of of_translate_address() other than 
as a hard failure.  So if the device naming heuristic were to be 
improved as suggested above, that message should probably be removed, or 
at least downgraded to a warning or debug message.

2) of_get_address() also uses OF_CHECK_COUNTS(), which is unhappy with 
#size-cells == 0.  That's not right.  #size-cells==0 is perfectly 
reasonable for enumerated children and should be supported.

I'm reasonably certain that the right fix is simply to remove:

	if (!OF_CHECK_COUNTS(na, ns))
		return NULL;

from of_get_address().  I looked at all of the places that use 
of_get_address().  With two exceptions, of_get_address() is followed by 
of_translate_address(), which will (correctly) fail if #size-cells is 0. 
  The two exceptions are for devices that can only exist below specific 
buses, which presumably are known to have nonzero #size-cells.

quoted

quoted

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org
https://lists.ozlabs.org/listinfo/devicetree-discuss