[OpenWrt-Devel] [PATCH] ar71xx: fix LED GPIO definitions for NETGEAR WNR2200

[John Crispin]

hi,

some more comments inline

On 13/03/2015 14:51, Markus Kohls wrote:
> Signed-off-by: Markus Kohls <markus at kola.li>
> 
> Now with corrected whitespaces and signed 
> 
> ---
>  .../ar71xx/files/arch/mips/ath79/mach-wnr2200.c    | 77 ++++++++++------------
>  1 file changed, 35 insertions(+), 42 deletions(-)
> 
> diff --git a/target/linux/ar71xx/files/arch/mips/ath79/mach-wnr2200.c b/target/linux/ar71xx/files/arch/mips/ath79/mach-wnr2200.c
> index bf7f9ee..085a01e 100644
> --- a/target/linux/ar71xx/files/arch/mips/ath79/mach-wnr2200.c
> +++ b/target/linux/ar71xx/files/arch/mips/ath79/mach-wnr2200.c
> @@ -14,7 +14,9 @@
>  #include <linux/mtd/partitions.h>
>  
>  #include <asm/mach-ath79/ath79.h>
> +#include <asm/mach-ath79/ar71xx_regs.h>
>  
> +#include "common.h"
>  #include "dev-ap9x-pci.h"
>  #include "dev-eth.h"
>  #include "dev-gpio-buttons.h"
> @@ -23,24 +25,26 @@
>  #include "dev-usb.h"
>  #include "machtypes.h"
>  
> -#define WNR2200_GPIO_LED_LAN2_AMBER	0
> -#define WNR2200_GPIO_LED_LAN4_AMBER	1
> -#define WNR2200_GPIO_LED_WPS		5
> -#define WNR2200_GPIO_LED_WAN_GREEN	7
> +// AR9287 GPIO LED

// is a c++ style comment. please use /* ... */ syntax

> +#define WNR2200_GPIO_LED_WLAN		0
> +#define WNR2200_GPIO_LED_PWR_AMBER	1
> +#define WNR2200_GPIO_LED_PWR_GREEN	2
> +#define WNR2200_GPIO_USB_5V		4
> +
> +// AR9287 GPIO BUTTON
> +#define WNR2200_GPIO_BUTTON_WIFI	3
> +#define WNR2200_GPIO_BUTTON_WPS 	5
> +#define WNR2200_GPIO_BUTTON_RESET	6
> +
> +// AR7241
> +#define WNR2200_GPIO_LED_WPS		7
>  #define WNR2200_GPIO_LED_USB		8
> +
> +#define WNR2200_GPIO_LED_LAN1_AMBER	6
> +#define WNR2200_GPIO_LED_LAN2_AMBER	0
>  #define WNR2200_GPIO_LED_LAN3_AMBER	11
> +#define WNR2200_GPIO_LED_LAN4_AMBER	1
>  #define WNR2200_GPIO_LED_WAN_AMBER	12
> -#define WNR2200_GPIO_LED_LAN1_GREEN	13
> -#define WNR2200_GPIO_LED_LAN2_GREEN	14
> -#define WNR2200_GPIO_LED_LAN3_GREEN	15
> -#define WNR2200_GPIO_LED_LAN4_GREEN	16
> -#define WNR2200_GPIO_LED_PWR_AMBER	21
> -#define WNR2200_GPIO_LED_PWR_GREEN	22
> -
> -#define WNR2200_GPIO_USB_POWER		24
> -
> -#define WNR2200_KEYS_POLL_INTERVAL	20 /* msecs */
> -#define WNR2200_KEYS_DEBOUNCE_INTERVAL	(3 * WNR2200_KEYS_POLL_INTERVAL)
>  
>  #define WNR2200_MAC0_OFFSET		0
>  #define WNR2200_MAC1_OFFSET		6
> @@ -48,10 +52,18 @@
>  
>  static struct gpio_led wnr2200_leds_gpio[] __initdata = {
>  	{
> +		.name		= "netgear:amber:lan1",
> +		.gpio		= WNR2200_GPIO_LED_LAN1_AMBER,
> +		.active_low	= 1,
> +	}, {
>  		.name		= "netgear:amber:lan2",
>  		.gpio		= WNR2200_GPIO_LED_LAN2_AMBER,
>  		.active_low	= 1,
>  	}, {
> +		.name		= "netgear:amber:lan3",
> +		.gpio		= WNR2200_GPIO_LED_LAN3_AMBER,
> +		.active_low	= 1,
> +	}, {
>  		.name		= "netgear:amber:lan4",
>  		.gpio		= WNR2200_GPIO_LED_LAN4_AMBER,
>  		.active_low	= 1,
> @@ -60,38 +72,14 @@ static struct gpio_led wnr2200_leds_gpio[] __initdata = {
>  		.gpio		= WNR2200_GPIO_LED_WPS,
>  		.active_low	= 1,
>  	}, {
> -		.name		= "netgear:green:wan",
> -		.gpio		= WNR2200_GPIO_LED_WAN_GREEN,
> -		.active_low	= 1,
> -	}, {
>  		.name		= "netgear:green:usb",
>  		.gpio		= WNR2200_GPIO_LED_USB,
>  		.active_low	= 1,
>  	}, {
> -		.name		= "netgear:amber:lan3",
> -		.gpio		= WNR2200_GPIO_LED_LAN3_AMBER,
> -		.active_low	= 1,
> -	}, {
>  		.name		= "netgear:amber:wan",
>  		.gpio		= WNR2200_GPIO_LED_WAN_AMBER,
>  		.active_low	= 1,
>  	}, {
> -		.name		= "netgear:green:lan1",
> -		.gpio		= WNR2200_GPIO_LED_LAN1_GREEN,
> -		.active_low	= 1,
> -	}, {
> -		.name		= "netgear:green:lan2",
> -		.gpio		= WNR2200_GPIO_LED_LAN2_GREEN,
> -		.active_low	= 1,
> -	}, {
> -		.name		= "netgear:green:lan3",
> -		.gpio		= WNR2200_GPIO_LED_LAN3_GREEN,
> -		.active_low	= 1,
> -	}, {
> -		.name		= "netgear:green:lan4",
> -		.gpio		= WNR2200_GPIO_LED_LAN4_GREEN,
> -		.active_low	= 1,
> -	}, {
>  		.name		= "netgear:amber:power",
>  		.gpio		= WNR2200_GPIO_LED_PWR_AMBER,
>  		.active_low	= 1,
> @@ -106,6 +94,8 @@ static void __init wnr2200_setup(void)
>  {
>  	u8 *art = (u8 *) KSEG1ADDR(0x1fff0000);
>  
> +	ath79_gpio_function_setup(AR724X_GPIO_FUNC_JTAG_DISABLE, 0);
> +
>  	ath79_register_mdio(0, 0x0);
>  
>  	ath79_init_mac(ath79_eth0_data.mac_addr, art+WNR2200_MAC0_OFFSET, 0);
> @@ -123,13 +113,16 @@ static void __init wnr2200_setup(void)
>  	ath79_register_m25p80(NULL);
>  	ap91_pci_init(art + WNR2200_PCIE_CALDATA_OFFSET, NULL);
>  
> +	ap9x_pci_setup_wmac_led_pin(0, WNR2200_GPIO_LED_WLAN);
> +
>  	ath79_register_leds_gpio(-1, ARRAY_SIZE(wnr2200_leds_gpio),
>  					wnr2200_leds_gpio);
>  
>  	/* enable power for the USB port */
> -	gpio_request_one(WNR2200_GPIO_USB_POWER,
> -			GPIOF_OUT_INIT_HIGH | GPIOF_EXPORT_DIR_FIXED,
> -			"USB power");

why is this not needed anymore ? and you should also remove the comment
above it

	John

> +	ap9x_pci_setup_wmac_gpio(0,
> +		BIT(WNR2200_GPIO_USB_5V) |
> +		BIT(WNR2200_GPIO_LED_PWR_AMBER) | BIT(WNR2200_GPIO_LED_PWR_GREEN),
> +		BIT(WNR2200_GPIO_USB_5V) | BIT(WNR2200_GPIO_LED_PWR_AMBER));
>  
>  	ath79_register_usb();
>  }
> 
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel