Subject: [PATCH 11/15] wireless: wl1271: introduce platform device support

From: [email-masked] (Adrian Hunter)
Date: Wed, 07 Jul 2010 22:59:14 +0300

Nicolas Pitre wrote:
> On Wed, 7 Jul 2010, Roger Quadros wrote:
> 
>> On 07/06/2010 10:51 PM, Hunter Adrian (Nokia-MS/Helsinki) wrote:
>>> For eMMC in omap_hsmmc, this is all done via claim_host / release_host
>>> which call ->enable() / ->disable() methods.  omap_hsmmc makes use of
>>> mmc_power_restore_host() which calls host->bus_ops->power_restore()
>>> which is not implemented for SDIO, but for MMC and SD it reinitializes
>>> the card.
> 
> This is IMHO a really bad design.  The power control decision has to 
> come from the top, not from the bottom.  And certainly not with a 
> U-turn dependency the omap_hsmmc is using.

The power control decision does come from the top via mmc_claim_host /
mmc_release_host.

> 
> I regret to say this, but the omap_hsmmc driver is becoming a total 
> mess.  The host controller driver has to be a dumb interface serving 
> requests from the hardware used by the upper layer stack, not the place 
> where decisions such as power handling should be made.  Think of it like 
> an ethernet driver.  No ethernet driver in Linux is telling the IP stack 
> when to shut down.

The omap_hsmmc driver does not tell the core to shut down the upper layers.
Instead the core tells the omap_hsmmc driver that it is "disabled" i.e.
not currently being used so it can start taking steps to save power.
That is sensible because not even the upper layers know when the next
activity will be.

> 
>> Shouldn't the power control intelligence (i.e. when to turn power ON/OFF) lie
>> with the bus drivers?
> 
> Absolutely!  And in the SDIO case that should lie with each function 
> drivers.  Please let's stop this omap_hsmmc madness.

You are dealing with a trivial case - turn off the power when not in use.
We have 3 power saving actions: enable DPS, put the card to sleep,
and finally power it off.  Each increases the latency to start up
again and so must be staggered.  With DPS-enabled the host controller can
be powered off at any time.  In that case the controller registers must be
restored.  There are numerous entry points to the driver.  Checking whether
to restore registers at every entry point is error prone and messy.
Instead we require that the core tells the driver when to enable and
disable.

> 
> 
> Nicolas
> --
> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>