diff mbox

[01/11] core: add a platform op to read sensors

Message ID 1423117857-32759-2-git-send-email-clg@fr.ibm.com
State Changes Requested
Headers show

Commit Message

Cédric Le Goater Feb. 5, 2015, 6:30 a.m. UTC
This patch introduces an initial framework to define a sensor_read
operation per platform. It also proposes a few helper routines to
work on the sensor 'handler' which identifies a sensor and attribute
in the OPAL_SENSOR_READ call.

Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
---

 Changes since RFC:

 - added a 'sensor_read' operation for the open power platform

 core/Makefile.inc            |    2 +-
 core/init.c                  |    4 ++++
 core/sensor.c                |   35 +++++++++++++++++++++++++++++
 hw/fsp/fsp-sensor.c          |    5 +----
 include/fsp.h                |    2 ++
 include/platform.h           |    6 +++++
 include/sensor.h             |   51 ++++++++++++++++++++++++++++++++++++++++++
 platforms/astbmc/astbmc.h    |    2 ++
 platforms/astbmc/common.c    |    6 +++++
 platforms/astbmc/firestone.c |    1 +
 platforms/astbmc/habanero.c  |    1 +
 platforms/astbmc/palmetto.c  |    1 +
 platforms/ibm-fsp/common.c   |    5 +++++
 platforms/ibm-fsp/firenze.c  |    1 +
 platforms/ibm-fsp/ibm-fsp.h  |    3 +++
 15 files changed, 120 insertions(+), 5 deletions(-)
 create mode 100644 core/sensor.c
 create mode 100644 include/sensor.h

Comments

Stewart Smith Feb. 12, 2015, 7:42 a.m. UTC | #1
Cédric Le Goater <clg@fr.ibm.com> writes:
> +static int64_t opal_sensor_read(uint32_t sensor_hndl, int token,
> +		uint32_t *sensor_data)
> +{
> +	if (platform.sensor_read)
> +		return platform.sensor_read(sensor_hndl, token, sensor_data);
> +
> +	return OPAL_UNSUPPORTED;
> +}

It seems that previous behavior is to return OPAL_PARAMETER when
OPAL_SENSOR_READ is called with an unsupported sensor handle (at least
from my reading of parse_sensor_id in hw/fsp/fsp-sensor.c)

It would also be great if you document the OPAL_SENSOR_READ API while
you're at it too.

It seems that on non-fsp systems you would have gotten OPAL_PARAMETER
back as kernel should have been calling OPAL_CHECK_TOKEN first.
Cédric Le Goater Feb. 16, 2015, 6:12 p.m. UTC | #2
On 02/12/2015 08:42 AM, Stewart Smith wrote:
> Cédric Le Goater <clg@fr.ibm.com> writes:
>> +static int64_t opal_sensor_read(uint32_t sensor_hndl, int token,
>> +		uint32_t *sensor_data)
>> +{
>> +	if (platform.sensor_read)
>> +		return platform.sensor_read(sensor_hndl, token, sensor_data);
>> +
>> +	return OPAL_UNSUPPORTED;
>> +}
> 
> It seems that previous behavior is to return OPAL_PARAMETER when
> OPAL_SENSOR_READ is called with an unsupported sensor handle (at least
> from my reading of parse_sensor_id in hw/fsp/fsp-sensor.c)

yes but OPAL_UNSUPPORTED seemed to be an improvement in that case. The return
value of opal_sensor_read() is only tested against OPAL_ASYNC_COMPLETION,
so I don't think we are breaking anything. 

> It would also be great if you document the OPAL_SENSOR_READ API while
> you're at it too.

OK. will do.

> It seems that on non-fsp systems you would have gotten OPAL_PARAMETER
> back as kernel should have been calling OPAL_CHECK_TOKEN first.

This is not the case. Something to fix in the kernel. I will look into it.

Thanks,

C.
Stewart Smith Feb. 16, 2015, 10:16 p.m. UTC | #3
Cedric Le Goater <clg@fr.ibm.com> writes:
> On 02/12/2015 08:42 AM, Stewart Smith wrote:
>> Cédric Le Goater <clg@fr.ibm.com> writes:
>>> +static int64_t opal_sensor_read(uint32_t sensor_hndl, int token,
>>> +		uint32_t *sensor_data)
>>> +{
>>> +	if (platform.sensor_read)
>>> +		return platform.sensor_read(sensor_hndl, token, sensor_data);
>>> +
>>> +	return OPAL_UNSUPPORTED;
>>> +}
>> 
>> It seems that previous behavior is to return OPAL_PARAMETER when
>> OPAL_SENSOR_READ is called with an unsupported sensor handle (at least
>> from my reading of parse_sensor_id in hw/fsp/fsp-sensor.c)
>
> yes but OPAL_UNSUPPORTED seemed to be an improvement in that case. The return
> value of opal_sensor_read() is only tested against OPAL_ASYNC_COMPLETION,
> so I don't think we are breaking anything.

Maybe split it into two patches, one explicitly replicating existing
behavior and the other making the (slight) modification? (along with
documentation and justifying why this doesn't break existing systems).

>> It would also be great if you document the OPAL_SENSOR_READ API while
>> you're at it too.
>
> OK. will do.
>
>> It seems that on non-fsp systems you would have gotten OPAL_PARAMETER
>> back as kernel should have been calling OPAL_CHECK_TOKEN first.
>
> This is not the case. Something to fix in the kernel. I will look into it.

Excellent. Look forward to seeing the patches!

(I've been trying to document if OPAL_CHECK_TOKEN should be used in
doc/opal-api )
diff mbox

Patch

diff --git a/core/Makefile.inc b/core/Makefile.inc
index 8540695ea035..e187df175e07 100644
--- a/core/Makefile.inc
+++ b/core/Makefile.inc
@@ -7,7 +7,7 @@  CORE_OBJS += timebase.o opal-msg.o pci.o pci-opal.o fast-reboot.o
 CORE_OBJS += device.o exceptions.o trace.o affinity.o vpd.o
 CORE_OBJS += hostservices.o platform.o nvram.o flash-nvram.o hmi.o
 CORE_OBJS += console-log.o ipmi.o time-utils.o pel.o pool.o errorlog.o
-CORE_OBJS += timer.o i2c.o rtc.o
+CORE_OBJS += timer.o i2c.o rtc.o sensor.o
 CORE=core/built-in.o
 
 CFLAGS_SKIP_core/relocate.o = -pg -fstack-protector-all
diff --git a/core/init.c b/core/init.c
index 189e7fc12801..ecee50139b79 100644
--- a/core/init.c
+++ b/core/init.c
@@ -43,6 +43,7 @@ 
 #include <libfdt/libfdt.h>
 #include <hostservices.h>
 #include <timer.h>
+#include <sensor.h>
 
 /*
  * Boot semaphore, incremented by each CPU calling in
@@ -619,6 +620,9 @@  void __noreturn main_cpu_entry(const void *fdt, u32 master_cpu)
 	if (platform.init)
 		platform.init();
 
+	/* Register routine to dispatch and read sensors */
+	sensor_init();
+
 	/* Setup dummy console nodes if it's enabled */
 	if (dummy_console_enabled())
 		dummy_console_add_nodes();
diff --git a/core/sensor.c b/core/sensor.c
new file mode 100644
index 000000000000..5db072e0e501
--- /dev/null
+++ b/core/sensor.c
@@ -0,0 +1,35 @@ 
+/* Copyright 2013-2014 IBM Corp.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *	http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+ * implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+
+#include <sensor.h>
+#include <skiboot.h>
+#include <opal.h>
+
+static int64_t opal_sensor_read(uint32_t sensor_hndl, int token,
+		uint32_t *sensor_data)
+{
+	if (platform.sensor_read)
+		return platform.sensor_read(sensor_hndl, token, sensor_data);
+
+	return OPAL_UNSUPPORTED;
+}
+
+void sensor_init(void)
+{
+	/* Register OPAL interface */
+	opal_register(OPAL_SENSOR_READ, opal_sensor_read, 3);
+}
diff --git a/hw/fsp/fsp-sensor.c b/hw/fsp/fsp-sensor.c
index c40ba23d35e2..0287ad5ac047 100644
--- a/hw/fsp/fsp-sensor.c
+++ b/hw/fsp/fsp-sensor.c
@@ -489,7 +489,7 @@  static int64_t parse_sensor_id(uint32_t id, struct opal_sensor_data *attr)
 }
 
 
-static int64_t fsp_opal_read_sensor(uint32_t sensor_hndl, int token,
+int64_t fsp_opal_read_sensor(uint32_t sensor_hndl, int token,
 		uint32_t *sensor_data)
 {
 	struct opal_sensor_data *attr;
@@ -728,9 +728,6 @@  void fsp_init_sensor(void)
 	/* Map TCE */
 	fsp_tce_map(PSI_DMA_SENSOR_BUF, sensor_buffer, PSI_DMA_SENSOR_BUF_SZ);
 
-	/* Register OPAL interface */
-	opal_register(OPAL_SENSOR_READ, fsp_opal_read_sensor, 3);
-
 	msg.resp = &resp;
 
 	/* Traverse using all the modifiers to know all the sensors available
diff --git a/include/fsp.h b/include/fsp.h
index 9bb7fd8de125..d4326580cbc7 100644
--- a/include/fsp.h
+++ b/include/fsp.h
@@ -767,6 +767,8 @@  extern void fsp_memory_err_init(void);
 
 /* Sensor */
 extern void fsp_init_sensor(void);
+extern int64_t fsp_opal_read_sensor(uint32_t sensor_hndl, int token,
+			uint32_t *sensor_data);
 
 /* Diagnostic */
 extern void fsp_init_diag(void);
diff --git a/include/platform.h b/include/platform.h
index b1aef490d296..c5387ec5dd9c 100644
--- a/include/platform.h
+++ b/include/platform.h
@@ -131,6 +131,12 @@  struct platform {
 	 */
 	bool		(*load_resource)(enum resource_id id,
 					 void *buf, size_t *len);
+
+	/*
+	 * Read a sensor value
+	 */
+	int64_t		(*sensor_read)(uint32_t sensor_hndl, int token,
+				       uint32_t *sensor_data);
 };
 
 extern struct platform __platforms_start;
diff --git a/include/sensor.h b/include/sensor.h
new file mode 100644
index 000000000000..385091ae1b87
--- /dev/null
+++ b/include/sensor.h
@@ -0,0 +1,51 @@ 
+/* Copyright 2013-2014 IBM Corp.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *	http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
+ * implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef __SENSOR_H
+#define __SENSOR_H
+
+/*
+ * A sensor handler is a four bytes value which identifies a sensor by
+ * its resource class (temperature, fans ...), a resource identifier
+ * and an attribute number (data, status, ...) :
+ *
+ *                 Res.
+ *     | Attr.  | Class  |   Resource Id  |
+ *     |--------|--------|----------------|
+ *
+ *
+ * Helper routines to build or use the sensor handler.
+ */
+#define sensor_make_handler(sensor_class, sensor_rid, sensor_attr) \
+	(((sensor_attr) << 24) | ((sensor_class) & 0xff) << 16 | \
+	 ((sensor_rid) & 0xffff))
+
+#define sensor_get_frc(handler)		(((handler) >> 16) & 0xff)
+#define sensor_get_rid(handler)		((handler) & 0xffff)
+#define sensor_get_attr(handler)	((handler) >> 24)
+
+/*
+ * Sensor families
+ *
+ * This identifier is used to dispatch calls to OPAL_SENSOR_READ to
+ * the appropriate component. FSP is the initial family.
+ */
+#define SENSOR_FSP 0x0
+
+
+extern void sensor_init(void);
+
+#endif /* __SENSOR_H */
diff --git a/platforms/astbmc/astbmc.h b/platforms/astbmc/astbmc.h
index cee475a9d867..7b1e5d32ec24 100644
--- a/platforms/astbmc/astbmc.h
+++ b/platforms/astbmc/astbmc.h
@@ -24,5 +24,7 @@  extern int64_t astbmc_ipmi_power_down(uint64_t request);
 extern void astbmc_init(void);
 extern void astbmc_ext_irq(unsigned int chip_id);
 extern int pnor_init(void);
+extern int64_t astbmc_sensor_read(uint32_t sensor_hndl, int token,
+				uint32_t *sensor_data);
 
 #endif /* __ASTBMC_H */
diff --git a/platforms/astbmc/common.c b/platforms/astbmc/common.c
index f9c988d724a5..7336bbfb83e3 100644
--- a/platforms/astbmc/common.c
+++ b/platforms/astbmc/common.c
@@ -337,3 +337,9 @@  void astbmc_early_init(void)
 	/* Setup UART and use it as console with interrupts */
 	uart_init(true);
 }
+
+int64_t __attrconst astbmc_sensor_read(uint32_t sensor_hndl __unused,
+	int token __unused, uint32_t *sensor_data __unused)
+{
+	return OPAL_UNSUPPORTED;
+}
diff --git a/platforms/astbmc/firestone.c b/platforms/astbmc/firestone.c
index 4a51e3fb9ede..0aaa4347e6e4 100644
--- a/platforms/astbmc/firestone.c
+++ b/platforms/astbmc/firestone.c
@@ -41,4 +41,5 @@  DECLARE_PLATFORM(firestone) = {
 	.external_irq		= astbmc_ext_irq,
 	.cec_power_down         = astbmc_ipmi_power_down,
 	.cec_reboot             = astbmc_ipmi_reboot,
+	.sensor_read		= astbmc_sensor_read,
 };
diff --git a/platforms/astbmc/habanero.c b/platforms/astbmc/habanero.c
index d442d1f06e45..fe8e2ef19271 100644
--- a/platforms/astbmc/habanero.c
+++ b/platforms/astbmc/habanero.c
@@ -49,4 +49,5 @@  DECLARE_PLATFORM(habanero) = {
 	.external_irq		= astbmc_ext_irq,
 	.cec_power_down         = astbmc_ipmi_power_down,
 	.cec_reboot             = astbmc_ipmi_reboot,
+	.sensor_read		= astbmc_sensor_read,
 };
diff --git a/platforms/astbmc/palmetto.c b/platforms/astbmc/palmetto.c
index a0030e802c51..ee492f78b25e 100644
--- a/platforms/astbmc/palmetto.c
+++ b/platforms/astbmc/palmetto.c
@@ -51,4 +51,5 @@  DECLARE_PLATFORM(palmetto) = {
 	.cec_power_down         = astbmc_ipmi_power_down,
 	.cec_reboot             = astbmc_ipmi_reboot,
 	.elog_commit		= ipmi_elog_commit,
+	.sensor_read		= astbmc_sensor_read,
 };
diff --git a/platforms/ibm-fsp/common.c b/platforms/ibm-fsp/common.c
index 5eb2a14cbecf..695d8e47db7c 100644
--- a/platforms/ibm-fsp/common.c
+++ b/platforms/ibm-fsp/common.c
@@ -203,3 +203,8 @@  int64_t ibm_fsp_cec_power_down(uint64_t request)
 	return OPAL_SUCCESS;
 }
 
+int64_t ibm_fsp_sensor_read(uint32_t sensor_hndl, int token,
+				uint32_t *sensor_data)
+{
+	return fsp_opal_read_sensor(sensor_hndl, token, sensor_data);
+}
diff --git a/platforms/ibm-fsp/firenze.c b/platforms/ibm-fsp/firenze.c
index 258a6b340c40..57b5ce57efae 100644
--- a/platforms/ibm-fsp/firenze.c
+++ b/platforms/ibm-fsp/firenze.c
@@ -399,4 +399,5 @@  DECLARE_PLATFORM(firenze) = {
 	.occ_timeout		= ibm_fsp_occ_timeout,
 	.elog_commit		= elog_fsp_commit,
 	.load_resource		= fsp_load_resource,
+	.sensor_read		= ibm_fsp_sensor_read,
 } ;
diff --git a/platforms/ibm-fsp/ibm-fsp.h b/platforms/ibm-fsp/ibm-fsp.h
index 160038ae5745..48a089143147 100644
--- a/platforms/ibm-fsp/ibm-fsp.h
+++ b/platforms/ibm-fsp/ibm-fsp.h
@@ -26,4 +26,7 @@  extern int64_t ibm_fsp_cec_reboot(void);
 struct errorlog;
 extern int elog_fsp_commit(struct errorlog *buf);
 
+extern int64_t ibm_fsp_sensor_read(uint32_t sensor_hndl, int token,
+				uint32_t *sensor_data);
+
 #endif /*  __IBM_FSP_COMMON_H */