Re: [OpenWrt-Devel] [PATCH] CC: ar71xx: add support for the TP-LINK TL-WR1043ND v3

[Matthias Schiffer]

On 02/07/2016 05:29 AM, Sven Roederer wrote:
> The hardware of the v3 is identical to the v2.
> 
> Based-on-patch-by: nbd (b9d5ee8)
> 
> - Backport to CC
> - add TPL-WR1043v3 to list of supported boards in config-menu
> - was tested for all 3 boards by freifunk-community Berlin
> 
> Signed-off-by: Sven Roederer 

Hi,
see my comments inline.

> ---
>  target/linux/ar71xx/image/Makefile | 22 
> ++
>  .../610-MIPS-ath79-openwrt-machines.patch  |  2 +-
>  2 files changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/target/linux/ar71xx/image/Makefile 
> b/target/linux/ar71xx/image/Makefile
> index 4336697..8805812 100644
> --- a/target/linux/ar71xx/image/Makefile
> +++ b/target/linux/ar71xx/image/Makefile
> @@ -598,6 +598,28 @@ define Device/tl-wr1041n-v2
>  endef
>  TARGET_DEVICES += tl-wr1041n-v2
>  
> +define Device/tl-wr1043nd-v1
> +$(Device/tplink-8m)
> +BOARDNAME := TL-WR1043ND
> +DEVICE_PROFILE := TLWR1043
> +TPLINK_HWID := 0x10430001
> +endef
> +
> +define Device/tl-wr1043nd-v2
> +$(Device/tplink-8mlzma)
> +BOARDNAME := TL-WR1043ND-v2
> +DEVICE_PROFILE := TLWR1043
> +TPLINK_HWID := 0x10430002
> +endef
> +
> +define Device/tl-wr1043nd-v3
> +$(Device/tplink-8mlzma)
> +BOARDNAME := TL-WR1043ND-v2
> +DEVICE_PROFILE := TLWR1043
> +TPLINK_HWID := 0x10430003
> +endef
> +TARGET_DEVICES += tl-wr1043nd-v1 tl-wr1043nd-v2 tl-wr1043nd-v3

This looks wrong. You are adding new-style definitions for all revisions
of the WR1043ND without removing the old-style definitions.

Either remove the old-style definitions, or just add an old-style
definition for the v3, like this:

https://github.com/freifunk-gluon/gluon/blob/master/patches/openwrt/0037-ar71xx-add-support-for-TP-Link-TL-WR1043ND-v3.patch

(on a related note: we have a lot of such backport patches in Gluon, I
should finally get down to submitting them here...)


> +
>  define Device/tl-wdr4900-v2
>  $(Device/tplink-8mlzma)
>  BOARDNAME := TL-WDR4900-v2
> diff --git 
> a/target/linux/ar71xx/patches-3.18/610-MIPS-ath79-openwrt-machines.patch 
> b/target/linux/ar71xx/patches-3.18/610-MIPS-ath79-openwrt-machines.patch
> index 2fa041b..ca1b194 100644
> --- a/target/linux/ar71xx/patches-3.18/610-MIPS-ath79-openwrt-machines.patch
> +++ b/target/linux/ar71xx/patches-3.18/610-MIPS-ath79-openwrt-machines.patch
> @@ -1268,7 +1268,7 @@
>  +select ATH79_DEV_WMAC
>  +
>  +config ATH79_MACH_TL_WR1043ND_V2
> -+bool "TP-LINK TL-WR1043ND v2 support"
> ++bool "TP-LINK TL-WR1043ND v2/v3 support"
>  +select SOC_QCA955X
>  +select ATH79_DEV_ETH
>  +select ATH79_DEV_GPIO_BUTTONS
> 
The Kconfig change is not necessary at all, these strings are never seen
by humans anyways (unless you use kernel_menuconfig or similar).

We could start to care, but this should be done in trunk first, and
there are many many more occurences of multiple models using the same
BOARDNAME (TL-WR741ND is used 7 times for some very different models,
I'm not sure if we want to list them all ;) ).

Thanks,
Matthias






signature.asc
Description: OpenPGP digital signature
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel

Re: [OpenWrt-Devel] [PATCH] CC: ar71xx: add support for the TP-LINK TL-WR1043ND v3

[Sven Roederer]

Hi,

you are right regarding the old-style vs. new-style. I have seen this, but 
forgot about. I'll fix this with a new patch.

The Kconfig-stuff is indeed only cosmetic, but as is will not break anything I 
think it's ok. There might be no general rule if all models should be listed 
or just some examples. It may depend on the number of models for this board 
and personal mood.

I prefer upstreaming working patches, to have others taking advantage of this 
work more easily.

Sven

Am Sunday 07 February 2016, 09:00:19 schrieben Sie:
> 
> This looks wrong. You are adding new-style definitions for all revisions
> of the WR1043ND without removing the old-style definitions.
> 

> 
> The Kconfig change is not necessary at all, these strings are never seen
> by humans anyways (unless you use kernel_menuconfig or similar).
> 


signature.asc
Description: This is a digitally signed message part.
___
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel