On Wed, May 21, 2008 at 09:28:31PM +0200, Pierre Ossman wrote:
> On Wed, 21 May 2008 22:47:13 +0400
> Anton Vorontsov <[EMAIL PROTECTED]> wrote:
>
> >
> > Calling get_cd() for every request smells like overhead, especially given
> > that that get_cd() could ask for GPIO status via relatively slow bus (like
> > I2C GPIO expanders). So, polling seems most reasonable solution here, no
> > need to call it very often.
> >
>
> Fair enough. You should probably add a comment about this somewhere so
> that people do not call get_cd() in the core request function and
> similar places. Place it so that both get_cd() and get_ro() are covered
> though, as it should be relevant for both.
I think this is applicable to the .set_ios() too.
[...]
> > + if (host->ops->get_cd) {
> > + int old_cd_status = host->cd_status;
> > +
> > + host->cd_status = !!host->ops->get_cd(host);
> > + if (!(old_cd_status ^ host->cd_status)) {
> > + mmc_bus_put(host);
> > + goto out;
> > + }
> > + }
> > +
>
> This should only be done when there is no bus handler. Since we are
> polling, we might actually miss the user removing and reinserting the
> card. The only way to check for that is to poke the card and see if it
> is still alive. This also means you won't need that state variable.
Yeah, this makes sense. Indeed pretty easy to trigger [if poll interval
increased to 3 seconds, for example].
> Also, that second if clause seems fit for an obfuscation contest. ;)
cd_status was a bitfield, so I thought that bit operations would be
appropriate. :-)
> > p.s. Since mmc_host_ops no longer the same for every instance of
> > mmc_spi, struct mmc_host_ops can't be const and should be allocated
> > dynamically.
>
> This can be solved by allowing get_cd() to return an error that will be
> treated as if get_cd() wasn't defined. -ENODEV seems suitable.
-ENOSYS (not implemented) sounds better for this purpose...
> (get_ro() needs the same treatment, but I haven't gotten around to
> that)
Ok. How about this version?
--
Anton Vorontsov
email: [EMAIL PROTECTED]
irc://irc.freenode.net/bd2
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
