Re: [RFC PATCH] dt-bindings: Add AST2600 i3c controller binding
From: Krzysztof Kozlowski <hidden>
Date: 2023-02-13 08:43:04
Also in:
linux-aspeed, linux-i3c
On 13/02/2023 08:41, Jeremy Kerr wrote:
This change adds a devicetree binding for the ast2600 i3c controller
1. Do not use "This commit/patch". https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95 2. Use subject prefixes matching the subsystem (which you can get for example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory your patch is touching). 3. Subject: drop second/last, redundant "binding". The "dt-bindings" prefix is already stating that these are bindings. 4. Where is the driver? Where is the DTS? Why do we want unused binding in the kernel?
quoted hunk ↗ jump to hunk
hardware. This is heavily based on the designware i3c hardware, plus a reset facility and two platform-specific properties: - sda-pullup-ohms: to specify the value of the configurable pullup resistors on the SDA line - global-regs: to reference the (ast2600-specific) i3c global register block, and the device index to use within it. Signed-off-by: Jeremy Kerr <jk@codeconstruct.com.au> --- RFC: the example in this depends on some not-yet-accepted patches for the clock and reset linkages: https://lore.kernel.org/linux-devicetree/cover.1676267865.git.jk@codeconstruct.com.au/T/ (local) I'm also keen to get some review on the pullup configuration too. --- .../bindings/i3c/aspeed,ast2600-i3c.yaml | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c.yamldiff --git a/Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c.yaml b/Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c.yaml new file mode 100644 index 000000000000..ef28a8b77c94 --- /dev/null +++ b/Documentation/devicetree/bindings/i3c/aspeed,ast2600-i3c.yaml@@ -0,0 +1,75 @@ +# SPDX-License-Identifier: GPL-2.0 +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/i3c/aspeed,ast2600-i3c.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: ASPEED AST2600 i3c controller + +maintainers: + - Jeremy Kerr <jk@codeconstruct.com.au> + +allOf: + - $ref: i3c.yaml# + +properties: + compatible: + const: aspeed,ast2600-i3c + + reg: + maxItems: 1 + + clocks: + maxItems: 1 + + resets: + maxItems: 1 + + interrupts: + maxItems: 1 + + sda-pullup-ohms: + enum: [545, 750, 2000] + default: 2000 + description: | + Value of SDA pullup resistor in Ohms
Why this is property of i3c, not pinctrl?
+ + global-regs:
Missing vendor prefix
+ $ref: /schemas/types.yaml#/definitions/phandle-array + description: | + A (phandle, controller index) reference to the i3c global register set + used for this device.
Missing items description: https://elixir.bootlin.com/linux/v5.18-rc1/source/Documentation/devicetree/bindings/soc/samsung/exynos-usi.yaml#L42
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - interrupts
+ - global-regs
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/ast2600-clock.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ i3c-master@2000 {
+ #address-cells = <3>;
+ #size-cells = <0>;
+ compatible = "aspeed,ast2600-i3c";
+ reg = <0x2000 0x1000>;compatible and reg are first properties.
+ clocks = <&syscon ASPEED_CLK_GATE_I3C0CLK>;
+ resets = <&syscon ASPEED_RESET_I3C0>;
+ global-regs = <&i3c_global 0>;
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_i3c1_default>;
+ interrupts = <GIC_SPI 102 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
+ i3c_global: i3c-global@0 {Drop node, unrelated.
+ compatible = "aspeed,ast2600-i3c-global", "simple-mfd", "syscon"; + resets = <&syscon ASPEED_RESET_I3C_DMA>; + reg = <0x0 0x1000>; + }; +...
Best regards, Krzysztof