Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756238AbbEUPVC (ORCPT ); Thu, 21 May 2015 11:21:02 -0400 Received: from mail-la0-f41.google.com ([209.85.215.41]:36813 "EHLO mail-la0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756208AbbEUPVA (ORCPT ); Thu, 21 May 2015 11:21:00 -0400 MIME-Version: 1.0 In-Reply-To: <1432219778.5092.44.camel@mhfsdcap03> 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> <1432219778.5092.44.camel@mhfsdcap03> Date: Thu, 21 May 2015 17:20:55 +0200 Message-ID: Subject: Re: [PATCH v2 4/6] soc: mediatek: Add SMI driver From: Matthias Brugger To: Yong Wu Cc: Daniel Kurtz , 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: 2628 Lines: 77 2015-05-21 16:49 GMT+02:00 Yong Wu : > 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. If you don't plan to add more clocks in the future, then I think the suggestion Dan made is the best one. Regards, 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/