| Message ID | 20230514054917.21318-7-quic_kriskura@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add multiport support for DWC3 controllers | expand |
On Sun, May 14, 2023 at 11:19:14AM +0530, Krishna Kurapati wrote: > QCOM SoC SA8295P's tertiary quad port controller supports 2 HS+SS > ports and 2 HS only ports. Add support for configuring PWR_EVENT_IRQ's > for all the ports during suspend/resume. > > Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> > --- > drivers/usb/dwc3/dwc3-qcom.c | 28 ++++++++++++++++++++++------ > 1 file changed, 22 insertions(+), 6 deletions(-) > > diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c > index 959fc925ca7c..7a9bce66295d 100644 > --- a/drivers/usb/dwc3/dwc3-qcom.c > +++ b/drivers/usb/dwc3/dwc3-qcom.c > @@ -37,7 +37,10 @@ > #define PIPE3_PHYSTATUS_SW BIT(3) > #define PIPE_UTMI_CLK_DIS BIT(8) > > -#define PWR_EVNT_IRQ_STAT_REG 0x58 > +#define PWR_EVNT_IRQ1_STAT_REG 0x58 > +#define PWR_EVNT_IRQ2_STAT_REG 0x1dc > +#define PWR_EVNT_IRQ3_STAT_REG 0x228 > +#define PWR_EVNT_IRQ4_STAT_REG 0x238 > #define PWR_EVNT_LPM_IN_L2_MASK BIT(4) > #define PWR_EVNT_LPM_OUT_L2_MASK BIT(5) > > @@ -93,6 +96,13 @@ struct dwc3_qcom { > struct icc_path *icc_path_apps; > }; > > +static u32 pwr_evnt_irq_stat_reg_offset[4] = { > + PWR_EVNT_IRQ1_STAT_REG, > + PWR_EVNT_IRQ2_STAT_REG, > + PWR_EVNT_IRQ3_STAT_REG, > + PWR_EVNT_IRQ4_STAT_REG, > +}; Indentation is off, as I believe Bjorn pointed out. > static inline void dwc3_qcom_setbits(void __iomem *base, u32 offset, u32 val) > { > u32 reg; > @@ -413,13 +423,16 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup) > { > u32 val; > int i, ret; > + struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3); > > if (qcom->is_suspended) > return 0; > > - val = readl(qcom->qscratch_base + PWR_EVNT_IRQ_STAT_REG); > - if (!(val & PWR_EVNT_LPM_IN_L2_MASK)) > - dev_err(qcom->dev, "HS-PHY not in L2\n"); > + for (i = 0; i < dwc->num_usb2_ports; i++) { > + val = readl(qcom->qscratch_base + pwr_evnt_irq_stat_reg_offset[i]); > + if (!(val & PWR_EVNT_LPM_IN_L2_MASK)) > + dev_err(qcom->dev, "HS-PHY%d not in L2\n", i); > + } You need check for NULL dwc as we just discussed and skip the above check if core has not probed yet. When testing this on the X13s I get: dwc3-qcom a4f8800.usb: HS-PHY2 not in L2 for the third port, whose status registers always seems to return zero (e.g. as if we're checking the wrong register?): dwc3-qcom a4f8800.usb: dwc3_qcom_suspend - phy 0, pwr_event_stat = 38103c dwc3-qcom a4f8800.usb: dwc3_qcom_suspend - phy 1, pwr_event_stat = 38103c dwc3-qcom a4f8800.usb: dwc3_qcom_suspend - phy 2, pwr_event_stat = 00 dwc3-qcom a4f8800.usb: dwc3_qcom_suspend - phy 3, pwr_event_stat = 140030 I verified that everything appears to work as expected on sa8295p-adp. Do you have any idea of what may be causing this? Johan
Hi Krishna, On Wed, Jun 07, 2023 at 02:16:37PM +0200, Johan Hovold wrote: > On Sun, May 14, 2023 at 11:19:14AM +0530, Krishna Kurapati wrote: > > static inline void dwc3_qcom_setbits(void __iomem *base, u32 offset, u32 val) > > { > > u32 reg; > > @@ -413,13 +423,16 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup) > > { > > u32 val; > > int i, ret; > > + struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3); > > > > if (qcom->is_suspended) > > return 0; > > > > - val = readl(qcom->qscratch_base + PWR_EVNT_IRQ_STAT_REG); > > - if (!(val & PWR_EVNT_LPM_IN_L2_MASK)) > > - dev_err(qcom->dev, "HS-PHY not in L2\n"); > > + for (i = 0; i < dwc->num_usb2_ports; i++) { > > + val = readl(qcom->qscratch_base + pwr_evnt_irq_stat_reg_offset[i]); > > + if (!(val & PWR_EVNT_LPM_IN_L2_MASK)) > > + dev_err(qcom->dev, "HS-PHY%d not in L2\n", i); > > + } > When testing this on the X13s I get: > > dwc3-qcom a4f8800.usb: HS-PHY2 not in L2 > > for the third port, whose status registers always seems to return zero > (e.g. as if we're checking the wrong register?): > > dwc3-qcom a4f8800.usb: dwc3_qcom_suspend - phy 0, pwr_event_stat = 38103c > dwc3-qcom a4f8800.usb: dwc3_qcom_suspend - phy 1, pwr_event_stat = 38103c > dwc3-qcom a4f8800.usb: dwc3_qcom_suspend - phy 2, pwr_event_stat = 00 > dwc3-qcom a4f8800.usb: dwc3_qcom_suspend - phy 3, pwr_event_stat = 140030 > > I verified that everything appears to work as expected on sa8295p-adp. > > Do you have any idea of what may be causing this? You never replied to this; do you have any idea why the status register for the second port seemingly always read back as 0 on the X13s? Johan
On 6/27/2023 9:13 PM, Johan Hovold wrote: > Hi Krishna, > > On Wed, Jun 07, 2023 at 02:16:37PM +0200, Johan Hovold wrote: >> On Sun, May 14, 2023 at 11:19:14AM +0530, Krishna Kurapati wrote: > >>> static inline void dwc3_qcom_setbits(void __iomem *base, u32 offset, u32 val) >>> { >>> u32 reg; >>> @@ -413,13 +423,16 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup) >>> { >>> u32 val; >>> int i, ret; >>> + struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3); >>> >>> if (qcom->is_suspended) >>> return 0; >>> >>> - val = readl(qcom->qscratch_base + PWR_EVNT_IRQ_STAT_REG); >>> - if (!(val & PWR_EVNT_LPM_IN_L2_MASK)) >>> - dev_err(qcom->dev, "HS-PHY not in L2\n"); >>> + for (i = 0; i < dwc->num_usb2_ports; i++) { >>> + val = readl(qcom->qscratch_base + pwr_evnt_irq_stat_reg_offset[i]); >>> + if (!(val & PWR_EVNT_LPM_IN_L2_MASK)) >>> + dev_err(qcom->dev, "HS-PHY%d not in L2\n", i); >>> + } > >> When testing this on the X13s I get: >> >> dwc3-qcom a4f8800.usb: HS-PHY2 not in L2 >> >> for the third port, whose status registers always seems to return zero >> (e.g. as if we're checking the wrong register?): >> >> dwc3-qcom a4f8800.usb: dwc3_qcom_suspend - phy 0, pwr_event_stat = 38103c >> dwc3-qcom a4f8800.usb: dwc3_qcom_suspend - phy 1, pwr_event_stat = 38103c >> dwc3-qcom a4f8800.usb: dwc3_qcom_suspend - phy 2, pwr_event_stat = 00 >> dwc3-qcom a4f8800.usb: dwc3_qcom_suspend - phy 3, pwr_event_stat = 140030 >> >> I verified that everything appears to work as expected on sa8295p-adp. >> >> Do you have any idea of what may be causing this? > > You never replied to this; do you have any idea why the status register > for the second port seemingly always read back as 0 on the X13s? > > Johan Hi Johan, Missed this mail. This never popped up on my system. So no idea what is different in Lenovo X13s. Might need to check with team internally. Regards, Krishna,
On Mon, Jul 03, 2023 at 12:35:48AM +0530, Krishna Kurapati PSSNV wrote: > On 6/27/2023 9:13 PM, Johan Hovold wrote: > > On Wed, Jun 07, 2023 at 02:16:37PM +0200, Johan Hovold wrote: > >> On Sun, May 14, 2023 at 11:19:14AM +0530, Krishna Kurapati wrote: > >>> - val = readl(qcom->qscratch_base + PWR_EVNT_IRQ_STAT_REG); > >>> - if (!(val & PWR_EVNT_LPM_IN_L2_MASK)) > >>> - dev_err(qcom->dev, "HS-PHY not in L2\n"); > >>> + for (i = 0; i < dwc->num_usb2_ports; i++) { > >>> + val = readl(qcom->qscratch_base + pwr_evnt_irq_stat_reg_offset[i]); > >>> + if (!(val & PWR_EVNT_LPM_IN_L2_MASK)) > >>> + dev_err(qcom->dev, "HS-PHY%d not in L2\n", i); > >>> + } > > > >> When testing this on the X13s I get: > >> > >> dwc3-qcom a4f8800.usb: HS-PHY2 not in L2 > >> > >> for the third port, whose status registers always seems to return zero > >> (e.g. as if we're checking the wrong register?): > >> > >> dwc3-qcom a4f8800.usb: dwc3_qcom_suspend - phy 0, pwr_event_stat = 38103c > >> dwc3-qcom a4f8800.usb: dwc3_qcom_suspend - phy 1, pwr_event_stat = 38103c > >> dwc3-qcom a4f8800.usb: dwc3_qcom_suspend - phy 2, pwr_event_stat = 00 > >> dwc3-qcom a4f8800.usb: dwc3_qcom_suspend - phy 3, pwr_event_stat = 140030 > >> > >> I verified that everything appears to work as expected on sa8295p-adp. > >> > >> Do you have any idea of what may be causing this? > > > > You never replied to this; do you have any idea why the status register > > for the second port seemingly always read back as 0 on the X13s? > Missed this mail. This never popped up on my system. So no idea what > is different in Lenovo X13s. Might need to check with team internally. Did you hear anything back regarding the above? Could it even be that the register offset it not correct for sc8280xp? Johan
On 7/14/2023 2:30 PM, Johan Hovold wrote: > On Mon, Jul 03, 2023 at 12:35:48AM +0530, Krishna Kurapati PSSNV wrote: >> On 6/27/2023 9:13 PM, Johan Hovold wrote: >>> On Wed, Jun 07, 2023 at 02:16:37PM +0200, Johan Hovold wrote: >>>> On Sun, May 14, 2023 at 11:19:14AM +0530, Krishna Kurapati wrote: > >>>>> - val = readl(qcom->qscratch_base + PWR_EVNT_IRQ_STAT_REG); >>>>> - if (!(val & PWR_EVNT_LPM_IN_L2_MASK)) >>>>> - dev_err(qcom->dev, "HS-PHY not in L2\n"); >>>>> + for (i = 0; i < dwc->num_usb2_ports; i++) { >>>>> + val = readl(qcom->qscratch_base + pwr_evnt_irq_stat_reg_offset[i]); >>>>> + if (!(val & PWR_EVNT_LPM_IN_L2_MASK)) >>>>> + dev_err(qcom->dev, "HS-PHY%d not in L2\n", i); >>>>> + } >>> >>>> When testing this on the X13s I get: >>>> >>>> dwc3-qcom a4f8800.usb: HS-PHY2 not in L2 >>>> >>>> for the third port, whose status registers always seems to return zero >>>> (e.g. as if we're checking the wrong register?): >>>> >>>> dwc3-qcom a4f8800.usb: dwc3_qcom_suspend - phy 0, pwr_event_stat = 38103c >>>> dwc3-qcom a4f8800.usb: dwc3_qcom_suspend - phy 1, pwr_event_stat = 38103c >>>> dwc3-qcom a4f8800.usb: dwc3_qcom_suspend - phy 2, pwr_event_stat = 00 >>>> dwc3-qcom a4f8800.usb: dwc3_qcom_suspend - phy 3, pwr_event_stat = 140030 >>>> >>>> I verified that everything appears to work as expected on sa8295p-adp. >>>> >>>> Do you have any idea of what may be causing this? >>> >>> You never replied to this; do you have any idea why the status register >>> for the second port seemingly always read back as 0 on the X13s? > >> Missed this mail. This never popped up on my system. So no idea what >> is different in Lenovo X13s. Might need to check with team internally. > > Did you hear anything back regarding the above? > > Could it even be that the register offset it not correct for sc8280xp? > Hi Johan, No. I rechecked the register offsets and they are proper. (same as what we are using in downstream). Adding Jack and Wesley to help with any suggestions here. Regards, Krishna,
On Fri, Jul 14, 2023 at 04:08:45PM +0530, Krishna Kurapati PSSNV wrote: > On 7/14/2023 2:30 PM, Johan Hovold wrote: > > On Mon, Jul 03, 2023 at 12:35:48AM +0530, Krishna Kurapati PSSNV wrote: > >> On 6/27/2023 9:13 PM, Johan Hovold wrote: > >>> On Wed, Jun 07, 2023 at 02:16:37PM +0200, Johan Hovold wrote: > >>>> On Sun, May 14, 2023 at 11:19:14AM +0530, Krishna Kurapati wrote: > > > >>>>> - val = readl(qcom->qscratch_base + PWR_EVNT_IRQ_STAT_REG); > >>>>> - if (!(val & PWR_EVNT_LPM_IN_L2_MASK)) > >>>>> - dev_err(qcom->dev, "HS-PHY not in L2\n"); > >>>>> + for (i = 0; i < dwc->num_usb2_ports; i++) { > >>>>> + val = readl(qcom->qscratch_base + pwr_evnt_irq_stat_reg_offset[i]); > >>>>> + if (!(val & PWR_EVNT_LPM_IN_L2_MASK)) > >>>>> + dev_err(qcom->dev, "HS-PHY%d not in L2\n", i); > >>>>> + } > >>> > >>>> When testing this on the X13s I get: > >>>> > >>>> dwc3-qcom a4f8800.usb: HS-PHY2 not in L2 > >>>> > >>>> for the third port, whose status registers always seems to return zero > >>>> (e.g. as if we're checking the wrong register?): > >>>> > >>>> dwc3-qcom a4f8800.usb: dwc3_qcom_suspend - phy 0, pwr_event_stat = 38103c > >>>> dwc3-qcom a4f8800.usb: dwc3_qcom_suspend - phy 1, pwr_event_stat = 38103c > >>>> dwc3-qcom a4f8800.usb: dwc3_qcom_suspend - phy 2, pwr_event_stat = 00 > >>>> dwc3-qcom a4f8800.usb: dwc3_qcom_suspend - phy 3, pwr_event_stat = 140030 > >>>> > >>>> I verified that everything appears to work as expected on sa8295p-adp. > >>>> > >>>> Do you have any idea of what may be causing this? > >>> > >>> You never replied to this; do you have any idea why the status register > >>> for the second port seemingly always read back as 0 on the X13s? > > > >> Missed this mail. This never popped up on my system. So no idea what > >> is different in Lenovo X13s. Might need to check with team internally. > > > > Did you hear anything back regarding the above? > > > > Could it even be that the register offset it not correct for sc8280xp? > No. I rechecked the register offsets and they are proper. (same as what > we are using in downstream). > > Adding Jack and Wesley to help with any suggestions here. Still no idea as to why this appears to be broken on sc8280xp and triggers an error on every suspend? Johan
On 21.07.2023 13:16, Johan Hovold wrote: > On Fri, Jul 14, 2023 at 04:08:45PM +0530, Krishna Kurapati PSSNV wrote: >> On 7/14/2023 2:30 PM, Johan Hovold wrote: >>> On Mon, Jul 03, 2023 at 12:35:48AM +0530, Krishna Kurapati PSSNV wrote: >>>> On 6/27/2023 9:13 PM, Johan Hovold wrote: >>>>> On Wed, Jun 07, 2023 at 02:16:37PM +0200, Johan Hovold wrote: >>>>>> On Sun, May 14, 2023 at 11:19:14AM +0530, Krishna Kurapati wrote: >>> >>>>>>> - val = readl(qcom->qscratch_base + PWR_EVNT_IRQ_STAT_REG); >>>>>>> - if (!(val & PWR_EVNT_LPM_IN_L2_MASK)) >>>>>>> - dev_err(qcom->dev, "HS-PHY not in L2\n"); >>>>>>> + for (i = 0; i < dwc->num_usb2_ports; i++) { >>>>>>> + val = readl(qcom->qscratch_base + pwr_evnt_irq_stat_reg_offset[i]); >>>>>>> + if (!(val & PWR_EVNT_LPM_IN_L2_MASK)) >>>>>>> + dev_err(qcom->dev, "HS-PHY%d not in L2\n", i); >>>>>>> + } >>>>> >>>>>> When testing this on the X13s I get: >>>>>> >>>>>> dwc3-qcom a4f8800.usb: HS-PHY2 not in L2 Sidenote, I get this on any Qcom device on any platform I try to enter suspend on, without these MP patches. Konrad
On Fri, Jul 21, 2023 at 02:10:07PM +0200, Konrad Dybcio wrote: > On 21.07.2023 13:16, Johan Hovold wrote: > > On Fri, Jul 14, 2023 at 04:08:45PM +0530, Krishna Kurapati PSSNV wrote: > >> On 7/14/2023 2:30 PM, Johan Hovold wrote: > >>> On Mon, Jul 03, 2023 at 12:35:48AM +0530, Krishna Kurapati PSSNV wrote: > >>>> On 6/27/2023 9:13 PM, Johan Hovold wrote: > >>>>> On Wed, Jun 07, 2023 at 02:16:37PM +0200, Johan Hovold wrote: > >>>>>> On Sun, May 14, 2023 at 11:19:14AM +0530, Krishna Kurapati wrote: > >>> > >>>>>>> - val = readl(qcom->qscratch_base + PWR_EVNT_IRQ_STAT_REG); > >>>>>>> - if (!(val & PWR_EVNT_LPM_IN_L2_MASK)) > >>>>>>> - dev_err(qcom->dev, "HS-PHY not in L2\n"); > >>>>>>> + for (i = 0; i < dwc->num_usb2_ports; i++) { > >>>>>>> + val = readl(qcom->qscratch_base + pwr_evnt_irq_stat_reg_offset[i]); > >>>>>>> + if (!(val & PWR_EVNT_LPM_IN_L2_MASK)) > >>>>>>> + dev_err(qcom->dev, "HS-PHY%d not in L2\n", i); > >>>>>>> + } > >>>>> > >>>>>> When testing this on the X13s I get: > >>>>>> > >>>>>> dwc3-qcom a4f8800.usb: HS-PHY2 not in L2 > Sidenote, I get this on any Qcom device on any platform I try > to enter suspend on, without these MP patches. Ok, that might provide some hint. But on sc8280xp (X13s) we only get it on one of the four MP ports (i.e. on one out of six ports in total). While on sa8295p-adp there are no such errors on any port. Johan
On 21.07.2023 14:54, Johan Hovold wrote: > On Fri, Jul 21, 2023 at 02:10:07PM +0200, Konrad Dybcio wrote: >> On 21.07.2023 13:16, Johan Hovold wrote: >>> On Fri, Jul 14, 2023 at 04:08:45PM +0530, Krishna Kurapati PSSNV wrote: >>>> On 7/14/2023 2:30 PM, Johan Hovold wrote: >>>>> On Mon, Jul 03, 2023 at 12:35:48AM +0530, Krishna Kurapati PSSNV wrote: >>>>>> On 6/27/2023 9:13 PM, Johan Hovold wrote: >>>>>>> On Wed, Jun 07, 2023 at 02:16:37PM +0200, Johan Hovold wrote: >>>>>>>> On Sun, May 14, 2023 at 11:19:14AM +0530, Krishna Kurapati wrote: >>>>> >>>>>>>>> - val = readl(qcom->qscratch_base + PWR_EVNT_IRQ_STAT_REG); >>>>>>>>> - if (!(val & PWR_EVNT_LPM_IN_L2_MASK)) >>>>>>>>> - dev_err(qcom->dev, "HS-PHY not in L2\n"); >>>>>>>>> + for (i = 0; i < dwc->num_usb2_ports; i++) { >>>>>>>>> + val = readl(qcom->qscratch_base + pwr_evnt_irq_stat_reg_offset[i]); >>>>>>>>> + if (!(val & PWR_EVNT_LPM_IN_L2_MASK)) >>>>>>>>> + dev_err(qcom->dev, "HS-PHY%d not in L2\n", i); >>>>>>>>> + } >>>>>>> >>>>>>>> When testing this on the X13s I get: >>>>>>>> >>>>>>>> dwc3-qcom a4f8800.usb: HS-PHY2 not in L2 > >> Sidenote, I get this on any Qcom device on any platform I try >> to enter suspend on, without these MP patches. > > Ok, that might provide some hint. But on sc8280xp (X13s) we only get it > on one of the four MP ports (i.e. on one out of six ports in total). > > While on sa8295p-adp there are no such errors on any port. I've been playing with 8450 and it looks like snps,dis_u2_susphy_quirk causes this error. The downstream tree contains this property and I'm inclined to believe it means that this platforms should define it (as the devicetrees are machine-generated to a degree, AFAIK), especially since this quirk does the exact same thing on a known-working downstream, namely unsetting DWC3_GUSB2PHYCFG_SUSPHY. Digging a bit deeper, dwc3-msm-core [1], the downstream version of dwc3-qcom performs a bit of a dance in a couple of places.. Look for that register name. Unfortunately I have little idea what the "USB2 suspend phy" is.. is it a PHY used in suspend? Is it the suspension of the USB2 PHY? No clue. [1] https://git.codelinaro.org/clo/la/kernel/msm-5.10/-/blob/KERNEL.PLATFORM.1.0.r2-08800-WAIPIOLE.0/drivers/usb/dwc3/dwc3-msm-core.c Konrad
On 8/11/2023 10:18 PM, Konrad Dybcio wrote: > On 21.07.2023 14:54, Johan Hovold wrote: >> On Fri, Jul 21, 2023 at 02:10:07PM +0200, Konrad Dybcio wrote: >>> On 21.07.2023 13:16, Johan Hovold wrote: >>>> On Fri, Jul 14, 2023 at 04:08:45PM +0530, Krishna Kurapati PSSNV wrote: >>>>> On 7/14/2023 2:30 PM, Johan Hovold wrote: >>>>>> On Mon, Jul 03, 2023 at 12:35:48AM +0530, Krishna Kurapati PSSNV wrote: >>>>>>> On 6/27/2023 9:13 PM, Johan Hovold wrote: >>>>>>>> On Wed, Jun 07, 2023 at 02:16:37PM +0200, Johan Hovold wrote: >>>>>>>>> On Sun, May 14, 2023 at 11:19:14AM +0530, Krishna Kurapati wrote: >>>>>> >>>>>>>>>> - val = readl(qcom->qscratch_base + PWR_EVNT_IRQ_STAT_REG); >>>>>>>>>> - if (!(val & PWR_EVNT_LPM_IN_L2_MASK)) >>>>>>>>>> - dev_err(qcom->dev, "HS-PHY not in L2\n"); >>>>>>>>>> + for (i = 0; i < dwc->num_usb2_ports; i++) { >>>>>>>>>> + val = readl(qcom->qscratch_base + pwr_evnt_irq_stat_reg_offset[i]); >>>>>>>>>> + if (!(val & PWR_EVNT_LPM_IN_L2_MASK)) >>>>>>>>>> + dev_err(qcom->dev, "HS-PHY%d not in L2\n", i); >>>>>>>>>> + } >>>>>>>> >>>>>>>>> When testing this on the X13s I get: >>>>>>>>> >>>>>>>>> dwc3-qcom a4f8800.usb: HS-PHY2 not in L2 >> >>> Sidenote, I get this on any Qcom device on any platform I try >>> to enter suspend on, without these MP patches. >> >> Ok, that might provide some hint. But on sc8280xp (X13s) we only get it >> on one of the four MP ports (i.e. on one out of six ports in total). >> >> While on sa8295p-adp there are no such errors on any port. > I've been playing with 8450 and it looks like snps,dis_u2_susphy_quirk > causes this error. > > The downstream tree contains this property and I'm inclined to believe > it means that this platforms should define it (as the devicetrees are > machine-generated to a degree, AFAIK), especially since this quirk does > the exact same thing on a known-working downstream, namely unsetting > DWC3_GUSB2PHYCFG_SUSPHY. > > Digging a bit deeper, dwc3-msm-core [1], the downstream version of dwc3-qcom > performs a bit of a dance in a couple of places.. Look for that register name. > > Unfortunately I have little idea what the "USB2 suspend phy" is.. is it a PHY > used in suspend? Is it the suspension of the USB2 PHY? No clue. > > [1] https://git.codelinaro.org/clo/la/kernel/msm-5.10/-/blob/KERNEL.PLATFORM.1.0.r2-08800-WAIPIOLE.0/drivers/usb/dwc3/dwc3-msm-core.c > The description for that bit (BIT(6)) as per the databook is as follows: --- 6 SUSPENDUSB20 R_W Suspend USB2.0 HS/FS/LS PHY (SusPHY) When set, USB2.0 PHY enters Suspend mode if Suspend conditions are valid. For DRD/OTG configurations, it is recommended that this bit is set to 0 during coreConsultant configuration. If it is set to 1, then the application must clear this bit after power-on reset. Application needs to set it to 1 after the core initialization completes. For all other configurations, this bit can be set to 1 during core configuration. Note: ■ In host mode, on reset, this bit is set to 1. Software can override this bit after reset. ■ In device mode, before issuing any device endpoint command when operating in 2.0 speeds, disable this bit and enable it after the command completes. If you issue a command without disabling this bit when the device is in L2 state and if mac2_clk (utmi_clk/ulpi_clk) is gated off, the command will not get completed --- "L2" is the term we say when PHY is suspended, i.e., the main PLL is shut off. Internally, I was able to find out that there are several conditions where phy can fail to enter L2. The entry into L2 is controlled by the USB controller itself, but can be limited by toggling GUSB2PHY SUSPENDABLE bit. if that bit is 0 then controller won't place HSPHY into L2. For the failure to enter L2, there can be several situations, like there may be some pending line state change that happened on the bus. But Johan's error seems to be different as the register itself reads zero which I don't understand. Regards, Krishna,
diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c index 959fc925ca7c..7a9bce66295d 100644 --- a/drivers/usb/dwc3/dwc3-qcom.c +++ b/drivers/usb/dwc3/dwc3-qcom.c @@ -37,7 +37,10 @@ #define PIPE3_PHYSTATUS_SW BIT(3) #define PIPE_UTMI_CLK_DIS BIT(8) -#define PWR_EVNT_IRQ_STAT_REG 0x58 +#define PWR_EVNT_IRQ1_STAT_REG 0x58 +#define PWR_EVNT_IRQ2_STAT_REG 0x1dc +#define PWR_EVNT_IRQ3_STAT_REG 0x228 +#define PWR_EVNT_IRQ4_STAT_REG 0x238 #define PWR_EVNT_LPM_IN_L2_MASK BIT(4) #define PWR_EVNT_LPM_OUT_L2_MASK BIT(5) @@ -93,6 +96,13 @@ struct dwc3_qcom { struct icc_path *icc_path_apps; }; +static u32 pwr_evnt_irq_stat_reg_offset[4] = { + PWR_EVNT_IRQ1_STAT_REG, + PWR_EVNT_IRQ2_STAT_REG, + PWR_EVNT_IRQ3_STAT_REG, + PWR_EVNT_IRQ4_STAT_REG, +}; + static inline void dwc3_qcom_setbits(void __iomem *base, u32 offset, u32 val) { u32 reg; @@ -413,13 +423,16 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup) { u32 val; int i, ret; + struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3); if (qcom->is_suspended) return 0; - val = readl(qcom->qscratch_base + PWR_EVNT_IRQ_STAT_REG); - if (!(val & PWR_EVNT_LPM_IN_L2_MASK)) - dev_err(qcom->dev, "HS-PHY not in L2\n"); + for (i = 0; i < dwc->num_usb2_ports; i++) { + val = readl(qcom->qscratch_base + pwr_evnt_irq_stat_reg_offset[i]); + if (!(val & PWR_EVNT_LPM_IN_L2_MASK)) + dev_err(qcom->dev, "HS-PHY%d not in L2\n", i); + } for (i = qcom->num_clocks - 1; i >= 0; i--) clk_disable_unprepare(qcom->clks[i]); @@ -446,6 +459,7 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom, bool wakeup) { int ret; int i; + struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3); if (!qcom->is_suspended) return 0; @@ -467,8 +481,10 @@ static int dwc3_qcom_resume(struct dwc3_qcom *qcom, bool wakeup) dev_warn(qcom->dev, "failed to enable interconnect: %d\n", ret); /* Clear existing events from PHY related to L2 in/out */ - dwc3_qcom_setbits(qcom->qscratch_base, PWR_EVNT_IRQ_STAT_REG, - PWR_EVNT_LPM_IN_L2_MASK | PWR_EVNT_LPM_OUT_L2_MASK); + for (i = 0; i < dwc->num_usb2_ports; i++) + dwc3_qcom_setbits(qcom->qscratch_base, + pwr_evnt_irq_stat_reg_offset[i], + PWR_EVNT_LPM_IN_L2_MASK | PWR_EVNT_LPM_OUT_L2_MASK); qcom->is_suspended = false;
QCOM SoC SA8295P's tertiary quad port controller supports 2 HS+SS ports and 2 HS only ports. Add support for configuring PWR_EVENT_IRQ's for all the ports during suspend/resume. Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com> --- drivers/usb/dwc3/dwc3-qcom.c | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-)