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

Message ID 1489418374-13824-1-git-send-email-hegdevasant@linux.vnet.ibm.com
State Superseded
Headers show

Commit Message

Vasant Hegde March 13, 2017, 3:19 p.m.
P9 onwards OPAL is building device tree for BMC based system using
HDAT. We are populating bmc/compatible node with ast 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>
---
Changes in v2:
  - Updated description

-Vasant

 platforms/astbmc/common.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Ananth N Mavinakayanahalli March 14, 2017, 8:51 a.m. | #1
On Mon, Mar 13, 2017 at 08:49:34PM +0530, Vasant Hegde wrote:
> P9 onwards OPAL is building device tree for BMC based system using
> HDAT. We are populating bmc/compatible node with ast 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>

Reviewed-by: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
Oliver O'Halloran March 15, 2017, 5:59 a.m. | #2
On Tue, Mar 14, 2017 at 2:19 AM, Vasant Hegde
<hegdevasant@linux.vnet.ibm.com> wrote:
> P9 onwards OPAL is building device tree for BMC based system using
> HDAT. We are populating bmc/compatible node with ast 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>
> ---
> Changes in v2:
>   - Updated description
>
> -Vasant
>
>  platforms/astbmc/common.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/platforms/astbmc/common.c b/platforms/astbmc/common.c
> index 6f29e61..dba22ae 100644
> --- a/platforms/astbmc/common.c
> +++ b/platforms/astbmc/common.c
> @@ -275,6 +275,10 @@ static void del_compatible(struct dt_node *node)
>  {
>         struct dt_property *prop;
>
> +       /* Do not delete compatible property on P9 */
> +       if (proc_gen > proc_gen_p8)
> +               return;
> +
>         prop = __dt_find_property(node, "compatible");
>         if (prop)
>                 dt_del_property(node, prop);
> --

I'm not sure this is the best fix for this. This function is only
called from astbmc_fixup_bmc_sensors() which exists to fix up issues
with the hostboot provided FDT on P8. Given we generate the entire DT
from the HDAT on P9 I don't really see the point of calling it at all.
Could we add a proc_gen check around the call to
astbmc_fixup_bmc_sensors() instead?

Oliver

> 2.5.5
>
> _______________________________________________
> Skiboot mailing list
> Skiboot@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot
Vasant Hegde April 2, 2017, 8:59 a.m. | #3
On 03/15/2017 11:29 AM, Oliver O'Halloran wrote:
> On Tue, Mar 14, 2017 at 2:19 AM, Vasant Hegde
> <hegdevasant@linux.vnet.ibm.com> wrote:
>> P9 onwards OPAL is building device tree for BMC based system using
>> HDAT. We are populating bmc/compatible node with ast 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>
>> ---
>> Changes in v2:
>>   - Updated description
>>
>> -Vasant
>>
>>  platforms/astbmc/common.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/platforms/astbmc/common.c b/platforms/astbmc/common.c
>> index 6f29e61..dba22ae 100644
>> --- a/platforms/astbmc/common.c
>> +++ b/platforms/astbmc/common.c
>> @@ -275,6 +275,10 @@ static void del_compatible(struct dt_node *node)
>>  {
>>         struct dt_property *prop;
>>
>> +       /* Do not delete compatible property on P9 */
>> +       if (proc_gen > proc_gen_p8)
>> +               return;
>> +
>>         prop = __dt_find_property(node, "compatible");
>>         if (prop)
>>                 dt_del_property(node, prop);
>> --
>
> I'm not sure this is the best fix for this. This function is only
> called from astbmc_fixup_bmc_sensors() which exists to fix up issues
> with the hostboot provided FDT on P8. Given we generate the entire DT
> from the HDAT on P9 I don't really see the point of calling it at all.
> Could we add a proc_gen check around the call to
> astbmc_fixup_bmc_sensors() instead?

Yep. Makes sense. Fixed in V3.

-Vasant

Patch

diff --git a/platforms/astbmc/common.c b/platforms/astbmc/common.c
index 6f29e61..dba22ae 100644
--- a/platforms/astbmc/common.c
+++ b/platforms/astbmc/common.c
@@ -275,6 +275,10 @@  static void del_compatible(struct dt_node *node)
 {
 	struct dt_property *prop;
 
+	/* Do not delete compatible property on P9 */
+	if (proc_gen > proc_gen_p8)
+		return;
+
 	prop = __dt_find_property(node, "compatible");
 	if (prop)
 		dt_del_property(node, prop);