Hi Krzysztof,
Thanks for the review.
On Mon, Sep 19, 2022 at 1:20 PM Krzysztof Kozlowski
<krzysztof.kozlowski@xxxxxxxxxx> wrote:
>
> On 17/09/2022 06:27, Sergio Paracuellos wrote:
> > SoC MT7621 I2C bindings used text format, so migrate them to YAML.
> >
> > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@xxxxxxxxx>
> > ---
>
> Again, do not base your patches on something old.
I will take it into account from now on. Since it was just an addition
and removal of a file I thought it was not important. So, I guess some
address I am using in CC is not listed in the MAINTAINERS file now??
>
> > Changes in v2:
> > - Maintain current maintainer Stefan Rose as listed in kernel MAINTAINERS file.
> >
> > .../devicetree/bindings/i2c/i2c-mt7621.txt | 25 -------
> > .../bindings/i2c/mediatek,mt7621-i2c.yaml | 68 +++++++++++++++++++
> > 2 files changed, 68 insertions(+), 25 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/i2c/i2c-mt7621.txt
> > create mode 100644 Documentation/devicetree/bindings/i2c/mediatek,mt7621-i2c.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/i2c/i2c-mt7621.txt b/Documentation/devicetree/bindings/i2c/i2c-mt7621.txt
> > deleted file mode 100644
> > index bc36f0eb94cd..000000000000
> > --- a/Documentation/devicetree/bindings/i2c/i2c-mt7621.txt
> > +++ /dev/null
> > @@ -1,25 +0,0 @@
> > -MediaTek MT7621/MT7628 I2C master controller
> > -
> > -Required properties:
> > -
> > -- compatible: Should be one of the following:
> > - - "mediatek,mt7621-i2c": for MT7621/MT7628/MT7688 platforms
> > -- #address-cells: should be 1.
> > -- #size-cells: should be 0.
> > -- reg: Address and length of the register set for the device
> > -- resets: phandle to the reset controller asserting this device in
> > - reset
> > - See ../reset/reset.txt for details.
> > -
> > -Optional properties :
> > -
> > -Example:
> > -
> > -i2c: i2c@900 {
> > - compatible = "mediatek,mt7621-i2c";
> > - reg = <0x900 0x100>;
> > - #address-cells = <1>;
> > - #size-cells = <0>;
> > - resets = <&rstctrl 16>;
> > - reset-names = "i2c";
> > -};
> > diff --git a/Documentation/devicetree/bindings/i2c/mediatek,mt7621-i2c.yaml b/Documentation/devicetree/bindings/i2c/mediatek,mt7621-i2c.yaml
> > new file mode 100644
> > index 000000000000..8234f770f529
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/i2c/mediatek,mt7621-i2c.yaml
> > @@ -0,0 +1,68 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/i2c/mediatek,mt7621-i2c.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +maintainers:
> > + - Stefan Roese <sr@xxxxxxx>
> > +
> > +title: Mediatek MT7621/MT7628 I2C master controller
> > +
> > +allOf:
> > + - $ref: /schemas/i2c/i2c-controller.yaml#
> > +
> > +properties:
> > + compatible:
> > + const: mediatek,mt7621-i2c
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + '#address-cells':
> > + const: 1
>
> No need, comes from core schema.
Will drop, then.
>
> > +
> > + '#size-cells':
> > + const: 0
>
> No need
ditto.
>
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + clock-names:
> > + const: i2c
>
> Why adding this?
>
> You need to describe in commit msg all deviations from pure conversion.
Looking into the users of this binding I added all the stuff I found
in dts nodes. So I think it is preferred to just make a pure
conversion and set unevaluatedProperties to true?
>
> > +
> > + resets:
> > + maxItems: 1
> > +
> > + reset-names:
> > + const: i2c
>
>
> Why adding this?
ditto.
>
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - resets
> > + - reset-names
>
> Why?
Will only required those included in a pure conversion of the txt, thanks.
>
> > + - "#address-cells"
> > + - "#size-cells"
>
> Use the same type of quote as in other places.
Understood.
>
> > +
> > +unevaluatedProperties: false
> > +
>
> Best regards,
> Krzysztof
Thanks,
Sergio Paracuellos
