Re: [PATCH v9 6/8] riscv: dts: spacemit: enable the i2c8 adapter

Yixun Lan
2025-07-24T21:11:18Z

Hi Alex,

On 15:25 Thu 24 Jul , Alex Elder wrote:
> Define properties for the I2C adapter that provides access to the
> SpacemiT P1 PMIC. Enable this adapter on the Banana Pi BPI-F3.
>
> Signed-off-by: Alex Elder <elder@riscstar.com>
> ---
> arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts | 15 +++++++++++++++
> arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi | 7 +++++++
> arch/riscv/boot/dts/spacemit/k1.dtsi | 11 +++++++++++
> 3 files changed, 33 insertions(+)
>
> diff --git a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
> index fe22c747c5012..7c9f91c88e01a 100644
> --- a/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
> +++ b/arch/riscv/boot/dts/spacemit/k1-bananapi-f3.dts
> @@ -40,6 +40,21 @@ &emmc {
> status = "okay";
> };
>
> +&i2c8 {
> + pinctrl-0 = <&i2c8_cfg>;
> + pinctrl-names = "default";
..
> + #address-cells = <1>;
> + #size-cells = <0>;
I think these two can be moved into dtsi, as they are
common and fixed properties for the i2c controller

> + status = "okay";
> +
> + pmic@41 {
> + compatible = "spacemit,p1";
> + reg = <0x41>;
> + interrupts = <64>;
..
> + status = "okay";
status property here can be dropped as enabled by default
> + };
> +};
> +
> &uart0 {
> pinctrl-names = "default";
> pinctrl-0 = <&uart0_2_cfg>;
> diff --git a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
> index 3810557374228..96d7a46d4bf77 100644
> --- a/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
> +++ b/arch/riscv/boot/dts/spacemit/k1-pinctrl.dtsi
> @@ -11,6 +11,13 @@
> #define K1_GPIO(x) (x / 32) (x % 32)
>
> &pinctrl {
> + i2c8_cfg: i2c8-cfg {
> + i2c8-0-pins {
> + pinmux = <K1_PADCONF(93, 0)>, /* PWR_SCL */
> + <K1_PADCONF(94, 0)>; /* PWR_SDA */
> + };
> + };
> +
> uart0_2_cfg: uart0-2-cfg {
> uart0-2-pins {
> pinmux = <K1_PADCONF(68, 2)>,
> diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
> index abde8bb07c95c..2a5a132d5a774 100644
> --- a/arch/riscv/boot/dts/spacemit/k1.dtsi
> +++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
> @@ -459,6 +459,17 @@ pwm7: pwm@d401bc00 {
> status = "disabled";
> };
>
> + i2c8: i2c@d401d800 {
> + compatible = "spacemit,k1-i2c";
> + reg = <0x0 0xd401d800 0x0 0x38>;
..
> + interrupts = <19>;
I'd suggest to move interrupts property after clock, see my similar
comment
https://lore.kernel.org/all/20250724121916-GYA748228@gentoo/

> + clocks = <&syscon_apbc CLK_TWSI8>,
> + <&syscon_apbc CLK_TWSI8_BUS>;
> + clock-names = "func", "bus";
> + clock-frequency = <400000>;
> + status = "disabled";
> + };
> +
> pinctrl: pinctrl@d401e000 {
> compatible = "spacemit,k1-pinctrl";
> reg = <0x0 0xd401e000 0x0 0x400>;
> --
> 2.43.0
>

--
Yixun Lan (dlan)