Re: [PATCH 2/2] Warn on node name unit-addresses with '0x' or leading 0s

[Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

On Mon, Feb 22, 2016 at 11:47 PM, David Gibson
<david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> On Mon, Feb 22, 2016 at 10:51:46AM -0600, Rob Herring wrote:
>> On Thu, Feb 18, 2016 at 11:07 PM, David Gibson
>> <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
>> > On Thu, Feb 11, 2016 at 02:46:59PM -0600, Rob Herring wrote:
>> >> Node name unit-addresses should never begin with 0x or leading 0s
>> >> regardless of whether they have a bus specific address (i.e. one with
>> >> commas) or not. Add warnings to check for these cases.
>> >
>> > Hmm.. I'm pretty sure that's true in practice, but it's not true in
>> > theory.  A bus could define it's unit address format just about
>> > however it wants, including with leading 0s.
>>
>> Only if it is not reviewed... This whole check is about what best
>> practices are, not what is possible.
>
> Hmm.  dtc checks are really about checking for best practice at the
> level of individual dts files, though, not bindings.

Checking simple-bus specifically would be checking a binding.

>> > I think a better approach would be to add a test specific to
>> > simple-bus devices (by looking at compatible on the parent) that fully
>> > checks the unit address.
>> >
>> > From there we can start adding tests for other bus types.
>>
>> simple-bus is easy enough,
>
> So, start with that, then tackle the next problem.
>
>> but then next up would be I2C and SPI. We
>> can't generically tell if a node is on I2C or SPI bus.
>
> Why not?  Or perhaps.. how generically do you need?  I think having a
> big list of i2c / spi controllers would be acceptable here, if not
> ideal.

So someone adds a new controller, puts crap in for unit addresses, and
then no warnings until that compatible string is added to dtc. And I'm
still left spending my time in reviews telling them to fix this
trivial crap.

That's roughly 60 I2C controllers (families, so multiple compatible
strings each) plus similar number of SPI controllers, OF-graph
binding, and random other things where reg gets used.

>
>> If we do have
>> some bus with wacky addresses, it should definitely have a bus
>> compatible and then we can simply exclude it from the check.
>>
>> Another option would be skipping the test if there are any commas (or
>> periods, etc.) in the unit address. That's pretty rare to begin with
>> and a single number is pretty much not a bus specific unit-address.
>
> Um.. no.. there are definitely bus types that don't typically use
> commas.  ISA, for one.

All the cases of ISA in the kernel tree at least would pass this test.
But we could either blacklist ISA or skip if any non-hex characters
are present.

BTW, my next patch is stricter node and property name checks on the
use of '#', '?', '.', '+', '*', and '_'. So if you don't think these
kinds to checks belong in dtc, then tell me and suggest how we should
check for this.

Rob