hi Russell,

On Thu, Mar 26, 2015 at 02:18:34PM +0000, Russell King - ARM Linux wrote:
> On Thu, Mar 26, 2015 at 07:13:36PM +0800, Leo Yan wrote:
> > +struct hisi_clock_data __init *hisi_clk_init(struct device_node *np,
> > +                                        int nr_clks)
> > +{
> > +   struct hisi_clock_data *clk_data;
> > +   void __iomem *base;
> > +
> > +   if (np) {
> > +           base = of_iomap(np, 0);
> > +           if (!base) {
> > +                   pr_err("failed to map Hisilicon clock registers\n");
> > +                   return NULL;
> > +           }
> > +           printk("%s: base %p\n", __func__, base);
> 
> Did you leave your debugging in?

Sorry, will remove it.

> > +   } else {
> > +           pr_err("failed to find Hisilicon clock node in DTS\n");
> > +           return NULL;
> > +   }
> 
> I know you're mostly only moving this code, but it would be far better if
> it were written:
> 
>       if (!np) {
>               pr_err("failed to find Hisilicon clock node in DTS\n");
>               return NULL;
>       }
> 
>       base = of_iomap(np, 0);
>       if (!base) {
>               pr_err("failed to map Hisilicon clock registers\n");
>               return NULL;
>       }
> 
> Possibly do this first as a separate patch, and then move the code.

Will do this.

> -- 
> FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
> according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to