On Thu, 2015-05-21 at 22:33 +0800, Daniel Kurtz wrote: > On Thu, May 21, 2015 at 3:30 PM, Matthias Brugger > <matthias.bgg@xxxxxxxxx> wrote: > > 2015-05-21 8:16 GMT+02:00 Yong Wu <yong.wu@xxxxxxxxxxxx>: > >> Hi Matthias, > >> Thanks very much for your suggestion. > >> Abort the smi clock name, Could you help check below. > >> The others I will improve in next time. > >> > >> On Tue, 2015-05-19 at 13:14 +0200, Matthias Brugger wrote: > >>> 2015-05-15 11:43 GMT+02:00 Yong Wu <yong.wu@xxxxxxxxxxxx>: > >>> > This patch add SMI(Smart Multimedia Interface) driver. This driver is > >>> > responsible to enable/disable iommu and control the clocks of each local arbiter. > >>> > > >> [snip] > >>> > + > >>> > +#define SMI_LARB_MMU_EN (0xf00) > >>> > +#define F_SMI_MMU_EN(port) (1 << (port)) > >>> > + > >>> > +enum { > >>> > + MTK_CLK_APB, > >>> > + MTK_CLK_SMI, > >>> > + MTK_CLK_MAX, > >>> > >>> Maybe add something like: > >>> MTK_CLK_FIRST = MTK_CLK_APB, > >>> to make the for loops better readable. > >>> > >> Then, Is it like this? : > >> enum { > >> MTK_CLK_FIRST = MTK_CLK_APB, > >> MTK_CLK_SMI, > >> MTK_CLK_MAX, > >> } > >> or the CLK_SMI also need MTK_CLK_SECOND = MTK_CLK_SMI. > > > > > > something like: > > enum { > > MTK_CLK_FIRST, > > MTK_CLK_APB = MTK_CLK_FIRST, > > MTK_CLK_SMI, > > MTK_CLK_LAST, > > } > > > > So you can rewrite the for loop: > > if (i = MTK_CLK_FIRST; i < MTK_CLK_LAST; i++) > > Actually, do we ever plan to add more clks per smi node? No. > If not, perhaps the whole driver would be simpler if you just > explicitly handle the apb & smi clocks: > > struct mtk_smi_larb { > void __iomem *base; > spinlock_t portlock; /* lock for config port */ > struct device *smi; > struct clk *clk_apb; > struct clk *clk_smi; > }; > > And then all of the loops become just a pair of clock operations. Thanks. I will try this and compare whether it will add many lines. > > Best Regards, > -Dan > > > > > Regards, > > Matthias > > > >>> > +}; > >>> > + > >>> > +struct mtk_smi_common { > >>> > + void __iomem *base; > >>> > >>> That seems to be never used. Please delete it. > >>> > >>> > + struct clk *clk[MTK_CLK_MAX]; > >>> > +}; > >>> > + > >>> > +struct mtk_smi_larb { > >>> > + void __iomem *base; > >>> > + spinlock_t portlock; /* lock for config port */ > >>> > + struct clk *clk[MTK_CLK_MAX]; > >>> > + struct device *smi; > >>> > +}; > >>> > + > >>> Thanks, > >>> Matthias > >> > > -- > > motzblog.wordpress.com -- 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