[OpenWrt-Devel] [PATCH 1/6] [kernel] ath9k: enable GPIO access for ar9287 and ar9285

Hartmut Knaack knaack.h at gmx.de
Fri Jan 22 18:54:13 EST 2016


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Michal Cieslakiewicz schrieb am 21.01.2016 um 00:11:
> From: Michal Cieslakiewicz <michal.cieslakiewicz at wp.pl>
> 
> Patch enables GPIO access for wireless chips ar9287 and ar9285.
> Key poller driver is activated when platform buttons are 
> defined. It also makes LEDs connected honour initial
> default_state and allows platform-supplied WLAN LED name.
> 
> Signed-off-by: Michal Cieslakiewicz <michal.cieslakiewicz at wp.pl>
> 

Looking pretty good over all. One problem throughout the whole set
however is, that you should diff from within your openwrt directory.
But the better way would be to use git format-patch.
Also, it might be a good idea to even split up this patch into the
topics that it covers: WLAN LED name and default state, registering
the gpio_chip and GPIO buttons. So that one patch addresses one issue.
Additional thoughts are inline.

> ---
> 
> This patch exports GPIOs connected to wireless chip. In some routers
> (for example Netgear WNR2000v3 which was used by me for development)
> certain buttons and LEDs are not connected to main SoC but to
> wifi chip instead.
> 
> Patch does the following:
> 
> * when CPTCFG_MAC80211_LEDS option is set and there are LEDs and/or 
>   buttons provided by device platform, ath9k module registers GPIO
>   chip, reserves GPIO pins and starts polling on keys using
>   'gpio-keys-polled' driver
> 
> * LED default_state is now honoured
> 
> * platform-supplied name for WLAN LED is supported (instead of
>   'ath9k-phy*' default)
> 
> GPIO chip base is auto-registered (as advised by kernel developers),
> so on my router pin numbers do not directly follow SoC GPIOs (0-19)
> but occupy numbers 53-63.
> 
> Due to nature of key polling, once started, module ath9k is locked
> and cannot be unloaded.
> 
> 
>  package/kernel/mac80211/patches/546-ath9k-gpio-enable.patch |  368 ++++++++++++
>  target/linux/generic/files/include/linux/ath9k_platform.h   |    5 
>  2 files changed, 373 insertions(+)
> 
> 
> diff -pruN a/openwrt/package/kernel/mac80211/patches/546-ath9k-gpio-enable.patch b/openwrt/package/kernel/mac80211/patches/546-ath9k-gpio-enable.patch
> --- a/openwrt/package/kernel/mac80211/patches/546-ath9k-gpio-enable.patch	1970-01-01 01:00:00.000000000 +0100
> +++ b/openwrt/package/kernel/mac80211/patches/546-ath9k-gpio-enable.patch	2016-01-20 19:56:46.681693574 +0100
> @@ -0,0 +1,368 @@
> +diff -pruN a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
> +--- a/drivers/net/wireless/ath/ath9k/ath9k.h	2016-01-20 19:52:13.849779144 +0100
> ++++ b/drivers/net/wireless/ath/ath9k/ath9k.h	2016-01-20 19:52:13.752784871 +0100
> +@@ -24,6 +24,7 @@
> + #include <linux/completion.h>
> + #include <linux/time.h>
> + #include <linux/hw_random.h>
> ++#include <linux/gpio/driver.h>
> + 
> + #include "common.h"
> + #include "debug.h"
> +@@ -817,6 +818,15 @@ void ath_fill_led_pin(struct ath_softc *
> + int ath_create_gpio_led(struct ath_softc *sc, int gpio, const char *name,
> +                         const char *trigger, bool active_low);
> + 
> ++/************************/
> ++/*    GPIO & Buttons    */
> ++/************************/
> ++
> ++void ath9k_register_gpio_chip(struct ath_softc *sc);
> ++void ath9k_unregister_gpio_chip(struct ath_softc *sc);
> ++void ath9k_init_buttons(struct ath_softc *sc);
> ++void ath9k_deinit_buttons(struct ath_softc *sc);
> ++
> + #else
> + static inline void ath_init_leds(struct ath_softc *sc)
> + {
> +@@ -828,6 +838,20 @@ static inline void ath_deinit_leds(struc
> + static inline void ath_fill_led_pin(struct ath_softc *sc)
> + {
> + }
> ++
> ++static inline void ath9k_register_gpio_chip(struct ath_softc *sc)
> ++{
> ++}
> ++static inline void ath9k_unregister_gpio_chip(struct ath_softc *sc)
> ++{
> ++}
> ++
> ++void ath9k_init_buttons(struct ath_softc *sc)
> ++{
> ++}
> ++void ath9k_deinit_buttons(struct ath_softc *sc)
> ++{
> ++}
> + #endif
> + 
> + /************************/
> +@@ -963,6 +987,12 @@ struct ath_led {
> + 	struct led_classdev cdev;
> + };
> + 
> ++struct ath9k_gpio_chip {
> ++	struct ath_softc *sc;
> ++	char label[32];
> ++	struct gpio_chip gchip;
> ++};
> ++
> + struct ath_softc {
> + 	struct ieee80211_hw *hw;
> + 	struct device *dev;
> +@@ -1017,6 +1047,10 @@ struct ath_softc {
> + #ifdef CPTCFG_MAC80211_LEDS
> + 	const char *led_default_trigger;
> + 	struct list_head leds;
> ++
> ++	struct ath9k_gpio_chip *gpiochip;
> ++
> ++	struct platform_device *btnpdev;	/* gpio-keys-polled */
> + #endif
> + 
> + #ifdef CPTCFG_ATH9K_DEBUGFS
> +diff -pruN a/drivers/net/wireless/ath/ath9k/gpio.c b/drivers/net/wireless/ath/ath9k/gpio.c
> +--- a/drivers/net/wireless/ath/ath9k/gpio.c	2016-01-20 19:52:13.859778553 +0100
> ++++ b/drivers/net/wireless/ath/ath9k/gpio.c	2016-01-20 19:52:13.761784340 +0100
> +@@ -22,6 +22,11 @@
> + /********************************/
> + 
> + #ifdef CPTCFG_MAC80211_LEDS
> ++
> ++#include <asm-generic/gpio.h>
> ++#include <linux/platform_device.h>
> ++#include <linux/gpio_keys.h>
> ++
> + static void ath_led_brightness(struct led_classdev *led_cdev,
> + 			       enum led_brightness brightness)
> + {
> +@@ -54,8 +59,15 @@ static int ath_add_led(struct ath_softc
> + 	ath9k_hw_cfg_output(sc->sc_ah, gpio->gpio,
> + 			    AR_GPIO_OUTPUT_MUX_AS_OUTPUT);
> + 
> +-	/* LED off */
> +-	ath9k_hw_set_gpio(sc->sc_ah, gpio->gpio, gpio->active_low);
> ++	/* set default LED state */
> ++	if (gpio->default_state == LEDS_GPIO_DEFSTATE_ON)
> ++		ath9k_hw_set_gpio(sc->sc_ah, gpio->gpio, !gpio->active_low);
> ++	else
> ++		ath9k_hw_set_gpio(sc->sc_ah, gpio->gpio, gpio->active_low);
> ++
> ++	/* if there is GPIO chip configured, reserve LED pin */
> ++	if (sc->gpiochip)
> ++		gpio_request(sc->gpiochip->gchip.base + gpio->gpio, gpio->name);
> + 
> + 	return 0;
> + }
> +@@ -113,6 +125,9 @@ void ath_deinit_leds(struct ath_softc *s
> + 
> + 	while (!list_empty(&sc->leds)) {
> + 		led = list_first_entry(&sc->leds, struct ath_led, list);
> ++		/* if there is GPIO chip configured, free LED pin */
> ++		if (sc->gpiochip)
> ++			gpio_free(sc->gpiochip->gchip.base + led->gpio->gpio);
> + 		list_del(&led->list);
> + 		ath_led_brightness(&led->cdev, LED_OFF);
> + 		led_classdev_unregister(&led->cdev);
> +@@ -135,6 +150,9 @@ void ath_init_leds(struct ath_softc *sc)
> + 	snprintf(led_name, sizeof(led_name), "ath9k-%s",
> + 		 wiphy_name(sc->hw->wiphy));
> + 
> ++	if (pdata && pdata->led_name)
> ++		strncpy(led_name, pdata->led_name, sizeof(led_name));
> ++
> + 	if (ath9k_led_blink)
> + 		trigger = sc->led_default_trigger;
> + 	else
> +@@ -179,6 +197,197 @@ void ath_fill_led_pin(struct ath_softc *
> + 	/* LED off, active low */
> + 	ath9k_hw_set_gpio(ah, ah->led_pin, (ah->config.led_active_high) ? 0 : 1);
> + }
> ++
> ++/************************/
> ++/*    GPIO & Buttons    */
> ++/************************/
> ++
> ++/* gpio_chip handler : set GPIO to input */
> ++static int ath9k_gpio_pin_cfg_input(struct gpio_chip *chip, unsigned offset)
> ++{
> ++	struct ath9k_gpio_chip *gc = container_of(chip, struct ath9k_gpio_chip,
> ++						  gchip);
> ++
> ++	ath9k_hw_cfg_gpio_input(gc->sc->sc_ah, offset);
> ++
> ++	return 0;
> ++}
> ++
> ++/* gpio_chip handler : set GPIO to output */
> ++static int ath9k_gpio_pin_cfg_output(struct gpio_chip *chip, unsigned offset,
> ++				     int value)
> ++{
> ++	struct ath9k_gpio_chip *gc = container_of(chip, struct ath9k_gpio_chip,
> ++						  gchip);
> ++
> ++	ath9k_hw_cfg_output(gc->sc->sc_ah, offset,
> ++			    AR_GPIO_OUTPUT_MUX_AS_OUTPUT);
> ++	ath9k_hw_set_gpio(gc->sc->sc_ah, offset, value);
> ++
> ++	return 0;
> ++}
> ++
> ++/* gpio_chip handler : query GPIO direction (0=out, 1=in) */
> ++static int ath9k_gpio_pin_get_dir(struct gpio_chip *chip, unsigned offset)
> ++{
> ++	struct ath9k_gpio_chip *gc = container_of(chip, struct ath9k_gpio_chip,
> ++						  gchip);
> ++	struct ath_hw *ah = gc->sc->sc_ah;
> ++
> ++	return !((REG_READ(ah, AR_GPIO_OE_OUT) >> ( offset * 2)) & 3);

The whitespace before offset should be removed.

> ++}
> ++
> ++/* gpio_chip handler : get GPIO pin value */
> ++static int ath9k_gpio_pin_get(struct gpio_chip *chip, unsigned offset)
> ++{
> ++	struct ath9k_gpio_chip *gc = container_of(chip, struct ath9k_gpio_chip,
> ++						  gchip);
> ++
> ++	return ath9k_hw_gpio_get(gc->sc->sc_ah, offset);
> ++}
> ++
> ++/* gpio_chip handler : set GPIO pin to value */
> ++static void ath9k_gpio_pin_set(struct gpio_chip *chip, unsigned offset, int value)

The last parameter should also be wrapped to stay within the 80 chars per line
limit.

> ++{
> ++	struct ath9k_gpio_chip *gc = container_of(chip, struct ath9k_gpio_chip,
> ++						  gchip);
> ++
> ++	ath9k_hw_set_gpio(gc->sc->sc_ah, offset, value);
> ++}
> ++
> ++/* register GPIO chip */
> ++void ath9k_register_gpio_chip(struct ath_softc *sc)
> ++{
> ++	struct ath9k_platform_data *pdata = sc->dev->platform_data;
> ++	struct ath9k_gpio_chip *gc;
> ++	u16 ng;
> ++
> ++	if (!pdata)
> ++		return;
> ++
> ++	/* create chip only if there are objects to handle */
> ++	if (pdata->num_btns > 0 || pdata->num_leds > 0) {

What is the benefit of this check? Why not create the gpio_chip just
based on the fact, if a supported AR928x device is present or not?
The support code will be there, already. So, the additional footprint
of the gpio_chip is pretty small.

> ++		/* for now only AR9285 and AR9287 are recognized */
> ++		if (AR_SREV_9287(sc->sc_ah))
> ++			ng = AR9287_NUM_GPIO;
> ++		else if (AR_SREV_9285(sc->sc_ah))
> ++			ng = AR9285_NUM_GPIO;
> ++		else
> ++			return;
> ++
> ++		gc = kzalloc(sizeof(struct ath9k_gpio_chip), GFP_KERNEL);
> ++
> ++		if (!gc)
> ++			return;
> ++
> ++		snprintf(gc->label, sizeof(gc->label), "ath9k-%s",
> ++			 wiphy_name(sc->hw->wiphy));
> ++		gc->gchip.label = gc->label;
> ++		gc->gchip.base = -1;	/* determine base automatically */
> ++		gc->gchip.ngpio = ng;
> ++		gc->gchip.direction_input = ath9k_gpio_pin_cfg_input;
> ++		gc->gchip.direction_output = ath9k_gpio_pin_cfg_output;
> ++		gc->gchip.get_direction = ath9k_gpio_pin_get_dir;
> ++		gc->gchip.get = ath9k_gpio_pin_get;
> ++		gc->gchip.set = ath9k_gpio_pin_set;
> ++		gc->gchip.owner = THIS_MODULE;
> ++
> ++		if (gpiochip_add(&gc->gchip)) {
> ++			kfree(gc);
> ++			return;
> ++		}
> ++		
> ++		sc->gpiochip = gc;
> ++		gc->sc = sc;
> ++	}
> ++}
> ++
> ++/* remove GPIO chip */
> ++void ath9k_unregister_gpio_chip(struct ath_softc *sc)
> ++{
> ++	struct ath9k_gpio_chip *gc = sc->gpiochip;
> ++
> ++	if (!gc)
> ++		return;
> ++
> ++	gpiochip_remove(&gc->gchip);
> ++	kfree(gc);
> ++	sc->gpiochip = NULL;
> ++}
> ++
> ++/* add GPIO buttons */
> ++void ath9k_init_buttons(struct ath_softc *sc)
> ++{
> ++	struct ath9k_platform_data *pdata = sc->dev->platform_data;
> ++	struct platform_device *pdev;
> ++	struct gpio_keys_platform_data gkpdata;
> ++	struct gpio_keys_button *bt;
> ++	int i;
> ++
> ++	if (!sc->gpiochip)
> ++		return;
> ++
> ++	bt = kmemdup(pdata->btns,
> ++	      	     pdata->num_btns * sizeof(struct gpio_keys_button),
> ++		     GFP_KERNEL);
> ++
> ++	if (!bt)
> ++		return;
> ++
> ++	pdev = platform_device_alloc("gpio-keys-polled", PLATFORM_DEVID_AUTO);
> ++
> ++	if (!pdev)
> ++		goto err_bt_free;
> ++
> ++	for (i = 0; i < pdata->num_btns; i++) {
> ++		ath9k_hw_cfg_gpio_input(sc->sc_ah, pdata->btns[i].gpio);
> ++		bt[i].gpio = sc->gpiochip->gchip.base + pdata->btns[i].gpio;
> ++	}
> ++
> ++	memset(&gkpdata, 0, sizeof(struct gpio_keys_platform_data));
> ++	gkpdata.buttons = bt;
> ++	gkpdata.nbuttons = pdata->num_btns;
> ++	gkpdata.poll_interval = pdata->btn_poll_interval;
> ++
> ++	if (platform_device_add_data(pdev, &gkpdata, sizeof(gkpdata)))
> ++		goto err_pdev_put;
> ++
> ++	if (platform_device_add(pdev))
> ++		goto err_pdev_put;
> ++
> ++	sc->btnpdev = pdev;
> ++
> ++	return;
> ++
> ++err_pdev_put:
> ++	platform_device_put(pdev);
> ++
> ++err_bt_free:
> ++	kfree(bt);
> ++}
> ++
> ++/* remove GPIO buttons */
> ++void ath9k_deinit_buttons(struct ath_softc *sc)
> ++{
> ++	struct gpio_keys_platform_data *gkpdata;
> ++	struct gpio_keys_button *bt = NULL;
> ++
> ++	if (!sc->gpiochip || !sc->btnpdev)
> ++		return;
> ++
> ++	gkpdata = sc->btnpdev->dev.platform_data;
> ++	if (gkpdata)
> ++		bt = gkpdata->buttons;
> ++
> ++	platform_device_del(sc->btnpdev);
> ++	platform_device_put(sc->btnpdev);
> ++
> ++	sc->btnpdev = NULL;
> ++
> ++	if (bt)
> ++		kfree(bt);
> ++}
> ++
> + #endif
> + 
> + /*******************/
> +diff -pruN a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
> +--- a/drivers/net/wireless/ath/ath9k/init.c	2016-01-20 19:52:13.840779675 +0100
> ++++ b/drivers/net/wireless/ath/ath9k/init.c	2016-01-20 19:52:13.742785462 +0100
> +@@ -990,7 +990,9 @@ int ath9k_init_device(u16 devid, struct
> + 			goto debug_cleanup;
> + 	}
> + 
> ++	ath9k_register_gpio_chip(sc);
> + 	ath_init_leds(sc);
> ++	ath9k_init_buttons(sc);
> + 	ath_start_rfkill_poll(sc);
> + 
> + 	return 0;
> +@@ -1036,7 +1038,9 @@ void ath9k_deinit_device(struct ath_soft
> + 	ath9k_ps_wakeup(sc);
> + 
> + 	wiphy_rfkill_stop_polling(sc->hw->wiphy);
> ++	ath9k_deinit_buttons(sc);
> + 	ath_deinit_leds(sc);
> ++	ath9k_unregister_gpio_chip(sc);
> + 
> + 	ath9k_ps_restore(sc);
> + 
> +diff -pruN a/include/linux/ath9k_platform.h b/include/linux/ath9k_platform.h
> +--- a/include/linux/ath9k_platform.h	2016-01-20 19:52:13.792782509 +0100
> ++++ b/include/linux/ath9k_platform.h	2016-01-20 19:52:13.689788592 +0100
> +@@ -30,6 +30,7 @@ struct ath9k_platform_data {
> + 	int led_pin;
> + 	u32 gpio_mask;
> + 	u32 gpio_val;
> ++	const char *led_name;
> + 
> + 	bool endian_check;
> + 	bool is_clk_25mhz;
> +@@ -45,6 +46,10 @@ struct ath9k_platform_data {
> + 
> + 	int num_leds;
> + 	const struct gpio_led *leds;
> ++
> ++	unsigned num_btns;
> ++	const struct gpio_keys_button *btns;
> ++	unsigned btn_poll_interval;
> + };
> + 
> + #endif /* _LINUX_ATH9K_PLATFORM_H */
> diff -pruN a/openwrt/target/linux/generic/files/include/linux/ath9k_platform.h b/openwrt/target/linux/generic/files/include/linux/ath9k_platform.h
> --- a/openwrt/target/linux/generic/files/include/linux/ath9k_platform.h	2016-01-20 19:52:13.981771349 +0100
> +++ b/openwrt/target/linux/generic/files/include/linux/ath9k_platform.h	2016-01-20 19:52:13.645791190 +0100
> @@ -30,6 +30,7 @@ struct ath9k_platform_data {
>  	int led_pin;
>  	u32 gpio_mask;
>  	u32 gpio_val;
> +	const char *led_name;
>  
>  	bool endian_check;
>  	bool is_clk_25mhz;
> @@ -45,6 +46,10 @@ struct ath9k_platform_data {
>  
>  	int num_leds;
>  	const struct gpio_led *leds;
> +
> +	unsigned num_btns;
> +	const struct gpio_keys_button *btns;
> +	unsigned btn_poll_interval;
>  };
>  
>  #endif /* _LINUX_ATH9K_PLATFORM_H */
> 
> 
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJWosEbAAoJEANoShj6yJFIduUP/32pf/p/G93RGBe6JfXYj1LY
X52LZZBL+M1IZ6DT/6G3Mt2oXQBC/v9Q1srs3A07gIDSf73401ZLDu0fBMXQYB02
OCM7ysGsQSqArflRbt/qV2TdhhhMo0LTwgNG7tB99kqRFEVqMvblxPiqIF6ilP4l
yh/OpeADKqGxE9UtezLvqNKFcPVJ9srrMddvEbSXQo1rfufyArxOnj63cgh0ye2l
ndbWE4n1x7rIxEIHti2uw33nz/AXGQk0XlRbQNCL2QYzN8oPwiBHPI4efFxC1dkO
EK5aa8oj0gOG76qUtCJ0tlsBoVG2uvR2M+hW/W5kLRsGU9TnzIE2QNYcKHH5VaGV
l9f1fufqzmhgMp9n0VF1GLSVKlCNNBi9BMwFw/xQ1aGkD6c7lkWTDou6cKWaD6AK
5iK0mAQ4W1KOT3UgGldNMszY7MwkKxaWy6Wxt6eNiHwP29dS7pw1RnR+RuAeqO1k
kG2ArRry9VSbtm0CqtrQuupipkbkdDRHxjWMOTCN5yrpyrA1lv4soUFYDCO1zxnH
u+LBERq4kCgIFTXA1BSs07VLa30LWR5xsn4ojzKQm5re7V4dUf7tAAI1SpuNa1Bi
Oru8JMTa25/X/uRE6GG86ZwgcNSjAmU4cbdXOIgoTcTbsq+JG1Klh0V+/6ydRA2k
mXpltHoQcwE/eyjfvDV7
=ESBW
-----END PGP SIGNATURE-----
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0xFAC89148.asc
Type: application/pgp-keys
Size: 3104 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/openwrt-devel/attachments/20160123/113e9930/attachment.bin>
-------------- next part --------------
_______________________________________________
openwrt-devel mailing list
openwrt-devel at lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel


More information about the openwrt-devel mailing list