* [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
* [PATCH v26 7/7] arm64: dts: imx8ulp-evk: add reserved memory property
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>
Reserve 1MB of DDR memory region due to EdgeLock Enclave's hardware
limitation restricting access to DDR addresses from 0x80000000
to 0xafffffff.
Signed-off-by: Pankaj Gupta <pankaj.gupta@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
---
arch/arm64/boot/dts/freescale/imx8ulp-evk.dts | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts b/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts
index 5dea66c1e7aa..16399d921e04 100644
--- a/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8ulp-evk.dts
@@ -1,6 +1,6 @@
// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
/*
- * Copyright 2021 NXP
+ * Copyright 2021, 2025 NXP
*/
/dts-v1/;
@@ -37,6 +37,12 @@ linux,cma {
linux,cma-default;
};
+ ele_reserved: memory@90000000 {
+ compatible = "shared-dma-pool";
+ reg = <0 0x90000000 0 0x100000>;
+ no-map;
+ };
+
m33_reserved: noncacheable-section@a8600000 {
reg = <0 0xa8600000 0 0x1000000>;
no-map;
@@ -259,6 +265,10 @@ &usdhc0 {
status = "okay";
};
+&hsm0 {
+ memory-region = <&ele_reserved>;
+};
+
&fec {
pinctrl-names = "default", "sleep";
pinctrl-0 = <&pinctrl_enet>;
--
2.43.0
^ permalink raw reply related
* [PATCH v26 5/7] firmware: drivers: imx: adds miscdev
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,
sashiko-bot
In-Reply-To: <20260629-imx-se-if-v26-0-146446285744@nxp.com>
From: Pankaj Gupta <pankaj.gupta@nxp.com>
Adds the driver for communication interface to secure-enclave, that
enables exchanging messages with NXP secure enclave HW IP(s)
like EdgeLock Enclave, from:
- User-Space Applications via character driver.
ABI documentation for the NXP secure-enclave driver.
User-space library using this driver:
- i.MX Secure Enclave library:
-- URL: https://github.com/nxp-imx/imx-secure-enclave.git,
- i.MX Secure Middle-Ware:
-- URL: https://github.com/nxp-imx/imx-smw.git
Following checks are performed on the incoming msg-header,
to block exchanging invalid arbitrary commands:
- maximum allowed words,
- check if command-tag & response-tag are valid
- version,
- command id validation check, to allow limited base-line API(s)
and restrict following:
- exchanging power management commands.
- reset requests.
- BBSM configuration requests.
- re-initializing the FW.
- RNG init
- CAAM resource release management
- SE's internal memory management.
from user-space.
Signed-off-by: Pankaj Gupta <pankaj.gupta@nxp.com>
---
Changed from v25 to v26:
1. se_ctrl: serialize command receiver registration
SE_IOCTL_ENABLE_CMD_RCV updates the global command receiver state in
priv->cmd_receiver_clbk_hdl, but it is currently protected only by the
per-file dev_ctx->fops_lock. Concurrent ioctl calls from different file
descriptors can therefore race and register multiple receivers against
the same priv instance.
Protect command receiver registration with priv->modify_lock, a global
mutex lock, which serializes modification to priv structure members.
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260514090457.2186933-1-pankaj.gupta@nxp.com?part=1
2. se_ctrl: keep priv alive for already-open device contexts
Open file descriptors keep per-file se_if_device_ctx objects alive after
driver teardown begins, but those contexts retain a pointer to struct
se_if_priv. Since priv was allocated with devm, teardown could free it
while existing file descriptors were still able to reach read(), write(),
ioctl(), or release(), leading to use-after-free.
Fix this by moving priv to explicit lifetime management and adding a
kref to keep it alive until the last already-open device context is
released. Each new device context takes a reference on priv and drops it
on final file close.
During teardown, mark active device contexts as cleaned up and tear down
their per-context resources without freeing the dev_ctx itself. Existing
file descriptors then fail subsequent read(), write(), and ioctl()
operations with -ENODEV, while close() performs the final dev_ctx free
and drops the corresponding priv reference.
This also folds in the close-path cleanup fix so the device context is
not freed while still inside the fops lock protected cleanup flow.
This ensures that priv remains valid for device contexts that were
already opened before teardown and prevents use-after-free when those
contexts later reach read(), write(), ioctl(), or release().
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260514090457.2186933-1-pankaj.gupta@nxp.com?part=1
3. se_ctrl: pin active contexts and serialize during teardown
fixing the teardown-side synchronization for active contexts.
se_if_probe_cleanup() walks the global dev_ctx_list while close()
can concurrently act on the same se_if_device_ctx objects. Add a kref to
struct se_if_device_ctx and split global detach from local per-context
cleanup so teardown can pin one context, detach it from global state,
clean it outside the global lock, and only then drop the temporary
reference. This prevents the cleanup walk from racing with concurrent
close on the lifetime of the current context.
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260514090457.2186933-1-pankaj.gupta@nxp.com?part=1
4. se_ctrl: preserve ioctl error across cleanup
se_ioctl_cmd_snd_rcv_rsp_handler() can set err to -EFAULT when copying
the response payload to userspace fails, but then immediately overwrites
that error with the return value of se_ioctl_cmd_snd_rcv_cleanup().
If cleanup succeeds, the function returns 0 and falsely reports success
to userspace even though the response copy failed.
Preserve the original operation error and only use the cleanup return
value when no earlier error was set.
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260528091446.3331006-1-pankaj.gupta@nxp.com?part=1
5. se_ctrl: copy userspace messages before header validation
The write path and the synchronous command/response ioctl path validate
message headers directly from userspace pointers before copying the full
message into kernel memory. That violates uaccess rules and can fault on
architectures that enforce privileged access restrictions to userspace
memory.
It also creates a double-fetch window where userspace can modify the
buffer after header validation but before the payload is copied, causing
the validated header to no longer match the data actually used by the
driver.
Fix this by copying the TX message into kernel memory first and
validating the header from that kernel-owned buffer in both the write
path and the command-send ioctl path.
Also reject userspace-provided response buffer sizes that are smaller
than a message header or larger than the maximum supported response size
for debug-dump replies before allocating the response buffer.
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260528091532.3331051-1-pankaj.gupta@nxp.com?part=1
6. se_ctrl: roll back iobuf reservation on setup failure
se_ioctl_setup_iobuf_handler() reserves space from the per-file shared
memory pool before copying user data and linking a buffer descriptor.
If copy_from_user() or add_b_desc_to_pending_list() fails after the
reservation, the function returns without restoring shared_mem->pos,
which permanently consumes part of the shared memory pool for that file
descriptor.
In addition, add_b_desc_to_pending_list() failure currently falls
through to the copy_to_user() path, which can return an ELE address to
userspace even though the setup failed.
Fix both issues by rolling back the reserved shared-memory position on
pre-link failures and returning immediately when descriptor linking
fails instead of falling through to the success copyout path.
If copy_to_user() fails after the descriptor has already been linked,
unlink the descriptor before rolling back the reserved shared-memory
position so the pool state and pending list remain consistent.
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260528091532.3331051-1-pankaj.gupta@nxp.com?part=1
7. se_ctrl: detect round_up() overflow in iobuf setup
se_ioctl_setup_iobuf_handler() aligns io.length with round_up(..., 8)
before checking the available shared memory space. On 32-bit builds,
round_up() can overflow for large io.length values and wrap aligned_len
to a smaller value, which can bypass the bounds check while later
memset() still uses the original unbounded io.length.
Detect the overflow by checking whether the aligned value became smaller
than the original length. Valid alignment must never reduce the value,
so this catches wraparound without relying on a SIZE_MAX-based check that
triggers tautological-compare warnings on 64-bit builds.
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
8. se_ctrl: pin miscdev-ctx to serialize open during teardown
fixing the teardown-side synchronization for new open attempts.
se_if_fops_open() can still race with teardown after starting
from the miscdevice and attempting to reach priv. Introduce a small
open-gate object associated with the miscdevice. The open path first
pins this gate, checks whether teardown has started, and then safely
takes a temporary reference on priv before dereferencing priv state.
Teardown marks the gate dead and detaches priv from it before
misc_deregister(), so new opens either complete while priv is still
valid or fail with -ENODEV without touching stale driver state.
Reported-by: sashiko-bot <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260514090457.2186933-1-pankaj.gupta@nxp.com?part=1
---
Documentation/ABI/testing/se-cdev | 44 ++
drivers/firmware/imx/ele_base_msg.c | 28 +
drivers/firmware/imx/ele_base_msg.h | 19 +
drivers/firmware/imx/ele_common.c | 38 ++
drivers/firmware/imx/ele_common.h | 6 +
drivers/firmware/imx/se_ctrl.c | 1013 ++++++++++++++++++++++++++++++++++-
drivers/firmware/imx/se_ctrl.h | 55 ++
include/uapi/linux/se_ioctl.h | 97 ++++
8 files changed, 1284 insertions(+), 16 deletions(-)
diff --git a/Documentation/ABI/testing/se-cdev b/Documentation/ABI/testing/se-cdev
new file mode 100644
index 000000000000..c6b8e16bda78
--- /dev/null
+++ b/Documentation/ABI/testing/se-cdev
@@ -0,0 +1,44 @@
+What: /dev/<se>_mu[0-9]+_ch[0-9]+
+Date: Mar 2025
+KernelVersion: 6.8
+Contact: linux-imx@nxp.com, pankaj.gupta@nxp.com
+Description:
+ NXP offers multiple hardware IP(s) for secure enclaves like EdgeLock-
+ Enclave(ELE), SECO. The character device file descriptors
+ /dev/<se>_mu*_ch* are the interface between userspace NXP's secure-
+ enclave shared library and the kernel driver.
+
+ The ioctl(2)-based ABI is defined and documented in
+ [include]<linux/firmware/imx/ele_mu_ioctl.h>.
+ ioctl(s) are used primarily for:
+
+ - shared memory management
+ - allocation of I/O buffers
+ - getting mu info
+ - setting a dev-ctx as receiver to receive all the commands from FW
+ - getting SoC info
+ - send command and receive command response
+
+ The following file operations are supported:
+
+ open(2)
+ Currently the only useful flags are O_RDWR.
+
+ read(2)
+ Every read() from the opened character device context is waiting on
+ wait_event_interruptible, that gets set by the registered mailbox callback
+ function, indicating a message received from the firmware on message-
+ unit.
+
+ write(2)
+ Every write() to the opened character device context needs to acquire
+ mailbox_lock before sending message on to the message unit.
+
+ close(2)
+ Stops and frees up the I/O contexts that were associated
+ with the file descriptor.
+
+Users: https://github.com/nxp-imx/imx-secure-enclave.git,
+ https://github.com/nxp-imx/imx-smw.git,
+ crypto/skcipher,
+ drivers/nvmem/imx-ocotp-ele.c
diff --git a/drivers/firmware/imx/ele_base_msg.c b/drivers/firmware/imx/ele_base_msg.c
index 66bae4c7d464..ec718d322abc 100644
--- a/drivers/firmware/imx/ele_base_msg.c
+++ b/drivers/firmware/imx/ele_base_msg.c
@@ -15,6 +15,34 @@
#define FW_DBG_DUMP_FIXED_STR "ELE"
+int ele_uapi_allowed_base_cmd(struct se_if_priv *priv,
+ struct se_msg_hdr *header)
+{
+ switch (header->command) {
+ case ELE_PING_REQ: return 0;
+ case ELE_DEBUG_DUMP_REQ: return 0;
+ case ELE_OEM_AUTH_CONTAINER_REQ: return 0;
+ case ELE_OEM_VERIFY_IMAGE_REQ: return 0;
+ case ELE_OEM_REL_CONTAINER_REQ: return 0;
+ case ELE_FW_LIFE_CYCLE_REQ: return 0;
+ case ELE_READ_FUSE_REQ: return 0;
+ case ELE_GET_FW_VERS_REQ: return 0;
+ case ELE_RETURN_LIFE_CYCLE_REQ: return 0;
+ case ELE_GET_EVENT_REQ: return 0;
+ case ELE_COMMIT_REQ: return 0;
+ case ELE_GEN_KEY_BLOB_REQ: return 0;
+ case ELE_GET_FW_STATUS_REQ: return 0;
+ case ELE_XIP_DECRYPT_REQ: return 0;
+ case ELE_WRITE_FUSE: return 0;
+ case ELE_GET_INFO_REQ: return 0;
+ case ELE_DEV_ATTEST_REQ: return 0;
+ case ELE_WRITE_SHADOW_FUSE_REQ: return 0;
+ case ELE_READ_SHADOW_FUSE_REQ: return 0;
+ default:
+ return -EACCES;
+ }
+}
+
int ele_get_info(struct se_if_priv *priv, struct ele_dev_info *s_info)
{
dma_addr_t get_info_addr = 0;
diff --git a/drivers/firmware/imx/ele_base_msg.h b/drivers/firmware/imx/ele_base_msg.h
index 74f87f57d96b..75e65e279193 100644
--- a/drivers/firmware/imx/ele_base_msg.h
+++ b/drivers/firmware/imx/ele_base_msg.h
@@ -15,6 +15,23 @@
#define ELE_NONE_VAL 0x0
+#define ELE_OEM_AUTH_CONTAINER_REQ 0x87
+#define ELE_OEM_VERIFY_IMAGE_REQ 0x88
+#define ELE_OEM_REL_CONTAINER_REQ 0x89
+#define ELE_FW_LIFE_CYCLE_REQ 0x95
+#define ELE_READ_FUSE_REQ 0x97
+#define ELE_GET_FW_VERS_REQ 0x9d
+#define ELE_RETURN_LIFE_CYCLE_REQ 0xa0
+#define ELE_GET_EVENT_REQ 0xa2
+#define ELE_COMMIT_REQ 0xa8
+#define ELE_GEN_KEY_BLOB_REQ 0xaf
+#define ELE_GET_FW_STATUS_REQ 0xc5
+#define ELE_XIP_DECRYPT_REQ 0xc6
+#define ELE_WRITE_FUSE 0xd6
+#define ELE_DEV_ATTEST_REQ 0xdb
+#define ELE_WRITE_SHADOW_FUSE_REQ 0xf2
+#define ELE_READ_SHADOW_FUSE_REQ 0xf3
+
#define ELE_GET_INFO_REQ 0xda
#define ELE_GET_INFO_REQ_MSG_SZ 0x10
#define ELE_GET_INFO_RSP_MSG_SZ 0x08
@@ -95,4 +112,6 @@ int ele_service_swap(struct se_if_priv *priv, phys_addr_t addr,
int ele_fw_authenticate(struct se_if_priv *priv, phys_addr_t contnr_addr,
phys_addr_t img_addr);
int ele_debug_dump(struct se_if_priv *priv);
+int ele_uapi_allowed_base_cmd(struct se_if_priv *priv,
+ struct se_msg_hdr *header);
#endif
diff --git a/drivers/firmware/imx/ele_common.c b/drivers/firmware/imx/ele_common.c
index b37ea1f14f75..79be0f8ed825 100644
--- a/drivers/firmware/imx/ele_common.c
+++ b/drivers/firmware/imx/ele_common.c
@@ -6,6 +6,23 @@
#include "ele_base_msg.h"
#include "ele_common.h"
+int se_chk_tx_msg_hdr(struct se_if_priv *priv, struct se_msg_hdr *header)
+{
+ if (!header->size || header->size > MAX_WORD_SIZE)
+ return -EINVAL;
+
+ if (header->tag != priv->if_defs->cmd_tag &&
+ header->tag != priv->if_defs->rsp_tag)
+ return -EINVAL;
+
+ if (header->ver == priv->if_defs->base_api_ver)
+ return ele_uapi_allowed_base_cmd(priv, header);
+ else if (header->ver == priv->if_defs->fw_api_ver)
+ return 0;
+
+ return -EINVAL;
+}
+
/*
* se_get_msg_chksum() - to calculate checksum word by word.
*
@@ -42,15 +59,36 @@ u32 se_get_msg_chksum(u32 *msg, u32 msg_len)
return chksum;
}
+void set_se_rcv_msg_timeout(struct se_if_priv *priv, u32 timeout_ms)
+{
+ priv->se_rcv_msg_timeout_ms = timeout_ms;
+}
+
int ele_msg_rcv(struct se_if_device_ctx *dev_ctx, struct se_clbk_handle *se_clbk_hdl)
{
+ bool is_rsp_wait_with_timeout = false;
bool wait_uninterruptible = false;
unsigned long remaining_jiffies;
+ unsigned long deadline_jiffies;
+ unsigned long timeout_jiffies;
unsigned long flags;
int ret;
remaining_jiffies = MAX_SCHEDULE_TIMEOUT;
+ if (dev_ctx->priv->cmd_receiver_clbk_hdl.dev_ctx != dev_ctx) {
+ is_rsp_wait_with_timeout = true;
+ timeout_jiffies = msecs_to_jiffies(dev_ctx->priv->se_rcv_msg_timeout_ms);
+ deadline_jiffies = jiffies + timeout_jiffies;
+ }
do {
+ if (is_rsp_wait_with_timeout) {
+ if (time_after_eq(jiffies, deadline_jiffies)) {
+ ret = -ETIMEDOUT;
+ break;
+ }
+ remaining_jiffies = deadline_jiffies - jiffies;
+ }
+
if (wait_uninterruptible)
ret = wait_for_completion_timeout(&se_clbk_hdl->done,
remaining_jiffies);
diff --git a/drivers/firmware/imx/ele_common.h b/drivers/firmware/imx/ele_common.h
index 5bac14439d7d..bdc13d1b715d 100644
--- a/drivers/firmware/imx/ele_common.h
+++ b/drivers/firmware/imx/ele_common.h
@@ -12,6 +12,11 @@
#define IMX_ELE_FW_DIR "imx/ele/"
+#define MAX_WORD_SIZE 0x20
+#define SE_RCV_MSG_DEFAULT_TIMEOUT 5000
+#define SE_RCV_MSG_LONG_TIMEOUT 5000000
+
+void set_se_rcv_msg_timeout(struct se_if_priv *priv, u32 val);
u32 se_get_msg_chksum(u32 *msg, u32 msg_len);
int ele_msg_rcv(struct se_if_device_ctx *dev_ctx, struct se_clbk_handle *se_clbk_hdl);
@@ -42,4 +47,5 @@ int se_save_imem_state(struct se_if_priv *priv, struct se_imem_buf *imem);
int se_restore_imem_state(struct se_if_priv *priv, struct se_imem_buf *imem);
+int se_chk_tx_msg_hdr(struct se_if_priv *priv, struct se_msg_hdr *header);
#endif /*__ELE_COMMON_H__ */
diff --git a/drivers/firmware/imx/se_ctrl.c b/drivers/firmware/imx/se_ctrl.c
index a4823f485f88..0ea062954baf 100644
--- a/drivers/firmware/imx/se_ctrl.c
+++ b/drivers/firmware/imx/se_ctrl.c
@@ -4,6 +4,7 @@
*/
#include <linux/bitfield.h>
+#include <linux/cleanup.h>
#include <linux/completion.h>
#include <linux/delay.h>
#include <linux/dev_printk.h>
@@ -16,6 +17,7 @@
#include <linux/genalloc.h>
#include <linux/init.h>
#include <linux/io.h>
+#include <linux/kref.h>
#include <linux/miscdevice.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
@@ -25,22 +27,19 @@
#include <linux/slab.h>
#include <linux/string.h>
#include <linux/sys_soc.h>
+#include <uapi/linux/se_ioctl.h>
#include "ele_base_msg.h"
#include "ele_common.h"
#include "se_ctrl.h"
+/* Maximum response buffer size in bytes for debug-dump replies. */
+#define MAX_ALLOWED_RX_MSG_SZ ELE_DEBUG_DUMP_RSP_SZ
+
#define MAX_SOC_INFO_DATA_SZ 256
#define MBOX_TX_NAME "tx"
#define MBOX_RX_NAME "rx"
-#define SE_TYPE_STR_DBG "dbg"
-#define SE_TYPE_STR_HSM "hsm"
-
-#define SE_TYPE_ID_DBG 0x1
-
-#define SE_TYPE_ID_HSM 0x2
-
struct se_fw_img_name {
const u8 *prim_fw_nm_in_rfs;
const u8 *seco_fw_nm_in_rfs;
@@ -133,6 +132,13 @@ char *get_se_if_name(u8 se_if_id)
return NULL;
}
+static u32 get_se_soc_id(struct se_if_priv *priv)
+{
+ const struct se_soc_info *se_info = device_get_match_data(priv->dev);
+
+ return se_info->soc_id;
+}
+
static struct se_fw_load_info *get_load_fw_instance(struct se_if_priv *priv)
{
return &var_se_info.load_fw;
@@ -199,11 +205,235 @@ static int get_se_soc_info(struct se_if_priv *priv, const struct se_soc_info *se
return 0;
}
+static int load_firmware(struct se_if_priv *priv, const u8 *se_img_file_to_load)
+{
+ const struct firmware *fw = NULL;
+ dma_addr_t se_fw_dma_addr;
+ phys_addr_t se_fw_phyaddr;
+ u8 *se_fw_buf;
+ int ret;
+
+ if (!se_img_file_to_load) {
+ dev_err(priv->dev, "FW image is not provided.");
+ return -EINVAL;
+ }
+ ret = request_firmware(&fw, se_img_file_to_load, priv->dev);
+ if (ret)
+ return ret;
+
+ dev_info(priv->dev, "loading firmware %s.", se_img_file_to_load);
+
+ /* allocate buffer to store the SE FW */
+ se_fw_buf = dma_alloc_coherent(priv->dev, fw->size, &se_fw_dma_addr, GFP_KERNEL);
+ if (!se_fw_buf) {
+ ret = -ENOMEM;
+ goto exit;
+ }
+
+ memcpy(se_fw_buf, fw->data, fw->size);
+ se_fw_phyaddr = dma_to_phys(priv->dev, se_fw_dma_addr);
+ ret = ele_fw_authenticate(priv, se_fw_phyaddr, se_fw_phyaddr);
+ if (ret < 0) {
+ dev_err(priv->dev,
+ "Error %pe: Authenticate & load SE firmware %s.",
+ ERR_PTR(ret), se_img_file_to_load);
+ ret = -EPERM;
+ }
+ dma_free_coherent(priv->dev, fw->size, se_fw_buf, se_fw_dma_addr);
+exit:
+ release_firmware(fw);
+
+ return ret;
+}
+
+static int se_load_firmware(struct se_if_priv *priv)
+{
+ struct se_fw_load_info *load_fw = get_load_fw_instance(priv);
+ int ret = 0;
+
+ if (!load_fw->is_fw_tobe_loaded)
+ return 0;
+
+ if (load_fw->imem.state == ELE_IMEM_STATE_BAD) {
+ ret = load_firmware(priv, load_fw->se_fw_img_nm->prim_fw_nm_in_rfs);
+ if (ret) {
+ dev_err(priv->dev, "Failed to load boot firmware.");
+ return -EPERM;
+ }
+ }
+
+ ret = load_firmware(priv, load_fw->se_fw_img_nm->seco_fw_nm_in_rfs);
+ if (ret) {
+ dev_err(priv->dev, "Failed to load runtime firmware.");
+ return -EPERM;
+ }
+
+ load_fw->is_fw_tobe_loaded = false;
+
+ return ret;
+}
+
+static int init_se_shared_mem(struct se_if_device_ctx *dev_ctx)
+{
+ struct se_shared_mem_mgmt_info *se_shared_mem_mgmt = &dev_ctx->se_shared_mem_mgmt;
+ struct se_if_priv *priv = dev_ctx->priv;
+
+ INIT_LIST_HEAD(&se_shared_mem_mgmt->pending_out);
+ INIT_LIST_HEAD(&se_shared_mem_mgmt->pending_in);
+
+ /*
+ * Allocate some memory for data exchanges with S40x.
+ * This will be used for data not requiring secure memory.
+ */
+ se_shared_mem_mgmt->non_secure_mem.ptr =
+ dma_alloc_coherent(priv->dev, MAX_DATA_SIZE_PER_USER,
+ &se_shared_mem_mgmt->non_secure_mem.dma_addr,
+ GFP_KERNEL);
+ if (!se_shared_mem_mgmt->non_secure_mem.ptr)
+ return -ENOMEM;
+
+ se_shared_mem_mgmt->non_secure_mem.size = MAX_DATA_SIZE_PER_USER;
+ se_shared_mem_mgmt->non_secure_mem.pos = 0;
+
+ return 0;
+}
+
+static void cleanup_se_shared_mem(struct se_if_device_ctx *dev_ctx)
+{
+ struct se_shared_mem_mgmt_info *se_shared_mem_mgmt = &dev_ctx->se_shared_mem_mgmt;
+ struct se_if_priv *priv = dev_ctx->priv;
+
+ /* Free non-secure shared buffer. */
+ dma_free_coherent(priv->dev, MAX_DATA_SIZE_PER_USER,
+ se_shared_mem_mgmt->non_secure_mem.ptr,
+ se_shared_mem_mgmt->non_secure_mem.dma_addr);
+
+ se_shared_mem_mgmt->non_secure_mem.ptr = NULL;
+ se_shared_mem_mgmt->non_secure_mem.dma_addr = 0;
+ se_shared_mem_mgmt->non_secure_mem.size = 0;
+ se_shared_mem_mgmt->non_secure_mem.pos = 0;
+}
+
+/* Need to copy the output data to user-device context.
+ */
+static int se_dev_ctx_cpy_out_data(struct se_if_device_ctx *dev_ctx)
+{
+ struct se_shared_mem_mgmt_info *se_shared_mem_mgmt = &dev_ctx->se_shared_mem_mgmt;
+ struct se_if_priv *priv = dev_ctx->priv;
+ struct se_buf_desc *b_desc, *temp;
+ bool do_cpy = true;
+
+ list_for_each_entry_safe(b_desc, temp, &se_shared_mem_mgmt->pending_out, link) {
+ if (b_desc->usr_buf_ptr && b_desc->shared_buf_ptr && do_cpy) {
+ dev_dbg(priv->dev, "Copying output data to user.");
+ if (do_cpy && copy_to_user(b_desc->usr_buf_ptr,
+ b_desc->shared_buf_ptr,
+ b_desc->size)) {
+ dev_err(priv->dev, "Failure copying output data to user.");
+ do_cpy = false;
+ }
+ }
+
+ if (b_desc->shared_buf_ptr)
+ memset(b_desc->shared_buf_ptr, 0, b_desc->size);
+
+ list_del(&b_desc->link);
+ kfree(b_desc);
+ }
+
+ return do_cpy ? 0 : -EFAULT;
+}
+
+/*
+ * Clean the used Shared Memory space,
+ * whether its Input Data copied from user buffers, or
+ * Data received from FW.
+ */
+static void se_dev_ctx_shared_mem_cleanup(struct se_if_device_ctx *dev_ctx)
+{
+ struct se_shared_mem_mgmt_info *se_shared_mem_mgmt = &dev_ctx->se_shared_mem_mgmt;
+ struct list_head *pending_lists[] = {&se_shared_mem_mgmt->pending_in,
+ &se_shared_mem_mgmt->pending_out};
+ struct se_buf_desc *b_desc, *temp;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(pending_lists); i++) {
+ list_for_each_entry_safe(b_desc, temp, pending_lists[i], link) {
+ if (b_desc->shared_buf_ptr)
+ memset(b_desc->shared_buf_ptr, 0, b_desc->size);
+
+ list_del(&b_desc->link);
+ kfree(b_desc);
+ }
+ }
+ se_shared_mem_mgmt->non_secure_mem.pos = 0;
+}
+
+static struct se_buf_desc *add_b_desc_to_pending_list(void *shared_ptr_with_pos,
+ struct se_ioctl_setup_iobuf *io,
+ struct se_if_device_ctx *dev_ctx)
+{
+ struct se_shared_mem_mgmt_info *se_shared_mem_mgmt = &dev_ctx->se_shared_mem_mgmt;
+ struct se_buf_desc *b_desc = NULL;
+
+ b_desc = kzalloc_obj(*b_desc, GFP_KERNEL);
+ if (!b_desc)
+ return ERR_PTR(-ENOMEM);
+
+ b_desc->shared_buf_ptr = shared_ptr_with_pos;
+ b_desc->usr_buf_ptr = io->user_buf;
+ b_desc->size = io->length;
+
+ if (io->flags & SE_IO_BUF_FLAGS_IS_INPUT) {
+ /*
+ * buffer is input:
+ * add an entry in the "pending input buffers" list so
+ * that copied data can be cleaned from shared memory
+ * later.
+ */
+ list_add_tail(&b_desc->link, &se_shared_mem_mgmt->pending_in);
+ } else {
+ /*
+ * buffer is output:
+ * add an entry in the "pending out buffers" list so data
+ * can be copied to user space when receiving Secure-Enclave
+ * response.
+ */
+ list_add_tail(&b_desc->link, &se_shared_mem_mgmt->pending_out);
+ }
+
+ return b_desc;
+}
+
+static void se_if_open_gate_release(struct kref *kref)
+{
+ struct se_if_open_gate *gate =
+ container_of(kref, struct se_if_open_gate, refcount);
+
+ kfree(gate);
+}
+
+static bool se_if_open_gate_get(struct se_if_open_gate *gate)
+{
+ if (!gate)
+ return false;
+
+ return kref_get_unless_zero(&gate->refcount);
+}
+
+static void se_if_open_gate_put(struct se_if_open_gate *gate)
+{
+ if (gate)
+ kref_put(&gate->refcount, se_if_open_gate_release);
+}
+
static int init_misc_device_context(struct se_if_priv *priv, int ch_id,
- struct se_if_device_ctx **new_dev_ctx)
+ struct se_if_device_ctx **new_dev_ctx,
+ const struct file_operations *se_if_fops)
{
const char *err_str = "Failed to allocate memory";
struct se_if_device_ctx *dev_ctx;
+ struct se_if_open_gate *gate = NULL;
int ret = -ENOMEM;
dev_ctx = kzalloc_obj(*dev_ctx, GFP_KERNEL);
@@ -217,18 +447,723 @@ static int init_misc_device_context(struct se_if_priv *priv, int ch_id,
if (!dev_ctx->devname)
goto exit;
+ mutex_init(&dev_ctx->fops_lock);
+
dev_ctx->priv = priv;
+ kref_init(&dev_ctx->refcount);
+ dev_ctx->cleanup_done = false;
*new_dev_ctx = dev_ctx;
+ gate = kzalloc_obj(*gate, GFP_KERNEL);
+ if (!gate)
+ goto exit;
+
+ mutex_init(&gate->lock);
+ kref_init(&gate->refcount); /* device-owned reference */
+ gate->priv = priv;
+ gate->dying = false;
+ priv->open_gate = gate;
+
+ /*
+ * The miscdevice storage is now owned by the open gate object.
+ * priv->priv_dev_ctx still keeps a pointer to that miscdevice.
+ */
+ dev_ctx->miscdev = &gate->miscdev;
+
+ dev_ctx->miscdev->name = dev_ctx->devname;
+ dev_ctx->miscdev->minor = MISC_DYNAMIC_MINOR;
+ dev_ctx->miscdev->fops = se_if_fops;
+ dev_ctx->miscdev->parent = priv->dev;
+ ret = misc_register(dev_ctx->miscdev);
+ if (ret) {
+ err_str = "Failed to register misc device.";
+ goto exit;
+ }
+
return ret;
exit:
*new_dev_ctx = NULL;
-
+ if (gate) {
+ priv->open_gate = NULL;
+ se_if_open_gate_put(gate);
+ }
kfree(dev_ctx->devname);
kfree(dev_ctx);
return dev_err_probe(priv->dev, ret, "%s", err_str);
}
+static void se_if_priv_release(struct kref *kref)
+{
+ struct se_if_priv *priv = container_of(kref, struct se_if_priv, refcount);
+
+ /* Free priv_dev_ctx if it exists */
+ if (priv->priv_dev_ctx) {
+ /*
+ * miscdev storage belongs to open_gate, not directly to
+ * priv_dev_ctx. The gate should already have been detached
+ * from priv during teardown.
+ */
+
+ kfree(priv->priv_dev_ctx->devname);
+ kfree(priv->priv_dev_ctx);
+ priv->priv_dev_ctx = NULL;
+ }
+
+ /*
+ * Be defensive: if teardown did not already drop the device-owned
+ * gate reference for some reason, release it here.
+ */
+ if (priv->open_gate) {
+ se_if_open_gate_put(priv->open_gate);
+ priv->open_gate = NULL;
+ }
+
+ /* Free any remaining resources that weren't devm-managed */
+ kfree(priv);
+}
+
+static void se_if_dev_ctx_release(struct kref *kref)
+{
+ struct se_if_device_ctx *dev_ctx =
+ container_of(kref, struct se_if_device_ctx, refcount);
+ struct se_if_priv *priv = dev_ctx->priv;
+
+ kfree(dev_ctx);
+
+ /* drop the priv reference owned by this device context */
+ kref_put(&priv->refcount, se_if_priv_release);
+}
+
+static void dlink_dev_ctx(struct se_if_device_ctx *dev_ctx)
+{
+ struct se_if_priv *priv = dev_ctx->priv;
+
+ /* check if this device was registered as command receiver */
+ if (priv->cmd_receiver_clbk_hdl.dev_ctx == dev_ctx) {
+ kfree(priv->cmd_receiver_clbk_hdl.rx_msg);
+ priv->cmd_receiver_clbk_hdl.rx_msg = NULL;
+ priv->cmd_receiver_clbk_hdl.dev_ctx = NULL;
+ }
+
+ if (!list_empty(&dev_ctx->link)) {
+ list_del_init(&dev_ctx->link);
+ priv->active_devctx_count--;
+ }
+}
+
+static void cleanup_dev_ctx(struct se_if_device_ctx *dev_ctx, bool is_fclose)
+{
+ scoped_guard(mutex, &dev_ctx->fops_lock) {
+ if (dev_ctx->cleanup_done)
+ goto exit;
+
+ se_dev_ctx_shared_mem_cleanup(dev_ctx);
+ cleanup_se_shared_mem(dev_ctx);
+
+ kfree(dev_ctx->devname);
+ dev_ctx->devname = NULL;
+ dev_ctx->cleanup_done = true;
+ }
+exit:
+ if (is_fclose)
+ kref_put(&dev_ctx->refcount, se_if_dev_ctx_release);
+}
+
+static void dlink_n_cleanup_dev_ctx(struct se_if_device_ctx *dev_ctx, bool is_fclose)
+{
+ struct se_if_priv *priv = dev_ctx->priv;
+
+ if (is_fclose) {
+ scoped_guard(mutex, &priv->modify_lock)
+ dlink_dev_ctx(dev_ctx);
+ }
+
+ cleanup_dev_ctx(dev_ctx, is_fclose);
+}
+
+static int init_device_context(struct se_if_priv *priv, int ch_id,
+ struct se_if_device_ctx **new_dev_ctx)
+{
+ struct se_if_device_ctx *dev_ctx;
+ int ret = 0;
+
+ dev_ctx = kzalloc_obj(*dev_ctx, GFP_KERNEL);
+
+ if (!dev_ctx)
+ return -ENOMEM;
+
+ dev_ctx->devname = kasprintf(GFP_KERNEL, "%s0_ch%d",
+ get_se_if_name(priv->if_defs->se_if_type),
+ ch_id);
+ if (!dev_ctx->devname) {
+ kfree(dev_ctx);
+ return -ENOMEM;
+ }
+
+ mutex_init(&dev_ctx->fops_lock);
+ dev_ctx->priv = priv;
+ dev_ctx->cleanup_done = false;
+ INIT_LIST_HEAD(&dev_ctx->link);
+ *new_dev_ctx = dev_ctx;
+
+ ret = init_se_shared_mem(dev_ctx);
+ if (ret < 0) {
+ kfree(dev_ctx->devname);
+ kfree(dev_ctx);
+ *new_dev_ctx = NULL;
+
+ return ret;
+ }
+
+ /* Take a reference to priv for this device context */
+ kref_get(&priv->refcount);
+
+ scoped_guard(mutex, &priv->modify_lock) {
+ list_add_tail(&dev_ctx->link, &priv->dev_ctx_list);
+ priv->active_devctx_count++;
+ }
+
+ return ret;
+}
+
+static int se_ioctl_cmd_snd_rcv_cleanup(struct se_if_device_ctx *dev_ctx, void __user *uarg,
+ struct se_ioctl_cmd_snd_rcv_rsp_info *cmd_snd_rcv_rsp_info)
+{
+ /* shared memory is allocated before this IOCTL */
+ se_dev_ctx_shared_mem_cleanup(dev_ctx);
+
+ if (copy_to_user(uarg, cmd_snd_rcv_rsp_info, sizeof(*cmd_snd_rcv_rsp_info))) {
+ dev_err(dev_ctx->priv->dev, "%s: Failed to copy cmd_snd_rcv_rsp_info from user.",
+ dev_ctx->devname);
+ return -EFAULT;
+ }
+
+ return 0;
+}
+
+static int se_ioctl_cmd_snd_rcv_rsp_handler(struct se_if_device_ctx *dev_ctx,
+ void __user *uarg)
+{
+ struct se_ioctl_cmd_snd_rcv_rsp_info cmd_snd_rcv_rsp_info = {0};
+ struct se_if_priv *priv = dev_ctx->priv;
+ int err = 0;
+ int cleanup_err = 0;
+
+ if (copy_from_user(&cmd_snd_rcv_rsp_info, uarg,
+ sizeof(cmd_snd_rcv_rsp_info))) {
+ dev_err(priv->dev,
+ "%s: Failed to copy cmd_snd_rcv_rsp_info from user.",
+ dev_ctx->devname);
+ se_ioctl_cmd_snd_rcv_cleanup(dev_ctx, uarg, &cmd_snd_rcv_rsp_info);
+ return -EFAULT;
+ }
+
+ if (cmd_snd_rcv_rsp_info.tx_buf_sz < SE_MU_HDR_SZ) {
+ dev_err(priv->dev, "%s: User buffer too small(%d < %d)",
+ dev_ctx->devname, cmd_snd_rcv_rsp_info.tx_buf_sz, SE_MU_HDR_SZ);
+ se_ioctl_cmd_snd_rcv_cleanup(dev_ctx, uarg, &cmd_snd_rcv_rsp_info);
+ return -ENOSPC;
+ }
+
+ struct se_api_msg *tx_msg __free(kfree) =
+ memdup_user(cmd_snd_rcv_rsp_info.tx_buf,
+ cmd_snd_rcv_rsp_info.tx_buf_sz);
+ if (IS_ERR(tx_msg)) {
+ err = PTR_ERR(tx_msg);
+ se_ioctl_cmd_snd_rcv_cleanup(dev_ctx, uarg, &cmd_snd_rcv_rsp_info);
+ return err;
+ }
+
+ err = se_chk_tx_msg_hdr(priv, &tx_msg->header);
+ if (err) {
+ se_ioctl_cmd_snd_rcv_cleanup(dev_ctx, uarg, &cmd_snd_rcv_rsp_info);
+ return err;
+ }
+
+ if (cmd_snd_rcv_rsp_info.rx_buf_sz < sizeof(struct se_msg_hdr) ||
+ cmd_snd_rcv_rsp_info.rx_buf_sz > MAX_ALLOWED_RX_MSG_SZ) {
+ se_ioctl_cmd_snd_rcv_cleanup(dev_ctx, uarg, &cmd_snd_rcv_rsp_info);
+ return -EINVAL;
+ }
+
+ if (tx_msg->header.tag != priv->if_defs->cmd_tag) {
+ se_ioctl_cmd_snd_rcv_cleanup(dev_ctx, uarg, &cmd_snd_rcv_rsp_info);
+ return -EINVAL;
+ }
+
+ if (tx_msg->header.ver == priv->if_defs->fw_api_ver &&
+ get_load_fw_instance(priv)->is_fw_tobe_loaded) {
+ err = se_load_firmware(priv);
+ if (err) {
+ dev_err(priv->dev, "Could not send msg as FW is not loaded.");
+ se_ioctl_cmd_snd_rcv_cleanup(dev_ctx, uarg, &cmd_snd_rcv_rsp_info);
+ return -EPERM;
+ }
+ }
+ set_se_rcv_msg_timeout(priv, SE_RCV_MSG_LONG_TIMEOUT);
+
+ struct se_api_msg *rx_msg __free(kfree) =
+ kzalloc(cmd_snd_rcv_rsp_info.rx_buf_sz, GFP_KERNEL);
+ if (!rx_msg) {
+ se_ioctl_cmd_snd_rcv_cleanup(dev_ctx, uarg, &cmd_snd_rcv_rsp_info);
+ return -ENOMEM;
+ }
+
+ err = ele_msg_send_rcv(dev_ctx, tx_msg, cmd_snd_rcv_rsp_info.tx_buf_sz,
+ rx_msg, cmd_snd_rcv_rsp_info.rx_buf_sz);
+ if (err < 0) {
+ se_ioctl_cmd_snd_rcv_cleanup(dev_ctx, uarg, &cmd_snd_rcv_rsp_info);
+ return err;
+ }
+
+ dev_dbg(priv->dev, "%s: %s %s.", dev_ctx->devname, __func__,
+ "message received, start transmit to user");
+
+ /* We may need to copy the output data to user before
+ * delivering the completion message.
+ */
+ err = se_dev_ctx_cpy_out_data(dev_ctx);
+ if (err < 0) {
+ se_ioctl_cmd_snd_rcv_cleanup(dev_ctx, uarg, &cmd_snd_rcv_rsp_info);
+ return err;
+ }
+
+ /* Copy data from the buffer */
+ print_hex_dump_debug("to user ", DUMP_PREFIX_OFFSET, 4, 4, rx_msg,
+ cmd_snd_rcv_rsp_info.rx_buf_sz, false);
+
+ if (copy_to_user(cmd_snd_rcv_rsp_info.rx_buf, rx_msg,
+ cmd_snd_rcv_rsp_info.rx_buf_sz)) {
+ dev_err(priv->dev, "%s: Failed to copy to user.", dev_ctx->devname);
+ err = -EFAULT;
+ }
+
+ cleanup_err = se_ioctl_cmd_snd_rcv_cleanup(dev_ctx, uarg, &cmd_snd_rcv_rsp_info);
+
+ if (!err)
+ err = cleanup_err;
+
+ return err;
+}
+
+static int se_ioctl_get_mu_info(struct se_if_device_ctx *dev_ctx,
+ void __user *uarg)
+{
+ struct se_if_priv *priv = dev_ctx->priv;
+ struct se_ioctl_get_if_info if_info;
+ struct se_if_node *if_node;
+ int err = 0;
+
+ if_node = container_of(priv->if_defs, typeof(*if_node), if_defs);
+
+ if_info.se_if_id = 0;
+ if_info.interrupt_idx = 0;
+ if_info.tz = 0;
+ if_info.did = 0;
+ if_info.cmd_tag = priv->if_defs->cmd_tag;
+ if_info.rsp_tag = priv->if_defs->rsp_tag;
+ if_info.success_tag = priv->if_defs->success_tag;
+ if_info.base_api_ver = priv->if_defs->base_api_ver;
+ if_info.fw_api_ver = priv->if_defs->fw_api_ver;
+
+ dev_dbg(priv->dev, "%s: info [se_if_id: %d, irq_idx: %d, tz: 0x%x, did: 0x%x].",
+ dev_ctx->devname, if_info.se_if_id, if_info.interrupt_idx, if_info.tz,
+ if_info.did);
+
+ if (copy_to_user(uarg, &if_info, sizeof(if_info))) {
+ dev_err(priv->dev, "%s: Failed to copy mu info to user.",
+ dev_ctx->devname);
+ err = -EFAULT;
+ }
+
+ return err;
+}
+
+/*
+ * Copy a buffer of data to/from the user and return the address to use in
+ * messages
+ */
+static int se_ioctl_setup_iobuf_handler(struct se_if_device_ctx *dev_ctx,
+ void __user *uarg)
+{
+ struct se_shared_mem *shared_mem = NULL;
+ struct se_ioctl_setup_iobuf io = {0};
+ struct se_buf_desc *b_desc = NULL;
+ size_t aligned_len = 0;
+ int err = 0;
+ u32 pos;
+
+ if (copy_from_user(&io, uarg, sizeof(io))) {
+ dev_err(dev_ctx->priv->dev, "%s: Failed copy iobuf config from user.",
+ dev_ctx->devname);
+ return -EFAULT;
+ }
+
+ dev_dbg(dev_ctx->priv->dev, "%s: io [buf: %p(%d) flag: %x].", dev_ctx->devname,
+ io.user_buf, io.length, io.flags);
+
+ if (io.length == 0 || !io.user_buf) {
+ /*
+ * Accept NULL pointers since some buffers are optional
+ * in FW commands. In this case we should return 0 as
+ * pointer to be embedded into the message.
+ * Skip all data copy part of code below.
+ */
+ io.ele_addr = 0;
+ goto copy;
+ }
+
+ aligned_len = round_up((size_t)io.length, 8);
+ if (aligned_len < io.length) {
+ dev_err(dev_ctx->priv->dev, "%s: Invalid buffer length.",
+ dev_ctx->devname);
+ return -EINVAL;
+ }
+
+ /* No specific requirement for this buffer. */
+ shared_mem = &dev_ctx->se_shared_mem_mgmt.non_secure_mem;
+
+ /* Check there is enough space in the shared memory. */
+ dev_dbg(dev_ctx->priv->dev, "%s: req_size = %zd, max_size= %d, curr_pos = %d",
+ dev_ctx->devname, aligned_len, shared_mem->size,
+ shared_mem->pos);
+
+ if (shared_mem->size < shared_mem->pos ||
+ aligned_len > (shared_mem->size - shared_mem->pos)) {
+ dev_err(dev_ctx->priv->dev, "%s: Not enough space in shared memory.",
+ dev_ctx->devname);
+ return -ENOMEM;
+ }
+
+ /* Allocate space in shared memory. 8 bytes aligned. */
+ pos = shared_mem->pos;
+ shared_mem->pos += aligned_len;
+ io.ele_addr = (u64)shared_mem->dma_addr + pos;
+
+ memset(shared_mem->ptr + pos, 0, io.length);
+ if ((io.flags & SE_IO_BUF_FLAGS_IS_INPUT) ||
+ (io.flags & SE_IO_BUF_FLAGS_IS_IN_OUT)) {
+ /*
+ * buffer is input:
+ * copy data from user space to this allocated buffer.
+ */
+ if (copy_from_user(shared_mem->ptr + pos, io.user_buf, io.length)) {
+ dev_err(dev_ctx->priv->dev,
+ "%s: Failed copy data to shared memory.",
+ dev_ctx->devname);
+ err = -EFAULT;
+ goto rollback;
+ }
+ }
+
+ b_desc = add_b_desc_to_pending_list(shared_mem->ptr + pos, &io, dev_ctx);
+ if (IS_ERR(b_desc)) {
+ err = PTR_ERR(b_desc);
+ dev_err(dev_ctx->priv->dev, "%s: Failed to allocate/link b_desc.",
+ dev_ctx->devname);
+ goto rollback;
+ }
+
+copy:
+ /* Provide the EdgeLock Enclave address to user space only if success.*/
+ if (copy_to_user(uarg, &io, sizeof(io))) {
+ dev_err(dev_ctx->priv->dev, "%s: Failed to copy iobuff setup to user.",
+ dev_ctx->devname);
+ err = -EFAULT;
+ if (b_desc) {
+ list_del(&b_desc->link);
+ kfree(b_desc);
+ }
+ goto rollback;
+ }
+ return err;
+
+rollback:
+ if (aligned_len) {
+ memset(shared_mem->ptr + pos, 0, aligned_len);
+ shared_mem->pos = pos;
+ }
+
+ return err;
+}
+
+/* IOCTL to provide SoC information */
+static int se_ioctl_get_se_soc_info_handler(struct se_if_device_ctx *dev_ctx,
+ void __user *uarg)
+{
+ struct se_ioctl_get_soc_info soc_info;
+ int err = -EINVAL;
+
+ soc_info.soc_id = get_se_soc_id(dev_ctx->priv);
+ soc_info.soc_rev = var_se_info.soc_rev;
+
+ err = copy_to_user(uarg, (u8 *)(&soc_info), sizeof(soc_info));
+ if (err) {
+ dev_err(dev_ctx->priv->dev, "%s: Failed to copy soc info to user.",
+ dev_ctx->devname);
+ err = -EFAULT;
+ }
+
+ return err;
+}
+
+/*
+ * File operations for user-space
+ */
+
+/* Write a message to the MU. */
+static ssize_t se_if_fops_write(struct file *fp, const char __user *buf,
+ size_t size, loff_t *ppos)
+{
+ struct se_if_device_ctx *dev_ctx = fp->private_data;
+ struct se_if_priv *priv;
+ int err;
+
+ scoped_cond_guard(mutex_intr, return -EBUSY, &dev_ctx->fops_lock) {
+ if (dev_ctx->cleanup_done)
+ return -ENODEV;
+
+ priv = dev_ctx->priv;
+
+ dev_dbg(priv->dev, "%s: write from buf (%p)%zu, ppos=%lld.", dev_ctx->devname,
+ buf, size, ((ppos) ? *ppos : 0));
+
+ if (dev_ctx != priv->cmd_receiver_clbk_hdl.dev_ctx)
+ return -EINVAL;
+
+ if (size < SE_MU_HDR_SZ) {
+ dev_err(priv->dev, "%s: User buffer too small(%zu < %d).",
+ dev_ctx->devname, size, SE_MU_HDR_SZ);
+ return -ENOSPC;
+ }
+
+ struct se_api_msg *tx_msg __free(kfree) = memdup_user(buf, size);
+ if (IS_ERR(tx_msg))
+ return PTR_ERR(tx_msg);
+
+ err = se_chk_tx_msg_hdr(priv, &tx_msg->header);
+ if (err)
+ return err;
+
+ print_hex_dump_debug("from user ", DUMP_PREFIX_OFFSET, 4, 4,
+ tx_msg, size, false);
+
+ err = ele_msg_send(dev_ctx, tx_msg, size);
+
+ return err;
+ }
+}
+
+/*
+ * Read a message from the MU.
+ * Blocking until a message is available.
+ */
+static ssize_t se_if_fops_read(struct file *fp, char __user *buf, size_t size,
+ loff_t *ppos)
+{
+ struct se_if_device_ctx *dev_ctx = fp->private_data;
+ struct se_if_priv *priv;
+ size_t copy_len;
+ int err;
+
+ scoped_cond_guard(mutex_intr, return -EBUSY, &dev_ctx->fops_lock) {
+ if (dev_ctx->cleanup_done)
+ return -ENODEV;
+
+ priv = dev_ctx->priv;
+
+ dev_dbg(priv->dev, "%s: read to buf %p(%zu), ppos=%lld.", dev_ctx->devname,
+ buf, size, ((ppos) ? *ppos : 0));
+
+ if (dev_ctx != priv->cmd_receiver_clbk_hdl.dev_ctx) {
+ err = -EINVAL;
+ goto exit;
+ }
+
+ err = ele_msg_rcv(dev_ctx, &priv->cmd_receiver_clbk_hdl);
+ if (err < 0) {
+ dev_err(priv->dev,
+ "%s: Er[0x%x]: Signal Interrupted. Current act-dev-ctx count: %d.",
+ dev_ctx->devname, err, dev_ctx->priv->active_devctx_count);
+ goto exit;
+ }
+
+ /* We may need to copy the output data to user before
+ * delivering the completion message.
+ */
+ err = se_dev_ctx_cpy_out_data(dev_ctx);
+ if (err < 0)
+ goto exit;
+
+ /* Copy data from the buffer */
+ print_hex_dump_debug("to user ", DUMP_PREFIX_OFFSET, 4, 4,
+ priv->cmd_receiver_clbk_hdl.rx_msg,
+ priv->cmd_receiver_clbk_hdl.rx_msg_sz,
+ false);
+
+ copy_len = min(size, priv->cmd_receiver_clbk_hdl.rx_msg_sz);
+
+ if (copy_to_user(buf, priv->cmd_receiver_clbk_hdl.rx_msg, copy_len))
+ err = -EFAULT;
+ else
+ err = copy_len;
+
+exit:
+ priv->cmd_receiver_clbk_hdl.rx_msg_sz = 0;
+
+ se_dev_ctx_shared_mem_cleanup(dev_ctx);
+
+ return err;
+ }
+}
+
+/* Open a character device. */
+static int se_if_fops_open(struct inode *nd, struct file *fp)
+{
+ struct miscdevice *miscdev = fp->private_data;
+ struct se_if_open_gate *gate;
+ struct se_if_device_ctx *misc_dev_ctx;
+ struct se_if_device_ctx *dev_ctx;
+ struct se_if_priv *priv;
+ int err = 0;
+
+ gate = container_of(miscdev, struct se_if_open_gate, miscdev);
+
+ if (!se_if_open_gate_get(gate))
+ return -ENODEV;
+
+ if (mutex_lock_interruptible(&gate->lock)) {
+ se_if_open_gate_put(gate);
+ return -EBUSY;
+ }
+
+ if (gate->dying || !gate->priv ||
+ !kref_get_unless_zero(&gate->priv->refcount)) {
+ err = -ENODEV;
+ goto out_unlock_gate;
+ }
+
+ priv = gate->priv;
+ mutex_unlock(&gate->lock);
+
+ misc_dev_ctx = priv->priv_dev_ctx;
+
+ if (mutex_lock_interruptible(&misc_dev_ctx->fops_lock)) {
+ err = -EBUSY;
+ goto out_put_priv;
+ }
+
+ if (misc_dev_ctx->cleanup_done) {
+ err = -ENODEV;
+ goto out_unlock_misc;
+ }
+
+ priv->dev_ctx_mono_count++;
+ err = init_device_context(priv, priv->dev_ctx_mono_count, &dev_ctx);
+ if (err) {
+ dev_err(priv->dev, "Failed[0x%x] to create dev-ctx.", err);
+ goto out_unlock_misc;
+ }
+
+ fp->private_data = dev_ctx;
+
+out_unlock_misc:
+ mutex_unlock(&misc_dev_ctx->fops_lock);
+out_put_priv:
+ kref_put(&priv->refcount, se_if_priv_release);
+ se_if_open_gate_put(gate);
+ return err;
+out_unlock_gate:
+ mutex_unlock(&gate->lock);
+ se_if_open_gate_put(gate);
+ return err;
+}
+
+/* Close a character device. */
+static int se_if_fops_close(struct inode *nd, struct file *fp)
+{
+ struct se_if_device_ctx *dev_ctx = fp->private_data;
+
+ dlink_n_cleanup_dev_ctx(dev_ctx, true);
+
+ return 0;
+}
+
+/* IOCTL entry point of a character device */
+static long se_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
+{
+ struct se_if_device_ctx *dev_ctx = fp->private_data;
+ struct se_if_priv *priv;
+ void __user *uarg = (void __user *)arg;
+ long err;
+
+ /* Prevent race during change of device context */
+ scoped_cond_guard(mutex_intr, return -EBUSY, &dev_ctx->fops_lock) {
+ if (dev_ctx->cleanup_done)
+ return -ENODEV;
+
+ priv = dev_ctx->priv;
+
+ switch (cmd) {
+ case SE_IOCTL_ENABLE_CMD_RCV: {
+ struct se_clbk_handle *se_clbk_hdl = &priv->cmd_receiver_clbk_hdl;
+ struct se_api_msg *new_rx_msg = NULL;
+ unsigned long flags;
+
+ guard(mutex)(&priv->modify_lock);
+ if (se_clbk_hdl->dev_ctx) {
+ err = -EBUSY;
+ break;
+ }
+ if (!se_clbk_hdl->rx_msg) {
+ new_rx_msg = kzalloc(MAX_NVM_MSG_LEN, GFP_KERNEL);
+ if (!new_rx_msg) {
+ err = -ENOMEM;
+ break;
+ }
+ }
+ spin_lock_irqsave(&se_clbk_hdl->clbk_rx_lock, flags);
+ if (new_rx_msg)
+ se_clbk_hdl->rx_msg = new_rx_msg;
+ reinit_completion(&se_clbk_hdl->done);
+ se_clbk_hdl->rx_msg_sz = MAX_NVM_MSG_LEN;
+ se_clbk_hdl->dev_ctx = dev_ctx;
+ spin_unlock_irqrestore(&se_clbk_hdl->clbk_rx_lock, flags);
+ err = 0;
+ break;
+ }
+ case SE_IOCTL_GET_MU_INFO:
+ err = se_ioctl_get_mu_info(dev_ctx, uarg);
+ break;
+ case SE_IOCTL_SETUP_IOBUF:
+ err = se_ioctl_setup_iobuf_handler(dev_ctx, uarg);
+ break;
+ case SE_IOCTL_GET_SOC_INFO:
+ err = se_ioctl_get_se_soc_info_handler(dev_ctx, uarg);
+ break;
+ case SE_IOCTL_CMD_SEND_RCV_RSP:
+ err = se_ioctl_cmd_snd_rcv_rsp_handler(dev_ctx, uarg);
+ break;
+ default:
+ err = -EINVAL;
+ dev_dbg(priv->dev, "%s: IOCTL %.8x not supported.",
+ dev_ctx->devname, cmd);
+ }
+ }
+
+ return err;
+}
+
+/* Char driver setup */
+static const struct file_operations se_if_fops = {
+ .open = se_if_fops_open,
+ .owner = THIS_MODULE,
+ .release = se_if_fops_close,
+ .unlocked_ioctl = se_ioctl,
+ .read = se_if_fops_read,
+ .write = se_if_fops_write,
+};
+
/* interface for managed res to free a mailbox channel */
static void if_mbox_free_channel(void *mbox_chan)
{
@@ -258,6 +1193,7 @@ static int se_if_request_channel(struct device *dev, struct mbox_chan **chan,
static void se_if_probe_cleanup(void *plat_dev)
{
+ struct se_if_device_ctx *dev_ctx;
struct platform_device *pdev = plat_dev;
struct se_fw_load_info *load_fw;
struct device *dev = &pdev->dev;
@@ -269,6 +1205,49 @@ static void se_if_probe_cleanup(void *plat_dev)
load_fw = get_load_fw_instance(priv);
+ /*
+ * Mark the private device context as cleanup_done first.
+ * This prevents new device contexts from being created in open().
+ */
+ if (priv->priv_dev_ctx) {
+ scoped_guard(mutex, &priv->modify_lock)
+ priv->priv_dev_ctx->cleanup_done = true;
+
+ if (priv->open_gate) {
+ scoped_guard(mutex, &priv->open_gate->lock) {
+ priv->open_gate->dying = true;
+ priv->open_gate->priv = NULL;
+ }
+ }
+
+ if (priv->priv_dev_ctx->miscdev)
+ misc_deregister(priv->priv_dev_ctx->miscdev);
+ }
+
+ while (true) {
+ dev_ctx = NULL;
+
+ scoped_guard(mutex, &priv->modify_lock) {
+ if (list_empty(&priv->dev_ctx_list))
+ goto out_done;
+
+ dev_ctx = list_first_entry(&priv->dev_ctx_list,
+ struct se_if_device_ctx, link);
+
+ /* pin this context so close() cannot free it under us */
+ kref_get(&dev_ctx->refcount);
+ dlink_dev_ctx(dev_ctx);
+ }
+
+ /*
+ * Local cleanup outside the global lock avoids ABBA deadlock
+ * with paths that already take dev_ctx->fops_lock first.
+ */
+ cleanup_dev_ctx(dev_ctx, false);
+ kref_put(&dev_ctx->refcount, se_if_dev_ctx_release);
+ }
+out_done:
+
/*
* In se_if_request_channel(), passed the clean-up functional
* pointer reference as action to devm_add_action_or_reset().
@@ -291,14 +1270,11 @@ static void se_if_probe_cleanup(void *plat_dev)
* un-set bit.
*/
of_reserved_mem_device_release(dev);
+
dev_set_drvdata(dev, NULL);
- if (priv->priv_dev_ctx) {
- kfree(priv->priv_dev_ctx->devname);
- kfree(priv->priv_dev_ctx);
- priv->priv_dev_ctx = NULL;
- }
- kfree(priv);
+ /* Drop the initial reference - priv will be freed when last fd closes */
+ kref_put(&priv->refcount, se_if_priv_release);
}
static int se_if_probe(struct platform_device *pdev)
@@ -322,15 +1298,18 @@ static int se_if_probe(struct platform_device *pdev)
return -ENOMEM;
priv->dev = dev;
+ kref_init(&priv->refcount);
priv->if_defs = &if_node->if_defs;
dev_set_drvdata(dev, priv);
mutex_init(&priv->se_if_cmd_lock);
+ mutex_init(&priv->modify_lock);
spin_lock_init(&priv->cmd_receiver_clbk_hdl.clbk_rx_lock);
spin_lock_init(&priv->waiting_rsp_clbk_hdl.clbk_rx_lock);
atomic_set(&priv->fw_busy, 0);
init_completion(&priv->waiting_rsp_clbk_hdl.done);
init_completion(&priv->cmd_receiver_clbk_hdl.done);
+ INIT_LIST_HEAD(&priv->dev_ctx_list);
ret = devm_add_action_or_reset(dev, se_if_probe_cleanup, pdev);
if (ret)
@@ -341,6 +1320,7 @@ static int se_if_probe(struct platform_device *pdev)
priv->se_mb_cl.tx_block = false;
priv->se_mb_cl.knows_txdone = true;
priv->se_mb_cl.rx_callback = se_if_rx_callback;
+ set_se_rcv_msg_timeout(priv, SE_RCV_MSG_DEFAULT_TIMEOUT);
ret = se_if_request_channel(dev, &priv->tx_chan, &priv->se_mb_cl, MBOX_TX_NAME);
if (ret)
@@ -365,7 +1345,7 @@ static int se_if_probe(struct platform_device *pdev)
"Failed to init reserved memory region.");
}
- ret = init_misc_device_context(priv, 0, &priv->priv_dev_ctx);
+ ret = init_misc_device_context(priv, 0, &priv->priv_dev_ctx, &se_if_fops);
if (ret)
return dev_err_probe(dev, ret,
"Failed[0x%x] to create device contexts.",
@@ -407,6 +1387,7 @@ static int se_suspend(struct device *dev)
struct se_fw_load_info *load_fw;
int ret = 0;
+ set_se_rcv_msg_timeout(priv, SE_RCV_MSG_DEFAULT_TIMEOUT);
load_fw = get_load_fw_instance(priv);
if (load_fw->imem_mgmt) {
diff --git a/drivers/firmware/imx/se_ctrl.h b/drivers/firmware/imx/se_ctrl.h
index 5d7cd10b4d02..49e619e0b7f8 100644
--- a/drivers/firmware/imx/se_ctrl.h
+++ b/drivers/firmware/imx/se_ctrl.h
@@ -15,10 +15,20 @@
#define SE_MSG_WORD_SZ 0x4
#define RES_STATUS(x) FIELD_GET(0x000000ff, x)
+#define MAX_DATA_SIZE_PER_USER (65 * 1024)
#define MAX_NVM_MSG_LEN (256)
#define MESSAGING_VERSION_6 0x6
#define MESSAGING_VERSION_7 0x7
+struct se_if_open_gate {
+ struct miscdevice miscdev;
+ struct se_if_priv *priv;
+ /* to lock to update the structure */
+ struct mutex lock;
+ struct kref refcount;
+ bool dying;
+};
+
struct se_clbk_handle {
struct se_if_device_ctx *dev_ctx;
struct completion done;
@@ -45,10 +55,42 @@ struct se_imem_buf {
u32 state;
};
+struct se_buf_desc {
+ u8 *shared_buf_ptr;
+ void __user *usr_buf_ptr;
+ u32 size;
+ struct list_head link;
+};
+
+struct se_shared_mem {
+ dma_addr_t dma_addr;
+ u32 size;
+ u32 pos;
+ u8 *ptr;
+};
+
+struct se_shared_mem_mgmt_info {
+ struct list_head pending_in;
+ struct list_head pending_out;
+
+ struct se_shared_mem non_secure_mem;
+};
+
/* Private struct for each char device instance. */
struct se_if_device_ctx {
struct se_if_priv *priv;
+ struct miscdevice *miscdev;
const char *devname;
+ bool cleanup_done;
+
+ /* process one file operation at a time. */
+ struct mutex fops_lock;
+
+ struct se_shared_mem_mgmt_info se_shared_mem_mgmt;
+ struct list_head link;
+
+ /* Add reference counting */
+ struct kref refcount;
};
/* Header of the messages exchange with the EdgeLock Enclave */
@@ -98,6 +140,19 @@ struct se_if_priv {
atomic_t fw_busy;
struct se_if_device_ctx *priv_dev_ctx;
+ struct list_head dev_ctx_list;
+
+ /* prevent modifying priv member variable in parallel. */
+ struct mutex modify_lock;
+ u32 active_devctx_count;
+ u32 dev_ctx_mono_count;
+ u32 se_rcv_msg_timeout_ms;
+
+ /* Add reference counting */
+ struct kref refcount;
+
+ /* stable gate used by .open() */
+ struct se_if_open_gate *open_gate;
};
char *get_se_if_name(u8 se_if_id);
diff --git a/include/uapi/linux/se_ioctl.h b/include/uapi/linux/se_ioctl.h
new file mode 100644
index 000000000000..0c948bdc8c26
--- /dev/null
+++ b/include/uapi/linux/se_ioctl.h
@@ -0,0 +1,97 @@
+/* SPDX-License-Identifier: (GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause*/
+/*
+ * Copyright 2025 NXP
+ */
+
+#ifndef SE_IOCTL_H
+#define SE_IOCTL_H
+
+#include <linux/types.h>
+
+#define SE_TYPE_STR_DBG "dbg"
+#define SE_TYPE_STR_HSM "hsm"
+#define SE_TYPE_ID_UNKWN 0x0
+#define SE_TYPE_ID_DBG 0x1
+#define SE_TYPE_ID_HSM 0x2
+/* IOCTL definitions. */
+
+struct se_ioctl_setup_iobuf {
+ void __user *user_buf;
+ __u32 length;
+ __u32 flags;
+ __u64 ele_addr;
+};
+
+struct se_ioctl_shared_mem_cfg {
+ __u32 base_offset;
+ __u32 size;
+};
+
+struct se_ioctl_get_if_info {
+ __u8 se_if_id;
+ __u8 interrupt_idx;
+ __u8 tz;
+ __u8 did;
+ __u8 cmd_tag;
+ __u8 rsp_tag;
+ __u8 success_tag;
+ __u8 base_api_ver;
+ __u8 fw_api_ver;
+};
+
+struct se_ioctl_cmd_snd_rcv_rsp_info {
+ __u32 __user *tx_buf;
+ int tx_buf_sz;
+ __u32 __user *rx_buf;
+ int rx_buf_sz;
+};
+
+struct se_ioctl_get_soc_info {
+ __u16 soc_id;
+ __u16 soc_rev;
+};
+
+/* IO Buffer Flags */
+#define SE_IO_BUF_FLAGS_IS_OUTPUT (0x00u)
+#define SE_IO_BUF_FLAGS_IS_INPUT (0x01u)
+#define SE_IO_BUF_FLAGS_USE_SEC_MEM (0x02u)
+#define SE_IO_BUF_FLAGS_USE_SHORT_ADDR (0x04u)
+#define SE_IO_BUF_FLAGS_IS_IN_OUT (0x10u)
+
+/* IOCTLS */
+#define SE_IOCTL 0x0A /* like MISC_MAJOR. */
+
+/*
+ * ioctl to designated the current fd as logical-reciever.
+ * This is ioctl is send when the nvm-daemon, a slave to the
+ * firmware is started by the user.
+ */
+#define SE_IOCTL_ENABLE_CMD_RCV _IO(SE_IOCTL, 0x01)
+
+/*
+ * ioctl to get the buffer allocated from the memory, which is shared
+ * between kernel and FW.
+ * Post allocation, the kernel tagged the allocated memory with:
+ * Output
+ * Input
+ * Input-Output
+ * Short address
+ * Secure-memory
+ */
+#define SE_IOCTL_SETUP_IOBUF _IOWR(SE_IOCTL, 0x03, struct se_ioctl_setup_iobuf)
+
+/*
+ * ioctl to get the mu information, that is used to exchange message
+ * with FW, from user-spaced.
+ */
+#define SE_IOCTL_GET_MU_INFO _IOR(SE_IOCTL, 0x04, struct se_ioctl_get_if_info)
+/*
+ * ioctl to get SoC Info from user-space.
+ */
+#define SE_IOCTL_GET_SOC_INFO _IOR(SE_IOCTL, 0x06, struct se_ioctl_get_soc_info)
+
+/*
+ * ioctl to send command and receive response from user-space.
+ */
+#define SE_IOCTL_CMD_SEND_RCV_RSP _IOWR(SE_IOCTL, 0x07, struct se_ioctl_cmd_snd_rcv_rsp_info)
+#endif
--
2.43.0
^ permalink raw reply related
* [PATCH v26 0/7] firmware: imx: driver for NXP secure-enclave
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,
Frieder Schrempf, kernel test robot, sashiko-bot
The NXP's i.MX EdgeLock Enclave, a HW IP creating an embedded secure
enclave within the SoC boundary to enable features like
- HSM
- SHE
- V2X
Communicates via message unit with linux kernel. This driver is
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 can communicate on single MU.
It exists on some i.MX processors. e.g. i.MX8ULP, i.MX93 etc.
-------
Changes in v26:
- Folded kernel test robot and Sashiko-bot fixes into the series.
- Added MAILBOX dependency for COMPILE_TEST builds.
- Hardened response waiter timeout handling and late-response processing.
- Serialized command receiver registration and callback-visible receiver state.
- Added iobuf round_up() overflow detection.
- Rolled back iobuf shared-memory reservation on setup failures.
- Bounded userspace-controlled response buffer size.
- Preserved ioctl operation errors across cleanup.
- Added explicit priv/dev_ctx lifetime handling and teardown/open serialization.
Testing:
- checkpatch.pl --strict: no warnings
- sparse: no warnings
- coccicheck on drivers/firmware/imx/: no driver-specific warnings
Reference:
- Link to v25: https://lore.kernel.org/r/20260122-imx-se-if-v25-0-5c3e3e3b69a8@nxp.com
Changes in v25:
5/7
- removes kernel bot reported warning errors.
3/7
- fix checkpatch --strict error.
1/2, 2/7, 4/7, 6/7 & 7/7
- No changes
Reference:
- Link to v24: https://lore.kernel.org/r/20260121-imx-se-if-v24-0-c5222df51cc2@nxp.com
Changes in v24:
5/7 & 3/7
- removes kernel bot reported warning errors.
1/2, 2/7, 4/7, 6/7 & 7/7
- No changes
Reference:
- Link to v23: https://lore.kernel.org/r/20251219-imx-se-if-v23-0-5c6773d00318@nxp.com
Changes in v23:
5/7
- removed un-neccessary 'kfree' from the func se_ioctl_cmd_snd_rcv_rsp_handler().
1/2, 2/7, 3/7, 4/7, 6/7 & 7/7
- No changes
Reference:
- Link to v22: https://lore.kernel.org/r/20251218-imx-se-if-v22-0-07418c872509@nxp.com
Changes in v22:
3/7 & 5/7
- reverted to previous change of using "__free(kfree)", by declare-and-initialize __free() vars next to their allocations.
1/7
- rename the se_fw.c to se_ctrl.c
2/7, 4/7, 5/7 & 7/7
- No changes.
Reference:
- Link to v21: https://lore.kernel.org/r/20251212-imx-se-if-v21-0-ee7d6052d848@nxp.com
Changes in v21:
3/7
- smatch warning fixes.
- Added "COMPILE_TEST" into "depends on IMX_MBOX && ARCH_MXC && ARM64"
- removed "__free(kfree)" & added kfree();
5/7
- removed "__free(kfree)" & added kfree();
1/2, 2/7, 4/7, & 7/7
- No changes
Reference:
- Link to v20: https://lore.kernel.org/r/20251203-imx-se-if-v20-0-a04a25c4255f@nxp.com
Changes in v20:
5/7:
- adds a func "se_chk_tx_msg_hdr", to check the validity of the in-coming message from usersapce.
1/2, 2/7, 3/7, 4/7, 6/7 & 7/7
- No changes
Reference:
- Link to v19: https://lore.kernel.org/r/20250927-imx-se-if-v19-0-d1e7e960c118@nxp.com
Changes in v19:
1/7
- Added 9 lines to the Introduction from line 73-82.
3/7
- Update the commit message for " For i.MX9x SoC(s) there is at least one
dedicated ELE MU(s) for each world - Linux(one or more) and OP-TEE OS (one or
more), that needs to be shared between them.."
Reference:
- Link to v18: https://lore.kernel.org/r/20250619-imx-se-if-v18-0-c98391ba446d@nxp.com
Changes in v18:
1/7
- Wrap both diagrams above in literal code block by using double-colon
3/7 & 5/7
- Collected Frank's R-b tag.
2/7, 4/7, 6/7 & 7/7
- No changes
Reference:
- Link to v17: https://lore.kernel.org/r/20250426-imx-se-if-v17-0-0c85155a50d1@nxp.com
Changes in v17:
- Changes to 3/7 & 5/7: to wrap code text at 80 character whereever possible.
Reference:
- Link to v16: https://lore.kernel.org/r/20250409-imx-se-if-v16-0-5394e5f3417e@nxp.com
Changes in v16:
- commit 3/7 and 4/7 are moved to end commits making them as 6/7 and 7/7 respectively.
- No change in 1/7 & 2/7.
7/7
- Collected Frank's R-b tag.
6/7
- commit message is updated to wrap at 75 characters.
5/7
- func add_b_desc_to_pending_list, removed the initialization of b_desc to
NULL.
- variable timeout in func ele_msg_rcv(), is renamed to timeout_ms.
- struct se_if_priv, member variable se_rcv_msg_timeout, is renamed to
se_rcv_msg_timeout_ms.
- in func load_firmware, move the label exit after dma_free_coherent.
4/7
- commit message is updated to wrap at 75 characters.
3/7
- ele_debug_dump, updated the assignment of keep_logging.
- ele_fw_authenticate function definition is updated to take two address
as arguments.
Reference:
- Link to v15: https://lore.kernel.org/r/20250407-imx-se-if-v15-0-e3382cecda01@nxp.com
Changes in v15:
- Patch 3/6 is split into two:
- 3/7: arm64: dts: imx8ulp-evk: add reserved memory property
- 4/7: arm64: dts: imx8ulp: add nxp secure enclave firmware
- No change in 1/7 & 2/7.
7/7
- removed the se_intance_id structure member variable.
- replace variable name from wait to timeout.
- used 'goto' to follow the common exit path calling "release_firmware(fw);" in case of error path.
- removed TBD string.
- Used ARRAY_SIZE(pending_lists).
- moved init_device_context after init_misc_device_context.
- defined err as long to avoid force convert in func
- added se_rcv_msg_timeout to priv, to control probe/suspend/resume per interface.
6/7
- removed the se_intance_id structure member variable.
- Added dev_ctx to the structure se_clbk_handle, too.
- Collected Frank's R-b tag.
5/7
- removed the se_intance_id structure member variable.
- since added se_if_probe_cleanup to devm, se_if_remove() is redundant. hence removed it.
- rename se_add_msg_chksum to se_get_msg_chksum
- added check if msg-size is 4 byte aligned.
- Fixed multiline comments.
- ele_debug_dump api is updated as part of comment disposition like single setting of flag "keep_logging" & adding if (ret).
- moved dev_err to dev_dbg, for imem save/restore functions.
- moved func get_se_if_name, from 7/7 to here.
3/7
- Updated the commit message.
- split the current patch into two:
-- 3/7 for board dts, and
-- 4/7 for chip dts
Reference:
- Link to v14: https://lore.kernel.org/r/20250327-imx-se-if-v14-0-2219448932e4@nxp.com
Changes in v14:
- Patch 5/5 is split into two:
- firmware: drivers: imx: adds miscdev
- Introduce dev-ctx dedicated to private.
-- Base patch before enabling misc-device context, to have the send-receive path, based on device context.
- No change in 1/6 & 2/6.
- Copied change logs from individual commits.
6/6
- moved definition of func se_load_firmware, from 4/6 patch to this patch.
- split init_device_context to init_misc_device_context.
- Different value of se_rcv_msg_timeout is required to be set. Receiving the response of 4K RSA operation can to take upto 3 minutes.
This long value cannot be set during Linux: boot-up and suspend-resume.
Hence, it will be set to default small-value during Linux: boot-up and suspend-resume.
- func se_dev_ctx_cpy_out_data(), in either case: do_cpy true or false, the clean-up needs to be done and it is implemented like wise.
Once do_cpy is false, no need to continue copy to user buffer. But continue to do clean-up. hence cannot return.
And every dev-ctx operation is done after taking the lock. Hence, two operations with same dev-ctx is not possible in parallel.
- func "init_device_context", for 0th misc dev_ctx, which is created at the time of probe, the device memory management is required. hence there is a difference.
- func "init_device_context", dev_er is replaced with return dev_err_probe.
- func "init_device_context", devm_add_action is replaced by devm_add_action_reset.
- removed type-cast from func se_ioctl_get_se_soc_info_handler().
- used scoped_cond_guard(mutex, _intr, return -EBUSY, &<mutex_lock>)
- combined dev_err & dev_dbg to one dev_err in se_if_fops_read().
- removed the structure member "se_shared_mem_mgmt->secure_mem".
4/6
- trimmed the ele_fetch_soc_info.
- removed the function ptr "se_info->se_fetch_soc_info" and replaced with ele_fetch_soc_info.
- moved definition of func se_load_firmware, to 6/6 patch.
- Different SoC, different ways to fetch soc_info. Generic function declaration for ele_fetch_soc_info() is needed. Hence wrapping ele_get_info() in it.
- Updated Kconfig help text for assertive tone.
- func ele_debug_dump is updated, to remove constructing the format string.
- removed the macro usage for SOC_ID_MASK.
- used low case hex number.
- Condition will never occur, where msg_len satisfy the following condition "msg_len % 4 != 0". Err msg is added if it occurs.
- Function description is added to se_add_msg_crc.
- timeout is added to function ele_msg_rcv, in 5/5 patch.
- local variable "header" is initialized with "tx_msg" and replaced "return err" with "return tx_msg_sz" in func ele_msg_send().
- replace function name from "exception_for_size" to "check_hdr_exception_for_sz"
- replaced "return ret > 0 ? 0 : -1;" with "return ret > 0 ? 0 : ret;" in func "se_save_imem_state".
- func "se_restore_imem_state", to return if the condition is false to proceed.
- removed casting by (void *).
- removed devm_kasprintf and done direct allocatiion for attr->soc_id = "i.MX8ULP" & attr->soc_id = "i.MX8ULP", & attr->family.
- Followed Reverse christmas tree order, whereever missing.
- There is no return if ele_fw_authenticate fails. Execution flow continue forward and execute the fucn dma_free_coherent().
- The loop is not for retry. The loop is needed to load secondary fw followed by loading primary fw, first. This is the case when ELE also got reset.
- dev_err_probe is corrected in func "se_if_request_channel".
3/6
-
Reference:
- Link to v13: https://lore.kernel.org/r/20250311-imx-se-if-v13-0-9cc6d8fd6d1c@nxp.com
Changes in v13:
5/5
- Updated the commit message for imperative mood.
- Remove the usage of macros- NODE_NAME, GET_ASCII_TO_U8, GET_IDX_FROM_DEV_NODE_NAME.
- Clean-up the return path by replacing "ret = -<err>; return ret;" with "return -<err>;"
- Clean-up the return path by replacing "ret = -<err>; goto exit;" with "return -<err>;"
- Removed goto statements from the entire driver, where there is no common code at function's exit.
- Fixes the check-patch erros reported with flag "--strict"
- Replaced devm_add_action, with devm_add_action_or_reset
- Removed the un-necesary and obvious code comments.
- Removed dev_probe_err at the exit of function se_if_probe().
4/5
- Clean-up the return path by replacing "ret = -<err>; return ret;" with "return -<err>;"
- Clean-up the return path by replacing "ret = -<err>; goto exit;" with "return -<err>;"
- Removed goto statements from the entire driver, where there is no common code at function's exit.
- fixes the check-patch erros reported with flag "--strict"
- removed the un-necesary and obvious code comments.
- variable received msg timeout to be different at boot-up & suspend/resume and send/recv ioctlis.
3/5
- compatible string is modified from "fsl,imx8ulp-se" to "fsl,imx8ulp-se-ele-hsm".
- updated the alias name.
2/5
- compatible string is modified from "fsl,imx8ulp-se" to "fsl,imx8ulp-se-ele-hsm".
- compatible string is modified from "fsl,imx93-se" to "fsl,imx93-se-ele-hsm".
- compatible string is modified from "fsl,imx95-se" to "fsl,imx95-se-ele-hsm".
- Mis-understood the +1 from Conor. Hence dropped the Reviewed-by tag.
- Collected Rob's R-b tag on v7 (https://lore.kernel.org/all/172589152997.4184616.5889493628960272898.robh@kernel.org/)
1/5
- No change
Reference:
- Link to v12: https://lore.kernel.org/r/20250120-imx-se-if-v12-0-c5ec9754570c@nxp.com
Changes in v12:
5/5
- increased the wait-timeout.
4/5
- rename flag "handle_susp_resm" to "imem_mgmt"
- moved the buffer allocation ot load_fw->imem.buf, to se_probe_if.
- setting imem state at initialization.
3/5
- No change
2/5
- No change
1/5
- No change
Reference:
- Link to v11: https://lore.kernel.org/r/20241220-imx-se-if-v11-0-0c7e65d7ae7b@nxp.com
Changes in v11:
5/5
- devname is constructed by concatinating get_se_if_name(se_if_id) & se_if_instance_id.
- ele_rcv_msg(), is updated to add the wait_interruptible_timeout for the non-NVM-Daemon message exchanges, such that in case of no response from FW,
Linux donot hangs.
- added a new helper function get_se_if_name(), to return the secure-enclave interface owner's name string.
- added a new helper function get_se_soc_id(), to return the secure-enclave's SoC id.
4/5
- moved the se_if_node_info member "soc_register", to the struct "se_if_node_info_list"; as soc registration done once, not per interface.
- moved the se_if_node_info member "se_fetch_soc_info", to the struct "se_if_node_info_list"; as soc info fetching is done once, not per interface.
- Added two member variable se_if_id and se_if_instance_id to struct se_if_defines.
- removed the member "se_name" from struct "se_if_node_info". Rather, it will constructed by concatinating get_se_if_name(se_if_id) & se_if_instance_id.
- moved the static global variable "se_version", to the newly created structure "struct se_var_info".
- moved the member "struct se_fw_load_info load_fw" of "se_if_node_info_list", to the newly created structure "struct se_var_info".
- Replaced RUNTIME_PM_OPS with SET_SYSTEM_SLEEP_PM_OPS, in power-managment ops.
3/5
- No change
2/5
- No change
1/5
- No change
Reference:
- Link to v10: https://lore.kernel.org/r/20241104-imx-se-if-v10-0-bf06083cc97f@nxp.com
v10: firmware: imx: driver for NXP secure-enclave
Changes in v10:
5/5
- replaced the u8, u16, u32, u64, with __u8, __u16, __u32, __u64 in
'include/uapi/linux/se_ioctl.h'.
4/5
- No change
3/5
- No change
2/5
- No change
1/5
- No change
Reference:
- Link to v9: https://lore.kernel.org/r/20241016-imx-se-if-v9-0-fd8fa0c04eab@nxp.com
Changes in v9:
4/5
- change se_if_remove function signature, required after rebase to v6.12-rc1.
- move the info->macros to a structure "struct se_if_defines if_defs".
- Removed "info" from "struct se_if_defines if_defs".
- Moved "mem_pool" from "struct se_if_defines if_defs" to "priv".
- Fetching "info" using container-of.
5/5
- Fetching "info" using container-of.
- Fixed issue reported by sparse.
Reference:
- Link to v8: https://lore.kernel.org/r/20241015-imx-se-if-v8-0-915438e267d3@nxp.com
Changes in v8:
5/5
- Remove the check for SE_IF_CTX_OPENED.
- replaced dev_ctx->priv-dev, priv->dev, whereever possible.
- func "if_misc_deregister" moved before func "init_device_context".
- func "init_device_context" before func "se_ioctl_cmd_snd_rcv_rsp_handler".
- func "se_if_fops_write" and "se_if_fops_read", are moved after func "se_ioctl_get_mu_info".
- non static functions "se_dev_ctx_cpy_out_data, se_dev_ctx_shared_mem_cleanup & init_device_context" are moved static and local scope.
- Removed back & forth between the two structs "struct se_if_device_ctx *dev_ctx" and "struct se_shared_mem_mgmt_info *se_shared_mem_mgmt"
- removed the NULL check for bdesc.
- fops_open, is corrected for acquiring the fops_lock.
- Fops_close, mutex unlock is removed. Infact check for waiting_rsp_clbk_hdl.dev_ctx, is removed.
- sema_init(&dev_ctx->fops_lock, 1);, replaced with Mutex.
- structure member se_notify, is removed.
4/5
- removed initializing err to zero in func ele_fetch_soc_info(),
- replaced 'return 0', with 'goto exit', if the condition (!priv->mem_pool) is true.
- replaced "struct *dev" with "struct se_if_priv *priv", in base_message API(s) and others.
- Created a separate structure "struct se_if_defines" to maintain interface's fixed values like cmd_tag, rsp_tag, success_tag etc.
- removed the macros "WORD_SZ", "SOC_VER_MASK", "DEFAULT_IMX_SOC_VER", "RESERVED_DMA_POOL".
- Added handling for "ctrl+c", by postponing the interrupt, till the response to the "command in flight" is received.
- Removed the mutext lock "se_if_lock".
- furnction prototype for "se_save_imem_state" and "se_restore_imem_state", is changed to pass "imem" by reference.
- Added a new structure "struct se_fw_load_info", dedicated to contain FW loading relevant info. It is a member of struct info_list.
- split "imem_mgmt_file_in_rfs" into two "prim_fw_nm_in_rfs" and "seco_fw_nm_in_rfs", to be part of "struct se_fw_load_info".
- moved the function "se_load_firmware" prior to func "if_mbox_free_channel".
- function "se_load_firmware" is updated to use "request_firmware", instead of "request_firmware_no_wait".
- function "se_load_firmware" is updated to load "primary" fw image, if the imem_state is not BAD. Then load the "secondary FW" image.
- Added a new mutex_lock in the function "se_load_firmware", for ensuring FW loading done once, when there are multiple application are in play.
- instead of "wait_queue_head_t wq", used "sruct completion".
- add devm_add_action with action as se_if_probe_cleanup.
Reference:
- Link to v7: https://lore.kernel.org/r/20240904-imx-se-if-v7-0-5afd2ab74264@nxp.com
Changes in v7:
5/5
- struct se_clbk_handle, is added with a member struct se_if_device_ctx *dev_ctx.
- func call to ele_miscdev_msg_rcv() & ele_miscdev_msg_send(), are removed.
- func se_ioctl_cmd_snd_rcv_rsp_handler(), is modified to remove the func call to ele_miscdev_msg_rcv() & ele_miscdev_msg_send()
- func se_ioctl_cmd_snd_rcv_rsp_handler is callig func ele_msg_send_rcv(), instead.
- Mutext "se_cmd_if_lock", handling is removed from this patch.
- func ele_miscdev_msg_send() is replaced with func ele_msg_send(), in fops_write.
- func ele_miscdev_msg_rcv() is replaced with func ele_msg_rcv(), in fops_read.
- fops_open is modified to create the new dev_ctx instance (using func init_device_context()), which is not registered as miscdev.
- Only one dev_ctx is registered as miscdev and its reference is stored in the struct se_if_priv, as priv_dev_ctx.
- Separate func cleanup_se_shared_mem() & func init_se_shared_mem(), for shared memory handling part of struct dev_ctx.
- Input param for func(s) ele_msg_rcv(), ele_msg_send() & ele_msg_send_rcv(), is replaced from struct se_if_priv to struct se_if_device_ctx.
4/5
- A new structure is defined name struct "se_clbk_handle", to contain members processed in mailbox call-back function.
- "struct se_if_priv" is modified to contain the two structures of "se_clbk_handle" - waiting_rsp_clbk_hdl & cmd_receiver_clbk_hdl.
- func ele_msg_rcv() is modified to take a new additional input reference param "struct se_clbk_handle *se_clbk_hdl".
- func ele_msg_send() is modified to take a new additional input tx_msg_sz.
- func ele_msg_send_rcv(), is modified to take 2 more inputs - tx_msg_sz & exp_rx_msg_sz.
- func se_val_rsp_hdr_n_status(), is modified to take input of rx_msg buffer, instead of header value, as input param.
- each caller of the func ele_msg_send_rcv(), is sending these two additional input params.
- func se_if_callback(), is modified to work on two structures of "se_clbk_handle" - waiting_rsp_clbk_hdl & cmd_receiver_clbk_hdl.
- Variable "max_dev_ctx", is removed from info & priv struture, as well its usage.
- New member variable "se_img_file_to_load", is added to structure "priv".
- Other member variables - rx_msg(ptr), rx_msg_sz, completion done & list of dev_ctxs, is removed from priv struture, along with their usage.
- func se_resume(), updated to wakeup the two "wq", part of "struct se_clbk_handle": priv->waiting_rsp_clbk_hdl & priv->cmd_receiver_clbk_hdl.
3/5
- Node name is changed from senclave-firmware@0 to "secure-enclave"
2/5
- Node name is changed to "secure-enclave".
Reference:
- Link to v6: https://lore.kernel.org/r/20240722-imx-se-if-v6-0-ee26a87b824a@nxp.com
Changes in v6:
5/5
- replaced scope_gaurd with gaurd.
4/5
- replaced scope_gaurd with gaurd.
- remove reading the regs property from dtb.
- Added NULL check for priv data fetched from device, as a sanity check, for ele_base_msg apis)
3/5
- replace firmware with senclave-firmware.
2/5
- replace firmware with senclave-firmware.
- drop description for mbox
- Replaced "items:" with maxItems:1 for "memory-region"
- Replaced "items:" with maxItems:1 for "sram"
- remove regs property.
- remove "$nodename"
Reference:
- Link to v5: https://lore.kernel.org/r/20240712-imx-se-if-v5-0-66a79903a872@nxp.com
Changes in v5:
2/5
- updated the description of mboxes
- updated the description & items for mbox-names.
- updated the description of memory-region
- move "additional properties: false" after allOf block.
- removed other example except one.
4/5
- Corrected the indentation in Kconfig.
- info members:mbox_tx_name & mbox_rx_name, are replaced with macros.
5/5
- Replaced "for secure enclaves", with "for secure enclaves"
- Replaced "user space" with "userspace".
- End the line "[include]<linux/firmware/imx/ele_mu_ioctl.h>" with a period.
Reference:
- Link to v4: https://lore.kernel.org/r/20240705-imx-se-if-v4-0-52d000e18a1d@nxp.com
Changes in v4:
1/5
a. Removed - from EdgeLock Enclave.
b. Removed , after "Each of the above feature,"
c. replace "can exists" with "can exist".
d.
-messaging units(MU) per SE. Each co-existing 'se' can have one or multiple exclusive
-MU(s), dedicated to itself. None of the MU is shared between two SEs.
+messaging units(MU) per SE. Each co-existing SE can have one or multiple exclusive
+MUs, dedicated to itself. None of the MU is shared between two SEs.
Communication of the MU is realized using the Linux mailbox driver.
e.
-All those SE interfaces 'se-if' that is/are dedicated to a particular SE, will be
-enumerated and provisioned under the very single 'SE' node.
+Although MU(s) is/are not shared between SE(s). But for SoC like i.MX95 which has
+multiple SE(s) like HSM, V2X-HSM, V2X-SHE; all the SE(s) and their interfaces 'se-if'
+that is/are dedicated to a particular SE will be enumerated and provisioned using the
+single compatible node("fsl,imx95-se").
f. Removed ",". Replaced for "Each 'se-if'," with "Each se-if'.
g. removed ","
- This layer is responsible for ensuring the communication protocol, that is defined
+ This layer is responsible for ensuring the communication protocol that is defined
h. removed "-"
- - FW can handle one command-message at a time.
+ - FW can handle one command message at a time.
i.
- Using these multiple device contexts, that are getting multiplexed over a single MU,
- user-space application(s) can call fops like write/read to send the command-message,
- and read back the command-response-message to/from Firmware.
- fops like read & write uses the above defined service layer API(s) to communicate with
+ Using these multiple device contexts that are getting multiplexed over a single MU,
+ userspace application(s) can call fops like write/read to send the command message,
+ and read back the command response message to/from Firmware.
+ fops like read & write use the above defined service layer API(s) to communicate with
Firmware.
j. Uppercase for word "Linux".
2/5
a. Rephrased the description to remove list of phandles.
b. Moved required before allOf:
+required:
+ - compatible
+ - reg
+ - mboxes
+ - mbox-names
+
+additionalProperties: false
+
allOf:
c. replaced not: required: with properties: <property-name>: false.
# memory-region
- not:
- required:
- - memory-region
+ properties:
+ memory-region: false
# sram
- else:
- not:
- required:
- - sram
d. Reduced examples. keeping example of i.MX95.
e. node-name is changed to "firmware@<hex>"
3/5
- node name changed to "firmware@<hex>".
4/5
- used sizeof(*s_info)
- return early, rather than doing goto exit, in ele_get_info().
- Use upper_32_bits() and lower_32_bits()
- use rx_msg here instead of priv->rx_msg
- Moved the status check to validate_rsp_hdr. Rename the function to "se_val_rsp_hdr_n_status"
- typecasting removed header = (struct se_msg_hdr *) msg;
- Converted the API name with prefix imx_ele_* or imx_se_*, to ele_* and se_*, respectively.
- Removed the functions definition & declaration for: free_phybuf_mem_pool() & get_phybuf_mem_pool()
- removed the mbox_free_channel() calls from clean-up.
- Flag "priv->flags" is removed.
- Converted the int se_if_probe_cleanup() to void se_if_probe_cleanup().
- Replaced NULL initialization of structure members: priv->cmd_receiver_dev & priv->waiting_rsp_dev , with comments.
- Removed the function's declaration get_phy_buf_mem_pool1
5/5
Changes to Documentation/ABI/testing/se-cdev.
a. Removed "-" from "secure-enclave" and "file-descriptor".
b. Removed "-" from "shared-library"
c. Replaced "get" with "getting".
d. Added description for the new IOCTL "send command and receive command response"
e. Replaced "wakeup_intruptible" with "wait_event_interruptible"
f. Removed ";"
g. Removd "," from "mailbox_lock,"
h. Replaced "free" with "frees"
i. In mailbox callback function, checking the buffer size before
copying.
Reference:
- Link to v3: https://lore.kernel.org/r/20240617-imx-se-if-v3-0-a7d28dea5c4a@nxp.com
Changes in v3:
5/5:
- Initialize tx_msg with NULL.
- memdup_user() returns an error pointer, not NULL. correct it by adding check for err_ptr.
- new IOCTL is added to send & recieve the message.
- replaced the while loop till list is empty, with list_for_each_entry.
- replaced __list_del_entry, with list_del.
- Removed the dev_err message from copy to user.
- Removed the casting of void *.
- corrected the typcasting in copy to user.
- removed un-necessary goto statement.
- Removed dead code for clean-up of memory.
- Removed un-mapping of secured memory
- Passing se_if_priv structure to init_device_context.
- Updated the below check to replace io.length with round_up(io.length).
if (shared_mem->size < shared_mem->pos|| io.length >= shared_mem->size - shared_mem->pos)
- Created a function to cleanup the list of shared memory buffers.
- Used list_for_each_entry_safe(). created a separate functions: se_dev_ctx_cpy_out_data() & se_dev_ctx_shared_mem_cleanup()
4/5
- Changed the compatible string to replace "-ele", to "-se".
- Declaration of imx_se_node_info, is done as const in the whole file
- Remove the unused macros from ele_base_msg.h
- Remove the function declaration get_phy_buf_mem_pool1, from the header file.
- Replace the use of dmam_alloc_coherent to dma_alloc_coherent
- Check for function pointer, before calling the fucntion pointer in imx_fetch_se_soc_info
- Removed the unused flag for SE_MU_IO_FLAGS_USE_SEC_MEM.
- Removed the unused macros WORD_SZ
- instead of struct device *dev, struct se_if_priv *priv, is used as argument to the funtions:se_save_imem_state, se_restore_imem_state, imx_fetch_se_soc_info
- Removed ret from validate_rsp_hdr.
- changed the prefix of the funtion: plat_add_msg_crc and plat_fill_cmd_msg_hdr.
- indentation correction for info structures.
- remove the check for priv not null from se_if_probe_cleanup
- Removed the casting of void *.
- se_load_firmware function is corrected for not freeing the buffer when allocation fails.
- Checking if get_imx_se_node_info() can return NULL, in se_if_probe()
- imem.size has type u32. return value from se_save_imem_state() will be assigned to imem.size in case of success only.
- removed the flag un-setting in case of failure. priv->flags &= (~RESERVED_DMA_POOL);
- removed the function call for devm_of_platform_populate(dev);
- Checking for not-NULL, before calling the funtion pointer se_fetch_soc_info.
- Removed the checking for reserved memory flag, before freeing up the reserved memory, in se_probe_if_cleanup.
3/5
- Changed the compatible string to replace "-ele", to "-se".
2/5
- to fix the warning error, replaced the "-ele" & "-v2x" in compatible string, to "-se".
- Added an example for ele@0 for compatible string "fsl,imx95-se"
Reference
- Link to v2: https://lore.kernel.org/r/20240523-imx-se-if-v2-0-5a6fd189a539@nxp.com
Changes in v2:
4/4
- Split this patch into two: 1. base driver & 2. Miscdev
- Initialize the return variable "err" as 0, before calling 'return err', in the file ele_common.c
- Fix the usage of un-iniitialized pointer variable, by initializing them with NULL, in ele_base_msg.c.
- Fix initializing the ret variable, to return the correct error code in case of issue.
- replaced dmam_alloc_coherent with dma_alloc_coherent.
- Replace the use of ELE_GET_INFO_READ_SZ, with sizeof(soc_info).
- Replaced -1 with -EPERM
- Removed the safety check on func-input param, in ele_get_info().
- fix the assigning data[1] with lower 32 address, rather than zero, for ele_fw_authenticate API.
- Correctly initializing the function's return error code, for file ele_base_msg.c.
- replaced 'return' with 'goto'.
- Use length in bytes.
- Corrected the structure se_msg_hdr.
- Moved setting of rx_msg to priv, into the function imx_ele_msg_send_rcv
- Will add lockdep_assert_held, to receive path, in v2.
- corrected the spacing at "ret = validate_rsp_hdr"
- FIELD_GET() used for RES_STATUS
- Re-write the structure soc_info, matching the information provided in response to this api.
- The "|" goes to the end of the previous line.
- Moved the locking and unlocking of the command lock to the caller of the function.
- removed the safety check for device private data.
- Structure memory reference, used to read message header.
- In the interrupt call back function, remove assigning waiting_rsp_dev to NULL, in case of response message rcv from FW.
- do while removed.
- replaced BIT(1) for RESERVED_DMA_POOL, to BIT(0)
- The backslash is removed while assigning the file name with absolute path to structure variable.fw_name_in_rfs =.
- Update the 'if' condition by removing "idx < 0".
- mbox_request_channel_byname() uses a "char" for the name not a u8. Corrected.
- devm managed resources, are not cleaned now, in function se_probe_if_cleanup
- Used dev_err_probe().
- Used %pe to print error string.
- remove "__maybe_unused" for "struct platform_device *enum_plat_dev __maybe_unused;"
- used FIELD_GET(), for RES_STATUS. Removed the use of MSG_TAG, MSG_COMMAND, MSG_SIZE, MSG_VER.
- Depricated the used of member of struct se_if_priv, bool no_dev_ctx_used;
- Moved the text explaing the synchronization logic via mutexes, from patch 1/4 to se_ctrl.h.
- removed the type casting of info_list = (struct imx_se_node_info_list *) device_get_match_data(dev->parent);
- Used static variable priv->soc_rev in the se_ctrl.c, replaced the following condition: if (info_list->soc_rev) to if (priv->soc_rev) for checking if this flow is already executed or not.
- imx_fetch_soc_info will return failure if the get_info function fails.
- Removed devm_free from imx_fetch_soc_info too.
3/3
- Made changes to move all the properties to parent node, without any child node.
2/4
- Use Hex pattern string.
- Move the properties to parent node, with no child node.
- Add i.MX95-ele to compatible nodes to fix the warning "/example-2/v2x: failed to match any schema with compatible: ['fsl,imx95-v2x']"
1/1
- Corrected the spelling from creats to creates.
- drop the braces around the plural 's' for interfaces
- written se in upper case SE.
- Replace "multiple message(s)" with messages.
- Removed too much details about locks.
Testing
- make CHECK_DTBS=y freescale/imx8ulp-evk.dtb;
- make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- -j8 dt_binding_check DT_SCHEMA_FILES=fsl,imx-se.yaml
- make C=1 CHECK=scripts/coccicheck drivers/firmware/imx/*.* W=1 > r.txt
- ./scripts/checkpatch.pl --git <>..HEAD
- Tested the Image and .dtb, on the i.MX8ULP.
Reference
- Link to v1: https://lore.kernel.org/r/20240510-imx-se-if-v1-0-27c5a674916d@nxp.com
---
Pankaj Gupta (7):
Documentation/firmware: add imx/se to other_interfaces
dt-bindings: arm: fsl: add imx-se-fw binding doc
firmware: imx: add driver for NXP EdgeLock Enclave
firmware: imx: device context dedicated to priv
firmware: drivers: imx: adds miscdev
arm64: dts: imx8ulp: add secure enclave node
arm64: dts: imx8ulp-evk: add reserved memory property
Documentation/ABI/testing/se-cdev | 44 +
.../devicetree/bindings/firmware/fsl,imx-se.yaml | 91 ++
.../driver-api/firmware/other_interfaces.rst | 133 ++
arch/arm64/boot/dts/freescale/imx8ulp-evk.dts | 12 +-
arch/arm64/boot/dts/freescale/imx8ulp.dtsi | 11 +-
drivers/firmware/imx/Kconfig | 13 +
drivers/firmware/imx/Makefile | 2 +
drivers/firmware/imx/ele_base_msg.c | 305 +++++
drivers/firmware/imx/ele_base_msg.h | 117 ++
drivers/firmware/imx/ele_common.c | 478 +++++++
drivers/firmware/imx/ele_common.h | 51 +
drivers/firmware/imx/se_ctrl.c | 1441 ++++++++++++++++++++
drivers/firmware/imx/se_ctrl.h | 159 +++
include/linux/firmware/imx/se_api.h | 14 +
include/uapi/linux/se_ioctl.h | 97 ++
15 files changed, 2965 insertions(+), 3 deletions(-)
---
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
change-id: 20240507-imx-se-if-a40055093dc6
Best regards,
--
Pankaj Gupta <pankaj.gupta@nxp.com>
^ permalink raw reply
* RE: [PATCH v9 04/12] reset: realtek: Add RTD1625-ISO reset controller driver
From: Yu-Chun Lin [林祐君] @ 2026-06-29 7:02 UTC (permalink / raw)
To: Philipp Zabel, mturquette@baylibre.com, sboyd@kernel.org,
robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
Edgar Lee [李承諭], afaerber@suse.com,
Jyan Chou [周芷安], bmasney@redhat.com
Cc: devicetree@vger.kernel.org, linux-clk@vger.kernel.org,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-realtek-soc@lists.infradead.org,
James Tai [戴志峰],
CY_Huang[黃鉦晏],
Stanley Chang[昌育德]
In-Reply-To: <36c8a3db6038db4973b6cd80c554339f5b71d32c.camel@pengutronix.de>
Hi Philipp,
>
> On Do, 2026-06-25 at 10:05 +0000, Yu-Chun Lin [林祐君] wrote:
> > Hi Philipp,
> >
> > > On Mi, 2026-06-24 at 19:29 +0800, Yu-Chun Lin wrote:
> > > > From: Cheng-Yu Lee <cylee12@realtek.com>
> > > >
> > > > Add support for the ISO (Isolation) domain reset controller on the
> > > > Realtek
> > > > RTD1625 SoC.
> > > >
> > > > The reset controller shares the same register space with the ISO
> > > > clock controller. To handle this shared register space, the reset
> > > > driver is implemented as an auxiliary driver. It will be
> > > > instantiated and probed via the auxiliary bus by the RTD1625-ISO clock
> controller driver.
> > > >
> > > > Signed-off-by: Cheng-Yu Lee <cylee12@realtek.com>
> > > > Co-developed-by: Yu-Chun Lin <eleanor.lin@realtek.com>
> > > > Signed-off-by: Yu-Chun Lin <eleanor.lin@realtek.com>
> > > > ---
> > > > Changes in v9:
> > > > - Extract reset-related code from the previous clock driver patch
> > > > (formerly patch 9 in v8).
> > > > ---
> > > > drivers/reset/realtek/Makefile | 2 +-
> > > > drivers/reset/realtek/reset-rtd1625-iso.c | 99
> > > > +++++++++++++++++++++++
> > > > 2 files changed, 100 insertions(+), 1 deletion(-) create mode
> > > > 100644 drivers/reset/realtek/reset-rtd1625-iso.c
> > > >
> > > > diff --git a/drivers/reset/realtek/Makefile
> > > > b/drivers/reset/realtek/Makefile index c3f605ffb11c..9007c9d5683b
> > > > 100644
> > > > --- a/drivers/reset/realtek/Makefile
> > > > +++ b/drivers/reset/realtek/Makefile
> > > > @@ -1,3 +1,3 @@
> > > > # SPDX-License-Identifier: GPL-2.0-only
> > > > obj-$(CONFIG_RESET_RTK_COMMON) += reset-rtk-common.o
> > > > -obj-$(CONFIG_RESET_RTD1625) += reset-rtd1625-crt.o
> > > > +obj-$(CONFIG_RESET_RTD1625) += reset-rtd1625-crt.o
> > > > +reset-rtd1625-iso.o
> > >
> > > Is there any benefit to these two being separate modules?
> > > I suggest you merge them into one: reset-rtd1625.o
> > >
> >
> > If I merge them into a single 'reset-rtd1625' module, both the 'crt'
> > and 'iso' clock drivers would trigger the probe process for the same
> > reset driver name, which would lead to a duplicate driver registration
> > error.
>
> What do you mean by duplicate driver registration error?
>
> There would only be one auxiliary_driver, with support for all three
> auxiliary_device_id's.
>
>
> regards
> Philipp
Ah, I understand what you mean now.
Previously, I only merged them in the Makefile but left two separate
'module_auxiliary_driver()' registrations in the C code, which caused the
duplicate registration error.
Following your suggestion, I will refactor the reset drivers into a single
'auxiliary_driver' entry with a single 'id_table' to
differentiate between the crt, iso, and iso_s reset types.
Best Regards,
Yu-Chun
^ permalink raw reply
* [PATCH v2] dt-bindings: arm-smmu: qcom: Add compatible for Maili SoC
From: Jingyi Wang @ 2026-06-29 6:56 UTC (permalink / raw)
To: Will Deacon, Robin Murphy, Joerg Roedel (AMD), Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: aiqun.yu, tingwei.zhang, trilok.soni, yijie.yang,
linux-arm-kernel, iommu, devicetree, linux-kernel, Jingyi Wang
Qualcomm Maili SoC includes apps smmu that implements arm,mmu-500,
which is used to translate device-visible virtual addresses to
physical addresses. Add compatible for it.
Signed-off-by: Jingyi Wang <jingyi.wang@oss.qualcomm.com>
---
Changes in v2:
- patch rebase
- Link to v1: https://lore.kernel.org/r/20260524-maili-smmu-v1-1-ec85b12fda83@oss.qualcomm.com
---
Documentation/devicetree/bindings/iommu/arm,smmu.yaml | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
index a701dec2fa0a..c48e9de1a2bd 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
@@ -39,6 +39,7 @@ properties:
- qcom,glymur-smmu-500
- qcom,hawi-smmu-500
- qcom,kaanapali-smmu-500
+ - qcom,maili-smmu-500
- qcom,milos-smmu-500
- qcom,nord-smmu-500
- qcom,qcm2290-smmu-500
---
base-commit: 3d5670d672ae08b8c534b7beed6f57c8b44e7b43
change-id: 20260628-maili-smmu-9ed0f2c189ad
Best regards,
--
Jingyi Wang <jingyi.wang@oss.qualcomm.com>
^ permalink raw reply related
* Re: [PATCH v3 4/7] arm64: dts: freescale: Convert to new media orientation definitions
From: Alexander Stein @ 2026-06-29 6:52 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Jacopo Mondi, Sakari Ailus, Jimmy Su, Matthias Fend,
Mikhail Rudenko, Daniel Scally, Jacopo Mondi, Michael Riesch,
Benjamin Mugnier, Sylvain Petinot, Laurent Pinchart, Paul Elder,
Martin Kepplinger, Quentin Schulz, Tommaso Merciai,
Svyatoslav Ryhel, Richard Acayan, Thierry Reding, Jonathan Hunter,
Frank Li, Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
Bjorn Andersson, Konrad Dybcio, Geert Uytterhoeven, Magnus Damm,
Heiko Stuebner, Kieran Bingham
Cc: linux-kernel, linux-media, devicetree, linux-tegra, linux, imx,
linux-arm-kernel, linux-arm-msm, linux-renesas-soc,
linux-rockchip, Kieran Bingham
In-Reply-To: <20260628-kbingham-orientation-v3-4-4ed92968aff8@ideasonboard.com>
Hi Kieran,
thanks for the patch.
Am Sonntag, 28. Juni 2026, 12:22:19 CEST schrieb Kieran Bingham:
> The orientation property for video interface devices now has definitions
> to prevent hardcoded integer values for the enum options.
>
> Update the users throughout the freescale/NXP device trees to use the new
> definitions.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
> .../boot/dts/freescale/imx8mp-tqma8mpql-mba8mp-ras314-imx219.dtso | 3 ++-
> arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi | 3 ++-
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mp-ras314-imx219.dtso b/arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mp-ras314-imx219.dtso
> index e5a2b3780215..7b44ae0f19b2 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mp-ras314-imx219.dtso
> +++ b/arch/arm64/boot/dts/freescale/imx8mp-tqma8mpql-mba8mp-ras314-imx219.dtso
> @@ -9,6 +9,7 @@
>
> #include <dt-bindings/gpio/gpio.h>
> #include <dt-bindings/media/video-interfaces.h>
> +#include <dt-bindings/media/video-interface-devices.h>
>
> #include "imx8mp-pinfunc.h"
>
> @@ -47,7 +48,7 @@ camera@10 {
> VANA-supply = <®_cam>;
> VDIG-supply = <®_cam>;
> VDDL-supply = <®_cam>;
> - orientation = <2>;
> + orientation = <MEDIA_ORIENTATION_EXTERNAL>;
> rotation = <0>;
>
> port {
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi b/arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi
> index f5d529c5baf3..178cfad93483 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mq-librem5.dtsi
> @@ -8,6 +8,7 @@
> #include "dt-bindings/input/input.h"
> #include <dt-bindings/interrupt-controller/irq.h>
> #include <dt-bindings/leds/common.h>
> +#include <dt-bindings/media/video-interface-devices.h>
> #include "dt-bindings/pwm/pwm.h"
> #include "dt-bindings/usb/pd.h"
> #include "imx8mq.dtsi"
> @@ -1116,7 +1117,7 @@ camera_front: camera@20 {
> vddd-supply = <®_vcam_1v2>;
> vddio-supply = <®_csi_1v8>;
> rotation = <90>;
> - orientation = <0>;
> + orientation = <MEDIA_ORIENTATION_FRONT>;
>
> port {
> camera1_ep: endpoint {
>
>
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/
^ permalink raw reply
* Re: [PATCH] drm/sun4i: Remove dependency on DRM simple helpers
From: Thomas Zimmermann @ 2026-06-29 6:51 UTC (permalink / raw)
To: Jernej Škrabec, Chen-Yu Tsai, Maarten Lankhorst,
Maxime Ripard, David Airlie, Simona Vetter, Samuel Holland,
Diogo Silva
Cc: dri-devel, linux-arm-kernel, linux-sunxi, linux-kernel
In-Reply-To: <PMdb9FVETQuPUnyh12NM2Q@gmail.com>
Hi
Am 27.06.26 um 08:55 schrieb Jernej Škrabec:
> Dne torek, 23. junij 2026 ob 22:34:40 Srednjeevropski poletni čas je Diogo Silva napisal(a):
>> Simple KMS helper are deprecated since they only add an intermediate
>> layer between drivers and the atomic modesetting.
>> This patch removes the dependency on drm simple helpers from sun4i
>> DRM drivers.
>>
>> Signed-off-by: Diogo Silva <diogompaissilva@gmail.com>
> This doesn't look as useful change. Is this tree-wide attempt? If so, it
> should be done in one go in one patch series, coordinated with DRM
> maintainers.
Yes, the interface is supposed to go away because it is fairly useless.
It's a low-effort task for beginners. Hence many people send out such
clean-up patches for the various drivers. If the change is acceptable
to you, it would be appreciated if you could merge it into the driver.
Best regards
Thomas
>
> Best regards,
> Jernej
>
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)
^ 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