[v3] platform/astbmc: Do not delete compatible property

Message ID 20170402090040.18931-1-hegdevasant@linux.vnet.ibm.com
State New
Headers show

Commit Message

Vasant Hegde April 2, 2017, 9 a.m.
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(-)

Comments

Vasant Hegde May 25, 2017, 6:25 a.m. | #1
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
Jeremy Kerr July 25, 2017, 5:31 a.m. | #2
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
Vasant Hegde July 25, 2017, 9:39 a.m. | #3
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
Jeremy Kerr July 26, 2017, 12:45 a.m. | #4
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
Vasant Hegde July 26, 2017, 10:11 a.m. | #5
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
Vasant Hegde Aug. 16, 2017, 4:43 a.m. | #6
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
Vasant Hegde Nov. 29, 2017, 12:58 p.m. | #7
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

Patch

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)