Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756079AbbEUOtw (ORCPT ); Thu, 21 May 2015 10:49:52 -0400 Received: from mailgw01.mediatek.com ([218.249.47.110]:37056 "EHLO mailgw01.mediatek.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1756008AbbEUOtp (ORCPT ); Thu, 21 May 2015 10:49:45 -0400 X-Listener-Flag: 11101 Message-ID: <1432219778.5092.44.camel@mhfsdcap03> Subject: Re: [PATCH v2 4/6] soc: mediatek: Add SMI driver From: Yong Wu To: Daniel Kurtz CC: Matthias Brugger , Rob Herring , Joerg Roedel , Robin Murphy , Will Deacon , Tomasz Figa , Lucas Stach , Mark Rutland , Catalin Marinas , , Sasha Hauer , srv_heupstream , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "open list:IOMMU DRIVERS" , Paul Bolle , "Arnd Bergmann" , , , Date: Thu, 21 May 2015 22:49:38 +0800 In-Reply-To: References: <1431683009-18158-1-git-send-email-yong.wu@mediatek.com> <1431683009-18158-5-git-send-email-yong.wu@mediatek.com> <1432188990.5092.12.camel@mhfsdcap03> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.2.3-0ubuntu6 Content-Transfer-Encoding: 7bit MIME-Version: 1.0 X-MTK: N Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3015 Lines: 99 On Thu, 2015-05-21 at 22:33 +0800, Daniel Kurtz wrote: > On Thu, May 21, 2015 at 3:30 PM, Matthias Brugger > wrote: > > 2015-05-21 8:16 GMT+02:00 Yong Wu : > >> 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 : > >>> > 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 linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/