* [PATCH v2] dt-bindings: gpu: Convert Samsung Image Scaler to dt-schema
[not found] <CGME20190926125619eucas1p249ac149ef1e1a3eb975dae94b08cd7be@eucas1p2.samsung.com>
@ 2019-09-26 12:56 ` Marek Szyprowski
2019-09-26 14:03 ` Krzysztof Kozlowski
0 siblings, 1 reply; 6+ messages in thread
From: Marek Szyprowski @ 2019-09-26 12:56 UTC (permalink / raw)
To: devicetree, dri-devel, linux-kernel, linux-samsung-soc
Cc: Rob Herring, Mark Rutland, Inki Dae, Krzysztof Kozlowski,
Maciej Falkowski, Marek Szyprowski
From: Maciej Falkowski <m.falkowski@samsung.com>
Convert Samsung Image Scaler to newer dt-schema format.
Signed-off-by: Maciej Falkowski <m.falkowski@samsung.com>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
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: 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@samsung.com>
+
+properties:
+ compatible:
+ enum:
+ - samsung,exynos5420-scaler
+ - samsung,exynos5433-scaler
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+if:
+ properties:
+ compatible:
+ contains:
+ const: samsung,exynos5420-scaler
+then:
+ properties:
+ clocks:
+ items:
+ - description: mscl clock
+
+ clock-names:
+ items:
+ - const: mscl
+else:
+ properties:
+ clocks:
+ items:
+ - description: mscl clock
+ - description: aclk clock
+ - description: aclk_xiu clock
+
+ clock-names:
+ items:
+ - const: pclk
+ - const: aclk
+ - const: aclk_xiu
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+
+examples:
+ - |
+ #include <dt-bindings/clock/exynos5420.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+ scaler@12800000 {
+ compatible = "samsung,exynos5420-scaler";
+ reg = <0x12800000 0x1294>;
+ interrupts = <GIC_SPI 220 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clock CLK_MSCL0>;
+ clock-names = "mscl";
+ };
+
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] dt-bindings: gpu: Convert Samsung Image Scaler to dt-schema
2019-09-26 12:56 ` [PATCH v2] dt-bindings: gpu: Convert Samsung Image Scaler to dt-schema Marek Szyprowski
@ 2019-09-26 14:03 ` Krzysztof Kozlowski
2019-09-26 14:47 ` Maciej Falkowski
0 siblings, 1 reply; 6+ messages in thread
From: Krzysztof Kozlowski @ 2019-09-26 14:03 UTC (permalink / raw)
To: Marek Szyprowski
Cc: devicetree, dri-devel, linux-kernel, linux-samsung-soc,
Rob Herring, Mark Rutland, Inki Dae, Maciej Falkowski
On Thu, Sep 26, 2019 at 02:56:14PM +0200, Marek Szyprowski wrote:
> From: Maciej Falkowski <m.falkowski@samsung.com>
>
> Convert Samsung Image Scaler to newer dt-schema format.
>
> Signed-off-by: Maciej Falkowski <m.falkowski@samsung.com>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> 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: 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@samsung.com>
> +
> +properties:
> + compatible:
> + enum:
> + - samsung,exynos5420-scaler
> + - samsung,exynos5433-scaler
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
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.
Midgard bindings could be used as example."
> +if:
> + properties:
> + compatible:
> + contains:
> + const: samsung,exynos5420-scaler
> +then:
> + properties:
> + clocks:
> + items:
> + - description: mscl clock
> +
> + clock-names:
> + items:
> + - const: mscl
> +else:
> + properties:
> + clocks:
> + items:
> + - description: mscl clock
> + - description: aclk clock
> + - description: aclk_xiu clock
> +
> + clock-names:
> + items:
> + - const: pclk
> + - const: aclk
> + - const: aclk_xiu
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - clock-names
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/exynos5420.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> + scaler@12800000 {
> + compatible = "samsung,exynos5420-scaler";
> + reg = <0x12800000 0x1294>;
> + interrupts = <GIC_SPI 220 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&clock CLK_MSCL0>;
> + clock-names = "mscl";
> + };
> +
Unneeded trailing line.
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] dt-bindings: gpu: Convert Samsung Image Scaler to dt-schema
2019-09-26 14:03 ` Krzysztof Kozlowski
@ 2019-09-26 14:47 ` Maciej Falkowski
2019-09-26 15:35 ` Rob Herring
0 siblings, 1 reply; 6+ messages in thread
From: Maciej Falkowski @ 2019-09-26 14:47 UTC (permalink / raw)
To: Krzysztof Kozlowski, Marek Szyprowski
Cc: devicetree, dri-devel, linux-kernel, linux-samsung-soc,
Rob Herring, Mark Rutland, Inki Dae
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@samsung.com>
>>
>> Convert Samsung Image Scaler to newer dt-schema format.
>>
>> Signed-off-by: Maciej Falkowski <m.falkowski@samsung.com>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>> 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@samsung.com>
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - samsung,exynos5420-scaler
>> + - samsung,exynos5433-scaler
>> +
>> + reg:
>> + maxItems: 1
>> +
>> + interrupts:
>> + maxItems: 1
>> +
Hi Krzysztof,
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.
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.
>
> Midgard bindings could be used as example."
>
>> +if:
>> + properties:
>> + compatible:
>> + contains:
>> + const: samsung,exynos5420-scaler
>> +then:
>> + properties:
>> + clocks:
>> + items:
>> + - description: mscl clock
>> +
>> + clock-names:
>> + items:
>> + - const: mscl
>> +else:
>> + properties:
>> + clocks:
>> + items:
>> + - description: mscl clock
>> + - description: aclk clock
>> + - description: aclk_xiu clock
>> +
>> + clock-names:
>> + items:
>> + - const: pclk
>> + - const: aclk
>> + - const: aclk_xiu
>> +
>> +required:
>> + - compatible
>> + - reg
>> + - interrupts
>> + - clocks
>> + - clock-names
>> +
>> +examples:
>> + - |
>> + #include <dt-bindings/clock/exynos5420.h>
>> + #include <dt-bindings/interrupt-controller/arm-gic.h>
>> +
>> + scaler@12800000 {
>> + compatible = "samsung,exynos5420-scaler";
>> + reg = <0x12800000 0x1294>;
>> + interrupts = <GIC_SPI 220 IRQ_TYPE_LEVEL_HIGH>;
>> + clocks = <&clock CLK_MSCL0>;
>> + clock-names = "mscl";
>> + };
>> +
> Unneeded trailing line.
>
> Best regards,
> Krzysztof
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] dt-bindings: gpu: Convert Samsung Image Scaler to dt-schema
2019-09-26 14:47 ` Maciej Falkowski
@ 2019-09-26 15:35 ` Rob Herring
2019-09-26 16:54 ` Maciej Falkowski
0 siblings, 1 reply; 6+ messages in thread
From: Rob Herring @ 2019-09-26 15:35 UTC (permalink / raw)
To: Maciej Falkowski
Cc: Krzysztof Kozlowski, Marek Szyprowski, devicetree, dri-devel,
linux-kernel, linux-samsung-soc, Mark Rutland, Inki Dae
On Thu, Sep 26, 2019 at 9:47 AM Maciej Falkowski
<m.falkowski@samsung.com> 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@samsung.com>
> >>
> >> Convert Samsung Image Scaler to newer dt-schema format.
> >>
> >> Signed-off-by: Maciej Falkowski <m.falkowski@samsung.com>
> >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> >> ---
> >> 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@samsung.com>
> >> +
> >> +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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] dt-bindings: gpu: Convert Samsung Image Scaler to dt-schema
2019-09-26 15:35 ` Rob Herring
@ 2019-09-26 16:54 ` Maciej Falkowski
2019-09-26 19:19 ` Rob Herring
0 siblings, 1 reply; 6+ messages in thread
From: Maciej Falkowski @ 2019-09-26 16:54 UTC (permalink / raw)
To: Rob Herring
Cc: Krzysztof Kozlowski, Marek Szyprowski, devicetree, dri-devel,
linux-kernel, linux-samsung-soc, Mark Rutland, Inki Dae
On 9/26/19 5:35 PM, Rob Herring wrote:
> On Thu, Sep 26, 2019 at 9:47 AM Maciej Falkowski
> <m.falkowski@samsung.com> 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@samsung.com>
>>>>
>>>> Convert Samsung Image Scaler to newer dt-schema format.
>>>>
>>>> Signed-off-by: Maciej Falkowski <m.falkowski@samsung.com>
>>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>> ---
>>>> 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@samsung.com>
>>>> +
>>>> +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.
Hi Rob,
Thank you for explaining that.
>> 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
So when properties are only defined inside if-then scope,
they are labeled as 'additional' as they are not defined inside
scope of 'properties'. It is mandatory then to mention 'clock' and
'clock-names' there if 'additionalProperties: false' .
However I had not set it intentionally as there are additional
properties in some bindings,
exactly 'iommu' and 'power-domains' are undocumented.
Is it a good way to put them in 'properties' just to be able to forbid
additional properties?
Best regards,
Maciej Falkowski
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] dt-bindings: gpu: Convert Samsung Image Scaler to dt-schema
2019-09-26 16:54 ` Maciej Falkowski
@ 2019-09-26 19:19 ` Rob Herring
0 siblings, 0 replies; 6+ messages in thread
From: Rob Herring @ 2019-09-26 19:19 UTC (permalink / raw)
To: Maciej Falkowski
Cc: Krzysztof Kozlowski, Marek Szyprowski, devicetree, dri-devel,
linux-kernel, linux-samsung-soc, Mark Rutland, Inki Dae
On Thu, Sep 26, 2019 at 11:54 AM Maciej Falkowski
<m.falkowski@samsung.com> wrote:
>
>
> On 9/26/19 5:35 PM, Rob Herring wrote:
> > On Thu, Sep 26, 2019 at 9:47 AM Maciej Falkowski
> > <m.falkowski@samsung.com> 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@samsung.com>
> >>>>
> >>>> Convert Samsung Image Scaler to newer dt-schema format.
> >>>>
> >>>> Signed-off-by: Maciej Falkowski <m.falkowski@samsung.com>
> >>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> >>>> ---
> >>>> 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@samsung.com>
> >>>> +
> >>>> +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.
> Hi Rob,
> Thank you for explaining that.
> >> 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
>
> So when properties are only defined inside if-then scope,
> they are labeled as 'additional' as they are not defined inside
> scope of 'properties'. It is mandatory then to mention 'clock' and
> 'clock-names' there if 'additionalProperties: false' .
Yes, which makes additionalProperties a bit fragile and can't be used
if we include a common schema. There's a fix for that coming in
json-schema draft8 called 'unevaluatedProperties'.
> However I had not set it intentionally as there are additional
> properties in some bindings,
> exactly 'iommu' and 'power-domains' are undocumented.
> Is it a good way to put them in 'properties' just to be able to forbid
> additional properties?
Simply put, they should be documented too.
Rob
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-09-26 19:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20190926125619eucas1p249ac149ef1e1a3eb975dae94b08cd7be@eucas1p2.samsung.com>
2019-09-26 12:56 ` [PATCH v2] dt-bindings: gpu: Convert Samsung Image Scaler to dt-schema Marek Szyprowski
2019-09-26 14:03 ` Krzysztof Kozlowski
2019-09-26 14:47 ` Maciej Falkowski
2019-09-26 15:35 ` Rob Herring
2019-09-26 16:54 ` Maciej Falkowski
2019-09-26 19:19 ` Rob Herring
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).