Re: [PATCH v6] dt-bindings: imx-pata: Convert to dtschema

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 10/03/2024 18:52, Animesh Agarwal wrote:

What is happening with your patches? It's 3rd or 4th version the same
day and while it was improving, this version has some weird changes.


> This patchset converts imx-pata bindings to DT schema.

Why did you changed the sentence from imperative? What for? Please read
again my comments.

> file name is changed to fsl,imx-pata to follow vendor,device scheme
> imx31-pata and imx51-pata are added in compatible to ensure this node compiles to
> imx31-pata.dtsi or imx51-pata.dtsi

What is imx31-pata.dtsi? Where is this file?

> oneOf is also added to allow the usage of imx27 alone.

These are not sentences. Please use regular imperative mood with full
stop and capital letters.

> Cleanups are done in patch 6

patch 6 of what? There is no patch 6 here.

"Convert foo bar to DT schema format. Add missing fsl,imx31-pata and
fsl,imx51-pata compatibles during conversion, because they are already
being used in existing DTS."


> 
> Signed-off-by: Animesh Agarwal <animeshagarwal28@gmail.com>
> 
> ---
> Changes in v6:
> - removed items before const due to single element.



> 
> Changes in v5:
> - added oneOf in compatible property to allow the usage of imx27 alone.
> 
> Changes in v4:
> - added fsl,imx31-pata in compatible property as enum
> - imx31-pata was not listed in compatible in original txt binding
> - adding imx31-pata in enum ensures the node compiles to imx31.dtsi
> 
> Changes in v3:
> - added fsl,imx51-pata in compatible property as enum
> - imx51-pata was not listed in compatible in original txt binding
> - adding imx51-pata in enum ensures the node compiles to imx31.dtsi
> - fsl,imx27-pata is added as a const to ensure it is present always
> 
> Changes in v2:
> - fixed style issues
> - compatible property now matches the examples
> - fixed yamllint warnings/errors
> ---
>  .../devicetree/bindings/ata/fsl,imx-pata.yaml | 43 +++++++++++++++++++
>  .../devicetree/bindings/ata/imx-pata.txt      | 16 -------
>  2 files changed, 43 insertions(+), 16 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/ata/fsl,imx-pata.yaml
>  delete mode 100644 Documentation/devicetree/bindings/ata/imx-pata.txt
> 
> diff --git a/Documentation/devicetree/bindings/ata/fsl,imx-pata.yaml b/Documentation/devicetree/bindings/ata/fsl,imx-pata.yaml
> new file mode 100644
> index 000000000000..c108a4b6636a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ata/fsl,imx-pata.yaml
> @@ -0,0 +1,43 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/ata/fsl,imx-pata.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale i.MX PATA Controller
> +
> +maintainers:
> +  - Animesh Agarwal <animeshagarwal28@gmail.com>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - items:
> +          - enum:
> +              - fsl,imx31-pata
> +              - fsl,imx51-pata
> +          - const: fsl,imx27-pata
> +      - const: fsl,imx27-pata
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    items:
> +      - description: PATA Controller interrupts
> +
> +  clocks:
> +    items:
> +      - description: PATA Controller clocks
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    pata: pata@83fe0000 {
> +        compatible = "fsl,imx51-pata","fsl,imx27-pata";
> +        reg = <0x83fe0000 0x4000>;
> +        interrupts = <70>;
> +        clocks = <&clks 161>;
> +    };
> +

Why adding this blank line? It was not here before and no one asked to
you to change anything at this place. How it is possible to edit one
piece of file and cause some entirely unrelated changes in other places?
Please use an editor which you are comfortable with - which you know how
to use.

The use `git add -p`, to see what you are adding to commit. DO NOT USE
`GIT ADD FILE` or `GIT ADD .`. Almost never... Think what you are adding
to the commit.

Best regards,
Krzysztof