* [PATCH 3/5] riscv: dts: spacemit: add 24c02 eeprom on K3 Pico-ITX board
From: Aurelien Jarno @ 2026-06-20 6:22 UTC (permalink / raw)
To: linux-kernel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Yixun Lan, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti
Cc: Aurelien Jarno,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list:RISC-V ARCHITECTURE,
open list:RISC-V SPACEMIT SoC Support
In-Reply-To: <20260620062238.3199108-1-aurelien@aurel32.net>
The K3 Pico-ITX board includes a 24c02 eeprom, that stores the product
name, the part name, the MAC address of the network interfaces and the
board's serial number. These values are also exposed via an
onie,tlv-layout nvmem layout.
The eeprom is marked as read-only since its contents are not supposed to
be modified.
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
arch/riscv/boot/dts/spacemit/k3-pico-itx.dts | 30 ++++++++++++++++++++
1 file changed, 30 insertions(+)
diff --git a/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts b/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
index 5fe9041ec5702..0a0c221c99e74 100644
--- a/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
+++ b/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
@@ -47,6 +47,36 @@ &i2c2 {
pinctrl-names = "default";
pinctrl-0 = <&i2c2_1_cfg>;
status = "okay";
+
+ eeprom@50 {
+ compatible = "atmel,24c02";
+ reg = <0x50>;
+ pagesize = <8>;
+ read-only;
+ size = <256>;
+ status = "okay";
+
+ nvmem-layout {
+ compatible = "onie,tlv-layout";
+
+ mac-address {
+ #nvmem-cell-cells = <1>;
+ };
+
+ num-macs {
+ };
+
+ part-number {
+ };
+
+ product-name {
+ };
+
+ serial-number {
+ };
+
+ };
+ };
};
&i2c8 {
--
2.53.0
^ permalink raw reply related
* Re: [PATCH 2/5] riscv: dts: spacemit: enable the i2c2 adapter on K3 Pico-ITX board
From: sashiko-bot @ 2026-06-20 6:29 UTC (permalink / raw)
To: Aurelien Jarno; +Cc: robh, devicetree, conor+dt
In-Reply-To: <20260620062238.3199108-3-aurelien@aurel32.net>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The pinctrl subnode is named `i2c2-0-pins` but belongs to the `i2c2-1-cfg` parent node.
- [Low] Missing space before the assignment operator in `bias-pull-up= <0>;`.
--
commit 4dc6c92d42831e85b38f3350c15707d0e2b3af18
Author: Aurelien Jarno <aurelien@aurel32.net>
riscv: dts: spacemit: enable the i2c2 adapter on K3 Pico-ITX board
This commit defines the I2C2 pinctrl properties and enables the adapter
on the K3 Pico-ITX board. It will be used by the 24c02 eeprom.
> diff --git a/arch/riscv/boot/dts/spacemit/k3-pinctrl.dtsi b/arch/riscv/boot/dts/spacemit/k3-pinctrl.dtsi
> index 23899d3f308a0..c9fe9c2e5aeee 100644
> --- a/arch/riscv/boot/dts/spacemit/k3-pinctrl.dtsi
> +++ b/arch/riscv/boot/dts/spacemit/k3-pinctrl.dtsi
[ ... ]
> @@ -45,6 +45,17 @@ gmac0-phy-0-pins {
> };
> };
>
> + /omit-if-no-ref/
> + i2c2_1_cfg: i2c2-1-cfg {
> + i2c2-0-pins {
[Severity: Low]
This isn't a bug, but should this subnode be named i2c2-1-pins to match
the i2c2-1-cfg parent node? It seems to diverge from the standard naming
pattern where the suffix of the parent and child matches.
> + pinmux = <K3_PADCONF(46, 5)>, /* i2c2 scl */
> + <K3_PADCONF(47, 5)>; /* i2c2 sda */
> +
> + bias-pull-up= <0>;
[Severity: Low]
This isn't a bug, but there is a missing space before the equals sign
here, which diverges from the spacing used in the surrounding code.
> + drive-strength = <25>;
> + };
> + };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260620062238.3199108-1-aurelien@aurel32.net?part=2
^ permalink raw reply
* Re: [PATCH v2 2/2] riscv: dts: spacemit: add QSPI support for K3 Pico-ITX
From: Aurelien Jarno @ 2026-06-20 6:23 UTC (permalink / raw)
To: Zhengyu He
Cc: Han Xu, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Yixun Lan, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti, linux-spi, imx, devicetree, linux-kernel,
linux-riscv, spacemit, Wei Fu, Cody Kang
In-Reply-To: <20260521-k3-pico-itx-qspi-v2-for-next-20260521-v2-2-52bce26e5fd8@gmail.com>
On 2026-05-21 22:44, Zhengyu He wrote:
> Add K3 QSPI controller node into k3.dtsi, and add related pinmux
> configuration.
>
> Enable QSPI on Pico-ITX board, and describe the NOR flash which wires
> to it.
>
> Signed-off-by: Cody Kang <cody.kang.hk@outlook.com>
> Signed-off-by: Zhengyu He <hezhy472013@gmail.com>
> ---
> Changes in v2:
> - Add "spacemit,k1-qspi" fallback to the K3 QSPI compatible.
> - Reordered Signed-off-by trailers.
> ---
> arch/riscv/boot/dts/spacemit/k3-pico-itx.dts | 58 ++++++++++++++++++++++++++++
> arch/riscv/boot/dts/spacemit/k3-pinctrl.dtsi | 21 ++++++++++
> arch/riscv/boot/dts/spacemit/k3.dtsi | 17 ++++++++
> 3 files changed, 96 insertions(+)
Thanks for the patchset.
Tested-by: Aurelien Jarno <aurelien@aurel32.net>
Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurelien@aurel32.net http://aurel32.net
^ permalink raw reply
* Re: [PATCH v7 0/2] arm64: dts: rockchip: add Vicharak Axon board support
From: Hrushiraj Gandhi @ 2026-06-20 5:37 UTC (permalink / raw)
To: heiko
Cc: krzk+dt, robh, conor+dt, devicetree, linux-arm-kernel,
linux-rockchip, linux-kernel
In-Reply-To: <20260608060940.52549-1-hrushirajg23@gmail.com>
Hi Heiko,
Just a friendly ping on this series.
The binding patch has received an Acked-by from Krzysztof and the
requested review comments from previous revisions have been addressed.
Could you please take a look when you have a chance? Any additional
feedback on either the binding or DTS patch would be greatly appreciated.
Thanks for your time.
Best regards,
Hrushiraj
^ permalink raw reply
* [PATCH v4] riscv: dts: spacemit: Use symbolic PDMA request numbers on K1
From: Guodong Xu @ 2026-06-20 5:07 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Yixun Lan, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Alexandre Ghiti
Cc: linux-kernel, dmaengine, devicetree, linux-riscv, spacemit,
Guodong Xu
The PDMA request numbers (DRQ) are fixed values specific to the SoC from
a hardware perspective. The detailed definition can be found in K1 User
Manual [1], Chapter 9.4.3 DMA Connectivity & Assignments. Add a DTS
header file to define the symbolic names for the DRQs of non-secure DMA
peripherals.
Convert the K1 SPI3 node to these macros.
Link: https://www.spacemit.com/community/document/info?lang=en&nodepath=hardware/key_stone/k1/k1_docs/k1_usermanual/9.Top_System.md [1]
Signed-off-by: Guodong Xu <docular.xu@gmail.com>
---
Changes in v4:
- Rework the commit message (Yixun Lan).
- Trim the k1-pdma.h file comment (Yixun Lan).
- Link to v3: https://patch.msgid.link/20260611-b4-k1-pdma-req-macros-v3-1-ae8416052571@gmail.com
Changes in v3:
- Move the request-number macros from include/dt-bindings/dma/ to a local
DTS header arch/riscv/boot/dts/spacemit/k1-pdma.h (Conor).
- Squash the header and its user into a single patch.
- Link to v2: https://patch.msgid.link/20260609-b4-k1-pdma-req-macros-v2-0-5d5d7b997b54@gmail.com
Changes in v2:
- Drop the #dma-cells description change in spacemit,k1-pdma.yaml; the request
numbers are hardware-fixed and unused by the driver (Conor)
- Link to v1: https://patch.msgid.link/20260607-b4-k1-pdma-req-macros-v1-0-5b2a3955007c@gmail.com
---
arch/riscv/boot/dts/spacemit/k1-pdma.h | 56 ++++++++++++++++++++++++++++++++++
arch/riscv/boot/dts/spacemit/k1.dtsi | 4 ++-
2 files changed, 59 insertions(+), 1 deletion(-)
diff --git a/arch/riscv/boot/dts/spacemit/k1-pdma.h b/arch/riscv/boot/dts/spacemit/k1-pdma.h
new file mode 100644
index 0000000000000..7e5ad3d7111d4
--- /dev/null
+++ b/arch/riscv/boot/dts/spacemit/k1-pdma.h
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * DMA request number (DRQ) definitions for non-secure peripherals of
+ * the SpacemiT K1 PDMA.
+ *
+ * Copyright (c) 2026 Guodong Xu <docular.xu@gmail.com>
+ */
+
+#ifndef _DTS_SPACEMIT_K1_PDMA_H
+#define _DTS_SPACEMIT_K1_PDMA_H
+
+#define K1_PDMA_UART0_TX 3
+#define K1_PDMA_UART0_RX 4
+#define K1_PDMA_UART2_TX 5
+#define K1_PDMA_UART2_RX 6
+#define K1_PDMA_UART3_TX 7
+#define K1_PDMA_UART3_RX 8
+#define K1_PDMA_UART4_TX 9
+#define K1_PDMA_UART4_RX 10
+#define K1_PDMA_I2C0_TX 11
+#define K1_PDMA_I2C0_RX 12
+#define K1_PDMA_I2C1_TX 13
+#define K1_PDMA_I2C1_RX 14
+#define K1_PDMA_I2C2_TX 15
+#define K1_PDMA_I2C2_RX 16
+#define K1_PDMA_I2C4_TX 17
+#define K1_PDMA_I2C4_RX 18
+#define K1_PDMA_SPI3_TX 19
+#define K1_PDMA_SPI3_RX 20
+#define K1_PDMA_I2S0_TX 21
+#define K1_PDMA_I2S0_RX 22
+#define K1_PDMA_I2S1_TX 23
+#define K1_PDMA_I2S1_RX 24
+#define K1_PDMA_UART5_TX 25
+#define K1_PDMA_UART5_RX 26
+#define K1_PDMA_UART6_TX 27
+#define K1_PDMA_UART6_RX 28
+#define K1_PDMA_UART7_TX 29
+#define K1_PDMA_UART7_RX 30
+#define K1_PDMA_UART8_TX 31
+#define K1_PDMA_UART8_RX 32
+#define K1_PDMA_UART9_TX 33
+#define K1_PDMA_UART9_RX 34
+#define K1_PDMA_I2C5_TX 35
+#define K1_PDMA_I2C5_RX 36
+#define K1_PDMA_I2C6_TX 37
+#define K1_PDMA_I2C6_RX 38
+#define K1_PDMA_I2C7_TX 39
+#define K1_PDMA_I2C7_RX 40
+#define K1_PDMA_I2C8_TX 41
+#define K1_PDMA_I2C8_RX 42
+#define K1_PDMA_CAN0_RX 43
+#define K1_PDMA_QSPI_RX 44
+#define K1_PDMA_QSPI_TX 45
+
+#endif /* _DTS_SPACEMIT_K1_PDMA_H */
diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
index 08a0f28d011fe..7d414e15d2cc2 100644
--- a/arch/riscv/boot/dts/spacemit/k1.dtsi
+++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
@@ -6,6 +6,8 @@
#include <dt-bindings/clock/spacemit,k1-syscon.h>
#include <dt-bindings/phy/phy.h>
+#include "k1-pdma.h"
+
/dts-v1/;
/ {
#address-cells = <2>;
@@ -1094,7 +1096,7 @@ spi3: spi@d401c000 {
clock-names = "core", "bus";
resets = <&syscon_apbc RESET_SSP3>;
interrupts = <55>;
- dmas = <&pdma 20>, <&pdma 19>;
+ dmas = <&pdma K1_PDMA_SPI3_RX>, <&pdma K1_PDMA_SPI3_TX>;
dma-names = "rx", "tx";
status = "disabled";
};
---
base-commit: 793cc54475b49b5b558902b5c13e4bfe66530a50
change-id: 20260607-b4-k1-pdma-req-macros-8d276d0126df
Best regards,
--
Guodong Xu <docular.xu@gmail.com>
^ permalink raw reply related
* Re: [PATCH v4 3/3] arm64: dts: qcom: Add Vicharak Axon Mini
From: Ajit Singh @ 2026-06-20 4:57 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Bjorn Andersson, Bartosz Golaszewski, Dmitry Baryshkov,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, linux-arm-msm,
devicetree, linux-kernel
In-Reply-To: <c16a2837-4b04-466a-bf4a-3717436c25be@oss.qualcomm.com>
On Fri, Jun 19, 2026 at 05:30:40PM +0530, Konrad Dybcio wrote:
> On 6/18/26 8:29 AM, Ajit Singh wrote:
> > On Tue, Jun 16, 2026 at 02:10:44PM +0530, Konrad Dybcio wrote:
> >> On 6/12/26 6:16 AM, Ajit Singh wrote:
> >>> On Wed, Jun 10, 2026 at 02:58:19PM +0530, Konrad Dybcio wrote:
> >>>> On 6/7/26 1:36 PM, Ajit Singh wrote:
> >>>>> Add DTS for the Vicharak Axon Mini board based on the Qualcomm
> >>>>> QCS6490 SoC.
> >>>>>
> >>>>> This adds debug UART, eMMC, UFS, SDIO WLAN, USB 2.0 host, PCIe
> >>>>> support along with regulators.
> >>>>>
> >>>>> The UFS ICE block is kept disabled because enabling it currently causes
> >>>>> an SError during qcom_ice_create() on this board. UFS works without ICE.
> >>>>>
> >>>>> Signed-off-by: Ajit Singh <blfizzyy@gmail.com>
> >>>>> ---
> >>>>
> >>>> [...]
> >>>>
> >>>>> + vreg_l12c_1p8: ldo12 {
> >>>>> + regulator-name = "vreg_l12c_1p8";
> >>>>> + regulator-min-microvolt = <1800000>;
> >>>>> + regulator-max-microvolt = <2000000>;
> >>>>> + regulator-initial-mode = <RPMH_REGULATOR_MODE_HPM>;
> >>>>> +
> >>>>> + /*
> >>>>> + * VREG_L12C_1P8 supplies the Ampak WLAN/BT module
> >>>>> + * VDDIO and the external 32.768 kHz oscillator.
> >>>>> + */
> >>>>
> >>>> Sorry for the long review timelines on the previous patch, many of us
> >>>> were out for conferences..
> >>>>
> >>>> Is the oscillator used for that WLAN module? Would you ideally like to
> >>>> be able to turn it on/off?
> >>>
> >>> yes, oscillator is used for WLAN modules. Oscillator is powered from the same
> >>> VREG_L12C rail as WLAN VDDIO, so there is no separate regulator control to put
> >>> in pwrseq. So I think this will work fine?
> >>
> >> Probably? My point is that you marked it as always-on, so it will *never*
> >> turn off right now. For e.g. Qualcomm wifi, there's some timing spec that
> >> needs to be met wrt delays between toggling various regulators and GPIOs
> >> going to the module, hence I suggested you may need some pwrseq inbetween
> >> to achieve reliable powering on/off
> >>
> > Right, I checked the module timing requirements.
> >
> > The module requires VBAT to be present before or at the same time as VDDIO, and
> > WL_REG_ON to be asserted only after VBAT/VDDIO are valid, around 2 sleep-clock
> > cycles later.
> >
> > On this board, VBAT is the shared VCC_3V3 rail and is enabled by hardware, so
> > it is already present before VDDIO. VREG_L12C supplies WLAN/BT VDDIO and is
> > kept on. The WL_REG_ON timing is handled by the existing mmc-pwrseq-simple
> > reset GPIO/delay before SDIO enumeration.
> >
> > So I think the current sequencing matches the module timing requirement.
>
> Since the sequencing is handled, can we drop the always-on property,
> perhaps by describing the SDIO WLAN?
>
> e.g. in arch/arm/boot/dts/st/ste-ux500-samsung-codina-tmo.dts there is:
>
> /* WLAN SDIO channel */
> mmc@80118000 {
> arm,primecell-periphid = <0x10480180>;
> max-frequency = <50000000>;
> bus-width = <4>;
> non-removable;
> cap-sd-highspeed;
> pinctrl-names = "default", "sleep";
> pinctrl-0 = <&mc1_a_2_default>;
> pinctrl-1 = <&mc1_a_2_sleep>;
> /*
> * GPIO-controlled voltage enablement: this drives
> * the WL_REG_ON line high when we use this device.
> * Represented as regulator to fill OCR mask.
> */
> vmmc-supply = <&wl_reg>;
>
> #address-cells = <1>;
> #size-cells = <0>;
> status = "okay";
>
> wifi@1 {
> compatible = "brcm,bcm4334-fmac", "brcm,bcm4329-fmac";
> reg = <1>;
> /* GPIO216 WL_HOST_WAKE */
> interrupt-parent = <&gpio6>;
> interrupts = <24 IRQ_TYPE_EDGE_FALLING>;
> interrupt-names = "host-wake";
> pinctrl-names = "default";
> pinctrl-0 = <&wlan_default_mode>;
> };
> };
>
> most notably though, it seems that the brcmfmac driver doesn't even use
> the regulator framework, probably because all of the SDIO WLANs that
> Linux supports were wired in a more "embedded" way, where the V(Q)MMC
> supplies were enough.. unless it's the case here too?
It might not be possible to drop regulator-always-on for VREG_L12C here. The
SDHC2 host already has its own vmmc/vqmmc rails:
vqmmc-supply = <&vreg_l2c_1p62>;
vmmc-supply = <&vreg_l6c_2p96>;
VREG_L12C is a separate module-side VDDIO rail for the AP6272S WLAN/BT module.
There is no separate GPIO-controlled enable for this rail;
as you said, since brcmfmac does not appear to consume an extra VDDIO regulator
from the SDIO child node, so unless there is a preferred way to model this
module-side VDDIO rail, I think VREG_L12C still needs to stay always-on.
>
> Konrad
Ajit
^ permalink raw reply
* 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
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