All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Vrabel <david.vrabel@csr.com>
To: Pierre Ossman <drzeus-list@drzeus.cx>
Cc: linux-kernel@vger.kernel.org, David Vrabel <david.vrabel@csr.com>
Subject: Re: [patch 3/4] sdio: extend sdio_readsb() and friends to handle any length of buffer
Date: Wed, 08 Aug 2007 14:22:20 +0100	[thread overview]
Message-ID: <46B9C38C.1030708@csr.com> (raw)
In-Reply-To: <20070808123759.26a74b2a@poseidon.drzeus.cx>

Pierre Ossman wrote:
> On Wed, 08 Aug 2007 11:19:33 +0100
> David Vrabel <david.vrabel@csr.com> wrote:
> 
>> We need to know the block size in use /before/ the start of the
>> transfer as we would like drivers to be able to perform transfers
>> with single commands as this can result in significantly better
>> performance[1].  The drivers for our (CSR's) WiFi chips should do
>> this.  This isn't some (as you suggested in a previous post) 'rare'
>> requirement.
>>
> 
> Well, there are more ways that can be achieved.
> 
> First, the driver could lock down the block size using
> sdio_force_block_size(). Then it knows what it gets.
> 
> Second, we could try to make it possible for the driver to indicate
> "feel free to pad this transfer". Then we could also remove the need
> for drivers to mess with buffers and keep such stuff in the core. We
> could even magically remove a memcpy() by setting up two sg entries,
> one for the data and one for the padding.

Setting the block size in io_rw_ext_helper() has several drawbacks, namely:

1. Reduces the flexibility of drivers to manage what commands are performed.
2. A performance penalty on the first transfer.
3. Greater code complexity.
4. Non-intuitive location for card initialization code.

Sure we could just through hoops and add (much) extra complexity to the
core, to improve item 1 but 2, 3 and 4 are insoluble. Given that setting
block size before the first command has /zero/ benefits[1], why bother?

Your insistence on this stupid idea baffles me, particularly in the
light of your other useful comments.

I would also like to advise that until a larger number of function
drivers become available that the core is kept as simple and as flexible
as possible.  Without knowing how different cards operate it is
difficult to know what's common behaviour and what's card specific.

My latest (and hopefully final) patch set follows.

David

[1] dynamic block sizing was (potentially) a useful benefit but I have
comprehensibly shown it's not beneficial.  The idea that is somehow
necessary to permit the core to change it's block size selection
algorithm is bogus.
-- 
David Vrabel, Software Engineer, Drivers group  Tel: +44 (0)1223 692562
CSR plc, Churchill House, Cambridge Business Park, Cowley Road, CB4 0WZ


.

  reply	other threads:[~2007-08-08 13:23 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-07-31 15:36 sdio: enhance IO_RW_EXTENDED support David Vrabel
2007-07-31 15:36 ` sdio: parameterize SDIO FBR register defines David Vrabel
2007-08-04 13:26   ` Pierre Ossman
2007-08-06 10:14     ` David Vrabel
2007-08-06 14:58       ` Pierre Ossman
2007-07-31 15:36 ` sdio: set the functions' block size David Vrabel
2007-08-04 13:30   ` Pierre Ossman
2007-08-06 10:04     ` David Vrabel
2007-07-31 15:36 ` sdio: extend sdio_readsb() and friends to handle any length of buffer David Vrabel
2007-08-04 13:35   ` Pierre Ossman
2007-08-04 13:23 ` sdio: enhance IO_RW_EXTENDED support Pierre Ossman
2007-08-06 10:31   ` David Vrabel
2007-08-06 15:12     ` Pierre Ossman
2007-08-06 15:32       ` David Vrabel
2007-08-06 18:06         ` Pierre Ossman
2007-08-06 20:01         ` Pierre Ossman
2007-08-07 12:51           ` David Vrabel
2007-08-07 12:53             ` [patch 1/4] sdio: parameterize SDIO FBR register defines David Vrabel
2007-08-07 12:54             ` [patch 2/4] sdio: set the functions' block size David Vrabel
2007-08-07 13:38               ` Pierre Ossman
2007-08-07 17:20                 ` David Vrabel
2007-08-07 17:54                   ` Pierre Ossman
2007-08-08  9:46                     ` David Vrabel
2007-08-08 10:06                       ` Pierre Ossman
2007-08-08 10:19                         ` David Vrabel
2007-08-07 12:55             ` [patch 3/4] sdio: extend sdio_readsb() and friends to handle any length of buffer David Vrabel
2007-08-07 13:42               ` Pierre Ossman
2007-08-07 20:17               ` Pierre Ossman
2007-08-08 10:19                 ` David Vrabel
2007-08-08 10:37                   ` Pierre Ossman
2007-08-08 13:22                     ` David Vrabel [this message]
2007-08-08 13:23                       ` [patch 1/3] sdio: add SDIO_FBR_BASE(f) macro David Vrabel
2007-08-08 13:23                       ` [patch 2/3] sdio: set the functions' block size David Vrabel
2007-08-08 13:24                       ` [patch 3/3] sdio: extend sdio_readsb() and friends to handle any length of buffer David Vrabel
2007-08-08 16:55                       ` [patch 3/4] " Pierre Ossman
2007-08-07 12:55             ` [patch 4/4] sdio: disable CD resistor David Vrabel
2007-08-07 13:43               ` Pierre Ossman
2007-08-07 14:46                 ` David Vrabel
2007-08-07 15:08                   ` Pierre Ossman
2007-08-07 13:33             ` sdio: enhance IO_RW_EXTENDED support Pierre Ossman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=46B9C38C.1030708@csr.com \
    --to=david.vrabel@csr.com \
    --cc=drzeus-list@drzeus.cx \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.