Re: [MFD] Patchset *UPDATED*

[ian <spyro@xxxxxxx>]

On Mon, 2008-07-21 at 14:02 +0200, Pierre Ossman wrote:


> > CMD12 has no argument. Why check?

> Because SDIO_LAUNCH_ICBM might take an argument specifying the silo to
> use.

Point made ;-) I hope to the gods ICBMs dont use SD for IO though ;-)

> > How would I do that? the hardware isnt going to just turn round and say
> > 'ow!' when I prod it.

> It should. That's how I figured out all the bits for the sdhci driver.
> There aren't that many attributes defining a response type, so you
> should be able to figure out the different bits.

I have a problem with this approach.

First, reverse engineering working drivers from winCE, whilst admittedly
not the definitive reference, these bits ARE used.

Secondly, whilst things might *appear* to work when they arent set this
way, its impossible to tell wether the bits may affect the hardwares
state machine or error handling in other ways that dont show up
immediately.

I'll go over the bits, but unless I see good reasons not to set them as
I do already, I am reluctant to change them.

> > Who knows? I was hoping to leave that there until I can get round to
> > implementing SDIO on this chipset, just in case. 
> 
> I don't see how SDIO changes anything. It's just interrupt handling.

Not on this chip. It involves a complete extra set of 'nearly the same'
registers that duplicate the main register set. I have yet to work out
why or how it works. It kinda looks like theres two HCs - an SD one and
an SDIO one, that can be switched between at will.

> I don't think it is possible to figure it out by querying the hardware
> (not without a bus sniffer at least). So I think the current code you
> have is as good as it gets.

:-)

> It's hardly a performance critical code path. Having a constant there
> implies that it is somehow independent, which isn't really the case as
> it is the result of the base clock and the maximum divisor.
> 
> How about 24000000 / 512 if you don't feel comfortable using the
> variable?

I think the constant is clearer. I can look at it and go 'oh, thats the
lowest clock speed' and not have to calculate it. If you like, I could
put

mmc->f_min = 46875; /* 24000000 / 512 */

As noted in the comments though, theres some uncertainty as to what the
actual clock speeds are on some devices using this driver.

> > Will see what I can do. ISTR not very many of the definitions are known.
>
> Do what you can. I noticed at least one thing that was fairly obvious
> (the bit for 4-bit mode).

Certainly.

> > I'll see what I can do, although my only machines with this device use
> > MMC for their root filesystem.
> 
> That could be a problem I guess. I don't suppose you could set up a
> minimalistic initrd to test from? Just remember to swap cards before
> testing. :)

I'll see :)

-Ian


-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php