Re: [PATCH 2/2] arm64: dts: cix: add FCH(S0)/S5 GPIO controllers for sky1
From: Krzysztof Kozlowski <krzk@kernel.org>
Date: 2026-03-07 15:22:42
Also in:
linux-arm-kernel, linux-gpio, lkml
On Fri, Mar 06, 2026 at 05:32:38PM +0800, Zichar Zhang wrote:
quoted hunk ↗ jump to hunk
From: "Zichar.Zhang" <redacted> Add Cadence GPIO controller nodes for Sky1 FCH(S0) and S5 domains in sky1.dtsi, and enable those controllers on sky1-orion-o6. Signed-off-by: Zichar Zhang <redacted> --- arch/arm64/boot/dts/cix/sky1-orion-o6.dts | 28 +++++ arch/arm64/boot/dts/cix/sky1.dtsi | 123 ++++++++++++++++++++++ 2 files changed, 151 insertions(+)diff --git a/arch/arm64/boot/dts/cix/sky1-orion-o6.dts b/arch/arm64/boot/dts/cix/sky1-orion-o6.dts index 4dee8cd0b86d..4dc76e0135ee 100644 --- a/arch/arm64/boot/dts/cix/sky1-orion-o6.dts +++ b/arch/arm64/boot/dts/cix/sky1-orion-o6.dts@@ -89,3 +89,31 @@ &pcie_x1_1_rc { &uart2 { status = "okay"; }; + +&s5_gpio0 {
I already asked cixtech contributors to read DTS coding style. More than once. Does it mean I need to ask EACH contributor that? Maybe create internal guideline to avoid trivial mistakes?
quoted hunk ↗ jump to hunk
+ status = "okay"; +}; + +&s5_gpio1 { + status = "okay"; +}; + +&s5_gpio2 { + status = "okay"; +}; + +&fch_gpio0 { + status = "okay"; +}; + +&fch_gpio1 { + status = "okay"; +}; + +&fch_gpio2 { + status = "okay"; +}; + +&fch_gpio3 { + status = "okay"; +};diff --git a/arch/arm64/boot/dts/cix/sky1.dtsi b/arch/arm64/boot/dts/cix/sky1.dtsi index 72f3b195a927..9ceaf8f68e83 100644 --- a/arch/arm64/boot/dts/cix/sky1.dtsi +++ b/arch/arm64/boot/dts/cix/sky1.dtsi@@ -185,6 +185,13 @@ psci { method = "smc"; }; + s5_gpio_apb_clk: s5-gpio-apb-clk {
Please use name for all fixed clocks which matches current format recommendation: 'clock-<freq>' (see also the pattern in the binding for any other options). https://web.git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/clock/fixed-clock.yaml
quoted hunk ↗ jump to hunk
+ compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <100000000>; + clock-output-names = "s5_gpio_apb_clk"; + }; + soc@0 { compatible = "simple-bus"; ranges = <0 0 0 0 0x20 0>;@@ -354,6 +361,74 @@ syscon: syscon@4160000 { #reset-cells = <1>; }; + fch_gpio0: gpio-controller@4120000 {
Again, read DTS coding style.
+ compatible = "cdns,gpio-r1p02"; + reg = <0x0 0x4120000 0x0 0x1000>; + clocks = <&scmi_clk CLK_TREE_FCH_GPIO_APB>; + clock-names = "fch_gpio_apb_clk";
This is pointless name. GPIO block does not take some "fch" input. You just called the input clock based on clock output which is completely misunderstanding of the DTS. Best regards, Krzysztof