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

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

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
>

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

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

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