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? 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. 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 -- Daniel Kurtz | Software Engineer | djkurtz@xxxxxxxxxx | 650.204.0722 -- 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