[v5,3/5] thermal/drivers/sun8i: get calibration data from two nvmem cells
Commit Message
The A523 processor has calibration data in two nvmem cells. To be able to
add support, the ability to add data from two cells into one array must be
added.
Signed-off-by: Mikhail Kalashnikov <iuncuim@gmail.com>
---
drivers/thermal/sun8i_thermal.c | 80 +++++++++++++++++++++------------
1 file changed, 51 insertions(+), 29 deletions(-)
Comments
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing or empty 'nvmem-cell-names' property causes an unconditional NULL pointer dereference in the calibration callback.
- [Medium] The newly introduced A523 calibration function accesses fixed indices of the dynamically sized caldata array without validating callen.
--
commit 385d056a66304901aadb5675fbf878e2aae0cfc7
Author: Mikhail Kalashnikov <iuncuim@gmail.com>
thermal/drivers/sun8i: get calibration data from two nvmem cells
This commit updates the sun8i thermal driver to support reading calibration
data from multiple nvmem cells by iterating over nvmem-cell-names. It appends
the data from each cell into a single dynamically allocated buffer which is
passed to the calibration callbacks. This provides the ability to support the
A523 processor which has calibration data split across two cells.
> diff --git a/drivers/thermal/sun8i_thermal.c b/drivers/thermal/sun8i_thermal.c
> index df0c269708ea7..3bdd62aa8bdf1 100644
> --- a/drivers/thermal/sun8i_thermal.c
> +++ b/drivers/thermal/sun8i_thermal.c
> @@ -301,43 +301,65 @@ static int sun50i_h6_ths_calibrate(struct ths_device *tmdev,
>
> static int sun8i_ths_calibrate(struct ths_device *tmdev)
> {
> - struct nvmem_cell *calcell;
> + struct nvmem_cell *calcell = NULL;
> struct device *dev = tmdev->dev;
> - u16 *caldata;
> - size_t callen;
> + struct device_node *np = dev_of_node(dev);
> + struct property *prop;
> + const char *cellname;
> + u8 *caldata = NULL;
> + size_t callen = 0;
> int ret = 0;
>
> - calcell = nvmem_cell_get(dev, "calibration");
> - if (IS_ERR(calcell)) {
[ ... ]
> + memcpy(caldata + callen, caldatapart, len);
> + callen += len;
> + kfree(caldatapart);
> }
>
> - tmdev->chip->calibrate(tmdev, caldata, callen);
> + tmdev->chip->calibrate(tmdev, (u16 *)caldata, callen);
[Severity: High]
If the nvmem-cell-names property is missing from the device tree, the
of_property_for_each_string() loop is bypassed entirely and caldata remains
NULL.
Does this unconditionally pass a NULL pointer to the backend calibration
callback?
Backend callbacks like sun8i_h3_ths_calibrate() dereference the array directly:
if (!caldata[0] || callen < 2 * tmdev->chip->sensor_num)
return -EINVAL;
If the device tree does not contain calibration nvmem cells, this will cause a
kernel panic on boot.
[Severity: Medium]
For the A523 processor, does sun55i_a523_ths_calibrate() validate that callen
is large enough before accessing the array?
Looking at sun55i_a523_ths_calibrate() which was recently introduced:
if (tmdev->chip->sensor_num == 1) {
sensor_reg = ((caldata[5] >> 8) | (caldata[6] << 8)) & TEMP_CALIB_MASK;
} else {
It accesses elements up to caldata[6] without verifying that callen covers it.
If a device tree defines nvmem cells that yield less than 14 bytes of data,
will these lookups read out of bounds?
On Sun, Jul 5, 2026 at 1:15 AM Mikhail Kalashnikov <iuncuim@gmail.com> wrote:
>
> The A523 processor has calibration data in two nvmem cells. To be able to
> add support, the ability to add data from two cells into one array must be
> added.
>
> Signed-off-by: Mikhail Kalashnikov <iuncuim@gmail.com>
Reviewed-by: Chen-Yu Tsai <wens@kernel.org>
Some of the variables could be moved into the of_property_for_each_string()
loop, but it works either way.
@@ -301,43 +301,65 @@ static int sun50i_h6_ths_calibrate(struct ths_device *tmdev,
static int sun8i_ths_calibrate(struct ths_device *tmdev)
{
- struct nvmem_cell *calcell;
+ struct nvmem_cell *calcell = NULL;
struct device *dev = tmdev->dev;
- u16 *caldata;
- size_t callen;
+ struct device_node *np = dev_of_node(dev);
+ struct property *prop;
+ const char *cellname;
+ u8 *caldata = NULL;
+ size_t callen = 0;
int ret = 0;
- calcell = nvmem_cell_get(dev, "calibration");
- if (IS_ERR(calcell)) {
- if (PTR_ERR(calcell) == -EPROBE_DEFER)
- return -EPROBE_DEFER;
- /*
- * Even if the external calibration data stored in sid is
- * not accessible, the THS hardware can still work, although
- * the data won't be so accurate.
- *
- * The default value of calibration register is 0x800 for
- * every sensor, and the calibration value is usually 0x7xx
- * or 0x8xx, so they won't be away from the default value
- * for a lot.
- *
- * So here we do not return error if the calibration data is
- * not available, except the probe needs deferring.
- */
- goto out;
- }
+ of_property_for_each_string(np, "nvmem-cell-names", prop, cellname) {
+ size_t len;
+ u8 *caldatapart;
+
+ calcell = of_nvmem_cell_get(np, cellname);
+ if (IS_ERR(calcell)) {
+ if (PTR_ERR(calcell) == -EPROBE_DEFER)
+ return -EPROBE_DEFER;
+ /*
+ * Even if the external calibration data stored in sid is
+ * not accessible, the THS hardware can still work, although
+ * the data won't be so accurate.
+ *
+ * The default value of calibration register is 0x800 for
+ * every sensor, and the calibration value is usually 0x7xx
+ * or 0x8xx, so they won't be away from the default value
+ * for a lot.
+ *
+ * So here we do not return error if the calibration data is
+ * not available, except the probe needs deferring.
+ */
+ goto out;
+ }
+
+ caldatapart = nvmem_cell_read(calcell, &len);
+ nvmem_cell_put(calcell);
+ calcell = NULL;
+ if (IS_ERR(caldatapart)) {
+ ret = PTR_ERR(caldatapart);
+ goto out;
+ }
+
+ caldata = devm_krealloc(dev, caldata, callen + len, GFP_KERNEL);
+ if (!caldata) {
+ kfree(caldatapart);
+ ret = -ENOMEM;
+ goto out;
+ }
- caldata = nvmem_cell_read(calcell, &callen);
- if (IS_ERR(caldata)) {
- ret = PTR_ERR(caldata);
- goto out;
+ memcpy(caldata + callen, caldatapart, len);
+ callen += len;
+ kfree(caldatapart);
}
- tmdev->chip->calibrate(tmdev, caldata, callen);
+ tmdev->chip->calibrate(tmdev, (u16 *)caldata, callen);
- kfree(caldata);
+ devm_kfree(dev, caldata);
+ caldata = NULL;
out:
- if (!IS_ERR(calcell))
+ if (!IS_ERR_OR_NULL(calcell))
nvmem_cell_put(calcell);
return ret;
}