This is clearly not true - those numbers refer to a 12V fan on my
NanoPC-T4's 12V PWM circuit, while the output circuit here is 5V. If you
really want a placeholder here maybe just use <0 255>, or figure out
some empirical values with a suitable 5V fan that are actually meaningful.
This can never be enabled since it doesn't exist in the design at all,
so it's one place where deletion *would* make good sense. AFAICS this
means you also don't need i2c4 enabled either.
+};
It might be nice to disable HDMI and all the other display pieces given
that the board is physically headless.
@@ -0,0 +1,179 @@+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)+/*+*FriendlyElecNanoPC-T4boarddevicetreesource+*+*Copyright(c)2020FriendlyElecComputerTech.Co.,Ltd.+*(http://www.friendlyarm.com)+*+*Copyright(c)2018CollaboraLtd.+*+*Copyright(c)2020JensenHuang<jensenhuang@friendlyarm.com>+*Copyright(c)2020MartyJones<mj8263788@gmail.com>+*Copyright(c)2021TianlingShen<cnsztl@gmail.com>+*/++/dts-v1/;+#include"rk3399-nanopi4.dtsi"++/{+model="FriendlyElec NanoPi R4S";+compatible="friendlyarm,nanopi-r4s","rockchip,rk3399";++/delete-node/gpio-leds;
Why? You could justify deleting &status_led, but redefining the whole
node from scratch seems unnecessary.
First of all, thank you for reviewing, and sorry for my poor English.
I need to redefine `pinctrl-0`, but if I use `/delete-property/
pinctrl-0;`, it will throw an error,
so maybe I made a mistake? And I will try again...
This is clearly not true - those numbers refer to a 12V fan on my
NanoPC-T4's 12V PWM circuit, while the output circuit here is 5V. If you
really want a placeholder here maybe just use <0 255>, or figure out
some empirical values with a suitable 5V fan that are actually meaningful.
Okay... I'll drop these as they're not really meaningful.
This can never be enabled since it doesn't exist in the design at all,
so it's one place where deletion *would* make good sense. AFAICS this
means you also don't need i2c4 enabled either.
Is it fine to disable i2c4 directly?
quoted
+};
It might be nice to disable HDMI and all the other display pieces given
that the board is physically headless.
@@ -0,0 +1,179 @@+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)+/*+*FriendlyElecNanoPC-T4boarddevicetreesource+*+*Copyright(c)2020FriendlyElecComputerTech.Co.,Ltd.+*(http://www.friendlyarm.com)+*+*Copyright(c)2018CollaboraLtd.+*+*Copyright(c)2020JensenHuang<jensenhuang@friendlyarm.com>+*Copyright(c)2020MartyJones<mj8263788@gmail.com>+*Copyright(c)2021TianlingShen<cnsztl@gmail.com>+*/++/dts-v1/;+#include"rk3399-nanopi4.dtsi"++/{+model="FriendlyElec NanoPi R4S";+compatible="friendlyarm,nanopi-r4s","rockchip,rk3399";++/delete-node/gpio-leds;
Why? You could justify deleting &status_led, but redefining the whole
node from scratch seems unnecessary.
First of all, thank you for reviewing, and sorry for my poor English.
I need to redefine `pinctrl-0`, but if I use `/delete-property/
pinctrl-0;`, it will throw an error,
so maybe I made a mistake? And I will try again...
You don't need to delete the property itself though - simply specifying
it replaces whatever previous value was inherited from the DTSI. Think
about how all those "status = ..." lines work, for example.
Similarly, given that you're redefining the led-0 node anyway you
wouldn't really *need* to delete that either; doing so just avoids the
extra &status_led label hanging around if the DTB is built with symbols,
and saves having to explicitly override/delete the default trigger
property if necessary.
Nit: (apologies for overlooking it before) there isn't an obvious
definitive order for the LEDs, but the order here is certainly not
consistent with anything. The most logical would probably be sys, wan,
lan since that's both in order of GPIO number and how they are
physically positioned relative to each other on the board/case (although
you could also argue for wan, lan, sys in that regard, depending on how
you look at it).
quoted
quoted
+ };
+
+ /delete-node/ gpio-keys;
Ditto - just removing the power key node itself should suffice.
This is clearly not true - those numbers refer to a 12V fan on my
NanoPC-T4's 12V PWM circuit, while the output circuit here is 5V. If you
really want a placeholder here maybe just use <0 255>, or figure out
some empirical values with a suitable 5V fan that are actually meaningful.
Okay... I'll drop these as they're not really meaningful.
This can never be enabled since it doesn't exist in the design at all,
so it's one place where deletion *would* make good sense. AFAICS this
means you also don't need i2c4 enabled either.
Is it fine to disable i2c4 directly?
I think it would make sense, since it's not physically available short
of trying to solder on to the 0201 pull-up resistors.
quoted
quoted
+};
It might be nice to disable HDMI and all the other display pieces given
that the board is physically headless.
I mean that if the other boards already support SDR104, they can
presumably support slower modes as well, so if these are worth having at
all then they could probably go in the common DTSI.
quoted
quoted
+};
+
What about the Bluetooth stuff on uart0?
R4S doesn't have it, so I guess I should disable uart0, like i2c4.
Yes, the UART itself isn't available on the board, and either way you
certainly don't want the kernel wasting time and possibly throwing
errors trying to probe a non-existent device through it.
Thanks,
Robin.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Nit: (apologies for overlooking it before) there isn't an obvious
definitive order for the LEDs, but the order here is certainly not
consistent with anything. The most logical would probably be sys, wan,
Looks like alphabetical sort order to me ;-)
lan since that's both in order of GPIO number and how they are
physically positioned relative to each other on the board/case (although
you could also argue for wan, lan, sys in that regard, depending on how
you look at it).
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Nit: (apologies for overlooking it before) there isn't an obvious
definitive order for the LEDs, but the order here is certainly not
consistent with anything. The most logical would probably be sys, wan,
Looks like alphabetical sort order to me ;-)
yep ... led-0, led-1, led-2 looks pretty sorted ;-)
Generally I'd prefer sorting by node-names ... especially as the phandle
is sort of optional for most things - and they sometimes come and go
in dt-files.
Heiko
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Nit: (apologies for overlooking it before) there isn't an obvious
definitive order for the LEDs, but the order here is certainly not
consistent with anything. The most logical would probably be sys, wan,
Generally I'd prefer sorting by node-names ... especially as the phandle
is sort of optional for most things - and they sometimes come and go
in dt-files.
The node names are sorted, too, as you've just pointed out...
Personally, I'm not so fond of the <foo>-%u node names, and prefer
<foo>-<function>. With the former, it's way too easy to have a silent
override in your .dts(i) stack.
Cfr. commit 45f5d5a9e34d3fe4 ("arm64: dts: renesas: r8a77995: draak:
Fix backlight regulator name")
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Geert,
On 2021-03-16 02:23 Geert Uytterhoeven [off-list ref] wrote:
Personally, I'm not so fond of the <foo>-%u node names, and prefer
<foo>-<function>. With the former, it's way too easy to have a silent
override in your .dts(i) stack.
Cfr. commit 45f5d5a9e34d3fe4 ("arm64: dts: renesas: r8a77995: draak:
Fix backlight regulator name")
How about using `lan-led`, `sys-led` and `wan-led` here?
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Hi Tianling,
CC Jacek, Pavel
On Tue, Mar 16, 2021 at 4:00 PM Tianling Shen [off-list ref] wrote:
On 2021-03-16 02:23 Geert Uytterhoeven [off-list ref] wrote:
quoted
Personally, I'm not so fond of the <foo>-%u node names, and prefer
<foo>-<function>. With the former, it's way too easy to have a silent
override in your .dts(i) stack.
Cfr. commit 45f5d5a9e34d3fe4 ("arm64: dts: renesas: r8a77995: draak:
Fix backlight regulator name")
How about using `lan-led`, `sys-led` and `wan-led` here?
Documentation/devicetree/bindings/leds/leds-gpio.yaml says "led-%u"
is the preferred form, but that anything containing "led" as a substring
is accepted. So I'd go for "led-lan" etc.
BTW, you can validate your DTB against the leds-gpio DT bindings
by running:
make dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/leds/leds-gpio.yaml
Background info for CCed parties:
https://lore.kernel.org/linux-arm-kernel/20210316150033.15987-1-cnsztl@gmail.com/
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On 2021-03-16 11:35, Geert Uytterhoeven [off-list ref] wrote：
Hi Tianling,
CC Jacek, Pavel
On Tue, Mar 16, 2021 at 4:00 PM Tianling Shen [off-list ref] wrote:
quoted
On 2021-03-16 02:23 Geert Uytterhoeven [off-list ref] wrote:
quoted
Personally, I'm not so fond of the <foo>-%u node names, and prefer
<foo>-<function>. With the former, it's way too easy to have a silent
override in your .dts(i) stack.
Cfr. commit 45f5d5a9e34d3fe4 ("arm64: dts: renesas: r8a77995: draak:
Fix backlight regulator name")
How about using `lan-led`, `sys-led` and `wan-led` here?
Documentation/devicetree/bindings/leds/leds-gpio.yaml says "led-%u"
is the preferred form, but that anything containing "led" as a substring
is accepted. So I'd go for "led-lan" etc.
BTW, you can validate your DTB against the leds-gpio DT bindings
by running:
make dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/leds/leds-gpio.yaml
Thank you so much!
I renamed them to `led-lan` etc., and the result of dtbs_check seems fine.
Background info for CCed parties:
https://lore.kernel.org/linux-arm-kernel/20210316150033.15987-1-cnsztl@gmail.com/
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
From: Pavel Machek <hidden> Date: 2021-03-16 19:39:36
On Tue 2021-03-16 16:34:50, Geert Uytterhoeven wrote:
Hi Tianling,
CC Jacek, Pavel
On Tue, Mar 16, 2021 at 4:00 PM Tianling Shen [off-list ref] wrote:
quoted
On 2021-03-16 02:23 Geert Uytterhoeven [off-list ref] wrote:
quoted
Personally, I'm not so fond of the <foo>-%u node names, and prefer
<foo>-<function>. With the former, it's way too easy to have a silent
override in your .dts(i) stack.
Cfr. commit 45f5d5a9e34d3fe4 ("arm64: dts: renesas: r8a77995: draak:
Fix backlight regulator name")
How about using `lan-led`, `sys-led` and `wan-led` here?
Documentation/devicetree/bindings/leds/leds-gpio.yaml says "led-%u"
is the preferred form, but that anything containing "led" as a substring
is accepted. So I'd go for "led-lan" etc.
BTW, you can validate your DTB against the leds-gpio DT bindings
by running:
make dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/leds/leds-gpio.yaml
Background info for CCed parties:
Hi Pavel,
On 2021-03-17 03:38, Pavel Machek [off-list ref] wrote:
On Tue 2021-03-16 16:34:50, Geert Uytterhoeven wrote:
quoted
Hi Tianling,
CC Jacek, Pavel
On Tue, Mar 16, 2021 at 4:00 PM Tianling Shen [off-list ref] wrote:
quoted
On 2021-03-16 02:23 Geert Uytterhoeven [off-list ref] wrote:
quoted
Personally, I'm not so fond of the <foo>-%u node names, and prefer
<foo>-<function>. With the former, it's way too easy to have a silent
override in your .dts(i) stack.
Cfr. commit 45f5d5a9e34d3fe4 ("arm64: dts: renesas: r8a77995: draak:
Fix backlight regulator name")
How about using `lan-led`, `sys-led` and `wan-led` here?
Documentation/devicetree/bindings/leds/leds-gpio.yaml says "led-%u"
is the preferred form, but that anything containing "led" as a substring
is accepted. So I'd go for "led-lan" etc.
BTW, you can validate your DTB against the leds-gpio DT bindings
by running:
make dtbs_check
DT_SCHEMA_FILES=Documentation/devicetree/bindings/leds/leds-gpio.yaml
Background info for CCed parties:
https://lore.kernel.org/linux-arm-kernel/20210316150033.15987-1-cnsztl@gmail.com/
I don't care much either way, lan-0 is okay as is lan-led.
but...
+ label = "nanopi-r4s:green:lan";
+ label = "nanopi-r4s:red:sys";
+ label = "nanopi-r4s:green:wan";
It would be good to have common labels, that means LED_FUNCTION_LAN,
LED_FUNCTION_WAN, and figuring out something better than "sys",
possibly LED_FUNCTION_FAULT?
LED_FUNCTION_POWER for "sys" would be fine, I think.
However, Documentation/leds/leds-class.rst says the form of naming is
"devicename:color:function",
and according to the given examples, as well as other dts(i), would it
be okay to use `green:lan`
etc. as the lable?
From: Pavel Machek <hidden> Date: 2021-04-25 21:25:43
Hi!
However, Documentation/leds/leds-class.rst says the form of naming is
"devicename:color:function",
and according to the given examples, as well as other dts(i), would it
be okay to use `green:lan`
etc. as the lable?
@@ -0,0 +1,179 @@+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)+/*+*FriendlyElecNanoPC-T4boarddevicetreesource+*+*Copyright(c)2020FriendlyElecComputerTech.Co.,Ltd.+*(http://www.friendlyarm.com)+*+*Copyright(c)2018CollaboraLtd.+*+*Copyright(c)2020JensenHuang<jensenhuang@friendlyarm.com>+*Copyright(c)2020MartyJones<mj8263788@gmail.com>+*Copyright(c)2021TianlingShen<cnsztl@gmail.com>+*/++/dts-v1/;+#include"rk3399-nanopi4.dtsi"++/{+model="FriendlyElec NanoPi R4S";+compatible="friendlyarm,nanopi-r4s","rockchip,rk3399";++/delete-node/gpio-leds;
Why? You could justify deleting &status_led, but redefining the whole
node from scratch seems unnecessary.
First of all, thank you for reviewing, and sorry for my poor English.
I need to redefine `pinctrl-0`, but if I use `/delete-property/
pinctrl-0;`, it will throw an error,
so maybe I made a mistake? And I will try again...
You don't need to delete the property itself though - simply specifying
it replaces whatever previous value was inherited from the DTSI. Think
about how all those "status = ..." lines work, for example.
I see, thank you so much!
Similarly, given that you're redefining the led-0 node anyway you
wouldn't really *need* to delete that either; doing so just avoids the
extra &status_led label hanging around if the DTB is built with symbols,
and saves having to explicitly override/delete the default trigger
property if necessary.
I plan to take advice from Geert, rename them to `lan-led`, `sys-led`,`wan-led`, so deleting `led-0` might to be need here...>
Nit: (apologies for overlooking it before) there isn't an obvious
definitive order for the LEDs, but the order here is certainly not
consistent with anything. The most logical would probably be sys, wan,
lan since that's both in order of GPIO number and how they are
physically positioned relative to each other on the board/case (although
you could also argue for wan, lan, sys in that regard, depending on how
you look at it).
quoted
quoted
quoted
+ };
+
+ /delete-node/ gpio-keys;
Ditto - just removing the power key node itself should suffice.
This is clearly not true - those numbers refer to a 12V fan on my
NanoPC-T4's 12V PWM circuit, while the output circuit here is 5V. If you
really want a placeholder here maybe just use <0 255>, or figure out
some empirical values with a suitable 5V fan that are actually meaningful.
Okay... I'll drop these as they're not really meaningful.
This can never be enabled since it doesn't exist in the design at all,
so it's one place where deletion *would* make good sense. AFAICS this
means you also don't need i2c4 enabled either.
Is it fine to disable i2c4 directly?
I think it would make sense, since it's not physically available short
of trying to solder on to the 0201 pull-up resistors.
quoted
quoted
quoted
+};
It might be nice to disable HDMI and all the other display pieces given
that the board is physically headless.
I mean that if the other boards already support SDR104, they can
presumably support slower modes as well, so if these are worth having at
all then they could probably go in the common DTSI.
I'm not sure, just based on the dts of R2S, and I added them here.
However they should be general for all NanoPi4 boards.
quoted
quoted
quoted
+};
+
What about the Bluetooth stuff on uart0?
R4S doesn't have it, so I guess I should disable uart0, like i2c4.
Yes, the UART itself isn't available on the board, and either way you
certainly don't want the kernel wasting time and possibly throwing
errors trying to probe a non-existent device through it.
Thanks,
Robin.