[Skiboot] [PATCH] occ-sensors: Export OCC sensors base address

[Stewart Smith]

Shilpasri G Bhat <shilpa.bhat at linux.vnet.ibm.com> writes:
> Add device tree property to export the starting address and size
> of OCC inband sensor area in main memory.

(pretty much same comments as on linux patch)

Why?

Are these to be used as sensors? If so, we should use existing
interfaces to expose them - I don't think we need a third way, and this
way has several deficiences: it doesn't list what the sensors are, what
units they're in, how often they're updated, or even what data types
they are. It's also not generic. I very much prefer a generic interface
to something that just tightly binds a particular CPU implementation
detail to the OS.

>
> Signed-off-by: Shilpasri G Bhat <shilpa.bhat at linux.vnet.ibm.com>
> ---
>  doc/device-tree/ibm,opal/power-mgt.rst | 13 +++++++++++++
>  hw/occ-sensor.c                        | 14 +++++++++++++-
>  2 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/doc/device-tree/ibm,opal/power-mgt.rst b/doc/device-tree/ibm,opal/power-mgt.rst
> index b326a24..0f489c0 100644
> --- a/doc/device-tree/ibm,opal/power-mgt.rst
> +++ b/doc/device-tree/ibm,opal/power-mgt.rst
> @@ -56,6 +56,8 @@ Example:
>                  ibm,pstate-ids = <0x0 0xffffffff 0xfffffffe 0xfffffffd 0xfffffffc 0xfffffffb 0xfffffffa 0xfffffff9 0xfffffff8 0xfffffff7 0xfffffff6 0xfffffff5 0xfffffff4 0xfffffff3 0xfffffff2 0xfffffff1 0xfffffff0 0xffffffef 0xffffffee 0xffffffed 0xffffffec 0xffffffeb 0xffffffea 0xffffffe9 0xffffffe8 0xffffffe7 0xffffffe6 0xffffffe5 0xffffffe4 0xffffffe3 0xffffffe2 0xffffffe1 0xffffffe0 0xffffffdf 0xffffffde 0xffffffdd 0xffffffdc 0xffffffdb 0xffffffda 0xffffffd9 0xffffffd8 0xffffffd7 0xffffffd6 0xffffffd5>;
>                  ibm,pstate-max = <0x0>;
>                  ibm,pstate-min = <0xffffffd5>;
> +                occ-sensors    = <0x00002003 0xffd80000 0x00000000 0x0004b000>;
> +                compatible     = "ibm,opal-occ-inband-sensors";
>         };
>       };
>     };
> @@ -132,3 +134,14 @@ ibm,pstate-core-max
>  This property is added when ultra_turbo(WOF) is enabled. This property gives
>  the list of max pstate for each 'n' number of active cores in the chip.
>  
> +occ-sensors
> +------------
> +
> +This property contains the starting address and size of the OCC
> +sensors memory region.
> +
> +compatible
> +----------
> +
> +The compatible string should be "ibm,opal-occ-inband-sensors" for the
> +kernel opal-occ mmap driver.

I don't think this documentation is adequate, it doesn't describe the
purpose of the interface nor how to use it.

> diff --git a/hw/occ-sensor.c b/hw/occ-sensor.c
> index 89f529f..142e3b4 100644
> --- a/hw/occ-sensor.c
> +++ b/hw/occ-sensor.c
> @@ -546,7 +546,7 @@ static const char *get_sensor_loc_string(enum occ_sensor_location loc)
>  void occ_sensors_init(void)
>  {
>  	struct proc_chip *chip;
> -	struct dt_node *sg;
> +	struct dt_node *sg, *power_mgt;
>  	int occ_num = 0, i;
>  
>  	/* OCC inband sensors is only supported in P9 */
> @@ -633,4 +633,16 @@ void occ_sensors_init(void)
>  		occ_add_sensor_groups(sg, phandles, phcount, chip->id);
>  		free(phandles);
>  	}
> +
> +	power_mgt = dt_find_by_path(dt_root, "/ibm,opal/power-mgt");
> +	if (!power_mgt) {
> +		prerror("OCC: dt node /ibm,opal/power-mgt not found\n");
> +		return;
> +	}
> +
> +	dt_add_property_u64s(power_mgt, "occ-sensors", occ_sensor_base,
> +			     OCC_SENSOR_DATA_BLOCK_SIZE * occ_num);
> +
> +	dt_add_property_string(power_mgt, "compatible",
> +			       "ibm,opal-occ-inband-sensors");
>  }
> -- 
> 1.8.3.1
>
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot

-- 
Stewart Smith
OPAL Architect, IBM.