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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <57732AD5.7010708@rock-chips.com>
Date:	Wed, 29 Jun 2016 09:56:37 +0800
From:	William Wu <william.wu@...k-chips.com>
To:	Heiko Stuebner <heiko@...ech.de>
CC:	gregkh@...uxfoundation.org, balbi@...nel.org,
	linux-rockchip@...ts.infradead.org, briannorris@...gle.com,
	dianders@...gle.com, kever.yang@...k-chips.com,
	huangtao@...k-chips.com, frank.wang@...k-chips.com,
	eddie.cai@...k-chips.com, John.Youn@...opsys.com,
	linux-kernel@...r.kernel.org, linux-usb@...r.kernel.org,
	sergei.shtylyov@...entembedded.com
Subject: Re: [PATCH v4 5/5] usb: dwc3: rockchip: add devicetree bindings documentation

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