[PATCH 0/3] Fix dtbs warnings for arch timer

[PATCH 0/3] Fix dtbs warnings for arch timer

[Kuldeep Singh]

This patchset is an attempt to resolve 'make dtbs_check' warning for
arch timer.

Patch 1 is done in preparation for following patches which defines
compatibles order in more clear way.
Patch 2 documents arm,cortex-a7-timer entry in bindings similar to an
existing entry arm,cortex-a15-timer.
Patch 3 adds above 2 properties in of_match list to bring them in
use. 

Please note, this patchset is based on
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git, master

Kuldeep Singh (3):
  dt-bindings: timer: Rearrange compatible entries of arch timer
  dt-bindings: timer: Document arm,cortex-a7-timer for arch timer
  clocksource: arch_timer: Add arm,cortex-a7/15-timer in of_match list

 .../devicetree/bindings/timer/arm,arch_timer.yaml   | 13 +++++--------
 drivers/clocksource/arm_arch_timer.c                |  2 ++
 2 files changed, 7 insertions(+), 8 deletions(-)

-- 
2.25.1

[PATCH 0/3] Fix dtbs warnings for arch timer

[Kuldeep Singh]

This patchset is an attempt to resolve 'make dtbs_check' warning for
arch timer.

Patch 1 is done in preparation for following patches which defines
compatibles order in more clear way.
Patch 2 documents arm,cortex-a7-timer entry in bindings similar to an
existing entry arm,cortex-a15-timer.
Patch 3 adds above 2 properties in of_match list to bring them in
use. 

Please note, this patchset is based on
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git, master

Kuldeep Singh (3):
  dt-bindings: timer: Rearrange compatible entries of arch timer
  dt-bindings: timer: Document arm,cortex-a7-timer for arch timer
  clocksource: arch_timer: Add arm,cortex-a7/15-timer in of_match list

 .../devicetree/bindings/timer/arm,arch_timer.yaml   | 13 +++++--------
 drivers/clocksource/arm_arch_timer.c                |  2 ++
 2 files changed, 7 insertions(+), 8 deletions(-)

-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/3] dt-bindings: timer: Rearrange compatible entries of arch timer

[Kuldeep Singh]

Compatibles entries of arch timer includes few extra items and enum
pairs which are redundant and can be simplified in a more clear, concise
and readable way. Do it.

Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
---
 .../devicetree/bindings/timer/arm,arch_timer.yaml    | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml b/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml
index df8ce87fd54b..ba2910f0a7b2 100644
--- a/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml
+++ b/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml
@@ -21,17 +21,13 @@ description: |+
 properties:
   compatible:
     oneOf:
+      - enum:
+          - arm,armv7-timer
+          - arm,armv8-timer
       - items:
           - enum:
               - arm,cortex-a15-timer
-          - enum:
-              - arm,armv7-timer
-      - items:
-          - enum:
-              - arm,armv7-timer
-      - items:
-          - enum:
-              - arm,armv8-timer
+          - const: arm,armv7-timer
 
   interrupts:
     minItems: 1
-- 
2.25.1

[PATCH 1/3] dt-bindings: timer: Rearrange compatible entries of arch timer

[Kuldeep Singh]

Compatibles entries of arch timer includes few extra items and enum
pairs which are redundant and can be simplified in a more clear, concise
and readable way. Do it.

Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
---
 .../devicetree/bindings/timer/arm,arch_timer.yaml    | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml b/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml
index df8ce87fd54b..ba2910f0a7b2 100644
--- a/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml
+++ b/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml
@@ -21,17 +21,13 @@ description: |+
 properties:
   compatible:
     oneOf:
+      - enum:
+          - arm,armv7-timer
+          - arm,armv8-timer
       - items:
           - enum:
               - arm,cortex-a15-timer
-          - enum:
-              - arm,armv7-timer
-      - items:
-          - enum:
-              - arm,armv7-timer
-      - items:
-          - enum:
-              - arm,armv8-timer
+          - const: arm,armv7-timer
 
   interrupts:
     minItems: 1
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/3] dt-bindings: timer: Document arm,cortex-a7-timer for arch timer

[Kuldeep Singh]

Renesas RZ/N1D platform uses compatible "arm,cortex-a7-timer" in
conjugation with "arm,armv7-timer". Since, initial entry is not
documented, it start raising dtbs_check warnings.

['arm,cortex-a7-timer', 'arm,armv7-timer'] is too long
'arm,cortex-a7-timer' is not one of ['arm,armv7-timer', 'arm,armv8-timer']
'arm,cortex-a7-timer' is not one of ['arm,cortex-a15-timer']

In general, removing an existing entry is mostly devastating considering
backward compatibility. Therefore, document it.

Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
---
 Documentation/devicetree/bindings/timer/arm,arch_timer.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml b/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml
index ba2910f0a7b2..ea390e5df71d 100644
--- a/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml
+++ b/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml
@@ -26,6 +26,7 @@ properties:
           - arm,armv8-timer
       - items:
           - enum:
+              - arm,cortex-a7-timer
               - arm,cortex-a15-timer
           - const: arm,armv7-timer
 
-- 
2.25.1

[PATCH 2/3] dt-bindings: timer: Document arm, cortex-a7-timer for arch timer

[Kuldeep Singh]

Renesas RZ/N1D platform uses compatible "arm,cortex-a7-timer" in
conjugation with "arm,armv7-timer". Since, initial entry is not
documented, it start raising dtbs_check warnings.

['arm,cortex-a7-timer', 'arm,armv7-timer'] is too long
'arm,cortex-a7-timer' is not one of ['arm,armv7-timer', 'arm,armv8-timer']
'arm,cortex-a7-timer' is not one of ['arm,cortex-a15-timer']

In general, removing an existing entry is mostly devastating considering
backward compatibility. Therefore, document it.

Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
---
 Documentation/devicetree/bindings/timer/arm,arch_timer.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml b/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml
index ba2910f0a7b2..ea390e5df71d 100644
--- a/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml
+++ b/Documentation/devicetree/bindings/timer/arm,arch_timer.yaml
@@ -26,6 +26,7 @@ properties:
           - arm,armv8-timer
       - items:
           - enum:
+              - arm,cortex-a7-timer
               - arm,cortex-a15-timer
           - const: arm,armv7-timer
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 3/3] clocksource: arch_timer: Add arm,cortex-a7/15-timer in of_match list

[Kuldeep Singh]

Few platforms such as Renesas RZ/N1D, Calxeda, Alpine etc. are using
arm,cortex-a15-timer and arm,cortex-a7-timer entries in conjugation with
arm,armv7-timer which are not currently defined in driver file. Add
these entries in arch_timer_of_match list to bring them in use.

Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
---
 drivers/clocksource/arm_arch_timer.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 1ecd52f903b8..805e60c09916 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -1266,6 +1266,8 @@ static int __init arch_timer_mem_register(void __iomem *base, unsigned int irq)
 static const struct of_device_id arch_timer_of_match[] __initconst = {
 	{ .compatible   = "arm,armv7-timer",    },
 	{ .compatible   = "arm,armv8-timer",    },
+	{ .compatible   = "arm,cortex-a7-timer",    },
+	{ .compatible   = "arm,cortex-a15-timer",    },
 	{},
 };
 
-- 
2.25.1

[PATCH 3/3] clocksource: arch_timer: Add arm, cortex-a7/15-timer in of_match list

[Kuldeep Singh]

Few platforms such as Renesas RZ/N1D, Calxeda, Alpine etc. are using
arm,cortex-a15-timer and arm,cortex-a7-timer entries in conjugation with
arm,armv7-timer which are not currently defined in driver file. Add
these entries in arch_timer_of_match list to bring them in use.

Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
---
 drivers/clocksource/arm_arch_timer.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 1ecd52f903b8..805e60c09916 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -1266,6 +1266,8 @@ static int __init arch_timer_mem_register(void __iomem *base, unsigned int irq)
 static const struct of_device_id arch_timer_of_match[] __initconst = {
 	{ .compatible   = "arm,armv7-timer",    },
 	{ .compatible   = "arm,armv8-timer",    },
+	{ .compatible   = "arm,cortex-a7-timer",    },
+	{ .compatible   = "arm,cortex-a15-timer",    },
 	{},
 };
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] clocksource: arch_timer: Add arm,cortex-a7/15-timer in of_match list

[Marc Zyngier]

Hi Kuldeep,

On Wed, 16 Mar 2022 09:54:32 +0000,
Kuldeep Singh <singh.kuldeep87k@gmail.com> wrote:
> 
> Few platforms such as Renesas RZ/N1D, Calxeda, Alpine etc. are using
> arm,cortex-a15-timer and arm,cortex-a7-timer entries in conjugation with
> arm,armv7-timer which are not currently defined in driver file. Add
> these entries in arch_timer_of_match list to bring them in use.

"arm,armv7-timer" already acts as a catch all (and is mandatory
anyway), and there is no difference in handling as a result of this.

So what is this fixing?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH 3/3] clocksource: arch_timer: Add arm, cortex-a7/15-timer in of_match list

[Marc Zyngier]

Hi Kuldeep,

On Wed, 16 Mar 2022 09:54:32 +0000,
Kuldeep Singh <singh.kuldeep87k@gmail.com> wrote:
> 
> Few platforms such as Renesas RZ/N1D, Calxeda, Alpine etc. are using
> arm,cortex-a15-timer and arm,cortex-a7-timer entries in conjugation with
> arm,armv7-timer which are not currently defined in driver file. Add
> these entries in arch_timer_of_match list to bring them in use.

"arm,armv7-timer" already acts as a catch all (and is mandatory
anyway), and there is no difference in handling as a result of this.

So what is this fixing?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] Fix dtbs warnings for arch timer

[Marc Zyngier]

On 2022-03-16 09:54, Kuldeep Singh wrote:
> This patchset is an attempt to resolve 'make dtbs_check' warning for
> arch timer.
> 
> Patch 1 is done in preparation for following patches which defines
> compatibles order in more clear way.
> Patch 2 documents arm,cortex-a7-timer entry in bindings similar to an
> existing entry arm,cortex-a15-timer.
> Patch 3 adds above 2 properties in of_match list to bring them in
> use.
> 
> Please note, this patchset is based on
> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git, 
> master
> 
> Kuldeep Singh (3):
>   dt-bindings: timer: Rearrange compatible entries of arch timer
>   dt-bindings: timer: Document arm,cortex-a7-timer for arch timer
>   clocksource: arch_timer: Add arm,cortex-a7/15-timer in of_match list
> 
>  .../devicetree/bindings/timer/arm,arch_timer.yaml   | 13 +++++--------
>  drivers/clocksource/arm_arch_timer.c                |  2 ++
>  2 files changed, 7 insertions(+), 8 deletions(-)

Please use my @kernel.org address exclusively. My @arm.com
address stopped working over two years ago, and the MAINTAINERS
file shows the right addresses.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH 0/3] Fix dtbs warnings for arch timer

[Marc Zyngier]

On 2022-03-16 09:54, Kuldeep Singh wrote:
> This patchset is an attempt to resolve 'make dtbs_check' warning for
> arch timer.
> 
> Patch 1 is done in preparation for following patches which defines
> compatibles order in more clear way.
> Patch 2 documents arm,cortex-a7-timer entry in bindings similar to an
> existing entry arm,cortex-a15-timer.
> Patch 3 adds above 2 properties in of_match list to bring them in
> use.
> 
> Please note, this patchset is based on
> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git, 
> master
> 
> Kuldeep Singh (3):
>   dt-bindings: timer: Rearrange compatible entries of arch timer
>   dt-bindings: timer: Document arm,cortex-a7-timer for arch timer
>   clocksource: arch_timer: Add arm,cortex-a7/15-timer in of_match list
> 
>  .../devicetree/bindings/timer/arm,arch_timer.yaml   | 13 +++++--------
>  drivers/clocksource/arm_arch_timer.c                |  2 ++
>  2 files changed, 7 insertions(+), 8 deletions(-)

Please use my @kernel.org address exclusively. My @arm.com
address stopped working over two years ago, and the MAINTAINERS
file shows the right addresses.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] dt-bindings: timer: Document arm,cortex-a7-timer for arch timer

[Krzysztof Kozlowski]

On 16/03/2022 10:54, Kuldeep Singh wrote:
> Renesas RZ/N1D platform uses compatible "arm,cortex-a7-timer" in
> conjugation with "arm,armv7-timer". Since, initial entry is not
> documented, it start raising dtbs_check warnings.
> 
> ['arm,cortex-a7-timer', 'arm,armv7-timer'] is too long
> 'arm,cortex-a7-timer' is not one of ['arm,armv7-timer', 'arm,armv8-timer']
> 'arm,cortex-a7-timer' is not one of ['arm,cortex-a15-timer']
> 
> In general, removing an existing entry is mostly devastating considering
> backward compatibility. Therefore, document it.

How removing undocumented and unused compatible is devastating for
backwards compatibility? I don't see it.

DTS is rather wrong, not bindings. Otherwise please explain it more, why
DTS is correct.



Best regards,
Krzysztof

Re: [PATCH 2/3] dt-bindings: timer: Document arm, cortex-a7-timer for arch timer

[Krzysztof Kozlowski]

On 16/03/2022 10:54, Kuldeep Singh wrote:
> Renesas RZ/N1D platform uses compatible "arm,cortex-a7-timer" in
> conjugation with "arm,armv7-timer". Since, initial entry is not
> documented, it start raising dtbs_check warnings.
> 
> ['arm,cortex-a7-timer', 'arm,armv7-timer'] is too long
> 'arm,cortex-a7-timer' is not one of ['arm,armv7-timer', 'arm,armv8-timer']
> 'arm,cortex-a7-timer' is not one of ['arm,cortex-a15-timer']
> 
> In general, removing an existing entry is mostly devastating considering
> backward compatibility. Therefore, document it.

How removing undocumented and unused compatible is devastating for
backwards compatibility? I don't see it.

DTS is rather wrong, not bindings. Otherwise please explain it more, why
DTS is correct.



Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] dt-bindings: timer: Rearrange compatible entries of arch timer

[Krzysztof Kozlowski]

On 16/03/2022 10:54, Kuldeep Singh wrote:
> Compatibles entries of arch timer includes few extra items and enum
> pairs which are redundant and can be simplified in a more clear, concise
> and readable way. Do it.
> 
> Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
> ---
>  .../devicetree/bindings/timer/arm,arch_timer.yaml    | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>


Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: timer: Rearrange compatible entries of arch timer

[Krzysztof Kozlowski]

On 16/03/2022 10:54, Kuldeep Singh wrote:
> Compatibles entries of arch timer includes few extra items and enum
> pairs which are redundant and can be simplified in a more clear, concise
> and readable way. Do it.
> 
> Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
> ---
>  .../devicetree/bindings/timer/arm,arch_timer.yaml    | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>


Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] clocksource: arch_timer: Add arm,cortex-a7/15-timer in of_match list

[Krzysztof Kozlowski]

On 16/03/2022 10:54, Kuldeep Singh wrote:
> Few platforms such as Renesas RZ/N1D, Calxeda, Alpine etc. are using
> arm,cortex-a15-timer and arm,cortex-a7-timer entries in conjugation with
> arm,armv7-timer which are not currently defined in driver file. Add
> these entries in arch_timer_of_match list to bring them in use.
> 

This looks wrong (also Marc pointed this out) and rationale is not
sufficient. Why do you need these compatibles in the driver?

Best regards,
Krzysztof

Re: [PATCH 3/3] clocksource: arch_timer: Add arm,cortex-a7/15-timer in of_match list

[Krzysztof Kozlowski]

On 16/03/2022 10:54, Kuldeep Singh wrote:
> Few platforms such as Renesas RZ/N1D, Calxeda, Alpine etc. are using
> arm,cortex-a15-timer and arm,cortex-a7-timer entries in conjugation with
> arm,armv7-timer which are not currently defined in driver file. Add
> these entries in arch_timer_of_match list to bring them in use.
> 

This looks wrong (also Marc pointed this out) and rationale is not
sufficient. Why do you need these compatibles in the driver?

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] Fix dtbs warnings for arch timer

[Kuldeep Singh]

On Wed, Mar 16, 2022 at 11:27:10AM +0000, Marc Zyngier wrote:
> On 2022-03-16 09:54, Kuldeep Singh wrote:
> > This patchset is an attempt to resolve 'make dtbs_check' warning for
> > arch timer.
> > 
> > Patch 1 is done in preparation for following patches which defines
> > compatibles order in more clear way.
> > Patch 2 documents arm,cortex-a7-timer entry in bindings similar to an
> > existing entry arm,cortex-a15-timer.
> > Patch 3 adds above 2 properties in of_match list to bring them in
> > use.
> > 
> > Please note, this patchset is based on
> > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git, master
> > 
> > Kuldeep Singh (3):
> >   dt-bindings: timer: Rearrange compatible entries of arch timer
> >   dt-bindings: timer: Document arm,cortex-a7-timer for arch timer
> >   clocksource: arch_timer: Add arm,cortex-a7/15-timer in of_match list
> > 
> >  .../devicetree/bindings/timer/arm,arch_timer.yaml   | 13 +++++--------
> >  drivers/clocksource/arm_arch_timer.c                |  2 ++
> >  2 files changed, 7 insertions(+), 8 deletions(-)
> 
> Please use my @kernel.org address exclusively. My @arm.com
> address stopped working over two years ago, and the MAINTAINERS
> file shows the right addresses.

Hi Marc,

I have tocmd and cccmd set in my gitconfig and it generated mail
addresses automatically so I assume it is correct.
tocmd ="`pwd`/scripts/get_maintainer.pl --nogit --nogit-fallback --norolestats --nol"

It seems your arm mail address got configured from binding file and not
from MAINTAINERS.

Regards
Kuldeep

> 
> Thanks,
> 
>         M.
> -- 
> Jazz is not dead. It just smells funny...

Re: [PATCH 0/3] Fix dtbs warnings for arch timer

[Kuldeep Singh]

On Wed, Mar 16, 2022 at 11:27:10AM +0000, Marc Zyngier wrote:
> On 2022-03-16 09:54, Kuldeep Singh wrote:
> > This patchset is an attempt to resolve 'make dtbs_check' warning for
> > arch timer.
> > 
> > Patch 1 is done in preparation for following patches which defines
> > compatibles order in more clear way.
> > Patch 2 documents arm,cortex-a7-timer entry in bindings similar to an
> > existing entry arm,cortex-a15-timer.
> > Patch 3 adds above 2 properties in of_match list to bring them in
> > use.
> > 
> > Please note, this patchset is based on
> > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git, master
> > 
> > Kuldeep Singh (3):
> >   dt-bindings: timer: Rearrange compatible entries of arch timer
> >   dt-bindings: timer: Document arm,cortex-a7-timer for arch timer
> >   clocksource: arch_timer: Add arm,cortex-a7/15-timer in of_match list
> > 
> >  .../devicetree/bindings/timer/arm,arch_timer.yaml   | 13 +++++--------
> >  drivers/clocksource/arm_arch_timer.c                |  2 ++
> >  2 files changed, 7 insertions(+), 8 deletions(-)
> 
> Please use my @kernel.org address exclusively. My @arm.com
> address stopped working over two years ago, and the MAINTAINERS
> file shows the right addresses.

Hi Marc,

I have tocmd and cccmd set in my gitconfig and it generated mail
addresses automatically so I assume it is correct.
tocmd ="`pwd`/scripts/get_maintainer.pl --nogit --nogit-fallback --norolestats --nol"

It seems your arm mail address got configured from binding file and not
from MAINTAINERS.

Regards
Kuldeep

> 
> Thanks,
> 
>         M.
> -- 
> Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] clocksource: arch_timer: Add arm,cortex-a7/15-timer in of_match list

[Kuldeep Singh]

On Wed, Mar 16, 2022 at 05:30:26PM +0100, Krzysztof Kozlowski wrote:
> On 16/03/2022 10:54, Kuldeep Singh wrote:
> > Few platforms such as Renesas RZ/N1D, Calxeda, Alpine etc. are using
> > arm,cortex-a15-timer and arm,cortex-a7-timer entries in conjugation with
> > arm,armv7-timer which are not currently defined in driver file. Add
> > these entries in arch_timer_of_match list to bring them in use.
> > 
> 
> This looks wrong (also Marc pointed this out) and rationale is not
> sufficient. Why do you need these compatibles in the driver?

Hi Krzysztof and Marc,

I find myself in trouble whenever dealing with compatible entries and
had 2 options when I stumble this issue.
	1. Remove unused compatible
	2. Add required compatible to binding and driver

My past experience and advise from other developer says not to remove an
existing compatible. And also I found "arm,cortex-a15-timer" in binding
which was again not documented and was present in DT. This prompted me
to go for second option and make necessary additions in binding and
driver following current entries.

As per your perspective, current configuration isn't apt which means
"arm,cortex-a15-timer" is a stub and is wrongly present in binding.

I also observed many other DTs have compatibles which are not present in
driver. What is an ideal idealogy behind such cases?

- Kuldeep

Re: [PATCH 3/3] clocksource: arch_timer: Add arm,cortex-a7/15-timer in of_match list

[Kuldeep Singh]

On Wed, Mar 16, 2022 at 05:30:26PM +0100, Krzysztof Kozlowski wrote:
> On 16/03/2022 10:54, Kuldeep Singh wrote:
> > Few platforms such as Renesas RZ/N1D, Calxeda, Alpine etc. are using
> > arm,cortex-a15-timer and arm,cortex-a7-timer entries in conjugation with
> > arm,armv7-timer which are not currently defined in driver file. Add
> > these entries in arch_timer_of_match list to bring them in use.
> > 
> 
> This looks wrong (also Marc pointed this out) and rationale is not
> sufficient. Why do you need these compatibles in the driver?

Hi Krzysztof and Marc,

I find myself in trouble whenever dealing with compatible entries and
had 2 options when I stumble this issue.
	1. Remove unused compatible
	2. Add required compatible to binding and driver

My past experience and advise from other developer says not to remove an
existing compatible. And also I found "arm,cortex-a15-timer" in binding
which was again not documented and was present in DT. This prompted me
to go for second option and make necessary additions in binding and
driver following current entries.

As per your perspective, current configuration isn't apt which means
"arm,cortex-a15-timer" is a stub and is wrongly present in binding.

I also observed many other DTs have compatibles which are not present in
driver. What is an ideal idealogy behind such cases?

- Kuldeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/3] dt-bindings: timer: Document arm,cortex-a7-timer for arch timer

[Kuldeep Singh]

On Wed, Mar 16, 2022 at 05:28:49PM +0100, Krzysztof Kozlowski wrote:
> On 16/03/2022 10:54, Kuldeep Singh wrote:
> > Renesas RZ/N1D platform uses compatible "arm,cortex-a7-timer" in
> > conjugation with "arm,armv7-timer". Since, initial entry is not
> > documented, it start raising dtbs_check warnings.
> > 
> > ['arm,cortex-a7-timer', 'arm,armv7-timer'] is too long
> > 'arm,cortex-a7-timer' is not one of ['arm,armv7-timer', 'arm,armv8-timer']
> > 'arm,cortex-a7-timer' is not one of ['arm,cortex-a15-timer']
> > 
> > In general, removing an existing entry is mostly devastating considering
> > backward compatibility. Therefore, document it.
> 
> How removing undocumented and unused compatible is devastating for
> backwards compatibility? I don't see it.
> 
> DTS is rather wrong, not bindings. Otherwise please explain it more, why
> DTS is correct.

Please see my comment on 3/3 patch.

- Kuldeep

Re: [PATCH 2/3] dt-bindings: timer: Document arm, cortex-a7-timer for arch timer

[Kuldeep Singh]

On Wed, Mar 16, 2022 at 05:28:49PM +0100, Krzysztof Kozlowski wrote:
> On 16/03/2022 10:54, Kuldeep Singh wrote:
> > Renesas RZ/N1D platform uses compatible "arm,cortex-a7-timer" in
> > conjugation with "arm,armv7-timer". Since, initial entry is not
> > documented, it start raising dtbs_check warnings.
> > 
> > ['arm,cortex-a7-timer', 'arm,armv7-timer'] is too long
> > 'arm,cortex-a7-timer' is not one of ['arm,armv7-timer', 'arm,armv8-timer']
> > 'arm,cortex-a7-timer' is not one of ['arm,cortex-a15-timer']
> > 
> > In general, removing an existing entry is mostly devastating considering
> > backward compatibility. Therefore, document it.
> 
> How removing undocumented and unused compatible is devastating for
> backwards compatibility? I don't see it.
> 
> DTS is rather wrong, not bindings. Otherwise please explain it more, why
> DTS is correct.

Please see my comment on 3/3 patch.

- Kuldeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] clocksource: arch_timer: Add arm,cortex-a7/15-timer in of_match list

[Marc Zyngier]

On Wed, 16 Mar 2022 17:41:08 +0000,
Kuldeep Singh <singh.kuldeep87k@gmail.com> wrote:
> 
> On Wed, Mar 16, 2022 at 05:30:26PM +0100, Krzysztof Kozlowski wrote:
> > On 16/03/2022 10:54, Kuldeep Singh wrote:
> > > Few platforms such as Renesas RZ/N1D, Calxeda, Alpine etc. are using
> > > arm,cortex-a15-timer and arm,cortex-a7-timer entries in conjugation with
> > > arm,armv7-timer which are not currently defined in driver file. Add
> > > these entries in arch_timer_of_match list to bring them in use.
> > > 
> > 
> > This looks wrong (also Marc pointed this out) and rationale is not
> > sufficient. Why do you need these compatibles in the driver?
> 
> Hi Krzysztof and Marc,
> 
> I find myself in trouble whenever dealing with compatible entries and
> had 2 options when I stumble this issue.
> 	1. Remove unused compatible

That'd be silly.

> 	2. Add required compatible to binding and driver

To the binding, yes. But to the driver?

> My past experience and advise from other developer says not to remove an
> existing compatible. And also I found "arm,cortex-a15-timer" in binding
> which was again not documented and was present in DT. This prompted me
> to go for second option and make necessary additions in binding and
> driver following current entries.

The "arm,cortex-a15-timer" compatible is documentation, and only
that. If, one day, we find a bug in this implementation, we could work
around it in the driver thanks to the separate compatible (although in
this case, we'd have much better way of doing that).

> As per your perspective, current configuration isn't apt which means
> "arm,cortex-a15-timer" is a stub and is wrongly present in binding.

That's not what I said. This compatible string is perfectly fine, and
accurately describe the HW. The driver doesn't need to know about the
fine details of the implementation, and is perfectly happy with the
current state of things.

Think of it as an instance of a class. The driver doesn't need to know
the instance, only that it is a certain class.

> I also observed many other DTs have compatibles which are not present in
> driver. What is an ideal idealogy behind such cases?

I think I've made myself clear above.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH 3/3] clocksource: arch_timer: Add arm, cortex-a7/15-timer in of_match list

[Marc Zyngier]

On Wed, 16 Mar 2022 17:41:08 +0000,
Kuldeep Singh <singh.kuldeep87k@gmail.com> wrote:
> 
> On Wed, Mar 16, 2022 at 05:30:26PM +0100, Krzysztof Kozlowski wrote:
> > On 16/03/2022 10:54, Kuldeep Singh wrote:
> > > Few platforms such as Renesas RZ/N1D, Calxeda, Alpine etc. are using
> > > arm,cortex-a15-timer and arm,cortex-a7-timer entries in conjugation with
> > > arm,armv7-timer which are not currently defined in driver file. Add
> > > these entries in arch_timer_of_match list to bring them in use.
> > > 
> > 
> > This looks wrong (also Marc pointed this out) and rationale is not
> > sufficient. Why do you need these compatibles in the driver?
> 
> Hi Krzysztof and Marc,
> 
> I find myself in trouble whenever dealing with compatible entries and
> had 2 options when I stumble this issue.
> 	1. Remove unused compatible

That'd be silly.

> 	2. Add required compatible to binding and driver

To the binding, yes. But to the driver?

> My past experience and advise from other developer says not to remove an
> existing compatible. And also I found "arm,cortex-a15-timer" in binding
> which was again not documented and was present in DT. This prompted me
> to go for second option and make necessary additions in binding and
> driver following current entries.

The "arm,cortex-a15-timer" compatible is documentation, and only
that. If, one day, we find a bug in this implementation, we could work
around it in the driver thanks to the separate compatible (although in
this case, we'd have much better way of doing that).

> As per your perspective, current configuration isn't apt which means
> "arm,cortex-a15-timer" is a stub and is wrongly present in binding.

That's not what I said. This compatible string is perfectly fine, and
accurately describe the HW. The driver doesn't need to know about the
fine details of the implementation, and is perfectly happy with the
current state of things.

Think of it as an instance of a class. The driver doesn't need to know
the instance, only that it is a certain class.

> I also observed many other DTs have compatibles which are not present in
> driver. What is an ideal idealogy behind such cases?

I think I've made myself clear above.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] Fix dtbs warnings for arch timer

[Marc Zyngier]

On Wed, 16 Mar 2022 17:20:51 +0000,
Kuldeep Singh <singh.kuldeep87k@gmail.com> wrote:
> 
> On Wed, Mar 16, 2022 at 11:27:10AM +0000, Marc Zyngier wrote:
> > On 2022-03-16 09:54, Kuldeep Singh wrote:
> > > This patchset is an attempt to resolve 'make dtbs_check' warning for
> > > arch timer.
> > > 
> > > Patch 1 is done in preparation for following patches which defines
> > > compatibles order in more clear way.
> > > Patch 2 documents arm,cortex-a7-timer entry in bindings similar to an
> > > existing entry arm,cortex-a15-timer.
> > > Patch 3 adds above 2 properties in of_match list to bring them in
> > > use.
> > > 
> > > Please note, this patchset is based on
> > > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git, master
> > > 
> > > Kuldeep Singh (3):
> > >   dt-bindings: timer: Rearrange compatible entries of arch timer
> > >   dt-bindings: timer: Document arm,cortex-a7-timer for arch timer
> > >   clocksource: arch_timer: Add arm,cortex-a7/15-timer in of_match list
> > > 
> > >  .../devicetree/bindings/timer/arm,arch_timer.yaml   | 13 +++++--------
> > >  drivers/clocksource/arm_arch_timer.c                |  2 ++
> > >  2 files changed, 7 insertions(+), 8 deletions(-)
> > 
> > Please use my @kernel.org address exclusively. My @arm.com
> > address stopped working over two years ago, and the MAINTAINERS
> > file shows the right addresses.
> 
> Hi Marc,
> 
> I have tocmd and cccmd set in my gitconfig and it generated mail
> addresses automatically so I assume it is correct.
> tocmd ="`pwd`/scripts/get_maintainer.pl --nogit --nogit-fallback --norolestats --nol"
> 
> It seems your arm mail address got configured from binding file and not
> from MAINTAINERS.

It is a bug in get_maintainer.pl. You'll have to manually apply the
.mailmap transformation.

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH 0/3] Fix dtbs warnings for arch timer

[Marc Zyngier]

On Wed, 16 Mar 2022 17:20:51 +0000,
Kuldeep Singh <singh.kuldeep87k@gmail.com> wrote:
> 
> On Wed, Mar 16, 2022 at 11:27:10AM +0000, Marc Zyngier wrote:
> > On 2022-03-16 09:54, Kuldeep Singh wrote:
> > > This patchset is an attempt to resolve 'make dtbs_check' warning for
> > > arch timer.
> > > 
> > > Patch 1 is done in preparation for following patches which defines
> > > compatibles order in more clear way.
> > > Patch 2 documents arm,cortex-a7-timer entry in bindings similar to an
> > > existing entry arm,cortex-a15-timer.
> > > Patch 3 adds above 2 properties in of_match list to bring them in
> > > use.
> > > 
> > > Please note, this patchset is based on
> > > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git, master
> > > 
> > > Kuldeep Singh (3):
> > >   dt-bindings: timer: Rearrange compatible entries of arch timer
> > >   dt-bindings: timer: Document arm,cortex-a7-timer for arch timer
> > >   clocksource: arch_timer: Add arm,cortex-a7/15-timer in of_match list
> > > 
> > >  .../devicetree/bindings/timer/arm,arch_timer.yaml   | 13 +++++--------
> > >  drivers/clocksource/arm_arch_timer.c                |  2 ++
> > >  2 files changed, 7 insertions(+), 8 deletions(-)
> > 
> > Please use my @kernel.org address exclusively. My @arm.com
> > address stopped working over two years ago, and the MAINTAINERS
> > file shows the right addresses.
> 
> Hi Marc,
> 
> I have tocmd and cccmd set in my gitconfig and it generated mail
> addresses automatically so I assume it is correct.
> tocmd ="`pwd`/scripts/get_maintainer.pl --nogit --nogit-fallback --norolestats --nol"
> 
> It seems your arm mail address got configured from binding file and not
> from MAINTAINERS.

It is a bug in get_maintainer.pl. You'll have to manually apply the
.mailmap transformation.

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] clocksource: arch_timer: Add arm,cortex-a7/15-timer in of_match list

[Kuldeep Singh]

On Wed, Mar 16, 2022 at 06:43:15PM +0000, Marc Zyngier wrote:
> On Wed, 16 Mar 2022 17:41:08 +0000,
> Kuldeep Singh <singh.kuldeep87k@gmail.com> wrote:
> > 
> > On Wed, Mar 16, 2022 at 05:30:26PM +0100, Krzysztof Kozlowski wrote:
> > > On 16/03/2022 10:54, Kuldeep Singh wrote:
> > > > Few platforms such as Renesas RZ/N1D, Calxeda, Alpine etc. are using
> > > > arm,cortex-a15-timer and arm,cortex-a7-timer entries in conjugation with
> > > > arm,armv7-timer which are not currently defined in driver file. Add
> > > > these entries in arch_timer_of_match list to bring them in use.
> > > > 
> > > 
> > > This looks wrong (also Marc pointed this out) and rationale is not
> > > sufficient. Why do you need these compatibles in the driver?
> > 
> > Hi Krzysztof and Marc,
> > 
> > I find myself in trouble whenever dealing with compatible entries and
> > had 2 options when I stumble this issue.
> > 	1. Remove unused compatible
> 
> That'd be silly.
> 
> > 	2. Add required compatible to binding and driver
> 
> To the binding, yes. But to the driver?
> 
> > My past experience and advise from other developer says not to remove an
> > existing compatible. And also I found "arm,cortex-a15-timer" in binding
> > which was again not documented and was present in DT. This prompted me
> > to go for second option and make necessary additions in binding and
> > driver following current entries.
> 
> The "arm,cortex-a15-timer" compatible is documentation, and only
> that. If, one day, we find a bug in this implementation, we could work
> around it in the driver thanks to the separate compatible (although in
> this case, we'd have much better way of doing that).
> 
> > As per your perspective, current configuration isn't apt which means
> > "arm,cortex-a15-timer" is a stub and is wrongly present in binding.
> 
> That's not what I said. This compatible string is perfectly fine, and
> accurately describe the HW. The driver doesn't need to know about the
> fine details of the implementation, and is perfectly happy with the
> current state of things.
> 
> Think of it as an instance of a class. The driver doesn't need to know
> the instance, only that it is a certain class.
> 

Thanks Marc for sharing knowledge. This was indeed helpful.
To sum up from what I understood, bindings and DTs should always be in
sync and driver file may not need to define all compatible entries as
long as purpose is served.

This means no driver change will be required to address
"arm,cortex-a7-timer". To which I have a question to Krzysztof.

Hi Krzysztof,

As per your comments on 2/3 patch, that it's DT which is not aligned
with binding w.r.t arm,cortex-a7-timer.

What makes "arm,cortex-a7-timer" an invalid entry from binding
perspective when we have a similar entry "arm,cortex-a15-timer" already
present?

I think we should share some common grounds here and keep both of them
in bindings or remove them altogether. I prefer first option, What's
your say?
Or please let me know in case there's better way to address this.

- Kuldeep

> > I also observed many other DTs have compatibles which are not present in
> > driver. What is an ideal idealogy behind such cases?
> 
> I think I've made myself clear above.
> 
> Thanks,
> 
> 	M.
> 
> -- 
> Without deviation from the norm, progress is not possible.

Re: [PATCH 3/3] clocksource: arch_timer: Add arm,cortex-a7/15-timer in of_match list

[Kuldeep Singh]

On Wed, Mar 16, 2022 at 06:43:15PM +0000, Marc Zyngier wrote:
> On Wed, 16 Mar 2022 17:41:08 +0000,
> Kuldeep Singh <singh.kuldeep87k@gmail.com> wrote:
> > 
> > On Wed, Mar 16, 2022 at 05:30:26PM +0100, Krzysztof Kozlowski wrote:
> > > On 16/03/2022 10:54, Kuldeep Singh wrote:
> > > > Few platforms such as Renesas RZ/N1D, Calxeda, Alpine etc. are using
> > > > arm,cortex-a15-timer and arm,cortex-a7-timer entries in conjugation with
> > > > arm,armv7-timer which are not currently defined in driver file. Add
> > > > these entries in arch_timer_of_match list to bring them in use.
> > > > 
> > > 
> > > This looks wrong (also Marc pointed this out) and rationale is not
> > > sufficient. Why do you need these compatibles in the driver?
> > 
> > Hi Krzysztof and Marc,
> > 
> > I find myself in trouble whenever dealing with compatible entries and
> > had 2 options when I stumble this issue.
> > 	1. Remove unused compatible
> 
> That'd be silly.
> 
> > 	2. Add required compatible to binding and driver
> 
> To the binding, yes. But to the driver?
> 
> > My past experience and advise from other developer says not to remove an
> > existing compatible. And also I found "arm,cortex-a15-timer" in binding
> > which was again not documented and was present in DT. This prompted me
> > to go for second option and make necessary additions in binding and
> > driver following current entries.
> 
> The "arm,cortex-a15-timer" compatible is documentation, and only
> that. If, one day, we find a bug in this implementation, we could work
> around it in the driver thanks to the separate compatible (although in
> this case, we'd have much better way of doing that).
> 
> > As per your perspective, current configuration isn't apt which means
> > "arm,cortex-a15-timer" is a stub and is wrongly present in binding.
> 
> That's not what I said. This compatible string is perfectly fine, and
> accurately describe the HW. The driver doesn't need to know about the
> fine details of the implementation, and is perfectly happy with the
> current state of things.
> 
> Think of it as an instance of a class. The driver doesn't need to know
> the instance, only that it is a certain class.
> 

Thanks Marc for sharing knowledge. This was indeed helpful.
To sum up from what I understood, bindings and DTs should always be in
sync and driver file may not need to define all compatible entries as
long as purpose is served.

This means no driver change will be required to address
"arm,cortex-a7-timer". To which I have a question to Krzysztof.

Hi Krzysztof,

As per your comments on 2/3 patch, that it's DT which is not aligned
with binding w.r.t arm,cortex-a7-timer.

What makes "arm,cortex-a7-timer" an invalid entry from binding
perspective when we have a similar entry "arm,cortex-a15-timer" already
present?

I think we should share some common grounds here and keep both of them
in bindings or remove them altogether. I prefer first option, What's
your say?
Or please let me know in case there's better way to address this.

- Kuldeep

> > I also observed many other DTs have compatibles which are not present in
> > driver. What is an ideal idealogy behind such cases?
> 
> I think I've made myself clear above.
> 
> Thanks,
> 
> 	M.
> 
> -- 
> Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/3] dt-bindings: timer: Rearrange compatible entries of arch timer

[Kuldeep Singh]

On Wed, Mar 16, 2022 at 05:29:07PM +0100, Krzysztof Kozlowski wrote:
> On 16/03/2022 10:54, Kuldeep Singh wrote:
> > Compatibles entries of arch timer includes few extra items and enum
> > pairs which are redundant and can be simplified in a more clear, concise
> > and readable way. Do it.
> > 
> > Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
> > ---
> >  .../devicetree/bindings/timer/arm,arch_timer.yaml    | 12 ++++--------
> >  1 file changed, 4 insertions(+), 8 deletions(-)
> > 
> 
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> 

Thanks for your time in reviewing this.

- Kuldeep

Re: [PATCH 1/3] dt-bindings: timer: Rearrange compatible entries of arch timer

[Kuldeep Singh]

On Wed, Mar 16, 2022 at 05:29:07PM +0100, Krzysztof Kozlowski wrote:
> On 16/03/2022 10:54, Kuldeep Singh wrote:
> > Compatibles entries of arch timer includes few extra items and enum
> > pairs which are redundant and can be simplified in a more clear, concise
> > and readable way. Do it.
> > 
> > Signed-off-by: Kuldeep Singh <singh.kuldeep87k@gmail.com>
> > ---
> >  .../devicetree/bindings/timer/arm,arch_timer.yaml    | 12 ++++--------
> >  1 file changed, 4 insertions(+), 8 deletions(-)
> > 
> 
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
> 

Thanks for your time in reviewing this.

- Kuldeep

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 3/3] clocksource: arch_timer: Add arm,cortex-a7/15-timer in of_match list

[Krzysztof Kozlowski]

On 17/03/2022 07:59, Kuldeep Singh wrote:
> On Wed, Mar 16, 2022 at 06:43:15PM +0000, Marc Zyngier wrote:
>> On Wed, 16 Mar 2022 17:41:08 +0000,
>> Kuldeep Singh <singh.kuldeep87k@gmail.com> wrote:
>>>
>>> On Wed, Mar 16, 2022 at 05:30:26PM +0100, Krzysztof Kozlowski wrote:
>>>> On 16/03/2022 10:54, Kuldeep Singh wrote:
>>>>> Few platforms such as Renesas RZ/N1D, Calxeda, Alpine etc. are using
>>>>> arm,cortex-a15-timer and arm,cortex-a7-timer entries in conjugation with
>>>>> arm,armv7-timer which are not currently defined in driver file. Add
>>>>> these entries in arch_timer_of_match list to bring them in use.
>>>>>
>>>>
>>>> This looks wrong (also Marc pointed this out) and rationale is not
>>>> sufficient. Why do you need these compatibles in the driver?
>>>
>>> Hi Krzysztof and Marc,
>>>
>>> I find myself in trouble whenever dealing with compatible entries and
>>> had 2 options when I stumble this issue.
>>> 	1. Remove unused compatible
>>
>> That'd be silly.
>>
>>> 	2. Add required compatible to binding and driver
>>
>> To the binding, yes. But to the driver?
>>
>>> My past experience and advise from other developer says not to remove an
>>> existing compatible. And also I found "arm,cortex-a15-timer" in binding
>>> which was again not documented and was present in DT. This prompted me
>>> to go for second option and make necessary additions in binding and
>>> driver following current entries.
>>
>> The "arm,cortex-a15-timer" compatible is documentation, and only
>> that. If, one day, we find a bug in this implementation, we could work
>> around it in the driver thanks to the separate compatible (although in
>> this case, we'd have much better way of doing that).
>>
>>> As per your perspective, current configuration isn't apt which means
>>> "arm,cortex-a15-timer" is a stub and is wrongly present in binding.
>>
>> That's not what I said. This compatible string is perfectly fine, and
>> accurately describe the HW. The driver doesn't need to know about the
>> fine details of the implementation, and is perfectly happy with the
>> current state of things.
>>
>> Think of it as an instance of a class. The driver doesn't need to know
>> the instance, only that it is a certain class.
>>
> 
> Thanks Marc for sharing knowledge. This was indeed helpful.
> To sum up from what I understood, bindings and DTs should always be in
> sync and driver file may not need to define all compatible entries as
> long as purpose is served.
> 
> This means no driver change will be required to address
> "arm,cortex-a7-timer". To which I have a question to Krzysztof.
> 
> Hi Krzysztof,
> 
> As per your comments on 2/3 patch, that it's DT which is not aligned
> with binding w.r.t arm,cortex-a7-timer.
> 
> What makes "arm,cortex-a7-timer" an invalid entry from binding
> perspective when we have a similar entry "arm,cortex-a15-timer" already
> present?
> 
> I think we should share some common grounds here and keep both of them
> in bindings or remove them altogether. I prefer first option, What's
> your say?

In this case the compatible should be added, just please explain it in
the message. Your previous commit msg was saying about disastrous
backward compatibility issue which so far does not exist here. It's
simply more detailed compatible.

There were few other cases where more detailed compatible was actually
unwanted, so that's why each case should be analyzed.

Best regards,
Krzysztof

Re: [PATCH 3/3] clocksource: arch_timer: Add arm,cortex-a7/15-timer in of_match list

[Krzysztof Kozlowski]

On 17/03/2022 07:59, Kuldeep Singh wrote:
> On Wed, Mar 16, 2022 at 06:43:15PM +0000, Marc Zyngier wrote:
>> On Wed, 16 Mar 2022 17:41:08 +0000,
>> Kuldeep Singh <singh.kuldeep87k@gmail.com> wrote:
>>>
>>> On Wed, Mar 16, 2022 at 05:30:26PM +0100, Krzysztof Kozlowski wrote:
>>>> On 16/03/2022 10:54, Kuldeep Singh wrote:
>>>>> Few platforms such as Renesas RZ/N1D, Calxeda, Alpine etc. are using
>>>>> arm,cortex-a15-timer and arm,cortex-a7-timer entries in conjugation with
>>>>> arm,armv7-timer which are not currently defined in driver file. Add
>>>>> these entries in arch_timer_of_match list to bring them in use.
>>>>>
>>>>
>>>> This looks wrong (also Marc pointed this out) and rationale is not
>>>> sufficient. Why do you need these compatibles in the driver?
>>>
>>> Hi Krzysztof and Marc,
>>>
>>> I find myself in trouble whenever dealing with compatible entries and
>>> had 2 options when I stumble this issue.
>>> 	1. Remove unused compatible
>>
>> That'd be silly.
>>
>>> 	2. Add required compatible to binding and driver
>>
>> To the binding, yes. But to the driver?
>>
>>> My past experience and advise from other developer says not to remove an
>>> existing compatible. And also I found "arm,cortex-a15-timer" in binding
>>> which was again not documented and was present in DT. This prompted me
>>> to go for second option and make necessary additions in binding and
>>> driver following current entries.
>>
>> The "arm,cortex-a15-timer" compatible is documentation, and only
>> that. If, one day, we find a bug in this implementation, we could work
>> around it in the driver thanks to the separate compatible (although in
>> this case, we'd have much better way of doing that).
>>
>>> As per your perspective, current configuration isn't apt which means
>>> "arm,cortex-a15-timer" is a stub and is wrongly present in binding.
>>
>> That's not what I said. This compatible string is perfectly fine, and
>> accurately describe the HW. The driver doesn't need to know about the
>> fine details of the implementation, and is perfectly happy with the
>> current state of things.
>>
>> Think of it as an instance of a class. The driver doesn't need to know
>> the instance, only that it is a certain class.
>>
> 
> Thanks Marc for sharing knowledge. This was indeed helpful.
> To sum up from what I understood, bindings and DTs should always be in
> sync and driver file may not need to define all compatible entries as
> long as purpose is served.
> 
> This means no driver change will be required to address
> "arm,cortex-a7-timer". To which I have a question to Krzysztof.
> 
> Hi Krzysztof,
> 
> As per your comments on 2/3 patch, that it's DT which is not aligned
> with binding w.r.t arm,cortex-a7-timer.
> 
> What makes "arm,cortex-a7-timer" an invalid entry from binding
> perspective when we have a similar entry "arm,cortex-a15-timer" already
> present?
> 
> I think we should share some common grounds here and keep both of them
> in bindings or remove them altogether. I prefer first option, What's
> your say?

In this case the compatible should be added, just please explain it in
the message. Your previous commit msg was saying about disastrous
backward compatibility issue which so far does not exist here. It's
simply more detailed compatible.

There were few other cases where more detailed compatible was actually
unwanted, so that's why each case should be analyzed.

Best regards,
Krzysztof

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/3] Fix dtbs warnings for arch timer

[Rob Herring]

On Wed, Mar 16, 2022 at 06:47:47PM +0000, Marc Zyngier wrote:
> On Wed, 16 Mar 2022 17:20:51 +0000,
> Kuldeep Singh <singh.kuldeep87k@gmail.com> wrote:
> > 
> > On Wed, Mar 16, 2022 at 11:27:10AM +0000, Marc Zyngier wrote:
> > > On 2022-03-16 09:54, Kuldeep Singh wrote:
> > > > This patchset is an attempt to resolve 'make dtbs_check' warning for
> > > > arch timer.
> > > > 
> > > > Patch 1 is done in preparation for following patches which defines
> > > > compatibles order in more clear way.
> > > > Patch 2 documents arm,cortex-a7-timer entry in bindings similar to an
> > > > existing entry arm,cortex-a15-timer.
> > > > Patch 3 adds above 2 properties in of_match list to bring them in
> > > > use.
> > > > 
> > > > Please note, this patchset is based on
> > > > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git, master
> > > > 
> > > > Kuldeep Singh (3):
> > > >   dt-bindings: timer: Rearrange compatible entries of arch timer
> > > >   dt-bindings: timer: Document arm,cortex-a7-timer for arch timer
> > > >   clocksource: arch_timer: Add arm,cortex-a7/15-timer in of_match list
> > > > 
> > > >  .../devicetree/bindings/timer/arm,arch_timer.yaml   | 13 +++++--------
> > > >  drivers/clocksource/arm_arch_timer.c                |  2 ++
> > > >  2 files changed, 7 insertions(+), 8 deletions(-)
> > > 
> > > Please use my @kernel.org address exclusively. My @arm.com
> > > address stopped working over two years ago, and the MAINTAINERS
> > > file shows the right addresses.
> > 
> > Hi Marc,
> > 
> > I have tocmd and cccmd set in my gitconfig and it generated mail
> > addresses automatically so I assume it is correct.
> > tocmd ="`pwd`/scripts/get_maintainer.pl --nogit --nogit-fallback --norolestats --nol"
> > 
> > It seems your arm mail address got configured from binding file and not
> > from MAINTAINERS.
> 
> It is a bug in get_maintainer.pl. You'll have to manually apply the
> .mailmap transformation.

I just sent a fix for this[1]. 'in file' emails were not honoring 
mailmap.

Rob

[1] https://lore.kernel.org/all/20220323193645.317514-1-robh@kernel.org/

Re: [PATCH 0/3] Fix dtbs warnings for arch timer

[Rob Herring]

On Wed, Mar 16, 2022 at 06:47:47PM +0000, Marc Zyngier wrote:
> On Wed, 16 Mar 2022 17:20:51 +0000,
> Kuldeep Singh <singh.kuldeep87k@gmail.com> wrote:
> > 
> > On Wed, Mar 16, 2022 at 11:27:10AM +0000, Marc Zyngier wrote:
> > > On 2022-03-16 09:54, Kuldeep Singh wrote:
> > > > This patchset is an attempt to resolve 'make dtbs_check' warning for
> > > > arch timer.
> > > > 
> > > > Patch 1 is done in preparation for following patches which defines
> > > > compatibles order in more clear way.
> > > > Patch 2 documents arm,cortex-a7-timer entry in bindings similar to an
> > > > existing entry arm,cortex-a15-timer.
> > > > Patch 3 adds above 2 properties in of_match list to bring them in
> > > > use.
> > > > 
> > > > Please note, this patchset is based on
> > > > git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git, master
> > > > 
> > > > Kuldeep Singh (3):
> > > >   dt-bindings: timer: Rearrange compatible entries of arch timer
> > > >   dt-bindings: timer: Document arm,cortex-a7-timer for arch timer
> > > >   clocksource: arch_timer: Add arm,cortex-a7/15-timer in of_match list
> > > > 
> > > >  .../devicetree/bindings/timer/arm,arch_timer.yaml   | 13 +++++--------
> > > >  drivers/clocksource/arm_arch_timer.c                |  2 ++
> > > >  2 files changed, 7 insertions(+), 8 deletions(-)
> > > 
> > > Please use my @kernel.org address exclusively. My @arm.com
> > > address stopped working over two years ago, and the MAINTAINERS
> > > file shows the right addresses.
> > 
> > Hi Marc,
> > 
> > I have tocmd and cccmd set in my gitconfig and it generated mail
> > addresses automatically so I assume it is correct.
> > tocmd ="`pwd`/scripts/get_maintainer.pl --nogit --nogit-fallback --norolestats --nol"
> > 
> > It seems your arm mail address got configured from binding file and not
> > from MAINTAINERS.
> 
> It is a bug in get_maintainer.pl. You'll have to manually apply the
> .mailmap transformation.

I just sent a fix for this[1]. 'in file' emails were not honoring 
mailmap.

Rob

[1] https://lore.kernel.org/all/20220323193645.317514-1-robh@kernel.org/

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel