Re: [PATCH v2 1/3] checks: add phandle with arg property checks

[Rob Herring <robh@kernel.org>]

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

On Thu, Aug 24, 2017 at 7:49 PM, David Gibson <david@gibson.dropbear.id.au>
wrote:
> On Thu, Aug 24, 2017 at 12:23:16PM -0500, Rob Herring wrote:
>> On Wed, Aug 23, 2017 at 9:03 PM, David Gibson
>> <david@gibson.dropbear.id.au> wrote:
>> > On Tue, Aug 22, 2017 at 06:02:06PM -0500, Rob Herring wrote:
>> >> Many common bindings follow the same pattern of client properties
>> >> containing a phandle and N arg cells where N is defined in the
provider
>> >> with a '#<specifier>-cells' property such as:
>> >>

[...]

>> However, the check here is not perfect because we could have
>> "<&phandle1 1 2>" when really it should be "<&phandle1 &phandle2
>> &phandle3>". We don't fail until we're checking the 2nd phandle.
>> That's why I added the "or bad phandle" and the cell # in the message
>> above. In the opposite case, we'd be silent. One thing that could be
>> done is double check things against the markers if they are present.
>
> Uh.. I don't really understand what you're getting at here.  We should
> be able to determine which of these cases it should be by the
> #whatever-cells at &phandle1.

If #whatever-cells is wrong, we can't tell immediately. Say it is 1 in the
above case, but really should be 2. Then on the 2nd loop iteration, we
think the value "2" is a phandle which will actually succeed in the lookup.
The failure we get is #whatever-cells couldn't be found (or maybe it is
found by luck, who knows) in the node pointed to by phandle 2. There's a
case of this in the kernel that I hit which took me a minute to realize
what was going on. The point is that if the property value has errors, we
may go into the weeds and the error reported may not be that precise.

Rob

[-- Attachment #2: Type: text/html, Size: 2224 bytes --]