[PATCH 0/4] dts: mediatek: add Cudy WR3000 V1 wireless router

[PATCH 0/4] dts: mediatek: add Cudy WR3000 V1 wireless router

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

Cudy WR3000 V1 is a MediaTek Filogic 820 based 802.11ax wireless router.

https://www.cudy.com/productinfo/1637722.html

Rafał Miłecki (4):
  dt-bindings: vendor-prefixes: add Cudy
  dt-bindings: arm64: dts: mediatek: Add Cudy WR3000 V1 router
  arm64: dts: mediatek: mt7981: add pinctrl
  arm64: dts: mediatek: Add Cudy WR3000 V1

 .../devicetree/bindings/arm/mediatek.yaml     |  1 +
 .../devicetree/bindings/vendor-prefixes.yaml  |  2 +
 arch/arm64/boot/dts/mediatek/Makefile         |  1 +
 .../dts/mediatek/mt7981b-cudy-wr3000-v1.dts   | 74 +++++++++++++++++++
 arch/arm64/boot/dts/mediatek/mt7981b.dtsi     | 37 ++++++++++
 5 files changed, 115 insertions(+)
 create mode 100644 arch/arm64/boot/dts/mediatek/mt7981b-cudy-wr3000-v1.dts

-- 
2.35.3

[PATCH 1/4] dt-bindings: vendor-prefixes: add Cudy

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

Cudy is a Chinese company providing networking products.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 Documentation/devicetree/bindings/vendor-prefixes.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/vendor-prefixes.yaml b/Documentation/devicetree/bindings/vendor-prefixes.yaml
index 2dbcdc5f5d7d..cb4dd176bf41 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.yaml
+++ b/Documentation/devicetree/bindings/vendor-prefixes.yaml
@@ -332,6 +332,8 @@ patternProperties:
     description: Czech Technical University in Prague
   "^cubietech,.*":
     description: Cubietech, Ltd.
+  "^cudy,.*":
+    description: Shenzhen Cudy Technology Co., Ltd.
   "^cui,.*":
     description: CUI Devices
   "^cypress,.*":
-- 
2.35.3

Re: [PATCH 1/4] dt-bindings: vendor-prefixes: add Cudy

[Krzysztof Kozlowski]

On 21/02/2024 08:35, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Cudy is a Chinese company providing networking products.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

[PATCH 2/4] dt-bindings: arm64: dts: mediatek: Add Cudy WR3000 V1 router

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

Cudy WR3000 V1 is an MT7981B (AKA Filogic 820) based wireless router.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 Documentation/devicetree/bindings/arm/mediatek.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/arm/mediatek.yaml b/Documentation/devicetree/bindings/arm/mediatek.yaml
index 09f9ffd3ff7b..1cf47db94c57 100644
--- a/Documentation/devicetree/bindings/arm/mediatek.yaml
+++ b/Documentation/devicetree/bindings/arm/mediatek.yaml
@@ -85,6 +85,7 @@ properties:
           - const: mediatek,mt7629
       - items:
           - enum:
+              - cudy,wr3000-v1
               - xiaomi,ax3000t
           - const: mediatek,mt7981b
       - items:
-- 
2.35.3

Re: [PATCH 2/4] dt-bindings: arm64: dts: mediatek: Add Cudy WR3000 V1 router

[Krzysztof Kozlowski]

On 21/02/2024 08:35, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Cudy WR3000 V1 is an MT7981B (AKA Filogic 820) based wireless router.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH 2/4] dt-bindings: arm64: dts: mediatek: Add Cudy WR3000 V1 router

[AngeloGioacchino Del Regno]

Il 21/02/24 08:35, Rafał Miłecki ha scritto:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Cudy WR3000 V1 is an MT7981B (AKA Filogic 820) based wireless router.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>

Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

[PATCH 3/4] arm64: dts: mediatek: mt7981: add pinctrl

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

MT7981 contains on-SoC PIN controller that is also a GPIO provider.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 arch/arm64/boot/dts/mediatek/mt7981b.dtsi | 37 +++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/arch/arm64/boot/dts/mediatek/mt7981b.dtsi b/arch/arm64/boot/dts/mediatek/mt7981b.dtsi
index 4feff3d1c5f4..fdd5c22cfc9c 100644
--- a/arch/arm64/boot/dts/mediatek/mt7981b.dtsi
+++ b/arch/arm64/boot/dts/mediatek/mt7981b.dtsi
@@ -86,6 +86,43 @@ pwm@10048000 {
 			#pwm-cells = <2>;
 		};
 
+		pio: pinctrl@11d00000 {
+			compatible = "mediatek,mt7981-pinctrl";
+			reg = <0 0x11d00000 0 0x1000>,
+			      <0 0x11c00000 0 0x1000>,
+			      <0 0x11c10000 0 0x1000>,
+			      <0 0x11d20000 0 0x1000>,
+			      <0 0x11e00000 0 0x1000>,
+			      <0 0x11e20000 0 0x1000>,
+			      <0 0x11f00000 0 0x1000>,
+			      <0 0x11f10000 0 0x1000>,
+			      <0 0x1000b000 0 0x1000>;
+			reg-names = "gpio", "iocfg_rt", "iocfg_rm", "iocfg_rb", "iocfg_lb",
+				    "iocfg_bl", "iocfg_tm", "iocfg_tl", "eint";
+			interrupt-controller;
+			interrupts = <GIC_SPI 225 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-parent = <&gic>;
+			gpio-ranges = <&pio 0 0 56>;
+			gpio-controller;
+			#gpio-cells = <2>;
+			#interrupt-cells = <2>;
+
+			mdio-pins {
+				mux {
+					function = "eth";
+					groups = "smi_mdc_mdio";
+				};
+			};
+
+			spi0-pins {
+				mux {
+					function = "spi";
+					groups = "spi0", "spi0_wp_hold";
+				};
+			};
+
+		};
+
 		clock-controller@15000000 {
 			compatible = "mediatek,mt7981-ethsys", "syscon";
 			reg = <0 0x15000000 0 0x1000>;
-- 
2.35.3

Re: [PATCH 3/4] arm64: dts: mediatek: mt7981: add pinctrl

[AngeloGioacchino Del Regno]

Il 21/02/24 08:35, Rafał Miłecki ha scritto:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> MT7981 contains on-SoC PIN controller that is also a GPIO provider.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>   arch/arm64/boot/dts/mediatek/mt7981b.dtsi | 37 +++++++++++++++++++++++
>   1 file changed, 37 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/mediatek/mt7981b.dtsi b/arch/arm64/boot/dts/mediatek/mt7981b.dtsi
> index 4feff3d1c5f4..fdd5c22cfc9c 100644
> --- a/arch/arm64/boot/dts/mediatek/mt7981b.dtsi
> +++ b/arch/arm64/boot/dts/mediatek/mt7981b.dtsi
> @@ -86,6 +86,43 @@ pwm@10048000 {
>   			#pwm-cells = <2>;
>   		};
>   
> +		pio: pinctrl@11d00000 {
> +			compatible = "mediatek,mt7981-pinctrl";
> +			reg = <0 0x11d00000 0 0x1000>,
> +			      <0 0x11c00000 0 0x1000>,
> +			      <0 0x11c10000 0 0x1000>,
> +			      <0 0x11d20000 0 0x1000>,
> +			      <0 0x11e00000 0 0x1000>,
> +			      <0 0x11e20000 0 0x1000>,
> +			      <0 0x11f00000 0 0x1000>,
> +			      <0 0x11f10000 0 0x1000>,
> +			      <0 0x1000b000 0 0x1000>;
> +			reg-names = "gpio", "iocfg_rt", "iocfg_rm", "iocfg_rb", "iocfg_lb",
> +				    "iocfg_bl", "iocfg_tm", "iocfg_tl", "eint";
> +			interrupt-controller;
> +			interrupts = <GIC_SPI 225 IRQ_TYPE_LEVEL_HIGH>;
> +			interrupt-parent = <&gic>;
> +			gpio-ranges = <&pio 0 0 56>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			#interrupt-cells = <2>;
> +
> +			mdio-pins {
> +				mux {

That's board specific. MDIO and SPI0 pins can be used as GPIO instead of,
respectively, ETH and SPI.

Must go to your board devicetree, not here: please move both.

Cheers,
Angelo

> +					function = "eth";
> +					groups = "smi_mdc_mdio";
> +				};
> +			};
> +
> +			spi0-pins {
> +				mux {
> +					function = "spi";
> +					groups = "spi0", "spi0_wp_hold";
> +				};
> +			};
> +
> +		};
> +
>   		clock-controller@15000000 {
>   			compatible = "mediatek,mt7981-ethsys", "syscon";
>   			reg = <0 0x15000000 0 0x1000>;

[PATCH 4/4] arm64: dts: mediatek: Add Cudy WR3000 V1

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

Cudy WR3000 V1 is an MT7981B (AKA Filogic 820) based wireless router. It
has 256 MiB of RAM, some LEDs & buttons and (not described yet) 4
Ethernet ports.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
 arch/arm64/boot/dts/mediatek/Makefile         |  1 +
 .../dts/mediatek/mt7981b-cudy-wr3000-v1.dts   | 74 +++++++++++++++++++
 2 files changed, 75 insertions(+)
 create mode 100644 arch/arm64/boot/dts/mediatek/mt7981b-cudy-wr3000-v1.dts

diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
index 37b4ca3a87c9..96da4ad640aa 100644
--- a/arch/arm64/boot/dts/mediatek/Makefile
+++ b/arch/arm64/boot/dts/mediatek/Makefile
@@ -8,6 +8,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-x20-dev.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
+dtb-$(CONFIG_ARCH_MEDIATEK) += mt7981b-cudy-wr3000-v1.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt7981b-xiaomi-ax3000t.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-acelink-ew-7886cax.dtb
 dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3.dtb
diff --git a/arch/arm64/boot/dts/mediatek/mt7981b-cudy-wr3000-v1.dts b/arch/arm64/boot/dts/mediatek/mt7981b-cudy-wr3000-v1.dts
new file mode 100644
index 000000000000..cb36a089518a
--- /dev/null
+++ b/arch/arm64/boot/dts/mediatek/mt7981b-cudy-wr3000-v1.dts
@@ -0,0 +1,74 @@
+// SPDX-License-Identifier: GPL-2.0-only OR MIT
+
+/dts-v1/;
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/leds/common.h>
+
+#include "mt7981b.dtsi"
+
+/ {
+	compatible = "cudy,wr3000-v1", "mediatek,mt7981b";
+	model = "Cudy WR3000 V1";
+
+	memory@40000000 {
+		reg = <0 0x40000000 0 0x10000000>;
+		device_type = "memory";
+	};
+
+	keys {
+		compatible = "gpio-keys";
+
+		key-wps {
+			label = "WPS";
+			gpios = <&pio 0 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_WPS_BUTTON>;
+		};
+
+		key-reset {
+			label = "RESET";
+			gpios = <&pio 1 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_RESTART>;
+		};
+	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		led-0 {
+			function = LED_FUNCTION_WAN;
+			color = <LED_COLOR_ID_BLUE>;
+			gpios = <&pio 5 GPIO_ACTIVE_LOW>;
+		};
+
+		led-1 {
+			function = LED_FUNCTION_WLAN_2GHZ;
+			color = <LED_COLOR_ID_BLUE>;
+			gpios = <&pio 6 GPIO_ACTIVE_LOW>;
+		};
+
+		led-2 {
+			function = LED_FUNCTION_WLAN_5GHZ;
+			color = <LED_COLOR_ID_BLUE>;
+			gpios = <&pio 7 GPIO_ACTIVE_LOW>;
+		};
+
+		led-3 {
+			function = LED_FUNCTION_LAN;
+			color = <LED_COLOR_ID_BLUE>;
+			gpios = <&pio 9 GPIO_ACTIVE_LOW>;
+		};
+
+		led-4 {
+			function = LED_FUNCTION_STATUS;
+			color = <LED_COLOR_ID_BLUE>;
+			gpios = <&pio 10 GPIO_ACTIVE_LOW>;
+		};
+
+		led-5 {
+			function = "online";
+			color = <LED_COLOR_ID_BLUE>;
+			gpios = <&pio 11 GPIO_ACTIVE_LOW>;
+		};
+	};
+};
-- 
2.35.3

Re: [PATCH 4/4] arm64: dts: mediatek: Add Cudy WR3000 V1

[AngeloGioacchino Del Regno]

Il 21/02/24 08:35, Rafał Miłecki ha scritto:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> Cudy WR3000 V1 is an MT7981B (AKA Filogic 820) based wireless router. It
> has 256 MiB of RAM, some LEDs & buttons and (not described yet) 4
> Ethernet ports.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
>   arch/arm64/boot/dts/mediatek/Makefile         |  1 +
>   .../dts/mediatek/mt7981b-cudy-wr3000-v1.dts   | 74 +++++++++++++++++++
>   2 files changed, 75 insertions(+)
>   create mode 100644 arch/arm64/boot/dts/mediatek/mt7981b-cudy-wr3000-v1.dts
> 
> diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
> index 37b4ca3a87c9..96da4ad640aa 100644
> --- a/arch/arm64/boot/dts/mediatek/Makefile
> +++ b/arch/arm64/boot/dts/mediatek/Makefile
> @@ -8,6 +8,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-x20-dev.dtb
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt7981b-cudy-wr3000-v1.dtb
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7981b-xiaomi-ax3000t.dtb
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-acelink-ew-7886cax.dtb
>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3.dtb
> diff --git a/arch/arm64/boot/dts/mediatek/mt7981b-cudy-wr3000-v1.dts b/arch/arm64/boot/dts/mediatek/mt7981b-cudy-wr3000-v1.dts
> new file mode 100644
> index 000000000000..cb36a089518a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/mediatek/mt7981b-cudy-wr3000-v1.dts
> @@ -0,0 +1,74 @@
> +// SPDX-License-Identifier: GPL-2.0-only OR MIT
> +
> +/dts-v1/;
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/leds/common.h>
> +
> +#include "mt7981b.dtsi"
> +
> +/ {
> +	compatible = "cudy,wr3000-v1", "mediatek,mt7981b";
> +	model = "Cudy WR3000 V1";
> +
> +	memory@40000000 {
> +		reg = <0 0x40000000 0 0x10000000>;
> +		device_type = "memory";
> +	};
> +
> +	keys {
> +		compatible = "gpio-keys";
> +
> +		key-wps {
> +			label = "WPS";
> +			gpios = <&pio 0 GPIO_ACTIVE_LOW>;
> +			linux,code = <KEY_WPS_BUTTON>;
> +		};
> +
> +		key-reset {
> +			label = "RESET";
> +			gpios = <&pio 1 GPIO_ACTIVE_LOW>;
> +			linux,code = <KEY_RESTART>;
> +		};
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +
> +		led-0 {
> +			function = LED_FUNCTION_WAN;
> +			color = <LED_COLOR_ID_BLUE>;
> +			gpios = <&pio 5 GPIO_ACTIVE_LOW>;

Can we please order those properties alphabetically, as it doesn't impact
on human readability in any way?
Just a nitpick, anyway.

color = <LED_COLOR_ID_BLUE>;
function = LED_FUNCTION_WAN;
gpios = <&pio 5 GPIO_ACTIVE_LOW>;

> +		};
> +
> +		led-1 {
> +			function = LED_FUNCTION_WLAN_2GHZ;
> +			color = <LED_COLOR_ID_BLUE>;
> +			gpios = <&pio 6 GPIO_ACTIVE_LOW>;
> +		};
> +
> +		led-2 {
> +			function = LED_FUNCTION_WLAN_5GHZ;
> +			color = <LED_COLOR_ID_BLUE>;
> +			gpios = <&pio 7 GPIO_ACTIVE_LOW>;
> +		};
> +
> +		led-3 {
> +			function = LED_FUNCTION_LAN;
> +			color = <LED_COLOR_ID_BLUE>;
> +			gpios = <&pio 9 GPIO_ACTIVE_LOW>;
> +		};
> +
> +		led-4 {
> +			function = LED_FUNCTION_STATUS;
> +			color = <LED_COLOR_ID_BLUE>;
> +			gpios = <&pio 10 GPIO_ACTIVE_LOW>;
> +		};
> +
> +		led-5 {
> +			function = "online";

Uhm, what does "online" mean?

Are you sure that you can't use any of the LED_FUNCTION_XXX standard definitions?

Cheers,
Angelo

Re: [PATCH 4/4] arm64: dts: mediatek: Add Cudy WR3000 V1

[Rafał Miłecki]

On 21.02.2024 11:11, AngeloGioacchino Del Regno wrote:
> Il 21/02/24 08:35, Rafał Miłecki ha scritto:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> Cudy WR3000 V1 is an MT7981B (AKA Filogic 820) based wireless router. It
>> has 256 MiB of RAM, some LEDs & buttons and (not described yet) 4
>> Ethernet ports.
>>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>>   arch/arm64/boot/dts/mediatek/Makefile         |  1 +
>>   .../dts/mediatek/mt7981b-cudy-wr3000-v1.dts   | 74 +++++++++++++++++++
>>   2 files changed, 75 insertions(+)
>>   create mode 100644 arch/arm64/boot/dts/mediatek/mt7981b-cudy-wr3000-v1.dts
>>
>> diff --git a/arch/arm64/boot/dts/mediatek/Makefile b/arch/arm64/boot/dts/mediatek/Makefile
>> index 37b4ca3a87c9..96da4ad640aa 100644
>> --- a/arch/arm64/boot/dts/mediatek/Makefile
>> +++ b/arch/arm64/boot/dts/mediatek/Makefile
>> @@ -8,6 +8,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb
>>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-x20-dev.dtb
>>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
>>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
>> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt7981b-cudy-wr3000-v1.dtb
>>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7981b-xiaomi-ax3000t.dtb
>>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-acelink-ew-7886cax.dtb
>>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3.dtb
>> diff --git a/arch/arm64/boot/dts/mediatek/mt7981b-cudy-wr3000-v1.dts b/arch/arm64/boot/dts/mediatek/mt7981b-cudy-wr3000-v1.dts
>> new file mode 100644
>> index 000000000000..cb36a089518a
>> --- /dev/null
>> +++ b/arch/arm64/boot/dts/mediatek/mt7981b-cudy-wr3000-v1.dts
>> @@ -0,0 +1,74 @@
>> +// SPDX-License-Identifier: GPL-2.0-only OR MIT
>> +
>> +/dts-v1/;
>> +#include <dt-bindings/input/input.h>
>> +#include <dt-bindings/gpio/gpio.h>
>> +#include <dt-bindings/leds/common.h>
>> +
>> +#include "mt7981b.dtsi"
>> +
>> +/ {
>> +    compatible = "cudy,wr3000-v1", "mediatek,mt7981b";
>> +    model = "Cudy WR3000 V1";
>> +
>> +    memory@40000000 {
>> +        reg = <0 0x40000000 0 0x10000000>;
>> +        device_type = "memory";
>> +    };
>> +
>> +    keys {
>> +        compatible = "gpio-keys";
>> +
>> +        key-wps {
>> +            label = "WPS";
>> +            gpios = <&pio 0 GPIO_ACTIVE_LOW>;
>> +            linux,code = <KEY_WPS_BUTTON>;
>> +        };
>> +
>> +        key-reset {
>> +            label = "RESET";
>> +            gpios = <&pio 1 GPIO_ACTIVE_LOW>;
>> +            linux,code = <KEY_RESTART>;
>> +        };
>> +    };
>> +
>> +    leds {
>> +        compatible = "gpio-leds";
>> +
>> +        led-0 {
>> +            function = LED_FUNCTION_WAN;
>> +            color = <LED_COLOR_ID_BLUE>;
>> +            gpios = <&pio 5 GPIO_ACTIVE_LOW>;
> 
> Can we please order those properties alphabetically, as it doesn't impact
> on human readability in any way?
> Just a nitpick, anyway.
> 
> color = <LED_COLOR_ID_BLUE>;
> function = LED_FUNCTION_WAN;
> gpios = <&pio 5 GPIO_ACTIVE_LOW>;

If I understand [1] correctly alphabetical / alpha-numerical order
applies to non-addresses nodes only.

Here I put "function" first as that seemed more important than a color.

Perhaps you could suggest an addition to Devicetree Sources (DTS) Coding
Style or point me to the missed part, please?

[1] https://www.kernel.org/doc/html/next/devicetree/bindings/dts-coding-style.html#order-of-properties-in-device-node



>> +        };
>> +
>> +        led-1 {
>> +            function = LED_FUNCTION_WLAN_2GHZ;
>> +            color = <LED_COLOR_ID_BLUE>;
>> +            gpios = <&pio 6 GPIO_ACTIVE_LOW>;
>> +        };
>> +
>> +        led-2 {
>> +            function = LED_FUNCTION_WLAN_5GHZ;
>> +            color = <LED_COLOR_ID_BLUE>;
>> +            gpios = <&pio 7 GPIO_ACTIVE_LOW>;
>> +        };
>> +
>> +        led-3 {
>> +            function = LED_FUNCTION_LAN;
>> +            color = <LED_COLOR_ID_BLUE>;
>> +            gpios = <&pio 9 GPIO_ACTIVE_LOW>;
>> +        };
>> +
>> +        led-4 {
>> +            function = LED_FUNCTION_STATUS;
>> +            color = <LED_COLOR_ID_BLUE>;
>> +            gpios = <&pio 10 GPIO_ACTIVE_LOW>;
>> +        };
>> +
>> +        led-5 {
>> +            function = "online";
> 
> Uhm, what does "online" mean?
> 
> Are you sure that you can't use any of the LED_FUNCTION_XXX standard definitions?

This device has:
1. "WAN Port" LED that indicates "WAN Port has connection" state
2. "Internet" LED that indicates "Connected to the Internet" state

I couldn't find any define that would fit Internet access LED case.

Re: [PATCH 4/4] arm64: dts: mediatek: Add Cudy WR3000 V1

[AngeloGioacchino Del Regno]

Il 21/02/24 11:18, Rafał Miłecki ha scritto:
> On 21.02.2024 11:11, AngeloGioacchino Del Regno wrote:
>> Il 21/02/24 08:35, Rafał Miłecki ha scritto:
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> Cudy WR3000 V1 is an MT7981B (AKA Filogic 820) based wireless router. It
>>> has 256 MiB of RAM, some LEDs & buttons and (not described yet) 4
>>> Ethernet ports.
>>>
>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>> ---
>>>   arch/arm64/boot/dts/mediatek/Makefile         |  1 +
>>>   .../dts/mediatek/mt7981b-cudy-wr3000-v1.dts   | 74 +++++++++++++++++++
>>>   2 files changed, 75 insertions(+)
>>>   create mode 100644 arch/arm64/boot/dts/mediatek/mt7981b-cudy-wr3000-v1.dts
>>>
>>> diff --git a/arch/arm64/boot/dts/mediatek/Makefile 
>>> b/arch/arm64/boot/dts/mediatek/Makefile
>>> index 37b4ca3a87c9..96da4ad640aa 100644
>>> --- a/arch/arm64/boot/dts/mediatek/Makefile
>>> +++ b/arch/arm64/boot/dts/mediatek/Makefile
>>> @@ -8,6 +8,7 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-evb.dtb
>>>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt6797-x20-dev.dtb
>>>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-rfb1.dtb
>>>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7622-bananapi-bpi-r64.dtb
>>> +dtb-$(CONFIG_ARCH_MEDIATEK) += mt7981b-cudy-wr3000-v1.dtb
>>>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7981b-xiaomi-ax3000t.dtb
>>>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-acelink-ew-7886cax.dtb
>>>   dtb-$(CONFIG_ARCH_MEDIATEK) += mt7986a-bananapi-bpi-r3.dtb
>>> diff --git a/arch/arm64/boot/dts/mediatek/mt7981b-cudy-wr3000-v1.dts 
>>> b/arch/arm64/boot/dts/mediatek/mt7981b-cudy-wr3000-v1.dts
>>> new file mode 100644
>>> index 000000000000..cb36a089518a
>>> --- /dev/null
>>> +++ b/arch/arm64/boot/dts/mediatek/mt7981b-cudy-wr3000-v1.dts
>>> @@ -0,0 +1,74 @@
>>> +// SPDX-License-Identifier: GPL-2.0-only OR MIT
>>> +
>>> +/dts-v1/;
>>> +#include <dt-bindings/input/input.h>
>>> +#include <dt-bindings/gpio/gpio.h>
>>> +#include <dt-bindings/leds/common.h>
>>> +
>>> +#include "mt7981b.dtsi"
>>> +
>>> +/ {
>>> +    compatible = "cudy,wr3000-v1", "mediatek,mt7981b";
>>> +    model = "Cudy WR3000 V1";
>>> +
>>> +    memory@40000000 {
>>> +        reg = <0 0x40000000 0 0x10000000>;
>>> +        device_type = "memory";
>>> +    };
>>> +
>>> +    keys {
>>> +        compatible = "gpio-keys";
>>> +
>>> +        key-wps {
>>> +            label = "WPS";
>>> +            gpios = <&pio 0 GPIO_ACTIVE_LOW>;
>>> +            linux,code = <KEY_WPS_BUTTON>;
>>> +        };
>>> +
>>> +        key-reset {
>>> +            label = "RESET";
>>> +            gpios = <&pio 1 GPIO_ACTIVE_LOW>;
>>> +            linux,code = <KEY_RESTART>;
>>> +        };
>>> +    };
>>> +
>>> +    leds {
>>> +        compatible = "gpio-leds";
>>> +
>>> +        led-0 {
>>> +            function = LED_FUNCTION_WAN;
>>> +            color = <LED_COLOR_ID_BLUE>;
>>> +            gpios = <&pio 5 GPIO_ACTIVE_LOW>;
>>
>> Can we please order those properties alphabetically, as it doesn't impact
>> on human readability in any way?
>> Just a nitpick, anyway.
>>
>> color = <LED_COLOR_ID_BLUE>;
>> function = LED_FUNCTION_WAN;
>> gpios = <&pio 5 GPIO_ACTIVE_LOW>;
> 
> If I understand [1] correctly alphabetical / alpha-numerical order
> applies to non-addresses nodes only.
> 
> Here I put "function" first as that seemed more important than a color.
> 
> Perhaps you could suggest an addition to Devicetree Sources (DTS) Coding
> Style or point me to the missed part, please?
> 
> [1] 
> https://www.kernel.org/doc/html/next/devicetree/bindings/dts-coding-style.html#order-of-properties-in-device-node
> 
> 

I didn't say that this was *mandatory*, but since alphanumeric ordering in device
trees is actually a thing for nodes, this could be as well applied to properties
inside when this doesn't impact on human readability.

As far as I'm aware, I'm not the only maintainer asking for that (am I?!)

If you've got any *strong opinion* about having `function` as the first property,
I'm full ears, and I will be okay with that. No problem on my side, I'm open for
alternatives when those make sense.

> 
>>> +        };
>>> +
>>> +        led-1 {
>>> +            function = LED_FUNCTION_WLAN_2GHZ;
>>> +            color = <LED_COLOR_ID_BLUE>;
>>> +            gpios = <&pio 6 GPIO_ACTIVE_LOW>;
>>> +        };
>>> +
>>> +        led-2 {
>>> +            function = LED_FUNCTION_WLAN_5GHZ;
>>> +            color = <LED_COLOR_ID_BLUE>;
>>> +            gpios = <&pio 7 GPIO_ACTIVE_LOW>;
>>> +        };
>>> +
>>> +        led-3 {
>>> +            function = LED_FUNCTION_LAN;
>>> +            color = <LED_COLOR_ID_BLUE>;
>>> +            gpios = <&pio 9 GPIO_ACTIVE_LOW>;
>>> +        };
>>> +
>>> +        led-4 {
>>> +            function = LED_FUNCTION_STATUS;
>>> +            color = <LED_COLOR_ID_BLUE>;
>>> +            gpios = <&pio 10 GPIO_ACTIVE_LOW>;
>>> +        };
>>> +
>>> +        led-5 {
>>> +            function = "online";
>>
>> Uhm, what does "online" mean?
>>
>> Are you sure that you can't use any of the LED_FUNCTION_XXX standard definitions?
> 
> This device has:
> 1. "WAN Port" LED that indicates "WAN Port has connection" state

Just a curiosity: is the WAN port ethernet, fiber or what?
(to entirely clarify in my brain if that LED is a carrier detect indicator,
but doesn't matter for this patch, it's really just a personal curiosity and
nothing else)

> 2. "Internet" LED that indicates "Connected to the Internet" state
> 
> I couldn't find any define that would fit Internet access LED case.

I was imagining something like that, but wasn't sure, so asking you was the best
choice.

I wonder if it makes sense to add a "wan-online" or "wan-connected" or .. I don't
know, some definition that makes common sense, to dt-bindings/leds/common.h to
somewhat standardize the function for that usecase?

One thing I'm (mostly) sure of, it makes sense to have the "wan-" prefix before
any word (because you can have sorts of authenticated LANs as well).

I don't think that there's anything hugely restrictive that would prevent you
from doing that: there are even "player-[1-5]" definitions for game controllers,
and yours is not the first router that I physically see with my own eyes that
has such a LED for that state.

After all, what's going to turn on that LED is userspace, and if we can get one
that works for all (without custom functions for each router), why not :-)