linux-kernel - Re: [PATCH v2 5/7] usb: dwc3: Add snps,ref-clock-frequency-hz property for ACPI

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <7e64e9c7-70e3-d833-34dc-272048d29038@synopsys.com>
Date:   Tue, 25 Jan 2022 02:21:52 +0000
From:   Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To:     Sean Anderson <sean.anderson@...o.com>,
        Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>
CC:     Felipe Balbi <balbi@...nel.org>,
        Balaji Prakash J <bjagadee@...eaurora.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Robert Hancock <robert.hancock@...ian.com>,
        Baruch Siach <baruch@...s.co.il>, Rob Herring <robh@...nel.org>
Subject: Re: [PATCH v2 5/7] usb: dwc3: Add snps,ref-clock-frequency-hz
 property for ACPI

Sean Anderson wrote:
> 
> 
> On 1/24/22 6:07 PM, Sean Anderson wrote:
>> On 1/24/22 5:44 PM, Thinh Nguyen wrote:
>>> Sean Anderson wrote:
>>>> This property allows setting the reference clock frequency properly for
>>>> ACPI-based systems. It is not documented under dt-bindings, since it is
>>>> not intended for use on DT-based systems. DT-based systems should use
>>>> the clocks property instead.
>>>>
>>>> Frequency is preferred over period since it has greater precision when
>>>> used in calculations.
>>>>
>>>> Signed-off-by: Sean Anderson <sean.anderson@...o.com>
>>>> ---
>>>>
>>>> Changes in v2:
>>>> - New
>>>>
>>>>  drivers/usb/dwc3/core.c | 6 ++++--
>>>>  drivers/usb/dwc3/core.h | 4 +++-
>>>>  2 files changed, 7 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>>>> index 883e119377f0..5f3dc5f6cbcb 100644
>>>> --- a/drivers/usb/dwc3/core.c
>>>> +++ b/drivers/usb/dwc3/core.c
>>>> @@ -350,8 +350,8 @@ static void dwc3_ref_clk_period(struct dwc3 *dwc)
>>>>  	u32 reg;
>>>>  	unsigned long decr, fladj, rate, period;
>>>>  
>>>> -	if (dwc->ref_clk) {
>>>> -		rate = clk_get_rate(dwc->ref_clk);
>>>> +	if (dwc->ref_clk || dwc->ref_clk_freq) {
>>>> +		rate = clk_get_rate(dwc->ref_clk) ?: dwc->ref_clk_freq;
>>>>  		if (!rate)
>>>>  			return;
>>>>  		period = NSEC_PER_SEC / rate;
>>>> @@ -1492,6 +1492,8 @@ static void dwc3_get_properties(struct dwc3 *dwc)
>>>>  				 &dwc->fladj);
>>>>  	device_property_read_u32(dev, "snps,ref-clock-period-ns",
>>>>  				 &dwc->ref_clk_per);
>>>> +	device_property_read_u32(dev, "snps,ref-clock-frequency-hz",
>>>> +				 &dwc->ref_clk_freq);
>>>
>>> Please also document in dwc3 DT file whenever we add a new property.
>>
>> This is intentionally undocumented, as noted in the commit message. 
>> Rob Herring has said that dt-bindings should only document properties
>> intended for device-tree.
> 
> context: https://urldefense.com/v3/__https://lore.kernel.org/all/20181219202734.GA31178@bogus/__;!!A4F2R9G_pg!IFaZE73-RQzYnhLSPKuFGqreudBcyVi6T9lGG5byblzoC9i_diaCBe_soAyHPCupua_T$ 
> 
> This patch was later resubmitted as 24bc6e68efa0 ("serial: sc16is7xx: 
> Respect clock-frequency property") without the dt-bindings documentation.
> 
> +CC Rob if he wants to comment on this specific situation.
> 
> --Sean

If this is acceptable, that's great! It opens up more options to the PCI
glue drivers. Maybe we should also document inline for properties that
don't exist in the DT binding.

Thanks,
Thinh