| Message ID | 20241015075134.1449458-2-TroyMitchell988@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Andi Shyti |
| Headers | show |
| Series | riscv: spacemit: add i2c support to K1 SoC | expand |
On 15/10/2024 09:51, Troy Mitchell wrote: > The i2c of K1 supports fast-speed-mode and high-speed-mode, s/i2c/I2C/ > and supports FIFO transmission. > > Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com> > --- > .../bindings/i2c/spacemit,k1-i2c.yaml | 59 +++++++++++++++++++ > 1 file changed, 59 insertions(+) > create mode 100644 Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml > > 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 000000000000..c1460ec2b323 > --- /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 There is no such vendor prefix. > + > + reg: > + maxItems: 1 > + > + interrupts: > + maxItems: 1 > + > + clocks: > + maxItems: 1 > + > + clock-frequency: > + description: > + Desired I2C bus clock frequency in Hz. As only fast and high-speed > + modes are supported by hardware, possible values are 100000 and 400000. > + enum: [100000, 400000] > + default: 100000 > + > + fifo-disable: Why is this a property of a board? Also, missing vendor prefix. > + type: boolean > + description: > + Whether to disable FIFO. If FIFO is turned on, it will be interrupted > + only when the FIFO depth is reached, which can reduce the frequency > + of interruption. > + default: false Drop > + > +unevaluatedProperties: false This goes after required: block. > + > +required: > + - compatible > + - reg > + - interrupts > + - clocks > + > +examples: > + - | > + i2c0: i2c@d4010800 { Drop unused alias > + compatible = "spacemit,k1-i2c"; Best regards, Krzysztof
On Tue, 15 Oct 2024 15:51:33 +0800, Troy Mitchell wrote: > The i2c of K1 supports fast-speed-mode and high-speed-mode, > and supports FIFO transmission. > > Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com> > --- > .../bindings/i2c/spacemit,k1-i2c.yaml | 59 +++++++++++++++++++ > 1 file changed, 59 insertions(+) > create mode 100644 Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml > My bot found errors running 'make dt_binding_check' on your patch: yamllint warnings/errors: dtschema/dtc warnings/errors: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.example.dtb: i2c@d4010800: reg: [[0, 3556837376], [0, 56]] is too long from schema $id: http://devicetree.org/schemas/i2c/spacemit,k1-i2c.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.example.dtb: i2c@d4010800: Unevaluated properties are not allowed ('reg' was unexpected) from schema $id: http://devicetree.org/schemas/i2c/spacemit,k1-i2c.yaml# doc reference errors (make refcheckdocs): See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20241015075134.1449458-2-TroyMitchell988@gmail.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
On Tue, Oct 15, 2024 at 10:02:21AM +0200, Krzysztof Kozlowski wrote: > On 15/10/2024 09:51, Troy Mitchell wrote: > > The i2c of K1 supports fast-speed-mode and high-speed-mode, > > s/i2c/I2C/ > > > and supports FIFO transmission. > > > > Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com> > > --- > > .../bindings/i2c/spacemit,k1-i2c.yaml | 59 +++++++++++++++++++ > > 1 file changed, 59 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml > > > > 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 000000000000..c1460ec2b323 > > --- /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 > > There is no such vendor prefix. 7cf3e9bfc63db ("dt-bindings: vendor-prefixes: add spacemit") will be in tomorrow's next.
On 2024/10/15 16:02, Krzysztof Kozlowski wrote: > On 15/10/2024 09:51, Troy Mitchell wrote: >> The i2c of K1 supports fast-speed-mode and high-speed-mode, > > s/i2c/I2C/ > >> and supports FIFO transmission. >> >> Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com> >> --- >> .../bindings/i2c/spacemit,k1-i2c.yaml | 59 +++++++++++++++++++ >> 1 file changed, 59 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml >> >> 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 000000000000..c1460ec2b323 >> --- /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 > > There is no such vendor prefix. > >> + >> + reg: >> + maxItems: 1 >> + >> + interrupts: >> + maxItems: 1 >> + >> + clocks: >> + maxItems: 1 >> + >> + clock-frequency: >> + description: >> + Desired I2C bus clock frequency in Hz. As only fast and high-speed >> + modes are supported by hardware, possible values are 100000 and 400000. >> + enum: [100000, 400000] >> + default: 100000 >> + >> + fifo-disable: > > Why is this a property of a board? > > Also, missing vendor prefix. > > >> + type: boolean >> + description: >> + Whether to disable FIFO. If FIFO is turned on, it will be interrupted >> + only when the FIFO depth is reached, which can reduce the frequency >> + of interruption. >> + default: false > > Drop It's a hardware FIFO instead of software. Is it unnecessary in this file? If is, why dma can be written in dt-binding. > >> + >> +unevaluatedProperties: false > > This goes after required: block. > > >> + >> +required: >> + - compatible >> + - reg >> + - interrupts >> + - clocks >> + >> +examples: >> + - | >> + i2c0: i2c@d4010800 { > > Drop unused alias > >> + compatible = "spacemit,k1-i2c"; > > Best regards, > Krzysztof > Best regards, Troy
On 16/10/2024 04:45, Troy Mitchell wrote: >>> + >>> + clock-frequency: >>> + description: >>> + Desired I2C bus clock frequency in Hz. As only fast and high-speed >>> + modes are supported by hardware, possible values are 100000 and 400000. >>> + enum: [100000, 400000] >>> + default: 100000 >>> + >>> + fifo-disable: >> >> Why is this a property of a board? Here, this ^^^^^^ >> >> Also, missing vendor prefix. >> >> >>> + type: boolean >>> + description: >>> + Whether to disable FIFO. If FIFO is turned on, it will be interrupted >>> + only when the FIFO depth is reached, which can reduce the frequency >>> + of interruption. >>> + default: false >> >> Drop > > It's a hardware FIFO instead of software. > Is it unnecessary in this file? > If is, why dma can be written in dt-binding. Because of what I asked earlier. Which 'dma' property are you asking about? 'use-dma'? There was rationale provided in favor. I would be more than happy to see similar rationale here. Best regards, Krzysztof
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 000000000000..c1460ec2b323 --- /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: + maxItems: 1 + + clock-frequency: + description: + Desired I2C bus clock frequency in Hz. As only fast and high-speed + modes are supported by hardware, possible values are 100000 and 400000. + enum: [100000, 400000] + default: 100000 + + fifo-disable: + type: boolean + description: + Whether to disable FIFO. If FIFO is turned on, it will be interrupted + only when the FIFO depth is reached, which can reduce the frequency + of interruption. + default: false + +unevaluatedProperties: false + +required: + - compatible + - reg + - interrupts + - clocks + +examples: + - | + i2c0: i2c@d4010800 { + compatible = "spacemit,k1-i2c"; + reg = <0x0 0xd4010800 0x0 0x38>; + interrupt-parent = <&plic>; + interrupts = <36>; + clocks = <&ccu 90>; + clock-frequency = <100000>; + }; + +...
The i2c of K1 supports fast-speed-mode and high-speed-mode, and supports FIFO transmission. Signed-off-by: Troy Mitchell <TroyMitchell988@gmail.com> --- .../bindings/i2c/spacemit,k1-i2c.yaml | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 Documentation/devicetree/bindings/i2c/spacemit,k1-i2c.yaml