* Re: [PATCH 6/6] clk: qcom: gpucc: Add Nord graphics clock controller support
From: Konrad Dybcio @ 2026-06-25 12:09 UTC (permalink / raw)
To: Taniya Das, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Brian Masney, Shawn Guo, Bartosz Golaszewski, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong, Konrad Dybcio
Cc: Ajit Pandey, Imran Shaik, Jagadeesh Kona, linux-arm-msm,
linux-clk, linux-kernel, devicetree
In-Reply-To: <20260623-nords_mm_v1-v1-6-860c84539804@oss.qualcomm.com>
On 6/23/26 12:54 PM, Taniya Das wrote:
> Add support for the GPU clock controllers (GPUCC) on the Qualcomm
> Nord platform.
>
> The platform includes two GPU clock controller instances,GPUCC
> and GPU2CC. Register support for both controllers, which provide
> clocks required for the graphics subsystem.
>
> Signed-off-by: Taniya Das <taniya.das@oss.qualcomm.com>
> ---
> drivers/clk/qcom/Kconfig | 11 +
> drivers/clk/qcom/Makefile | 1 +
> drivers/clk/qcom/gpu2cc-nord.c | 546 +++++++++++++++++++++++++++++++++++++
> drivers/clk/qcom/gpucc-nord.c | 593 +++++++++++++++++++++++++++++++++++++++++
GPU_2_CC and GPU_CC seem to have a different set of clocks and
resets. If that's not physically the case, please align the
lists. Otherwise, please use a separate bindings header for each
one
Konrad
^ permalink raw reply
* [PATCH v3 3/3] arm64: dts: qcom: sm8250-xiaomi-elish: Add ov8856 front camera
From: Xin Xu @ 2026-06-25 12:08 UTC (permalink / raw)
To: konrad.dybcio, andersson, konradybcio
Cc: linux-arm-msm, devicetree, linux-kernel, Xin Xu
In-Reply-To: <tencent_47473BB31C9142449F8C7C1BE69D9071510A@qq.com>
Add the ov8856 front camera, connected on CCI1 to CSIPHY4 and
powered by pm8008 LDOs and other supplies.
Signed-off-by: Xin Xu <xxsemail@qq.com>
---
Changes in v3:
- Reorder pinctrl properties in camera_front_active.
- Use the new cam_mclk3_default from sm8250.dtsi and remove board-local
mclk3 node.
Changes in v2:
- Fix coding style (property order)
.../dts/qcom/sm8250-xiaomi-elish-common.dtsi | 63 +++++++++++++++++++
1 file changed, 63 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8250-xiaomi-elish-common.dtsi b/arch/arm64/boot/dts/qcom/sm8250-xiaomi-elish-common.dtsi
index 05c366eaeff1..19aff83ecf7b 100644
--- a/arch/arm64/boot/dts/qcom/sm8250-xiaomi-elish-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250-xiaomi-elish-common.dtsi
@@ -4,6 +4,7 @@
*/
#include <dt-bindings/arm/qcom,ids.h>
+#include <dt-bindings/media/video-interfaces.h>
#include <dt-bindings/phy/phy.h>
#include <dt-bindings/regulator/qcom,rpmh-regulator.h>
#include <dt-bindings/usb/pd.h>
@@ -531,6 +532,61 @@ vreg_l7f_1p8: ldo7 {
};
};
+&camss {
+ status = "okay";
+
+ vdda-phy-supply = <&vreg_l5a_0p88>;
+ vdda-pll-supply = <&vreg_l9a_1p2>;
+
+ ports {
+ port@4 {
+ csiphy4_ep: endpoint {
+ clock-lanes = <7>;
+ data-lanes = <0 1>;
+ bus-type = <MEDIA_BUS_TYPE_CSI2_DPHY>;
+ remote-endpoint = <&ov8856_front_ep>;
+ };
+ };
+ };
+};
+
+&cci1 {
+ status = "okay";
+};
+
+&cci1_i2c1 {
+ camera_front: camera@10 {
+ compatible = "ovti,ov8856";
+ reg = <0x10>;
+
+ avdd-supply = <&vreg_l5p>;
+ dovdd-supply = <&vreg_l1c_1p8>;
+ dvdd-supply = <&vreg_l1p>;
+
+ clocks = <&camcc CAM_CC_MCLK3_CLK>;
+ clock-names = "xvclk";
+ assigned-clocks = <&camcc CAM_CC_MCLK3_CLK>;
+ assigned-clock-rates = <19200000>;
+
+ reset-gpios = <&tlmm 109 GPIO_ACTIVE_LOW>;
+
+ pinctrl-0 = <&cam_mclk3_default &camera_front_active>;
+ pinctrl-names = "default";
+
+ orientation = <0>; /* Front facing */
+ rotation = <270>;
+
+ port {
+ ov8856_front_ep: endpoint {
+ data-lanes = <1 2>;
+ link-frequencies = /bits/ 64
+ <720000000 360000000>;
+ remote-endpoint = <&csiphy4_ep>;
+ };
+ };
+ };
+};
+
&cdsp {
firmware-name = "qcom/sm8250/xiaomi/elish/cdsp.mbn";
status = "okay";
@@ -891,6 +947,13 @@ pm8008_reset_default: pm8008-reset-default-state {
bias-pull-up;
};
+ camera_front_active: camera-front-active-state {
+ pins = "gpio109";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
wlan_en_state: wlan-default-state {
pins = "gpio20";
function = "gpio";
--
2.53.0
^ permalink raw reply related
* [PATCH 1/3] arm64: dts: ti: Add PLL1 refclk to J784S4 SoC SERDES node
From: Gokul Praveen @ 2026-06-25 12:08 UTC (permalink / raw)
To: vigneshr, kristo, conor+dt, c-vankar, s-vadapalli, krzk+dt, davem,
andrew+netdev, pabeni, kuba, g-praveen, devicetree, linux-kernel
In-Reply-To: <20260625120805.28948-1-g-praveen@ti.com>
Add PLL1 refclk to "clocks" and "clock-names" parameter of SERDES2 node
as "assigned clocks" parameter has PLL1 and serdes multilink configuration
fails without PLL1.
Signed-off-by: Gokul Praveen <g-praveen@ti.com>
---
arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
index 78fcd0c40abc..da8d582574d0 100644
--- a/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
@@ -122,8 +122,9 @@ serdes2: serdes@5020000 {
resets = <&serdes_wiz2 0>;
reset-names = "torrent_reset";
clocks = <&serdes_wiz2 TI_WIZ_PLL0_REFCLK>,
+ <&serdes_wiz2 TI_WIZ_PLL1_REFCLK>,
<&serdes_wiz2 TI_WIZ_PHY_EN_REFCLK>;
- clock-names = "refclk", "phy_en_refclk";
+ clock-names = "refclk","pll1_refclk", "phy_en_refclk";
assigned-clocks = <&serdes_wiz2 TI_WIZ_PLL0_REFCLK>,
<&serdes_wiz2 TI_WIZ_PLL1_REFCLK>,
<&serdes_wiz2 TI_WIZ_REFCLK_DIG>;
--
2.34.1
^ permalink raw reply related
* [PATCH 2/3] net: ethernet: ti: am65-cpsw: Fix MAC configuration for SGMII mode
From: Gokul Praveen @ 2026-06-25 12:08 UTC (permalink / raw)
To: vigneshr, kristo, conor+dt, c-vankar, s-vadapalli, krzk+dt, davem,
andrew+netdev, pabeni, kuba, g-praveen, devicetree, linux-kernel
In-Reply-To: <20260625120805.28948-1-g-praveen@ti.com>
Fix MAC configuration of CPSW Ethernet Switch in SGMII mode by
modifying the CPSW_SGMII_MR_ADV_ABILITY_REG and configuring MAC
in master mode as per CPSGMII Module specification.
Signed-off-by: Gokul Praveen <g-praveen@ti.com>
---
drivers/net/ethernet/ti/am65-cpsw-nuss.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index 434a31080855..291697d82491 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -84,6 +84,7 @@
#define AM65_CPSW_SGMII_CONTROL_REG 0x010
#define AM65_CPSW_SGMII_MR_ADV_ABILITY_REG 0x018
#define AM65_CPSW_SGMII_CONTROL_MR_AN_ENABLE BIT(0)
+#define AM65_CPSW_SGMII_CONTROL_MASTER BIT(5)
#define AM65_CPSW_CTL_VLAN_AWARE BIT(1)
#define AM65_CPSW_CTL_P0_ENABLE BIT(2)
@@ -2061,8 +2062,10 @@ static void am65_cpsw_nuss_mac_config(struct phylink_config *config, unsigned in
if (common->pdata.extra_modes & BIT(state->interface)) {
if (state->interface == PHY_INTERFACE_MODE_SGMII) {
- writel(ADVERTISE_SGMII,
- port->sgmii_base + AM65_CPSW_SGMII_MR_ADV_ABILITY_REG);
+ writel(ADVERTISE_SGMII | LPA_SGMII_1000FULL | LPA_SGMII_LINK,
+ port->sgmii_base + AM65_CPSW_SGMII_MR_ADV_ABILITY_REG);
+ writel(AM65_CPSW_SGMII_CONTROL_MR_AN_ENABLE,
+ port->sgmii_base + AM65_CPSW_SGMII_CONTROL_REG);
cpsw_sl_ctl_set(port->slave.mac_sl, CPSW_SL_CTL_EXT_EN);
} else {
cpsw_sl_ctl_clr(port->slave.mac_sl, CPSW_SL_CTL_EXT_EN);
@@ -2076,8 +2079,6 @@ static void am65_cpsw_nuss_mac_config(struct phylink_config *config, unsigned in
CPSW_SL_CTL_XGIG | CPSW_SL_CTL_XGMII_EN);
}
- writel(AM65_CPSW_SGMII_CONTROL_MR_AN_ENABLE,
- port->sgmii_base + AM65_CPSW_SGMII_CONTROL_REG);
}
}
--
2.34.1
^ permalink raw reply related
* [PATCH 3/3] arm64: dts: ti: k3-j784s4: Add overlay for dual port USXGMII+SGMII mode
From: Gokul Praveen @ 2026-06-25 12:08 UTC (permalink / raw)
To: vigneshr, kristo, conor+dt, c-vankar, s-vadapalli, krzk+dt, davem,
andrew+netdev, pabeni, kuba, g-praveen, devicetree, linux-kernel
In-Reply-To: <20260625120805.28948-1-g-praveen@ti.com>
The CPSW9G instance of the CPSW Ethernet Switch supports SGMII and
USXGMII mode with MAC Ports 1 and 2 of the instance, which are
connected to ENET Expansion 1 and ENET Expansion 2 slots on
the EVM respectively, through the Serdes2 instance of the SERDES.
Enable CPSW9G MAC Ports 1 and 2 in fixed-link configuration
SGMII and USXGMII mode at 1Gbps and 5 Gbps each.
Signed-off-by: Gokul Praveen <g-praveen@ti.com>
---
arch/arm64/boot/dts/ti/Makefile | 11 ++-
...s4-evm-sgmii-usxgmii-exp1-exp2_board1.dtso | 93 ++++++++++++++++++
...s4-evm-usxgmii-sgmii-exp1-exp2_board2.dtso | 96 +++++++++++++++++++
3 files changed, 198 insertions(+), 2 deletions(-)
create mode 100644 arch/arm64/boot/dts/ti/k3-j784s4-evm-sgmii-usxgmii-exp1-exp2_board1.dtso
create mode 100644 arch/arm64/boot/dts/ti/k3-j784s4-evm-usxgmii-sgmii-exp1-exp2_board2.dtso
diff --git a/arch/arm64/boot/dts/ti/Makefile b/arch/arm64/boot/dts/ti/Makefile
index 371f9a043fe5..253706415833 100644
--- a/arch/arm64/boot/dts/ti/Makefile
+++ b/arch/arm64/boot/dts/ti/Makefile
@@ -199,6 +199,8 @@ dtb-$(CONFIG_ARCH_K3) += k3-j784s4-evm-pcie0-pcie1-ep.dtbo
dtb-$(CONFIG_ARCH_K3) += k3-j784s4-evm-quad-port-eth-exp1.dtbo
dtb-$(CONFIG_ARCH_K3) += k3-j784s4-evm-usxgmii-exp1-exp2.dtbo
dtb-$(CONFIG_ARCH_K3) += k3-j784s4-j742s2-evm-usb0-type-a.dtbo
+dtb-$(CONFIG_ARCH_K3) += k3-j784s4-evm-sgmii-usxgmii-exp1-exp2_board1.dtbo
+dtb-$(CONFIG_ARCH_K3) += k3-j784s4-evm-usxgmii-sgmii-exp1-exp2_board2.dtbo
# Boards with J742S2 SoC
dtb-$(CONFIG_ARCH_K3) += k3-j742s2-evm.dtb
@@ -338,6 +340,10 @@ k3-j784s4-evm-usb0-type-a-dtbs := k3-j784s4-evm.dtb \
k3-j784s4-j742s2-evm-usb0-type-a.dtbo
k3-j784s4-evm-usxgmii-exp1-exp2-dtbs := k3-j784s4-evm.dtb \
k3-j784s4-evm-usxgmii-exp1-exp2.dtbo
+k3-j784s4-evm-sgmii-usxgmii-exp1-exp2_board1-dtbs := k3-j784s4-evm.dtb \
+ k3-j784s4-evm-sgmii-usxgmii-exp1-exp2_board1.dtbo
+k3-j784s4-evm-usxgmii-sgmii-exp1-exp2_board2-dtbs := k3-j784s4-evm.dtb \
+ k3-j784s4-evm-usxgmii-sgmii-exp1-exp2_board2.dtbo
dtb- += k3-am625-beagleplay-csi2-ov5640.dtb \
k3-am625-beagleplay-csi2-tevi-ov5640.dtb \
k3-am625-phyboard-lyra-disable-eth-phy.dtb \
@@ -404,7 +410,8 @@ dtb- += k3-am625-beagleplay-csi2-ov5640.dtb \
k3-j784s4-evm-pcie0-pcie1-ep.dtb \
k3-j784s4-evm-quad-port-eth-exp1.dtb \
k3-j784s4-evm-usb0-type-a.dtb \
- k3-j784s4-evm-usxgmii-exp1-exp2.dtb
-
+ k3-j784s4-evm-usxgmii-exp1-exp2.dtb \
+ k3-j784s4-evm-usxgmii-sgmii-exp1-exp2_board2.dtb \
+ k3-j784s4-evm-sgmii-usxgmii-exp1-exp2_board1.dtb \
# Enable support for device-tree overlays
DTC_FLAGS := -@
diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-evm-sgmii-usxgmii-exp1-exp2_board1.dtso b/arch/arm64/boot/dts/ti/k3-j784s4-evm-sgmii-usxgmii-exp1-exp2_board1.dtso
new file mode 100644
index 000000000000..553546b281a9
--- /dev/null
+++ b/arch/arm64/boot/dts/ti/k3-j784s4-evm-sgmii-usxgmii-exp1-exp2_board1.dtso
@@ -0,0 +1,93 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR MIT */
+/**
+ * DT Overlay for CPSW9G in dual port fixed-link USXGMII mode using ENET-1
+ * and ENET-2 Expansion slots of J784S4 EVM.
+ *
+ * Copyright (C) 2024 Texas Instruments Incorporated - https://www.ti.com/
+ */
+
+/dts-v1/;
+/plugin/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/phy/phy-cadence.h>
+#include <dt-bindings/phy/phy.h>
+
+#include "k3-serdes.h"
+
+&{/} {
+ aliases {
+ ethernet1 = "/bus@100000/ethernet@c000000/ethernet-ports/port@1";
+ ethernet2 = "/bus@100000/ethernet@c000000/ethernet-ports/port@2";
+ ethernet3 = "/bus@100000/ethernet@c200000/ethernet-ports/port@1";
+ };
+};
+
+&main_cpsw0 {
+ pinctrl-names = "default";
+ status = "okay";
+};
+
+&main_cpsw0_port1 {
+ phy-mode = "sgmii";
+ mac-address = [00 00 00 00 00 00];
+ phys = <&cpsw0_phy_gmii_sel 1>, <&serdes2_sgmii2_link>;
+ phy-names = "mac", "serdes";
+ status = "okay";
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+};
+
+&main_cpsw0_port2 {
+ phy-mode = "usxgmii";
+ mac-address = [00 00 00 00 00 00];
+ phys = <&cpsw0_phy_gmii_sel 2>, <&serdes2_usxgmii_link>;
+ phy-names = "mac", "serdes";
+ status = "okay";
+ fixed-link {
+ speed = <5000>;
+ full-duplex;
+ };
+};
+
+&serdes_wiz2 {
+ assigned-clocks = <&k3_clks 406 6>, <&k3_clks 406 5>;
+ //assigned-clock-parents = <&k3_clks 406 9>; /* Use 100 MHz clock for SGMII */
+ status = "okay";
+};
+
+&serdes2 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "okay";
+
+ assigned-clock-parents = <&k3_clks 406 5>,
+ <&k3_clks 406 6>,
+ <&k3_clks 406 5>;
+
+ serdes2_sgmii2_link: phy@1 {
+ reg = <2>;
+ cdns,num-lanes = <1>;
+ #phy-cells = <0>;
+ cdns,phy-type = <PHY_TYPE_SGMII>;
+ resets = <&serdes_wiz2 3>;
+ };
+ serdes2_usxgmii_link: phy@0 {
+ reg = <3>;
+ cdns,num-lanes = <1>;
+ #phy-cells = <0>;
+ cdns,phy-type = <PHY_TYPE_USXGMII>;
+ resets = <&serdes_wiz2 4>;
+ };
+};
+
+&serdes_ln_ctrl {
+ idle-states = <J784S4_SERDES0_LANE0_PCIE1_LANE0>, <J784S4_SERDES0_LANE1_PCIE1_LANE1>,
+ <J784S4_SERDES0_LANE2_IP3_UNUSED>, <J784S4_SERDES0_LANE3_USB>,
+ <J784S4_SERDES1_LANE0_PCIE0_LANE0>, <J784S4_SERDES1_LANE1_PCIE0_LANE1>,
+ <J784S4_SERDES1_LANE2_PCIE0_LANE2>, <J784S4_SERDES1_LANE3_PCIE0_LANE3>,
+ <J784S4_SERDES2_LANE0_IP2_UNUSED>, <J784S4_SERDES2_LANE1_IP2_UNUSED>,
+ <J784S4_SERDES2_LANE2_QSGMII_LANE1>, <J784S4_SERDES2_LANE3_QSGMII_LANE2>;
+};
diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-evm-usxgmii-sgmii-exp1-exp2_board2.dtso b/arch/arm64/boot/dts/ti/k3-j784s4-evm-usxgmii-sgmii-exp1-exp2_board2.dtso
new file mode 100644
index 000000000000..c98be3bbf2c8
--- /dev/null
+++ b/arch/arm64/boot/dts/ti/k3-j784s4-evm-usxgmii-sgmii-exp1-exp2_board2.dtso
@@ -0,0 +1,96 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR MIT */
+/**
+ * DT Overlay for CPSW9G in dual port fixed-link USXGMII mode using ENET-1
+ * and ENET-2 Expansion slots of J784S4 EVM.
+ *
+ * Copyright (C) 2024 Texas Instruments Incorporated - https://www.ti.com/
+ */
+
+/dts-v1/;
+/plugin/;
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/phy/phy-cadence.h>
+#include <dt-bindings/phy/phy.h>
+
+#include "k3-serdes.h"
+
+&{/} {
+ aliases {
+ ethernet1 = "/bus@100000/ethernet@c000000/ethernet-ports/port@1";
+ ethernet2 = "/bus@100000/ethernet@c000000/ethernet-ports/port@2";
+ ethernet3 = "/bus@100000/ethernet@c200000/ethernet-ports/port@1";
+ };
+};
+
+&main_cpsw0 {
+ pinctrl-names = "default";
+ status = "okay";
+};
+
+&main_cpsw0_port1 {
+ phy-mode = "usxgmii";
+ mac-address = [00 00 00 00 00 00];
+ phys = <&cpsw0_phy_gmii_sel 1>, <&serdes2_usxgmii_link>;
+ phy-names = "mac", "serdes";
+ status = "okay";
+ fixed-link {
+ speed = <5000>;
+ full-duplex;
+ };
+};
+
+&main_cpsw0_port2 {
+ phy-mode = "sgmii";
+ mac-address = [00 00 00 00 00 00];
+ phys = <&cpsw0_phy_gmii_sel 2>, <&serdes2_sgmii2_link>;
+ phy-names = "mac", "serdes";
+ status = "okay";
+ fixed-link {
+ speed = <1000>;
+ full-duplex;
+ };
+};
+
+&serdes_wiz2 {
+ assigned-clocks = <&k3_clks 406 6>, <&k3_clks 406 5>;
+ //assigned-clock-parents = <&k3_clks 406 9>; /* Use 156.25 MHz clock for USXGMII */
+ status = "okay";
+};
+
+&serdes2 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ status = "okay";
+
+ assigned-clock-parents = <&k3_clks 406 5>,
+ <&k3_clks 406 6>,
+ <&k3_clks 406 5>;
+
+
+
+ serdes2_sgmii2_link: phy@1 {
+ reg = <3>;
+ cdns,num-lanes = <1>;
+ #phy-cells = <0>;
+ cdns,phy-type = <PHY_TYPE_SGMII>;
+ resets = <&serdes_wiz2 4>;
+ };
+ serdes2_usxgmii_link: phy@0 {
+ reg = <2>;
+ cdns,num-lanes = <1>;
+ #phy-cells = <0>;
+ cdns,phy-type = <PHY_TYPE_USXGMII>;
+ resets = <&serdes_wiz2 3>;
+ };
+
+};
+
+&serdes_ln_ctrl {
+ idle-states = <J784S4_SERDES0_LANE0_PCIE1_LANE0>, <J784S4_SERDES0_LANE1_PCIE1_LANE1>,
+ <J784S4_SERDES0_LANE2_IP3_UNUSED>, <J784S4_SERDES0_LANE3_USB>,
+ <J784S4_SERDES1_LANE0_PCIE0_LANE0>, <J784S4_SERDES1_LANE1_PCIE0_LANE1>,
+ <J784S4_SERDES1_LANE2_PCIE0_LANE2>, <J784S4_SERDES1_LANE3_PCIE0_LANE3>,
+ <J784S4_SERDES2_LANE0_IP2_UNUSED>, <J784S4_SERDES2_LANE1_IP2_UNUSED>,
+ <J784S4_SERDES2_LANE2_QSGMII_LANE1>, <J784S4_SERDES2_LANE3_QSGMII_LANE2>;
+};
--
2.34.1
^ permalink raw reply related
* [PATCH 0/3] Add support for dual port USXGMII+SGMII
From: Gokul Praveen @ 2026-06-25 12:08 UTC (permalink / raw)
To: vigneshr, kristo, conor+dt, c-vankar, s-vadapalli, krzk+dt, davem,
andrew+netdev, pabeni, kuba, g-praveen, devicetree, linux-kernel
This patch series enables dual port USXGMII and SGMII Ethernet mode
support on the TI J784S4 EVM platform using the CPSW9G Ethernet switch.
The CPSW9G instance supports both SGMII and USXGMII modes simultaneously
on MAC Ports 1 and 2, which connect to ENET Expansion 1 and ENET
Expansion 2 slots through the SERDES2 instance. The series includes:
1. A fix to the SERDES2 node to add the missing PLL1 refclk, which is
required for multilink SERDES configuration to function properly.
2. A driver fix for the am65-cpsw Ethernet driver to properly configure
the MAC in SGMII mode according to the CPSGMII Module specification,
including correct advertisement ability register settings and master
mode configuration.
3. Device tree overlay support for two board configurations:
- Board 1: Port 1 in SGMII mode (1Gbps) + Port 2 in USXGMII mode (5Gbps)
- Board 2: Port 1 in USXGMII mode (5Gbps) + Port 2 in SGMII mode (1Gbps)
Both configurations use fixed-link mode and provide flexible multi-speed
networking options for the J784S4 platform.
Boot logs:
https://gist.github.com/GokulPraveen2001/7a09cc5ed0151d264f32cbbc31896605
Gokul Praveen (3):
arm64: dts: ti: Add PLL1 refclk to J784S4 SoC SERDES node
net: ethernet: ti: am65-cpsw: Fix MAC configuration for SGMII mode
arm64: dts: ti: k3-j784s4: Add overlay for dual port USXGMII+SGMII
mode
arch/arm64/boot/dts/ti/Makefile | 11 ++-
...s4-evm-sgmii-usxgmii-exp1-exp2_board1.dtso | 97 +++++++++++++++++++
...s4-evm-usxgmii-sgmii-exp1-exp2_board2.dtso | 97 +++++++++++++++++++
arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi | 3 +-
drivers/net/ethernet/ti/am65-cpsw-nuss.c | 7 +-
5 files changed, 209 insertions(+), 6 deletions(-)
create mode 100644 arch/arm64/boot/dts/ti/k3-j784s4-evm-sgmii-usxgmii-exp1-exp2_board1.dtso
create mode 100644 arch/arm64/boot/dts/ti/k3-j784s4-evm-usxgmii-sgmii-exp1-exp2_board2.dtso
--
2.34.1
^ permalink raw reply
* Re: [PATCH 4/6] clk: qcom: Add Nord display clock controller support
From: Konrad Dybcio @ 2026-06-25 12:06 UTC (permalink / raw)
To: Taniya Das, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Brian Masney, Shawn Guo, Bartosz Golaszewski, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong, Konrad Dybcio
Cc: Ajit Pandey, Imran Shaik, Jagadeesh Kona, linux-arm-msm,
linux-clk, linux-kernel, devicetree
In-Reply-To: <20260623-nords_mm_v1-v1-4-860c84539804@oss.qualcomm.com>
On 6/23/26 12:54 PM, Taniya Das wrote:
> Add support for the display clock controllers (DISPCC) on the
> Qualcomm Nord platform.
>
> The platform includes two display clock controller instances,
> display0 and display1. Register support for both controllers.
>
> Signed-off-by: Taniya Das <taniya.das@oss.qualcomm.com>
> ---
[...]
> +enum {
> + DT_BI_TCXO,
> + DT_BI_TCXO_AO,
This one can probably be removed, both files
I think the rest looks good
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* [PATCH v3 2/3] arm64: dts: qcom: sm8250: Add cam_mclk3_default pinctrl
From: Xin Xu @ 2026-06-25 12:05 UTC (permalink / raw)
To: konrad.dybcio, andersson, konradybcio
Cc: linux-arm-msm, devicetree, linux-kernel, Xin Xu
In-Reply-To: <tencent_D4AB219A942591742C3A6DA4DFB0EBA49505@qq.com>
Add a default pin configuration for CAM_MCLK3 (GPIO97), which is used
as a master clock for camera sensors on SM8250 boards.
Signed-off-by: Xin Xu <xxsemail@qq.com>
---
arch/arm64/boot/dts/qcom/sm8250.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8250.dtsi b/arch/arm64/boot/dts/qcom/sm8250.dtsi
index 7076720413ab..05e765616112 100644
--- a/arch/arm64/boot/dts/qcom/sm8250.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250.dtsi
@@ -5165,6 +5165,13 @@ mclk-pins {
};
};
+ cam_mclk3_default: cam-mclk3-default-state {
+ pins = "gpio97";
+ function = "cam_mclk";
+ drive-strength = <4>;
+ bias-disable;
+ };
+
cci0_default: cci0-default-state {
cci0_i2c0_default: cci0-i2c0-default-pins {
/* SDA, SCL */
--
2.53.0
^ permalink raw reply related
* [PATCH v3 1/3] arm64: dts: qcom: sm8250-xiaomi-elish: Add pm8008 PMIC
From: Xin Xu @ 2026-06-25 11:50 UTC (permalink / raw)
To: konrad.dybcio, andersson, konradybcio
Cc: linux-arm-msm, devicetree, linux-kernel, Xin Xu
Add the pm8008 PMIC node on i2c15 with seven LDOs,
using GPIO84 as interrupt and GPIO76 as reset.
Signed-off-by: Xin Xu <xxsemail@qq.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
Changes in v3:
- Split pinctrl into two separate states for interrupt and reset, drop
input-enable.
- Reorder bias properties after drive-strength.
Changes in v2:
- Fix coding style (blank line, interrupts-extended, property order,
drop output-high)
- Correct voltage constraints for ldo1 and ldo2
.../dts/qcom/sm8250-xiaomi-elish-common.dtsi | 90 +++++++++++++++++++
1 file changed, 90 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sm8250-xiaomi-elish-common.dtsi b/arch/arm64/boot/dts/qcom/sm8250-xiaomi-elish-common.dtsi
index 51b57c697a75..05c366eaeff1 100644
--- a/arch/arm64/boot/dts/qcom/sm8250-xiaomi-elish-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/sm8250-xiaomi-elish-common.dtsi
@@ -599,6 +599,82 @@ fuel-gauge@55 {
};
};
+&i2c15 {
+ clock-frequency = <400000>;
+
+ status = "okay";
+
+ pm8008: pmic@8 {
+ compatible = "qcom,pm8008";
+ reg = <0x8>;
+
+ interrupts-extended = <&tlmm 84 IRQ_TYPE_EDGE_RISING>;
+ reset-gpios = <&tlmm 76 GPIO_ACTIVE_LOW>;
+
+ vdd-l1-l2-supply = <&vreg_s8c_1p35>;
+ vdd-l3-l4-supply = <&vreg_bob>;
+ vdd-l5-supply = <&vreg_bob>;
+ vdd-l6-supply = <&vreg_bob>;
+ vdd-l7-supply = <&vreg_bob>;
+
+ pinctrl-0 = <&pm8008_int_default>, <&pm8008_reset_default>;
+ pinctrl-names = "default";
+
+ gpio-controller;
+ #gpio-cells = <2>;
+ gpio-ranges = <&pm8008 0 0 2>;
+
+ interrupt-controller;
+ #interrupt-cells = <2>;
+
+ #thermal-sensor-cells = <0>;
+
+ regulators {
+ vreg_l1p: ldo1 {
+ regulator-name = "vreg_l1p";
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ };
+
+ vreg_l2p: ldo2 {
+ regulator-name = "vreg_l2p";
+ regulator-min-microvolt = <1200000>;
+ regulator-max-microvolt = <1200000>;
+ };
+
+ vreg_l3p: ldo3 {
+ regulator-name = "vreg_l3p";
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ };
+
+ vreg_l4p: ldo4 {
+ regulator-name = "vreg_l4p";
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ };
+
+ vreg_l5p: ldo5 {
+ regulator-name = "vreg_l5p";
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2800000>;
+ };
+
+ vreg_l6p: ldo6 {
+ regulator-name = "vreg_l6p";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <1800000>;
+ };
+
+ vreg_l7p: ldo7 {
+ regulator-name = "vreg_l7p";
+ regulator-min-microvolt = <2800000>;
+ regulator-max-microvolt = <2900000>;
+ };
+ };
+ };
+};
+
&mdss {
status = "okay";
};
@@ -801,6 +877,20 @@ bt_en_state: bt-default-state {
bias-pull-up;
};
+ pm8008_int_default: pm8008-int-default-state {
+ pins = "gpio84";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
+ pm8008_reset_default: pm8008-reset-default-state {
+ pins = "gpio76";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-pull-up;
+ };
+
wlan_en_state: wlan-default-state {
pins = "gpio20";
function = "gpio";
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v4 2/9] usb: misc: qcom_eud: add sysfs attribute for port selection
From: Konrad Dybcio @ 2026-06-25 11:51 UTC (permalink / raw)
To: Elson Serrao, Bjorn Andersson, Dmitry Baryshkov,
Greg Kroah-Hartman, Konrad Dybcio, Krzysztof Kozlowski,
Rob Herring, Conor Dooley, Souradeep Chowdhury
Cc: linux-arm-msm, devicetree, linux-usb, linux-kernel
In-Reply-To: <20260501170635.2641748-3-elson.serrao@oss.qualcomm.com>
On 5/1/26 7:06 PM, Elson Serrao wrote:
> EUD can be mapped to either the primary USB port or the secondary USB port
> depending on the value of the EUD_PORT_SEL register. Add a 'port' sysfs
> attribute to allow userspace to select which port EUD should operate on
> and update the ABI documentation. This is needed for systems with dual
> USB ports where EUD needs to be accessible on either port depending on the
> system configuration and use case.
>
> Signed-off-by: Elson Serrao <elson.serrao@oss.qualcomm.com>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Konrad
^ permalink raw reply
* Re: [PATCH v1 4/8] arm64: dts: qcom: shikra: Add soundwire and macro nodes
From: Konrad Dybcio @ 2026-06-25 11:45 UTC (permalink / raw)
To: Mohammad Rafi Shaik, Srinivas Kandagatla, Bjorn Andersson,
Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-sound, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20260616201315.2565115-5-mohammad.rafi.shaik@oss.qualcomm.com>
On 6/16/26 10:13 PM, Mohammad Rafi Shaik wrote:
> Add SoC-level SoundWire masters and LPASS RX/VA macro nodes, along with
> DMIC and SWR pinctrl states required by the audio data path.
>
> Keep these nodes disabled in shikra.dtsi so board dts files can selectively
> enable and configure them.
>
> Signed-off-by: Mohammad Rafi Shaik <mohammad.rafi.shaik@oss.qualcomm.com>
> ---
Just a couple style nits:
[...]
> + rxmacro: codec@a040000 {
> + compatible = "qcom,shikra-lpass-rx-macro";
> + reg = <0x0 0x0a040000 0x0 0x1000>;
> +
> + pinctrl-0 = <&rx_swr_active>;
> + pinctrl-names = "default";
Let's move this down.. maybe above all the #cells
> +
> + clocks = <&audiocorecc AUDIO_CORE_CC_RX_MCLK_CLK>,
> + <&audiocorecc AUDIO_CORE_CC_RX_MCLK_2X_CLK>,
Please align all <s
> + <&vamacro>;
> + clock-names = "mclk",
> + "npl",
> + "fsgen";
> +
> + #clock-cells = <0>;
> + clock-output-names = "mclk";
> + #sound-dai-cells = <1>;
> + status = "disabled";
Let's keep a \n before 'status', everywhere
Konrad
^ permalink raw reply
* Re: [PATCH 4/5] media: i2c: vd55g1: Add support for vd55g4
From: Benjamin Mugnier @ 2026-06-25 11:41 UTC (permalink / raw)
To: Jacopo Mondi
Cc: Sylvain Petinot, Sakari Ailus, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Hans Verkuil, linux-media,
linux-kernel, devicetree
In-Reply-To: <ajkKkSeDNoijIsub@zed>
Hi Jacopo,
Le 22/06/2026 à 12:16, Jacopo Mondi a écrit :
> Hi Benjamin
>
> On Tue, Apr 28, 2026 at 10:40:58AM +0200, Benjamin Mugnier wrote:
>> vd55g4 is the same device as vd65g4 but outputs in monochrome instead of
>> RGB. Adapt the driver structure according to this new variant, and add
>> its support.
>>
>> Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
>> ---
>> drivers/media/i2c/vd55g1.c | 110 ++++++++++++++++++++++++++++++---------------
>> 1 file changed, 74 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/media/i2c/vd55g1.c b/drivers/media/i2c/vd55g1.c
>> index 2c962fcb41d2..9f62fc0428a1 100644
>> --- a/drivers/media/i2c/vd55g1.c
>> +++ b/drivers/media/i2c/vd55g1.c
>> @@ -29,11 +29,7 @@
>>
>> /* Register Map */
>> #define VD55G1_REG_MODEL_ID CCI_REG32_LE(0x0000)
>> -#define VD55G1_MODEL_ID_VD55G1 0x53354731 /* Mono */
>> -#define VD55G1_MODEL_ID_VD65G4 0x53354733 /* RGB */
>> -#define VD55G1_REG_REVISION CCI_REG16_LE(0x0004)
>> -#define VD55G1_REVISION_CCB 0x2020
>> -#define VD55G1_REVISION_BAYER 0x3030
>> +#define VD55G1_REG_COLOR_VERSION CCI_REG32_LE(0x0670)
>> #define VD55G1_REG_FWPATCH_REVISION CCI_REG16_LE(0x0012)
>> #define VD55G1_REG_FWPATCH_START_ADDR CCI_REG8(0x2000)
>> #define VD55G1_REG_SYSTEM_FSM CCI_REG8(0x001c)
>> @@ -138,8 +134,39 @@
>> #define VD55G1_MIPI_RATE_MIN (250 * MEGA)
>> #define VD55G1_MIPI_RATE_MAX (1200 * MEGA)
>>
>> -#define VD55G1_MODEL_ID_NAME(id) \
>> - ((id) == VD55G1_MODEL_ID_VD55G1 ? "vd55g1" : "vd65g4")
>> +enum vd55g1_model_id {
>> + VD55G1_MODEL_ID_2 = 0x53354731,
>> + VD55G1_MODEL_ID_3 = 0x53354733,
>> +};
>> +
>> +enum vd55g1_color_version {
>> + VD55G1_COLOR_VERSION_MONO = 0x0,
>> + VD55G1_COLOR_VERSION_BAYER = 0x1,
>
> nit: you don't need to initialize the enum members here
Thank you, will do.
>
>> +};
>> +
>> +struct vd55g1_version {
>> + char *name;
>> + enum vd55g1_model_id id;
>> + enum vd55g1_color_version color;
>> +};
>> +
>> +static const struct vd55g1_version vd55g1_versions[] = {
>> + {
>> + .name = "vd55g1",
>> + .id = VD55G1_MODEL_ID_2,
>> + .color = VD55G1_COLOR_VERSION_MONO,
>> + },
>> + {
>> + .name = "vd55g4",
>> + .id = VD55G1_MODEL_ID_3,
>> + .color = VD55G1_COLOR_VERSION_MONO,
>> + },
>> + {
>> + .name = "vd65g4",
>> + .id = VD55G1_MODEL_ID_3,
>> + .color = VD55G1_COLOR_VERSION_BAYER,
>> + },
>> +};
>>
>> static const u8 vd55g1_patch_array[] = {
>> 0x44, 0x03, 0x09, 0x02, 0xe6, 0x01, 0x42, 0x00, 0xea, 0x01, 0x42, 0x00,
>> @@ -535,7 +562,7 @@ struct vd55g1_vblank_limits {
>>
>> struct vd55g1 {
>> struct device *dev;
>> - unsigned int id;
>> + const struct vd55g1_version *version;
>> struct v4l2_subdev sd;
>> struct media_pad pad;
>> struct regulator_bulk_data supplies[ARRAY_SIZE(vd55g1_supply_name)];
>> @@ -628,7 +655,7 @@ static u32 vd55g1_get_fmt_code(struct vd55g1 *sensor, u32 code)
>> {
>> unsigned int i, j;
>>
>> - if (sensor->id == VD55G1_MODEL_ID_VD55G1)
>> + if (sensor->version->color != VD55G1_COLOR_VERSION_BAYER)
>> return code;
>
> As pointed out in the previous patch, you seem to have 2 mono formats.
> Is this still ok ?
>
>>
>> for (i = 0; i < ARRAY_SIZE(vd55g1_mbus_formats_bayer); i++) {
>> @@ -1183,8 +1210,8 @@ static int vd55g1_patch(struct vd55g1 *sensor)
>> u64 patch;
>> int ret = 0;
>>
>> - /* vd55g1 needs a patch while vd65g4 does not */
>> - if (sensor->id == VD55G1_MODEL_ID_VD55G1) {
>> + /* Version 2 needs a patch while version 3 does not */
>> + if (sensor->version->id == VD55G1_MODEL_ID_2) {
>> vd55g1_write_array(sensor, VD55G1_REG_FWPATCH_START_ADDR,
>> sizeof(vd55g1_patch_array),
>> vd55g1_patch_array, &ret);
>
> You might want to consider renaming vd55g1_patch_array ?
It doesn't really patch an array, it writes an array of values into
continuous i2c registers. Just like before converting to cci_write() I
used to have a vd55g1_write() function to write a register, I derived a
vd55g1_write_array() function. Here it happens to be a firmware patch,
but it could be anything you want ;)
>
>> @@ -1256,7 +1283,7 @@ static int vd55g1_enum_mbus_code(struct v4l2_subdev *sd,
>> struct vd55g1 *sensor = to_vd55g1(sd);
>> u32 base_code;
>>
>> - if (sensor->id == VD55G1_MODEL_ID_VD55G1) {
>> + if (sensor->version->color != VD55G1_COLOR_VERSION_BAYER) {
>> if (code->index >= ARRAY_SIZE(vd55g1_mbus_formats_mono))
>> return -EINVAL;
>> base_code = vd55g1_mbus_formats_mono[code->index];
>> @@ -1372,7 +1399,7 @@ static int vd55g1_init_state(struct v4l2_subdev *sd,
>> if (ret)
>> return ret;
>>
>> - if (sensor->id == VD55G1_MODEL_ID_VD55G1)
>> + if (sensor->version->color != VD55G1_COLOR_VERSION_BAYER)
>> code = vd55g1_mbus_formats_mono[VD55G1_MBUS_CODE_IDX_DEF];
>> else
>> code = vd55g1_mbus_formats_bayer[VD55G1_MBUS_CODE_IDX_DEF][0];
>> @@ -1659,38 +1686,48 @@ static int vd55g1_init_ctrls(struct vd55g1 *sensor)
>> return ret;
>> }
>>
>> +static const struct vd55g1_version *
>> + vd55g1_get_version(enum vd55g1_model_id id,
>> + enum vd55g1_color_version color)
>
> Should you indent one tab left ?
>
checkpatch.pl is fine with both. I can indent left, it looks cleaner.
>> +{
>> + unsigned int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(vd55g1_versions); i++) {
>
> You can declare i inside the for loop
>
Yes, thank you.
>> + if (vd55g1_versions[i].id == id &&
>> + vd55g1_versions[i].color == color)
>> + return &vd55g1_versions[i];
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> static int vd55g1_detect(struct vd55g1 *sensor)
>> {
>> - unsigned int dt_id = (uintptr_t)device_get_match_data(sensor->dev);
>> - u64 rev, id;
>> - int ret;
>> + const struct vd55g1_version *dt_version =
>> + device_get_match_data(sensor->dev);
>> + const struct vd55g1_version *version;
>> + u64 color, id;
>> + int ret = 0;
>>
>> - ret = vd55g1_read(sensor, VD55G1_REG_MODEL_ID, &id, NULL);
>> + vd55g1_read(sensor, VD55G1_REG_MODEL_ID, &id, &ret);
>> + vd55g1_read(sensor, VD55G1_REG_COLOR_VERSION, &color, &ret);
>> if (ret)
>> return ret;
>>
>> - if (id != VD55G1_MODEL_ID_VD55G1 && id != VD55G1_MODEL_ID_VD65G4) {
>> - dev_warn(sensor->dev, "Unsupported sensor id 0x%x\n",
>> - (u32)id);
>> + version = vd55g1_get_version(id, color);
>> + if (!version) {
>> + dev_warn(sensor->dev, "Unsupported sensor version, expected %s\n",
>> + dt_version->name);
>> return -ENODEV;
>> }
>> - if (id != dt_id) {
>> - dev_err(sensor->dev, "Probed sensor %s and device tree definition (%s) mismatch",
>> - VD55G1_MODEL_ID_NAME(id), VD55G1_MODEL_ID_NAME(dt_id));
>> + if (version->id != dt_version->id ||
>> + version->color != dt_version->color) {
>> + dev_err(sensor->dev, "Probed sensor version %s and device tree definition %s mismatch",
>> + version->name, dt_version->name);
>> return -ENODEV;
>> }
>> - sensor->id = id;
>>
>> - ret = vd55g1_read(sensor, VD55G1_REG_REVISION, &rev, NULL);
>> - if (ret)
>> - return ret;
>> -
>> - if ((id == VD55G1_MODEL_ID_VD55G1 && rev != VD55G1_REVISION_CCB) &&
>> - (id == VD55G1_MODEL_ID_VD65G4 && rev != VD55G1_REVISION_BAYER)) {
>> - dev_err(sensor->dev, "Unsupported sensor revision 0x%x for sensor %s\n",
>> - (u16)rev, VD55G1_MODEL_ID_NAME(id));
>> - return -ENODEV;
>> - }
>> + sensor->version = version;
>>
>> return 0;
>> }
>> @@ -2048,8 +2085,9 @@ static void vd55g1_remove(struct i2c_client *client)
>> }
>>
>> static const struct of_device_id vd55g1_dt_ids[] = {
>> - { .compatible = "st,vd55g1", .data = (void *)VD55G1_MODEL_ID_VD55G1 },
>> - { .compatible = "st,vd65g4", .data = (void *)VD55G1_MODEL_ID_VD65G4 },
>> + { .compatible = "st,vd55g1", .data = (void *)&vd55g1_versions[0] },
>> + { .compatible = "st,vd55g4", .data = (void *)&vd55g1_versions[1] },
>> + { .compatible = "st,vd65g4", .data = (void *)&vd55g1_versions[2] },
>> { /* sentinel */ }
>> };
>
> All minors
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>
> Thanks
> j
>
>> MODULE_DEVICE_TABLE(of, vd55g1_dt_ids);
>>
>> --
>> 2.43.0
>>
>>
--
Regards,
Benjamin
^ permalink raw reply
* Re: [PATCH 1/5] media: i2c: vd55g1: Fix media bus code initialization
From: Benjamin Mugnier @ 2026-06-25 11:41 UTC (permalink / raw)
To: Jacopo Mondi
Cc: Sylvain Petinot, Sakari Ailus, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Hans Verkuil, linux-media,
linux-kernel, devicetree
In-Reply-To: <ajj90hhNwx7bLkOZ@zed>
Hi Jacopo,
Thank you for your review.
Le 22/06/2026 à 11:28, Jacopo Mondi a écrit :
> Hi Benjamin
>
> On Tue, Apr 28, 2026 at 10:40:55AM +0200, Benjamin Mugnier wrote:
>> In the driver initialization, the index of the default media bus code
>> from the supported media bus code array is passed directly to the
>> vd55g1_get_fmt_code() function instead of the proper media bus code.
>>
>> This works correctly as a proper media bus code is set after
>> initialization but could not have been the case. This also resulted in
>> mutliple "Unsupported mbus format" error messages.
>>
>> Retrieve the media bus code from the media bus code array, and pass this
>> media bus code to vd55g1_get_fmt_code() instead of the code index.
>>
>> Rename VD55G1_MBUS_CODE_DEF to VD55G1_MBUS_CODE_IDX_DEF and
>> VD55G1_MODE_DEF to VD55G1_MODE_IDX_DEF while at it to avoid future
>> confusions. Display the guilty error code in warning message.
>>
>> Fixes: e138e7f00042 ("media: i2c: vd55g1: Add support for vd65g4 RGB variant")
>>
> You should cc stable for fixes
>
> Cc: stable@vger.kernel.org
>
We talked about this very recently and somehow I still forgot.
>
> The CI should have flagged that, but for some reason it didn't run
> properly on your series
> https://gitlab.freedesktop.org/linux-media/users/patchwork/-/pipelines/1655147
>
>> Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
>> ---
>> drivers/media/i2c/vd55g1.c | 17 +++++++++++------
>> 1 file changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/media/i2c/vd55g1.c b/drivers/media/i2c/vd55g1.c
>> index 78d18c028154..1e9db21322e3 100644
>> --- a/drivers/media/i2c/vd55g1.c
>> +++ b/drivers/media/i2c/vd55g1.c
>> @@ -114,9 +114,9 @@
>>
>> #define VD55G1_WIDTH 804
>> #define VD55G1_HEIGHT 704
>> -#define VD55G1_MODE_DEF 0
>> +#define VD55G1_MODE_IDX_DEF 0
>> #define VD55G1_NB_GPIOS 4
>> -#define VD55G1_MBUS_CODE_DEF 0
>> +#define VD55G1_MBUS_CODE_IDX_DEF 0
>> #define VD55G1_DGAIN_DEF 256
>> #define VD55G1_AGAIN_DEF 19
>> #define VD55G1_EXPO_MAX_TERM 64
>> @@ -634,7 +634,7 @@ static u32 vd55g1_get_fmt_code(struct vd55g1 *sensor, u32 code)
>
> Unrelated, but it seems you now have 2 codes for MONO. Does
>
> if (sensor->id == VD55G1_MODEL_ID_VD55G1)
> return code;
>
> need an update ?>
Not in this patch because it does not add the new MONO sensor, but in
4/5 I separated the model ID from the color code. Example for the vd55g4 :
.name = "vd55g4",
.id = VD55G1_MODEL_ID_3,
.color = VD55G1_COLOR_VERSION_MONO,
So the patch 4/5 updates the previous 'if' you mentioned to check the
color member instead of the model :
if (sensor->version->color != VD55G1_COLOR_VERSION_BAYER)
Which IMO is a good way to handle this problematic. Tell me if you're
thinking about something else.
>> goto adapt_bayer_pattern;
>> }
>> }
>> - dev_warn(sensor->dev, "Unsupported mbus format\n");
>> + dev_warn(sensor->dev, "Unsupported mbus format: 0x%x\n", code);
>>
>> return code;
>>
>> @@ -1347,6 +1347,7 @@ static int vd55g1_init_state(struct v4l2_subdev *sd,
>> {
>> struct vd55g1 *sensor = to_vd55g1(sd);
>> struct v4l2_subdev_format fmt = { 0 };
>> + int code;
>> struct v4l2_subdev_route routes[] = {
>> { .flags = V4L2_SUBDEV_ROUTE_FL_ACTIVE }
>> };
>> @@ -1361,9 +1362,13 @@ static int vd55g1_init_state(struct v4l2_subdev *sd,
>> if (ret)
>> return ret;
>>
>> - vd55g1_update_pad_fmt(sensor, &vd55g1_supported_modes[VD55G1_MODE_DEF],
>> - vd55g1_get_fmt_code(sensor, VD55G1_MBUS_CODE_DEF),
>> - &fmt.format);
>> + if (sensor->id == VD55G1_MODEL_ID_VD55G1)
>> + code = vd55g1_mbus_formats_mono[VD55G1_MBUS_CODE_IDX_DEF];
>> + else
>> + code = vd55g1_mbus_formats_bayer[VD55G1_MBUS_CODE_IDX_DEF][0];
>
> Being this a multi-dimensional array, I don't seem much value in
> defining VD55G1_MBUS_CODE_IDX_DEF if this is the only place where it
> is used. What's the meaning of VD55G1_MBUS_CODE_IDX_DEF for
> vd55g1_mbus_formats_bayer ? Does it represent the bitwidth or does it
> represent the bayer pattern ?
For vd55g1_mbus_formats_bayer, the first dimension of the array is the
bitwidth, and the second one is the bayer pattern.
>
> I would rather define a
> VD55G1_DEF_MBUS_CODE_MONO MEDIA_BUS_FMT_Y8_1X8
> VD55G1_DEF_MBUS_CODE_BAYER MEDIA_BUS_FMT_SRGGB8_1X8
>
> Or maybe do
>
> code = vd55g1_mbus_formats_bayer[VD55G1_MBUS_CODE_IDX_DEF]
> [VD55G1_MBUS_CODE_IDX_DEF];
>
> if easier.
>
> I understand it's a minor, so up to you.
As you mentioned it's only used here. I won't mind removing
VD55G1_MBUS_CODE_IDX_DEF entirely and do :
code = vd55g1_mbus_formats_bayer[0][0];
Does that sound okay ?
>
>
>
>> + vd55g1_update_pad_fmt(sensor,
>> + &vd55g1_supported_modes[VD55G1_MODE_IDX_DEF],
>> + vd55g1_get_fmt_code(sensor, code), &fmt.format);
>>
>> return vd55g1_set_pad_fmt(sd, sd_state, &fmt);
>> }
>>
>> --
>> 2.43.0
>>
>>
--
Regards,
Benjamin
^ permalink raw reply
* Re: [PATCH 2/5] media: i2c: vd55g1: Remove spurious pad format update on init_state()
From: Benjamin Mugnier @ 2026-06-25 11:41 UTC (permalink / raw)
To: Jacopo Mondi
Cc: Sylvain Petinot, Sakari Ailus, Mauro Carvalho Chehab, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Hans Verkuil, linux-media,
linux-kernel, devicetree
In-Reply-To: <ajkAdol1szOK_XPB@zed>
Hi Jacopo,
Le 22/06/2026 à 11:30, Jacopo Mondi a écrit :
> Hi Benjamin
>
> On Tue, Apr 28, 2026 at 10:40:56AM +0200, Benjamin Mugnier wrote:
>> vd55g1_update_pad_fmt() is called in vd55g1_init_state(). But
>> vd55g1_set_pad_fmt(), called at the end of vd55g1_init_state(), also
>> calls vd55g1_update_pad_fmt() itself.
>>
>> Enhance readability and clear confusion by only preparing the format in
>> vd55g1_init_state() and let vd55g1_set_pad_fmt() update it instead,
>> effectively calling it only 1 time instead of 2.
>>
>> Fixes: e138e7f00042 ("media: i2c: vd55g1: Add support for vd65g4 RGB variant")
>
> Does this qualify as a fix ?
That's a good question, indeed this patch does not correct any 'bug' per
se. It's more of a code flow error so I think you're correct. I'm fine
with removing the 'Fixes:' tag.
>
> I think you could maybe squash it with the previous one if you want
> also this change to be backported as part of a larger fix
>
I don't know, this patch solves another issue than the previous patch
does. I'd like to keep it this in the git history if you don't mind.
>>
>> Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>
> Thanks
> j
>
>> ---
>> drivers/media/i2c/vd55g1.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/media/i2c/vd55g1.c b/drivers/media/i2c/vd55g1.c
>> index 1e9db21322e3..e44174056ace 100644
>> --- a/drivers/media/i2c/vd55g1.c
>> +++ b/drivers/media/i2c/vd55g1.c
>> @@ -1366,9 +1366,9 @@ static int vd55g1_init_state(struct v4l2_subdev *sd,
>> code = vd55g1_mbus_formats_mono[VD55G1_MBUS_CODE_IDX_DEF];
>> else
>> code = vd55g1_mbus_formats_bayer[VD55G1_MBUS_CODE_IDX_DEF][0];
>> - vd55g1_update_pad_fmt(sensor,
>> - &vd55g1_supported_modes[VD55G1_MODE_IDX_DEF],
>> - vd55g1_get_fmt_code(sensor, code), &fmt.format);
>> + fmt.format.code = vd55g1_get_fmt_code(sensor, code);
>> + fmt.format.width = vd55g1_supported_modes[VD55G1_MODE_IDX_DEF].width;
>> + fmt.format.height = vd55g1_supported_modes[VD55G1_MODE_IDX_DEF].height;
>>
>> return vd55g1_set_pad_fmt(sd, sd_state, &fmt);
>> }
>>
>> --
>> 2.43.0
>>
>>
--
Regards,
Benjamin
^ permalink raw reply
* Re: [PATCH v1 3/3] arm64: dts: qcom: x1e78100-t14s: Add thermal zones for keyboard skin and charging sensors
From: Konrad Dybcio @ 2026-06-25 11:36 UTC (permalink / raw)
To: Daniel Lezcano, sre, hansg, ilpo.jarvinen, linux, andersson,
konradybcio, robh, krzk+dt, conor+dt
Cc: bryan.odonoghue, platform-driver-x86, linux-kernel, linux-hwmon,
linux-arm-msm, devicetree
In-Reply-To: <20260624210825.264454-4-daniel.lezcano@oss.qualcomm.com>
On 6/24/26 11:08 PM, Daniel Lezcano wrote:
> Add thermal zone descriptions for the keyboard skin temperature and
> battery charging circuitry sensors exposed by the embedded controller.
>
> Expose the EC as a thermal sensor provider and define two thermal zones
> using the temperatures reported by the EC:
>
> - a keyboard skin temperature zone with passive and hot trip points,
> - a charging circuitry temperature zone with multiple passive trip
> points and CPU frequency mitigation levels.
>
> The charging thermal zone progressively throttles the different CPU
> clusters as the charging circuitry temperature rises and triggers a hot
> condition at the highest trip point.
>
> This provides thermal framework integration for the EC temperature
> sensors and enables platform thermal management through standard thermal
> zone definitions.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@oss.qualcomm.com>
> ---
[...]
> + ec-charging-thermal {
> + polling-delay = <5000>;
> + polling-delay-passive = <2000>;
> +
> + thermal-sensors = <&ec 3>;
> +
> + trips {
> + psv0: trip-point0 {
"ec_charging_tripN", please - these labels are file-wide, so it's
better if they're not overly short
> + temperature = <55000>;
> + hysteresis = <0>;
> + type = "passive";
> + };
> +
> + alert0: trip-point1 {
> + temperature = <63000>;
> + hysteresis = <0>;
> + type = "hot";
> + };
> + };
> +
> + cooling-maps {
> + map0 {
> + trip = <&psv0>;
> + cooling-device = <&cpu0 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu1 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu2 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu3 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu4 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu5 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu6 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu7 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu8 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu9 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu10 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>,
> + <&cpu11 THERMAL_NO_LIMIT THERMAL_NO_LIMIT>;
> +
> + };
Stray \n above, might also possibly want to throttle the GPU.
I don't know.
Konrad
^ permalink raw reply
* [PATCH] arm64: dts: ti: k3-am62a7-sk: Add bootph-all property in cpsw_mac_syscon node
From: Chintan Vankar @ 2026-06-25 11:32 UTC (permalink / raw)
To: Conor Dooley, Krzysztof Kozlowski, Rob Herring, Tero Kristo,
Vignesh Raghavendra, Nishanth Menon
Cc: c-vankar, linux-kernel, devicetree, linux-arm-kernel
Ethernet boot requires CPSW node to be present starting from R5 SPL stage.
Add "bootph-all" property in CPSW MAC's eFuse node "cpsw_mac_syscon" to
enable this node during SPL stage along with later boot stage so that CPSW
port will get static MAC address.
Signed-off-by: Chintan Vankar <c-vankar@ti.com>
---
Hello All,
This patch is based on linux-next tagged next-20260623.
arch/arm64/boot/dts/ti/k3-am62a7-sk.dts | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts b/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
index 821a9705bb7d..d3b3675e7a8f 100644
--- a/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
+++ b/arch/arm64/boot/dts/ti/k3-am62a7-sk.dts
@@ -230,6 +230,10 @@ AM62AX_MCU_IOPAD(0x0030, PIN_OUTPUT, 0) /* (C8) WKUP_UART0_RTSn */
};
};
+&cpsw_mac_syscon {
+ bootph-all;
+};
+
/* WKUP UART0 is used for DM firmware logs */
&wkup_uart0 {
pinctrl-names = "default";
--
2.34.1
^ permalink raw reply related
* Re: [PATCH v2 2/2] arm64: dts: qcom: sm8250-xiaomi-elish: add ov8856 front camera
From: Xin Xu @ 2026-06-25 11:30 UTC (permalink / raw)
To: konrad.dybcio
Cc: andersson, devicetree, konradybcio, linux-arm-msm, linux-kernel,
xxsemail
In-Reply-To: <896f3704-e7bb-48da-a501-0c9b0228573d@oss.qualcomm.com>
On Wed, 2026-06-24 at 13:56 +0200, Konrad Dybcio wrote:
> On 6/22/26 8:52 PM, Xin Xu wrote:
> > Add the ov8856 front camera, connected on CCI1 to CSIPHY4 and
> > powered by pm8008 LDOs and other supplies.
> >
> > Signed-off-by: Xin Xu <xxsemail@qq.com>
> > ---
>
> [...]
>
> > &cdsp {
> > firmware-name = "qcom/sm8250/xiaomi/elish/cdsp.mbn";
> > status = "okay";
> > @@ -877,6 +933,20 @@ bt_en_state: bt-default-state {
> > bias-pull-up;
> > };
> >
> > + camera_front_active: camera-front-active-state {
> > + pins = "gpio109";
> > + function = "gpio";
> > + bias-disable;
> > + drive-strength = <2>;
> > + };
> > +
> > + mclk3_active: mclk3-active-state {
> > + pins = "gpio97";
> > + function = "cam_mclk";
> > + bias-disable;
> > + drive-strength = <4>;
> > + };
>
> nit: other nodes have drive-strength before bias-disable
>
> Also let's move the mclk pin state definition to sm8250, it's not
> specific to this board
>
> Konrad
I will fix the coding style issues in the next patch.
Also, in the next version of the series, the mclk pin state
will be applied to sm8250.dtsi in a separate patch.
Best regards,
Xin Xu
^ permalink raw reply
* Re: [PATCH v4 04/19] firmware: qcom: scm: use dev_err_probe() for dload address failure
From: Mukesh Ojha @ 2026-06-25 11:29 UTC (permalink / raw)
To: sashiko-reviews; +Cc: conor+dt, robh, devicetree
In-Reply-To: <20260624192213.C82691F000E9@smtp.kernel.org>
On Wed, Jun 24, 2026 at 07:22:13PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] The global `qcom_tzmem_dev` pointer is set during `qcom_tzmem_enable()` but is never cleared on probe failure, causing subsequent probe retries to permanently fail with `-EBUSY`.
> - [High] The probe function calls `of_reserved_mem_device_init()` but does not call `of_reserved_mem_device_release()` on subsequent error paths, leading to permanent probe failure or resource leaks upon probe retry.
> --
>
> commit 6dc15526252b78a9c217f06ed35ed4966a794450
> Author: Mukesh Ojha <mukesh.ojha@oss.qualcomm.com>
>
> firmware: qcom: scm: use dev_err_probe() for dload address failure
>
> Replace the bare return ret after qcom_scm_find_dload_address() with
> dev_err_probe() to produce a consistent, deferred-probe-aware error
> message when the download-mode address cannot be resolved.
>
> > diff --git a/drivers/firmware/qcom/qcom_scm.c b/drivers/firmware/qcom/qcom_scm.c
> > index c8a84836887f0..6cd4d9468456f 100644
> > --- a/drivers/firmware/qcom/qcom_scm.c
> > +++ b/drivers/firmware/qcom/qcom_scm.c
> > @@ -2750,7 +2750,8 @@ static int qcom_scm_probe(struct platform_device *pdev)
> > scm->dev = &pdev->dev;
> > ret = qcom_scm_find_dload_address(&pdev->dev, &scm->dload_mode_addr);
> > if (ret < 0)
> > - return ret;
> > + return dev_err_probe(&pdev->dev, ret,
> > + "Failed to get download mode address\n");
> >
> > mutex_init(&scm->scm_bw_lock);
>
> [Severity: High]
> This is a pre-existing issue, but does qcom_scm_probe() properly release
> resources on later probe failures?
>
> Later in qcom_scm_probe(), of_reserved_mem_device_init() is called to setup
> reserved memory:
>
> drivers/firmware/qcom/qcom_scm.c:qcom_scm_probe() {
> ...
> ret = of_reserved_mem_device_init(scm->dev);
> if (ret && ret != -ENODEV)
> return dev_err_probe(scm->dev, ret,
> "Failed to setup the reserved memory region for TZ mem\n");
> ...
> }
>
> However, subsequent error paths, such as platform_get_irq_optional() returning
> -EPROBE_DEFER, exit directly without calling of_reserved_mem_device_release().
>
> If the probe is deferred, will this cause the next probe attempt to fail with
> -EBUSY, or leak memory by duplicating the rmem_assigned_device list entry on
> every retry?
>
> [Severity: High]
> This is a pre-existing issue, but does qcom_tzmem_enable() cause a permanent
> probe failure if qcom_scm_probe() defers later in the function?
>
> qcom_tzmem_enable() sets a static qcom_tzmem_dev pointer in qcom_tzmem.c:
>
> drivers/firmware/qcom/qcom_tzmem.c:qcom_tzmem_enable() {
> if (qcom_tzmem_dev)
> return -EBUSY;
>
> qcom_tzmem_dev = dev;
>
> return qcom_tzmem_init();
> }
>
> Since qcom_scm_probe() does not clear this pointer if it defers after this
> call, will subsequent probe retries permanently fail with -EBUSY when
> qcom_tzmem_enable() checks the still-set pointer?
>
Since they are existing issues, sent patches for them in the list
separately.
--
> Sashiko AI review · https://sashiko.dev/#/patchset/20260624190830.3131112-1-mukesh.ojha@oss.qualcomm.com?part=4
--
-Mukesh Ojha
^ permalink raw reply
* Re: [PATCH v2 1/2] dt-bindings: power: supply: Add TI BQ25630 charger
From: Waqar Hameed @ 2026-06-25 11:27 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Sebastian Reichel, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
kernel, linux-pm, devicetree, linux-kernel
In-Reply-To: <20260622-whimsical-inescapable-pronghorn-5f94bb@quoll>
On Mon, Jun 22, 2026 at 13:06 +0200 Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Thu, Jun 18, 2026 at 03:37:59PM +0200, Waqar Hameed wrote:
>> +allOf:
>> + - $ref: power-supply.yaml#
>> +
>> +properties:
>> + compatible:
>> + const: ti,bq25630
>> +
>> + reg:
>> + const: 0x6b
>> + description:
>> + Device I2C address.
>
> Drop description, obvious.
I see that several others actually don't have a description for this
property. I'll drop it!
>
>> +
>> + interrupts:
>> + maxItems: 1
>> + description: |
>
> Do not need '|' unless you need to preserve formatting.
Right, let's remove it.
>
>> + Device sends active low 256 µs pulse. Type should therefore be
>> + IRQ_TYPE_EDGE_FALLING.
>> +
>> + monitored-battery: true
>
> Drop this one
I was actually unsure about this one, but several other bindings had
this one. We can remove it.
>
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - interrupts
>> + - monitored-battery
>> +
>> +additionalProperties: false
>
> And here use 'unevaluatedProperties: false' instead.
Hm, reading the documentation [1], `unevaluatedProperties` should be
used, because of the "include" of `power-supply.yaml`. I'll change to
that.
However, there are tons of other places where `additionalProperties` is
used instead _with_ a reference to `power-supply.yaml` (which is what I
followed). Are all of them actually "wrong" then?
>
> With these fixed:
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Thank you for your review Krzysztof!
[1] `Documentation/devicetree/bindings/writing-schema.rst`
^ permalink raw reply
* [PATCH v4] dt-bindings: pwm: st,sti-pwm: convert to DT schema
From: Charan Pedumuru @ 2026-06-25 11:24 UTC (permalink / raw)
To: Uwe Kleine-König, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Lee Jones
Cc: linux-pwm, devicetree, linux-kernel, Charan Pedumuru
Convert STMicroelectronics STiH41x PWM/Capture controller binding
to DT schema.
Changes during conversion:
- Fix compatible string from "st,pwm" to "st,sti-pwm" to match the
actual hardware variant naming convention used across STi bindings.
- Drop pinctrl-names from the required list as pinctrl properties are
inherited and validated by the pinctrl schema.
Signed-off-by: Charan Pedumuru <charan.pedumuru@gmail.com>
---
Changes in v4:
- Fix compatible to "st,sti-pwm" in examples.
- Modify subject line to point to the correct YAML file.
- Link to v3: https://patch.msgid.link/20260625-st-pwm-v3-1-ace9bff88529@gmail.com
Changes in v3:
- Fix compatible to "st,sti-pwm".
- Drop pinctrl-names from required.
- Modify the commit message to explain the changes made.
- Change maintainer to "Lee Jones <lee.jones@linaro.org>".
- Link to v2: https://patch.msgid.link/20260618-st-pwm-v2-1-c792d5795ce2@gmail.com
Changes in v2:
- Add the missing interrupts to the required following the old binding.
- Modify the commit message to explain the changes made.
- Link to v1: https://patch.msgid.link/20260613-st-pwm-v1-1-458c2c89709a@gmail.com
---
Documentation/devicetree/bindings/pwm/pwm-st.txt | 43 ----------
.../devicetree/bindings/pwm/st,sti-pwm.yaml | 92 ++++++++++++++++++++++
2 files changed, 92 insertions(+), 43 deletions(-)
diff --git a/Documentation/devicetree/bindings/pwm/pwm-st.txt b/Documentation/devicetree/bindings/pwm/pwm-st.txt
deleted file mode 100644
index 19fce774cafa..000000000000
--- a/Documentation/devicetree/bindings/pwm/pwm-st.txt
+++ /dev/null
@@ -1,43 +0,0 @@
-STMicroelectronics PWM driver bindings
---------------------------------------
-
-Required parameters:
-- compatible : "st,pwm"
-- #pwm-cells : Number of cells used to specify a PWM. First cell
- specifies the per-chip index of the PWM to use and the
- second cell is the period in nanoseconds - fixed to 2
- for STiH41x.
-- reg : Physical base address and length of the controller's
- registers.
-- pinctrl-names: Set to "default".
-- pinctrl-0: List of phandles pointing to pin configuration nodes
- for PWM module.
- For Pinctrl properties, please refer to [1].
-- clock-names: Valid entries are "pwm" and/or "capture".
-- clocks: phandle of the clock used by the PWM module.
- For Clk properties, please refer to [2].
-- interrupts: IRQ for the Capture device
-
-Optional properties:
-- st,pwm-num-chan: Number of available PWM channels. Default is 0.
-- st,capture-num-chan: Number of available Capture channels. Default is 0.
-
-[1] Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
-[2] Documentation/devicetree/bindings/clock/clock-bindings.txt
-
-Example:
-
-pwm1: pwm@fe510000 {
- compatible = "st,pwm";
- reg = <0xfe510000 0x68>;
- #pwm-cells = <2>;
- pinctrl-names = "default";
- pinctrl-0 = <&pinctrl_pwm1_chan0_default
- &pinctrl_pwm1_chan1_default
- &pinctrl_pwm1_chan2_default
- &pinctrl_pwm1_chan3_default>;
- clocks = <&clk_sysin>;
- clock-names = "pwm";
- st,pwm-num-chan = <4>;
- st,capture-num-chan = <2>;
-};
diff --git a/Documentation/devicetree/bindings/pwm/st,sti-pwm.yaml b/Documentation/devicetree/bindings/pwm/st,sti-pwm.yaml
new file mode 100644
index 000000000000..e980c76b54e7
--- /dev/null
+++ b/Documentation/devicetree/bindings/pwm/st,sti-pwm.yaml
@@ -0,0 +1,92 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/pwm/st,sti-pwm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STMicroelectronics STiH41x PWM/Capture controller
+
+maintainers:
+ - Lee Jones <lee.jones@linaro.org>
+
+description:
+ The STiH41x PWM controller supports both PWM output and input capture
+ functionality. It provides multiple PWM output channels for generating
+ variable duty-cycle waveforms, and multiple input capture channels for
+ measuring external signal periods and pulse widths. PWM output channels
+ and input capture channels are configured independently via
+ st,pwm-num-chan and st,capture-num-chan respectively.
+
+allOf:
+ - $ref: pwm.yaml#
+
+properties:
+ compatible:
+ const: st,sti-pwm
+
+ reg:
+ maxItems: 1
+
+ "#pwm-cells":
+ const: 2
+
+ pinctrl-names:
+ const: default
+
+ clock-names:
+ items:
+ enum: [pwm, capture]
+ minItems: 1
+ maxItems: 2
+
+ clocks:
+ minItems: 1
+ maxItems: 2
+
+ interrupts:
+ description: IRQ line for the capture device.
+ maxItems: 1
+
+ st,pwm-num-chan:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: Number of available PWM output channels.
+ minimum: 0
+ maximum: 4
+ default: 0
+
+ st,capture-num-chan:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: Number of available input capture channels.
+ minimum: 0
+ maximum: 4
+ default: 0
+
+required:
+ - reg
+ - compatible
+ - "#pwm-cells"
+ - clock-names
+ - clocks
+ - interrupts
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ pwm@fe510000 {
+ compatible = "st,sti-pwm";
+ reg = <0xfe510000 0x68>;
+ interrupts = <GIC_SPI 128 IRQ_TYPE_LEVEL_HIGH>;
+ #pwm-cells = <2>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_pwm1_chan0_default
+ &pinctrl_pwm1_chan1_default
+ &pinctrl_pwm1_chan2_default
+ &pinctrl_pwm1_chan3_default>;
+ clocks = <&clk_sysin>;
+ clock-names = "pwm";
+ st,pwm-num-chan = <4>;
+ st,capture-num-chan = <2>;
+ };
+...
---
base-commit: c425609d6ac4012c8bbf01ec2e10e801b1923a7b
change-id: 20260613-st-pwm-1feade6cfea3
Best regards,
--
Charan Pedumuru <charan.pedumuru@gmail.com>
^ permalink raw reply related
* Re: [PATCH 2/2] iio: adc: Add Nuvoton MA35D1 EADC driver
From: sashiko-bot @ 2026-06-25 11:20 UTC (permalink / raw)
To: Chi-Wen Weng; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260625110638.38438-3-cwweng.linux@gmail.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Multi-channel buffered scans read stale or actively converting data for subsequent channels due to premature interrupt mapping.
- [High] Unpaced software re-triggering loop in the buffer interrupt handler causes an IRQ storm and livelock.
- [High] Missing sign-extension and incorrect scan type for negative differential readings.
- [High] Device suspend does not synchronize with ongoing direct conversions, leading to potential hardware aborts.
- [Medium] Differential negative channel configuration from device tree is accepted but completely ignored by hardware configuration.
--
commit 71778e9c3ef41a6d7da62ab5f861733a638e3f61
Author: Chi-Wen Weng <cwweng@nuvoton.com>
iio: adc: Add Nuvoton MA35D1 EADC driver
This patch adds an IIO driver for the Nuvoton MA35D1 Enhanced ADC
controller, supporting both direct reads and triggered buffered capture.
> diff --git a/drivers/iio/adc/ma35d1_eadc.c b/drivers/iio/adc/ma35d1_eadc.c
> new file mode 100644
> --- /dev/null
> +++ b/drivers/iio/adc/ma35d1_eadc.c
[ ... ]
> +static void ma35d1_adc_config_sample(struct ma35d1_adc *adc,
> + unsigned int sample, unsigned int channel)
> +{
> + u32 reg = MA35D1_EADC_SCTL(sample);
> +
> + ma35d1_adc_rmw(adc, reg,
> + MA35D1_EADC_SCTL_CHSEL_MASK |
> + MA35D1_EADC_SCTL_TRGSEL_MASK,
> + FIELD_PREP(MA35D1_EADC_SCTL_CHSEL_MASK, channel) |
> + MA35D1_EADC_SCTL_TRGSEL_ADINT0);
> +}
[Severity: Medium]
Will the hardware properly sample differential pairs if only the positive
channel is configured here?
The driver parses differential configurations using diff-channels and stores
the negative channel in chan->channel2, but only the positive channel is ever
written to the hardware CHSEL mask here.
If the hardware relies on arbitrary negative channel configurations rather
than fixed hardware pairings, the negative pin won't be sampled correctly.
Additionally, ma35d1_adc_parse_channels() doesn't seem to validate against
any fixed hardware pair requirements.
[ ... ]
> +static irqreturn_t ma35d1_adc_trigger_handler(int irq, void *p)
> +{
> + struct iio_poll_func *pf = p;
> + struct iio_dev *indio_dev = pf->indio_dev;
> + struct ma35d1_adc *adc = iio_priv(indio_dev);
> + int i;
> +
> + for (i = 0; i < adc->scan_chancnt; i++)
> + adc->scan.channels[i] =
> + ma35d1_adc_read(adc, MA35D1_EADC_DAT(i)) &
> + MA35D1_EADC_DAT_MASK;
[Severity: High]
Will this loop read stale or actively converting data for channels 1 through N
during multi-channel scans?
In ma35d1_adc_buffer_postenable(), the interrupt source is hardcoded to sample
module 0:
ma35d1_adc_rmw(adc, MA35D1_EADC_INTSRC0,
MA35D1_EADC_INTSRC0_ADINT0,
MA35D1_EADC_INTSRC0_ADINT0);
This means the interrupt fires as soon as the first channel finishes
converting. When this threaded handler wakes up and immediately reads all
DAT(i) registers, the subsequent sample modules may not have completed their
conversions yet.
> +
> + iio_push_to_buffers_with_timestamp(indio_dev, &adc->scan, pf->timestamp);
> + iio_trigger_notify_done(adc->trig);
> +
> + ma35d1_adc_rmw(adc, MA35D1_EADC_CTL, MA35D1_EADC_CTL_ADCIEN0,
> + MA35D1_EADC_CTL_ADCIEN0);
> + ma35d1_adc_write(adc, MA35D1_EADC_SWTRG, 1);
[Severity: High]
Does unconditionally triggering the next conversion here cause an interrupt
storm?
Since the driver starts the conversion in postenable by writing SWTRG = 1,
and then unconditionally restarts it immediately at the end of the handler,
this creates a 0-delay free-running loop without any hardware timer or
pacing mechanism. This can livelock the system by flooding the CPU with
interrupts as fast as the ADC converts.
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int ma35d1_adc_read_conversion(struct iio_dev *indio_dev,
> + const struct iio_chan_spec *chan,
> + int *val)
> +{
[ ... ]
> + timeout = wait_for_completion_interruptible_timeout(&adc->completion,
> + MA35D1_EADC_TIMEOUT);
> + ma35d1_adc_disable_irq(adc);
> +
> + if (timeout < 0)
> + return timeout;
> + if (!timeout)
> + return -ETIMEDOUT;
> +
> + *val = ma35d1_adc_read(adc, MA35D1_EADC_DAT(0)) & MA35D1_EADC_DAT_MASK;
[Severity: High]
Will this return massive positive values for negative differential readings?
Differential channels return two's complement values. Without using
sign_extend32() before assigning to the 32-bit *val, negative voltage
differences will be incorrectly returned to userspace as large positive
integers instead of signed values.
[ ... ]
> +static void ma35d1_adc_init_channel(struct ma35d1_adc *adc,
> + struct iio_chan_spec *chan, u32 vinp,
> + u32 vinn, int scan_index, bool differential)
> +{
[ ... ]
> + chan->info_mask_separate = BIT(IIO_CHAN_INFO_RAW);
> + chan->scan_type.sign = 'u';
[Severity: High]
Should differential channels be marked as signed instead of unsigned?
When differential is true, this forces the scan type sign to 'u', which
incorrectly informs the IIO subsystem that the differential data is unsigned
instead of two's complement.
[ ... ]
> +static int ma35d1_adc_suspend(struct device *dev)
> +{
> + struct iio_dev *indio_dev = dev_get_drvdata(dev);
> + struct ma35d1_adc *adc = iio_priv(indio_dev);
> +
> + if (iio_buffer_enabled(indio_dev))
> + return -EBUSY;
> +
> + ma35d1_adc_hw_disable(adc);
> + clk_disable_unprepare(adc->clk);
[Severity: High]
Can this race with active direct conversions and cause a bus fault?
ma35d1_adc_suspend() disables the peripheral clock without acquiring
adc->lock or checking iio_device_claim_direct_mode().
If a concurrent direct read is waiting in ma35d1_adc_read_conversion():
timeout = wait_for_completion_interruptible_timeout(&adc->completion,
MA35D1_EADC_TIMEOUT);
ma35d1_adc_disable_irq(adc);
and suspend disables the clock, the wait may time out or be interrupted.
Accessing the ADC registers immediately afterward in ma35d1_adc_disable_irq()
with the clock disabled could result in a synchronous external abort.
> +
> + return 0;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625110638.38438-1-cwweng.linux@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: iio: adc: Add Nuvoton MA35D1 EADC
From: sashiko-bot @ 2026-06-25 11:18 UTC (permalink / raw)
To: Chi-Wen Weng; +Cc: devicetree, conor+dt, robh
In-Reply-To: <20260625110638.38438-2-cwweng.linux@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing `resets` property for a hardware block that has a dedicated reset line.
--
commit 1f48c0ec2f3eb7b2358c1e0e2db87a26859d5fc5
Author: Chi-Wen Weng <cwweng@nuvoton.com>
dt-bindings: iio: adc: Add Nuvoton MA35D1 EADC
This commit adds the devicetree binding for the Enhanced ADC controller
found on Nuvoton MA35D1 SoCs. It describes the register region, interrupt,
functional clock, and standard channel child nodes.
> diff --git a/Documentation/devicetree/bindings/iio/adc/nuvoton,ma35d1-eadc.yaml b/Documentation/devicetree/bindings/iio/adc/nuvoton,ma35d1-eadc.yaml
> new file mode 100644
> index 0000000000000..ae7ad0f7689ab
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/nuvoton,ma35d1-eadc.yaml
[ ... ]
> +properties:
> + compatible:
> + const: nuvoton,ma35d1-eadc
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
[Severity: Low]
Is the resets property missing from this binding? The Nuvoton MA35D1 SoC
explicitly provides a reset line for the Enhanced ADC block (defined as
MA35D1_RESET_EADC0 in include/dt-bindings/reset/nuvoton,ma35d1-reset.h).
Should the resets property be declared here and added to the required list
to accurately reflect the hardware dependencies and avoid dt-schema
validation errors if a board uses the reset line?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625110638.38438-1-cwweng.linux@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH v8 2/2] hwmon: Add Eswin EIC7700 PVT sensor driver
From: Julian Braha @ 2026-06-25 11:16 UTC (permalink / raw)
To: hehuan1, linux, robh, krzk+dt, conor+dt, p.zabel, linux-hwmon,
devicetree, linux-kernel
Cc: ningyu, linmin, pinkesh.vaghela, luyulin, dongxuyang
In-Reply-To: <20260625061214.1648-1-hehuan1@eswincomputing.com>
Hi Huan,
On 6/25/26 07:12, hehuan1@eswincomputing.com wrote:
> +config SENSORS_EIC7700_PVT
> + tristate "Eswin EIC7700 Voltage, Temperature sensor driver"
> + depends on ARCH_ESWIN || COMPILE_TEST
> + depends on HWMON
You already put SENSORS_EIC7700_PVT inside the 'if HWMON .. endif', so
you don't need the 'depends on HWMON'. It creates a duplicate
dependency.
- Julian Braha
^ permalink raw reply
* Re: [RFC PATCH 2/3] pinctrl: mediatek: mt8516/mt8167: Migrate driver to pinctrl-paris platform
From: sashiko-bot @ 2026-06-25 11:16 UTC (permalink / raw)
To: Luca Leonardo Scorcia; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260625104742.113803-3-l.scorcia@gmail.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Critical] Pinmux mode field spacing is incorrectly changed from 4 bits to 3 bits per pin, corrupting all pinmux configurations.
- [High] Incorrect register bit offsets and widths for Drive Strength (DRV) and Slew Rate (SR) corrupt hardware configuration.
- [High] SMT register bit mappings are swapped between pins 106 and 109.
- [High] SMT bit mappings for MT8516 pins 70-73 (MSDC2 DAT) are incorrectly assigned the MT8167 layout, breaking MT8516 SD/eMMC.
- [High] Driver drops support for `mediatek,pctl-regmap` without providing a fallback, breaking ABI backward compatibility with existing device trees.
--
commit b99ee859c088809165f9a4794ca1471b372efd64
Author: Luca Leonardo Scorcia <l.scorcia@gmail.com>
pinctrl: mediatek: mt8516/mt8167: Migrate driver to pinctrl-paris platform
Migrate the mt8516/mt8167 pinctrl driver to the paris platform driver.
This change lets us correctly describe the two base addresses (GPIO/EINT)
used by the SoCs in their device tree. It also adds support for driving
strength capability and R1R0 pullup-pulldown on pins.
> diff --git a/drivers/pinctrl/mediatek/pinctrl-mt8516.c b/drivers/pinctrl/mediatek/pinctrl-mt8516.c
> --- a/drivers/pinctrl/mediatek/pinctrl-mt8516.c
> +++ b/drivers/pinctrl/mediatek/pinctrl-mt8516.c
[ ... ]
> +static const struct mtk_pin_field_calc mt8516_pin_mode_range[] = {
> + PIN_FIELD15(0, 124, 0x300, 0x10, 0, 3),
> +};
[Severity: Critical]
Does this configuration cause a regression in pin multiplexing across the SoC?
The legacy driver used a 4-bit shift per pin to pack 5 pins per 32-bit
register. By passing 3 as the width here, the framework calculates mode
offsets using 3 bits per pin. This causes pinmux writes to align with
the wrong bits and can overwrite the configurations of adjacent pins.
[ ... ]
> +static const struct mtk_pin_field_calc mt8516_pin_smt_range[] = {
[ ... ]
> + PIN_FIELD16(70, 70, 0xa30, 0x10, 3, 1),
> + PIN_FIELD16(71, 71, 0xa30, 0x10, 4, 1),
> + PIN_FIELD16(72, 72, 0xa30, 0x10, 5, 1),
> + PIN_FIELD16(73, 73, 0xa30, 0x10, 6, 1),
[Severity: High]
Does this break the SMT configuration for the MT8516 MSDC2 interface?
The unified array assigns the MT8167 layout to pins 70-73 (bits 3, 4, 5, 6).
The MT8516 requires bits 6, 5, 4, 3, as defined in its IES layout and the
legacy driver.
[ ... ]
> + PIN_FIELD16(106, 106, 0xa20, 0x10, 13, 1),
> + PIN_FIELD16(107, 107, 0xa20, 0x10, 14, 1),
> + PIN_FIELD16(108, 108, 0xa20, 0x10, 15, 1),
> + PIN_FIELD16(109, 109, 0xa30, 0x10, 0, 1),
[Severity: High]
Are the SMT bits for pins 106 and 109 swapped?
In the IES range, pin 106 maps to 0x930 bit 0 and pin 109 maps to 0x920
bit 13. SMT layouts typically mirror IES layouts offset by 0x100.
The mappings here write to the wrong hardware bits for these pins.
[ ... ]
> +static const struct mtk_pin_field_calc mt8516_pin_drv_range[] = {
> + PINS_FIELD16(0, 4, 0xd00, 0x10, 0, 2),
[ ... ]
> +static const struct mtk_pin_field_calc mt8516_pin_sr_range[] = {
> + PINS_FIELD16(0, 4, 0xd00, 0x10, 3, 1),
[Severity: High]
Can this corrupt the Slew Rate hardware configuration?
Setting the Slew Rate offset to 3, and using a 2-bit width starting at bit 0
for the Drive Strength of these pins, differs from the hardware requirements.
Additionally, a 2-bit mask restricts DRV_GRP4 pins (such as MSDC) from
supporting drive strengths greater than 8mA, causing the framework to reject
higher values.
[ ... ]
> +static struct platform_driver mt8516_pinctrl_driver = {
> .driver = {
> .name = "mediatek-mt8516-pinctrl",
> - .of_match_table = mt8516_pctrl_match,
> - .pm = pm_sleep_ptr(&mtk_eint_pm_ops),
> + .of_match_table = mt8516_pinctrl_of_match,
> + .pm = pm_sleep_ptr(&mtk_paris_pinctrl_pm_ops),
> },
> + .probe = mtk_paris_pinctrl_probe,
> };
[Severity: High]
Will this migration cause probing to fail on older device trees?
The paris framework probe relies on devm_platform_ioremap_resource_byname
which strictly requires a reg-names property. The legacy driver supported a
mediatek,pctl-regmap fallback. Dropping this fallback might introduce a
regression with existing device trees lacking the reg-names property.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625104742.113803-1-l.scorcia@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH v19 6/7] coresight: ctcu: enable byte-cntr for TMC ETR devices
From: sashiko-bot @ 2026-06-25 11:09 UTC (permalink / raw)
To: Jie Gan; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260625-enable-byte-cntr-for-ctcu-v19-6-8fbbf22e8381@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 11 potential issue(s) to consider:
- [Critical] Global function pointer accessed locklessly leads to Use-After-Free on driver unload
- [Critical] Global sysfs ops pointer triggers NULL pointer dereference on ETR without CTCU
- [High] Driver remove/unbind blocks indefinitely waiting for userspace file closure
- [High] ctcu_byte_cntr_stop() skips setting enable=false for SYSFS mode, causing infinite read hangs
- [High] TOCTOU race re-enables ETR hardware after framework disables it
- [High] Missing wake_up() in error path deadlocks driver remove()
- [High] Unsynchronized checks of irq_enabled lead to resource leaks and driver unbind hangs
- [High] Data race on concurrent read() calls causes NULL pointer dereference
- [Medium] Spurious Interrupt Handling due to missing hardware status check in shared IRQ
- [Medium] Error codes (-ERESTARTSYS) from byte_cntr_ops are swallowed and convert to EOF
- [Medium] Masked error code breaks sysfs reading fallback on memory failure
--
commit 128ffef0065b7b8a8c73e5e3f9b0ce8bebea0593
Author: Jie Gan <jie.gan@oss.qualcomm.com>
coresight: ctcu: enable byte-cntr for TMC ETR devices
This patch introduces the byte-cntr function provided by the CTCU device to
transfer data from the ETR buffer to userspace. It sets up an interrupt
handler to track data size thresholds and switches buffers when full or on
timeout. It also integrates with the TMC ETR framework via sysfs ops.
> diff --git a/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c b/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c
> new file mode 100644
> index 0000000000000..5ab97a71f02f6
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-ctcu-byte-cntr.c
[ ... ]
> +static irqreturn_t byte_cntr_handler(int irq, void *data)
> +{
> + struct ctcu_byte_cntr *byte_cntr_data = data;
> +
> + atomic_inc(&byte_cntr_data->irq_cnt);
> + wake_up(&byte_cntr_data->wq);
> +
> + return IRQ_HANDLED;
> +}
[Severity: Medium]
Is this handler missing a hardware status check?
Since the IRQ is registered with IRQF_SHARED, another device sharing the line
could fire an interrupt. By unconditionally returning IRQ_HANDLED without
verifying the interrupt source, the kernel's spurious interrupt detection
might be suppressed, and the CTCU buffer switch logic could be triggered
prematurely.
[ ... ]
> +static struct ctcu_byte_cntr *ctcu_get_byte_cntr(struct coresight_device *ctcu,
> + struct coresight_device *etr)
> +{
> + struct ctcu_drvdata *drvdata = dev_get_drvdata(ctcu->dev.parent);
[Severity: Critical]
Could this dereference a NULL pointer if an ETR without a connected CTCU
is used?
If the system has multiple ETRs and one lacks a CTCU, the globally registered
byte_cntr_sysfs_ops might pass a NULL ctcu pointer here. This would cause a
panic when ctcu->dev.parent is accessed.
[ ... ]
> +static bool ctcu_byte_cntr_switch_buffer(struct tmc_drvdata *etr_drvdata,
> + struct ctcu_byte_cntr *byte_cntr_data)
> +{
[ ... ]
> + raw_spin_unlock_irqrestore(&etr_drvdata->spinlock, flags);
> +
> + /* Restart the ETR once a free buffer is available */
> + if (found_free_buf &&
> + coresight_get_mode(etr_drvdata->csdev) != CS_MODE_DISABLED)
> + tmc_etr_enable_disable_hw(etr_drvdata, true);
> +
> + return found_free_buf;
> +}
[Severity: High]
Does this lockless check introduce a race condition?
If a concurrent operation disables the trace session and drops the lock before
setting the mode to CS_MODE_DISABLED, this code might incorrectly re-enable
the hardware after the framework has explicitly disabled it.
[ ... ]
> +static ssize_t tmc_byte_cntr_get_data(struct tmc_drvdata *etr_drvdata, loff_t pos,
> + size_t len, char **bufpp)
> +{
[ ... ]
> + pos = byte_cntr_data->buf_node->pos;
> + sysfs_buf = byte_cntr_data->buf_node->sysfs_buf;
> + actual = tmc_etr_read_sysfs_buf(sysfs_buf, pos, len, bufpp);
> + if (actual <= 0) {
> + /* Reset buf_node upon reading is finished or failed */
> + byte_cntr_data->buf_node->reading = false;
> + byte_cntr_data->buf_node = NULL;
[Severity: High]
Can concurrent read operations cause a NULL pointer dereference here?
If two threads read from the same ETR file descriptor, they could concurrently
access and modify byte_cntr_data->buf_node without serialization, causing one
thread to clear the pointer while the other is still using it.
[ ... ]
> +static int tmc_read_prepare_byte_cntr(struct tmc_drvdata *etr_drvdata)
> +{
> + struct coresight_device *ctcu = tmc_etr_get_ctcu_device(etr_drvdata);
> + struct ctcu_byte_cntr *byte_cntr_data;
> + unsigned long flags;
> + int ret = 0;
> +
> + /* byte-cntr is operating with SYSFS mode being enabled only */
> + if (coresight_get_mode(etr_drvdata->csdev) != CS_MODE_SYSFS)
> + return -EINVAL;
> +
> + byte_cntr_data = ctcu_get_byte_cntr(ctcu, etr_drvdata->csdev);
> + if (!byte_cntr_data || !byte_cntr_data->irq_enabled)
> + return -EINVAL;
> +
> + raw_spin_lock_irqsave(&byte_cntr_data->spin_lock, flags);
[Severity: High]
Does checking irq_enabled outside the spinlock create a race?
A concurrent sysfs write could set irq_enabled to 0 immediately before the
lock is acquired. If reading is then set to true, read_unprepare() will later
abort early upon seeing irq_enabled is 0. This would leak the buffer list and
leave reading permanently true, causing the driver unbind to hang forever.
[ ... ]
> + byte_cntr_data->reading = true;
> + raw_spin_unlock_irqrestore(&byte_cntr_data->spin_lock, flags);
> + /* Setup an available etr_buf_list for byte-cntr */
> + ret = tmc_create_etr_buf_list(etr_drvdata, 2);
> + if (ret) {
> + byte_cntr_data->reading = false;
> + return ret;
> + }
[Severity: High]
Is this error path missing a wake_up() call?
If this allocation fails, the reading flag is cleared, but concurrent readers
waiting in the driver's remove function won't be notified, potentially
deadlocking the unbind process.
[ ... ]
> +void ctcu_byte_cntr_stop(struct coresight_device *csdev, struct coresight_path *path)
> +{
> + struct coresight_device *sink = coresight_get_sink(path);
> + struct ctcu_byte_cntr *byte_cntr_data;
> +
> + if (coresight_get_mode(sink) == CS_MODE_SYSFS)
> + return;
[Severity: High]
Should the enable flag be cleared for SYSFS mode here?
By returning early, enable remains true. Readers waiting in
tmc_byte_cntr_get_data() might never receive the EOF condition when the
trace is stopped, causing them to loop indefinitely.
> diff --git a/drivers/hwtracing/coresight/coresight-ctcu-core.c b/drivers/hwtracing/coresight/coresight-ctcu-core.c
> index e8720026c9e3f..2da1a6f3d29f4 100644
> --- a/drivers/hwtracing/coresight/coresight-ctcu-core.c
> +++ b/drivers/hwtracing/coresight/coresight-ctcu-core.c
[ ... ]
> +static void ctcu_platform_remove(struct platform_device *pdev)
> +{
> + struct ctcu_drvdata *drvdata = platform_get_drvdata(pdev);
> + struct ctcu_byte_cntr *byte_cntr_data;
> + unsigned long flags;
> + int i;
> +
> + if (WARN_ON(!drvdata))
> + return;
> +
> + /*
> + * Signal all active byte-cntr readers to exit, then wait for them to
> + * finish before resetting the ops pointer and freeing driver data.
> + * Without this, a reader blocked in wait_event_interruptible_timeout()
> + * would access the freed ctcu_drvdata wait-queue head (use-after-free).
> + */
> + for (i = 0; i < ETR_MAX_NUM; i++) {
> + byte_cntr_data = &drvdata->byte_cntr_data[i];
> + raw_spin_lock_irqsave(&byte_cntr_data->spin_lock, flags);
> + /* Set enable=false for all ports to signal teardown to racing readers */
> + byte_cntr_data->enable = false;
> + if (!byte_cntr_data->reading) {
> + raw_spin_unlock_irqrestore(&byte_cntr_data->spin_lock, flags);
> + continue;
> + }
> + raw_spin_unlock_irqrestore(&byte_cntr_data->spin_lock, flags);
> + wake_up_all(&byte_cntr_data->wq);
> + wait_event(byte_cntr_data->wq, !byte_cntr_data->reading);
> + }
[Severity: High]
Could this wait indefinitely during driver remove?
The reading flag is only cleared when the userspace file is closed. A
userspace process leaving the file descriptor open could permanently block
module unload or device unbind.
> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c
> index 2b26ce6455a7b..e78f8891f11e0 100644
> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c
> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c
[ ... ]
> +ssize_t tmc_etr_get_sysfs_trace(struct tmc_drvdata *drvdata,
> + loff_t pos, size_t len, char **bufpp)
> +{
> + ssize_t ret;
> + const struct tmc_sysfs_ops *byte_cntr_ops = READ_ONCE(byte_cntr_sysfs_ops);
> +
> + if (byte_cntr_ops) {
> + ret = byte_cntr_ops->get_trace_data(drvdata, pos, len, bufpp);
> + /* Return the filled buffer */
> + if (ret > 0 || ret == -ENOMEM)
> + return ret;
> + }
> +
> + return tmc_etr_read_sysfs_buf(drvdata->sysfs_buf, pos, len, bufpp);
> +}
[Severity: Medium]
Does this silently swallow -ERESTARTSYS?
If a signal interrupts the wait inside tmc_byte_cntr_get_data(), it returns
-ERESTARTSYS, which falls through here to a normal sysfs buffer read. This
might convert the signal into an EOF or return unsynced data instead of
letting userspace handle the signal cleanly.
[ ... ]
> +int tmc_read_prepare_etr(struct tmc_drvdata *drvdata)
> +{
> + int ret = 0;
> + unsigned long flags;
> + const struct tmc_sysfs_ops *byte_cntr_ops;
>
> /* config types are set a boot time and never change */
> if (WARN_ON_ONCE(drvdata->config_type != TMC_CONFIG_TYPE_ETR))
> return -EINVAL;
>
> + byte_cntr_ops = READ_ONCE(byte_cntr_sysfs_ops);
> + if (byte_cntr_ops) {
> + ret = byte_cntr_ops->read_prepare(drvdata);
[Severity: Critical]
Is it safe to access this global function pointer locklessly?
If the trace file is opened and the CTCU module is concurrently unloaded, the
memory backing byte_cntr_ops could be freed before read_prepare() executes,
leading to a use-after-free regression.
> + if (!ret || ret == -EBUSY)
> + return ret;
> +
> + ret = 0;
> + }
[Severity: Medium]
Why are errors like -ENOMEM from read_prepare() masked here?
If the memory allocation fails inside tmc_read_prepare_byte_cntr(), hiding the
error breaks the fallback sysfs reading path, leading to delays and hanging
operations instead of propagating the failure properly.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260625-enable-byte-cntr-for-ctcu-v19-0-8fbbf22e8381@oss.qualcomm.com?part=6
^ 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