On Monday 08 January 2018 07:47 AM, David Lechner wrote: > +void __init da850_pll_clk_init(void __iomem *pll0, void __iomem *pll1) > +{ > + const struct davinci_pll_divclk_info *info; > + > + davinci_pll_clk_register("pll0", "ref_clk", pll0); I think this will be more readable with empty lines between these function calls. So here .. > + davinci_pll_aux_clk_register("pll0_aux_clk", "ref_clk", pll0); .. here > + for (info = da850_pll0_divclk_info; info->name; info++) > + davinci_pll_divclk_register(info, pll0); > + > + davinci_pll_clk_register("pll1", "ref_clk", pll1); .. and here. > + for (info = da850_pll1_divclk_info; info->name; info++) > + davinci_pll_divclk_register(info, pll1); I see that you have included empty line only when changing the PLL, but I feel its more readable with the additional spacing suggested. Same comment for other patches too with similar function implemented. With that minor comment: Reviewed-by: Sekhar Nori <nsekhar@xxxxxx> Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html