Hi,
> Le 18 janv. 2017 à 09:05, Mathias Kresin <d...@kresin.me> a écrit :
>
> Hey Thibaut,
>
> see my comment inline.
>
> Mathias
>
> 17.01.2017 23:12, ha...@slashdirt.org:
>> --- a/target/linux/ramips/base-files/etc/diag.sh
>> +++ b/target/linux/ramips/base-files/etc/diag.sh
>> @@ -53,7 +53,8 @@ get_status_led() {
>> jhr-n825r|\
>> mpr-a1|\
>> mpr-a2|\
>> - mzk-ex750np)
>> + mzk-ex750np|\
>> + wn3000rpv3)
>> status_led="$board:red:power"
>
> Any specific reason why you are using the red led here? I mean, you are
> switching on the green led already in the dts and red is at least for me some
> kind of warning signal, which I would not expect to see if everything is fine.
Yes, this is to match stock firmware’s behavior:
This is a dual-color led. At power on, the boot loader brings both LEDs on,
giving an orange status color. During boot up, LEDE will signal the boot /
failsafe by blinking the red led, giving an orange-green blink. After boot is
completed, the red LED is turned off to give a solid green.
> If you decide to use the green led, you can remove the red power off led from
> /etc/board.d/01_leds.
I think it’s fine the way it is. It also matches the EX2700 setup which is a
very similar device.
>> --- /dev/null
>> +++ b/target/linux/ramips/dts/WN3000RPV3.dts
>> @@ -0,0 +1,148 @@
>> +/dts-v1/;
>> +
>> +#include "mt7620a.dtsi"
>> +
>
> Please include <dt-bindings/gpio/gpio.h> here as well.
>
> Use the GPIO_ACTIVE_LOW and GPIO_ACTIVE_HIGH macros afterwards in stead of 1
> and 0 in the gpio parameters.
>
> Check the recent ramips board additions for examples.
Done.
>> +
>> + l_arrow {
>> + label = "wn3000rpv3:blue:l_arrow";
>
> I guess it would be more clear to use leftarrow and rightarrow here instead
> of abbreviations.
Done.
>> + gpios = <&gpio0 7 1>;
>> + };
>> +
>> + r_arrow {
>> + label = "wn3000rpv3:blue:r_arrow";
>> + gpios = <&gpio0 8 1>;
>> + };
>> + };
>> +
>> + gpio-keys-polled {
>> + compatible = "gpio-keys-polled";
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + poll-interval = <20>;
>> +
>> + reset {
>> + label = "reset";
>> + gpios = <&gpio0 1 1>;
>> + linux,code = <KEY_RESTART>;
>> + };
>> +
>> + wps {
>> + label = "wps";
>> + gpios = <&gpio0 2 1>;
>> + linux,code = <KEY_WPS_BUTTON>;
>> + };
>> + };
>> +};
>> +
>> +&gpio0 {
>> + status = "okay";
>> +};
>
> It should be safe to drop the gpio0 node. It should be already enabled in the
> mt7620a.dtsi.
Done.
>> +
>> +&gpio1 {
>> + status = "okay";
>> +};
>> +
>> +&spi0 {
>> + status = "okay";
>> +
>> + m25p80@0 {
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + compatible = "jedec,spi-nor";
>> + reg = <0>;
>> + linux,modalias = "m25p80", "mx25l6405d";
>
> Drop the linux,modalias line. It was only required for kernel < 4.4.
Done.
>> + spi-max-frequency = <10000000>;
>> +
>> + partition@0 {
>> + label = "u-boot";
>> + reg = <0x0 0x30000>;
>> + read-only;
>> + };
>> +
>> + partition@30000 {
>> + label = "u-boot-env";
>> + reg = <0x30000 0x10000>;
>> + read-only;
>> + };
>> +
>> + partition@40000 {
>> + label = "firmware";
>> + reg = <0x40000 0x7b0000>;
>> + };
>> +
>> + art: partition@7f0000 {
>> + label = "art";
>> + reg = <0x7f0000 0x10000>;
>> + read-only;
>> + };
>> + };
>> +};
>> +
>> +ðernet {
>> + mtd-mac-address = <&art 0x0>;
>> +};
>> +
>> +&wmac {
>> + mtd-mac-address = <&art 0x6>;
>> + ralink,mtd-eeprom = <&art 0x1000>;
>> +};
>> +
>> +&pinctrl {
>> + state_default: pinctrl0 {
>> + default {
>> + // spi refclk: pins 37, 38, 39
>> + // uartf: pins 8, 9, 10, 11, 12, 13, 14
>> + // i2c: pins 1, 2
>
> Remove the comments. If they are important, add the information to the commit
> message.
Done.
Thanks.
Thibaut
_______________________________________________
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev
