Question about "unit address format error" of DTC

Question about "unit address format error" of DTC

[Masahiro Yamada]

Hi Rob, David,


I am just curious about a DTC warning.


For example,

soc {
    compatible = "simple-bus";
    #address-cells = <1>;
    #size-cells = <1>;
    ranges;

    foo@11111111 {
         compatible = "foo";
         reg = <0x54006800 0x40>;
    };
};

This emits the following warning.

Warning (simple_bus_reg): Node /soc/foo@11111111 simple-bus unit
address format error, expected "54006800"


But, if I replace "simple-bus" with "simple-mfd",
DTC is completely happy.

Why is this check limited to simple bus / PCI bus?

For other cases, is mismatching @<address>  allowed?



-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Question about "unit address format error" of DTC

[Rob Herring]

On Thu, Nov 9, 2017 at 6:14 AM, Masahiro Yamada
<yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org> wrote:
> Hi Rob, David,
>
>
> I am just curious about a DTC warning.
>
>
> For example,
>
> soc {
>     compatible = "simple-bus";
>     #address-cells = <1>;
>     #size-cells = <1>;
>     ranges;
>
>     foo@11111111 {
>          compatible = "foo";
>          reg = <0x54006800 0x40>;
>     };
> };
>
> This emits the following warning.
>
> Warning (simple_bus_reg): Node /soc/foo@11111111 simple-bus unit
> address format error, expected "54006800"
>
>
> But, if I replace "simple-bus" with "simple-mfd",
> DTC is completely happy.

We could probably add simple-mfd to the check. Though, if you have
addresses, then you probably should use simple-bus. I'm not a bit fan
of simple-mfd in general.

> Why is this check limited to simple bus / PCI bus?

Because bus types are free to define their own unit address format to
some extent.

> For other cases, is mismatching @<address>  allowed?

No, but it is not checked. Most other cases such as I2C and SPI buses
aren't checked because we have no generic way to key off of them. We
need schema data with compatible strings of those bus controllers to
do the checking.

Any bus type needs to define its unit address format. Generally, it
must match data fields in reg property and distinct fields are
separated by commas.

Rob

Re: Question about "unit address format error" of DTC

[Masahiro Yamada]

Hi Rob,

2017-11-09 23:15 GMT+09:00 Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>:
> On Thu, Nov 9, 2017 at 6:14 AM, Masahiro Yamada
> <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org> wrote:
>> Hi Rob, David,
>>
>>
>> I am just curious about a DTC warning.
>>
>>
>> For example,
>>
>> soc {
>>     compatible = "simple-bus";
>>     #address-cells = <1>;
>>     #size-cells = <1>;
>>     ranges;
>>
>>     foo@11111111 {
>>          compatible = "foo";
>>          reg = <0x54006800 0x40>;
>>     };
>> };
>>
>> This emits the following warning.
>>
>> Warning (simple_bus_reg): Node /soc/foo@11111111 simple-bus unit
>> address format error, expected "54006800"
>>
>>
>> But, if I replace "simple-bus" with "simple-mfd",
>> DTC is completely happy.
>
> We could probably add simple-mfd to the check. Though, if you have
> addresses, then you probably should use simple-bus. I'm not a bit fan
> of simple-mfd in general.
>
>> Why is this check limited to simple bus / PCI bus?
>
> Because bus types are free to define their own unit address format to
> some extent.
>
>> For other cases, is mismatching @<address>  allowed?
>
> No, but it is not checked. Most other cases such as I2C and SPI buses
> aren't checked because we have no generic way to key off of them. We
> need schema data with compatible strings of those bus controllers to
> do the checking.
>
> Any bus type needs to define its unit address format. Generally, it
> must match data fields in reg property and distinct fields are
> separated by commas.
>

Thanks.
I had a more specific example I wanted to ask,
but your last comment "distinct fields are separated by commas"
probably answered the question.


The following is an example of NVMEM data cells.
The generic binding is Documentation/devicetree/binding/nvmem/nvmem.txt


reg:    specifies the offset in byte within the storage device.

bits:   Is pair of bit location and number of bits, which specifies offset
        in bit and number of bits within the address range specified
by reg property.
        Offset takes values from 0-7.


So, it is possible to have multiple data cells
that share the same "reg".

@<reg-offset>,<bit-position>
was the idea in my mind, but I was not 100% sure.

         qfprom: qfprom@00700000 {
                ...

                /* Data cells */
                calib0: calib@10,2 {
                        reg = <0x10 0x4>;
                        bits = <2 2>;
                };

                calib1: calib@10,6 {
                        reg = <0x10 0x4>;
                        bits = <6 2>
                };

                calib2: calib@14,2 {
                        reg = <0x14 0x4>;
                        bits = <2 2>;
                };

                calib3: calib@14,6 {
                        reg = <0x14 0x4>;
                        bits = <6 2>
                };
                ...
        };


>From your comment, probably, this is the right thing to do.



-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Question about "unit address format error" of DTC

[Rob Herring]

On Thu, Nov 9, 2017 at 9:09 AM, Masahiro Yamada
<yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org> wrote:
> Hi Rob,
>
> 2017-11-09 23:15 GMT+09:00 Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>:
>> On Thu, Nov 9, 2017 at 6:14 AM, Masahiro Yamada
>> <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org> wrote:
>>> Hi Rob, David,
>>>
>>>
>>> I am just curious about a DTC warning.
>>>
>>>
>>> For example,
>>>
>>> soc {
>>>     compatible = "simple-bus";
>>>     #address-cells = <1>;
>>>     #size-cells = <1>;
>>>     ranges;
>>>
>>>     foo@11111111 {
>>>          compatible = "foo";
>>>          reg = <0x54006800 0x40>;
>>>     };
>>> };
>>>
>>> This emits the following warning.
>>>
>>> Warning (simple_bus_reg): Node /soc/foo@11111111 simple-bus unit
>>> address format error, expected "54006800"
>>>
>>>
>>> But, if I replace "simple-bus" with "simple-mfd",
>>> DTC is completely happy.
>>
>> We could probably add simple-mfd to the check. Though, if you have
>> addresses, then you probably should use simple-bus. I'm not a bit fan
>> of simple-mfd in general.
>>
>>> Why is this check limited to simple bus / PCI bus?
>>
>> Because bus types are free to define their own unit address format to
>> some extent.
>>
>>> For other cases, is mismatching @<address>  allowed?
>>
>> No, but it is not checked. Most other cases such as I2C and SPI buses
>> aren't checked because we have no generic way to key off of them. We
>> need schema data with compatible strings of those bus controllers to
>> do the checking.
>>
>> Any bus type needs to define its unit address format. Generally, it
>> must match data fields in reg property and distinct fields are
>> separated by commas.
>>
>
> Thanks.
> I had a more specific example I wanted to ask,
> but your last comment "distinct fields are separated by commas"
> probably answered the question.
>
>
> The following is an example of NVMEM data cells.
> The generic binding is Documentation/devicetree/binding/nvmem/nvmem.txt
>
>
> reg:    specifies the offset in byte within the storage device.
>
> bits:   Is pair of bit location and number of bits, which specifies offset
>         in bit and number of bits within the address range specified
> by reg property.
>         Offset takes values from 0-7.
>
> So, it is possible to have multiple data cells
> that share the same "reg".

In hindsight, I think I'd do away with bits and just define that reg
is the offset and size in bits. Maybe we did discuss that and I've
just forgotten the reasons not to do that.

>
> @<reg-offset>,<bit-position>
> was the idea in my mind, but I was not 100% sure.

Yeah, this is fine. Bonus points if you write a check in dtc for it. I
think we can match on #nvmem-cells in the parent node.

>
>          qfprom: qfprom@00700000 {
>                 ...
>
>                 /* Data cells */
>                 calib0: calib@10,2 {
>                         reg = <0x10 0x4>;
>                         bits = <2 2>;
>                 };
>
>                 calib1: calib@10,6 {
>                         reg = <0x10 0x4>;
>                         bits = <6 2>
>                 };
>
>                 calib2: calib@14,2 {
>                         reg = <0x14 0x4>;
>                         bits = <2 2>;
>                 };
>
>                 calib3: calib@14,6 {
>                         reg = <0x14 0x4>;
>                         bits = <6 2>
>                 };
>                 ...
>         };
>
>
> From your comment, probably, this is the right thing to do.
>
>
>
> --
> Best Regards
> Masahiro Yamada
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Question about "unit address format error" of DTC

[Masahiro Yamada]

Hi Rob,

2017-11-10 3:04 GMT+09:00 Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>:
> On Thu, Nov 9, 2017 at 9:09 AM, Masahiro Yamada
> <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org> wrote:
>> Hi Rob,
>>
>> 2017-11-09 23:15 GMT+09:00 Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>:
>>> On Thu, Nov 9, 2017 at 6:14 AM, Masahiro Yamada
>>> <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org> wrote:
>>>> Hi Rob, David,
>>>>
>>>>
>>>> I am just curious about a DTC warning.
>>>>
>>>>
>>>> For example,
>>>>
>>>> soc {
>>>>     compatible = "simple-bus";
>>>>     #address-cells = <1>;
>>>>     #size-cells = <1>;
>>>>     ranges;
>>>>
>>>>     foo@11111111 {
>>>>          compatible = "foo";
>>>>          reg = <0x54006800 0x40>;
>>>>     };
>>>> };
>>>>
>>>> This emits the following warning.
>>>>
>>>> Warning (simple_bus_reg): Node /soc/foo@11111111 simple-bus unit
>>>> address format error, expected "54006800"
>>>>
>>>>
>>>> But, if I replace "simple-bus" with "simple-mfd",
>>>> DTC is completely happy.
>>>
>>> We could probably add simple-mfd to the check. Though, if you have
>>> addresses, then you probably should use simple-bus. I'm not a bit fan
>>> of simple-mfd in general.
>>>
>>>> Why is this check limited to simple bus / PCI bus?
>>>
>>> Because bus types are free to define their own unit address format to
>>> some extent.
>>>
>>>> For other cases, is mismatching @<address>  allowed?
>>>
>>> No, but it is not checked. Most other cases such as I2C and SPI buses
>>> aren't checked because we have no generic way to key off of them. We
>>> need schema data with compatible strings of those bus controllers to
>>> do the checking.
>>>
>>> Any bus type needs to define its unit address format. Generally, it
>>> must match data fields in reg property and distinct fields are
>>> separated by commas.
>>>
>>
>> Thanks.
>> I had a more specific example I wanted to ask,
>> but your last comment "distinct fields are separated by commas"
>> probably answered the question.
>>
>>
>> The following is an example of NVMEM data cells.
>> The generic binding is Documentation/devicetree/binding/nvmem/nvmem.txt
>>
>>
>> reg:    specifies the offset in byte within the storage device.
>>
>> bits:   Is pair of bit location and number of bits, which specifies offset
>>         in bit and number of bits within the address range specified
>> by reg property.
>>         Offset takes values from 0-7.
>>
>> So, it is possible to have multiple data cells
>> that share the same "reg".
>
> In hindsight, I think I'd do away with bits and just define that reg
> is the offset and size in bits. Maybe we did discuss that and I've
> just forgotten the reasons not to do that.


Yeah, I like the idea, but "in hindsight".



>>
>> @<reg-offset>,<bit-position>
>> was the idea in my mind, but I was not 100% sure.
>
> Yeah, this is fine. Bonus points if you write a check in dtc for it. I
> think we can match on #nvmem-cells in the parent node.


I have no idea when I can look into it.

I think most (all?) of DTC checks are from your contribution.
It would be appreciated if you care to it.


One idea for generic solution might be DTC checks:

  - @* exactly matches to reg

     or

  - The first field of comma separated @*,*,... matches to reg


I do not know if other patterns exist.






-- 
Best Regards
Masahiro Yamada
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Question about "unit address format error" of DTC

[Rob Herring]

On Thu, Nov 9, 2017 at 7:48 PM, Masahiro Yamada
<yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org> wrote:
> Hi Rob,
>
> 2017-11-10 3:04 GMT+09:00 Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>:
>> On Thu, Nov 9, 2017 at 9:09 AM, Masahiro Yamada
>> <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org> wrote:
>>> Hi Rob,
>>>
>>> 2017-11-09 23:15 GMT+09:00 Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>:
>>>> On Thu, Nov 9, 2017 at 6:14 AM, Masahiro Yamada
>>>> <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org> wrote:
>>>>> Hi Rob, David,
>>>>>
>>>>>
>>>>> I am just curious about a DTC warning.
>>>>>
>>>>>
>>>>> For example,
>>>>>
>>>>> soc {
>>>>>     compatible = "simple-bus";
>>>>>     #address-cells = <1>;
>>>>>     #size-cells = <1>;
>>>>>     ranges;
>>>>>
>>>>>     foo@11111111 {
>>>>>          compatible = "foo";
>>>>>          reg = <0x54006800 0x40>;
>>>>>     };
>>>>> };
>>>>>
>>>>> This emits the following warning.
>>>>>
>>>>> Warning (simple_bus_reg): Node /soc/foo@11111111 simple-bus unit
>>>>> address format error, expected "54006800"
>>>>>
>>>>>
>>>>> But, if I replace "simple-bus" with "simple-mfd",
>>>>> DTC is completely happy.
>>>>
>>>> We could probably add simple-mfd to the check. Though, if you have
>>>> addresses, then you probably should use simple-bus. I'm not a bit fan
>>>> of simple-mfd in general.
>>>>
>>>>> Why is this check limited to simple bus / PCI bus?
>>>>
>>>> Because bus types are free to define their own unit address format to
>>>> some extent.
>>>>
>>>>> For other cases, is mismatching @<address>  allowed?
>>>>
>>>> No, but it is not checked. Most other cases such as I2C and SPI buses
>>>> aren't checked because we have no generic way to key off of them. We
>>>> need schema data with compatible strings of those bus controllers to
>>>> do the checking.
>>>>
>>>> Any bus type needs to define its unit address format. Generally, it
>>>> must match data fields in reg property and distinct fields are
>>>> separated by commas.
>>>>
>>>
>>> Thanks.
>>> I had a more specific example I wanted to ask,
>>> but your last comment "distinct fields are separated by commas"
>>> probably answered the question.
>>>
>>>
>>> The following is an example of NVMEM data cells.
>>> The generic binding is Documentation/devicetree/binding/nvmem/nvmem.txt
>>>
>>>
>>> reg:    specifies the offset in byte within the storage device.
>>>
>>> bits:   Is pair of bit location and number of bits, which specifies offset
>>>         in bit and number of bits within the address range specified
>>> by reg property.
>>>         Offset takes values from 0-7.
>>>
>>> So, it is possible to have multiple data cells
>>> that share the same "reg".
>>
>> In hindsight, I think I'd do away with bits and just define that reg
>> is the offset and size in bits. Maybe we did discuss that and I've
>> just forgotten the reasons not to do that.
>
>
> Yeah, I like the idea, but "in hindsight".
>
>
>
>>>
>>> @<reg-offset>,<bit-position>
>>> was the idea in my mind, but I was not 100% sure.
>>
>> Yeah, this is fine. Bonus points if you write a check in dtc for it. I
>> think we can match on #nvmem-cells in the parent node.
>
>
> I have no idea when I can look into it.
>
> I think most (all?) of DTC checks are from your contribution.

Recently, yes. Except for fixing all my bugs. :)

> It would be appreciated if you care to it.

I'd appreciate contributions from others. :)

> One idea for generic solution might be DTC checks:
>
>   - @* exactly matches to reg
>
>      or
>
>   - The first field of comma separated @*,*,... matches to reg

David rejected generic checks of the unit-address beyond what we have
now. Part of the problem is what is valid vs. what is considered best
practice today. Or to put another way, things we'd like to check and
discourage, but aren't something we should fix in existing
trees/bindings.

Rob

Re: Question about "unit address format error" of DTC

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 5436 bytes --]

On Fri, Nov 10, 2017 at 09:48:46AM -0600, Rob Herring wrote:
> On Thu, Nov 9, 2017 at 7:48 PM, Masahiro Yamada
> <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org> wrote:
> > Hi Rob,
> >
> > 2017-11-10 3:04 GMT+09:00 Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>:
> >> On Thu, Nov 9, 2017 at 9:09 AM, Masahiro Yamada
> >> <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org> wrote:
> >>> Hi Rob,
> >>>
> >>> 2017-11-09 23:15 GMT+09:00 Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>:
> >>>> On Thu, Nov 9, 2017 at 6:14 AM, Masahiro Yamada
> >>>> <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org> wrote:
> >>>>> Hi Rob, David,
> >>>>>
> >>>>>
> >>>>> I am just curious about a DTC warning.
> >>>>>
> >>>>>
> >>>>> For example,
> >>>>>
> >>>>> soc {
> >>>>>     compatible = "simple-bus";
> >>>>>     #address-cells = <1>;
> >>>>>     #size-cells = <1>;
> >>>>>     ranges;
> >>>>>
> >>>>>     foo@11111111 {
> >>>>>          compatible = "foo";
> >>>>>          reg = <0x54006800 0x40>;
> >>>>>     };
> >>>>> };
> >>>>>
> >>>>> This emits the following warning.
> >>>>>
> >>>>> Warning (simple_bus_reg): Node /soc/foo@11111111 simple-bus unit
> >>>>> address format error, expected "54006800"
> >>>>>
> >>>>>
> >>>>> But, if I replace "simple-bus" with "simple-mfd",
> >>>>> DTC is completely happy.
> >>>>
> >>>> We could probably add simple-mfd to the check. Though, if you have
> >>>> addresses, then you probably should use simple-bus. I'm not a bit fan
> >>>> of simple-mfd in general.
> >>>>
> >>>>> Why is this check limited to simple bus / PCI bus?
> >>>>
> >>>> Because bus types are free to define their own unit address format to
> >>>> some extent.
> >>>>
> >>>>> For other cases, is mismatching @<address>  allowed?
> >>>>
> >>>> No, but it is not checked. Most other cases such as I2C and SPI buses
> >>>> aren't checked because we have no generic way to key off of them. We
> >>>> need schema data with compatible strings of those bus controllers to
> >>>> do the checking.
> >>>>
> >>>> Any bus type needs to define its unit address format. Generally, it
> >>>> must match data fields in reg property and distinct fields are
> >>>> separated by commas.
> >>>>
> >>>
> >>> Thanks.
> >>> I had a more specific example I wanted to ask,
> >>> but your last comment "distinct fields are separated by commas"
> >>> probably answered the question.
> >>>
> >>>
> >>> The following is an example of NVMEM data cells.
> >>> The generic binding is Documentation/devicetree/binding/nvmem/nvmem.txt
> >>>
> >>>
> >>> reg:    specifies the offset in byte within the storage device.
> >>>
> >>> bits:   Is pair of bit location and number of bits, which specifies offset
> >>>         in bit and number of bits within the address range specified
> >>> by reg property.
> >>>         Offset takes values from 0-7.
> >>>
> >>> So, it is possible to have multiple data cells
> >>> that share the same "reg".
> >>
> >> In hindsight, I think I'd do away with bits and just define that reg
> >> is the offset and size in bits. Maybe we did discuss that and I've
> >> just forgotten the reasons not to do that.
> >
> >
> > Yeah, I like the idea, but "in hindsight".
> >
> >
> >
> >>>
> >>> @<reg-offset>,<bit-position>
> >>> was the idea in my mind, but I was not 100% sure.
> >>
> >> Yeah, this is fine. Bonus points if you write a check in dtc for it. I
> >> think we can match on #nvmem-cells in the parent node.
> >
> >
> > I have no idea when I can look into it.
> >
> > I think most (all?) of DTC checks are from your contribution.
> 
> Recently, yes. Except for fixing all my bugs. :)
> 
> > It would be appreciated if you care to it.
> 
> I'd appreciate contributions from others. :)
> 
> > One idea for generic solution might be DTC checks:
> >
> >   - @* exactly matches to reg
> >
> >      or
> >
> >   - The first field of comma separated @*,*,... matches to reg
> 
> David rejected generic checks of the unit-address beyond what we have
> now. Part of the problem is what is valid vs. what is considered best
> practice today. Or to put another way, things we'd like to check and
> discourage, but aren't something we should fix in existing
> trees/bindings.

Right.

The problem is what does "matches to reg" mean.  Equals reg rendered
as a hex string?  That works for simple cases, but not for more
complex busses.  That won't work for PCI - even the first element
before a , has to be deciphered from the devfn bits encoded within the
various words of the 'reg' address.  A similar scheme would make sense
for many complex busses.

There are other variants to, like I think you can have things like
@i80 on old ISA busses to represent things in IO space rather than
MMIO space.  Not much used nowadays, though.

But the point is that the formatting of the unit address is entirely
up to the bus type.  In traditional OF the unit address could be
determined automatically, but that's because it didn't include just
the device tree, but actual methods for all the devices and busses
that knew how to correctly format the address from 'reg'.

-- 
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: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]