Au1xxx MMC driver

Au1xxx MMC driver

[Jordan Crouse]

(Sucked off of gmane):

>> 1. au1xxx mmc driver
>> 
>>    mmc_remove_host() does a safe shutdown of the MMC host, removing
>>    cards and then powering down.  This must be called prior to the
>>    driver thinking of tearing anything down.
>> 
>>    As for those disable_irq()...enable_irq(), are you aware that MMC
>>    can start talking to the host as soon as you've called mmc_add_host() ?
>> 

> I'm also concerned about the ammount of protocol awareness in this
> driver. Is there a spec available for this hardware? Perhaps the MMC
> layer can export more information so that we can avoid switches on
> specific MMC commands?

Spec is here: 

http://tinyurl.com/dslkv  (Horribly long URL and registration required,
unfortunately).

In this case, the controller needs to be specifically told what command
and response type it should expect, thus the opcode switch.
I don't really think this is an  unreasonable demand to be put on the 
hardware driver, and its certainly way more HW specific then the upper 
layers need to be.

As for Russell's comments - yes, the mm_remove_host() should be called
first, thats my ugly.  As for the second one - we haven't had problems
so far with enabling the interrupt after the mmc_add_host() command -
but I can see how its entirely possible thats just a result of luck, and
not skill.   I'll look in to ways to fix that up. 

Jordan
-- 
Jordan Crouse
Senior Linux Engineer
AMD - Personal Connectivity Solutions Group
<www.amd.com/embeddedprocessors>

Re: Au1xxx MMC driver

[Pierre Ossman]

Jordan Crouse wrote:
>
>> I'm also concerned about the ammount of protocol awareness in this
>> driver. Is there a spec available for this hardware? Perhaps the MMC
>> layer can export more information so that we can avoid switches on
>> specific MMC commands?
>>     
>
> Spec is here: 
>
> http://tinyurl.com/dslkv  (Horribly long URL and registration required,
> unfortunately).
>   

Very nice. First manufacturer I've seen that gives out a spec. without 
requiring your soul. :)

> In this case, the controller needs to be specifically told what command
> and response type it should expect, thus the opcode switch.
> I don't really think this is an  unreasonable demand to be put on the 
> hardware driver, and its certainly way more HW specific then the upper 
> layers need to be.
>
>   

I've read the spec. and the command response type seems to be the only 
thing required. So protocol awareness is acceptable there provided there 
is a failure case for any unknown type.

For the command type field the driver should check the number of blocks 
and direction, not the command code. SDIO seems more difficult. But 
since we do not support that atm we still have the option of making sure 
any future implementations consider the needs of this hardware. I doubt 
there will be a need for checking command codes here either.

In summary, I see no need for protocol awareness other than response types.

Rgds
Pierre

Re: Au1xxx MMC driver

[Pierre Ossman]

Just a quick extra comment. It would be nice if you could use the host 
controller name when doing the initial printk:

-         printk(KERN_INFO DRIVER_NAME ": MMC Controller %d set up at 
%8.8X (mode=%s)\n",
-                host->id, host->iobase, dma ? "dma" : "pio");
+         printk(KERN_INFO "%s: MMC Controller %d set up at %8.8X 
(mode=%s)\n",
+                mmc_hostname(mmc), host->id, host->iobase, dma ? "dma" 
: "pio");

It gives a nice, visible connection between the host name and the hardware.

Rgds
Pierre