Messages in this thread | | | From | Alexandre Mergnat <> | Date | Mon, 28 Nov 2022 15:03:31 +0100 | Subject | Re: [PATCH v6 05/10] dt-bindings: soc: mediatek: convert pwrap documentation |
| |
Le dim. 27 nov. 2022 à 14:11, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> a écrit : > > >> + pmic: > >> + type: object > > > > What's here? Other schema? If not then maybe compatible? What about > > description? > > I guess this was comment from Rob, so it's fine.
Yes it is.
> >> +allOf: > >> + - if: > >> + properties: > >> + compatible: > >> + contains: > >> + const: mediatek,mt8365-pwrap > >> + then: > >> + properties: > >> + clocks: > >> + minItems: 4 > >> + > >> + clock-names: > >> + minItems: 4 > > > > else: > > ??? > > Actually this looks less complete than your previous patch. > > else: > clocks: > maxItems: 2 > same for clock-names >
I think I’ve followed the feedback done here [1] I’ve declared `minItems: 2` globally and override it to 4 if mediatek,mt8365-pwrap is used. Isn’t it the right way to implement it ?
> >> + compatible = "mediatek,mt8135-pwrap"; > >> + reg = <0 0x1000f000 0 0x1000>, > > > > This does not match your unit address. No warnings when compile testing? > >
There are no warnings when compile testing. I will fix the unit address anyway, sorry.
> >> + <0 0x11017000 0 0x1000>; > >> + reg-names = "pwrap", "pwrap-bridge"; > >> + interrupts = <GIC_SPI 128 IRQ_TYPE_LEVEL_HIGH>; > >> + clocks = <&clk26m>, <&clk26m>; > >> + clock-names = "spi", "wrap"; > >> + resets = <&infracfg MT8135_INFRA_PMIC_WRAP_RST>, > >> + <&pericfg MT8135_PERI_PWRAP_BRIDGE_SW_RST>; > >> + reset-names = "pwrap", "pwrap-bridge"; > > > > Missing pmic. Make your example complete. > > Probably pmic should be skipped, I understand it is described in MFD > binding. >
Put the pmic in the example have 2 constraints: - The original pmic "mediatek,mt6397" isn’t supported by a yaml schema, so I’ve a dt_binding_check fail: `failed to match any schema with compatible: ['mediatek,mt6397']` - If I put another pmic that supports a yaml schema, I need to put all required properties for the pmic, which I thought was unnecessary since it’s already done in its own schema and can change for another pmic, so less consistent.
Then yes, IMHO, PMIC should be skipped in the example.
[1] https://lore.kernel.org/all/fe898d24-54fa-56bb-8067-b422a3a52ff5@collabora.com/
Alex
| |