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

Arnd Bergmann
2013-03-14T22:53:04Z

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