Re: [PATCH v4] usb_8dev: Add support for USB2CAN interface from 8 devices
Hi Bernd,
still a few issues with error handling.
+/* Send open command to device */
+static int usb_8dev_cmd_open(struct usb_8dev *dev)
+{
+struct can_bittiming *bt = dev-can.bittiming;
+struct usb_8dev_cmd_msg outmsg;
+struct usb_8dev_cmd_msg inmsg;
+u32 flags = 0;
+u32 beflags;
+u16 bebrp;
+u32 ctrlmode = dev-can.ctrlmode;
+
+if (ctrlmode CAN_CTRLMODE_LOOPBACK)
+flags |= USB_8DEV_LOOPBACK;
+if (ctrlmode CAN_CTRLMODE_LISTENONLY)
+flags |= USB_8DEV_SILENT;
+if (ctrlmode CAN_CTRLMODE_ONE_SHOT)
+flags |= USB_8DEV_DISABLE_AUTO_RESTRANS;
+
+flags |= USB_8DEV_STATUS_FRAME;
+
+memset(outmsg, 0, sizeof(struct usb_8dev_cmd_msg));
sizeof(outmsg) is better, here and maybe in other places.
+outmsg.command = USB_8DEV_OPEN;
+outmsg.opt1 = USB_8DEV_BAUD_MANUAL;
+outmsg.data[0] = (bt-prop_seg + bt-phase_seg1);
Minor issue. Brackets not needed.
+outmsg.data[1] = bt-phase_seg2;
+outmsg.data[2] = bt-sjw;
+
+/* BRP */
+bebrp = cpu_to_be16((u16) bt-brp);
+memcpy(outmsg.data[3], bebrp, sizeof(bebrp));
+
+/* flags */
+beflags = cpu_to_be32(flags);
+memcpy(outmsg.data[5], beflags, sizeof(beflags));
+
+return usb_8dev_send_cmd(dev, outmsg, inmsg);
+}
...
+
+/* Read data and status frames */
+static void usb_8dev_rx_can_msg(struct usb_8dev *dev,
+struct usb_8dev_rx_msg *msg)
+{
+struct can_frame *cf;
+struct sk_buff *skb;
+struct net_device_stats *stats = dev-netdev-stats;
+
+if (msg-type == USB_8DEV_TYPE_CAN_FRAME) {
+skb = alloc_can_skb(dev-netdev, cf);
+if (!skb)
+return;
+
+cf-can_id = be32_to_cpu(msg-id);
+cf-can_dlc = get_can_dlc(msg-dlc 0xF);
+
+if (msg-flags USB_8DEV_EXTID)
+cf-can_id |= CAN_EFF_FLAG;
+
+if (msg-flags USB_8DEV_RTR)
+cf-can_id |= CAN_RTR_FLAG;
+else
+memcpy(cf-data, msg-data, cf-can_dlc);
+} else if (msg-type == USB_8DEV_TYPE_ERROR_FRAME
+ msg-flags == USB_8DEV_ERR_FLAG) {
+
+/*
+ * Error message:
+ * byte 0: Status
+ * byte 1: bit 7: Receive Passive
+ * byte 1: bit 0-6: Receive Error Counter
+ * byte 2: Transmit Error Counter
+ * byte 3: Always 0 (maybe reserved for future use)
+ */
+
+u8 state = msg-data[0];
+u8 rxerr = msg-data[1] USB_8DEV_RP_MASK;
+u8 txerr = msg-data[2];
+int rx_errors = 0;
+int tx_errors = 0;
+
+skb = alloc_can_err_skb(dev-netdev, cf);
+if (!skb)
+return;
+
+switch (state) {
+case USB_8DEV_STATUSMSG_OK:
+dev-can.state = CAN_STATE_ERROR_ACTIVE;
+cf-can_id |= CAN_ERR_PROT;
+cf-data[2] = CAN_ERR_PROT_ACTIVE;
+break;
+case USB_8DEV_STATUSMSG_BUSOFF:
+dev-can.state = CAN_STATE_BUS_OFF;
+cf-can_id |= CAN_ERR_BUSOFF;
+can_bus_off(dev-netdev);
+break;
+case USB_8DEV_STATUSMSG_OVERRUN:
Overrun is not a bus-erroror state change. It should be treated
separately. More below...
+case USB_8DEV_STATUSMSG_BUSLIGHT:
+case USB_8DEV_STATUSMSG_BUSHEAVY:
+dev-can.state = CAN_STATE_ERROR_WARNING;
Please set the state below when you re-treat BUSLIGHT and BUSHEAVY.
+cf-can_id |= CAN_ERR_CRTL;
+break;
+default:
+dev-can.state = CAN_STATE_ERROR_WARNING;
Bus-errors do not necessarily change the state. Please remove.
+cf-can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
+dev-can.can_stats.bus_error++;
+break;
+}
+
+switch (state) {
+case USB_8DEV_STATUSMSG_ACK:
+cf-can_id |= CAN_ERR_ACK;
+tx_errors = 1;
+break;
+case USB_8DEV_STATUSMSG_CRC:
+cf-data[2] |= CAN_ERR_PROT_BIT;
+rx_errors = 1;
+break;
+case USB_8DEV_STATUSMSG_BIT0:
+cf-data[2] |= CAN_ERR_PROT_BIT0;
+tx_errors = 1;
+break;
+case USB_8DEV_STATUSMSG_BIT1:
+cf-data[2] |= CAN_ERR_PROT_BIT1;
+tx_errors = 1;
+break;
+case USB_8DEV_STATUSMSG_FORM:
+cf-data[2] |= CAN_ERR_PROT_FORM;
+rx_errors = 1;
+break;
+case USB_8DEV_STATUSMSG_STUFF:
+cf-data[2] |= CAN_ERR_PROT_STUFF;
+rx_errors = 1;
+break;
+case USB_8DEV_STATUSMSG_OVERRUN:
+cf-data[1] = (txerr rxerr) ?
+CAN_ERR_CRTL_TX_OVERFLOW :
+CAN_ERR_CRTL_RX_OVERFLOW;
+cf-data[2] |= CAN_ERR_PROT_OVERLOAD;
+stats-rx_over_errors++;
The overrun normally means the a message could not be
[RFC v4] usb/gadget: the start of the configfs interface
|# modprobe dummy_hcd num=2 |# find /sys/kernel/config/ -ls | 5570 drwxr-xr-x 3 root root0 Nov 29 17:26 /sys/kernel/config/ | 5580 drwxr-xr-x 5 root root0 Nov 29 17:26 /sys/kernel/config/usb_gadget | 5610 drwxr-xr-x 4 root root0 Nov 29 17:26 /sys/kernel/config/usb_gadget/udcs | 5690 drwxr-xr-x 2 root root0 Nov 29 17:26 /sys/kernel/config/usb_gadget/udcs/dummy_udc.1 | 5680 drwxr-xr-x 2 root root0 Nov 29 17:26 /sys/kernel/config/usb_gadget/udcs/dummy_udc.0 | 5600 drwxr-xr-x 2 root root0 Nov 29 17:26 /sys/kernel/config/usb_gadget/gadgets | # lsmod | Module Size Used by | dummy_hcd 20287 0 | udc10219 1 dummy_hcd |# mkdir /sys/kernel/config/usb_gadget/gadgets/oha |# cd /sys/kernel/config/usb_gadget/gadgets/oha |# mkdir configs/def.1 |# mkdir configs/def.2 |# mkdir functions/acm.ttyS1 |# find . -ls root@squsb:/sys/kernel/config/usb_gadget/gadgets/oha# find . -ls 93230 drwxr-xr-x 4 root root0 Dec 5 12:37 . 93250 drwxr-xr-x 4 root root0 Dec 5 12:35 ./configs 93280 drwxr-xr-x 2 root root0 Dec 5 12:37 ./configs/def.2 8680 -rw-r--r-- 1 root root 4096 Dec 5 12:39 ./configs/def.2/sConfiguration 8690 -rw-r--r-- 1 root root 4096 Dec 5 12:39 ./configs/def.2/bmAttributes 8700 -rw-r--r-- 1 root root 4096 Dec 5 12:39 ./configs/def.2/MaxPower 102930 drwxr-xr-x 2 root root0 Dec 5 12:37 ./configs/def.1 8710 -rw-r--r-- 1 root root 4096 Dec 5 12:39 ./configs/def.1/sConfiguration 8720 -rw-r--r-- 1 root root 4096 Dec 5 12:39 ./configs/def.1/bmAttributes 8730 -rw-r--r-- 1 root root 4096 Dec 5 12:39 ./configs/def.1/MaxPower 93240 drwxr-xr-x 4 root root0 Dec 5 12:35 ./functions 83980 drwxr-xr-x 2 root root0 Dec 5 12:37 ./functions/acm.ttyS1 8740 -r--r--r-- 1 root root 4096 Dec 5 12:39 ./functions/acm.ttyS1/port_num 102940 drwxr-xr-x 2 root root0 Dec 5 12:37 ./functions/acm.ttyS0 8750 -r--r--r-- 1 root root 4096 Dec 5 12:39 ./functions/acm.ttyS0/port_num 8760 -rw-r--r-- 1 root root 4096 Dec 5 12:39 ./sSerialNumber 8770 -rw-r--r-- 1 root root 4096 Dec 5 12:39 ./sProduct 8780 -rw-r--r-- 1 root root 4096 Dec 5 12:39 ./sManufacturer 8790 -rw-r--r-- 1 root root 4096 Dec 5 12:39 ./bcdUSB 8800 -rw-r--r-- 1 root root 4096 Dec 5 12:39 ./idProduct 8810 -rw-r--r-- 1 root root 4096 Dec 5 12:39 ./idVendor 8820 -rw-r--r-- 1 root root 4096 Dec 5 12:39 ./bMaxPacketSize0 8830 -rw-r--r-- 1 root root 4096 Dec 5 12:39 ./bDeviceProtocol 8840 -rw-r--r-- 1 root root 4096 Dec 5 12:39 ./bDeviceSubClass 8850 -rw-r--r-- 1 root root 4096 Dec 5 12:39 ./bDeviceClass |# mkdir default.0 |mkdir: cannot create directory `default.0': Invalid argument |oha# mkdir another.2 |mkdir: cannot create directory `another.1': File exists v3…v4 - moved functions from the root folde down to the gadget as suggested by Michał - configs have now their own configs folder as suggested by Michał. The folder is still name.bConfigurationValue where name becomes the sConfiguration. Is this usefull should we just stilc configs/bConfigurationValue/ ? - added configfs support to the ACM function. The port_num attribute is exported by f_acm. An argument has been added to the USB alloc function to distinguish between old (use facm_configure() to configure and configfs interface (expose a config_node). The port_num is currently a dumb counter. It will require some function re-work to make it work. scheduled for v5: - sym linking function into config. - string rework. This will add a strings folder incl. language code like strings/409/manufacturer as suggested by Alan. v2…v3 - replaced one ifndef by ifdef as suggested by Micahał - strstr()/strchr() function_make as suggested by Micahł - replace [iSerialNumber|iProduct|iManufacturer] with [sSerialNumber|sProduct|sManufacturer] as suggested by Alan - added creation of config descriptors v1…v2 - moved gadgets from configfs' root directory into /udcs/ within our usb_gadget folder. Requested by Andrzej Michał - use a dot as a delimiter between function's name and its instance's name as suggested by Michał - renamed all config_item_type, configfs_group_operations, make_group, drop_item as suggested by suggested by Andrzej to remain consisten within this file and within other configfs users - Since configfs.c and
Re: [PATCH v3 23/23] mfd: omap-usb-host: Don't spam console on clk_set_parent failure
Hello.
On 04-12-2012 18:31, Roger Quadros wrote:
clk_set_parent is expected to fail on OMAP3 platforms. We don't
consider that as fatal so don't spam console.
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/mfd/omap-usb-host.c | 18 +-
1 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 0bb54393..e5257dc 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -657,32 +657,32 @@ static int __devinit usbhs_omap_probe(struct
platform_device *pdev)
}
if (is_ehci_phy_mode(pdata-port_mode[0])) {
- /* for OMAP3 , the clk set paretn fails */
+ /* for OMAP3, clk_set_parent fails */
ret = clk_set_parent(omap-utmi_clk[0],
omap-xclk60mhsp1_ck);
if (ret != 0)
- dev_err(dev, xclk60mhsp1_ck set parent
- failed error:%d\n, ret);
+ dev_dbg(dev, xclk60mhsp1_ck set parent failed : %d\n,
+ ret);
} else if (is_ehci_tll_mode(pdata-port_mode[0])) {
ret = clk_set_parent(omap-utmi_clk[0],
omap-init_60m_fclk);
if (ret != 0)
- dev_err(dev, init_60m_fclk set parent
- failed error:%d\n, ret);
+ dev_dbg(dev, P0 init_60m_fclk set parent failed: %d\n,
+ ret);
}
if (is_ehci_phy_mode(pdata-port_mode[1])) {
ret = clk_set_parent(omap-utmi_clk[1],
omap-xclk60mhsp2_ck);
if (ret != 0)
- dev_err(dev, xclk60mhsp2_ck set parent
- failed error:%d\n, ret);
+ dev_dbg(dev, xclk60mhsp2_ck set parent failed : %d\n,
+ ret);
} else if (is_ehci_tll_mode(pdata-port_mode[1])) {
ret = clk_set_parent(omap-utmi_clk[1],
omap-init_60m_fclk);
if (ret != 0)
- dev_err(dev, init_60m_fclk set parent
- failed error:%d\n, ret);
+ dev_dbg(dev, P1 init_60m_fclk set parent failed: %d\n,
+ ret);
Hm, you sometimes put a space before colon in the error message and
sometimes not. Inconsistent. :-)
WBR, Sergei
--
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 04/23] mfd: omap-usb-tll: Use devm_kzalloc/ioremap and clean up error path
Hello.
On 04-12-2012 18:12, Roger Quadros wrote:
Use devm_ variants of kzalloc() and ioremap(). Simplify the error path.
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/mfd/omap-usb-tll.c | 37 +++--
1 files changed, 11 insertions(+), 26 deletions(-)
diff --git a/drivers/mfd/omap-usb-tll.c b/drivers/mfd/omap-usb-tll.c
index e67cafc..828207f 100644
--- a/drivers/mfd/omap-usb-tll.c
+++ b/drivers/mfd/omap-usb-tll.c
[...]
@@ -230,28 +229,21 @@ static int __devinit usbtll_omap_probe(struct
platform_device *pdev)
if (IS_ERR(tll-usbtll_p1_fck)) {
ret = PTR_ERR(tll-usbtll_p1_fck);
dev_err(dev, usbtll_p1_fck failed error:%d\n, ret);
- goto err_tll;
+ return ret;
}
tll-usbtll_p2_fck = clk_get(dev, usb_tll_hs_usb_ch1_clk);
if (IS_ERR(tll-usbtll_p2_fck)) {
ret = PTR_ERR(tll-usbtll_p2_fck);
dev_err(dev, usbtll_p2_fck failed error:%d\n, ret);
- goto err_usbtll_p1_fck;
+ goto err_p2_fck;
}
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res) {
- dev_err(dev, usb tll get resource failed\n);
- ret = -ENODEV;
- goto err_usbtll_p2_fck;
- }
Not clear why you removed the error check...
-
- base = ioremap(res-start, resource_size(res));
+ base = devm_request_and_ioremap(dev, res);
if (!base) {
- dev_err(dev, TLL ioremap failed\n);
- ret = -ENOMEM;
- goto err_usbtll_p2_fck;
+ ret = -EADDRNOTAVAIL;
Why you changed this from ENOMEM?
WBR, Sergei
--
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 PATCH 1/5] drivers : introduce device_path api
Hi Jassi, On 12/01/2012 09:49 AM, Jassi Brar wrote: On Tue, Nov 27, 2012 at 10:32 PM, Greg KH gre...@linuxfoundation.org wrote: On Tue, Nov 27, 2012 at 11:30:11AM -0500, Alan Stern wrote: We should have a more generic solution in a more central location, like the device core. For example, each struct platform_device could have a list of resources to be enabled when the device is bound to a driver and disabled when the device is unbound. Such a list could include regulators, clocks, and whatever else people can invent. In this case, when it is created the ehci-omap.0 platform device could get an entry pointing to the LAN95xx's regulator. Then the regulator would automatically be turned on when the platform device is bound to the ehci-omap driver. How does this sound? That sounds much better, and it might come in handy for other types of devices than platform devices, so feel free to put this on the core 'struct device' instead if needed. Isn't enabling/disabling of clocks and regulators what dev.probe()/remove() of any driver already does? If we mean only board specific setup/teardown, still it would mean having the device core to do an extra 'probe' call when the current probe() is already meant to do whatever it takes to bring the device up. To my untrained eyes, it seem like messing with the probe()/remove()'s semantics. IMHO, if we really must implement something like that, may be we should employ some 'broadcast mechanism' so that anything anywhere in kernel knows which new device has been probed()/removed() successfully. I haven't thought exactly how because I am not sure even that would be the right way to approach PandaBoard's problem. Looking at Figure 15 – Panda USBB1 Interface Block Diagram of http://pandaboard.org/sites/default/files/board_reference/pandaboard-es-b/panda-es-b-manual.pdf gives me visions ... 1) OMAP doesn't provide the USB root-hub, but only ULPIPHY. It is USB3320C chip that employs OMAP's ULPIPHY to provide the root-hub. So we should have a platform device for USB3320C, the probe() of which should simply Actually the EHCI controller within OMAP provides the root hub with 3 ports but no PHY. One of them is connected to the USB3320 which is just a ULPI PHY. a) Enable REFCLK (FREF_CLK3_OUT) b) Reset itself by cycling RESETB (GPIO_62) c) Create one ehci-omap platform device c) is not appropriate. ehci-omap must be created before usb3320. (or in appropriate order if the above isn't) That way insmod/rmmod'ing the USB3320C driver would power-up/down the whole subsystem. Yes, this is where we can think of a generic PHY driver to make sure thy PHY has necessary resources enabled. In the Panda case it would be the PHY clock and RESET gpio. The LAN95xx chip still needs to be powered up and the PHY driver isn't the right place for that (though it could be done there as a hack). The closest we can get to emulating right USB behavior is to map the SET/CLEAR PORT_FEATURE of the root hub port to the regulator that powers the LAN95xx. This way, we still don't fall in the dynamically probed space, so we could still provide the regulator information to the ehci hub via platform data and handle the regulator in ehci_hub_control (Set/ClearPortFeature). I'm looking at this as a software workaround for all platforms not implementing the EHCI set port power feature correctly. The same could be repeated for other HCDs as well if required. cheers, -roger -- 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 23/23] mfd: omap-usb-host: Don't spam console on clk_set_parent failure
On 12/05/2012 03:42 PM, Sergei Shtylyov wrote:
Hello.
On 04-12-2012 18:31, Roger Quadros wrote:
clk_set_parent is expected to fail on OMAP3 platforms. We don't
consider that as fatal so don't spam console.
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/mfd/omap-usb-host.c | 18 +-
1 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 0bb54393..e5257dc 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -657,32 +657,32 @@ static int __devinit usbhs_omap_probe(struct
platform_device *pdev)
}
if (is_ehci_phy_mode(pdata-port_mode[0])) {
-/* for OMAP3 , the clk set paretn fails */
+/* for OMAP3, clk_set_parent fails */
ret = clk_set_parent(omap-utmi_clk[0],
omap-xclk60mhsp1_ck);
if (ret != 0)
-dev_err(dev, xclk60mhsp1_ck set parent
-failed error:%d\n, ret);
+dev_dbg(dev, xclk60mhsp1_ck set parent failed : %d\n,
+ret);
} else if (is_ehci_tll_mode(pdata-port_mode[0])) {
ret = clk_set_parent(omap-utmi_clk[0],
omap-init_60m_fclk);
if (ret != 0)
-dev_err(dev, init_60m_fclk set parent
-failed error:%d\n, ret);
+dev_dbg(dev, P0 init_60m_fclk set parent failed: %d\n,
+ret);
}
if (is_ehci_phy_mode(pdata-port_mode[1])) {
ret = clk_set_parent(omap-utmi_clk[1],
omap-xclk60mhsp2_ck);
if (ret != 0)
-dev_err(dev, xclk60mhsp2_ck set parent
-failed error:%d\n, ret);
+dev_dbg(dev, xclk60mhsp2_ck set parent failed : %d\n,
+ret);
} else if (is_ehci_tll_mode(pdata-port_mode[1])) {
ret = clk_set_parent(omap-utmi_clk[1],
omap-init_60m_fclk);
if (ret != 0)
-dev_err(dev, init_60m_fclk set parent
-failed error:%d\n, ret);
+dev_dbg(dev, P1 init_60m_fclk set parent failed: %d\n,
+ret);
Hm, you sometimes put a space before colon in the error message and
sometimes not. Inconsistent. :-)
That was because it fit in 80 characters without the space. I'm not sure
what is more important, fitting in 80 or consistency in the print
message. Maybe i should have removed the spaces everywhere so that it is
consistent as well. :)
cheers,
-roger
--
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: Add device quirk for Microsoft VX700 webcam
Add device quirk for Microsoft Lifecam VX700 v2.0 webcams.
Fixes squeaking noise of the microphone.
Signed-off-by: Andreas Fleig andreasfl...@gmail.com
---
diff --git a/drivers/usb/core/quirks.c b/drivers/usb/core/quirks.c
index fdefd9c..3113c1d 100644
--- a/drivers/usb/core/quirks.c
+++ b/drivers/usb/core/quirks.c
@@ -43,6 +43,9 @@ static const struct usb_device_id usb_quirk_list[] = {
/* Creative SB Audigy 2 NX */
{ USB_DEVICE(0x041e, 0x3020), .driver_info = USB_QUIRK_RESET_RESUME },
+ /* Microsoft LifeCam-VX700 v2.0 */
+ { USB_DEVICE(0x045e, 0x0770), .driver_info = USB_QUIRK_RESET_RESUME },
+
/* Logitech Quickcam Fusion */
{ USB_DEVICE(0x046d, 0x08c1), .driver_info = USB_QUIRK_RESET_RESUME },
--
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: Correlating SOF with host system time
On Wed, 5 Dec 2012, Stefan Tauner wrote: Running NTP over a USB-based network link would certainly be the easiest solution, if your device can support it. Over the long run, it might even be more accurate on average than using SOF packets. We are talking about microcontrollers with a few kB RAM at most, so just cross-compiling any (S)NTP client wont cut it probably :) So the SOF approach seems to be way more elegant and easy to implement to me... in theory. Your theory omits an important point: The host system generally doesn't care exactly when frames start. Forcing it to keep track of these events adds overhead. Another nice property is that SOF packets are sent for every frame in any case (but errors) and they are broadcasted, which is both advantageous regarding bandwidth usage and independent of the number of devices attached. It may not be all that advantageous after all, because you still have to unicast the information for relating timestamps to SOF packets. Since you have to do this anyway, you might as well use those unicast packets for synchronization instead of the SOF packets. Regarding long-term stability/precision i am not sure i can agree with you. If i throw the same amount of statistics/algorithm complexity at the SOF scheme and a software-exclusive approach i dont see how the latter could be better :) Exactly: _If_. 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 01/10] USB: Set usb port's DeviceRemovable according acpi information in EHCI
On Wed, 5 Dec 2012, Lan Tianyu wrote:
Hi Alan:
how about following patch?
Index: usb/drivers/usb/core/hub.c
===
--- usb.orig/drivers/usb/core/hub.c
+++ usb/drivers/usb/core/hub.c
+void usb_hub_adjust_DeviceRemovable(struct usb_device *hdev,
+ struct usb_hub_descriptor *desc)
+{
+ enum usb_port_connect_type connect_type;
+ int i;
+
+ if (!hub_is_superspeed(hdev)) {
+ for (i = 1; i = hdev-maxchild; i++) {
+ connect_type =
+ usb_get_hub_port_connect_type(hdev, i);
+
+ if (connect_type ==
USB_PORT_CONNECT_TYPE_HARD_WIRED) {
+ u8 mask = 1 (i%8);
+
+ if (!(desc-u.hs.DeviceRemovable[i/8]
mask))
+ dev_dbg(hdev-dev, usb2.0
There's no need to specify usb2.0. The kernel log already mentions
the hub's speed.
port%d's DeviceRemovable is changed to 1 according platform
information.\n, i);
s/according/according to/
+
+ desc-u.hs.DeviceRemovable[i/8]
+ |= mask;
You might as well put this line inside the if statement.
+ }
+ }
+ } else {
+ u16 port_removable =
+ le16_to_cpu(desc-u.ss.DeviceRemovable);
+
+ for (i = 1; i = hdev-maxchild; i++) {
+ connect_type =
+ usb_get_hub_port_connect_type(hdev, i);
+
+ if (connect_type ==
USB_PORT_CONNECT_TYPE_HARD_WIRED) {
+ u16 mask = 1 i;
+
+ if (!(port_removable mask))
+ dev_dbg(hdev-dev, usb3.0
port%d's DeviceRemovable is changed to 1 according platform
information.\n, i);
+
+ port_removable |= mask;
Same comments here.
+ }
+ }
+
+ desc-u.ss.DeviceRemovable =
+ cpu_to_le16(port_removable);
+ }
+}
The rest looks okay. Remember to run your patches through checkpatch
before submitting them.
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 v4] usb_8dev: Add support for USB2CAN interface from 8 devices
On 12/05/2012 04:49 PM, Oliver Hartkopp wrote: On 05.12.2012 11:13, Wolfgang Grandegger wrote: +outmsg.command = USB_8DEV_OPEN; +outmsg.opt1 = USB_8DEV_BAUD_MANUAL; +outmsg.data[0] = (bt-prop_seg + bt-phase_seg1); Minor issue. Brackets not needed. +outmsg.data[1] = bt-phase_seg2; +outmsg.data[2] = bt-sjw; + That's correct from a compilers point of view. But in this case i would preserve the [0] as it is better to read in conjunction with the two lines below ( ..[1] ..[2] ) The semantic has to be understood by humans ;-) I think Wolfgang was talking about the round bracket not the square bracket. :D Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH RESEND] tty: don't dead lock while flushing workqueue
On 12/03/2012 06:41 PM, Peter Hurley wrote: The lock logic for tty_set_ldisc() is wrong. Despite existing code in tty_set_ldisc() and tty_ldisc_hangup(), the ldisc_mutex does **not** (and should not) play a role in acquiring or releasing ldisc references. The only thing that needs to happen here is below (don't actually use below because I just hand-edited it): Hmm. What about I stay in sync with the code that is already in tree and if the wrong locking gets removed in both places later on? Alan, what do you prefer? See http://lkml.org/lkml/2012/11/21/347 drivers/tty/tty_ldisc.c | 13 + 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/tty/tty_ldisc.c b/drivers/tty/tty_ldisc.c index 0f2a2c5..fb76818 100644 --- a/drivers/tty/tty_ldisc.c +++ b/drivers/tty/tty_ldisc.c @@ -930,16 +930,21 @@ void tty_ldisc_release(struct tty_struct *tty, struct tty_struct *o_tty) */ - tty_lock_pair(tty, o_tty); tty_ldisc_halt(tty); tty_ldisc_flush_works(tty); + tty_lock_pair(tty, o_tty); /* This will need doing differently if we need to lock */ tty_ldisc_kill(tty); - if (o_tty) tty_ldisc_kill(o_tty); Sebastian -- 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 v4] usb_8dev: Add support for USB2CAN interface from 8 devices
On 12/05/2012 05:00 PM, Marc Kleine-Budde wrote: On 12/05/2012 04:49 PM, Oliver Hartkopp wrote: On 05.12.2012 11:13, Wolfgang Grandegger wrote: +outmsg.command = USB_8DEV_OPEN; +outmsg.opt1 = USB_8DEV_BAUD_MANUAL; +outmsg.data[0] = (bt-prop_seg + bt-phase_seg1); Minor issue. Brackets not needed. +outmsg.data[1] = bt-phase_seg2; +outmsg.data[2] = bt-sjw; + That's correct from a compilers point of view. But in this case i would preserve the [0] as it is better to read in conjunction with the two lines below ( ..[1] ..[2] ) The semantic has to be understood by humans ;-) I think Wolfgang was talking about the round bracket not the square bracket. :D Yep. Sorry for confusion. Wolfgang. -- 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 PATCH 4/5] arm: omap2: support port power on lan95xx devices
On Wed, 5 Dec 2012, Andy Green wrote:
The details of this aren't clear yet. For instance, should the device
core try to match the port with the asset info, or should this be done
by the USB code when the port is created?
Currently what I have (this is before changing it to pm domain, but this
should be unchanged) lets the asset define a callback op which will
receive notifications for all added child devices that have the device
the asset is bound to as an ancestor.
So you would bind an asset to the host controller device...
static struct device_asset assets_ehci_omap0[] = {
{
.name = -0:1.0/port1,
.asset = assets_ehci_omap0_smsc_port,
.ops = device_descendant_attach_default_asset_ops,
},
{ }
};
...with this descendant filter callback pointing to a generic end of
the device path matcher.
when device_descendant_attach_default_asset_ops() sees the child that
was foretold has appeared (and it only hears about children of
ehci-omap.0 in this case), it binds the assets pointed to by .asset to
the new child before its probe. assets_ehci_omap0_smsc_port is an array
of the actual regulator and clock that need switching by the child. So
the effect is to magic the right assets to the child device just before
it gets added (and probe called etc).
This is working well and the matcher helper is small and simple.
Right. The question is should it be done in this somewhat roundabout
way (you've got two separate assets for setting up this one thing), or
should the USB port code be responsible for explicitly checking if
there are any applicable assets when the port is created?
The advantange of the second approach is that it doesn't involve
checking all the ancestors every time a new device is added (and it
involves only one asset). The disadvantage is that it works only for
USB ports.
To answer the question, we need to know how other subsystems might
want to use the asset-matching approach. My guess is that only a small
number of device types would care about assets (in the same way that
assets matter to USB ports but not to other USB device types). And it
might not be necessary to check against every ancestor; we might know
beforehand where to check (just as the USB port would know to look for
an asset attached to the host controller device).
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 PATCH 1/5] Device Power: introduce power controller
On 12/03/2012 05:00 AM, Ming Lei wrote: On Mon, Dec 3, 2012 at 12:02 AM, Andy Green andy.gr...@linaro.org wrote: On 02/12/12 23:01, the mail apparently from Ming Lei included: Power controller is an abstract on simple power on/off switch. One power controller can bind to more than one device, which provides power logically, for example, we can think one usb port in hub provides power to the usb device attached to the port, even though the power is supplied actually by other ways, eg. the usb hub is a self-power device. From hardware view, more than one device can share one power domain, and power controller can power on if one of these devices need to provide power, and power off if all these devices don't need to provide power. What stops us using struct regulator here? If you have child regulators supplied by a parent supply, isn't that the right semantic already without introducing a whole new thing? Apologies if I missed the point. There are two purposes: One is to hide the implementation details of the power controller because the user doesn't care how it is implemented, maybe clock, regulator, gpio and other platform dependent stuffs involved, so the patch simplify the usage from the view of users. Which user are you talking about? AFAIK for Panda we only need a regulator and a clock for each root port. IMHO we should just use the existing regulator and clock frameworks. Another is that several users may share one power controller, and the introduced power controller can help users to use it. True. e.g. the same regulator could be used to power 3 root hub ports in a ganged setup. But the regulator framework is sufficient to deal with that. Each port will call regulator_get() and regulator_enable() and the physical regulator won't be disabled till all of them have called regulator_disable(). regards, -roger -- 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 v2 03/11] USB: Allow USB 3.0 ports to be disabled.
On Tue, 4 Dec 2012, Sarah Sharp wrote:
If hot and warm reset fails, or a port remains in the Compliance Mode,
the USB core needs to be able to disable a USB 3.0 port. Unlike USB 2.0
ports, once the port is placed into the Disabled link state, it will not
report any new device connects. To get device connect notifications, we
need to put the link into the Disabled state, and then the RxDetect
state.
The xHCI driver needs to atomically clear all change bits on USB 3.0
port disable, so that we get Port Status Change Events for future port
changes.
(We could technically do this in the USB core instead of in the xHCI
roothub code, since the port state machine can't advance out of the
disabled state until we set the link state to RxDetect. However,
external USB 3.0 hubs don't need this code. They are level-triggered,
not edge-triggered like xHCI, so they will continue to send interrupt
events when any change bit is set. Therefore it doesn't make sense to
put this code in the USB core.)
Please merge the two previous paragraphs into one, so that it's clear
you're referring to the atomically clear all change bits requirement.
This patch is part of a series to fix several reports of infinite loops
on device enumeration failure. This includes John, when he boots with
a USB 3.0 device (Roseweil eusb3 enclosure) attached to his NEC 0.96
host controller. The fix requires warm reset support, so it does not
make sense to backport this patch to stable kernels without warm reset
support.
This patch should be backported to kernels as old as 3.2, contain the
commit ID 75d7cf72ab9fa01dc70877aa5c68e8ef477229dc usbcore: refine warm
reset logic
Signed-off-by: Sarah Sharp sarah.a.sh...@linux.intel.com
Reported-by: John Covici cov...@ccs.covici.com
Cc: sta...@vger.kernel.org
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -877,6 +877,60 @@ static int hub_hub_status(struct usb_hub *hub,
return ret;
}
+static int hub_set_port_link_state(struct usb_hub *hub, int port1,
+ unsigned int link_status)
+{
+ return set_port_feature(hub-hdev,
+ port1 | (link_status 3),
Shouldn't this be 8?
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -761,12 +761,39 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq,
u16 wValue,
/* Software should not attempt to set
* port link state above '5' (Rx.Detect) and the port
* must be enabled.
*/
if ((temp PORT_PE) == 0 ||
- (link_state USB_SS_PORT_LS_RX_DETECT)) {
+ (link_state USB_SS_PORT_LS_U3)) {
xhci_warn(xhci, Cannot set link state.\n);
goto error;
}
Looks like the comment needs to be updated too.
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 RESEND] tty: don't dead lock while flushing workqueue
On Wed, 05 Dec 2012 17:15:40 +0100 Sebastian Andrzej Siewior bige...@linutronix.de wrote: On 12/03/2012 06:41 PM, Peter Hurley wrote: The lock logic for tty_set_ldisc() is wrong. Despite existing code in tty_set_ldisc() and tty_ldisc_hangup(), the ldisc_mutex does **not** (and should not) play a role in acquiring or releasing ldisc references. The only thing that needs to happen here is below (don't actually use below because I just hand-edited it): Hmm. What about I stay in sync with the code that is already in tree and if the wrong locking gets removed in both places later on? Alan, what do you prefer? So long as it ends up right I don't care 8) -- 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 v2 06/11] USB: Handle warm reset failure on empty port.
On Tue, 4 Dec 2012, Sarah Sharp wrote:
An empty port can transition to either Inactive or Compliance Mode if a
newly connected USB 3.0 device fails to link train. In that case, we
issue a warm reset. Some devices, such as John's Roseweil eusb3
enclosure, slip back into Compliance Mode after the warm reset.
The current warm reset code does not check for device connect status on
warm reset completion, and it incorrectly reports the warm reset
succeeded. This causes the USB core to attempt to send a Set Address
control transfer to a port in Compliance Mode, which will always fail.
Make hub_port_wait_reset check the current connect status and link state
after the warm reset completes. Return a failure status if the device
is disconnected or the link state is Compliance Mode or SS.Inactive.
Make hub_events disable the port if warm reset fails. This will disable
the port, and then bring it back into the RxDetect state. Make the USB
core ignore the connect change until the device reconnects.
Is this really the right thing to do? It means that drivers will
continue to submit URBs.
Note that this patch does NOT handle connected devices slipping into the
Inactive state very well. This is a concern, because devices can go
into the Inactive state on U1/U2 exit failure. However, the fix for
that case is too large for stable, so it will be submitted in a separate
patch.
This patch should be backported to kernels as old as 3.2, contain the
commit ID 75d7cf72ab9fa01dc70877aa5c68e8ef477229dc usbcore: refine warm
reset logic
Signed-off-by: Sarah Sharp sarah.a.sh...@linux.intel.com
Reported-by: John Covici cov...@ccs.covici.com
Cc: sta...@vger.kernel.org
---
drivers/usb/core/hub.c | 18 +++---
1 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index b7b055f..85977de 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2601,8 +2601,15 @@ static int hub_port_wait_reset(struct usb_hub *hub,
int port1,
return 0;
}
} else {
- if (portchange USB_PORT_STAT_C_BH_RESET)
- return 0;
+ if (!(portchange USB_PORT_STAT_C_BH_RESET))
+ goto delay;
Does this bit get set as soon as the reset is finished, or not until
later? (The spec isn't clear about this, or I'm not looking in the
right place.)
If it gets set as soon as the reset is finished then this test isn't
needed, because you already added a test for USB_PORT_STAT_RESET.
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 v2 05/11] USB: Ignore port state until reset completes.
On Tue, 4 Dec 2012, Sarah Sharp wrote:
The port reset code bails out early if the current connect status is
cleared (device disconnected). If we're issuing a hot reset, it may
also look at the link state before the reset is finished.
Section 10.14.2.6 of the USB 3.0 spec says that when a port enters the
Error state or Resetting state, the port connection bit retains the
value from the previous state. Therefore we can't trust it until the
reset finishes. Also, the xHCI spec section 4.19.1.2.5 says software
shall ignore the link state while the port is resetting, as it can be in
an unknown state.
The port state during reset is also unknown for USB 2.0 hubs. The hub
sends a reset signal by driving the bus into an SE0 state. This
overwhelms the connect signal from the device, so the port can't tell
whether anything is connected or not.
Fix the port reset code to ignore the port link state and current
connect bit until the reset finishes.
This patch should be backported to all stable kernels.
Signed-off-by: Sarah Sharp sarah.a.sh...@linux.intel.com
Cc: sta...@vger.kernel.org
---
drivers/usb/core/hub.c |5 +
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index b9ce5e8..b7b055f 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2534,6 +2534,10 @@ static int hub_port_wait_reset(struct usb_hub *hub,
int port1,
if (ret 0)
return ret;
+ /* The port state is unknown until the reset completes. */
+ if ((portstatus USB_PORT_STAT_RESET))
+ goto delay;
+
/*
* Some buggy devices require a warm reset to be issued even
* when the port appears not to be connected.
@@ -2601,6 +2605,7 @@ static int hub_port_wait_reset(struct usb_hub *hub, int
port1,
return 0;
}
+delay:
/* switch to the long delay after two short delay failures */
if (delay_time = 2 * HUB_SHORT_RESET_TIME)
delay = HUB_LONG_RESET_TIME;
At some point this entire loop should be turned inside out. The
outline should be:
for (delay_time = 0; ...) {
msleep(delay);
ret = hub_port_status(...);
if (reset finished)
break;
adjust the delay time
}
Do all the stuff involving the various status bits...
Maybe you'd prefer to make this change after some of the other patches
in this series. That would be fine, so long as it does eventually get
made.
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: xhci problem
On Tue, Dec 04, 2012 at 08:28:30PM -0500, Allan Dennis wrote: I hope you won't be annoyed by my question... here goes: It's my job to answer questions, so fire away. :) I have 2TB and 3TB SATA drives, both connected to my Intense PC (Ivy Bridge tiny PC) on separate external drive enclosures on USB 3 cables. I have gentoo linux, and was running kernel 3.4.9 where both drives weren't recognized properly. Now I updated to 3.5.7 and the 2TB is good to go (yay!) but the 3TB guy is still giving me problems. Here is the dmesg output as I plugged in the 3TB (sdc) after the 2TB (sdb) was successful, with debug enabled in the kernel config. Is this a known issue? Can I help you figure out what's going wrong here by providing logs or whatnot? I can't see anything wrong on the xHCI side, so I suspect the problem might be that the SCSI layer doesn't like some response the hard drive is sending. I would suggest you take a usbmon trace, following the directions here: http://lxr.linux.no/#linux/Documentation/usb/usbmon.txt Please hit 'reply-all' when you send the trace, since I've Cced the linux-usb and usb-storage mailing lists. Sarah Sharp [ 134.333823] xhci_hcd :03:00.0: Port Status Change Event for port 3 [ 134.333848] hub 6-0:1.0: state 7 ports 2 chg evt 0002 [ 134.333854] xhci_hcd :03:00.0: get port status, actual port 0 status = 0x21203 [ 134.333857] xhci_hcd :03:00.0: Get port status returned 0x10203 [ 134.333865] xhci_hcd :03:00.0: clear port connect change, actual port 0 status = 0x1203 [ 134.333868] hub 6-0:1.0: port 1, status 0203, change 0001, 5.0 Gb/s [ 134.333872] xhci_hcd :03:00.0: get port status, actual port 0 status = 0x1203 [ 134.333873] xhci_hcd :03:00.0: Get port status returned 0x203 [ 134.359543] xhci_hcd :03:00.0: get port status, actual port 0 status = 0x1203 [ 134.359546] xhci_hcd :03:00.0: Get port status returned 0x203 [ 134.385514] xhci_hcd :03:00.0: get port status, actual port 0 status = 0x1203 [ 134.385518] xhci_hcd :03:00.0: Get port status returned 0x203 [ 134.411486] xhci_hcd :03:00.0: get port status, actual port 0 status = 0x1203 [ 134.411490] xhci_hcd :03:00.0: Get port status returned 0x203 [ 134.437459] xhci_hcd :03:00.0: get port status, actual port 0 status = 0x1203 [ 134.437463] xhci_hcd :03:00.0: Get port status returned 0x203 [ 134.437467] hub 6-0:1.0: debounce: port 1: total 100ms stable 100ms status 0x203 [ 134.437470] xhci_hcd :03:00.0: // Ding dong! [ 134.437497] xhci_hcd :03:00.0: Slot 2 output ctx = 0xb0003000 (dma) [ 134.437500] xhci_hcd :03:00.0: Slot 2 input ctx = 0xb0004000 (dma) [ 134.437505] xhci_hcd :03:00.0: Set slot id 2 dcbaa entry 8800b003a010 to 0xb0003000 [ 134.437514] xhci_hcd :03:00.0: set port reset, actual port 0 status = 0x1211 [ 134.437524] xhci_hcd :03:00.0: Port Status Change Event for port 3 [ 134.488408] xhci_hcd :03:00.0: get port status, actual port 0 status = 0x201203 [ 134.488412] xhci_hcd :03:00.0: Get port status returned 0x100203 [ 134.539353] xhci_hcd :03:00.0: clear port reset change, actual port 0 status = 0x1203 [ 134.539360] xhci_hcd :03:00.0: Set root hub portnum to 3 [ 134.539361] xhci_hcd :03:00.0: Set fake root hub portnum to 1 [ 134.539363] xhci_hcd :03:00.0: udev-tt = (null) [ 134.539365] xhci_hcd :03:00.0: udev-ttport = 0x0 [ 134.539367] xhci_hcd :03:00.0: Slot ID 2 Input Context: [ 134.539369] xhci_hcd :03:00.0: @8800b0004000 (virt) @b0004000 (dma) 0x00 - drop flags [ 134.539371] xhci_hcd :03:00.0: @8800b0004004 (virt) @b0004004 (dma) 0x03 - add flags [ 134.539373] xhci_hcd :03:00.0: @8800b0004008 (virt) @b0004008 (dma) 0x00 - rsvd2[0] [ 134.539375] xhci_hcd :03:00.0: @8800b000400c (virt) @b000400c (dma) 0x00 - rsvd2[1] [ 134.539377] xhci_hcd :03:00.0: @8800b0004010 (virt) @b0004010 (dma) 0x00 - rsvd2[2] [ 134.539379] xhci_hcd :03:00.0: @8800b0004014 (virt) @b0004014 (dma) 0x00 - rsvd2[3] [ 134.539381] xhci_hcd :03:00.0: @8800b0004018 (virt) @b0004018 (dma) 0x00 - rsvd2[4] [ 134.539383] xhci_hcd :03:00.0: @8800b000401c (virt) @b000401c (dma) 0x00 - rsvd2[5] [ 134.539385] xhci_hcd :03:00.0: @8800b0004020 (virt) @b0004020 (dma) 0x00 - rsvd64[0] [ 134.539387] xhci_hcd :03:00.0: @8800b0004028 (virt) @b0004028 (dma) 0x00 - rsvd64[1] [ 134.539389] xhci_hcd :03:00.0: @8800b0004030 (virt) @b0004030 (dma) 0x00 - rsvd64[2] [ 134.539391] xhci_hcd :03:00.0: @8800b0004038 (virt) @b0004038 (dma) 0x00 - rsvd64[3] [ 134.539392] xhci_hcd :03:00.0: Slot Context: [ 134.539394] xhci_hcd :03:00.0: @8800b0004040 (virt) @b0004040 (dma) 0x840 - dev_info [ 134.539396] xhci_hcd :03:00.0: @8800b0004044 (virt)
Re: [PATCH v3] usb_8dev: Add support for USB2CAN interface from 8 devices
Hi Marc!
+default:
+netdev_info(netdev, Rx URB aborted (%d)\n,
+ urb-status);
+goto resubmit_urb;
+}
+
+while (pos urb-actual_length) {
+struct usb_8dev_rx_msg *msg;
+
+if (pos + sizeof(struct usb_8dev_rx_msg) urb-actual_length) {
+netdev_err(dev-netdev, format error\n);
+break;
is resubmitting the urb the correct way to handle this problem?
Suggestions? (maybe CAN_ERR_CRTL_UNSPEC ??)
+
+stats-tx_dropped++;
+}
+} else {
+/* Slow down tx path */
+if (atomic_read(dev-active_tx_urbs) = MAX_TX_URBS ||
+dev-free_slots 5) {
where's the 5 coming from?
From ems_usb driver.
+netif_stop_queue(netdev);
+}
+}
+
+/*
+ * Release our reference to this URB, the USB core will eventually
free
+ * it entirely.
+ */
+usb_free_urb(urb);
+
+return NETDEV_TX_OK;
+
+nomem:
+dev_kfree_skb(skb);
+stats-tx_dropped++;
+
+return NETDEV_TX_OK;
+}
+
+static int usb_8dev_get_berr_counter(const struct net_device *netdev,
+ struct can_berr_counter *bec)
+{
+struct usb_8dev *dev = netdev_priv(netdev);
+
+bec-txerr = dev-bec.txerr;
+bec-rxerr = dev-bec.rxerr;
+
+return 0;
+}
+
+/* Start USB device */
+static int usb_8dev_start(struct usb_8dev *dev)
+{
+struct net_device *netdev = dev-netdev;
+int err, i;
+
+dev-free_slots = 15; /* initial size */
there does the 15 come from?
ditto
+ * Check device and firmware.
+ * Set supported modes and bittiming constants.
+ * Allocate some memory.
+ */
+static int usb_8dev_probe(struct usb_interface *intf,
+ const struct usb_device_id *id)
+{
+struct net_device *netdev;
+struct usb_8dev *dev;
+int i, err = -ENOMEM;
+u32 version;
+char buf[18];
where does this 18 come from?
String USB2CAN converter + trailing 0.
+struct usb_device *usbdev = interface_to_usbdev(intf);
+
+/* product id looks strange, better we also check iProdukt string */
iProduct?
Check if usbdev-descriptor.iProduct == USB2CAN converter.
+if (usb_string(usbdev, usbdev-descriptor.iProduct, buf,
+ sizeof(buf)) 0 strcmp(buf, USB2CAN converter)) {
+dev_info(usbdev-dev, ignoring: not an USB2CAN converter\n);
+return -ENODEV;
+}
+
+netdev = alloc_candev(sizeof(struct usb_8dev), MAX_TX_URBS);
+if (!netdev) {
+dev_err(intf-dev, Couldn't alloc candev\n);
+return -ENOMEM;
+}
+
+dev = netdev_priv(netdev);
+
+dev-udev = usbdev;
+dev-netdev = netdev;
+
+dev-can.state = CAN_STATE_STOPPED;
+dev-can.clock.freq = USB_8DEV_ABP_CLOCK;
+dev-can.bittiming_const = usb_8dev_bittiming_const;
+dev-can.do_set_mode = usb_8dev_set_mode;
+dev-can.do_get_berr_counter = usb_8dev_get_berr_counter;
+dev-can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
+ CAN_CTRLMODE_LISTENONLY |
+ CAN_CTRLMODE_ONE_SHOT;
Have you actually tested one shot? What happens if a can frame cannot be
transmitted?
Not really. Can someone explain what one-shot exactly means and what is the
correct behavior.
I've only tested without any other device connected. I got the same errors like
in normal mode.
regards,
Bernd
--
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 v2 05/11] USB: Ignore port state until reset completes.
On Wed, Dec 05, 2012 at 12:19:34PM -0500, Alan Stern wrote:
On Tue, 4 Dec 2012, Sarah Sharp wrote:
@@ -2534,6 +2534,10 @@ static int hub_port_wait_reset(struct usb_hub *hub,
int port1,
if (ret 0)
return ret;
+ /* The port state is unknown until the reset completes. */
+ if ((portstatus USB_PORT_STAT_RESET))
+ goto delay;
+
/*
* Some buggy devices require a warm reset to be issued even
* when the port appears not to be connected.
@@ -2601,6 +2605,7 @@ static int hub_port_wait_reset(struct usb_hub *hub,
int port1,
return 0;
}
+delay:
/* switch to the long delay after two short delay failures */
if (delay_time = 2 * HUB_SHORT_RESET_TIME)
delay = HUB_LONG_RESET_TIME;
At some point this entire loop should be turned inside out. The
outline should be:
for (delay_time = 0; ...) {
msleep(delay);
ret = hub_port_status(...);
if (reset finished)
break;
adjust the delay time
}
Do all the stuff involving the various status bits...
Maybe you'd prefer to make this change after some of the other patches
in this series. That would be fine, so long as it does eventually get
made.
Ok, I'll refactor this in the patches that aren't bound for stable.
Sarah Sharp
--
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 01/23] mfd: omap-usb-host: get rid of cpu_is_omap..() macros
* Roger Quadros rog...@ti.com [121204 06:15]:
Instead of using cpu_is_omap..() macros in the device driver we
rely on information provided in the platform data.
The only information we need is whether the USB Host module has
a single ULPI bypass control bit for all ports or individual bypass
control bits for each port. OMAP3 REV2.1 and earlier have the former.
Thanks for moving this one earlier in the series. Looks like
you're missing Samuel as the MFD maintainer from your cc.
You should resend again with Samuel Ortiz sa...@linux.intel.com
added.
Maybe check the patches in this series with scripts/get_maintainer.pl
to see who all should be cc:ed?
Regards,
Tony
Signed-off-by: Roger Quadros rog...@ti.com
CC: Tony Lindgren t...@atomide.com
---
arch/arm/mach-omap2/usb-host.c |4
drivers/mfd/omap-usb-host.c|2 +-
include/linux/platform_data/usb-omap.h |3 +++
3 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/arch/arm/mach-omap2/usb-host.c b/arch/arm/mach-omap2/usb-host.c
index d1dbe12..2e44e8a 100644
--- a/arch/arm/mach-omap2/usb-host.c
+++ b/arch/arm/mach-omap2/usb-host.c
@@ -508,6 +508,10 @@ void __init usbhs_init(const struct
usbhs_omap_board_data *pdata)
if (cpu_is_omap34xx()) {
setup_ehci_io_mux(pdata-port_mode);
setup_ohci_io_mux(pdata-port_mode);
+
+ if (omap_rev() = OMAP3430_REV_ES2_1)
+ usbhs_data.single_ulpi_bypass = true;
+
} else if (cpu_is_omap44xx()) {
setup_4430ehci_io_mux(pdata-port_mode);
setup_4430ohci_io_mux(pdata-port_mode);
diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index cebfe0a..fe7906b 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -384,7 +384,7 @@ static void omap_usbhs_init(struct device *dev)
reg = ~OMAP_UHH_HOSTCONFIG_P3_CONNECT_STATUS;
/* Bypass the TLL module for PHY mode operation */
- if (cpu_is_omap3430() (omap_rev() = OMAP3430_REV_ES2_1)) {
+ if (pdata-single_ulpi_bypass) {
dev_dbg(dev, OMAP3 ES version = ES2.1\n);
if (is_ehci_phy_mode(pdata-port_mode[0]) ||
is_ehci_phy_mode(pdata-port_mode[1]) ||
diff --git a/include/linux/platform_data/usb-omap.h
b/include/linux/platform_data/usb-omap.h
index 8570bcf..ef65b67 100644
--- a/include/linux/platform_data/usb-omap.h
+++ b/include/linux/platform_data/usb-omap.h
@@ -59,6 +59,9 @@ struct usbhs_omap_platform_data {
struct ehci_hcd_omap_platform_data *ehci_data;
struct ohci_hcd_omap_platform_data *ohci_data;
+
+ /* OMAP3 = ES2.1 have a single ulpi bypass control bit */
+ unsignedsingle_ulpi_bypass:1;
};
/*-*/
--
1.7.4.1
--
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 v2 07/11] xhci: Avoid dead ports, add roothub port polling.
On Tue, 4 Dec 2012, Sarah Sharp wrote: The USB core hub thread (khubd) is designed with external USB hubs in mind. It expects that if a port status change bit is set, the hub will continue to send a notification through the hub status data transfer. Basically, it expects hub notifications to be level-triggered. The xHCI host controller is designed to be edge-triggered. When all port status change bits are clear, and a new change bit is set, the xHC will generate a Port Status Change Event. If another change bit is set in the same port status register before the first bit is cleared, it will not send another event. This explanation is okay, but in general I often find the terms edge-triggered and level-triggered to be rather confusing in this sort of context. The real issue here is what input will turn on the IRQ signal (or cause an event to be raised). With xHCI, that input is the or of all the status-change bits in a port register -- a new IRQ won't be generated until the or changes from 0 to 1. With EHCI, on the other hand, a 0 - 1 change in any of the bits will cause a new IRQ. Both controllers use edge-triggered IRQs. The difference lies in what edges they are monitoring. + /* + * xHCI portsc bits are level-triggered. An event is sent on the first + * change bit, but won't be sent if another change bit is still set. + * Poll to avoid losing change bits. + */ The first two lines of this comment don't express the idea very well. I suggest something more like: /* * xHCI port-status-change events occur when the or of all the * status-change bits in the portsc register changes from 0 to 1. * New status changes won't cause an event if any other change * bits are still set. When an event occurs, switch over to * polling to avoid losing status changes. */ 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 2/2] usb/core: consider link speed while looking at bMaxPower
On Mon, Dec 03, 2012 at 04:28:38PM -0500, Alan Stern wrote:
On Mon, 3 Dec 2012, Sarah Sharp wrote:
This code in hcd.c:usb_add_hcd needs to change:
/* starting here, usbcore will pay attention to this root hub */
rhdev-bus_mA = min(500u, hcd-power_budget);
In fact, this line is wrong. The 500u should be multiplied by the
number of ports -- and of course, it should be larger for SuperSpeed
root hubs.
But I don't know the number of ports here. I could set this here to 500/900
and multiple later in hub_configure() by the number of ports.
Anyway, do you think this is correct? Anything 500 for non-SS will replaced
with 500 in hub_configure:
| if (hdev == hdev-bus-root_hub) {
| if (hdev-bus_mA == 0 || hdev-bus_mA = full_load)
| hub-mA_per_port = full_load;
| else {
| hub-mA_per_port = hdev-bus_mA;
| hub-limited_power = 1;
| }
going through the tree I saw most people set it to something around 200 (if at
all). There are a few omap/musb boards setting it to 500.
The interresting part is davinci of course. So there is comment that says
|irlml6401 switches over 1A in under 8 msec
and it calls davinci_setup_usb(1000, 8); which writes 255 in the power member
which musb in turn multiplies by 2. So we have 500mA again ;) I have no idea
how many ports they have.
The else statement after the above code also needs to change:
} else if ((hubstatus (1 USB_DEVICE_SELF_POWERED)) == 0) {
dev_dbg(hub_dev, hub controller current requirement:
%dmA\n,
hub-descriptor-bHubContrCurrent);
hub-limited_power = 1;
if (hdev-maxchild 0) {
Hmmm... This test doesn't look very important. Who cares about hubs
with no ports?
I made it go away.
int remaining = hdev-bus_mA -
hub-descriptor-bHubContrCurrent;
if (remaining hdev-maxchild * 100)
dev_warn(hub_dev,
insufficient power available
to use all downstream ports\n);
hub-mA_per_port = 100; /* 7.2.1.1 */
I think this is trying to set the mA_per_port to the unconfigured device
power, which 7.2.1.1 says is one unit load.
Not to the unconfigured device power but simply to one unit load.
7.2.1 says:
External ports in a bus-powered hub can supply only one unit
load per port regardless of the current draw on the other ports
of that hub.
The comment actually should refer to 7.2.1, not 7.2.1.1.
Fixed up.
and the others as well. Will post them in a jiffy or two.
Sebastian
--
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 1/2 v2] usb/core: consider link speed while looking at bMaxPower
The USB 2.0 specification says that bMaxPower is the maximum power
consumption expressed in 2 mA units and the USB 3.0 specification says
that it is expressed in 8 mA units.
This patch adds a helper function usb_get_max_power() which computes the
value based on config usb_device's speed value. The sysfs interface
the device descriptor dump compute the value on their own.
Cc: Sarah Sharp sarah.a.sh...@linux.intel.com
Cc: Greg Kroah-Hartman gre...@linuxfoundation.org
Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
---
v1…v2: - moved usb_get_max_power() to usb.h
- removed the multiplier argument from the macro and implemented
show_bMaxPower from scratch
drivers/usb/core/devices.c | 13 ++---
drivers/usb/core/generic.c |2 +-
drivers/usb/core/hub.c |2 +-
drivers/usb/core/message.c |2 +-
drivers/usb/core/sysfs.c | 37 -
drivers/usb/core/usb.h |9 +
6 files changed, 50 insertions(+), 15 deletions(-)
diff --git a/drivers/usb/core/devices.c b/drivers/usb/core/devices.c
index cbacea9..e33224e 100644
--- a/drivers/usb/core/devices.c
+++ b/drivers/usb/core/devices.c
@@ -316,17 +316,23 @@ static char *usb_dump_iad_descriptor(char *start, char
*end,
*/
static char *usb_dump_config_descriptor(char *start, char *end,
const struct usb_config_descriptor *desc,
- int active)
+ int active, int speed)
{
+ int mul;
+
if (start end)
return start;
+ if (speed == USB_SPEED_SUPER)
+ mul = 8;
+ else
+ mul = 2;
start += sprintf(start, format_config,
/* mark active/actual/current cfg. */
active ? '*' : ' ',
desc-bNumInterfaces,
desc-bConfigurationValue,
desc-bmAttributes,
-desc-bMaxPower * 2);
+desc-bMaxPower * mul);
return start;
}
@@ -342,7 +348,8 @@ static char *usb_dump_config(int speed, char *start, char
*end,
if (!config)
/* getting these some in 2.3.7; none in 2.3.6 */
return start + sprintf(start, (null Cfg. desc.)\n);
- start = usb_dump_config_descriptor(start, end, config-desc, active);
+ start = usb_dump_config_descriptor(start, end, config-desc, active,
+ speed);
for (i = 0; i USB_MAXIADS; i++) {
if (config-intf_assoc[i] == NULL)
break;
diff --git a/drivers/usb/core/generic.c b/drivers/usb/core/generic.c
index eff2010..271e761 100644
--- a/drivers/usb/core/generic.c
+++ b/drivers/usb/core/generic.c
@@ -100,7 +100,7 @@ int usb_choose_configuration(struct usb_device *udev)
*/
/* Rule out configs that draw too much bus current */
- if (c-desc.bMaxPower * 2 udev-bus_mA) {
+ if (usb_get_max_power(udev, c) udev-bus_mA) {
insufficient_power++;
continue;
}
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index a815fd2..4735425 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4152,7 +4152,7 @@ hub_power_remaining (struct usb_hub *hub)
/* Unconfigured devices may not use more than 100mA,
* or 8mA for OTG ports */
if (udev-actconfig)
- delta = udev-actconfig-desc.bMaxPower * 2;
+ delta = usb_get_max_power(udev, udev-actconfig);
else if (port1 != udev-bus-otg_port || hdev-parent)
delta = 100;
else
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 131f736..444d30e 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1751,7 +1751,7 @@ int usb_set_configuration(struct usb_device *dev, int
configuration)
}
}
- i = dev-bus_mA - cp-desc.bMaxPower * 2;
+ i = dev-bus_mA - usb_get_max_power(dev, cp);
if (i 0)
dev_warn(dev-dev, new config #%d exceeds power
limit by %dmA\n,
diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c
index 818e4a0..472fcb1 100644
--- a/drivers/usb/core/sysfs.c
+++ b/drivers/usb/core/sysfs.c
@@ -17,7 +17,7 @@
#include usb.h
/* Active configuration fields */
-#define usb_actconfig_show(field, multiplier, format_string) \
+#define usb_actconfig_show(field, format_string) \
static ssize_t show_##field(struct device *dev, \
struct device_attribute *attr, char *buf) \
{
[PATCH 2/2] usb/core: update power budget for SuperSpeed
Sarah pointed out that the USB3.0 spec also updates the amount of power
that may be consumed by the device and quoted 9.2.5.1:
|The amount of current draw for SuperSpeed devices are increased to 150
|mA for low-power devices and 900 mA for high-power
This patch tries to update all users to use the larger values for
SuperSpeed devices and use the old ones for everything else.
While here, two other changes suggested by Alan:
- the comment referering to 7.2.1.1 has been updated to 7.2.1 which is
the correct source of the action.
- the check for hubs with zero ports has been removed.
Signed-off-by: Sebastian Andrzej Siewior bige...@linutronix.de
---
drivers/usb/core/hcd.c |6 +-
drivers/usb/core/hub.c | 45 -
2 files changed, 37 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 4225d5e..d2e0f20 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2506,7 +2506,11 @@ int usb_add_hcd(struct usb_hcd *hcd,
}
/* starting here, usbcore will pay attention to this root hub */
- rhdev-bus_mA = min(500u, hcd-power_budget);
+ if (rhdev-speed == USB_SPEED_SUPER)
+ rhdev-bus_mA = 900;
+ else
+ rhdev-bus_mA = 500;
+ rhdev-bus_mA = min_t(unsigned, rhdev-bus_mA, hcd-power_budget);
if ((retval = register_root_hub(hcd)) != 0)
goto err_register_root_hub;
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 4735425..43ac130 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1292,6 +1292,8 @@ static int hub_configure(struct usb_hub *hub,
unsigned int pipe;
int maxp, ret, i;
char *message = out of memory;
+ unsigned unit_load;
+ unsigned full_load;
hub-buffer = kmalloc(sizeof(*hub-buffer), GFP_KERNEL);
if (!hub-buffer) {
@@ -1338,6 +1340,13 @@ static int hub_configure(struct usb_hub *hub,
}
wHubCharacteristics = le16_to_cpu(hub-descriptor-wHubCharacteristics);
+ if (hub_is_superspeed(hdev)) {
+ unit_load = 150;
+ full_load = 900;
+ } else {
+ unit_load = 100;
+ full_load = 500;
+ }
/* FIXME for USB 3.0, skip for now */
if ((wHubCharacteristics HUB_CHAR_COMPOUND)
@@ -1458,32 +1467,32 @@ static int hub_configure(struct usb_hub *hub,
}
le16_to_cpus(hubstatus);
if (hdev == hdev-bus-root_hub) {
- if (hdev-bus_mA == 0 || hdev-bus_mA = 500)
- hub-mA_per_port = 500;
+ if (hdev-bus_mA == 0 || hdev-bus_mA = full_load)
+ hub-mA_per_port = full_load;
else {
hub-mA_per_port = hdev-bus_mA;
hub-limited_power = 1;
}
} else if ((hubstatus (1 USB_DEVICE_SELF_POWERED)) == 0) {
+ int remaining = hdev-bus_mA -
+ hub-descriptor-bHubContrCurrent;
+
dev_dbg(hub_dev, hub controller current requirement: %dmA\n,
hub-descriptor-bHubContrCurrent);
hub-limited_power = 1;
- if (hdev-maxchild 0) {
- int remaining = hdev-bus_mA -
- hub-descriptor-bHubContrCurrent;
- if (remaining hdev-maxchild * 100)
- dev_warn(hub_dev,
+ if (remaining hdev-maxchild * unit_load)
+ dev_warn(hub_dev,
insufficient power available
to use all downstream ports\n);
- hub-mA_per_port = 100; /* 7.2.1.1 */
- }
+ hub-mA_per_port = unit_load; /* 7.2.1 */
+
} else {/* Self-powered external hub */
/* FIXME: What about battery-powered external hubs that
* provide less current per port? */
- hub-mA_per_port = 500;
+ hub-mA_per_port = full_load;
}
- if (hub-mA_per_port 500)
+ if (hub-mA_per_port full_load)
dev_dbg(hub_dev, %umA bus power budget for each child\n,
hub-mA_per_port);
@@ -4145,16 +4154,21 @@ hub_power_remaining (struct usb_hub *hub)
for (port1 = 1; port1 = hdev-maxchild; ++port1) {
struct usb_device *udev = hub-ports[port1 - 1]-child;
int delta;
+ unsignedunit_load;
if (!udev)
continue;
+ if (hub_is_superspeed(udev))
+ unit_load = 150;
+ else
+ unit_load = 100;
/* Unconfigured devices may not use more than 100mA,
Re: [RFC v2 10/11] USB: Rip out recursive call on warm port reset.
On Tue, 4 Dec 2012, Sarah Sharp wrote:
When a hot reset fails on a USB 3.0 port, the current port reset code
recursively calls hub_port_reset inside hub_port_wait_reset. This isn't
ideal, since we should avoid recursive calls in the kernel, and it also
doesn't allow us to issue multiple warm resets on reset failures.
Rip out the recursive call. Instead, add code to hub_port_reset to
issue a warm reset if the hot reset fails, and try multiple warm resets
before giving up on the port.
This is better than before. There is still a little bit I don't quite
follow...
In hub_port_wait_reset, remove the recursive call and re-indent. The
code is basically the same, except:
1. It bails out early if the port has transitioned to Inactive or
Compliance Mode after the reset completed.
2. It doesn't consider a connect status change to be a failed reset. If
multiple warm resets needed to be issued, the connect status may have
changed, so we need to ignore that and look at the port link state
instead. hub_port_reset will now do that.
In fact, we might as well do the same thing for non-SuperSpeed
connections too. The kernel probably is smart enough not to get
confused if the user manages to replace one device with another while a
reset is in progress.
3. It unconditionally sets udev-speed on all types of successful
resets. The old recursive code would set the port speed when the second
hub_port_reset returned.
With the new code in hub_port_reset, the 'warm' variable may change on a
transition from a hot reset to a warm reset. So hub_port_finish_reset
could be called with 'warm' set to true when a connected USB device has
been reset. Fix the code by unconditionally updating the device number,
informing the host that the device has been reset, and setting the
device state to default.
Why was it necessary to skip these things before, when warm was set?
Was it merely an optimization? If not, do the same reasons still
apply?
In hub_port_finish_reset, unconditionally clear the connect status
change (CSC) bit for USB 3.0 hubs when the port reset is done. If we
had to issue multiple warm resets for a device, that bit may have been
set if the device went into SS.Inactive and then was successfully warm
reset.
Signed-off-by: Sarah Sharp sarah.a.sh...@linux.intel.com
---
drivers/usb/core/hub.c | 159 +--
1 files changed, 71 insertions(+), 88 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index d8de712..a502857 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2538,80 +2538,39 @@ static int hub_port_wait_reset(struct usb_hub *hub,
int port1,
- if (!(portstatus USB_PORT_STAT_CONNECTION) ||
- hub_port_warm_reset_required(hub,
- portstatus))
- return -ENOTCONN;
+ /* if we've finished resetting, then break out of
+ * the loop
+ */
+ if (!(portstatus USB_PORT_STAT_RESET)
This test is redundant thanks to your 5/11 patch, right?
@@ -2703,10 +2663,33 @@ static int hub_port_reset(struct usb_hub *hub, int
port1,
status);
}
- /* return on disconnect or reset */
+ /* Check for disconnect or reset */
if (status == 0 || status == -ENOTCONN || status == -ENODEV) {
- hub_port_finish_reset(hub, port1, udev, status, warm);
- goto done;
+ hub_port_finish_reset(hub, port1, udev, status);
+
+ if (!hub_is_superspeed(hub-hdev))
+ goto done;
+
+ /*
+ * If a USB 3.0 device migrates from reset to an error
+ * state, re-issue the warm reset.
+ */
+ if (hub_port_status(hub, port1,
+ portstatus, portchange) 0)
+ goto done;
Hmmm. Is there any reasonable way to get the portstatus value from
hub_port_finish_reset instead of asking for it again?
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 v2 11/11] USB: Fix connected device switch to Inactive state.
On Tue, 4 Dec 2012, Sarah Sharp wrote:
A USB 3.0 device can transition to the Inactive state if a U1 or U2 exit
transition fails. The current code in hub_events simply issues a warm
reset, but does not call any pre-reset or post-reset driver methods (or
unbind/rebind drivers without them). Therefore the drivers won't know
their device has just been reset.
hub_events should instead call usb_reset_device. This means
hub_port_reset now needs to figure out whether it should issue a warm
reset or a hot reset.
Remove the FIXME note about needing disconnect() for a NOTATTACHED
device. This patch fixes that.
Actually, I think it was fixed many years ago but the FIXME comment
was never removed. :-)
Signed-off-by: Sarah Sharp sarah.a.sh...@linux.intel.com
---
drivers/usb/core/hub.c | 29 -
1 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index a502857..9341302 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2609,7 +2609,6 @@ static void hub_port_finish_reset(struct usb_hub *hub,
int port1,
case -ENODEV:
clear_port_feature(hub-hdev,
port1, USB_PORT_FEAT_C_RESET);
- /* FIXME need disconnect() for NOTATTACHED device */
if (hub_is_superspeed(hub-hdev)) {
clear_port_feature(hub-hdev, port1,
USB_PORT_FEAT_C_BH_PORT_RESET);
@@ -2643,6 +2642,18 @@ static int hub_port_reset(struct usb_hub *hub, int
port1,
* Some companion controllers don't like it when they mix.
*/
down_read(ehci_cf_port_reset_rwsem);
+ } else if (!warm) {
+ /*
+ * If the caller hasn't explicitly requested a warm reset,
+ * double check and see if one is needed.
+ */
+ status = hub_port_status(hub, port1,
+ portstatus, portchange);
+ if (status 0)
+ goto done;
+
+ if (hub_port_warm_reset_required(hub, portstatus))
+ warm = true;
Would it be better to turn warm into a 3-valued argument: true,
false, or dont_know? Then this test could be skipped if the caller
wanted to do a regular reset.
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 2/2] usb/core: consider link speed while looking at bMaxPower
On Wed, 5 Dec 2012, Sebastian Andrzej Siewior wrote:
On Mon, Dec 03, 2012 at 04:28:38PM -0500, Alan Stern wrote:
On Mon, 3 Dec 2012, Sarah Sharp wrote:
This code in hcd.c:usb_add_hcd needs to change:
/* starting here, usbcore will pay attention to this root hub */
rhdev-bus_mA = min(500u, hcd-power_budget);
In fact, this line is wrong. The 500u should be multiplied by the
number of ports -- and of course, it should be larger for SuperSpeed
root hubs.
But I don't know the number of ports here.
Ah, right. That's probably a large part of the reason it wasn't done
correctly. Or maybe I just wasn't thinking clearly when I wrote it...
I could set this here to 500/900
and multiple later in hub_configure() by the number of ports.
Better yet, move that entire bus_mA calculation from usb_add_hcd to
hub_configure. It should be something like this:
if (hdev == hdev-bus-root_hub) {
+ if (hcd-power_budget 0)
+ hdev-bus_mA = hcd-power_budget;
+ else
+ hdev-bus_mA = full_load * hdev-maxchild;
+ if (hdev-bus_mA = full_load)
- if (hdev-bus_mA == 0 || hdev-bus_mA = full_load)
hub-mA_per_port = full_load;
else {
hub-mA_per_port = hdev-bus_mA;
hub-limited_power = 1;
}
Leaving rhdev-bus_mA set to 0 for a while shouldn't hurt anything,
because our config descriptors for root hubs all have bMaxPower equal
to 0.
In case this isn't clear (which seems quite likely), hdev-bus_mA
represents the total amount of current available for use by the hub
itself plus its downstream devices. So on a regular PC, where each
root-hub port can supply a full load, we say that the root hub uses no
power and hdev-bus_mA is a full load times the number of ports.
This all makes more sense if you think about external hubs.
Anyway, do you think this is correct? Anything 500 for non-SS will replaced
with 500 in hub_configure:
| if (hdev == hdev-bus-root_hub) {
| if (hdev-bus_mA == 0 || hdev-bus_mA = full_load)
| hub-mA_per_port = full_load;
| else {
| hub-mA_per_port = hdev-bus_mA;
| hub-limited_power = 1;
| }
True, hub-mA_per_port isn't affected by the multiplication. But look
at the calculation in hub_power_remaining -- it won't work right if
hdev-bus_mA isn't set correctly.
going through the tree I saw most people set it to something around 200 (if at
all). There are a few omap/musb boards setting it to 500.
People haven't paid all that much attention to it. Most likely it
doesn't matter all that much except for OTG hosts.
The interresting part is davinci of course. So there is comment that says
|irlml6401 switches over 1A in under 8 msec
and it calls davinci_setup_usb(1000, 8); which writes 255 in the power member
which musb in turn multiplies by 2. So we have 500mA again ;) I have no idea
how many ports they have.
Hopefully they will work correctly with these changes regardless.
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 1/2 v2] usb/core: consider link speed while looking at bMaxPower
On Wed, 5 Dec 2012, Sebastian Andrzej Siewior wrote: The USB 2.0 specification says that bMaxPower is the maximum power consumption expressed in 2 mA units and the USB 3.0 specification says that it is expressed in 8 mA units. This patch adds a helper function usb_get_max_power() which computes the value based on config usb_device's speed value. The sysfs interface the device descriptor dump compute the value on their own. Why doesn't the sysfs function use usb_get_max_power? Am I missing something? 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 2/2] usb/core: update power budget for SuperSpeed
On Wed, 5 Dec 2012, Sebastian Andrzej Siewior wrote:
Sarah pointed out that the USB3.0 spec also updates the amount of power
that may be consumed by the device and quoted 9.2.5.1:
|The amount of current draw for SuperSpeed devices are increased to 150
|mA for low-power devices and 900 mA for high-power
This patch tries to update all users to use the larger values for
SuperSpeed devices and use the old ones for everything else.
While here, two other changes suggested by Alan:
- the comment referering to 7.2.1.1 has been updated to 7.2.1 which is
the correct source of the action.
- the check for hubs with zero ports has been removed.
@@ -4145,16 +4154,21 @@ hub_power_remaining (struct usb_hub *hub)
for (port1 = 1; port1 = hdev-maxchild; ++port1) {
struct usb_device *udev = hub-ports[port1 - 1]-child;
int delta;
+ unsignedunit_load;
if (!udev)
continue;
+ if (hub_is_superspeed(udev))
+ unit_load = 150;
+ else
+ unit_load = 100;
/* Unconfigured devices may not use more than 100mA,
* or 8mA for OTG ports */
This comment should be updated too (one unit load instead of 100mA).
BTW, does OTG support SuperSpeed?
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 v4] usb_8dev: Add support for USB2CAN interface from 8 devices
On 12/04/2012 11:43 PM, Bernd Krumboeck wrote:
Add device driver for USB2CAN interface from 8 devices
(http://www.8devices.com).
[...]
+/* Send data to device */
+static netdev_tx_t usb_8dev_start_xmit(struct sk_buff *skb,
+ struct net_device *netdev)
+{
+struct usb_8dev *dev = netdev_priv(netdev);
I just noticed, it's unusual to name you private data pointer dev, I
suggest to name it priv. Maybe you can rename your private data struct
to: struct usb_8dev_priv.
+struct net_device_stats *stats = netdev-stats;
+struct can_frame *cf = (struct can_frame *) skb-data;
+struct usb_8dev_tx_msg *msg;
+struct urb *urb;
+struct usb_8dev_tx_urb_context *context = NULL;
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [PATCH v3] usb_8dev: Add support for USB2CAN interface from 8 devices
On 12/05/2012 06:36 PM, Bernd Krumboeck wrote:
Hi Marc!
+default:
+netdev_info(netdev, Rx URB aborted (%d)\n,
+ urb-status);
+goto resubmit_urb;
+}
+
+while (pos urb-actual_length) {
+struct usb_8dev_rx_msg *msg;
+
+if (pos + sizeof(struct usb_8dev_rx_msg)
urb-actual_length) {
+netdev_err(dev-netdev, format error\n);
+break;
is resubmitting the urb the correct way to handle this problem?
Suggestions? (maybe CAN_ERR_CRTL_UNSPEC ??)
It's not an error on the CAN protocol level, but the USB communication
is broken. I just had a look at the kvaser usb driver, it's doing a
resubmit, too. So it seems to be okay.
+
+stats-tx_dropped++;
+}
+} else {
+/* Slow down tx path */
+if (atomic_read(dev-active_tx_urbs) = MAX_TX_URBS ||
+dev-free_slots 5) {
where's the 5 coming from?
From ems_usb driver.
H, is the variable free_slots used?
+netif_stop_queue(netdev);
+}
+}
+
+/*
+ * Release our reference to this URB, the USB core will eventually
free
+ * it entirely.
+ */
+usb_free_urb(urb);
+
+return NETDEV_TX_OK;
+
+nomem:
+dev_kfree_skb(skb);
+stats-tx_dropped++;
+
+return NETDEV_TX_OK;
+}
+
+static int usb_8dev_get_berr_counter(const struct net_device *netdev,
+ struct can_berr_counter *bec)
+{
+struct usb_8dev *dev = netdev_priv(netdev);
+
+bec-txerr = dev-bec.txerr;
+bec-rxerr = dev-bec.rxerr;
+
+return 0;
+}
+
+/* Start USB device */
+static int usb_8dev_start(struct usb_8dev *dev)
+{
+struct net_device *netdev = dev-netdev;
+int err, i;
+
+dev-free_slots = 15; /* initial size */
there does the 15 come from?
ditto
dito :)
+ * Check device and firmware.
+ * Set supported modes and bittiming constants.
+ * Allocate some memory.
+ */
+static int usb_8dev_probe(struct usb_interface *intf,
+ const struct usb_device_id *id)
+{
+struct net_device *netdev;
+struct usb_8dev *dev;
+int i, err = -ENOMEM;
+u32 version;
+char buf[18];
where does this 18 come from?
String USB2CAN converter + trailing 0.
okay
+struct usb_device *usbdev = interface_to_usbdev(intf);
+
+/* product id looks strange, better we also check iProdukt
string */
iProduct?
I mean typo iProdukt vs iProduct.
Check if usbdev-descriptor.iProduct == USB2CAN converter.
+if (usb_string(usbdev, usbdev-descriptor.iProduct, buf,
+ sizeof(buf)) 0 strcmp(buf, USB2CAN converter)) {
+dev_info(usbdev-dev, ignoring: not an USB2CAN converter\n);
+return -ENODEV;
+}
+
+netdev = alloc_candev(sizeof(struct usb_8dev), MAX_TX_URBS);
+if (!netdev) {
+dev_err(intf-dev, Couldn't alloc candev\n);
+return -ENOMEM;
+}
+
+dev = netdev_priv(netdev);
+
+dev-udev = usbdev;
+dev-netdev = netdev;
+
+dev-can.state = CAN_STATE_STOPPED;
+dev-can.clock.freq = USB_8DEV_ABP_CLOCK;
+dev-can.bittiming_const = usb_8dev_bittiming_const;
+dev-can.do_set_mode = usb_8dev_set_mode;
+dev-can.do_get_berr_counter = usb_8dev_get_berr_counter;
+dev-can.ctrlmode_supported = CAN_CTRLMODE_LOOPBACK |
+ CAN_CTRLMODE_LISTENONLY |
+ CAN_CTRLMODE_ONE_SHOT;
Have you actually tested one shot? What happens if a can frame cannot be
transmitted?
Not really. Can someone explain what one-shot exactly means and what is
the correct behavior.
I've only tested without any other device connected. I got the same
errors like in normal mode.
Can has a built in collision avoidance protocol. If a collision is about
to happen the sender with the higher CAN id will back of and try again
later. In one shot mode it will not try again. The question is, how
behaves the dongle firmware, as you have to free your tx message somehow.
To test, simply send with one station with canid 1, the other one in
one-shot-mode with canid 0x7ff.
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions| Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917- |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
signature.asc
Description: OpenPGP digital signature
Re: [RFC PATCH 4/5] arm: omap2: support port power on lan95xx devices
On 06/12/12 00:42, the mail apparently from Alan Stern included:
On Wed, 5 Dec 2012, Andy Green wrote:
The details of this aren't clear yet. For instance, should the device
core try to match the port with the asset info, or should this be done
by the USB code when the port is created?
Currently what I have (this is before changing it to pm domain, but this
should be unchanged) lets the asset define a callback op which will
receive notifications for all added child devices that have the device
the asset is bound to as an ancestor.
So you would bind an asset to the host controller device...
static struct device_asset assets_ehci_omap0[] = {
{
.name = -0:1.0/port1,
.asset = assets_ehci_omap0_smsc_port,
.ops = device_descendant_attach_default_asset_ops,
},
{ }
};
...with this descendant filter callback pointing to a generic end of
the device path matcher.
when device_descendant_attach_default_asset_ops() sees the child that
was foretold has appeared (and it only hears about children of
ehci-omap.0 in this case), it binds the assets pointed to by .asset to
the new child before its probe. assets_ehci_omap0_smsc_port is an array
of the actual regulator and clock that need switching by the child. So
the effect is to magic the right assets to the child device just before
it gets added (and probe called etc).
This is working well and the matcher helper is small and simple.
Right. The question is should it be done in this somewhat roundabout
way (you've got two separate assets for setting up this one thing), or
should the USB port code be responsible for explicitly checking if
there are any applicable assets when the port is created?
The advantange of the second approach is that it doesn't involve
checking all the ancestors every time a new device is added (and it
involves only one asset). The disadvantage is that it works only for
USB ports.
It's done in two steps to strongly filter candidate child devices
against being children of a known platform device. If you go around
that, you will be back to full device path matching with wildcards at
the USB child to determine if he is the one. So that's a feature not
an issue I think.
I can see doing it generically or not is equally a political issue as a
technical one, but I think if we bother to add this kind of support we
should prefer to do it generally. It's going to be about the same
amount of code.
As Tony Lindgren said, even board-omap4panda.c itself is deprecated, to
project platform info into USB stack you either have to add DT code into
usb stack then to go find things directly, or provide a generic
methodology like assets which have the dt bindings done outside of any
subsystem and apply their operations outside the subsystem too.
To answer the question, we need to know how other subsystems might
want to use the asset-matching approach. My guess is that only a small
number of device types would care about assets (in the same way that
assets matter to USB ports but not to other USB device types). And it
might not be necessary to check against every ancestor; we might know
beforehand where to check (just as the USB port would know to look for
an asset attached to the host controller device).
Yes I think it boils down to buses in general can benefit from this.
They're the thing that dynamically - later - create child devices you
might need to target with what was ultimately platform information.
On Panda the other bus thing that can benefit is the WLAN module on
SDIO. In fact that's a very illuminating case for your question above.
Faced with exactly the same problem, that they needed to project
platform info on to SDIO-probed device instance to tell it what clock
rate it had been given, they invented an api which you can see today in
board-omap4panda.c and other boards there, wl12xx_set_platform_data().
This is from board-4430sdp:
static struct wl12xx_platform_data omap4_sdp4430_wlan_data __initdata = {
.board_ref_clock = WL12XX_REFCLOCK_26,
.board_tcxo_clock = WL12XX_TCXOCLOCK_26,
};
...
ret = wl12xx_set_platform_data(omap4_sdp4430_wlan_data);
You can find the other end of it here in
drivers/net/wireless/ti/wlcore/wl12xx_platform_data.c
static struct wl12xx_platform_data *platform_data;
int __init wl12xx_set_platform_data(const struct wl12xx_platform_data *data)
{
if (platform_data)
return -EBUSY;
if (!data)
return -EINVAL;
platform_data = kmemdup(data, sizeof(*data), GFP_KERNEL);
if (!platform_data)
return -ENOMEM;
return 0;
}
when the driver for the device instance wants to get its platform data
it reads the static pointer via another api. Obviously if you want two
modules on your platform that's not so good.
I doubt that's the only SDIO device that wants to know platform info.
So I think by
Re: Correlating SOF with host system time
On Wed, Dec 5, 2012 at 8:58 AM, Stefan Tauner stefan.tau...@student.tuwien.ac.at wrote: On Tue, 4 Dec 2012 16:27:00 -0500 (EST) Alan Stern st...@rowland.harvard.edu wrote: I don't think referencing times to SOF packets is the best approach, although it probably is the approach that would yield the most precision. How precise do you want your synchronization to be? It should be below 1ms accuracy, but being more precise does never hurt of course :) BTW anyone interested in really accurate USB synchronization should read the paper Sub-nanosecond Distributed Synchronisation via the Universal Serial Bus; they use custom hardware though. Interestingly you mentioned this article. Back in 2008, we have some discussions about this topic in Microchip forum. http://www.microchip.com/forums/tm.aspx?m=329799 Running NTP over a USB-based network link would certainly be the easiest solution, if your device can support it. Over the long run, it might even be more accurate on average than using SOF packets. We are talking about microcontrollers with a few kB RAM at most, so just cross-compiling any (S)NTP client wont cut it probably :) So the SOF approach seems to be way more elegant and easy to implement to me... in theory. Another nice property is that SOF packets are sent for every frame in any case (but errors) and they are broadcasted, which is both advantageous regarding bandwidth usage and independent of the number of devices attached. Regarding long-term stability/precision i am not sure i can agree with you. If i throw the same amount of statistics/algorithm complexity at the SOF scheme and a software-exclusive approach i dont see how the latter could be better :) You might find some of the information in the Microchip forum helpful. -- Xiaofan -- 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: [llvmlinux] [PATCH V2 2/3] Remove VLAIS usage from gadget code
On 2012.12.04 23.24, Sebastian Andrzej Siewior wrote: VLAIS is not something they are willing to accept (for various reasons). There are other patches to LLVM that are still working Is this not described in C99 6.7.2.1p16? No, this is not the case - flexible array members are a very different beast, which are in-and-of-themselves of debatable merit. C99 6.7.2.1p8 explicitly states that VLAIS are illegal. The same language is present in C11 (same section): A member of a structure or union may have any object type other than a variably modified type. In C99 6.7.5.1p3, the definition of a variably modified type is given: .. [snip] ... If the nested sequence of declarators in a full declarator contains a variable length array type, the type specified by the full declarator is said to be variably modified. It would require incredible, earth-moving acts for VLAIS to become a part of the ISO/IEC standard for the C programming language. P.S. As a general note, while I do agree that it is preferable to rewrite code using VLAIS instead of utilizing a macro, I would ask that some reconsideration be given, unless there are Linux kernel developers who are willing and able to implement the necessary rewrites. The LLVMLinux project does not wish to decrease the code quality of the Linux kernel, and certainly we do not intend to cause any GCC breakage. However, I think that rewrites to all the relevant VLAIS code would be quite time consuming for the LLVMLinux team, and the proposed macro does not appear to me to present any particular evil, other than by nature of being a macro. If faced between the choice of either accepting this VLAIS patch, or not being able to compile the Linux kernel with standard-compliant, largely GCC-compatible compilers such as Clang and icc, I would think that accepting the VLAIS patch would be preferable. (note also that I have only come in on the end of this thread, and I may have missed part of the context). -- Bryce Adelstein-Lelbach aka wash STE||AR Group, Center for Computation and Technology, LSU -- 860-808-7497 - Cell 225-578-6182 - Work (no voicemail) -- stellar.cct.lsu.edu boost-spirit.com llvm.linuxfoundation.org -- signature.asc Description: Digital signature
Re: Unreliable USB3 with NEC uPD720200 and Delock Cardreader
On Wed, Nov 28, 2012 at 02:54:06PM -0800, Sarah Sharp wrote: On Mon, Nov 26, 2012 at 10:48:03PM +0100, Bjørn Mork wrote: Sarah Sharp sarah.a.sh...@linux.intel.com writes: It looks like both Ulrich and Andrew have the same issue. I also have a Lenovo x220, and I confirmed that when I turn on PCI runtime suspend, the NEC host controller does not report port status changes when a new USB device is plugged in. I'm running 3.6.7, and I'm pretty sure that runtime suspend worked for the NEC host on some older kernel. I don't think the NEC host went into D3cold on that kernel, though. Is there a way to disable D3cold and just use D3hot instead? Yes, you have /sys/bus/pci/devices/.../d3cold_allowed See Documentation/ABI/testing/sysfs-bus-pci If this really is a problem with the D3cold support that went into 3.6 then I guess you should include Huang Ying in the discussions as well (CCed). Turning off D3 cold didn't help. Once the PCI device is suspended, connect events do not generate an interrupt. I'll go see if I can figure out which kernel this worked on and bisect. Wakeup from D3 works fine on the 3.5.0 kernel, but fails on 3.6.2. I haven't fully bisected yet. In debugging, I found that if you only enable runtime suspend for the NEC host controller, the host successfully comes out of D3 when you plug in a USB device. However, if you enable runtime PM for the parent PCIe root port, it stops working. Disabling D3cold for both devices did not help. It looks like a PCI issue, so what sort of debugging info do you need from me? Sarah Sharp -- 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 PATCH 4/5] arm: omap2: support port power on lan95xx devices
Hi, Well, I'm not any less busy than yesterday, as it turns out, but I'm expecting that to continue tomorrow, so I may as well have a look at it now. :-) On Tuesday, December 04, 2012 12:10:32 PM Alan Stern wrote: [CC: list trimmed; the people who were on it should be subscribed to at least one of the lists anyway.] [...] Not only regulators involved, clock or other things might be involved too. Also the same power domain might be shared with more than one port, so it is better to introduce power domain to the problem. Looks generic_pm_domain is overkill, so I introduced power controller which only focuses on power on/off and being shared by multiple devices. Even though it is overkill, it may be better than introducing a new abstraction. After all, this is exactly the sort of thing that PM domains were originally created to handle. Rafael, do you have any advice on this? The generic_pm_domain structure is fairly complicated, there's a lot of code in drivers/base/power/domain.c (it's the biggest source file in its directory), and I'm not aware of any documentation. Yeah, documentation is missing, which obviously is my fault. That code is designed to cover the case in which multiple devices share a power switch that can be used to remove power from all of them at once (eg. through a register write). It also assumes that there will be a stop operation, such as disable clock, allowing each device in the domain to be put into a shallow low-power state (individually) without the necessity to save its state. Device states only have to be saved when the power switch is about to be used, which generally happens when they all have been stopped (their runtime PM status is RPM_SUSPENDED). The stop operation may be defined in a different way for each device in the domain (actually, that applies to the save state, restore state, and start - opposite to stop - operations too) and PM QoS latency constraints are used to decide if and when to turn the whole domain off. Moreover, it supports hierarchies of power domains that may be pretty much arbitarily complicated. A big part of this code is for the handling of system suspend/resume in such a way that it is consistent with runtime PM. For USB ports I'd recommend to use something simpler. :-) Thanks, Rafael -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. -- 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: Correlating SOF with host system time
On Thu, 6 Dec 2012 08:16:26 +0800 Xiaofan Chen xiaof...@gmail.com wrote: Interestingly you mentioned this article. Back in 2008, we have some discussions about this topic in Microchip forum. http://www.microchip.com/forums/tm.aspx?m=329799 My favorite search engine told me about it of course while i did my research :) Sadly no one posted some results of what he tried, but thanks for mentioning it. The biggest problem i noticed with PTP/IEEE1588 scheme is that it relies on the presumption that transmission delays are equal in both directions. This is probably way too optimistic for USB(?). OTOH it might not matter much depending on the precision needs. -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner -- 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 PATCH 1/5] Device Power: introduce power controller
On Thu, Dec 6, 2012 at 12:49 AM, Roger Quadros rog...@ti.com wrote: On 12/03/2012 05:00 AM, Ming Lei wrote: On Mon, Dec 3, 2012 at 12:02 AM, Andy Green andy.gr...@linaro.org wrote: On 02/12/12 23:01, the mail apparently from Ming Lei included: Power controller is an abstract on simple power on/off switch. One power controller can bind to more than one device, which provides power logically, for example, we can think one usb port in hub provides power to the usb device attached to the port, even though the power is supplied actually by other ways, eg. the usb hub is a self-power device. From hardware view, more than one device can share one power domain, and power controller can power on if one of these devices need to provide power, and power off if all these devices don't need to provide power. What stops us using struct regulator here? If you have child regulators supplied by a parent supply, isn't that the right semantic already without introducing a whole new thing? Apologies if I missed the point. There are two purposes: One is to hide the implementation details of the power controller because the user doesn't care how it is implemented, maybe clock, regulator, gpio and other platform dependent stuffs involved, so the patch simplify the usage from the view of users. Which user are you talking about? Here it is the usb port device. At least, there are many boards which have hardwired and self-powered usb devices, so in theory they can benefits from the power controller. Maybe only regulator and clock can't be covered completely for other boards. The patch can make usb port deal with the 'power controller' only, and make it avoid to deal with regulators/clocks/... directly. Thanks, -- Ming Lei -- 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: Correlating SOF with host system time
On Wed, 5 Dec 2012 10:35:31 -0500 (EST) Alan Stern st...@rowland.harvard.edu wrote: On Wed, 5 Dec 2012, Stefan Tauner wrote: Running NTP over a USB-based network link would certainly be the easiest solution, if your device can support it. Over the long run, it might even be more accurate on average than using SOF packets. We are talking about microcontrollers with a few kB RAM at most, so just cross-compiling any (S)NTP client wont cut it probably :) So the SOF approach seems to be way more elegant and easy to implement to me... in theory. Your theory omits an important point: The host system generally doesn't care exactly when frames start. Forcing it to keep track of these events adds overhead. Yes, it does, but that overhead is largely negligible when you have an overpowered x86 machine on one side and a number of tiny microcontrollers on the other. :) Another nice property is that SOF packets are sent for every frame in any case (but errors) and they are broadcasted, which is both advantageous regarding bandwidth usage and independent of the number of devices attached. It may not be all that advantageous after all, because you still have to unicast the information for relating timestamps to SOF packets. Since you have to do this anyway, you might as well use those unicast packets for synchronization instead of the SOF packets. Yes, but OTOH the timestamp information is not needed that often once the clocks are in sync (and kept that way, e.g. by good enough oscillators or by SOF packets). But that's also true for a scheme w/o frame numbers; so i think that's what i will end up trying first: averaging the latency by sending a burst of messages forth and back, then continuing to sync via SOF packets for a while (if that's necessary), repeat; and see how far i get with that or a variation of it. Thanks for the inputs and ill report back, when i have some interesting findings. I would still like to know how one would/should implement the frame number-based scheme in a current linux kernel if anyone wants to elaborate on that... -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner -- 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 PATCH 1/5] Device Power: introduce power controller
On 6 December 2012 06:57, Ming Lei tom.leim...@gmail.com wrote: On Thu, Dec 6, 2012 at 12:49 AM, Roger Quadros rog...@ti.com wrote: On 12/03/2012 05:00 AM, Ming Lei wrote: On Mon, Dec 3, 2012 at 12:02 AM, Andy Green andy.gr...@linaro.org wrote: On 02/12/12 23:01, the mail apparently from Ming Lei included: Power controller is an abstract on simple power on/off switch. One power controller can bind to more than one device, which provides power logically, for example, we can think one usb port in hub provides power to the usb device attached to the port, even though the power is supplied actually by other ways, eg. the usb hub is a self-power device. From hardware view, more than one device can share one power domain, and power controller can power on if one of these devices need to provide power, and power off if all these devices don't need to provide power. What stops us using struct regulator here? If you have child regulators supplied by a parent supply, isn't that the right semantic already without introducing a whole new thing? Apologies if I missed the point. There are two purposes: One is to hide the implementation details of the power controller because the user doesn't care how it is implemented, maybe clock, regulator, gpio and other platform dependent stuffs involved, so the patch simplify the usage from the view of users. Which user are you talking about? Here it is the usb port device. At least, there are many boards which have hardwired and self-powered usb devices, so in theory they can benefits from the power controller. Maybe only regulator and clock can't be covered completely for other boards. The patch can make usb port deal with the 'power controller' only, and make it avoid to deal with regulators/clocks/... directly. I am curious too, except for clocks and voltage supplies (regulators), what other external resources does a chip need ? -- 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] usb_8dev: Add support for USB2CAN interface from 8 devices
Hi Mark!
Am 2012-12-05 22:40, schrieb Marc Kleine-Budde:
On 12/05/2012 06:36 PM, Bernd Krumboeck wrote:
Hi Marc!
+default:
+netdev_info(netdev, Rx URB aborted (%d)\n,
+ urb-status);
+goto resubmit_urb;
+}
+
+while (pos urb-actual_length) {
+struct usb_8dev_rx_msg *msg;
+
+if (pos + sizeof(struct usb_8dev_rx_msg)
urb-actual_length) {
+netdev_err(dev-netdev, format error\n);
+break;
is resubmitting the urb the correct way to handle this problem?
Suggestions? (maybe CAN_ERR_CRTL_UNSPEC ??)
It's not an error on the CAN protocol level, but the USB communication
is broken. I just had a look at the kvaser usb driver, it's doing a
resubmit, too. So it seems to be okay.
I suggested CAN_ERR_CRTL_UNSPEC not CAN_ERR_PROT_UNSPEC. ;-)
+
+stats-tx_dropped++;
+}
+} else {
+/* Slow down tx path */
+if (atomic_read(dev-active_tx_urbs) = MAX_TX_URBS ||
+dev-free_slots 5) {
where's the 5 coming from?
From ems_usb driver.
H, is the variable free_slots used?
Ahm, no.
Have you actually tested one shot? What happens if a can frame cannot be
transmitted?
Not really. Can someone explain what one-shot exactly means and what is
the correct behavior.
I've only tested without any other device connected. I got the same
errors like in normal mode.
Can has a built in collision avoidance protocol. If a collision is about
to happen the sender with the higher CAN id will back of and try again
later. In one shot mode it will not try again. The question is, how
behaves the dongle firmware, as you have to free your tx message somehow.
To test, simply send with one station with canid 1, the other one in
one-shot-mode with canid 0x7ff.
Thanks for the explanation, maybe Gerd can test. I still have no other device.
:-(
regards,
Bernd
--
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] usb_8dev: Add support for USB2CAN interface from 8 devices
Hi Marc!
Am 2012-12-04 22:45, schrieb Marc Kleine-Budde:
+
+/* create a URB, and a buffer for it */
+urb = usb_alloc_urb(0, GFP_KERNEL);
+if (!urb) {
+netdev_err(netdev, No memory left for URBs\n);
+return -ENOMEM;
who will free the already allocated urbs if i != 0 ?
This function tries to submit 10 urbs (MAX_RX_URBS) for receiving. When we
could not
submit all, we still proceed as long as we could submit at least 1.
We must not free, because we should break the loop not return from the function.
I'll correct.
+}
+
+buf = usb_alloc_coherent(dev-udev, RX_BUFFER_SIZE, GFP_KERNEL,
+ urb-transfer_dma);
+if (!buf) {
+netdev_err(netdev, No memory left for USB buffer\n);
+usb_free_urb(urb);
same problem here for coherent
dito.
+return -ENOMEM;
+}
+
+usb_fill_bulk_urb(urb, dev-udev,
+ usb_rcvbulkpipe(dev-udev,
+ USB_8DEV_ENDP_DATA_RX),
+ buf, RX_BUFFER_SIZE,
+ usb_8dev_read_bulk_callback, dev);
+urb-transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+usb_anchor_urb(urb, dev-rx_submitted);
+
+err = usb_submit_urb(urb, GFP_KERNEL);
+if (err) {
+if (err == -ENODEV)
+netif_device_detach(dev-netdev);
+
+usb_unanchor_urb(urb);
+usb_free_coherent(dev-udev, RX_BUFFER_SIZE, buf,
+ urb-transfer_dma);
same here
add a loop that runs backwards at the end of the function
dito.
+break;
+}
+
+/* Drop reference, USB core will take care of freeing it */
+usb_free_urb(urb);
+}
+
+/* Did we submit any URBs */
+if (i == 0) {
+netdev_warn(netdev, couldn't setup read URBs\n);
+return err;
+}
+
Marc
regards,
Bernd
--
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: musb: set TXMAXP and AUTOSET for full speed bulk in device mode
From: supriya karanth supriya.kara...@stericsson.com
The TXMAXP register is not set correctly for full speed bulk case
when the can_bulk_split() is used. Without this PIO transfers will
not take place correctly
The mult factor needs to be updated correctly for the
can_bulk_split() case
The AUTOSET bit in the TXCSR is not being set if the mult
factor is greater than 0 for the High Bandwidth ISO case.
But the mult factor is also greater than 0 in case of Full speed
bulk transfers with the packet splitting in TXMAXP register
Without the AUTOSET the DMA transfers will not progress in mode1
Signed-off-by: supriya karanth supriya.kara...@stericsson.com
Signed-off-by: Praveena NADAHALLY praveen.nadaha...@stericsson.com
Acked-by: Linus Walleij linus.wall...@linaro.org
---
drivers/usb/musb/musb_gadget.c | 20 ++--
1 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index b6b84da..8fb0c1f 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -408,7 +408,19 @@ static void txstate(struct musb *musb, struct musb_request
*req)
csr |= (MUSB_TXCSR_DMAENAB
| MUSB_TXCSR_DMAMODE
| MUSB_TXCSR_MODE);
- if (!musb_ep-hb_mult)
+ /*
+* Enable Autoset according to table
+* below
+* bulk_split hb_mult Autoset_Enable
+* 0 0 Yes(Normal)
+* 0 0 No(High BW ISO)
+* 1 0 Yes(HS bulk)
+* 1 0 Yes(FS bulk)
+*/
+ if (!musb_ep-hb_mult ||
+ (musb_ep-hb_mult
+can_bulk_split(musb,
+ musb_ep-type)))
csr |= MUSB_TXCSR_AUTOSET;
}
csr = ~MUSB_TXCSR_P_UNDERRUN;
@@ -1113,9 +1125,13 @@ static int musb_gadget_enable(struct usb_ep *ep,
*/
if (musb-double_buffer_not_ok)
musb_writew(regs, MUSB_TXMAXP, hw_ep-max_packet_sz_tx);
- else
+ else {
+ if (can_bulk_split(musb, musb_ep-type))
+ musb_ep-hb_mult = (hw_ep-max_packet_sz_tx /
+ musb_ep-packet_sz) - 1;
musb_writew(regs, MUSB_TXMAXP, musb_ep-packet_sz
| (musb_ep-hb_mult 11));
+ }
csr = MUSB_TXCSR_MODE | MUSB_TXCSR_CLRDATATOG;
if (musb_readw(regs, MUSB_TXCSR)
--
1.7.1
--
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: musb: set AUTOSET for full speed bulk DMA transfer in host mode
From: supriya karanth supriya.kara...@stericsson.com
The mult factor is not updated properly for the can_bulk_split()
case.
The AUTOSET bit in the TXCSR is not being set if the mult
factor is greater than 1 for the High Bandwidth ISO case.
But the mult factor is also greater than 1 in case of Full speed
bulk transfers with the packet splitting in TXMAXP register
enabled with can_bulk_split().
Without the AUTOSET the DMA transfers will not progress in mode1
Signed-off-by: supriya karanth supriya.kara...@stericsson.com
Signed-off-by: Praveena NADAHALLY praveen.nadaha...@stericsson.com
Acked-by: Linus Walleij linus.wall...@linaro.org
---
drivers/usb/musb/musb_host.c | 20
1 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index 3df6a76..b3e663c 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -634,7 +634,17 @@ static bool musb_tx_dma_program(struct dma_controller *dma,
mode = 1;
csr |= MUSB_TXCSR_DMAMODE | MUSB_TXCSR_DMAENAB;
/* autoset shouldn't be set in high bandwidth */
- if (qh-hb_mult == 1)
+ /*
+* Enable Autoset according to table
+* below
+* bulk_split hb_mult Autoset_Enable
+* 0 1 Yes(Normal)
+* 0 1 No(High BW ISO)
+* 1 1 Yes(HS bulk)
+* 1 1 Yes(FS bulk)
+*/
+ if (qh-hb_mult == 1 || (qh-hb_mult 1
+ can_bulk_split(hw_ep-musb, qh-type)))
csr |= MUSB_TXCSR_AUTOSET;
} else {
mode = 0;
@@ -794,10 +804,12 @@ static void musb_ep_program(struct musb *musb, u8 epnum,
if (musb-double_buffer_not_ok)
musb_writew(epio, MUSB_TXMAXP,
hw_ep-max_packet_sz_tx);
- else if (can_bulk_split(musb, qh-type))
+ else if (can_bulk_split(musb, qh-type)) {
+ qh-hb_mult = hw_ep-max_packet_sz_tx
+ / packet_sz;
musb_writew(epio, MUSB_TXMAXP, packet_sz
- | ((hw_ep-max_packet_sz_tx /
- packet_sz) - 1) 11);
+ | ((qh-hb_mult) - 1) 11);
+ }
else
musb_writew(epio, MUSB_TXMAXP,
qh-maxpacket |
--
1.7.1
--
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: Unreliable USB3 with NEC uPD720200 and Delock Cardreader
On Wed, 2012-12-05 at 16:33 -0800, Sarah Sharp wrote:
On Wed, Nov 28, 2012 at 02:54:06PM -0800, Sarah Sharp wrote:
On Mon, Nov 26, 2012 at 10:48:03PM +0100, Bjørn Mork wrote:
Sarah Sharp sarah.a.sh...@linux.intel.com writes:
It looks like both Ulrich and Andrew have the same issue. I also have a
Lenovo x220, and I confirmed that when I turn on PCI runtime suspend,
the NEC host controller does not report port status changes when a new
USB device is plugged in.
I'm running 3.6.7, and I'm pretty sure that runtime suspend worked for
the NEC host on some older kernel. I don't think the NEC host went into
D3cold on that kernel, though. Is there a way to disable D3cold and
just use D3hot instead?
Yes, you have /sys/bus/pci/devices/.../d3cold_allowed
See Documentation/ABI/testing/sysfs-bus-pci
If this really is a problem with the D3cold support that went into 3.6
then I guess you should include Huang Ying in the discussions as well
(CCed).
Turning off D3 cold didn't help. Once the PCI device is suspended,
connect events do not generate an interrupt.
I'll go see if I can figure out which kernel this worked on and bisect.
Wakeup from D3 works fine on the 3.5.0 kernel, but fails on 3.6.2. I
haven't fully bisected yet.
In debugging, I found that if you only enable runtime suspend for the
NEC host controller, the host successfully comes out of D3 when you plug
in a USB device. However, if you enable runtime PM for the parent PCIe root
port, it stops working. Disabling D3cold for both devices did not help.
It looks like a PCI issue, so what sort of debugging info do you need
from me?
I do some debug with my NEC uPD720200, the patch is attached. I found
that NEC uPD720200 can not generate PME interrupt, so it can be remote
waken up only via polling. But if the PCIe bridge connected to NEC
uPD720200 goes into suspended, we can not poll it. Maybe we need a way
to disable PCIe bridge suspended if the underlying device can only be
waken up via polling.
Best Regards,
Huang Ying
Index: linux.git/drivers/pci/pci.c
===
--- linux.git.orig/drivers/pci/pci.c 2012-12-06 10:57:31.485231614 +0800
+++ linux.git/drivers/pci/pci.c 2012-12-06 10:57:31.541230913 +0800
@@ -1473,12 +1473,15 @@
dev-pme_poll = false;
if (pci_check_pme_status(dev)) {
+ if (pme_poll_reset)
+ dev_info(dev-dev, pme wakeup\n);
+ else
+ dev_info(dev-dev, poll wakeup\n);
pci_wakeup_event(dev);
pm_request_resume(dev-dev);
}
return 0;
}
-
/**
* pci_pme_wakeup_bus - Walk given bus and wake up devices on it, if necessary.
* @bus: Top bus of the subtree to walk.
Index: linux.git/drivers/pci/pci-acpi.c
===
--- linux.git.orig/drivers/pci/pci-acpi.c 2012-12-06 10:57:31.485231614 +0800
+++ linux.git/drivers/pci/pci-acpi.c 2012-12-06 10:57:31.542230901 +0800
@@ -56,6 +56,7 @@
if (!pci_dev-pm_cap || !pci_dev-pme_support
|| pci_check_pme_status(pci_dev)) {
+ dev_info(pci_dev-dev, pme wakeup\n);
if (pci_dev-pme_poll)
pci_dev-pme_poll = false;
Index: linux.git/drivers/pci/pcie/pme.c
===
--- linux.git.orig/drivers/pci/pcie/pme.c 2012-12-06 10:57:31.485231614 +0800
+++ linux.git/drivers/pci/pcie/pme.c 2012-12-06 10:57:31.542230901 +0800
@@ -79,6 +79,7 @@
list_for_each_entry(dev, bus-devices, bus_list) {
/* Skip PCIe devices in case we started from a root port. */
if (!pci_is_pcie(dev) pci_check_pme_status(dev)) {
+ dev_info(dev-dev, pme wakeup\n);
if (dev-pme_poll)
dev-pme_poll = false;
@@ -144,6 +145,7 @@
port-pme_poll = false;
if (pci_check_pme_status(port)) {
+ dev_info(dev-dev, pme wakeup\n);
pm_request_resume(port-dev);
found = true;
} else {
@@ -188,6 +190,7 @@
/* The device is there, but we have to check its PME status. */
found = pci_check_pme_status(dev);
if (found) {
+ dev_info(dev-dev, pme wakeup\n);
if (dev-pme_poll)
dev-pme_poll = false;
Re: [PATCH 08/10] usb: add usb port auto power off mechanism
On 2012年11月29日 03:37, Alan Stern wrote:
On Sat, 17 Nov 2012, Lan Tianyu wrote:
This patch is to add usb port auto power off mechanism.
When usb device is suspending, usb core will suspend usb port and
usb port runtime pm callback will clear PORT_POWER feature to
power off port if all conditions were met. These conditions are
remote wakeup disable, pm qos NO_POWER_OFF flag clear and persist
enable. When device is suspended and power off, usb core
will ignore port's connect and enable change event to keep the device
not being disconnected. When it resumes, power on port again.
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2861,6 +2869,19 @@ int usb_port_suspend(struct usb_device *udev,
pm_message_t msg)
udev-port_is_suspended = 1;
msleep(10);
}
+
+/*
+ * Check whether current status is meeting requirement of
+ * usb port power off mechanism
+ */
+if (!udev-do_remote_wakeup
+ (dev_pm_qos_flags(port_dev-dev,
+PM_QOS_FLAG_NO_POWER_OFF) != PM_QOS_FLAGS_ALL)
+ udev-persist_enabled
+ !status)
+if (!pm_runtime_put_sync(port_dev-dev))
+port_dev-power_on = false;
You shouldn't need to change port_dev-power_on here.
@@ -2981,10 +3021,36 @@ static int finish_port_resume(struct usb_device
*udev)
int usb_port_resume(struct usb_device *udev, pm_message_t msg)
{
struct usb_hub *hub = hdev_to_hub(udev-parent);
+struct usb_port *port_dev = hub-ports[udev-portnum - 1];
int port1 = udev-portnum;
int status;
u16 portchange, portstatus;
+
+set_bit(port1, hub-busy_bits);
+
+if (!port_dev-power_on) {
+status = pm_runtime_get_sync(port_dev-dev);
+if (status 0) {
+dev_dbg(udev-dev, can't resume usb port, status
%d\n,
+status);
+return status;
You must not return without clearing hub-busy_bits. Same thing
applies later on.
+}
+
+port_dev-power_on = true;
This isn't necessary.
Hi Alan:
Doing port_dev-power_on = true in usb_hub_set_port_power() just after
set PORT_POWER feature will cause device to be disconnected. If user set
PM Qos NO_POWER_OFF flag after the device was power off, the port would
be power on and do port_dev-power_on = true. But the port connect
change event couldn't be ignored and the device would be disconnected.
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -72,6 +72,9 @@ static int usb_port_runtime_resume(struct device *dev)
struct usb_device *hdev =
to_usb_device(dev-parent-parent);
+if (port_dev-power_on)
+return 0;
+
Move these lines into usb_hub_set_port_power, and have that routine set
port_dev-power_on when the Set-Feature request succeeds.
return usb_hub_set_port_power(hdev, port_dev-portnum,
true);
}
@@ -81,13 +84,21 @@ static int usb_port_runtime_suspend(struct device *dev)
struct usb_port *port_dev = to_usb_port(dev);
struct usb_device *hdev =
to_usb_device(dev-parent-parent);
+int retval;
if (dev_pm_qos_flags(port_dev-dev, PM_QOS_FLAG_NO_POWER_OFF)
== PM_QOS_FLAGS_ALL)
return -EAGAIN;
-return usb_hub_set_port_power(hdev, port_dev-portnum,
+if (!port_dev-power_on)
+return 0;
+
+retval = usb_hub_set_port_power(hdev, port_dev-portnum,
false);
+if (!retval)
+port_dev-power_on = false;
+
Move all this port_dev-power_on stuff into usb_hub_set_port_power.
Then no changes will be needed here.
Alan Stern
--
Best regards
Tianyu Lan
--
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 01/10] USB: Set usb port's DeviceRemovable according acpi information in EHCI
On 2012年12月05日 23:58, Alan Stern wrote:
On Wed, 5 Dec 2012, Lan Tianyu wrote:
Hi Alan:
how about following patch?
Index: usb/drivers/usb/core/hub.c
===
--- usb.orig/drivers/usb/core/hub.c
+++ usb/drivers/usb/core/hub.c
+void usb_hub_adjust_DeviceRemovable(struct usb_device *hdev,
+ struct usb_hub_descriptor *desc)
+{
+ enum usb_port_connect_type connect_type;
+ int i;
+
+ if (!hub_is_superspeed(hdev)) {
+ for (i = 1; i = hdev-maxchild; i++) {
+ connect_type =
+ usb_get_hub_port_connect_type(hdev, i);
+
+ if (connect_type ==
USB_PORT_CONNECT_TYPE_HARD_WIRED) {
+ u8 mask = 1 (i%8);
+
+ if (!(desc-u.hs.DeviceRemovable[i/8]
mask))
+ dev_dbg(hdev-dev, usb2.0
There's no need to specify usb2.0. The kernel log already mentions
the hub's speed.
port%d's DeviceRemovable is changed to 1 according platform
information.\n, i);
s/according/according to/
+
+ desc-u.hs.DeviceRemovable[i/8]
+ |= mask;
You might as well put this line inside the if statement.
+ }
+ }
+ } else {
+ u16 port_removable =
+ le16_to_cpu(desc-u.ss.DeviceRemovable);
+
+ for (i = 1; i = hdev-maxchild; i++) {
+ connect_type =
+ usb_get_hub_port_connect_type(hdev, i);
+
+ if (connect_type ==
USB_PORT_CONNECT_TYPE_HARD_WIRED) {
+ u16 mask = 1 i;
+
+ if (!(port_removable mask))
+ dev_dbg(hdev-dev, usb3.0
port%d's DeviceRemovable is changed to 1 according platform
information.\n, i);
+
+ port_removable |= mask;
Same comments here.
+ }
+ }
+
+ desc-u.ss.DeviceRemovable =
+ cpu_to_le16(port_removable);
+ }
+}
The rest looks okay. Remember to run your patches through checkpatch
before submitting them.
Ok. I get it.
Alan Stern
--
Best regards
Tianyu Lan
--
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
