> >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? >
Setting usb_conf.CFGRST doesn't reset the PUSB_PWR_FST_REG_ACCESS. Regards, Pawel >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