Hi Daniel, > -----Original Message----- > From: openwrt-devel [mailto:[email protected]] > On Behalf Of Daniel Golle > Sent: Montag, 4. Januar 2021 20:19 > To: Paul Fertser <[email protected]> > Cc: Gary Cooper <[email protected]>; [email protected] > 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 <[email protected]> > > > > 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@0 {
flash@0
[...]
> + SBL1@0 {
This should be partition@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
openpgp-digital-signature.asc
Description: PGP signature
_______________________________________________ openwrt-devel mailing list [email protected] https://lists.openwrt.org/mailman/listinfo/openwrt-devel
