Linux-USB Archive on lore.kernel.org
 help / color / Atom feed
* Re: Policy to keep USB ports powered in low-power states
       [not found] <CAHX4x86QCrkrnPEfrup8k96wyqg=QR_vgetYLqP1AEa02fx1vw@mail.gmail.com>
@ 2019-08-13  6:02 ` Greg Kroah-Hartman
  2019-08-14 20:12   ` Nick Crews
  0 siblings, 1 reply; 7+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-13  6:02 UTC (permalink / raw)
  To: Nick Crews; +Cc: linux-usb, Enric Balletbo i Serra, linux-kernel, Daniel Kurtz

On Mon, Aug 12, 2019 at 06:08:43PM -0600, Nick Crews wrote:
> Hi Greg!

Hi!

First off, please fix your email client to not send html so that vger
does not reject your messages :)

> I am working on a Chrome OS device that supports a policy called "USB Power
> Share," which allows users to turn the laptop into a charge pack for their
> phone. When the policy is enabled, power will be supplied to the USB ports
> even when the system is in low power states such as S3 and S5. When
> disabled, then no power will be supplied in S3 and S5. I wrote a driver
> <https://lore.kernel.org/patchwork/patch/1062995/> for this already as part
> of drivers/platform/chrome/, but Enric Balletbo i Serra, the maintainer,
> had the reasonable suggestion of trying to move this into the USB subsystem.

Correct suggestion.

> Has anything like this been done before? Do you have any preliminary
> thoughts on this before I start writing code? A few things that I haven't
> figured out yet:
> - How to make this feature only available on certain devices. Using device
> tree? Kconfig? Making a separate driver just for this device that plugs
> into the USB core?
> - The feature is only supported on some USB ports, so we need a way of
> filtering on a per-port basis.

Look at the drivers/usb/typec/ code, I think that should do everything
you need here as this is a typec standard functionality, right?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Policy to keep USB ports powered in low-power states
  2019-08-13  6:02 ` Policy to keep USB ports powered in low-power states Greg Kroah-Hartman
@ 2019-08-14 20:12   ` Nick Crews
  2019-08-14 21:20     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 7+ messages in thread
From: Nick Crews @ 2019-08-14 20:12 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: linux-usb, Enric Balletbo i Serra, linux-kernel, Daniel Kurtz

Thanks for the fast response!

On Tue, Aug 13, 2019 at 12:02 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Mon, Aug 12, 2019 at 06:08:43PM -0600, Nick Crews wrote:
> > Hi Greg!
>
> Hi!
>
> First off, please fix your email client to not send html so that vger
> does not reject your messages :)

Thanks, should be good now.

>
> > I am working on a Chrome OS device that supports a policy called "USB Power
> > Share," which allows users to turn the laptop into a charge pack for their
> > phone. When the policy is enabled, power will be supplied to the USB ports
> > even when the system is in low power states such as S3 and S5. When
> > disabled, then no power will be supplied in S3 and S5. I wrote a driver
> > <https://lore.kernel.org/patchwork/patch/1062995/> for this already as part
> > of drivers/platform/chrome/, but Enric Balletbo i Serra, the maintainer,
> > had the reasonable suggestion of trying to move this into the USB subsystem.
>
> Correct suggestion.
>
> > Has anything like this been done before? Do you have any preliminary
> > thoughts on this before I start writing code? A few things that I haven't
> > figured out yet:
> > - How to make this feature only available on certain devices. Using device
> > tree? Kconfig? Making a separate driver just for this device that plugs
> > into the USB core?
> > - The feature is only supported on some USB ports, so we need a way of
> > filtering on a per-port basis.
>
> Look at the drivers/usb/typec/ code, I think that should do everything
> you need here as this is a typec standard functionality, right?

Unfortunately this is for USB 2.0 ports, so it's not type-C.
Is the type-C code still worth looking at?

>
> thanks,
>
> greg k-h

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Policy to keep USB ports powered in low-power states
  2019-08-14 20:12   ` Nick Crews
@ 2019-08-14 21:20     ` Greg Kroah-Hartman
  2019-08-15  0:08       ` Nick Crews
  0 siblings, 1 reply; 7+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-14 21:20 UTC (permalink / raw)
  To: Nick Crews; +Cc: linux-usb, Enric Balletbo i Serra, linux-kernel, Daniel Kurtz

On Wed, Aug 14, 2019 at 02:12:07PM -0600, Nick Crews wrote:
> Thanks for the fast response!
> 
> On Tue, Aug 13, 2019 at 12:02 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Mon, Aug 12, 2019 at 06:08:43PM -0600, Nick Crews wrote:
> > > Hi Greg!
> >
> > Hi!
> >
> > First off, please fix your email client to not send html so that vger
> > does not reject your messages :)
> 
> Thanks, should be good now.
> 
> >
> > > I am working on a Chrome OS device that supports a policy called "USB Power
> > > Share," which allows users to turn the laptop into a charge pack for their
> > > phone. When the policy is enabled, power will be supplied to the USB ports
> > > even when the system is in low power states such as S3 and S5. When
> > > disabled, then no power will be supplied in S3 and S5. I wrote a driver
> > > <https://lore.kernel.org/patchwork/patch/1062995/> for this already as part
> > > of drivers/platform/chrome/, but Enric Balletbo i Serra, the maintainer,
> > > had the reasonable suggestion of trying to move this into the USB subsystem.
> >
> > Correct suggestion.
> >
> > > Has anything like this been done before? Do you have any preliminary
> > > thoughts on this before I start writing code? A few things that I haven't
> > > figured out yet:
> > > - How to make this feature only available on certain devices. Using device
> > > tree? Kconfig? Making a separate driver just for this device that plugs
> > > into the USB core?
> > > - The feature is only supported on some USB ports, so we need a way of
> > > filtering on a per-port basis.
> >
> > Look at the drivers/usb/typec/ code, I think that should do everything
> > you need here as this is a typec standard functionality, right?
> 
> Unfortunately this is for USB 2.0 ports, so it's not type-C.
> Is the type-C code still worth looking at?

If this is for USB 2, does it use the "non-standard" hub commands to
turn on and off power?  If so, why not just use the usbreset userspace
program for that?

And how are you turning a USB 2 port into a power source?  That feels
really odd given the spec.  Is this part of the standard somewhere or
just a firmware/hardware hack that you are adding to a device?

Is there some port information in the firmware that describes this
functionality?  If so, can you expose it through sysfs to the port that
way?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Policy to keep USB ports powered in low-power states
  2019-08-14 21:20     ` Greg Kroah-Hartman
@ 2019-08-15  0:08       ` Nick Crews
  2019-08-15 23:42         ` Duncan Laurie
  0 siblings, 1 reply; 7+ messages in thread
From: Nick Crews @ 2019-08-15  0:08 UTC (permalink / raw)
  To: Duncan Laurie
  Cc: linux-usb, Enric Balletbo i Serra, linux-kernel, Daniel Kurtz,
	Greg Kroah-Hartman

Adding Duncan Laurie who I think has some more intimate knowledge
of how this is implemented in HW. Duncan, could you correct or elaborate
on my answers below as you see fit? Also, sorry if I make some beginner
mistakes here, I'm just getting familiar with the USB subsystem, and thanks for
your patience.

On Wed, Aug 14, 2019 at 3:20 PM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Wed, Aug 14, 2019 at 02:12:07PM -0600, Nick Crews wrote:
> > Thanks for the fast response!
> >
> > On Tue, Aug 13, 2019 at 12:02 AM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Mon, Aug 12, 2019 at 06:08:43PM -0600, Nick Crews wrote:
> > > > Hi Greg!
> > >
> > > Hi!
> > >
> > > First off, please fix your email client to not send html so that vger
> > > does not reject your messages :)
> >
> > Thanks, should be good now.
> >
> > >
> > > > I am working on a Chrome OS device that supports a policy called "USB Power
> > > > Share," which allows users to turn the laptop into a charge pack for their
> > > > phone. When the policy is enabled, power will be supplied to the USB ports
> > > > even when the system is in low power states such as S3 and S5. When
> > > > disabled, then no power will be supplied in S3 and S5. I wrote a driver
> > > > <https://lore.kernel.org/patchwork/patch/1062995/> for this already as part
> > > > of drivers/platform/chrome/, but Enric Balletbo i Serra, the maintainer,
> > > > had the reasonable suggestion of trying to move this into the USB subsystem.
> > >
> > > Correct suggestion.
> > >
> > > > Has anything like this been done before? Do you have any preliminary
> > > > thoughts on this before I start writing code? A few things that I haven't
> > > > figured out yet:
> > > > - How to make this feature only available on certain devices. Using device
> > > > tree? Kconfig? Making a separate driver just for this device that plugs
> > > > into the USB core?
> > > > - The feature is only supported on some USB ports, so we need a way of
> > > > filtering on a per-port basis.
> > >
> > > Look at the drivers/usb/typec/ code, I think that should do everything
> > > you need here as this is a typec standard functionality, right?
> >
> > Unfortunately this is for USB 2.0 ports, so it's not type-C.
> > Is the type-C code still worth looking at?
>
> If this is for USB 2, does it use the "non-standard" hub commands to
> turn on and off power?  If so, why not just use the usbreset userspace
> program for that?

It does not use the standard hub commands. The USB ports are controlled
by an Embedded Controller (EC), so to control this policy we send a command
to the EC. Since the command to send to the EC is very specific, this would need
to go into a "hub driver" unique for these Wilco devices. We would make it so
that the normal hub registration is intercepted by something that sees this is a
Wilco device, and instead register the hub as a "wilco-hub", which has its own
special "power_share" sysfs attribute, but still is treated as a normal USB hub
otherwise?

>
> And how are you turning a USB 2 port into a power source?  That feels
> really odd given the spec.  Is this part of the standard somewhere or
> just a firmware/hardware hack that you are adding to a device?

The EC twiddles something in the port' HW so that the port turns into a
DCP (Dedicated Charging Port) and only supplies power, not data. So I
think yes, this is a bit of a hack that does not conform to the spec.

>
> Is there some port information in the firmware that describes this
> functionality?  If so, can you expose it through sysfs to the port that
> way?

[I'm not sure I'm answering your question, but] I believe that we could
make the BIOS firmware describe the USB ports' capabilities, and the
kernel's behavior would be gated upon what the firmware reports. I see
that struct usb_port already contains a "quirks" field, should we add a
POWER_SHARE quirk to include/linux/usb/quirks.h? I would guess that
should that should be reserved for quirks shared between many USB
devices/hubs?

Thanks,
Nick

>
> thanks,
>
> greg k-h

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Policy to keep USB ports powered in low-power states
  2019-08-15  0:08       ` Nick Crews
@ 2019-08-15 23:42         ` Duncan Laurie
  2019-08-16  9:12           ` Greg Kroah-Hartman
  0 siblings, 1 reply; 7+ messages in thread
From: Duncan Laurie @ 2019-08-15 23:42 UTC (permalink / raw)
  To: Nick Crews
  Cc: linux-usb, Enric Balletbo i Serra, linux-kernel, Daniel Kurtz,
	Greg Kroah-Hartman

On Wed, Aug 14, 2019 at 6:08 PM Nick Crews <ncrews@chromium.org> wrote:
>
> Adding Duncan Laurie who I think has some more intimate knowledge
> of how this is implemented in HW. Duncan, could you correct or elaborate
> on my answers below as you see fit? Also, sorry if I make some beginner
> mistakes here, I'm just getting familiar with the USB subsystem, and thanks for
> your patience.
>
> On Wed, Aug 14, 2019 at 3:20 PM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Aug 14, 2019 at 02:12:07PM -0600, Nick Crews wrote:
> > > Thanks for the fast response!
> > >
> > > On Tue, Aug 13, 2019 at 12:02 AM Greg Kroah-Hartman
> > > <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Mon, Aug 12, 2019 at 06:08:43PM -0600, Nick Crews wrote:
> > > > > Hi Greg!
> > > >
> > > > Hi!
> > > >
> > > > First off, please fix your email client to not send html so that vger
> > > > does not reject your messages :)
> > >
> > > Thanks, should be good now.
> > >
> > > >
> > > > > I am working on a Chrome OS device that supports a policy called "USB Power
> > > > > Share," which allows users to turn the laptop into a charge pack for their
> > > > > phone. When the policy is enabled, power will be supplied to the USB ports
> > > > > even when the system is in low power states such as S3 and S5. When
> > > > > disabled, then no power will be supplied in S3 and S5. I wrote a driver
> > > > > <https://lore.kernel.org/patchwork/patch/1062995/> for this already as part
> > > > > of drivers/platform/chrome/, but Enric Balletbo i Serra, the maintainer,
> > > > > had the reasonable suggestion of trying to move this into the USB subsystem.
> > > >
> > > > Correct suggestion.
> > > >
> > > > > Has anything like this been done before? Do you have any preliminary
> > > > > thoughts on this before I start writing code? A few things that I haven't
> > > > > figured out yet:
> > > > > - How to make this feature only available on certain devices. Using device
> > > > > tree? Kconfig? Making a separate driver just for this device that plugs
> > > > > into the USB core?
> > > > > - The feature is only supported on some USB ports, so we need a way of
> > > > > filtering on a per-port basis.
> > > >
> > > > Look at the drivers/usb/typec/ code, I think that should do everything
> > > > you need here as this is a typec standard functionality, right?
> > >
> > > Unfortunately this is for USB 2.0 ports, so it's not type-C.
> > > Is the type-C code still worth looking at?
> >
> > If this is for USB 2, does it use the "non-standard" hub commands to
> > turn on and off power?  If so, why not just use the usbreset userspace
> > program for that?
>
> It does not use the standard hub commands. The USB ports are controlled
> by an Embedded Controller (EC), so to control this policy we send a command
> to the EC. Since the command to send to the EC is very specific, this would need
> to go into a "hub driver" unique for these Wilco devices. We would make it so
> that the normal hub registration is intercepted by something that sees this is a
> Wilco device, and instead register the hub as a "wilco-hub", which has its own
> special "power_share" sysfs attribute, but still is treated as a normal USB hub
> otherwise?
>


I would say it is somewhat similar to the USB port power control which
eventually calls into usb_acpi_set_power_state() but in this case it only
affects the behavior when the system is NOT running.

This design has a standalone USB charge power controller on the board
that passes through the USB2 D+/D- pins from one port and is able to do
BC1.2 negotiation when the host controller is not powered, assuming
the chip has been enabled by the Embedded Controller.


>
> >
> > And how are you turning a USB 2 port into a power source?  That feels
> > really odd given the spec.  Is this part of the standard somewhere or
> > just a firmware/hardware hack that you are adding to a device?
>
> The EC twiddles something in the port' HW so that the port turns into a
> DCP (Dedicated Charging Port) and only supplies power, not data. So I
> think yes, this is a bit of a hack that does not conform to the spec.
>
> >
> > Is there some port information in the firmware that describes this
> > functionality?  If so, can you expose it through sysfs to the port that
> > way?
>
> [I'm not sure I'm answering your question, but] I believe that we could
> make the BIOS firmware describe the USB ports' capabilities, and the
> kernel's behavior would be gated upon what the firmware reports. I see
> that struct usb_port already contains a "quirks" field, should we add a
> POWER_SHARE quirk to include/linux/usb/quirks.h? I would guess that
> should that should be reserved for quirks shared between many USB
> devices/hubs?
>


We could add a Device Property to the affected USB port in ACPI and
describe it that way, similar to other properties like 'vcc-supply', 'clocks',
'vbus-detect', etc and hook it into the phy-generic driver.

However I'm not clear on whether the phy driver binding works with XHCI
when using ACPI, so this may not be an appropriate place either.

-duncan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Policy to keep USB ports powered in low-power states
  2019-08-15 23:42         ` Duncan Laurie
@ 2019-08-16  9:12           ` Greg Kroah-Hartman
  2019-08-16 17:02             ` Duncan Laurie
  0 siblings, 1 reply; 7+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-16  9:12 UTC (permalink / raw)
  To: Duncan Laurie
  Cc: Nick Crews, linux-usb, Enric Balletbo i Serra, linux-kernel,
	Daniel Kurtz

On Thu, Aug 15, 2019 at 05:42:05PM -0600, Duncan Laurie wrote:
> On Wed, Aug 14, 2019 at 6:08 PM Nick Crews <ncrews@chromium.org> wrote:
> >
> > Adding Duncan Laurie who I think has some more intimate knowledge
> > of how this is implemented in HW. Duncan, could you correct or elaborate
> > on my answers below as you see fit? Also, sorry if I make some beginner
> > mistakes here, I'm just getting familiar with the USB subsystem, and thanks for
> > your patience.
> >
> > On Wed, Aug 14, 2019 at 3:20 PM Greg Kroah-Hartman
> > <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Wed, Aug 14, 2019 at 02:12:07PM -0600, Nick Crews wrote:
> > > > Thanks for the fast response!
> > > >
> > > > On Tue, Aug 13, 2019 at 12:02 AM Greg Kroah-Hartman
> > > > <gregkh@linuxfoundation.org> wrote:
> > > > >
> > > > > On Mon, Aug 12, 2019 at 06:08:43PM -0600, Nick Crews wrote:
> > > > > > Hi Greg!
> > > > >
> > > > > Hi!
> > > > >
> > > > > First off, please fix your email client to not send html so that vger
> > > > > does not reject your messages :)
> > > >
> > > > Thanks, should be good now.
> > > >
> > > > >
> > > > > > I am working on a Chrome OS device that supports a policy called "USB Power
> > > > > > Share," which allows users to turn the laptop into a charge pack for their
> > > > > > phone. When the policy is enabled, power will be supplied to the USB ports
> > > > > > even when the system is in low power states such as S3 and S5. When
> > > > > > disabled, then no power will be supplied in S3 and S5. I wrote a driver
> > > > > > <https://lore.kernel.org/patchwork/patch/1062995/> for this already as part
> > > > > > of drivers/platform/chrome/, but Enric Balletbo i Serra, the maintainer,
> > > > > > had the reasonable suggestion of trying to move this into the USB subsystem.
> > > > >
> > > > > Correct suggestion.
> > > > >
> > > > > > Has anything like this been done before? Do you have any preliminary
> > > > > > thoughts on this before I start writing code? A few things that I haven't
> > > > > > figured out yet:
> > > > > > - How to make this feature only available on certain devices. Using device
> > > > > > tree? Kconfig? Making a separate driver just for this device that plugs
> > > > > > into the USB core?
> > > > > > - The feature is only supported on some USB ports, so we need a way of
> > > > > > filtering on a per-port basis.
> > > > >
> > > > > Look at the drivers/usb/typec/ code, I think that should do everything
> > > > > you need here as this is a typec standard functionality, right?
> > > >
> > > > Unfortunately this is for USB 2.0 ports, so it's not type-C.
> > > > Is the type-C code still worth looking at?
> > >
> > > If this is for USB 2, does it use the "non-standard" hub commands to
> > > turn on and off power?  If so, why not just use the usbreset userspace
> > > program for that?
> >
> > It does not use the standard hub commands. The USB ports are controlled
> > by an Embedded Controller (EC), so to control this policy we send a command
> > to the EC. Since the command to send to the EC is very specific, this would need
> > to go into a "hub driver" unique for these Wilco devices. We would make it so
> > that the normal hub registration is intercepted by something that sees this is a
> > Wilco device, and instead register the hub as a "wilco-hub", which has its own
> > special "power_share" sysfs attribute, but still is treated as a normal USB hub
> > otherwise?
> >
> 
> 
> I would say it is somewhat similar to the USB port power control which
> eventually calls into usb_acpi_set_power_state() but in this case it only
> affects the behavior when the system is NOT running.

Ok, if this is when the system is not running, why does Linux need to be
involved at all?

And if Linux is running, why not just follow the USB spec and not create
your own craziness?

> This design has a standalone USB charge power controller on the board
> that passes through the USB2 D+/D- pins from one port and is able to do
> BC1.2 negotiation when the host controller is not powered, assuming
> the chip has been enabled by the Embedded Controller.

So it does follow the spec?  Or does not?  I can't determine here.

If the EC is in charge of all of this, why does Linux need to get
involved?

> > > And how are you turning a USB 2 port into a power source?  That feels
> > > really odd given the spec.  Is this part of the standard somewhere or
> > > just a firmware/hardware hack that you are adding to a device?
> >
> > The EC twiddles something in the port' HW so that the port turns into a
> > DCP (Dedicated Charging Port) and only supplies power, not data. So I
> > think yes, this is a bit of a hack that does not conform to the spec.
> >
> > >
> > > Is there some port information in the firmware that describes this
> > > functionality?  If so, can you expose it through sysfs to the port that
> > > way?
> >
> > [I'm not sure I'm answering your question, but] I believe that we could
> > make the BIOS firmware describe the USB ports' capabilities, and the
> > kernel's behavior would be gated upon what the firmware reports. I see
> > that struct usb_port already contains a "quirks" field, should we add a
> > POWER_SHARE quirk to include/linux/usb/quirks.h? I would guess that
> > should that should be reserved for quirks shared between many USB
> > devices/hubs?
> 
> We could add a Device Property to the affected USB port in ACPI and
> describe it that way, similar to other properties like 'vcc-supply', 'clocks',
> 'vbus-detect', etc and hook it into the phy-generic driver.
> 
> However I'm not clear on whether the phy driver binding works with XHCI
> when using ACPI, so this may not be an appropriate place either.

Why would you have DT involved if you are using acpi?  :)

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Policy to keep USB ports powered in low-power states
  2019-08-16  9:12           ` Greg Kroah-Hartman
@ 2019-08-16 17:02             ` Duncan Laurie
  0 siblings, 0 replies; 7+ messages in thread
From: Duncan Laurie @ 2019-08-16 17:02 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Nick Crews, linux-usb, Enric Balletbo i Serra, linux-kernel,
	Daniel Kurtz

On Fri, Aug 16, 2019 at 2:12 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
>
> On Thu, Aug 15, 2019 at 05:42:05PM -0600, Duncan Laurie wrote:
> > On Wed, Aug 14, 2019 at 6:08 PM Nick Crews <ncrews@chromium.org> wrote:
> > >
> > > Adding Duncan Laurie who I think has some more intimate knowledge
> > > of how this is implemented in HW. Duncan, could you correct or elaborate
> > > on my answers below as you see fit? Also, sorry if I make some beginner
> > > mistakes here, I'm just getting familiar with the USB subsystem, and thanks for
> > > your patience.
> > >
> > > On Wed, Aug 14, 2019 at 3:20 PM Greg Kroah-Hartman
> > > <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Wed, Aug 14, 2019 at 02:12:07PM -0600, Nick Crews wrote:
> > > > > Thanks for the fast response!
> > > > >
> > > > > On Tue, Aug 13, 2019 at 12:02 AM Greg Kroah-Hartman
> > > > > <gregkh@linuxfoundation.org> wrote:
> > > > > >
> > > > > > On Mon, Aug 12, 2019 at 06:08:43PM -0600, Nick Crews wrote:
> > > > > > > Hi Greg!
> > > > > >
> > > > > > Hi!
> > > > > >
> > > > > > First off, please fix your email client to not send html so that vger
> > > > > > does not reject your messages :)
> > > > >
> > > > > Thanks, should be good now.
> > > > >
> > > > > >
> > > > > > > I am working on a Chrome OS device that supports a policy called "USB Power
> > > > > > > Share," which allows users to turn the laptop into a charge pack for their
> > > > > > > phone. When the policy is enabled, power will be supplied to the USB ports
> > > > > > > even when the system is in low power states such as S3 and S5. When
> > > > > > > disabled, then no power will be supplied in S3 and S5. I wrote a driver
> > > > > > > <https://lore.kernel.org/patchwork/patch/1062995/> for this already as part
> > > > > > > of drivers/platform/chrome/, but Enric Balletbo i Serra, the maintainer,
> > > > > > > had the reasonable suggestion of trying to move this into the USB subsystem.
> > > > > >
> > > > > > Correct suggestion.
> > > > > >
> > > > > > > Has anything like this been done before? Do you have any preliminary
> > > > > > > thoughts on this before I start writing code? A few things that I haven't
> > > > > > > figured out yet:
> > > > > > > - How to make this feature only available on certain devices. Using device
> > > > > > > tree? Kconfig? Making a separate driver just for this device that plugs
> > > > > > > into the USB core?
> > > > > > > - The feature is only supported on some USB ports, so we need a way of
> > > > > > > filtering on a per-port basis.
> > > > > >
> > > > > > Look at the drivers/usb/typec/ code, I think that should do everything
> > > > > > you need here as this is a typec standard functionality, right?
> > > > >
> > > > > Unfortunately this is for USB 2.0 ports, so it's not type-C.
> > > > > Is the type-C code still worth looking at?
> > > >
> > > > If this is for USB 2, does it use the "non-standard" hub commands to
> > > > turn on and off power?  If so, why not just use the usbreset userspace
> > > > program for that?
> > >
> > > It does not use the standard hub commands. The USB ports are controlled
> > > by an Embedded Controller (EC), so to control this policy we send a command
> > > to the EC. Since the command to send to the EC is very specific, this would need
> > > to go into a "hub driver" unique for these Wilco devices. We would make it so
> > > that the normal hub registration is intercepted by something that sees this is a
> > > Wilco device, and instead register the hub as a "wilco-hub", which has its own
> > > special "power_share" sysfs attribute, but still is treated as a normal USB hub
> > > otherwise?
> > >
> >
> >
> > I would say it is somewhat similar to the USB port power control which
> > eventually calls into usb_acpi_set_power_state() but in this case it only
> > affects the behavior when the system is NOT running.
>
> Ok, if this is when the system is not running, why does Linux need to be
> involved at all?
>
> And if Linux is running, why not just follow the USB spec and not create
> your own craziness?
>
> > This design has a standalone USB charge power controller on the board
> > that passes through the USB2 D+/D- pins from one port and is able to do
> > BC1.2 negotiation when the host controller is not powered, assuming
> > the chip has been enabled by the Embedded Controller.
>
> So it does follow the spec?  Or does not?  I can't determine here.
>


I didn't realize the part had a public datasheet:
https://www.dialog-semiconductor.com/sites/default/files/xslgc55544cr105_09292017.pdf

It is really only concerned with following the BC1.2 spec and not
interfering with the USB protocol part.


> If the EC is in charge of all of this, why does Linux need to get
> involved?
>

Only because we are looking to expose a policy to control the behavior
of this chip at the OS level.

Most systems would put this in as an option in the BIOS but we do
not have setup menus on Chrome OS and we want to have the policy
controlled directly, preferably without resorting to an opaque interface
to a userspace utility.

To that end we have added a number of different EC controls and are
looking to fit them into the appropriate subsystems wherever possible.
As you can see they don't always fit naturally..


> > > > And how are you turning a USB 2 port into a power source?  That feels
> > > > really odd given the spec.  Is this part of the standard somewhere or
> > > > just a firmware/hardware hack that you are adding to a device?
> > >
> > > The EC twiddles something in the port' HW so that the port turns into a
> > > DCP (Dedicated Charging Port) and only supplies power, not data. So I
> > > think yes, this is a bit of a hack that does not conform to the spec.
> > >
> > > >
> > > > Is there some port information in the firmware that describes this
> > > > functionality?  If so, can you expose it through sysfs to the port that
> > > > way?
> > >
> > > [I'm not sure I'm answering your question, but] I believe that we could
> > > make the BIOS firmware describe the USB ports' capabilities, and the
> > > kernel's behavior would be gated upon what the firmware reports. I see
> > > that struct usb_port already contains a "quirks" field, should we add a
> > > POWER_SHARE quirk to include/linux/usb/quirks.h? I would guess that
> > > should that should be reserved for quirks shared between many USB
> > > devices/hubs?
> >
> > We could add a Device Property to the affected USB port in ACPI and
> > describe it that way, similar to other properties like 'vcc-supply', 'clocks',
> > 'vbus-detect', etc and hook it into the phy-generic driver.
> >
> > However I'm not clear on whether the phy driver binding works with XHCI
> > when using ACPI, so this may not be an appropriate place either.
>
> Why would you have DT involved if you are using acpi?  :)
>

This would come in via the _DSD method of passing parameters to
specific ACPI devices.


-duncan

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CAHX4x86QCrkrnPEfrup8k96wyqg=QR_vgetYLqP1AEa02fx1vw@mail.gmail.com>
2019-08-13  6:02 ` Policy to keep USB ports powered in low-power states Greg Kroah-Hartman
2019-08-14 20:12   ` Nick Crews
2019-08-14 21:20     ` Greg Kroah-Hartman
2019-08-15  0:08       ` Nick Crews
2019-08-15 23:42         ` Duncan Laurie
2019-08-16  9:12           ` Greg Kroah-Hartman
2019-08-16 17:02             ` Duncan Laurie

Linux-USB Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-usb/0 linux-usb/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-usb linux-usb/ https://lore.kernel.org/linux-usb \
		linux-usb@vger.kernel.org linux-usb@archiver.kernel.org
	public-inbox-index linux-usb


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-usb


AGPL code for this site: git clone https://public-inbox.org/ public-inbox