* Re: [PATCH v7 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: sashiko-bot @ 2026-05-22 14:53 UTC (permalink / raw)
To: Bryan O'Donoghue
Cc: robh, devicetree, conor+dt, olteanv, neil.armstrong, vkoul,
linux-phy
In-Reply-To: <20260522-x1e-csi2-phy-v7-1-79cb1280fad6@linaro.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The device tree example fails schema validation because `port@2`'s endpoint includes `data-lanes` and `clock-lanes` which are forbidden by the schema.
--
commit f15340b905d3f2f95e02027798a2eb5d4ed06bf5
Author: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
This commit adds a new base schema for the x1e80100 MIPI CSI2 PHY devices.
It supports CPHY, DPHY, and DPHY split-mode by modeling the CSIPHY devices
as individual nodes with three distinct ports.
> diff --git a/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
> new file mode 100644
> index 0000000000000..f434fb04447f6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
[ ... ]
> + port@2:
> + $ref: /schemas/graph.yaml#/$defs/port-base
> + description: Output to CAMSS controller.
> +
> + properties:
> + endpoint:
> + $ref: /schemas/graph.yaml#/$defs/endpoint-base
> + unevaluatedProperties: false
> + properties:
> + remote-endpoint: true
> + required:
> + - remote-endpoint
[ ... ]
> +examples:
> + - |
[ ... ]
> + port@2 {
> + reg = <2>;
> + csiphy0_out_ep: endpoint {
> + data-lanes = <3>;
> + clock-lanes = <4>;
[Severity: Low]
Will this example fail dt_binding_check schema validation?
The schema definition above for port@2's endpoint points to endpoint-base and
sets unevaluatedProperties to false, without locally defining data-lanes or
clock-lanes.
However, the example uses them here. Does the port@2 definition need a
reference to video-interfaces.yaml like the other ports, or should these
properties be removed from the example?
> + remote-endpoint = <&controller_in>;
> + };
> + };
> + };
> + };
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260522-x1e-csi2-phy-v7-0-79cb1280fad6@linaro.org?part=1
^ permalink raw reply
* Re: [PATCH 00/14] media: Add V4L2 H.264 stateless encode and VC8000E support
From: Nicolas Dufresne @ 2026-05-22 14:47 UTC (permalink / raw)
To: Paul Kocialkowski, devicetree, imx, linux-arm-kernel,
linux-kernel, linux-media
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Benjamin Gaignard,
Philipp Zabel, Mauro Carvalho Chehab, Hans Verkuil, Marco Felsch,
Michael Tretter
In-Reply-To: <20260522101653.2565125-1-paulk@sys-base.io>
[-- Attachment #1: Type: text/plain, Size: 5306 bytes --]
Hi Paul,
Le vendredi 22 mai 2026 à 12:16 +0200, Paul Kocialkowski a écrit :
> This series introduces support for the V4L2 H.264 stateless encode uAPI,
> core and support in the hantro driver for the Verisilicon VC8000E.
>
> While this is a first version that will likely need some level of rework,
> it is already usable for most common use-cases and supports constant
> bitrate rate-control.
>
> A GStreamer tree can be used to test the series at:
> https://github.com/paulkocialkowski/gstreamer/tree/v4l2codecs/h264enc
> And an example pipeline would look like:
> gst-launch-1.0 videotestsrc pattern=smpte num-buffers=25 ! video/x-raw,width=640,height=480 ! v4l2slh264enc rate-control=cbr bitrate=8000000 qp-min=8 qp-max=42 ! h264parse ! matroskamux ! filesink location=encode.mkv
>
> Note that documentation for the new uAPI is intentionally left out of
> this series since it has not yet received approval.
The unfortunate part is that the spec is important for the actual review and
interops analyses. That omission qualify this submission as RFC from my point of
view. Any chances you can reply to this cover filling the blank for the few
important bit (something similar but non spec style of:
https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/dev-stateless-decoder.html#initialization
- Querying capabilities
- Initialization
- Encoding
- Dynamic Resolution Changes**
The other sections are just going to be the same imho. The ** for the DRC is
because H.264 is not that complicated, its intra only, so you can reset the
encoder (use the drain flow). So that's probably something for future us
(VP9/AV1). Adding your view on these topics (or just matching what you have
implemented) will make review more constructive.
cheers,
Nicolas
>
> Marco Felsch (2):
> media: hantro: use hantro_decoded_buffer only for dst_vq
> arm64: dts: imx8mp: add VC8000E encoder node
>
> Paul Kocialkowski (12):
> media: h264: Add a more generic reflist builder init
> media: uapi: Add H.264 stateless encode support
> media: h264: Add SPS video definitions
> media: h264: Add stateless encode core
> media: h264: Add stateless encode rbsp
> media: h264: Add stateless encode reference management
> media: h264: Add stateless encode rate control
> media: verisilicon: Report default pixel coding for non-JPEG and fix JPEG case
> media: verisilicon: Cancel job with runtime pm put/clk disable on failure
> media: verisilicon: Add common encoder parm and frameintervals ioctls
> media: verisilicon: Add support for the VC8000E H.264 encoder
> media: verilisicon: imx8m: Add support for the VC8000E on i.MX8MP
>
> arch/arm64/boot/dts/freescale/imx8mp.dtsi | 11 +
> drivers/media/platform/verisilicon/Kconfig | 1 +
> drivers/media/platform/verisilicon/Makefile | 2 +
> drivers/media/platform/verisilicon/hantro.h | 17 +
> .../media/platform/verisilicon/hantro_drv.c | 180 +-
> .../media/platform/verisilicon/hantro_h264.c | 6 +-
> .../media/platform/verisilicon/hantro_hw.h | 28 +
> .../media/platform/verisilicon/hantro_v4l2.c | 123 +-
> .../platform/verisilicon/hantro_vc8000e.c | 68 +
> .../verisilicon/hantro_vc8000e_h264_enc.c | 883 +++++++
> .../verisilicon/hantro_vc8000e_regs.h | 2129 +++++++++++++++++
> .../media/platform/verisilicon/imx8m_vpu_hw.c | 113 +
> drivers/media/v4l2-core/Kconfig | 4 +
> drivers/media/v4l2-core/Makefile | 2 +
> drivers/media/v4l2-core/v4l2-ctrls-core.c | 62 +
> drivers/media/v4l2-core/v4l2-ctrls-defs.c | 4 +
> drivers/media/v4l2-core/v4l2-h264-enc-rbsp.c | 1173 +++++++++
> drivers/media/v4l2-core/v4l2-h264-enc-rc.c | 558 +++++
> drivers/media/v4l2-core/v4l2-h264-enc.c | 1322 ++++++++++
> drivers/media/v4l2-core/v4l2-h264.c | 69 +
> include/media/v4l2-ctrls.h | 2 +
> include/media/v4l2-h264-enc-rbsp.h | 72 +
> include/media/v4l2-h264-enc-rc.h | 108 +
> include/media/v4l2-h264-enc.h | 135 ++
> include/media/v4l2-h264.h | 146 ++
> include/uapi/linux/v4l2-controls.h | 33 +
> include/uapi/linux/videodev2.h | 1 +
> 27 files changed, 7231 insertions(+), 21 deletions(-)
> create mode 100644 drivers/media/platform/verisilicon/hantro_vc8000e.c
> create mode 100644 drivers/media/platform/verisilicon/hantro_vc8000e_h264_enc.c
> create mode 100644 drivers/media/platform/verisilicon/hantro_vc8000e_regs.h
> create mode 100644 drivers/media/v4l2-core/v4l2-h264-enc-rbsp.c
> create mode 100644 drivers/media/v4l2-core/v4l2-h264-enc-rc.c
> create mode 100644 drivers/media/v4l2-core/v4l2-h264-enc.c
> create mode 100644 include/media/v4l2-h264-enc-rbsp.h
> create mode 100644 include/media/v4l2-h264-enc-rc.h
> create mode 100644 include/media/v4l2-h264-enc.h
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* [PATCH] arm64: dts: qcom: kodiak: Elite-ify LPASS macros
From: Luca Weiss @ 2026-05-22 14:46 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, cros-qcom-dts-watchers
Cc: ~postmarketos/upstreaming, phone-devel, linux-arm-msm, devicetree,
linux-kernel, Luca Weiss
Due to initial kodiak/sc7280 bringup being done for Chrome platforms,
some Chrome-specific bits still remain in kodiak.dtsi, like the clocks
and power-domains for the LPASS RX/TX/WSA/VA macros.
Move them to sc7280-chrome-common.dtsi and put Elite (q6afecc)
equivalents in its place. The qcs6490-audioreach.dtsi file can also drop
deletion of power-domains properties then.
This follows previous commits moving Chrome-specific configuration to
the correct file, leaving kodiak.dtsi for Elite and
qcs6490-audioreach.dtsi for AudioReach.
No functional change intended. The clock-output-names property will now
exist for both Chrome and AudioReach devices but this shouldn't have any
relevant effect. And WSA macro clocks weren't added to Chrome because I
don't believe this would've ever worked given it already referenced
q6afecc and the nodes were originally added during AudioReach bringup.
Signed-off-by: Luca Weiss <luca.weiss@fairphone.com>
---
arch/arm64/boot/dts/qcom/kodiak.dtsi | 48 +++++++++++++---------
arch/arm64/boot/dts/qcom/qcs6490-audioreach.dtsi | 6 ---
arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi | 38 +++++++++++++++++
3 files changed, 67 insertions(+), 25 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/kodiak.dtsi b/arch/arm64/boot/dts/qcom/kodiak.dtsi
index c51beada8c7d..bdfa6cd47a4a 100644
--- a/arch/arm64/boot/dts/qcom/kodiak.dtsi
+++ b/arch/arm64/boot/dts/qcom/kodiak.dtsi
@@ -2681,14 +2681,18 @@ lpass_rx_macro: codec@3200000 {
pinctrl-names = "default";
pinctrl-0 = <&lpass_rx_swr_clk>, <&lpass_rx_swr_data>;
- clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>,
- <&lpass_aon LPASS_AON_CC_TX_MCLK_2X_CLK>,
+ clocks = <&q6afecc LPASS_CLK_ID_TX_CORE_MCLK LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
+ <&q6afecc LPASS_CLK_ID_TX_CORE_NPL_MCLK LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
+ <&q6afecc LPASS_HW_MACRO_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
+ <&q6afecc LPASS_HW_DCODEC_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
<&lpass_va_macro>;
- clock-names = "mclk", "npl", "fsgen";
+ clock-names = "mclk",
+ "npl",
+ "macro",
+ "dcodec",
+ "fsgen";
- power-domains = <&lpass_hm LPASS_CORE_CC_LPASS_CORE_HM_GDSC>,
- <&lpass_aon LPASS_AON_CC_LPASS_AUDIO_HM_GDSC>;
- power-domain-names = "macro", "dcodec";
+ clock-output-names = "mclk";
#clock-cells = <0>;
#sound-dai-cells = <1>;
@@ -2734,14 +2738,18 @@ lpass_tx_macro: codec@3220000 {
pinctrl-names = "default";
pinctrl-0 = <&lpass_tx_swr_clk>, <&lpass_tx_swr_data>;
- clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>,
- <&lpass_aon LPASS_AON_CC_TX_MCLK_2X_CLK>,
+ clocks = <&q6afecc LPASS_CLK_ID_TX_CORE_MCLK LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
+ <&q6afecc LPASS_CLK_ID_TX_CORE_NPL_MCLK LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
+ <&q6afecc LPASS_HW_MACRO_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
+ <&q6afecc LPASS_HW_DCODEC_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
<&lpass_va_macro>;
- clock-names = "mclk", "npl", "fsgen";
+ clock-names = "mclk",
+ "npl",
+ "macro",
+ "dcodec",
+ "fsgen";
- power-domains = <&lpass_hm LPASS_CORE_CC_LPASS_CORE_HM_GDSC>,
- <&lpass_aon LPASS_AON_CC_LPASS_AUDIO_HM_GDSC>;
- power-domain-names = "macro", "dcodec";
+ clock-output-names = "mclk";
#clock-cells = <0>;
#sound-dai-cells = <1>;
@@ -2785,8 +2793,8 @@ lpass_wsa_macro: codec@3240000 {
compatible = "qcom,sc7280-lpass-wsa-macro";
reg = <0x0 0x03240000 0x0 0x1000>;
- clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>,
- <&lpass_aon LPASS_AON_CC_TX_MCLK_2X_CLK>,
+ clocks = <&q6afecc LPASS_CLK_ID_TX_CORE_MCLK LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
+ <&q6afecc LPASS_CLK_ID_TX_CORE_NPL_MCLK LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
<&q6afecc LPASS_HW_MACRO_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
<&q6afecc LPASS_HW_DCODEC_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
<&lpass_va_macro>;
@@ -2858,12 +2866,14 @@ lpass_va_macro: codec@3370000 {
compatible = "qcom,sc7280-lpass-va-macro";
reg = <0 0x03370000 0 0x1000>;
- clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>;
- clock-names = "mclk";
+ clocks = <&q6afecc LPASS_CLK_ID_TX_CORE_MCLK LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
+ <&q6afecc LPASS_HW_MACRO_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
+ <&q6afecc LPASS_HW_DCODEC_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>;
+ clock-names = "mclk",
+ "macro",
+ "dcodec";
- power-domains = <&lpass_hm LPASS_CORE_CC_LPASS_CORE_HM_GDSC>,
- <&lpass_aon LPASS_AON_CC_LPASS_AUDIO_HM_GDSC>;
- power-domain-names = "macro", "dcodec";
+ clock-output-names = "fsgen";
#clock-cells = <0>;
#sound-dai-cells = <1>;
diff --git a/arch/arm64/boot/dts/qcom/qcs6490-audioreach.dtsi b/arch/arm64/boot/dts/qcom/qcs6490-audioreach.dtsi
index c1867711298b..037a5f6c030a 100644
--- a/arch/arm64/boot/dts/qcom/qcs6490-audioreach.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcs6490-audioreach.dtsi
@@ -11,8 +11,6 @@
#include <dt-bindings/sound/qcom,q6dsp-lpass-ports.h>
&lpass_rx_macro {
- /delete-property/ power-domains;
- /delete-property/ power-domain-names;
clocks = <&q6prmcc LPASS_CLK_ID_TX_CORE_MCLK LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
<&q6prmcc LPASS_CLK_ID_TX_CORE_NPL_MCLK LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
<&q6prmcc LPASS_HW_MACRO_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
@@ -33,8 +31,6 @@ &lpass_tlmm {
};
&lpass_tx_macro {
- /delete-property/ power-domains;
- /delete-property/ power-domain-names;
clocks = <&q6prmcc LPASS_CLK_ID_TX_CORE_MCLK LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
<&q6prmcc LPASS_CLK_ID_TX_CORE_NPL_MCLK LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
<&q6prmcc LPASS_HW_MACRO_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
@@ -48,8 +44,6 @@ &lpass_tx_macro {
};
&lpass_va_macro {
- /delete-property/ power-domains;
- /delete-property/ power-domain-names;
clocks = <&q6prmcc LPASS_CLK_ID_VA_CORE_MCLK LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
<&q6prmcc LPASS_HW_MACRO_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>,
<&q6prmcc LPASS_HW_DCODEC_VOTE LPASS_CLK_ATTRIBUTE_COUPLE_NO>;
diff --git a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
index debf62baec9b..5b25501484fa 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-chrome-common.dtsi
@@ -67,11 +67,49 @@ &lpass_hm {
status = "okay";
};
+&lpass_rx_macro {
+ power-domains = <&lpass_hm LPASS_CORE_CC_LPASS_CORE_HM_GDSC>,
+ <&lpass_aon LPASS_AON_CC_LPASS_AUDIO_HM_GDSC>;
+ power-domain-names = "macro",
+ "dcodec";
+
+ clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>,
+ <&lpass_aon LPASS_AON_CC_TX_MCLK_2X_CLK>,
+ <&lpass_va_macro>;
+ clock-names = "mclk",
+ "npl",
+ "fsgen";
+};
+
&lpass_tlmm {
/delete-property/ clocks;
/delete-property/ clock-names;
};
+&lpass_tx_macro {
+ clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>,
+ <&lpass_aon LPASS_AON_CC_TX_MCLK_2X_CLK>,
+ <&lpass_va_macro>;
+ clock-names = "mclk",
+ "npl",
+ "fsgen";
+
+ power-domains = <&lpass_hm LPASS_CORE_CC_LPASS_CORE_HM_GDSC>,
+ <&lpass_aon LPASS_AON_CC_LPASS_AUDIO_HM_GDSC>;
+ power-domain-names = "macro",
+ "dcodec";
+};
+
+&lpass_va_macro {
+ clocks = <&lpass_aon LPASS_AON_CC_TX_MCLK_CLK>;
+ clock-names = "mclk";
+
+ power-domains = <&lpass_hm LPASS_CORE_CC_LPASS_CORE_HM_GDSC>,
+ <&lpass_aon LPASS_AON_CC_LPASS_AUDIO_HM_GDSC>;
+ power-domain-names = "macro",
+ "dcodec";
+};
+
&lpasscc {
status = "okay";
};
---
base-commit: 1e45adb287ae5d431afc9900b4d387f4e73d9406
change-id: 20260522-kodiak-elite-macros-f4e5c185f63a
Best regards,
--
Luca Weiss <luca.weiss@fairphone.com>
^ permalink raw reply related
* [PATCH v7 2/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
From: Bryan O'Donoghue @ 2026-05-22 14:45 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: Bryan O'Donoghue, Vladimir Zapolskiy, linux-arm-msm,
linux-phy, linux-media, devicetree, linux-kernel,
Bryan O'Donoghue
In-Reply-To: <20260522-x1e-csi2-phy-v7-0-79cb1280fad6@linaro.org>
Add a new MIPI CSI2 driver in DPHY mode initially. The entire set of
existing CAMSS CSI PHY init sequences are imported in order to save time
and effort in later patches.
The following devices are supported in this drop:
"qcom,x1e80100-csi2-phy"
In-line with other PHY drivers the process node is included in the name.
Data-lane and clock lane positioning and polarity selection via newly
amended struct phy_configure_opts_mipi_dphy{} is supported.
The Qualcomm 3PH class of PHYs can do both DPHY and CPHY mode. For now only
DPHY is supported.
In porting some of the logic over from camss-csiphy*.c to here its also
possible to rationalise some of the code.
In particular use of regulator_bulk and clk_bulk as well as dropping the
seemingly useless and unused interrupt handler.
The PHY sequences and a lot of the logic that goes with them are well
proven in CAMSS and mature so the main thing to watch out for here is how
to get the right sequencing of regulators, clocks and register-writes.
The register init sequence table is imported verbatim from the existing
CAMSS csiphy driver. A follow-up series will rework the table to extract
the repetitive per-lane pattern into a loop.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
MAINTAINERS | 10 +
drivers/phy/qualcomm/Kconfig | 14 +
drivers/phy/qualcomm/Makefile | 5 +
drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c | 371 ++++++++++++++++++++
drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c | 386 +++++++++++++++++++++
drivers/phy/qualcomm/phy-qcom-mipi-csi2.h | 95 +++++
6 files changed, 881 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 63389fea5d150..3b5da8a40383f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22018,6 +22018,16 @@ S: Maintained
F: Documentation/devicetree/bindings/media/qcom,*-iris.yaml
F: drivers/media/platform/qcom/iris/
+QUALCOMM MIPI CSI2 PHY DRIVER
+M: Bryan O'Donoghue <bod@kernel.org>
+L: linux-phy@lists.infradead.org
+L: linux-media@vger.kernel.org
+L: linux-arm-msm@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/phy/qcom,*-csi2-phy.yaml
+F: drivers/phy/qualcomm/phy-qcom-mipi-csi2*.c
+F: drivers/phy/qualcomm/phy-qcom-mipi-csi2*.h
+
QUALCOMM NAND CONTROLLER DRIVER
M: Manivannan Sadhasivam <mani@kernel.org>
L: linux-mtd@lists.infradead.org
diff --git a/drivers/phy/qualcomm/Kconfig b/drivers/phy/qualcomm/Kconfig
index 60a0ead127fa9..779a3511ba852 100644
--- a/drivers/phy/qualcomm/Kconfig
+++ b/drivers/phy/qualcomm/Kconfig
@@ -28,6 +28,20 @@ config PHY_QCOM_EDP
Enable this driver to support the Qualcomm eDP PHY found in various
Qualcomm chipsets.
+config PHY_QCOM_MIPI_CSI2
+ tristate "Qualcomm MIPI CSI2 PHY driver"
+ depends on ARCH_QCOM || COMPILE_TEST
+ depends on OF
+ depends on PM
+ depends on COMMON_CLK
+ select GENERIC_PHY
+ select GENERIC_PHY_MIPI_DPHY
+ help
+ Enable this to support the MIPI CSI2 PHY driver found in various
+ Qualcomm chipsets. This PHY is used to connect MIPI CSI2
+ camera sensors to the CSI Decoder in the Qualcomm Camera Subsystem
+ CAMSS.
+
config PHY_QCOM_IPQ4019_USB
tristate "Qualcomm IPQ4019 USB PHY driver"
depends on OF && (ARCH_QCOM || COMPILE_TEST)
diff --git a/drivers/phy/qualcomm/Makefile b/drivers/phy/qualcomm/Makefile
index b71a6a0bed3f1..382cb594b06b6 100644
--- a/drivers/phy/qualcomm/Makefile
+++ b/drivers/phy/qualcomm/Makefile
@@ -6,6 +6,11 @@ obj-$(CONFIG_PHY_QCOM_IPQ4019_USB) += phy-qcom-ipq4019-usb.o
obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA) += phy-qcom-ipq806x-sata.o
obj-$(CONFIG_PHY_QCOM_M31_USB) += phy-qcom-m31.o
obj-$(CONFIG_PHY_QCOM_M31_EUSB) += phy-qcom-m31-eusb2.o
+
+phy-qcom-mipi-csi2-objs += phy-qcom-mipi-csi2-core.o \
+ phy-qcom-mipi-csi2-3ph-dphy.o
+obj-$(CONFIG_PHY_QCOM_MIPI_CSI2) += phy-qcom-mipi-csi2.o
+
obj-$(CONFIG_PHY_QCOM_PCIE2) += phy-qcom-pcie2.o
obj-$(CONFIG_PHY_QCOM_QMP_COMBO) += phy-qcom-qmp-combo.o phy-qcom-qmp-usbc.o
diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
new file mode 100644
index 0000000000000..1e7729d4ee5d0
--- /dev/null
+++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c
@@ -0,0 +1,371 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Qualcomm MSM Camera Subsystem - CSIPHY Module 3phase v1.0
+ *
+ * Copyright (c) 2011-2015, The Linux Foundation. All rights reserved.
+ * Copyright (C) 2016-2025 Linaro Ltd.
+ */
+
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/time64.h>
+
+#include "phy-qcom-mipi-csi2.h"
+
+#define CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(offset, n) ((offset) + 0x4 * (n))
+#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL0_PHY_SW_RESET BIT(0)
+#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL5_CLK_ENABLE BIT(7)
+#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_COMMON_PWRDN_B BIT(0)
+#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_SHOW_REV_ID BIT(1)
+#define CSIPHY_3PH_CMN_CSI_COMMON_CTRL10_IRQ_CLEAR_CMD BIT(0)
+#define CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(offset, n) ((offset) + 0xb0 + 0x4 * (n))
+
+#define CSIPHY_2PH_LN_CSI_2PHASE_CTRLn(n) (0x4 * (n))
+
+/*
+ * 3 phase CSI has 19 common status regs with only 0-10 being used
+ * and 11-18 being reserved.
+ */
+#define CSI_COMMON_STATUS_NUM 11
+/*
+ * There are a number of common control registers
+ * The offset to clear the CSIPHY IRQ status starts @ 22
+ * So to clear CSI_COMMON_STATUS0 this is CSI_COMMON_CONTROL22, STATUS1 is
+ * CONTROL23 and so on
+ */
+#define CSI_CTRL_STATUS_INDEX 22
+
+/*
+ * There are 43 COMMON_CTRL registers with regs after # 33 being reserved
+ */
+#define CSI_CTRL_MAX 33
+
+#define CSIPHY_DEFAULT_PARAMS 0
+#define CSIPHY_SETTLE_CNT_LOWER_BYTE 2
+#define CSIPHY_SKEW_CAL 7
+
+/* 4nm 2PH v 2.1.2 2p5Gbps 4 lane DPHY mode */
+static const struct
+mipi_csi2phy_lane_regs lane_regs_x1e80100[] = {
+ /* Power up lanes 2ph mode */
+ {.reg_addr = 0x1014, .reg_data = 0xd5, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x101c, .reg_data = 0x7a, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x1018, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+
+ {.reg_addr = 0x0094, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x00a0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0090, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0098, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0094, .reg_data = 0x07, .delay_us = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0030, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0000, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0038, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x002c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0034, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x001c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0014, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x003c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0004, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0020, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0008, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
+ {.reg_addr = 0x0010, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0094, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
+ {.reg_addr = 0x005c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
+ {.reg_addr = 0x0060, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
+ {.reg_addr = 0x0064, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
+
+ {.reg_addr = 0x0e94, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0ea0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0e90, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0e98, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0e94, .reg_data = 0x07, .delay_us = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0e30, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0e28, .reg_data = 0x04, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0e00, .reg_data = 0x80, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0e0c, .reg_data = 0xff, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0e38, .reg_data = 0x1f, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0e2c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0e34, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0e1c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0e14, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0e3c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0e04, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0e20, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0e08, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
+ {.reg_addr = 0x0e10, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
+
+ {.reg_addr = 0x0494, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x04a0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0490, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0498, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0494, .reg_data = 0x07, .delay_us = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0430, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0400, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0438, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x042c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0434, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x041c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0414, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x043c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0404, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0420, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0408, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
+ {.reg_addr = 0x0410, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0494, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
+ {.reg_addr = 0x045c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
+ {.reg_addr = 0x0460, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
+ {.reg_addr = 0x0464, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
+
+ {.reg_addr = 0x0894, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x08a0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0890, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0898, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0894, .reg_data = 0x07, .delay_us = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0830, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0800, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0838, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x082c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0834, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x081c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0814, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x083c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0804, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0820, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0808, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
+ {.reg_addr = 0x0810, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0894, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
+ {.reg_addr = 0x085c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
+ {.reg_addr = 0x0860, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
+ {.reg_addr = 0x0864, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
+
+ {.reg_addr = 0x0c94, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0ca0, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0c90, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0c98, .reg_data = 0x08, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0c94, .reg_data = 0x07, .delay_us = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0c30, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0c00, .reg_data = 0x8e, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0c38, .reg_data = 0xfe, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0c2c, .reg_data = 0x01, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0c34, .reg_data = 0x0f, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0c1c, .reg_data = 0x0a, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0c14, .reg_data = 0x60, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0c3c, .reg_data = 0xb8, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0c04, .reg_data = 0x0c, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0c20, .reg_data = 0x00, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0c08, .reg_data = 0x10, .param_type = CSIPHY_SETTLE_CNT_LOWER_BYTE},
+ {.reg_addr = 0x0c10, .reg_data = 0x52, .param_type = CSIPHY_DEFAULT_PARAMS},
+ {.reg_addr = 0x0c94, .reg_data = 0xd7, .param_type = CSIPHY_SKEW_CAL},
+ {.reg_addr = 0x0c5c, .reg_data = 0x00, .param_type = CSIPHY_SKEW_CAL},
+ {.reg_addr = 0x0c60, .reg_data = 0xbd, .param_type = CSIPHY_SKEW_CAL},
+ {.reg_addr = 0x0c64, .reg_data = 0x7f, .param_type = CSIPHY_SKEW_CAL},
+};
+
+static inline const struct mipi_csi2phy_device_regs *
+csi2phy_dev_to_regs(struct mipi_csi2phy_device *csi2phy)
+{
+ return &csi2phy->soc_cfg->reg_info;
+}
+
+static void phy_qcom_mipi_csi2_hw_version_read(struct mipi_csi2phy_device *csi2phy)
+{
+ const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
+ u32 tmp;
+
+ writel(CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_SHOW_REV_ID, csi2phy->base +
+ CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 6));
+
+ tmp = readl_relaxed(csi2phy->base +
+ CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->common_regs_offset, 12));
+ csi2phy->hw_version = tmp;
+
+ tmp = readl_relaxed(csi2phy->base +
+ CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->common_regs_offset, 13));
+ csi2phy->hw_version |= (tmp << 8) & 0xFF00;
+
+ tmp = readl_relaxed(csi2phy->base +
+ CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->common_regs_offset, 14));
+ csi2phy->hw_version |= (tmp << 16) & 0xFF0000;
+
+ tmp = readl_relaxed(csi2phy->base +
+ CSIPHY_3PH_CMN_CSI_COMMON_STATUSn(regs->common_regs_offset, 15));
+ csi2phy->hw_version |= (tmp << 24) & 0xFF000000;
+
+ dev_dbg_once(csi2phy->dev, "CSIPHY 3PH HW Version = 0x%08x\n", csi2phy->hw_version);
+}
+
+/*
+ * phy_qcom_mipi_csi2_reset - Perform software reset on CSIPHY module
+ * @phy_qcom_mipi_csi2: CSIPHY device
+ */
+static void phy_qcom_mipi_csi2_reset(struct mipi_csi2phy_device *csi2phy)
+{
+ const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
+
+ writel(CSIPHY_3PH_CMN_CSI_COMMON_CTRL0_PHY_SW_RESET,
+ csi2phy->base + CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 0));
+ usleep_range(5000, 8000);
+ writel(0x0, csi2phy->base +
+ CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 0));
+}
+
+/*
+ * phy_qcom_mipi_csi2_settle_cnt_calc - Calculate settle count value
+ *
+ * Helper function to calculate settle count value. This is
+ * based on the CSI2 T_hs_settle parameter which in turn
+ * is calculated based on the CSI2 transmitter link frequency.
+ *
+ * Return settle count value or 0 if the CSI2 link frequency
+ * is not available
+ */
+static u8 phy_qcom_mipi_csi2_settle_cnt_calc(s64 link_freq, u32 timer_clk_rate)
+{
+ u32 t_hs_prepare_max_ps;
+ u32 timer_period_ps;
+ u32 t_hs_settle_ps;
+ u8 settle_cnt;
+ u32 ui_ps;
+
+ if (link_freq <= 0)
+ return 0;
+
+ ui_ps = div_u64(PSEC_PER_SEC, link_freq);
+ ui_ps /= 2;
+ t_hs_prepare_max_ps = 85000 + 6 * ui_ps;
+ t_hs_settle_ps = t_hs_prepare_max_ps;
+
+ timer_period_ps = div_u64(PSEC_PER_SEC, timer_clk_rate);
+ settle_cnt = t_hs_settle_ps / timer_period_ps - 6;
+
+ return settle_cnt;
+}
+
+static void
+phy_qcom_mipi_csi2_gen2_config_lanes(struct mipi_csi2phy_device *csi2phy,
+ u8 settle_cnt)
+{
+ const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
+ const struct mipi_csi2phy_lane_regs *r = regs->init_seq;
+ int i, array_size = regs->lane_array_size;
+ u32 val;
+
+ for (i = 0; i < array_size; i++, r++) {
+ switch (r->param_type) {
+ case CSIPHY_SETTLE_CNT_LOWER_BYTE:
+ val = settle_cnt & 0xff;
+ break;
+ case CSIPHY_SKEW_CAL:
+ /* TODO: support application of skew from dt flag */
+ continue;
+ default:
+ val = r->reg_data;
+ break;
+ }
+ writel(val, csi2phy->base + r->reg_addr);
+ if (r->delay_us)
+ udelay(r->delay_us);
+ }
+}
+
+static int phy_qcom_mipi_csi2_lanes_enable(struct mipi_csi2phy_device *csi2phy,
+ struct mipi_csi2phy_stream_cfg *cfg)
+{
+ const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
+ struct mipi_csi2phy_lanes_cfg *lane_cfg = &cfg->lane_cfg;
+ u8 settle_cnt;
+ u8 val;
+ int i;
+
+ settle_cnt = phy_qcom_mipi_csi2_settle_cnt_calc(cfg->link_freq, csi2phy->timer_clk_rate);
+
+ /* Lane position enable in common reg offset */
+ val = BIT(csi2phy->stream_cfg.lane_cfg.clk.pos);
+ for (i = 0; i < cfg->num_data_lanes; i++)
+ val |= BIT(lane_cfg->data[i].pos);
+
+ writel(val, csi2phy->base +
+ CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 5));
+
+ /* Lane configuration for polarity @ CSIPHY-base + CTRL9 */
+ for (i = 0; i < cfg->num_data_lanes; i++) {
+ if (lane_cfg->data[i].pol) {
+ writel(BIT(2), csi2phy->base + CSIPHY_2PH_LN_CSI_2PHASE_CTRLn(9));
+ }
+ }
+
+ val = CSIPHY_3PH_CMN_CSI_COMMON_CTRL6_COMMON_PWRDN_B;
+ writel(val, csi2phy->base +
+ CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 6));
+
+ val = 0x02;
+ writel(val, csi2phy->base +
+ CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 7));
+
+ val = 0x00;
+ writel(val, csi2phy->base +
+ CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 0));
+
+ phy_qcom_mipi_csi2_gen2_config_lanes(csi2phy, settle_cnt);
+
+ /* IRQ_MASK registers - disable all interrupts */
+ for (i = CSI_COMMON_STATUS_NUM; i < CSI_CTRL_STATUS_INDEX; i++) {
+ writel(0, csi2phy->base +
+ CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, i));
+ }
+
+ return 0;
+}
+
+static void
+phy_qcom_mipi_csi2_lanes_disable(struct mipi_csi2phy_device *csi2phy,
+ struct mipi_csi2phy_stream_cfg *cfg)
+{
+ const struct mipi_csi2phy_device_regs *regs = csi2phy_dev_to_regs(csi2phy);
+
+ writel(0, csi2phy->base +
+ CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 5));
+
+ writel(0, csi2phy->base +
+ CSIPHY_3PH_CMN_CSI_COMMON_CTRLn(regs->common_regs_offset, 6));
+}
+
+static const struct mipi_csi2phy_hw_ops phy_qcom_mipi_csi2_ops_3ph_1_0 = {
+ .hw_version_read = phy_qcom_mipi_csi2_hw_version_read,
+ .reset = phy_qcom_mipi_csi2_reset,
+ .lanes_enable = phy_qcom_mipi_csi2_lanes_enable,
+ .lanes_disable = phy_qcom_mipi_csi2_lanes_disable,
+};
+
+static const char * const x1e_clks[] = {
+ "core",
+ "timer"
+};
+
+static const char * const x1e_supplies[] = {
+ "vdda-0p9",
+ "vdda-1p2"
+};
+
+static const char * const x1e_genpd_names[] = {
+ "mmcx",
+ "mx",
+};
+
+const struct mipi_csi2phy_soc_cfg mipi_csi2_dphy_4nm_x1e = {
+ .ops = &phy_qcom_mipi_csi2_ops_3ph_1_0,
+ .reg_info = {
+ .init_seq = lane_regs_x1e80100,
+ .lane_array_size = ARRAY_SIZE(lane_regs_x1e80100),
+ .common_regs_offset = 0x1000,
+ },
+ .supply_names = (const char **)x1e_supplies,
+ .num_supplies = ARRAY_SIZE(x1e_supplies),
+ .clk_names = (const char **)x1e_clks,
+ .num_clk = ARRAY_SIZE(x1e_clks),
+ .opp_clk = x1e_clks[0],
+ .timer_clk = x1e_clks[1],
+ .genpd_names = (const char **)x1e_genpd_names,
+ .num_genpd_names = ARRAY_SIZE(x1e_genpd_names),
+};
diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
new file mode 100644
index 0000000000000..578926bfaef45
--- /dev/null
+++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c
@@ -0,0 +1,386 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025, Linaro Ltd.
+ */
+#include <dt-bindings/phy/phy.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/pm_opp.h>
+#include <linux/phy/phy.h>
+#include <linux/phy/phy-mipi-dphy.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
+#include <linux/slab.h>
+
+#include "phy-qcom-mipi-csi2.h"
+
+static int
+phy_qcom_mipi_csi2_set_clock_rates(struct mipi_csi2phy_device *csi2phy,
+ s64 link_freq)
+{
+ struct device *dev = csi2phy->dev;
+ unsigned long opp_rate = link_freq / 4;
+ struct dev_pm_opp *opp;
+ long timer_rate;
+ int i, ret;
+
+ opp = dev_pm_opp_find_freq_ceil(dev, &opp_rate);
+ if (IS_ERR(opp)) {
+ dev_err(csi2phy->dev, "Couldn't find ceiling for %lld Hz\n",
+ link_freq);
+ return PTR_ERR(opp);
+ }
+
+ for (i = 0; i < csi2phy->pd_list->num_pds; i++) {
+ unsigned int perf = dev_pm_opp_get_required_pstate(opp, i);
+
+ ret = dev_pm_genpd_set_performance_state(csi2phy->pd_list->pd_devs[i], perf);
+ if (ret) {
+ dev_err(csi2phy->dev, "Couldn't set perf state %u\n",
+ perf);
+ dev_pm_opp_put(opp);
+ goto unset_pstate;
+ }
+ }
+ dev_pm_opp_put(opp);
+
+ ret = dev_pm_opp_set_rate(dev, opp_rate);
+ if (ret) {
+ dev_err(csi2phy->dev, "dev_pm_opp_set_rate() fail\n");
+ goto unset_opp_rate;
+ }
+
+ timer_rate = clk_round_rate(csi2phy->timer_clk, link_freq / 4);
+ if (timer_rate <= 0) {
+ ret = timer_rate;
+ goto unset_opp_rate;
+ }
+
+ ret = clk_set_rate(csi2phy->timer_clk, timer_rate);
+ if (ret)
+ goto unset_opp_rate;
+
+ csi2phy->timer_clk_rate = timer_rate;
+
+ return 0;
+
+unset_opp_rate:
+ dev_pm_opp_set_rate(dev, 0);
+
+unset_pstate:
+ while (i--)
+ dev_pm_genpd_set_performance_state(csi2phy->pd_list->pd_devs[i], 0);
+
+ return ret;
+}
+
+static int phy_qcom_mipi_csi2_configure(struct phy *phy,
+ union phy_configure_opts *opts)
+{
+ struct mipi_csi2phy_device *csi2phy = phy_get_drvdata(phy);
+ struct phy_configure_opts_mipi_dphy *dphy_cfg = &opts->mipi_dphy;
+ struct mipi_csi2phy_stream_cfg *stream_cfg = &csi2phy->stream_cfg;
+ int ret;
+
+ ret = phy_mipi_dphy_config_validate(dphy_cfg);
+ if (ret)
+ return ret;
+
+ if (dphy_cfg->lanes < 1 || dphy_cfg->lanes > CSI2_MAX_DATA_LANES)
+ return -EINVAL;
+
+ stream_cfg->link_freq = dphy_cfg->hs_clk_rate;
+
+ return 0;
+}
+
+static int phy_qcom_mipi_csi2_power_on(struct phy *phy)
+{
+ struct mipi_csi2phy_device *csi2phy = phy_get_drvdata(phy);
+ const struct mipi_csi2phy_hw_ops *ops = csi2phy->soc_cfg->ops;
+ struct device *dev = &phy->dev;
+ int ret;
+
+ ret = regulator_bulk_enable(csi2phy->soc_cfg->num_supplies,
+ csi2phy->supplies);
+ if (ret)
+ return ret;
+
+ ret = phy_qcom_mipi_csi2_set_clock_rates(csi2phy, csi2phy->stream_cfg.link_freq);
+ if (ret)
+ goto poweroff_phy;
+
+ ret = clk_bulk_prepare_enable(csi2phy->soc_cfg->num_clk,
+ csi2phy->clks);
+ if (ret) {
+ dev_err(dev, "failed to enable clocks, %d\n", ret);
+ goto unset_rate;
+ }
+
+ ops->reset(csi2phy);
+
+ ops->hw_version_read(csi2phy);
+
+ return ops->lanes_enable(csi2phy, &csi2phy->stream_cfg);
+
+unset_rate:
+ dev_pm_opp_set_rate(csi2phy->dev, 0);
+
+poweroff_phy:
+ regulator_bulk_disable(csi2phy->soc_cfg->num_supplies,
+ csi2phy->supplies);
+
+ return ret;
+}
+
+static int phy_qcom_mipi_csi2_power_off(struct phy *phy)
+{
+ struct mipi_csi2phy_device *csi2phy = phy_get_drvdata(phy);
+ const struct mipi_csi2phy_hw_ops *ops = csi2phy->soc_cfg->ops;
+ int i;
+
+ ops->lanes_disable(csi2phy, &csi2phy->stream_cfg);
+
+ for (i = 0; i < csi2phy->pd_list->num_pds; i++)
+ dev_pm_genpd_set_performance_state(csi2phy->pd_list->pd_devs[i], 0);
+
+ clk_bulk_disable_unprepare(csi2phy->soc_cfg->num_clk,
+ csi2phy->clks);
+ dev_pm_opp_set_rate(csi2phy->dev, 0);
+ regulator_bulk_disable(csi2phy->soc_cfg->num_supplies,
+ csi2phy->supplies);
+
+ return 0;
+}
+
+static const struct phy_ops phy_qcom_mipi_csi2_ops = {
+ .configure = phy_qcom_mipi_csi2_configure,
+ .power_on = phy_qcom_mipi_csi2_power_on,
+ .power_off = phy_qcom_mipi_csi2_power_off,
+ .owner = THIS_MODULE,
+};
+
+static struct phy *qcom_csi2_phy_xlate(struct device *dev,
+ const struct of_phandle_args *args)
+{
+ struct mipi_csi2phy_device *csi2phy = dev_get_drvdata(dev);
+
+ if (args->args[0] != PHY_TYPE_DPHY) {
+ dev_err(csi2phy->dev, "mode %d -EOPNOTSUPP\n", args->args[0]);
+ return ERR_PTR(-EOPNOTSUPP);
+ }
+
+ csi2phy->phy_mode = args->args[0];
+
+ return csi2phy->phy;
+}
+
+static int phy_qcom_mipi_csi2_attach_pm_domains(struct mipi_csi2phy_device *csi2phy)
+{
+ const struct dev_pm_domain_attach_data pd_data = {
+ .pd_names = csi2phy->soc_cfg->genpd_names,
+ .num_pd_names = csi2phy->soc_cfg->num_genpd_names,
+ };
+
+ return devm_pm_domain_attach_list(csi2phy->dev, &pd_data, &csi2phy->pd_list);
+}
+
+static int phy_qcom_mipi_csi2_parse_routing(struct mipi_csi2phy_device *csi2phy)
+{
+ struct mipi_csi2phy_stream_cfg *stream_cfg = &csi2phy->stream_cfg;
+ u32 lane_polarities[CSI2_MAX_DATA_LANES + 1];
+ u32 data_lanes[CSI2_MAX_DATA_LANES];
+ struct device *dev = csi2phy->dev;
+ struct fwnode_handle *ep;
+ int num_polarities;
+ int num_data_lanes;
+ u32 clock_lane;
+ int i, ret;
+
+ ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 1, 0,
+ FWNODE_GRAPH_ENDPOINT_NEXT);
+ if (ep) {
+ fwnode_handle_put(ep);
+ dev_err(dev, "DPHY split mode is not supported\n");
+ return -EOPNOTSUPP;
+ }
+
+ ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 0, 0, 0);
+ if (!ep) {
+ dev_err(dev, "Missing port@0\n");
+ return -ENODEV;
+ }
+
+ num_data_lanes = fwnode_property_count_u32(ep, "data-lanes");
+ if (num_data_lanes < 1 || num_data_lanes > CSI2_MAX_DATA_LANES) {
+ ret = -EINVAL;
+ dev_err(dev, "Invalid data-lanes count: %d\n", num_data_lanes);
+ goto out_put;
+ }
+ stream_cfg->num_data_lanes = num_data_lanes;
+
+ ret = fwnode_property_read_u32_array(ep, "data-lanes", data_lanes,
+ stream_cfg->num_data_lanes);
+ if (ret) {
+ dev_err(dev, "Failed to read data-lanes: %d\n", ret);
+ goto out_put;
+ }
+
+ ret = fwnode_property_read_u32(ep, "clock-lanes", &clock_lane);
+ if (ret) {
+ clock_lane = CSI2_DEFAULT_CLK_LN;
+ dev_info(dev, "Using default clock-lane %d\n",
+ CSI2_DEFAULT_CLK_LN);
+ }
+
+ /* lane-polarities: optional, up to num_data_lanes + 1 entries */
+ memset(lane_polarities, 0x00, sizeof(lane_polarities));
+ num_polarities = fwnode_property_count_u32(ep, "lane-polarities");
+ if (num_polarities > 0) {
+ if (num_polarities != stream_cfg->num_data_lanes + 1) {
+ ret = -EINVAL;
+ dev_err(dev, "clock+data-lane %d/polarities %d mismatch\n",
+ stream_cfg->num_data_lanes + 1, num_polarities);
+ goto out_put;
+ }
+
+ ret = fwnode_property_read_u32_array(ep, "lane-polarities", lane_polarities,
+ num_polarities);
+ if (ret) {
+ dev_err(dev, "Failed to read lane-polarities: %d\n", ret);
+ goto out_put;
+ }
+ }
+
+ for (i = 0; i < csi2phy->stream_cfg.num_data_lanes; i++) {
+ csi2phy->stream_cfg.lane_cfg.data[i].pos = data_lanes[i];
+ csi2phy->stream_cfg.lane_cfg.data[i].pol = lane_polarities[i + 1];
+ }
+ csi2phy->stream_cfg.lane_cfg.clk.pos = clock_lane;
+ csi2phy->stream_cfg.lane_cfg.clk.pol = lane_polarities[0];
+
+ ret = 0;
+
+out_put:
+ fwnode_handle_put(ep);
+
+ return ret;
+}
+
+static int phy_qcom_mipi_csi2_probe(struct platform_device *pdev)
+{
+ unsigned int i, num_clk, num_supplies;
+ struct mipi_csi2phy_device *csi2phy;
+ struct phy_provider *phy_provider;
+ struct device *dev = &pdev->dev;
+ struct phy *generic_phy;
+ int ret;
+
+ csi2phy = devm_kzalloc(dev, sizeof(*csi2phy), GFP_KERNEL);
+ if (!csi2phy)
+ return -ENOMEM;
+
+ csi2phy->dev = dev;
+ dev_set_drvdata(dev, csi2phy);
+
+ csi2phy->soc_cfg = device_get_match_data(&pdev->dev);
+
+ if (!csi2phy->soc_cfg)
+ return -EINVAL;
+
+ num_clk = csi2phy->soc_cfg->num_clk;
+ csi2phy->clks = devm_kzalloc(dev, sizeof(*csi2phy->clks) * num_clk, GFP_KERNEL);
+ if (!csi2phy->clks)
+ return -ENOMEM;
+
+ ret = phy_qcom_mipi_csi2_parse_routing(csi2phy);
+ if (ret)
+ return ret;
+
+ ret = phy_qcom_mipi_csi2_attach_pm_domains(csi2phy);
+ if (ret < 0)
+ return dev_err_probe(dev, ret, "Failed to attach power-domain list\n");
+
+ for (i = 0; i < num_clk; i++)
+ csi2phy->clks[i].id = csi2phy->soc_cfg->clk_names[i];
+
+ ret = devm_clk_bulk_get(dev, num_clk, csi2phy->clks);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to get clocks\n");
+
+ csi2phy->timer_clk = devm_clk_get(dev, csi2phy->soc_cfg->timer_clk);
+ if (IS_ERR(csi2phy->timer_clk)) {
+ return dev_err_probe(dev, PTR_ERR(csi2phy->timer_clk),
+ "Failed to get timer clock\n");
+ }
+
+ ret = devm_pm_opp_set_clkname(dev, csi2phy->soc_cfg->opp_clk);
+ if (ret)
+ return dev_err_probe(dev, ret, "Failed to set opp clkname\n");
+
+ ret = devm_pm_opp_of_add_table(dev);
+ if (ret && ret != -ENODEV)
+ return dev_err_probe(dev, ret, "invalid OPP table in device tree\n");
+
+ num_supplies = csi2phy->soc_cfg->num_supplies;
+ csi2phy->supplies = devm_kzalloc(dev, sizeof(*csi2phy->supplies) * num_supplies,
+ GFP_KERNEL);
+ if (!csi2phy->supplies)
+ return -ENOMEM;
+
+ for (i = 0; i < num_supplies; i++)
+ csi2phy->supplies[i].supply = csi2phy->soc_cfg->supply_names[i];
+
+ ret = devm_regulator_bulk_get(dev, num_supplies, csi2phy->supplies);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "failed to get regulator supplies\n");
+
+ csi2phy->base = devm_platform_ioremap_resource(pdev, 0);
+ if (IS_ERR(csi2phy->base))
+ return PTR_ERR(csi2phy->base);
+
+ generic_phy = devm_phy_create(dev, NULL, &phy_qcom_mipi_csi2_ops);
+ if (IS_ERR(generic_phy)) {
+ ret = PTR_ERR(generic_phy);
+ return dev_err_probe(dev, ret, "failed to create phy\n");
+ }
+ csi2phy->phy = generic_phy;
+
+ phy_set_drvdata(generic_phy, csi2phy);
+
+ phy_provider = devm_of_phy_provider_register(dev, qcom_csi2_phy_xlate);
+ if (!IS_ERR(phy_provider))
+ dev_dbg(dev, "Registered MIPI CSI2 PHY device\n");
+
+ return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static const struct of_device_id phy_qcom_mipi_csi2_of_match_table[] = {
+ { .compatible = "qcom,x1e80100-csi2-phy", .data = &mipi_csi2_dphy_4nm_x1e },
+ { }
+};
+MODULE_DEVICE_TABLE(of, phy_qcom_mipi_csi2_of_match_table);
+
+static struct platform_driver phy_qcom_mipi_csi2_driver = {
+ .probe = phy_qcom_mipi_csi2_probe,
+ .driver = {
+ .name = "qcom-mipi-csi2-phy",
+ .of_match_table = phy_qcom_mipi_csi2_of_match_table,
+ },
+};
+
+module_platform_driver(phy_qcom_mipi_csi2_driver);
+
+MODULE_DESCRIPTION("Qualcomm MIPI CSI2 PHY driver");
+MODULE_AUTHOR("Bryan O'Donoghue <bryan.odonoghue@linaro.org>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h b/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h
new file mode 100644
index 0000000000000..e7c1ce00916e3
--- /dev/null
+++ b/drivers/phy/qualcomm/phy-qcom-mipi-csi2.h
@@ -0,0 +1,95 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ *
+ * Qualcomm MIPI CSI2 CPHY/DPHY driver
+ *
+ * Copyright (C) 2025 Linaro Ltd.
+ */
+#ifndef __PHY_QCOM_MIPI_CSI2_H__
+#define __PHY_QCOM_MIPI_CSI2_H__
+
+#include <linux/phy/phy.h>
+
+#define CSI2_MAX_DATA_LANES 4
+#define CSI2_DEFAULT_CLK_LN 7
+
+struct mipi_csi2phy_lane {
+ u8 pos;
+ u8 pol;
+};
+
+struct mipi_csi2phy_lanes_cfg {
+ struct mipi_csi2phy_lane data[CSI2_MAX_DATA_LANES];
+ struct mipi_csi2phy_lane clk;
+};
+
+struct mipi_csi2phy_stream_cfg {
+ s64 link_freq;
+ u8 num_data_lanes;
+ struct mipi_csi2phy_lanes_cfg lane_cfg;
+};
+
+struct mipi_csi2phy_device;
+
+struct mipi_csi2phy_hw_ops {
+ void (*hw_version_read)(struct mipi_csi2phy_device *csi2phy_dev);
+ void (*reset)(struct mipi_csi2phy_device *csi2phy_dev);
+ int (*lanes_enable)(struct mipi_csi2phy_device *csi2phy_dev,
+ struct mipi_csi2phy_stream_cfg *cfg);
+ void (*lanes_disable)(struct mipi_csi2phy_device *csi2phy_dev,
+ struct mipi_csi2phy_stream_cfg *cfg);
+};
+
+struct mipi_csi2phy_lane_regs {
+ const s32 reg_addr;
+ const s32 reg_data;
+ const u32 delay_us;
+ const u32 param_type;
+};
+
+struct mipi_csi2phy_device_regs {
+ const struct mipi_csi2phy_lane_regs *init_seq;
+ const int lane_array_size;
+ const u32 common_regs_offset;
+};
+
+struct mipi_csi2phy_soc_cfg {
+ const struct mipi_csi2phy_hw_ops *ops;
+ const struct mipi_csi2phy_device_regs reg_info;
+
+ const char ** const supply_names;
+ const unsigned int num_supplies;
+
+ const char ** const clk_names;
+ const unsigned int num_clk;
+
+ const char * const opp_clk;
+ const char * const timer_clk;
+
+ const char ** const genpd_names;
+ const unsigned int num_genpd_names;
+};
+
+struct mipi_csi2phy_device {
+ struct device *dev;
+ u8 phy_mode;
+
+ struct phy *phy;
+ void __iomem *base;
+
+ struct clk_bulk_data *clks;
+ struct clk *timer_clk;
+ u32 timer_clk_rate;
+
+ struct regulator_bulk_data *supplies;
+ struct dev_pm_domain_list *pd_list;
+
+ const struct mipi_csi2phy_soc_cfg *soc_cfg;
+ struct mipi_csi2phy_stream_cfg stream_cfg;
+
+ u32 hw_version;
+};
+
+extern const struct mipi_csi2phy_soc_cfg mipi_csi2_dphy_4nm_x1e;
+
+#endif /* __PHY_QCOM_MIPI_CSI2_H__ */
--
2.54.0
^ permalink raw reply related
* [PATCH v7 1/2] dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
From: Bryan O'Donoghue @ 2026-05-22 14:45 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: Bryan O'Donoghue, Vladimir Zapolskiy, linux-arm-msm,
linux-phy, linux-media, devicetree, linux-kernel,
Bryan O'Donoghue
In-Reply-To: <20260522-x1e-csi2-phy-v7-0-79cb1280fad6@linaro.org>
Add a base schema initially compatible with x1e80100 to describe MIPI CSI2
PHY devices.
The hardware can support both CPHY, DPHY and a special split-mode DPHY.
The schema here defines three ports:
port@0:
The first input port where a sensor is always required.
port@1:
A second optional input port which if present implies DPHY split-mode.
port@2:
A third always required output port which connects to the controller.
The CSIPHY devices have their own pinouts on the SoC as well as their own
individual voltage rails.
The need to model voltage rails on a per-PHY basis leads us to define
CSIPHY devices as individual nodes.
Two nice outcomes in terms of schema and DT arise from this change.
1. The ability to define on a per-PHY basis voltage rails.
2. The ability to require those voltage.
We have had a complete bodge upstream for this where a single set of
voltage rail for all CSIPHYs has been buried inside of CAMSS.
Much like the I2C bus which is dedicated to Camera sensors - the CCI bus in
CAMSS parlance, the CSIPHY devices should be individually modelled.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
.../bindings/phy/qcom,x1e80100-csi2-phy.yaml | 208 +++++++++++++++++++++
1 file changed, 208 insertions(+)
diff --git a/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
new file mode 100644
index 0000000000000..f434fb04447f6
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom,x1e80100-csi2-phy.yaml
@@ -0,0 +1,208 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/qcom,x1e80100-csi2-phy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm CSI2 PHY
+
+maintainers:
+ - Bryan O'Donoghue <bod@kernel.org>
+
+description:
+ Qualcomm MIPI CSI2 C-PHY/D-PHY combination PHY. Connects MIPI CSI2 sensors
+ to Qualcomm's Camera CSI Decoder. The PHY supports both C-PHY and D-PHY
+ modes.
+
+properties:
+ compatible:
+ const: qcom,x1e80100-csi2-phy
+
+ reg:
+ maxItems: 1
+
+ "#phy-cells":
+ const: 1
+ description:
+ The single cell specifies the PHY operating mode.
+
+ clocks:
+ maxItems: 2
+
+ clock-names:
+ items:
+ - const: core
+ - const: timer
+
+ interrupts:
+ maxItems: 1
+
+ operating-points-v2:
+ maxItems: 1
+
+ power-domains:
+ items:
+ - description: MMCX voltage rail
+ - description: MXC or MXA voltage rail
+
+ power-domain-names:
+ items:
+ - const: mmcx
+ - const: mx
+
+ vdda-0p9-supply:
+ description: Phandle to a 0.9V regulator supply to a PHY.
+
+ vdda-1p2-supply:
+ description: Phandle to 1.2V regulator supply to a PHY.
+
+ ports:
+ $ref: /schemas/graph.yaml#/properties/ports
+
+ properties:
+ port@0:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ description: Sensor input. Always present.
+
+ properties:
+ endpoint:
+ $ref: /schemas/media/video-interfaces.yaml#
+ unevaluatedProperties: false
+ properties:
+ data-lanes:
+ minItems: 1
+ maxItems: 4
+ clock-lanes:
+ maxItems: 1
+ remote-endpoint: true
+ required:
+ - data-lanes
+ - remote-endpoint
+
+ port@1:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ description:
+ Second sensor input. When present, indicates DPHY split mode.
+
+ properties:
+ endpoint:
+ $ref: /schemas/media/video-interfaces.yaml#
+ unevaluatedProperties: false
+ properties:
+ data-lanes:
+ maxItems: 1
+ clock-lanes:
+ maxItems: 1
+ remote-endpoint: true
+ required:
+ - data-lanes
+ - clock-lanes
+ - remote-endpoint
+
+ port@2:
+ $ref: /schemas/graph.yaml#/$defs/port-base
+ description: Output to CAMSS controller.
+
+ properties:
+ endpoint:
+ $ref: /schemas/graph.yaml#/$defs/endpoint-base
+ unevaluatedProperties: false
+ properties:
+ remote-endpoint: true
+ required:
+ - remote-endpoint
+
+ required:
+ - port@0
+ - port@2
+
+required:
+ - compatible
+ - reg
+ - "#phy-cells"
+ - clocks
+ - clock-names
+ - interrupts
+ - operating-points-v2
+ - power-domains
+ - power-domain-names
+ - vdda-0p9-supply
+ - vdda-1p2-supply
+ - ports
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/clock/qcom,x1e80100-camcc.h>
+ #include <dt-bindings/clock/qcom,x1e80100-gcc.h>
+ #include <dt-bindings/power/qcom,rpmhpd.h>
+
+ csiphy4: csiphy@ace4000 {
+ compatible = "qcom,x1e80100-csi2-phy";
+ reg = <0x0ace4000 0x2000>;
+ #phy-cells = <1>;
+
+ clocks = <&camcc CAM_CC_CSIPHY0_CLK>,
+ <&camcc CAM_CC_CSI0PHYTIMER_CLK>;
+ clock-names = "core",
+ "timer";
+
+ operating-points-v2 = <&csiphy_opp_table>;
+
+ interrupts = <GIC_SPI 477 IRQ_TYPE_EDGE_RISING>;
+
+ power-domains = <&rpmhpd RPMHPD_MMCX>,
+ <&rpmhpd RPMHPD_MX>;
+ power-domain-names = "mmcx",
+ "mx";
+
+ vdda-0p9-supply = <&vreg_l2c_0p8>;
+ vdda-1p2-supply = <&vreg_l1c_1p2>;
+
+ ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ csiphy0_in_ep: endpoint {
+ data-lanes = <0 1>;
+ clock-lanes = <2>;
+ remote-endpoint = <&sensor_out>;
+ };
+ };
+
+ port@2 {
+ reg = <2>;
+ csiphy0_out_ep: endpoint {
+ data-lanes = <3>;
+ clock-lanes = <4>;
+ remote-endpoint = <&controller_in>;
+ };
+ };
+ };
+ };
+
+ csiphy_opp_table: opp-table {
+ compatible = "operating-points-v2";
+
+ opp-300000000 {
+ opp-hz = /bits/ 64 <300000000>;
+ required-opps = <&rpmhpd_opp_low_svs_d1>,
+ <&rpmhpd_opp_low_svs_d1>;
+ };
+
+ opp-400000000 {
+ opp-hz = /bits/ 64 <400000000>;
+ required-opps = <&rpmhpd_opp_low_svs>,
+ <&rpmhpd_opp_low_svs_d1>;
+ };
+
+ opp-480000000 {
+ opp-hz = /bits/ 64 <480000000>;
+ required-opps = <&rpmhpd_opp_low_svs>,
+ <&rpmhpd_opp_low_svs_d1>;
+ };
+ };
--
2.54.0
^ permalink raw reply related
* [PATCH v7 0/2] phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
From: Bryan O'Donoghue @ 2026-05-22 14:45 UTC (permalink / raw)
To: Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong
Cc: Bryan O'Donoghue, Vladimir Zapolskiy, linux-arm-msm,
linux-phy, linux-media, devicetree, linux-kernel,
Bryan O'Donoghue
Changes in v7:
- Made CONFIG_PM a dependency. Sashiko commented on the pd_list being NULL
and suggested I check the pointer but, we need to ramp the rails when
switching clock rate so we need CONFIG_PM full stop. - Sashiko.ai, Bryan
- Added unwind operation for performance state error path - Sashiko
- Made clock-lanes genuinely optional for the supported use-case. - Sashiko
- Fixed the enable of lanes. Thus far we have had forever it seems
val |= BIT(lane.pos * 2) which I've never looked at much because it
has always worked. But looking at how to switch on polarities I realised
the relevant register is a linear bitmask without gaps so the correct
method is `val |= BIT(lane.pos)`.
This needs an update in the legacy PHY too in another series - Bryan
- I opted not to do any of the "but if DT send junk into your driver" fixes
from Sashiko since TBH I think the code would be Spaghetti afterwards.
We trust DT and if DT is wrong we fix it, we don't try to graph its
relative (in)sanity.
- Fixed my example in the yaml. Sashiko
- Link to v6: https://patch.msgid.link/20260521-x1e-csi2-phy-v6-0-9d73d9bd7d20@linaro.org
Changes in v6:
- Taking feedback from lively debate added ports and
endpoints to the PHY - Neil, Vlad
- Detection of split mode by way of which ports are declared.
port@0 is always a sensor input.
port@1 is optional and if present implies split-mode
port@2 is always the output. - Dmitry, Neil, Vlad.
- Split mode is left as -ENOTSUPP unless/until someone with the appropriate
hardware can take on responsibility to drive to completion.
- Extending phy_config_opts dropped.
I think this is a worthwhile extension but this series no longer depends
on it so dropped. - Bryan
- MX/MXC.
Two OPP tables one for CSIPHY0/1/2 scaling MXC one for CSIPHY4 keeping
MXA at LOWSVS_D1 - to be implemented in DT not here. Taniya, Konrad, Bryan
- Changed MAINTAINERS from Supported to Maintained.
Hobby time for me right now. - Bryan
- Link to v5: https://lore.kernel.org/r/20260326-x1e-csi2-phy-v5-0-0c0fc7f5c01b@linaro.org
v5:
- Adds support to apply passed parameters for clock/data position/polarity - Neil
- Drops GEN1/GEN2 differentiation this can be reconstituted if GEN1 ever
gets supported in this driver - Dmitry
- Drops camnoc_axi, cpas_ahb - Konrad
- Renames csiphy->core csiphy_timer->timer - Konrad
- Renames rail from 0p8 to 0p9 schematics say VDD_A_CSI_n_0P9 - Konrad
- TITAN_TOP_GDSC dropped - Konrad
- Passes PHY_QCOM_CSI2_MODE_{DPHY|CPHY|SPLIT_DPHY} with the controller
selecting the mode. Only DPHY mode is supported but the method to pass
CPHY or split-mode DPHY configuration is there.
Since split-mode is a Qualcomm specific mode the PHY modes are defined in
our binding instead of adding a new type to include/linux/phy/phy.h - bod
- Depends-on: https://lore.kernel.org/r/20260325-dphy-params-extension-v1-0-c6df5599284a@linaro.org
- Link to v4: https://lore.kernel.org/r/20260315-x1e-csi2-phy-v4-0-90c09203888d@linaro.org
v4:
- MMCX, MCX and MX/MXA power-domains added - Dmitry, Vijay, Konrad
- power-domain-names added as required - bod
- opp-tables amended to capture RPMHPD deps - Dmitry, Vijay
- Switched to dev_pm_opp_set_rate, dev_pm_domain_attach_by_name etc
dropped inherited CAMSS code - Dmitry
- Amended parameters structure to specify power-domain name list - bod
- Removed dead defines - Dmitry
- Noted in CSIPHY commit log intention to rework patterns of
PHY lane configs into loops/defines/bit-fields later - Dmitry, bod
- Lowercase hex throughout - Dmitry
- The yaml and code in this driver doesn't care if the node is a
sibling or a sub-node of CAMSS confirmed to work both ways - Dmitry, bod
- Link to v3: https://lore.kernel.org/r/20260226-x1e-csi2-phy-v3-0-11e608759410@linaro.org
v3:
- Resending this to make clear this submission is additive to x1e/Hamoa
The existing bindings and code will continue to work
Bindings are added only, nothing is subtracted from existing ABI.
- Link to v2: https://lore.kernel.org/r/20260225-x1e-csi2-phy-v2-0-7756edb67ea9@linaro.org
v2:
In this updated version
- Added operating-point support
The csiphy clock sets the OPP prior to setting the rate
for csiphy and csiphy_timer - Konrad
- Combo mode
Combo mode in CAMSS yaml has been added. Right now
no code has been changed in the PHY driver to support it as
I don't have hardware to test. In principle though it can
be supported. - Vladimir
- CSIPHY init sequences
I left these as their "magic number formats". With my diminished
status as a non-qcom VPN person - I can no longer see what the bits
map to. Moreover this is the situation any non-VPN community member
will be in when submitting CSIPHY sequences derived from downstream.
I think it is perfectly reasonable to take public CSIPHY init sequences
as magic numbers. If someone with bit-level access wants to enumerate
the bits that's fine but, it shouldn't gate in the interim. - Konrad/bod
- Sensor endpoints
I've stuck to the format used by every other CSIPHY in upstream.
Sensor endpoints hit the CAMSS/CSID endpoint not a endpoint in the PHY.
Given the proposed changes to CAMSS though to support "combo mode" I
think this should achieve the same outcome - multiple sensors on the one
PHY without introducing endpoints into the PHY that no other CSIPHY in
upstream currently has.
- Bitmask of enabled lanes
Work needs to be done in the v4l2 layer to really support this.
I propose making a separate series dedicated to non-linear bit
interpretation after merging this so as to contain the scope of the
series to something more bite (byte haha) sized. - Konrad/bod
- Link to v1: https://lore.kernel.org/r/20250710-x1e-csi2-phy-v1-0-74acbb5b162b@linaro.org
v1:
This short series adds a CSI2 MIPI PHY driver, initially supporting D-PHY
mode. The core logic and init sequences come directly from CAMSS and are
working on at least five separate x1e devices.
The rationale to instantiate CSI2 PHYs as standalone devices instead of as
sub-nodes of CAMSS is as follows.
1. Precedence
CAMSS has a dedicated I2C bus called CCI Camera Control Interface.
We model this controller as its own separate device in devicetree.
This makes sense and CCI/I2C is a well defined bus type already modelled
in Linux.
MIPI CSI2 PHY devices similarly fit into a well defined separate
bus/device structure.
Contrast to another CAMSS component such as VFE, CSID or TPG these
components only interact with other CAMSS inputs/outputs unlike CSIPHY
which interacts with non-SoC components.
2. Hardware pinouts and rails
The CSI2 PHY has its own data/clock lanes out from the SoC and indeed
has its own incoming power-rails.
3. Other devicetree schemas
There are several examples throughout the kernel of CSI PHYs modeled as
standalone devices which one assumes follows the same reasoning as given
above.
I've been working on this on-and-off since the end of April:
Link: https://lore.kernel.org/linux-media/c5cf0155-f839-4db9-b865-d39b56bb1e0a@linaro.org
There is another proposal to have the PHYs be subdevices of CAMSS but, I
believe we should go with a "full fat" PHY to match best practices in
drivers/phy/qualcomm/*.
Using the standard PHY API and the parameter passing that goes with it
allows us to move away from custom interfaces in CAMSS and to conform more
clearly to established PHY paradigms such as the QMP combo PHY.
Looking at existing compat strings I settled on
"qcom,x1e80100-mipi-csi2-combo-phy" deliberately omitting reference to the
fact the PHY is built on a four nano-meter process node, which seems to
match recent submissions to QMP PHY.
My first pass at this driver included support for the old two phase
devices:
Link: https://git.codelinaro.org/bryan.odonoghue/kernel/-/commit/a504c28d109296c93470340cfe7281231f573bcb#b6e59ed7db94c9da22e492bb03fcda6a4300983c
I realised that the device tree schema changes required to support a
comprehensive conversion of all CAMSS to this driver would be an
almost certainly be unacceptable ABI break or at the very least an enormous
amount of work and verification so I instead aimed to support just one new
SoC in the submission.
I've retained the callback indirections give us scope to add in another type of
future PHY including potentially adding in the 2PH later on.
This driver is tested and working on x1e/Hamoa and has been tested as not
breaking sc8280xp/Makena and sm8250/Kona.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
---
Bryan O'Donoghue (2):
dt-bindings: phy: qcom: Add CSI2 C-PHY/DPHY schema
phy: qcom-mipi-csi2: Add a CSI2 MIPI DPHY driver
.../bindings/phy/qcom,x1e80100-csi2-phy.yaml | 208 +++++++++++
MAINTAINERS | 10 +
drivers/phy/qualcomm/Kconfig | 14 +
drivers/phy/qualcomm/Makefile | 5 +
drivers/phy/qualcomm/phy-qcom-mipi-csi2-3ph-dphy.c | 371 ++++++++++++++++++++
drivers/phy/qualcomm/phy-qcom-mipi-csi2-core.c | 386 +++++++++++++++++++++
drivers/phy/qualcomm/phy-qcom-mipi-csi2.h | 95 +++++
7 files changed, 1089 insertions(+)
---
base-commit: a1db83cc6f7e88a166c77d9060507ec01d617784
change-id: 20250710-x1e-csi2-phy-f6434b651d3a
Best regards,
--
Bryan O'Donoghue <bryan.odonoghue@linaro.org>
^ permalink raw reply
* Re: [PATCH v2 2/2] iio: light: veml3328: add support for new device
From: Jonathan Cameron @ 2026-05-22 14:37 UTC (permalink / raw)
To: Joshua Crofts
Cc: David Lechner, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Javier Carrasco, Rishi Gupta,
linux-iio, devicetree, linux-kernel
In-Reply-To: <20260520-veml3328-v2-2-a57f0d9e28d4@gmail.com>
On Wed, 20 May 2026 23:15:38 +0200
Joshua Crofts <joshua.crofts1@gmail.com> wrote:
> Add support for the Vishay VEML3328 RGB/IR light sensor communicating
> via I2C (SMBus compatible).
>
> Also add a new entry for said driver into Kconfig and Makefile.
>
> Assisted-by: Gemini:3.1-Pro
> Signed-off-by: Joshua Crofts <joshua.crofts1@gmail.com>
Hi Joshua,
Sashiko did a really good job on this. I'm always amazed at how much it
understands of how these attributes are used.
https://sashiko.dev/#/patchset/20260520-veml3328-v2-0-a57f0d9e28d4%40gmail.com
My biggest comment is I'd look at making the scale from integration / gain
calculations something that is precomputed that you just look up.
You can have separate ones for light and intensity channels or be a little
cheeky and just use the light one for everything - given intensity doesn't
have fixed units as such we can scale by the extra factors in the illuminance
calculation and not worry about what they are.
> diff --git a/drivers/iio/light/veml3328.c b/drivers/iio/light/veml3328.c
> new file mode 100644
> index 000000000000..fdfa8fd1666b
> --- /dev/null
> +++ b/drivers/iio/light/veml3328.c
> @@ -0,0 +1,416 @@
> +
> +#define VEML3328_CHAN_SPEC(_color, _addr) { \
> + .type = IIO_INTENSITY, \
> + .modified = 1, \
> + .channel2 = IIO_MOD_LIGHT_##_color, \
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
> + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME) | \
> + BIT(IIO_CHAN_INFO_SCALE), \
> + .info_mask_shared_by_all_available = BIT(IIO_CHAN_INFO_INT_TIME) | \
> + BIT(IIO_CHAN_INFO_SCALE), \
> + .address = _addr, \
> +}
> +
> +static const struct iio_chan_spec veml3328_channels[] = {
> + {
> + .type = IIO_LIGHT,
> +
> + .info_mask_separate = BIT(IIO_CHAN_INFO_RAW) |
> + BIT(IIO_CHAN_INFO_SCALE),
> + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_INT_TIME),
Sashiko is kind of right and kind of wrong on this.
As it is shared by all, BIT(IIO_CHAN_INFO_SCALE)
should be here. It won't have a function affect as we don't verify
they all have it and as long as one does the file will get created.
So this is more about consistency than correctness.
Raises more questions though. Is it correct scale? I think not.
Looks like you have scaling for the intensity channels returning
different values from the light one.
So I think the above INTENSITY should move IIO_CHAN_INFO_SCALE
to .info_mask_shared_by_type and equivalent for the available.
> + .address = VEML3328_REG_DATA_G,
> + },
> + VEML3328_CHAN_SPEC(CLEAR, VEML3328_REG_DATA_C),
> + VEML3328_CHAN_SPEC(RED, VEML3328_REG_DATA_R),
> + VEML3328_CHAN_SPEC(GREEN, VEML3328_REG_DATA_G),
> + VEML3328_CHAN_SPEC(BLUE, VEML3328_REG_DATA_B),
> + VEML3328_CHAN_SPEC(IR, VEML3328_REG_DATA_IR),
> +};
> +
> +/* scale values per datasheet */
> +static const int veml3328_scale_vals[][2] = {
Sashiko wins again. Whilst we don't get a nice standard unit for
IIO_INTENSITY channels for _raw * _scale, the expectation is that
for the same input light intensity this should remain constant.
I.e. If _scale value double _raw should halve.
I assume these are x0.5 x1 x2 and x4 gains applied. Those will
result in _raw changing by those factors, therefore to get a stable
output these should be inverted.
{ 2, 0 },
{ 1, 0 },
{ 0, 500000 },
{ 0, 250000 },
Make sure you test _raw * _scale is stable as a result.
> + { 0, 500000 },
> + { 1, 0 },
> + { 2, 0 },
> + { 4, 0 },
> +};
> +
> +/* integration times in microseconds */
> +static const int veml3328_it_times[][2] = {
> + { 0, 50 * USEC_PER_MSEC },
> + { 0, 100 * USEC_PER_MSEC },
> + { 0, 200 * USEC_PER_MSEC },
> + { 0, 400 * USEC_PER_MSEC },
> +};
> +
> +
> +static int veml3328_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int *val, int *val2, long mask)
> +{
> + struct veml3328_data *data = iio_priv(indio_dev);
> + struct regmap *regmap = data->regmap;
> + struct device *dev = regmap_get_device(regmap);
> + unsigned int reg_val;
> + int ret;
> + int reg;
> +
> + PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND(dev, pm);
> + ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
> + if (ret)
> + return ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW:
> + ret = regmap_read(regmap, chan->address, ®_val);
> + if (ret)
> + return ret;
> +
> + *val = reg_val;
> + return IIO_VAL_INT;
> +
> + case IIO_CHAN_INFO_INT_TIME:
> + ret = regmap_read(regmap, VEML3328_REG_CONF, ®_val);
> + if (ret)
> + return ret;
> +
> + reg = FIELD_GET(VEML3328_CONF_IT_MASK, reg_val);
> + if (reg >= ARRAY_SIZE(veml3328_it_times))
> + return -EINVAL;
> +
> + *val = veml3328_it_times[reg][0];
> + *val2 = veml3328_it_times[reg][1];
> + return IIO_VAL_INT_PLUS_MICRO;
> +
> + case IIO_CHAN_INFO_SCALE:
> + ret = regmap_read(regmap, VEML3328_REG_CONF, ®_val);
> + if (ret)
> + return ret;
> +
> + if (chan->type == IIO_LIGHT) {
> + int it_inx, gain_inx;
> + int it_us, scale_u;
> +
> + it_inx = FIELD_GET(VEML3328_CONF_IT_MASK, reg_val);
> + gain_inx = FIELD_GET(VEML3328_CONF_GAIN_MASK, reg_val);
> +
> + if (it_inx >= ARRAY_SIZE(veml3328_it_times) ||
> + gain_inx >= ARRAY_SIZE(veml3328_scale_vals))
> + return -EINVAL;
> +
> + it_us = veml3328_it_times[it_inx][1];
> +
> + /*
> + * Equation for calculating ambient light scale:
> + * Scale = 0.384 * (50000 / Current_IT) * (1x / Current_Gain)
Good. This has the inverses missing for the intensity channels.
As I note below, there are quite a few accesses to what is basically a 4x4 array
of values based on selection of gain and selection of integration time.
I'd be tempted to precompute them and make all these paths look ups.
> + */
> + scale_u = 384 * USEC_PER_MSEC;
> + scale_u = scale_u * 50000 / it_us;
> +
> + switch (gain_inx) {
> + case 0:
> + scale_u *= 2;
> + break;
> + case 1:
> + break;
> + case 2:
> + scale_u /= 2;
> + break;
> + case 3:
> + scale_u /= 4;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + *val = 0;
> + *val2 = scale_u;
> +
> + return IIO_VAL_INT_PLUS_MICRO;
> + }
> +
> + reg = FIELD_GET(VEML3328_CONF_GAIN_MASK, reg_val);
> + if (reg >= ARRAY_SIZE(veml3328_scale_vals))
> + return -EINVAL;
> +
> + *val = veml3328_scale_vals[reg][0];
> + *val2 = veml3328_scale_vals[reg][1];
> + return IIO_VAL_INT_PLUS_MICRO;
I would flip the logic and do this one out of line. It's shorter!
Mind you it should still be taking integration time into account to
achieve a stable value whatever the integration time (assuming we don't
hit limits).
> +
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int veml3328_read_avail(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + const int **vals, int *type, int *length,
> + long mask)
> +{
> + switch (mask) {
> + case IIO_CHAN_INFO_INT_TIME:
> + *length = ARRAY_SIZE(veml3328_it_times) * 2;
> + *vals = (const int *)veml3328_it_times;
> + *type = IIO_VAL_INT_PLUS_MICRO;
> + return IIO_AVAIL_LIST;
> +
> + case IIO_CHAN_INFO_SCALE:
As mentioned else where this needs to work for both light and intensity
channels and incorporate the affect of integration time.
> + *length = ARRAY_SIZE(veml3328_scale_vals) * 2;
> + *vals = (const int *)veml3328_scale_vals;
> + *type = IIO_VAL_INT_PLUS_MICRO;
> + return IIO_AVAIL_LIST;
> +
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +static int veml3328_write_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + int val, int val2, long mask)
> +{
> + struct veml3328_data *data = iio_priv(indio_dev);
> + struct regmap *regmap = data->regmap;
> + struct device *dev = regmap_get_device(regmap);
> + int ret;
> + int i;
> +
> + PM_RUNTIME_ACQUIRE_IF_ENABLED_AUTOSUSPEND(dev, pm);
> + ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
> + if (ret)
> + return ret;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_INT_TIME:
> + if (val != 0)
> + return -EINVAL;
> +
> + for (i = 0; i < ARRAY_SIZE(veml3328_it_times); i++) {
> + if (veml3328_it_times[i][1] == val2)
> + break;
> + }
> +
> + if (i == ARRAY_SIZE(veml3328_it_times))
> + return -EINVAL;
> +
> + return regmap_update_bits(regmap, VEML3328_REG_CONF,
> + VEML3328_CONF_IT_MASK,
> + FIELD_PREP(VEML3328_CONF_IT_MASK, i));
> +
> + case IIO_CHAN_INFO_SCALE:
So this is always a bit of a pain to handle, but you need to take the
integration time into account.
Also this currently applies to the IIO_LIGHT channel as well so you need
to handle that as well which means reversing the maths above to find the
right gain for the current integration time. There are only 16 options so
I'd be tempted to precompute them all.
> + for (i = 0; i < ARRAY_SIZE(veml3328_scale_vals); i++) {
> + if (val == veml3328_scale_vals[i][0] &&
> + val2 == veml3328_scale_vals[i][1])
> + break;
> + }
> +
> + if (i == ARRAY_SIZE(veml3328_scale_vals))
> + return -EINVAL;
> +
> + return regmap_update_bits(regmap, VEML3328_REG_CONF,
> + VEML3328_CONF_GAIN_MASK,
> + FIELD_PREP(VEML3328_CONF_GAIN_MASK, i));
> +
> + default:
> + return -EINVAL;
> + }
> +}
>
^ permalink raw reply
* Re: [PATCH v2 2/2] remoteproc: xlnx: enable auto boot feature
From: Mathieu Poirier @ 2026-05-22 14:35 UTC (permalink / raw)
To: tanmay.shah
Cc: andersson, robh, krzk+dt, conor+dt, michal.simek, ben.levinsky,
linux-remoteproc, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <be63e9a0-e325-46eb-9c03-54dc22878ed6@amd.com>
On Thu, 21 May 2026 at 12:48, Shah, Tanmay <tanmays@amd.com> wrote:
>
>
>
> On 5/21/2026 1:38 PM, Shah, Tanmay wrote:
> > Hello,
> >
> > Thank you for the reviews, please find my comments below:
> >
> > On 5/21/2026 12:48 PM, Mathieu Poirier wrote:
> >> Good morning,
> >>
> >> I don't recal reviewing the first revision of this set. Can you provide a link
> >> to it so that I can read the comments that were provided?
> >>
> >
> > Here it is:
> > https://lore.kernel.org/linux-remoteproc/20260422202558.2362971-1-tanmay.shah@amd.com/
> >
> > The device-tree bindings needed rework in v1, so I sent v2, before we
> > ever reviewed the driver part.
> >
> >
> >> On Fri, May 01, 2026 at 07:37:07AM -0700, Tanmay Shah wrote:
> >>> remoteproc framework has capability to start (or attach to) the remote
> >>
> >> The remoteproc framework...
> >>
> >
> > Ack.
> >
> >>> processor automatically if auto boot flag is set by the driver during
> >>> probe. If remote core is not started before the Linux boot, and linux is
> >>> expected to start the remote core then it uses "firmware-name" property
> >>> to load default firmware during auto boot.
> >>>
> >>> Signed-off-by: Tanmay Shah <tanmay.shah@amd.com>
> >>> ---
> >>> drivers/remoteproc/xlnx_r5_remoteproc.c | 48 +++++++++++++++++--------
> >>> 1 file changed, 34 insertions(+), 14 deletions(-)
> >>>
> >>> diff --git a/drivers/remoteproc/xlnx_r5_remoteproc.c b/drivers/remoteproc/xlnx_r5_remoteproc.c
> >>> index 45a62cb98072..652030f9cea2 100644
> >>> --- a/drivers/remoteproc/xlnx_r5_remoteproc.c
> >>> +++ b/drivers/remoteproc/xlnx_r5_remoteproc.c
> >>> @@ -899,17 +899,18 @@ static const struct rproc_ops zynqmp_r5_rproc_ops = {
> >>> };
> >>>
> >>> /**
> >>> - * zynqmp_r5_add_rproc_core() - Add core data to framework.
> >>> - * Allocate and add struct rproc object for each r5f core
> >>> + * zynqmp_r5_alloc_rproc_core() - alloc rproc core data structure
> >>> + * Allocate struct rproc object for each r5f core
> >>> * This is called for each individual r5f core
> >>> *
> >>> * @cdev: Device node of each r5 core
> >>> *
> >>> * Return: zynqmp_r5_core object for success else error code pointer
> >>> */
> >>> -static struct zynqmp_r5_core *zynqmp_r5_add_rproc_core(struct device *cdev)
> >>> +static struct zynqmp_r5_core *zynqmp_r5_alloc_rproc_core(struct device *cdev)
> >>
> >> Why is there a need to change the function's name?
> >>
> >
> > Before, the function was actually adding the rproc core by calling
> > rproc_add() function, but now it only allocates the memory by calling
> > rproc_alloc(). For auto boot to work it's important to add rproc core
> > after all the other hw is initialized (such as mbox, tcm, sram,
> > power-domains etc). More details below [1].
> >
> >>> {
> >>> struct zynqmp_r5_core *r5_core;
> >>> + const char *fw_name = NULL;
> >>> struct rproc *r5_rproc;
> >>> int ret;
> >>>
> >>> @@ -918,10 +919,15 @@ static struct zynqmp_r5_core *zynqmp_r5_add_rproc_core(struct device *cdev)
> >>> if (ret)
> >>> return ERR_PTR(ret);
> >>>
> >>> + ret = rproc_of_parse_firmware(cdev, 0, &fw_name);
> >>> + if (ret < 0 && ret != -EINVAL)
> >>> + return ERR_PTR(dev_err_probe(cdev, ret,
> >>> + "failed to parse firmware-name\n"));
> >>> +
> >>> /* Allocate remoteproc instance */
> >>> r5_rproc = rproc_alloc(cdev, dev_name(cdev),
> >>> &zynqmp_r5_rproc_ops,
> >>> - NULL, sizeof(struct zynqmp_r5_core));
> >>> + fw_name, sizeof(struct zynqmp_r5_core));
> >>> if (!r5_rproc) {
> >>> dev_err(cdev, "failed to allocate memory for rproc instance\n");
> >>> return ERR_PTR(-ENOMEM);
> >>> @@ -932,6 +938,11 @@ static struct zynqmp_r5_core *zynqmp_r5_add_rproc_core(struct device *cdev)
> >>> r5_rproc->recovery_disabled = true;
> >>> r5_rproc->has_iommu = false;
> >>> r5_rproc->auto_boot = false;
> >>> +
> >>> + /* attempt to boot automatically if the firmware-name is provided */
> >>> + if (fw_name)
> >>> + r5_rproc->auto_boot = true;
> >>> +
> >>
> >> What happens when a firmware name needs to be provided in the DT but you don't
> >> want to automatically boot the remote processor?
> >>
> >
> > I think that use case is not needed. If the user/system-designer doesn't
> > want auto-boot, then having firmware-name in the device-tree serves no
> > purpose. User can always load the firmware via sysfs once kernel boots.
> >
> >>> r5_core = r5_rproc->priv;
> >>> r5_core->dev = cdev;
> >>> r5_core->np = dev_of_node(cdev);
> >>> @@ -941,13 +952,6 @@ static struct zynqmp_r5_core *zynqmp_r5_add_rproc_core(struct device *cdev)
> >>> goto free_rproc;
> >>> }
> >>>
> >>> - /* Add R5 remoteproc core */
> >>> - ret = rproc_add(r5_rproc);
> >>> - if (ret) {
> >>> - dev_err(cdev, "failed to add r5 remoteproc\n");
> >>> - goto free_rproc;
> >>> - }
> >>> -
> >>
> >> I'm not sure why there is a need to move this to zynqmp_r5_cluster_init()? Is
> >> it simply to make the error path easier to handle? If so, please do that in a
> >> separate patch.
> >>
> >
> > [1] This was moved to make auto-boot work. The remote core can auto-boot
> > only after other hardware is initialized. The zynqmp_r5_core_init()
> > initializes sram, TCM and power-domains of the core. Also, mailbox is
> > requested before zynqmp_r5_core_init() as well. We can't auto-boot core
> > directly without all this. So, I had to move rproc_add() at the end of
> > the cluster init, and rename above function from
> > zynqmp_r5_add_rproc_core to zynqmp_r5_alloc_rproc_core.
> >
> > If you prefer, I will add above explanation in the commit text, or as
> > comment right before rproc_add().
> >
> >
> >
> >>> r5_core->rproc = r5_rproc;
> >>> return r5_core;
> >>>
> >>> @@ -1280,6 +1284,7 @@ static int zynqmp_r5_core_init(struct zynqmp_r5_cluster *cluster,
> >>> if (zynqmp_r5_get_rsc_table_va(r5_core))
> >>> dev_dbg(r5_core->dev, "rsc tbl not found\n");
> >>> r5_core->rproc->state = RPROC_DETACHED;
> >>> + r5_core->rproc->auto_boot = true;
> >>
> >> I thought this was done in zynqmp_r5_add_rproc_core() - what am I missing?
> >>
> >
> > That function is now zynqmp_r5_alloc_core() as mentioned above. Also,
> > until now, auto_boot was set to 'false' only to show that it is
> > disabled. It is actually used and enabled now.
> >
>
> "I thought this was done in zynqmp_r5_add_rproc_core() - what am I missing?"
>
> I probably misunderstood this comment. Here is the correct explanation:
>
> The auto_boot setting in the zynqmp_r5_alloc_core() is done if the
> 'firmware-name' property is present in the device-tree.
>
> Here it is done, if the remote core is already running. This is to
> support attach-detach use case.
>
> So, auto_boot is possible in two cases: 1) If firmware-name property is
> available (Linux boots the remote), 2) If firmware is already loaded and
> remote was started by the boot loader. (Linux attaches to the running
> remote).
>
> This is the second use case.
>
Thanks for the clarifications, I'll have another look at this set.
> Thanks,
> Tanmay
>
> >> Thanks,
> >> Mathieu
> >>
> >>> }
> >>> }
> >>>
> >>> @@ -1304,7 +1309,7 @@ static int zynqmp_r5_cluster_init(struct zynqmp_r5_cluster *cluster)
> >>> enum rpu_oper_mode fw_reg_val;
> >>> struct device **child_devs;
> >>> enum rpu_tcm_comb tcm_mode;
> >>> - int core_count, ret, i;
> >>> + int core_count, ret, i, j;
> >>> struct mbox_info *ipi;
> >>>
> >>> ret = of_property_read_u32(dev_node, "xlnx,cluster-mode", &cluster_mode);
> >>> @@ -1390,7 +1395,7 @@ static int zynqmp_r5_cluster_init(struct zynqmp_r5_cluster *cluster)
> >>> child_devs[i] = &child_pdev->dev;
> >>>
> >>> /* create and add remoteproc instance of type struct rproc */
> >>> - r5_cores[i] = zynqmp_r5_add_rproc_core(&child_pdev->dev);
> >>> + r5_cores[i] = zynqmp_r5_alloc_rproc_core(&child_pdev->dev);
> >>> if (IS_ERR(r5_cores[i])) {
> >>> ret = PTR_ERR(r5_cores[i]);
> >>> r5_cores[i] = NULL;
> >>> @@ -1435,16 +1440,31 @@ static int zynqmp_r5_cluster_init(struct zynqmp_r5_cluster *cluster)
> >>> goto release_r5_cores;
> >>> }
> >>>
> >>> + for (j = 0; j < cluster->core_count; j++) {
> >>> + /* Add R5 remoteproc core */
> >>> + ret = rproc_add(r5_cores[j]->rproc);
> >>> + if (ret) {
> >>> + dev_err_probe(r5_cores[j]->dev, ret,
> >>> + "failed to add remoteproc\n");
> >>> + goto delete_r5_cores;
> >>> + }
> >>> + }
> >>> +
> >>> kfree(child_devs);
> >>> return 0;
> >>>
> >>> +delete_r5_cores:
> >>> + i = core_count - 1;
> >>> + /* delete previous added rproc */
> >>> + while (--j >= 0)
> >>> + rproc_del(r5_cores[j]->rproc);
> >>> +
> >>> release_r5_cores:
> >>> while (i >= 0) {
> >>> put_device(child_devs[i]);
> >>> if (r5_cores[i]) {
> >>> zynqmp_r5_free_mbox(r5_cores[i]->ipi);
> >>> of_reserved_mem_device_release(r5_cores[i]->dev);
> >>> - rproc_del(r5_cores[i]->rproc);
> >>> rproc_free(r5_cores[i]->rproc);
> >>> }
> >>> i--;
> >>> --
> >>> 2.34.1
> >>>
> >
>
^ permalink raw reply
* Re: [PATCH v2 3/5] arm64: dts: qcom: Add Shikra CQ7790M SoM platform
From: Komal Bajaj @ 2026-05-22 14:29 UTC (permalink / raw)
To: Konrad Dybcio, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Vinod Koul, Neil Armstrong,
Wesley Cheng, Ulf Hansson
Cc: linux-arm-msm, devicetree, linux-kernel, linux-phy, linux-mmc,
monish.chunara, Rakesh Kota
In-Reply-To: <0b1c66ba-d841-460c-b702-8b4530d56f73@oss.qualcomm.com>
On 5/19/2026 5:21 PM, Konrad Dybcio wrote:
> On 5/19/26 1:21 PM, Komal Bajaj wrote:
>> Add device tree include for the CQ7790M variant of the Shikra
> I don't think CQ7790M is Shikra
Thanks for catching it. CQ7790M is Eliza. Will correct it.
Thanks
Komal
^ permalink raw reply
* Re: [PATCH v2 3/5] arm64: dts: qcom: Add Shikra CQ7790M SoM platform
From: Komal Bajaj @ 2026-05-22 14:27 UTC (permalink / raw)
To: sashiko-reviews
Cc: robh, devicetree, olteanv, conor+dt, vkoul, neil.armstrong,
linux-phy
In-Reply-To: <20260519120238.1C84CC2BCB3@smtp.kernel.org>
On 5/19/2026 5:32 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] The SoM device tree partially defines PMIC regulators (LDOs) but completely omits their internal parent SMPS regulators (S3, S4) and the required supply linkage properties.
> - [High] The SoM integrates the PM4125 PMIC, but the device tree include for the PMIC's SPMI components (`pm4125.dtsi`) is missing.
> --
>
> commit 1edd30ea88da411f667f786ef5494cebf1420c78
> Author: Komal Bajaj <komal.bajaj@oss.qualcomm.com>
>
> arm64: dts: qcom: Add Shikra CQ7790M SoM platform
>
> This commit adds the device tree include for the CQ7790M variant of the
> Shikra System-on-Module. It includes the common shikra.dtsi and adds
> PM4125 PMIC regulator definitions specific to this variant.
>
>> diff --git a/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi b/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
>> new file mode 100644
>> index 0000000000000..401e71720519d
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
>> @@ -0,0 +1,112 @@
>> +// SPDX-License-Identifier: BSD-3-Clause
>> +/*
>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>> + */
>> +
>> +#include "shikra.dtsi"
> The commit message mentions the SoM integrates the PM4125 PMIC, but is
> pm4125.dtsi supposed to be included here as well?
>
> Without including the PMIC's SPMI components (like pm4125_gpios,
> pm4125_pwrkey), it seems carrier boards won't be able to address or utilize
> these peripherals.
Will add in next revision.
>
>> +
>> +&rpm_requests {
>> + regulators {
>> + compatible = "qcom,rpm-pm2250-regulators";
>> +
>> + pm4125_s2: s2 {
>> + regulator-min-microvolt = <1000000>;
>> + regulator-max-microvolt = <1200000>;
>> + };
> Does this regulator node need to define the internal parent SMPS regulators
> like s3 and s4, as well as the vdd_X-supply linkage properties for the child
> LDOs?
>
> The driver drivers/regulator/qcom_smd-regulator.c expects supply mappings for
> pm2250 (e.g., vdd_l1_l2_l3_l5_l6_l7_l8_l9_l10_l11_l12).
>
> Without representing this fixed internal power routing in the SoM's device
> tree, the core framework might not be able to correctly manage the parent
> supply voltages and states when child LDOs are toggled.
This can be ignored, refer discussion:
https://lore.kernel.org/all/20260519120238.1C84CC2BCB3@smtp.kernel.org/
Thanks
Komal
>
>> +
>> + pm4125_l3: l3 {
>> + regulator-min-microvolt = <624000>;
>> + regulator-max-microvolt = <650000>;
>> + };
^ permalink raw reply
* RE: [PATCH v3 8/8] iio: temperature: ltc2983: Add support for ADT7604
From: Stan, Liviu @ 2026-05-22 14:24 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner
Cc: David Lechner, Sa, Nuno, Andy Shevchenko, Hennerich, Michael,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Miclaus, Antoniu,
Francesco Lavra, linux-iio@vger.kernel.org,
linux-kernel@vger.kernel.org, linux, devicetree@vger.kernel.org
In-Reply-To: <20260522143444.4582e370@jic23-huawei>
On Fri, 22 May 2026 14:34:44 +0100 Jonathan Cameron <jic23@kernel.org> wrote:
> On Thu, 21 May 2026 19:43:01 +0300
> Liviu Stan <liviu.stan@analog.com> wrote:
>
...
>
> The sashiko comment about the completion race looks real to me.
> Can you take a look at that. Might be some odd reason why we can't
> reinit_completion() before triggering the capture but that would
> be very unusual.
>
I don't see any reason why we couldn't put it before the regmap_write,
it has also already been done in ltc2983_eeprom_cmd(). So, I will change
it in v4.
> Otherwise just two really small things I missed in earlier reviews.
> If everything else was ready to go I'd have tweaked these whilst
> applying but seems you'll be doing a v4 so over to you!
Understood, will change them in v4!:) Thanks!
On Fri, 22 May 2026 09:09:33 -0500 David Lechner <dlechner@baylibre.com> wrote:
> On 5/22/26 8:23 AM, Liviu Stan wrote:
> > On Thu, 21 May 2026 19:18:28 +0000 sashiko-bot@kernel.org wrote:
> >
> >> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
...
> >> [Severity: High]
> >> This is a pre-existing issue, but can this function experience spurious
> >> read timeouts due to a race condition with the completion?
> >>
> >> Looking at the rest of this function:
> >>
> >> ret = regmap_write(st->regmap, LTC2983_STATUS_REG, start_conversion);
> >> if (ret)
> >> return ret;
> >>
> >> reinit_completion(&st->completion);
> >>
> >> If the hardware conversion finishes very quickly after regmap_write(),
> >> could the interrupt fire and call complete() before the CPU executes
> >> reinit_completion()? This would overwrite the completion event, causing the
> >> subsequent wait_for_completion_timeout() to spuriously time out.
> >>
> >> Would it be safer to call reinit_completion() before initiating the hardware
> >> action?
> >
> > This is theoretically valid but in practice, the conversion requires at minimum
> > two 82ms cycles (167ms), so there is no realistic window for the interrupt to
> > race with reinit_completion(). What do you guys think?
>
> I would still move it before the write so that it looks correct
> and others can copy/paste the logic on other devices.
That makes sense. Thanks!
Would this be considered a fix?
Liviu
^ permalink raw reply
* Re: [PATCH 6/8] arm64: dts: qcom: kaanapali: Add GPU cooling
From: Gaurav Kohli @ 2026-05-22 14:21 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Akhil P Oommen, Will Deacon, Robin Murphy, Joerg Roedel,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, Rob Clark, Dmitry Baryshkov, Abhinav Kumar,
Jessica Zhang, Marijn Suijten, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Sean Paul,
linux-arm-kernel, iommu, devicetree, linux-kernel, linux-arm-msm,
freedreno, dri-devel
In-Reply-To: <vjdz65uy4pxlbt2e4wlwvdu5llwazyopeqb3caswvejenm5iv7@3s3p7iz4otef>
On 5/17/2026 11:56 PM, Dmitry Baryshkov wrote:
> On Thu, May 14, 2026 at 12:17:54PM +0530, Gaurav Kohli wrote:
>>
>>
>> On 5/13/2026 11:23 PM, Dmitry Baryshkov wrote:
>>> On Tue, May 12, 2026 at 03:53:20AM +0530, Akhil P Oommen wrote:
>>>> From: Gaurav Kohli <gaurav.kohli@oss.qualcomm.com>
>>>>
>>>> Unlike the CPU, the GPU does not throttle its speed automatically when it
>>>> reaches high temperatures.
>>>>
>>>> Set up GPU cooling by throttling the GPU speed
>>>> when reaching 105°C.
>>>>
>>>> Signed-off-by: Gaurav Kohli <gaurav.kohli@oss.qualcomm.com>
>>>> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
>>>> ---
>>>> arch/arm64/boot/dts/qcom/kaanapali.dtsi | 165 ++++++++++++++++++++++++++------
>>>> 1 file changed, 135 insertions(+), 30 deletions(-)
>>>>
>>>> +
>>>> thermal-sensors = <&tsens5 0>;
>>>> trips {
>>>> - gpuss-0-hot {
>>>> - temperature = <120000>;
>>>> + gpuss_0_alert0: gpuss-0-alert0 {
>>>> + temperature = <105000>;
>>>> hysteresis = <5000>;
>>>> - type = "hot";
>>>> + type = "passive";
>>>> };
>>>
>>> Why don't we keep both passive and hot trip points?
>>>
>>
>> Need guidance here, we are keeping passive at low temp so still hot trip is
>> needed for such cases.
>
> I think we are saying the same. Keep both passive and hot trip points.
>
Sure, will send updated one.
>>
>>>> gpuss-0-critical {
>>>>
>>>
>>
>
^ permalink raw reply
* Re: [PATCH 2/2] remoteproc: qcom: pas: Add Shikra remoteproc support
From: Dmitry Baryshkov @ 2026-05-22 14:18 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Komal Bajaj, Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-arm-msm,
linux-remoteproc, devicetree, linux-kernel, Bibek Kumar Patro
In-Reply-To: <db13f51a-98d5-4793-9c7d-3edccc3a603c@oss.qualcomm.com>
On Fri, May 22, 2026 at 02:13:08PM +0200, Konrad Dybcio wrote:
> On 5/19/26 5:03 PM, Komal Bajaj wrote:
> > On 5/14/2026 12:48 AM, Dmitry Baryshkov wrote:
> >> On Thu, May 14, 2026 at 12:17:31AM +0530, Komal Bajaj wrote:
> >>> From: Bibek Kumar Patro <bibek.patro@oss.qualcomm.com>
> >>>
> >>> Add the CDSP, LPAICP and MPSS Peripheral Authentication Service support
> >>> for the Qualcomm Shikra SoC.
> >>>
> >>> Signed-off-by: Bibek Kumar Patro <bibek.patro@oss.qualcomm.com>
> >>> Signed-off-by: Komal Bajaj <komal.bajaj@oss.qualcomm.com>
> >>> ---
>
> [...]
>
> > Ack, I'll address it in next revision.
> >
> >> point you can use sc8180x_mpss_resource instead.
> >
> > minidump_id is required for Shikra. (decrypt_shutdown is not applicable and will be removed in the next revision.)
> > For minidump_id, we still need to use shikra_mpss_resource instead of sc8180x_mpss_resource. <?>
>
> Is there a chance the same ID would be applicable to 8180 as well,
> just that we missed it in the past?
Do we know, which platforms were the first to support minidump at all?
>
> Konrad
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH 00/11] arm64: dts: renesas: Specify ethernet PHY reset timings
From: Marek Vasut @ 2026-05-22 14:15 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: linux-arm-kernel, Conor Dooley, Geert Uytterhoeven,
Krzysztof Kozlowski, Magnus Damm, Rob Herring, devicetree,
linux-kernel, linux-renesas-soc
In-Reply-To: <CAMuHMdXoRBA-A8e=R8A4QOwdhVf2Xvca+PCD=UaKEjgFZU7RdA@mail.gmail.com>
On 5/22/26 3:18 PM, Geert Uytterhoeven wrote:
Hello Geert,
> On Tue, 5 May 2026 at 05:43, Marek Vasut
> <marek.vasut+renesas@mailbox.org> wrote:
>> This is the same patch for various boards, details are below.
>> The discussion that prompted this patchset is at [0].
>
> Thanks for your series!
>
>> The KSZ9031RNX reference manual [1] DS00002096H page 60 FIGURE 7-7:
>
> DS00002117K page 62 FIGURE 7-5
>
> (and page 74 on the older document I had ;-)
>
>> POWER-UP/POWER-DOWN/RESET TIMING Note 2 states, that after the
>> de-assertion of reset, wait a minimum of 100 us before starting
>> programming on the MIIM (MDC/MDIO) interface. Set DT property
>> reset-deassert-us to three times that, 300 us, to provide ample
>> time between reset deassertion and MDIO access.
>>
>> The KSZ9031RNX reference manual [1] DS00002096H page 60 TABLE 7-7:
>> POWER-UP/POWER-DOWN/RESET TIMING PARAMETERS row tSR Stable supply
>
> page 62 TABLE 7-4
>
>> voltages to de-assertion of reset is at minimum 10 ms. Set DT
>> property reset-assert-us to 10ms because the KSZ9031RNX RM does
>> not explicitly spell out how long the reset has to be asserted,
>> but this at least covers the worst case scenario.
>>
>> The Gray Hawk patch in this series depends on [2].
>>
>> [0] https://lore.kernel.org/all/CAMuHMdXJvrsXitGagqZJ_STdTTh_s1cBAKf6+esihaVWjfn40g@mail.gmail.com/
>> [1] https://ww1.microchip.com/downloads/aemDocuments/documents/UNG/ProductDocuments/DataSheets/KSZ9031MNX-Data-Sheet-DS00002096.pdf
>
> This link leads to the KSZ9031MNX part. Correct link is:
>
> https://ww1.microchip.com/downloads/aemDocuments/documents/UNG/ProductDocuments/DataSheets/KSZ9031RNX-Data-Sheet-DS00002117.pdf
>
>> [2] https://lore.kernel.org/all/20260504225428.114959-1-marek.vasut+renesas@mailbox.org/
>
> Shall I do the big search-and-replace while applying?
Yes please. Thank you for spotting this.
^ permalink raw reply
* Re: [PATCH v3 8/8] iio: temperature: ltc2983: Add support for ADT7604
From: David Lechner @ 2026-05-22 14:09 UTC (permalink / raw)
To: Liviu Stan, Jonathan Cameron, Nuno Sá, Andy Shevchenko,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Antoniu Miclaus, Francesco Lavra, linux-iio, linux-kernel, linux,
devicetree
In-Reply-To: <20260522132321.1641-1-liviu.stan@analog.com>
On 5/22/26 8:23 AM, Liviu Stan wrote:
> On Thu, 21 May 2026 19:18:28 +0000 sashiko-bot@kernel.org wrote:
>
>> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>>
>> New issues:
>> - [High] Reading the IIO_RESISTANCE channel bypasses hardware fault detection, causing hardware errors (like broken sensor traces) to be silently reported as valid data.
>>
>> Pre-existing issues:
>> - [High] reinit_completion() is called after initiating the hardware conversion, creating a race condition that can cause spurious read timeouts.
>> --
>>
>> commit d382a4cbefd41723ab5ddd91fd399d716c8faaec
>> Author: Liviu Stan <liviu.stan@analog.com>
>> Subject: iio: temperature: ltc2983: Add support for ADT7604
>>
>> This commit adds support for the ADT7604 temperature sensor by repurposing
>> custom RTD and thermistor types as copper trace resistance and leak detector
>> sensors. It introduces two new software sensor types and exposes a new
>> IIO_RESISTANCE channel reading from a separate result register bank. The
>> ltc2983_chip_info struct is also extended to use a 64-bit supported_sensors
>> bitmask for the new types.
>>
>>> diff --git a/drivers/iio/temperature/ltc2983.c b/drivers/iio/temperature/ltc2983.c
>>> index 326f843f42713..e2ffeee026eef 100644
>>> --- a/drivers/iio/temperature/ltc2983.c
>>> +++ b/drivers/iio/temperature/ltc2983.c
>>
>> [ ... ]
>>
>>> @@ -1205,7 +1502,8 @@ static struct ltc2983_sensor *ltc2983_temp_new(struct fwnode_handle *child,
>>> }
>>>
>>> static int ltc2983_chan_read(struct ltc2983_data *st,
>>> - const struct ltc2983_sensor *sensor, int *val)
>>> + const struct ltc2983_sensor *sensor,
>>> + u32 base_reg, int *val)
>>> {
>>> struct device *dev = &st->spi->dev;
>>> u32 start_conversion = 0;
>>
>> [Severity: High]
>> This is a pre-existing issue, but can this function experience spurious
>> read timeouts due to a race condition with the completion?
>>
>> Looking at the rest of this function:
>>
>> ret = regmap_write(st->regmap, LTC2983_STATUS_REG, start_conversion);
>> if (ret)
>> return ret;
>>
>> reinit_completion(&st->completion);
>>
>> If the hardware conversion finishes very quickly after regmap_write(),
>> could the interrupt fire and call complete() before the CPU executes
>> reinit_completion()? This would overwrite the completion event, causing the
>> subsequent wait_for_completion_timeout() to spuriously time out.
>>
>> Would it be safer to call reinit_completion() before initiating the hardware
>> action?
>
> This is theoretically valid but in practice, the conversion requires at minimum
> two 82ms cycles (167ms), so there is no realistic window for the interrupt to
> race with reinit_completion(). What do you guys think?
I would still move it before the write so that it looks correct
and others can copy/paste the logic on other devices.
^ permalink raw reply
* Re: [PATCH 3/5] arm64: dts: freescale: imx95-toradex-smarc: move CM7 node to SoC DTSI
From: Francesco Dolcini @ 2026-05-22 14:02 UTC (permalink / raw)
To: Laurentiu Mihalcea
Cc: Bjorn Andersson, Mathieu Poirier, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Frank Li, Sascha Hauer,
Peng Fan, Fabio Estevam, Pengutronix Kernel Team,
linux-remoteproc, devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20260522111849.783-4-laurentiumihalcea111@gmail.com>
On Fri, May 22, 2026 at 04:18:47AM -0700, Laurentiu Mihalcea wrote:
> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
>
> The CM7 remoteproc configuration is common to multiple MX95-based
> platforms (e.g. MX95-19x19-EVK, MX95-15x15-FRDM, SMARC-IMX95, etc.).
> Therefore, move the node to the MX95 SoC DTSI. While at it, split the mbox
> channels using <>.
>
> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com>
...
> diff --git a/arch/arm64/boot/dts/freescale/imx95.dtsi b/arch/arm64/boot/dts/freescale/imx95.dtsi
> index 3e35c956a4d7..f8760ac067fa 100644
> --- a/arch/arm64/boot/dts/freescale/imx95.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx95.dtsi
> @@ -272,6 +272,13 @@ opp-1000000000 {
> };
> };
>
> + cm7: remoteproc-cm7 {
> + compatible = "fsl,imx95-cm7";
> + mboxes = <&mu7 0 1>, <&mu7 1 1>, <&mu7 3 1>;
> + mbox-names = "tx", "rx", "rxdb";
> + status = "disabled";
> + };
> +
As sashiko noticed, this is introducing a regression. No-one is enabling
this node anymore.
Francesco
^ permalink raw reply
* Re: [PATCH v2 05/11] arm64: dts: ti: k3-am62-verdin: Add Toradex Capacitive Touch Display 10.1" DSI
From: sashiko-bot @ 2026-05-22 14:02 UTC (permalink / raw)
To: Vitor Soares; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260522132014.226721-18-ivitro@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incorrect interrupt trigger polarity for the ILI2132 touch controller.
--
commit e23b19a55c448169a4462f9aea9b95fee61f05ad
Author: Vitor Soares <vitor.soares@toradex.com>
arm64: dts: ti: k3-am62-verdin: Add Toradex Capacitive Touch Display 10.1" DSI
This commit adds a device tree overlay for the Toradex Capacitive Touch
Display 10.1" on the Verdin DSI_1 interface. It configures the SN65DSI83
DSI-to-LVDS bridge and the Ilitek ILI2132 touch controller. It also updates
the Makefile to provide ready-to-use DTBs in both WiFi and non-Wifi SoM
variants.
Link: https://developer.toradex.com/hardware/accessories/displays/capacitive-touch-display-101inch-dsi
Link: https://developer.toradex.com/hardware/accessories/add-ons/dsi-display-adapter/
> diff --git a/arch/arm64/boot/dts/ti/k3-am625-verdin-panel-cap-touch-10inch-dsi.dtso b/arch/arm64/boot/dts/ti/k3-am625-verdin-panel-cap-touch-10inch-dsi.dtso
> new file mode 100644
> index 0000000000000..de0148ddd5969
> --- /dev/null
> +++ b/arch/arm64/boot/dts/ti/k3-am625-verdin-panel-cap-touch-10inch-dsi.dtso
[ ... ]
> + touch@41 {
> + compatible = "ilitek,ili2132";
> + reg = <0x41>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_dsi1_int>, <&pinctrl_i2s_2_bclk_gpio>;
> + /* Verdin GPIO_9_DSI (SODIMM 17) - TOUCH_INT# */
> + interrupt-parent = <&main_gpio1>;
> + interrupts = <49 IRQ_TYPE_EDGE_RISING>;
[Severity: High]
Should this interrupt trigger be configured as IRQ_TYPE_EDGE_FALLING instead?
The comment above indicates the hardware signal is TOUCH_INT#, which implies
an active-low signal. If the touch controller holds the interrupt line low
until the touch data is read by the host over I2C, waiting for a rising edge
might cause a deadlock where the line never goes high because the kernel won't
read the data without seeing an interrupt first.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260522132014.226721-13-ivitro@gmail.com?part=5
^ permalink raw reply
* Re: [PATCH v2 10/10] arm64: dts: renesas: r9a09g087: add MTU3 support
From: Geert Uytterhoeven @ 2026-05-22 13:55 UTC (permalink / raw)
To: Cosmin Tanislav
Cc: Biju Das, Daniel Lezcano, Thomas Gleixner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Magnus Damm, Michael Turquette,
Stephen Boyd, Lee Jones, Philipp Zabel, linux-iio,
linux-renesas-soc, linux-kernel, devicetree, linux-clk
In-Reply-To: <20260410163530.383818-11-cosmin-gabriel.tanislav.xa@renesas.com>
On Fri, 10 Apr 2026 at 18:37, Cosmin Tanislav
<cosmin-gabriel.tanislav.xa@renesas.com> wrote:
> The Renesas RZ/N2H (R9A09G087) SoC has an MTU3 block.
>
> Add support for it.
>
> Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v7.2.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* RE: [PATCH v3 2/8] iio: temperature: ltc2983: Use local device pointer consistently
From: Stan, Liviu @ 2026-05-22 13:56 UTC (permalink / raw)
To: Jonathan Cameron
Cc: David Lechner, Sa, Nuno, Andy Shevchenko, Hennerich, Michael,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Miclaus, Antoniu,
Francesco Lavra, linux-iio@vger.kernel.org,
linux-kernel@vger.kernel.org, linux, devicetree@vger.kernel.org
In-Reply-To: <20260522140255.27445c41@jic23-huawei>
On Fri, 22 May 2026 14:02:55 +0100 Jonathan Cameron <jic23@kernel.org> wrote:
> On Thu, 21 May 2026 19:42:55 +0300
> Liviu Stan <liviu.stan@analog.com> wrote:
>
> > Some functions define a local 'dev' pointer but still use bare
> > '&st->spi->dev' in some code paths, and some don't have it at all.
> > Replace bare references with the local pointer for consistency.
> >
> > Signed-off-by: Liviu Stan <liviu.stan@analog.com>
>
> Hi Liviu,
>
> When doing this sort of change, one of the advantages is often that code lines
> get shorter. So look at the lines touched and see if the wrapping remains
> appropriate.
>
> At least some of the cases I point out below already fitted on one line
> under 80 chars but none the less they are now even shorter so that needs
> tidying up. Note that a few other cases are just over 80 chars.
> Take a look at those and decide if readability is improved much by just
> going a few characters over. That line length isn't the hard rule it
> used to be!
>
Hi,
Understood, I'll tidy them up! Thanks!
Liviu
^ permalink raw reply
* Re: [PATCH v2 09/10] arm64: dts: renesas: r9a09g077: add MTU3 support
From: Geert Uytterhoeven @ 2026-05-22 13:55 UTC (permalink / raw)
To: Cosmin Tanislav
Cc: Biju Das, Daniel Lezcano, Thomas Gleixner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Magnus Damm, Michael Turquette,
Stephen Boyd, Lee Jones, Philipp Zabel, linux-iio,
linux-renesas-soc, linux-kernel, devicetree, linux-clk
In-Reply-To: <20260410163530.383818-10-cosmin-gabriel.tanislav.xa@renesas.com>
On Fri, 10 Apr 2026 at 18:37, Cosmin Tanislav
<cosmin-gabriel.tanislav.xa@renesas.com> wrote:
> The Renesas RZ/T2H (R9A09G077) SoC has an MTU3 block.
>
> Add support for it.
>
> Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v7.2.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH v2 01/11] arm64: dts: ti: k3-am62-verdin: Add Toradex DSI to LVDS adapter with 10.1" display
From: sashiko-bot @ 2026-05-22 13:52 UTC (permalink / raw)
To: Vitor Soares; +Cc: conor+dt, robh, devicetree
In-Reply-To: <20260522132014.226721-14-ivitro@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] Nodes use specific device or misleading names instead of the required generic class names.
- [Low] The fixed regulator node 'regulator-3v3-lvds-bridge' is missing the strictly required 'regulator-name' property.
--
commit 8eb886986d19e2608d6989dd557b140a93c31c32
Author: Vitor Soares <vitor.soares@toradex.com>
arm64: dts: ti: k3-am62-verdin: Add Toradex DSI to LVDS adapter with 10.1" display
This commit adds a device tree overlay for the Toradex DSI to LVDS Adapter
connected to a 10.1" capacitive touch display. It configures the SN65DSI84
bridge, an LVDS panel, and an Atmel MaxTouch controller on the Verdin DSI_1
interface.
Link: https://developer.toradex.com/hardware/accessories/add-ons/dsi-lvds-adapter
Link: https://developer.toradex.com/hardware/accessories/displays/capacitive-touch-display-101inch-lvds
[ ... ]
> diff --git a/arch/arm64/boot/dts/ti/k3-am625-verdin-dsi-to-lvds-panel-cap-touch-10inch.dtso b/arch/arm64/boot/dts/ti/k3-am625-verdin-dsi-to-lvds-panel-cap-touch-10inch.dtso
> new file mode 100644
> index 0000000000000..0e873f2ccf65a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/ti/k3-am625-verdin-dsi-to-lvds-panel-cap-touch-10inch.dtso
> @@ -0,0 +1,123 @@
[ ... ]
> +&{/} {
> + backlight_pwm3: backlight-pwm3 {
[Severity: Low]
Should this node use a generic name like 'backlight' instead of
'backlight-pwm3' to comply with the Devicetree Specification recommendations?
Using non-generic names might cause schema validation warnings during
dtbs_check.
> + compatible = "pwm-backlight";
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_i2s_2_d_out_gpio>;
> + brightness-levels = <0 45 63 88 119 158 203 255>;
> + default-brightness-level = <4>;
> + /* Verdin I2S_2_D_OUT as GPIO (SODIMM 46) */
> + enable-gpios = <&main_gpio0 34 GPIO_ACTIVE_HIGH>;
> + power-supply = <®_3v3>;
> + /* Verdin PWM_3_DSI (SODIMM 19) */
> + pwms = <&epwm1 0 6666667 PWM_POLARITY_INVERTED>;
> + };
> +
> + panel-lvds-bridge {
[Severity: Low]
Does this node name deviate from the Devicetree Specification recommendations?
Using a generic name like 'panel' is usually preferred and helps avoid
schema validation warnings.
> + compatible = "logictechno,lt170410-2whc";
> + backlight = <&backlight_pwm3>;
> + power-supply = <®_3v3_lvds_bridge>;
> +
> + port {
> + panel_lvds_bridge_in: endpoint {
> + remote-endpoint = <&dsi_lvds_bridge_out>;
> + };
> + };
> + };
> +
> + reg_3v3_lvds_bridge: regulator-3v3-lvds-bridge {
> + compatible = "regulator-fixed";
> + regulator-max-microvolt = <3300000>;
> + regulator-min-microvolt = <3300000>;
[Severity: Low]
Is the 'regulator-name' property missing here?
According to the fixed regulator bindings in
Documentation/devicetree/bindings/regulator/fixed-regulator.yaml, this
property is mandatory when the compatible string is "regulator-fixed".
Omitting it will cause dtbs_check errors.
> + };
> +};
[ ... ]
> +/* Verdin I2C_1 */
> +&main_i2c1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + bridge@2c {
> + compatible = "ti,sn65dsi84";
[ ... ]
> + };
> + };
> +
> + touch@4a {
[Severity: Low]
Could we use the generic class name 'touchscreen@4a' here instead of
'touch@4a' to align with standard device tree conventions?
> + compatible = "atmel,maxtouch";
> + reg = <0x4a>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_dsi1_int>, <&pinctrl_i2s_2_bclk_gpio>;
> + /* Verdin GPIO_9_DSI (SODIMM 17) - TOUCH_INT# */
> + interrupt-parent = <&main_gpio1>;
> + interrupts = <49 IRQ_TYPE_EDGE_FALLING>;
> + /* Verdin I2S_2_BCLK (SODIMM 42) - TOUCH_RESET# */
> + reset-gpios = <&main_gpio0 35 GPIO_ACTIVE_LOW>;
> + };
> +};
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260522132014.226721-13-ivitro@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH v5 2/2] regulator: add SGM3804 Dual Output driver
From: sashiko-bot @ 2026-05-22 13:50 UTC (permalink / raw)
To: Neil Armstrong; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260522-topic-sm8650-ayaneo-pocket-s2-sgm3804-v5-2-bd6b1c300ecc@linaro.org>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Enable GPIOs are requested with `GPIOD_ASIS` but never explicitly configured as outputs, preventing the regulator from powering up.
- [High] The driver performs I2C communication immediately after asserting the enable GPIO, bypassing the required IC power-on delay.
- [Medium] Incorrect regmap state handling on `regcache_sync()` failure leads to silent I2C write failures for active rails.
- [Medium] Hardware is powered down before setting the regmap to cache-only mode in `sgm3804_disable()`, creating a race condition.
- [Low] The `ctx->lock` mutex is initialized without a corresponding `mutex_destroy()`, bypassing mutex debugging.
--
commit cd53576c2dc0f1b61e28aacf0d0ccbf347793fe0
Author: KancyJoe <kancy2333@outlook.com>
regulator: add SGM3804 Dual Output driver
Add support for the SG Micro SGM3804 Single Inductor Dual Output
Buck/Boost Converter. It uses a regmap cache because registers are only
writable when the enable GPIOs are active.
> diff --git a/drivers/regulator/sgm3804-regulator.c b/drivers/regulator/sgm3804-regulator.c
> new file mode 100644
> index 0000000000000..c3406cfb73d03
> --- /dev/null
> +++ b/drivers/regulator/sgm3804-regulator.c
[ ... ]
> +static int sgm3804_sync_regcache_state(struct sgm3804_data *ctx)
> +{
> + guard(mutex)(&ctx->lock);
> +
> + /* If both GPIOs are down, IC is powered down and I2C writes will fail */
> + if (!gpiod_get_value_cansleep(ctx->gpios[SGM3804_POS_RAIL]) &&
> + !gpiod_get_value_cansleep(ctx->gpios[SGM3804_NEG_RAIL])) {
> + regcache_cache_only(ctx->regmap, true);
> + regcache_mark_dirty(ctx->regmap);
> + } else {
> + int ret;
> +
> + /* At least a GPIO is up, we can write registers */
> + regcache_cache_only(ctx->regmap, false);
> + ret = regcache_sync(ctx->regmap);
> + if (ret) {
> + regcache_cache_only(ctx->regmap, true);
[Severity: Medium]
If one regulator is already successfully enabled, and the user attempts to
enable the second one but the I2C transfer fails, does this permanently stick
the regmap cache in cache-only mode?
Any subsequent voltage or discharge changes to the active rail will only update
the software cache and fail to write to the physical hardware.
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
[ ... ]
> +static int sgm3804_enable(struct regulator_dev *rdev)
> +{
> + struct sgm3804_data *ctx = rdev->reg_data;
> + int ret;
> +
> + ret = gpiod_set_value_cansleep(ctx->gpios[rdev_get_id(rdev)], 1);
> + if (ret)
> + return ret;
> +
> + ret = sgm3804_sync_regcache_state(ctx);
[Severity: High]
Does this perform I2C communication immediately after asserting the enable
GPIO, bypassing the required IC power-on delay?
The driver specifies .enable_time = 40000, but because the driver issues
I2C commands synchronously inside the enable callback, the regulator core's
delay hasn't occurred yet.
Will sending I2C commands immediately result in I2C NACK errors?
> + if (ret)
> + goto err;
> +
> + return 0;
> +
> +err:
> + gpiod_set_value_cansleep(ctx->gpios[rdev_get_id(rdev)], 0);
> + return ret;
> +}
> +
> +static int sgm3804_disable(struct regulator_dev *rdev)
> +{
> + struct sgm3804_data *ctx = rdev->reg_data;
> + int ret;
> +
> + ret = gpiod_set_value_cansleep(ctx->gpios[rdev_get_id(rdev)], 0);
> + if (ret)
> + return ret;
> +
> + return sgm3804_sync_regcache_state(ctx);
[Severity: Medium]
Is there a race condition here if the hardware is powered down before
setting the regmap to cache-only mode?
If this is the last active rail, the physical IC loses power immediately.
If another thread concurrently accesses the shared regmap to set the voltage
on the other rail, it could attempt an I2C transaction on the powered-off IC.
> +}
[ ... ]
> +static int sgm3804_probe(struct i2c_client *i2c)
> +{
> + struct device *dev = &i2c->dev;
> + struct sgm3804_data *ctx;
> + int ret, i;
> +
> + ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> + if (!ctx)
> + return -ENOMEM;
> +
> + mutex_init(&ctx->lock);
[Severity: Low]
Is this missing a corresponding mutex_destroy()?
Since ctx is allocated with devm_kzalloc(), the memory will be freed without
destroying the mutex, which bypasses the kernel's CONFIG_DEBUG_MUTEXES
infrastructure.
Would it be better to use devm_mutex_init() instead?
> +
> + ctx->regmap = devm_regmap_init_i2c(i2c, &sgm3804_regmap_config);
> + if (IS_ERR(ctx->regmap))
> + return dev_err_probe(dev, PTR_ERR(ctx->regmap),
> + "failed to init regmap\n");
> +
> + /* Get enable GPIOs */
> + for (i = 0; i < ARRAY_SIZE(sgm3804_regulator_desc); i++) {
> + const struct regulator_desc *reg = &sgm3804_regulator_desc[i];
> + struct fwnode_handle *child;
> +
> + child = device_get_named_child_node(dev, reg->of_match);
> + if (!child) {
> + dev_err(dev, "missing child '%s'\n", reg->of_match);
> + return -EINVAL;
> + }
> +
> + ctx->gpios[i] = devm_fwnode_gpiod_get(dev, child, "enable",
> + GPIOD_ASIS, reg->name);
[Severity: High]
Does this prevent the regulator from powering up?
The GPIOD_ASIS flag leaves the pin's hardware direction unconfigured.
If the bootloader left the pin as an input, calling gpiod_set_value_cansleep()
in sgm3804_enable() will fail and return -EPERM because the pin is not
configured as an output.
Could the GPIO direction be explicitly configured as an output instead?
> + fwnode_handle_put(child);
> + if (IS_ERR(ctx->gpios[i]))
> + return dev_err_probe(dev, PTR_ERR(ctx->gpios[i]),
> + "failed to get '%s' enable GPIO\n",
> + reg->name);
> + }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260522-topic-sm8650-ayaneo-pocket-s2-sgm3804-v5-0-bd6b1c300ecc@linaro.org?part=2
^ permalink raw reply
* Re: [PATCH v2] arm64: dts: qcom: sm8650-ayaneo-pocket-s2: add display nodes
From: sashiko-bot @ 2026-05-22 13:47 UTC (permalink / raw)
To: Neil Armstrong; +Cc: conor+dt, devicetree, robh, wsa+renesas
In-Reply-To: <20260522-topic-sm8650-ayaneo-pocket-s2-display-dt-v2-1-cdd4b70e5a16@linaro.org>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The Goodix GT911 touchscreen node specifies `reset-gpios` but omits the required `irq-gpios` property, silently disabling the driver's hardware reset and power management logic.
- [High] The device tree relies on hardware components (SGM3804 regulator and SY7758 backlight) that currently have no supporting driver or device tree binding in the kernel.
- [Medium] The `mdp_vsync_active` and `mdp_vsync_suspend` pinctrl states are defined but never referenced in the display or MDSS nodes, preventing the hardware VSYNC signal from being routed.
- [Low] The backlight node is named `sy7758@2e` instead of a generic name like `backlight@2e`.
--
commit a7774645b5e43100828950906f34539f8e7aadef
Author: KancyJoe <kancy2333@outlook.com>
arm64: dts: qcom: sm8650-ayaneo-pocket-s2: add display nodes
This commit adds device tree nodes for the dual DSI panel, the SGM3804
regulator, and the SY7758 backlight controller. It also enables the GPU
and the touch controller to provide full display support.
> diff --git a/arch/arm64/boot/dts/qcom/sm8650-ayaneo-pocket-s2.dts b/arch/arm64/boot/dts/qcom/sm8650-ayaneo-pocket-s2.dts
> index 0dc994f4e48d9..af890e76813b7 100644
> --- a/arch/arm64/boot/dts/qcom/sm8650-ayaneo-pocket-s2.dts
> +++ b/arch/arm64/boot/dts/qcom/sm8650-ayaneo-pocket-s2.dts
[ ... ]
> @@ -1016,6 +1040,30 @@ wcd_usbss_sbu_mux: endpoint {
> };
> };
>
> +&i2c4 {
> + clock-frequency = <400000>;
> +
> + status = "okay";
> +
> + touchscreen@5d {
> + compatible = "goodix,gt911";
> + reg = <0x5d>;
> +
> + interrupt-parent = <&tlmm>;
> + interrupts = <162 IRQ_TYPE_EDGE_FALLING>;
> +
> + reset-gpios = <&tlmm 161 GPIO_ACTIVE_HIGH>;
[Severity: High]
Is the irq-gpios property missing from this node?
The Goodix driver requires retrieving the interrupt GPIO via the irq-gpios
property to execute its hardware reset sequence, as the INT pin state
determines the I2C address.
When irq-gpios is omitted, the driver's irq_pin_access_method falls back
to IRQ_PIN_ACCESS_NONE. This completely bypasses the goodix_reset()
function during probe.
Additionally, goodix_suspend() and goodix_resume() will return early without
issuing the deep sleep command, which can cause power drain during system
suspend.
> + VDDIO-supply = <&vreg_l14b_3p2>;
> + AVDD28-supply = <&vreg_l14b_3p2>;
> +
> + touchscreen-size-x = <1440>;
> + touchscreen-size-y = <2560>;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&ts_reset_default>, <&ts_irq_default>;
> + };
> +};
> +
[ ... ]
> @@ -1053,6 +1101,49 @@ redriver_ss_in: endpoint {
> };
> };
>
> +&i2c9 {
> + status = "okay";
> +
> + /* Screen power */
> + regulator@3e {
> + compatible = "sgmicro,sgm3804";
[Severity: High]
Do these components currently have supporting drivers and device tree bindings
upstream?
Because there are no existing drivers for sgmicro,sgm3804 or silergy,sy7758
in the kernel, they will not probe.
When the panel driver attempts to acquire its essential supplies (vsp-supply,
vsn-supply, and backlight), it will hit a permanent -EPROBE_DEFER, preventing
the entire display subsystem from initializing.
> + reg = <0x3e>;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&sgm3804_default>;
> +
> + vin-supply = <&vph_pwr>;
> +
[ ... ]
> + /* Backlight */
> + sy7758_backlight: sy7758@2e {
[Severity: Low]
Could this node be named using a generic class like backlight@2e?
The Devicetree Specification recommends using generic node names that reflect
the function of the device rather than the specific model or IC name.
Non-generic names can trigger validation errors during dt_binding_check.
> + compatible = "silergy,sy7758";
> + reg = <0x2e>;
> +
> + pinctrl-names = "default";
> + pinctrl-0 = <&sy7758_default>;
> +
> + vddio-supply = <&sy7758_vdd33_reg>;
> + enable-gpios = <&tlmm 164 GPIO_ACTIVE_HIGH>;
> + };
> +};
[ ... ]
> @@ -1411,6 +1603,20 @@ fan_int: fan-int-state {
> bias-pull-up;
> };
>
> + mdp_vsync_active: mdp-vsync-active-state {
> + pins = "gpio86";
> + function = "mdp_vsync";
> + drive-strength = <2>;
> + bias-pull-down;
> + };
> +
> + mdp_vsync_suspend: mdp-vsync-suspend-state {
> + pins = "gpio86";
> + function = "mdp_vsync";
> + drive-strength = <2>;
> + bias-pull-down;
> + };
> +
[Severity: Medium]
Are these mdp_vsync pinctrl states supposed to be referenced in the panel or
MDSS nodes?
They appear to be defined here but are never assigned to the pinctrl-0 or
pinctrl-1 properties of the display components. Command-mode panels typically
rely on this Tearing Effect (TE) signal to synchronize frame updates.
Without routing the signal, the display controller may fall back to software
synchronization or timeout waiting for TE interrupts, which can result in
frame drops or tearing.
> upd720201_avdd33: upd720201-avdd33-state {
> pins = "gpio123";
> function = "gpio";
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260522-topic-sm8650-ayaneo-pocket-s2-display-dt-v2-1-cdd4b70e5a16@linaro.org?part=1
^ permalink raw reply
* Re: [PATCH v2 06/11] arm64: dts: ti: k3-am62-verdin: Add Toradex Capacitive Touch Display 7" DSI
From: sashiko-bot @ 2026-05-22 13:44 UTC (permalink / raw)
To: Vitor Soares; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260522132014.226721-19-ivitro@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The ILI2132 touchscreen interrupt `TOUCH_INT#` is configured to trigger on a rising edge (`IRQ_TYPE_EDGE_RISING`), which is incorrect for an active-low signal.
--
commit 19d0e0e35f5e02a5ec8add36ff8be23133d5e81b
Author: Vitor Soares <vitor.soares@toradex.com>
arm64: dts: ti: k3-am62-verdin: Add Toradex Capacitive Touch Display 7" DSI
This commit adds a device tree overlay for the Toradex Capacitive Touch Display
7" DSI on the Verdin DSI_1 interface. The display uses a Texas Instruments
SN65DSI83 bridge and an Ilitek ILI2132 capacitive touch controller.
Link: https://developer.toradex.com/hardware/accessories/displays/capacitive-touch-display-7inch-dsi
Link: https://developer.toradex.com/hardware/accessories/add-ons/dsi-display-adapter/
> diff --git a/arch/arm64/boot/dts/ti/k3-am625-verdin-panel-cap-touch-7inch-dsi.dtso b/arch/arm64/boot/dts/ti/k3-am625-verdin-panel-cap-touch-7inch-dsi.dtso
> new file mode 100644
> index 0000000000000..0fa8306324b38
> --- /dev/null
> +++ b/arch/arm64/boot/dts/ti/k3-am625-verdin-panel-cap-touch-7inch-dsi.dtso
[ ... ]
> + touch@41 {
> + compatible = "ilitek,ili2132";
> + reg = <0x41>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&pinctrl_dsi1_int>, <&pinctrl_i2s_2_bclk_gpio>;
> + /* Verdin GPIO_9_DSI (SODIMM 17) - TOUCH_INT# */
> + interrupt-parent = <&main_gpio1>;
> + interrupts = <49 IRQ_TYPE_EDGE_RISING>;
[Severity: High]
Does this interrupt configuration match the hardware signal?
The comment identifies the signal as TOUCH_INT#, where the # suffix typically
denotes an active-low signal. However, it is configured here to trigger on
a rising edge.
For an active-low signal, a rising edge occurs when the touch controller
releases the interrupt line rather than when it asserts it. This might result
in dropped touch events or an unresponsive touch input.
Should this be configured as IRQ_TYPE_LEVEL_LOW or IRQ_TYPE_EDGE_FALLING
instead?
> + /* Verdin I2S_2_BCLK (SODIMM 42) - TOUCH_RESET# */
> + reset-gpios = <&main_gpio0 35 GPIO_ACTIVE_LOW>;
> + };
> +};
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260522132014.226721-13-ivitro@gmail.com?part=6
^ permalink raw reply
* Re: [PATCH v2 08/10] arm64: dts: renesas: r9a07g0{43,44,54}: remove TCIU8 interrupt from MTU3
From: Geert Uytterhoeven @ 2026-05-22 13:43 UTC (permalink / raw)
To: Cosmin Tanislav
Cc: Biju Das, Daniel Lezcano, Thomas Gleixner, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Magnus Damm, Michael Turquette,
Stephen Boyd, Lee Jones, Philipp Zabel, linux-iio,
linux-renesas-soc, linux-kernel, devicetree, linux-clk
In-Reply-To: <20260410163530.383818-9-cosmin-gabriel.tanislav.xa@renesas.com>
On Fri, 10 Apr 2026 at 18:37, Cosmin Tanislav
<cosmin-gabriel.tanislav.xa@renesas.com> wrote:
> The TCIU8 interrupt used to be documented in earlier revisions of the
> user manuals, but has since been removed. The corresponding entry is now
> marked as reserved in the interrupt mapping tables of all supported
> SoCs.
>
> * Page 486, Table 8.2 Interrupt mapping (7/13) in the Renesas RZ/G2UL
> Rev.1.40 User Manual
> * Page 363, Table 8.2 Interrupt Mapping (6/13) in the Renesas RZ/Five
> Rev.1.30 User Manual
> * Page 528, Table 8.2 Interrupt mapping (7/13) in the Renesas RZ/G2L
> and RZ/G2LC Rev.1.50 User Manual
> * Page 540, Table 8.2 Interrupt mapping (7/13) in the Renesas RZ/V2L
> Rev.1.50 User Manual
>
> Remove the TCIU8 interrupt. This does not cause any breakage as the
> driver does not make use of the interrupts.
>
> Signed-off-by: Cosmin Tanislav <cosmin-gabriel.tanislav.xa@renesas.com>
> ---
>
> V2:
> * reword to mention that TCIU8 used to exist
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v7.2.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox