* Re: [PATCH] ARM: enable interrupts when arm_notify_die() is handling user mode errors
From: Sebastian Andrzej Siewior @ 2026-06-29 7:57 UTC (permalink / raw)
To: Xie Yuanbin
Cc: linux, rmk+kernel, clrkwllms, rostedt, linusw, arnd,
linux-arm-kernel, linux-kernel, linux-rt-devel, liaohua4,
lilinjie8
In-Reply-To: <20260626082953.47503-1-xieyuanbin1@huawei.com>
On 2026-06-26 16:29:53 [+0800], Xie Yuanbin wrote:
> My personal view is as follows: First, an Unhandled kernel fault indicates
> that the kernel has encountered an error, which is different from an
> Unhandled user fault. An Unhandled user fault can be artificially
> constructed by user programs, whereas a healthy kernel, in theory, should
> not trigger an Unhandled kernel fault.
correct.
> When a kernel has already encountered a fault, I think that printing fault
> information is more meaningful than improving the kernel's real-time
> performance. Imagine this: The interrupts enable here, and at the same
> time an interrupt arrives, and then another kernel error is triggered
> within the interrupt context. The log would be a disaster.
The problem is actually hitting that "error" spot before oops_enter().
In oops_enter() not only interrupts are disabled but also the emergency
mode of the console is enabled. That means all console output is written
via the atomic_write interface to the console as it the output happens.
At this point, your real-time guarantees are gone. There is no need to
worry about anything here since everything is lost.
> But no matter what, the above are merely my personal views,
> and I respect the maintainer's opinions.
Restricting it to user context is reasonable: Here you kill the task and
need enabled interrupts in order to send the signal.
For kernel threads it is not needed because the "printing" happens with
disabled interrupts anyway and without any buffering. Should this event
not end with panic() then it will kill the kernel thread and enable
interrupts before doing so.
Sebastian
^ permalink raw reply
* [PATCH v2] arm64: dts: amlogic: add some device nodes for A9
From: Xianwei Zhao via B4 Relay @ 2026-06-29 7:52 UTC (permalink / raw)
To: Neil Armstrong, Kevin Hilman, Jerome Brunet, Martin Blumenstingl,
Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-kernel, linux-amlogic, devicetree, linux-kernel,
Xianwei Zhao
From: Xianwei Zhao <xianwei.zhao@amlogic.com>
Add pinctrl and irqchip-gpio device nodes for A9 SoC.
Signed-off-by: Xianwei Zhao <xianwei.zhao@amlogic.com>
---
Add pinctrl and irqchip-gpio device node for A9.
---
Changes in v2:
- Reorder device node by address.
- Link to v1: https://lore.kernel.org/r/20260629-a9-node-v1-1-42ff6a0c16ab@amlogic.com
---
arch/arm64/boot/dts/amlogic/amlogic-a9.dtsi | 140 ++++++++++++++++++++++++++++
1 file changed, 140 insertions(+)
diff --git a/arch/arm64/boot/dts/amlogic/amlogic-a9.dtsi b/arch/arm64/boot/dts/amlogic/amlogic-a9.dtsi
index 660c8556a864..b0e0fadeed82 100644
--- a/arch/arm64/boot/dts/amlogic/amlogic-a9.dtsi
+++ b/arch/arm64/boot/dts/amlogic/amlogic-a9.dtsi
@@ -6,6 +6,7 @@
#include <dt-bindings/interrupt-controller/irq.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/pinctrl/amlogic,pinctrl.h>
/ {
interrupt-parent = <&gic>;
@@ -97,6 +98,95 @@ soc {
#size-cells = <2>;
ranges;
+ apb: bus@fe000000 {
+ compatible = "simple-bus";
+ reg = <0x0 0xfe000000 0x0 0x480000>;
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges = <0x0 0x0 0x0 0xfe000000 0x0 0x480000>;
+
+ periphs_pinctrl: pinctrl@4000 {
+ compatible = "amlogic,pinctrl-a9";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges = <0x0 0x0 0x0 0x4000 0x0 0x340>;
+
+ gpioz: gpio@c0 {
+ reg = <0 0xc0 0 0x20>, <0 0x18 0 0x8>;
+ reg-names = "gpio", "mux";
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-ranges = <&periphs_pinctrl 0 (AMLOGIC_GPIO_Z<<8) 16>;
+ };
+
+ gpiox: gpio@100 {
+ reg = <0 0x100 0 0x24>, <0 0xc 0 0x8>;
+ reg-names = "gpio", "mux";
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-ranges = <&periphs_pinctrl 0 (AMLOGIC_GPIO_X<<8) 18>;
+ };
+
+ gpioh: gpio@140 {
+ reg = <0 0x140 0 0x20>, <0 0x2c 0 0x4>;
+ reg-names = "gpio", "mux";
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-ranges = <&periphs_pinctrl 0 (AMLOGIC_GPIO_H<<8) 8>;
+ };
+
+ gpiom: gpio@1a0 {
+ reg = <0 0x1a0 0 0x20>, <0 0x20 0 0x4>;
+ reg-names = "gpio", "mux";
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-ranges = <&periphs_pinctrl 0 (AMLOGIC_GPIO_M<<8) 8>;
+ };
+
+ gpiob: gpio@240 {
+ reg = <0 0x240 0 0x20>, <0 0x0 0 0x8>;
+ reg-names = "gpio", "mux";
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-ranges = <&periphs_pinctrl 0 (AMLOGIC_GPIO_B<<8) 14>;
+ };
+
+ gpioa: gpio@280 {
+ reg = <0 0x280 0 0x24>, <0 0x40 0 0xc>;
+ reg-names = "gpio", "mux";
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-ranges = <&periphs_pinctrl 0 (AMLOGIC_GPIO_A<<8) 20>;
+ };
+
+ gpioy: gpio@2c0 {
+ reg = <0 0x2c0 0 0x20>, <0 0x30 0 0x8>;
+ reg-names = "gpio", "mux";
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-ranges = <&periphs_pinctrl 0 (AMLOGIC_GPIO_Y<<8) 10>;
+ };
+
+ gpiocc: gpio@300 {
+ reg = <0 0x300 0 0x20>, <0 0x14 0 0x4>;
+ reg-names = "gpio", "mux";
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-ranges = <&periphs_pinctrl 0 (AMLOGIC_GPIO_CC<<8) 2>;
+ };
+ };
+
+ gpio_intc: interrupt-controller@4080 {
+ compatible = "amlogic,a9-gpio-intc",
+ "amlogic,meson-gpio-intc";
+ reg = <0x0 0x4080 0x0 0x20>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ amlogic,channel-interrupts =
+ <10 11 12 13 14 15 16 17 18 19 20 21>;
+ };
+ };
+
gic: interrupt-controller@ff800000 {
compatible = "arm,gic-v3";
#interrupt-cells = <3>;
@@ -114,6 +204,56 @@ aobus: bus@ffa00000 {
#size-cells = <2>;
ranges = <0x0 0x0 0x0 0xffa00000 0x0 0x100000>;
+ aobus_pinctrl: pinctrl@4000 {
+ compatible = "amlogic,pinctrl-a9";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges = <0x0 0x0 0x0 0x4000 0x0 0x0e0>;
+
+ gpioao: gpio@1c {
+ reg = <0 0x1c 0 0x20>, <0 0x0 0 0x8>;
+ reg-names = "gpio", "mux";
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-ranges = <&aobus_pinctrl 0 (AMLOGIC_GPIO_AO<<8) 13>;
+ };
+
+ gpioc: gpio@3c {
+ reg = <0 0x3c 0 0x20>, <0 0x10 0 0x4>;
+ reg-names = "gpio", "mux";
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-ranges = <&aobus_pinctrl 0 (AMLOGIC_GPIO_C<<8) 7>;
+ };
+
+ gpiod: gpio@5c {
+ reg = <0 0x5c 0 0x24>, <0 0x8 0 0x8>;
+ reg-names = "gpio", "mux";
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-ranges = <&aobus_pinctrl 0 (AMLOGIC_GPIO_D<<8) 18>;
+ };
+
+ test_n: gpio@c0 {
+ reg = <0 0xc0 0 0x20>;
+ reg-names = "gpio";
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-ranges = <&aobus_pinctrl 0 (AMLOGIC_GPIO_TEST_N<<8) 1>;
+ };
+ };
+
+ gpio_ao_intc: interrupt-controller@4080 {
+ compatible = "amlogic,a9-gpio-ao-intc",
+ "amlogic,meson-gpio-intc";
+ reg = <0x0 0x4080 0x0 0x34>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ amlogic,channel-interrupts =
+ <384 385 386 387 388 389 390 391 392 393
+ 394 395 396 397 398 399 400 401 402 403>;
+ };
+
uart_b: serial@1e000 {
compatible = "amlogic,a9-uart",
"amlogic,meson-s4-uart";
---
base-commit: 3d5670d672ae08b8c534b7beed6f57c8b44e7b43
change-id: 20260629-a9-node-3e6fceba7c90
Best regards,
--
Xianwei Zhao <xianwei.zhao@amlogic.com>
^ permalink raw reply related
* Re: [PATCH v26 3/7] firmware: imx: add driver for NXP EdgeLock Enclave
From: Lothar Waßmann @ 2026-06-29 7:50 UTC (permalink / raw)
To: pankaj.gupta
Cc: Jonathan Corbet, Shuah Khan, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Frank Li, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Pankaj Gupta, linux-doc, linux-kernel, devicetree,
imx, linux-arm-kernel, Frieder Schrempf, kernel test robot,
sashiko-bot
In-Reply-To: <20260629-imx-se-if-v26-3-146446285744@nxp.com>
Hi,
On Mon, 29 Jun 2026 17:51:59 +0530 pankaj.gupta@oss.nxp.com wrote:
> From: Pankaj Gupta <pankaj.gupta@nxp.com>
>
> Add MU-based communication interface for secure enclave.
>
> NXP hardware IP(s) for secure-enclaves like Edgelock Enclave(ELE), are
> embedded in the SoC to support the features like HSM, SHE & V2X, using
> message based communication interface.
>
> The secure enclave FW communicates with Linux over single or multiple
> dedicated messaging unit(MU) based interface(s).
> Exists on i.MX SoC(s) like i.MX8ULP, i.MX93, i.MX95 etc.
>
> For i.MX9x SoC(s) there is at least one dedicated ELE MU(s) for each
> world - Linux(one or more) and OPTEE-OS (one or more).
>
[...]
> + ret = se_fill_cmd_msg_hdr(priv, (struct se_msg_hdr *)&tx_msg->header,
> + ELE_GET_INFO_REQ, ELE_GET_INFO_REQ_MSG_SZ,
> + true);
> + if (ret)
> + goto exit;
[...]
> +/* Fill a command message header with a given command ID and length in bytes. */
> +static inline int se_fill_cmd_msg_hdr(struct se_if_priv *priv, struct se_msg_hdr *hdr,
> + u8 cmd, u32 len, bool is_base_api)
> +{
> + hdr->tag = priv->if_defs->cmd_tag;
> + hdr->ver = (is_base_api) ? priv->if_defs->base_api_ver : priv->if_defs->fw_api_ver;
> + hdr->command = cmd;
> + hdr->size = len >> 2;
> +
> + return 0;
> +}
>
I don't see a point in having a function always return zero and
implement error checks that will never be used.
[...]
> +static const struct of_device_id se_match[] = {
> + { .compatible = "fsl,imx8ulp-se-ele-hsm", .data = &imx8ulp_se_ele_hsm},
> + { .compatible = "fsl,imx93-se-ele-hsm", .data = &imx93_se_ele_hsm},
> + {},
>
Since the last entry in this array must be a NULL entry, there should be
no comma after the {}. This will generate a compile error if (e.g. by
patch conflict resolution) an entry is added after the end marker.
Lothar Waßmann
^ permalink raw reply
* [PATCH v2 4/4] arm64: dts: freescale: Add common DTS overlay for IMX-AUD-IO add-on board
From: chancel.liu @ 2026-06-29 7:47 UTC (permalink / raw)
To: robh, krzk+dt, conor+dt, Frank.Li, s.hauer
Cc: kernel, festevam, devicetree, linux-kernel, imx, linux-arm-kernel
In-Reply-To: <20260629074734.3643227-1-chancel.liu@oss.nxp.com>
From: Chancel Liu <chancel.liu@nxp.com>
Add common DTS overlay for the IMX-AUD-IO add-on board[1] which
connects to the base board through a fsl,aud-io-slot connector.
This board features a CS42888 codec providing 2 microphone inputs, 2
line inputs and 6 channels audio output capability.
[1]https://www.nxp.com/part/IMX-AUD-IO
Signed-off-by: Chancel Liu <chancel.liu@nxp.com>
---
arch/arm64/boot/dts/freescale/Makefile | 4 ++
arch/arm64/boot/dts/freescale/imx-aud-io.dtso | 57 +++++++++++++++++++
2 files changed, 61 insertions(+)
create mode 100644 arch/arm64/boot/dts/freescale/imx-aud-io.dtso
diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index 8ddaab127ab9..f2417c20300c 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -672,11 +672,15 @@ dtb-$(CONFIG_ARCH_MXC) += imx95-15x15-evk-pcie0-ep.dtb
imx95-19x19-evk-pcie0-ep-dtbs += imx95-19x19-evk.dtb imx-pcie0-ep.dtbo
imx95-19x19-evk-pcie1-ep-dtbs += imx95-19x19-evk.dtb imx-pcie1-ep.dtbo
dtb-$(CONFIG_ARCH_MXC) += imx95-19x19-evk-pcie0-ep.dtb imx95-19x19-evk-pcie1-ep.dtb
+imx95-19x19-evk-aud-io-dtbs += imx95-19x19-evk.dtb imx-aud-io.dtbo
+dtb-$(CONFIG_ARCH_MXC) += imx95-19x19-evk-aud-io.dtb
dtb-$(CONFIG_ARCH_MXC) += imx95-libra-rdk-fpsc.dtb
dtb-$(CONFIG_ARCH_MXC) += imx95-19x19-verdin-evk.dtb
dtb-$(CONFIG_ARCH_MXC) += imx952-evk.dtb
+imx952-evk-aud-io-dtbs += imx952-evk.dtb imx-aud-io.dtbo
+dtb-$(CONFIG_ARCH_MXC) += imx952-evk-aud-io.dtb
imx8mm-kontron-dl-dtbs := imx8mm-kontron-bl.dtb imx8mm-kontron-dl.dtbo
imx8mm-kontron-bl-lte-dtbs := imx8mm-kontron-bl.dtb imx8mm-kontron-bl-lte.dtbo
diff --git a/arch/arm64/boot/dts/freescale/imx-aud-io.dtso b/arch/arm64/boot/dts/freescale/imx-aud-io.dtso
new file mode 100644
index 000000000000..311b7f984576
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx-aud-io.dtso
@@ -0,0 +1,57 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Common Device Tree for the IMX-AUD-IO add-on board[1].
+ * It connects to the base board through an AUD-IO slot.
+ *
+ * [1]https://www.nxp.com/part/IMX-AUD-IO
+ *
+ * Copyright 2026 NXP
+ */
+
+/dts-v1/;
+/plugin/;
+
+#include <dt-bindings/gpio/gpio.h>
+
+&{/} {
+ aud-io-sound-cs42888 {
+ compatible = "fsl,imx-audio-card";
+ model = "imx-cs42888";
+
+ pri-dai-link {
+ link-name = "cs42888";
+ format = "i2s";
+ fsl,mclk-equal-bclk;
+
+ codec {
+ sound-dai = <&audio_io_codec>;
+ };
+
+ cpu {
+ sound-dai = <&aud_io_cpu>;
+ };
+ };
+ };
+};
+
+&aud_io_i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ audio_io_codec: codec@48 {
+ compatible = "cirrus,cs42888";
+ reg = <0x48>;
+ clocks = <&aud_io_slot 0>;
+ clock-names = "mclk";
+ VA-supply = <&aud_io_reg_5v>;
+ VD-supply = <&aud_io_reg_3v3>;
+ VLS-supply = <&aud_io_reg_1v8>;
+ VLC-supply = <&aud_io_reg_1v8>;
+ #sound-dai-cells = <0>;
+ reset-gpios = <&aud_io_slot 0 GPIO_ACTIVE_LOW>;
+ };
+};
+
+&aud_io_cpu {
+ status = "okay";
+};
--
2.50.1
^ permalink raw reply related
* [PATCH v2 3/4] arm64: dts: freescale: imx952-evk: Add IMX-AUD-IO board support
From: chancel.liu @ 2026-06-29 7:47 UTC (permalink / raw)
To: robh, krzk+dt, conor+dt, Frank.Li, s.hauer
Cc: kernel, festevam, devicetree, linux-kernel, imx, linux-arm-kernel
In-Reply-To: <20260629074734.3643227-1-chancel.liu@oss.nxp.com>
From: Chancel Liu <chancel.liu@nxp.com>
IMX-AUD-IO is an add-on board which can be connected to i.MX952 EVK
through a physical connector. This connector is described as a
fsl,aud-io-slot connector to expose a constrained subset of GPIO and
clock resources to the add-on board using fixed electrical wiring.
Also add required regulator, sound CPU DAI and I2C bus configuration to
support IMX-AUD-IO on this base board.
Signed-off-by: Chancel Liu <chancel.liu@nxp.com>
---
arch/arm64/boot/dts/freescale/imx952-evk.dts | 70 +++++++++++++++++++-
1 file changed, 68 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/imx952-evk.dts b/arch/arm64/boot/dts/freescale/imx952-evk.dts
index 62d1c1c7c501..80480c802f19 100644
--- a/arch/arm64/boot/dts/freescale/imx952-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx952-evk.dts
@@ -43,6 +43,19 @@ aliases {
spi6 = &lpspi7;
};
+ aud_io_slot: connector {
+ compatible = "fsl,imx952-evk-aud-io", "fsl,imx95-19x19-evk-aud-io";
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-map = <0 0 &pcal6416 8 1>;
+ gpio-map-mask = <0xffff 0x0>;
+ /* Only pass through GPIO polarity flag bit 0. */
+ gpio-map-pass-thru = <0x0 0x1>;
+ #clock-cells = <1>;
+ clock-map = <0 &scmi_clk IMX952_CLK_SAI2>;
+ clock-map-mask = <0xff>;
+ };
+
bt_sco_codec: audio-codec-bt-sco {
#sound-dai-cells = <1>;
compatible = "linux,bt-sco";
@@ -114,13 +127,29 @@ reg_1p8v: regulator-1p8v {
regulator-name = "+V1.8_SW";
};
- reg_vref_1v8: regulator-adc-vref {
+ aud_io_reg_1v8: reg_vref_1v8: regulator-adc-vref {
compatible = "regulator-fixed";
regulator-name = "vref_1v8";
regulator-min-microvolt = <1800000>;
regulator-max-microvolt = <1800000>;
};
+ aud_io_reg_3v3: regulator-aud-io-3v3 {
+ compatible = "regulator-fixed";
+ regulator-name = "aud-io-3v3";
+ regulator-max-microvolt = <3300000>;
+ regulator-min-microvolt = <3300000>;
+ gpio = <&pcal6416 11 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ };
+
+ aud_io_reg_5v: regulator-aud-io-5v {
+ compatible = "regulator-fixed";
+ regulator-name = "aud-io-5v";
+ regulator-max-microvolt = <5000000>;
+ regulator-min-microvolt = <5000000>;
+ };
+
reg_audio_pwr: regulator-audio-pwr {
compatible = "regulator-fixed";
regulator-name = "audio-pwr";
@@ -323,7 +352,7 @@ i2c4_pcal6408: gpio@21 {
};
};
-&lpi2c6 {
+aud_io_i2c: &lpi2c6 {
clock-frequency = <100000>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_lpi2c6>;
@@ -468,6 +497,27 @@ &sai1 {
status = "okay";
};
+aud_io_cpu: &sai2 {
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_sai2>;
+ clocks = <&scmi_clk IMX952_CLK_BUSNETCMIX>, <&clk_dummy>,
+ <&scmi_clk IMX952_CLK_SAI2>, <&clk_dummy>,
+ <&clk_dummy>, <&scmi_clk IMX952_CLK_AUDIOPLL1>,
+ <&scmi_clk IMX952_CLK_AUDIOPLL2>;
+ clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3", "pll8k", "pll11k";
+ assigned-clocks = <&scmi_clk IMX952_CLK_AUDIOPLL1_VCO>,
+ <&scmi_clk IMX952_CLK_AUDIOPLL2_VCO>,
+ <&scmi_clk IMX952_CLK_AUDIOPLL1>,
+ <&scmi_clk IMX952_CLK_AUDIOPLL2>,
+ <&scmi_clk IMX952_CLK_SAI2>;
+ assigned-clock-parents = <0>, <0>, <0>, <0>,
+ <&scmi_clk IMX952_CLK_AUDIOPLL1>;
+ assigned-clock-rates = <3932160000>, <3612672000>,
+ <393216000>, <361267200>, <12288000>;
+ fsl,sai-mclk-direction-output;
+ fsl,sai-asynchronous;
+};
+
&sai3 {
assigned-clocks = <&scmi_clk IMX952_CLK_AUDIOPLL1_VCO>,
<&scmi_clk IMX952_CLK_AUDIOPLL2_VCO>,
@@ -688,6 +738,22 @@ IMX952_PAD_SAI1_TXD0__AONMIX_TOP_GPIO1_IO_13 0x51e
>;
};
+ pinctrl_sai2: sai2grp {
+ fsl,pins = <
+ IMX952_PAD_ENET2_MDIO__NETCMIX_TOP_SAI2_RX_BCLK 0x31e
+ IMX952_PAD_ENET2_MDC__NETCMIX_TOP_SAI2_RX_SYNC 0x31e
+ IMX952_PAD_ENET2_TD3__NETCMIX_TOP_SAI2_RX_DATA_0 0x31e
+ IMX952_PAD_ENET2_TD2__NETCMIX_TOP_SAI2_RX_DATA_1 0x31e
+ IMX952_PAD_ENET2_TXC__NETCMIX_TOP_SAI2_TX_BCLK 0x31e
+ IMX952_PAD_ENET2_TX_CTL__NETCMIX_TOP_SAI2_TX_SYNC 0x31e
+ IMX952_PAD_ENET2_RX_CTL__NETCMIX_TOP_SAI2_TX_DATA_0 0x31e
+ IMX952_PAD_ENET2_RXC__NETCMIX_TOP_SAI2_TX_DATA_1 0x31e
+ IMX952_PAD_ENET2_RD0__NETCMIX_TOP_SAI2_TX_DATA_2 0x31e
+ IMX952_PAD_ENET2_RD1__NETCMIX_TOP_SAI2_TX_DATA_3 0x31e
+ IMX952_PAD_ENET2_RD2__NETCMIX_TOP_SAI2_MCLK 0x31e
+ >;
+ };
+
pinctrl_sai3: sai3grp {
fsl,pins = <
IMX952_PAD_GPIO_IO17__WAKEUPMIX_TOP_SAI3_MCLK 0x31e
--
2.50.1
^ permalink raw reply related
* [PATCH v2 2/4] arm64: dts: freescale: imx95-19x19-evk: Add IMX-AUD-IO board support
From: chancel.liu @ 2026-06-29 7:47 UTC (permalink / raw)
To: robh, krzk+dt, conor+dt, Frank.Li, s.hauer
Cc: kernel, festevam, devicetree, linux-kernel, imx, linux-arm-kernel
In-Reply-To: <20260629074734.3643227-1-chancel.liu@oss.nxp.com>
From: Chancel Liu <chancel.liu@nxp.com>
IMX-AUD-IO is an add-on board which can be connected to i.MX95 19x19
EVK through a physical connector. This connector is described as a
fsl,aud-io-slot connector to expose a constrained subset of GPIO and
clock resources to the add-on board using fixed electrical wiring.
Also add required regulator, sound CPU DAI and I2C bus configuration to
support IMX-AUD-IO on this base board.
Signed-off-by: Chancel Liu <chancel.liu@nxp.com>
---
.../boot/dts/freescale/imx95-19x19-evk.dts | 55 ++++++++++++++++++-
1 file changed, 53 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts b/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts
index 2e463bc7c601..f5ca8edf74af 100644
--- a/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts
@@ -43,6 +43,19 @@ aliases {
serial4 = &lpuart5;
};
+ aud_io_slot: connector {
+ compatible = "fsl,imx95-19x19-evk-aud-io";
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-map = <0 0 &i2c6_pcal6416 8 1>;
+ gpio-map-mask = <0xffff 0x0>;
+ /* Only pass through GPIO polarity flag bit 0. */
+ gpio-map-pass-thru = <0x0 0x1>;
+ #clock-cells = <1>;
+ clock-map = <0 &scmi_clk IMX95_CLK_SAI2>;
+ clock-map-mask = <0xff>;
+ };
+
bt_sco_codec: audio-codec-bt-sco {
#sound-dai-cells = <1>;
compatible = "linux,bt-sco";
@@ -94,7 +107,7 @@ flexcan2_phy: can-phy1 {
standby-gpios = <&i2c4_gpio_expander_21 3 GPIO_ACTIVE_LOW>;
};
- reg_vref_1v8: regulator-1p8v {
+ aud_io_reg_1v8: reg_vref_1v8: regulator-1p8v {
compatible = "regulator-fixed";
regulator-max-microvolt = <1800000>;
regulator-min-microvolt = <1800000>;
@@ -108,6 +121,22 @@ reg_3p3v: regulator-3p3v {
regulator-name = "+V3.3_SW";
};
+ aud_io_reg_3v3: regulator-aud-io-3v3 {
+ compatible = "regulator-fixed";
+ regulator-name = "aud-io-3v3";
+ regulator-max-microvolt = <3300000>;
+ regulator-min-microvolt = <3300000>;
+ gpio = <&i2c6_pcal6416 11 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ };
+
+ aud_io_reg_5v: regulator-aud-io-5v {
+ compatible = "regulator-fixed";
+ regulator-name = "aud-io-5v";
+ regulator-max-microvolt = <5000000>;
+ regulator-min-microvolt = <5000000>;
+ };
+
reg_audio_pwr: regulator-audio-pwr {
compatible = "regulator-fixed";
regulator-name = "audio-pwr";
@@ -382,7 +411,7 @@ i2c5_pcal6408: gpio@21 {
};
};
-&lpi2c6 {
+aud_io_i2c: &lpi2c6 {
clock-frequency = <100000>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_lpi2c6>;
@@ -598,6 +627,28 @@ &sai1 {
status = "okay";
};
+aud_io_cpu: &sai2 {
+ #sound-dai-cells = <0>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_sai2>;
+ clocks = <&scmi_clk IMX95_CLK_BUSNETCMIX>, <&dummy>,
+ <&scmi_clk IMX95_CLK_SAI2>, <&dummy>,
+ <&dummy>, <&scmi_clk IMX95_CLK_AUDIOPLL1>,
+ <&scmi_clk IMX95_CLK_AUDIOPLL2>;
+ clock-names = "bus", "mclk0", "mclk1", "mclk2", "mclk3", "pll8k", "pll11k";
+ assigned-clocks = <&scmi_clk IMX95_CLK_AUDIOPLL1_VCO>,
+ <&scmi_clk IMX95_CLK_AUDIOPLL2_VCO>,
+ <&scmi_clk IMX95_CLK_AUDIOPLL1>,
+ <&scmi_clk IMX95_CLK_AUDIOPLL2>,
+ <&scmi_clk IMX95_CLK_SAI2>;
+ assigned-clock-parents = <0>, <0>, <0>, <0>,
+ <&scmi_clk IMX95_CLK_AUDIOPLL1>;
+ assigned-clock-rates = <3932160000>, <3612672000>,
+ <393216000>, <361267200>, <12288000>;
+ fsl,sai-mclk-direction-output;
+ fsl,sai-asynchronous;
+};
+
&sai3 {
#sound-dai-cells = <0>;
pinctrl-names = "default";
--
2.50.1
^ permalink raw reply related
* [PATCH v2 1/4] dt-bindings: connector: Add fsl,aud-io-slot binding
From: chancel.liu @ 2026-06-29 7:47 UTC (permalink / raw)
To: robh, krzk+dt, conor+dt, Frank.Li, s.hauer
Cc: kernel, festevam, devicetree, linux-kernel, imx, linux-arm-kernel
In-Reply-To: <20260629074734.3643227-1-chancel.liu@oss.nxp.com>
From: Chancel Liu <chancel.liu@nxp.com>
The NXP AUD-IO slot represents a physically present I/O connector on
the base board. It acts as a nexus that exposes a constrained set of
I/O resources, such as GPIOs, clocks and interrupts, through fixed
electrical wiring. All actual hardware providers reside on the base
board. The connector node only defines index-based mappings to those
providers.
This connector type is present on i.MX95 19x19 EVK and i.MX952 EVK,
where it is used to attach the IMX-AUD-IO audio expansion card[1]. The
same add-on board can be reused across different base boards that carry
this connector.
[1]https://www.nxp.com/part/IMX-AUD-IO
Signed-off-by: Chancel Liu <chancel.liu@nxp.com>
---
.../bindings/connector/fsl,aud-io-slot.yaml | 113 ++++++++++++++++++
1 file changed, 113 insertions(+)
create mode 100644 Documentation/devicetree/bindings/connector/fsl,aud-io-slot.yaml
diff --git a/Documentation/devicetree/bindings/connector/fsl,aud-io-slot.yaml b/Documentation/devicetree/bindings/connector/fsl,aud-io-slot.yaml
new file mode 100644
index 000000000000..5085574d221b
--- /dev/null
+++ b/Documentation/devicetree/bindings/connector/fsl,aud-io-slot.yaml
@@ -0,0 +1,113 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/connector/fsl,aud-io-slot.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP AUD-IO Slot
+
+maintainers:
+ - Frank Li <Frank.li@nxp.com>
+ - Chancel Liu <chancel.liu@nxp.com>
+
+description:
+ The NXP AUD-IO slot represents a physically present I/O connector on
+ the base board. It acts as a nexus that exposes a constrained set of
+ I/O resources, such as GPIOs, clocks and interrupts, through fixed
+ electrical wiring. All actual hardware providers reside on the base
+ board. The connector node only defines index-based mappings to those
+ providers. This connector type is present on i.MX95 19x19 EVK and
+ i.MX952 EVK, where it is used to attach the IMX-AUD-IO expansion card.
+
+properties:
+ compatible:
+ oneOf:
+ - items:
+ - enum:
+ - fsl,imx952-evk-aud-io
+ - const: fsl,imx95-19x19-evk-aud-io
+ - const: fsl,imx95-19x19-evk-aud-io
+
+ gpio-controller: true
+
+ '#gpio-cells':
+ const: 2
+
+ gpio-map:
+ minItems: 1
+ maxItems: 32
+
+ gpio-map-mask:
+ items:
+ - const: 0xffff
+ - const: 0x0
+
+ gpio-map-pass-thru:
+ items:
+ - const: 0x0
+ - const: 0x1
+
+ '#clock-cells':
+ const: 1
+
+ clock-map:
+ minItems: 1
+ maxItems: 16
+
+ clock-map-mask:
+ items:
+ - const: 0xff
+
+ clock-map-pass-thru: true
+
+ '#address-cells':
+ const: 0
+
+ interrupt-controller: true
+
+ '#interrupt-cells':
+ const: 2
+
+ interrupt-map:
+ minItems: 1
+ maxItems: 32
+
+ interrupt-map-mask:
+ items:
+ - const: 0xffff
+ - const: 0x0
+
+allOf:
+ - $ref: /schemas/gpio/gpio-nexus-node.yaml#
+ - $ref: /schemas/clock/clock-nexus-node.yaml#
+
+required:
+ - compatible
+ - gpio-controller
+ - '#gpio-cells'
+ - gpio-map
+ - gpio-map-mask
+ - gpio-map-pass-thru
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ connector {
+ compatible = "fsl,imx95-19x19-evk-aud-io";
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-map = <0 0 &gpio1 8 1>;
+ gpio-map-mask = <0xffff 0x0>;
+ gpio-map-pass-thru = <0x0 0x1>;
+ #clock-cells = <1>;
+ clock-map = <0 &clk 1>;
+ clock-map-mask = <0xff>;
+ #address-cells = <0>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ interrupt-map-mask = <0xffff 0x0>;
+ interrupt-map = <0 0 &gpio2 27 IRQ_TYPE_LEVEL_LOW>;
+ };
--
2.50.1
^ permalink raw reply related
* [PATCH v26 6/7] arm64: dts: imx8ulp: add secure enclave node
From: pankaj.gupta @ 2026-06-29 12:22 UTC (permalink / raw)
To: Jonathan Corbet, Shuah Khan, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Frank Li, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Pankaj Gupta
Cc: linux-doc, linux-kernel, devicetree, imx, linux-arm-kernel
In-Reply-To: <20260629-imx-se-if-v26-0-146446285744@nxp.com>
From: Pankaj Gupta <pankaj.gupta@nxp.com>
Add support for NXP secure enclave called EdgeLock Enclave firmware (se-fw)
for imx8ulp-evk.
Add label sram0 for sram@2201f000 and add secure-enclave node
Signed-off-by: Pankaj Gupta <pankaj.gupta@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
arch/arm64/boot/dts/freescale/imx8ulp.dtsi | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
index c6d1bb9edf38..933127f870c6 100644
--- a/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8ulp.dtsi
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
/*
- * Copyright 2021 NXP
+ * Copyright 2021, 2025 NXP
*/
#include <dt-bindings/clock/imx8ulp-clock.h>
@@ -153,7 +153,7 @@ sosc: clock-sosc {
#clock-cells = <0>;
};
- sram@2201f000 {
+ sram0: sram@2201f000 {
compatible = "mmio-sram";
reg = <0x0 0x2201f000 0x0 0x1000>;
@@ -185,6 +185,13 @@ scmi_sensor: protocol@15 {
#thermal-sensor-cells = <1>;
};
};
+
+ hsm0: secure-enclave {
+ compatible = "fsl,imx8ulp-se-ele-hsm";
+ mbox-names = "tx", "rx";
+ mboxes = <&s4muap 0 0>, <&s4muap 1 0>;
+ sram = <&sram0>;
+ };
};
cm33: remoteproc-cm33 {
--
2.43.0
^ permalink raw reply related
* [PATCH v2 0/4] arm64: dts: freescale: Add IMX-AUD-IO add-on board support
From: chancel.liu @ 2026-06-29 7:47 UTC (permalink / raw)
To: robh, krzk+dt, conor+dt, Frank.Li, s.hauer
Cc: kernel, festevam, devicetree, linux-kernel, imx, linux-arm-kernel
From: Chancel Liu <chancel.liu@nxp.com>
This patch series adds support for the IMX-AUD-IO[1] audio expansion
card on the i.MX95 19x19 EVK and i.MX952 EVK platforms.
A connector is present on the base board to attach the IMX-AUD-IO card.
This connector is described by the fsl,aud-io-slot binding. It acts as
a nexus that exposes a constrained set of I/O resources, such as GPIOs,
clocks and interrupts, through fixed electrical wiring. All actual
hardware providers reside on the base board.
Also update the EVK base Device Trees to describe the connector and
required board-level resources. Add a DTS overlay to enable the
IMX-AUD-IO board.
The approach used here is inspired by the discussion[2] where a
connector is described as a nexus mapping board-level resources. One
important difference in this case is that the IMX-AUD-IO add-on board
is not a hot-pluggable device. The board is connected to the connector
before boot and remains present for the lifetime of the system. As a
result, the connector is modeled to describe fixed, boot-time hardware
wiring rather than runtime attachment or hot-plug handling.
This series depends on Miquel Raynal's clock nexus binding/core support[3].
In particular, Documentation/devicetree/bindings/clock/clock-nexus-node.yaml
must be available for dt_binding_check. This series does not include
these changes to avoid duplication.
[1]https://www.nxp.com/part/IMX-AUD-IO
[2]https://lore.kernel.org/linux-devicetree/20250902105710.00512c6d@booty/
[3]https://lore.kernel.org/all/20260327-schneider-v7-0-rc1-crypto-v1-10-5e6ff7853994@bootlin.com/
Changes in v2:
- Drop [PATCH 1/5] clk: Add support for clock nexus dt bindings. Same
change is already present in Miquel Raynal's ongoing series
- Drop PWM properties from binding. This connector carries no PWM signals
- Rename compatible from fsl,io-connector to board-specific compatibles:
fsl,imx95-19x19-evk-aud-io and fsl,imx952-evk-aud-io. The i.MX952 EVK
uses fsl,imx95-19x19-evk-aud-io as fallback
- Add property constraints instead of redefining common types
- Fix inconsistent quote style for '#interrupt-cells' in binding
- Add required GPIO nexus properties and reference gpio-nexus-node.yaml
and clock-nexus-node.yaml
- Make gpio-map-pass-thru a const value to pass through only polarity
bit (bit 0)
- Modify dts accordingly and some commit messages
Chancel Liu (4):
dt-bindings: connector: Add fsl,aud-io-slot binding
arm64: dts: freescale: imx95-19x19-evk: Add IMX-AUD-IO board support
arm64: dts: freescale: imx952-evk: Add IMX-AUD-IO board support
arm64: dts: freescale: Add common DTS overlay for IMX-AUD-IO add-on
board
.../bindings/connector/fsl,aud-io-slot.yaml | 113 ++++++++++++++++++
arch/arm64/boot/dts/freescale/Makefile | 4 +
arch/arm64/boot/dts/freescale/imx-aud-io.dtso | 57 +++++++++
.../boot/dts/freescale/imx95-19x19-evk.dts | 55 ++++++++-
arch/arm64/boot/dts/freescale/imx952-evk.dts | 70 ++++++++++-
5 files changed, 295 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/connector/fsl,aud-io-slot.yaml
create mode 100644 arch/arm64/boot/dts/freescale/imx-aud-io.dtso
--
2.50.1
^ permalink raw reply
* [PATCH v26 2/7] dt-bindings: arm: fsl: add imx-se-fw binding doc
From: pankaj.gupta @ 2026-06-29 12:21 UTC (permalink / raw)
To: Jonathan Corbet, Shuah Khan, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Frank Li, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Pankaj Gupta
Cc: linux-doc, linux-kernel, devicetree, imx, linux-arm-kernel
In-Reply-To: <20260629-imx-se-if-v26-0-146446285744@nxp.com>
From: Pankaj Gupta <pankaj.gupta@nxp.com>
The NXP security hardware IP(s) like: i.MX EdgeLock Enclave, V2X etc.,
creates an embedded secure enclave within the SoC boundary to enable
features like:
- HSM
- SHE
- V2X
Secure-Enclave(s) communication interface are typically via message
unit, i.e., based on mailbox linux kernel driver. This driver enables
communication ensuring well defined message sequence protocol between
Application Core and enclave's firmware.
Driver configures multiple misc-device on the MU, for multiple
user-space applications, to be able to communicate over single MU.
It exists on some i.MX processors. e.g. i.MX8ULP, i.MX93 etc.
Signed-off-by: Pankaj Gupta <pankaj.gupta@nxp.com>
Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
.../devicetree/bindings/firmware/fsl,imx-se.yaml | 91 ++++++++++++++++++++++
1 file changed, 91 insertions(+)
diff --git a/Documentation/devicetree/bindings/firmware/fsl,imx-se.yaml b/Documentation/devicetree/bindings/firmware/fsl,imx-se.yaml
new file mode 100644
index 000000000000..fa81adbf9b80
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/fsl,imx-se.yaml
@@ -0,0 +1,91 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/firmware/fsl,imx-se.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP i.MX HW Secure Enclave(s) EdgeLock Enclave
+
+maintainers:
+ - Pankaj Gupta <pankaj.gupta@nxp.com>
+
+description: |
+ NXP's SoC may contain one or multiple embedded secure-enclave HW
+ IP(s) like i.MX EdgeLock Enclave, V2X etc. These NXP's HW IP(s)
+ enables features like
+ - Hardware Security Module (HSM),
+ - Security Hardware Extension (SHE), and
+ - Vehicular to Anything (V2X)
+
+ Communication interface to the secure-enclaves(se) is based on the
+ messaging unit(s).
+
+properties:
+ compatible:
+ enum:
+ - fsl,imx8ulp-se-ele-hsm
+ - fsl,imx93-se-ele-hsm
+ - fsl,imx95-se-ele-hsm
+
+ mboxes:
+ items:
+ - description: mailbox phandle to send message to se firmware
+ - description: mailbox phandle to receive message from se firmware
+
+ mbox-names:
+ items:
+ - const: tx
+ - const: rx
+
+ memory-region:
+ maxItems: 1
+
+ sram:
+ maxItems: 1
+
+required:
+ - compatible
+ - mboxes
+ - mbox-names
+
+allOf:
+ # memory-region
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - fsl,imx8ulp-se-ele-hsm
+ - fsl,imx93-se-ele-hsm
+ then:
+ required:
+ - memory-region
+ else:
+ properties:
+ memory-region: false
+
+ # sram
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - fsl,imx8ulp-se-ele-hsm
+ then:
+ required:
+ - sram
+
+ else:
+ properties:
+ sram: false
+
+additionalProperties: false
+
+examples:
+ - |
+ secure-enclave {
+ compatible = "fsl,imx95-se-ele-hsm";
+ mboxes = <&ele_mu0 0 0>, <&ele_mu0 1 0>;
+ mbox-names = "tx", "rx";
+ };
+...
--
2.43.0
^ permalink raw reply related
* [PATCH 5/5] media: nxp: imx8-isi: Add additional 32-bit RGB format support
From: Guoniu Zhou @ 2026-06-29 7:44 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Christian Hemp,
Stefan Riedmueller, Jacopo Mondi
Cc: Dong Aisheng, Guoniu Zhou, linux-media, imx, linux-arm-kernel,
linux-kernel, Guoniu Zhou, Robert Chiras
In-Reply-To: <20260629-isi-v1-0-deebfdb1b07b@oss.nxp.com>
Add support for additional 32-bit RGB pixel formats (BGRA32, RGBA32,
BGRX32, RGBX32, ARGB2101010) and extend existing ABGR32 format with
full memory-to-memory capabilities to meet Android requirements.
All formats support capture, M2M input, and M2M output operations,
enabling complete format conversion pipelines.
Signed-off-by: Guoniu Zhou <guoniu.zhou@oss.nxp.com>
Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
---
.../media/platform/nxp/imx8-isi/imx8-isi-video.c | 59 +++++++++++++++++++++-
1 file changed, 58 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
index 05b51b98344b..ef638af350fe 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
@@ -160,12 +160,69 @@ static const struct mxc_isi_format_info mxc_isi_formats[] = {
}, {
.mbus_code = MEDIA_BUS_FMT_RGB888_1X24,
.fourcc = V4L2_PIX_FMT_ABGR32,
- .type = MXC_ISI_VIDEO_CAP | MXC_ISI_VIDEO_M2M_CAP,
+ .type = MXC_ISI_VIDEO_CAP | MXC_ISI_VIDEO_M2M_OUT
+ | MXC_ISI_VIDEO_M2M_CAP,
+ .isi_in_format = CHNL_MEM_RD_CTRL_IMG_TYPE_XRGB8,
.isi_out_format = CHNL_IMG_CTRL_FORMAT_ARGB8888,
.mem_planes = 1,
.color_planes = 1,
.depth = { 32 },
.encoding = MXC_ISI_ENC_RGB,
+ }, {
+ .mbus_code = MEDIA_BUS_FMT_RGB888_1X24,
+ .fourcc = V4L2_PIX_FMT_BGRA32,
+ .type = MXC_ISI_VIDEO_CAP | MXC_ISI_VIDEO_M2M_OUT
+ | MXC_ISI_VIDEO_M2M_CAP,
+ .isi_in_format = CHNL_MEM_RD_CTRL_IMG_TYPE_RGBX8,
+ .isi_out_format = CHNL_IMG_CTRL_FORMAT_RGBA8888,
+ .mem_planes = 1,
+ .color_planes = 1,
+ .depth = { 32 },
+ .encoding = MXC_ISI_ENC_RGB,
+ }, {
+ .mbus_code = MEDIA_BUS_FMT_RGB888_1X24,
+ .fourcc = V4L2_PIX_FMT_RGBA32,
+ .type = MXC_ISI_VIDEO_CAP | MXC_ISI_VIDEO_M2M_OUT
+ | MXC_ISI_VIDEO_M2M_CAP,
+ .isi_in_format = CHNL_MEM_RD_CTRL_IMG_TYPE_XBGR8,
+ .isi_out_format = CHNL_IMG_CTRL_FORMAT_ABGR8888,
+ .mem_planes = 1,
+ .color_planes = 1,
+ .depth = { 32 },
+ .encoding = MXC_ISI_ENC_RGB,
+ }, {
+ .mbus_code = MEDIA_BUS_FMT_RGB888_1X24,
+ .fourcc = V4L2_PIX_FMT_BGRX32,
+ .type = MXC_ISI_VIDEO_CAP | MXC_ISI_VIDEO_M2M_OUT
+ | MXC_ISI_VIDEO_M2M_CAP,
+ .isi_in_format = CHNL_MEM_RD_CTRL_IMG_TYPE_RGBX8,
+ .isi_out_format = CHNL_IMG_CTRL_FORMAT_RGBX888,
+ .mem_planes = 1,
+ .color_planes = 1,
+ .depth = { 32 },
+ .encoding = MXC_ISI_ENC_RGB,
+ }, {
+ .mbus_code = MEDIA_BUS_FMT_RGB888_1X24,
+ .fourcc = V4L2_PIX_FMT_RGBX32,
+ .type = MXC_ISI_VIDEO_CAP | MXC_ISI_VIDEO_M2M_OUT
+ | MXC_ISI_VIDEO_M2M_CAP,
+ .isi_in_format = CHNL_MEM_RD_CTRL_IMG_TYPE_XBGR8,
+ .isi_out_format = CHNL_IMG_CTRL_FORMAT_XBGR888,
+ .mem_planes = 1,
+ .color_planes = 1,
+ .depth = { 32 },
+ .encoding = MXC_ISI_ENC_RGB,
+ }, {
+ .mbus_code = MEDIA_BUS_FMT_RGB888_1X24,
+ .fourcc = V4L2_PIX_FMT_ARGB2101010,
+ .type = MXC_ISI_VIDEO_CAP | MXC_ISI_VIDEO_M2M_OUT
+ | MXC_ISI_VIDEO_M2M_CAP,
+ .isi_in_format = CHNL_MEM_RD_CTRL_IMG_TYPE_A2RGB10,
+ .isi_out_format = CHNL_IMG_CTRL_FORMAT_A2RGB10,
+ .mem_planes = 1,
+ .color_planes = 1,
+ .depth = { 32 },
+ .encoding = MXC_ISI_ENC_RGB,
},
/*
* RAW formats
--
2.34.1
^ permalink raw reply related
* [PATCH 4/5] media: nxp: imx8-isi: Correct color map between V4L2 and ISI
From: Guoniu Zhou @ 2026-06-29 7:44 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Christian Hemp,
Stefan Riedmueller, Jacopo Mondi
Cc: Dong Aisheng, Guoniu Zhou, linux-media, imx, linux-arm-kernel,
linux-kernel, Guoniu Zhou, stable
In-Reply-To: <20260629-isi-v1-0-deebfdb1b07b@oss.nxp.com>
Correct color map between V4L2_PIX_FMT_XBGR32 and ISI input
format XRGB8 when ISI works at memory to memory mode.
Fixes: cf21f328fcaf ("media: nxp: Add i.MX8 ISI driver")
Cc: stable@vger.kernel.org
Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
---
drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
index 5eb448f4c26f..05b51b98344b 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
@@ -151,7 +151,7 @@ static const struct mxc_isi_format_info mxc_isi_formats[] = {
.fourcc = V4L2_PIX_FMT_XBGR32,
.type = MXC_ISI_VIDEO_CAP | MXC_ISI_VIDEO_M2M_OUT
| MXC_ISI_VIDEO_M2M_CAP,
- .isi_in_format = CHNL_MEM_RD_CTRL_IMG_TYPE_XBGR8,
+ .isi_in_format = CHNL_MEM_RD_CTRL_IMG_TYPE_XRGB8,
.isi_out_format = CHNL_IMG_CTRL_FORMAT_XRGB888,
.mem_planes = 1,
.color_planes = 1,
--
2.34.1
^ permalink raw reply related
* [PATCH 3/5] media: nxp: imx8-isi: Add 16-bit raw Bayer format support
From: guoniu.zhou @ 2026-06-29 7:44 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Christian Hemp,
Stefan Riedmueller, Jacopo Mondi
Cc: Dong Aisheng, Guoniu Zhou, linux-media, imx, linux-arm-kernel,
linux-kernel, Guoniu Zhou, Laurentiu Palcu
In-Reply-To: <20260629-isi-v1-0-deebfdb1b07b@oss.nxp.com>
From: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
Add support for 16-bit raw Bayer formats (SBGGR16, SGBRG16, SGRBG16,
SRGGB16) to both the pipeline subdev and video capture interface.
These formats are commonly used by high-end image sensors that output
16-bit raw data, enabling the ISI to process and capture full dynamic
range from such sensors.
Signed-off-by: Laurentiu Palcu <laurentiu.palcu@oss.nxp.com>
Signed-off-by: Guoniu Zhou <guoniu.zhou@oss.nxp.com>
---
.../media/platform/nxp/imx8-isi/imx8-isi-pipe.c | 24 +++++++++++++++
.../media/platform/nxp/imx8-isi/imx8-isi-video.c | 36 ++++++++++++++++++++++
2 files changed, 60 insertions(+)
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c
index 2d0843c86534..e58925d71164 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-pipe.c
@@ -179,6 +179,30 @@ static const struct mxc_isi_bus_format_info mxc_isi_bus_formats[] = {
.pads = BIT(MXC_ISI_PIPE_PAD_SINK)
| BIT(MXC_ISI_PIPE_PAD_SOURCE),
.encoding = MXC_ISI_ENC_RAW,
+ }, {
+ .mbus_code = MEDIA_BUS_FMT_SBGGR16_1X16,
+ .output = MEDIA_BUS_FMT_SBGGR16_1X16,
+ .pads = BIT(MXC_ISI_PIPE_PAD_SINK)
+ | BIT(MXC_ISI_PIPE_PAD_SOURCE),
+ .encoding = MXC_ISI_ENC_RAW,
+ }, {
+ .mbus_code = MEDIA_BUS_FMT_SGBRG16_1X16,
+ .output = MEDIA_BUS_FMT_SGBRG16_1X16,
+ .pads = BIT(MXC_ISI_PIPE_PAD_SINK)
+ | BIT(MXC_ISI_PIPE_PAD_SOURCE),
+ .encoding = MXC_ISI_ENC_RAW,
+ }, {
+ .mbus_code = MEDIA_BUS_FMT_SGRBG16_1X16,
+ .output = MEDIA_BUS_FMT_SGRBG16_1X16,
+ .pads = BIT(MXC_ISI_PIPE_PAD_SINK)
+ | BIT(MXC_ISI_PIPE_PAD_SOURCE),
+ .encoding = MXC_ISI_ENC_RAW,
+ }, {
+ .mbus_code = MEDIA_BUS_FMT_SRGGB16_1X16,
+ .output = MEDIA_BUS_FMT_SRGGB16_1X16,
+ .pads = BIT(MXC_ISI_PIPE_PAD_SINK)
+ | BIT(MXC_ISI_PIPE_PAD_SOURCE),
+ .encoding = MXC_ISI_ENC_RAW,
},
/* JPEG */
{
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
index fe4adfa3a1f0..5eb448f4c26f 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-video.c
@@ -356,6 +356,42 @@ static const struct mxc_isi_format_info mxc_isi_formats[] = {
.color_planes = 1,
.depth = { 16 },
.encoding = MXC_ISI_ENC_RAW,
+ }, {
+ .mbus_code = MEDIA_BUS_FMT_SBGGR16_1X16,
+ .fourcc = V4L2_PIX_FMT_SBGGR16,
+ .type = MXC_ISI_VIDEO_CAP,
+ .isi_out_format = CHNL_IMG_CTRL_FORMAT_RAW16,
+ .mem_planes = 1,
+ .color_planes = 1,
+ .depth = { 16 },
+ .encoding = MXC_ISI_ENC_RAW,
+ }, {
+ .mbus_code = MEDIA_BUS_FMT_SGBRG16_1X16,
+ .fourcc = V4L2_PIX_FMT_SGBRG16,
+ .type = MXC_ISI_VIDEO_CAP,
+ .isi_out_format = CHNL_IMG_CTRL_FORMAT_RAW16,
+ .mem_planes = 1,
+ .color_planes = 1,
+ .depth = { 16 },
+ .encoding = MXC_ISI_ENC_RAW,
+ }, {
+ .mbus_code = MEDIA_BUS_FMT_SGRBG16_1X16,
+ .fourcc = V4L2_PIX_FMT_SGRBG16,
+ .type = MXC_ISI_VIDEO_CAP,
+ .isi_out_format = CHNL_IMG_CTRL_FORMAT_RAW16,
+ .mem_planes = 1,
+ .color_planes = 1,
+ .depth = { 16 },
+ .encoding = MXC_ISI_ENC_RAW,
+ }, {
+ .mbus_code = MEDIA_BUS_FMT_SRGGB16_1X16,
+ .fourcc = V4L2_PIX_FMT_SRGGB16,
+ .type = MXC_ISI_VIDEO_CAP,
+ .isi_out_format = CHNL_IMG_CTRL_FORMAT_RAW16,
+ .mem_planes = 1,
+ .color_planes = 1,
+ .depth = { 16 },
+ .encoding = MXC_ISI_ENC_RAW,
},
/* JPEG */
{
--
2.34.1
^ permalink raw reply related
* [PATCH 2/5] media: nxp: imx8-isi: Fix per-stream reference counting for multiplexed streams
From: Guoniu Zhou @ 2026-06-29 7:44 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Christian Hemp,
Stefan Riedmueller, Jacopo Mondi
Cc: Dong Aisheng, Guoniu Zhou, linux-media, imx, linux-arm-kernel,
linux-kernel, Guoniu Zhou, stable
In-Reply-To: <20260629-isi-v1-0-deebfdb1b07b@oss.nxp.com>
The ISI crossbar fails to properly enable multiple streams from different
virtual channels on the same input pad. Only the first stream gets enabled
in hardware, subsequent streams are silently ignored.
The driver uses a single enable_count per input to track the input state.
When enable_count is non-zero, the code assumes the input is already active
and skips calling v4l2_subdev_enable_streams() for additional streams:
Call 1: enable_streams(stream 0)
-> enable_count == 0, enable gasket and stream 0 in hardware
-> enable_count = 1
Call 2: enable_streams(stream 1)
-> enable_count == 1, skip hardware enable (BUG!)
-> enable_count = 2
-> stream 1 never gets enabled
Similarly on disable, when enable_count reaches zero, ALL streams are
disabled regardless of which streams are actually still active.
Fix this by tracking per-stream state using:
- enabled_streams (u64 bitmask): tracks which streams are currently enabled
- enabled_count[] (array): per-stream reference counter to support the same
stream being enabled/disabled multiple times
Now each stream is independently enabled/disabled in hardware based on the
enabled_streams bitmask, while enabled_count[] provides reference counting
for scenarios where the same stream is enabled multiple times, such as
duplicate cases in the ISI stream.
Fixes: cf21f328fcaf ("media: nxp: Add i.MX8 ISI driver")
Cc: stable@vger.kernel.org
Signed-off-by: Guoniu Zhou <guoniu.zhou@oss.nxp.com>
---
.../media/platform/nxp/imx8-isi/imx8-isi-core.h | 4 +-
.../platform/nxp/imx8-isi/imx8-isi-crossbar.c | 121 +++++++++++++++++----
2 files changed, 104 insertions(+), 21 deletions(-)
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
index 7547a6559d4c..bb2cfba27e20 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-core.h
@@ -185,7 +185,9 @@ struct mxc_isi_dma_buffer {
};
struct mxc_isi_input {
- unsigned int enable_count;
+ u64 enabled_streams;
+ /* Counter per stream */
+ unsigned int *enabled_count;
};
struct mxc_isi_crossbar {
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
index 29f14d30dbbb..a4a063c60c76 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
@@ -345,6 +345,8 @@ static int mxc_isi_crossbar_enable_streams(struct v4l2_subdev *sd,
struct mxc_isi_crossbar *xbar = to_isi_crossbar(sd);
struct v4l2_subdev *remote_sd;
struct mxc_isi_input *input;
+ u64 streams_to_enable;
+ unsigned long stream;
u64 sink_streams;
u32 sink_pad;
u32 remote_pad;
@@ -358,30 +360,47 @@ static int mxc_isi_crossbar_enable_streams(struct v4l2_subdev *sd,
input = &xbar->inputs[sink_pad];
- /*
- * TODO: Track per-stream enable counts to support multiplexed
- * streams.
- */
- if (!input->enable_count) {
+ if (!input->enabled_streams) {
ret = mxc_isi_crossbar_gasket_enable(xbar, state, remote_sd,
remote_pad, sink_pad);
if (ret)
return ret;
+ }
+
+ /*
+ * Track per-stream enable counts to support multiplexed streams.
+ * Only enable streams that are not already enabled.
+ */
+ streams_to_enable = sink_streams & ~input->enabled_streams;
+ if (streams_to_enable) {
ret = v4l2_subdev_enable_streams(remote_sd, remote_pad,
- sink_streams);
+ streams_to_enable);
if (ret) {
dev_err(xbar->isi->dev,
"failed to enable streams 0x%llx on '%s':%u: %d\n",
- sink_streams, remote_sd->name, remote_pad, ret);
- mxc_isi_crossbar_gasket_disable(xbar, sink_pad);
- return ret;
+ streams_to_enable, remote_sd->name, remote_pad, ret);
+ goto err_gasket_disable;
}
+
+ input->enabled_streams |= streams_to_enable;
}
- input->enable_count++;
+ /* Increment reference count for all requested streams */
+ for (stream = 0; stream < xbar->num_sources; stream++) {
+ if (!(sink_streams & BIT(stream)))
+ continue;
+
+ input->enabled_count[stream]++;
+ }
return 0;
+
+err_gasket_disable:
+ if (!input->enabled_streams)
+ mxc_isi_crossbar_gasket_disable(xbar, sink_pad);
+
+ return ret;
}
static int mxc_isi_crossbar_disable_streams(struct v4l2_subdev *sd,
@@ -391,6 +410,8 @@ static int mxc_isi_crossbar_disable_streams(struct v4l2_subdev *sd,
struct mxc_isi_crossbar *xbar = to_isi_crossbar(sd);
struct v4l2_subdev *remote_sd;
struct mxc_isi_input *input;
+ u64 streams_to_disable = 0;
+ unsigned long stream;
u64 sink_streams;
u32 sink_pad;
u32 remote_pad;
@@ -404,19 +425,36 @@ static int mxc_isi_crossbar_disable_streams(struct v4l2_subdev *sd,
input = &xbar->inputs[sink_pad];
- input->enable_count--;
+ /*
+ * Decrease the enable count for each stream. Only disable streams
+ * whose count reaches zero.
+ */
+ for (stream = 0; stream < xbar->num_sources; stream++) {
+ if (!(sink_streams & BIT(stream)))
+ continue;
- if (!input->enable_count) {
- ret = v4l2_subdev_disable_streams(remote_sd, remote_pad,
- sink_streams);
- if (ret)
- dev_err(xbar->isi->dev,
- "failed to disable streams 0x%llx on '%s':%u: %d\n",
- sink_streams, remote_sd->name, remote_pad, ret);
+ if (!(input->enabled_streams & BIT(stream)))
+ continue;
- mxc_isi_crossbar_gasket_disable(xbar, sink_pad);
+ if (--input->enabled_count[stream] == 0)
+ streams_to_disable |= BIT(stream);
}
+ if (!streams_to_disable)
+ return 0;
+
+ ret = v4l2_subdev_disable_streams(remote_sd, remote_pad,
+ streams_to_disable);
+ if (ret)
+ dev_err(xbar->isi->dev,
+ "failed to disable streams 0x%llx on '%s':%u: %d\n",
+ streams_to_disable, remote_sd->name, remote_pad, ret);
+
+ input->enabled_streams &= ~streams_to_disable;
+
+ if (!input->enabled_streams)
+ mxc_isi_crossbar_gasket_disable(xbar, sink_pad);
+
return ret;
}
@@ -447,6 +485,42 @@ static const struct media_entity_operations mxc_isi_cross_entity_ops = {
* Init & cleanup
*/
+static int mxc_isi_stream_counters_alloc(struct mxc_isi_crossbar *xbar)
+{
+ unsigned int i;
+ int ret;
+
+ for (i = 0; i < xbar->num_sinks; ++i) {
+ struct mxc_isi_input *input = &xbar->inputs[i];
+
+ input->enabled_count = kcalloc(xbar->num_sources,
+ sizeof(*input->enabled_count),
+ GFP_KERNEL);
+ if (!input->enabled_count) {
+ dev_err(xbar->isi->dev,
+ "failed to alloc memory for ISI input(%d)\n", i);
+ ret = -ENOMEM;
+ goto err_free;
+ }
+ }
+
+ return 0;
+
+err_free:
+ while (i--)
+ kfree(xbar->inputs[i].enabled_count);
+
+ return ret;
+}
+
+static void mxc_isi_stream_counters_free(struct mxc_isi_crossbar *xbar)
+{
+ unsigned int i;
+
+ for (i = 0; i < xbar->num_sinks; ++i)
+ kfree(xbar->inputs[i].enabled_count);
+}
+
int mxc_isi_crossbar_init(struct mxc_isi_dev *isi)
{
struct mxc_isi_crossbar *xbar = &isi->crossbar;
@@ -484,6 +558,10 @@ int mxc_isi_crossbar_init(struct mxc_isi_dev *isi)
goto err_free;
}
+ ret = mxc_isi_stream_counters_alloc(xbar);
+ if (ret)
+ goto err_free;
+
for (i = 0; i < xbar->num_sinks; ++i)
xbar->pads[i].flags = MEDIA_PAD_FL_SINK
| MEDIA_PAD_FL_MUST_CONNECT;
@@ -492,7 +570,7 @@ int mxc_isi_crossbar_init(struct mxc_isi_dev *isi)
ret = media_entity_pads_init(&sd->entity, num_pads, xbar->pads);
if (ret)
- goto err_free;
+ goto err_free_cnt;
ret = v4l2_subdev_init_finalize(sd);
if (ret < 0)
@@ -502,6 +580,8 @@ int mxc_isi_crossbar_init(struct mxc_isi_dev *isi)
err_entity:
media_entity_cleanup(&sd->entity);
+err_free_cnt:
+ mxc_isi_stream_counters_free(xbar);
err_free:
kfree(xbar->pads);
kfree(xbar->inputs);
@@ -512,6 +592,7 @@ int mxc_isi_crossbar_init(struct mxc_isi_dev *isi)
void mxc_isi_crossbar_cleanup(struct mxc_isi_crossbar *xbar)
{
v4l2_subdev_cleanup(&xbar->sd);
+ mxc_isi_stream_counters_free(xbar);
media_entity_cleanup(&xbar->sd.entity);
kfree(xbar->pads);
kfree(xbar->inputs);
--
2.34.1
^ permalink raw reply related
* [PATCH 1/5] media: nxp: imx8-isi: Fix stream ID validation bypass in crossbar routing
From: Guoniu Zhou @ 2026-06-29 7:44 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Christian Hemp,
Stefan Riedmueller, Jacopo Mondi
Cc: Dong Aisheng, Guoniu Zhou, linux-media, imx, linux-arm-kernel,
linux-kernel, Guoniu Zhou, stable
In-Reply-To: <20260629-isi-v1-0-deebfdb1b07b@oss.nxp.com>
The crossbar routing validation has a critical bug where it validates
the wrong routing table, allowing userspace to bypass validation entirely.
The __mxc_isi_crossbar_set_routing() function is called to validate and
apply a new routing table from userspace. However, the validation loop
iterates over state->routing (the currently active routing table) instead
of the routing parameter (the new table being validated):
for_each_active_route(&state->routing, route) {
This means userspace can submit any invalid routing configuration and it
will pass validation as long as the currently active routing is valid.
This is a security issue as it allows userspace to configure routes that
violate hardware constraints, potentially causing undefined hardware
behavior.
Fix by validating the routing table that will actually be applied:
for_each_active_route(routing, route) {
Additionally, add validation to enforce hardware constraints that were
previously missing:
- SOURCE stream must be 0 (ISI pipes are hardcoded to stream 0)
- SINK stream must be less than the ISI channel count
- Memory input can only route to the first pipeline (existing check)
Fixes: cf21f328fcaf ("media: nxp: Add i.MX8 ISI driver")
Cc: stable@vger.kernel.org
Signed-off-by: Guoniu Zhou <guoniu.zhou@oss.nxp.com>
---
.../platform/nxp/imx8-isi/imx8-isi-crossbar.c | 24 ++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
index c580c831972e..29f14d30dbbb 100644
--- a/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
+++ b/drivers/media/platform/nxp/imx8-isi/imx8-isi-crossbar.c
@@ -106,8 +106,28 @@ static int __mxc_isi_crossbar_set_routing(struct v4l2_subdev *sd,
if (ret)
return ret;
- /* The memory input can be routed to the first pipeline only. */
- for_each_active_route(&state->routing, route) {
+ /*
+ * Validate routes against hardware constraints:
+ * - SOURCE stream must be 0 (pipes are hardcoded to stream 0)
+ * - SINK stream must be < ISI channel count
+ * - Memory input can only route to the first pipeline
+ */
+ for_each_active_route(routing, route) {
+ if (route->source_stream != 0) {
+ dev_dbg(xbar->isi->dev,
+ "route to pipe %u must use source_stream=0, got %u\n",
+ route->source_pad - xbar->num_sinks,
+ route->source_stream);
+ return -ENXIO;
+ }
+
+ if (route->sink_stream >= xbar->num_sources) {
+ dev_dbg(xbar->isi->dev,
+ "sink_stream %u exceeds hardware limit %u\n",
+ route->sink_stream, xbar->num_sources - 1);
+ return -ENXIO;
+ }
+
if (route->sink_pad == xbar->num_sinks - 1 &&
route->source_pad != xbar->num_sinks) {
dev_dbg(xbar->isi->dev,
--
2.34.1
^ permalink raw reply related
* [PATCH 0/5] imx8-isi: Bug fixes and format support enhancements
From: Guoniu Zhou @ 2026-06-29 7:44 UTC (permalink / raw)
To: Laurent Pinchart, Mauro Carvalho Chehab, Frank Li, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam, Christian Hemp,
Stefan Riedmueller, Jacopo Mondi
Cc: Dong Aisheng, Guoniu Zhou, linux-media, imx, linux-arm-kernel,
linux-kernel, Guoniu Zhou, stable, Laurentiu Palcu, Robert Chiras
This series addresses critical bugs in the imx8-isi driver and extends
format support for high-end sensors and Android requirements.
Patch 1 fixes a critical stream ID validation bug in the crossbar routing
that could allow userspace to configure invalid routes causing undefined
behavior.
Patch 2 fixes a stream reference counting bug in the crossbar that would
incorrectly disable active streams when one stream on a multiplexed input
is disabled.
Patch 3 adds support for 16-bit raw Bayer formats (SBGGR16, SGBRG16,
SGRBG16, SRGGB16) commonly used by high-end image sensors.
Patch 4 fixes incorrect color mapping for XBGR32 format in memory-to-memory
mode (marked for stable backport).
Patch 5 extends RGB format support by adding BGRA32, RGBA32, BGRX32, RGBX32,
and ARGB2101010 formats with full M2M capabilities to meet Android
requirements.
Signed-off-by: Guoniu Zhou <guoniu.zhou@oss.nxp.com>
---
Guoniu Zhou (4):
media: nxp: imx8-isi: Fix stream ID validation bypass in crossbar routing
media: nxp: imx8-isi: Fix per-stream reference counting for multiplexed streams
media: nxp: imx8-isi: Correct color map between V4L2 and ISI
media: nxp: imx8-isi: Add additional 32-bit RGB format support
Laurentiu Palcu (1):
media: nxp: imx8-isi: Add 16-bit raw Bayer format support
.../media/platform/nxp/imx8-isi/imx8-isi-core.h | 4 +-
.../platform/nxp/imx8-isi/imx8-isi-crossbar.c | 145 +++++++++++++++++----
.../media/platform/nxp/imx8-isi/imx8-isi-pipe.c | 24 ++++
.../media/platform/nxp/imx8-isi/imx8-isi-video.c | 97 +++++++++++++-
4 files changed, 245 insertions(+), 25 deletions(-)
---
base-commit: 06cb687a5132fcffe624c0070576ab852ac6b568
change-id: 20260626-isi-00f05b044ac9
Best regards,
--
Guoniu Zhou <guoniu.zhou@oss.nxp.com>
^ permalink raw reply
* Re: 答复: [外部邮件] Re: [PATCH] arm64: mm: refresh stale pmd snapshot after split_contpmd()
From: David Hildenbrand (Arm) @ 2026-06-29 7:36 UTC (permalink / raw)
To: Li,Rongqing, Yang Shi, Catalin Marinas, Will Deacon, Ryan Roberts,
Ard Biesheuvel, Anshuman Khandual, Kevin Brodsky,
Chaitanya S Prakash, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <7ac17ef889d242c29f39898a95364df9@baidu.com>
On 6/27/26 04:46, Li,Rongqing wrote:
>>
>>
>>
>> On 6/25/26 4:39 AM, lirongqing wrote:
>>> From: Li RongQing <lirongqing@baidu.com>
>>>
>>> split_contpmd() modifies the pmd entries in-place by clearing the CONT
>>> bit, but the local 'pmd' variable still holds the old snapshot with
>>> CONT set. The subsequent split_pmd() call uses this stale value to
>>> derive the pgprot for the new PTE entries via pmd_pgprot(), causing
>>> the resulting PTEs to be populated with incorrect protection bits.
>>
>> If I read the code correctly, CONT bit is cleared by split_pmd(), then the bit
>> may be set again for PTEs if we want to have cont ptes. So I don't see any
>> problem, did I miss something?
>>
>
> You are right, there's no functional issue with the current code.
> However, I think explicitly re-reading the pmd is the safer and clearer
> approach — it makes the intent obvious (we need the post-modification state)
> rather than relying on the implicit assumption that "CONT bit doesn't affect pgprot."
Note that it's going to be another read (READ_ONCE()) that cannot be optimized
out. Manual pmd_mknoncont() might be cheaper.
But then, nobody care about it being set or not ...
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH 2/2] iio: adc: Add Nuvoton MA35D1 EADC driver
From: Chi-Wen Weng @ 2026-06-29 7:32 UTC (permalink / raw)
To: David Lechner, jic23, robh, krzk+dt, conor+dt
Cc: nuno.sa, andy, linux-arm-kernel, linux-iio, devicetree,
linux-kernel, cwweng
In-Reply-To: <66ad5723-6589-473a-91da-b13e2ba90aeb@baylibre.com>
Hi David,
Thanks for the detailed review.
After looking at your comments and the other review feedback, I plan to
simplify v2 and limit the initial upstream driver to direct raw reads for
the external single-ended ADC channels.
In v2, I will drop the triggered buffer support, the device trigger and
the differential channel support for now. Buffered capture and
differential inputs can be added later as follow-up patches once the
scan sequencing, trigger model and differential pair constraints are
handled properly.
This also means that the scan buffer layout comments will no longer
apply to v2, since the triggered-buffer path will be removed from the
initial submission.
I will address the other driver comments in v2:
- drop the unused struct device pointer,
- remove the triggered-buffer and trigger-related Kconfig selects,
- switch the register access helpers to regmap,
- avoid forcing the external reference path unconditionally,
- add optional vref-supply handling,
- add IIO_CHAN_INFO_SCALE based on the selected reference source,
- use the internal reference when no vref-supply is provided,
- add a named macro for the sample-time field,
- drop the unused channel address field,
- drop datasheet_name from the initial driver,
- use device_for_each_child_node_scoped(),
- use devm_mutex_init(),
- keep only INDIO_DIRECT_MODE for the initial driver.
For the firmware-described channels, v2 will only accept the external
ADC input channels 0 to 7. The internal VBAT channel and differential
inputs will not be described or exposed by the initial driver.
Thanks,
Chi-Wen
David Lechner 於 2026/6/28 上午 04:52 寫道:
> On 6/25/26 6:06 AM, Chi-Wen Weng wrote:
>> From: Chi-Wen Weng <cwweng@nuvoton.com>
>>
>> Add an IIO driver for the Nuvoton MA35D1 Enhanced ADC controller.
>>
>> The driver supports direct raw reads and triggered buffered capture. The
>> controller end-of-conversion interrupt is exposed as the device trigger
>> and is used to push samples into the IIO buffer.
>>
>> Channels are described by firmware child nodes and can be configured as
>> single-ended or differential inputs. Since the differential enable bit is
>> global, mixed single-ended and differential buffered scans are rejected.
>>
>> DMA support is intentionally not included in this initial upstream driver;
>> conversions are handled through the interrupt-driven path.
>>
>> Signed-off-by: Chi-Wen Weng <cwweng@nuvoton.com>
>> ---
>> drivers/iio/adc/Kconfig | 10 +
>> drivers/iio/adc/Makefile | 1 +
>> drivers/iio/adc/ma35d1_eadc.c | 636 ++++++++++++++++++++++++++++++++++
>> 3 files changed, 647 insertions(+)
>> create mode 100644 drivers/iio/adc/ma35d1_eadc.c
>>
>> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
>> index 1c663c98c6c9..43409999a94b 100644
>> --- a/drivers/iio/adc/Kconfig
>> +++ b/drivers/iio/adc/Kconfig
>> @@ -981,6 +981,16 @@ config LTC2497
>> To compile this driver as a module, choose M here: the module will be
>> called ltc2497.
>>
>> +config MA35D1_EADC
>> + tristate "MA35D1 EADC driver"
>> + select IIO_BUFFER
>> + select IIO_TRIGGERED_BUFFER
>> + help
>> + Say yes here to build support for MA35D1 EADC.
>> +
>> + To compile this driver as a module, choose M here: the module will be
>> + called ma35d1.
>> +
>> config MAX1027
>> tristate "Maxim max1027 ADC driver"
>> depends on SPI
>> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
>> index 707dd708912f..7b9b38688223 100644
>> --- a/drivers/iio/adc/Makefile
>> +++ b/drivers/iio/adc/Makefile
>> @@ -85,6 +85,7 @@ obj-$(CONFIG_LTC2471) += ltc2471.o
>> obj-$(CONFIG_LTC2485) += ltc2485.o
>> obj-$(CONFIG_LTC2496) += ltc2496.o ltc2497-core.o
>> obj-$(CONFIG_LTC2497) += ltc2497.o ltc2497-core.o
>> +obj-$(CONFIG_MA35D1_EADC) += ma35d1_eadc.o
>> obj-$(CONFIG_MAX1027) += max1027.o
>> obj-$(CONFIG_MAX11100) += max11100.o
>> obj-$(CONFIG_MAX1118) += max1118.o
>> diff --git a/drivers/iio/adc/ma35d1_eadc.c b/drivers/iio/adc/ma35d1_eadc.c
>> new file mode 100644
>> index 000000000000..0c075126e139
>> --- /dev/null
>> +++ b/drivers/iio/adc/ma35d1_eadc.c
>> @@ -0,0 +1,636 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * Nuvoton MA35D1 EADC driver
>> + *
>> + * Copyright (c) 2026 Nuvoton Technology Corp.
>> + */
>> +
>> +#include <linux/bitfield.h>
>> +#include <linux/bits.h>
>> +#include <linux/bitmap.h>
>> +#include <linux/clk.h>
>> +#include <linux/completion.h>
>> +#include <linux/device.h>
>> +#include <linux/err.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/mod_devicetable.h>
>> +#include <linux/mutex.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pm.h>
>> +#include <linux/property.h>
>> +
>> +#include <linux/iio/buffer.h>
>> +#include <linux/iio/iio.h>
>> +#include <linux/iio/trigger.h>
>> +#include <linux/iio/trigger_consumer.h>
>> +#include <linux/iio/triggered_buffer.h>
>> +
>> +#define MA35D1_EADC_DAT(n) (0x00 + (n) * 0x04)
>> +#define MA35D1_EADC_CTL 0x50
>> +#define MA35D1_EADC_SWTRG 0x54
>> +#define MA35D1_EADC_SCTL(n) (0x80 + (n) * 0x04)
>> +#define MA35D1_EADC_INTSRC0 0xd0
>> +#define MA35D1_EADC_STATUS2 0xf8
>> +#define MA35D1_EADC_SELSMP0 0x140
>> +#define MA35D1_EADC_REFADJCTL 0x150
>> +
>> +#define MA35D1_EADC_CTL_ADCEN BIT(0)
>> +#define MA35D1_EADC_CTL_ADCIEN0 BIT(2)
>> +#define MA35D1_EADC_CTL_DIFFEN BIT(8)
>> +
>> +#define MA35D1_EADC_SCTL_CHSEL_MASK GENMASK(3, 0)
>> +#define MA35D1_EADC_SCTL_TRGDLY_MASK GENMASK(15, 8)
>> +#define MA35D1_EADC_SCTL_TRGSEL_MASK GENMASK(21, 16)
>> +#define MA35D1_EADC_SCTL_TRGSEL_ADINT0 \
>> + FIELD_PREP(MA35D1_EADC_SCTL_TRGSEL_MASK, 2)
>> +
>> +#define MA35D1_EADC_DAT_MASK GENMASK(11, 0)
>> +#define MA35D1_EADC_STATUS2_ADIF0 BIT(0)
>> +#define MA35D1_EADC_INTSRC0_ADINT0 BIT(0)
>> +#define MA35D1_EADC_REFADJCTL_EXT_VREF BIT(0)
>> +
>> +#define MA35D1_EADC_MAX_CHANNELS 9
>> +#define MA35D1_EADC_MAX_SAMPLE_MODULES 16
>> +#define MA35D1_EADC_CHAN_NAME_LEN 16
>> +#define MA35D1_EADC_TIMEOUT msecs_to_jiffies(1000)
>> +
>> +struct ma35d1_adc {
>> + struct device *dev;
> It looks like this is never used, so we can drop it.
>
>> + void __iomem *regs;
>> + struct clk *clk;
>> + struct completion completion;
>> + /* Protects direct conversions against concurrent register access. */
>> + struct mutex lock;
>> + struct iio_trigger *trig;
>> + unsigned int scan_chancnt;
>> + bool scan_differential;
>> + char chan_name[MA35D1_EADC_MAX_CHANNELS][MA35D1_EADC_CHAN_NAME_LEN];
>> + struct {
>> + u16 channels[MA35D1_EADC_MAX_SAMPLE_MODULES];
>> + aligned_s64 timestamp;
>> + } scan;
> Unless the hardware requires all channels to be read at once, we should
> use this instead:
>
> IIO_DECLARE_BUFFER_WITH_TS(u16, scan, MA35D1_EADC_MAX_SAMPLE_MODULES);
>
> It means an array with enough room for MA35D1_EADC_MAX_SAMPLE_MODULES u16
> data points plus an aligned timestamp, but doesn't specify where the
> timestamp will be as it could be in a different place depending on how
> many channels are read.
>
> Also, this is only used in one function, so can just be stack-allocated
> in that function (with ` = { };` to zero it) instead of allocating it here.
>
>> +};
>> +
>> +static inline u32 ma35d1_adc_read(struct ma35d1_adc *adc, u32 reg)
>> +{
>> + return readl(adc->regs + reg);
>> +}
>> +
>> +static inline void ma35d1_adc_write(struct ma35d1_adc *adc, u32 reg, u32 val)
>> +{
>> + writel(val, adc->regs + reg);
>> +}
>> +
>> +static void ma35d1_adc_rmw(struct ma35d1_adc *adc, u32 reg, u32 mask, u32 val)
>> +{
>> + u32 tmp;
>> +
>> + tmp = ma35d1_adc_read(adc, reg);
>> + tmp &= ~mask;
>> + tmp |= val;
>> + ma35d1_adc_write(adc, reg, tmp);
>> +}
> Why not use regmap?
>
>> +
>> +static void ma35d1_adc_set_diff(struct ma35d1_adc *adc, bool differential)
>> +{
>> + ma35d1_adc_rmw(adc, MA35D1_EADC_CTL, MA35D1_EADC_CTL_DIFFEN,
>> + differential ? MA35D1_EADC_CTL_DIFFEN : 0);
>> +}
>> +
>> +static void ma35d1_adc_config_sample(struct ma35d1_adc *adc,
>> + unsigned int sample, unsigned int channel)
>> +{
>> + u32 reg = MA35D1_EADC_SCTL(sample);
>> +
>> + ma35d1_adc_rmw(adc, reg,
>> + MA35D1_EADC_SCTL_CHSEL_MASK |
>> + MA35D1_EADC_SCTL_TRGSEL_MASK,
>> + FIELD_PREP(MA35D1_EADC_SCTL_CHSEL_MASK, channel) |
>> + MA35D1_EADC_SCTL_TRGSEL_ADINT0);
>> +}
>> +
>> +static void ma35d1_adc_disable_irq(struct ma35d1_adc *adc)
>> +{
>> + ma35d1_adc_rmw(adc, MA35D1_EADC_CTL, MA35D1_EADC_CTL_ADCIEN0, 0);
>> +}
>> +
>> +static void ma35d1_adc_hw_init(struct ma35d1_adc *adc)
>> +{
>> + ma35d1_adc_disable_irq(adc);
>> + ma35d1_adc_rmw(adc, MA35D1_EADC_CTL,
>> + MA35D1_EADC_CTL_ADCEN, MA35D1_EADC_CTL_ADCEN);
>> + ma35d1_adc_write(adc, MA35D1_EADC_STATUS2, MA35D1_EADC_STATUS2_ADIF0);
>> + ma35d1_adc_rmw(adc, MA35D1_EADC_INTSRC0,
>> + MA35D1_EADC_INTSRC0_ADINT0,
>> + MA35D1_EADC_INTSRC0_ADINT0);
>> + ma35d1_adc_rmw(adc, MA35D1_EADC_REFADJCTL,
>> + MA35D1_EADC_REFADJCTL_EXT_VREF,
>> + MA35D1_EADC_REFADJCTL_EXT_VREF);
> As mentioned elsewhere, external reference doesn't make sense unless
> we can get the reference voltage from it.
>
>> + ma35d1_adc_rmw(adc, MA35D1_EADC_SELSMP0, GENMASK(1, 0), 3);
>> +}
>> +
>> +static void ma35d1_adc_hw_disable(void *data)
>> +{
>> + struct ma35d1_adc *adc = data;
>> +
>> + ma35d1_adc_disable_irq(adc);
> ma35d1_adc_hw_init() doesn't enable the IRQ, so this seems a bit unbalanced.
>
>> + ma35d1_adc_rmw(adc, MA35D1_EADC_CTL, MA35D1_EADC_CTL_ADCEN, 0);
>> +}
>> +
>> +static irqreturn_t ma35d1_adc_isr(int irq, void *data)
>> +{
>> + struct iio_dev *indio_dev = data;
>> + struct ma35d1_adc *adc = iio_priv(indio_dev);
>> + u32 status;
>> +
>> + status = ma35d1_adc_read(adc, MA35D1_EADC_STATUS2);
>> + if (!(status & MA35D1_EADC_STATUS2_ADIF0))
>> + return IRQ_NONE;
>> +
>> + ma35d1_adc_write(adc, MA35D1_EADC_STATUS2, MA35D1_EADC_STATUS2_ADIF0);
>> +
>> + if (iio_buffer_enabled(indio_dev)) {
>> + ma35d1_adc_disable_irq(adc);
>> + iio_trigger_poll(adc->trig);
>> + } else {
>> + complete(&adc->completion);
>> + }
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static irqreturn_t ma35d1_adc_trigger_handler(int irq, void *p)
>> +{
>> + struct iio_poll_func *pf = p;
>> + struct iio_dev *indio_dev = pf->indio_dev;
>> + struct ma35d1_adc *adc = iio_priv(indio_dev);
>> + int i;
>> +
>> + for (i = 0; i < adc->scan_chancnt; i++)
>> + adc->scan.channels[i] =
>> + ma35d1_adc_read(adc, MA35D1_EADC_DAT(i)) &
>> + MA35D1_EADC_DAT_MASK;
>> +
>> + iio_push_to_buffers_with_timestamp(indio_dev, &adc->scan, pf->timestamp);
>> + iio_trigger_notify_done(adc->trig);
>> +
>> + ma35d1_adc_rmw(adc, MA35D1_EADC_CTL, MA35D1_EADC_CTL_ADCIEN0,
>> + MA35D1_EADC_CTL_ADCIEN0);
>> + ma35d1_adc_write(adc, MA35D1_EADC_SWTRG, 1);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int ma35d1_adc_read_conversion(struct iio_dev *indio_dev,
>> + const struct iio_chan_spec *chan,
>> + int *val)
>> +{
>> + struct ma35d1_adc *adc = iio_priv(indio_dev);
>> + long timeout;
>> +
>> + reinit_completion(&adc->completion);
>> +
>> + ma35d1_adc_write(adc, MA35D1_EADC_STATUS2, MA35D1_EADC_STATUS2_ADIF0);
>> + ma35d1_adc_rmw(adc, MA35D1_EADC_SCTL(0),
>> + MA35D1_EADC_SCTL_CHSEL_MASK |
>> + MA35D1_EADC_SCTL_TRGSEL_MASK,
>> + FIELD_PREP(MA35D1_EADC_SCTL_CHSEL_MASK,
>> + chan->channel));
>> + ma35d1_adc_set_diff(adc, chan->differential);
>> + ma35d1_adc_rmw(adc, MA35D1_EADC_CTL, MA35D1_EADC_CTL_ADCIEN0,
>> + MA35D1_EADC_CTL_ADCIEN0);
>> + ma35d1_adc_write(adc, MA35D1_EADC_SWTRG, 1);
>> +
>> + timeout = wait_for_completion_interruptible_timeout(&adc->completion,
>> + MA35D1_EADC_TIMEOUT);
>> + ma35d1_adc_disable_irq(adc);
>> +
>> + if (timeout < 0)
>> + return timeout;
>> + if (!timeout)
>> + return -ETIMEDOUT;
>> +
>> + *val = ma35d1_adc_read(adc, MA35D1_EADC_DAT(0)) & MA35D1_EADC_DAT_MASK;
>> +
>> + return 0;
>> +}
>> +
>> +static int ma35d1_adc_read_raw(struct iio_dev *indio_dev,
>> + const struct iio_chan_spec *chan,
>> + int *val, int *val2, long mask)
>> +{
>> + struct ma35d1_adc *adc = iio_priv(indio_dev);
>> + int ret;
>> +
>> + switch (mask) {
>> + case IIO_CHAN_INFO_RAW:
>> + if (!iio_device_claim_direct(indio_dev))
>> + return -EBUSY;
>> +
>> + mutex_lock(&adc->lock);
>> + ret = ma35d1_adc_read_conversion(indio_dev, chan, val);
>> + mutex_unlock(&adc->lock);
>> +
>> + iio_device_release_direct(indio_dev);
>> + if (ret)
>> + return ret;
>> +
>> + return IIO_VAL_INT;
>> + default:
>> + return -EINVAL;
>> + }
>> +}
>> +
>> +static int ma35d1_adc_validate_scan(struct iio_dev *indio_dev,
>> + const unsigned long *scan_mask)
>> +{
>> + const struct iio_chan_spec *chan;
>> + bool have_single = false;
>> + bool have_diff = false;
>> + unsigned int count = 0;
>> + unsigned long bit;
>> +
>> + for_each_set_bit(bit, scan_mask, indio_dev->masklength) {
>> + chan = &indio_dev->channels[bit];
>> +
>> + if (chan->type == IIO_TIMESTAMP)
>> + continue;
> Timestamp channel will never be set (it is handled differently), so we
> don't need to check here.
>
>> + count++;
>> + if (chan->differential)
>> + have_diff = true;
>> + else
>> + have_single = true;
>> + }
>> +
>> + if (!count || count > MA35D1_EADC_MAX_SAMPLE_MODULES)
>> + return -EINVAL;
>> +
>> + if (have_single && have_diff)
>> + return -EINVAL;
>> +
>> + return 0;
>> +}
>> +
>> +static int ma35d1_adc_update_scan_mode(struct iio_dev *indio_dev,
>> + const unsigned long *scan_mask)
>> +{
>> + struct ma35d1_adc *adc = iio_priv(indio_dev);
>> + const struct iio_chan_spec *chan;
>> + unsigned int sample = 0;
>> + unsigned long bit;
>> + bool differential = false;
>> + int ret;
>> +
>> + ret = ma35d1_adc_validate_scan(indio_dev, scan_mask);
>> + if (ret)
>> + return ret;
>> +
>> + for_each_set_bit(bit, scan_mask, indio_dev->masklength) {
>> + chan = &indio_dev->channels[bit];
>> + if (chan->type == IIO_TIMESTAMP)
>> + continue;
> Same here.
>
>> +
>> + if (!sample)
>> + differential = chan->differential;
>> +
>> + ma35d1_adc_config_sample(adc, sample, chan->channel);
>> + sample++;
>> + }
>> +
>> + adc->scan_chancnt = sample;
>> + adc->scan_differential = differential;
>> +
>> + return 0;
>> +}
>> +
>> +static int ma35d1_adc_buffer_postenable(struct iio_dev *indio_dev)
>> +{
>> + struct ma35d1_adc *adc = iio_priv(indio_dev);
>> + int i;
>> +
>> + if (!adc->scan_chancnt)
>> + return -EINVAL;
> Should this even be possible because of ma35d1_adc_validate_scan()?
>
>> +
>> + ma35d1_adc_write(adc, MA35D1_EADC_STATUS2, MA35D1_EADC_STATUS2_ADIF0);
>> + ma35d1_adc_rmw(adc, MA35D1_EADC_INTSRC0,
>> + MA35D1_EADC_INTSRC0_ADINT0,
>> + MA35D1_EADC_INTSRC0_ADINT0);
>> + ma35d1_adc_rmw(adc, MA35D1_EADC_REFADJCTL,
>> + MA35D1_EADC_REFADJCTL_EXT_VREF,
>> + MA35D1_EADC_REFADJCTL_EXT_VREF);
> How can we use the external V_REF without knowing what is connected to it?
>
> I would expect this to use internal reference unless the devicetree specified
> a vref-supply.
>
>> + ma35d1_adc_rmw(adc, MA35D1_EADC_SELSMP0, GENMASK(1, 0), 3);
> There should be a macro to say what field GENMASK(1, 0) is.
>
>> + ma35d1_adc_set_diff(adc, adc->scan_differential);
>> +
>> + for (i = 0; i < adc->scan_chancnt; i++)
>> + ma35d1_adc_rmw(adc, MA35D1_EADC_SCTL(i),
>> + MA35D1_EADC_SCTL_TRGDLY_MASK,
>> + MA35D1_EADC_SCTL_TRGDLY_MASK);
>> +
>> + ma35d1_adc_rmw(adc, MA35D1_EADC_CTL, MA35D1_EADC_CTL_ADCIEN0,
>> + MA35D1_EADC_CTL_ADCIEN0);
>> + ma35d1_adc_write(adc, MA35D1_EADC_SWTRG, 1);
>> +
>> + return 0;
>> +}
>> +
>> +static int ma35d1_adc_buffer_predisable(struct iio_dev *indio_dev)
>> +{
>> + struct ma35d1_adc *adc = iio_priv(indio_dev);
>> + int i;
>> +
>> + ma35d1_adc_disable_irq(adc);
>> + for (i = 0; i < adc->scan_chancnt; i++)
>> + ma35d1_adc_rmw(adc, MA35D1_EADC_SCTL(i),
>> + MA35D1_EADC_SCTL_TRGSEL_MASK, 0);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct iio_buffer_setup_ops ma35d1_adc_buffer_ops = {
>> + .postenable = ma35d1_adc_buffer_postenable,
>> + .predisable = ma35d1_adc_buffer_predisable,
>> +};
>> +
>> +static const struct iio_info ma35d1_adc_info = {
>> + .read_raw = ma35d1_adc_read_raw,
>> + .update_scan_mode = ma35d1_adc_update_scan_mode,
>> +};
>> +
>> +static const struct iio_trigger_ops ma35d1_adc_trigger_ops = {
>> + .validate_device = iio_trigger_validate_own_device,
>> +};
>> +
>> +static void ma35d1_adc_init_channel(struct ma35d1_adc *adc,
>> + struct iio_chan_spec *chan, u32 vinp,
>> + u32 vinn, int scan_index, bool differential)
>> +{
>> + char *name = adc->chan_name[vinp];
>> +
>> + chan->type = IIO_VOLTAGE;
>> + chan->indexed = 1;
>> + chan->channel = vinp;
>> + chan->address = vinp;
> Setting address is reduant if it is always going to be the same as channel.
> We can just use channel directly instead. address doesn't appear to be
> used anyway.
>
>> + chan->scan_index = scan_index;
>> + chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
> This should also have IIO_CHAN_INFO_SCALE that is based on the reference source.
>
>> + chan->scan_type.sign = 'u';
> This field has a new name:
>
> chan->scan_type.format = IIO_SCAN_FORMAT_UNSIGNED_INT;
>
>> + chan->scan_type.realbits = 12;
>> + chan->scan_type.storagebits = 16;
>> + chan->scan_type.endianness = IIO_CPU;
>> +
>> + if (differential) {
>> + chan->differential = 1;
>> + chan->channel2 = vinn;
>> + snprintf(name, MA35D1_EADC_CHAN_NAME_LEN, "in%d-in%d", vinp,
>> + vinn);
>> + } else {
>> + snprintf(name, MA35D1_EADC_CHAN_NAME_LEN, "in%d", vinp);
>> + }
>> +
>> + chan->datasheet_name = name;
> We usually don't use datasheet_name. It would make more sense to implement
> labels and get the label from devicetree.
>
>
>> +}
>> +
>> +static int ma35d1_adc_parse_channels(struct iio_dev *indio_dev,
>> + struct device *dev)
>> +{
>> + struct ma35d1_adc *adc = iio_priv(indio_dev);
>> + DECLARE_BITMAP(used_channels, MA35D1_EADC_MAX_CHANNELS);
>> + struct fwnode_handle *child;
>> + struct iio_chan_spec *channels;
>> + int num_channels;
>> + int scan_index = 0;
>> + int ret;
>> +
>> + bitmap_zero(used_channels, MA35D1_EADC_MAX_CHANNELS);
>> +
>> + num_channels = device_get_child_node_count(dev);
>> + if (!num_channels)
>> + return dev_err_probe(dev, -ENODATA,
>> + "no ADC channels configured\n");
>> +
>> + if (num_channels > MA35D1_EADC_MAX_CHANNELS)
>> + return dev_err_probe(dev, -EINVAL, "too many ADC channels\n");
>> +
>> + channels = devm_kcalloc(dev, num_channels + 1, sizeof(*channels),
>> + GFP_KERNEL);
>> + if (!channels)
>> + return -ENOMEM;
>> +
>> + device_for_each_child_node(dev, child) {
> Can use device_for_each_child_node_scoped() here to avoid needing to put
> handle on all return paths.
>
>> + u32 diff[2];
>> + u32 reg;
>> + bool differential = false;
>> +
>> + ret = fwnode_property_read_u32(child, "reg", ®);
>> + if (ret) {
>> + fwnode_handle_put(child);
>> + return dev_err_probe(dev, ret,
>> + "missing channel reg property\n");
>> + }
>> +
>> + if (reg >= MA35D1_EADC_MAX_CHANNELS) {
>> + fwnode_handle_put(child);
>> + return dev_err_probe(dev, -EINVAL,
>> + "invalid ADC channel %u\n", reg);
>> + }
>> +
>> + if (test_and_set_bit(reg, used_channels)) {
>> + fwnode_handle_put(child);
>> + return dev_err_probe(dev, -EINVAL,
>> + "duplicate ADC channel %u\n", reg);
>> + }
>> +
>> + if (fwnode_property_present(child, "diff-channels")) {
>> + ret = fwnode_property_read_u32_array(child,
>> + "diff-channels",
>> + diff,
>> + ARRAY_SIZE(diff));
>> + if (ret) {
>> + fwnode_handle_put(child);
>> + return dev_err_probe(dev, ret,
>> + "invalid diff-channels for channel %u\n",
>> + reg);
>> + }
>> +
>> + if (diff[0] != reg ||
>> + diff[1] >= MA35D1_EADC_MAX_CHANNELS ||
>> + diff[0] == diff[1]) {
>> + fwnode_handle_put(child);
>> + return dev_err_probe(dev, -EINVAL,
>> + "invalid differential ADC channel %u-%u\n",
>> + diff[0], diff[1]);
>> + }
>> +
>> + if (test_and_set_bit(diff[1], used_channels)) {
>> + fwnode_handle_put(child);
>> + return dev_err_probe(dev, -EINVAL,
>> + "ADC channel %u already used\n",
>> + diff[1]);
>> + }
>> +
>> + differential = true;
>> + }
>> +
>> + ma35d1_adc_init_channel(adc, &channels[scan_index], reg,
>> + differential ? diff[1] : 0,
>> + scan_index, differential);
>> + scan_index++;
>> + }
>> +
>> + channels[scan_index] = (struct iio_chan_spec)
> The macro already includes (struct iio_chan_spec) so we don't need it here.
>
>> + IIO_CHAN_SOFT_TIMESTAMP(scan_index);
>> +
>> + indio_dev->channels = channels;
>> + indio_dev->num_channels = scan_index + 1;
>> + indio_dev->masklength = indio_dev->num_channels;
>> +
>> + return 0;
>> +}
>> +
>> +static int ma35d1_adc_setup_trigger(struct iio_dev *indio_dev,
>> + struct device *dev)
>> +{
>> + struct ma35d1_adc *adc = iio_priv(indio_dev);
>> + int ret;
>> +
>> + adc->trig = devm_iio_trigger_alloc(dev, "%s-trigger", dev_name(dev));
>> + if (!adc->trig)
>> + return -ENOMEM;
>> +
>> + adc->trig->ops = &ma35d1_adc_trigger_ops;
>> + iio_trigger_set_drvdata(adc->trig, indio_dev);
>> +
>> + ret = devm_iio_trigger_register(dev, adc->trig);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "failed to register trigger\n");
>> +
>> + ret = iio_trigger_set_immutable(indio_dev, adc->trig);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "failed to set trigger\n");
>> +
>> + return 0;
>> +}
>> +
>> +static int ma35d1_adc_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct iio_dev *indio_dev;
>> + struct ma35d1_adc *adc;
>> + int irq;
>> + int ret;
>> +
>> + indio_dev = devm_iio_device_alloc(dev, sizeof(*adc));
>> + if (!indio_dev)
>> + return -ENOMEM;
>> + adc = iio_priv(indio_dev);
>> + adc->dev = dev;
>> + mutex_init(&adc->lock);
> devm_mutex_init()
>
>> + init_completion(&adc->completion);
>> +
>> + adc->regs = devm_platform_ioremap_resource(pdev, 0);
>> + if (IS_ERR(adc->regs))
>> + return dev_err_probe(dev, PTR_ERR(adc->regs),
>> + "failed to map registers\n");
>> +
>> + adc->clk = devm_clk_get_enabled(dev, NULL);
>> + if (IS_ERR(adc->clk))
>> + return dev_err_probe(dev, PTR_ERR(adc->clk),
>> + "failed to get and enable ADC clock\n");
>> +
>> + indio_dev->name = "ma35d1-eadc";
>> + indio_dev->modes = INDIO_DIRECT_MODE | INDIO_BUFFER_TRIGGERED;
> devm_iio_triggered_buffer_setup() sets the INDIO_BUFFER_TRIGGERED
> flag, so we don't need it here.
>
>> + indio_dev->info = &ma35d1_adc_info;
>> +
>> + ret = ma35d1_adc_parse_channels(indio_dev, dev);
>> + if (ret)
>> + return ret;
>> +
>> + ma35d1_adc_hw_init(adc);
>> +
>> + ret = devm_add_action_or_reset(dev, ma35d1_adc_hw_disable, adc);
>> + if (ret)
>> + return ret;
>> +
>> + ret = ma35d1_adc_setup_trigger(indio_dev, dev);
>> + if (ret)
>> + return ret;
>> +
>> + irq = platform_get_irq(pdev, 0);
>> + if (irq < 0)
>> + return irq;
>> +
>> + ret = devm_request_irq(dev, irq, ma35d1_adc_isr, 0, dev_name(dev),
>> + indio_dev);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "failed to request IRQ %d\n", irq);
>> +
>> + ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
>> + iio_pollfunc_store_time,
>> + ma35d1_adc_trigger_handler,
>> + &ma35d1_adc_buffer_ops);
>> + if (ret)
>> + return dev_err_probe(dev, ret,
>> + "failed to setup triggered buffer\n");
>> +
>> + platform_set_drvdata(pdev, indio_dev);
>> +
>> + ret = devm_iio_device_register(dev, indio_dev);
>> + if (ret)
>> + return dev_err_probe(dev, ret, "failed to register IIO device\n");
>> +
>> + return 0;
>> +}
>> +
>> +static int ma35d1_adc_suspend(struct device *dev)
>> +{
>> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
>> + struct ma35d1_adc *adc = iio_priv(indio_dev);
>> +
>> + if (iio_buffer_enabled(indio_dev))
>> + return -EBUSY;
>> +
>> + ma35d1_adc_hw_disable(adc);
>> + clk_disable_unprepare(adc->clk);
>> +
>> + return 0;
>> +}
>> +
>> +static int ma35d1_adc_resume(struct device *dev)
>> +{
>> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
>> + struct ma35d1_adc *adc = iio_priv(indio_dev);
>> + int ret;
>> +
>> + ret = clk_prepare_enable(adc->clk);
>> + if (ret)
>> + return ret;
>> +
>> + ma35d1_adc_hw_init(adc);
>> +
>> + return 0;
>> +}
>> +
>> +static DEFINE_SIMPLE_DEV_PM_OPS(ma35d1_adc_pm_ops,
>> + ma35d1_adc_suspend, ma35d1_adc_resume);
>> +
>> +static const struct of_device_id ma35d1_adc_of_match[] = {
>> + { .compatible = "nuvoton,ma35d1-eadc" },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(of, ma35d1_adc_of_match);
>> +
>> +static struct platform_driver ma35d1_adc_driver = {
>> + .probe = ma35d1_adc_probe,
>> + .driver = {
>> + .name = "ma35d1-eadc",
>> + .of_match_table = ma35d1_adc_of_match,
>> + .pm = pm_sleep_ptr(&ma35d1_adc_pm_ops),
>> + },
>> +};
>> +module_platform_driver(ma35d1_adc_driver);
>> +
>> +MODULE_AUTHOR("Chi-Wen Weng <cwweng@nuvoton.com>");
>> +MODULE_DESCRIPTION("Nuvoton MA35D1 EADC driver");
>> +MODULE_LICENSE("GPL");
^ permalink raw reply
* RE: [External Mail] Re: [PATCH v3 3/7] net: wwan: t9xx: Add control DMA interface
From: Wu. JackBB (GSM) @ 2026-06-29 7:29 UTC (permalink / raw)
To: Andrew Lunn
Cc: Loic Poulain, Sergey Ryazanov, Johannes Berg, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Wen-Zhi Huang, Shi-Wei Yeh, Minano Tseng, Matthias Brugger,
AngeloGioacchino Del Regno, Simon Horman, Jonathan Corbet,
Shuah Khan, linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org, linux-doc@vger.kernel.org
In-Reply-To: <dc007021-9013-47b2-b06a-fe4a50d26722@lunn.ch>
Hi Andrew,
> > +static inline void mtk_cldma_clr_bd_dsc(struct cldma_drv_info *drv_info,
> > + struct bd_dsc *bd_dsc_pool, int nr_bds)
>
> No inline functions in C files. Please let the compiler decide.
Will remove inline from all static functions in .c files.
> > + bd_dsc->skb = __dev_alloc_skb(req->frag_size, GFP_KERNEL);
> > + if (!bd_dsc->skb) {
> > + dev_warn((mdev)->dev, "Failed to alloc SKB\n");
>
> You might want to rate limit this, and the other similar messages in
> the data path, otherwise it could be a DOS.
Will switch to dev_warn_ratelimited() for data path warnings.
> > + } while (++cnt < 10);
>
> Please use one of the helpers from iopoll.h.
Will replace with read_poll_timeout() in v4.
> > + devm_kfree(trans->mdev->dev, srv_que);
>
> It is unusual to see devm_kfree(). Why is it needed?
Will remove redundant devm_kfree() calls and let devres handle
cleanup on probe failure.
> > +static unsigned int ctrl_port_chl_mtu;
>
> Is this a global variable? Why is it not part of priv?
> > +module_param(ctrl_port_chl_mtu, uint, 0644);
>
> No modules parameters please. If this is an MTU, why not use the
> normal networking interfaces to set the MTU?
Will remove the module parameter entirely. The control port MTU
is determined by the device during handshake.
Thanks.
Jack Wu
^ permalink raw reply
* Re: [RFC PATCH net-next v8 03/12] net: phylink: add phylink_release_pcs() to externally release a PCS
From: Maxime Chevallier @ 2026-06-29 7:28 UTC (permalink / raw)
To: Christian Marangi
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Simon Horman, Jonathan Corbet, Shuah Khan, Lorenzo Bianconi,
Heiner Kallweit, Russell King, Saravana Kannan, Philipp Zabel,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
netdev, devicetree, linux-kernel, linux-doc, linux-arm-kernel,
linux-mediatek, llvm
In-Reply-To: <6a421c2d.1e0bfc4f.b2eb6.06fb@mx.google.com>
On 6/29/26 09:18, Christian Marangi wrote:
> Nobody stops from implementing custom parser in fill_available_pcs and
> return a PCS pointer created directly by the MAC. (In such case late and
> release won't be needed as everything is present when phylink_create is
> called)
Ah alright :)
So my only request then would be to update the phylink porting guide to at
least point to the PCS documentation you're adding to say that the select_pcs()
op is deprecated :
https://docs.kernel.org/networking/sfp-phylink.html#rough-guide-to-converting-a-network-driver-to-sfp-phylink
Thanks,
Maxime
>
^ permalink raw reply
* RE: [PATCH] soc: imx8m: fix clock reference leak in imx8m_soc_prepare()
From: Peng Fan (OSS) @ 2026-06-29 7:21 UTC (permalink / raw)
To: Felix Gu, Frank Li, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Marco Felsch, Shawn Guo
Cc: imx@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20260626-soc-imx8m-v1-1-62657e3376dc@gmail.com>
> Subject: [PATCH] soc: imx8m: fix clock reference leak in
> imx8m_soc_prepare()
>
> When clk_prepare_enable() fails, the error path does not release the
> clock reference obtained by of_clk_get_by_name(). Add clk_put() to
> the error path before iounmap().
>
> Fixes: 390c01073f5d ("soc: imx8m: Cleanup with adding
> imx8m_soc_[un]prepare")
> Signed-off-by: Felix Gu <ustc.gu@gmail.com>
> ---
> drivers/soc/imx/soc-imx8m.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/soc/imx/soc-imx8m.c b/drivers/soc/imx/soc-
> imx8m.c index fc080e56f50d..de19972435f9 100644
> --- a/drivers/soc/imx/soc-imx8m.c
> +++ b/drivers/soc/imx/soc-imx8m.c
> @@ -150,10 +150,12 @@ static int imx8m_soc_prepare(struct
> platform_device *pdev, const char *ocotp_com
>
> ret = clk_prepare_enable(drvdata->clk);
> if (ret)
> - goto err_clk;
> + goto put_clk;
>
> return 0;
>
> +put_clk:
> + clk_put(drvdata->clk);
> err_clk:
It should be safe to directly put "clk_put" here, no
need extra label.
Regards
Peng.
> iounmap(drvdata->ocotp_base);
> return ret;
>
> ---
> base-commit: 30ffa8de54e5cc80d93fd211ca134d1764a7011f
> change-id: 20260626-soc-imx8m-938e89104044
>
> Best regards,
> --
> Felix Gu <ustc.gu@gmail.com>
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: iio: adc: Add Nuvoton MA35D1 EADC
From: Chi-Wen Weng @ 2026-06-29 7:11 UTC (permalink / raw)
To: David Lechner, jic23, robh, krzk+dt, conor+dt
Cc: nuno.sa, andy, linux-arm-kernel, linux-iio, devicetree,
linux-kernel, cwweng
In-Reply-To: <40485b4e-6585-42a1-9b84-3019328574c5@baylibre.com>
Hi David,
Thanks for the review.
> Datasheet says there are 4 interrupts.
Yes, the controller has four EOC interrupt outputs, ADINT0 to ADINT3.
The initial driver only uses ADINT0, but the hardware does provide four
interrupt lines.
I will update the binding to allow up to four interrupt entries and
document the order as ADINT0, ADINT1, ADINT2 and ADINT3. The example
will keep a single interrupt entry since that is the only one used by
the initial driver.
> Should there be an optional vref-supply for the V_REF pin?
Yes, I agree. I will add an optional vref-supply property.
I will also update the driver so that it does not force the external
reference path unconditionally. If vref-supply is present, the driver
will enable it and use it to report the ADC scale. Otherwise, the driver
will use the internal reference path.
> Should there be a dmas property? Datasheet says it supports PDMA
transfer.
The hardware does support PDMA, but DMA support is intentionally not
included in this initial upstream version. The initial driver will only
support interrupt-driven direct raw reads, and the MA35D1 PDMA provider
is not upstream yet.
I would prefer to leave dmas/dma-names out of the initial binding and
add them later together with DMA support. Please let me know if you
would prefer optional DMA properties to be described now.
> I assume 8 is for the internal batter voltage channel? Often, we don't
> include fixed internal channels like this in the devicetree since they
> are always the same and don't depend on external wiring.
Correct. Channels 0 to 7 are the external ADC input pins, while channel
8 is the internal VBAT input. I will limit the DT child channel nodes to
external channels 0 to 7.
If VBAT support is added later, it can be exposed by the driver as a
fixed internal channel rather than being described by devicetree.
> adc.yaml already specifies minItems and maxItems, so we don't need to
> repeat it.
Since I plan to simplify v2 and drop differential channel support from
the initial submission, I will remove diff-channels from the initial
binding.
Differential input support can be added later once the fixed hardware
pair constraints and signed output handling are implemented in the
driver.
> This (and reg) are uint32, so don't really need minimum: 0.
>
> Also, I assume that 8 is for the internal battery voltage channel,
> which wouldn't make sense as part of a differential input.
Will fix. The v2 binding will restrict child nodes to external channels
0 to 7 and drop the unnecessary minimum: 0.
Thanks,
Chi-Wen
David Lechner 於 2026/6/28 上午 04:05 寫道:
> On 6/25/26 6:06 AM, Chi-Wen Weng wrote:
>> From: Chi-Wen Weng <cwweng@nuvoton.com>
>>
>> Add devicetree binding for the Enhanced ADC controller found on
>> Nuvoton MA35D1 SoCs.
>>
>> The controller has one register region, one interrupt and one functional
>> clock. ADC inputs are described using standard channel child nodes,
>> including optional differential channel pairs.
>>
>> Signed-off-by: Chi-Wen Weng <cwweng@nuvoton.com>
>> ---
>> .../bindings/iio/adc/nuvoton,ma35d1-eadc.yaml | 100 ++++++++++++++++++
>> 1 file changed, 100 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/iio/adc/nuvoton,ma35d1-eadc.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton,ma35d1-eadc.yaml b/Documentation/devicetree/bindings/iio/adc/nuvoton,ma35d1-eadc.yaml
>> new file mode 100644
>> index 000000000000..ae7ad0f7689a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,ma35d1-eadc.yaml
>> @@ -0,0 +1,100 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/iio/adc/nuvoton,ma35d1-eadc.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Nuvoton MA35D1 Enhanced Analog to Digital Converter
>> +
>> +maintainers:
>> + - Chi-Wen Weng <cwweng@nuvoton.com>
>> +
>> +description: |
>> + The Nuvoton MA35D1 Enhanced Analog to Digital Converter (EADC) is a
>> + 12-bit ADC controller integrated in the MA35D1 SoC. Each enabled ADC
>> + input is described by a child channel node.
>> +
>> +properties:
>> + compatible:
>> + const: nuvoton,ma35d1-eadc
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + interrupts:
>> + maxItems: 1
> Datasheet says there are 4 interrupts.
>
>> +
>> + clocks:
>> + maxItems: 1
> Should there be an optional vref-supply for the V_REF pin?
>
> Should there be a dmas property? Datasheet says it supports
> PDMA transfer.
>
>> +
>> + '#address-cells':
>> + const: 1
>> +
>> + '#size-cells':
>> + const: 0
>> +
>> +patternProperties:
>> + '^channel@[0-8]$':
>> + type: object
>> + $ref: adc.yaml
>> + unevaluatedProperties: false
>> +
>> + properties:
>> + reg:
>> + minimum: 0
>> + maximum: 8
> I assume 8 is for the internal batter voltage channel? Often, we don't
> include fixed internal channels like this in the devicetree since they
> are always the same and don't depend on external wiring.
>
>> +
>> + diff-channels:
>> + minItems: 2
>> + maxItems: 2
> adc.yaml already specifies minItems and maxItems, so we don't need to repeat it.
>
>> + items:
>> + minimum: 0
>> + maximum: 8
> This (and reg) are uint32, so don't really need minimum: 0.
>
> Also, I assume that 8 is for the internal battery voltage channel, which
> wouldn't make sense as part of a differential input.
>
>> +
>> + required:
>> + - reg
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - interrupts
>> + - clocks
>> + - '#address-cells'
>> + - '#size-cells'
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>> + #include <dt-bindings/interrupt-controller/irq.h>
>> +
>> + soc {
>> + #address-cells = <2>;
>> + #size-cells = <2>;
>> +
>> + adc@40430000 {
>> + compatible = "nuvoton,ma35d1-eadc";
>> + reg = <0x0 0x40430000 0x0 0x10000>;
>> + interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>;
>> + clocks = <&clk EADC_GATE>;
>> +
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + channel@0 {
>> + reg = <0>;
>> + };
>> +
>> + channel@1 {
>> + reg = <1>;
>> + };
>> +
>> + channel@2 {
>> + reg = <2>;
>> + diff-channels = <2 3>;
>> + };
>> + };
>> + };
>> +...
^ permalink raw reply
* Re: [PATCH v3 2/4] clk: en7523: add support for dedicated PCIe PERSTOUT reset
From: Philipp Zabel @ 2026-06-29 7:07 UTC (permalink / raw)
To: Christian Marangi, Bjorn Helgaas, Lorenzo Pieralisi,
Krzysztof Wilczyński, Manivannan Sadhasivam, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Ryder Lee, Michael Turquette,
Stephen Boyd, Brian Masney, Matthias Brugger,
AngeloGioacchino Del Regno, Jianjun Wang, linux-pci, devicetree,
linux-kernel, linux-mediatek, linux-clk, linux-arm-kernel
In-Reply-To: <20260627121450.3529133-3-ansuelsmth@gmail.com>
On Sa, 2026-06-27 at 14:14 +0200, Christian Marangi wrote:
> Add support for resetting the PCIe lines with the PERSTOUT reset. These
> special reset are controlled by the PCIC register and are specific to each
> of the 3 PCIe lines.
>
> Notice that reset logic is inverted for these bit where 0 is assert and 1
> deassert. This is intenrally handled in the reset function.
^^
typo: internally
> PCI enable/disable are updated to drop PERSTOUT bits in favor dedicated
> reset handling.
>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
> drivers/clk/clk-en7523.c | 39 ++++++++++++++++++++++++++++-----------
> 1 file changed, 28 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/clk/clk-en7523.c b/drivers/clk/clk-en7523.c
> index 1ab0e2eca5d3..c9b21d9bf2f3 100644
> --- a/drivers/clk/clk-en7523.c
> +++ b/drivers/clk/clk-en7523.c
[...]
> @@ -754,14 +756,21 @@ static int en7523_reset_update(struct reset_controller_dev *rcdev,
> unsigned long id, bool assert)
> {
> struct en_rst_data *rst_data = container_of(rcdev, struct en_rst_data, rcdev);
> - void __iomem *addr = rst_data->base + rst_data->bank_ofs[id / RST_NR_PER_BANK];
> + u32 offset = rst_data->bank_ofs[id / RST_NR_PER_BANK];
> + void __iomem *addr = rst_data->base + offset;
> + bool inverted = false;
> u32 val;
>
> + /* For PCIC reset logic is inverted, 0:assert 1:deassert*/
> + if (offset == REG_NP_SCU_PCIC)
> + inverted = true;
> +
> val = readl(addr);
> + val &= ~BIT(id % RST_NR_PER_BANK);
> if (assert)
> - val |= BIT(id % RST_NR_PER_BANK);
> + val |= inverted ? 0 : BIT(id % RST_NR_PER_BANK);
> else
> - val &= ~BIT(id % RST_NR_PER_BANK);
> + val |= inverted ? BIT(id % RST_NR_PER_BANK) : 0;
You can simplify this into a single (assert ^ inverted) condition.
regards
Philipp
^ permalink raw reply
* Re: [PATCH 2/2] iio: adc: Add Nuvoton MA35D1 EADC driver
From: Chi-Wen Weng @ 2026-06-29 7:06 UTC (permalink / raw)
To: Andy Shevchenko
Cc: jic23, robh, krzk+dt, conor+dt, dlechner, nuno.sa, andy,
linux-arm-kernel, linux-iio, devicetree, linux-kernel, cwweng
In-Reply-To: <aj52jaJK3qwvQu2k@ashevche-desk.local>
Hi Andy,
Thanks for the review and the kind words.
I will address the cleanup comments in v2:
- trim the include list and add the missing specific headers such as
linux/types.h and linux/jiffies.h,
- rename the RMW helper to ma35d1_adc_update(),
- mask the update value in the helper,
- add set/clear bits helpers,
- use loop-local unsigned int indices where applicable,
- use devm_kasprintf() or drop the channel name handling if it is no
longer needed,
- switch to device_for_each_child_node_scoped(),
- simplify the IRQ and triggered-buffer error paths.
> > + if (have_single && have_diff)
> > + return -EINVAL;
>
> Is it possible IRL?
The EADC differential enable bit is global, so the check was intended to
reject buffered scans containing both single-ended and differential
channels.
However, after looking at the hardware constraints and the other review
comments, I plan to simplify v2 and reduce the initial upstream driver
scope. The v2 driver will focus on direct raw reads for the external
single-ended ADC channels only.
Triggered buffered capture, the device trigger and differential channel
support will be dropped from the initial submission. They can be added
later as follow-up patches once the scan sequencing, trigger model and
differential pair constraints are handled properly.
Thanks,
Chi-Wen
Andy Shevchenko 於 2026/6/26 下午 08:54 寫道:
> On Thu, Jun 25, 2026 at 07:06:38PM +0800, Chi-Wen Weng wrote:
>
>> Add an IIO driver for the Nuvoton MA35D1 Enhanced ADC controller.
>>
>> The driver supports direct raw reads and triggered buffered capture. The
>> controller end-of-conversion interrupt is exposed as the device trigger
>> and is used to push samples into the IIO buffer.
>>
>> Channels are described by firmware child nodes and can be configured as
>> single-ended or differential inputs. Since the differential enable bit is
>> global, mixed single-ended and differential buffered scans are rejected.
>>
>> DMA support is intentionally not included in this initial upstream driver;
>> conversions are handled through the interrupt-driven path.
> Nice written driver, some small issues here and there, and I think in a couple
> of versions it will stabilize and can be accepted.
>
> ...
>
>> +#include <linux/bitfield.h>
>> +#include <linux/bits.h>
> No need, bitmap.h covers this.
>
>> +#include <linux/bitmap.h>
>> +#include <linux/clk.h>
>> +#include <linux/completion.h>
>> +#include <linux/device.h>
> No need, covered by platform_device.h.
>
>> +#include <linux/err.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/kernel.h>
> No way this header should be in the mere drivers.
>
>> +#include <linux/module.h>
>> +#include <linux/mod_devicetable.h>
>> +#include <linux/mutex.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pm.h>
>> +#include <linux/property.h>
> Also missing some headers, such as types.h.
>
> ...
>
>> +#define MA35D1_EADC_TIMEOUT msecs_to_jiffies(1000)
> + jiffies.h
>
> ...
>
>> +static inline u32 ma35d1_adc_read(struct ma35d1_adc *adc, u32 reg)
>> +{
>> + return readl(adc->regs + reg);
>> +}
>> +
>> +static inline void ma35d1_adc_write(struct ma35d1_adc *adc, u32 reg, u32 val)
>> +{
>> + writel(val, adc->regs + reg);
>> +}
>> +
>> +static void ma35d1_adc_rmw(struct ma35d1_adc *adc, u32 reg, u32 mask, u32 val)
> Name it _update() to be aligned with the _read() and _write() above.
>
>> +{
>> + u32 tmp;
>> +
>> + tmp = ma35d1_adc_read(adc, reg);
>> + tmp &= ~mask;
>> + tmp |= val;
> Correct pattern is to use
>
> tmp = (tmp & ~mask) | (val & mask);
>
>> + ma35d1_adc_write(adc, reg, tmp);
>> +}
> ...
>
>> +static void ma35d1_adc_config_sample(struct ma35d1_adc *adc,
>> + unsigned int sample, unsigned int channel)
>> +{
>> + u32 reg = MA35D1_EADC_SCTL(sample);
> I don't see the need of this variable, use the value directly.
>
>> + ma35d1_adc_rmw(adc, reg,
>> + MA35D1_EADC_SCTL_CHSEL_MASK |
>> + MA35D1_EADC_SCTL_TRGSEL_MASK,
>> + FIELD_PREP(MA35D1_EADC_SCTL_CHSEL_MASK, channel) |
>> + MA35D1_EADC_SCTL_TRGSEL_ADINT0);
>> +}
> ...
>
>> +static irqreturn_t ma35d1_adc_trigger_handler(int irq, void *p)
>> +{
>> + struct iio_poll_func *pf = p;
>> + struct iio_dev *indio_dev = pf->indio_dev;
>> + struct ma35d1_adc *adc = iio_priv(indio_dev);
>> + int i;
>> +
>> + for (i = 0; i < adc->scan_chancnt; i++)
> for (unsigned int i = 0; i < adc->scan_chancnt; i++)
>
>> + adc->scan.channels[i] =
>> + ma35d1_adc_read(adc, MA35D1_EADC_DAT(i)) &
>> + MA35D1_EADC_DAT_MASK;
>> +
>> + iio_push_to_buffers_with_timestamp(indio_dev, &adc->scan, pf->timestamp);
>> + iio_trigger_notify_done(adc->trig);
>> +
>> + ma35d1_adc_rmw(adc, MA35D1_EADC_CTL, MA35D1_EADC_CTL_ADCIEN0,
>> + MA35D1_EADC_CTL_ADCIEN0);
>> + ma35d1_adc_write(adc, MA35D1_EADC_SWTRG, 1);
>> +
>> + return IRQ_HANDLED;
>> +}
> ...
>
>> +static int ma35d1_adc_validate_scan(struct iio_dev *indio_dev,
>> + const unsigned long *scan_mask)
>> +{
>> + const struct iio_chan_spec *chan;
>> + bool have_single = false;
>> + bool have_diff = false;
>> + unsigned int count = 0;
>> + unsigned long bit;
>> +
>> + for_each_set_bit(bit, scan_mask, indio_dev->masklength) {
>> + chan = &indio_dev->channels[bit];
>> +
>> + if (chan->type == IIO_TIMESTAMP)
>> + continue;
>> + count++;
> Make it last in the loop, it will be standard pattern. Otherwise it's hard to
> read and find. Also it's recommended to split assignment and definition
> for better maintenance.
>
> unsigned int count;
> ...
> count = 0;
> for_each_set_bit(bit, scan_mask, indio_dev->masklength) {
> ...
> count++;
> }
>
>> + if (chan->differential)
>> + have_diff = true;
>> + else
>> + have_single = true;
>> + }
>> +
>> + if (!count || count > MA35D1_EADC_MAX_SAMPLE_MODULES)
>> + return -EINVAL;
>> + if (have_single && have_diff)
>> + return -EINVAL;
> Is it possible IRL?
>
>> + return 0;
>> +}
> ...
>
>> +static int ma35d1_adc_buffer_postenable(struct iio_dev *indio_dev)
>> +{
>> + struct ma35d1_adc *adc = iio_priv(indio_dev);
>> + int i;
>> +
>> + if (!adc->scan_chancnt)
>> + return -EINVAL;
>> +
>> + ma35d1_adc_write(adc, MA35D1_EADC_STATUS2, MA35D1_EADC_STATUS2_ADIF0);
>> + ma35d1_adc_rmw(adc, MA35D1_EADC_INTSRC0,
>> + MA35D1_EADC_INTSRC0_ADINT0,
>> + MA35D1_EADC_INTSRC0_ADINT0);
>> + ma35d1_adc_rmw(adc, MA35D1_EADC_REFADJCTL,
>> + MA35D1_EADC_REFADJCTL_EXT_VREF,
>> + MA35D1_EADC_REFADJCTL_EXT_VREF);
>> + ma35d1_adc_rmw(adc, MA35D1_EADC_SELSMP0, GENMASK(1, 0), 3);
>> + ma35d1_adc_set_diff(adc, adc->scan_differential);
>> + for (i = 0; i < adc->scan_chancnt; i++)
> for (unsigned int i = 0; i < adc->scan_chancnt; i++)
>
>> + ma35d1_adc_rmw(adc, MA35D1_EADC_SCTL(i),
>> + MA35D1_EADC_SCTL_TRGDLY_MASK,
>> + MA35D1_EADC_SCTL_TRGDLY_MASK);
>> +
>> + ma35d1_adc_rmw(adc, MA35D1_EADC_CTL, MA35D1_EADC_CTL_ADCIEN0,
>> + MA35D1_EADC_CTL_ADCIEN0);
>> + ma35d1_adc_write(adc, MA35D1_EADC_SWTRG, 1);
>> +
>> + return 0;
>> +}
>> +
>> +static int ma35d1_adc_buffer_predisable(struct iio_dev *indio_dev)
>> +{
>> + struct ma35d1_adc *adc = iio_priv(indio_dev);
>> + int i;
>> +
>> + ma35d1_adc_disable_irq(adc);
>> + for (i = 0; i < adc->scan_chancnt; i++)
>> + ma35d1_adc_rmw(adc, MA35D1_EADC_SCTL(i),
>> + MA35D1_EADC_SCTL_TRGSEL_MASK, 0);
> Ditto.
>
> Also looking to the cases of setting 0s, I would rather have a helper
> _set_bits() / _clear_bits() in conjunction with _update().
>
>> + return 0;
>> +}
> ...
>
>> +static void ma35d1_adc_init_channel(struct ma35d1_adc *adc,
>> + struct iio_chan_spec *chan, u32 vinp,
>> + u32 vinn, int scan_index, bool differential)
>> +{
>> + char *name = adc->chan_name[vinp];
>> +
>> + chan->type = IIO_VOLTAGE;
>> + chan->indexed = 1;
>> + chan->channel = vinp;
>> + chan->address = vinp;
>> + chan->scan_index = scan_index;
>> + chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
>> + chan->scan_type.sign = 'u';
>> + chan->scan_type.realbits = 12;
>> + chan->scan_type.storagebits = 16;
>> + chan->scan_type.endianness = IIO_CPU;
>> +
>> + if (differential) {
>> + chan->differential = 1;
>> + chan->channel2 = vinn;
>> + snprintf(name, MA35D1_EADC_CHAN_NAME_LEN, "in%d-in%d", vinp,
>> + vinn);
> Can compiler prove the buffer size is enough?
>
>> + } else {
>> + snprintf(name, MA35D1_EADC_CHAN_NAME_LEN, "in%d", vinp);
>> + }
>> +
>> + chan->datasheet_name = name;
> Why not use devm_kasprintf() instead?
>
>> +}
> ...
>
>> +static int ma35d1_adc_parse_channels(struct iio_dev *indio_dev,
>> + struct device *dev)
>> +{
>> + struct ma35d1_adc *adc = iio_priv(indio_dev);
>> + DECLARE_BITMAP(used_channels, MA35D1_EADC_MAX_CHANNELS);
>> + struct fwnode_handle *child;
>> + struct iio_chan_spec *channels;
>> + int num_channels;
>> + int scan_index = 0;
>> + int ret;
>> +
>> + bitmap_zero(used_channels, MA35D1_EADC_MAX_CHANNELS);
>> +
>> + num_channels = device_get_child_node_count(dev);
>> + if (!num_channels)
>> + return dev_err_probe(dev, -ENODATA,
>> + "no ADC channels configured\n");
>> +
>> + if (num_channels > MA35D1_EADC_MAX_CHANNELS)
> Perhaps >= ?
>
>> + return dev_err_probe(dev, -EINVAL, "too many ADC channels\n");
>> +
>> + channels = devm_kcalloc(dev, num_channels + 1, sizeof(*channels),
>> + GFP_KERNEL);
>> + if (!channels)
>> + return -ENOMEM;
>> +
>> + device_for_each_child_node(dev, child) {
> Use _scoped() variant.
>
>> + u32 diff[2];
>> + u32 reg;
>> + bool differential = false;
>> +
>> + ret = fwnode_property_read_u32(child, "reg", ®);
>> + if (ret) {
>> + fwnode_handle_put(child);
>> + return dev_err_probe(dev, ret,
>> + "missing channel reg property\n");
>> + }
>> +
>> + if (reg >= MA35D1_EADC_MAX_CHANNELS) {
>> + fwnode_handle_put(child);
>> + return dev_err_probe(dev, -EINVAL,
>> + "invalid ADC channel %u\n", reg);
>> + }
>> +
>> + if (test_and_set_bit(reg, used_channels)) {
>> + fwnode_handle_put(child);
>> + return dev_err_probe(dev, -EINVAL,
>> + "duplicate ADC channel %u\n", reg);
>> + }
>> +
>> + if (fwnode_property_present(child, "diff-channels")) {
>> + ret = fwnode_property_read_u32_array(child,
>> + "diff-channels",
>> + diff,
>> + ARRAY_SIZE(diff));
>> + if (ret) {
>> + fwnode_handle_put(child);
>> + return dev_err_probe(dev, ret,
>> + "invalid diff-channels for channel %u\n",
>> + reg);
>> + }
>> +
>> + if (diff[0] != reg ||
>> + diff[1] >= MA35D1_EADC_MAX_CHANNELS ||
>> + diff[0] == diff[1]) {
>> + fwnode_handle_put(child);
>> + return dev_err_probe(dev, -EINVAL,
>> + "invalid differential ADC channel %u-%u\n",
>> + diff[0], diff[1]);
>> + }
>> +
>> + if (test_and_set_bit(diff[1], used_channels)) {
>> + fwnode_handle_put(child);
>> + return dev_err_probe(dev, -EINVAL,
>> + "ADC channel %u already used\n",
>> + diff[1]);
>> + }
>> +
>> + differential = true;
>> + }
>> +
>> + ma35d1_adc_init_channel(adc, &channels[scan_index], reg,
>> + differential ? diff[1] : 0,
>> + scan_index, differential);
>> + scan_index++;
>> + }
>> +
>> + channels[scan_index] = (struct iio_chan_spec)
>> + IIO_CHAN_SOFT_TIMESTAMP(scan_index);
>> +
>> + indio_dev->channels = channels;
>> + indio_dev->num_channels = scan_index + 1;
>> + indio_dev->masklength = indio_dev->num_channels;
>> +
>> + return 0;
>> +}
> ...
>
>> + ret = devm_request_irq(dev, irq, ma35d1_adc_isr, 0, dev_name(dev),
>> + indio_dev);
> Make it a single line, here it's fine.
>
>> + if (ret)
>> + return dev_err_probe(dev, ret, "failed to request IRQ %d\n", irq);
> Remove duplicate error message.
>
> ...
>
>> + ret = devm_iio_triggered_buffer_setup(dev, indio_dev,
>> + iio_pollfunc_store_time,
>> + ma35d1_adc_trigger_handler,
>> + &ma35d1_adc_buffer_ops);
>> + if (ret)
>> + return dev_err_probe(dev, ret,
>> + "failed to setup triggered buffer\n");
> So, it seems this is very rarely can be not -ENOMEM, and hence it's 99.99% dead
> code, just
>
> return ret;
>
^ permalink raw reply
* Re: [RFC PATCH net-next v8 03/12] net: phylink: add phylink_release_pcs() to externally release a PCS
From: Maxime Chevallier @ 2026-06-29 7:04 UTC (permalink / raw)
To: Christian Marangi
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Simon Horman, Jonathan Corbet, Shuah Khan, Lorenzo Bianconi,
Heiner Kallweit, Russell King, Saravana Kannan, Philipp Zabel,
Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
netdev, devicetree, linux-kernel, linux-doc, linux-arm-kernel,
linux-mediatek, llvm
In-Reply-To: <6a3fc312.6161eb1e.3441bb.c0de@mx.google.com>
Hi Christian,
On 6/27/26 14:33, Christian Marangi wrote:
> On Thu, Jun 25, 2026 at 04:13:14PM +0200, Maxime Chevallier wrote:
>> Hello Christian,
>>
>> On 6/18/26 14:57, Christian Marangi wrote:
>>> Add phylink_release_pcs() to externally release a PCS from a phylink
>>> instance. This can be used to handle case when a single PCS needs to be
>>> removed and the phylink instance needs to be refreshed.
>>>
>>> On calling phylink_release_pcs(), the PCS will be removed from the
>>> phylink internal PCS list and the phylink supported_interfaces value is
>>> reparsed with the remaining PCS interfaces.
>>>
>>> Also a phylink resolve is triggered to handle the PCS removal.
>>>
>>> The flag force_major_config is set to make phylink resolve reconfigure
>>> the interface (even if it didn't change).
>>> This is needed to handle the special case when the current PCS used
>>> by phylink is removed and a major_config is needed to propagae the
>>> configuration change. With this option enabled we also force mac_config
>>> even if the PHY link is not up for the in-band case.
>>>
>>> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
>>> ---
>>> drivers/net/phy/phylink.c | 56 +++++++++++++++++++++++++++++++++++++++
>>> include/linux/phylink.h | 2 ++
>>> 2 files changed, 58 insertions(+)
>>>
>>> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
>>> index c38bcd43b8c8..064d6f5a06da 100644
>>> --- a/drivers/net/phy/phylink.c
>>> +++ b/drivers/net/phy/phylink.c
>>> @@ -158,6 +158,8 @@ static const phy_interface_t phylink_sfp_interface_preference[] = {
>>> static DECLARE_PHY_INTERFACE_MASK(phylink_sfp_interfaces);
>>>
>>> static void phylink_run_resolve(struct phylink *pl);
>>> +static void phylink_link_down(struct phylink *pl);
>>> +static void phylink_pcs_disable(struct phylink_pcs *pcs);
>>>
>>> /**
>>> * phylink_set_port_modes() - set the port type modes in the ethtool mask
>>> @@ -918,6 +920,60 @@ static void phylink_resolve_an_pause(struct phylink_link_state *state)
>>> }
>>> }
>>>
>>> +/**
>>> + * phylink_release_pcs - Removes a PCS from the phylink PCS available list
>>> + * @pcs: a pointer to the phylink_pcs struct to be released
>>> + *
>>> + * This function release a PCS from the phylink PCS available list if
>>> + * actually in use. It also refreshes the supported interfaces of the
>>> + * phylink instance by copying the supported interfaces from the phylink
>>> + * conf and merging the supported interfaces of the remaining available PCS
>>> + * in the list and trigger a resolve.
>>> + */
>>> +void phylink_release_pcs(struct phylink_pcs *pcs)
>>> +{
>>> + struct phylink *pl;
>>> +
>>> + ASSERT_RTNL();
>>> +
>>> + pl = pcs->phylink;
>>> + if (!pl)
>>> + return;
>>> +
>>> + mutex_lock(&pl->state_mutex);
>>> +
>>> + list_del(&pcs->list);
>>> + pcs->phylink = NULL;
>>> +
>>> + /*
>>> + * Check if we are removing the PCS currently
>>> + * in use by phylink. If this is the case, tear down
>>> + * the link, force phylink resolve to reconfigure the
>>> + * interface mode, disable the current PCS and set the
>>> + * phylink PCS to NULL.
>>> + */
>>> + if (pl->pcs == pcs) {
>>> + phylink_link_down(pl);
>>> + phylink_pcs_disable(pl->pcs);
>>> +
>>> + pl->force_major_config = true;
>>> + pl->pcs = NULL;
>>> + }
>>> +
>>> + mutex_unlock(&pl->state_mutex);
>>> +
>>> + /* Refresh supported interfaces */
>>> + phy_interface_copy(pl->supported_interfaces,
>>> + pl->config->supported_interfaces);
>>> + list_for_each_entry(pcs, &pl->pcs_list, list)
>>> + phy_interface_or(pl->supported_interfaces,
>>> + pl->supported_interfaces,
>>> + pcs->supported_interfaces);
>>
>> I've given more thought to that 'supported_interfaces' thing. This
>> patchset redefines the meaning of
>>
>> pl->config->supported_interfaces
>>
>> Currently, it's filled by the MAC driver and means "Every interface
>> we can support, including the ones provided by PCSs that we can use
>> with this MAC".
>>
>> It now becomes "Every interface we support without needing a PCS", at
>> least the way I understand that.
>>
>
> Wait but with the current code using the OR logic, it still follows
> "Every interface we can support...". The modes that needs a PCS are
> specificed with the pcs_interfaces mask in phylink_config.
you current code is correct, I was mostly concerned about the doc
that goes along with it :)
So in the end, we'd have something like (simplified):
pl->config.supported_interfaces = RGMII_xx | SGMII | 1000BaseX
pl->config.pcs_interfaces = SGMII | 1000BaseX
pcs->supported_interface = SGMII| 1000BaseX
correct ?
>
> The late add and release operates on the phylink supported_interfaces ONLY
> when the MAC didn't specify support for it (by removing it as only the PCS
> will declare support for it)
>
> The confusion is present because everything is validated later on
> major_config so those supported_interfaces are just an HINT that are later
> verified with get_caps and with the pcs_validate OPs.
>
> Adding the supported_interfaces to phylink is really to keep an original
> reference of the value. This is to address a pattern I have notice where
> the MAC driver always OR the interfaces with the one supported by the PCS.
> (I remember it was pointed out by Russell)
>
> But I'm more than open to discussion as this is something marginal to the
> whole implementation, I'm also questioning if this OR is actually useful to
> anything on the nth tought on this.
>
> One thing that I notice is that parsing this early with AND might be
> problematic at phylink_create, but I still have to evaluate that.
>
> My take is that would be good to have some review also on the other logic
> as I think I reached a point where Sashiko starts to comments on more or
> less unreal problem.
True, TBH all the fwnode part is something I'm a bit less familiar with though
so maybe someone else can browse through that.
FWIW, I've tested that whole series on a board that has "legacy" PCS board
that has mvpp2 and 2 possible PCSs, and it seems to work fine so no regressions
there :)
A side note with the "legacy" naming, I'd rather have it called "built-in" or
something like that, I don't see a clear path to porting the existing code to
fwnode without breaking DT compat, as it's likely we'll have to remove the PCS
register ranges out of the MAC's range.
Thanks for this work anyway, this is great !
Maxime
^ 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