Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755830AbbEUOdh (ORCPT ); Thu, 21 May 2015 10:33:37 -0400 Received: from mail-oi0-f50.google.com ([209.85.218.50]:34189 "EHLO mail-oi0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753776AbbEUOde (ORCPT ); Thu, 21 May 2015 10:33:34 -0400 MIME-Version: 1.0 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> From: Daniel Kurtz Date: Thu, 21 May 2015 22:33:13 +0800 Message-ID: Subject: Re: [PATCH v2 4/6] soc: mediatek: Add SMI driver To: Matthias Brugger Cc: Yong Wu , Rob Herring , Joerg Roedel , Robin Murphy , Will Deacon , Tomasz Figa , Lucas Stach , Mark Rutland , Catalin Marinas , linux-mediatek@lists.infradead.org, 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 , mitchelh@codeaurora.org, k.zhang@mediatek.com, youhua.li@mediatek.com Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2792 Lines: 104 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? 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@google.com | 650.204.0722 -- 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/