Pawel, On 11/05/23 11:00 am, Pawel Laszczak wrote: >> >> +Pawel & Peter >> >> On 09/05/2023 08:58, Ravi Gunasekaran wrote: >>> Hi Roger, >>> >>> On 05/05/23 6:02 pm, Roger Quadros wrote: >>>> Hi Ravi, >>>> >>>> On 05/05/2023 15:13, Ravi Gunasekaran wrote: >>>>> From: Aswath Govindraju <a-govindr...@ti.com> >>>>> >>>>> When the device port is in a low power state [U3/L2/Not Connected], >>>>> accesses to usb device registers may take a long time. This could >>>>> lead to potential core hang when the controller registers are >>>>> accessed after the port is disabled by setting DEVDS field. Setting >>>>> the fast register access bit ensures that the PHY clock is keeping up in >> active state. >>>>> >>>>> Therefore, set fast access bit to ensure the accesses to device >>>>> registers are quick even in low power states. >>>>> >>>>> Signed-off-by: Aswath Govindraju <a-govindr...@ti.com> >>>>> --- >>>>> drivers/usb/cdns3/gadget.c | 4 ++++ >>>>> 1 file changed, 4 insertions(+) >>>>> >>>>> diff --git a/drivers/usb/cdns3/gadget.c b/drivers/usb/cdns3/gadget.c >>>>> index fcaeab9cc1..fddc8c931a 100644 >>>>> --- a/drivers/usb/cdns3/gadget.c >>>>> +++ b/drivers/usb/cdns3/gadget.c >>>>> @@ -2321,6 +2321,9 @@ static void cdns3_gadget_config(struct >> cdns3_device *priv_dev) >>>>> writel(USB_IEN_INIT, ®s->usb_ien); >>>>> writel(USB_CONF_CLK2OFFDS | USB_CONF_L1DS, ®s->usb_conf); >>>>> >>>>> + /* Set the Fast access bit */ >>>>> + writel(PUSB_PWR_FST_REG_ACCESS, &priv_dev->regs->usb_pwr); >>>>> + >>>> >>>> Should this be done in cdns3_gadget_udc_start() so it is symmetric? >>>> >>> >>> cdns3_gadget_config() is called in cdns3_gadget_udc_start() and >>> cdns3_gadget_resume(). These settings seems to be needed during resume >>> as well. >> >> But this bit was never cleared in suspend so why do you need to set it again >> it >> in resume? >> > > In my opinion setting PUSB_PWR_FST_REG_ACCESS is not needed in > cdns3_gadget_resume, but it should not have any negative impact for > driver. The cdns3_gadget_config function seems like a good place for > setting this bit. >
In the suspend(), USB_CONF_CFGRST (register: usb_conf.CFGRST) is set, resetting the USB device configuration. Does this mean PUSB_PWR_FST_REG_ACCESS will also be reset? Regards, Ravi > Regards, > Pawel Laszczak > >> The commit log says that this bit must be kept set in low power states. >> >>> >>>>> cdns3_configure_dmult(priv_dev, NULL); >>>>> >>>>> cdns3_gadget_pullup(&priv_dev->gadget, 1); @@ -2378,6 +2381,7 >> @@ >>>>> static int cdns3_gadget_udc_stop(struct usb_gadget *gadget) >>>>> >>>>> /* disable interrupt for device */ >>>>> writel(0, &priv_dev->regs->usb_ien); >>>>> + writel(0, &priv_dev->regs->usb_pwr); >>>>> writel(USB_CONF_DEVDS, &priv_dev->regs->usb_conf); >>>>> >>>>> return ret; >>>>> >>>>> base-commit: a25dcda452bf6a6de72764a8d990d72e5def643d >>>> >> >> -- >> cheers, >> -roger