[Skiboot] [PATCH] Add fwts annotation for duplicate DT node entries.

[Oliver O'Halloran]

On Fri, Nov 18, 2016 at 10:24 PM, Vasant Hegde
<hegdevasant at linux.vnet.ibm.com> wrote:
> On 11/17/2016 11:08 AM, ppaidipe at linux.vnet.ibm.com wrote:
>>
>> From: Pridhiviraj Paidipeddi <ppaidipe at linux.vnet.ibm.com>
>>
>> Reference bug: https://github.com/open-power/op-build/issues/751
>>
>> Signed-off-by: Pridhiviraj Paidipeddi <ppaidipe at linux.vnet.ibm.com>
>> ---
>>   core/device.c | 12 +++++++++++-
>>   1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/core/device.c b/core/device.c
>> index 63b5df8..cf19f9b 100644
>> --- a/core/device.c
>> +++ b/core/device.c
>> @@ -116,7 +116,17 @@ bool dt_attach_root(struct dt_node *parent, struct
>> dt_node *root)
>>
>>                 /* Look for duplicates */
>>                 if (cmp == 0) {
>> -                       prerror("DT: %s failed, duplicate %s\n",
>> +                       /**
>> +                         * @fwts-label DTHasDuplicateNodeID
>> +                         * @fwts-advice OPAL Expands the Flatten Device
>> Tree(FDT) generated
>> +                         *     by hostboot. During Expansion of FDT, OPAL
>> observed the same node id
>
>
> better s/same node id/duplicate node/?
>
> Also this description is too specific to hostboot.. But OPAL can parse fdt
> from any source and we can hit similar issue. May be you should generalize
> this little bit.

This is true, but we only really use FDT-only booting in development
environments and the FWTS mainly exists to explain skiboot failures to
OpenPower Partners. That said, there's probably other situations which
can cause a similar failure, but I can't think of any off to top of my
head.

>
> -Vasant
>
>> +                         *     assigned multiple times. This means
>> platform xml can contain either
>> +                         *     duplicate node ID for same hardware device
>> or for different hardware
>> +                         *     devices. This is a bug in firmware
>> component of platform xml, due to
>> +                         *     which firmware/OPAL won't add the hardware
>> device found with a duplicate
>> +                         *     node ID into DT. And corresponding device
>> is not functional.
>> +                         */

dt_attach_root() is called when creating any new DT node and not just
when expanding the FDT. This sort of failure (and any FWTS labels)
should be specifc to the FDT expansion path so why not re-work the
error handling for dt_attach_root() so that it's inside
dt_expand_node() instead? That's the logical place for it IMO.

>> +                       prlog(PR_ERR, "DT: %s failed, duplicate %s\n",
>>                                 __func__, root->name);
>>                         return false;
>>                 }
>>
>
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot