Messages in this thread | | | Date | Sat, 4 Oct 2008 21:51:22 +0200 | From | Pierre Ossman <> | Subject | Re: RFC: Driver for CB710/720 memory card reader (MMC part) - v2 |
| |
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. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |