* [PATCH v11 1/3] media: dt-bindings: Add CSI Pixel Formatter DT bindings
From: guoniu.zhou @ 2026-06-23 3:56 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Laurent Pinchart, Frank Li, Abel Vesa, Peng Fan,
Michael Turquette, Stephen Boyd
Cc: imx, linux-media, devicetree, linux-arm-kernel, linux-kernel,
linux-clk, Guoniu Zhou, Frank Li, Krzysztof Kozlowski
In-Reply-To: <20260623-csi_formatter-v11-0-a792fe9c1502@oss.nxp.com>
From: Guoniu Zhou <guoniu.zhou@nxp.com>
The i.MX95 CSI pixel formatting module uses packet info, pixel and
non-pixel data from the CSI-2 host controller and reformat them to
match Pixel Link(PL) definition.
Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
Changes in v11:
- Add Rb tags from Frank Li and Krzysztof Kozlowski
Changes in v10:
- Drop syscon parent node from example
- Drop Reviewed-by tags from Frank and Krzysztof due to binding changes
- Add description for reg property
- Add space after formatter@20 before opening brace in example
- Enhance the port description with more detailed information
- Delete the blank line immediately following the endpoint in example
Changes in v9:
- Use direct node instead of syscon wrapper in example
Changes in v8:
- Use standard port reference instead of video-interfaces.yaml
- Add parent syscon node in example to show device integration
- Add required constraints for port@0 and port@1 in ports node
Changes in v7:
- Change compatible to imx95-csi-formatter as IP is i.MX95 specific per Marco's suggestion
Link: https://lore.kernel.org/linux-media/20260511-csi_formatter-v6-0-01028e312e2b@oss.nxp.com/T/#mcd135b3de179b3cb69daa1fd6e0e8e27c85b3332
---
.../bindings/media/fsl,imx95-csi-formatter.yaml | 88 ++++++++++++++++++++++
1 file changed, 88 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/fsl,imx95-csi-formatter.yaml b/Documentation/devicetree/bindings/media/fsl,imx95-csi-formatter.yaml
new file mode 100644
index 000000000000..58c4e1cc056b
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/fsl,imx95-csi-formatter.yaml
@@ -0,0 +1,88 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/fsl,imx95-csi-formatter.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: i.MX95 CSI Pixel Formatter
+
+maintainers:
+ - Guoniu Zhou <guoniu.zhou@nxp.com>
+
+description:
+ The CSI pixel formatting module found on i.MX95 uses packet info, pixel
+ and non-pixel data from the CSI-2 host controller and reformat them to
+ match Pixel Link(PL) definition.
+
+properties:
+ compatible:
+ const: fsl,imx95-csi-formatter
+
+ reg:
+ maxItems: 1
+ description: Register offset and size within the parent syscon
+
+ clocks:
+ maxItems: 1
+
+ power-domains:
+ maxItems: 1
+
+ ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+
+ properties:
+ port@0:
+ $ref: /schemas/graph.yaml#/properties/port
+ description:
+ Input port, connects to MIPI CSI-2 receiver output (IDI interface)
+
+ port@1:
+ $ref: /schemas/graph.yaml#/properties/port
+ description:
+ Output port, connects to ISI input via Pixel Link (PL)
+
+ required:
+ - port@0
+ - port@1
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - power-domains
+ - ports
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/nxp,imx95-clock.h>
+
+ formatter@20 {
+ compatible = "fsl,imx95-csi-formatter";
+ reg = <0x20 0x100>;
+ clocks = <&cameramix_csr IMX95_CLK_CAMBLK_CSI2_FOR0>;
+ power-domains = <&scmi_devpd 3>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ endpoint {
+ remote-endpoint = <&mipi_csi_0_out>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+
+ endpoint {
+ remote-endpoint = <&isi_in_2>;
+ };
+ };
+ };
+ };
--
2.34.1
^ permalink raw reply related
* [PATCH v11 0/3] media: nxp: Add CSI Pixel Formatter support
From: guoniu.zhou @ 2026-06-23 3:56 UTC (permalink / raw)
To: Mauro Carvalho Chehab, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Shawn Guo, Sascha Hauer, Pengutronix Kernel Team,
Fabio Estevam, Laurent Pinchart, Frank Li, Abel Vesa, Peng Fan,
Michael Turquette, Stephen Boyd
Cc: imx, linux-media, devicetree, linux-arm-kernel, linux-kernel,
linux-clk, Guoniu Zhou, Frank Li, Krzysztof Kozlowski
CSI Pixel Formatter is a module found on i.MX95. It could unpack the
pixels received by the formatter and reformat them to meet the pixel
link format requirement.
This patch series adds a new V4L2 driver for CSI Pixel Formatter.
Background
----------
The Camera CSR binding was upstreamed during the pre-silicon phase when
the hardware integration details were not fully finalized. At that time,
the syscon binding focused on the CSR's primary role as a shared register
provider for clock gating, QoS, and error handling across Camera domain IPs.
However, the Camera CSR hardware is actually a mixed-function register
container with varying control levels over different IP blocks:
- ISP/CSI: Clock gating only
- ISI: QoS configuration
- CSI: Buffer overflow handling
- LPCAC: Error handling only
- ISP: Pixel link selection
- Formatter: Complete IP control (full register set)
Why Child Nodes Now
-------------------
The CSI formatter is unique among these IPs - it's an independent hardware
block whose complete control registers reside within the CSR address
space, not just a few configuration bits. This architectural detail was
not reflected in the original binding because:
1. The pre-silicon binding focused on simple register-level controls
2. The syscon pattern for complete IP control was not initially considered
Discussion with original CSR author(Peng Fan) confirmed this reflects the
intended hardware design that wasn't fully captured during pre-silicon
upstreaming.
v4l2-compliance 1.28.1-5233, 64 bits, 64-bit time_t
v4l2-compliance SHA: fc15e229d9d3 2024-07-23 19:22:15
Compliance test for device /dev/v4l-subdev9:
Driver Info:
Driver version : 7.1.0
Capabilities : 0x00000002
Streams Support
Client Capabilities: 0x0000000000000003
streams interval-uses-which
Required ioctls:
test VIDIOC_SUDBEV_QUERYCAP: OK
test invalid ioctls: OK
Allow for multiple opens:
test second /dev/v4l-subdev9 open: OK
test VIDIOC_SUBDEV_QUERYCAP: OK
test for unlimited opens: OK
Debug ioctls:
test VIDIOC_LOG_STATUS: OK (Not Supported)
Input ioctls:
test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
test VIDIOC_ENUMAUDIO: OK (Not Supported)
test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
test VIDIOC_G/S_AUDIO: OK (Not Supported)
Inputs: 0 Audio Inputs: 0 Tuners: 0
Output ioctls:
test VIDIOC_G/S_MODULATOR: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_ENUMAUDOUT: OK (Not Supported)
test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
test VIDIOC_G/S_AUDOUT: OK (Not Supported)
Outputs: 0 Audio Outputs: 0 Modulators: 0
Input/Output configuration ioctls:
test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
test VIDIOC_G/S_EDID: OK (Not Supported)
Sub-Device routing ioctls:
test Try VIDIOC_SUBDEV_G_ROUTING/VIDIOC_SUBDEV_S_ROUTING: OK
test Active VIDIOC_SUBDEV_G_ROUTING/VIDIOC_SUBDEV_S_ROUTING: OK
Control ioctls:
test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK (Not Supported)
test VIDIOC_QUERYCTRL: OK (Not Supported)
test VIDIOC_G/S_CTRL: OK (Not Supported)
test VIDIOC_G/S/TRY_EXT_CTRLS: OK (Not Supported)
test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK (Not Supported)
test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
Standard Controls: 0 Private Controls: 0
Format ioctls:
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK (Not Supported)
test VIDIOC_G/S_PARM: OK (Not Supported)
test VIDIOC_G_FBUF: OK (Not Supported)
test VIDIOC_G_FMT: OK (Not Supported)
test VIDIOC_TRY_FMT: OK (Not Supported)
test VIDIOC_S_FMT: OK (Not Supported)
test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
test Cropping: OK (Not Supported)
test Composing: OK (Not Supported)
test Scaling: OK (Not Supported)
Codec ioctls:
test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
test VIDIOC_G_ENC_INDEX: OK (Not Supported)
test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
Buffer ioctls:
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK (Not Supported)
test CREATE_BUFS maximum buffers: OK
test VIDIOC_REMOVE_BUFS: OK
test VIDIOC_EXPBUF: OK (Not Supported)
test Requests: OK (Not Supported)
Total for device /dev/v4l-subdev9: 47, Succeeded: 47, Failed: 0, Warnings: 0
Signed-off-by: Guoniu Zhou <guoniu.zhou@nxp.com>
---
Changes in v11:
- Drop [PATCH v10 1/4] dt-bindings: clock: imx95-blk-ctl: Use single quotes consistently (Krzysztof)
- Move properties to top-level and use if:then:else (Krzysztof/Frank)
- Link to v10: https://lore.kernel.org/r/20260618-csi_formatter-v10-0-f23830312ba5@oss.nxp.com
Changes in v10:
- Rebase to latest media/next
- [NEW PATCH] Use single quotes consistently (Krzysztof Kozlowski)
- Drop syscon parent node from example
- Drop Reviewed-by tags from Frank and Krzysztof due to binding changes,
requesting re-review
- Add description for reg property
- Add space after formatter@20 before opening brace in example
- Enhance the port description with more detailed information
- Delete the blank line immediately following the endpoint in example
- Use single quotes for regex pattern to be consistent (Krzysztof Kozlowski)
- Add formatter subnode binding and camera-csr syscon example
- Update commit title and message
- Use u8 for vc in csi_formatter_get_vc() and drop vc < 0 check
- Add MFD_SYSCON dependency to Kconfig
- Fix stream/VC mapping potential mismatch in start/stop_stream functions
- Link to v9: https://lore.kernel.org/r/20260526-csi_formatter-v9-0-ca3d8c334c39@oss.nxp.com
Changes in v9:
- [NEW PATCH] Fix formatter as syscon child node issue
- Link to v8: https://lore.kernel.org/r/20260525-csi_formatter-v8-0-6b646231224b@oss.nxp.com
Changes in v8:
- Rebase to latest media/next
- Use standard port reference instead of video-interfaces.yaml
- Add parent syscon node in example to show device integration
- Remove fmt field and look up format from subdev state instead
- Unify function and structure naming to use csi_formatter_ prefix
- Remove misleading alignment comment from set_fmt function
- Optimize get_frame_desc to call once per start_stream
- Replace V4L2_FRAME_DESC_ENTRY_MAX with CSI_FORMATTER_VC_NUM in loops
- Remove redundant debug message in enable_streams
- Use MEDIA_PAD_FL_MUST_CONNECT flag instead of manual link check
- Link to v7: https://lore.kernel.org/r/20260518-csi_formatter-v7-0-562b750557e3@oss.nxp.com
Changes in v7:
- Change compatible to imx95-csi-formatter as IP is i.MX95 specific per Marco's suggestion
Link: https://lore.kernel.org/linux-media/20260511-csi_formatter-v6-0-01028e312e2b@oss.nxp.com/T/#mcd135b3de179b3cb69daa1fd6e0e8e27c85b3332
- Update references from imx9 to imx95 for consistency with dt-bindings
- Enable PM runtime before async registration
- Link to v6: https://lore.kernel.org/r/20260511-csi_formatter-v6-0-01028e312e2b@oss.nxp.com
Changes in v6:
- Rebase to latest media/next
- Update v4l2-compliace test
- Remove unused header includes
- Unify macro naming: VCx/VCX -> VC and parameter x -> vc
- Remove unused format field from csi_formatter struct
- Use compact initialization for formats array
- Make find_csi_format() return NULL instead of default format
- Use unsigned int for array index in find_csi_format()
- Add err_ prefix to error handling labels
- Add v4l2_subdev_cleanup() and reorder cleanup sequence
- Update enable_streams debug output format
- Rename VC_MAX to VC_NUM and fix boundary check
- Update CSI formatter Kconfig description
- Use v4l2_subdev_get_frame_desc_passthrough() helper
- Fix error paths in async registration and probe
- Add mutex to protect enabled_streams
- Switch to devm_pm_runtime_enable()
- Remove redundant num_routes check in set_routing
- Optimize get_index_by_dt() and add warning for unsupported type
- csi_formatter_start/stop_stream: Process all streams in mask
- Link to v5: https://lore.kernel.org/r/20260123-csi_formatter-v5-0-d5b803f867bf@nxp.com
Changes in v5:
- Remove CSI_FORMATTER_DRV_NAME macro since only use once.
- Remove sd->owner = THIS_MODULE;
- Simplify code by using DEFINE_RUNTIME_DEV_PM_OPS macro.
- Link to v4: https://lore.kernel.org/r/20260122-csi_formatter-v4-0-6f6fcad1c33a@nxp.com
Changes in v4:
- Rebase to latest media/next.
- Add comments to describe the index field in formatter_dt_to_index_map array.
- Link to v3: https://lore.kernel.org/r/20251219-csi_formatter-v3-0-8680d6d87091@nxp.com
Changes in v3:
- Rename nxp,imx9-csi-formatter.yaml to fsl,imx9-csi-formatter.yaml.
- Drop clock-names property.
- Drop macro IMX95_PD_CAMERA definition and use a constant directly.
[PATCH 1/2] media: dt-bindings: Add CSI Pixel Formatter DT bindings
- Remove the assignment driver.owner = THIS_MODULE.
- Assign struct fwnode_handle *ep __free(fwnode_handle) when definition.
- Update yaml file name for csi formatter in MAINTAINERS.
[PATCH 2/2] media: nxp: Add i.MX9 CSI pixel formatter v4l2 driver
- Link to v2: https://lore.kernel.org/r/20251217-csi_formatter-v2-0-62168af80210@nxp.com
Changes in v2:
- Delete "|" for description key.
- Add empty line between child node and property.
- Delete labels for endpoint of child nodes.
[PATCH 1/2] media: dt-bindings: Add CSI Pixel Formatter DT bindings
- Update commit message.
- Use the value defined by bellow macros directly since they are used only once.
#define CSI_FORMATTER_DEF_MBUS_CODE MEDIA_BUS_FMT_UYVY8_1X16
#define CSI_FORMATTER_DEF_PIX_WIDTH 1920U
#define CSI_FORMATTER_DEF_PIX_HEIGHT 1080U
#define CSI_FORMATTER_MAX_PIX_WIDTH 0xffff
#define CSI_FORMATTER_MAX_PIX_HEIGHT 0xffff
- Use macro pm_ptr() to fix build warning when CONFIG_PM is disabled.
- Finish route loop by break statement, instead of goto.
- Return dev_err_probe() when meet errors in probe() function instead of dev_err().
- Remove MODULE_ALIAS().
- Refine .enable(.dsable)_stream callback implementation, include bellow changes:
Add stream checking.
Fix potential pm runtime count unbalance issue.
Add stop stream error handling when enabling remote subdev stream.
- Use __free(fwnode_handle) to drop reference to a device node automatically.
[PATCH 2/2] media: nxp: Add i.MX9 CSI pixel formatter v4l2 driver
- Link to v1: https://lore.kernel.org/r/20251203-csi_formatter-v1-0-eb9e1147b49e@nxp.com
---
Guoniu Zhou (3):
media: dt-bindings: Add CSI Pixel Formatter DT bindings
dt-bindings: clock: imx95-blk-ctl: Define formatter child node schema
media: nxp: Add i.MX95 CSI pixel formatter v4l2 driver
.../bindings/clock/nxp,imx95-blk-ctl.yaml | 71 ++
.../bindings/media/fsl,imx95-csi-formatter.yaml | 88 +++
MAINTAINERS | 8 +
drivers/media/platform/nxp/Kconfig | 15 +
drivers/media/platform/nxp/Makefile | 1 +
drivers/media/platform/nxp/imx95-csi-formatter.c | 775 +++++++++++++++++++++
6 files changed, 958 insertions(+)
---
base-commit: 06cb687a5132fcffe624c0070576ab852ac6b568
change-id: 20251125-csi_formatter-e6d29316dce6
Best regards,
--
Guoniu Zhou <guoniu.zhou@oss.nxp.com>
^ permalink raw reply
* RE: [PATCH 1/2] dt-bindings: clock: Drop incorrect usage of double '::'
From: Alim Akhtar @ 2026-06-22 11:42 UTC (permalink / raw)
To: 'Krzysztof Kozlowski', 'Bjorn Andersson',
'Konrad Dybcio', 'Rob Herring',
'Krzysztof Kozlowski', 'Conor Dooley',
'Peter Griffin', 'Michael Turquette',
'Stephen Boyd', 'Brian Masney',
'Sylwester Nawrocki', 'Chanwoo Choi',
'Sam Protsenko', 'Rob Clark',
'Dmitry Baryshkov', 'Abhinav Kumar',
'Jessica Zhang', 'Sean Paul',
'Marijn Suijten', 'David Airlie',
'Simona Vetter', 'Maarten Lankhorst',
'Maxime Ripard', 'Thomas Zimmermann',
'Inki Dae', 'Seung-Woo Kim',
'Kyungmin Park', 'Andi Shyti',
'Georgi Djakov', 'Lee Jones',
'Pavel Machek', 'Hans Verkuil',
'Mauro Carvalho Chehab', 'Ulf Hansson',
'Peter Rosin', 'Vinod Koul',
'Neil Armstrong', 'Linus Walleij',
'Geert Uytterhoeven', 'Magnus Damm',
'Sebastian Reichel', 'Javier Martinez Canillas',
'Liam Girdwood', 'Mark Brown',
'Greg Kroah-Hartman', 'Jiri Slaby',
'Srinivas Kandagatla',
'Bartlomiej Zolnierkiewicz', 'Rafael J. Wysocki',
'Daniel Lezcano', 'Zhang Rui',
'Lukasz Luba', 'Jonathan Marek',
'Taniya Das', 'Robert Marko',
'Christian Marangi', 'Stephan Gerhold',
'Adam Skladowski', 'Sireesh Kodali',
'Barnabas Czeman', 'Imran Shaik',
'Sricharan Ramabadhran', 'Anusha Rao',
'Luo Jie', 'Tomasz Figa', 'Chanho Park',
'Sunyeal Hong', 'Shin Son',
'Krishna Manikandan', 'Jacek Anaszewski',
'Jaehoon Chung', 'Marek Szyprowski',
'Alina Yu', 'Andy Gross',
'Niklas Söderlund', 'Wesley Cheng',
linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel,
linux-samsung-soc, linux-clk, dri-devel, freedreno, linux-i2c,
linux-pm, linux-leds, linux-media, linux-mmc, linux-phy,
linux-gpio, linux-renesas-soc, linux-serial, linux-sound,
linux-usb, cpgs
In-Reply-To: <20260622101606.485961-3-krzysztof.kozlowski@oss.qualcomm.com>
> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Sent: Monday, June 22, 2026 3:46 PM
> To: Bjorn Andersson <andersson@kernel.org>; Konrad Dybcio
> <konradybcio@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof
> Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>;
> Peter Griffin <peter.griffin@linaro.org>; Alim Akhtar
> <alim.akhtar@samsung.com>; Michael Turquette
> <mturquette@baylibre.com>; Stephen Boyd <sboyd@kernel.org>; Brian
> Masney <bmasney@redhat.com>; Sylwester Nawrocki
[Snip]
> Documentation/devicetree/bindings/clock/qcom,videocc.yaml | 2 +-
> .../devicetree/bindings/clock/samsung,exynos5260-clock.yaml | 6 +++---
> .../devicetree/bindings/clock/samsung,exynos5410-clock.yaml | 2 +-
> .../devicetree/bindings/clock/samsung,exynos5433-clock.yaml | 2 +-
> .../devicetree/bindings/clock/samsung,exynos7-clock.yaml | 2 +-
> .../devicetree/bindings/clock/samsung,exynos850-clock.yaml | 2 +-
> .../bindings/clock/samsung,exynosautov9-clock.yaml | 2 +-
> .../bindings/clock/samsung,exynosautov920-clock.yaml | 2 +-
> .../devicetree/bindings/clock/samsung,s5pv210-clock.yaml | 2 +-
Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>
^ permalink raw reply
* RE: [PATCH 2/2] dt-bindings: Drop incorrect usage of double '::'
From: Alim Akhtar @ 2026-06-22 11:45 UTC (permalink / raw)
To: 'Krzysztof Kozlowski', 'Bjorn Andersson',
'Konrad Dybcio', 'Rob Herring',
'Krzysztof Kozlowski', 'Conor Dooley',
'Peter Griffin', 'Michael Turquette',
'Stephen Boyd', 'Brian Masney',
'Sylwester Nawrocki', 'Chanwoo Choi',
'Sam Protsenko', 'Rob Clark',
'Dmitry Baryshkov', 'Abhinav Kumar',
'Jessica Zhang', 'Sean Paul',
'Marijn Suijten', 'David Airlie',
'Simona Vetter', 'Maarten Lankhorst',
'Maxime Ripard', 'Thomas Zimmermann',
'Inki Dae', 'Seung-Woo Kim',
'Kyungmin Park', 'Andi Shyti',
'Georgi Djakov', 'Lee Jones',
'Pavel Machek', 'Hans Verkuil',
'Mauro Carvalho Chehab', 'Ulf Hansson',
'Peter Rosin', 'Vinod Koul',
'Neil Armstrong', 'Linus Walleij',
'Geert Uytterhoeven', 'Magnus Damm',
'Sebastian Reichel', 'Javier Martinez Canillas',
'Liam Girdwood', 'Mark Brown',
'Greg Kroah-Hartman', 'Jiri Slaby',
'Srinivas Kandagatla',
'Bartlomiej Zolnierkiewicz', 'Rafael J. Wysocki',
'Daniel Lezcano', 'Zhang Rui',
'Lukasz Luba', 'Jonathan Marek',
'Taniya Das', 'Robert Marko',
'Christian Marangi', 'Stephan Gerhold',
'Adam Skladowski', 'Sireesh Kodali',
'Barnabas Czeman', 'Imran Shaik',
'Sricharan Ramabadhran', 'Anusha Rao',
'Luo Jie', 'Tomasz Figa', 'Chanho Park',
'Sunyeal Hong', 'Shin Son',
'Krishna Manikandan', 'Jacek Anaszewski',
'Jaehoon Chung', 'Marek Szyprowski',
'Alina Yu', 'Andy Gross',
'Niklas Söderlund', 'Wesley Cheng',
linux-arm-msm, devicetree, linux-kernel, linux-arm-kernel,
linux-samsung-soc, linux-clk, dri-devel, freedreno, linux-i2c,
linux-pm, linux-leds, linux-media, linux-mmc, linux-phy,
linux-gpio, linux-renesas-soc, linux-serial, linux-sound,
linux-usb, cpgs
In-Reply-To: <20260622101606.485961-4-krzysztof.kozlowski@oss.qualcomm.com>
> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Sent: Monday, June 22, 2026 3:46 PM
> To: Bjorn Andersson <andersson@kernel.org>; Konrad Dybcio
> <konradybcio@kernel.org>; Rob Herring <robh@kernel.org>; Krzysztof
> Kozlowski <krzk+dt@kernel.org>; Conor Dooley <conor+dt@kernel.org>;
> Peter Griffin <peter.griffin@linaro.org>; Alim Akhtar
> <alim.akhtar@samsung.com>; Michael Turquette
> <mturquette@baylibre.com>; Stephen Boyd <sboyd@kernel.org>; Brian
> Masney <bmasney@redhat.com>; Sylwester Nawrocki
[Snip]
> soc@vger.kernel.org; linux-serial@vger.kernel.org; linux-
> sound@vger.kernel.org; linux-usb@vger.kernel.org
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Subject: [PATCH 2/2] dt-bindings: Drop incorrect usage of double '::'
>
> There is no use of double colon '::' in YAML. OTOH, the literal style block, e.g.
> using '|' treats all characters as content [1] therefore single use of ':' in
> descriptions is perfectly fine, whenever '|' is used.
>
> Cleanup existing code, so the confusing style won't be re-used in new
> contributions.
>
> Link: https://protect2.fireeye.com/v1/url?k=20b000b4-490b6806-20b18bfb-
> 905a08a8515a-b42887ea7482314e&q=1&e=9fffcc8f-6266-432d-a638-
> 208efe86c9d7&u=https%3A%2F%2Fyaml.org%2Fspec%2F1.2.2%2F%23literal-
> style [1]
> Signed-off-by: Krzysztof Kozlowski
> <krzysztof.kozlowski@oss.qualcomm.com>
>
For Samsung IPs related
Reviewed-by: Alim Akhtar <alim.akhtar@samsung.com>
^ permalink raw reply
* [PATCH V2 8/8] arm64: dts: imx8qxp-mek: Describe the PCIe M.2 Key E connector
From: Sherry Sun (OSS) @ 2026-06-23 3:07 UTC (permalink / raw)
To: robh, krzk+dt, conor+dt, Frank.Li, s.hauer, kernel, festevam,
amitkumar.karwar, neeraj.sanjaykale, marcel, luiz.dentz,
hongxing.zhu, l.stach, lpieralisi, kwilczynski, mani, bhelgaas,
brgl
Cc: imx, linux-pci, linux-arm-kernel, devicetree, linux-kernel,
linux-bluetooth, linux-pm, sherry.sun
In-Reply-To: <20260623030736.1421537-1-sherry.sun@oss.nxp.com>
From: Sherry Sun <sherry.sun@nxp.com>
The i.MX8QXP-MEK has the PCIe M.2 Mechanical Key E connector to connect
wireless connectivity cards over PCIe and UART interfaces. Hence,
describe the connector node and link it with the PCIe b Root Port and
LPUART1 nodes through graph port/endpoint.
The M.2 Key E connector is powered by a 3.3V fixed regulator
(reg_3v3) on board.
Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
arch/arm64/boot/dts/freescale/imx8qxp-mek.dts | 54 ++++++++++++++-----
1 file changed, 41 insertions(+), 13 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
index a9b967d0a9be..c9fe4034cc2d 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
+++ b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts
@@ -40,6 +40,37 @@ memory@80000000 {
reg = <0x00000000 0x80000000 0 0x40000000>;
};
+ m2-connector {
+ compatible = "pcie-m2-e-connector";
+ vpcie3v3-supply = <®_3v3>;
+ w-disable1-gpios = <&pca9557_a 2 GPIO_ACTIVE_LOW>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+ m2_e_pcie_ep: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&pcieb_port0_ep>;
+ };
+ };
+
+ port@3 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <3>;
+ m2_e_uart_ep: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&lpuart1_ep>;
+ };
+ };
+ };
+ };
+
reg_usdhc2_vmmc: usdhc2-vmmc {
compatible = "regulator-fixed";
regulator-name = "SD1_SPWR";
@@ -157,15 +188,6 @@ reg_3v3: regulator-3v3 {
regulator-max-microvolt = <3300000>;
};
- reg_pcieb: regulator-pcie {
- compatible = "regulator-fixed";
- regulator-max-microvolt = <3300000>;
- regulator-min-microvolt = <3300000>;
- regulator-name = "mpcie_3v3";
- gpio = <&pca9557_a 2 GPIO_ACTIVE_HIGH>;
- enable-active-high;
- };
-
reg_audio: regulator-audio {
compatible = "regulator-fixed";
regulator-max-microvolt = <3300000>;
@@ -696,8 +718,10 @@ &lpuart1 {
pinctrl-0 = <&pinctrl_lpuart1>;
status = "okay";
- bluetooth {
- compatible = "nxp,88w8987-bt";
+ port {
+ lpuart1_ep: endpoint {
+ remote-endpoint = <&m2_e_uart_ep>;
+ };
};
};
@@ -746,8 +770,12 @@ &pcie0_ep {
&pcieb_port0 {
reset-gpios = <&lsio_gpio4 0 GPIO_ACTIVE_LOW>;
- vpcie-supply = <®_pcieb>;
- vpcie3v3aux-supply = <®_pcieb>;
+
+ port {
+ pcieb_port0_ep: endpoint {
+ remote-endpoint = <&m2_e_pcie_ep>;
+ };
+ };
};
&scu_key {
--
2.50.1
^ permalink raw reply related
* [PATCH V2 7/8] arm64: dts: imx8qm-mek: Describe the PCIe M.2 Key E connector
From: Sherry Sun (OSS) @ 2026-06-23 3:07 UTC (permalink / raw)
To: robh, krzk+dt, conor+dt, Frank.Li, s.hauer, kernel, festevam,
amitkumar.karwar, neeraj.sanjaykale, marcel, luiz.dentz,
hongxing.zhu, l.stach, lpieralisi, kwilczynski, mani, bhelgaas,
brgl
Cc: imx, linux-pci, linux-arm-kernel, devicetree, linux-kernel,
linux-bluetooth, linux-pm, sherry.sun
In-Reply-To: <20260623030736.1421537-1-sherry.sun@oss.nxp.com>
From: Sherry Sun <sherry.sun@nxp.com>
The i.MX8QM-MEK has the PCIe M.2 Mechanical Key E connector to connect
wireless connectivity cards over PCIe and UART interfaces. Hence,
describe the connector node and link it with the PCIe a Root Port and
LPUART1 nodes through graph port/endpoint.
The M.2 Key E connector is powered by a 3.3V fixed regulator
(reg_3v3) on board.
Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
arch/arm64/boot/dts/freescale/imx8qm-mek.dts | 58 +++++++++++++++-----
1 file changed, 43 insertions(+), 15 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8qm-mek.dts b/arch/arm64/boot/dts/freescale/imx8qm-mek.dts
index 5e725ad8aef9..4c02592cfe14 100644
--- a/arch/arm64/boot/dts/freescale/imx8qm-mek.dts
+++ b/arch/arm64/boot/dts/freescale/imx8qm-mek.dts
@@ -32,6 +32,39 @@ memory@80000000 {
reg = <0x00000000 0x80000000 0 0x40000000>;
};
+ m2-connector {
+ compatible = "pcie-m2-e-connector";
+ pinctrl-0 = <&pinctrl_pciea_reg>;
+ pinctrl-names = "default";
+ vpcie3v3-supply = <®_3v3>;
+ w-disable1-gpios = <&lsio_gpio1 13 GPIO_ACTIVE_LOW>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+ m2_e_pcie_ep: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&pciea_port0_ep>;
+ };
+ };
+
+ port@3 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <3>;
+ m2_e_uart_ep: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&lpuart1_ep>;
+ };
+ };
+ };
+ };
+
xtal24m: clock-xtal24m {
compatible = "fixed-clock";
#clock-cells = <0>;
@@ -320,17 +353,6 @@ reg_can2_stby: regulator-can2-stby {
vin-supply = <®_can2_en>;
};
- reg_pciea: regulator-pcie {
- compatible = "regulator-fixed";
- pinctrl-0 = <&pinctrl_pciea_reg>;
- pinctrl-names = "default";
- regulator-max-microvolt = <3300000>;
- regulator-min-microvolt = <3300000>;
- regulator-name = "mpcie_3v3";
- gpio = <&lsio_gpio1 13 GPIO_ACTIVE_HIGH>;
- enable-active-high;
- };
-
reg_usb_otg1_vbus: regulator-usbotg1-vbus {
compatible = "regulator-fixed";
regulator-name = "usb_otg1_vbus";
@@ -718,8 +740,10 @@ &lpuart1 {
pinctrl-0 = <&pinctrl_lpuart1>;
status = "okay";
- bluetooth {
- compatible = "nxp,88w8987-bt";
+ port {
+ lpuart1_ep: endpoint {
+ remote-endpoint = <&m2_e_uart_ep>;
+ };
};
};
@@ -818,8 +842,12 @@ &pciea {
&pciea_port0 {
reset-gpios = <&lsio_gpio4 29 GPIO_ACTIVE_LOW>;
- vpcie-supply = <®_pciea>;
- vpcie3v3aux-supply = <®_pciea>;
+
+ port {
+ pciea_port0_ep: endpoint {
+ remote-endpoint = <&m2_e_pcie_ep>;
+ };
+ };
};
&pcieb {
--
2.50.1
^ permalink raw reply related
* [PATCH V2 6/8] arm64: dts: imx8dxl-evk: Describe the PCIe M.2 Key E connector
From: Sherry Sun (OSS) @ 2026-06-23 3:07 UTC (permalink / raw)
To: robh, krzk+dt, conor+dt, Frank.Li, s.hauer, kernel, festevam,
amitkumar.karwar, neeraj.sanjaykale, marcel, luiz.dentz,
hongxing.zhu, l.stach, lpieralisi, kwilczynski, mani, bhelgaas,
brgl
Cc: imx, linux-pci, linux-arm-kernel, devicetree, linux-kernel,
linux-bluetooth, linux-pm, sherry.sun
In-Reply-To: <20260623030736.1421537-1-sherry.sun@oss.nxp.com>
From: Sherry Sun <sherry.sun@nxp.com>
The i.MX8DXL-EVK has the PCIe M.2 Mechanical Key E connector to connect
wireless connectivity cards over PCIe and UART interfaces. Hence,
describe the connector node and link it with the PCIe b Root Port and
LPUART1 nodes through graph port/endpoint.
The M.2 Key E connector is powered by a 3.3V fixed regulator
(reg_audio_3v3), add a reg_3v3 label to avoid confusion.
Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
arch/arm64/boot/dts/freescale/imx8dxl-evk.dts | 56 ++++++++++++++-----
1 file changed, 42 insertions(+), 14 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8dxl-evk.dts b/arch/arm64/boot/dts/freescale/imx8dxl-evk.dts
index 1084164d1381..6afee1f1a9fc 100644
--- a/arch/arm64/boot/dts/freescale/imx8dxl-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8dxl-evk.dts
@@ -42,6 +42,37 @@ memory@80000000 {
reg = <0x00000000 0x80000000 0 0x40000000>;
};
+ m2-connector {
+ compatible = "pcie-m2-e-connector";
+ vpcie3v3-supply = <®_3v3>;
+ w-disable1-gpios = <&pca6416_1 13 GPIO_ACTIVE_LOW>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+ m2_e_pcie_ep: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&pcieb_port0_ep>;
+ };
+ };
+
+ port@3 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <3>;
+ m2_e_uart_ep: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&lpuart1_ep>;
+ };
+ };
+ };
+ };
+
reserved-memory {
#address-cells = <2>;
#size-cells = <2>;
@@ -182,15 +213,6 @@ mii_select: regulator-4 {
regulator-always-on;
};
- reg_pcieb: regulator-pcieb {
- compatible = "regulator-fixed";
- regulator-max-microvolt = <3300000>;
- regulator-min-microvolt = <3300000>;
- regulator-name = "reg_pcieb";
- gpio = <&pca6416_1 13 GPIO_ACTIVE_HIGH>;
- enable-active-high;
- };
-
reg_audio_5v: regulator-audio-pwr {
compatible = "regulator-fixed";
regulator-name = "audio-5v";
@@ -200,7 +222,7 @@ reg_audio_5v: regulator-audio-pwr {
regulator-boot-on;
};
- reg_audio_3v3: regulator-audio-3v3 {
+ reg_3v3: reg_audio_3v3: regulator-audio-3v3 {
compatible = "regulator-fixed";
regulator-name = "audio-3v3";
regulator-min-microvolt = <3300000>;
@@ -623,8 +645,10 @@ &lpuart1 {
pinctrl-0 = <&pinctrl_lpuart1>;
status = "okay";
- bluetooth {
- compatible = "nxp,88w8987-bt";
+ port {
+ lpuart1_ep: endpoint {
+ remote-endpoint = <&m2_e_uart_ep>;
+ };
};
};
@@ -690,8 +714,12 @@ &pcie0_ep {
&pcieb_port0 {
reset-gpios = <&lsio_gpio4 0 GPIO_ACTIVE_LOW>;
- vpcie-supply = <®_pcieb>;
- vpcie3v3aux-supply = <®_pcieb>;
+
+ port {
+ pcieb_port0_ep: endpoint {
+ remote-endpoint = <&m2_e_pcie_ep>;
+ };
+ };
};
&sai0 {
--
2.50.1
^ permalink raw reply related
* [PATCH V2 5/8] arm64: dts: imx95-19x19-evk: Describe the PCIe M.2 Key E connector
From: Sherry Sun (OSS) @ 2026-06-23 3:07 UTC (permalink / raw)
To: robh, krzk+dt, conor+dt, Frank.Li, s.hauer, kernel, festevam,
amitkumar.karwar, neeraj.sanjaykale, marcel, luiz.dentz,
hongxing.zhu, l.stach, lpieralisi, kwilczynski, mani, bhelgaas,
brgl
Cc: imx, linux-pci, linux-arm-kernel, devicetree, linux-kernel,
linux-bluetooth, linux-pm, sherry.sun
In-Reply-To: <20260623030736.1421537-1-sherry.sun@oss.nxp.com>
From: Sherry Sun <sherry.sun@nxp.com>
The i.MX95-19x19-EVK has the PCIe M.2 Mechanical Key E connector to
connect wireless connectivity cards over PCIe and UART interfaces. Hence,
describe the connector node and link it with the PCIe 0 Root Port and
LPUART5 nodes through graph port/endpoint.
Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
.../boot/dts/freescale/imx95-19x19-evk.dts | 55 ++++++++++++++-----
1 file changed, 41 insertions(+), 14 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts b/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts
index c08731dfb1ee..d2c0345f0d61 100644
--- a/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx95-19x19-evk.dts
@@ -57,6 +57,37 @@ memory@80000000 {
reg = <0x0 0x80000000 0 0x80000000>;
};
+ m2-connector {
+ compatible = "pcie-m2-e-connector";
+ vpcie3v3-supply = <®_m2_pwr>;
+ w-disable1-gpios = <&i2c7_pcal6524 6 GPIO_ACTIVE_LOW>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+ m2_e_pcie_ep: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&pcie0_port0_ep>;
+ };
+ };
+
+ port@3 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <3>;
+ m2_e_uart_ep: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&lpuart5_ep>;
+ };
+ };
+ };
+ };
+
fan0: pwm-fan {
compatible = "pwm-fan";
#cooling-cells = <2>;
@@ -145,16 +176,6 @@ reg_m2_pwr: regulator-m2-pwr {
startup-delay-us = <5000>;
};
- reg_pcie0: regulator-pcie {
- compatible = "regulator-fixed";
- regulator-name = "PCIE_WLAN_EN";
- regulator-min-microvolt = <3300000>;
- regulator-max-microvolt = <3300000>;
- vin-supply = <®_m2_pwr>;
- gpio = <&i2c7_pcal6524 6 GPIO_ACTIVE_HIGH>;
- enable-active-high;
- };
-
reg_slot_pwr: regulator-slot-pwr {
compatible = "regulator-fixed";
regulator-name = "PCIe slot-power";
@@ -477,8 +498,10 @@ &lpuart5 {
pinctrl-0 = <&pinctrl_uart5>;
status = "okay";
- bluetooth {
- compatible = "nxp,88w8987-bt";
+ port {
+ lpuart5_ep: endpoint {
+ remote-endpoint = <&m2_e_uart_ep>;
+ };
};
};
@@ -555,8 +578,12 @@ &pcie0_ep {
&pcie0_port0 {
reset-gpios = <&i2c7_pcal6524 5 GPIO_ACTIVE_LOW>;
- vpcie-supply = <®_pcie0>;
- vpcie3v3aux-supply = <®_pcie0>;
+
+ port {
+ pcie0_port0_ep: endpoint {
+ remote-endpoint = <&m2_e_pcie_ep>;
+ };
+ };
};
&pcie1 {
--
2.50.1
^ permalink raw reply related
* [PATCH V2 4/8] arm64: dts: imx8mq-evk: Describe the PCIe M.2 Key E connector
From: Sherry Sun (OSS) @ 2026-06-23 3:07 UTC (permalink / raw)
To: robh, krzk+dt, conor+dt, Frank.Li, s.hauer, kernel, festevam,
amitkumar.karwar, neeraj.sanjaykale, marcel, luiz.dentz,
hongxing.zhu, l.stach, lpieralisi, kwilczynski, mani, bhelgaas,
brgl
Cc: imx, linux-pci, linux-arm-kernel, devicetree, linux-kernel,
linux-bluetooth, linux-pm, sherry.sun
In-Reply-To: <20260623030736.1421537-1-sherry.sun@oss.nxp.com>
From: Sherry Sun <sherry.sun@nxp.com>
The i.MX8MQ-EVK has the PCIe M.2 Mechanical Key E connector to connect
wireless connectivity cards over PCIe and UART interfaces. Hence,
describe the connector node and link it with the PCIe 1 Root Port and
UART3 nodes through graph port/endpoint.
Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
arch/arm64/boot/dts/freescale/imx8mq-evk.dts | 44 ++++++++++++++++++--
1 file changed, 40 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
index 71504a0af87f..482e5203e879 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mq-evk.dts
@@ -21,6 +21,36 @@ memory@40000000 {
reg = <0x00000000 0x40000000 0 0xc0000000>;
};
+ m2-connector {
+ compatible = "pcie-m2-e-connector";
+ vpcie3v3-supply = <®_pcie1>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <0>;
+ m2_e_pcie_ep: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&pcie1_port0_ep>;
+ };
+ };
+
+ port@3 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ reg = <3>;
+ m2_e_uart_ep: endpoint@0 {
+ reg = <0>;
+ remote-endpoint = <&uart3_ep>;
+ };
+ };
+ };
+ };
+
pcie0_refclk: pcie0-refclk {
compatible = "fixed-clock";
#clock-cells = <0>;
@@ -420,8 +450,12 @@ &pcie1_ep {
&pcie1_port0 {
reset-gpios = <&gpio5 12 GPIO_ACTIVE_LOW>;
- vpcie-supply = <®_pcie1>;
- vpcie3v3aux-supply = <®_pcie1>;
+
+ port {
+ pcie1_port0_ep: endpoint {
+ remote-endpoint = <&m2_e_pcie_ep>;
+ };
+ };
};
&pgc_gpu {
@@ -506,8 +540,10 @@ &uart3 { /* BT */
uart-has-rtscts;
status = "okay";
- bluetooth {
- compatible = "nxp,88w8987-bt";
+ port {
+ uart3_ep: endpoint {
+ remote-endpoint = <&m2_e_uart_ep>;
+ };
};
};
--
2.50.1
^ permalink raw reply related
* [PATCH V2 3/8] Bluetooth: btnxpuart: Add M.2 Bluetooth device support using pwrseq
From: Sherry Sun (OSS) @ 2026-06-23 3:07 UTC (permalink / raw)
To: robh, krzk+dt, conor+dt, Frank.Li, s.hauer, kernel, festevam,
amitkumar.karwar, neeraj.sanjaykale, marcel, luiz.dentz,
hongxing.zhu, l.stach, lpieralisi, kwilczynski, mani, bhelgaas,
brgl
Cc: imx, linux-pci, linux-arm-kernel, devicetree, linux-kernel,
linux-bluetooth, linux-pm, sherry.sun
In-Reply-To: <20260623030736.1421537-1-sherry.sun@oss.nxp.com>
From: Sherry Sun <sherry.sun@nxp.com>
Power supply to the M.2 Bluetooth device attached to the host using M.2
connector is controlled using the 'uart' pwrseq device. So add support for
getting the pwrseq device if the OF graph link is present. Once obtained,
pwrseq_power_on() is called to power up the M.2 Bluetooth card. The power
sequencer descriptor is obtained via devm_pwrseq_get(), so the power-off
and cleanup are handled automatically when the device is unbound.
Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
drivers/bluetooth/btnxpuart.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
index e7036a48ce48..438ccaa2b56d 100644
--- a/drivers/bluetooth/btnxpuart.c
+++ b/drivers/bluetooth/btnxpuart.c
@@ -9,6 +9,8 @@
#include <linux/serdev.h>
#include <linux/of.h>
+#include <linux/of_graph.h>
+#include <linux/pwrseq/consumer.h>
#include <linux/skbuff.h>
#include <linux/unaligned.h>
#include <linux/firmware.h>
@@ -1866,6 +1868,18 @@ static int nxp_serdev_probe(struct serdev_device *serdev)
return err;
}
+ if (of_graph_is_present(dev_of_node(&serdev->ctrl->dev))) {
+ struct pwrseq_desc *pwrseq;
+
+ pwrseq = devm_pwrseq_get(&serdev->ctrl->dev, "uart");
+ if (IS_ERR(pwrseq))
+ return PTR_ERR(pwrseq);
+
+ err = pwrseq_power_on(pwrseq);
+ if (err)
+ return err;
+ }
+
/* Initialize and register HCI device */
hdev = hci_alloc_dev();
if (!hdev) {
--
2.50.1
^ permalink raw reply related
* [PATCH V2 2/8] power: sequencing: pcie-m2: Add PCI ID for NXP 88W9098 and AW693 Bluetooth
From: Sherry Sun (OSS) @ 2026-06-23 3:07 UTC (permalink / raw)
To: robh, krzk+dt, conor+dt, Frank.Li, s.hauer, kernel, festevam,
amitkumar.karwar, neeraj.sanjaykale, marcel, luiz.dentz,
hongxing.zhu, l.stach, lpieralisi, kwilczynski, mani, bhelgaas,
brgl
Cc: imx, linux-pci, linux-arm-kernel, devicetree, linux-kernel,
linux-bluetooth, linux-pm, sherry.sun
In-Reply-To: <20260623030736.1421537-1-sherry.sun@oss.nxp.com>
From: Sherry Sun <sherry.sun@nxp.com>
88W9098 is a NXP Wi-Fi/BT combo chip with PCI device ID 0x2b43 under
Marvell Extended vendor ID. AW693 is a NXP Wi-Fi/BT combo chip with
PCI device ID 0x3003 under NXP/Philips vendor ID.
Add both chips to pwrseq_m2_pci_ids[] so that the pwrseq-pcie-m2 driver
can create the Bluetooth serdev device when these cards are inserted into
a PCIe M.2 Key E connector.
Both chips use "nxp,88w8987-bt" as the serdev compatible string, which
is the entry point for the btnxpuart driver. The driver identifies the
actual chip variant at runtime via chip ID auto-detection and loads the
appropriate firmware accordingly.
Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
---
drivers/power/sequencing/pwrseq-pcie-m2.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/power/sequencing/pwrseq-pcie-m2.c b/drivers/power/sequencing/pwrseq-pcie-m2.c
index b5ed80d03953..e3ba9169144d 100644
--- a/drivers/power/sequencing/pwrseq-pcie-m2.c
+++ b/drivers/power/sequencing/pwrseq-pcie-m2.c
@@ -186,6 +186,10 @@ static int pwrseq_pcie_m2_match(struct pwrseq_device *pwrseq,
}
static const struct pci_device_id pwrseq_m2_pci_ids[] = {
+ { PCI_DEVICE(PCI_VENDOR_ID_MARVELL_EXT, 0x2b43),
+ .driver_data = (kernel_ulong_t)"nxp,88w8987-bt" },
+ { PCI_DEVICE(PCI_VENDOR_ID_PHILIPS, 0x3003),
+ .driver_data = (kernel_ulong_t)"nxp,88w8987-bt" },
{ PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x1103),
.driver_data = (kernel_ulong_t)"qcom,wcn6855-bt" },
{ PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x1107),
--
2.50.1
^ permalink raw reply related
* [PATCH V2 1/8] PCI: imx6: Add skip_pwrctrl_off flag support
From: Sherry Sun (OSS) @ 2026-06-23 3:07 UTC (permalink / raw)
To: robh, krzk+dt, conor+dt, Frank.Li, s.hauer, kernel, festevam,
amitkumar.karwar, neeraj.sanjaykale, marcel, luiz.dentz,
hongxing.zhu, l.stach, lpieralisi, kwilczynski, mani, bhelgaas,
brgl
Cc: imx, linux-pci, linux-arm-kernel, devicetree, linux-kernel,
linux-bluetooth, linux-pm, sherry.sun
In-Reply-To: <20260623030736.1421537-1-sherry.sun@oss.nxp.com>
From: Sherry Sun <sherry.sun@nxp.com>
Use dw_pcie_rp::skip_pwrctrl_off to avoid powering off devices during
suspend to preserve wakeup capability of the devices and also not to power
on the devices in the init path.
This allows controller power-off to be skipped when some devices(e.g. M.2
cards key E without auxiliary power) required to support PCIe L2 link state
and wake-up mechanisms.
Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
---
drivers/pci/controller/dwc/pci-imx6.c | 36 +++++++++++++++++----------
1 file changed, 23 insertions(+), 13 deletions(-)
diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 0fa716d1ed75..ff5a9565dbbf 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1382,16 +1382,20 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)
}
}
- ret = pci_pwrctrl_create_devices(dev);
- if (ret) {
- dev_err(dev, "failed to create pwrctrl devices\n");
- goto err_reg_disable;
+ if (!pci->suspended) {
+ ret = pci_pwrctrl_create_devices(dev);
+ if (ret) {
+ dev_err(dev, "failed to create pwrctrl devices\n");
+ goto err_reg_disable;
+ }
}
- ret = pci_pwrctrl_power_on_devices(dev);
- if (ret) {
- dev_err(dev, "failed to power on pwrctrl devices\n");
- goto err_pwrctrl_destroy;
+ if (!pp->skip_pwrctrl_off) {
+ ret = pci_pwrctrl_power_on_devices(dev);
+ if (ret) {
+ dev_err(dev, "failed to power on pwrctrl devices\n");
+ goto err_pwrctrl_destroy;
+ }
}
ret = imx_pcie_clk_enable(imx_pcie);
@@ -1460,9 +1464,10 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)
err_clk_disable:
imx_pcie_clk_disable(imx_pcie);
err_pwrctrl_power_off:
- pci_pwrctrl_power_off_devices(dev);
+ if (!pp->skip_pwrctrl_off)
+ pci_pwrctrl_power_off_devices(dev);
err_pwrctrl_destroy:
- if (ret != -EPROBE_DEFER)
+ if (ret != -EPROBE_DEFER && !pci->suspended)
pci_pwrctrl_destroy_devices(dev);
err_reg_disable:
if (imx_pcie->vpcie)
@@ -1482,7 +1487,8 @@ static void imx_pcie_host_exit(struct dw_pcie_rp *pp)
}
imx_pcie_clk_disable(imx_pcie);
- pci_pwrctrl_power_off_devices(pci->dev);
+ if (!pci->pp.skip_pwrctrl_off)
+ pci_pwrctrl_power_off_devices(pci->dev);
if (imx_pcie->vpcie)
regulator_disable(imx_pcie->vpcie);
}
@@ -1990,12 +1996,16 @@ static int imx_pcie_probe(struct platform_device *pdev)
static void imx_pcie_shutdown(struct platform_device *pdev)
{
struct imx_pcie *imx_pcie = platform_get_drvdata(pdev);
+ struct dw_pcie *pci = imx_pcie->pci;
+ struct dw_pcie_rp *pp = &pci->pp;
/* bring down link, so bootloader gets clean state in case of reboot */
imx_pcie_assert_core_reset(imx_pcie);
imx_pcie_assert_perst(imx_pcie, true);
- pci_pwrctrl_power_off_devices(&pdev->dev);
- pci_pwrctrl_destroy_devices(&pdev->dev);
+ if (!pp->skip_pwrctrl_off)
+ pci_pwrctrl_power_off_devices(&pdev->dev);
+ if (!pci->suspended)
+ pci_pwrctrl_destroy_devices(&pdev->dev);
}
static const struct imx_pcie_drvdata drvdata[] = {
--
2.50.1
^ permalink raw reply related
* [PATCH V2 0/8] Add PCIe M.2 Key E connector support for NXP i.MX boards
From: Sherry Sun (OSS) @ 2026-06-23 3:07 UTC (permalink / raw)
To: robh, krzk+dt, conor+dt, Frank.Li, s.hauer, kernel, festevam,
amitkumar.karwar, neeraj.sanjaykale, marcel, luiz.dentz,
hongxing.zhu, l.stach, lpieralisi, kwilczynski, mani, bhelgaas,
brgl
Cc: imx, linux-pci, linux-arm-kernel, devicetree, linux-kernel,
linux-bluetooth, linux-pm, sherry.sun
From: Sherry Sun <sherry.sun@nxp.com>
This series adds support for NXP Wi-Fi/BT combo chips (88W9098, AW693)
inserted into PCIe M.2 Key E connectors on several i.MX EVK/MEK boards.
For M.2 cards that rely on PCIe L2 link state and wake-up mechanisms, the
card must remain powered during suspend. Patch #1 uses the existing
dw_pcie_rp::skip_pwrctrl_off flag to skip power-off during suspend and skip
power-on during the init path.
Also the btnxpuart driver is extended to obtain a pwrseq descriptor via the
OF graph on the UART controller device in patch #3.
Note: Patch #4-8 in this patch set depends on the following [1] and [2]
DTS patches.
[1] https://lore.kernel.org/all/20260616105201.3214395-1-sherry.sun@oss.nxp.com/
[2] https://lore.kernel.org/all/20260520084904.2424253-1-sherry.sun@oss.nxp.com/
---
Changes in V2:
1. Rebased on top of 7.1.0.
2. Removed the power_off error handling in patch #3 because pwrseq_put()
would call pwrseq_power_off() to automatically clean.
3. Collected the tags.
---
Sherry Sun (8):
PCI: imx6: Add skip_pwrctrl_off flag support
power: sequencing: pcie-m2: Add PCI ID for NXP 88W9098 and AW693
Bluetooth
Bluetooth: btnxpuart: Add M.2 Bluetooth device support using pwrseq
arm64: dts: imx8mq-evk: Describe the PCIe M.2 Key E connector
arm64: dts: imx95-19x19-evk: Describe the PCIe M.2 Key E connector
arm64: dts: imx8dxl-evk: Describe the PCIe M.2 Key E connector
arm64: dts: imx8qm-mek: Describe the PCIe M.2 Key E connector
arm64: dts: imx8qxp-mek: Describe the PCIe M.2 Key E connector
arch/arm64/boot/dts/freescale/imx8dxl-evk.dts | 56 +++++++++++++-----
arch/arm64/boot/dts/freescale/imx8mq-evk.dts | 44 ++++++++++++--
arch/arm64/boot/dts/freescale/imx8qm-mek.dts | 58 ++++++++++++++-----
arch/arm64/boot/dts/freescale/imx8qxp-mek.dts | 54 ++++++++++++-----
.../boot/dts/freescale/imx95-19x19-evk.dts | 55 +++++++++++++-----
drivers/bluetooth/btnxpuart.c | 14 +++++
drivers/pci/controller/dwc/pci-imx6.c | 36 +++++++-----
drivers/power/sequencing/pwrseq-pcie-m2.c | 4 ++
8 files changed, 248 insertions(+), 73 deletions(-)
--
2.50.1
^ permalink raw reply
* RE: [PATCH v33 4/5] i2c: ast2600: Add controller driver for AST2600 new register set
From: Ryan Chen @ 2026-06-23 3:01 UTC (permalink / raw)
To: sashiko-reviews@lists.linux.dev
Cc: robh@kernel.org, conor+dt@kernel.org, devicetree@vger.kernel.org
In-Reply-To: <20260623012848.6BA8D1F000E9@smtp.kernel.org>
> Subject: Re: [PATCH v33 4/5] i2c: ast2600: Add controller driver for AST2600
> new register set
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to
> consider:
> - [Medium] The AST2600_I2CM_BUS_RECOVER_FAIL interrupt is handled in
> the ISR but never enabled in the Interrupt Enable Register (IER), causing bus
> recovery failures to time out.
> --
This is a false positive; no change is needed (and the suggested change is
not possible in hardware).
I2CM10 (IER) bit 15 is read-only/Reserved on the AST2600 — there is no
interrupt-enable bit for BUS_RECOVER_FAIL, so it cannot be enabled in the
IER. Writing it there is a no-op. (This was already addressed in v32:
BUS_RECOVER_FAIL was removed from the IER writes for exactly this reason.)
The BUS_RECOVER_FAIL handler is still reached, and recovery failure is
reported immediately rather than via the full completion timeout. The
recovery-complete interrupt is enabled by BUS_RECOVER (bit 13), which is
already present in every IER write (PKT_DONE | BUS_RECOVER). The hardware
raises the interrupt when the RECOVER_CMD_EN command finishes and reports
the outcome in the ISR:
success -> ISR bit 13 (BUS_RECOVER) -> cmd_err = 0, complete()
failure -> ISR bit 15 (BUS_RECOVER_FAIL) -> cmd_err = -EPROTO, complete()
Both outcomes call complete() and wake ast2600_i2c_recover_bus() out of
wait_for_completion_timeout(), so a failed recovery returns -EPROTO
immediately and does not wait out the timeout. BUS_RECOVER_FAIL (bit 15)
and BUS_RECOVER (bit 13) are also mutually exclusive in the ISR, so the
two branches never race.
>
> commit d7e183b301ba820c2bd844ca60f5af7e37834e10
> Author: Ryan Chen <ryan_chen@aspeedtech.com>
>
> i2c: ast2600: Add controller driver for AST2600 new register set
>
> This commit adds a new I2C controller driver for the ASPEED AST2600,
> supporting its new register layout. The new capabilities include packet-based
> transfers, dual pool buffer mode, and hardware-assisted bus recovery.
>
> > diff --git a/drivers/i2c/busses/i2c-ast2600.c
> > b/drivers/i2c/busses/i2c-ast2600.c
> > new file mode 100644
> > --- /dev/null
> > +++ b/drivers/i2c/busses/i2c-ast2600.c
>
> [ ... ]
>
> > +static int ast2600_i2c_controller_irq(struct ast2600_i2c_bus
> > +*i2c_bus) {
> > + u32 sts = readl(i2c_bus->reg_base + AST2600_I2CM_ISR);
> > + u32 ctrl;
> > +
> > + sts &= ~AST2600_I2CM_SMBUS_ALERT;
> > +
> > + if (sts & AST2600_I2CM_BUS_RECOVER_FAIL) {
> > + writel(AST2600_I2CM_BUS_RECOVER_FAIL, i2c_bus->reg_base +
> AST2600_I2CM_ISR);
> > + ctrl = readl(i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
> > + writel(0, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
> > + writel(ctrl, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
> > + WRITE_ONCE(i2c_bus->msgs, NULL);
> > + i2c_bus->cmd_err = -EPROTO;
> > + complete(&i2c_bus->cmd_complete);
> > + return 1;
> > + }
>
> [Severity: Medium]
> Since the AST2600_I2CM_BUS_RECOVER_FAIL bit is never enabled in the
> Interrupt Enable Register (IER), will this error handling block ever execute?
>
> [ ... ]
>
> > +static int ast2600_i2c_controller_xfer(struct i2c_adapter *adap,
> > +struct i2c_msg *msgs, int num) {
>
> [ ... ]
>
> > + timeout = wait_for_completion_timeout(&i2c_bus->cmd_complete,
> i2c_bus->adap.timeout);
> > + if (timeout == 0) {
> > + u32 ctrl = readl(i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
> > +
> > + dev_dbg(i2c_bus->dev, "timeout isr[%x], sts[%x]\n",
> > + readl(i2c_bus->reg_base + AST2600_I2CM_ISR),
> > + readl(i2c_bus->reg_base + AST2600_I2CC_STS_AND_BUFF));
> > +
> > + writel(0, i2c_bus->reg_base + AST2600_I2CM_IER);
> > + synchronize_irq(i2c_bus->irq);
> > + writel(readl(i2c_bus->reg_base + AST2600_I2CM_ISR),
> > + i2c_bus->reg_base + AST2600_I2CM_ISR);
> > +
> > + writel(ctrl & ~AST2600_I2CC_MASTER_EN, i2c_bus->reg_base +
> AST2600_I2CC_FUN_CTRL);
> > + writel(ctrl, i2c_bus->reg_base + AST2600_I2CC_FUN_CTRL);
> > +
> > + WRITE_ONCE(i2c_bus->msgs, NULL);
> > + writel(AST2600_I2CM_PKT_DONE | AST2600_I2CM_BUS_RECOVER,
> > + i2c_bus->reg_base + AST2600_I2CM_IER);
>
> [Severity: Medium]
> Should this mask include AST2600_I2CM_BUS_RECOVER_FAIL?
>
> If hardware attempts bus recovery and fails, it sets the BUS_RECOVER_FAIL bit
> in the ISR. However, because it isn't enabled in the IER, the CPU won't get the
> interrupt and will instead wait out the full completion timeout in
> ast2600_i2c_recover_bus() rather than failing immediately.
>
> [ ... ]
>
> > +static int ast2600_i2c_probe(struct platform_device *pdev) {
>
> [ ... ]
>
> > + ret = devm_request_irq(dev, i2c_bus->irq, ast2600_i2c_bus_irq, 0,
> > + dev_name(dev), i2c_bus);
> > + if (ret < 0) {
> > + ret = dev_err_probe(dev, ret, "Unable to request irq %d\n",
> > + i2c_bus->irq);
> > + goto err;
> > + }
> > +
> > + writel(AST2600_I2CM_PKT_DONE | AST2600_I2CM_BUS_RECOVER,
> > + i2c_bus->reg_base + AST2600_I2CM_IER);
>
> [Severity: Medium]
> Similarly here in ast2600_i2c_probe(), does the initial IER configuration need
> to enable AST2600_I2CM_BUS_RECOVER_FAIL to ensure instant recovery
> failures?
>
> --
> Sashiko AI review ·
> https://sashiko.dev/#/patchset/20260623-upstream_i2c-v33-0-6d5338fc56ed@
> aspeedtech.com?part=4
^ permalink raw reply
* [PATCH v5 1/2] dt-bindings: clock: Add Amlogic A9 AO clock controller
From: Jian Hu via B4 Relay @ 2026-06-23 2:55 UTC (permalink / raw)
To: Neil Armstrong, Jerome Brunet, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Xianwei Zhao,
Kevin Hilman, Martin Blumenstingl
Cc: linux-amlogic, linux-clk, devicetree, linux-kernel,
linux-arm-kernel, Jian Hu, Conor Dooley
In-Reply-To: <20260623-a9_aoclk-v5-0-c7cb1ff9ebf1@amlogic.com>
From: Jian Hu <jian.hu@amlogic.com>
Add the Always-On clock controller dt-bindings for the Amlogic A9
SoC family.
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Jian Hu <jian.hu@amlogic.com>
---
.../bindings/clock/amlogic,a9-aoclkc.yaml | 76 ++++++++++++++++++++++
include/dt-bindings/clock/amlogic,a9-aoclkc.h | 76 ++++++++++++++++++++++
2 files changed, 152 insertions(+)
diff --git a/Documentation/devicetree/bindings/clock/amlogic,a9-aoclkc.yaml b/Documentation/devicetree/bindings/clock/amlogic,a9-aoclkc.yaml
new file mode 100644
index 000000000000..1fa9b3a32fbb
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/amlogic,a9-aoclkc.yaml
@@ -0,0 +1,76 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2026 Amlogic, Inc. All rights reserved
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/clock/amlogic,a9-aoclkc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Amlogic A9 Series Always-On Clock Controller
+
+maintainers:
+ - Neil Armstrong <neil.armstrong@linaro.org>
+ - Jerome Brunet <jbrunet@baylibre.com>
+ - Jian Hu <jian.hu@amlogic.com>
+ - Xianwei Zhao <xianwei.zhao@amlogic.com>
+
+properties:
+ compatible:
+ const: amlogic,a9-aoclkc
+
+ reg:
+ maxItems: 1
+
+ '#clock-cells':
+ const: 1
+
+ clocks:
+ minItems: 5
+ items:
+ - description: input oscillator
+ - description: input fclk div 3
+ - description: input fclk div 4
+ - description: input fclk div 5
+ - description: input sys clk
+ - description: external fixed 32k (optional)
+
+ clock-names:
+ minItems: 5
+ items:
+ - const: xtal
+ - const: fdiv3
+ - const: fdiv4
+ - const: fdiv5
+ - const: sys
+ - const: ext_32k
+
+required:
+ - compatible
+ - reg
+ - '#clock-cells'
+ - clocks
+ - clock-names
+
+additionalProperties: false
+
+examples:
+ - |
+ soc {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ clock-controller@0 {
+ compatible = "amlogic,a9-aoclkc";
+ reg = <0x0 0x0 0x0 0x58>;
+ #clock-cells = <1>;
+ clocks = <&xtal>,
+ <&scmi_clk 14>,
+ <&scmi_clk 16>,
+ <&scmi_clk 18>,
+ <&scmi_clk 21>;
+ clock-names = "xtal",
+ "fdiv3",
+ "fdiv4",
+ "fdiv5",
+ "sys";
+ };
+ };
diff --git a/include/dt-bindings/clock/amlogic,a9-aoclkc.h b/include/dt-bindings/clock/amlogic,a9-aoclkc.h
new file mode 100644
index 000000000000..a7d704d4b58e
--- /dev/null
+++ b/include/dt-bindings/clock/amlogic,a9-aoclkc.h
@@ -0,0 +1,76 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (C) 2026 Amlogic, Inc. All rights reserved.
+ */
+
+#ifndef __AMLOGIC_A9_AO_CLKC_H
+#define __AMLOGIC_A9_AO_CLKC_H
+
+#define CLKID_AO_XTAL_IN 0
+#define CLKID_AO_XTAL 1
+#define CLKID_AO_SYS 2
+#define CLKID_AO_SYS_I3C 3
+#define CLKID_AO_SYS_RTC_REG 4
+#define CLKID_AO_SYS_CLKTREE 5
+#define CLKID_AO_SYS_RST_CTRL 6
+#define CLKID_AO_SYS_PAD 7
+#define CLKID_AO_SYS_RTC_DIG 8
+#define CLKID_AO_SYS_IRQ 9
+#define CLKID_AO_SYS_PWRCTRL 10
+#define CLKID_AO_SYS_PWM_A 11
+#define CLKID_AO_SYS_PWM_B 12
+#define CLKID_AO_SYS_PWM_C 13
+#define CLKID_AO_SYS_PWM_D 14
+#define CLKID_AO_SYS_PWM_E 15
+#define CLKID_AO_SYS_PWM_F 16
+#define CLKID_AO_SYS_PWM_G 17
+#define CLKID_AO_SYS_I2C_A 18
+#define CLKID_AO_SYS_I2C_B 19
+#define CLKID_AO_SYS_I2C_C 20
+#define CLKID_AO_SYS_I2C_D 21
+#define CLKID_AO_SYS_SED 22
+#define CLKID_AO_SYS_IR_CTRL 23
+#define CLKID_AO_SYS_UART_B 24
+#define CLKID_AO_SYS_UART_C 25
+#define CLKID_AO_SYS_UART_D 26
+#define CLKID_AO_SYS_UART_E 27
+#define CLKID_AO_SYS_SPISG_0 28
+#define CLKID_AO_SYS_RTC_SECURE 29
+#define CLKID_AO_SYS_CEC 30
+#define CLKID_AO_SYS_AOCPU 31
+#define CLKID_AO_SYS_SRAM 32
+#define CLKID_AO_SYS_SPISG_1 33
+#define CLKID_AO_SYS_SPISG_2 34
+#define CLKID_AO_PWM_A_SEL 35
+#define CLKID_AO_PWM_A_DIV 36
+#define CLKID_AO_PWM_A 37
+#define CLKID_AO_PWM_B_SEL 38
+#define CLKID_AO_PWM_B_DIV 39
+#define CLKID_AO_PWM_B 40
+#define CLKID_AO_PWM_C_SEL 41
+#define CLKID_AO_PWM_C_DIV 42
+#define CLKID_AO_PWM_C 43
+#define CLKID_AO_PWM_D_SEL 44
+#define CLKID_AO_PWM_D_DIV 45
+#define CLKID_AO_PWM_D 46
+#define CLKID_AO_PWM_E_SEL 47
+#define CLKID_AO_PWM_E_DIV 48
+#define CLKID_AO_PWM_E 49
+#define CLKID_AO_PWM_F_SEL 50
+#define CLKID_AO_PWM_F_DIV 51
+#define CLKID_AO_PWM_F 52
+#define CLKID_AO_PWM_G_SEL 53
+#define CLKID_AO_PWM_G_DIV 54
+#define CLKID_AO_PWM_G 55
+#define CLKID_AO_RTC_DUALDIV_IN 56
+#define CLKID_AO_RTC_DUALDIV_DIV 57
+#define CLKID_AO_RTC_DUALDIV_SEL 58
+#define CLKID_AO_RTC_DUALDIV 59
+#define CLKID_AO_RTC 60
+#define CLKID_AO_CEC_DUALDIV_IN 61
+#define CLKID_AO_CEC_DUALDIV_DIV 62
+#define CLKID_AO_CEC_DUALDIV_SEL 63
+#define CLKID_AO_CEC_DUALDIV 64
+#define CLKID_AO_CEC 65
+
+#endif /* __AMLOGIC_A9_AO_CLKC_H */
--
2.47.1
^ permalink raw reply related
* [PATCH v5 2/2] clk: amlogic: Add A9 AO clock controller driver
From: Jian Hu via B4 Relay @ 2026-06-23 2:55 UTC (permalink / raw)
To: Neil Armstrong, Jerome Brunet, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Xianwei Zhao,
Kevin Hilman, Martin Blumenstingl
Cc: linux-amlogic, linux-clk, devicetree, linux-kernel,
linux-arm-kernel, Jian Hu
In-Reply-To: <20260623-a9_aoclk-v5-0-c7cb1ff9ebf1@amlogic.com>
From: Jian Hu <jian.hu@amlogic.com>
Add the Always-on clock controller driver for the Amlogic A9 SoC family.
Signed-off-by: Jian Hu <jian.hu@amlogic.com>
---
drivers/clk/meson/Kconfig | 13 ++
drivers/clk/meson/Makefile | 1 +
drivers/clk/meson/a9-aoclk.c | 489 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 503 insertions(+)
diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
index cf8cf3f9e4ee..b71299898197 100644
--- a/drivers/clk/meson/Kconfig
+++ b/drivers/clk/meson/Kconfig
@@ -132,6 +132,19 @@ config COMMON_CLK_A1_PERIPHERALS
device, A1 SoC Family. Say Y if you want A1 Peripherals clock
controller to work.
+config COMMON_CLK_A9_AO
+ tristate "Amlogic A9 SoC AO clock controller support"
+ depends on ARM64 || COMPILE_TEST
+ default ARCH_MESON
+ select COMMON_CLK_MESON_REGMAP
+ select COMMON_CLK_MESON_CLKC_UTILS
+ select COMMON_CLK_MESON_DUALDIV
+ imply COMMON_CLK_SCMI
+ help
+ Support for the AO clock controller on Amlogic A311Y3 based
+ device, AKA A9.
+ Say Y if you want A9 AO clock controller to work.
+
config COMMON_CLK_C3_PLL
tristate "Amlogic C3 PLL clock controller"
depends on ARM64
diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index c6719694a242..f89d027c282c 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_COMMON_CLK_AXG) += axg.o axg-aoclk.o
obj-$(CONFIG_COMMON_CLK_AXG_AUDIO) += axg-audio.o
obj-$(CONFIG_COMMON_CLK_A1_PLL) += a1-pll.o
obj-$(CONFIG_COMMON_CLK_A1_PERIPHERALS) += a1-peripherals.o
+obj-$(CONFIG_COMMON_CLK_A9_AO) += a9-aoclk.o
obj-$(CONFIG_COMMON_CLK_C3_PLL) += c3-pll.o
obj-$(CONFIG_COMMON_CLK_C3_PERIPHERALS) += c3-peripherals.o
obj-$(CONFIG_COMMON_CLK_GXBB) += gxbb.o gxbb-aoclk.o
diff --git a/drivers/clk/meson/a9-aoclk.c b/drivers/clk/meson/a9-aoclk.c
new file mode 100644
index 000000000000..c82d4b1b5b44
--- /dev/null
+++ b/drivers/clk/meson/a9-aoclk.c
@@ -0,0 +1,489 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR MIT)
+/*
+ * Copyright (C) 2026 Amlogic, Inc. All rights reserved
+ */
+
+#include <dt-bindings/clock/amlogic,a9-aoclkc.h>
+#include <linux/clk-provider.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include "clk-regmap.h"
+#include "clk-dualdiv.h"
+#include "meson-clkc-utils.h"
+
+#define AO_OSCIN_CTRL 0x00
+#define AO_SYS_CLK0 0x04
+#define AO_PWM_CLK_A_CTRL 0x1c
+#define AO_PWM_CLK_B_CTRL 0x20
+#define AO_PWM_CLK_C_CTRL 0x24
+#define AO_PWM_CLK_D_CTRL 0x28
+#define AO_PWM_CLK_E_CTRL 0x2c
+#define AO_PWM_CLK_F_CTRL 0x30
+#define AO_PWM_CLK_G_CTRL 0x34
+#define AO_CEC_CTRL0 0x38
+#define AO_CEC_CTRL1 0x3c
+#define AO_RTC_BY_OSCIN_CTRL0 0x50
+#define AO_RTC_BY_OSCIN_CTRL1 0x54
+
+#define A9_COMP_SEL(_name, _reg, _shift, _mask, _pdata) \
+ MESON_COMP_SEL(a9_ao_, _name, _reg, _shift, _mask, _pdata, NULL, 0, 0)
+
+#define A9_COMP_DIV(_name, _reg, _shift, _width) \
+ MESON_COMP_DIV(a9_ao_, _name, _reg, _shift, _width, 0, CLK_SET_RATE_PARENT)
+
+#define A9_COMP_GATE(_name, _reg, _bit) \
+ MESON_COMP_GATE(a9_ao_, _name, _reg, _bit, CLK_SET_RATE_PARENT)
+
+static struct clk_regmap a9_ao_xtal_in = {
+ .data = &(struct clk_regmap_gate_data){
+ .offset = AO_OSCIN_CTRL,
+ .bit_idx = 3,
+ },
+ .hw.init = &(struct clk_init_data) {
+ .name = "ao_xtal_in",
+ .ops = &clk_regmap_gate_ops,
+ .parent_data = &(const struct clk_parent_data) {
+ .fw_name = "xtal",
+ },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_regmap a9_ao_xtal = {
+ .data = &(struct clk_regmap_mux_data) {
+ .offset = AO_OSCIN_CTRL,
+ .mask = 0x1,
+ .shift = 0,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "ao_xtal",
+ .ops = &clk_regmap_mux_ops,
+ /* ext_32k is from external PAD, do not automatically reparent */
+ .parent_data = (const struct clk_parent_data []) {
+ { .hw = &a9_ao_xtal_in.hw },
+ { .fw_name = "ext_32k", },
+ },
+ .num_parents = 2,
+ .flags = CLK_SET_RATE_NO_REPARENT,
+ },
+};
+
+static struct clk_regmap a9_ao_sys = {
+ .data = &(struct clk_regmap_mux_data) {
+ .offset = AO_OSCIN_CTRL,
+ .mask = 0x1,
+ .shift = 1,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "ao_sys",
+ .ops = &clk_regmap_mux_ops,
+ .parent_data = (const struct clk_parent_data []) {
+ { .hw = &a9_ao_xtal.hw },
+ { .fw_name = "sys", },
+ },
+ .num_parents = 2,
+ },
+};
+
+static const struct clk_parent_data a9_ao_pclk_parents = { .hw = &a9_ao_sys.hw };
+
+#define A9_AO_PCLK(_name, _bit, _flags) \
+ MESON_PCLK(a9_ao_sys_##_name, AO_SYS_CLK0, _bit, \
+ &a9_ao_pclk_parents, _flags)
+
+/*
+ * A9 integrates a low-power microprocessor (Always-on CPU: AOCPU). Some AO sys
+ * clocks control the AOCPU modules. Mark the AOCPU-related clocks with
+ * CLK_IS_CRITICAL to avoid them being disabled and impacting AOCPU functionality.
+ * AOCPU-related clocks list:
+ * - clktree
+ * - rst_ctrl
+ * - pad
+ * - irq
+ * - pwrctrl
+ * - aocpu
+ * - sram
+ */
+static A9_AO_PCLK(i3c, 0, 0);
+static A9_AO_PCLK(rtc_reg, 1, 0);
+static A9_AO_PCLK(clktree, 2, CLK_IS_CRITICAL);
+static A9_AO_PCLK(rst_ctrl, 3, CLK_IS_CRITICAL);
+static A9_AO_PCLK(pad, 4, CLK_IS_CRITICAL);
+static A9_AO_PCLK(rtc_dig, 5, 0);
+static A9_AO_PCLK(irq, 6, CLK_IS_CRITICAL);
+static A9_AO_PCLK(pwrctrl, 7, CLK_IS_CRITICAL);
+static A9_AO_PCLK(pwm_a, 8, 0);
+static A9_AO_PCLK(pwm_b, 9, 0);
+static A9_AO_PCLK(pwm_c, 10, 0);
+static A9_AO_PCLK(pwm_d, 11, 0);
+static A9_AO_PCLK(pwm_e, 12, 0);
+static A9_AO_PCLK(pwm_f, 13, 0);
+static A9_AO_PCLK(pwm_g, 14, 0);
+static A9_AO_PCLK(i2c_a, 15, 0);
+static A9_AO_PCLK(i2c_b, 16, 0);
+static A9_AO_PCLK(i2c_c, 17, 0);
+static A9_AO_PCLK(i2c_d, 18, 0);
+static A9_AO_PCLK(sed, 19, 0);
+static A9_AO_PCLK(ir_ctrl, 20, 0);
+static A9_AO_PCLK(uart_b, 21, 0);
+static A9_AO_PCLK(uart_c, 22, 0);
+static A9_AO_PCLK(uart_d, 23, 0);
+static A9_AO_PCLK(uart_e, 24, 0);
+static A9_AO_PCLK(spisg_0, 25, 0);
+static A9_AO_PCLK(rtc_secure, 26, 0);
+static A9_AO_PCLK(cec, 27, 0);
+static A9_AO_PCLK(aocpu, 28, CLK_IS_CRITICAL);
+static A9_AO_PCLK(sram, 29, CLK_IS_CRITICAL);
+static A9_AO_PCLK(spisg_1, 30, 0);
+static A9_AO_PCLK(spisg_2, 31, 0);
+
+static const struct clk_parent_data a9_ao_pwm_parents[] = {
+ { .hw = &a9_ao_xtal.hw },
+ { .fw_name = "fdiv5", },
+ { .fw_name = "fdiv4", },
+ { .fw_name = "fdiv3", }
+};
+
+static A9_COMP_SEL(pwm_a, AO_PWM_CLK_A_CTRL, 9, 0x7, a9_ao_pwm_parents);
+static A9_COMP_DIV(pwm_a, AO_PWM_CLK_A_CTRL, 0, 8);
+static A9_COMP_GATE(pwm_a, AO_PWM_CLK_A_CTRL, 8);
+
+static A9_COMP_SEL(pwm_b, AO_PWM_CLK_B_CTRL, 9, 0x7, a9_ao_pwm_parents);
+static A9_COMP_DIV(pwm_b, AO_PWM_CLK_B_CTRL, 0, 8);
+static A9_COMP_GATE(pwm_b, AO_PWM_CLK_B_CTRL, 8);
+
+static A9_COMP_SEL(pwm_c, AO_PWM_CLK_C_CTRL, 9, 0x7, a9_ao_pwm_parents);
+static A9_COMP_DIV(pwm_c, AO_PWM_CLK_C_CTRL, 0, 8);
+static A9_COMP_GATE(pwm_c, AO_PWM_CLK_C_CTRL, 8);
+
+static A9_COMP_SEL(pwm_d, AO_PWM_CLK_D_CTRL, 9, 0x7, a9_ao_pwm_parents);
+static A9_COMP_DIV(pwm_d, AO_PWM_CLK_D_CTRL, 0, 8);
+static A9_COMP_GATE(pwm_d, AO_PWM_CLK_D_CTRL, 8);
+
+static A9_COMP_SEL(pwm_e, AO_PWM_CLK_E_CTRL, 9, 0x7, a9_ao_pwm_parents);
+static A9_COMP_DIV(pwm_e, AO_PWM_CLK_E_CTRL, 0, 8);
+static A9_COMP_GATE(pwm_e, AO_PWM_CLK_E_CTRL, 8);
+
+static A9_COMP_SEL(pwm_f, AO_PWM_CLK_F_CTRL, 9, 0x7, a9_ao_pwm_parents);
+static A9_COMP_DIV(pwm_f, AO_PWM_CLK_F_CTRL, 0, 8);
+static A9_COMP_GATE(pwm_f, AO_PWM_CLK_F_CTRL, 8);
+
+static A9_COMP_SEL(pwm_g, AO_PWM_CLK_G_CTRL, 9, 0x7, a9_ao_pwm_parents);
+static A9_COMP_DIV(pwm_g, AO_PWM_CLK_G_CTRL, 0, 8);
+static A9_COMP_GATE(pwm_g, AO_PWM_CLK_G_CTRL, 8);
+
+static struct clk_regmap a9_ao_rtc_dualdiv_in = {
+ .data = &(struct clk_regmap_gate_data){
+ .offset = AO_RTC_BY_OSCIN_CTRL0,
+ .bit_idx = 31,
+ },
+ .hw.init = &(struct clk_init_data) {
+ .name = "ao_rtc_dualdiv_in",
+ .ops = &clk_regmap_gate_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &a9_ao_xtal.hw
+ },
+ .num_parents = 1,
+ },
+};
+
+static const struct meson_clk_dualdiv_param a9_ao_dualdiv_table[] = {
+ { 733, 732, 8, 11, 1 },
+ { /* sentinel */ }
+};
+
+static struct clk_regmap a9_ao_rtc_dualdiv_div = {
+ .data = &(struct meson_clk_dualdiv_data){
+ .n1 = {
+ .reg_off = AO_RTC_BY_OSCIN_CTRL0,
+ .shift = 0,
+ .width = 12,
+ },
+ .n2 = {
+ .reg_off = AO_RTC_BY_OSCIN_CTRL0,
+ .shift = 12,
+ .width = 12,
+ },
+ .m1 = {
+ .reg_off = AO_RTC_BY_OSCIN_CTRL1,
+ .shift = 0,
+ .width = 12,
+ },
+ .m2 = {
+ .reg_off = AO_RTC_BY_OSCIN_CTRL1,
+ .shift = 12,
+ .width = 12,
+ },
+ .dual = {
+ .reg_off = AO_RTC_BY_OSCIN_CTRL0,
+ .shift = 28,
+ .width = 1,
+ },
+ .table = a9_ao_dualdiv_table,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "a9_ao_rtc_dualdiv_div",
+ .ops = &meson_clk_dualdiv_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &a9_ao_rtc_dualdiv_in.hw
+ },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_regmap a9_ao_rtc_dualdiv_sel = {
+ .data = &(struct clk_regmap_mux_data) {
+ .offset = AO_RTC_BY_OSCIN_CTRL1,
+ .mask = 0x1,
+ .shift = 24,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "ao_rtc_dualdiv_sel",
+ .ops = &clk_regmap_mux_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &a9_ao_rtc_dualdiv_div.hw,
+ &a9_ao_rtc_dualdiv_in.hw,
+ },
+ .num_parents = 2,
+ .flags = CLK_SET_RATE_PARENT,
+ },
+};
+
+static struct clk_regmap a9_ao_rtc_dualdiv = {
+ .data = &(struct clk_regmap_gate_data){
+ .offset = AO_RTC_BY_OSCIN_CTRL0,
+ .bit_idx = 30,
+ },
+ .hw.init = &(struct clk_init_data) {
+ .name = "ao_rtc_dualdiv",
+ .ops = &clk_regmap_gate_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &a9_ao_rtc_dualdiv_sel.hw
+ },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ },
+};
+
+static struct clk_regmap a9_ao_rtc = {
+ .data = &(struct clk_regmap_mux_data) {
+ .offset = AO_RTC_BY_OSCIN_CTRL1,
+ .mask = 0x1,
+ .shift = 30,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "ao_rtc",
+ .ops = &clk_regmap_mux_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &a9_ao_xtal.hw,
+ &a9_ao_rtc_dualdiv.hw,
+ },
+ .num_parents = 2,
+ .flags = CLK_SET_RATE_PARENT,
+ },
+};
+
+static struct clk_regmap a9_ao_cec_dualdiv_in = {
+ .data = &(struct clk_regmap_gate_data){
+ .offset = AO_CEC_CTRL0,
+ .bit_idx = 31,
+ },
+ .hw.init = &(struct clk_init_data) {
+ .name = "ao_cec_dualdiv_in",
+ .ops = &clk_regmap_gate_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &a9_ao_xtal.hw
+ },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_regmap a9_ao_cec_dualdiv_div = {
+ .data = &(struct meson_clk_dualdiv_data){
+ .n1 = {
+ .reg_off = AO_CEC_CTRL0,
+ .shift = 0,
+ .width = 12,
+ },
+ .n2 = {
+ .reg_off = AO_CEC_CTRL0,
+ .shift = 12,
+ .width = 12,
+ },
+ .m1 = {
+ .reg_off = AO_CEC_CTRL1,
+ .shift = 0,
+ .width = 12,
+ },
+ .m2 = {
+ .reg_off = AO_CEC_CTRL1,
+ .shift = 12,
+ .width = 12,
+ },
+ .dual = {
+ .reg_off = AO_CEC_CTRL0,
+ .shift = 28,
+ .width = 1,
+ },
+ .table = a9_ao_dualdiv_table,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "ao_cec_dualdiv_div",
+ .ops = &meson_clk_dualdiv_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &a9_ao_cec_dualdiv_in.hw
+ },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_regmap a9_ao_cec_dualdiv_sel = {
+ .data = &(struct clk_regmap_mux_data) {
+ .offset = AO_CEC_CTRL1,
+ .mask = 0x1,
+ .shift = 24,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "ao_cec_dualdiv_sel",
+ .ops = &clk_regmap_mux_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &a9_ao_cec_dualdiv_div.hw,
+ &a9_ao_cec_dualdiv_in.hw,
+ },
+ .num_parents = 2,
+ .flags = CLK_SET_RATE_PARENT,
+ },
+};
+
+static struct clk_regmap a9_ao_cec_dualdiv = {
+ .data = &(struct clk_regmap_gate_data){
+ .offset = AO_CEC_CTRL0,
+ .bit_idx = 30,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "ao_cec_dualdiv",
+ .ops = &clk_regmap_gate_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &a9_ao_cec_dualdiv_sel.hw
+ },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ },
+};
+
+static struct clk_regmap a9_ao_cec = {
+ .data = &(struct clk_regmap_mux_data) {
+ .offset = AO_CEC_CTRL1,
+ .mask = 0x1,
+ .shift = 30,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "ao_cec",
+ .ops = &clk_regmap_mux_ops,
+ .parent_hws = (const struct clk_hw *[]) {
+ &a9_ao_cec_dualdiv.hw,
+ &a9_ao_rtc.hw,
+ },
+ .num_parents = 2,
+ .flags = CLK_SET_RATE_PARENT,
+ },
+};
+
+static struct clk_hw *a9_ao_hw_clks[] = {
+ [CLKID_AO_XTAL_IN] = &a9_ao_xtal_in.hw,
+ [CLKID_AO_XTAL] = &a9_ao_xtal.hw,
+ [CLKID_AO_SYS] = &a9_ao_sys.hw,
+ [CLKID_AO_SYS_I3C] = &a9_ao_sys_i3c.hw,
+ [CLKID_AO_SYS_RTC_REG] = &a9_ao_sys_rtc_reg.hw,
+ [CLKID_AO_SYS_CLKTREE] = &a9_ao_sys_clktree.hw,
+ [CLKID_AO_SYS_RST_CTRL] = &a9_ao_sys_rst_ctrl.hw,
+ [CLKID_AO_SYS_PAD] = &a9_ao_sys_pad.hw,
+ [CLKID_AO_SYS_RTC_DIG] = &a9_ao_sys_rtc_dig.hw,
+ [CLKID_AO_SYS_IRQ] = &a9_ao_sys_irq.hw,
+ [CLKID_AO_SYS_PWRCTRL] = &a9_ao_sys_pwrctrl.hw,
+ [CLKID_AO_SYS_PWM_A] = &a9_ao_sys_pwm_a.hw,
+ [CLKID_AO_SYS_PWM_B] = &a9_ao_sys_pwm_b.hw,
+ [CLKID_AO_SYS_PWM_C] = &a9_ao_sys_pwm_c.hw,
+ [CLKID_AO_SYS_PWM_D] = &a9_ao_sys_pwm_d.hw,
+ [CLKID_AO_SYS_PWM_E] = &a9_ao_sys_pwm_e.hw,
+ [CLKID_AO_SYS_PWM_F] = &a9_ao_sys_pwm_f.hw,
+ [CLKID_AO_SYS_PWM_G] = &a9_ao_sys_pwm_g.hw,
+ [CLKID_AO_SYS_I2C_A] = &a9_ao_sys_i2c_a.hw,
+ [CLKID_AO_SYS_I2C_B] = &a9_ao_sys_i2c_b.hw,
+ [CLKID_AO_SYS_I2C_C] = &a9_ao_sys_i2c_c.hw,
+ [CLKID_AO_SYS_I2C_D] = &a9_ao_sys_i2c_d.hw,
+ [CLKID_AO_SYS_SED] = &a9_ao_sys_sed.hw,
+ [CLKID_AO_SYS_IR_CTRL] = &a9_ao_sys_ir_ctrl.hw,
+ [CLKID_AO_SYS_UART_B] = &a9_ao_sys_uart_b.hw,
+ [CLKID_AO_SYS_UART_C] = &a9_ao_sys_uart_c.hw,
+ [CLKID_AO_SYS_UART_D] = &a9_ao_sys_uart_d.hw,
+ [CLKID_AO_SYS_UART_E] = &a9_ao_sys_uart_e.hw,
+ [CLKID_AO_SYS_SPISG_0] = &a9_ao_sys_spisg_0.hw,
+ [CLKID_AO_SYS_RTC_SECURE] = &a9_ao_sys_rtc_secure.hw,
+ [CLKID_AO_SYS_CEC] = &a9_ao_sys_cec.hw,
+ [CLKID_AO_SYS_AOCPU] = &a9_ao_sys_aocpu.hw,
+ [CLKID_AO_SYS_SRAM] = &a9_ao_sys_sram.hw,
+ [CLKID_AO_SYS_SPISG_1] = &a9_ao_sys_spisg_1.hw,
+ [CLKID_AO_SYS_SPISG_2] = &a9_ao_sys_spisg_2.hw,
+ [CLKID_AO_PWM_A_SEL] = &a9_ao_pwm_a_sel.hw,
+ [CLKID_AO_PWM_A_DIV] = &a9_ao_pwm_a_div.hw,
+ [CLKID_AO_PWM_A] = &a9_ao_pwm_a.hw,
+ [CLKID_AO_PWM_B_SEL] = &a9_ao_pwm_b_sel.hw,
+ [CLKID_AO_PWM_B_DIV] = &a9_ao_pwm_b_div.hw,
+ [CLKID_AO_PWM_B] = &a9_ao_pwm_b.hw,
+ [CLKID_AO_PWM_C_SEL] = &a9_ao_pwm_c_sel.hw,
+ [CLKID_AO_PWM_C_DIV] = &a9_ao_pwm_c_div.hw,
+ [CLKID_AO_PWM_C] = &a9_ao_pwm_c.hw,
+ [CLKID_AO_PWM_D_SEL] = &a9_ao_pwm_d_sel.hw,
+ [CLKID_AO_PWM_D_DIV] = &a9_ao_pwm_d_div.hw,
+ [CLKID_AO_PWM_D] = &a9_ao_pwm_d.hw,
+ [CLKID_AO_PWM_E_SEL] = &a9_ao_pwm_e_sel.hw,
+ [CLKID_AO_PWM_E_DIV] = &a9_ao_pwm_e_div.hw,
+ [CLKID_AO_PWM_E] = &a9_ao_pwm_e.hw,
+ [CLKID_AO_PWM_F_SEL] = &a9_ao_pwm_f_sel.hw,
+ [CLKID_AO_PWM_F_DIV] = &a9_ao_pwm_f_div.hw,
+ [CLKID_AO_PWM_F] = &a9_ao_pwm_f.hw,
+ [CLKID_AO_PWM_G_SEL] = &a9_ao_pwm_g_sel.hw,
+ [CLKID_AO_PWM_G_DIV] = &a9_ao_pwm_g_div.hw,
+ [CLKID_AO_PWM_G] = &a9_ao_pwm_g.hw,
+ [CLKID_AO_RTC_DUALDIV_IN] = &a9_ao_rtc_dualdiv_in.hw,
+ [CLKID_AO_RTC_DUALDIV_DIV] = &a9_ao_rtc_dualdiv_div.hw,
+ [CLKID_AO_RTC_DUALDIV_SEL] = &a9_ao_rtc_dualdiv_sel.hw,
+ [CLKID_AO_RTC_DUALDIV] = &a9_ao_rtc_dualdiv.hw,
+ [CLKID_AO_RTC] = &a9_ao_rtc.hw,
+ [CLKID_AO_CEC_DUALDIV_IN] = &a9_ao_cec_dualdiv_in.hw,
+ [CLKID_AO_CEC_DUALDIV_DIV] = &a9_ao_cec_dualdiv_div.hw,
+ [CLKID_AO_CEC_DUALDIV_SEL] = &a9_ao_cec_dualdiv_sel.hw,
+ [CLKID_AO_CEC_DUALDIV] = &a9_ao_cec_dualdiv.hw,
+ [CLKID_AO_CEC] = &a9_ao_cec.hw,
+};
+
+static const struct meson_clkc_data a9_ao_clkc_data = {
+ .hw_clks = {
+ .hws = a9_ao_hw_clks,
+ .num = ARRAY_SIZE(a9_ao_hw_clks),
+ },
+};
+
+static const struct of_device_id a9_ao_clkc_match_table[] = {
+ {
+ .compatible = "amlogic,a9-aoclkc",
+ .data = &a9_ao_clkc_data,
+ },
+ { }
+};
+MODULE_DEVICE_TABLE(of, a9_ao_clkc_match_table);
+
+static struct platform_driver a9_ao_clkc_driver = {
+ .probe = meson_clkc_mmio_probe,
+ .driver = {
+ .name = "a9-aoclkc",
+ .of_match_table = a9_ao_clkc_match_table,
+ },
+};
+module_platform_driver(a9_ao_clkc_driver);
+
+MODULE_DESCRIPTION("Amlogic A9 Always-ON Clock Controller driver");
+MODULE_AUTHOR("Jian Hu <jian.hu@amlogic.com>");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("CLK_MESON");
--
2.47.1
^ permalink raw reply related
* [PATCH v5 0/2] clk: amlogic: Add A9 AO clock controller
From: Jian Hu via B4 Relay @ 2026-06-23 2:55 UTC (permalink / raw)
To: Neil Armstrong, Jerome Brunet, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Xianwei Zhao,
Kevin Hilman, Martin Blumenstingl
Cc: linux-amlogic, linux-clk, devicetree, linux-kernel,
linux-arm-kernel, Jian Hu, Conor Dooley
This series adds Amlogic A9 AO clock support, including dt-binding and AO clock driver.
Signed-off-by: Jian Hu <jian.hu@amlogic.com>
---
Changes in v5:
- Include <linux/module.h>.
- Correct duandiv name.
- Link to v4: https://lore.kernel.org/r/20260618-a9_aoclk-v4-0-569d0425e50c@amlogic.com
Changes in v4:
- Drop CLK_IS_CRITICAL for ao_xtal_in clock.
- Drop CLK_HW_INIT* and revert to explicit clock declarations.
- Link to v3: https://lore.kernel.org/r/20260610-a9_aoclk-v3-0-b7592d6c31e2@amlogic.com
Changes in v3:
- Move COMPILE_TEST after 'depends on ARM64' reported by sashiko-bot.
- Rename i2c3 to i3c reported by sashiko-bot.
- Reword the comment describing ao_xtal_in's flags.
- Use struct clk_init_data to describe ao_xtal_in's hw.init.
- Link to v2: https://lore.kernel.org/r/20260603-a9_aoclk-v2-0-f47ea616ee78@amlogic.com
Changes in v2:
- Split the A9 clock driver and send the AO clock separately.
- Rename aobus to soc.
- Use CLK_HW_INIT_FW_NAME to describe clk_init_data.
- Use CLK_HW_INIT_PARENTS_DATA to describe clk_init_data.
- Use a9_ao prefix for MESON_COMP_SEL.
- Correct duandiv name.
- Fix pwm b reg.
- Link to v1: https://lore.kernel.org/all/20260511-b4-a9_clk-v1-0-41cb4071b7c9@amlogic.com/
---
Jian Hu (2):
dt-bindings: clock: Add Amlogic A9 AO clock controller
clk: amlogic: Add A9 AO clock controller driver
.../bindings/clock/amlogic,a9-aoclkc.yaml | 76 ++++
drivers/clk/meson/Kconfig | 13 +
drivers/clk/meson/Makefile | 1 +
drivers/clk/meson/a9-aoclk.c | 489 +++++++++++++++++++++
include/dt-bindings/clock/amlogic,a9-aoclkc.h | 76 ++++
5 files changed, 655 insertions(+)
---
base-commit: ca89c88bcf69daca829044c638a8163d5ce47af0
change-id: 20260603-a9_aoclk-bbf531badc63
Best regards,
--
Jian Hu <jian.hu@amlogic.com>
^ permalink raw reply
* RE: [PATCH v33 5/5] i2c: ast2600: Add target mode support
From: Ryan Chen @ 2026-06-23 2:55 UTC (permalink / raw)
To: sashiko-reviews@lists.linux.dev
Cc: devicetree@vger.kernel.org, conor+dt@kernel.org, robh@kernel.org
In-Reply-To: <20260623012822.B6AC01F00A3E@smtp.kernel.org>
> Subject: Re: [PATCH v33 5/5] i2c: ast2600: Add target mode support
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to
> consider:
> - [High] Coalesced STOP and SLAVE_MATCH interrupts incorrectly pass the
> previous transaction's buffered RX bytes to the new transaction.
> - [High] Unconditional enablement of target interrupts and unmasked checks
> of I2CS_ISR in `ast2600_i2c_controller_xfer()` causes unhandled IRQ storms
> and permanent -EBUSY deadlocks when no slave is registered.
> --
>
> commit 12f36e43a7bec16d6ebecb3d928d5bb09f47373e
> Author: Ryan Chen <ryan_chen@aspeedtech.com>
> Subject: i2c: ast2600: Add target mode support
>
> This commit adds target mode support to the AST2600 I2C driver.
> Features include target interrupt handling, address match, and response logic.
> This complements the existing controller-mode support to enable dual-role
> capability.
>
> > diff --git a/drivers/i2c/busses/i2c-ast2600.c
> > b/drivers/i2c/busses/i2c-ast2600.c
> > --- a/drivers/i2c/busses/i2c-ast2600.c
> > +++ b/drivers/i2c/busses/i2c-ast2600.c
>
> [ ... ]
>
> > @@ -348,6 +353,253 @@ static int ast2600_i2c_recover_bus(struct
> > ast2600_i2c_bus *i2c_bus)
> > + case AST2600_I2CS_SLAVE_PENDING | AST2600_I2CS_STOP |
> > + AST2600_I2CS_TX_NAK | AST2600_I2CS_SLAVE_MATCH |
> AST2600_I2CS_RX_DONE:
> > + case AST2600_I2CS_SLAVE_PENDING | AST2600_I2CS_WAIT_RX_DMA |
> AST2600_I2CS_STOP |
> > + AST2600_I2CS_TX_NAK | AST2600_I2CS_SLAVE_MATCH |
> AST2600_I2CS_RX_DONE:
> > + i2c_slave_event(i2c_bus->target, I2C_SLAVE_STOP, &value);
> > + i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_REQUESTED,
> &value);
> > + target_rx_len =
> AST2600_I2CC_GET_RX_BUF_LEN(readl(i2c_bus->reg_base +
> > + AST2600_I2CC_BUFF_CTRL));
> > + for (i = 0; i < target_rx_len; i++) {
> > + value = readb(i2c_bus->buf_base + i2c_bus->buf_size + i);
> > + i2c_slave_event(i2c_bus->target, I2C_SLAVE_WRITE_RECEIVED,
> &value);
> > + }
>
> [Severity: High]
> Does this incorrectly pass the previous transaction's buffered RX bytes to the
> new transaction?
>
> The handler in ast2600_i2c_target_packet_buff_irq() emits I2C_SLAVE_STOP
> and I2C_SLAVE_WRITE_REQUESTED for the new transaction before delivering
> the remaining I2C_SLAVE_WRITE_RECEIVED bytes from the previous
> transaction.
These cases carry TX_NAK in addition to SLAVE_PENDING | STOP | SLAVE_MATCH
| RX_DONE. In target mode TX_NAK means the target was transmitting and the
master NAKed the last byte — i.e. the previous transaction was a master
read from the target. A read produces no slave-RX data, so there are no
"previous transaction" bytes sitting in the RX buffer.
SLAVE_PENDING (I2CS24[29], read-only) means a previous slave-receive has
not been serviced yet while a new address match is already pending, so
this single interrupt spans two back-to-back transactions. The RX_DONE and
the buffered bytes therefore belong to the new write transaction. The
emitted order is correct:
I2C_SLAVE_STOP -> terminate the previous (read) transaction
I2C_SLAVE_WRITE_REQUESTED -> start the new write transaction
I2C_SLAVE_WRITE_RECEIVED -> the new transaction's data from the buffer
So no previous-transaction data is misattributed.
>
> [ ... ]
>
> > @@ -684,6 +956,21 @@ static int ast2600_i2c_controller_xfer(struct
> i2c_adapter *adap, struct i2c_msg
> > return ret;
> > }
> >
> > +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> > + if (READ_ONCE(i2c_bus->target_active))
> > + return -EBUSY;
> > + /*
> > + * Controller and target share the same buffer register. A target
> > + * transaction can update buffer state asynchronously via IRQ, so block
> > + * controller transfers while target is active to avoid buffer corruption.
> > + */
> > + writel(0, i2c_bus->reg_base + AST2600_I2CS_IER);
> > + if (readl(i2c_bus->reg_base + AST2600_I2CS_ISR) ||
> READ_ONCE(i2c_bus->target_active)) {
> > + writel(AST2600_I2CS_PKT_DONE, i2c_bus->reg_base +
> AST2600_I2CS_IER);
> > + return -EBUSY;
> > + }
> > +#endif
> > +
> > i2c_bus->cmd_err = 0;
> > i2c_bus->msgs = msgs;
> > i2c_bus->msgs_index = 0;
> > i2c_bus->msgs_count = num;
> > WRITE_ONCE(i2c_bus->stop_pending, false);
> > reinit_completion(&i2c_bus->cmd_complete);
> > ret = ast2600_i2c_do_start(i2c_bus);
> > +#if IS_ENABLED(CONFIG_I2C_SLAVE)
> > + /* avoid race condition target is wait and controller wait 1st target
> operate */
> > + writel(AST2600_I2CS_PKT_DONE, i2c_bus->reg_base +
> AST2600_I2CS_IER);
> > +#endif
>
> [Severity: High]
> Will this logic cause unhandled IRQ storms and permanent -EBUSY deadlocks
> when no slave is registered?
>
> When CONFIG_I2C_SLAVE is enabled but no slave is bound (i2c_bus->target is
> NULL), ast2600_i2c_controller_xfer() unconditionally writes to I2CS_IER to
> re-enable target interrupts. If a target hardware event occurs, it will trigger an
> interrupt that ast2600_i2c_bus_irq() ignores, leading to an IRQ storm.
>
> Additionally, because the interrupt is never handled and I2CS_ISR is never
> cleared, subsequent calls to ast2600_i2c_controller_xfer() will see a non-zero
> I2CS_ISR and permanently abort with -EBUSY.
>
The chain assumes a target hardware event can occur with no slave
registered, which cannot happen. Target events (address match, RX,
INACTIVE_TO, …) are gated by SLAVE_EN (FUN_CTRL bit 1), not by I2CS_IER.
SLAVE_EN is left clear by ast2600_i2c_init(), set only in reg_target(),
and cleared again in unreg_target(). With no slave bound SLAVE_EN == 0, so
the slave block never matches an address and never sets any I2CS_ISR bit.
ast2600_i2c_init() also clears I2CS_ISR at probe, and nothing sets it while
SLAVE_EN is clear. Enabling I2CS_IER in that state is therefore harmless:
I2CS_ISR stays zero, so there is no IRQ storm, and the I2CS_ISR interlock
never returns a spurious -EBUSY.
> --
> Sashiko AI review ·
> https://sashiko.dev/#/patchset/20260623-upstream_i2c-v33-0-6d5338fc56ed@
> aspeedtech.com?part=5
^ permalink raw reply
* Re: [PATCH V13 8/9] iio: imu: inv_icm42607: Add Gyroscope to icm42607
From: Chris Morgan @ 2026-06-23 2:37 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Chris Morgan, linux-iio, andy, nuno.sa, dlechner,
jean-baptiste.maneyrol, linux-rockchip, devicetree, heiko,
conor+dt, krzk+dt, robh, andriy.shevchenko
In-Reply-To: <20260621183408.032e18b4@jic23-huawei>
On Sun, Jun 21, 2026 at 06:34:08PM +0100, Jonathan Cameron wrote:
> On Mon, 15 Jun 2026 12:25:51 -0500
> Chris Morgan <macroalpha82@gmail.com> wrote:
>
> > From: Chris Morgan <macromorgan@hotmail.com>
> >
> > Add gyroscope functions to the icm42607 driver.
> >
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> There is another bit of sashiko stuff in here. Please
> take a look
> https://sashiko.dev/#/patchset/20260615172554.160910-1-macroalpha82%40gmail.com
>
> I think it is correct about there being a path in which the
> gyro ends up always enabled along side anything else after
> the first read.
>
> In general there seems to be a bit of mix on whether the caller
> or the power management function should be responsible for the caching
> of state.
>
> I know you look at Sashiko so I could just have waited a bit, but
> I was reviewing anyway so took a look and having done that might
> as well highlight some stuff!
>
> Jonathan
>
> > diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> > index eb239987a1ce..23ca7529825c 100644
> > --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> > +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> > @@ -218,6 +218,49 @@ int inv_icm42607_set_accel_conf(struct inv_icm42607_state *st,
> > st->conf.temp_en, sleep_ms);
> > }
> >
> > +int inv_icm42607_set_gyro_conf(struct inv_icm42607_state *st,
> > + struct inv_icm42607_sensor_conf *conf,
> > + unsigned int *sleep_ms)
> > +{
> > + struct inv_icm42607_sensor_conf *oldconf = &st->conf.gyro;
> > + unsigned int val;
> > + int ret;
> > +
> > + if (conf->mode < 0)
> > + conf->mode = oldconf->mode;
> > + if (conf->fs < 0)
> > + conf->fs = oldconf->fs;
> > + if (conf->odr < 0)
> > + conf->odr = oldconf->odr;
> > + if (conf->filter < 0)
> > + conf->filter = oldconf->filter;
> > +
> > + if (conf->fs != oldconf->fs || conf->odr != oldconf->odr) {
> > + val = FIELD_PREP(INV_ICM42607_GYRO_CONFIG0_FS_SEL_MASK,
> > + conf->fs);
> > + val |= FIELD_PREP(INV_ICM42607_GYRO_CONFIG0_ODR_MASK,
> > + conf->odr);
> > + ret = regmap_write(st->map, INV_ICM42607_REG_GYRO_CONFIG0, val);
> > + if (ret)
> > + return ret;
> > + oldconf->fs = conf->fs;
> > + oldconf->odr = conf->odr;
> > + }
> > +
> > + if (conf->filter != oldconf->filter) {
> > + val = FIELD_PREP(INV_ICM42607_GYRO_CONFIG1_FILTER_MASK,
> > + conf->filter);
> > + ret = regmap_update_bits(st->map, INV_ICM42607_REG_GYRO_CONFIG1,
> > + INV_ICM42607_GYRO_CONFIG1_FILTER_MASK, val);
> > + if (ret)
> > + return ret;
> > + oldconf->filter = conf->filter;
> > + }
> > +
> > + return inv_icm42607_set_pwr_mgmt0(st, conf->mode, st->conf.accel.mode,
> > + st->conf.temp_en, sleep_ms);
> > +}
>
> > diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c
> > new file mode 100644
> > index 000000000000..ef73560b39d7
> > --- /dev/null
> > +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c
>
> > +static int inv_icm42607_gyro_read_sensor(struct iio_dev *indio_dev,
> > + struct iio_chan_spec const *chan,
> > + s16 *val)
> > +{
> > + struct inv_icm42607_sensor_conf conf = INV_ICM42607_SENSOR_CONF_INIT;
> > + struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
> > + struct inv_icm42607_sensor_state *gyro_st = iio_priv(indio_dev);
> > + struct device *dev = regmap_get_device(st->map);
> > + unsigned int reg;
> > + u8 data[2];
> > + int ret;
> > +
> > + if (chan->type != IIO_ANGL_VEL)
> > + return -EINVAL;
> > +
> > + switch (chan->channel2) {
> > + case IIO_MOD_X:
> > + reg = INV_ICM42607_REG_GYRO_DATA_X1;
> > + break;
> > + case IIO_MOD_Y:
> > + reg = INV_ICM42607_REG_GYRO_DATA_Y1;
> > + break;
> > + case IIO_MOD_Z:
> > + reg = INV_ICM42607_REG_GYRO_DATA_Z1;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
> > + ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
> > + if (ret)
> > + return ret;
> > +
> > + guard(mutex)(&st->lock);
> > +
> > + /* enable gyro sensor */
> > + conf.mode = gyro_st->power_mode;
> > + conf.filter = gyro_st->filter;
> > + ret = inv_icm42607_set_gyro_conf(st, &conf, NULL);
> The sashiko report is basically:
>
> This turns it on, and runtime pm will turn it off but the
> state cached ends up such that it is turned on again for
> any runtime pm resume.
Yep, for this and for the accel sensor it looks like it gets stuck on.
I've revamped this code for the next round and so far it looks like the
sensor is powering down correctly.
>
> > + if (ret)
> > + return ret;
> > +
> > + /* read gyro register data */
> > + ret = regmap_bulk_read(st->map, reg, data, sizeof(data));
> > + if (ret)
> > + return ret;
> > +
> > + *val = get_unaligned_be16(data);
> > + if (*val == INV_ICM42607_DATA_INVALID)
> > + return -EINVAL;
> > +
> > + return 0;
> > +}
>
Thank you,
Chris
^ permalink raw reply
* Re: [PATCH V13 6/9] iio: imu: inv_icm42607: Add Temp Support in icm42607
From: Chris Morgan @ 2026-06-23 2:35 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Chris Morgan, linux-iio, andy, nuno.sa, dlechner,
jean-baptiste.maneyrol, linux-rockchip, devicetree, heiko,
conor+dt, krzk+dt, robh, andriy.shevchenko
In-Reply-To: <20260621182612.1a19278a@jic23-huawei>
On Sun, Jun 21, 2026 at 06:26:12PM +0100, Jonathan Cameron wrote:
> On Mon, 15 Jun 2026 12:25:49 -0500
> Chris Morgan <macroalpha82@gmail.com> wrote:
>
> > From: Chris Morgan <macromorgan@hotmail.com>
> >
> > Add functions for reading temperature sensor data.
> >
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> Another sashiko reported thing. I'd definitely have missed
> this one and I think it is correct.
>
> When I get caught up I'll post a thread to see if people
> feel we should generally just ask for Sashiko to reply on
> list.
>
> Jonathan
>
> > diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> > index 64f5d263de4f..644cd7f821b9 100644
> > --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> > +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> > @@ -162,6 +162,24 @@ static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st,
> > return 0;
> > }
> >
> > +int inv_icm42607_set_temp_conf(struct inv_icm42607_state *st, bool enable,
> > + unsigned int *sleep_ms)
> > +{
> > + unsigned int val;
> > + int ret;
> > +
> > + val = FIELD_PREP(INV_ICM42607_TEMP_CONFIG0_FILTER_MASK,
> > + INV_ICM42607_FILTER_BW_34HZ);
> > + ret = regmap_update_bits(st->map, INV_ICM42607_REG_TEMP_CONFIG0,
> > + INV_ICM42607_TEMP_CONFIG0_FILTER_MASK, val);
> > + if (ret)
> > + return ret;
> > +
> > + return inv_icm42607_set_pwr_mgmt0(st, st->conf.gyro.mode,
> > + st->conf.accel.mode, enable,
> > + sleep_ms);
> > +}
>
> > diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c
> > new file mode 100644
> > index 000000000000..9a60e1a478b0
> > --- /dev/null
> > +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c
>
> > +static int inv_icm42607_temp_read(struct inv_icm42607_state *st, s16 *temp)
> > +{
> > + struct device *dev = regmap_get_device(st->map);
> > + u8 raw[2];
> > + int ret;
> > +
> > + PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
> > + ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
> > + if (ret)
> > + return ret;
> > +
> > + guard(mutex)(&st->lock);
> > +
> > + st->conf.temp_en = true;
>
> Sashiko points out (seems right) that this sets the internal state
> before the power management routine expects it to be set. So why
> is this here as opposed to just passing true into the
> function that follows?
I'm going to take setting of state out of the power management function
entirely, as well as killing off any "temp_en" since technically there
is nothing to enable on the temp sensor. I'll do a check when you try
to read the temp to see if any sensors are on and if none are then I
will power up the accel sensor (since it uses about 5x less power than
the gyro per the datasheet).
>
> > + ret = inv_icm42607_set_temp_conf(st, st->conf.temp_en, NULL);
> > + st->conf.temp_en = false;
> > + if (ret)
> > + return ret;
> > +
> > + ret = regmap_bulk_read(st->map, INV_ICM42607_REG_TEMP_DATA1,
> > + raw, sizeof(raw));
> > + if (ret)
> > + return ret;
> > +
> > + *temp = get_unaligned_be16(raw);
> > + if (*temp == INV_ICM42607_DATA_INVALID)
> > + return -EINVAL;
> > +
> > + return 0;
> > +}
Thank you,
Chris
^ permalink raw reply
* Re: [PATCH V13 5/9] iio: imu: inv_icm42607: Add PM support for icm42607
From: Chris Morgan @ 2026-06-23 2:33 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Chris Morgan, linux-iio, andy, nuno.sa, dlechner,
jean-baptiste.maneyrol, linux-rockchip, devicetree, heiko,
conor+dt, krzk+dt, robh, andriy.shevchenko
In-Reply-To: <20260621181948.21d40a09@jic23-huawei>
On Sun, Jun 21, 2026 at 06:19:48PM +0100, Jonathan Cameron wrote:
> On Mon, 15 Jun 2026 12:25:48 -0500
> Chris Morgan <macroalpha82@gmail.com> wrote:
>
> > From: Chris Morgan <macromorgan@hotmail.com>
> >
> > Add power management support for the ICM42607 device driver.
> >
> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> A few things from taking a look at the sashiko report:
> https://sashiko.dev/#/patchset/20260615172554.160910-1-macroalpha82%40gmail.com
>
> > ---
> > drivers/iio/imu/inv_icm42607/inv_icm42607.h | 18 +++
> > .../iio/imu/inv_icm42607/inv_icm42607_core.c | 139 ++++++++++++++++++
> > .../iio/imu/inv_icm42607/inv_icm42607_i2c.c | 1 +
> > .../iio/imu/inv_icm42607/inv_icm42607_spi.c | 1 +
> > 4 files changed, 159 insertions(+)
> >
> > diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607.h b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> > index a6a58571935f..4f4f541027dc 100644
> > --- a/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> > +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
>
> > @@ -334,11 +345,18 @@ struct inv_icm42607_state {
> > #define INV_ICM42607_GYRO_STOP_TIME_MS 45
> > #define INV_ICM42607_TEMP_STARTUP_TIME_MS 77
> >
> > +/*
> > + * Suspend delay assumed from other icm42600 series device, not
> > + * documented in datasheet.
> > + */
> > +#define INV_ICM42607_SUSPEND_DELAY_MS (2 * USEC_PER_MSEC)
>
> Sashiko had a valid comment on this. MSEC_PER_SEC seems more
> appropriate given this is 2 seconds in milli seconds.
>
> > +
> > typedef int (*inv_icm42607_bus_setup)(struct inv_icm42607_state *);
> >
> > extern const struct regmap_config inv_icm42607_regmap_config;
> > extern const struct inv_icm42607_hw inv_icm42607_hw_data;
> > extern const struct inv_icm42607_hw inv_icm42607p_hw_data;
> > +extern const struct dev_pm_ops inv_icm42607_pm_ops;
> >
> > int inv_icm42607_core_probe(struct regmap *regmap,
> > const struct inv_icm42607_hw *hw,
> > diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> > index 4b8e19091786..64f5d263de4f 100644
> > --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> > +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> > @@ -4,6 +4,7 @@
> > */
> >
> > #include <linux/bitfield.h>
> > +#include <linux/cleanup.h>
> > #include <linux/delay.h>
> > #include <linux/dev_printk.h>
> > #include <linux/device/devres.h>
> > @@ -11,6 +12,7 @@
> > #include <linux/iio/iio.h>
> > #include <linux/module.h>
> > #include <linux/mutex.h>
> > +#include <linux/pm_runtime.h>
> > #include <linux/regmap.h>
> > #include <linux/regulator/consumer.h>
> > #include <linux/time.h>
> > @@ -103,6 +105,63 @@ const struct inv_icm42607_hw inv_icm42607p_hw_data = {
> > };
> > EXPORT_SYMBOL_NS_GPL(inv_icm42607p_hw_data, "IIO_ICM42607");
> >
> > +static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st,
> > + enum inv_icm42607_sensor_mode gyro,
> > + enum inv_icm42607_sensor_mode accel,
> > + bool temp, unsigned int *sleep_ms)
> > +{
> > + enum inv_icm42607_sensor_mode oldaccel = st->conf.accel.mode;
> > + enum inv_icm42607_sensor_mode oldgyro = st->conf.gyro.mode;
> > + bool oldtemp = st->conf.temp_en;
> > + unsigned int sleepval_ms;
> > + unsigned int val;
> > + int ret;
> > +
> > + if (gyro == oldgyro && accel == oldaccel && temp == oldtemp)
> > + return 0;
> > +
> > + /*
> > + * Datasheet on page 14.26 says we need to ensure the gyro sensor is on
> > + * for a minimum of 45ms. So if we transition from an on state to an
> > + * off state wait 45ms to ensure a sufficient pause before power off.
>
> Sashiko commented on this.. I think what we could do with adding to the
> comment is what the path is that didn't pass through this function which would
> ensure we have been on for 30 of this msecs already.
I'm going to track whatever time the gyro started, and then if less
than 45ms has elapsed just pause the remaining amount of time.
>
> > + */
> > + if (!gyro && oldgyro)
> > + fsleep(INV_ICM42607_GYRO_STOP_TIME_MS * USEC_PER_MSEC);
> > +
> > + val = FIELD_PREP(INV_ICM42607_PWR_MGMT0_GYRO_MODE_MASK, gyro);
> > + val |= FIELD_PREP(INV_ICM42607_PWR_MGMT0_ACCEL_MODE_MASK, accel);
> > + ret = regmap_write(st->map, INV_ICM42607_REG_PWR_MGMT0, val);
> > + if (ret)
> > + return ret;
> > +
> > + st->conf.gyro.mode = gyro;
> > + st->conf.accel.mode = accel;
> > + st->conf.temp_en = temp;
> > +
> > + /*
> > + * If a state change occurs from off to on, sleep for the startup
> > + * time of the sensor, unless a sleep_ms is specified. Since more
> > + * than one sensor can be transitioned from off to on, select the
> > + * maximum time from each of the sensors changing from off to on.
> > + */
> > + sleepval_ms = 0;
> > + if (temp && !oldtemp)
> > + sleepval_ms = max(sleepval_ms, INV_ICM42607_TEMP_STARTUP_TIME_MS);
> > +
> > + if (accel && !oldaccel)
> > + sleepval_ms = max(sleepval_ms, INV_ICM42607_ACCEL_STARTUP_TIME_MS);
> > +
> > + if (gyro && !oldgyro)
> > + sleepval_ms = max(sleepval_ms, INV_ICM42607_GYRO_STARTUP_TIME_MS);
> > +
> > + if (sleep_ms)
> > + *sleep_ms = sleepval_ms;
> > + else if (sleepval_ms)
> > + fsleep(sleepval_ms * USEC_PER_MSEC);
> > +
> > + return 0;
> > +}
>
> >
> > int inv_icm42607_core_probe(struct regmap *regmap,
> > @@ -236,6 +305,8 @@ int inv_icm42607_core_probe(struct regmap *regmap,
> > if (!st)
> > return -ENOMEM;
> >
> > + dev_set_drvdata(dev, st);
> > +
> > ret = devm_mutex_init(dev, &st->lock);
> > if (ret)
> > return ret;
> > @@ -271,10 +342,78 @@ int inv_icm42607_core_probe(struct regmap *regmap,
> > if (ret)
> > return ret;
> >
> > + ret = devm_pm_runtime_set_active_enabled(dev);
> > + if (ret)
> > + return ret;
> > +
> > + pm_runtime_set_autosuspend_delay(dev, INV_ICM42607_SUSPEND_DELAY_MS);
> > + pm_runtime_use_autosuspend(dev);
> Sashiko does put out some stuff here. Please take a look and work out or
> test if it is right (I think not but haven't checked that carefully!)
> From a quick look I think that the auto disabling of autosuspend does a
> rpm_idle() that should result in it suspending...
>
I see a few other drivers adding one more call to
devm_pm_runtime_enable() so I'm going to see how that works out.
>
> > +
> > return 0;
> > }
> > EXPORT_SYMBOL_NS_GPL(inv_icm42607_core_probe, "IIO_ICM42607");
Thank you,
Chris
^ permalink raw reply
* Re: [PATCH v11 5/5] iio: adc: versal-sysmon: add oversampling support
From: sashiko-bot @ 2026-06-23 2:21 UTC (permalink / raw)
To: Salih Erim; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260623014036.3865402-6-salih.erim@amd.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing hardware synchronization for oversampling ratio caches during initialization.
--
commit 25a143792e9062675e1abe4133ac067208b02232
Author: Salih Erim <salih.erim@amd.com>
iio: adc: versal-sysmon: add oversampling support
Add support for reading and writing the oversampling ratio through the IIO
oversampling_ratio attribute. The hardware supports averaging 2, 4, 8, or 16
samples, plus a ratio of 1 (no averaging).
> diff --git a/drivers/iio/adc/versal-sysmon-core.c b/drivers/iio/adc/versal-sysmon-core.c
> index 8f2c502d9cb6d..1b55d343982ef 100644
> --- a/drivers/iio/adc/versal-sysmon-core.c
> +++ b/drivers/iio/adc/versal-sysmon-core.c
[ ... ]
> @@ -846,6 +999,8 @@ int devm_versal_sysmon_core_probe(struct device *dev, struct regmap *regmap)
>
> sysmon = iio_priv(indio_dev);
> sysmon->regmap = regmap;
> + sysmon->temp_oversampling = 1;
> + sysmon->supply_oversampling = 1;
[Severity: Medium]
Could this lead to a mismatch between the software cache and the hardware
state if the bootloader or firmware previously configured a non-default
oversampling ratio?
Since the driver initializes temp_oversampling and supply_oversampling to 1
without reading the initial state from the hardware or forcing the hardware
registers to match the defaults, userspace might read an oversampling ratio
of 1 via sysfs while the hardware is actually still actively averaging
multiple samples.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260623014036.3865402-1-salih.erim@amd.com?part=5
^ permalink raw reply
* Re: [PATCH v11 4/5] iio: adc: versal-sysmon: add threshold event support
From: sashiko-bot @ 2026-06-23 2:15 UTC (permalink / raw)
To: Salih Erim; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260623014036.3865402-5-salih.erim@amd.com>
Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] Use-After-Free due to flawed devres initialization order for the delayed worker.
- [High] Error paths in the IRQ handler skip clearing hardware flags but unconditionally return IRQ_HANDLED, causing a hard lockup.
- [High] The delayed worker inadvertently clears all pending interrupts from the hardware, causing unrelated events like voltage alarms to be permanently lost.
- [Medium] Atomic context violation causes a kernel panic if the device tree configures an IRQ for an I2C device.
- [Medium] Negative temperatures invoke undefined left-shift behavior and are not masked to 16 bits, causing 0xFFFFxxxx to be written to the threshold registers.
- [Medium] Race condition between sysfs event disable and the unmask worker re-enables disabled interrupts, causing spurious events.
- [Medium] Cached hysteresis value is not reverted if the hardware threshold write fails.
--
commit c435c4c9fc09d3cf9f148b3e41fd576f78414383
Author: Salih Erim <salih.erim@amd.com>
iio: adc: versal-sysmon: add threshold event support
Add threshold event support for temperature and supply voltage channels.
Temperature uses hysteresis mode while voltage uses window mode. It sets
up interrupt handlers and a delayed worker to poll for condition clear.
> diff --git a/drivers/iio/adc/versal-sysmon-core.c b/drivers/iio/adc/versal-sysmon-core.c
> --- a/drivers/iio/adc/versal-sysmon-core.c
> +++ b/drivers/iio/adc/versal-sysmon-core.c
[ ... ]
> @@ -52,6 +102,16 @@ static const struct iio_chan_spec temp_channels[] = {
> SYSMON_CHAN_TEMP(3, SYSMON_TEMP_MIN_MIN, "min_min"),
> };
>
> +static void sysmon_q8p7_to_millicelsius(s16 raw_data, int *val)
> +{
> + *val = (raw_data * MILLIDEGREE_PER_DEGREE) >> SYSMON_FRACTIONAL_SHIFT;
> +}
> +
> +static void sysmon_millicelsius_to_q8p7(u32 *raw_data, int val)
> +{
> + *raw_data = (val << SYSMON_FRACTIONAL_SHIFT) / MILLIDEGREE_PER_DEGREE;
> +}
[Severity: Medium]
Does this cause undefined behavior and corrupt hardware registers for negative
temperatures?
If val is a negative temperature, the division yields a negative integer.
Assigning this to the u32 pointer raw_data will sign-extend it, filling the
upper 16 bits with 1s. Unlike the voltage path which explicitly casts to
(u16), this value is written directly to the SYSMON_TEMP_TH_UP and
SYSMON_TEMP_TH_LOW registers later in sysmon_write_event_value(), which
might corrupt reserved or mode configuration fields in the hardware.
[ ... ]
> +static int sysmon_write_event_config(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + bool state)
> +{
[ ... ]
> + case IIO_TEMP:
> + if (state) {
> + ret = regmap_write(sysmon->regmap, SYSMON_IER, mask);
> + if (ret)
> + return ret;
> +
> + scoped_guard(spinlock_irq, &sysmon->irq_lock)
> + sysmon->temp_mask &= ~mask;
> + } else {
> + ret = regmap_write(sysmon->regmap, SYSMON_IDR, mask);
> + if (ret)
> + return ret;
> +
> + scoped_guard(spinlock_irq, &sysmon->irq_lock)
> + sysmon->temp_mask |= mask;
> + }
> + return 0;
[Severity: Medium]
Is there a race condition here with sysmon_unmask_worker() that could lead to
spurious events?
If sysmon_unmask_worker() runs concurrently during the disable path, it
could acquire irq_lock right after SYSMON_IDR is written but before
sysmon->temp_mask is updated. The worker would read the old temp_mask
(indicating the event is still enabled) and write to SYSMON_IER,
re-enabling the interrupt in hardware. Once the worker releases the lock,
this sysfs thread completes the update, leaving the software state as
disabled but the hardware actively armed.
[ ... ]
> +static int sysmon_write_event_value(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + enum iio_event_type type,
> + enum iio_event_direction dir,
> + enum iio_event_info info,
> + int val, int val2)
> +{
[ ... ]
> + case IIO_TEMP:
> + switch (info) {
[ ... ]
> + case IIO_EV_INFO_HYSTERESIS:
> + if (val < 0)
> + return -EINVAL;
> +
> + sysmon->temp_hysteresis = val;
> +
> + return sysmon_update_temp_lower(sysmon);
[Severity: Medium]
Does this leave the cached hysteresis out of sync with hardware if the write
fails?
If sysmon_update_temp_lower() fails to write to the hardware register and
returns an error, sysmon->temp_hysteresis retains the new value. Future
writes to the upper threshold will use this corrupted cached value to
recompute and overwrite the lower threshold register.
[ ... ]
> +static int sysmon_handle_event(struct iio_dev *indio_dev, u32 event)
> +{
[ ... ]
> + switch (event) {
> + case SYSMON_BIT_TEMP:
> + sysmon_push_event(indio_dev, SYSMON_TEMP_MAX);
> +
> + ret = regmap_write(sysmon->regmap, SYSMON_IDR, BIT(SYSMON_BIT_TEMP));
> + if (ret)
> + return ret;
[Severity: High]
If this regmap_write() fails, does this cause an unrecoverable interrupt storm?
If we return an error early, the level-sensitive hardware interrupt remains
asserted because it was not masked or cleared. The caller, sysmon_iio_irq(),
ignores this error and returns IRQ_HANDLED, bypassing the core spurious
interrupt detector. The still-asserted hardware interrupt will immediately
re-trigger the handler, creating a hard lockup.
[ ... ]
> +static void sysmon_unmask_worker(struct work_struct *work)
> +{
> + struct sysmon *sysmon =
> + container_of(work, struct sysmon, sysmon_unmask_work.work);
> + unsigned int isr;
> +
> + /*
> + * If the ISR read fails, skip processing to avoid acting
> + * on undefined data.
> + */
> + scoped_guard(spinlock_irq, &sysmon->irq_lock) {
> + if (regmap_read(sysmon->regmap, SYSMON_ISR, &isr))
> + break;
[Severity: Medium]
Will this trigger a 'scheduling while atomic' panic if the regmap is backed
by I2C?
The code acquires a spinlock with IRQs disabled and then calls regmap_read().
If fwnode_irq_get() succeeds for an I2C device, the regmap relies on a mutex
and will attempt to sleep, which isn't allowed while holding a spinlock. The
same issue appears in sysmon_iio_irq(), which executes in hardirq context
and blindly requests a hardirq regardless of regmap sleepability.
> + regmap_write(sysmon->regmap, SYSMON_ISR, isr);
[Severity: High]
Does this unconditionally clear all pending interrupts from the hardware,
causing unrelated events to be permanently lost?
SYSMON_ISR is a Write-1-to-Clear register. If a voltage alarm (like
SYSMON_BIT_ALARM0) fires and is set in SYSMON_ISR right before this worker
reads the register, writing it back will clear the alarm. Since
sysmon_unmask_temp() strictly ignores all bits except the temperature mask,
the voltage alarm is permanently discarded without generating an IIO event.
> + sysmon_unmask_temp(sysmon, isr);
> + }
> +
> + if (sysmon->masked_temp)
> + schedule_delayed_work(&sysmon->sysmon_unmask_work,
> + msecs_to_jiffies(SYSMON_UNMASK_WORK_DELAY_MS));
> + else
> + regmap_write(sysmon->regmap, SYSMON_STATUS_RESET, 1);
> +}
> +
> +static irqreturn_t sysmon_iio_irq(int irq, void *data)
> +{
[ ... ]
> + sysmon_handle_events(indio_dev, isr);
> + schedule_delayed_work(&sysmon->sysmon_unmask_work,
> + msecs_to_jiffies(SYSMON_UNMASK_WORK_DELAY_MS));
> +
> + return IRQ_HANDLED;
> +}
[Severity: High]
Since the return value of sysmon_handle_events() is ignored, will we always
return IRQ_HANDLED even if we failed to clear the hardware interrupt condition?
As mentioned above, if sysmon_handle_events() encounters an error masking
the interrupt, returning IRQ_HANDLED perpetually hides the issue from the IRQ
subsystem, leading to an interrupt storm.
[ ... ]
> +static int sysmon_init_interrupt(struct sysmon *sysmon,
> + struct device *dev,
> + struct iio_dev *indio_dev,
> + int irq)
> +{
[ ... ]
> + INIT_DELAYED_WORK(&sysmon->sysmon_unmask_work, sysmon_unmask_worker);
> +
> + ret = regmap_read(sysmon->regmap, SYSMON_IMR, &imr);
> + if (ret)
> + return ret;
> + sysmon->temp_mask = imr & SYSMON_TEMP_INTR_MASK;
> +
> + ret = devm_request_irq(dev, irq, sysmon_iio_irq, 0, "sysmon-irq", indio_dev);
> + if (ret)
> + return ret;
> +
> + return devm_add_action_or_reset(dev, sysmon_disable_interrupts, sysmon);
> +}
[Severity: High]
Does this devres initialization order cause a Use-After-Free during teardown?
devres cleans up resources in reverse order (LIFO). Because
devm_add_action_or_reset() is called after devm_request_irq(),
sysmon_disable_interrupts() will execute first. It calls
cancel_delayed_work_sync(), but the IRQ handler is still active.
If an interrupt fires concurrently or is already in-flight, sysmon_iio_irq()
can unconditionally call schedule_delayed_work(), placing the work back on
the queue.
When the IRQ is subsequently freed and the sysmon memory is released, the
delayed worker will execute on freed memory 500ms later.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260623014036.3865402-1-salih.erim@amd.com?part=4
^ permalink raw reply
* Re: [PATCH v11 3/5] iio: adc: versal-sysmon: add I2C driver
From: sashiko-bot @ 2026-06-23 2:03 UTC (permalink / raw)
To: Salih Erim; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260623014036.3865402-4-salih.erim@amd.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Non-atomic I2C read transaction releases the bus lock, making it vulnerable to bus interleaving.
--
commit 0b46526090ca97299b6d37544af48b39e72bb565
Author: Salih Erim <salih.erim@amd.com>
iio: adc: versal-sysmon: add I2C driver
This commit adds an I2C transport driver for the Versal SysMon block,
allowing external masters to read voltage and temperature measurements. It
implements a regmap backend using I2C but does not support events due to
the lack of an interrupt line.
> diff --git a/drivers/iio/adc/versal-sysmon-i2c.c b/drivers/iio/adc/versal-sysmon-i2c.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/iio/adc/versal-sysmon-i2c.c
[ ... ]
> +static int sysmon_i2c_reg_read(void *context, unsigned int reg,
> + unsigned int *val)
> +{
> + struct i2c_client *client = context;
> + u8 write_buf[8] = { };
> + u8 read_buf[4];
> + int ret;
> +
> + put_unaligned_le16(reg >> 2, &write_buf[SYSMON_I2C_REG_OFS]);
> + write_buf[SYSMON_I2C_INSTR_OFS] = SYSMON_I2C_INSTR_READ;
> +
> + ret = i2c_master_send(client, write_buf, sizeof(write_buf));
> + if (ret < 0)
> + return ret;
> + if (ret != sizeof(write_buf))
> + return -EIO;
> +
> + ret = i2c_master_recv(client, read_buf, sizeof(read_buf));
[Severity: High]
Does this sequence allow another device to interleave its I2C traffic on a
shared bus?
Looking at sysmon_i2c_reg_read(), the register read operation is split into
separate i2c_master_send() and i2c_master_recv() calls.
Because i2c_master_send() releases the I2C adapter bus lock before returning,
another kernel thread can perform a bus transaction between these two calls.
If an intervening bus transaction occurs, could it reset the sysmon device's
I2C state machine, discarding the previously written command payload and
corrupting the subsequent read?
Would using i2c_transfer() to perform both the write and read messages as a
single atomic transaction prevent this interleaving?
> + if (ret < 0)
> + return ret;
> + if (ret != sizeof(read_buf))
> + return -EIO;
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260623014036.3865402-1-salih.erim@amd.com?part=3
^ permalink raw reply
* RE: [PATCH 3/8] Bluetooth: btnxpuart: Add M.2 Bluetooth device support using pwrseq
From: Sherry Sun @ 2026-06-23 1:57 UTC (permalink / raw)
To: Frank Li (OSS), Sherry Sun (OSS)
Cc: robh@kernel.org, krzk+dt@kernel.org, conor+dt@kernel.org,
Frank Li, s.hauer@pengutronix.de, kernel@pengutronix.de,
festevam@gmail.com, Amitkumar Karwar, Neeraj Sanjay Kale,
marcel@holtmann.org, luiz.dentz@gmail.com, Hongxing Zhu,
l.stach@pengutronix.de, lpieralisi@kernel.org,
kwilczynski@kernel.org, mani@kernel.org, bhelgaas@google.com,
brgl@kernel.org, imx@lists.linux.dev, linux-pci@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-bluetooth@vger.kernel.org,
linux-pm@vger.kernel.org
In-Reply-To: <VI0PR04MB121141C8A15758C0244AA351F92EF2@VI0PR04MB12114.eurprd04.prod.outlook.com>
> Subject: RE: [PATCH 3/8] Bluetooth: btnxpuart: Add M.2 Bluetooth device
> support using pwrseq
>
> > On Thu, Jun 18, 2026 at 06:10:42PM +0800, Sherry Sun (OSS) wrote:
> > > From: Sherry Sun <sherry.sun@nxp.com>
> > >
> > > Power supply to the M.2 Bluetooth device attached to the host using
> > > M.2 connector is controlled using the 'uart' pwrseq device. So add
> > > support for getting the pwrseq device if the OF graph link is present.
> > > Once obtained, the existing pwrseq APIs can be used to control the
> > > power supplies of the
> > > M.2 card.
> > >
> > > Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
> > > ---
> > > drivers/bluetooth/btnxpuart.c | 33
> > > ++++++++++++++++++++++++++++++---
> > > 1 file changed, 30 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/bluetooth/btnxpuart.c
> > > b/drivers/bluetooth/btnxpuart.c index e7036a48ce48..1aa8972f0dab
> > > 100644
> > > --- a/drivers/bluetooth/btnxpuart.c
> > > +++ b/drivers/bluetooth/btnxpuart.c
> > > @@ -9,6 +9,8 @@
> > >
> > > #include <linux/serdev.h>
> > > #include <linux/of.h>
> > > +#include <linux/of_graph.h>
> > > +#include <linux/pwrseq/consumer.h>
> > > #include <linux/skbuff.h>
> > > #include <linux/unaligned.h>
> > > #include <linux/firmware.h>
> > > @@ -211,6 +213,7 @@ struct btnxpuart_dev {
> > >
> > > struct ps_data psdata;
> > > struct btnxpuart_data *nxp_data;
> > > + struct pwrseq_desc *pwrseq;
> > > struct reset_control *pdn;
> > > struct hci_uart hu;
> > > };
> > > @@ -1866,11 +1869,27 @@ static int nxp_serdev_probe(struct
> > serdev_device *serdev)
> > > return err;
> > > }
> > >
> > > + if (of_graph_is_present(dev_of_node(&serdev->ctrl->dev))) {
> > > + struct pwrseq_desc *pwrseq;
> > > +
> > > + pwrseq = devm_pwrseq_get(&serdev->ctrl->dev, "uart");
> > > + if (IS_ERR(pwrseq))
> > > + return PTR_ERR(pwrseq);
> > > +
> > > + nxpdev->pwrseq = pwrseq;
> > > + err = pwrseq_power_on(pwrseq);
> > > + if (err) {
> > > + dev_err(&serdev->dev, "Failed to power on
> > pwrseq\n");
> > > + return err;
> > > + }
> >
> > Can you provide helper function like devm clk get and enabled?
> > like devm_pwrsq_get_on()
> >
> > So simple below error handle.
>
> Ok, will try.
>
Hi Frank,
I re-checked the devm_pwrseq_get() and found that it has already
registered devm_pwrseq_put → pwrseq_put(), and pwrseq_put()
calls pwrseq_power_off() when desc->powered_on == true.
The unwind is fully automatic. So no extra cleanup action is needed
for power-off, also no need for the helper function like
devm_pwrsq_get_on() here, I will directly delete the power_off
error handling in V2.
Best Regards
Sherry
>
> >
> > > + }
> > > +
> > > /* Initialize and register HCI device */
> > > hdev = hci_alloc_dev();
> > > if (!hdev) {
> > > dev_err(&serdev->dev, "Can't allocate HCI device\n");
> > > - return -ENOMEM;
> > > + err = -ENOMEM;
> > > + goto err_pwrseq_power_off;
> > > }
> > >
> > > reset_control_deassert(nxpdev->pdn);
> > > @@ -1903,11 +1922,14 @@ static int nxp_serdev_probe(struct
> > > serdev_device *serdev)
> > >
> > > if (hci_register_dev(hdev) < 0) {
> > > dev_err(&serdev->dev, "Can't register HCI device\n");
> > > + err = -ENODEV;
> > > goto probe_fail;
> > > }
> > >
> > > - if (ps_setup(hdev))
> > > + if (ps_setup(hdev)) {
> > > + err = -ENODEV;
> > > goto probe_fail;
> > > + }
> > >
> > > hci_devcd_register(hdev, nxp_coredump, nxp_coredump_hdr,
> > > nxp_coredump_notify);
> > > @@ -1917,7 +1939,10 @@ static int nxp_serdev_probe(struct
> > > serdev_device *serdev)
> > > probe_fail:
> > > reset_control_assert(nxpdev->pdn);
> > > hci_free_dev(hdev);
> > > - return -ENODEV;
> > > +err_pwrseq_power_off:
> > > + if (nxpdev->pwrseq)
> > > + pwrseq_power_off(nxpdev->pwrseq);
> > > + return err;
> > > }
> > >
> > > static void nxp_serdev_remove(struct serdev_device *serdev) @@
> > > -1944,6 +1969,8 @@ static void nxp_serdev_remove(struct
> > > serdev_device
> > *serdev)
> > > ps_cleanup(nxpdev);
> > > hci_unregister_dev(hdev);
> > > reset_control_assert(nxpdev->pdn);
> > > + if (nxpdev->pwrseq)
> > > + pwrseq_power_off(nxpdev->pwrseq);
> > > hci_free_dev(hdev);
> > > }
> > >
> > > --
> > > 2.50.1
> > >
> > >
^ 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