All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Claus H. Stovgaard" <cst@phaseone.com>
To: Anurag Kumar Vulisha <anuragku@xilinx.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Felipe Balbi <balbi@kernel.org>
Cc: "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"v.anuragkumar@gmail.com" <v.anuragkumar@gmail.com>,
	Rob Weber <rob@gnarbox.com>
Subject: [0/3] usb: gadget: Add support for disabling U1 and U2 entries
Date: Fri, 3 May 2019 15:52:03 +0200	[thread overview]
Message-ID: <1556891523.24062.31.camel@phaseone.com> (raw)

Hi Anurag

On Fri, 2019-05-03 at 07:34 +0000, Anurag Kumar Vulisha wrote:
> Hi Claus,
> Thanks for testing and voting for this patch.

I have first tested your patches today. My test setup is a ZynqMP
device, running kernel 4.14 (Xilinx version) with your patches.
I then create an overlay for the devicetree with the new parameters,
and unbind/bind the dwc3 driver.

Next I have a host running Windows 10 and a MacBook pro with Type-C
ports. For logging the communication I use a Total Phase Beagle USB3
5000 V2 analyzer.

The test showed that OS-X does as expected. When BOS descriptor
(bU1DevExtLat and bU2DevExtLat) returns 0, it does not enable LPM.

Windows 10 on the other hand does not, and even though it received 0 as
bU1DevExtLat and bU2DevExtLat it send Set Sel with U1SEL 85 us, U1PEL 0
us, U2SEL 85 us and U2PEL 0 us.
Next the Windows 10 host sends the U1 Enable and the U2 enable as Set
Device Feature, resulting in the system entering U2.

> > 
> > Just today I was making another solution for this feature, using
> > the
> > configfs instead of the devicetree. Though thinks your solution is
> > better, as it uses the U1DevExitLat and U2DevExitLat instead. I
> > just
> > added my solution to the bottem of the mail for reference.
> > 
> > [1] https://www.spinics.net/lists/linux-usb/msg179393.html
> > 
> Your approach below is also good, but you are just avoiding the
> gadget dwc3
> controller from entering into U1 and U2 states by disabling the
> ACCEPTU1ENA
> and ACCEPTU2ENA bits in DCTL but not preventing the host from sending
> the
> LG0_U1 and LGO_U2 link command signaling to the gadget. The host will
> keep
> on trying to get the link into U1 or U2 by sending LGO_U1 or LGO_U2
> and the
> gadget rejects these signals by sending LXU link command. To avoid
> this extra
> overhead I thought that sending zero  value in the BOS descriptor's
> U1DevExitLat and U2DevExitLat fields would be the best option. Host
> on seeing
> U 1 & U2 Exit Latencies doesn't initiate LPM U1 and U2 commands.
> 
> Thanks,
> Anurag Kumar Vulisha

Correct that it does not prevent the host from sending LG0_U1 and
LG0_U2, and there is your solution better on hosts using the BOS
descriptor for disabling LPM. So based on my test with Windows 10, I
think we should combine the solutions. To prevent LG0_U1/LG0_U2 when
possible and still being able to completely disable U1/U2.

Regarding interface for controlling it. I am very novice regarding
Linux kernel development, but would think the BOS descriptor control
would be better from a configfs interface then devicetree as I don't
see BOS descriptor as hardware specific. I am more in doubt about the
forcing of U1/U2 as I did with setting hardware registers, as it
control hardware registers. So will like to hear from other more
experienced developers.

Regards
Claus

WARNING: multiple messages have this Message-ID (diff)
From: "Claus H. Stovgaard" <cst@phaseone.com>
To: Anurag Kumar Vulisha <anuragku@xilinx.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Rob Herring <robh+dt@kernel.org>,
	"Mark Rutland" <mark.rutland@arm.com>,
	Felipe Balbi <balbi@kernel.org>
Cc: "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
	"v.anuragkumar@gmail.com" <v.anuragkumar@gmail.com>,
	Rob Weber <rob@gnarbox.com>
Subject: Re: [PATCH 0/3] usb: gadget: Add support for disabling U1 and U2 entries
Date: Fri, 3 May 2019 15:52:03 +0200	[thread overview]
Message-ID: <1556891523.24062.31.camel@phaseone.com> (raw)
Message-ID: <20190503135203.VWNheO0BKME7gKyq6J7SHVnks317zv1oicKWFLCJE20@z> (raw)
In-Reply-To: <BYAPR02MB55917AF9083F9718B713FBB4A7350@BYAPR02MB5591.namprd02.prod.outlook.com>

Hi Anurag

On Fri, 2019-05-03 at 07:34 +0000, Anurag Kumar Vulisha wrote:
> Hi Claus,
> Thanks for testing and voting for this patch.

I have first tested your patches today. My test setup is a ZynqMP
device, running kernel 4.14 (Xilinx version) with your patches.
I then create an overlay for the devicetree with the new parameters,
and unbind/bind the dwc3 driver.

Next I have a host running Windows 10 and a MacBook pro with Type-C
ports. For logging the communication I use a Total Phase Beagle USB3
5000 V2 analyzer.

The test showed that OS-X does as expected. When BOS descriptor
(bU1DevExtLat and bU2DevExtLat) returns 0, it does not enable LPM.

Windows 10 on the other hand does not, and even though it received 0 as
bU1DevExtLat and bU2DevExtLat it send Set Sel with U1SEL 85 us, U1PEL 0
us, U2SEL 85 us and U2PEL 0 us.
Next the Windows 10 host sends the U1 Enable and the U2 enable as Set
Device Feature, resulting in the system entering U2.

> > 
> > Just today I was making another solution for this feature, using
> > the
> > configfs instead of the devicetree. Though thinks your solution is
> > better, as it uses the U1DevExitLat and U2DevExitLat instead. I
> > just
> > added my solution to the bottem of the mail for reference.
> > 
> > [1] https://www.spinics.net/lists/linux-usb/msg179393.html
> > 
> Your approach below is also good, but you are just avoiding the
> gadget dwc3
> controller from entering into U1 and U2 states by disabling the
> ACCEPTU1ENA
> and ACCEPTU2ENA bits in DCTL but not preventing the host from sending
> the
> LG0_U1 and LGO_U2 link command signaling to the gadget. The host will
> keep
> on trying to get the link into U1 or U2 by sending LGO_U1 or LGO_U2
> and the
> gadget rejects these signals by sending LXU link command. To avoid
> this extra
> overhead I thought that sending zero  value in the BOS descriptor's
> U1DevExitLat and U2DevExitLat fields would be the best option. Host
> on seeing
> U 1 & U2 Exit Latencies doesn't initiate LPM U1 and U2 commands.
> 
> Thanks,
> Anurag Kumar Vulisha

Correct that it does not prevent the host from sending LG0_U1 and
LG0_U2, and there is your solution better on hosts using the BOS
descriptor for disabling LPM. So based on my test with Windows 10, I
think we should combine the solutions. To prevent LG0_U1/LG0_U2 when
possible and still being able to completely disable U1/U2.

Regarding interface for controlling it. I am very novice regarding
Linux kernel development, but would think the BOS descriptor control
would be better from a configfs interface then devicetree as I don't
see BOS descriptor as hardware specific. I am more in doubt about the
forcing of U1/U2 as I did with setting hardware registers, as it
control hardware registers. So will like to hear from other more
experienced developers.

Regards
Claus

             reply	other threads:[~2019-05-03 13:52 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-03 13:52 Claus H. Stovgaard [this message]
2019-05-03 13:52 ` [PATCH 0/3] usb: gadget: Add support for disabling U1 and U2 entries Claus H. Stovgaard
  -- strict thread matches above, loose matches on Subject: below --
2019-05-03  7:34 [0/3] " Anurag Kumar Vulisha
2019-05-03  7:34 ` [PATCH 0/3] " Anurag Kumar Vulisha
2019-05-02 21:36 [0/3] " claus.stovgaard
2019-05-02 21:36 ` [PATCH 0/3] " claus.stovgaard
2019-05-02 10:20 [2/3] usb: gadget: send usb_gadget as an argument in get_config_params Anurag Kumar Vulisha
2019-05-02 10:20 ` [PATCH 2/3] " Anurag Kumar Vulisha
2019-05-02 10:20 ` Anurag Kumar Vulisha
2019-05-02 10:20 [1/3] doc: dt: bindings: usb: dwc3: Update entries for disabling U1 and U2 Anurag Kumar Vulisha
2019-05-02 10:20 ` [PATCH 1/3] " Anurag Kumar Vulisha
2019-05-02 10:20 ` Anurag Kumar Vulisha
2019-05-02 10:20 [PATCH 0/3] usb: gadget: Add support for disabling U1 and U2 entries Anurag Kumar Vulisha
2019-05-02 10:20 ` Anurag Kumar Vulisha
2019-05-02 10:20 ` [3/3] usb: dwc3: " Anurag Kumar Vulisha
2019-05-02 10:20   ` [PATCH 3/3] " Anurag Kumar Vulisha
2019-05-02 10:20   ` Anurag Kumar Vulisha
2019-05-06 19:21   ` Thinh Nguyen
2019-05-06 20:58     ` Claus H. Stovgaard
2019-05-07  9:50       ` Anurag Kumar Vulisha
2019-05-07 13:17         ` Claus H. Stovgaard
2019-05-07 14:09           ` Anurag Kumar Vulisha
2019-05-07 18:42         ` Thinh Nguyen
2019-05-07  9:46     ` Anurag Kumar Vulisha

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1556891523.24062.31.camel@phaseone.com \
    --to=cst@phaseone.com \
    --cc=anuragku@xilinx.com \
    --cc=balbi@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=rob@gnarbox.com \
    --cc=robh+dt@kernel.org \
    --cc=v.anuragkumar@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.