[PATCH v2 1/2] mac80211: add 802.11ad-support

[Adrian Schmutzler]

Hi Daniel,

> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-bounces at lists.openwrt.org]
> On Behalf Of Daniel Golle
> Sent: Montag, 4. Januar 2021 20:19
> To: Paul Fertser <fercerpav at gmail.com>
> Cc: Gary Cooper <gaco at bitmessage.de>; openwrt-devel at lists.openwrt.org
> Subject: Re: [PATCH v2 1/2] mac80211: add 802.11ad-support
> 
> Hi Paul,
> Hi Gary,
> 
> On Mon, Jan 04, 2021 at 08:27:49PM +0300, Paul Fertser wrote:
> > From: Gary Cooper <gaco at bitmessage.de>
> >
> > This adds some logic to properly populate defaults in
> > /etc/config/wireless and support for WPA*-GCMP. TP-Link AD7200,
> > Mikrotik WAP60g, LHGG-60ad, etc
> 
> I've split this thing up a bit more and merged it with what lynxis already had in
> his staging tree.
> Plus missing PKG_RELEASE bumps added where needed.
> 
> Please review and test the 80211ad branch of my staging tree:
> https://git.openwrt.org/?p=openwrt/staging/dangole.git;a=shortlog;h=refs/
> heads/80211ad

A few remarks on the device-support part:

> +tplink,ad7200)
> +	ucidef_set_led_usbport "usb1" "USB 1" "blue:usb_1" "usb1-port1" "usb2-port1"
> +	ucidef_set_led_usbport "usb2" "USB 2" "blue:usb_3" "usb3-port1" "usb4-port1"
> +	ucidef_set_led_switch "wan" "wan" "blue:wan" "switch0" "0x02"
> +	ucidef_set_led_switch "lan" "lan" "blue:lan" "switch0" "0x3c"
> +	ucidef_set_led_wlan "wlan2g" "wlan2g" "blue:wlan2g" "phy2tpt"
> +	ucidef_set_led_wlan "wlan5g" "wlan5g" "blue:wlan5g" "phy1tpt"
> +	ucidef_set_led_netdev "wlan60g" "wlan60g" "blue:wlan60g" "wlan0"
> +	;;

I'm not sure what's the state with ipq806x, but probably one can move the USB and/or Wifi triggers to DTS.
If possible, the wlan60g trigger should be converted to phy0something, as that will make it independent of the interface name.

Apart from that, I only found caldata extraction for two of three radios. Is that correct or a mistake?

> +	model = "TP-Link Talon AD7200";
> +	compatible = "tplink,ad7200", "qcom,ipq8064";

I'd decide for one name (either with or without "Talon"), but not mix them. If "Talon" is an alternate name ("nickname"), it should be put behind the proper name in brackets ("TP-Link AD7200 (Talon)").

> +	keys {
[...]
> +		ledgeneral {
> +			label = "ledswitch";

One should decide for one name in both label and node name. I'd recommend "led-enable", as "ledswitch" had me think of a networking switch LED until I read the linux,code property.

> +			gpios = <&qcom_pinmux 53 GPIO_ACTIVE_LOW>;
> +			linux,code = <KEY_LIGHTS_TOGGLE>;
> +		};
> +	};

> +	leds {
[...]
> +		usb1 {
> +			label = "blue:usb_1";

I'd remove the underscore here, same for usb_3 directly following here.

Apart from that, one could sort all those LEDs by $something

[...]
> +		m25p80 at 0 {

flash at 0

[...]
> +			SBL1 at 0 {

This should be partition at 0 and so on for the following partitions.

> +define Device/tplink_ad7200
> +	$(call Device/TpSafeImage)
> +	DEVICE_VENDOR := TP-Link
> +	DEVICE_MODEL := Talon AD7200

With or without Talon, see above.

> +	DEVICE_VARIANT := v1

Either we only have v1 and can drop this line, or the device should be named tplink,ad7200-v1.

> +	SOC := qcom-ipq8064
> +	BLOCKSIZE := 128k
> +	PAGESIZE := 2048
> +	BOARD_NAME := ad7200

This can be dropped.

> +	SUPPORTED_DEVICES += ad7200

This can be dropped.

Thanks for finally taking care of the 11ad support.

Best

Adrian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: openpgp-digital-signature.asc
Type: application/pgp-signature
Size: 834 bytes
Desc: not available
URL: <http://lists.openwrt.org/pipermail/openwrt-devel/attachments/20210104/00c01b6f/attachment.sig>