Linux-Devicetree Archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/2] mips: dts: ralink: Add support for TP-Link HC220 G5 v1 board
@ 2023-06-05 15:01 Liviu Dudau
  2023-06-05 15:01 ` [PATCH v3 1/2] " Liviu Dudau
  2023-06-05 15:01 ` [PATCH v3 2/2] dt-bindings: mips: Add bindings " Liviu Dudau
  0 siblings, 2 replies; 8+ messages in thread
From: Liviu Dudau @ 2023-06-05 15:01 UTC (permalink / raw
  To: Arınç ÜNAL
  Cc: Thomas Bogendoerfer, Paul Burton, Rob Herring, Sergio Paracuellos,
	Conor Dooley, Krzysztof Kozlowski, linux-mips, linux-kernel,
	devicetree, Liviu Dudau

Add device tree and bindings for the TP-Link HC220 G5 v1 wireless AP,
a consumer product from TP-Link based on MT7621.

Changes since v2:
 - Remove WIP nodes for NAND flash that accidentally got included in v2
 - Fix commit message with the actual 5GHz WiFi chip name
 - Remove gmac and mdio nodes from device tree as defaults are sufficient
 - Added sub-nodes under pcie for each of the WiFi chip with appropriate
   compatible string.
 - Collect Acks received for the device tree bindings patch

Changes since v1:
 - Changed compatible for the board to "tplink,hc220-g5-v1"
 - Updated the DSA switch nodes to better reflect actual usage.
 - Disabled the fixed-link in gmac1
 - Added device tree bindings in Documentation/

v2: https://lore.kernel.org/linux-mips/20230529150833.526084-1-liviu@dudau.co.uk/
v1: https://lore.kernel.org/linux-mips/20230509200125.309026-1-liviu@dudau.co.uk/

Liviu Dudau (2):
  mips: dts: ralink: Add support for TP-Link HC220 G5 v1 board
  dt-bindings: mips: Add bindings for TP-Link HC220 G5 v1 board

 .../devicetree/bindings/mips/ralink.yaml      |  1 +
 arch/mips/boot/dts/ralink/Makefile            |  3 +-
 .../dts/ralink/mt7621-tplink-hc220-g5-v1.dts  | 92 +++++++++++++++++++
 3 files changed, 95 insertions(+), 1 deletion(-)
 create mode 100644 arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts

-- 
2.40.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3 1/2] mips: dts: ralink: Add support for TP-Link HC220 G5 v1 board
  2023-06-05 15:01 [PATCH v3 0/2] mips: dts: ralink: Add support for TP-Link HC220 G5 v1 board Liviu Dudau
@ 2023-06-05 15:01 ` Liviu Dudau
  2023-06-05 16:35   ` Arınç ÜNAL
  2023-06-05 15:01 ` [PATCH v3 2/2] dt-bindings: mips: Add bindings " Liviu Dudau
  1 sibling, 1 reply; 8+ messages in thread
From: Liviu Dudau @ 2023-06-05 15:01 UTC (permalink / raw
  To: Arınç ÜNAL
  Cc: Thomas Bogendoerfer, Paul Burton, Rob Herring, Sergio Paracuellos,
	Conor Dooley, Krzysztof Kozlowski, linux-mips, linux-kernel,
	devicetree, Liviu Dudau

This WiFi AP is based on a MT7621 SoC with 128MiB RAM, 128MiB NAND,
a MT7603 2.4GHz WiFi and a MT7613 5GHz WiFi chips integrated on the board,
connected to the main SoC over PCIe.

The device uses NMBM over NAND, which is not currently supported in the
mainline, so NAND node is skipped in this revision.

Signed-off-by: Liviu Dudau <liviu@dudau.co.uk>
---
 arch/mips/boot/dts/ralink/Makefile            |  3 +-
 .../dts/ralink/mt7621-tplink-hc220-g5-v1.dts  | 92 +++++++++++++++++++
 2 files changed, 94 insertions(+), 1 deletion(-)
 create mode 100644 arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts

diff --git a/arch/mips/boot/dts/ralink/Makefile b/arch/mips/boot/dts/ralink/Makefile
index 11732b8c8163a..d27d7e8c700fe 100644
--- a/arch/mips/boot/dts/ralink/Makefile
+++ b/arch/mips/boot/dts/ralink/Makefile
@@ -8,6 +8,7 @@ dtb-$(CONFIG_DTB_VOCORE2)	+= vocore2.dtb
 
 dtb-$(CONFIG_SOC_MT7621) += \
 	mt7621-gnubee-gb-pc1.dtb \
-	mt7621-gnubee-gb-pc2.dtb
+	mt7621-gnubee-gb-pc2.dtb \
+	mt7621-tplink-hc220-g5-v1.dtb
 
 obj-$(CONFIG_BUILTIN_DTB)	+= $(addsuffix .o, $(dtb-y))
diff --git a/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts
new file mode 100644
index 0000000000000..859aaa1c1bc2b
--- /dev/null
+++ b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts
@@ -0,0 +1,92 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+/dts-v1/;
+
+#include "mt7621.dtsi"
+
+#include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/input/input.h>
+#include <dt-bindings/leds/common.h>
+
+/ {
+	compatible = "tplink,hc220-g5-v1", "mediatek,mt7621-soc";
+	model = "TP-Link HC220 G5 v1";
+
+	memory@0 {
+		device_type = "memory";
+		reg = <0x00000000 0x8000000>;
+	};
+
+	chosen {
+		bootargs = "earlycon console=ttyS0,115200";
+	};
+
+	gpio-keys {
+		compatible = "gpio-keys";
+
+		key-reset {
+			label = "reset";
+			gpios = <&gpio 8 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_RESTART>;
+		};
+
+		key-wps {
+			label = "wps";
+			gpios = <&gpio 16 GPIO_ACTIVE_LOW>;
+			linux,code = <KEY_WPS_BUTTON>;
+		};
+	};
+
+	leds {
+		compatible = "gpio-leds";
+
+		red {
+			color = <LED_COLOR_ID_RED>;
+			function = LED_FUNCTION_FAULT;
+			gpios = <&gpio 13 GPIO_ACTIVE_HIGH>;
+		};
+
+		green {
+			color = <LED_COLOR_ID_GREEN>;
+			function = LED_FUNCTION_POWER;
+			gpios = <&gpio 14 GPIO_ACTIVE_HIGH>;
+			linux,default-trigger = "default-on";
+		};
+
+		blue {
+			color = <LED_COLOR_ID_BLUE>;
+			function = LED_FUNCTION_WPS;
+			gpios = <&gpio 15 GPIO_ACTIVE_HIGH>;
+		};
+	};
+};
+
+&pcie {
+	status = "okay";
+
+	pcie@0,0 {
+		compatible = "mediatek,mt76";
+	};
+
+	pcie@1,0 {
+		compatible = "mediatek,mt76";
+	};
+};
+
+&switch0 {
+	ports {
+		port@0 {
+			status = "okay";
+			label = "lan2";
+		};
+
+		port@1 {
+			status = "okay";
+			label = "lan1";
+		};
+
+		port@2 {
+			status = "okay";
+			label = "wan";
+		};
+	};
+};
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v3 2/2] dt-bindings: mips: Add bindings for TP-Link HC220 G5 v1 board
  2023-06-05 15:01 [PATCH v3 0/2] mips: dts: ralink: Add support for TP-Link HC220 G5 v1 board Liviu Dudau
  2023-06-05 15:01 ` [PATCH v3 1/2] " Liviu Dudau
@ 2023-06-05 15:01 ` Liviu Dudau
  1 sibling, 0 replies; 8+ messages in thread
From: Liviu Dudau @ 2023-06-05 15:01 UTC (permalink / raw
  To: Arınç ÜNAL
  Cc: Thomas Bogendoerfer, Paul Burton, Rob Herring, Sergio Paracuellos,
	Conor Dooley, Krzysztof Kozlowski, linux-mips, linux-kernel,
	devicetree, Liviu Dudau, Conor Dooley

Add bindings for the compatible string used for the TP-Link's
HC220 G5 V1 board, a wireless AP based on MT7621.

Signed-off-by: Liviu Dudau <liviu@dudau.co.uk>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Acked-by: Sergio Paracuellos <sergio.paracuellos@gmail.com>
---
 Documentation/devicetree/bindings/mips/ralink.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/mips/ralink.yaml b/Documentation/devicetree/bindings/mips/ralink.yaml
index 704b5b5951271..53c1f66353770 100644
--- a/Documentation/devicetree/bindings/mips/ralink.yaml
+++ b/Documentation/devicetree/bindings/mips/ralink.yaml
@@ -80,6 +80,7 @@ properties:
           - enum:
               - gnubee,gb-pc1
               - gnubee,gb-pc2
+              - tplink,hc220-g5-v1
           - const: mediatek,mt7621-soc
 
 additionalProperties: true
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/2] mips: dts: ralink: Add support for TP-Link HC220 G5 v1 board
  2023-06-05 15:01 ` [PATCH v3 1/2] " Liviu Dudau
@ 2023-06-05 16:35   ` Arınç ÜNAL
  2023-06-05 21:01     ` Liviu Dudau
  0 siblings, 1 reply; 8+ messages in thread
From: Arınç ÜNAL @ 2023-06-05 16:35 UTC (permalink / raw
  To: Liviu Dudau, Rob Herring, Krzysztof Kozlowski
  Cc: Thomas Bogendoerfer, Paul Burton, Sergio Paracuellos,
	Conor Dooley, linux-mips, linux-kernel, devicetree

On 5.06.2023 18:01, Liviu Dudau wrote:
> This WiFi AP is based on a MT7621 SoC with 128MiB RAM, 128MiB NAND,
> a MT7603 2.4GHz WiFi and a MT7613 5GHz WiFi chips integrated on the board,
> connected to the main SoC over PCIe.
> 
> The device uses NMBM over NAND, which is not currently supported in the
> mainline, so NAND node is skipped in this revision.
> 
> Signed-off-by: Liviu Dudau <liviu@dudau.co.uk>
> ---
>   arch/mips/boot/dts/ralink/Makefile            |  3 +-
>   .../dts/ralink/mt7621-tplink-hc220-g5-v1.dts  | 92 +++++++++++++++++++
>   2 files changed, 94 insertions(+), 1 deletion(-)
>   create mode 100644 arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts
> 
> diff --git a/arch/mips/boot/dts/ralink/Makefile b/arch/mips/boot/dts/ralink/Makefile
> index 11732b8c8163a..d27d7e8c700fe 100644
> --- a/arch/mips/boot/dts/ralink/Makefile
> +++ b/arch/mips/boot/dts/ralink/Makefile
> @@ -8,6 +8,7 @@ dtb-$(CONFIG_DTB_VOCORE2)	+= vocore2.dtb
>   
>   dtb-$(CONFIG_SOC_MT7621) += \
>   	mt7621-gnubee-gb-pc1.dtb \
> -	mt7621-gnubee-gb-pc2.dtb
> +	mt7621-gnubee-gb-pc2.dtb \
> +	mt7621-tplink-hc220-g5-v1.dtb
>   
>   obj-$(CONFIG_BUILTIN_DTB)	+= $(addsuffix .o, $(dtb-y))
> diff --git a/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts
> new file mode 100644
> index 0000000000000..859aaa1c1bc2b
> --- /dev/null
> +++ b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts
> @@ -0,0 +1,92 @@
> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +/dts-v1/;
> +
> +#include "mt7621.dtsi"
> +
> +#include <dt-bindings/gpio/gpio.h>
> +#include <dt-bindings/input/input.h>
> +#include <dt-bindings/leds/common.h>
> +
> +/ {
> +	compatible = "tplink,hc220-g5-v1", "mediatek,mt7621-soc";
> +	model = "TP-Link HC220 G5 v1";
> +
> +	memory@0 {
> +		device_type = "memory";
> +		reg = <0x00000000 0x8000000>;

Please use 8 digit addressing for the memory start and size offsets:

0x00000000 0x08000000

> +	};
> +
> +	chosen {
> +		bootargs = "earlycon console=ttyS0,115200";
> +	};
> +
> +	gpio-keys {
> +		compatible = "gpio-keys";
> +
> +		key-reset {
> +			label = "reset";
> +			gpios = <&gpio 8 GPIO_ACTIVE_LOW>;
> +			linux,code = <KEY_RESTART>;
> +		};
> +
> +		key-wps {
> +			label = "wps";
> +			gpios = <&gpio 16 GPIO_ACTIVE_LOW>;
> +			linux,code = <KEY_WPS_BUTTON>;
> +		};
> +	};
> +
> +	leds {
> +		compatible = "gpio-leds";
> +
> +		red {

Usually the led name would point to the component the LED is used for.

> +			color = <LED_COLOR_ID_RED>;
> +			function = LED_FUNCTION_FAULT;

Is there a specific reason you're using leds/common.h,
color & function instead of 'label = "red:ledname"'?

> +			gpios = <&gpio 13 GPIO_ACTIVE_HIGH>;
> +		};
> +
> +		green {
> +			color = <LED_COLOR_ID_GREEN>;
> +			function = LED_FUNCTION_POWER;
> +			gpios = <&gpio 14 GPIO_ACTIVE_HIGH>;
> +			linux,default-trigger = "default-on";
> +		};
> +
> +		blue {
> +			color = <LED_COLOR_ID_BLUE>;
> +			function = LED_FUNCTION_WPS;
> +			gpios = <&gpio 15 GPIO_ACTIVE_HIGH>;
> +		};

Every led node needs the "led-" prefix to satisfy the leds-gpio.yaml
schema. You can check for dt-schema warnings using this command:

ARCH=mips make clean dtbs_check

Ignore the warning for mediatek,mt7621-eth.

> +	};
> +};
> +
> +&pcie {
> +	status = "okay";
> +
> +	pcie@0,0 {
> +		compatible = "mediatek,mt76";
> +	};
> +
> +	pcie@1,0 {
> +		compatible = "mediatek,mt76";
> +	};

Both radios work with this then?

Also, I see a bunch of warnings now that the mediatek,mt76 compatible
string is added. The warning from schemas/pci/pci-bus.yaml is concerning.

/mnt/Documents/for-netnext/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dtb: pcie@1e140000: pcie@0,0:compatible: ['mediatek,mt76'] does not contain items matching the given schema
	From schema: /mnt/Documents/for-netnext/Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
/mnt/Documents/for-netnext/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dtb: pcie@1e140000: pcie@0,0: Unevaluated properties are not allowed ('compatible' was unexpected)
	From schema: /mnt/Documents/for-netnext/Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
/mnt/Documents/for-netnext/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dtb: pcie@1e140000: Unevaluated properties are not allowed ('#address-cells', '#interrupt-cells', '#size-cells', 'device_type', 'interrupt-map', 'interrupt-map-mask', 'reset-gpios' were unexpected)
	From schema: /mnt/Documents/for-netnext/Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
/mnt/Documents/for-netnext/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dtb: pcie@1e140000: pcie@0,0:compatible: ['mediatek,mt76'] does not contain items matching the given schema
	From schema: /usr/lib/python3/dist-packages/dtschema/schemas/pci/pci-bus.yaml
/mnt/Documents/for-netnext/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dtb: pcie@0,0: clocks: [[2, 23]] is too short
	From schema: /mnt/Documents/for-netnext/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.yaml
/mnt/Documents/for-netnext/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dtb: pcie@0,0: Unevaluated properties are not allowed ('#address-cells', '#interrupt-cells', '#size-cells', 'clocks', 'device_type', 'interrupt-map', 'interrupt-map-mask', 'phy-names', 'phys', 'ranges' were unexpected)
	From schema: /mnt/Documents/for-netnext/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.yaml

Rob, Krzysztof any ideas what to do? The PCI child node is supposed to
be the properties of the wireless device. But the compatible string
doesn't match the schema on schemas/pci/pci-bus.yaml.

       compatible:
         contains:
           pattern: "^(pci[0-9a-f]{3,4},[0-9a-f]{1,4}|pciclass,[0-9a-f]{4,6})$"

Liviu, in the meantime, you should submit this patch without this
compatible string. I will handle this issue.

Arınç

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/2] mips: dts: ralink: Add support for TP-Link HC220 G5 v1 board
  2023-06-05 16:35   ` Arınç ÜNAL
@ 2023-06-05 21:01     ` Liviu Dudau
  2023-06-06  5:24       ` Arınç ÜNAL
  0 siblings, 1 reply; 8+ messages in thread
From: Liviu Dudau @ 2023-06-05 21:01 UTC (permalink / raw
  To: Arınç ÜNAL
  Cc: Rob Herring, Krzysztof Kozlowski, Thomas Bogendoerfer,
	Paul Burton, Sergio Paracuellos, Conor Dooley, linux-mips,
	linux-kernel, devicetree

On Mon, Jun 05, 2023 at 07:35:44PM +0300, Arınç ÜNAL wrote:
> On 5.06.2023 18:01, Liviu Dudau wrote:
> > This WiFi AP is based on a MT7621 SoC with 128MiB RAM, 128MiB NAND,
> > a MT7603 2.4GHz WiFi and a MT7613 5GHz WiFi chips integrated on the board,
> > connected to the main SoC over PCIe.
> > 
> > The device uses NMBM over NAND, which is not currently supported in the
> > mainline, so NAND node is skipped in this revision.
> > 
> > Signed-off-by: Liviu Dudau <liviu@dudau.co.uk>
> > ---
> >   arch/mips/boot/dts/ralink/Makefile            |  3 +-
> >   .../dts/ralink/mt7621-tplink-hc220-g5-v1.dts  | 92 +++++++++++++++++++
> >   2 files changed, 94 insertions(+), 1 deletion(-)
> >   create mode 100644 arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts
> > 
> > diff --git a/arch/mips/boot/dts/ralink/Makefile b/arch/mips/boot/dts/ralink/Makefile
> > index 11732b8c8163a..d27d7e8c700fe 100644
> > --- a/arch/mips/boot/dts/ralink/Makefile
> > +++ b/arch/mips/boot/dts/ralink/Makefile
> > @@ -8,6 +8,7 @@ dtb-$(CONFIG_DTB_VOCORE2)	+= vocore2.dtb
> >   dtb-$(CONFIG_SOC_MT7621) += \
> >   	mt7621-gnubee-gb-pc1.dtb \
> > -	mt7621-gnubee-gb-pc2.dtb
> > +	mt7621-gnubee-gb-pc2.dtb \
> > +	mt7621-tplink-hc220-g5-v1.dtb
> >   obj-$(CONFIG_BUILTIN_DTB)	+= $(addsuffix .o, $(dtb-y))
> > diff --git a/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts
> > new file mode 100644
> > index 0000000000000..859aaa1c1bc2b
> > --- /dev/null
> > +++ b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts
> > @@ -0,0 +1,92 @@
> > +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +/dts-v1/;
> > +
> > +#include "mt7621.dtsi"
> > +
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/input/input.h>
> > +#include <dt-bindings/leds/common.h>
> > +
> > +/ {
> > +	compatible = "tplink,hc220-g5-v1", "mediatek,mt7621-soc";
> > +	model = "TP-Link HC220 G5 v1";
> > +
> > +	memory@0 {
> > +		device_type = "memory";
> > +		reg = <0x00000000 0x8000000>;
> 
> Please use 8 digit addressing for the memory start and size offsets:
> 
> 0x00000000 0x08000000

Will do.

> 
> > +	};
> > +
> > +	chosen {
> > +		bootargs = "earlycon console=ttyS0,115200";
> > +	};
> > +
> > +	gpio-keys {
> > +		compatible = "gpio-keys";
> > +
> > +		key-reset {
> > +			label = "reset";
> > +			gpios = <&gpio 8 GPIO_ACTIVE_LOW>;
> > +			linux,code = <KEY_RESTART>;
> > +		};
> > +
> > +		key-wps {
> > +			label = "wps";
> > +			gpios = <&gpio 16 GPIO_ACTIVE_LOW>;
> > +			linux,code = <KEY_WPS_BUTTON>;
> > +		};
> > +	};
> > +
> > +	leds {
> > +		compatible = "gpio-leds";
> > +
> > +		red {
> 
> Usually the led name would point to the component the LED is used for.

These are "generic" LEDs controlled from the userspace. The original firmware
uses GREEN for normal operations, RED for faults and BLUE for when WPS is
enabled. I'm not sure if there are any standard bindings that I can use here.

> 
> > +			color = <LED_COLOR_ID_RED>;
> > +			function = LED_FUNCTION_FAULT;
> 
> Is there a specific reason you're using leds/common.h,
> color & function instead of 'label = "red:ledname"'?

I actually can't remember why I've created them this way. I might've been
under the impression that giving them standard colour names will make it
easier for userspace to identify and use them, but as I haven't yet
investigated into how I'm going to use the device I'm unaware of any
userspace requirements. It's possible OpenWRT or LEDE have some strong
definitions, but I'm not aware of them as I don't use any of the distros.


> 
> > +			gpios = <&gpio 13 GPIO_ACTIVE_HIGH>;
> > +		};
> > +
> > +		green {
> > +			color = <LED_COLOR_ID_GREEN>;
> > +			function = LED_FUNCTION_POWER;
> > +			gpios = <&gpio 14 GPIO_ACTIVE_HIGH>;
> > +			linux,default-trigger = "default-on";
> > +		};
> > +
> > +		blue {
> > +			color = <LED_COLOR_ID_BLUE>;
> > +			function = LED_FUNCTION_WPS;
> > +			gpios = <&gpio 15 GPIO_ACTIVE_HIGH>;
> > +		};
> 
> Every led node needs the "led-" prefix to satisfy the leds-gpio.yaml
> schema. You can check for dt-schema warnings using this command:
> 
> ARCH=mips make clean dtbs_check
> 
> Ignore the warning for mediatek,mt7621-eth.

Sure, will run the checks before submitting v4.

> 
> > +	};
> > +};
> > +
> > +&pcie {
> > +	status = "okay";
> > +
> > +	pcie@0,0 {
> > +		compatible = "mediatek,mt76";
> > +	};
> > +
> > +	pcie@1,0 {
> > +		compatible = "mediatek,mt76";
> > +	};
> 
> Both radios work with this then?

Yes, they work with and without the compatible property.

> 
> Also, I see a bunch of warnings now that the mediatek,mt76 compatible
> string is added. The warning from schemas/pci/pci-bus.yaml is concerning.
> 
> /mnt/Documents/for-netnext/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dtb: pcie@1e140000: pcie@0,0:compatible: ['mediatek,mt76'] does not contain items matching the given schema
> 	From schema: /mnt/Documents/for-netnext/Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
> /mnt/Documents/for-netnext/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dtb: pcie@1e140000: pcie@0,0: Unevaluated properties are not allowed ('compatible' was unexpected)
> 	From schema: /mnt/Documents/for-netnext/Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
> /mnt/Documents/for-netnext/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dtb: pcie@1e140000: Unevaluated properties are not allowed ('#address-cells', '#interrupt-cells', '#size-cells', 'device_type', 'interrupt-map', 'interrupt-map-mask', 'reset-gpios' were unexpected)
> 	From schema: /mnt/Documents/for-netnext/Documentation/devicetree/bindings/pci/mediatek,mt7621-pcie.yaml
> /mnt/Documents/for-netnext/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dtb: pcie@1e140000: pcie@0,0:compatible: ['mediatek,mt76'] does not contain items matching the given schema
> 	From schema: /usr/lib/python3/dist-packages/dtschema/schemas/pci/pci-bus.yaml
> /mnt/Documents/for-netnext/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dtb: pcie@0,0: clocks: [[2, 23]] is too short
> 	From schema: /mnt/Documents/for-netnext/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.yaml
> /mnt/Documents/for-netnext/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dtb: pcie@0,0: Unevaluated properties are not allowed ('#address-cells', '#interrupt-cells', '#size-cells', 'clocks', 'device_type', 'interrupt-map', 'interrupt-map-mask', 'phy-names', 'phys', 'ranges' were unexpected)
> 	From schema: /mnt/Documents/for-netnext/Documentation/devicetree/bindings/net/wireless/mediatek,mt76.yaml
> 
> Rob, Krzysztof any ideas what to do? The PCI child node is supposed to
> be the properties of the wireless device. But the compatible string
> doesn't match the schema on schemas/pci/pci-bus.yaml.

TBH, I've always found the attempts to add device tree nodes for PCI(e) devices
amusing. The bus is supposed to be queried and one can learn from device ID
what hardware they're talking to. But then you need device tree (or ACPI for x86_64)
to layer on top of that information about where EEPROM data might be, and other
metadata that blurs the "DT describes the hardware" line (you could technically
reformat the NAND and place the EEPROM data somewhere else on this platform).

> 
>       compatible:
>         contains:
>           pattern: "^(pci[0-9a-f]{3,4},[0-9a-f]{1,4}|pciclass,[0-9a-f]{4,6})$"
> 
> Liviu, in the meantime, you should submit this patch without this
> compatible string. I will handle this issue.

OK, I will drop the compatible strings from v4.

Thanks for the quick review!

Best regards,
Liviu

> 
> Arınç

-- 
Everyone who uses computers frequently has had, from time to time,
a mad desire to attack the precocious abacus with an axe.
       	   	      	     	  -- John D. Clark, Ignition!

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/2] mips: dts: ralink: Add support for TP-Link HC220 G5 v1 board
  2023-06-05 21:01     ` Liviu Dudau
@ 2023-06-06  5:24       ` Arınç ÜNAL
  2023-06-06 14:31         ` Liviu Dudau
  0 siblings, 1 reply; 8+ messages in thread
From: Arınç ÜNAL @ 2023-06-06  5:24 UTC (permalink / raw
  To: Liviu Dudau
  Cc: Rob Herring, Krzysztof Kozlowski, Thomas Bogendoerfer,
	Paul Burton, Sergio Paracuellos, Conor Dooley, linux-mips,
	linux-kernel, devicetree

On 6.06.2023 00:01, Liviu Dudau wrote:
> On Mon, Jun 05, 2023 at 07:35:44PM +0300, Arınç ÜNAL wrote:
>> On 5.06.2023 18:01, Liviu Dudau wrote:
>>> This WiFi AP is based on a MT7621 SoC with 128MiB RAM, 128MiB NAND,
>>> a MT7603 2.4GHz WiFi and a MT7613 5GHz WiFi chips integrated on the board,
>>> connected to the main SoC over PCIe.
>>>
>>> The device uses NMBM over NAND, which is not currently supported in the
>>> mainline, so NAND node is skipped in this revision.
>>>
>>> Signed-off-by: Liviu Dudau <liviu@dudau.co.uk>
>>> ---
>>>    arch/mips/boot/dts/ralink/Makefile            |  3 +-
>>>    .../dts/ralink/mt7621-tplink-hc220-g5-v1.dts  | 92 +++++++++++++++++++
>>>    2 files changed, 94 insertions(+), 1 deletion(-)
>>>    create mode 100644 arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts
>>>
>>> diff --git a/arch/mips/boot/dts/ralink/Makefile b/arch/mips/boot/dts/ralink/Makefile
>>> index 11732b8c8163a..d27d7e8c700fe 100644
>>> --- a/arch/mips/boot/dts/ralink/Makefile
>>> +++ b/arch/mips/boot/dts/ralink/Makefile
>>> @@ -8,6 +8,7 @@ dtb-$(CONFIG_DTB_VOCORE2)	+= vocore2.dtb
>>>    dtb-$(CONFIG_SOC_MT7621) += \
>>>    	mt7621-gnubee-gb-pc1.dtb \
>>> -	mt7621-gnubee-gb-pc2.dtb
>>> +	mt7621-gnubee-gb-pc2.dtb \
>>> +	mt7621-tplink-hc220-g5-v1.dtb
>>>    obj-$(CONFIG_BUILTIN_DTB)	+= $(addsuffix .o, $(dtb-y))
>>> diff --git a/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts
>>> new file mode 100644
>>> index 0000000000000..859aaa1c1bc2b
>>> --- /dev/null
>>> +++ b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts
>>> @@ -0,0 +1,92 @@
>>> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +/dts-v1/;
>>> +
>>> +#include "mt7621.dtsi"
>>> +
>>> +#include <dt-bindings/gpio/gpio.h>
>>> +#include <dt-bindings/input/input.h>
>>> +#include <dt-bindings/leds/common.h>
>>> +
>>> +/ {
>>> +	compatible = "tplink,hc220-g5-v1", "mediatek,mt7621-soc";
>>> +	model = "TP-Link HC220 G5 v1";
>>> +
>>> +	memory@0 {
>>> +		device_type = "memory";
>>> +		reg = <0x00000000 0x8000000>;
>>
>> Please use 8 digit addressing for the memory start and size offsets:
>>
>> 0x00000000 0x08000000
> 
> Will do.
> 
>>
>>> +	};
>>> +
>>> +	chosen {
>>> +		bootargs = "earlycon console=ttyS0,115200";
>>> +	};
>>> +
>>> +	gpio-keys {
>>> +		compatible = "gpio-keys";
>>> +
>>> +		key-reset {
>>> +			label = "reset";
>>> +			gpios = <&gpio 8 GPIO_ACTIVE_LOW>;
>>> +			linux,code = <KEY_RESTART>;
>>> +		};
>>> +
>>> +		key-wps {
>>> +			label = "wps";
>>> +			gpios = <&gpio 16 GPIO_ACTIVE_LOW>;
>>> +			linux,code = <KEY_WPS_BUTTON>;
>>> +		};
>>> +	};
>>> +
>>> +	leds {
>>> +		compatible = "gpio-leds";
>>> +
>>> +		red {
>>
>> Usually the led name would point to the component the LED is used for.
> 
> These are "generic" LEDs controlled from the userspace. The original firmware
> uses GREEN for normal operations, RED for faults and BLUE for when WPS is
> enabled. I'm not sure if there are any standard bindings that I can use here.

Looking at:

https://www.kernel.org/doc/html/latest/leds/leds-class.html#led-device-naming

You could use red:fault, green:power, and blue:wps. For node names, 
led-fault, led-power, and led-wps.

Arınç

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/2] mips: dts: ralink: Add support for TP-Link HC220 G5 v1 board
  2023-06-06  5:24       ` Arınç ÜNAL
@ 2023-06-06 14:31         ` Liviu Dudau
  2023-06-06 15:22           ` Arınç ÜNAL
  0 siblings, 1 reply; 8+ messages in thread
From: Liviu Dudau @ 2023-06-06 14:31 UTC (permalink / raw
  To: Arınç ÜNAL
  Cc: Rob Herring, Krzysztof Kozlowski, Thomas Bogendoerfer,
	Paul Burton, Sergio Paracuellos, Conor Dooley, linux-mips,
	linux-kernel, devicetree

On Tue, Jun 06, 2023 at 08:24:48AM +0300, Arınç ÜNAL wrote:
> On 6.06.2023 00:01, Liviu Dudau wrote:
> > On Mon, Jun 05, 2023 at 07:35:44PM +0300, Arınç ÜNAL wrote:
> > > On 5.06.2023 18:01, Liviu Dudau wrote:
> > > > This WiFi AP is based on a MT7621 SoC with 128MiB RAM, 128MiB NAND,
> > > > a MT7603 2.4GHz WiFi and a MT7613 5GHz WiFi chips integrated on the board,
> > > > connected to the main SoC over PCIe.
> > > > 
> > > > The device uses NMBM over NAND, which is not currently supported in the
> > > > mainline, so NAND node is skipped in this revision.
> > > > 
> > > > Signed-off-by: Liviu Dudau <liviu@dudau.co.uk>
> > > > ---
> > > >    arch/mips/boot/dts/ralink/Makefile            |  3 +-
> > > >    .../dts/ralink/mt7621-tplink-hc220-g5-v1.dts  | 92 +++++++++++++++++++
> > > >    2 files changed, 94 insertions(+), 1 deletion(-)
> > > >    create mode 100644 arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts
> > > > 
> > > > diff --git a/arch/mips/boot/dts/ralink/Makefile b/arch/mips/boot/dts/ralink/Makefile
> > > > index 11732b8c8163a..d27d7e8c700fe 100644
> > > > --- a/arch/mips/boot/dts/ralink/Makefile
> > > > +++ b/arch/mips/boot/dts/ralink/Makefile
> > > > @@ -8,6 +8,7 @@ dtb-$(CONFIG_DTB_VOCORE2)	+= vocore2.dtb
> > > >    dtb-$(CONFIG_SOC_MT7621) += \
> > > >    	mt7621-gnubee-gb-pc1.dtb \
> > > > -	mt7621-gnubee-gb-pc2.dtb
> > > > +	mt7621-gnubee-gb-pc2.dtb \
> > > > +	mt7621-tplink-hc220-g5-v1.dtb
> > > >    obj-$(CONFIG_BUILTIN_DTB)	+= $(addsuffix .o, $(dtb-y))
> > > > diff --git a/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts
> > > > new file mode 100644
> > > > index 0000000000000..859aaa1c1bc2b
> > > > --- /dev/null
> > > > +++ b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts
> > > > @@ -0,0 +1,92 @@
> > > > +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +/dts-v1/;
> > > > +
> > > > +#include "mt7621.dtsi"
> > > > +
> > > > +#include <dt-bindings/gpio/gpio.h>
> > > > +#include <dt-bindings/input/input.h>
> > > > +#include <dt-bindings/leds/common.h>
> > > > +
> > > > +/ {
> > > > +	compatible = "tplink,hc220-g5-v1", "mediatek,mt7621-soc";
> > > > +	model = "TP-Link HC220 G5 v1";
> > > > +
> > > > +	memory@0 {
> > > > +		device_type = "memory";
> > > > +		reg = <0x00000000 0x8000000>;
> > > 
> > > Please use 8 digit addressing for the memory start and size offsets:
> > > 
> > > 0x00000000 0x08000000
> > 
> > Will do.
> > 
> > > 
> > > > +	};
> > > > +
> > > > +	chosen {
> > > > +		bootargs = "earlycon console=ttyS0,115200";
> > > > +	};
> > > > +
> > > > +	gpio-keys {
> > > > +		compatible = "gpio-keys";
> > > > +
> > > > +		key-reset {
> > > > +			label = "reset";
> > > > +			gpios = <&gpio 8 GPIO_ACTIVE_LOW>;
> > > > +			linux,code = <KEY_RESTART>;
> > > > +		};
> > > > +
> > > > +		key-wps {
> > > > +			label = "wps";
> > > > +			gpios = <&gpio 16 GPIO_ACTIVE_LOW>;
> > > > +			linux,code = <KEY_WPS_BUTTON>;
> > > > +		};
> > > > +	};
> > > > +
> > > > +	leds {
> > > > +		compatible = "gpio-leds";
> > > > +
> > > > +		red {
> > > 
> > > Usually the led name would point to the component the LED is used for.
> > 
> > These are "generic" LEDs controlled from the userspace. The original firmware
> > uses GREEN for normal operations, RED for faults and BLUE for when WPS is
> > enabled. I'm not sure if there are any standard bindings that I can use here.
> 
> Looking at:
> 
> https://www.kernel.org/doc/html/latest/leds/leds-class.html#led-device-naming
> 
> You could use red:fault, green:power, and blue:wps. For node names,
> led-fault, led-power, and led-wps.

Without making any changes in the device tree, because of the use of 'function' property,
I get this:

# ls -al /sys/class/leds/
drwxr-xr-x    2 root     root             0 Jun  6 14:24 .
drwxr-xr-x   37 root     root             0 Jan  1  1970 ..
lrwxrwxrwx    1 root     root             0 Jun  6 14:24 blue:wps -> ../../devices/platform/leds/leds/blue:wps
lrwxrwxrwx    1 root     root             0 Jun  6 14:24 green:power -> ../../devices/platform/leds/leds/green:power
lrwxrwxrwx    1 root     root             0 Jun  6 14:24 red:fault -> ../../devices/platform/leds/leds/red:fault

May I suggest that I change only the node names and not add a label, keeping the 'function' property instead?

Best regards,
Liviu

> 
> Arınç

-- 
Everyone who uses computers frequently has had, from time to time,
a mad desire to attack the precocious abacus with an axe.
       	   	      	     	  -- John D. Clark, Ignition!

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v3 1/2] mips: dts: ralink: Add support for TP-Link HC220 G5 v1 board
  2023-06-06 14:31         ` Liviu Dudau
@ 2023-06-06 15:22           ` Arınç ÜNAL
  0 siblings, 0 replies; 8+ messages in thread
From: Arınç ÜNAL @ 2023-06-06 15:22 UTC (permalink / raw
  To: Liviu Dudau
  Cc: Rob Herring, Krzysztof Kozlowski, Thomas Bogendoerfer,
	Paul Burton, Sergio Paracuellos, Conor Dooley, linux-mips,
	linux-kernel, devicetree

On 6.06.2023 17:31, Liviu Dudau wrote:
> On Tue, Jun 06, 2023 at 08:24:48AM +0300, Arınç ÜNAL wrote:
>> On 6.06.2023 00:01, Liviu Dudau wrote:
>>> On Mon, Jun 05, 2023 at 07:35:44PM +0300, Arınç ÜNAL wrote:
>>>> On 5.06.2023 18:01, Liviu Dudau wrote:
>>>>> This WiFi AP is based on a MT7621 SoC with 128MiB RAM, 128MiB NAND,
>>>>> a MT7603 2.4GHz WiFi and a MT7613 5GHz WiFi chips integrated on the board,
>>>>> connected to the main SoC over PCIe.
>>>>>
>>>>> The device uses NMBM over NAND, which is not currently supported in the
>>>>> mainline, so NAND node is skipped in this revision.
>>>>>
>>>>> Signed-off-by: Liviu Dudau <liviu@dudau.co.uk>
>>>>> ---
>>>>>     arch/mips/boot/dts/ralink/Makefile            |  3 +-
>>>>>     .../dts/ralink/mt7621-tplink-hc220-g5-v1.dts  | 92 +++++++++++++++++++
>>>>>     2 files changed, 94 insertions(+), 1 deletion(-)
>>>>>     create mode 100644 arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts
>>>>>
>>>>> diff --git a/arch/mips/boot/dts/ralink/Makefile b/arch/mips/boot/dts/ralink/Makefile
>>>>> index 11732b8c8163a..d27d7e8c700fe 100644
>>>>> --- a/arch/mips/boot/dts/ralink/Makefile
>>>>> +++ b/arch/mips/boot/dts/ralink/Makefile
>>>>> @@ -8,6 +8,7 @@ dtb-$(CONFIG_DTB_VOCORE2)	+= vocore2.dtb
>>>>>     dtb-$(CONFIG_SOC_MT7621) += \
>>>>>     	mt7621-gnubee-gb-pc1.dtb \
>>>>> -	mt7621-gnubee-gb-pc2.dtb
>>>>> +	mt7621-gnubee-gb-pc2.dtb \
>>>>> +	mt7621-tplink-hc220-g5-v1.dtb
>>>>>     obj-$(CONFIG_BUILTIN_DTB)	+= $(addsuffix .o, $(dtb-y))
>>>>> diff --git a/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts
>>>>> new file mode 100644
>>>>> index 0000000000000..859aaa1c1bc2b
>>>>> --- /dev/null
>>>>> +++ b/arch/mips/boot/dts/ralink/mt7621-tplink-hc220-g5-v1.dts
>>>>> @@ -0,0 +1,92 @@
>>>>> +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>>>> +/dts-v1/;
>>>>> +
>>>>> +#include "mt7621.dtsi"
>>>>> +
>>>>> +#include <dt-bindings/gpio/gpio.h>
>>>>> +#include <dt-bindings/input/input.h>
>>>>> +#include <dt-bindings/leds/common.h>
>>>>> +
>>>>> +/ {
>>>>> +	compatible = "tplink,hc220-g5-v1", "mediatek,mt7621-soc";
>>>>> +	model = "TP-Link HC220 G5 v1";
>>>>> +
>>>>> +	memory@0 {
>>>>> +		device_type = "memory";
>>>>> +		reg = <0x00000000 0x8000000>;
>>>>
>>>> Please use 8 digit addressing for the memory start and size offsets:
>>>>
>>>> 0x00000000 0x08000000
>>>
>>> Will do.
>>>
>>>>
>>>>> +	};
>>>>> +
>>>>> +	chosen {
>>>>> +		bootargs = "earlycon console=ttyS0,115200";
>>>>> +	};
>>>>> +
>>>>> +	gpio-keys {
>>>>> +		compatible = "gpio-keys";
>>>>> +
>>>>> +		key-reset {
>>>>> +			label = "reset";
>>>>> +			gpios = <&gpio 8 GPIO_ACTIVE_LOW>;
>>>>> +			linux,code = <KEY_RESTART>;
>>>>> +		};
>>>>> +
>>>>> +		key-wps {
>>>>> +			label = "wps";
>>>>> +			gpios = <&gpio 16 GPIO_ACTIVE_LOW>;
>>>>> +			linux,code = <KEY_WPS_BUTTON>;
>>>>> +		};
>>>>> +	};
>>>>> +
>>>>> +	leds {
>>>>> +		compatible = "gpio-leds";
>>>>> +
>>>>> +		red {
>>>>
>>>> Usually the led name would point to the component the LED is used for.
>>>
>>> These are "generic" LEDs controlled from the userspace. The original firmware
>>> uses GREEN for normal operations, RED for faults and BLUE for when WPS is
>>> enabled. I'm not sure if there are any standard bindings that I can use here.
>>
>> Looking at:
>>
>> https://www.kernel.org/doc/html/latest/leds/leds-class.html#led-device-naming
>>
>> You could use red:fault, green:power, and blue:wps. For node names,
>> led-fault, led-power, and led-wps.
> 
> Without making any changes in the device tree, because of the use of 'function' property,
> I get this:
> 
> # ls -al /sys/class/leds/
> drwxr-xr-x    2 root     root             0 Jun  6 14:24 .
> drwxr-xr-x   37 root     root             0 Jan  1  1970 ..
> lrwxrwxrwx    1 root     root             0 Jun  6 14:24 blue:wps -> ../../devices/platform/leds/leds/blue:wps
> lrwxrwxrwx    1 root     root             0 Jun  6 14:24 green:power -> ../../devices/platform/leds/leds/green:power
> lrwxrwxrwx    1 root     root             0 Jun  6 14:24 red:fault -> ../../devices/platform/leds/leds/red:fault
> 
> May I suggest that I change only the node names and not add a label, keeping the 'function' property instead?

It seems to achieve the same result so, sure.

Arınç

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-06-06 15:22 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-05 15:01 [PATCH v3 0/2] mips: dts: ralink: Add support for TP-Link HC220 G5 v1 board Liviu Dudau
2023-06-05 15:01 ` [PATCH v3 1/2] " Liviu Dudau
2023-06-05 16:35   ` Arınç ÜNAL
2023-06-05 21:01     ` Liviu Dudau
2023-06-06  5:24       ` Arınç ÜNAL
2023-06-06 14:31         ` Liviu Dudau
2023-06-06 15:22           ` Arınç ÜNAL
2023-06-05 15:01 ` [PATCH v3 2/2] dt-bindings: mips: Add bindings " Liviu Dudau

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).