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

[Stephen Warren]

On 06/28/2012 02:58 PM, Mitch Bradley wrote:
> On 6/28/2012 10:51 AM, Stephen Warren wrote:
>> On 06/28/2012 02:28 PM, Mitch Bradley wrote:
...
>>> 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.
>>
>> There's more needed than that:-(
>>
>> I think of_get_address() still wants to check the address-cells value.
>> Splitting up OF_CHECK_COUNTS as follows:
>>
>> #define OF_CHECK_ADDR_COUNTS(na) ((na) > 0 && (na) <= OF_MAX_ADDR_CELLS)
>> #define OF_CHECK_COUNTS(na, ns)    (OF_CHECK_ADDR_COUNTS(na) && (ns) > 0)
>>
>> ... and using OF_CHECK_ADDR_COUNTS in place of OF_CHECK_COUNTS in
>> of_get_address() would allow that.
> 
> I agree.
> 
>> However, that still leaves of_device_make_bus_id() calling
>> of_translate_address() in order to generate the device name. That will
>> issue the diagnostic and fall back to the global counter.
> 
> My suggestion was to eliminate the diagnostic message or downgrade it to
> warning or debug level.  Then the naming heuristic would be:
> 
>    First try of_translate_address() to get a globally-unique address.
>    Failing that, try of_get_address() to get a locally-unique bound
>      address.
>    Failing that, use a counter.

Ah yes.

I'd somewhat prefer to avoid calling of_translate_address() when we
know it's going to fail though, by passing a parameter to
of_device_make_bus_id() indicating whether to use that or
of_get_address(). The parameter would be set based on enumerated-bus vs.
any other bus type. I coded that locally and it seems to work OK. That
way, the diagnostic in of_translate_address() can be left unchanged; it
seems like it really is an error.

>> It seems like
>> we could make of_device_make_bus_id() call of_get_address() instead of
>> of_translate_address(). However, that would only give the value of the
>> reg property, and not apply the translations all the way to the top of
>> the tree. That in turn means that the following two nodes would be named
>> the same:
...
>> But that would still expose us to the same non-unique-name issue if
>> those were enumerated buses:
>>
>> / {
>>      container-a {
>>          compatible = "enumerated-bus";
>>          ranges = <...>;
>>          regulator {
>>              reg = <0>;
>>          };
>>      };
>>      container-b {
>>          compatible = "enumerated-bus";
>>          ranges = <...>;
>>          regulator {
>>              reg = <0>;
>>          };
>>      };
>> };
>>
>> Do we need to include the full DT path to an enumerated device, or some
>> other unique data for the node's bus, in the device name of a child of
>> an enumerated bus?
> 
> I don't know.  Is the device name space flat?

Hmmm. It depends on where you look.

I end up with /sys/devices/regulators.3/[01].regulator/ this way, the
path structure of which is defined by the parent/child device
relationship, and implies that device names are relative to their parent.

However, device names are treated as a flat namespace in many other
places. For example, clocks, regulators, pinctrl settings, etc. are
looked up by device name, and the names in the lookup tables are just
the device name itself, with no reference to the device's parent.