* Re: [PATCH 4/5] dt-bindings: arm: mediatek: Add MT8127 Amazon ford
From: sashiko-bot @ 2026-06-17 2:29 UTC (permalink / raw)
To: Zakariya Hadrami via B4 Relay; +Cc: devicetree, linux-watchdog, robh, conor+dt
In-Reply-To: <20260617-mt8127-amazon-ford-basic-v1-4-d02ad15ac359@proton.me>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The device tree compatible block for `amazon,ford` is incorrectly sorted. The file specifies that entries should be sorted by SoC compatible and then alphabetically by board compatible. Because `amazon,ford` and `mediatek,mt8127-moose` share the same SoC compatible (`mediatek,mt8127`), `amazon,ford` should be placed before `mediatek,mt8127-moose`.
--
commit 5d4dd6329b44b144dcfbe78c4a1cf82524b51ef6
Author: Zakariya Hadrami <zkh1@proton.me>
dt-bindings: arm: mediatek: Add MT8127 Amazon ford
This commit adds the device tree binding for the Amazon ford tablet based
on the MediaTek MT8127 SoC.
> diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml b/Documentation/devicetree/bindings/arm/mediatek.yaml
> index 382d0eb4d0af6..5ddc79689df91 100644
> --- a/Documentation/devicetree/bindings/arm/mediatek.yaml
> +++ b/Documentation/devicetree/bindings/arm/mediatek.yaml
> @@ -124,6 +124,10 @@ properties:
> - enum:
> - mediatek,mt8127-moose
> - const: mediatek,mt8127
> + - items:
> + - enum:
> + - amazon,ford
> + - const: mediatek,mt8127
[Severity: Low]
This isn't a bug, but should the amazon,ford board compatible block be
inserted alphabetically before mediatek,mt8127-moose? Since both share the
mediatek,mt8127 SoC compatible, sorting alphabetically by the board
compatible would usually place amazon,ford first.
> - items:
> - enum:
> - mediatek,mt8135-evbp1
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-mt8127-amazon-ford-basic-v1-0-d02ad15ac359@proton.me?part=4
^ permalink raw reply
* Re: [PATCH 2/5] ARM: dts: mediatek: mt8127: Add watchdog support
From: sashiko-bot @ 2026-06-17 2:30 UTC (permalink / raw)
To: Zakariya Hadrami via B4 Relay; +Cc: robh, linux-watchdog, conor+dt, devicetree
In-Reply-To: <20260617-mt8127-amazon-ford-basic-v1-2-d02ad15ac359@proton.me>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Device tree nodes are not sorted by unit address.
--
commit 7452ca22e20f1d82f0774e2f81b86b3196a78313
Author: Zakariya Hadrami <zkh1@proton.me>
ARM: dts: mediatek: mt8127: Add watchdog support
This commit adds the watchdog node to the mt8127 device tree. It is
disabled by default since it was not present initially.
> diff --git a/arch/arm/boot/dts/mediatek/mt8127.dtsi b/arch/arm/boot/dts/mediatek/mt8127.dtsi
> index bd61ec7e70c0c..1855dda42710f 100644
> --- a/arch/arm/boot/dts/mediatek/mt8127.dtsi
> +++ b/arch/arm/boot/dts/mediatek/mt8127.dtsi
> @@ -159,5 +159,12 @@ uart3: serial@11005000 {
> clocks = <&uart_clk>;
> status = "disabled";
> };
> +
> + watchdog: watchdog@10007000 {
[Severity: Low]
Should this node be sorted by its unit address?
It looks like watchdog@10007000 is being placed after serial@11005000 at the
end of the soc node. Device tree styling conventions usually require sibling
nodes to be sorted in ascending order of their unit address, which would
place this earlier in the file (likely before timer@10008000).
> + compatible = "mediatek,mt8127-wdt","mediatek,mt6589-wdt";
> + reg = <0 0x10007000 0 0x100>;
> + interrupts = <GIC_SPI 88 IRQ_TYPE_LEVEL_LOW>;
> + status = "disabled";
> + };
> };
> };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-mt8127-amazon-ford-basic-v1-0-d02ad15ac359@proton.me?part=2
^ permalink raw reply
* Re: [PATCH 3/4] input: misc: Add Qualcomm SPMI PMIC haptics driver
From: Fenglin Wu @ 2026-06-17 2:31 UTC (permalink / raw)
To: Konrad Dybcio, linux-arm-msm, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Lee Jones, Stephen Boyd,
Bjorn Andersson, Konrad Dybcio
Cc: David Collins, Subbaraman Narayanamurthy, Kamal Wadhwa, kernel,
linux-input, devicetree, linux-kernel
In-Reply-To: <eb693705-c0c3-427b-a924-5aa907fd65bb@oss.qualcomm.com>
>> + ret = ptn_bulk_write(h, HAP_PTN_FIFO_DIN_0_REG, &data[i], 4);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + for (; i < len; i++) {
>> + ret = ptn_write(h, HAP_PTN_FIFO_DIN_1B_REG, (u8)data[i]);
>> + if (ret)
>> + return ret;
>> + }
> So if i'm reading this right, the first loop will always write
> 4*(len//4) bytes and the second one will be entered at most once,
> to write len rem 4 bytes.. should this be an if instead?
I should put a comment for clarification. Here’s some background: FIFO
data writing supports both 4-byte bulk writes using registers
[HAP_PTN_FIFO_DIN_0_REG ... HAP_PTN_FIFO_DIN_3_REG], and 1-byte writes
using the HAP_PTN_FIFO_DIN_1B_REG register. The 4-byte bulk write is
more efficient, especially for waveform which has several Kb data, and
it helps to reduce software latency when loading effects and reduce the
delay in triggering vibration. It also helps prevent the FIFO from
running dry during data refill in FIFO-empty interrupts. Typically, we
use 4-byte writes for the initial 4-byte aligned data, and 1-byte writes
for any trailing remainder.
So it still needs a 'for' loop here since the remainder could be more
than 1 byte.
>> +
>> + return 0;
>> +}
>> +
>> +/*
>> + * Configure the hardware FIFO memory boundary.
>> + * FIFO occupies addresses [0, fifo_len).
>> + */
>> +static int haptics_configure_fifo_mmap(struct qcom_haptics *h)
>> +{
>> + u32 fifo_len, fifo_units;
>> +
>> + /* Config all memory space for FIFO usage for now */
> What's the not-"for now" endgame for this?
The hardware supports more modes than the two currently supported in the
driver. One of these, called 'PAT_MEM' mode, also shares memory space
with FIFO mode. However, 'PAT_MEM' requires memory to be pre-reserved
and waveform data to be pre-loaded. The entire 8K bytes of memory can be
divided into partitions, and it is configurable, with FIFO mode always
using the first partition [0, fifo_len], where 'fifo_len' is set via the
'MMAP_FIFO_REG' register. 'PAT_MEM' mode plays waveform using data
preloaded in a memory bank defined by the registers
'PATX_MEM_START_ADDR_REG' and 'PATTERN_SPMI_PATX_LEN_REG' (they are not
defined in the driver). Since PAT_MEM is mainly intended for
hardware-triggered vibrations, such as a signal from a dedicated GPIO
triggering a short vibration with a preloaded waveform, and although it
also supports software triggers, I haven't found a suitable way to
support it well into the driver under input FF framework yet. So, I am
currently allocating the entire 8K FIFO memory for FIFO mode only. We
can adjust this later if we find a better way to incorporate 'PAT_MEM'
mode into the driver.
^ permalink raw reply
* RE: [PATCH v32 4/5] i2c: ast2600: Add controller driver for AST2600 new register set
From: Ryan Chen @ 2026-06-17 2:44 UTC (permalink / raw)
To: sashiko-reviews@lists.linux.dev
Cc: conor+dt@kernel.org, devicetree@vger.kernel.org, robh@kernel.org
In-Reply-To: <20260611054624.25BC51F00893@smtp.kernel.org>
Hello,
> Subject: Re: [PATCH v32 4/5] i2c: ast2600: Add controller driver for AST2600
> new register set
>
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to
> consider:
> - [High] The driver fails to send a NAK on the last byte of an intermediate read
> message, causing the slave to hold SDA low and block the subsequent Repeated
> Start.
> - [High] The interrupt handler does not clear error status bits in the
> Write-1-to-Clear (W1C) ISR register, permanently wedging the controller after
> a single error.
> - [Medium] Bus recovery failure and other error interrupts are not enabled in
> the Interrupt Enable Register (IER), leading to needless blocking timeouts.
> --
>
> commit 01c0588191421a92b3d96b7bc246be1fe0958d9f
> Author: Ryan Chen <ryan_chen@aspeedtech.com>
>
> i2c: ast2600: Add controller driver for AST2600 new register set
>
> The AST2600 introduces a new I2C controller register layout, selectable at
> runtime via global control registers. Compared to the legacy layout used on
> AST2400/AST2500, the new layout separates controller (master) and target
> (slave) registers and adds support for packet-based transfers
>
> > diff --git a/drivers/i2c/busses/i2c-ast2600.c
> > b/drivers/i2c/busses/i2c-ast2600.c
> > new file mode 100644
> > --- /dev/null
> > +++ b/drivers/i2c/busses/i2c-ast2600.c
>
> [ ... ]
>
> > +static int ast2600_i2c_setup_buff_rx(u32 cmd, struct ast2600_i2c_bus
> > +*i2c_bus) {
> > + struct i2c_msg *msg = &i2c_bus->msgs[i2c_bus->msgs_index];
> > + int xfer_len = msg->len - i2c_bus->controller_xfer_cnt;
> > +
> > + cmd |= AST2600_I2CM_PKT_EN | AST2600_I2CM_RX_BUFF_EN |
> > +AST2600_I2CM_RX_CMD;
> > +
> > + if (cmd & AST2600_I2CM_START_CMD)
> > + cmd |= AST2600_I2CM_PKT_ADDR(msg->addr);
> > +
> > + if (msg->flags & I2C_M_RECV_LEN) {
> > + dev_dbg(i2c_bus->dev, "smbus read\n");
> > + xfer_len = 1;
> > + } else if (xfer_len > i2c_bus->buf_size) {
> > + xfer_len = i2c_bus->buf_size;
> > + } else if (i2c_bus->msgs_index + 1 == i2c_bus->msgs_count) {
> > + cmd |= CONTROLLER_TRIGGER_LAST_STOP;
> > + }
>
> [Severity: High]
> Does this logic omit the NAK for intermediate read messages?
>
> The CONTROLLER_TRIGGER_LAST_STOP flag (which applies the hardware NAK
> via
> AST2600_I2CM_RX_CMD_LAST) is strictly tied to the absolute final message in
> the array. If the transfer contains an intermediate read message, such as a read
> before a Repeated Start, the last byte will be ACKed.
>
> This will cause the slave to expect another byte and hold SDA low, leading to
> an arbitration loss or bus hang when attempting the Repeated Start.
Will update with following.
} else {
cmd |= AST2600_I2CM_RX_CMD_LAST;
if (i2c_bus->msgs_index + 1 == i2c_bus->msgs_count)
cmd |= AST2600_I2CM_STOP_CMD;
}
>
> [ ... ]
>
> > +static int ast2600_i2c_controller_irq(struct ast2600_i2c_bus
> > +*i2c_bus) {
> > + u32 sts = readl(i2c_bus->reg_base + AST2600_I2CM_ISR);
> > + u32 ctrl;
> > +
> > + sts &= ~AST2600_I2CM_SMBUS_ALERT;
> > +
> > + if (sts & AST2600_I2CM_BUS_RECOVER_FAIL) {
> > + writel(AST2600_I2CM_BUS_RECOVER_FAIL, i2c_bus->reg_base +
> AST2600_I2CM_ISR);
> > + ctrl = readl(i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
> > + writel(0, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
> > + writel(ctrl, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
> > + WRITE_ONCE(i2c_bus->msgs, NULL);
> > + i2c_bus->cmd_err = -EPROTO;
> > + complete(&i2c_bus->cmd_complete);
> > + return 1;
> > + }
> > +
> > + if (sts & AST2600_I2CM_BUS_RECOVER) {
> > + writel(AST2600_I2CM_BUS_RECOVER, i2c_bus->reg_base +
> AST2600_I2CM_ISR);
> > + WRITE_ONCE(i2c_bus->msgs, NULL);
> > + i2c_bus->cmd_err = 0;
> > + complete(&i2c_bus->cmd_complete);
> > + return 1;
> > + }
> > +
> > + i2c_bus->cmd_err = ast2600_i2c_irq_err_to_errno(sts);
> > + if (i2c_bus->cmd_err) {
> > + WRITE_ONCE(i2c_bus->msgs, NULL);
> > + writel(AST2600_I2CM_PKT_DONE, i2c_bus->reg_base +
> AST2600_I2CM_ISR);
> > + complete(&i2c_bus->cmd_complete);
> > + return 1;
> > + }
>
> [Severity: High]
> Will this permanently wedge the controller after a single error?
>
> When an error occurs, the error path explicitly writes only
> AST2600_I2CM_PKT_DONE to the ISR. W1C error bits like
> AST2600_I2CM_ARBIT_LOSS or AST2600_I2CM_TX_NAK are never written
> back and will remain set.
>
> Because the error path calls complete() successfully, the timeout fallback path
> that could reset the controller is bypassed. On the very next I2C transaction, a
> new PKT_DONE interrupt will cause the handler to read the stale error bits,
> instantly fail the transaction, and loop in this state.
>
> The same issue appears to exist in ast2600_i2c_controller_packet_irq() for bits
> like AST2600_I2CM_PKT_ERROR.
In packet mode the error status is not cleared bit-by-bit. Per the
datasheet description of I2CM14[16] (PKT_DONE), clearing PKT_DONE also
clears bit[6:0], bit[15:13] and bit[18:17] — which covers arbitration
loss, the timeout bits, ABNORMAL and PKT_ERROR. Writing back only
PKT_DONE therefore clears all pending error status as well; the
controller is not wedged. Writing the full ISR value back would be
functionally equivalent.
>
> [ ... ]
>
> > +static int ast2600_i2c_probe(struct platform_device *pdev) {
>
> [ ... ]
>
> > + ret = devm_request_irq(dev, i2c_bus->irq, ast2600_i2c_bus_irq, 0,
> > + dev_name(dev), i2c_bus);
> > + if (ret < 0) {
> > + ret = dev_err_probe(dev, ret, "Unable to request irq %d\n",
> > + i2c_bus->irq);
> > + goto err;
> > + }
> > +
> > + writel(AST2600_I2CM_PKT_DONE | AST2600_I2CM_BUS_RECOVER,
> > + i2c_bus->reg_base + AST2600_I2CM_IER);
> > +
> > + ret = i2c_add_adapter(&i2c_bus->adap);
>
> [Severity: Medium]
> Are bus recovery failure interrupts intentionally left disabled?
>
> The interrupt enable register is configured here without including the
> AST2600_I2CM_BUS_RECOVER_FAIL bit. If a bus recovery attempt fails, the
> hardware will assert the bit in the ISR, but the interrupt handler will never be
> triggered.
>
> This will cause the driver to needlessly block for the entire duration of
> wait_for_completion_timeout() instead of handling the failure immediately.
Per the datasheet description of I2CM10[16], in packet mode only bit 16
(PKT_DONE) and optionally bit 12 (SMBus Alert) need to be enabled; the
other enable bits have no effect. All error conditions (arbitration loss,
timeouts, ABNORMAL) are reported through PKT_ERROR folded into PKT_DONE,
so they are delivered without a separate enable and do not rely on the
xfer timeout. In addition, I2CM10[15] (Bus Recover Fail enable) is
RO/Reserved, so it cannot be enabled in the first place.
>
> --
> Sashiko AI review ·
> https://sashiko.dev/#/patchset/20260611-upstream_i2c-v32-0-b66eba921d01
> @aspeedtech.com?part=4
^ permalink raw reply
* [PATCH v4 0/2] pwm: Add Nuvoton MA35D1 PWM controller support
From: Chi-Wen Weng @ 2026-06-17 2:59 UTC (permalink / raw)
To: ukleinek, robh, krzk+dt, conor+dt
Cc: linux-arm-kernel, linux-pwm, devicetree, linux-kernel, cwweng,
cwweng.linux
From: Chi-Wen Weng <cwweng@nuvoton.com>
This series adds support for the Nuvoton MA35D1 PWM controller.
The MA35D1 PWM controller provides 6 PWM channels. The hardware supports
several counter types and output modes. This driver configures the controller
to use up-counting mode, auto-reload mode and independent output mode. The
waveform generator is configured to drive the output high at the zero point
and low at the compare-up point.
For the up-counting mode used by this driver, the counter counts from 0 to
PERIOD inclusive. Therefore, the programmed period is PERIOD + 1 cycles. The
hardware can generate 0% duty cycle with CMPDAT = 0 and 100% duty cycle with
CMPDAT > PERIOD. To keep 100% duty cycle representable, the driver limits the
maximum PERIOD value to 0xfffe and reserves CMPDAT = 0xffff for the full-duty
case.
The hardware buffers PERIOD and CMPDAT updates when IMMLDENn is disabled. The
driver keeps IMMLDENn disabled, so period and duty cycle updates take effect
at the end of the current period. Polarity and waveform-control changes are
applied directly and may cause transient output changes if the PWM is running.
When the PWM output is disabled by clearing POENn, the output pin is put into
tri-state according to the MA35D1 reference manual.
Changes in v4:
- Add a Limitations section to describe the hardware capabilities and driver
limitations.
- Add a link to the MA35D1 reference manual.
- Replace register address macros containing the base pointer with register
offset macros.
- Add readl/writel/rmw helper functions.
- Rename TOTAL_CHANNELS to NUM_CHANNELS.
- Use unsigned long for the cached clock rate.
- Use devm_clk_rate_exclusive_get().
- Configure polarity before enabling the counter and output.
- Add controller initialization for up-counting, auto-reload and independent
output mode.
- Configure the waveform generator for zero-point-high and compare-up-low
output.
- Fix the period conversion because the hardware period is PERIOD + 1 cycles.
- Limit the maximum PERIOD value to 0xfffe so that CMPDAT = 0xffff can be used
to generate 100% duty cycle.
- Use CNTEN and POEN bits to report the enabled state in .get_state().
- Disable the PWM by clearing POENn and CNTENn.
- Fix error message capitalization and trailing newlines.
- Fix coding style issues reported by review/checkpatch-style tools.
Changes in v3:
- Update nuvoton,ma35d1-pwm.yaml
- Add maintainers entry
- Increse "#pwm-cells" to 3
- Update ma35d1 pwm driver
- Make include header and macros definitions organized alphabetically
- Rename macros REG_PWM_XXXX to MA35D1_REG_PWM_XXXX
- Add macros for register address
v2 resend:
- Remove wrong 'Reviewed-by' tags.
Changes in v2:
- Update nuvoton,ma35d1-pwm.yaml
- Fix 'maxItems' of 'reg' to 1.
- Remove unused label
- Update ma35d1 pwm driver
- Remove MODULE_ALIAS()
- Add chip->atomic = true
Chi-Wen Weng (2):
dt-bindings: pwm: Add Nuvoton MA35D1 PWM controller
pwm: Add Nuvoton MA35D1 PWM controller support
.../bindings/pwm/nuvoton,ma35d1-pwm.yaml | 45 +++
drivers/pwm/Kconfig | 9 +
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-ma35d1.c | 344 ++++++++++++++++++
4 files changed, 399 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/nuvoton,ma35d1-pwm.yaml
create mode 100644 drivers/pwm/pwm-ma35d1.c
--
2.25.1
^ permalink raw reply
* [PATCH v4 1/2] dt-bindings: pwm: Add Nuvoton MA35D1 PWM controller
From: Chi-Wen Weng @ 2026-06-17 2:59 UTC (permalink / raw)
To: ukleinek, robh, krzk+dt, conor+dt
Cc: linux-arm-kernel, linux-pwm, devicetree, linux-kernel, cwweng,
cwweng.linux, Krzysztof Kozlowski
In-Reply-To: <20260617025925.2539334-1-cwweng.linux@gmail.com>
From: Chi-Wen Weng <cwweng@nuvoton.com>
Add device tree binding for the Nuvoton MA35D1 PWM controller.
The MA35D1 PWM controller provides 6 PWM channels and uses one register
region and one functional clock. The binding uses the standard PWM binding
with three PWM cells.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Chi-Wen Weng <cwweng@nuvoton.com>
---
.../bindings/pwm/nuvoton,ma35d1-pwm.yaml | 45 +++++++++++++++++++
1 file changed, 45 insertions(+)
create mode 100644 Documentation/devicetree/bindings/pwm/nuvoton,ma35d1-pwm.yaml
diff --git a/Documentation/devicetree/bindings/pwm/nuvoton,ma35d1-pwm.yaml b/Documentation/devicetree/bindings/pwm/nuvoton,ma35d1-pwm.yaml
new file mode 100644
index 000000000000..47a59bdd14d0
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/nuvoton,ma35d1-pwm.yaml
@@ -0,0 +1,45 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/nuvoton,ma35d1-pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nuvoton MA35D1 PWM controller
+
+maintainers:
+ - Chi-Wen Weng <cwweng@nuvoton.com>
+
+allOf:
+ - $ref: pwm.yaml#
+
+properties:
+ compatible:
+ enum:
+ - nuvoton,ma35d1-pwm
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ "#pwm-cells":
+ const: 3
+
+required:
+ - compatible
+ - reg
+ - clocks
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
+
+ pwm@40580000 {
+ compatible = "nuvoton,ma35d1-pwm";
+ reg = <0x40580000 0x400>;
+ clocks = <&clk EPWM0_GATE>;
+ #pwm-cells = <3>;
+ };
--
2.25.1
^ permalink raw reply related
* [PATCH v4 2/2] pwm: Add Nuvoton MA35D1 PWM controller support
From: Chi-Wen Weng @ 2026-06-17 2:59 UTC (permalink / raw)
To: ukleinek, robh, krzk+dt, conor+dt
Cc: linux-arm-kernel, linux-pwm, devicetree, linux-kernel, cwweng,
cwweng.linux, Trevor Gamblin
In-Reply-To: <20260617025925.2539334-1-cwweng.linux@gmail.com>
From: Chi-Wen Weng <cwweng@nuvoton.com>
Add a PWM framework driver for the Nuvoton MA35D1 PWM controller.
The MA35D1 PWM controller provides 6 PWM channels. The hardware supports
up, down and up-down counter types, auto-reload and one-shot modes, and
independent and complementary output modes. This driver configures all
channels to up-counting mode, auto-reload mode and independent output mode.
The waveform generator is configured to drive the output high at the zero
point and low at the compare-up point. In up-counting mode the counter
counts from 0 to PERIOD inclusive, so the PWM period is PERIOD + 1 cycles.
With the selected waveform actions, CMPDAT = 0 generates 0% duty cycle and
CMPDAT > PERIOD generates 100% duty cycle. Limit PERIOD to 0xfffe so that
CMPDAT = 0xffff can be used for the full-duty case.
PERIOD and CMPDAT updates are buffered by the hardware and take effect at
the end of the current period because IMMLDENn is left disabled. When the
PWM output is disabled, POENn is cleared and the output pin is put into
tri-state.
Reviewed-by: Trevor Gamblin <tgamblin@baylibre.com>
Signed-off-by: Chi-Wen Weng <cwweng@nuvoton.com>
---
drivers/pwm/Kconfig | 9 +
drivers/pwm/Makefile | 1 +
drivers/pwm/pwm-ma35d1.c | 344 +++++++++++++++++++++++++++++++++++++++
3 files changed, 354 insertions(+)
create mode 100644 drivers/pwm/pwm-ma35d1.c
diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
index e8886a9b64d9..355131e6efac 100644
--- a/drivers/pwm/Kconfig
+++ b/drivers/pwm/Kconfig
@@ -463,6 +463,15 @@ config PWM_LPSS_PLATFORM
To compile this driver as a module, choose M here: the module
will be called pwm-lpss-platform.
+config PWM_MA35D1
+ tristate "Nuvoton MA35D1 PWM support"
+ depends on ARCH_MA35 || COMPILE_TEST
+ help
+ Generic PWM framework driver for Nuvoton MA35D1.
+
+ To compile this driver as a module, choose M here: the module
+ will be called pwm-ma35d1.
+
config PWM_MAX7360
tristate "MAX7360 PWMs"
depends on MFD_MAX7360
diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
index 5630a521a7cf..7ad761ea27d1 100644
--- a/drivers/pwm/Makefile
+++ b/drivers/pwm/Makefile
@@ -40,6 +40,7 @@ obj-$(CONFIG_PWM_LPC32XX) += pwm-lpc32xx.o
obj-$(CONFIG_PWM_LPSS) += pwm-lpss.o
obj-$(CONFIG_PWM_LPSS_PCI) += pwm-lpss-pci.o
obj-$(CONFIG_PWM_LPSS_PLATFORM) += pwm-lpss-platform.o
+obj-$(CONFIG_PWM_MA35D1) += pwm-ma35d1.o
obj-$(CONFIG_PWM_MAX7360) += pwm-max7360.o
obj-$(CONFIG_PWM_MC33XS2410) += pwm-mc33xs2410.o
obj-$(CONFIG_PWM_MEDIATEK) += pwm-mediatek.o
diff --git a/drivers/pwm/pwm-ma35d1.c b/drivers/pwm/pwm-ma35d1.c
new file mode 100644
index 000000000000..c07eedeca035
--- /dev/null
+++ b/drivers/pwm/pwm-ma35d1.c
@@ -0,0 +1,344 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Driver for the Nuvoton MA35D1 PWM controller
+ *
+ * Copyright (C) 2026 Nuvoton Corporation
+ * Chi-Wen Weng <cwweng@nuvoton.com>
+ *
+ * Reference Manual:
+ * https://www.nuvoton.com.cn/resource-download.jsp?tp_GUID=DA05-MA35D16
+ *
+ * Limitations:
+ * - The hardware supports 6 PWM channels.
+ * - The hardware supports up, down and up-down counter types. This driver
+ * configures all channels to up-counting mode.
+ * - The hardware supports auto-reload and one-shot counter modes. This driver
+ * configures all channels to auto-reload mode.
+ * - The hardware supports independent and complementary output modes. This
+ * driver configures all channels to independent output mode.
+ * - The hardware supports programmable waveform actions at zero, period and
+ * compare points. This driver uses zero point high and compare-up point low
+ * actions for normal PWM output.
+ * - In up-counting mode, the counter counts from 0 to PERIOD inclusive. With
+ * zero point high and compare-up point low actions, CMPDAT = 0 produces 0%
+ * duty and CMPDAT > PERIOD produces 100% duty.
+ * - The driver limits PERIOD to 0xfffe so that CMPDAT can be set greater than
+ * PERIOD to generate a 100% duty cycle.
+ * - Period and duty cycle changes are buffered by hardware and take effect at
+ * the end of the current period because IMMLDENn is left disabled.
+ * - Polarity changes are applied directly and may cause a transient output
+ * change if the PWM output is running.
+ * - When disabled, the output pin is put in tri-state by clearing POENn.
+ */
+
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/io.h>
+#include <linux/math64.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pwm.h>
+
+#define MA35D1_REG_PWM_CTL0 0x00
+#define MA35D1_REG_PWM_CTL1 0x04
+#define MA35D1_REG_PWM_CNTEN 0x20
+#define MA35D1_REG_PWM_PERIOD(ch) (0x30 + 4 * (ch))
+#define MA35D1_REG_PWM_CMPDAT(ch) (0x50 + 4 * (ch))
+#define MA35D1_REG_PWM_WGCTL0 0xb0
+#define MA35D1_REG_PWM_WGCTL1 0xb4
+#define MA35D1_REG_PWM_POLCTL 0xd4
+#define MA35D1_REG_PWM_POEN 0xd8
+
+#define MA35D1_PWM_CTL1_CNTMODE_MASK(ch) BIT(16 + (ch))
+#define MA35D1_PWM_CTL1_OUTMODE_MASK(ch) BIT(24 + ((ch) / 2))
+
+#define MA35D1_PWM_WGCTL_ACTION_MASK 0x3
+#define MA35D1_PWM_WGCTL_ACTION_LOW 1
+#define MA35D1_PWM_WGCTL_ACTION_HIGH 2
+
+#define MA35D1_PWM_WGCTL_ZERO_HIGH(ch) \
+ (MA35D1_PWM_WGCTL_ACTION_HIGH << (2 * (ch)))
+#define MA35D1_PWM_WGCTL_CMP_UP_LOW(ch) \
+ (MA35D1_PWM_WGCTL_ACTION_LOW << (2 * (ch)))
+
+#define MA35D1_PWM_CNTEN_EN(ch) BIT(ch)
+#define MA35D1_PWM_POEN_EN(ch) BIT(ch)
+#define MA35D1_PWM_POLCTL_INV(ch) BIT(ch)
+
+#define MA35D1_PWM_MAX_CMPDAT 0xffff
+#define MA35D1_PWM_MAX_PERIOD 0xfffe
+#define MA35D1_PWM_MAX_PERIOD_CYCLES (MA35D1_PWM_MAX_PERIOD + 1)
+#define MA35D1_PWM_NUM_CHANNELS 6
+
+struct nuvoton_pwm {
+ void __iomem *base;
+ unsigned long clkrate;
+};
+
+static inline struct nuvoton_pwm *nuvoton_pwm_from_chip(struct pwm_chip *chip)
+{
+ return pwmchip_get_drvdata(chip);
+}
+
+static inline u32 nuvoton_pwm_ctl1_cnttype_mask(unsigned int ch)
+{
+ return MA35D1_PWM_WGCTL_ACTION_MASK << (2 * ch);
+}
+
+static inline u32 nuvoton_pwm_wgctl_zero_mask(unsigned int ch)
+{
+ return MA35D1_PWM_WGCTL_ACTION_MASK << (2 * ch);
+}
+
+static inline u32 nuvoton_pwm_wgctl_period_mask(unsigned int ch)
+{
+ return MA35D1_PWM_WGCTL_ACTION_MASK << (16 + 2 * ch);
+}
+
+static inline u32 nuvoton_pwm_wgctl_cmp_up_mask(unsigned int ch)
+{
+ return MA35D1_PWM_WGCTL_ACTION_MASK << (2 * ch);
+}
+
+static inline u32 nuvoton_pwm_wgctl_cmp_down_mask(unsigned int ch)
+{
+ return MA35D1_PWM_WGCTL_ACTION_MASK << (16 + 2 * ch);
+}
+
+static inline u32 nuvoton_pwm_readl(struct nuvoton_pwm *nvtpwm,
+ unsigned int offset)
+{
+ return readl(nvtpwm->base + offset);
+}
+
+static inline void nuvoton_pwm_writel(struct nuvoton_pwm *nvtpwm,
+ unsigned int offset, u32 value)
+{
+ writel(value, nvtpwm->base + offset);
+}
+
+static inline void nuvoton_pwm_rmw(struct nuvoton_pwm *nvtpwm,
+ unsigned int offset, u32 mask, u32 value)
+{
+ u32 reg;
+
+ reg = nuvoton_pwm_readl(nvtpwm, offset);
+ reg &= ~mask;
+ reg |= value & mask;
+ nuvoton_pwm_writel(nvtpwm, offset, reg);
+}
+
+static void nuvoton_pwm_init(struct nuvoton_pwm *nvtpwm)
+{
+ u32 ctl1_mask = 0;
+ u32 wgctl0_mask = 0;
+ u32 wgctl0_val = 0;
+ u32 wgctl1_mask = 0;
+ u32 wgctl1_val = 0;
+ int ch;
+
+ for (ch = 0; ch < MA35D1_PWM_NUM_CHANNELS; ch++) {
+ /* CNTTYPEn = 00: up counter type */
+ ctl1_mask |= nuvoton_pwm_ctl1_cnttype_mask(ch);
+
+ /* CNTMODEn = 0: auto-reload mode */
+ ctl1_mask |= MA35D1_PWM_CTL1_CNTMODE_MASK(ch);
+
+ /* ZPCTLn = 10: output high at zero point */
+ wgctl0_mask |= nuvoton_pwm_wgctl_zero_mask(ch);
+ wgctl0_val |= MA35D1_PWM_WGCTL_ZERO_HIGH(ch);
+
+ /* PRDPCTLn = 00: do nothing at period point */
+ wgctl0_mask |= nuvoton_pwm_wgctl_period_mask(ch);
+
+ /* CMPUCTLn = 01: output low at compare up point */
+ wgctl1_mask |= nuvoton_pwm_wgctl_cmp_up_mask(ch);
+ wgctl1_val |= MA35D1_PWM_WGCTL_CMP_UP_LOW(ch);
+
+ /* CMPDCTLn = 00: do nothing at compare down point */
+ wgctl1_mask |= nuvoton_pwm_wgctl_cmp_down_mask(ch);
+ }
+
+ for (ch = 0; ch < MA35D1_PWM_NUM_CHANNELS; ch += 2) {
+ /* OUTMODEn = 0: independent mode */
+ ctl1_mask |= MA35D1_PWM_CTL1_OUTMODE_MASK(ch);
+ }
+
+ nuvoton_pwm_rmw(nvtpwm, MA35D1_REG_PWM_CTL1, ctl1_mask, 0);
+ nuvoton_pwm_rmw(nvtpwm, MA35D1_REG_PWM_WGCTL0,
+ wgctl0_mask, wgctl0_val);
+ nuvoton_pwm_rmw(nvtpwm, MA35D1_REG_PWM_WGCTL1,
+ wgctl1_mask, wgctl1_val);
+}
+
+static int nuvoton_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
+ const struct pwm_state *state)
+{
+ struct nuvoton_pwm *nvtpwm = nuvoton_pwm_from_chip(chip);
+ u32 ch = pwm->hwpwm;
+ u64 duty_cycles, period_cycles;
+ u32 cmpdat, period;
+
+ if (!state->enabled) {
+ nuvoton_pwm_rmw(nvtpwm, MA35D1_REG_PWM_POEN,
+ MA35D1_PWM_POEN_EN(ch), 0);
+ nuvoton_pwm_rmw(nvtpwm, MA35D1_REG_PWM_CNTEN,
+ MA35D1_PWM_CNTEN_EN(ch), 0);
+
+ return 0;
+ }
+
+ period_cycles = mul_u64_u64_div_u64(nvtpwm->clkrate,
+ state->period,
+ NSEC_PER_SEC);
+ if (!period_cycles)
+ return -EINVAL;
+
+ if (period_cycles > MA35D1_PWM_MAX_PERIOD_CYCLES)
+ period_cycles = MA35D1_PWM_MAX_PERIOD_CYCLES;
+
+ duty_cycles = mul_u64_u64_div_u64(nvtpwm->clkrate,
+ state->duty_cycle,
+ NSEC_PER_SEC);
+ if (duty_cycles > period_cycles)
+ duty_cycles = period_cycles;
+
+ if (state->polarity == PWM_POLARITY_NORMAL)
+ nuvoton_pwm_rmw(nvtpwm, MA35D1_REG_PWM_POLCTL,
+ MA35D1_PWM_POLCTL_INV(ch), 0);
+ else
+ nuvoton_pwm_rmw(nvtpwm, MA35D1_REG_PWM_POLCTL,
+ MA35D1_PWM_POLCTL_INV(ch),
+ MA35D1_PWM_POLCTL_INV(ch));
+
+ /*
+ * In up-counting mode the counter counts from 0 to PERIOD inclusive.
+ * With zero point high and compare-up point low actions:
+ * - CMPDAT = 0 produces 0% duty.
+ * - CMPDAT > PERIOD produces 100% duty.
+ * PERIOD is limited to 0xfffe, so duty_cycles can be written directly
+ * to CMPDAT and still fit in the 16-bit compare field for 100% duty.
+ */
+ period = period_cycles - 1;
+ cmpdat = duty_cycles;
+
+ nuvoton_pwm_writel(nvtpwm, MA35D1_REG_PWM_PERIOD(ch), period);
+ nuvoton_pwm_writel(nvtpwm, MA35D1_REG_PWM_CMPDAT(ch), cmpdat);
+
+ nuvoton_pwm_rmw(nvtpwm, MA35D1_REG_PWM_CNTEN,
+ MA35D1_PWM_CNTEN_EN(ch), MA35D1_PWM_CNTEN_EN(ch));
+ nuvoton_pwm_rmw(nvtpwm, MA35D1_REG_PWM_POEN,
+ MA35D1_PWM_POEN_EN(ch), MA35D1_PWM_POEN_EN(ch));
+
+ return 0;
+}
+
+static int nuvoton_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
+ struct pwm_state *state)
+{
+ struct nuvoton_pwm *nvtpwm = nuvoton_pwm_from_chip(chip);
+ u32 ch = pwm->hwpwm;
+ u32 cmpdat, cnten, period, poen, polctl;
+ u64 duty_cycles, period_cycles;
+
+ cnten = nuvoton_pwm_readl(nvtpwm, MA35D1_REG_PWM_CNTEN);
+ poen = nuvoton_pwm_readl(nvtpwm, MA35D1_REG_PWM_POEN);
+ polctl = nuvoton_pwm_readl(nvtpwm, MA35D1_REG_PWM_POLCTL);
+ period = nuvoton_pwm_readl(nvtpwm, MA35D1_REG_PWM_PERIOD(ch)) &
+ MA35D1_PWM_MAX_CMPDAT;
+ cmpdat = nuvoton_pwm_readl(nvtpwm, MA35D1_REG_PWM_CMPDAT(ch)) &
+ MA35D1_PWM_MAX_CMPDAT;
+
+ period_cycles = period + 1;
+ if (cmpdat > period)
+ duty_cycles = period_cycles;
+ else
+ duty_cycles = cmpdat;
+
+ state->enabled = (cnten & MA35D1_PWM_CNTEN_EN(ch)) &&
+ (poen & MA35D1_PWM_POEN_EN(ch));
+ state->polarity = (polctl & MA35D1_PWM_POLCTL_INV(ch)) ?
+ PWM_POLARITY_INVERSED : PWM_POLARITY_NORMAL;
+ state->period = DIV64_U64_ROUND_UP(period_cycles * NSEC_PER_SEC,
+ nvtpwm->clkrate);
+ state->duty_cycle = DIV64_U64_ROUND_UP(duty_cycles * NSEC_PER_SEC,
+ nvtpwm->clkrate);
+
+ return 0;
+}
+
+static const struct pwm_ops nuvoton_pwm_ops = {
+ .apply = nuvoton_pwm_apply,
+ .get_state = nuvoton_pwm_get_state,
+};
+
+static int nuvoton_pwm_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct pwm_chip *chip;
+ struct nuvoton_pwm *nvtpwm;
+ struct clk *clk;
+ int ret;
+
+ chip = devm_pwmchip_alloc(dev, MA35D1_PWM_NUM_CHANNELS,
+ sizeof(*nvtpwm));
+ if (IS_ERR(chip))
+ return PTR_ERR(chip);
+
+ nvtpwm = nuvoton_pwm_from_chip(chip);
+
+ nvtpwm->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(nvtpwm->base))
+ return PTR_ERR(nvtpwm->base);
+
+ clk = devm_clk_get_enabled(dev, NULL);
+ if (IS_ERR(clk))
+ return dev_err_probe(dev, PTR_ERR(clk),
+ "Unable to get the clock\n");
+
+ ret = devm_clk_rate_exclusive_get(dev, clk);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "Unable to get exclusive clock rate\n");
+
+ nvtpwm->clkrate = clk_get_rate(clk);
+ if (!nvtpwm->clkrate)
+ return dev_err_probe(dev, -EINVAL,
+ "PWM clock rate is zero\n");
+
+ if (nvtpwm->clkrate > NSEC_PER_SEC)
+ return dev_err_probe(dev, -EINVAL,
+ "PWM clock out of range (%lu)\n",
+ nvtpwm->clkrate);
+
+ nuvoton_pwm_init(nvtpwm);
+
+ chip->ops = &nuvoton_pwm_ops;
+ chip->atomic = true;
+
+ ret = devm_pwmchip_add(dev, chip);
+ if (ret)
+ return dev_err_probe(dev, ret, "Unable to add PWM chip\n");
+
+ return 0;
+}
+
+static const struct of_device_id nuvoton_pwm_of_match[] = {
+ { .compatible = "nuvoton,ma35d1-pwm" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, nuvoton_pwm_of_match);
+
+static struct platform_driver nuvoton_pwm_driver = {
+ .probe = nuvoton_pwm_probe,
+ .driver = {
+ .name = "nuvoton-pwm",
+ .of_match_table = nuvoton_pwm_of_match,
+ },
+};
+module_platform_driver(nuvoton_pwm_driver);
+
+MODULE_AUTHOR("Chi-Wen Weng <cwweng@nuvoton.com>");
+MODULE_DESCRIPTION("Nuvoton MA35D1 PWM driver");
+MODULE_LICENSE("GPL");
--
2.25.1
^ permalink raw reply related
* [PATCH 1/3] PCI: rcar-gen4: Configure AXIINTC if iMSI-RX not used
From: Marek Vasut @ 2026-06-17 2:59 UTC (permalink / raw)
To: linux-pci
Cc: Marek Vasut, Yoshihiro Shimoda, Krzysztof Wilczyński,
Bjorn Helgaas, Catalin Marinas, Conor Dooley, Geert Uytterhoeven,
Krzysztof Kozlowski, Lorenzo Pieralisi, Manivannan Sadhasivam,
Marc Zyngier, Rob Herring, devicetree, linux-arm-kernel,
linux-doc, linux-kernel, linux-renesas-soc
In case MSI are enabled, but DWC built-in iMSI-RX is not in use, the
MSI are handled via GIC ITS. Configure all controller MSI registers
fully.
Set or clear MSI capability register MSICAP0 MSI enable MSIE bit and
PCIe Interrupt Status 0 Enable register PCIEINTSTS0EN MSI interrupt
enable MSI_CTRL_INT bit according to MSI enable state, set both bits
if MSI are enabled, clear both bits if MSI are disabled.
If MSI are disabled, or MSI are enabled and iMSI-RX is used, then
deconfigure AXIINTCADDR and AXIINTCCONT to 0, which disables any
pass through of MSI TLPs onto the AXI bus and then further into
GIC ITS translation registers.
If MSI are enabled and iMSI-RX is not used, the configure AXIINTCADDR
with target address of GIC ITS translation registers, and configure
AXIINTCCONT to enable MSI TLP pass through onto AXI bus and into the
GIC ITS. This specific configuration allows handling of MSI via the
GIC ITS instead of integrated iMSI-RX.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
NOTE: This would not be possible without prior work from Shimoda-san
---
Cc: "Krzysztof Wilczyński" <kwilczynski@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
---
drivers/pci/controller/dwc/pcie-rcar-gen4.c | 53 +++++++++++++++++++--
1 file changed, 48 insertions(+), 5 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
index 485cfa8bd9692..ba6e3bedd6d0a 100644
--- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
+++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
@@ -31,6 +31,10 @@
#define DEVICE_TYPE_RC BIT(4)
#define BIFUR_MOD_SET_ON BIT(0)
+/* MSI Capability */
+#define MSICAP0 0x0050
+#define MSICAP0_MSIE BIT(16)
+
/* PCIe Interrupt Status 0 */
#define PCIEINTSTS0 0x0084
@@ -55,6 +59,16 @@
#define APP_HOLD_PHY_RST BIT(16)
#define APP_LTSSM_ENABLE BIT(0)
+/* INTC address */
+#define AXIINTCADDR 0x0a00
+/* GITS GIC ITS translation register */
+#define AXIINTCADDR_VAL 0xf1050000
+
+/* INTC control & mask */
+#define AXIINTCCONT 0x0a04
+#define INTC_EN BIT(31)
+#define INTC_MASK GENMASK(11, 2)
+
/* PCIe Power Management Control */
#define PCIEPWRMNGCTRL 0x0070
#define APP_CLK_REQ_N BIT(11)
@@ -305,6 +319,39 @@ static struct rcar_gen4_pcie *rcar_gen4_pcie_alloc(struct platform_device *pdev)
return rcar;
}
+static void rcar_gen4_pcie_host_msi_init(struct dw_pcie_rp *pp)
+{
+ struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
+ struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
+ u32 val;
+
+ /* Make sure MSICAP0 MSIE is configured. */
+ val = dw_pcie_readl_dbi(dw, MSICAP0);
+ if (pci_msi_enabled())
+ val |= MSICAP0_MSIE;
+ else
+ val &= ~MSICAP0_MSIE;
+ dw_pcie_writel_dbi(dw, MSICAP0, val);
+
+ if (!pci_msi_enabled() || pp->use_imsi_rx) {
+ /* Clear AXIINTC mapping. */
+ writel(0, rcar->base + AXIINTCADDR);
+ writel(0, rcar->base + AXIINTCCONT);
+ } else {
+ /* Point AXIINTC to GIC ITS and enable. */
+ writel(AXIINTCADDR_VAL, rcar->base + AXIINTCADDR);
+ writel(INTC_EN | INTC_MASK, rcar->base + AXIINTCCONT);
+ }
+
+ /* Configure MSI interrupt signal */
+ val = readl(rcar->base + PCIEINTSTS0EN);
+ if (pci_msi_enabled())
+ val |= MSI_CTRL_INT;
+ else
+ val &= ~MSI_CTRL_INT;
+ writel(val, rcar->base + PCIEINTSTS0EN);
+}
+
static int rcar_gen4_pcie_enable_device(struct pci_host_bridge *bridge,
struct pci_dev *dev)
{
@@ -359,7 +406,6 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
struct dw_pcie *dw = to_dw_pcie_from_pp(pp);
struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
int ret;
- u32 val;
if (pp->bridge)
pp->bridge->enable_device = rcar_gen4_pcie_enable_device;
@@ -379,10 +425,7 @@ static int rcar_gen4_pcie_host_init(struct dw_pcie_rp *pp)
dw_pcie_writel_dbi2(dw, PCI_BASE_ADDRESS_0, 0x0);
dw_pcie_writel_dbi2(dw, PCI_BASE_ADDRESS_1, 0x0);
- /* Enable MSI interrupt signal */
- val = readl(rcar->base + PCIEINTSTS0EN);
- val |= MSI_CTRL_INT;
- writel(val, rcar->base + PCIEINTSTS0EN);
+ rcar_gen4_pcie_host_msi_init(pp);
msleep(PCIE_T_PVPERL_MS); /* pe_rst requires 100msec delay */
--
2.53.0
^ permalink raw reply related
* [PATCH 2/3] irqchip/gic-v3: Add Renesas R-Car Gen4 erratum workaround
From: Marek Vasut @ 2026-06-17 2:59 UTC (permalink / raw)
To: linux-pci
Cc: Marek Vasut, Yoshihiro Shimoda, Krzysztof Wilczyński,
Bjorn Helgaas, Catalin Marinas, Conor Dooley, Geert Uytterhoeven,
Krzysztof Kozlowski, Lorenzo Pieralisi, Manivannan Sadhasivam,
Marc Zyngier, Rob Herring, devicetree, linux-arm-kernel,
linux-doc, linux-kernel, linux-renesas-soc
In-Reply-To: <20260617030008.154449-1-marek.vasut+renesas@mailbox.org>
Renesas R-Car S4/V4H/V4M GIC600 integration has address width for AXI
or APB interface configured to 32 bit, it can therefore access only
the first 4 GiB of physical address space. This information comes from
R-Car V4H Interface Specification sheet, there is currently no technical
update number assigned to this limitation. Further input from hardware
engineer indicates that this limitation also applies to R-Car S4 and V4M.
Name the limitation GEN4GICITS1, and add a driver quirk to mitigate this
limitation.
Note that the 0x0201743b GIC600 ID is not Renesas-specific, it is
common for many ARM GICv3 implementations. Therefore, add an extra
of_machine_is_compatible() check.
The GIC600 implementation in R-Car S4/V4H/V4M is r1p6.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
NOTE: This would not be possible without prior work from Shimoda-san
https://lore.kernel.org/all/20240214052050.1966439-1-yoshihiro.shimoda.uh@renesas.com/
---
Cc: "Krzysztof Wilczyński" <kwilczynski@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
---
Documentation/arch/arm64/silicon-errata.rst | 1 +
arch/arm64/Kconfig | 9 +++++++++
drivers/irqchip/irq-gic-v3-its.c | 20 ++++++++++++++++++++
3 files changed, 30 insertions(+)
diff --git a/Documentation/arch/arm64/silicon-errata.rst b/Documentation/arch/arm64/silicon-errata.rst
index 014aa1c215a16..b0c68b64f5ac2 100644
--- a/Documentation/arch/arm64/silicon-errata.rst
+++ b/Documentation/arch/arm64/silicon-errata.rst
@@ -352,6 +352,7 @@ stable kernels.
+----------------+-----------------+-----------------+-----------------------------+
| Qualcomm Tech. | Kryo4xx Gold | N/A | ARM64_ERRATUM_1286807 |
+----------------+-----------------+-----------------+-----------------------------+
+| Renesas | S4/V4H/V4M | N/A | RENESAS_ERRATUM_GEN4GICITS1 |
+----------------+-----------------+-----------------+-----------------------------+
| Rockchip | RK3588 | #3588001 | ROCKCHIP_ERRATUM_3588001 |
+----------------+-----------------+-----------------+-----------------------------+
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index b3afe0688919b..b9e17ce475e61 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1382,6 +1382,15 @@ config NVIDIA_CARMEL_CNP_ERRATUM
If unsure, say Y.
+config RENESAS_ERRATUM_GEN4GICITS1
+ bool "Renesas R-Car Gen4: GIC600 can not access physical addresses above 4 GiB"
+ default y
+ help
+ The Renesas R-Car Gen4 S4/V4H/V4M GIC600 SoC integrations have AXI
+ addressing limited to the first 32-bit of physical address space.
+
+ If unsure, say Y.
+
config ROCKCHIP_ERRATUM_3568002
bool "Rockchip 3568002: GIC600 can not access physical addresses higher than 4GB"
default y
diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index b57d81ad33a0a..ec3756f29cf1a 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -4901,6 +4901,18 @@ static bool __maybe_unused its_enable_rk3568002(void *data)
return true;
}
+static bool __maybe_unused its_enable_renesas_gen4(void *data)
+{
+ if (!of_machine_is_compatible("renesas,r8a779f0") &&
+ !of_machine_is_compatible("renesas,r8a779g0") &&
+ !of_machine_is_compatible("renesas,r8a779h0"))
+ return false;
+
+ gfp_flags_quirk |= GFP_DMA32;
+
+ return true;
+}
+
static const struct gic_quirk its_quirks[] = {
#ifdef CONFIG_CAVIUM_ERRATUM_22375
{
@@ -4975,6 +4987,14 @@ static const struct gic_quirk its_quirks[] = {
.mask = 0xffffffff,
.init = its_enable_rk3568002,
},
+#endif
+#ifdef CONFIG_RENESAS_ERRATUM_GEN4GICITS1
+ {
+ .desc = "ITS: Renesas R-Car Gen4 GIC600 32-bit limit",
+ .iidr = 0x0201743b,
+ .mask = 0xffffffff,
+ .init = its_enable_renesas_gen4,
+ },
#endif
{
}
--
2.53.0
^ permalink raw reply related
* [PATCH 3/3] arm64: dts: renesas: r8a779g0: Add GICv3 ITS and update PCIe nodes
From: Marek Vasut @ 2026-06-17 2:59 UTC (permalink / raw)
To: linux-pci
Cc: Marek Vasut, Yoshihiro Shimoda, Krzysztof Wilczyński,
Bjorn Helgaas, Catalin Marinas, Conor Dooley, Geert Uytterhoeven,
Krzysztof Kozlowski, Lorenzo Pieralisi, Manivannan Sadhasivam,
Marc Zyngier, Rob Herring, devicetree, linux-arm-kernel,
linux-doc, linux-kernel, linux-renesas-soc
In-Reply-To: <20260617030008.154449-1-marek.vasut+renesas@mailbox.org>
This SoC implements GIC600 with GICv3 ITS and PCIe host mode on this
SoC can use it. Add GIC ITS node into GIC node, update interrupt-map
and add msi-map into PCIe controller node.
The GIC ITS does have master interface to issue transactions to RAM.
The interface does support cacheable transactions, however, it does
not support shareable attribute, because the AXI port signals are tied
to inactive in this implementation. Therefore, add "dma-noncoherent"
DT property into the GIC ITS subnode.
The GIC redistributor does not have cacheable/shareable, therefore
add "dma-noncoherent" DT property into the GIC node.
Signed-off-by: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
---
NOTE: This would not be possible without prior work from Shimoda-san
https://lore.kernel.org/all/20240214052144.1966569-1-yoshihiro.shimoda.uh@renesas.com/
---
Cc: "Krzysztof Wilczyński" <kwilczynski@kernel.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Conor Dooley <conor+dt@kernel.org>
Cc: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Manivannan Sadhasivam <mani@kernel.org>
Cc: Marc Zyngier <maz@kernel.org>
Cc: Rob Herring <robh@kernel.org>
Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: linux-renesas-soc@vger.kernel.org
---
arch/arm64/boot/dts/renesas/r8a779g0.dtsi | 31 ++++++++++++++++-------
1 file changed, 22 insertions(+), 9 deletions(-)
diff --git a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
index 3a8af825bb253..82e864acf2601 100644
--- a/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
+++ b/arch/arm64/boot/dts/renesas/r8a779g0.dtsi
@@ -792,6 +792,7 @@ pciec0: pcie@e65d0000 {
resets = <&cpg 624>;
reset-names = "pwr";
max-link-speed = <4>;
+ msi-parent = <&its>;
num-lanes = <2>;
#address-cells = <3>;
#size-cells = <2>;
@@ -802,10 +803,10 @@ pciec0: pcie@e65d0000 {
dma-ranges = <0x42000000 0 0x00000000 0 0x00000000 1 0x00000000>;
#interrupt-cells = <1>;
interrupt-map-mask = <0 0 0 7>;
- interrupt-map = <0 0 0 1 &gic GIC_SPI 449 IRQ_TYPE_LEVEL_HIGH>,
- <0 0 0 2 &gic GIC_SPI 449 IRQ_TYPE_LEVEL_HIGH>,
- <0 0 0 3 &gic GIC_SPI 449 IRQ_TYPE_LEVEL_HIGH>,
- <0 0 0 4 &gic GIC_SPI 449 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-map = <0 0 0 1 &gic 0 0 GIC_SPI 449 IRQ_TYPE_LEVEL_HIGH>,
+ <0 0 0 2 &gic 0 0 GIC_SPI 449 IRQ_TYPE_LEVEL_HIGH>,
+ <0 0 0 3 &gic 0 0 GIC_SPI 449 IRQ_TYPE_LEVEL_HIGH>,
+ <0 0 0 4 &gic 0 0 GIC_SPI 449 IRQ_TYPE_LEVEL_HIGH>;
snps,enable-cdm-check;
status = "disabled";
@@ -839,6 +840,7 @@ pciec1: pcie@e65d8000 {
resets = <&cpg 625>;
reset-names = "pwr";
max-link-speed = <4>;
+ msi-parent = <&its>;
num-lanes = <2>;
#address-cells = <3>;
#size-cells = <2>;
@@ -849,10 +851,10 @@ pciec1: pcie@e65d8000 {
dma-ranges = <0x42000000 0 0x00000000 0 0x00000000 1 0x00000000>;
#interrupt-cells = <1>;
interrupt-map-mask = <0 0 0 7>;
- interrupt-map = <0 0 0 1 &gic GIC_SPI 456 IRQ_TYPE_LEVEL_HIGH>,
- <0 0 0 2 &gic GIC_SPI 456 IRQ_TYPE_LEVEL_HIGH>,
- <0 0 0 3 &gic GIC_SPI 456 IRQ_TYPE_LEVEL_HIGH>,
- <0 0 0 4 &gic GIC_SPI 456 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-map = <0 0 0 1 &gic 0 0 GIC_SPI 456 IRQ_TYPE_LEVEL_HIGH>,
+ <0 0 0 2 &gic 0 0 GIC_SPI 456 IRQ_TYPE_LEVEL_HIGH>,
+ <0 0 0 3 &gic 0 0 GIC_SPI 456 IRQ_TYPE_LEVEL_HIGH>,
+ <0 0 0 4 &gic 0 0 GIC_SPI 456 IRQ_TYPE_LEVEL_HIGH>;
snps,enable-cdm-check;
status = "disabled";
@@ -2131,11 +2133,22 @@ ipmmu_mm: iommu@eefc0000 {
gic: interrupt-controller@f1000000 {
compatible = "arm,gic-v3";
#interrupt-cells = <3>;
- #address-cells = <0>;
+ #address-cells = <2>;
+ #size-cells = <2>;
interrupt-controller;
reg = <0x0 0xf1000000 0 0x20000>,
<0x0 0xf1060000 0 0x110000>;
interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>;
+ dma-noncoherent;
+
+ ranges = <0x0 0x0 0x0 0xf1000000 0x0 0x200000>;
+
+ its: msi-controller@40000 {
+ compatible = "arm,gic-v3-its";
+ reg = <0x0 0x40000 0x0 0x20000>;
+ dma-noncoherent;
+ msi-controller;
+ };
};
gpu: gpu@fd000000 {
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v4 2/2] pwm: Add Nuvoton MA35D1 PWM controller support
From: sashiko-bot @ 2026-06-17 3:09 UTC (permalink / raw)
To: Chi-Wen Weng; +Cc: robh, devicetree, conor+dt
In-Reply-To: <20260617025925.2539334-3-cwweng.linux@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The driver unconditionally clobbers PWM waveform generator and control registers in probe(), breaking seamless handover for active PWM channels.
--
commit 1c5d12cc5f94bf9ab4e36bc34c306ce883661f7e
Author: Chi-Wen Weng <cwweng@nuvoton.com>
pwm: Add Nuvoton MA35D1 PWM controller support
Add a PWM framework driver for the Nuvoton MA35D1 PWM controller.
> diff --git a/drivers/pwm/pwm-ma35d1.c b/drivers/pwm/pwm-ma35d1.c
> new file mode 100644
> index 0000000000000..c07eedeca035b
> --- /dev/null
> +++ b/drivers/pwm/pwm-ma35d1.c
[ ... ]
> +static void nuvoton_pwm_init(struct nuvoton_pwm *nvtpwm)
> +{
> + u32 ctl1_mask = 0;
> + u32 wgctl0_mask = 0;
> + u32 wgctl0_val = 0;
> + u32 wgctl1_mask = 0;
> + u32 wgctl1_val = 0;
> + int ch;
> +
> + for (ch = 0; ch < MA35D1_PWM_NUM_CHANNELS; ch++) {
> + /* CNTTYPEn = 00: up counter type */
> + ctl1_mask |= nuvoton_pwm_ctl1_cnttype_mask(ch);
> +
> + /* CNTMODEn = 0: auto-reload mode */
> + ctl1_mask |= MA35D1_PWM_CTL1_CNTMODE_MASK(ch);
> +
> + /* ZPCTLn = 10: output high at zero point */
> + wgctl0_mask |= nuvoton_pwm_wgctl_zero_mask(ch);
> + wgctl0_val |= MA35D1_PWM_WGCTL_ZERO_HIGH(ch);
> +
> + /* PRDPCTLn = 00: do nothing at period point */
> + wgctl0_mask |= nuvoton_pwm_wgctl_period_mask(ch);
> +
> + /* CMPUCTLn = 01: output low at compare up point */
> + wgctl1_mask |= nuvoton_pwm_wgctl_cmp_up_mask(ch);
> + wgctl1_val |= MA35D1_PWM_WGCTL_CMP_UP_LOW(ch);
> +
> + /* CMPDCTLn = 00: do nothing at compare down point */
> + wgctl1_mask |= nuvoton_pwm_wgctl_cmp_down_mask(ch);
> + }
> +
> + for (ch = 0; ch < MA35D1_PWM_NUM_CHANNELS; ch += 2) {
> + /* OUTMODEn = 0: independent mode */
> + ctl1_mask |= MA35D1_PWM_CTL1_OUTMODE_MASK(ch);
> + }
> +
> + nuvoton_pwm_rmw(nvtpwm, MA35D1_REG_PWM_CTL1, ctl1_mask, 0);
> + nuvoton_pwm_rmw(nvtpwm, MA35D1_REG_PWM_WGCTL0,
> + wgctl0_mask, wgctl0_val);
> + nuvoton_pwm_rmw(nvtpwm, MA35D1_REG_PWM_WGCTL1,
> + wgctl1_mask, wgctl1_val);
> +}
[ ... ]
> +static int nuvoton_pwm_probe(struct platform_device *pdev)
> +{
[ ... ]
> + if (nvtpwm->clkrate > NSEC_PER_SEC)
> + return dev_err_probe(dev, -EINVAL,
> + "PWM clock out of range (%lu)\n",
> + nvtpwm->clkrate);
> +
> + nuvoton_pwm_init(nvtpwm);
[Severity: Medium]
Will this unconditionally clobber the waveform generator and control
registers for all channels during probe()?
If the bootloader has already enabled a PWM channel (such as for a regulator
or display backlight) with a different waveform configuration, altering these
global control registers while the channel is running might corrupt its output.
Also, since nuvoton_pwm_get_state() relies on PERIOD and CMPDAT values,
altering their hardware interpretation here means the PWM core will inherit
an incorrect initial state.
Should the driver check if channels are already active via CNTEN and POEN
before modifying their configuration to allow for a seamless handover?
> +
> + chip->ops = &nuvoton_pwm_ops;
> + chip->atomic = true;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617025925.2539334-1-cwweng.linux@gmail.com?part=2
^ permalink raw reply
* [PATCH] dt-bindings: thermal: qcom-tsens: Document the maili Temperature Sensor
From: Haritha S K via B4 Relay @ 2026-06-17 4:34 UTC (permalink / raw)
To: Amit Kucheria, Thara Gopinath, Rafael J. Wysocki, Daniel Lezcano,
Zhang Rui, Lukasz Luba, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: linux-pm, linux-arm-msm, devicetree, linux-kernel,
manaf.pallikunhi, gaurav.kohli, Haritha S K
From: Haritha S K <haritha.k@oss.qualcomm.com>
Document the Temperature Sensor (TSENS) on the Qualcomm maili SoC.
Signed-off-by: Haritha S K <haritha.k@oss.qualcomm.com>
---
Documentation/devicetree/bindings/thermal/qcom-tsens.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
index f0efaa8349ee..5a8f7673e730 100644
--- a/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
+++ b/Documentation/devicetree/bindings/thermal/qcom-tsens.yaml
@@ -58,6 +58,7 @@ properties:
- qcom,glymur-tsens
- qcom,hawi-tsens
- qcom,kaanapali-tsens
+ - qcom,maili-tsens
- qcom,milos-tsens
- qcom,nord-tsens
- qcom,msm8953-tsens
---
base-commit: a76326dca329a7a738ba367529868dfbafb4a403
change-id: 20260616-b4-maili-upstream-3-b198aaa8969f
Best regards,
--
Haritha S K <haritha.k@oss.qualcomm.com>
^ permalink raw reply related
* Re: [PATCH 1/2] dt-bindings: soc: qcom: Document CDSP Power Management
From: Krzysztof Kozlowski @ 2026-06-17 4:39 UTC (permalink / raw)
To: Vignesh Viswanathan, Bjorn Andersson
Cc: Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Liam Girdwood, Mark Brown, linux-arm-msm, devicetree,
linux-kernel
In-Reply-To: <3cbcffee-a2d1-494d-b4d4-378c2ae395e6@oss.qualcomm.com>
On 26/05/2026 09:57, Vignesh Viswanathan wrote:
>>
>>> +
>>> + vdd-mx-supply:
>>> + description:
>>> + Phandle to the MX voltage regulator. This is the actual hardware regulator
>>> + (e.g., from MP8899 PMIC) that supplies power to the CDSP MX rail. Optional
>>> + on boards where MX rail is always-on or not present.
>>> +
>>> + regulators:
>>> + type: object
>>> + description:
>>> + Virtual regulators provided by this driver for consumption by the CDSP
>>> + remoteproc driver. These virtual regulators pass through enable/disable
>>> + requests to the actual hardware regulators (vdd-cx-supply, vdd-mx-supply).
>>
>> These regulators doesn't exist in reality, they are only here because
>> you choose to split the description of your remoteproc implementation in
>> two.
>
> Yes, this is because the actual regulator can be controlled by two independent entities,
> the standard PAS driver and this CDSP driver when CDSP requests via the SMEM channel.
There is no such entity as PAS driver and CDSP driver. You are mixing
the hardware with Linux drivers.
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v7 0/6] Enable I2C on SA8255p Qualcomm platforms
From: Praveen Talari @ 2026-06-17 4:50 UTC (permalink / raw)
To: Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Konrad Dybcio, Bjorn Andersson, Mukesh Kumar Savaliya,
Viken Dadhaniya
Cc: Mattijs Korpershoek, Praveen Talari, linux-arm-msm, linux-i2c,
devicetree, linux-kernel, bjorn.andersson, konrad.dybcio,
aniket.randive, chandana.chiluveru, prasad.sodagudi,
Krzysztof Kozlowski, Nikunj Kela
The Qualcomm automotive SA8255p SoC relies on firmware to configure
platform resources, including clocks, interconnects and TLMM.
The driver requests resources operations over SCMI using power
and performance protocols.
The SCMI power protocol enables or disables resources like clocks,
interconnect paths, and TLMM (GPIOs) using runtime PM framework APIs,
such as resume/suspend, to control power states(on/off).
The SCMI performance protocol manages I2C frequency, with each
frequency rate represented by a performance level. The driver uses
geni_se_set_perf_opp() API to request the desired frequency rate..
As part of geni_se_set_perf_opp(), the OPP for the requested frequency
is obtained using dev_pm_opp_find_freq_floor() and the performance
level is set using dev_pm_opp_set_opp().
Tested-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
Changes in v7:
- Rebased all patches on latest tip
- Removed SOC related patches which were merged and available in
linux-next
https://lore.kernel.org/all/177950280353.1097700.8285469548487310751.b4-ty@kernel.org/
- Link to v6: https://lore.kernel.org/all/20260227061544.1785978-1-praveen.talari@oss.qualcomm.com
Changes in v4:
- Added a new patch(4/13) to handle core clk as part of
geni_se_clks_off/on().
To: Andi Shyti <andi.shyti@kernel.org>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Konrad Dybcio <konradybcio@kernel.org>
To: Bjorn Andersson <andersson@kernel.org>
To: Praveen Talari <praveen.talari@oss.qualcomm.com>
To: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
To: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-i2c@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: bjorn.andersson@oss.qualcomm.com
Cc: konrad.dybcio@oss.qualcomm.com
Cc: aniket.randive@oss.qualcomm.com
Cc: chandana.chiluveru@oss.qualcomm.com
Cc: prasad.sodagudi@oss.qualcomm.com
---
Praveen Talari (6):
dt-bindings: i2c: Describe SA8255p
i2c: qcom-geni: Isolate serial engine setup
i2c: qcom-geni: Move resource initialization to separate function
i2c: qcom-geni: Use resources helper APIs in runtime PM functions
i2c: qcom-geni: Store of_device_id data in driver private struct
i2c: qcom-geni: Enable I2C on SA8255p Qualcomm platforms
.../bindings/i2c/qcom,sa8255p-geni-i2c.yaml | 64 ++++
drivers/i2c/busses/i2c-qcom-geni.c | 322 ++++++++++-----------
2 files changed, 220 insertions(+), 166 deletions(-)
---
base-commit: 8d6dbbbe3ba62de0a63e962ee004afb848c8e3ac
change-id: 20260616-enable-i2c-on-sa8255p-432b23838e18
Best regards,
--
Praveen Talari <praveen.talari@oss.qualcomm.com>
^ permalink raw reply
* [PATCH v7 1/6] dt-bindings: i2c: Describe SA8255p
From: Praveen Talari @ 2026-06-17 4:50 UTC (permalink / raw)
To: Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Konrad Dybcio, Bjorn Andersson, Mukesh Kumar Savaliya,
Viken Dadhaniya
Cc: Mattijs Korpershoek, Praveen Talari, linux-arm-msm, linux-i2c,
devicetree, linux-kernel, bjorn.andersson, konrad.dybcio,
aniket.randive, chandana.chiluveru, prasad.sodagudi,
Krzysztof Kozlowski, Nikunj Kela
In-Reply-To: <20260617-enable-i2c-on-sa8255p-v7-0-ad736dbeab57@oss.qualcomm.com>
Add DT bindings for the QUP GENI I2C controller on sa8255p platforms.
SA8255p platform abstracts resources such as clocks, interconnect and
GPIO pins configuration in Firmware. SCMI power and perf protocol
are utilized to request resource configurations.
SA8255p platform does not require the Serial Engine (SE) common properties
as the SE firmware is loaded and managed by the TrustZone (TZ) secure
environment.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Co-developed-by: Nikunj Kela <quic_nkela@quicinc.com>
Signed-off-by: Nikunj Kela <quic_nkela@quicinc.com>
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
v2->v3:
- Added Reviewed-by tag
v1->v2:
Krzysztof:
- Added dma properties in example node
- Removed minItems from power-domains property
- Added in commit text about common property
---
.../bindings/i2c/qcom,sa8255p-geni-i2c.yaml | 64 ++++++++++++++++++++++
1 file changed, 64 insertions(+)
diff --git a/Documentation/devicetree/bindings/i2c/qcom,sa8255p-geni-i2c.yaml b/Documentation/devicetree/bindings/i2c/qcom,sa8255p-geni-i2c.yaml
new file mode 100644
index 000000000000..a61e40b5cbc1
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/qcom,sa8255p-geni-i2c.yaml
@@ -0,0 +1,64 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/qcom,sa8255p-geni-i2c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm SA8255p QUP GENI I2C Controller
+
+maintainers:
+ - Praveen Talari <praveen.talari@oss.qualcomm.com>
+
+properties:
+ compatible:
+ const: qcom,sa8255p-geni-i2c
+
+ reg:
+ maxItems: 1
+
+ dmas:
+ maxItems: 2
+
+ dma-names:
+ items:
+ - const: tx
+ - const: rx
+
+ interrupts:
+ maxItems: 1
+
+ power-domains:
+ maxItems: 2
+
+ power-domain-names:
+ items:
+ - const: power
+ - const: perf
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - power-domains
+
+allOf:
+ - $ref: /schemas/i2c/i2c-controller.yaml#
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/dma/qcom-gpi.h>
+
+ i2c@a90000 {
+ compatible = "qcom,sa8255p-geni-i2c";
+ reg = <0xa90000 0x4000>;
+ interrupts = <GIC_SPI 357 IRQ_TYPE_LEVEL_HIGH>;
+ dmas = <&gpi_dma0 0 0 QCOM_GPI_I2C>,
+ <&gpi_dma0 1 0 QCOM_GPI_I2C>;
+ dma-names = "tx", "rx";
+ power-domains = <&scmi0_pd 0>, <&scmi0_dvfs 0>;
+ power-domain-names = "power", "perf";
+ };
+...
--
2.34.1
^ permalink raw reply related
* [PATCH v7 2/6] i2c: qcom-geni: Isolate serial engine setup
From: Praveen Talari @ 2026-06-17 4:50 UTC (permalink / raw)
To: Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Konrad Dybcio, Bjorn Andersson, Mukesh Kumar Savaliya,
Viken Dadhaniya
Cc: Mattijs Korpershoek, Praveen Talari, linux-arm-msm, linux-i2c,
devicetree, linux-kernel, bjorn.andersson, konrad.dybcio,
aniket.randive, chandana.chiluveru, prasad.sodagudi
In-Reply-To: <20260617-enable-i2c-on-sa8255p-v7-0-ad736dbeab57@oss.qualcomm.com>
Moving the serial engine setup to geni_i2c_init() API for a cleaner
probe function and utilizes the PM runtime API to control resources
instead of direct clock-related APIs for better resource management.
Enables reusability of the serial engine initialization like
hibernation and deep sleep features where hardware context is lost.
Acked-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
v6->v7:
- Added Reviewed-by tag from konrad and mukesh.
v4->v5:
- Added Reviewed-by tag.
v3->v4:
viken:
- Added Acked-by tag
- Removed extra space before invoke of geni_i2c_init().
v1->v2:
Bjorn:
- Updated commit text.
---
drivers/i2c/busses/i2c-qcom-geni.c | 158 ++++++++++++++++++-------------------
1 file changed, 75 insertions(+), 83 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index d2f5055b0b10..3ee0be228d7c 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -990,10 +990,77 @@ static int setup_gpi_dma(struct geni_i2c_dev *gi2c)
return ret;
}
+static int geni_i2c_init(struct geni_i2c_dev *gi2c)
+{
+ const struct geni_i2c_desc *desc = NULL;
+ u32 proto, tx_depth;
+ bool fifo_disable;
+ int ret;
+
+ ret = pm_runtime_resume_and_get(gi2c->se.dev);
+ if (ret < 0) {
+ dev_err(gi2c->se.dev, "error turning on device :%d\n", ret);
+ return ret;
+ }
+
+ proto = geni_se_read_proto(&gi2c->se);
+ if (proto == GENI_SE_INVALID_PROTO) {
+ ret = geni_load_se_firmware(&gi2c->se, GENI_SE_I2C);
+ if (ret) {
+ dev_err_probe(gi2c->se.dev, ret, "i2c firmware load failed ret: %d\n", ret);
+ goto err;
+ }
+ } else if (proto != GENI_SE_I2C) {
+ ret = dev_err_probe(gi2c->se.dev, -ENXIO, "Invalid proto %d\n", proto);
+ goto err;
+ }
+
+ desc = device_get_match_data(gi2c->se.dev);
+ if (desc && desc->no_dma_support) {
+ fifo_disable = false;
+ gi2c->no_dma = true;
+ } else {
+ fifo_disable = readl_relaxed(gi2c->se.base + GENI_IF_DISABLE_RO) & FIFO_IF_DISABLE;
+ }
+
+ if (fifo_disable) {
+ /* FIFO is disabled, so we can only use GPI DMA */
+ gi2c->gpi_mode = true;
+ ret = setup_gpi_dma(gi2c);
+ if (ret)
+ goto err;
+
+ dev_dbg(gi2c->se.dev, "Using GPI DMA mode for I2C\n");
+ } else {
+ gi2c->gpi_mode = false;
+ tx_depth = geni_se_get_tx_fifo_depth(&gi2c->se);
+
+ /* I2C Master Hub Serial Elements doesn't have the HW_PARAM_0 register */
+ if (!tx_depth && desc)
+ tx_depth = desc->tx_fifo_depth;
+
+ if (!tx_depth) {
+ ret = dev_err_probe(gi2c->se.dev, -EINVAL,
+ "Invalid TX FIFO depth\n");
+ goto err;
+ }
+
+ gi2c->tx_wm = tx_depth - 1;
+ geni_se_init(&gi2c->se, gi2c->tx_wm, tx_depth);
+ geni_se_config_packing(&gi2c->se, BITS_PER_BYTE,
+ PACKING_BYTES_PW, true, true, true);
+
+ dev_dbg(gi2c->se.dev, "i2c fifo/se-dma mode. fifo depth:%d\n", tx_depth);
+ }
+
+err:
+ pm_runtime_put(gi2c->se.dev);
+ return ret;
+}
+
static int geni_i2c_probe(struct platform_device *pdev)
{
struct geni_i2c_dev *gi2c;
- u32 proto, tx_depth, fifo_disable;
int ret;
struct device *dev = &pdev->dev;
const struct geni_i2c_desc *desc = NULL;
@@ -1073,101 +1140,26 @@ static int geni_i2c_probe(struct platform_device *pdev)
if (ret)
return ret;
- ret = clk_prepare_enable(gi2c->core_clk);
- if (ret)
- return ret;
-
- ret = geni_se_resources_on(&gi2c->se);
- if (ret) {
- dev_err_probe(dev, ret, "Error turning on resources\n");
- goto err_clk;
- }
- proto = geni_se_read_proto(&gi2c->se);
- if (proto == GENI_SE_INVALID_PROTO) {
- ret = geni_load_se_firmware(&gi2c->se, GENI_SE_I2C);
- if (ret) {
- dev_err_probe(dev, ret, "i2c firmware load failed ret: %d\n", ret);
- goto err_resources;
- }
- } else if (proto != GENI_SE_I2C) {
- ret = dev_err_probe(dev, -ENXIO, "Invalid proto %d\n", proto);
- goto err_resources;
- }
-
- if (desc && desc->no_dma_support) {
- fifo_disable = false;
- gi2c->no_dma = true;
- } else {
- fifo_disable = readl_relaxed(gi2c->se.base + GENI_IF_DISABLE_RO) & FIFO_IF_DISABLE;
- }
-
- if (fifo_disable) {
- /* FIFO is disabled, so we can only use GPI DMA */
- gi2c->gpi_mode = true;
- ret = setup_gpi_dma(gi2c);
- if (ret)
- goto err_resources;
-
- dev_dbg(dev, "Using GPI DMA mode for I2C\n");
- } else {
- gi2c->gpi_mode = false;
- tx_depth = geni_se_get_tx_fifo_depth(&gi2c->se);
-
- /* I2C Master Hub Serial Elements doesn't have the HW_PARAM_0 register */
- if (!tx_depth && desc)
- tx_depth = desc->tx_fifo_depth;
-
- if (!tx_depth) {
- ret = dev_err_probe(dev, -EINVAL,
- "Invalid TX FIFO depth\n");
- goto err_resources;
- }
-
- gi2c->tx_wm = tx_depth - 1;
- geni_se_init(&gi2c->se, gi2c->tx_wm, tx_depth);
- geni_se_config_packing(&gi2c->se, BITS_PER_BYTE,
- PACKING_BYTES_PW, true, true, true);
-
- dev_dbg(dev, "i2c fifo/se-dma mode. fifo depth:%d\n", tx_depth);
- }
-
- clk_disable_unprepare(gi2c->core_clk);
- ret = geni_se_resources_off(&gi2c->se);
- if (ret) {
- dev_err_probe(dev, ret, "Error turning off resources\n");
- goto err_dma;
- }
-
- ret = geni_icc_disable(&gi2c->se);
- if (ret)
- goto err_dma;
-
pm_runtime_set_suspended(gi2c->se.dev);
pm_runtime_set_autosuspend_delay(gi2c->se.dev, I2C_AUTO_SUSPEND_DELAY);
pm_runtime_use_autosuspend(gi2c->se.dev);
pm_runtime_enable(gi2c->se.dev);
+ ret = geni_i2c_init(gi2c);
+ if (ret < 0) {
+ pm_runtime_disable(gi2c->se.dev);
+ return ret;
+ }
+
ret = i2c_add_adapter(&gi2c->adap);
if (ret) {
dev_err_probe(dev, ret, "Error adding i2c adapter\n");
pm_runtime_disable(gi2c->se.dev);
- goto err_dma;
+ return ret;
}
dev_dbg(dev, "Geni-I2C adaptor successfully added\n");
- return ret;
-
-err_resources:
- geni_se_resources_off(&gi2c->se);
-err_clk:
- clk_disable_unprepare(gi2c->core_clk);
-
- return ret;
-
-err_dma:
- release_gpi_dma(gi2c);
-
return ret;
}
--
2.34.1
^ permalink raw reply related
* [PATCH v7 3/6] i2c: qcom-geni: Move resource initialization to separate function
From: Praveen Talari @ 2026-06-17 4:50 UTC (permalink / raw)
To: Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Konrad Dybcio, Bjorn Andersson, Mukesh Kumar Savaliya,
Viken Dadhaniya
Cc: Mattijs Korpershoek, Praveen Talari, linux-arm-msm, linux-i2c,
devicetree, linux-kernel, bjorn.andersson, konrad.dybcio,
aniket.randive, chandana.chiluveru, prasad.sodagudi
In-Reply-To: <20260617-enable-i2c-on-sa8255p-v7-0-ad736dbeab57@oss.qualcomm.com>
Refactor the resource initialization in geni_i2c_probe() by introducing
a new geni_i2c_resources_init() function and utilizing the common
geni_se_resources_init() framework and clock frequency mapping, making the
probe function cleaner.
Acked-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
v4->v5:
- Added a Reviewed-by tag.
- Removed core clk, has_core_clk and icc_ddr variable.
v3->v4:
- Added Acked-by tag.
v1->v2:
- Updated commit text.
---
drivers/i2c/busses/i2c-qcom-geni.c | 64 ++++++++++++++------------------------
1 file changed, 24 insertions(+), 40 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index 3ee0be228d7c..a7672b6c2bc0 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -107,7 +107,6 @@ struct geni_i2c_dev {
int cur_wr;
int cur_rd;
spinlock_t lock;
- struct clk *core_clk;
u32 clk_freq_out;
const struct geni_i2c_clk_fld *clk_fld;
void *dma_buf;
@@ -124,8 +123,6 @@ struct geni_i2c_dev {
};
struct geni_i2c_desc {
- bool has_core_clk;
- char *icc_ddr;
bool no_dma_support;
unsigned int tx_fifo_depth;
};
@@ -1058,6 +1055,23 @@ static int geni_i2c_init(struct geni_i2c_dev *gi2c)
return ret;
}
+static int geni_i2c_resources_init(struct geni_i2c_dev *gi2c)
+{
+ int ret;
+
+ ret = geni_se_resources_init(&gi2c->se);
+ if (ret)
+ return ret;
+
+ ret = geni_i2c_clk_map_idx(gi2c);
+ if (ret)
+ return dev_err_probe(gi2c->se.dev, ret, "Invalid clk frequency %d Hz\n",
+ gi2c->clk_freq_out);
+
+ return geni_icc_set_bw_ab(&gi2c->se, GENI_DEFAULT_BW, GENI_DEFAULT_BW,
+ Bps_to_icc(gi2c->clk_freq_out));
+}
+
static int geni_i2c_probe(struct platform_device *pdev)
{
struct geni_i2c_dev *gi2c;
@@ -1077,16 +1091,6 @@ static int geni_i2c_probe(struct platform_device *pdev)
desc = device_get_match_data(&pdev->dev);
- if (desc && desc->has_core_clk) {
- gi2c->core_clk = devm_clk_get(dev, "core");
- if (IS_ERR(gi2c->core_clk))
- return PTR_ERR(gi2c->core_clk);
- }
-
- gi2c->se.clk = devm_clk_get(dev, "se");
- if (IS_ERR(gi2c->se.clk) && !has_acpi_companion(dev))
- return PTR_ERR(gi2c->se.clk);
-
ret = device_property_read_u32(dev, "clock-frequency",
&gi2c->clk_freq_out);
if (ret) {
@@ -1101,16 +1105,15 @@ static int geni_i2c_probe(struct platform_device *pdev)
if (gi2c->irq < 0)
return gi2c->irq;
- ret = geni_i2c_clk_map_idx(gi2c);
- if (ret)
- return dev_err_probe(dev, ret, "Invalid clk frequency %d Hz\n",
- gi2c->clk_freq_out);
-
gi2c->adap.algo = &geni_i2c_algo;
init_completion(&gi2c->done);
spin_lock_init(&gi2c->lock);
platform_set_drvdata(pdev, gi2c);
+ ret = geni_i2c_resources_init(gi2c);
+ if (ret)
+ return ret;
+
/* Keep interrupts disabled initially to allow for low-power modes */
ret = devm_request_irq(dev, gi2c->irq, geni_i2c_irq, IRQF_NO_AUTOEN,
dev_name(dev), gi2c);
@@ -1123,23 +1126,6 @@ static int geni_i2c_probe(struct platform_device *pdev)
gi2c->adap.dev.of_node = dev->of_node;
strscpy(gi2c->adap.name, "Geni-I2C", sizeof(gi2c->adap.name));
- ret = geni_icc_get(&gi2c->se, desc ? desc->icc_ddr : "qup-memory");
- if (ret)
- return ret;
- /*
- * Set the bus quota for core and cpu to a reasonable value for
- * register access.
- * Set quota for DDR based on bus speed.
- */
- gi2c->se.icc_paths[GENI_TO_CORE].avg_bw = GENI_DEFAULT_BW;
- gi2c->se.icc_paths[CPU_TO_GENI].avg_bw = GENI_DEFAULT_BW;
- if (!desc || desc->icc_ddr)
- gi2c->se.icc_paths[GENI_TO_DDR].avg_bw = Bps_to_icc(gi2c->clk_freq_out);
-
- ret = geni_icc_set_bw(&gi2c->se);
- if (ret)
- return ret;
-
pm_runtime_set_suspended(gi2c->se.dev);
pm_runtime_set_autosuspend_delay(gi2c->se.dev, I2C_AUTO_SUSPEND_DELAY);
pm_runtime_use_autosuspend(gi2c->se.dev);
@@ -1192,7 +1178,7 @@ static int __maybe_unused geni_i2c_runtime_suspend(struct device *dev)
return ret;
}
- clk_disable_unprepare(gi2c->core_clk);
+ clk_disable_unprepare(gi2c->se.core_clk);
return geni_icc_disable(&gi2c->se);
}
@@ -1206,7 +1192,7 @@ static int __maybe_unused geni_i2c_runtime_resume(struct device *dev)
if (ret)
return ret;
- ret = clk_prepare_enable(gi2c->core_clk);
+ ret = clk_prepare_enable(gi2c->se.core_clk);
if (ret)
goto out_icc_disable;
@@ -1219,7 +1205,7 @@ static int __maybe_unused geni_i2c_runtime_resume(struct device *dev)
return 0;
out_clk_disable:
- clk_disable_unprepare(gi2c->core_clk);
+ clk_disable_unprepare(gi2c->se.core_clk);
out_icc_disable:
geni_icc_disable(&gi2c->se);
@@ -1260,8 +1246,6 @@ static const struct dev_pm_ops geni_i2c_pm_ops = {
};
static const struct geni_i2c_desc i2c_master_hub = {
- .has_core_clk = true,
- .icc_ddr = NULL,
.no_dma_support = true,
.tx_fifo_depth = 16,
};
--
2.34.1
^ permalink raw reply related
* [PATCH v7 4/6] i2c: qcom-geni: Use resources helper APIs in runtime PM functions
From: Praveen Talari @ 2026-06-17 4:50 UTC (permalink / raw)
To: Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Konrad Dybcio, Bjorn Andersson, Mukesh Kumar Savaliya,
Viken Dadhaniya
Cc: Mattijs Korpershoek, Praveen Talari, linux-arm-msm, linux-i2c,
devicetree, linux-kernel, bjorn.andersson, konrad.dybcio,
aniket.randive, chandana.chiluveru, prasad.sodagudi
In-Reply-To: <20260617-enable-i2c-on-sa8255p-v7-0-ad736dbeab57@oss.qualcomm.com>
To manage GENI serial engine resources during runtime power management,
drivers currently need to call functions for ICC, clock, and
SE resource operations in both suspend and resume paths, resulting in
code duplication across drivers.
The new geni_se_resources_activate() and geni_se_resources_deactivate()
helper APIs addresses this issue by providing a streamlined method to
enable or disable all resources based, thereby eliminating redundancy
across drivers.
Acked-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
v5->v6:
Konrad:
- Updated last return value as "0".
- Added Reviewed-by
v3->v4:
- Added Acked-by tag.
v1->v2:
Bjorn:
- Remove geni_se_resources_state() API.
- Used geni_se_resources_activate() and geni_se_resources_deactivate()
to enable/disable resources.
---
drivers/i2c/busses/i2c-qcom-geni.c | 24 ++++--------------------
1 file changed, 4 insertions(+), 20 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index a7672b6c2bc0..8f8b74c4e88a 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -1172,15 +1172,14 @@ static int __maybe_unused geni_i2c_runtime_suspend(struct device *dev)
struct geni_i2c_dev *gi2c = dev_get_drvdata(dev);
disable_irq(gi2c->irq);
- ret = geni_se_resources_off(&gi2c->se);
+
+ ret = geni_se_resources_deactivate(&gi2c->se);
if (ret) {
enable_irq(gi2c->irq);
return ret;
}
- clk_disable_unprepare(gi2c->se.core_clk);
-
- return geni_icc_disable(&gi2c->se);
+ return 0;
}
static int __maybe_unused geni_i2c_runtime_resume(struct device *dev)
@@ -1188,28 +1187,13 @@ static int __maybe_unused geni_i2c_runtime_resume(struct device *dev)
int ret;
struct geni_i2c_dev *gi2c = dev_get_drvdata(dev);
- ret = geni_icc_enable(&gi2c->se);
+ ret = geni_se_resources_activate(&gi2c->se);
if (ret)
return ret;
- ret = clk_prepare_enable(gi2c->se.core_clk);
- if (ret)
- goto out_icc_disable;
-
- ret = geni_se_resources_on(&gi2c->se);
- if (ret)
- goto out_clk_disable;
-
enable_irq(gi2c->irq);
return 0;
-
-out_clk_disable:
- clk_disable_unprepare(gi2c->se.core_clk);
-out_icc_disable:
- geni_icc_disable(&gi2c->se);
-
- return ret;
}
static int __maybe_unused geni_i2c_suspend_noirq(struct device *dev)
--
2.34.1
^ permalink raw reply related
* [PATCH v7 5/6] i2c: qcom-geni: Store of_device_id data in driver private struct
From: Praveen Talari @ 2026-06-17 4:50 UTC (permalink / raw)
To: Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Konrad Dybcio, Bjorn Andersson, Mukesh Kumar Savaliya,
Viken Dadhaniya
Cc: Mattijs Korpershoek, Praveen Talari, linux-arm-msm, linux-i2c,
devicetree, linux-kernel, bjorn.andersson, konrad.dybcio,
aniket.randive, chandana.chiluveru, prasad.sodagudi
In-Reply-To: <20260617-enable-i2c-on-sa8255p-v7-0-ad736dbeab57@oss.qualcomm.com>
To avoid repeatedly fetching and checking platform data across various
functions, store the struct of_device_id data directly in the i2c
private structure. This change enhances code maintainability and reduces
redundancy.
Acked-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
v6->v7
- Added Reviewed-by from konrad.
v5->v6
Konrad
- Updated in geni_i2c_acpi_match() as suggested
- Moved geni_i2c_acpi_match below to avoid errors of geni-i2c
v4->v5
Konrad
- Added a null check after platform data struct.
v3->v4
- Added Acked-by tag.
Konrad
- Removed icc_ddr from platfrom data struct
---
drivers/i2c/busses/i2c-qcom-geni.c | 46 ++++++++++++++++++++------------------
1 file changed, 24 insertions(+), 22 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index 8f8b74c4e88a..92cccb1cce69 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -77,6 +77,11 @@ enum geni_i2c_err_code {
#define XFER_TIMEOUT HZ
#define RST_TIMEOUT HZ
+struct geni_i2c_desc {
+ bool no_dma_support;
+ unsigned int tx_fifo_depth;
+};
+
#define QCOM_I2C_MIN_NUM_OF_MSGS_MULTI_DESC 2
/**
@@ -120,11 +125,7 @@ struct geni_i2c_dev {
bool is_tx_multi_desc_xfer;
u32 num_msgs;
struct geni_i2c_gpi_multi_desc_xfer i2c_multi_desc_config;
-};
-
-struct geni_i2c_desc {
- bool no_dma_support;
- unsigned int tx_fifo_depth;
+ const struct geni_i2c_desc *dev_data;
};
struct geni_i2c_err_log {
@@ -941,15 +942,6 @@ static const struct i2c_algorithm geni_i2c_algo = {
.functionality = geni_i2c_func,
};
-#ifdef CONFIG_ACPI
-static const struct acpi_device_id geni_i2c_acpi_match[] = {
- { "QCOM0220"},
- { "QCOM0411" },
- { }
-};
-MODULE_DEVICE_TABLE(acpi, geni_i2c_acpi_match);
-#endif
-
static void release_gpi_dma(struct geni_i2c_dev *gi2c)
{
if (gi2c->rx_c)
@@ -989,7 +981,6 @@ static int setup_gpi_dma(struct geni_i2c_dev *gi2c)
static int geni_i2c_init(struct geni_i2c_dev *gi2c)
{
- const struct geni_i2c_desc *desc = NULL;
u32 proto, tx_depth;
bool fifo_disable;
int ret;
@@ -1012,8 +1003,7 @@ static int geni_i2c_init(struct geni_i2c_dev *gi2c)
goto err;
}
- desc = device_get_match_data(gi2c->se.dev);
- if (desc && desc->no_dma_support) {
+ if (gi2c->dev_data->no_dma_support) {
fifo_disable = false;
gi2c->no_dma = true;
} else {
@@ -1033,8 +1023,8 @@ static int geni_i2c_init(struct geni_i2c_dev *gi2c)
tx_depth = geni_se_get_tx_fifo_depth(&gi2c->se);
/* I2C Master Hub Serial Elements doesn't have the HW_PARAM_0 register */
- if (!tx_depth && desc)
- tx_depth = desc->tx_fifo_depth;
+ if (!tx_depth && gi2c->se.core_clk)
+ tx_depth = gi2c->dev_data->tx_fifo_depth;
if (!tx_depth) {
ret = dev_err_probe(gi2c->se.dev, -EINVAL,
@@ -1077,7 +1067,6 @@ static int geni_i2c_probe(struct platform_device *pdev)
struct geni_i2c_dev *gi2c;
int ret;
struct device *dev = &pdev->dev;
- const struct geni_i2c_desc *desc = NULL;
gi2c = devm_kzalloc(dev, sizeof(*gi2c), GFP_KERNEL);
if (!gi2c)
@@ -1089,7 +1078,9 @@ static int geni_i2c_probe(struct platform_device *pdev)
if (IS_ERR(gi2c->se.base))
return PTR_ERR(gi2c->se.base);
- desc = device_get_match_data(&pdev->dev);
+ gi2c->dev_data = device_get_match_data(&pdev->dev);
+ if (!gi2c->dev_data)
+ return -EINVAL;
ret = device_property_read_u32(dev, "clock-frequency",
&gi2c->clk_freq_out);
@@ -1229,13 +1220,24 @@ static const struct dev_pm_ops geni_i2c_pm_ops = {
NULL)
};
+static const struct geni_i2c_desc geni_i2c = {};
+
static const struct geni_i2c_desc i2c_master_hub = {
.no_dma_support = true,
.tx_fifo_depth = 16,
};
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id geni_i2c_acpi_match[] = {
+ { "QCOM0220", (kernel_ulong_t)&geni_i2c},
+ { "QCOM0411", (kernel_ulong_t)&geni_i2c},
+ { }
+};
+MODULE_DEVICE_TABLE(acpi, geni_i2c_acpi_match);
+#endif
+
static const struct of_device_id geni_i2c_dt_match[] = {
- { .compatible = "qcom,geni-i2c" },
+ { .compatible = "qcom,geni-i2c", .data = &geni_i2c },
{ .compatible = "qcom,geni-i2c-master-hub", .data = &i2c_master_hub },
{}
};
--
2.34.1
^ permalink raw reply related
* [PATCH v7 6/6] i2c: qcom-geni: Enable I2C on SA8255p Qualcomm platforms
From: Praveen Talari @ 2026-06-17 4:50 UTC (permalink / raw)
To: Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Konrad Dybcio, Bjorn Andersson, Mukesh Kumar Savaliya,
Viken Dadhaniya
Cc: Mattijs Korpershoek, Praveen Talari, linux-arm-msm, linux-i2c,
devicetree, linux-kernel, bjorn.andersson, konrad.dybcio,
aniket.randive, chandana.chiluveru, prasad.sodagudi
In-Reply-To: <20260617-enable-i2c-on-sa8255p-v7-0-ad736dbeab57@oss.qualcomm.com>
The Qualcomm automotive SA8255p SoC relies on firmware to configure
platform resources, including clocks, interconnects and TLMM.
The driver requests resources operations over SCMI using power
and performance protocols.
The SCMI power protocol enables or disables resources like clocks,
interconnect paths, and TLMM (GPIOs) using runtime PM framework APIs,
such as resume/suspend, to control power on/off.
The SCMI performance protocol manages I2C frequency, with each
frequency rate represented by a performance level. The driver uses
geni_se_set_perf_opp() API to request the desired frequency rate..
As part of geni_se_set_perf_opp(), the OPP for the requested frequency
is obtained using dev_pm_opp_find_freq_floor() and the performance
level is set using dev_pm_opp_set_opp().
Acked-by: Viken Dadhaniya <viken.dadhaniya@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Praveen Talari <praveen.talari@oss.qualcomm.com>
---
v6->v7
- Added Reviewed-by from konrad.
v3->v4:
- Added Acked-by tag.
V1->v2:
From kernel test robot:
- Initialized ret to "0" in resume/suspend callbacks.
Bjorn:
- Used seperate APIs for the resouces enable/disable.
---
drivers/i2c/busses/i2c-qcom-geni.c | 56 ++++++++++++++++++++++++++++----------
1 file changed, 42 insertions(+), 14 deletions(-)
diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
index 92cccb1cce69..96dbf04138be 100644
--- a/drivers/i2c/busses/i2c-qcom-geni.c
+++ b/drivers/i2c/busses/i2c-qcom-geni.c
@@ -80,6 +80,10 @@ enum geni_i2c_err_code {
struct geni_i2c_desc {
bool no_dma_support;
unsigned int tx_fifo_depth;
+ int (*resources_init)(struct geni_se *se);
+ int (*set_rate)(struct geni_se *se, unsigned long freq);
+ int (*power_on)(struct geni_se *se);
+ int (*power_off)(struct geni_se *se);
};
#define QCOM_I2C_MIN_NUM_OF_MSGS_MULTI_DESC 2
@@ -200,8 +204,9 @@ static int geni_i2c_clk_map_idx(struct geni_i2c_dev *gi2c)
return -EINVAL;
}
-static void qcom_geni_i2c_conf(struct geni_i2c_dev *gi2c)
+static int qcom_geni_i2c_conf(struct geni_se *se, unsigned long freq)
{
+ struct geni_i2c_dev *gi2c = dev_get_drvdata(se->dev);
const struct geni_i2c_clk_fld *itr = gi2c->clk_fld;
u32 val;
@@ -214,6 +219,7 @@ static void qcom_geni_i2c_conf(struct geni_i2c_dev *gi2c)
val |= itr->t_low_cnt << LOW_COUNTER_SHFT;
val |= itr->t_cycle_cnt;
writel_relaxed(val, gi2c->se.base + SE_I2C_SCL_COUNTERS);
+ return 0;
}
static void geni_i2c_err_misc(struct geni_i2c_dev *gi2c)
@@ -919,7 +925,9 @@ static int geni_i2c_xfer(struct i2c_adapter *adap,
return ret;
}
- qcom_geni_i2c_conf(gi2c);
+ ret = gi2c->dev_data->set_rate(&gi2c->se, gi2c->clk_freq_out);
+ if (ret)
+ return ret;
if (gi2c->gpi_mode)
ret = geni_i2c_gpi_xfer(gi2c, msgs, num);
@@ -1045,8 +1053,9 @@ static int geni_i2c_init(struct geni_i2c_dev *gi2c)
return ret;
}
-static int geni_i2c_resources_init(struct geni_i2c_dev *gi2c)
+static int geni_i2c_resources_init(struct geni_se *se)
{
+ struct geni_i2c_dev *gi2c = dev_get_drvdata(se->dev);
int ret;
ret = geni_se_resources_init(&gi2c->se);
@@ -1101,7 +1110,7 @@ static int geni_i2c_probe(struct platform_device *pdev)
spin_lock_init(&gi2c->lock);
platform_set_drvdata(pdev, gi2c);
- ret = geni_i2c_resources_init(gi2c);
+ ret = gi2c->dev_data->resources_init(&gi2c->se);
if (ret)
return ret;
@@ -1159,15 +1168,17 @@ static void geni_i2c_shutdown(struct platform_device *pdev)
static int __maybe_unused geni_i2c_runtime_suspend(struct device *dev)
{
- int ret;
+ int ret = 0;
struct geni_i2c_dev *gi2c = dev_get_drvdata(dev);
disable_irq(gi2c->irq);
- ret = geni_se_resources_deactivate(&gi2c->se);
- if (ret) {
- enable_irq(gi2c->irq);
- return ret;
+ if (gi2c->dev_data->power_off) {
+ ret = gi2c->dev_data->power_off(&gi2c->se);
+ if (ret) {
+ enable_irq(gi2c->irq);
+ return ret;
+ }
}
return 0;
@@ -1175,12 +1186,14 @@ static int __maybe_unused geni_i2c_runtime_suspend(struct device *dev)
static int __maybe_unused geni_i2c_runtime_resume(struct device *dev)
{
- int ret;
+ int ret = 0;
struct geni_i2c_dev *gi2c = dev_get_drvdata(dev);
- ret = geni_se_resources_activate(&gi2c->se);
- if (ret)
- return ret;
+ if (gi2c->dev_data->power_on) {
+ ret = gi2c->dev_data->power_on(&gi2c->se);
+ if (ret)
+ return ret;
+ }
enable_irq(gi2c->irq);
@@ -1220,11 +1233,25 @@ static const struct dev_pm_ops geni_i2c_pm_ops = {
NULL)
};
-static const struct geni_i2c_desc geni_i2c = {};
+static const struct geni_i2c_desc geni_i2c = {
+ .resources_init = geni_i2c_resources_init,
+ .set_rate = qcom_geni_i2c_conf,
+ .power_on = geni_se_resources_activate,
+ .power_off = geni_se_resources_deactivate,
+};
static const struct geni_i2c_desc i2c_master_hub = {
.no_dma_support = true,
.tx_fifo_depth = 16,
+ .resources_init = geni_i2c_resources_init,
+ .set_rate = qcom_geni_i2c_conf,
+ .power_on = geni_se_resources_activate,
+ .power_off = geni_se_resources_deactivate,
+};
+
+static const struct geni_i2c_desc sa8255p_geni_i2c = {
+ .resources_init = geni_se_domain_attach,
+ .set_rate = geni_se_set_perf_opp,
};
#ifdef CONFIG_ACPI
@@ -1239,6 +1266,7 @@ MODULE_DEVICE_TABLE(acpi, geni_i2c_acpi_match);
static const struct of_device_id geni_i2c_dt_match[] = {
{ .compatible = "qcom,geni-i2c", .data = &geni_i2c },
{ .compatible = "qcom,geni-i2c-master-hub", .data = &i2c_master_hub },
+ { .compatible = "qcom,sa8255p-geni-i2c", .data = &sa8255p_geni_i2c },
{}
};
MODULE_DEVICE_TABLE(of, geni_i2c_dt_match);
--
2.34.1
^ permalink raw reply related
* RE: [PATCH net v5 1/4] net: ethernet: oa_tc6: Interrupt is active low, level triggered.
From: Selvamani Rajagopal @ 2026-06-17 4:54 UTC (permalink / raw)
To: Parthiban.Veerasooran@microchip.com, andrew+netdev@lunn.ch,
davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
pabeni@redhat.com, robh@kernel.org, krzk+dt@kernel.org,
conor+dt@kernel.org, Piergiorgio Beruto
Cc: andrew@lunn.ch, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Conor.Dooley@microchip.com,
devicetree@vger.kernel.org
In-Reply-To: <7c89df6b-32ac-46c8-8400-945879037f2e@microchip.com>
> Subject: Re: [PATCH net v5 1/4] net: ethernet: oa_tc6: Interrupt is active low, level
> triggered.
>
>
> Hi Selvamani,
>
> I did a quick test by connecting Mikroe LAN8651 Click to a Raspberry Pi
> 4 and shared the feedback below. Please let me know if you need any
> further details.
Parthiban,
Thanks for testing this.
Though the NULL pointer reference after skb_put is a clue, I am working with our team to see we can see this crash in our setup.
Will keep you updated.
>
> [ 8276.691064] eth1: Receive buffer overflow error
> [ 8281.662600] Unable to handle kernel NULL pointer dereference at
> virtual address 0000000000000074> drm_panel_orientation_quirks backlight nfnetlink
> [ 8281.839427] pc : skb_put+0x14/0x80
> [ 8281.842864] lr : oa_tc6_macphy_threaded_irq+0x428/0x880 [lan865x_t1s]
^ permalink raw reply
* Re: [PATCH v7 2/6] i2c: qcom-geni: Isolate serial engine setup
From: sashiko-bot @ 2026-06-17 5:00 UTC (permalink / raw)
To: Praveen Talari; +Cc: robh, devicetree, conor+dt
In-Reply-To: <20260617-enable-i2c-on-sa8255p-v7-2-ad736dbeab57@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Asynchronous pm_runtime_put() followed immediately by pm_runtime_disable() leaves hardware permanently powered on during probe failures.
- [Medium] DMA channels allocated during hardware initialization are permanently leaked if I2C adapter registration fails.
--
commit 111d823f58de8c30c14ab44aa8466c66a5e8d761
Author: Praveen Talari <praveen.talari@oss.qualcomm.com>
i2c: qcom-geni: Isolate serial engine setup
Moving the serial engine setup to geni_i2c_init() API for a cleaner
probe function and utilizes the PM runtime API to control resources
instead of direct clock-related APIs for better resource management.
> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index d2f5055b0b108..3ee0be228d7c1 100644
> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
> @@ -990,10 +990,77 @@ static int setup_gpi_dma(struct geni_i2c_dev *gi2c)
> return ret;
> }
>
> +static int geni_i2c_init(struct geni_i2c_dev *gi2c)
> +{
> + const struct geni_i2c_desc *desc = NULL;
> + u32 proto, tx_depth;
> + bool fifo_disable;
> + int ret;
> +
> + ret = pm_runtime_resume_and_get(gi2c->se.dev);
[ ... ]
> + if (fifo_disable) {
> + /* FIFO is disabled, so we can only use GPI DMA */
> + gi2c->gpi_mode = true;
> + ret = setup_gpi_dma(gi2c);
> + if (ret)
> + goto err;
[ ... ]
> +err:
> + pm_runtime_put(gi2c->se.dev);
> + return ret;
> +}
> +
> static int geni_i2c_probe(struct platform_device *pdev)
> {
[ ... ]
> pm_runtime_set_suspended(gi2c->se.dev);
> pm_runtime_set_autosuspend_delay(gi2c->se.dev, I2C_AUTO_SUSPEND_DELAY);
> pm_runtime_use_autosuspend(gi2c->se.dev);
> pm_runtime_enable(gi2c->se.dev);
>
> + ret = geni_i2c_init(gi2c);
> + if (ret < 0) {
> + pm_runtime_disable(gi2c->se.dev);
[Severity: Medium]
If geni_i2c_init() fails, it drops its runtime PM reference using the
asynchronous pm_runtime_put(). Will calling pm_runtime_disable() right
afterwards cancel the pending suspend work, leaving the device in the
RPM_ACTIVE state and leaking power resources?
> + return ret;
> + }
> +
> ret = i2c_add_adapter(&gi2c->adap);
> if (ret) {
> dev_err_probe(dev, ret, "Error adding i2c adapter\n");
> pm_runtime_disable(gi2c->se.dev);
> - goto err_dma;
> + return ret;
[Severity: Medium]
Does this code leak the DMA channels if i2c_add_adapter() fails?
Before this patch, a failure here would jump to err_dma which called
release_gpi_dma(gi2c). Now it returns directly, but geni_i2c_init()
may have successfully allocated the DMA channels via setup_gpi_dma().
Additionally, similar to the check above, will pm_runtime_disable() cancel
the async suspend from the successful geni_i2c_init() path, leaving the
hardware powered on?
> }
>
> dev_dbg(dev, "Geni-I2C adaptor successfully added\n");
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-enable-i2c-on-sa8255p-v7-0-ad736dbeab57@oss.qualcomm.com?part=2
^ permalink raw reply
* Re: [PATCH v7 5/6] i2c: qcom-geni: Store of_device_id data in driver private struct
From: sashiko-bot @ 2026-06-17 5:02 UTC (permalink / raw)
To: Praveen Talari; +Cc: robh, devicetree, conor+dt
In-Reply-To: <20260617-enable-i2c-on-sa8255p-v7-5-ad736dbeab57@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Accessing a non-existent member `core_clk` in `struct geni_se` causes a build failure and alters fallback logic.
- [High] DMA channels are permanently leaked if `i2c_add_adapter()` fails during probe.
Pre-existing issues:
- [High] Multi-descriptor DMA buffer arrays are leaked on early transfer errors or partial allocation failures.
--
commit 1f9fcc815478494b91c04d32de4b9e7efa6fac42
Author: Praveen Talari <praveen.talari@oss.qualcomm.com>
i2c: qcom-geni: Store of_device_id data in driver private struct
This commit updates the driver to store the struct of_device_id data
directly in the i2c private structure. The goal is to improve code
maintainability and avoid repeatedly fetching and checking platform
data across different functions.
> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index 8f8b74c4e88a4..92cccb1cce698 100644
> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
[ ... ]
> @@ -1033,8 +1023,8 @@ static int geni_i2c_init(struct geni_i2c_dev *gi2c)
> tx_depth = geni_se_get_tx_fifo_depth(&gi2c->se);
>
> /* I2C Master Hub Serial Elements doesn't have the HW_PARAM_0 register */
> - if (!tx_depth && desc)
> - tx_depth = desc->tx_fifo_depth;
> + if (!tx_depth && gi2c->se.core_clk)
> + tx_depth = gi2c->dev_data->tx_fifo_depth;
[Severity: High]
Does struct geni_se have a core_clk member? Looking at geni-se.h, it appears
this will cause a build failure since core_clk doesn't exist.
Additionally, if this is changed to gi2c->se.clk, will it alter the original
fallback logic? Previously, if a master-hub device lacked this clock, it
correctly fell back to using desc->tx_fifo_depth. With this change, if the
clock is missing, tx_depth might remain 0 and cause a probe failure.
[ ... ]
> @@ -1089,7 +1078,9 @@ static int geni_i2c_probe(struct platform_device *pdev)
> if (IS_ERR(gi2c->se.base))
> return PTR_ERR(gi2c->se.base);
>
> - desc = device_get_match_data(&pdev->dev);
> + gi2c->dev_data = device_get_match_data(&pdev->dev);
> + if (!gi2c->dev_data)
> + return -EINVAL;
[Severity: High]
During the refactoring in this patch series, did we lose the cleanup for
DMA channels in geni_i2c_probe()?
If i2c_add_adapter() fails after geni_i2c_init() has successfully requested
GPI DMA channels via setup_gpi_dma(), release_gpi_dma() is not called:
geni_i2c_probe() {
...
ret = geni_i2c_init(gi2c);
if (ret < 0) { ... }
ret = i2c_add_adapter(&gi2c->adap);
if (ret) {
dev_err_probe(dev, ret, "Error adding i2c adapter\n");
pm_runtime_disable(gi2c->se.dev);
return ret;
}
...
}
Are tx_c and rx_c permanently leaked here on failure?
[ ... ]
[Severity: High]
This is a pre-existing issue, but do multi-descriptor DMA buffer arrays
leak on early transfer errors or partial allocation failures?
In geni_i2c_gpi_xfer(), if the second allocation fails, it jumps to the
error label:
geni_i2c_gpi_xfer() {
...
tx_multi_xfer->dma_buf = kcalloc(gi2c->num_msgs, ...);
tx_multi_xfer->dma_addr = kcalloc(gi2c->num_msgs, ...);
if (!tx_multi_xfer->dma_buf || !tx_multi_xfer->dma_addr) {
ret = -ENOMEM;
goto err;
}
...
err:
geni_i2c_gpi_multi_desc_unmap(gi2c, msgs, &i);
}
Inside geni_i2c_gpi_multi_desc_unmap(), the arrays are only freed if
the loop reaches the end:
geni_i2c_gpi_multi_desc_unmap() {
...
if (tx_multi_xfer->unmap_msg_cnt == gi2c->num_msgs - 1) {
kfree(tx_multi_xfer->dma_buf);
kfree(tx_multi_xfer->dma_addr);
break;
}
...
}
During an early abort or partial allocation failure, the arrays are not
freed. Should they be cleaned up unconditionally?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-enable-i2c-on-sa8255p-v7-0-ad736dbeab57@oss.qualcomm.com?part=5
^ permalink raw reply
* Re: [PATCH v7 1/6] dt-bindings: i2c: Describe SA8255p
From: sashiko-bot @ 2026-06-17 5:04 UTC (permalink / raw)
To: Praveen Talari; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260617-enable-i2c-on-sa8255p-v7-1-ad736dbeab57@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Incomplete schema requirements for power domains.
- [Low] Missing `#address-cells` and `#size-cells` in the example block.
--
commit 028881c69d7286ec4ecd4e0f30436fe9b7b8594c
Author: Praveen Talari <praveen.talari@oss.qualcomm.com>
dt-bindings: i2c: Describe SA8255p
Add DT bindings for the QUP GENI I2C controller on sa8255p platforms.
The platform abstracts resources in firmware via SCMI and uses TZ for
Serial Engine management, omitting standard SE properties.
> diff --git a/Documentation/devicetree/bindings/i2c/qcom,sa8255p-geni-i2c.yaml b/Documentation/devicetree/bindings/i2c/qcom,sa8255p-geni-i2c.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/qcom,sa8255p-geni-i2c.yaml
[ ... ]
> +properties:
> + compatible:
> + const: qcom,sa8255p-geni-i2c
[ ... ]
> + power-domains:
> + maxItems: 2
[Severity: Medium]
Since the SA8255p platform requires both the power and perf domains to
function properly, should this also specify minItems: 2? Currently,
maxItems: 2 implicitly defaults to minItems: 1, allowing a configuration
with missing domains to pass schema validation.
> +
> + power-domain-names:
> + items:
> + - const: power
> + - const: perf
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - power-domains
[Severity: Medium]
Should power-domain-names also be included in the required block?
Without it, an incomplete device tree node could pass schema validation but
fail at runtime when the driver attempts to request the missing domains by
name.
> +
> +allOf:
> + - $ref: /schemas/i2c/i2c-controller.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + #include <dt-bindings/dma/qcom-gpi.h>
> +
> + i2c@a90000 {
> + compatible = "qcom,sa8255p-geni-i2c";
[Severity: Low]
Does this example need #address-cells and #size-cells properties?
Because the schema inherits from i2c-controller.yaml, an I2C controller
node is inherently treated as a bus requiring #address-cells = <1> and
#size-cells = <0>. Omitting these might trigger a dt_binding_check schema
validation error.
> + reg = <0xa90000 0x4000>;
> + interrupts = <GIC_SPI 357 IRQ_TYPE_LEVEL_HIGH>;
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-enable-i2c-on-sa8255p-v7-0-ad736dbeab57@oss.qualcomm.com?part=1
^ permalink raw reply
* Re: [PATCH v7 3/6] i2c: qcom-geni: Move resource initialization to separate function
From: sashiko-bot @ 2026-06-17 5:05 UTC (permalink / raw)
To: Praveen Talari; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260617-enable-i2c-on-sa8255p-v7-3-ad736dbeab57@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [Critical] Build failure due to undefined GENI SE resource initialization and PM functions
- [High] GPI DMA channels are permanently leaked on probe failure.
- [High] Asynchronous pm_runtime_put() is canceled by pm_runtime_disable() on probe error paths, leaking power.
Pre-existing issues:
- [High] Missing synchronous suspend before disabling runtime PM during device removal.
--
commit 3d26b1870da68dc53ac527a03242c49f148d835f
Author: Praveen Talari <praveen.talari@oss.qualcomm.com>
i2c: qcom-geni: Move resource initialization to separate function
Refactor the resource initialization in geni_i2c_probe() by introducing
a new geni_i2c_resources_init() function and utilizing the common
geni_se_resources_init() framework and clock frequency mapping, making the
probe function cleaner.
> diff --git a/drivers/i2c/busses/i2c-qcom-geni.c b/drivers/i2c/busses/i2c-qcom-geni.c
> index 3ee0be228d7c1..a7672b6c2bc07 100644
> --- a/drivers/i2c/busses/i2c-qcom-geni.c
> +++ b/drivers/i2c/busses/i2c-qcom-geni.c
[ ... ]
> @@ -1058,6 +1055,23 @@ static int geni_i2c_init(struct geni_i2c_dev *gi2c)
> return ret;
> }
>
> +static int geni_i2c_resources_init(struct geni_i2c_dev *gi2c)
> +{
> + int ret;
> +
> + ret = geni_se_resources_init(&gi2c->se);
[Severity: Critical]
Are we sure this function is available in the current tree? It appears
geni_se_resources_init() is undefined, which might cause a build failure.
> + if (ret)
> + return ret;
> +
> + ret = geni_i2c_clk_map_idx(gi2c);
> + if (ret)
> + return dev_err_probe(gi2c->se.dev, ret, "Invalid clk frequency %d Hz\n",
> + gi2c->clk_freq_out);
> +
> + return geni_icc_set_bw_ab(&gi2c->se, GENI_DEFAULT_BW, GENI_DEFAULT_BW,
> + Bps_to_icc(gi2c->clk_freq_out));
[Severity: Critical]
Is geni_icc_set_bw_ab() defined? Similar to the call above, it looks like this
is an unresolved symbol that could break the build.
> +}
> +
> static int geni_i2c_probe(struct platform_device *pdev)
> {
> struct geni_i2c_dev *gi2c;
[ ... ]
> @@ -1101,16 +1105,15 @@ static int geni_i2c_probe(struct platform_device *pdev)
> if (gi2c->irq < 0)
> return gi2c->irq;
>
> - ret = geni_i2c_clk_map_idx(gi2c);
> - if (ret)
> - return dev_err_probe(dev, ret, "Invalid clk frequency %d Hz\n",
> - gi2c->clk_freq_out);
> -
> gi2c->adap.algo = &geni_i2c_algo;
> init_completion(&gi2c->done);
> spin_lock_init(&gi2c->lock);
> platform_set_drvdata(pdev, gi2c);
>
> + ret = geni_i2c_resources_init(gi2c);
> + if (ret)
> + return ret;
> +
[Severity: High]
Does this probe function properly clean up resources if it fails later on?
If the subsequent call to i2c_add_adapter() fails later in geni_i2c_probe(),
the function returns without freeing the DMA channels that were allocated
during geni_i2c_init() via setup_gpi_dma(). Could this permanently leak the
GPI DMA channels?
[Severity: High]
Is there a risk of leaking power during probe error handling?
If geni_i2c_init() or i2c_add_adapter() fails, geni_i2c_probe() calls
pm_runtime_disable() immediately. However, geni_i2c_init() queues an async
pm_runtime_put() in its own error path. Since pm_runtime_disable() cancels
any pending async suspend work, does this leave the hardware indefinitely
powered on in the RPM_ACTIVE state?
[ ... ]
> @@ -1260,8 +1246,6 @@ static const struct dev_pm_ops geni_i2c_pm_ops = {
> };
>
> static const struct geni_i2c_desc i2c_master_hub = {
> - .has_core_clk = true,
> - .icc_ddr = NULL,
> .no_dma_support = true,
> .tx_fifo_depth = 16,
> };
[Severity: Critical]
Does this patch series introduce undefined power management functions here?
It appears the i2c_master_hub struct is assigned geni_se_resources_activate
and geni_se_resources_deactivate for its power management fields, but these
functions do not seem to be defined anywhere in the kernel tree.
[Severity: High]
This is a pre-existing issue, but does geni_i2c_remove() properly quiesce
the device before tearing down power management?
The geni_i2c_remove() function calls pm_runtime_disable() without a prior
synchronization barrier. If an asynchronous suspend is pending from a
recently completed I2C transaction, the immediate pm_runtime_disable() could
cancel it. Should a call to pm_runtime_put_sync() or
pm_runtime_force_suspend() be added to ensure it is fully suspended?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-enable-i2c-on-sa8255p-v7-0-ad736dbeab57@oss.qualcomm.com?part=3
^ 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