Re: [PATCH 1/5] mmc: mxcmmc: add mpc512x SDHC support

[Arnd Bergmann <arnd@arndb.de>]

On Thursday 14 March 2013, Anatolij Gustschin wrote:
> I wanted to avoid additional levels of indirection and function calls
> on i.MX. If something like
> 
> static inline u32 mxcmci_readl(struct mxcmci_host *host, int reg)
> {
> #if IS_ENABLED(CONFIG_PPC_MPC512x)
>         return in_be32(host->base + reg);
> #else
>         return readl(host->base + reg); 
> #endif
> }
> 
> is acceptable, I'll use it.

I think that's ok. A single #ifdef around the four functions might be
nicer though. You could also use ioread32_be on powerpc and write it
like

static inline u32 mxcmci_readl(struct mxcmci_host *host, int reg)
{
	if (IS_ENABLED(CONFIG_PPC_MPC512x))
		return ioread32_be(host->base + reg);
	else
		return readl(host->base + reg); 
}

> > Does mpc512x have no clock management? I think it should still
> > work without modifications if CONFIG_HAVE_CLK is disabled.
> > In that case, devm_clk_get() will return NULL and we don't
> > error out here.
> 
> It does have some clock management (a platform clock driver) and
> the platform selects CONFIG_HAVE_CLK. But we do not have "ipg"
> and "per" clocks on that platform, but "sdhc_clk" instead.

As Sascha said, they should really use the same name, although it's
possible that neither sdhc_clk nor ipg is a good name here.

	Arnd