RE: [PATCH v3] usb: typec: get the vbus source and charge values from the devicetree

[Jun Li]

Hi
> -Original Message-
> From: Guenter Roeck  On Behalf Of Guenter Roeck
> Sent: 2018年9月14日 1:35
> To: Angus Ainslie 
> Cc: Peter Chen ; Heikki Krogerus
> ; Greg Kroah-Hartman
> ; linux-usb@vger.kernel.org; lkml
> ; Peter Chen ; Jun Li
> 
> Subject: Re: [PATCH v3] usb: typec: get the vbus source and charge values 
> from the
> devicetree
> 
> On Thu, Sep 13, 2018 at 05:10:09AM -0600, Angus Ainslie wrote:
> > >
> > >staging: typec: don't do vbus source disable for dead battery
> > >
> > >In PTN5110 design, DisableSourceVBUS command also disables the sink
> > >enable signal because the EN_SNK can be used to source higher
> > >voltage, and, there is only one TCPC command to disable sourcing
> > >voltage without telling whether to disable 5V or the high voltage,
> > >and to keep the design simple they designed the PTN5110 to disable
> > >both. with this fact, we use the flag drive_vbus to check if the
> > >source vbus enable was issued, if yes we then do vbus source disable,
> > >in dead battery case, we never did vbus source enable, so will not
> > >issue vbus source disable command.
> > >
> >
> > Thanks Peter, this sounds like the missing piece of information and I
> > think some form of the code below will fix that.
> >
> > There is still the issue that my board will need some way of
> > controlling the initial state of vbus-sink.
> >
> > @Guenter: would my initial patch be acceptable to set the default
> > state of vbus-source and vbus-sink. Would you like some code to sanity
> > check that both were not enabled at the same time ?
> >
> Seems to me this is indeed a chip specific problem. I don't think a fix 
> belongs into the tcpm
> code. As mentioned before, the current status (ie drive_vbus) should be 
> readable from the
> chip. I am not sure though if we should add a
> PTN5110 specific quirk to the driver to handle the situation. I am concerned 
> that fixing this
> as suggested below for PTN5110 may cause trouble with other chips. Maybe not, 
> but I'd
> rather be cautious.

Yes, this is a chip specific problem, the reason DisableSourceVBUS command also 
disables
the sink enable signal because the EN_SNK of PTN5110 can be used to source 
higher voltage
And, there is only one TCPC command to disable sourcing voltage without telling 
whether to
disable 5V or the high voltage, and to keep the design simple, TPN5110 is 
designed to disable
both.

The patch below is to not issue a vbus disable command if it was not enabled,
from my point view it should be OK and has the benefit to save one command.

Li Jun
> 
> Thanks,
> Guenter
> 
> > >Acked-by: Peter Chen 
> > >Signed-off-by: Li Jun 
> > >---
> > > drivers/staging/typec/tcpci.c | 5 ++---
> > > 1 file changed, 2 insertions(+), 3 deletions(-)
> > >
> > >diff --git a/drivers/staging/typec/tcpci.c
> > >b/drivers/staging/typec/tcpci.c index 2d4fbb8aac5e..7352207224b5
> > >100644
> > >--- a/drivers/staging/typec/tcpci.c
> > >+++ b/drivers/staging/typec/tcpci.c
> > >@@ -381,9 +381,8 @@ static int tcpci_set_vbus(struct tcpc_dev *tcpc,
> > >bool source, bool sink)
> > >  struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> > >  int ret;
> > >
> > >- /* Disable both source and sink first before enabling anything */
> > >-
> > >- if (!source) {
> > >+ /* Only disable source if it was enabled */ if (!source &&
> > >+ tcpci->drive_vbus) {
> > >  ret = regmap_write(tcpci->regmap, TCPC_COMMAND,
> > > TCPC_CMD_DISABLE_SRC_VBUS);
> > >  if (ret < 0)
> >
> > The version of struct tcpci doesn't have a drive_vbus. Where should
> > drive_vbus get set and cleared ?
> >
> > Is this a more complete version of what you intended ?
> >
> > diff --git a/drivers/usb/typec/tcpci.c b/drivers/usb/typec/tcpci.c
> > index ac6b418b15f1..d6168163df7b 100644
> > --- a/drivers/usb/typec/tcpci.c
> > +++ b/drivers/usb/typec/tcpci.c
> > @@ -28,6 +28,7 @@ struct tcpci {
> > struct regmap *regmap;
> >
> > bool controls_vbus;
> > +   bool drive_vbus;
> >
> > struct tcpc_dev tcpc;
> > struct tcpci_data *data;
> > @@ -277,7 +278,9 @@ static int tcpci_set_vbus(struct tcpc_dev *tcpc,
> > bool source, bool sink)
> >
> > /* Disable both source and sink first before enabling anything
> > */
> >
> > -   if (!source) {
> > +   if (!source && tcpci->drive_vbus) {
> > +   tcpci->drive_vbus = false;
> > +
> > ret = regmap_write(tcpci->regmap, TCPC_COMMAND,
> >TCPC_CMD_DISABLE_SRC_VBUS);
> > if (ret < 0)
> > @@ -292,6 +295,8 @@ static int tcpci_set_vbus(struct tcpc_dev *tcpc,
> > bool source, bool sink)
> > }
> >
> > if (source) {
> > +   tcpci->drive_vbus = true;
> > +
> > ret = regmap_write(tcpci->regmap, TCPC_COMMAND,
> >TCPC_CMD_SRC_VBUS_DEFAULT);
> > if (ret < 0)
> > @@ -503,6 +508,7 @@ struct tcpci *tcpci_register_port(struct device
> > *dev, struct tcpci_data *data)
> >

RE: [PATCH v3] usb: typec: get the vbus source and charge values from the devicetree

[Jun Li]

Hi
> -Original Message-
> From: Angus Ainslie 
> Sent: 2018年9月13日 19:10
> To: Peter Chen 
> Cc: li...@roeck-us.net; Heikki Krogerus ; 
> Greg
> Kroah-Hartman ; linux-usb@vger.kernel.org; lkml
> ; Peter Chen ; Jun Li
> ; Guenter Roeck 
> Subject: Re: [PATCH v3] usb: typec: get the vbus source and charge values 
> from the
> devicetree
> 
> On 2018-09-13 01:27, Peter Chen wrote:
> > On Thu, Sep 13, 2018 at 12:35 AM Guenter Roeck 
> > wrote:
> >>
> >> On Wed, Sep 12, 2018 at 10:08:58AM -0600, Angus Ainslie wrote:
> >> > On 2018-09-11 09:33, Guenter Roeck wrote:
> >> > >I cant put my finger on it but this seems wrong. As i said both
> >> > >src and sink should never be true at the same time. I also din’t
> >> > >understand why turning off src should power off your board.
> >> > >Ultimately my concern is that we may be just painting over the
> >> > >real problem, and that would be really bad to do with dt properties.
> >> > >
> >> >
> >> > I agree that this doesn't seem like the correct way of solving the 
> >> > problem.
> >> > On this HW (Emcraft iMX8M BSB) I think the PTN5110 chip has been
> >> > connected correctly so I'm assuming that it is some quirk of the PTN5110.
> >> >
> >> > I didn't design the HW or the chip. This is a workaround for "quirky"
> >> > hardware and there may be others that don't behave exactly as expected.
> >> >
> >>
> >> I wouldn't be that sure about that. It may as well be that the tcpc
> >> driver and/or the tcpm driver are doing something wrong when
> >> initializing.
> >>
> >> I didn't really understand the logs you sent out earlier. It looked
> >> like the system would loose power if the TCPC_CMD_DISABLE_SRC_VBUS
> >> command is sent.  That doesn't really make sense to me since it
> >> indicates that the chip sources power to the remote, and turning that
> >> off should not result in a local loss of power.
> >>
> >> Note that the chip is supposed to be able to report if it is sourcing
> >> vbus and if VBUS is present, in the POWER_STATUS register. Another
> >> question is the content of the ROLE_CONTROL register when the system
> >> boots, and the DEVICE_CAPABILITIES settings.
> >>
> >> Overall I suspect that we don't handle startup for your system
> >> correctly in the tcpc driver. The ideal solution would be to find a
> >> solution which does not require any devicetree properties, but to do
> >> that we'll need to get a better understanding about your system's
> >> requirements.
> >>
> >> Guenter
> >
> > Hi Angus,
> >
> > Would you please check if below patch can fix your issue?
> >
> > staging: typec: don't do vbus source disable for dead battery
> >
> > In PTN5110 design, DisableSourceVBUS command also disables the sink
> > enable signal because the EN_SNK can be used to source higher voltage,
> > and, there is only one TCPC command to disable sourcing voltage
> > without telling whether to disable 5V or the high voltage, and to keep
> > the design simple they designed the PTN5110 to disable both. with this
> > fact, we use the flag drive_vbus to check if the source vbus enable
> > was issued, if yes we then do vbus source disable, in dead battery
> > case, we never did vbus source enable, so will not issue vbus source
> > disable command.
> >
> 
> Thanks Peter, this sounds like the missing piece of information and I think 
> some form of
> the code below will fix that.
> 
> There is still the issue that my board will need some way of controlling the 
> initial state of
> vbus-sink.
> 
> @Guenter: would my initial patch be acceptable to set the default state of 
> vbus-source and
> vbus-sink. Would you like some code to sanity check that both were not 
> enabled at the
> same time ?

I agree Guenter using a dt property is not the proper way here(maybe valid
for your HW with only typec power supply), I think a right way is to change
tcpm to handle this kind case(like the existing vbus_never_low flag) so we
will have a common handling

if (vbus_on && CC_state_is_RP)
dead_battery = true;
bypass tcpm_init_vbus or only enable sink.

Re: [PATCH 2/2] typec: tcpm: Add option to maintain current limit at Vsafe5V

[Badhri Jagan Sridharan]

On Thu, Sep 13, 2018 at 10:07 AM Jack Pham  wrote:
>
> On Thu, Sep 13, 2018 at 6:43 AM Badhri Jagan Sridharan
>  wrote:
> >
> > On Wed, Sep 12, 2018 at 11:39 PM Jack Pham  wrote:
> > >
> > > Hello Badhri,
> > >
> > > On Wed, Sep 12, 2018 at 07:11:13PM -0700, Badhri Jagan Sridharan wrote:
> > > > During hard reset, TCPM turns off the charging path.
> > > > The spec provides an option for Sink to either drop to vSafe5V or 
> > > > vSafe0V.
> > >
> > > This doesn't make sense. By definition the sink isn't sourcing VBUS, so
> > > how can it control whether to allow the voltage to be 5V or 0V?
> >
> > The way I understand it, this is for the current limits that can be
> > set on the Sink side.
> > During hard reset, sink has to fallback to VSafe5V or VSafe0V if
> > higher pd contract was negotiated.
>
> Ok, you are talking about sink current draw limits but vSafe{0,5}V are
> voltage definitions so these are orthogonal. Again, the sink can't
> directly dictate the voltage that's being sourced so I don't see how it
> has a choice here. If a PD contract was negotiated for greater than 5V
> and a hard reset happens then yes the voltage would fall to 0V and then
> rise back to 5V and during this time sink needs to draw appropriate
> current.

>
> > > > From USB_PD_R3_0
> > > > 2.6.2 Sink Operation
> > > > ..
> > > > Serious errors are handled by Hard Reset Signaling issued by either Port
> > > > Partner. A Hard Reset:
> > > > resets protocol as for a Soft Reset but also returns the power supply to
> > > > USB Default Operation (vSafe0V or vSafe5V output) in order to protect 
> > > > the
> > > > Sink.
> > >
> > > I can see how the wording here "vSafe0V *or* vSafe5V" is misleading, as
> > > I think it actually is both. In later parts of the spec, the source's
> > > VBUS behavior is well defined in that it must first drop to vSafe0V
> > > and then return to vSafe5V. Please refer to section 7.1.5.
> >
> >
> > Yeah thats for the source. But for sink, Say if the source isnt PD, then,
> > sink initiated hard resets happen during the connection. Sink would hard 
> > reset
> > couple of times before deeming that the partner is non PD. When connected
> > to Type-A ports/non-pd partner, vbus is not going to likely drop so there 
> > isnt
> > a reason to setcharge to false or drop the input current limit. Do you 
> > agree ?
>
> Sure that makes sense. In this case I wonder if TCPM even needs to call
> set_charge(false) considering it does not yet know if the partner is PD
> capable or not. For sure, if the partner is PD capable and contract had
> been previously established, we'd definitely need to set_current_limit()
> to default levels and/or turn off charging.
>
> But in the case of hard reset attempts to try to determine if the source
> will send its capabilities (thereby being PD capable), wouldn't the
> initial default current limits still be in place? I think this is the
> point you're trying to make, that there is no need to disrupt charging
> if a hard reset is not going to cause VBUS to reset.

Yes that's right ! I wasnt sure how to put that in words. Thanks for
doing that !
You do concur right ?

>
> To me it sounds like what you're trying to do makes sense only if you
> can make a run-time determination of a partner's PD capability, and not
> based on a config option.

Yes this should be possible. port->pd_capable already has that info.

To sum it up:
if the partner is pd capable, set charge to false in SNK_HARD_RESET_SINK_OFF and
readjust current limits to default in SNK_HARD_RESET_SINK_ON and turn
on charging.

If partner is not pd capable, do not turn off charging nor adjust
current limit as host port is
not going to respond to hard reset.

Does it make sense ?

>
> Thanks,
> Jack
>
> --
> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project

RE: [PATCH v12 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU

[Ajay Gupta]

Hi Heikki and Peter,

> > > Can we get the working set in while the issues is being debugged?
> >
> > I'm not the one making the decision, and I don't even know if this is
> > going through the i2c or the usb tree? If it's going through the i2c
> > tree you need a tag from the usb people (Greg?) on patch 2/2, and if
> > it's going through the usb tree, you need a tag from Wolfram on patch
> > 1/2. As I said, I'm not involved with that part, I'm just reviewing
> > these patches because I felt like it.
> >
> > The remaining issue that bothers me is the looping reads, and your
> > email address reveals that you should be in a very good position to
> > work out why they do not work, and fix it or let us know why they
> > don't.
I am working with different teams to debug this and I think it may take
some time to know the root cause.

>> However, your responses indicate that you have given up. That
> > coupled with the fact that the datasheet is not publicly available
> > (but why, seems a little over-protective to think the interface to an
> > i2c controller is worth all that much) makes me think that perhaps
> > this little detail might never be fixed if it's not fixed now. Once
> > merged, there is no pressure on you to actually do anything about it,
> > and others are stuck in darkness without a spec.
> 
> I got similar impression. Ajay, you have really made it look like you just 
> want
> to dump the code even though it still has problems, then lift your arms and
> never look back. I'm sure that is not the case, but the fact that you are
> continuously making pleas, asking us to just accept the code even though it is
> not ready, does give that impression.
> I can appreciate that you are in a hurry, and I know you have a lot of other
> tasks that also need your attention, just like everybody, but I'm asking you 
> to
> take a deep breath, and take one more look at these two drivers. Go over the
> code as a whole instead of trying to fix the problems as fast as possible
> (you've sent a new version almost daily).
> I'm certain you can figure out how to fix that last issue. You are almost 
> there.
> 
> And when you are ready, please include a cover letter in your next version and
> provide some background for these patches if you can.
Ok.

Thanks
Ajay

--nvpublic
> Ideally you could tell something about the platform that has that the PD
> controller and the I2C host. There you can also mention which tree you think
> these patches should go through, usb or i2c. I'm guessing usb based on the
> fact that the I2C host controller driver seems to be there just for the USB PD
> controller, at least for now.
> 
> 
> Thanks,
> 
> --
> heikki

Re: [PATCH 1/2] typec: tcpm: Do not disconnect link for self powered devices

[Badhri Jagan Sridharan]

On Thu, Sep 13, 2018 at 10:51 AM Guenter Roeck  wrote:
>
> On Thu, Sep 13, 2018 at 04:24:08PM +0300, Heikki Krogerus wrote:
> > +Guenter
> >
> > On Wed, Sep 12, 2018 at 07:11:12PM -0700, Badhri Jagan Sridharan wrote:
> > > During HARD_RESET the data link is disconnected.
> > > For self powered device, the spec is advising against doing that.
> > >
> > > >From USB_PD_R3_0
> > > 7.1.5 Response to Hard Resets
> > > Device operation during and after a Hard Reset is defined as follows:
> > > Self-powered devices Should Not disconnect from USB during a Hard Reset
> > > (see Section 9.1.2).
> > > Bus powered devices will disconnect from USB during a Hard Reset due to 
> > > the
> > > loss of their power source.
> > >
> > > Tackle this by letting TCPM know whether the device is self or bus 
> > > powered.
> > >
> > > This overcomes unnecessary port disconnections from hard reset.
> > > Also, speeds up the enumeration time when connected to Type-A ports.
> > >
> > > Signed-off-by: Badhri Jagan Sridharan 
> > > ---
> > >  drivers/usb/typec/tcpm.c | 6 +-
> > >  include/linux/usb/tcpm.h | 1 +
> > >  2 files changed, 6 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> > > index 4f1f4215f3d6..a4e0c027a2a9 100644
> > > --- a/drivers/usb/typec/tcpm.c
> > > +++ b/drivers/usb/typec/tcpm.c
> > > @@ -3270,7 +3270,11 @@ static void run_state_machine(struct tcpm_port 
> > > *port)
> > > memset(&port->pps_data, 0, sizeof(port->pps_data));
> > > tcpm_set_vconn(port, false);
> > > tcpm_set_charge(port, false);
> > > -   tcpm_set_roles(port, false, TYPEC_SINK, TYPEC_DEVICE);
> > > +
> > > +   if (port->tcpc->config->self_powered)
> >
> > Add a member for that flag to the struct tcpm_port, and check that
> > here instead. I'll explain why below.
> >
> > > +   tcpm_set_roles(port, true, TYPEC_SINK, TYPEC_DEVICE);
> > > +   else
> > > +   tcpm_set_roles(port, false, TYPEC_SINK, TYPEC_DEVICE);
> > >
>
> Maybe I am missing something, but
> tcpm_set_roles(port, port->self_powered,
>TYPEC_SINK, TYPEC_DEVICE);
> should accomplish the same (assuming the flag is moved to tcpm_port).

Yeah Makes sense to me..
>
> Does the code at SRC_HARD_RESET_VBUS_OFF: also have to be changed,
> or only SNK_HARD_RESET_SINK_OFF: ? In other words, does this only
> apply if the port is a sink, or also if is is source ?

Good question ! I dont see any direct guidance on the spec.
All that I can see is the following:

>From 2.6.1 Source Operation:
Restores the Port’s data role to DFP.

>From 7.1.5 Response to Hard Resets:
"The Source Shall meet both tSafe5V and tSafe0V relative to the start
of the voltage transition as shown in Figure 7-8."

I am leaning towards to changing the link state from false to true for
SRC_HARD_RESET_VBUS_OFF
as well. This is to keep it symmetrical. What do you think ?

>
> Guenter
>
> > > /*
> > >  * VBUS may or may not toggle, depending on the adapter.
> > >  * If it doesn't toggle, transition to SNK_HARD_RESET_SINK_ON
> > > diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
> > > index 7e7fbfb84e8e..50c74a77db55 100644
> > > --- a/include/linux/usb/tcpm.h
> > > +++ b/include/linux/usb/tcpm.h
> > > @@ -89,6 +89,7 @@ struct tcpc_config {
> > > enum typec_port_data data;
> > > enum typec_role default_role;
> > > bool try_role_hw;   /* try.{src,snk} implemented in hardware */
> > > +   bool self_powered;  /* port belongs to a self powered device */
> >
> > I'm not sure we should add any more members to that structure, but
> > maybe it's not a problem for now. We can't quite yet get rid of that
> > structure.
> >
> > You do need to introduce a new device property already. Then read the

This would be part of usb-connector binding. right ?

> > value for that new member you added to struct tcpm_port in
> > tcpm_fw_get_caps() and in tcpm_copy_caps().
> >
> >
> > Thanks,
> >
> > --
> > heikki

Inaccessible dual-role port on CherryTrail

[Rob Weber]

Hi linux-usb,

I'm currently bringing up a custom board that uses a CherryTrail
processor and I'm having quite a bit of trouble accessing the dual-role
port from Linux.

Our system includes two USB 3.0-capable ports with Type-C connectors.
One port is designed to be a host-only port (downstream-facing),
while the other port is designed to be a dual-role port. The USB 2.0
data lines and the SuperSpeed lines are connected to the SoC. Our
system uses two USB Power Delivery controllers to help with PD
negotiations. We have an OTG_ID signal connected to the SoC.
Depending on the data role negotiation (which we detect from the PD
controller), we either tie that signal to GND or let it float.
I'm running a 4.9.115 kernel built using Yocto with a few patches applied
to enable HDMI audio.

I've been working very closely with our BIOS vendor to initialize the
CherryTrail SoC's embedded host and device controllers properly. I've been
able to validate that both of our USB-C ports work at both 2.0 and 3.0 speeds
from the BIOS, but Linux only has access to our host port. The dual-role
port is not operational from Linux.

I've attached a copy of the lspci -vv output for both of the PCI
controllers that the kernel recognizes. I also enabled tracing during
the boot process for the dwc3 and xhci-hcd drivers and have attached
the trace output. Please note that these traces not only include the
initialization process, but also probing the g_mass_storage module.
I also toggled the dwc3 mode from device to host using debugfs and
attached a USB storage device to the dual-role port. Unfortunately there
was no activity seen in the trace output besides toggling the dwc3 mode.

The fact that downstream devices are accessible from the BIOS and not
Linux indicates to me that there's either a configuration issue when the
BIOS hands off USB control to the kernel, or the kernel is not compiled
properly to support the SoC's internal controllers.

I would appreciate it if you could take a quick look at the trace and
lspci output and let me know if anything seems to be strange about the
controller driver initialization. Might the controller register values
indicate that the controller is in some sort of disabled state? I am
working pretty closely with the BIOS vendor so I'm also able to request
BIOS changes if need be.

I would also appreciate any feedback on further debugging tips. I've
been using devmem2 to inspect the MMIO registers of the host controller
and comparing those values to the expected values in the SoC datasheet.
the xHCI debugfs directory doesn't appear in my debugfs which is also
kind of strange. I might also try using kgdb to step through the
initialization process.

Thank you for taking a look into this situation! Please let me know
if you have any questions.

Cheers,
Rob Weber
# tracer: nop
#
# entries-in-buffer/entries-written: 521/521   #P:4
#
#  _-=> irqs-off
# / _=> need-resched
#| / _---=> hardirq/softirq
#|| / _--=> preempt-depth
#||| / delay
#   TASK-PID   CPU#  TIMESTAMP  FUNCTION
#  | |   |      | |
   swapper/0-1 [002]  2.960105: dwc3_readl: addr 
c9e00140 value 20204008
   swapper/0-1 [002]  2.960114: dwc3_readl: addr 
c9e00144 value 0260c93b
   swapper/0-1 [002]  2.960117: dwc3_readl: addr 
c9e00148 value 008086a0
   swapper/0-1 [002]  2.960119: dwc3_readl: addr 
c9e0014c value 10420085
   swapper/0-1 [002]  2.960121: dwc3_readl: addr 
c9e00150 value 47a22004
   swapper/0-1 [002]  2.960124: dwc3_readl: addr 
c9e00154 value 04202088
   swapper/0-1 [002]  2.960126: dwc3_readl: addr 
c9e00158 value 02f60020
   swapper/0-1 [002]  2.960129: dwc3_readl: addr 
c9e0015c value 038507e6
   swapper/0-1 [002]  2.960131: dwc3_readl: addr 
c9e00600 value 02f6
   swapper/0-1 [002]  2.960152: dwc3_readl: addr 
c9e00120 value 5533260a
   swapper/0-1 [002]  2.960154: dwc3_writel: addr 
c9e00128 value 00040973
   swapper/0-1 [002]  2.960156: dwc3_writel: addr 
c9e00704 value 4000
   swapper/0-1 [002]  2.960158: dwc3_readl: addr 
c9e00704 value 4000
   swapper/0-1 [002]  2.960160: dwc3_readl: addr 
c9e00704 value 4000
   swapper/0-1 [002]  2.960164: dwc3_readl: addr 
c9e00704 value 4000
   swapper/0-1 [002]  2.960166: dwc3_readl: addr 
c9e00704 value 4000
   swapper/0-1 [002]  2.960169: dwc3_readl: addr 
c9e00704 value 4000
   swapper/0-1 [002]  2.960171: dwc3_readl: addr 
c9e0070

Re: [PATCH v4 4/8] usb: dwc3: implement stream transfer timeout

[Thinh Nguyen]

Hi Anurag,

On 9/8/2018 8:03 AM, Anurag Kumar Vulisha wrote:
> According to dwc3 databook when streams are used, it may be possible
> for the host and device become out of sync, where device may wait for
> host to issue prime transcation and host may wait for device to issue
> erdy. To avoid such deadlock, timeout needs to be implemented. After
> timeout occurs, device will first stop transfer and restart the transfer
> again. This patch does the same.
>
> Signed-off-by: Anurag Kumar Vulisha 
> Reviewed-by: Thinh Nguyen 
> ---
>  Chnages in v4:
>   1. Added description for stream timeout timer as suggested by
>  "Thinh Nguyen"
>
>  Changes in v3:
>   1. Added the changes suggested by "Thinh Nguyen"
>
>  Changes in v2:
>   1. Changed STREAM_TIMEOUT to STREAM_TIMEOUT_MS as suggested by
>  "Andy Shevchenko"
> ---
>  drivers/usb/dwc3/core.h   |  7 +++
>  drivers/usb/dwc3/gadget.c | 45 +
>  2 files changed, 52 insertions(+)
>
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 5bfb625..f62e8c4 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -633,6 +633,11 @@ struct dwc3_event_buffer {
>  
>  #define DWC3_TRB_NUM 256
>  
> +/*
> + * Timeout value in msecs used by stream_timeout_timer when streams are 
> enabled
> + */
> +#define STREAM_TIMEOUT_MS50
> +
>  /**
>   * struct dwc3_ep - device side endpoint representation
>   * @endpoint: usb endpoint
> @@ -656,6 +661,7 @@ struct dwc3_event_buffer {
>   * @name: a human readable name e.g. ep1out-bulk
>   * @direction: true for TX, false for RX
>   * @stream_capable: true when streams are enabled
> + * @stream_timeout_timer: timeout timer used by bulk streams
>   */
>  struct dwc3_ep {
>   struct usb_ep   endpoint;
> @@ -705,6 +711,7 @@ struct dwc3_ep {
>  
>   unsigneddirection:1;
>   unsignedstream_capable:1;
> + struct timer_list   stream_timeout_timer;
>  };
>  
>  enum dwc3_phy {
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 13ea282..306d4c5 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -254,6 +254,7 @@ int dwc3_send_gadget_generic_command(struct dwc3 *dwc, 
> unsigned cmd, u32 param)
>  }
>  
>  static int __dwc3_gadget_wakeup(struct dwc3 *dwc);
> +static void stream_timeout_function(struct timer_list *arg);
>  
>  /**
>   * dwc3_send_gadget_ep_cmd - issue an endpoint command
> @@ -574,6 +575,17 @@ static int dwc3_gadget_set_ep_config(struct dwc3_ep 
> *dep, unsigned int action)
>   | DWC3_DEPCFG_STREAM_EVENT_EN
>   | DWC3_DEPCFG_XFER_COMPLETE_EN;
>   dep->stream_capable = true;
> +
> + /*
> +  * When BULK streams are enabled it may be possible for the host
> +  * and device become out of sync, where device may wait for host
> +  * to issue prime transcation and host may wait for device to
> +  * issue ERDY. To avoid such deadlock, timeout needs to be
> +  * implemented. After timeout occurs, device will first stop
> +  * transfer and restart the transfer again.
> +  */
> + timer_setup(&dep->stream_timeout_timer,
> + stream_timeout_function, 0);
>   }
>  
>   if (!usb_endpoint_xfer_control(desc))
> @@ -730,6 +742,9 @@ static int __dwc3_gadget_ep_disable(struct dwc3_ep *dep)
>  
>   trace_dwc3_gadget_ep_disable(dep);
>  
> + if (dep->stream_capable)
> + del_timer(&dep->stream_timeout_timer);
> +
>   dwc3_remove_requests(dwc, dep);
>  
>   /* make sure HW endpoint isn't stalled */
> @@ -1257,6 +1272,12 @@ static int __dwc3_gadget_kick_transfer(struct dwc3_ep 
> *dep)
>   return ret;
>   }
>  
> + if (starting && dep->stream_capable) {
> + dep->stream_timeout_timer.expires = jiffies +
> + msecs_to_jiffies(STREAM_TIMEOUT_MS);
> + add_timer(&dep->stream_timeout_timer);
> + }
> +
>   return 0;
>  }
>  
> @@ -2403,6 +2424,13 @@ static void 
> dwc3_gadget_endpoint_transfer_in_progress(struct dwc3_ep *dep,
>   stop = true;
>   }
>  
> + /*
> +  * Delete the timer that was started in __dwc3_gadget_kick_transfer()
> +  * for stream capable endpoints.
> +  */
> + if (dep->stream_capable)
> + del_timer(&dep->stream_timeout_timer);
> +
>   dwc3_gadget_ep_cleanup_completed_requests(dep, event, status);
>  
>   if (stop) {
> @@ -2487,6 +2515,11 @@ static void dwc3_endpoint_interrupt(struct dwc3 *dwc,
>   }
>   break;
>   case DWC3_DEPEVT_STREAMEVT:
> + if (event->status == DEPEVT_STREAMEVT_FOUND)
> + del_timer(&dep->stream_timeout_timer);
> + else
> + dev_dbg(dwc->d

Re: [PATCH 1/2] typec: tcpm: Do not disconnect link for self powered devices

[Guenter Roeck]

On Thu, Sep 13, 2018 at 04:24:08PM +0300, Heikki Krogerus wrote:
> +Guenter
> 
> On Wed, Sep 12, 2018 at 07:11:12PM -0700, Badhri Jagan Sridharan wrote:
> > During HARD_RESET the data link is disconnected.
> > For self powered device, the spec is advising against doing that.
> > 
> > >From USB_PD_R3_0
> > 7.1.5 Response to Hard Resets
> > Device operation during and after a Hard Reset is defined as follows:
> > Self-powered devices Should Not disconnect from USB during a Hard Reset
> > (see Section 9.1.2).
> > Bus powered devices will disconnect from USB during a Hard Reset due to the
> > loss of their power source.
> > 
> > Tackle this by letting TCPM know whether the device is self or bus powered.
> > 
> > This overcomes unnecessary port disconnections from hard reset.
> > Also, speeds up the enumeration time when connected to Type-A ports.
> > 
> > Signed-off-by: Badhri Jagan Sridharan 
> > ---
> >  drivers/usb/typec/tcpm.c | 6 +-
> >  include/linux/usb/tcpm.h | 1 +
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> > index 4f1f4215f3d6..a4e0c027a2a9 100644
> > --- a/drivers/usb/typec/tcpm.c
> > +++ b/drivers/usb/typec/tcpm.c
> > @@ -3270,7 +3270,11 @@ static void run_state_machine(struct tcpm_port *port)
> > memset(&port->pps_data, 0, sizeof(port->pps_data));
> > tcpm_set_vconn(port, false);
> > tcpm_set_charge(port, false);
> > -   tcpm_set_roles(port, false, TYPEC_SINK, TYPEC_DEVICE);
> > +
> > +   if (port->tcpc->config->self_powered)
> 
> Add a member for that flag to the struct tcpm_port, and check that
> here instead. I'll explain why below.
> 
> > +   tcpm_set_roles(port, true, TYPEC_SINK, TYPEC_DEVICE);
> > +   else
> > +   tcpm_set_roles(port, false, TYPEC_SINK, TYPEC_DEVICE);
> > 

Maybe I am missing something, but
tcpm_set_roles(port, port->self_powered,
   TYPEC_SINK, TYPEC_DEVICE);
should accomplish the same (assuming the flag is moved to tcpm_port).

Does the code at SRC_HARD_RESET_VBUS_OFF: also have to be changed,
or only SNK_HARD_RESET_SINK_OFF: ? In other words, does this only
apply if the port is a sink, or also if is is source ?

Guenter

> > /*
> >  * VBUS may or may not toggle, depending on the adapter.
> >  * If it doesn't toggle, transition to SNK_HARD_RESET_SINK_ON
> > diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
> > index 7e7fbfb84e8e..50c74a77db55 100644
> > --- a/include/linux/usb/tcpm.h
> > +++ b/include/linux/usb/tcpm.h
> > @@ -89,6 +89,7 @@ struct tcpc_config {
> > enum typec_port_data data;
> > enum typec_role default_role;
> > bool try_role_hw;   /* try.{src,snk} implemented in hardware */
> > +   bool self_powered;  /* port belongs to a self powered device */
> 
> I'm not sure we should add any more members to that structure, but
> maybe it's not a problem for now. We can't quite yet get rid of that
> structure.
> 
> You do need to introduce a new device property already. Then read the
> value for that new member you added to struct tcpm_port in
> tcpm_fw_get_caps() and in tcpm_copy_caps().
> 
> 
> Thanks,
> 
> -- 
> heikki

Re: [PATCH v3] usb: typec: get the vbus source and charge values from the devicetree

[Guenter Roeck]

On Thu, Sep 13, 2018 at 05:10:09AM -0600, Angus Ainslie wrote:
> >
> >staging: typec: don't do vbus source disable for dead battery
> >
> >In PTN5110 design, DisableSourceVBUS command also disables the sink
> >enable signal because the EN_SNK can be used to source higher voltage,
> >and, there is only one TCPC command to disable sourcing voltage without
> >telling whether to disable 5V or the high voltage, and to keep the
> >design simple they designed the PTN5110 to disable both. with this
> >fact, we use the flag drive_vbus to check if the source vbus enable was
> >issued, if yes we then do vbus source disable, in dead battery case,
> >we never did vbus source enable, so will not issue vbus source disable
> >command.
> >
> 
> Thanks Peter, this sounds like the missing piece of information and I think
> some form of the code below will fix that.
> 
> There is still the issue that my board will need some way of controlling the
> initial state of vbus-sink.
> 
> @Guenter: would my initial patch be acceptable to set the default state of
> vbus-source and vbus-sink. Would you like some code to sanity check that
> both were not enabled at the same time ?
> 
Seems to me this is indeed a chip specific problem. I don't think a fix belongs
into the tcpm code. As mentioned before, the current status (ie drive_vbus)
should be readable from the chip. I am not sure though if we should add a
PTN5110 specific quirk to the driver to handle the situation. I am concerned
that fixing this as suggested below for PTN5110 may cause trouble with other
chips. Maybe not, but I'd rather be cautious.

Thanks,
Guenter

> >Acked-by: Peter Chen 
> >Signed-off-by: Li Jun 
> >---
> > drivers/staging/typec/tcpci.c | 5 ++---
> > 1 file changed, 2 insertions(+), 3 deletions(-)
> >
> >diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
> >index 2d4fbb8aac5e..7352207224b5 100644
> >--- a/drivers/staging/typec/tcpci.c
> >+++ b/drivers/staging/typec/tcpci.c
> >@@ -381,9 +381,8 @@ static int tcpci_set_vbus(struct tcpc_dev *tcpc,
> >bool source, bool sink)
> >  struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
> >  int ret;
> >
> >- /* Disable both source and sink first before enabling anything */
> >-
> >- if (!source) {
> >+ /* Only disable source if it was enabled */
> >+ if (!source && tcpci->drive_vbus) {
> >  ret = regmap_write(tcpci->regmap, TCPC_COMMAND,
> > TCPC_CMD_DISABLE_SRC_VBUS);
> >  if (ret < 0)
> 
> The version of struct tcpci doesn't have a drive_vbus. Where should
> drive_vbus get set and cleared ?
> 
> Is this a more complete version of what you intended ?
> 
> diff --git a/drivers/usb/typec/tcpci.c b/drivers/usb/typec/tcpci.c
> index ac6b418b15f1..d6168163df7b 100644
> --- a/drivers/usb/typec/tcpci.c
> +++ b/drivers/usb/typec/tcpci.c
> @@ -28,6 +28,7 @@ struct tcpci {
> struct regmap *regmap;
> 
> bool controls_vbus;
> +   bool drive_vbus;
> 
> struct tcpc_dev tcpc;
> struct tcpci_data *data;
> @@ -277,7 +278,9 @@ static int tcpci_set_vbus(struct tcpc_dev *tcpc, bool
> source, bool sink)
> 
> /* Disable both source and sink first before enabling anything */
> 
> -   if (!source) {
> +   if (!source && tcpci->drive_vbus) {
> +   tcpci->drive_vbus = false;
> +
> ret = regmap_write(tcpci->regmap, TCPC_COMMAND,
>TCPC_CMD_DISABLE_SRC_VBUS);
> if (ret < 0)
> @@ -292,6 +295,8 @@ static int tcpci_set_vbus(struct tcpc_dev *tcpc, bool
> source, bool sink)
> }
> 
> if (source) {
> +   tcpci->drive_vbus = true;
> +
> ret = regmap_write(tcpci->regmap, TCPC_COMMAND,
>TCPC_CMD_SRC_VBUS_DEFAULT);
> if (ret < 0)
> @@ -503,6 +508,7 @@ struct tcpci *tcpci_register_port(struct device *dev,
> struct tcpci_data *data)
> tcpci->dev = dev;
> tcpci->data = data;
> tcpci->regmap = data->regmap;
> +   tcpci->drive_vbus = false;
> 
> tcpci->tcpc.init = tcpci_init;
> tcpci->tcpc.get_vbus = tcpci_get_vbus;
> 
> 
> 

Re: [PATCH 2/2] typec: tcpm: Add option to maintain current limit at Vsafe5V

[Jack Pham]

On Thu, Sep 13, 2018 at 6:43 AM Badhri Jagan Sridharan
 wrote:
>
> On Wed, Sep 12, 2018 at 11:39 PM Jack Pham  wrote:
> >
> > Hello Badhri,
> >
> > On Wed, Sep 12, 2018 at 07:11:13PM -0700, Badhri Jagan Sridharan wrote:
> > > During hard reset, TCPM turns off the charging path.
> > > The spec provides an option for Sink to either drop to vSafe5V or vSafe0V.
> >
> > This doesn't make sense. By definition the sink isn't sourcing VBUS, so
> > how can it control whether to allow the voltage to be 5V or 0V?
>
> The way I understand it, this is for the current limits that can be
> set on the Sink side.
> During hard reset, sink has to fallback to VSafe5V or VSafe0V if
> higher pd contract was negotiated.

Ok, you are talking about sink current draw limits but vSafe{0,5}V are
voltage definitions so these are orthogonal. Again, the sink can't
directly dictate the voltage that's being sourced so I don't see how it
has a choice here. If a PD contract was negotiated for greater than 5V
and a hard reset happens then yes the voltage would fall to 0V and then
rise back to 5V and during this time sink needs to draw appropriate
current.

> > > From USB_PD_R3_0
> > > 2.6.2 Sink Operation
> > > ..
> > > Serious errors are handled by Hard Reset Signaling issued by either Port
> > > Partner. A Hard Reset:
> > > resets protocol as for a Soft Reset but also returns the power supply to
> > > USB Default Operation (vSafe0V or vSafe5V output) in order to protect the
> > > Sink.
> >
> > I can see how the wording here "vSafe0V *or* vSafe5V" is misleading, as
> > I think it actually is both. In later parts of the spec, the source's
> > VBUS behavior is well defined in that it must first drop to vSafe0V
> > and then return to vSafe5V. Please refer to section 7.1.5.
>
>
> Yeah thats for the source. But for sink, Say if the source isnt PD, then,
> sink initiated hard resets happen during the connection. Sink would hard reset
> couple of times before deeming that the partner is non PD. When connected
> to Type-A ports/non-pd partner, vbus is not going to likely drop so there isnt
> a reason to setcharge to false or drop the input current limit. Do you agree ?

Sure that makes sense. In this case I wonder if TCPM even needs to call
set_charge(false) considering it does not yet know if the partner is PD
capable or not. For sure, if the partner is PD capable and contract had
been previously established, we'd definitely need to set_current_limit()
to default levels and/or turn off charging.

But in the case of hard reset attempts to try to determine if the source
will send its capabilities (thereby being PD capable), wouldn't the
initial default current limits still be in place? I think this is the
point you're trying to make, that there is no need to disrupt charging
if a hard reset is not going to cause VBUS to reset.

To me it sounds like what you're trying to do makes sense only if you
can make a run-time determination of a partner's PD capability, and not
based on a config option.

Thanks,
Jack

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] option: Improve Quectel EP06 detection

[Kristian Evensen]

Hello again,

On Thu, Sep 13, 2018 at 11:17 AM Johan Hovold  wrote:
> Kristian would you mind giving it a try?

I just finished backporting + testing your patch with our 4.14-kernel
(mine is already there) and it works great. The driver correctly
handles different EP06-configurations.

Thanks a lot!

BR,
Kristian

Re: [PATCH 1/5] misc: rtsx_usb: Use USB remote wakeup signaling for card insertion detection

[Alan Stern]

On Thu, 13 Sep 2018, Kai-Heng Feng wrote:

> I am working on the next version of this series, and the last missing  
> puzzle is to differentiate system-wide resume from runtime resume in  
> usb_driver's resume() and reset_resume() callback.
> 
> The parent device, rtsx_usb, has two child devices, rtsx_usb_ms and  
> rtsx_usb_sdmmc.
> pm_request_resume() is used in rtsx_usb's resume() to facilitate USB remote  
> wakeup signaling, so we don't need to poll the card slot status.
> But this has a side effect: during system resume the rtsx_usb calls  
> pm_request_resume() in its resume(), so child devices calls their  
> runtime_resume() instead of resume() callback.

Have you actually observed this?  It shouldn't happen.  
pm_request_resume() schedules a runtime resume on the PM work queue, 
but this work queue is frozen during system sleep.  It doesn't unfreeze 
until after all the devices have been restored to full power.  Thus the 
child device's resume() callback should be invoked before 
runtime_resume().

> So, is it reasonable to pass pm_message_t to resume() and reset_resume()  
> callbacks and use PMSG_IS_AUTO() to differentiate them?

It should not be necessary to do this.

Alan Stern

Re: [PATCH 2/2] typec: tcpm: Add option to maintain current limit at Vsafe5V

[Badhri Jagan Sridharan]

On Wed, Sep 12, 2018 at 11:39 PM Jack Pham  wrote:
>
> Hello Badhri,
>
> On Wed, Sep 12, 2018 at 07:11:13PM -0700, Badhri Jagan Sridharan wrote:
> > During hard reset, TCPM turns off the charging path.
> > The spec provides an option for Sink to either drop to vSafe5V or vSafe0V.
>
> This doesn't make sense. By definition the sink isn't sourcing VBUS, so
> how can it control whether to allow the voltage to be 5V or 0V?

The way I understand it, this is for the current limits that can be
set on the Sink side.
During hard reset, sink has to fallback to VSafe5V or VSafe0V if
higher pd contract was negotiated.

>
>
> > From USB_PD_R3_0
> > 2.6.2 Sink Operation
> > ..
> > Serious errors are handled by Hard Reset Signaling issued by either Port
> > Partner. A Hard Reset:
> > resets protocol as for a Soft Reset but also returns the power supply to
> > USB Default Operation (vSafe0V or vSafe5V output) in order to protect the
> > Sink.
>
> I can see how the wording here "vSafe0V *or* vSafe5V" is misleading, as
> I think it actually is both. In later parts of the spec, the source's
> VBUS behavior is well defined in that it must first drop to vSafe0V
> and then return to vSafe5V. Please refer to section 7.1.5.


Yeah thats for the source. But for sink, Say if the source isnt PD, then,
sink initiated hard resets happen during the connection. Sink would hard reset
couple of times before deeming that the partner is non PD. When connected
to Type-A ports/non-pd partner, vbus is not going to likely drop so there isnt
a reason to setcharge to false or drop the input current limit. Do you agree ?

>
>
> > Add a config option to tcpc_dev and let the device specific driver decide
> > what needs to be done.
> >
> > Signed-off-by: Badhri Jagan Sridharan 
> > ---
> >  drivers/usb/typec/tcpm.c | 7 ++-
> >  include/linux/usb/tcpm.h | 1 +
> >  2 files changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> > index a4e0c027a2a9..350d1a7c4543 100644
> > --- a/drivers/usb/typec/tcpm.c
> > +++ b/drivers/usb/typec/tcpm.c
> > @@ -3269,7 +3269,12 @@ static void run_state_machine(struct tcpm_port *port)
> >   case SNK_HARD_RESET_SINK_OFF:
> >   memset(&port->pps_data, 0, sizeof(port->pps_data));
> >   tcpm_set_vconn(port, false);
> > - tcpm_set_charge(port, false);
> > + if (port->tcpc->config->vsafe_5v_hard_reset)
>
> Therefore I think this config option doesn't make sense.
>
> > + tcpm_set_current_limit(port,
> > +tcpm_get_current_limit(port),
> > +5000);
>
> But I do think this might still be useful at least in ensuring the sink
> returns to drawing only default power during the transition before
> re-establishing a contract. Given that the sink can't control when
> exactly VBUS will go to 0V, is it ok to call set_current_limit() even if
> VBUS is momentarily 0V, so at least it is in preparation for when VBUS
> turns back on? Or would it be safer to do it during the
> SNK_HARD_RESET_SINK_ON state after we know VBUS is back to vSafe5V?

IMHO Doing it in SNK_HARD_RESET_SINK_ON makes more sense when
vsafe_5v_hard_reset
is not set.

>
>
> > + else
> > + tcpm_set_charge(port, false);
>
> Regards,
> Jack
> --
> The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project

Re: [PATCH 2/2] typec: tcpm: Add option to maintain current limit at Vsafe5V

[Badhri Jagan Sridharan]

+ Guenter
On Thu, Sep 13, 2018 at 6:43 AM Badhri Jagan Sridharan
 wrote:
>
> On Wed, Sep 12, 2018 at 11:39 PM Jack Pham  wrote:
> >
> > Hello Badhri,
> >
> > On Wed, Sep 12, 2018 at 07:11:13PM -0700, Badhri Jagan Sridharan wrote:
> > > During hard reset, TCPM turns off the charging path.
> > > The spec provides an option for Sink to either drop to vSafe5V or vSafe0V.
> >
> > This doesn't make sense. By definition the sink isn't sourcing VBUS, so
> > how can it control whether to allow the voltage to be 5V or 0V?
>
> The way I understand it, this is for the current limits that can be
> set on the Sink side.
> During hard reset, sink has to fallback to VSafe5V or VSafe0V if
> higher pd contract was negotiated.
>
> >
> >
> > > From USB_PD_R3_0
> > > 2.6.2 Sink Operation
> > > ..
> > > Serious errors are handled by Hard Reset Signaling issued by either Port
> > > Partner. A Hard Reset:
> > > resets protocol as for a Soft Reset but also returns the power supply to
> > > USB Default Operation (vSafe0V or vSafe5V output) in order to protect the
> > > Sink.
> >
> > I can see how the wording here "vSafe0V *or* vSafe5V" is misleading, as
> > I think it actually is both. In later parts of the spec, the source's
> > VBUS behavior is well defined in that it must first drop to vSafe0V
> > and then return to vSafe5V. Please refer to section 7.1.5.
>
>
> Yeah thats for the source. But for sink, Say if the source isnt PD, then,
> sink initiated hard resets happen during the connection. Sink would hard reset
> couple of times before deeming that the partner is non PD. When connected
> to Type-A ports/non-pd partner, vbus is not going to likely drop so there isnt
> a reason to setcharge to false or drop the input current limit. Do you agree ?
>
> >
> >
> > > Add a config option to tcpc_dev and let the device specific driver decide
> > > what needs to be done.
> > >
> > > Signed-off-by: Badhri Jagan Sridharan 
> > > ---
> > >  drivers/usb/typec/tcpm.c | 7 ++-
> > >  include/linux/usb/tcpm.h | 1 +
> > >  2 files changed, 7 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> > > index a4e0c027a2a9..350d1a7c4543 100644
> > > --- a/drivers/usb/typec/tcpm.c
> > > +++ b/drivers/usb/typec/tcpm.c
> > > @@ -3269,7 +3269,12 @@ static void run_state_machine(struct tcpm_port 
> > > *port)
> > >   case SNK_HARD_RESET_SINK_OFF:
> > >   memset(&port->pps_data, 0, sizeof(port->pps_data));
> > >   tcpm_set_vconn(port, false);
> > > - tcpm_set_charge(port, false);
> > > + if (port->tcpc->config->vsafe_5v_hard_reset)
> >
> > Therefore I think this config option doesn't make sense.
> >
> > > + tcpm_set_current_limit(port,
> > > +tcpm_get_current_limit(port),
> > > +5000);
> >
> > But I do think this might still be useful at least in ensuring the sink
> > returns to drawing only default power during the transition before
> > re-establishing a contract. Given that the sink can't control when
> > exactly VBUS will go to 0V, is it ok to call set_current_limit() even if
> > VBUS is momentarily 0V, so at least it is in preparation for when VBUS
> > turns back on? Or would it be safer to do it during the
> > SNK_HARD_RESET_SINK_ON state after we know VBUS is back to vSafe5V?
>
> IMHO Doing it in SNK_HARD_RESET_SINK_ON makes more sense when
> vsafe_5v_hard_reset
> is not set.
>
> >
> >
> > > + else
> > > + tcpm_set_charge(port, false);
> >
> > Regards,
> > Jack
> > --
> > The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> > a Linux Foundation Collaborative Project

Re: [PATCH 1/2] typec: tcpm: Do not disconnect link for self powered devices

[Heikki Krogerus]

+Guenter

On Wed, Sep 12, 2018 at 07:11:12PM -0700, Badhri Jagan Sridharan wrote:
> During HARD_RESET the data link is disconnected.
> For self powered device, the spec is advising against doing that.
> 
> >From USB_PD_R3_0
> 7.1.5 Response to Hard Resets
> Device operation during and after a Hard Reset is defined as follows:
> Self-powered devices Should Not disconnect from USB during a Hard Reset
> (see Section 9.1.2).
> Bus powered devices will disconnect from USB during a Hard Reset due to the
> loss of their power source.
> 
> Tackle this by letting TCPM know whether the device is self or bus powered.
> 
> This overcomes unnecessary port disconnections from hard reset.
> Also, speeds up the enumeration time when connected to Type-A ports.
> 
> Signed-off-by: Badhri Jagan Sridharan 
> ---
>  drivers/usb/typec/tcpm.c | 6 +-
>  include/linux/usb/tcpm.h | 1 +
>  2 files changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> index 4f1f4215f3d6..a4e0c027a2a9 100644
> --- a/drivers/usb/typec/tcpm.c
> +++ b/drivers/usb/typec/tcpm.c
> @@ -3270,7 +3270,11 @@ static void run_state_machine(struct tcpm_port *port)
>   memset(&port->pps_data, 0, sizeof(port->pps_data));
>   tcpm_set_vconn(port, false);
>   tcpm_set_charge(port, false);
> - tcpm_set_roles(port, false, TYPEC_SINK, TYPEC_DEVICE);
> +
> + if (port->tcpc->config->self_powered)

Add a member for that flag to the struct tcpm_port, and check that
here instead. I'll explain why below.

> + tcpm_set_roles(port, true, TYPEC_SINK, TYPEC_DEVICE);
> + else
> + tcpm_set_roles(port, false, TYPEC_SINK, TYPEC_DEVICE);
> 
>   /*
>* VBUS may or may not toggle, depending on the adapter.
>* If it doesn't toggle, transition to SNK_HARD_RESET_SINK_ON
> diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
> index 7e7fbfb84e8e..50c74a77db55 100644
> --- a/include/linux/usb/tcpm.h
> +++ b/include/linux/usb/tcpm.h
> @@ -89,6 +89,7 @@ struct tcpc_config {
>   enum typec_port_data data;
>   enum typec_role default_role;
>   bool try_role_hw;   /* try.{src,snk} implemented in hardware */
> + bool self_powered;  /* port belongs to a self powered device */

I'm not sure we should add any more members to that structure, but
maybe it's not a problem for now. We can't quite yet get rid of that
structure.

You do need to introduce a new device property already. Then read the
value for that new member you added to struct tcpm_port in
tcpm_fw_get_caps() and in tcpm_copy_caps().


Thanks,

-- 
heikki

[PATCH 04/10] usb: xhci-mtk: improve bandwidth scheduling

[Mathias Nyman]

From: Chunfeng Yun 

Mainly improve SuperSpeed ISOC bandwidth in last microframe,
and LowSpeed/FullSpeed IN INT/ISOC bandwidth in split and
idle microframes by introduing a bandwidth budget table;

Signed-off-by: Chunfeng Yun 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-mtk-sch.c | 162 +---
 drivers/usb/host/xhci-mtk.h |   2 +
 2 files changed, 104 insertions(+), 60 deletions(-)

diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
index 057f453..7efd890 100644
--- a/drivers/usb/host/xhci-mtk-sch.c
+++ b/drivers/usb/host/xhci-mtk-sch.c
@@ -18,6 +18,11 @@
 #define HS_BW_BOUNDARY 6144
 /* usb2 spec section11.18.1: at most 188 FS bytes per microframe */
 #define FS_PAYLOAD_MAX 188
+/*
+ * max number of microframes for split transfer,
+ * for fs isoc in : 1 ss + 1 idle + 7 cs
+ */
+#define TT_MICROFRAMES_MAX 9
 
 /* mtk scheduler bitmasks */
 #define EP_BPKTS(p)((p) & 0x3f)
@@ -64,20 +69,57 @@ static int get_bw_index(struct xhci_hcd *xhci, struct 
usb_device *udev,
return bw_index;
 }
 
+static u32 get_esit(struct xhci_ep_ctx *ep_ctx)
+{
+   u32 esit;
+
+   esit = 1 << CTX_TO_EP_INTERVAL(le32_to_cpu(ep_ctx->ep_info));
+   if (esit > XHCI_MTK_MAX_ESIT)
+   esit = XHCI_MTK_MAX_ESIT;
+
+   return esit;
+}
+
+static struct mu3h_sch_ep_info *create_sch_ep(struct usb_device *udev,
+   struct usb_host_endpoint *ep, struct xhci_ep_ctx *ep_ctx)
+{
+   struct mu3h_sch_ep_info *sch_ep;
+   u32 len_bw_budget_table;
+   size_t mem_size;
+
+   if (is_fs_or_ls(udev->speed))
+   len_bw_budget_table = TT_MICROFRAMES_MAX;
+   else if ((udev->speed == USB_SPEED_SUPER)
+   && usb_endpoint_xfer_isoc(&ep->desc))
+   len_bw_budget_table = get_esit(ep_ctx);
+   else
+   len_bw_budget_table = 1;
+
+   mem_size = sizeof(struct mu3h_sch_ep_info) +
+   len_bw_budget_table * sizeof(u32);
+   sch_ep = kzalloc(mem_size, GFP_KERNEL);
+   if (!sch_ep)
+   return ERR_PTR(-ENOMEM);
+
+   sch_ep->ep = ep;
+
+   return sch_ep;
+}
+
 static void setup_sch_info(struct usb_device *udev,
struct xhci_ep_ctx *ep_ctx, struct mu3h_sch_ep_info *sch_ep)
 {
u32 ep_type;
-   u32 ep_interval;
-   u32 max_packet_size;
+   u32 maxpkt;
u32 max_burst;
u32 mult;
u32 esit_pkts;
u32 max_esit_payload;
+   u32 *bwb_table = sch_ep->bw_budget_table;
+   int i;
 
ep_type = CTX_TO_EP_TYPE(le32_to_cpu(ep_ctx->ep_info2));
-   ep_interval = CTX_TO_EP_INTERVAL(le32_to_cpu(ep_ctx->ep_info));
-   max_packet_size = MAX_PACKET_DECODED(le32_to_cpu(ep_ctx->ep_info2));
+   maxpkt = MAX_PACKET_DECODED(le32_to_cpu(ep_ctx->ep_info2));
max_burst = CTX_TO_MAX_BURST(le32_to_cpu(ep_ctx->ep_info2));
mult = CTX_TO_EP_MULT(le32_to_cpu(ep_ctx->ep_info));
max_esit_payload =
@@ -85,9 +127,10 @@ static void setup_sch_info(struct usb_device *udev,
le32_to_cpu(ep_ctx->ep_info)) << 16) |
 CTX_TO_MAX_ESIT_PAYLOAD(le32_to_cpu(ep_ctx->tx_info));
 
-   sch_ep->esit = 1 << ep_interval;
+   sch_ep->esit = get_esit(ep_ctx);
sch_ep->offset = 0;
sch_ep->burst_mode = 0;
+   sch_ep->repeat = 0;
 
if (udev->speed == USB_SPEED_HIGH) {
sch_ep->cs_count = 0;
@@ -98,7 +141,6 @@ static void setup_sch_info(struct usb_device *udev,
 * in a interval
 */
sch_ep->num_budget_microframes = 1;
-   sch_ep->repeat = 0;
 
/*
 * xHCI spec section6.2.3.4
@@ -106,26 +148,30 @@ static void setup_sch_info(struct usb_device *udev,
 * opportunities per microframe
 */
sch_ep->pkts = max_burst + 1;
-   sch_ep->bw_cost_per_microframe = max_packet_size * sch_ep->pkts;
+   sch_ep->bw_cost_per_microframe = maxpkt * sch_ep->pkts;
+   bwb_table[0] = sch_ep->bw_cost_per_microframe;
} else if (udev->speed == USB_SPEED_SUPER) {
/* usb3_r1 spec section4.4.7 & 4.4.8 */
sch_ep->cs_count = 0;
+   sch_ep->burst_mode = 1;
/*
 * some device's (d)wBytesPerInterval is set as 0,
 * then max_esit_payload is 0, so evaluate esit_pkts from
 * mult and burst
 */
-   esit_pkts = DIV_ROUND_UP(max_esit_payload, max_packet_size);
+   esit_pkts = DIV_ROUND_UP(max_esit_payload, maxpkt);
if (esit_pkts == 0)
esit_pkts = (mult + 1) * (max_burst + 1);
 
if (ep_type == INT_IN_EP || ep_type == INT_OUT_EP) {
sch_ep->pkts = esit_pkts;
sch_ep->num_budget_microframes = 1;
-

[PATCH 06/10] usb: xhci-mtk: supports SSP without external USB3 gen2 hub

[Mathias Nyman]

From: Chunfeng Yun 

Supports SSP scheduling only for SSP device directly connected
to root hub but not through external USB3 gen2 hub which need
use a new scheduling way.

Signed-off-by: Chunfeng Yun 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-mtk-sch.c | 17 +++--
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
index 36050a1..fea 100644
--- a/drivers/usb/host/xhci-mtk-sch.c
+++ b/drivers/usb/host/xhci-mtk-sch.c
@@ -13,6 +13,7 @@
 #include "xhci.h"
 #include "xhci-mtk.h"
 
+#define SSP_BW_BOUNDARY13
 #define SS_BW_BOUNDARY 51000
 /* table 5-5. High-speed Isoc Transaction Limits in usb_20 spec */
 #define HS_BW_BOUNDARY 6144
@@ -25,7 +26,7 @@
 #define TT_MICROFRAMES_MAX 9
 
 /* mtk scheduler bitmasks */
-#define EP_BPKTS(p)((p) & 0x3f)
+#define EP_BPKTS(p)((p) & 0x7f)
 #define EP_BCSCOUNT(p) (((p) & 0x7) << 8)
 #define EP_BBM(p)  ((p) << 11)
 #define EP_BOFFSET(p)  ((p) & 0x3fff)
@@ -56,7 +57,7 @@ static int get_bw_index(struct xhci_hcd *xhci, struct 
usb_device *udev,
 
virt_dev = xhci->devs[udev->slot_id];
 
-   if (udev->speed == USB_SPEED_SUPER) {
+   if (udev->speed >= USB_SPEED_SUPER) {
if (usb_endpoint_dir_out(&ep->desc))
bw_index = (virt_dev->real_port - 1) * 2;
else
@@ -177,7 +178,7 @@ static struct mu3h_sch_ep_info *create_sch_ep(struct 
usb_device *udev,
 
if (is_fs_or_ls(udev->speed))
len_bw_budget_table = TT_MICROFRAMES_MAX;
-   else if ((udev->speed == USB_SPEED_SUPER)
+   else if ((udev->speed >= USB_SPEED_SUPER)
&& usb_endpoint_xfer_isoc(&ep->desc))
len_bw_budget_table = get_esit(ep_ctx);
else
@@ -249,7 +250,7 @@ static void setup_sch_info(struct usb_device *udev,
sch_ep->pkts = max_burst + 1;
sch_ep->bw_cost_per_microframe = maxpkt * sch_ep->pkts;
bwb_table[0] = sch_ep->bw_cost_per_microframe;
-   } else if (udev->speed == USB_SPEED_SUPER) {
+   } else if (udev->speed >= USB_SPEED_SUPER) {
/* usb3_r1 spec section4.4.7 & 4.4.8 */
sch_ep->cs_count = 0;
sch_ep->burst_mode = 1;
@@ -511,8 +512,12 @@ static int check_sch_bw(struct usb_device *udev,
break;
}
 
-   bw_boundary = (udev->speed == USB_SPEED_SUPER)
-   ? SS_BW_BOUNDARY : HS_BW_BOUNDARY;
+   if (udev->speed == USB_SPEED_SUPER_PLUS)
+   bw_boundary = SSP_BW_BOUNDARY;
+   else if (udev->speed == USB_SPEED_SUPER)
+   bw_boundary = SS_BW_BOUNDARY;
+   else
+   bw_boundary = HS_BW_BOUNDARY;
 
/* check bandwidth */
if (min_bw > bw_boundary)
-- 
2.7.4

[PATCH 10/10] xhci-pci: allow host runtime PM as default for Intel Alpine and Titan Ridge

[Mathias Nyman]

The xhci controller on Alpine and Titan Ridge keeps the whole thunderbolt
awake if the host controller is not allowed tp sleep.
This is the case even if no USB devices are connected to the host.

Because of this bigger impact, allow runtime pm as default for these xhci
controllers in the driver.

Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-pci.c | 24 
 drivers/usb/host/xhci.h |  1 +
 2 files changed, 25 insertions(+)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index aef66ad..213af17 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -41,6 +41,13 @@
 #define PCI_DEVICE_ID_INTEL_BROXTON_B_XHCI 0x1aa8
 #define PCI_DEVICE_ID_INTEL_APL_XHCI   0x5aa8
 #define PCI_DEVICE_ID_INTEL_DNV_XHCI   0x19d0
+#define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_2C_XHCI   0x15b5
+#define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_4C_XHCI   0x15b6
+#define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_2C_XHCI 0x15db
+#define PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_4C_XHCI 0x15d4
+#define PCI_DEVICE_ID_INTEL_TITAN_RIDGE_2C_XHCI0x15e9
+#define PCI_DEVICE_ID_INTEL_TITAN_RIDGE_4C_XHCI0x15ec
+#define PCI_DEVICE_ID_INTEL_TITAN_RIDGE_DD_XHCI0x15f0
 
 #define PCI_DEVICE_ID_AMD_PROMONTORYA_40x43b9
 #define PCI_DEVICE_ID_AMD_PROMONTORYA_30x43ba
@@ -191,6 +198,16 @@ static void xhci_pci_quirks(struct device *dev, struct 
xhci_hcd *xhci)
 pdev->device == PCI_DEVICE_ID_INTEL_DNV_XHCI))
xhci->quirks |= XHCI_MISSING_CAS;
 
+   if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
+   (pdev->device == PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_2C_XHCI ||
+pdev->device == PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_4C_XHCI ||
+pdev->device == PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_2C_XHCI ||
+pdev->device == PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_C_4C_XHCI ||
+pdev->device == PCI_DEVICE_ID_INTEL_TITAN_RIDGE_2C_XHCI ||
+pdev->device == PCI_DEVICE_ID_INTEL_TITAN_RIDGE_4C_XHCI ||
+pdev->device == PCI_DEVICE_ID_INTEL_TITAN_RIDGE_DD_XHCI))
+   xhci->quirks |= XHCI_DEFAULT_PM_RUNTIME_ALLOW;
+
if (pdev->vendor == PCI_VENDOR_ID_ETRON &&
pdev->device == PCI_DEVICE_ID_EJ168) {
xhci->quirks |= XHCI_RESET_ON_RESUME;
@@ -334,6 +351,9 @@ static int xhci_pci_probe(struct pci_dev *dev, const struct 
pci_device_id *id)
/* USB-2 and USB-3 roothubs initialized, allow runtime pm suspend */
pm_runtime_put_noidle(&dev->dev);
 
+   if (xhci->quirks & XHCI_DEFAULT_PM_RUNTIME_ALLOW)
+   pm_runtime_allow(&dev->dev);
+
return 0;
 
 put_usb3_hcd:
@@ -351,6 +371,10 @@ static void xhci_pci_remove(struct pci_dev *dev)
 
xhci = hcd_to_xhci(pci_get_drvdata(dev));
xhci->xhc_state |= XHCI_STATE_REMOVING;
+
+   if (xhci->quirks & XHCI_DEFAULT_PM_RUNTIME_ALLOW)
+   pm_runtime_forbid(&dev->dev);
+
if (xhci->shared_hcd) {
usb_remove_hcd(xhci->shared_hcd);
usb_put_hcd(xhci->shared_hcd);
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index b635785..bf0b369 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1848,6 +1848,7 @@ struct xhci_hcd {
 #define XHCI_SUSPEND_DELAY BIT_ULL(30)
 #define XHCI_INTEL_USB_ROLE_SW BIT_ULL(31)
 #define XHCI_ZERO_64B_REGS BIT_ULL(32)
+#define XHCI_DEFAULT_PM_RUNTIME_ALLOW  BIT_ULL(33)
 
unsigned intnum_active_eps;
unsigned intlimit_active_eps;
-- 
2.7.4

[PATCH 08/10] usb: host: xhci-plat: add platform TPL support

[Mathias Nyman]

From: Peter Chen 

The TPL support is used to identify targeted devices during
EH2.0 and EH3.0 certification test, the user can add "tpl-support"
at dts to enable this feature.

Signed-off-by: Peter Chen 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-plat.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 8dc77e3..1924ffd 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "xhci.h"
 #include "xhci-plat.h"
@@ -300,6 +301,8 @@ static int xhci_plat_probe(struct platform_device *pdev)
hcd->skip_phy_initialization = 1;
}
 
+   hcd->tpl_support = of_usb_host_tpl_support(sysdev->of_node);
+   xhci->shared_hcd->tpl_support = hcd->tpl_support;
ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
if (ret)
goto disable_usb_phy;
-- 
2.7.4

[PATCH 09/10] xhci: Use soft retry to recover faster from transaction errors

[Mathias Nyman]

Use soft retry to recover from a USB Transaction Errors that are caused by
temporary error conditions. The USB device is not aware that the xHC
has halted the endpoint, and will be waiting for another retry

A Soft Retry perform additional retries and recover from an error which has
caused the xHC to halt an endpoint.

Soft retry has some limitations:
Soft Retry attempts shall not be performed on Isoch endpoints
Soft Retry attempts shall not be performed if the device is behind a TT in
a HS Hub

Software shall limit the number of unsuccessful Soft Retry attempts to
prevent an infinite loop.

For more details on Soft retry see xhci specs  4.6.8.1

Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c | 19 +++
 drivers/usb/host/xhci.h  |  2 ++
 2 files changed, 21 insertions(+)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index f0a99aa..c41341e 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1155,6 +1155,10 @@ static void xhci_handle_cmd_reset_ep(struct xhci_hcd 
*xhci, int slot_id,
/* Clear our internal halted state */
xhci->devs[slot_id]->eps[ep_index].ep_state &= ~EP_HALTED;
}
+
+   /* if this was a soft reset, then restart */
+   if ((le32_to_cpu(trb->generic.field[3])) & TRB_TSP)
+   ring_doorbell_for_active_rings(xhci, slot_id, ep_index);
 }
 
 static void xhci_handle_cmd_enable_slot(struct xhci_hcd *xhci, int slot_id,
@@ -2132,10 +2136,16 @@ static int process_bulk_intr_td(struct xhci_hcd *xhci, 
struct xhci_td *td,
union xhci_trb *ep_trb, struct xhci_transfer_event *event,
struct xhci_virt_ep *ep, int *status)
 {
+   struct xhci_slot_ctx *slot_ctx;
struct xhci_ring *ep_ring;
u32 trb_comp_code;
u32 remaining, requested, ep_trb_len;
+   unsigned int slot_id;
+   int ep_index;
 
+   slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->flags));
+   slot_ctx = xhci_get_slot_ctx(xhci, xhci->devs[slot_id]->out_ctx);
+   ep_index = TRB_TO_EP_ID(le32_to_cpu(event->flags)) - 1;
ep_ring = xhci_dma_to_transfer_ring(ep, le64_to_cpu(event->buffer));
trb_comp_code = GET_COMP_CODE(le32_to_cpu(event->transfer_len));
remaining = EVENT_TRB_LEN(le32_to_cpu(event->transfer_len));
@@ -2144,6 +2154,7 @@ static int process_bulk_intr_td(struct xhci_hcd *xhci, 
struct xhci_td *td,
 
switch (trb_comp_code) {
case COMP_SUCCESS:
+   ep_ring->err_count = 0;
/* handle success with untransferred data as short packet */
if (ep_trb != td->last_trb || remaining) {
xhci_warn(xhci, "WARN Successful completion on short 
TX\n");
@@ -2167,6 +2178,14 @@ static int process_bulk_intr_td(struct xhci_hcd *xhci, 
struct xhci_td *td,
ep_trb_len  = 0;
remaining   = 0;
break;
+   case COMP_USB_TRANSACTION_ERROR:
+   if ((ep_ring->err_count++ > MAX_SOFT_RETRY) ||
+   le32_to_cpu(slot_ctx->tt_info) & TT_SLOT)
+   break;
+   *status = 0;
+   xhci_cleanup_halted_endpoint(xhci, slot_id, ep_index,
+   ep_ring->stream_id, td, EP_SOFT_RESET);
+   return 0;
default:
/* do nothing */
break;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 6230a57..b635785 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1496,6 +1496,7 @@ static inline const char *xhci_trb_type_string(u8 type)
 /* How much data is left before the 64KB boundary? */
 #define TRB_BUFF_LEN_UP_TO_BOUNDARY(addr)  (TRB_MAX_BUFF_SIZE - \
(addr & (TRB_MAX_BUFF_SIZE - 1)))
+#define MAX_SOFT_RETRY 3
 
 struct xhci_segment {
union xhci_trb  *trbs;
@@ -1583,6 +1584,7 @@ struct xhci_ring {
 * if we own the TRB (if we are the consumer).  See section 4.9.1.
 */
u32 cycle_state;
+   unsigned interr_count;
unsigned intstream_id;
unsigned intnum_segs;
unsigned intnum_trbs_free;
-- 
2.7.4

[PATCH 07/10] usb: typec: pci: Enable Intel USB role mux on Apollo Lake platforms

[Mathias Nyman]

From: Heikki Krogerus 

Intel Apollo Lake has the same internal USB role mux as
Intel Cherry Trail.

Signed-off-by: Heikki Krogerus 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-pci.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 6372edf..aef66ad 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -179,10 +179,12 @@ static void xhci_pci_quirks(struct device *dev, struct 
xhci_hcd *xhci)
xhci->quirks |= XHCI_PME_STUCK_QUIRK;
}
if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
-pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI) {
+   pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI)
xhci->quirks |= XHCI_SSIC_PORT_UNUSED;
+   if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
+   (pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI ||
+pdev->device == PCI_DEVICE_ID_INTEL_APL_XHCI))
xhci->quirks |= XHCI_INTEL_USB_ROLE_SW;
-   }
if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
(pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI ||
 pdev->device == PCI_DEVICE_ID_INTEL_APL_XHCI ||
-- 
2.7.4

[PATCH 05/10] usb: xhci-mtk: supports bandwidth scheduling with multi-TT

[Mathias Nyman]

From: Chunfeng Yun 

Supports LowSpeed and FullSpeed INT/ISOC bandwidth scheduling
with USB multi-TT

Signed-off-by: Chunfeng Yun 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-mtk-sch.c | 247 ++--
 drivers/usb/host/xhci-mtk.h |  21 
 2 files changed, 258 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
index 7efd890..36050a1 100644
--- a/drivers/usb/host/xhci-mtk-sch.c
+++ b/drivers/usb/host/xhci-mtk-sch.c
@@ -80,10 +80,98 @@ static u32 get_esit(struct xhci_ep_ctx *ep_ctx)
return esit;
 }
 
+static struct mu3h_sch_tt *find_tt(struct usb_device *udev)
+{
+   struct usb_tt *utt = udev->tt;
+   struct mu3h_sch_tt *tt, **tt_index, **ptt;
+   unsigned int port;
+   bool allocated_index = false;
+
+   if (!utt)
+   return NULL;/* Not below a TT */
+
+   /*
+* Find/create our data structure.
+* For hubs with a single TT, we get it directly.
+* For hubs with multiple TTs, there's an extra level of pointers.
+*/
+   tt_index = NULL;
+   if (utt->multi) {
+   tt_index = utt->hcpriv;
+   if (!tt_index) {/* Create the index array */
+   tt_index = kcalloc(utt->hub->maxchild,
+   sizeof(*tt_index), GFP_KERNEL);
+   if (!tt_index)
+   return ERR_PTR(-ENOMEM);
+   utt->hcpriv = tt_index;
+   allocated_index = true;
+   }
+   port = udev->ttport - 1;
+   ptt = &tt_index[port];
+   } else {
+   port = 0;
+   ptt = (struct mu3h_sch_tt **) &utt->hcpriv;
+   }
+
+   tt = *ptt;
+   if (!tt) {  /* Create the mu3h_sch_tt */
+   tt = kzalloc(sizeof(*tt), GFP_KERNEL);
+   if (!tt) {
+   if (allocated_index) {
+   utt->hcpriv = NULL;
+   kfree(tt_index);
+   }
+   return ERR_PTR(-ENOMEM);
+   }
+   INIT_LIST_HEAD(&tt->ep_list);
+   tt->usb_tt = utt;
+   tt->tt_port = port;
+   *ptt = tt;
+   }
+
+   return tt;
+}
+
+/* Release the TT above udev, if it's not in use */
+static void drop_tt(struct usb_device *udev)
+{
+   struct usb_tt *utt = udev->tt;
+   struct mu3h_sch_tt *tt, **tt_index, **ptt;
+   int i, cnt;
+
+   if (!utt || !utt->hcpriv)
+   return; /* Not below a TT, or never allocated */
+
+   cnt = 0;
+   if (utt->multi) {
+   tt_index = utt->hcpriv;
+   ptt = &tt_index[udev->ttport - 1];
+   /*  How many entries are left in tt_index? */
+   for (i = 0; i < utt->hub->maxchild; ++i)
+   cnt += !!tt_index[i];
+   } else {
+   tt_index = NULL;
+   ptt = (struct mu3h_sch_tt **)&utt->hcpriv;
+   }
+
+   tt = *ptt;
+   if (!tt || !list_empty(&tt->ep_list))
+   return; /* never allocated , or still in use*/
+
+   *ptt = NULL;
+   kfree(tt);
+
+   if (cnt == 1) {
+   utt->hcpriv = NULL;
+   kfree(tt_index);
+   }
+}
+
 static struct mu3h_sch_ep_info *create_sch_ep(struct usb_device *udev,
struct usb_host_endpoint *ep, struct xhci_ep_ctx *ep_ctx)
 {
struct mu3h_sch_ep_info *sch_ep;
+   struct mu3h_sch_tt *tt = NULL;
u32 len_bw_budget_table;
size_t mem_size;
 
@@ -101,6 +189,15 @@ static struct mu3h_sch_ep_info *create_sch_ep(struct 
usb_device *udev,
if (!sch_ep)
return ERR_PTR(-ENOMEM);
 
+   if (is_fs_or_ls(udev->speed)) {
+   tt = find_tt(udev);
+   if (IS_ERR(tt)) {
+   kfree(sch_ep);
+   return ERR_PTR(-ENOMEM);
+   }
+   }
+
+   sch_ep->sch_tt = tt;
sch_ep->ep = ep;
 
return sch_ep;
@@ -128,6 +225,8 @@ static void setup_sch_info(struct usb_device *udev,
 CTX_TO_MAX_ESIT_PAYLOAD(le32_to_cpu(ep_ctx->tx_info));
 
sch_ep->esit = get_esit(ep_ctx);
+   sch_ep->ep_type = ep_type;
+   sch_ep->maxpkt = maxpkt;
sch_ep->offset = 0;
sch_ep->burst_mode = 0;
sch_ep->repeat = 0;
@@ -197,8 +296,13 @@ static void setup_sch_info(struct usb_device *udev,
}
} else if (is_fs_or_ls(udev->speed)) {
sch_ep->pkts = 1; /* at most one packet for each microframe */
+
+   /*
+* num_budget_microframes and cs_count will be updated when
+* check TT for INT_OUT_EP, ISOC/INT_IN_EP type
+*/
sch_ep->cs_count = DIV_ROUND_UP(maxpkt, FS_PAYLOAD_MAX);
-   sch_

[PATCH 02/10] usb: xhci-mtk: use maximum ESIT payload of endpiont context

[Mathias Nyman]

From: Chunfeng Yun 

Make use of maximum ESIT payload of endpoint context to calculate
the number of packets to send in each ESIT

Signed-off-by: Chunfeng Yun 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-mtk-sch.c | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
index fa33d6e..46fe0a2 100644
--- a/drivers/usb/host/xhci-mtk-sch.c
+++ b/drivers/usb/host/xhci-mtk-sch.c
@@ -73,12 +73,17 @@ static void setup_sch_info(struct usb_device *udev,
u32 max_burst;
u32 mult;
u32 esit_pkts;
+   u32 max_esit_payload;
 
ep_type = CTX_TO_EP_TYPE(le32_to_cpu(ep_ctx->ep_info2));
ep_interval = CTX_TO_EP_INTERVAL(le32_to_cpu(ep_ctx->ep_info));
max_packet_size = MAX_PACKET_DECODED(le32_to_cpu(ep_ctx->ep_info2));
max_burst = CTX_TO_MAX_BURST(le32_to_cpu(ep_ctx->ep_info2));
mult = CTX_TO_EP_MULT(le32_to_cpu(ep_ctx->ep_info));
+   max_esit_payload =
+   (CTX_TO_MAX_ESIT_PAYLOAD_HI(
+   le32_to_cpu(ep_ctx->ep_info)) << 16) |
+CTX_TO_MAX_ESIT_PAYLOAD(le32_to_cpu(ep_ctx->tx_info));
 
sch_ep->esit = 1 << ep_interval;
sch_ep->offset = 0;
@@ -105,7 +110,15 @@ static void setup_sch_info(struct usb_device *udev,
} else if (udev->speed == USB_SPEED_SUPER) {
/* usb3_r1 spec section4.4.7 & 4.4.8 */
sch_ep->cs_count = 0;
-   esit_pkts = (mult + 1) * (max_burst + 1);
+   /*
+* some device's (d)wBytesPerInterval is set as 0,
+* then max_esit_payload is 0, so evaluate esit_pkts from
+* mult and burst
+*/
+   esit_pkts = DIV_ROUND_UP(max_esit_payload, max_packet_size);
+   if (esit_pkts == 0)
+   esit_pkts = (mult + 1) * (max_burst + 1);
+
if (ep_type == INT_IN_EP || ep_type == INT_OUT_EP) {
sch_ep->pkts = esit_pkts;
sch_ep->num_budget_microframes = 1;
-- 
2.7.4

[PATCH 00/10] xhci features for usb-next

[Mathias Nyman]

Hi Greg

A few new features for xhci, better transaction error handling,
default runtime PM allowing for Intel Alpine and Tiran Ridge xhci controllers,
and Mediatek related xhci improvements.

-Mathias

Chunfeng Yun (6):
  usb: xhci-mtk: resume USB3 roothub first
  usb: xhci-mtk: use maximum ESIT payload of endpiont context
  usb: xhci-mtk: fix ISOC error when interval is zero
  usb: xhci-mtk: improve bandwidth scheduling
  usb: xhci-mtk: supports bandwidth scheduling with multi-TT
  usb: xhci-mtk: supports SSP without external USB3 gen2 hub

Heikki Krogerus (1):
  usb: typec: pci: Enable Intel USB role mux on Apollo Lake platforms

Mathias Nyman (2):
  xhci: Use soft retry to recover faster from transaction errors
  xhci-pci: allow host runtime PM as default for Intel Alpine and Titan
Ridge

Peter Chen (1):
  usb: host: xhci-plat: add platform TPL support

 drivers/usb/host/xhci-mtk-sch.c | 429 +---
 drivers/usb/host/xhci-mtk.c |   4 +-
 drivers/usb/host/xhci-mtk.h |  23 +++
 drivers/usb/host/xhci-pci.c |  30 ++-
 drivers/usb/host/xhci-plat.c|   3 +
 drivers/usb/host/xhci-ring.c|  19 ++
 drivers/usb/host/xhci.h |   3 +
 7 files changed, 437 insertions(+), 74 deletions(-)

-- 
2.7.4

[PATCH 01/10] usb: xhci-mtk: resume USB3 roothub first

[Mathias Nyman]

From: Chunfeng Yun 

Give USB3 devices a better chance to enumerate at USB3 speeds if
they are connected to a suspended host.
Porting from "671ffdff5b13 xhci: resume USB 3 roothub first"

Signed-off-by: Chunfeng Yun 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-mtk.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-mtk.c b/drivers/usb/host/xhci-mtk.c
index 7334da9..71d0d33 100644
--- a/drivers/usb/host/xhci-mtk.c
+++ b/drivers/usb/host/xhci-mtk.c
@@ -642,10 +642,10 @@ static int __maybe_unused xhci_mtk_resume(struct device 
*dev)
xhci_mtk_host_enable(mtk);
 
xhci_dbg(xhci, "%s: restart port polling\n", __func__);
-   set_bit(HCD_FLAG_POLL_RH, &hcd->flags);
-   usb_hcd_poll_rh_status(hcd);
set_bit(HCD_FLAG_POLL_RH, &xhci->shared_hcd->flags);
usb_hcd_poll_rh_status(xhci->shared_hcd);
+   set_bit(HCD_FLAG_POLL_RH, &hcd->flags);
+   usb_hcd_poll_rh_status(hcd);
return 0;
 }
 
-- 
2.7.4

[PATCH 03/10] usb: xhci-mtk: fix ISOC error when interval is zero

[Mathias Nyman]

From: Chunfeng Yun 

If the interval equal zero, needn't round up to power of two
for the number of packets in each ESIT, so fix it.

Signed-off-by: Chunfeng Yun 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-mtk-sch.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-mtk-sch.c b/drivers/usb/host/xhci-mtk-sch.c
index 46fe0a2..057f453 100644
--- a/drivers/usb/host/xhci-mtk-sch.c
+++ b/drivers/usb/host/xhci-mtk-sch.c
@@ -126,7 +126,9 @@ static void setup_sch_info(struct usb_device *udev,
}
 
if (ep_type == ISOC_IN_EP || ep_type == ISOC_OUT_EP) {
-   if (esit_pkts <= sch_ep->esit)
+   if (sch_ep->esit == 1)
+   sch_ep->pkts = esit_pkts;
+   else if (esit_pkts <= sch_ep->esit)
sch_ep->pkts = 1;
else
sch_ep->pkts = roundup_pow_of_two(esit_pkts)
-- 
2.7.4

Re: [PATCH v12 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU

[Heikki Krogerus]

Hi,

On Thu, Sep 13, 2018 at 09:36:58AM +0200, Peter Rosin wrote:
> On 2018-09-13 00:22, Ajay Gupta wrote:
> > Hi Peter,
> > Can we get the working set in while the issues is being debugged?
> 
> I'm not the one making the decision, and I don't even know if this
> is going through the i2c or the usb tree? If it's going through the
> i2c tree you need a tag from the usb people (Greg?) on patch 2/2,
> and if it's going through the usb tree, you need a tag from Wolfram
> on patch 1/2. As I said, I'm not involved with that part, I'm just
> reviewing these patches because I felt like it.
> 
> The remaining issue that bothers me is the looping reads, and your
> email address reveals that you should be in a very good position to
> work out why they do not work, and fix it or let us know why they
> don't. However, your responses indicate that you have given up. That
> coupled with the fact that the datasheet is not publicly available
> (but why, seems a little over-protective to think the interface to an
> i2c controller is worth all that much) makes me think that perhaps
> this little detail might never be fixed if it's not fixed now. Once
> merged, there is no pressure on you to actually do anything about it,
> and others are stuck in darkness without a spec.

I got similar impression. Ajay, you have really made it look like you
just want to dump the code even though it still has problems, then
lift your arms and never look back. I'm sure that is not the case, but
the fact that you are continuously making pleas, asking us to just
accept the code even though it is not ready, does give that
impression.

I can appreciate that you are in a hurry, and I know you have a lot
of other tasks that also need your attention, just like everybody, but
I'm asking you to take a deep breath, and take one more look at these
two drivers. Go over the code as a whole instead of trying to fix the
problems as fast as possible (you've sent a new version almost daily).
I'm certain you can figure out how to fix that last issue. You are
almost there.

And when you are ready, please include a cover letter in your next
version and provide some background for these patches if you can.
Ideally you could tell something about the platform that has that the
PD controller and the I2C host. There you can also mention which tree
you think these patches should go through, usb or i2c. I'm guessing
usb based on the fact that the I2C host controller driver seems to be
there just for the USB PD controller, at least for now.


Thanks,

-- 
heikki

Re: [PATCH v3] usb: typec: get the vbus source and charge values from the devicetree

[Angus Ainslie]

On 2018-09-13 01:27, Peter Chen wrote:
On Thu, Sep 13, 2018 at 12:35 AM Guenter Roeck  
wrote:


On Wed, Sep 12, 2018 at 10:08:58AM -0600, Angus Ainslie wrote:
> On 2018-09-11 09:33, Guenter Roeck wrote:
> >I cant put my finger on it but this seems wrong. As i said both src
> >and sink should never be true at the same time. I also din’t
> >understand why turning off src should power off your board. Ultimately
> >my concern is that we may be just painting over the real problem, and
> >that would be really bad to do with dt properties.
> >
>
> I agree that this doesn't seem like the correct way of solving the problem.
> On this HW (Emcraft iMX8M BSB) I think the PTN5110 chip has been connected
> correctly so I'm assuming that it is some quirk of the PTN5110.
>
> I didn't design the HW or the chip. This is a workaround for "quirky"
> hardware and there may be others that don't behave exactly as expected.
>

I wouldn't be that sure about that. It may as well be that the tcpc 
driver

and/or the tcpm driver are doing something wrong when initializing.

I didn't really understand the logs you sent out earlier. It looked 
like
the system would loose power if the TCPC_CMD_DISABLE_SRC_VBUS command 
is
sent.  That doesn't really make sense to me since it indicates that 
the
chip sources power to the remote, and turning that off should not 
result

in a local loss of power.

Note that the chip is supposed to be able to report if it is sourcing 
vbus
and if VBUS is present, in the POWER_STATUS register. Another question 
is
the content of the ROLE_CONTROL register when the system boots, and 
the

DEVICE_CAPABILITIES settings.

Overall I suspect that we don't handle startup for your system 
correctly
in the tcpc driver. The ideal solution would be to find a solution 
which

does not require any devicetree properties, but to do that we'll need
to get a better understanding about your system's requirements.

Guenter


Hi Angus,

Would you please check if below patch can fix your issue?

staging: typec: don't do vbus source disable for dead battery

In PTN5110 design, DisableSourceVBUS command also disables the sink
enable signal because the EN_SNK can be used to source higher voltage,
and, there is only one TCPC command to disable sourcing voltage without
telling whether to disable 5V or the high voltage, and to keep the
design simple they designed the PTN5110 to disable both. with this
fact, we use the flag drive_vbus to check if the source vbus enable was
issued, if yes we then do vbus source disable, in dead battery case,
we never did vbus source enable, so will not issue vbus source disable
command.



Thanks Peter, this sounds like the missing piece of information and I 
think some form of the code below will fix that.


There is still the issue that my board will need some way of controlling 
the initial state of vbus-sink.


@Guenter: would my initial patch be acceptable to set the default state 
of vbus-source and vbus-sink. Would you like some code to sanity check 
that both were not enabled at the same time ?



Acked-by: Peter Chen 
Signed-off-by: Li Jun 
---
 drivers/staging/typec/tcpci.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/typec/tcpci.c 
b/drivers/staging/typec/tcpci.c

index 2d4fbb8aac5e..7352207224b5 100644
--- a/drivers/staging/typec/tcpci.c
+++ b/drivers/staging/typec/tcpci.c
@@ -381,9 +381,8 @@ static int tcpci_set_vbus(struct tcpc_dev *tcpc,
bool source, bool sink)
  struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
  int ret;

- /* Disable both source and sink first before enabling anything */
-
- if (!source) {
+ /* Only disable source if it was enabled */
+ if (!source && tcpci->drive_vbus) {
  ret = regmap_write(tcpci->regmap, TCPC_COMMAND,
 TCPC_CMD_DISABLE_SRC_VBUS);
  if (ret < 0)


The version of struct tcpci doesn't have a drive_vbus. Where should 
drive_vbus get set and cleared ?


Is this a more complete version of what you intended ?

diff --git a/drivers/usb/typec/tcpci.c b/drivers/usb/typec/tcpci.c
index ac6b418b15f1..d6168163df7b 100644
--- a/drivers/usb/typec/tcpci.c
+++ b/drivers/usb/typec/tcpci.c
@@ -28,6 +28,7 @@ struct tcpci {
struct regmap *regmap;

bool controls_vbus;
+   bool drive_vbus;

struct tcpc_dev tcpc;
struct tcpci_data *data;
@@ -277,7 +278,9 @@ static int tcpci_set_vbus(struct tcpc_dev *tcpc, 
bool source, bool sink)


/* Disable both source and sink first before enabling anything 
*/


-   if (!source) {
+   if (!source && tcpci->drive_vbus) {
+   tcpci->drive_vbus = false;
+
ret = regmap_write(tcpci->regmap, TCPC_COMMAND,
   TCPC_CMD_DISABLE_SRC_VBUS);
if (ret < 0)
@@ -292,6 +295,8 @@ static int tcpci_set_vbus(struct tcpc_dev *tcpc, 
bool source, bool sink)

}

if (source) {
+   tcpci->drive_vbus = true;
+
ret = regmap_write(tcpci->regmap, TCPC_COMM

Re: [PATCH] usbcore: Select UAC3 configuration for audio if present

[Oliver Neukum]

On Di, 2018-09-11 at 23:36 +0530, saranya.go...@intel.com wrote:
> @@ -121,6 +132,23 @@ int usb_choose_configuration(struct usb_device *udev)
>  #endif
> }
>  
> +   /*
> +* Select first configuration as default for audio so that
> +* devices that don't comply with UAC3 protocol are supported.
> +* But, still iterate through other configurations and
> +* select UAC3 compliant config if present.
> +*/
> +   if (i == 0 && num_configs > 1 && desc && is_audio(desc)) {
> +   best = c;
> +   continue;
> +   }
> +
> +   if (i > 0 && desc && is_audio(desc)) {
> +   if (is_uac3_config(desc))
> +   best = c;
> +   break;
> +   }

Hi,

that looks like a special case. Couldn't we do

for (all configuration) {

1. take first
2. if this configuration is generic and the current is specific change
current
3. if if this configuration is the same interface class as current and
protocol is higher, change current

}
Regards
Oliver

Re: [PATCH v2 03/17] compat_ioctl: use correct compat_ptr() translation in drivers

[Felipe Balbi]

Arnd Bergmann  writes:

> A handful of drivers all have a trivial wrapper around their ioctl
> handler, but don't call the compat_ptr() conversion function at the
> moment. In practice this does not matter, since none of them are used
> on the s390 architecture and for all other architectures, compat_ptr()
> does not do anything, but using the new generic_compat_ioctl_ptrarg
> helper makes it more correct in theory, and simplifies the code.
>
> Signed-off-by: Arnd Bergmann 

Acked-by: Felipe Balbi 

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH 1/5] misc: rtsx_usb: Use USB remote wakeup signaling for card insertion detection

[Kai-Heng Feng]

Hi Alan, Ulf,

at 14:17, Kai-Heng Feng  wrote:


Although rtsx_usb doesn't support card removal detection, card insertion
will resume rtsx_usb by USB remote wakeup signaling.

When rtsx_usb gets resumed, also resumes its child devices,
rtsx_usb_sdmmc and rtsx_usb_ms, to notify them there's a card in its
slot.

Signed-off-by: Kai-Heng Feng 
---
 drivers/misc/cardreader/rtsx_usb.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/misc/cardreader/rtsx_usb.c  
b/drivers/misc/cardreader/rtsx_usb.c

index b97903ff1a72..f7a66f614085 100644
--- a/drivers/misc/cardreader/rtsx_usb.c
+++ b/drivers/misc/cardreader/rtsx_usb.c
@@ -723,8 +723,15 @@ static int rtsx_usb_suspend(struct usb_interface  
*intf, pm_message_t message)

return 0;
 }

+static int rtsx_usb_resume_child(struct device *dev, void *data)
+{
+   pm_request_resume(dev);
+   return 0;
+}
+
 static int rtsx_usb_resume(struct usb_interface *intf)
 {
+   device_for_each_child(&intf->dev, NULL, rtsx_usb_resume_child);
return 0;
 }

@@ -734,6 +741,7 @@ static int rtsx_usb_reset_resume(struct usb_interface  
*intf)

(struct rtsx_ucr *)usb_get_intfdata(intf);

rtsx_usb_reset_chip(ucr);
+   device_for_each_child(&intf->dev, NULL, rtsx_usb_resume_child);
return 0;
 }


I am working on the next version of this series, and the last missing  
puzzle is to differentiate system-wide resume from runtime resume in  
usb_driver's resume() and reset_resume() callback.


The parent device, rtsx_usb, has two child devices, rtsx_usb_ms and  
rtsx_usb_sdmmc.
pm_request_resume() is used in rtsx_usb's resume() to facilitate USB remote  
wakeup signaling, so we don't need to poll the card slot status.
But this has a side effect: during system resume the rtsx_usb calls  
pm_request_resume() in its resume(), so child devices calls their  
runtime_resume() instead of resume() callback.


So, is it reasonable to pass pm_message_t to resume() and reset_resume()  
callbacks and use PMSG_IS_AUTO() to differentiate them?


Kai-Heng



--
2.17.1

Re: [PATCH 1/1] usb: host: xhci-plat: add platform TPL support

[Mathias Nyman]

On 13.09.2018 09:54, Peter Chen wrote:

On Wed, Jul 18, 2018 at 2:45 PM Peter Chen  wrote:


The TPL support is used to identify targeted devices during
EH2.0 and EH3.0 certification test, the user can add "tpl-support"
at dts to enable this feature.

Signed-off-by: Peter Chen 
---
  drivers/usb/host/xhci-plat.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index c1b22fc64e38..270976cf4156 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -18,6 +18,7 @@
  #include 
  #include 
  #include 
+#include 

  #include "xhci.h"
  #include "xhci-plat.h"
@@ -299,6 +300,8 @@ static int xhci_plat_probe(struct platform_device *pdev)
 hcd->skip_phy_initialization = 1;
 }

+   hcd->tpl_support = of_usb_host_tpl_support(sysdev->of_node);
+   xhci->shared_hcd->tpl_support = hcd->tpl_support;
 ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
 if (ret)
 goto disable_usb_phy;
--
2.14.1



Ping...



Thanks, adding to queue

-Mathias

Re: [PATCH] option: Improve Quectel EP06 detection

[Kristian Evensen]

Hi Johan,

On Thu, Sep 13, 2018 at 11:17 AM Johan Hovold  wrote:
> Kristian would you mind giving it a try?

Yes, thank you very much. I will give the patches a go during the day
today and report back.

> Oh, also note that I dropped the RSVD(5) for the ADB interface in your
> patch since it uses a different subclass anyway. I'll submit both
> patches in a series.

Thanks, I completely forgot about that.

BR,
Kristian

[PATCH] usb: dwc3: add EXTCON dependency for qcom

[Arnd Bergmann]

Like the omap back-end, we get a link error with CONFIG_EXTCON=m
when building the qcom back-end into the kernel:

drivers/usb/dwc3/dwc3-qcom.o: In function `dwc3_qcom_probe':
dwc3-qcom.c:(.text+0x13dc): undefined reference to `extcon_get_edev_by_phandle'
dwc3-qcom.c:(.text+0x1b18): undefined reference to 
`devm_extcon_register_notifier'
dwc3-qcom.c:(.text+0x1b9c): undefined reference to `extcon_get_state'

Do the same thing as OMAP and add an explicit dependency on
EXTCON.

Fixes: a4333c3a6ba9 ("usb: dwc3: Add Qualcomm DWC3 glue driver")
Signed-off-by: Arnd Bergmann 
---
 drivers/usb/dwc3/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/Kconfig b/drivers/usb/dwc3/Kconfig
index 518ead12458d..1a0404fda596 100644
--- a/drivers/usb/dwc3/Kconfig
+++ b/drivers/usb/dwc3/Kconfig
@@ -113,7 +113,7 @@ config USB_DWC3_ST
 
 config USB_DWC3_QCOM
tristate "Qualcomm Platform"
-   depends on ARCH_QCOM || COMPILE_TEST
+   depends on EXTCON && (ARCH_QCOM || COMPILE_TEST)
depends on OF
default USB_DWC3
help
-- 
2.18.0

[PATCH 2/2] USB: serial: option: add two-endpoints device-id flag

[Johan Hovold]

Allow matching on interfaces having two endpoints by adding a new
device-id flag.

This allows for the handling of devices whose interface numbers can
change (e.g. Quectel EP06) to be contained in the device-id table.

Cc: stable 
Signed-off-by: Johan Hovold 
---
 drivers/usb/serial/option.c | 17 +++--
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index 382feafbd127..241a73e172a1 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -561,6 +561,9 @@ static void option_instat_callback(struct urb *urb);
 /* Interface is reserved */
 #define RSVD(ifnum)((BIT(ifnum) & 0xff) << 0)
 
+/* Interface must have two endpoints */
+#define NUMEP2 BIT(16)
+
 
 static const struct usb_device_id option_ids[] = {
{ USB_DEVICE(OPTION_VENDOR_ID, OPTION_PRODUCT_COLT) },
@@ -1082,7 +1085,7 @@ static const struct usb_device_id option_ids[] = {
{ USB_DEVICE(QUECTEL_VENDOR_ID, QUECTEL_PRODUCT_BG96),
  .driver_info = RSVD(4) },
{ USB_DEVICE_AND_INTERFACE_INFO(QUECTEL_VENDOR_ID, 
QUECTEL_PRODUCT_EP06, 0xff, 0xff, 0xff),
- .driver_info = RSVD(1) | RSVD(2) | RSVD(3) | RSVD(4) },
+ .driver_info = RSVD(1) | RSVD(2) | RSVD(3) | RSVD(4) | NUMEP2 },
{ USB_DEVICE_AND_INTERFACE_INFO(QUECTEL_VENDOR_ID, 
QUECTEL_PRODUCT_EP06, 0xff, 0, 0) },
{ USB_DEVICE(CMOTECH_VENDOR_ID, CMOTECH_PRODUCT_6001) },
{ USB_DEVICE(CMOTECH_VENDOR_ID, CMOTECH_PRODUCT_CMU_300) },
@@ -1986,7 +1989,6 @@ static int option_probe(struct usb_serial *serial,
 {
struct usb_interface_descriptor *iface_desc =
&serial->interface->cur_altsetting->desc;
-   struct usb_device_descriptor *dev_desc = &serial->dev->descriptor;
unsigned long device_flags = id->driver_info;
 
/* Never bind to the CD-Rom emulation interface */
@@ -2002,16 +2004,11 @@ static int option_probe(struct usb_serial *serial,
return -ENODEV;
 
/*
-* Don't bind to the QMI device of the Quectel EP06/EG06/EM06. Class,
-* subclass and protocol is 0xff for both the diagnostic port and the
-* QMI interface, but the diagnostic port only has two endpoints (QMI
-* has three).
+* Allow matching on bNumEndpoints for devices whose interface numbers
+* change (e.g. Quectel EP06).
 */
-   if (dev_desc->idVendor == cpu_to_le16(QUECTEL_VENDOR_ID) &&
-   dev_desc->idProduct == cpu_to_le16(QUECTEL_PRODUCT_EP06) &&
-   iface_desc->bInterfaceSubClass && iface_desc->bNumEndpoints == 3) {
+   if (device_flags & NUMEP2 && iface_desc->bNumEndpoints != 2)
return -ENODEV;
-   }
 
/* Store the device flags so we can use them during attach. */
usb_set_serial_data(serial, (void *)device_flags);
-- 
2.19.0

[PATCH 1/2] USB: serial: option: improve Quectel EP06 detection

[Johan Hovold]

From: Kristian Evensen 

The Quectel EP06 (and EM06/EG06) LTE modem supports updating the USB
configuration, without the VID/PID or configuration number changing.
When the configuration is updated and interfaces are added/removed, the
interface numbers are updated. This causes our current code for matching
EP06 not to work as intended, as the assumption about reserved
interfaces no longer holds. If for example the diagnostic (first)
interface is removed, option will (try to) bind to the QMI interface.

This patch improves EP06 detection by replacing the current match with
two matches, and those matches check class, subclass and protocol as
well as VID and PID. The diag interface exports class, subclass and
protocol as 0xff. For the other serial interfaces, class is 0xff and
subclass and protocol are both 0x0.

The modem can export the following devices and always in this order:
diag, nmea, at, ppp. qmi and adb. This means that diag can only ever be
interface 0, and interface numbers 1-5 should be marked as reserved. The
three other serial devices can have interface numbers 0-3, but I have
not marked any interfaces as reserved. The reason is that the serial
devices are the only interfaces exported by the device where subclass
and protocol is 0x0.

QMI exports the same class, subclass and protocol values as the diag
interface. However, the two interfaces have different number of
endpoints, QMI has three and diag two. I have added a check for number
of interfaces if VID/PID matches the EP06, and we ignore the device if
number of interfaces equals three (and subclass is set).

Signed-off-by: Kristian Evensen 
Acked-by: Dan Williams 
[ johan: drop uneeded RSVD(5) for ADB ]
Cc: stable 
Signed-off-by: Johan Hovold 
---
 drivers/usb/serial/option.c | 18 --
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index 0215b70c4efc..382feafbd127 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -1081,8 +1081,9 @@ static const struct usb_device_id option_ids[] = {
  .driver_info = RSVD(4) },
{ USB_DEVICE(QUECTEL_VENDOR_ID, QUECTEL_PRODUCT_BG96),
  .driver_info = RSVD(4) },
-   { USB_DEVICE(QUECTEL_VENDOR_ID, QUECTEL_PRODUCT_EP06),
- .driver_info = RSVD(4) | RSVD(5) },
+   { USB_DEVICE_AND_INTERFACE_INFO(QUECTEL_VENDOR_ID, 
QUECTEL_PRODUCT_EP06, 0xff, 0xff, 0xff),
+ .driver_info = RSVD(1) | RSVD(2) | RSVD(3) | RSVD(4) },
+   { USB_DEVICE_AND_INTERFACE_INFO(QUECTEL_VENDOR_ID, 
QUECTEL_PRODUCT_EP06, 0xff, 0, 0) },
{ USB_DEVICE(CMOTECH_VENDOR_ID, CMOTECH_PRODUCT_6001) },
{ USB_DEVICE(CMOTECH_VENDOR_ID, CMOTECH_PRODUCT_CMU_300) },
{ USB_DEVICE(CMOTECH_VENDOR_ID, CMOTECH_PRODUCT_6003),
@@ -1985,6 +1986,7 @@ static int option_probe(struct usb_serial *serial,
 {
struct usb_interface_descriptor *iface_desc =
&serial->interface->cur_altsetting->desc;
+   struct usb_device_descriptor *dev_desc = &serial->dev->descriptor;
unsigned long device_flags = id->driver_info;
 
/* Never bind to the CD-Rom emulation interface */
@@ -1999,6 +2001,18 @@ static int option_probe(struct usb_serial *serial,
if (device_flags & RSVD(iface_desc->bInterfaceNumber))
return -ENODEV;
 
+   /*
+* Don't bind to the QMI device of the Quectel EP06/EG06/EM06. Class,
+* subclass and protocol is 0xff for both the diagnostic port and the
+* QMI interface, but the diagnostic port only has two endpoints (QMI
+* has three).
+*/
+   if (dev_desc->idVendor == cpu_to_le16(QUECTEL_VENDOR_ID) &&
+   dev_desc->idProduct == cpu_to_le16(QUECTEL_PRODUCT_EP06) &&
+   iface_desc->bInterfaceSubClass && iface_desc->bNumEndpoints == 3) {
+   return -ENODEV;
+   }
+
/* Store the device flags so we can use them during attach. */
usb_set_serial_data(serial, (void *)device_flags);
 
-- 
2.19.0

Re: [PATCH] option: Improve Quectel EP06 detection

[Johan Hovold]

On Wed, Sep 12, 2018 at 10:34:43PM +0200, Bjørn Mork wrote:
> Dan Williams  writes:
> 
> > The fact that the firmware implementation has the ability to change the
> > endpoints is unrelated to Kristian's case, and that alone is
> > justification for this to be quirked in the driver.  People other than
> > Kristian will undoubtedly use the functionality, on platforms less
> > limited.
> 
> FWIW, I agree with Dan and Kristian on this. It's a documented feature,
> and it will be used. The reasons are irrelevant. The firmware
> implementation is inconvenient, but we should still strive to make it
> Just Work in Linux.  Kristian's solution does that.
>
> > Also most Huawei modems have the ability to change their layout and
> > configuration just like the EP06 via the U2DIAG and SETPORT commands.
> 
> Yes, but they are nice enough to use unique class/subclass/protocol
> triplets for their functions so it's easy to support the changing
> layout.  At least as long as they use their own VID and not some laptop
> vendor's..
> 
> The Sierra Wireless strategy, using fixed interface numbers leaving
> "holes" is another fine solution to the problem.  Or they could have
> allocated unique VIDs per function combination, as long as the number of
> valid combinations are low.  But they didn't.  It's not like it's the
> first bad firmware design we've had to deal with.  Let's just work
> around it, like we always do.  No need to make life difficult for end
> users just because Quectel makes life difficult for us.

Ok, thanks for everyone for your input. As Lars I was sceptical about
this, but if this is contained to Quectel and we have a solutions which
hopefully covers all permutations for their other devices, let's go
along with this.

I'd prefer seeing this contained in the device-id table as far as
possible rather than maintaining a second table of product ids in
probe() so I've cooked up a patch (on top of this one) adding a new
device-id match flag.

Kristian would you mind giving it a try? 

Oh, also note that I dropped the RSVD(5) for the ADB interface in your
patch since it uses a different subclass anyway. I'll submit both
patches in a series.

Thanks,
Johan

Re: [PATCH v12 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU

[Peter Rosin]

On 2018-09-13 00:22, Ajay Gupta wrote:
> Hi Peter,
> Can we get the working set in while the issues is being debugged?

I'm not the one making the decision, and I don't even know if this
is going through the i2c or the usb tree? If it's going through the
i2c tree you need a tag from the usb people (Greg?) on patch 2/2,
and if it's going through the usb tree, you need a tag from Wolfram
on patch 1/2. As I said, I'm not involved with that part, I'm just
reviewing these patches because I felt like it.

The remaining issue that bothers me is the looping reads, and your
email address reveals that you should be in a very good position to
work out why they do not work, and fix it or let us know why they
don't. However, your responses indicate that you have given up. That
coupled with the fact that the datasheet is not publicly available
(but why, seems a little over-protective to think the interface to an
i2c controller is worth all that much) makes me think that perhaps
this little detail might never be fixed if it's not fixed now. Once
merged, there is no pressure on you to actually do anything about it,
and others are stuck in darkness without a spec.

> So I assume nothing more left to check on this for now.

Have you hooked up a scope to the I2C bus while trying the various
attempts to get looping reads working? What makes a difference?
Have you talked to the hardware people and described what you have
attempted? Maybe they know what is missing?

Can the datasheet be made public so that someone with more passion
can take a crack at it?

Cheers,
Peter

Re: [PATCH v3] usb: typec: get the vbus source and charge values from the devicetree

[Peter Chen]

On Thu, Sep 13, 2018 at 12:35 AM Guenter Roeck  wrote:
>
> On Wed, Sep 12, 2018 at 10:08:58AM -0600, Angus Ainslie wrote:
> > On 2018-09-11 09:33, Guenter Roeck wrote:
> > >I cant put my finger on it but this seems wrong. As i said both src
> > >and sink should never be true at the same time. I also din’t
> > >understand why turning off src should power off your board. Ultimately
> > >my concern is that we may be just painting over the real problem, and
> > >that would be really bad to do with dt properties.
> > >
> >
> > I agree that this doesn't seem like the correct way of solving the problem.
> > On this HW (Emcraft iMX8M BSB) I think the PTN5110 chip has been connected
> > correctly so I'm assuming that it is some quirk of the PTN5110.
> >
> > I didn't design the HW or the chip. This is a workaround for "quirky"
> > hardware and there may be others that don't behave exactly as expected.
> >
>
> I wouldn't be that sure about that. It may as well be that the tcpc driver
> and/or the tcpm driver are doing something wrong when initializing.
>
> I didn't really understand the logs you sent out earlier. It looked like
> the system would loose power if the TCPC_CMD_DISABLE_SRC_VBUS command is
> sent.  That doesn't really make sense to me since it indicates that the
> chip sources power to the remote, and turning that off should not result
> in a local loss of power.
>
> Note that the chip is supposed to be able to report if it is sourcing vbus
> and if VBUS is present, in the POWER_STATUS register. Another question is
> the content of the ROLE_CONTROL register when the system boots, and the
> DEVICE_CAPABILITIES settings.
>
> Overall I suspect that we don't handle startup for your system correctly
> in the tcpc driver. The ideal solution would be to find a solution which
> does not require any devicetree properties, but to do that we'll need
> to get a better understanding about your system's requirements.
>
> Guenter

Hi Angus,

Would you please check if below patch can fix your issue?

staging: typec: don't do vbus source disable for dead battery

In PTN5110 design, DisableSourceVBUS command also disables the sink
enable signal because the EN_SNK can be used to source higher voltage,
and, there is only one TCPC command to disable sourcing voltage without
telling whether to disable 5V or the high voltage, and to keep the
design simple they designed the PTN5110 to disable both. with this
fact, we use the flag drive_vbus to check if the source vbus enable was
issued, if yes we then do vbus source disable, in dead battery case,
we never did vbus source enable, so will not issue vbus source disable
command.

Acked-by: Peter Chen 
Signed-off-by: Li Jun 
---
 drivers/staging/typec/tcpci.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/typec/tcpci.c b/drivers/staging/typec/tcpci.c
index 2d4fbb8aac5e..7352207224b5 100644
--- a/drivers/staging/typec/tcpci.c
+++ b/drivers/staging/typec/tcpci.c
@@ -381,9 +381,8 @@ static int tcpci_set_vbus(struct tcpc_dev *tcpc,
bool source, bool sink)
  struct tcpci *tcpci = tcpc_to_tcpci(tcpc);
  int ret;

- /* Disable both source and sink first before enabling anything */
-
- if (!source) {
+ /* Only disable source if it was enabled */
+ if (!source && tcpci->drive_vbus) {
  ret = regmap_write(tcpci->regmap, TCPC_COMMAND,
 TCPC_CMD_DISABLE_SRC_VBUS);
  if (ret < 0)