| Message ID | 20240310175217.20981-1-animeshagarwal28@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v6] dt-bindings: imx-pata: Convert to dtschema | expand |
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
On 10/03/2024 21:30, Krzysztof Kozlowski wrote: > 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. BTW, If this was not clear, I am quite fed up with these patches, so keep the rule of one version per day. You made quite a lot of changes which were not necessary and I have impression that you should just double check your code *before* sending next version. Best regards, Krzysztof
On Mon, Mar 11, 2024 at 2:00 AM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> 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. I'll stick to 1 version in 1 day from now on. > Why did you changed the sentence from imperative? What for? Please read > again my comments. Ok, I'll change it back to imperative. > What is imx31-pata.dtsi? Where is this file? Sorry for this, I will add the complete path now. > These are not sentences. Please use regular imperative mood with full > stop and capital letters. Noted. > patch 6 of what? There is no patch 6 here. I wanted to say patch v6. > "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." Got it! Adding this in the new patch now. > 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. Sorry for this too. I'll be more cautious while posting. > 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. Noted. Thanks for your patience and time. Animesh
On Mon, Mar 11, 2024 at 2:03 AM Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote: > BTW, If this was not clear, I am quite fed up with these patches, so > keep the rule of one version per day. You made quite a lot of changes > which were not necessary and I have impression that you should just > double check your code *before* sending next version. This was my first attempt at a contribution to the linux kernel. I have learned a lot, I feel like I have wasted a ton of your time. I always try to not make any mistakes before posting but it was clearly not a good try. Moving forward I'll be a lot more cautious and write better code and add proper explanation for the changes I made. Thanks & Regards Animesh
On 3/11/24 12:33, Animesh Agarwal wrote: > On Mon, Mar 11, 2024 at 2:03 AM Krzysztof Kozlowski > <krzysztof.kozlowski@linaro.org> wrote: >> BTW, If this was not clear, I am quite fed up with these patches, so >> keep the rule of one version per day. You made quite a lot of changes >> which were not necessary and I have impression that you should just >> double check your code *before* sending next version. > This was my first attempt at a contribution to the linux kernel. I > have learned a lot, I feel like I have wasted a ton of your time. > I always try to not make any mistakes before posting but it was > clearly not a good try. > Moving forward I'll be a lot more cautious and write better code and > add proper explanation for the changes I made. It is simple: the commit message should always explain *WHAT* you did and *WHY*. This is to give some context to reviewers and to help with checking that your code actually does what you explained. This also helps with potential future issues with a change as the commit message remains in the git log history. Regardless of the version of your patch, always have the what & why explained in your commit message. This implies that the commit message must change if the patch content changes between versions. Keep in mind that the changelog added to a patch is lost when the patch is applied, but the commit message remains. > > Thanks & Regards > Animesh
On Mon, Mar 11, 2024 at 9:39 AM Damien Le Moal <dlemoal@kernel.org> wrote: > It is simple: the commit message should always explain *WHAT* you did and > *WHY*. This is to give some context to reviewers and to help with checking that > your code actually does what you explained. This also helps with potential > future issues with a change as the commit message remains in the git log history. > > Regardless of the version of your patch, always have the what & why explained > in your commit message. This implies that the commit message must change if the > patch content changes between versions. Keep in mind that the changelog added > to a patch is lost when the patch is applied, but the commit message remains. Thank you for your feedback and guidance. Your advice regarding the necessity of explaining both the *WHAT* and *WHY* behind each change is duly noted. Moving forward, I will ensure that my commit messages provide comprehensive context to facilitate smoother reviewing processes and to maintain a clear log history for potential future issues. Thanks & regards, Animesh
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>; + }; + diff --git a/Documentation/devicetree/bindings/ata/imx-pata.txt b/Documentation/devicetree/bindings/ata/imx-pata.txt deleted file mode 100644 index f1172f00188a..000000000000 --- a/Documentation/devicetree/bindings/ata/imx-pata.txt +++ /dev/null @@ -1,16 +0,0 @@ -* Freescale i.MX PATA Controller - -Required properties: -- compatible: "fsl,imx27-pata" -- reg: Address range of the PATA Controller -- interrupts: The interrupt of the PATA Controller -- clocks: the clocks for the PATA Controller - -Example: - - pata: pata@83fe0000 { - compatible = "fsl,imx51-pata", "fsl,imx27-pata"; - reg = <0x83fe0000 0x4000>; - interrupts = <70>; - clocks = <&clks 161>; - };
This patchset converts imx-pata bindings to DT schema. 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 oneOf is also added to allow the usage of imx27 alone. Cleanups are done in patch 6 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