All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ARM: dts: vexpress: Replace '_' with '-' in node names
@ 2018-05-09 16:48 ` Sudeep Holla
  0 siblings, 0 replies; 20+ messages in thread
From: Sudeep Holla @ 2018-05-09 16:48 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree, Liviu Dudau; +Cc: Rob Herring, Sudeep Holla

The latest DTC throws warnings for character '_' in the node names.

Warning (node_name_chars_strict): /sysreg@10000/sys_led: Character '_' not recommended in node name
Warning (node_name_chars_strict): /sysreg@10000/sys_mci: Character '_' not recommended in node name
Warning (node_name_chars_strict): /sysreg@10000/sys_flash: Character '_' not recommended in node name

The general recommendation is to use character '-' for all the node names.
This patch fixes the warnings following the recommendation.

Cc: Liviu Dudau <liviu.dudau@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
index 7b8ff5b3b912..58e73131ecef 100644
--- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
+++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
@@ -77,19 +77,19 @@
 					compatible = "arm,vexpress-sysreg";
 					reg = <0x010000 0x1000>;
 
-					v2m_led_gpios: sys_led {
+					v2m_led_gpios: sys-led {
 						compatible = "arm,vexpress-sysreg,sys_led";
 						gpio-controller;
 						#gpio-cells = <2>;
 					};
 
-					v2m_mmc_gpios: sys_mci {
+					v2m_mmc_gpios: sys-mci {
 						compatible = "arm,vexpress-sysreg,sys_mci";
 						gpio-controller;
 						#gpio-cells = <2>;
 					};
 
-					v2m_flash_gpios: sys_flash {
+					v2m_flash_gpios: sys-flash {
 						compatible = "arm,vexpress-sysreg,sys_flash";
 						gpio-controller;
 						#gpio-cells = <2>;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH] ARM: dts: vexpress: Replace '_' with '-' in node names
@ 2018-05-09 16:48 ` Sudeep Holla
  0 siblings, 0 replies; 20+ messages in thread
From: Sudeep Holla @ 2018-05-09 16:48 UTC (permalink / raw)
  To: linux-arm-kernel

The latest DTC throws warnings for character '_' in the node names.

Warning (node_name_chars_strict): /sysreg at 10000/sys_led: Character '_' not recommended in node name
Warning (node_name_chars_strict): /sysreg at 10000/sys_mci: Character '_' not recommended in node name
Warning (node_name_chars_strict): /sysreg at 10000/sys_flash: Character '_' not recommended in node name

The general recommendation is to use character '-' for all the node names.
This patch fixes the warnings following the recommendation.

Cc: Liviu Dudau <liviu.dudau@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
index 7b8ff5b3b912..58e73131ecef 100644
--- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
+++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
@@ -77,19 +77,19 @@
 					compatible = "arm,vexpress-sysreg";
 					reg = <0x010000 0x1000>;
 
-					v2m_led_gpios: sys_led {
+					v2m_led_gpios: sys-led {
 						compatible = "arm,vexpress-sysreg,sys_led";
 						gpio-controller;
 						#gpio-cells = <2>;
 					};
 
-					v2m_mmc_gpios: sys_mci {
+					v2m_mmc_gpios: sys-mci {
 						compatible = "arm,vexpress-sysreg,sys_mci";
 						gpio-controller;
 						#gpio-cells = <2>;
 					};
 
-					v2m_flash_gpios: sys_flash {
+					v2m_flash_gpios: sys-flash {
 						compatible = "arm,vexpress-sysreg,sys_flash";
 						gpio-controller;
 						#gpio-cells = <2>;
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH] ARM: dts: vexpress: Replace '_' with '-' in node names
  2018-05-09 16:48 ` Sudeep Holla
@ 2018-05-09 21:14   ` Rob Herring
  -1 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2018-05-09 21:14 UTC (permalink / raw)
  To: Sudeep Holla, Linus Walleij
  Cc: devicetree, Liviu Dudau,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

On Wed, May 9, 2018 at 11:48 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> The latest DTC throws warnings for character '_' in the node names.
>
> Warning (node_name_chars_strict): /sysreg@10000/sys_led: Character '_' not recommended in node name
> Warning (node_name_chars_strict): /sysreg@10000/sys_mci: Character '_' not recommended in node name
> Warning (node_name_chars_strict): /sysreg@10000/sys_flash: Character '_' not recommended in node name
>
> The general recommendation is to use character '-' for all the node names.
> This patch fixes the warnings following the recommendation.
>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
> index 7b8ff5b3b912..58e73131ecef 100644
> --- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
> +++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
> @@ -77,19 +77,19 @@
>                                         compatible = "arm,vexpress-sysreg";
>                                         reg = <0x010000 0x1000>;
>
> -                                       v2m_led_gpios: sys_led {
> +                                       v2m_led_gpios: sys-led {

Except this is a gpio-controller so it should have 'gpio' for its node
name. (I have a dtc check written for that, but there are too many
false positives.)

But then you have 3 of them and no addressing, so you need to add reg
property (with the register's offset and size) and unit-address.

I'm surprised Linus W accepted these a GPIO when they are not really
general purpose, but then lots of things slip in.

Rob

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH] ARM: dts: vexpress: Replace '_' with '-' in node names
@ 2018-05-09 21:14   ` Rob Herring
  0 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2018-05-09 21:14 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, May 9, 2018 at 11:48 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> The latest DTC throws warnings for character '_' in the node names.
>
> Warning (node_name_chars_strict): /sysreg at 10000/sys_led: Character '_' not recommended in node name
> Warning (node_name_chars_strict): /sysreg at 10000/sys_mci: Character '_' not recommended in node name
> Warning (node_name_chars_strict): /sysreg at 10000/sys_flash: Character '_' not recommended in node name
>
> The general recommendation is to use character '-' for all the node names.
> This patch fixes the warnings following the recommendation.
>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
> index 7b8ff5b3b912..58e73131ecef 100644
> --- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
> +++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
> @@ -77,19 +77,19 @@
>                                         compatible = "arm,vexpress-sysreg";
>                                         reg = <0x010000 0x1000>;
>
> -                                       v2m_led_gpios: sys_led {
> +                                       v2m_led_gpios: sys-led {

Except this is a gpio-controller so it should have 'gpio' for its node
name. (I have a dtc check written for that, but there are too many
false positives.)

But then you have 3 of them and no addressing, so you need to add reg
property (with the register's offset and size) and unit-address.

I'm surprised Linus W accepted these a GPIO when they are not really
general purpose, but then lots of things slip in.

Rob

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] ARM: dts: vexpress: Replace '_' with '-' in node names
  2018-05-09 21:14   ` Rob Herring
@ 2018-05-10 10:51     ` Sudeep Holla
  -1 siblings, 0 replies; 20+ messages in thread
From: Sudeep Holla @ 2018-05-10 10:51 UTC (permalink / raw)
  To: Rob Herring, Linus Walleij
  Cc: devicetree, Liviu Dudau, linux-arm-kernel, Sudeep Holla



On 09/05/18 22:14, Rob Herring wrote:
> On Wed, May 9, 2018 at 11:48 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> The latest DTC throws warnings for character '_' in the node names.
>>
>> Warning (node_name_chars_strict): /sysreg@10000/sys_led: Character '_' not recommended in node name
>> Warning (node_name_chars_strict): /sysreg@10000/sys_mci: Character '_' not recommended in node name
>> Warning (node_name_chars_strict): /sysreg@10000/sys_flash: Character '_' not recommended in node name
>>
>> The general recommendation is to use character '-' for all the node names.
>> This patch fixes the warnings following the recommendation.
>>
>> Cc: Liviu Dudau <liviu.dudau@arm.com>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>>  arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>> index 7b8ff5b3b912..58e73131ecef 100644
>> --- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>> +++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>> @@ -77,19 +77,19 @@
>>                                         compatible = "arm,vexpress-sysreg";
>>                                         reg = <0x010000 0x1000>;
>>
>> -                                       v2m_led_gpios: sys_led {
>> +                                       v2m_led_gpios: sys-led {
> 
> Except this is a gpio-controller so it should have 'gpio' for its node
> name. (I have a dtc check written for that, but there are too many
> false positives.)
> 

True, sorry I didn't look at it in detail.

> But then you have 3 of them and no addressing, so you need to add reg
> property (with the register's offset and size) and unit-address.
> 

Indeed. I had a look at the history but couldn't gather much. All I
could get is that this is one of those weird mix of all functionality on
ARM Ltd platforms which fits no subsystem. Me and Lorenzo has similar
issue on TC2 platform. Pawel seem to have plumed this system control
registers block into MFD and GPIO long back.

> I'm surprised Linus W accepted these a GPIO when they are not really
> general purpose, but then lots of things slip in.
> 

I assume all these happened in early days of DT.

I will drop this for now. I will take a look if these nodes can be made
better to align with standard gpio controller nodes.

-- 
Regards,
Sudeep

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH] ARM: dts: vexpress: Replace '_' with '-' in node names
@ 2018-05-10 10:51     ` Sudeep Holla
  0 siblings, 0 replies; 20+ messages in thread
From: Sudeep Holla @ 2018-05-10 10:51 UTC (permalink / raw)
  To: linux-arm-kernel



On 09/05/18 22:14, Rob Herring wrote:
> On Wed, May 9, 2018 at 11:48 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> The latest DTC throws warnings for character '_' in the node names.
>>
>> Warning (node_name_chars_strict): /sysreg at 10000/sys_led: Character '_' not recommended in node name
>> Warning (node_name_chars_strict): /sysreg at 10000/sys_mci: Character '_' not recommended in node name
>> Warning (node_name_chars_strict): /sysreg at 10000/sys_flash: Character '_' not recommended in node name
>>
>> The general recommendation is to use character '-' for all the node names.
>> This patch fixes the warnings following the recommendation.
>>
>> Cc: Liviu Dudau <liviu.dudau@arm.com>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>>  arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>> index 7b8ff5b3b912..58e73131ecef 100644
>> --- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>> +++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>> @@ -77,19 +77,19 @@
>>                                         compatible = "arm,vexpress-sysreg";
>>                                         reg = <0x010000 0x1000>;
>>
>> -                                       v2m_led_gpios: sys_led {
>> +                                       v2m_led_gpios: sys-led {
> 
> Except this is a gpio-controller so it should have 'gpio' for its node
> name. (I have a dtc check written for that, but there are too many
> false positives.)
> 

True, sorry I didn't look at it in detail.

> But then you have 3 of them and no addressing, so you need to add reg
> property (with the register's offset and size) and unit-address.
> 

Indeed. I had a look at the history but couldn't gather much. All I
could get is that this is one of those weird mix of all functionality on
ARM Ltd platforms which fits no subsystem. Me and Lorenzo has similar
issue on TC2 platform. Pawel seem to have plumed this system control
registers block into MFD and GPIO long back.

> I'm surprised Linus W accepted these a GPIO when they are not really
> general purpose, but then lots of things slip in.
> 

I assume all these happened in early days of DT.

I will drop this for now. I will take a look if these nodes can be made
better to align with standard gpio controller nodes.

-- 
Regards,
Sudeep

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] ARM: dts: vexpress: Replace '_' with '-' in node names
  2018-05-10 10:51     ` Sudeep Holla
@ 2018-05-10 14:05       ` Rob Herring
  -1 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2018-05-10 14:05 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: devicetree, Linus Walleij, Liviu Dudau,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

On Thu, May 10, 2018 at 5:51 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>
> On 09/05/18 22:14, Rob Herring wrote:
>> On Wed, May 9, 2018 at 11:48 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>> The latest DTC throws warnings for character '_' in the node names.
>>>
>>> Warning (node_name_chars_strict): /sysreg@10000/sys_led: Character '_' not recommended in node name
>>> Warning (node_name_chars_strict): /sysreg@10000/sys_mci: Character '_' not recommended in node name
>>> Warning (node_name_chars_strict): /sysreg@10000/sys_flash: Character '_' not recommended in node name
>>>
>>> The general recommendation is to use character '-' for all the node names.
>>> This patch fixes the warnings following the recommendation.
>>>
>>> Cc: Liviu Dudau <liviu.dudau@arm.com>
>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>> ---
>>>  arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>>> index 7b8ff5b3b912..58e73131ecef 100644
>>> --- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>>> +++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>>> @@ -77,19 +77,19 @@
>>>                                         compatible = "arm,vexpress-sysreg";
>>>                                         reg = <0x010000 0x1000>;
>>>
>>> -                                       v2m_led_gpios: sys_led {
>>> +                                       v2m_led_gpios: sys-led {
>>
>> Except this is a gpio-controller so it should have 'gpio' for its node
>> name. (I have a dtc check written for that, but there are too many
>> false positives.)
>>
>
> True, sorry I didn't look at it in detail.
>
>> But then you have 3 of them and no addressing, so you need to add reg
>> property (with the register's offset and size) and unit-address.
>>
>
> Indeed. I had a look at the history but couldn't gather much. All I
> could get is that this is one of those weird mix of all functionality on
> ARM Ltd platforms which fits no subsystem. Me and Lorenzo has similar
> issue on TC2 platform. Pawel seem to have plumed this system control
> registers block into MFD and GPIO long back.
>
>> I'm surprised Linus W accepted these a GPIO when they are not really
>> general purpose, but then lots of things slip in.
>>
>
> I assume all these happened in early days of DT.
>
> I will drop this for now. I will take a look if these nodes can be made
> better to align with standard gpio controller nodes.

Why not just make the changes I suggested? It shouldn't break
anything. You can add reg property even though the kernel doesn't use
it.

Rob

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH] ARM: dts: vexpress: Replace '_' with '-' in node names
@ 2018-05-10 14:05       ` Rob Herring
  0 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2018-05-10 14:05 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, May 10, 2018 at 5:51 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>
> On 09/05/18 22:14, Rob Herring wrote:
>> On Wed, May 9, 2018 at 11:48 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>> The latest DTC throws warnings for character '_' in the node names.
>>>
>>> Warning (node_name_chars_strict): /sysreg at 10000/sys_led: Character '_' not recommended in node name
>>> Warning (node_name_chars_strict): /sysreg at 10000/sys_mci: Character '_' not recommended in node name
>>> Warning (node_name_chars_strict): /sysreg at 10000/sys_flash: Character '_' not recommended in node name
>>>
>>> The general recommendation is to use character '-' for all the node names.
>>> This patch fixes the warnings following the recommendation.
>>>
>>> Cc: Liviu Dudau <liviu.dudau@arm.com>
>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>> ---
>>>  arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 6 +++---
>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>>> index 7b8ff5b3b912..58e73131ecef 100644
>>> --- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>>> +++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>>> @@ -77,19 +77,19 @@
>>>                                         compatible = "arm,vexpress-sysreg";
>>>                                         reg = <0x010000 0x1000>;
>>>
>>> -                                       v2m_led_gpios: sys_led {
>>> +                                       v2m_led_gpios: sys-led {
>>
>> Except this is a gpio-controller so it should have 'gpio' for its node
>> name. (I have a dtc check written for that, but there are too many
>> false positives.)
>>
>
> True, sorry I didn't look at it in detail.
>
>> But then you have 3 of them and no addressing, so you need to add reg
>> property (with the register's offset and size) and unit-address.
>>
>
> Indeed. I had a look at the history but couldn't gather much. All I
> could get is that this is one of those weird mix of all functionality on
> ARM Ltd platforms which fits no subsystem. Me and Lorenzo has similar
> issue on TC2 platform. Pawel seem to have plumed this system control
> registers block into MFD and GPIO long back.
>
>> I'm surprised Linus W accepted these a GPIO when they are not really
>> general purpose, but then lots of things slip in.
>>
>
> I assume all these happened in early days of DT.
>
> I will drop this for now. I will take a look if these nodes can be made
> better to align with standard gpio controller nodes.

Why not just make the changes I suggested? It shouldn't break
anything. You can add reg property even though the kernel doesn't use
it.

Rob

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] ARM: dts: vexpress: Replace '_' with '-' in node names
  2018-05-10 14:05       ` Rob Herring
@ 2018-05-10 14:16         ` Sudeep Holla
  -1 siblings, 0 replies; 20+ messages in thread
From: Sudeep Holla @ 2018-05-10 14:16 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, Linus Walleij, Liviu Dudau,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Sudeep Holla



On 10/05/18 15:05, Rob Herring wrote:
> On Thu, May 10, 2018 at 5:51 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>
>>
>> On 09/05/18 22:14, Rob Herring wrote:
>>> On Wed, May 9, 2018 at 11:48 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>>> The latest DTC throws warnings for character '_' in the node names.
>>>>
>>>> Warning (node_name_chars_strict): /sysreg@10000/sys_led: Character '_' not recommended in node name
>>>> Warning (node_name_chars_strict): /sysreg@10000/sys_mci: Character '_' not recommended in node name
>>>> Warning (node_name_chars_strict): /sysreg@10000/sys_flash: Character '_' not recommended in node name
>>>>
>>>> The general recommendation is to use character '-' for all the node names.
>>>> This patch fixes the warnings following the recommendation.
>>>>
>>>> Cc: Liviu Dudau <liviu.dudau@arm.com>
>>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>>> ---
>>>>  arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 6 +++---
>>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>>>> index 7b8ff5b3b912..58e73131ecef 100644
>>>> --- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>>>> +++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>>>> @@ -77,19 +77,19 @@
>>>>                                         compatible = "arm,vexpress-sysreg";
>>>>                                         reg = <0x010000 0x1000>;
>>>>
>>>> -                                       v2m_led_gpios: sys_led {
>>>> +                                       v2m_led_gpios: sys-led {
>>>
>>> Except this is a gpio-controller so it should have 'gpio' for its node
>>> name. (I have a dtc check written for that, but there are too many
>>> false positives.)
>>>
>>
>> True, sorry I didn't look at it in detail.
>>
>>> But then you have 3 of them and no addressing, so you need to add reg
>>> property (with the register's offset and size) and unit-address.
>>>
>>
>> Indeed. I had a look at the history but couldn't gather much. All I
>> could get is that this is one of those weird mix of all functionality on
>> ARM Ltd platforms which fits no subsystem. Me and Lorenzo has similar
>> issue on TC2 platform. Pawel seem to have plumed this system control
>> registers block into MFD and GPIO long back.
>>
>>> I'm surprised Linus W accepted these a GPIO when they are not really
>>> general purpose, but then lots of things slip in.
>>>
>>
>> I assume all these happened in early days of DT.
>>
>> I will drop this for now. I will take a look if these nodes can be made
>> better to align with standard gpio controller nodes.
> 
> Why not just make the changes I suggested? It shouldn't break
> anything. You can add reg property even though the kernel doesn't use
> it.
> 

Ah OK. That makes sense. IIRC it used to have some @<offset> value
previously which has some correlation to HW block. I will dig that out.
Sorry I missed that you were suggesting to add reg property.

-- 
Regards,
Sudeep

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH] ARM: dts: vexpress: Replace '_' with '-' in node names
@ 2018-05-10 14:16         ` Sudeep Holla
  0 siblings, 0 replies; 20+ messages in thread
From: Sudeep Holla @ 2018-05-10 14:16 UTC (permalink / raw)
  To: linux-arm-kernel



On 10/05/18 15:05, Rob Herring wrote:
> On Thu, May 10, 2018 at 5:51 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>
>>
>> On 09/05/18 22:14, Rob Herring wrote:
>>> On Wed, May 9, 2018 at 11:48 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>>>> The latest DTC throws warnings for character '_' in the node names.
>>>>
>>>> Warning (node_name_chars_strict): /sysreg at 10000/sys_led: Character '_' not recommended in node name
>>>> Warning (node_name_chars_strict): /sysreg at 10000/sys_mci: Character '_' not recommended in node name
>>>> Warning (node_name_chars_strict): /sysreg at 10000/sys_flash: Character '_' not recommended in node name
>>>>
>>>> The general recommendation is to use character '-' for all the node names.
>>>> This patch fixes the warnings following the recommendation.
>>>>
>>>> Cc: Liviu Dudau <liviu.dudau@arm.com>
>>>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>>>> ---
>>>>  arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 6 +++---
>>>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>>>> index 7b8ff5b3b912..58e73131ecef 100644
>>>> --- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>>>> +++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>>>> @@ -77,19 +77,19 @@
>>>>                                         compatible = "arm,vexpress-sysreg";
>>>>                                         reg = <0x010000 0x1000>;
>>>>
>>>> -                                       v2m_led_gpios: sys_led {
>>>> +                                       v2m_led_gpios: sys-led {
>>>
>>> Except this is a gpio-controller so it should have 'gpio' for its node
>>> name. (I have a dtc check written for that, but there are too many
>>> false positives.)
>>>
>>
>> True, sorry I didn't look at it in detail.
>>
>>> But then you have 3 of them and no addressing, so you need to add reg
>>> property (with the register's offset and size) and unit-address.
>>>
>>
>> Indeed. I had a look at the history but couldn't gather much. All I
>> could get is that this is one of those weird mix of all functionality on
>> ARM Ltd platforms which fits no subsystem. Me and Lorenzo has similar
>> issue on TC2 platform. Pawel seem to have plumed this system control
>> registers block into MFD and GPIO long back.
>>
>>> I'm surprised Linus W accepted these a GPIO when they are not really
>>> general purpose, but then lots of things slip in.
>>>
>>
>> I assume all these happened in early days of DT.
>>
>> I will drop this for now. I will take a look if these nodes can be made
>> better to align with standard gpio controller nodes.
> 
> Why not just make the changes I suggested? It shouldn't break
> anything. You can add reg property even though the kernel doesn't use
> it.
> 

Ah OK. That makes sense. IIRC it used to have some @<offset> value
previously which has some correlation to HW block. I will dig that out.
Sorry I missed that you were suggesting to add reg property.

-- 
Regards,
Sudeep

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 1/2] ARM: dts: vexpress: use standard gpio bindings for sys_{led, mci, flash}
  2018-05-09 16:48 ` Sudeep Holla
@ 2018-05-11 10:23   ` Sudeep Holla
  -1 siblings, 0 replies; 20+ messages in thread
From: Sudeep Holla @ 2018-05-11 10:23 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree, Liviu Dudau; +Cc: Rob Herring, Sudeep Holla

Commit 2cff6dba57b7 ("ARM: dts: vexpress: fix node name unit-address presence warnings")
removed the unit address as there was no associated reg property in
these sysreg nodes.

Also the latest DTC throws warnings for character '_' in the node names.

Warning (node_name_chars_strict): /sysreg@10000/sys_led: Character '_' not recommended in node name
Warning (node_name_chars_strict): /sysreg@10000/sys_mci: Character '_' not recommended in node name
Warning (node_name_chars_strict): /sysreg@10000/sys_flash: Character '_' not recommended in node name

The correct way to fix this as well as the original unit-address presence
warnings is to use the standard gpio controller binding and specify the
reg properties as per the hardware as it was before.

However note that Vexpress sysreg MFD driver will still continue to use
the hardcoded values for compatibility reasons.

Cc: Liviu Dudau <liviu.dudau@arm.com>
Suggested-by: Rob Herring <robh@kernel.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 11 ++++++++---
 arch/arm/boot/dts/vexpress-v2m.dtsi     | 11 ++++++++---
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
index 7b8ff5b3b912..a8586a0b957d 100644
--- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
+++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
@@ -76,21 +76,26 @@
 				v2m_sysreg: sysreg@10000 {
 					compatible = "arm,vexpress-sysreg";
 					reg = <0x010000 0x1000>;
+					#address-cells = <1>;
+					#size-cells = <1>;

-					v2m_led_gpios: sys_led {
+					v2m_led_gpios: gpio@8 {
 						compatible = "arm,vexpress-sysreg,sys_led";
+						reg = <0x008 4>;
 						gpio-controller;
 						#gpio-cells = <2>;
 					};

-					v2m_mmc_gpios: sys_mci {
+					v2m_mmc_gpios: gpio@48 {
 						compatible = "arm,vexpress-sysreg,sys_mci";
+						reg = <0x048 4>;
 						gpio-controller;
 						#gpio-cells = <2>;
 					};

-					v2m_flash_gpios: sys_flash {
+					v2m_flash_gpios: gpio@4c {
 						compatible = "arm,vexpress-sysreg,sys_flash";
+						reg = <0x04c 4>;
 						gpio-controller;
 						#gpio-cells = <2>;
 					};
diff --git a/arch/arm/boot/dts/vexpress-v2m.dtsi b/arch/arm/boot/dts/vexpress-v2m.dtsi
index 9cd5e146abd5..37ecccebd937 100644
--- a/arch/arm/boot/dts/vexpress-v2m.dtsi
+++ b/arch/arm/boot/dts/vexpress-v2m.dtsi
@@ -76,21 +76,26 @@
 				v2m_sysreg: sysreg@0 {
 					compatible = "arm,vexpress-sysreg";
 					reg = <0x00000 0x1000>;
+					#address-cells = <1>;
+					#size-cells = <1>;

-					v2m_led_gpios: sys_led {
+					v2m_led_gpios: gpio@8 {
 						compatible = "arm,vexpress-sysreg,sys_led";
+						reg = <0x008 4>;
 						gpio-controller;
 						#gpio-cells = <2>;
 					};

-					v2m_mmc_gpios: sys_mci {
+					v2m_mmc_gpios: gpio@48 {
 						compatible = "arm,vexpress-sysreg,sys_mci";
+						reg = <0x048 4>;
 						gpio-controller;
 						#gpio-cells = <2>;
 					};

-					v2m_flash_gpios: sys_flash {
+					v2m_flash_gpios: gpio@4c {
 						compatible = "arm,vexpress-sysreg,sys_flash";
+						reg = <0x04c 4>;
 						gpio-controller;
 						#gpio-cells = <2>;
 					};
--
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 1/2] ARM: dts: vexpress: use standard gpio bindings for sys_{led, mci, flash}
@ 2018-05-11 10:23   ` Sudeep Holla
  0 siblings, 0 replies; 20+ messages in thread
From: Sudeep Holla @ 2018-05-11 10:23 UTC (permalink / raw)
  To: linux-arm-kernel

Commit 2cff6dba57b7 ("ARM: dts: vexpress: fix node name unit-address presence warnings")
removed the unit address as there was no associated reg property in
these sysreg nodes.

Also the latest DTC throws warnings for character '_' in the node names.

Warning (node_name_chars_strict): /sysreg at 10000/sys_led: Character '_' not recommended in node name
Warning (node_name_chars_strict): /sysreg at 10000/sys_mci: Character '_' not recommended in node name
Warning (node_name_chars_strict): /sysreg at 10000/sys_flash: Character '_' not recommended in node name

The correct way to fix this as well as the original unit-address presence
warnings is to use the standard gpio controller binding and specify the
reg properties as per the hardware as it was before.

However note that Vexpress sysreg MFD driver will still continue to use
the hardcoded values for compatibility reasons.

Cc: Liviu Dudau <liviu.dudau@arm.com>
Suggested-by: Rob Herring <robh@kernel.org>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 11 ++++++++---
 arch/arm/boot/dts/vexpress-v2m.dtsi     | 11 ++++++++---
 2 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
index 7b8ff5b3b912..a8586a0b957d 100644
--- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
+++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
@@ -76,21 +76,26 @@
 				v2m_sysreg: sysreg at 10000 {
 					compatible = "arm,vexpress-sysreg";
 					reg = <0x010000 0x1000>;
+					#address-cells = <1>;
+					#size-cells = <1>;

-					v2m_led_gpios: sys_led {
+					v2m_led_gpios: gpio at 8 {
 						compatible = "arm,vexpress-sysreg,sys_led";
+						reg = <0x008 4>;
 						gpio-controller;
 						#gpio-cells = <2>;
 					};

-					v2m_mmc_gpios: sys_mci {
+					v2m_mmc_gpios: gpio at 48 {
 						compatible = "arm,vexpress-sysreg,sys_mci";
+						reg = <0x048 4>;
 						gpio-controller;
 						#gpio-cells = <2>;
 					};

-					v2m_flash_gpios: sys_flash {
+					v2m_flash_gpios: gpio at 4c {
 						compatible = "arm,vexpress-sysreg,sys_flash";
+						reg = <0x04c 4>;
 						gpio-controller;
 						#gpio-cells = <2>;
 					};
diff --git a/arch/arm/boot/dts/vexpress-v2m.dtsi b/arch/arm/boot/dts/vexpress-v2m.dtsi
index 9cd5e146abd5..37ecccebd937 100644
--- a/arch/arm/boot/dts/vexpress-v2m.dtsi
+++ b/arch/arm/boot/dts/vexpress-v2m.dtsi
@@ -76,21 +76,26 @@
 				v2m_sysreg: sysreg at 0 {
 					compatible = "arm,vexpress-sysreg";
 					reg = <0x00000 0x1000>;
+					#address-cells = <1>;
+					#size-cells = <1>;

-					v2m_led_gpios: sys_led {
+					v2m_led_gpios: gpio at 8 {
 						compatible = "arm,vexpress-sysreg,sys_led";
+						reg = <0x008 4>;
 						gpio-controller;
 						#gpio-cells = <2>;
 					};

-					v2m_mmc_gpios: sys_mci {
+					v2m_mmc_gpios: gpio at 48 {
 						compatible = "arm,vexpress-sysreg,sys_mci";
+						reg = <0x048 4>;
 						gpio-controller;
 						#gpio-cells = <2>;
 					};

-					v2m_flash_gpios: sys_flash {
+					v2m_flash_gpios: gpio at 4c {
 						compatible = "arm,vexpress-sysreg,sys_flash";
+						reg = <0x04c 4>;
 						gpio-controller;
 						#gpio-cells = <2>;
 					};
--
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 2/2] ARM: dts: vexpress: replace '_' with '-' in node names
  2018-05-11 10:23   ` Sudeep Holla
@ 2018-05-11 10:23     ` Sudeep Holla
  -1 siblings, 0 replies; 20+ messages in thread
From: Sudeep Holla @ 2018-05-11 10:23 UTC (permalink / raw)
  To: linux-arm-kernel, devicetree, Liviu Dudau; +Cc: Rob Herring, Sudeep Holla

The latest DTC throws warnings for character '_' in the node names.

Warning (node_name_chars_strict): /pmu_a15: Character '_' not recommended in node name
Warning (node_name_chars_strict): /pmu_a7: Character '_' not recommended in node name

The general recommendation is to use character '-' for all the node names.
This patch fixes the warnings following the recommendation.

Cc: Liviu Dudau <liviu.dudau@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

v1->v2:
	- This patch is newly added in v2 and was missing earlier

diff --git a/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts b/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts
index 65a874ea66be..ac6b90e9d806 100644
--- a/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts
+++ b/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts
@@ -204,7 +204,7 @@
 			     <1 10 0xf08>;
 	};

-	pmu_a15 {
+	pmu-a15 {
 		compatible = "arm,cortex-a15-pmu";
 		interrupts = <0 68 4>,
 			     <0 69 4>;
@@ -212,7 +212,7 @@
 				     <&cpu1>;
 	};

-	pmu_a7 {
+	pmu-a7 {
 		compatible = "arm,cortex-a7-pmu";
 		interrupts = <0 128 4>,
 			     <0 129 4>,
--
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 2/2] ARM: dts: vexpress: replace '_' with '-' in node names
@ 2018-05-11 10:23     ` Sudeep Holla
  0 siblings, 0 replies; 20+ messages in thread
From: Sudeep Holla @ 2018-05-11 10:23 UTC (permalink / raw)
  To: linux-arm-kernel

The latest DTC throws warnings for character '_' in the node names.

Warning (node_name_chars_strict): /pmu_a15: Character '_' not recommended in node name
Warning (node_name_chars_strict): /pmu_a7: Character '_' not recommended in node name

The general recommendation is to use character '-' for all the node names.
This patch fixes the warnings following the recommendation.

Cc: Liviu Dudau <liviu.dudau@arm.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

v1->v2:
	- This patch is newly added in v2 and was missing earlier

diff --git a/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts b/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts
index 65a874ea66be..ac6b90e9d806 100644
--- a/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts
+++ b/arch/arm/boot/dts/vexpress-v2p-ca15_a7.dts
@@ -204,7 +204,7 @@
 			     <1 10 0xf08>;
 	};

-	pmu_a15 {
+	pmu-a15 {
 		compatible = "arm,cortex-a15-pmu";
 		interrupts = <0 68 4>,
 			     <0 69 4>;
@@ -212,7 +212,7 @@
 				     <&cpu1>;
 	};

-	pmu_a7 {
+	pmu-a7 {
 		compatible = "arm,cortex-a7-pmu";
 		interrupts = <0 128 4>,
 			     <0 129 4>,
--
2.7.4

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 1/2] ARM: dts: vexpress: use standard gpio bindings for sys_{led, mci, flash}
  2018-05-11 10:23   ` Sudeep Holla
@ 2018-05-11 12:10     ` Rob Herring
  -1 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2018-05-11 12:10 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: devicetree, Liviu Dudau,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE

On Fri, May 11, 2018 at 5:23 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> Commit 2cff6dba57b7 ("ARM: dts: vexpress: fix node name unit-address presence warnings")
> removed the unit address as there was no associated reg property in
> these sysreg nodes.
>
> Also the latest DTC throws warnings for character '_' in the node names.
>
> Warning (node_name_chars_strict): /sysreg@10000/sys_led: Character '_' not recommended in node name
> Warning (node_name_chars_strict): /sysreg@10000/sys_mci: Character '_' not recommended in node name
> Warning (node_name_chars_strict): /sysreg@10000/sys_flash: Character '_' not recommended in node name
>
> The correct way to fix this as well as the original unit-address presence
> warnings is to use the standard gpio controller binding and specify the
> reg properties as per the hardware as it was before.
>
> However note that Vexpress sysreg MFD driver will still continue to use
> the hardcoded values for compatibility reasons.
>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Suggested-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 11 ++++++++---
>  arch/arm/boot/dts/vexpress-v2m.dtsi     | 11 ++++++++---
>  2 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
> index 7b8ff5b3b912..a8586a0b957d 100644
> --- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
> +++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
> @@ -76,21 +76,26 @@
>                                 v2m_sysreg: sysreg@10000 {
>                                         compatible = "arm,vexpress-sysreg";
>                                         reg = <0x010000 0x1000>;
> +                                       #address-cells = <1>;
> +                                       #size-cells = <1>;

You need a ranges here and below to translate from 0-0x10000 range.

With that,

Reviewed-by: Rob Herring <robh@kernel.org>

>
> -                                       v2m_led_gpios: sys_led {
> +                                       v2m_led_gpios: gpio@8 {
>                                                 compatible = "arm,vexpress-sysreg,sys_led";
> +                                               reg = <0x008 4>;
>                                                 gpio-controller;
>                                                 #gpio-cells = <2>;
>                                         };
>
> -                                       v2m_mmc_gpios: sys_mci {
> +                                       v2m_mmc_gpios: gpio@48 {
>                                                 compatible = "arm,vexpress-sysreg,sys_mci";
> +                                               reg = <0x048 4>;
>                                                 gpio-controller;
>                                                 #gpio-cells = <2>;
>                                         };
>
> -                                       v2m_flash_gpios: sys_flash {
> +                                       v2m_flash_gpios: gpio@4c {
>                                                 compatible = "arm,vexpress-sysreg,sys_flash";
> +                                               reg = <0x04c 4>;
>                                                 gpio-controller;
>                                                 #gpio-cells = <2>;
>                                         };
> diff --git a/arch/arm/boot/dts/vexpress-v2m.dtsi b/arch/arm/boot/dts/vexpress-v2m.dtsi
> index 9cd5e146abd5..37ecccebd937 100644
> --- a/arch/arm/boot/dts/vexpress-v2m.dtsi
> +++ b/arch/arm/boot/dts/vexpress-v2m.dtsi
> @@ -76,21 +76,26 @@
>                                 v2m_sysreg: sysreg@0 {
>                                         compatible = "arm,vexpress-sysreg";
>                                         reg = <0x00000 0x1000>;
> +                                       #address-cells = <1>;
> +                                       #size-cells = <1>;
>
> -                                       v2m_led_gpios: sys_led {
> +                                       v2m_led_gpios: gpio@8 {
>                                                 compatible = "arm,vexpress-sysreg,sys_led";
> +                                               reg = <0x008 4>;
>                                                 gpio-controller;
>                                                 #gpio-cells = <2>;
>                                         };
>
> -                                       v2m_mmc_gpios: sys_mci {
> +                                       v2m_mmc_gpios: gpio@48 {
>                                                 compatible = "arm,vexpress-sysreg,sys_mci";
> +                                               reg = <0x048 4>;
>                                                 gpio-controller;
>                                                 #gpio-cells = <2>;
>                                         };
>
> -                                       v2m_flash_gpios: sys_flash {
> +                                       v2m_flash_gpios: gpio@4c {
>                                                 compatible = "arm,vexpress-sysreg,sys_flash";
> +                                               reg = <0x04c 4>;
>                                                 gpio-controller;
>                                                 #gpio-cells = <2>;
>                                         };
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 1/2] ARM: dts: vexpress: use standard gpio bindings for sys_{led, mci, flash}
@ 2018-05-11 12:10     ` Rob Herring
  0 siblings, 0 replies; 20+ messages in thread
From: Rob Herring @ 2018-05-11 12:10 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, May 11, 2018 at 5:23 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
> Commit 2cff6dba57b7 ("ARM: dts: vexpress: fix node name unit-address presence warnings")
> removed the unit address as there was no associated reg property in
> these sysreg nodes.
>
> Also the latest DTC throws warnings for character '_' in the node names.
>
> Warning (node_name_chars_strict): /sysreg at 10000/sys_led: Character '_' not recommended in node name
> Warning (node_name_chars_strict): /sysreg at 10000/sys_mci: Character '_' not recommended in node name
> Warning (node_name_chars_strict): /sysreg at 10000/sys_flash: Character '_' not recommended in node name
>
> The correct way to fix this as well as the original unit-address presence
> warnings is to use the standard gpio controller binding and specify the
> reg properties as per the hardware as it was before.
>
> However note that Vexpress sysreg MFD driver will still continue to use
> the hardcoded values for compatibility reasons.
>
> Cc: Liviu Dudau <liviu.dudau@arm.com>
> Suggested-by: Rob Herring <robh@kernel.org>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> ---
>  arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 11 ++++++++---
>  arch/arm/boot/dts/vexpress-v2m.dtsi     | 11 ++++++++---
>  2 files changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
> index 7b8ff5b3b912..a8586a0b957d 100644
> --- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
> +++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
> @@ -76,21 +76,26 @@
>                                 v2m_sysreg: sysreg at 10000 {
>                                         compatible = "arm,vexpress-sysreg";
>                                         reg = <0x010000 0x1000>;
> +                                       #address-cells = <1>;
> +                                       #size-cells = <1>;

You need a ranges here and below to translate from 0-0x10000 range.

With that,

Reviewed-by: Rob Herring <robh@kernel.org>

>
> -                                       v2m_led_gpios: sys_led {
> +                                       v2m_led_gpios: gpio at 8 {
>                                                 compatible = "arm,vexpress-sysreg,sys_led";
> +                                               reg = <0x008 4>;
>                                                 gpio-controller;
>                                                 #gpio-cells = <2>;
>                                         };
>
> -                                       v2m_mmc_gpios: sys_mci {
> +                                       v2m_mmc_gpios: gpio at 48 {
>                                                 compatible = "arm,vexpress-sysreg,sys_mci";
> +                                               reg = <0x048 4>;
>                                                 gpio-controller;
>                                                 #gpio-cells = <2>;
>                                         };
>
> -                                       v2m_flash_gpios: sys_flash {
> +                                       v2m_flash_gpios: gpio at 4c {
>                                                 compatible = "arm,vexpress-sysreg,sys_flash";
> +                                               reg = <0x04c 4>;
>                                                 gpio-controller;
>                                                 #gpio-cells = <2>;
>                                         };
> diff --git a/arch/arm/boot/dts/vexpress-v2m.dtsi b/arch/arm/boot/dts/vexpress-v2m.dtsi
> index 9cd5e146abd5..37ecccebd937 100644
> --- a/arch/arm/boot/dts/vexpress-v2m.dtsi
> +++ b/arch/arm/boot/dts/vexpress-v2m.dtsi
> @@ -76,21 +76,26 @@
>                                 v2m_sysreg: sysreg at 0 {
>                                         compatible = "arm,vexpress-sysreg";
>                                         reg = <0x00000 0x1000>;
> +                                       #address-cells = <1>;
> +                                       #size-cells = <1>;
>
> -                                       v2m_led_gpios: sys_led {
> +                                       v2m_led_gpios: gpio at 8 {
>                                                 compatible = "arm,vexpress-sysreg,sys_led";
> +                                               reg = <0x008 4>;
>                                                 gpio-controller;
>                                                 #gpio-cells = <2>;
>                                         };
>
> -                                       v2m_mmc_gpios: sys_mci {
> +                                       v2m_mmc_gpios: gpio at 48 {
>                                                 compatible = "arm,vexpress-sysreg,sys_mci";
> +                                               reg = <0x048 4>;
>                                                 gpio-controller;
>                                                 #gpio-cells = <2>;
>                                         };
>
> -                                       v2m_flash_gpios: sys_flash {
> +                                       v2m_flash_gpios: gpio at 4c {
>                                                 compatible = "arm,vexpress-sysreg,sys_flash";
> +                                               reg = <0x04c 4>;
>                                                 gpio-controller;
>                                                 #gpio-cells = <2>;
>                                         };
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 1/2] ARM: dts: vexpress: use standard gpio bindings for sys_{led,mci,flash}
  2018-05-11 12:10     ` Rob Herring
@ 2018-05-11 13:16       ` Sudeep Holla
  -1 siblings, 0 replies; 20+ messages in thread
From: Sudeep Holla @ 2018-05-11 13:16 UTC (permalink / raw)
  To: Rob Herring
  Cc: devicetree, Liviu Dudau,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Sudeep Holla



On 11/05/18 13:10, Rob Herring wrote:
> On Fri, May 11, 2018 at 5:23 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> Commit 2cff6dba57b7 ("ARM: dts: vexpress: fix node name unit-address presence warnings")
>> removed the unit address as there was no associated reg property in
>> these sysreg nodes.
>>
>> Also the latest DTC throws warnings for character '_' in the node names.
>>
>> Warning (node_name_chars_strict): /sysreg@10000/sys_led: Character '_' not recommended in node name
>> Warning (node_name_chars_strict): /sysreg@10000/sys_mci: Character '_' not recommended in node name
>> Warning (node_name_chars_strict): /sysreg@10000/sys_flash: Character '_' not recommended in node name
>>
>> The correct way to fix this as well as the original unit-address presence
>> warnings is to use the standard gpio controller binding and specify the
>> reg properties as per the hardware as it was before.
>>
>> However note that Vexpress sysreg MFD driver will still continue to use
>> the hardcoded values for compatibility reasons.
>>
>> Cc: Liviu Dudau <liviu.dudau@arm.com>
>> Suggested-by: Rob Herring <robh@kernel.org>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>>  arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 11 ++++++++---
>>  arch/arm/boot/dts/vexpress-v2m.dtsi     | 11 ++++++++---
>>  2 files changed, 16 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>> index 7b8ff5b3b912..a8586a0b957d 100644
>> --- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>> +++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>> @@ -76,21 +76,26 @@
>>                                 v2m_sysreg: sysreg@10000 {
>>                                         compatible = "arm,vexpress-sysreg";
>>                                         reg = <0x010000 0x1000>;
>> +                                       #address-cells = <1>;
>> +                                       #size-cells = <1>;
> 
> You need a ranges here and below to translate from 0-0x10000 range.
> 
> With that,
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> 

Thanks, I have appended it with below change

Regards,
Sudeep

--

diff --git c/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
w/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
index a8586a0b957d..4488c8fe213a 100644
--- c/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
+++ w/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
@@ -78,6 +78,7 @@
                                        reg = <0x010000 0x1000>;
                                        #address-cells = <1>;
                                        #size-cells = <1>;
+                                       ranges = <0 0x10000 0x1000>;
                                        v2m_led_gpios: gpio@8 {
                                                compatible =
"arm,vexpress-sysreg,sys_led";
diff --git c/arch/arm/boot/dts/vexpress-v2m.dtsi
w/arch/arm/boot/dts/vexpress-v2m.dtsi
index 37ecccebd937..4db42f6326a3 100644
--- c/arch/arm/boot/dts/vexpress-v2m.dtsi
+++ w/arch/arm/boot/dts/vexpress-v2m.dtsi
@@ -78,6 +78,7 @@
                                        reg = <0x00000 0x1000>;
                                        #address-cells = <1>;
                                        #size-cells = <1>;
+                                       ranges = <0 0 0x1000>;
                                        v2m_led_gpios: gpio@8 {
                                                compatible =
"arm,vexpress-sysreg,sys_led";

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 1/2] ARM: dts: vexpress: use standard gpio bindings for sys_{led,mci,flash}
@ 2018-05-11 13:16       ` Sudeep Holla
  0 siblings, 0 replies; 20+ messages in thread
From: Sudeep Holla @ 2018-05-11 13:16 UTC (permalink / raw)
  To: linux-arm-kernel



On 11/05/18 13:10, Rob Herring wrote:
> On Fri, May 11, 2018 at 5:23 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> Commit 2cff6dba57b7 ("ARM: dts: vexpress: fix node name unit-address presence warnings")
>> removed the unit address as there was no associated reg property in
>> these sysreg nodes.
>>
>> Also the latest DTC throws warnings for character '_' in the node names.
>>
>> Warning (node_name_chars_strict): /sysreg at 10000/sys_led: Character '_' not recommended in node name
>> Warning (node_name_chars_strict): /sysreg at 10000/sys_mci: Character '_' not recommended in node name
>> Warning (node_name_chars_strict): /sysreg at 10000/sys_flash: Character '_' not recommended in node name
>>
>> The correct way to fix this as well as the original unit-address presence
>> warnings is to use the standard gpio controller binding and specify the
>> reg properties as per the hardware as it was before.
>>
>> However note that Vexpress sysreg MFD driver will still continue to use
>> the hardcoded values for compatibility reasons.
>>
>> Cc: Liviu Dudau <liviu.dudau@arm.com>
>> Suggested-by: Rob Herring <robh@kernel.org>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>>  arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 11 ++++++++---
>>  arch/arm/boot/dts/vexpress-v2m.dtsi     | 11 ++++++++---
>>  2 files changed, 16 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>> index 7b8ff5b3b912..a8586a0b957d 100644
>> --- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>> +++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>> @@ -76,21 +76,26 @@
>>                                 v2m_sysreg: sysreg at 10000 {
>>                                         compatible = "arm,vexpress-sysreg";
>>                                         reg = <0x010000 0x1000>;
>> +                                       #address-cells = <1>;
>> +                                       #size-cells = <1>;
> 
> You need a ranges here and below to translate from 0-0x10000 range.
> 
> With that,
> 
> Reviewed-by: Rob Herring <robh@kernel.org>
> 

Thanks, I have appended it with below change

Regards,
Sudeep

--

diff --git c/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
w/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
index a8586a0b957d..4488c8fe213a 100644
--- c/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
+++ w/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
@@ -78,6 +78,7 @@
                                        reg = <0x010000 0x1000>;
                                        #address-cells = <1>;
                                        #size-cells = <1>;
+                                       ranges = <0 0x10000 0x1000>;
                                        v2m_led_gpios: gpio at 8 {
                                                compatible =
"arm,vexpress-sysreg,sys_led";
diff --git c/arch/arm/boot/dts/vexpress-v2m.dtsi
w/arch/arm/boot/dts/vexpress-v2m.dtsi
index 37ecccebd937..4db42f6326a3 100644
--- c/arch/arm/boot/dts/vexpress-v2m.dtsi
+++ w/arch/arm/boot/dts/vexpress-v2m.dtsi
@@ -78,6 +78,7 @@
                                        reg = <0x00000 0x1000>;
                                        #address-cells = <1>;
                                        #size-cells = <1>;
+                                       ranges = <0 0 0x1000>;
                                        v2m_led_gpios: gpio at 8 {
                                                compatible =
"arm,vexpress-sysreg,sys_led";

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH] ARM: dts: vexpress: Replace '_' with '-' in node names
  2018-05-09 21:14   ` Rob Herring
@ 2018-05-23  8:02     ` Linus Walleij
  -1 siblings, 0 replies; 20+ messages in thread
From: Linus Walleij @ 2018-05-23  8:02 UTC (permalink / raw)
  To: Rob Herring
  Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Liviu Dudau,
	moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
	Sudeep Holla

On Wed, May 9, 2018 at 11:14 PM, Rob Herring <robh+dt@kernel.org> wrote:
> On Wed, May 9, 2018 at 11:48 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> The latest DTC throws warnings for character '_' in the node names.
>>
>> Warning (node_name_chars_strict): /sysreg@10000/sys_led: Character '_' not recommended in node name
>> Warning (node_name_chars_strict): /sysreg@10000/sys_mci: Character '_' not recommended in node name
>> Warning (node_name_chars_strict): /sysreg@10000/sys_flash: Character '_' not recommended in node name
>>
>> The general recommendation is to use character '-' for all the node names.
>> This patch fixes the warnings following the recommendation.
>>
>> Cc: Liviu Dudau <liviu.dudau@arm.com>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>>  arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>> index 7b8ff5b3b912..58e73131ecef 100644
>> --- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>> +++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>> @@ -77,19 +77,19 @@
>>                                         compatible = "arm,vexpress-sysreg";
>>                                         reg = <0x010000 0x1000>;
>>
>> -                                       v2m_led_gpios: sys_led {
>> +                                       v2m_led_gpios: sys-led {
>
> Except this is a gpio-controller so it should have 'gpio' for its node
> name. (I have a dtc check written for that, but there are too many
> false positives.)
>
> But then you have 3 of them and no addressing, so you need to add reg
> property (with the register's offset and size) and unit-address.
>
> I'm surprised Linus W accepted these a GPIO when they are not really
> general purpose, but then lots of things slip in.

I guess is was back in this day when we had a finger constantly
on the fastforward button for DT conversion, and a few not so
elegant things slipped in.

I was annoyed by this thing later, especially since others started
to use it as a consistency argument "well you allowed this so
now allow this other crazy thing that looks the same" :D

I suspect I either was not CC:ed or I just sucked at shepherding
this, I try to do better these days.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH] ARM: dts: vexpress: Replace '_' with '-' in node names
@ 2018-05-23  8:02     ` Linus Walleij
  0 siblings, 0 replies; 20+ messages in thread
From: Linus Walleij @ 2018-05-23  8:02 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, May 9, 2018 at 11:14 PM, Rob Herring <robh+dt@kernel.org> wrote:
> On Wed, May 9, 2018 at 11:48 AM, Sudeep Holla <sudeep.holla@arm.com> wrote:
>> The latest DTC throws warnings for character '_' in the node names.
>>
>> Warning (node_name_chars_strict): /sysreg at 10000/sys_led: Character '_' not recommended in node name
>> Warning (node_name_chars_strict): /sysreg at 10000/sys_mci: Character '_' not recommended in node name
>> Warning (node_name_chars_strict): /sysreg at 10000/sys_flash: Character '_' not recommended in node name
>>
>> The general recommendation is to use character '-' for all the node names.
>> This patch fixes the warnings following the recommendation.
>>
>> Cc: Liviu Dudau <liviu.dudau@arm.com>
>> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
>> ---
>>  arch/arm/boot/dts/vexpress-v2m-rs1.dtsi | 6 +++---
>>  1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>> index 7b8ff5b3b912..58e73131ecef 100644
>> --- a/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>> +++ b/arch/arm/boot/dts/vexpress-v2m-rs1.dtsi
>> @@ -77,19 +77,19 @@
>>                                         compatible = "arm,vexpress-sysreg";
>>                                         reg = <0x010000 0x1000>;
>>
>> -                                       v2m_led_gpios: sys_led {
>> +                                       v2m_led_gpios: sys-led {
>
> Except this is a gpio-controller so it should have 'gpio' for its node
> name. (I have a dtc check written for that, but there are too many
> false positives.)
>
> But then you have 3 of them and no addressing, so you need to add reg
> property (with the register's offset and size) and unit-address.
>
> I'm surprised Linus W accepted these a GPIO when they are not really
> general purpose, but then lots of things slip in.

I guess is was back in this day when we had a finger constantly
on the fastforward button for DT conversion, and a few not so
elegant things slipped in.

I was annoyed by this thing later, especially since others started
to use it as a consistency argument "well you allowed this so
now allow this other crazy thing that looks the same" :D

I suspect I either was not CC:ed or I just sucked at shepherding
this, I try to do better these days.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2018-05-23  8:02 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-09 16:48 [PATCH] ARM: dts: vexpress: Replace '_' with '-' in node names Sudeep Holla
2018-05-09 16:48 ` Sudeep Holla
2018-05-09 21:14 ` Rob Herring
2018-05-09 21:14   ` Rob Herring
2018-05-10 10:51   ` Sudeep Holla
2018-05-10 10:51     ` Sudeep Holla
2018-05-10 14:05     ` Rob Herring
2018-05-10 14:05       ` Rob Herring
2018-05-10 14:16       ` Sudeep Holla
2018-05-10 14:16         ` Sudeep Holla
2018-05-23  8:02   ` Linus Walleij
2018-05-23  8:02     ` Linus Walleij
2018-05-11 10:23 ` [PATCH v2 1/2] ARM: dts: vexpress: use standard gpio bindings for sys_{led, mci, flash} Sudeep Holla
2018-05-11 10:23   ` Sudeep Holla
2018-05-11 10:23   ` [PATCH v2 2/2] ARM: dts: vexpress: replace '_' with '-' in node names Sudeep Holla
2018-05-11 10:23     ` Sudeep Holla
2018-05-11 12:10   ` [PATCH v2 1/2] ARM: dts: vexpress: use standard gpio bindings for sys_{led, mci, flash} Rob Herring
2018-05-11 12:10     ` Rob Herring
2018-05-11 13:16     ` [PATCH v2 1/2] ARM: dts: vexpress: use standard gpio bindings for sys_{led,mci,flash} Sudeep Holla
2018-05-11 13:16       ` Sudeep Holla

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.