[PATCH v2 1/2] ath6kl_sdio: Add reset gpio module parameter for CHIP_PWD_L pin

[Julian Calaby]

Hi Steve,

A few more comments:

On Thu, Nov 19, 2015 at 10:20 AM, Steve deRosier <derosier at gmail.com> wrote:
> Many ath6k chips have a reset pin, usually labeled CHIP_PWD_L. This pin can
> be pulled low by the host processor to hold the wifi chip in reset. By
> holding the chip in reset, the lowest power consumption available can be
> achieved.
>
> This adds a module parameter so the ath6kl_sdio driver can control the
> CHIP_PWD_L pin if the implementer so desires. This code is only available
> if GPIOLIB is configured.
>
> Signed-off-by: Steve deRosier <steve.derosier at lairdtech.com>
> ---
>  drivers/net/wireless/ath/ath6kl/sdio.c | 73 +++++++++++++++++++++++++++++++++-
>  1 file changed, 72 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath6kl/sdio.c b/drivers/net/wireless/ath/ath6kl/sdio.c
> index eab0ab9..c0755eb 100644
> --- a/drivers/net/wireless/ath/ath6kl/sdio.c
> +++ b/drivers/net/wireless/ath/ath6kl/sdio.c
> @@ -23,6 +23,7 @@
>  #include <linux/mmc/sdio_ids.h>
>  #include <linux/mmc/sdio.h>
>  #include <linux/mmc/sd.h>
> +#include <linux/gpio.h>
>  #include "hif.h"
>  #include "hif-ops.h"
>  #include "target.h"
> @@ -67,8 +68,15 @@ struct ath6kl_sdio {
>
>         /* protects access to wr_asyncq */
>         spinlock_t wr_async_lock;
> +

Extra line.

>  };
>
> +static unsigned int reset_pwd_gpio = ARCH_NR_GPIOS;
> +#ifdef CONFIG_GPIOLIB
> +module_param(reset_pwd_gpio, uint, 0644);
> +MODULE_PARM_DESC(reset_pwd_gpio, "WIFI CHIP_PWD reset pin GPIO");
> +#endif
> +
>  #define CMD53_ARG_READ          0
>  #define CMD53_ARG_WRITE         1
>  #define CMD53_ARG_BLOCK_BASIS   1
> @@ -1414,20 +1422,83 @@ static struct sdio_driver ath6kl_sdio_driver = {
>         .drv.pm = ATH6KL_SDIO_PM_OPS,
>  };
>
> +/* Delay to avoid the mmc driver calling the probe on the prior notice of
> + * the chip, which we just killed. If this is missing, it results in a
> + * spurious warning:
> + * "ath6kl_sdio: probe of mmc0:0001:1 failed with error -110"
> + *
> + * Time chosen experimentally, with padding
> + */
> +#define ATH6KL_MMC_PROBE_DELAY 150

IIRC, this should be at the top of the file with the other #defines.

> +
> +static int ath6kl_sdio_init_gpio(void)
> +{
> +       int ret = 0;
> +
> +       if (gpio_is_valid(reset_pwd_gpio)) {
> +               /* Request the reset GPIO, and assert it to make sure we get a
> +                * clean boot in-case we had a floating input or other issue.
> +                */
> +               ret = gpio_request_one(reset_pwd_gpio,
> +                                      GPIOF_OUT_INIT_LOW |
> +                                      GPIOF_EXPORT_DIR_FIXED,
> +                                      "WIFI_RESET");
> +               if (ret) {
> +                       ath6kl_err("Unable to get WIFI power gpio: %d\n", ret);
> +                       return ret;
> +               }
> +
> +               ath6kl_dbg(ATH6KL_DBG_SUSPEND, "Setup wifi gpio #%d\n",
> +                          reset_pwd_gpio);
> +               usleep_range(20, 50); /* Pin must be asserted at least 5 usec */
> +               gpio_set_value(reset_pwd_gpio, 1); /* De-assert the pin */
> +
> +               msleep(ATH6KL_MMC_PROBE_DELAY);
> +       }
> +
> +       return 0;
> +}
> +
> +static void ath6kl_sdio_release_gpio(void)
> +{
> +       if (gpio_is_valid(reset_pwd_gpio)) {
> +               /* Be sure we leave the chip in reset when we unload and also
> +                * release the GPIO
> +                */
> +               gpio_set_value(reset_pwd_gpio, 0);
> +               gpio_free(reset_pwd_gpio);
> +       }
> +}
> +
>  static int __init ath6kl_sdio_init(void)
>  {
>         int ret;
>
> -       ret = sdio_register_driver(&ath6kl_sdio_driver);
> +       ret = ath6kl_sdio_init_gpio();
>         if (ret)
> +               goto err_gpio;
> +
> +       ret = sdio_register_driver(&ath6kl_sdio_driver);
> +       if (ret) {
>                 ath6kl_err("sdio driver registration failed: %d\n", ret);
> +               goto err_register;
> +       }
>
>         return ret;
> +
> +err_register:
> +       ath6kl_sdio_release_gpio();
> +
> +err_gpio:
> +       return ret;
>  }
>
>  static void __exit ath6kl_sdio_exit(void)
>  {
>         sdio_unregister_driver(&ath6kl_sdio_driver);
> +
> +       if (gpio_is_valid(reset_pwd_gpio))

This if statement is unnecessary as you do it again inside
ath6kl_sdio_release_gpio()

Thanks,

-- 
Julian Calaby

Email: julian.calaby at gmail.com
Profile: http://www.google.com/profiles/julian.calaby/