| Message ID | 20240808105722.7222-1-macpaul.lin@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | dt-bindings: mfd: mediatek: mt6397: Convert to DT schema format | expand |
On 08/08/2024 12:57, Macpaul Lin wrote: > Convert the mfd: mediatek: mt6397 binding to DT schema format. > > New updates in this conversion: > - Align generic names of DT schema "audio-codec" and "regulators". > - mt6397-regulators: Replace the "txt" reference with newly added DT > schema. > > Signed-off-by: Sen Chu <sen.chu@mediatek.com> > Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com> > --- > .../bindings/mfd/mediatek,mt6397.yaml | 202 ++++++++++++++++++ > .../devicetree/bindings/mfd/mt6397.txt | 110 ---------- You are doing conversions in odd order... and ignore my comments. The example from your regulator binding is supposed to be here - I wrote it last time. Due to doing changes totally unsynchronized, this CANNOT be merged without unnecessary maintainer coordination, because of dependency. Sorry, that's not how it works for MFD devices. Perform conversion of entire device in ONE patchset. > 2 files changed, 202 insertions(+), 110 deletions(-) > create mode 100644 Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml > delete mode 100644 Documentation/devicetree/bindings/mfd/mt6397.txt > > Changes for v1: > - This patch depends on conversion of mediatek,mt6397-regulator.yaml > [1] https://lore.kernel.org/lkml/20240807091738.18387-1-macpaul.lin@mediatek.com/T/ > > diff --git a/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml b/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml > new file mode 100644 > index 0000000..cfbf2215 > --- /dev/null > +++ b/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml > @@ -0,0 +1,202 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/mfd/mediatek,mt6397.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: MediaTek MT6397/MT6323 Multifunction Device > + > +maintainers: > + - Sen Chu <sen.chu@mediatek.com> > + - Macpaul Lin <macpaul.lin@mediatek.com> > + > +description: | > + MT6397/MT6323 is a multifunction device with the following sub modules: MFD is Linuxism, avoid it. > + - Regulator > + - RTC > + - Audio codec > + - GPIO > + - Clock > + - LED > + - Keys > + - Power controller > + > + It is interfaced to host controller using SPI interface by a proprietary hardware > + called PMIC wrapper or pwrap. MT6397/MT6323 MFD is a child device of pwrap. > + See the following for pwarp node definitions: > + ../soc/mediatek/mediatek,pwrap.yaml Drop, instead add proper ref or compatible in parent node. > + > + This document describes the binding for MFD device and its sub module. Drop > + > +properties: > + compatible: > + oneOf: > + - enum: > + - mediatek,mt6323 > + - mediatek,mt6331 # "mediatek,mt6331" for PMIC MT6331 and MT6332. > + - mediatek,mt6357 > + - mediatek,mt6358 > + - mediatek,mt6359 > + - mediatek,mt6397 > + - items: > + - enum: > + - mediatek,mt6366 # "mediatek,mt6366", "mediatek,mt6358" for PMIC MT6366 Drop comment, it is obvious. Don't repeat constraints in free form text. > + - const: mediatek,mt6358 > + > + interrupts: > + maxItems: 1 > + > + interrupt-controller: true > + > + "#interrupt-cells": > + const: 2 > + > + rtc: > + type: object > + unevaluatedProperties: false > + description: > + Real Time Clock (RTC) > + See ../rtc/rtc-mt6397.txt No, convert the binding. > + properties: > + compatible: > + oneOf: > + - enum: > + - mediatek,mt6323-rtc > + - mediatek,mt6331-rtc > + - mediatek,mt6358-rtc > + - mediatek,mt6397-rtc > + - items: > + - enum: > + - mediatek,mt6366-rtc # RTC MT6366 Drop all such comments. > + - const: mediatek,mt6358-rtc > + > + regulators: > + type: object > + oneOf: > + - $ref: /schemas/regulator/mediatek,mt6358-regulator.yaml > + - $ref: /schemas/regulator/mediatek,mt6397-regulator.yaml And how is it supposed to be tested? > + unevaluatedProperties: false > + description: > + Regulators > + For mt6323, see ../regulator/mt6323-regulator.txt Drop, useless. > + properties: > + compatible: > + oneOf: > + - enum: > + - mediatek,mt6323-regulator > + - mediatek,mt6358-regulator > + - mediatek,mt6397-regulator > + - items: > + - enum: > + - mediatek,mt6366-regulator # Regulator MT6366 > + - const: mediatek,mt6358-regulator > + > + audio-codec: > + type: object > + unevaluatedProperties: false This does not make sense. You do not have any ref here. > + description: > + Audio codec > + properties: > + compatible: > + oneOf: > + - enum: > + - mediatek,mt6397-codec > + - mediatek,mt6358-sound > + - items: > + - enum: > + - mediatek,mt6366-sound # Codec MT6366 > + - const: mediatek,mt6358-sound This wasn't in the old binding. Commit msg also does not explain why you are doing changes from conversion. > + > + clk: > + type: object > + unevaluatedProperties: false Again, no, it does not work like this. See example schema for explanation of this. Convert all children - entire device. Then either use ref or additionalProperties: true. See Qualcomm mdss bindings for example. > + description: > + Clock Your descriptions are useless. You just said "clk" node is "clock". Really? > + properties: > + compatible: > + const: mediatek,mt6397-clk > + > + led: > + type: object > + unevaluatedProperties: false > + description: > + LED > + See ../leds/leds-mt6323.txt for more information No > + properties: > + compatible: > + const: mediatek,mt6323-led > + > + keys: > + type: object > + $ref: /schemas/input/mediatek,pmic-keys.yaml > + unevaluatedProperties: false > + description: Keys Keys are keys? Could keys be anything else? > + > +additionalProperties: false > + > +examples: > + - | > + #include <dt-bindings/interrupt-controller/arm-gic.h> > + > + pwrap { Drop > + pmic { > + compatible = "mediatek,mt6397"; > + interrupts-extended = <&pio 222 IRQ_TYPE_LEVEL_HIGH>; > + interrupt-controller; > + #interrupt-cells = <2>; > + > + mt6397_codec: audio-codec { > + compatible = "mediatek,mt6397-codec"; > + }; > + > + mt6397_regulators: regulators { > + compatible = "mediatek,mt6397-regulator"; > + > + mt6397_vpca15_reg: buck_vpca15 { > + regulator-name = "vpca15"; > + regulator-min-microvolt = <850000>; > + regulator-max-microvolt = <1400000>; > + regulator-ramp-delay = <12500>; > + regulator-always-on; > + }; > + > + mt6397_vgp4_reg: ldo_vgp4 { > + regulator-name = "vgp4"; > + regulator-min-microvolt = <1200000>; > + regulator-max-microvolt = <3300000>; > + regulator-enable-ramp-delay = <218>; > + }; > + }; Incomplete. The parent device example is supposed to be 100% complete. Best regards, Krzysztof
On Thu, 08 Aug 2024 18:57:22 +0800, Macpaul Lin wrote: > Convert the mfd: mediatek: mt6397 binding to DT schema format. > > New updates in this conversion: > - Align generic names of DT schema "audio-codec" and "regulators". > - mt6397-regulators: Replace the "txt" reference with newly added DT > schema. > > Signed-off-by: Sen Chu <sen.chu@mediatek.com> > Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com> > --- > .../bindings/mfd/mediatek,mt6397.yaml | 202 ++++++++++++++++++ > .../devicetree/bindings/mfd/mt6397.txt | 110 ---------- > 2 files changed, 202 insertions(+), 110 deletions(-) > create mode 100644 Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml > delete mode 100644 Documentation/devicetree/bindings/mfd/mt6397.txt > > Changes for v1: > - This patch depends on conversion of mediatek,mt6397-regulator.yaml > [1] https://lore.kernel.org/lkml/20240807091738.18387-1-macpaul.lin@mediatek.com/T/ > 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/mfd/mediatek,mt6397.example.dtb: pmic: regulators: 'oneOf' conditional failed, one must be fixed: 'buck_vpca15', 'ldo_vgp4' do not match any of the regexes: '^(buck_)?v(core|dram1|gpu|modem|pa|proc1[12]|s[12])$', '^(ldo_)?v((aux|cn|io|rf)18|camio)$', '^(ldo_)?v(aud|bif|cn|fe|io)28$', '^(ldo_)?v(a|rf)12$', '^(ldo_)?v(cama[12]|camd|cn33|dram2|efuse|emc|ibr|ldo28|m18|mc|mch|mddr|sim[12])$', '^(ldo_)?vsram[_-](core|gpu|others|proc1[12])$', '^(ldo_)?vusb$', '^(ldo_)?vxo22$', 'pinctrl-[0-9]+' False schema does not allow {'compatible': ['mediatek,mt6397-regulator'], 'buck_vpca15': {'regulator-name': ['vpca15'], 'regulator-min-microvolt': [[850000]], 'regulator-max-microvolt': [[1400000]], 'regulator-ramp-delay': 12500, 'regulator-always-on': True}, 'ldo_vgp4': {'regulator-name': ['vgp4'], 'regulator-min-microvolt': [[1200000]], 'regulator-max-microvolt': [[3300000]], 'regulator-enable-ramp-delay': 218}} /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/mediatek,mt6397.example.dtb: pmic: regulators:compatible: 'oneOf' conditional failed, one must be fixed: ['mediatek,mt6397-regulator'] is too short 'mediatek,mt6358-regulator' was expected 'mediatek,mt6366-regulator' was expected from schema $id: http://devicetree.org/schemas/mfd/mediatek,mt6397.yaml# /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mfd/mediatek,mt6397.example.dtb: pmic: regulators: Unevaluated properties are not allowed ('buck_vpca15', 'ldo_vgp4' were unexpected) from schema $id: http://devicetree.org/schemas/mfd/mediatek,mt6397.yaml# doc reference errors (make refcheckdocs): Warning: Documentation/devicetree/bindings/input/mediatek,pmic-keys.yaml references a file that doesn't exist: Documentation/devicetree/bindings/mfd/mt6397.txt Warning: Documentation/devicetree/bindings/leds/leds-mt6323.txt references a file that doesn't exist: Documentation/devicetree/bindings/mfd/mt6397.txt Documentation/devicetree/bindings/input/mediatek,pmic-keys.yaml: Documentation/devicetree/bindings/mfd/mt6397.txt Documentation/devicetree/bindings/leds/leds-mt6323.txt: Documentation/devicetree/bindings/mfd/mt6397.txt See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240808105722.7222-1-macpaul.lin@mediatek.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 8/8/24 20:04, Krzysztof Kozlowski wrote: > > > External email : Please do not click links or open attachments until you > have verified the sender or the content. > > On 08/08/2024 12:57, Macpaul Lin wrote: >> Convert the mfd: mediatek: mt6397 binding to DT schema format. >> >> New updates in this conversion: >> - Align generic names of DT schema "audio-codec" and "regulators". >> - mt6397-regulators: Replace the "txt" reference with newly added DT >> schema. >> >> Signed-off-by: Sen Chu <sen.chu@mediatek.com> >> Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com> >> --- >> .../bindings/mfd/mediatek,mt6397.yaml | 202 ++++++++++++++++++ >> .../devicetree/bindings/mfd/mt6397.txt | 110 ---------- > > You are doing conversions in odd order... and ignore my comments. The > example from your regulator binding is supposed to be here - I wrote it > last time. > > Due to doing changes totally unsynchronized, this CANNOT be merged > without unnecessary maintainer coordination, because of dependency. > > Sorry, that's not how it works for MFD devices. > > Perform conversion of entire device in ONE patchset. Okay, will collect the conversion of mt6323-regulator.txt and rtc-mt6397.txt in the next version. >> 2 files changed, 202 insertions(+), 110 deletions(-) >> create mode 100644 Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml >> delete mode 100644 Documentation/devicetree/bindings/mfd/mt6397.txt >> >> Changes for v1: >> - This patch depends on conversion of mediatek,mt6397-regulator.yaml >> [1] https://lore.kernel.org/lkml/20240807091738.18387-1-macpaul.lin@mediatek.com/T/ [snip] >> +$id: http://devicetree.org/schemas/mfd/mediatek,mt6397.yaml# >> +$schema: http://devicetree.org/meta-schemas/core.yaml# >> + >> +title: MediaTek MT6397/MT6323 Multifunction Device >> + >> +maintainers: >> + - Sen Chu <sen.chu@mediatek.com> >> + - Macpaul Lin <macpaul.lin@mediatek.com> >> + >> +description: | >> + MT6397/MT6323 is a multifunction device with the following sub modules: > > MFD is Linuxism, avoid it. Will replace MFD with "power management system chip with sub-modules" something like this in next version. >> + - Regulator >> + - RTC >> + - Audio codec >> + - GPIO >> + - Clock >> + - LED >> + - Keys >> + - Power controller >> + >> + It is interfaced to host controller using SPI interface by a proprietary hardware >> + called PMIC wrapper or pwrap. MT6397/MT6323 MFD is a child device of pwrap. >> + See the following for pwarp node definitions: >> + ../soc/mediatek/mediatek,pwrap.yaml > > Drop, instead add proper ref or compatible in parent node. I'm confused here. I've checked mediatek,mt6357.yaml as a reference . It uses the similar method here. "See the following for pwarp node definitions:" "Documentation/devicetree/bindings/soc/mediatek/mediatek,pwrap.yaml" If "$ref: /schemas/soc/mediatek/mediatek,pwrap.yaml" is added here, dt_bindings_check will complain the following errors and more. Documentation/devicetree/bindings/mfd/mediatek,mt6397.example.dtb: pmic: compatible: 'oneOf' conditional failed, one must be fixed: ['mediatek,mt6397'] is too short 'mediatek,mt6397' is not one of ['mediatek,mt2701-pwrap', 'mediatek,mt6765-pwrap', 'mediatek,mt6779-pwrap', 'mediatek,mt6795-pwrap', 'mediatek,mt6797-pwrap', 'mediatek,mt6873-pwrap', 'mediatek,mt7622-pwrap', 'mediatek,mt8135-pwrap', 'mediatek,mt8173-pwrap', 'mediatek,mt8183-pwrap', 'mediatek,mt8186-pwrap', 'mediatek,mt8195-pwrap', 'mediatek,mt8365-pwrap', 'mediatek,mt8516-pwrap'] 'mediatek,mt6397' is not one of ['mediatek,mt8186-pwrap', 'mediatek,mt8195-pwrap'] 'mediatek,mt6397' is not one of ['mediatek,mt8188-pwrap'] from schema $id: http://devicetree.org/schemas/mfd/mediatek,mt6397.yaml# Which also conflicts with the comments in the examples.. >> + pwrap { > > Drop Please help to check if a $ref or a compatible of pwrap should be added here. >> + >> + This document describes the binding for MFD device and its sub module. > > Drop Will fix it in next version. >> + >> +properties: >> + compatible: >> + oneOf: >> + - enum: >> + - mediatek,mt6323 >> + - mediatek,mt6331 # "mediatek,mt6331" for PMIC MT6331 and MT6332. >> + - mediatek,mt6357 >> + - mediatek,mt6358 >> + - mediatek,mt6359 >> + - mediatek,mt6397 >> + - items: >> + - enum: >> + - mediatek,mt6366 # "mediatek,mt6366", "mediatek,mt6358" for PMIC MT6366 > > Drop comment, it is obvious. Don't repeat constraints in free form text. Will fix it in next version. > >> + - const: mediatek,mt6358 >> + >> + interrupts: >> + maxItems: 1 >> + >> + interrupt-controller: true >> + >> + "#interrupt-cells": >> + const: 2 >> + >> + rtc: >> + type: object >> + unevaluatedProperties: false >> + description: >> + Real Time Clock (RTC) >> + See ../rtc/rtc-mt6397.txt > > No, convert the binding. Will convert it rtc-mt6397.txt and put it into "mfd/mediatek,mt6397.yaml" together. > >> + properties: >> + compatible: >> + oneOf: >> + - enum: >> + - mediatek,mt6323-rtc >> + - mediatek,mt6331-rtc >> + - mediatek,mt6358-rtc >> + - mediatek,mt6397-rtc >> + - items: >> + - enum: >> + - mediatek,mt6366-rtc # RTC MT6366 > > Drop all such comments. > >> + - const: mediatek,mt6358-rtc >> + >> + regulators: >> + type: object >> + oneOf: >> + - $ref: /schemas/regulator/mediatek,mt6358-regulator.yaml >> + - $ref: /schemas/regulator/mediatek,mt6397-regulator.yaml > > And how is it supposed to be tested? The dt_bindings_check didn't complain eny thing about these. Of course I've included the conversion patch of mediatek,mt6397-regulator.yaml. >> + unevaluatedProperties: false >> + description: >> + Regulators >> + For mt6323, see ../regulator/mt6323-regulator.txt > > Drop, useless. Should I convert it to DT schema and add to $ref above together? > >> + properties: >> + compatible: >> + oneOf: >> + - enum: >> + - mediatek,mt6323-regulator >> + - mediatek,mt6358-regulator >> + - mediatek,mt6397-regulator >> + - items: >> + - enum: >> + - mediatek,mt6366-regulator # Regulator MT6366 >> + - const: mediatek,mt6358-regulator >> + >> + audio-codec: >> + type: object >> + unevaluatedProperties: false > > This does not make sense. You do not have any ref here. The dt_bindings_check will complain error here. Will replace it with "additionalProperties: false". >> + description: >> + Audio codec >> + properties: >> + compatible: >> + oneOf: >> + - enum: >> + - mediatek,mt6397-codec >> + - mediatek,mt6358-sound >> + - items: >> + - enum: >> + - mediatek,mt6366-sound # Codec MT6366 >> + - const: mediatek,mt6358-sound > > This wasn't in the old binding. Commit msg also does not explain why you > are doing changes from conversion. Will update new added item into commit message in next version. >> + >> + clk: >> + type: object >> + unevaluatedProperties: false > > Again, no, it does not work like this. See example schema for > explanation of this. Will replace it with "additionalProperties: false". > Convert all children - entire device. Then either use ref or > additionalProperties: true. See Qualcomm mdss bindings for example. There is no more children available for the clock node of this PMIC. This is a clock buffer node. However, there are no sub nodes or any public document explain these clock buffer in public domain. What I've got is the compatible string in the driver. >> + description: >> + Clock > > Your descriptions are useless. You just said "clk" node is "clock". Really? Will improve it in next version. >> + properties: >> + compatible: >> + const: mediatek,mt6397-clk >> + >> + led: >> + type: object >> + unevaluatedProperties: false >> + description: >> + LED >> + See ../leds/leds-mt6323.txt for more information > > No Will convert "leds-mt6323.txt" and move it together with "mfd/mediatek,mt6397.yaml" >> + properties: >> + compatible: >> + const: mediatek,mt6323-led >> + >> + keys: >> + type: object >> + $ref: /schemas/input/mediatek,pmic-keys.yaml >> + unevaluatedProperties: false >> + description: Keys > > Keys are keys? Could keys be anything else? Will fix it in the next version. >> + >> +additionalProperties: false >> + >> +examples: >> + - | >> + #include <dt-bindings/interrupt-controller/arm-gic.h> >> + >> + pwrap { > > Drop Will fix it in the next version. >> + pmic { >> + compatible = "mediatek,mt6397"; >> + interrupts-extended = <&pio 222 IRQ_TYPE_LEVEL_HIGH>; >> + interrupt-controller; >> + #interrupt-cells = <2>; >> + >> + mt6397_codec: audio-codec { >> + compatible = "mediatek,mt6397-codec"; >> + }; >> + >> + mt6397_regulators: regulators { >> + compatible = "mediatek,mt6397-regulator"; >> + >> + mt6397_vpca15_reg: buck_vpca15 { >> + regulator-name = "vpca15"; >> + regulator-min-microvolt = <850000>; >> + regulator-max-microvolt = <1400000>; >> + regulator-ramp-delay = <12500>; >> + regulator-always-on; >> + }; >> + >> + mt6397_vgp4_reg: ldo_vgp4 { >> + regulator-name = "vgp4"; >> + regulator-min-microvolt = <1200000>; >> + regulator-max-microvolt = <3300000>; >> + regulator-enable-ramp-delay = <218>; >> + }; >> + }; > > Incomplete. > > The parent device example is supposed to be 100% complete. Will complete the example with MT6397 and MT6323 as reference in the next version. > Best regards, > Krzysztof Thanks for the review and still some questions listed above. Please help to clarify the correction method for the questions. Best regards, Macpaul Lin
On 23/08/2024 08:44, Macpaul Lin wrote: > > On 8/8/24 20:04, Krzysztof Kozlowski wrote: >> >> >> External email : Please do not click links or open attachments until you >> have verified the sender or the content. >> >> On 08/08/2024 12:57, Macpaul Lin wrote: >>> Convert the mfd: mediatek: mt6397 binding to DT schema format. >>> >>> New updates in this conversion: >>> - Align generic names of DT schema "audio-codec" and "regulators". >>> - mt6397-regulators: Replace the "txt" reference with newly added DT >>> schema. >>> >>> Signed-off-by: Sen Chu <sen.chu@mediatek.com> >>> Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com> >>> --- >>> .../bindings/mfd/mediatek,mt6397.yaml | 202 ++++++++++++++++++ >>> .../devicetree/bindings/mfd/mt6397.txt | 110 ---------- >> >> You are doing conversions in odd order... and ignore my comments. The >> example from your regulator binding is supposed to be here - I wrote it >> last time. >> >> Due to doing changes totally unsynchronized, this CANNOT be merged >> without unnecessary maintainer coordination, because of dependency. >> >> Sorry, that's not how it works for MFD devices. >> >> Perform conversion of entire device in ONE patchset. > > Okay, will collect the conversion of mt6323-regulator.txt and > rtc-mt6397.txt in the next version. > >>> 2 files changed, 202 insertions(+), 110 deletions(-) >>> create mode 100644 Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml >>> delete mode 100644 Documentation/devicetree/bindings/mfd/mt6397.txt >>> >>> Changes for v1: >>> - This patch depends on conversion of mediatek,mt6397-regulator.yaml >>> [1] https://lore.kernel.org/lkml/20240807091738.18387-1-macpaul.lin@mediatek.com/T/ > > [snip] > >>> +$id: http://devicetree.org/schemas/mfd/mediatek,mt6397.yaml# >>> +$schema: http://devicetree.org/meta-schemas/core.yaml# >>> + >>> +title: MediaTek MT6397/MT6323 Multifunction Device >>> + >>> +maintainers: >>> + - Sen Chu <sen.chu@mediatek.com> >>> + - Macpaul Lin <macpaul.lin@mediatek.com> >>> + >>> +description: | >>> + MT6397/MT6323 is a multifunction device with the following sub modules: >> >> MFD is Linuxism, avoid it. > > Will replace MFD with "power management system chip with sub-modules" > something like this in next version. > >>> + - Regulator >>> + - RTC >>> + - Audio codec >>> + - GPIO >>> + - Clock >>> + - LED >>> + - Keys >>> + - Power controller >>> + >>> + It is interfaced to host controller using SPI interface by a proprietary hardware >>> + called PMIC wrapper or pwrap. MT6397/MT6323 MFD is a child device of pwrap. >>> + See the following for pwarp node definitions: >>> + ../soc/mediatek/mediatek,pwrap.yaml >> >> Drop, instead add proper ref or compatible in parent node. > > I'm confused here. I've checked mediatek,mt6357.yaml as a reference > . > It uses the similar method here. > "See the following for pwarp node definitions:" > "Documentation/devicetree/bindings/soc/mediatek/mediatek,pwrap.yaml" What exactly is confusing? The other example is wrong. It's not a schema, but free form text. Write schema so it would be applied and would validate the DTS. > > If "$ref: /schemas/soc/mediatek/mediatek,pwrap.yaml" is added here, > dt_bindings_check will complain the following errors and more. > > Documentation/devicetree/bindings/mfd/mediatek,mt6397.example.dtb: pmic: > compatible: 'oneOf' conditional failed, one must be fixed: > ['mediatek,mt6397'] is too short > 'mediatek,mt6397' is not one of ['mediatek,mt2701-pwrap', > 'mediatek,mt6765-pwrap', 'mediatek,mt6779-pwrap', > 'mediatek,mt6795-pwrap', 'mediatek,mt6797-pwrap', > 'mediatek,mt6873-pwrap', 'mediatek,mt7622-pwrap', > 'mediatek,mt8135-pwrap', 'mediatek,mt8173-pwrap', > 'mediatek,mt8183-pwrap', 'mediatek,mt8186-pwrap', > 'mediatek,mt8195-pwrap', 'mediatek,mt8365-pwrap', 'mediatek,mt8516-pwrap'] > 'mediatek,mt6397' is not one of ['mediatek,mt8186-pwrap', > 'mediatek,mt8195-pwrap'] > 'mediatek,mt6397' is not one of ['mediatek,mt8188-pwrap'] > from schema $id: > http://devicetree.org/schemas/mfd/mediatek,mt6397.yaml# > > Which also conflicts with the comments in the examples.. So investigate and fix this, instead of hiding the problem. > >> + pwrap { > > > > Drop > > Please help to check if a $ref or a compatible of pwrap should be added > here. Where did you add the $ref? The child node should have it, not parent, obviously. Just look at one of many other examples having children. ... >> >>> + - const: mediatek,mt6358-rtc >>> + >>> + regulators: >>> + type: object >>> + oneOf: >>> + - $ref: /schemas/regulator/mediatek,mt6358-regulator.yaml >>> + - $ref: /schemas/regulator/mediatek,mt6397-regulator.yaml >> >> And how is it supposed to be tested? > > The dt_bindings_check didn't complain eny thing about these. Really? Then checkout the maintainer tree, apply this patch and test again. You know, it is impossible for us to apply a patch on top of linux-next... > Of course I've included the conversion patch of > mediatek,mt6397-regulator.yaml. > >>> + unevaluatedProperties: false >>> + description: >>> + Regulators >>> + For mt6323, see ../regulator/mt6323-regulator.txt >> >> Drop, useless. > Should I convert it to DT schema and add to $ref above together? Or just use compatibles. There are also examples of this - in MFD devices, Qcom display. > >> >>> + properties: >>> + compatible: >>> + oneOf: >>> + - enum: >>> + - mediatek,mt6323-regulator >>> + - mediatek,mt6358-regulator >>> + - mediatek,mt6397-regulator >>> + - items: >>> + - enum: >>> + - mediatek,mt6366-regulator # Regulator MT6366 >>> + - const: mediatek,mt6358-regulator >>> + >>> + audio-codec: >>> + type: object >>> + unevaluatedProperties: false >> >> This does not make sense. You do not have any ref here. > > The dt_bindings_check will complain error here. > Will replace it with "additionalProperties: false". > > >>> + description: >>> + Audio codec >>> + properties: >>> + compatible: >>> + oneOf: >>> + - enum: >>> + - mediatek,mt6397-codec >>> + - mediatek,mt6358-sound >>> + - items: >>> + - enum: >>> + - mediatek,mt6366-sound # Codec MT6366 >>> + - const: mediatek,mt6358-sound >> >> This wasn't in the old binding. Commit msg also does not explain why you >> are doing changes from conversion. > > Will update new added item into commit message in next version. > >>> + >>> + clk: >>> + type: object >>> + unevaluatedProperties: false >> >> Again, no, it does not work like this. See example schema for >> explanation of this. > > Will replace it with "additionalProperties: false". > >> Convert all children - entire device. Then either use ref or >> additionalProperties: true. See Qualcomm mdss bindings for example. Oh, look here, I even mentioned Qualcomm to use as an example... > > There is no more children available for the clock node of this PMIC. > This is a clock buffer node. However, there are no sub nodes or any > public document explain these clock buffer in public domain. > What I've got is the compatible string in the driver. Then I don't know what you want to express here. > >>> + description: >>> + Clock >> >> Your descriptions are useless. You just said "clk" node is "clock". Really? > > Will improve it in next version. Best regards, Krzysztof
diff --git a/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml b/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml new file mode 100644 index 0000000..cfbf2215 --- /dev/null +++ b/Documentation/devicetree/bindings/mfd/mediatek,mt6397.yaml @@ -0,0 +1,202 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/mfd/mediatek,mt6397.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: MediaTek MT6397/MT6323 Multifunction Device + +maintainers: + - Sen Chu <sen.chu@mediatek.com> + - Macpaul Lin <macpaul.lin@mediatek.com> + +description: | + MT6397/MT6323 is a multifunction device with the following sub modules: + - Regulator + - RTC + - Audio codec + - GPIO + - Clock + - LED + - Keys + - Power controller + + It is interfaced to host controller using SPI interface by a proprietary hardware + called PMIC wrapper or pwrap. MT6397/MT6323 MFD is a child device of pwrap. + See the following for pwarp node definitions: + ../soc/mediatek/mediatek,pwrap.yaml + + This document describes the binding for MFD device and its sub module. + +properties: + compatible: + oneOf: + - enum: + - mediatek,mt6323 + - mediatek,mt6331 # "mediatek,mt6331" for PMIC MT6331 and MT6332. + - mediatek,mt6357 + - mediatek,mt6358 + - mediatek,mt6359 + - mediatek,mt6397 + - items: + - enum: + - mediatek,mt6366 # "mediatek,mt6366", "mediatek,mt6358" for PMIC MT6366 + - const: mediatek,mt6358 + + interrupts: + maxItems: 1 + + interrupt-controller: true + + "#interrupt-cells": + const: 2 + + rtc: + type: object + unevaluatedProperties: false + description: + Real Time Clock (RTC) + See ../rtc/rtc-mt6397.txt + properties: + compatible: + oneOf: + - enum: + - mediatek,mt6323-rtc + - mediatek,mt6331-rtc + - mediatek,mt6358-rtc + - mediatek,mt6397-rtc + - items: + - enum: + - mediatek,mt6366-rtc # RTC MT6366 + - const: mediatek,mt6358-rtc + + regulators: + type: object + oneOf: + - $ref: /schemas/regulator/mediatek,mt6358-regulator.yaml + - $ref: /schemas/regulator/mediatek,mt6397-regulator.yaml + unevaluatedProperties: false + description: + Regulators + For mt6323, see ../regulator/mt6323-regulator.txt + properties: + compatible: + oneOf: + - enum: + - mediatek,mt6323-regulator + - mediatek,mt6358-regulator + - mediatek,mt6397-regulator + - items: + - enum: + - mediatek,mt6366-regulator # Regulator MT6366 + - const: mediatek,mt6358-regulator + + audio-codec: + type: object + unevaluatedProperties: false + description: + Audio codec + properties: + compatible: + oneOf: + - enum: + - mediatek,mt6397-codec + - mediatek,mt6358-sound + - items: + - enum: + - mediatek,mt6366-sound # Codec MT6366 + - const: mediatek,mt6358-sound + + clk: + type: object + unevaluatedProperties: false + description: + Clock + properties: + compatible: + const: mediatek,mt6397-clk + + led: + type: object + unevaluatedProperties: false + description: + LED + See ../leds/leds-mt6323.txt for more information + properties: + compatible: + const: mediatek,mt6323-led + + keys: + type: object + $ref: /schemas/input/mediatek,pmic-keys.yaml + unevaluatedProperties: false + description: Keys + properties: + compatible: + oneOf: + - enum: + - mediatek,mt6323-keys + - mediatek,mt6331-keys + - mediatek,mt6397-keys + + power-controller: + type: object + unevaluatedProperties: false + description: + Power controller + See ../power/reset/mt6323-poweroff.txt + properties: + compatible: + const: mediatek,mt6323-pwrc + + pin-controller: + type: object + $ref: /schemas/pinctrl/mediatek,mt65xx-pinctrl.yaml + unevaluatedProperties: false + description: + Pin controller + properties: + compatible: + const: mediatek,mt6397-pinctrl + +required: + - compatible + - regulators + +additionalProperties: false + +examples: + - | + #include <dt-bindings/interrupt-controller/arm-gic.h> + + pwrap { + pmic { + compatible = "mediatek,mt6397"; + interrupts-extended = <&pio 222 IRQ_TYPE_LEVEL_HIGH>; + interrupt-controller; + #interrupt-cells = <2>; + + mt6397_codec: audio-codec { + compatible = "mediatek,mt6397-codec"; + }; + + mt6397_regulators: regulators { + compatible = "mediatek,mt6397-regulator"; + + mt6397_vpca15_reg: buck_vpca15 { + regulator-name = "vpca15"; + regulator-min-microvolt = <850000>; + regulator-max-microvolt = <1400000>; + regulator-ramp-delay = <12500>; + regulator-always-on; + }; + + mt6397_vgp4_reg: ldo_vgp4 { + regulator-name = "vgp4"; + regulator-min-microvolt = <1200000>; + regulator-max-microvolt = <3300000>; + regulator-enable-ramp-delay = <218>; + }; + }; + }; + }; diff --git a/Documentation/devicetree/bindings/mfd/mt6397.txt b/Documentation/devicetree/bindings/mfd/mt6397.txt deleted file mode 100644 index 10540aa..0000000 --- a/Documentation/devicetree/bindings/mfd/mt6397.txt +++ /dev/null @@ -1,110 +0,0 @@ -MediaTek MT6397/MT6323 Multifunction Device Driver - -MT6397/MT6323 is a multifunction device with the following sub modules: -- Regulator -- RTC -- Audio codec -- GPIO -- Clock -- LED -- Keys -- Power controller - -It is interfaced to host controller using SPI interface by a proprietary hardware -called PMIC wrapper or pwrap. MT6397/MT6323 MFD is a child device of pwrap. -See the following for pwarp node definitions: -../soc/mediatek/mediatek,pwrap.yaml - -This document describes the binding for MFD device and its sub module. - -Required properties: -compatible: - "mediatek,mt6323" for PMIC MT6323 - "mediatek,mt6331" for PMIC MT6331 and MT6332 - "mediatek,mt6357" for PMIC MT6357 - "mediatek,mt6358" for PMIC MT6358 - "mediatek,mt6359" for PMIC MT6359 - "mediatek,mt6366", "mediatek,mt6358" for PMIC MT6366 - "mediatek,mt6397" for PMIC MT6397 - -Optional subnodes: - -- rtc - Required properties: Should be one of follows - - compatible: "mediatek,mt6323-rtc" - - compatible: "mediatek,mt6331-rtc" - - compatible: "mediatek,mt6358-rtc" - - compatible: "mediatek,mt6397-rtc" - For details, see ../rtc/rtc-mt6397.txt -- regulators - Required properties: - - compatible: "mediatek,mt6323-regulator" - see ../regulator/mt6323-regulator.txt - - compatible: "mediatek,mt6358-regulator" - - compatible: "mediatek,mt6366-regulator", "mediatek-mt6358-regulator" - see ../regulator/mt6358-regulator.txt - - compatible: "mediatek,mt6397-regulator" - see ../regulator/mt6397-regulator.txt -- codec - Required properties: - - compatible: "mediatek,mt6397-codec" or "mediatek,mt6358-sound" -- clk - Required properties: - - compatible: "mediatek,mt6397-clk" -- led - Required properties: - - compatible: "mediatek,mt6323-led" - see ../leds/leds-mt6323.txt - -- keys - Required properties: Should be one of the following - - compatible: "mediatek,mt6323-keys" - - compatible: "mediatek,mt6331-keys" - - compatible: "mediatek,mt6397-keys" - see ../input/mtk-pmic-keys.txt - -- power-controller - Required properties: - - compatible: "mediatek,mt6323-pwrc" - For details, see ../power/reset/mt6323-poweroff.txt - -- pin-controller - Required properties: - - compatible: "mediatek,mt6397-pinctrl" - For details, see ../pinctrl/pinctrl-mt65xx.txt - -Example: - pwrap: pwrap@1000f000 { - compatible = "mediatek,mt8135-pwrap"; - - ... - - pmic { - compatible = "mediatek,mt6397"; - - codec: mt6397codec { - compatible = "mediatek,mt6397-codec"; - }; - - regulators { - compatible = "mediatek,mt6397-regulator"; - - mt6397_vpca15_reg: buck_vpca15 { - regulator-compatible = "buck_vpca15"; - regulator-name = "vpca15"; - regulator-min-microvolt = <850000>; - regulator-max-microvolt = <1400000>; - regulator-ramp-delay = <12500>; - regulator-always-on; - }; - - mt6397_vgp4_reg: ldo_vgp4 { - regulator-compatible = "ldo_vgp4"; - regulator-name = "vgp4"; - regulator-min-microvolt = <1200000>; - regulator-max-microvolt = <3300000>; - regulator-enable-ramp-delay = <218>; - }; - }; - }; - };