| Message ID | 20240826065415.19641-1-macpaul.lin@mediatek.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | dt-bindings: mfd: mediatek,mt6357: Fixup reference to pwrap node | expand |
On 26/08/2024 08:54, Macpaul Lin wrote: > The mt6357 is a subnode of pwrap node. Previously, the documentation > only included a note in the description of mt6357. This change adds the > appropriate $ref for pwrap to ensure clarity and correctness. Heh? The schema described mt6357, not pwrap. Why are you changing it? > > $ref: /schemas/soc/mediatek/mediatek,pwrap.yaml > > Additionally, the indentation for the pmic section has been adjusted > to match the corresponding structure. > > Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com> > --- > .../bindings/mfd/mediatek,mt6357.yaml | 124 +++++++++--------- > 1 file changed, 65 insertions(+), 59 deletions(-) > > Changes for v1: > - This patch has been made based on linux-next/master branch. > > diff --git a/Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml b/Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml > index b67fbe0..5f4f540 100644 > --- a/Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml > +++ b/Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml > @@ -22,69 +22,75 @@ description: | > > It is interfaced to host controller using SPI interface by a proprietary hardware > called PMIC wrapper or pwrap. This MFD is a child device of pwrap. > - See the following for pwrap node definitions: > - Documentation/devicetree/bindings/soc/mediatek/mediatek,pwrap.yaml > > properties: > - compatible: > - const: mediatek,mt6357 How does this schema is being selected without compatible? > - > - interrupts: > - maxItems: 1 > - > - interrupt-controller: true > - > - "#interrupt-cells": > - const: 2 > - > - mediatek,hp-pull-down: > - description: > - Earphone driver positive output stage short to > - the audio reference ground. > - type: boolean > - > - mediatek,micbias0-microvolt: > - description: Selects MIC Bias 0 output voltage. > - enum: [1700000, 1800000, 1900000, 2000000, > - 2100000, 2500000, 2600000, 2700000] > - default: 1700000 > - > - mediatek,micbias1-microvolt: > - description: Selects MIC Bias 1 output voltage. > - enum: [1700000, 1800000, 1900000, 2000000, > - 2100000, 2500000, 2600000, 2700000] > - default: 1700000 > - > - regulators: > - type: object > - $ref: /schemas/regulator/mediatek,mt6357-regulator.yaml > - unevaluatedProperties: false > - description: > - List of MT6357 BUCKs and LDOs regulators. > - > - rtc: > + pwrap: With the diff it is tricky to say what you are doing, but it feels wrong. I don't understand the rationale, the problem being fixed here, and considering unusual diff, this just looks wrong approach. Bindings do not work like that - you do not reference the parent inside the child. There is nowhere example for that, so I have no clue how did you come up with it. Best regards, Krzysztof
On Mon, Aug 26, 2024 at 02:54:15PM +0800, Macpaul Lin wrote: > The mt6357 is a subnode of pwrap node. Previously, the documentation > only included a note in the description of mt6357. This change adds the > appropriate $ref for pwrap to ensure clarity and correctness. I think this change is wrong and the existing binding is fine. Adding the ref overcomplicates the binding completely, and stating that this is a child node of another device is sufficient. Instead, if anything, the pwrap binding should have a ref to /this/ binding. Thanks, Conor. > > $ref: /schemas/soc/mediatek/mediatek,pwrap.yaml > > Additionally, the indentation for the pmic section has been adjusted > to match the corresponding structure. > > Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com> > --- > .../bindings/mfd/mediatek,mt6357.yaml | 124 +++++++++--------- > 1 file changed, 65 insertions(+), 59 deletions(-) > > Changes for v1: > - This patch has been made based on linux-next/master branch. > > diff --git a/Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml b/Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml > index b67fbe0..5f4f540 100644 > --- a/Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml > +++ b/Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml > @@ -22,69 +22,75 @@ description: | > > It is interfaced to host controller using SPI interface by a proprietary hardware > called PMIC wrapper or pwrap. This MFD is a child device of pwrap. > - See the following for pwrap node definitions: > - Documentation/devicetree/bindings/soc/mediatek/mediatek,pwrap.yaml > > properties: > - compatible: > - const: mediatek,mt6357 > - > - interrupts: > - maxItems: 1 > - > - interrupt-controller: true > - > - "#interrupt-cells": > - const: 2 > - > - mediatek,hp-pull-down: > - description: > - Earphone driver positive output stage short to > - the audio reference ground. > - type: boolean > - > - mediatek,micbias0-microvolt: > - description: Selects MIC Bias 0 output voltage. > - enum: [1700000, 1800000, 1900000, 2000000, > - 2100000, 2500000, 2600000, 2700000] > - default: 1700000 > - > - mediatek,micbias1-microvolt: > - description: Selects MIC Bias 1 output voltage. > - enum: [1700000, 1800000, 1900000, 2000000, > - 2100000, 2500000, 2600000, 2700000] > - default: 1700000 > - > - regulators: > - type: object > - $ref: /schemas/regulator/mediatek,mt6357-regulator.yaml > - unevaluatedProperties: false > - description: > - List of MT6357 BUCKs and LDOs regulators. > - > - rtc: > + pwrap: > type: object > - $ref: /schemas/rtc/rtc.yaml# > - unevaluatedProperties: false > - description: > - MT6357 Real Time Clock. > + $ref: /schemas/soc/mediatek/mediatek,pwrap.yaml > properties: > - compatible: > - const: mediatek,mt6357-rtc > - start-year: true > - required: > - - compatible > - > - keys: > - type: object > - $ref: /schemas/input/mediatek,pmic-keys.yaml > - unevaluatedProperties: false > - description: > - MT6357 power and home keys. > - > -required: > - - compatible > - - regulators > + pmic: > + type: object > + additionalProperties: false > + properties: > + compatible: > + const: mediatek,mt6357 > + > + interrupts: > + maxItems: 1 > + > + interrupt-controller: true > + > + "#interrupt-cells": > + const: 2 > + > + mediatek,hp-pull-down: > + description: > + Earphone driver positive output stage short to > + the audio reference ground. > + type: boolean > + > + mediatek,micbias0-microvolt: > + description: Selects MIC Bias 0 output voltage. > + enum: [1700000, 1800000, 1900000, 2000000, > + 2100000, 2500000, 2600000, 2700000] > + default: 1700000 > + > + mediatek,micbias1-microvolt: > + description: Selects MIC Bias 1 output voltage. > + enum: [1700000, 1800000, 1900000, 2000000, > + 2100000, 2500000, 2600000, 2700000] > + default: 1700000 > + > + regulators: > + type: object > + $ref: /schemas/regulator/mediatek,mt6357-regulator.yaml > + unevaluatedProperties: false > + description: > + List of MT6357 BUCKs and LDOs regulators. > + > + rtc: > + type: object > + $ref: /schemas/rtc/rtc.yaml# > + unevaluatedProperties: false > + description: > + MT6357 Real Time Clock. > + properties: > + compatible: > + const: mediatek,mt6357-rtc > + start-year: true > + required: > + - compatible > + > + keys: > + type: object > + $ref: /schemas/input/mediatek,pmic-keys.yaml > + unevaluatedProperties: false > + description: > + MT6357 power and home keys. > + > + required: > + - compatible > + - regulators > > additionalProperties: false > > -- > 2.45.2 > >
On 26/08/2024 17:50, Conor Dooley wrote: > On Mon, Aug 26, 2024 at 02:54:15PM +0800, Macpaul Lin wrote: >> The mt6357 is a subnode of pwrap node. Previously, the documentation >> only included a note in the description of mt6357. This change adds the >> appropriate $ref for pwrap to ensure clarity and correctness. > > I think this change is wrong and the existing binding is fine. > Adding the ref overcomplicates the binding completely, and stating that > this is a child node of another device is sufficient. > > Instead, if anything, the pwrap binding should have a ref to /this/ > binding. Yes or list allowed compatibles for child node. Best regards, Krzysztof
On 8/26/24 23:50, Conor Dooley wrote: > On Mon, Aug 26, 2024 at 02:54:15PM +0800, Macpaul Lin wrote: >> The mt6357 is a subnode of pwrap node. Previously, the documentation >> only included a note in the description of mt6357. This change adds the >> appropriate $ref for pwrap to ensure clarity and correctness. > > I think this change is wrong and the existing binding is fine. > Adding the ref overcomplicates the binding completely, and stating that > this is a child node of another device is sufficient. > > Instead, if anything, the pwrap binding should have a ref to /this/ > binding. > > Thanks, > Conor. Thanks for the clarification of this parent-child relationship of the binding. Will apply to further conversion tasks. There are many PMIC devices belongs to the pwrap bindings. Hope I'll have time to fix this soon. >> >> $ref: /schemas/soc/mediatek/mediatek,pwrap.yaml >> >> Additionally, the indentation for the pmic section has been adjusted >> to match the corresponding structure. Best regards, Macpaul Lin
diff --git a/Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml b/Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml index b67fbe0..5f4f540 100644 --- a/Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml +++ b/Documentation/devicetree/bindings/mfd/mediatek,mt6357.yaml @@ -22,69 +22,75 @@ description: | It is interfaced to host controller using SPI interface by a proprietary hardware called PMIC wrapper or pwrap. This MFD is a child device of pwrap. - See the following for pwrap node definitions: - Documentation/devicetree/bindings/soc/mediatek/mediatek,pwrap.yaml properties: - compatible: - const: mediatek,mt6357 - - interrupts: - maxItems: 1 - - interrupt-controller: true - - "#interrupt-cells": - const: 2 - - mediatek,hp-pull-down: - description: - Earphone driver positive output stage short to - the audio reference ground. - type: boolean - - mediatek,micbias0-microvolt: - description: Selects MIC Bias 0 output voltage. - enum: [1700000, 1800000, 1900000, 2000000, - 2100000, 2500000, 2600000, 2700000] - default: 1700000 - - mediatek,micbias1-microvolt: - description: Selects MIC Bias 1 output voltage. - enum: [1700000, 1800000, 1900000, 2000000, - 2100000, 2500000, 2600000, 2700000] - default: 1700000 - - regulators: - type: object - $ref: /schemas/regulator/mediatek,mt6357-regulator.yaml - unevaluatedProperties: false - description: - List of MT6357 BUCKs and LDOs regulators. - - rtc: + pwrap: type: object - $ref: /schemas/rtc/rtc.yaml# - unevaluatedProperties: false - description: - MT6357 Real Time Clock. + $ref: /schemas/soc/mediatek/mediatek,pwrap.yaml properties: - compatible: - const: mediatek,mt6357-rtc - start-year: true - required: - - compatible - - keys: - type: object - $ref: /schemas/input/mediatek,pmic-keys.yaml - unevaluatedProperties: false - description: - MT6357 power and home keys. - -required: - - compatible - - regulators + pmic: + type: object + additionalProperties: false + properties: + compatible: + const: mediatek,mt6357 + + interrupts: + maxItems: 1 + + interrupt-controller: true + + "#interrupt-cells": + const: 2 + + mediatek,hp-pull-down: + description: + Earphone driver positive output stage short to + the audio reference ground. + type: boolean + + mediatek,micbias0-microvolt: + description: Selects MIC Bias 0 output voltage. + enum: [1700000, 1800000, 1900000, 2000000, + 2100000, 2500000, 2600000, 2700000] + default: 1700000 + + mediatek,micbias1-microvolt: + description: Selects MIC Bias 1 output voltage. + enum: [1700000, 1800000, 1900000, 2000000, + 2100000, 2500000, 2600000, 2700000] + default: 1700000 + + regulators: + type: object + $ref: /schemas/regulator/mediatek,mt6357-regulator.yaml + unevaluatedProperties: false + description: + List of MT6357 BUCKs and LDOs regulators. + + rtc: + type: object + $ref: /schemas/rtc/rtc.yaml# + unevaluatedProperties: false + description: + MT6357 Real Time Clock. + properties: + compatible: + const: mediatek,mt6357-rtc + start-year: true + required: + - compatible + + keys: + type: object + $ref: /schemas/input/mediatek,pmic-keys.yaml + unevaluatedProperties: false + description: + MT6357 power and home keys. + + required: + - compatible + - regulators additionalProperties: false
The mt6357 is a subnode of pwrap node. Previously, the documentation only included a note in the description of mt6357. This change adds the appropriate $ref for pwrap to ensure clarity and correctness. $ref: /schemas/soc/mediatek/mediatek,pwrap.yaml Additionally, the indentation for the pmic section has been adjusted to match the corresponding structure. Signed-off-by: Macpaul Lin <macpaul.lin@mediatek.com> --- .../bindings/mfd/mediatek,mt6357.yaml | 124 +++++++++--------- 1 file changed, 65 insertions(+), 59 deletions(-) Changes for v1: - This patch has been made based on linux-next/master branch.