| Message ID | 20170402090040.18931-1-hegdevasant@linux.vnet.ibm.com |
|---|---|
| State | Accepted |
| Headers | show |
On 04/02/2017 02:30 PM, Vasant Hegde wrote: > P9 onwards OPAL is building device tree for BMC based system using > HDAT. We are populating bmc/compatible node with bmc version. Hence > do not delete this property. > Stewart, Can you take a look of this patch? -Vasant
Hi Vasant, > P9 onwards OPAL is building device tree for BMC based system using > HDAT. We are populating bmc/compatible node with bmc version. Hence > do not delete this property. ast_bmc_fixup_bmc_sensors() does a few things, but I think I see what you're getting at here; this change looks good to me. However, we should probably validate that the generated DT actually looks OK before we do a release with this patch (and thus releasing those compatible strings into the wild). To help with that, could you provide a sample of the generated tree? Any changes there would be to separate code though, so: Acked-by: Jeremy Kerr <jk@ozlabs.org> Cheers, Jeremy
On 07/25/2017 11:01 AM, Jeremy Kerr wrote: > Hi Vasant, > >> P9 onwards OPAL is building device tree for BMC based system using >> HDAT. We are populating bmc/compatible node with bmc version. Hence >> do not delete this property. > > ast_bmc_fixup_bmc_sensors() does a few things, but I think I see what > you're getting at here; this change looks good to me. > > However, we should probably validate that the generated DT actually > looks OK before we do a release with this patch (and thus releasing > those compatible strings into the wild). To help with that, could you > provide a sample of the generated tree? Jeremy, Please find the BMC section of device tree. Let me know if you need full dts output. bmc { compatible = "ibm,ast2500,openbmc"; #address-cells = <0x1>; #size-cells = <0x0>; phandle = <0x2>; sensors { #address-cells = <0x1>; #size-cells = <0x0>; phandle = <0x3>; sensor@a7 { compatible = "ibm,ipmi-sensor"; phandle = <0x19>; reg = <0xa7>; ipmi-sensor-type = <0x1>; }; sensor@ae { compatible = "ibm,ipmi-sensor"; phandle = <0x20>; reg = <0xae>; ipmi-sensor-type = <0xc>; }; sensor@0 { compatible = "ibm,ipmi-sensor"; phandle = <0x14>; reg = <0x0>; ipmi-sensor-type = <0xcc>; }; sensor@a5 { compatible = "ibm,ipmi-sensor"; phandle = <0x17>; reg = <0xa5>; ipmi-sensor-type = <0x1>; }; sensor@ac { compatible = "ibm,ipmi-sensor"; phandle = <0x1e>; reg = <0xac>; ipmi-sensor-type = <0xc>; }; sensor@5a { compatible = "ibm,ipmi-sensor"; phandle = <0x16>; reg = <0x5a>; ipmi-sensor-type = <0x7>; }; sensor@9 { compatible = "ibm,ipmi-sensor"; phandle = <0x12>; reg = <0x9>; ipmi-sensor-type = <0xc8>; }; sensor@b3 { compatible = "ibm,ipmi-sensor"; phandle = <0x21>; reg = <0xb3>; ipmi-sensor-type = <0x1>; }; sensor@aa { compatible = "ibm,ipmi-sensor"; phandle = <0x1c>; reg = <0xaa>; ipmi-sensor-type = <0xc>; }; sensor@7 { compatible = "ibm,ipmi-sensor"; phandle = <0xf>; reg = <0x7>; ipmi-sensor-type = <0xc3>; }; sensor@e { compatible = "ibm,ipmi-sensor"; phandle = <0x6>; reg = <0xe>; ipmi-sensor-type = <0xc7>; }; sensor@5 { compatible = "ibm,ipmi-sensor"; phandle = <0xb>; reg = <0x5>; ipmi-sensor-type = <0x1f>; }; sensor@c { compatible = "ibm,ipmi-sensor"; phandle = <0x4>; reg = <0xc>; ipmi-sensor-type = <0xc7>; }; sensor@3 { compatible = "ibm,ipmi-sensor"; phandle = <0x9>; reg = <0x3>; ipmi-sensor-type = <0xf>; }; sensor@a { compatible = "ibm,ipmi-sensor"; phandle = <0xd>; reg = <0xa>; ipmi-sensor-type = <0xc1>; }; sensor@a8 { compatible = "ibm,ipmi-sensor"; phandle = <0x1a>; reg = <0xa8>; ipmi-sensor-type = <0xc>; }; sensor@1 { compatible = "ibm,ipmi-sensor"; phandle = <0xa>; reg = <0x1>; ipmi-sensor-type = <0x12>; }; sensor@a6 { compatible = "ibm,ipmi-sensor"; phandle = <0x18>; reg = <0xa6>; ipmi-sensor-type = <0xc>; }; sensor@ad { compatible = "ibm,ipmi-sensor"; phandle = <0x1f>; reg = <0xad>; ipmi-sensor-type = <0x1>; }; sensor@b4 { compatible = "ibm,ipmi-sensor"; phandle = <0x22>; reg = <0xb4>; ipmi-sensor-type = <0xc>; }; sensor@ab { compatible = "ibm,ipmi-sensor"; phandle = <0x1d>; reg = <0xab>; ipmi-sensor-type = <0x1>; }; sensor@8 { compatible = "ibm,ipmi-sensor"; phandle = <0x11>; reg = <0x8>; ipmi-sensor-type = <0xc6>; }; sensor@f { compatible = "ibm,ipmi-sensor"; phandle = <0x7>; reg = <0xf>; ipmi-sensor-type = <0xc7>; }; sensor@6 { compatible = "ibm,ipmi-sensor"; phandle = <0xe>; reg = <0x6>; ipmi-sensor-type = <0xc2>; }; sensor@d { compatible = "ibm,ipmi-sensor"; phandle = <0x5>; reg = <0xd>; ipmi-sensor-type = <0xc7>; }; sensor@59 { compatible = "ibm,ipmi-sensor"; phandle = <0x15>; reg = <0x59>; ipmi-sensor-type = <0x1>; }; sensor@10 { compatible = "ibm,ipmi-sensor"; phandle = <0x8>; reg = <0x10>; ipmi-sensor-type = <0xc7>; }; sensor@4 { compatible = "ibm,ipmi-sensor"; phandle = <0x10>; reg = <0x4>; ipmi-sensor-type = <0xc4>; }; sensor@b { compatible = "ibm,ipmi-sensor"; phandle = <0x13>; reg = <0xb>; ipmi-sensor-type = <0xca>; }; sensor@a9 { compatible = "ibm,ipmi-sensor"; phandle = <0x1b>; reg = <0xa9>; ipmi-sensor-type = <0x1>; }; sensor@2 { compatible = "ibm,ipmi-sensor"; phandle = <0xc>; reg = <0x -Vasant
Hi Vasant, > Please find the BMC section of device tree. Let me know if you need full > dts output. Thanks! What you've provided is sufficient. > bmc { > compatible = "ibm,ast2500,openbmc"; That looks like a pretty strange compatible value; it should be in the format <vendor>,<device>... Do you know who manages those inputs on the HDAT side? Cheers, Jeremy
On 07/26/2017 06:15 AM, Jeremy Kerr wrote: > Hi Vasant, > Jeremy, .../... > >> bmc { >> compatible = "ibm,ast2500,openbmc"; > > That looks like a pretty strange compatible value; it should be in the > format <vendor>,<device>... They use vendor,hwname,swname format. > > Do you know who manages those inputs on the HDAT side? Venkatesh (in CC) manages these data. -Vasant
On 07/26/2017 06:15 AM, Jeremy Kerr wrote: > Hi Vasant, > >> Please find the BMC section of device tree. Let me know if you need full >> dts output. > > Thanks! What you've provided is sufficient. Jeremy , Can you Ack this patch as compatible property is not related to this patch? > >> bmc { >> compatible = "ibm,ast2500,openbmc"; > If we really want to fix this then we have to ask HDAT folks to split above property to something like device and software. Alternatively we can fix it in OPAL as well. -Vasant
On 04/02/2017 02:30 PM, Vasant Hegde wrote: > P9 onwards OPAL is building device tree for BMC based system using > HDAT. We are populating bmc/compatible node with bmc version. Hence > do not delete this property. Stewart, Looks like this one is not yet merged. Can you please pull this patch? -Vasant
Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes: > P9 onwards OPAL is building device tree for BMC based system using > HDAT. We are populating bmc/compatible node with bmc version. Hence > do not delete this property. > > CC: Jeremy Kerr <jk@ozlabs.org> > CC: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com> > Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> This does get us properties like this though: compatible "ibm,ast2500,supermicro" compatible "ibm,ast2500,openbmc" which... are certainly wrong. This is, however, what HDAT is giving us, so arguably this is a HDAT bug. In fact, this even violates the HDAT spec, which says: P9 IBM systems with FSP: ibm,fsp2,fips P9 IBM systems with BMC: ibm,ast2500,openbmc P9 IBM systems with SuperMicro: supermicro,ast2500,aten which is not what we get, we seem to always get what I pasted above (for p9dsu and witherspoon respectively). Soo... what should we do about that? While the second comma in the compatible property is odd, it doesn't seem to violate the DT spec. I'm okay with merging this as-is, as long as we fix up the oddities in a subsequent patch though. So, it's in master as of bb0079ea8490cbc138b9d14da30051cc0dabd381
On 05/25/2018 04:32 AM, Stewart Smith wrote: > Vasant Hegde <hegdevasant@linux.vnet.ibm.com> writes: >> P9 onwards OPAL is building device tree for BMC based system using >> HDAT. We are populating bmc/compatible node with bmc version. Hence >> do not delete this property. >> >> CC: Jeremy Kerr <jk@ozlabs.org> >> CC: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com> >> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> > > This does get us properties like this though: > compatible "ibm,ast2500,supermicro" > compatible "ibm,ast2500,openbmc" > > which... are certainly wrong. This is, however, what HDAT is giving us, > so arguably this is a HDAT bug. Yes. Fix should come from HDAT/xml. > > In fact, this even violates the HDAT spec, which says: > > P9 IBM systems with FSP: ibm,fsp2,fips > P9 IBM systems with BMC: ibm,ast2500,openbmc > P9 IBM systems with SuperMicro: supermicro,ast2500,aten Yes. This is wrong. @Nagendra, Can you fix this one in HDAT/xml? > > which is not what we get, we seem to always get what I pasted above (for > p9dsu and witherspoon respectively). > > Soo... what should we do about that? While the second comma in the > compatible property is odd, it doesn't seem to violate the DT spec. Yeah. But that's what we get. -Vasant
diff --git a/platforms/astbmc/common.c b/platforms/astbmc/common.c index 6f29e61..3fad2d4 100644 --- a/platforms/astbmc/common.c +++ b/platforms/astbmc/common.c @@ -336,7 +336,8 @@ static void astbmc_fixup_dt(void) make sure we have one. */ astbmc_fixup_dt_system_id(); - astbmc_fixup_bmc_sensors(); + if (proc_gen == proc_gen_p8) + astbmc_fixup_bmc_sensors(); } static void astbmc_fixup_psi_bar(void)
P9 onwards OPAL is building device tree for BMC based system using HDAT. We are populating bmc/compatible node with bmc version. Hence do not delete this property. CC: Jeremy Kerr <jk@ozlabs.org> CC: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com> Signed-off-by: Vasant Hegde <hegdevasant@linux.vnet.ibm.com> --- platforms/astbmc/common.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)