From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753386AbbDNJI3 (ORCPT ); Tue, 14 Apr 2015 05:08:29 -0400 Received: from mailgw01.mediatek.com ([218.249.47.110]:56623 "EHLO mailgw01.mediatek.com" rhost-flags-OK-FAIL-OK-FAIL) by vger.kernel.org with ESMTP id S1751787AbbDNJIT (ORCPT ); Tue, 14 Apr 2015 05:08:19 -0400 X-Listener-Flag: 11101 Message-ID: <1429002474.14855.7.camel@mhfsdcap03> Subject: Re: [PATCH 3/5] dt-bindings: mediatek: Add smi dts binding From: Yong Wu To: Mark Rutland CC: Rob Herring , Joerg Roedel , Matthias Brugger , Robin Murphy , Will Deacon , Daniel Kurtz , Tomasz Figa , Lucas Stach , Catalin Marinas , "linux-mediatek@lists.infradead.org" , Sasha Hauer , "srv_heupstream@mediatek.com" , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "iommu@lists.linux-foundation.org" Date: Tue, 14 Apr 2015 17:07:54 +0800 In-Reply-To: <20150306111338.GD8700@leverpostej> References: <1425638900-24989-1-git-send-email-yong.wu@mediatek.com> <1425638900-24989-4-git-send-email-yong.wu@mediatek.com> <20150306111338.GD8700@leverpostej> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.2.3-0ubuntu6 Content-Transfer-Encoding: 8bit MIME-Version: 1.0 X-MTK: N Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Mark, Thanks very much for review. About the clock name should be the PoV of _this_ device. Could you help check below? On Fri, 2015-03-06 at 11:13 +0000, Mark Rutland wrote: > On Fri, Mar 06, 2015 at 10:48:18AM +0000, yong.wu@mediatek.com wrote: > > From: Yong Wu > > > > This patch add smi binding document. > > Please move binding documents to the start of the series. It makes > things far easier to review. > > > > > Signed-off-by: Yong Wu > > +++ b/Documentation/devicetree/bindings/soc/mediatek/mediatek,smi.txt > > @@ -0,0 +1,17 @@ > > +SMI hardware block diagram please help check > > + > > +Required properties: > > +- compatible : must be "mediatek,mediatek,mt8173-smi-larb" > > Double vendor prefix? > > What does "larb" mean? It would be nice for the intorductory paragraph > in this file to explain. > > > +- reg : the register of each local arbiter > > +- clocks : the clocks of each local arbiter > > +- clock-name: larb_sub*(3 clockes at most) > > The names required _must_ be specified here, or clock-names is > pointless. > > The clock names should be from the PoV of _this_ device (i.e. they > should be the names of the inputs) not from the PoV of the provider > (i.e. they should not be the names of the outputs from the provider). > > Mark. > After we check with our SMI Designer. Every SMI local arbiter need two clocks, which is called APB clocks and SMI clock. APB clock : Advanced Peripheral Bus Clock. It is the clock for setting the register of local arbiter. SMI clock : Smart Multimedia Interface Clock, It is the clock for transfering the data and command. And all the local arbiters need the smi common clock, so we separate it. Then I prepare to design the smi the dtsi like this: smi_common:smi@14022000 { compatible = “mediate, mt8173-smi”; reg = <0 0x14022000 0 0x1000>; clocks = <&mmsys MM_SMI_COMMON>; clocks-names = “smi_common”; }; larb0: larb@14021000 { compatible = “mediate, mt8173-smi-larb”; reg = <0 0x14021000 0 0x1000>; smi = <&smi_common>; clocks = <&mmsys MM_SMI_LARB0>, <&mmsys MM_SMI_LARB0>; clocks-names = “apb_clk”, “smi_clk”; }; larb1: larb@16010000 { compatible = “mediate, mt8173-smi-larb”; reg = <0 0x16010000 0 0x1000>; smi = <&smi_common>; clocks = <&vdecsys VDEC_CKEN>, <&mmsys VDEC_LARB_CKEN>; clocks-names = “apb_clk”, “smi_clk”; }; … In some local arbiter, the source clock of the APB clock and the SMI clock may be the same, like larb0. so the two clocks are the same. And they may be different in other local arbiteres, like larb1. If it is designed like this, is it ok? > > + > > +Example: > > + larb1:larb@16010000 { > > + compatible = "mediatek,mt8173-smi-larb"; > > + reg = <0 0x16010000 0 0x1000>; > > + clocks = <&mmsys MM_SMI_COMMON>, > > + <&vdecsys VDEC_CKEN>, > > + <&vdecsys VDEC_LARB_CKEN>; > > + clock-names = "larb_sub0", "larb_sub1", "larb_sub2"; > > + }; > > -- > > 1.8.1.1.dirty > > > >