Re: [PATCH] Revert "sdhci-of-esdhc: Support 8BIT bus width."

Kevin Hao
2015-05-19T09:20:21Z

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

On Sun, May 17, 2015 at 08:36:07AM +0000, Joakim Tjernlund wrote:
> On Sun, 2015-05-17 at 13:06 +0800, Kevin Hao wrote:
> > >
> > > How about this one:
> > >
> > > From af6b18c056b6064424bd2ab1f9989bbadae5e701 Mon Sep 17 00:00:00 2001
> > > From: Joakim Tjernlund <joakim.tjernlund@transmode.se>
> > > Date: Mon, 20 Apr 2015 22:36:55 +0200
> > > Subject: [PATCHv3] sdhci-of-esdhc: Support 8BIT bus width.
> > >
> > > esdhc_readb()/esdhc_writeb() did not adjust for 8BIT.
> >
> > Do we really need this for the 8bit bus support? There is already a specific
> > API for setting the bus width, this change seems unnecessary to me. That is
> > also why I choose to revert that patch. Did I miss something?
>
> We do, the bus API really only works well when the bus bits are in another
> register but the HOST_CONTROL register.

Sorry, I didn't get what you mean. Could you elaborate a bit more?

> The only reason 4BIT works is because its bit placement is where
> SDHCI expects it to be. 8BIT is not, so unless readb/writeb funktions compensate
> for that they will overwrite what the bus API set earlier.

But the implementation of esdhc_pltfm_set_bus_width() already take care about
this. And in the current kernel, we only set the bus width via this API. So why
do you think that it can be overwrote?

static void esdhc_pltfm_set_bus_width(struct sdhci_host *host, int width)
{
u32 ctrl;

switch (width) {
case MMC_BUS_WIDTH_8:
ctrl = ESDHC_CTRL_8BITBUS;
break;

case MMC_BUS_WIDTH_4:
ctrl = ESDHC_CTRL_4BITBUS;
break;

default:
ctrl = 0;
break;
}

clrsetbits_be32(host->ioaddr + SDHCI_HOST_CONTROL,
ESDHC_CTRL_BUSWIDTH_MASK, ctrl);
}

>
> Atleast this is my understanding, Ulf?
>
> Didn't this patch work for you either?

I don't have a 8bit card at hand. For 4bit card, it works with or without this
change.

Thanks,
Kevin

>
> Jocke

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]