Re: [PATCH 2/4] wl1251: move power GPIO handling into the driver

2013-11-05 Thread Pavel Machek
On Sun 2013-10-27 17:14:27, Sebastian Reichel wrote:
 Move the power GPIO handling from the board code into
 the driver. This is a dependency for device tree support.
 
 Signed-off-by: Sebastian Reichel s...@debian.org

Reviewed-by: Pavel Machek pa...@ucw.cz

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/4] wl1251: move power GPIO handling into the driver

2013-10-28 Thread Grazvydas Ignotas
On Sun, Oct 27, 2013 at 10:12 PM, Sebastian Reichel s...@debian.org wrote:
 On Sun, Oct 27, 2013 at 08:24:16PM +0400, Alexander Shiyan wrote:
  Move the power GPIO handling from the board code into
  the driver. This is a dependency for device tree support.
 
  Signed-off-by: Sebastian Reichel s...@debian.org
  ---
   arch/arm/mach-omap2/board-omap3pandora.c |  2 ++
   arch/arm/mach-omap2/board-rx51-peripherals.c | 11 ++
   drivers/net/wireless/ti/wl1251/sdio.c| 21 +-
   drivers/net/wireless/ti/wl1251/spi.c | 33 
  ++--
   drivers/net/wireless/ti/wl1251/wl1251.h  |  2 +-
   include/linux/wl12xx.h   |  2 +-
   6 files changed, 43 insertions(+), 28 deletions(-)
 ...
  diff --git a/include/linux/wl12xx.h b/include/linux/wl12xx.h
  index b516b4f..a9c723b 100644
  --- a/include/linux/wl12xx.h
  +++ b/include/linux/wl12xx.h
  @@ -49,7 +49,7 @@ enum {
   };
 
   struct wl1251_platform_data {
  -   void (*set_power)(bool enable);
  +   int power_gpio;
  /* SDIO only: IRQ number if WLAN_IRQ line is used, 0 for SDIO IRQs */
  int irq;
  bool use_eeprom;
  --

 What a reason for not using regulator API here with GPIO-based
 regulator?

 I think this pin is not used as power supply, but like an enable pin
 for low power states. Of course the regulator API could still be
 (mis?)used for this, but I think it would be the first linux device
 driver doing this.

 Note: I don't have wl1251 documentation.

When wl12xx family of chips is connected through SDIO, we already have
that pin set up as a regulator controlled with the help of mmc
subsystem. When time comes to communicate with the chip, mmc subsystem
sees this as yet another SD card and looks for associated regulator
for it, and the board file has that set up as a fixed regulator
controlling that pin (see pandora_vmmc3 in
arch/arm/mach-omap2/board-omap3pandora.c). To prevent poweroff after
first SDIO communications are over, pm_runtime calls are used in
drivers/net/wireless/ti/wl1251/sdio.c .

I don't know if something similar can be done done in SPI case, but
I'm sure this is not the first your-so-called regulator misuse.

GraÅžvydas
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/4] wl1251: move power GPIO handling into the driver

2013-10-28 Thread Mark Brown
On Mon, Oct 28, 2013 at 07:29:52PM +0200, Grazvydas Ignotas wrote:

 When wl12xx family of chips is connected through SDIO, we already have
 that pin set up as a regulator controlled with the help of mmc
 subsystem. When time comes to communicate with the chip, mmc subsystem
 sees this as yet another SD card and looks for associated regulator
 for it, and the board file has that set up as a fixed regulator
 controlling that pin (see pandora_vmmc3 in
 arch/arm/mach-omap2/board-omap3pandora.c). To prevent poweroff after
 first SDIO communications are over, pm_runtime calls are used in
 drivers/net/wireless/ti/wl1251/sdio.c .

Is this actually controlling VMMC though, or is it some other control?
If it's not controlling VMMC then it shouldn't say that it is.

 I don't know if something similar can be done done in SPI case, but
 I'm sure this is not the first your-so-called regulator misuse.

It's not the first but that doesn't make controlling something other
than a regulator through the regulator API any less broken.


signature.asc
Description: Digital signature


Re: [PATCH 2/4] wl1251: move power GPIO handling into the driver

2013-10-28 Thread Sebastian Reichel
Hi,

On Mon, Oct 28, 2013 at 12:23:54PM -0700, Mark Brown wrote:
 On Mon, Oct 28, 2013 at 07:29:52PM +0200, Grazvydas Ignotas wrote:
 
  When wl12xx family of chips is connected through SDIO, we already have
  that pin set up as a regulator controlled with the help of mmc
  subsystem. When time comes to communicate with the chip, mmc subsystem
  sees this as yet another SD card and looks for associated regulator
  for it, and the board file has that set up as a fixed regulator
  controlling that pin (see pandora_vmmc3 in
  arch/arm/mach-omap2/board-omap3pandora.c). To prevent poweroff after
  first SDIO communications are over, pm_runtime calls are used in
  drivers/net/wireless/ti/wl1251/sdio.c .
 
 Is this actually controlling VMMC though, or is it some other control?
 If it's not controlling VMMC then it shouldn't say that it is.
 
  I don't know if something similar can be done done in SPI case, but
  I'm sure this is not the first your-so-called regulator misuse.
 
 It's not the first but that doesn't make controlling something other
 than a regulator through the regulator API any less broken.

I gave it a second try to find out details for this pin:

1. The pin is named PMEN in the Nokia N900 schematics
2. PMEN is described as Power management enable - system shutdown
   in a crippled datasheet of the wl1253, which I found in the internet.

I don't think this is supposed to be handled by the regulator API.

-- Sebastian


signature.asc
Description: Digital signature


Re: [PATCH 2/4] wl1251: move power GPIO handling into the driver

2013-10-28 Thread Mark Brown
On Tue, Oct 29, 2013 at 12:26:26AM +0100, Sebastian Reichel wrote:

 1. The pin is named PMEN in the Nokia N900 schematics
 2. PMEN is described as Power management enable - system shutdown
in a crippled datasheet of the wl1253, which I found in the internet.

 I don't think this is supposed to be handled by the regulator API.

I agree, this sounds like a GPIO that the driver should be understanding
directly to me.


signature.asc
Description: Digital signature


Re: [PATCH 2/4] wl1251: move power GPIO handling into the driver

2013-10-27 Thread Sebastian Reichel
On Sun, Oct 27, 2013 at 08:24:16PM +0400, Alexander Shiyan wrote:
  Move the power GPIO handling from the board code into
  the driver. This is a dependency for device tree support.
  
  Signed-off-by: Sebastian Reichel s...@debian.org
  ---
   arch/arm/mach-omap2/board-omap3pandora.c |  2 ++
   arch/arm/mach-omap2/board-rx51-peripherals.c | 11 ++
   drivers/net/wireless/ti/wl1251/sdio.c| 21 +-
   drivers/net/wireless/ti/wl1251/spi.c | 33 
  ++--
   drivers/net/wireless/ti/wl1251/wl1251.h  |  2 +-
   include/linux/wl12xx.h   |  2 +-
   6 files changed, 43 insertions(+), 28 deletions(-)
 ...
  diff --git a/include/linux/wl12xx.h b/include/linux/wl12xx.h
  index b516b4f..a9c723b 100644
  --- a/include/linux/wl12xx.h
  +++ b/include/linux/wl12xx.h
  @@ -49,7 +49,7 @@ enum {
   };
   
   struct wl1251_platform_data {
  -   void (*set_power)(bool enable);
  +   int power_gpio;
  /* SDIO only: IRQ number if WLAN_IRQ line is used, 0 for SDIO IRQs */
  int irq;
  bool use_eeprom;
  -- 
 
 What a reason for not using regulator API here with GPIO-based
 regulator?

I think this pin is not used as power supply, but like an enable pin
for low power states. Of course the regulator API could still be
(mis?)used for this, but I think it would be the first linux device
driver doing this.

Note: I don't have wl1251 documentation.

-- Sebastian


signature.asc
Description: Digital signature