On Sun, 13 Mar 2011, Magnus Damm wrote:
> On Fri, Mar 11, 2011 at 5:09 PM, Guennadi Liakhovetski
> <g.liakhovetski@xxxxxx> wrote:
> > Not all mn57xx / tmio implementations have registers above oxff.
> > Accessing them on thise platforms is dangerous. In some cases it leads
> > to address wrapping to addresses below 0x100, which corrupts random
> > unrelated registers.
> >
> > Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>
> > ---
> >
> > v2:
>
> > --- a/drivers/mmc/host/tmio_mmc_pio.c
> > +++ b/drivers/mmc/host/tmio_mmc_pio.c
>
> Nice, it looks like this patch should apply on your earlier broken out
> patch set...
>
> > + if (resource_size(res) > 0x100) {
> > + sd_ctrl_write16(host, CTL_CLK_AND_WAIT_CTL, 0x0000);
> > + msleep(10);
> > + }
>
> ... but now since you've separated the tmio specific code from the
> SDHI implementation, why don't you just make all the offsets above
> 0x100 stay in the tmio code, and let the SDHI only access below 0xff?
The current split is:
tmio_mmc_pio.c: _all_ driver functionality in PIO mode, including MMC API
tmio_mmc_dma.c: DMA
tmio_mmc.c: _only_ MFD glue - no IO at all
sh_mobile_sdhi.c: _only_ platform glue for SDHI - no IO at all
This split seems logical to me - every file performs one function. But I
do agree, that special-casing addresses > 0xff in the generic core is not
very nice... We could otherwise modify the actual read / write accessors
to check, whether _each_ register address is within the resource, but that
would be too much of a performance hit. But pulling those couple of > 0xff
accesses out of the driver proper and into the MFD glue didn't look like a
very logical thing to me. I thought this version provided both - no
performance hit and preserved clean function separation - at the cost of
"slight ugliness." Opinions?
Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
