Hi Magnus,
On Wed, Mar 16, 2011 at 09:37:50AM +0900, Magnus Damm wrote:
> Hi Simon,
>
> Thanks for your work on this!
[ snip ]
>
> On Wed, Mar 16, 2011 at 7:27 AM, Simon Horman <horms@xxxxxxxxxxxx> wrote:
> > This allows a ROM-able zImage to be written to eSD and for SuperH Mobile
> > ARM to boot directly from the SDHI hardware block.
> >
> > This is achieved by the MaskROM loading the first portion of the image into
> > MERAM and then jumping to it. ÂThis portion contains loader code which
> > copies the entire image to SDRAM and jumps to it. From there the zImage
> > boot code proceeds as normal, uncompressing the image into its final
> > location and then jumping to it.
> >
> > Cc: Paul Mundt <lethal@xxxxxxxxxxxx>
> > Cc: Magnus Damm <magnus.damm@xxxxxxxxx>
> > Cc: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
> > Signed-off-by: Simon Horman <horms@xxxxxxxxxxxx>
> >
> > ---
> >
> > This patch is based on the for-next branch of
> > Russell King's linux-2.6-arm tree
> >
> > v2
> > * Consistently use __raw_readw(). As pointed out by Paul Mundt
> >
> > v3
> > * Remove mmcif_update_progress2(), it was for debugging during development
> > * Move CPU specific code into mach/sdhi.h
> > ÂAs requested by Magnus Damm
> > * Use linux/mmc/tmio.h now that it exists
> > * Remove SDHI_EXT_SWAP, it is unused
> > * Replace use of MMCIF_PROGRESS_* with MMC_PROGRESS_*
> > * Don't include linux/mmc/sh_mmcif.h
> > Â+ Replace use of MMCIF_CE_RESP_CMD12 with RESP_CMD12
> > Â+ Include linux/io.h
> >
> > v4
> > * Move definition of SDHI_BASE into CPU-specific code.
> > ÂThanks to Magnus Damm.
> > ---
>
> > +asmlinkage void mmc_loader(unsigned short *buf, unsigned long len)
> > +{
> > + Â Â Â int high_capacity;
> > +
> > + Â Â Â mmc_init_progress();
> > +
> > + Â Â Â mmc_update_progress(MMC_PROGRESS_ENTER);
> > + Â Â Â sdhi_boot_enter();
> > +
> > + Â Â Â /* setup SDHI hardware */
> > + Â Â Â mmc_update_progress(MMC_PROGRESS_INIT);
> > + Â Â Â high_capacity = sdhi_boot_init(SDHI_BASE);
> > + Â Â Â if (high_capacity < 0)
> > + Â Â Â Â Â Â Â goto err;
> > +
> > + Â Â Â mmc_update_progress(MMC_PROGRESS_LOAD);
> > + Â Â Â /* load kernel */
> > + Â Â Â if (sdhi_boot_do_read(SDHI_BASE, high_capacity,
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â 0, /* Kernel is at block 1 */
> > + Â Â Â Â Â Â Â Â Â Â Â Â Â Â (len + TMIO_BBS - 1) / TMIO_BBS, buf))
> > + Â Â Â Â Â Â Â goto err;
> > +
> > + Â Â Â sdhi_boot_cleanup();
> > +
> > + Â Â Â mmc_update_progress(MMC_PROGRESS_DONE);
> > +
> > + Â Â Â return;
> > +err:
> > + Â Â Â __raw_writel(__raw_readl(PORTR031_000DR) | 1, PORTR031_000DR);
> > + Â Â Â for(;;);
> > +
> > +}
>
> Sorry for not catching this earlier, but this __raw_writel() to
> PORTR031_000DR is cpu specific as well. So please move that to the CPU
> specific code.
I will just remove that code, its was useful for development
but I didn't mean to include it in my patch submission.
> Not sure if it makes sense at this point, but perhaps it's a good idea
> to move the mmc_loader() function into the CPU specific portion. As
> you know, the CPU itself has multiple SDHI hardware blocks, and
> because of that we want the common SDHI loader to be written to
> support any SDHI hardware block instance.
Wouldn't that mean moving all of
arch/arm/boot/compressed/sdhi-shmobile.c into CPU specific code?
That could easily be achived by just guarding its compilation with
CONFIG_ARCH_SH7372 (as mmcif-sh7372.c already is) and perhaps
renaming the file to sdhi-sh7372.c. We could probably move
arch/arm/mach-shmobile/include/mach/sdhi-sh7372.h back into
sdhi-shmobile.c.
> Right now the SDHI_BASE variable is limiting the shared SDHI loader
> code to a fixed hardware block instance. That's fine because we only
> boot from a single SDHI hardware block instance on sh7372, but future
> processors most likely support selecting boot SDHI hardware block
> instance.
So mmc_loader() would need to take SDHI_BASE as an argument?
That sounds like a fairly small amount of refactoring.
How do you envisage that the hardware block would be selected?
At compile time through Kconfig? If so the current #define mechanism
might be sufficient.
> Apart from those minor bits I think the code is getting into a really
> good shape!
Thanks
--
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
