Message ID | 20220425084800.2021-2-allen-kh.cheng@mediatek.com |
---|---|
State | Changes Requested, archived |
Headers | show |
Series | dt-bindings: nvmem: mediatek: Convert mtk-efuse binding to YAML | expand |
Context | Check | Description |
---|---|---|
robh/checkpatch | warning | total: 0 errors, 1 warnings, 70 lines checked |
robh/patch-applied | success | |
robh/dtbs-check | warning | build log |
robh/dt-meta-schema | success |
On 25/04/2022 10:48, Allen-KH Cheng wrote: > Convert MediaTek eFuse devicetree binding to YAML. > > Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com> > --- > .../devicetree/bindings/nvmem/mtk,efuse.yaml | 70 +++++++++++++++++++ The vendor prefix is mediatek. Quoting my previous reply: Same comments as usual, so "vendor,device-name", e.g. "mediatek,efuse" if this is going to match all possible future MediaTek chips or "mediatek,mt7622-efuse" so keep it mediatek,efuse.yaml. > .../devicetree/bindings/nvmem/mtk-efuse.txt | 43 ------------ > 2 files changed, 70 insertions(+), 43 deletions(-) > create mode 100644 Documentation/devicetree/bindings/nvmem/mtk,efuse.yaml > delete mode 100644 Documentation/devicetree/bindings/nvmem/mtk-efuse.txt > > diff --git a/Documentation/devicetree/bindings/nvmem/mtk,efuse.yaml b/Documentation/devicetree/bindings/nvmem/mtk,efuse.yaml > new file mode 100644 > index 000000000000..d056bc61dd5b > --- /dev/null > +++ b/Documentation/devicetree/bindings/nvmem/mtk,efuse.yaml > @@ -0,0 +1,70 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/nvmem/mtk,efuse.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: MediaTek eFuse device tree bindings No changes here. Please read my comments from your v1, don't ignore them. > + > +maintainers: > + - Lala Lin <lala.lin@mediatek.com> > + - Allen-KH Cheng <allen-kh.cheng@mediatek.com> > + > +allOf: > + - $ref: "nvmem.yaml#" > + > +properties: > + compatible: > + oneOf: > + - enum: > + - mediatek,mt8173-efuse > + - mediatek,efuse Still no changes... > + - items: > + - enum: > + - mediatek,mt7622-efuse > + - mediatek,mt7623-efuse > + - mediatek,mt8183-efuse > + - mediatek,mt8192-efuse > + - mediatek,mt8195-efuse > + - mediatek,mt8516-efuse > + - const: mediatek,efuse > + > + reg: > + maxItems: 1 > + > +patternProperties: > + "^.*@[0-9a-f]+$": > + type: object > + > + properties: > + reg: > + maxItems: 1 > + description: > + Offset and size in bytes within the storage device. > + > + required: > + - reg > + > + additionalProperties: false Still no changes. > + > +required: > + - compatible > + - reg > + > +unevaluatedProperties: false > + > +examples: > + - | > + Still no changes. Best regards, Krzysztof
On 26/04/2022 08:23, allen-kh.cheng wrote: >>> +properties: >>> + compatible: >>> + oneOf: >>> + - enum: >>> + - mediatek,mt8173-efuse >>> + - mediatek,efuse >> >> Still no changes... >> > > I just want to confirm again. > > "Generic compatibles should not be used standalone" > > It seems we should remove mediatek,efuse and keep "mediatek,mt8173- > efuse"in binding. have I got that right? You should comment for which chipsets this compatible is and add a deprecated:true. In such case it cannot be part of enum but separate item in this oneOf. Best regards, Krzysztof
On 26/04/2022 12:02, allen-kh.cheng wrote: > Hi Krzysztof, > > On Tue, 2022-04-26 at 08:31 +0200, Krzysztof Kozlowski wrote: >> On 26/04/2022 08:23, allen-kh.cheng wrote: >>>>> +properties: >>>>> + compatible: >>>>> + oneOf: >>>>> + - enum: >>>>> + - mediatek,mt8173-efuse >>>>> + - mediatek,efuse >>>> >>>> Still no changes... >>>> >>> >>> I just want to confirm again. >>> >>> "Generic compatibles should not be used standalone" >>> >>> It seems we should remove mediatek,efuse and keep "mediatek,mt8173- >>> efuse"in binding. have I got that right? >> >> You should comment for which chipsets this compatible is and add a >> deprecated:true. In such case it cannot be part of enum but separate >> item in this oneOf. >> >> >> Best regards, >> Krzysztof > > Thanks for your suggestions, I would plan to send PATCHs as below, > > We have a PATCH 01 for current accepted dts > > properties: > compatible: > oneOf: > - enum: > - mediatek,efuse > > - mediatek,mt8173-efuse > description: Only mt8173-efuse > with generic fallback should be used > - items: > - enum: > > - mediatek,mt7622-efuse > ... > - const: mediatek,efuse > > Then add PATCH 02 to deprecate it > > properties: > compatible: > oneOf: > - enum: > - mediatek,efuse > - mediatek,mt8173-efuse > deprecated: true > description: The mediatek,efuse is a generic fallback for other > Chipset. Do not use the single compatible such as mediatek,efuse > or mediatek,mt8173-efuse. It is deprecated. > - items: > - enum: > - mediatek,mt7622-efuse > ... > - const: mediatek,efuse > > > PATCH 03 for 8173 > > update mt8173.dtsi > > change compatible from "mediatek,mt8173-efuse" to "mediatek,mt8173- > efuse", "mediatek,efuse"; > > > Do you think it'd be okay ? The idea is correct, but as I said it cannot be part of enum, but separate item in oneOf. You should see an error when testing your patch. Best regards, Krzysztof
On 27/04/2022 11:28, allen-kh.cheng wrote: > Hi Krzysztof, > > On Tue, 2022-04-26 at 12:14 +0200, Krzysztof Kozlowski wrote: >> On 26/04/2022 12:02, allen-kh.cheng wrote: >>> Hi Krzysztof, >>> >>> On Tue, 2022-04-26 at 08:31 +0200, Krzysztof Kozlowski wrote: >>>> On 26/04/2022 08:23, allen-kh.cheng wrote: >>>>>>> +properties: >>>>>>> + compatible: >>>>>>> + oneOf: >>>>>>> + - enum: >>>>>>> + - mediatek,mt8173-efuse >>>>>>> + - mediatek,efuse >>>>>> >>>>>> Still no changes... >>>>>> >>>>> >>>>> I just want to confirm again. >>>>> >>>>> "Generic compatibles should not be used standalone" >>>>> >>>>> It seems we should remove mediatek,efuse and keep >>>>> "mediatek,mt8173- >>>>> efuse"in binding. have I got that right? >>>> >>>> You should comment for which chipsets this compatible is and add >>>> a >>>> deprecated:true. In such case it cannot be part of enum but >>>> separate >>>> item in this oneOf. >>>> >>>> >>>> Best regards, >>>> Krzysztof >>> >>> Thanks for your suggestions, I would plan to send PATCHs as below, >>> >>> We have a PATCH 01 for current accepted dts >>> >>> properties: >>> compatible: >>> oneOf: >>> - enum: >>> - mediatek,efuse >>> >>> - mediatek,mt8173-efuse >>> description: Only mt8173-efuse >>> with generic fallback should be used >>> - items: >>> - enum: >>> >>> - mediatek,mt7622-efuse >>> ... >>> - const: mediatek,efuse >>> >>> Then add PATCH 02 to deprecate it >>> >>> properties: >>> compatible: >>> oneOf: >>> - enum: >>> - mediatek,efuse >>> - mediatek,mt8173-efuse >>> deprecated: true >>> description: The mediatek,efuse is a generic fallback for >>> other >>> Chipset. Do not use the single compatible such as mediatek,efuse >>> or mediatek,mt8173-efuse. It is deprecated. >>> - items: >>> - enum: >>> - mediatek,mt7622-efuse >>> ... >>> - const: mediatek,efuse >>> >>> >>> PATCH 03 for 8173 >>> >>> update mt8173.dtsi >>> >>> change compatible from "mediatek,mt8173-efuse" to "mediatek,mt8173- >>> efuse", "mediatek,efuse"; >>> >>> >>> Do you think it'd be okay ? >> >> The idea is correct, but as I said it cannot be part of enum, but >> separate item in oneOf. You should see an error when testing your >> patch. >> >> >> Best regards, >> Krzysztof > > I have tested > make DT_CHECKER_FLAGS=-m dt_binding_check > DT_SCHEMA_FILES=Documentation/devicetree/bindings/nvmem/mtk,efuse.yaml > > make ARCH=arm64 dtbs_check > DT_SCHEMA_FILES=Documentation/devicetree/bindings/nvmem/mtk,efuse.yaml > > Is the following correct as final version ? Almost :) > > properties: > compatible: > oneOf: > - const: mediatek,mt8173-efuse > #Don't use this in new dts files This compatible above is correct for mt8173, isn't it? > deprecated: true > - const: > mediatek,efuse > deprecated: true > description: > > Please use mediatek,efuse as fallback. Description does not match. This should be something like: "MediaTek efuse for MT8173. Deprecated, use mediatek,mt8173-efuse instead" Best regards, Krzysztof
On 27/04/2022 12:00, allen-kh.cheng wrote: > I think there are two cases in mediatek efuse dirver now. > > Case 1, > const: mediatek,efuse is deprecated. > const: mediatek,mt8173-efuse is remained. All mediatek chipsets will > use mediatek,mt8173-efuse as fallback. > > Case 2, > const: mediatek,efuse is deprecated. > const: mediatek,mt8173-efuse is deprecated. > > All mediatek chipsets(include ediatek,mt8173-efuse) will use > mediatek,efuse as fallback. > > Which one do you think is better? Indeed, I forgot that mt8173 would also fallback to generic efuse. Indeed let's go with case 2, so your proposal before was correct. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/nvmem/mtk,efuse.yaml b/Documentation/devicetree/bindings/nvmem/mtk,efuse.yaml new file mode 100644 index 000000000000..d056bc61dd5b --- /dev/null +++ b/Documentation/devicetree/bindings/nvmem/mtk,efuse.yaml @@ -0,0 +1,70 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/nvmem/mtk,efuse.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: MediaTek eFuse device tree bindings + +maintainers: + - Lala Lin <lala.lin@mediatek.com> + - Allen-KH Cheng <allen-kh.cheng@mediatek.com> + +allOf: + - $ref: "nvmem.yaml#" + +properties: + compatible: + oneOf: + - enum: + - mediatek,mt8173-efuse + - mediatek,efuse + - items: + - enum: + - mediatek,mt7622-efuse + - mediatek,mt7623-efuse + - mediatek,mt8183-efuse + - mediatek,mt8192-efuse + - mediatek,mt8195-efuse + - mediatek,mt8516-efuse + - const: mediatek,efuse + + reg: + maxItems: 1 + +patternProperties: + "^.*@[0-9a-f]+$": + type: object + + properties: + reg: + maxItems: 1 + description: + Offset and size in bytes within the storage device. + + required: + - reg + + additionalProperties: false + +required: + - compatible + - reg + +unevaluatedProperties: false + +examples: + - | + + efuse: efuse@10206000 { + compatible = "mediatek,mt8173-efuse"; + reg = <0x10206000 0x1000>; + #address-cells = <1>; + #size-cells = <1>; + + /* Data cells */ + thermal_calibration: calib@528 { + reg = <0x528 0xc>; + }; + }; +... diff --git a/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt b/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt deleted file mode 100644 index 39d529599444..000000000000 --- a/Documentation/devicetree/bindings/nvmem/mtk-efuse.txt +++ /dev/null @@ -1,43 +0,0 @@ -= Mediatek MTK-EFUSE device tree bindings = - -This binding is intended to represent MTK-EFUSE which is found in most Mediatek SOCs. - -Required properties: -- compatible: should be - "mediatek,mt7622-efuse", "mediatek,efuse": for MT7622 - "mediatek,mt7623-efuse", "mediatek,efuse": for MT7623 - "mediatek,mt8173-efuse" or "mediatek,efuse": for MT8173 - "mediatek,mt8192-efuse", "mediatek,efuse": for MT8192 - "mediatek,mt8195-efuse", "mediatek,efuse": for MT8195 - "mediatek,mt8516-efuse", "mediatek,efuse": for MT8516 -- reg: Should contain registers location and length -- bits: contain the bits range by offset and size - -= Data cells = -Are child nodes of MTK-EFUSE, bindings of which as described in -bindings/nvmem/nvmem.txt - -Example: - - efuse: efuse@10206000 { - compatible = "mediatek,mt8173-efuse"; - reg = <0 0x10206000 0 0x1000>; - #address-cells = <1>; - #size-cells = <1>; - - /* Data cells */ - thermal_calibration: calib@528 { - reg = <0x528 0xc>; - }; - }; - -= Data consumers = -Are device nodes which consume nvmem data cells. - -For example: - - thermal { - ... - nvmem-cells = <&thermal_calibration>; - nvmem-cell-names = "calibration"; - };
Convert MediaTek eFuse devicetree binding to YAML. Signed-off-by: Allen-KH Cheng <allen-kh.cheng@mediatek.com> --- .../devicetree/bindings/nvmem/mtk,efuse.yaml | 70 +++++++++++++++++++ .../devicetree/bindings/nvmem/mtk-efuse.txt | 43 ------------ 2 files changed, 70 insertions(+), 43 deletions(-) create mode 100644 Documentation/devicetree/bindings/nvmem/mtk,efuse.yaml delete mode 100644 Documentation/devicetree/bindings/nvmem/mtk-efuse.txt