| Message ID | 20220917042721.527345-1-sergio.paracuellos@gmail.com |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | [v2,1/2] dt-bindings: i2c: migrate mt7621 text bindings to YAML | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | warning | total: 0 errors, 2 warnings, 68 lines checked |
| robh/patch-applied | success | |
| robh/dtbs-check | warning | build log |
| robh/dt-meta-schema | success |
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@gmail.com> > --- Again, do not base your patches on something old. > 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@denx.de> > + > +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. > + > + '#size-cells': > + const: 0 No need > + > + clocks: > + maxItems: 1 > + > + clock-names: > + const: i2c Why adding this? You need to describe in commit msg all deviations from pure conversion. > + > + resets: > + maxItems: 1 > + > + reset-names: > + const: i2c Why adding this? > + > +required: > + - compatible > + - reg > + - resets > + - reset-names Why? > + - "#address-cells" > + - "#size-cells" Use the same type of quote as in other places. > + > +unevaluatedProperties: false > + Best regards, Krzysztof
On 17/09/2022 06:27, Sergio Paracuellos wrote: > Mediatek MT7621 i2c driver Bindings have been migrated from text > to YAML. Hence, properly update this file accordingly. So you allow to have a warning added in patch #1 and then you fix it? > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com> > --- > Changes in v2: > - Add this new patch to the series to align MAINTAINERS files > with new YAML doc > > MAINTAINERS | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 08620b9a44fc..bac21d599181 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -12669,7 +12669,7 @@ MEDIATEK MT7621/28/88 I2C DRIVER > M: Stefan Roese <sr@denx.de> > L: linux-i2c@vger.kernel.org > S: Maintained > -F: Documentation/devicetree/bindings/i2c/i2c-mt7621.txt > +F: Documentation/devicetree/bindings/i2c/mediatek,mt7621-i2c.yaml This must be squashed, otherwise it is not bisectable and you have warnings. Best regards, Krzysztof
Hi Krzysztof, Thanks for the review. On Mon, Sep 19, 2022 at 1:20 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 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@gmail.com> > > --- > > 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@denx.de> > > + > > +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
On Mon, Sep 19, 2022 at 1:25 PM Krzysztof Kozlowski <krzk@kernel.org> wrote: > > On 17/09/2022 06:27, Sergio Paracuellos wrote: > > Mediatek MT7621 i2c driver Bindings have been migrated from text > > to YAML. Hence, properly update this file accordingly. > > So you allow to have a warning added in patch #1 and then you fix it? > > > > > Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com> > > --- > > Changes in v2: > > - Add this new patch to the series to align MAINTAINERS files > > with new YAML doc > > > > MAINTAINERS | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 08620b9a44fc..bac21d599181 100644 > > --- a/MAINTAINERS > > +++ b/MAINTAINERS > > @@ -12669,7 +12669,7 @@ MEDIATEK MT7621/28/88 I2C DRIVER > > M: Stefan Roese <sr@denx.de> > > L: linux-i2c@vger.kernel.org > > S: Maintained > > -F: Documentation/devicetree/bindings/i2c/i2c-mt7621.txt > > +F: Documentation/devicetree/bindings/i2c/mediatek,mt7621-i2c.yaml > > This must be squashed, otherwise it is not bisectable and you have warnings. Understood will squash both patches together in the next version. Thanks, Sergio Paracuellos > > > Best regards, > Krzysztof
On 19/09/2022 13:49, Sergio Paracuellos wrote: > Hi Krzysztof, > > Thanks for the review. > > On Mon, Sep 19, 2022 at 1:20 PM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> 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@gmail.com> >>> --- >> >> 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?? You keep cc-ing my address which was changed in mainline around half a year ago. Patches end up in different mailbox. (...) >> >>> + >>> + 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? No, unevaluatedProperties must stay false. As I said: "You need to describe in commit msg all deviations from pure conversion." I did not say preferred is to make pure conversion... Best regards, Krzysztof
On Mon, Sep 19, 2022 at 2:00 PM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > > On 19/09/2022 13:49, Sergio Paracuellos wrote: > > Hi Krzysztof, > > > > Thanks for the review. > > > > On Mon, Sep 19, 2022 at 1:20 PM Krzysztof Kozlowski > > <krzysztof.kozlowski@linaro.org> 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@gmail.com> > >>> --- > >> > >> 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?? > > You keep cc-ing my address which was changed in mainline around half a > year ago. Patches end up in different mailbox. Sorry for inconvenience, will cc correct address on next version. > > > (...) > > >> > >>> + > >>> + 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? > > No, unevaluatedProperties must stay false. As I said: > "You need to describe in commit msg all deviations from pure conversion." > > I did not say preferred is to make pure conversion... Pretty clear, thanks. > > Best regards, > Krzysztof Best regards, Sergio Paracuellos
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@denx.de> + +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 + + '#size-cells': + const: 0 + + clocks: + maxItems: 1 + + clock-names: + const: i2c + + resets: + maxItems: 1 + + reset-names: + const: i2c + +required: + - compatible + - reg + - resets + - reset-names + - "#address-cells" + - "#size-cells" + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/clock/mt7621-clk.h> + #include <dt-bindings/reset/mt7621-reset.h> + + i2c: i2c@900 { + compatible = "mediatek,mt7621-i2c"; + reg = <0x900 0x100>; + clocks = <&sysc MT7621_CLK_I2C>; + clock-names = "i2c"; + resets = <&sysc MT7621_RST_I2C>; + reset-names = "i2c"; + + #address-cells = <1>; + #size-cells = <0>; + + pinctrl-names = "default"; + pinctrl-0 = <&i2c_pins>; + };
SoC MT7621 I2C bindings used text format, so migrate them to YAML. Signed-off-by: Sergio Paracuellos <sergio.paracuellos@gmail.com> --- 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