Re: [PATCH 0/1] MMC: SDIO driver for Intel Moorestown platform

[Pierre Ossman <pierre@ossman.eu>]

[-- Attachment #1: Type: text/plain, Size: 2757 bytes --]

On Mon, 15 Jun 2009 20:05:14 +0800
"Li, Jiebing" <jiebing.li@intel.com> wrote:

> 
> Hi Pierre and all,
> 
> Here's a SDIO driver code patch for Intel Moorestown platform. Per your response of my code submission last month, I modified the code and hope that you could spare time to review this patch, which is used for SDIO card power management. It's very important for MID products like Moorestown, as power saving is the key point for such embedded products.
> 

First off, you need to split up your changes into several patches, one
for each feature. I refuse to review a patch that contains several
changes like that. Look at other commits in the kernel tree and how
they separate each functional change into its own patch.

> Last time you asked me the use cases for the code. The following is my statement:
> 
> 1. sdio_bus_suspend()/sdui_bus_resume() and mmc_sdio_suspend()/mmc_sdio_resume():
> Currently SDIO bus driver doesn't have suspend/resume interface, We need this to suspend the functions one by one and then the whole card when the platform is put in S3/S4/S5 states.
> 

This should happen anyway as the host controller's suspend functions
are called when there is a system wide PM state change. As the MMC bus
powers down, the SDIO devices will get removed. What extra
functionality is it you need in this area?

> 2. sdio_external_suspend_device()/sdio_external_resume_device():
> Also, similar to USB, we would like to be able to selectively suspend a particular function using sysfs interface for power management scheme (non-ACPI) if the function is not in use. 

Like Matthew pointed out, this is not desirable. The drivers should be
intelligent enough to determine when to enter low power states by
themselves.

I think you need to have a high-level discussion with Matthew and
Rafael first on how we want the PM to be performed before I can approve
any such changes.

> 3. sdio_reset_device():
> It comes to solve a scenario where there is a hang in the Host Interface HW, (i.e. the driver cannot access the device).  In this case the function driver can detect that something is broken (using some sw watchdog for example), but it cannot reset its function (host interface is broken). 
> 

I assume you're talking about the SDIO card here, not the host
controller as the host controller driver should fairly easily detect
when it has locked up.

The idea is sensible, but I'll have to see a proper patch before I can
make any comments on your approach.

Rgds
-- 
     -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]