* [PATCH v33 2/5] i2c: aspeed: Read clock-frequency via i2c_parse_fw_timings()
From: Ryan Chen @ 2026-06-23 1:15 UTC (permalink / raw)
To: jk, andriy.shevchenko, Andi Shyti, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Joel Stanley, Andrew Jeffery,
Benjamin Herrenschmidt, Rayn Chen, Philipp Zabel
Cc: linux-i2c, devicetree, linux-arm-kernel, linux-aspeed,
linux-kernel, openbmc, Ryan Chen
In-Reply-To: <20260623-upstream_i2c-v33-0-6d5338fc56ed@aspeedtech.com>
Use i2c_parse_fw_timings() to read the standard "clock-frequency"
property, and fall back to "bus-frequency" only when the standard
property is absent.
This honors device trees written against the updated
aspeed,ast2600-i2c binding without silently falling back to 100 kHz,
while keeping existing in-tree device trees using "bus-frequency"
working.
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
Changes in v31:
- Zero-initialise `struct i2c_timings timings` so the bus-frequency
fallback runs when clock-frequency is absent (Sashiko AI review).
---
drivers/i2c/busses/i2c-aspeed.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index a26b74c71206..f00bd779146e 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -1000,6 +1000,7 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
const struct of_device_id *match;
struct aspeed_i2c_bus *bus;
struct clk *parent_clk;
+ struct i2c_timings timings = {};
int irq, ret;
bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
@@ -1025,12 +1026,18 @@ static int aspeed_i2c_probe_bus(struct platform_device *pdev)
}
reset_control_deassert(bus->rst);
- ret = of_property_read_u32(pdev->dev.of_node,
- "bus-frequency", &bus->bus_frequency);
- if (ret < 0) {
- dev_err(&pdev->dev,
- "Could not read bus-frequency property\n");
- bus->bus_frequency = I2C_MAX_STANDARD_MODE_FREQ;
+ i2c_parse_fw_timings(&pdev->dev, &timings, false);
+ if (timings.bus_freq_hz) {
+ bus->bus_frequency = timings.bus_freq_hz;
+ } else {
+ ret = of_property_read_u32(pdev->dev.of_node,
+ "bus-frequency",
+ &bus->bus_frequency);
+ if (ret < 0) {
+ dev_err(&pdev->dev,
+ "Could not read clock-frequency or bus-frequency property\n");
+ bus->bus_frequency = I2C_MAX_STANDARD_MODE_FREQ;
+ }
}
match = of_match_node(aspeed_i2c_bus_of_table, pdev->dev.of_node);
--
2.34.1
^ permalink raw reply related
* [PATCH v33 1/5] dt-bindings: i2c: Split AST2600 binding into a new YAML
From: Ryan Chen @ 2026-06-23 1:15 UTC (permalink / raw)
To: jk, andriy.shevchenko, Andi Shyti, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Joel Stanley, Andrew Jeffery,
Benjamin Herrenschmidt, Rayn Chen, Philipp Zabel
Cc: linux-i2c, devicetree, linux-arm-kernel, linux-aspeed,
linux-kernel, openbmc, Ryan Chen, Conor Dooley
In-Reply-To: <20260623-upstream_i2c-v33-0-6d5338fc56ed@aspeedtech.com>
The AST2600 I2C controller introduces a completely new register layout
with separate controller and target register blocks, unlike the mixed
register layout used by AST2400/AST2500.
Move AST2600 I2C binding from aspeed,i2c.yaml to a dedicated
aspeed,ast2600-i2c.yaml schema.
Besides the split, this also adjusts for AST2600-specific requirements.
- describe two reg regions (controller register block + buffer block);
the second region is optional (minItems: 1) so existing AST2600 DTs
that only declare the controller register block continue to validate
- use clock-frequency for bus speed description
- interrupts are required on AST2600
- use correct DTS coding style in example
No compatible strings are changed.
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
Changes in v31:
- Commit message body: clarify that the second reg region is optional
(minItems: 1) rather than required, matching the schema and the v30
backward-compatibility fix (Sashiko AI review).
Changes in v30:
- Add minItems: 1 to reg so existing AST2600 DTs with a single reg
region continue to validate (Sashiko AI review)
- Retain bus-frequency as a deprecated property to avoid breaking
existing AST2600 DTs under unevaluatedProperties: false
(Sashiko AI review)
Changes in v26:
- commit message: include details of changes from original binding
- fix example property ordering to follow DTS coding style
- use consistent "AST2600" naming
---
.../bindings/i2c/aspeed,ast2600-i2c.yaml | 73 ++++++++++++++++++++++
.../devicetree/bindings/i2c/aspeed,i2c.yaml | 3 +-
MAINTAINERS | 1 +
3 files changed, 75 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml b/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
new file mode 100644
index 000000000000..abc614315dff
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
@@ -0,0 +1,73 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/aspeed,ast2600-i2c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ASPEED I2C on the AST2600 SoCs
+
+maintainers:
+ - Ryan Chen <ryan_chen@aspeedtech.com>
+
+allOf:
+ - $ref: /schemas/i2c/i2c-controller.yaml#
+
+properties:
+ compatible:
+ enum:
+ - aspeed,ast2600-i2c-bus
+
+ reg:
+ minItems: 1
+ items:
+ - description: controller registers
+ - description: controller buffer space
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ clock-frequency:
+ description: Desired operating frequency of the I2C bus in Hz.
+ minimum: 500
+ maximum: 4000000
+ default: 100000
+
+ bus-frequency:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ deprecated: true
+ description:
+ Legacy name for clock-frequency. Existing AST2600 device trees
+ used this before the binding was split out. New device trees
+ should use the standard clock-frequency property instead.
+ minimum: 500
+ maximum: 4000000
+
+ resets:
+ maxItems: 1
+
+required:
+ - reg
+ - compatible
+ - clocks
+ - resets
+ - interrupts
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/aspeed-clock.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ i2c@80 {
+ compatible = "aspeed,ast2600-i2c-bus";
+ reg = <0x80 0x80>, <0xc00 0x20>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ clocks = <&syscon ASPEED_CLK_APB>;
+ resets = <&syscon ASPEED_RESET_I2C>;
+ clock-frequency = <100000>;
+ interrupts = <GIC_SPI 110 IRQ_TYPE_LEVEL_HIGH>;
+ };
diff --git a/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml b/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
index 5b9bd2feda3b..d4e4f412feba 100644
--- a/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
@@ -4,7 +4,7 @@
$id: http://devicetree.org/schemas/i2c/aspeed,i2c.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#
-title: ASPEED I2C on the AST24XX, AST25XX, and AST26XX SoCs
+title: ASPEED I2C on the AST24XX, AST25XX SoCs
maintainers:
- Rayn Chen <rayn_chen@aspeedtech.com>
@@ -17,7 +17,6 @@ properties:
enum:
- aspeed,ast2400-i2c-bus
- aspeed,ast2500-i2c-bus
- - aspeed,ast2600-i2c-bus
reg:
minItems: 1
diff --git a/MAINTAINERS b/MAINTAINERS
index 882214b0e7db..f9c929e86e64 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2596,6 +2596,7 @@ R: Joel Stanley <joel@jms.id.au>
L: linux-i2c@vger.kernel.org
L: openbmc@lists.ozlabs.org (moderated for non-subscribers)
S: Maintained
+F: Documentation/devicetree/bindings/i2c/aspeed,ast2600-i2c.yaml
F: Documentation/devicetree/bindings/i2c/aspeed,i2c.yaml
F: Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2400-i2c-ic.yaml
F: drivers/i2c/busses/i2c-aspeed.c
--
2.34.1
^ permalink raw reply related
* [PATCH v33 0/5] Add ASPEED AST2600 I2C controller driver
From: Ryan Chen @ 2026-06-23 1:15 UTC (permalink / raw)
To: jk, andriy.shevchenko, Andi Shyti, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Joel Stanley, Andrew Jeffery,
Benjamin Herrenschmidt, Rayn Chen, Philipp Zabel
Cc: linux-i2c, devicetree, linux-arm-kernel, linux-aspeed,
linux-kernel, openbmc, Ryan Chen, Conor Dooley
This series adds support for the AST2600 I2C controller “new register
set” implementation.
The AST2600 I2C controller introduces a revised register layout which
separates controller and target functionality into distinct register
blocks, and extends clock divider configuration and packet-based
transfer support compared to the legacy mixed register layout used on
earlier ASPEED SoCs.
The current driver implementation for the AST2600 I2C peripheral is
through the hardware's "compatibility mode", which exposes a register
set that matches the previous generation hardware (AST2500 and earlier).
Instead, add a driver that works in new-register-set mode, to allow the
new features, and will provide support for future hardware that will
not implement compatibility mode.
In order to support the new mode, we need a DT binding change to
reflect the reference to the global register set. Since the binding
still represents the same (AST2600 SoC) physical hardware, we continue
to use the existing compatible string of "aspeed,ast2600-i2c-bus".
However: since we're changing semantics for an existing binding, we
allow backwards compatibility by selecting on presence/absence of the
newly-added properties, and fall back to the old driver (ie., in
compatibility mode) when we detect a DT using the old binding spec.
Specifically:
- ast2600-i2c-bus nodes that provide the `aspeed,global-regs` property
(present in the new binding and absent in the legacy binding) will be
successfully probed by the new driver
- ast2600-i2c-bus nodes without `aspeed,global-regs` continue to use the
existing driver (in legacy register mode), ensuring that platforms
with the current DTBs remain functional
Signed-off-by: Ryan Chen <ryan_chen@aspeedtech.com>
---
Changes in v33:
- 4/5: Fix missing NAK on the last byte of an intermediate read
message: set RX_CMD_LAST on the final chunk of every read message
and append STOP_CMD only when it is also the last message of the
transfer.
- Link to v32: https://lore.kernel.org/r/20260611-upstream_i2c-v32-0-b66eba921d01@aspeedtech.com
Changes in v32:
- 1/5: add MAINTAINERS entry for aspeed,ast2600-i2c.yaml in the same
patch that creates the file.
- 3/5: add if/then conditional schema: when aspeed,global-regs is
present, require reg to have at least two items. The new driver
unconditionally maps resource index 1 (the buffer SRAM region); a DT
with one reg entry and aspeed,global-regs passes schema validation but
fails probe. The constraint makes the schema consistent with driver
behaviour.
- 4/5: address follow-on code review issues:
- add MAINTAINERS entry for drivers/i2c/busses/i2c-ast2600.c.
- fix interrupt storm when msgs is NULL: clear PKT_DONE in the IRQ
handler; per the datasheet this auto-clears all associated status
bits.
- fix out-of-bounds: guard msgs_index against msgs_count before
indexing msgs array in ast2600_i2c_controller_packet_irq().
- fix use-after-free: WRITE_ONCE() null msgs before complete() in
all IRQ completion paths so trailing IRQs bail out immediately.
- fix race in timeout path: null msgs before re-enabling IER so a
late IRQ cannot access the caller's freed message buffer.
- fix 0-length SMBus block read hanging the bus: issue STOP via
CONTROLLER_TRIGGER_LAST_STOP, set stop_pending, poll for NORMAL_STOP.
- initialise clk_div_reg to I2CCG_DIV_CTRL and global_ctrl to 0
to avoid uninitialized values if regmap_read() fails.
- guard against clock-frequency = <0> in DT; default to 100 kHz
to prevent divide-by-zero in ast2600_i2c_ac_timing_config().
- remove AST2600_I2CM_BUS_RECOVER_FAIL from IER writes; bit 15 is
Reserved in I2CM10 (IER) and only exists as a status bit in I2CM14.
- 5/5: address follow-on code review issues:
- fix target RX data loss in master-abort path: remove BUFF_CTRL
zeroing that discarded pending target RX data stored in bits [29:24].
- fix use-after-free in master-abort path: null msgs and re-enable
IER before complete(), not after, preventing stale IRQ from touching
the newly-installed msgs of the next transfer.
- fix shared-buffer corruption on coalesced STOP+SLAVE_MATCH IRQ:
restore the SLAVE_PENDING guard on target_active = false. SLAVE_PENDING
(bit 29) is set when a new address-match is queued before the previous
DMA receive completes; clearing target_active in that case allows the
controller to overwrite the shared Tx/Rx buffer.
- use READ_ONCE() for all process-context reads of target_active;
the IRQ path writes it with WRITE_ONCE() and plain loads allow the
compiler to cache a stale value across the IER-disable window.
- Link to v31: https://lore.kernel.org/r/20260603-upstream_i2c-v31-0-ba7a02714f22@aspeedtech.com
Changes in v31:
- 1/5: clarify in the commit message that the second reg region is
optional (minItems: 1), matching the schema change from v30.
- 2/5: zero-initialise struct i2c_timings so the bus-frequency fallback
correctly triggers when clock-frequency is absent in the DT.
- 4/5: fix zero-length RX: ast2600_i2c_setup_buff_rx() now returns
-EINVAL for xfer_len <= 0, propagated through the controller packet
IRQ handler to abort the transfer instead of hanging until SW timeout.
- 4/5: address follow-on code review issues:
- Guard controller_packet_irq() against NULL msgs (post-timeout UAF).
- Clamp HW-reported xfer_len via ast2600_i2c_clamp_len() in TX_ACK
and RX_DONE to prevent out-of-bounds writes on HW length glitches.
- Use regmap_update_bits() for I2CG_CTRL to avoid clobbering shared
global bits across parallel bus probes (TOCTOU fix).
- Fix SMBus block read with recv_len == 0: set controller_xfer_cnt =
msg->len to satisfy the "msg done" check without an extra 1-byte RX.
- Mirror the controller timeout sequence in recover_bus() timeout path
(disable IER, synchronize_irq(), W1C ISR, reset master, restore IER).
- Remove unused #include <linux/of_device.h>.
- Remove dead adap.algo_data assignment in probe().
- 5/5: address follow-on target-mode code review issues:
- Clear target_active on any STOP (not just STOP without SLAVE_PENDING),
fixing a deadlock under coalesced IRQ events.
- Enable target IER in reg_target() rather than unconditionally in
probe(), matching the disable in unreg_target().
- Re-arm HW in SLAVE_PENDING|RX_DONE|WAIT_TX_DMA|STOP ISR case
(missing CMD_STS write left bus SCL-stretched until INACTIVE_TO).
- Default target ISR case: write TARGET_TRIGGER_CMD instead of
silently breaking, preventing bus hang on unhandled states.
- W1C-clear ADDR1/2/3_NAK bits in HW in target_irq() to prevent
stale NAK bits from bouncing controller transfers with -EBUSY.
- unreg_target(): write 0 to ADDR_CTRL instead of masking with
ADDR1_MASK, which left ADDR1_ENABLE (BIT(7)) set after unregister.
- Link to v30: https://lore.kernel.org/r/20260528-upstream_i2c-v30-0-5d4f9adc3530@aspeedtech.com
Changes in v30:
- 1/5: aspeed,ast2600-i2c.yaml: keep backward compatibility for
existing in-tree AST2600 device trees (Sashiko AI review).
- reg: add minItems: 1 so legacy single-reg DTs still validate.
- retain bus-frequency as a deprecated property so DTs that still
use it are not rejected by unevaluatedProperties: false.
- 2/5: new patch "i2c: aspeed: Read clock-frequency via
i2c_parse_fw_timings()". The legacy i2c-aspeed driver now reads
the standard clock-frequency property first and falls back to
bus-frequency, avoiding a silent 100 kHz downgrade when a DT
follows the updated binding but still binds to the legacy
driver (Sashiko AI review).
- 4/5: address Sashiko AI code review feedback:
- Use manual i2c_add_adapter() / i2c_del_adapter() instead of
devm_i2c_add_adapter() so the adapter is torn down before the
hardware is disabled in remove(); otherwise client .remove()
callbacks can fail or hang after FUN_CTRL/IER have been cleared.
- synchronize_irq() and clear pending IRQ status on the controller
timeout path to avoid the ISR racing with the next transfer and
touching freed msgs.
- Use clamp_t() for AC TIMING divisor / scl_low / scl_high so
extreme clock-frequency values cannot underflow into the unsigned
domain and corrupt the AC TIMING register.
- Derive the RX buffer offset from buf_size instead of hardcoding
0x10, since the dual-pool split is configurable.
- Clamp i2c-scl-clk-low-timeout-us to the TTIMEOUT field's 5-bit
range (max 31 * 1024us) and emit a dev_warn() instead of letting
AST2600_I2CC_TTIMEOUT()'s mask silently truncate larger values.
- Return -EBUSY (not -ENOMEM) for every ast2600_i2c_do_start()
failure path in the controller packet IRQ handler (NORMAL_STOP,
TX_ACK, and RX_DONE branches).
- Advertise I2C_AQ_NO_ZERO_LEN_READ via i2c_adapter_quirks so the
i2c-core rejects zero-byte reads before they reach the driver.
The AST2600 packet engine cannot encode a zero-length RX command
and would otherwise stall waiting for an RX_DONE that never
arrives.
- 5/5: address Sashiko AI code review feedback:
- Force-stop path (target IRQ aborting an in-flight controller
transfer): disable the controller IER and W1C-clear pending ISR
before calling complete(), then restore the IER after the
wake-up. Without the disable/clear sequence the controller IRQ
handler can race with the target abort path and double-complete
or touch freed msgs.
- unreg_target() teardown ordering: disable the target IER first,
then disable SLAVE_EN / clear ADDR_CTRL, synchronize_irq(), W1C
pending ISR, and only then NULL i2c_bus->target and clear
target_active. The old order left IER enabled while target was
being cleared, allowing an in-flight handler to dereference a
target pointer the caller had already freed.
- reg_target() bring-up ordering: assign i2c_bus->target before
enabling SLAVE_EN. Otherwise an IRQ that fires after SLAVE_EN
is set but before the pointer is stored finds target == NULL,
exits without clearing the ISR, and the unmasked event re-fires
as an IRQ storm.
- Use writel() instead of writeb() when staging a TX byte into
the target buffer. The AST2600 buffer SRAM only supports 32-bit
accesses; byte writes are silently dropped (or, on some
revisions, raise a bus fault), so a SLAVE_READ_REQUESTED reply
never reaches the master.
- reg_target() rejects 10-bit client addresses with
-EAFNOSUPPORT. AST2600_I2CS_ADDR1 is only a 7-bit field;
without the check, the high bits of a 10-bit address overflow
into the adjacent ADDR2 field and silently corrupt a second
target slot.
- Initialise the local `u8 value` to 0 in the target packet IRQ
handler. Its address is passed to i2c_slave_event() for events
such as I2C_SLAVE_STOP / I2C_SLAVE_READ_REQUESTED; a slave
backend that reads the byte before writing would otherwise leak
uninitialised kernel stack.
- Link to v29: https://lore.kernel.org/r/20260415-upstream_i2c-v29-0-317c1a905ae1@aspeedtech.com
Changes in v29:
- 2/4: remove aspeed,enable-dma properties.
- 3/4: update commit message remove transfer mode selection.
- 3/4: remove sysfs file.
- 3/4: remove define I2C_TARGET_MSG_BUF_SIZE and AST2600_I2C_DMA_SIZE.
- 3/4: remove buf_index in struct ast2600_i2c_bus.
- 3/4, 4/4: remove dma/byte mode, use buffer mode only.
- 4/4: fix race between unreg_target and IRQ handler.
- 4/4: move i2cs ier enable from ast2600_i2c_init to probe after master ier enable.
- Link to v28: https://lore.kernel.org/r/20260330-upstream_i2c-v28-0-17bdae39c5cb@aspeedtech.com
Changes in v28:
- 2/4: update commit message correspond with aspeed,enable-dma.
- 2/4: remove aspeed,transfer-mode and add aspeed,enable-dma property
and description.
- 2/4: Fix aspeed,enable-dma description to reflect hardware capability
rather than software behavior.
- 3/4: Separate xfer_mode_store into distinct parse and availability-check
steps by introducing ast2600_i2c_xfer_mode_check().
- 3/4: fix tx dma memcpy source point address.
- 3/4: Use a temporary variable for devm_platform_get_and_ioremap_resource()
to avoid storing an ERR_PTR in i2c_bus->buf_base; drop the redundant
NULL assignment in the error path since i2c_bus is kzalloc()ed.
- 3/4: Add ABI documentation file
Documentation/ABI/testing/sysfs-driver-ast2600-i2c.
- 4/4: fix typo condication -> condition.
- 4/4: fix compile error, when disable CONFIG_I2C_SLAVE.
- Link to v27: https://lore.kernel.org/r/20260324-upstream_i2c-v27-0-f19b511c8c28@aspeedtech.com
Changes in v27:
- 1/4 use aspeed,enable-dma instead aspeed,transfer-mode.
- 2/4 remove aspeed,transfer-mode selection instad aspeed,transfer-mode
- 2/4 add sysfs for xfer mode.
- Link to v26: https://lore.kernel.org/r/20260309-upstream_i2c-v26-0-5fedcff8ffe8@aspeedtech.com
Changes in v26:
- 1/4: binding reworks based on review feedback
- Link to v25: https://lore.kernel.org/r/20260225-upstream_i2c-v25-0-9f4bdd954f3f@aspeedtech.com
Changes in v25:
- Use b4 to send series.
- Rebase on v7.0-rc1.
- Clarify cover letter and commit logs based on review feedback.
- Remove the i2c-aspeed-core multiplexer infrastructure and
implement driver selection via conditional -ENODEV handling
in individual probe() functions.
- 3/4: incorporate review feedback and refactor new driver
- Link to v24: https://lore.kernel.org/r/20251118014034.820988-1-ryan_chen@aspeedtech.com
Changes in v24:
- aspeed,ast2600-i2c.yaml
- fix make dt_binding_check blank warning.
- Link to v23: https://lore.kernel.org/all/20251117025040.3622984-1-ryan_chen@aspeedtech.com/
Changes in v23:
- update typo patch (1/4) commit message.
- aspeed,ast2600-i2c.yaml
- update reg and description.
- i2c-ast2600.c controller
- replace ast2600_select_i2c_clock to ast2600_i2c_ac_timing_config.
- i2c-ast2600.c target
- I2C_TARGET_MSG_BUF_SIZE 256 to 4096
- remove blank line.
- refine Master comment description to controller
- Link to v22: https://lore.kernel.org/all/20251112085649.1903631-1-ryan_chen@aspeedtech.com/
Changes in v22:
- update patch (1/4) commit message add dts example reason.
- aspeed,ast2600-i2c.yaml @patch (1/4)
- rename ast2600-i2c.yaml to aspeed,ast2600-i2c.yaml.
- update reg, clock-frequency description.
- aspeed,ast2600-i2c.yaml @patch (2/4)
- aspeed,transfer-mode, aspeed,transfer-mode add for ast2600.
- i2c-aspeed-core.c,h @patch (3/4)
- add i2c-aspeed-core allow both old and new device trees using the
same compatible string "aspeed,ast2600-i2c-bus".
- Link to v21: https://lore.kernel.org/all/20251027061240.3427875-1-ryan_chen@aspeedtech.com/
Changes in v21:
- update patch (1/4) commit message
- i2c-ast2600.c
- move rst to local variable in ast2600_i2c_probe().
- Link to v20: https://lore.kernel.org/all/20251021013548.2375190-1-ryan_chen@aspeedtech.com/
Changes in v20:
- ast2600-i2c.yaml
- fix warning at make dt_binding_check.
- Link to v19: https://lore.kernel.org/all/20251020013200.1858325-1-ryan_chen@aspeedtech.com/
Changes in v19:
- Split AST2600 binding into its own YAML file
- Removed `aspeed,ast2600-i2c-bus` from `aspeed,i2c.yaml`
- Added `aspeed,global-regs` and `aspeed,transfer-mode` to AST2600 binding
- Link to v18: https://lore.kernel.org/all/20250820051832.3605405-1-ryan_chen@aspeedtech.com/
Changes in v18:
- refine patch (1/3) commit message (reason for commit not list.)
- i2c-ast2600.c
- remove redundant reset_control_deassert in driver probe.
- remove reset_control_assert(i2c_bus->rst) in driver remove.
- Link to v17: https://lore.kernel.org/all/20250814084156.1650432-1-ryan_chen@aspeedtech.com/
Changes in v17:
- move i2c new mode register and feature into driver commit message.
- aspeed,i2c.yaml
- remove multi-master properties.
- use aspeed,transfer-mode properties for aspeed,enable-byte/enable-dma.
-i2c-ast2600.c
- rename dma_safe_buf to controller_dma_safe_buf.
- fix ast2600_i2c_recover_bus return overflow warnings.
- add ast2600_i2c_target_packet_buff_irq unhandle case.
- add parameter "cmd" in ast2600_i2c_setup_dma_rx,
ast2600_i2c_setup_buff_rx, ast2600_i2c_setup_byte_rx
- use reset_control_deassert replace
devm_reset_control_get_shared_deasserted.
- useaspeed,transfer-mode properties for transfer mode setting.
- change compatible = "aspeed,ast2600-i2cv2" to "aspeed,ast2600-i2c-bus".
- Link to v16: https://lore.kernel.org/all/20250224055936.1804279-1-ryan_chen@aspeedtech.com/
Changes in v16:
- aspeed,i2c.yaml: add aspeed,enable-byte properties for force byte mode.
- i2c-ast2600.c
- change include asm/unaligned.h to linux/unaligned.h.
- add reset timeout councter when slave active timeout.
- modify issue i2c_recovery_bus before slave re-enable.
- add aspeed,enable-byte properties.
- Link to v15: https://lore.kernel.org/all/20241007035235.2254138-1-ryan_chen@aspeedtech.com/
Changes in v15:
- i2c-ast2600.c
- add include unaligned.h
- rename all master -> controller, slave -> target.
- keep multi-master to align property.
- remove no used element in ast2600_i2c_bus.
- Link to v14: https://lore.kernel.org/all/20241002070213.1165263-1-ryan_chen@aspeedtech.com/
Changes in v14:
- aspeed,i2c.yaml
- v13 change people reviewed-by tag, v14 fixed to original people tag,
modify to Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
- struct ast2600_i2c_bus layout optimal.
- ast2600_select_i2c_clock refine.
- ast2600_i2c_recover_bus overridden fix.
- dma_mapping_error() returned error code shadowed modify.
- buffer register in a 4-byte aligned simplified
- remove smbus alert
- Link to v13: https://lore.kernel.org/all/20240819092850.1590758-1-ryan_chen@aspeedtech.com/
Changes in v13:
- separate i2c master and slave driver to be two patchs.
- modify include header list, add bits.h include. remove of*.h
- modify (((x) >> 24) & GENMASK(5, 0)) to (((x) & GENMASK(29, 24)) >> 24)
- modify ast2600_select_i2c_clock function implement.
- modify ast2600_i2c_recover_bus function u32 claim to
u32 state = readl(i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF);
- Link to v12: https://lore.kernel.org/all/20230714074522.23827-1-ryan_chen@aspeedtech.com/
Changes in v12:
- aspeed,i2c.yaml
- add Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
- i2c-ast2600.c
- update include by alphabetical order
- make just a one TAB and put the last two lines on the single one
- remove no used timing_table structre
- remove enum explicit assinment
- rewritten to avoid this and using loop in ast2600_select_i2c_clock
- use GENMASK for most 0xffff
- remove too many parentheses
- use str_read_write replace read write string
- remove redundant blank line after ast2600_i2c_bus_of_table
- fix wrong multi-line style of the comment
- use macro for i2c standard speeds
- remove useless noise dev_info
- Link to v11: https://lore.kernel.org/all/20230430041712.3247998-1-ryan_chen@aspeedtech.com/
Changes in v11:
- aspeed,i2c.yaml
- no change, the same with v10.
- i2c-ast2600.c
- modify alert_enable from int -> boolean.
- modify dbg string recovery -> recover.
- remove no need to init 0.
- remove new line after break.
- remove unneeded empty line.
- modify dma_alloc_coherent to dmam_alloc_coherent
- modify probe nomem return dev_err_probe
- modify i2c_add_adapter to devm_i2c_adapter
- modify checkpatch: Alignment should match open parenthesis
- modify checkpatch: braces {} should be used on all arms of this statement
- modify checkpatch: Unbalanced braces around else statement
- Link to v10: https://lore.kernel.org/all/20230415012848.1777768-1-ryan_chen@aspeedtech.com/
Changes in v10:
- aspeed,i2c.yaml
- move unevaluatedProperties after allOf.
- remove extra one blank line.
- i2c-ast2600.c
- no change, the same with v8.
- Link to v9: https://lore.kernel.org/all/20230405022825.333246-1-ryan_chen@aspeedtech.com/
Changes in v9:
- aspeed,i2c.yaml
- backoff to v7.
- no fix typo in maintainer's name and email. this would be another patch.
- no remove address-cells, size-cells, this would be another patch.
- use aspeed,enable-dma property instead of aspeed,xfer-mode selection.
- fix allOf and else false properties for aspeed,ast2600-i2cv2.
- i2c-ast2600.c
- no change, the same with v8
- Link to v8: https://lore.kernel.org/all/20230330073259.485606-1-ryan_chen@aspeedtech.com/
Changes in v8:
- aspeed,i2c.yaml
- modify commit message.
- Fix typo in maintainer's name and email.
- remove address-cells, size-cells.
- i2c-ast2600.c
- move "i2c timeout counter" comment description before property_read.
- remove redundant code "return ret" in probe end.
- Link to v7: https://lore.kernel.org/all/20230327092524.3916389-1-ryan_chen@aspeedtech.com/
Changes in v7:
- aspeed,i2c.yaml
- Update ASPEED I2C maintainers email.
- use aspeed,enable-dma property instead of aspeed,xfer-mode selection.
- fix allOf and else false properties for aspeed,ast2600-i2cv2.
- i2c-ast2600.c
- remove aspeed,xfer-mode instead of aspeed,enable-dma mode. buffer mode
is default.
- remove aspeed,timeout instead of i2c-scl-clk-low-timeout-us for
timeout setting.
- Link to v6: https://lore.kernel.org/all/20230226031321.3126756-1-ryan_chen@aspeedtech.com/
Changes in v6:
- remove aspeed,i2cv2.yaml, merge to aspeed,i2c.yaml -add support for
i2cv2 properites.
- i2c-ast2600.c
- fix ast2600_i2c_remove ordering.
- remove ast2600_i2c_probe goto labels, and add dev_err_probe -remove
redundant deb_dbg debug message.
- rename gr_regmap -> global_regs
- Link to v5: https://lore.kernel.org/all/20230220061745.1973981-1-ryan_chen@aspeedtech.com/
Changes in v5:
- remove ast2600-i2c-global.yaml, i2c-ast2600-global.c.
- i2c-ast2600.c
- remove legacy clock divide, all go for new clock divide.
- remove duplicated read isr.
- remove no used driver match
- fix probe return for each labels return.
- global use mfd driver, driver use phandle to regmap read/write.
- rename aspeed,i2c-ast2600.yaml to aspeed,i2cv2.yaml -remove bus-frequency.
- add required aspeed,gr
- add timeout, byte-mode, buff-mode properites.
- Link to v4: https://lore.kernel.org/all/20230201103359.1742140-1-ryan_chen@aspeedtech.com/
Changes in v4:
- fix i2c-ast2600.c driver buffer mode use single buffer conflit in
master slave mode both enable.
- fix kmemleak issue when use dma mode.
- fix typo aspeed,i2c-ast2600.yaml compatible is "aspeed,ast2600-i2c"
- fix typo aspeed,i2c-ast2600.ymal to aspeed,i2c-ast2600.yaml
- Link to v3: https://lore.kernel.org/all/20220516064900.30517-1-ryan_chen@aspeedtech.com/
Changes in v3:
- fix i2c global clock divide default value.
- remove i2c slave no used dev_dbg info.
- Link to v2: https://lore.kernel.org/all/20220413101735.27678-1-ryan_chen@aspeedtech.com/
Changes in v2:
- add i2c global ymal file commit.
- rename file name from new to ast2600.
aspeed-i2c-new-global.c -> i2c-ast2600-global.c
aspeed-i2c-new-global.h -> i2c-ast2600-global.h
i2c-new-aspeed.c -> i2c-ast2600.c
- rename all driver function name to ast2600.
- Link to v1: https://lore.kernel.org/all/20220323004009.943298-1-ryan_chen@aspeedtech.com/
---
Ryan Chen (5):
dt-bindings: i2c: Split AST2600 binding into a new YAML
i2c: aspeed: Read clock-frequency via i2c_parse_fw_timings()
dt-bindings: i2c: ast2600-i2c.yaml: Add global-regs properties
i2c: ast2600: Add controller driver for AST2600 new register set
i2c: ast2600: Add target mode support
.../bindings/i2c/aspeed,ast2600-i2c.yaml | 88 ++
.../devicetree/bindings/i2c/aspeed,i2c.yaml | 3 +-
MAINTAINERS | 2 +
drivers/i2c/busses/Makefile | 2 +-
drivers/i2c/busses/i2c-aspeed.c | 24 +-
drivers/i2c/busses/i2c-ast2600.c | 1292 ++++++++++++++++++++
6 files changed, 1402 insertions(+), 9 deletions(-)
---
base-commit: a293ec25d59dd96309058c70df5a4dd0f889a1e4
change-id: 20260223-upstream_i2c-ebd07f89739c
Best regards,
--
Ryan Chen <ryan_chen@aspeedtech.com>
^ permalink raw reply
* Re: [PATCH 3/8] arm64: dts: qcom: sm8450: Modify GPU operating points
From: sashiko-bot @ 2026-06-23 1:10 UTC (permalink / raw)
To: Esteban Urrutia via B4 Relay
Cc: devicetree, robh, neil.armstrong, vkoul, conor+dt, olteanv,
linux-phy
In-Reply-To: <20260622-sm8450-qol-v1-3-37e2ee8df9da@proton.me>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Removing the 3 lowest GPU OPPs breaks the 1:1 index mapping between the GPU frequency table and the hardcoded GMU bandwidth table in the Adreno DRM driver, causing severe memory bandwidth starvation at high GPU frequencies.
--
commit d0ec9d0cbceaf15f2295a4c6a613a8ca7a354eea
Author: Esteban Urrutia <esteuwu@proton.me>
arm64: dts: qcom: sm8450: Modify GPU operating points
This commit removes the 317MHz, 285MHz, and 220MHz GPU operating points from
the SM8450 device tree, as they do not exist in the downstream device trees.
> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> index e34e3c05bf74c..5e331a25e22a2 100644
> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> @@ -2495,21 +2495,6 @@ opp-350000000 {
> opp-hz = /bits/ 64 <350000000>;
> opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
> };
> -
> - opp-317000000 {
> - opp-hz = /bits/ 64 <317000000>;
> - opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
> - };
[Severity: High]
Will removing these 3 lowest GPU OPPs break the index mapping between the
GPU frequency table and the GMU bandwidth table in the Adreno driver?
The Adreno A730 driver doesn't use dynamic bandwidth voting. Instead, it
sends a hardcoded table of exactly 12 memory bandwidth levels to the GMU
firmware:
drivers/gpu/drm/msm/adreno/a6xx_hfi.c:a730_build_bw_table() {
...
msg->bw_level_num = 12;
...
}
The GMU firmware applies the bandwidth vote corresponding to the GPU
performance level index. Previously, with 12 GPU OPPs, the maximum frequency
mapped to the maximum bandwidth at index 11.
By removing 3 OPPs here, the active OPPs are reduced to 9. Does this mean the
maximum frequency of 818MHz will now map to performance index 8 instead?
If so, this would cause the GMU to apply the lower bandwidth vote from index 8
when the GPU is at maximum frequency, severely reducing the available memory
bandwidth and bottlenecking the GPU under heavy load.
> -
> - opp-285000000 {
> - opp-hz = /bits/ 64 <285000000>;
> - opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS_D1>;
> - };
> -
> - opp-220000000 {
> - opp-hz = /bits/ 64 <220000000>;
> - opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS_D1>;
> - };
> };
> };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260622-sm8450-qol-v1-0-37e2ee8df9da@proton.me?part=3
^ permalink raw reply
* Re: [PATCH 5/8] iommu/arm-smmu-qcom: Add SM8450 MDSS compatible
From: sashiko-bot @ 2026-06-23 1:07 UTC (permalink / raw)
To: Esteban Urrutia via B4 Relay
Cc: robh, devicetree, vkoul, olteanv, conor+dt, linux-phy,
neil.armstrong
In-Reply-To: <20260622-sm8450-qol-v1-5-37e2ee8df9da@proton.me>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The SMMU client match table is missing several other modern Qualcomm MDSS compatibles (e.g., SM8350, SM8550, SM8650, SM8750, SM6125), causing them to suffer from unhandled context faults and broken display functionality.
--
commit d66a67918c0ac7790f3063bd520102cd893aafb7
Author: Esteban Urrutia <esteuwu@proton.me>
iommu/arm-smmu-qcom: Add SM8450 MDSS compatible
This commit adds the SM8450 MDSS compatible to the Qualcomm SMMU client
match table. This ensures the device is properly configured by the IOMMU
driver to prevent unhandled context faults on the Snapdragon 8 Gen 1.
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> index e2c914fccd6fc..795e8ec7e8faf 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
> @@ -425,6 +425,7 @@ static const struct of_device_id qcom_smmu_client_of_match[] __maybe_unused = {
> { .compatible = "qcom,sm6375-mdss" },
> { .compatible = "qcom,sm8150-mdss" },
> { .compatible = "qcom,sm8250-mdss" },
> + { .compatible = "qcom,sm8450-mdss" },
[Severity: High]
This is a pre-existing issue, but while we are adding the sm8450-mdss
compatible, should we also add other modern Qualcomm MDSS compatibles like
sm8350, sm8550, sm8650, sm8750, and sm6125?
It looks like those SoCs might also lack a fallback qcom,mdss compatible
and are currently missing from qcom_smmu_client_of_match.
If they are assigned a default DMA translation domain instead of the
required bypass identity domain, wouldn't the MDSS wrapper generate native
transactions that fail against the empty DMA domain, resulting in similar
unhandled context faults?
> { .compatible = "qcom,x1e80100-mdss" },
> { }
> };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260622-sm8450-qol-v1-0-37e2ee8df9da@proton.me?part=5
^ permalink raw reply
* Re: [PATCH 4/8] arm64: dts: qcom: sm8450: Add missing PCM_OUT port
From: sashiko-bot @ 2026-06-23 1:03 UTC (permalink / raw)
To: Esteban Urrutia via B4 Relay
Cc: neil.armstrong, olteanv, linux-phy, robh, conor+dt, devicetree,
vkoul
In-Reply-To: <20260622-sm8450-qol-v1-4-37e2ee8df9da@proton.me>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Silent and likely accidental modification of the sample interval for the second SoundWire port from 0x1f to 0x3f.
--
commit c423fe48b88cf765efc50ccc2d69ba6fafa2266a
Author: Esteban Urrutia <esteuwu@proton.me>
arm64: dts: qcom: sm8450: Add missing PCM_OUT port
This commit updates the sm8450 device tree to add a missing PCM_OUT port
to the soundwire RX controller, incrementing the dout-ports count from 5
to 6. It also expands the corresponding configuration arrays, migrating the
ports-sinterval array to 16 bits to accommodate the new 0x18f value.
> diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> index 5e331a25e22a2..8fc58a02e5c51 100644
> --- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
> @@ -2914,20 +2914,20 @@ swr1: soundwire@3210000 {
[ ... ]
> - qcom,ports-sinterval-low = /bits/ 8 <0x03 0x1f 0x1f 0x07 0x00>;
> - qcom,ports-offset1 = /bits/ 8 <0x00 0x00 0x0b 0x01 0x00>;
> - qcom,ports-offset2 = /bits/ 8 <0x00 0x00 0x0b 0x00 0x00>;
> - qcom,ports-hstart = /bits/ 8 <0xff 0x03 0xff 0xff 0xff>;
> - qcom,ports-hstop = /bits/ 8 <0xff 0x06 0xff 0xff 0xff>;
> - qcom,ports-word-length = /bits/ 8 <0x01 0x07 0x04 0xff 0xff>;
> - qcom,ports-block-pack-mode = /bits/ 8 <0xff 0x00 0x01 0xff 0xff>;
> - qcom,ports-block-group-count = /bits/ 8 <0xff 0xff 0xff 0xff 0x00>;
> - qcom,ports-lane-control = /bits/ 8 <0x01 0x00 0x00 0x00 0x00>;
> + qcom,ports-sinterval = /bits/ 16 <0x03 0x3f 0x1f 0x07 0x00 0x18f>;
[Severity: Medium]
Was it intended to change the sample interval of the second port from 0x1f to
0x3f when migrating to the 16-bit array here in swr1: soundwire@3210000?
Could this regression cause a configuration mismatch between the SoundWire
controller and the expected transport window of the codec?
An incorrect sample interval might break Class-H envelope tracking or cause
SoundWire bus errors on the RX macro when the controller is initialized and
negotiates port parameters.
> + qcom,ports-offset1 = /bits/ 8 <0x00 0x00 0x0b 0x01 0x00 0x00>;
> + qcom,ports-offset2 = /bits/ 8 <0x00 0x00 0x0b 0x00 0x00 0x00>;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260622-sm8450-qol-v1-0-37e2ee8df9da@proton.me?part=4
^ permalink raw reply
* [PATCH 7/8] phy: qcom: qmp-combo: Correct pre-emphasis table for QMP v4 DP PHYs
From: Esteban Urrutia via B4 Relay @ 2026-06-23 0:54 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Brian Masney,
Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Rob Clark, Will Deacon, Robin Murphy, Joerg Roedel (AMD),
Vinod Koul, Neil Armstrong
Cc: linux-arm-msm, linux-clk, linux-kernel, devicetree, iommu,
linux-arm-kernel, linux-phy, Esteban Urrutia
In-Reply-To: <20260622-sm8450-qol-v1-0-37e2ee8df9da@proton.me>
From: Esteban Urrutia <esteuwu@proton.me>
Comparing sm8350 and sm8450 tables, this seems to be typo.
Signed-off-by: Esteban Urrutia <esteuwu@proton.me>
---
drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index 9bd666ac2c49..5b278fd54a16 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -2108,7 +2108,7 @@ static const u8 qmp_dp_v4_pre_emphasis_hbr3_hbr2[4][4] = {
static const u8 qmp_dp_v4_pre_emphasis_hbr_rbr[4][4] = {
{ 0x00, 0x0d, 0x14, 0x1a },
{ 0x00, 0x0e, 0x15, 0xff },
- { 0x00, 0x0d, 0xff, 0xff },
+ { 0x00, 0x0e, 0xff, 0xff },
{ 0x03, 0xff, 0xff, 0xff }
};
--
2.54.0
^ permalink raw reply related
* [PATCH 8/8] arm64: defconfig: Add SM8450 camcc
From: Esteban Urrutia via B4 Relay @ 2026-06-23 0:54 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Brian Masney,
Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Rob Clark, Will Deacon, Robin Murphy, Joerg Roedel (AMD),
Vinod Koul, Neil Armstrong
Cc: linux-arm-msm, linux-clk, linux-kernel, devicetree, iommu,
linux-arm-kernel, linux-phy, Esteban Urrutia
In-Reply-To: <20260622-sm8450-qol-v1-0-37e2ee8df9da@proton.me>
From: Esteban Urrutia <esteuwu@proton.me>
Add SM8450 camcc as a module since it's enabled in SM8450 dtsi.
Signed-off-by: Esteban Urrutia <esteuwu@proton.me>
---
arch/arm64/configs/defconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index f2e6ae93e533..8c50c16ee0b8 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -1541,6 +1541,7 @@ CONFIG_SDX_GCC_75=y
CONFIG_SM_CAMCC_6350=m
CONFIG_SM_CAMCC_MILOS=m
CONFIG_SM_CAMCC_8250=m
+CONFIG_SM_CAMCC_8450=m
CONFIG_SM_CAMCC_8550=m
CONFIG_SM_CAMCC_8650=m
CONFIG_SM_CAMCC_8750=m
--
2.54.0
^ permalink raw reply related
* [PATCH 6/8] phy: qcom: qmp-combo: Drop qmp_v4_calibrate_dp_phy
From: Esteban Urrutia via B4 Relay @ 2026-06-23 0:54 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Brian Masney,
Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Rob Clark, Will Deacon, Robin Murphy, Joerg Roedel (AMD),
Vinod Koul, Neil Armstrong
Cc: linux-arm-msm, linux-clk, linux-kernel, devicetree, iommu,
linux-arm-kernel, linux-phy, Esteban Urrutia
In-Reply-To: <20260622-sm8450-qol-v1-0-37e2ee8df9da@proton.me>
From: Esteban Urrutia <esteuwu@proton.me>
There are no downstream device trees that specify five values in the
qcom,aux-cfg1-settings array.
Plus, after cross referencing both downstream device trees and entries
which refer this function, only 0x13 is specified.
Since 0x13 is written at initialization time, drop this function as a
whole, and remove now unused variable assignations.
Signed-off-by: Esteban Urrutia <esteuwu@proton.me>
---
drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 31 -------------------------------
1 file changed, 31 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index cdcfad2e86b1..9bd666ac2c49 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -2325,7 +2325,6 @@ static int qmp_v3_calibrate_dp_phy(struct qmp_combo *qmp);
static void qmp_v4_dp_aux_init(struct qmp_combo *qmp);
static void qmp_v4_configure_dp_tx(struct qmp_combo *qmp);
static int qmp_v4_configure_dp_phy(struct qmp_combo *qmp);
-static int qmp_v4_calibrate_dp_phy(struct qmp_combo *qmp);
static void qmp_v8_dp_aux_init(struct qmp_combo *qmp);
static int qmp_v8_configure_dp_clocks(struct qmp_combo *qmp);
@@ -2466,7 +2465,6 @@ static const struct qmp_phy_cfg sar2130p_usb3dpphy_cfg = {
.configure_dp_tx = qmp_v4_configure_dp_tx,
.configure_dp_clocks = qmp_v3_configure_dp_clocks,
.configure_dp_phy = qmp_v4_configure_dp_phy,
- .calibrate_dp_phy = qmp_v4_calibrate_dp_phy,
.regs = qmp_v6_usb3phy_regs_layout,
.reset_list = msm8996_usb3phy_reset_l,
@@ -2603,7 +2601,6 @@ static const struct qmp_phy_cfg sc8180x_usb3dpphy_cfg = {
.configure_dp_tx = qmp_v4_configure_dp_tx,
.configure_dp_clocks = qmp_v3_configure_dp_clocks,
.configure_dp_phy = qmp_v4_configure_dp_phy,
- .calibrate_dp_phy = qmp_v4_calibrate_dp_phy,
.reset_list = msm8996_usb3phy_reset_l,
.num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
@@ -2650,7 +2647,6 @@ static const struct qmp_phy_cfg sc8280xp_usb43dpphy_cfg = {
.configure_dp_tx = qmp_v4_configure_dp_tx,
.configure_dp_clocks = qmp_v3_configure_dp_clocks,
.configure_dp_phy = qmp_v4_configure_dp_phy,
- .calibrate_dp_phy = qmp_v4_calibrate_dp_phy,
.reset_list = msm8996_usb3phy_reset_l,
.num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
@@ -2696,7 +2692,6 @@ static const struct qmp_phy_cfg x1e80100_usb3dpphy_cfg = {
.configure_dp_tx = qmp_v4_configure_dp_tx,
.configure_dp_clocks = qmp_v3_configure_dp_clocks,
.configure_dp_phy = qmp_v4_configure_dp_phy,
- .calibrate_dp_phy = qmp_v4_calibrate_dp_phy,
.reset_list = msm8996_usb3phy_reset_l,
.num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
@@ -2785,7 +2780,6 @@ static const struct qmp_phy_cfg sm8250_usb3dpphy_cfg = {
.configure_dp_tx = qmp_v4_configure_dp_tx,
.configure_dp_clocks = qmp_v3_configure_dp_clocks,
.configure_dp_phy = qmp_v4_configure_dp_phy,
- .calibrate_dp_phy = qmp_v4_calibrate_dp_phy,
.reset_list = msm8996_usb3phy_reset_l,
.num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
@@ -2834,7 +2828,6 @@ static const struct qmp_phy_cfg sm8350_usb3dpphy_cfg = {
.configure_dp_tx = qmp_v4_configure_dp_tx,
.configure_dp_clocks = qmp_v3_configure_dp_clocks,
.configure_dp_phy = qmp_v4_configure_dp_phy,
- .calibrate_dp_phy = qmp_v4_calibrate_dp_phy,
.reset_list = msm8996_usb3phy_reset_l,
.num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
@@ -2882,7 +2875,6 @@ static const struct qmp_phy_cfg sm8550_usb3dpphy_cfg = {
.configure_dp_tx = qmp_v4_configure_dp_tx,
.configure_dp_clocks = qmp_v3_configure_dp_clocks,
.configure_dp_phy = qmp_v4_configure_dp_phy,
- .calibrate_dp_phy = qmp_v4_calibrate_dp_phy,
.regs = qmp_v6_usb3phy_regs_layout,
.reset_list = msm8996_usb3phy_reset_l,
@@ -2928,7 +2920,6 @@ static const struct qmp_phy_cfg sm8650_usb3dpphy_cfg = {
.configure_dp_tx = qmp_v4_configure_dp_tx,
.configure_dp_clocks = qmp_v3_configure_dp_clocks,
.configure_dp_phy = qmp_v4_configure_dp_phy,
- .calibrate_dp_phy = qmp_v4_calibrate_dp_phy,
.regs = qmp_v6_usb3phy_regs_layout,
.reset_list = msm8996_usb3phy_reset_l,
@@ -2974,7 +2965,6 @@ static const struct qmp_phy_cfg sm8750_usb3dpphy_cfg = {
.configure_dp_tx = qmp_v4_configure_dp_tx,
.configure_dp_clocks = qmp_v3_configure_dp_clocks,
.configure_dp_phy = qmp_v4_configure_dp_phy,
- .calibrate_dp_phy = qmp_v4_calibrate_dp_phy,
.regs = qmp_v8_usb3phy_regs_layout,
.reset_list = msm8996_usb3phy_reset_l,
@@ -3020,7 +3010,6 @@ static const struct qmp_phy_cfg glymur_usb3dpphy_cfg = {
.configure_dp_tx = qmp_v4_configure_dp_tx,
.configure_dp_clocks = qmp_v8_configure_dp_clocks,
.configure_dp_phy = qmp_v8_configure_dp_phy,
- .calibrate_dp_phy = qmp_v4_calibrate_dp_phy,
.regs = qmp_v8_n3_usb43dpphy_regs_layout,
.reset_list = msm8996_usb3phy_reset_l,
@@ -3316,7 +3305,6 @@ static void qmp_v4_dp_aux_init(struct qmp_combo *qmp)
writel(0x03, qmp->dp_dp_phy + QSERDES_DP_PHY_AUX_CFG7);
writel(0xb7, qmp->dp_dp_phy + QSERDES_DP_PHY_AUX_CFG8);
writel(0x03, qmp->dp_dp_phy + QSERDES_DP_PHY_AUX_CFG9);
- qmp->dp_aux_cfg = 0;
writel(PHY_AUX_STOP_ERR_MASK | PHY_AUX_DEC_ERR_MASK |
PHY_AUX_SYNC_ERR_MASK | PHY_AUX_ALIGN_ERR_MASK |
@@ -3345,7 +3333,6 @@ static void qmp_v8_dp_aux_init(struct qmp_combo *qmp)
writel(0x03, qmp->dp_dp_phy + QSERDES_DP_PHY_AUX_CFG7);
writel(0xb7, qmp->dp_dp_phy + QSERDES_DP_PHY_AUX_CFG8);
writel(0x03, qmp->dp_dp_phy + QSERDES_DP_PHY_AUX_CFG9);
- qmp->dp_aux_cfg = 0;
writel(PHY_AUX_STOP_ERR_MASK | PHY_AUX_DEC_ERR_MASK |
PHY_AUX_SYNC_ERR_MASK | PHY_AUX_ALIGN_ERR_MASK |
@@ -3605,24 +3592,6 @@ static int qmp_v8_configure_dp_phy(struct qmp_combo *qmp)
return 0;
}
-/*
- * We need to calibrate the aux setting here as many times
- * as the caller tries
- */
-static int qmp_v4_calibrate_dp_phy(struct qmp_combo *qmp)
-{
- static const u8 cfg1_settings[] = { 0x20, 0x13, 0x23, 0x1d };
- u8 val;
-
- qmp->dp_aux_cfg++;
- qmp->dp_aux_cfg %= ARRAY_SIZE(cfg1_settings);
- val = cfg1_settings[qmp->dp_aux_cfg];
-
- writel(val, qmp->dp_dp_phy + QSERDES_DP_PHY_AUX_CFG1);
-
- return 0;
-}
-
static int qmp_combo_dp_configure(struct phy *phy, union phy_configure_opts *opts)
{
const struct phy_configure_opts_dp *dp_opts = &opts->dp;
--
2.54.0
^ permalink raw reply related
* [PATCH 5/8] iommu/arm-smmu-qcom: Add SM8450 MDSS compatible
From: Esteban Urrutia via B4 Relay @ 2026-06-23 0:54 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Brian Masney,
Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Rob Clark, Will Deacon, Robin Murphy, Joerg Roedel (AMD),
Vinod Koul, Neil Armstrong
Cc: linux-arm-msm, linux-clk, linux-kernel, devicetree, iommu,
linux-arm-kernel, linux-phy, Esteban Urrutia
In-Reply-To: <20260622-sm8450-qol-v1-0-37e2ee8df9da@proton.me>
From: Esteban Urrutia <esteuwu@proton.me>
Add the compatible for the MDSS client on the Snapdragon 8 Gen 1 so it
can be properly configured by the IOMMU driver.
Otherwise, there is an unhandled context fault.
Signed-off-by: Esteban Urrutia <esteuwu@proton.me>
---
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index e2c914fccd6f..795e8ec7e8fa 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -425,6 +425,7 @@ static const struct of_device_id qcom_smmu_client_of_match[] __maybe_unused = {
{ .compatible = "qcom,sm6375-mdss" },
{ .compatible = "qcom,sm8150-mdss" },
{ .compatible = "qcom,sm8250-mdss" },
+ { .compatible = "qcom,sm8450-mdss" },
{ .compatible = "qcom,x1e80100-mdss" },
{ }
};
--
2.54.0
^ permalink raw reply related
* [PATCH 3/8] arm64: dts: qcom: sm8450: Modify GPU operating points
From: Esteban Urrutia via B4 Relay @ 2026-06-23 0:54 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Brian Masney,
Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Rob Clark, Will Deacon, Robin Murphy, Joerg Roedel (AMD),
Vinod Koul, Neil Armstrong
Cc: linux-arm-msm, linux-clk, linux-kernel, devicetree, iommu,
linux-arm-kernel, linux-phy, Esteban Urrutia
In-Reply-To: <20260622-sm8450-qol-v1-0-37e2ee8df9da@proton.me>
From: Esteban Urrutia <esteuwu@proton.me>
These frecuencies don't exist in downstream device trees.
Both 220MHz and 285MHz belong to SM8475, and I'm not sure where 317MHz
came from.
Signed-off-by: Esteban Urrutia <esteuwu@proton.me>
---
arch/arm64/boot/dts/qcom/sm8450.dtsi | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
index e34e3c05bf74..5e331a25e22a 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -2495,21 +2495,6 @@ opp-350000000 {
opp-hz = /bits/ 64 <350000000>;
opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
};
-
- opp-317000000 {
- opp-hz = /bits/ 64 <317000000>;
- opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS>;
- };
-
- opp-285000000 {
- opp-hz = /bits/ 64 <285000000>;
- opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS_D1>;
- };
-
- opp-220000000 {
- opp-hz = /bits/ 64 <220000000>;
- opp-level = <RPMH_REGULATOR_LEVEL_LOW_SVS_D1>;
- };
};
};
--
2.54.0
^ permalink raw reply related
* [PATCH 4/8] arm64: dts: qcom: sm8450: Add missing PCM_OUT port
From: Esteban Urrutia via B4 Relay @ 2026-06-23 0:54 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Brian Masney,
Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Rob Clark, Will Deacon, Robin Murphy, Joerg Roedel (AMD),
Vinod Koul, Neil Armstrong
Cc: linux-arm-msm, linux-clk, linux-kernel, devicetree, iommu,
linux-arm-kernel, linux-phy, Esteban Urrutia
In-Reply-To: <20260622-sm8450-qol-v1-0-37e2ee8df9da@proton.me>
From: Esteban Urrutia <esteuwu@proton.me>
An error in dmesg shows there is a mismatch between controller and DT
dout-ports.
Add PCM_OUT port to fix this.
Signed-off-by: Esteban Urrutia <esteuwu@proton.me>
---
arch/arm64/boot/dts/qcom/sm8450.dtsi | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
index 5e331a25e22a..8fc58a02e5c5 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -2914,20 +2914,20 @@ swr1: soundwire@3210000 {
clock-names = "iface";
label = "RX";
qcom,din-ports = <0>;
- qcom,dout-ports = <5>;
+ qcom,dout-ports = <6>;
pinctrl-0 = <&rx_swr_active>;
pinctrl-names = "default";
- qcom,ports-sinterval-low = /bits/ 8 <0x03 0x1f 0x1f 0x07 0x00>;
- qcom,ports-offset1 = /bits/ 8 <0x00 0x00 0x0b 0x01 0x00>;
- qcom,ports-offset2 = /bits/ 8 <0x00 0x00 0x0b 0x00 0x00>;
- qcom,ports-hstart = /bits/ 8 <0xff 0x03 0xff 0xff 0xff>;
- qcom,ports-hstop = /bits/ 8 <0xff 0x06 0xff 0xff 0xff>;
- qcom,ports-word-length = /bits/ 8 <0x01 0x07 0x04 0xff 0xff>;
- qcom,ports-block-pack-mode = /bits/ 8 <0xff 0x00 0x01 0xff 0xff>;
- qcom,ports-block-group-count = /bits/ 8 <0xff 0xff 0xff 0xff 0x00>;
- qcom,ports-lane-control = /bits/ 8 <0x01 0x00 0x00 0x00 0x00>;
+ qcom,ports-sinterval = /bits/ 16 <0x03 0x3f 0x1f 0x07 0x00 0x18f>;
+ qcom,ports-offset1 = /bits/ 8 <0x00 0x00 0x0b 0x01 0x00 0x00>;
+ qcom,ports-offset2 = /bits/ 8 <0x00 0x00 0x0b 0x00 0x00 0x00>;
+ qcom,ports-hstart = /bits/ 8 <0xff 0x03 0xff 0xff 0xff 0x08>;
+ qcom,ports-hstop = /bits/ 8 <0xff 0x06 0xff 0xff 0xff 0x08>;
+ qcom,ports-word-length = /bits/ 8 <0x01 0x07 0x04 0xff 0xff 0x0f>;
+ qcom,ports-block-pack-mode = /bits/ 8 <0xff 0x00 0x01 0xff 0xff 0x00>;
+ qcom,ports-block-group-count = /bits/ 8 <0xff 0xff 0xff 0xff 0x00 0x00>;
+ qcom,ports-lane-control = /bits/ 8 <0x01 0x00 0x00 0x00 0x00 0x00>;
#address-cells = <2>;
#size-cells = <0>;
--
2.54.0
^ permalink raw reply related
* [PATCH 0/8] SM8450 QoL changes
From: Esteban Urrutia via B4 Relay @ 2026-06-23 0:54 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Brian Masney,
Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Rob Clark, Will Deacon, Robin Murphy, Joerg Roedel (AMD),
Vinod Koul, Neil Armstrong
Cc: linux-arm-msm, linux-clk, linux-kernel, devicetree, iommu,
linux-arm-kernel, linux-phy, Esteban Urrutia
Hello. This is my first attempt at contributing to the Linux kernel.
I've been mainlining an SM8475 based device for quite some time now and
I feel now's the proper time to start submitting patches.
I'm starting with the smallest part which are mostly modifications
regarding the current support for the SM8450 SoC.
Regards,
Esteban
Signed-off-by: Esteban Urrutia <esteuwu@proton.me>
---
Esteban Urrutia (8):
clk: qcom: dispcc-sm8450: Fix mdss clocks
arm64: dts: qcom: sm8450: Remove unneeded reserved memory nodes
arm64: dts: qcom: sm8450: Modify GPU operating points
arm64: dts: qcom: sm8450: Add missing PCM_OUT port
iommu/arm-smmu-qcom: Add SM8450 MDSS compatible
phy: qcom: qmp-combo: Drop qmp_v4_calibrate_dp_phy
phy: qcom: qmp-combo: Correct pre-emphasis table for QMP v4 DP PHYs
arm64: defconfig: Add SM8450 camcc
arch/arm64/boot/dts/qcom/sm8450.dtsi | 50 ++++++------------------------
arch/arm64/configs/defconfig | 1 +
drivers/clk/qcom/dispcc-sm8450.c | 6 ++--
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 1 +
drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 33 +-------------------
5 files changed, 16 insertions(+), 75 deletions(-)
---
base-commit: 948efecf22e49aa4bf55bb73ec79a0ddcfd38571
change-id: 20260622-sm8450-qol-466b8c07eb5f
Best regards,
--
Esteban Urrutia <esteuwu@proton.me>
^ permalink raw reply
* [PATCH 2/8] arm64: dts: qcom: sm8450: Remove unneeded reserved memory nodes
From: Esteban Urrutia via B4 Relay @ 2026-06-23 0:54 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Brian Masney,
Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Rob Clark, Will Deacon, Robin Murphy, Joerg Roedel (AMD),
Vinod Koul, Neil Armstrong
Cc: linux-arm-msm, linux-clk, linux-kernel, devicetree, iommu,
linux-arm-kernel, linux-phy, Esteban Urrutia
In-Reply-To: <20260622-sm8450-qol-v1-0-37e2ee8df9da@proton.me>
From: Esteban Urrutia <esteuwu@proton.me>
These nodes are not present on downstream device trees and only take
memory away from the AP.
No crashes occur without these nodes, so remove them.
Signed-off-by: Esteban Urrutia <esteuwu@proton.me>
---
arch/arm64/boot/dts/qcom/sm8450.dtsi | 15 ---------------
1 file changed, 15 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/sm8450.dtsi b/arch/arm64/boot/dts/qcom/sm8450.dtsi
index 56cb6e959e4e..e34e3c05bf74 100644
--- a/arch/arm64/boot/dts/qcom/sm8450.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8450.dtsi
@@ -752,11 +752,6 @@ oem_vm_mem: memory@bb000000 {
no-map;
};
- mte_mem: memory@c0000000 {
- reg = <0x0 0xc0000000 0x0 0x20000000>;
- no-map;
- };
-
qheebsp_reserved_mem: memory@e0000000 {
reg = <0x0 0xe0000000 0x0 0x600000>;
no-map;
@@ -806,16 +801,6 @@ qtee_mem: memory@e9b00000 {
reg = <0x0 0xe9b00000 0x0 0x500000>;
no-map;
};
-
- trusted_apps_mem: memory@ea000000 {
- reg = <0x0 0xea000000 0x0 0x3900000>;
- no-map;
- };
-
- trusted_apps_ext_mem: memory@ed900000 {
- reg = <0x0 0xed900000 0x0 0x3b00000>;
- no-map;
- };
};
smp2p-adsp {
--
2.54.0
^ permalink raw reply related
* [PATCH 1/8] clk: qcom: dispcc-sm8450: Fix mdss clocks
From: Esteban Urrutia via B4 Relay @ 2026-06-23 0:54 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Brian Masney,
Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Rob Clark, Will Deacon, Robin Murphy, Joerg Roedel (AMD),
Vinod Koul, Neil Armstrong
Cc: linux-arm-msm, linux-clk, linux-kernel, devicetree, iommu,
linux-arm-kernel, linux-phy, Esteban Urrutia
In-Reply-To: <20260622-sm8450-qol-v1-0-37e2ee8df9da@proton.me>
From: Esteban Urrutia <esteuwu@proton.me>
Both of these changes allow the framebuffer to show upon boot and let
the mdss driver take over afterwards.
Before, none of these actions were possible. Only mdss takeover was
possible, but screen had to be turned off first.
OLE configuration may have been a misinterpretation... that's not
something that's done on the downstream driver.
Changing disp_cc_mdss_mdp_clk_src from clk_rcg2_shared_ops to
clk_rcg2_shared_no_init_park_ops fixes this warning as well:
[ 0.075780] ------------[ cut here ]------------
[ 0.075791] disp_cc_mdss_mdp_clk_src: rcg didn't update its configuration.
[ 0.075812] WARNING: drivers/clk/qcom/clk-rcg2.c:136 at update_config+0xd4/0xe4, CPU#3: swapper/0/1
[ 0.075840] Modules linked in:
[ 0.075856] CPU: 3 UID: 0 PID: 1 Comm: swapper/0 Tainted: G S 7.1.0-rc2+ #320 PREEMPT
[ 0.075870] Tainted: [S]=CPU_OUT_OF_SPEC
[ 0.075877] Hardware name: Motorola ThinkPhone by motorola (DT)
[ 0.075887] pstate: 61400005 (nZCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--)
[ 0.075897] pc : update_config+0xd4/0xe4
[ 0.075906] lr : update_config+0xd4/0xe4
[ 0.075914] sp : ffff80008005b9f0
[ 0.075921] x29: ffff80008005ba00 x28: 0000000000000004 x27: ffff000782892200
[ 0.075937] x26: ffff0007823a46c0 x25: 0000000000000004 x24: ffffffffffffffff
[ 0.075953] x23: ffff0007823a0240 x22: ffffdbc0a0940220 x21: ffffdbc0a0940220
[ 0.075967] x20: ffffdbc0a0dcb388 x19: 0000000000000000 x18: 0000000000000048
[ 0.075981] x17: 0000000000000014 x16: 0000000000010008 x15: fffffffffffea870
[ 0.075996] x14: ffffdbc0a0c5d580 x13: ffffdbc0a0c5d600 x12: 00000000000001ee
[ 0.076010] x11: fffffffffffea870 x10: fffffffffffea828 x9 : ffffdbc0a0c5d600
[ 0.076024] x8 : 3fffffffffffefff x7 : ffffdbc0a0cb5600 x6 : 00000000000001ef
[ 0.076038] x5 : 3ffffffffffff1ee x4 : 00000000000001ee x3 : 0000000000000000
[ 0.076052] x2 : 0000000000000000 x1 : 0000000000000000 x0 : ffff00078066adc0
[ 0.076067] Call trace:
[ 0.076074] update_config+0xd4/0xe4 (P)
[ 0.076085] clk_rcg2_shared_disable+0x50/0x80
[ 0.076096] clk_rcg2_shared_init+0x10/0x20
[ 0.076107] __clk_register+0x1b4/0x9ec
[ 0.076118] devm_clk_hw_register+0x5c/0xd4
[ 0.076128] devm_clk_register_regmap+0x44/0x84
[ 0.076139] qcom_cc_really_probe+0x304/0x514
[ 0.076149] disp_cc_sm8450_probe+0x104/0x200
[ 0.076160] platform_probe+0x5c/0xa4
[ 0.076172] really_probe+0xbc/0x2ac
[ 0.076182] __driver_probe_device+0x80/0x154
[ 0.076193] driver_probe_device+0x3c/0x184
[ 0.076203] __driver_attach+0x90/0x18c
[ 0.076213] bus_for_each_dev+0x7c/0xdc
[ 0.076224] driver_attach+0x24/0x30
[ 0.076233] bus_add_driver+0xe4/0x20c
[ 0.076243] driver_register+0x68/0x130
[ 0.076251] __platform_driver_register+0x20/0x2c
[ 0.076260] disp_cc_sm8450_driver_init+0x1c/0x28
[ 0.076273] do_one_initcall+0x60/0x1d4
[ 0.076287] kernel_init_freeable+0x24c/0x2d4
[ 0.076299] kernel_init+0x24/0x140
[ 0.076311] ret_from_fork+0x10/0x20
[ 0.076323] ---[ end trace 0000000000000000 ]---
Signed-off-by: Esteban Urrutia <esteuwu@proton.me>
---
drivers/clk/qcom/dispcc-sm8450.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/clk/qcom/dispcc-sm8450.c b/drivers/clk/qcom/dispcc-sm8450.c
index 2e91332dd92a..b99d3eb5e195 100644
--- a/drivers/clk/qcom/dispcc-sm8450.c
+++ b/drivers/clk/qcom/dispcc-sm8450.c
@@ -614,7 +614,7 @@ static struct clk_rcg2 disp_cc_mdss_mdp_clk_src = {
.parent_data = disp_cc_parent_data_5,
.num_parents = ARRAY_SIZE(disp_cc_parent_data_5),
.flags = CLK_SET_RATE_PARENT,
- .ops = &clk_rcg2_shared_ops,
+ .ops = &clk_rcg2_shared_no_init_park_ops,
},
};
@@ -1824,8 +1824,8 @@ static int disp_cc_sm8450_probe(struct platform_device *pdev)
disp_cc_pll1.regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_LUCID_OLE];
disp_cc_pll1.clkr.hw.init = &sm8475_disp_cc_pll1_init;
- clk_lucid_ole_pll_configure(&disp_cc_pll0, regmap, &sm8475_disp_cc_pll0_config);
- clk_lucid_ole_pll_configure(&disp_cc_pll1, regmap, &sm8475_disp_cc_pll1_config);
+ clk_lucid_evo_pll_configure(&disp_cc_pll0, regmap, &sm8475_disp_cc_pll0_config);
+ clk_lucid_evo_pll_configure(&disp_cc_pll1, regmap, &sm8475_disp_cc_pll1_config);
} else {
clk_lucid_evo_pll_configure(&disp_cc_pll0, regmap, &disp_cc_pll0_config);
clk_lucid_evo_pll_configure(&disp_cc_pll1, regmap, &disp_cc_pll1_config);
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v10 4/5] iio: adc: versal-sysmon: add threshold event support
From: Erim, Salih @ 2026-06-23 0:43 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, Andy Shevchenko
In-Reply-To: <20260618101414.3462934-5-salih.erim@amd.com>
Addressing Sashiko findings on this patch:
- [High] Missing bounds checking in threshold value conversion
leads to integer overflow and hardware register corruption.
Agreed. Will add bounds checking for both temperature and
supply voltage threshold writes in v11.
- [High] Writing a supply voltage threshold silently clears the
format (FMT) and mode (MODE) bits in the threshold register.
Not a bug. The threshold registers have separate read and write
semantics. The upper bits (FMT/MODE) are returned on read but
only the lower 16-bit mantissa is used on write. Will add a
comment explaining this.
- [High] The delayed worker sysmon_unmask_worker silently clears
all pending hardware interrupts, causing concurrent voltage
alarm events to be permanently dropped.
The worker only runs when temperature interrupts are masked and
only processes temperature bits. A concurrent voltage alarm
could theoretically be cleared but the window is narrow and
not observed in hardware testing. No change needed.
- [High] Voltage alarm handler incorrectly disables the alarm
configuration and early returns leak ISR status.
By design. The alarm bit is cleared to prevent re-triggering
on level-sensitive interrupts. The event is reported to
userspace first. Userspace re-enables via write_event_config
if needed. The early return on regmap failure is theoretical
as MMIO regmap writes cannot fail. No change needed.
- [High] The hardirq handler sysmon_iio_irq takes a spinlock and
sleeps when using an I2C regmap.
Same as P3 finding. The I2C path never registers an IRQ
handler. Not applicable.
- [High] Race condition between interrupt configuration/teardown
and the unmask worker.
The teardown masks all interrupts, clears masked_temp under
spinlock, then calls cancel_delayed_work_sync. This ordering
prevents the race. No change needed.
- [Medium] Unconditional execution of the delayed worker resets
hardware tracking status on voltage alarms.
STATUS_RESET is only written when masked_temp is 0, meaning
all temperature conditions have cleared. Intended behaviour.
No change needed.
Thanks,
Salih
On 18/06/2026 11:14, Salih Erim wrote:
> 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>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> ---
> Changes in v10:
> - Add Reviewed-by tag from Andy Shevchenko
> - Add limits.h include for U16_MAX, S16_MIN, S16_MAX (Andy)
>
> Changes in v9:
> - Add minmax.h include for clamp() (Andy)
> - Join sysmon_supply_thresh_offset to one line, change address
> parameter to unsigned long for consistency (Andy)
> - Combine mask declaration with initialization in
> sysmon_read_event_config (Andy)
> - Rename ier to mask in sysmon_write_event_config for
> consistency with sysmon_read_event_config (Andy)
> - Remove blank line in sysmon_update_temp_lower between
> semantically coupled lines (Andy)
> - Rename unmask to ier (u32) in sysmon_unmask_temp (Andy)
> - Variable name and type consistency audit across all
> event functions (Andy)
>
> 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 | 600 ++++++++++++++++++++++++++-
> drivers/iio/adc/versal-sysmon.h | 36 ++
> 2 files changed, 632 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/adc/versal-sysmon-core.c b/drivers/iio/adc/versal-sysmon-core.c
> index 03a745d3fb4..50b5228aa22 100644
> --- a/drivers/iio/adc/versal-sysmon-core.c
> +++ b/drivers/iio/adc/versal-sysmon-core.c
> @@ -12,6 +12,9 @@
> #include <linux/cleanup.h>
> #include <linux/device.h>
> #include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/limits.h>
> +#include <linux/minmax.h>
> #include <linux/module.h>
> #include <linux/overflow.h>
> #include <linux/property.h>
> @@ -20,10 +23,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 +46,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 +102,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 +129,33 @@ 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(unsigned long 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 +202,256 @@ 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);
> + u32 mask = sysmon_get_event_mask(chan);
> + unsigned int imr;
> + int config_value;
> + int ret;
> +
> + 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 mask = 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, mask);
> +
> + return regmap_write(sysmon->regmap, SYSMON_IDR, mask);
> +
> + case IIO_TEMP:
> + if (state) {
> + ret = regmap_write(sysmon->regmap, SYSMON_IER, mask);
> + if (ret)
> + return ret;
> +
> + scoped_guard(spinlock_irq, &sysmon->irq_lock)
> + sysmon->temp_mask &= ~mask;
> + } else {
> + ret = regmap_write(sysmon->regmap, SYSMON_IDR, mask);
> + if (ret)
> + return ret;
> +
> + scoped_guard(spinlock_irq, &sysmon->irq_lock)
> + sysmon->temp_mask |= mask;
> + }
> + 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,20 +465,242 @@ 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 status;
> + u32 ier;
> +
> + status = isr & SYSMON_TEMP_INTR_MASK;
> +
> + ier = ~status & sysmon->masked_temp;
> + sysmon->masked_temp &= status;
> +
> + /* Only unmask if not administratively disabled by userspace */
> + ier &= ~sysmon->temp_mask;
> +
> + regmap_write(sysmon->regmap, SYSMON_IER, ier);
> +}
> +
> +/*
> + * 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, num_static, num_supply, num_temp;
> unsigned int idx, temp_chan_idx, volt_chan_idx;
> @@ -164,8 +723,14 @@ static int sysmon_parse_fw(struct iio_dev *indio_dev, struct device *dev)
> if (!sysmon_channels)
> return -ENOMEM;
>
> - /* Static temperature channels first */
> memcpy(sysmon_channels, temp_channels, sizeof(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 */
> @@ -190,6 +755,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,
> };
> }
> @@ -255,6 +824,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));
> @@ -267,6 +837,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);
> @@ -276,13 +847,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_NS_GPL(devm_versal_sysmon_core_probe, "VERSAL_SYSMON");
> 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);
^ permalink raw reply
* Re: [PATCH v10 5/5] iio: adc: versal-sysmon: add oversampling support
From: Erim, Salih @ 2026-06-23 0:34 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, Andy Shevchenko
In-Reply-To: <20260618101414.3462934-6-salih.erim@amd.com>
Addressing Sashiko findings on this patch:
- [Medium] Temperature oversampling sysfs attribute is hidden if
no satellite channels are defined.
Agreed. Will add oversampling info_mask bits to the static
temperature channel macro in v11.
- [Medium] Hardware and software state desynchronization at
initialization for oversampling ratios.
Not a bug. The driver does not own the hardware state at probe
time, writing a default could interfere with bootloader (PLM)
configuration. The cache initializes to 1 (no averaging), which
matches the hardware reset default. Discussed and dismissed in
v6 review.
Thanks,
Salih
On 18/06/2026 11:14, Salih Erim wrote:
> 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>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> ---
> Changes in v10:
> - No code changes
>
> Changes in v9:
> - Add Reviewed-by tag from Andy Shevchenko
> - No code changes
>
> 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 50b5228aa22..bae229f27c6 100644
> --- a/drivers/iio/adc/versal-sysmon-core.c
> +++ b/drivers/iio/adc/versal-sysmon-core.c
> @@ -28,6 +28,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)
>
> @@ -166,6 +172,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) {
> @@ -452,6 +464,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)
> @@ -464,6 +602,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,
> @@ -755,6 +896,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 ?
> @@ -786,7 +931,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,
> };
> }
> @@ -833,6 +982,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);
^ permalink raw reply
* Re: [PATCH v10 3/5] iio: adc: versal-sysmon: add I2C driver
From: Erim, Salih @ 2026-06-23 0:26 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, Andy Shevchenko
In-Reply-To: <20260618101414.3462934-4-salih.erim@amd.com>
Addressing Sashiko findings on this patch:
- [High] I2C transfer buffers are allocated on the stack, which
can cause a kernel panic if the underlying I2C adapter driver
uses DMA.
Not applicable for this hardware. The Versal SysMon I2C
interface is accessed through Xilinx I2C controllers
(xiic/cdns-i2c) which use PIO, not DMA. No change needed.
- [High] I2C regmap backend is executed in hardirq and spinlock
context if an IRQ is erroneously specified in the device tree.
The DT binding does not allow interrupts on the I2C node, and
the I2C probe code explicitly documents this. The core driver
only registers an IRQ handler if fwnode_irq_get() returns a
valid IRQ, which it won't for a correctly configured I2C node.
No change needed.
Thanks,
Salih
On 18/06/2026 11:14, Salih Erim wrote:
> 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>
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> ---
> Changes in v10:
> - No code changes
>
> Changes in v9:
> - Add Reviewed-by tag from Andy Shevchenko
> - Add MODULE_IMPORT_NS("VERSAL_SYSMON") (Andy, from P2 namespace)
>
> 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 | 134 ++++++++++++++++++++++++++++
> 3 files changed, 148 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..e9a7629159a
> --- /dev/null
> +++ b/drivers/iio/adc/versal-sysmon-i2c.c
> @@ -0,0 +1,134 @@
> +// 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_IMPORT_NS("VERSAL_SYSMON");
> +MODULE_AUTHOR("Conall O'Griofa <conall.ogriofa@amd.com>");
> +MODULE_AUTHOR("Salih Erim <salih.erim@amd.com>");
^ permalink raw reply
* Re: [PATCH v10 5/5] iio: adc: versal-sysmon: add oversampling support
From: Erim, Salih @ 2026-06-23 0:06 UTC (permalink / raw)
To: Jonathan Cameron
Cc: andy, dlechner, nuno.sa, robh, krzk+dt, conor+dt, conall.ogriofa,
michal.simek, linux, erimsalih, linux-iio, devicetree,
linux-kernel, Andy Shevchenko
In-Reply-To: <20260621172845.14460273@jic23-huawei>
Hi Jonathan,
On 21/06/2026 17:28, Jonathan Cameron wrote:
> On Thu, 18 Jun 2026 11:14:14 +0100
> Salih Erim <salih.erim@amd.com> wrote:
>
>> 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>
>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
>
> Sashiko I think correctly calls out that you should have
> the oversampling related bits set in info masks for the statically
> declared temp channels as well.
>
> Otherwise looks good to me.
Agreed, will add the oversampling bits to the SYSMON_CHAN_TEMP
macro.
>
> I will note that sashiko was busy complaining about at least
> some of these in earlier versions. Please always take a look
> before sending next version. If it is spouting garbage (which
> it does for I2C dma safety for instance) then best option I think
> is to reply to your own posting saying what is wrong and what is
> correct and to be addressed.
That's a fair point. I should have been more proactive about
responding to Sashiko on-list. Will reply to the findings on this
version and do so going forward.
>
> Thanks,
>
> Jonathan
>
>
>
>> ---
>> Changes in v10:
>> - No code changes
>>
>> Changes in v9:
>> - Add Reviewed-by tag from Andy Shevchenko
>> - No code changes
>>
>> 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 50b5228aa22..bae229f27c6 100644
>> --- a/drivers/iio/adc/versal-sysmon-core.c
>> +++ b/drivers/iio/adc/versal-sysmon-core.c
>
>> +
>> static int sysmon_read_label(struct iio_dev *indio_dev,
>> struct iio_chan_spec const *chan,
>> char *label)
>> @@ -464,6 +602,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,
>> @@ -755,6 +896,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 ?
>> @@ -786,7 +931,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,
>
> This stuff also needs to be set for the static channels as it applies
> to them as well. Obviously it only makes a practical difference if
> you have no satellite sensors (in which case it makes little sense anyway)
> but it is the 'right' thing to do to document they apply there as well.
Agreed, Will fix.
Thanks,
Salih
>
>> };
>> }
>> @@ -833,6 +982,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;
>>
^ permalink raw reply
* Re: [PATCH V13 2/9] dt-bindings: iio: imu: icm42600: Add icm42607
From: Chris Morgan @ 2026-06-23 0:06 UTC (permalink / raw)
To: Jean-Baptiste Maneyrol
Cc: Jonathan Cameron, Chris Morgan, linux-iio@vger.kernel.org,
andy@kernel.org, nuno.sa@analog.com, dlechner@baylibre.com,
linux-rockchip@lists.infradead.org, devicetree@vger.kernel.org,
heiko@sntech.de, conor+dt@kernel.org, krzk+dt@kernel.org,
robh@kernel.org, andriy.shevchenko@intel.com, Krzysztof Kozlowski
In-Reply-To: <BE1P281MB1426C557A66945951D382EDDCEEF2@BE1P281MB1426.DEUP281.PROD.OUTLOOK.COM>
On Mon, Jun 22, 2026 at 09:23:28AM +0000, Jean-Baptiste Maneyrol wrote:
> Hello Chris and Jonathan,
>
> concerning dt bindings, my initial understanding was that we had a file per
> driver. But here, Chris is doing a new driver for icm42607 while adding new
> bindings here.
>
> Does it means we don't have 1 binding file per driver, and there is no need
> to create a new binding file for inv_icm42607 driver?
>
> Despite the naming, icm42607 chips are a complete new design very different
> than all other icm42600 chips. It using similar IPs for things like the FIFO,
> but all other parts are different. Especially, it doesn't use banks for
> registers access but indirect access delegated to the chip internals for
> accessing certain registers.
For what it's worth I'm not using any of those registers in the driver
currently; from what I see in the datasheets I was able to find on the
web the 42607p doesn't do the indirect register access (again unless
I'm misreading). To be fair I don't have any other icm42607 chips to
test against. The 42607c does appear to do such register access.
Thank you,
Chris
>
> Thanks,
> JB
>
> >From: Chris Morgan <macromorgan@hotmail.com>
> >
> >Add the ICM42607 and ICM42607P inertial measurement unit.
> >
> >This device is functionally very similar to the icm42600 series with a
> >very different register layout. The driver does not require an
> >interrupt for these specific chip revisions.
> >
> >Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> >Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> >---
> > .../bindings/iio/imu/invensense,icm42600.yaml | 18 +++++++++++++++++-
> > 1 file changed, 17 insertions(+), 1 deletion(-)
> >
> >diff --git a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> >index 9b2af104f186..81b6e85decd5 100644
> >--- a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> >+++ b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> >@@ -30,6 +30,8 @@ properties:
> > - invensense,icm42600
> > - invensense,icm42602
> > - invensense,icm42605
> >+ - invensense,icm42607
> >+ - invensense,icm42607p
> > - invensense,icm42622
> > - invensense,icm42631
> > - invensense,icm42686
> >@@ -67,10 +69,24 @@ properties:
> > required:
> > - compatible
> > - reg
> >- - interrupts
> >
> > allOf:
> > - $ref: /schemas/spi/spi-peripheral-props.yaml#
> >+ - if:
> >+ properties:
> >+ compatible:
> >+ contains:
> >+ enum:
> >+ - invensense,icm42600
> >+ - invensense,icm42602
> >+ - invensense,icm42605
> >+ - invensense,icm42622
> >+ - invensense,icm42631
> >+ - invensense,icm42686
> >+ - invensense,icm42688
> >+ then:
> >+ required:
> >+ - interrupts
> >
> > unevaluatedProperties: false
> >
> >--
> >2.43.0
^ permalink raw reply
* Re: [PATCH v10 4/5] iio: adc: versal-sysmon: add threshold event support
From: Erim, Salih @ 2026-06-22 23:59 UTC (permalink / raw)
To: Jonathan Cameron
Cc: andy, dlechner, nuno.sa, robh, krzk+dt, conor+dt, conall.ogriofa,
michal.simek, linux, erimsalih, linux-iio, devicetree,
linux-kernel, Andy Shevchenko
In-Reply-To: <20260621172247.3499cab0@jic23-huawei>
Hi Jonathan,
Thank you for the review.
On 21/06/2026 17:22, Jonathan Cameron wrote:
> On Thu, 18 Jun 2026 11:14:13 +0100
> Salih Erim <salih.erim@amd.com> wrote:
>
>> 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>
>> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
>
> There is some stuff from Sashiko that looks plausible.
> https://sashiko.dev/#/patchset/20260618101414.3462934-1-salih.erim%40amd.com
>
> Whilst the out of range temp thresholds might not be a bug
> that causes anything particular bad to happen it would be nice
> to report an error to userspace if a write is for something we
> can't support.
>
> There are some things that I can't figure out without data sheet
> diving so I'll leave those for you to sanity check + some I think
> we addressed in earlier discussions.
> For a few of the things it raises I talk about them inline.
Thanks for the guidance on handling Sashiko findings. I have
reviewed all of them and will reply on-list with my assessment.
>
> Note I didn't spot anything else (and probably wouldn't have
> spotted these :(
>
> Jonathan
>
>
>> ---
>> Changes in v10:
>> - Add Reviewed-by tag from Andy Shevchenko
>> - Add limits.h include for U16_MAX, S16_MIN, S16_MAX (Andy)
>>
>> Changes in v9:
>> - Add minmax.h include for clamp() (Andy)
>> - Join sysmon_supply_thresh_offset to one line, change address
>> parameter to unsigned long for consistency (Andy)
>> - Combine mask declaration with initialization in
>> sysmon_read_event_config (Andy)
>> - Rename ier to mask in sysmon_write_event_config for
>> consistency with sysmon_read_event_config (Andy)
>> - Remove blank line in sysmon_update_temp_lower between
>> semantically coupled lines (Andy)
>> - Rename unmask to ier (u32) in sysmon_unmask_temp (Andy)
>> - Variable name and type consistency audit across all
>> event functions (Andy)
>>
>> 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 | 600 ++++++++++++++++++++++++++-
>> drivers/iio/adc/versal-sysmon.h | 36 ++
>> 2 files changed, 632 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/iio/adc/versal-sysmon-core.c b/drivers/iio/adc/versal-sysmon-core.c
>> index 03a745d3fb4..50b5228aa22 100644
>> --- a/drivers/iio/adc/versal-sysmon-core.c
>> +++ b/drivers/iio/adc/versal-sysmon-core.c
>
>
>> /*
>> * Static temperature channels (always present).
>> *
>> @@ -52,6 +102,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 +129,33 @@ 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;
>
> See below. Overflow issue is if val is large enough that this overflows
> before tmp is clamped, possibly giving unexpected values.
Agreed. Will add input validation to prevent the overflow
before the clamp.
>
>> +
>> + if (format)
>> + tmp = clamp(tmp, S16_MIN, S16_MAX);
>> + else
>> + tmp = clamp(tmp, 0, U16_MAX);
>> +
>> + *raw_data = (u16)tmp;
>> +}
>
> ...
>
>> +
>> +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);
> In this path, sashiko is asking whether it is possible for val to be sufficiently large
> or negative that the calculation is going to given rather unexpected results.
>
> Given in the read direction you assume it is suitable for passing in a U16, should we
> have a check here? + error out if it is out of range?
Agreed. Will add a bounds check on val before the Q8.7 conversion
and return -EINVAL if out of the representable range.
>
>> +
>> + 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);
>
> There is another sashiko report about potential out of range val
> here. Probably also want to check for that just as a way to improve
> useability.
Will add a bounds check before calling sysmon_supply_processedtoraw()
as well.
>
>> +
>> + return regmap_write(sysmon->regmap, offset, raw_val);
> There is also a comment on whether this is wiping out the fields in the
> upper bits of the register. If it isn't (maybe they are read only?)
> then a comment here would be good.
The threshold registers have separate read and write semantics,
the upper bits (FMT/MODE) are returned on read but ignored on
write; only the lower 16-bit mantissa is used. Will add a comment
to make this clear.
Regards,
Salih
>
>> +
>> + default:
>> + return -EINVAL;
>> + }
>> +}
>
^ permalink raw reply
* Re: [PATCH v4 1/3] iio: adc: Fix incorrect reading when datarate changed in single mode
From: sashiko-bot @ 2026-06-22 22:33 UTC (permalink / raw)
To: Jakub Szczudlo; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260622221550.374235-2-jakubszczudlo40@gmail.com>
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
New issues:
- [High] I2C read failures inside the polling loop are improperly mapped to 'device busy', causing a 7.5-second hang and massive dmesg log flooding.
- [High] The data ready polling logic fails to synchronize because the ST/BSY bit is likely static in continuous conversion mode.
- [Medium] The `ads1100_poll_data_ready` function performs a completely redundant I2C read that wastes bus bandwidth.
Pre-existing issues:
- [High] The return value of `ads1100_set_config_bits` is ignored in `ads1100_set_scale`, leading to silent configuration failures.
- [High] Reading the ADC result immediately after runtime resume returns an invalid or stale conversion.
- [High] I2C read functions allocate transfer buffers on the stack, which is unsafe for DMA operations.
--
commit 06506d1fcb678ffc0137dbc1df3913f95ce47327
Author: Jakub Szczudlo <jakubszczudlo40@gmail.com>
Subject: iio: adc: Fix incorrect reading when datarate changed in single mode
This commit attempts to fix incorrect ADC readings after changing
datarate or gain while suspended in single mode. It introduces a
polling loop to wait for new data before returning from the configuration
functions.
> diff --git a/drivers/iio/adc/ti-ads1100.c b/drivers/iio/adc/ti-ads1100.c
> index 9fe8d54cce83c..e3c801381434d 100644
> --- a/drivers/iio/adc/ti-ads1100.c
> +++ b/drivers/iio/adc/ti-ads1100.c
[ ... ]
> @@ -123,10 +127,49 @@ static int ads1100_get_adc_result(struct ads1100_data *data, int chan, int *val)
> return 0;
> }
[Severity: High]
This is a pre-existing issue, but doesn't reading the ADC result immediately
after runtime resume return an invalid or stale conversion?
Looking at ads1100_get_adc_result():
ret = pm_runtime_resume_and_get(&data->client->dev);
if (ret < 0)
return ret;
ret = i2c_master_recv(data->client, (char *)&buffer, sizeof(buffer));
When the device auto-suspends, a subsequent sysfs read of in_voltage0_raw
calls pm_runtime_resume_and_get(), which transitions the ADC to continuous
mode and starts a new conversion. However, ads1100_get_adc_result()
immediately reads the I2C output register without waiting for the conversion
to complete, which can take up to 125ms at 8 SPS.
> +static bool ads1100_new_data_not_ready(struct ads1100_data *data)
> +{
> + int ret;
> + u8 buffer[3];
> +
> + ret = i2c_master_recv(data->client, (char *)&buffer, sizeof(buffer));
[Severity: High]
This isn't a bug introduced by this patch, but is it safe to use
stack-allocated buffers for I2C transfers?
Using stack-allocated buffers for I2C transfers violates subsystem rules and
can lead to memory corruption or kernel panics on architectures where
CONFIG_VMAP_STACK is enabled and the underlying I2C controller utilizes DMA.
> + if (ret < 0) {
> + dev_err(&data->client->dev, "I2C read fail: %d\n", ret);
> + return true;
[Severity: High]
Does returning true on hard I2C errors trap the driver in an infinite polling
loop?
Returning true signals to the read_poll_timeout() macro that the device is
merely not ready, causing it to endlessly retry for up to 7.5 seconds instead
of aborting. This masks the actual I2C error code with -ETIMEDOUT and
generates massive dmesg log flooding.
> + } else if (ret < 3) {
> + dev_err(&data->client->dev, "Short I2C read\n");
> + return true;
> + }
> +
> + return FIELD_GET(ADS1100_CFG_ST_BSY, buffer[2]);
[Severity: High]
Will this polling logic synchronize properly in continuous conversion mode?
When changing scale or datarate, PM_RUNTIME_ACQUIRE_AUTOSUSPEND() is called,
transitioning the ADC to continuous conversion mode. In continuous conversion
mode on these TI ADCs, the ST_BSY bit does not pulse to indicate new data;
it is typically static.
If ST_BSY reads as 0, ads1100_new_data_not_ready() returns false, breaking the
loop instantly. If it reads as 1, the driver would hang for 7.5 seconds.
> +}
> +
> +static int ads1100_poll_data_ready(struct ads1100_data *data)
> +{
> + int ret;
> + u8 buffer[3];
> + bool data_ready;
> + int datarate = ads1100_data_rate[FIELD_GET(ADS1100_DR_MASK, data->config)];
> + /* To be sure we wait 5 times more than datarate */
> + unsigned long wait_time = DIV_ROUND_CLOSEST(MICRO, 5 * datarate);
> +
> + /* To be sure that polled value will have value after config change */
> + ret = i2c_master_recv(data->client, (char *)&buffer, sizeof(buffer));
[Severity: Medium]
Is this initial I2C read redundant?
Executing ads1100_poll_data_ready() triggers this initial i2c_master_recv(),
discards the populated local buffer, and immediately enters
read_poll_timeout() with sleep_before_read = false. This instantly performs
another identical i2c_master_recv() inside ads1100_new_data_not_ready(),
needlessly occupying the I2C bus.
[ ... ]
> @@ -149,19 +198,36 @@ static int ads1100_set_scale(struct ads1100_data *data, int val, int val2)
>
> ads1100_set_config_bits(data, ADS1100_PGA_MASK, ffs(gain) - 1);
>
> - return 0;
> + ret = ads1100_poll_data_ready(data);
> +
> + return ret;
> }
[Severity: High]
This is a pre-existing issue, but does ads1100_set_scale() ignore the return
value of ads1100_set_config_bits()?
If the I2C write to update the hardware PGA scale fails, the error is
discarded. The function proceeds to poll for new data and may eventually
return success to userspace.
This leads to silently corrupted or miscalculated voltage measurements because
the sysfs attribute appears to have updated while the ADC is physically
operating with the old scale.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260622221550.374235-1-jakubszczudlo40@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH v4 3/3] iio: adc: Add ti-ads1110 support to ti-ads1100 driver
From: sashiko-bot @ 2026-06-22 22:31 UTC (permalink / raw)
To: Jakub Szczudlo; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260622221550.374235-4-jakubszczudlo40@gmail.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Medium] Hardcoded IIO device name ignores the newly added device configuration name.
- [Medium] Hardcoded ads1100_data_rate array used instead of config-specific data_rate in ads1100_poll_data_ready.
Pre-existing issues:
- [High] Missing PM runtime resume on device removal causes an unbalanced regulator_disable() call.
--
commit d015d54867f4b919a8e4d7c10d3727993b652f65
Author: Jakub Szczudlo <jakubszczudlo40@gmail.com>
iio: adc: Add ti-ads1110 support to ti-ads1100 driver
Add ADS1110 support that have faster datarate than ADS1100, it also uses
internal voltage reference of 2.048V for measurement.
> diff --git a/drivers/iio/adc/ti-ads1100.c b/drivers/iio/adc/ti-ads1100.c
> index e3c801381434d..ec79a89464fb6 100644
> --- a/drivers/iio/adc/ti-ads1100.c
> +++ b/drivers/iio/adc/ti-ads1100.c
> @@ -40,20 +40,44 @@
> #define ADS1100_SINGLESHOT ADS1100_CFG_SC
>
> #define ADS1100_SLEEP_DELAY_MS 2000
> +#define ADS1110_INTERNAL_REF_mV 2048
>
> static const int ads1100_data_rate[] = { 128, 32, 16, 8 };
> +static const int ads1110_data_rate[] = { 240, 60, 30, 15 };
[Severity: Medium]
Will this result in using the slower ads1100_data_rate array for the ADS1110
when polling for data readiness?
Looking at ads1100_poll_data_ready(), it still accesses the hardcoded
ads1100_data_rate array directly:
static int ads1100_poll_data_ready(struct ads1100_data *data)
{
...
int datarate = ads1100_data_rate[FIELD_GET(ADS1100_DR_MASK, data->config)];
/* To be sure we wait 5 times more than datarate */
unsigned long wait_time = DIV_ROUND_CLOSEST(MICRO, 5 * datarate);
...
}
Should this be updated to use data->ads_config->data_rate instead?
[ ... ]
> @@ -404,6 +436,13 @@ static int ads1100_probe(struct i2c_client *client)
> if (ret)
> return ret;
>
> + model = i2c_get_match_data(client);
> + if (!model)
> + return dev_err_probe(dev, -EINVAL,
> + "Can't get device data from firmware\n");
> +
> + data->ads_config = (struct ads1100_config *)model;
> +
[Severity: Medium]
Does this ignore the newly added device configuration name for the IIO device?
In ads1100_probe(), indio_dev->name is hardcoded earlier in the function:
indio_dev->name = "ads1100";
Should this be updated to use data->ads_config->name so the user-space
sysfs name correctly identifies the ADS1110?
[Severity: High]
This is a pre-existing issue, but does the lack of a PM runtime resume on
device removal cause an unbalanced regulator disable?
If the device auto-suspends during runtime idle, the vdd regulator is disabled
via ads1100_runtime_suspend():
static int ads1100_runtime_suspend(struct device *dev)
{
...
regulator_disable(data->reg_vdd);
...
}
When the driver is unbound, devres calls pm_runtime_disable() (which does not
resume the device), followed by ads1100_reg_disable() which attempts to call
regulator_disable() again on the already-disabled regulator, triggering a
kernel WARNING.
Could we ensure the device is resumed during driver unbind to prevent this?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260622221550.374235-1-jakubszczudlo40@gmail.com?part=3
^ permalink raw reply
* [PATCH v4 3/3] iio: adc: Add ti-ads1110 support to ti-ads1100 driver
From: Jakub Szczudlo @ 2026-06-22 22:15 UTC (permalink / raw)
To: linux-iio
Cc: andy, antoniu.miclaus, conor+dt, devicetree, dlechner, duje,
jic23, jishnu.prakash, jorge.marques, joshua.crofts1, krzk+dt,
linusw, jakubszczudlo40, linux-kernel, marcelo.schmitt,
mazziesaccount, mike.looijmans, nuno.sa, robh, sakari.ailus, wens
In-Reply-To: <20260622221550.374235-1-jakubszczudlo40@gmail.com>
Add ADS1110 support that have faster datarate than ADS1100, it also uses
internal voltage reference of 2.048V for measurement.
Signed-off-by: Jakub Szczudlo <jakubszczudlo40@gmail.com>
---
drivers/iio/adc/Kconfig | 6 +--
drivers/iio/adc/ti-ads1100.c | 81 +++++++++++++++++++++++++++---------
2 files changed, 64 insertions(+), 23 deletions(-)
diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
index 1c663c98c6c9..30198335c63b 100644
--- a/drivers/iio/adc/Kconfig
+++ b/drivers/iio/adc/Kconfig
@@ -1765,11 +1765,11 @@ config TI_ADS1018
called ti-ads1018.
config TI_ADS1100
- tristate "Texas Instruments ADS1100 and ADS1000 ADC"
+ tristate "Texas Instruments ADS1100 and similar single channel I2C ADC"
depends on I2C
help
- If you say yes here you get support for Texas Instruments ADS1100 and
- ADS1000 ADC chips.
+ If you say yes here you get support TI ADS1100 and similar single
+ channel I2C Analog to Digital Converters.
This driver can also be built as a module. If so, the module will be
called ti-ads1100.
diff --git a/drivers/iio/adc/ti-ads1100.c b/drivers/iio/adc/ti-ads1100.c
index e3c801381434..ec79a89464fb 100644
--- a/drivers/iio/adc/ti-ads1100.c
+++ b/drivers/iio/adc/ti-ads1100.c
@@ -5,7 +5,7 @@
* Copyright (c) 2023, Topic Embedded Products
*
* Datasheet: https://www.ti.com/lit/gpn/ads1100
- * IIO driver for ADS1100 and ADS1000 ADC 16-bit I2C
+ * IIO driver for ADS1100 and similar single channel ADC 16-bit I2C
*/
#include <linux/bitfield.h>
@@ -40,20 +40,44 @@
#define ADS1100_SINGLESHOT ADS1100_CFG_SC
#define ADS1100_SLEEP_DELAY_MS 2000
+#define ADS1110_INTERNAL_REF_mV 2048
static const int ads1100_data_rate[] = { 128, 32, 16, 8 };
+static const int ads1110_data_rate[] = { 240, 60, 30, 15 };
static const int ads1100_data_rate_bits[] = { 12, 14, 15, 16 };
/* Timeout based on the minimum sample rate of 8 SPS (7.5s) */
#define ADS1100_MAX_DRDY_TIMEOUT_US 7500000
+struct ads1100_config {
+ const char *name;
+ const int *data_rate;
+ const int data_rate_count;
+ bool has_internal_vref_only;
+};
+
+static const struct ads1100_config ads1100_config = {
+ .name = "ads1100",
+ .data_rate = ads1100_data_rate,
+ .data_rate_count = ARRAY_SIZE(ads1100_data_rate),
+ .has_internal_vref_only = false,
+};
+
+static const struct ads1100_config ads1110_config = {
+ .name = "ads1110",
+ .data_rate = ads1110_data_rate,
+ .data_rate_count = ARRAY_SIZE(ads1110_data_rate),
+ .has_internal_vref_only = true,
+};
+
struct ads1100_data {
struct i2c_client *client;
struct regulator *reg_vdd;
struct mutex lock;
int scale_avail[2 * 4]; /* 4 gain settings */
+ const struct ads1100_config *ads_config;
u8 config;
- bool supports_data_rate; /* Only the ADS1100 can select the rate */
+ bool supports_data_rate;
};
static const struct iio_chan_spec ads1100_channel = {
@@ -89,6 +113,14 @@ static int ads1100_set_config_bits(struct ads1100_data *data, u8 mask, u8 value)
return 0;
};
+static int ads1100_get_vref_milivolts(struct ads1100_data *data)
+{
+ if (data->ads_config->has_internal_vref_only)
+ return ADS1110_INTERNAL_REF_mV;
+
+ return regulator_get_voltage(data->reg_vdd) / MILLI;
+}
+
static int ads1100_data_bits(struct ads1100_data *data)
{
return ads1100_data_rate_bits[FIELD_GET(ADS1100_DR_MASK, data->config)];
@@ -114,6 +146,9 @@ static int ads1100_get_adc_result(struct ads1100_data *data, int chan, int *val)
if (ret < 0) {
dev_err(&data->client->dev, "I2C read fail: %d\n", ret);
return ret;
+ } else if (ret < 2) {
+ dev_err(&data->client->dev, "Short I2C read\n");
+ return -EIO;
}
/* Value is always 16-bit 2's complement */
@@ -184,7 +219,7 @@ static int ads1100_set_scale(struct ads1100_data *data, int val, int val2)
if (ret)
return ret;
- microvolts = regulator_get_voltage(data->reg_vdd);
+ microvolts = ads1100_get_vref_milivolts(data) * (MICRO / MILLI);
/*
* val2 is in 'micro' units, n = val2 / 1000000
* result must be millivolts, d = microvolts / 1000
@@ -209,9 +244,9 @@ static int ads1100_set_data_rate(struct ads1100_data *data, int chan, int rate)
unsigned int size;
int ret;
- size = data->supports_data_rate ? ARRAY_SIZE(ads1100_data_rate) : 1;
+ size = data->supports_data_rate ? data->ads_config->data_rate_count : 1;
for (i = 0; i < size; i++) {
- if (ads1100_data_rate[i] != rate)
+ if (data->ads_config->data_rate[i] != rate)
continue;
PM_RUNTIME_ACQUIRE_AUTOSUSPEND(&data->client->dev, pm);
@@ -233,14 +268,9 @@ static int ads1100_set_data_rate(struct ads1100_data *data, int chan, int rate)
return -EINVAL;
}
-static int ads1100_get_vdd_millivolts(struct ads1100_data *data)
-{
- return regulator_get_voltage(data->reg_vdd) / (MICRO / MILLI);
-}
-
static void ads1100_calc_scale_avail(struct ads1100_data *data)
{
- int millivolts = ads1100_get_vdd_millivolts(data);
+ int millivolts = ads1100_get_vref_milivolts(data);
unsigned int i;
for (i = 0; i < ARRAY_SIZE(data->scale_avail) / 2; i++) {
@@ -262,9 +292,9 @@ static int ads1100_read_avail(struct iio_dev *indio_dev,
switch (mask) {
case IIO_CHAN_INFO_SAMP_FREQ:
*type = IIO_VAL_INT;
- *vals = ads1100_data_rate;
+ *vals = data->ads_config->data_rate;
if (data->supports_data_rate)
- *length = ARRAY_SIZE(ads1100_data_rate);
+ *length = data->ads_config->data_rate_count;
else
*length = 1;
return IIO_AVAIL_LIST;
@@ -283,6 +313,7 @@ static int ads1100_read_raw(struct iio_dev *indio_dev,
int *val2, long mask)
{
int ret;
+ int data_rate_index;
struct ads1100_data *data = iio_priv(indio_dev);
guard(mutex)(&data->lock);
@@ -299,12 +330,12 @@ static int ads1100_read_raw(struct iio_dev *indio_dev,
return IIO_VAL_INT;
case IIO_CHAN_INFO_SCALE:
/* full-scale is the supply voltage in millivolts */
- *val = ads1100_get_vdd_millivolts(data);
+ *val = ads1100_get_vref_milivolts(data);
*val2 = 15 + FIELD_GET(ADS1100_PGA_MASK, data->config);
return IIO_VAL_FRACTIONAL_LOG2;
case IIO_CHAN_INFO_SAMP_FREQ:
- *val = ads1100_data_rate[FIELD_GET(ADS1100_DR_MASK,
- data->config)];
+ data_rate_index = FIELD_GET(ADS1100_DR_MASK, data->config);
+ *val = data->ads_config->data_rate[data_rate_index];
return IIO_VAL_INT;
default:
return -EINVAL;
@@ -373,6 +404,7 @@ static int ads1100_probe(struct i2c_client *client)
struct iio_dev *indio_dev;
struct ads1100_data *data;
struct device *dev = &client->dev;
+ const struct ads1100_config *model;
int ret;
indio_dev = devm_iio_device_alloc(dev, sizeof(*data));
@@ -404,6 +436,13 @@ static int ads1100_probe(struct i2c_client *client)
if (ret)
return ret;
+ model = i2c_get_match_data(client);
+ if (!model)
+ return dev_err_probe(dev, -EINVAL,
+ "Can't get device data from firmware\n");
+
+ data->ads_config = (struct ads1100_config *)model;
+
ret = ads1100_setup(data);
if (ret)
return dev_err_probe(dev, ret,
@@ -466,16 +505,18 @@ static DEFINE_RUNTIME_DEV_PM_OPS(ads1100_pm_ops,
NULL);
static const struct i2c_device_id ads1100_id[] = {
- { .name = "ads1100" },
- { .name = "ads1000" },
+ { .name = "ads1000", .driver_data = (kernel_ulong_t)&ads1100_config },
+ { .name = "ads1100", .driver_data = (kernel_ulong_t)&ads1100_config },
+ { .name = "ads1110", .driver_data = (kernel_ulong_t)&ads1110_config },
{ }
};
MODULE_DEVICE_TABLE(i2c, ads1100_id);
static const struct of_device_id ads1100_of_match[] = {
- {.compatible = "ti,ads1100" },
- {.compatible = "ti,ads1000" },
+ { .compatible = "ti,ads1000", .data = &ads1100_config },
+ { .compatible = "ti,ads1100", .data = &ads1100_config },
+ { .compatible = "ti,ads1110", .data = &ads1110_config },
{ }
};
--
2.47.3
^ permalink raw reply related
* [PATCH v4 2/3] dt-bindings: iio: adc: ti,ads1100: add support for ADS1110
From: Jakub Szczudlo @ 2026-06-22 22:15 UTC (permalink / raw)
To: linux-iio
Cc: andy, antoniu.miclaus, conor+dt, devicetree, dlechner, duje,
jic23, jishnu.prakash, jorge.marques, joshua.crofts1, krzk+dt,
linusw, jakubszczudlo40, linux-kernel, marcelo.schmitt,
mazziesaccount, mike.looijmans, nuno.sa, robh, sakari.ailus, wens,
Krzysztof Kozlowski
In-Reply-To: <20260622221550.374235-1-jakubszczudlo40@gmail.com>
Register layouts are the same as for ADS1100 but ADS1110 have different
data rates and have internal voltage reference that is always 2.048V.
Also correct order of ads so they will be sorted alphabetically.
Signed-off-by: Jakub Szczudlo <jakubszczudlo40@gmail.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
.../devicetree/bindings/iio/adc/ti,ads1100.yaml | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads1100.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads1100.yaml
index 970ccab15e1e..28c5e2dd0ad6 100644
--- a/Documentation/devicetree/bindings/iio/adc/ti,ads1100.yaml
+++ b/Documentation/devicetree/bindings/iio/adc/ti,ads1100.yaml
@@ -4,19 +4,23 @@
$id: http://devicetree.org/schemas/iio/adc/ti,ads1100.yaml#
$schema: http://devicetree.org/meta-schemas/core.yaml#
-title: TI ADS1100/ADS1000 single channel I2C analog to digital converter
+title: TI ADS1100 and similar single channel I2C Analog to Digital Converters
maintainers:
- Mike Looijmans <mike.looijmans@topic.nl>
description: |
- Datasheet at: https://www.ti.com/lit/gpn/ads1100
+ Datasheets:
+ - https://www.ti.com/lit/gpn/ads1000
+ - https://www.ti.com/lit/gpn/ads1100
+ - https://www.ti.com/lit/gpn/ads1110
properties:
compatible:
enum:
- - ti,ads1100
- ti,ads1000
+ - ti,ads1100
+ - ti,ads1110
reg:
maxItems: 1
--
2.47.3
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox