Re: [PATCH] ricoh_mmc: Use suspend/resume_noirq (v2) (resend)

[Philip Langdale]

On Thu, 8 Jan 2009 23:14:52 +0100, "Rafael J. Wysocki" <rjw@xxxxxxx> wrote:
> On Wednesday 07 January 2009, philipl@xxxxxxxxx wrote:
>> If ricoh_mmc suspends before sdhci_pci, it will pull the card
>> out from under the controller, which could leave the system in
>> a very confused state.
> 
> Does it really happen?  That depends on which of them is registered
first.

Yes, it does. It seems that due to the typical ordering of the PCI devices,
ricoh_mmc loads first.

The larger context is how the hardware works and what ricoh_mmc does.

The hardware is a multiple function PCI device with a separate function for
each type of media card. SD and MMC are special because they use the same
physical format, and consequently the same physical contacts. Inside the
chip, a basic detection routine runs on insertion, and the card then
appears
to the appropriate controller function.

Now, the ricoh chip implements the standard SD host controller spec but
as no such thing exists for MMC, it implements its own custom one. Linux
only has an SDHCI driver, but due to how the hardware works, we can support
MMC cards through it - but only if the controller can see them!

For that to happen, we must disable the MMC function - when that happens,
the card is routed to the SD function and an insertion event is generated.
Similarly, if you reactive the MMC function, the card will be seen to
disapper on the SD side.

The ricoh_mmc driver automates this enable/disable process. It is bound to
the MMC function but actually acts on another function of the Ricoh chip
(typically the firewire controller) to do the actual disabling.

For consistency, the driver reactivates the mmc function at suspend time
and deactivates it again on resume.

Now, if the ricoh_mmc driver suspends before the SD driver, it will
reactive
the MMC function and trigger this removal event on the SD side. Then, when
the SD driver does its suspend/resume cycle, the card will be unexpectedly
absent, and confusion follows.

If ricoh_mmc uses suspend/resume_noirq, it both ensures that it runs at
the right time and that the insertion/removal events are not reported.

>> Using suspend/resume_noirq ensures that sdhci_pci suspends first
>> and resumes second.
> 
> Well, I'm not sure if this is the best approach, but I don't know what
the
> dependencies between the devices are, so can you please explain that
> to me?

I hope my write up answers this question.
 
> That said, if you want to suspend-resume ricoh_mmc with interrupts
> disabled,
> please use the legacy PCI .suspend_late() and .resume_early() callbacks
for
> that, since in the new framework the core will carry out some standard PM
> operations in addition to your .suspend_noirq() and .resume_noirq().  It
> may
> not be what you want in this case, though.

I'd need to know what those other operations are. I originally wrote this
patch with suspend_late and resume_early but the first pm_ops patch I saw
removed those. Are you now saying that both suspend_late/resume_early and
noirq callbacks are supported now.

It's all a bit confusing :-)

--phil
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/