On Oct 21, 2010, at 9:57 AM, Nicolas Pitre wrote:

> On Thu, 21 Oct 2010, Philip Rakity wrote:
> 
>> 
>> Sometimes it is useful for the SD driver to know the type of card that is in 
>> use so that dynamic sd bus clocking can be enabled or disabled.
>> Dynamic clocks are a feature of sd 3.0 and we can support this in our v2.0 
>> controller.
>> 
>> The problem is that some SDIO cards are not happy when the clocks are 
>> dynamically stopped.  I have not seen any issues with SD/mmc/eMMC cards.  I 
>> suspect the reason for the SDIO problems is that the cards need the clock 
>> since SDIO cards can interrupt the CPU for service and the other card types 
>> cannot.
>> 
>> The information about the type of card being used is known by the mmc layer 
>> when 
>> 
>> mmc_alloc_card(struct mmc_host *host, struct device_type *type) --- 
>> core/bus.c
>> 
>> is called but the sd driver cannot get to this information when set_ios() is 
>> called by the mmc layer since the host->card field is filled in too late.
>> 
>> I have added host->card = card to
>> 
>> struct mmc_card *mmc_alloc_card(struct mmc_host *host, struct device_type 
>> *type)
>> {
>>      struct mmc_card *card;
>> 
>>      card = kzalloc(sizeof(struct mmc_card), GFP_KERNEL);
>>      if (!card)
>>              return ERR_PTR(-ENOMEM);
>> 
>>      card->host = host;
>> 
>>      device_initialize(&card->dev);
>> 
>>      card->dev.parent = mmc_classdev(host);
>>      card->dev.bus = &mmc_bus_type;
>>      card->dev.release = mmc_release_card;
>>      card->dev.type = type;
>> +     host->card = card;
>>      return card;
>> }
>> 
>> and this works for me provided the sd driver remembers to check for card 
>> being NULL  in the driver specific set_clock code.
>> 
>> My pseudo code is 
>> 
>> if (host->mmc->card == NULL)
>>     dynamic_clocks=FALSE;
>> else if host->mmc->card == MMC_TYPE_SDIO)
>>      dynamic_clocks = TRUE;
>> else 
>>      dynamic_clocks = FALSE;
>> 
>> 
>> My question is:  Is there a better way to pass the type of card to the sd 
>> driver?
> 
> I don't like this.  Again we should strive to make the controller driver 
> as simple as possible and let the core code handle this kind of clocking 
> logic as much as possible.  The OMAP driver is again a terribly bad 
> example to follow.

Not looking at OMAP

> 
> If there are cases where the clock still has to remain active with some 
> cards, it is far better to take care of such exceptions in only one 
> place instead of having to add a card quirk in every controller drivers.  
> The controller driver should remain card agnostic as much as possible 
> and not take decisions on its own based on card type.

Was not going to add a quirk.  The low level platform specific code for
sdhci.c can handle this if it knows the card type.  It can program the
appropriate hardware registers.

> 
> Stopping the clock would also be a good thing even for those controllers 
> without dynamic clock support. The core could certainly stop the clock 
> manually after a period of inactivity for example.


agree except that stopping the clock for SDIO cards is not a good thing to do 
in our experience.
if the core can signal this via set_ios this works for me.  This is why I 
suggested the code telling
set_ios the card type which allowed the platform specific code to enable or not 
dynamic 
clocks.

> 
> So if your controller hardware has the ability to dynamically stop the 
> clock on its own, then the driver should simply expose that knowledge to 
> the core with a capability bit, and the core should tell the driver to 
> enable that mode through mmc_set_ios() when applicable.

sd 3.0 spec (not marvell specific) suppports dynamic clocking as well as 
marvell v2.0
controller.

I can add a CAP bit -- but for SDIO cards we will certainly set this to not 
disable the clocks.

100,000 foot summary
a) add new mmc->caps bit indicating dynamic clocks can be supported in the 
controller
b) add new field in ios that indicates if clocks should be dynamic
c) pass this to set_ios
d) add platform specific hook to set_ios to control the dynamic clocking option.

Is my understanding correct ?



> 
> 
> Nicolas

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

Reply via email to