Date: Mon, 6 Aug 2007 20:06:55 +0200
From: Pierre Ossman <drzeus-list@drzeus.cx>
To: David Vrabel <david.vrabel@csr.com>
Cc: linux-kernel@vger.kernel.org
Subject: Re: sdio: enhance IO_RW_EXTENDED support
Message-ID: <20070806200655.77c73130@poseidon.drzeus.cx>
In-Reply-To: <46B73F18.5030109@csr.com>
References: <11858961933491-git-send-email-david.vrabel@csr.com>
	<20070804152304.65ed8f1b@poseidon.drzeus.cx>
	<46B6F877.7060504@csr.com>
	<20070806171207.59fafa18@poseidon.drzeus.cx>
	<46B73F18.5030109@csr.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 3028
Lines: 85

On Mon, 06 Aug 2007 16:32:40 +0100
David Vrabel <david.vrabel@csr.com> wrote:

> 
> Drivers may care about the block size though so you can't have it
> changing behind their backs. e.g., they may need to pad data to a
> multiple of the block size.
> 

Well, we have to assume that they aren't just padding to pass the time.

I can see a couple of reasons:

1. They are padding because it made their code easier by allowing just
one transfer.

This is what I believe is the common case, and one that will go away if
we allow the core free access to the block size. All the complexity is
in the core and the drivers don't even have to bother with setting a
block size.

2. They must write an exact number of bytes to the card before it
activates.

As far as the core is concerned, padding and "real" data are the same
thing, so this poses no restriction on how the core can fiddle with
block sizes and transfers.

3. The entire transfer must be in one transaction.

Now this is a problem as the core might prefer to do two transfers
(probably one block and one byte).

As I believe this will be a rare case, we should try to make sure we
can handle this in a way that can keep less fussy transactions simple.
So I propose changing your sdio_set_block_size() API to
sdio_force_block_size().

That way the driver can lock the block size when it has particular
needs, yet keep it under the (assumingly more optimal) control of the
core at other times.

One could have a calling convention such as specifying a block size of
zero to turn off the forced block size.

One could also use this as a less complex way of informing the drivers
of host restrictions. If the block size specified isn't possible, we
can return an error code stating such. Without that, every driver that
needs this would need to duplicate code that computes possible block
size and would make our life a pain when we want to add new host
restrictions.

> > 
> > I have a counter example. I have here a Marvell wifi card which
> > needs a firmware upload. And it seems to be rather picky about
> > parameters during that upload.
> >
> 
> sdio_set_block_size(func, 64); /* ew, this card is fussy */
> 
> download_firmware();
> 
> sdio_set_block_size(func, func->max_blksize); /* Ahhh, back to the
> card's default */
> 
> If a card is fussy about the block size I don't think there's anything
> cleaner than specifying directly in the driver.
> 

Well, the driver has to specify the information somehow. As to how,
there are a number of options. My proposed sdio_force_block_size() will
work here as well.

Rgds
-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  PulseAudio, core developer          http://pulseaudio.org
  rdesktop, core developer          http://www.rdesktop.org
-
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/