* Re: [PATCH v1 3/3] arm64: dts: qcom: x1e78100-t14s: Add thermal zones for keyboard skin and charging sensors
From: sashiko-bot @ 2026-06-24 21:18 UTC (permalink / raw)
To: Daniel Lezcano; +Cc: linux-hwmon, robh, conor+dt, devicetree
In-Reply-To: <20260624210825.264454-4-daniel.lezcano@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit message claims the charging thermal zone uses 'multiple passive trip points' to 'progressively throttle the different CPU clusters', but the implementation provides only a single passive trip point that throttles all CPUs simultaneously.
--
commit 165036bedb34a29493f4bd0a7af23dab152aff0c
Author: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
arm64: dts: qcom: x1e78100-t14s: Add thermal zones for keyboard skin and charging sensors
Add thermal zone descriptions for the keyboard skin temperature and battery
charging circuitry sensors exposed by the embedded controller.
> - a charging circuitry temperature zone with multiple passive trip
> points and CPU frequency mitigation levels.
>
> The charging thermal zone progressively throttles the different CPU
> clusters as the charging circuitry temperature rises and triggers a hot
> condition at the highest trip point.
[Severity: Medium]
Does the implementation match this commit message description?
The commit message claims there are multiple passive trip points and
progressive throttling across different CPU clusters. Is a chunk of the patch
missing, or should the commit message be updated to reflect the final design
shown below?
> diff --git a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi
> --- a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi
> +++ b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi
[ ... ]
> + ec-charging-thermal {
> + polling-delay = <5000>;
> + polling-delay-passive = <2000>;
> +
> + thermal-sensors = <&ec 3>;
> +
> + trips {
> + psv0: trip-point0 {
> + temperature = <55000>;
> + hysteresis = <0>;
> + type = "passive";
> + };
> +
> + alert0: trip-point1 {
> + temperature = <63000>;
> + hysteresis = <0>;
> + type = "hot";
> + };
> + };
[Severity: Medium]
Only a single passive trip point is defined here in ec-charging-thermal,
which doesn't match the "multiple passive trip points" mentioned in the
commit message.
> +
> + cooling-maps {
> + map0 {
> + trip = <&psv0>;
> + cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu8 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu9 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu10 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu11 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +
> + };
> + };
[Severity: Medium]
Additionally, all 12 CPUs are tied uniformly to this single trip point.
Should this define progressive throttling across different CPU clusters as
described in the commit message?
> +
> + };
> +};
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624210825.264454-1-daniel.lezcano@oss.qualcomm.com?part=3
^ permalink raw reply
* Re: [PATCH V14 0/9] Add Invensense ICM42607
From: Chris Morgan @ 2026-06-24 21:18 UTC (permalink / raw)
To: Chris Morgan
Cc: linux-iio, andy, nuno.sa, dlechner, jic23, jean-baptiste.maneyrol,
linux-rockchip, devicetree, heiko, conor+dt, krzk+dt, robh,
andriy.shevchenko
In-Reply-To: <20260624182350.50467-1-macroalpha82@gmail.com>
On Wed, Jun 24, 2026 at 01:23:39PM -0500, Chris Morgan wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
>
> Add support for the ICM42607 IMU. This sensor shares the same
> functionality but a different register layout with the existing
> ICM42600.
>
> This driver should work with the ICM42607 and ICM42607P over both I2C
> and SPI, however only the ICM42607P over I2C could be tested.
>
> Changes Since V1:
> - Instead of creating a new driver, merged with the existing inv_icm42600
> driver. This necessitated adding some code to the existing driver to
> permit using a different register layout for the same functionality.
> - Split changes up a bit more to decrease the size of the individual
> patches. Note that patch 0004 is still pretty hefty; if I need to split
> further I may need to create some temporary stub functions.
> - Used guard() and PM_RUNTIME_ACQUIRE_AUTOSUSPEND() on the new functions
> per Jonathan's recommendations.
>
> Changes Since V2:
> - Went back to using a new driver on advice from Invensense engineer.
> - Further split changes up into smaller chunks of functionality. Note
> still that the largest patch is approximately 900 lines, and that while
> the driver compiles cleanly at each commit it is not able to drive the
> hardware until the commit that adds the Interrupt (as it also adds the
> Makefile).
> - Change the error to a warning when the devicetree binding does not match
> the hardware ID.
> - Dropped the ack on the devicetree bindings, as I am creating a new file
> (for a new driver) instead of modifying the existing one.
>
> Changes Since V3:
> - Numerous small fixes (too many to list here). Thank you to everyone who
> provided feedback.
> - Split power management additions into an additional commit to break
> things up further.
> - Consolidated devicetree documentation in existing
> invensense,icm42600.yaml file.
> - Removed most of the FIELD_PREP from header file to c files to make code
> easier to read.
> - Changed scale values to 2D arrays for Gyro and Accelerometer.
> - Removed IIO_CHAN_INFO_CALIBBIAS attribute.
>
> Changes Since V4:
> - Additional numerous small fixes, thank you again for all the feedback.
> - Dropped power control API and instead run device in low noise mode.
> - Split devicetree bindings into two distinct changes.
> - Reordered adding of enums and structs to main header file so that they
> are only brought in when needed.
> - Stopped using enum for driver data and instead am using pointer to
> device specific driver data.
>
> Changes Since V5:
> - Corrected use of "dev_warn_probe" to just "dev_warn".
> - Fixed some return scenarios which would unconditionally return 0
> when an error was present.
> - Corrected use of max() to min() for bounds checking. max() was
> incorrect.
> - Fixed using "st->conf.accel.odr" in the gyroscope function. It
> should have been "st->conf.gyro.odr" which it now is.
> - Additional small fixes suggested by "sashiko.dev".
> - Added a regmap cache. I used the datasheet to try and determine
> which registers might change without explicit writes.
>
> Changes Since V6:
> - Corrected additional errors identified by sashiko.dev, mostly
> fixing potential deadlocks, missing calls for pm runtime, and
> potential overflow issues.
>
> Changes Since V7:
> - Dropped Wake on Movement patches, since some of the functionality
> was only available for a device on which I cannot test.
> - Dropped support for SPI 3-Wire mode, since it complicated the
> bus setup (and I lack the hardware to test such features anyway).
> - Fixed a few additional bugs identified by sashiko.dev bot.
>
> Changes Since V8:
> - Added back IRQ dropped accidentally when dropping wake on movement
> patches.
> - Dropped "Reviewed-By" tag on patch 2 because of substantial changes
> made to devicetree binding documentation.
> - Additional small fixes as suggested.
>
> Changes Since V9:
> - Removed interrupts (and buffers) from the driver. I previously was
> unable to detect deadlocks because it turns out my IRQ was not even
> wired correctly in my device.
> - Updated devicetree binding commits to make interrupts optional for
> users of the icm42607 driver.
>
> Changes Since V10:
> - Explicitly specified enum values in header file.
> - Removed additional dead code for buffer handling.
> - Cleaned up headers.
> - Added additional locks as requested by sashiko.dev bot.
>
> Changes Since V11:
> - Since driver has shrunk in size considerably, moved i2c bits into
> first code commit. This ensures that the very first commit with code
> can now be compiled. The commit after that adds SPI support as it
> was in the previous versions.
> - Used pahole to optimize inv_icm42607_state. Reordering elements
> reduced size in memory from 384 bytes to 256 bytes.
> - Added a map of all readable registers and all writeable registers
> according to the datasheet.
> - Added back some missing headers pointed out by the maintainers.
> - Added FIELD_PREP in a few more places to make the code more
> obvious on what it's doing.
> - Added a comment to the power management code to note that
> temperature sensor being enabled doesn't matter as the clocks
> are off by default when the gyro and accel channels are off.
> - Removed iio_device_claim_direct() calls since it was no longer
> needed.
> - Fixed shared_by_all attributes for temperature sensor.
> - Additional miscellanous fixes as requested.
>
> Changes Since V12:
> - Removed aligned buffer from inv_icm42607_state struct as we do not
> currently have the need for it.
> - Corrected the order of the odr values in the accel and gyro files
> as the values were out of order (the place in the array corresponds
> to the register value).
> - Stopped setting the clock value depending upon the temp config. The
> datasheet advised to keep using the default value.
> - Corrected logic when changing between states. We only need to pause
> when a sensor changes from off to an on state or when the gyro
> changes from an on state to off.
> - Added missing includes for several files.
>
> Changes Since V13:
> - Refactored inv_icm42607_set_accel_conf() and
> inv_icm42607_set_gyro_conf() into a single function.
> - Refactored inv_icm42607_accel_read_sensor() and
> inv_icm42607_gyro_read_sensor() into a single function.
> - Merged inv_icm42607_set_temp_conf() into initial init function
> since it only really needs to be called once.
> - Saved adding temp sensor for last and updated
> inv_icm42607_temp_read() to either confirm other sensors are already
> enabled or enable the accelerometer so it can get a reading.
> - Updated inv_icm42607_set_pwr_mgmt0() so that it does not update the
> sensor mode and forcibly keep the sensor enabled.
> - Added inv_icm42607_temp_filter_bw enums since it appears to use
> different values than the accel or gyro sensor.
> - Set the temp startup time from 77ms to 77us, as I previously misread
> the datasheet.
> - Additional minor fixes.
>
> Chris Morgan (9):
> dt-bindings: iio: imu: icm42600: Add mount-matrix
> dt-bindings: iio: imu: icm42600: Add icm42607
> iio: imu: inv_icm42607: Add inv_icm42607 Core Driver
> iio: imu: inv_icm42607: Add SPI For icm42607
> iio: imu: inv_icm42607: Add PM support for icm42607
> iio: imu: inv_icm42607: Add Accelerometer for icm42607
> iio: imu: inv_icm42607: Add Gyroscope to icm42607
> iio: imu: inv_icm42607: Add Temp Support in icm42607
> arm64: dts: rockchip: Add icm42607p IMU for RG-DS
>
> .../bindings/iio/imu/invensense,icm42600.yaml | 20 +-
> .../dts/rockchip/rk3568-anbernic-rg-ds.dts | 8 +-
> drivers/iio/imu/Kconfig | 1 +
> drivers/iio/imu/Makefile | 1 +
> drivers/iio/imu/inv_icm42607/Kconfig | 30 +
> drivers/iio/imu/inv_icm42607/Makefile | 13 +
> drivers/iio/imu/inv_icm42607/inv_icm42607.h | 423 ++++++++++++
> .../iio/imu/inv_icm42607/inv_icm42607_accel.c | 317 +++++++++
> .../iio/imu/inv_icm42607/inv_icm42607_core.c | 606 ++++++++++++++++++
> .../iio/imu/inv_icm42607/inv_icm42607_gyro.c | 313 +++++++++
> .../iio/imu/inv_icm42607/inv_icm42607_i2c.c | 98 +++
> .../iio/imu/inv_icm42607/inv_icm42607_spi.c | 108 ++++
> .../iio/imu/inv_icm42607/inv_icm42607_temp.c | 99 +++
> .../iio/imu/inv_icm42607/inv_icm42607_temp.h | 37 ++
> 14 files changed, 2072 insertions(+), 2 deletions(-)
> create mode 100644 drivers/iio/imu/inv_icm42607/Kconfig
> create mode 100644 drivers/iio/imu/inv_icm42607/Makefile
> create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607.h
> create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_accel.c
> create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c
> create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_i2c.c
> create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_spi.c
> create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c
> create mode 100644 drivers/iio/imu/inv_icm42607/inv_icm42607_temp.h
>
> --
> 2.43.0
>
I got sashiko.dev's review, and agree with everything except 2 points.
1) It asks "Is it appropriate to omit the interrupts property based on
driver behavior rather than hardware wiring?" I would say no, but in
my case the hardware is not wired for an interrupt so it's moot.
2) It asks "Does this code update the software state cache for the
sensor mode?" No, it doesn't. The goal is for the device to stay
powered up for up to 2 seconds until runtime PM turns it off if it
isn't active, but I don't mind something turning around and shutting
the hardware down if it's not in use before then. Anything that needs
to read the specific sensor in question will be holding the mutex so
that there shouldn't be a risk of another thread turning stuff off
while I'm trying to use it, I don't think. What I want to avoid is
calling to turn the hardware on for a one-shot read and then
immediately turning it off once I'm done. I can wait the two seconds
for runtime PM to do it when the alternative is to call the shutdown
routine every single time for one-shot reads and wait for the startup
and in the case of the gyro shutdown times otherwise.
Thoughts?
I can fix the remaining issues it points out and resubmit.
Thank you,
Chris
^ permalink raw reply
* Re: [PATCH 0/2] rtc: aspeed: Add AST2700 RTC support
From: Alexandre Belloni @ 2026-06-24 21:16 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Joel Stanley,
Andrew Jeffery, Tommy Huang
Cc: linux-rtc, devicetree, linux-kernel, linux-arm-kernel,
linux-aspeed
In-Reply-To: <20260601-ast2700-rtc-v1-0-15d4ca46500a@aspeedtech.com>
On Mon, 01 Jun 2026 17:14:05 +0800, Tommy Huang wrote:
> This patch series adds support for the ASPEED AST2700 RTC define,
> includeing dt-binding in the Document and rtc-aspeed.c
Applied, thanks!
[1/2] dt-bindings: rtc: add ASPEED AST2700 compatible
https://git.kernel.org/abelloni/c/5e7f746bc106
[2/2] rtc: aspeed: add AST2700 compatible
https://git.kernel.org/abelloni/c/3319cfeeb8c4
Best regards,
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH v5] dt-bindings: rtc: epson,rx6110: Convert to DT Schema
From: Alexandre Belloni @ 2026-06-24 21:15 UTC (permalink / raw)
To: robh, krzk+dt, conor+dt, Udaya Kiran Challa
Cc: skhan, me, linux-rtc, devicetree, linux-kernel
In-Reply-To: <20260514173851.25088-1-challauday369@gmail.com>
On Thu, 14 May 2026 23:03:32 +0530, Udaya Kiran Challa wrote:
> Convert the Epson RX6110 Real Time Clock devicetree binding
> from the legacy text format to DT schema.
Applied, thanks!
[1/1] dt-bindings: rtc: epson,rx6110: Convert to DT Schema
https://git.kernel.org/abelloni/c/c7ab7504631d
Best regards,
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* Re: [PATCH v3 0/6] rtc: m41t93: add new features alarm, clock out, watchdog
From: Alexandre Belloni @ 2026-06-24 21:15 UTC (permalink / raw)
To: krzk+dt, robh, conor+dt, Akhilesh Patil
Cc: skhan, linux-rtc, devicetree, linux-kernel, akhileshpatilvnit
In-Reply-To: <cover.1758379856.git.akhilesh@ee.iitb.ac.in>
On Sat, 20 Sep 2025 20:30:48 +0530, Akhilesh Patil wrote:
> This patch series adds following to m41t93 rtc driver.
>
> Functionalities:
> - Alarm support (support to configure alarm 1)
> - Square wave output support
> - Watchdog support
>
> [...]
Applied, thanks!
[1/6] dt-bindings: rtc: Add ST m41t93
https://git.kernel.org/abelloni/c/b70598b3769e
[2/6] rtc: m41t93: add device tree support
https://git.kernel.org/abelloni/c/081e3de28c76
[3/6] rtc: m41t93: migrate to regmap api for register access
https://git.kernel.org/abelloni/c/d86e8682593c
[4/6] rtc: m41t93: Add alarm support
https://git.kernel.org/abelloni/c/5b55ff7cace4
[5/6] rtc: m41t93: Add square wave clock provider support
https://git.kernel.org/abelloni/c/5cb6816cd81b
[6/6] rtc: m41t93: Add watchdog support
https://git.kernel.org/abelloni/c/15ec2ce19962
Best regards,
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply
* [PATCH v3] ASoC: dt-bindings: Convert cirrus,cs35l36 to DT schema
From: David Heidelberg via B4 Relay @ 2026-06-24 21:10 UTC (permalink / raw)
To: David Rhodes, Richard Fitzgerald, Liam Girdwood, Mark Brown,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, patches,
Bjorn Helgaas
Cc: David Rhodes, linux-sound, devicetree, linux-kernel, phone-devel,
David Rhodes, David Heidelberg
From: David Heidelberg <david@ixit.cz>
Convert CS35L36 Speaker Amplifier to yaml.
Changes:
- maintainers email to the generic Cirrus email
- Both the codec and downstream worked just fine without
VP-supply provided. Align with datasheet for similar models.
- add dai-common.yaml to cover for '#sound-dai-cells',
'sound-name-prefix'
- updated not yet implemented:
cirrus,weak-fet-delay -> cirrus,classh-wk-fet-delay-ms
(in both definition and example)
cirrus,weak-fet-thld -> cirrus,weak-fet-thld-millivolt
(only in the example)
- added two required properties:
cirrus,boost-ctl-millivolt
cirrus,boost-peak-milliamp
Assisted-by: OpenAI:gpt-4
Reviewed-by: David Rhodes <David.Rhodes@cirrus.com>
Co-developed-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
Signed-off-by: David Heidelberg <david@ixit.cz>
---
Relevant for Pixel 3 / 3XL / 4.
---
Changes in v3:
- Corrected properties which has been unimplemented and also
conflicting between definition and the sample.
- Removed units from description where units are part of the property
name.
- Added sound into the $id path. (Mark)
- Readded James and removed Bjorn. (Rob)
- Made path to dai-common shorter.
- Added two required properties
cirrus,boost-ctl-millivolt and cirrus,boost-peak-milliamp.
- Use $defs for the cirrus,vpbr-config to prevent matching it against
vendor defined properties. The driver is already mainlined with
cirrus, vendor part, so we have to keep it compatible.
- Link to v2: https://lore.kernel.org/r/20260624-dt-cirrus-cs35l36-v2-1-74eccdbd8fe4@ixit.cz
Changes in v2:
- Rename the commit. (Mark)
- Link to v1: https://lore.kernel.org/r/20260618-dt-cirrus-cs35l36-v1-1-1a43515666ad@ixit.cz
---
.../devicetree/bindings/sound/cirrus,cs35l36.yaml | 240 +++++++++++++++++++++
.../devicetree/bindings/sound/cs35l36.txt | 168 ---------------
2 files changed, 240 insertions(+), 168 deletions(-)
diff --git a/Documentation/devicetree/bindings/sound/cirrus,cs35l36.yaml b/Documentation/devicetree/bindings/sound/cirrus,cs35l36.yaml
new file mode 100644
index 0000000000000..2a142b32acf50
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/cirrus,cs35l36.yaml
@@ -0,0 +1,240 @@
+# SPDX-License-Identifier: GPL-2.0-only
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/cirrus,cs35l36.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Cirrus Logic CS35L36 Speaker Amplifier
+
+maintainers:
+ - David Rhodes <drhodes@opensource.cirrus.com>
+ - patches@opensource.cirrus.com
+
+description:
+ CS35L36 is a boosted mono Class D amplifier
+
+allOf:
+ - $ref: dai-common.yaml#
+
+properties:
+ compatible:
+ enum:
+ - cirrus,cs35l36
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ VA-supply:
+ description: Voltage regulator of analog internal section
+
+ VP-supply:
+ description: Voltage regulator of boost converter
+
+ reset-gpios:
+ maxItems: 1
+
+ cirrus,boost-ctl-millivolt:
+ description: Boost converter output voltage (step 50)
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 2550
+ maximum: 12000
+
+ cirrus,boost-peak-milliamp:
+ description: Boost-converter peak current limit (step 50)
+ $ref: /schemas/types.yaml#/definitions/uint32
+ default: 4500
+ minimum: 1600
+ maximum: 4500
+
+ cirrus,boost-ind-nanohenry:
+ description: Initial inductor estimation reference value (1000=1μH, 1200=1.2μH)
+ $ref: /schemas/types.yaml#/definitions/uint32
+ default: 1000
+
+ cirrus,multi-amp-mode:
+ description: Hi-Z ASP port when more than one amplifier in system
+ type: boolean
+
+ cirrus,boost-ctl-select:
+ description: Boost converter control source selection
+ $ref: /schemas/types.yaml#/definitions/uint32
+ default: 1
+ enum:
+ - 0 # Control Port
+ - 1 # Class
+ - 2 # Sync
+
+ cirrus,amp-pcm-inv:
+ description: Invert incoming PCM data when true
+ type: boolean
+
+ cirrus,imon-pol-inv:
+ description: Invert polarity of outbound IMON feedback when true
+ type: boolean
+
+ cirrus,vmon-pol-inv:
+ description: Invert polarity of outbound VMON feedback when true
+ type: boolean
+
+ cirrus,dcm-mode-enable:
+ description: Enable boost converter automatic Discontinuous Conduction Mode
+ type: boolean
+
+ cirrus,weak-fet-disable:
+ description: Reduce output driver strength in Weak-FET Drive Mode when true
+ type: boolean
+
+ cirrus,classh-wk-fet-delay-ms:
+ description: Weak-FET entry delay
+ default: 100
+ enum: [0, 5, 10, 50, 100, 200, 500, 1000]
+
+ cirrus,classh-weak-fet-thld-millivolt:
+ description: Weak-FET drive threshold
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum: [50, 100, 150, 200, 250, 300, 350, 400, 450, 500, 550, 600, 650, 700]
+
+ cirrus,temp-warn-threshold:
+ description: Overtemperature warning threshold
+ $ref: /schemas/types.yaml#/definitions/uint32
+ default: 2
+ enum:
+ - 0 # 105°C
+ - 1 # 115°C
+ - 2 # 125°C
+ - 3 # 135°C
+
+ cirrus,irq-drive-select:
+ description: Interrupt output driver type
+ $ref: /schemas/types.yaml#/definitions/uint32
+ default: 1
+ enum:
+ - 0 # open-drain
+ - 1 # push-pull
+
+ cirrus,irq-gpio-select:
+ description: Programmable IRQ pin selection
+ $ref: /schemas/types.yaml#/definitions/uint32
+ enum:
+ - 0 # PDM_DATA/SWIRE_SD/INT
+ - 1 # GPIO
+
+ cirrus,vpbr-config:
+ $ref: "#/$defs/vpbr-config"
+
+$defs:
+ vpbr-config:
+ description: Brownout prevention configuration sub-node
+ type: object
+ additionalProperties: false
+
+ properties:
+ cirrus,vpbr-en:
+ description: VBST brownout prevention enable
+ $ref: /schemas/types.yaml#/definitions/uint32
+ default: 0
+ enum:
+ - 0 # disabled
+ - 1 # enabled
+
+ cirrus,vpbr-thld:
+ description: Initial VPBR threshold voltage
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 2
+ maximum: 31
+
+ cirrus,vpbr-atk-rate:
+ description: Attenuation attack step rate
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 7
+
+ cirrus,vpbr-atk-vol:
+ description: VP brownout prevention step size
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 7
+
+ cirrus,vpbr-max-attn:
+ description: Maximum attenuation during VP brownout prevention (dB)
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 15
+
+ cirrus,vpbr-wait:
+ description: Delay between brownout clearance and attenuation release (ms)
+ $ref: /schemas/types.yaml#/definitions/uint32
+ default: 1
+ enum:
+ - 0 # 10
+ - 1 # 100
+ - 2 # 250
+ - 3 # 500
+
+ cirrus,vpbr-rel-rate:
+ description: Attenuation release step rate
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 7
+
+ cirrus,vpbr-mute-en:
+ description: Mute audio if maximum attenuation reached
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - VA-supply
+ - cirrus,boost-ctl-millivolt
+ - cirrus,boost-peak-milliamp
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ codec@40 {
+ compatible = "cirrus,cs35l36";
+ reg = <0x40>;
+ VA-supply = <&dummy_vreg>;
+ VP-supply = <&dummy_vreg>;
+ reset-gpios = <&gpio0 54 GPIO_ACTIVE_HIGH>;
+ interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
+
+ cirrus,boost-ind-nanohenry = <1000>;
+ cirrus,boost-ctl-millivolt = <10000>;
+ cirrus,boost-peak-milliamp = <4500>;
+ cirrus,boost-ctl-select = <0>;
+ cirrus,classh-wk-fet-delay-ms = <100>;
+ cirrus,classh-weak-fet-thld-millivolt = <100>;
+ cirrus,temp-warn-threshold = <1>;
+ cirrus,multi-amp-mode;
+ cirrus,irq-drive-select = <1>;
+ cirrus,irq-gpio-select = <1>;
+
+ cirrus,vpbr-config {
+ cirrus,vpbr-en = <0>;
+ cirrus,vpbr-thld = <5>;
+ cirrus,vpbr-atk-rate = <2>;
+ cirrus,vpbr-atk-vol = <1>;
+ cirrus,vpbr-max-attn = <9>;
+ cirrus,vpbr-wait = <1>;
+ cirrus,vpbr-rel-rate = <5>;
+ cirrus,vpbr-mute-en = <0>;
+ };
+ };
+ };
+
+...
diff --git a/Documentation/devicetree/bindings/sound/cs35l36.txt b/Documentation/devicetree/bindings/sound/cs35l36.txt
deleted file mode 100644
index d34117b8558e5..0000000000000
--- a/Documentation/devicetree/bindings/sound/cs35l36.txt
+++ /dev/null
@@ -1,168 +0,0 @@
-CS35L36 Speaker Amplifier
-
-Required properties:
-
- - compatible : "cirrus,cs35l36"
-
- - reg : the I2C address of the device for I2C
-
- - VA-supply, VP-supply : power supplies for the device,
- as covered in
- Documentation/devicetree/bindings/regulator/regulator.txt.
-
- - cirrus,boost-ctl-millivolt : Boost Voltage Value. Configures the boost
- converter's output voltage in mV. The range is from 2550mV to 12000mV with
- increments of 50mV.
- (Default) VP
-
- - cirrus,boost-peak-milliamp : Boost-converter peak current limit in mA.
- Configures the peak current by monitoring the current through the boost FET.
- Range starts at 1600mA and goes to a maximum of 4500mA with increments of
- 50mA.
- (Default) 4.50 Amps
-
- - cirrus,boost-ind-nanohenry : Inductor estimation LBST reference value.
- Seeds the digital boost converter's inductor estimation block with the initial
- inductance value to reference.
-
- 1000 = 1uH (Default)
- 1200 = 1.2uH
-
-Optional properties:
- - cirrus,multi-amp-mode : Boolean to determine if there are more than
- one amplifier in the system. If more than one it is best to Hi-Z the ASP
- port to prevent bus contention on the output signal
-
- - cirrus,boost-ctl-select : Boost converter control source selection.
- Selects the source of the BST_CTL target VBST voltage for the boost
- converter to generate.
- 0x00 - Control Port Value
- 0x01 - Class H Tracking (Default)
- 0x10 - MultiDevice Sync Value
-
- - cirrus,amp-pcm-inv : Boolean to determine Amplifier will invert incoming
- PCM data
-
- - cirrus,imon-pol-inv : Boolean to determine Amplifier will invert the
- polarity of outbound IMON feedback data
-
- - cirrus,vmon-pol-inv : Boolean to determine Amplifier will invert the
- polarity of outbound VMON feedback data
-
- - cirrus,dcm-mode-enable : Boost converter automatic DCM Mode enable.
- This enables the digital boost converter to operate in a low power
- (Discontinuous Conduction) mode during low loading conditions.
-
- - cirrus,weak-fet-disable : Boolean : The strength of the output drivers is
- reduced when operating in a Weak-FET Drive Mode and must not be used to drive
- a large load.
-
- - cirrus,classh-wk-fet-delay : Weak-FET entry delay. Controls the delay
- (in ms) before the Class H algorithm switches to the weak-FET voltage
- (after the audio falls and remains below the value specified in WKFET_AMP_THLD).
-
- 0 = 0ms
- 1 = 5ms
- 2 = 10ms
- 3 = 50ms
- 4 = 100ms (Default)
- 5 = 200ms
- 6 = 500ms
- 7 = 1000ms
-
- - cirrus,classh-weak-fet-thld-millivolt : Weak-FET amplifier drive threshold.
- Configures the signal threshold at which the PWM output stage enters
- weak-FET operation. The range is 50mV to 700mV in 50mV increments.
-
- - cirrus,temp-warn-threshold : Amplifier overtemperature warning threshold.
- Configures the threshold at which the overtemperature warning condition occurs.
- When the threshold is met, the overtemperature warning attenuation is applied
- and the TEMP_WARN_EINT interrupt status bit is set.
- If TEMP_WARN_MASK = 0, INTb is asserted.
-
- 0 = 105C
- 1 = 115C
- 2 = 125C (Default)
- 3 = 135C
-
- - cirrus,irq-drive-select : Selects the driver type of the selected interrupt
- output.
-
- 0 = Open-drain
- 1 = Push-pull (Default)
-
- - cirrus,irq-gpio-select : Selects the pin to serve as the programmable
- interrupt output.
-
- 0 = PDM_DATA / SWIRE_SD / INT (Default)
- 1 = GPIO
-
-Optional properties for the "cirrus,vpbr-config" Sub-node
-
- - cirrus,vpbr-en : VBST brownout prevention enable. Configures whether the
- VBST brownout prevention algorithm is enabled or disabled.
-
- 0 = VBST brownout prevention disabled (default)
- 1 = VBST brownout prevention enabled
-
- See Section 7.31.1 VPBR Config for configuration options & further details
-
- - cirrus,vpbr-thld : Initial VPBR threshold. Configures the VP brownout
- threshold voltage
-
- - cirrus,cirrus,vpbr-atk-rate : Attenuation attack step rate. Configures the
- amount delay between consecutive volume attenuation steps when a brownout
- condition is present and the VP brownout condition is in an attacking state.
-
- - cirrus,vpbr-atk-vol : VP brownout prevention step size. Configures the VP
- brownout prevention attacking attenuation step size when operating in either
- digital volume or analog gain modes.
-
- - cirrus,vpbr-max-attn : Maximum attenuation that the VP brownout prevention
- can apply to the audio signal.
-
- - cirrus,vpbr-wait : Configures the delay time between a brownout condition
- no longer being present and the VP brownout prevention entering an attenuation
- release state.
-
- - cirrus,vpbr-rel-rate : Attenuation release step rate. Configures the delay
- between consecutive volume attenuation release steps when a brownout condition
- is not longer present and the VP brownout is in an attenuation release state.
-
- - cirrus,vpbr-mute-en : During the attack state, if the vpbr-max-attn value
- is reached, the error condition still remains, and this bit is set, the audio
- is muted.
-
-Example:
-
-cs35l36: cs35l36@40 {
- compatible = "cirrus,cs35l36";
- reg = <0x40>;
- VA-supply = <&dummy_vreg>;
- VP-supply = <&dummy_vreg>;
- reset-gpios = <&gpio0 54 0>;
- interrupt-parent = <&gpio8>;
- interrupts = <3 IRQ_TYPE_LEVEL_LOW>;
-
- cirrus,boost-ind-nanohenry = <1000>;
- cirrus,boost-ctl-millivolt = <10000>;
- cirrus,boost-peak-milliamp = <4500>;
- cirrus,boost-ctl-select = <0x00>;
- cirrus,weak-fet-delay = <0x04>;
- cirrus,weak-fet-thld = <0x01>;
- cirrus,temp-warn-threshold = <0x01>;
- cirrus,multi-amp-mode;
- cirrus,irq-drive-select = <0x01>;
- cirrus,irq-gpio-select = <0x01>;
-
- cirrus,vpbr-config {
- cirrus,vpbr-en = <0x00>;
- cirrus,vpbr-thld = <0x05>;
- cirrus,vpbr-atk-rate = <0x02>;
- cirrus,vpbr-atk-vol = <0x01>;
- cirrus,vpbr-max-attn = <0x09>;
- cirrus,vpbr-wait = <0x01>;
- cirrus,vpbr-rel-rate = <0x05>;
- cirrus,vpbr-mute-en = <0x00>;
- };
-};
---
base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6
change-id: 20260618-dt-cirrus-cs35l36-99c466fb13fd
Best regards,
--
David Heidelberg <david@ixit.cz>
^ permalink raw reply related
* [PATCH v1 3/3] arm64: dts: qcom: x1e78100-t14s: Add thermal zones for keyboard skin and charging sensors
From: Daniel Lezcano @ 2026-06-24 21:08 UTC (permalink / raw)
To: sre, hansg, ilpo.jarvinen, linux, andersson, konradybcio, robh,
krzk+dt, conor+dt
Cc: bryan.odonoghue, platform-driver-x86, linux-kernel, linux-hwmon,
linux-arm-msm, devicetree
In-Reply-To: <20260624210825.264454-1-daniel.lezcano@oss.qualcomm.com>
Add thermal zone descriptions for the keyboard skin temperature and
battery charging circuitry sensors exposed by the embedded controller.
Expose the EC as a thermal sensor provider and define two thermal zones
using the temperatures reported by the EC:
- a keyboard skin temperature zone with passive and hot trip points,
- a charging circuitry temperature zone with multiple passive trip
points and CPU frequency mitigation levels.
The charging thermal zone progressively throttles the different CPU
clusters as the charging circuitry temperature rises and triggers a hot
condition at the highest trip point.
This provides thermal framework integration for the EC temperature
sensors and enables platform thermal management through standard thermal
zone definitions.
Signed-off-by: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
---
.../qcom/x1e78100-lenovo-thinkpad-t14s.dtsi | 68 ++++++++++++++++++-
1 file changed, 67 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi
index 5d49df41be02..e27533665c8d 100644
--- a/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi
+++ b/arch/arm64/boot/dts/qcom/x1e78100-lenovo-thinkpad-t14s.dtsi
@@ -979,7 +979,7 @@ &i2c6 {
status = "okay";
- embedded-controller@28 {
+ ec: embedded-controller@28 {
compatible = "lenovo,thinkpad-t14s-ec";
reg = <0x28>;
@@ -988,6 +988,8 @@ embedded-controller@28 {
pinctrl-0 = <&ec_int_n_default>;
pinctrl-names = "default";
+ #thermal-sensor-cells = <1>;
+
wakeup-source;
};
};
@@ -1729,3 +1731,67 @@ &usb_mp_qmpphy1 {
status = "okay";
};
+
+&thermal_zones {
+ ec-keyboard-thermal {
+ polling-delay = <5000>;
+ polling-delay-passive = <1000>;
+
+ thermal-sensors = <&ec 1>;
+
+ trips {
+ trip-point0 {
+ temperature = <55000>;
+ hysteresis = <2000>;
+ type = "passive";
+ };
+
+ trip-point1 {
+ temperature = <62000>;
+ hysteresis = <0>;
+ type = "hot";
+ };
+ };
+ };
+
+ ec-charging-thermal {
+ polling-delay = <5000>;
+ polling-delay-passive = <2000>;
+
+ thermal-sensors = <&ec 3>;
+
+ trips {
+ psv0: trip-point0 {
+ temperature = <55000>;
+ hysteresis = <0>;
+ type = "passive";
+ };
+
+ alert0: trip-point1 {
+ temperature = <63000>;
+ hysteresis = <0>;
+ type = "hot";
+ };
+ };
+
+ cooling-maps {
+ map0 {
+ trip = <&psv0>;
+ cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu8 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu9 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu10 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
+ <&cpu11 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
+
+ };
+ };
+
+ };
+};
--
2.53.0
^ permalink raw reply related
* [PATCH v1 2/3] platform: arm64: thinkpad-t14s-ec: Wire EC thermal events to hwmon
From: Daniel Lezcano @ 2026-06-24 21:08 UTC (permalink / raw)
To: sre, hansg, ilpo.jarvinen, linux, andersson, konradybcio, robh,
krzk+dt, conor+dt
Cc: bryan.odonoghue, platform-driver-x86, linux-kernel, linux-hwmon,
linux-arm-msm, devicetree
In-Reply-To: <20260624210825.264454-1-daniel.lezcano@oss.qualcomm.com>
The EC generates thermal zone status change notifications for a subset
of the exposed temperature sensors. Wire these EC events to the hwmon
notification framework so userspace can be informed when a thermal alarm
state changes.
Associate each hwmon temperature channel with its corresponding EC
thermal event and emit hwmon_temp_alarm notifications through
hwmon_notify_event() when the EC reports a thermal zone status change.
Also register thermal zones in the hwmon chip capabilities and keep a
reference to the hwmon device to allow event propagation from the IRQ
handler.
This allows userspace monitoring tools to receive thermal alarm
updates without polling the sensors and gives the opportuniy to the
kernel to cool them down.
Signed-off-by: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
---
drivers/platform/arm64/lenovo-thinkpad-t14s.c | 66 ++++++++++++++-----
1 file changed, 51 insertions(+), 15 deletions(-)
diff --git a/drivers/platform/arm64/lenovo-thinkpad-t14s.c b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
index 142464623f0e..276bb51da33a 100644
--- a/drivers/platform/arm64/lenovo-thinkpad-t14s.c
+++ b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
@@ -107,10 +107,13 @@ struct t14s_ec_led_classdev {
struct t14s_ec_hwmon_sys_thermx {
const char *label;
int reg;
+ u8 event;
};
struct t14s_ec_hwmon {
+ struct device *dev;
struct t14s_ec_hwmon_sys_thermx *sys_thermx;
+ size_t num_sys_thermx;
};
struct t14s_ec {
@@ -493,6 +496,20 @@ static int t14s_input_probe(struct t14s_ec *ec)
return input_register_device(ec->inputdev);
}
+static void t14s_ec_hwmon_notify_event(struct t14s_ec *ec, u8 event)
+{
+ for (int i = 0; i < ec->ec_hwmon.num_sys_thermx; i++) {
+ if (ec->ec_hwmon.sys_thermx[i].event != event)
+ continue;
+
+ hwmon_notify_event(ec->ec_hwmon.dev, hwmon_temp,
+ hwmon_temp_alarm, i);
+
+ dev_dbg(ec->dev, "Thermal Zone (%s) Status Change Event\n",
+ ec->ec_hwmon.sys_thermx[i].label);
+ }
+}
+
static irqreturn_t t14s_ec_irq_handler(int irq, void *data)
{
struct t14s_ec *ec = data;
@@ -542,13 +559,9 @@ static irqreturn_t t14s_ec_irq_handler(int irq, void *data)
dev_dbg(ec->dev, "LID closed\n");
break;
case T14S_EC_EVT_THERMAL_TZ40:
- dev_dbg(ec->dev, "Thermal Zone 40 Status Change Event (CPU/GPU)\n");
- break;
case T14S_EC_EVT_THERMAL_TZ42:
- dev_dbg(ec->dev, "Thermal Zone 42 Status Change Event (Battery)\n");
- break;
case T14S_EC_EVT_THERMAL_TZ39:
- dev_dbg(ec->dev, "Thermal Zone 39 Status Change Event (CPU/GPU)\n");
+ t14s_ec_hwmon_notify_event(ec, val);
break;
case T14S_EC_EVT_KEY_FN_G:
dev_dbg(ec->dev, "FN + G - toggle double-tapping\n");
@@ -658,6 +671,7 @@ static const struct hwmon_ops t14s_ec_hwmon_ops = {
};
static const struct hwmon_channel_info *t14s_ec_hwmon_info[] = {
+ HWMON_CHANNEL_INFO(chip, HWMON_C_REGISTER_TZ),
HWMON_CHANNEL_INFO(temp,
HWMON_T_INPUT | HWMON_T_LABEL,
HWMON_T_INPUT | HWMON_T_LABEL,
@@ -676,14 +690,34 @@ static const struct hwmon_chip_info t14s_ec_chip_info = {
static int t14s_ec_hwmon_probe(struct t14s_ec *ec)
{
- struct device *dev;
struct t14s_ec_hwmon_sys_thermx sys_thermx[] = {
- { T14S_EC_SYS_THERM0, "soc" },
- { T14S_EC_SYS_THERM1, "keyboard" },
- { T14S_EC_SYS_THERM2, "base" },
- { T14S_EC_SYS_THERM3, "pmbm" },
- { T14S_EC_SYS_THERM6, "qtm" },
- { T14S_EC_SYS_THERM7, "ssd" },
+ {
+ .label = "soc",
+ .reg = T14S_EC_SYS_THERM0,
+ .event = T14S_EC_EVT_THERMAL_TZ39
+ },
+ {
+ .label = "keyboard",
+ .reg = T14S_EC_SYS_THERM1,
+ .event = T14S_EC_EVT_THERMAL_TZ40
+ },
+ {
+ .label = "base",
+ .reg = T14S_EC_SYS_THERM2,
+ },
+ {
+ .label = "pmbm",
+ .reg = T14S_EC_SYS_THERM3,
+ .event = T14S_EC_EVT_THERMAL_TZ42
+ },
+ {
+ .label = "qtm",
+ .reg = T14S_EC_SYS_THERM6
+ },
+ {
+ .label = "ssd",
+ .reg = T14S_EC_SYS_THERM7
+ },
};
ec->ec_hwmon.sys_thermx = devm_kmemdup_array(ec->dev, sys_thermx,
@@ -692,10 +726,12 @@ static int t14s_ec_hwmon_probe(struct t14s_ec *ec)
if (!ec->ec_hwmon.sys_thermx)
return -ENOMEM;
- dev = devm_hwmon_device_register_with_info(ec->dev, "t14s_ec", ec,
- &t14s_ec_chip_info, NULL);
+ ec->ec_hwmon.num_sys_thermx = ARRAY_SIZE(sys_thermx);
+
+ ec->ec_hwmon.dev = devm_hwmon_device_register_with_info(ec->dev, "t14s_ec", ec,
+ &t14s_ec_chip_info, NULL);
- return PTR_ERR_OR_ZERO(dev);
+ return PTR_ERR_OR_ZERO(ec->ec_hwmon.dev);
}
static int t14s_ec_probe(struct i2c_client *client)
--
2.53.0
^ permalink raw reply related
* [PATCH v1 1/3] drivers/platform: lenovo-t14s-ec: Add hwmon support for temperatures and fan speed
From: Daniel Lezcano @ 2026-06-24 21:08 UTC (permalink / raw)
To: sre, hansg, ilpo.jarvinen, linux, andersson, konradybcio, robh,
krzk+dt, conor+dt
Cc: bryan.odonoghue, platform-driver-x86, linux-kernel, linux-hwmon,
linux-arm-msm, devicetree
In-Reply-To: <20260624210825.264454-1-daniel.lezcano@oss.qualcomm.com>
Expose the Lenovo ThinkPad T14s EC environmental sensors through
the hwmon subsystem.
The driver now registers a hwmon device providing access to six EC
temperature sensors corresponding to the SoC, keyboard area, base
cover, PMIC/charging circuitry, QTM module and SSD. Sensor labels
are exported to allow user space to identify each measurement.
Additionally, expose the system fan speed by reading the fan RPM
registers from the embedded controller.
This allows standard monitoring tools such as lm-sensors to report
platform temperatures and fan speed.
Signed-off-by: Daniel Lezcano daniel.lezcano@oss.qualcomm.com
---
drivers/platform/arm64/lenovo-thinkpad-t14s.c | 147 ++++++++++++++++++
1 file changed, 147 insertions(+)
diff --git a/drivers/platform/arm64/lenovo-thinkpad-t14s.c b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
index 5590302a5694..142464623f0e 100644
--- a/drivers/platform/arm64/lenovo-thinkpad-t14s.c
+++ b/drivers/platform/arm64/lenovo-thinkpad-t14s.c
@@ -11,6 +11,7 @@
#include <linux/delay.h>
#include <linux/dev_printk.h>
#include <linux/err.h>
+#include <linux/hwmon.h>
#include <linux/i2c.h>
#include <linux/input.h>
#include <linux/input/sparse-keymap.h>
@@ -67,6 +68,16 @@
#define T14S_EC_EVT_KEY_FN_F11 0x7a
#define T14S_EC_EVT_KEY_FN_G 0x7e
+#define T14S_EC_SYS_THERM0 0x78 /* SoC (CPU+GPU) */
+#define T14S_EC_SYS_THERM1 0x79 /* Keyboard */
+#define T14S_EC_SYS_THERM2 0x7a /* Back cover */
+#define T14S_EC_SYS_THERM3 0x7b /* Charger / PMIC */
+#define T14S_EC_SYS_THERM6 0x7c /* QTM West */
+#define T14S_EC_SYS_THERM7 0x7d /* SSD */
+
+#define T14S_EC_FAN_RPM_LSB 0x84
+#define T14S_EC_FAN_RPM_MSB 0x85
+
/* Hardware LED blink rate is 1 Hz (500ms off, 500ms on) */
#define T14S_EC_BLINK_RATE_ON_OFF_MS 500
@@ -93,9 +104,19 @@ struct t14s_ec_led_classdev {
struct t14s_ec *ec;
};
+struct t14s_ec_hwmon_sys_thermx {
+ const char *label;
+ int reg;
+};
+
+struct t14s_ec_hwmon {
+ struct t14s_ec_hwmon_sys_thermx *sys_thermx;
+};
+
struct t14s_ec {
struct regmap *regmap;
struct device *dev;
+ struct t14s_ec_hwmon ec_hwmon;
struct t14s_ec_led_classdev led_pwr_btn;
struct t14s_ec_led_classdev led_chrg_orange;
struct t14s_ec_led_classdev led_chrg_white;
@@ -555,6 +576,128 @@ static irqreturn_t t14s_ec_irq_handler(int irq, void *data)
return IRQ_HANDLED;
}
+static umode_t t14s_ec_hwmon_is_visible(const void *drvdata,
+ enum hwmon_sensor_types type,
+ u32 attr, int channel)
+{
+ switch (type) {
+ case hwmon_temp:
+ return 0444;
+ case hwmon_fan:
+ return 0444;
+ default:
+ return 0;
+ }
+}
+
+static int t14s_ec_hwmon_read_string(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, const char **str)
+{
+ struct t14s_ec *ec = dev_get_drvdata(dev);
+ switch (type) {
+ case hwmon_temp:
+ if (attr == hwmon_temp_label) {
+ *str = ec->ec_hwmon.sys_thermx[channel].label;
+ return 0;
+ }
+ break;
+ default:
+ return -EOPNOTSUPP;
+ }
+
+ return -EOPNOTSUPP;
+}
+
+static int t14s_ec_hwmon_read(struct device *dev, enum hwmon_sensor_types type,
+ u32 attr, int channel, long *val)
+{
+ struct t14s_ec *ec = dev_get_drvdata(dev);
+ unsigned int value;
+ int ret;
+
+ switch (type) {
+ case hwmon_temp:
+ if (attr == hwmon_temp_input) {
+ ret = t14s_ec_read(ec, ec->ec_hwmon.sys_thermx[channel].reg, &value);
+ if (ret)
+ return ret;
+ *val = value * 1000;
+
+ return 0;
+ }
+ break;
+
+ case hwmon_fan:
+ if (attr == hwmon_fan_input) {
+ int lsb, msb;
+ ret = t14s_ec_read(ec, T14S_EC_FAN_RPM_LSB, &lsb);
+ if (ret)
+ return ret;
+
+ ret = t14s_ec_read(ec, T14S_EC_FAN_RPM_MSB, &msb);
+ if (ret)
+ return ret;
+
+ *val = 0;
+ *val = lsb + (msb << 8);
+
+ return 0;
+ }
+ break;
+ default:
+ break;
+ }
+
+ return -EOPNOTSUPP;
+}
+
+static const struct hwmon_ops t14s_ec_hwmon_ops = {
+ .is_visible = t14s_ec_hwmon_is_visible,
+ .read = t14s_ec_hwmon_read,
+ .read_string = t14s_ec_hwmon_read_string,
+};
+
+static const struct hwmon_channel_info *t14s_ec_hwmon_info[] = {
+ HWMON_CHANNEL_INFO(temp,
+ HWMON_T_INPUT | HWMON_T_LABEL,
+ HWMON_T_INPUT | HWMON_T_LABEL,
+ HWMON_T_INPUT | HWMON_T_LABEL,
+ HWMON_T_INPUT | HWMON_T_LABEL,
+ HWMON_T_INPUT | HWMON_T_LABEL,
+ HWMON_T_INPUT | HWMON_T_LABEL),
+ HWMON_CHANNEL_INFO(fan, HWMON_F_INPUT),
+ NULL
+};
+
+static const struct hwmon_chip_info t14s_ec_chip_info = {
+ .ops = &t14s_ec_hwmon_ops,
+ .info = t14s_ec_hwmon_info,
+};
+
+static int t14s_ec_hwmon_probe(struct t14s_ec *ec)
+{
+ struct device *dev;
+ struct t14s_ec_hwmon_sys_thermx sys_thermx[] = {
+ { T14S_EC_SYS_THERM0, "soc" },
+ { T14S_EC_SYS_THERM1, "keyboard" },
+ { T14S_EC_SYS_THERM2, "base" },
+ { T14S_EC_SYS_THERM3, "pmbm" },
+ { T14S_EC_SYS_THERM6, "qtm" },
+ { T14S_EC_SYS_THERM7, "ssd" },
+ };
+
+ ec->ec_hwmon.sys_thermx = devm_kmemdup_array(ec->dev, sys_thermx,
+ ARRAY_SIZE(sys_thermx),
+ sizeof(sys_thermx[0]), GFP_KERNEL);
+ if (!ec->ec_hwmon.sys_thermx)
+ return -ENOMEM;
+
+ dev = devm_hwmon_device_register_with_info(ec->dev, "t14s_ec", ec,
+ &t14s_ec_chip_info, NULL);
+
+ return PTR_ERR_OR_ZERO(dev);
+}
+
static int t14s_ec_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
@@ -590,6 +733,10 @@ static int t14s_ec_probe(struct i2c_client *client)
if (ret < 0)
return ret;
+ ret = t14s_ec_hwmon_probe(ec);
+ if (ret < 0)
+ return ret;
+
ret = devm_request_threaded_irq(dev, client->irq, NULL,
t14s_ec_irq_handler,
IRQF_ONESHOT, dev_name(dev), ec);
--
2.53.0
^ permalink raw reply related
* [PATCH v1 0/3] Lenovo ThinkPad T14s EC thermal monitoring and thermal zone integration
From: Daniel Lezcano @ 2026-06-24 21:08 UTC (permalink / raw)
To: sre, hansg, ilpo.jarvinen, linux, andersson, konradybcio, robh,
krzk+dt, conor+dt
Cc: bryan.odonoghue, platform-driver-x86, linux-kernel, linux-hwmon,
linux-arm-msm, devicetree
Hi,
This series extends the Lenovo ThinkPad T14s embedded controller driver
with environmental monitoring capabilities and integrates the exposed
sensors into the Linux thermal framework.
The EC provides access to several platform temperature sensors covering
the SoC, keyboard area, bottom cover, charging circuitry, QTM module and
SSD, as well as fan speed information. These sensors are currently used
by the firmware for thermal management but are not exposed to Linux.
The first patch adds hwmon support for the EC temperature sensors and fan
RPM reporting.
The second patch wires EC thermal notifications into the hwmon event
framework, allowing userspace to receive alarm notifications when the EC
reports thermal zone state changes.
The last patch exposes the EC as a thermal sensor provider in the device
tree and defines thermal zones for the keyboard skin temperature and the
charging circuitry temperature. This allows the generic thermal
framework to react to EC-reported temperatures and apply standard Linux
thermal mitigation policies.
With the latest patch, this series fixes thermal issues happening on
this platform where a kernel compilation leads to a system reboot.
Tested on a Lenovo ThinkPad T14s Gen 6 (Snapdragon X Elite).
Thanks,
Daniel
Daniel Lezcano (3):
drivers/platform: lenovo-t14s-ec: Add hwmon support for temperatures
and fan speed
platform: arm64: thinkpad-t14s-ec: Wire EC thermal events to hwmon
arm64: dts: qcom: x1e78100-t14s: Add thermal zones for keyboard skin
and charging sensors
.../qcom/x1e78100-lenovo-thinkpad-t14s.dtsi | 68 +++++-
drivers/platform/arm64/lenovo-thinkpad-t14s.c | 193 +++++++++++++++++-
2 files changed, 255 insertions(+), 6 deletions(-)
--
2.53.0
^ permalink raw reply
* Re: [PATCH v6 5/9] media: synopsys: Use media_async_register_subdev() to simplify code
From: sashiko-bot @ 2026-06-24 21:07 UTC (permalink / raw)
To: Frank.Li; +Cc: devicetree, Frank.Li, robh, imx, conor+dt
In-Reply-To: <20260624-imx8qxp_pcam-v6-5-4b3f45920d2f@nxp.com>
Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
New issues:
- [Critical] Critical out-of-bounds heap write in v4l2_async_nf_parse_fwnode() due to mismatched endpoint counting.
- [High] Mixed goto-based error handling and __free() cleanup violates subsystem rules and leaves a dangling pointer.
- [High] Hardcoded array indexing for pads mismatches the dynamic Device Tree enumeration order.
- [High] Heap out-of-bounds access and routing validation failure on single-endpoint Device Trees.
- [High] Memory leak of `sd->active_state` on subdev registration failure.
- [High] Unconditional memory leak of `sd->subdev_notifier` during driver removal.
- [High] Leaked async connections and fwnode references on error path.
Pre-existing issues:
- [High] Lockless traversal of the media entity links list causes a race condition.
--
commit 892732bd4672cbe12751cfc2979a3f508c5cde6d
Author: Frank Li <Frank.Li@nxp.com>
Subject: media: synopsys: Use media_async_register_subdev() to simplify code
Use the media_async_register_subdev() to simplify the driver.
Replace the local subdev registration and media pad setup code with
media_async_register_subdev(). Reduce boilerplate code and aligns the
driver with the common pattern used by simple subdevices that each media
pad has one endpoint in fwnode.
> diff --git a/drivers/media/platform/synopsys/dw-mipi-csi2rx.c b/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
> index f51367409ff46..0fabc89a49b80 100644
> --- a/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
> +++ b/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
[ ... ]
> @@ -360,9 +350,10 @@ dw_mipi_csi2rx_find_format(struct dw_mipi_csi2rx_device *csi2, u32 mbus_code)
>
> static int dw_mipi_csi2rx_start(struct dw_mipi_csi2rx_device *csi2)
> {
> + struct media_pad *sink_pad = &csi2->sd.entity.pads[V4L2_SUBDEV_1TO1_PADS_SINK];
[Severity: High]
Does this hardcoded index match the dynamic pad allocation from the Device
Tree?
Because v4l2_async_nf_parse_fwnode() populates the pads array strictly in the
order endpoints appear in the Device Tree, accessing index
V4L2_SUBDEV_1TO1_PADS_SINK unconditionally here assumes the sink pad is always
at index 0.
If a platform's Device Tree lists the source endpoint before the sink
endpoint, will the driver access the wrong pad and potentially read incorrect
hardware parameters (like lane counts)?
> + u32 lanes = sink_pad->vep.bus.mipi_csi2.num_data_lanes;
> struct media_pad *source_pad;
[ ... ]
> @@ -549,12 +539,12 @@ static int dw_mipi_csi2rx_enable_streams(struct v4l2_subdev *sd,
> u64 mask;
> int ret;
>
> - sink_pad = &sd->entity.pads[DW_MIPI_CSI2RX_PAD_SINK];
> + sink_pad = &sd->entity.pads[V4L2_SUBDEV_1TO1_PADS_SINK];
> remote_pad = media_pad_remote_pad_first(sink_pad);
[Severity: High]
This isn't a bug introduced by this patch, but does this lockless list
traversal race with userspace ioctls?
Calling media_pad_remote_pad_first() iterates the media entity links list:
drivers/media/mc/mc-entity.c:media_pad_remote_pad_first() {
...
for_each_media_entity_data_link(pad->entity, link) {
...
}
Since v4l2_subdev_enable_streams() does not acquire the global
mdev->graph_mutex, could this traversal race with userspace ioctls like
MEDIA_IOC_SETUP_LINK that mutate the graph structure, leading to linked list
corruption or a use-after-free?
> remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
[ ... ]
> @@ -649,9 +639,9 @@ static int dw_mipi_csi2rx_init_state(struct v4l2_subdev *sd,
> {
> struct v4l2_subdev_route routes[] = {
> {
> - .sink_pad = DW_MIPI_CSI2RX_PAD_SINK,
> + .sink_pad = V4L2_SUBDEV_1TO1_PADS_SINK,
> .sink_stream = 0,
> - .source_pad = DW_MIPI_CSI2RX_PAD_SRC,
> + .source_pad = V4L2_SUBDEV_1TO1_PADS_SOURCE,
[Severity: High]
Could this cause an out-of-bounds heap access if the Device Tree only defines
a single endpoint?
When __media_async_register_subdev() dynamically sizes the pads array based
on ep_count, a Device Tree with only a sink endpoint will result in an array
of size 1.
Since the driver unconditionally sets up active routes referring to
V4L2_SUBDEV_1TO1_PADS_SOURCE (index 1), won't this attempt to access pads[1]
on a size-1 array?
> .source_stream = 0,
> .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
> },
[ ... ]
> @@ -764,45 +697,12 @@ static int dw_mipi_csi2rx_register(struct dw_mipi_csi2rx_device *csi2)
> snprintf(sd->name, sizeof(sd->name), "dw-mipi-csi2rx %s",
> dev_name(csi2->dev));
>
> - pads[DW_MIPI_CSI2RX_PAD_SINK].flags = MEDIA_PAD_FL_SINK |
> - MEDIA_PAD_FL_MUST_CONNECT;
> - pads[DW_MIPI_CSI2RX_PAD_SRC].flags = MEDIA_PAD_FL_SOURCE;
> - ret = media_entity_pads_init(&sd->entity, DW_MIPI_CSI2RX_PAD_MAX, pads);
> + ret = media_async_register_subdev(&csi2->sd);
[Severity: High]
Does calling media_async_register_subdev() here expose a dangling pointer due
to mixed goto and __free() usage in __media_async_register_subdev()?
In drivers/media/v4l2-core/v4l2-fwnode.c:__media_async_register_subdev(), the
pads array is bound to scope-based cleanup and assigned to sd->entity.pads:
struct media_pad *pads __free(kfree) = kzalloc_objs(struct media_pad,
ep_count);
...
sd->entity.pads = pads;
If __v4l2_async_register_subdev() fails, the code jumps to out_unregister:
out_unregister:
v4l2_async_nf_unregister(notifier);
Because the function returns, pads is automatically freed, but sd->entity.pads
is not set to NULL. Does this leave a dangling pointer in the subdevice?
[Severity: High]
Could media_async_register_subdev() leak the active state if registration
fails?
In __media_async_register_subdev(), v4l2_subdev_init_finalize() dynamically
allocates sd->active_state. If the subsequent call to
__v4l2_async_register_subdev() fails:
ret = v4l2_subdev_init_finalize(sd);
if (ret)
goto out_unregister;
ret = __v4l2_async_register_subdev(sd, module);
if (ret < 0)
goto out_unregister;
The error path jumps to out_unregister and returns without calling
v4l2_subdev_cleanup(). Does this bypass freeing the active state?
[Severity: Critical]
Can this call trigger a heap out-of-bounds write if the Device Tree contains
disabled or disconnected endpoints?
In __media_async_register_subdev(), the pads array is allocated based on
ep_count, which only counts available connected endpoints (flags=0):
struct media_pad *pads __free(kfree) = kzalloc_objs(struct media_pad,
ep_count);
However, v4l2_async_nf_parse_fwnode() iterates over all endpoints without
filtering:
fwnode_graph_for_each_endpoint_scoped(dev_fwnode(dev), ep) {
...
ret = v4l2_fwnode_endpoint_parse(ep, &pad->vep);
If there are disconnected endpoints in the Device Tree, the loop will iterate
more times than ep_count. Will this increment the pad pointer beyond the
allocated array and overwrite adjacent memory?
[Severity: High]
Does media_async_register_subdev() leak async connections and fwnode
references if an error occurs during endpoint parsing?
In v4l2_async_nf_parse_fwnode(), newly allocated async connections are added
to the notifier. If an error occurs in a subsequent loop iteration:
if (IS_ERR(asd))
return dev_err_probe(dev, PTR_ERR(asd), "failed to add notifier\n");
The function returns directly. Then in __media_async_register_subdev():
ret = v4l2_async_nf_parse_fwnode(dev, pads, notifier);
if (ret < 0)
return ret;
This bypasses the out_cleanup label and fails to call v4l2_async_nf_cleanup(),
which seems to leak everything added to the notifier so far.
> if (ret)
> - goto err_notifier_unregister;
[ ... ]
> @@ -993,7 +895,7 @@ static void dw_mipi_csi2rx_remove(struct platform_device *pdev)
> {
> struct dw_mipi_csi2rx_device *csi2 = platform_get_drvdata(pdev);
>
> - dw_mipi_csi2rx_unregister(csi2);
> + media_async_subdev_cleanup(&csi2->sd);
[Severity: High]
Does media_async_subdev_cleanup() unconditionally leak the subdevice notifier?
The function __media_async_register_subdev() dynamically allocates the
notifier and assigns it to sd->subdev_notifier. During teardown here,
media_async_subdev_cleanup() calls v4l2_async_nf_cleanup() to clean the lists:
drivers/media/v4l2-core/v4l2-fwnode.c:media_async_subdev_cleanup() {
...
v4l2_async_nf_cleanup(sd->subdev_notifier);
...
}
However, it does not appear to call kfree(sd->subdev_notifier). Will this
leak the notifier structure every time the module is unbound or removed?
> phy_exit(csi2->phy);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624-imx8qxp_pcam-v6-0-4b3f45920d2f@nxp.com?part=5
^ permalink raw reply
* Re: [PATCH v6 7/9] media: nxp: add V4L2 subdev driver for camera parallel interface (CPI)
From: sashiko-bot @ 2026-06-24 21:03 UTC (permalink / raw)
To: Frank.Li; +Cc: conor+dt, devicetree, imx, robh, Frank.Li
In-Reply-To: <20260624-imx8qxp_pcam-v6-7-4b3f45920d2f@nxp.com>
Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [High] Hardcoded pad indices lead to a NULL pointer dereference in `imx_cpi_set_fmt()` if the device tree specifies fewer than 2 endpoints.
- [High] Hardcoded pad indices cause memory corruption and use of uninitialized flags if device tree endpoints are reordered.
- [High] Pending asynchronous PM runtime suspend is cancelled during device removal, permanently leaking hardware clocks.
- [Medium] The Kconfig symbol `VIDEO_IMX_PARALLEL_CPI` selects a non-existent symbol `V4L2_1TO1`.
- [Medium] Unchecked return value from `of_device_get_match_data()` leads to a NULL pointer dereference if bound via sysfs.
- [Medium] Incorrect pad arguments passed to `v4l2_subdev_state_xlate_streams()`.
- [Low] The V2 register macros for `CPI_CTRL_V2_REG1` overlap at bit 16 and exceed the 32-bit register width.
--
commit 547923ad3a1ab4157dbe156e0b9151600e5bb65f
Author: Alice Yuan <alice.yuan@nxp.com>
media: nxp: add V4L2 subdev driver for camera parallel interface (CPI)
This commit adds a V4L2 sub-device driver for the CPI controller found
on i.MX8QXP, i.MX8QM, and i.MX93 SoCs. It supports parallel camera
sensors and enables image data capture through a parallel interface.
> diff --git a/drivers/media/platform/nxp/Kconfig b/drivers/media/platform/nxp/Kconfig
> index 40e3436669e21..90f7c792003f2 100644
> --- a/drivers/media/platform/nxp/Kconfig
> +++ b/drivers/media/platform/nxp/Kconfig
> @@ -39,6 +39,18 @@ config VIDEO_IMX_MIPI_CSIS
[ ... ]
> +config VIDEO_IMX_PARALLEL_CPI
> + tristate "NXP i.MX8/i.MX9 Parallel CPI Driver"
> + depends on ARCH_MXC || COMPILE_TEST
> + depends on VIDEO_DEV
> + select MEDIA_CONTROLLER
> + select V4L2_1TO1
[Severity: Medium]
Is this selecting a non-existent Kconfig symbol? V4L2_1TO1 does not appear
to exist in the kernel tree. Selecting a missing symbol can cause unmet
dependency warnings or silent build failures.
> + select V4L2_FWNODE
> + select VIDEO_V4L2_SUBDEV_API
> + help
> + Video4Linux2 sub-device driver for PARALLEL CPI receiver found
> + on some iMX8 and iMX9 SoCs.
> diff --git a/drivers/media/platform/nxp/imx-parallel-cpi.c b/drivers/media/platform/nxp/imx-parallel-cpi.c
> new file mode 100644
> index 0000000000000..0f74b51608715
> --- /dev/null
> +++ b/drivers/media/platform/nxp/imx-parallel-cpi.c
> @@ -0,0 +1,629 @@
[ ... ]
> +/* CPI INTERFACE CONTROL REG1 */
> +#define CPI_CTRL_REG1_PIXEL_WIDTH(v) FIELD_PREP(GENMASK(15, 0), (v))
> +#define CPI_CTRL_REG1_VSYNC_PULSE(v) FIELD_PREP(GENMASK(31, 16), (v))
> +
> +#define CPI_CTRL_V2_REG1_PIXEL_WIDTH(v) FIELD_PREP(GENMASK(16, 0), (v))
> +#define CPI_CTRL_V2_REG1_VSYNC_PULSE(v) FIELD_PREP(GENMASK(31, 16), (v))
[Severity: Low]
Do these V2 register macros overlap? The GENMASK(16, 0) and GENMASK(31, 16)
overlap at bit 16, resulting in a 33-bit span for what appears to be a
32-bit register. While currently unused, could this corrupt register
values if utilized in the future?
> +
> +/* Need match field DATA_TYPE_IN definition at CPI CTRL register */
[ ... ]
> +static void imx_cpi_hw_config(struct imx_cpi_device *pcpidev,
> + const struct imx_cpi_pix_format *pcpidev_fmt)
> +{
> + u32 flags = pcpidev->sd.entity.pads[V4L2_SUBDEV_1TO1_PADS_SINK].vep.bus.parallel.flags;
[Severity: High]
Can hardcoding the pad index lead to reading uninitialized memory here?
The pads array is populated based on the order endpoints appear in the
Device Tree. If a device tree defines the source endpoint before the
sink endpoint, pads[0] will correspond to the source pad instead of
the sink pad.
> + const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
> + bool hsync_pol = flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH;
[ ... ]
> +static int imx_cpi_set_fmt(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *sd_state,
> + struct v4l2_subdev_format *sdformat)
> +{
> + struct imx_cpi_pix_format const *pcpidev_fmt;
> + struct v4l2_mbus_framefmt *fmt;
[ ... ]
> + /* Propagate the format from sink to source. */
> + fmt = v4l2_subdev_state_get_format(sd_state, V4L2_SUBDEV_1TO1_PADS_SOURCE);
> + *fmt = sdformat->format;
[Severity: High]
Can this result in a NULL pointer dereference?
If a misconfigured device tree only specifies a single endpoint,
media_async_register_subdev() will allocate exactly one pad, meaning
sd->entity.num_pads = 1.
When v4l2_subdev_state_get_format() is explicitly called with
V4L2_SUBDEV_1TO1_PADS_SOURCE (index 1), it will return NULL since the
index is out of bounds. The pointer is then blindly dereferenced.
> +
> + /* The format on the source pad might change due to unpacking. */
> + fmt->code = pcpidev_fmt->output;
> +
> + return 0;
> +}
[ ... ]
> +static int imx_cpi_enable_streams(struct v4l2_subdev *sd,
> + struct v4l2_subdev_state *state, u32 pad,
> + u64 streams_mask)
> +{
> + struct imx_cpi_device *pcpidev = sd_to_imx_cpi_device(sd);
> + const struct imx_cpi_pix_format *pcpidev_fmt;
> + const struct v4l2_mbus_framefmt *format;
> + struct media_pad *sink_pad, *remote_pad;
> + struct device *dev = pcpidev->dev;
> + struct v4l2_subdev *remote_sd;
> + u64 mask;
> + int ret;
> +
> + sink_pad = &sd->entity.pads[V4L2_SUBDEV_1TO1_PADS_SINK];
> + remote_pad = media_pad_remote_pad_first(sink_pad);
> + remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
[Severity: High]
If the device tree endpoints are reordered as mentioned previously, can
this cast the wrong entity type to a v4l2_subdev?
If pads[0] corresponds to the source pad, its remote entity will be a
video_device, not a v4l2_subdev. Calling media_entity_to_v4l2_subdev()
on a video_device produces an invalid pointer that will crash the kernel
when dereferenced.
> +
> + mask = v4l2_subdev_state_xlate_streams(state, V4L2_SUBDEV_1TO1_PADS_SINK,
> + V4L2_SUBDEV_1TO1_PADS_SOURCE,
> + &streams_mask);
[Severity: Medium]
Are the pad arguments reversed here?
Since v4l2_subdev_enable_streams() is called on the source pad,
streams_mask represents streams on the source pad. Passing
V4L2_SUBDEV_1TO1_PADS_SINK as the first argument informs the helper that
streams_mask belongs to the sink pad. Could this silently break routing
logic if asymmetrical stream IDs are ever used?
> +
> + format = v4l2_subdev_state_get_format(state, V4L2_SUBDEV_1TO1_PADS_SINK);
[ ... ]
> +static void imx_cpi_remove(struct platform_device *pdev)
> +{
> + struct imx_cpi_device *pcpidev = platform_get_drvdata(pdev);
> +
> + media_async_subdev_cleanup(&pcpidev->sd);
> +}
[Severity: High]
Could this leak hardware clocks if an asynchronous suspend is pending?
When streams are disabled, pm_runtime_put_autosuspend() is called to
queue an async suspend. If the driver is unbound before the async work
executes, imx_cpi_remove() will run.
Since the driver relies on devm_pm_runtime_enable() for PM cleanup,
pm_runtime_disable() will flush and cancel the pending async suspend,
which leaves the device permanently powered on. Should this call
pm_runtime_put_sync() or explicitly suspend the device if an async put
was previously used?
> +
> +static int imx_cpi_probe(struct platform_device *pdev)
> +{
> + struct imx_cpi_device *pcpidev;
> + struct device *dev = &pdev->dev;
> + struct v4l2_subdev *sd;
> + int ret;
> +
> + pcpidev = devm_kzalloc(dev, sizeof(*pcpidev), GFP_KERNEL);
> + if (!pcpidev)
> + return -ENOMEM;
> +
> + pcpidev->dev = dev;
> + platform_set_drvdata(pdev, pcpidev);
> +
> + pcpidev->pdata = of_device_get_match_data(dev);
[Severity: Medium]
Does this missing NULL check lead to a crash when binding via sysfs?
If an administrator manually binds the driver to a platform device without
an Open Firmware node via sysfs, of_device_get_match_data() returns NULL.
The driver will later unconditionally dereference pcpidev->pdata (such as
in imx_cpi_sw_reset()), leading to a kernel panic instead of gracefully
failing.
> + pcpidev->mode = PI_GATE_CLOCK_MODE;
> +
> + pcpidev->regs = devm_platform_ioremap_resource(pdev, 0);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624-imx8qxp_pcam-v6-0-4b3f45920d2f@nxp.com?part=7
^ permalink raw reply
* Re: [PATCH v6 3/9] media: subdev: Add media_async_register_subdev() helper
From: sashiko-bot @ 2026-06-24 21:00 UTC (permalink / raw)
To: Frank.Li; +Cc: conor+dt, Frank.Li, robh, imx, devicetree
In-Reply-To: <20260624-imx8qxp_pcam-v6-3-4b3f45920d2f@nxp.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Heap Buffer Overflow during endpoint parsing
- [High] NULL Pointer Dereference of `sd->internal_ops`
- [Medium] Resource Leak / Subsystem Guideline Violation due to goto and scope-based cleanup mixing
- [Low] Code Quality / Misspelled diagnostic messages
--
commit 050e10af8571659357b0d6bb808eab4e58871636
Author: Frank Li <Frank.Li@nxp.com>
media: subdev: Add media_async_register_subdev() helper
This commit introduces media_async_register_subdev() to register a V4L2
sub-device with the asynchronous framework. It parses the firmware graph
and sets up common notifiers and media pads. This aims to reduce duplicated
code and simplify error handling in V4L2 sub-device drivers.
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> index 62a3a452f7884..169059654478f 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
[ ... ]
> @@ -1302,6 +1303,160 @@ int __v4l2_async_register_subdev_sensor(struct v4l2_subdev *sd, struct module *m
[ ... ]
> +static int
> +v4l2_async_nf_parse_fwnode(struct device *dev, struct media_pad *pads,
> + struct v4l2_async_notifier *notifier)
> +{
> + struct v4l2_subdev *sd = notifier->sd;
> + struct v4l2_async_connection *asd;
> + struct media_pad *pad;
> + int ret;
> +
> + if (!sd->internal_ops->set_pad_by_ep)
[Severity: High]
If a driver passes a subdevice without internal_ops initialized, will this
dereference a NULL pointer?
> + return dev_err_probe(dev, -EINVAL,
> + "Missed valiate_endpoint() callback\n");
[Severity: Low]
This isn't a bug, but there's a typo in "valiate" and it references the wrong
callback name (set_pad_by_ep).
> + pad = pads;
> +
> + fwnode_graph_for_each_endpoint_scoped(dev_fwnode(dev), ep) {
> + u32 flags;
> +
> + ret = v4l2_fwnode_endpoint_parse(ep, &pad->vep);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to parse endpoint\n");
> +
> + ret = sd->internal_ops->set_pad_by_ep(sd, pad);
> + if (ret < 0)
> + return dev_err_probe(dev, ret, "Can support endponit\n");
[Severity: Low]
This isn't a bug, but "endponit" is misspelled, and the logic seems inverted
since this prints on an error path where the endpoint cannot be supported.
> +
> + flags = pad->flags;
> +
> + pad++;
[Severity: High]
Could this loop write out of bounds of the pads array?
In __media_async_register_subdev(), ep_count is calculated with:
ep_count = fwnode_graph_get_endpoint_count(dev_fwnode(dev), 0);
which only counts endpoints that have a remote connection, and the pads
array is allocated to exactly that size.
Since fwnode_graph_for_each_endpoint_scoped() unconditionally iterates over
all endpoints, if the firmware graph contains any unconnected endpoints, won't
this execute more times than ep_count and write into adjacent heap memory?
> +
> + if (flags & MEDIA_PAD_FL_SOURCE)
> + continue; /* Bypass source port */
> +
> + notifier->ops = &v4l2_common_notifier_ops;
> +
> + asd = v4l2_async_nf_add_fwnode_remote(notifier, ep,
> + struct v4l2_async_connection);
> + if (IS_ERR(asd))
> + return dev_err_probe(dev, PTR_ERR(asd),
> + "failed to add notifier\n");
> + }
> +
> + return 0;
> +}
[ ... ]
> +int __media_async_register_subdev(struct v4l2_subdev *sd, struct module *module)
> +{
> + struct device *dev = sd->dev;
> + u32 ep_count;
> + int ret;
> +
> + if (WARN_ON(!sd->dev))
> + return -ENODEV;
> +
> + struct v4l2_async_notifier *notifier __free(kfree) = kzalloc_obj(*notifier);
> + if (!notifier)
> + return -ENOMEM;
> +
> + v4l2_async_subdev_nf_init(notifier, sd);
> +
> + ep_count = fwnode_graph_get_endpoint_count(dev_fwnode(dev), 0);
> + if (!ep_count)
> + return dev_err_probe(dev, -EINVAL, "No connected endpoints\n");
> +
> + struct media_pad *pads __free(kfree) = kzalloc_objs(struct media_pad, ep_count);
> + if (!pads)
> + return -ENOMEM;
> +
> + ret = v4l2_async_nf_parse_fwnode(dev, pads, notifier);
> + if (ret < 0)
> + return ret;
[Severity: Medium]
If v4l2_async_nf_parse_fwnode() fails, does returning directly bypass
the v4l2_async_nf_cleanup() call that would normally happen via the
out_cleanup label?
The notifier itself is freed via cleanup helpers, but it seems the
v4l2_async_connection structures allocated in the parse loop and their fwnode
references will be leaked if we don't explicitly call v4l2_async_nf_cleanup().
> +
> + ret = media_entity_pads_init(&sd->entity, ep_count, pads);
> + if (ret)
> + goto out_cleanup;
> +
> + ret = v4l2_async_nf_register(notifier);
> + if (ret < 0)
> + goto out_cleanup;
> +
> + ret = v4l2_subdev_init_finalize(sd);
> + if (ret)
> + goto out_unregister;
> +
> + ret = __v4l2_async_register_subdev(sd, module);
> + if (ret < 0)
> + goto out_unregister;
[Severity: Medium]
If __v4l2_async_register_subdev() fails, we jump to out_unregister, which
cleans up the notifier and returns.
Does this leak the active_state allocated earlier by
v4l2_subdev_init_finalize(sd) since v4l2_subdev_cleanup() is bypassed?
> +
> + sd->subdev_notifier = no_free_ptr(notifier);
> + retain_and_null_ptr(pads);
> +
> + return 0;
> +
> +out_unregister:
> + v4l2_async_nf_unregister(notifier);
> +
> +out_cleanup:
> + v4l2_async_nf_cleanup(notifier);
> +
> + return ret;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624-imx8qxp_pcam-v6-0-4b3f45920d2f@nxp.com?part=3
^ permalink raw reply
* Re: [PATCH v6 8/9] arm64: dts: imx8: add camera parallel interface (CPI) node
From: sashiko-bot @ 2026-06-24 21:00 UTC (permalink / raw)
To: Frank.Li; +Cc: conor+dt, imx, robh, devicetree, Frank.Li
In-Reply-To: <20260624-imx8qxp_pcam-v6-8-4b3f45920d2f@nxp.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Mismatch between DTS port assignment (`port@4`) and the dt-binding documentation which specifies `port@6` for the Parallel RX.
- [Low] Devicetree schema validation error: 'ports' is a strictly required property but is missing from the base `parallel_cpi` node.
--
commit 80a1591fa539edb7320ce7a2c3b3f4dda7b4baa1
Author: Frank Li <Frank.Li@nxp.com>
arm64: dts: imx8: add camera parallel interface (CPI) node
This commit adds the camera parallel interface (CPI) node for imx8. It
defines the base cpi node in imx8-ss-img.dtsi and configures the endpoint
connections in imx8qxp-ss-img.dtsi.
> diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> --- a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
[ ... ]
> @@ -222,6 +222,19 @@ irqsteer_parallel: irqsteer@58260000 {
> status = "disabled";
> };
>
> + parallel_cpi: cpi@58261000 {
> + compatible = "fsl,imx8qxp-pcif";
> + reg = <0x58261000 0x1000>;
[Severity: Low]
Does this base node need a ports property to pass dtbs_check validation?
The fsl,imx93-pcif.yaml schema strictly requires the ports property, and
dt-validate processes nodes regardless of whether status is disabled.
Will building devicetrees that include imx8-ss-img.dtsi without extending
this node trigger validation errors?
> + clocks = <&pi0_pxl_lpcg IMX_LPCG_CLK_0>,
> + <&pi0_ipg_lpcg IMX_LPCG_CLK_4>;
> + clock-names = "pixel", "ipg";
> + assigned-clocks = <&clk IMX_SC_R_PI_0 IMX_SC_PM_CLK_PER>;
> + assigned-clock-parents = <&clk IMX_SC_R_PI_0_PLL IMX_SC_PM_CLK_PLL>;
> + assigned-clock-rates = <160000000>;
> + power-domains = <&pd IMX_SC_R_PI_0>;
> + status = "disabled";
> + };
> diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> --- a/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
[ ... ]
> @@ -62,6 +62,14 @@ isi_in_2: endpoint {
> remote-endpoint = <&mipi_csi0_out>;
> };
> };
> +
> + port@4 {
> + reg = <4>;
> +
> + isi_in_4: endpoint {
> + remote-endpoint = <¶llel_cpi_out>;
> + };
> + };
[Severity: Low]
Does port@4 match the devicetree binding documentation?
The Documentation/devicetree/bindings/media/fsl,imx8qxp-isi.yaml
documentation explicitly specifies port@6 as the CSI-2 Parallel RX.
Even though the driver configures the hardware correctly with num_ports = 5,
does adding port@4 here introduce an undocumented port and create a schema
mismatch?
> };
> };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624-imx8qxp_pcam-v6-0-4b3f45920d2f@nxp.com?part=8
^ permalink raw reply
* Re: [PATCH v6 6/9] dt-bindings: media: add i.MX parallel CPI support
From: sashiko-bot @ 2026-06-24 20:57 UTC (permalink / raw)
To: Frank.Li; +Cc: robh, Frank.Li, conor+dt, imx, devicetree
In-Reply-To: <20260624-imx8qxp_pcam-v6-6-4b3f45920d2f@nxp.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unhandled resource leaks (memory and fwnode references) and double-free risk in `__media_async_register_subdev` and its cleanup path.
--
commit 050e10af8571659357b0d6bb808eab4e58871636
Author: Frank Li <Frank.Li@nxp.com>
media: subdev: Add media_async_register_subdev() helper
This commit adds media_async_register_subdev(), a helper to register a V4L2
sub-device with the asynchronous sub-device framework. It parses the firmware
graph, creates media pads for endpoints, and registers asynchronous notifiers.
This centralizes common initialization and link creation to reduce boilerplate.
> diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
> index 62a3a452f7884..169059654478f 100644
> --- a/drivers/media/v4l2-core/v4l2-fwnode.c
> +++ b/drivers/media/v4l2-core/v4l2-fwnode.c
> @@ -1302,6 +1303,160 @@ int __v4l2_async_register_subdev_sensor(struct v4l2_subdev *sd, struct module *m
> }
> EXPORT_SYMBOL_GPL(__v4l2_async_register_subdev_sensor);
>
[ ... ]
> +void media_async_subdev_cleanup(struct v4l2_subdev *sd)
> +{
> + v4l2_async_unregister_subdev(sd);
> + v4l2_subdev_cleanup(sd);
> + media_entity_cleanup(&sd->entity);
> + v4l2_async_nf_unregister(sd->subdev_notifier);
> + v4l2_async_nf_cleanup(sd->subdev_notifier);
> + kfree(sd->entity.pads);
> +}
> +EXPORT_SYMBOL_GPL(media_async_subdev_cleanup);
[Severity: High]
Does this cleanup function leak fwnode references?
v4l2_async_nf_parse_fwnode() parses endpoints directly into pad->vep, taking
an fwnode reference via fwnode_handle_get(). It appears this cleanup function
fails to iterate over the pads to call v4l2_fwnode_endpoint_free(&pad->vep) to
release these references.
Also, if a driver explicitly invokes this cleanup helper to recover after
__media_async_register_subdev() fails, will kfree(sd->entity.pads) cause a
double-free panic?
> +
> +int __media_async_register_subdev(struct v4l2_subdev *sd, struct module *module)
> +{
> + struct device *dev = sd->dev;
> + u32 ep_count;
> + int ret;
> +
[ ... ]
> + struct media_pad *pads __free(kfree) = kzalloc_objs(struct media_pad, ep_count);
> + if (!pads)
> + return -ENOMEM;
> +
> + ret = v4l2_async_nf_parse_fwnode(dev, pads, notifier);
> + if (ret < 0)
> + return ret;
> +
> + ret = media_entity_pads_init(&sd->entity, ep_count, pads);
> + if (ret)
> + goto out_cleanup;
> +
> + ret = v4l2_async_nf_register(notifier);
> + if (ret < 0)
> + goto out_cleanup;
> +
> + ret = v4l2_subdev_init_finalize(sd);
> + if (ret)
> + goto out_unregister;
> +
> + ret = __v4l2_async_register_subdev(sd, module);
> + if (ret < 0)
> + goto out_unregister;
> +
> + sd->subdev_notifier = no_free_ptr(notifier);
> + retain_and_null_ptr(pads);
> +
> + return 0;
> +
> +out_unregister:
> + v4l2_async_nf_unregister(notifier);
> +
> +out_cleanup:
> + v4l2_async_nf_cleanup(notifier);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(__media_async_register_subdev);
[Severity: High]
If __v4l2_async_register_subdev() fails, the error path returns without
calling v4l2_subdev_cleanup(sd) to free the active state allocated by
v4l2_subdev_init_finalize(). Can this leak sd->active_state?
Additionally, pads is allocated with __free(kfree) and linked to
sd->entity.pads. On a registration failure, pads is automatically freed when
the function returns, but sd->entity.pads is not cleared.
Could this leave a dangling pointer that causes a double-free if error
recovery is attempted?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624-imx8qxp_pcam-v6-0-4b3f45920d2f@nxp.com?part=6
^ permalink raw reply
* Re: [PATCH v6 1/9] media: mc-entity: Store parsed V4L2 fwnode endpoint in media_pad
From: sashiko-bot @ 2026-06-24 20:56 UTC (permalink / raw)
To: Frank.Li; +Cc: Frank.Li, imx, conor+dt, devicetree, robh
In-Reply-To: <20260624-imx8qxp_pcam-v6-1-4b3f45920d2f@nxp.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Captured fwnode endpoint contains a dangling pointer to `local_fwnode` if the caller drops its reference after parsing.
- [Medium] Embedding a large, V4L2-specific `struct v4l2_fwnode_endpoint` into the generic Media Controller `struct media_pad` breaks subsystem abstraction and unconditionally bloats memory.
--
commit 7acfb56349845d6638c963825f0f6b087ebaa54d
Author: Frank Li <Frank.Li@nxp.com>
media: mc-entity: Store parsed V4L2 fwnode endpoint in media_pad
This commit captures the parsed v4l2 fwnode endpoint information directly
within struct media_pad to allow reuse by consumers without reparsing.
> diff --git a/include/media/media-entity.h b/include/media/media-entity.h
> index d9b72cd87d524..4a3785cd9f370 100644
> --- a/include/media/media-entity.h
> +++ b/include/media/media-entity.h
> @@ -20,6 +20,8 @@
> #include <linux/minmax.h>
> #include <linux/types.h>
>
> +#include <media/v4l2-fwnode.h>
> +
[Severity: Medium]
Does this include break subsystem isolation? struct media_pad is a generic
structure used across several subsystems like alsa, dvb, drm, and iio.
Including a v4l2-specific header here appears to tie the generic media
controller to v4l2.
[ ... ]
> @@ -240,7 +243,7 @@ struct media_pad {
> u16 num_links;
> enum media_pad_signal_type sig_type;
> unsigned long flags;
> -
> + struct v4l2_fwnode_endpoint vep;
[Severity: Medium]
Is it expected to embed this v4l2-specific structure into every generic
media_pad? The v4l2_fwnode_endpoint struct is quite large, and
unconditionally embedding it adds memory overhead for all non-v4l2
entities that do not use fwnode endpoints.
[Severity: Critical]
Can this cause a use-after-free regression with the fwnode pointers?
When the endpoint is parsed, v4l2_fwnode_endpoint_parse() assigns the
fwnode to vep->base.local_fwnode without taking a reference count. This
was safe when vep was a transient stack variable, but now that it is
stored persistently in the media_pad, the caller might drop the
reference.
For example, if the caller iterates using a scoped iterator:
drivers/media/v4l2-core/v4l2-fwnode.c:v4l2_async_nf_parse_fwnode() {
...
fwnode_graph_for_each_endpoint_scoped(dev_fwnode(dev), ep) {
ret = v4l2_fwnode_endpoint_parse(ep, &pad->vep);
...
}
The scoped iterator automatically drops the reference to ep at the end
of the iteration, leaving pad->vep.base.local_fwnode as a dangling
pointer.
Later access or pointer comparisons, like in v4l2_common_notifier_bound(),
might dereference freed memory if the fwnode is deleted.
> /*
> * The fields below are private, and should only be accessed via
> * appropriate functions.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624-imx8qxp_pcam-v6-0-4b3f45920d2f@nxp.com?part=1
^ permalink raw reply
* Re: [PATCH v6 2/9] media: subdev: Add set_pad_by_ep() callback to internal ops
From: sashiko-bot @ 2026-06-24 20:50 UTC (permalink / raw)
To: Frank.Li; +Cc: imx, Frank.Li, devicetree, robh, conor+dt
In-Reply-To: <20260624-imx8qxp_pcam-v6-2-4b3f45920d2f@nxp.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Severe layering violation by tightly coupling the core Media Controller framework with V4L2-specific firmware parsing.
- [Low] Typo in the callback documentation.
--
commit 3e6e9fb5a2a23ceb5cd559adeba28603dd1f9e0f
Author: Frank Li <Frank.Li@nxp.com>
media: subdev: Add set_pad_by_ep() callback to internal ops
This commit adds a set_pad_by_ep() callback to v4l2_subdev_internal_ops.
The callback is invoked while parsing firmware node endpoints and allows
subdevice drivers to configure media pad properties based on endpoint
information.
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index d256b7ec8f848..eb652eb76d33f 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -935,6 +935,10 @@ struct v4l2_subdev_ops {
> * the v4l2_subdev structure. It is almost certainly required for any
> * sub-device that sets the V4L2_SUBDEV_FL_HAS_DEVNODE flag.
> *
> + * @set_pad_by_ep: Set pad informaiton by fwnode endpoint, parsed fwnode already
[Severity: Low]
This isn't a bug, but there is a typo in information (informaiton).
> + * saved into pad->vep. return < 0 means can't support this type
> + * endpoint. Set pad->flags according to pad->vep information.
> + *
> * .. note::
> * Never call this from drivers, only the v4l2 framework can call
> * these ops.
> @@ -947,6 +951,7 @@ struct v4l2_subdev_internal_ops {
> int (*open)(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh);
> int (*close)(struct v4l2_subdev *sd, struct v4l2_subdev_fh *fh);
> void (*release)(struct v4l2_subdev *sd);
> + int (*set_pad_by_ep)(struct v4l2_subdev *sd, struct media_pad *pad);
[Severity: Medium]
Does this callback signature enforce an architectural layering violation?
By omitting the vep argument, the callback design expects the parsed V4L2
endpoint configuration to be passed via pad->vep. struct media_pad is part of
the generic Media Controller API, which is shared across various subsystems
like ALSA and DVB.
Embedding the V4L2-specific struct v4l2_fwnode_endpoint directly into the
generic struct media_pad creates an inappropriate subsystem dependency and
wastes memory for all non-V4L2 media pads.
Could the V4L2-specific endpoint be passed directly as an argument to the
callback, such as:
int (*set_pad_by_ep)(struct v4l2_subdev *sd, struct media_pad *pad,
struct v4l2_fwnode_endpoint *vep);
> };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624-imx8qxp_pcam-v6-0-4b3f45920d2f@nxp.com?part=2
^ permalink raw reply
* Re: [PATCH v2 1/3] dt-bindings: power: limits: Describe Qualcomm SPEL hardware
From: Daniel Lezcano @ 2026-06-24 20:41 UTC (permalink / raw)
To: Krzysztof Kozlowski, Manaf Meethalavalappu Pallikunhi
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Rafael J. Wysocki,
Bjorn Andersson, Konrad Dybcio, Gaurav Kohli, linux-arm-msm,
devicetree, linux-kernel, linux-pm
In-Reply-To: <d0528339-8be9-4895-a054-625df96ca926@kernel.org>
Hi Krzysztof,
Le 24/06/2026 à 12:45, Krzysztof Kozlowski a écrit :
> On 23/06/2026 11:47, Manaf Meethalavalappu Pallikunhi wrote:
>> Hi Krzysztof,
>>
>>
>> On 6/22/2026 5:58 PM, Krzysztof Kozlowski wrote:
>>> On Sat, Jun 20, 2026 at 02:09:08AM +0530, Manaf Meethalavalappu Pallikunhi wrote:
>>>> The Qualcomm SoC Power and Electrical Limits (SPEL) provides hardware
>>>> based power monitoring and limiting capabilities for various domains.
>>>>
>>>> Add a DeviceTree binding to describe the SPEL block on Qualcomm's SoC.
>>>>
>>>> Signed-off-by: Manaf Meethalavalappu Pallikunhi <manaf.pallikunhi@oss.qualcomm.com>
>>>> ---
>>>> .../bindings/power/limits/qcom,spel.yaml | 47 ++++++++++++++++++++++
>>>> MAINTAINERS | 6 +++
>>>> 2 files changed, 53 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/power/limits/qcom,spel.yaml b/Documentation/devicetree/bindings/power/limits/qcom,spel.yaml
>>>
>>> What is "limits" directory for? What sort of class of devices fit there?
>>
>> Added for devices that integrate with the powercap framework (exposed
>> via sys/class/powercap). These devices are responsible for enforcing and
>
> That's a driver answer. I asked about class of devices. powercap
> framework is Linux thing, not a class of devices.
>
> Please describe hardware, not Linux frameworks.>>> monitoring power consumption limits across different domains, such as
>> the system, SoC, or specific subsystems. Any other better directory ?
>
> I don't know what is this hardware doing and commit msg is quite short
> on explanation. Power monitoring is usually hwmon, but probably this is
> not a hwmon.
Right, it is relatively new in the device tree bindings. There is the
Intel RAPL which is the same device but it does not belong to the DT
namespace.
RAPL : Running Average Power Limit
SPEL : SoC Power and Electrical Limits
It allows power capping and read the average power consumption for a
specific device (or/and an energy counter)
Basically you can set a power constraint (power limit) to a device and
this one won't consume more than that power (the power limitation
strategy is managed under the hood by the firmware depending on the
device - lower OPP, idle injection, modem weaker signal, etc ...).
The RAPL or the SPEL have a hierarchical power limitation. For example:
SoC
|
------------------------
| |
Cluster0 Cluster1
| |
----------------- -----------------
| | | | | | | |
CPU0 CPU1 CPU2 CPU3 CPU4 CPU5 CPU6 CPU7
If you specify a power limit to 'SoC', then the power consumption of
Cluster0 + Cluster1 <= SoC
If Cluster0 power consumption decreases, then Cluster1 is allowed to use
more power until Cluster0 + Cluster1 <= SoC
For me it sounds reasonable to put the device description under power/limits
^ permalink raw reply
* [PATCH v6 9/9] arm64: dts: imx8qxp-mek: add parallel ov5640 camera support
From: Frank.Li @ 2026-06-24 20:37 UTC (permalink / raw)
To: Sakari Ailus, Mauro Carvalho Chehab, Michael Riesch,
Laurent Pinchart, Frank Li, Martin Kepplinger-Novakovic,
Rui Miguel Silva, Purism Kernel Team, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam
Cc: linux-media, linux-kernel, imx, Guoniu Zhou, devicetree,
linux-arm-kernel
In-Reply-To: <20260624-imx8qxp_pcam-v6-0-4b3f45920d2f@nxp.com>
From: Frank Li <Frank.Li@nxp.com>
Add parallel ov5640 nodes in imx8qxp-mek and create overlay file to enable
it because it can work at two mode: MIPI CSI and parallel mode.
Reviewed-by: Guoniu Zhou <guoniu.zhou@nxp.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
changes in v6
- add Guoniu's reviewed-by tags
changes in v4
- add hsync-active = <1>
changes in v3
- replace csi with cpi.
- use imx8qxp-mek-ov5640-cpi.dtso since csi use imx8qxp-mek-ov5640-csi.dtso
change in v2
- move ov5640 part to overlay file
- rename to imx8qxp-mek-ov5640-parallel.dtso
- remove data-lanes
---
arch/arm64/boot/dts/freescale/Makefile | 3 +
.../boot/dts/freescale/imx8qxp-mek-ov5640-cpi.dtso | 83 ++++++++++++++++++++++
2 files changed, 86 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
index 001ca3a12c0ae..3b9e9844f11ef 100644
--- a/arch/arm64/boot/dts/freescale/Makefile
+++ b/arch/arm64/boot/dts/freescale/Makefile
@@ -554,6 +554,9 @@ dtb-$(CONFIG_ARCH_MXC) += imx8qxp-mek-pcie-ep.dtb
imx8qxp-mek-ov5640-csi-dtbs := imx8qxp-mek.dtb imx8qxp-mek-ov5640-csi.dtbo
dtb-${CONFIG_ARCH_MXC} += imx8qxp-mek-ov5640-csi.dtb
+imx8qxp-mek-ov5640-cpi-dtbs := imx8qxp-mek.dtb imx8qxp-mek-ov5640-cpi.dtbo
+dtb-${CONFIG_ARCH_MXC} += imx8qxp-mek-ov5640-cpi.dtb
+
dtb-$(CONFIG_ARCH_MXC) += imx8qxp-tqma8xqp-mba8xx.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8qxp-tqma8xqps-mb-smarc-2.dtb
dtb-$(CONFIG_ARCH_MXC) += imx8ulp-9x9-evk.dtb
diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-mek-ov5640-cpi.dtso b/arch/arm64/boot/dts/freescale/imx8qxp-mek-ov5640-cpi.dtso
new file mode 100644
index 0000000000000..9fbdd798f17d6
--- /dev/null
+++ b/arch/arm64/boot/dts/freescale/imx8qxp-mek-ov5640-cpi.dtso
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright 2025 NXP
+ */
+
+/dts-v1/;
+/plugin/;
+
+#include <dt-bindings/clock/imx8-lpcg.h>
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/media/video-interfaces.h>
+#include <dt-bindings/pinctrl/pads-imx8qxp.h>
+
+&cm40_i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ ov5640_pi: camera@3c {
+ compatible = "ovti,ov5640";
+ reg = <0x3c>;
+ clocks = <&pi0_misc_lpcg IMX_LPCG_CLK_0>;
+ clock-names = "xclk";
+ assigned-clocks = <&pi0_misc_lpcg IMX_LPCG_CLK_0>;
+ assigned-clock-rates = <24000000>;
+ AVDD-supply = <®_2v8>;
+ DOVDD-supply = <®_1v8>;
+ DVDD-supply = <®_1v5>;
+ pinctrl-0 = <&pinctrl_parallel_cpi>;
+ pinctrl-names = "default";
+ powerdown-gpios = <&lsio_gpio3 2 GPIO_ACTIVE_HIGH>;
+ reset-gpios = <&lsio_gpio3 3 GPIO_ACTIVE_LOW>;
+
+ port {
+ ov5640_pi_ep: endpoint {
+ bus-type = <MEDIA_BUS_TYPE_PARALLEL>;
+ bus-width = <8>;
+ hsync-active = <1>;
+ pclk-sample = <1>;
+ remote-endpoint = <¶llel_cpi_in>;
+ vsync-active = <0>;
+ };
+ };
+ };
+};
+
+&iomuxc {
+ pinctrl_parallel_cpi: parallelcpigrp {
+ fsl,pins = <
+ IMX8QXP_CSI_D00_CI_PI_D02 0xc0000041
+ IMX8QXP_CSI_D01_CI_PI_D03 0xc0000041
+ IMX8QXP_CSI_D02_CI_PI_D04 0xc0000041
+ IMX8QXP_CSI_D03_CI_PI_D05 0xc0000041
+ IMX8QXP_CSI_D04_CI_PI_D06 0xc0000041
+ IMX8QXP_CSI_D05_CI_PI_D07 0xc0000041
+ IMX8QXP_CSI_D06_CI_PI_D08 0xc0000041
+ IMX8QXP_CSI_D07_CI_PI_D09 0xc0000041
+
+ IMX8QXP_CSI_MCLK_CI_PI_MCLK 0xc0000041
+ IMX8QXP_CSI_PCLK_CI_PI_PCLK 0xc0000041
+ IMX8QXP_CSI_HSYNC_CI_PI_HSYNC 0xc0000041
+ IMX8QXP_CSI_VSYNC_CI_PI_VSYNC 0xc0000041
+ IMX8QXP_CSI_EN_LSIO_GPIO3_IO02 0xc0000041
+ IMX8QXP_CSI_RESET_LSIO_GPIO3_IO03 0xc0000041
+ >;
+ };
+};
+
+&isi {
+ status = "okay";
+};
+
+¶llel_cpi {
+ status = "okay";
+
+ ports {
+ port@0 {
+ parallel_cpi_in: endpoint {
+ hsync-active = <1>;
+ remote-endpoint = <&ov5640_pi_ep>;
+ };
+ };
+ };
+};
--
2.43.0
^ permalink raw reply related
* [PATCH v6 8/9] arm64: dts: imx8: add camera parallel interface (CPI) node
From: Frank.Li @ 2026-06-24 20:37 UTC (permalink / raw)
To: Sakari Ailus, Mauro Carvalho Chehab, Michael Riesch,
Laurent Pinchart, Frank Li, Martin Kepplinger-Novakovic,
Rui Miguel Silva, Purism Kernel Team, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam
Cc: linux-media, linux-kernel, imx, Guoniu Zhou, devicetree,
linux-arm-kernel
In-Reply-To: <20260624-imx8qxp_pcam-v6-0-4b3f45920d2f@nxp.com>
From: Frank Li <Frank.Li@nxp.com>
Add camera parallel interface (CPI) node.
Reviewed-by: Guoniu Zhou <guoniu.zhou@nxp.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
changes in v6
- add Guoniu Zhou's review by
changes in v4
- none
changes in v3
- replace csi with cpi.
changes in v2
- update compatible string to match binding's change
---
arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi | 13 +++++++++++
arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi | 27 +++++++++++++++++++++++
2 files changed, 40 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
index a72b2f1c4a1b2..b504f99f6acdb 100644
--- a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
@@ -222,6 +222,19 @@ irqsteer_parallel: irqsteer@58260000 {
status = "disabled";
};
+ parallel_cpi: cpi@58261000 {
+ compatible = "fsl,imx8qxp-pcif";
+ reg = <0x58261000 0x1000>;
+ clocks = <&pi0_pxl_lpcg IMX_LPCG_CLK_0>,
+ <&pi0_ipg_lpcg IMX_LPCG_CLK_4>;
+ clock-names = "pixel", "ipg";
+ assigned-clocks = <&clk IMX_SC_R_PI_0 IMX_SC_PM_CLK_PER>;
+ assigned-clock-parents = <&clk IMX_SC_R_PI_0_PLL IMX_SC_PM_CLK_PLL>;
+ assigned-clock-rates = <160000000>;
+ power-domains = <&pd IMX_SC_R_PI_0>;
+ status = "disabled";
+ };
+
pi0_ipg_lpcg: clock-controller@58263004 {
compatible = "fsl,imx8qxp-lpcg";
reg = <0x58263004 0x4>;
diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
index 232cf25dadfcd..5aae15540d6cb 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qxp-ss-img.dtsi
@@ -62,6 +62,14 @@ isi_in_2: endpoint {
remote-endpoint = <&mipi_csi0_out>;
};
};
+
+ port@4 {
+ reg = <4>;
+
+ isi_in_4: endpoint {
+ remote-endpoint = <¶llel_cpi_out>;
+ };
+ };
};
};
@@ -95,3 +103,22 @@ &jpegenc {
&mipi_csi_1 {
status = "disabled";
};
+
+¶llel_cpi {
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ };
+
+ port@1 {
+ reg = <1>;
+
+ parallel_cpi_out: endpoint {
+ remote-endpoint = <&isi_in_4>;
+ };
+ };
+ };
+};
--
2.43.0
^ permalink raw reply related
* [PATCH v6 7/9] media: nxp: add V4L2 subdev driver for camera parallel interface (CPI)
From: Frank.Li @ 2026-06-24 20:37 UTC (permalink / raw)
To: Sakari Ailus, Mauro Carvalho Chehab, Michael Riesch,
Laurent Pinchart, Frank Li, Martin Kepplinger-Novakovic,
Rui Miguel Silva, Purism Kernel Team, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam
Cc: linux-media, linux-kernel, imx, Guoniu Zhou, devicetree,
linux-arm-kernel, Alice Yuan, Robert Chiras, Zhipeng Wang
In-Reply-To: <20260624-imx8qxp_pcam-v6-0-4b3f45920d2f@nxp.com>
From: Alice Yuan <alice.yuan@nxp.com>
Add a V4L2 sub-device driver for the CPI controller found on i.MX8QXP,
i.MX8QM, and i.MX93 SoCs. This controller supports parallel camera sensors
and enables image data capture through a parallel interface.
Signed-off-by: Alice Yuan <alice.yuan@nxp.com>
Signed-off-by: Robert Chiras <robert.chiras@nxp.com>
Signed-off-by: Zhipeng Wang <zhipeng.wang_1@nxp.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change in v6
- Don't use subdev_1to1
- use new api media_async_register_subdev()
- remove used switch -case
Change in v5
- Use subdev_1to1 register function
- Use v4l2_subdev_get_frame_desc_passthrough
- Use dwc csi2 similar logic enable/disable stream
- Add route settup at imx_cpi_init_state()
- Remove V2 register layout support, add it later
change in v4
- remove unnecesary header file.
- use devm_bulk_clk_get().
- update kConfig i.MX8/i.MX9
- Remove define IMX_CPI_DEF_PIX_WIDTH ..., which used once only
- drop get_interface_ctrl_reg1_param
- drop uv-swap
- drop imx_cpi_link_setup by use immutable link.
- use enable/disable_stream() replace depericated .s_stream.
- remove dbg print and reg dump functions.
- use goto/.remove() to do manual cleanup.
- remove imx93 support. Add it later.
change in v3
- replace csi with cpi
- use __free(fwnode_handle) to simpilfy code
- remove imx91 driver data, which is the same as imx93
change in v2
- remove MODULE_ALIAS
- use devm_pm_runtime_enable() and cleanup remove function
- change output format to 1x16. controller convert 2x8 to 1x16 format
---
MAINTAINERS | 1 +
drivers/media/platform/nxp/Kconfig | 12 +
drivers/media/platform/nxp/Makefile | 1 +
drivers/media/platform/nxp/imx-parallel-cpi.c | 629 ++++++++++++++++++++++++++
4 files changed, 643 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 51d5c62e3fdea..045a06d0bb216 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16263,6 +16263,7 @@ F: Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
F: Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
F: Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
F: drivers/media/platform/nxp/imx-mipi-csis.c
+F: drivers/media/platform/nxp/imx-parallel-cpi.c
F: drivers/media/platform/nxp/imx7-media-csi.c
F: drivers/media/platform/nxp/imx8mq-mipi-csi2.c
diff --git a/drivers/media/platform/nxp/Kconfig b/drivers/media/platform/nxp/Kconfig
index 40e3436669e21..90f7c792003f2 100644
--- a/drivers/media/platform/nxp/Kconfig
+++ b/drivers/media/platform/nxp/Kconfig
@@ -39,6 +39,18 @@ config VIDEO_IMX_MIPI_CSIS
Video4Linux2 sub-device driver for the MIPI CSI-2 CSIS receiver
v3.3/v3.6.3 found on some i.MX7 and i.MX8 SoCs.
+config VIDEO_IMX_PARALLEL_CPI
+ tristate "NXP i.MX8/i.MX9 Parallel CPI Driver"
+ depends on ARCH_MXC || COMPILE_TEST
+ depends on VIDEO_DEV
+ select MEDIA_CONTROLLER
+ select V4L2_1TO1
+ select V4L2_FWNODE
+ select VIDEO_V4L2_SUBDEV_API
+ help
+ Video4Linux2 sub-device driver for PARALLEL CPI receiver found
+ on some iMX8 and iMX9 SoCs.
+
source "drivers/media/platform/nxp/imx8-isi/Kconfig"
# mem2mem drivers
diff --git a/drivers/media/platform/nxp/Makefile b/drivers/media/platform/nxp/Makefile
index 4d90eb7136525..5346919d2f108 100644
--- a/drivers/media/platform/nxp/Makefile
+++ b/drivers/media/platform/nxp/Makefile
@@ -7,5 +7,6 @@ obj-y += imx8-isi/
obj-$(CONFIG_VIDEO_IMX7_CSI) += imx7-media-csi.o
obj-$(CONFIG_VIDEO_IMX8MQ_MIPI_CSI2) += imx8mq-mipi-csi2.o
obj-$(CONFIG_VIDEO_IMX_MIPI_CSIS) += imx-mipi-csis.o
+obj-$(CONFIG_VIDEO_IMX_PARALLEL_CPI) += imx-parallel-cpi.o
obj-$(CONFIG_VIDEO_IMX_PXP) += imx-pxp.o
obj-$(CONFIG_VIDEO_MX2_EMMAPRP) += mx2_emmaprp.o
diff --git a/drivers/media/platform/nxp/imx-parallel-cpi.c b/drivers/media/platform/nxp/imx-parallel-cpi.c
new file mode 100644
index 0000000000000..0f74b51608715
--- /dev/null
+++ b/drivers/media/platform/nxp/imx-parallel-cpi.c
@@ -0,0 +1,629 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * i.MX Parallel CPI receiver driver.
+ *
+ * Copyright 2019-2025 NXP
+ *
+ */
+
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/errno.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/limits.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+
+#include <media/v4l2-common.h>
+#include <media/v4l2-device.h>
+#include <media/v4l2-event.h>
+#include <media/v4l2-fwnode.h>
+#include <media/v4l2-mc.h>
+#include <media/v4l2-subdev.h>
+
+/* CI_PI INTERFACE CONTROL */
+#define IF_CTRL_REG_PL_ENABLE BIT(0)
+#define IF_CTRL_REG_PL_VALID BIT(1)
+#define IF_CTRL_REG_DATA_TYPE_SEL BIT(8)
+#define IF_CTRL_REG_DATA_TYPE(x) FIELD_PREP(GENMASK(13, 9), (x))
+
+#define DATA_TYPE_OUT_NULL 0x00
+#define DATA_TYPE_OUT_RGB 0x04
+#define DATA_TYPE_OUT_YUV444 0x08
+#define DATA_TYPE_OUT_YYU420_ODD 0x10
+#define DATA_TYPE_OUT_YYU420_EVEN 0x12
+#define DATA_TYPE_OUT_YYY_ODD 0x18
+#define DATA_TYPE_OUT_UYVY_EVEN 0x1a
+#define DATA_TYPE_OUT_RAW 0x1c
+
+#define IF_CTRL_REG_IF_FORCE_HSYNV_OVERRIDE 0x4
+#define IF_CTRL_REG_IF_FORCE_VSYNV_OVERRIDE 0x2
+#define IF_CTRL_REG_IF_FORCE_DATA_ENABLE_OVERRIDE 0x1
+
+/* CPI INTERFACE CONTROL REG */
+#define CPI_CTRL_REG_CPI_EN BIT(0)
+#define CPI_CTRL_REG_PIXEL_CLK_POL BIT(1)
+#define CPI_CTRL_REG_HSYNC_POL BIT(2)
+#define CPI_CTRL_REG_VSYNC_POL BIT(3)
+#define CPI_CTRL_REG_DE_POL BIT(4)
+#define CPI_CTRL_REG_PIXEL_DATA_POL BIT(5)
+#define CPI_CTRL_REG_CCIR_EXT_VSYNC_EN BIT(6)
+#define CPI_CTRL_REG_CCIR_EN BIT(7)
+#define CPI_CTRL_REG_CCIR_VIDEO_MODE BIT(8)
+#define CPI_CTRL_REG_CCIR_NTSC_EN BIT(9)
+#define CPI_CTRL_REG_CCIR_VSYNC_RESET_EN BIT(10)
+#define CPI_CTRL_REG_CCIR_ECC_ERR_CORRECT_EN BIT(11)
+#define CPI_CTRL_REG_HSYNC_FORCE_EN BIT(12)
+#define CPI_CTRL_REG_VSYNC_FORCE_EN BIT(13)
+#define CPI_CTRL_REG_GCLK_MODE_EN BIT(14)
+#define CPI_CTRL_REG_VALID_SEL BIT(15)
+#define CPI_CTRL_REG_RAW_OUT_SEL BIT(16)
+#define CPI_CTRL_REG_HSYNC_OUT_SEL BIT(17)
+#define CPI_CTRL_REG_HSYNC_PULSE(x) FIELD_PREP(GENMASK(21, 19), (x))
+#define CPI_CTRL_REG_UV_SWAP_EN BIT(22)
+#define CPI_CTRL_REG_DATA_TYPE_IN(x) FIELD_PREP(GENMASK(26, 23), (x))
+#define CPI_CTRL_REG_MASK_VSYNC_COUNTER(x) FIELD_PREP(GENMASK(28, 27), (x))
+#define CPI_CTRL_REG_SOFTRST BIT(31)
+
+/* CPI INTERFACE STATUS */
+#define CPI_STATUS_FIELD_TOGGLE BIT(0)
+#define CPI_STATUS_ECC_ERROR BIT(1)
+
+/* CPI INTERFACE CONTROL REG1 */
+#define CPI_CTRL_REG1_PIXEL_WIDTH(v) FIELD_PREP(GENMASK(15, 0), (v))
+#define CPI_CTRL_REG1_VSYNC_PULSE(v) FIELD_PREP(GENMASK(31, 16), (v))
+
+#define CPI_CTRL_V2_REG1_PIXEL_WIDTH(v) FIELD_PREP(GENMASK(16, 0), (v))
+#define CPI_CTRL_V2_REG1_VSYNC_PULSE(v) FIELD_PREP(GENMASK(31, 16), (v))
+
+/* Need match field DATA_TYPE_IN definition at CPI CTRL register */
+enum cpi_in_data_type {
+ CPI_IN_DT_UYVY_BT656_8 = 0x0,
+ CPI_IN_DT_UYVY_BT656_10,
+ CPI_IN_DT_RGB_8,
+ CPI_IN_DT_BGR_8,
+ CPI_IN_DT_YVYU_8 = 0x5,
+ CPI_IN_DT_YUV_8,
+ CPI_IN_DT_RAW_8 = 0x9,
+ CPI_IN_DT_RAW_10,
+};
+
+enum {
+ PI_GATE_CLOCK_MODE,
+ PI_CCIR_MODE,
+};
+
+enum {
+ PI_V1,
+};
+
+struct imx_cpi_plat_data {
+ u32 version;
+ u32 if_ctrl_reg;
+ u32 interface_status;
+ u32 interface_ctrl_reg;
+ u32 interface_ctrl_reg1;
+};
+
+struct imx_cpi_device {
+ struct device *dev;
+ void __iomem *regs;
+ struct clk_bulk_data *clks;
+ int num_clks;
+
+ struct v4l2_subdev sd;
+
+ const struct imx_cpi_plat_data *pdata;
+
+ u32 enabled_streams;
+ u8 mode;
+};
+
+struct imx_cpi_pix_format {
+ u32 code;
+ u32 output;
+ u32 data_type;
+ u8 width;
+};
+
+static const struct imx_cpi_pix_format imx_cpi_formats[] = {
+ /* YUV formats. */
+ {
+ .code = MEDIA_BUS_FMT_UYVY8_2X8,
+ .output = MEDIA_BUS_FMT_UYVY8_1X16,
+ .data_type = CPI_IN_DT_UYVY_BT656_8,
+ .width = 16,
+ }, {
+ .code = MEDIA_BUS_FMT_YUYV8_2X8,
+ .output = MEDIA_BUS_FMT_YUYV8_1X16,
+ .data_type = CPI_IN_DT_YVYU_8,
+ .width = 16,
+ },
+};
+
+static const struct imx_cpi_plat_data imx8qxp_pdata = {
+ .version = PI_V1,
+ .if_ctrl_reg = 0x0,
+ .interface_status = 0x20,
+ .interface_ctrl_reg = 0x10,
+ .interface_ctrl_reg1 = 0x30,
+};
+
+static const struct imx_cpi_pix_format *find_imx_cpi_format(u32 code)
+{
+ unsigned int i;
+
+ for (i = 0; i < ARRAY_SIZE(imx_cpi_formats); i++)
+ if (code == imx_cpi_formats[i].code)
+ return &imx_cpi_formats[i];
+
+ return NULL;
+}
+
+static void imx_cpi_sw_reset(struct imx_cpi_device *pcpidev)
+{
+ const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
+ u32 val;
+
+ /* Softwaret Reset */
+ val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
+ val |= CPI_CTRL_REG_SOFTRST;
+ writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
+
+ fsleep(500);
+ val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
+ val &= ~CPI_CTRL_REG_SOFTRST;
+ writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
+}
+
+static void imx_cpi_hw_config(struct imx_cpi_device *pcpidev,
+ const struct imx_cpi_pix_format *pcpidev_fmt)
+{
+ u32 flags = pcpidev->sd.entity.pads[V4L2_SUBDEV_1TO1_PADS_SINK].vep.bus.parallel.flags;
+ const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
+ bool hsync_pol = flags & V4L2_MBUS_HSYNC_ACTIVE_HIGH;
+ bool vsync_pol = flags & V4L2_MBUS_VSYNC_ACTIVE_HIGH;
+ u32 val;
+
+ /* Software Reset */
+ imx_cpi_sw_reset(pcpidev);
+
+ /* Config PL Data Type */
+ val = IF_CTRL_REG_DATA_TYPE(DATA_TYPE_OUT_YUV444);
+ val |= IF_CTRL_REG_PL_ENABLE | IF_CTRL_REG_PL_VALID;
+ writel(val, pcpidev->regs + pdata->if_ctrl_reg);
+
+ /* Config CTRL REG */
+ val = CPI_CTRL_REG_HSYNC_FORCE_EN | CPI_CTRL_REG_VSYNC_FORCE_EN;
+
+ val |= CPI_CTRL_REG_DATA_TYPE_IN(pcpidev_fmt->data_type) |
+ FIELD_PREP(CPI_CTRL_REG_HSYNC_POL, hsync_pol) |
+ FIELD_PREP(CPI_CTRL_REG_VSYNC_POL, vsync_pol) |
+ FIELD_PREP(CPI_CTRL_REG_PIXEL_CLK_POL, 0) |
+ CPI_CTRL_REG_MASK_VSYNC_COUNTER(3) |
+ CPI_CTRL_REG_HSYNC_PULSE(2);
+
+ if (pcpidev_fmt->code == MEDIA_BUS_FMT_YUYV8_2X8 ||
+ pcpidev_fmt->code == MEDIA_BUS_FMT_UYVY8_2X8)
+ val |= CPI_CTRL_REG_UV_SWAP_EN;
+
+ if (pcpidev->mode == PI_GATE_CLOCK_MODE) {
+ val |= CPI_CTRL_REG_GCLK_MODE_EN;
+ } else if (pcpidev->mode == PI_CCIR_MODE) {
+ val |= (CPI_CTRL_REG_CCIR_EN |
+ CPI_CTRL_REG_CCIR_VSYNC_RESET_EN |
+ CPI_CTRL_REG_CCIR_EXT_VSYNC_EN |
+ CPI_CTRL_REG_CCIR_ECC_ERR_CORRECT_EN);
+ }
+
+ writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
+}
+
+static void imx_cpi_config_ctrl_reg1(struct imx_cpi_device *pcpidev,
+ const struct v4l2_mbus_framefmt *format)
+{
+ const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
+ u32 pixel_width;
+ u32 vsync_pulse;
+ u32 val;
+
+ pixel_width = format->width - 1;
+ vsync_pulse = format->width << 1;
+
+ val = CPI_CTRL_REG1_PIXEL_WIDTH(pixel_width) |
+ CPI_CTRL_REG1_VSYNC_PULSE(vsync_pulse);
+ writel(val, pcpidev->regs + pdata->interface_ctrl_reg1);
+}
+
+static void imx_cpi_enable(struct imx_cpi_device *pcpidev)
+{
+ const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
+ u32 val;
+
+ /* Enable CPI */
+ val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
+ val |= CPI_CTRL_REG_CPI_EN;
+ writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
+
+ /* Disable SYNC Force */
+ val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
+ val &= ~(CPI_CTRL_REG_HSYNC_FORCE_EN | CPI_CTRL_REG_VSYNC_FORCE_EN);
+ writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
+}
+
+static void imx_cpi_disable(struct imx_cpi_device *pcpidev)
+{
+ const struct imx_cpi_plat_data *pdata = pcpidev->pdata;
+ u32 val;
+
+ /* Enable Sync Force */
+ val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
+ val |= CPI_CTRL_REG_HSYNC_FORCE_EN | CPI_CTRL_REG_VSYNC_FORCE_EN;
+ writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
+
+ /* Disable CPI */
+ val = readl(pcpidev->regs + pdata->interface_ctrl_reg);
+ val &= ~CPI_CTRL_REG_CPI_EN;
+ writel(val, pcpidev->regs + pdata->interface_ctrl_reg);
+
+ /* Disable Pixel Link */
+ val = readl(pcpidev->regs + pdata->if_ctrl_reg);
+ val &= ~(IF_CTRL_REG_PL_VALID | IF_CTRL_REG_PL_ENABLE);
+ writel(val, pcpidev->regs + pdata->if_ctrl_reg);
+}
+
+static struct imx_cpi_device *sd_to_imx_cpi_device(struct v4l2_subdev *sdev)
+{
+ return container_of(sdev, struct imx_cpi_device, sd);
+}
+
+static const struct media_entity_operations imx_cpi_entity_ops = {
+ .link_validate = v4l2_subdev_link_validate,
+ .get_fwnode_pad = v4l2_subdev_get_fwnode_pad_1_to_1,
+};
+
+static int imx_cpi_set_fmt(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_format *sdformat)
+{
+ struct imx_cpi_pix_format const *pcpidev_fmt;
+ struct v4l2_mbus_framefmt *fmt;
+
+ /*
+ * The Parallel cpi can't transcode in any way, the source format
+ * can't be modified.
+ */
+ if (sdformat->pad == V4L2_SUBDEV_1TO1_PADS_SOURCE)
+ return v4l2_subdev_get_fmt(sd, sd_state, sdformat);
+
+ pcpidev_fmt = find_imx_cpi_format(sdformat->format.code);
+ if (!pcpidev_fmt)
+ pcpidev_fmt = &imx_cpi_formats[0];
+
+ fmt = v4l2_subdev_state_get_format(sd_state, sdformat->pad);
+
+ fmt->code = pcpidev_fmt->code;
+ fmt->width = sdformat->format.width;
+ fmt->height = sdformat->format.height;
+ fmt->field = V4L2_FIELD_NONE;
+ fmt->colorspace = sdformat->format.colorspace;
+ fmt->quantization = sdformat->format.quantization;
+ fmt->xfer_func = sdformat->format.xfer_func;
+ fmt->ycbcr_enc = sdformat->format.ycbcr_enc;
+
+ sdformat->format = *fmt;
+
+ /* Propagate the format from sink to source. */
+ fmt = v4l2_subdev_state_get_format(sd_state, V4L2_SUBDEV_1TO1_PADS_SOURCE);
+ *fmt = sdformat->format;
+
+ /* The format on the source pad might change due to unpacking. */
+ fmt->code = pcpidev_fmt->output;
+
+ return 0;
+}
+
+static int imx_cpi_init_state(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state)
+{
+ struct v4l2_subdev_route routes[] = {
+ {
+ .sink_pad = V4L2_SUBDEV_1TO1_PADS_SINK,
+ .sink_stream = 0,
+ .source_pad = V4L2_SUBDEV_1TO1_PADS_SOURCE,
+ .source_stream = 0,
+ .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
+ },
+ };
+ struct v4l2_subdev_krouting routing = {
+ .len_routes = ARRAY_SIZE(routes),
+ .num_routes = ARRAY_SIZE(routes),
+ .routes = routes,
+ };
+ struct v4l2_mbus_framefmt *fmt;
+
+ fmt = v4l2_subdev_state_get_format(state, 0);
+
+ fmt->code = imx_cpi_formats[0].code;
+ fmt->width = 1920;
+ fmt->height = 1080;
+
+ fmt->colorspace = V4L2_COLORSPACE_SMPTE170M;
+ fmt->xfer_func = V4L2_MAP_XFER_FUNC_DEFAULT(fmt->colorspace);
+ fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt->colorspace);
+ fmt->quantization = V4L2_MAP_QUANTIZATION_DEFAULT(false,
+ fmt->colorspace,
+ fmt->ycbcr_enc);
+
+ return v4l2_subdev_set_routing_with_fmt(sd, state, &routing, fmt);
+}
+
+static int imx_cpi_disable_streams(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state, u32 pad,
+ u64 streams_mask)
+{
+ struct imx_cpi_device *pcpidev = sd_to_imx_cpi_device(sd);
+ struct media_pad *sink_pad, *remote_pad;
+ struct device *dev = pcpidev->dev;
+ struct v4l2_subdev *remote_sd;
+ u64 mask;
+ int ret;
+
+ sink_pad = &sd->entity.pads[V4L2_SUBDEV_1TO1_PADS_SINK];
+ remote_pad = media_pad_remote_pad_first(sink_pad);
+ remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
+
+ mask = v4l2_subdev_state_xlate_streams(state, V4L2_SUBDEV_1TO1_PADS_SINK,
+ V4L2_SUBDEV_1TO1_PADS_SOURCE,
+ &streams_mask);
+
+ ret = v4l2_subdev_disable_streams(remote_sd, remote_pad->index, mask);
+ if (ret)
+ dev_err(dev, "failed to disable streams on remote subdev: %d\n", ret);
+
+ pcpidev->enabled_streams &= ~streams_mask;
+
+ if (!pcpidev->enabled_streams) {
+ imx_cpi_disable(pcpidev);
+ pm_runtime_put_autosuspend(dev);
+ }
+
+ return 0;
+}
+
+static int imx_cpi_enable_streams(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *state, u32 pad,
+ u64 streams_mask)
+{
+ struct imx_cpi_device *pcpidev = sd_to_imx_cpi_device(sd);
+ const struct imx_cpi_pix_format *pcpidev_fmt;
+ const struct v4l2_mbus_framefmt *format;
+ struct media_pad *sink_pad, *remote_pad;
+ struct device *dev = pcpidev->dev;
+ struct v4l2_subdev *remote_sd;
+ u64 mask;
+ int ret;
+
+ sink_pad = &sd->entity.pads[V4L2_SUBDEV_1TO1_PADS_SINK];
+ remote_pad = media_pad_remote_pad_first(sink_pad);
+ remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
+
+ mask = v4l2_subdev_state_xlate_streams(state, V4L2_SUBDEV_1TO1_PADS_SINK,
+ V4L2_SUBDEV_1TO1_PADS_SOURCE,
+ &streams_mask);
+
+ format = v4l2_subdev_state_get_format(state, V4L2_SUBDEV_1TO1_PADS_SINK);
+ pcpidev_fmt = find_imx_cpi_format(format->code);
+
+ if (!pcpidev->enabled_streams) {
+ ret = pm_runtime_resume_and_get(dev);
+ if (ret)
+ return ret;
+
+ imx_cpi_hw_config(pcpidev, pcpidev_fmt);
+ imx_cpi_config_ctrl_reg1(pcpidev, format);
+ imx_cpi_enable(pcpidev);
+ }
+
+ ret = v4l2_subdev_enable_streams(remote_sd, remote_pad->index, mask);
+ if (ret)
+ goto err_cpi_stop;
+
+ pcpidev->enabled_streams |= streams_mask;
+
+ return 0;
+
+err_cpi_stop:
+ /* Stop CSI hardware if no streams are enabled */
+ if (!pcpidev->enabled_streams)
+ imx_cpi_disable(pcpidev);
+
+ pm_runtime_put_autosuspend(dev);
+
+ return ret;
+}
+
+static int imx_cpi_enum_mbus_code(struct v4l2_subdev *sd,
+ struct v4l2_subdev_state *sd_state,
+ struct v4l2_subdev_mbus_code_enum *code)
+{
+ /*
+ * The PARALLEL CPI can't transcode in any way, the source format
+ * is identical to the sink format.
+ */
+ if (code->pad == V4L2_SUBDEV_1TO1_PADS_SOURCE) {
+ struct v4l2_mbus_framefmt *fmt;
+
+ if (code->index > 0)
+ return -EINVAL;
+
+ fmt = v4l2_subdev_state_get_format(sd_state, code->pad);
+ code->code = fmt->code;
+ return 0;
+ }
+
+ if (code->pad != V4L2_SUBDEV_1TO1_PADS_SINK)
+ return -EINVAL;
+
+ if (code->index >= ARRAY_SIZE(imx_cpi_formats))
+ return -EINVAL;
+
+ code->code = imx_cpi_formats[code->index].code;
+
+ return 0;
+}
+
+static int
+imx_cpi_set_pad_by_ep(struct v4l2_subdev *sd, struct media_pad *pad)
+{
+ struct v4l2_fwnode_endpoint *vep = &pad->vep;
+
+ if (vep->base.port == V4L2_SUBDEV_1TO1_PADS_SINK) {
+ /* Sink port */
+
+ if (vep->bus_type != V4L2_MBUS_PARALLEL)
+ return -EINVAL;
+
+ pad->flags = MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_MUST_CONNECT;
+
+ return 0;
+ }
+
+ if (vep->base.port == V4L2_SUBDEV_1TO1_PADS_SOURCE) {
+ /* Source port */
+ pad->flags = MEDIA_PAD_FL_SOURCE | MEDIA_PAD_FL_MUST_CONNECT;
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
+static const struct v4l2_subdev_video_ops imx_cpi_video_ops = {
+ .s_stream = v4l2_subdev_s_stream_helper,
+};
+
+static const struct v4l2_subdev_pad_ops imx_cpi_pad_ops = {
+ .enum_mbus_code = imx_cpi_enum_mbus_code,
+ .get_fmt = v4l2_subdev_get_fmt,
+ .set_fmt = imx_cpi_set_fmt,
+ .get_frame_desc = v4l2_subdev_get_frame_desc_passthrough,
+ .enable_streams = imx_cpi_enable_streams,
+ .disable_streams = imx_cpi_disable_streams,
+};
+
+static const struct v4l2_subdev_ops imx_cpi_subdev_ops = {
+ .pad = &imx_cpi_pad_ops,
+ .video = &imx_cpi_video_ops,
+};
+
+static const struct v4l2_subdev_internal_ops imx_cpi_internal_ops = {
+ .init_state = imx_cpi_init_state,
+ .set_pad_by_ep = imx_cpi_set_pad_by_ep,
+};
+
+/* ----------------------------------------------------------------------
+ * Suspend/resume
+ */
+
+static int imx_cpi_runtime_suspend(struct device *dev)
+{
+ struct imx_cpi_device *pcpidev = dev_get_drvdata(dev);
+
+ clk_bulk_disable_unprepare(pcpidev->num_clks, pcpidev->clks);
+
+ return 0;
+}
+
+static int imx_cpi_runtime_resume(struct device *dev)
+{
+ struct imx_cpi_device *pcpidev = dev_get_drvdata(dev);
+
+ return clk_bulk_prepare_enable(pcpidev->num_clks, pcpidev->clks);
+}
+
+static const struct dev_pm_ops imx_cpi_pm_ops = {
+ RUNTIME_PM_OPS(imx_cpi_runtime_suspend, imx_cpi_runtime_resume, NULL)
+};
+
+static void imx_cpi_remove(struct platform_device *pdev)
+{
+ struct imx_cpi_device *pcpidev = platform_get_drvdata(pdev);
+
+ media_async_subdev_cleanup(&pcpidev->sd);
+}
+
+static int imx_cpi_probe(struct platform_device *pdev)
+{
+ struct imx_cpi_device *pcpidev;
+ struct device *dev = &pdev->dev;
+ struct v4l2_subdev *sd;
+ int ret;
+
+ pcpidev = devm_kzalloc(dev, sizeof(*pcpidev), GFP_KERNEL);
+ if (!pcpidev)
+ return -ENOMEM;
+
+ pcpidev->dev = dev;
+ platform_set_drvdata(pdev, pcpidev);
+
+ pcpidev->pdata = of_device_get_match_data(dev);
+ pcpidev->mode = PI_GATE_CLOCK_MODE;
+
+ pcpidev->regs = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(pcpidev->regs))
+ return dev_err_probe(dev, PTR_ERR(pcpidev->regs),
+ "Failed to get regs\n");
+
+ pcpidev->num_clks = devm_clk_bulk_get_all(dev, &pcpidev->clks);
+ if (pcpidev->num_clks < 0)
+ return pcpidev->num_clks;
+
+ sd = &pcpidev->sd;
+
+ v4l2_subdev_init(sd, &imx_cpi_subdev_ops);
+
+ sd->internal_ops = &imx_cpi_internal_ops;
+ snprintf(sd->name, sizeof(sd->name), "parallel-%s",
+ dev_name(pcpidev->dev));
+
+ sd->flags |= V4L2_SUBDEV_FL_HAS_DEVNODE;
+
+ sd->entity.function = MEDIA_ENT_F_VID_IF_BRIDGE;
+ sd->entity.ops = &imx_cpi_entity_ops;
+
+ sd->dev = pcpidev->dev;
+
+ pm_runtime_use_autosuspend(dev);
+ ret = devm_pm_runtime_enable(dev);
+ if (ret)
+ return ret;
+
+ return media_async_register_subdev(sd);
+}
+
+static const struct of_device_id imx_cpi_of_match[] = {
+ { .compatible = "fsl,imx8qxp-pcif", .data = &imx8qxp_pdata },
+ { },
+};
+
+MODULE_DEVICE_TABLE(of, imx_cpi_of_match);
+
+static struct platform_driver imx_cpi_driver = {
+ .probe = imx_cpi_probe,
+ .remove = imx_cpi_remove,
+ .driver = {
+ .of_match_table = imx_cpi_of_match,
+ .name = "imx-parallel-cpi",
+ .pm = pm_ptr(&imx_cpi_pm_ops),
+ },
+};
+
+module_platform_driver(imx_cpi_driver);
+
+MODULE_DESCRIPTION("i.MX9 Parallel CPI receiver driver");
+MODULE_LICENSE("GPL");
--
2.43.0
^ permalink raw reply related
* [PATCH v6 6/9] dt-bindings: media: add i.MX parallel CPI support
From: Frank.Li @ 2026-06-24 20:37 UTC (permalink / raw)
To: Sakari Ailus, Mauro Carvalho Chehab, Michael Riesch,
Laurent Pinchart, Frank Li, Martin Kepplinger-Novakovic,
Rui Miguel Silva, Purism Kernel Team, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam
Cc: linux-media, linux-kernel, imx, Guoniu Zhou, devicetree,
linux-arm-kernel, Alice Yuan, Krzysztof Kozlowski
In-Reply-To: <20260624-imx8qxp_pcam-v6-0-4b3f45920d2f@nxp.com>
From: Alice Yuan <alice.yuan@nxp.com>
Document the binding for parallel CPI controller found in i.MX8QXP, i.MX93
and i.MX91 SoCs.
Signed-off-by: Alice Yuan <alice.yuan@nxp.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Chagnes in v4
- add Laurent Pinchart's review by tag
- fix $ref: /schemas/graph.yaml#/$defs/port-base, original is
$ref: /schemas/graph.yaml#/properties/port-base
Change in v3:
- use enum at compatible string
- add ref to video-interfaces.yaml#
- use cpi as node name in examples.
- replace csi (Camera Serial Interface) with CPI (Camera Parallel Interface)
in commit message.
Change in v2:
- use pcif surfix as Laurent Pinchart's suggest.
- put power-domains into required list
---
.../devicetree/bindings/media/fsl,imx93-pcif.yaml | 126 +++++++++++++++++++++
MAINTAINERS | 1 +
2 files changed, 127 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/fsl,imx93-pcif.yaml b/Documentation/devicetree/bindings/media/fsl,imx93-pcif.yaml
new file mode 100644
index 0000000000000..9dd0331f6ef75
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/fsl,imx93-pcif.yaml
@@ -0,0 +1,126 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/media/fsl,imx93-pcif.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: i.MX8/9 Parallel Camera Interface
+
+maintainers:
+ - Frank Li <Frank.Li@nxp.com>
+
+description: |
+ This is device node for the Parallel Camera Interface which enables the
+ chip to connect directly to external Parallel CMOS image sensors.
+ Supports up to 80MHz input clock from sensor.
+ Supports the following input data formats
+ - 8-bit/10-bit Camera Sensor Interface (CSI)
+ - 8-bit data port for RGB, YCbCr, and YUV data input
+ - 8-bit/10-bit data ports for Bayer data input
+ Parallel Camera Interface is hooked to the Imaging subsystem via the
+ Pixel Link.
+
+properties:
+ compatible:
+ oneOf:
+ - enum:
+ - fsl,imx8qxp-pcif
+ - fsl,imx93-pcif
+ - items:
+ - enum:
+ - fsl,imx91-pcif
+ - const: fsl,imx93-pcif
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 2
+
+ clock-names:
+ items:
+ - const: pixel
+ - const: ipg
+
+ power-domains:
+ maxItems: 1
+
+ ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+
+ properties:
+ port@0:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ unevaluatedProperties: false
+ description: Input port node.
+
+ properties:
+ endpoint:
+ $ref: video-interfaces.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ bus-type:
+ const: 5
+
+ port@1:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ unevaluatedProperties: false
+ description: Output port node.
+
+ properties:
+ endpoint:
+ $ref: video-interfaces.yaml#
+ unevaluatedProperties: false
+
+ properties:
+ bus-type:
+ const: 5
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - clock-names
+ - power-domains
+ - ports
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/imx93-clock.h>
+ #include <dt-bindings/power/fsl,imx93-power.h>
+
+ cpi@4ac10070 {
+ compatible = "fsl,imx93-pcif";
+ reg = <0x4ac10070 0x10>;
+ clocks = <&clk IMX93_CLK_MIPI_CSI_GATE>,
+ <&clk IMX93_CLK_MEDIA_APB>;
+ clock-names = "pixel", "ipg";
+ assigned-clocks = <&clk IMX93_CLK_CAM_PIX>;
+ assigned-clock-parents = <&clk IMX93_CLK_VIDEO_PLL>;
+ assigned-clock-rates = <140000000>;
+ power-domains = <&media_blk_ctrl IMX93_MEDIABLK_PD_MIPI_CSI>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+
+ endpoint {
+ remote-endpoint = <&mt9m114_ep>;
+ };
+ };
+
+ port@1 {
+ reg = <1>;
+ endpoint {
+ remote-endpoint = <&isi_in>;
+ };
+ };
+ };
+ };
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index 895a87b571c35..51d5c62e3fdea 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16258,6 +16258,7 @@ L: linux-media@vger.kernel.org
S: Maintained
T: git git://linuxtv.org/media.git
F: Documentation/admin-guide/media/imx7.rst
+F: Documentation/devicetree/bindings/media/fsl,imx93-pcif.yaml
F: Documentation/devicetree/bindings/media/nxp,imx-mipi-csi2.yaml
F: Documentation/devicetree/bindings/media/nxp,imx7-csi.yaml
F: Documentation/devicetree/bindings/media/nxp,imx8mq-mipi-csi2.yaml
--
2.43.0
^ permalink raw reply related
* [PATCH v6 5/9] media: synopsys: Use media_async_register_subdev() to simplify code
From: Frank.Li @ 2026-06-24 20:37 UTC (permalink / raw)
To: Sakari Ailus, Mauro Carvalho Chehab, Michael Riesch,
Laurent Pinchart, Frank Li, Martin Kepplinger-Novakovic,
Rui Miguel Silva, Purism Kernel Team, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam
Cc: linux-media, linux-kernel, imx, Guoniu Zhou, devicetree,
linux-arm-kernel
In-Reply-To: <20260624-imx8qxp_pcam-v6-0-4b3f45920d2f@nxp.com>
From: Frank Li <Frank.Li@nxp.com>
Use the media_async_register_subdev() to simplify the driver.
Replace the local subdev registration and media pad setup code with
media_async_register_subdev(). Reduce boilerplate code and aligns the
driver with the common pattern used by simple subdevices that each media
pad has one endpoint in fwnode.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v6
- use media_async_register_subdev()
- remove sd_1to1.
change in v5
new patch
previous method:
https://lore.kernel.org/imx/20260226-v4l2_init_register-v2-2-902d7140f9fa@nxp.com/
---
drivers/media/platform/synopsys/dw-mipi-csi2rx.c | 178 +++++------------------
1 file changed, 40 insertions(+), 138 deletions(-)
diff --git a/drivers/media/platform/synopsys/dw-mipi-csi2rx.c b/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
index f51367409ff46..0fabc89a49b80 100644
--- a/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
+++ b/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
@@ -78,12 +78,6 @@ enum dw_mipi_csi2rx_regs_index {
DW_MIPI_CSI2RX_MAX,
};
-enum {
- DW_MIPI_CSI2RX_PAD_SINK,
- DW_MIPI_CSI2RX_PAD_SRC,
- DW_MIPI_CSI2RX_PAD_MAX,
-};
-
struct dw_mipi_csi2rx_device;
struct dw_mipi_csi2rx_drvdata {
@@ -112,12 +106,8 @@ struct dw_mipi_csi2rx_device {
const struct dw_mipi_csi2rx_format *formats;
unsigned int formats_num;
- struct media_pad pads[DW_MIPI_CSI2RX_PAD_MAX];
- struct v4l2_async_notifier notifier;
struct v4l2_subdev sd;
- enum v4l2_mbus_type bus_type;
- u32 lanes_num;
u64 enabled_streams;
const struct dw_mipi_csi2rx_drvdata *drvdata;
@@ -360,9 +350,10 @@ dw_mipi_csi2rx_find_format(struct dw_mipi_csi2rx_device *csi2, u32 mbus_code)
static int dw_mipi_csi2rx_start(struct dw_mipi_csi2rx_device *csi2)
{
+ struct media_pad *sink_pad = &csi2->sd.entity.pads[V4L2_SUBDEV_1TO1_PADS_SINK];
+ u32 lanes = sink_pad->vep.bus.mipi_csi2.num_data_lanes;
struct media_pad *source_pad;
union phy_configure_opts opts;
- u32 lanes = csi2->lanes_num;
u32 control = 0;
s64 link_freq;
int ret;
@@ -370,8 +361,7 @@ static int dw_mipi_csi2rx_start(struct dw_mipi_csi2rx_device *csi2)
if (lanes < 1 || lanes > 4)
return -EINVAL;
- source_pad = media_pad_remote_pad_unique(
- &csi2->pads[DW_MIPI_CSI2RX_PAD_SINK]);
+ source_pad = media_pad_remote_pad_unique(sink_pad);
if (IS_ERR(source_pad))
return PTR_ERR(source_pad);
@@ -380,7 +370,7 @@ static int dw_mipi_csi2rx_start(struct dw_mipi_csi2rx_device *csi2)
if (link_freq < 0)
return link_freq;
- switch (csi2->bus_type) {
+ switch (sink_pad->vep.bus_type) {
case V4L2_MBUS_CSI2_DPHY:
ret = phy_mipi_dphy_get_default_config_for_hsclk(link_freq * 2,
lanes, &opts.mipi_dphy);
@@ -458,16 +448,16 @@ dw_mipi_csi2rx_enum_mbus_code(struct v4l2_subdev *sd,
struct dw_mipi_csi2rx_device *csi2 = to_csi2(sd);
switch (code->pad) {
- case DW_MIPI_CSI2RX_PAD_SRC:
+ case V4L2_SUBDEV_1TO1_PADS_SOURCE:
if (code->index)
return -EINVAL;
code->code =
v4l2_subdev_state_get_format(sd_state,
- DW_MIPI_CSI2RX_PAD_SINK)->code;
+ V4L2_SUBDEV_1TO1_PADS_SINK)->code;
return 0;
- case DW_MIPI_CSI2RX_PAD_SINK:
+ case V4L2_SUBDEV_1TO1_PADS_SINK:
if (code->index >= csi2->formats_num)
return -EINVAL;
@@ -487,7 +477,7 @@ static int dw_mipi_csi2rx_set_fmt(struct v4l2_subdev *sd,
struct v4l2_mbus_framefmt *sink, *src;
/* the format on the source pad always matches the sink pad */
- if (format->pad == DW_MIPI_CSI2RX_PAD_SRC)
+ if (format->pad == V4L2_SUBDEV_1TO1_PADS_SOURCE)
return v4l2_subdev_get_fmt(sd, state, format);
sink = v4l2_subdev_state_get_format(state, format->pad, format->stream);
@@ -549,12 +539,12 @@ static int dw_mipi_csi2rx_enable_streams(struct v4l2_subdev *sd,
u64 mask;
int ret;
- sink_pad = &sd->entity.pads[DW_MIPI_CSI2RX_PAD_SINK];
+ sink_pad = &sd->entity.pads[V4L2_SUBDEV_1TO1_PADS_SINK];
remote_pad = media_pad_remote_pad_first(sink_pad);
remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
- mask = v4l2_subdev_state_xlate_streams(state, DW_MIPI_CSI2RX_PAD_SINK,
- DW_MIPI_CSI2RX_PAD_SRC,
+ mask = v4l2_subdev_state_xlate_streams(state, V4L2_SUBDEV_1TO1_PADS_SINK,
+ V4L2_SUBDEV_1TO1_PADS_SOURCE,
&streams_mask);
if (!csi2->enabled_streams) {
@@ -608,12 +598,12 @@ static int dw_mipi_csi2rx_disable_streams(struct v4l2_subdev *sd,
u64 mask;
int ret;
- sink_pad = &sd->entity.pads[DW_MIPI_CSI2RX_PAD_SINK];
+ sink_pad = &sd->entity.pads[V4L2_SUBDEV_1TO1_PADS_SINK];
remote_pad = media_pad_remote_pad_first(sink_pad);
remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
- mask = v4l2_subdev_state_xlate_streams(state, DW_MIPI_CSI2RX_PAD_SINK,
- DW_MIPI_CSI2RX_PAD_SRC,
+ mask = v4l2_subdev_state_xlate_streams(state, V4L2_SUBDEV_1TO1_PADS_SINK,
+ V4L2_SUBDEV_1TO1_PADS_SOURCE,
&streams_mask);
ret = v4l2_subdev_disable_streams(remote_sd, remote_pad->index, mask);
@@ -649,9 +639,9 @@ static int dw_mipi_csi2rx_init_state(struct v4l2_subdev *sd,
{
struct v4l2_subdev_route routes[] = {
{
- .sink_pad = DW_MIPI_CSI2RX_PAD_SINK,
+ .sink_pad = V4L2_SUBDEV_1TO1_PADS_SINK,
.sink_stream = 0,
- .source_pad = DW_MIPI_CSI2RX_PAD_SRC,
+ .source_pad = V4L2_SUBDEV_1TO1_PADS_SOURCE,
.source_stream = 0,
.flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE,
},
@@ -666,95 +656,38 @@ static int dw_mipi_csi2rx_init_state(struct v4l2_subdev *sd,
&default_format);
}
-static const struct v4l2_subdev_internal_ops dw_mipi_csi2rx_internal_ops = {
- .init_state = dw_mipi_csi2rx_init_state,
-};
-
-static int dw_mipi_csi2rx_notifier_bound(struct v4l2_async_notifier *notifier,
- struct v4l2_subdev *sd,
- struct v4l2_async_connection *asd)
+static int dw_mipi_set_pad_by_ep(struct v4l2_subdev *sd, struct media_pad *pad)
{
- struct dw_mipi_csi2rx_device *csi2 =
- container_of(notifier, struct dw_mipi_csi2rx_device, notifier);
- struct media_pad *sink_pad = &csi2->pads[DW_MIPI_CSI2RX_PAD_SINK];
- int ret;
-
- ret = v4l2_create_fwnode_links_to_pad(sd, sink_pad,
- MEDIA_LNK_FL_ENABLED);
- if (ret) {
- dev_err(csi2->dev, "failed to link source pad of %s\n",
- sd->name);
- return ret;
- }
-
- return 0;
-}
+ struct v4l2_fwnode_endpoint *vep = &pad->vep;
-static const struct v4l2_async_notifier_operations dw_mipi_csi2rx_notifier_ops = {
- .bound = dw_mipi_csi2rx_notifier_bound,
-};
-
-static int dw_mipi_csi2rx_register_notifier(struct dw_mipi_csi2rx_device *csi2)
-{
- struct v4l2_async_connection *asd;
- struct v4l2_async_notifier *ntf = &csi2->notifier;
- struct v4l2_fwnode_endpoint vep;
- struct v4l2_subdev *sd = &csi2->sd;
- struct device *dev = csi2->dev;
- int ret;
-
- struct fwnode_handle *ep __free(fwnode_handle) =
- fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 0, 0, 0);
- if (!ep)
- return dev_err_probe(dev, -ENODEV, "failed to get endpoint\n");
-
- vep.bus_type = V4L2_MBUS_UNKNOWN;
- ret = v4l2_fwnode_endpoint_parse(ep, &vep);
- if (ret)
- return dev_err_probe(dev, ret, "failed to parse endpoint\n");
-
- if (vep.bus_type != V4L2_MBUS_CSI2_DPHY &&
- vep.bus_type != V4L2_MBUS_CSI2_CPHY)
- return dev_err_probe(dev, -EINVAL,
- "invalid bus type of endpoint\n");
-
- csi2->bus_type = vep.bus_type;
- csi2->lanes_num = vep.bus.mipi_csi2.num_data_lanes;
+ if (vep->base.port == V4L2_SUBDEV_1TO1_PADS_SINK) {
+ if (vep->bus_type != V4L2_MBUS_CSI2_DPHY &&
+ vep->bus_type != V4L2_MBUS_CSI2_CPHY)
+ return -EINVAL;
- v4l2_async_subdev_nf_init(ntf, sd);
- ntf->ops = &dw_mipi_csi2rx_notifier_ops;
+ pad->flags = MEDIA_PAD_FL_SINK | MEDIA_PAD_FL_MUST_CONNECT;
- asd = v4l2_async_nf_add_fwnode_remote(ntf, ep,
- struct v4l2_async_connection);
- if (IS_ERR(asd)) {
- ret = PTR_ERR(asd);
- goto err_nf_cleanup;
+ return 0;
}
- ret = v4l2_async_nf_register(ntf);
- if (ret) {
- ret = dev_err_probe(dev, ret, "failed to register notifier\n");
- goto err_nf_cleanup;
+ if (vep->base.port == V4L2_SUBDEV_1TO1_PADS_SOURCE) {
+ pad->flags = MEDIA_PAD_FL_SOURCE | MEDIA_PAD_FL_MUST_CONNECT;
+ return 0;
}
- return 0;
-
-err_nf_cleanup:
- v4l2_async_nf_cleanup(ntf);
-
- return ret;
+ return -EINVAL;
}
+static const struct v4l2_subdev_internal_ops dw_mipi_csi2rx_internal_ops = {
+ .init_state = dw_mipi_csi2rx_init_state,
+ .set_pad_by_ep = dw_mipi_set_pad_by_ep,
+};
+
static int dw_mipi_csi2rx_register(struct dw_mipi_csi2rx_device *csi2)
{
- struct media_pad *pads = csi2->pads;
struct v4l2_subdev *sd = &csi2->sd;
int ret;
- ret = dw_mipi_csi2rx_register_notifier(csi2);
- if (ret)
- goto err;
-
v4l2_subdev_init(sd, &dw_mipi_csi2rx_ops);
sd->dev = csi2->dev;
sd->entity.ops = &dw_mipi_csi2rx_media_ops;
@@ -764,45 +697,12 @@ static int dw_mipi_csi2rx_register(struct dw_mipi_csi2rx_device *csi2)
snprintf(sd->name, sizeof(sd->name), "dw-mipi-csi2rx %s",
dev_name(csi2->dev));
- pads[DW_MIPI_CSI2RX_PAD_SINK].flags = MEDIA_PAD_FL_SINK |
- MEDIA_PAD_FL_MUST_CONNECT;
- pads[DW_MIPI_CSI2RX_PAD_SRC].flags = MEDIA_PAD_FL_SOURCE;
- ret = media_entity_pads_init(&sd->entity, DW_MIPI_CSI2RX_PAD_MAX, pads);
+ ret = media_async_register_subdev(&csi2->sd);
if (ret)
- goto err_notifier_unregister;
-
- ret = v4l2_subdev_init_finalize(sd);
- if (ret)
- goto err_entity_cleanup;
-
- ret = v4l2_async_register_subdev(sd);
- if (ret) {
- dev_err(sd->dev, "failed to register CSI-2 subdev\n");
- goto err_subdev_cleanup;
- }
+ return dev_err_probe(sd->dev, ret,
+ "failed to register CSI-2 subdev\n");
return 0;
-
-err_subdev_cleanup:
- v4l2_subdev_cleanup(sd);
-err_entity_cleanup:
- media_entity_cleanup(&sd->entity);
-err_notifier_unregister:
- v4l2_async_nf_unregister(&csi2->notifier);
- v4l2_async_nf_cleanup(&csi2->notifier);
-err:
- return ret;
-}
-
-static void dw_mipi_csi2rx_unregister(struct dw_mipi_csi2rx_device *csi2)
-{
- struct v4l2_subdev *sd = &csi2->sd;
-
- v4l2_async_unregister_subdev(sd);
- v4l2_subdev_cleanup(sd);
- media_entity_cleanup(&sd->entity);
- v4l2_async_nf_unregister(&csi2->notifier);
- v4l2_async_nf_cleanup(&csi2->notifier);
}
static void imx93_csi2rx_dphy_assert_reset(struct dw_mipi_csi2rx_device *csi2)
@@ -879,12 +779,14 @@ static void imx93_csi2rx_dphy_ipi_enable(struct dw_mipi_csi2rx_device *csi2)
static int imx93_csi2rx_wait_for_phy_stopstate(struct dw_mipi_csi2rx_device *csi2)
{
+ struct media_pad *sink_pad = &csi2->sd.entity.pads[V4L2_SUBDEV_1TO1_PADS_SINK];
+ u32 num_lanes = sink_pad->vep.bus.mipi_csi2.num_data_lanes;
struct device *dev = csi2->dev;
u32 stopstate_mask;
u32 val;
int ret;
- stopstate_mask = DPHY_STOPSTATE_CLK_LANE | GENMASK(csi2->lanes_num - 1, 0);
+ stopstate_mask = DPHY_STOPSTATE_CLK_LANE | GENMASK(num_lanes - 1, 0);
ret = read_poll_timeout(dw_mipi_csi2rx_read, val,
(val & stopstate_mask) == stopstate_mask,
@@ -993,7 +895,7 @@ static void dw_mipi_csi2rx_remove(struct platform_device *pdev)
{
struct dw_mipi_csi2rx_device *csi2 = platform_get_drvdata(pdev);
- dw_mipi_csi2rx_unregister(csi2);
+ media_async_subdev_cleanup(&csi2->sd);
phy_exit(csi2->phy);
}
--
2.43.0
^ permalink raw reply related
* [PATCH v6 4/9] media: synopsys: Use v4l2_subdev_get_frame_desc_passthrough()
From: Frank.Li @ 2026-06-24 20:37 UTC (permalink / raw)
To: Sakari Ailus, Mauro Carvalho Chehab, Michael Riesch,
Laurent Pinchart, Frank Li, Martin Kepplinger-Novakovic,
Rui Miguel Silva, Purism Kernel Team, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam
Cc: linux-media, linux-kernel, imx, Guoniu Zhou, devicetree,
linux-arm-kernel
In-Reply-To: <20260624-imx8qxp_pcam-v6-0-4b3f45920d2f@nxp.com>
From: Frank Li <Frank.Li@nxp.com>
Replace the local frame descriptor callback implementation with
v4l2_subdev_get_frame_desc_passthrough().
This helper provides the same functionality while avoiding duplicate
code and simplifying the driver implementation.
Reviewed-by: Guoniu Zhou <guoniu.zhou@nxp.com>
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v6
- collect gouniu's review by
change in v5
- new patch
---
drivers/media/platform/synopsys/dw-mipi-csi2rx.c | 22 +---------------------
1 file changed, 1 insertion(+), 21 deletions(-)
diff --git a/drivers/media/platform/synopsys/dw-mipi-csi2rx.c b/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
index 41e48365167e5..f51367409ff46 100644
--- a/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
+++ b/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
@@ -630,31 +630,11 @@ static int dw_mipi_csi2rx_disable_streams(struct v4l2_subdev *sd,
return ret;
}
-static int
-dw_mipi_csi2rx_get_frame_desc(struct v4l2_subdev *sd, unsigned int pad,
- struct v4l2_mbus_frame_desc *fd)
-{
- struct dw_mipi_csi2rx_device *csi2 = to_csi2(sd);
- struct v4l2_subdev *remote_sd;
- struct media_pad *remote_pad;
-
- remote_pad = media_pad_remote_pad_unique(&csi2->pads[DW_MIPI_CSI2RX_PAD_SINK]);
- if (IS_ERR(remote_pad)) {
- dev_err(csi2->dev, "can't get remote source pad\n");
- return PTR_ERR(remote_pad);
- }
-
- remote_sd = media_entity_to_v4l2_subdev(remote_pad->entity);
-
- return v4l2_subdev_call(remote_sd, pad, get_frame_desc,
- remote_pad->index, fd);
-}
-
static const struct v4l2_subdev_pad_ops dw_mipi_csi2rx_pad_ops = {
.enum_mbus_code = dw_mipi_csi2rx_enum_mbus_code,
.get_fmt = v4l2_subdev_get_fmt,
.set_fmt = dw_mipi_csi2rx_set_fmt,
- .get_frame_desc = dw_mipi_csi2rx_get_frame_desc,
+ .get_frame_desc = v4l2_subdev_get_frame_desc_passthrough,
.set_routing = dw_mipi_csi2rx_set_routing,
.enable_streams = dw_mipi_csi2rx_enable_streams,
.disable_streams = dw_mipi_csi2rx_disable_streams,
--
2.43.0
^ permalink raw reply related
* [PATCH v6 3/9] media: subdev: Add media_async_register_subdev() helper
From: Frank.Li @ 2026-06-24 20:37 UTC (permalink / raw)
To: Sakari Ailus, Mauro Carvalho Chehab, Michael Riesch,
Laurent Pinchart, Frank Li, Martin Kepplinger-Novakovic,
Rui Miguel Silva, Purism Kernel Team, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sascha Hauer,
Pengutronix Kernel Team, Fabio Estevam
Cc: linux-media, linux-kernel, imx, Guoniu Zhou, devicetree,
linux-arm-kernel
In-Reply-To: <20260624-imx8qxp_pcam-v6-0-4b3f45920d2f@nxp.com>
From: Frank Li <Frank.Li@nxp.com>
Add media_async_register_subdev(), a helper to register a V4L2 sub-device
with the asynchronous sub-device framework.
The helper assumes a 1:1 mapping between firmware endpoints and media pads.
During registration it parses the firmware graph, creates media pads for
all endpoints, and registers common asynchronous notifiers for sink
endpoints. These notifiers automatically create media links when the
corresponding remote source devices become available.
The set_pad_by_ep() callback allows drivers to determine the media pad
associated with a firmware endpoint and identify whether the endpoint
represents a sink pad.
By centralizing firmware graph parsing, media pad creation, notifier
registration, and link creation, this helper reduces duplicated code and
simplifies error handling in V4L2 sub-device drivers.
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
change in v6
- new patch
---
drivers/media/v4l2-core/v4l2-fwnode.c | 155 ++++++++++++++++++++++++++++++++++
include/media/v4l2-async.h | 39 +++++++++
2 files changed, 194 insertions(+)
diff --git a/drivers/media/v4l2-core/v4l2-fwnode.c b/drivers/media/v4l2-core/v4l2-fwnode.c
index 62a3a452f7884..169059654478f 100644
--- a/drivers/media/v4l2-core/v4l2-fwnode.c
+++ b/drivers/media/v4l2-core/v4l2-fwnode.c
@@ -26,6 +26,7 @@
#include <media/v4l2-async.h>
#include <media/v4l2-fwnode.h>
+#include <media/v4l2-mc.h>
#include <media/v4l2-subdev.h>
#include "v4l2-subdev-priv.h"
@@ -1302,6 +1303,160 @@ int __v4l2_async_register_subdev_sensor(struct v4l2_subdev *sd, struct module *m
}
EXPORT_SYMBOL_GPL(__v4l2_async_register_subdev_sensor);
+static int v4l2_common_notifier_bound(struct v4l2_async_notifier *notifier,
+ struct v4l2_subdev *sd,
+ struct v4l2_async_connection *asd)
+{
+ struct media_pad *pad = NULL;
+ int ret;
+
+ if (asd->match.type != V4L2_ASYNC_MATCH_TYPE_FWNODE)
+ return -EINVAL;
+
+ if (!asd->match.fwnode)
+ return -EINVAL;
+
+ struct fwnode_handle *remote __free(fwnode_handle) =
+ fwnode_graph_get_remote_endpoint(asd->match.fwnode);
+
+ for (int i = 0; i < notifier->sd->entity.num_pads; i++) {
+ if (notifier->sd->entity.pads[i].vep.base.local_fwnode == remote) {
+ pad = ¬ifier->sd->entity.pads[i];
+ break;
+ }
+ }
+
+ if (!pad) {
+ dev_err(notifier->sd->dev, "failed to find sink pad\n");
+ return -EINVAL;
+ }
+
+ ret = v4l2_create_fwnode_links_to_pad(sd, pad, MEDIA_LNK_FL_ENABLED);
+ if (ret) {
+ dev_err(sd->dev, "failed to link source pad\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct v4l2_async_notifier_operations v4l2_common_notifier_ops = {
+ .bound = v4l2_common_notifier_bound,
+};
+
+static int
+v4l2_async_nf_parse_fwnode(struct device *dev, struct media_pad *pads,
+ struct v4l2_async_notifier *notifier)
+{
+ struct v4l2_subdev *sd = notifier->sd;
+ struct v4l2_async_connection *asd;
+ struct media_pad *pad;
+ int ret;
+
+ if (!sd->internal_ops->set_pad_by_ep)
+ return dev_err_probe(dev, -EINVAL,
+ "Missed valiate_endpoint() callback\n");
+ pad = pads;
+
+ fwnode_graph_for_each_endpoint_scoped(dev_fwnode(dev), ep) {
+ u32 flags;
+
+ ret = v4l2_fwnode_endpoint_parse(ep, &pad->vep);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to parse endpoint\n");
+
+ ret = sd->internal_ops->set_pad_by_ep(sd, pad);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "Can support endponit\n");
+
+ flags = pad->flags;
+
+ pad++;
+
+ if (flags & MEDIA_PAD_FL_SOURCE)
+ continue; /* Bypass source port */
+
+ notifier->ops = &v4l2_common_notifier_ops;
+
+ asd = v4l2_async_nf_add_fwnode_remote(notifier, ep,
+ struct v4l2_async_connection);
+ if (IS_ERR(asd))
+ return dev_err_probe(dev, PTR_ERR(asd),
+ "failed to add notifier\n");
+ }
+
+ return 0;
+}
+
+void media_async_subdev_cleanup(struct v4l2_subdev *sd)
+{
+ v4l2_async_unregister_subdev(sd);
+ v4l2_subdev_cleanup(sd);
+ media_entity_cleanup(&sd->entity);
+ v4l2_async_nf_unregister(sd->subdev_notifier);
+ v4l2_async_nf_cleanup(sd->subdev_notifier);
+ kfree(sd->entity.pads);
+}
+EXPORT_SYMBOL_GPL(media_async_subdev_cleanup);
+
+int __media_async_register_subdev(struct v4l2_subdev *sd, struct module *module)
+{
+ struct device *dev = sd->dev;
+ u32 ep_count;
+ int ret;
+
+ if (WARN_ON(!sd->dev))
+ return -ENODEV;
+
+ struct v4l2_async_notifier *notifier __free(kfree) = kzalloc_obj(*notifier);
+ if (!notifier)
+ return -ENOMEM;
+
+ v4l2_async_subdev_nf_init(notifier, sd);
+
+ ep_count = fwnode_graph_get_endpoint_count(dev_fwnode(dev), 0);
+ if (!ep_count)
+ return dev_err_probe(dev, -EINVAL, "No connected endpoints\n");
+
+ struct media_pad *pads __free(kfree) = kzalloc_objs(struct media_pad, ep_count);
+ if (!pads)
+ return -ENOMEM;
+
+ ret = v4l2_async_nf_parse_fwnode(dev, pads, notifier);
+ if (ret < 0)
+ return ret;
+
+ ret = media_entity_pads_init(&sd->entity, ep_count, pads);
+ if (ret)
+ goto out_cleanup;
+
+ ret = v4l2_async_nf_register(notifier);
+ if (ret < 0)
+ goto out_cleanup;
+
+ ret = v4l2_subdev_init_finalize(sd);
+ if (ret)
+ goto out_unregister;
+
+ ret = __v4l2_async_register_subdev(sd, module);
+ if (ret < 0)
+ goto out_unregister;
+
+ sd->subdev_notifier = no_free_ptr(notifier);
+ retain_and_null_ptr(pads);
+
+ return 0;
+
+out_unregister:
+ v4l2_async_nf_unregister(notifier);
+
+out_cleanup:
+ v4l2_async_nf_cleanup(notifier);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(__media_async_register_subdev);
+
MODULE_DESCRIPTION("V4L2 fwnode binding parsing library");
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Sakari Ailus <sakari.ailus@linux.intel.com>");
diff --git a/include/media/v4l2-async.h b/include/media/v4l2-async.h
index 54a2d9620ed5b..ca41820f776c5 100644
--- a/include/media/v4l2-async.h
+++ b/include/media/v4l2-async.h
@@ -345,4 +345,43 @@ __v4l2_async_register_subdev_sensor(struct v4l2_subdev *sd, struct module *modul
* @sd: pointer to &struct v4l2_subdev
*/
void v4l2_async_unregister_subdev(struct v4l2_subdev *sd);
+
+enum v4l2_subdev_1to1_pads {
+ V4L2_SUBDEV_1TO1_PADS_SINK,
+ V4L2_SUBDEV_1TO1_PADS_SOURCE,
+ V4L2_SUBDEV_1TO1_PADS_TOTAL,
+};
+
+/**
+ * media_async_register_subdev - registers a sub-device to the asynchronous
+ * sub-device framework and parse set up common
+ * related devices
+ *
+ * @sd: pointer to struct &v4l2_subdev
+ *
+ * Register a V4L2 sub-device with the asynchronous sub-device framework.
+ * In addition to v4l2_async_register_subdev(), this function parses the
+ * firmware graph, creates media pads for the endpoints, and registers common
+ * notifiers to create media links between connected devices.
+ *
+ * This function also init media_pads.
+ *
+ * The sub-device is unregistered and cleanup by media_async_subdev_cleanup()
+ *
+ * While registered, the subdev module is marked as in-use.
+ *
+ * An error is returned if the module is no longer loaded on any attempts
+ * to register it.
+ */
+#define media_async_register_subdev(sd_1to1) \
+ __media_async_register_subdev(sd_1to1, THIS_MODULE)
+
+int __media_async_register_subdev(struct v4l2_subdev *sd_1to1, struct module *module);
+
+/**
+ * media_async_subdev_cleanup - unregistered and cleanup subdev and media pads
+ * @sd_1to1: pointer to struct &v4l2_subdev_1to1
+ */
+void media_async_subdev_cleanup(struct v4l2_subdev *sd_1to1);
+
#endif
--
2.43.0
^ permalink raw reply related
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