* Re: [PATCH V2 2/3] arm64: dts: qcom: monaco-evk: Enable SDHCI for SD Card via overlay
From: sashiko-bot @ 2026-06-16 13:15 UTC (permalink / raw)
To: Monish Chunara; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260616130347.3096034-3-monish.chunara@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The device tree overlay fails to clear the inherited eMMC configuration (`no-sd`, `non-removable`) from the base DT, resulting in completely broken SD card support.
- [Medium] The overlay configures a 1.8V-capable regulator and UHS-I OPPs, but explicitly disables 1.8V signaling, rendering the UHS-I configuration unreachable dead code.
--
commit 5cd7b1bc67719c7d2577b7397199af1024203df8
Author: Monish Chunara <monish.chunara@oss.qualcomm.com>
arm64: dts: qcom: monaco-evk: Enable SDHCI for SD Card via overlay
The monaco EVK board supports either eMMC or SD-card, but only one can be
active at a time. This commit enables the SDHCI on the monaco EVK board to
support SD Card storage via a device tree overlay, allowing eMMC to be
enabled via a separate overlay.
> diff --git a/arch/arm64/boot/dts/qcom/monaco-evk-sd-card.dtso b/arch/arm64/boot/dts/qcom/monaco-evk-sd-card.dtso
> new file mode 100644
> index 0000000000000..bc4ea12587a2e
> --- /dev/null
> +++ b/arch/arm64/boot/dts/qcom/monaco-evk-sd-card.dtso
[ ... ]
> +&sdhc_1 {
> + vmmc-supply = <&vmmc_sdc>;
> + vqmmc-supply = <&vreg_sdc>;
> +
> + pinctrl-0 = <&sdc1_state_on>, <&sd_cd>;
> + pinctrl-1 = <&sdc1_state_off>, <&sd_cd>;
> + pinctrl-names = "default", "sleep";
> +
> + cap-sd-highspeed;
> + no-1-8-v;
[Severity: Medium]
Will the no-1-8-v property prevent the UHS-I configuration from working?
The overlay configures a 1.8V-capable regulator (vreg_sdc) and adds
high-frequency OPPs (100MHz, 202MHz) to sdhc1_opp_table below, but explicitly
disabling 1.8V signaling forbids the MMC core from initiating the 1.8V
voltage switch.
This appears to limit the host to standard High Speed and makes the 1.8V
regulator state and UHS-I OPPs unreachable.
> +
> + bus-width = <4>;
> + cd-gpios = <&tlmm 11 GPIO_ACTIVE_LOW>;
> + no-mmc;
> + no-sdio;
[Severity: High]
Does this overlay properly clear the inherited eMMC configuration from the
base device tree?
The base device tree (monaco-evk.dts) configures &sdhc_1 with no-sd and
non-removable for default eMMC support. Since device tree overlays merge
properties, and this overlay doesn't use /delete-property/ for no-sd and
non-removable, the resulting node will have both no-sd and no-mmc
simultaneously.
Could this cause the MMC core to refuse to probe both SD and MMC cards?
> +
> + status = "okay";
> +};
> +
> +&sdhc1_opp_table {
> + opp-100000000 {
> + opp-hz = /bits/ 64 <100000000>;
> + required-opps = <&rpmhpd_opp_low_svs>;
> + };
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616130347.3096034-1-monish.chunara@oss.qualcomm.com?part=2
^ permalink raw reply
* [PATCH v8 0/5] iio: adc: add Versal SysMon driver
From: Salih Erim @ 2026-06-16 13:15 UTC (permalink / raw)
To: jic23, andy
Cc: dlechner, nuno.sa, robh, krzk+dt, conor+dt, conall.ogriofa,
michal.simek, linux, erimsalih, linux-iio, devicetree,
linux-kernel, Salih Erim
This series adds a new IIO driver for the AMD/Xilinx Versal System
Monitor (SysMon), providing on-chip voltage and temperature monitoring.
The Versal SysMon measures up to 160 supply voltages and reads up to
64 temperature satellites distributed across the SoC. The hardware
also provides aggregated device temperature registers: the current
max and min across all active satellites, and peak/trough values
recorded since last hardware reset. The device can be accessed via
memory-mapped I/O or via an I2C interface.
The driver is split into a bus-agnostic core module using the regmap
API, an MMIO platform driver, and an I2C driver. This allows the
same IIO logic to be shared across different bus transports.
Previous submissions:
v7: https://lore.kernel.org/all/20260614233722.2603459-1-salih.erim@amd.com/
v6: https://lore.kernel.org/all/20260611222738.2035062-1-salih.erim@amd.com/
v5: https://lore.kernel.org/all/20260608183801.1257051-1-salih.erim@amd.com/
v4: https://lore.kernel.org/all/20260606051707.535281-1-salih.erim@amd.com/
v3: https://lore.kernel.org/all/20260527114211.174288-1-salih.erim@amd.com/
v2: https://lore.kernel.org/all/20260502111951.538488-1-salih.erim@amd.com/
v1: https://lore.kernel.org/all/cover.1757061697.git.michal.simek@amd.com/
Changes in v8:
- Address Andy v7 review items: MILLIDEGREE_PER_DEGREE, overflow.h,
devm_versal_sysmon_core_probe rename, volatile register comment,
regmap_test_bits, unsigned int for osr_write, ~0 instead of
~0U (Andy)
- Fix devm teardown race in interrupt cleanup (Sashiko)
Changes in v7:
- Move TEMP event onto channel 0; drop OT as separate IIO
channel; use single temp_channels array with runtime event
attachment (Jonathan)
- Return administrative state from temp_mask in read_event_config
instead of transient hardware IMR (Jonathan, Sashiko)
- Add devm cleanup to mask HW interrupts on driver unbind (Sashiko)
- Add documentation comment for static aggregate temp channels
(Jonathan)
- Split sysmon_osr_write into per-type helpers (Jonathan)
Changes in v6:
- Address all Andy v5 review items: switch(chan->type) in event
functions, reversed xmas tree ordering, macro brace placement,
scoped_guard in unmask worker, combined regmap error checks,
join single-line constructs, IWYU header fixes (Andy)
- Fix fwnode_irq_get() to propagate only -EPROBE_DEFER; treating
all negatives as fatal broke probe on I2C nodes without
interrupts property
Changes in v5:
- Core: add err.h include (IWYU) (Andy)
- Core: drop (int) cast on MILLI in scale assignment (Andy)
- Core: sign_extend32() instead of (s16) cast (Andy)
- Core: remove unneeded parentheses in voltage address
calculation (Andy)
- Core: drop NULL checks before fwnode_get_child_node_count
(NULL-aware) (Andy)
- Core: nested size_add() for overflow-safe allocation (Andy)
- Core: if (ret) instead of if (ret < 0) for fwnode property
reads (Andy)
- Core: remove outer parentheses in satellite address
calculation (Andy)
- Core: loop index declared in for() scope (Andy)
- MMIO: add err.h, types.h includes (IWYU) (Andy)
- Header: remove unused types.h include and struct iio_dev
forward declaration at P2 stage (Andy)
- I2C: add err.h, mod_devicetable.h includes (IWYU) (Andy)
- Events: clamp() instead of clamp_t() (Andy)
- Events: regmap_assign_bits() instead of separate set/clear (Andy)
- Events: remove unneeded parentheses (2 places) (Andy)
- Events: for_each_set_bit on single line (Andy)
- Events: regmap_clear_bits() instead of regmap_update_bits() (Andy)
- Events: simplify unmask XOR to ~status & masked_temp (Andy)
- Events: add comment explaining unmask &= ~temp_mask logic (Andy)
- Events: split container_of across two lines (Andy)
- Events: move ISR write after !isr check (Andy)
- Events: unsigned int for init_hysteresis address param (Andy)
- Events: add comment explaining error check policy in
worker/IRQ (Andy)
- Events: nested size_add() for overflow-safe allocation (Andy)
- Events: propagate negative from fwnode_irq_get() for
EPROBE_DEFER (Andy)
- Events: pass irq instead of has_irq to sysmon_parse_fw (Andy)
- Oversampling: remove unneeded parentheses (Andy)
- Oversampling: use struct regmap *map local variable (Andy)
- Oversampling: switch instead of redundant if/if on
channel_type (Andy)
- Oversampling: add CONFIG register readback fence after
oversampling update to prevent NoC bus hang from posted
writes (found during hardware stress testing)
Changes in v4:
- Core: temperature channels use RAW + SCALE (IIO_VAL_FRACTIONAL,
1000/128) instead of PROCESSED; voltage channels use PROCESSED
only, drop RAW; drop scan_type from all channel macros (Jonathan)
- Core: move __free(fwnode_handle) declarations down to just
above use; devm_regmap_init() on one line; lock comment
describes RMW sequences and cached state (Jonathan)
- Events: merge event channels into static temp array -- two
arrays (with/without events) selected by has_irq; event-only
channels have no info_mask (Jonathan)
- Events: blank lines, fit under 80 chars, default returns error,
return early in each case, guard(spinlock) in IRQ handler
(Jonathan)
- Events: take irq_lock in write_event_config for temp_mask
updates (Sashiko)
- I2C: replace enum with defines, use unaligned accessors for
data and register offset packing, named initializer in
i2c_device_id (Jonathan)
- I2C: drop bitfield.h, add unaligned.h
- Oversampling: return directly, remove else after early returns,
rename mask defines, blank lines (Jonathan)
- Oversampling: move oversampling read inside guard(mutex) scope
- Fix v2 lore link in cover letter
Changes in v3:
- DT binding: single compatible, voltage-channels rename, single
quotes, drop label/bipolar/xlnx,aie-temp (Krzysztof)
- Core: IWYU throughout, __free(fwnode_handle), sign_extend32(),
size_add(), dev_err_probe(), s16 param, remove (int) casts,
drop SYSMON_MILLI in favor of (int)MILLI, rename _ext to _name
in SYSMON_CHAN_TEMP macro (Andy, Jonathan)
- Core: fwnode_irq_get() moved to core_probe, remove sysmon->dev/
indio_dev/irq from struct, describe protected data in lock
comment, add RAW+PROCESSED comment (Jonathan)
- I2C: IWYU, remove wrapper struct, explicit enum values, sizeof()
for buffers, = { } initializers, adapt to core_probe interface
change (Andy, Krzysztof)
- Events: IWYU, FIELD_GET/FIELD_PREP, regmap_set/clear_bits,
clamp_t, !!, IRQ_RETVAL(), devm_delayed_work_autocancel,
loop var scope, error checks, remove redundant else, logical
param splits, spinlock safety comment (Andy)
- Events: hysteresis rework -- store as millicelsius, hardcode
ALARM_CONFIG to hysteresis mode, compute lower threshold from
(upper - hysteresis), remove falling threshold for temperature,
single event spec per channel with IIO_EV_DIR_RISING, push
IIO_EV_DIR_RISING for temp and IIO_EV_DIR_EITHER for voltage
(Jonathan)
Tested on VCK190 (single SLR, MMIO path, 7 supplies, 10 temperature
satellites). I2C compile-tested.
A follow-up series will add thermal zone integration, secure firmware
access, and I2C remote monitoring.
Salih Erim (5):
dt-bindings: iio: adc: add xlnx,versal-sysmon binding
iio: adc: add Versal SysMon driver
iio: adc: versal-sysmon: add I2C driver
iio: adc: versal-sysmon: add threshold event support
iio: adc: versal-sysmon: add oversampling support
.../bindings/iio/adc/xlnx,versal-sysmon.yaml | 154 +++
MAINTAINERS | 7 +
drivers/iio/adc/Kconfig | 33 +
drivers/iio/adc/Makefile | 3 +
drivers/iio/adc/versal-sysmon-core.c | 1036 +++++++++++++++++
drivers/iio/adc/versal-sysmon-i2c.c | 133 +++
drivers/iio/adc/versal-sysmon.c | 93 ++
drivers/iio/adc/versal-sysmon.h | 120 ++
8 files changed, 1579 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/xlnx,versal-sysmon.yaml
create mode 100644 drivers/iio/adc/versal-sysmon-core.c
create mode 100644 drivers/iio/adc/versal-sysmon-i2c.c
create mode 100644 drivers/iio/adc/versal-sysmon.c
create mode 100644 drivers/iio/adc/versal-sysmon.h
--
2.48.1
^ permalink raw reply
* [PATCH v8 1/5] dt-bindings: iio: adc: add xlnx,versal-sysmon binding
From: Salih Erim @ 2026-06-16 13:15 UTC (permalink / raw)
To: jic23, andy
Cc: dlechner, nuno.sa, robh, krzk+dt, conor+dt, conall.ogriofa,
michal.simek, linux, erimsalih, linux-iio, devicetree,
linux-kernel, Salih Erim, Krzysztof Kozlowski
In-Reply-To: <20260616131559.3029543-1-salih.erim@amd.com>
Add devicetree binding for the AMD/Xilinx Versal System Monitor (SysMon).
The Versal SysMon is the successor to the Zynq UltraScale+ AMS block,
providing on-chip voltage and temperature monitoring. The hardware
supports up to 160 supply voltage measurement points and up to 64
temperature satellites distributed across the SoC, with configurable
threshold alarms and oversampling. The device can be accessed via
memory-mapped I/O or via an I2C interface.
Supply and temperature channels are described as child nodes under
container nodes, referencing the standard adc.yaml binding for
channel properties.
Co-developed-by: Michal Simek <michal.simek@amd.com>
Signed-off-by: Michal Simek <michal.simek@amd.com>
Signed-off-by: Salih Erim <salih.erim@amd.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
Changes in v8:
- No code changes
Changes in v7:
- No code changes
Changes in v6:
- No code changes
Changes in v5:
- No code changes
Changes in v4:
- Add Reviewed-by tag from Krzysztof Kozlowski
Changes in v3:
- Use single compatible (xlnx,versal-sysmon only), remove
xlnx,versal-sysmon-i2c (Krzysztof)
- Rename supply-channels container to voltage-channels (Krzysztof)
- Use single quotes in patternProperties regex (Krzysztof)
- Drop label description from channel properties (Krzysztof)
- Drop bipolar from channel properties (Krzysztof)
- Remove xlnx,aie-temp property from binding and example (Krzysztof)
Changes in v2:
- Restructured to container nodes (supply-channels, temperature-channels)
with channel@N children referencing adc.yaml
- Added xlnx,versal-sysmon-i2c compatible
- Descriptions rewritten to describe hardware only
- Example simplified to #address-cells = <1>
- Interrupt example uses GIC_SPI/IRQ_TYPE_LEVEL_HIGH constants
- Commit description explains hardware context instead of schema layout
- reg required for both MMIO and I2C, interrupts optional
- Hex unit-addresses (channel@a not channel@10) per DTSpec
- patternProperties regex updated to accept hex digits [0-9a-f]
- Example trimmed to minimal variants (one basic + one bipolar supply,
one AIE temperature channel)
.../bindings/iio/adc/xlnx,versal-sysmon.yaml | 154 ++++++++++++++++++
1 file changed, 154 insertions(+)
create mode 100644 Documentation/devicetree/bindings/iio/adc/xlnx,versal-sysmon.yaml
diff --git a/Documentation/devicetree/bindings/iio/adc/xlnx,versal-sysmon.yaml b/Documentation/devicetree/bindings/iio/adc/xlnx,versal-sysmon.yaml
new file mode 100644
index 00000000000..1ad58e3d616
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/xlnx,versal-sysmon.yaml
@@ -0,0 +1,154 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2022 - 2026, Advanced Micro Devices, Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/xlnx,versal-sysmon.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AMD/Xilinx Versal System Monitor
+
+maintainers:
+ - Salih Erim <salih.erim@amd.com>
+
+description:
+ The AMD/Xilinx Versal System Monitor (SysMon) is the successor to the
+ Zynq UltraScale+ AMS block. It provides on-chip voltage and temperature
+ monitoring with up to 160 voltage measurement points and up to
+ 64 temperature satellites distributed across the SoC. The hardware
+ supports configurable threshold alarms and oversampling. The device
+ can be accessed via memory-mapped I/O or via an I2C interface.
+
+properties:
+ compatible:
+ const: xlnx,versal-sysmon
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ '#io-channel-cells':
+ const: 1
+
+ voltage-channels:
+ type: object
+ description:
+ Container for voltage measurement channels.
+
+ properties:
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ patternProperties:
+ '^channel@([0-9a-f]|[1-9][0-9a-f])$':
+ $ref: adc.yaml
+
+ description:
+ Measures a voltage rail. The register index and rail
+ name are assigned by the hardware design tool (Vivado).
+
+ properties:
+ reg:
+ minimum: 0
+ maximum: 159
+ description:
+ Voltage measurement register index assigned by the hardware
+ design tool.
+
+ required:
+ - reg
+ - label
+
+ unevaluatedProperties: false
+
+ required:
+ - '#address-cells'
+ - '#size-cells'
+
+ additionalProperties: false
+
+ temperature-channels:
+ type: object
+ description:
+ Container for temperature satellite measurement channels.
+
+ properties:
+ '#address-cells':
+ const: 1
+
+ '#size-cells':
+ const: 0
+
+ patternProperties:
+ '^channel@([1-9a-f]|[1-3][0-9a-f]|40)$':
+ $ref: adc.yaml
+
+ description:
+ Reads a temperature satellite sensor. Each satellite monitors
+ a specific region of the SoC die.
+
+ properties:
+ reg:
+ minimum: 1
+ maximum: 64
+ description:
+ Temperature satellite number (1-based hardware index).
+
+ required:
+ - reg
+ - label
+
+ unevaluatedProperties: false
+
+ required:
+ - '#address-cells'
+ - '#size-cells'
+
+ additionalProperties: false
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ sysmon@f1270000 {
+ compatible = "xlnx,versal-sysmon";
+ reg = <0xf1270000 0x4000>;
+ interrupts = <GIC_SPI 144 IRQ_TYPE_LEVEL_HIGH>;
+ #io-channel-cells = <1>;
+
+ voltage-channels {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ channel@0 {
+ reg = <0>;
+ label = "vccaux";
+ };
+
+ channel@3 {
+ reg = <3>;
+ label = "vcc_ram";
+ bipolar;
+ };
+ };
+
+ temperature-channels {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ channel@a {
+ reg = <10>;
+ label = "aie-temp-ch1";
+ };
+ };
+ };
--
2.48.1
^ permalink raw reply related
* [PATCH v8 3/5] iio: adc: versal-sysmon: add I2C driver
From: Salih Erim @ 2026-06-16 13:15 UTC (permalink / raw)
To: jic23, andy
Cc: dlechner, nuno.sa, robh, krzk+dt, conor+dt, conall.ogriofa,
michal.simek, linux, erimsalih, linux-iio, devicetree,
linux-kernel, Salih Erim
In-Reply-To: <20260616131559.3029543-1-salih.erim@amd.com>
Add an I2C transport driver for the Versal SysMon block. The SysMon
provides an I2C slave interface that allows an external master to
read voltage and temperature measurements through the same register
map used by the MMIO path.
The I2C command frame is an 8-byte structure containing a 4-byte data
payload, a 2-byte register offset, and a 1-byte instruction field.
Read operations send the frame with a read instruction, then receive
a 4-byte response containing the register value.
Events are not supported on the I2C path because there is no
interrupt line and the I2C regmap backend cannot be called from
atomic context.
Co-developed-by: Conall O'Griofa <conall.ogriofa@amd.com>
Signed-off-by: Conall O'Griofa <conall.ogriofa@amd.com>
Signed-off-by: Salih Erim <salih.erim@amd.com>
---
Changes in v8:
- Add volatile register comment for regmap cache (Andy)
- Update devm_versal_sysmon_core_probe call site (Andy, from P2 rename)
Changes in v7:
- No code changes
Changes in v6:
- Add types.h include (IWYU) (Andy)
- Add local struct device *dev, join devm_regmap_init on
one line (Andy)
Changes in v5:
- Add err.h, mod_devicetable.h includes (IWYU) (Andy)
Changes in v4:
- Replace enum with defines for I2C frame offsets (Jonathan)
- Use get_unaligned_le32() for read data reassembly (Jonathan)
- Use put_unaligned_le32/le16() for write data and register offset
packing (Jonathan)
- Named initializer in i2c_device_id (Jonathan)
- Drop bitfield.h, add unaligned.h (FIELD_GET/FIELD_PREP replaced
by unaligned accessors)
Changes in v3:
- IWYU: fix includes (Andy)
- Enum: assign all values explicitly for HW-mapped fields (Andy)
- Remove sysmon_i2c wrapper struct, pass i2c_client directly
(Andy)
- Use sizeof() for I2C buffer lengths instead of defines (Andy)
- Use = { } instead of = { 0 } for initializers (Andy)
- Use single compatible xlnx,versal-sysmon (Krzysztof)
- Adapt to core_probe interface change: irq moved to core,
remove irq parameter from bus driver (Jonathan)
Changes in v2:
- New patch (I2C was deferred to Series B in v1)
- Uses regmap API with custom I2C read/write callbacks
- Shares core module with MMIO driver via sysmon_core_probe()
- No event support (I2C has no interrupt line)
- Separate VERSAL_SYSMON_I2C Kconfig symbol
- Reverse Christmas Tree variable ordering in read/write functions
drivers/iio/adc/Kconfig | 13 +++
drivers/iio/adc/Makefile | 1 +
drivers/iio/adc/versal-sysmon-i2c.c | 133 ++++++++++++++++++++++++++++
3 files changed, 147 insertions(+)
create mode 100644 drivers/iio/adc/versal-sysmon-i2c.c
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index c7f19057484..8f9fc9de74a 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1963,6 +1963,19 @@ config VERSAL_SYSMON
To compile this driver as a module, choose M here: the module
will be called versal-sysmon.
+config VERSAL_SYSMON_I2C
+ tristate "AMD Versal SysMon I2C driver"
+ depends on I2C
+ select VERSAL_SYSMON_CORE
+ help
+ Say yes here to have support for the AMD/Xilinx Versal System
+ Monitor (SysMon) via I2C interface. This driver enables voltage
+ and temperature monitoring when the Versal chip has SysMon
+ configured with I2C access.
+
+ To compile this driver as a module, choose M here: the module
+ will be called versal-sysmon-i2c.
+
config VF610_ADC
tristate "Freescale vf610 ADC driver"
depends on HAS_IOMEM
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index d7696b1b157..5abb611fe46 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -169,6 +169,7 @@ obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o
obj-$(CONFIG_TWL6030_GPADC) += twl6030-gpadc.o
obj-$(CONFIG_VERSAL_SYSMON_CORE) += versal-sysmon-core.o
obj-$(CONFIG_VERSAL_SYSMON) += versal-sysmon.o
+obj-$(CONFIG_VERSAL_SYSMON_I2C) += versal-sysmon-i2c.o
obj-$(CONFIG_VF610_ADC) += vf610_adc.o
obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o
obj-$(CONFIG_XILINX_AMS) += xilinx-ams.o
diff --git a/drivers/iio/adc/versal-sysmon-i2c.c b/drivers/iio/adc/versal-sysmon-i2c.c
new file mode 100644
index 00000000000..cd8d5846ff9
--- /dev/null
+++ b/drivers/iio/adc/versal-sysmon-i2c.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AMD Versal SysMon I2C driver
+ *
+ * Copyright (C) 2023 - 2026, Advanced Micro Devices, Inc.
+ */
+
+#include <linux/bits.h>
+#include <linux/err.h>
+#include <linux/i2c.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+
+#include "versal-sysmon.h"
+
+#define SYSMON_I2C_INSTR_READ BIT(2)
+#define SYSMON_I2C_INSTR_WRITE BIT(3)
+
+/*
+ * I2C command frame layout (8 bytes):
+ * [0..3] data payload (little-endian u32)
+ * [4..5] register offset >> 2 (little-endian u16)
+ * [6] instruction (read/write)
+ * [7] reserved
+ */
+#define SYSMON_I2C_DATA_OFS 0
+#define SYSMON_I2C_REG_OFS 4
+#define SYSMON_I2C_INSTR_OFS 6
+
+static int sysmon_i2c_reg_read(void *context, unsigned int reg,
+ unsigned int *val)
+{
+ struct i2c_client *client = context;
+ u8 write_buf[8] = { };
+ u8 read_buf[4];
+ int ret;
+
+ put_unaligned_le16(reg >> 2, &write_buf[SYSMON_I2C_REG_OFS]);
+ write_buf[SYSMON_I2C_INSTR_OFS] = SYSMON_I2C_INSTR_READ;
+
+ ret = i2c_master_send(client, write_buf, sizeof(write_buf));
+ if (ret < 0)
+ return ret;
+ if (ret != sizeof(write_buf))
+ return -EIO;
+
+ ret = i2c_master_recv(client, read_buf, sizeof(read_buf));
+ if (ret < 0)
+ return ret;
+ if (ret != sizeof(read_buf))
+ return -EIO;
+
+ *val = get_unaligned_le32(read_buf);
+
+ return 0;
+}
+
+static int sysmon_i2c_reg_write(void *context, unsigned int reg,
+ unsigned int val)
+{
+ struct i2c_client *client = context;
+ u8 write_buf[8] = { };
+ int ret;
+
+ put_unaligned_le32(val, &write_buf[SYSMON_I2C_DATA_OFS]);
+ put_unaligned_le16(reg >> 2, &write_buf[SYSMON_I2C_REG_OFS]);
+ write_buf[SYSMON_I2C_INSTR_OFS] = SYSMON_I2C_INSTR_WRITE;
+
+ ret = i2c_master_send(client, write_buf, sizeof(write_buf));
+ if (ret < 0)
+ return ret;
+ if (ret != sizeof(write_buf))
+ return -EIO;
+
+ return 0;
+}
+
+/*
+ * Almost all registers are volatile (live ADC readings, interrupt
+ * status). The rest are not accessed often enough to benefit from
+ * caching.
+ */
+static const struct regmap_config sysmon_i2c_regmap_config = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = SYSMON_REG_STRIDE,
+ .max_register = SYSMON_MAX_REG,
+ .reg_read = sysmon_i2c_reg_read,
+ .reg_write = sysmon_i2c_reg_write,
+};
+
+static int sysmon_i2c_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct regmap *regmap;
+
+ regmap = devm_regmap_init(dev, NULL, client, &sysmon_i2c_regmap_config);
+ if (IS_ERR(regmap))
+ return PTR_ERR(regmap);
+
+ /* I2C has no IRQ connection; events are not supported */
+ return devm_versal_sysmon_core_probe(dev, regmap);
+}
+
+static const struct of_device_id sysmon_i2c_of_match_table[] = {
+ { .compatible = "xlnx,versal-sysmon" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, sysmon_i2c_of_match_table);
+
+static const struct i2c_device_id sysmon_i2c_id_table[] = {
+ { .name = "versal-sysmon" },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, sysmon_i2c_id_table);
+
+static struct i2c_driver sysmon_i2c_driver = {
+ .probe = sysmon_i2c_probe,
+ .driver = {
+ .name = "versal-sysmon-i2c",
+ .of_match_table = sysmon_i2c_of_match_table,
+ },
+ .id_table = sysmon_i2c_id_table,
+};
+module_i2c_driver(sysmon_i2c_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("AMD Versal SysMon I2C Driver");
+MODULE_AUTHOR("Conall O'Griofa <conall.ogriofa@amd.com>");
+MODULE_AUTHOR("Salih Erim <salih.erim@amd.com>");
--
2.48.1
^ permalink raw reply related
* [PATCH v8 2/5] iio: adc: add Versal SysMon driver
From: Salih Erim @ 2026-06-16 13:15 UTC (permalink / raw)
To: jic23, andy
Cc: dlechner, nuno.sa, robh, krzk+dt, conor+dt, conall.ogriofa,
michal.simek, linux, erimsalih, linux-iio, devicetree,
linux-kernel, Salih Erim
In-Reply-To: <20260616131559.3029543-1-salih.erim@amd.com>
Add the core driver and MMIO platform driver for the AMD/Xilinx Versal
System Monitor (SysMon) block.
The SysMon block resides in the platform management controller (PMC) and
provides on-chip voltage and temperature monitoring through a 10-bit,
200 kSPS ADC. It can monitor up to 160 voltage channels and 64
temperature satellites distributed across the SoC, with a consistent
sample rate of 8 kSPS per channel regardless of how many channels are
enabled.
The hardware also provides four aggregate temperature registers that
are always present regardless of the device tree configuration: the
current max and min across all active satellites, and the peak and
trough values recorded since the last hardware reset.
The driver is split into two compilation units:
- versal-sysmon-core: Channel parsing, IIO registration, read_raw
- versal-sysmon: MMIO platform driver with custom regmap accessors
Voltage results are stored in a 19-bit modified floating-point format
and converted to millivolts. Temperature results are stored in Q8.7
signed fixed-point Celsius format and converted to millicelsius.
The MMIO regmap backend uses a custom reg_write accessor that
automatically unlocks the NPI (NoC programming interface) lock
register before each write, as required by the hardware. The regmap
is configured with fast_io since the underlying MMIO accessors are
safe to call from atomic context.
Co-developed-by: Michal Simek <michal.simek@amd.com>
Signed-off-by: Michal Simek <michal.simek@amd.com>
Signed-off-by: Salih Erim <salih.erim@amd.com>
---
Changes in v8:
- Use MILLIDEGREE_PER_DEGREE instead of MILLI for temperature
scale (Andy)
- Add overflow.h include for size_add() (Andy)
- Rename sysmon_core_probe to devm_versal_sysmon_core_probe (Andy)
Changes in v7:
- Add documentation comment explaining the four static aggregate
temperature registers (Jonathan)
Changes in v6:
- Macro brace on separate line for SYSMON_CHAN_TEMP (Andy)
- Remove unneeded = 0 init on num_supply, num_temp (Andy)
- Use temporary variable for channel count allocation (Andy)
- Initialize idx directly after memcpy (Andy)
- Join info_mask_separate on one line (Andy)
- Join info_mask_shared_by_type on one line (Andy)
- Join devm_kcalloc on one line (Andy)
- Reversed xmas tree ordering in sysmon_parse_fw
Changes in v5:
- Add err.h include to core (IWYU) (Andy)
- Drop (int) cast on MILLI in scale assignment (Andy)
- sign_extend32() instead of (s16) cast for temperature raw (Andy)
- Remove unneeded parentheses in voltage address calculation (Andy)
- Drop NULL checks before fwnode_get_child_node_count (Andy)
- Nested size_add() for overflow-safe allocation (Andy)
- if (ret) instead of if (ret < 0) for fwnode property reads (Andy)
- Remove outer parentheses in satellite address calculation (Andy)
- Loop index declared in for() scope (Andy)
- MMIO: add err.h, types.h includes (IWYU) (Andy)
- Header: remove unused types.h include and struct iio_dev
forward declaration (Andy)
Changes in v4:
- Temperature: RAW + SCALE (IIO_VAL_FRACTIONAL, 1000/128) instead
of PROCESSED (Jonathan)
- Voltage: PROCESSED only, drop RAW (Jonathan)
- Drop scan_type from all channel macros (Jonathan)
- Move __free(fwnode_handle) declarations down to just above use
(Jonathan)
- devm_regmap_init() on one line (Jonathan)
- Lock comment: describe RMW sequences and cached state (Jonathan)
- Remove sysmon_q8p7_to_millicelsius() from this patch; the function
is now introduced in P4 where it is first used
Changes in v3:
- IWYU: add array_size.h, string.h, types.h to core; audit and
fix header and MMIO driver includes (Andy)
- Rename _ext to _name in SYSMON_CHAN_TEMP macro parameter (Andy,
Jonathan)
- Use .info_mask_separate = BIT() style in SYSMON_CHAN_TEMP (Andy)
- Use s16 parameter in sysmon_q8p7_to_millicelsius (Andy)
- Use sign_extend32() in sysmon_supply_rawtoprocessed (Andy)
- Split sysmon_read_raw parameters logically across lines (Andy)
- Remove redundant (int) casts on regval (Andy)
- Split num_supply/num_temp initialization (Andy)
- Use __free(fwnode_handle) cleanup, remove goto err_put (Andy)
- Use size_add() for overflow-safe allocation (Andy)
- Use dev_err_probe() in sysmon_parse_fw error paths (Jonathan)
- Move fwnode_irq_get() to core_probe, remove irq parameter
from bus driver interfaces (Jonathan)
- Use (int)MILLI at call sites, drop SYSMON_MILLI define (Andy,
Jonathan)
- Remove sysmon->dev, sysmon->indio_dev, sysmon->irq from struct;
pass as local variables or use regmap_get_device() (Jonathan)
- Use struct device *dev local in sysmon_platform_probe (Andy)
- Describe protected data in lock comment (Jonathan)
- Add comment explaining RAW+PROCESSED co-exposure (Jonathan)
Changes in v2:
- Split into core (versal-sysmon-core.c) + MMIO platform driver
(versal-sysmon.c) + shared header (versal-sysmon.h)
- Uses regmap API instead of direct readl/writel
- MMIO regmap uses custom callbacks with NPI unlock in write path
- Reverse Christmas Tree variable ordering throughout
- Header include order fixed
- MAINTAINERS entry folded in with wildcard F: pattern
- Kconfig: hidden VERSAL_SYSMON_CORE + VERSAL_SYSMON selects it
- Kconfig/Makefile: alphabetical ordering (VERSAL before VF610)
- Bounds validation on DT reg values
- Named constants replace magic numbers (SYSMON_REG_STRIDE,
SYSMON_SUPPLY_MANTISSA_BITS, SYSMON_MILLI)
- kernel-doc for exported sysmon_core_probe() and sysmon_parse_fw()
- Supply voltage conversion uses proper two's complement sign
extension (s16 cast) matching the hardware specification
- Register offsets sorted by address in header
- Each patch introduces only the defines, fields, and includes
it uses (no dead code in any commit)
- Removed unused linux/limits.h and linux/units.h includes
- Renamed iio_dev_info to sysmon_iio_info
- regmap_write return values checked in probe init path
MAINTAINERS | 7 +
drivers/iio/adc/Kconfig | 20 ++
drivers/iio/adc/Makefile | 2 +
drivers/iio/adc/versal-sysmon-core.c | 291 +++++++++++++++++++++++++++
drivers/iio/adc/versal-sysmon.c | 93 +++++++++
drivers/iio/adc/versal-sysmon.h | 67 ++++++
6 files changed, 480 insertions(+)
create mode 100644 drivers/iio/adc/versal-sysmon-core.c
create mode 100644 drivers/iio/adc/versal-sysmon.c
create mode 100644 drivers/iio/adc/versal-sysmon.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 2fb1c75afd1..46762c8496d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -29216,6 +29216,13 @@ F: Documentation/devicetree/bindings/memory-controllers/xlnx,versal-net-ddrmc5.y
F: drivers/edac/versalnet_edac.c
F: include/linux/cdx/edac_cdx_pcol.h
+XILINX VERSAL SYSMON DRIVER
+M: Salih Erim <salih.erim@amd.com>
+L: linux-iio@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/iio/adc/xlnx,versal-sysmon.yaml
+F: drivers/iio/adc/versal-sysmon*
+
XILINX WATCHDOG DRIVER
M: Srinivas Neeli <srinivas.neeli@amd.com>
R: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index a9dedbb8eb4..c7f19057484 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1943,6 +1943,26 @@ config TWL6030_GPADC
This driver can also be built as a module. If so, the module will be
called twl6030-gpadc.
+config VERSAL_SYSMON_CORE
+ tristate
+ select REGMAP
+
+config VERSAL_SYSMON
+ tristate "AMD Versal SysMon driver"
+ depends on ARCH_ZYNQMP || COMPILE_TEST
+ depends on HAS_IOMEM
+ select VERSAL_SYSMON_CORE
+ help
+ Say yes here to have support for the AMD/Xilinx Versal System
+ Monitor (SysMon). This driver provides voltage and temperature
+ monitoring through the IIO subsystem.
+
+ The SysMon measures up to 160 supply voltages and reads up to
+ 64 temperature satellites distributed across the SoC.
+
+ To compile this driver as a module, choose M here: the module
+ will be called versal-sysmon.
+
config VF610_ADC
tristate "Freescale vf610 ADC driver"
depends on HAS_IOMEM
diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
index 097357d146b..d7696b1b157 100644
--- a/drivers/iio/adc/Makefile
+++ b/drivers/iio/adc/Makefile
@@ -167,6 +167,8 @@ obj-$(CONFIG_TI_TLC4541) += ti-tlc4541.o
obj-$(CONFIG_TI_TSC2046) += ti-tsc2046.o
obj-$(CONFIG_TWL4030_MADC) += twl4030-madc.o
obj-$(CONFIG_TWL6030_GPADC) += twl6030-gpadc.o
+obj-$(CONFIG_VERSAL_SYSMON_CORE) += versal-sysmon-core.o
+obj-$(CONFIG_VERSAL_SYSMON) += versal-sysmon.o
obj-$(CONFIG_VF610_ADC) += vf610_adc.o
obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o
obj-$(CONFIG_XILINX_AMS) += xilinx-ams.o
diff --git a/drivers/iio/adc/versal-sysmon-core.c b/drivers/iio/adc/versal-sysmon-core.c
new file mode 100644
index 00000000000..a1d4a13b724
--- /dev/null
+++ b/drivers/iio/adc/versal-sysmon-core.c
@@ -0,0 +1,291 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AMD Versal SysMon core driver
+ *
+ * Copyright (C) 2019 - 2022, Xilinx, Inc.
+ * Copyright (C) 2022 - 2026, Advanced Micro Devices, Inc.
+ */
+
+#include <linux/array_size.h>
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/cleanup.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/overflow.h>
+#include <linux/property.h>
+#include <linux/regmap.h>
+#include <linux/string.h>
+#include <linux/sysfs.h>
+#include <linux/units.h>
+
+#include <linux/iio/iio.h>
+
+#include "versal-sysmon.h"
+
+#define SYSMON_CHAN_TEMP(_chan, _address, _name) \
+{ \
+ .type = IIO_TEMP, \
+ .indexed = 1, \
+ .address = _address, \
+ .channel = _chan, \
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE), \
+ .datasheet_name = _name, \
+}
+
+/*
+ * Static temperature channels (always present).
+ *
+ * These are hardware-computed aggregate registers across all active
+ * temperature satellites:
+ * temp: current max temperature across all active satellites
+ * min: current min temperature across all active satellites
+ * max_max: highest peak recorded since last hardware reset
+ * min_min: lowest trough recorded since last hardware reset
+ */
+static const struct iio_chan_spec temp_channels[] = {
+ SYSMON_CHAN_TEMP(0, SYSMON_TEMP_MAX, "temp"),
+ SYSMON_CHAN_TEMP(1, SYSMON_TEMP_MIN, "min"),
+ SYSMON_CHAN_TEMP(2, SYSMON_TEMP_MAX_MAX, "max_max"),
+ SYSMON_CHAN_TEMP(3, SYSMON_TEMP_MIN_MIN, "min_min"),
+};
+
+static void sysmon_supply_rawtoprocessed(int raw_data, int *val)
+{
+ int mantissa, format, exponent;
+
+ mantissa = FIELD_GET(SYSMON_MANTISSA_MASK, raw_data);
+ exponent = SYSMON_SUPPLY_MANTISSA_BITS - FIELD_GET(SYSMON_MODE_MASK, raw_data);
+ format = FIELD_GET(SYSMON_FMT_MASK, raw_data);
+ /*
+ * When format bit is set the mantissa is two's complement
+ * (per hardware spec); sign-extend to int for correct arithmetic.
+ */
+ if (format)
+ mantissa = sign_extend32(mantissa, 15);
+
+ *val = (mantissa * (int)MILLI) >> exponent;
+}
+
+static int sysmon_read_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
+{
+ struct sysmon *sysmon = iio_priv(indio_dev);
+ unsigned int regval;
+ int ret;
+
+ guard(mutex)(&sysmon->lock);
+
+ switch (chan->type) {
+ case IIO_TEMP:
+ if (mask == IIO_CHAN_INFO_SCALE) {
+ /* Q8.7 to millicelsius: raw * 1000 / 128 */
+ *val = MILLIDEGREE_PER_DEGREE;
+ *val2 = BIT(SYSMON_FRACTIONAL_SHIFT);
+ return IIO_VAL_FRACTIONAL;
+ }
+ if (mask != IIO_CHAN_INFO_RAW)
+ return -EINVAL;
+
+ ret = regmap_read(sysmon->regmap, chan->address, ®val);
+ if (ret)
+ return ret;
+
+ *val = sign_extend32(regval, 15);
+ return IIO_VAL_INT;
+
+ case IIO_VOLTAGE:
+ if (mask != IIO_CHAN_INFO_PROCESSED)
+ return -EINVAL;
+
+ ret = regmap_read(sysmon->regmap,
+ chan->address * SYSMON_REG_STRIDE +
+ SYSMON_SUPPLY_BASE, ®val);
+ if (ret)
+ return ret;
+
+ sysmon_supply_rawtoprocessed(regval, val);
+ return IIO_VAL_INT;
+
+ default:
+ return -EINVAL;
+ }
+}
+
+static int sysmon_read_label(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ char *label)
+{
+ if (chan->datasheet_name)
+ return sysfs_emit(label, "%s\n", chan->datasheet_name);
+
+ return -EINVAL;
+}
+
+static const struct iio_info sysmon_iio_info = {
+ .read_raw = sysmon_read_raw,
+ .read_label = sysmon_read_label,
+};
+
+/**
+ * sysmon_parse_fw() - Parse firmware nodes and configure IIO channels.
+ * @indio_dev: IIO device instance
+ * @dev: Parent device
+ *
+ * Reads voltage-channels and temperature-channels container nodes from
+ * firmware and builds the IIO channel array. Static temperature channels
+ * are prepended, followed by supply and satellite channels from DT.
+ *
+ * Return: 0 on success, negative errno on failure.
+ */
+static int sysmon_parse_fw(struct iio_dev *indio_dev, struct device *dev)
+{
+ unsigned int num_chan, idx, temp_chan_idx, volt_chan_idx;
+ unsigned int num_supply, num_temp;
+ struct iio_chan_spec *sysmon_channels;
+ const char *label;
+ u32 reg;
+ int ret;
+
+ struct fwnode_handle *supply_node __free(fwnode_handle) =
+ device_get_named_child_node(dev, "voltage-channels");
+ num_supply = fwnode_get_child_node_count(supply_node);
+
+ struct fwnode_handle *temp_node __free(fwnode_handle) =
+ device_get_named_child_node(dev, "temperature-channels");
+ num_temp = fwnode_get_child_node_count(temp_node);
+
+ num_chan = size_add(num_temp, size_add(ARRAY_SIZE(temp_channels), num_supply));
+ sysmon_channels = devm_kcalloc(dev, num_chan, sizeof(*sysmon_channels), GFP_KERNEL);
+ if (!sysmon_channels)
+ return -ENOMEM;
+
+ /* Static temperature channels first */
+ memcpy(sysmon_channels, temp_channels, sizeof(temp_channels));
+ idx = ARRAY_SIZE(temp_channels);
+
+ /* Supply channels from DT */
+ fwnode_for_each_child_node_scoped(supply_node, child) {
+ ret = fwnode_property_read_u32(child, "reg", ®);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "missing reg for supply channel\n");
+
+ if (reg > SYSMON_SUPPLY_IDX_MAX)
+ return dev_err_probe(dev, -EINVAL,
+ "supply reg %u exceeds max %u\n",
+ reg, SYSMON_SUPPLY_IDX_MAX);
+
+ ret = fwnode_property_read_string(child, "label", &label);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "missing label for supply channel\n");
+
+ sysmon_channels[idx++] = (struct iio_chan_spec) {
+ .type = IIO_VOLTAGE,
+ .indexed = 1,
+ .address = reg,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+ .datasheet_name = label,
+ };
+ }
+
+ /* Temperature satellite channels from DT */
+ fwnode_for_each_child_node_scoped(temp_node, child) {
+ ret = fwnode_property_read_u32(child, "reg", ®);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "missing reg for temp channel\n");
+
+ if (reg < 1 || reg > SYSMON_TEMP_SAT_MAX)
+ return dev_err_probe(dev, -EINVAL,
+ "temp reg %u out of range [1..%u]\n",
+ reg, SYSMON_TEMP_SAT_MAX);
+
+ ret = fwnode_property_read_string(child, "label", &label);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "missing label for temp channel\n");
+
+ sysmon_channels[idx++] = (struct iio_chan_spec) {
+ .type = IIO_TEMP,
+ .indexed = 1,
+ .address = SYSMON_TEMP_SAT_BASE +
+ (reg - 1) * SYSMON_REG_STRIDE,
+ .info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
+ .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
+ .datasheet_name = label,
+ };
+ }
+
+ indio_dev->num_channels = idx;
+ indio_dev->info = &sysmon_iio_info;
+
+ /*
+ * Assign per-type sequential channel numbers.
+ * IIO sysfs uses type prefix (in_tempN, in_voltageN)
+ * so numbers only need to be unique within each type.
+ */
+ temp_chan_idx = 0;
+ volt_chan_idx = 0;
+ for (unsigned int idx = 0; idx < indio_dev->num_channels; idx++) {
+ if (sysmon_channels[idx].type == IIO_TEMP)
+ sysmon_channels[idx].channel = temp_chan_idx++;
+ else
+ sysmon_channels[idx].channel = volt_chan_idx++;
+ }
+
+ indio_dev->channels = sysmon_channels;
+
+ return 0;
+}
+
+/**
+ * devm_versal_sysmon_core_probe() - Initialize Versal SysMon core
+ * @dev: Parent device
+ * @regmap: Register map for hardware access
+ *
+ * Return: 0 on success, negative errno on failure.
+ */
+int devm_versal_sysmon_core_probe(struct device *dev, struct regmap *regmap)
+{
+ struct iio_dev *indio_dev;
+ struct sysmon *sysmon;
+ int ret;
+
+ indio_dev = devm_iio_device_alloc(dev, sizeof(*sysmon));
+ if (!indio_dev)
+ return -ENOMEM;
+
+ sysmon = iio_priv(indio_dev);
+ sysmon->regmap = regmap;
+
+ ret = devm_mutex_init(dev, &sysmon->lock);
+ if (ret)
+ return ret;
+
+ /* Disable all interrupts and clear pending status */
+ ret = regmap_write(sysmon->regmap, SYSMON_IDR, SYSMON_INTR_ALL_MASK);
+ if (ret)
+ return ret;
+ ret = regmap_write(sysmon->regmap, SYSMON_ISR, SYSMON_INTR_ALL_MASK);
+ if (ret)
+ return ret;
+
+ indio_dev->name = "versal-sysmon";
+ indio_dev->modes = INDIO_DIRECT_MODE;
+
+ ret = sysmon_parse_fw(indio_dev, dev);
+ if (ret)
+ return ret;
+
+ return devm_iio_device_register(dev, indio_dev);
+}
+EXPORT_SYMBOL_GPL(devm_versal_sysmon_core_probe);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("AMD Versal SysMon Core Driver");
+MODULE_AUTHOR("Salih Erim <salih.erim@amd.com>");
diff --git a/drivers/iio/adc/versal-sysmon.c b/drivers/iio/adc/versal-sysmon.c
new file mode 100644
index 00000000000..ee56e4e029b
--- /dev/null
+++ b/drivers/iio/adc/versal-sysmon.c
@@ -0,0 +1,93 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AMD Versal SysMon MMIO platform driver
+ *
+ * Copyright (C) 2019 - 2022, Xilinx, Inc.
+ * Copyright (C) 2022 - 2026, Advanced Micro Devices, Inc.
+ */
+
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/types.h>
+
+#include "versal-sysmon.h"
+
+struct sysmon_mmio {
+ void __iomem *base;
+};
+
+static int sysmon_mmio_reg_read(void *context, unsigned int reg,
+ unsigned int *val)
+{
+ struct sysmon_mmio *mmio = context;
+
+ *val = readl(mmio->base + reg);
+ return 0;
+}
+
+static int sysmon_mmio_reg_write(void *context, unsigned int reg,
+ unsigned int val)
+{
+ struct sysmon_mmio *mmio = context;
+
+ /* NPI must be unlocked before any register write except to NPI_LOCK */
+ if (reg != SYSMON_NPI_LOCK)
+ writel(SYSMON_NPI_UNLOCK_CODE, mmio->base + SYSMON_NPI_LOCK);
+ writel(val, mmio->base + reg);
+
+ return 0;
+}
+
+static const struct regmap_config sysmon_mmio_regmap_config = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = SYSMON_REG_STRIDE,
+ .max_register = SYSMON_MAX_REG,
+ .reg_read = sysmon_mmio_reg_read,
+ .reg_write = sysmon_mmio_reg_write,
+ .fast_io = true,
+};
+
+static int sysmon_platform_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct sysmon_mmio *mmio;
+ struct regmap *regmap;
+
+ mmio = devm_kzalloc(dev, sizeof(*mmio), GFP_KERNEL);
+ if (!mmio)
+ return -ENOMEM;
+
+ mmio->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(mmio->base))
+ return PTR_ERR(mmio->base);
+
+ regmap = devm_regmap_init(dev, NULL, mmio, &sysmon_mmio_regmap_config);
+ if (IS_ERR(regmap))
+ return PTR_ERR(regmap);
+
+ return devm_versal_sysmon_core_probe(dev, regmap);
+}
+
+static const struct of_device_id sysmon_of_match_table[] = {
+ { .compatible = "xlnx,versal-sysmon" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, sysmon_of_match_table);
+
+static struct platform_driver sysmon_platform_driver = {
+ .probe = sysmon_platform_probe,
+ .driver = {
+ .name = "versal-sysmon",
+ .of_match_table = sysmon_of_match_table,
+ },
+};
+module_platform_driver(sysmon_platform_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("AMD Versal SysMon Platform Driver");
+MODULE_AUTHOR("Salih Erim <salih.erim@amd.com>");
diff --git a/drivers/iio/adc/versal-sysmon.h b/drivers/iio/adc/versal-sysmon.h
new file mode 100644
index 00000000000..e27a5357575
--- /dev/null
+++ b/drivers/iio/adc/versal-sysmon.h
@@ -0,0 +1,67 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * AMD Versal SysMon driver
+ *
+ * Copyright (C) 2019 - 2022, Xilinx, Inc.
+ * Copyright (C) 2022 - 2026, Advanced Micro Devices, Inc.
+ */
+
+#ifndef _VERSAL_SYSMON_H_
+#define _VERSAL_SYSMON_H_
+
+#include <linux/bits.h>
+#include <linux/mutex.h>
+
+struct device;
+struct regmap;
+
+/* Register offsets (sorted by address) */
+#define SYSMON_NPI_LOCK 0x000C
+#define SYSMON_ISR 0x0044
+#define SYSMON_IDR 0x0050
+#define SYSMON_TEMP_MAX 0x1030
+#define SYSMON_TEMP_MIN 0x1034
+#define SYSMON_SUPPLY_BASE 0x1040
+#define SYSMON_TEMP_MIN_MIN 0x1F8C
+#define SYSMON_TEMP_MAX_MAX 0x1F90
+#define SYSMON_TEMP_SAT_BASE 0x1FAC
+#define SYSMON_MAX_REG 0x24C0
+
+/* NPI unlock value written to SYSMON_NPI_LOCK */
+#define SYSMON_NPI_UNLOCK_CODE 0xF9E8D7C6
+
+/* Register stride: 4 bytes per 32-bit register */
+#define SYSMON_REG_STRIDE 4
+
+#define SYSMON_SUPPLY_IDX_MAX 159
+#define SYSMON_TEMP_SAT_MAX 64
+#define SYSMON_INTR_ALL_MASK GENMASK(31, 0)
+
+/* Supply voltage conversion register fields */
+#define SYSMON_MANTISSA_MASK GENMASK(15, 0)
+#define SYSMON_FMT_MASK BIT(16)
+#define SYSMON_MODE_MASK GENMASK(18, 17)
+
+/* Q8.7 fractional shift */
+#define SYSMON_FRACTIONAL_SHIFT 7U
+#define SYSMON_SUPPLY_MANTISSA_BITS 16
+
+/**
+ * struct sysmon - Driver data for Versal SysMon
+ * @regmap: register map for hardware access
+ * @lock: protects read-modify-write sequences on threshold registers
+ * and cached state that spans multiple regmap calls
+ */
+struct sysmon {
+ struct regmap *regmap;
+ /*
+ * Protects read-modify-write sequences on threshold registers
+ * and cached state (oversampling ratios, hysteresis values)
+ * that spans multiple regmap calls.
+ */
+ struct mutex lock;
+};
+
+int devm_versal_sysmon_core_probe(struct device *dev, struct regmap *regmap);
+
+#endif /* _VERSAL_SYSMON_H_ */
--
2.48.1
^ permalink raw reply related
* [PATCH v8 4/5] iio: adc: versal-sysmon: add threshold event support
From: Salih Erim @ 2026-06-16 13:15 UTC (permalink / raw)
To: jic23, andy
Cc: dlechner, nuno.sa, robh, krzk+dt, conor+dt, conall.ogriofa,
michal.simek, linux, erimsalih, linux-iio, devicetree,
linux-kernel, Salih Erim
In-Reply-To: <20260616131559.3029543-1-salih.erim@amd.com>
Add threshold event support for temperature and supply voltage
channels.
Temperature events:
- Rising threshold with configurable value on the device
temperature channel (current max across all satellites)
- Per-channel hysteresis as a millicelsius value
- Event direction is IIO_EV_DIR_RISING (hysteresis mode)
Supply voltage events:
- Rising/falling threshold per supply channel
- Per-channel alarm enable via alarm configuration registers
The hardware supports both window and hysteresis alarm modes for
temperature. This driver uses hysteresis mode, where the upper
threshold triggers the alarm and the lower threshold clears it
(re-arm point). The hardware has a single ISR bit per temperature
channel with no indication of which threshold was crossed, so
hysteresis mode is the natural fit. The lower threshold register
is computed internally as (upper - hysteresis).
Hysteresis is stored in the driver as a millicelsius value,
initialized from the hardware registers at probe. Writing the
rising threshold or hysteresis recomputes the lower register.
ALARM_CONFIG is hard-coded to hysteresis mode during init.
The hardware also provides a separate over-temperature (OT)
threshold, but it is not exposed through IIO as it serves as a
hardware safety mechanism for platform shutdown. OT will be
exposed through the thermal framework in a follow-up series.
The interrupt handler masks active threshold interrupts (which are
level-sensitive) and schedules a delayed worker to poll for condition
clear before unmasking. When no hardware IRQ is available, event
specs are not attached and interrupt init is skipped, since the
I2C regmap backend cannot be called from atomic context.
When disabling a supply channel alarm, the group interrupt remains
active if any other channel in the same alarm group still has an
alarm enabled.
A devm cleanup action masks all interrupts on driver unbind to
prevent unhandled interrupt storms after the IRQ handler is freed.
Signed-off-by: Salih Erim <salih.erim@amd.com>
---
Changes in v8:
- Use MILLIDEGREE_PER_DEGREE in q8p7 conversion functions (Andy)
- Use regmap_test_bits() in sysmon_read_alarm_config (Andy)
- Join sysmon_parse_fw signature onto one line (Andy)
- Fix devm teardown race: replace devm_delayed_work_autocancel
with INIT_DELAYED_WORK; fold cancel_delayed_work_sync into
sysmon_disable_interrupts to prevent the worker from
re-enabling interrupts after the IRQ handler is freed (Sashiko)
- Drop devm-helpers.h include (no longer needed)
Changes in v7:
- Move TEMP threshold event onto channel 0; drop OT as
separate IIO channel -- OT is a hardware safety mechanism
better suited for the thermal framework follow-up (Jonathan)
- Use single temp_channels array; attach event spec to
channel 0 at runtime when IRQ is available, matching the
pattern used for supply channels (Jonathan)
- Remove sysmon_temp_thresh_offset; use SYSMON_TEMP_TH_UP
and SYSMON_TEMP_TH_LOW defines directly at call sites
- Return administrative state from temp_mask in
read_event_config instead of transient hardware IMR
(Jonathan, Sashiko)
- Add devm_add_action_or_reset to mask all HW interrupts
on driver unbind (Sashiko)
- Remove SYSMON_CHAN_TEMP_EVENT macro, SYSMON_ADDR_TEMP_EVENT,
SYSMON_ADDR_OT_EVENT, SYSMON_BIT_OT, SYSMON_OT_HYST_MASK,
OT_TH_LOW/UP registers, ot_hysteresis from struct
- Simplify sysmon_get_event_mask, sysmon_update_temp_lower,
sysmon_init_hysteresis -- all now operate on single TEMP
channel only
Changes in v6:
- Remove types.h from header (not needed at any stage) (Andy)
- Macro brace on separate line for SYSMON_CHAN_TEMP_EVENT (Andy)
- switch(chan->type) in all event functions instead of cascading
if statements (Andy)
- switch(info) in read/write_event_value for nested
dispatch (Andy)
- Reversed xmas tree in sysmon_update_temp_lower and
sysmon_init_hysteresis (Andy)
- scoped_guard(spinlock_irq) with error check in
sysmon_unmask_worker (Andy)
- Combined regmap_read error check with || in
sysmon_iio_irq (Andy)
- Join devm_request_irq on one line (Andy)
- Fix fwnode_irq_get() to propagate only -EPROBE_DEFER;
treating all negatives as fatal broke probe on I2C nodes
without interrupts property
Changes in v5:
- clamp() instead of clamp_t() (Andy)
- regmap_assign_bits() instead of separate set/clear (Andy)
- Remove unneeded parentheses (2 places) (Andy)
- for_each_set_bit on single line (Andy)
- regmap_clear_bits() instead of regmap_update_bits() (Andy)
- Simplify unmask XOR to ~status & masked_temp (Andy)
- Add comment explaining unmask &= ~temp_mask logic (Andy)
- Split container_of across two lines (Andy)
- Move ISR write after !isr check to avoid writing 0 (Andy)
- unsigned int for init_hysteresis address param (Andy)
- Add comment explaining error check policy in worker/IRQ (Andy)
- Nested size_add() for overflow-safe allocation (Andy)
- Propagate negative from fwnode_irq_get() for
EPROBE_DEFER (Andy)
- Pass irq instead of has_irq to sysmon_parse_fw (Andy)
Changes in v4:
- Merge event channels into static temp array; two arrays
(with/without events) selected by has_irq (Jonathan)
- Event-only channels have no info_mask; their addresses are
logical identifiers, not readable registers
- Drop RAW for voltage events, keep PROCESSED only (Jonathan)
- Drop scan_type from event channel macro (Jonathan)
- Blank lines between call+error-check blocks (Jonathan)
- Fit under 80 chars on one line where possible (Jonathan)
- default case returns -EINVAL instead of break (Jonathan)
- sysmon_handle_event: return early in each case (Jonathan)
- guard(spinlock) in sysmon_iio_irq, return IRQ_NONE/IRQ_HANDLED
directly (Jonathan)
- Take irq_lock in write_event_config for temp_mask updates to
synchronize with unmask worker (Sashiko)
Changes in v3:
- IWYU: add new includes, group iio headers with blank line (Andy)
- Reduce casts in millicelsius_to_q8p7, consistent style with
q8p7_to_millicelsius (Andy)
- Use clamp_t with typed constants, remove tmp & U16_MAX (Andy)
- Use !! to return 0/1 from read_alarm_config (Andy)
- Use regmap_set_bits/clear_bits in write_alarm_config (Andy)
- Add comment explaining spinlock is safe (I2C never reaches
event code path) (Andy)
- Add comment explaining IMR negation logic (Andy)
- Split read_event_value/write_event_value parameters logically
across lines (Andy)
- Move mask/shift after regmap_read error check (Andy)
- Remove redundant else in read_event_value and
write_event_value (Andy)
- Use named constant for hysteresis bit, if-else not ternary
(Andy)
- Loop variable declared in for() scope (Andy)
- Add error checks in sysmon_handle_event (Andy)
- Use IRQ_RETVAL() macro (Andy)
- Use devm_delayed_work_autocancel instead of manual INIT +
devm_add_action (Andy)
- Use FIELD_GET/FIELD_PREP for hysteresis register bits
(Jonathan)
- Split OT vs TEMP handling with FIELD_GET (Jonathan)
- Rework hysteresis: store as millicelsius value, hardcode
ALARM_CONFIG to hysteresis mode, compute lower threshold
from (upper - hysteresis), initialize from HW at probe
(Jonathan)
- Remove falling threshold for temperature; single event
spec per channel with IIO_EV_DIR_RISING (Jonathan)
- Push IIO_EV_DIR_RISING events for temperature,
IIO_EV_DIR_EITHER for voltage (Jonathan)
Changes in v2:
- Reverse Christmas Tree variable ordering in all functions
- Named constants for hysteresis bits: SYSMON_OT_HYST_BIT,
SYSMON_TEMP_HYST_BIT instead of magic 0x1/0x2
- SYSMON_ALARM_BITS_PER_REG replaces magic number 32
- SYSMON_ALARM_OFFSET() helper macro deduplicates alarm register
offset computation
- BIT() macro for shift expressions in conversion functions
- Hysteresis input validated to single-bit range (0 or 1)
- Event channels only created when irq > 0 (I2C safety)
- Group alarm interrupt stays active while any channel in the
group has an alarm enabled
- write_event_value returns -EINVAL for unhandled types
- IRQ_NONE returned for spurious interrupts
- Q8.7 write path uses multiplication instead of left-shift
to avoid undefined behavior with negative temperatures
- (u16) mask prevents garbage in reserved register bits
- regmap_write return values checked for IER/IDR writes
- devm cleanup ordering: cancel_work before request_irq
drivers/iio/adc/versal-sysmon-core.c | 610 ++++++++++++++++++++++++++-
drivers/iio/adc/versal-sysmon.h | 36 ++
2 files changed, 638 insertions(+), 8 deletions(-)
diff --git a/drivers/iio/adc/versal-sysmon-core.c b/drivers/iio/adc/versal-sysmon-core.c
index a1d4a13b724..8d567f9fa05 100644
--- a/drivers/iio/adc/versal-sysmon-core.c
+++ b/drivers/iio/adc/versal-sysmon-core.c
@@ -12,6 +12,7 @@
#include <linux/cleanup.h>
#include <linux/device.h>
#include <linux/err.h>
+#include <linux/interrupt.h>
#include <linux/module.h>
#include <linux/overflow.h>
#include <linux/property.h>
@@ -20,10 +21,18 @@
#include <linux/sysfs.h>
#include <linux/units.h>
+#include <linux/iio/events.h>
#include <linux/iio/iio.h>
#include "versal-sysmon.h"
+/* TEMP hysteresis mode bit in SYSMON_TEMP_EV_CFG */
+#define SYSMON_TEMP_HYST_MASK BIT(1)
+
+/* Compute alarm register offset from a channel address */
+#define SYSMON_ALARM_OFFSET(addr) \
+ (SYSMON_ALARM_REG + ((addr) / SYSMON_ALARM_BITS_PER_REG) * SYSMON_REG_STRIDE)
+
#define SYSMON_CHAN_TEMP(_chan, _address, _name) \
{ \
.type = IIO_TEMP, \
@@ -35,6 +44,45 @@
.datasheet_name = _name, \
}
+enum sysmon_alarm_bit {
+ SYSMON_BIT_ALARM0 = 0,
+ SYSMON_BIT_ALARM1 = 1,
+ SYSMON_BIT_ALARM2 = 2,
+ SYSMON_BIT_ALARM3 = 3,
+ SYSMON_BIT_ALARM4 = 4,
+ SYSMON_BIT_TEMP = 9,
+};
+
+/* Temperature event specification: rising threshold + hysteresis only */
+static const struct iio_event_spec sysmon_temp_events[] = {
+ {
+ .type = IIO_EV_TYPE_THRESH,
+ .dir = IIO_EV_DIR_RISING,
+ .mask_separate = BIT(IIO_EV_INFO_ENABLE) |
+ BIT(IIO_EV_INFO_VALUE) |
+ BIT(IIO_EV_INFO_HYSTERESIS),
+ },
+};
+
+/* Supply event specifications */
+static const struct iio_event_spec sysmon_supply_events[] = {
+ {
+ .type = IIO_EV_TYPE_THRESH,
+ .dir = IIO_EV_DIR_RISING,
+ .mask_separate = BIT(IIO_EV_INFO_VALUE),
+ },
+ {
+ .type = IIO_EV_TYPE_THRESH,
+ .dir = IIO_EV_DIR_FALLING,
+ .mask_separate = BIT(IIO_EV_INFO_VALUE),
+ },
+ {
+ .type = IIO_EV_TYPE_THRESH,
+ .dir = IIO_EV_DIR_EITHER,
+ .mask_separate = BIT(IIO_EV_INFO_ENABLE),
+ },
+};
+
/*
* Static temperature channels (always present).
*
@@ -52,6 +100,16 @@ static const struct iio_chan_spec temp_channels[] = {
SYSMON_CHAN_TEMP(3, SYSMON_TEMP_MIN_MIN, "min_min"),
};
+static void sysmon_q8p7_to_millicelsius(s16 raw_data, int *val)
+{
+ *val = (raw_data * MILLIDEGREE_PER_DEGREE) >> SYSMON_FRACTIONAL_SHIFT;
+}
+
+static void sysmon_millicelsius_to_q8p7(u32 *raw_data, int val)
+{
+ *raw_data = (val << SYSMON_FRACTIONAL_SHIFT) / MILLIDEGREE_PER_DEGREE;
+}
+
static void sysmon_supply_rawtoprocessed(int raw_data, int *val)
{
int mantissa, format, exponent;
@@ -69,6 +127,34 @@ static void sysmon_supply_rawtoprocessed(int raw_data, int *val)
*val = (mantissa * (int)MILLI) >> exponent;
}
+static void sysmon_supply_processedtoraw(int val, u32 reg_val, u32 *raw_data)
+{
+ int exponent = FIELD_GET(SYSMON_MODE_MASK, reg_val);
+ int format = FIELD_GET(SYSMON_FMT_MASK, reg_val);
+ int scale, tmp;
+
+ scale = BIT(SYSMON_SUPPLY_MANTISSA_BITS - exponent);
+ tmp = (val * scale) / (int)MILLI;
+
+ if (format)
+ tmp = clamp(tmp, S16_MIN, S16_MAX);
+ else
+ tmp = clamp(tmp, 0, U16_MAX);
+
+ *raw_data = (u16)tmp;
+}
+
+static int sysmon_supply_thresh_offset(int address,
+ enum iio_event_direction dir)
+{
+ if (dir == IIO_EV_DIR_RISING)
+ return (address * SYSMON_REG_STRIDE) + SYSMON_SUPPLY_TH_UP;
+ if (dir == IIO_EV_DIR_FALLING)
+ return (address * SYSMON_REG_STRIDE) + SYSMON_SUPPLY_TH_LOW;
+
+ return -EINVAL;
+}
+
static int sysmon_read_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
int *val, int *val2, long mask)
@@ -115,6 +201,259 @@ static int sysmon_read_raw(struct iio_dev *indio_dev,
}
}
+static u32 sysmon_get_event_mask(const struct iio_chan_spec *chan)
+{
+ if (chan->type == IIO_TEMP)
+ return BIT(SYSMON_BIT_TEMP);
+
+ return BIT(chan->address / SYSMON_ALARM_BITS_PER_REG);
+}
+
+static int sysmon_read_alarm_config(struct sysmon *sysmon,
+ unsigned long address)
+{
+ u32 shift = address % SYSMON_ALARM_BITS_PER_REG;
+ u32 offset = SYSMON_ALARM_OFFSET(address);
+
+ return regmap_test_bits(sysmon->regmap, offset, BIT(shift));
+}
+
+static int sysmon_write_alarm_config(struct sysmon *sysmon,
+ unsigned long address, bool enable)
+{
+ u32 shift = address % SYSMON_ALARM_BITS_PER_REG;
+ u32 offset = SYSMON_ALARM_OFFSET(address);
+
+ return regmap_assign_bits(sysmon->regmap, offset, BIT(shift), enable);
+}
+
+static int sysmon_read_event_config(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir)
+{
+ struct sysmon *sysmon = iio_priv(indio_dev);
+ unsigned int imr;
+ int config_value;
+ u32 mask;
+ int ret;
+
+ mask = sysmon_get_event_mask(chan);
+
+ ret = regmap_read(sysmon->regmap, SYSMON_IMR, &imr);
+ if (ret)
+ return ret;
+
+ /* IMR bits are 1=masked, invert to get 1=enabled */
+ imr = ~imr;
+
+ switch (chan->type) {
+ case IIO_VOLTAGE:
+ config_value = sysmon_read_alarm_config(sysmon, chan->address);
+ if (config_value < 0)
+ return config_value;
+ return config_value && (imr & mask);
+
+ case IIO_TEMP:
+ /*
+ * Return the administrative state, not the hardware IMR.
+ * The IRQ handler temporarily masks the interrupt during
+ * the polling window; reading IMR would show it as disabled.
+ * temp_mask bit is set when administratively disabled.
+ */
+ return !(sysmon->temp_mask & mask);
+
+ default:
+ return -EINVAL;
+ }
+}
+
+static int sysmon_write_event_config(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ bool state)
+{
+ u32 offset = SYSMON_ALARM_OFFSET(chan->address);
+ struct sysmon *sysmon = iio_priv(indio_dev);
+ u32 ier = sysmon_get_event_mask(chan);
+ unsigned int alarm_config;
+ int ret;
+
+ guard(mutex)(&sysmon->lock);
+
+ switch (chan->type) {
+ case IIO_VOLTAGE:
+ ret = sysmon_write_alarm_config(sysmon, chan->address, state);
+ if (ret)
+ return ret;
+
+ ret = regmap_read(sysmon->regmap, offset, &alarm_config);
+ if (ret)
+ return ret;
+
+ if (alarm_config)
+ return regmap_write(sysmon->regmap, SYSMON_IER, ier);
+
+ return regmap_write(sysmon->regmap, SYSMON_IDR, ier);
+
+ case IIO_TEMP:
+ if (state) {
+ ret = regmap_write(sysmon->regmap, SYSMON_IER, ier);
+ if (ret)
+ return ret;
+
+ scoped_guard(spinlock_irq, &sysmon->irq_lock)
+ sysmon->temp_mask &= ~ier;
+ } else {
+ ret = regmap_write(sysmon->regmap, SYSMON_IDR, ier);
+ if (ret)
+ return ret;
+
+ scoped_guard(spinlock_irq, &sysmon->irq_lock)
+ sysmon->temp_mask |= ier;
+ }
+ return 0;
+
+ default:
+ return -EINVAL;
+ }
+}
+
+/*
+ * Recompute the lower threshold register from upper threshold and
+ * cached hysteresis. Called when either upper threshold or hysteresis
+ * is written.
+ */
+static int sysmon_update_temp_lower(struct sysmon *sysmon)
+{
+ unsigned int upper_reg;
+ int upper_mc, lower_mc;
+ u32 raw_val;
+ int ret;
+
+ ret = regmap_read(sysmon->regmap, SYSMON_TEMP_TH_UP, &upper_reg);
+ if (ret)
+ return ret;
+
+ sysmon_q8p7_to_millicelsius(upper_reg, &upper_mc);
+
+ lower_mc = upper_mc - sysmon->temp_hysteresis;
+ sysmon_millicelsius_to_q8p7(&raw_val, lower_mc);
+
+ return regmap_write(sysmon->regmap, SYSMON_TEMP_TH_LOW, raw_val);
+}
+
+static int sysmon_read_event_value(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ enum iio_event_info info,
+ int *val, int *val2)
+{
+ struct sysmon *sysmon = iio_priv(indio_dev);
+ unsigned int reg_val;
+ int offset;
+ int ret;
+
+ guard(mutex)(&sysmon->lock);
+
+ switch (chan->type) {
+ case IIO_TEMP:
+ switch (info) {
+ case IIO_EV_INFO_VALUE:
+ ret = regmap_read(sysmon->regmap, SYSMON_TEMP_TH_UP, ®_val);
+ if (ret)
+ return ret;
+
+ sysmon_q8p7_to_millicelsius(reg_val, val);
+
+ return IIO_VAL_INT;
+
+ case IIO_EV_INFO_HYSTERESIS:
+ *val = sysmon->temp_hysteresis;
+ return IIO_VAL_INT;
+
+ default:
+ return -EINVAL;
+ }
+
+ case IIO_VOLTAGE:
+ offset = sysmon_supply_thresh_offset(chan->address, dir);
+ if (offset < 0)
+ return offset;
+
+ ret = regmap_read(sysmon->regmap, offset, ®_val);
+ if (ret)
+ return ret;
+
+ sysmon_supply_rawtoprocessed(reg_val, val);
+
+ return IIO_VAL_INT;
+
+ default:
+ return -EINVAL;
+ }
+}
+
+static int sysmon_write_event_value(struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan,
+ enum iio_event_type type,
+ enum iio_event_direction dir,
+ enum iio_event_info info,
+ int val, int val2)
+{
+ struct sysmon *sysmon = iio_priv(indio_dev);
+ unsigned int reg_val;
+ u32 raw_val;
+ int offset;
+ int ret;
+
+ guard(mutex)(&sysmon->lock);
+
+ switch (chan->type) {
+ case IIO_TEMP:
+ switch (info) {
+ case IIO_EV_INFO_VALUE:
+ sysmon_millicelsius_to_q8p7(&raw_val, val);
+
+ ret = regmap_write(sysmon->regmap, SYSMON_TEMP_TH_UP, raw_val);
+ if (ret)
+ return ret;
+
+ /* Recompute lower = upper - hysteresis */
+ return sysmon_update_temp_lower(sysmon);
+
+ case IIO_EV_INFO_HYSTERESIS:
+ if (val < 0)
+ return -EINVAL;
+
+ sysmon->temp_hysteresis = val;
+
+ return sysmon_update_temp_lower(sysmon);
+
+ default:
+ return -EINVAL;
+ }
+
+ case IIO_VOLTAGE:
+ offset = sysmon_supply_thresh_offset(chan->address, dir);
+ if (offset < 0)
+ return offset;
+
+ ret = regmap_read(sysmon->regmap, offset, ®_val);
+ if (ret)
+ return ret;
+
+ sysmon_supply_processedtoraw(val, reg_val, &raw_val);
+
+ return regmap_write(sysmon->regmap, offset, raw_val);
+
+ default:
+ return -EINVAL;
+ }
+}
+
static int sysmon_read_label(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
char *label)
@@ -128,24 +467,245 @@ static int sysmon_read_label(struct iio_dev *indio_dev,
static const struct iio_info sysmon_iio_info = {
.read_raw = sysmon_read_raw,
.read_label = sysmon_read_label,
+ .read_event_config = sysmon_read_event_config,
+ .write_event_config = sysmon_write_event_config,
+ .read_event_value = sysmon_read_event_value,
+ .write_event_value = sysmon_write_event_value,
};
+static void sysmon_push_event(struct iio_dev *indio_dev, u32 address)
+{
+ const struct iio_chan_spec *chan;
+ enum iio_event_direction dir;
+
+ for (unsigned int i = 0; i < indio_dev->num_channels; i++) {
+ if (indio_dev->channels[i].address != address)
+ continue;
+
+ chan = &indio_dev->channels[i];
+ /* Temp uses hysteresis mode (rising only), voltage uses window */
+ dir = (chan->type == IIO_TEMP) ? IIO_EV_DIR_RISING :
+ IIO_EV_DIR_EITHER;
+ iio_push_event(indio_dev,
+ IIO_UNMOD_EVENT_CODE(chan->type,
+ chan->channel,
+ IIO_EV_TYPE_THRESH,
+ dir),
+ iio_get_time_ns(indio_dev));
+ }
+}
+
+static int sysmon_handle_event(struct iio_dev *indio_dev, u32 event)
+{
+ u32 alarm_flag_offset = SYSMON_ALARM_FLAG + event * SYSMON_REG_STRIDE;
+ u32 alarm_reg_offset = SYSMON_ALARM_REG + event * SYSMON_REG_STRIDE;
+ struct sysmon *sysmon = iio_priv(indio_dev);
+ unsigned long alarm_flag_reg;
+ unsigned int reg_val;
+ u32 address, bit;
+ int ret;
+
+ switch (event) {
+ case SYSMON_BIT_TEMP:
+ sysmon_push_event(indio_dev, SYSMON_TEMP_MAX);
+
+ ret = regmap_write(sysmon->regmap, SYSMON_IDR, BIT(SYSMON_BIT_TEMP));
+ if (ret)
+ return ret;
+
+ sysmon->masked_temp |= BIT(SYSMON_BIT_TEMP);
+ return 0;
+
+ case SYSMON_BIT_ALARM0:
+ case SYSMON_BIT_ALARM1:
+ case SYSMON_BIT_ALARM2:
+ case SYSMON_BIT_ALARM3:
+ case SYSMON_BIT_ALARM4:
+ ret = regmap_read(sysmon->regmap, alarm_flag_offset, ®_val);
+ if (ret)
+ return ret;
+
+ alarm_flag_reg = reg_val;
+
+ for_each_set_bit(bit, &alarm_flag_reg, SYSMON_ALARM_BITS_PER_REG) {
+ address = bit + SYSMON_ALARM_BITS_PER_REG * event;
+ sysmon_push_event(indio_dev, address);
+ ret = regmap_clear_bits(sysmon->regmap, alarm_reg_offset, BIT(bit));
+ if (ret)
+ return ret;
+ }
+
+ return regmap_write(sysmon->regmap, alarm_flag_offset, alarm_flag_reg);
+
+ default:
+ return -EINVAL;
+ }
+}
+
+static void sysmon_handle_events(struct iio_dev *indio_dev,
+ unsigned long events)
+{
+ unsigned int bit;
+
+ for_each_set_bit(bit, &events, SYSMON_NO_OF_EVENTS)
+ sysmon_handle_event(indio_dev, bit);
+}
+
+static void sysmon_unmask_temp(struct sysmon *sysmon, unsigned int isr)
+{
+ unsigned int unmask, status;
+
+ status = isr & SYSMON_TEMP_INTR_MASK;
+
+ unmask = ~status & sysmon->masked_temp;
+ sysmon->masked_temp &= status;
+
+ /* Only unmask if not administratively disabled by userspace */
+ unmask &= ~sysmon->temp_mask;
+
+ regmap_write(sysmon->regmap, SYSMON_IER, unmask);
+}
+
+/*
+ * Versal threshold interrupts are level-sensitive. Active threshold
+ * interrupts are masked in the handler and polled via delayed work
+ * until the condition clears, then unmasked.
+ */
+static void sysmon_unmask_worker(struct work_struct *work)
+{
+ struct sysmon *sysmon =
+ container_of(work, struct sysmon, sysmon_unmask_work.work);
+ unsigned int isr;
+
+ /*
+ * If the ISR read fails, skip processing to avoid acting
+ * on undefined data.
+ */
+ scoped_guard(spinlock_irq, &sysmon->irq_lock) {
+ if (regmap_read(sysmon->regmap, SYSMON_ISR, &isr))
+ break;
+ regmap_write(sysmon->regmap, SYSMON_ISR, isr);
+ sysmon_unmask_temp(sysmon, isr);
+ }
+
+ if (sysmon->masked_temp)
+ schedule_delayed_work(&sysmon->sysmon_unmask_work,
+ msecs_to_jiffies(SYSMON_UNMASK_WORK_DELAY_MS));
+ else
+ regmap_write(sysmon->regmap, SYSMON_STATUS_RESET, 1);
+}
+
+static irqreturn_t sysmon_iio_irq(int irq, void *data)
+{
+ struct iio_dev *indio_dev = data;
+ struct sysmon *sysmon = iio_priv(indio_dev);
+ unsigned int isr, imr;
+
+ guard(spinlock)(&sysmon->irq_lock);
+
+ if (regmap_read(sysmon->regmap, SYSMON_ISR, &isr) ||
+ regmap_read(sysmon->regmap, SYSMON_IMR, &imr))
+ return IRQ_NONE;
+
+ isr &= ~imr;
+ if (!isr)
+ return IRQ_NONE;
+
+ regmap_write(sysmon->regmap, SYSMON_ISR, isr);
+
+ sysmon_handle_events(indio_dev, isr);
+ schedule_delayed_work(&sysmon->sysmon_unmask_work,
+ msecs_to_jiffies(SYSMON_UNMASK_WORK_DELAY_MS));
+
+ return IRQ_HANDLED;
+}
+
+static void sysmon_disable_interrupts(void *data)
+{
+ struct sysmon *sysmon = data;
+
+ regmap_write(sysmon->regmap, SYSMON_IDR, SYSMON_INTR_ALL_MASK);
+
+ scoped_guard(spinlock_irq, &sysmon->irq_lock)
+ sysmon->masked_temp = 0;
+
+ cancel_delayed_work_sync(&sysmon->sysmon_unmask_work);
+}
+
+static int sysmon_init_interrupt(struct sysmon *sysmon,
+ struct device *dev,
+ struct iio_dev *indio_dev,
+ int irq)
+{
+ unsigned int imr;
+ int ret;
+
+ /* Events not supported without IRQ (e.g. I2C path) */
+ if (!irq)
+ return 0;
+
+ INIT_DELAYED_WORK(&sysmon->sysmon_unmask_work, sysmon_unmask_worker);
+
+ ret = regmap_read(sysmon->regmap, SYSMON_IMR, &imr);
+ if (ret)
+ return ret;
+ sysmon->temp_mask = imr & SYSMON_TEMP_INTR_MASK;
+
+ ret = devm_request_irq(dev, irq, sysmon_iio_irq, 0, "sysmon-irq", indio_dev);
+ if (ret)
+ return ret;
+
+ return devm_add_action_or_reset(dev, sysmon_disable_interrupts, sysmon);
+}
+
+/*
+ * Initialize the cached hysteresis for a temperature channel from the
+ * current hardware threshold registers: hysteresis = upper - lower.
+ */
+static int sysmon_init_hysteresis(struct sysmon *sysmon, int *hysteresis)
+{
+ unsigned int upper_reg, lower_reg;
+ int upper_mc, lower_mc;
+ int ret;
+
+ ret = regmap_read(sysmon->regmap, SYSMON_TEMP_TH_UP, &upper_reg);
+ if (ret)
+ return ret;
+
+ ret = regmap_read(sysmon->regmap, SYSMON_TEMP_TH_LOW, &lower_reg);
+ if (ret)
+ return ret;
+
+ sysmon_q8p7_to_millicelsius(upper_reg, &upper_mc);
+ sysmon_q8p7_to_millicelsius(lower_reg, &lower_mc);
+ *hysteresis = upper_mc - lower_mc;
+
+ return 0;
+}
+
/**
* sysmon_parse_fw() - Parse firmware nodes and configure IIO channels.
* @indio_dev: IIO device instance
* @dev: Parent device
+ * @irq: IRQ number (positive enables event channels, 0 disables)
*
* Reads voltage-channels and temperature-channels container nodes from
* firmware and builds the IIO channel array. Static temperature channels
- * are prepended, followed by supply and satellite channels from DT.
+ * and event channels are prepended, followed by supply and satellite
+ * channels from DT.
+ *
+ * Event channels and per-channel event specs are only added when the
+ * device has an IRQ. I2C devices have no interrupt line, and the I2C
+ * regmap cannot be called from atomic context, so events are not
+ * supported on that path.
*
* Return: 0 on success, negative errno on failure.
*/
-static int sysmon_parse_fw(struct iio_dev *indio_dev, struct device *dev)
+static int sysmon_parse_fw(struct iio_dev *indio_dev, struct device *dev, int irq)
{
- unsigned int num_chan, idx, temp_chan_idx, volt_chan_idx;
- unsigned int num_supply, num_temp;
+ unsigned int num_chan, num_static, idx, temp_chan_idx, volt_chan_idx;
struct iio_chan_spec *sysmon_channels;
+ unsigned int num_supply, num_temp;
const char *label;
u32 reg;
int ret;
@@ -158,14 +718,21 @@ static int sysmon_parse_fw(struct iio_dev *indio_dev, struct device *dev)
device_get_named_child_node(dev, "temperature-channels");
num_temp = fwnode_get_child_node_count(temp_node);
- num_chan = size_add(num_temp, size_add(ARRAY_SIZE(temp_channels), num_supply));
+ num_static = ARRAY_SIZE(temp_channels);
+ num_chan = size_add(num_temp, size_add(num_static, num_supply));
sysmon_channels = devm_kcalloc(dev, num_chan, sizeof(*sysmon_channels), GFP_KERNEL);
if (!sysmon_channels)
return -ENOMEM;
- /* Static temperature channels first */
memcpy(sysmon_channels, temp_channels, sizeof(temp_channels));
- idx = ARRAY_SIZE(temp_channels);
+
+ /* Attach event spec to channel 0 when IRQ is available */
+ if (irq > 0) {
+ sysmon_channels[0].event_spec = sysmon_temp_events;
+ sysmon_channels[0].num_event_specs = ARRAY_SIZE(sysmon_temp_events);
+ }
+
+ idx = num_static;
/* Supply channels from DT */
fwnode_for_each_child_node_scoped(supply_node, child) {
@@ -189,6 +756,10 @@ static int sysmon_parse_fw(struct iio_dev *indio_dev, struct device *dev)
.indexed = 1,
.address = reg,
.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+ .event_spec = irq > 0 ?
+ sysmon_supply_events : NULL,
+ .num_event_specs = irq > 0 ?
+ ARRAY_SIZE(sysmon_supply_events) : 0,
.datasheet_name = label,
};
}
@@ -254,6 +825,7 @@ int devm_versal_sysmon_core_probe(struct device *dev, struct regmap *regmap)
{
struct iio_dev *indio_dev;
struct sysmon *sysmon;
+ int irq;
int ret;
indio_dev = devm_iio_device_alloc(dev, sizeof(*sysmon));
@@ -266,6 +838,7 @@ int devm_versal_sysmon_core_probe(struct device *dev, struct regmap *regmap)
ret = devm_mutex_init(dev, &sysmon->lock);
if (ret)
return ret;
+ spin_lock_init(&sysmon->irq_lock);
/* Disable all interrupts and clear pending status */
ret = regmap_write(sysmon->regmap, SYSMON_IDR, SYSMON_INTR_ALL_MASK);
@@ -275,13 +848,34 @@ int devm_versal_sysmon_core_probe(struct device *dev, struct regmap *regmap)
if (ret)
return ret;
+ irq = fwnode_irq_get(dev_fwnode(dev), 0);
+ if (irq == -EPROBE_DEFER)
+ return dev_err_probe(dev, irq, "failed to get IRQ\n");
+
indio_dev->name = "versal-sysmon";
indio_dev->modes = INDIO_DIRECT_MODE;
- ret = sysmon_parse_fw(indio_dev, dev);
+ ret = sysmon_parse_fw(indio_dev, dev, irq);
if (ret)
return ret;
+ if (irq > 0) {
+ /* Set hysteresis mode for temperature threshold */
+ ret = regmap_set_bits(sysmon->regmap, SYSMON_TEMP_EV_CFG,
+ SYSMON_TEMP_HYST_MASK);
+ if (ret)
+ return ret;
+
+ /* Initialize cached hysteresis from hardware registers */
+ ret = sysmon_init_hysteresis(sysmon, &sysmon->temp_hysteresis);
+ if (ret)
+ return ret;
+
+ ret = sysmon_init_interrupt(sysmon, dev, indio_dev, irq);
+ if (ret)
+ return ret;
+ }
+
return devm_iio_device_register(dev, indio_dev);
}
EXPORT_SYMBOL_GPL(devm_versal_sysmon_core_probe);
diff --git a/drivers/iio/adc/versal-sysmon.h b/drivers/iio/adc/versal-sysmon.h
index e27a5357575..9fe2793757a 100644
--- a/drivers/iio/adc/versal-sysmon.h
+++ b/drivers/iio/adc/versal-sysmon.h
@@ -11,6 +11,8 @@
#include <linux/bits.h>
#include <linux/mutex.h>
+#include <linux/spinlock_types.h>
+#include <linux/workqueue.h>
struct device;
struct regmap;
@@ -18,12 +20,22 @@ struct regmap;
/* Register offsets (sorted by address) */
#define SYSMON_NPI_LOCK 0x000C
#define SYSMON_ISR 0x0044
+#define SYSMON_IMR 0x0048
+#define SYSMON_IER 0x004C
#define SYSMON_IDR 0x0050
#define SYSMON_TEMP_MAX 0x1030
#define SYSMON_TEMP_MIN 0x1034
#define SYSMON_SUPPLY_BASE 0x1040
+#define SYSMON_ALARM_FLAG 0x1018
+#define SYSMON_ALARM_REG 0x1940
+#define SYSMON_TEMP_TH_LOW 0x1970
+#define SYSMON_TEMP_TH_UP 0x1974
+#define SYSMON_SUPPLY_TH_LOW 0x1980
+#define SYSMON_SUPPLY_TH_UP 0x1C80
+#define SYSMON_TEMP_EV_CFG 0x1F84
#define SYSMON_TEMP_MIN_MIN 0x1F8C
#define SYSMON_TEMP_MAX_MAX 0x1F90
+#define SYSMON_STATUS_RESET 0x1F94
#define SYSMON_TEMP_SAT_BASE 0x1FAC
#define SYSMON_MAX_REG 0x24C0
@@ -35,8 +47,12 @@ struct regmap;
#define SYSMON_SUPPLY_IDX_MAX 159
#define SYSMON_TEMP_SAT_MAX 64
+#define SYSMON_NO_OF_EVENTS 32
#define SYSMON_INTR_ALL_MASK GENMASK(31, 0)
+/* ISR/IMR temperature alarm mask (bit 9) */
+#define SYSMON_TEMP_INTR_MASK BIT(9)
+
/* Supply voltage conversion register fields */
#define SYSMON_MANTISSA_MASK GENMASK(15, 0)
#define SYSMON_FMT_MASK BIT(16)
@@ -46,11 +62,21 @@ struct regmap;
#define SYSMON_FRACTIONAL_SHIFT 7U
#define SYSMON_SUPPLY_MANTISSA_BITS 16
+/* Bits per alarm register */
+#define SYSMON_ALARM_BITS_PER_REG 32
+
+#define SYSMON_UNMASK_WORK_DELAY_MS 500
+
/**
* struct sysmon - Driver data for Versal SysMon
* @regmap: register map for hardware access
* @lock: protects read-modify-write sequences on threshold registers
* and cached state that spans multiple regmap calls
+ * @irq_lock: protects interrupt mask register updates (MMIO path only)
+ * @masked_temp: currently masked temperature alarm bits
+ * @temp_mask: temperature interrupt configuration mask
+ * @temp_hysteresis: cached DEVICE_TEMP hysteresis in millicelsius
+ * @sysmon_unmask_work: re-enables events after alarm condition clears
*/
struct sysmon {
struct regmap *regmap;
@@ -60,6 +86,16 @@ struct sysmon {
* that spans multiple regmap calls.
*/
struct mutex lock;
+ /*
+ * Protects interrupt mask register updates. Only used on the
+ * MMIO path (fast_io regmap); I2C has no IRQ and never reaches
+ * the event code that takes this lock.
+ */
+ spinlock_t irq_lock;
+ unsigned int masked_temp;
+ unsigned int temp_mask;
+ int temp_hysteresis;
+ struct delayed_work sysmon_unmask_work;
};
int devm_versal_sysmon_core_probe(struct device *dev, struct regmap *regmap);
--
2.48.1
^ permalink raw reply related
* [PATCH v8 5/5] iio: adc: versal-sysmon: add oversampling support
From: Salih Erim @ 2026-06-16 13:15 UTC (permalink / raw)
To: jic23, andy
Cc: dlechner, nuno.sa, robh, krzk+dt, conor+dt, conall.ogriofa,
michal.simek, linux, erimsalih, linux-iio, devicetree,
linux-kernel, Salih Erim
In-Reply-To: <20260616131559.3029543-1-salih.erim@amd.com>
Add support for reading and writing the oversampling ratio through
the IIO oversampling_ratio attribute. The hardware supports averaging
2, 4, 8, or 16 samples, plus a ratio of 1 (no averaging).
Temperature and supply channels share oversampling configuration at
the type level (all temperature channels share one ratio, all supply
channels share another), exposed through info_mask_shared_by_type.
The hardware encoding uses sample_count / 2 in a 4-bit field within
the CONFIG register. Per-channel averaging enable registers must also
be updated to activate or deactivate averaging.
Signed-off-by: Salih Erim <salih.erim@amd.com>
---
Changes in v8:
- Use unsigned int for val parameter and hw_val in both
osr_write helpers (Andy)
- Use ~0 instead of ~0U for avg enable bitmask (Andy)
Changes in v7:
- Split sysmon_osr_write into sysmon_osr_write_temp and
sysmon_osr_write_supply; caller dispatches with if/else
on chan->type (Jonathan)
- Restore HW encoding comment in both helpers; fix
cross-reference in sysmon_osr_write_supply
Changes in v6:
- Fix FIELD_PREP indentation in sysmon_osr_write (Andy)
- unsigned int for loop index in sysmon_write_raw (Andy)
Changes in v5:
- Remove unneeded parentheses in i * SYSMON_REG_STRIDE (Andy)
- Use struct regmap *map local variable in
sysmon_set_avg_enable (Andy)
- switch instead of redundant if/if on channel_type (Andy)
- Add CONFIG register readback fence after oversampling update
to prevent NoC bus hang from posted writes (found during
hardware stress testing)
Changes in v4:
- Return directly from sysmon_set_avg_enable calls, remove
else after early returns, drop unreachable return 0 (Jonathan)
- Rename mask defines to SYSMON_CONFIG_SUPPLY_OSR and
SYSMON_CONFIG_TEMP_SAT_OSR (Jonathan)
- Drop "bits X:Y" from GENMASK comments (Jonathan)
- Blank lines after if (ret) return ret blocks (Jonathan)
- Move oversampling read inside guard(mutex) scope
Changes in v3:
- No changes
Changes in v2:
- EN_AVG per-channel bitmask registers written with all-ones
instead of boolean 1 when oversampling is enabled
- EN_AVG write errors propagated to userspace
- Oversampling limited to satellite temp and supply channels;
static temp channels do not participate
- Oversampling exposes actual sample counts (1,2,4,8,16) to
userspace with internal HW register translation
- write_raw_get_fmt returns IIO_VAL_INT for oversampling ratio
- HW encoding documented (sample_count/2, not log2)
- oversampling_avail is const int[] (type match fix)
drivers/iio/adc/versal-sysmon-core.c | 153 ++++++++++++++++++++++++++-
drivers/iio/adc/versal-sysmon.h | 17 +++
2 files changed, 169 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/adc/versal-sysmon-core.c b/drivers/iio/adc/versal-sysmon-core.c
index 8d567f9fa05..309c39b515f 100644
--- a/drivers/iio/adc/versal-sysmon-core.c
+++ b/drivers/iio/adc/versal-sysmon-core.c
@@ -26,6 +26,12 @@
#include "versal-sysmon.h"
+/*
+ * Oversampling ratio values exposed to userspace via IIO.
+ * Actual number of samples averaged: 1=none, 2=2x, 4=4x, 8=8x, 16=16x.
+ */
+static const int sysmon_oversampling_avail[] = { 1, 2, 4, 8, 16 };
+
/* TEMP hysteresis mode bit in SYSMON_TEMP_EV_CFG */
#define SYSMON_TEMP_HYST_MASK BIT(1)
@@ -165,6 +171,12 @@ static int sysmon_read_raw(struct iio_dev *indio_dev,
guard(mutex)(&sysmon->lock);
+ if (mask == IIO_CHAN_INFO_OVERSAMPLING_RATIO) {
+ *val = (chan->type == IIO_TEMP) ? sysmon->temp_oversampling :
+ sysmon->supply_oversampling;
+ return IIO_VAL_INT;
+ }
+
switch (chan->type) {
case IIO_TEMP:
if (mask == IIO_CHAN_INFO_SCALE) {
@@ -454,6 +466,132 @@ static int sysmon_write_event_value(struct iio_dev *indio_dev,
}
}
+static int sysmon_set_avg_enable(struct sysmon *sysmon,
+ u32 base, u32 count, u32 val)
+{
+ struct regmap *map = sysmon->regmap;
+ int ret;
+
+ for (unsigned int i = 0; i < count; i++) {
+ ret = regmap_write(map, base + i * SYSMON_REG_STRIDE, val);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int sysmon_osr_write_temp(struct sysmon *sysmon, unsigned int val)
+{
+ /*
+ * HW register encoding is sample_count / 2:
+ * 0=none, 1=2x, 2=4x, 4=8x, 8=16x (not log2-based).
+ */
+ unsigned int hw_val = val >> 1;
+ unsigned int readback;
+ int ret;
+
+ ret = regmap_update_bits(sysmon->regmap, SYSMON_CONFIG,
+ SYSMON_CONFIG_TEMP_SAT_OSR,
+ FIELD_PREP(SYSMON_CONFIG_TEMP_SAT_OSR, hw_val));
+ if (ret)
+ return ret;
+
+ /*
+ * Readback fence: the SysMon CONFIG register resides in the
+ * PMC domain behind the NoC. A posted write may not reach the
+ * hardware before the next MMIO access. Reading the register
+ * back forces the interconnect to complete the write, preventing
+ * a bus hang on the subsequent access.
+ */
+ regmap_read(sysmon->regmap, SYSMON_CONFIG, &readback);
+
+ return sysmon_set_avg_enable(sysmon, SYSMON_TEMP_EN_AVG_BASE,
+ SYSMON_TEMP_EN_AVG_COUNT,
+ hw_val ? ~0 : 0);
+}
+
+static int sysmon_osr_write_supply(struct sysmon *sysmon, unsigned int val)
+{
+ /* HW encoding: sample_count / 2 (see sysmon_osr_write_temp) */
+ unsigned int hw_val = val >> 1;
+ unsigned int readback;
+ int ret;
+
+ ret = regmap_update_bits(sysmon->regmap, SYSMON_CONFIG,
+ SYSMON_CONFIG_SUPPLY_OSR,
+ FIELD_PREP(SYSMON_CONFIG_SUPPLY_OSR, hw_val));
+ if (ret)
+ return ret;
+
+ /* Readback fence -- see sysmon_osr_write_temp for details */
+ regmap_read(sysmon->regmap, SYSMON_CONFIG, &readback);
+
+ return sysmon_set_avg_enable(sysmon, SYSMON_SUPPLY_EN_AVG_BASE,
+ SYSMON_SUPPLY_EN_AVG_COUNT,
+ hw_val ? ~0 : 0);
+}
+
+static int sysmon_write_raw(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
+{
+ struct sysmon *sysmon = iio_priv(indio_dev);
+ unsigned int i;
+ int ret;
+
+ if (mask != IIO_CHAN_INFO_OVERSAMPLING_RATIO)
+ return -EINVAL;
+
+ for (i = 0; i < ARRAY_SIZE(sysmon_oversampling_avail); i++) {
+ if (val == sysmon_oversampling_avail[i])
+ break;
+ }
+ if (i == ARRAY_SIZE(sysmon_oversampling_avail))
+ return -EINVAL;
+
+ guard(mutex)(&sysmon->lock);
+
+ if (chan->type == IIO_TEMP) {
+ ret = sysmon_osr_write_temp(sysmon, val);
+ if (ret)
+ return ret;
+ sysmon->temp_oversampling = val;
+ } else {
+ ret = sysmon_osr_write_supply(sysmon, val);
+ if (ret)
+ return ret;
+ sysmon->supply_oversampling = val;
+ }
+
+ return 0;
+}
+
+static int sysmon_write_raw_get_fmt(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ long mask)
+{
+ if (mask == IIO_CHAN_INFO_OVERSAMPLING_RATIO)
+ return IIO_VAL_INT;
+
+ return -EINVAL;
+}
+
+static int sysmon_read_avail(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ const int **vals, int *type,
+ int *length, long mask)
+{
+ if (mask != IIO_CHAN_INFO_OVERSAMPLING_RATIO)
+ return -EINVAL;
+
+ *vals = sysmon_oversampling_avail;
+ *type = IIO_VAL_INT;
+ *length = ARRAY_SIZE(sysmon_oversampling_avail);
+
+ return IIO_AVAIL_LIST;
+}
+
static int sysmon_read_label(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
char *label)
@@ -466,6 +604,9 @@ static int sysmon_read_label(struct iio_dev *indio_dev,
static const struct iio_info sysmon_iio_info = {
.read_raw = sysmon_read_raw,
+ .write_raw = sysmon_write_raw,
+ .write_raw_get_fmt = sysmon_write_raw_get_fmt,
+ .read_avail = sysmon_read_avail,
.read_label = sysmon_read_label,
.read_event_config = sysmon_read_event_config,
.write_event_config = sysmon_write_event_config,
@@ -756,6 +897,10 @@ static int sysmon_parse_fw(struct iio_dev *indio_dev, struct device *dev, int ir
.indexed = 1,
.address = reg,
.info_mask_separate = BIT(IIO_CHAN_INFO_PROCESSED),
+ .info_mask_shared_by_type =
+ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+ .info_mask_shared_by_type_available =
+ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
.event_spec = irq > 0 ?
sysmon_supply_events : NULL,
.num_event_specs = irq > 0 ?
@@ -787,7 +932,11 @@ static int sysmon_parse_fw(struct iio_dev *indio_dev, struct device *dev, int ir
.address = SYSMON_TEMP_SAT_BASE +
(reg - 1) * SYSMON_REG_STRIDE,
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW),
- .info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),
+ .info_mask_shared_by_type =
+ BIT(IIO_CHAN_INFO_SCALE) |
+ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
+ .info_mask_shared_by_type_available =
+ BIT(IIO_CHAN_INFO_OVERSAMPLING_RATIO),
.datasheet_name = label,
};
}
@@ -834,6 +983,8 @@ int devm_versal_sysmon_core_probe(struct device *dev, struct regmap *regmap)
sysmon = iio_priv(indio_dev);
sysmon->regmap = regmap;
+ sysmon->temp_oversampling = 1;
+ sysmon->supply_oversampling = 1;
ret = devm_mutex_init(dev, &sysmon->lock);
if (ret)
diff --git a/drivers/iio/adc/versal-sysmon.h b/drivers/iio/adc/versal-sysmon.h
index 9fe2793757a..bb9a75bf71c 100644
--- a/drivers/iio/adc/versal-sysmon.h
+++ b/drivers/iio/adc/versal-sysmon.h
@@ -23,11 +23,13 @@ struct regmap;
#define SYSMON_IMR 0x0048
#define SYSMON_IER 0x004C
#define SYSMON_IDR 0x0050
+#define SYSMON_CONFIG 0x0100
#define SYSMON_TEMP_MAX 0x1030
#define SYSMON_TEMP_MIN 0x1034
#define SYSMON_SUPPLY_BASE 0x1040
#define SYSMON_ALARM_FLAG 0x1018
#define SYSMON_ALARM_REG 0x1940
+#define SYSMON_SUPPLY_EN_AVG_BASE 0x1958
#define SYSMON_TEMP_TH_LOW 0x1970
#define SYSMON_TEMP_TH_UP 0x1974
#define SYSMON_SUPPLY_TH_LOW 0x1980
@@ -37,6 +39,7 @@ struct regmap;
#define SYSMON_TEMP_MAX_MAX 0x1F90
#define SYSMON_STATUS_RESET 0x1F94
#define SYSMON_TEMP_SAT_BASE 0x1FAC
+#define SYSMON_TEMP_EN_AVG_BASE 0x24B4
#define SYSMON_MAX_REG 0x24C0
/* NPI unlock value written to SYSMON_NPI_LOCK */
@@ -53,6 +56,16 @@ struct regmap;
/* ISR/IMR temperature alarm mask (bit 9) */
#define SYSMON_TEMP_INTR_MASK BIT(9)
+/* SYSMON_CONFIG: supply oversampling ratio */
+#define SYSMON_CONFIG_SUPPLY_OSR GENMASK(17, 14)
+
+/* SYSMON_CONFIG: temperature satellite oversampling ratio */
+#define SYSMON_CONFIG_TEMP_SAT_OSR GENMASK(27, 24)
+
+/* Per-channel averaging enable register counts */
+#define SYSMON_SUPPLY_EN_AVG_COUNT 5
+#define SYSMON_TEMP_EN_AVG_COUNT 2
+
/* Supply voltage conversion register fields */
#define SYSMON_MANTISSA_MASK GENMASK(15, 0)
#define SYSMON_FMT_MASK BIT(16)
@@ -77,6 +90,8 @@ struct regmap;
* @temp_mask: temperature interrupt configuration mask
* @temp_hysteresis: cached DEVICE_TEMP hysteresis in millicelsius
* @sysmon_unmask_work: re-enables events after alarm condition clears
+ * @temp_oversampling: current temp oversampling ratio
+ * @supply_oversampling: current supply oversampling ratio
*/
struct sysmon {
struct regmap *regmap;
@@ -96,6 +111,8 @@ struct sysmon {
unsigned int temp_mask;
int temp_hysteresis;
struct delayed_work sysmon_unmask_work;
+ unsigned int temp_oversampling;
+ unsigned int supply_oversampling;
};
int devm_versal_sysmon_core_probe(struct device *dev, struct regmap *regmap);
--
2.48.1
^ permalink raw reply related
* Re: [PATCH 2/2] arm64: dts: qcom: ipq5210: Add QPIC SPI NAND controller support
From: Konrad Dybcio @ 2026-06-16 13:18 UTC (permalink / raw)
To: Varadarajan Narayanan, Mark Brown, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, sadre Alam, Bjorn Andersson,
Konrad Dybcio
Cc: linux-arm-msm, linux-spi, devicetree, linux-kernel
In-Reply-To: <20260514-ipq5210-nand-v1-2-cbdd7492e826@oss.qualcomm.com>
On 5/14/26 8:45 AM, Varadarajan Narayanan wrote:
> Add device tree nodes for QPIC SPI NAND flash controller support on
> ipq5210 SoC.
>
> The ipq5210 SoC includes a QPIC controller that supports SPI NAND flash
> devices with hardware ECC capabilities and DMA support through BAM (Bus
> Access Manager).
>
> Signed-off-by: Varadarajan Narayanan <varadarajan.narayanan@oss.qualcomm.com>
> ---
> arch/arm64/boot/dts/qcom/ipq5210-rdp504.dts | 42 +++++++++++++++++++++++++++++
> arch/arm64/boot/dts/qcom/ipq5210.dtsi | 29 ++++++++++++++++++++
Please align with your colleague's submission for similar platforms:
https://lore.kernel.org/linux-arm-msm/20260306113940.1654304-3-quic_mdalam@quicinc.com/
->
- separate commit for SoC and board
- is the BAM host v1.7.0 or v1.7.4?
- style: clock/dma-names, 1 entry per line, please
Konrad
^ permalink raw reply
* Re: [PATCH 1/3] of/fdt: fix misleading elfcorehdr reservation success message
From: Rob Herring @ 2026-06-16 13:19 UTC (permalink / raw)
To: Sang-Heon Jeon
Cc: Saravana Kannan, Huacai Chen, WANG Xuerui, Thomas Bogendoerfer,
devicetree, loongarch, linux-mips
In-Reply-To: <20260614161503.2219681-2-ekffu200098@gmail.com>
On Mon, Jun 15, 2026 at 01:15:01AM +0900, Sang-Heon Jeon wrote:
> fdt_reserve_elfcorehdr() does not check the return value of
> memblock_reserve(), so a success message is falsely printed when the
> reservation fails.
>
> Check the return value and warn on failure instead.
>
> Signed-off-by: Sang-Heon Jeon <ekffu200098@gmail.com>
> ---
> drivers/of/fdt.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index 26f66046cc32..d985c07d7c5c 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -479,7 +479,10 @@ static void __init fdt_reserve_elfcorehdr(void)
> return;
> }
>
> - memblock_reserve(elfcorehdr_addr, elfcorehdr_size);
> + if (memblock_reserve(elfcorehdr_addr, elfcorehdr_size)) {
> + pr_warn("Failed to reserve memory for elfcorehdr\n");
I would think memblock_reserve() should always succeed and if not it
should print a message rather than having every caller print a message.
Rob
^ permalink raw reply
* Re: [PATCH v4 1/3] arm64: dts: qcom: sdm670: add default uart pinctrl nodes
From: Konrad Dybcio @ 2026-06-16 13:25 UTC (permalink / raw)
To: pabloyoyoista, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel, Richard Acayan,
~postmarketos/upstreaming
In-Reply-To: <20260601-pabloyoyoista-debug-uart-on-rdacayan-next-v4-1-c5d51dd96ca0@postmarketos.org>
On 6/1/26 6:55 PM, Pablo Correa Gómez via B4 Relay wrote:
> From: Pablo Correa Gómez <pabloyoyoista@postmarketos.org>
>
> This is a pre-requisite to enable UART in sargo and bonito. Values for
> the pins have been taken from sdm845, and cross-checking dowstream,
> where available.
>
> Signed-off-by: Pablo Correa Gómez <pabloyoyoista@postmarketos.org>
> ---
[...]
> + qup_uart0_default: qup-uart0-default-state {
> + qup_uart0_tx: tx-pins {
> + pins = "gpio2";
> + function = "qup0";
> + };
> +
> + qup_uart0_rx: rx-pins {
> + pins = "gpio3";
> + function = "qup0";
> + };
Let's drop the unused labels and let's bring the default config
(bias-disable + drive-strength = <2>) to the SoC DT, like we have
in glymur.dtsi
Please also cross-check whether the pull-up you enable in your
phone's DT is actually necessary (and whether it's defined as such
in the vendor DT - FWIW it's not in the sdm670-pinctrl.dtsi common
one)
Konrad
^ permalink raw reply
* [PATCH v3 1/2] dt-bindings: phy: qcom,usb-hs-phy: add qcom,hs-drv-slope
From: Herman van Hazendonk @ 2026-06-16 13:26 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Philipp Zabel, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel, llvm,
Herman van Hazendonk, konrad.dybcio, dmitry.baryshkov
In-Reply-To: <20260616-submit-phy-usb-hs-vendor-init-seq-v3-0-7d21fb1d1484@herrie.org>
The MSM8x60 / APQ8060 PHY needs three vendor ULPI register tweaks for
stable USB operation: pre-emphasis level, CDR auto-reset and SE1
gating in registers 0x32 and 0x36. A survey of MSM8x60-class
downstream board files (Qualcomm SURF/FFA/Fluid/Dragon, Samsung
Galaxy S2 family, Sony Xperia, HTC and HP TouchPad) shows that those
three values are identical across every reference board and can be
hardcoded in the driver behind the existing
qcom,usb-hs-phy-msm8660 compatible.
The only board-specific value is the 4-bit HS driver slope in bits
[3:0] of register 0x32:
HP TouchPad 5
HTC MSM8660 ports 1
Qualcomm / Samsung / Sony reference boards 0 (silicon default)
Add a qcom,hs-drv-slope property carrying that 4-bit value, valid
only on the qcom,usb-hs-phy-msm8660 variant. When the property is
absent the driver leaves the silicon default in place, matching the
behaviour of the Qualcomm reference platform.
No public Qualcomm documentation describes how the 4-bit value maps
to an actual slew rate, V/ns or %; the bits are an opaque hardware
control whose meaning only Qualcomm knows. The legal range (0..15)
comes from the field width in the downstream
arch/arm/mach-msm/include/mach/msm_hsusb_hw.h
(ULPI_HSDRVSLOPE_MASK == 0x0F). Boards must therefore copy the
value from their downstream/vendor kernel; this is a measured /
tuned-per-layout knob, not a derived one.
Assisted-by: Claude:claude-opus-4-7 dt_binding_check checkpatch
Assisted-by: Sashiko:claude-opus-4-7
Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
---
.../devicetree/bindings/phy/qcom,usb-hs-phy.yaml | 89 +++++++++++++++-------
1 file changed, 63 insertions(+), 26 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.yaml
index e03b516c698c..e605f5683f7d 100644
--- a/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.yaml
@@ -9,32 +9,43 @@ title: Qualcomm's USB HS PHY
maintainers:
- Bjorn Andersson <bjorn.andersson@linaro.org>
-if:
- properties:
- compatible:
- contains:
- enum:
- - qcom,usb-hs-phy-apq8064
- - qcom,usb-hs-phy-msm8660
- - qcom,usb-hs-phy-msm8960
-then:
- properties:
- resets:
- maxItems: 1
-
- reset-names:
- const: por
-
-else:
- properties:
- resets:
- minItems: 2
- maxItems: 2
-
- reset-names:
- items:
- - const: phy
- - const: por
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,usb-hs-phy-apq8064
+ - qcom,usb-hs-phy-msm8660
+ - qcom,usb-hs-phy-msm8960
+ then:
+ properties:
+ resets:
+ maxItems: 1
+
+ reset-names:
+ const: por
+
+ else:
+ properties:
+ resets:
+ minItems: 2
+ maxItems: 2
+
+ reset-names:
+ items:
+ - const: phy
+ - const: por
+
+ - if:
+ not:
+ properties:
+ compatible:
+ contains:
+ const: qcom,usb-hs-phy-msm8660
+ then:
+ properties:
+ qcom,hs-drv-slope: false
properties:
compatible:
@@ -85,6 +96,15 @@ properties:
the address is offset from the ULPI_EXT_VENDOR_SPECIFIC address
- description: value
+ qcom,hs-drv-slope:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: >
+ 4-bit HS driver slope written to bits [3:0] of ULPI vendor
+ register 0x32. Board-specific tuning value; absent means
+ leave silicon default. Only valid on qcom,usb-hs-phy-msm8660.
+ minimum: 0
+ maximum: 15
+
required:
- clocks
- clock-names
@@ -114,3 +134,20 @@ examples:
};
};
};
+
+ - |
+ usb-controller {
+ #reset-cells = <1>;
+
+ ulpi {
+ phy {
+ compatible = "qcom,usb-hs-phy-msm8660", "qcom,usb-hs-phy";
+ #phy-cells = <0>;
+ clocks = <&clk 0>, <&clk 1>;
+ clock-names = "ref", "sleep";
+ resets = <&otg 0>;
+ reset-names = "por";
+ qcom,hs-drv-slope = <5>;
+ };
+ };
+ };
--
2.43.0
^ permalink raw reply related
* [PATCH v3 0/2] phy: qcom: usb-hs: MSM8x60 vendor ULPI init
From: Herman van Hazendonk @ 2026-06-16 13:26 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Philipp Zabel, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel, llvm,
Herman van Hazendonk, konrad.dybcio, dmitry.baryshkov
v3 (this round):
- Re-introduce a much smaller DT binding patch following Konrad's
"do we have values for MTP/QRD" question and Dmitry's
"qcom,hsdrvslope (or similarly named) property in DT" suggestion.
- Survey of every MSM8x60-class downstream tree I could reach --
Qualcomm reference (SURF/FFA/Fluid/Dragon/Fusion via
board-msm8x60.c on android.googlesource.com), Samsung Galaxy S2
family (Q1 / Celox / Dali / generic 8x60 MTP), Sony MSM8660
(sony-kernel-msm8660), HTC MSM8660 ports
(shooter / holiday / pyramid / doubleshot / shooter_u / ruby) and
HP TouchPad -- shows that pre-emphasis, CDR auto-reset and SE1
gating values are *identical* across every reference board.
Only the 4-bit HS driver slope in reg 0x32 [3:0] varies.
- Patch 1/2 adds a single qcom,hs-drv-slope DT property (u32,
range 0..15) gated to the qcom,usb-hs-phy-msm8660 compatible.
- Patch 2/2 hardcodes the three platform-wide writes in the driver
behind the same compatible match, consumes qcom,hs-drv-slope for
the board-specific bits, and leaves the silicon default in place
when the property is absent -- which matches Qualcomm's own MTP,
Samsung and Sony reference behaviour.
- The bit-level meaning we *do* have comes from Code Aurora's
downstream arch/arm/mach-msm/include/mach/msm_hsusb_hw.h, which
Samsung and HP both shipped byte-for-byte identical.
- Per Dmitry's request, both commit messages call out explicitly
that there is no public Qualcomm documentation describing how the
4-bit slope value maps to an actual slew rate / V/ns / %. The
field is an opaque hardware control; boards must copy the value
from their vendor / downstream kernel as a measured-per-layout
knob, not a derived one.
v2:
- Dropped the original qcom,vendor-init-seq DT property entirely
and folded all the vendor-register programming into the driver
behind the qcom,usb-hs-phy-msm8660 compatible.
- HS driver slope was hardcoded in v2. v3 promotes that one
varying value to a DT property as Dmitry requested.
Companion TouchPad DTS work (flipping the PHY compatible from
"qcom,usb-hs-phy-apq8064" to "qcom,usb-hs-phy-msm8660" and adding
qcom,hs-drv-slope = <5>) will be sent separately with the rest of
the apq8060-tenderloin DT series.
On-device validation (HP TouchPad / APQ8060):
- Booted with v3 + the upcoming DTS hookup. PHY driver bound,
msm_hsusb HS link came up at high-speed. No regression vs the v2
hardcoded build.
Build / schema verification:
- dt_binding_check DT_SCHEMA_FILES=.../qcom,usb-hs-phy.yaml: clean.
- dtbs_check on qcom-apq8060-dragonboard.dtb and
qcom-msm8960-cdp.dtb (the two existing in-tree usb-hs-phy
consumers): clean.
- drivers/phy/qualcomm/phy-qcom-usb-hs.o builds clean.
- checkpatch.pl --strict: no warnings on either patch.
Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
---
Herman van Hazendonk (2):
dt-bindings: phy: qcom,usb-hs-phy: add qcom,hs-drv-slope
phy: qcom: usb-hs: program MSM8x60 vendor ULPI registers on power-on
.../devicetree/bindings/phy/qcom,usb-hs-phy.yaml | 89 +++++++++++++++-------
drivers/phy/qualcomm/phy-qcom-usb-hs.c | 68 +++++++++++++++++
2 files changed, 131 insertions(+), 26 deletions(-)
---
base-commit: 944125b4c454b58d2fe6e35f1087a932b2050dff
change-id: 20260616-submit-phy-usb-hs-vendor-init-seq-ad39d29ccaf5
Best regards,
--
Herman van Hazendonk <github.com@herrie.org>
^ permalink raw reply
* Re: [PATCH v3 03/10] dt-bindings: input: microchip,cap11xx: Update datasheet URL and LED reg range
From: Jun Yan @ 2026-06-16 13:34 UTC (permalink / raw)
To: conor
Cc: conor+dt, devicetree, dmitry.torokhov, jerrysteve1101, krzk+dt,
linux-input, linux-kernel, robh
In-Reply-To: <20260615-splinter-subtitle-a88cf08320e8@spud>
> On Mon, Jun 15, 2026 at 10:20:29PM +0800, Jun Yan wrote:
> > - Add datasheet links for all supported CAP11xx variants.
> > - Update LED node regex and replace enum constraints with minimum/maximum
> > for LED reg ranges in preparation for CAP1114 support.
> >=20
> > CAP1114 has 11 LED channels. minimum/maximum constraints are easier to
> > maintain than long enum lists when expanding channel count later.
> >=20
> > Signed-off-by: Jun Yan <jerrysteve1101@gmail.com>
> > ---
> > .../bindings/input/microchip,cap11xx.yaml | 17 ++++++++++++++---
> > 1 file changed, 14 insertions(+), 3 deletions(-)
> >=20
> > diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.ya=
> ml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> > index 7ade03f1b32b..9578c7c206a2 100644
> > --- a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> > +++ b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> > @@ -10,6 +10,15 @@ description: |
> > The Microchip CAP1xxx Family of RightTouchTM multiple-channel capaciti=
> ve
> > touch controllers and LED drivers. The device communication via I2C on=
> ly.
> > =20
> > + For more product information please see the links below:
> > + CAP1106: https://ww1.microchip.com/downloads/en/DeviceDoc/00001624B.=
> pdf
> > + CAP1126: https://ww1.microchip.com/downloads/en/DeviceDoc/00001623B.=
> pdf
> > + CAP1188: https://ww1.microchip.com/downloads/en/DeviceDoc/00001620C.=
> pdf
> > + CAP1203: https://ww1.microchip.com/downloads/en/DeviceDoc/00001572B.=
> pdf
> > + CAP1206: https://ww1.microchip.com/downloads/en/DeviceDoc/00001567B.=
> pdf
> > + CAP1293: https://ww1.microchip.com/downloads/en/DeviceDoc/00001566B.=
> pdf
> > + CAP1298: https://ww1.microchip.com/downloads/en/DeviceDoc/00001571B.=
> pdf
> > +
> > maintainers:
> > - Rob Herring <robh@kernel.org>
> > =20
> > @@ -124,14 +133,16 @@ properties:
> > The number of entries must correspond to the number of channels.
> > =20
> > patternProperties:
> > - "^led@[0-7]$":
> > + "^led@[0-9a-f]$":
>
> This should be done alongside the cap1114 change, not here I think. The
> constraint relaxation doesn't make sense because the user is arriving in
> a later patch.
> With it moved,
Thanks for your review feedback. I'll move this constraint change into
the cap1114 patch and update it in v4.
> Acked-by: Conor Dooley <conor.dooley@microchip.com>
>
> Although, should it not be led@[0-9a-b] if the max is 11?
>
Agreed. I will fix this in v4.
> pw-bot: changes-requested
>
> Cheers,
> Conor.
>
> > type: object
> > description: CAP11xx LEDs
> > $ref: /schemas/leds/common.yaml#
> > =20
> > properties:
> > reg:
> > - enum: [0, 1, 2, 3, 4, 5, 6, 7]
> > + description: LED channel number
> > + minimum: 0
> > + maximum: 7
> > =20
> > label: true
> > =20
> > @@ -158,7 +169,7 @@ allOf:
> > - microchip,cap1298
> > then:
> > patternProperties:
> > - "^led@[0-7]$": false
> > + "^led@": false
> > =20
> > - if:
> > properties:
> > --=20
> > 2.54.0
> >=20
^ permalink raw reply
* [PATCH v3 2/2] phy: qcom: usb-hs: program MSM8x60 vendor ULPI registers on power-on
From: Herman van Hazendonk @ 2026-06-16 13:26 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Philipp Zabel, Nathan Chancellor,
Nick Desaulniers, Bill Wendling, Justin Stitt
Cc: linux-arm-msm, linux-phy, devicetree, linux-kernel, llvm,
Herman van Hazendonk, konrad.dybcio, dmitry.baryshkov
In-Reply-To: <20260616-submit-phy-usb-hs-vendor-init-seq-v3-0-7d21fb1d1484@herrie.org>
The MSM8x60-class PHY needs three vendor-register tweaks for stable
USB operation, which the legacy msm_otg driver used to drive from
board platform data. A survey of every MSM8x60-class downstream tree
(Qualcomm SURF/FFA/Fluid/Dragon, Samsung Galaxy S2 family, Sony
Xperia, HTC MSM8660 ports and HP TouchPad) shows that two of the
three settings are identical across every board:
- reg 0x32 [5:4] = 11b: pre-emphasis level set to 20%
- reg 0x36 bit 1 = 1, bit 2 = 1: CDR auto-reset and SE1 gating
disabled (the legacy driver inverts these bits, so setting them
disables the function)
Hardcode those two unconditionally behind the existing
qcom,usb-hs-phy-msm8660 compatible. The bit-level documentation
comes from the Code Aurora downstream header
arch/arm/mach-msm/include/mach/msm_hsusb_hw.h, which Samsung and HP
both shipped byte-for-byte identical.
The third setting -- reg 0x32 [3:0] HS driver slope -- is genuinely
board-specific (HP TouchPad uses 5, HTC MSM8660 ports use 1, every
Qualcomm/Samsung/Sony reference board leaves the silicon default of
0) and is consumed from the new qcom,hs-drv-slope DT property. When
the property is absent the silicon default is preserved.
No public Qualcomm documentation describes how the 4-bit slope value
maps to an actual slew rate, V/ns or %; the field is an opaque
hardware control whose semantics only Qualcomm knows. Boards must
copy the value from their vendor / downstream kernel -- this is a
measured / tuned-per-layout knob, not a derived one. We program the
4 bits verbatim and trust the silicon to do the right thing.
The writes live behind a runtime flag that only matches
"qcom,usb-hs-phy-msm8660" so the existing MSM8226/8916/8960/8974
consumers are untouched. They are issued *after*
reset_control_reset() so the values survive the register restore the
reset performs.
Note: HTC MSM8660 vendor kernels additionally write 0x0C to reg 0x31.
The HP TouchPad webOS kernel does not touch that register and USB is
stable without it, so those bits are omitted here until documentation
is available to explain what they control.
Assisted-by: Claude:claude-opus-4-7 sparse smatch clang-analyzer coccinelle checkpatch
Assisted-by: Sashiko:claude-opus-4-7
Signed-off-by: Herman van Hazendonk <github.com@herrie.org>
---
drivers/phy/qualcomm/phy-qcom-usb-hs.c | 68 ++++++++++++++++++++++++++++++++++
1 file changed, 68 insertions(+)
diff --git a/drivers/phy/qualcomm/phy-qcom-usb-hs.c b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
index 98a18987f1be..a7649a09e82c 100644
--- a/drivers/phy/qualcomm/phy-qcom-usb-hs.c
+++ b/drivers/phy/qualcomm/phy-qcom-usb-hs.c
@@ -20,6 +20,14 @@
# define ULPI_MISC_A_VBUSVLDEXTSEL BIT(1)
# define ULPI_MISC_A_VBUSVLDEXT BIT(0)
+/* MSM8x60 vendor ULPI registers (raw addresses, not ULPI_EXT_VENDOR_SPECIFIC). */
+#define ULPI_MSM_CONFIG_REG3 0x32
+# define ULPI_MSM_HSDRVSLOPE_MASK GENMASK(3, 0)
+# define ULPI_MSM_PRE_EMPHASIS_MASK GENMASK(5, 4)
+# define ULPI_MSM_PRE_EMPHASIS_20PCT (3 << 4)
+#define ULPI_MSM_DIGOUT_CTRL 0x36
+# define ULPI_MSM_CDR_AUTORESET BIT(1)
+# define ULPI_MSM_SE1_GATE BIT(2)
struct ulpi_seq {
u8 addr;
@@ -37,6 +45,9 @@ struct qcom_usb_hs_phy {
struct ulpi_seq *init_seq;
struct extcon_dev *vbus_edev;
struct notifier_block vbus_notify;
+ bool msm8x60_init;
+ bool hs_drv_slope_present;
+ u8 hs_drv_slope;
};
static int qcom_usb_hs_phy_set_mode(struct phy *phy,
@@ -105,6 +116,41 @@ qcom_usb_hs_phy_vbus_notifier(struct notifier_block *nb, unsigned long event,
return ulpi_write(uphy->ulpi, addr, ULPI_MISC_A_VBUSVLDEXT);
}
+/*
+ * RMW the vendor registers to preserve silicon reserved bits.
+ * In reg 0x36 the legacy semantics are inverted: setting
+ * CDR_AUTORESET / SE1_GATE *disables* those functions.
+ */
+static int qcom_usb_hs_phy_msm8x60_init(struct qcom_usb_hs_phy *uphy)
+{
+ struct ulpi *ulpi = uphy->ulpi;
+ int reg32, reg36, ret;
+
+ reg32 = ulpi_read(ulpi, ULPI_MSM_CONFIG_REG3);
+ if (reg32 < 0)
+ return reg32;
+
+ reg32 &= ~ULPI_MSM_PRE_EMPHASIS_MASK;
+ reg32 |= ULPI_MSM_PRE_EMPHASIS_20PCT;
+
+ if (uphy->hs_drv_slope_present) {
+ reg32 &= ~ULPI_MSM_HSDRVSLOPE_MASK;
+ reg32 |= uphy->hs_drv_slope & ULPI_MSM_HSDRVSLOPE_MASK;
+ }
+
+ ret = ulpi_write(ulpi, ULPI_MSM_CONFIG_REG3, reg32);
+ if (ret)
+ return ret;
+
+ reg36 = ulpi_read(ulpi, ULPI_MSM_DIGOUT_CTRL);
+ if (reg36 < 0)
+ return reg36;
+
+ reg36 |= ULPI_MSM_CDR_AUTORESET | ULPI_MSM_SE1_GATE;
+
+ return ulpi_write(ulpi, ULPI_MSM_DIGOUT_CTRL, reg36);
+}
+
static int qcom_usb_hs_phy_power_on(struct phy *phy)
{
struct qcom_usb_hs_phy *uphy = phy_get_drvdata(phy);
@@ -154,6 +200,12 @@ static int qcom_usb_hs_phy_power_on(struct phy *phy)
goto err_ulpi;
}
+ if (uphy->msm8x60_init) {
+ ret = qcom_usb_hs_phy_msm8x60_init(uphy);
+ if (ret)
+ goto err_ulpi;
+ }
+
if (uphy->vbus_edev) {
state = extcon_get_state(uphy->vbus_edev, EXTCON_USB);
/* setup initial state */
@@ -214,6 +266,22 @@ static int qcom_usb_hs_phy_probe(struct ulpi *ulpi)
return -ENOMEM;
ulpi_set_drvdata(ulpi, uphy);
uphy->ulpi = ulpi;
+ uphy->msm8x60_init = of_device_is_compatible(ulpi->dev.of_node,
+ "qcom,usb-hs-phy-msm8660");
+
+ if (uphy->msm8x60_init) {
+ u32 slope;
+
+ if (!of_property_read_u32(ulpi->dev.of_node,
+ "qcom,hs-drv-slope", &slope)) {
+ if (slope > ULPI_MSM_HSDRVSLOPE_MASK)
+ return dev_err_probe(&ulpi->dev, -EINVAL,
+ "qcom,hs-drv-slope out of range (max %lu)\n",
+ ULPI_MSM_HSDRVSLOPE_MASK);
+ uphy->hs_drv_slope = slope;
+ uphy->hs_drv_slope_present = true;
+ }
+ }
size = of_property_count_u8_elems(ulpi->dev.of_node, "qcom,init-seq");
if (size < 0)
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2 7/8] dt-bindings: display: allwinner: Split H616 DE33 layer reg space
From: Andre Przywara @ 2026-06-16 13:38 UTC (permalink / raw)
To: Krzysztof Kozlowski, Jernej Škrabec, wens
Cc: samuel, mripard, maarten.lankhorst, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, mturquette, sboyd, dri-devel, devicetree,
linux-arm-kernel, linux-sunxi, linux-kernel, linux-clk
In-Reply-To: <b1f03d81-85db-4303-89ff-64440b6e1890@kernel.org>
Hi,
On 6/16/26 05:51, Krzysztof Kozlowski wrote:
> On 15/06/2026 17:47, Jernej Škrabec wrote:
>> Dne ponedeljek, 15. junij 2026 ob 06:28:54 Srednjeevropski poletni čas je Krzysztof Kozlowski napisal(a):
>>> On 14/06/2026 16:08, Jernej Škrabec wrote:
>>>> Dne ponedeljek, 25. maj 2026 ob 14:10:38 Srednjeevropski poletni čas je Krzysztof Kozlowski napisal(a):
>>>>> On 24/05/2026 23:33, Chen-Yu Tsai wrote:
>>>>>> Hi,
>>>>>>
>>>>>> (resent from new email)
>>>>>>
>>>>>> On Thu, May 14, 2026 at 2:04 PM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>>>>>
>>>>>>> On Sat, May 09, 2026 at 09:00:14PM +0200, Jernej Skrabec wrote:
>>>>>>>> From: Jernej Skrabec <jernej.skrabec@gmail.com>
>>>>>>>>
>>>>>>>> As it turns out, current H616 DE33 binding was written based on
>>>>>>>> incomplete understanding of DE33 design. Namely, planes are shared
>>>>>>>> resource and not tied to specific mixer, which was the case for previous
>>>>>>>> generations of Display Engine (DE3 and earlier).
>>>>>>>>
>>>>>>>> This means that current DE33 binding doesn't properly reflect HW and
>>>>>>>> using it would mean that second mixer (used for second display output)
>>>>>>>> can't be supported.
>>>>>>>>
>>>>>>>> Remove layer register space, which will be represented with additional
>>>>>>>> node, and replace it with phandle, which will point to that new, shared
>>>>>>>> node. That way, all mixers can share same layers.
>>>>>>>>
>>>>>>>> There is no user of this binding yet, so changes can be made safely,
>>>>>>>> without breaking any backward compatibility.
>>>>>>>
>>>>>>> There is user. git grep gives me:
>>>>>>> drivers/gpu/drm/sun4i/sun8i_mixer.c
>>>>>>>
>>>>>>> which means this is a released ABI. As I understood, the old code was
>>>>>>
>>>>>> We held off on merging the DT changes so that we could rework this.
>>>>>> I can't find the actual request though. It was probably over IRC.
>>>>>>
>>>>>>> working fine but just did not support all use cases. Why this cannot be
>>>>>>> kept backwards compatible?
>>>>>>
>>>>>> AFAIK the "planes" block is shared between two display mixers. As the
>>>>>> commit message explains, this prevents using the second mixer, since
>>>>>> only one of them can claim and map the register space. And on the H700
>>>>>> (which is the same die as the H616 discussed here but with more exposed
>>>>>> interfaces), there could actually be a use case for the second mixer.
>>>>>
>>>>> It explains why you want to make the changes but not why you cannot keep
>>>>> it backwards compatible.
>>>>
>>>> I guess it can be backward compatible, but I don't think it makes sense.
>>>> Yes, original driver implemented original DT bindings, but there is no node
>>>> which uses that binding. If there is no user of that, why would driver
>>>
>>> Did you check all out of tree users of the ABI? All vendor kernels,
>>> forks and all of them for which the ABI was made for?
>>
>> Since when do we care about out of tree users? I understand that drivers
>
> Since always? That is the meaning of ABI. Otherwise there is no point to
> discuss ABI at all. Why would it exist if you had all DTS inside kernel
> always matching the code?
In general I would agree with Krzysztof, merged binding means we need to
stick to it, but in this case I think that would be over the top. As
Jernej said, there are no users, since we didn't commit on the DTs, and
the DE33 graphics support in the kernel (bindings and code) is
incomplete: it's really just the mixer, but no other components (TCON or
output PHYs) required. So it never worked as such.
In hindsight we could say that we should have never merged the bindings
without having fully working, reviewed and accepted driver code, for the
whole display chain. Which we need to do because we create those
bindings based on reverse engineering efforts, not by looking at
documentation or design documents (which we don't have).
>> must support old device tree files. Once they work, compatibility must
>> be carried forward. But that's not the case here.
>>
>> In any case, vendor kernels have completely different DT structure. This
>> was developed independently from them. Take a look at [1] how BSP DT looks
>> like, specifically Display Engine node.
>>
>> Of course there are some distros which grab WIP patches from mailing lists
>> soon after they are available. For example, I know that Armbian carried old
>> WIP patches which used old ABI. However, such distros generally don't care
>> about exact solution and ditch patches as soon as proper solution is merged
>> upstream or even when better WIP patches come around. DT files in such
>> distros get updated alongside kernel, they are not hidden in firmware.
>>
>
> I am not talking about BSP. I am talking about out of tree users for
> which we defined the ABI and called it that way.
If you are looking for DT users outside of the Linux kernel, this is the
result of a quick check (list of users from ChatGPT, checks by myself
with git clone/git grep):
Zephyr RTOS: no graphics support
FreeBSD: no H616 support
OpenBSD: no H616 graphics support
NetBSD: no H616 support
U-Boot: no H616 graphics support
Barebox: no H616 support
TF-A: no graphics support
OP-TEE: no (Allwinner) graphics support
EDK2: no Allwinner support
Cheers,
Andre
^ permalink raw reply
* Re: [PATCH v7 9/9] arm64: dts: mediatek: Add MediaTek MT6392 PMIC dtsi
From: Rob Herring @ 2026-06-16 13:39 UTC (permalink / raw)
To: Luca Leonardo Scorcia
Cc: linux-mediatek, Val Packett, Dmitry Torokhov, Krzysztof Kozlowski,
Conor Dooley, Sen Chu, Sean Wang, Macpaul Lin, Lee Jones,
Matthias Brugger, AngeloGioacchino Del Regno, Liam Girdwood,
Mark Brown, Linus Walleij, Louis-Alexis Eyraud, Julien Massot,
Fabien Parent, Akari Tsuyukusa, Chen Zhong, linux-input,
devicetree, linux-kernel, linux-pm, linux-arm-kernel, linux-gpio
In-Reply-To: <20260615071836.362883-10-l.scorcia@gmail.com>
On Mon, Jun 15, 2026 at 09:16:15AM +0200, Luca Leonardo Scorcia wrote:
> From: Val Packett <val@packett.cool>
>
> Add the dts to be included by all boards using the MT6392 PMIC,
> providing support for regulator, keys, pinctrl and RTC.
>
> Signed-off-by: Val Packett <val@packett.cool>
> Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>
> ---
> arch/arm64/boot/dts/mediatek/mt6392.dtsi | 75 ++++++++++++++++++++++++
> 1 file changed, 75 insertions(+)
> create mode 100644 arch/arm64/boot/dts/mediatek/mt6392.dtsi
Nothing is using this so it is a dead file that doesn't get tested.
Rob
^ permalink raw reply
* Re: [PATCH v7 0/9] Add support for MT6392 PMIC
From: Rob Herring @ 2026-06-16 13:42 UTC (permalink / raw)
To: Luca Leonardo Scorcia
Cc: linux-mediatek, Dmitry Torokhov, Krzysztof Kozlowski,
Conor Dooley, Sen Chu, Sean Wang, Macpaul Lin, Lee Jones,
Matthias Brugger, AngeloGioacchino Del Regno, Liam Girdwood,
Mark Brown, Linus Walleij, Julien Massot, Louis-Alexis Eyraud,
Val Packett, Fabien Parent, Akari Tsuyukusa, Chen Zhong,
linux-input, devicetree, linux-kernel, linux-pm, linux-arm-kernel,
linux-gpio
In-Reply-To: <20260615071836.362883-1-l.scorcia@gmail.com>
On Mon, Jun 15, 2026 at 09:16:06AM +0200, Luca Leonardo Scorcia wrote:
> The MediaTek MT6392 PMIC is usually found on devices powered by
> the MT8516/MT8167 SoC and is yet another MT6323/MT6397 variant.
>
> This series is mostly based around patches submitted a couple
> years ago by Fabien Parent and not merged and from Val Packett's
> submission from Jan 2025 that included extra cleanups, fixes, and a
> new dtsi file similar to ones that exist for other PMICs. Some
> comments weren't addressed and the series was ultimately not merged.
>
> These patches enable four functions: keys, regulator, pinctrl and RTC.
> Mono speaker amp will follow later as I need to work further on the
> audio codec.
>
> I added a handful of device tree improvements to fix some dtbs_check
> errors, added support for the pinctrl device and addressed the comments
> from last year's reviews.
>
> Please note that patch 0006 and 0008 depend on patch 0005 as they need the
> registers.h file, but belong to different driver areas. I'm not sure if
> I'm supposed to squash them even if they belong to different driver
> areas of if it's fine like this. Any advice is welcome.
>
> The series has been tested on Xiaomi Mi Smart Clock X04G and on the
> Lenovo Smart Clock 2 CD-24502F.
>
> Changes in v7:
> - Removed patch 0008 dependency on patch 0003.
> - Reintroduced the regulator driver. In earlier revisions of this series,
> it was proposed to remove the dedicated compatible for the regulator
> device [3]. The driver does not use actually it, but it is not possible
> at this time to remove it from the bindings since it's a required
> property.
>
> Making the regulator-required property conditional was NACKed in [5],
> with the suggestion to create a separate binding altogether for devices
> that do not require the compatible property. I tried implementing this,
> but since the parent device needs to be declared as compatible with
> mt6323, it leads to a warning in dt_binding_check since mt6323 would
> be declared as a compatible in both mt6392 and mt6397.
>
> In the end the only regulator driver from the mt6397 documentation that
> still declares an of_match is mt6397-regulator and it does not seem
> to be necessary, so it should be possible to remove it and make the
> regulator compatible optional for all regulators, but that change would
> probably deserve its own separate patch series.
I don't really follow what the issue is here, but compatible should
never be optional.
Rob
^ permalink raw reply
* Re: [PATCH v7 1/9] dt-bindings: mfd: mt6397: Add MT6392 PMIC
From: Rob Herring (Arm) @ 2026-06-16 13:43 UTC (permalink / raw)
To: Luca Leonardo Scorcia
Cc: Lee Jones, Mark Brown, linux-input, linux-pm, Louis-Alexis Eyraud,
linux-kernel, Julien Massot, Macpaul Lin, Krzysztof Kozlowski,
Liam Girdwood, Linus Walleij, Conor Dooley, Dmitry Torokhov,
Akari Tsuyukusa, AngeloGioacchino Del Regno, Chen Zhong,
Fabien Parent, linux-mediatek, Val Packett, linux-arm-kernel,
Matthias Brugger, devicetree, Sean Wang, Sen Chu, linux-gpio
In-Reply-To: <20260615071836.362883-2-l.scorcia@gmail.com>
On Mon, 15 Jun 2026 09:16:07 +0200, Luca Leonardo Scorcia wrote:
> Describe the MT6392 PMIC and its RTC and regulator devices. This device
> is mostly based on MT6323 with some similarities to MT6397 and is usually
> found on boards using the MT8516/MT8167 SoC.
>
> Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>
> ---
> .../devicetree/bindings/mfd/mediatek,mt6397.yaml | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
Acked-by: Rob Herring (Arm) <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v3 05/10] ARM: dts: qcom: msm8960: add RPM clock controller and fix USB clocks
From: Konrad Dybcio @ 2026-06-16 13:43 UTC (permalink / raw)
To: Antony Kurniawan Soemardi, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Lee Jones, Konrad Dybcio
Cc: Krzysztof Kozlowski, linux-arm-msm, linux-clk, devicetree,
linux-kernel, phone-devel, Rudraksha Gupta
In-Reply-To: <efb42f6c-b26a-4ee0-a5e7-0a25a0a41f50@smankusors.com>
On 6/16/26 3:04 PM, Antony Kurniawan Soemardi wrote:
> On 6/9/2026 7:21 PM, Konrad Dybcio wrote:
>> On 6/1/26 10:51 AM, Antony Kurniawan Soemardi via B4 Relay wrote:
>>> From: Antony Kurniawan Soemardi <linux@smankusors.com>
>>> @@ -507,8 +519,12 @@ usb1: usb@12500000 {
>>> reg = <0x12500000 0x200>,
>>> <0x12500200 0x200>;
>>> interrupts = <GIC_SPI 100 IRQ_TYPE_LEVEL_HIGH>;
>>> - clocks = <&gcc USB_HS1_XCVR_CLK>, <&gcc USB_HS1_H_CLK>;
>>> - clock-names = "core", "iface";
>>> + clocks = <&gcc USB_HS1_H_CLK>,
>>> + <&rpmcc RPM_DAYTONA_FABRIC_CLK>,
>>> + <&gcc USB_HS1_XCVR_CLK>;
>>> + clock-names = "iface",
>>> + "core",
>>> + "fs";
>>
>> The bindings change you sent changes the expectations - "core" used
>> to be the first clock. And I would guesstimate that the
>> DAYTONA_FABRIC clock is not really "core" - does downstream do any
>> ratesetting on the other two?
>
> Looking at the downstream, I can only find HS1_XCVR being set to 60MHz, DAYTONA_FABRIC being set to the max rate (just for voting purposes?). I don't see any clk_set_rate for HS1_P though.
>
> Would you rather the other way around? Like "core", "iface", and "fs"? My concern is that such a change would result in a large number of warnings for newer SoC device trees.
I didn't notice you're actually aligning the order with bindings. I was
under the impression this was a random change.
For the clock assignments themselves, I think the schema reflects a
full-speed (i.e. usb 1.x) core.. I dug out some ancient doc that says
that we should have:
- ahb (bus clock - perhaps daytona in this case?)
- system (core clock for the thing, >55 Mhz for compliant HS operation
or at least 35 MHz for any sort of operation)
- ulpi_clk (60 MHz, coming from the USB PHY) (we can probably ignore this
in our description)
- inactivity_timer since there's a BAM instance attached to this host
(possibly handled implicitly)
for the record, there's 4 hosts:
USB1_HS @ 0x12500000 (this one)
USB2_HSIC @ 0x12520000
USB1_FS @ 0x18000000
USB2_FS @ 0x18100000
I don't know if they are all exposed and functional though
Konrad
^ permalink raw reply
* Re: [PATCH v8 3/5] iio: adc: versal-sysmon: add I2C driver
From: sashiko-bot @ 2026-06-16 13:44 UTC (permalink / raw)
To: Salih Erim; +Cc: robh, devicetree, conor+dt
In-Reply-To: <20260616131559.3029543-4-salih.erim@amd.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] If a device tree specifies an IRQ for the I2C device, the core driver will register a hardirq handler that calls sleeping I2C regmap functions.
--
commit 08d83b16d0694386536f5b4f39bdb131024076df
Author: Salih Erim <salih.erim@amd.com>
iio: adc: versal-sysmon: add I2C driver
This commit adds an I2C transport driver for the Versal SysMon block. It
creates an I2C regmap backend and delegates to devm_versal_sysmon_core_probe()
to initialize the device.
> diff --git a/drivers/iio/adc/versal-sysmon-i2c.c b/drivers/iio/adc/versal-sysmon-i2c.c
> new file mode 100644
> index 0000000000000..cd8d5846ff9b2
> --- /dev/null
> +++ b/drivers/iio/adc/versal-sysmon-i2c.c
[ ... ]
> +static int sysmon_i2c_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + struct regmap *regmap;
> +
> + regmap = devm_regmap_init(dev, NULL, client, &sysmon_i2c_regmap_config);
> + if (IS_ERR(regmap))
> + return PTR_ERR(regmap);
> +
> + /* I2C has no IRQ connection; events are not supported */
> + return devm_versal_sysmon_core_probe(dev, regmap);
[Severity: High]
If a device tree incorrectly provides an interrupts property for the I2C
node, will devm_versal_sysmon_core_probe() register an interrupt handler?
Looking at devm_versal_sysmon_core_probe() in versal-sysmon-core.c:
irq = fwnode_irq_get(dev_fwnode(dev), 0);
...
if (irq > 0) {
...
ret = sysmon_init_interrupt(sysmon, dev, indio_dev, irq);
If an interrupt is registered, the handler sysmon_iio_irq() will run in
hardirq context, acquire a spinlock, and call regmap_read():
guard(spinlock)(&sysmon->irq_lock);
if (regmap_read(sysmon->regmap, SYSMON_ISR, &isr) || ...
Because the I2C regmap backend uses i2c_master_recv() which can sleep,
won't this trigger a sleeping in atomic context panic?
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616131559.3029543-1-salih.erim@amd.com?part=3
^ permalink raw reply
* Re: [PATCH v4 2/6] pinctrl: qcom: Register functions before enabling pinctrl
From: Konrad Dybcio @ 2026-06-16 13:44 UTC (permalink / raw)
To: contact, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, MyungJoo Ham, Chanwoo Choi,
Guru Das Srinagesh, Linus Walleij, Rob Clark, Kees Cook,
Tony Luck, Guilherme G. Piccoli
Cc: linux-arm-msm, devicetree, linux-kernel, linux-gpio, phone-devel
In-Reply-To: <20260609-mainline-send-v1-sending-v4-2-83768fbf404d@alex-min.fr>
On 6/9/26 10:13 AM, Alexandre MINETTE via B4 Relay wrote:
> From: Alexandre MINETTE <contact@alex-min.fr>
>
> pinctrl consumers can request states while the pinctrl core enables the
> controller. On Qualcomm pinctrl drivers this can happen before the SoC
> function list has been registered, which leaves the function table
> incomplete during state lookup.
>
> On APQ8064 this can fail while claiming pinctrl hogs:
>
> apq8064-pinctrl 800000.pinctrl: invalid function ps_hold in map table
> apq8064-pinctrl 800000.pinctrl: error claiming hogs: -22
> apq8064-pinctrl 800000.pinctrl: could not claim hogs: -22
>
> Register Qualcomm pinctrl with devm_pinctrl_register_and_init(), add the
> SoC pin functions, and only then enable the pinctrl device.
>
> Signed-off-by: Alexandre MINETTE <contact@alex-min.fr>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v4 6/6] ARM: dts: qcom: Add Samsung Galaxy S4
From: Konrad Dybcio @ 2026-06-16 13:45 UTC (permalink / raw)
To: contact, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, MyungJoo Ham, Chanwoo Choi,
Guru Das Srinagesh, Linus Walleij, Rob Clark, Kees Cook,
Tony Luck, Guilherme G. Piccoli
Cc: linux-arm-msm, devicetree, linux-kernel, linux-gpio, phone-devel
In-Reply-To: <20260609-mainline-send-v1-sending-v4-6-83768fbf404d@alex-min.fr>
On 6/9/26 10:13 AM, Alexandre MINETTE via B4 Relay wrote:
> From: Alexandre MINETTE <contact@alex-min.fr>
>
> Add a device tree for the Samsung Galaxy S4, codenamed jflte.
>
> This has been tested on a Samsung Galaxy S4 GT-I9505. The initial support
> covers UART, USB peripheral mode with USB networking, the front LED and
> the physical buttons.
>
> Signed-off-by: Alexandre MINETTE <contact@alex-min.fr>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v2] arm64: dts: qcom: kodiak: avoid EFI overlap for ADSP remote heap
From: Konrad Dybcio @ 2026-06-16 13:48 UTC (permalink / raw)
To: Jianping Li, andersson, konradybcio, robh, krzk+dt, conor+dt
Cc: linux-arm-msm, devicetree, linux-kernel, ekansh.gupta, stable
In-Reply-To: <20260429073443.2027-1-jianping.li@oss.qualcomm.com>
On 4/29/26 9:34 AM, Jianping Li wrote:
> On KODIAK platforms boot can fail when the DT "adsp-rpc-remote-heap"
> reserved-memory region overlaps with firmware allocations (UEFI/EFI
> runtime). The kernel then reports failure to reserve the region and
> subsequent EFI runtime activity may trigger aborts.
>
> The remote heap node was described as a fixed "no-map" region, which
> turns it into a hard carveout. Replace it with a "shared-dma-pool"
> reserved memory region with reusable CMA-backed allocation, specifying
> alignment and size.
>
> This avoids hard carveouts and reduces the chance of conflicting with
> firmware memory maps while keeping an explicit pool for ADSP remote
> heap usage.
>
> Fixes: 90a58ffa9c55 ("arm64: dts: qcom: kodiak: Add memory region for audiopd")
> Cc: stable@kernel.org
> Signed-off-by: Jianping Li <jianping.li@oss.qualcomm.com>
> ---
> Patch v1: https://lore.kernel.org/all/20260423063502.484-1-jianping.li@oss.qualcomm.com/
> Changes in v2:
> - Correct the value of alloc-ranges
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH] arm64: dts: qcom: talos-evk-som: Enable Adreno 612 GPU
From: Konrad Dybcio @ 2026-06-16 13:49 UTC (permalink / raw)
To: Akhil P Oommen, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel, freedreno, Jie Zhang
In-Reply-To: <20260427-talos-evt-gpu-v1-1-d40b6dffa108@oss.qualcomm.com>
On 4/27/26 8:26 PM, Akhil P Oommen wrote:
> From: Jie Zhang <jie.zhang@oss.qualcomm.com>
>
> Enable GPU for talos-evk-som platform and provide path for zap
> shader.
>
> Signed-off-by: Jie Zhang <jie.zhang@oss.qualcomm.com>
> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v7 3/9] regulator: dt-bindings: Add MediaTek MT6392 PMIC
From: Rob Herring @ 2026-06-16 13:50 UTC (permalink / raw)
To: Luca Leonardo Scorcia
Cc: linux-mediatek, Dmitry Torokhov, Krzysztof Kozlowski,
Conor Dooley, Sen Chu, Sean Wang, Macpaul Lin, Lee Jones,
Matthias Brugger, AngeloGioacchino Del Regno, Liam Girdwood,
Mark Brown, Linus Walleij, Val Packett, Julien Massot,
Louis-Alexis Eyraud, Fabien Parent, Akari Tsuyukusa, Chen Zhong,
linux-input, devicetree, linux-kernel, linux-pm, linux-arm-kernel,
linux-gpio
In-Reply-To: <20260615071836.362883-4-l.scorcia@gmail.com>
On Mon, Jun 15, 2026 at 09:16:09AM +0200, Luca Leonardo Scorcia wrote:
> Add bindings for the regulators found in the MediaTek MT6392 PMIC,
> usually found in board designs using the MediaTek MT8516/MT8167 SoCs.
>
> Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>
> ---
> .../regulator/mediatek,mt6392-regulator.yaml | 234 ++++++++++++++++++
> .../regulator/mediatek,mt6392-regulator.h | 24 ++
> 2 files changed, 258 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/regulator/mediatek,mt6392-regulator.yaml
> create mode 100644 include/dt-bindings/regulator/mediatek,mt6392-regulator.h
>
> diff --git a/Documentation/devicetree/bindings/regulator/mediatek,mt6392-regulator.yaml b/Documentation/devicetree/bindings/regulator/mediatek,mt6392-regulator.yaml
> new file mode 100644
> index 000000000000..197041df4ba1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/regulator/mediatek,mt6392-regulator.yaml
> @@ -0,0 +1,234 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/regulator/mediatek,mt6392-regulator.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek MT6392 regulator
> +
> +maintainers:
> + - Luca Leonardo Scorcia <l.scorcia@gmail.com>
> +
> +description:
> + MT6392 is a power management system chip containing three buck converters and
> + 23 LDOs. All voltage regulators provided by the PMIC are described as
> + sub-nodes of this node.
> +
> +properties:
> + compatible:
> + items:
> + - const: mediatek,mt6392-regulator
> +
> + vproc-supply:
> + description: Supply for buck regulator vproc
> + vcore-supply:
> + description: Supply for buck regulator vcore
> + vsys-supply:
> + description: Supply for buck regulator vsys
> + avddldo-supply:
> + description: |
Don't need '|' if no formatting to preserve. Elsewhere too.
> + Supply for AVDD LDOs (vm, vio18, vcn18, vcamd, vcamio). According to the data sheet
> + this is an internal supply derived from vsys.
> + ldo1-supply:
> + description: Supply for LDOs group 1 (vaud28, vxo22, vaud22, vadc18, vcama, vrtc)
> + ldo2-supply:
> + description: Supply for LDOs group 2 (vcn35, vio28, vmc, vmch, vefuse, vdig18)
> + ldo3-supply:
> + description: Supply for LDOs group 3 (vusb, vemc3v3, vcamaf, vgp1, vgp2, vm25)
> +
> +patternProperties:
> + "^v(core|proc|sys)$":
> + description: Buck regulators
> + type: object
> + $ref: regulator.yaml#
> + properties:
> + regulator-allowed-modes:
> + description: |
> + BUCK regulators can set regulator-initial-mode and regulator-allowed-modes to
> + values specified in dt-bindings/regulator/mediatek,mt6392-regulator.h
> + items:
> + enum: [0, 1]
minItems: 1
maxItems: 2
? Because if there are only 2 modes, can't have more entries than that,
right? Though wouldn't 2 entries be the same as no property present
because I would assume the default is all modes. I shouldn't have to
assume though.
> + unevaluatedProperties: false
Place this after $ref. Easier to read than after indented blocks.
> +
> + "^v(adc18|camio|cn18|io18|xo22|m25|aud28|io28|rtc|usb)$":
> + description: LDOs with fixed output and mode setting
> + type: object
> + $ref: regulator.yaml#
> + properties:
> + regulator-allowed-modes:
> + description: |
> + LDO regulators can set regulator-initial-mode and regulator-allowed-modes to
> + values specified in dt-bindings/regulator/mediatek,mt6392-regulator.h
> + items:
> + enum: [0, 1]
> + unevaluatedProperties: false
> +
> + "^v(cama|dig18)$":
> + description: LDOs with fixed output without mode setting
> + type: object
> + $ref: regulator.yaml#
> + unevaluatedProperties: false
> +
> + "^v(aud22|camaf|camd|cn35|efuse|emc3v3|gp1|gp2|m|mc|mch)$":
> + description: LDOs with adjustable output
> + type: object
> + $ref: regulator.yaml#
> + properties:
> + regulator-allowed-modes: false
> + unevaluatedProperties: false
> +
> +required:
> + - compatible
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + regulators {
> + compatible = "mediatek,mt6392-regulator";
Drop the example. Put 1 complete example in the MFD schema rather than
incomplete examples.
Rob
^ 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