[PATCH 0/2] net: wireless: ti: wl1251: sdio: remove ti,power-gpio

[PATCH 0/2] net: wireless: ti: wl1251: sdio: remove ti,power-gpio

[H. Nikolaus Schaller]

The driver has been updated to use the mmc/sdio core
which does full power control. So we do no longer need
the power control gipo.

Note that it is still needed for the SPI based interface
(N900).

Suggested by: Ulf Hansson <ulf.hansson@linaro.org>
Tested by: H. Nikolaus Schaller <hns@goldelico.com> # OpenPandora 600MHz

H. Nikolaus Schaller (2):
  DTS: bindings: wl1251: mark ti,power-gpio as optional
  net: wireless: ti: wl1251: sdio: remove ti,power-gpio

 .../bindings/net/wireless/ti,wl1251.txt       |  3 +-
 drivers/net/wireless/ti/wl1251/sdio.c         | 30 -------------------
 2 files changed, 2 insertions(+), 31 deletions(-)

-- 
2.23.0

[PATCH 1/2] DTS: bindings: wl1251: mark ti,power-gpio as optional

[H. Nikolaus Schaller]

It is now only useful for SPI interface.
Power control of SDIO mode is done through mmc core.

Suggested by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt b/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt
index f38950560982..88fd28d15eac 100644
--- a/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt
+++ b/Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt
@@ -9,11 +9,12 @@ Required properties:
 - spi-max-frequency : Maximum SPI clocking speed of device in Hz
 - interrupts :        Should contain interrupt line
 - vio-supply :        phandle to regulator providing VIO
-- ti,power-gpio :     GPIO connected to chip's PMEN pin
 
 Optional properties:
 - ti,wl1251-has-eeprom : boolean, the wl1251 has an eeprom connected, which
                          provides configuration data (calibration, MAC, ...)
+- ti,power-gpio :	 GPIO connected to chip's PMEN pin if operated in
+			 SPI mode
 - Please consult Documentation/devicetree/bindings/spi/spi-bus.txt
   for optional SPI connection related properties,
 
-- 
2.23.0

Re: [PATCH 1/2] DTS: bindings: wl1251: mark ti,power-gpio as optional

[Rob Herring]

On Sun, 24 Nov 2019 11:35:45 +0100, "H. Nikolaus Schaller" wrote:
> It is now only useful for SPI interface.
> Power control of SDIO mode is done through mmc core.
> 
> Suggested by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
>  Documentation/devicetree/bindings/net/wireless/ti,wl1251.txt | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

[PATCH 2/2] net: wireless: ti: wl1251: sdio: remove ti,power-gpio

[H. Nikolaus Schaller]

Remove handling of this property from code.
Note that wl->power_gpio is still needed in
the header file for SPI mode (N900).

Suggested by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
---
 drivers/net/wireless/ti/wl1251/sdio.c | 30 ---------------------------
 1 file changed, 30 deletions(-)

diff --git a/drivers/net/wireless/ti/wl1251/sdio.c b/drivers/net/wireless/ti/wl1251/sdio.c
index df0c20a555e3..82446196f9a8 100644
--- a/drivers/net/wireless/ti/wl1251/sdio.c
+++ b/drivers/net/wireless/ti/wl1251/sdio.c
@@ -15,9 +15,7 @@
 #include <linux/wl12xx.h>
 #include <linux/irq.h>
 #include <linux/pm_runtime.h>
-#include <linux/gpio.h>
 #include <linux/of.h>
-#include <linux/of_gpio.h>
 #include <linux/of_irq.h>
 
 #include "wl1251.h"
@@ -162,15 +160,6 @@ static int wl1251_sdio_set_power(struct wl1251 *wl, bool enable)
 printk("%s %d\n", __func__, enable);
 
 	if (enable) {
-		/*
-		 * Power is controlled by runtime PM, but we still call board
-		 * callback in case it wants to do any additional setup,
-		 * for example enabling clock buffer for the module.
-		 */
-		if (gpio_is_valid(wl->power_gpio))
-			gpio_set_value(wl->power_gpio, true);
-
-
 		ret = pm_runtime_get_sync(&func->dev);
 		if (ret < 0) {
 			pm_runtime_put_sync(&func->dev);
@@ -188,9 +177,6 @@ printk("%s %d\n", __func__, enable);
 		ret = pm_runtime_put_sync(&func->dev);
 		if (ret < 0)
 			goto out;
-
-		if (gpio_is_valid(wl->power_gpio))
-			gpio_set_value(wl->power_gpio, false);
 	}
 
 out:
@@ -245,27 +231,11 @@ printk("%s: of=%pOFcC\n", __func__, np);
 
 	wl1251_board_data = wl1251_get_platform_data();
 	if (!IS_ERR(wl1251_board_data)) {
-		wl->power_gpio = wl1251_board_data->power_gpio;
 		wl->irq = wl1251_board_data->irq;
 		wl->use_eeprom = wl1251_board_data->use_eeprom;
 	} else if (np) {
 		wl->use_eeprom =of_property_read_bool(np, "ti,wl1251-has-eeprom");
-		wl->power_gpio = of_get_named_gpio(np, "ti,power-gpio", 0);
 		wl->irq = of_irq_get(np, 0);
-
-		if (wl->power_gpio == -EPROBE_DEFER || wl->irq == -EPROBE_DEFER) {
-			ret = -EPROBE_DEFER;
-			goto disable;
-		}
-	}
-
-	if (gpio_is_valid(wl->power_gpio)) {
-		ret = devm_gpio_request(&func->dev, wl->power_gpio,
-								"wl1251 power");
-		if (ret) {
-			wl1251_error("Failed to request gpio: %d\n", ret);
-			goto disable;
-		}
 	}
 
 	if (wl->irq) {
-- 
2.23.0

Re: [PATCH 2/2] net: wireless: ti: wl1251: sdio: remove ti,power-gpio

[H. Nikolaus Schaller]

> Am 24.11.2019 um 11:35 schrieb H. Nikolaus Schaller <hns@goldelico.com>:
> 
> Remove handling of this property from code.
> Note that wl->power_gpio is still needed in
> the header file for SPI mode (N900).
> 
> Suggested by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
> drivers/net/wireless/ti/wl1251/sdio.c | 30 ---------------------------
> 1 file changed, 30 deletions(-)
> 
> diff --git a/drivers/net/wireless/ti/wl1251/sdio.c b/drivers/net/wireless/ti/wl1251/sdio.c
> index df0c20a555e3..82446196f9a8 100644
> --- a/drivers/net/wireless/ti/wl1251/sdio.c
> +++ b/drivers/net/wireless/ti/wl1251/sdio.c
> @@ -15,9 +15,7 @@
> #include <linux/wl12xx.h>
> #include <linux/irq.h>
> #include <linux/pm_runtime.h>
> -#include <linux/gpio.h>
> #include <linux/of.h>
> -#include <linux/of_gpio.h>
> #include <linux/of_irq.h>
> 
> #include "wl1251.h"
> @@ -162,15 +160,6 @@ static int wl1251_sdio_set_power(struct wl1251 *wl, bool enable)
> printk("%s %d\n", __func__, enable);
> 
> 	if (enable) {
> -		/*
> -		 * Power is controlled by runtime PM, but we still call board
> -		 * callback in case it wants to do any additional setup,
> -		 * for example enabling clock buffer for the module.
> -		 */
> -		if (gpio_is_valid(wl->power_gpio))
> -			gpio_set_value(wl->power_gpio, true);
> -
> -
> 		ret = pm_runtime_get_sync(&func->dev);
> 		if (ret < 0) {
> 			pm_runtime_put_sync(&func->dev);
> @@ -188,9 +177,6 @@ printk("%s %d\n", __func__, enable);
> 		ret = pm_runtime_put_sync(&func->dev);
> 		if (ret < 0)
> 			goto out;
> -
> -		if (gpio_is_valid(wl->power_gpio))
> -			gpio_set_value(wl->power_gpio, false);
> 	}
> 
> out:
> @@ -245,27 +231,11 @@ printk("%s: of=%pOFcC\n", __func__, np);
> 
> 	wl1251_board_data = wl1251_get_platform_data();
> 	if (!IS_ERR(wl1251_board_data)) {
> -		wl->power_gpio = wl1251_board_data->power_gpio;
> 		wl->irq = wl1251_board_data->irq;
> 		wl->use_eeprom = wl1251_board_data->use_eeprom;
> 	} else if (np) {
> 		wl->use_eeprom =of_property_read_bool(np, "ti,wl1251-has-eeprom");
> -		wl->power_gpio = of_get_named_gpio(np, "ti,power-gpio", 0);
> 		wl->irq = of_irq_get(np, 0);
> -
> -		if (wl->power_gpio == -EPROBE_DEFER || wl->irq == -EPROBE_DEFER) {

^^^ spotted a bug myself... wl->irq check must not be removed.

Noted for v2.


> -			ret = -EPROBE_DEFER;
> -			goto disable;
> -		}
> -	}
> -
> -	if (gpio_is_valid(wl->power_gpio)) {
> -		ret = devm_gpio_request(&func->dev, wl->power_gpio,
> -								"wl1251 power");
> -		if (ret) {
> -			wl1251_error("Failed to request gpio: %d\n", ret);
> -			goto disable;
> -		}
> 	}
> 
> 	if (wl->irq) {
> -- 
> 2.23.0
> 

Re: [PATCH 2/2] net: wireless: ti: wl1251: sdio: remove ti,power-gpio

[Kalle Valo]

"H. Nikolaus Schaller" <hns@goldelico.com> writes:

> Remove handling of this property from code.
> Note that wl->power_gpio is still needed in
> the header file for SPI mode (N900).
>
> Suggested by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
> ---
>  drivers/net/wireless/ti/wl1251/sdio.c | 30 ---------------------------
>  1 file changed, 30 deletions(-)

Please use "wl1251: " as title prefix, no need to have the full
directory structure there.

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH 2/2] net: wireless: ti: wl1251: sdio: remove ti,power-gpio

[H. Nikolaus Schaller]

> Am 26.11.2019 um 16:53 schrieb Kalle Valo <kvalo@codeaurora.org>:
> 
> "H. Nikolaus Schaller" <hns@goldelico.com> writes:
> 
>> Remove handling of this property from code.
>> Note that wl->power_gpio is still needed in
>> the header file for SPI mode (N900).
>> 
>> Suggested by: Ulf Hansson <ulf.hansson@linaro.org>
>> Signed-off-by: H. Nikolaus Schaller <hns@goldelico.com>
>> ---
>> drivers/net/wireless/ti/wl1251/sdio.c | 30 ---------------------------
>> 1 file changed, 30 deletions(-)
> 
> Please use "wl1251: " as title prefix, no need to have the full
> directory structure there.

Ok, noted for v2.

BR and thanks,
Nikolaus

Re: [PATCH 0/2] net: wireless: ti: wl1251: sdio: remove ti,power-gpio

[Kalle Valo]

"H. Nikolaus Schaller" <hns@goldelico.com> writes:

> The driver has been updated to use the mmc/sdio core
> which does full power control. So we do no longer need
> the power control gipo.
>
> Note that it is still needed for the SPI based interface
> (N900).
>
> Suggested by: Ulf Hansson <ulf.hansson@linaro.org>
> Tested by: H. Nikolaus Schaller <hns@goldelico.com> # OpenPandora 600MHz
>
> H. Nikolaus Schaller (2):
>   DTS: bindings: wl1251: mark ti,power-gpio as optional
>   net: wireless: ti: wl1251: sdio: remove ti,power-gpio
>
>  .../bindings/net/wireless/ti,wl1251.txt       |  3 +-
>  drivers/net/wireless/ti/wl1251/sdio.c         | 30 -------------------
>  2 files changed, 2 insertions(+), 31 deletions(-)

Via which tree are these planned to go? Please always document that in
the cover letter so that maintainers don't need to guess.

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [PATCH 0/2] net: wireless: ti: wl1251: sdio: remove ti,power-gpio

[H. Nikolaus Schaller]

> Am 26.11.2019 um 16:51 schrieb Kalle Valo <kvalo@codeaurora.org>:
> 
> "H. Nikolaus Schaller" <hns@goldelico.com> writes:
> 
>> The driver has been updated to use the mmc/sdio core
>> which does full power control. So we do no longer need
>> the power control gipo.
>> 
>> Note that it is still needed for the SPI based interface
>> (N900).
>> 
>> Suggested by: Ulf Hansson <ulf.hansson@linaro.org>
>> Tested by: H. Nikolaus Schaller <hns@goldelico.com> # OpenPandora 600MHz
>> 
>> H. Nikolaus Schaller (2):
>>  DTS: bindings: wl1251: mark ti,power-gpio as optional
>>  net: wireless: ti: wl1251: sdio: remove ti,power-gpio
>> 
>> .../bindings/net/wireless/ti,wl1251.txt       |  3 +-
>> drivers/net/wireless/ti/wl1251/sdio.c         | 30 -------------------
>> 2 files changed, 2 insertions(+), 31 deletions(-)
> 
> Via which tree are these planned to go? Please always document that in
> the cover letter so that maintainers don't need to guess.

Well, how should I know that better than maintainers?

I don't know who manages which trees and who feels responsible.
So I have to guess even more.

I just use the get_maintainer.pl script to address everybody listed by it,
assuming that it does the right thing.

If the script doesn't do a good enough job it should be improved.

BR and thanks,
Nikolaus

Re: [Letux-kernel] [PATCH 0/2] net: wireless: ti: wl1251: sdio: remove ti, power-gpio

[Andreas Kemnade]

On Tue, 26 Nov 2019 15:51:28 +0000
Kalle Valo <kvalo@codeaurora.org> wrote:

> "H. Nikolaus Schaller" <hns@goldelico.com> writes:
> 
> > The driver has been updated to use the mmc/sdio core
> > which does full power control. So we do no longer need
> > the power control gipo.
> >
> > Note that it is still needed for the SPI based interface
> > (N900).
> >
> > Suggested by: Ulf Hansson <ulf.hansson@linaro.org>
> > Tested by: H. Nikolaus Schaller <hns@goldelico.com> # OpenPandora 600MHz
> >
> > H. Nikolaus Schaller (2):
> >   DTS: bindings: wl1251: mark ti,power-gpio as optional
> >   net: wireless: ti: wl1251: sdio: remove ti,power-gpio
> >
> >  .../bindings/net/wireless/ti,wl1251.txt       |  3 +-
> >  drivers/net/wireless/ti/wl1251/sdio.c         | 30 -------------------
> >  2 files changed, 2 insertions(+), 31 deletions(-)  
> 
> Via which tree are these planned to go? Please always document that in
> the cover letter so that maintainers don't need to guess.
> 
Hmm, this is about bisect issues and the former misc pandora
fix/cleanup series now in linux-next that make you doubt it should go
via wireless/net?
So the question is whether it depends on that?

Regards,
Andreas