diff mbox series

[1/2] usb: mtu3: disable USB2 LPM

Message ID 1593410434-19406-1-git-send-email-chunfeng.yun@mediatek.com (mailing list archive)
State New, archived
Headers show
Series [1/2] usb: mtu3: disable USB2 LPM | expand

Commit Message

Chunfeng Yun (云春峰) June 29, 2020, 6 a.m. UTC
A SuperSpeed device shall include the USB 2.0 extension descriptor
and shall support LPM when operating in USB 2.0 HS mode(see usb3.2
spec9.6.2.1). But we always don't support it, so disable it by
default, otherwise device will enter LPM suspend mode when
connected to Win10 system.

Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
---
 drivers/usb/mtu3/mtu3_core.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Markus Elfring June 29, 2020, 7:30 a.m. UTC | #1
> A SuperSpeed device shall include the USB 2.0 extension descriptor
> and shall support LPM when operating in USB 2.0 HS mode(see usb3.2
> spec9.6.2.1). But we always don't support it, so disable it by
> default, otherwise device will enter LPM suspend mode when
> connected to Win10 system.

How do you think about a wording variant like the following?

   Change description:
   A SuperSpeed device shall include the USB 2.0 extension descriptor
   and shall support Link Power Management when operating in USB 2.0
   High Speed mode. (See also: USB 3.2 specification 9.6.2.1)
   But we do not support it generally. Thus disable this functionality
   by default.
   Otherwise, the device will enter LPM suspend mode when connected
   to Win10 system.


Would you like to add the tag “Fixes” to the commit message?

Regards,
Markus
Peter Chen June 29, 2020, 7:37 a.m. UTC | #2
On Mon, Jun 29, 2020 at 2:04 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
>
> A SuperSpeed device shall include the USB 2.0 extension descriptor
> and shall support LPM when operating in USB 2.0 HS mode(see usb3.2
> spec9.6.2.1). But we always don't support it, so disable it by
> default, otherwise device will enter LPM suspend mode when
> connected to Win10 system.

Linux also supports USB2 LPM. Besides, USB-IF CH9 test will check
LPM support if the device is USB 2.1, how could you deal with it?

Peter

>
> Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> ---
>  drivers/usb/mtu3/mtu3_core.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/usb/mtu3/mtu3_core.c b/drivers/usb/mtu3/mtu3_core.c
> index 9dd0216..50d6a40 100644
> --- a/drivers/usb/mtu3/mtu3_core.c
> +++ b/drivers/usb/mtu3/mtu3_core.c
> @@ -604,6 +604,8 @@ static void mtu3_regs_init(struct mtu3 *mtu)
>         mtu3_clrbits(mbase, U3D_MISC_CTRL, VBUS_FRC_EN | VBUS_ON);
>         /* enable automatical HWRW from L1 */
>         mtu3_setbits(mbase, U3D_POWER_MANAGEMENT, LPM_HRWE);
> +       /* always reject LPM request */
> +       mtu3_setbits(mbase, U3D_POWER_MANAGEMENT, LPM_MODE(1));
>
>         /* use new QMU format when HW version >= 0x1003 */
>         if (mtu->gen2cp)
> --
> 1.9.1
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Greg Kroah-Hartman June 29, 2020, 8:10 a.m. UTC | #3
On Mon, Jun 29, 2020 at 09:30:58AM +0200, Markus Elfring wrote:
> > A SuperSpeed device shall include the USB 2.0 extension descriptor
> > and shall support LPM when operating in USB 2.0 HS mode(see usb3.2
> > spec9.6.2.1). But we always don't support it, so disable it by
> > default, otherwise device will enter LPM suspend mode when
> > connected to Win10 system.
> 
> How do you think about a wording variant like the following?
> 
>    Change description:
>    A SuperSpeed device shall include the USB 2.0 extension descriptor
>    and shall support Link Power Management when operating in USB 2.0
>    High Speed mode. (See also: USB 3.2 specification 9.6.2.1)
>    But we do not support it generally. Thus disable this functionality
>    by default.
>    Otherwise, the device will enter LPM suspend mode when connected
>    to Win10 system.
> 
> 
> Would you like to add the tag “Fixes” to the commit message?
> 
> Regards,
> Markus

Hi,

This is the semi-friendly patch-bot of Greg Kroah-Hartman.

Markus, you seem to have sent a nonsensical or otherwise pointless
review comment to a patch submission on a Linux kernel developer mailing
list.  I strongly suggest that you not do this anymore.  Please do not
bother developers who are actively working to produce patches and
features with comments that, in the end, are a waste of time.

Patch submitter, please ignore Markus's suggestion; you do not need to
follow it at all.  The person/bot/AI that sent it is being ignored by
almost all Linux kernel maintainers for having a persistent pattern of
behavior of producing distracting and pointless commentary, and
inability to adapt to feedback.  Please feel free to also ignore emails
from them.

thanks,

greg k-h's patch email bot
Markus Elfring June 29, 2020, 8:15 a.m. UTC | #4
> A SuperSpeed device shall include the USB 2.0 extension descriptor
…

If you would insist to combine the presented update steps for this software module,
a cover letter would be helpful together with following improvements.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=9ebcfadb0610322ac537dd7aa5d9cbc2b2894c68#n785

Regards,
Markus
Chunfeng Yun (云春峰) June 30, 2020, 7:03 a.m. UTC | #5
On Mon, 2020-06-29 at 15:37 +0800, Peter Chen wrote:
> On Mon, Jun 29, 2020 at 2:04 PM Chunfeng Yun <chunfeng.yun@mediatek.com> wrote:
> >
> > A SuperSpeed device shall include the USB 2.0 extension descriptor
> > and shall support LPM when operating in USB 2.0 HS mode(see usb3.2
> > spec9.6.2.1). But we always don't support it, so disable it by
> > default, otherwise device will enter LPM suspend mode when
> > connected to Win10 system.
> 
> Linux also supports USB2 LPM. Besides, USB-IF CH9 test will check
> LPM support if the device is USB 2.1, how could you deal with it?
Indeed need support it for SS device, I'll check it again, thanks a lot

> 
> Peter
> 
> >
> > Signed-off-by: Chunfeng Yun <chunfeng.yun@mediatek.com>
> > ---
> >  drivers/usb/mtu3/mtu3_core.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/usb/mtu3/mtu3_core.c b/drivers/usb/mtu3/mtu3_core.c
> > index 9dd0216..50d6a40 100644
> > --- a/drivers/usb/mtu3/mtu3_core.c
> > +++ b/drivers/usb/mtu3/mtu3_core.c
> > @@ -604,6 +604,8 @@ static void mtu3_regs_init(struct mtu3 *mtu)
> >         mtu3_clrbits(mbase, U3D_MISC_CTRL, VBUS_FRC_EN | VBUS_ON);
> >         /* enable automatical HWRW from L1 */
> >         mtu3_setbits(mbase, U3D_POWER_MANAGEMENT, LPM_HRWE);
> > +       /* always reject LPM request */
> > +       mtu3_setbits(mbase, U3D_POWER_MANAGEMENT, LPM_MODE(1));
> >
> >         /* use new QMU format when HW version >= 0x1003 */
> >         if (mtu->gen2cp)
> > --
> > 1.9.1
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff mbox series

Patch

diff --git a/drivers/usb/mtu3/mtu3_core.c b/drivers/usb/mtu3/mtu3_core.c
index 9dd0216..50d6a40 100644
--- a/drivers/usb/mtu3/mtu3_core.c
+++ b/drivers/usb/mtu3/mtu3_core.c
@@ -604,6 +604,8 @@  static void mtu3_regs_init(struct mtu3 *mtu)
 	mtu3_clrbits(mbase, U3D_MISC_CTRL, VBUS_FRC_EN | VBUS_ON);
 	/* enable automatical HWRW from L1 */
 	mtu3_setbits(mbase, U3D_POWER_MANAGEMENT, LPM_HRWE);
+	/* always reject LPM request */
+	mtu3_setbits(mbase, U3D_POWER_MANAGEMENT, LPM_MODE(1));
 
 	/* use new QMU format when HW version >= 0x1003 */
 	if (mtu->gen2cp)