W dniu 14.12.2020 o 17:28, Adrian Schmutzler pisze:
> Hi,
>
> only have time for a quick subset of answers:
>
[...]
>>>> arduino,yun|\
>>>> buffalo,bhr-4grv2|\
>>>> devolo,magic-2-wifi|\
>>>> diff --git a/target/linux/ath79/dts/qca9550_mojo_c-75.dts
>>>> b/target/linux/ath79/dts/qca9550_mojo_c-75.dts
>>>> new file mode 100644
>>>> index 000000000000..51046a740a02
>>>> --- /dev/null
>>>> +++ b/target/linux/ath79/dts/qca9550_mojo_c-75.dts
>>>> @@ -0,0 +1,173 @@
>>>> +// SPDX-License-Identifier: GPL-2.0-or-later OR MIT
>>>> +
>>>> +#include <dt-bindings/gpio/gpio.h>
>>>> +#include <dt-bindings/input/input.h>
>>>> +
>>>> +#include "qca955x.dtsi"
>>>
>>> This include needs to be first (after the license).
>>
>> I followed Linux kernel style in which first we add global includes in
>> alphabetical order then local includes. Will change that.
>
> This is just because dts-v1 is introduced from ath79.dtsi via qca955x.dtsi
> and dts-v1 has to be before anything else. This is also done like this in
> kernel (but not consistently), e.g. ipq4019/ipq806x.
Ok.
[...]
>>>> +
>>>> + aliases {
>>>> + label-mac-device = ð0;
>>>> + led-boot = &led_power;
>>>> + led-failsafe = &led_power;
>>>> + led-upgrade = &led_power;
>>>
>>> No led-running?
>>
>> The power LED is by default on, so it makes led-running redundant. Is there
>> something else also using that alias?
>
> default-on is first, then diag.sh starts doing stuff due to led-boot, i.e.
> will have the LED blinking. I'm not sure what happens after that without
> led-running, i.e. is the LED turned off or on. Anyway, that will be the job
> of diag.sh then, not of default-on anymore.
The power led behaves as intended with every led-* state and stays on after, so
I don't think there's a need to duplicate that in userspace and will keep the
current aliases. If I'll come across any issues with that, I'll send a patch.
>
>>
>>>
>>>> + };
>>>> +
>>>> + keys {
>>>> + compatible = "gpio-keys";
>>>> +
>>>> + reset {
>>>> + linux,code = <KEY_RESTART>;
>>>> + gpios = <&gpio 17 GPIO_ACTIVE_LOW>;
>>>
>>> missing label?
>>
>> The label is set from the node name by default.
>
> Hmm, so essentially we could drop label for about 50 % of our keys (in
> contrast to the more complex situation with LEDs)?
Sorry, got confused with LEDs and thought that's the same with gpio input, I'll
add label to this node.
>
>>
>>>
>>>> + };
>>>> + };
>>>> +
>>>> + leds {
>>>> + compatible = "gpio-leds";
>>>> +
>>>> + led_power: power {
>>>> + label = "amber:power";
>>>> + gpios = <&gpio 14 GPIO_ACTIVE_HIGH>;
>>>> + default-state = "on";
>>>> + };
>>>> +
>>>> + wlan2g {
>>>> + label = "green:wlan2g";
>>>> + gpios = <&gpio 13 GPIO_ACTIVE_LOW>;
>>>> + linux,default-trigger = "phy1tpt";
>>>> + };
>>>> + };
>>>> +};
>>>> +
>>>> +ð0 {
>>>> + status = "okay";
>>>> +
>>>> + mtd-mac-address = <&art 0x0>;
>>>> + phy-handle = <&phy0>;
>>>> + phy-mode = "rgmii";
>>>> + pll-data = <0xa6000000 0x00000101 0x00001616>; };
>>>> +
>>>> +ð1 {
>>>> + status = "okay";
>>>> +
>>>> + mtd-mac-address = <&art 0x6>;
>>>> + phy-mode = "sgmii";
>>>> + pll-data = <0x03000101 0x00000101 0x00001616>;
>>>> +
>>>> + fixed-link {
>>>> + speed = <1000>;
>>>> + full-duplex;
>>>> + };
>>>> +};
>>>> +
>>>> +&mdio0 {
>>>> + status = "okay";
>>>> +
>>>> + phy0: ethernet-phy@0 {
>>>> + reg = <0>;
>>>> + qca,ar8327-initvals = <
>>>> + 0x0c 0x00080080
>>>> + 0x04 0x07600000
>>>> + 0x58 0xc935c935
>>>> + 0x5c 0x03ffff00
>>>> + 0x7c 0x0000007e
>>>> + 0x94 0x0000007e
>>>> + >;
>>>> + };
>>>> +};
>>>
>>> Please move mdio0 up so it's either directly before or after eth0.
>>
>> The nodes are sorted alphabetically. There are no includes specified
>> between nodes, which would make following applied sorting not possible.
>> What are the criteria saying that this node should be above ð nodes?
>
> Well, mdioX is a subnode of ethX. Thus, I tend to see them as related, and
> that's probably also the reason why they are typically grouped together in
> the ar****/qca**** DTSI files.
> It just makes reading the config easier for me when they are grouped this
> way. I won't force you to change that if you think alphabetic sorting is
> superior.
Ok, that's sensible I'll move it after eth0.
[...]
>
> Best
>
> Adrian
>
Regards.
--
TMN
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/mailman/listinfo/openwrt-devel
