From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752621AbcF2B54 (ORCPT ); Tue, 28 Jun 2016 21:57:56 -0400 Received: from regular1.263xmail.com ([211.150.99.133]:58983 "EHLO regular1.263xmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752440AbcF2B5n (ORCPT ); Tue, 28 Jun 2016 21:57:43 -0400 X-263anti-spam: KSV:0; X-MAIL-GRAY: 0 X-MAIL-DELIVERY: 1 X-KSVirus-check: 0 X-ABS-CHECKED: 4 X-ADDR-CHECKED: 0 X-RL-SENDER: wulf@rock-chips.com X-FST-TO: heiko@sntech.de X-SENDER-IP: 58.22.7.114 X-LOGIN-NAME: wulf@rock-chips.com X-UNIQUE-TAG: X-ATTACHMENT-NUM: 0 X-DNS-TYPE: 0 Message-ID: <57732AD5.7010708@rock-chips.com> Date: Wed, 29 Jun 2016 09:56:37 +0800 From: William Wu User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.6.0 MIME-Version: 1.0 To: Heiko Stuebner CC: gregkh@linuxfoundation.org, balbi@kernel.org, linux-rockchip@lists.infradead.org, briannorris@google.com, dianders@google.com, kever.yang@rock-chips.com, huangtao@rock-chips.com, frank.wang@rock-chips.com, eddie.cai@rock-chips.com, John.Youn@synopsys.com, linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org, sergei.shtylyov@cogentembedded.com Subject: Re: [PATCH v4 5/5] usb: dwc3: rockchip: add devicetree bindings documentation References: <1464870896-25612-1-git-send-email-william.wu@rock-chips.com> <1518073.zeMq7N85Id@phil> <5771EC6C.3070707@rock-chips.com> <1652918.4YDfA4VDoG@phil> In-Reply-To: <1652918.4YDfA4VDoG@phil> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Dear Heiko, On 06/29/2016 12:41 AM, Heiko Stuebner wrote: > Hi William, > > Am Dienstag, 28. Juni 2016, 11:18:04 schrieb William Wu: >>>> So about the usb3 controller clk management, I think it should contain >>>> the following clk: >>>> 1. aclk_usb3otg1 >>>> 2. aclk_usb3otg0 >>>> 3. aclk_usb3_grf >>> correct, aclk_usb3otgX would then be the busclk for each controller if >>> I'm not mistaken and the grf clock should also get enabled, like we >>> also plan on doing for the vio_grf clock in the display area. >> OK, so aclk_usb3_grf should be marked as critical, right? > nope ... the consensus for the vio_grf clock was that the driver accessing > it should control it. So for the usb3_grf clock this would be your dwc3- > based driver being responsible for the grf-clock. Ah, I misunderstand before. I'll control usb3_grf in dwc3 driver. > > >> I found that most of the grf clocks haven't been marked as critical, >> apart from vio_grf. So may I keep the aclk_usb3_grf in usb3 dts, and >> remove it after clock manager adds it to critical clocks? > you should keep the usb3-grf clock in the dts all the time. OK, I'll keep it in the dts. > > >>>> 4. aclk_usb3_noc >>>> For "aclk_usb3_noc", I discuss with our clk manager, the clk is always >>>> on before, >>>> but according to upstream maintainer's suggestion, we need to manage >>>> the >>>> noc clk by each module. >>> can you point me to this discussion? The bus-interconnect is a very >>> separate component, which we don't model so far and thus we have all >>> the noc clocks simply marked as critical. >>> >>> As this clock doesn't belong to the actual usb controller block, but >>> said >>> separate component, handling it in the controller seems somehow wrong to >>> me. >>> >>> So my (current) opinion would again be to mark the noc clock as critical >>> for the time being. >> Sorry, it seems that I get the new information about clk management too >> late.:-) >> >> There's no dedicated discussion about noc clk, but similar to grf clock, >> please refer to "https://patchwork.kernel.org/patch/9171467/" for add >> pclk_vio_grf to critical clock on the RK3399, and you have agreed on >> that mark vio grf clk as critical. So I agree with your opinion, thanks~ > The difference as I see it is, that the GRF parts are _part_ of the usb3 > controller, while the interconnect really is a separate component, > connecting the controller to the rest of the system. > > ---------- > | USB3 |----[Interconnect]----[rest of the system] > | [+GRF] | > ---------- > > So the controller binding + driver should handle all clocks it needs to > operate. We currently don't model and handle the separate interconnect > though, so that noc clock should be critical for the time being. Thanks for your professional explanation. Now I think I have more clearly understood the clocks. And I'll upload a new patch later. > > Heiko > > > >