[v2] dt-bindings: gpu: Convert Samsung Image Rotator to dt-schema
diff mbox series

Message ID 20190912161538.4321-1-m.falkowski@samsung.com
State Superseded
Headers show
Series
  • [v2] dt-bindings: gpu: Convert Samsung Image Rotator to dt-schema
Related show

Checks

Context Check Description
robh/dt-meta-schema fail build log
robh/checkpatch warning "total: 0 errors, 2 warnings, 52 lines checked"

Commit Message

Maciej Falkowski Sept. 12, 2019, 4:15 p.m. UTC
Convert Samsung Image Rotator to newer dt-schema format.

Signed-off-by: Maciej Falkowski <m.falkowski@samsung.com>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
v2:
- add required properties
- add proper commit recipients
---
 .../bindings/gpu/samsung-rotator.txt          | 28 ----------
 .../bindings/gpu/samsung-rotator.yaml         | 52 +++++++++++++++++++
 2 files changed, 52 insertions(+), 28 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/gpu/samsung-rotator.txt
 create mode 100644 Documentation/devicetree/bindings/gpu/samsung-rotator.yaml

Comments

Krzysztof Kozlowski Sept. 13, 2019, 6:29 a.m. UTC | #1
On Thu, Sep 12, 2019 at 06:15:38PM +0200, Maciej Falkowski wrote:
> Convert Samsung Image Rotator to newer dt-schema format.
> 
> Signed-off-by: Maciej Falkowski <m.falkowski@samsung.com>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

Just to make it clear, Marek's signed-off should appear for one of
conditions:
 - he contributed some source code to your patch,
 - he took your patch, rebased, send by himself (not a case here, I
   think),
 - he contributed significant ideas, although for this there is a
   "Co-developed-by" tag.

If someone made just review - add Reviewed-by. If someone suggested the
patch - add Suggested-by.

> ---
> v2:
> - add required properties
> - add proper commit recipients
> ---
>  .../bindings/gpu/samsung-rotator.txt          | 28 ----------
>  .../bindings/gpu/samsung-rotator.yaml         | 52 +++++++++++++++++++
>  2 files changed, 52 insertions(+), 28 deletions(-)
>  delete mode 100644 Documentation/devicetree/bindings/gpu/samsung-rotator.txt
>  create mode 100644 Documentation/devicetree/bindings/gpu/samsung-rotator.yaml
> 
> diff --git a/Documentation/devicetree/bindings/gpu/samsung-rotator.txt b/Documentation/devicetree/bindings/gpu/samsung-rotator.txt
> deleted file mode 100644
> index 3aca2578da0b..000000000000
> --- a/Documentation/devicetree/bindings/gpu/samsung-rotator.txt
> +++ /dev/null
> @@ -1,28 +0,0 @@
> -* Samsung Image Rotator
> -
> -Required properties:
> -  - compatible : value should be one of the following:
> -	* "samsung,s5pv210-rotator" for Rotator IP in S5PV210
> -	* "samsung,exynos4210-rotator" for Rotator IP in Exynos4210
> -	* "samsung,exynos4212-rotator" for Rotator IP in Exynos4212/4412
> -	* "samsung,exynos5250-rotator" for Rotator IP in Exynos5250
> -
> -  - reg : Physical base address of the IP registers and length of memory
> -	  mapped region.
> -
> -  - interrupts : Interrupt specifier for rotator interrupt, according to format
> -		 specific to interrupt parent.
> -
> -  - clocks : Clock specifier for rotator clock, according to generic clock
> -	     bindings. (See Documentation/devicetree/bindings/clock/exynos*.txt)
> -
> -  - clock-names : Names of clocks. For exynos rotator, it should be "rotator".
> -
> -Example:
> -	rotator@12810000 {
> -		compatible = "samsung,exynos4210-rotator";
> -		reg = <0x12810000 0x1000>;
> -		interrupts = <0 83 0>;
> -		clocks = <&clock 278>;
> -		clock-names = "rotator";
> -	};
> diff --git a/Documentation/devicetree/bindings/gpu/samsung-rotator.yaml b/Documentation/devicetree/bindings/gpu/samsung-rotator.yaml
> new file mode 100644
> index 000000000000..96afafe98388
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/gpu/samsung-rotator.yaml
> @@ -0,0 +1,52 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/gpu/samsung-rotator.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Samsung Image Rotator

Thanks Maciej for working on this.

I am trying to make the names more consistent:
1. For Exynos bindings:
Samsung Exynos SoC xxx yyy zzz

2. For multiple SoCs (S3C, S5P, Exynos etc):
Samsung SoC xxx yyy zzz

Currently the names are mixture of legacy and new names. It is a
nit-pick but makes all bindings look like a part of bigger effort, not
bunch of patches done by random people. :)

If there are no objections, maybe you could change it to:
Samsung SoC Image Rotator

> +
> +maintainers:
> +  - Inki Dae <inki.dae@samsung.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - "samsung,s5pv210-rotator"    # for Rotator IP in S5PV210
> +      - "samsung,exynos4210-rotator" # for Rotator IP in Exynos4210
> +      - "samsung,exynos4212-rotator" # for Rotator IP in Exynos4212/4412
> +      - "samsung,exynos5250-rotator" # for Rotator IP in Exynos5250

The comments are duplicating the compatible, so skip them.

> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    description: |
> +      Clock specifier for rotator clock according to generic clock
> +      bindings. (See Documentation/devicetree/bindings/clock/exynos*.txt)

Skip the description. Clocks property is a well-known binding.

> +    maxItems: 1
> +
> +  clock-names:
> +    items:
> +    - const: rotator
> +    maxItems: 1

I think there is no need to maxItems since all possible items are fixed.

Best regards,
Krzysztof


> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +
> +examples:
> +  - |
> +    rotator@12810000 {
> +        compatible = "samsung,exynos4210-rotator";
> +        reg = <0x12810000 0x1000>;
> +        interrupts = <0 83 0>;
> +        clocks = <&clock 278>;
> +        clock-names = "rotator";
> +    };
> +
> -- 
> 2.17.1
>
Marek Szyprowski Sept. 13, 2019, 9:15 a.m. UTC | #2
Hi Krzysztof,

On 9/13/19 8:29 AM, Krzysztof Kozlowski wrote:
> On Thu, Sep 12, 2019 at 06:15:38PM +0200, Maciej Falkowski wrote:
>> Convert Samsung Image Rotator to newer dt-schema format.
>>
>> Signed-off-by: Maciej Falkowski <m.falkowski@samsung.com>
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Just to make it clear, Marek's signed-off should appear for one of
> conditions:
>   - he contributed some source code to your patch,
>   - he took your patch, rebased, send by himself (not a case here, I
>     think),
>   - he contributed significant ideas, although for this there is a
>     "Co-developed-by" tag.
>
> If someone made just review - add Reviewed-by. If someone suggested the
> patch - add Suggested-by.

My signed-off here was added to mark that this patch is allowed to be 
submitted to the public mailing list, as I have required company 
permissions for such activity. It is not that uncommon that a given 
patch has more than one signed-off and still the main author has the 
first signed-off tag.

 > ...

Best regards
Krzysztof Kozlowski Sept. 13, 2019, 9:29 a.m. UTC | #3
On Fri, 13 Sep 2019 at 11:15, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
>
> Hi Krzysztof,
>
> On 9/13/19 8:29 AM, Krzysztof Kozlowski wrote:
> > On Thu, Sep 12, 2019 at 06:15:38PM +0200, Maciej Falkowski wrote:
> >> Convert Samsung Image Rotator to newer dt-schema format.
> >>
> >> Signed-off-by: Maciej Falkowski <m.falkowski@samsung.com>
> >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > Just to make it clear, Marek's signed-off should appear for one of
> > conditions:
> >   - he contributed some source code to your patch,
> >   - he took your patch, rebased, send by himself (not a case here, I
> >     think),
> >   - he contributed significant ideas, although for this there is a
> >     "Co-developed-by" tag.
> >
> > If someone made just review - add Reviewed-by. If someone suggested the
> > patch - add Suggested-by.
>
> My signed-off here was added to mark that this patch is allowed to be
> submitted to the public mailing list, as I have required company
> permissions for such activity. It is not that uncommon that a given
> patch has more than one signed-off and still the main author has the
> first signed-off tag.

Thanks for explanations. If I understand correctly, your SoB appears
because some internal Samsung rules. That is not what SoB is meant
for:
https://elixir.bootlin.com/linux/v5.3-rc8/source/Documentation/process/submitting-patches.rst#L431
If you do not contributed to the patch, did not touch it (e.g.
rebasing) and you are not sending it, then your SoB should not be
there. It looks like the same madness with Kyungmin Park long time
ago:
https://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux.git/commit/?h=for-next&id=6c6cf64b16438eac6da9a90412a82316ad196e7c
Every patch was marked with SoB even though he was not involved at all
in the process.

That's not what kernel's SoB is for.

Best regards,
Krzysztof
Krzysztof Kozlowski Sept. 13, 2019, 9:34 a.m. UTC | #4
On Fri, 13 Sep 2019 at 11:29, Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
> On Fri, 13 Sep 2019 at 11:15, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> >
> > Hi Krzysztof,
> >
> > On 9/13/19 8:29 AM, Krzysztof Kozlowski wrote:
> > > On Thu, Sep 12, 2019 at 06:15:38PM +0200, Maciej Falkowski wrote:
> > >> Convert Samsung Image Rotator to newer dt-schema format.
> > >>
> > >> Signed-off-by: Maciej Falkowski <m.falkowski@samsung.com>
> > >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > > Just to make it clear, Marek's signed-off should appear for one of
> > > conditions:
> > >   - he contributed some source code to your patch,
> > >   - he took your patch, rebased, send by himself (not a case here, I
> > >     think),
> > >   - he contributed significant ideas, although for this there is a
> > >     "Co-developed-by" tag.
> > >
> > > If someone made just review - add Reviewed-by. If someone suggested the
> > > patch - add Suggested-by.
> >
> > My signed-off here was added to mark that this patch is allowed to be
> > submitted to the public mailing list, as I have required company
> > permissions for such activity. It is not that uncommon that a given
> > patch has more than one signed-off and still the main author has the
> > first signed-off tag.
>
> Thanks for explanations. If I understand correctly, your SoB appears
> because some internal Samsung rules. That is not what SoB is meant
> for:
> https://elixir.bootlin.com/linux/v5.3-rc8/source/Documentation/process/submitting-patches.rst#L431
> If you do not contributed to the patch, did not touch it (e.g.
> rebasing) and you are not sending it, then your SoB should not be
> there. It looks like the same madness with Kyungmin Park long time
> ago:
> https://git.kernel.org/pub/scm/linux/kernel/git/krzk/linux.git/commit/?h=for-next&id=6c6cf64b16438eac6da9a90412a82316ad196e7c
> Every patch was marked with SoB even though he was not involved at all
> in the process.
>
> That's not what kernel's SoB is for.

Ah, and if you meant that Maciej does not have the permissions to send
the patches, then your SoB along with patch going through your account
makes sense. Otherwise, adding your SoB and sending by Maciej, is a
joke of company regulations. It means absolutely nothing and then
strip it out.

Best regards,
Krzysztof

Patch
diff mbox series

diff --git a/Documentation/devicetree/bindings/gpu/samsung-rotator.txt b/Documentation/devicetree/bindings/gpu/samsung-rotator.txt
deleted file mode 100644
index 3aca2578da0b..000000000000
--- a/Documentation/devicetree/bindings/gpu/samsung-rotator.txt
+++ /dev/null
@@ -1,28 +0,0 @@ 
-* Samsung Image Rotator
-
-Required properties:
-  - compatible : value should be one of the following:
-	* "samsung,s5pv210-rotator" for Rotator IP in S5PV210
-	* "samsung,exynos4210-rotator" for Rotator IP in Exynos4210
-	* "samsung,exynos4212-rotator" for Rotator IP in Exynos4212/4412
-	* "samsung,exynos5250-rotator" for Rotator IP in Exynos5250
-
-  - reg : Physical base address of the IP registers and length of memory
-	  mapped region.
-
-  - interrupts : Interrupt specifier for rotator interrupt, according to format
-		 specific to interrupt parent.
-
-  - clocks : Clock specifier for rotator clock, according to generic clock
-	     bindings. (See Documentation/devicetree/bindings/clock/exynos*.txt)
-
-  - clock-names : Names of clocks. For exynos rotator, it should be "rotator".
-
-Example:
-	rotator@12810000 {
-		compatible = "samsung,exynos4210-rotator";
-		reg = <0x12810000 0x1000>;
-		interrupts = <0 83 0>;
-		clocks = <&clock 278>;
-		clock-names = "rotator";
-	};
diff --git a/Documentation/devicetree/bindings/gpu/samsung-rotator.yaml b/Documentation/devicetree/bindings/gpu/samsung-rotator.yaml
new file mode 100644
index 000000000000..96afafe98388
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpu/samsung-rotator.yaml
@@ -0,0 +1,52 @@ 
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/gpu/samsung-rotator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Samsung Image Rotator
+
+maintainers:
+  - Inki Dae <inki.dae@samsung.com>
+
+properties:
+  compatible:
+    enum:
+      - "samsung,s5pv210-rotator"    # for Rotator IP in S5PV210
+      - "samsung,exynos4210-rotator" # for Rotator IP in Exynos4210
+      - "samsung,exynos4212-rotator" # for Rotator IP in Exynos4212/4412
+      - "samsung,exynos5250-rotator" # for Rotator IP in Exynos5250
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    description: |
+      Clock specifier for rotator clock according to generic clock
+      bindings. (See Documentation/devicetree/bindings/clock/exynos*.txt)
+    maxItems: 1
+
+  clock-names:
+    items:
+    - const: rotator
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+
+examples:
+  - |
+    rotator@12810000 {
+        compatible = "samsung,exynos4210-rotator";
+        reg = <0x12810000 0x1000>;
+        interrupts = <0 83 0>;
+        clocks = <&clock 278>;
+        clock-names = "rotator";
+    };
+