Hi, > -----Original Message----- > From: Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> > Sent: Wednesday, June 1, 2022 12:48 AM > To: Jun Li (OSS) <jun.li@xxxxxxxxxxx>; Thinh Nguyen > <Thinh.Nguyen@xxxxxxxxxxxx>; gregkh@xxxxxxxxxxxxxxxxxxx; > balbi@xxxxxxxxxx > Cc: linux-usb@xxxxxxxxxxxxxxx; J.D. Yue <jindong.yue@xxxxxxx> > Subject: Re: [PATCH] usb: dwc3: add power down scale setting > > Jun Li (OSS) wrote: > > > > > >> -----Original Message----- > >> From: Thinh Nguyen <Thinh.Nguyen@xxxxxxxxxxxx> > >> Sent: Saturday, May 28, 2022 9:46 AM > >> To: Jun Li <jun.li@xxxxxxx>; gregkh@xxxxxxxxxxxxxxxxxxx; > >> balbi@xxxxxxxxxx > >> Cc: linux-usb@xxxxxxxxxxxxxxx; J.D. Yue <jindong.yue@xxxxxxx> > >> Subject: Re: [PATCH] usb: dwc3: add power down scale setting > >> > >> Thinh Nguyen wrote: > >>> Li Jun wrote: > >>>> Some SoC(e.g NXP imx8MQ) may have a wrong default power down scale > >>>> setting so need init it to be the correct value, the power down > >>>> scale setting description in DWC3 databook: > >>>> > >>>> Power Down Scale (PwrDnScale) > >>>> The USB3 suspend_clk input replaces pipe3_rx_pclk as a clock source > >>>> to a small part of the USB3 core that operates when the SS PHY is > >>>> in its lowest power (P3) state, and therefore does not provide a clock. > >>>> The Power Down Scale field specifies how many suspend_clk periods > >>>> fit into a 16 kHz clock period. When performing the division, round > >>>> up the remainder. > >>>> For example, when using an 8-bit/16-bit/32-bit PHY and 25-MHz > >>>> Suspend clock, Power Down Scale = 25000 kHz/16 kHz = 13'd1563 > >>>> (rounder up) > >>>> > >>>> So use the suspend clock rate to calculate it. > >> > >> Also, shouldn't the value selected be the max_rate of the suspend > >> clock and not just the value from clk_get_rate()? > > > > In my case, the suspend_clk is fixed, seems max rate is only Used by > > clock provider and there is no API to get max_rate for clock users. > > > > Seems like only the user/designer of the device knows the max rate for > the suspend_clk if the clock frequency varies. We may not want to > inadvertently overwrite the correct/tested default value for all > devices. Maybe we also need a new device property to inform the > controller of the power down scale value and give the user an option to > override the calculated value here. Understood, considering this is a rare case(wrong default value), and DT maintainer Rob does not like continue expand dwc3 huge property list for this kind of soc level thing, instead, propose implies by compatible, do you think is it acceptable to use compatible check like below for this? if (of_device_is_compatible(node, "fsl,imx8mq-dwc3")) thanks Li Jun > > Thanks, > Thinh
