Re: [PATCH RESEND v5 1/2] dt-bindings: i2c: spacemit: add support for K1 SoC

Yixun Lan
2025-03-03T09:35:17Z

On 13:30 Mon 03 Mar , Troy Mitchell wrote:
> The I2C of K1 supports fast-speed-mode and high-speed-mode,
> and supports FIFO transmission.
>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> Signed-off-by: Troy Mitchell <troymitchell988@gmail.com>
> ---
> .../devicetree/bindings/i2c/spacemit,k1-i2c.yaml | 59 ++++++++++++++++++++++
> 1 file changed, 59 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml b/Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml
> new file mode 100644
> index 0000000000000000000000000000000000000000..db49f1f473e6f166f534b276c86b3951d86341c3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml
> @@ -0,0 +1,59 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/i2c/spacemit,k1-i2c.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: I2C controller embedded in SpacemiT's K1 SoC
> +
> +maintainers:
> + - Troy Mitchell <troymitchell988@gmail.com>
> +
> +properties:
> + compatible:
> + const: spacemit,k1-i2c
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
..
> + clocks:
> + minItems: 2
> + maxItems: 2
> +
> + clock-names:
> + minItems: 2
> + maxItems: 2
I'd suggest to give a brief description and explicit clock name here,
you can consult marvell,mv64xxx-i2c.yaml for example

> +
> + clock-frequency:
> + description: |
> + K1 support three different modes which running different frequencies
> + standard speed mode: up to 100000 (100Hz)
> + fast speed mode : up to 400000 (400Hz)
> + high speed mode : up to 3300000 (3.3Mhz)
> + default: 400000
> + maximum: 3300000
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + i2c@d4010800 {
> + compatible = "spacemit,k1-i2c";
> + reg = <0xd4010800 0x38>;
> + interrupt-parent = <&plic>;
> + interrupts = <36>;
> + clocks = <&ccu 176>, <&ccu 90>;
> + clock-names = "apb", "twsi";
9.1.4.61 TWSI0 CLOCK RESET CONTROL REGISTER(APBC_TWSI0_CLK_RST)
https://developer.spacemit.com/documentation?token=LCrKwWDasiJuROkVNusc2pWTnEb#part594
from above docs, there are two clocks
bit[1] - FNCLK, TWSI0 Functional Clock Enable/Disable
bit[0] - APBCLK, TWSI0 APB Bus Clock Enable/Disable

I'd suggest to name it according to the functionality, thus 'func', 'bus'
clock, not its source.. which would make it more system wide consistent

> + clock-frequency = <100000>;
> + };
> +
> +...
>
> --
> 2.34.1
>

--
Yixun Lan (dlan)
Gentoo Linux Developer
GPG Key ID AABEFD55