Re: [PATCH v3] phy: Add new Exynos5 USB 3.0 PHY driver
Hi,
On Monday 20 January 2014 07:12 PM, Vivek Gautam wrote:
> Add a new driver for the USB 3.0 PHY on Exynos5 series of SoCs.
> The new driver uses the generic PHY framework and will interact
> with DWC3 controller present on Exynos5 series of SoCs.
> Thereby, removing old phy-samsung-usb3 driver and related code
> used untill now which was based on usb/phy framework.
>
> Signed-off-by: Vivek Gautam
> ---
>
> Changes from v2:
> 1) Added support for multiple PHYs (UTMI+ and PIPE3) and
>related changes in the driver structuring.
> 2) Added a xlate function to get the required phy out of
>number of PHYs in mutiple PHY scenerio.
> 3) Changed the names of few structures and variables to
>have a clearer meaning.
> 4) Added 'usb3phy_config' structure to take care of mutiple
>phys for a SoC having 'exynos5_usb3phy_drv_data' driver data.
> 5) Not deleting support for old driver 'phy-samsung-usb3' until
>required support for generic phy is added to DWC3.
>
> .../devicetree/bindings/phy/samsung-phy.txt| 49 ++
> drivers/phy/Kconfig|8 +
> drivers/phy/Makefile |1 +
> drivers/phy/phy-exynos5-usb3.c | 621
>
> 4 files changed, 679 insertions(+)
> create mode 100644 drivers/phy/phy-exynos5-usb3.c
>
> diff --git a/Documentation/devicetree/bindings/phy/samsung-phy.txt
> b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> index c0fccaa..57079f8 100644
> --- a/Documentation/devicetree/bindings/phy/samsung-phy.txt
> +++ b/Documentation/devicetree/bindings/phy/samsung-phy.txt
> @@ -20,3 +20,52 @@ Required properties:
> - compatible : should be "samsung,exynos5250-dp-video-phy";
> - reg : offset and length of the Display Port PHY register set;
> - #phy-cells : from the generic PHY bindings, must be 0;
> +
> +Samsung Exynos5 SoC series USB 3.0 PHY controller
> +--
> +
> +Required properties:
> +- compatible : Should be set to one of the following supported values:
> + - "samsung,exynos5250-usb3phy" - for exynos5250 SoC,
> + - "samsung,exynos5420-usb3phy" - for exynos5420 SoC.
> +- reg : Register offset and length of USB 3.0 PHY register set;
> +- clocks: Clock IDs array as required by the controller
> +- clock-names: names of clocks correseponding to IDs in the clock property;
> + Required clocks:
> + - phy: main PHY clock (same as USB 3.0 controller IP clock),
> + used for register access.
> + - usb3phy_refclk: PHY's reference clock (usually crystal clock),
> + associated by phy name, used to determine bit values for
> + clock settings register.
> + Additional clock required for Exynos5420:
> + - usb30_sclk_100m: Additional special clock used for PHY operation
> +depicted as 'sclk_usbphy30' in CMU of Exynos5420.
> +- samsung,syscon-phandle: phandle for syscon interface, which is used to
> + control pmu registers for power isolation.
> +- #phy-cells : from the generic PHY bindings, must be 1;
> +
> +For "samsung,exynos5250-usb3phy" and "samsung,exynos5420-usb3phy" compatible
> +PHYs, the second cell in the PHY specifier identifies the PHY id, which is
> +interpreted as follows:
> + 0 - UTMI+ type phy,
> + 1 - PIPE3 type phy,
> +
> +Example:
> + usb3_phy: usbphy@1210 {
> + compatible = "samsung,exynos5250-usb3phy";
> + reg = <0x1210 0x100>;
> + clocks = <&clock 286>, <&clock 1>;
> + clock-names = "phy", "usb3phy_refclk";
> + samsung,syscon-phandle = <&pmu_syscon>;
> + #phy-cells = <1>;
> + };
> +
> +- aliases: For SoCs like Exynos5420 having multiple USB PHY controllers,
> +'usb3_phy' nodes should have numbered alias in the aliases node,
> +in the form of usb3phyN, N = 0, 1... (depending on number of
> +controllers).
> +Example:
> + aliases {
> + usb3phy0 = &usb3_phy0;
> + usb3phy1 = &usb3_phy1;
> + };
> diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
> index 330ef2d..32f9f38 100644
> --- a/drivers/phy/Kconfig
> +++ b/drivers/phy/Kconfig
> @@ -51,4 +51,12 @@ config PHY_EXYNOS_DP_VIDEO
> help
> Support for Display Port PHY found on Samsung EXYNOS SoCs.
>
> +config PHY_EXYNOS5_USB3
> + tristate "Exynos5 SoC series USB 3.0 PHY driver"
> + depends on ARCH_EXYNOS5
> + select GENERIC_PHY
> + select MFD_SYSCON
add depends on 'HAS_IOMEM'. Someone reported getting
undefined reference to `devm_ioremap_resource' with it.
> + help
> + Enable USB 3.0 PHY support for Exynos 5 SoC series
> +
> endmenu
> diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
> index d0caae9..9c06a61 100644
> --- a/drivers/phy/Makefile
> +++ b/drivers/phy/Makefile
> @@ -7,3 +7,4 @@ obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
> obj-$(CONFIG_PHY_EXYNOS_M
Re: [PATCH 1/3] usb: musb: musb_host: Enable ISOCH IN handling for AM335x host
On 1/24/2014 11:13 PM, Sergei Shtylyov wrote:
Hello.
On 24-01-2014 18:14, George Cherian wrote:
Enable the isochrounous IN handling for AM335x HOST.
Reprogram CPPI to receive consecutive ISOCH frames in the same URB.
Sigh, I knew CPPI ISO path was broken for years but didn't have
time to fix it. :-(
Signed-off-by: George Cherian
--- git rebase
drivers/usb/musb/musb_host.c | 29 ++---
1 file changed, 26 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index ed45572..5b6482c 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -1689,9 +1689,11 @@ void musb_host_rx(struct musb *musb, u8 epnum)
| MUSB_RXCSR_H_AUTOREQ
| MUSB_RXCSR_AUTOCLEAR
| MUSB_RXCSR_RXPKTRDY);
+
Not needed.
musb_writew(hw_ep->regs, MUSB_RXCSR, val);
-#if defined(CONFIG_USB_INVENTRA_DMA) || defined(CONFIG_USB_UX500_DMA)
+#if defined(CONFIG_USB_INVENTRA_DMA) ||
defined(CONFIG_USB_UX500_DMA) || \
+defined(CONFIG_USB_TI_CPPI41_DMA)
The DaVinci CPPI 3.0 should probably also be included here...
Should'nt that be a separate patch as this one is specific to AM335X.
if (usb_pipeisoc(pipe)) {
struct usb_iso_packet_descriptor *d;
@@ -1706,8 +1708,28 @@ void musb_host_rx(struct musb *musb, u8 epnum)
if (++qh->iso_idx >= urb->number_of_packets)
done = true;
-else
+else {
If you're making the *else* arm use {}, you should make all the
other arms of *if* statement also have them -- see
Documentation/CodingStyle.
Will fix in v2!!!
WBR, Sergei
--
-George
--
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 3/3] usb: musb: musb_cppi41: Handle ISOCH differently and not use the hrtimer.
On 1/24/2014 11:16 PM, Sergei Shtylyov wrote:
Hello.
On 24-01-2014 18:14, George Cherian wrote:
In case of ISOCH transfers the hrtimer workaround for the hardware issue
is not very reliable. Instead of checking musb_is_tx_fifo_empty() in
hrtimer
routine, schedule a completion work and check the same in completion
work.
Signed-off-by: George Cherian
---
drivers/usb/musb/musb_cppi41.c | 54
+-
1 file changed, 53 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/musb/musb_cppi41.c
b/drivers/usb/musb/musb_cppi41.c
index 39ee516..8075562 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
[...]
@@ -165,6 +178,32 @@ static void cppi41_trans_done(struct
cppi41_dma_channel *cppi41_channel)
}
}
+static void cppi_trans_done_work(struct work_struct *work)
+{
+unsigned long flags;
+struct cppi41_dma_channel *cppi41_channel =
+container_of(work, struct cppi41_dma_channel, dma_completion);
+struct cppi41_dma_controller *controller =
cppi41_channel->controller;
+struct musb *musb = controller->musb;
+struct musb_hw_ep *hw_ep = cppi41_channel->hw_ep;
+bool empty;
+
+if (!cppi41_channel->is_tx && is_isoc(hw_ep, 1)) {
+spin_lock_irqsave(&musb->lock, flags);
+cppi41_trans_done(cppi41_channel);
+spin_unlock_irqrestore(&musb->lock, flags);
This *if* arm is clearly over-indented by one tab. Compare with
below *else* arm.
Will Fix in v2
+} else {
+empty = musb_is_tx_fifo_empty(hw_ep);
+if (empty) {
+spin_lock_irqsave(&musb->lock, flags);
+cppi41_trans_done(cppi41_channel);
+spin_unlock_irqrestore(&musb->lock, flags);
+} else {
+ schedule_work(&cppi41_channel->dma_completion);
+}
+}
+}
+
WBR, Sergei
--
-George
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/3] usb: musb: musb_cppi41: Handle ISOCH differently and not use the hrtimer.
In case of ISOCH transfers the hrtimer workaround for the hardware issue
is not very reliable. Instead of checking musb_is_tx_fifo_empty() in hrtimer
routine, schedule a completion work and check the same in completion work.
Signed-off-by: George Cherian
---
drivers/usb/musb/musb_cppi41.c | 53 ++
1 file changed, 53 insertions(+)
diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index 39ee516..5b0f2a5 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -39,6 +39,7 @@ struct cppi41_dma_channel {
u32 transferred;
u32 packet_sz;
struct list_head tx_check;
+ struct work_struct dma_completion;
};
#define MUSB_DMA_NUM_CHANNELS 15
@@ -112,6 +113,18 @@ static bool musb_is_tx_fifo_empty(struct musb_hw_ep *hw_ep)
return true;
}
+static bool is_isoc(struct musb_hw_ep *hw_ep, bool in)
+{
+ if (in && hw_ep->in_qh) {
+ if (hw_ep->in_qh->type == USB_ENDPOINT_XFER_ISOC)
+ return true;
+ } else if (hw_ep->out_qh) {
+ if (hw_ep->out_qh->type == USB_ENDPOINT_XFER_ISOC)
+ return true;
+ }
+ return false;
+}
+
static void cppi41_dma_callback(void *private_data);
static void cppi41_trans_done(struct cppi41_dma_channel *cppi41_channel)
@@ -165,6 +178,32 @@ static void cppi41_trans_done(struct cppi41_dma_channel
*cppi41_channel)
}
}
+static void cppi_trans_done_work(struct work_struct *work)
+{
+ unsigned long flags;
+ struct cppi41_dma_channel *cppi41_channel =
+ container_of(work, struct cppi41_dma_channel, dma_completion);
+ struct cppi41_dma_controller *controller = cppi41_channel->controller;
+ struct musb *musb = controller->musb;
+ struct musb_hw_ep *hw_ep = cppi41_channel->hw_ep;
+ bool empty;
+
+ if (!cppi41_channel->is_tx && is_isoc(hw_ep, 1)) {
+ spin_lock_irqsave(&musb->lock, flags);
+ cppi41_trans_done(cppi41_channel);
+ spin_unlock_irqrestore(&musb->lock, flags);
+ } else {
+ empty = musb_is_tx_fifo_empty(hw_ep);
+ if (empty) {
+ spin_lock_irqsave(&musb->lock, flags);
+ cppi41_trans_done(cppi41_channel);
+ spin_unlock_irqrestore(&musb->lock, flags);
+ } else {
+ schedule_work(&cppi41_channel->dma_completion);
+ }
+ }
+}
+
static enum hrtimer_restart cppi41_recheck_tx_req(struct hrtimer *timer)
{
struct cppi41_dma_controller *controller;
@@ -228,6 +267,14 @@ static void cppi41_dma_callback(void *private_data)
transferred < cppi41_channel->packet_sz)
cppi41_channel->prog_len = 0;
+ if (!cppi41_channel->is_tx) {
+ if (is_isoc(hw_ep, 1))
+ schedule_work(&cppi41_channel->dma_completion);
+ else
+ cppi41_trans_done(cppi41_channel);
+ goto out;
+ }
+
empty = musb_is_tx_fifo_empty(hw_ep);
if (empty) {
cppi41_trans_done(cppi41_channel);
@@ -264,6 +311,10 @@ static void cppi41_dma_callback(void *private_data)
goto out;
}
}
+ if (is_isoc(hw_ep, 0)) {
+ schedule_work(&cppi41_channel->dma_completion);
+ goto out;
+ }
list_add_tail(&cppi41_channel->tx_check,
&controller->early_tx_list);
if (!hrtimer_active(&controller->early_tx)) {
@@ -620,6 +671,8 @@ static int cppi41_dma_controller_start(struct
cppi41_dma_controller *controller)
cppi41_channel->port_num = port;
cppi41_channel->is_tx = is_tx;
INIT_LIST_HEAD(&cppi41_channel->tx_check);
+ INIT_WORK(&cppi41_channel->dma_completion,
+ cppi_trans_done_work);
musb_dma = &cppi41_channel->channel;
musb_dma->private_data = cppi41_channel;
--
1.8.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 v2 0/3] Enable ISOCH IN handling for AM335x
This series enables the ISOCH IN handling for AM335x HOST mode. With this series webcam devices are now supported under AM335x MUSB HOST with CPPI DMA enabled. Patch 1 - Enable basic ISOCH IN handling Patch 2 - Make CPPI aware of hb transfers Patch 3 - Using hrtimer based polling for tx empty leads to hang , fixes it v1 -> v2 Fix code-style comments. George Cherian (3): usb: musb: musb_host: Enable ISOCH IN handling for AM335x host usb: musb: musb_cppi41: Make CPPI aware of high bandwidth transfers usb: musb: musb_cppi41: Handle ISOCH differently and not use the hrtimer. drivers/usb/musb/musb_cppi41.c | 67 +- drivers/usb/musb/musb_host.c | 30 --- 2 files changed, 92 insertions(+), 5 deletions(-) -- 1.8.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 v2 2/3] usb: musb: musb_cppi41: Make CPPI aware of high bandwidth transfers
Enable CPPI to handle high bandwidth transfers, especially to support
webcam captures. Use a single bd to get the whole of the data in case of
high bandwidth transfers.
Signed-off-by: George Cherian
---
drivers/usb/musb/musb_cppi41.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/drivers/usb/musb/musb_cppi41.c b/drivers/usb/musb/musb_cppi41.c
index f889296..39ee516 100644
--- a/drivers/usb/musb/musb_cppi41.c
+++ b/drivers/usb/musb/musb_cppi41.c
@@ -448,12 +448,25 @@ static int cppi41_dma_channel_program(struct dma_channel
*channel,
dma_addr_t dma_addr, u32 len)
{
int ret;
+ struct cppi41_dma_channel *cppi41_channel = channel->private_data;
+ int hb_mult = 0;
BUG_ON(channel->status == MUSB_DMA_STATUS_UNKNOWN ||
channel->status == MUSB_DMA_STATUS_BUSY);
+ if (is_host_active(cppi41_channel->controller->musb)) {
+ if (cppi41_channel->is_tx)
+ hb_mult = cppi41_channel->hw_ep->out_qh->hb_mult;
+ else
+ hb_mult = cppi41_channel->hw_ep->in_qh->hb_mult;
+ }
+
channel->status = MUSB_DMA_STATUS_BUSY;
channel->actual_len = 0;
+
+ if (hb_mult)
+ packet_sz = hb_mult * (packet_sz & 0x7FF);
+
ret = cppi41_configure_channel(channel, packet_sz, mode, dma_addr, len);
if (!ret)
channel->status = MUSB_DMA_STATUS_FREE;
--
1.8.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 v2 1/3] usb: musb: musb_host: Enable ISOCH IN handling for AM335x host
Enable the isochrounous IN handling for AM335x HOST.
Reprogram CPPI to receive consecutive ISOCH frames in the same URB.
Signed-off-by: George Cherian
---
drivers/usb/musb/musb_host.c | 30 ++
1 file changed, 26 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index ed45572..79b7510 100644
--- a/drivers/usb/musb/musb_host.c
+++ b/drivers/usb/musb/musb_host.c
@@ -1691,7 +1691,8 @@ void musb_host_rx(struct musb *musb, u8 epnum)
| MUSB_RXCSR_RXPKTRDY);
musb_writew(hw_ep->regs, MUSB_RXCSR, val);
-#if defined(CONFIG_USB_INVENTRA_DMA) || defined(CONFIG_USB_UX500_DMA)
+#if defined(CONFIG_USB_INVENTRA_DMA) || defined(CONFIG_USB_UX500_DMA) || \
+ defined(CONFIG_USB_TI_CPPI41_DMA)
if (usb_pipeisoc(pipe)) {
struct usb_iso_packet_descriptor *d;
@@ -1704,10 +1705,30 @@ void musb_host_rx(struct musb *musb, u8 epnum)
if (d->status != -EILSEQ && d->status != -EOVERFLOW)
d->status = 0;
- if (++qh->iso_idx >= urb->number_of_packets)
+ if (++qh->iso_idx >= urb->number_of_packets) {
done = true;
- else
+ } else {
+#if defined(CONFIG_USB_TI_CPPI41_DMA)
+ struct dma_controller *c;
+ dma_addr_t *buf;
+ u32 length, ret;
+
+ c = musb->dma_controller;
+ buf = (void *)
+ urb->iso_frame_desc[qh->iso_idx].offset
+ + (u32)urb->transfer_dma;
+
+ length =
+ urb->iso_frame_desc[qh->iso_idx].length;
+
+ val |= MUSB_RXCSR_DMAENAB;
+ musb_writew(hw_ep->regs, MUSB_RXCSR, val);
+
+ ret = c->channel_program(dma, qh->maxpacket,
+ 0, (u32) buf, length);
+#endif
done = false;
+ }
} else {
/* done if urb buffer is full or short packet is recd */
@@ -1747,7 +1768,8 @@ void musb_host_rx(struct musb *musb, u8 epnum)
}
/* we are expecting IN packets */
-#if defined(CONFIG_USB_INVENTRA_DMA) || defined(CONFIG_USB_UX500_DMA)
+#if defined(CONFIG_USB_INVENTRA_DMA) || defined(CONFIG_USB_UX500_DMA) || \
+ defined(CONFIG_USB_TI_CPPI41_DMA)
if (dma) {
struct dma_controller *c;
u16 rx_count;
--
1.8.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: [OpenOCD-devel] [libusb] Announcing libusb-1.0.18 (as well as libusbx-1.0.18 *FINAL*)
Hi Pete. On Sun, 26 Jan 2014 17:07:42 +, Pete Batard wrote: > I, and many others, happen to think users of libusb deserve more than > one release in 4 years, even more so as continuous major development has > been going on. I disagree. If libusb works fine, no need to fix bugs that are not present. If the USB standard does not change, no need to change the library. Since libusb is a core library, I find it much more important that it stays reliable. Each time there is a non-bugfix change to a library, there is a risk of introducing new bugs. I'd personally prefer stable quality code over code that has features added every day. OpenOCD is a good example; it's been an open wound for a while, but the current developers are very serious and focus on fixing bugs, rather than adding new features. In my opinion, that's the right way to go. So I'd prefer that if there's a version of the USB library the has to be changed often, that it would have a different name; it would be fine to keep the name libusbx for this purpose, so that the name libusb would not deviate from it's previous stable reputation. Love Jens -- 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/3] usb: musb: musb_host: Enable ISOCH IN handling for AM335x host
Hello. On 27-01-2014 13:21, George Cherian wrote: Enable the isochrounous IN handling for AM335x HOST. Reprogram CPPI to receive consecutive ISOCH frames in the same URB. Sigh, I knew CPPI ISO path was broken for years but didn't have time to fix it. :-( Signed-off-by: George Cherian --- git rebase drivers/usb/musb/musb_host.c | 29 ++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c index ed45572..5b6482c 100644 --- a/drivers/usb/musb/musb_host.c +++ b/drivers/usb/musb/musb_host.c @@ -1689,9 +1689,11 @@ void musb_host_rx(struct musb *musb, u8 epnum) [...] musb_writew(hw_ep->regs, MUSB_RXCSR, val); -#if defined(CONFIG_USB_INVENTRA_DMA) || defined(CONFIG_USB_UX500_DMA) +#if defined(CONFIG_USB_INVENTRA_DMA) || defined(CONFIG_USB_UX500_DMA) || \ +defined(CONFIG_USB_TI_CPPI41_DMA) The DaVinci CPPI 3.0 should probably also be included here... Should'nt that be a separate patch as this one is specific to AM335X. Of course. 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: [OpenOCD-devel] [libusb] Announcing libusb-1.0.18 (as well as libusbx-1.0.18 *FINAL*)
Hi Jens, On 2014.01.27 12:25, Jens Bauer wrote: On Sun, 26 Jan 2014 17:07:42 +, Pete Batard wrote: I, and many others, happen to think users of libusb deserve more than one release in 4 years, even more so as continuous major development has been going on. I disagree. If libusb works fine, no need to fix bugs that are not present. Shall I remind you that, the only reason people can run OpenOCD on Windows, using libusb (rather than libusbx) is that *we* pushed Peter, through the release of libusbx, to release his version of libusb 1.0.9, that was the first to include Windows support. If you followed his earlier dismissive comments on OpenOCD and elsewhere about the Windows backend being subpar, then you can only come to the logical conclusion that he would probably never have released otherwise (since there hasn't been any other release of libusb until this new one). So, you are basically implying that it was fine for OpenOCD to remain officially unavailable on Windows, until such time Peter felt that the Windows backend was to his liking. But in the same sentence, you also kind of prove my point that, if the Windows backend was as subpar as Peter makes it to be, you couldn't really be saying that "libusb works fine" for your purpose, since I don't recall seeing much complaint from OpenOCD Windows users on our lists. Furthermore, we also did fix some pretty major bugs since libusb 1.0.9 was released (please take a look at our Changelog). Or are you under the impression that libusb is a lot more stable and much less in need of development and bugfix than OpenOCD? You may have been lucky to never run into a libusb bug when using OpenOCD. But I think the vast majority of OpenOCD and libusb users will prefer relying on actual bug fixes, from an up to date library, rather than luck. If the USB standard does not change, no need to change the library. Ah, but the USB standard did change, and we did add support for a bunch of newly introduced USB 3.0 constructs (BOS, etc). Also, if you are planning to use OpenOCD through an USB 3.0 controller on Windows 7, you very much want to use the latest libusb, as we have to regularly add the names of new HCD root hubs (which each manufacturer provides) into the library. Without this, you can forget about using OpenOCD through an USB 3.0 port. This is very straightforward low risk fix to add (and we actually did one of those in this release -> "VIA xHCI support"). Do you really want to tell Windows 7 users with a VIA USB 3.0 controller, and that want to use OpenOCD to access an FTDI device for instance, that they are MUCH better off waiting a couple of years for a "more stable" libusb release (whatever that means)? Since libusb is a core library, I find it much more important that it stays reliable. Which is our goal. Contrary to Peter's propaganda, we are committed to fixing, improving, and trying to make libusb more reliable. It looks to me like Peter seems to be under this weird impression that any software development that isn't under his direct control can only be rushed and have complete disregard for stability. But if your idea of stability, when there are very important bug to fix as well as much requested features to add (such as hotplug), is to only release once in 5 years, I think you are mistaking stability for immobility. Each time there is a non-bugfix change to a library, there is a risk of introducing new bugs. Should you advise the cancellation the next release of OpenOCD then? If not, it makes no more sense to be against this release of libusb as it is to be against the next release of OpenOCD. That is, unless you consider that, unlike OpenOCD ones, the current libusb developers and mailing list contributors are just a bunch of amateurs who have little clue on how to develop serious, stable code. But if that is the case, I will kindly ask you to try to back up what made you reach this conclusion with facts, rather than hearsay. I'd personally prefer stable quality code over code that has features added every day. Hyperbole. Disproved below. OpenOCD is a good example; it's been an open wound for a while, but the current developers are very serious and focus on fixing bugs, rather than adding new features. OK, so Peter's propaganda has worked, and you are under the impression that the current libusb development team and contributors are NOT serious, don't care about bugs and just want to add shiny features. If perusing through the libusb-devel and libusbx-devel mailing lists is not enough to prove the opposite, let me show you our Changelog then, for 1.0.17 (libusbx, released 5 months ago) to 1.0.18, and which was linked in the announcement (http://log.libusb.info): 2014-01-25: v1.0.18 * Fix multiple memory leaks * Fix a crash when HID transfers return no data on Windows * Ensure all pending events are consumed * Improve Android and ucLinux sup
Re: [PATCH] phy-rcar-usb-gen2: add device tree support
Hi,
On Sun, Jan 26, 2014 at 05:05:01PM +, Ben Dooks wrote:
> Add support for the phy-rcar-gen2-usb driver to be probed from device tree.
>
> Signed-off-by: Ben Dooks
> Reviewed-by: Ian Molton
> ---
> Fixes from v2:
> - fix missing of_if patch
> (I clearly should not be let near git-rebase when hungry)
>
> Fixes from v1:
> - use of_property_reasd-bool()
> - remove unused of_id variable
>
> Cc: linux-usb@vger.kernel.org
> Cc: linux...@vger.kernel.org
> Cc: Magnus Damm
> Cc: Simon Horman
> Cc: devicet...@vger.kernel.org
it's a good practice to Cc maintainers ;-)
patch looks good to me, but I'd like to get an Ack from one of DT
maintainers
> ---
> drivers/usb/phy/phy-rcar-gen2-usb.c | 31 ++-
> 1 file changed, 26 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/usb/phy/phy-rcar-gen2-usb.c
> b/drivers/usb/phy/phy-rcar-gen2-usb.c
> index db3ab34..e4665b9 100644
> --- a/drivers/usb/phy/phy-rcar-gen2-usb.c
> +++ b/drivers/usb/phy/phy-rcar-gen2-usb.c
> @@ -15,6 +15,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
>
> @@ -167,6 +168,12 @@ out:
> spin_unlock_irqrestore(&priv->lock, flags);
> }
>
> +static struct of_device_id rcar_gen2_usb_phy_ofmatch[] = {
> + { .compatible = "renesas,usb-phy-r8a7790", },
> + { .compatible = "renesas,rcar-gen2-usb-phy", },
> + { },
> +};
> +
> static int rcar_gen2_usb_phy_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> @@ -178,7 +185,7 @@ static int rcar_gen2_usb_phy_probe(struct platform_device
> *pdev)
> int retval;
>
> pdata = dev_get_platdata(&pdev->dev);
> - if (!pdata) {
> + if (!pdata && !dev->of_node) {
> dev_err(dev, "No platform data\n");
> return -EINVAL;
> }
> @@ -203,16 +210,29 @@ static int rcar_gen2_usb_phy_probe(struct
> platform_device *pdev)
> spin_lock_init(&priv->lock);
> priv->clk = clk;
> priv->base = base;
> - priv->ugctrl2 = pdata->chan0_pci ?
> - USBHS_UGCTRL2_USB0_PCI : USBHS_UGCTRL2_USB0_HS;
> - priv->ugctrl2 |= pdata->chan2_pci ?
> - USBHS_UGCTRL2_USB2_PCI : USBHS_UGCTRL2_USB2_SS;
> priv->phy.dev = dev;
> priv->phy.label = dev_name(dev);
> priv->phy.init = rcar_gen2_usb_phy_init;
> priv->phy.shutdown = rcar_gen2_usb_phy_shutdown;
> priv->phy.set_suspend = rcar_gen2_usb_phy_set_suspend;
>
> + if (dev->of_node) {
> + if (of_property_read_bool(dev->of_node, "renesas,usb0-hs"))
> + priv->ugctrl2 = USBHS_UGCTRL2_USB0_HS;
> + else
> + priv->ugctrl2 = USBHS_UGCTRL2_USB0_PCI;
> +
> + if (of_property_read_bool(dev->of_node, "renesas,usb2-ss"))
> + priv->ugctrl2 |= USBHS_UGCTRL2_USB2_SS;
> + else
> + priv->ugctrl2 |= USBHS_UGCTRL2_USB2_PCI;
> + } else {
> + priv->ugctrl2 = pdata->chan0_pci ?
> + USBHS_UGCTRL2_USB0_PCI : USBHS_UGCTRL2_USB0_HS;
> + priv->ugctrl2 |= pdata->chan2_pci ?
> + USBHS_UGCTRL2_USB2_PCI : USBHS_UGCTRL2_USB2_SS;
> + }
> +
> retval = usb_add_phy(&priv->phy, USB_PHY_TYPE_USB2);
> if (retval < 0) {
> dev_err(dev, "Failed to add USB phy\n");
> @@ -236,6 +256,7 @@ static int rcar_gen2_usb_phy_remove(struct
> platform_device *pdev)
> static struct platform_driver rcar_gen2_usb_phy_driver = {
> .driver = {
> .name = "usb_phy_rcar_gen2",
> + .of_match_table = rcar_gen2_usb_phy_ofmatch,
> },
> .probe = rcar_gen2_usb_phy_probe,
> .remove = rcar_gen2_usb_phy_remove,
> --
> 1.8.5.2
>
> --
> 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
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v2 1/1] USB: EHCI: wait more than 3ms until the device enters full-speed idle
On Mon, 27 Jan 2014, Peter Chen wrote:
> If the high-speed device does not enter full-speed idle after
> wakeup on disconnect logic has effected, there will be an
> unexpected disconnect wakeup interrupt due to the bus is still SE0.
I think you mean "If the high-speed device does not enter full-speed
idle _before_ wakeup on disconnect logic has taken effect..."
It sounds like this is a bug in your EHCI hardware. The
wake-on-disconnect logic should never take effect until after the port
goes into full-speed idle.
> Signed-off-by: Peter Chen
> ---
>
> Changes for v2:
> Using usleep_range instead of mdelay
>
> drivers/usb/host/ehci-hub.c | 12
> 1 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
> index 47b858f..976294c 100644
> --- a/drivers/usb/host/ehci-hub.c
> +++ b/drivers/usb/host/ehci-hub.c
> @@ -301,6 +301,18 @@ static int ehci_bus_suspend (struct usb_hcd *hcd)
>
> if (t1 != t2) {
> ehci_writel(ehci, t2, reg);
> + if ((t2 & PORT_WKDISC_E)
> + && (ehci_port_speed(ehci, t2) ==
> + USB_PORT_STAT_HIGH_SPEED)) {
> + /*
> + * If the high-speed device has not switched
> + * to full-speed idle before WKDISC_E has
> + * effected, there will be a WKDISC event.
> + */
> + spin_unlock_irq(&ehci->lock);
> + usleep_range(3500, 4000);
> + spin_lock_irq(&ehci->lock);
> + }
This doesn't look like it will do what you want. t2 already has the
PORT_WKDISC_E bit set, so once the
ehci_writel(ehci, t2, reg);
has executed, it is already too late. Instead, you should write (t2 &
~PORT_WKDISC_E), then wait a few ms, and then write t2.
Since this bug seems to affect only a few EHCI controllers, we should
have a quirk flag for it. There's no need to make everybody wait 3.5-4
ms just for the sake of a few pieces of buggy hardware.
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] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
Hi Felipe, On Tue, Jan 21, 2014 at 08:47:25AM -0600, Felipe Balbi wrote: > On Tue, Jan 21, 2014 at 03:41:38PM +0530, Kishon Vijay Abraham I wrote: > > Since PHYs for dwc3 is optional (not all SoCs that have DWC3 use PHYs), > > do not return from probe if the USB PHY library returns -ENODEV as that > > this isn't correct, they all have PHYs, some of them might not be > controllable. > > > indicates the platform does not have PHY. > > not really, that indicates the current platform tried to grab a PHY and > the PHY doesn't exist. If there's anybody with a non-controllable PHY > and someone gives me a really good reason for not using the generic > no-op PHY, then we should add a flag and we could: > > if (!likely(dwc->flags & DWC3_USB2PHY_DRIVER_NOT_NEEDED)) > dwc3_grab_phys(dwc); Why would you need to know if the PHY drivers are needed or not explicitly in your controller driver? > But I really want to see the argument against using no-op. As far as I > could see, everybody needs a PHY driver one way or another, some > platforms just haven't sent any PHY driver upstream and have their own > hacked up solution to avoid using the PHY layer. Not true in our case. Platforms using Intel's SoCs and chip sets may or may not have controllable USB PHY. Quite often they don't. The Baytrails have usually ULPI PHY for USB2, but that does not mean they provide any vendor specific functions or any need for a driver in any case. Are we talking about the old USB PHY library or the new PHY framework with the no-op PHY driver? Well, in any case, I don't understand what is the purpose of the no-op PHY driver. What are you drying to achieve with that? Thanks, -- heikki -- 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
Many USB ethernet devices are broken over xhci
Many of the net/usb ethernet drivers (including common ones like the smsc95xx) will fail to transmit all packet length properly when connected to a USB3 port (ie using the xhci driver). The underlying problem is that they assume the host controller will honour the URB_ZERO_PACKET flag - which usbnet sets because they specify FLAG_SEND_ZLP. However no one has ever added support for URB_ZERO_PACKET to the xhci driver - so packets that need padding (probably 512 bytes after any header is added) will not be sent correctly and may have very adverse effects on the usb target. The ax179_178a driver avoids this by not setting FLAG_SEND_ZLP, modifying the packet header, and appending an extra zero byte. (Which has been responsible for its own set of panics.) I don't think this can be fixed by just clearing (or ignoring) FLAG_SEND_ZLP as the extra byte will also confuse things. It needs to be fixed in the xhci code. I wrote this patch a while ago - worked for me with the ax179_178a driver. http://www.spinics.net/lists/linux-usb/msg97370.html The patch is a bit difficult to read, the v1 version contained a copy of the new function. http://www.spinics.net/lists/linux-usb/msg97183.html I don't think anything significant has changed (in the main kernel sources) since I wrote the patch. David -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: phy: Quiet unable to find transceiver message
On Sat, Jan 25, 2014 at 03:24:55PM -0500, Josh Boyer wrote:
> On Sat, Jan 25, 2014 at 10:37 AM, Alan Stern
> wrote:
> > On Sat, 25 Jan 2014, Josh Boyer wrote:
> >
> >> commit 1ae5799ef6317 ("usb: hcd: Initialize USB phy if needed") allows
> >> the USB layer to initialize external PHYs if needed. However, a PHY is
> >> not needed in all cases. The usb_get_phy_device function will print
> >> an error message, "unable to find transceiver" but everything still
> >> functions normally.
> >>
> >> Drop the severity of this message to pr_debug.
> >>
> >> Signed-off-by: Josh Boyer
> >> ---
> >> drivers/usb/phy/phy.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c
> >> index e6f61e4..c7fe880 100644
> >> --- a/drivers/usb/phy/phy.c
> >> +++ b/drivers/usb/phy/phy.c
> >> @@ -228,7 +228,7 @@ struct usb_phy *usb_get_phy_dev(struct device *dev, u8
> >> index)
> >>
> >> phy = __usb_find_phy_dev(dev, &phy_bind_list, index);
> >> if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
> >> - pr_err("unable to find transceiver\n");
> >> + pr_debug("unable to find transceiver\n");
> >> goto err0;
> >> }
> >
> > Wouldn't it make more sense to change this to dev_debug? As it stands,
> > the user has no idea which device is lacking a transceiver.
>
> Quite possibly, yes. I'm not overly familiar with the subsystem and
> was just writing up what Felipe suggested.
>
> > (The same is probably true for other log messages in this source file.)
>
> I don't disagree, but I'd rather someone with more experience in the
> USB subsystem do that kind of broader audit/change. I'd be happy to
> test.
yeah, I just sent a patch where I forgot to switch over to dev_dbg(), if
you can do that for both messages and remove the out of memory message,
I'd be glad to take your patch instead of mine.
cheers
--
balbi
signature.asc
Description: Digital signature
RE: Max. libusb bulk send/receive values lowered with xhci?
From: Jérôme Carretero > I was happily using big (10MB) buffers before, and with recent kernels, > when using USB3, I had to reduce the size of my buffers a lot. > By the way, I couldn't find any information on a maximum size for the > bulk transfers using libusb, maybe you know about that also ? > > So, using v3.13, this what I get from the kernel when doing a bulk read > of 4 MiB: > > [ 506.856282] xhci_hcd :00:14.0: Too many fragments 256, max 63 ... > I saw your 3.12-td-fragment-failure branch and tried it; there, > sometimes the transfers don't work, with: > > xhci_hcd :00:14.0: WARN Event TRB for slot 10 ep 4 with no TDs queued? > python2: page allocation failure: order:10, mode:0x1040d0 I've had a quick look and the reason for the allocation failure is fairly obvious. The libusb ioctl is handled by proc_do_submiturb() it will use scatter-gather for long requests, but always chops things up into 16k fragments. So (as in the trace above) a 4MB transfers requires 256 fragments. If the number of segments exceeds the advertised sg_tablesize (which is now 128) then it falls back on using a single fragment. For a 4MB buffer this is 1024 contiguous pages - not surprisingly it sometimes fails (it really ought to sleep - but that is another issue). Possibly proc_do_submit() should use longer fragments [1] in order to get below the sg_tablesise limit. However this is still doomed to fail. A single 16MB buffers crosses at least 255 64kB boundaries so the xhci driver will need 256 or 257 TRB to describe the buffer. The only way for xhci to accept these transfers is to apply the patch I posted last week that checks for aligned buffers and skips the 'pad with NOPs' code if they are aligned, and then set sg_tablesize to ~0. The 'struct usb_bus' currently contains 2 fields associated with scatter- gather: - no_sg_constraint:1 is set by xhci and checked by usbnet/ax88179_178a before it uses 'randomly aligned' fragments. - sg_tablesize is supposed to be the limit on the number of sg fragments. ehci, ohci and uhci either set 0 or ~0. xhci currently sets TRBS_PER_SEGMENT/2 == 128 (previously 32, older ~0). Some code only checks for non-zero. It would be better if the former were changed to be a limit on the number of 'unconstrained' fragments; since that limit is somewhat different (in xhci) from the limit on the number of aligned fragments. Alternatively both could be treated at booleans, and we just hope that any fragment limits aren't exceeded. [1] I don't know if it is best to try to allocate 2^n pages, falling back on smaller sizes (if they'll meet the fragment count limit) rather than allocating equal sized fragments. The code should probably also be willing to allocate more fragments if it can't allocated even 16k blocks. However processing variable-sized fragment lists requires that the sg[].length field not be modified by the dma_map code - I don't know if that is generally true? David
Re: [PATCH 1/2] usb: dwc3: core: continue probing if usb phy library returns -ENODEV/-ENXIO
Hi, On Mon, Jan 27, 2014 at 05:08:55PM +0200, Heikki Krogerus wrote: > > > Since PHYs for dwc3 is optional (not all SoCs that have DWC3 use PHYs), > > > do not return from probe if the USB PHY library returns -ENODEV as that > > > > this isn't correct, they all have PHYs, some of them might not be > > controllable. > > > > > indicates the platform does not have PHY. > > > > not really, that indicates the current platform tried to grab a PHY and > > the PHY doesn't exist. If there's anybody with a non-controllable PHY > > and someone gives me a really good reason for not using the generic > > no-op PHY, then we should add a flag and we could: > > > > if (!likely(dwc->flags & DWC3_USB2PHY_DRIVER_NOT_NEEDED)) > > dwc3_grab_phys(dwc); > > Why would you need to know if the PHY drivers are needed or not > explicitly in your controller driver? because, one way or another, they all do need it. Except for quirky ones like AM437x where a USB3 IP was hardwired into USB2-only mode, so it really lacks a USB3 PHY. > > But I really want to see the argument against using no-op. As far as I > > could see, everybody needs a PHY driver one way or another, some > > platforms just haven't sent any PHY driver upstream and have their own > > hacked up solution to avoid using the PHY layer. > > Not true in our case. Platforms using Intel's SoCs and chip sets may > or may not have controllable USB PHY. Quite often they don't. The > Baytrails have usually ULPI PHY for USB2, but that does not mean they > provide any vendor specific functions or any need for a driver in any > case. that's different from what I heard. > Are we talking about the old USB PHY library or the new PHY framework > with the no-op PHY driver? > > Well, in any case, I don't understand what is the purpose of the no-op > PHY driver. What are you drying to achieve with that? I'm trying to avoid supporting 500 different combinations for a single driver. We already support old USB PHY layer and generic PHY layer, now they both need to be made optional. The old USB PHY layer also provides a no-op, now called phy-generic, which is actually pretty useful. On top of all that, I'm sure you'll subscribe to the idea of preventing dwc3 from becoming another MUSB which supports several different configurations and none work correctly. I much prefer to take baby steps which are well thought-out and very well exercised, so I'll be very pedantic about proof of testing. An invasive change such as $subject needs to be very well-tested in different architectures with different Kconfig choices before I'd feel comfortable applying it. Also, the assumptions made in $subject are just plain wrong, which gets me really iffy about applying it or not. cheers -- balbi signature.asc Description: Digital signature
[PATCH] usb: phy: move some error messages to debug
the PHY layer is supposed to be optional,
considering some PHY have no control bus
for SW to poke around.
After commit 1ae5799 (usb: hcd: Initialize
USB phy if needed) any HCD which didn't provide
a PHY driver would emit annoying error messages.
In this patch we're decreasing those messages
to debugging only and we also add a PHY prefix
so we know where they're coming from.
Reported-by: Josh Boyer
Signed-off-by: Felipe Balbi
---
drivers/usb/phy/phy.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c
index e6f61e4..29840c2 100644
--- a/drivers/usb/phy/phy.c
+++ b/drivers/usb/phy/phy.c
@@ -130,7 +130,7 @@ struct usb_phy *usb_get_phy(enum usb_phy_type type)
phy = __usb_find_phy(&phy_list, type);
if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
- pr_err("unable to find transceiver of type %s\n",
+ pr_debug("PHY: unable to find transceiver of type %s\n",
usb_phy_type_string(type));
goto err0;
}
@@ -228,7 +228,7 @@ struct usb_phy *usb_get_phy_dev(struct device *dev, u8
index)
phy = __usb_find_phy_dev(dev, &phy_bind_list, index);
if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
- pr_err("unable to find transceiver\n");
+ pr_debug("PHY: unable to find transceiver\n");
goto err0;
}
@@ -424,10 +424,8 @@ int usb_bind_phy(const char *dev_name, u8 index,
unsigned long flags;
phy_bind = kzalloc(sizeof(*phy_bind), GFP_KERNEL);
- if (!phy_bind) {
- pr_err("phy_bind(): No memory for phy_bind");
+ if (!phy_bind)
return -ENOMEM;
- }
phy_bind->dev_name = dev_name;
phy_bind->phy_dev_name = phy_dev_name;
--
1.8.5.2
--
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 v5 00/12] libusbg: Cleanup, bug fix and refactoring.
Dear Matt, Please excuse me my passivity after discussuon about libusbg some time ago. I had to close some other issues before taking up this one. Recently I looked into code of libusbg, build it and found some errors which are fixed in attached patches. Moreover I have done some clean up and minor refactoring. I have used some defines made by Stanislaw Wadas, so this patch depends on: http://thread.gmane.org/gmane.linux.usb.general/102014 I hope that you will accept enclosed patched and update the repository. -- Best regards, Krzysztof Opasiak Samsung R&D Institute Poland Samsung Electronics --- Changes since v4: - add missing break statement in switch - remove change-id's form commit messages Changes since v3: - change header guards name - change one comment to /* */ - rebase onto current HEAD Changes since v2: - replace // comments with /* */ - add missing parenthesis in commit message - fix minor spelling mistakes in commit messages Changes since v1: - replace memcpy with direct structure assignment - Change goto lables on more meaningful in example - Remove additional check in gadget_read_string Krzysztof Opasiak (12): libusbg: Surround header with include guards. libusbg: Add missing return statement in non-void functions. libusbg: Fix gadget-acm-ecm example to cleanup at exit. libusbg: Move directory creation before writing attributes. libusbg: Fix memory leak when unable to create directory. libusbg: Add error handling to gadget_read_string(). libusbg: Add missing config attrs parsing while new config creation. libusbg: Separate parsing gadget attributes and strings. libusbg: Initialize gadget attributes and strings while gadget creation. libusbg: Move symlink creation after memory allocation. libusbg: Replace memcpy with structure assignment. libusbg: Replace directory names with defines. examples/gadget-acm-ecm.c | 20 include/usbg/usbg.h |4 ++ src/usbg.c| 111 + 3 files changed, 86 insertions(+), 49 deletions(-) -- 1.7.9.5 -- 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 v5 12/12] libusbg: Replace directory names with defines.
Replace strings, functions, configs strings placed
everywhere in code with macro defintions STRINGS_DIR,
FUNCTIONS_DIR and CONFIGS_DIR.
Signed-off-by: Krzysztof Opasiak
---
src/usbg.c | 39 ---
1 file changed, 24 insertions(+), 15 deletions(-)
diff --git a/src/usbg.c b/src/usbg.c
index fe20c16..b2d3596 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -26,6 +26,10 @@
#include
#include
+#define STRINGS_DIR "strings"
+#define CONFIGS_DIR "configs"
+#define FUNCTIONS_DIR "functions"
+
/**
* @file usbg.c
* @todo Handle buffer overflows
@@ -223,7 +227,7 @@ static int usbg_parse_functions(char *path, struct gadget
*g)
struct dirent **dent;
char fpath[USBG_MAX_PATH_LENGTH];
- sprintf(fpath, "%s/%s/functions", path, g->name);
+ sprintf(fpath, "%s/%s/%s", path, g->name, FUNCTIONS_DIR);
TAILQ_INIT(&g->functions);
@@ -297,7 +301,7 @@ static int usbg_parse_configs(char *path, struct gadget *g)
struct dirent **dent;
char cpath[USBG_MAX_PATH_LENGTH];
- sprintf(cpath, "%s/%s/configs", path, g->name);
+ sprintf(cpath, "%s/%s/%s", path, g->name, CONFIGS_DIR);
TAILQ_INIT(&g->configs);
@@ -333,9 +337,14 @@ static void usbg_parse_attrs(char *path, struct gadget *g)
static void usbg_parse_strings(char *path, struct gadget *g)
{
/* Strings - hardcoded to U.S. English only for now */
- usbg_read_string(path, g->name, "strings/0x409/serialnumber",
g->str_ser);
- usbg_read_string(path, g->name, "strings/0x409/manufacturer",
g->str_mnf);
- usbg_read_string(path, g->name, "strings/0x409/product", g->str_prd);
+ int lang = LANG_US_ENG;
+ char spath[USBG_MAX_PATH_LENGTH];
+
+ sprintf(spath, "%s/%s/%s/0x%x", path, g->name, STRINGS_DIR, lang);
+
+ usbg_read_string(spath, "", "serialnumber", g->str_ser);
+ usbg_read_string(spath, "", "manufacturer", g->str_mnf);
+ usbg_read_string(spath, "", "product", g->str_prd);
}
static int usbg_parse_gadgets(char *path, struct state *s)
@@ -354,7 +363,7 @@ static int usbg_parse_gadgets(char *path, struct state *s)
g->parent = s;
/* UDC bound to, if any */
usbg_read_string(path, g->name, "UDC", g->udc);
- usbg_parse_configs(path, g);
+ usbg_parse_attrs(path, g);
usbg_parse_strings(path, g);
usbg_parse_functions(path, g);
usbg_parse_configs(path, g);
@@ -596,7 +605,7 @@ void usbg_set_gadget_serial_number(struct gadget *g, int
lang, char *serno)
{
char path[USBG_MAX_PATH_LENGTH];
- sprintf(path, "%s/%s/%s/0x%x", g->path, g->name, "strings", lang);
+ sprintf(path, "%s/%s/%s/0x%x", g->path, g->name, STRINGS_DIR, lang);
mkdir(path, S_IRWXU|S_IRWXG|S_IRWXO);
@@ -609,7 +618,7 @@ void usbg_set_gadget_manufacturer(struct gadget *g, int
lang, char *mnf)
{
char path[USBG_MAX_PATH_LENGTH];
- sprintf(path, "%s/%s/%s/0x%x", g->path, g->name, "strings", lang);
+ sprintf(path, "%s/%s/%s/0x%x", g->path, g->name, STRINGS_DIR, lang);
mkdir(path, S_IRWXU|S_IRWXG|S_IRWXO);
@@ -622,7 +631,7 @@ void usbg_set_gadget_product(struct gadget *g, int lang,
char *prd)
{
char path[USBG_MAX_PATH_LENGTH];
- sprintf(path, "%s/%s/%s/0x%x", g->path, g->name, "strings", lang);
+ sprintf(path, "%s/%s/%s/0x%x", g->path, g->name, STRINGS_DIR, lang);
mkdir(path, S_IRWXU|S_IRWXG|S_IRWXO);
@@ -651,7 +660,7 @@ struct function *usbg_create_function(struct gadget *g,
enum function_type type,
return NULL;
}
- sprintf(fpath, "%s/%s/functions/%s", g->path, g->name, name);
+ sprintf(fpath, "%s/%s/%s/%s", g->path, g->name, FUNCTIONS_DIR, name);
f = malloc(sizeof(struct function));
if (!f) {
@@ -660,7 +669,7 @@ struct function *usbg_create_function(struct gadget *g,
enum function_type type,
}
strcpy(f->name, name);
- sprintf(f->path, "%s/%s/%s", g->path, g->name, "functions");
+ sprintf(f->path, "%s/%s/%s", g->path, g->name, FUNCTIONS_DIR);
f->type = type;
ret = mkdir(fpath, S_IRWXU|S_IRWXG|S_IRWXO);
@@ -706,7 +715,7 @@ struct config *usbg_create_config(struct gadget *g, char
*name)
return NULL;
}
- sprintf(cpath, "%s/%s/configs/%s", g->path, g->name, name);
+ sprintf(cpath, "%s/%s/%s/%s", g->path, g->name, CONFIGS_DIR, name);
c = malloc(sizeof(struct config));
if (!c) {
@@ -716,7 +725,7 @@ struct config *usbg_create_config(struct gadget *g, char
*name)
TAILQ_INIT(&c->bindings);
strcpy(c->name, name);
- sprintf(c->path, "%s/%s/%s/%s", g->path, g->name, "configs", name);
+ sprintf(c->path, "%s/%s/%s", g->path, g->name, CONFIGS_DIR);
ret = mkdir(cpath, S_IRWXU|S_IRWXG|S_IRWXO);
if (ret < 0) {
@@ -759,7 +768,7 @@ void us
[PATCH v5 11/12] libusbg: Replace memcpy with structure assignment.
Use the assign operator for structure instead of using
memcpy with hard coded size.
Signed-off-by: Krzysztof Opasiak
---
src/usbg.c |8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/usbg.c b/src/usbg.c
index 515239a..fe20c16 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -198,12 +198,12 @@ static void usbg_parse_function_attrs(struct function *f)
usbg_read_string(f->path, f->name, "dev_addr", str_addr);
addr = ether_aton(str_addr);
if (addr)
- memcpy(&f->attr.net.dev_addr, addr, sizeof(struct
ether_addr));
+ f->attr.net.dev_addr = *addr;
usbg_read_string(f->path, f->name, "host_addr", str_addr);
addr = ether_aton(str_addr);
if(addr)
- memcpy(&f->attr.net.host_addr, addr, sizeof(struct
ether_addr));
+ f->attr.net.host_addr = *addr;
usbg_read_string(f->path, f->name, "ifname",
f->attr.net.ifname);
f->attr.net.qmult = usbg_read_dec(f->path, f->name, "qmult");
@@ -867,7 +867,7 @@ void usbg_set_net_dev_addr(struct function *f, struct
ether_addr *dev_addr)
{
char *str_addr;
- memcpy(&f->attr.net.dev_addr, dev_addr, 6);
+ f->attr.net.dev_addr = *dev_addr;
str_addr = ether_ntoa(dev_addr);
usbg_write_string(f->path, f->name, "dev_addr", str_addr);
@@ -877,7 +877,7 @@ void usbg_set_net_host_addr(struct function *f, struct
ether_addr *host_addr)
{
char *str_addr;
- memcpy(&f->attr.net.host_addr, host_addr, 6);
+ f->attr.net.host_addr = *host_addr;
str_addr = ether_ntoa(host_addr);
usbg_write_string(f->path, f->name, "host_addr", str_addr);
--
1.7.9.5
--
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 v5 08/12] libusbg: Separate parsing gadget attributes and strings.
Gadget attributes and strings are logically independent,
so they should be initialized in separate functions.
Signed-off-by: Krzysztof Opasiak
---
src/usbg.c | 11 +++
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/src/usbg.c b/src/usbg.c
index ab9f253..11a9abf 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -319,9 +319,6 @@ static int usbg_parse_configs(char *path, struct gadget *g)
static void usbg_parse_attrs(char *path, struct gadget *g)
{
- /* UDC bound to, if any */
- usbg_read_string(path, g->name, "UDC", g->udc);
-
/* Actual attributes */
g->dclass = usbg_read_hex(path, g->name, "bDeviceClass");
g->dsubclass = usbg_read_hex(path, g->name, "bDeviceSubClass");
@@ -331,7 +328,10 @@ static void usbg_parse_attrs(char *path, struct gadget *g)
g->bcdusb = usbg_read_hex(path, g->name, "bcdUSB");
g->vendor = usbg_read_hex(path, g->name, "idVendor");
g->product = usbg_read_hex(path, g->name, "idProduct");
+}
+static void usbg_parse_strings(char *path, struct gadget *g)
+{
/* Strings - hardcoded to U.S. English only for now */
usbg_read_string(path, g->name, "strings/0x409/serialnumber",
g->str_ser);
usbg_read_string(path, g->name, "strings/0x409/manufacturer",
g->str_mnf);
@@ -352,7 +352,10 @@ static int usbg_parse_gadgets(char *path, struct state *s)
strcpy(g->name, dent[i]->d_name);
strcpy(g->path, s->path);
g->parent = s;
- usbg_parse_attrs(path, g);
+ /* UDC bound to, if any */
+ usbg_read_string(path, g->name, "UDC", g->udc);
+ usbg_parse_configs(path, g);
+ usbg_parse_strings(path, g);
usbg_parse_functions(path, g);
usbg_parse_configs(path, g);
TAILQ_INSERT_TAIL(&s->gadgets, g, gnode);
--
1.7.9.5
--
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 v5 06/12] libusbg: Add error handling to gadget_read_string().
Add error handling when gadget_read_buf() returns NULL.
If read of string fails, the string should be set as empty.
Signed-off-by: Krzysztof Opasiak
---
src/usbg.c | 23 +--
1 file changed, 17 insertions(+), 6 deletions(-)
diff --git a/src/usbg.c b/src/usbg.c
index ca444db..6f752a9 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -123,11 +123,18 @@ static int usbg_read_int(char *path, char *name, char
*file, int base)
static void usbg_read_string(char *path, char *name, char *file, char *buf)
{
- char *p;
+ char *p = NULL;
+
+ p = usbg_read_buf(path, name, file, buf);
+ /* Check whether read was successful */
+ if (p != NULL) {
+ if ((p = strchr(buf, '\n')) != NULL)
+ *p = '\0';
+ } else {
+ /* Set this as empty string */
+ *buf = '\0';
+ }
- usbg_read_buf(path, name, file, buf);
- if ((p = strchr(buf, '\n')) != NULL)
- *p = '\0';
}
static void usbg_write_buf(char *path, char *name, char *file, char *buf)
@@ -190,10 +197,14 @@ static void usbg_parse_function_attrs(struct function *f)
case F_RNDIS:
usbg_read_string(f->path, f->name, "dev_addr", str_addr);
addr = ether_aton(str_addr);
- memcpy(&f->attr.net.dev_addr, addr, 6);
+ if (addr)
+ memcpy(&f->attr.net.dev_addr, addr, sizeof(struct
ether_addr));
+
usbg_read_string(f->path, f->name, "host_addr", str_addr);
addr = ether_aton(str_addr);
- memcpy(&f->attr.net.host_addr, addr, 6);
+ if(addr)
+ memcpy(&f->attr.net.host_addr, addr, sizeof(struct
ether_addr));
+
usbg_read_string(f->path, f->name, "ifname",
f->attr.net.ifname);
f->attr.net.qmult = usbg_read_dec(f->path, f->name, "qmult");
break;
--
1.7.9.5
--
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 v5 10/12] libusbg: Move symlink creation after memory allocation.
Move creation of symlink after memory allocation for
binding structure. Fix missing initialization of parent.
Signed-off-by: Krzysztof Opasiak
---
src/usbg.c | 13 +++--
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/src/usbg.c b/src/usbg.c
index 9ee3d35..515239a 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -794,21 +794,22 @@ int usbg_add_config_function(struct config *c, char
*name, struct function *f)
sprintf(bpath, "%s/%s", c->path, name);
sprintf(fpath, "%s/%s", f->path, f->name);
- ret = symlink(fpath, bpath);
- if (ret < 0) {
- ERRORNO("%s -> %s\n", bpath, fpath);
- return ret;
- }
-
b = malloc(sizeof(struct binding));
if (!b) {
ERRORNO("allocating binding\n");
return -1;
}
+ ret = symlink(fpath, bpath);
+ if (ret < 0) {
+ ERRORNO("%s -> %s\n", bpath, fpath);
+ return ret;
+ }
+
strcpy(b->name, name);
strcpy(b->path, bpath);
b->target = f;
+ b->parent = c;
/* Insert in string order */
if (TAILQ_EMPTY(&c->bindings) ||
--
1.7.9.5
--
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 v5 09/12] libusbg: Initialize gadget attributes and strings while gadget creation.
Fix gadget_create_gadget function to initialize gadget attributes
and strings with default values provided by kernel.
Signed-off-by: Krzysztof Opasiak
---
src/usbg.c |5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/src/usbg.c b/src/usbg.c
index 11a9abf..9ee3d35 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -526,8 +526,6 @@ struct gadget *usbg_create_gadget(struct state *s, char
*name,
strcpy(g->name, name);
sprintf(g->path, "%s", s->path);
g->parent = s;
- g->vendor = vendor;
- g->product = product;
ret = mkdir(gpath, S_IRWXU|S_IRWXG|S_IRWXO);
if (ret < 0) {
@@ -539,6 +537,9 @@ struct gadget *usbg_create_gadget(struct state *s, char
*name,
usbg_write_hex16(s->path, name, "idVendor", vendor);
usbg_write_hex16(s->path, name, "idProduct", product);
+ usbg_parse_attrs(s->path, g);
+ usbg_parse_strings(s->path, g);
+
/* Insert in string order */
if (TAILQ_EMPTY(&s->gadgets) ||
(strcmp(name, TAILQ_FIRST(&s->gadgets)->name) < 0))
--
1.7.9.5
--
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 v5 05/12] libusbg: Fix memory leak when unable to create directory.
Free the memory allocated for gadget/config/function structure
when faild to create suitable directory.
Signed-off-by: Krzysztof Opasiak
---
src/usbg.c |3 +++
1 file changed, 3 insertions(+)
diff --git a/src/usbg.c b/src/usbg.c
index e9bd418..ca444db 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -518,6 +518,7 @@ struct gadget *usbg_create_gadget(struct state *s, char
*name,
ret = mkdir(gpath, S_IRWXU|S_IRWXG|S_IRWXO);
if (ret < 0) {
ERRORNO("%s\n", gpath);
+ free(g);
return NULL;
}
@@ -650,6 +651,7 @@ struct function *usbg_create_function(struct gadget *g,
enum function_type type,
ret = mkdir(fpath, S_IRWXU|S_IRWXG|S_IRWXO);
if (ret < 0) {
ERRORNO("%s\n", fpath);
+ free(f);
return NULL;
}
@@ -704,6 +706,7 @@ struct config *usbg_create_config(struct gadget *g, char
*name)
ret = mkdir(cpath, S_IRWXU|S_IRWXG|S_IRWXO);
if (ret < 0) {
ERRORNO("%s\n", cpath);
+ free(c);
return NULL;
}
--
1.7.9.5
--
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 v5 02/12] libusbg: Add missing return statement in non-void functions.
Add return 0 in functions which return non-void to suppress compiler complaint. Signed-off-by: Krzysztof Opasiak --- src/usbg.c |4 1 file changed, 4 insertions(+) diff --git a/src/usbg.c b/src/usbg.c index c88358d..b13c666 100644 --- a/src/usbg.c +++ b/src/usbg.c @@ -228,6 +228,8 @@ static int usbg_parse_functions(char *path, struct gadget *g) free(dent[i]); } free(dent); + + return 0; } static void usbg_parse_config_attrs(struct config *c) @@ -300,6 +302,8 @@ static int usbg_parse_configs(char *path, struct gadget *g) free(dent[i]); } free(dent); + + return 0; } static void usbg_parse_attrs(char *path, struct gadget *g) -- 1.7.9.5 -- 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 v5 04/12] libusbg: Move directory creation before writing attributes.
Change order of gadget creation and attribute writting
to fix No such file or directory error while creating
new gadget.
Signed-off-by: Krzysztof Opasiak
---
src/usbg.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/src/usbg.c b/src/usbg.c
index b13c666..e9bd418 100644
--- a/src/usbg.c
+++ b/src/usbg.c
@@ -515,15 +515,15 @@ struct gadget *usbg_create_gadget(struct state *s, char
*name,
g->vendor = vendor;
g->product = product;
- usbg_write_hex16(s->path, name, "idVendor", vendor);
- usbg_write_hex16(s->path, name, "idProduct", product);
-
ret = mkdir(gpath, S_IRWXU|S_IRWXG|S_IRWXO);
if (ret < 0) {
ERRORNO("%s\n", gpath);
return NULL;
}
+ usbg_write_hex16(s->path, name, "idVendor", vendor);
+ usbg_write_hex16(s->path, name, "idProduct", product);
+
/* Insert in string order */
if (TAILQ_EMPTY(&s->gadgets) ||
(strcmp(name, TAILQ_FIRST(&s->gadgets)->name) < 0))
@@ -647,14 +647,14 @@ struct function *usbg_create_function(struct gadget *g,
enum function_type type,
sprintf(f->path, "%s/%s/%s", g->path, g->name, "functions");
f->type = type;
- usbg_parse_function_attrs(f);
-
ret = mkdir(fpath, S_IRWXU|S_IRWXG|S_IRWXO);
if (ret < 0) {
ERRORNO("%s\n", fpath);
return NULL;
}
+ usbg_parse_function_attrs(f);
+
/* Insert in string order */
if (TAILQ_EMPTY(&g->functions) ||
(strcmp(name, TAILQ_FIRST(&g->functions)->name) < 0))
--
1.7.9.5
--
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 v5 07/12] libusbg: Add missing config attrs parsing while new config creation.
Afther creation of configuration its attributes left uninitialized. Config attrs should be initialized with default values provided by kernel. Signed-off-by: Krzysztof Opasiak --- src/usbg.c |2 ++ 1 file changed, 2 insertions(+) diff --git a/src/usbg.c b/src/usbg.c index 6f752a9..ab9f253 100644 --- a/src/usbg.c +++ b/src/usbg.c @@ -721,6 +721,8 @@ struct config *usbg_create_config(struct gadget *g, char *name) return NULL; } + usbg_parse_config_attrs(c); + /* Insert in string order */ if (TAILQ_EMPTY(&g->configs) || (strcmp(name, TAILQ_FIRST(&g->configs)->name) < 0)) -- 1.7.9.5 -- 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 v5 01/12] libusbg: Surround header with include guards.
Surround header with include guards to protect against multiple inclusion. Signed-off-by: Krzysztof Opasiak --- include/usbg/usbg.h |4 1 file changed, 4 insertions(+) diff --git a/include/usbg/usbg.h b/include/usbg/usbg.h index 319cc1e..d2c9f0c 100644 --- a/include/usbg/usbg.h +++ b/include/usbg/usbg.h @@ -14,6 +14,9 @@ * Lesser General Public License for more details. */ +#ifndef __USBG_H__ +#define __USBG_H__ + #include #include #include @@ -439,3 +442,4 @@ extern void usbg_set_net_qmult(struct function *f, int qmult); /** * @} */ +#endif /* __USBG_H__ */ -- 1.7.9.5 -- 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 v5 03/12] libusbg: Fix gadget-acm-ecm example to cleanup at exit.
Make use of previously unused variable ret to cleanup
after successful gadget creation.
Signed-off-by: Krzysztof Opasiak
---
examples/gadget-acm-ecm.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/examples/gadget-acm-ecm.c b/examples/gadget-acm-ecm.c
index 45d8eff..e84c300 100644
--- a/examples/gadget-acm-ecm.c
+++ b/examples/gadget-acm-ecm.c
@@ -39,13 +39,13 @@ int main(void)
s = usbg_init("/sys/kernel/config");
if (!s) {
fprintf(stderr, "Error on USB gadget init\n");
- goto error1;
+ goto out1;
}
g = usbg_create_gadget(s, "g1", VENDOR, PRODUCT);
if (!g) {
fprintf(stderr, "Error on create gadget\n");
- goto error2;
+ goto out2;
}
usbg_set_gadget_serial_number(g, LANG_US_ENG, "0123456789");
usbg_set_gadget_manufacturer(g, LANG_US_ENG, "Foo Inc.");
@@ -54,25 +54,25 @@ int main(void)
f_acm0 = usbg_create_function(g, F_ACM, "usb0");
if (!f_acm0) {
fprintf(stderr, "Error creating acm0 function\n");
- goto error2;
+ goto out2;
}
f_acm1 = usbg_create_function(g, F_ACM, "usb1");
if (!f_acm1) {
fprintf(stderr, "Error creating acm1 function\n");
- goto error2;
+ goto out2;
}
f_ecm = usbg_create_function(g, F_ECM, "usb0");
if (!f_ecm) {
fprintf(stderr, "Error creating ecm function\n");
- goto error2;
+ goto out2;
}
c = usbg_create_config(g, "c.1");
if (!c) {
fprintf(stderr, "Error creating config\n");
- goto error2;
+ goto out2;
}
usbg_set_config_string(c, LANG_US_ENG, "CDC 2xACM+ECM");
usbg_add_config_function(c, "acm.GS0", f_acm0);
@@ -81,11 +81,11 @@ int main(void)
usbg_enable_gadget(g, DEFAULT_UDC);
- return 0;
+ ret = 0;
-error2:
+out2:
usbg_cleanup(s);
-error1:
- return -EINVAL;
+out1:
+ return ret;
}
--
1.7.9.5
--
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] phy-rcar-usb-gen2: add device tree support
Hello.
On 01/26/2014 08:05 PM, Ben Dooks wrote:
Add support for the phy-rcar-gen2-usb driver to be probed from device tree.
Signed-off-by: Ben Dooks
Reviewed-by: Ian Molton
---
Fixes from v2:
- fix missing of_if patch
(I clearly should not be let near git-rebase when hungry)
Fixes from v1:
- use of_property_reasd-bool()
- remove unused of_id variable
Cc: linux-usb@vger.kernel.org
Cc: linux...@vger.kernel.org
Cc: Magnus Damm
Cc: Simon Horman
Cc: devicet...@vger.kernel.org
---
drivers/usb/phy/phy-rcar-gen2-usb.c | 31 ++-
1 file changed, 26 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/phy/phy-rcar-gen2-usb.c
b/drivers/usb/phy/phy-rcar-gen2-usb.c
index db3ab34..e4665b9 100644
--- a/drivers/usb/phy/phy-rcar-gen2-usb.c
+++ b/drivers/usb/phy/phy-rcar-gen2-usb.c
[...]
@@ -167,6 +168,12 @@ out:
spin_unlock_irqrestore(&priv->lock, flags);
}
+static struct of_device_id rcar_gen2_usb_phy_ofmatch[] = {
+ { .compatible = "renesas,usb-phy-r8a7790", },
+ { .compatible = "renesas,rcar-gen2-usb-phy", },
Frankly speaking, I don't understand the need for the clearly duplicate
entries.
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 1/5] xhci: Don't change the ownership of LINK TRB until all the TRB are written
On Thu, Jan 23, 2014 at 10:35:56AM +, David Laight wrote: > From: Sarah Sharp > > Hi David, > > > > I've been thinking about this some more, and I'd like to propose a much > > simpler solution. > > > > The TD fragment rules didn't go into the xHCI specification until the > > 1.0 revision. The code that follows those rules only seems to trigger > > issues for 0.96 ASMedia hosts, while Intel 1.0 hosts benefit from the > > code when USB ethernet devices are attached. The patch that changed the > > link TRB behavior (commit 6c12db90f19727c76990e7f4801c67a148b30111) was > > submitted in 2010, and the xHCI 1.0 spec didn't come out until later > > that year, so it's unlikely that Synopsys host is a 1.0 host either. > > Ah - so I was right in thinking that a LINK TRB mustn't point to > a TRB that is still owned by the driver. For that *particular* piece of hardware. This is not an issue for other xHCI hosts; this is not something that's spec mandated. > The thing is, that patch has a timing bug, and I think that is what Walt > is hitting. It is there regardless of my NOP change. That does not make sense. Walt confirmed that your new patch does not help him. Therefore either your patch doesn't close the race condition, or Walt's ASMedia host does not suffer from it. Either way, I have no hard proof your patch is necessary and correct. > In order to hit the bug I've fixed you need two things: > 1) An xhci controller that doesn't like 'dangling' LINK TRB. >(These will be the ones that made the 6c12 fix needed.) > and: > 2a) A host system where the timings of the PCIe transfers (especially > the latency) are such that the controller manages to read a > LINK TRB that prepare_transfer() has updated while queueing a > new transfer in response to a transfer completion event. > or: > 2b) To setup a second transfer just as the last transfer is completing. > I'm not sure this can happen for disks, but it could happen for > network traffic. Neither of us has deep technical knowledge of how this host hardware works. We can argue about theoretical race conditions and link TRB pre-fetches and packet pending flags, but in the end, we don't know how this buggy host is designed. We don't know if the timings you mentioned in #2a are actually a problem. In fact, looking back at the discussion around this patch, I brought up the same issue you did about link TRB activation, and the original patch submitter confirmed it was fine: http://marc.info/?l=linux-usb&m=127325508724339&w=2 > So this fix is actually just a version of the 6c12 patch that > actually works! The original patch works, and the theoretical race condition this patchset was supposed to fix is not an issue, based on that link. If your new commit causes issues with Synopsys hosts, we'll address it when someone actually reports the issue. In the meantime, I'm going to go with the simplest fix, and disable the no-op TRB code for pre-1.0 hosts. You are welcome to resubmit this patchset without the patches that rely on the first patch, if you like. 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: [BUG] FL1009: xHCI host not responding to stop endpoint command.
Hi Thomas and Sarah, Thomas Petazzoni writes: > On Thu, 23 Jan 2014 09:24:41 +0100, Arnaud Ebalard wrote: > >> The various Armada-based devices I have are NAS which do not have PCIe >> slots to plug additional devices (everything is soldered). I don't know >> which device Thomas used for its tests. Just in case, I also added Willy >> in CC: who have various boards and may also have done more test with >> additional PCIe devices and CONFIG_PCI_MSI enabled on 3.13 kernel. > > The device I've used to test MSI is a e1000e PCIe Intel network card. > It uses one MSI interrupt, so admittedly, the MSI testing is quite > limited for now. I had a second thought this WE about a previous question from Sarah: my platforms do not have a PCIe extension slots to test other devices but the RN102 does have an additional device connected on the PCIe bus: a Marvell 88SE9170 SATA Controller. I have put below the output of lspci -vvv (on a 3.13.0-rc8 kernel w/ CONFIG_PCI_MSI enabled) in case you can spot something obviously wrong in it: 00:01.0 PCI bridge: Marvell Technology Group Ltd. Device 7846 (prog-if 00 [Normal decode]) Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr+ Stepping- SERR+ FastB2B- DisINTx- Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- TAbort- Reset- FastB2B- PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn- 00:02.0 PCI bridge: Marvell Technology Group Ltd. Device 7846 (prog-if 00 [Normal decode]) Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr+ Stepping- SERR+ FastB2B- DisINTx- Status: Cap- 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- TAbort- Reset- FastB2B- PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn- 01:00.0 SATA controller: Marvell Technology Group Ltd. Device 9170 (rev 12) (prog-if 01 [AHCI 1.0]) Subsystem: Marvell Technology Group Ltd. Device 9170 Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr+ Stepping- SERR+ FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- TAbort- SERR- http://vger.kernel.org/majordomo-info.html
Re: [PATCH] phy-rcar-usb-gen2: add device tree support
On 27/01/14 18:23, Sergei Shtylyov wrote:
Hello.
On 01/26/2014 08:05 PM, Ben Dooks wrote:
[snip]
+static struct of_device_id rcar_gen2_usb_phy_ofmatch[] = {
+{ .compatible = "renesas,usb-phy-r8a7790", },
+{ .compatible = "renesas,rcar-gen2-usb-phy", },
Frankly speaking, I don't understand the need for the clearly
duplicate entries.
Thanks, will look into remove it.
Anyone else have any comments on this?
--
Ben Dooks http://www.codethink.co.uk/
Senior Engineer Codethink - Providing Genius
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2] usb: phy: move some error messages to dev_dbg
the PHY layer is supposed to be optional,
considering some PHY have no control bus
for SW to poke around.
After commit 1ae5799 (usb: hcd: Initialize
USB phy if needed) any HCD which didn't provide
a PHY driver would emit annoying error messages.
In this patch we're decreasing those messages
to dev_dbg for debugging only and so we know where
they're coming from.
Reported-by: Josh Boyer
Signed-off-by: Felipe Balbi
Signed-off-by: Josh Boyer
---
v2: Switch to using dev_dbg
drivers/usb/phy/phy.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c
index e6f61e4..db18011 100644
--- a/drivers/usb/phy/phy.c
+++ b/drivers/usb/phy/phy.c
@@ -130,7 +130,7 @@ struct usb_phy *usb_get_phy(enum usb_phy_type type)
phy = __usb_find_phy(&phy_list, type);
if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
- pr_err("unable to find transceiver of type %s\n",
+ dev_dbg(phy->dev, "unable to find transceiver of type %s\n",
usb_phy_type_string(type));
goto err0;
}
@@ -228,7 +228,7 @@ struct usb_phy *usb_get_phy_dev(struct device *dev, u8
index)
phy = __usb_find_phy_dev(dev, &phy_bind_list, index);
if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
- pr_err("unable to find transceiver\n");
+ dev_dbg(dev, "unable to find transceiver\n");
goto err0;
}
@@ -424,10 +424,8 @@ int usb_bind_phy(const char *dev_name, u8 index,
unsigned long flags;
phy_bind = kzalloc(sizeof(*phy_bind), GFP_KERNEL);
- if (!phy_bind) {
- pr_err("phy_bind(): No memory for phy_bind");
+ if (!phy_bind)
return -ENOMEM;
- }
phy_bind->dev_name = dev_name;
phy_bind->phy_dev_name = phy_dev_name;
--
1.8.5.3
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: phy: move some error messages to dev_dbg
Hello.
On 01/27/2014 10:23 PM, Josh Boyer wrote:
the PHY layer is supposed to be optional,
considering some PHY have no control bus
for SW to poke around.
After commit 1ae5799 (usb: hcd: Initialize
USB phy if needed) any HCD which didn't provide
a PHY driver would emit annoying error messages.
In this patch we're decreasing those messages
to dev_dbg for debugging only and so we know where
they're coming from.
Reported-by: Josh Boyer
Signed-off-by: Felipe Balbi
Signed-off-by: Josh Boyer
---
v2: Switch to using dev_dbg
drivers/usb/phy/phy.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c
index e6f61e4..db18011 100644
--- a/drivers/usb/phy/phy.c
+++ b/drivers/usb/phy/phy.c
@@ -130,7 +130,7 @@ struct usb_phy *usb_get_phy(enum usb_phy_type type)
phy = __usb_find_phy(&phy_list, type);
if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
- pr_err("unable to find transceiver of type %s\n",
+ dev_dbg(phy->dev, "unable to find transceiver of type %s\n",
'phy' is possibly invalid (error ptr) at this point, you cannot
dereference it.
usb_phy_type_string(type));
goto err0;
}
@@ -228,7 +228,7 @@ struct usb_phy *usb_get_phy_dev(struct device *dev, u8
index)
phy = __usb_find_phy_dev(dev, &phy_bind_list, index);
if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
- pr_err("unable to find transceiver\n");
+ dev_dbg(dev, "unable to find transceiver\n");
Same here.
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 v2] usb: phy: move some error messages to dev_dbg
On Mon, Jan 27, 2014 at 3:44 PM, Sergei Shtylyov
wrote:
> Hello.
>
>
> On 01/27/2014 10:23 PM, Josh Boyer wrote:
>
>> the PHY layer is supposed to be optional,
>> considering some PHY have no control bus
>> for SW to poke around.
>
>
>> After commit 1ae5799 (usb: hcd: Initialize
>> USB phy if needed) any HCD which didn't provide
>> a PHY driver would emit annoying error messages.
>
>
>> In this patch we're decreasing those messages
>> to dev_dbg for debugging only and so we know where
>> they're coming from.
>
>
>> Reported-by: Josh Boyer
>> Signed-off-by: Felipe Balbi
>> Signed-off-by: Josh Boyer
>> ---
>
>
>> v2: Switch to using dev_dbg
>
>
>> drivers/usb/phy/phy.c | 8 +++-
>> 1 file changed, 3 insertions(+), 5 deletions(-)
>
>
>> diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c
>> index e6f61e4..db18011 100644
>> --- a/drivers/usb/phy/phy.c
>> +++ b/drivers/usb/phy/phy.c
>> @@ -130,7 +130,7 @@ struct usb_phy *usb_get_phy(enum usb_phy_type type)
>>
>> phy = __usb_find_phy(&phy_list, type);
>> if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
>> - pr_err("unable to find transceiver of type %s\n",
>> + dev_dbg(phy->dev, "unable to find transceiver of type
>> %s\n",
>
>
>'phy' is possibly invalid (error ptr) at this point, you cannot
> dereference it.
Oh, yes. Duh, I should have spotted that.
Felipe, can we just go with your original patch? It avoids having to
worry about the dev parameter to dev_dbg.
josh
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: phy: move some error messages to dev_dbg
On 01/27/2014 11:44 PM, Sergei Shtylyov wrote:
the PHY layer is supposed to be optional,
considering some PHY have no control bus
for SW to poke around.
After commit 1ae5799 (usb: hcd: Initialize
USB phy if needed) any HCD which didn't provide
a PHY driver would emit annoying error messages.
In this patch we're decreasing those messages
to dev_dbg for debugging only and so we know where
they're coming from.
Reported-by: Josh Boyer
Signed-off-by: Felipe Balbi
Signed-off-by: Josh Boyer
---
v2: Switch to using dev_dbg
drivers/usb/phy/phy.c | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c
index e6f61e4..db18011 100644
--- a/drivers/usb/phy/phy.c
+++ b/drivers/usb/phy/phy.c
@@ -130,7 +130,7 @@ struct usb_phy *usb_get_phy(enum usb_phy_type type)
phy = __usb_find_phy(&phy_list, type);
if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
-pr_err("unable to find transceiver of type %s\n",
+dev_dbg(phy->dev, "unable to find transceiver of type %s\n",
'phy' is possibly invalid (error ptr) at this point, you cannot
dereference it.
usb_phy_type_string(type));
goto err0;
}
@@ -228,7 +228,7 @@ struct usb_phy *usb_get_phy_dev(struct device *dev, u8
index)
phy = __usb_find_phy_dev(dev, &phy_bind_list, index);
if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
-pr_err("unable to find transceiver\n");
+dev_dbg(dev, "unable to find transceiver\n");
Same here.
Oh, not at all the same, sorry about that.
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 v2] usb: phy: move some error messages to dev_dbg
Hi,
On Mon, Jan 27, 2014 at 11:44:16PM +0300, Sergei Shtylyov wrote:
> >diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c
> >index e6f61e4..db18011 100644
> >--- a/drivers/usb/phy/phy.c
> >+++ b/drivers/usb/phy/phy.c
> >@@ -130,7 +130,7 @@ struct usb_phy *usb_get_phy(enum usb_phy_type type)
> >
> > phy = __usb_find_phy(&phy_list, type);
> > if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
> >-pr_err("unable to find transceiver of type %s\n",
> >+dev_dbg(phy->dev, "unable to find transceiver of type %s\n",
>
>'phy' is possibly invalid (error ptr) at this point, you cannot
> dereference it.
right, this one can't be a dev_dbg(), unfortunately.
> >@@ -228,7 +228,7 @@ struct usb_phy *usb_get_phy_dev(struct device *dev, u8
> >index)
> >
> > phy = __usb_find_phy_dev(dev, &phy_bind_list, index);
> > if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
> >-pr_err("unable to find transceiver\n");
> >+dev_dbg(dev, "unable to find transceiver\n");
>
>Same here.
here he's using the caller's dev pointer, which is just fine.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v2] usb: phy: move some error messages to dev_dbg
On Mon, Jan 27, 2014 at 02:46:40PM -0500, Josh Boyer wrote:
> On Mon, Jan 27, 2014 at 3:44 PM, Sergei Shtylyov
> wrote:
> > Hello.
> >
> >
> > On 01/27/2014 10:23 PM, Josh Boyer wrote:
> >
> >> the PHY layer is supposed to be optional,
> >> considering some PHY have no control bus
> >> for SW to poke around.
> >
> >
> >> After commit 1ae5799 (usb: hcd: Initialize
> >> USB phy if needed) any HCD which didn't provide
> >> a PHY driver would emit annoying error messages.
> >
> >
> >> In this patch we're decreasing those messages
> >> to dev_dbg for debugging only and so we know where
> >> they're coming from.
> >
> >
> >> Reported-by: Josh Boyer
> >> Signed-off-by: Felipe Balbi
> >> Signed-off-by: Josh Boyer
> >> ---
> >
> >
> >> v2: Switch to using dev_dbg
> >
> >
> >> drivers/usb/phy/phy.c | 8 +++-
> >> 1 file changed, 3 insertions(+), 5 deletions(-)
> >
> >
> >> diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c
> >> index e6f61e4..db18011 100644
> >> --- a/drivers/usb/phy/phy.c
> >> +++ b/drivers/usb/phy/phy.c
> >> @@ -130,7 +130,7 @@ struct usb_phy *usb_get_phy(enum usb_phy_type type)
> >>
> >> phy = __usb_find_phy(&phy_list, type);
> >> if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
> >> - pr_err("unable to find transceiver of type %s\n",
> >> + dev_dbg(phy->dev, "unable to find transceiver of type
> >> %s\n",
> >
> >
> >'phy' is possibly invalid (error ptr) at this point, you cannot
> > dereference it.
>
> Oh, yes. Duh, I should have spotted that.
>
> Felipe, can we just go with your original patch? It avoids having to
> worry about the dev parameter to dev_dbg.
fine by me, no problem. Can I get a Tested-by or Reviewed-by on that ?
(Tested-by gets more points heh)
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v2] usb: phy: move some error messages to dev_dbg
On Mon, 27 Jan 2014, Josh Boyer wrote:
> On Mon, Jan 27, 2014 at 3:44 PM, Sergei Shtylyov
> wrote:
> > Hello.
> >
> >
> > On 01/27/2014 10:23 PM, Josh Boyer wrote:
> >
> >> the PHY layer is supposed to be optional,
> >> considering some PHY have no control bus
> >> for SW to poke around.
> >
> >
> >> After commit 1ae5799 (usb: hcd: Initialize
> >> USB phy if needed) any HCD which didn't provide
> >> a PHY driver would emit annoying error messages.
> >
> >
> >> In this patch we're decreasing those messages
> >> to dev_dbg for debugging only and so we know where
> >> they're coming from.
> >
> >
> >> Reported-by: Josh Boyer
> >> Signed-off-by: Felipe Balbi
> >> Signed-off-by: Josh Boyer
> >> ---
> >
> >
> >> v2: Switch to using dev_dbg
> >
> >
> >> drivers/usb/phy/phy.c | 8 +++-
> >> 1 file changed, 3 insertions(+), 5 deletions(-)
> >
> >
> >> diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c
> >> index e6f61e4..db18011 100644
> >> --- a/drivers/usb/phy/phy.c
> >> +++ b/drivers/usb/phy/phy.c
> >> @@ -130,7 +130,7 @@ struct usb_phy *usb_get_phy(enum usb_phy_type type)
> >>
> >> phy = __usb_find_phy(&phy_list, type);
> >> if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
> >> - pr_err("unable to find transceiver of type %s\n",
> >> + dev_dbg(phy->dev, "unable to find transceiver of type
> >> %s\n",
> >
> >
> >'phy' is possibly invalid (error ptr) at this point, you cannot
> > dereference it.
>
> Oh, yes. Duh, I should have spotted that.
>
> Felipe, can we just go with your original patch? It avoids having to
> worry about the dev parameter to dev_dbg.
Josh, don't worry about it. Simply rewrite the patch using pr_debug
here, but leave the second instance as dev_dbg.
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: [OpenOCD-devel] [libusb] Announcing libusb-1.0.18 (as well as libusbx-1.0.18 *FINAL*)
Hi Pete. What you are saying to me is meaningless. On Mon, 27 Jan 2014 13:59:22 +, Pete Batard wrote: > OK, so Peter's propaganda has worked. Pete, you are young and have many good qualities. One of them is to analyze and find errors. But you have a problem. You feel attacked whenever anyone says something that you do not agree with. You start to defend yourself, instead of trying to understand the actual problem, thus you misunderstand people around you. If you do not try to understand the problems, you will end up fixing the wrong errors and mark the errors "fixed". The errors won't go away that way. You need to understand what people are saying instead of pretending that you understand. I am aware that you will feel attacked by what I'm writing, but that's not the purpose; the purpose is that you become aware of this, so you will have a chance of winning. Love Jens -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: phy: move some error messages to debug
On Mon, Jan 27, 2014 at 10:27 AM, Felipe Balbi wrote: > the PHY layer is supposed to be optional, > considering some PHY have no control bus > for SW to poke around. > > After commit 1ae5799 (usb: hcd: Initialize > USB phy if needed) any HCD which didn't provide > a PHY driver would emit annoying error messages. > > In this patch we're decreasing those messages > to debugging only and we also add a PHY prefix > so we know where they're coming from. > > Reported-by: Josh Boyer > Signed-off-by: Felipe Balbi Tested-by: Josh Boyer Thanks. josh -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: phy: move some error messages to dev_dbg
On Mon, Jan 27, 2014 at 3:27 PM, Alan Stern wrote:
> On Mon, 27 Jan 2014, Josh Boyer wrote:
>
>> On Mon, Jan 27, 2014 at 3:44 PM, Sergei Shtylyov
>> wrote:
>> > Hello.
>> >
>> >
>> > On 01/27/2014 10:23 PM, Josh Boyer wrote:
>> >
>> >> the PHY layer is supposed to be optional,
>> >> considering some PHY have no control bus
>> >> for SW to poke around.
>> >
>> >
>> >> After commit 1ae5799 (usb: hcd: Initialize
>> >> USB phy if needed) any HCD which didn't provide
>> >> a PHY driver would emit annoying error messages.
>> >
>> >
>> >> In this patch we're decreasing those messages
>> >> to dev_dbg for debugging only and so we know where
>> >> they're coming from.
>> >
>> >
>> >> Reported-by: Josh Boyer
>> >> Signed-off-by: Felipe Balbi
>> >> Signed-off-by: Josh Boyer
>> >> ---
>> >
>> >
>> >> v2: Switch to using dev_dbg
>> >
>> >
>> >> drivers/usb/phy/phy.c | 8 +++-
>> >> 1 file changed, 3 insertions(+), 5 deletions(-)
>> >
>> >
>> >> diff --git a/drivers/usb/phy/phy.c b/drivers/usb/phy/phy.c
>> >> index e6f61e4..db18011 100644
>> >> --- a/drivers/usb/phy/phy.c
>> >> +++ b/drivers/usb/phy/phy.c
>> >> @@ -130,7 +130,7 @@ struct usb_phy *usb_get_phy(enum usb_phy_type type)
>> >>
>> >> phy = __usb_find_phy(&phy_list, type);
>> >> if (IS_ERR(phy) || !try_module_get(phy->dev->driver->owner)) {
>> >> - pr_err("unable to find transceiver of type %s\n",
>> >> + dev_dbg(phy->dev, "unable to find transceiver of type
>> >> %s\n",
>> >
>> >
>> >'phy' is possibly invalid (error ptr) at this point, you cannot
>> > dereference it.
>>
>> Oh, yes. Duh, I should have spotted that.
>>
>> Felipe, can we just go with your original patch? It avoids having to
>> worry about the dev parameter to dev_dbg.
>
> Josh, don't worry about it. Simply rewrite the patch using pr_debug
> here, but leave the second instance as dev_dbg.
Yeah, not a big deal. I can send a fixed up patch, but it likely
won't be until tomorrow. In the meantime, I tested Felipe's patch and
replied accordingly.
josh
--
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: [BUG] FL1009: xHCI host not responding to stop endpoint command.
Hi Willy, Willy Tarreau writes: >> > I don't know if it should have included FL1009, it was just a guess, >> > based on the fact that the 0x1000 and 0x1400 devices did need MSI >> > disabled. I can attempt to ask the Fresco Logic folks I know, but I'm >> > not sure if/when I'll get a response back. >> > >> > That still doesn't necessarily rule out MSI issues in the Marvell PCI >> > host controller code. Can you attach another PCI device with MSI >> > support under the host and see if it works? >> >> The various Armada-based devices I have are NAS which do not have PCIe >> slots to plug additional devices (everything is soldered). I don't know >> which device Thomas used for its tests. Just in case, I also added Willy >> in CC: who have various boards and may also have done more test with >> additional PCIe devices and CONFIG_PCI_MSI enabled on 3.13 kernel. > > I've been running an intel i350 dual-port NIC (igb driver) supporting > MSI on the mirabox, and it used to work in 3.10+many of the patches > coming from the Free-electrons team. Some recent changes to the PCI > code introduced a regression preventing this driver from correctly > registering an MSI interrupt, and I did not have enough time to > investigate it deep enough to fix it. That said, I know how to hack > it to work again, so if it can be of any use, I can run a test on > the mirabox (armada370) and on the XPGP board (armadaXP). Thanks for the proposal, Willy. I guess Thomas can tell better than me what kind of tests would help ruling out a problem in MSI support and put the blame on FL chip ;-) Thomas, if you need me to test something on some of my platforms, do not hesitate. Cheers, a+ -- 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/1] usbip/userspace/libsrc/names.c: memory leak
From: Heinrich Schuchardt
p is freed if NULL.
p is leaked if second calloc fails.
Signed-off-by: Heinrich Schuchardt
---
drivers/staging/usbip/userspace/libsrc/names.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/usbip/userspace/libsrc/names.c
b/drivers/staging/usbip/userspace/libsrc/names.c
index 3c8d28b..b2904e8 100644
--- a/drivers/staging/usbip/userspace/libsrc/names.c
+++ b/drivers/staging/usbip/userspace/libsrc/names.c
@@ -170,12 +170,12 @@ static void *my_malloc(size_t size)
p = calloc(1, sizeof(struct pool));
if (!p) {
- free(p);
return NULL;
}
p->mem = calloc(1, size);
if (!p->mem)
+ free(p);
return NULL;
p->next = pool_head;
--
1.7.10.4
--
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: RocketU 1144B 1144BM 2-ports detected instead of 4-ports
The precise meaning of dead would be: Absolutely nothing is logged when I plug a device into one of the other two ports. But for instance I have a bus powered usb sound card that has an led indicating it's on, that led illuminates. I also have a usb dvd drive which has an led that illuminates if it negotiates usb2.0 speed; this led does NOT illuminate. I will build a 3.13 kernel tomorrow. Let me know if the following logs are what you're looking for on the new kernel: if I've missed anything or included any crap you don't want. [0.688127] pci :08:00.0: [1b21:1042] type 00 class 0x0c0330 [0.688150] pci :08:00.0: reg 0x10: [mem 0xcf80-0xcf807fff 64bit] [0.688264] pci :08:00.0: PME# supported from D3hot D3cold ... [0.695545] pci :0a:00.0: [1b21:1042] type 00 class 0x0c0330 [0.695569] pci :0a:00.0: reg 0x10: [mem 0xcf70-0xcf707fff 64bit] [0.695683] pci :0a:00.0: PME# supported from D3hot D3cold ... [1.383101] xhci_hcd :08:00.0: xHCI Host Controller [1.383302] scsi0 : sata_mv [1.383429] xhci_hcd :08:00.0: new USB bus registered, assigned bus number 1 [1.393427] scsi1 : sata_mv [1.395974] scsi2 : sata_mv [1.398951] scsi3 : sata_mv [1.401726] scsi4 : sata_mv [1.405166] scsi5 : sata_mv [1.407752] scsi6 : sata_mv [1.410397] scsi7 : sata_mv [1.413130] ata1: SATA max UDMA/133 mmio m1048576@0xcf40 port 0xcf422000 irq 20 [1.413358] ata2: SATA max UDMA/133 mmio m1048576@0xcf40 port 0xcf424000 irq 20 [1.413577] ata3: SATA max UDMA/133 mmio m1048576@0xcf40 port 0xcf426000 irq 20 [1.413769] ata4: SATA max UDMA/133 mmio m1048576@0xcf40 port 0xcf428000 irq 20 [1.413977] ata5: SATA max UDMA/133 mmio m1048576@0xcf40 port 0xcf432000 irq 20 [1.414220] ata6: SATA max UDMA/133 mmio m1048576@0xcf40 port 0xcf434000 irq 20 [1.414438] ata7: SATA max UDMA/133 mmio m1048576@0xcf40 port 0xcf436000 irq 20 [1.414647] ata8: SATA max UDMA/133 mmio m1048576@0xcf40 port 0xcf438000 irq 20 [1.446883] xhci_hcd :08:00.0: irq 74 for MSI/MSI-X [1.446891] xhci_hcd :08:00.0: irq 75 for MSI/MSI-X [1.446899] xhci_hcd :08:00.0: irq 76 for MSI/MSI-X [1.446905] xhci_hcd :08:00.0: irq 77 for MSI/MSI-X [1.446911] xhci_hcd :08:00.0: irq 78 for MSI/MSI-X [1.446917] xhci_hcd :08:00.0: irq 79 for MSI/MSI-X [1.446924] xhci_hcd :08:00.0: irq 80 for MSI/MSI-X [1.446930] xhci_hcd :08:00.0: irq 81 for MSI/MSI-X [1.448604] ACPI: PCI Interrupt Link [LNKB] enabled at IRQ 16 [1.448980] mptbase: ioc1: Initiating bringup [1.449588] usb usb1: New USB device found, idVendor=1d6b, idProduct=0002 [1.449790] usb usb1: New USB device strings: Mfr=3, Product=2, SerialNumber=1 [1.450015] usb usb1: Product: xHCI Host Controller [1.450208] usb usb1: Manufacturer: Linux 3.12-1-rt-amd64 xhci_hcd [1.450392] usb usb1: SerialNumber: :08:00.0 [1.454624] hub 1-0:1.0: USB hub found [1.454918] hub 1-0:1.0: 2 ports detected [1.458912] xhci_hcd :08:00.0: xHCI Host Controller [1.459106] xhci_hcd :08:00.0: new USB bus registered, assigned bus number 2 [1.459393] usb usb2: New USB device found, idVendor=1d6b, idProduct=0003 [1.459588] usb usb2: New USB device strings: Mfr=3, Product=2, SerialNumber=1 [1.459791] usb usb2: Product: xHCI Host Controller [1.45] usb usb2: Manufacturer: Linux 3.12-1-rt-amd64 xhci_hcd [1.460199] usb usb2: SerialNumber: :08:00.0 [1.464525] hub 2-0:1.0: USB hub found [1.464740] hub 2-0:1.0: 2 ports detected [1.483030] xhci_hcd :0a:00.0: xHCI Host Controller [1.483304] xhci_hcd :0a:00.0: new USB bus registered, assigned bus number 3 [1.546906] xhci_hcd :0a:00.0: irq 82 for MSI/MSI-X [1.546913] xhci_hcd :0a:00.0: irq 83 for MSI/MSI-X [1.546921] xhci_hcd :0a:00.0: irq 84 for MSI/MSI-X [1.546928] xhci_hcd :0a:00.0: irq 85 for MSI/MSI-X [1.546934] xhci_hcd :0a:00.0: irq 86 for MSI/MSI-X [1.546940] xhci_hcd :0a:00.0: irq 87 for MSI/MSI-X [1.548538] usb usb3: New USB device found, idVendor=1d6b, idProduct=0002 [1.548806] usb usb3: New USB device strings: Mfr=3, Product=2, SerialNumber=1 [1.549068] usb usb3: Product: xHCI Host Controller [1.549297] usb usb3: Manufacturer: Linux 3.12-1-rt-amd64 xhci_hcd [1.549524] usb usb3: SerialNumber: :0a:00.0 :08:00.0 USB controller: ASMedia Technology Inc. ASM1042 SuperSpeed USB Host Controller (prog-if 30 [XHCI]) Subsystem: Device 174c:2104 Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- Stepping- SERR+ FastB2B- DisINTx+ Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- Latency: 0, Cache Line Size: 64 bytes Interrupt: pin A routed to IRQ 16 Region 0: Memory at cf80 (64-bit, non-prefetchable) [size=32K]
Re: [OpenOCD-devel] [libusb] Announcing libusb-1.0.18 (as well as libusbx-1.0.18 *FINAL*)
On 2014.01.27 20:44, Jens Bauer wrote: Pete, you are young Not a good start. What if I told you I started programming in the mid 80s? Do I still qualify? But you have a problem. You feel attacked whenever anyone says something that you do not agree with. Yes, I tend to be annoyed when people seem to suggest to go against what the majority of our collective users and contributors has regularly expressed they wanted which is: a single libusb, where bugs get fixed on regular basis, and where much requested features, such as hotplug support, do make it into a release. If your suggestion wasn't that it might be for the best if libusb was split into 2 separate projects, again, right after we finally managed to undo the earlier split which the vast majority of all of us (users, contributors, maintainers) would really have preferred to do without, then please clarify. Also, as you should know, most projects, that have the capacity, tend to go around with a development branch and a stable branch, which is actually also something we've been toying about, with a long proposed but yet to be implemented 2.0 branch. How comes you didn't mention something like that? So yeah, I am flabbergasted as to why you would even *remotely* suggest to go back to splitting libusb, especially when your argument appears to boil down to one's subjective perception of stability associated with a name. instead of trying to understand the actual problem You can try to clarify how one is to interpret what you said earlier. Or you can go on a meaningless ad-hominen. Your choice. Regards, /Pete -- 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/1] usbip/userspace/libsrc/names.c: memory leak
On Mon, Jan 27, 2014 at 11:29:48PM +0100, xypron.g...@gmx.de wrote: > From: Heinrich Schuchardt > > p is freed if NULL. Not a real problem, right? > p is leaked if second calloc fails. You just created a new bug in your "fix" :( Please run your patches through scripts/checkpatch.pl, odds are, it would have caught this error. greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: RocketU 1144B 1144BM 2-ports detected instead of 4-ports
On Mon, Jan 27, 2014 at 02:29:53PM -0800, nubjub wrote: > The precise meaning of dead would be: Absolutely nothing is logged when > I plug a device into one of the other two ports. But for instance I have > a bus powered usb sound card that has an led indicating it's on, that > led illuminates. I also have a usb dvd drive which has an led that > illuminates if it negotiates usb2.0 speed; this led does NOT illuminate. > > I will build a 3.13 kernel tomorrow. Let me know if the following logs > are what you're looking for on the new kernel: if I've missed anything > or included any crap you don't want. Close, can you provide the full boot log after the pci device is created? Or was this it? It shows 2 XHCI controllers in the system, perhaps one of them isn't working for the "failure"? Also, just the output of 'lspci' (no -v option), and 'lsusb' (again, no '-v') will be fine to start with. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] usbip/userspace/libsrc/names.c: memory leak
Hello.
On 01/28/2014 01:29 AM, xypron.g...@gmx.de wrote:
From: Heinrich Schuchardt
p is freed if NULL.
This is not an issue.
p is leaked if second calloc fails.
Signed-off-by: Heinrich Schuchardt
---
drivers/staging/usbip/userspace/libsrc/names.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/usbip/userspace/libsrc/names.c
b/drivers/staging/usbip/userspace/libsrc/names.c
index 3c8d28b..b2904e8 100644
--- a/drivers/staging/usbip/userspace/libsrc/names.c
+++ b/drivers/staging/usbip/userspace/libsrc/names.c
@@ -170,12 +170,12 @@ static void *my_malloc(size_t size)
p = calloc(1, sizeof(struct pool));
if (!p) {
- free(p);
return NULL;
}
{} not needed anymore.
p->mem = calloc(1, size);
if (!p->mem)
+ free(p);
return NULL;
How about {} around the *if* arm?
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 1/1] usbip/userspace/libsrc/names.c: memory leak
On Mon, Jan 27, 2014 at 11:29:48PM +0100, xypron.g...@gmx.de wrote:
> From: Heinrich Schuchardt
Fix your email account so we can get this automatically from your email.
Currently the From header on your email is just:
From: xypron.g...@gmx.de
without your name.
>
> p is freed if NULL.
> p is leaked if second calloc fails.
>
> Signed-off-by: Heinrich Schuchardt
> ---
> drivers/staging/usbip/userspace/libsrc/names.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/usbip/userspace/libsrc/names.c
> b/drivers/staging/usbip/userspace/libsrc/names.c
> index 3c8d28b..b2904e8 100644
> --- a/drivers/staging/usbip/userspace/libsrc/names.c
> +++ b/drivers/staging/usbip/userspace/libsrc/names.c
> @@ -170,12 +170,12 @@ static void *my_malloc(size_t size)
>
> p = calloc(1, sizeof(struct pool));
> if (!p) {
> - free(p);
> return NULL;
> }
Remove the curly braces from here since they are no longer needed.
>
> p->mem = calloc(1, size);
> if (!p->mem)
> + free(p);
> return NULL;
Add the curly braces here. They are required.
regards,
dan carpenter
--
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] xhci: Don't add NOP TRB for aligned bulk sg transfers
It helps if you Cc testers on your patches. Adding Jérôme, please test.
Sarah Sharp
On Thu, Jan 23, 2014 at 01:42:28PM +, David Laight wrote:
> Transfer requests for usb disks can contain a large number of 4k fragments.
> Assume that it doesn't matter if there are LINK TRB in fragment lists
> that are 1k aligned.
>
> This should stop errors when transfer requests for usb disks contain
> more fragments that will fit in a ring segment.
>
> Signed-off-by: David Laight
> ---
> Note that this patch just allows large numbers of aligned fragments for
> bulk scatter-gather transfers.
> It doesn't remove the need for the patch that changes the way the ownership
> of the the first TRB is set in order to avoid the controller seeing 'dangling'
> LINK TRBs.
> However it does reduce the likelyhood of dangling LINK TRBs, so might seem to
> make the other patch unnecessary.
>
> I've only compile tested it - but it should be fine.
>
> drivers/usb/host/xhci-ring.c | 49
> ++--
> 1 file changed, 38 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index a0b248c..7a4efd2 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -2932,7 +2932,8 @@ static void queue_trb(struct xhci_hcd *xhci, struct
> xhci_ring *ring,
> * FIXME allocate segments if the ring is full.
> */
> static int prepare_ring(struct xhci_hcd *xhci, struct xhci_ring *ep_ring,
> - u32 ep_state, unsigned int num_trbs, gfp_t mem_flags)
> + u32 ep_state, unsigned int num_trbs, gfp_t mem_flags,
> + bool aligned_xfer)
> {
> unsigned int num_trbs_needed;
>
> @@ -2980,6 +2981,9 @@ static int prepare_ring(struct xhci_hcd *xhci, struct
> xhci_ring *ep_ring,
>* Simplest solution is to fill the trb before the
>* LINK with nop commands.
>*/
> + if (aligned_xfer)
> + /* Caller says buffer is aligned */
> + break;
> if (num_trbs == 1 || num_trbs <= usable || usable == 0)
> break;
>
> @@ -3073,7 +3077,8 @@ static int prepare_transfer(struct xhci_hcd *xhci,
> unsigned int num_trbs,
> struct urb *urb,
> unsigned int td_index,
> - gfp_t mem_flags)
> + gfp_t mem_flags,
> + bool aligned_xfer)
> {
> int ret;
> struct urb_priv *urb_priv;
> @@ -3090,7 +3095,7 @@ static int prepare_transfer(struct xhci_hcd *xhci,
>
> ret = prepare_ring(xhci, ep_ring,
> le32_to_cpu(ep_ctx->ep_info) & EP_STATE_MASK,
> -num_trbs, mem_flags);
> +num_trbs, mem_flags, aligned_xfer);
> if (ret)
> return ret;
>
> @@ -3117,7 +3122,8 @@ static int prepare_transfer(struct xhci_hcd *xhci,
> return 0;
> }
>
> -static unsigned int count_sg_trbs_needed(struct xhci_hcd *xhci, struct urb
> *urb)
> +static unsigned int count_sg_trbs_needed(struct xhci_hcd *xhci, struct urb
> *urb,
> + bool *aligned_xfer)
> {
> int num_sgs, num_trbs, running_total, temp, i;
> struct scatterlist *sg;
> @@ -3125,11 +3131,30 @@ static unsigned int count_sg_trbs_needed(struct
> xhci_hcd *xhci, struct urb *urb)
> sg = NULL;
> num_sgs = urb->num_mapped_sgs;
> temp = urb->transfer_buffer_length;
> + *aligned_xfer = true;
>
> num_trbs = 0;
> for_each_sg(urb->sg, sg, num_sgs, i) {
> unsigned int len = sg_dma_len(sg);
>
> + /*
> + * The 1.0 spec says LINK TRB are only allowed at data offsets
> + * that are multiples of the transfer burst size.
> + * While the burst size might be 16k the effect is probably
> + * that the transfer is split. In particular a non-full-length
> + * data block might be sent - terminating a bulk transfer.
> + *
> + * prepare_ring() ensures that the data TRB don't cross a LINK,
> + * but for usb disks we see large numbers of fragments of 4k
> + * and that exceeds the size of each ring segment.
> + *
> + * If all the fragments start and end on 1k boundaries tell
> + * prepare_ring() not to worry about LINK TRB.
> + */
> + if ((sg_dma_address(sg) | sg_dma_len(sg)) & 1023)
> + /* Fragment not 1k aligned */
> + *aligned_xfer = false;
> +
> /* Scatter gather list entries may cross 64KB boundaries */
> running_total = TRB_MAX_BUFF_SIZE -
> (sg_dma_address(sg) & (TRB_MAX_BUFF_SIZE - 1));
> @@ -3284,6 +3309,7 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci,
> gfp_t mem_flags,
> b
Re: [PATCH 1/1] usbip/userspace/libsrc/names.c: memory leak
On Mon, Jan 27, 2014 at 02:50:04PM -0800, Greg KH wrote: > On Mon, Jan 27, 2014 at 11:29:48PM +0100, xypron.g...@gmx.de wrote: > > From: Heinrich Schuchardt > > > > p is freed if NULL. > > Not a real problem, right? > > > p is leaked if second calloc fails. > > You just created a new bug in your "fix" :( > > Please run your patches through scripts/checkpatch.pl, odds are, it > would have caught this error. > Checkpatch doesn't catch the problems here. I thought it would have caught the style issue but apparently it only looks for extra curly braces when you run it in --file mode. Fengguang would hopefully have caught the missing curly braces bug with Coccinelle. regards, dan carpenter -- 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] HID: usbhid: quirk for CY-TM75 75 inch Touch Overlay
There is timeout error during initialization:
kernel: [ 11.733104] hid-multitouch 0003:1870:0110.0001: usb_submit_urb(ctrl)
failed: -1
kernel: [ 11.734093] hid-multitouch 0003:1870:0110.0001: timeout initializing
reports
Adding quirk HID_QUIRK_NO_INIT_REPORTS can solve the problem.
Signed-off-by: Yufeng Shen
---
drivers/hid/hid-ids.h | 1 +
drivers/hid/usbhid/hid-quirks.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index f9304cb..ece2997 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -636,6 +636,7 @@
#define USB_VENDOR_ID_NEXIO0x1870
#define USB_DEVICE_ID_NEXIO_MULTITOUCH_420 0x010d
+#define USB_DEVICE_ID_NEXIO_MULTITOUCH_PTI0750 0x0110
#define USB_VENDOR_ID_NEXTWINDOW 0x1926
#define USB_DEVICE_ID_NEXTWINDOW_TOUCHSCREEN 0x0003
diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
index 0db9a67..be5270a 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -74,6 +74,7 @@ static const struct hid_blacklist {
{ USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28,
HID_QUIRK_NOGET },
{ USB_VENDOR_ID_MGE, USB_DEVICE_ID_MGE_UPS, HID_QUIRK_NOGET },
{ USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GX680R_LED_PANEL,
HID_QUIRK_NO_INIT_REPORTS },
+ { USB_VENDOR_ID_NEXIO, USB_DEVICE_ID_NEXIO_MULTITOUCH_PTI0750,
HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_NOVATEK, USB_DEVICE_ID_NOVATEK_MOUSE,
HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN,
HID_QUIRK_NO_INIT_REPORTS },
{ USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN1,
HID_QUIRK_NO_INIT_REPORTS },
--
1.8.5.3
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] xhci: Don't trace all short/incomplete receives
On Wed, Jan 22, 2014 at 09:46:58AM +, David Laight wrote:
> From: Sarah Sharp
> > On Tue, Jan 21, 2014 at 12:02:56PM +, David Laight wrote:
> > > Don't trace short receives if URB_SHORT_NOT_OK is set.
> > > Short receives are normal for USB ethernet devices.
> > >
> > > Don't trace unexpected incomplete receives if XHCI_TRUST_TX_LENGTH is set.
> > > Ratelimit the trace.
> >
> > Your patch does more than what you wrote here.
> >
> > > Signed-off-by: David Laight
> > > ---
> > > If these two traces ever happen, then they will happen for every receive
> > > packet when using USB ethernet.
> > > If you need to enable the xhci_warn or xhci_dgb traces you don't want to
> > > be spammed with trace (syslogd will soon fill the disk).
> > >
> > > These patches won't apply to 3.12 because the trace texts have changed,
> > > however 3.12 also needs a kernel recompile to enable the traces and
> > > anyone doing that can probably manage to patch them out.
> >
> > This is a cleanup, so it won't go into 3.12. Only bug fixes get
> > backported to the stable kernels. The messages are annoying, but they
> > don't trigger a bug. People can work around them by turning off
> > CONFIG_USB_DEBUG.
>
> With these traces you really can't run USB ethernet with USB_DEBUG enabled.
> 3.12 probably requires a recompilation to get USB_DEBUG enabled (and I hope
> none of the main distributions enable it by default), so it may not be
> that important.
>
> If 3.13 has the dynamic debug code in it then these traces really need
> removing.
Dynamic debug is off by default in 3.13, even with CONFIG_USB_DEBUG
turned on.
> > > Changes for v2:
> > > Fixed so that it applies to Linus's current tree.
> > >
> > > drivers/usb/host/xhci-ring.c | 38 ++
> > > 1 file changed, 18 insertions(+), 20 deletions(-)
> > >
> > > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> > > index a0b248c..0b3dd16 100644
> > > --- a/drivers/usb/host/xhci-ring.c
> > > +++ b/drivers/usb/host/xhci-ring.c
> > > @@ -2301,36 +2301,34 @@ static int process_bulk_intr_td(struct xhci_hcd
> > > *xhci, struct xhci_td *td,
> > > switch (trb_comp_code) {
> > > case COMP_SUCCESS:
> > > /* Double check that the HW transferred everything. */
> > > - if (event_trb != td->last_trb ||
> > > - EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)) != 0) {
> > > - xhci_warn(xhci, "WARN Successful completion "
> > > - "on short TX\n");
> > > - if (td->urb->transfer_flags & URB_SHORT_NOT_OK)
> > > - *status = -EREMOTEIO;
> > > - else
> > > - *status = 0;
> > > - if ((xhci->quirks & XHCI_TRUST_TX_LENGTH))
> > > - trb_comp_code = COMP_SHORT_TX;
> > > - } else {
> > > + if (event_trb == td->last_trb &&
> > > + EVENT_TRB_LEN(le32_to_cpu(event->transfer_len)) == 0) {
> > > *status = 0;
> > > + break;
> >
> > Your patch changes the behavior of the code here, for when the status
> > variable is set to either zero or -EREMOTEIO. The code is hard to
> > reason about, so this really needs to be a separate patch from the one
> > that removes the traces. Please send a patchset with two patches: one
> > to remove the trace, and another to clean up setting *status, in
> > whichever order makes the code clearest in the *status patch.
>
> I've not deliberately changed the behaviour.
> I just inverted the first test so that that the test for URB_SHORT_NOT_OK
> was only done once.
You don't seem to understand that a patch description is a proof of
correctness. I need to verify, from just looking at the patch, that it
does not change behavior. E.g. "*status is set to -EREMOTEIO by foo
function, so it's safe to remove the assignment here" If there's
information I can't see by simply looking at the patch chunks, please
explain that.
> > > }
> > > - break;
> > > + if (xhci->quirks & XHCI_TRUST_TX_LENGTH)
> > > + trb_comp_code = COMP_SHORT_TX;
> >
> > I don't think changing the trb_comp_code is a good idea. There's a lot
> > of code that relies on it later, and it would take me a bit to figure
> > out if changing it is safe.
>
> That code was always there (look at the patch fragment above).
> It is in the code path where an inbound transfer is shorter than the
> size of the bulk rx buffer but the completion code was COMP_SUCCESS not
> COMP_SHORT_TX - which means that the xhci controller gave the wrong code.
> Any later code will expect the 'short transfer' result.
Again, this explanation needs to be in the patch description.
> I actually think that it should be done unconditionally (ie even if the
> the XHCI_TRUST_TX_LENGTH quirk isn't set). ie the code should either
> trust the completion code (unless the quirk is set) or trea
Re: Many USB ethernet devices are broken over xhci
David, you are overwhelming my capacity to handle patches right now. I appreciate that you would like to clean up the driver, but the bug fixes you're sending me are getting lost in the noise of random cleanup patches. Can you please limit the xHCI patches you send to bug fixes for now? Also, can you resend all the patches you consider to be serious bug fixes in one single patchset? Thanks, Sarah Sharp On Mon, Jan 27, 2014 at 04:06:22PM +, David Laight wrote: > Many of the net/usb ethernet drivers (including common ones like > the smsc95xx) will fail to transmit all packet length properly > when connected to a USB3 port (ie using the xhci driver). > > The underlying problem is that they assume the host controller > will honour the URB_ZERO_PACKET flag - which usbnet sets because > they specify FLAG_SEND_ZLP. > > However no one has ever added support for URB_ZERO_PACKET to > the xhci driver - so packets that need padding (probably 512 > bytes after any header is added) will not be sent correctly > and may have very adverse effects on the usb target. > > The ax179_178a driver avoids this by not setting FLAG_SEND_ZLP, > modifying the packet header, and appending an extra zero byte. > (Which has been responsible for its own set of panics.) > > I don't think this can be fixed by just clearing (or ignoring) > FLAG_SEND_ZLP as the extra byte will also confuse things. > > It needs to be fixed in the xhci code. > > I wrote this patch a while ago - worked for me with the ax179_178a > driver. http://www.spinics.net/lists/linux-usb/msg97370.html > > The patch is a bit difficult to read, the v1 version contained a copy of > the new function. http://www.spinics.net/lists/linux-usb/msg97183.html > > I don't think anything significant has changed (in the main kernel > sources) since I wrote the patch. > > David > > > > -- 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/1] usbip/userspace/libsrc/names.c: memory leak
On Tue, Jan 28, 2014 at 02:02:12AM +0300, Dan Carpenter wrote: > On Mon, Jan 27, 2014 at 02:50:04PM -0800, Greg KH wrote: > > On Mon, Jan 27, 2014 at 11:29:48PM +0100, xypron.g...@gmx.de wrote: > > > From: Heinrich Schuchardt > > > > > > p is freed if NULL. > > > > Not a real problem, right? > > > > > p is leaked if second calloc fails. > > > > You just created a new bug in your "fix" :( > > > > Please run your patches through scripts/checkpatch.pl, odds are, it > > would have caught this error. > > > > Checkpatch doesn't catch the problems here. I thought it would have > caught the style issue but apparently it only looks for extra curly > braces when you run it in --file mode. Ah, that's good to know. > Fengguang would hopefully have caught the missing curly braces bug with > Coccinelle. Is Coccinelle run on the userspace .c code in the kernel? thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Many USB ethernet devices are broken over xhci
On Mon, Jan 27, 2014 at 04:06:22PM +, David Laight wrote: > Many of the net/usb ethernet drivers (including common ones like > the smsc95xx) will fail to transmit all packet length properly > when connected to a USB3 port (ie using the xhci driver). That's odd, as I've never had a problem with my USB 3.0 ethernet device (sorry, don't remember what driver it uses), and I stress it all the time. I've also run two different USB 2 ethernet devices on xhci with no issues at all, so perhaps the "many" statement might not be true? > The underlying problem is that they assume the host controller > will honour the URB_ZERO_PACKET flag - which usbnet sets because > they specify FLAG_SEND_ZLP. > > However no one has ever added support for URB_ZERO_PACKET to > the xhci driver - so packets that need padding (probably 512 > bytes after any header is added) will not be sent correctly > and may have very adverse effects on the usb target. Really? How has things been working so well up to now without this support? Doesn't the hardware automatically add this padding with no explicit need for the xhci driver to do something? > The ax179_178a driver avoids this by not setting FLAG_SEND_ZLP, > modifying the packet header, and appending an extra zero byte. > (Which has been responsible for its own set of panics.) > > I don't think this can be fixed by just clearing (or ignoring) > FLAG_SEND_ZLP as the extra byte will also confuse things. > > It needs to be fixed in the xhci code. > > I wrote this patch a while ago - worked for me with the ax179_178a > driver. http://www.spinics.net/lists/linux-usb/msg97370.html Doing multiple things in a patch is generally considered bad-form, and this is quite hard to follow. > The patch is a bit difficult to read, the v1 version contained a copy of > the new function. http://www.spinics.net/lists/linux-usb/msg97183.html That's not helpful to the people having to review the v2 patch :) > I don't think anything significant has changed (in the main kernel > sources) since I wrote the patch. Then resubmit it, don't post links to it on the web, there's nothing we can do with them, sorry. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] xhci: Limit TD fragments to 1.0 xHCI hosts only.
Walt reports that the PCI add-in card for his 0.96 ASMedia host dies in a particular machine. This symptom goes away when commit 35773dac5f862cb1c82ea151eba3e2f6de51ec3e "usb: xhci: Link TRB must not occur within a USB payload burst" is reverted. Only 1.0 xHCI hosts actually need TD fragments, so limit that patch to only work on 1.0 hosts. Leave the scatter-gather entry length limitation in tact, to ensure consistent buffer limitation lengths across host controller versions. Otherwise we'll have some very confused driver writers. Signed-off-by: Sarah Sharp Reported-by: walt Cc: sta...@vger.kernel.org # 3.12 Cc: David Laight --- Walt, will you please put your ASMedia host into the system it failed on, and test this patch on 3.13 (or the last vanilla kernel it failed on)? Thanks, Sarah Sharp drivers/usb/host/xhci-ring.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index a0b248c34526..bdda69fd1651 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2971,6 +2971,8 @@ static int prepare_ring(struct xhci_hcd *xhci, struct xhci_ring *ep_ring, TRBS_PER_SEGMENT - 1 - trb; u32 nop_cmd; + if (xhci->hci_version < 0x100) + break; /* * Section 4.11.7.1 TD Fragments states that a link * TRB must only occur at the boundary between -- 1.8.5.2 -- 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 00/10] xhci: re-work command queue management
Hi Keith, You've told me in the past that you've run into an issue where you can hang the xHCI driver when one of your TeleMetrum boards refuses to respond to a Set Address command. Can you test the following patchset, and see if it fixes your problem? I've applied the patchset against 3.13 here: https://git.kernel.org/cgit/linux/kernel/git/sarah/xhci.git/log/?h=for-usb-next-command-queue Thanks, Sarah Sharp On Mon, Jan 13, 2014 at 05:05:49PM +0200, Mathias Nyman wrote: > This is an attempt to re-work and solve the issues in xhci command > queue management that Sarah has descibed earlier: > > Right now, the command management in the xHCI driver is rather ad-hock. > Different parts of the driver all submit commands, including interrupt > handling routines, functions called from the USB core (with or without the > bus bandwidth mutex held). > Some times they need to wait for the command to complete, and sometimes > they just issue the command and don't care about the result of the command. > > The places that wait on a command all time the command for five seconds, > and then attempt to cancel the command. > Unfortunately, that means if several commands are issued at once, and one of > them times out, all the commands timeout, even though the host hasn't gotten > a chance to service them yet. > > This is apparent with some devices that take a long time to respond to the > Set Address command during device enumeration (when the device is plugged in). > If a driver for a different device attempts to change alternate interface > settings at the same time (causing a Configure Endpoint command to be issued), > both commands timeout. > > Instead of having each command timeout after five seconds, the driver should > wait indefinitely in an uninterruptible sleep on the command completion. > A global command queue manager should time whatever command is currently > running, and cancel that command after five seconds. > > If the commands were in a list, like TDs currently are, it may be easier to > keep > track of where the command ring dequeue pointer is, and avoid racing with > events. > We may need to have parts of the driver that issue commands without waiting on > them still put the commands in the command list. > > The Implementation: > --- > > First step is to create a list of the commands submitted to the command queue. > To accomplish this each command is required to be submitted with a properly > filled command structure containing completion, status variable and a pointer > to > the command TRB that will be used. > > The first 7 patches are all about creating these command structures and > submitting them when we queue commands. > The command structures are allocated on the fly, the commands that are > submitted > in interrupt context are allocated with GFP_ATOMIC. > > Next, the global command queue is introduced. Commands are added to the queue > when trb's are queued, and remove when the commad completes. > Also switch to use the status variable and completion in the command struct. > > A new timer handles command timeout, the timer is kicked every time when a > command finishes and there's a new command waiting in the queue, or when a new > command is submitted to an _empty_ command queue. > Timer is deleted when the the last command on the queue finishes (empty queue) > > The old cancel_cmd_list is removed. > When the timer expires we simply tag the current command as "ABORTED" and > start > the ring abortion process. Functions waiting for an aborted command to finish > are > called after the command abortion is completed. > > Mathias Nyman (10): > xhci: Use command structures when calling xhci_configure_endpoint > xhci: use a command structure internally in xhci_address_device() > xhci: use command structures for xhci_queue_slot_control() > xhci: use command structures for xhci_queue_stop_endpoint() > xhci: use command structure for xhci_queue_new_dequeue_state() > xhci: use command structures for xhci_queue_reset_ep() > xhci: Use command structured when queuing commands > xhci: Add a global command queue > xhci: Use completion and status in global command queue > xhci: rework command timeout and cancellation, > > drivers/usb/host/xhci-hub.c | 42 ++-- > drivers/usb/host/xhci-mem.c | 22 +- > drivers/usb/host/xhci-ring.c | 532 > ++- > drivers/usb/host/xhci.c | 264 +++-- > drivers/usb/host/xhci.h | 43 ++-- > 5 files changed, 373 insertions(+), 530 deletions(-) > > -- > 1.8.1.2 > -- 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/1] usbip/userspace/libsrc/names.c: memory leak
On Mon, Jan 27, 2014 at 03:27:36PM -0800, Greg KH wrote: > On Tue, Jan 28, 2014 at 02:02:12AM +0300, Dan Carpenter wrote: > > On Mon, Jan 27, 2014 at 02:50:04PM -0800, Greg KH wrote: > > > On Mon, Jan 27, 2014 at 11:29:48PM +0100, xypron.g...@gmx.de wrote: > > > > From: Heinrich Schuchardt > > > > > > > > p is freed if NULL. > > > > > > Not a real problem, right? > > > > > > > p is leaked if second calloc fails. > > > > > > You just created a new bug in your "fix" :( > > > > > > Please run your patches through scripts/checkpatch.pl, odds are, it > > > would have caught this error. > > > > > > > Checkpatch doesn't catch the problems here. I thought it would have > > caught the style issue but apparently it only looks for extra curly > > braces when you run it in --file mode. > > Ah, that's good to know. > > > Fengguang would hopefully have caught the missing curly braces bug with > > Coccinelle. > > Is Coccinelle run on the userspace .c code in the kernel? Hm... I'm not sure. Fengguang, do you know if we would have caught the missing curly braces in this patch? http://lkml.org/lkml/2014/1/27/460 regards, dan carpenter -- 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: RocketU 1144B 1144BM 2-ports detected instead of 4-ports
On 01/27/2014 02:52 PM, Greg KH wrote: On Mon, Jan 27, 2014 at 02:29:53PM -0800, nubjub wrote: The precise meaning of dead would be: Absolutely nothing is logged when I plug a device into one of the other two ports. But for instance I have a bus powered usb sound card that has an led indicating it's on, that led illuminates. I also have a usb dvd drive which has an led that illuminates if it negotiates usb2.0 speed; this led does NOT illuminate. I will build a 3.13 kernel tomorrow. Let me know if the following logs are what you're looking for on the new kernel: if I've missed anything or included any crap you don't want. Close, can you provide the full boot log after the pci device is created? Or was this it? I stopped copying when it appeared it had ceased to deal with that device. The following is done still on the 3.10.11, but I thought I'd try again right now. It shows 2 XHCI controllers in the system, perhaps one of them isn't working for the "failure"? There are four ASMedia chips on the card, each with a single port attached to it. I would expect it to find four of these controllers and 1 port detected for each one. As I said on one occasion recently it found ASMedia devices. Also, just the output of 'lspci' (no -v option), and 'lsusb' (again, no '-v') will be fine to start with. full lspci :00:00.0 RAM memory: NVIDIA Corporation MCP55 Memory Controller (rev a2) :00:01.0 ISA bridge: NVIDIA Corporation MCP55 LPC Bridge (rev a3) :00:01.1 SMBus: NVIDIA Corporation MCP55 SMBus (rev a3) :00:02.0 USB controller: NVIDIA Corporation MCP55 USB Controller (rev a1) :00:02.1 USB controller: NVIDIA Corporation MCP55 USB Controller (rev a2) :00:04.0 IDE interface: NVIDIA Corporation MCP55 IDE (rev a1) :00:05.0 IDE interface: NVIDIA Corporation MCP55 SATA Controller (rev a3) :00:05.1 IDE interface: NVIDIA Corporation MCP55 SATA Controller (rev a3) :00:05.2 IDE interface: NVIDIA Corporation MCP55 SATA Controller (rev a3) :00:06.0 PCI bridge: NVIDIA Corporation MCP55 PCI bridge (rev a2) :00:06.1 Audio device: NVIDIA Corporation MCP55 High Definition Audio (rev a2) :00:08.0 Bridge: NVIDIA Corporation MCP55 Ethernet (rev a3) :00:09.0 Bridge: NVIDIA Corporation MCP55 Ethernet (rev a3) :00:0a.0 PCI bridge: NVIDIA Corporation MCP55 PCI Express bridge (rev a3) :00:0d.0 PCI bridge: NVIDIA Corporation MCP55 PCI Express bridge (rev a3) :00:0f.0 PCI bridge: NVIDIA Corporation MCP55 PCI Express bridge (rev a3) :00:18.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Family 10h Processor HyperTransport Configuration :00:18.1 Host bridge: Advanced Micro Devices, Inc. [AMD] Family 10h Processor Address Map :00:18.2 Host bridge: Advanced Micro Devices, Inc. [AMD] Family 10h Processor DRAM Controller :00:18.3 Host bridge: Advanced Micro Devices, Inc. [AMD] Family 10h Processor Miscellaneous Control :00:18.4 Host bridge: Advanced Micro Devices, Inc. [AMD] Family 10h Processor Link Control :00:19.0 Host bridge: Advanced Micro Devices, Inc. [AMD] Family 10h Processor HyperTransport Configuration :00:19.1 Host bridge: Advanced Micro Devices, Inc. [AMD] Family 10h Processor Address Map :00:19.2 Host bridge: Advanced Micro Devices, Inc. [AMD] Family 10h Processor DRAM Controller :00:19.3 Host bridge: Advanced Micro Devices, Inc. [AMD] Family 10h Processor Miscellaneous Control :00:19.4 Host bridge: Advanced Micro Devices, Inc. [AMD] Family 10h Processor Link Control :01:04.0 Network controller: Ralink corp. RT2800 802.11n PCI :01:05.0 FireWire (IEEE 1394): Texas Instruments TSB43AB22A IEEE-1394a-2000 Controller (PHY/Link) [iOHCI-Lynx] :05:00.0 PCI bridge: PLX Technology, Inc. PEX 8609 8-lane, 8-Port PCI Express Gen 2 (5.0 GT/s) Switch with DMA (rev ba) :05:00.1 System peripheral: PLX Technology, Inc. PEX 8609 8-lane, 8-Port PCI Express Gen 2 (5.0 GT/s) Switch with DMA (rev ba) :06:01.0 PCI bridge: PLX Technology, Inc. PEX 8609 8-lane, 8-Port PCI Express Gen 2 (5.0 GT/s) Switch with DMA (rev ba) :06:05.0 PCI bridge: PLX Technology, Inc. PEX 8609 8-lane, 8-Port PCI Express Gen 2 (5.0 GT/s) Switch with DMA (rev ba) :06:07.0 PCI bridge: PLX Technology, Inc. PEX 8609 8-lane, 8-Port PCI Express Gen 2 (5.0 GT/s) Switch with DMA (rev ba) :06:09.0 PCI bridge: PLX Technology, Inc. PEX 8609 8-lane, 8-Port PCI Express Gen 2 (5.0 GT/s) Switch with DMA (rev ba) :08:00.0 USB controller: ASMedia Technology Inc. ASM1042 SuperSpeed USB Host Controller :0a:00.0 USB controller: ASMedia Technology Inc. ASM1042 SuperSpeed USB Host Controller :18:00.0 VGA compatible controller: NVIDIA Corporation G92 [GeForce 8800 GT] (rev a2) :2b:00.0 PCI bridge: NEC Corporation uPD720400 PCI Express - PCI/PCI-X Bridge (rev 06) :2b:00.1 PCI bridge: NEC Corporation uPD720400 PCI Express - PCI/PCI-X Bridge (rev 06) :2c:04.0 SCSI storage controller: LS
Re: [PATCH v2 1/1] USB: EHCI: wait more than 3ms until the device enters full-speed idle
On Mon, Jan 27, 2014 at 10:52:48AM -0500, Alan Stern wrote:
> On Mon, 27 Jan 2014, Peter Chen wrote:
>
> > If the high-speed device does not enter full-speed idle after
> > wakeup on disconnect logic has effected, there will be an
> > unexpected disconnect wakeup interrupt due to the bus is still SE0.
>
> I think you mean "If the high-speed device does not enter full-speed
> idle _before_ wakeup on disconnect logic has taken effect..."
Yes, thanks.
>
> It sounds like this is a bug in your EHCI hardware. The
> wake-on-disconnect logic should never take effect until after the port
> goes into full-speed idle.
Where EHCI spec said that? I can't find it at 2.3.9 and 4.3.
>
> > Signed-off-by: Peter Chen
> > ---
> >
> > Changes for v2:
> > Using usleep_range instead of mdelay
> >
> > drivers/usb/host/ehci-hub.c | 12
> > 1 files changed, 12 insertions(+), 0 deletions(-)
> >
> > diff --git a/drivers/usb/host/ehci-hub.c b/drivers/usb/host/ehci-hub.c
> > index 47b858f..976294c 100644
> > --- a/drivers/usb/host/ehci-hub.c
> > +++ b/drivers/usb/host/ehci-hub.c
> > @@ -301,6 +301,18 @@ static int ehci_bus_suspend (struct usb_hcd *hcd)
> >
> > if (t1 != t2) {
> > ehci_writel(ehci, t2, reg);
> > + if ((t2 & PORT_WKDISC_E)
> > + && (ehci_port_speed(ehci, t2) ==
> > + USB_PORT_STAT_HIGH_SPEED)) {
> > + /*
> > +* If the high-speed device has not switched
> > +* to full-speed idle before WKDISC_E has
> > +* effected, there will be a WKDISC event.
> > +*/
> > + spin_unlock_irq(&ehci->lock);
> > + usleep_range(3500, 4000);
> > + spin_lock_irq(&ehci->lock);
> > + }
>
> This doesn't look like it will do what you want. t2 already has the
> PORT_WKDISC_E bit set, so once the
>
> ehci_writel(ehci, t2, reg);
>
> has executed, it is already too late. Instead, you should write (t2 &
> ~PORT_WKDISC_E), then wait a few ms, and then write t2.
Yes, it is safe for writing suspendM first, wait 3-4ms, then write
PORT_WKDISC_E. The reason why my proposal change is ok is the wakeup logic
has still not taken effect before the PHY enters low power mode.
>
> Since this bug seems to affect only a few EHCI controllers, we should
> have a quirk flag for it. There's no need to make everybody wait 3.5-4
> ms just for the sake of a few pieces of buggy hardware.
>
OK, I will add the quirk if you can point me it is not a standard ehci
operation.
--
Best Regards,
Peter Chen
--
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] HID: usbhid: quirk for CY-TM75 75 inch Touch Overlay
Hi,
On Mon, Jan 27, 2014 at 6:02 PM, Yufeng Shen wrote:
> There is timeout error during initialization:
> kernel: [ 11.733104] hid-multitouch 0003:1870:0110.0001:
> usb_submit_urb(ctrl) failed: -1
> kernel: [ 11.734093] hid-multitouch 0003:1870:0110.0001: timeout
> initializing reports
>
> Adding quirk HID_QUIRK_NO_INIT_REPORTS can solve the problem.
Could you please test the quirk HID_QUIRK_NO_INIT_INPUT_REPORTS instead?
The patch is perfectly fine, but usually when it fails, only the input
reports are non-readable, whereas features should. This way, you will
still get the value of the features, which _may_ be sensible for the
maxcontact information.
BTW, HID_QUIRK_NO_INIT_INPUT_REPORTS has been introduced in v3.12 and
is the default for Win 8 certified devices.
Cheers,
Benjamin
>
> Signed-off-by: Yufeng Shen
> ---
> drivers/hid/hid-ids.h | 1 +
> drivers/hid/usbhid/hid-quirks.c | 1 +
> 2 files changed, 2 insertions(+)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index f9304cb..ece2997 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -636,6 +636,7 @@
>
> #define USB_VENDOR_ID_NEXIO0x1870
> #define USB_DEVICE_ID_NEXIO_MULTITOUCH_420 0x010d
> +#define USB_DEVICE_ID_NEXIO_MULTITOUCH_PTI0750 0x0110
>
> #define USB_VENDOR_ID_NEXTWINDOW 0x1926
> #define USB_DEVICE_ID_NEXTWINDOW_TOUCHSCREEN 0x0003
> diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
> index 0db9a67..be5270a 100644
> --- a/drivers/hid/usbhid/hid-quirks.c
> +++ b/drivers/hid/usbhid/hid-quirks.c
> @@ -74,6 +74,7 @@ static const struct hid_blacklist {
> { USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28,
> HID_QUIRK_NOGET },
> { USB_VENDOR_ID_MGE, USB_DEVICE_ID_MGE_UPS, HID_QUIRK_NOGET },
> { USB_VENDOR_ID_MSI, USB_DEVICE_ID_MSI_GX680R_LED_PANEL,
> HID_QUIRK_NO_INIT_REPORTS },
> + { USB_VENDOR_ID_NEXIO, USB_DEVICE_ID_NEXIO_MULTITOUCH_PTI0750,
> HID_QUIRK_NO_INIT_REPORTS },
> { USB_VENDOR_ID_NOVATEK, USB_DEVICE_ID_NOVATEK_MOUSE,
> HID_QUIRK_NO_INIT_REPORTS },
> { USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN,
> HID_QUIRK_NO_INIT_REPORTS },
> { USB_VENDOR_ID_PIXART, USB_DEVICE_ID_PIXART_OPTICAL_TOUCH_SCREEN1,
> HID_QUIRK_NO_INIT_REPORTS },
> --
> 1.8.5.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
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
