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 > > > >
