* Re: [RFC PATCH 1/3] dt-bindings: iio: temperature: Add STS30 devicetree bindings
From: sashiko-bot @ 2026-06-20 4:52 UTC (permalink / raw)
To: Maxwell Doose; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260620044010.1082621-2-m32285159@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The STS30 device tree binding omits the `interrupts` property, preventing users from modeling the hardware's ALERT pin.
--
commit 91523cd6a186b322c33e0fb78f1316cdc24a39af
Author: Maxwell Doose <m32285159@gmail.com>
dt-bindings: iio: temperature: Add STS30 devicetree bindings
This commit adds device tree bindings for the Sensirion STS30 family of
digital temperature sensors, which includes the STS30, STS31, and STS35.
> diff --git a/Documentation/devicetree/bindings/iio/temperature/sensirion,sts30.yaml b/Documentation/devicetree/bindings/iio/temperature/sensirion,sts30.yaml
> new file mode 100644
> index 0000000000000..dc0c6b7832fb8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/temperature/sensirion,sts30.yaml
[ ... ]
> +properties:
> + compatible:
> + enum:
> + - sensirion,sts30
> + - sensirion,sts31
> + - sensirion,sts35
> +
> + reg:
> + maxItems: 1
> +
> + vdd-supply: true
> +
> +required:
> + - compatible
> + - reg
> +
> +additionalProperties: false
[Severity: Medium]
Does this schema prevent modeling the hardware's ALERT pin?
The Sensirion STS3x family hardware features an ALERT pin that can be wired
to an interrupt controller to signal high and low temperature thresholds.
Because this schema enforces additionalProperties: false but omits the
interrupts property from the properties block, any device tree that
accurately models the physical hardware connection will fail
dt_binding_check.
Should the interrupts property be explicitly listed to allow accurately
describing the hardware capabilities, even if the current IIO driver does
not yet utilize the ALERT pin?
> +
> +examples:
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260620044010.1082621-1-m32285159@gmail.com?part=1
^ permalink raw reply
* Re: [RFC PATCH 2/3] iio: temperature: Add STS30 temperature sensor driver
From: sashiko-bot @ 2026-06-20 4:48 UTC (permalink / raw)
To: Maxwell Doose; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260620044010.1082621-3-m32285159@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The return value of `sts30_reset()` is ignored during probe, allowing phantom device registration on failure.
- [High] The driver implements a Clock Stretching Disabled I2C sequence but uses Clock Stretching Enabled command codes, resulting in a protocol mismatch.
--
commit 1a183c37bbba5fe87dfcc6019f964e51526fec2b
Author: Maxwell Doose <m32285159@gmail.com>
iio: temperature: Add STS30 temperature sensor driver
Add a driver for the Sensirion STS30 family of temperature sensor
drivers over I2C. The driver supports single-shot non-clock stretched
readings using a specified delay based on the repeatability/delay.
> diff --git a/drivers/iio/temperature/sts30.c b/drivers/iio/temperature/sts30.c
> new file mode 100644
> index 0000000000000..b49deb1d62c21
> --- /dev/null
> +++ b/drivers/iio/temperature/sts30.c
[ ... ]
> +#define STS30_MEAS_SIZE 3
> +
> +#define STS30_COMMAND_READ_HIGH_REPEAT 0x2C06
> +#define STS30_COMMAND_READ_MED_REPEAT 0x2C0D
> +#define STS30_COMMAND_READ_LOW_REPEAT 0x2C10
[Severity: High]
The commit message mentions using non-clock stretched readings, but are
these the correct commands? According to the STS3x datasheet, the 0x2Cxx
series commands are for "Clock Stretching Enabled", which require a
Repeated START condition between the write and read phases.
[ ... ]
> +static int sts30_read(struct sts30_data *data, u16 command, u16 *val)
> +{
> + u8 tmp[2];
> + u8 buf[STS30_MEAS_SIZE];
> + int ret;
> +
> + put_unaligned_be16(command, tmp);
> +
> + ret = i2c_master_send(data->client, tmp, sizeof(tmp));
> + if (ret < 0)
> + return ret;
> + if (ret != sizeof(tmp))
> + return -EIO;
> +
> + fsleep(data->delay);
[Severity: High]
Will sending a STOP condition here abort the measurement? Since
i2c_master_send() issues a STOP condition rather than a Repeated START,
this sequence violates the protocol for the Clock Stretching Enabled
commands (0x2Cxx) defined above.
Should this use the "Clock Stretching Disabled" commands (e.g., 0x2400)
instead, or alternatively use a single i2c_transfer() with a Repeated START?
[ ... ]
> +static int sts30_probe(struct i2c_client *client)
> +{
> + struct iio_dev *indio_dev;
> + struct sts30_data *data;
> + int ret;
[ ... ]
> + ret = sts30_reset(data);
> +
> + return devm_iio_device_register(&client->dev, indio_dev);
> +}
[Severity: High]
Is it intentional to ignore the return value of sts30_reset() here?
If the reset fails due to the hardware being missing or a broken I2C bus,
this will unconditionally register a phantom device to userspace. Should we
check 'ret' and return the error code instead?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260620044010.1082621-1-m32285159@gmail.com?part=2
^ permalink raw reply
* [RFC PATCH 3/3] MAINTAINERS: Add entry for Sensirion STS30 driver
From: Maxwell Doose @ 2026-06-20 4:40 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
open list:IIO SUBSYSTEM AND DRIVERS,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
In-Reply-To: <20260620044010.1082621-1-m32285159@gmail.com>
Add the entry for the STS30 temperature sensor driver in MAINTAINERS.
Signed-off-by: Maxwell Doose <m32285159@gmail.com>
---
MAINTAINERS | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index d95d3ef77773..6bb361ac213f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -24487,6 +24487,12 @@ F: drivers/iio/chemical/sps30.c
F: drivers/iio/chemical/sps30_i2c.c
F: drivers/iio/chemical/sps30_serial.c
+SENSIRION STS30 TEMPERATURE SENSOR DRIVER
+M: Maxwell Doose <m32285159@gmail.com>
+S: Maintained
+F: Documentation/devicetree/bindings/iio/temperature/sensirion,sts30.yaml
+F: drivers/iio/temperature/sts30.c
+
SERIAL DEVICE BUS
M: Rob Herring <robh@kernel.org>
L: linux-serial@vger.kernel.org
--
2.54.0
^ permalink raw reply related
* [RFC PATCH 2/3] iio: temperature: Add STS30 temperature sensor driver
From: Maxwell Doose @ 2026-06-20 4:40 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
open list:IIO SUBSYSTEM AND DRIVERS,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
In-Reply-To: <20260620044010.1082621-1-m32285159@gmail.com>
Add a driver for the Sensirion STS30 family of temperature sensor
drivers over I2C. The STS30 family of sensors includes the STS30, STS31,
and STS35, all of which are supported by this driver, since they all
share the same commands, etc. and only differ in accuracy and tolerance.
The driver currently supports single-shot non-clock stretched readings,
by using a specified delay based on the repeatability/delay specified
by the user. The repeatability/delay can be changed at any time through
sysfs.
Additionally add Kconfig and Makefile entries for the driver.
Signed-off-by: Maxwell Doose <m32285159@gmail.com>
---
drivers/iio/temperature/Kconfig | 11 ++
drivers/iio/temperature/Makefile | 1 +
drivers/iio/temperature/sts30.c | 321 +++++++++++++++++++++++++++++++
3 files changed, 333 insertions(+)
create mode 100644 drivers/iio/temperature/sts30.c
diff --git a/drivers/iio/temperature/Kconfig b/drivers/iio/temperature/Kconfig
index 9328b2250ace..c23a9c40c725 100644
--- a/drivers/iio/temperature/Kconfig
+++ b/drivers/iio/temperature/Kconfig
@@ -88,6 +88,17 @@ config MLX90635
This driver can also be built as a module. If so, the module will
be called mlx90635.
+config STS30
+ tristate "STS30/STS31/STS35 temperature sensor"
+ depends on I2C
+ select CRC8
+ help
+ If you say yes here you get support for the Sensirion STS30,
+ STS31, and STS35 temperature sensors over I2C.
+
+ This driver can also be built as a module. If so, the module will
+ be called sts30.
+
config TMP006
tristate "TMP006 infrared thermopile sensor"
depends on I2C
diff --git a/drivers/iio/temperature/Makefile b/drivers/iio/temperature/Makefile
index 07d6e65709f7..9c9bf8d1b70e 100644
--- a/drivers/iio/temperature/Makefile
+++ b/drivers/iio/temperature/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_MCP9600) += mcp9600.o
obj-$(CONFIG_MLX90614) += mlx90614.o
obj-$(CONFIG_MLX90632) += mlx90632.o
obj-$(CONFIG_MLX90632) += mlx90635.o
+obj-$(CONFIG_STS30) += sts30.o
obj-$(CONFIG_TMP006) += tmp006.o
obj-$(CONFIG_TMP007) += tmp007.o
obj-$(CONFIG_TMP117) += tmp117.o
diff --git a/drivers/iio/temperature/sts30.c b/drivers/iio/temperature/sts30.c
new file mode 100644
index 000000000000..b49deb1d62c2
--- /dev/null
+++ b/drivers/iio/temperature/sts30.c
@@ -0,0 +1,321 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2026 Maxwell Doose
+ *
+ * Sensirion STS30 temperature sensor driver
+ *
+ * Datasheet: https://sensirion.com/media/documents/1DA31AFD/65D613A8/Datasheet_STS3x_DIS.pdf
+ *
+ * Author: Maxwell Doose <m32285159@gmail.com>
+ */
+
+#include <linux/bits.h>
+#include <linux/cleanup.h>
+#include <linux/crc8.h>
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/errno.h>
+#include <linux/export.h>
+#include <linux/iio/iio.h>
+#include <linux/iio/types.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+
+/* Amount of bytes received from the STS30 after a read command */
+#define STS30_MEAS_SIZE 3
+
+#define STS30_COMMAND_READ_HIGH_REPEAT 0x2C06
+#define STS30_COMMAND_READ_MED_REPEAT 0x2C0D
+#define STS30_COMMAND_READ_LOW_REPEAT 0x2C10
+
+/* Soft reset command */
+#define STS30_COMMAND_RESET 0x30A2
+
+/*
+ * sts30 includes a CRC8 checksum at the end of its i2c responses. The polynomial
+ * is used to generate the CRC8 table and the seed is the starting value.
+ */
+#define STS30_CRC8_POLYNOMIAL 0x31
+#define STS30_CRC8_SEED 0xFF
+
+DECLARE_CRC8_TABLE(sts30_crc_table);
+
+enum sts30_read_delays {
+ STS30_REPEAT_LOW = 4500,
+ STS30_REPEAT_MED = 6000,
+ STS30_REPEAT_HIGH = 15000
+};
+
+/**
+ * struct sts30_data - data structure for STS30 driver
+ *
+ * @client: underlying i2c client data structure
+ * @lock: mutex for serialized communication on the i2c bus
+ * @delay: measurement duration for the current repeatability mode
+ */
+struct sts30_data {
+ struct i2c_client *client;
+ struct mutex lock;
+ /*
+ * sts30 has three potential repeatability/measurement durations. We need to
+ * account for them while reading the i2c bus.
+ *
+ * See section 2.2 in the datasheet for more info on processing times.
+ */
+ enum sts30_read_delays delay;
+};
+
+static int sts30_verify_crc8(struct sts30_data *data, u8 buf[STS30_MEAS_SIZE])
+{
+ int crc;
+
+ crc = crc8(sts30_crc_table, buf, 2, STS30_CRC8_SEED);
+ if (crc != buf[2]) {
+ dev_err(&data->client->dev, "Expected CRC8 value of 0x%02x, got 0x%02x\n",
+ buf[2], crc);
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static int sts30_read(struct sts30_data *data, u16 command, u16 *val)
+{
+ u8 tmp[2];
+ u8 buf[STS30_MEAS_SIZE];
+ int ret;
+
+ put_unaligned_be16(command, tmp);
+
+ ret = i2c_master_send(data->client, tmp, sizeof(tmp));
+ if (ret < 0)
+ return ret;
+ if (ret != sizeof(tmp))
+ return -EIO;
+
+ fsleep(data->delay);
+
+ ret = i2c_master_recv(data->client, buf, sizeof(buf));
+ if (ret < 0)
+ return ret;
+ if (ret != sizeof(buf))
+ return -EIO;
+
+ *val = get_unaligned_be16(buf);
+
+ ret = sts30_verify_crc8(data, buf);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int sts30_write(struct sts30_data *data, u16 command)
+{
+ u8 buf[2];
+ int ret;
+
+ put_unaligned_be16(command, buf);
+
+ ret = i2c_master_send(data->client, buf, sizeof(buf));
+ if (ret < 0)
+ return ret;
+ if (ret != sizeof(buf))
+ return -EIO;
+
+ return 0;
+}
+
+static int sts30_reset(struct sts30_data *data)
+{
+ int ret;
+
+ guard(mutex)(&data->lock);
+
+ ret = sts30_write(data, STS30_COMMAND_RESET);
+ if (ret)
+ return ret;
+
+ fsleep(1500);
+
+ return 0;
+}
+
+static int sts30_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int *val, int *val2,
+ long mask)
+{
+ struct sts30_data *data = iio_priv(indio_dev);
+ int ret;
+ u16 tmp;
+
+ guard(mutex)(&data->lock);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ switch (data->delay) {
+ case STS30_REPEAT_LOW:
+ ret = sts30_read(data, STS30_COMMAND_READ_LOW_REPEAT, &tmp);
+ break;
+ case STS30_REPEAT_MED:
+ ret = sts30_read(data, STS30_COMMAND_READ_MED_REPEAT, &tmp);
+ break;
+ case STS30_REPEAT_HIGH:
+ ret = sts30_read(data, STS30_COMMAND_READ_HIGH_REPEAT, &tmp);
+ break;
+ default:
+ dev_warn(&data->client->dev, "Repeatability state corrupted, got: %d\n",
+ data->delay);
+ return -EINVAL;
+ }
+
+ if (ret)
+ return ret;
+
+ *val = tmp;
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_OFFSET:
+ /*
+ * We use this constant -16852 as calculated using the formula
+ * in the datasheet. See section 4.12 in the data sheet for more
+ * info.
+ */
+ *val = -16852;
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ *val = 175000;
+ *val2 = 65535;
+ return IIO_VAL_FRACTIONAL;
+ case IIO_CHAN_INFO_INT_TIME:
+ *val = 0;
+ *val2 = data->delay;
+ return IIO_VAL_INT_PLUS_MICRO;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int sts30_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan, int val, int val2,
+ long mask)
+{
+ struct sts30_data *data = iio_priv(indio_dev);
+
+ if (val)
+ return -EINVAL;
+
+ switch (mask) {
+ case IIO_CHAN_INFO_INT_TIME: {
+ guard(mutex)(&data->lock);
+
+ switch (val2) {
+ case STS30_REPEAT_LOW:
+ data->delay = STS30_REPEAT_LOW;
+ break;
+ case STS30_REPEAT_MED:
+ data->delay = STS30_REPEAT_MED;
+ break;
+ case STS30_REPEAT_HIGH:
+ data->delay = STS30_REPEAT_HIGH;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+ }
+ default:
+ return -EINVAL;
+ }
+}
+
+static const struct iio_info sts30_info = {
+ .read_raw = sts30_read_raw,
+ .write_raw = sts30_write_raw
+};
+
+static const struct iio_chan_spec sts30_channels[] = {
+ {
+ .type = IIO_TEMP,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_OFFSET) |
+ BIT(IIO_CHAN_INFO_SCALE) | BIT(IIO_CHAN_INFO_INT_TIME)
+ },
+};
+
+static const struct i2c_device_id sts30_id[] = {
+ { "sts30" },
+ { "sts31" },
+ { "sts35" },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, sts30_id);
+
+static int sts30_probe(struct i2c_client *client)
+{
+ struct iio_dev *indio_dev;
+ struct sts30_data *data;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ indio_dev->name = client->name;
+ indio_dev->info = &sts30_info;
+ indio_dev->channels = sts30_channels;
+ indio_dev->num_channels = ARRAY_SIZE(sts30_channels);
+ indio_dev->modes = INDIO_DIRECT_MODE;
+
+ data = iio_priv(indio_dev);
+ data->client = client;
+ data->delay = STS30_REPEAT_HIGH;
+
+ ret = devm_mutex_init(&client->dev, &data->lock);
+ if (ret)
+ return ret;
+
+ i2c_set_clientdata(client, indio_dev);
+
+ ret = sts30_reset(data);
+
+ return devm_iio_device_register(&client->dev, indio_dev);
+}
+
+static const struct of_device_id sts30_of_match[] = {
+ { .compatible = "sensirion,sts30" },
+ { .compatible = "sensirion,sts31" },
+ { .compatible = "sensirion,sts35" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, sts30_of_match);
+
+static struct i2c_driver sts30_driver = {
+ .driver = {
+ .name = "sts30",
+ .of_match_table = sts30_of_match,
+ },
+ .probe = sts30_probe,
+ .id_table = sts30_id,
+};
+
+static int __init sts30_init(void)
+{
+ crc8_populate_msb(sts30_crc_table, STS30_CRC8_POLYNOMIAL);
+
+ return i2c_add_driver(&sts30_driver);
+}
+module_init(sts30_init);
+
+static void __exit sts30_exit(void)
+{
+ i2c_del_driver(&sts30_driver);
+}
+module_exit(sts30_exit);
+
+MODULE_AUTHOR("Maxwell Doose <m32285159@gmail.com>");
+MODULE_DESCRIPTION("Sensirion STS30 temperature sensor driver");
+MODULE_LICENSE("GPL");
--
2.54.0
^ permalink raw reply related
* [RFC PATCH 1/3] dt-bindings: iio: temperature: Add STS30 devicetree bindings
From: Maxwell Doose @ 2026-06-20 4:40 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
open list:IIO SUBSYSTEM AND DRIVERS,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
In-Reply-To: <20260620044010.1082621-1-m32285159@gmail.com>
Add the devicetree bindings for the STS30 family of temperature sensors.
The STS30 family of sensors includes the STS30, STS31, and STS35.
All devices in the STS30 family share the same commands, timings, etc.
The only difference between them is their measurement accuracy and
tolerance.
Signed-off-by: Maxwell Doose <m32285159@gmail.com>
---
.../iio/temperature/sensirion,sts30.yaml | 49 +++++++++++++++++++
1 file changed, 49 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/temperature/sensirion,sts30.yaml
diff --git a/Documentation/devicetree/bindings/iio/temperature/sensirion,sts30.yaml b/Documentation/devicetree/bindings/iio/temperature/sensirion,sts30.yaml
new file mode 100644
index 000000000000..dc0c6b7832fb
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/temperature/sensirion,sts30.yaml
@@ -0,0 +1,49 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/temperature/sensirion,sts30.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STS30, STS31, and STS35 temperature iio sensors
+
+maintainers:
+ - Maxwell Doose <m32285159@gmail.com>
+
+description: |
+ Family of digital temperature sensors from Sensirion with I2C interface.
+ All devices are compatible with each other, their only differences are
+ their levels of accuracy.
+
+ Datasheet available at:
+ https://sensirion.com/media/documents/1DA31AFD/65D613A8/Datasheet_STS3x_DIS.pdf
+
+properties:
+ compatible:
+ enum:
+ - sensirion,sts30
+ - sensirion,sts31
+ - sensirion,sts35
+
+ reg:
+ maxItems: 1
+
+ vdd-supply: true
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ temperature@4a {
+ compatible = "sensirion,sts30";
+ reg = <0x4a>;
+ vdd-supply = <®_3v3>;
+ };
+ };
--
2.54.0
^ permalink raw reply related
* [RFC PATCH 0/3] Add Sensirion STS30 temperature sensor driver
From: Maxwell Doose @ 2026-06-20 4:40 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
open list:IIO SUBSYSTEM AND DRIVERS,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
Hi all,
This patch adds support for the Sensirion STS30 family of temperature
sensor drivers. It currently supports single-shot non-clock stretched
readings as well as the entirety of the STS30 family.
I'm sending this as an RFC patch as I unfortunately do not have the
hardware right now. I have compile-tested it successfully and have done
a tad bit of testing on QEMU, with it properly registering with no
errors reported in dmesg. Unfortunately due to the way I had compiled
the kernel used to test the driver it did not expose the I2C bus to
userspace properly and I was not able to retrieve values from the
driver, but I can continue more testing over the next few days and am
happy to answer any questions anyone may have. Additionally, regarding
dmesg logs, I neglected to save them while in my testing, but as stated
previously, I can continue more testing over the next couple of days.
Maxwell Doose (3):
dt-bindings: iio: temperature: Add STS30 devicetree bindings
iio: temperature: Add STS30 temperature sensor driver
MAINTAINERS: Add entry for Sensirion STS30 driver
.../iio/temperature/sensirion,sts30.yaml | 49 +++
MAINTAINERS | 6 +
drivers/iio/temperature/Kconfig | 11 +
drivers/iio/temperature/Makefile | 1 +
drivers/iio/temperature/sts30.c | 321 ++++++++++++++++++
5 files changed, 388 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/temperature/sensirion,sts30.yaml
create mode 100644 drivers/iio/temperature/sts30.c
--
2.54.0
^ permalink raw reply
* Re: [PATCH v3 3/3] media: i2c: os02g10: implement crop handling with set_selection
From: Elgin Perumbilly @ 2026-06-20 4:08 UTC (permalink / raw)
To: Sakari Ailus, Laurent Pinchart
Cc: Tarang Raval, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Hans Verkuil, Hans de Goede,
Vladimir Zapolskiy, Mehdi Djait, Sylvain Petinot,
Benjamin Mugnier, Bryan O'Donoghue, Heimir Thor Sverrisson,
Hardevsinh Palaniya, linux-media@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <ajP0TtXuQhx40pbU@kekkonen.localdomain>
Hi Sakari, Laurent
> On Thu, Jun 18, 2026 at 04:02:53PM +0300, Laurent Pinchart wrote:
> > On Thu, Jun 18, 2026 at 02:47:23PM +0300, Sakari Ailus wrote:
> > > Hi Elgin,
> > >
> > > On Fri, Apr 24, 2026 at 02:55:47PM +0530, Elgin Perumbilly wrote:
> > > > From: Tarang Raval <tarang.raval@siliconsignals.io>
> > > >
> > > > Add crop support to os02g10 by implementing .set_selection() and
> > > > storing the crop rectangle in subdev state.
> > > >
> > > > Initialize the default crop to the active area, make set_fmt() use the
> > > > current crop, and update the output format when the crop size changes.
> > > > Also program the sensor window from the active crop/format state instead
> > > > of using the fixed supported_modes entry.
> > > >
> > > > This allows userspace to configure the sensor crop window explicitly.
> > >
> > > Please wait for the Common Raw Sensor Model patches to be merged before
> > > adding this -- we don't have an established way to configure cropping
> > > before that. Some drivers might do something but it's all a bit haphazard.
> >
> > Does it mean we should wait for the raw sensor model to merge this
> > driver ?
>
> I only suggested this for the 3rd patch.
What is the conclusion?
Should I drop the 3rd patch for the time being, or include it in
the next revision?
Best Regards,
Elgin
^ permalink raw reply
* Re: [PATCH v6 17/21] RISC-V: perf: Add Qemu virt machine events
From: Atish Patra @ 2026-06-20 0:37 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-perf-users, conor+dt, robh, devicetree
In-Reply-To: <20260609062221.D90C81F00893@smtp.kernel.org>
On 6/8/26 11:22 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The patch incorrectly overloads the generic 'unimplemented' or 'non-commercial' CPU IDs (`mvendorid=0`, `marchid=0`, `mimpid=0`) to uniquely identify the QEMU virt machine, applying QEMU-specific PMU events globally to any minimal or academic RISC-V core.
> --
>
> commit 61b476b524390d76a4db3dcbab4b57114ff0ebc8
> Author: Atish Patra <atishp@rivosinc.com>
>
> RISC-V: perf: Add Qemu virt machine events
>
> This patch adds a minimal set of legacy perf events for the QEMU virt
> machine to the vendor table, allowing users to use them when counter
> delegation is enabled.
>
>> diff --git a/arch/riscv/include/asm/vendorid_list.h b/arch/riscv/include/asm/vendorid_list.h
>> index 7f5030ee1fcf8..603aa2b21c0ba 100644
>> --- a/arch/riscv/include/asm/vendorid_list.h
>> +++ b/arch/riscv/include/asm/vendorid_list.h
>> @@ -11,4 +11,8 @@
>> #define SIFIVE_VENDOR_ID 0x489
>> #define THEAD_VENDOR_ID 0x5b7
>>
>> +#define QEMU_VIRT_VENDOR_ID 0x000
>> +#define QEMU_VIRT_IMPL_ID 0x000
>> +#define QEMU_VIRT_ARCH_ID 0x000
>> +
> [Severity: Medium]
> Do these definitions overload the generic "unimplemented" or "non-commercial"
> CPU IDs to uniquely identify the QEMU virt machine?
>
> According to the RISC-V Privileged Architecture specification, a value of 0
> in the mvendorid, marchid, and mimpid CSRs indicates that the ID is either
> unimplemented or represents a non-commercial implementation.
That's what virt machine exposes. We need actually what is being exposed
in virt machine
as it still remains one of the most commonly used platforms in RISC-V.
>> diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
>> index 00b84b28117a8..74acac54328e0 100644
>> --- a/drivers/perf/riscv_pmu_sbi.c
>> +++ b/drivers/perf/riscv_pmu_sbi.c
> [ ... ]
>
>> @@ -453,7 +454,42 @@ struct riscv_vendor_pmu_events {
>> .hw_event_map = _hw_event_map, .cache_event_map = _cache_event_map, \
>> .attrs_events = _attrs },
>>
> [ ... ]
>
>> static struct riscv_vendor_pmu_events pmu_vendor_events_table[] = {
>> + RISCV_VENDOR_PMU_EVENTS(QEMU_VIRT_VENDOR_ID, QEMU_VIRT_ARCH_ID, QEMU_VIRT_IMPL_ID,
>> + qemu_virt_hw_event_map, qemu_virt_cache_event_map,
>> + qemu_virt_event_group)
>> };
> [Severity: Medium]
> By binding the 0x000 ID tuple to QEMU's specific PMU event maps here, will
> the kernel mistakenly assume any generic core returning 0 is a QEMU virt
> machine?
>
> If that non-QEMU core implements PMU counter delegation (Ssccfg/Smcdeleg) but
> uses standard or different raw event encodings, this could unconditionally
> program QEMU's arbitrary raw event values into the core's PMU configuration
> registers, silently corrupting performance monitoring statistics.
N/A
^ permalink raw reply
* Re: [PATCH v6 16/21] RISC-V: perf: Add legacy event encodings via sysfs
From: Atish Patra @ 2026-06-20 0:35 UTC (permalink / raw)
To: sashiko-reviews; +Cc: conor+dt, devicetree, robh, linux-perf-users
In-Reply-To: <20260609062110.1D0C61F00893@smtp.kernel.org>
On 6/8/26 11:21 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The globally scoped pmu_ctr_list array is leaked if PMU counter discovery fails, compounded by a flawed error code cast.
> --
>
> commit 7d64e724e6eff197f10e62cdd709e07f0a5c9a7d
> Author: Atish Patra <atishp@rivosinc.com>
>
> RISC-V: perf: Add legacy event encodings via sysfs
>
> Define sysfs details for the legacy events so that any tool can
> parse these to understand the minimum set of legacy events
> supported by the platform. The sysfs entry will describe both event
> encoding and corresponding counter map so that an perf event can be
> programmed accordingly.
>
>> diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
>> index 4b4f151a0744d..00b84b28117a8 100644
>> --- a/drivers/perf/riscv_pmu_sbi.c
>> +++ b/drivers/perf/riscv_pmu_sbi.c
>> @@ -130,7 +130,20 @@ static struct attribute_group riscv_cdeleg_pmu_format_group = {
>> .attrs = riscv_cdeleg_pmu_formats_attr,
>> };
> [ ... ]
>
> [Severity: Medium]
> Does this code leak the pmu_ctr_list array if PMU counter discovery fails?
>
> In rvpmu_find_ctrs(), pmu_ctr_list is allocated via kcalloc(), but the function
> bails out without freeing it if num_sbi_counters or num_deleg_counters
> exceeds RISCV_MAX_COUNTERS:
>
> drivers/perf/riscv_pmu_sbi.c:rvpmu_find_ctrs() {
> ...
> pmu_ctr_list = kcalloc(RISCV_MAX_COUNTERS, sizeof(*pmu_ctr_list),
> GFP_KERNEL);
> if (!pmu_ctr_list)
> return -ENOMEM;
>
> if (riscv_pmu_cdeleg_available_boot())
> num_deleg_counters = rvpmu_deleg_find_ctrs();
>
> /* This is required for firmware counters even if the above is true */
> if (riscv_pmu_sbi_available_boot())
> num_sbi_counters = rvpmu_sbi_find_num_ctrs();
>
> if (num_sbi_counters > RISCV_MAX_COUNTERS || num_deleg_counters > RISCV_MAX_COUNTERS)
> return -ENOSPC;
> ...
> }
>
> Also, could num_sbi_counters inadvertently capture negative error codes?
>
> rvpmu_sbi_find_num_ctrs() returns an int which can be a negative error
> code. Storing it in the u32 num_sbi_counters implicitly casts
> it to a very large positive value.
>
> This triggers the RISCV_MAX_COUNTERS check and causes the function to abort
> with -ENOSPC, losing the original error.
>
> The caller rvpmu_device_probe() then fails to clean up the leaked globally
> scoped pmu_ctr_list pointer during its error handling:
>
> drivers/perf/riscv_pmu_sbi.c:rvpmu_device_probe() {
> ...
> num_counters = rvpmu_find_ctrs();
> if (num_counters < 0) {
> pr_err("SBI PMU extension doesn't provide any counters\n");
> goto out_free;
> }
> ...
> out_free:
> kfree(pmu);
> return ret;
> }
valid points but same as issue raised in PATCH11. Fixed in v7.
^ permalink raw reply
* Re: [PATCH v6 11/21] RISC-V: perf: Modify the counter discovery mechanism
From: Atish Patra @ 2026-06-20 0:31 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-perf-users, devicetree, conor+dt, robh
In-Reply-To: <20260609061751.59CD71F00898@smtp.kernel.org>
On 6/8/26 11:17 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Negative error codes returned by SBI are improperly cast to an unsigned integer, and systems with >64 counters are incorrectly aborted instead of clamped.
> --
>
> commit ea0f7376968d4f5943d9d279886b0b59e6dfd41c
> Author: Atish Patra <atishp@rivosinc.com>
> Subject: RISC-V: perf: Modify the counter discovery mechanism
>
> This commit modifies the PMU counter discovery mechanism to use a hybrid
> scheme. It utilizes counter delegation for hardware PMU counters while falling
> back to the SBI PMU for firmware counters, and updates logging appropriately.
>
>> diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
>> index 7f21c16003f04..57ab15beab3e1 100644
>> --- a/drivers/perf/riscv_pmu_sbi.c
>> +++ b/drivers/perf/riscv_pmu_sbi.c
> [ ... ]
>
>> @@ -1159,16 +1180,42 @@ static void rvpmu_ctr_stop(struct perf_event *event, unsigned long flag)
>> /* TODO: Counter delegation implementation */
>> }
>>
>> -static int rvpmu_find_num_ctrs(void)
>> +static int rvpmu_find_ctrs(void)
>> {
>> - return rvpmu_sbi_find_num_ctrs();
>> - /* TODO: Counter delegation implementation */
>> -}
>> + u32 num_sbi_counters = 0, num_deleg_counters = 0;
>> + u32 num_hw_ctr = 0, num_fw_ctr = 0, num_ctr = 0;
>> + /*
>> + * We don't know how many firmware counters are available. Just allocate
>> + * for maximum counters the driver can support. The default is 64 anyways.
>> + */
>> + pmu_ctr_list = kcalloc(RISCV_MAX_COUNTERS, sizeof(*pmu_ctr_list),
>> + GFP_KERNEL);
>> + if (!pmu_ctr_list)
>> + return -ENOMEM;
>>
>> -static int rvpmu_get_ctrinfo(int nctr, unsigned long *mask)
>> -{
>> - return rvpmu_sbi_get_ctrinfo(nctr, mask);
>> - /* TODO: Counter delegation implementation */
>> + if (riscv_pmu_cdeleg_available_boot())
>> + num_deleg_counters = rvpmu_deleg_find_ctrs();
>> +
>> + /* This is required for firmware counters even if the above is true */
>> + if (riscv_pmu_sbi_available_boot())
>> + num_sbi_counters = rvpmu_sbi_find_num_ctrs();
>> +
>> + if (num_sbi_counters > RISCV_MAX_COUNTERS || num_deleg_counters > RISCV_MAX_COUNTERS)
>> + return -ENOSPC;
> [Severity: High]
> Does this code inadvertently convert negative error codes from
> rvpmu_sbi_find_num_ctrs() into large unsigned values?
>
> Since num_sbi_counters is defined as a u32, if rvpmu_sbi_find_num_ctrs()
> returns an error code like -EOPNOTSUPP, it will wrap to a large positive
> integer, instantly satisfying the > RISCV_MAX_COUNTERS condition. This seems
> to mask the original error code.
>
> If the > RISCV_MAX_COUNTERS condition is met, the function returns -ENOSPC
> early. Can this leak the pmu_ctr_list allocated via kcalloc() just above,
> since there is no free before the return?
Valid. Fixed in v7.
> Additionally, could this introduce a regression for platforms legitimately
> exposing more than 64 counters? Previously, the driver appeared to safely
> clamp the usable count to 64 and continue, whereas this new logic seems to
> abort the driver probe entirely.
That's the current max counter supported by the driver. So it is fine to
abort
until such a case arises to avoid confusion.
> [ ... ]
>
^ permalink raw reply
* Re: [PATCH v6 10/21] RISC-V: perf: Restructure the SBI PMU code
From: Atish Patra @ 2026-06-20 0:17 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-perf-users, robh, conor+dt, devicetree
In-Reply-To: <20260609061839.70B9B1F00893@smtp.kernel.org>
On 6/8/26 11:18 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
> Pre-existing issues:
> - [Critical] NULL pointer dereference when tearing down the PMU on error paths in probe.
> - [High] Missing cleanup on error paths in rvpmu_device_probe() leads to Use-After-Free and resource leaks.
> - [High] Broadcasting `SCOUNTEREN` updates to all CPUs in `mm_cpumask` leaks counter access to userspace.
> --
>
> commit 497e3337649afaedc330eca31dba2dda185d6179
> Author: Atish Patra <atishp@rivosinc.com>
>
> RISC-V: perf: Restructure the SBI PMU code
>
> With Ssccfg/Smcdeleg, supervisor mode can program and access the hpmcounters
> and events directly, without the SBI PMU extension. The SBI PMU extension is
> still required for firmware counters. Restructure the existing SBI PMU code so
> the hpmcounter/event helpers can be shared between the SBI and the counter
> delegation paths that follow.
>
>> diff --git a/drivers/perf/riscv_pmu_sbi.c b/drivers/perf/riscv_pmu_sbi.c
>> index 385af5e6e6d0d..7f21c16003f04 100644
>> --- a/drivers/perf/riscv_pmu_sbi.c
>> +++ b/drivers/perf/riscv_pmu_sbi.c
> [ ... ]
>
>> @@ -1305,7 +1349,7 @@ static void riscv_pmu_destroy(struct riscv_pmu *pmu)
>> cpuhp_state_remove_instance(CPUHP_AP_PERF_RISCV_STARTING, &pmu->node);
>> }
> [Severity: Critical]
> This is a pre-existing issue, but does this code cause a NULL pointer
> dereference on error paths?
>
> In rvpmu_device_probe(), if an error occurs (such as perf_pmu_register(),
> pmu_sbi_snapshot_alloc(), or cpuhp_state_add_instance() failing), the error
> path jumps to out_unregister and calls riscv_pmu_destroy().
>
> Because cpuhp_state_add_instance() either was not called yet, or failed and
> did not hash the node, pmu->node.pprev is NULL. The function
> cpuhp_state_remove_instance() does not check if the node is unhashed and
> directly calls hlist_del(). This attempts to write to the previous node
> pointer, which crashes the kernel with a NULL pointer dereference.
Good catch. Fixed in v7.
I will have a separate patch for these fixes.
>> -static void pmu_sbi_event_init(struct perf_event *event)
>> +static void rvpmu_event_init(struct perf_event *event)
> [ ... ]
>
>> @@ -1347,14 +1391,14 @@ static void pmu_sbi_event_mapped(struct perf_event *event, struct mm_struct *mm)
>> * that it is possible to do so to avoid any race.
>> * And we must notify all cpus here because threads that currently run
>> * on other cpus will try to directly access the counter too without
>> - * calling pmu_sbi_ctr_start.
>> + * calling rvpmu_sbi_ctr_start.
>> */
>> if (event->hw.flags & PERF_EVENT_FLAG_USER_ACCESS)
>> on_each_cpu_mask(mm_cpumask(mm),
>> - pmu_sbi_set_scounteren, (void *)event, 1);
>> + rvpmu_set_scounteren, (void *)event, 1);
>> }
> [Severity: High]
> This isn't a bug introduced by this patch, but does broadcasting scounteren
> updates to all CPUs leak counter access to userspace?
>
> When a user-accessible event is mapped, rvpmu_set_scounteren is broadcast
> across all CPUs where the process has threads running. This enables the
> scounteren bit corresponding to event->hw.idx.
>
> However, event->hw.idx is a dynamic, per-CPU hardware counter index allocated
> only on the CPU where the event was scheduled. Broadcasting this index across
> all CPUs in the mask grants userspace access to that specific hardware
> counter slot on CPUs where the event is not running.
>
> Could a thread on another CPU read from that counter slot, potentially leaking
> the count of whatever unrelated perf event is active on that counter?
It's a valid concern. Though it is very difficult to exploit to leak the
counter values
through this approach as direct user access for programmable counters is
a niche use case to begin with.
I think we can toggle the scounteren bit in add/delete path to avoid the
leak. I will experiment
that idea and send a patch.
>> -static void pmu_sbi_event_unmapped(struct perf_event *event, struct mm_struct *mm)
>> +static void rvpmu_event_unmapped(struct perf_event *event, struct mm_struct *mm)
> [ ... ]
>
>> @@ -1419,7 +1463,7 @@ static const struct ctl_table sbi_pmu_sysctl_table[] = {
>> },
>> };
>>
>> -static int pmu_sbi_device_probe(struct platform_device *pdev)
>> +static int rvpmu_device_probe(struct platform_device *pdev)
>> {
>> struct riscv_pmu *pmu = NULL;
>> int ret = -ENODEV;
> [Severity: High]
> This is a pre-existing issue, but does this probe function correctly clean
> up resources on error paths?
>
> If initialization fails after perf_pmu_register(), the code jumps to the
> out_unregister label:
>
> out_unregister:
> riscv_pmu_destroy(pmu);
>
> out_free:
> kfree(pmu);
> return ret;
>
> However, riscv_pmu_destroy() does not call perf_pmu_unregister(). This leaves
> the core perf subsystem with a dangling pointer to the freed pmu structure,
> which can lead to a use-after-free when the perf core attempts to access it.
>
> Additionally, the per-CPU memory allocated for pmu->hw_events and the per-CPU
> interrupts requested via request_percpu_irq() are never freed on these error
> paths, resulting in resource leaks.
Nice catch. Fixed in v7.
^ permalink raw reply
* Re: [PATCH v6 21/21] TEST(do-not-upstream): fake qemu vendor JSON + mapfile entry for CounterIDMask path
From: Atish Patra @ 2026-06-20 0:04 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-perf-users, robh, devicetree, conor+dt
In-Reply-To: <20260609062023.0D53F1F00893@smtp.kernel.org>
On 6/8/26 11:20 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [Medium] This patch is explicitly marked 'TEST(do-not-upstream)' and adds fake testing events, which should not be merged into the mainline kernel.
>
> Pre-existing issues:
> - [High] Unconditional NULL pointer dereference in `get_cpuid`.
> --
>
> commit d0ac889c30ab39821cba995143de6c24abe2efcc
> Author: Atish Patra <atishp@meta.com>
>
> TEST(do-not-upstream): fake qemu vendor JSON + mapfile entry for CounterIDMask path
>
> This patch adds fake QEMU vendor JSON events and a corresponding mapfile entry
> for the CounterIDMask path to test PMU masking.
>
>> diff --git a/tools/perf/pmu-events/arch/riscv/mapfile.csv b/tools/perf/pmu-events/arch/riscv/mapfile.csv
>> index 87cfb0e0849fc..3533a8c0253f7 100644
>> --- a/tools/perf/pmu-events/arch/riscv/mapfile.csv
>> +++ b/tools/perf/pmu-events/arch/riscv/mapfile.csv
>> @@ -24,3 +24,4 @@
>> 0x602-0x3-0x0,v1,openhwgroup/cva6,core
>> 0x67e-0x80000000db0000[89]0-0x[[:xdigit:]]+,v1,starfive/dubhe-80,core
>> 0x31e-0x8000000000008a45-0x[[:xdigit:]]+,v1,andes/ax45,core
>> +0x0-0x0-0x0,v1,qemu/virt,core
> [Severity: Medium]
> Since this patch is explicitly marked "TEST(do-not-upstream)" and adds fake
> PMU events strictly for testing, should it be merged into the mainline
> kernel?
>
> Merging this will populate the perf tool's PMU event tables with non-existent
> hardware events.
N/A. this patch is just an example and marked explicitly not for merging.
>> diff --git a/tools/perf/pmu-events/arch/riscv/qemu/virt/events.json b/tools/perf/pmu-events/arch/riscv/qemu/virt/events.json
>> new file mode 100644
>> index 0000000000000..294c4ed645f67
>> --- /dev/null
>> +++ b/tools/perf/pmu-events/arch/riscv/qemu/virt/events.json
>> @@ -0,0 +1,26 @@
>> +[
>> + {
>> + "EventName": "fake-json-any",
> [ ... ]
>
> While reviewing this, I also noticed a pre-existing issue in
> tools/perf/arch/riscv/util/header.c:
>
> tools/perf/arch/riscv/util/header.c:get_cpuid() {
> ...
> char *cpuid = _get_cpuid();
> int ret = 0;
>
> if (sz < strlen(cpuid)) {
> ret = -EINVAL;
> goto free;
> }
> ...
> }
>
> [Severity: High]
> This is a pre-existing issue, but can get_cpuid() cause a segmentation fault
> here? If _get_cpuid() returns NULL (for example, if /proc/cpuinfo is missing
> or lacks specific fields in a container or QEMU without proper emulation), it
> appears the unconditional call to strlen(cpuid) will dereference that NULL
> pointer.
Yes. I will send a separate patch series for these fixes.
^ permalink raw reply
* Re: [PATCH v6 09/21] dt-bindings: riscv: add Counter delegation ISA extensions description
From: Atish Patra @ 2026-06-19 23:49 UTC (permalink / raw)
To: sashiko-reviews; +Cc: conor+dt, linux-perf-users, robh, devicetree
In-Reply-To: <20260609061239.160901F00893@smtp.kernel.org>
On 6/8/26 11:12 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Medium] Incorrect dependency for smcdeleg: depends on sscsrind instead of smcsrind.
> - [Medium] The ssccfg extension is over-constrained by strictly requiring smcntrpmf.
> - [Low] Contradictory description for the smcdeleg extension.
> --
>
> commit 53fb261803de68f9caaa685d63588c9cac6c0f5c
> Author: Atish Patra <atishp@rivosinc.com>
>
> dt-bindings: riscv: add Counter delegation ISA extensions description
>
> This commit adds device tree binding descriptions for the Smcdeleg and
> Ssccfg RISC-V ISA extensions. It also specifies the conditional dependencies
> required for these extensions to pass schema validation.
>
>> diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
>> index ece3edccee42c..2845e8e2999a7 100644
>> --- a/Documentation/devicetree/bindings/riscv/extensions.yaml
>> +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
>> @@ -181,6 +181,13 @@ properties:
>> changes to interrupts as frozen at commit ccbddab ("Merge pull
>> request #42 from riscv/jhauser-2023-RC4") of riscv-aia.
>>
>> + - const: smcdeleg
>> + description: |
>> + The standard Smcdeleg supervisor-level extension for the machine mode
> [Severity: Low]
> This isn't a bug, but should this say machine-level instead of
> supervisor-level? Since it has an Sm prefix and is meant for machine mode,
> calling it a supervisor-level extension might be a copy-paste typo.
In this case, it is not true. Quoting the RISC-V spec.
For a RISC-V hardware platform, Smcdeleg and Ssccfg must always be
implemented in tandem.
https://docs.riscv.org/reference/isa/v20260120/priv/smcdeleg.html
>> + to delegate the hpmcounters to supervisor mode so that they are
>> + directly accessible in the supervisor mode as ratified in the
>> + 20240213 version of the privileged ISA specification.
> [ ... ]
>
>> @@ -1135,6 +1150,36 @@ properties:
>> allOf:
>> - const: zilsd
>> - const: zca
>> + # Smcdeleg depends on Sscsrind, Zihpm, Zicntr
>> + - if:
>> + contains:
>> + const: smcdeleg
>> + then:
>> + allOf:
>> + - contains:
>> + const: sscsrind
> [Severity: Medium]
> Does smcdeleg actually depend on sscsrind here?
>
> Since smcdeleg is a machine-mode extension that uses M-mode indirect CSRs to
> delegate counters, shouldn't it depend on the smcsrind extension instead of
> sscsrind? If a valid hardware configuration implements smcsrind and smcdeleg
> without sscsrind, would this schema incorrectly reject it?
Yes. Answered above.
>> + - contains:
>> + const: zihpm
>> + - contains:
>> + const: zicntr
>> + # Ssccfg depends on Smcdeleg, Sscsrind, Zihpm, Zicntr, Sscofpmf, Smcntrpmf
>> + - if:
>> + contains:
>> + const: ssccfg
>> + then:
>> + allOf:
>> + - contains:
>> + const: smcdeleg
>> + - contains:
>> + const: sscsrind
>> + - contains:
>> + const: sscofpmf
>> + - contains:
>> + const: smcntrpmf
> [Severity: Medium]
> Is smcntrpmf a strict requirement for ssccfg?
>
> The ssccfg extension provides supervisor-mode access to delegated counters,
> but the kernel implementation seems to only conditionally expose
> cyclecfg/instretcfg CSRs if smcntrpmf is present. If ssccfg can exist
> independently, will requiring smcntrpmf in this binding incorrectly fail
> validation for valid hardware that omits it?
While it is ideal to have that, we can relax that dependency.
Smcntrpmf is strictly dependant on Smcdeleg/Ssccfg extensions
implementation but not the other way around.
>> + - contains:
>> + const: zihpm
>> + - contains:
>> + const: zicntr
^ permalink raw reply
* Re: [PATCH v6 08/21] RISC-V: Add Ssccfg/Smcdeleg ISA extension definition and parsing
From: Atish Patra @ 2026-06-19 23:44 UTC (permalink / raw)
To: sashiko-reviews; +Cc: robh, linux-perf-users, conor+dt, devicetree, linux-riscv
In-Reply-To: <20260609061405.1943F1F00893@smtp.kernel.org>
On 6/8/26 11:14 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Supervisor extension Ssccfg incorrectly depends on Machine extension Smcdeleg, breaking virtualization.
> - [Medium] The Smcdeleg extension validation incorrectly checks for the Supervisor-level CSR indirection extension (SSCSRIND) instead of the Machine-level one (SMCSRIND).
> --
>
> commit 53e139b078ec8b41f58b09bfd3d37e7132a15d45
> Author: Atish Patra <atishp@rivosinc.com>
>
> RISC-V: Add Ssccfg/Smcdeleg ISA extension definition and parsing
>
> This commit adds definitions and parsing logic for the RISC-V Ssccfg
> and Smcdeleg ISA extensions. It introduces validation functions for
> both extensions and adds them to the riscv_isa_ext array.
+linux-riscv
Sashiko probably has a bug where it did not determine correctly which
mailing lists to cc and left linux-riscv.
>> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
>> index 1452521d740a6..1fe647e035157 100644
>> --- a/arch/riscv/kernel/cpufeature.c
>> +++ b/arch/riscv/kernel/cpufeature.c
>> @@ -330,6 +330,27 @@ static const unsigned int riscv_a_exts[] = {
>> RISCV_ISA_EXT_ZKNE, \
>> RISCV_ISA_EXT_ZKNH
>>
>> +static int riscv_ext_smcdeleg_validate(const struct riscv_isa_ext_data *data,
>> + const unsigned long *isa_bitmap)
>> +{
>> + if (__riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_SSCSRIND) &&
> ^^^^^^^^^^^^^^^^^^^^^^
> [Severity: Medium]
> Since Smcdeleg is a Machine-mode extension, shouldn't its validation depend on
> the Machine-level CSR indirect access extension (SMCSRIND) rather than the
> Supervisor-level extension (SSCSRIND)?
The reason for this dependency is that both Smcdeleg and SSCSRIND are
required for using counter
delegation usage in S-mode.
> If the kernel runs in M-mode, or on hardware that implements SMCSRIND but not
> SSCSRIND, this validation might incorrectly fail.
Not a valid use.
>> + __riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_ZIHPM) &&
>> + __riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_ZICNTR))
>> + return 0;
>> +
>> + return -EPROBE_DEFER;
>> +}
>> +
>> +static int riscv_ext_ssccfg_validate(const struct riscv_isa_ext_data *data,
>> + const unsigned long *isa_bitmap)
>> +{
>> + if (!riscv_ext_smcdeleg_validate(data, isa_bitmap) &&
>> + __riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_SMCDELEG))
> [Severity: High]
> Does this break supervisor counter delegation in virtualized environments?
>
> Smcdeleg is a Machine-mode extension, and hypervisors typically do not expose
> M-mode extensions to guest VMs. A guest kernel running in VS-mode won't see
> Smcdeleg in its ISA string, causing this validation to unconditionally fail
> and return -EPROBE_DEFER.
>
> Should Ssccfg validation only depend on S-mode visible extensions?
No. The spec is not designed that way. Counter delegation is not
available in VS-mode.
Both comments are N/A
>> + return 0;
>> +
>> + return -EPROBE_DEFER;
>> +}
^ permalink raw reply
* Re: [PATCH v6 06/21] dt-bindings: riscv: add Smcntrpmf ISA extension description
From: Atish Patra @ 2026-06-19 23:40 UTC (permalink / raw)
To: sashiko-reviews; +Cc: robh, conor+dt, devicetree, linux-perf-users
In-Reply-To: <20260609060915.6C7461F00893@smtp.kernel.org>
On 6/8/26 11:09 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Low] The `smcntrpmf` extension is inserted in the wrong alphabetical order.
> - [Low] The description incorrectly refers to `Smcntrpmf` as a 'supervisor-level' extension.
> --
>
> commit 9fd028f70a33e1a78db2f635eed4200c33b6b90d
> Author: Atish Patra <atishp@rivosinc.com>
>
> dt-bindings: riscv: add Smcntrpmf ISA extension description
>
> Add the description for the Smcntrpmf ISA extension.
>
>> diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
>> index 4be557dc215de..ece3edccee42c 100644
>> --- a/Documentation/devicetree/bindings/riscv/extensions.yaml
>> +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
>> @@ -189,6 +189,12 @@ properties:
>> mechanism in M-mode as ratified in the 20240326 version of the
>> privileged ISA specification.
>>
>> + - const: smcntrpmf
> [Severity: Low]
> Is this inserted in the correct alphabetical order?
>
> The multi-letter extensions list in this file explicitly requires entries to be
> sorted alphanumerically. Alphabetically, smcntrpmf ("c-n") should come before
> the preceding extension (smcsrind, "c-s").
>
>> + description: |
>> + The standard Smcntrpmf supervisor-level extension for the machine mode
> [Severity: Low]
> Is "supervisor-level" accurate here?
>
> In the RISC-V ISA naming convention, the "Sm" prefix denotes a standard
> machine-level extension, while "Ss" denotes a supervisor-level extension.
>
> Calling it a "supervisor-level extension for the machine mode" seems
> contradictory. Could this be a copy-paste error?
>
>> + to enable privilege mode filtering for cycle and instret counters as
>> + ratified in the 20240326 version of the privileged ISA specification.
>> +
>> - const: smmpm
>> description: |
>> The standard Smmpm extension for M-mode pointer masking as
Fixed in v7.
^ permalink raw reply
* Re: [PATCH v6 03/21] dt-bindings: riscv: add Sxcsrind ISA extension description
From: Atish Patra @ 2026-06-19 23:39 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-perf-users, conor+dt, robh, devicetree
In-Reply-To: <20260609060921.547691F00893@smtp.kernel.org>
On 6/8/26 11:09 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Low] The `sscsrind` extension is inserted out of alphanumeric order, violating the explicit formatting rule in the binding file.
> - [Low] The description for `smcsrind` contains a copy-paste error incorrectly labeling it as a 'supervisor-level' extension, and both `smcsrind` and `sscsrind` contain a grammatical typo.
> --
>
> commit a4e1326a37463e042ca0a72ff72d93f2be1d22f8
> Author: Atish Patra <atishp@rivosinc.com>
>
> dt-bindings: riscv: add Sxcsrind ISA extension description
>
> Add the S[m|s]csrind ISA extension description.
>
>> diff --git a/Documentation/devicetree/bindings/riscv/extensions.yaml b/Documentation/devicetree/bindings/riscv/extensions.yaml
>> index 2b0a8a93bb214..4be557dc215de 100644
>> --- a/Documentation/devicetree/bindings/riscv/extensions.yaml
>> +++ b/Documentation/devicetree/bindings/riscv/extensions.yaml
>> @@ -181,6 +181,14 @@ properties:
>> changes to interrupts as frozen at commit ccbddab ("Merge pull
>> request #42 from riscv/jhauser-2023-RC4") of riscv-aia.
>>
>> + - const: smcsrind
>> + description: |
>> + The standard Smcsrind supervisor-level extension extends the
>> + indirect CSR access mechanism defined by the Smaia extension. This
> [Severity: Low]
> Is this description accurate for smcsrind? The m in Smcsrind indicates
> it is a machine-level extension, rather than a supervisor-level extension.
>
>> + extension allows other ISA extension to use indirect CSR access
> [Severity: Low]
> This isn't a bug, but there is a typo here. Should "ISA extension" be
> pluralized to "ISA extensions"?
>
>> + mechanism in M-mode as ratified in the 20240326 version of the
>> + privileged ISA specification.
>> +
> [ ... ]
>
>> @@ -199,6 +207,14 @@ properties:
>> added by other RISC-V extensions in H/S/VS/U/VU modes and as
>> ratified at commit a28bfae (Ratified (#7)) of riscv-state-enable.
>>
>> + - const: sscsrind
>> + description: |
>> + The standard Sscsrind supervisor-level extension extends the
>> + indirect CSR access mechanism defined by the Ssaia extension. This
>> + extension allows other ISA extension to use indirect CSR access
> [Severity: Low]
> This isn't a bug, but there is the same typo here. Should "ISA extension" be
> pluralized to "ISA extensions"?
>
>> + mechanism in S-mode as ratified in the 20240326 version of the
>> + privileged ISA specification.
>> +
>> - const: ssaia
> [Severity: Low]
> Should sscsrind be placed after ssaia rather than before it? The
> extensions in this file are expected to be sorted alphanumerically.
Fixed the typos in v7.
>> description: |
>> The standard Ssaia supervisor-level extension for the advanced
^ permalink raw reply
* Re: [PATCH v6 04/21] RISC-V: Define indirect CSR access helpers
From: Atish Patra @ 2026-06-19 23:39 UTC (permalink / raw)
To: sashiko-reviews; +Cc: robh, devicetree, linux-perf-users, conor+dt
In-Reply-To: <20260609061511.276E31F00893@smtp.kernel.org>
On 6/8/26 11:15 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Unsafe statement-expression macro definitions causing delayed evaluation and variable shadowing hazards.
> --
>
> commit 572b7555547c1b5bc07701482ba67d379f6a2177
> Author: Atish Patra <atishp@rivosinc.com>
>
> RISC-V: Define indirect CSR access helpers
>
> This commit introduces helper macros to read, write, and perform WARL
> operations on indirect CSRs. The macros use statement-expressions to handle
> the multiple instructions required while disabling interrupts.
>
>> diff --git a/arch/riscv/include/asm/csr_ind.h b/arch/riscv/include/asm/csr_ind.h
>> new file mode 100644
>> index 0000000000000..6fd7d44dc640a
>> --- /dev/null
>> +++ b/arch/riscv/include/asm/csr_ind.h
>> @@ -0,0 +1,44 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * Copyright (C) 2024 Rivos Inc.
>> + */
>> +
>> +#ifndef _ASM_RISCV_CSR_IND_H
>> +#define _ASM_RISCV_CSR_IND_H
>> +
>> +#include <linux/irqflags.h>
>> +
>> +#include <asm/csr.h>
>> +
>> +#define csr_ind_read(iregcsr, iselbase, iseloff) ({ \
>> + unsigned long __value = 0; \
>> + unsigned long __flags; \
> [Severity: Medium]
> Will these generic variable names (__value, __flags) cause shadowing issues if
> the caller passes in arguments with the same names?
>
> Evaluating macro arguments into unique local variables before entering the
> IRQ-disabled region would avoid this regression.
>
>> + local_irq_save(__flags); \
>> + csr_write(CSR_ISELECT, (iselbase) + (iseloff)); \
>> + __value = csr_read(iregcsr); \
>> + local_irq_restore(__flags); \
>> + __value; \
>> +})
>> +
>> +#define csr_ind_write(iregcsr, iselbase, iseloff, value) ({ \
>> + unsigned long __flags; \
>> + local_irq_save(__flags); \
>> + csr_write(CSR_ISELECT, (iselbase) + (iseloff)); \
>> + csr_write(iregcsr, (value)); \
> [Severity: Medium]
> Since value is evaluated here after CSR_ISELECT has been programmed, what
> happens if a caller passes an expression that accesses another indirect CSR,
> such as a nested csr_ind_read(), as the value argument?
>
> It appears the nested call would overwrite CSR_ISELECT, causing the write to
> silently corrupt the wrong hardware register. Evaluating arguments into local
> variables before disabling interrupts would prevent this, and also avoid the
> risk of evaluating expressions with side effects while interrupts are disabled.
>
>> + local_irq_restore(__flags); \
>> +})
>> +
>> +#define csr_ind_warl(iregcsr, iselbase, iseloff, warl_val) ({ \
>> + unsigned long __old_val = 0, __value = 0; \
>> + unsigned long __flags; \
> [Severity: Medium]
> Do these generic local variables risk shadowing caller arguments?
>
>> + local_irq_save(__flags); \
>> + csr_write(CSR_ISELECT, (iselbase) + (iseloff)); \
>> + __old_val = csr_read(iregcsr); \
>> + csr_write(iregcsr, (warl_val)); \
> [Severity: Medium]
> Is there a risk that evaluating warl_val here could overwrite CSR_ISELECT if
> the caller provides an expression that performs another indirect CSR access?
>
>> + __value = csr_read(iregcsr); \
>> + csr_write(iregcsr, __old_val); \
>> + local_irq_restore(__flags); \
>> + __value; \
>> +})
>> +
>> +#endif
Given the callers, I don't think the possibilities raised by Sashiko can
actually happen.
It's a macro. So the new callers should ensure they are invoking it with
proper argument.
N/A.
^ permalink raw reply
* Re: [PATCH] arm64: dts: qcom: sdm845-oneplus: add panel rails to simplefb
From: Sam Day @ 2026-06-19 23:15 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <c4a70b1e-ea48-49d2-afa6-639b73983729@oss.qualcomm.com>
Hello Konrad,
On Saturday, 20 June 2026 at 1:11 AM, Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> wrote:
> On 6/16/26 7:27 AM, Sam Day via B4 Relay wrote:
> > From: Sam Day <me@samcday.com>
> >
> > These regulators are marked regulator-boot-on, but that doesn't
> > guarantee they'll stay alive as long as the simplefb does. Adding the
> > explicit supplies ensures that booting with MDSS disabled doesn't
> > switch the panel off 30 seconds after boot.
>
> Why would you boot without MDSS if you want the panel to function?
For my particular use-case [1] I'm trying to avoid taking over the display
hardware entirely, and use simplefb until kexecing into the real kernel. In
this case the user might halt in a pre-boot menu UI for a while.
I can also imagine this being similarly useful for "recovery kernel" use-cases
where again, it may not be desirable to assume control of MDSS/MDP/DSI blocks.
[1]: https://github.com/samcday/pocketboot
Regards,
-Sam
>
> Konrad
>
^ permalink raw reply
* [PATCH] arm64: dts: broadcom: bcm2712: Remove non-functional EL2 virtual timer
From: Daniel Drake @ 2026-06-19 20:48 UTC (permalink / raw)
To: maz, robh, krzk+dt, conor+dt, florian.fainelli,
bcm-kernel-feedback-list
Cc: devicetree, linux-rpi-kernel, linux-arm-kernel, m.szyprowski,
andrea.porta, Daniel Drake
Commit d87773de9efe1 ("clocksource/drivers/arm_arch_timer: Default to
EL2 virtual timer when running VHE") causes boot to hang on
Raspberry Pi 5. The newly-selected EL2 virtual timer does not generate
any interrupts, even though the GIC_DIST_ENABLE_SET flag has been
confirmed set via readback.
The reasons for this failure are unknown, however it is likely that
this timer was never tested. Raspberry Pi's original devicetree did
not include this timer interrupt; it was only introduced via a
suggestion[1] made in code review as part of the upstreaming process.
(Current RPi firmware versions do include this timer, but only because
they rebased on top of the upstreamed devicetree starting with
Linux 6.12)
Until more is known about this non-firing timer interrupt, remove
the devicetree entry to enable RPi5 devices to boot.
[1] https://lore.kernel.org/all/12363be5b11c752b7155cc0c416fdfd2@kernel.org/
Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Closes: https://lore.kernel.org/all/ea15cce1-b393-43f6-8d58-3d6f90f0c0cd@samsung.com/
Signed-off-by: Daniel Drake <dan@reactivated.net>
---
arch/arm64/boot/dts/broadcom/bcm2712.dtsi | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/arm64/boot/dts/broadcom/bcm2712.dtsi b/arch/arm64/boot/dts/broadcom/bcm2712.dtsi
index 761c59d90ffc..09ff5e9959d3 100644
--- a/arch/arm64/boot/dts/broadcom/bcm2712.dtsi
+++ b/arch/arm64/boot/dts/broadcom/bcm2712.dtsi
@@ -678,8 +678,6 @@ IRQ_TYPE_LEVEL_LOW)>,
<GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) |
IRQ_TYPE_LEVEL_LOW)>,
<GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) |
- IRQ_TYPE_LEVEL_LOW)>,
- <GIC_PPI 12 (GIC_CPU_MASK_SIMPLE(4) |
IRQ_TYPE_LEVEL_LOW)>;
};
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v4] powerpc: dts: Build devicetrees of enabled platforms
From: Madhavan Srinivasan @ 2026-06-19 21:59 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), J. Neuschäfer
Cc: devicetree, linuxppc-dev, linux-kernel, Geert Uytterhoeven
In-Reply-To: <20260315-mpc83xx-dtb-v4-1-243849be4280@posteo.net>
On Sun, 15 Mar 2026 22:45:13 +0100, J. Neuschäfer wrote:
> Follow the same approach as other architectures such as Arm or RISC-V,
> and build devicetrees based on platforms selected in Kconfig. This makes
> it unnecessary to use CONFIG_OF_ALL_DTBS on PowerPC in order to build
> DTB files.
>
> This makes it easier to use other build and test infrastructure such as
> `make dtbs_check`, and is a first step towards generating FIT images
> that include all the relevant DTBs with `make image.fit`.
>
> [...]
Applied to powerpc/next.
[1/1] powerpc: dts: Build devicetrees of enabled platforms
https://git.kernel.org/powerpc/c/2442f10c6eae63392452ab84a813dbc1f87c724d
cheers
^ permalink raw reply
* Re: [PATCH 0/2] Add missing model properties to PowerPC DTs
From: Madhavan Srinivasan @ 2026-06-19 21:59 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Michael Ellerman,
Nicholas Piggin, Christophe Leroy (CS GROUP), J. Neuschäfer
Cc: devicetree, linuxppc-dev, linux-kernel
In-Reply-To: <20260313-ppc-model-v1-0-bf19b3d1b65d@posteo.net>
On Fri, 13 Mar 2026 11:14:22 +0100, J. Neuschäfer wrote:
> The devicetree spec requires a /model property, but many of the powerpc
> devicetrees don't have one. This series is an attempt to fix it.
>
> One practical advantage is that scripts/make_fit.py will no longer throw
> an error when invoked with the powerpc DTs.
>
>
> [...]
Applied to powerpc/next.
[1/2] powerpc: dts: mpc8315erdb: Add missing model property
https://git.kernel.org/powerpc/c/d16cb0a9a42f7478c039ecaeba241872bc0b567a
[2/2] powerpc: dts: Add missing model properties
https://git.kernel.org/powerpc/c/ec07a2f1d337dd5a816431724a876026296369a8
cheers
^ permalink raw reply
* Re: [PATCH v3 2/7] gpio: regmap: add gpio_regmap_get_gpiochip() accessor
From: Linus Walleij @ 2026-06-19 21:08 UTC (permalink / raw)
To: Michael Walle
Cc: Bartosz Golaszewski, Andy Shevchenko, robh@kernel.org,
krzk+dt@kernel.org, conor+dt@kernel.org, afaerber@suse.com,
wbg@kernel.org, mathieu.dubois-briand@bootlin.com,
lars@metafoo.de, Michael.Hennerich@analog.com, jic23@kernel.org,
nuno.sa@analog.com, andy@kernel.org, dlechner@baylibre.com,
TY_Chang[張子逸], linux-gpio@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-realtek-soc@lists.infradead.org, linux-iio@vger.kernel.org,
CY_Huang[黃鉦晏],
Stanley Chang[昌育德],
James Tai [戴志峰],
Yu-Chun Lin [林祐君]
In-Reply-To: <DJ3QVMZ6XLW9.1M9W541O92QWJ@kernel.org>
On Mon, Jun 8, 2026 at 4:41 PM Michael Walle <mwalle@kernel.org> wrote:
> >>> Without an accessor like gpio_regmap_get_gpiochip(), we cannot retrieve the
> >>> gpio_chip instantiated inside gpio-regmap.c to fulfill these requirements in our
> >>> map() function.
>
> Why is gpiochip_irq_reqres() called in the first place? Isn't that
> only called if the irq handling is set up via gc->irq.chip and not
> via gpiochip_irqchip_add_domain() like in gpio-regmap?
Not really, the gpiochip_irq_reqres() is called to mark that a
GPIO line is used for IRQ, so the gpiolib cannot turn this
GPIO into an output line, gpiod_direction_out() will fail
on lines used for IRQ. So it's a failsafe.
You can live without it of course, but then you don't get
this failsafe.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH] arm64: dts: broadcom: bcm2712: Remove non-functional EL2 virtual timer
From: Marek Szyprowski @ 2026-06-19 21:04 UTC (permalink / raw)
To: Daniel Drake, maz, robh, krzk+dt, conor+dt, florian.fainelli,
bcm-kernel-feedback-list
Cc: devicetree, linux-rpi-kernel, linux-arm-kernel, andrea.porta
In-Reply-To: <20260619204832.586079-1-dan@reactivated.net>
On 19.06.2026 22:48, Daniel Drake wrote:
> Commit d87773de9efe1 ("clocksource/drivers/arm_arch_timer: Default to
> EL2 virtual timer when running VHE") causes boot to hang on
> Raspberry Pi 5. The newly-selected EL2 virtual timer does not generate
> any interrupts, even though the GIC_DIST_ENABLE_SET flag has been
> confirmed set via readback.
>
> The reasons for this failure are unknown, however it is likely that
> this timer was never tested. Raspberry Pi's original devicetree did
> not include this timer interrupt; it was only introduced via a
> suggestion[1] made in code review as part of the upstreaming process.
> (Current RPi firmware versions do include this timer, but only because
> they rebased on top of the upstreamed devicetree starting with
> Linux 6.12)
>
> Until more is known about this non-firing timer interrupt, remove
> the devicetree entry to enable RPi5 devices to boot.
>
> [1] https://lore.kernel.org/all/12363be5b11c752b7155cc0c416fdfd2@kernel.org/
>
> Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Closes: https://lore.kernel.org/all/ea15cce1-b393-43f6-8d58-3d6f90f0c0cd@samsung.com/
> Signed-off-by: Daniel Drake <dan@reactivated.net>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> arch/arm64/boot/dts/broadcom/bcm2712.dtsi | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/broadcom/bcm2712.dtsi b/arch/arm64/boot/dts/broadcom/bcm2712.dtsi
> index 761c59d90ffc..09ff5e9959d3 100644
> --- a/arch/arm64/boot/dts/broadcom/bcm2712.dtsi
> +++ b/arch/arm64/boot/dts/broadcom/bcm2712.dtsi
> @@ -678,8 +678,6 @@ IRQ_TYPE_LEVEL_LOW)>,
> <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) |
> IRQ_TYPE_LEVEL_LOW)>,
> <GIC_PPI 10 (GIC_CPU_MASK_SIMPLE(4) |
> - IRQ_TYPE_LEVEL_LOW)>,
> - <GIC_PPI 12 (GIC_CPU_MASK_SIMPLE(4) |
> IRQ_TYPE_LEVEL_LOW)>;
> };
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply
* Re: [PATCH v2 2/3] powercap: qcom: Add SPEL powercap driver
From: sashiko-bot @ 2026-06-19 20:50 UTC (permalink / raw)
To: Manaf Meethalavalappu Pallikunhi; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260620-qcom_spel_driver_upstream-v2-2-a3ee6837c18f@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The devm-allocated `sp` structure is dereferenced in the asynchronous `spel_release_zone` callback, causing a Use-After-Free when the driver is unbound.
- [High] Manual kfree of the dynamically allocated `sp->domains` array leads to a Use-After-Free when devres attempts to run `mutex_destroy` on its embedded devm-managed mutexes.
- [Medium] The driver incorrectly exposes phantom powercap constraints for domains lacking hardware support, breaking UAPI expectations.
- [Medium] The formula used to calculate `sp->time_unit` contradicts the driver's own documentation and standard scaling logic, resulting in incorrect time unit conversion.
--
commit 53ea37bb963bb111e31263f8aaab73464a037f7d
Author: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
Subject: powercap: qcom: Add SPEL powercap driver
This commit adds support for the Qualcomm SoC Power and Electrical Limits
(SPEL) hardware. It enables power limits to be enforced and monitored on
Qualcomm SoCs across multiple domains using the powercap framework.
> diff --git a/drivers/powercap/qcom_spel.c b/drivers/powercap/qcom_spel.c
> new file mode 100644
> index 0000000000000..4dd91cf36ccca
> --- /dev/null
> +++ b/drivers/powercap/qcom_spel.c
[ ... ]
> +static int spel_release_zone(struct powercap_zone *power_zone)
> +{
> + struct spel_domain *sd = power_zone_to_spel_domain(power_zone);
> + struct spel_system *sp = sd->sp;
> +
> + /* Free the domains array when the last zone (SYS domain) is released */
> + if (sd->id == SPEL_DOMAIN_SYS)
> + kfree(sp->domains);
[Severity: High]
If the device is unbound while a sysfs file is held open, devres will free
the devm_kzalloc allocated sp structure immediately. When the user later
closes the sysfs file, the powercap framework will asynchronously call this
release function. Does accessing sp->domains here cause a use-after-free
read?
[Severity: High]
Also, earlier in spel_init_system(), devm_mutex_init() is called on sd->lock
which is embedded inside this dynamically allocated sp->domains array:
spel_init_system() {
...
ret = devm_mutex_init(dev, &sd->lock);
If sp->domains is manually freed here with kfree(), will devres trigger a
use-after-free write when it later runs mutex_destroy() on the embedded
locks during device removal or probe failure?
[ ... ]
> +static void spel_init_domains(struct spel_system *sp)
> +{
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(domain_info); i++) {
> + struct spel_domain *sd = &sp->domains[i];
> +
> + sd->sp = sp;
> + snprintf(sd->name, SPEL_DOMAIN_NAME_LENGTH, "%s",
> + domain_info[i].name);
> + sd->id = i;
> + sd->status_reg = sp->node_base + domain_info[i].offset;
> +
> + /* PL1 is always supported (required for powercap registration) */
> + sp->limits[i] = BIT(POWER_LIMIT1);
[Severity: Medium]
Is it strictly required for all domains to have PL1 enabled?
The powercap framework supports zones with zero constraints for purely
monitoring purposes. Since hardware limits are only defined for the SYS and
SOC domains in the constraints array, unconditionally setting PL1 for other
domains (like CL0 or IGPU) causes constraint sysfs files to be created that
will always fail with an error when read or written. Does this expose a
broken UAPI?
> + }
> +}
> +
> +static void spel_update_unit(struct spel_system *sp)
> +{
> + u32 value, shift;
> +
> + /* Read power_unit and time_unit from offset 0x0 */
> + value = readl(sp->config_base);
> +
> + /*
> + * Unit calculation: 1 / (2^shift)
> + * Masks limit: TIME_UNIT (4 bits, max 15), POWER_UNIT (3 bits, max 7).
> + */
> + shift = FIELD_GET(POWER_UNIT_MASK, value);
> + sp->power_unit = 1000000 / (1 << shift);
> +
> + shift = FIELD_GET(TIME_UNIT_MASK, value);
> + /*
> + * Time window in register is in milliseconds.
> + */
> + sp->time_unit = 1000 * (1 << shift);
[Severity: Medium]
The comment above states the unit calculation is 1 / (2^shift).
For the power unit, the driver correctly divides. However, for the time unit
it multiplies by the shift value instead. Should this be a division
(1000 / (1 << shift)) to correctly scale the time unit into microseconds?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260620-qcom_spel_driver_upstream-v2-0-a3ee6837c18f@oss.qualcomm.com?part=2
^ permalink raw reply
* Re: [PATCH 09/12] rtc: rzn1: Use temporary variable for struct device
From: Wolfram Sang @ 2026-06-19 20:48 UTC (permalink / raw)
To: Lad, Prabhakar
Cc: Miquel Raynal, Alexandre Belloni, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
Magnus Damm, linux-rtc, linux-renesas-soc, devicetree,
linux-kernel, Biju Das, Fabrizio Castro, Lad Prabhakar
In-Reply-To: <CA+V-a8t-phAuaH5g_3Nt=2d=KPw2QqA6r1dqOZv9k-Xhp5q2Zw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1657 bytes --]
Hi Prabhakar,
> > > Could userspace still interact with the RTC during this window via ioctls
> > > or sysfs, potentially causing synchronous bus faults since the hardware is
> > > already suspended? Should teardown be bound using
> > > devm_add_action_or_reset() to guarantee correct reverse execution order?
> >
> > [wsa] Or maybe not use devm then?
> >
> Maybe just add two `devm_add_action_or_reset()` calls: one to clear
> interrupts and another to call pm_runtime_put()?
Why two? I wouldn't think it matters much if the two are separated. Main
problem seems to me the disabled clocks because of pm_runtime_put()? But
maybe I am overlooking sth?
> > > [Severity: High]
> > > This is a pre-existing issue, but does using pm_runtime_put() here (and in
> > > rzn1_rtc_remove) leave the device powered on indefinitely?
> > >
> > > Since devm_pm_runtime_enable(dev) schedules pm_runtime_disable() to
> > > execute during devres cleanup, calling pm_runtime_put() only queues an
> > > asynchronous idle check. The immediate return triggers devres cleanup,
> > > which executes a barrier that explicitly cancels pending async operations.
> > >
> > > Should this use pm_runtime_put_sync() instead to ensure the device is
> > > synchronously suspended before teardown?
> > >
> Although there were some patches accepted for similar kind of issue
> reported by Sashiko, do you think I should switch to
> pm_runtime_put_sync() (Ive not seen any issues)
I am not a PM expert, so I can't guide you. The report from Sashiko
sounds reasonable to me. But you'd have to look up the code path to
verify the reasoning. Or ask an PM expert.
Happy hacking,
Wolfram
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox