Thread (25 messages) 25 messages, 2 authors, 2009-03-08

Re: [PATCH 02/11] sdhci: Add support for bus-specific IO memory accessors

From: Pierre Ossman <hidden>
Date: 2009-02-21 15:58:19
Also in: lkml

On Fri, 13 Feb 2009 17:40:39 +0300
Anton Vorontsov [off-list ref] wrote:
No, on eSDHC the registers are big-endian, 32-bit width, with, for
example, two 16-bit "logical" registers packed into it.

That is,

 0x4  0x5 0x6  0x7
|~~~~~~~~:~~~~~~~~|
| BLKCNT : BLKSZ  |
|________:________|
 31              0

( The register looks wrong, right? BLKSZ should be at 0x4. But imagine
  that you swapped bytes in this 32 bit register... then the registers
  and their byte addresses will look normal. )

So if we try to issue readw(SDHCI_BLOCK_SIZE), i.e. readw(0x4):

- We'll read BLKCNT, while we wanted BLKSZ. This is because the
  address bits should be translated before we try word or byte
  reads/writes.
- On powerpc read{l,w}() convert the read value from little-endian
  to big-endian byte order, which is wrong for our case (the
  register is big-endian already).

That means that we have to convert address, but we don't want to
convert the result of read/write ops.
*cries*

Now this is just incredibly horrible. Why the hell did they try to use
the sdhci interface and then do stupid things like this?
quoted
quoted
+static inline void sdhci_writel(struct sdhci_host *host, u32 val, int reg)
+{
+	host->writel(host, val, reg);
+}
Having to override these are worst case scenario
Hm. It's not a worst case scenario, it's a normal scenario for
eSDHC. Why should we treat eSDHC as a second-class citizen?
Because it's complete and utter crap. Freescale has completely ignored
the basic register interface requirements of the SDHCI spec. Treating
eSDHC as a second-class citizen is generous IMO.
quoted
as far as I'm
concerned, so I'd prefer something like:

if (!host->ops->writel)
	writel(host->ioaddr + reg, val);
else
	host->ops->writel(host, val, reg);
Surely the overhead isn't measurable... but why we purposely make
things worse?
We can most likely do some micro-optimisation do make the compare part
cheaper, but the point was to avoid a function call for all the
properly implemented controllers out there. We could have a flag so
that it only has to check host->flags, which will most likely be in the
cache anyway.

Overhead for eSDHC is not a concern in my book, what is interesting is
how much this change slows things down for other controllers.

Rgds
-- 
     -- Pierre Ossman

  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.

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help