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

Pierre Ossman
2009-02-21T15:58:19Z

[-- Attachment #1: Type: text/plain, Size: 2701 bytes --]

On Fri, 13 Feb 2009 17:40:39 +0300
Anton Vorontsov <avorontsov@ru.mvista.com> 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?

> > > +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.

> > 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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]