PATCH: Config option to pad SDIO sizes

[Pierre Ossman]

On Fri, 4 Jul 2008 14:20:27 +0200
"Johan Adolfsson" <Johan.Adolfsson at axis.com> wrote:

> 
> 
> > -----Original Message-----
> > From: Pierre Ossman [mailto:drzeus-mmc at drzeus.cx] 
> > Sent: den 1 juli 2008 13:46
> > To: Johan Adolfsson
> > Cc: 'Dan Williams'; 'Johan Adolfsson'; 
> > libertas-dev at lists.infradead.org
> > Subject: Re: PATCH: Config option to pad SDIO sizes
> > 
> > Please do some testing though, as I'd like to have this included in
> > 2.6.27.
> 
> Seems to work, but I managed to get a "timeout waiting for command 16"
> debug message when I did iwconfig just after /etc/init.d/wlan.eth1 start
> Not sure it's related though (not that repeatable).
> 

I get two of those on every insertion. They are not transport related as
far as I can tell.

> 
> I guess the readability  of /4)*4 vs &~3 is a matter of taste and it
> gave the same code for three different platforms.
> But removing the if () would IMO improve readability and have the positive
> sideeffect of a 22-57% reduction in number of instructions
> (it's not many instructions - but it's low hanging fruit and done in 
> the network path so if it was my call, the if would go away)
> 

Fair enough. I'll remove the if.

> 
> > > Possibly make the function inline ?
> > > 
> > 
> > The ambition is to make it more complex to properly deal with more
> > nasty controllers, so it wouldn't stay inline for long anyway.
> 
> Ok, could the complex multi controller support be conditional to avoid
> overhead for the embedded solution with known hardware?
> 

In theory, but it's not that easy to do without making a mess of the
code. It's also a maintenance problem as you'd have to code information
about controller problems in two places.

I vote for keeping this and if you can measure a performance problem,
we can add some inline wrapper around the function that can quickly
return for known good controllers. Premature optimisation and all
that...

> > 
> > Just trying to solve a related problem at the same time (one that the
> > code ripped out of if_sdio sort of solves). Imagine if you have a
> > transfer of 1233 bytes. First you'd need to pad it to 1236 bytes to
> > handle 32-bit size alignment bugs. But transferring this chunk would
> > result in one request of 2 * 512 bytes, and one of 212 bytes. Padding
> > it to 1536 bytes allows you to a single request with 3 * 512 bytes.
> 
> Aha.
> Is it a certain fact that padding to multiple blocksizes is always
> better than multiple transfers?
> (Considering all the interrupt traffic for a single transfer with an
> SDHCI like controller, and with small blocksizes (512) I would guess 
> it is soo, but anyway?.
> 

There are probably cases where it is a loss. But for the general case
it should be an improvement. Besides, now that the logic is in one
place, it is easy to improve if performance problems are found. :)

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.