Hi, Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> writes: >>>> On 1/24/22 5:46 PM, Thinh Nguyen wrote: >>>>> Sean Anderson wrote: >>>>>> GUCTL.REFCLKPER can only account for clock frequencies with integer >>>>>> periods. To address this, program REFCLK_FLADJ with the relative error >>>>>> caused by period truncation. The formula given in the register reference >>>>>> has been rearranged to allow calculation based on rate (instead of >>>>>> period), and to allow for fixed-point arithmetic. >>>>>> >>>>>> Additionally, calculate a value for 240MHZDECR. This configures a >>>>>> simulated 240Mhz clock using a counter with one fractional bit (PLS1). >>>>>> >>>>>> This register is programmed only for versions >= 2.50a, since this is >>>>>> the check also used by commit db2be4e9e30c ("usb: dwc3: Add frame length >>>>>> adjustment quirk"). >>>>>> >>>>>> Signed-off-by: Sean Anderson <sean.anderson@xxxxxxxx> >>>>>> --- >>>>>> >>>>>> Changes in v2: >>>>>> - Also program GFLADJ.240MHZDECR >>>>>> - Don't program GFLADJ if the version is < 2.50a >>>>>> >>>>>> drivers/usb/dwc3/core.c | 37 +++++++++++++++++++++++++++++++++++-- >>>>>> drivers/usb/dwc3/core.h | 3 +++ >>>>>> 2 files changed, 38 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c >>>>>> index 5214daceda86..883e119377f0 100644 >>>>>> --- a/drivers/usb/dwc3/core.c >>>>>> +++ b/drivers/usb/dwc3/core.c >>>>>> @@ -348,7 +348,7 @@ static void dwc3_frame_length_adjustment(struct dwc3 *dwc) >>>>>> static void dwc3_ref_clk_period(struct dwc3 *dwc) >>>>>> { >>>>>> u32 reg; >>>>>> - unsigned long rate, period; >>>>>> + unsigned long decr, fladj, rate, period; >>>>> >>>>> Minor style nit: Felipe prefers to keep the declaration on separate >>>>> lines, let's keep it consistent with the rest in this driver. >>>> >>>> So >>>> >>>> unsigned int decr; >>>> unsigned int fladj; >>>> unsigned int rate; >>>> unsigned int period; >>>> >>>> ? >>>> >>>> Frankly that seems rather verbose. >>> >>> A couple of the benefits of having it like this is to help with viewing >>> git-blame if we introduce new variables and help with backporting fix >>> patch a bit simpler. Mainly I'm just following Felipe's style and keep >>> it consistent in this driver, but I don't think it's a big deal. >> >> *shrug* >> >> If it's the subsystem style I will rewrite it. >> > > Felipe also prefers Reverse Christmas Tree style. that's also part of the coding style and probably documented somewhere -- balbi