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

[Pierre Ossman <drzeus@drzeus.cx>]

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

On Fri, 6 Feb 2009 21:06:45 +0300
Anton Vorontsov <avorontsov@ru.mvista.com> wrote:

> Currently the SDHCI driver works with PCI accessors (write{l,b,w} and
> read{l,b,w}).
> 
> With this patch drivers may change memory accessors, so that we can
> support hosts with "weird" IO memory access requirments.
> 
> For example, in "FSL eSDHC" SDHCI hardware all registers are 32 bit
> width, with big-endian addressing. That is, readb(0x2f) should turn
> into readb(0x2c), and readw(0x2c) should be translated to
> le16_to_cpu(readw(0x2e)).
> 
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---

I was hoping we wouldn't have to do a lot of magic in the accessors
since the spec is rather clear on the register interface. :/

Let's see if I've understood this correctly.

1. The CPU is big-endian but the register are little-endian (as the
spec requires). I was under the impression that the read*/write*
accessor handled any endian conversion between the bus and the cpu? How
do e.g. PCI work on Sparc?

2. Register access must be done 32 bits at a time. Now this is just
broken and might cause big problems as some registers cannot just be
read and written back to. OTOH you refer to readw() in your example,
not readl(). What's the deal here?

> +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 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);

and maybe even a likely() up there.

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: 197 bytes --]