| Message ID | 20230208103709.116896-2-angelogioacchino.delregno@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Panfrost: Improve and add MediaTek SoCs support | expand |
On Wed, Feb 8, 2023 at 6:37 PM AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> wrote: > > The sram-supply is MediaTek-specific, it is and will ever be used > only for the mediatek,mt8183-mali compatible due to the addition of > the mediatek-regulator-coupler driver: change the binding to add > this supply when mediatek,mt8183-mali is present as a compatible > instead of disabling it when not present. > > This is done in preparation for adding new bindings for other > MediaTek SoCs, such as MT8192 and others. > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > --- > Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > index 78964c140b46..69212f3b1328 100644 > --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > @@ -57,8 +57,6 @@ properties: > > mali-supply: true > > - sram-supply: true > - Have you tried actually validating the device trees against this? Based on my previous tests this gives out errors. The reason is that each conditional is a separate sub-schema, and the validator is run against each schema and sub-schema separately, instead of collapsing matching schemas and sub-schemas together and validating once. So we'll get a validation error on sram-supply not being a valid property when validating current mt8183 against the base schema. We have a similar issue with power-domain-names, for which I'll send a patch to fix. See the following for the fix: http://git.kernel.org/wens/c/d1adb38ab2ad0442755607c2bcc726cc17cce2c7 and the following for what I did for MT8192 on top of the previous patch: http://git.kernel.org/wens/c/049bd164884398d7e5f72c710da6aaa9a95bc10a Regards ChenYu > operating-points-v2: true > > power-domains: > @@ -157,6 +155,7 @@ allOf: > - const: core0 > - const: core1 > - const: core2 > + sram-supply: true > > required: > - sram-supply > @@ -166,7 +165,6 @@ allOf: > properties: > power-domains: > maxItems: 1 > - sram-supply: false > - if: > properties: > compatible: > -- > 2.39.1 >
Il 09/02/23 03:50, Chen-Yu Tsai ha scritto: > On Wed, Feb 8, 2023 at 6:37 PM AngeloGioacchino Del Regno > <angelogioacchino.delregno@collabora.com> wrote: >> >> The sram-supply is MediaTek-specific, it is and will ever be used >> only for the mediatek,mt8183-mali compatible due to the addition of >> the mediatek-regulator-coupler driver: change the binding to add >> this supply when mediatek,mt8183-mali is present as a compatible >> instead of disabling it when not present. >> >> This is done in preparation for adding new bindings for other >> MediaTek SoCs, such as MT8192 and others. >> >> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> >> --- >> Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml | 4 +--- >> 1 file changed, 1 insertion(+), 3 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml >> index 78964c140b46..69212f3b1328 100644 >> --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml >> +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml >> @@ -57,8 +57,6 @@ properties: >> >> mali-supply: true >> >> - sram-supply: true >> - > > Have you tried actually validating the device trees against this? > Based on my previous tests this gives out errors. I did... and I didn't get any complaint... but perhaps something went wrong on my side? I mean, I can retry just to be sure. > > The reason is that each conditional is a separate sub-schema, and the > validator is run against each schema and sub-schema separately, instead > of collapsing matching schemas and sub-schemas together and validating > once. So we'll get a validation error on sram-supply not being a valid > property when validating current mt8183 against the base schema. > > We have a similar issue with power-domain-names, for which I'll send > a patch to fix. See the following for the fix: > > http://git.kernel.org/wens/c/d1adb38ab2ad0442755607c2bcc726cc17cce2c7 > > and the following for what I did for MT8192 on top of the previous patch: > > http://git.kernel.org/wens/c/049bd164884398d7e5f72c710da6aaa9a95bc10a > Thanks for the pointer, btw Cheers, Angelo > > Regards > ChenYu > >> operating-points-v2: true >> >> power-domains: >> @@ -157,6 +155,7 @@ allOf: >> - const: core0 >> - const: core1 >> - const: core2 >> + sram-supply: true >> >> required: >> - sram-supply >> @@ -166,7 +165,6 @@ allOf: >> properties: >> power-domains: >> maxItems: 1 >> - sram-supply: false >> - if: >> properties: >> compatible: >> -- >> 2.39.1 >>
On Thu, Feb 09, 2023 at 10:15:33AM +0100, AngeloGioacchino Del Regno wrote: > Il 09/02/23 03:50, Chen-Yu Tsai ha scritto: > > On Wed, Feb 8, 2023 at 6:37 PM AngeloGioacchino Del Regno > > <angelogioacchino.delregno@collabora.com> wrote: > > > > > > The sram-supply is MediaTek-specific, it is and will ever be used > > > only for the mediatek,mt8183-mali compatible due to the addition of > > > the mediatek-regulator-coupler driver: change the binding to add > > > this supply when mediatek,mt8183-mali is present as a compatible > > > instead of disabling it when not present. > > > > > > This is done in preparation for adding new bindings for other > > > MediaTek SoCs, such as MT8192 and others. > > > > > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> > > > --- > > > Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml | 4 +--- > > > 1 file changed, 1 insertion(+), 3 deletions(-) > > > > > > diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > > index 78964c140b46..69212f3b1328 100644 > > > --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > > +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml > > > @@ -57,8 +57,6 @@ properties: > > > > > > mali-supply: true > > > > > > - sram-supply: true > > > - > > > > Have you tried actually validating the device trees against this? > > Based on my previous tests this gives out errors. > > I did... and I didn't get any complaint... but perhaps something went wrong > on my side? > > I mean, I can retry just to be sure. You should need unevaluatedProperties instead of additionalProperties for this to work. The latter cannot 'see' into an if/then schema. But really we want the default top level to allow this and then disallow it in an if/then schema. Rob
diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml index 78964c140b46..69212f3b1328 100644 --- a/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml +++ b/Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml @@ -57,8 +57,6 @@ properties: mali-supply: true - sram-supply: true - operating-points-v2: true power-domains: @@ -157,6 +155,7 @@ allOf: - const: core0 - const: core1 - const: core2 + sram-supply: true required: - sram-supply @@ -166,7 +165,6 @@ allOf: properties: power-domains: maxItems: 1 - sram-supply: false - if: properties: compatible:
The sram-supply is MediaTek-specific, it is and will ever be used only for the mediatek,mt8183-mali compatible due to the addition of the mediatek-regulator-coupler driver: change the binding to add this supply when mediatek,mt8183-mali is present as a compatible instead of disabling it when not present. This is done in preparation for adding new bindings for other MediaTek SoCs, such as MT8192 and others. Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com> --- Documentation/devicetree/bindings/gpu/arm,mali-bifrost.yaml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)