On Thu, Sep 26, 2019 at 9:47 AM Maciej Falkowski
<m.falkowski@xxxxxxxxxxx> wrote:
>
>
> On 9/26/19 4:03 PM, Krzysztof Kozlowski wrote:
> > On Thu, Sep 26, 2019 at 02:56:14PM +0200, Marek Szyprowski wrote:
> >> From: Maciej Falkowski <m.falkowski@xxxxxxxxxxx>
> >>
> >> Convert Samsung Image Scaler to newer dt-schema format.
> >>
> >> Signed-off-by: Maciej Falkowski <m.falkowski@xxxxxxxxxxx>
> >> Signed-off-by: Marek Szyprowski <m.szyprowski@xxxxxxxxxxx>
> >> ---
> >> v2:
> >> - Removed quotation marks from string in 'compatible' property
> >> - Added if-then statement for 'clocks' and 'clock-names' property
> >> - Added include directive to example
> >> - Added GIC_SPI macro to example
> >>
> >> Best regards,
> >> Maciej Falkowski
> >> ---
> >> .../bindings/gpu/samsung-scaler.txt | 27 -------
> >> .../bindings/gpu/samsung-scaler.yaml | 71 +++++++++++++++++++
> >> 2 files changed, 71 insertions(+), 27 deletions(-)
> >> delete mode 100644 Documentation/devicetree/bindings/gpu/samsung-scaler.txt
> >> create mode 100644 Documentation/devicetree/bindings/gpu/samsung-scaler.yaml
> >>
> >> diff --git a/Documentation/devicetree/bindings/gpu/samsung-scaler.txt b/Documentation/devicetree/bindings/gpu/samsung-scaler.txt
> >> deleted file mode 100644
> >> index 9c3d98105dfd..000000000000
> >> --- a/Documentation/devicetree/bindings/gpu/samsung-scaler.txt
> >> +++ /dev/null
> >> @@ -1,27 +0,0 @@
> >> -* Samsung Exynos Image Scaler
> >> -
> >> -Required properties:
> >> - - compatible : value should be one of the following:
> >> - (a) "samsung,exynos5420-scaler" for Scaler IP in Exynos5420
> >> - (b) "samsung,exynos5433-scaler" for Scaler IP in Exynos5433
> >> -
> >> - - reg : Physical base address of the IP registers and length of memory
> >> - mapped region.
> >> -
> >> - - interrupts : Interrupt specifier for scaler interrupt, according to format
> >> - specific to interrupt parent.
> >> -
> >> - - clocks : Clock specifier for scaler clock, according to generic clock
> >> - bindings. (See Documentation/devicetree/bindings/clock/exynos*.txt)
> >> -
> >> - - clock-names : Names of clocks. For exynos scaler, it should be "mscl"
> >> - on 5420 and "pclk", "aclk" and "aclk_xiu" on 5433.
> >> -
> >> -Example:
> >> - scaler@12800000 {
> >> - compatible = "samsung,exynos5420-scaler";
> >> - reg = <0x12800000 0x1294>;
> >> - interrupts = <0 220 IRQ_TYPE_LEVEL_HIGH>;
> >> - clocks = <&clock CLK_MSCL0>;
> >> - clock-names = "mscl";
> >> - };
> >> diff --git a/Documentation/devicetree/bindings/gpu/samsung-scaler.yaml b/Documentation/devicetree/bindings/gpu/samsung-scaler.yaml
> >> new file mode 100644
> >> index 000000000000..af19930d052e
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/gpu/samsung-scaler.yaml
> >> @@ -0,0 +1,71 @@
> >> +# SPDX-License-Identifier: GPL-2.0
> >> +%YAML 1.2
> >> +---
> >> +$id: https://protect2.fireeye.com/url?k=1ffa720fd467d028.1ffbf940-9a5a550397b4da2b&u=http://devicetree.org/schemas/gpu/samsung-scaler.yaml#
> >> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> >> +
> >> +title: Samsung Exynos SoC Image Scaler
> >> +
> >> +maintainers:
> >> + - Inki Dae <inki.dae@xxxxxxxxxxx>
> >> +
> >> +properties:
> >> + compatible:
> >> + enum:
> >> + - samsung,exynos5420-scaler
> >> + - samsung,exynos5433-scaler
> >> +
> >> + reg:
> >> + maxItems: 1
> >> +
> >> + interrupts:
> >> + maxItems: 1
> >> +
>
> Hi Krzysztof,
Please work on your quoting. Reply below what you are replying to.
>
> By "Midgard" I assume that you referred to
> 'Documentation/devicetree/bindings/gpu/arm,mali-midgard.yaml'.
>
> I think that 'clocks' and 'clock-names' properties before if statement
> serve different purpose in this schema.
> It totally has about 10 different compatibles grouped in five pairs.
> Then schema declares for 'clocks' minItems as one and maxItems as two and
> later it overrides this boundaries with if statement for particular
> compatibles.
It's not an override, but an AND. So what's under 'properties' has to
be the looser constraints than what is under an if/then schema.
> Well, then clearly, the purpose is to declare boundaries for all of
> pairs and
> not to provide easy-to-find definition for this properties.
>
> In my schema I directly set boundaries per compatible with single
> if-else statement.
> I didn't know what to put before then as if statement is already
> self-explanatory.
>
> Best regards,
> Maciej Falkowski
>
> > I am repeating myself... leave the clocks and clock-names.
> >
> > "I think it is worth to leave the clocks and clock-names here (could be
> > empty or with min/max values for number of items). This makes it easy to
> > find the properties by humans.
I agree.
Let me put it another way. You need to add an 'additionalProperties:
false' and (I think) to make that work you'll need them listed here.
Rob
