lkml.org 
[lkml]   [2008]   [Oct]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: 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/

\
 
 \ /
  Last update: 2008-10-04 21:53    [W:0.075 / U:0.468 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site