| Message ID | 20220119002438.106079-6-sean.anderson@seco.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usb: dwc3: Calculate REFCLKPER et. al. from reference clock | expand |
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. Thanks, Thinh > > dwc->dis_metastability_quirk = device_property_read_bool(dev, > "snps,dis_metastability_quirk"); > diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h > index eb9c1efced05..00a792459fec 100644 > --- a/drivers/usb/dwc3/core.h > +++ b/drivers/usb/dwc3/core.h > @@ -988,7 +988,8 @@ struct dwc3_scratchpad_array { > * @regs: base address for our registers > * @regs_size: address space size > * @fladj: frame length adjustment > - * @ref_clk_per: reference clock period configuration > + * @ref_clk_per: reference clock period; deprecated in favor of @ref_clk_freq > + * @ref_clk_freq: reference clock frequency to use if @ref_clk is missing > * @irq_gadget: peripheral controller's IRQ number > * @otg_irq: IRQ number for OTG IRQs > * @current_otg_role: current role of operation while using the OTG block > @@ -1171,6 +1172,7 @@ struct dwc3 { > > u32 fladj; > u32 ref_clk_per; > + u32 ref_clk_freq; > u32 irq_gadget; > u32 otg_irq; > u32 current_otg_role;
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. --Sean
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://lore.kernel.org/all/20181219202734.GA31178@bogus/ 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
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
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); dwc->dis_metastability_quirk = device_property_read_bool(dev, "snps,dis_metastability_quirk"); diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index eb9c1efced05..00a792459fec 100644 --- a/drivers/usb/dwc3/core.h +++ b/drivers/usb/dwc3/core.h @@ -988,7 +988,8 @@ struct dwc3_scratchpad_array { * @regs: base address for our registers * @regs_size: address space size * @fladj: frame length adjustment - * @ref_clk_per: reference clock period configuration + * @ref_clk_per: reference clock period; deprecated in favor of @ref_clk_freq + * @ref_clk_freq: reference clock frequency to use if @ref_clk is missing * @irq_gadget: peripheral controller's IRQ number * @otg_irq: IRQ number for OTG IRQs * @current_otg_role: current role of operation while using the OTG block @@ -1171,6 +1172,7 @@ struct dwc3 { u32 fladj; u32 ref_clk_per; + u32 ref_clk_freq; u32 irq_gadget; u32 otg_irq; u32 current_otg_role;
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(-)