[PATCH v2 0/2] SCPI (pre-v1.0): fix reading sensor value

[Martin Blumenstingl]

I observed the following "strange" value when trying to read the SCPI
temperature sensor on my Amlogic GXM S912 device:
$ cat /sys/class/hwmon/hwmon0/temp1_input
6875990994467160116

The value reported by the original kernel (Amlogic vendor kernel, after
a reboot obviously) was 53C.
The Amlogic SCPI driver only uses a single 32bit value to read the
sensor value, instead of two. After stripping the upper 32bits from
above value gives "52" as result, which is basically identical to
what the vendor kernel reports.
I also compared this with the value shown by u-boot (since there's
less delay between "reboot to u-boot" compared to "reboot from mainline
kernel to vendor kernel") and the temperature reported by u-boot always
matches the lower 32bits of the value from scpi-hwmon temp1_input.

Version 1 of this series introduced a separate function for reading the
sensor value on pre-v1.0 SCPI firmwares. I also tried initializing the
"buf" variable in scpi_sensor_get_value() but this didn't work (because
we request a 64bit payload from the SCPI firmware, but the firmware only
replies with a 32bit payload).

Version 2 is different as it does not require a "legacy implementation"
for scpi_sensor_get_value(). Instead the rx_buf is zeroed out before
reading the buffer from the mailbox. This works fine as well, since the
sensor_value.hi_val field was added after sensor_value.lo_val (meaning
it is backwards compatible, as long as hi_val is all zeroes on
pre-v1.0 SCPI firmwares).
A small benefit we get from this: we are now able to handle all
commands where new fields are introduced at the end of receive buffer
(which might be relevant also for future SCPI implementations - but
this is pure speculation).
I did not remove the memset(buf, 0, len) in scpi_process_cmd() because
I am not sure if there are v1.0 SCPI firmwares out there which respond
that X bytes are available in the rx_buf while it actually writes more
data (X + n where n > 0 bytes) than indicated.


Changes since v1:
- zero out the rx_buf before reading the mbox buffer (see long
  description above) instead of introducing a separate legacy command
  for reading the sensor value
- added patch 2/2 which validates the payload lengths (so nobody can
  read or write data beyond rx_buf or tx_buf). This optional and patch
  1/2 can be applied without it


Martin Blumenstingl (2):
  firmware: arm_scpi: zero RX buffer before requesting data from the
    mbox
  firmware: arm_scpi: check the payload length in scpi_send_message

 drivers/firmware/arm_scpi.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

-- 
2.10.2