On Thu, 14 Mar 2013 17:50:08 +0100
Arnd Bergmann <arnd@xxxxxxxx> wrote:
> On Thursday 14 March 2013 17:40:49 Anatolij Gustschin wrote:
>
> > +
> > +struct mxcmci_reg_ops mxcmci_reg_ops = {
> > + .read_l = mpcmci_readl,
> > + .write_l = mpcmci_writel,
> > + .read_w = mpcmci_readw,
> > + .write_w = mpcmci_writew,
> > +};
> > +#else
> > +struct mxcmci_reg_ops mxcmci_reg_ops;
> > +#endif
>
> Should the struct be static?
yes, it should.
> > +static inline u32 mxcmci_readl(struct mxcmci_host *host, int reg)
> > +{
> > + if (host->reg_ops->read_l)
> > + return host->reg_ops->read_l(host, reg);
> > + else
> > + return readl(host->base + reg);
> > +}
>
> This seems a bit strange. I would suggest you either use the ops structure
> all the time and provide an imx variant, or you make it completely
> compile-time selected.
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.
> > @@ -1026,24 +1115,33 @@ static int mxcmci_probe(struct platform_device *pdev)
> > host->res = r;
> > host->irq = irq;
> >
> > - host->clk_ipg = devm_clk_get(&pdev->dev, "ipg");
> > - if (IS_ERR(host->clk_ipg)) {
> > - ret = PTR_ERR(host->clk_ipg);
> > - goto out_iounmap;
> > - }
> > + if (!is_mpc512x_mmc(host)) {
> > + host->clk_ipg = devm_clk_get(&pdev->dev, "ipg");
> > + if (IS_ERR(host->clk_ipg)) {
> > + ret = PTR_ERR(host->clk_ipg);
> > + goto out_iounmap;
> > + }
>
> 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.
Thanks,
Anatolij
--
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
