Re: RFC: Driver for CB710/720 memory card reader (MMC part) - v2

[Pierre Ossman <drzeus-mmc@drzeus.cx>]

On Thu, 25 Sep 2008 08:29:24 +0200
Michał Mirosław <mirq-linux@rere.qmqm.pl> wrote:

> 
> This should be almost ready. mmc-test passes up to a point where it
> tries non-sector-size writes to the card - I didn't have time to look
> into this further, yet.
> 

If you can't figure it out, just make sure the driver fails the
requests with -EINVAL.

> > > +/* sg-to-PIO buffer */
> [and its API]
> > Why this complex system? Can't you use the handlers the kernel already
> > provides? You also get a lot of special handling with those, e.g.
> > highmem.
> 
> I looked at linux/lib/scatterlist.c and found nothing really useful. If
> there were some preconditions always met for scatterlists - like that block
> does not span multiple pages, or at least 16-byte block don't, then most
> of this code can go away. It looks complicated, because it implements
> a special iterator interface that always returns multiple-of-16-byte blocks
> for reading or writing. This makes the PIO loops very simple and fast
> (the bounce_buffer case was never triggered by mmc-block in my tests).
> 

The buffers leave no alignment guarantees unfortunately. 

Have a look at the current sdhci.c PIO routines though. It uses the sg
iterator helpers and keeps track of four byte chunks (as opposed to the
16 byte ones you need).

> > > +static void cb710_mmc_enable_irq(struct cb710_chip *chip, int enable)
> > > +{
> > > +	unsigned long flags;
> > > +
> > > +	spin_lock_irqsave(&chip->irq_lock, flags);
> > > +	__cb710_mmc_enable_irq(chip, enable);
> > > +	spin_unlock_irqrestore(&chip->irq_lock, flags);
> > > +}
> > This is a fairly useless wrapper. Look over how it is called instead.
> 
> Can you expand on this? Right now irq handler doesn't call
> __cb710_mmc_enable_irq() but modifies registers itself. This will
> change eventually.
> 

Just require that the lock must be held by the caller when invoking
cb710_mmc_enable_irq(). It's usually easier to keep track of locks by
keeping the lock handling at entry points into the driver.

Rgds
-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  rdesktop, core developer          http://www.rdesktop.org

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.