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

[Thinh Nguyen <Thinh.Nguyen@synopsys.com>]

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