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?

> +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.

> @@ -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.

        Arnd
_______________________________________________
devicetree-discuss mailing list
[email protected]
https://lists.ozlabs.org/listinfo/devicetree-discuss

Reply via email to