* Re: [PATCH 2/13] dt-bindings: sound: Add Qualcomm QAIF binding
From: Harendra Gautam @ 2026-06-23 12:17 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: Mark Brown, Liam Girdwood, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-sound, linux-arm-msm, devicetree,
linux-kernel
In-Reply-To: <69d2e4a7-eb2e-42f2-83e4-0d8ca4b62da8@kernel.org>
On Wed, Jun 17, 2026 at 1:29 AM Srinivas Kandagatla <srini@kernel.org> wrote:
>
>
>
> On 6/5/26 11:37 AM, Harendra Gautam wrote:
> > Add a Devicetree binding for the Qualcomm Audio Interface (QAIF) CPU DAI
> > controller used on the Shikra audio platform.
> >
> > QAIF moves PCM data between system memory and external serial audio
> > interfaces through the AIF path, and between memory and the internal Bolero
> > digital codec through the CIF path. The controller needs a binding so
> > platform Devicetree files can describe its MMIO region, DMA IOMMU stream,
> > clocks, interrupt, DAI cells and per-interface AIF configuration.
> >
> > Describe the single register region, one EE interrupt, the required GCC
> > LPASS and audio core clocks, the DMA IOMMU mapping, and 'aif-interface@N'
> > child nodes used for static PCM, TDM or MI2S configuration.
> >
> > Signed-off-by: Harendra Gautam <harendra.gautam@oss.qualcomm.com>
> > ---
> > .../devicetree/bindings/sound/qcom,qaif.yaml | 353 ++++++++++++++++++
> > 1 file changed, 353 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/sound/qcom,qaif.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/sound/qcom,qaif.yaml b/Documentation/devicetree/bindings/sound/qcom,qaif.yaml
>
> Pl run dt-bindings checks before posting.
> > new file mode 100644
> > index 000000000000..5b385e05a650
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/sound/qcom,qaif.yaml
> > @@ -0,0 +1,361 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/sound/qcom,qaif.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Qualcomm Audio Interface (QAIF) CPU DAI Controller
> > +
> > +maintainers:
> > + - Harendra Gautam <harendra.gautam@oss.qualcomm.com>
> > +
> > +description:
> > + |
> > + The Qualcomm Audio Interface (QAIF) is a fully configurable DMA-based
> > + audio subsystem controller. It serialises and deserialises PCM audio
> > + between system memory and external serial audio peripherals (PCM, TDM,
> > + I2S, MI2S) through the AIF path, and transfers parallel audio between
> > + memory and an internal WCD codec through the CIF path.
> > +
> > + AIF (Audio Interface): up to 13 multi-lane Unified Audio Interfaces,
> > + each supporting up to 8 independent data lanes. Each lane is individually
> > + configurable as TX (output/speaker) or RX (input/mic). All lanes of an
> > + interface share a single bit clock and frame sync. Supported modes are
> > + PCM (short/long sync), TDM, and MI2S (stereo/mono). Per-interface
> > + configuration includes sync source (master/slave), sync mode, sync delay,
> > + sync inversion, slot width (8/16/24/32-bit), sample width, active slot
> > + masks (up to 32 slots), bits-per-lane frame size, lane enable/direction
> > + masks, loopback, output-enable control, and full-cycle path support for
> > + long chip-to-chip connections.
> > +
> > + CIF (Codec Interface): up to 32 RDDMA (playback) and 32 WRDMA (capture)
> > + channels connecting to an internal codec over a parallel bus. Each channel
> > + supports active-channel enable mask (up to 16 channels), frame-sync
> > + selection, frame-sync delay, frame-sync output gating, dynamic clock
> > + gating, and 16-bit packing/unpacking.
> > +
> > + Note on RX/TX naming convention: in QAIF, RX refers to the capture path
> > + (audio received from the interface into memory) and TX refers to the
> > + playback path (audio transmitted from memory to the interface). This
> > + applies to both AIF lane directions and CIF slot/mask properties.
> > +
> > + DMA engine: RDDMA fetches audio from DDR/TCM/LPM into a shared SRAM
> > + latency buffer (SHRAM) and drains it to the interface. WRDMA collects
> > + data from the interface into SHRAM and writes it to memory. Each DMA
> > + owns a private SHRAM region defined by start address and length registers.
> > + Burst sizes of 1/2/4/8/16 beats (64-bit) are supported with up to 4
> > + outstanding transactions per DMA. Two QSB master ports (QXM0 for TCM,
> > + QXM1 for DDR/LPM) provide the memory interface.
> > +
> > + Resources are partitioned among up to 5 Execution Engines (EEs) via
> > + EE map registers. Each EE owns a set of DMAs, audio interfaces, and
> > + interface groups, and receives its own independent interrupt output.
> > + The interrupt hierarchy has a two-level structure: a summary register
> > + identifies the event class (DMA period, underflow/overflow, error
> > + response, audio interface underflow/overflow, group done, rate detector,
> > + VFR), and per-resource status registers identify the specific channel.
> > +
> > + Interface grouping (bonding) allows up to 6 groups of audio and codec
> > + interfaces to start synchronously and align their DMA period interrupts
> > + within half a frame duration using the RDDMA padding feature.
> > +
> > + Two rate detector blocks measure the frequency of incoming frame sync or
> > + word select signals and generate interrupts on rate change, undetected
> > + rate, or sync timeout.
> > +
> > + Block diagram::
> > +
> > + System Memory (DDR / LPM / TCM)
> > + +---------------------------------+
> > + | Circular Buffers (ping-pong) |
> > + +----------+----------+-----------+
> > + | ^
> > + 64-bit AXI 64-bit AXI
> > + | |
> > + +----------v----------+-----------+
> > + | QSB Master Ports |
> > + | +----------+ +----------+ |
> > + | | QXM0 | | QXM1 | |
> > + | +----+-----+ +-----+----+ |
> > + +-------|--------------|----------+
> > + | |
> > + +-------v--------------v----------+
> > + | Shared RAM (SHRAM) |
> > + | +------------+ +------------+ |
> > + | | QXM0 Read | | QXM0 Write | |
> > + | | SHRAM | | SHRAM | |
> > + | +------------+ +------------+ |
> > + | +------------+ +------------+ |
> > + | | QXM1 Read | | QXM1 Write | |
> > + | | SHRAM | | SHRAM | |
> > + | +------------+ +------------+ |
> > + +---+--------+--------+-------+---+
> > + | | | |
> > + +---v--+ +--v---+ +--v---+ +-v----+
> > + |RDDMA | |RDDMA | |WRDMA | |WRDMA |
> > + | AIF | | CIF | | AIF | | CIF |
> > + |[0..n]| |[0..n]| |[0..n]| |[0..n]|
> > + +--+---+ +--+---+ +--+---+ +-+----+
> > + | | ^ ^
> > + | TX | TX | RX | RX
> > + v v | |
> > + +--+--------------------+ +----+----------+
> > + | Unified Audio Intf | | Codec DMA |
> > + | (AIF 0..12) | | Interface |
> > + | | | (CIF) |
> > + | AUD_INTFa block: | | |
> > + | - Serializer (TX) | | RDDMA: DDR -> |
> > + | - De-serializer (RX) | | internal |
> > + | - Sync gen/detect | | codec |
> > + | - Up to 8 data lanes | | WRDMA: codec |
> > + | - PCM / TDM / MI2S | | -> DDR |
> > + | - Near Pad Logic | | Up to 16 ch |
> > + +--+--------------------+ +----+----------+
> > + | Lane 0..7 (TX/RX) | Parallel bus
> > + | Bit clk + Frame sync | + Frame sync
> > + v v
> > + +--+--------+ +------+------+
> > + | External | | Internal |
> > + | Serial | | Digital |
> > + | Peripherals| | Codec |
> > + | (PCM/TDM/ | | (Bolero/ |
> > + | MI2S) | | WCD) |
> > + +-----------+ +-------------+
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - qcom,shikra-qaif-cpu
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + iommus:
> > + maxItems: 1
> > +
> > + clocks:
> > + minItems: 15
> > + maxItems: 15
> > +
> > + clock-names:
> > + items:
> > + - const: lpass_config_clk
> > + - const: lpass_core_axim_clk
> > + - const: aud_dma_clk
> > + - const: aud_dma_mem_clk
> > + - const: bus_clk
> > + - const: aif_if0_ebit_clk
> > + - const: aif_if0_ibit_clk
> > + - const: aif_if1_ebit_clk
> > + - const: aif_if1_ibit_clk
> > + - const: aif_if2_ebit_clk
> > + - const: aif_if2_ibit_clk
> > + - const: aif_if3_ebit_clk
> > + - const: aif_if3_ibit_clk
> > + - const: ext_mclka_clk
> > + - const: ext_mclkb_clk
>
>
> Also do we really need to specify these 15 clocks even though I use only
> one aif interface on my board.
As an example added all supported clock in this binding, in board
specific DT required clocks can be used.
--Harendra
>
> should some of these clocks belong to each aif child node instead of
> global qaif-cpu?
some of these clocks are interface related like aif_if0_ibit, kept in
parent node by taking reference from lpass-cpu driver.
--Harendra
>
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + '#sound-dai-cells':
> > + const: 1
> > +
> > + '#address-cells':
> > + const: 1
> > +
> > + '#size-cells':
> > + const: 0
> > +
> > + status: true
> > +
> > +patternProperties:
> > + "^aif-interface@[0-9a-f]+$":
> > + type: object
> > + description:
> > + AIF interface configuration child node. The compatible string
> > + identifies the serial protocol the interface is wired for on the
> > + board. The unit address matches the hardware AIF interface index.
> > + properties:
> > + compatible:
> > + enum:
> > + - qcom,qaif-pcm-dai
> > + - qcom,qaif-tdm-dai
> > + - qcom,qaif-mi2s-dai
> > + reg:
> > + maxItems: 1
> > + description: |
> > + Hardware AIF interface index (AUD_INTFa block index). This value
> > + also serves as the ALSA DAI ID; it corresponds directly to the
> > + QAIF_MI2S_TDM_AIFn constants in <dt-bindings/sound/qcom,qaif.h>
> > + (e.g. reg = <2> selects QAIF_MI2S_TDM_AIF2).
> > + qcom,qaif-aif-sync-mode:
> > + $ref: /schemas/types.yaml#/definitions/uint32
>
> These should be enum instead of uint32, simillar comments apply to some
> of the properties that have only few supported values.
Okay
>
> > + description:
> > + Sync mode. Use QAIF_AIF_SYNC_MODE_SHORT (0) for short (pulse)
> > + sync or QAIF_AIF_SYNC_MODE_LONG (1) for long (level) sync.
> > + qcom,qaif-aif-sync-src:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description:
> > + Sync source. Use QAIF_AIF_SYNC_SRC_SLAVE (0) for slave mode
> > + or QAIF_AIF_SYNC_SRC_MASTER (1) for master mode.
> > + qcom,qaif-aif-invert-sync:
> > + type: boolean
> > + description: Invert the frame sync polarity.
> > + qcom,qaif-aif-sync-delay:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description: Number of bit-clock cycles to delay the data relative to sync.
>
> This looks redundant to qcom,qaif-aif-sync-mode, which already indicates
> the delay information?
SYNC_MODE defines the pulse width/duration (Short/Long/One-slot),
whereas SYNC_DELAY specifies the independent cycle offset for the MSB
data start relative to the sync edge.
>
> > + qcom,qaif-aif-slot-width-rx:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description:
> > + RX slot width in bits. This is a board-specific hardware constraint
> > + determined by the wiring of the serial audio interface.
> > + qcom,qaif-aif-slot-width-tx:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description:
> > + TX slot width in bits. This is a board-specific hardware constraint
> > + determined by the wiring of the serial audio interface.
> > + qcom,qaif-aif-slot-en-rx-mask:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description:
> > + Bitmask of active RX slots. Board-specific — determined by which
> > + TDM slots the codec is wired to on this board.
> > + qcom,qaif-aif-slot-en-tx-mask:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description:
> > + Bitmask of active TX slots. Board-specific — determined by which
> > + TDM slots the codec is wired to on this board.
> > + qcom,qaif-aif-loopback:
> > + type: boolean
> > + description: Enable loopback mode (presence enables loopback).
>
> What is this mode used for, testing ?
this is used to test/validate DMA and interface level loopback with QAIF IP.
>
> > + qcom,qaif-aif-ctrl-data-oe:
> > + type: boolean
> > + description: Enable output drive on the control/data line.
>
> will this be ever false?
Yes, as per HPG whenever there is point to point connection this has
to be false.
>
> > + qcom,qaif-aif-lane-config:
> > + $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > + description:
> > + Lane configuration matrix. Each row is a pair <enable direction>
> > + for one lane starting from lane 0, up to 8 lanes. Use
> > + QAIF_AIF_LANE_ENABLE (1) or QAIF_AIF_LANE_DISABLE (0) for enable.
> > + Use QAIF_AIF_LANE_DIR_TX (0) for TX (speaker) or QAIF_AIF_LANE_DIR_RX
> > + (1) for RX (mic). TX and RX lanes should each be grouped contiguously.
> what do you mean ? can you elobrate how can you enforce this?
intention to have matrix from DT, where each rows are Lanes and in
each row, col[0] = lane enable/disable and col[1]=spkr/mic
>
> > + maxItems: 8
> > + items:
> > + items:
> > + - description: Lane enable (0 = disabled, 1 = enabled)
> > + enum: [0, 1]
> > + - description: Lane direction (0 = TX/speaker, 1 = RX/mic)
> > + enum: [0, 1]
> > + qcom,qaif-aif-full-cycle-en:
> > + type: boolean
> > + description: Enable full-cycle sync (effective in sync master mode).
> > + qcom,qaif-aif-bits-per-lane:
> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description:
> > + Number of slots per lane. The frame length is computed as
>
> bits per lane?
Yes.
>
> > + slot-width multiplied by bits-per-lane.
> > + if:
> > + properties:
> > + compatible:
> > + const: qcom,qaif-mi2s-dai
> > + then:
> > + description:
> > + MI2S interface. Sync mode and slot-enable masks are fixed by the
> > + MI2S protocol and must not be set in DT. Mono/stereo mode is
> > + determined at runtime from the stream channel count.
> > + properties:
> > + qcom,qaif-aif-sync-mode: false
> > + qcom,qaif-aif-slot-en-rx-mask: false
> > + qcom,qaif-aif-slot-en-tx-mask: false
> > + else:
> > + description:
> > + PCM or TDM interface. Sync mode and slot-enable masks are
> > + board-specific and must be provided. Mono mode does not apply.
> > + required:
> > + - qcom,qaif-aif-sync-mode
> > + - qcom,qaif-aif-slot-en-rx-mask
> > + - qcom,qaif-aif-slot-en-tx-mask
> > +
> > + required:
> > + - compatible
> > + - reg
> > + additionalProperties: false
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - iommus
> do we
yes.
> > + - clocks
> > + - clock-names
> > + - interrupts
> > + - '#sound-dai-cells'
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + /* Shikra platform example */
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > + #include <dt-bindings/interrupt-controller/irq.h>
> > + #include <dt-bindings/sound/qcom,qaif.h>
> > + #include <dt-bindings/clock/qcom,shikra-audiocorecc.h>
> > + #include <dt-bindings/clock/qcom,gcc-shikra.h>
> > +
> > + qaif_cpu: audio@a000000 {
> > + compatible = "qcom,shikra-qaif-cpu";
> > + reg = <0x0 0x0a000000 0x0 0x20000>;
> > + iommus = <&apps_smmu 0x1c0 0x0>;
> > + clocks = <&gcc GCC_LPASS_CONFIG_CLK>,
> > + <&gcc GCC_LPASS_CORE_AXIM_CLK>,
> > + <&audiocorecc AUDIO_CORE_CC_AUD_DMA_CLK>,
> > + <&audiocorecc AUDIO_CORE_CC_AUD_DMA_MEM_CLK>,
> > + <&audiocorecc AUDIO_CORE_CC_BUS_CLK>,
> > + <&audiocorecc AUDIO_CORE_CC_AIF_IF0_EBIT_CLK>,
> > + <&audiocorecc AUDIO_CORE_CC_AIF_IF0_IBIT_CLK>,
> > + <&audiocorecc AUDIO_CORE_CC_AIF_IF1_EBIT_CLK>,
> > + <&audiocorecc AUDIO_CORE_CC_AIF_IF1_IBIT_CLK>,
> > + <&audiocorecc AUDIO_CORE_CC_AIF_IF2_EBIT_CLK>,
> > + <&audiocorecc AUDIO_CORE_CC_AIF_IF2_IBIT_CLK>,
> > + <&audiocorecc AUDIO_CORE_CC_AIF_IF3_EBIT_CLK>,
> > + <&audiocorecc AUDIO_CORE_CC_AIF_IF3_IBIT_CLK>,
> > + <&audiocorecc AUDIO_CORE_CC_EXT_MCLKA_OUT_CLK>,
> > + <&audiocorecc AUDIO_CORE_CC_EXT_MCLKB_OUT_CLK>;
> > + clock-names = "lpass_config_clk",
> > + "lpass_core_axim_clk",
> > + "aud_dma_clk",
> > + "aud_dma_mem_clk",
> > + "bus_clk",
> > + "aif_if0_ebit_clk",
> > + "aif_if0_ibit_clk",
> > + "aif_if1_ebit_clk",
> > + "aif_if1_ibit_clk",
> > + "aif_if2_ebit_clk",
> > + "aif_if2_ibit_clk",
> > + "aif_if3_ebit_clk",
> > + "aif_if3_ibit_clk",
> > + "ext_mclka_clk",
> > + "ext_mclkb_clk";
> > + #sound-dai-cells = <1>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + interrupts = <GIC_SPI 331 IRQ_TYPE_LEVEL_HIGH>;
> > + status = "okay";
> > +
> > + qaif_aif_if2: aif-interface@2 {
> > + compatible = "qcom,qaif-tdm-dai";
> > + reg = <QAIF_MI2S_TDM_AIF2>;
> > + qcom,qaif-aif-sync-mode = <QAIF_AIF_SYNC_MODE_LONG>;
> > + qcom,qaif-aif-sync-src = <QAIF_AIF_SYNC_SRC_MASTER>;
> > + qcom,qaif-aif-sync-delay = <1>;
> > + qcom,qaif-aif-slot-width-rx = <32>;
> > + qcom,qaif-aif-slot-width-tx = <32>;
> > + qcom,qaif-aif-slot-en-rx-mask = <0x3>;
> > + qcom,qaif-aif-slot-en-tx-mask = <0x3>;
> > + qcom,qaif-aif-ctrl-data-oe;
> > + /* Lane 0: RX (mic); Lane 1: TX (speaker) */
> > + qcom,qaif-aif-lane-config = <QAIF_AIF_LANE_ENABLE QAIF_AIF_LANE_DIR_RX>,
> > + <QAIF_AIF_LANE_ENABLE QAIF_AIF_LANE_DIR_TX>;
> > + /* frame length = slot-width (32) * bits-per-lane (2) = 64 bits */
> > + qcom,qaif-aif-bits-per-lane = <2>;
> > + };
> > + };
>
^ permalink raw reply
* Re: [PATCH v2 07/12] ARM: dts: microchip: sama7d65: Add ADC node
From: sashiko-bot @ 2026-06-23 12:15 UTC (permalink / raw)
To: Varshini Rajendran; +Cc: conor+dt, robh, devicetree
In-Reply-To: <20260623105944.128840-8-varshini.rajendran@microchip.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch introduces a new `vddout25` fixed regulator node in `sama7d65.dtsi`, but this addition is completely undocumented in the commit message, which exclusively mentions adding the ADC node.
--
commit ee5a93263289a5fcd9ba9faf0c09df7c63701b1d
Author: Varshini Rajendran <varshini.rajendran@microchip.com>
ARM: dts: microchip: sama7d65: Add ADC node
This commit adds the device tree node for the ADC controller in the
sama7d65 SoC.
> diff --git a/arch/arm/boot/dts/microchip/sama7d65.dtsi b/arch/arm/boot/dts/microchip/sama7d65.dtsi
> index 94d49e20dc794..ba775459a816e 100644
> --- a/arch/arm/boot/dts/microchip/sama7d65.dtsi
> +++ b/arch/arm/boot/dts/microchip/sama7d65.dtsi
[ ... ]
> @@ -95,6 +96,16 @@ slow_xtal: clock-slowxtal {
> };
> };
>
> + vddout25: fixed-regulator-vddout25 {
> + compatible = "regulator-fixed";
> +
> + regulator-name = "VDDOUT25";
[Severity: Medium]
Is this regulator intentionally included in this patch? The commit message
exclusively mentions adding the ADC node, but this adds a new root-level
vddout25 fixed regulator node. Should this be split into a separate commit
or documented in the commit message?
> + regulator-min-microvolt = <2500000>;
> + regulator-max-microvolt = <2500000>;
> + regulator-boot-on;
> + status = "disabled";
> + };
> +
> ns_sram: sram@100000 {
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260623105944.128840-1-varshini.rajendran@microchip.com?part=7
^ permalink raw reply
* Re: [PATCH] of_numa: fix return -EINVAL when numa-node-id is not found in last node
From: Sang-Heon Jeon @ 2026-06-23 12:08 UTC (permalink / raw)
To: Rob Herring
Cc: Saravana Kannan, Andrew Morton, Dan Williams, David Hildenbrand,
devicetree, Mike Rapoport (Microsoft)
In-Reply-To: <20260623014246.GA3897146-robh@kernel.org>
On Tue, Jun 23, 2026 at 10:42 AM Rob Herring <robh@kernel.org> wrote:
>
> On Sun, Jun 21, 2026 at 11:39:18PM +0900, Sang-Heon Jeon wrote:
> > When the numa-node-id property is not found in the last memory node,
> > of_property_read_u32() returns -EINVAL, which then becomes the return
> > value of of_numa_parse_memory_nodes(), even though earlier memory nodes
> > were parsed successfully.
> >
> > Commit 7e488677a54a ("of, numa: return -EINVAL when no numa-node-id is
> > found") meant -EINVAL to be returned only when the numa-node-id property
> > is not found at all, not when it is found in an earlier memory node but
> > not in the last.
> >
> > Check whether at least one memory node was parsed successfully, and return
> > 0 in that case, -EINVAL otherwise, so the return value of
> > of_property_read_u32() for the last memory node no longer corrupts the
> > overall return value.
>
> IDK, it's arguable that an incomplete DT isn't valid and something we
> need to support. Is missing numa-node-id valid or it's just better to
> have at least partially
> configured NUMA nodes?
You're right.
Since the first implementation 298535c00a2c ("of, numa: Add NUMA of
binding implementation.") skips memory nodes without `numa-node-id`,
I only try to fix the error case introduced by 7e488677a54a ("of,
numa: return -EINVAL when no numa-node-id is found").
IMHO, if changing the logic is acceptable, rejecting DT with an
incomplete NUMA configuration also seems reasonable.
We already call memblock_validate_numa_coverage() after
of_numa_parse_memory_nodes(),
so such an incomplete configuration would most likely fail from that
validation anyway.
So, would it be OK to reject a DT with an incomplete NUMA
configuration? If so, I'll make a patch in that direction instead.
> Rob
Best Regards,
Sang-Heon Jeon
^ permalink raw reply
* Re: [PATCH v4 5/5] clk: rockchip: rk3588: add GATE_GRF clocks for I2S MCLK output to IO
From: Heiko Stübner @ 2026-06-23 12:05 UTC (permalink / raw)
To: Daniele Briguglio, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Diederik de Haas
Cc: Nicolas Frattaroli, linux-clk, devicetree, linux-arm-kernel,
linux-rockchip, linux-kernel, Ricardo Pardini
In-Reply-To: <DJGDSS875DDO.22TYPVYK5X8KZ@cknow-tech.com>
Hi Diederick,
Am Dienstag, 23. Juni 2026, 13:10:49 Mitteleuropäische Sommerzeit schrieb Diederik de Haas:
> [Resending it against v4 which wasn't present in my INBOX, but was the
> version accepted and used in my kernel]
>
> On Tue Jun 23, 2026 at 1:08 PM CEST, Daniele Briguglio wrote:
> > The I2S MCLK outputs on RK3588 are gated by bits in the SYS_GRF
> > register SOC_CON6 (offset 0x318). These gates control whether the
> > internal CRU MCLK signals reach the external IO pins connected to
> > audio codecs.
> >
> > The kernel should explicitly manage these gates so that audio
> > functionality does not depend on bootloader register state. This is
> > analogous to what was done for RK3576 SAI MCLK outputs [1].
> >
> > Register the SYS_GRF as an auxiliary GRF with grf_type_sys using
> > rockchip_clk_add_grf(), and add GATE_GRF entries for all four I2S
> > MCLK output gates:
> >
> > - I2S0_8CH_MCLKOUT_TO_IO (bit 0)
> > - I2S1_8CH_MCLKOUT_TO_IO (bit 1)
> > - I2S2_2CH_MCLKOUT_TO_IO (bit 2)
> > - I2S3_2CH_MCLKOUT_TO_IO (bit 7)
> >
> > Board DTS files that need MCLK on an IO pin can reference these
> > clocks, e.g.:
> >
> > clocks = <&cru I2S0_8CH_MCLKOUT_TO_IO>;
> >
> > Tested on the Youyeetoo YY3588 (RK3588) with an ES8388 codec on I2S0.
>
> Doesn't this break audio on a lot of RK3588 based boards?
> I have a kernel with this patch set and since then analog audio on my NanoPC-T6
> LTS and my WIP NanoPC-T6 Plus stopped working.
> Until I did s/I2S0_8CH_MCLKOUT/I2S0_8CH_MCLKOUT_TO_IO/ in my dts[i] files.
>
> And I wouldn't be surprised if the same thing applies to other RK3588 based
> boards? The same dtb file with a 7.1 kernel, without this patch set, works.
can you check if adding CLK_IGNORE_UNUSED [0] changes the situation for you?
What I assume is happening is that when the clocks were not declared they were
just left running, while now the kernel turns of unused (but defined) clocks.
[0] example in
https://elixir.bootlin.com/linux/v7.1.1/source/drivers/clk/rockchip/clk-rk3588.c#L865
^ permalink raw reply
* Re: [PATCH 3/3] backlight: lp8864: Convert from LED to backlight class driver
From: Sverdlin, Alexander @ 2026-06-23 11:59 UTC (permalink / raw)
To: daniel@riscstar.com
Cc: dri-devel@lists.freedesktop.org, danielt@kernel.org,
linux-leds@vger.kernel.org, devicetree@vger.kernel.org,
linux-fbdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <ajpw1w150SXm8Gbi@aspen.lan>
Hi Daniel,
On Tue, 2026-06-23 at 12:41 +0100, Daniel Thompson wrote:
> > The mechanism we have regarding hot plugging currently is just I2C bridge,
> > which de-registers and registers the bridged bus. So no additional drivers
> > are required, as long as I2C devices are self-contained and not glued with
> > platform devices.
> >
> > So bottom line is, I'd prefer to just add the backlight interface to the
> > existing driver, no matter where it would live in the future.
>
> Is there any reasion that LP8864/LP8866 is unique in appearing on a
> hotplugged I2C bus? In other words if support for dynamism is added
> specifically to leds-lp8864.c rather than in led_bl.c then what will stop
> the same dynamic tricks from being adde to other LED drivers?
LP8864 is not special in this regard, not different from adp8860_bl.c,
adp8870_bl.c or lm3639_bl.c, which would work in my application.
So I just wanted to add currently missing backlight_device_register() to
LP8864, but there were no such dual-API drivers under drivers/leds/ that's
wht I started with moving LP8864 driver into drivers/video/backlight/.
led_bl.c however could only be hot-pluggable either if one actively
creates/destroys platform device somewhere in the code programmatically
or uses DT overlays.
Just adding backlight API to LP8864 would be much more idiomatic, I'd
say.
--
Alexander Sverdlin
Siemens AG
www.siemens.com
^ permalink raw reply
* Re: [PATCH v2 05/12] nvmem: microchip-otpc: add tag-based packet lookup
From: sashiko-bot @ 2026-06-23 11:58 UTC (permalink / raw)
To: Varshini Rajendran; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260623105944.128840-6-varshini.rajendran@microchip.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing serialization in hardware register accesses during NVMEM read leads to data corruption on concurrent access.
--
commit 9261dabb0a9ba8aa6fc617051559e1385e26b1cc
Author: Varshini Rajendran <varshini.rajendran@microchip.com>
nvmem: microchip-otpc: add tag-based packet lookup
Add support for accessing OTP packets by their 4-byte ASCII tag while
preserving backward compatibility with the existing ID-based lookup. The
driver resolves offsets matching valid legacy selectors through ID
lookup, falling back to tag lookup for other values.
> diff --git a/drivers/nvmem/microchip-otpc.c b/drivers/nvmem/microchip-otpc.c
> index df979e8549fdb..cbb4822a97c04 100644
> --- a/drivers/nvmem/microchip-otpc.c
> +++ b/drivers/nvmem/microchip-otpc.c
[ ... ]
> @@ -154,12 +232,11 @@ static int mchp_otpc_read(void *priv, unsigned int off, void *val,
> int ret, payload_size;
>
> /*
> - * We reach this point with off being multiple of stride = 4 to
> - * be able to cross the subsystem. Inside the driver we use continuous
> - * unsigned integer numbers for packet id, thus divide off by 4
> - * before passing it to mchp_otpc_id_to_packet().
> + * From this point the offset has to be translated into the actual
> + * packet. For this we traverse the table of contents stored in a list
> + * "packet" based on the access type - packet id or tag.
> */
> - packet = mchp_otpc_id_to_packet(otpc, off / 4);
> + packet = mchp_otpc_resolve_packet(otpc, off);
[Severity: High]
This is a pre-existing issue, but does mchp_otpc_read() need serialization to
prevent concurrent hardware register accesses?
Since the NVMEM core does not serialize reg_read callbacks, multiple threads
reading the NVMEM device simultaneously could interleave hardware register
manipulations.
If mchp_otpc_read() loops over mchp_otpc_prepare_read() concurrently:
Thread A:
writel_relaxed(tmp, otpc->base + MCHP_OTPC_MR); // Sets address
Thread B:
writel_relaxed(tmp, otpc->base + MCHP_OTPC_MR); // Overwrites address
Could Thread B overwrite the address Thread A just wrote before Thread A
sets MCHP_OTPC_CR_READ? Both threads would then poll the same status register
and consume from the same data registers, which could lead to corrupted
payloads or invalid hardware state.
> if (!packet)
> return -EINVAL;
> offset = packet->offset;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260623105944.128840-1-varshini.rajendran@microchip.com?part=5
^ permalink raw reply
* Re: [PATCH v9 2/2] i3c: master: Add driver for AMD AXI I3C master controller
From: sashiko-bot @ 2026-06-23 11:57 UTC (permalink / raw)
To: Shubham Patil; +Cc: linux-i3c, devicetree, Frank.Li, conor+dt, robh
In-Reply-To: <20260623114417.2578189-3-shubhamsanjay.patil@amd.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The driver incorrectly calculates `actual_len` for early-terminated read transfers, over-reporting received bytes and exposing padding data.
- [Medium] Mixing goto-based error handling with scope-based cleanup (__free) in the same function.
- [Medium] Zero-length I2C/I3C transfers with NULL buffers are incorrectly rejected with -EINVAL.
- [Medium] Reaching DAA limits (maximum devices or exhausted dynamic addresses) causes the driver to discard all successfully enumerated devices.
--
commit 31faf134bb0fbc7c1ffe6801a15207207118c870
Author: Manikanta Guntupalli <manikanta.guntupalli@amd.com>
i3c: master: Add driver for AMD AXI I3C master controller
Add an I3C master driver and maintainers fragment for the AMD I3C bus
controller.
> diff --git a/drivers/i3c/master/amd-i3c-master.c b/drivers/i3c/master/amd-i3c-master.c
> new file mode 100644
> index 0000000000000..34ab1028c3ce8
> --- /dev/null
> +++ b/drivers/i3c/master/amd-i3c-master.c
[ ... ]
> +static void xi3c_master_rd_from_rx_fifo(struct xi3c_master *master,
> + struct xi3c_cmd *cmd)
> +{
> + u16 rx_data_available;
> + u16 copy_len;
> + u16 len;
> +
> + rx_data_available = xi3c_rd_fifo_level(master);
> + len = rx_data_available * XI3C_WORD_LEN;
> +
> + if (!len)
> + return;
> +
> + copy_len = min_t(u16, len, cmd->rx_len);
> + xi3c_readl_fifo(master->membase + XI3C_RD_FIFO_OFFSET,
> + (u8 *)cmd->rx_buf, copy_len);
> +
> + cmd->rx_buf = (u8 *)cmd->rx_buf + copy_len;
> + cmd->rx_len -= copy_len;
[Severity: High]
Does this logic correctly account for early-terminated read transfers?
If an I3C target terminates a read early with a non-word-aligned byte count,
the hardware RX FIFO might indicate a word is available, but padding bytes
are included. This driver reads the full word and decrements rx_len. Later
in xi3c_master_i3c_xfers(), actual_len is calculated from this modified
rx_len, which could over-report the received bytes and expose padding data
to upper layers.
> +}
> +
> +static int xi3c_master_read(struct xi3c_master *master, struct xi3c_cmd *cmd)
> +{
> + unsigned long timeout;
> + u32 status_reg;
> + int ret;
> +
> + if (!cmd->rx_buf || cmd->rx_len > XI3C_MAXDATA_LENGTH)
> + return -EINVAL;
[Severity: Medium]
Will this fail on valid zero-length transfers? Zero-length reads, such as
standard SMBus Quick commands or i2cdetect scans, typically have rx_len = 0
and a NULL rx_buf. The check here would incorrectly reject them.
[ ... ]
> +static int xi3c_master_write(struct xi3c_master *master, struct xi3c_cmd *cmd)
> +{
> + unsigned long timeout;
> + u16 cmd_len;
> +
> + if (!cmd->tx_buf || cmd->tx_len > XI3C_MAXDATA_LENGTH)
> + return -EINVAL;
[Severity: Medium]
Similarly, does this incorrectly reject valid zero-length writes with a NULL
tx_buf?
[ ... ]
> +static int xi3c_master_do_daa(struct i3c_master_controller *m)
> +{
> + u8 pid_bufs[XI3C_MAX_DEVS][XI3C_DAA_SLAVEINFO_READ_BYTECOUNT] = {};
> + struct xi3c_master *master = to_xi3c_master(m);
> + struct xi3c_xfer *xfer __free(kfree) = NULL;
[Severity: Medium]
Is it safe to mix __free() scope-based cleanup with goto-based error handling
in this function?
There are explicit kernel guidelines against combining goto-based error
handling with __free() scope-based resource management in the same function,
as it can lead to fragile and confusing execution flow.
[ ... ]
> + while (true) {
> + struct xi3c_cmd *cmd = &xfer->cmds[0];
> + u8 daa_byte;
> +
> + if (master->daa.index >= XI3C_MAX_DEVS) {
> + ret = -ENOSPC;
> + goto err_daa;
> + }
> +
> + addr = i3c_master_get_free_addr(m, last_addr + 1);
> + if (addr < 0) {
> + ret = addr;
> + goto err_daa;
> + }
[Severity: Medium]
If the bus reaches the maximum device count or runs out of free dynamic
addresses, jumping to err_daa will abort the enumeration process without
registering any devices that were successfully enumerated prior to this.
Is it intended to discard the earlier successfully assigned devices instead
of keeping them?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260623114417.2578189-1-shubhamsanjay.patil@amd.com?part=2
^ permalink raw reply
* Re: [PATCH] arm64: dts: qcom: eliza: Fix disp_cc_mdss_mdp_clk_src RCG stall on Eliza EVK
From: Krzysztof Kozlowski @ 2026-06-23 11:56 UTC (permalink / raw)
To: Konrad Dybcio, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, devicetree,
linux-kernel
In-Reply-To: <3d24b1bc-baca-417a-8f60-a060cab5719a@oss.qualcomm.com>
On 23/06/2026 13:43, Konrad Dybcio wrote:
>
>> My warning probably can be fixed same way as:
>> https://lore.kernel.org/all/20260622-sm8450-qol-v1-1-37e2ee8df9da@proton.me/
>
> Quite possibly. IIRC Mike Tipton wasn't a huge fan of park-at-init
> to begin with.
>
I just checked and it helps here, so lets abandon this patch.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 2/2] dt-bindings: Drop incorrect usage of double '::'
From: Daniel Lezcano @ 2026-06-23 11:45 UTC (permalink / raw)
To: Krzysztof Kozlowski, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Peter Griffin, Alim Akhtar,
Michael Turquette, Stephen Boyd, Brian Masney, Sylwester Nawrocki,
Chanwoo Choi, Sam Protsenko, Rob Clark, Dmitry Baryshkov,
Abhinav Kumar, Jessica Zhang, Sean Paul, Marijn Suijten,
David Airlie, Simona Vetter, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, Inki Dae, Seung-Woo Kim, Kyungmin Park,
Andi Shyti, Georgi Djakov, Lee Jones, Pavel Machek, Hans Verkuil,
Mauro Carvalho Chehab, Ulf Hansson, Peter Rosin, Vinod Koul,
Neil Armstrong, Linus Walleij, Geert Uytterhoeven, Magnus Damm,
Sebastian Reichel, Javier Martinez Canillas, Liam Girdwood,
Mark Brown, Greg Kroah-Hartman, Jiri Slaby, Srinivas Kandagatla,
Bartlomiej Zolnierkiewicz, Rafael J. Wysocki, Daniel Lezcano,
Zhang Rui, Lukasz Luba, Jonathan Marek, Taniya Das, Robert Marko,
Christian Marangi, Stephan Gerhold, Adam Skladowski,
Sireesh Kodali, Barnabas Czeman, Imran Shaik,
Sricharan Ramabadhran, Anusha Rao, Luo Jie, Tomasz Figa,
Chanho Park, Sunyeal Hong, Shin Son, Krishna Manikandan,
Jacek Anaszewski, Jaehoon Chung, Marek Szyprowski, Alina Yu,
Andy Gross, Niklas Söderlund, Wesley Cheng, linux-arm-msm,
devicetree, linux-kernel, linux-arm-kernel, linux-samsung-soc,
linux-clk, dri-devel, freedreno, linux-i2c, linux-pm, linux-leds,
linux-media, linux-mmc, linux-phy, linux-gpio, linux-renesas-soc,
linux-serial, linux-sound, linux-usb
In-Reply-To: <20260622101606.485961-4-krzysztof.kozlowski@oss.qualcomm.com>
On 6/22/26 12:16, Krzysztof Kozlowski wrote:
> There is no use of double colon '::' in YAML. OTOH, the literal style
> block, e.g. using '|' treats all characters as content [1] therefore
> single use of ':' in descriptions is perfectly fine, whenever '|' is
> used.
>
> Cleanup existing code, so the confusing style won't be re-used in new
> contributions.
>
> Link: https://yaml.org/spec/1.2.2/#literal-style [1]
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
>
> ---
>
> Intention for this patch is to go via Rob's tree.
> ---
> .../devicetree/bindings/arm/qcom-soc.yaml | 4 ++--
> .../devicetree/bindings/arm/qcom.yaml | 4 ++--
> .../bindings/arm/samsung/samsung-soc.yaml | 4 ++--
> .../display/msm/dsi-controller-main.yaml | 20 +++++++++----------
> .../display/samsung/samsung,fimd.yaml | 4 ++--
> .../bindings/i2c/samsung,s3c2410-i2c.yaml | 2 +-
> .../interconnect/qcom,msm8998-bwmon.yaml | 2 +-
> .../interconnect/samsung,exynos-bus.yaml | 14 ++++++-------
> .../bindings/leds/qcom,pm8058-led.yaml | 4 ++--
> .../bindings/leds/skyworks,aat1290.yaml | 6 +++---
> .../bindings/media/cec/cec-gpio.yaml | 2 +-
> .../bindings/mmc/samsung,exynos-dw-mshc.yaml | 2 +-
> .../devicetree/bindings/mux/mux-consumer.yaml | 4 ++--
> .../bindings/phy/samsung,mipi-video-phy.yaml | 4 ++--
> .../bindings/phy/samsung,usb2-phy.yaml | 2 +-
> .../bindings/phy/samsung,usb3-drd-phy.yaml | 2 +-
> .../bindings/pinctrl/samsung,pinctrl.yaml | 2 +-
> .../bindings/power/renesas,rcar-sysc.yaml | 2 +-
> .../bindings/power/reset/restart-handler.yaml | 8 ++++----
> .../bindings/regulator/maxim,max77802.yaml | 4 ++--
> .../bindings/regulator/richtek,rtq2208.yaml | 2 +-
> .../bindings/serial/qcom,msm-uartdm.yaml | 2 +-
> .../devicetree/bindings/slimbus/slimbus.yaml | 4 ++--
> .../bindings/soc/qcom/qcom,apr-services.yaml | 2 +-
> .../bindings/soc/qcom/qcom,rpmh-rsc.yaml | 8 ++++----
> .../bindings/soc/qcom/qcom,wcnss.yaml | 2 +-
> .../bindings/soc/renesas/renesas-soc.yaml | 4 ++--
> .../bindings/sound/qcom,q6asm-dais.yaml | 2 +-
> .../thermal/samsung,exynos-thermal.yaml | 4 ++--
Acked-by: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com> # thermal
^ permalink raw reply
* [PATCH v9 2/2] i3c: master: Add driver for AMD AXI I3C master controller
From: Shubham Patil @ 2026-06-23 11:44 UTC (permalink / raw)
To: git, michal.simek, alexandre.belloni, Frank.Li, robh, krzk+dt,
conor+dt, pgaj, wsa+renesas, tommaso.merciai.xr, arnd,
quic_msavaliy, Shyam-sundar.S-k, sakari.ailus, billy_tsai, kees,
gustavoars, jarkko.nikula, jorge.marques, linux-i3c, devicetree,
linux-kernel, linux-arch, linux-hardening
Cc: radhey.shyam.pandey, srinivas.goud, shubhrajyoti.datta,
shubhamsanjay.patil, Manikanta Guntupalli
In-Reply-To: <20260623114417.2578189-1-shubhamsanjay.patil@amd.com>
From: Manikanta Guntupalli <manikanta.guntupalli@amd.com>
Add an I3C master driver and maintainers fragment for the AMD I3C bus
controller.
The driver currently supports the I3C bus operating in SDR mode,
with features including Dynamic Address Assignment, private data
transfers, and CCC transfers in both broadcast and direct modes. It
also supports operation in I2C mode.
The controller's data FIFOs are accessed big-endian; the driver performs
this conversion locally using ioread32be()/iowrite32be() with the
helpers, so it does not depend on any core FIFO-endianness helpers.
Signed-off-by: Manikanta Guntupalli <manikanta.guntupalli@amd.com>
Co-developed-by: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
Signed-off-by: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
Co-developed-by: Shubham Patil <shubhamsanjay.patil@amd.com>
Signed-off-by: Shubham Patil <shubhamsanjay.patil@amd.com>
---
Changes for V9:
Updated commit description to note that the driver performs big-endian
FIFO accesses locally (the v8 core-helper patches were dropped).
Dropped the big-endian MMIO infrastructure patches from the series
("asm-generic/io.h: Add big-endian MMIO accessors", "i3c: fix
big-endian FIFO transfers", and "i3c: master: Add endianness support
for i3c_readl_fifo()/i3c_writel_fifo()"). The driver now performs
big-endian FIFO accesses locally using ioread32be()/iowrite32be() with
get_unaligned()/put_unaligned(), so the series is self-contained and no
longer includes internals.h.
Replaced the async completion/transfer-queue machinery with a simple
synchronous transfer path under the existing mutex.
Reworked response handling: added enum i3c_error_code to struct
xi3c_cmd, named the response codes, return -ENODEV/-EIO as appropriate
and set err = I3C_ERROR_M2/M0 so the i3c core and callers can tell a
NACK apart from a bus error; propagate err to CCC commands and to each
priv xfer (including actual_len).
Switched from .priv_xfers to the new .i3c_xfers op; reject non-SDR
modes with -EOPNOTSUPP and report actual_len.
Reworked DAA: assign addresses incrementally, bound the device count
(-ENOSPC), detect end-of-enumeration via -ENODEV, zero-initialize the
PID buffers, and check i3c_master_add_i3c_dev_locked().
Avoid busy-spinning: sleep with usleep_range() in the FIFO drain/fill
loops.
Use FIELD_PREP() with named command-FIFO field masks instead of
open-coded shifts, and convert the register-accessor macros to inline
functions.
Split the overloaded timeout macro into XI3C_RESP_TIMEOUT_US and
XI3C_XFER_TIMEOUT_MS with documented units, and add
XI3C_POLL_INTERVAL_US.
xi3c_clk_cfg(): use NSEC_PER_SEC and named timing constants, guard
against unsigned underflow, and handle I3C_BUS_MODE_MIXED_SLOW.
Dropped ENTHDR from supports_ccc_cmd() (SDR-only), and dispatch CCCs
using the I3C_CCC_DIRECT bit.
Use const for TX buffers and drop the related casts; use parity8() for
the DAA parity bit.
Updated MODULE_DESCRIPTION and authors, the copyright year, renamed the
Kconfig symbol to AMD_AXI_I3C_MASTER, and fixed the MAINTAINERS entry
(title, mailing list, and the correct binding filename).
Changes for V8:
Used time_left instead of timeout.
Used __free(kfree) for xfer to simplify err path in multiple places.
Changes for V7:
Updated timeout macro name.
Updated xi3c_master_wr_to_tx_fifo() and xi3c_master_rd_from_rx_fifo()
to use i3c_writel_fifo() and i3c_readl_fifo().
Changes for V6:
Removed typecast for xi3c_getrevisionnumber(), xi3c_wrfifolevel(),
and xi3c_rdfifolevel().
Replaced dynamic allocation with a static variable for pid_bcr_dcr.
Fixed sparse warning in do_daa by typecasting the address parity value
to u8.
Fixed sparse warning in xi3c_master_bus_init by typecasting the pid value
to u64 in info.pid calculation.
Changes for V5:
Used GENMASK_ULL for PID mask as it's 64bit mask.
Changes for V4:
Updated timeout macros.
Removed type casting for xi3c_is_resp_available() macro.
Used ioread32() and iowrite32() instead of readl() and writel()
to keep consistency.
Read XI3C_RESET_OFFSET reg before udelay().
Removed xi3c_master_free_xfer() and directly used kfree().
Skipped checking return value of i3c_master_add_i3c_dev_locked().
Used devm_mutex_init() instead of mutex_init().
Changes for V3:
Resolved merge conflicts.
Changes for V2:
Updated commit description.
Added mixed mode support with clock configuration.
Converted smaller functions into inline functions.
Used FIELD_GET() in xi3c_get_response().
Updated xi3c_master_rd_from_rx_fifo() to use cmd->rx_buf.
Used parity8() for address parity calculation.
Added guards for locks.
Dropped num_targets and updated xi3c_master_do_daa().
Used __free(kfree) in xi3c_master_send_bdcast_ccc_cmd().
Dropped PM runtime support.
Updated xi3c_master_read() and xi3c_master_write() with
xi3c_is_resp_available() check.
Created separate functions: xi3c_master_init() and xi3c_master_reinit().
Used xi3c_master_init() in bus initialization and xi3c_master_reinit()
in error paths.
Added DAA structure to xi3c_master structure.
---
MAINTAINERS | 8 +
drivers/i3c/master/Kconfig | 15 +
drivers/i3c/master/Makefile | 1 +
drivers/i3c/master/amd-i3c-master.c | 1060 +++++++++++++++++++++++++++
4 files changed, 1084 insertions(+)
create mode 100644 drivers/i3c/master/amd-i3c-master.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 461a3eed6129..bfaa6999913c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1035,6 +1035,14 @@ L: linux-sound@vger.kernel.org
S: Supported
F: sound/soc/amd/
+AMD AXI I3C MASTER DRIVER
+M: Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
+M: Shubham Patil <shubhamsanjay.patil@amd.com>
+L: linux-i3c@lists.infradead.org
+S: Maintained
+F: Documentation/devicetree/bindings/i3c/xlnx,axi-i3c-1.0.yaml
+F: drivers/i3c/master/amd-i3c-master.c
+
AMD AXI W1 DRIVER
M: Kris Chaplin <kris.chaplin@amd.com>
R: Thomas Delev <thomas.delev@amd.com>
diff --git a/drivers/i3c/master/Kconfig b/drivers/i3c/master/Kconfig
index 2609f2b18e0a..da96d2aaa399 100644
--- a/drivers/i3c/master/Kconfig
+++ b/drivers/i3c/master/Kconfig
@@ -86,3 +86,18 @@ config RENESAS_I3C
This driver can also be built as a module. If so, the module will be
called renesas-i3c.
+
+config AMD_AXI_I3C_MASTER
+ tristate "AMD AXI I3C Master driver"
+ depends on HAS_IOMEM
+ help
+ Support for the AMD AXI I3C master controller, a soft IP used on
+ AMD (Xilinx) FPGAs and adaptive SoCs with ARM or MicroBlaze
+ processors.
+
+ The controller currently supports Standard Data Rate (SDR) mode.
+ Features include Dynamic Address Assignment, private transfers,
+ and CCC transfers in both broadcast and direct modes.
+
+ This driver can also be built as a module. If so, the module
+ will be called amd-i3c-master.
diff --git a/drivers/i3c/master/Makefile b/drivers/i3c/master/Makefile
index 816a227b6f7a..8d82196dcf83 100644
--- a/drivers/i3c/master/Makefile
+++ b/drivers/i3c/master/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_AST2600_I3C_MASTER) += ast2600-i3c-master.o
obj-$(CONFIG_SVC_I3C_MASTER) += svc-i3c-master.o
obj-$(CONFIG_MIPI_I3C_HCI) += mipi-i3c-hci/
obj-$(CONFIG_RENESAS_I3C) += renesas-i3c.o
+obj-$(CONFIG_AMD_AXI_I3C_MASTER) += amd-i3c-master.o
diff --git a/drivers/i3c/master/amd-i3c-master.c b/drivers/i3c/master/amd-i3c-master.c
new file mode 100644
index 000000000000..34ab1028c3ce
--- /dev/null
+++ b/drivers/i3c/master/amd-i3c-master.c
@@ -0,0 +1,1060 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * I3C master driver for the AMD I3C controller.
+ *
+ * Copyright (C) 2026, Advanced Micro Devices, Inc.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+#include <linux/cleanup.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/i3c/master.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <linux/time.h>
+#include <linux/unaligned.h>
+
+#define XI3C_VERSION_OFFSET 0x00 /* Version Register */
+#define XI3C_RESET_OFFSET 0x04 /* Soft Reset Register */
+#define XI3C_CR_OFFSET 0x08 /* Control Register */
+#define XI3C_ADDRESS_OFFSET 0x0C /* Target Address Register */
+#define XI3C_SR_OFFSET 0x10 /* Status Register */
+#define XI3C_CMD_FIFO_OFFSET 0x20 /* I3C Command FIFO Register */
+#define XI3C_WR_FIFO_OFFSET 0x24 /* I3C Write Data FIFO Register */
+#define XI3C_RD_FIFO_OFFSET 0x28 /* I3C Read Data FIFO Register */
+#define XI3C_RESP_STATUS_FIFO_OFFSET 0x2C /* I3C Response status FIFO Register */
+#define XI3C_FIFO_LVL_STATUS_OFFSET 0x30 /* CMD slots free | WR-FIFO free (words) */
+#define XI3C_FIFO_LVL_STATUS_1_OFFSET 0x34 /* RESP fill | RD-FIFO fill level (words) */
+#define XI3C_SCL_HIGH_TIME_OFFSET 0x38 /* I3C SCL HIGH Register */
+#define XI3C_SCL_LOW_TIME_OFFSET 0x3C /* I3C SCL LOW Register */
+#define XI3C_SDA_HOLD_TIME_OFFSET 0x40 /* I3C SDA HOLD Register */
+#define XI3C_TSU_START_OFFSET 0x48 /* I3C START SETUP Register */
+#define XI3C_THD_START_OFFSET 0x4C /* I3C START HOLD Register */
+#define XI3C_TSU_STOP_OFFSET 0x50 /* I3C STOP Setup Register */
+#define XI3C_OD_SCL_HIGH_TIME_OFFSET 0x54 /* I3C OD SCL HIGH Register */
+#define XI3C_OD_SCL_LOW_TIME_OFFSET 0x58 /* I3C OD SCL LOW Register */
+#define XI3C_PID0_OFFSET 0x6C /* LSB 4 bytes of the PID */
+#define XI3C_PID1_BCR_DCR 0x70 /* MSB 2 bytes of the PID, BCR and DCR */
+
+#define XI3C_CR_EN_MASK BIT(0) /* Core Enable */
+#define XI3C_CR_RESUME_MASK BIT(2) /* Core Resume */
+#define XI3C_SR_RESP_NOT_EMPTY_MASK BIT(4) /* Resp Fifo not empty status mask */
+#define XI3C_RD_FIFO_NOT_EMPTY_MASK BIT(15) /* Read Fifo not empty status mask */
+
+#define XI3C_BCR_MASK GENMASK(23, 16)
+#define XI3C_DCR_MASK GENMASK(31, 24)
+#define XI3C_PID_MASK GENMASK_ULL(63, 16)
+#define XI3C_TIMING_MASK GENMASK(17, 0)
+#define XI3C_REV_NUM_MASK GENMASK(15, 8)
+#define XI3C_PID1_MASK GENMASK(15, 0)
+#define XI3C_FIFO_LEVEL_MASK GENMASK(15, 0)
+#define XI3C_RESP_CODE_MASK GENMASK(8, 5)
+#define XI3C_RESP_CODE_SUCCESS 0 /* Transfer completed OK */
+#define XI3C_RESP_CODE_NO_TARGET 2 /* 7E NACK: no target on bus */
+#define XI3C_RESP_CODE_NACK 3 /* Target NACK / CE2 / DAA end */
+#define XI3C_ADDR_MASK GENMASK(6, 0)
+#define XI3C_FIFOS_RST_MASK GENMASK(4, 1)
+
+/* Command FIFO word layout (bit ranges encoded in the GENMASK/BIT args) */
+#define XI3C_CMD_TYPE GENMASK(3, 0) /* command type */
+#define XI3C_CMD_TERMINATE BIT(4) /* terminate (last cmd of xfer) */
+#define XI3C_CMD_ADDR GENMASK(15, 8) /* target address << 1 | RnW */
+#define XI3C_CMD_LEN GENMASK(27, 16) /* payload length in bytes */
+#define XI3C_CMD_TID GENMASK(31, 28) /* transfer ID */
+
+#define XI3C_OD_TLOW_NS 500000
+#define XI3C_OD_THIGH_NS 41000
+#define XI3C_I2C_TCASMIN_NS 600000
+#define XI3C_TCASMIN_NS 260000
+#define XI3C_MAXDATA_LENGTH 4095
+#define XI3C_MAX_DEVS 32
+#define XI3C_DAA_SLAVEINFO_READ_BYTECOUNT 8
+
+#define XI3C_THOLD_MIN_REV0 5 /* Min SDA hold cycles, rev 0 IP */
+#define XI3C_THOLD_MIN_REV1 6 /* Min SDA hold cycles, rev >= 1 IP */
+#define XI3C_CYCLE_ADJUST 2 /* SCL/SDA pre-bias for HW pipeline */
+#define XI3C_FIFO_RESET_DELAY_US 10 /* HW settling time after FIFO reset */
+#define XI3C_POLL_INTERVAL_US 10 /* readl_poll_timeout() sleep slice */
+
+#define XI3C_I2C_MODE 0
+#define XI3C_I2C_TID 0
+#define XI3C_SDR_MODE 1
+#define XI3C_SDR_TID 1
+
+#define XI3C_WORD_LEN 4
+
+/*
+ * XI3C_RESP_TIMEOUT_US is in microseconds because it is passed as the
+ * timeout_us argument of readl_poll_timeout(). XI3C_XFER_TIMEOUT_MS is in
+ * milliseconds because it feeds msecs_to_jiffies(). Keep the two units
+ * distinct in the names so callers cannot mix them up.
+ */
+#define XI3C_RESP_TIMEOUT_US 500000
+#define XI3C_XFER_TIMEOUT_MS 1000
+
+struct xi3c_cmd {
+ const void *tx_buf;
+ void *rx_buf;
+ u16 tx_len;
+ u16 rx_len;
+ u8 addr;
+ u8 type;
+ u8 tid;
+ bool rnw;
+ bool is_daa;
+ bool continued;
+ enum i3c_error_code err;
+};
+
+struct xi3c_xfer {
+ unsigned int ncmds;
+ struct xi3c_cmd cmds[] __counted_by(ncmds);
+};
+
+/**
+ * struct xi3c_master - I3C master controller state.
+ * @base: I3C master controller embedded by the framework.
+ * @dev: Pointer to the backing device structure.
+ * @membase: Memory base of the HW registers.
+ * @pclk: Input clock driving the controller.
+ * @lock: Serializes transfers and CCC submission.
+ * @daa: ENTDAA enumeration state.
+ * @daa.addrs: Dynamic addresses assigned in enumeration order.
+ * @daa.index: Number of responders enumerated so far.
+ */
+struct xi3c_master {
+ struct i3c_master_controller base;
+ struct device *dev;
+ void __iomem *membase;
+ struct clk *pclk;
+ struct mutex lock; /* serializes transfers and CCC submission */
+ struct {
+ u8 addrs[XI3C_MAX_DEVS];
+ u8 index;
+ } daa;
+};
+
+static inline struct xi3c_master *
+to_xi3c_master(struct i3c_master_controller *master)
+{
+ return container_of(master, struct xi3c_master, base);
+}
+
+static inline u8 xi3c_get_revision_number(struct xi3c_master *master)
+{
+ return FIELD_GET(XI3C_REV_NUM_MASK,
+ ioread32(master->membase + XI3C_VERSION_OFFSET));
+}
+
+static inline u16 xi3c_wr_fifo_level(struct xi3c_master *master)
+{
+ return ioread32(master->membase + XI3C_FIFO_LVL_STATUS_OFFSET) &
+ XI3C_FIFO_LEVEL_MASK;
+}
+
+static inline u16 xi3c_rd_fifo_level(struct xi3c_master *master)
+{
+ return ioread32(master->membase + XI3C_FIFO_LVL_STATUS_1_OFFSET) &
+ XI3C_FIFO_LEVEL_MASK;
+}
+
+static inline bool xi3c_is_resp_available(struct xi3c_master *master)
+{
+ return FIELD_GET(XI3C_SR_RESP_NOT_EMPTY_MASK,
+ ioread32(master->membase + XI3C_SR_OFFSET));
+}
+
+static int xi3c_get_response(struct xi3c_master *master, struct xi3c_cmd *cmd)
+{
+ u32 response_data;
+ u32 resp_reg;
+ u8 code;
+ int ret;
+
+ ret = readl_poll_timeout(master->membase + XI3C_SR_OFFSET,
+ resp_reg,
+ resp_reg & XI3C_SR_RESP_NOT_EMPTY_MASK,
+ XI3C_POLL_INTERVAL_US, XI3C_RESP_TIMEOUT_US);
+ if (ret) {
+ dev_err(master->dev, "XI3C response timeout\n");
+ return ret;
+ }
+
+ response_data = ioread32(master->membase + XI3C_RESP_STATUS_FIFO_OFFSET);
+ code = FIELD_GET(XI3C_RESP_CODE_MASK, response_data);
+
+ switch (code) {
+ case XI3C_RESP_CODE_SUCCESS:
+ cmd->err = I3C_ERROR_UNKNOWN;
+ return 0;
+ case XI3C_RESP_CODE_NO_TARGET:
+ case XI3C_RESP_CODE_NACK:
+ /*
+ * Target did not ACK. Record it as I3C_ERROR_M2 so callers
+ * (and the i3c core, which keys on err == I3C_ERROR_M2) can
+ * tell a NACK apart from other failures. A normal transfer
+ * surfaces this as -EIO per the i3c_xfer contract; the DAA
+ * path instead expects -ENODEV as its enumeration terminator.
+ */
+ cmd->err = I3C_ERROR_M2;
+ return cmd->is_daa ? -ENODEV : -EIO;
+ default:
+ cmd->err = I3C_ERROR_M0;
+ dev_err(master->dev, "XI3C transfer error, response code %u\n",
+ code);
+ return -EIO;
+ }
+}
+
+static inline void xi3c_writesl_be(void __iomem *addr, const void *buffer,
+ unsigned int count)
+{
+ const u32 *buf = buffer;
+
+ while (count--)
+ iowrite32be(get_unaligned(buf++), addr);
+}
+
+static inline void xi3c_readsl_be(const void __iomem *addr, void *buffer,
+ unsigned int count)
+{
+ u32 *buf = buffer;
+
+ while (count--)
+ put_unaligned(ioread32be(addr), buf++);
+}
+
+static inline void xi3c_writel_fifo(void __iomem *addr, const void *buf,
+ int nbytes)
+{
+ xi3c_writesl_be(addr, buf, nbytes / 4);
+ if (nbytes & 3) {
+ u32 tmp = 0;
+
+ memcpy(&tmp, (const u8 *)buf + (nbytes & ~3), nbytes & 3);
+ xi3c_writesl_be(addr, &tmp, 1);
+ }
+}
+
+static inline void xi3c_readl_fifo(const void __iomem *addr, void *buf,
+ int nbytes)
+{
+ xi3c_readsl_be(addr, buf, nbytes / 4);
+ if (nbytes & 3) {
+ u32 tmp;
+
+ xi3c_readsl_be(addr, &tmp, 1);
+ memcpy((u8 *)buf + (nbytes & ~3), &tmp, nbytes & 3);
+ }
+}
+
+static void xi3c_master_write_to_cmdfifo(struct xi3c_master *master,
+ struct xi3c_cmd *cmd, u16 len)
+{
+ u32 transfer_cmd;
+ u8 addr;
+
+ addr = ((cmd->addr & XI3C_ADDR_MASK) << 1) | (u8)cmd->rnw;
+
+ transfer_cmd = FIELD_PREP(XI3C_CMD_TYPE, cmd->type);
+ transfer_cmd |= FIELD_PREP(XI3C_CMD_TERMINATE, !cmd->continued);
+ transfer_cmd |= FIELD_PREP(XI3C_CMD_ADDR, addr);
+ transfer_cmd |= FIELD_PREP(XI3C_CMD_TID, cmd->tid);
+
+ /*
+ * For dynamic addressing, an additional 1-byte length must be added
+ * to the command FIFO to account for the address present in the TX FIFO
+ */
+ if (cmd->is_daa) {
+ xi3c_writel_fifo(master->membase + XI3C_WR_FIFO_OFFSET,
+ cmd->tx_buf, cmd->tx_len);
+
+ len++;
+ }
+
+ transfer_cmd |= FIELD_PREP(XI3C_CMD_LEN, len);
+ iowrite32(transfer_cmd, master->membase + XI3C_CMD_FIFO_OFFSET);
+}
+
+static inline void xi3c_master_enable(struct xi3c_master *master)
+{
+ iowrite32(ioread32(master->membase + XI3C_CR_OFFSET) | XI3C_CR_EN_MASK,
+ master->membase + XI3C_CR_OFFSET);
+}
+
+static inline void xi3c_master_disable(struct xi3c_master *master)
+{
+ iowrite32(ioread32(master->membase + XI3C_CR_OFFSET) & ~XI3C_CR_EN_MASK,
+ master->membase + XI3C_CR_OFFSET);
+}
+
+static inline void xi3c_master_resume(struct xi3c_master *master)
+{
+ iowrite32(ioread32(master->membase + XI3C_CR_OFFSET) |
+ XI3C_CR_RESUME_MASK, master->membase + XI3C_CR_OFFSET);
+}
+
+static void xi3c_master_reset_fifos(struct xi3c_master *master)
+{
+ u32 data;
+
+ /* Assert FIFO reset. */
+ data = ioread32(master->membase + XI3C_RESET_OFFSET);
+ data |= XI3C_FIFOS_RST_MASK;
+ iowrite32(data, master->membase + XI3C_RESET_OFFSET);
+ /* Read-back flushes the posted write before the settling delay below. */
+ ioread32(master->membase + XI3C_RESET_OFFSET);
+ udelay(XI3C_FIFO_RESET_DELAY_US);
+
+ /* De-assert FIFO reset, then wait for the FIFOs to come back up. */
+ data &= ~XI3C_FIFOS_RST_MASK;
+ iowrite32(data, master->membase + XI3C_RESET_OFFSET);
+ ioread32(master->membase + XI3C_RESET_OFFSET);
+ udelay(XI3C_FIFO_RESET_DELAY_US);
+}
+
+static inline void xi3c_master_init(struct xi3c_master *master)
+{
+ /* Reset fifos */
+ xi3c_master_reset_fifos(master);
+
+ /* Enable controller */
+ xi3c_master_enable(master);
+}
+
+static inline void xi3c_master_reinit(struct xi3c_master *master)
+{
+ /* Reset fifos */
+ xi3c_master_reset_fifos(master);
+
+ /* Resume controller */
+ xi3c_master_resume(master);
+}
+
+static struct xi3c_xfer *xi3c_master_alloc_xfer(unsigned int ncmds)
+{
+ struct xi3c_xfer *xfer;
+
+ xfer = kzalloc(struct_size(xfer, cmds, ncmds), GFP_KERNEL);
+ if (!xfer)
+ return NULL;
+
+ xfer->ncmds = ncmds;
+
+ return xfer;
+}
+
+static void xi3c_master_rd_from_rx_fifo(struct xi3c_master *master,
+ struct xi3c_cmd *cmd)
+{
+ u16 rx_data_available;
+ u16 copy_len;
+ u16 len;
+
+ rx_data_available = xi3c_rd_fifo_level(master);
+ len = rx_data_available * XI3C_WORD_LEN;
+
+ if (!len)
+ return;
+
+ copy_len = min_t(u16, len, cmd->rx_len);
+ xi3c_readl_fifo(master->membase + XI3C_RD_FIFO_OFFSET,
+ (u8 *)cmd->rx_buf, copy_len);
+
+ cmd->rx_buf = (u8 *)cmd->rx_buf + copy_len;
+ cmd->rx_len -= copy_len;
+}
+
+static int xi3c_master_read(struct xi3c_master *master, struct xi3c_cmd *cmd)
+{
+ unsigned long timeout;
+ u32 status_reg;
+ int ret;
+
+ if (!cmd->rx_buf || cmd->rx_len > XI3C_MAXDATA_LENGTH)
+ return -EINVAL;
+
+ /* Fill command fifo */
+ xi3c_master_write_to_cmdfifo(master, cmd, cmd->rx_len);
+
+ if (!cmd->rx_len)
+ return 0;
+
+ ret = readl_poll_timeout(master->membase + XI3C_SR_OFFSET,
+ status_reg,
+ status_reg & (XI3C_RD_FIFO_NOT_EMPTY_MASK |
+ XI3C_SR_RESP_NOT_EMPTY_MASK),
+ XI3C_POLL_INTERVAL_US, XI3C_RESP_TIMEOUT_US);
+ if (ret) {
+ dev_err(master->dev, "XI3C read timeout\n");
+ return ret;
+ }
+
+ if (!(status_reg & XI3C_RD_FIFO_NOT_EMPTY_MASK))
+ return 0;
+
+ timeout = jiffies + msecs_to_jiffies(XI3C_XFER_TIMEOUT_MS);
+
+ /* Read data from rx fifo */
+ while (cmd->rx_len > 0 && !xi3c_is_resp_available(master)) {
+ if (time_after(jiffies, timeout)) {
+ dev_err(master->dev, "XI3C read timeout\n");
+ return -EIO;
+ }
+ xi3c_master_rd_from_rx_fifo(master, cmd);
+ usleep_range(XI3C_POLL_INTERVAL_US, 2 * XI3C_POLL_INTERVAL_US);
+ }
+
+ /* Read remaining data */
+ xi3c_master_rd_from_rx_fifo(master, cmd);
+
+ return 0;
+}
+
+static void xi3c_master_wr_to_tx_fifo(struct xi3c_master *master,
+ struct xi3c_cmd *cmd)
+{
+ u16 wrfifo_space;
+ u16 len;
+
+ wrfifo_space = xi3c_wr_fifo_level(master);
+ if (cmd->tx_len > wrfifo_space * XI3C_WORD_LEN)
+ len = wrfifo_space * XI3C_WORD_LEN;
+ else
+ len = cmd->tx_len;
+
+ if (len) {
+ xi3c_writel_fifo(master->membase + XI3C_WR_FIFO_OFFSET, cmd->tx_buf,
+ len);
+
+ cmd->tx_buf = (const u8 *)cmd->tx_buf + len;
+ cmd->tx_len -= len;
+ }
+}
+
+static int xi3c_master_write(struct xi3c_master *master, struct xi3c_cmd *cmd)
+{
+ unsigned long timeout;
+ u16 cmd_len;
+
+ if (!cmd->tx_buf || cmd->tx_len > XI3C_MAXDATA_LENGTH)
+ return -EINVAL;
+
+ cmd_len = cmd->tx_len;
+
+ /* Fill Tx fifo */
+ xi3c_master_wr_to_tx_fifo(master, cmd);
+
+ /* Write to command fifo */
+ xi3c_master_write_to_cmdfifo(master, cmd, cmd_len);
+
+ timeout = jiffies + msecs_to_jiffies(XI3C_XFER_TIMEOUT_MS);
+ /* Fill if any remaining data to tx fifo */
+ while (cmd->tx_len > 0 && !xi3c_is_resp_available(master)) {
+ if (time_after(jiffies, timeout)) {
+ dev_err(master->dev, "XI3C write timeout\n");
+ return -EIO;
+ }
+
+ xi3c_master_wr_to_tx_fifo(master, cmd);
+ usleep_range(XI3C_POLL_INTERVAL_US, 2 * XI3C_POLL_INTERVAL_US);
+ }
+
+ return 0;
+}
+
+static int xi3c_master_xfer(struct xi3c_master *master, struct xi3c_cmd *cmd)
+{
+ int ret;
+
+ if (cmd->rnw)
+ ret = xi3c_master_read(master, cmd);
+ else
+ ret = xi3c_master_write(master, cmd);
+
+ if (ret)
+ goto err_xfer_out;
+
+ ret = xi3c_get_response(master, cmd);
+ if (ret)
+ goto err_xfer_out;
+
+ return 0;
+
+err_xfer_out:
+ xi3c_master_reinit(master);
+ return ret;
+}
+
+static int xi3c_master_common_xfer(struct xi3c_master *master,
+ struct xi3c_xfer *xfer)
+{
+ unsigned int i;
+ int ret;
+
+ guard(mutex)(&master->lock);
+
+ for (i = 0; i < xfer->ncmds; i++) {
+ ret = xi3c_master_xfer(master, &xfer->cmds[i]);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int xi3c_master_do_daa(struct i3c_master_controller *m)
+{
+ u8 pid_bufs[XI3C_MAX_DEVS][XI3C_DAA_SLAVEINFO_READ_BYTECOUNT] = {};
+ struct xi3c_master *master = to_xi3c_master(m);
+ struct xi3c_xfer *xfer __free(kfree) = NULL;
+ struct xi3c_cmd *daa_cmd;
+ int addr, ret, i;
+ u8 last_addr = 0;
+ u8 *pid_buf;
+ u8 ccc_id;
+
+ xfer = xi3c_master_alloc_xfer(1);
+ if (!xfer)
+ return -ENOMEM;
+
+ /* Fill ENTDAA CCC */
+ ccc_id = I3C_CCC_ENTDAA;
+ daa_cmd = &xfer->cmds[0];
+ daa_cmd->addr = I3C_BROADCAST_ADDR;
+ daa_cmd->rnw = false;
+ daa_cmd->tx_buf = &ccc_id;
+ daa_cmd->tx_len = 1;
+ daa_cmd->type = XI3C_SDR_MODE;
+ daa_cmd->tid = XI3C_SDR_TID;
+ daa_cmd->continued = true;
+
+ ret = xi3c_master_common_xfer(master, xfer);
+ /*
+ * A NACK on the ENTDAA broadcast (I3C_ERROR_M2) means no slaves are
+ * present to enter DAA. Treat as a successful no-op after letting
+ * err_daa reinitialize the controller.
+ */
+ if (ret && daa_cmd->err == I3C_ERROR_M2) {
+ ret = 0;
+ goto err_daa;
+ }
+ if (ret)
+ goto err_daa;
+
+ master->daa.index = 0;
+
+ while (true) {
+ struct xi3c_cmd *cmd = &xfer->cmds[0];
+ u8 daa_byte;
+
+ if (master->daa.index >= XI3C_MAX_DEVS) {
+ ret = -ENOSPC;
+ goto err_daa;
+ }
+
+ addr = i3c_master_get_free_addr(m, last_addr + 1);
+ if (addr < 0) {
+ ret = addr;
+ goto err_daa;
+ }
+
+ pid_buf = pid_bufs[master->daa.index];
+
+ daa_byte = (addr << 1) | (parity8(addr) ^ 1);
+
+ cmd->tx_buf = &daa_byte;
+ cmd->tx_len = 1;
+ cmd->addr = I3C_BROADCAST_ADDR;
+ cmd->rnw = true;
+ cmd->rx_buf = pid_buf;
+ cmd->rx_len = XI3C_DAA_SLAVEINFO_READ_BYTECOUNT;
+ cmd->is_daa = true;
+ cmd->type = XI3C_SDR_MODE;
+ cmd->tid = XI3C_SDR_TID;
+ cmd->continued = true;
+
+ ret = xi3c_master_common_xfer(master, xfer);
+
+ /*
+ * End of enumeration: the next responder NACK'd the
+ * dynamic-address grant, surfaced as -ENODEV.
+ * xi3c_master_xfer() has already reset the FIFOs and
+ * resumed the core for us; just exit the loop and
+ * register the responders collected so far.
+ */
+ if (ret == -ENODEV) {
+ ret = 0;
+ break;
+ }
+ if (ret)
+ goto err_daa;
+
+ master->daa.addrs[master->daa.index] = addr;
+ last_addr = addr;
+ master->daa.index++;
+ }
+
+ for (i = 0; i < master->daa.index; i++) {
+ u64 pid;
+
+ ret = i3c_master_add_i3c_dev_locked(m, master->daa.addrs[i]);
+ if (ret)
+ goto err_daa;
+
+ pid = FIELD_GET(XI3C_PID_MASK,
+ get_unaligned_be64(pid_bufs[i]));
+ dev_dbg(master->dev, "Client %d: PID: 0x%llx\n", i, pid);
+ }
+
+ return 0;
+
+err_daa:
+ xi3c_master_reinit(master);
+ return ret;
+}
+
+static bool
+xi3c_master_supports_ccc_cmd(struct i3c_master_controller *master,
+ const struct i3c_ccc_cmd *cmd)
+{
+ if (cmd->ndests > 1)
+ return false;
+
+ switch (cmd->id) {
+ case I3C_CCC_ENEC(true):
+ case I3C_CCC_ENEC(false):
+ case I3C_CCC_DISEC(true):
+ case I3C_CCC_DISEC(false):
+ case I3C_CCC_ENTAS(0, true):
+ case I3C_CCC_ENTAS(0, false):
+ case I3C_CCC_RSTDAA(true):
+ case I3C_CCC_RSTDAA(false):
+ case I3C_CCC_ENTDAA:
+ case I3C_CCC_SETMWL(true):
+ case I3C_CCC_SETMWL(false):
+ case I3C_CCC_SETMRL(true):
+ case I3C_CCC_SETMRL(false):
+ case I3C_CCC_SETDASA:
+ case I3C_CCC_SETNEWDA:
+ case I3C_CCC_GETMWL:
+ case I3C_CCC_GETMRL:
+ case I3C_CCC_GETPID:
+ case I3C_CCC_GETBCR:
+ case I3C_CCC_GETDCR:
+ case I3C_CCC_GETSTATUS:
+ case I3C_CCC_GETMXDS:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static int xi3c_master_send_bdcast_ccc_cmd(struct xi3c_master *master,
+ struct i3c_ccc_cmd *ccc)
+{
+ struct xi3c_xfer *xfer __free(kfree) = NULL;
+ u8 *buf __free(kfree) = NULL;
+ struct xi3c_cmd *cmd;
+ u16 xfer_len;
+ int ret;
+
+ if (ccc->dests[0].payload.len >= XI3C_MAXDATA_LENGTH)
+ return -EINVAL;
+
+ xfer_len = ccc->dests[0].payload.len + 1;
+
+ xfer = xi3c_master_alloc_xfer(1);
+ if (!xfer)
+ return -ENOMEM;
+
+ buf = kmalloc(xfer_len, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ buf[0] = ccc->id;
+ memcpy(&buf[1], ccc->dests[0].payload.data, ccc->dests[0].payload.len);
+
+ cmd = &xfer->cmds[0];
+ cmd->addr = ccc->dests[0].addr;
+ cmd->rnw = ccc->rnw;
+ cmd->tx_buf = buf;
+ cmd->tx_len = xfer_len;
+ cmd->type = XI3C_SDR_MODE;
+ cmd->tid = XI3C_SDR_TID;
+ cmd->continued = false;
+
+ ret = xi3c_master_common_xfer(master, xfer);
+ ccc->err = cmd->err;
+
+ return ret;
+}
+
+static int xi3c_master_send_direct_ccc_cmd(struct xi3c_master *master,
+ struct i3c_ccc_cmd *ccc)
+{
+ struct xi3c_xfer *xfer __free(kfree) = NULL;
+ struct xi3c_cmd *cmd;
+ int ret;
+
+ if (ccc->dests[0].payload.len > XI3C_MAXDATA_LENGTH)
+ return -EINVAL;
+
+ xfer = xi3c_master_alloc_xfer(2);
+ if (!xfer)
+ return -ENOMEM;
+
+ /* Broadcasted message */
+ cmd = &xfer->cmds[0];
+ cmd->addr = I3C_BROADCAST_ADDR;
+ cmd->rnw = false;
+ cmd->tx_buf = &ccc->id;
+ cmd->tx_len = 1;
+ cmd->type = XI3C_SDR_MODE;
+ cmd->tid = XI3C_SDR_TID;
+ cmd->continued = true;
+
+ /* Directed message */
+ cmd = &xfer->cmds[1];
+ cmd->addr = ccc->dests[0].addr;
+ cmd->rnw = ccc->rnw;
+ if (cmd->rnw) {
+ cmd->rx_buf = ccc->dests[0].payload.data;
+ cmd->rx_len = ccc->dests[0].payload.len;
+ } else {
+ cmd->tx_buf = ccc->dests[0].payload.data;
+ cmd->tx_len = ccc->dests[0].payload.len;
+ }
+ cmd->type = XI3C_SDR_MODE;
+ cmd->tid = XI3C_SDR_TID;
+ cmd->continued = false;
+
+ ret = xi3c_master_common_xfer(master, xfer);
+
+ /*
+ * Report the broadcast command's error if it failed, otherwise the
+ * directed command's, so a NACK on either phase reaches the caller.
+ */
+ ccc->err = xfer->cmds[0].err ? xfer->cmds[0].err : xfer->cmds[1].err;
+
+ return ret;
+}
+
+static int xi3c_master_send_ccc_cmd(struct i3c_master_controller *m,
+ struct i3c_ccc_cmd *cmd)
+{
+ struct xi3c_master *master = to_xi3c_master(m);
+
+ if (cmd->id & I3C_CCC_DIRECT)
+ return xi3c_master_send_direct_ccc_cmd(master, cmd);
+
+ return xi3c_master_send_bdcast_ccc_cmd(master, cmd);
+}
+
+static int xi3c_master_i3c_xfers(struct i3c_dev_desc *dev,
+ struct i3c_xfer *xfers,
+ int nxfers, enum i3c_xfer_mode mode)
+{
+ struct i3c_master_controller *m = i3c_dev_get_master(dev);
+ struct xi3c_master *master = to_xi3c_master(m);
+ struct xi3c_xfer *xfer __free(kfree) = NULL;
+ int i, ret;
+
+ if (!nxfers)
+ return 0;
+
+ if (mode != I3C_SDR)
+ return -EOPNOTSUPP;
+
+ for (i = 0; i < nxfers; i++)
+ if (xfers[i].len > XI3C_MAXDATA_LENGTH)
+ return -EINVAL;
+
+ xfer = xi3c_master_alloc_xfer(nxfers);
+ if (!xfer)
+ return -ENOMEM;
+
+ for (i = 0; i < nxfers; i++) {
+ struct xi3c_cmd *cmd = &xfer->cmds[i];
+
+ cmd->addr = dev->info.dyn_addr;
+ cmd->rnw = xfers[i].rnw;
+
+ if (cmd->rnw) {
+ cmd->rx_buf = xfers[i].data.in;
+ cmd->rx_len = xfers[i].len;
+ } else {
+ cmd->tx_buf = xfers[i].data.out;
+ cmd->tx_len = xfers[i].len;
+ }
+
+ cmd->type = XI3C_SDR_MODE;
+ cmd->tid = XI3C_SDR_TID;
+ cmd->continued = (i + 1) < nxfers;
+ }
+
+ ret = xi3c_master_common_xfer(master, xfer);
+
+ for (i = 0; i < nxfers; i++) {
+ xfers[i].err = xfer->cmds[i].err;
+ if (xfers[i].rnw)
+ xfers[i].actual_len = xfers[i].len - xfer->cmds[i].rx_len;
+ }
+
+ return ret;
+}
+
+static int xi3c_master_i2c_xfers(struct i2c_dev_desc *dev,
+ struct i2c_msg *xfers,
+ int nxfers)
+{
+ struct i3c_master_controller *m = i2c_dev_get_master(dev);
+ struct xi3c_master *master = to_xi3c_master(m);
+ struct xi3c_xfer *xfer __free(kfree) = NULL;
+ int i;
+
+ if (!nxfers)
+ return 0;
+
+ for (i = 0; i < nxfers; i++)
+ if (xfers[i].len > XI3C_MAXDATA_LENGTH)
+ return -EINVAL;
+
+ xfer = xi3c_master_alloc_xfer(nxfers);
+ if (!xfer)
+ return -ENOMEM;
+
+ for (i = 0; i < nxfers; i++) {
+ struct xi3c_cmd *cmd = &xfer->cmds[i];
+
+ cmd->addr = xfers[i].addr & XI3C_ADDR_MASK;
+ cmd->rnw = !!(xfers[i].flags & I2C_M_RD);
+
+ if (cmd->rnw) {
+ cmd->rx_buf = xfers[i].buf;
+ cmd->rx_len = xfers[i].len;
+ } else {
+ cmd->tx_buf = xfers[i].buf;
+ cmd->tx_len = xfers[i].len;
+ }
+
+ cmd->type = XI3C_I2C_MODE;
+ cmd->tid = XI3C_I2C_TID;
+ cmd->continued = (i + 1) < nxfers;
+ }
+
+ return xi3c_master_common_xfer(master, xfer);
+}
+
+static int xi3c_clk_cfg(struct xi3c_master *master, unsigned long sclhz, u8 mode)
+{
+ unsigned long core_rate, core_periodns;
+ u32 tcasmin, tsustart, tsustop, thdstart;
+ u32 thigh, tlow, thold;
+ u32 odthigh, odtlow;
+
+ core_rate = clk_get_rate(master->pclk);
+ if (!core_rate)
+ return -EINVAL;
+
+ if (!sclhz)
+ return -EINVAL;
+
+ core_periodns = DIV_ROUND_UP(NSEC_PER_SEC, core_rate);
+
+ thigh = DIV_ROUND_UP(core_rate, sclhz) >> 1;
+ tlow = thigh;
+
+ if (thigh <= XI3C_CYCLE_ADJUST)
+ return -EINVAL;
+
+ /* Hold time : 40% of tlow time */
+ thold = (tlow * 4) / 10;
+
+ if (xi3c_get_revision_number(master) == 0)
+ thold = max_t(u32, thold, XI3C_THOLD_MIN_REV0);
+ else
+ thold = max_t(u32, thold, XI3C_THOLD_MIN_REV1);
+
+ iowrite32((thigh - XI3C_CYCLE_ADJUST) & XI3C_TIMING_MASK,
+ master->membase + XI3C_SCL_HIGH_TIME_OFFSET);
+ iowrite32((tlow - XI3C_CYCLE_ADJUST) & XI3C_TIMING_MASK,
+ master->membase + XI3C_SCL_LOW_TIME_OFFSET);
+ iowrite32((thold - XI3C_CYCLE_ADJUST) & XI3C_TIMING_MASK,
+ master->membase + XI3C_SDA_HOLD_TIME_OFFSET);
+
+ if (mode == XI3C_I2C_MODE) {
+ iowrite32((thigh - XI3C_CYCLE_ADJUST) & XI3C_TIMING_MASK,
+ master->membase + XI3C_OD_SCL_HIGH_TIME_OFFSET);
+ iowrite32((tlow - XI3C_CYCLE_ADJUST) & XI3C_TIMING_MASK,
+ master->membase + XI3C_OD_SCL_LOW_TIME_OFFSET);
+
+ tcasmin = DIV_ROUND_UP(XI3C_I2C_TCASMIN_NS, core_periodns);
+ } else {
+ odtlow = DIV_ROUND_UP(XI3C_OD_TLOW_NS, core_periodns);
+ odthigh = DIV_ROUND_UP(XI3C_OD_THIGH_NS, core_periodns);
+
+ odtlow = max(tlow, odtlow);
+ odthigh = min(thigh, odthigh);
+
+ if (odthigh <= XI3C_CYCLE_ADJUST)
+ return -EINVAL;
+
+ iowrite32((odthigh - XI3C_CYCLE_ADJUST) & XI3C_TIMING_MASK,
+ master->membase + XI3C_OD_SCL_HIGH_TIME_OFFSET);
+ iowrite32((odtlow - XI3C_CYCLE_ADJUST) & XI3C_TIMING_MASK,
+ master->membase + XI3C_OD_SCL_LOW_TIME_OFFSET);
+
+ tcasmin = DIV_ROUND_UP(XI3C_TCASMIN_NS, core_periodns);
+ }
+
+ thdstart = max(thigh, tcasmin);
+ tsustart = max(tlow, tcasmin);
+ tsustop = max(tlow, tcasmin);
+
+ iowrite32((tsustart - XI3C_CYCLE_ADJUST) & XI3C_TIMING_MASK,
+ master->membase + XI3C_TSU_START_OFFSET);
+ iowrite32((thdstart - XI3C_CYCLE_ADJUST) & XI3C_TIMING_MASK,
+ master->membase + XI3C_THD_START_OFFSET);
+ iowrite32((tsustop - XI3C_CYCLE_ADJUST) & XI3C_TIMING_MASK,
+ master->membase + XI3C_TSU_STOP_OFFSET);
+
+ return 0;
+}
+
+static int xi3c_master_bus_init(struct i3c_master_controller *m)
+{
+ struct xi3c_master *master = to_xi3c_master(m);
+ struct i3c_bus *bus = i3c_master_get_bus(m);
+ struct i3c_device_info info = {};
+ unsigned long sclhz;
+ u32 pid1_bcr_dcr;
+ u8 mode;
+ int ret;
+
+ switch (bus->mode) {
+ case I3C_BUS_MODE_MIXED_FAST:
+ case I3C_BUS_MODE_MIXED_LIMITED:
+ case I3C_BUS_MODE_MIXED_SLOW:
+ mode = XI3C_I2C_MODE;
+ sclhz = bus->scl_rate.i2c;
+ break;
+ case I3C_BUS_MODE_PURE:
+ mode = XI3C_SDR_MODE;
+ sclhz = bus->scl_rate.i3c;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ ret = xi3c_clk_cfg(master, sclhz, mode);
+ if (ret)
+ return ret;
+
+ xi3c_master_init(master);
+
+ /* Get an address for the master. */
+ ret = i3c_master_get_free_addr(m, 0);
+ if (ret < 0)
+ return ret;
+
+ info.dyn_addr = ret;
+
+ /* Write the dynamic address value to the address register. */
+ iowrite32(info.dyn_addr, master->membase + XI3C_ADDRESS_OFFSET);
+
+ /* Read PID, BCR and DCR values, and assign to i3c device info. */
+ pid1_bcr_dcr = ioread32(master->membase + XI3C_PID1_BCR_DCR);
+ info.pid = ((u64)FIELD_GET(XI3C_PID1_MASK, pid1_bcr_dcr) << 32) |
+ ioread32(master->membase + XI3C_PID0_OFFSET);
+ info.bcr = FIELD_GET(XI3C_BCR_MASK, pid1_bcr_dcr);
+ info.dcr = FIELD_GET(XI3C_DCR_MASK, pid1_bcr_dcr);
+
+ return i3c_master_set_info(&master->base, &info);
+}
+
+static void xi3c_master_bus_cleanup(struct i3c_master_controller *m)
+{
+ struct xi3c_master *master = to_xi3c_master(m);
+
+ xi3c_master_disable(master);
+}
+
+static const struct i3c_master_controller_ops xi3c_master_ops = {
+ .bus_init = xi3c_master_bus_init,
+ .bus_cleanup = xi3c_master_bus_cleanup,
+ .do_daa = xi3c_master_do_daa,
+ .supports_ccc_cmd = xi3c_master_supports_ccc_cmd,
+ .send_ccc_cmd = xi3c_master_send_ccc_cmd,
+ .i3c_xfers = xi3c_master_i3c_xfers,
+ .i2c_xfers = xi3c_master_i2c_xfers,
+};
+
+static int xi3c_master_probe(struct platform_device *pdev)
+{
+ struct xi3c_master *master;
+ int ret;
+
+ master = devm_kzalloc(&pdev->dev, sizeof(*master), GFP_KERNEL);
+ if (!master)
+ return -ENOMEM;
+
+ master->dev = &pdev->dev;
+
+ master->membase = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(master->membase))
+ return dev_err_probe(master->dev, PTR_ERR(master->membase),
+ "Failed to map registers\n");
+
+ master->pclk = devm_clk_get_enabled(master->dev, NULL);
+ if (IS_ERR(master->pclk))
+ return dev_err_probe(master->dev, PTR_ERR(master->pclk),
+ "Failed to get and enable clock\n");
+
+ ret = devm_mutex_init(master->dev, &master->lock);
+ if (ret)
+ return ret;
+
+ platform_set_drvdata(pdev, master);
+
+ return i3c_master_register(&master->base, master->dev,
+ &xi3c_master_ops, false);
+}
+
+static void xi3c_master_remove(struct platform_device *pdev)
+{
+ struct xi3c_master *master = platform_get_drvdata(pdev);
+
+ i3c_master_unregister(&master->base);
+}
+
+static const struct of_device_id xi3c_master_of_ids[] = {
+ { .compatible = "xlnx,axi-i3c-1.0" },
+ { },
+};
+MODULE_DEVICE_TABLE(of, xi3c_master_of_ids);
+
+static struct platform_driver xi3c_master_driver = {
+ .probe = xi3c_master_probe,
+ .remove = xi3c_master_remove,
+ .driver = {
+ .name = "axi-i3c-master",
+ .of_match_table = xi3c_master_of_ids,
+ },
+};
+module_platform_driver(xi3c_master_driver);
+
+MODULE_AUTHOR("Manikanta Guntupalli <manikanta.guntupalli@amd.com>");
+MODULE_AUTHOR("Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>");
+MODULE_AUTHOR("Shubham Patil <shubhamsanjay.patil@amd.com>");
+MODULE_DESCRIPTION("AMD AXI I3C master driver");
+MODULE_LICENSE("GPL");
--
2.34.1
^ permalink raw reply related
* [PATCH v9 1/2] dt-bindings: i3c: Add AMD I3C master controller support
From: Shubham Patil @ 2026-06-23 11:44 UTC (permalink / raw)
To: git, michal.simek, alexandre.belloni, Frank.Li, robh, krzk+dt,
conor+dt, pgaj, wsa+renesas, tommaso.merciai.xr, arnd,
quic_msavaliy, Shyam-sundar.S-k, sakari.ailus, billy_tsai, kees,
gustavoars, jarkko.nikula, jorge.marques, linux-i3c, devicetree,
linux-kernel, linux-arch, linux-hardening
Cc: radhey.shyam.pandey, srinivas.goud, shubhrajyoti.datta,
shubhamsanjay.patil, Manikanta Guntupalli
In-Reply-To: <20260623114417.2578189-1-shubhamsanjay.patil@amd.com>
From: Manikanta Guntupalli <manikanta.guntupalli@amd.com>
Add device tree binding documentation for the AMD I3C master controller
version 1.0.
Signed-off-by: Manikanta Guntupalli <manikanta.guntupalli@amd.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
---
Changes for V9:
None.
Changes for V8:
None.
Changes for V7:
Added i3c controller version details to commit description.
Changes for V6:
Corrected the file name for $id in yaml to fix the dtschema warning.
Changes for V5:
Renamed the xlnx,axi-i3c.yaml file into xlnx,axi-i3c-1.0.yaml.
Changes for V4:
Added h/w documentation details.
Changes for V3:
Updated commit description.
Corrected the order of properties and removed resets property.
Added compatible to required list.
Added interrupts to example.
Changes for V2:
Updated commit subject and description.
Moved allOf to after required.
Removed xlnx,num-targets property.
---
.../bindings/i3c/xlnx,axi-i3c-1.0.yaml | 56 +++++++++++++++++++
1 file changed, 56 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i3c/xlnx,axi-i3c-1.0.yaml
diff --git a/Documentation/devicetree/bindings/i3c/xlnx,axi-i3c-1.0.yaml b/Documentation/devicetree/bindings/i3c/xlnx,axi-i3c-1.0.yaml
new file mode 100644
index 000000000000..75f677696f02
--- /dev/null
+++ b/Documentation/devicetree/bindings/i3c/xlnx,axi-i3c-1.0.yaml
@@ -0,0 +1,56 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i3c/xlnx,axi-i3c-1.0.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AMD I3C master
+
+maintainers:
+ - Shubhrajyoti Datta <shubhrajyoti.datta@amd.com>
+ - Shubham Patil <shubhamsanjay.patil@amd.com>
+
+description:
+ The AXI-I3C IP is an I3C Controller with an AXI4-Lite interface, compatible
+ with the MIPI I3C Specification v1.1.1. The design includes bidirectional I/O
+ buffers that implement open collector drivers for the SDA and SCL signals.
+ External pull-up resistors are required to properly hold the bus at a Logic-1
+ level when the drivers are released.
+
+ For more details, please see https://docs.amd.com/r/en-US/pg439-axi-i3c
+
+properties:
+ compatible:
+ const: xlnx,axi-i3c-1.0
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - clocks
+
+allOf:
+ - $ref: i3c.yaml#
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ i3c@80000000 {
+ compatible = "xlnx,axi-i3c-1.0";
+ reg = <0x80000000 0x10000>;
+ clocks = <&zynqmp_clk 71>;
+ interrupt-parent = <&imux>;
+ interrupts = <0 89 4>;
+ #address-cells = <3>;
+ #size-cells = <0>;
+ };
+...
--
2.34.1
^ permalink raw reply related
* [PATCH v9 0/2] Add AMD I3C master controller driver and bindings
From: Shubham Patil @ 2026-06-23 11:44 UTC (permalink / raw)
To: git, michal.simek, alexandre.belloni, Frank.Li, robh, krzk+dt,
conor+dt, pgaj, wsa+renesas, tommaso.merciai.xr, arnd,
quic_msavaliy, Shyam-sundar.S-k, sakari.ailus, billy_tsai, kees,
gustavoars, jarkko.nikula, jorge.marques, linux-i3c, devicetree,
linux-kernel, linux-arch, linux-hardening
Cc: radhey.shyam.pandey, srinivas.goud, shubhrajyoti.datta,
shubhamsanjay.patil
This patch series introduces support for the AMD I3C master controller,
including the device tree binding and driver implementation.
Compared to v8, the big-endian MMIO accessor and i3c FIFO-endianness
patches have been dropped; the driver now handles big-endian FIFO
accesses internally.
Note: There was an extended gap since v8 due to the transfer of ownership
of this series from Manikanta to Shubham. This transition contributed
to the delay in releasing the v9 update addressing the v8 review comments.
We appreciate your patience.
---
Changes for V9:
Dropped the three big-endian MMIO/FIFO infrastructure patches; the
driver now handles big-endian FIFO accesses internally.
Replaced the async transfer-queue with a synchronous transfer path.
Reworked error/response handling using enum i3c_error_code and proper
-ENODEV/-EIO returns; propagate err to CCCs and priv xfers.
Switched to the new .i3c_xfers op (reject non-SDR, report actual_len).
Reworked DAA (incremental address assignment, -ENOSPC bound, -ENODEV
end-of-enumeration, checked device registration).
Avoid busy-spin with usleep_range(); use FIELD_PREP() and inline
helpers; split the timeout macros with documented units.
Dropped ENTHDR (SDR-only); updated MAINTAINERS, Kconfig symbol
(AMD_AXI_I3C_MASTER), authors and binding maintainers.
Changes for V8:
Included dependent patch "i3c: fix big-endian FIFO transfers"
to this series as [3/5].
Resolved conflicts with "i3c: fix big-endian FIFO transfers".
Updated description.
Used time_left instead of timeout.
Used __free(kfree) for xfer to simplify err path in multiple places.
Changes for V7:
Added i3c controller version details to commit description.
Added Reviewed-by tag to binding patch [1/4].
Added big-endian MMIO accessors [2/4].
Added endianness support for i3c_readl_fifo() and i3c_writel_fifo() [3/4].
Updated timeout macro name.
Updated xi3c_master_wr_to_tx_fifo() and xi3c_master_rd_from_rx_fifo()
to use i3c_writel_fifo() and i3c_readl_fifo().
Changes for V6:
Corrected the $id in the YAML file to match the filename and fix
the dtschema warning.
Removed typecast for xi3c_getrevisionnumber(), xi3c_wrfifolevel(),
and xi3c_rdfifolevel().
Replaced dynamic allocation with a static variable for pid_bcr_dcr.
Fixed sparse warning in do_daa by typecasting the address parity value
to u8.
Fixed sparse warning in xi3c_master_bus_init by typecasting the pid value
to u64 in info.pid calculation.
Changes for V5:
Renamed the xlnx,axi-i3c.yaml file into xlnx,axi-i3c-1.0.yaml.
Used GENMASK_ULL for PID mask as it's 64bit mask.
Changes for V4:
Added h/w documentation details.
Updated timeout macros.
Removed type casting for xi3c_is_resp_available() macro.
Used ioread32() and iowrite32() instead of readl() and writel()
to keep consistency.
Read XI3C_RESET_OFFSET reg before udelay().
Removed xi3c_master_free_xfer() and directly used kfree().
Skipped checking return value of i3c_master_add_i3c_dev_locked().
Used devm_mutex_init() instead of mutex_init().
Changes for V3:
Updated commit description.
Corrected the order of properties and removed resets property.
Added compatible to required list.
Added interrupts to example.
Resolved merge conflicts.
Changes for V2:
Updated commit subject and description.
Moved allOf to after required.
Removed xlnx,num-targets property.
Added mixed mode support with clock configuration.
Converted smaller functions into inline functions.
Used FIELD_GET() in xi3c_get_response().
Updated xi3c_master_rd_from_rx_fifo() to use cmd->rx_buf.
Used parity8() for address parity calculation.
Added guards for locks.
Dropped num_targets and updated xi3c_master_do_daa().
Used __free(kfree) in xi3c_master_send_bdcast_ccc_cmd().
Dropped PM runtime support.
Updated xi3c_master_read() and xi3c_master_write() with
xi3c_is_resp_available() check.
Created separate functions: xi3c_master_init() and xi3c_master_reinit().
Used xi3c_master_init() in bus initialization and xi3c_master_reinit()
in error paths.
Added DAA structure to xi3c_master structure.
---
Manikanta Guntupalli (2):
dt-bindings: i3c: Add AMD I3C master controller support
i3c: master: Add driver for AMD AXI I3C master controller
.../bindings/i3c/xlnx,axi-i3c-1.0.yaml | 56 +
MAINTAINERS | 8 +
drivers/i3c/master/Kconfig | 15 +
drivers/i3c/master/Makefile | 1 +
drivers/i3c/master/amd-i3c-master.c | 1060 +++++++++++++++++
5 files changed, 1140 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i3c/xlnx,axi-i3c-1.0.yaml
create mode 100644 drivers/i3c/master/amd-i3c-master.c
--
2.34.1
^ permalink raw reply
* Re: [PATCH] arm64: dts: qcom: eliza: Fix disp_cc_mdss_mdp_clk_src RCG stall on Eliza EVK
From: Konrad Dybcio @ 2026-06-23 11:43 UTC (permalink / raw)
To: Krzysztof Kozlowski, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, devicetree,
linux-kernel
In-Reply-To: <6ad8d604-b04e-4f24-b616-980f0e18b4c5@oss.qualcomm.com>
On 6/23/26 1:42 PM, Krzysztof Kozlowski wrote:
> On 23/06/2026 13:38, Konrad Dybcio wrote:
>> On 6/23/26 1:31 PM, Krzysztof Kozlowski wrote:
>>> On 23/06/2026 13:27, Krzysztof Kozlowski wrote:
>>>> Eliza EVK (eliza-cqs-evk.dts) does not have display enabled, however its
>>>> Display Clock Controller is enabled and references parent clocks from
>>>> DSI PHYs. Devices which in base DTSI do not have all required resources
>>>> available (e.g. because they are simply disabled), should not be enabled
>>>> in the first place.
>>>>
>>>> Having DISPCC enabled without DSI PHYs causes clock reparenting issues
>>>> and warning on Eliza EVK:
>>>>
>>>> disp_cc_mdss_mdp_clk_src: rcg didn't update its configuration.
>>>> WARNING: drivers/clk/qcom/clk-rcg2.c:136 at update_config+0xd4/0xe4, CPU#1: udevd/273
>>>> ...
>>>> update_config (drivers/clk/qcom/clk-rcg2.c:136 (discriminator 2)) (P)
>>>> clk_rcg2_shared_disable (drivers/clk/qcom/clk-rcg2.c:1471)
>>>> clk_rcg2_shared_init (drivers/clk/qcom/clk-rcg2.c:1540)
>>>> __clk_register (drivers/clk/clk.c:3959 drivers/clk/clk.c:4368)
>>>> devm_clk_hw_register (drivers/clk/clk.c:4448 (discriminator 1) drivers/clk/clk.c:4672 (discriminator 1))
>>>> devm_clk_register_regmap (drivers/clk/qcom/clk-regmap.c:104)
>>>> qcom_cc_really_probe (drivers/clk/qcom/common.c:418)
>>>> qcom_cc_probe (drivers/clk/qcom/common.c:445)
>>>> disp_cc_eliza_probe (dispcc-eliza.c:?) dispcc_eliza
>>>> platform_probe (drivers/base/platform.c:1432)
>>>>
>>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
>>>> ---
>>>> arch/arm64/boot/dts/qcom/eliza-mtp.dts | 4 ++++
>>>> arch/arm64/boot/dts/qcom/eliza.dtsi | 1 +
>>>> 2 files changed, 5 insertions(+)
>>>
>>>
>>> I should call it RFC, because this feels like a band-aid and should be
>>> fixed in clock drivers maybe. Eventually DISPCC should be enabled on
>>> Eliza EVK for HDMI, but DSI PHY will stay disabled.
>>
>> I'd say all of that hardware should be kept enabled, if only to
>> make sure that it's parked safely
>>
>
> You mean enable DSI PHY, even though there is nothing attached?
Yes, so that Linux can shut it off no matter its initial state (which
may include "partially initialized" or "partially shut down")
> My warning probably can be fixed same way as:
> https://lore.kernel.org/all/20260622-sm8450-qol-v1-1-37e2ee8df9da@proton.me/
Quite possibly. IIRC Mike Tipton wasn't a huge fan of park-at-init
to begin with.
Konrad
^ permalink raw reply
* [PATCH v1] arm64: dts: ti: k3-am62-verdin: Add RPi Touch Display 2 7-inch
From: Francesco Dolcini @ 2026-06-23 11:43 UTC (permalink / raw)
To: Nishanth Menon, Vignesh Raghavendra, Tero Kristo, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Francesco Dolcini, linux-arm-kernel, devicetree, linux-kernel
From: Francesco Dolcini <francesco.dolcini@toradex.com>
Add a device tree overlay for the Raspberry Pi Touch Display 2 7" on
the Verdin DSI_1 interface.
Link: https://www.raspberrypi.com/products/touch-display-2/
Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
---
arch/arm64/boot/dts/ti/Makefile | 5 +
.../ti/k3-am625-verdin-rpi-display-2-7in.dtso | 102 ++++++++++++++++++
2 files changed, 107 insertions(+)
create mode 100644 arch/arm64/boot/dts/ti/k3-am625-verdin-rpi-display-2-7in.dtso
diff --git a/arch/arm64/boot/dts/ti/Makefile b/arch/arm64/boot/dts/ti/Makefile
index 371f9a043fe5..f7f5448fcd84 100644
--- a/arch/arm64/boot/dts/ti/Makefile
+++ b/arch/arm64/boot/dts/ti/Makefile
@@ -48,6 +48,7 @@ dtb-$(CONFIG_ARCH_K3) += k3-am625-verdin-ov5640.dtbo
dtb-$(CONFIG_ARCH_K3) += k3-am625-verdin-panel-cap-touch-10inch-dsi.dtbo
dtb-$(CONFIG_ARCH_K3) += k3-am625-verdin-panel-cap-touch-10inch-lvds.dtbo
dtb-$(CONFIG_ARCH_K3) += k3-am625-verdin-panel-cap-touch-7inch-dsi.dtbo
+dtb-$(CONFIG_ARCH_K3) += k3-am625-verdin-rpi-display-2-7in.dtbo
dtb-$(CONFIG_ARCH_K3) += k3-am625-verdin-uart4-mcu.dtbo
dtb-$(CONFIG_ARCH_K3) += k3-am625-verdin-wifi-dahlia-dsi-to-hdmi.dtb
dtb-$(CONFIG_ARCH_K3) += k3-am625-verdin-wifi-dahlia-panel-cap-touch-10inch-dsi.dtb
@@ -244,6 +245,9 @@ k3-am625-verdin-wifi-dev-ov5640-dtbs := k3-am625-verdin-wifi-dev.dtb \
k3-am625-verdin-wifi-dev-panel-cap-touch-7inch-dsi-dtbs := \
k3-am625-verdin-wifi-dev.dtb \
k3-am625-verdin-panel-cap-touch-7inch-dsi.dtbo
+k3-am625-verdin-wifi-dev-rpi-display-2-7in-dtbs := \
+ k3-am625-verdin-wifi-dev.dtb \
+ k3-am625-verdin-rpi-display-2-7in.dtbo
k3-am625-verdin-wifi-dev-uart4-mcu-dtbs := k3-am625-verdin-wifi-dev.dtb \
k3-am625-verdin-uart4-mcu.dtbo
k3-am625-verdin-wifi-mallow-panel-cap-touch-10inch-lvds-dtbs := \
@@ -357,6 +361,7 @@ dtb- += k3-am625-beagleplay-csi2-ov5640.dtb \
k3-am625-verdin-wifi-dev-ov5640-24mhz.dtb \
k3-am625-verdin-wifi-dev-ov5640.dtb \
k3-am625-verdin-wifi-dev-panel-cap-touch-7inch-dsi.dtb \
+ k3-am625-verdin-wifi-dev-rpi-display-2-7in.dtb \
k3-am625-verdin-wifi-dev-uart4-mcu.dtb \
k3-am625-verdin-wifi-mallow-panel-cap-touch-10inch-lvds.dtb \
k3-am62-lp-sk-hdmi-audio.dtb \
diff --git a/arch/arm64/boot/dts/ti/k3-am625-verdin-rpi-display-2-7in.dtso b/arch/arm64/boot/dts/ti/k3-am625-verdin-rpi-display-2-7in.dtso
new file mode 100644
index 000000000000..9a2e7a170a28
--- /dev/null
+++ b/arch/arm64/boot/dts/ti/k3-am625-verdin-rpi-display-2-7in.dtso
@@ -0,0 +1,102 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
+/*
+ * Copyright (c) Toradex
+ *
+ * Raspberry Pi Touch Display 2 7" on Verdin DSI_1 and I2C_2_DSI
+ *
+ * https://www.raspberrypi.com/products/touch-display-2/
+ */
+
+/dts-v1/;
+/plugin/;
+
+#include <dt-bindings/gpio/gpio.h>
+
+&{/} {
+ backlight_rpi: backlight-rpi-display {
+ compatible = "pwm-backlight";
+ brightness-levels = <0 31>;
+ default-brightness-level = <15>;
+ num-interpolated-steps = <31>;
+ pwms = <&mcu_display_rpi 0 200000 0>;
+ };
+
+ reg_display_rpi: regulator-rpi-display {
+ compatible = "regulator-fixed";
+ regulator-max-microvolt = <5000000>;
+ regulator-min-microvolt = <5000000>;
+ regulator-name = "rpi-display";
+ };
+
+ reg_display_rpi_touch: regulator-rpi-display-touch {
+ compatible = "regulator-fixed";
+ gpio = <&mcu_display_rpi 1 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ regulator-max-microvolt = <3300000>;
+ regulator-min-microvolt = <3300000>;
+ regulator-name = "rpi-display-touch";
+ startup-delay-us = <50000>;
+ };
+
+};
+
+/* Verdin I2C_2_DSI */
+&main_i2c2 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ mcu_display_rpi: regulator@45 {
+ compatible = "raspberrypi,touchscreen-panel-regulator-v2";
+ reg = <0x45>;
+ #gpio-cells = <2>;
+ #pwm-cells = <3>;
+ gpio-controller;
+ };
+
+ touchscreen@5d {
+ compatible = "goodix,gt911";
+ reg = <0x5d>;
+ AVDD28-supply = <®_display_rpi_touch>;
+ touchscreen-size-x = <720>;
+ touchscreen-size-y = <1280>;
+ };
+};
+
+&dsi_bridge {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ status = "okay";
+
+ panel@0 {
+ compatible = "raspberrypi,dsi-7inch", "ilitek,ili9881c";
+ reg = <0>;
+ backlight = <&backlight_rpi>;
+ power-supply = <®_display_rpi>;
+ reset-gpios = <&mcu_display_rpi 0 GPIO_ACTIVE_LOW>;
+
+ port {
+ dsi_panel_in: endpoint {
+ remote-endpoint = <&dsi_bridge_out>;
+ };
+ };
+ };
+};
+
+&dsi_bridge_ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@1 {
+ reg = <1>;
+
+ dsi_bridge_out: endpoint {
+ data-lanes = <1 2>;
+ remote-endpoint = <&dsi_panel_in>;
+ };
+ };
+};
+
+&dss {
+ status = "okay";
+};
--
2.47.3
^ permalink raw reply related
* Re: [PATCH] arm64: dts: qcom: eliza: Fix disp_cc_mdss_mdp_clk_src RCG stall on Eliza EVK
From: Krzysztof Kozlowski @ 2026-06-23 11:42 UTC (permalink / raw)
To: Konrad Dybcio, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, devicetree,
linux-kernel
In-Reply-To: <be95b95b-dbcb-4b80-94dd-a7e97ef4c446@oss.qualcomm.com>
On 23/06/2026 13:38, Konrad Dybcio wrote:
> On 6/23/26 1:31 PM, Krzysztof Kozlowski wrote:
>> On 23/06/2026 13:27, Krzysztof Kozlowski wrote:
>>> Eliza EVK (eliza-cqs-evk.dts) does not have display enabled, however its
>>> Display Clock Controller is enabled and references parent clocks from
>>> DSI PHYs. Devices which in base DTSI do not have all required resources
>>> available (e.g. because they are simply disabled), should not be enabled
>>> in the first place.
>>>
>>> Having DISPCC enabled without DSI PHYs causes clock reparenting issues
>>> and warning on Eliza EVK:
>>>
>>> disp_cc_mdss_mdp_clk_src: rcg didn't update its configuration.
>>> WARNING: drivers/clk/qcom/clk-rcg2.c:136 at update_config+0xd4/0xe4, CPU#1: udevd/273
>>> ...
>>> update_config (drivers/clk/qcom/clk-rcg2.c:136 (discriminator 2)) (P)
>>> clk_rcg2_shared_disable (drivers/clk/qcom/clk-rcg2.c:1471)
>>> clk_rcg2_shared_init (drivers/clk/qcom/clk-rcg2.c:1540)
>>> __clk_register (drivers/clk/clk.c:3959 drivers/clk/clk.c:4368)
>>> devm_clk_hw_register (drivers/clk/clk.c:4448 (discriminator 1) drivers/clk/clk.c:4672 (discriminator 1))
>>> devm_clk_register_regmap (drivers/clk/qcom/clk-regmap.c:104)
>>> qcom_cc_really_probe (drivers/clk/qcom/common.c:418)
>>> qcom_cc_probe (drivers/clk/qcom/common.c:445)
>>> disp_cc_eliza_probe (dispcc-eliza.c:?) dispcc_eliza
>>> platform_probe (drivers/base/platform.c:1432)
>>>
>>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
>>> ---
>>> arch/arm64/boot/dts/qcom/eliza-mtp.dts | 4 ++++
>>> arch/arm64/boot/dts/qcom/eliza.dtsi | 1 +
>>> 2 files changed, 5 insertions(+)
>>
>>
>> I should call it RFC, because this feels like a band-aid and should be
>> fixed in clock drivers maybe. Eventually DISPCC should be enabled on
>> Eliza EVK for HDMI, but DSI PHY will stay disabled.
>
> I'd say all of that hardware should be kept enabled, if only to
> make sure that it's parked safely
>
You mean enable DSI PHY, even though there is nothing attached?
My warning probably can be fixed same way as:
https://lore.kernel.org/all/20260622-sm8450-qol-v1-1-37e2ee8df9da@proton.me/
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 3/3] backlight: lp8864: Convert from LED to backlight class driver
From: Daniel Thompson @ 2026-06-23 11:41 UTC (permalink / raw)
To: Sverdlin, Alexander
Cc: linux-leds@vger.kernel.org, afd@ti.com, jingoohan1@gmail.com,
linux-fbdev@vger.kernel.org, pavel@kernel.org, lee@kernel.org,
dri-devel@lists.freedesktop.org, robh@kernel.org,
linux-kernel@vger.kernel.org, krzk+dt@kernel.org,
danielt@kernel.org, conor+dt@kernel.org, deller@gmx.de,
devicetree@vger.kernel.org
In-Reply-To: <acb13aca040ab72b9e53abee3a5c8733ebc5b8dd.camel@siemens.com>
On Tue, Jun 16, 2026 at 07:17:23AM +0000, Sverdlin, Alexander wrote:
> Hi Andrew!
>
> On Mon, 2026-06-15 at 14:51 -0500, Andrew Davis wrote:
> > > Move the TI LP8864/LP8866 driver from drivers/leds/ to
> > > drivers/video/backlight/
> >
> > Why move it? You can register a backlight device from any directory.
>
> I'm personally fine with the driver residing in drivers/leds, it's
> just that currently there are no combined drivers there, the combined
> drivers providing both interfaces only live in video/backlight.
>
> But if it's OK from the maintainers' perspective, it will be even
> more consistent regarding Kconfig symbol.
>
> > > and convert it to register a backlight class
> > > device as its primary interface.
> > >
> >
> > What do you mean by "primary"? You should be able to register with
> > both frameworks and have the driver interop between as needed.
>
> Well, I only meant the user's (or my own) perspective, sorry for confusion.
>
> > > The motivation is a use case on a hot-pluggable segment of an I2C bus.
> > > The generic led-backlight driver (drivers/video/backlight/led_bl.c) is a
> > > platform driver and as such inherently non-hotpluggable.
> >
> > That isn't strictly true, there is platform_device_{del,unregister}(), so
> > whatever your mechanism for removing the I2C device would be, the same
> > could be done to the led_bl device before then removing the I2C device.
>
> led_bl is not really designed to act on dynamically instantiated devices,
> it's very much device-tree affine (of_count_phandle_with_args(), etc...)
>
> > We don't want to have to move every LED driver that could possibly
> > be used as a backlight to the backlight framework, the led_bl.c
> > handles adapting LED->backlight as needed. So what you really need
> > here is to de-couple led_bl.c from DT so it can better handle dynamic
> > add/remove. Then this LED driver simply could register a "led-backlight"
> > platform driver to handle the backlight interface, and remove the
> > backlight device when it itself (the LED device) is removed.
>
> The mechanism we have regarding hot plugging currently is just I2C bridge,
> which de-registers and registers the bridged bus. So no additional drivers
> are required, as long as I2C devices are self-contained and not glued with
> platform devices.
>
> So bottom line is, I'd prefer to just add the backlight interface to the
> existing driver, no matter where it would live in the future.
Is there any reasion that LP8864/LP8866 is unique in appearing on a
hotplugged I2C bus? In other words if support for dynamism is added
specifically to leds-lp8864.c rather than in led_bl.c then what will stop
the same dynamic tricks from being adde to other LED drivers?
Daniel.
^ permalink raw reply
* Re: [PATCH] arm64: dts: qcom: eliza: Fix disp_cc_mdss_mdp_clk_src RCG stall on Eliza EVK
From: Konrad Dybcio @ 2026-06-23 11:38 UTC (permalink / raw)
To: Krzysztof Kozlowski, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm, devicetree,
linux-kernel
In-Reply-To: <de941d2d-df5d-44b6-b95a-437e35917cd5@oss.qualcomm.com>
On 6/23/26 1:31 PM, Krzysztof Kozlowski wrote:
> On 23/06/2026 13:27, Krzysztof Kozlowski wrote:
>> Eliza EVK (eliza-cqs-evk.dts) does not have display enabled, however its
>> Display Clock Controller is enabled and references parent clocks from
>> DSI PHYs. Devices which in base DTSI do not have all required resources
>> available (e.g. because they are simply disabled), should not be enabled
>> in the first place.
>>
>> Having DISPCC enabled without DSI PHYs causes clock reparenting issues
>> and warning on Eliza EVK:
>>
>> disp_cc_mdss_mdp_clk_src: rcg didn't update its configuration.
>> WARNING: drivers/clk/qcom/clk-rcg2.c:136 at update_config+0xd4/0xe4, CPU#1: udevd/273
>> ...
>> update_config (drivers/clk/qcom/clk-rcg2.c:136 (discriminator 2)) (P)
>> clk_rcg2_shared_disable (drivers/clk/qcom/clk-rcg2.c:1471)
>> clk_rcg2_shared_init (drivers/clk/qcom/clk-rcg2.c:1540)
>> __clk_register (drivers/clk/clk.c:3959 drivers/clk/clk.c:4368)
>> devm_clk_hw_register (drivers/clk/clk.c:4448 (discriminator 1) drivers/clk/clk.c:4672 (discriminator 1))
>> devm_clk_register_regmap (drivers/clk/qcom/clk-regmap.c:104)
>> qcom_cc_really_probe (drivers/clk/qcom/common.c:418)
>> qcom_cc_probe (drivers/clk/qcom/common.c:445)
>> disp_cc_eliza_probe (dispcc-eliza.c:?) dispcc_eliza
>> platform_probe (drivers/base/platform.c:1432)
>>
>> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
>> ---
>> arch/arm64/boot/dts/qcom/eliza-mtp.dts | 4 ++++
>> arch/arm64/boot/dts/qcom/eliza.dtsi | 1 +
>> 2 files changed, 5 insertions(+)
>
>
> I should call it RFC, because this feels like a band-aid and should be
> fixed in clock drivers maybe. Eventually DISPCC should be enabled on
> Eliza EVK for HDMI, but DSI PHY will stay disabled.
I'd say all of that hardware should be kept enabled, if only to
make sure that it's parked safely
Konrad
^ permalink raw reply
* Re: [PATCH v11 5/5] iio: adc: versal-sysmon: add oversampling support
From: Erim, Salih @ 2026-06-23 11:37 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: <20260623014036.3865402-6-salih.erim@amd.com>
Addressing Sashiko findings on this patch:
- [Medium] Missing hardware synchronization for oversampling
ratio caches during initialization.
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 23/06/2026 02:40, 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 v11:
> - Add oversampling info_mask bits to static SYSMON_CHAN_TEMP
> macro (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 | 159 ++++++++++++++++++++++++++-
> drivers/iio/adc/versal-sysmon.h | 17 +++
> 2 files changed, 174 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/adc/versal-sysmon-core.c b/drivers/iio/adc/versal-sysmon-core.c
> index 8f2c502d9cb..1b55d343982 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)
>
> @@ -42,7 +48,11 @@
> .address = _address, \
> .channel = _chan, \
> .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 = _name, \
> }
>
> @@ -166,6 +176,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) {
> @@ -465,6 +481,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)
> @@ -477,6 +619,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,
> @@ -768,6 +913,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 ?
> @@ -799,7 +948,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,
> };
> }
> @@ -846,6 +999,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 v11 4/5] iio: adc: versal-sysmon: add threshold event support
From: Erim, Salih @ 2026-06-23 11:36 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: <20260623014036.3865402-5-salih.erim@amd.com>
Addressing Sashiko findings on this patch:
- [High] Use-After-Free due to flawed devres initialization order
for the delayed worker.
sysmon_disable_interrupts masks all interrupts via SYSMON_IDR
before cancelling the worker. Once masked, no new interrupts
fire and the handler cannot re-schedule the work. The IRQ
handler only schedules work after processing active ISR bits,
which requires unmasked interrupts. No issue.
- [High] Error paths in the IRQ handler skip clearing hardware
flags but unconditionally return IRQ_HANDLED.
Theoretical only. The regmap uses MMIO with fast_io, so
regmap_write cannot fail. The error paths exist for API
correctness. No change needed.
- [High] The delayed worker clears all pending interrupts,
causing voltage alarms to be lost.
The worker only runs when temperature interrupts are masked.
Supply alarms are handled directly in the IRQ handler and are
never masked. A concurrent voltage alarm between the worker's
ISR read and write could theoretically be cleared but the
window is narrow and not observed in testing. No change needed.
- [Medium] Atomic context violation on I2C devices.
The I2C path does not register an IRQ handler.
fwnode_irq_get() returns 0 for I2C nodes and
sysmon_init_interrupt skips IRQ registration. Not applicable.
- [Medium] Negative temperatures invoke undefined left-shift
behavior and are not masked to 16 bits.
Addressed in v11 with bounds checking. Temperature threshold
writes are now validated against the Q8.7 range (-256000 to
255992 mC) before conversion. Out-of-range values return
-EINVAL. The computed lower threshold is also clamped.
- [Medium] Race condition between sysfs event disable and the
unmask worker.
The worker runs on a 500ms delayed schedule. The race window
between the IDR write and temp_mask update is microseconds.
If the worker runs in this window, it self-corrects on the
next cycle. No change needed.
- [Medium] Cached hysteresis value is not reverted if the
hardware threshold write fails.
The regmap uses MMIO with fast_io. MMIO writes cannot fail.
The error path exists for API correctness. No change needed.
Thanks,
Salih
On 23/06/2026 02:40, 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 v11:
> - Add bounds check for temperature threshold writes; return
> -EINVAL if out of Q8.7 range (Jonathan)
> - Add bounds check for supply voltage threshold writes to
> prevent integer overflow (Jonathan)
> - Clamp computed lower threshold to Q8.7 range in
> sysmon_update_temp_lower (found during audit)
> - Add comment explaining that threshold register upper bits
> (FMT/MODE) are ignored on write (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 | 613 ++++++++++++++++++++++++++-
> drivers/iio/adc/versal-sysmon.h | 36 ++
> 2 files changed, 645 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/iio/adc/versal-sysmon-core.c b/drivers/iio/adc/versal-sysmon-core.c
> index 03a745d3fb4..8f2c502d9cb 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,269 @@ 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 = clamp(upper_mc - sysmon->temp_hysteresis, -256000, 255992);
> + 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:
> + /* Q8.7 signed range: -256000 to +255992 mC */
> + if (val < -256000 || val > 255992)
> + return -EINVAL;
> +
> + 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;
> +
> + /* Clamp to prevent overflow in processedtoraw conversion */
> + if (val < -32768 || val > 32767)
> + return -EINVAL;
> +
> + sysmon_supply_processedtoraw(val, reg_val, &raw_val);
> +
> + /*
> + * The hardware threshold register returns FMT and MODE
> + * bits in the upper 16 bits on read, but only the lower
> + * 16-bit mantissa is used on write.
> + */
> + 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 +478,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 +736,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 +768,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 +837,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 +850,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 +860,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 7/8] phy: qcom: qmp-combo: Correct pre-emphasis table for QMP v4 DP PHYs
From: Konrad Dybcio @ 2026-06-23 11:36 UTC (permalink / raw)
To: esteuwu, 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, Dmitry Baryshkov
Cc: linux-arm-msm, linux-clk, linux-kernel, devicetree, iommu,
linux-arm-kernel, linux-phy
In-Reply-To: <20260622-sm8450-qol-v1-7-37e2ee8df9da@proton.me>
On 6/23/26 2:54 AM, Esteban Urrutia via B4 Relay wrote:
> 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 }
It seems like 8350/8450 should be using what this driver calls
v5 tables, with this fixup:
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index cdcfad2e86b1..63a4f2127e3c 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -2134,7 +2134,7 @@ static const u8 qmp_dp_v5_voltage_swing_hbr3_hbr2[4][4] = {
};
static const u8 qmp_dp_v5_pre_emphasis_hbr_rbr[4][4] = {
- { 0x20, 0x2d, 0x34, 0x3a },
+ { 0x20, 0x2e, 0x35, 0x3b },
{ 0x20, 0x2e, 0x35, 0xff },
{ 0x20, 0x2e, 0xff, 0xff },
{ 0x24, 0xff, 0xff, 0xff }
+Dmitry please confirm
Konrad
^ permalink raw reply related
* Re: [PATCH v11 3/5] iio: adc: versal-sysmon: add I2C driver
From: Erim, Salih @ 2026-06-23 11:33 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: <20260623014036.3865402-4-salih.erim@amd.com>
Addressing Sashiko findings on this patch:
- [High] Non-atomic I2C read transaction releases the bus lock,
making it vulnerable to bus interleaving.
The SysMon I2C interface is typically on a dedicated bus with
no other devices. The current implementation works correctly
in all tested configurations. Switching to i2c_transfer() with
Repeated Start would match the TRM protocol diagram but would
require hardware testing on a board with I2C-accessible SysMon.
No change for now.
Thanks,
Salih
On 23/06/2026 02:40, 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 v11:
> - No code changes
>
> 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 net] net: ethernet: qualcomm: ppe: Demote from supported and fix maintainer addresses
From: Andrew Lunn @ 2026-06-23 11:33 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Jie Luo, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Brian Masney, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Lei Wei, Suruchi Agarwal, Pavithra R, linux-kernel,
linux-arm-msm, linux-clk, devicetree, netdev
In-Reply-To: <f8441903-c768-46a1-8f95-b1b25d420a2c@oss.qualcomm.com>
> If address did not work for half a year, I really doubt that you commit
> to above.
I tend to agree. Maybe we should set it to Orphaned, and then decide
in 6 months time if it can be set back to Maintained?
Andrew
^ permalink raw reply
* Re: [PATCH] arm64: dts: qcom: eliza: Fix disp_cc_mdss_mdp_clk_src RCG stall on Eliza EVK
From: Krzysztof Kozlowski @ 2026-06-23 11:31 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260623112755.317180-2-krzysztof.kozlowski@oss.qualcomm.com>
On 23/06/2026 13:27, Krzysztof Kozlowski wrote:
> Eliza EVK (eliza-cqs-evk.dts) does not have display enabled, however its
> Display Clock Controller is enabled and references parent clocks from
> DSI PHYs. Devices which in base DTSI do not have all required resources
> available (e.g. because they are simply disabled), should not be enabled
> in the first place.
>
> Having DISPCC enabled without DSI PHYs causes clock reparenting issues
> and warning on Eliza EVK:
>
> disp_cc_mdss_mdp_clk_src: rcg didn't update its configuration.
> WARNING: drivers/clk/qcom/clk-rcg2.c:136 at update_config+0xd4/0xe4, CPU#1: udevd/273
> ...
> update_config (drivers/clk/qcom/clk-rcg2.c:136 (discriminator 2)) (P)
> clk_rcg2_shared_disable (drivers/clk/qcom/clk-rcg2.c:1471)
> clk_rcg2_shared_init (drivers/clk/qcom/clk-rcg2.c:1540)
> __clk_register (drivers/clk/clk.c:3959 drivers/clk/clk.c:4368)
> devm_clk_hw_register (drivers/clk/clk.c:4448 (discriminator 1) drivers/clk/clk.c:4672 (discriminator 1))
> devm_clk_register_regmap (drivers/clk/qcom/clk-regmap.c:104)
> qcom_cc_really_probe (drivers/clk/qcom/common.c:418)
> qcom_cc_probe (drivers/clk/qcom/common.c:445)
> disp_cc_eliza_probe (dispcc-eliza.c:?) dispcc_eliza
> platform_probe (drivers/base/platform.c:1432)
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> ---
> arch/arm64/boot/dts/qcom/eliza-mtp.dts | 4 ++++
> arch/arm64/boot/dts/qcom/eliza.dtsi | 1 +
> 2 files changed, 5 insertions(+)
I should call it RFC, because this feels like a band-aid and should be
fixed in clock drivers maybe. Eventually DISPCC should be enabled on
Eliza EVK for HDMI, but DSI PHY will stay disabled.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH net] net: ethernet: qualcomm: ppe: Demote from supported and fix maintainer addresses
From: Andrew Lunn @ 2026-06-23 11:31 UTC (permalink / raw)
To: Jie Luo
Cc: Krzysztof Kozlowski, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Brian Masney, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Lei Wei, Suruchi Agarwal, Pavithra R,
linux-kernel, linux-arm-msm, linux-clk, devicetree, netdev
In-Reply-To: <8b0560ae-af5c-4d54-be02-d186be1d799c@oss.qualcomm.com>
On Tue, Jun 23, 2026 at 05:42:34PM +0800, Jie Luo wrote:
>
>
> On 6/23/2026 4:10 PM, Andrew Lunn wrote:
> >> Driver is not supported - in terms of how netdev understands supported
> >> commitment - if maintainer does not care to receive the patches for its
> >> code, so demote it to "maintained" to reflect true status.
> >
> > Maybe "Orphan" would be better, if the listed Maintainer is not doing
> > any Maintainer work?
> >
> > Andrew
>
> Hello Andrew, Krzysztof,
> I will continue to maintain the listed drivers, so their status can
> remain Supported.
Please understand that being a Maintainer requires that you respond to
patches and questions about this driver, give Reviewed-by:, ask for
patches to be changed etc. If you don't respond, ideally with 2 to 3
days, the driver will be set to Orphaned.
If you want to maintain the Supported status, we can help you set up
the needed CI system, and get it registered so it reports the results.
Andrew
^ permalink raw reply
* Re: [PATCH 2/2] arm64: dts: socfpga: agilex7-gen2: Add initial device tree
From: sashiko-bot @ 2026-06-23 11:28 UTC (permalink / raw)
To: muhammad.nazim.amirul.nazle.asmade; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260623111716.16690-3-muhammad.nazim.amirul.nazle.asmade@altera.com>
Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [Critical] Missing mandatory ARM architected timer (`arm,armv8-timer`) node.
- [High] The ATF (ARM Trusted Firmware) reserved memory is improperly declared as a shared DMA pool.
- [Medium] Missing Energy Aware Scheduling (EAS) capacity and topology maps for heterogeneous big.LITTLE CPUs.
- [Medium] Missing UART/serial console and `chosen` node.
- [Medium] Missing VGIC maintenance interrupt in the GICv3 node.
- [Low] Invalid combination of `reg` and `alignment` properties in `reserved-memory` nodes.
- [Low] Incorrect `#address-cells` for 64-bit CPUs.
--
commit 686f550501dd7f94cdc969838594ed32ff4cbd42
Author: Nazim Amirul <muhammad.nazim.amirul.nazle.asmade@altera.com>
arm64: dts: socfpga: agilex7-gen2: Add initial device tree
This patch adds the initial device tree support for the Intel SoCFPGA Agilex7
Gen2 platform. It includes the SoC DTSI and the SoCDK board DTS, featuring
a heterogeneous CPU cluster with Cortex-A520 and Cortex-A720 cores.
> diff --git a/arch/arm64/boot/dts/intel/socfpga_agilex7_gen2.dtsi b/arch/arm64/boot/dts/intel/socfpga_agilex7_gen2.dtsi
> new file mode 100644
> index 0000000000000..e3b777408f9a6
> --- /dev/null
> +++ b/arch/arm64/boot/dts/intel/socfpga_agilex7_gen2.dtsi
> @@ -0,0 +1,119 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2026, Altera Corporation
> + */
> +/dts-v1/;
> +#include <dt-bindings/interrupt-controller/arm-gic.h>
> +#include <dt-bindings/interrupt-controller/irq.h>
> +
> +/ {
> + compatible = "intel,socfpga-agilex7-gen2";
> + #address-cells = <2>;
> + #size-cells = <2>;
[Severity: Critical]
Is it intentional to omit the arm,armv8-timer node from the root level? Without
the architectural timer node, the system may fail to establish a clocksource
and clockevents, which typically leads to an indefinite hang during early boot
when timer_probe() fails.
[Severity: Medium]
Does this platform need a chosen node and UART serial console configured at
the root level? Omitting them usually results in no interactive console or
kernel boot logs over serial, which makes early boot validation very difficult.
> +
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + atf_reserved: atf@80000000 {
> + compatible = "shared-dma-pool";
[Severity: High]
Does declaring the ATF reserved memory as a shared-dma-pool expose the secure
EL3 memory region to the kernel's DMA allocator? If the kernel resolves a DMA
allocation to this region and a device writes to it, it could overwrite the
TrustZone firmware, potentially triggering a secure access violation (SError).
> + reg = <0x0 0x80000000 0x0 0x100000>;
> + alignment = <0x1000>;
[Severity: Low]
Is it necessary to include the alignment property when reg is already
explicitly provided? Device tree schema validation typically rejects combining
reg and alignment in reserved-memory nodes, as the kernel parser typically
ignores alignment when reg is present.
> + no-map;
> + };
> +
> + service_reserved: svcbuffer@80100000 {
> + compatible = "shared-dma-pool";
> + reg = <0x0 0x80100000 0x0 0xf00000>;
> + alignment = <0x1000>;
[Severity: Low]
Similar to the ATF reserved node, is the alignment property needed here when a
reg property is explicitly defined?
> + no-map;
> + };
> + };
> +
> + cpus {
> + #address-cells = <1>;
[Severity: Low]
Should #address-cells be 2 for 64-bit ARM CPUs? While it might work if the
Aff3 field is zero, setting it to 1 truncates the MPIDR_EL1 representation to
32 bits and violates ARM64 device tree binding conventions.
> + #size-cells = <0>;
> +
> + cpu0: cpu@0 {
> + compatible = "arm,cortex-a520";
> + device_type = "cpu";
> + enable-method = "psci";
> + reg = <0x0>;
> + };
[ ... ]
> + cpu3: cpu@300 {
> + compatible = "arm,cortex-a720";
[Severity: Medium]
Are the Energy Aware Scheduling (EAS) capacity and topology maps missing for
these heterogeneous cores? Without them, the scheduler might treat the
Cortex-A520 and Cortex-A720 as having identical performance capabilities,
leading to suboptimal task placement.
> + device_type = "cpu";
> + enable-method = "psci";
> + reg = <0x300>;
> + };
> + };
> +
> + psci {
> + compatible = "arm,psci-0.2";
> + method = "smc";
> + };
> +
> + intc: interrupt-controller@7000000 {
> + compatible = "arm,gic-v3";
> + reg = <0x0 0x7000000 0x0 0x10000>,
> + <0x0 0x7080000 0x0 0x100000>;
> + ranges;
> + #interrupt-cells = <3>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> + interrupt-controller;
> + #redistributor-regions = <1>;
> + redistributor-stride = <0x0 0x40000>;
[Severity: Medium]
Does the GICv3 node need to declare a VGIC maintenance interrupt? Without this
interrupt, the KVM hypervisor might not receive maintenance interrupts,
which can break virtualized interrupt state management for guest VMs.
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260623111716.16690-1-muhammad.nazim.amirul.nazle.asmade@altera.com?part=2
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox