Re: [PATCH v4 5/5] usb: dwc3: rockchip: add devicetree bindings documentation

[Heiko Stuebner <heiko@sntech.de>]

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.


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


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


Heiko