Re: [PATCH v3] usbip: dynamically allocate idev by nports found in sysfs
On Wed, May 23, 2018 at 10:44:57AM -0600, Shuah Khan wrote: > On 05/23/2018 03:22 AM, Michael Grzeschik wrote: > > As the amount of available ports varies by the kernels build > > configuration. To remove the limitation of the fixed 128 ports > > we allocate the amount of idevs by using the number we get > > from the kernel. > > > > Signed-off-by: Michael Grzeschik > > --- > > v1 -> v2: - reworked memory allocation into one calloc call > > - added error path on allocation failure > > v2 -> v3: - moved check for available nports to beginning of function > > > > Hmm. With this patch I see a segfault when I run usbip port command. > I think this patch is incomplete and more changes are needed to the > code that references the idev array. > > I can't take this patch. I missed that get_nports depends on vhci_driver->hc_device which is not initialized that early. I will rework it to get the hc_device by parameter and send v4. Thanks, Michael -- Pengutronix e.K. | | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0| Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | signature.asc Description: PGP signature
Re: [PATCH/RFC v4 2/4] usb: gadget: udc: renesas_usb3: Add register of usb role switch
Hi Rob,
On Wed, May 23, 2018 at 5:00 PM, Rob Herring wrote:
> On Wed, May 23, 2018 at 1:52 AM, Yoshihiro Shimoda
> wrote:
>>> From: Rob Herring, Sent: Wednesday, May 23, 2018 2:13 AM
>>> On Tue, May 22, 2018 at 09:01:07PM +0900, Yoshihiro Shimoda wrote:
>>> > --- a/Documentation/devicetree/bindings/usb/renesas_usb3.txt
>>> > +++ b/Documentation/devicetree/bindings/usb/renesas_usb3.txt
>>> > @@ -19,6 +19,9 @@ Required properties:
>>> > Optional properties:
>>> >- phys: phandle + phy specifier pair
>>> >- phy-names: must be "usb"
>>> > + - The connection to a usb3.0 host node needs by using OF graph
>>> > bindings for
>>> > +usb role switch.
>>> > + - port@0 = USB3.0 host port.
>>>
>>> On the host side, this might conflict with the USB connector binding.
>>>
>>> I would either make sure this can work with the connector binding by
>>> having 2 endpoints on the HS or SS port or just use the 'companion'
>>> property defined in usb-generic.txt.
>>
>> I don't understand the first one now... This means the renesas_usb3 should
>> follow
>> USB connector binding and have 2 endpoints for the usb role switch to avoid
>> the conflict like below?
>> - port1@0: Super Speed (SS), present in SS capable connectors (from
>> usb-connector.txt).
>> - port1@1: USB3.0 host port.
>
> I'm confused, SS and USB3.0 are the same essentially. It would be:
>
> port@1/endpoint@0: SS host port
> port@1/endpoint@1: SS device port
>
>> About the 'companion' in usb-generic.txt, the property intends to be used
>> for EHCI or host side
>> like the commit log [1]. If there is accept to use 'companion' for this
>> patch, I think it will
>> be simple to achieve this role switch feature. However, in last month, I
>> submitted a similar patch [2]
>> that has "renesas,host" property, but I got reply from Andy [3] and Heikki
>> [4]. So, I'm
>> trying to improve the device connection framework [5] now.
>
> I think this case is rare enough that we don't need a general solution
> using OF graph, so I'm fine with a simple, single property to link the
> 2 nodes. Either reusing "companion" or "renesas,host" is fine by me.
I'd go for the standard "companion" over "renesas,host"[*].
[*] Doh, we have another one ("renesas,bonding"), invented when I wasn't
aware of the existence of "companion" yet...
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 3/4] usb: typec: fusb302: Fix debugfs issue
Removing the "fusb302" debugfs directory when unloading
the driver. That allows the driver to be loaded more then
one time. The directory will not get actually removed until
it is empty, so only after the last instance has been
removed.
This fixes an issue where the driver can't be re-loaded if
it has been unloaded as the "fusb302" debugfs directory
already exists.
Fixes: 76f0c53d08b9 ("usb: typec: fusb302: Move out of staging")
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/fusb302/fusb302.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/typec/fusb302/fusb302.c
b/drivers/usb/typec/fusb302/fusb302.c
index eba6bb890b17..9c1eba9ea004 100644
--- a/drivers/usb/typec/fusb302/fusb302.c
+++ b/drivers/usb/typec/fusb302/fusb302.c
@@ -234,6 +234,7 @@ static int fusb302_debugfs_init(struct fusb302_chip *chip)
static void fusb302_debugfs_exit(struct fusb302_chip *chip)
{
debugfs_remove(chip->dentry);
+ debugfs_remove(rootdir);
}
#else
--
2.17.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/4] usb: roles: intel_xhci: Always allow user control
Trying to determine the USB port type with this mux is very
difficult. To simplify the situation, always allow user
control, even if the port is USB Type-C port.
Reviewed-by: Hans de Goede
Signed-off-by: Heikki Krogerus
---
.../usb/roles/intel-xhci-usb-role-switch.c| 23 ++-
1 file changed, 2 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c
b/drivers/usb/roles/intel-xhci-usb-role-switch.c
index 28102127b9d5..6e922b50b674 100644
--- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
+++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
@@ -38,20 +38,6 @@ struct intel_xhci_usb_data {
void __iomem *base;
};
-struct intel_xhci_acpi_match {
- const char *hid;
- int hrv;
-};
-
-/*
- * ACPI IDs for PMICs which do not support separate data and power role
- * detection (USB ACA detection for micro USB OTG), we allow userspace to
- * change the role manually on these.
- */
-static const struct intel_xhci_acpi_match allow_userspace_ctrl_ids[] = {
- { "INT33F4", 3 }, /* X-Powers AXP288 PMIC */
-};
-
static int intel_xhci_usb_set_role(struct device *dev, enum usb_role role)
{
struct intel_xhci_usb_data *data = dev_get_drvdata(dev);
@@ -127,9 +113,10 @@ static enum usb_role intel_xhci_usb_get_role(struct device
*dev)
return role;
}
-static struct usb_role_switch_desc sw_desc = {
+static const struct usb_role_switch_desc sw_desc = {
.set = intel_xhci_usb_set_role,
.get = intel_xhci_usb_get_role,
+ .allow_userspace_control = true,
};
static int intel_xhci_usb_probe(struct platform_device *pdev)
@@ -137,7 +124,6 @@ static int intel_xhci_usb_probe(struct platform_device
*pdev)
struct device *dev = &pdev->dev;
struct intel_xhci_usb_data *data;
struct resource *res;
- int i;
data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
@@ -150,11 +136,6 @@ static int intel_xhci_usb_probe(struct platform_device
*pdev)
if (!data->base)
return -ENOMEM;
- for (i = 0; i < ARRAY_SIZE(allow_userspace_ctrl_ids); i++)
- if (acpi_dev_present(allow_userspace_ctrl_ids[i].hid, "1",
-allow_userspace_ctrl_ids[i].hrv))
- sw_desc.allow_userspace_control = true;
-
platform_set_drvdata(pdev, data);
data->role_sw = usb_role_switch_register(dev, &sw_desc);
--
2.17.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/4] platform: x86: intel_cht_int33fe: Fix dependencies
The driver will not probe unless bq24190 is loaded, so making it a dependency. Signed-off-by: Heikki Krogerus Cc: Wolfram Sang Cc: Darren Hart Cc: Andy Shevchenko --- drivers/i2c/busses/Kconfig | 3 +-- drivers/platform/x86/Kconfig | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig index 99edffae27f6..4f8df2ec87b1 100644 --- a/drivers/i2c/busses/Kconfig +++ b/drivers/i2c/busses/Kconfig @@ -202,8 +202,7 @@ config I2C_CHT_WC Note this controller is hooked up to a TI bq24292i charger-IC, combined with a FUSB302 Type-C port-controller as such it is advised - to also select CONFIG_CHARGER_BQ24190=m and CONFIG_TYPEC_FUSB302=m - (the fusb302 driver currently is in drivers/staging). + to also select CONFIG_TYPEC_FUSB302=m. config I2C_NFORCE2 tristate "Nvidia nForce2, nForce3 and nForce4" diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig index 566644bb496a..f27cb186437d 100644 --- a/drivers/platform/x86/Kconfig +++ b/drivers/platform/x86/Kconfig @@ -866,6 +866,7 @@ config ACPI_CMPC 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) ---help--- This driver add support for the INT33FE ACPI device found on some Intel Cherry Trail devices. @@ -877,8 +878,7 @@ config INTEL_CHT_INT33FE i2c drivers for these chips can bind to the them. If you enable this driver it is advised to also select - CONFIG_TYPEC_FUSB302=m, CONFIG_CHARGER_BQ24190=m and - CONFIG_BATTERY_MAX17042=m. + CONFIG_TYPEC_FUSB302=m and CONFIG_BATTERY_MAX17042=m. config INTEL_INT0002_VGPIO tristate "Intel ACPI INT0002 Virtual GPIO driver" -- 2.17.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 4/4] usb: roles: intel_xhci: Enable runtime PM
This fixes an issue where the mux does not get configured
when the parent device is suspended. The registers for this
mux are mapped to the parent device MMIO (usually xHCI PCI
device), so in order for the driver to be able to program
the registers, the parent device must be resumed.
Reported-by: Sathyanarayanan Kuppuswamy
Fixes: f6fb9ec02be1 ("usb: roles: Add Intel xHCI USB role switch driver")
Signed-off-by: Heikki Krogerus
---
drivers/usb/roles/intel-xhci-usb-role-switch.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c
b/drivers/usb/roles/intel-xhci-usb-role-switch.c
index 6e922b50b674..1fb3dd0f1dfa 100644
--- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
+++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
/* register definition */
@@ -56,6 +57,8 @@ static int intel_xhci_usb_set_role(struct device *dev, enum
usb_role role)
return -EIO;
}
+ pm_runtime_get_sync(dev);
+
/* Set idpin value as requested */
val = readl(data->base + DUAL_ROLE_CFG0);
switch (role) {
@@ -84,13 +87,17 @@ static int intel_xhci_usb_set_role(struct device *dev, enum
usb_role role)
/* Polling on CFG1 register to confirm mode switch.*/
do {
val = readl(data->base + DUAL_ROLE_CFG1);
- if (!!(val & HOST_MODE) == (role == USB_ROLE_HOST))
+ if (!!(val & HOST_MODE) == (role == USB_ROLE_HOST)) {
+ pm_runtime_put(dev);
return 0;
+ }
/* Interval for polling is set to about 5 - 10 ms */
usleep_range(5000, 1);
} while (time_before(jiffies, timeout));
+ pm_runtime_put(dev);
+
dev_warn(dev, "Timeout waiting for role-switch\n");
return -ETIMEDOUT;
}
@@ -101,7 +108,9 @@ static enum usb_role intel_xhci_usb_get_role(struct device
*dev)
enum usb_role role;
u32 val;
+ pm_runtime_get_sync(dev);
val = readl(data->base + DUAL_ROLE_CFG0);
+ pm_runtime_put(dev);
if (!(val & SW_IDPIN))
role = USB_ROLE_HOST;
@@ -142,6 +151,9 @@ static int intel_xhci_usb_probe(struct platform_device
*pdev)
if (IS_ERR(data->role_sw))
return PTR_ERR(data->role_sw);
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(dev);
+
return 0;
}
--
2.17.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/4] usb: typec: fixes for Cherry Trails
Hi, This is third version of patches that fix various problems I encountered while testing my USB Type-C Alternate Mode patches with GPD Win board (Intel Cherry Trail based). I added one more patch to this series. Sathyanarayanan reported an issue where the mux does not get configured when xHCI is suspended. To fix that we need to enable runtime PM in the mux driver. Other changes since v2: constifying struct usb_role_switch_desc sw_desc, and improvements to the commit messages. Link to the original patches: https://lkml.org/lkml/2018/4/30/350 Heikki Krogerus (4): usb: roles: intel_xhci: Always allow user control platform: x86: intel_cht_int33fe: Fix dependencies usb: typec: fusb302: Fix debugfs issue usb: roles: intel_xhci: Enable runtime PM drivers/i2c/busses/Kconfig| 3 +- drivers/platform/x86/Kconfig | 4 +- .../usb/roles/intel-xhci-usb-role-switch.c| 37 --- drivers/usb/typec/fusb302/fusb302.c | 1 + 4 files changed, 19 insertions(+), 26 deletions(-) -- 2.17.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/4] usb: typec: fusb302: Fix debugfs issue
On 05/24/2018 01:18 AM, Heikki Krogerus wrote:
Removing the "fusb302" debugfs directory when unloading
the driver. That allows the driver to be loaded more then
one time. The directory will not get actually removed until
it is empty, so only after the last instance has been
removed.
This fixes an issue where the driver can't be re-loaded if
it has been unloaded as the "fusb302" debugfs directory
already exists.
Fixes: 76f0c53d08b9 ("usb: typec: fusb302: Move out of staging")
Signed-off-by: Heikki Krogerus
Reviewed-by: Guenter Roeck
---
drivers/usb/typec/fusb302/fusb302.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/typec/fusb302/fusb302.c
b/drivers/usb/typec/fusb302/fusb302.c
index eba6bb890b17..9c1eba9ea004 100644
--- a/drivers/usb/typec/fusb302/fusb302.c
+++ b/drivers/usb/typec/fusb302/fusb302.c
@@ -234,6 +234,7 @@ static int fusb302_debugfs_init(struct fusb302_chip *chip)
static void fusb302_debugfs_exit(struct fusb302_chip *chip)
{
debugfs_remove(chip->dentry);
+ debugfs_remove(rootdir);
}
#else
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: typec: wcove: Remove dependency on HW FSM
The USB Type-C PHY in Intel WhiskeyCove PMIC has build-in
USB Type-C state machine which we were relying on to
configure the CC lines correctly. This patch removes that
dependency and configures the CC line according to commands
from the port manager (tcpm.c) in wcove_set_cc().
This fixes an issue where USB devices attached to the USB
Type-C port do not get enumerated. When acting as
source/host, the HW FSM sometimes fails to configure the PHY
correctly.
Fixes: 3c4fb9f16921 ("usb: typec: wcove: start using tcpm for USB PD support")
Cc: sta...@vger.kernel.org
Signed-off-by: Heikki Krogerus
---
drivers/usb/typec/typec_wcove.c | 30 --
1 file changed, 28 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/typec/typec_wcove.c b/drivers/usb/typec/typec_wcove.c
index 39cff11ec7a2..423208e19383 100644
--- a/drivers/usb/typec/typec_wcove.c
+++ b/drivers/usb/typec/typec_wcove.c
@@ -202,6 +202,10 @@ static int wcove_init(struct tcpc_dev *tcpc)
struct wcove_typec *wcove = tcpc_to_wcove(tcpc);
int ret;
+ ret = regmap_write(wcove->regmap, USBC_CONTROL1, 0);
+ if (ret)
+ return ret;
+
/* Unmask everything */
ret = regmap_write(wcove->regmap, USBC_IRQMASK1, 0);
if (ret)
@@ -285,8 +289,30 @@ static int wcove_get_cc(struct tcpc_dev *tcpc, enum
typec_cc_status *cc1,
static int wcove_set_cc(struct tcpc_dev *tcpc, enum typec_cc_status cc)
{
- /* XXX: Relying on the HW FSM to configure things correctly for now */
- return 0;
+ struct wcove_typec *wcove = tcpc_to_wcove(tcpc);
+ unsigned int ctrl;
+
+ switch (cc) {
+ case TYPEC_CC_RD:
+ ctrl = USBC_CONTROL1_MODE_SNK;
+ break;
+ case TYPEC_CC_RP_DEF:
+ ctrl = USBC_CONTROL1_CURSRC_UA_80 | USBC_CONTROL1_MODE_SRC;
+ break;
+ case TYPEC_CC_RP_1_5:
+ ctrl = USBC_CONTROL1_CURSRC_UA_180 | USBC_CONTROL1_MODE_SRC;
+ break;
+ case TYPEC_CC_RP_3_0:
+ ctrl = USBC_CONTROL1_CURSRC_UA_330 | USBC_CONTROL1_MODE_SRC;
+ break;
+ case TYPEC_CC_OPEN:
+ ctrl = 0;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return regmap_write(wcove->regmap, USBC_CONTROL1, ctrl);
}
static int wcove_set_polarity(struct tcpc_dev *tcpc, enum typec_cc_polarity
pol)
--
2.17.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: dwc2: gadget: ISOC's starting flow improvement
To start ISOC transfers in handlers dwc2_gadget_handle_nak() and
dwc2_gadget_handle_out_token_ep_disabled() driver reads current frame
number, based on which, set target frame number to start first ISOC
transfer.
In case if system's high IRQ latency and multiple EP's asserted
interrupt in same frame, there are high probability that when reading
current frame number in EP's handlers, actual frame number can be
increased. As result for bInterval > 1, starting target frame
will be set wrongly and all ISOC packets will be dropped.
In patch "usb: dwc2: Change reading of current frame number flow"
reading of current frame number done ASAP in common interrupt handler.
This frame number stored in frame_number variable which used as
starting frame number for ISOC EP's in above mentioned handlers.
Signed-off-by: Minas Harutyunyan
---
drivers/usb/dwc2/gadget.c | 19 ---
1 file changed, 4 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 676712159980..fedf4dd65cc5 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2749,23 +2749,16 @@ static void
dwc2_gadget_handle_out_token_ep_disabled(struct dwc2_hsotg_ep *ep)
struct dwc2_hsotg *hsotg = ep->parent;
int dir_in = ep->dir_in;
u32 doepmsk;
- u32 tmp;
if (dir_in || !ep->isochronous)
return;
- /*
-* Store frame in which irq was asserted here, as
-* it can change while completing request below.
-*/
- tmp = dwc2_hsotg_read_frameno(hsotg);
-
dwc2_hsotg_complete_request(hsotg, ep, get_ep_head(ep), 0);
if (using_desc_dma(hsotg)) {
if (ep->target_frame == TARGET_FRAME_INITIAL) {
/* Start first ISO Out */
- ep->target_frame = tmp;
+ ep->target_frame = hsotg->frame_number;
dwc2_gadget_start_isoc_ddma(ep);
}
return;
@@ -2773,11 +2766,9 @@ static void
dwc2_gadget_handle_out_token_ep_disabled(struct dwc2_hsotg_ep *ep)
if (ep->interval > 1 &&
ep->target_frame == TARGET_FRAME_INITIAL) {
- u32 dsts;
u32 ctrl;
- dsts = dwc2_readl(hsotg->regs + DSTS);
- ep->target_frame = dwc2_hsotg_read_frameno(hsotg);
+ ep->target_frame = hsotg->frame_number;
dwc2_gadget_incr_frame_num(ep);
ctrl = dwc2_readl(hsotg->regs + DOEPCTL(ep->index));
@@ -2813,25 +2804,23 @@ static void dwc2_gadget_handle_nak(struct dwc2_hsotg_ep
*hs_ep)
{
struct dwc2_hsotg *hsotg = hs_ep->parent;
int dir_in = hs_ep->dir_in;
- u32 tmp;
if (!dir_in || !hs_ep->isochronous)
return;
if (hs_ep->target_frame == TARGET_FRAME_INITIAL) {
- tmp = dwc2_hsotg_read_frameno(hsotg);
if (using_desc_dma(hsotg)) {
dwc2_hsotg_complete_request(hsotg, hs_ep,
get_ep_head(hs_ep), 0);
- hs_ep->target_frame = tmp;
+ hs_ep->target_frame = hsotg->frame_number;
dwc2_gadget_incr_frame_num(hs_ep);
dwc2_gadget_start_isoc_ddma(hs_ep);
return;
}
- hs_ep->target_frame = tmp;
+ hs_ep->target_frame = hsotg->frame_number;
if (hs_ep->interval > 1) {
u32 ctrl = dwc2_readl(hsotg->regs +
DIEPCTL(hs_ep->index));
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Serdev: USB device and sysdev probing ala i2c
Hi Ricardo, On Wed, May 23, 2018 at 11:17:20AM +0200, Ricardo Ribalda Delgado wrote: > Hi > > I have a flash controller connected to the main computer via a usb to > serial. My plan is to expose it to the system as a video4linux subdevice. > > With the inclusion of serdev I was expecting that it would be as easy as > adding a i2c device, but seems that there are some functionality that it is > still not implemented: > > 1) Serdev for usb serial devices. Right, I didn't want to enable serdev for USB serial before we have determined how to handle hotplugging (e.g. in serdev core or by making sure every serdev driver can handle devices going away at any time) in order to avoid having things crash left and right. I have out-of-tree code for USB serial that I use for testing purposes, so it's mostly a matter of finding the time to think this through. > 2) Instatiating via sysfs. Something like > echo hci_nokia > /sys/bus/serio/devices/serio0/new_device > (inspired in: echo eeprom 0x50 > /sys/bus/i2c/devices/i2c-3/new_device) Serdev currently only supports device tree and ACPI. Using out-of-tree code, you could load a device tree fragment during runtime to describe your serial bus (or you just amend the device tree). Using device tree overlays would have the benefit of being able to describe associated resources (e.g. reset gpios) which a simple compatible string (or equivalent) would not. But there are examples where a simple compatible string would do, for example an existing CEC device presenting itself as a generic USB CDC device (hopefully with a dedicated VID/PID so that no user-space configuration is needed at all). > 3) Support for probing: Like hwmon for i2c What would you probe for (since there is no generic protocol for serial devices)? > -Are these two things in the drawing board? > -if not, would it be something worth considering/reviewing for > upstream? So the first two points have been given some thought and is something we'd want to have eventually, while the third point has mostly been rejected (I think). Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Serdev: USB device and sysdev probing ala i2c
Hi Johan, On Thu, May 24, 2018 at 2:07 PM Johan Hovold wrote: > Hi Ricardo, > On Wed, May 23, 2018 at 11:17:20AM +0200, Ricardo Ribalda Delgado wrote: > > Hi > > > > I have a flash controller connected to the main computer via a usb to > > serial. My plan is to expose it to the system as a video4linux subdevice. > > > > With the inclusion of serdev I was expecting that it would be as easy as > > adding a i2c device, but seems that there are some functionality that it is > > still not implemented: > > > > 1) Serdev for usb serial devices. > Right, I didn't want to enable serdev for USB serial before we have > determined how to handle hotplugging (e.g. in serdev core or by making > sure every serdev driver can handle devices going away at any time) in > order to avoid having things crash left and right. > I have out-of-tree code for USB serial that I use for testing purposes, > so it's mostly a matter of finding the time to think this through. Could you share those patches? I would love to test them. In my setup the system does not support hotplugg and/or power saving. > > 2) Instatiating via sysfs. Something like > > echo hci_nokia > /sys/bus/serio/devices/serio0/new_device > > (inspired in: echo eeprom 0x50 > /sys/bus/i2c/devices/i2c-3/new_device) > Serdev currently only supports device tree and ACPI. Using out-of-tree > code, you could load a device tree fragment during runtime to describe > your serial bus (or you just amend the device tree). > Using device tree overlays would have the benefit of being able to > describe associated resources (e.g. reset gpios) which a simple > compatible string (or equivalent) would not. > But there are examples where a simple compatible string would do, for > example an existing CEC device presenting itself as a generic USB CDC > device (hopefully with a dedicated VID/PID so that no user-space > configuration is needed at all). What about platform devices? Can it be instatiated like that? > > 3) Support for probing: Like hwmon for i2c > What would you probe for (since there is no generic protocol for serial > devices)? Just write to the device and expect something in return. I know it can be dangerous in most cases, but hey, this is how the modems are detected in userspace and works fine :). > > -Are these two things in the drawing board? > > -if not, would it be something worth considering/reviewing for > > upstream? > So the first two points have been given some thought and is something > we'd want to have eventually, while the third point has mostly been > rejected (I think). I did not expect the last point to be enabled by default, but it could be useful for some platforms if you know that your serial devices are tolerant to some "noise" Thanks! > Johan -- Ricardo Ribalda -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/12] usb: usbtmc: Support Read Status Byte with SRQ per file handle
Zitat von Oliver Neukum : Am Montag, den 21.05.2018, 21:00 + schrieb guido@kiener- muenchen.de: I looked for a race here, but I do not find a race between open and release, since a refcount of "file_data->data->kref" is always hold by usbtmc_probe/disconnect. However I see a race between usbtmc_open and usbtmc_disconnect. Are these callback functions called mutual exclusive? No, they are not. In the meantime I found these conflictive hints: https://github.com/torvalds/linux/commit/52a749992ca6a0fd304609af40ed3bfd6cef4660 and https://elixir.bootlin.com/linux/v4.17-rc6/source/include/linux/usb.h#L1164 What do you think? My current feeling is that open/disconnect is mutual exclusive. We also could verify what really happens. Thanks, Guido I'm not sure, but if not, then I think we have the same problem in usb-skeleton.c In usb-skeleton.c a race exists. You are right. Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/12] usb: usbtmc: Add ioctls for trigger, EOM bit and TermChar
Zitat von Oliver Neukum : Am Donnerstag, den 17.05.2018, 19:03 +0200 schrieb Guido Kiener: + retval = usb_bulk_msg(data->usb_dev, + usb_sndbulkpipe(data->usb_dev, + data->bulk_out), + buffer, USBTMC_HEADER_SIZE, + &actual, file_data->timeout); + + /* Store bTag (in case we need to abort) */ + data->bTag_last_write = data->bTag; + + /* Increment bTag -- and increment again if zero */ + data->bTag++; + if (!data->bTag) + data->bTag++; + Independent of whether this needs to be split up, do you really want to do this regardless of usb_bulk_msg() returning an error? Regards Oliver I think it is ok. Our devices do not care much about the right order of sequence numbers. It is just a hint to abort the last messages. And the client should know the last message numbers itself. The current implementation does it the same way. https://elixir.bootlin.com/linux/v4.17-rc6/source/drivers/usb/class/usbtmc.c#L835 Regards Guido -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 07/12] usb: usbtmc: Add ioctl USBTMC488_IOCTL_WAIT_SRQ
Zitat von Oliver Neukum :
Am Donnerstag, den 17.05.2018, 19:03 +0200 schrieb Guido Kiener:
+static int usbtmc488_ioctl_wait_srq(struct usbtmc_file_data *file_data,
+ unsigned int __user *arg)
+{
+ struct usbtmc_device_data *data = file_data->data;
+ struct device *dev = &data->intf->dev;
+ int rv;
+ unsigned int timeout;
+ unsigned long expire;
+
+ if (!data->iin_ep_present) {
+ dev_dbg(dev, "no interrupt endpoint present\n");
+ return -EFAULT;
+ }
+
+ if (get_user(timeout, arg))
+ return -EFAULT;
+
+ expire = msecs_to_jiffies(timeout);
+
+ mutex_unlock(&data->io_mutex);
There is such a thing as threads sharing file descriptors.
That leads to the question what happens to the mutex if this
ioctl() is called multiple times.
Regards
Oliver
Multiple threads can wait with the same or different file descriptors.
When an SRQ interrupt occurs, all threads and file descriptors are
informed concurrently with wake_up_interruptible_all(&data->waitq);
The "_all" is already fixed in 02/12.
Regards,
Guido
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: f_uvc: Expose configuration name through video node
Hi Kieran,
Thank you for the patch.
On Wednesday, 23 May 2018 23:39:47 EEST Kieran Bingham wrote:
> From: Kieran Bingham
>
> When utilising multiple instantiations of a UVC gadget on a composite
> device, there is no clear method to link a particular configuration to
> its respective video node.
>
> Provide a means for identifying the correct video node by exposing the
> name of the function configuration through sysfs.
>
> Signed-off-by: Kieran Bingham
> ---
> drivers/usb/gadget/function/f_uvc.c | 24 +++-
> 1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/function/f_uvc.c
> b/drivers/usb/gadget/function/f_uvc.c index 1affa8e3a974..f326d1707633
> 100644
> --- a/drivers/usb/gadget/function/f_uvc.c
> +++ b/drivers/usb/gadget/function/f_uvc.c
> @@ -421,10 +421,21 @@ uvc_function_disconnect(struct uvc_device *uvc)
> * USB probe and disconnect
> */
>
> +static ssize_t config_show(struct device *dev, struct device_attribute
> *attr, + char *buf)
> +{
> + struct uvc_device *uvc = dev_get_drvdata(dev);
> +
> + return sprintf(buf, "%s\n", uvc->func.fi->group.cg_item.ci_name);
> +}
> +
> +static DEVICE_ATTR_RO(config);
You need to document the attribute in Documentation/ABI/.
I wonder whether we should use a more explicit name for the attribute. Maybe
function_name ? usb_function_name ? usb_configfs_name ? Feel free to propose a
better option.
> +
> static int
> uvc_register_video(struct uvc_device *uvc)
> {
> struct usb_composite_dev *cdev = uvc->func.config->cdev;
> + int ret;
>
> /* TODO reference counting. */
> uvc->vdev.v4l2_dev = &uvc->v4l2_dev;
> @@ -437,7 +448,17 @@ uvc_register_video(struct uvc_device *uvc)
>
> video_set_drvdata(&uvc->vdev, uvc);
>
> - return video_register_device(&uvc->vdev, VFL_TYPE_GRABBER, -1);
> + ret = video_register_device(&uvc->vdev, VFL_TYPE_GRABBER, -1);
> + if (ret < 0)
> + return ret;
> +
> + ret = device_create_file(&uvc->vdev.dev, &dev_attr_config);
> + if (ret < 0) {
> + video_unregister_device(&uvc->vdev);
> + return ret;
> + }
> +
> + return 0;
> }
>
> #define UVC_COPY_DESCRIPTOR(mem, dst, desc) \
> @@ -875,6 +896,7 @@ static void uvc_unbind(struct usb_configuration *c,
> struct usb_function *f)
>
> INFO(cdev, "%s\n", __func__);
>
> + device_remove_file(&uvc->vdev.dev, &dev_attr_config);
> video_unregister_device(&uvc->vdev);
> v4l2_device_unregister(&uvc->v4l2_dev);
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: usb HC busted?
Hi
On 24.05.2018 00:29, Sudip Mukherjee wrote:
Hi Mathias,
On Fri, May 18, 2018 at 04:19:02PM +0300, Mathias Nyman wrote:
On 18.05.2018 16:04, Sudip Mukherjee wrote:
Hi Mathias,
On Fri, May 18, 2018 at 03:55:04PM +0300, Mathias Nyman wrote:
Hi,
Looks like event for Transfer block (TRB) at 0x32a21060 was never completed,
or at least not handled by xhci driver.
(either the event was never issued by hw, or something got messed up in the
driver along the way)
HC doesn't look busted, it continues sending transfer completions events.
it is already at event 0x32a211d0, which is 23 TRBS later. (one TRB is 0x10)
This small log sinppet doesnt' say much about the reasons.
Can you enable tracing for xhci and send me the output.
We have finally reproduced the error while traces were on. The trace and
the relevant part of the dmesg (when the error starts) are in:
https://drive.google.com/open?id=1PbcYwL1a9ndtHw1MNjE6uVqb0fFX9jV8
Will request you to have a look and suggest what might be going wrong here.
Log show two rings having the same TRB segment dma address, this will
completely mess up the transfer:
While allocating rigs the enque pointers for the two rings are the same:
461.859315: xhci_ring_alloc: ISOC efa4e580: enq
0x33386000(0x33386000) deq
0x33386000(0x33386000) segs 2 stream 0 ...bs
461.859320: xhci_ring_alloc: ISOC f0ce1f00: enq
0x33386000(0x33386000) deq
0x33386000(0x33386000) segs 2 stream 0 ...
URBs for ISOC IN transfers are queued on EP3 at enqueue address (33386000 to
33386140)
461.859998: xhci_urb_enqueue: ep3in-isoc: urb f0ec0e00 pipe 4294528 slot 8
length 0/170 sgs 0/0 stream 0 flags 00010302
461.860004: xhci_queue_trb: ISOC: Buffer 2b480240 length 17 TD size 0
intr 0 type 'Isoch' flags b:i:I:c:s:I:e:c
461.860006: xhci_inc_enq: ISOC f0ce1f00: enq
0x33386010(0x33386000) deq 0x33386000(0x33386000
Later URBs for ISOC OUT transfers are queued at the same address, this should
not happen:
461.901175: xhci_urb_enqueue: ep3out-isoc: urb ecec2600 pipe 100096 slot 8
length 0/51 sgs 0/0 stream 0 flags 00010002
461.901180: xhci_queue_trb: ISOC: Buffer 2d9fa805 length 17 TD size 0
intr 0 type 'Isoch' flags b:i:I:c:s:i:e:c
461.901181: xhci_inc_enq: ISOC efa4e580: enq
0x33386010(0x33386000) deq
0x33386000(0x33386000)
So something goes really wrong when allocating or setting up the rings in one
of these functions:
xhci_ring_alloc()
xhci_alloc_segments_for_ring()
xhci_initialize_ring_info()
xhci_segment_alloc()
xhci_link_segments()
dma_pool_zalloc()
To verify and rule out dma_pool_zalloc(), could you apply the attached patch
and reproduce with new logs?
Thanks
-Mathias
>From 7aee4db28204fddff6cbc1534b8d50f13fd0b141 Mon Sep 17 00:00:00 2001
From: Mathias Nyman
Date: Thu, 24 May 2018 15:37:41 +0300
Subject: [PATCH] xhci: testpatch, add custom trace for ring segment alloc
for custom debugging only
---
drivers/usb/host/xhci-mem.c | 10 ++
drivers/usb/host/xhci-trace.h | 5 +
2 files changed, 15 insertions(+)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index e5ace89..7d343ad 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -44,10 +44,15 @@ static struct xhci_segment *xhci_segment_alloc(struct xhci_hcd *xhci,
return NULL;
}
+ xhci_dbg_trace(xhci, trace_xhci_ring_mem_detail,
+ "MATTU xhci_segment_alloc dma @ %pad", &dma);
+
if (max_packet) {
seg->bounce_buf = kzalloc(max_packet, flags);
if (!seg->bounce_buf) {
dma_pool_free(xhci->segment_pool, seg->trbs, dma);
+ xhci_dbg_trace(xhci, trace_xhci_ring_mem_detail,
+ "MATTU xhci segment free dma @ %pad", &dma);
kfree(seg);
return NULL;
}
@@ -58,6 +63,9 @@ static struct xhci_segment *xhci_segment_alloc(struct xhci_hcd *xhci,
seg->trbs[i].link.control |= cpu_to_le32(TRB_CYCLE);
}
seg->dma = dma;
+ xhci_dbg_trace(xhci, trace_xhci_ring_mem_detail,
+ "MATTU xhci segment alloc seg->dma @ %pad", &seg->dma);
+
seg->next = NULL;
return seg;
@@ -67,6 +75,8 @@ static void xhci_segment_free(struct xhci_hcd *xhci, struct xhci_segment *seg)
{
if (seg->trbs) {
dma_pool_free(xhci->segment_pool, seg->trbs, seg->dma);
+ xhci_dbg_trace(xhci, trace_xhci_ring_mem_detail,
+ "MATTU xhci segment free seg->dma @ %p", &seg->dma);
seg->trbs = NULL;
}
kfree(seg->bounce_buf);
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 35bdd06..951e371 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -72,6 +72,11 @@ DEFINE_EVENT(xhci_log_msg, xhci_dbg_ring_expansion,
TP_ARGS(vaf)
);
+DEFINE_EVENT(xhci_log_msg, xhci_ring_mem_detail,
+ TP_PROTO(struct va_format *vaf),
+ TP_ARGS(vaf)
+);
+
DECLARE_EVENT_CLASS(xhci_log_ctx,
TP_PROTO(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx,
unsi
Re: [RFC] driver core: don't hold dev's parent lock when using async probe
On Tue, May 22, 2018 at 01:09:44PM -0400, Alan Stern wrote:
> On Tue, 22 May 2018, martin_liu wrote:
>
> > not sure if we still need 'bf74ad5bc417 ("[PATCH] Hold the
> > device's parent's lock during probe and remove")' since it has
> > been there over 10 years. If we still need it and hard to fix it
> > , the simple way is to find a place not to allow USB subsystem
> > drivers to have async probe capability. Any suggestion is welcome.
>
> I don't think the "allows_async_probing" attribute is the best way to
> attack this. Some other approach, like a special-purpose flag, might
> be better.
>
> Yes, USB still needs to have parent's locks held during probing.
> Here's the reason. A USB device can have multiple interfaces, each
> bound to its own driver. A driver may sometimes need to issue a reset,
> but in USB there's no way to reset a single interface. Only the entire
> device can be reset, and of course this affects all the interfaces.
> Therefore a driver needs to acquire the device lock before it can issue
> a reset.
>
> The problem is that the driver's thread may already hold the device
> lock. During a normal probe sequence, for example, the interfaces get
> probed by the hub driver while it owns the device lock. But for probes
> under other circumstances (for example, if the user writes to the
> driver's "bind" attribute in sysfs), the device lock might not be held.
>
> A driver cannot tell these two cases apart. The only way to make it
> work all the time is to have the caller _always_ hold the device lock
> while the driver is probed (or the removed, for that matter).
>
> Alan Stern
Thanks for the reply and more detail about the backgroud. I'd like to
have a conclusion about it. Please kindly correct me if my understanding
is wrong. Regarding to the "special-purpose flag", do you mean we could
find a place in USB subsystem to have the flag set (not sure if it's
easy to find it). Driver core would be base on the flag to decide if we
need to hold the device's parent's lock.
Martin
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: dwc2: fix memory leak in gadget_init()
Freed allocated request for ep0 to prevent memory leak in case when
dwc2_driver_probe() failed.
Signed-off-by: Grigor Tovmasyan
Cc: Stefan Wahren
Cc: Marek Szyprowski
---
drivers/usb/dwc2/gadget.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index f0d9ccf1d665..7ccf56da1e50 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -4739,9 +4739,11 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg)
}
ret = usb_add_gadget_udc(dev, &hsotg->gadget);
- if (ret)
+ if (ret) {
+ dwc2_hsotg_ep_free_request(&hsotg->eps_out[0]->ep,
+ hsotg->ctrl_req);
return ret;
-
+ }
dwc2_hsotg_dump(hsotg);
return 0;
@@ -4755,6 +4757,7 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg)
int dwc2_hsotg_remove(struct dwc2_hsotg *hsotg)
{
usb_del_gadget_udc(&hsotg->gadget);
+ dwc2_hsotg_ep_free_request(&hsotg->eps_out[0]->ep, hsotg->ctrl_req);
return 0;
}
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: dwc2: fix memory leak in gadget_init()
Hi Stefan, Marek
Please test this patch and tell if it will cause any problem.
Thanks,
Grigor
On 5/24/2018 6:22 PM, Grigor Tovmasyan wrote:
> Freed allocated request for ep0 to prevent memory leak in case when
> dwc2_driver_probe() failed.
>
> Signed-off-by: Grigor Tovmasyan
> Cc: Stefan Wahren
> Cc: Marek Szyprowski
> ---
> drivers/usb/dwc2/gadget.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index f0d9ccf1d665..7ccf56da1e50 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -4739,9 +4739,11 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg)
> }
>
> ret = usb_add_gadget_udc(dev, &hsotg->gadget);
> - if (ret)
> + if (ret) {
> + dwc2_hsotg_ep_free_request(&hsotg->eps_out[0]->ep,
> +hsotg->ctrl_req);
> return ret;
> -
> + }
> dwc2_hsotg_dump(hsotg);
>
> return 0;
> @@ -4755,6 +4757,7 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg)
> int dwc2_hsotg_remove(struct dwc2_hsotg *hsotg)
> {
> usb_del_gadget_udc(&hsotg->gadget);
> + dwc2_hsotg_ep_free_request(&hsotg->eps_out[0]->ep, hsotg->ctrl_req);
>
> return 0;
> }
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: hub: Per-port setting to use old enumeration scheme
On Thu, 24 May 2018, Nicolas Boichat wrote: > On Thu, May 24, 2018 at 12:39 AM, Greg Kroah-Hartman > wrote: > > On Wed, May 23, 2018 at 10:03:55AM -0400, Alan Stern wrote: > >> On Wed, 23 May 2018, Nicolas Boichat wrote: > >> > >> > The "old" enumeration scheme is considerably faster (it takes > >> > ~294ms instead of ~439ms to get the descriptor). > >> > > >> > It is currently only possible to use the old scheme globally > >> > (/sys/module/usbcore/parameters/old_scheme_first), which is not > >> > desirable as the new scheme was introduced to increase compatibility > >> > with more devices. > >> > > >> > However, in our case, we care about time-to-active for a specific > >> > USB device (which we make the firmware for), on a specific port > >> > (that is pogo-pin based: not a standard USB port). This new > >> > sysfs option makes it possible to use the old scheme on a single > >> > port only. > >> > > >> > Signed-off-by: Nicolas Boichat > >> > --- > >> > > >> > There are other "quirks" that we could add to reduce further > >> > enumeration time (e.g. reduce USB debounce time, reduce TRSTRCY > >> > to 10ms instead of 50ms as used currently), but the logic is quite > >> > similar, so it'd be good to have this reviewed first. > >> > >> I'm not opposed to the idea in principle, although I don't like your > >> implementation because it breaks the original old_scheme_first > >> parameter. > > I don't think it breaks the original parameter? I mean, > /sys/module/usbcore/parameters/old_scheme_first is still a global > default, while bit 0 of /sys/bus/usb/devices/x/y/z/quirks becomes a > port-specific override. Oops, sorry, my mistake. My email client wrapped the last line of use_new_scheme(), and as a result I missed the fact that old_scheme_first was getting or'ed into the expression. > >> How do you arrange to set the new quirk before the device is > >> discovered? > > > > Yeah, this last question is what I had when looking at this. Or does it > > not matter at first boot and only matters for wake-up? > > It does not matter on boot, we have plenty of time to enumerate the > device. We use USB (auto-)suspend and remote wake, so no > re-enumeration there either. It only matters on unplug/replug where > the device needs to be re-enumerated. > > Somewhere in an init script, we would do this (we know in advance that > usb1 port2 is the bus/port where we have our pogo-pin USB interface, > so we can hard-code the path): > echo 1 > /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port2/quirks > > We could try to add ACPI support (just like connect_type), but we > don't strictly need it for our application. Okay, that makes sense. So this change looks okay to me. Acked-by: Alan Stern Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Fwd: usb: uas: device reset most the time while enumeration- usb3.0
Oliver, On 2018-05-17 19:04, Oliver Neukum wrote: Am Donnerstag, den 17.05.2018, 12:29 +0530 schrieb Tushar Nimkar: Those commands issued from different layers say: sd.. uas.. scsi.. so making them to go one after other. Once REPORT_LUN done go with READ_CAPACITY_16. This is only for the UAS devices. I believe no disturb to BOT behavior. Hi, this is good news. 1. We cannot slow down all UAS devices because a few are broken. This would need to be selective. 2. What is insufficient about "shost->async_scan" for your approach? Unfortunately for our build CONFIG_SCSI_SCAN_ASYNC wan not set. And enabling it solves our issue of enumeration. But as per bellow commit "If you have built SCSI as modules, enabling this option can be a problem as the devices may not have been found by the time your system expects them to have been." commit 21db1882f79a1ad5977cae6766376a63f60ec414 Author: Matthew Wilcox Date: Wed Nov 22 13:24:52 2006 -0700 [SCSI] Add Kconfig option for asynchronous SCSI scanning Without this patch, the user has to add a kernel command line parameter to get asynchronous SCSI scanning. Now they can select the default at compile time and still override it at boot time if they need to. Signed-off-by: Matthew Wilcox Signed-off-by: James Bottomley We have built SCSI as module will it cause any problem to enable CONFIG_SCSI_SCAN_ASYNC ? Regards Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- Best Regards, Tushar R Nimkar QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Query] checking hub port status while USB 2.0 port is resuming.
On Thu, 24 May 2018, Anshuman Gupta wrote:
> On Wed, May 23, 2018 at 09:55:40AM -0400, Alan Stern wrote:
> > On Wed, 23 May 2018, Anshuman Gupta wrote:
> >
> > > On Tue, May 22, 2018 at 11:54:19AM -0400, Alan Stern wrote:
> > > > On Tue, 22 May 2018, Anshuman Gupta wrote:
> > > >
> > > > > On Tue, May 22, 2018 at 09:53:09AM -0400, Alan Stern wrote:
> > > > > > On Tue, 22 May 2018, Anshuman Gupta wrote:
> > > > > >
> > > > > Thanks Alan for your quick response.
> > > > > > > Hi,
> > > > > > >
> > > > > > > When a xhci USB2.0 port is resuming and waiting for resume
> > > > > > > signaling, completion of
> > > > > > > USB_RESUME_TIMEOUT, it just reports the port status as
> > > > > > > USB_PORT_STAT_SUSPEND,
> > > > > > > and let the usbcore to check the port status again.
> > > > > > >
> > > > > > > https://elixir.bootlin.com/linux/v4.17-rc6/source/drivers/usb/host/xhci-hub.c#L960
> > > > > > >
> > > > > > >
> > > > > > > USB core just checks the port status in usb_port_resume function
> > > > > > > and if port status is in suspended state, it clears the port
> > > > > > > suspend feature.
> > > > > > >
> > > > > > > https://elixir.bootlin.com/linux/v4.17-rc6/source/drivers/usb/core/hub.c#L3441
> > > > > > >
> > > > > > > But when system resumes due to a USB remote wakeup event from
> > > > > > > suspend state,
> > > > > > > and port which causes remote wakeup is still waiting for resume
> > > > > > > signaling,
> > > > > > > usb_port_resume function in this particular case clears the port
> > > > > > > suspend feature,
> > > > > > > and wait for 40ms again(USB_RESUME_TIMEOUT).
> > > > > > >
> > > > > > > Query regarding above case:
> > > > > > > Here in this case USB core will miss the wake up event from the
> > > > > > > port which causes
> > > > > > > the remote wake.
> > > > > > > https://elixir.bootlin.com/linux/v4.17-rc6/source/drivers/usb/core/hub.c#L3444
> > > > > >
> > > > > > What do you mean? You said above: "system resumes due to a USB
> > > > > > remote
> > > > > > wakeup event". The fact that the system is resuming indicates that
> > > > > > it
> > > > > > did _not_ miss the wakeup event.
> > > > > >
> > > > > What was i meaning, if system come out of suspend due to a key
> > > > > pressed on usb keyboard.
> > > > > For the above mention case, it will not raise any pm_wakeup_event so
> > > > > the notification
> > > > > of the pm_wakeup_event for usb keyboard will get missed.
> > > >
> > > > Okay, let's say you're right. What difference does it make if the
> > > > pm_wakeup_event gets lost? Will that cause a problem?
> > > There is no as such problem for usb core but on chrome-os we want to
> > > increment
> > > the wakeup-count for the usb device which is causing wakeup from suspend.
> > >
> > > So to address above mention case, is it appropriate that hub_port_status
> > > URB completion
> > > can wait for port to be resume, if the particular port was resuming at
> > > that instant?
> >
> > I'm not sure what you're asking. If the port's suspend feature is set,
> > of course the kernel has to clear the feature and wait for the port to
> > resume. If the suspend feature isn't set then the pm_wakeup_event will
> > be sent, which is what you want.
> What i was looking for, report the pm_wakeup_event for the port, which is
> the source of
> remote wakeup, as below code snippet from usb_port_resume() does that.
>
>/* Skip the initial Clear-Suspend step for a remote wakeup */
> status = hub_port_status(hub, port1, &portstatus, &portchange);
> if (status == 0 && !port_is_suspended(hub, portstatus)) {
> if (portchange & USB_PORT_STAT_C_SUSPEND)
> pm_wakeup_event(&udev->dev, 0);
> goto SuspendCleared;
> }
>
> But in some cases as i mentioned above, if a port is resuming, it waits for
> 40ms to report
> change in port status and port change bits, In those cases usb_port_resume
> function
> will not increment wakeup-count and it will clear the suspend feature, so
> now we can not
> distinguish whethwer a port has resume due to remote wakeup or by clearing
> its suspend
> feature by usb_port_resume function.
>
> We want to address this issue, Is there any way so that hub_port_status can
> wait
> for port resume completion?
Let me make sure I understand. You want to slow down system resume of
xHCI root hubs by 40 ms in order to detect and report port wakeup
requests. Is that correct?
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] driver core: don't hold dev's parent lock when using async probe
On Thu, 24 May 2018, Martin Liu wrote:
> On Tue, May 22, 2018 at 01:09:44PM -0400, Alan Stern wrote:
> > On Tue, 22 May 2018, martin_liu wrote:
> >
> > > not sure if we still need 'bf74ad5bc417 ("[PATCH] Hold the
> > > device's parent's lock during probe and remove")' since it has
> > > been there over 10 years. If we still need it and hard to fix it
> > > , the simple way is to find a place not to allow USB subsystem
> > > drivers to have async probe capability. Any suggestion is welcome.
> >
> > I don't think the "allows_async_probing" attribute is the best way to
> > attack this. Some other approach, like a special-purpose flag, might
> > be better.
> >
> > Yes, USB still needs to have parent's locks held during probing.
> > Here's the reason. A USB device can have multiple interfaces, each
> > bound to its own driver. A driver may sometimes need to issue a reset,
> > but in USB there's no way to reset a single interface. Only the entire
> > device can be reset, and of course this affects all the interfaces.
> > Therefore a driver needs to acquire the device lock before it can issue
> > a reset.
> >
> > The problem is that the driver's thread may already hold the device
> > lock. During a normal probe sequence, for example, the interfaces get
> > probed by the hub driver while it owns the device lock. But for probes
> > under other circumstances (for example, if the user writes to the
> > driver's "bind" attribute in sysfs), the device lock might not be held.
> >
> > A driver cannot tell these two cases apart. The only way to make it
> > work all the time is to have the caller _always_ hold the device lock
> > while the driver is probed (or the removed, for that matter).
> >
> > Alan Stern
>
> Thanks for the reply and more detail about the backgroud. I'd like to
> have a conclusion about it. Please kindly correct me if my understanding
> is wrong. Regarding to the "special-purpose flag", do you mean we could
> find a place in USB subsystem to have the flag set (not sure if it's
> easy to find it). Driver core would be base on the flag to decide if we
> need to hold the device's parent's lock.
Yes, except that the flag would not be in the USB subsystem. It would
be in the device, device_type, or bus_type structure, so that the
driver core could access it.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
4.16 issue with mbim modem and ping with size > 14552 bytes
Hello, I have an issue with an USB mbim modem when trying to send with ping more than 14552 bytes: it looks like to me a kernel issue, but not at the cdc_mbim or cdc_ncm level, anyway not sure, so I'm reporting the issue. My kernel is 4.16. The device is the following: root@L2122:~# ifconfig wwp0s20u7i2 Link encap:Ethernet HWaddr be:3d:f2:f4:0d:e9 inet addr:2.193.7.73 Bcast:0.0.0.0 Mask:255.255.255.252 inet6 addr: fe80::bc3d:f2ff:fef4:de9/64 Scope:Link UP BROADCAST RUNNING NOARP MULTICAST MTU:1500 Metric:1 RX packets:5 errors:0 dropped:0 overruns:0 frame:0 TX packets:55 errors:0 dropped:0 overruns:0 carrier:0 collisions:0 txqueuelen:1000 RX bytes:759 (759.0 B) TX bytes:6275 (6.2 KB) Sending ping sized 14552 no issue: root@L2122:~# ping -s 14552 8.8.8.8 PING 8.8.8.8 (8.8.8.8) 14552(14580) bytes of data. 1448 bytes from 8.8.8.8: icmp_seq=1 ttl=52 (truncated) 1448 bytes from 8.8.8.8: icmp_seq=2 ttl=52 (truncated) 1448 bytes from 8.8.8.8: icmp_seq=3 ttl=52 (truncated) 1448 bytes from 8.8.8.8: icmp_seq=4 ttl=52 (truncated) 1448 bytes from 8.8.8.8: icmp_seq=5 ttl=52 (truncated) 1448 bytes from 8.8.8.8: icmp_seq=6 ttl=52 (truncated) 1448 bytes from 8.8.8.8: icmp_seq=7 ttl=52 (truncated) 1448 bytes from 8.8.8.8: icmp_seq=8 ttl=52 (truncated) ^C --- 8.8.8.8 ping statistics --- 8 packets transmitted, 8 received, 0% packet loss, time 7008ms rtt min/avg/max/mdev = 54.887/83.154/102.563/18.502 ms root@L2122:~# ifconfig wwp0s20u7i2 Link encap:Ethernet HWaddr be:3d:f2:f4:0d:e9 inet addr:2.193.7.73 Bcast:0.0.0.0 Mask:255.255.255.252 inet6 addr: fe80::bc3d:f2ff:fef4:de9/64 Scope:Link UP BROADCAST RUNNING NOARP MULTICAST MTU:1500 Metric:1 RX packets:15 errors:0 dropped:0 overruns:0 frame:0 TX packets:161 errors:0 dropped:0 overruns:0 carrier:0 collisions:0 txqueuelen:1000 RX bytes:12583 (12.5 KB) TX bytes:125999 (125.9 KB) If I try ping sized 14554, it does not work root@L2122:~# ping -s 14554 8.8.8.8 PING 8.8.8.8 (8.8.8.8) 14554(14582) bytes of data. ^C --- 8.8.8.8 ping statistics --- 7 packets transmitted, 0 received, 100% packet loss, time 6122ms and I see tx errors in the network interface root@L2122:~# ifconfig wwp0s20u7i2 Link encap:Ethernet HWaddr be:3d:f2:f4:0d:e9 inet addr:2.193.7.73 Bcast:0.0.0.0 Mask:255.255.255.252 inet6 addr: fe80::bc3d:f2ff:fef4:de9/64 Scope:Link UP BROADCAST RUNNING NOARP MULTICAST MTU:1500 Metric:1 RX packets:20 errors:0 dropped:0 overruns:0 frame:0 TX packets:190 errors:5 dropped:0 overruns:0 carrier:0 collisions:0 txqueuelen:1000 RX bytes:12943 (12.9 KB) TX bytes:142476 (142.4 KB) but the real problem is that the network interface seems not to be working anymore: root@L2122:~# ping 8.8.8.8 PING 8.8.8.8 (8.8.8.8) 56(84) bytes of data. ^C --- 8.8.8.8 ping statistics --- 10 packets transmitted, 0 received, 100% packet loss, time 9193ms root@L2122:~# root@L2122:~# ifconfig wwp0s20u7i2 Link encap:Ethernet HWaddr be:3d:f2:f4:0d:e9 inet addr:2.193.7.73 Bcast:0.0.0.0 Mask:255.255.255.252 inet6 addr: fe80::bc3d:f2ff:fef4:de9/64 Scope:Link UP BROADCAST RUNNING NOARP MULTICAST MTU:1500 Metric:1 RX packets:20 errors:0 dropped:0 overruns:0 frame:0 TX packets:190 errors:20 dropped:0 overruns:0 carrier:0 collisions:0 txqueuelen:1000 RX bytes:12943 (12.9 KB) TX bytes:142476 (142.4 KB) Nothing relevant in the kernel log. Anyone can suggest me how to debug this further? Thanks in advance, Daniele -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [GIT PULL] USB for v4.18 merge window
On Mon, May 21, 2018 at 11:25:09AM +0300, Felipe Balbi wrote: > > Hi Greg, > > Here's my pull request for v4.18 merge window. Let me know if you want > anything to be changed. > > For the rest of the week I'll have very scarce access to email, so > replies may be delayed. > > Cheers > > The following changes since commit 6d08b06e67cd117f6992c46611dfb4ce267cd71e: > > Linux 4.17-rc2 (2018-04-22 19:20:09 -0700) > > are available in the Git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git > tags/usb-for-v4.18 There was a conflict in drivers/usb/mtu3/Kconfig, but I think I fixed it up properly. Otherwise all looks good, pulled and pushed out, thanks. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 4.16 issue with mbim modem and ping with size > 14552 bytes
On Thu, May 24, 2018 at 05:04:49PM +0200, Daniele Palmas wrote: > Hello, > > I have an issue with an USB mbim modem when trying to send with ping > more than 14552 bytes: it looks like to me a kernel issue, but not at > the cdc_mbim or cdc_ncm level, anyway not sure, so I'm reporting the > issue. > > My kernel is 4.16. The device is the following: Does older kernels work, or is this something that has always been there? I ask, as my mobile provider does horrible things to large packet sizes. So much so that I have to set the mtu to 1280 just to get things to work properly when tethering my phone through to my laptop. So this might be a network provider issue :) thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] driver core: don't hold dev's parent lock when using async probe
On Thu, May 24, 2018 at 11:02:57AM -0400, Alan Stern wrote:
> On Thu, 24 May 2018, Martin Liu wrote:
>
> > On Tue, May 22, 2018 at 01:09:44PM -0400, Alan Stern wrote:
> > > On Tue, 22 May 2018, martin_liu wrote:
> > >
> > > > not sure if we still need 'bf74ad5bc417 ("[PATCH] Hold the
> > > > device's parent's lock during probe and remove")' since it has
> > > > been there over 10 years. If we still need it and hard to fix it
> > > > , the simple way is to find a place not to allow USB subsystem
> > > > drivers to have async probe capability. Any suggestion is welcome.
> > >
> > > I don't think the "allows_async_probing" attribute is the best way to
> > > attack this. Some other approach, like a special-purpose flag, might
> > > be better.
> > >
> > > Yes, USB still needs to have parent's locks held during probing.
> > > Here's the reason. A USB device can have multiple interfaces, each
> > > bound to its own driver. A driver may sometimes need to issue a reset,
> > > but in USB there's no way to reset a single interface. Only the entire
> > > device can be reset, and of course this affects all the interfaces.
> > > Therefore a driver needs to acquire the device lock before it can issue
> > > a reset.
> > >
> > > The problem is that the driver's thread may already hold the device
> > > lock. During a normal probe sequence, for example, the interfaces get
> > > probed by the hub driver while it owns the device lock. But for probes
> > > under other circumstances (for example, if the user writes to the
> > > driver's "bind" attribute in sysfs), the device lock might not be held.
> > >
> > > A driver cannot tell these two cases apart. The only way to make it
> > > work all the time is to have the caller _always_ hold the device lock
> > > while the driver is probed (or the removed, for that matter).
> > >
> > > Alan Stern
> >
> > Thanks for the reply and more detail about the backgroud. I'd like to
> > have a conclusion about it. Please kindly correct me if my understanding
> > is wrong. Regarding to the "special-purpose flag", do you mean we could
> > find a place in USB subsystem to have the flag set (not sure if it's
> > easy to find it). Driver core would be base on the flag to decide if we
> > need to hold the device's parent's lock.
>
> Yes, except that the flag would not be in the USB subsystem. It would
> be in the device, device_type, or bus_type structure, so that the
> driver core could access it.
>
> Alan Stern
Thanks for the quick feedback and the suggestion. will try to figure out how
it works.
Martin
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] usb: gadget: uvc: Expose configuration name through video node
From: Kieran Bingham
When utilising multiple instantiations of a UVC gadget on a composite
device, there is no clear method to link a particular configuration to
its respective video node.
Provide a means for identifying the correct video node by exposing the
name of the function configuration through sysfs.
Signed-off-by: Kieran Bingham
---
v2:
- Fix commit title (f_uvc -> uvc)
- Change identifier file name (now function_name)
- Document in ABI
.../ABI/testing/configfs-usb-gadget-uvc | 5
drivers/usb/gadget/function/f_uvc.c | 24 ++-
2 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc
b/Documentation/ABI/testing/configfs-usb-gadget-uvc
index 1ba0d0fda9c0..9281e2aa38df 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget-uvc
+++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc
@@ -263,3 +263,8 @@ Description:Specific streaming header descriptors
is connected
bmInfo - capabilities of this video streaming
interface
+
+What:
/sys/class/udc/udc.name/device/gadget/video4linux/video.name/function_name
+Date: May 2018
+KernelVersion: 4.19
+Description: UVC configfs function instance name
diff --git a/drivers/usb/gadget/function/f_uvc.c
b/drivers/usb/gadget/function/f_uvc.c
index d82cd61676d3..c8627cc698f8 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -421,10 +421,21 @@ uvc_function_disconnect(struct uvc_device *uvc)
* USB probe and disconnect
*/
+static ssize_t function_name_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct uvc_device *uvc = dev_get_drvdata(dev);
+
+ return sprintf(buf, "%s\n", uvc->func.fi->group.cg_item.ci_name);
+}
+
+static DEVICE_ATTR_RO(function_name);
+
static int
uvc_register_video(struct uvc_device *uvc)
{
struct usb_composite_dev *cdev = uvc->func.config->cdev;
+ int ret;
/* TODO reference counting. */
uvc->vdev.v4l2_dev = &uvc->v4l2_dev;
@@ -437,7 +448,17 @@ uvc_register_video(struct uvc_device *uvc)
video_set_drvdata(&uvc->vdev, uvc);
- return video_register_device(&uvc->vdev, VFL_TYPE_GRABBER, -1);
+ ret = video_register_device(&uvc->vdev, VFL_TYPE_GRABBER, -1);
+ if (ret < 0)
+ return ret;
+
+ ret = device_create_file(&uvc->vdev.dev, &dev_attr_function_name);
+ if (ret < 0) {
+ video_unregister_device(&uvc->vdev);
+ return ret;
+ }
+
+ return 0;
}
#define UVC_COPY_DESCRIPTOR(mem, dst, desc) \
@@ -877,6 +898,7 @@ static void uvc_unbind(struct usb_configuration *c, struct
usb_function *f)
INFO(cdev, "%s\n", __func__);
+ device_remove_file(&uvc->vdev.dev, &dev_attr_function_name);
video_unregister_device(&uvc->vdev);
v4l2_device_unregister(&uvc->v4l2_dev);
--
2.17.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: hub: Per-port setting to use old enumeration scheme
On Thu, May 24, 2018 at 07:42:00AM +0800, Nicolas Boichat wrote: > On Thu, May 24, 2018 at 12:39 AM, Greg Kroah-Hartman > wrote: > > On Wed, May 23, 2018 at 10:03:55AM -0400, Alan Stern wrote: > >> On Wed, 23 May 2018, Nicolas Boichat wrote: > >> > >> > The "old" enumeration scheme is considerably faster (it takes > >> > ~294ms instead of ~439ms to get the descriptor). > >> > > >> > It is currently only possible to use the old scheme globally > >> > (/sys/module/usbcore/parameters/old_scheme_first), which is not > >> > desirable as the new scheme was introduced to increase compatibility > >> > with more devices. > >> > > >> > However, in our case, we care about time-to-active for a specific > >> > USB device (which we make the firmware for), on a specific port > >> > (that is pogo-pin based: not a standard USB port). This new > >> > sysfs option makes it possible to use the old scheme on a single > >> > port only. > >> > > >> > Signed-off-by: Nicolas Boichat > >> > --- > >> > > >> > There are other "quirks" that we could add to reduce further > >> > enumeration time (e.g. reduce USB debounce time, reduce TRSTRCY > >> > to 10ms instead of 50ms as used currently), but the logic is quite > >> > similar, so it'd be good to have this reviewed first. > >> > >> I'm not opposed to the idea in principle, although I don't like your > >> implementation because it breaks the original old_scheme_first > >> parameter. > > I don't think it breaks the original parameter? I mean, > /sys/module/usbcore/parameters/old_scheme_first is still a global > default, while bit 0 of /sys/bus/usb/devices/x/y/z/quirks becomes a > port-specific override. > > >> Let's see what some other people think. > >> > >> Yours is a rather special case, because you know exactly what device > >> will be attached to a specific port. Still, I can see that sort of > >> thing happening in constrained and special-purpose settings. > >> > >> How do you arrange to set the new quirk before the device is > >> discovered? > > > > Yeah, this last question is what I had when looking at this. Or does it > > not matter at first boot and only matters for wake-up? > > It does not matter on boot, we have plenty of time to enumerate the > device. We use USB (auto-)suspend and remote wake, so no > re-enumeration there either. It only matters on unplug/replug where > the device needs to be re-enumerated. How does this device get unplugged/replugged if it is connected directly to the device? > Somewhere in an init script, we would do this (we know in advance that > usb1 port2 is the bus/port where we have our pogo-pin USB interface, > so we can hard-code the path): > echo 1 > /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port2/quirks > > We could try to add ACPI support (just like connect_type), but we > don't strictly need it for our application. Isn't there an "internal" ACPI flag for USB ports, or is that what connect_type is? Why wouldn't that work here instead? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Serdev: USB device and sysdev probing ala i2c
On Thu, May 24, 2018 at 7:18 AM, Ricardo Ribalda Delgado wrote: > Hi Johan, > > On Thu, May 24, 2018 at 2:07 PM Johan Hovold wrote: > >> Hi Ricardo, > >> On Wed, May 23, 2018 at 11:17:20AM +0200, Ricardo Ribalda Delgado wrote: >> > Hi >> > >> > I have a flash controller connected to the main computer via a usb to >> > serial. My plan is to expose it to the system as a video4linux > subdevice. >> > >> > With the inclusion of serdev I was expecting that it would be as easy as >> > adding a i2c device, but seems that there are some functionality that > it is >> > still not implemented: >> > >> > 1) Serdev for usb serial devices. > >> Right, I didn't want to enable serdev for USB serial before we have >> determined how to handle hotplugging (e.g. in serdev core or by making >> sure every serdev driver can handle devices going away at any time) in >> order to avoid having things crash left and right. > >> I have out-of-tree code for USB serial that I use for testing purposes, >> so it's mostly a matter of finding the time to think this through. > > Could you share those patches? I would love to test them. > In my setup the system does not support hotplugg and/or power saving. > > >> > 2) Instatiating via sysfs. Something like >> > echo hci_nokia > /sys/bus/serio/devices/serio0/new_device >> > (inspired in: echo eeprom 0x50 > /sys/bus/i2c/devices/i2c-3/new_device) > >> Serdev currently only supports device tree and ACPI. Using out-of-tree >> code, you could load a device tree fragment during runtime to describe >> your serial bus (or you just amend the device tree). > >> Using device tree overlays would have the benefit of being able to >> describe associated resources (e.g. reset gpios) which a simple >> compatible string (or equivalent) would not. > >> But there are examples where a simple compatible string would do, for >> example an existing CEC device presenting itself as a generic USB CDC >> device (hopefully with a dedicated VID/PID so that no user-space >> configuration is needed at all). > > What about platform devices? Can it be instatiated like that? Platform devices generally mean memory mapped devices. How would that work thru serial? I guess you could have some serial to mmio bridge exposed thru regmap. The fundamental problem here is you need a parent device node to apply a DT overlay to and a USB device hotplugged has no DT device node. The system you are running on may not even have a DT (like a PC). If you have an overlay of the downstream devices, they have to be a child of something for the overlay to apply to. We could just create virtual device nodes for the purposes of applying overlays to. Another option would be allowing multiple DTs. Then you aren't even using overlays (what's the point of an overlay when a system has no real DT to begin with). That also would mean they are completely independent from any real DT or other instances (you may want to plug in multiple of the same device). Rob -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Serdev: USB device and sysdev probing ala i2c
Hi Rob
On Thu, May 24, 2018 at 6:49 PM Rob Herring wrote:
> On Thu, May 24, 2018 at 7:18 AM, Ricardo Ribalda Delgado
> wrote:
> > Hi Johan,
> >
> > On Thu, May 24, 2018 at 2:07 PM Johan Hovold wrote:
> >
> >> Hi Ricardo,
> >
> >> On Wed, May 23, 2018 at 11:17:20AM +0200, Ricardo Ribalda Delgado
wrote:
> >> > Hi
> >> >
> >> > I have a flash controller connected to the main computer via a usb to
> >> > serial. My plan is to expose it to the system as a video4linux
> > subdevice.
> >> >
> >> > With the inclusion of serdev I was expecting that it would be as
easy as
> >> > adding a i2c device, but seems that there are some functionality that
> > it is
> >> > still not implemented:
> >> >
> >> > 1) Serdev for usb serial devices.
> >
> >> Right, I didn't want to enable serdev for USB serial before we have
> >> determined how to handle hotplugging (e.g. in serdev core or by making
> >> sure every serdev driver can handle devices going away at any time) in
> >> order to avoid having things crash left and right.
> >
> >> I have out-of-tree code for USB serial that I use for testing purposes,
> >> so it's mostly a matter of finding the time to think this through.
> >
> > Could you share those patches? I would love to test them.
> > In my setup the system does not support hotplugg and/or power saving.
> >
> >
> >> > 2) Instatiating via sysfs. Something like
> >> > echo hci_nokia > /sys/bus/serio/devices/serio0/new_device
> >> > (inspired in: echo eeprom 0x50 >
/sys/bus/i2c/devices/i2c-3/new_device)
> >
> >> Serdev currently only supports device tree and ACPI. Using out-of-tree
> >> code, you could load a device tree fragment during runtime to describe
> >> your serial bus (or you just amend the device tree).
> >
> >> Using device tree overlays would have the benefit of being able to
> >> describe associated resources (e.g. reset gpios) which a simple
> >> compatible string (or equivalent) would not.
> >
> >> But there are examples where a simple compatible string would do, for
> >> example an existing CEC device presenting itself as a generic USB CDC
> >> device (hopefully with a dedicated VID/PID so that no user-space
> >> configuration is needed at all).
> >
> > What about platform devices? Can it be instatiated like that?
> Platform devices generally mean memory mapped devices. How would that
> work thru serial? I guess you could have some serial to mmio bridge
> exposed thru regmap.
Actually I was thinking about a module that is autoloaded based on a acpi
string and then creates the serial devices with something like:
static struct serio_board_info ledctrl = {
.modalias = "serio_ledctrl",
};
serio_adap = serio_get_adapter(0);
if (!serio_adap){
printk(KERN_ERR "Could not find serio bus.");
return -EIO;
}
serio_client=serio_new_device(serio_adap, ledctrl);
Yes, there would be one driver per board, and that does not scale on the
kernel tree, but for out of tree is really helpful (and maybe also for
notebooks that already have their own platform driver on the tree)
> The fundamental problem here is you need a parent device node to apply
> a DT overlay to and a USB device hotplugged has no DT device node. The
> system you are running on may not even have a DT (like a PC). If you
> have an overlay of the downstream devices, they have to be a child of
> something for the overlay to apply to. We could just create virtual
> device nodes for the purposes of applying overlays to. Another option
> would be allowing multiple DTs. Then you aren't even using overlays
> (what's the point of an overlay when a system has no real DT to begin
> with). That also would mean they are completely independent from any
> real DT or other instances (you may want to plug in multiple of the
> same device).
In my usecase the DT is way too overkilled. The previously mentioned
echo tty > /sys/bus/serio/devices/serio0/unregister
echo hci_nokia > /sys/bus/serio/devices/serio0/new_device
would be the most convenient way to use/experiment with serio.
I am not taking into consideration hot pluggable devices. I am thinking
about a use case where the usb2serial is soldered in the board.
> Rob
--
Ricardo Ribalda
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/4] usb: roles: intel_xhci: Enable runtime PM
Hi,
On 05/24/2018 01:18 AM, Heikki Krogerus wrote:
This fixes an issue where the mux does not get configured
when the parent device is suspended. The registers for this
mux are mapped to the parent device MMIO (usually xHCI PCI
device), so in order for the driver to be able to program
the registers, the parent device must be resumed.
Reported-by: Sathyanarayanan Kuppuswamy
Fixes: f6fb9ec02be1 ("usb: roles: Add Intel xHCI USB role switch driver")
Signed-off-by: Heikki Krogerus
Tested-by: Kuppuswamy Sathyanarayanan
---
drivers/usb/roles/intel-xhci-usb-role-switch.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/roles/intel-xhci-usb-role-switch.c
b/drivers/usb/roles/intel-xhci-usb-role-switch.c
index 6e922b50b674..1fb3dd0f1dfa 100644
--- a/drivers/usb/roles/intel-xhci-usb-role-switch.c
+++ b/drivers/usb/roles/intel-xhci-usb-role-switch.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
#include
/* register definition */
@@ -56,6 +57,8 @@ static int intel_xhci_usb_set_role(struct device *dev, enum
usb_role role)
return -EIO;
}
+ pm_runtime_get_sync(dev);
+
/* Set idpin value as requested */
val = readl(data->base + DUAL_ROLE_CFG0);
switch (role) {
@@ -84,13 +87,17 @@ static int intel_xhci_usb_set_role(struct device *dev, enum
usb_role role)
/* Polling on CFG1 register to confirm mode switch.*/
do {
val = readl(data->base + DUAL_ROLE_CFG1);
- if (!!(val & HOST_MODE) == (role == USB_ROLE_HOST))
+ if (!!(val & HOST_MODE) == (role == USB_ROLE_HOST)) {
+ pm_runtime_put(dev);
return 0;
+ }
/* Interval for polling is set to about 5 - 10 ms */
usleep_range(5000, 1);
} while (time_before(jiffies, timeout));
+ pm_runtime_put(dev);
+
dev_warn(dev, "Timeout waiting for role-switch\n");
return -ETIMEDOUT;
}
@@ -101,7 +108,9 @@ static enum usb_role intel_xhci_usb_get_role(struct device
*dev)
enum usb_role role;
u32 val;
+ pm_runtime_get_sync(dev);
val = readl(data->base + DUAL_ROLE_CFG0);
+ pm_runtime_put(dev);
if (!(val & SW_IDPIN))
role = USB_ROLE_HOST;
@@ -142,6 +151,9 @@ static int intel_xhci_usb_probe(struct platform_device
*pdev)
if (IS_ERR(data->role_sw))
return PTR_ERR(data->role_sw);
+ pm_runtime_set_active(dev);
+ pm_runtime_enable(dev);
+
return 0;
}
--
Sathyanarayanan Kuppuswamy
Linux kernel developer
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: gadget: uvc: Expose configuration name through video node
Hi Kieran,
Thank you for the patch.
On Thursday, 24 May 2018 19:16:12 EEST Kieran Bingham wrote:
> From: Kieran Bingham
>
> When utilising multiple instantiations of a UVC gadget on a composite
> device, there is no clear method to link a particular configuration to
> its respective video node.
>
> Provide a means for identifying the correct video node by exposing the
> name of the function configuration through sysfs.
>
> Signed-off-by: Kieran Bingham
Reviewed-by: Laurent Pinchart
> ---
> v2:
> - Fix commit title (f_uvc -> uvc)
> - Change identifier file name (now function_name)
> - Document in ABI
>
> .../ABI/testing/configfs-usb-gadget-uvc | 5
> drivers/usb/gadget/function/f_uvc.c | 24 ++-
> 2 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uvc
> b/Documentation/ABI/testing/configfs-usb-gadget-uvc index
> 1ba0d0fda9c0..9281e2aa38df 100644
> --- a/Documentation/ABI/testing/configfs-usb-gadget-uvc
> +++ b/Documentation/ABI/testing/configfs-usb-gadget-uvc
> @@ -263,3 +263,8 @@ Description: Specific streaming header descriptors
> is connected
> bmInfo - capabilities of this video streaming
> interface
> +
> +What:
> /sys/class/udc/udc.name/device/gadget/video4linux/video.name/
functio
> n_name +Date: May 2018
> +KernelVersion: 4.19
> +Description: UVC configfs function instance name
> diff --git a/drivers/usb/gadget/function/f_uvc.c
> b/drivers/usb/gadget/function/f_uvc.c index d82cd61676d3..c8627cc698f8
> 100644
> --- a/drivers/usb/gadget/function/f_uvc.c
> +++ b/drivers/usb/gadget/function/f_uvc.c
> @@ -421,10 +421,21 @@ uvc_function_disconnect(struct uvc_device *uvc)
> * USB probe and disconnect
> */
>
> +static ssize_t function_name_show(struct device *dev,
> + struct device_attribute *attr, char *buf)
> +{
> + struct uvc_device *uvc = dev_get_drvdata(dev);
> +
> + return sprintf(buf, "%s\n", uvc->func.fi->group.cg_item.ci_name);
> +}
> +
> +static DEVICE_ATTR_RO(function_name);
> +
> static int
> uvc_register_video(struct uvc_device *uvc)
> {
> struct usb_composite_dev *cdev = uvc->func.config->cdev;
> + int ret;
>
> /* TODO reference counting. */
> uvc->vdev.v4l2_dev = &uvc->v4l2_dev;
> @@ -437,7 +448,17 @@ uvc_register_video(struct uvc_device *uvc)
>
> video_set_drvdata(&uvc->vdev, uvc);
>
> - return video_register_device(&uvc->vdev, VFL_TYPE_GRABBER, -1);
> + ret = video_register_device(&uvc->vdev, VFL_TYPE_GRABBER, -1);
> + if (ret < 0)
> + return ret;
> +
> + ret = device_create_file(&uvc->vdev.dev, &dev_attr_function_name);
> + if (ret < 0) {
> + video_unregister_device(&uvc->vdev);
> + return ret;
> + }
> +
> + return 0;
> }
>
> #define UVC_COPY_DESCRIPTOR(mem, dst, desc) \
> @@ -877,6 +898,7 @@ static void uvc_unbind(struct usb_configuration *c,
> struct usb_function *f)
>
> INFO(cdev, "%s\n", __func__);
>
> + device_remove_file(&uvc->vdev.dev, &dev_attr_function_name);
> video_unregister_device(&uvc->vdev);
> v4l2_device_unregister(&uvc->v4l2_dev);
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFT/PATCH 00/38] usb: dwc3: gadget: Rework & Refactoring
Hi Felipe, On Friday, 20 April 2018 13:57:23 EEST Felipe Balbi wrote: > Bin Liu writes: > >> Felipe Balbi writes: > > Bin Liu writes: > >> On Mon, Apr 09, 2018 at 02:26:14PM +0300, Felipe Balbi wrote: > >>> Hi guys, > >>> > >>> I've been working on this series for a while now. I feels like > >>> after this series the transfer management code is far easier to > >>> read and understand. > >>> > >>> Based on my tests, I have no regressions. Tested g_mass_storage > >>> and all of testusb's tests (including ISOC). > >>> > >>> Patches are also available on dwc3-improve-isoc-endpoints in my > >>> k.org tree. Test reports would be VERY, VERY, VERY welcome. Please > >>> give this a go so we avoid regressions on v4.18. > >> > >> Have you tested g_webcam? I just tested your > >> dwc3-improve-isoc-endpoints > > > > for isoc I only tested g_zero. > > Then writting down details on what I observed probably won't help you > :( > >> > >> I got webcam running here, it sure _is_ helpful to let me know how you > > > > Great! > > > >> trigger the problem ;-) Also, high-speed or super-speed? > > > > Both. Long story short - super-speed doesn't stream the yuv video, > > gadget side kernel prints "VS request completed with status -18." > > flooding messages. > > > > high-speed does stream the video, but stopping the host application > > (both yavta and luvcview) causes gadget side kernel prints error message > > (I didn't keep the log). Then restart the host application doesn't > > stream the video any more. What is your test platform ? I'm using a TI AM574x EVM and I can't get video to stream at all in high-speed. Super-speed seems out of question as the only port supporting device mode on that board is connect to a DWC3 instance that is limited to high-speed :-/ I'm testing v4.17-rc5 without this patch series applied, please let me know if I should apply it first. > > To run the test: > > > > gadget side: > > # modprobe g_webcam (streaming_maxpacket=3072 for super-speed) > > # uvc-gadget > > > > host side: > > $ yavta -c -f YUYV -t 1/30 -s 640x360 -n4 /dev/video1, or > > $ luvcview -d /dev/video1 -f yuv > > okay, found the problem. This is actually a problem on webcam gadget > which kills the stream in case of Missed Isoc. The reason why it "works" > with dwc3 today is because dwc3, up until now, is really harsh whenever > we miss and interval. > > Currently we stop the transfer and wait for the next XferNotReady. This > may cause us to loose many frames. > > I've been talking with Laurent Pinchart (in Cc) about what would be the > best way to sort this out and, our conclusion, is that webcam gadget > should have a way for a single buffer to be treated as erroneous, but it > shouldn't stop the stream. > > There's also another error in webcam gadget where default interval, > maxburst and maxpacket aren't very good for HS or SS. > > Note that streaming_interval is, by default, 1. Which for FS means 1ms, > but for HS/SS it means 1 * 125us. So host side is actually polling > webcam gadget every uFrame. I got webcam gadget to behave really well > with streaming_interval=4 streaming_maxburst=16 > streaming_maxpacket=3072. With that, in SS, I can get around 100 > fps. There are a few cases when FPS drops to 33, but that's because it > coincides with a missed isoc and webcam stops and restarts the stream. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFT/PATCH 00/38] usb: dwc3: gadget: Rework & Refactoring
Hello again, On Thursday, 24 May 2018 22:59:18 EEST Laurent Pinchart wrote: > On Friday, 20 April 2018 13:57:23 EEST Felipe Balbi wrote: > > Bin Liu writes: > >>> Felipe Balbi writes: > >> Bin Liu writes: > >>> On Mon, Apr 09, 2018 at 02:26:14PM +0300, Felipe Balbi wrote: > Hi guys, > > I've been working on this series for a while now. I feels like > after this series the transfer management code is far easier to > read and understand. > > Based on my tests, I have no regressions. Tested g_mass_storage > and all of testusb's tests (including ISOC). > > Patches are also available on dwc3-improve-isoc-endpoints in my > k.org tree. Test reports would be VERY, VERY, VERY welcome. Please > give this a go so we avoid regressions on v4.18. > >>> > >>> Have you tested g_webcam? I just tested your > >>> dwc3-improve-isoc-endpoints > >> > >> for isoc I only tested g_zero. > > > > Then writting down details on what I observed probably won't help you > > > > :( > >>> > >>> I got webcam running here, it sure _is_ helpful to let me know how you > >> > >> Great! > >> > >>> trigger the problem ;-) Also, high-speed or super-speed? > >> > >> Both. Long story short - super-speed doesn't stream the yuv video, > >> gadget side kernel prints "VS request completed with status -18." > >> flooding messages. > >> > >> high-speed does stream the video, but stopping the host application > >> (both yavta and luvcview) causes gadget side kernel prints error message > >> (I didn't keep the log). Then restart the host application doesn't > >> stream the video any more. > > What is your test platform ? I'm using a TI AM574x EVM and I can't get video > to stream at all in high-speed. Super-speed seems out of question as the > only port supporting device mode on that board is connect to a DWC3 > instance that is limited to high-speed :-/ I'm testing v4.17-rc5 without > this patch series applied, please let me know if I should apply it first. I tried to merge your dwc3-improve-isoc-endpoints branch but it unfortunately didn't help. I still can't capture any frame on the host. With and without your patches I get the following messages in the device's kernel log. - At gadget creation and plug time: [ 12.361591] configfs-gadget gadget: uvc_function_bind [ 16.783801] configfs-gadget gadget: high-speed config #1: c [ 16.790032] configfs-gadget gadget: uvc_function_set_alt(0, 0) [ 16.796166] configfs-gadget gadget: reset UVC Control [ 16.801537] configfs-gadget gadget: uvc_function_set_alt(1, 0) [ 16.818799] configfs-gadget gadget: uvc_function_set_alt(1, 0) - At stream start time: [ 23.093891] configfs-gadget gadget: uvc_function_set_alt(1, 1) [ 23.103072] configfs-gadget gadget: reset UVC [ 23.08] dwc3 488d.usb: ep2in: ran out of requests and everything stops there. > >> To run the test: > >> > >> gadget side: > >># modprobe g_webcam (streaming_maxpacket=3072 for super-speed) > >># uvc-gadget > >> > >> host side: > >>$ yavta -c -f YUYV -t 1/30 -s 640x360 -n4 /dev/video1, or > >>$ luvcview -d /dev/video1 -f yuv > > > > okay, found the problem. This is actually a problem on webcam gadget > > which kills the stream in case of Missed Isoc. The reason why it "works" > > with dwc3 today is because dwc3, up until now, is really harsh whenever > > we miss and interval. > > > > Currently we stop the transfer and wait for the next XferNotReady. This > > may cause us to loose many frames. > > > > I've been talking with Laurent Pinchart (in Cc) about what would be the > > best way to sort this out and, our conclusion, is that webcam gadget > > should have a way for a single buffer to be treated as erroneous, but it > > shouldn't stop the stream. > > > > There's also another error in webcam gadget where default interval, > > maxburst and maxpacket aren't very good for HS or SS. > > > > Note that streaming_interval is, by default, 1. Which for FS means 1ms, > > but for HS/SS it means 1 * 125us. So host side is actually polling > > webcam gadget every uFrame. I got webcam gadget to behave really well > > with streaming_interval=4 streaming_maxburst=16 > > streaming_maxpacket=3072. With that, in SS, I can get around 100 > > fps. There are a few cases when FPS drops to 33, but that's because it > > coincides with a missed isoc and webcam stops and restarts the stream. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Query] checking hub port status while USB 2.0 port is resuming.
On Thu, 24 May 2018, Anshuman Gupta wrote:
> > > > I'm not sure what you're asking. If the port's suspend feature is set,
> > > > of course the kernel has to clear the feature and wait for the port to
> > > > resume. If the suspend feature isn't set then the pm_wakeup_event will
> > > > be sent, which is what you want.
> > > What i was looking for, report the pm_wakeup_event for the port, which
> > > is the source of
> > > remote wakeup, as below code snippet from usb_port_resume() does that.
> > >
> > >/* Skip the initial Clear-Suspend step for a remote wakeup */
> > > status = hub_port_status(hub, port1, &portstatus, &portchange);
> > > if (status == 0 && !port_is_suspended(hub, portstatus)) {
> > > if (portchange & USB_PORT_STAT_C_SUSPEND)
> > > pm_wakeup_event(&udev->dev, 0);
> > > goto SuspendCleared;
> > > }
> > >
> > > But in some cases as i mentioned above, if a port is resuming, it waits
> > > for 40ms to report
> > > change in port status and port change bits, In those cases
> > > usb_port_resume function
> > > will not increment wakeup-count and it will clear the suspend feature,
> > > so now we can not
> > > distinguish whethwer a port has resume due to remote wakeup or by
> > > clearing its suspend
> > > feature by usb_port_resume function.
> > >
> > > We want to address this issue, Is there any way so that hub_port_status
> > > can wait
> > > for port resume completion?
> >
> > Let me make sure I understand. You want to slow down system resume of
> > xHCI root hubs by 40 ms in order to detect and report port wakeup
> > requests. Is that correct?
> yes we want to detect and report wakeup event for above mention issue, but
> not in favour
> of adding 40ms delay. Please suggest if there is any better way of doing
> that.
It sounds like you want the code in usb_port_resume() to check and see
whether the port has already received a wakeup signal and hasn't
finished processing it yet.
There's no way to do this for ports on external hubs, but in principle
it can be done for ports on root hubs. If we see that a wakeup signal
was received, we can call pm_wakeup_event(). Will that be good enough?
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: hub: Per-port setting to use old enumeration scheme
On Fri, May 25, 2018 at 12:21 AM, Greg Kroah-Hartman wrote: > On Thu, May 24, 2018 at 07:42:00AM +0800, Nicolas Boichat wrote: >> On Thu, May 24, 2018 at 12:39 AM, Greg Kroah-Hartman >> wrote: >> > On Wed, May 23, 2018 at 10:03:55AM -0400, Alan Stern wrote: >> >> On Wed, 23 May 2018, Nicolas Boichat wrote: >> >> >> >> > The "old" enumeration scheme is considerably faster (it takes >> >> > ~294ms instead of ~439ms to get the descriptor). >> >> > >> >> > It is currently only possible to use the old scheme globally >> >> > (/sys/module/usbcore/parameters/old_scheme_first), which is not >> >> > desirable as the new scheme was introduced to increase compatibility >> >> > with more devices. >> >> > >> >> > However, in our case, we care about time-to-active for a specific >> >> > USB device (which we make the firmware for), on a specific port >> >> > (that is pogo-pin based: not a standard USB port). This new >> >> > sysfs option makes it possible to use the old scheme on a single >> >> > port only. >> >> > >> >> > Signed-off-by: Nicolas Boichat >> >> > --- >> >> > >> >> > There are other "quirks" that we could add to reduce further >> >> > enumeration time (e.g. reduce USB debounce time, reduce TRSTRCY >> >> > to 10ms instead of 50ms as used currently), but the logic is quite >> >> > similar, so it'd be good to have this reviewed first. >> >> >> >> I'm not opposed to the idea in principle, although I don't like your >> >> implementation because it breaks the original old_scheme_first >> >> parameter. >> >> I don't think it breaks the original parameter? I mean, >> /sys/module/usbcore/parameters/old_scheme_first is still a global >> default, while bit 0 of /sys/bus/usb/devices/x/y/z/quirks becomes a >> port-specific override. >> >> >> Let's see what some other people think. >> >> >> >> Yours is a rather special case, because you know exactly what device >> >> will be attached to a specific port. Still, I can see that sort of >> >> thing happening in constrained and special-purpose settings. >> >> >> >> How do you arrange to set the new quirk before the device is >> >> discovered? >> > >> > Yeah, this last question is what I had when looking at this. Or does it >> > not matter at first boot and only matters for wake-up? >> >> It does not matter on boot, we have plenty of time to enumerate the >> device. We use USB (auto-)suspend and remote wake, so no >> re-enumeration there either. It only matters on unplug/replug where >> the device needs to be re-enumerated. > > How does this device get unplugged/replugged if it is connected directly > to the device? It is external. Essentially, this is a tablet with a detachable keyboard/touchpad. The interface between tablet and base is USB, over pogo pins. The port is non-standard (pogo, not normal USB), and we fully control the firmware on the base side as well, which allows us to take shortcuts like this: we know exactly what device will be connected on that port. >> Somewhere in an init script, we would do this (we know in advance that >> usb1 port2 is the bus/port where we have our pogo-pin USB interface, >> so we can hard-code the path): >> echo 1 > /sys/bus/usb/devices/usb1/1-0:1.0/usb1-port2/quirks >> >> We could try to add ACPI support (just like connect_type), but we >> don't strictly need it for our application. > > Isn't there an "internal" ACPI flag for USB ports, or is that > what connect_type is? Why wouldn't that work here instead? What we may need here is something like "external but non-standard, so we can ignore compatibility constraints when enumerating"... Thanks, -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: 4.16 issue with mbim modem and ping with size > 14552 bytes
Hi Greg, 2018-05-24 17:53 GMT+02:00 Greg KH : > On Thu, May 24, 2018 at 05:04:49PM +0200, Daniele Palmas wrote: >> Hello, >> >> I have an issue with an USB mbim modem when trying to send with ping >> more than 14552 bytes: it looks like to me a kernel issue, but not at >> the cdc_mbim or cdc_ncm level, anyway not sure, so I'm reporting the >> issue. >> >> My kernel is 4.16. The device is the following: > > Does older kernels work, or is this something that has always been > there? > Not tested yet, I'm going to do. > I ask, as my mobile provider does horrible things to large packet sizes. > So much so that I have to set the mtu to 1280 just to get things to work > properly when tethering my phone through to my laptop. So this might be > a network provider issue :) > Yeah, I thought the same, so I tried the same scenario with Windows 10 but it is working fine. Thanks, Daniele > thanks, > > greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH/RFC v4 2/4] usb: gadget: udc: renesas_usb3: Add register of usb role switch
Hi Rob, Geert-san,
> From: Geert Uytterhoeven, Sent: Thursday, May 24, 2018 5:18 PM
>
> Hi Rob,
>
> On Wed, May 23, 2018 at 5:00 PM, Rob Herring wrote:
> >>> > Optional properties:
> >>> >- phys: phandle + phy specifier pair
> >>> >- phy-names: must be "usb"
> >>> > + - The connection to a usb3.0 host node needs by using OF graph
> >>> > bindings for
> >>> > +usb role switch.
> >>> > + - port@0 = USB3.0 host port.
> >>>
> >>> On the host side, this might conflict with the USB connector binding.
> >>>
> >>> I would either make sure this can work with the connector binding by
> >>> having 2 endpoints on the HS or SS port or just use the 'companion'
> >>> property defined in usb-generic.txt.
> >>
> >> I don't understand the first one now... This means the renesas_usb3 should
> >> follow
> >> USB connector binding and have 2 endpoints for the usb role switch to avoid
> >> the conflict like below?
> >> - port1@0: Super Speed (SS), present in SS capable connectors (from
> >> usb-connector.txt).
> >> - port1@1: USB3.0 host port.
> >
> > I'm confused, SS and USB3.0 are the same essentially. It would be:
> >
> > port@1/endpoint@0: SS host port
> > port@1/endpoint@1: SS device port
Thank you for the comment. It's better than my description.
> >> About the 'companion' in usb-generic.txt, the property intends to be used
> >> for EHCI or host side
> >> like the commit log [1]. If there is accept to use 'companion' for this
> >> patch, I think it will
> >> be simple to achieve this role switch feature. However, in last month, I
> >> submitted a similar patch [2]
> >> that has "renesas,host" property, but I got reply from Andy [3] and Heikki
> >> [4]. So, I'm
> >> trying to improve the device connection framework [5] now.
> >
> > I think this case is rare enough that we don't need a general solution
> > using OF graph, so I'm fine with a simple, single property to link the
> > 2 nodes. Either reusing "companion" or "renesas,host" is fine by me.
>
> I'd go for the standard "companion" over "renesas,host"[*].
>
> [*] Doh, we have another one ("renesas,bonding"), invented when I wasn't
> aware of the existence of "companion" yet...
Thank you for the comments. So, I'll reuse "companion" for it.
Best regards,
Yoshihiro Shimoda
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like
> that.
> -- Linus Torvalds
Re: [PATCH] usb: gadget: dwc2: fix memory leak in gadget_init()
Hi Grigor,
On 2018-05-24 16:22, Grigor Tovmasyan wrote:
> Freed allocated request for ep0 to prevent memory leak in case when
> dwc2_driver_probe() failed.
>
> Signed-off-by: Grigor Tovmasyan
> Cc: Stefan Wahren
> Cc: Marek Szyprowski
Works fine on my Exynos SoC based test boards.
Tested-by: Marek Szyprowski
> ---
> drivers/usb/dwc2/gadget.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index f0d9ccf1d665..7ccf56da1e50 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -4739,9 +4739,11 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg)
> }
>
> ret = usb_add_gadget_udc(dev, &hsotg->gadget);
> - if (ret)
> + if (ret) {
> + dwc2_hsotg_ep_free_request(&hsotg->eps_out[0]->ep,
> +hsotg->ctrl_req);
> return ret;
> -
> + }
> dwc2_hsotg_dump(hsotg);
>
> return 0;
> @@ -4755,6 +4757,7 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg)
> int dwc2_hsotg_remove(struct dwc2_hsotg *hsotg)
> {
> usb_del_gadget_udc(&hsotg->gadget);
> + dwc2_hsotg_ep_free_request(&hsotg->eps_out[0]->ep, hsotg->ctrl_req);
>
> return 0;
> }
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: dwc2: fix memory leak in gadget_init()
Hi Grigor,
> Grigor Tovmasyan hat am 24. Mai 2018 um 16:22
> geschrieben:
>
>
> Freed allocated request for ep0 to prevent memory leak in case when
> dwc2_driver_probe() failed.
>
> Signed-off-by: Grigor Tovmasyan
> Cc: Stefan Wahren
> Cc: Marek Szyprowski
currently i don't much time, but at least my tests with Raspberry Pi Zero W
were successful.
Tested-by: Stefan Wahren
It would be nice if you can setup some real testing devices at Synopsys in
order to identify such regressions faster. kernelci.org only tests the host
role of the Raspberry Pi.
Thanks
Stefan
> ---
> drivers/usb/dwc2/gadget.c | 7 +--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index f0d9ccf1d665..7ccf56da1e50 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -4739,9 +4739,11 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg)
> }
>
> ret = usb_add_gadget_udc(dev, &hsotg->gadget);
> - if (ret)
> + if (ret) {
> + dwc2_hsotg_ep_free_request(&hsotg->eps_out[0]->ep,
> +hsotg->ctrl_req);
> return ret;
> -
> + }
> dwc2_hsotg_dump(hsotg);
>
> return 0;
> @@ -4755,6 +4757,7 @@ int dwc2_gadget_init(struct dwc2_hsotg *hsotg)
> int dwc2_hsotg_remove(struct dwc2_hsotg *hsotg)
> {
> usb_del_gadget_udc(&hsotg->gadget);
> + dwc2_hsotg_ep_free_request(&hsotg->eps_out[0]->ep, hsotg->ctrl_req);
>
> return 0;
> }
> --
> 2.11.0
>
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: hub: Per-port setting to use old enumeration scheme
On Fri, May 25, 2018 at 06:05:16AM +0800, Nicolas Boichat wrote: > On Fri, May 25, 2018 at 12:21 AM, Greg Kroah-Hartman > wrote: > > On Thu, May 24, 2018 at 07:42:00AM +0800, Nicolas Boichat wrote: > >> On Thu, May 24, 2018 at 12:39 AM, Greg Kroah-Hartman > >> wrote: > >> > On Wed, May 23, 2018 at 10:03:55AM -0400, Alan Stern wrote: > >> >> On Wed, 23 May 2018, Nicolas Boichat wrote: > >> >> > >> >> > The "old" enumeration scheme is considerably faster (it takes > >> >> > ~294ms instead of ~439ms to get the descriptor). > >> >> > > >> >> > It is currently only possible to use the old scheme globally > >> >> > (/sys/module/usbcore/parameters/old_scheme_first), which is not > >> >> > desirable as the new scheme was introduced to increase compatibility > >> >> > with more devices. > >> >> > > >> >> > However, in our case, we care about time-to-active for a specific > >> >> > USB device (which we make the firmware for), on a specific port > >> >> > (that is pogo-pin based: not a standard USB port). This new > >> >> > sysfs option makes it possible to use the old scheme on a single > >> >> > port only. > >> >> > > >> >> > Signed-off-by: Nicolas Boichat > >> >> > --- > >> >> > > >> >> > There are other "quirks" that we could add to reduce further > >> >> > enumeration time (e.g. reduce USB debounce time, reduce TRSTRCY > >> >> > to 10ms instead of 50ms as used currently), but the logic is quite > >> >> > similar, so it'd be good to have this reviewed first. > >> >> > >> >> I'm not opposed to the idea in principle, although I don't like your > >> >> implementation because it breaks the original old_scheme_first > >> >> parameter. > >> > >> I don't think it breaks the original parameter? I mean, > >> /sys/module/usbcore/parameters/old_scheme_first is still a global > >> default, while bit 0 of /sys/bus/usb/devices/x/y/z/quirks becomes a > >> port-specific override. > >> > >> >> Let's see what some other people think. > >> >> > >> >> Yours is a rather special case, because you know exactly what device > >> >> will be attached to a specific port. Still, I can see that sort of > >> >> thing happening in constrained and special-purpose settings. > >> >> > >> >> How do you arrange to set the new quirk before the device is > >> >> discovered? > >> > > >> > Yeah, this last question is what I had when looking at this. Or does it > >> > not matter at first boot and only matters for wake-up? > >> > >> It does not matter on boot, we have plenty of time to enumerate the > >> device. We use USB (auto-)suspend and remote wake, so no > >> re-enumeration there either. It only matters on unplug/replug where > >> the device needs to be re-enumerated. > > > > How does this device get unplugged/replugged if it is connected directly > > to the device? > > It is external. Essentially, this is a tablet with a detachable > keyboard/touchpad. The interface between tablet and base is USB, over > pogo pins. The port is non-standard (pogo, not normal USB), and we > fully control the firmware on the base side as well, which allows us > to take shortcuts like this: we know exactly what device will be > connected on that port. Ah, ok, that makes more sense, thanks for the explanation. I'll go queue this up in a bit. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: hub: Per-port setting to use old enumeration scheme
On Wed, May 23, 2018 at 10:16:56AM +0800, Nicolas Boichat wrote:
> The "old" enumeration scheme is considerably faster (it takes
> ~294ms instead of ~439ms to get the descriptor).
>
> It is currently only possible to use the old scheme globally
> (/sys/module/usbcore/parameters/old_scheme_first), which is not
> desirable as the new scheme was introduced to increase compatibility
> with more devices.
>
> However, in our case, we care about time-to-active for a specific
> USB device (which we make the firmware for), on a specific port
> (that is pogo-pin based: not a standard USB port). This new
> sysfs option makes it possible to use the old scheme on a single
> port only.
>
> Signed-off-by: Nicolas Boichat
> ---
>
> There are other "quirks" that we could add to reduce further
> enumeration time (e.g. reduce USB debounce time, reduce TRSTRCY
> to 10ms instead of 50ms as used currently), but the logic is quite
> similar, so it'd be good to have this reviewed first.
>
> drivers/usb/core/hub.c | 13 +
> drivers/usb/core/hub.h | 1 +
> drivers/usb/core/port.c | 23 +++
> include/linux/usb.h | 7 +++
> 4 files changed, 40 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index c2d993d3816f0..f900f66a62856 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -2636,7 +2636,7 @@ static unsigned hub_is_wusb(struct usb_hub *hub)
> #define SET_ADDRESS_TRIES2
> #define GET_DESCRIPTOR_TRIES 2
> #define SET_CONFIG_TRIES (2 * (use_both_schemes + 1))
> -#define USE_NEW_SCHEME(i)((i) / 2 == (int)old_scheme_first)
> +#define USE_NEW_SCHEME(i, scheme)((i) / 2 == (int)scheme)
>
> #define HUB_ROOT_RESET_TIME 60 /* times are in msec */
> #define HUB_SHORT_RESET_TIME 10
> @@ -2651,12 +2651,16 @@ static unsigned hub_is_wusb(struct usb_hub *hub)
> * enumeration failures, so disable this enumeration scheme for USB3
> * devices.
> */
> -static bool use_new_scheme(struct usb_device *udev, int retry)
> +static bool use_new_scheme(struct usb_device *udev, int retry,
> +struct usb_port *port_dev)
> {
> + int old_scheme_first_port =
> + port_dev->quirks & USB_PORT_QUIRK_OLD_SCHEME;
> +
> if (udev->speed >= USB_SPEED_SUPER)
> return false;
>
> - return USE_NEW_SCHEME(retry);
> + return USE_NEW_SCHEME(retry, old_scheme_first_port || old_scheme_first);
> }
>
> /* Is a USB 3.0 port in the Inactive or Compliance Mode state?
> @@ -4392,6 +4396,7 @@ hub_port_init(struct usb_hub *hub, struct usb_device
> *udev, int port1,
> {
> struct usb_device *hdev = hub->hdev;
> struct usb_hcd *hcd = bus_to_hcd(hdev->bus);
> + struct usb_port *port_dev = hub->ports[port1 - 1];
> int retries, operations, retval, i;
> unsigneddelay = HUB_SHORT_RESET_TIME;
> enum usb_device_speed oldspeed = udev->speed;
> @@ -4513,7 +4518,7 @@ hub_port_init(struct usb_hub *hub, struct usb_device
> *udev, int port1,
> for (retries = 0; retries < GET_DESCRIPTOR_TRIES; (++retries,
> msleep(100))) {
> bool did_new_scheme = false;
>
> - if (use_new_scheme(udev, retry_counter)) {
> + if (use_new_scheme(udev, retry_counter, port_dev)) {
> struct usb_device_descriptor *buf;
> int r = 0;
>
> diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
> index 4dc769ee9c740..4accfb63f7dcb 100644
> --- a/drivers/usb/core/hub.h
> +++ b/drivers/usb/core/hub.h
> @@ -98,6 +98,7 @@ struct usb_port {
> struct mutex status_lock;
> u32 over_current_count;
> u8 portnum;
> + u32 quirks;
> unsigned int is_superspeed:1;
> unsigned int usb3_lpm_u1_permit:1;
> unsigned int usb3_lpm_u2_permit:1;
> diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
> index 6979bde87d310..4a21431953953 100644
> --- a/drivers/usb/core/port.c
> +++ b/drivers/usb/core/port.c
> @@ -50,6 +50,28 @@ static ssize_t over_current_count_show(struct device *dev,
> }
> static DEVICE_ATTR_RO(over_current_count);
>
> +static ssize_t quirks_show(struct device *dev,
> +struct device_attribute *attr, char *buf)
> +{
> + struct usb_port *port_dev = to_usb_port(dev);
> +
> + return sprintf(buf, "%08x\n", port_dev->quirks);
> +}
> +
> +static ssize_t quirks_store(struct device *dev, struct device_attribute
> *attr,
> + const char *buf, size_t count)
> +{
> + struct usb_port *port_dev = to_usb_port(dev);
> + u32 value;
> +
> + if (kstrtou32(buf, 16, &value))
> + return -EINVAL;
> +
> + port_dev->quirks = value;
> + return count;
> +}
> +static DEVICE_ATTR_RW(quirks);
> +
> static ssize_t usb3_lpm_permit_show(struct device *dev,
> struct device_attribute *attr,
