ARM: dts: vexpress: Replace '_' with '-' in node names

Message ID 1525884482-19855-1-git-send-email-sudeep.holla@arm.com
State New
Headers show
Series
  • ARM: dts: vexpress: Replace '_' with '-' in node names
Related show

Commit Message

Sudeep Holla May 9, 2018, 4:48 p.m.
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(-)

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Rob Herring May 9, 2018, 9:14 p.m. | #1
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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sudeep Holla May 10, 2018, 10:51 a.m. | #2
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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring May 10, 2018, 2:05 p.m. | #3
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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Sudeep Holla May 10, 2018, 2:16 p.m. | #4
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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Walleij May 23, 2018, 8:02 a.m. | #5
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
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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>;