Re: Fw: keyboard-problem on bpi-r2 since 4.17
On Tue, Sep 04, 2018 at 06:39:20AM +0200, Frank Wunderlich wrote: > > > > Hi, > > i have problems with usb-keyboard on bananapi-r2 since 4.17. same keyboard > works till 4.16. > In 4.19-rc1 same issue occours. Keyboard is recognized and on keypress it is > disconnected > and connected again without anything written to console. > > dmesg (printk-debuglevel=8) looks like this: > > [ 77.292068] usb 1-1: USB disconnect, device number 2 > [ 77.292068] usb 1-1: USB disconnect, device number 2 > [ 77.472554] evbug: Disconnected device: input0 > [ 77.472554] evbug: Disconnected device: input0 > [ 77.632390] evbug: Disconnected device: input1 > [ 77.632390] evbug: Disconnected device: input1 > [ 77.773255] evbug: Disconnected device: input2 > [ 77.773255] evbug: Disconnected device: input2 > [ 78.342590] usb 1-1: new low-speed USB device number 3 using xhci-mtk > [ 78.342590] usb 1-1: new low-speed USB device number 3 using xhci-mtk > [ 78.545576] input: USB USB Keykoard as > /devices/platform/1a1c.usb/usb1/1-1/1-1:1.0/0003:1C4F:0002.0003/input/input4 > [ 78.545576] input: USB USB Keykoard as > /devices/platform/1a1c.usb/usb1/1-1/1-1:1.0/0003:1C4F:0002.0003/input/input4 > [ 78.627589] evbug: Connected device: input4 (USB USB Keykoard at > usb-1a1c.usb-1/input0) > [ 78.636266] hid-generic 0003:1C4F:0002.0003: input,hidraw0: USB HID > v1.10 Keyboard [USB USB Keykoard] on usb-1a1c.usb-1/input0 > [ 78.627589] evbug: Connected device: input4 (USB USB Keykoard at > usb-1a1c.usb-1/input0) > [ 78.655044] input: USB USB Keykoard Consumer Control as > /devices/platform/1a1c.usb/usb1/1-1/1-1:1.1/0003:1C4F:0002.0004/input/input5 > [ 78.636266] hid-generic 0003:1C4F:0002.0003: input,hidraw0: USB HID > v1.10 Keyboard [USB USB Keykoard] on usb-1a1c.usb-1/input0 > [ 78.655044] input: USB USB Keykoard Consumer Control as > /devices/platform/1a1c.usb/usb1/1-1/1-1:1.1/0003:1C4F:0002.0004/input/input5 > [ 78.734340] evbug: Connected device: input5 (USB USB Keykoard > Consumer Control at usb-1a1c.usb-1/input1) > [ 78.746118] input: USB USB Keykoard System Control as > /devices/platform/1a1c.usb/usb1/1-1/1-1:1.1/0003:1C4F:0002.0004/input/input6 > [ 78.734340] e[ 78.760069] evbug: Connected device: input6 (USB USB > Keykoard System Control at usb-1a1c.usb-1/input1) > vbug: Connected [ 78.770893] hid-generic 0003:1C4F:0002.0004: > input,hidraw1: USB HID v1.10 Device [USB USB Keykoard] on > usb-1a1c.usb-1/inpu > t1 > device: input5 (USB USB Keykoard Consumer Control at > usb-1a1c.usb-1/input1) > [ 78.746118] input: USB USB Keykoard System Control as > /devices/platform/1a1c.usb/usb1/1-1/1-1:1.1/0003:1C4F:0002.0004/input/input6 > [ 78.760069] evbug: Connected device: input6 (USB USB Keykoard System > Control at usb-1a1c.usb-1/input1) > [ 78.770893] hid-generic 0003:1C4F:0002.0004: input,hidraw1: USB HID > v1.10 Device [USB USB Keykoard] on usb-1a1c.usb-1/input1 > > can i debug this further? > > i see that mtk_xhci-driver was rewritten to use new api > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/host/xhci-mtk.c?h=v4.17-rc1=6ae9f5062aa6f5a301c16715c601c05bc9aa450e > > maybe this is the reason, but i don't know how to debug it. i tried > disabling acpi and apic via cmdline to exclude anything acpi-related. > > maybe you have an idea... Can you use 'git bisect' to track down the offending commit? thanks, greg k-h
Re: [PATCH 00/25] Change tty_port(standard)_install's return type
Hi Jaejoong. > Change return type for tty functions. Patch No.01 > tty: Change return type to void Adding this patch first will generate a lot of warnings until all users are updated. It is usual practice to prepare all users and then apply the infrastructure changes as the last patch. Then people will not see a lot of warnings when they build something in the middle, and I guess current stack set may also generate a few mails from the 0-day build infrastructure. > isdn: i4l: isdn_tty: Change return type to void And a nitpick on the patch description. This patch do not change any return type, but it ignore the return value og tty_part_install(). Same goes for all ramaining patches. Sam
Fw: keyboard-problem on bpi-r2 since 4.17
Hi, i have problems with usb-keyboard on bananapi-r2 since 4.17. same keyboard works till 4.16. In 4.19-rc1 same issue occours. Keyboard is recognized and on keypress it is disconnected and connected again without anything written to console. dmesg (printk-debuglevel=8) looks like this: [ 77.292068] usb 1-1: USB disconnect, device number 2 [ 77.292068] usb 1-1: USB disconnect, device number 2 [ 77.472554] evbug: Disconnected device: input0 [ 77.472554] evbug: Disconnected device: input0 [ 77.632390] evbug: Disconnected device: input1 [ 77.632390] evbug: Disconnected device: input1 [ 77.773255] evbug: Disconnected device: input2 [ 77.773255] evbug: Disconnected device: input2 [ 78.342590] usb 1-1: new low-speed USB device number 3 using xhci-mtk [ 78.342590] usb 1-1: new low-speed USB device number 3 using xhci-mtk [ 78.545576] input: USB USB Keykoard as /devices/platform/1a1c.usb/usb1/1-1/1-1:1.0/0003:1C4F:0002.0003/input/input4 [ 78.545576] input: USB USB Keykoard as /devices/platform/1a1c.usb/usb1/1-1/1-1:1.0/0003:1C4F:0002.0003/input/input4 [ 78.627589] evbug: Connected device: input4 (USB USB Keykoard at usb-1a1c.usb-1/input0) [ 78.636266] hid-generic 0003:1C4F:0002.0003: input,hidraw0: USB HID v1.10 Keyboard [USB USB Keykoard] on usb-1a1c.usb-1/input0 [ 78.627589] evbug: Connected device: input4 (USB USB Keykoard at usb-1a1c.usb-1/input0) [ 78.655044] input: USB USB Keykoard Consumer Control as /devices/platform/1a1c.usb/usb1/1-1/1-1:1.1/0003:1C4F:0002.0004/input/input5 [ 78.636266] hid-generic 0003:1C4F:0002.0003: input,hidraw0: USB HID v1.10 Keyboard [USB USB Keykoard] on usb-1a1c.usb-1/input0 [ 78.655044] input: USB USB Keykoard Consumer Control as /devices/platform/1a1c.usb/usb1/1-1/1-1:1.1/0003:1C4F:0002.0004/input/input5 [ 78.734340] evbug: Connected device: input5 (USB USB Keykoard Consumer Control at usb-1a1c.usb-1/input1) [ 78.746118] input: USB USB Keykoard System Control as /devices/platform/1a1c.usb/usb1/1-1/1-1:1.1/0003:1C4F:0002.0004/input/input6 [ 78.734340] e[ 78.760069] evbug: Connected device: input6 (USB USB Keykoard System Control at usb-1a1c.usb-1/input1) vbug: Connected [ 78.770893] hid-generic 0003:1C4F:0002.0004: input,hidraw1: USB HID v1.10 Device [USB USB Keykoard] on usb-1a1c.usb-1/inpu t1 device: input5 (USB USB Keykoard Consumer Control at usb-1a1c.usb-1/input1) [ 78.746118] input: USB USB Keykoard System Control as /devices/platform/1a1c.usb/usb1/1-1/1-1:1.1/0003:1C4F:0002.0004/input/input6 [ 78.760069] evbug: Connected device: input6 (USB USB Keykoard System Control at usb-1a1c.usb-1/input1) [ 78.770893] hid-generic 0003:1C4F:0002.0004: input,hidraw1: USB HID v1.10 Device [USB USB Keykoard] on usb-1a1c.usb-1/input1 can i debug this further? i see that mtk_xhci-driver was rewritten to use new api https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/host/xhci-mtk.c?h=v4.17-rc1=6ae9f5062aa6f5a301c16715c601c05bc9aa450e maybe this is the reason, but i don't know how to debug it. i tried disabling acpi and apic via cmdline to exclude anything acpi-related. maybe you have an idea... regards Frank
Re: [PATCH v2 06/10] platform: x86: intel_cht_int33fe: Fix the identifier for the mux connection
Hi, On 03-09-18 15:36, Heikki Krogerus wrote: PI3USB30532 is used for muxing the port to DisplayPort on CHT platforms, so changing the connection ID so that the mux will get assigned to the alternate mode device and not the port device. Connection ID "typec-mux" is now reserved for Accessory Modes. Signed-off-by: Heikki Krogerus --- drivers/platform/x86/intel_cht_int33fe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c index b0cef48f77af..f8c881f871f9 100644 --- a/drivers/platform/x86/intel_cht_int33fe.c +++ b/drivers/platform/x86/intel_cht_int33fe.c @@ -179,7 +179,7 @@ static int cht_int33fe_probe(struct i2c_client *client) data->connections[0].id = "typec-switch"; data->connections[1].endpoint[0] = "i2c-fusb302"; data->connections[1].endpoint[1] = "i2c-pi3usb30532"; - data->connections[1].id = "typec-mux"; + data->connections[1].id = "idff01m01"; Hmm, so the mux will start in open connection and needs to be moved from open to TYPEC_STATE_USB when doing USB3, I assume the alternative driver is only going to come into play when actually using a DP over Type-C capable device/dongle, so what will do the TYPEC_STATE_SAFE -> TYPEC_STATE_USB switching now ? Regards, Hans
Re: Nothing in /sys/class/udc
On Mon, Sep 3, 2018 at 4:50 PM Ranran wrote:
>
> On Mon, Sep 3, 2018 at 4:22 PM Ranran wrote:
> >
> > On Mon, Sep 3, 2018 at 3:40 PM Greg KH wrote:
> > >
> > > On Mon, Sep 03, 2018 at 09:39:14AM +0300, Ranran wrote:
> > > > Hello,
> > > >
> > > > I try to add gadget configfs as described in:
> > > > https://www.kernel.org/doc/Documentation/usb/gadget_configfs.txt
> > > > Yet, I find nothing in /sys/class/udc:
> > > >
> > > > user@user-VirtualBox:~/tegra$ ls /sys/class/udc/ -al
> > > > total 0
> > > > drwxr-xr-x 2 root root 0 Sep 3 00:30 .
> > > > drwxr-xr-x 58 root root 0 Sep 3 00:30 ..
> > > >
> > > > I also don't have dwc2, but dwc3:
> > > > user@user-VirtualBox:~/tegra$ lsmod | grep dw
> > > > dwc3 90112 0
> > > > ulpi 16384 1 dwc3
> > > > udc_core 24576 2 dwc3,libcomposite
> > > > user@user-VirtualBox:~/tegra$
> > > >
> > > > Kernel is 4.4.50.
> > >
> > > Why do you think the dwc3 driver will work here? Do you really have
> > > this hardware being emulated?
> > >
> > Hi,
> >
> > I check it with Intel's E3900.
> > In datasheet, I can't find it being described in details in datasheet,
> > yet being linked with another document ("see Section Little-Endian and
> > Big-Endian in the DWC SuperSpeed USB 3.0 Controller User Guide" )
> > https://www.mouser.com/ds/2/612/atom-e3800-family-datasheet-915661.pdf
> > I have installed module dwc3 and also dwc2 (both compiled as modules),
> > but nothing appear in /sys/class/udc
> > I also tried to add all options in device-drivers->usb->usb
> > gadget->usb peripheral control into kernel, (not as modules, but built
> > into kernel).
> > Yet, on checking
> > ls /sys/class/udc
> > I only find dummy_udc.0 , and nothing else.
> >
>
> I suspect that there is no device controller supported with this com
> express (congatec type 10), probably this is the only explanations for
> the empty /sys/class/udc.
>
A fix for my last comment:
usb device controller should be supported with conga-MA5 (contains
Intel's appolo lake), so I am still not sure why I find /sys/class/udc
empty.
In congatec MA-5 document it is said:
"The conga-MA5 offers six USB 2.0 interfaces on the COM Express
connector including one USB 2.0 Dual Role port. The xHCI host
controller in the SoC complies with USB standard 1.1 and 2.0 with
high-speed, full-speed and low-speed USB signalling. Note USB Dual
Role is only supported under Linux. The port is a standard USB Host
port under Windows."
Maybe some configuration in my .config is missing ? (I enabled all
controllers in device-drivers->usb->usb-> gadget->usb peripheral
control , so I am not sure what is still missing)
Thank you for any idea,
ranran
> Best Regards,
> ranran
>
> > Thank you for any idea,
> > ranran
> >
> > > And 4.4.50 is very old, please use a modern kernel please.
> > >
> > > thanks,
> > >
> > > greg k-h
Re: [PATCH v2 00/10] usb: typec: A few more improvements for Intel CHT
On Mon, Sep 3, 2018 at 4:37 PM Heikki Krogerus wrote: > > Hi, > > I've now removed the conditional creation of the mux device, and the > connection for it that was checked in intel_cht_int33fe.c. I'm instead > making the intel_cht_int33fe driver depend on the mux drivers. I also > added a trivial cleanup patch (patch 10/10) for the fusb302.c to this > series, and also a few fixes (1/10 and 2/10) to the mux handling. > > The origin thread can be read here: > https://www.spinics.net/lists/linux-usb/msg172033.html > > Acked-by: Andy Shevchenko for PDx86 bits on condition that Hans is fine with them. > Heikki Krogerus (10): > usb: typec: Take care of driver module reference counting > usb: roles: Handle driver reference counting > platform: x86: intel_cht_int33fe: Add dependency on muxes > drivers: base: Helpers for adding device connection descriptions > platform: x86: intel_cht_int33fe: Register all connections at once > platform: x86: intel_cht_int33fe: Fix the identifier for the mux > connection > platform: x86: intel_cht_int33fe: Add connections for the USB Type-C > port > usb: typec: class: Don't use port parent for getting mux handles > platform: x86: intel_cht_int33fe: Remove the old connections for the > muxes > usb: typec: fusb302: reorganizing the probe function a little > > drivers/platform/x86/Kconfig | 2 ++ > drivers/platform/x86/intel_cht_int33fe.c | 20 + > drivers/usb/common/roles.c | 15 -- > drivers/usb/typec/class.c| 38 ++-- > drivers/usb/typec/fusb302/fusb302.c | 25 ++-- > drivers/usb/typec/mux.c | 17 --- > include/linux/device.h | 24 +++ > 7 files changed, 82 insertions(+), 59 deletions(-) > > -- > 2.18.0 > -- With Best Regards, Andy Shevchenko
Re: Nothing in /sys/class/udc
On Mon, Sep 3, 2018 at 4:22 PM Ranran wrote:
>
> On Mon, Sep 3, 2018 at 3:40 PM Greg KH wrote:
> >
> > On Mon, Sep 03, 2018 at 09:39:14AM +0300, Ranran wrote:
> > > Hello,
> > >
> > > I try to add gadget configfs as described in:
> > > https://www.kernel.org/doc/Documentation/usb/gadget_configfs.txt
> > > Yet, I find nothing in /sys/class/udc:
> > >
> > > user@user-VirtualBox:~/tegra$ ls /sys/class/udc/ -al
> > > total 0
> > > drwxr-xr-x 2 root root 0 Sep 3 00:30 .
> > > drwxr-xr-x 58 root root 0 Sep 3 00:30 ..
> > >
> > > I also don't have dwc2, but dwc3:
> > > user@user-VirtualBox:~/tegra$ lsmod | grep dw
> > > dwc3 90112 0
> > > ulpi 16384 1 dwc3
> > > udc_core 24576 2 dwc3,libcomposite
> > > user@user-VirtualBox:~/tegra$
> > >
> > > Kernel is 4.4.50.
> >
> > Why do you think the dwc3 driver will work here? Do you really have
> > this hardware being emulated?
> >
> Hi,
>
> I check it with Intel's E3900.
> In datasheet, I can't find it being described in details in datasheet,
> yet being linked with another document ("see Section Little-Endian and
> Big-Endian in the DWC SuperSpeed USB 3.0 Controller User Guide" )
> https://www.mouser.com/ds/2/612/atom-e3800-family-datasheet-915661.pdf
> I have installed module dwc3 and also dwc2 (both compiled as modules),
> but nothing appear in /sys/class/udc
> I also tried to add all options in device-drivers->usb->usb
> gadget->usb peripheral control into kernel, (not as modules, but built
> into kernel).
> Yet, on checking
> ls /sys/class/udc
> I only find dummy_udc.0 , and nothing else.
>
I suspect that there is no device controller supported with this com
express (congatec type 10), probably this is the only explanations for
the empty /sys/class/udc.
Best Regards,
ranran
> Thank you for any idea,
> ranran
>
> > And 4.4.50 is very old, please use a modern kernel please.
> >
> > thanks,
> >
> > greg k-h
[PATCH v2 09/10] platform: x86: intel_cht_int33fe: Remove the old connections for the muxes
USB Type-C class driver now expects the muxes to be always assigned to the ports and not controllers, so the connections for the mux and fusb302 can be removed. Signed-off-by: Heikki Krogerus --- drivers/platform/x86/intel_cht_int33fe.c | 11 ++- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c index eb0dd687ee54..05826e3e9037 100644 --- a/drivers/platform/x86/intel_cht_int33fe.c +++ b/drivers/platform/x86/intel_cht_int33fe.c @@ -174,23 +174,16 @@ static int cht_int33fe_probe(struct i2c_client *client) return -EPROBE_DEFER; /* Wait for i2c-adapter to load */ } - data->connections[0].endpoint[0] = "i2c-fusb302"; + data->connections[0].endpoint[0] = "port0"; data->connections[0].endpoint[1] = "i2c-pi3usb30532"; data->connections[0].id = "typec-switch"; - data->connections[1].endpoint[0] = "i2c-fusb302"; + data->connections[1].endpoint[0] = "port0"; data->connections[1].endpoint[1] = "i2c-pi3usb30532"; data->connections[1].id = "idff01m01"; data->connections[2].endpoint[0] = "i2c-fusb302"; data->connections[2].endpoint[1] = "intel_xhci_usb_sw-role-switch"; data->connections[2].id = "usb-role-switch"; - data->connections[3].endpoint[0] = "port0"; - data->connections[3].endpoint[1] = "i2c-pi3usb30532"; - data->connections[3].id = "typec-switch"; - data->connections[4].endpoint[0] = "port0"; - data->connections[4].endpoint[1] = "i2c-pi3usb30532"; - data->connections[4].id = "idff01m01"; - device_connections_add(data->connections); memset(_info, 0, sizeof(board_info)); -- 2.18.0
[PATCH v2 05/10] platform: x86: intel_cht_int33fe: Register all connections at once
We can register all device connection descriptors with a
single call to device_connections_add().
Signed-off-by: Heikki Krogerus
---
drivers/platform/x86/intel_cht_int33fe.c | 14 --
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/platform/x86/intel_cht_int33fe.c
b/drivers/platform/x86/intel_cht_int33fe.c
index 39d4100c60a2..b0cef48f77af 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -34,7 +34,7 @@ struct cht_int33fe_data {
struct i2c_client *fusb302;
struct i2c_client *pi3usb30532;
/* Contain a list-head must be per device */
- struct device_connection connections[3];
+ struct device_connection connections[4];
};
/*
@@ -184,9 +184,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
data->connections[2].endpoint[1] = "intel_xhci_usb_sw-role-switch";
data->connections[2].id = "usb-role-switch";
- device_connection_add(>connections[0]);
- device_connection_add(>connections[1]);
- device_connection_add(>connections[2]);
+ device_connections_add(data->connections);
memset(_info, 0, sizeof(board_info));
strlcpy(board_info.type, "typec_fusb302", I2C_NAME_SIZE);
@@ -217,9 +215,7 @@ static int cht_int33fe_probe(struct i2c_client *client)
if (data->max17047)
i2c_unregister_device(data->max17047);
- device_connection_remove(>connections[2]);
- device_connection_remove(>connections[1]);
- device_connection_remove(>connections[0]);
+ device_connections_remove(data->connections);
return -EPROBE_DEFER; /* Wait for the i2c-adapter to load */
}
@@ -233,9 +229,7 @@ static int cht_int33fe_remove(struct i2c_client *i2c)
if (data->max17047)
i2c_unregister_device(data->max17047);
- device_connection_remove(>connections[2]);
- device_connection_remove(>connections[1]);
- device_connection_remove(>connections[0]);
+ device_connections_remove(data->connections);
return 0;
}
--
2.18.0
[PATCH v2 04/10] drivers: base: Helpers for adding device connection descriptions
Introducing helpers for adding and removing multiple device
connection descriptions at once.
Signed-off-by: Heikki Krogerus
---
include/linux/device.h | 24
1 file changed, 24 insertions(+)
diff --git a/include/linux/device.h b/include/linux/device.h
index 8f882549edee..3f1066a9e1c3 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -773,6 +773,30 @@ struct device *device_connection_find(struct device *dev,
const char *con_id);
void device_connection_add(struct device_connection *con);
void device_connection_remove(struct device_connection *con);
+/**
+ * device_connections_add - Add multiple device connections at once
+ * @cons: Zero terminated array of device connection descriptors
+ */
+static inline void device_connections_add(struct device_connection *cons)
+{
+ struct device_connection *c;
+
+ for (c = cons; c->endpoint[0]; c++)
+ device_connection_add(c);
+}
+
+/**
+ * device_connections_remove - Remove multiple device connections at once
+ * @cons: Zero terminated array of device connection descriptors
+ */
+static inline void device_connections_remove(struct device_connection *cons)
+{
+ struct device_connection *c;
+
+ for (c = cons; c->endpoint[0]; c++)
+ device_connection_remove(c);
+}
+
/**
* enum device_link_state - Device link states.
* @DL_STATE_NONE: The presence of the drivers is not being tracked.
--
2.18.0
[PATCH v2 02/10] usb: roles: Handle driver reference counting
This fixes potential "BUG: unable to handle kernel paging
request at ..." from happening.
Fixes: fde0aa6c175a ("usb: common: Small class for USB role switches")
Cc:
Signed-off-by: Heikki Krogerus
---
drivers/usb/common/roles.c | 15 ---
1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/common/roles.c b/drivers/usb/common/roles.c
index 15cc76e22123..3d8a776e55ee 100644
--- a/drivers/usb/common/roles.c
+++ b/drivers/usb/common/roles.c
@@ -109,8 +109,15 @@ static void *usb_role_switch_match(struct
device_connection *con, int ep,
*/
struct usb_role_switch *usb_role_switch_get(struct device *dev)
{
- return device_connection_find_match(dev, "usb-role-switch", NULL,
- usb_role_switch_match);
+ struct usb_role_switch *sw;
+
+ sw = device_connection_find_match(dev, "usb-role-switch", NULL,
+ usb_role_switch_match);
+
+ if (!IS_ERR(sw))
+ WARN_ON(!try_module_get(sw->dev.parent->driver->owner));
+
+ return sw;
}
EXPORT_SYMBOL_GPL(usb_role_switch_get);
@@ -122,8 +129,10 @@ EXPORT_SYMBOL_GPL(usb_role_switch_get);
*/
void usb_role_switch_put(struct usb_role_switch *sw)
{
- if (!IS_ERR_OR_NULL(sw))
+ if (!IS_ERR_OR_NULL(sw)) {
put_device(>dev);
+ module_put(sw->dev.parent->driver->owner);
+ }
}
EXPORT_SYMBOL_GPL(usb_role_switch_put);
--
2.18.0
[PATCH v2 03/10] platform: x86: intel_cht_int33fe: Add dependency on muxes
The connections create clear dependency on the muxes. fusb302 fails to probe unless we have the mux drivers available. Signed-off-by: Heikki Krogerus --- drivers/platform/x86/Kconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 0c1aa6c314f5..bdac939de223 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -867,6 +867,8 @@ config INTEL_CHT_INT33FE tristate "Intel Cherry Trail ACPI INT33FE Driver" depends on X86 && ACPI && I2C && REGULATOR depends on CHARGER_BQ24190=y || (CHARGER_BQ24190=m && m) + depends on USB_ROLES_INTEL_XHCI=y || (USB_ROLES_INTEL_XHCI=m && m) + depends on TYPEC_MUX_PI3USB30532=y || (TYPEC_MUX_PI3USB30532=m && m) ---help--- This driver add support for the INT33FE ACPI device found on some Intel Cherry Trail devices. -- 2.18.0
[PATCH v2 00/10] usb: typec: A few more improvements for Intel CHT
Hi, I've now removed the conditional creation of the mux device, and the connection for it that was checked in intel_cht_int33fe.c. I'm instead making the intel_cht_int33fe driver depend on the mux drivers. I also added a trivial cleanup patch (patch 10/10) for the fusb302.c to this series, and also a few fixes (1/10 and 2/10) to the mux handling. The origin thread can be read here: https://www.spinics.net/lists/linux-usb/msg172033.html Heikki Krogerus (10): usb: typec: Take care of driver module reference counting usb: roles: Handle driver reference counting platform: x86: intel_cht_int33fe: Add dependency on muxes drivers: base: Helpers for adding device connection descriptions platform: x86: intel_cht_int33fe: Register all connections at once platform: x86: intel_cht_int33fe: Fix the identifier for the mux connection platform: x86: intel_cht_int33fe: Add connections for the USB Type-C port usb: typec: class: Don't use port parent for getting mux handles platform: x86: intel_cht_int33fe: Remove the old connections for the muxes usb: typec: fusb302: reorganizing the probe function a little drivers/platform/x86/Kconfig | 2 ++ drivers/platform/x86/intel_cht_int33fe.c | 20 + drivers/usb/common/roles.c | 15 -- drivers/usb/typec/class.c| 38 ++-- drivers/usb/typec/fusb302/fusb302.c | 25 ++-- drivers/usb/typec/mux.c | 17 --- include/linux/device.h | 24 +++ 7 files changed, 82 insertions(+), 59 deletions(-) -- 2.18.0
[PATCH v2 07/10] platform: x86: intel_cht_int33fe: Add connections for the USB Type-C port
Assigning the mux to the USB Type-C port on top of fusb302.
That will prepare this driver for the change in the USB
Type-C class code, where the class driver will assume the
muxes to be always assigned to the ports and not the
controllers.
Once the USB Type-C class driver has been updated, the
connections between the mux and fusb302 can be dropped.
Signed-off-by: Heikki Krogerus
---
drivers/platform/x86/intel_cht_int33fe.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/platform/x86/intel_cht_int33fe.c
b/drivers/platform/x86/intel_cht_int33fe.c
index f8c881f871f9..eb0dd687ee54 100644
--- a/drivers/platform/x86/intel_cht_int33fe.c
+++ b/drivers/platform/x86/intel_cht_int33fe.c
@@ -34,7 +34,7 @@ struct cht_int33fe_data {
struct i2c_client *fusb302;
struct i2c_client *pi3usb30532;
/* Contain a list-head must be per device */
- struct device_connection connections[4];
+ struct device_connection connections[6];
};
/*
@@ -184,6 +184,13 @@ static int cht_int33fe_probe(struct i2c_client *client)
data->connections[2].endpoint[1] = "intel_xhci_usb_sw-role-switch";
data->connections[2].id = "usb-role-switch";
+ data->connections[3].endpoint[0] = "port0";
+ data->connections[3].endpoint[1] = "i2c-pi3usb30532";
+ data->connections[3].id = "typec-switch";
+ data->connections[4].endpoint[0] = "port0";
+ data->connections[4].endpoint[1] = "i2c-pi3usb30532";
+ data->connections[4].id = "idff01m01";
+
device_connections_add(data->connections);
memset(_info, 0, sizeof(board_info));
--
2.18.0
[PATCH v2 08/10] usb: typec: class: Don't use port parent for getting mux handles
It is not possible to use the parent of the port device when
requesting mux handles as the parent may be a multiport USB
Type-C or PD controller. The muxes must be assigned to the
ports, not the controllers.
This will also move the requesting of the muxes after the
port device is initialized.
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/class.c | 38 +++---
1 file changed, 15 insertions(+), 23 deletions(-)
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index c202975f8097..5b969339d1b3 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -1501,7 +1501,7 @@ typec_port_register_altmode(struct typec_port *port,
sprintf(id, "id%04xm%02x", desc->svid, desc->mode);
- mux = typec_mux_get(port->dev.parent, id);
+ mux = typec_mux_get(>dev, id);
if (IS_ERR(mux))
return ERR_CAST(mux);
@@ -1541,18 +1541,6 @@ struct typec_port *typec_register_port(struct device
*parent,
return ERR_PTR(id);
}
- port->sw = typec_switch_get(cap->fwnode ? >dev : parent);
- if (IS_ERR(port->sw)) {
- ret = PTR_ERR(port->sw);
- goto err_switch;
- }
-
- port->mux = typec_mux_get(parent, "typec-mux");
- if (IS_ERR(port->mux)) {
- ret = PTR_ERR(port->mux);
- goto err_mux;
- }
-
switch (cap->type) {
case TYPEC_PORT_SRC:
port->pwr_role = TYPEC_SOURCE;
@@ -1593,13 +1581,26 @@ struct typec_port *typec_register_port(struct device
*parent,
port->port_type = cap->type;
port->prefer_role = cap->prefer_role;
+ device_initialize(>dev);
port->dev.class = typec_class;
port->dev.parent = parent;
port->dev.fwnode = cap->fwnode;
port->dev.type = _port_dev_type;
dev_set_name(>dev, "port%d", id);
- ret = device_register(>dev);
+ port->sw = typec_switch_get(>dev);
+ if (IS_ERR(port->sw)) {
+ put_device(>dev);
+ return ERR_CAST(port->sw);
+ }
+
+ port->mux = typec_mux_get(>dev, "typec-mux");
+ if (IS_ERR(port->mux)) {
+ put_device(>dev);
+ return ERR_CAST(port->mux);
+ }
+
+ ret = device_add(>dev);
if (ret) {
dev_err(parent, "failed to register port (%d)\n", ret);
put_device(>dev);
@@ -1607,15 +1608,6 @@ struct typec_port *typec_register_port(struct device
*parent,
}
return port;
-
-err_mux:
- typec_switch_put(port->sw);
-
-err_switch:
- ida_simple_remove(_index_ida, port->id);
- kfree(port);
-
- return ERR_PTR(ret);
}
EXPORT_SYMBOL_GPL(typec_register_port);
--
2.18.0
[PATCH v2 01/10] usb: typec: Take care of driver module reference counting
Functions typec_mux_get() and typec_switch_get() already
make sure that the mux device reference count is
incremented, but the same must be done to the driver module
as well to prevent the drivers from being unloaded in the
middle of operation.
This fixes a potential "BUG: unable to handle kernel paging
request at ..." from happening.
Fixes: 93dd2112c7b2 ("usb: typec: mux: Get the mux identifier from function
parameter")
Cc:
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/mux.c | 17 +
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/typec/mux.c b/drivers/usb/typec/mux.c
index ddaac63ecf12..d990aa510fab 100644
--- a/drivers/usb/typec/mux.c
+++ b/drivers/usb/typec/mux.c
@@ -9,6 +9,7 @@
#include
#include
+#include
#include
#include
@@ -49,8 +50,10 @@ struct typec_switch *typec_switch_get(struct device *dev)
mutex_lock(_lock);
sw = device_connection_find_match(dev, "typec-switch", NULL,
typec_switch_match);
- if (!IS_ERR_OR_NULL(sw))
+ if (!IS_ERR_OR_NULL(sw)) {
+ WARN_ON(!try_module_get(sw->dev->driver->owner));
get_device(sw->dev);
+ }
mutex_unlock(_lock);
return sw;
@@ -65,8 +68,10 @@ EXPORT_SYMBOL_GPL(typec_switch_get);
*/
void typec_switch_put(struct typec_switch *sw)
{
- if (!IS_ERR_OR_NULL(sw))
+ if (!IS_ERR_OR_NULL(sw)) {
+ module_put(sw->dev->driver->owner);
put_device(sw->dev);
+ }
}
EXPORT_SYMBOL_GPL(typec_switch_put);
@@ -136,8 +141,10 @@ struct typec_mux *typec_mux_get(struct device *dev, const
char *name)
mutex_lock(_lock);
mux = device_connection_find_match(dev, name, NULL, typec_mux_match);
- if (!IS_ERR_OR_NULL(mux))
+ if (!IS_ERR_OR_NULL(mux)) {
+ WARN_ON(!try_module_get(mux->dev->driver->owner));
get_device(mux->dev);
+ }
mutex_unlock(_lock);
return mux;
@@ -152,8 +159,10 @@ EXPORT_SYMBOL_GPL(typec_mux_get);
*/
void typec_mux_put(struct typec_mux *mux)
{
- if (!IS_ERR_OR_NULL(mux))
+ if (!IS_ERR_OR_NULL(mux)) {
+ module_put(mux->dev->driver->owner);
put_device(mux->dev);
+ }
}
EXPORT_SYMBOL_GPL(typec_mux_put);
--
2.18.0
[PATCH v2 06/10] platform: x86: intel_cht_int33fe: Fix the identifier for the mux connection
PI3USB30532 is used for muxing the port to DisplayPort on CHT platforms, so changing the connection ID so that the mux will get assigned to the alternate mode device and not the port device. Connection ID "typec-mux" is now reserved for Accessory Modes. Signed-off-by: Heikki Krogerus --- drivers/platform/x86/intel_cht_int33fe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/platform/x86/intel_cht_int33fe.c b/drivers/platform/x86/intel_cht_int33fe.c index b0cef48f77af..f8c881f871f9 100644 --- a/drivers/platform/x86/intel_cht_int33fe.c +++ b/drivers/platform/x86/intel_cht_int33fe.c @@ -179,7 +179,7 @@ static int cht_int33fe_probe(struct i2c_client *client) data->connections[0].id = "typec-switch"; data->connections[1].endpoint[0] = "i2c-fusb302"; data->connections[1].endpoint[1] = "i2c-pi3usb30532"; - data->connections[1].id = "typec-mux"; + data->connections[1].id = "idff01m01"; data->connections[2].endpoint[0] = "i2c-fusb302"; data->connections[2].endpoint[1] = "intel_xhci_usb_sw-role-switch"; data->connections[2].id = "usb-role-switch"; -- 2.18.0
[PATCH v2 10/10] usb: typec: fusb302: reorganizing the probe function a little
The debugfs needs to be initialized as the last step in
probe in this case. The struct dentry *rootdir can't be
pointing to anything unless driver probe really finishes
successfully.
It is also not necessary to clear the i2c clientdata if the
probe fails, so removing the extra label used for that.
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/fusb302/fusb302.c | 25 +
1 file changed, 9 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/typec/fusb302/fusb302.c
b/drivers/usb/typec/fusb302/fusb302.c
index 82bed9810be6..5f5864273e16 100644
--- a/drivers/usb/typec/fusb302/fusb302.c
+++ b/drivers/usb/typec/fusb302/fusb302.c
@@ -1730,7 +1730,6 @@ static int fusb302_probe(struct i2c_client *client,
return -ENOMEM;
chip->i2c_client = client;
- i2c_set_clientdata(client, chip);
chip->dev = >dev;
chip->tcpc_config = fusb302_tcpc_config;
chip->tcpc_dev.config = >tcpc_config;
@@ -1756,22 +1755,17 @@ static int fusb302_probe(struct i2c_client *client,
return -EPROBE_DEFER;
}
- fusb302_debugfs_init(chip);
+ chip->vbus = devm_regulator_get(chip->dev, "vbus");
+ if (IS_ERR(chip->vbus))
+ return PTR_ERR(chip->vbus);
chip->wq = create_singlethread_workqueue(dev_name(chip->dev));
- if (!chip->wq) {
- ret = -ENOMEM;
- goto clear_client_data;
- }
+ if (!chip->wq)
+ return -ENOMEM;
+
INIT_DELAYED_WORK(>bc_lvl_handler, fusb302_bc_lvl_handler_work);
init_tcpc_dev(>tcpc_dev);
- chip->vbus = devm_regulator_get(chip->dev, "vbus");
- if (IS_ERR(chip->vbus)) {
- ret = PTR_ERR(chip->vbus);
- goto destroy_workqueue;
- }
-
if (client->irq) {
chip->gpio_int_n_irq = client->irq;
} else {
@@ -1797,15 +1791,15 @@ static int fusb302_probe(struct i2c_client *client,
goto tcpm_unregister_port;
}
enable_irq_wake(chip->gpio_int_n_irq);
+ fusb302_debugfs_init(chip);
+ i2c_set_clientdata(client, chip);
+
return ret;
tcpm_unregister_port:
tcpm_unregister_port(chip->tcpm_port);
destroy_workqueue:
destroy_workqueue(chip->wq);
-clear_client_data:
- i2c_set_clientdata(client, NULL);
- fusb302_debugfs_exit(chip);
return ret;
}
@@ -1816,7 +1810,6 @@ static int fusb302_remove(struct i2c_client *client)
tcpm_unregister_port(chip->tcpm_port);
destroy_workqueue(chip->wq);
- i2c_set_clientdata(client, NULL);
fusb302_debugfs_exit(chip);
return 0;
--
2.18.0
Re: Nothing in /sys/class/udc
On Mon, Sep 3, 2018 at 3:40 PM Greg KH wrote:
>
> On Mon, Sep 03, 2018 at 09:39:14AM +0300, Ranran wrote:
> > Hello,
> >
> > I try to add gadget configfs as described in:
> > https://www.kernel.org/doc/Documentation/usb/gadget_configfs.txt
> > Yet, I find nothing in /sys/class/udc:
> >
> > user@user-VirtualBox:~/tegra$ ls /sys/class/udc/ -al
> > total 0
> > drwxr-xr-x 2 root root 0 Sep 3 00:30 .
> > drwxr-xr-x 58 root root 0 Sep 3 00:30 ..
> >
> > I also don't have dwc2, but dwc3:
> > user@user-VirtualBox:~/tegra$ lsmod | grep dw
> > dwc3 90112 0
> > ulpi 16384 1 dwc3
> > udc_core 24576 2 dwc3,libcomposite
> > user@user-VirtualBox:~/tegra$
> >
> > Kernel is 4.4.50.
>
> Why do you think the dwc3 driver will work here? Do you really have
> this hardware being emulated?
>
Hi,
I check it with Intel's E3900.
In datasheet, I can't find it being described in details in datasheet,
yet being linked with another document ("see Section Little-Endian and
Big-Endian in the DWC SuperSpeed USB 3.0 Controller User Guide" )
https://www.mouser.com/ds/2/612/atom-e3800-family-datasheet-915661.pdf
I have installed module dwc3 and also dwc2 (both compiled as modules),
but nothing appear in /sys/class/udc
I also tried to add all options in device-drivers->usb->usb
gadget->usb peripheral control into kernel, (not as modules, but built
into kernel).
Yet, on checking
ls /sys/class/udc
I only find dummy_udc.0 , and nothing else.
Thank you for any idea,
ranran
> And 4.4.50 is very old, please use a modern kernel please.
>
> thanks,
>
> greg k-h
Re: [PATCH 0/8] usb: typec: A few more improvements for Intel CHT
On Mon, Sep 03, 2018 at 03:08:46PM +0300, Andy Shevchenko wrote: > On Mon, Sep 3, 2018 at 10:19 AM Heikki Krogerus > wrote: > > > > On Mon, Sep 03, 2018 at 09:17:13AM +0300, Andy Shevchenko wrote: > > > On Fri, Aug 31, 2018 at 5:21 PM Heikki Krogerus > > > wrote: > > > > > > > > Hi, > > > > > > > > The second last patch in this series will make it possible to use > > > > multiport USB Type-C and PD controllers with the muxes. The CHT > > > > connections are simply adapted to that. The rest of the series will > > > > mainly allow us to use the USB Type-C on CHT boards even without a > > > > USB device controller. > > > > > > > > > > Throught which tree you are planning to direct this? > > > > The series is about USB Type-C, so USB would make sense to me. But > > it's up to you guys of course. > > My vacation is about to start, and from my prospective patch series > looks good (taking into account the changes Hans asked for). > > Thus, > > Acked-by: Andy Shevchenko > > for PDx86 bits on condition that Hans is fine with them. OK, thanks Andy. Because of the change, we have to make the int33fe driver depend on the muxes, so since you are about to leave, I'll resend the patches right now. Hope you have time to take a look at them. In any case, have good vacation :-) Cheers, -- heikki
[PATCH v2] usb: Avoid use-after-free by flushing endpoints early in usb_set_interface()
The steps taken by usb core to set a new interface is very different from what is done on the xHC host side. xHC hardware will do everything in one go. One command is used to set up new endpoints, free old endpoints, check bandwidth, and run the new endpoints. All this is done by xHC when usb core asks the hcd to check for available bandwidth. At this point usb core has not yet flushed the old endpoints, which will cause use-after-free issues in xhci driver as queued URBs are cancelled on a re-allocated endpoint. To resolve this add a call to usb_disable_interface() which will flush the endpoints before calling usb_hcd_alloc_bandwidth() Additional checks in xhci driver will also be implemented to gracefully handle stale URB cancel on freed and re-allocated endpoints Cc: Reported-by: Sudip Mukherjee Signed-off-by: Mathias Nyman --- v2: update kerneldoc as well --- drivers/usb/core/message.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index 228672f..bfa5eda 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1341,6 +1341,11 @@ void usb_enable_interface(struct usb_device *dev, * is submitted that needs that bandwidth. Some other operating systems * allocate bandwidth early, when a configuration is chosen. * + * xHCI reserves bandwidth and configures the alternate setting in + * usb_hcd_alloc_bandwidth(). If it fails the original interface altsetting + * may be disabled. Drivers cannot rely on any particular alternate + * setting being in effect after a failure. + * * This call is synchronous, and may not be used in an interrupt context. * Also, drivers must not change altsettings while urbs are scheduled for * endpoints in that interface; all such urbs must first be completed @@ -1376,6 +1381,12 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate) alternate); return -EINVAL; } + /* +* usb3 hosts configure the interface in usb_hcd_alloc_bandwidth, +* including freeing dropped endpoint ring buffers. +* Make sure the interface endpoints are flushed before that +*/ + usb_disable_interface(dev, iface, false); /* Make sure we have enough bandwidth for this alternate interface. * Remove the current alt setting and add the new alt setting. -- 2.7.4
Re: Nothing in /sys/class/udc
On Mon, Sep 03, 2018 at 09:39:14AM +0300, Ranran wrote: > Hello, > > I try to add gadget configfs as described in: > https://www.kernel.org/doc/Documentation/usb/gadget_configfs.txt > Yet, I find nothing in /sys/class/udc: > > user@user-VirtualBox:~/tegra$ ls /sys/class/udc/ -al > total 0 > drwxr-xr-x 2 root root 0 Sep 3 00:30 . > drwxr-xr-x 58 root root 0 Sep 3 00:30 .. > > I also don't have dwc2, but dwc3: > user@user-VirtualBox:~/tegra$ lsmod | grep dw > dwc3 90112 0 > ulpi 16384 1 dwc3 > udc_core 24576 2 dwc3,libcomposite > user@user-VirtualBox:~/tegra$ > > Kernel is 4.4.50. Why do you think the dwc3 driver will work here? Do you really have this hardware being emulated? And 4.4.50 is very old, please use a modern kernel please. thanks, greg k-h
Re: [PATCH] usb: Avoid use-after-free by flushing endpoints early in usb_set_interface()
On 31.08.2018 17:39, Alan Stern wrote: On Fri, 31 Aug 2018, Mathias Nyman wrote: The steps taken by usb core to set a new interface is very different from what is done on the xHC host side. xHC hardware will do everything in one go. One command is used to set up new endpoints, free old endpoints, check bandwidth, and run the new endpoints. All this is done by xHC when usb core asks the hcd to check for available bandwidth. At this point usb core has not yet flushed the old endpoints, which will cause use-after-free issues in xhci driver as queued URBs are cancelled on a re-allocated endpoint. To resolve this add a call to usb_disable_interface() which will flush the endpoints before calling usb_hcd_alloc_bandwidth() Additional checks in xhci driver will also be implemented to gracefully handle stale URB cancel on freed and re-allocated endpoints Cc: Reported-by: Sudip Mukherjee Signed-off-by: Mathias Nyman --- drivers/usb/core/message.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c index 228672f..304bef2 100644 --- a/drivers/usb/core/message.c +++ b/drivers/usb/core/message.c @@ -1377,6 +1377,13 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate) return -EINVAL; } + /* +* usb3 hosts configure the interface in usb_hcd_alloc_bandwidth, +* including freeing dropped endpoint ring buffers. +* Make sure the interface endpoints are flushed before that +*/ + usb_disable_interface(dev, iface, false); + /* Make sure we have enough bandwidth for this alternate interface. * Remove the current alt setting and add the new alt setting. */ Please also update the kerneldoc for this function. It should now specify that if the request fails, the original interface altsetting may be disabled. Drivers cannot rely on any particular alternate setting being in effect after a failure. Alan Stern Sure, thanks, will do -Mathias
Re: [PATCH 0/8] usb: typec: A few more improvements for Intel CHT
On Mon, Sep 3, 2018 at 10:19 AM Heikki Krogerus wrote: > > On Mon, Sep 03, 2018 at 09:17:13AM +0300, Andy Shevchenko wrote: > > On Fri, Aug 31, 2018 at 5:21 PM Heikki Krogerus > > wrote: > > > > > > Hi, > > > > > > The second last patch in this series will make it possible to use > > > multiport USB Type-C and PD controllers with the muxes. The CHT > > > connections are simply adapted to that. The rest of the series will > > > mainly allow us to use the USB Type-C on CHT boards even without a > > > USB device controller. > > > > > > > Throught which tree you are planning to direct this? > > The series is about USB Type-C, so USB would make sense to me. But > it's up to you guys of course. My vacation is about to start, and from my prospective patch series looks good (taking into account the changes Hans asked for). Thus, Acked-by: Andy Shevchenko for PDx86 bits on condition that Hans is fine with them. -- With Best Regards, Andy Shevchenko
Re: [PATCH 0/8] usb: typec: A few more improvements for Intel CHT
Hi, On Mon, Sep 03, 2018 at 10:01:52AM +0200, Hans de Goede wrote: > Besides my comments on patches 3 and 4, one small nitpick, > platform is spelled wrong (as plarform) in the subject of > a number of the patches. Thanks Hans. I'll fix them. Cheers, -- heikki
Re: [PATCH 4/8] usb: xhci: pci: Only create Intel mux device when it's needed
On Mon, Sep 03, 2018 at 10:01:01AM +0200, Hans de Goede wrote: > This patch and "[PATCH 3/8] plarform: x86: intel_cht_int33fe: Use the USB > role switch conditionally" > both assume that the mux will be in host mode when Linux boots, so we do not > need to > touch it. I'm not sure that is a valid assumption. > > More over the USB DP- / DP+ lines should be floated when in device > mode, otherwise USB BC1.2 charger detection will not work, some > PMIC-s have their mux for the data lines and will decouple them > from the SoCs usb data lines when doing charger detection, but > others simply piggy-back on the datalines and are hardwired to > the DP- / DP+ lines of the SoC, if we then do not float the datalines > the PMICs BC detection logic sees a USB host and assumes its a SDP > (standard downstream port) and will limit the charging to 500mA. > > This is at least true for all devices with an AXP288 PMIC, of which > there are a lot (all recent cheap CHT designs use this PMIC). > > So I believe it is best to drop patches 3 and 4 from this patch-set. OK, fair enough. I'll drop those. Thanks, -- heikki
Re: [PATCH 0/9] usb: dwc2: device: Add service interval support
On 8/29/2018 8:58 PM, Grigor Tovmasyan wrote: > This patch set adds Service Interval support for device mode. > > When this mode is enabled core is able to send data any u(f) in current > service interval. > > Also in this mode core is able to accept L1 tokens for ISOC IN endpoints. > > Reference clock was added in the core to track SOF number internally. > Because of some inaccuracies of reference clock new interrupt was added > to initiate remote wake up and keep sync with the host frame number. > > The new interrupt register were added GINTSTS2 for that interrupt. > > > > Grigor Tovmasyan (9): >usb: dwc2: Update registers definitions to support service interval >usb: dwc2: Add core parameter for service interval support >usb: dwc2: Add dwc2_gadget_dec_frame_num_by_one() function >usb: dwc2: Update target (u)frame calculation >usb: dwc2: Add definitions for new registers >usb: dwc2: gadget: Add parameters for GREFCLK register >usb: dwc2: gadget: Program GREFCLK register >usb: dwc2: gadget: enable WKUP_ALERT interrupt >usb: dwc2: gadget: Add handler for WkupAlert interrupt > > drivers/usb/dwc2/core.h| 29 +++ > drivers/usb/dwc2/debugfs.c | 1 + > drivers/usb/dwc2/gadget.c | 91 > ++ > drivers/usb/dwc2/hw.h | 15 > drivers/usb/dwc2/params.c | 6 +++ > 5 files changed, 142 insertions(+) > Acked-by: Minas Harutyunyan
Re: [PATCH 0/8] usb: typec: A few more improvements for Intel CHT
Hi, On 31-08-18 16:20, Heikki Krogerus wrote: Hi, The second last patch in this series will make it possible to use multiport USB Type-C and PD controllers with the muxes. The CHT connections are simply adapted to that. The rest of the series will mainly allow us to use the USB Type-C on CHT boards even without a USB device controller. Heikki Krogerus (8): drivers: base: Helpers for adding device connection descriptions plarform: x86: intel_cht_int33fe: Register all connections at once plarform: x86: intel_cht_int33fe: Use the USB role switch conditionally usb: xhci: pci: Only create Intel mux device when it's needed plarform: x86: intel_cht_int33fe: Fix the identifier for the mux connection plarform: x86: intel_cht_int33fe: Add connections for the USB Type-C port usb: typec: class: Don't use port parent for getting mux handles plarform: x86: intel_cht_int33fe: Remove the old connections for the muxes drivers/platform/x86/intel_cht_int33fe.c | 34 +++-- drivers/usb/host/xhci-pci.c | 20 +++-- drivers/usb/typec/class.c| 38 ++-- include/linux/device.h | 24 +++ 4 files changed, 74 insertions(+), 42 deletions(-) Besides my comments on patches 3 and 4, one small nitpick, platform is spelled wrong (as plarform) in the subject of a number of the patches. Regards, Hans
Re: [PATCH 4/8] usb: xhci: pci: Only create Intel mux device when it's needed
Hi,
On 31-08-18 16:20, Heikki Krogerus wrote:
Only create thre Intel role mux device if the platform has
USB peripheral controller PCI device.
While here, enable the role mux on Apollo Lake platforms.
Signed-off-by: Heikki Krogerus
Cc: Mathias Nyman
---
drivers/usb/host/xhci-pci.c | 20 +---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 6372edf339d9..ea651c2adcfd 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -75,6 +75,17 @@ static int xhci_pci_reinit(struct xhci_hcd *xhci, struct
pci_dev *pdev)
return 0;
}
+static int xhci_pci_board_has_udc(void)
+{
+ struct pci_dev *udc = pci_get_class(PCI_CLASS_SERIAL_USB_DEVICE, NULL);
+
+ if (udc) {
+ pci_dev_put(udc);
+ return true;
+ }
+ return false;
+}
+
static void xhci_pci_quirks(struct device *dev, struct xhci_hcd *xhci)
{
struct pci_dev *pdev = to_pci_dev(dev);
@@ -179,15 +190,18 @@ 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;
- 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 ||
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_CHERRYVIEW_XHCI ||
+pdev->device == PCI_DEVICE_ID_INTEL_APL_XHCI) &&
+ xhci_pci_board_has_udc())
+ xhci->quirks |= XHCI_INTEL_USB_ROLE_SW;
if (pdev->vendor == PCI_VENDOR_ID_ETRON &&
pdev->device == PCI_DEVICE_ID_EJ168) {
This patch and "[PATCH 3/8] plarform: x86: intel_cht_int33fe: Use the USB role
switch conditionally"
both assume that the mux will be in host mode when Linux boots, so we do not
need to
touch it. I'm not sure that is a valid assumption.
More over the USB DP- / DP+ lines should be floated when in device
mode, otherwise USB BC1.2 charger detection will not work, some
PMIC-s have their mux for the data lines and will decouple them
from the SoCs usb data lines when doing charger detection, but
others simply piggy-back on the datalines and are hardwired to
the DP- / DP+ lines of the SoC, if we then do not float the datalines
the PMICs BC detection logic sees a USB host and assumes its a SDP
(standard downstream port) and will limit the charging to 500mA.
This is at least true for all devices with an AXP288 PMIC, of which
there are a lot (all recent cheap CHT designs use this PMIC).
So I believe it is best to drop patches 3 and 4 from this patch-set.
Regards,
Hans
Re: [PATCH 0/8] usb: typec: A few more improvements for Intel CHT
On Mon, Sep 03, 2018 at 09:17:13AM +0300, Andy Shevchenko wrote: > On Fri, Aug 31, 2018 at 5:21 PM Heikki Krogerus > wrote: > > > > Hi, > > > > The second last patch in this series will make it possible to use > > multiport USB Type-C and PD controllers with the muxes. The CHT > > connections are simply adapted to that. The rest of the series will > > mainly allow us to use the USB Type-C on CHT boards even without a > > USB device controller. > > > > Throught which tree you are planning to direct this? The series is about USB Type-C, so USB would make sense to me. But it's up to you guys of course. Thanks, -- heikki
Re: [PATCH 4/8] usb: xhci: pci: Only create Intel mux device when it's needed
On Mon, Sep 03, 2018 at 09:01:47AM +0300, Andy Shevchenko wrote:
> On Fri, Aug 31, 2018 at 5:21 PM Heikki Krogerus
> wrote:
> >
> > Only create thre Intel role mux device if the platform has
> > USB peripheral controller PCI device.
> >
> > While here, enable the role mux on Apollo Lake platforms.
>
> > +static int xhci_pci_board_has_udc(void)
> > +{
> > + struct pci_dev *udc = pci_get_class(PCI_CLASS_SERIAL_USB_DEVICE,
> > NULL);
> > +
> > + if (udc) {
> > + pci_dev_put(udc);
> > + return true;
> > + }
> > + return false;
> > +}
>
> Looks like a code duplication with patch 3. Does it make sense to have
> this in some header (usb.h?)?
I don't know. The check is very PCI specific. I'm not sure ush.h
would be appropriate place for it. I don't know where should it go?
Right now the check is only needed on Intel CHT (in both patches), so
I figured that it's better wait for an other user before introducing
a helper for it. Would that make sense?
Thanks,
--
heikki
Nothing in /sys/class/udc
Hello, I try to add gadget configfs as described in: https://www.kernel.org/doc/Documentation/usb/gadget_configfs.txt Yet, I find nothing in /sys/class/udc: user@user-VirtualBox:~/tegra$ ls /sys/class/udc/ -al total 0 drwxr-xr-x 2 root root 0 Sep 3 00:30 . drwxr-xr-x 58 root root 0 Sep 3 00:30 .. I also don't have dwc2, but dwc3: user@user-VirtualBox:~/tegra$ lsmod | grep dw dwc3 90112 0 ulpi 16384 1 dwc3 udc_core 24576 2 dwc3,libcomposite user@user-VirtualBox:~/tegra$ Kernel is 4.4.50. Thank you for any idea, ranran
Re: [PATCH 0/8] usb: typec: A few more improvements for Intel CHT
On Fri, Aug 31, 2018 at 5:21 PM Heikki Krogerus wrote: > > Hi, > > The second last patch in this series will make it possible to use > multiport USB Type-C and PD controllers with the muxes. The CHT > connections are simply adapted to that. The rest of the series will > mainly allow us to use the USB Type-C on CHT boards even without a > USB device controller. > Throught which tree you are planning to direct this? > > Heikki Krogerus (8): > drivers: base: Helpers for adding device connection descriptions > plarform: x86: intel_cht_int33fe: Register all connections at once > plarform: x86: intel_cht_int33fe: Use the USB role switch > conditionally > usb: xhci: pci: Only create Intel mux device when it's needed > plarform: x86: intel_cht_int33fe: Fix the identifier for the mux > connection > plarform: x86: intel_cht_int33fe: Add connections for the USB Type-C > port > usb: typec: class: Don't use port parent for getting mux handles > plarform: x86: intel_cht_int33fe: Remove the old connections for the > muxes > > drivers/platform/x86/intel_cht_int33fe.c | 34 +++-- > drivers/usb/host/xhci-pci.c | 20 +++-- > drivers/usb/typec/class.c| 38 ++-- > include/linux/device.h | 24 +++ > 4 files changed, 74 insertions(+), 42 deletions(-) > > -- > 2.18.0 > -- With Best Regards, Andy Shevchenko
Re: [PATCH 4/8] usb: xhci: pci: Only create Intel mux device when it's needed
On Fri, Aug 31, 2018 at 5:21 PM Heikki Krogerus
wrote:
>
> Only create thre Intel role mux device if the platform has
> USB peripheral controller PCI device.
>
> While here, enable the role mux on Apollo Lake platforms.
> +static int xhci_pci_board_has_udc(void)
> +{
> + struct pci_dev *udc = pci_get_class(PCI_CLASS_SERIAL_USB_DEVICE,
> NULL);
> +
> + if (udc) {
> + pci_dev_put(udc);
> + return true;
> + }
> + return false;
> +}
Looks like a code duplication with patch 3. Does it make sense to have
this in some header (usb.h?)?
--
With Best Regards,
Andy Shevchenko
