Re: [GIT PULL] USB-serial fixes for v4.19-rc7
On Sun, Sep 30, 2018 at 06:27:17PM +0200, Johan Hovold wrote: > Hi Greg, > > Here are some device-id patches for 4.19-rc7 (unless you prefer to hold them > off for 4.20). > > The option patches are a bit more intrusive than the usual device-id patches, > but allows us to support a particular Quectel modem in non-standard > configurations. To simplify dealing with stable-backports these also carry a > stable tag. Now pulled, thanks. greg k-h
Re: UVC gadget changes for v4.20
Hi Felipe, (CC'ing Greg, in case you're on vacation) Ping ? I'd really like to get this merged in v4.20. Do you think that would be possible ? On Tuesday, 25 September 2018 19:58:50 EEST Laurent Pinchart wrote: > Hi Felipe, > > The following changes since commit 5b394b2ddf0347bef56e50c69a58773c94343ff3: > > Linux 4.19-rc1 (2018-08-26 14:11:59 -0700) > > are available in the Git repository at: > > git://linuxtv.org/pinchartl/media.git tags/uvcg-20180925 > > for you to fetch changes up to 3fb2fd76eda265ce5421318de38dd9b9f7c54737: > > usb: gadget: uvc: configfs: Use %u to print unsigned int values > (2018-09-25 18:48:10 +0300) > > > UVC gadget updates for v4.20 > > - configfs cleanups, fixes and extensions > - Endianness fixes > - Miscellaneous cleanups > > > Joel Pepper (2): > usb: gadget: uvc: configfs: Add bFrameIndex attributes > usb: gadget: uvc: configfs: Prevent format changes after linking > header > > Laurent Pinchart (14): > usb: gadget: uvc: configfs: Don't wrap groups unnecessarily > usb: gadget: uvc: configfs: Add section header comments > usb: gadget: uvc: configfs: Drop leaked references to config items > usb: gadget: uvc: configfs: Allocate groups dynamically > usb: gadget: uvc: configfs: Add interface number attributes > usb: gadget: uvc: configfs: Add bFormatIndex attributes > usb: gadget: uvc: Factor out video USB request queueing > usb: gadget: uvc: Only halt video streaming endpoint in bulk mode > usb: gadget: uvc: Replace plain printk() with dev_*() > usb: gadget: uvc: Remove uvc_set_trace_param() function > usb: video: Fix endianness mismatches in descriptor structures > usb: gadget: uvc: configfs: Fix operation on big endian platforms > usb: gadget: uvc: configfs: Simplify attributes macros > usb: gadget: uvc: configfs: Use %u to print unsigned int values > > Paul Elder (1): > usb: gadget: uvc: configfs: Sort frame intervals upon writing > > Documentation/ABI/testing/configfs-usb-gadget-uvc | 24 + > drivers/usb/gadget/function/f_uvc.c | 57 +- > drivers/usb/gadget/function/u_uvc.h |3 + > drivers/usb/gadget/function/uvc.h | 16 +- > drivers/usb/gadget/function/uvc_configfs.c| 1168 + > drivers/usb/gadget/function/uvc_v4l2.c|4 +- > drivers/usb/gadget/function/uvc_video.c | 48 +- > drivers/usb/gadget/function/uvc_video.h |2 +- > include/uapi/linux/usb/video.h| 304 +++--- > 9 files changed, 916 insertions(+), 710 deletions(-) -- Regards, Laurent Pinchart
RE: [PATCH v12 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
Hi Wolfram, > > > > > > 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. > > We got confirmation from HW team about 4 byte read limitation. There > > has to be a STOP after every single read cycle. One read cycle > > supports maximum of > > 4 byte burst. I will update the patches with a comment on this. > > Could it be that this is more an SMBus controller than an I2C controller? > I haven't looked at the specs but maybe populating smbus_xfer instead of > master_xfer is an option here? I think its more of i2c controller and we do mention " .max_read_len = 4" in " struct i2c_adapter_quirks". Do you still see something missing here? Thanks Ajay --nvpublic
RE: [PATCH v12 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
Hi Peter, > 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. > > We got confirmation from HW team about 4 byte read limitation. There > > has to be a STOP after every single read cycle. One read cycle > > supports maximum of > > 4 byte burst. I will update the patches with a comment on this. > > Thanks for digging into this! And if the HW team says it's not possible, then > of > course my objection falls flat. However, you only mention "read cycle", and > based on your defines (I2C_MST_CNTL_CYCLE_TRIGGER) that seems to be > terminology from the spec. Comment "read cycle" and cycle in define I2C_MST_CNTL_CYCLE_TRIGGER is not related. I should say "There has to be a STOP after every single read". Thanks Ajay --nvpublic > Yet, you apparently do writes without triggering a > cycle. Do the HW team have anything to say about doing reads without > triggering a "cycle"? > > Cheers, > Peter
Re: [PATCH v12 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
On 2018-10-01 21:33, Ajay Gupta wrote: > 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. > We got confirmation from HW team about 4 byte read limitation. There has to > be a STOP after every single read cycle. One read cycle supports maximum of > 4 byte burst. I will update the patches with a comment on this. Thanks for digging into this! And if the HW team says it's not possible, then of course my objection falls flat. However, you only mention "read cycle", and based on your defines (I2C_MST_CNTL_CYCLE_TRIGGER) that seems to be terminology from the spec. Yet, you apparently do writes without triggering a cycle. Do the HW team have anything to say about doing reads without triggering a "cycle"? Cheers, Peter
Re: [PATCH 2/3] [PATCH v2 2/3] usb: typec: tcpm: Do not disconnect link for self powered devices
Sure. Just resent the patch stack !
On Mon, Oct 1, 2018 at 2:54 AM Heikki Krogerus
wrote:
>
> On Fri, Sep 28, 2018 at 04:47:02PM -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
>
> Could you rebase these on top of the latest usb-next. These are not
> going to apply cleanly on top of it because tcpm.c and all port
> drivers are now in their own directory drivers/usb/typec/tcpm/.
>
> Reviewed-by: Heikki Krogerus
>
> > -
> > Version history:
> >
> > V2:
> > Based on feedback from heikki.kroge...@linux.intel.com
> > - self_powered added to the struct tcpm_port which is populated from
> > a. "connector" node of the device tree in tcpm_fw_get_caps()
> > b. "self_powered" node of the tcpc_config in tcpm_copy_caps
> >
> > Based on feedbase from li...@roeck-us.net
> > - Code was refactored
> > - SRC_HARD_RESET_VBUS_OFF sets the link state to false based
> > on self_powered flag
> >
> > V1 located here:
> > https://lkml.org/lkml/2018/9/13/94
> > ---
> > drivers/usb/typec/tcpm.c | 12 ++--
> > include/linux/usb/tcpm.h | 1 +
> > 2 files changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> > index 4f1f4215f3d6e..c3ac0e46106b4 100644
> > --- a/drivers/usb/typec/tcpm.c
> > +++ b/drivers/usb/typec/tcpm.c
> > @@ -317,6 +317,9 @@ struct tcpm_port {
> > /* Deadline in jiffies to exit src_try_wait state */
> > unsigned long max_wait;
> >
> > + /* port belongs to a self powered device */
> > + bool self_powered;
> > +
> > #ifdef CONFIG_DEBUG_FS
> > struct dentry *dentry;
> > struct mutex logbuffer_lock;/* log buffer access lock */
> > @@ -3257,7 +3260,8 @@ static void run_state_machine(struct tcpm_port *port)
> > case SRC_HARD_RESET_VBUS_OFF:
> > tcpm_set_vconn(port, true);
> > tcpm_set_vbus(port, false);
> > - tcpm_set_roles(port, false, TYPEC_SOURCE, TYPEC_HOST);
> > + tcpm_set_roles(port, port->self_powered, TYPEC_SOURCE,
> > +TYPEC_HOST);
> > tcpm_set_state(port, SRC_HARD_RESET_VBUS_ON,
> > PD_T_SRC_RECOVER);
> > break;
> > case SRC_HARD_RESET_VBUS_ON:
> > @@ -3270,7 +3274,8 @@ 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);
> > + tcpm_set_roles(port, port->self_powered, 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
> > @@ -4409,6 +4414,8 @@ static int tcpm_fw_get_caps(struct tcpm_port *port,
> > return -EINVAL;
> > port->operating_snk_mw = mw / 1000;
> >
> > + port->self_powered = fwnode_property_read_bool(fwnode,
> > "self-powered");
> > +
> > return 0;
> > }
> >
> > @@ -4717,6 +4724,7 @@ static int tcpm_copy_caps(struct tcpm_port *port,
> > port->typec_caps.prefer_role = tcfg->default_role;
> > port->typec_caps.type = tcfg->type;
> > port->typec_caps.data = tcfg->data;
> > + port->self_powered = port->tcpc->config->self_powered;
> >
> > return 0;
> > }
> > diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
> > index 7e7fbfb84e8e3..50c74a77db55c 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 */
> >
> > const struct typec_altmode_desc *alt_modes;
> > };
>
> Thanks,
>
> --
> heikki
[PATCH v3 1/3] dt-bindings: connector: Add self-powered property
>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. Therefore it is necessary to know whether the port belongs to a device which is self powered or bus powered. This change adds "self-powered" flag to the connector class which present indicates that the port belongs to a device that is self powered. Else it is bus powered usb device. Signed-off-by: Badhri Jagan Sridharan --- Changes is v3: - Rebase on top of usb-next - no change w.r.t to this patch. Same as previous versions. No v2 version as the patch was introduced there. --- Documentation/devicetree/bindings/connector/usb-connector.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/connector/usb-connector.txt b/Documentation/devicetree/bindings/connector/usb-connector.txt index d90e17e2428b..a9a2f2fc44f2 100644 --- a/Documentation/devicetree/bindings/connector/usb-connector.txt +++ b/Documentation/devicetree/bindings/connector/usb-connector.txt @@ -14,6 +14,8 @@ Optional properties: - label: symbolic name for the connector, - type: size of the connector, should be specified in case of USB-A, USB-B non-fullsize connectors: "mini", "micro". +- self-powered: Set this property if the usb device that has its own power + source. Optional properties for usb-c-connector: - power-role: should be one of "source", "sink" or "dual"(DRP) if typec -- 2.19.0.605.g01d371f741-goog
[PATCH v3 3/3] usb: typec: tcpm: charge current handling for sink during hard reset
During the initial connect to a non-pd port, sink would hard reset
twice before deeming that the port partner is non-pd. TCPM sets the
the charge path to false during the hard reset. This causes unnecessary
connects/disconnects of charge path and makes port take longer to
charge from the non-pd ports. Avoid this by not setting the charge path
to false unless the partner has already identified to be pd capable.
When partner is a pd port, set the charge path to false in
SNK_HARD_RESET_SINK_OFF. Set the current limits to default value based
of CC pull up and resume the charge path when port enters
SNK_HARD_RESET_SINK_ON.
Signed-off-by: Badhri Jagan Sridharan
Changes in V3:
Rebase on top of usb-next
Changes in V2:
Based on feedback of ja...@codeaurora.org
- vsafe_5v_hard_reset flag from tcpc_config is removed
- Patch only differentiates between pd port partner and non-pd port
partner
V1 version of the patch is here:
https://lkml.org/lkml/2018/9/14/11
---
drivers/usb/typec/tcpm/tcpm.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index c3ac0e46106b..c25a69922ee6 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -3273,7 +3273,8 @@ 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->pd_capable)
+ tcpm_set_charge(port, false);
tcpm_set_roles(port, port->self_powered, TYPEC_SINK,
TYPEC_DEVICE);
/*
@@ -3305,6 +3306,12 @@ static void run_state_machine(struct tcpm_port *port)
* Similar, dual-mode ports in source mode should transition
* to PE_SNK_Transition_to_default.
*/
+ if (port->pd_capable) {
+ tcpm_set_current_limit(port,
+ tcpm_get_current_limit(port),
+ 5000);
+ tcpm_set_charge(port, true);
+ }
tcpm_set_attached_state(port, true);
tcpm_set_state(port, SNK_STARTUP, 0);
break;
--
2.19.0.605.g01d371f741-goog
[PATCH v3 2/3] usb: typec: tcpm: Do not disconnect link for self powered devices
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
-
Version history:
V3:
Rebase on top of usb-next
V2:
Based on feedback from heikki.kroge...@linux.intel.com
- self_powered added to the struct tcpm_port which is populated from
a. "connector" node of the device tree in tcpm_fw_get_caps()
b. "self_powered" node of the tcpc_config in tcpm_copy_caps
Based on feedbase from li...@roeck-us.net
- Code was refactored
- SRC_HARD_RESET_VBUS_OFF sets the link state to false based
on self_powered flag
V1 located here:
https://lkml.org/lkml/2018/9/13/94
---
drivers/usb/typec/tcpm/tcpm.c | 12 ++--
include/linux/usb/tcpm.h | 1 +
2 files changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 4f1f4215f3d6..c3ac0e46106b 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -317,6 +317,9 @@ struct tcpm_port {
/* Deadline in jiffies to exit src_try_wait state */
unsigned long max_wait;
+ /* port belongs to a self powered device */
+ bool self_powered;
+
#ifdef CONFIG_DEBUG_FS
struct dentry *dentry;
struct mutex logbuffer_lock;/* log buffer access lock */
@@ -3257,7 +3260,8 @@ static void run_state_machine(struct tcpm_port *port)
case SRC_HARD_RESET_VBUS_OFF:
tcpm_set_vconn(port, true);
tcpm_set_vbus(port, false);
- tcpm_set_roles(port, false, TYPEC_SOURCE, TYPEC_HOST);
+ tcpm_set_roles(port, port->self_powered, TYPEC_SOURCE,
+ TYPEC_HOST);
tcpm_set_state(port, SRC_HARD_RESET_VBUS_ON, PD_T_SRC_RECOVER);
break;
case SRC_HARD_RESET_VBUS_ON:
@@ -3270,7 +3274,8 @@ 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);
+ tcpm_set_roles(port, port->self_powered, 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
@@ -4409,6 +4414,8 @@ static int tcpm_fw_get_caps(struct tcpm_port *port,
return -EINVAL;
port->operating_snk_mw = mw / 1000;
+ port->self_powered = fwnode_property_read_bool(fwnode, "self-powered");
+
return 0;
}
@@ -4717,6 +4724,7 @@ static int tcpm_copy_caps(struct tcpm_port *port,
port->typec_caps.prefer_role = tcfg->default_role;
port->typec_caps.type = tcfg->type;
port->typec_caps.data = tcfg->data;
+ port->self_powered = port->tcpc->config->self_powered;
return 0;
}
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 */
const struct typec_altmode_desc *alt_modes;
};
--
2.19.0.605.g01d371f741-goog
Re: [PATCH v12 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
On Mon, Oct 01, 2018 at 07:33:02PM +, Ajay Gupta wrote: > 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. > We got confirmation from HW team about 4 byte read limitation. There has to > be a STOP after every single read cycle. One read cycle supports maximum of > 4 byte burst. I will update the patches with a comment on this. Could it be that this is more an SMBus controller than an I2C controller? I haven't looked at the specs but maybe populating smbus_xfer instead of master_xfer is an option here? signature.asc Description: PGP signature
RE: [PATCH v12 1/2] i2c: buses: add i2c bus driver for NVIDIA GPU
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. We got confirmation from HW team about 4 byte read limitation. There has to be a STOP after every single read cycle. One read cycle supports maximum of 4 byte burst. I will update the patches with a comment on this. Thanks Ajay --nvpublic > > > > --
Re: [PATCH] usbip: fix vhci_hcd controller counting
On 09/20/2018 02:29 PM, Maciej Żenczykowski wrote:
> From: Maciej Żenczykowski
>
> Without this usbip fails on a machine with devices
> that lexicographically come after vhci_hcd.
>
> ie.
> $ ls -l /sys/devices/platform
> ...
> drwxr-xr-x. 4 root root0 Sep 19 16:21 serial8250
> -rw-r--r--. 1 root root 4096 Sep 19 23:50 uevent
> drwxr-xr-x. 6 root root0 Sep 20 13:15 vhci_hcd.0
> drwxr-xr-x. 4 root root0 Sep 19 16:22 w83627hf.656
>
> Because it detects 'w83627hf.656' as another vhci_hcd controller,
> and then fails to be able to talk to it.
>
> Note: this doesn't actually fix usbip's support for multiple
> controllers... that's still broken for other reasons
> ("vhci_hcd.0" is hardcoded in a string macro), but is enough to
> actually make it work on the above machine.
>
> See also:
> https://bugzilla.redhat.com/show_bug.cgi?id=1631148
>
> Cc: Jonathan Dieter
> Cc: Valentina Manea
> Cc: Shuah Khan
> Cc: linux-usb@vger.kernel.org
> Signed-off-by: Maciej Żenczykowski
> ---
> tools/usb/usbip/libsrc/vhci_driver.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/usb/usbip/libsrc/vhci_driver.c
> b/tools/usb/usbip/libsrc/vhci_driver.c
> index 4204359c9fee..8159fd98680b 100644
> --- a/tools/usb/usbip/libsrc/vhci_driver.c
> +++ b/tools/usb/usbip/libsrc/vhci_driver.c
> @@ -150,7 +150,7 @@ static int get_nports(struct udev_device *hc_device)
>
> static int vhci_hcd_filter(const struct dirent *dirent)
> {
> - return strcmp(dirent->d_name, "vhci_hcd") >= 0;
> + return !strncmp(dirent->d_name, "vhci_hcd.", 9);
> }
>
> static int get_ncontrollers(void)
>
Thanks for the patch. Looks good to me.
Greg! Can you please pick this up.
Acked-by: Shuah Khan (Samsung OSG)
thanks,
-- Shuah
thanks,
-- Shuah
Re: USB ports on Thunderbolt 3 Dock always doesn't work after resume from suspend
On 01.10.2018 10:17, Heikki Krogerus wrote: +Mika, Mathias On Sat, Sep 29, 2018 at 08:51:43AM +0200, Ondrej Holy wrote: Hi, I recently got new Lenovo Thinkpad T480s with the ThinkPad Thunderbolt 3 Dock. The USB ports (but probably also audio and ethernet) on the dock always don't work after resume from suspend on up-to-date Fedora 29 with kernel-4.18.9-300.fc29.x86_64. HDMI port in the dock seems works (but with some delay). It doesn't work even with latest available kernel-4.19.0-0.rc5.git0.1.fc30.x86_64 from rawhide. Replugging the dock usually helps to fix that issue. Some probably relevant lines from dmesg after resume: [ 6528.075126] xhci_hcd :0b:00.0: Refused to change power state, currently in D3 [ 6528.075127] xhci_hcd :09:00.0: Refused to change power state, currently in D3 [ 6528.075139] xhci_hcd :0b:00.0: WARN: xHC restore state timeout [ 6528.075140] xhci_hcd :09:00.0: WARN: xHC restore state timeout [ 6528.075140] xhci_hcd :0b:00.0: PCI post-resume error -110! [ 6528.075141] xhci_hcd :09:00.0: PCI post-resume error -110! [ 6528.075141] xhci_hcd :0b:00.0: HC died; cleaning up [ 6528.075142] xhci_hcd :09:00.0: HC died; cleaning up [ 6528.075150] dpm_run_callback(): pci_pm_resume+0x0/0xa0 returns -110 [ 6528.075153] dpm_run_callback(): pci_pm_resume+0x0/0xa0 returns -110 [ 6528.075155] PM: Device :0b:00.0 failed to resume async: error -110 [ 6528.075157] PM: Device :09:00.0 failed to resume async: error -110 xhci driver will react like this if the xHC host is not powered up after resume. Log states it is still in D3 suspend state instead of D0. I'm guessing there is some powermanagement issue for thunderbolt connected PCI devices in resume, looking at dock firmware could be a good place to start. -Mathias
[PATCH] net: usbnet: make driver_info const
From: Ben Dooks
The driver_info field that is used for describing each of the usb-net
drivers using the usbnet.c core all declare their information as const
and the usbnet.c itself does not try and modify the struct.
It is therefore a good idea to make this const in the usbnet.c structure
in case anyone tries to modify it.
Signed-off-by: Ben Dooks
Signed-off-by: Ben Dooks
---
drivers/net/usb/usbnet.c | 12 ++--
include/linux/usb/usbnet.h | 2 +-
2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 770aa624147f..d6b3833c292d 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -802,7 +802,7 @@ static void usbnet_terminate_urbs(struct usbnet *dev)
int usbnet_stop (struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
- struct driver_info *info = dev->driver_info;
+ const struct driver_info *info = dev->driver_info;
int retval, pm, mpn;
clear_bit(EVENT_DEV_OPEN, &dev->flags);
@@ -865,7 +865,7 @@ int usbnet_open (struct net_device *net)
{
struct usbnet *dev = netdev_priv(net);
int retval;
- struct driver_info *info = dev->driver_info;
+ const struct driver_info *info = dev->driver_info;
if ((retval = usb_autopm_get_interface(dev->intf)) < 0) {
netif_info(dev, ifup, dev->net,
@@ -1205,7 +1205,7 @@ usbnet_deferred_kevent (struct work_struct *work)
}
if (test_bit (EVENT_LINK_RESET, &dev->flags)) {
- struct driver_info *info = dev->driver_info;
+ const struct driver_info *info = dev->driver_info;
int retval = 0;
clear_bit (EVENT_LINK_RESET, &dev->flags);
@@ -1353,7 +1353,7 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
unsigned intlength;
struct urb *urb = NULL;
struct skb_data *entry;
- struct driver_info *info = dev->driver_info;
+ const struct driver_info *info = dev->driver_info;
unsigned long flags;
int retval;
@@ -1646,7 +1646,7 @@ usbnet_probe (struct usb_interface *udev, const struct
usb_device_id *prod)
struct usbnet *dev;
struct net_device *net;
struct usb_host_interface *interface;
- struct driver_info *info;
+ const struct driver_info*info;
struct usb_device *xdev;
int status;
const char *name;
@@ -1662,7 +1662,7 @@ usbnet_probe (struct usb_interface *udev, const struct
usb_device_id *prod)
}
name = udev->dev.driver->name;
- info = (struct driver_info *) prod->driver_info;
+ info = (const struct driver_info *) prod->driver_info;
if (!info) {
dev_dbg (&udev->dev, "blacklisted by %s\n", name);
return -ENODEV;
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index e2ec3582e549..d8860f2d0976 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -28,7 +28,7 @@ struct usbnet {
/* housekeeping */
struct usb_device *udev;
struct usb_interface*intf;
- struct driver_info *driver_info;
+ const struct driver_info *driver_info;
const char *driver_name;
void*driver_priv;
wait_queue_head_t wait;
--
2.19.0
[PATCH] usb: xhci: pci: Enable Intel USB role mux on Apollo Lake platforms
Intel Apollo Lake has the same internal USB role mux as
Intel Cherry Trail.
Cc:
Signed-off-by: Heikki Krogerus
---
Hi,
The patch with the topic fixed.
Thanks,
---
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 6372edf339d9..aef66adfb0cb 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.19.0
Re: [RFT PATCH 2/2] xhci: handle port status events for removed USB3 hcd
On 28.09.2018 21:10, Jack Pham wrote: Hi Mathias, Jack, Peter, do these patches solve the remove issues you are seeing? At my two USB3 platforms, only apply the 1st patch can fix my problem. Maybe my USB3 port change interrupt occurs always before removing USB2 HCD. It's possible yes. Peter Ditto. I think the xhci_irq() is getting triggered by something during usb_remove_hcd() (usb_disconnect on the root hub?) but is able to complete before it returns. That is, the NULL pointer dereference is resolved yet I don't see that "ignore port event for removed USB3 hcd" message at all. Regardless, it's good to have here just in case, so Tested-by: Jack Pham Will you be sending this as separate patches for -rc vs -stable? Thanks, Jack Thanks, adding tested-by tags. I'll send them to -rc with stable tag, and then later send a backported version to older kernel once I have a upstream commit ID I can refer to. Thanks -Mathias
Re: [PATCH v2 RESEND 3/3] usb: typec: pci: Enable Intel USB role mux on Apollo Lake platforms
On Mon, Oct 01, 2018 at 06:44:51PM +0300, Heikki Krogerus wrote: > Hi, > > Not a biggie, but why was the topic for this patch changed? > > I'm not sure Apollo lakes even have Type-C connectors. The mux is > needed with the uAB connectors. My bad. The topic was wrong already when I send the patch to you. Sorry :-) Br, -- heikki
Re: [PATCH v2 RESEND 3/3] usb: typec: pci: Enable Intel USB role mux on Apollo Lake platforms
Hi,
Not a biggie, but why was the topic for this patch changed?
I'm not sure Apollo lakes even have Type-C connectors. The mux is
needed with the uAB connectors.
On Mon, Oct 01, 2018 at 06:36:09PM +0300, Mathias Nyman wrote:
> From: Heikki Krogerus
>
> Intel Apollo Lake has the same internal USB role mux as
> Intel Cherry Trail.
>
> Cc:
> 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 722860e..51dd8e0 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_SUNRISEPOINT_LP_XHCI ||
> --
> 2.7.4
--
heikki
[PATCH v2 RESEND 2/3] usb: xhci-mtk: resume USB3 roothub first
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" Cc: 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
Re: [PATCH v2 0/3] xhci fixes for usb-linus
On 28.09.2018 15:51, Greg KH wrote: On Thu, Sep 20, 2018 at 06:43:19PM +0300, Mathias Nyman wrote: Hi Greg Second try, shuffling patches between for-usb-linus and for-usb-next A few patches that makes sure USB3 devices enumerate to correct speed after resume on Mediatek hosts, enables role mux on Apollo lake platforms, and adds the missing cold attach status (CAS) bit quirk to Intel Sunrise Point xhci controllers. Changes since v1 - Moved following patches from this series to for-usb-next (4.20) xhci: Avoid USB autosuspend when resuming USB2 ports. usb: xhci: tegra: Firmware header is little endian - Added patches to this series from for-usb-next queue: usb: xhci-mtk: resume USB3 roothub first usb: typec: pci: Enable Intel USB role mux on Apollo Lake platforms - Added stable tags Chunfeng Yun (1): usb: xhci-mtk: resume USB3 roothub first Heikki Krogerus (1): usb: typec: pci: Enable Intel USB role mux on Apollo Lake platforms Mathias Nyman (1): xhci: Add missing CAS workaround for Intel Sunrise Point xHCI drivers/usb/host/xhci-mtk.c | 4 ++-- drivers/usb/host/xhci-pci.c | 8 ++-- 2 files changed, 8 insertions(+), 4 deletions(-) I only received 2 patches here, not 3, am I missing something on my end? Can you resend all of these again? Sure, resending -Mathias
[PATCH v2 RESEND 3/3] usb: typec: pci: Enable Intel USB role mux on Apollo Lake platforms
From: Heikki Krogerus
Intel Apollo Lake has the same internal USB role mux as
Intel Cherry Trail.
Cc:
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 722860e..51dd8e0 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_SUNRISEPOINT_LP_XHCI ||
--
2.7.4
[PATCH v2 RESEND 1/3] xhci: Add missing CAS workaround for Intel Sunrise Point xHCI
The workaround for missing CAS bit is also needed for xHC on Intel sunrisepoint PCH. For more details see: Intel 100/c230 series PCH specification update Doc #332692-006 Errata #8 Cc: Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-pci.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c index 6372edf..722860e 100644 --- a/drivers/usb/host/xhci-pci.c +++ b/drivers/usb/host/xhci-pci.c @@ -185,6 +185,8 @@ static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci) } if (pdev->vendor == PCI_VENDOR_ID_INTEL && (pdev->device == PCI_DEVICE_ID_INTEL_CHERRYVIEW_XHCI || +pdev->device == PCI_DEVICE_ID_INTEL_SUNRISEPOINT_LP_XHCI || +pdev->device == PCI_DEVICE_ID_INTEL_SUNRISEPOINT_H_XHCI || pdev->device == PCI_DEVICE_ID_INTEL_APL_XHCI || pdev->device == PCI_DEVICE_ID_INTEL_DNV_XHCI)) xhci->quirks |= XHCI_MISSING_CAS; -- 2.7.4
[PATCH v2 RESEND 0/3] xhci fixes for usb-linus
Hi Greg Resending v2 due to lost patch 3/3 Second try, shuffling patches between for-usb-linus and for-usb-next A few patches that makes sure USB3 devices enumerate to correct speed after resume on Mediatek hosts, enables role mux on Apollo lake platforms, and adds the missing cold attach status (CAS) bit quirk to Intel Sunrise Point xhci controllers. Changes since v1 - Moved following patches from this series to for-usb-next (4.20) xhci: Avoid USB autosuspend when resuming USB2 ports. usb: xhci: tegra: Firmware header is little endian - Added patches to this series from for-usb-next queue: usb: xhci-mtk: resume USB3 roothub first usb: typec: pci: Enable Intel USB role mux on Apollo Lake platforms - Added stable tags Chunfeng Yun (1): usb: xhci-mtk: resume USB3 roothub first Heikki Krogerus (1): usb: typec: pci: Enable Intel USB role mux on Apollo Lake platforms Mathias Nyman (1): xhci: Add missing CAS workaround for Intel Sunrise Point xHCI drivers/usb/host/xhci-mtk.c | 4 ++-- drivers/usb/host/xhci-pci.c | 8 ++-- 2 files changed, 8 insertions(+), 4 deletions(-) -- 2.7.4
Re: [PATCH] usb: roles: intel_xhci: Determine current role by DUAL_ROLE_CFG1
On Mon, Oct 01, 2018 at 04:23:31PM +0200, Hans de Goede wrote: > On 29-09-18 16:26, Yu Wang wrote: > > The USB PHY mux switch depend on both USB2/USB3 PHY state and xHCI/xDCI > > controller state. The role can't be switched if related states haven't > > satisfied. That is why we need to poll the DUAL_ROLE_CFG1 to check if > > the role switched successful or not. > > > > So the SW_IDPIN and SW_VBUS_VALID bits can't determine the current > > acting role. > > > > This patch changes the logic for getting role logic. > > I guess this matches up with the recent patches to change the > role-switching on non Cherry Trail devices to use the DRD_CONFIG bits > instead of the SW_IDPIN bit? > > AFAIK under normal circumstances with our current code, > DUAL_ROLE_CFG1 will always follow SW_IDPIN, so I'm not entirly sure > what you are trying to fix here? So if I've understood correctly, there are certain conditions inside both the xHCI and the DWC3 (which is here called xDCI) blocks that have to be met before the internal mux state actually gets changed. That means that there is no guarantee that the SW_IDPIN bit tells the actual state of the mux. > IOW what problem are you seeing without this patch and how does > this fix it ? If there is a change that we may in some situations read a wrong state for the mux, then that is a problem. But it would be good to explain the actual case that you are seeing in the commit message. And this patch should probable also be marked as a fix for the original commit that introduced the driver. > If this is related to the patch to use the DRD_CONFIG bits on > non Cherry Trail devices, then please first submit a new version > of the patch for that which leaves the functionality on CHT > devices as is (as discussed before). I'm not sure if this is directly related to that, but Yu, can you confirm that? I'm guessing that this is indirectly related to that patch. DRD_CONFIG bits apparently force the state of the mux, ignoring the state of xHCI and DWC3, so by using them the problem of reading wrong state for they mux could never happen. In any case, the rationale for the patch sounds perfectly reasonable to me. Even if we need to use the DRD_CONFIG bits in some cases, IMO we still should rely on the CFG1 to get the real state of the mux. Thanks, -- heikki
Re: [PATCH] usb: roles: intel_xhci: Determine current role by DUAL_ROLE_CFG1
Hi,
On 01-10-18 16:53, Wang, Yu1 wrote:
Hi Hans,
On 18-10-01 16:23:31, Hans de Goede wrote:
Hi,
On 29-09-18 16:26, Yu Wang wrote:
The USB PHY mux switch depend on both USB2/USB3 PHY state and xHCI/xDCI
controller state. The role can't be switched if related states haven't
satisfied. That is why we need to poll the DUAL_ROLE_CFG1 to check if
the role switched successful or not.
So the SW_IDPIN and SW_VBUS_VALID bits can't determine the current
acting role.
This patch changes the logic for getting role logic.
I guess this matches up with the recent patches to change the
role-switching on non Cherry Trail devices to use the DRD_CONFIG bits
instead of the SW_IDPIN bit?
Sorry, I just register this mailing list. Don't know the DRD_CONFIG
history... Can you please help share the archives link? Somehow, the
linux-usb archive link can't be opened successful in my environment, I
am not sure if I am checking the correct archive address.
I'm talking about this patch:
https://patchwork.kernel.org/patch/10591729/
AFAIK under normal circumstances with our current code,
DUAL_ROLE_CFG1 will always follow SW_IDPIN, so I'm not entirly sure
what you are trying to fix here?
I am developing on Intel ApolloLake platform. As my understanding, this
silicon logic for PHY role switch should be same compared between APL
and Cherry Trail.
As I mentioned in the commit message. The PHY switching is not only
depend on the SW_IDPIN bits. The SW_IDPIN/SW_VBUS_VALID are only the
"trigger" for PHY mux switch. But the silicon for DRD requires some
specific states of xHCI/dwc3 and USB2/USB3 PHYs. If they are not
satisfied. then the switch will be failed. That is why we poll the
CFG1.bit29 with timeout mechanism.
IOW what problem are you seeing without this patch and how does
this fix it ?
We met this issue with Intel ApolloLake platform, somehow the role
switch get failed with timeout in intel_xhci_usb_set_role. That is mean
the role set failed, but when trying to get the current acting role, it
indicate it is under USB_ROLE_DEVICE.
About the issue why switch failed. We are still under investigation. But
to report incorrect current acting device is not make sense. This patch
is resolving this issue.
If this is related to the patch to use the DRD_CONFIG bits on
non Cherry Trail devices, then please first submit a new version
of the patch for that which leaves the functionality on CHT
devices as is (as discussed before).
In my perspective, this patch should be compatible with all related
Intel platforms. From current software logic, we set the role switch
through SW_IDPIN/SW_VBUS_VALID bits, and we check if the current
acting mode to determine if set role is successful or not, and report
-ETIMEOUT if switch failed.
From user space perspective, set the sysfs to switch to device mode, but
get -ETIMEOUT which is means the device mode switch failed. And trying
to get the current mode through this sysfs, it is showing under device
mode... I think it is not make sense.
I understand with that explanation the proposed change seems
reasonable. I will test it on a Cherry Trail tablet (soonish I need
to make some time for this) and then give my acked-by.
Regards,
Hans
Regards,
Hans
Signed-off-by: Yu Wang
---
drivers/usb/roles/intel-xhci-usb-role-switch.c | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c
b/drivers/usb/roles/intel-xhci-usb-role-switch.c
index 1fb3dd0..c118c9a 100644
--- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
+++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
@@ -110,14 +110,18 @@ static enum usb_role intel_xhci_usb_get_role(struct
device *dev)
pm_runtime_get_sync(dev);
val = readl(data->base + DUAL_ROLE_CFG0);
- pm_runtime_put(dev);
- if (!(val & SW_IDPIN))
- role = USB_ROLE_HOST;
- else if (val & SW_VBUS_VALID)
- role = USB_ROLE_DEVICE;
- else
+ if ((val & SW_IDPIN) && !(val & SW_VBUS_VALID))
role = USB_ROLE_NONE;
+ else {
+ val = readl(data->base + DUAL_ROLE_CFG1);
+ if (val & HOST_MODE)
+ role = USB_ROLE_HOST;
+ else
+ role = USB_ROLE_DEVICE;
+ }
+
+ pm_runtime_put(dev);
return role;
}
Re: [PATCH] usb: roles: intel_xhci: Determine current role by DUAL_ROLE_CFG1
Hi,
On 29-09-18 16:26, Yu Wang wrote:
The USB PHY mux switch depend on both USB2/USB3 PHY state and xHCI/xDCI
controller state. The role can't be switched if related states haven't
satisfied. That is why we need to poll the DUAL_ROLE_CFG1 to check if
the role switched successful or not.
So the SW_IDPIN and SW_VBUS_VALID bits can't determine the current
acting role.
This patch changes the logic for getting role logic.
I guess this matches up with the recent patches to change the
role-switching on non Cherry Trail devices to use the DRD_CONFIG bits
instead of the SW_IDPIN bit?
AFAIK under normal circumstances with our current code,
DUAL_ROLE_CFG1 will always follow SW_IDPIN, so I'm not entirly sure
what you are trying to fix here?
IOW what problem are you seeing without this patch and how does
this fix it ?
If this is related to the patch to use the DRD_CONFIG bits on
non Cherry Trail devices, then please first submit a new version
of the patch for that which leaves the functionality on CHT
devices as is (as discussed before).
Regards,
Hans
Signed-off-by: Yu Wang
---
drivers/usb/roles/intel-xhci-usb-role-switch.c | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c
b/drivers/usb/roles/intel-xhci-usb-role-switch.c
index 1fb3dd0..c118c9a 100644
--- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
+++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
@@ -110,14 +110,18 @@ static enum usb_role intel_xhci_usb_get_role(struct
device *dev)
pm_runtime_get_sync(dev);
val = readl(data->base + DUAL_ROLE_CFG0);
- pm_runtime_put(dev);
- if (!(val & SW_IDPIN))
- role = USB_ROLE_HOST;
- else if (val & SW_VBUS_VALID)
- role = USB_ROLE_DEVICE;
- else
+ if ((val & SW_IDPIN) && !(val & SW_VBUS_VALID))
role = USB_ROLE_NONE;
+ else {
+ val = readl(data->base + DUAL_ROLE_CFG1);
+ if (val & HOST_MODE)
+ role = USB_ROLE_HOST;
+ else
+ role = USB_ROLE_DEVICE;
+ }
+
+ pm_runtime_put(dev);
return role;
}
Re: [PATCH] dt-bindings: usb-xhci: Document r8a7744 support
On Thu, Sep 27, 2018 at 02:42:38PM +0100, Biju Das wrote: > Document r8a7744 xhci support. The driver will use the fallback > compatible string "renesas,rcar-gen2-xhci", therefore no driver > change is needed. Reviewed-by: Simon Horman
Re: [PATCH] dt-bindings: usb: renesas_usbhs: Add support for r8a7744
On Thu, Sep 27, 2018 at 02:01:16PM +0100, Biju Das wrote: > Document support for RZ/G1N (R8A7744) SoC. > > Signed-off-by: Biju Das > Reviewed-by: Chris Paterson Reviewed-by: Simon Horman
Re: [PATCH 2/3] [PATCH v2 2/3] usb: typec: tcpm: Do not disconnect link for self powered devices
On Fri, Sep 28, 2018 at 04:47:02PM -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
Could you rebase these on top of the latest usb-next. These are not
going to apply cleanly on top of it because tcpm.c and all port
drivers are now in their own directory drivers/usb/typec/tcpm/.
Reviewed-by: Heikki Krogerus
> -
> Version history:
>
> V2:
> Based on feedback from heikki.kroge...@linux.intel.com
> - self_powered added to the struct tcpm_port which is populated from
> a. "connector" node of the device tree in tcpm_fw_get_caps()
> b. "self_powered" node of the tcpc_config in tcpm_copy_caps
>
> Based on feedbase from li...@roeck-us.net
> - Code was refactored
> - SRC_HARD_RESET_VBUS_OFF sets the link state to false based
> on self_powered flag
>
> V1 located here:
> https://lkml.org/lkml/2018/9/13/94
> ---
> drivers/usb/typec/tcpm.c | 12 ++--
> include/linux/usb/tcpm.h | 1 +
> 2 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/typec/tcpm.c b/drivers/usb/typec/tcpm.c
> index 4f1f4215f3d6e..c3ac0e46106b4 100644
> --- a/drivers/usb/typec/tcpm.c
> +++ b/drivers/usb/typec/tcpm.c
> @@ -317,6 +317,9 @@ struct tcpm_port {
> /* Deadline in jiffies to exit src_try_wait state */
> unsigned long max_wait;
>
> + /* port belongs to a self powered device */
> + bool self_powered;
> +
> #ifdef CONFIG_DEBUG_FS
> struct dentry *dentry;
> struct mutex logbuffer_lock;/* log buffer access lock */
> @@ -3257,7 +3260,8 @@ static void run_state_machine(struct tcpm_port *port)
> case SRC_HARD_RESET_VBUS_OFF:
> tcpm_set_vconn(port, true);
> tcpm_set_vbus(port, false);
> - tcpm_set_roles(port, false, TYPEC_SOURCE, TYPEC_HOST);
> + tcpm_set_roles(port, port->self_powered, TYPEC_SOURCE,
> +TYPEC_HOST);
> tcpm_set_state(port, SRC_HARD_RESET_VBUS_ON, PD_T_SRC_RECOVER);
> break;
> case SRC_HARD_RESET_VBUS_ON:
> @@ -3270,7 +3274,8 @@ 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);
> + tcpm_set_roles(port, port->self_powered, 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
> @@ -4409,6 +4414,8 @@ static int tcpm_fw_get_caps(struct tcpm_port *port,
> return -EINVAL;
> port->operating_snk_mw = mw / 1000;
>
> + port->self_powered = fwnode_property_read_bool(fwnode, "self-powered");
> +
> return 0;
> }
>
> @@ -4717,6 +4724,7 @@ static int tcpm_copy_caps(struct tcpm_port *port,
> port->typec_caps.prefer_role = tcfg->default_role;
> port->typec_caps.type = tcfg->type;
> port->typec_caps.data = tcfg->data;
> + port->self_powered = port->tcpc->config->self_powered;
>
> return 0;
> }
> diff --git a/include/linux/usb/tcpm.h b/include/linux/usb/tcpm.h
> index 7e7fbfb84e8e3..50c74a77db55c 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 */
>
> const struct typec_altmode_desc *alt_modes;
> };
Thanks,
--
heikki
Re: [PATCH 0/2] USB: serial: gpio line-name fix and FT232R CBUS gpio support
On Sun, Sep 30, 2018 at 2:29 PM Johan Hovold wrote:
> Turns out gpiolib still doesn't like having non-unique line names, so
> drop the line names from the recently added FTX cbus gpio
> implementation before adding support also for FT232R.
Oh.
> Linus, we finally got around to adding gpio support for FTDI devices;
> see commit
>
> ba93cc7da896 ("USB: serial: ftdi_sio: implement GPIO support for FT-X
> devices")
>
> in my usb-next branch (and linux-next).
This is good news, I think it's a pretty neat way for people to get
a few inexpensive GPIOs from their serial adapters.
> The gpiolib warnings and inability to use the legacy sysfs interface
> prevents us from setting the line names however as someone is bound to
> plugin more than one of these devices at some point. I think we
> discussed this issue with the name space and hotpluggable devices a few
> years ago, but looks like this topic may need to be revisited.
Hm I guess the right long-term fix is to allow per-gpiochip unique
names rather than enforcing globally unique names.
The idea is to make it possible for userspace to look up a GPIO
on a chip by name, so if the gpiochip has a unique name,
and the line name is unique on that chip it should be good
enough.
Yours,
Linus Walleij
Re: [PATCH -next] USB: cypress_m8: remove set but not used variables 'actual_size, iflag'
On Sun, Sep 30, 2018 at 06:02:24PM +0200, Johan Hovold wrote:
> On Sat, Sep 29, 2018 at 09:54:03AM +, YueHaibing wrote:
> > Fixes gcc '-Wunused-but-set-variable' warning:
> >
> > drivers/usb/serial/cypress_m8.c: In function 'cypress_send':
> > drivers/usb/serial/cypress_m8.c:689:33: warning:
> > variable 'actual_size' set but not used [-Wunused-but-set-variable]
> >
> > drivers/usb/serial/cypress_m8.c: In function 'cypress_set_termios':
> > drivers/usb/serial/cypress_m8.c:866:18: warning:
> > variable 'iflag' set but not used [-Wunused-but-set-variable]
> >
> > Signed-off-by: YueHaibing
> > ---
> > drivers/usb/serial/cypress_m8.c | 11 ++-
> > 1 file changed, 2 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/usb/serial/cypress_m8.c
> > b/drivers/usb/serial/cypress_m8.c
> > index 31c6091..98dff12 100644
> > --- a/drivers/usb/serial/cypress_m8.c
> > +++ b/drivers/usb/serial/cypress_m8.c
> > @@ -686,7 +686,7 @@ static int cypress_write(struct tty_struct *tty, struct
> > usb_serial_port *port,
> >
> > static void cypress_send(struct usb_serial_port *port)
> > {
> > - int count = 0, result, offset, actual_size;
> > + int count = 0, result, offset;
> > struct cypress_private *priv = usb_get_serial_port_data(port);
> > struct device *dev = &port->dev;
> > unsigned long flags;
> > @@ -758,12 +758,6 @@ static void cypress_send(struct usb_serial_port *port)
> > priv->write_urb_in_use = 1;
> > spin_unlock_irqrestore(&priv->lock, flags);
> >
> > - if (priv->cmd_ctrl)
> > - actual_size = 1;
> > - else
> > - actual_size = count +
> > - (priv->pkt_fmt == packet_format_1 ? 2 : 1);
> > -
>
> This looks like we have a bug in the driver, and sure enough we do.
> Commit 9aa8dae7b1fa ("cypress_m8: use usb_fill_int_urb where
> appropriate") incorrectly started setting the transfer length to the
> size of the buffer.
>
This is like the third time recently where we've had found a real bug.
YueHaibing, could you mention in the commit message where the variable
became unused? It would help reviewing. It's not a Fixes tag but it
would look like this:
"After commit 9aa8dae7b1fa ("cypress_m8: use usb_fill_int_urb where
appropriate") then we don't use actual_size any more so that can
be removed."
regards,
dan carpenter
Re: USB ports on Thunderbolt 3 Dock always doesn't work after resume from suspend
+Mika, Mathias On Sat, Sep 29, 2018 at 08:51:43AM +0200, Ondrej Holy wrote: > Hi, > > I recently got new Lenovo Thinkpad T480s with the ThinkPad Thunderbolt > 3 Dock. The USB ports (but probably also audio and ethernet) on the > dock always don't work after resume from suspend on up-to-date Fedora > 29 with kernel-4.18.9-300.fc29.x86_64. HDMI port in the dock seems > works (but with some delay). It doesn't work even with latest > available kernel-4.19.0-0.rc5.git0.1.fc30.x86_64 from rawhide. > Replugging the dock usually helps to fix that issue. > > Some probably relevant lines from dmesg after resume: > [ 6528.075126] xhci_hcd :0b:00.0: Refused to change power state, > currently in D3 > [ 6528.075127] xhci_hcd :09:00.0: Refused to change power state, > currently in D3 > [ 6528.075139] xhci_hcd :0b:00.0: WARN: xHC restore state timeout > [ 6528.075140] xhci_hcd :09:00.0: WARN: xHC restore state timeout > [ 6528.075140] xhci_hcd :0b:00.0: PCI post-resume error -110! > [ 6528.075141] xhci_hcd :09:00.0: PCI post-resume error -110! > [ 6528.075141] xhci_hcd :0b:00.0: HC died; cleaning up > [ 6528.075142] xhci_hcd :09:00.0: HC died; cleaning up > [ 6528.075150] dpm_run_callback(): pci_pm_resume+0x0/0xa0 returns -110 > [ 6528.075153] dpm_run_callback(): pci_pm_resume+0x0/0xa0 returns -110 > [ 6528.075155] PM: Device :0b:00.0 failed to resume async: error -110 > [ 6528.075157] PM: Device :09:00.0 failed to resume async: error -110 > > The T480s has the latest available BIOS version, 1.25. Not sure what > firmware version is in the dock, because I don't know how to check > that on Linux. > > Full dmesg output you can find on the following bug report: > https://bugzilla.kernel.org/show_bug.cgi?id=201255 > > Is there anything else what I can provide? > > Regards > > Ondrej > -- > Ondrej Holy > Software Engineer, Core Desktop Development > Red Hat Czech s.r.o -- heikki
Re: [PATCH] usb: dwc2: Fix HiKey regression caused by power_down feature
Hi John, On 9/28/2018 22:33, John Stultz wrote: > On Thu, Sep 27, 2018 at 5:33 AM, Artur Petrosyan > wrote: >> We would like to buy the HiKey board to perform testes. >> We found this HiKey LeMaker to have USB 2.0 ports >> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ebay.com_itm_HiKey-2DLeMaker-2Dversion-2D2GB-2DKirin-2D620-2DSoC-2D8-2Dcore-2DARM-2DCortex-2DA53-2DCPU-2DARM-2DMali-2D450_263958047308-3Fhash-3Ditem3d75202a4c-3Ag-3AaGsAAOSwOkxbqqot&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=9hPBFKCJ_nBjJhGVrrlYOeOQjP_HlVzYqrC_D7niMJI&m=HfWIbiU-a2SLoi9FO8n-LdbihLldWvXHklN-XMe1_R4&s=pir0Pt3SPLgSuceSufYhNMmgjgUwQqB3-oXWz4oLbA0&e= >> on ebay. >> >> Could you please confirm that it is the right board to test the issues >> you mention. > > Yep. That's the one. > > thanks > -john > Thank you John. So we will buy and receive the HiKey LeMaker as soon as possible to perform tests and fix the issues you got on the platform. We are sorry for this issues you face. We will do our best to have those issues fixed and provide you the best performance. Regards, Artur
