[Skiboot] [PATCH] occ-sensor: clean dt properties if sensor is not available

[Balamuruhan S]

On Mon, Oct 14, 2019 at 10:32:16PM +1100, Oliver O'Halloran wrote:
> On Mon, Oct 14, 2019 at 8:52 PM Balamuruhan S <bala24 at linux.ibm.com> wrote:
> >
> > In `occ_sensor_init()` device tree node is created for sensor-goups
> > and performs `occ_sensor_sanity()` check to initialize the device
> > tree. But if there are no sensors like in Qemu, sanity check fails
> > but still device tree populates the sensor-groups node wrongly as
> > the node created is not cleaned up.
> 
> Does having the empty node causing any problems? Nuisance warnings or whatever?

No, it doesn't cause any problem, I did observe an empty hanging node
of `sensor-groups`.

> 
> >
> > Signed-off-by: Balamuruhan S <bala24 at linux.ibm.com>
> > ---
> >  hw/occ-sensor.c | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/occ-sensor.c b/hw/occ-sensor.c
> > index d06ca725..ec116c9a 100644
> > --- a/hw/occ-sensor.c
> > +++ b/hw/occ-sensor.c
> > @@ -491,6 +491,7 @@ bool occ_sensors_init(void)
> >         struct dt_node *sg, *exports;
> >         int occ_num = 0, i;
> >         bool has_gpu = false;
> > +       bool has_sensor = false;
> >
> >         /* OCC inband sensors is only supported in P9 */
> >         if (proc_gen != proc_gen_p9)
> > @@ -526,7 +527,9 @@ bool occ_sensors_init(void)
> >                 md = get_names_block(hb);
> >
> >                 /* Sanity check of the Sensor Data Header Block */
> > -               if (!occ_sensor_sanity(hb, chip->id))
> > +               if (occ_sensor_sanity(hb, chip->id))
> > +                       has_sensor = true;
> > +               else
> >                         continue;
> >
> >                 phandles = malloc(hb->nr_sensors * sizeof(u32));
> > @@ -587,6 +590,10 @@ bool occ_sensors_init(void)
> >                 free(phandles);
> >                 free(ptype);
> >         }
> > +       if (!has_sensor) {
> > +               /* clear the device tree property for sensors */
> > +               dt_free(sg);
> > +       }
> 
> Would it make more sense to use list_empty(&sg->children) as the test?

yes, we can avoid introducing `has_sensor` instead by using list_empty().

Thanks Oliver.

> 
> >
> >         if (!occ_num)
> >                 return false;
> > --
> > 2.14.5
> >