Re: [PATCH 0/4] Add Gadget Bus
On Mon, May 19, 2014 at 05:40:29PM +0200, Andrzej Pietrasiewicz wrote: > Hi Peter, > > W dniu 16.05.2014 11:00, Peter Chen pisze: > >Hi Felipe & Alan, > > > >To continue with topic discussed at > >http://www.spinics.net/lists/linux-usb/msg105279.html, > >I implement the gadget bus to bind udc to gadget driver follow > >most ideas from your two. > > > > While the idea is interesting I think some aspects of the implementation > must be thought over. > > With the series applied g_ether fails. The offending commit is: > > 4c3efd6b1e227831c919bc2059d0aa080692f257 is the first bad commit > commit 4c3efd6b1e227831c919bc2059d0aa080692f257 > Author: Peter Chen > Date: Fri May 16 17:00:20 2014 +0800 > > usb: gadget: core: add implementation of gadget bus > > What happens is this: > > $ modprobe g_ether > [ 315.671192] using random self ethernet address > [ 315.674393] using random host ethernet address > [ 315.682421] usb0: HOST MAC 82:2a:41:f3:c0:12 > [ 315.687382] usb0: MAC d2:64:6a:c1:bc:6f > [ 315.689788] using random self ethernet address > [ 315.701331] using random host ethernet address > [ 315.707344] g_ether gadget: Ethernet Gadget, version: Memorial Day 2008 > [ 315.716012] g_ether gadget: g_ether ready > [ 315.730432] s3c-hsotg s3c-hsotg: bound driver g_ether > [ 315.748347] Unable to handle kernel NULL pointer dereference at virtual > address > [ 315.755008] s3c-hsotg s3c-hsotg: GINTSTS_USBSusp > [ 315.770636] pgd = e750c000 > [ 315.771873] [] *pgd= > [ 315.777413] Internal error: Oops: 5 [#1] PREEMPT ARM > [ 315.781040] Modules linked in: usb_f_eem g_ether(+) usb_f_rndis u_ether > libcomposite > [ 315.788758] CPU: 0 PID: 2727 Comm: modprobe Not tainted 3.15.0-rc4+ #371 > [ 315.795426] task: e77b5680 ti: e748a000 task.ti: e748a000 > [ 315.800809] PC is at module_add_driver+0x48/0xd0 > [ 315.805394] LR is at sysfs_do_create_link_sd+0x78/0xc8 > [ 315.810505] pc : []lr : []psr: 8013 > [ 315.810505] sp : e748bd58 ip : e748bd20 fp : e748bd74 > [ 315.821941] r10: r9 : bf0264f4 r8 : bf003290 > [ 315.827140] r7 : r6 : c05cd6e0 r5 : bf006d78 r4 : bf024510 > [ 315.833640] r3 : bf024344 r2 : r1 : c04b732c r0 : 00d0 > [ 315.840141] Flags: Nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment > user > [ 315.847246] Control: 10c5387d Table: 5750c019 DAC: 0015 > [ 315.852964] Process modprobe (pid: 2727, stack limit = 0xe748a238) > [ 315.859117] Stack: (0xe748bd58 to 0xe748c000) > [ 315.863452] bd40: > bf024510 > [ 315.871601] bd60: e75d6780 c05cd6e0 e748bd9c e748bd78 c0297608 c02a6744 > bf024344 e748bd88 > [ 315.879746] bd80: bf024510 bf0015a8 bf001f04 bf003270 e748bdb4 e748bda0 > c0298b08 c0297508 > [ 315.887892] bda0: bf0244c8 bf0015a8 e748bdc4 e748bdb8 c02ca358 c0298a8c > e748bdec e748bdc8 > [ 315.896037] bdc0: bf0013a8 c02ca320 e748bf48 0001 bf024558 > e6a4bfc0 e748a000 > [ 315.904183] bde0: e748bdfc e748bdf0 bf026508 bf001308 e748be7c e748be00 > c000891c bf026500 > [ 315.912329] be00: e748be2c e748be10 c0058a04 c03b8ec0 c05b41bc > bf02454c > [ 315.920475] be20: e748be3c e748be30 c0056bd4 c00589b4 e748be64 e748be40 > c0047828 c0056bc8 > [ 315.928620] be40: e748be50 e748bf48 0001 bf024558 e748bf48 > 0001 bf024558 > [ 315.936766] be60: e6a4bfc0 0001 bf02454c c0077af8 e748bf3c e748be80 > c007a560 c0008854 > [ 315.944912] be80: bf024558 7fff c0077e94 e748bebc eaa34000 > 0001 bf024558 > [ 315.953057] bea0: e748bed4 bf02454c bf024594 e748a000 bf0246ac > e748bf04 > [ 315.961203] bec0: e748bfa4 e748bed0 c0013d28 c0092660 eaa5b000 > > [ 315.969348] bee0: > > [ 315.977494] bf00: 2013 00028318 > b6dc9000 b6f68114 > [ 315.985640] bf20: 0080 c000f948 e748a000 e748bfa4 e748bf40 > c007ae34 c0078da0 > [ 315.993786] bf40: c0108f00 eaa34000 00028318 eaa52228 eaa52071 > eaa5bc08 06f8 > [ 316.001931] bf60: 0ab8 002b 002c > 0015 > [ 316.010077] bf80: 000f b6fc0bf8 0004 b6fc0cf8 > e748bfa8 > [ 316.018224] bfa0: c000f6a0 c007ad8c b6fc0bf8 0004 b6dc9000 00028318 > b6f68114 b6dc9000 > [ 316.026369] bfc0: b6fc0bf8 0004 b6fc0cf8 0080 b6fbe1e8 00028318 > b6f68114 > [ 316.034515] bfe0: b6f72134 be955948 b6f5f190 b6ec1db0 6010 b6dc9000 > 00040040 > [ 316.042675] [] (module_add_driver) from [] > (bus_add_driver+0x10c/0x214) > [ 316.050985] [] (bus_add_driver) from [] > (driver_register+0x88/0x104) > [ 316.059049] [] (driver_register) from [] > (usb_gadget_probe_driver+0x44/0x54) > [ 316.067826] [] (usb_gadget_probe_driver) from [] > (usb_
Re: Longcheer SU9800 usb 3g modem 1c9e:9800
On Monday, May 19, 2014 02:19:52 PM you wrote:
> On Sat, 2014-05-17 at 05:08 +0700, Alif Mubarak Ahmad wrote:
> > This device vendor and product id is 1c9e:9800
> > It is working as serial interface with generic usbserial driver.
> > I thought it is more suitable to use usbserial option driver, which
> > has better capability distinguishing between modem serial interface
> > and micro sd storage interface.
>
> Hi,
>
> does this solve the problem?
>
> Regards
> Oliver
>
> From a086773ae5dd1c417264937d049b93f45bac3991 Mon Sep 17 00:00:00
2001
> From: Oliver Neukum
> Date: Mon, 19 May 2014 14:13:51 +0200
> Subject: [PATCH] option: add device ID
>
> Reported by Alif Mubarak Ahmad:
>
> This device vendor and product id is 1c9e:9800
> It is working as serial interface with generic usbserial driver.
> I thought it is more suitable to use usbserial option driver, which has
> better capability distinguishing between modem serial interface and micro
> sd storage interface.
>
> Signed-off-by: Oliver Neukum
> ---
> drivers/usb/serial/option.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
> index f213ee9..5853fdf 100644
> --- a/drivers/usb/serial/option.c
> +++ b/drivers/usb/serial/option.c
> @@ -351,6 +351,9 @@ static void option_instat_callback(struct urb *urb);
> /* Zoom */
> #define ZOOM_PRODUCT_45970x9607
>
> +/* SU9800 usb 3g modem*/
> +#define SU9800_PRODUCT 0x9800
> +
> /* Haier products */
> #define HAIER_VENDOR_ID 0x201e
> #define HAIER_PRODUCT_CE100 0x2009
> @@ -1575,6 +1578,7 @@ static const struct usb_device_id option_ids[] = {
> { USB_DEVICE(LONGCHEER_VENDOR_ID,
FOUR_G_SYSTEMS_PRODUCT_W14),
> .driver_info = (kernel_ulong_t)&four_g_w14_blacklist
> },
> + { USB_DEVICE_INTERFACE_CLASS(LONGCHEER_VENDOR_ID,
SU9800_PRODUCT, 0xff) },
> { USB_DEVICE(LONGCHEER_VENDOR_ID, ZOOM_PRODUCT_4597) },
> { USB_DEVICE(LONGCHEER_VENDOR_ID, IBALL_3_5G_CONNECT) },
> { USB_DEVICE(HAIER_VENDOR_ID, HAIER_PRODUCT_CE100) },
I've applied the patch, but it seems rejected. I am using kernel 3.10.36 for
use with OpenWrt.
Applying patch generic/821-fixed-su9800.patch
patching file drivers/usb/serial/option.c
Hunk #1 succeeded at 326 with fuzz 2 (offset -25 lines).
patch unexpectedly ends in middle of line
Hunk #2 FAILED at 1578.
1 out of 2 hunks FAILED -- rejects in file drivers/usb/serial/option.c
Patch generic/821-fixed-su9800.patch does not apply (enforce with -f)
And now I'm trying to patch it manually. Here is the diff file generated by
quilt.
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -326,6 +326,9 @@ static void option_instat_callback(struc
/* Zoom */
#define ZOOM_PRODUCT_4597 0x9607
+/* SU9800 usb 3g modem*/
+#define SU9800_PRODUCT 0x9800
+
/* Haier products */
#define HAIER_VENDOR_ID0x201e
#define HAIER_PRODUCT_CE1000x2009
@@ -1503,6 +1506,7 @@ static const struct usb_device_id option
{ USB_DEVICE(LONGCHEER_VENDOR_ID, FOUR_G_SYSTEMS_PRODUCT_W14),
.driver_info = (kernel_ulong_t)&four_g_w14_blacklist
},
+ { USB_DEVICE_INTERFACE_CLASS(LONGCHEER_VENDOR_ID,
SU9800_PRODUCT, 0xff)
{ USB_DEVICE(LONGCHEER_VENDOR_ID, ZOOM_PRODUCT_4597) },
{ USB_DEVICE(LONGCHEER_VENDOR_ID, IBALL_3_5G_CONNECT) },
{ USB_DEVICE(HAIER_VENDOR_ID, HAIER_PRODUCT_CE100) },
For your information, manually triggering echo 1c9e 9800 > /sys/bus/usb-
serial/drivers/option1/new_id works just fine.
--
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: msm: fix bug in probe()
On Mon, 2014-05-19 at 23:35 +0300, Dan Carpenter wrote:
> My previous patch introduced a bug which prevented this driver from
> loading. devm_ioremap_resource() has a call to
> devm_request_mem_region() which will fail because the address space is
> shared between this PHY driver and CI device controller driver.
>
> Fixes: 10f0577aa5cb ('usb: phy: msm: change devm_ioremap() to
> devm_ioremap_resource()')
> Reported-by:"Ivan T. Ivanov"
> Signed-off-by: Dan Carpenter
>
> diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
> index 4f88174..ced34f3 100644
> --- a/drivers/usb/phy/phy-msm-usb.c
> +++ b/drivers/usb/phy/phy-msm-usb.c
> @@ -1586,9 +1586,11 @@ static int msm_otg_probe(struct platform_device *pdev)
> np ? "alt_core" : "usb_hs_core_clk");
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> - motg->regs = devm_ioremap_resource(&pdev->dev, res);
> - if (IS_ERR(motg->regs))
> - return PTR_ERR(motg->regs);
> + if (!res)
> + return -EINVAL;
> + motg->regs = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> + if (!motg->regs)
> + return -ENOMEM;
Well, I don't think that your previous patch was merged anywhere.
Or I am missing something? So you have to just made v3 of your
original patch.
Regards,
Ivan
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] USB: musb: Add a work_struct to recover from babble errors
why not reuse the musb_irq_work?
On Wed, Apr 2, 2014 at 7:58 PM, Daniel Mack wrote:
> Handle BABBLE interrupt error conditions from a work struct handler.
> This indirection is necessary as we can't be certain that the phy
> functions don't sleep.
>
> Platform layer implementation may pass a babble error down to the core
> in order to handle it.
>
> Signed-off-by: Daniel Mack
> ---
> drivers/usb/musb/musb_core.c | 35 +++
> drivers/usb/musb/musb_core.h | 1 +
> 2 files changed, 36 insertions(+)
>
> diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
> index 0757690..61da471 100644
> --- a/drivers/usb/musb/musb_core.c
> +++ b/drivers/usb/musb/musb_core.c
> @@ -848,6 +848,10 @@ b_host:
> }
> }
>
> + /* handle babble condition */
> + if (int_usb & MUSB_INTR_BABBLE)
> + schedule_work(&musb->recover_work);
> +
> #if 0
> /* REVISIT ... this would be for multiplexing periodic endpoints, or
> * supporting transfer phasing to prevent exceeding ISO bandwidth
> @@ -1746,6 +1750,34 @@ static void musb_irq_work(struct work_struct *data)
> }
> }
>
> +/* Recover from babble interrupt conditions */
> +static void musb_recover_work(struct work_struct *data)
> +{
> + struct musb *musb = container_of(data, struct musb, recover_work);
> + int status;
> +
> + musb_platform_reset(musb);
> +
> + usb_phy_vbus_off(musb->xceiv);
> + udelay(100);
> +
> + usb_phy_vbus_on(musb->xceiv);
> + udelay(100);
> +
> + /*
> +* When a babble condition occurs, the musb controller removes the
> +* session bit and the endpoint config is lost.
> +*/
> + if (musb->dyn_fifo)
> + status = ep_config_from_table(musb);
> + else
> + status = ep_config_from_hw(musb);
> +
> + /* start the session again */
> + if (status == 0)
> + musb_start(musb);
> +}
> +
> /* --
> * Init support
> */
> @@ -1913,6 +1945,7 @@ musb_init_controller(struct device *dev, int nIrq, void
> __iomem *ctrl)
>
> /* Init IRQ workqueue before request_irq */
> INIT_WORK(&musb->irq_work, musb_irq_work);
> + INIT_WORK(&musb->recover_work, musb_recover_work);
> INIT_DELAYED_WORK(&musb->deassert_reset_work, musb_deassert_reset);
> INIT_DELAYED_WORK(&musb->finish_resume_work, musb_host_finish_resume);
>
> @@ -2008,6 +2041,7 @@ fail4:
>
> fail3:
> cancel_work_sync(&musb->irq_work);
> + cancel_work_sync(&musb->recover_work);
> cancel_delayed_work_sync(&musb->finish_resume_work);
> cancel_delayed_work_sync(&musb->deassert_reset_work);
> if (musb->dma_controller)
> @@ -2073,6 +2107,7 @@ static int musb_remove(struct platform_device *pdev)
> dma_controller_destroy(musb->dma_controller);
>
> cancel_work_sync(&musb->irq_work);
> + cancel_work_sync(&musb->recover_work);
> cancel_delayed_work_sync(&musb->finish_resume_work);
> cancel_delayed_work_sync(&musb->deassert_reset_work);
> musb_free(musb);
> diff --git a/drivers/usb/musb/musb_core.h b/drivers/usb/musb/musb_core.h
> index 5514e4c..47e8874 100644
> --- a/drivers/usb/musb/musb_core.h
> +++ b/drivers/usb/musb/musb_core.h
> @@ -297,6 +297,7 @@ struct musb {
>
> irqreturn_t (*isr)(int, void *);
> struct work_struct irq_work;
> + struct work_struct recover_work;
> struct delayed_work deassert_reset_work;
> struct delayed_work finish_resume_work;
> u16 hwvers;
> --
> 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
--
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: Longcheer SU9800 usb 3g modem 1c9e:9800
On Tue, 2014-05-20 at 14:02 +0700, Alif Mubarak Ahmad wrote: > On Monday, May 19, 2014 02:19:52 PM you wrote: > I've applied the patch, but it seems rejected. I am using kernel 3.10.36 for > use with OpenWrt. It was against git HEAD. Sorry. > For your information, manually triggering echo 1c9e 9800 > /sys/bus/usb- > serial/drivers/option1/new_id works just fine. Looks good, but I'd prefer a full test. Thanks Oliver -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-next v2 00/12] cdc_ncm: add buffer tuning and stats using ethtool
[adding Greg Suarez to the Cc list after noticing that he was missing
from this thread. Sorry Greg, that was not my intention. This
discussion is just as relevant to cdc_mbim as to cdc_ncm, defining a
new common userspace API for all the cdc_ncm based drivers]
Bjørn Mork writes:
> Lars Melin writes:
>
>> Your target audience is embedded systems with limited cpu power and
>> buffer memory, right?
>> If so, then you can't expect them to have ethtool included and their
>> developers are not likely to be happy over having to "waste" another
>> 100KB in order to tune a 20KB driver.
>> My vote goes for sysfs.
>
> That's of course a very good point.
>
> I will argue that you often need ethtool anyway for basic stuff like
> forcing duplex/speed, enabling debug messages, turning on/off pause
> frames etc. But I don't doubt you know what you are talking about here
> :-) So I guess many small embedded devices don't include an ethtool
> binary by default. I do wonder how much we should adapt to that though?
> I understand that you don't see it this way, but others might actually
> see it as an advantage if we're forcing ethtool onto these devices...
>
> Anyway, I'll start looking at an alternative sysfs implementation so we
> can discuss this in terms of actual code.
As this is just a very informal RFC, I'm just attaching the patch to
this mail. Please test and/or comment.
The patch works for me. It doesn't remove the ethtool coalescing
support, but I will do that in the final submission if we decide to go
for this sysfs interface instead.
Example output:
bjorn@nemi:~$ grep . /sys/class/net/wwan0/cdc_ncm/*
/sys/class/net/wwan0/cdc_ncm/rx_max:16384
/sys/class/net/wwan0/cdc_ncm/tx_max:4096
/sys/class/net/wwan0/cdc_ncm/tx_timer_usecs:400
I should probably also add a bit more sanity checking of the input for
the final submission. Maybe even make sure the buffers match the
alignment? Or should that be up to the user?
But we could add information about the device alignment requirements so
that the end user (or userspace application) can make informed decisions.
The whole NTB parameter struct is vital information which is currently only
available for userspace in a debug log message. Exporting it as a set
of sysfs read only attributes would be nice. Maybe? Or am I just
overdoing things now?
It's also tempting to add the 'timer restart count' to this API. I left
it out of the ethtool based version because I couldn't make it fit
anywhere. But it is part of the driver's frame aggregation algorithm.
Bjørn
>From 017c9fc8702132ed76b9c28a196203b1788ef4c8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B8rn=20Mork?=
Date: Mon, 19 May 2014 16:08:49 +0200
Subject: [RFC] net: cdc_ncm: add sysfs support for buffer tuning
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Adding a sysfs group to the network device, allowing tuning of
buffers and tx timer.
Signed-off-by: Bjørn Mork
---
drivers/net/usb/cdc_ncm.c | 143 --
1 file changed, 124 insertions(+), 19 deletions(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index ad2a386a6e92..30b943200ca3 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -191,11 +191,9 @@ static const struct ethtool_ops cdc_ncm_ethtool_ops = {
.set_coalesce = cdc_ncm_set_coalesce,
};
-/* handle rx_max and tx_max changes */
-static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx)
+static u32 cdc_ncm_check_rx_max(struct usbnet *dev, u32 new_rx)
{
struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
- u8 iface_no = ctx->control->cur_altsetting->desc.bInterfaceNumber;
u32 val, max, min;
/* clamp new_rx to sane values */
@@ -210,10 +208,125 @@ static void cdc_ncm_update_rxtx_max(struct usbnet *dev, u32 new_rx, u32 new_tx)
}
val = clamp_t(u32, new_rx, min, max);
- if (val != new_rx) {
- dev_dbg(&dev->intf->dev, "rx_max must be in the [%u, %u] range. Using %u\n",
- min, max, val);
- }
+ if (val != new_rx)
+ dev_dbg(&dev->intf->dev, "rx_max must be in the [%u, %u] range\n", min, max);
+
+ return val;
+}
+
+static u32 cdc_ncm_check_tx_max(struct usbnet *dev, u32 new_tx)
+{
+ struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0];
+ u32 val, max, min;
+
+ /* clamp new_tx to sane values */
+ min = ctx->max_datagram_size + ctx->max_ndp_size + sizeof(struct usb_cdc_ncm_nth16);
+ max = min_t(u32, CDC_NCM_NTB_MAX_SIZE_TX, le32_to_cpu(ctx->ncm_parm.dwNtbOutMaxSize));
+
+ /* some devices set dwNtbOutMaxSize too low for the above default */
+ min = min(min, max);
+
+ val = clamp_t(u32, new_tx, min, max);
+ if (val != new_tx)
+ dev_dbg(&dev->intf->dev, "tx_max must be in the [%u, %u] range\n", min, max);
+
+ return val;
+}
+
+/* sysfs attributes */
+static ssize_t cdc_ncm_show_rx_max(struct device *d, struct device_attribute *attr, char *buf)
+{
+ struct usbnet *dev = netdev_priv(to_net_dev(d));
+ s
Re: [PATCH] usb: ehci: Enable support for 64bit EHCI host controllers in arm64
On Monday 19 May 2014 16:42:18 Rob Herring wrote: > On Mon, May 19, 2014 at 10:56 AM, Catalin Marinas > wrote: > > On Mon, May 19, 2014 at 10:44:51AM +0100, Arnd Bergmann wrote: > > > On Monday 19 May 2014 10:03:40 Catalin Marinas wrote: > > > > On Mon, May 19, 2014 at 09:32:43AM +0100, Arnd Bergmann wrote: > > > We definitely have to fix this very quickly, before people start building > > > real arm64 systems and shipping them. > > > > > > We should not merge any hacks that attempt to work around the problem, > > > but try to come to a conclusion how to handle them properly. > > > My hope was that we could just always set the dma mask to whatever > > > the DT says it should be to keep the burden from device drivers, > > > unless they want to restrict it further (e.g. when the specific > > > peripheral hardware has a bug that prevents us from using high addresses, > > > even though the SoC in theory supports it everywhere). > > > > I agree. > > > > > Rob Herring argued that we should always mimic PCI and call > > dma_set_mask() > > > in drivers but default to a 32-bit mask otherwise, independent of whether > > > the hardware can do more or less than that, IIRC. > > > > Can we not default to something built up from dma-ranges? Or 32-bit if > > dma-ranges property is missing? > > > > My reasoning was the information may not come from DT. For AHCI, the 32 vs. > 64 bit capability is in a capability register and the DMA mask is set based > on that. This information only exists with-in the driver. > > I think some sort of capability merging is needed here to merge a bus mask > with a device mask. The current api doesn't seem to support this as > adjusting the set mask is expected to fail if the requested mask is not > supported. Here is how I see it working. Devices can have a default mask > (32-bit typically). The default is simply the common case. Really the mask > should only be set for DMA capable devices, but many drivers fail to set > the mask. Devices can then enlarge or shrink what they support based on > knowing the IP block features or getting capabilities from the IP block > such as the AHCI case. This mask then needs to be adjusted (only shrinking) > if the parent bus or platform has restrictions which is the part that > should come from dma-ranges. Where do you think the shrinking should be done then? I'd like to see that as part of the initial bus probe that Santosh just implemented to set the offset. I think it would be reasonable to apply whatever the bus can do there, but limit it to 32-bit. > Keep in mind that dma-ranges is defined as a bus property, not a property > of a device. I don't have any issue with allowing it in a device, but I > don't think it should be required and am not yet convinced it is needed. Right. Arnd -- 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: msm: fix bug in probe()
On Tue, May 20, 2014 at 10:19:12AM +0300, Ivan T. Ivanov wrote:
> On Mon, 2014-05-19 at 23:35 +0300, Dan Carpenter wrote:
> > My previous patch introduced a bug which prevented this driver from
> > loading. devm_ioremap_resource() has a call to
> > devm_request_mem_region() which will fail because the address space is
> > shared between this PHY driver and CI device controller driver.
> >
> > Fixes: 10f0577aa5cb ('usb: phy: msm: change devm_ioremap() to
> > devm_ioremap_resource()')
> > Reported-by:"Ivan T. Ivanov"
> > Signed-off-by: Dan Carpenter
> >
> > diff --git a/drivers/usb/phy/phy-msm-usb.c b/drivers/usb/phy/phy-msm-usb.c
> > index 4f88174..ced34f3 100644
> > --- a/drivers/usb/phy/phy-msm-usb.c
> > +++ b/drivers/usb/phy/phy-msm-usb.c
> > @@ -1586,9 +1586,11 @@ static int msm_otg_probe(struct platform_device
> > *pdev)
> > np ? "alt_core" : "usb_hs_core_clk");
> >
> > res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > - motg->regs = devm_ioremap_resource(&pdev->dev, res);
> > - if (IS_ERR(motg->regs))
> > - return PTR_ERR(motg->regs);
> > + if (!res)
> > + return -EINVAL;
> > + motg->regs = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> > + if (!motg->regs)
> > + return -ENOMEM;
>
> Well, I don't think that your previous patch was merged anywhere.
> Or I am missing something?
Yeah. It was merged.
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 3/5] usb: gadget: net2280: Pass checkpacth.pl test
From: Alan Stern ... > > -static struct usb_request * > > -net2280_alloc_request (struct usb_ep *_ep, gfp_t gfp_flags) > > +static struct usb_request *net2280_alloc_request(struct usb_ep *_ep, > > + gfp_t gfp_flags) > > What's with the extreme indentation on the continuation line? The > style used here is for continuation lines to be indented by two stops > relative to the first line. Also why move the line break? The Linux kernel has both styles, and the original one (with the function name starting in column 1) makes it much easier to search to the definition. 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 3/5] usb: gadget: net2280: Pass checkpacth.pl test
Not for grep... But if this is an issue I have no problem going back to the original. We can spend a whole year just talking about codestyle. On Tue, May 20, 2014 at 10:52 AM, David Laight wrote: > From: Alan Stern > ... >> > -static struct usb_request * >> > -net2280_alloc_request (struct usb_ep *_ep, gfp_t gfp_flags) >> > +static struct usb_request *net2280_alloc_request(struct usb_ep *_ep, >> > + gfp_t gfp_flags) >> >> What's with the extreme indentation on the continuation line? The >> style used here is for continuation lines to be indented by two stops >> relative to the first line. > > Also why move the line break? > The Linux kernel has both styles, and the original one (with the > function name starting in column 1) makes it much easier to search > to the definition. > > David > > > -- Ricardo Ribalda -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: About gadget configfs
> > On Tue, 6 May 2014, Peter Chen wrote: > > > 3. Besides, I have tried two configurations, and each configuration > > with one function, I find the host PC always set configuration 1 (The > first > > configuration), any ways to switch configuration at host side? > > libusb? > > You can switch configurations on a Linux host by writing to a sysfs > file: > > echo N >/sys/bus/usb/devices/.../bConfigurationValue > > where N is the bConfigurationValue of the config you want to use and > "..." is the pathname for the device. > Yes, it may work from host side, but it seems there are some problems at gadget side when switch configurations. root@freescale /sys/kernel/config/usb_gadget/g1$ ln -s functions/mass_storage.1/ configs/c.1/ root@freescale /sys/kernel/config/usb_gadget/g1$ mkdir functions/gser.1 root@freescale /sys/kernel/config/usb_gadget/g1$ ln -s functions/gser.1 configs/ c.2/ root@freescale /sys/kernel/config/usb_gadget/g1$ root@freescale /sys/kernel/config/usb_gadget/g1$ echo udc-0 > UDC sh: write error: No such device root@freescale /sys/kernel/config/usb_gadget/g1$ echo ci_hdrc.0 > UDC root@freescale /sys/kernel/config/usb_gadget/g1$ configfs-gadget gadget: high-speed config #1: c =>At host side run: echo 2 > /sys/bus/usb/devices/1-6/bConfigurationValue root@freescale /sys/kernel/config/usb_gadget/g1$ root@freescale /sys/kernel/config/usb_gadget/g1$ configfs-gadget gadget: high-speed config #2: c ci_hdrc ci_hdrc.0: enabling a non-empty endpoint! [ cut here ] WARNING: CPU: 0 PID: 764 at /home/b29397/work/projects/upstream/usb/usb/kernel/locking/lockdep.c:3159 __lock_acquire+0x68c/0x90c() DEBUG_LOCKS_WARN_ON(id >= MAX_LOCKDEP_KEYS) Modules linked in: usb_f_serial u_serial usb_f_mass_storage libcomposite ci_hdrc_imx usbmisc_imx ci_hdrc CPU: 0 PID: 764 Comm: file-storage Not tainted 3.15.0-rc3+ #222 Backtrace: [<80011d88>] (dump_backtrace) from [<80012128>] (show_stack+0x18/0x1c) r6:807f3b94 r5: r4: r3: [<80012110>] (show_stack) from [<80672900>] (dump_stack+0x80/0x9c) [<80672880>] (dump_stack) from [<80028b40>] (warn_slowpath_common+0x6c/0x8c) r5:0c57 r4:be64fd40 [<80028ad4>] (warn_slowpath_common) from [<80028c04>] (warn_slowpath_fmt+0x38/0x40) r8:0448 r7: r6:be713484 r5: r4: [<80028bd0>] (warn_slowpath_fmt) from [<8006698c>] (__lock_acquire+0x68c/0x90c) r3:807f4abc r2:807f115c [<80066300>] (__lock_acquire) from [<8006719c>] (lock_acquire+0x68/0x7c) r10: r9: r8:0002 r7:0080 r6:6093 r5:be64e000 r4: [<80067134>] (lock_acquire) from [<8067dd24>] (_raw_spin_lock+0x34/0x44) r7:be55f220 r6:be713474 r5:0001 r4:be713474 [<8067dcf0>] (_raw_spin_lock) from [<7f026cdc>] (bulk_out_complete+0x34/0x7c [usb_f_mass_storage]) r5: r4:be713400 [<7f026ca8>] (bulk_out_complete [usb_f_mass_storage]) from [<7f0020a0>] (ep_dequeue+0xcc/0xf8 [ci_hdrc]) r7:8013 r6:be76aeb4 r5:be76ae80 r4:bebc04e4 [<7f001fd4>] (ep_dequeue [ci_hdrc]) from [<7f028484>] (handle_exception+0x11c/0x3f0 [usb_f_mass_storage]) r7:be64e000 r6:6013 r5:be5a3400 r4:0002 [<7f028368>] (handle_exception [usb_f_mass_storage]) from [<7f02a9fc>] (fsg_main_thread+0x154/0x1f4 [usb_f_mass_storage]) r8:fff8 r7:fff7 r6:be5a3474 r5:be64e000 r4:be5a3400 [<7f02a8a8>] (fsg_main_thread [usb_f_mass_storage]) from [<80046c24>] (kthread+0xd0/0xec) r10: r8: r7:7f02a8a8 r6:be5a3400 r5:be70d940 r4: [<80046b54>] (kthread) from [<8000ea48>] (ret_from_fork+0x14/0x2c) r7: r6: r5:80046b54 r4:be70d940 ---[ end trace f883df719f2cfac6 ]--- b29397@nchen-desktop:~$ lsusb -d 15a2:0054 -v Bus 001 Device 006: ID 15a2:0054 Freescale Semiconductor, Inc. Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 0.00 bDeviceClass0 (Defined at Interface level) bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize064 idVendor 0x15a2 Freescale Semiconductor, Inc. idProduct 0x0054 bcdDevice3.15 iManufacturer 1 Freescale iProduct2 FSL i.mx6q sabreSD Board iSerial 3 123456ABCDEF bNumConfigurations 2 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 32 bNumInterfaces 1 bConfigurationValue 1 iConfiguration 0 bmAttributes 0x80 (Bus Powered) MaxPower6mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 2 bInterfaceClass 8 Mass Storage bInterfaceSubClass 6 SCSI bInterfaceProtocol 80 Bulk (Zip) iInterface 4 Mass Storage Endpoint Descriptor:
Re: [PATCH 2/3] usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers
Hi Geert-san,
(2014/05/19 20:58), Geert Uytterhoeven wrote:
> Hi Shimoda-san,
>
> On Mon, May 19, 2014 at 12:08 PM, Yoshihiro Shimoda
> wrote:
< snip >
>> +config USB_XHCI_RCAR
>> + tristate "xHCI support for Renesas R-Car SoCs"
>> + select USB_XHCI_PLATFORM
>> + depends on ARCH_SHMOBILE || COMPILE_TEST
>> + ---help---
>> + Say 'Y' to enable the support for the xHCI host controller
>> + found in Renesas R-Car ARM SoCs.
>
> Does R-Car Gen1 also have xHCI, and is it compatible?
> If not, you may want to call this driver USB_XHCI_RCAR2.
R-Car Gen1 doesn't have xHCI.
However, next generation of R-Car may have xHCI. (But, I don't know it is
compatible.)
If we call this driver "USB_XHCI_RCAR2", should we also change filename to
"xhci-rcar2.[ch]"?
< snip >
>> static int xhci_plat_start(struct usb_hcd *hcd)
>> {
>> + struct device_node *of_node = hcd->self.controller->of_node;
>> +
>> + if (of_device_is_compatible(of_node, "renesas,r8a7790-xhci") ||
>> + of_device_is_compatible(of_node, "renesas,r8a7790-xhci"))
>
> r8a7791, as Magnus already pointed out.
Yes, I will correct this.
>> + xhci_rcar_start(hcd);
>
> If CONFIG_USB_XHCI_RCAR is not defined, xhci_rcar_start() is a dummy
> function, but the of_device_is_compatible() checks will still be compiled in.
>
> Hence perhaps an #ifdef CONFIG_USB_XHCI_RCAR is warranted here,
> possibly combined with inclusion of a C-source file, like is done in
> drivers/usb/host/ohci-hcd.c? It's up to the USB maintainer to decide this,
> though.
This implementation is similar with the following patch. And the patch already
got
"Acked-by" from Mathias Nyman of USB XHCI DRIVER's maintainer.
http://marc.info/?l=linux-usb&m=140014933101775&w=2
< snip >
>> @@ -165,6 +172,15 @@ static int xhci_plat_probe(struct platform_device *pdev)
>> goto unmap_registers;
>> }
>>
>> + if (of_device_is_compatible(pdev->dev.of_node,
>> + "renesas,r8a7790-xhci") ||
>> + of_device_is_compatible(pdev->dev.of_node,
>> + "renesas,r8a7791-xhci")) {
>> + ret = xhci_rcar_init_quirk(pdev);
>
> Same here.
>
Same above.
< snip >
>> --- /dev/null
>> +++ b/drivers/usb/host/xhci-rcar.c
>
>> +/* USB3.0 Configuraion */
>
> Configuration
I ran the "aspell -c" command, and I found other 2 typos. ("Initilization" and
"Porariy")
So, I will correct these typos.
< snip >
>> + for (index = 0; index < fw->size; index += 4) {
>> + for (data = 0, j = 3; j >= 0; j--) {
>> + if ((j + index) >= fw->size)
>> + continue;
>> + data |= fw->data[index + j] << (8 * j);
>> + }
>
> This is your custom get_unaligned_le32(), to avoid reading beyond the end
> of the buffer if its size is not a multiple of 4 bytes?
Yes, I would like to avoid it.
> Is there some way to just use get_unaligned_le32()?
Yes, I will remove the custom get_unaligned_le32() and add the following code.
Do you think that this code is good?
int i;
u32 data;
u8 buf[4];
< snip >
for (i = 0; i < fw->size; i += 4) {
memset(buf, 0, sizeof(buf));
memcpy(buf, &fw->data[i], min(sizeof(buf), fw->size - i));
data = get_unaligned_le32(buf);
Best regards,
Yoshihiro Shimoda
> If you want to keep it, I would rewrite it as
>
> for (data = 0, j = 3; j >= 0; j--) {
> if ((j + index) < fw->size)
> data |= fw->data[index + j] << (8 * j);
> }
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like
> that.
> -- Linus Torval
>
--
Yoshihiro Shimoda
EC No.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers
Hi Magnus-san,
(2014/05/19 19:21), Magnus Damm wrote:
> Hi Shimoda-san,
>
> Thanks for your patches, I did however find one typo below:
>
> On Mon, May 19, 2014 at 7:08 PM, Yoshihiro Shimoda
> wrote:
< snip >
>> static int xhci_plat_start(struct usb_hcd *hcd)
>> {
>> + struct device_node *of_node = hcd->self.controller->of_node;
>> +
>> + if (of_device_is_compatible(of_node, "renesas,r8a7790-xhci") ||
>> + of_device_is_compatible(of_node, "renesas,r8a7790-xhci"))
>> + xhci_rcar_start(hcd);
>> +
>
> This is most likely a typo - I believe this is supposed to be r8a7790
> and r8a7791?
Thank you for the review! Yes, this is a typo... I will correct this.
Best regards,
Yoshihiro Shimoda
> Cheers,
>
> / magnus
>
--
Yoshihiro Shimoda
EC No.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers
Hello,
(2014/05/19 21:14), Sergei Shtylyov wrote:
> Hello.
>
> On 19-05-2014 14:08, Yoshihiro Shimoda wrote:
< snip >
>> static int xhci_plat_start(struct usb_hcd *hcd)
>> {
>> +struct device_node *of_node = hcd->self.controller->of_node;
>> +
>> +if (of_device_is_compatible(of_node, "renesas,r8a7790-xhci") ||
>> +of_device_is_compatible(of_node, "renesas,r8a7790-xhci"))
>
> Perhaps "renesas,r8a7791-xhci"?
Yes. I will correct this.
< snip >
>> +/* FW Download Control & Status */
>> +#define RCAR_USB3_DL_CTRL 0x250
>
> Already #define'd.
Thank you for the point! I will remove this.
>> +/* USB3.0 Configuraion */
>
> Configuration.
I will correct this.
>> +int xhci_rcar_start(struct usb_hcd *hcd)
>> +{
>> +if (hcd->regs != NULL) {
>> +u32 temp;
>
> Need empty line here... and should perhaps return error if hcd->regs NULL?
I will add empty line.
If this function returns error and xhci_plat_start() also returns error,
the xhci driver was not able to work.
So, I will change the prototype of this function to "void".
Best regards,
Yoshihiro Shimoda
>> +/* Interrupt Enable */
>> +temp = readl(hcd->regs + RCAR_USB3_INT_ENA);
>> +temp |= RCAR_USB3_INT_ENA_VAL;
>> +writel(temp, hcd->regs + RCAR_USB3_INT_ENA);
>> +/* LCLK Select */
>> +writel(RCAR_USB3_LCLK_ENA_VAL, hcd->regs + RCAR_USB3_LCLK);
>> +/* USB3.0 Configuration */
>> +writel(RCAR_USB3_CONF1_VAL, hcd->regs + RCAR_USB3_CONF1);
>> +writel(RCAR_USB3_CONF2_VAL, hcd->regs + RCAR_USB3_CONF2);
>> +writel(RCAR_USB3_CONF3_VAL, hcd->regs + RCAR_USB3_CONF3);
>> +/* USB3.0 Polariy */
>> +writel(RCAR_USB3_RX_POL_VAL, hcd->regs + RCAR_USB3_RX_POL);
>> +writel(RCAR_USB3_TX_POL_VAL, hcd->regs + RCAR_USB3_TX_POL);
>> +}
>> +
>> +return 0;
>> +}
> [...]
>
> WBR, Sergei
>
--
Yoshihiro Shimoda
EC No.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/10] xhci: 'noxhci_port_switch' kernel parameter
On 05/20/2014 04:01 AM, Greg KH wrote: > On Thu, May 08, 2014 at 07:25:55PM +0300, Mathias Nyman wrote: >> From: Dan Williams >> >> Add a command line switch for disabling ehci port switchover. Useful >> for working around / debugging xhci incompatibilities where ehci >> operation is available. >> >> Reference: http://marc.info/?l=linux-usb&m=138920063001509&w=2 >> >> Cc: Sarah Sharp >> Cc: Mathias Nyman >> Cc: Holger Hans Peter Freyther >> Suggested-by: Alan Stern >> Signed-off-by: Dan Williams >> Signed-off-by: Mathias Nyman >> --- >> Documentation/kernel-parameters.txt | 3 +++ >> drivers/usb/host/pci-quirks.c | 15 +-- >> 2 files changed, 16 insertions(+), 2 deletions(-) >> >> diff --git a/Documentation/kernel-parameters.txt >> b/Documentation/kernel-parameters.txt >> index 4384217..fc3403114 100644 >> --- a/Documentation/kernel-parameters.txt >> +++ b/Documentation/kernel-parameters.txt >> @@ -2251,6 +2251,9 @@ bytes respectively. Such letter suffixes can also be >> entirely omitted. >> >> nox2apic[X86-64,APIC] Do not enable x2APIC mode. >> >> +noxhci_port_switch >> +[USB] Use EHCI instead of XHCI where available >> + > > Ugh, I really don't like new command line options. > > Especially one that isn't very well documented. Why would someone want > to enable this? What problem is it solving? Can we detect this > automatically and do it for the user? Is this just for prototype > hardware that has not shipped? What hardware needs this? > > I need a whole lot more documentation at the very least before I will > apply this. > On Intel hardware with both ehci and xhci controllers we can select if a usb2 port is controlled by ehci or xhci. This capability can be checked from Intel xhci pci config space. Xhci driver checks this on boot and switches over the supported ports. This is a feature in Intel Panther point and later chipsets, in shipped hardware. Its working quite well in most cases, but sometimes vendors claim they support switchover, but then forget to connect some wires, and the usb2 port ends up dead after switching. A recently found case is the Sony VAIO T-series. (I'll send you a different patch for that shortly) http://marc.info/?l=linux-usb&m=139993106029340&w=2 This is the extreme case that the usb2 ports appears completely dead. Other reasons are that some devices might work better under ehci than xhci, and users want to enforce the ehci opton. For powermanagement developers it's nice to disable switchover as it turns out some hardware are quirky with port switchover and suspend/resume. (might need to turn port back to ehci before suspending). I don't think we can detect this automatically. Dan, can you add more documentation to this patch? -Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/10] xhci: 'noxhci_port_switch' kernel parameter
At Tue, 20 May 2014 12:47:36 +0300, Mathias Nyman wrote: > > On 05/20/2014 04:01 AM, Greg KH wrote: > > On Thu, May 08, 2014 at 07:25:55PM +0300, Mathias Nyman wrote: > >> From: Dan Williams > >> > >> Add a command line switch for disabling ehci port switchover. Useful > >> for working around / debugging xhci incompatibilities where ehci > >> operation is available. > >> > >> Reference: http://marc.info/?l=linux-usb&m=138920063001509&w=2 > >> > >> Cc: Sarah Sharp > >> Cc: Mathias Nyman > >> Cc: Holger Hans Peter Freyther > >> Suggested-by: Alan Stern > >> Signed-off-by: Dan Williams > >> Signed-off-by: Mathias Nyman > >> --- > >> Documentation/kernel-parameters.txt | 3 +++ > >> drivers/usb/host/pci-quirks.c | 15 +-- > >> 2 files changed, 16 insertions(+), 2 deletions(-) > >> > >> diff --git a/Documentation/kernel-parameters.txt > >> b/Documentation/kernel-parameters.txt > >> index 4384217..fc3403114 100644 > >> --- a/Documentation/kernel-parameters.txt > >> +++ b/Documentation/kernel-parameters.txt > >> @@ -2251,6 +2251,9 @@ bytes respectively. Such letter suffixes can also be > >> entirely omitted. > >> > >>nox2apic[X86-64,APIC] Do not enable x2APIC mode. > >> > >> + noxhci_port_switch > >> + [USB] Use EHCI instead of XHCI where available > >> + > > > > Ugh, I really don't like new command line options. > > > > Especially one that isn't very well documented. Why would someone want > > to enable this? What problem is it solving? Can we detect this > > automatically and do it for the user? Is this just for prototype > > hardware that has not shipped? What hardware needs this? > > > > I need a whole lot more documentation at the very least before I will > > apply this. > > > > On Intel hardware with both ehci and xhci controllers we can select if a usb2 > port > is controlled by ehci or xhci. This capability can be checked from Intel xhci > pci > config space. Xhci driver checks this on boot and switches over the supported > ports. > > This is a feature in Intel Panther point and later chipsets, in shipped > hardware. > Its working quite well in most cases, but sometimes vendors claim they support > switchover, but then forget to connect some wires, and the usb2 port ends up > dead > after switching. > > A recently found case is the Sony VAIO T-series. (I'll send you a different > patch > for that shortly) > http://marc.info/?l=linux-usb&m=139993106029340&w=2 > > This is the extreme case that the usb2 ports appears completely dead. > Other reasons are that some devices might work better under ehci than xhci, > and users want to enforce the ehci opton. For powermanagement developers it's > nice > to disable switchover as it turns out some hardware are quirky with port > switchover and suspend/resume. (might need to turn port back to ehci before > suspending). > > I don't think we can detect this automatically. > > Dan, can you add more documentation to this patch? While we're at it: can we implement a bitmask instead? We've seen lots of HP laptops having Webcams or BT devices that don't work XHCI but only with EHCI. For making them working properly, the specific xhci ports have to be disabled. But, we don't want to kill the whole XHCI at all. The single boolean option doesn't work for such a case. thanks, Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers
On Monday 19 May 2014 19:08:05 Yoshihiro Shimoda wrote:
>
> #include "xhci.h"
> #include "xhci-mvebu.h"
> +#include "xhci-rcar.h"
>
> static void xhci_plat_quirks(struct device *dev, struct xhci_hcd *xhci)
> {
> @@ -39,6 +40,12 @@ static int xhci_plat_setup(struct usb_hcd *hcd)
>
> static int xhci_plat_start(struct usb_hcd *hcd)
> {
> + struct device_node *of_node = hcd->self.controller->of_node;
> +
> + if (of_device_is_compatible(of_node, "renesas,r8a7790-xhci") ||
> + of_device_is_compatible(of_node, "renesas,r8a7790-xhci"))
> + xhci_rcar_start(hcd);
> +
> return xhci_run(hcd);
> }
>
> @@ -165,6 +172,15 @@ static int xhci_plat_probe(struct platform_device *pdev)
> goto unmap_registers;
> }
>
> + if (of_device_is_compatible(pdev->dev.of_node,
> + "renesas,r8a7790-xhci") ||
> + of_device_is_compatible(pdev->dev.of_node,
> + "renesas,r8a7791-xhci")) {
> + ret = xhci_rcar_init_quirk(pdev);
> + if (ret)
> + goto disable_clk;
> + }
> +
> ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
> if (ret)
> goto disable_clk;
> @@ -270,6 +286,8 @@ static const struct of_device_id usb_xhci_of_match[] = {
> { .compatible = "xhci-platform" },
> { .compatible = "marvell,armada-375-xhci"},
> { .compatible = "marvell,armada-380-xhci"},
> + { .compatible = "renesas,r8a7790-xhci"},
> + { .compatible = "renesas,r8a7791-xhci"},
> { },
> };
> MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
Like the drivers before, this is way more than a quirk, and deserves to
be its own driver. It would be better to have an abstract way to split
out soc specific xhci front-ends and export functions from the xhci-platform
code.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] usb: host: xhci-plat: add support for the R-Car H2 and M2 xHCI controllers
Hi Shimoda-san,
On Tue, May 20, 2014 at 11:35 AM, Yoshihiro Shimoda
wrote:
> (2014/05/19 20:58), Geert Uytterhoeven wrote:
>> On Mon, May 19, 2014 at 12:08 PM, Yoshihiro Shimoda
>> wrote:
> < snip >
>>> +config USB_XHCI_RCAR
>>> + tristate "xHCI support for Renesas R-Car SoCs"
>>> + select USB_XHCI_PLATFORM
>>> + depends on ARCH_SHMOBILE || COMPILE_TEST
>>> + ---help---
>>> + Say 'Y' to enable the support for the xHCI host controller
>>> + found in Renesas R-Car ARM SoCs.
>>
>> Does R-Car Gen1 also have xHCI, and is it compatible?
>> If not, you may want to call this driver USB_XHCI_RCAR2.
>
> R-Car Gen1 doesn't have xHCI.
> However, next generation of R-Car may have xHCI. (But, I don't know it is
> compatible.)
> If we call this driver "USB_XHCI_RCAR2", should we also change filename to
> "xhci-rcar2.[ch]"?
Iff you change the config symbol, please also change the filename.
But given the uncertainty about future version, you can leave it like it is.
>>> + xhci_rcar_start(hcd);
>>
>> If CONFIG_USB_XHCI_RCAR is not defined, xhci_rcar_start() is a dummy
>> function, but the of_device_is_compatible() checks will still be compiled in.
>>
>> Hence perhaps an #ifdef CONFIG_USB_XHCI_RCAR is warranted here,
>> possibly combined with inclusion of a C-source file, like is done in
>> drivers/usb/host/ohci-hcd.c? It's up to the USB maintainer to decide this,
>> though.
>
> This implementation is similar with the following patch. And the patch
> already got
> "Acked-by" from Mathias Nyman of USB XHCI DRIVER's maintainer.
>
> http://marc.info/?l=linux-usb&m=140014933101775&w=2
Fine. It can be fixed later by the maintainer, when the driver has gained too
many compatible checks ;-)
>>> + for (index = 0; index < fw->size; index += 4) {
>>> + for (data = 0, j = 3; j >= 0; j--) {
>>> + if ((j + index) >= fw->size)
>>> + continue;
>>> + data |= fw->data[index + j] << (8 * j);
>>> + }
>>
>> This is your custom get_unaligned_le32(), to avoid reading beyond the end
>> of the buffer if its size is not a multiple of 4 bytes?
>
> Yes, I would like to avoid it.
>
>> Is there some way to just use get_unaligned_le32()?
>
> Yes, I will remove the custom get_unaligned_le32() and add the following code.
> Do you think that this code is good?
>
> int i;
> u32 data;
> u8 buf[4];
> < snip >
> for (i = 0; i < fw->size; i += 4) {
> memset(buf, 0, sizeof(buf));
> memcpy(buf, &fw->data[i], min(sizeof(buf), fw->size - i));
> data = get_unaligned_le32(buf);
I'm sorry, but IMHO this looks worse.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] xhci: Switch only Intel Lynx Point-LP ports to EHCI on shutdown.
Patch "xhci: Switch Intel Lynx Point ports to EHCI on shutdown."
commit c09ec25d3684cad74d851c0f028a495999591279 is not fully correct
It switches both Lynx Point and Lynx Point-LP ports to EHCI on shutdown.
On some Lynx Point machines it causes spurious interrupt,
which wake the system: bugzilla.kernel.org/show_bug.cgi?id=76291
On Lynx Point-LP on the contrary switching ports to EHCI seems to be
necessary to fix these spurious interrupts.
Signed-off-by: Denis Turischev
Reported-by: Wulf Richartz
---
drivers/usb/host/xhci-pci.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 08a5f92..d25987e 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -134,7 +134,9 @@ static void xhci_pci_quirks(struct device *dev, struct
xhci_hcd *xhci)
*/
if (pdev->subsystem_vendor == PCI_VENDOR_ID_HP)
xhci->quirks |= XHCI_SPURIOUS_WAKEUP;
-
+ }
+ if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
+ pdev->device == PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI) {
xhci->quirks |= XHCI_SPURIOUS_REBOOT;
}
if (pdev->vendor == PCI_VENDOR_ID_ETRON &&
--
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] usb: ehci: Enable support for 64bit EHCI host controllers in arm64
On Mon, May 19, 2014 at 05:55:56PM +0100, Arnd Bergmann wrote: > On Monday 19 May 2014 16:56:08 Catalin Marinas wrote: > > On Mon, May 19, 2014 at 10:44:51AM +0100, Arnd Bergmann wrote: > > > On Monday 19 May 2014 10:03:40 Catalin Marinas wrote: > > > > On Mon, May 19, 2014 at 09:32:43AM +0100, Arnd Bergmann wrote: > > > > > The more important question is what happens to high buffers allocated > > > > > elsewhere > > > > > that get passed into dma_map_sg by a device driver. Depending on the > > > > > DT properties > > > > > of the device and its parents, this needs to do one of three things: > > > > > > > > > > a) translate the 64-bit virtual address into a 64-bit bus address > > > > > b) create an IOMMU entry for the 64-bit address and pass the 32-bit > > > > > IOMMU > > > > >address to the driver > > > > > c) use the swiotlb code to create a bounce buffer at a 32-bit DMA > > > > > address > > > > >and copy the data around > > > > > > > > > > It's definitely wrong to just hardcode a DMA mask in the driver > > > > > because that > > > > > code doesn't know which of the three cases is being used. Moreover, > > > > > you can't > > > > > do it using an #ifdef CONFIG_ARM64, because it's completely > > > > > independent of > > > > > the architecture, and we need to do the exact same logic on ARM32 and > > > > > any > > > > > other architecture. > > > > > > > > I agree. > > > > > > > > The problem we currently have is system topology description to pass the > > > > DMA mask and in a hierarchical way. I can see Santosh's patches > > > > introducing dma-ranges but the coherent dma mask still set as 32-bit. We > > > > can use the dma-ranges to infer a mask but that's only specific to the > > > > device and the driver doesn't know whether it goes through an iommu or > > > > not. > > > > > > We definitely have to fix this very quickly, before people start building > > > real arm64 systems and shipping them. > > > > > > We should not merge any hacks that attempt to work around the problem, > > > but try to come to a conclusion how to handle them properly. > > > My hope was that we could just always set the dma mask to whatever > > > the DT says it should be to keep the burden from device drivers, > > > unless they want to restrict it further (e.g. when the specific > > > peripheral hardware has a bug that prevents us from using high addresses, > > > even though the SoC in theory supports it everywhere). > > > > I agree. > > > > > Rob Herring argued that we should always mimic PCI and call dma_set_mask() > > > in drivers but default to a 32-bit mask otherwise, independent of whether > > > the hardware can do more or less than that, IIRC. > > > > Can we not default to something built up from dma-ranges? Or 32-bit if > > dma-ranges property is missing? > > We probably want to default to 32-bit for arm32 in the absence of dma-ranges. > For arm64, I'd prefer if we could always mandate dma-ranges to be present > for each bus, just like we mandate ranges to be present. > I hope it's not too late for that. > > dma_set_mask should definitely look at the dma-ranges properties, and the > helper that Santosh just introduced should give us all the information > we need. We just need to decide on the correct behavior. Last time I looked at Santosh's patches I thought the dma-ranges is per device rather than per bus. We could make it per bus only and let the device call dma_set_mask() explicitly if it wants to restrict it further. > > > While we currently don't have a set of swiotlb DMA ops on ARM32, we do > > > have it on ARM64, and I think we should be using them properly. It should > > > really not be hard to implement a proper dma_set_mask() function for > > > ARM64 that gets is able to set up the swiotlb based on the dma-ranges > > > properties and always returns success but leaves the mask unchanged. > > > > The swiotlb bounce buffer needs to be pre-allocated at boot, otherwise > > we don't have any guarantees. Since we can't honour random masks anyway, > > we stick to ZONE_DMA which is currently in the 4G limit. But the driver > > calls dma_set_mask() too late for any further swiotlb setup. > > > > With IOMMU we can be more flexible around dma_set_mask(), can be done at > > run-time. > > What we can do with swiotlb is to check if the mask is smaller than ZONE_DMA. > If it ever is, we have to fail dma_set_mask and hope the driver can fall > back to PIO mode or it will have to fail its probe() function. dma_set_(coherent_)mask check swiotlb_dma_supported() which returns false if io_tlb_end goes beyond the device mask. So we just need to ensure that io_tlb is allocated within ZONE_DMA. > For dma_set_coherent_mask(), we also have to fail any call that tries to > set a mask larger than what the device hardware can do. Unlike that, > dma_set_mask() can succeed with any mask, we just have to enable swiotlb > if the mask that the driver wants is larger than what the hardware can > do. Currently we c
Re: [PATCH] usb: ehci: Enable support for 64bit EHCI host controllers in arm64
On Tuesday 20 May 2014 12:02:46 Catalin Marinas wrote: > On Mon, May 19, 2014 at 05:55:56PM +0100, Arnd Bergmann wrote: > > On Monday 19 May 2014 16:56:08 Catalin Marinas wrote: > > > On Mon, May 19, 2014 at 10:44:51AM +0100, Arnd Bergmann wrote: > > > > On Monday 19 May 2014 10:03:40 Catalin Marinas wrote: > > We probably want to default to 32-bit for arm32 in the absence of > > dma-ranges. > > For arm64, I'd prefer if we could always mandate dma-ranges to be present > > for each bus, just like we mandate ranges to be present. > > I hope it's not too late for that. > > > > dma_set_mask should definitely look at the dma-ranges properties, and the > > helper that Santosh just introduced should give us all the information > > we need. We just need to decide on the correct behavior. > > Last time I looked at Santosh's patches I thought the dma-ranges is per > device rather than per bus. We could make it per bus only and let the > device call dma_set_mask() explicitly if it wants to restrict it > further. Can you check again? I've read the code again yesterday to check this, and I concluded it was correctly doing this per bus. > > > > While we currently don't have a set of swiotlb DMA ops on ARM32, we do > > > > have it on ARM64, and I think we should be using them properly. It > > > > should > > > > really not be hard to implement a proper dma_set_mask() function for > > > > ARM64 that gets is able to set up the swiotlb based on the dma-ranges > > > > properties and always returns success but leaves the mask unchanged. > > > > > > The swiotlb bounce buffer needs to be pre-allocated at boot, otherwise > > > we don't have any guarantees. Since we can't honour random masks anyway, > > > we stick to ZONE_DMA which is currently in the 4G limit. But the driver > > > calls dma_set_mask() too late for any further swiotlb setup. > > > > > > With IOMMU we can be more flexible around dma_set_mask(), can be done at > > > run-time. > > > > What we can do with swiotlb is to check if the mask is smaller than > > ZONE_DMA. > > If it ever is, we have to fail dma_set_mask and hope the driver can fall > > back to PIO mode or it will have to fail its probe() function. > > dma_set_(coherent_)mask check swiotlb_dma_supported() which returns > false if io_tlb_end goes beyond the device mask. So we just need to > ensure that io_tlb is allocated within ZONE_DMA. Makes sense for dma_set_mask. Why do you do the same thing for coherent_mask? Shouldn't that check against ZONE_DMA instead? > > For dma_set_coherent_mask(), we also have to fail any call that tries to > > set a mask larger than what the device hardware can do. Unlike that, > > dma_set_mask() can succeed with any mask, we just have to enable swiotlb > > if the mask that the driver wants is larger than what the hardware can > > do. > > Currently we can't satisfy any arbitrarily small dma mask even with > swiotlb since the bounce buffer is just guaranteed to be in ZONE_DMA. > Swiotlb allows for smaller masks but we need to reserve the io_tlb > buffer early during boot and at smaller addresses. For example, > swiotlb_alloc_coherent() first tries __get_free_pages(GFP_DMA) and if > the coherent_dma_mask isn't matched, it frees the pages and falls back > to the io_tlb buffer. However, I don't think it's worth going for masks > smaller than 32-bit on arm64. Is that safe for noncoherent systems? I'd expect the io_tlb buffer to be cached there, which means we can't use it for coherent allocations. > CMA is pretty similar to swiotlb with regards to pre-allocated buffers > for coherent dma. We currently don't limit it for arm64 but I think we > should just limit it to ZONE_DMA because we can't tell what masks the > devices need. We could parse the DT for dma-ranges but we can still have > explicit dma_set_coherent_mask() calls to make it smaller. > > Yet another issue is what we actually mean by ZONE_DMA. If we have > devices with different dma_pfn_offset (as per Santosh's patches), > ZONE_DMA would mean different things for them since phys_to_dma() may no > longer be the same for a single SoC. I never figured out how that works. Arnd -- 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: Longcheer SU9800 usb 3g modem 1c9e:9800
On Tuesday, May 20, 2014 09:35:40 AM Oliver Neukum wrote: > It was against git HEAD. Sorry. Nevermind > Looks good, but I'd prefer a full test. > > Thanks > Oliver I've compiled the firmware and tested it. It works perfectly. Here is some dmesg of OpenWrt boot. The 3g dongle is detected in 15 seconds. [ 15.54] usbcore: registered new interface driver option [ 15.55] usbserial: USB Serial support registered for GSM modem (1-port) [ 15.55] option 1-1:1.0: GSM modem (1-port) converter detected [ 15.57] usb 1-1: GSM modem (1-port) converter now attached to ttyUSB0 [ 15.57] option 1-1:1.1: GSM modem (1-port) converter detected [ 15.58] usb 1-1: GSM modem (1-port) converter now attached to ttyUSB1 [ 15.59] option 1-1:1.2: GSM modem (1-port) converter detected [ 15.60] usb 1-1: GSM modem (1-port) converter now attached to ttyUSB2 and on 21 seconds ... [ 21.38] option1 ttyUSB0: GSM modem (1-port) converter now disconnected from ttyUSB0 And here is kernel usb debug (/sys/kernel/debug/usb/devices) T: Bus=01 Lev=00 Prnt=00 Port=00 Cnt=00 Dev#= 1 Spd=480 MxCh= 1 B: Alloc= 1/800 us ( 0%), #Int= 1, #Iso= 0 D: Ver= 2.00 Cls=09(hub ) Sub=00 Prot=01 MxPS=64 #Cfgs= 1 P: Vendor=1d6b ProdID=0002 Rev= 3.10 S: Manufacturer=Linux 3.10.36 ehci_hcd S: Product=EHCI Host Controller S: SerialNumber=ehci-platform C:* #Ifs= 1 Cfg#= 1 Atr=e0 MxPwr= 0mA I:* If#= 0 Alt= 0 #EPs= 1 Cls=09(hub ) Sub=00 Prot=00 Driver=hub E: Ad=81(I) Atr=03(Int.) MxPS= 4 Ivl=256ms T: Bus=01 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#= 2 Spd=480 MxCh= 0 D: Ver= 2.00 Cls=00(>ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1 P: Vendor=1c9e ProdID=9800 Rev= 0.00 S: Manufacturer=USB Modem S: Product=USB Modem S: SerialNumber=1234567890ABCDEF C:* #Ifs= 4 Cfg#= 1 Atr=e0 MxPwr=500mA I:* If#= 0 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=(none) E: Ad=81(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms E: Ad=01(O) Atr=02(Bulk) MxPS= 512 Ivl=4ms I:* If#= 1 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=option E: Ad=82(I) Atr=03(Int.) MxPS= 64 Ivl=2ms E: Ad=83(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms E: Ad=02(O) Atr=02(Bulk) MxPS= 512 Ivl=4ms I:* If#= 2 Alt= 0 #EPs= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option E: Ad=84(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms E: Ad=03(O) Atr=02(Bulk) MxPS= 512 Ivl=4ms I:* If#= 3 Alt= 0 #EPs= 2 Cls=08(stor.) Sub=06 Prot=50 Driver=(none) E: Ad=04(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms E: Ad=85(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms It seems the option driver is actually working with this device. It's able to distinguish between modem serial interface and storage interface. FYI, no storage driver is compiled due to limited size of flash memory - only 4MB of flash. Finally, here is ip address obtained via ppp, with the modem device on ttyUSB1 root@OpenWrt:/# ip addr ls 3g-wan 9: 3g-wan: mtu 1500 qdisc fq_codel state UNKNOWN group default qlen 3 link/ppp inet 10.215.213.41 peer 10.64.64.64/32 scope global 3g-wan valid_lft forever preferred_lft forever Looks good, isn't? Thanks. Alive4Ever -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: usb5303: add support for reference clock specified in device tree
USB3503 chip supports 8 values of reference clock. The value is
specified by REF_SEL[1:0] pins and INT_N line. This patch add support
for getting 'refclk' clock, enabling it and setting INT_N line according
to the value of the gathered clock. If no clock has been specified,
driver defaults to the old behaviour (assuming that clock has been
specified by REF_SEL pins from primary reference clock frequencies
table).
Signed-off-by: Marek Szyprowski
---
drivers/usb/misc/usb3503.c | 28 ++--
1 file changed, 26 insertions(+), 2 deletions(-)
Hello,
This extension to USB3503 driver is needed to add support for OdroidU3
board, which uses 24MHz reference clock, sourced directly from CLKOUT
line from Exynos4412 SoC.
Best regards
Marek Szyprowski
Samsung R&D Institute Poland
diff --git a/drivers/usb/misc/usb3503.c b/drivers/usb/misc/usb3503.c
index a31641e18d19..52cb7549b775 100644
--- a/drivers/usb/misc/usb3503.c
+++ b/drivers/usb/misc/usb3503.c
@@ -18,6 +18,7 @@
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
+#include
#include
#include
#include
@@ -57,10 +58,12 @@ struct usb3503 {
enum usb3503_mode mode;
struct regmap *regmap;
struct device *dev;
+ struct clk *clk;
u8 port_off_mask;
int gpio_intn;
int gpio_reset;
int gpio_connect;
+ boolsecondary_ref_clk;
};
static int usb3503_reset(struct usb3503 *hub, int state)
@@ -186,6 +189,25 @@ static int usb3503_probe(struct usb3503 *hub)
} else if (np) {
hub->port_off_mask = 0;
+ hub->clk = devm_clk_get(dev, "refclk");
+ if (!IS_ERR(hub->clk)) {
+ unsigned long rate;
+
+ clk_prepare_enable(hub->clk);
+ rate = clk_get_rate(hub->clk);
+
+ if (rate == 3840 || rate == 2600 ||
+ rate == 1920 || rate == 1200)
+ hub->secondary_ref_clk = 0;
+ else if (rate == 2400 || rate == 2700 ||
+ rate == 2500 || rate == 5000)
+ hub->secondary_ref_clk = 1;
+ else
+ dev_err(dev,
+ "unsupported reference clock rate
(%d)\n",
+ rate);
+ }
+
property = of_get_property(np, "disabled-ports", &len);
if (property && (len / sizeof(u32)) > 0) {
int i;
@@ -213,8 +235,10 @@ static int usb3503_probe(struct usb3503 *hub)
dev_err(dev, "Ports disabled with no control interface\n");
if (gpio_is_valid(hub->gpio_intn)) {
- err = devm_gpio_request_one(dev, hub->gpio_intn,
- GPIOF_OUT_INIT_HIGH, "usb3503 intn");
+ int val = hub->secondary_ref_clk ? GPIOF_OUT_INIT_LOW :
+ GPIOF_OUT_INIT_HIGH;
+ err = devm_gpio_request_one(dev, hub->gpio_intn, val,
+ "usb3503 intn");
if (err) {
dev_err(dev,
"unable to request GPIO %d as connect pin
(%d)\n",
--
1.9.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] usb: usb5303: add support for reference clock specified in device tree
On Tue, May 20, 2014 at 01:53:42PM +0200, Marek Szyprowski wrote:
> + hub->clk = devm_clk_get(dev, "refclk");
> + if (!IS_ERR(hub->clk)) {
This won't handle deferred probe - the driver should error out if it
gets -EPROBE_DEFER since it means the clock exists and might be provided
later on.
> + unsigned long rate;
> +
> + clk_prepare_enable(hub->clk);
> + rate = clk_get_rate(hub->clk);
No error checking here.
> +
> + if (rate == 3840 || rate == 2600 ||
> + rate == 1920 || rate == 1200)
> + hub->secondary_ref_clk = 0;
> + else if (rate == 2400 || rate == 2700 ||
> + rate == 2500 || rate == 5000)
> + hub->secondary_ref_clk = 1;
> + else
> + dev_err(dev,
> + "unsupported reference clock rate
> (%d)\n",
> + rate);
This looks like a switch statement. Should the driver not try to set
the clock to a supported rate if it's not already at one rather than
error out - it seems like a more constructive thing to do?
signature.asc
Description: Digital signature
[PATCH] drivers: phy: exynos4x12-phy: fix HSIC1 power on/off sequence
>From experiments with real hardware, it seems that URSTCON bits for
HSIC0 and HSIC1 seems to be swapped, so updated macro definitions for
them. HSIC1 also requires enabling power to device phy to get properly
enabled (similar hack is already implemented for HSIC0).
Signed-off-by: Marek Szyprowski
---
Hello,
This fix for Exynos4x12 USB2 PHY driver is needed to add support for
OdroidU3 board, which uses both HSIC0 and HSIC1 ports. Without it only
LAN chip, which is connected to HSIC0 works correctly.
Best regards
Marek Szyprowski
Samsung R&D Institute Poland
---
drivers/phy/phy-exynos4x12-usb2.c | 48 +++
drivers/phy/phy-samsung-usb2.h| 2 +-
2 files changed, 35 insertions(+), 15 deletions(-)
diff --git a/drivers/phy/phy-exynos4x12-usb2.c
b/drivers/phy/phy-exynos4x12-usb2.c
index d92a7cc5698a..431568b5cc61 100644
--- a/drivers/phy/phy-exynos4x12-usb2.c
+++ b/drivers/phy/phy-exynos4x12-usb2.c
@@ -87,8 +87,16 @@
#define EXYNOS_4x12_URSTCON_OTG_PHYLINKBIT(2)
#define EXYNOS_4x12_URSTCON_HOST_PHY BIT(3)
#define EXYNOS_4x12_URSTCON_PHY1 BIT(4)
-#define EXYNOS_4x12_URSTCON_HSIC0 BIT(5)
-#define EXYNOS_4x12_URSTCON_HSIC1 BIT(6)
+/*
+ * According to Exynos 4x12 reference manual the values for
+ * EXYNOS_4x12_URSTCON_HSIC are:
+ * URSTCON_HSIC0 = BIT(5)
+ * URSTCON_HSIC1 = BIT(6)
+ * but from experiments with real hardware the above 2 bitfields
+ * seems to be swapped, so define them to match the actual
+ * hardware */
+#define EXYNOS_4x12_URSTCON_HSIC1 BIT(5)
+#define EXYNOS_4x12_URSTCON_HSIC0 BIT(6)
#define EXYNOS_4x12_URSTCON_HOST_LINK_ALL BIT(7)
#define EXYNOS_4x12_URSTCON_HOST_LINK_P0 BIT(8)
#define EXYNOS_4x12_URSTCON_HOST_LINK_P1 BIT(9)
@@ -216,14 +224,15 @@ static void exynos4x12_phy_pwr(struct
samsung_usb2_phy_instance *inst, bool on)
break;
case EXYNOS4x12_HSIC0:
phypwr =EXYNOS_4x12_UPHYPWR_HSIC0;
- rstbits = EXYNOS_4x12_URSTCON_HSIC1 |
+ rstbits = EXYNOS_4x12_URSTCON_HSIC0 |
EXYNOS_4x12_URSTCON_HOST_LINK_P0 |
EXYNOS_4x12_URSTCON_HOST_PHY;
break;
case EXYNOS4x12_HSIC1:
phypwr =EXYNOS_4x12_UPHYPWR_HSIC1;
rstbits = EXYNOS_4x12_URSTCON_HSIC1 |
- EXYNOS_4x12_URSTCON_HOST_LINK_P1;
+ EXYNOS_4x12_URSTCON_HOST_LINK_P1 |
+ EXYNOS_4x12_URSTCON_HOST_PHY;
break;
};
@@ -256,18 +265,22 @@ static void exynos4x12_phy_pwr(struct
samsung_usb2_phy_instance *inst, bool on)
static int exynos4x12_power_on(struct samsung_usb2_phy_instance *inst)
{
struct samsung_usb2_phy_driver *drv = inst->drv;
+ struct samsung_usb2_phy_instance *device =
+ &drv->instances[EXYNOS4x12_DEVICE];
- inst->enabled = 1;
+ inst->enabled++;
exynos4x12_setup_clk(inst);
exynos4x12_phy_pwr(inst, 1);
exynos4x12_isol(inst, 0);
/* Power on the device, as it is necessary for HSIC to work */
- if (inst->cfg->id == EXYNOS4x12_HSIC0) {
- struct samsung_usb2_phy_instance *device =
- &drv->instances[EXYNOS4x12_DEVICE];
- exynos4x12_phy_pwr(device, 1);
- exynos4x12_isol(device, 0);
+ if (inst->cfg->id == EXYNOS4x12_HSIC0 ||
+ inst->cfg->id == EXYNOS4x12_HSIC1) {
+ if (!device->enabled) {
+ exynos4x12_phy_pwr(device, 1);
+ exynos4x12_isol(device, 0);
+ }
+ device->enabled++;
}
return 0;
@@ -279,13 +292,20 @@ static int exynos4x12_power_off(struct
samsung_usb2_phy_instance *inst)
struct samsung_usb2_phy_instance *device =
&drv->instances[EXYNOS4x12_DEVICE];
- inst->enabled = 0;
+ inst->enabled--;
+ if (inst->enabled)
+ return 0;
+
exynos4x12_isol(inst, 1);
exynos4x12_phy_pwr(inst, 0);
- if (inst->cfg->id == EXYNOS4x12_HSIC0 && !device->enabled) {
- exynos4x12_isol(device, 1);
- exynos4x12_phy_pwr(device, 0);
+ if (inst->cfg->id == EXYNOS4x12_HSIC0 ||
+ inst->cfg->id == EXYNOS4x12_HSIC1) {
+ device->enabled--;
+ if (!device->enabled) {
+ exynos4x12_isol(device, 1);
+ exynos4x12_phy_pwr(device, 0);
+ }
}
return 0;
diff --git a/drivers/phy/phy-samsung-usb2.h b/drivers/phy/phy-samsung-usb2.h
index 51a16016a214..848744ab8f38 100644
--- a/drivers/phy/phy-samsung-usb2.h
+++ b/drivers/phy/phy-samsung-usb2.h
@@ -29,7 +29,7 @@
Re: [PATCH] usb: usb5303: add support for reference clock specified in device tree
Hello,
On 2014-05-20 13:57, Mark Brown wrote:
On Tue, May 20, 2014 at 01:53:42PM +0200, Marek Szyprowski wrote:
> + hub->clk = devm_clk_get(dev, "refclk");
> + if (!IS_ERR(hub->clk)) {
This won't handle deferred probe - the driver should error out if it
gets -EPROBE_DEFER since it means the clock exists and might be provided
later on.
Ok, I will add such case here.
> + unsigned long rate;
> +
> + clk_prepare_enable(hub->clk);
> + rate = clk_get_rate(hub->clk);
No error checking here.
> +
> + if (rate == 3840 || rate == 2600 ||
> + rate == 1920 || rate == 1200)
> + hub->secondary_ref_clk = 0;
> + else if (rate == 2400 || rate == 2700 ||
> + rate == 2500 || rate == 5000)
> + hub->secondary_ref_clk = 1;
> + else
> + dev_err(dev,
> + "unsupported reference clock rate (%d)\n",
> + rate);
This looks like a switch statement. Should the driver not try to set
the clock to a supported rate if it's not already at one rather than
error out - it seems like a more constructive thing to do?
Hmm, the problem here is that you cannot determine the right rate. The
rate is
encoded on REF_SEL[1:0] pins (usually soldered to some resistors) and
cannot be
read via i2c commands. To set add support for rate setting, I would need
to add
one more property with correct ref clock rate. Is this okay?
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xhci: Switch only Intel Lynx Point-LP ports to EHCI on shutdown.
Hi Denis,
thank you very much, works fine !
But my diff to the original xhcp-pci.c of Kernel 3.14.4 is:
$ diff xhci-pci.c-original xhci-pci.c-changed
137,138d136
<
< xhci->quirks |= XHCI_SPURIOUS_REBOOT;
139a138,141
> if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
> pdev->device == PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI) {
> xhci->quirks |= XHCI_SPURIOUS_REBOOT;
> }
just fyi
regards Wulf
Am 20.05.2014 13:00, schrieb Denis Turischev:
> Patch "xhci: Switch Intel Lynx Point ports to EHCI on shutdown."
> commit c09ec25d3684cad74d851c0f028a495999591279 is not fully correct
>
> It switches both Lynx Point and Lynx Point-LP ports to EHCI on shutdown.
> On some Lynx Point machines it causes spurious interrupt,
> which wake the system: bugzilla.kernel.org/show_bug.cgi?id=76291
>
> On Lynx Point-LP on the contrary switching ports to EHCI seems to be
> necessary to fix these spurious interrupts.
>
> Signed-off-by: Denis Turischev
> Reported-by: Wulf Richartz
> ---
> drivers/usb/host/xhci-pci.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 08a5f92..d25987e 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -134,7 +134,9 @@ static void xhci_pci_quirks(struct device *dev, struct
> xhci_hcd *xhci)
>*/
> if (pdev->subsystem_vendor == PCI_VENDOR_ID_HP)
> xhci->quirks |= XHCI_SPURIOUS_WAKEUP;
> -
> + }
> + if (pdev->vendor == PCI_VENDOR_ID_INTEL &&
> + pdev->device == PCI_DEVICE_ID_INTEL_LYNXPOINT_LP_XHCI) {
> xhci->quirks |= XHCI_SPURIOUS_REBOOT;
> }
> if (pdev->vendor == PCI_VENDOR_ID_ETRON &&
--
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: ehci: Enable support for 64bit EHCI host controllers in arm64
On Tue, May 20, 2014 at 12:08:58PM +0100, Arnd Bergmann wrote: > On Tuesday 20 May 2014 12:02:46 Catalin Marinas wrote: > > On Mon, May 19, 2014 at 05:55:56PM +0100, Arnd Bergmann wrote: > > > On Monday 19 May 2014 16:56:08 Catalin Marinas wrote: > > > > On Mon, May 19, 2014 at 10:44:51AM +0100, Arnd Bergmann wrote: > > > > > On Monday 19 May 2014 10:03:40 Catalin Marinas wrote: > > > We probably want to default to 32-bit for arm32 in the absence of > > > dma-ranges. > > > For arm64, I'd prefer if we could always mandate dma-ranges to be present > > > for each bus, just like we mandate ranges to be present. > > > I hope it's not too late for that. > > > > > > dma_set_mask should definitely look at the dma-ranges properties, and the > > > helper that Santosh just introduced should give us all the information > > > we need. We just need to decide on the correct behavior. > > > > Last time I looked at Santosh's patches I thought the dma-ranges is per > > device rather than per bus. We could make it per bus only and let the > > device call dma_set_mask() explicitly if it wants to restrict it > > further. > > Can you check again? I've read the code again yesterday to check this, > and I concluded it was correctly doing this per bus. You are right, I missed the fact that of_dma_get_range() checks the dma-ranges property in the node's parent. So what we need is setting the default dma mask based on the size information in dma-ranges to something like this: mask = rounddown_pow_of_two(size) - 1; dma_set_mask(dev, mask);/* or dma_set_mask_and_coherent() */ > > > > > While we currently don't have a set of swiotlb DMA ops on ARM32, we do > > > > > have it on ARM64, and I think we should be using them properly. It > > > > > should > > > > > really not be hard to implement a proper dma_set_mask() function for > > > > > ARM64 that gets is able to set up the swiotlb based on the dma-ranges > > > > > properties and always returns success but leaves the mask unchanged. > > > > > > > > The swiotlb bounce buffer needs to be pre-allocated at boot, otherwise > > > > we don't have any guarantees. Since we can't honour random masks anyway, > > > > we stick to ZONE_DMA which is currently in the 4G limit. But the driver > > > > calls dma_set_mask() too late for any further swiotlb setup. > > > > > > > > With IOMMU we can be more flexible around dma_set_mask(), can be done at > > > > run-time. > > > > > > What we can do with swiotlb is to check if the mask is smaller than > > > ZONE_DMA. > > > If it ever is, we have to fail dma_set_mask and hope the driver can fall > > > back to PIO mode or it will have to fail its probe() function. > > > > dma_set_(coherent_)mask check swiotlb_dma_supported() which returns > > false if io_tlb_end goes beyond the device mask. So we just need to > > ensure that io_tlb is allocated within ZONE_DMA. > > Makes sense for dma_set_mask. Why do you do the same thing for > coherent_mask? Shouldn't that check against ZONE_DMA instead? It depends on the meaning of coherent_dma_mask. As I understand it, that's the dma mask use for dma_alloc_coherent() to return a memory buffer that the device is able to access. I don't see it much different from the dma_mask used by the streaming API. I guess some hardware could have different masks here if they have cache coherency only for certain memory ranges (and the coherent_dma_mask would be smaller than dma_mask). > > > For dma_set_coherent_mask(), we also have to fail any call that tries to > > > set a mask larger than what the device hardware can do. Unlike that, > > > dma_set_mask() can succeed with any mask, we just have to enable swiotlb > > > if the mask that the driver wants is larger than what the hardware can > > > do. > > > > Currently we can't satisfy any arbitrarily small dma mask even with > > swiotlb since the bounce buffer is just guaranteed to be in ZONE_DMA. > > Swiotlb allows for smaller masks but we need to reserve the io_tlb > > buffer early during boot and at smaller addresses. For example, > > swiotlb_alloc_coherent() first tries __get_free_pages(GFP_DMA) and if > > the coherent_dma_mask isn't matched, it frees the pages and falls back > > to the io_tlb buffer. However, I don't think it's worth going for masks > > smaller than 32-bit on arm64. > > Is that safe for noncoherent systems? I'd expect the io_tlb buffer > to be cached there, which means we can't use it for coherent allocations. For non-coherent systems, the current arm64 approach is to take the address returned by swiotlb_alloc_coherent() and remap it as Normal NonCacheable (see the arm64 __dma_alloc_noncoherent()). The CPU should only access the dma buffer via the non-cacheable mapping. Of course, another approach would be to change the cacheability attributes of the io_tlb buffer (or the CMA one) but current approach has the advantage that the io_tlb buffer can be used for both coherent and non-coherent devices. -- Catalin -- To unsubscrib
Re: [PATCH 2/9] usb: gadget: net2280: Dont use magic numbers
On Mon, 19 May 2014, Ricardo Ribalda Delgado wrote:
> Instead of using magic numbers use #defines
>
> Signed-off-by: Ricardo Ribalda Delgado
> ---
> drivers/usb/gadget/net2280.c | 68
> +++-
> drivers/usb/gadget/net2280.h | 1 +
> 2 files changed, 36 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/usb/gadget/net2280.c b/drivers/usb/gadget/net2280.c
> index 8112d91..ba1fdd8 100644
> --- a/drivers/usb/gadget/net2280.c
> +++ b/drivers/usb/gadget/net2280.c
> @@ -152,7 +152,7 @@ static inline void enable_pciirqenb(struct net2280_ep *ep)
> {
> u32 tmp = readl(&ep->dev->regs->pciirqenb0);
>
> - if (ep->dev->pdev->vendor == 0x17cc)
> + if (ep->dev->pdev->vendor == PCI_VENDOR_ID_PLX_LEGACY)
You ought to be able to make this considerably smaller by using a
quirk flag in ep->dev. The flag can indicate whether or not the
controller uses the LEGACY interface.
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: ASMedia AS2105 interface on USB 3.0 corrupts data
On Tue, 20 May 2014, Heinz Diehl wrote: > Hi, > > the use of the ASMedia AS2105 controller on USB 3.0 leads to the > syslog being spammed with a lot of entries like this: > > usb 9-2: reset SuperSpeed USB device number 3 using xhci_hcd > [ 232.682214] usb 9-2: Parent hub missing LPM exit latency info. Power > management will be impacted. > [ 232.683435] xhci_hcd :02:00.0: xHCI xhci_drop_endpoint called with > disabled ep 8800c751ff00 > [ 232.683444] xhci_hcd :02:00.0: xHCI xhci_drop_endpoint called with > disabled ep 8800c751ff40 > > After some minutes, the filesystem on the HDD gets corrupted, and the > HDD gets unmounted/dropped out (disconnected). > > Tried three totally different machines with different hardware > components, with the same result. The AS2105 works flawlessly using > Windows 7-64, as it does connected to USB 2.0/1.1 in Linux. I have no idea if this will help, but you can try posting a usbmon trace showing what happens when the device is plugged in and the errors occur. 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
[PATCH] option: add device ID
From: Oliver Neukum
Reported by Alif Mubarak Ahmad:
This device vendor and product id is 1c9e:9800
It is working as serial interface with generic usbserial driver.
I thought it is more suitable to use usbserial option driver, which has better
capability distinguishing between modem serial interface and micro sd storage
interface.
Signed-off-by: Oliver Neukum
Tested-by: Alif Mubarak Ahmad
---
drivers/usb/serial/option.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index f213ee9..5853fdf 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -351,6 +351,9 @@ static void option_instat_callback(struct urb *urb);
/* Zoom */
#define ZOOM_PRODUCT_4597 0x9607
+/* SU9800 usb 3g modem*/
+#define SU9800_PRODUCT 0x9800
+
/* Haier products */
#define HAIER_VENDOR_ID0x201e
#define HAIER_PRODUCT_CE1000x2009
@@ -1575,6 +1578,7 @@ static const struct usb_device_id option_ids[] = {
{ USB_DEVICE(LONGCHEER_VENDOR_ID, FOUR_G_SYSTEMS_PRODUCT_W14),
.driver_info = (kernel_ulong_t)&four_g_w14_blacklist
},
+ { USB_DEVICE_INTERFACE_CLASS(LONGCHEER_VENDOR_ID, SU9800_PRODUCT, 0xff)
},
{ USB_DEVICE(LONGCHEER_VENDOR_ID, ZOOM_PRODUCT_4597) },
{ USB_DEVICE(LONGCHEER_VENDOR_ID, IBALL_3_5G_CONNECT) },
{ USB_DEVICE(HAIER_VENDOR_ID, HAIER_PRODUCT_CE100) },
--
1.8.4.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 2/9] usb: gadget: net2280: Dont use magic numbers
Hello Alan
In this case, there are only two family of devices supported by the
driver. Maybe quirks are too much.
At the end:
ep->dev->pdev->vendor == PCI_VENDOR_ID_PLX_LEGACY
is not much bigger than
ep->dev->quirk_flag & PLX_NET2280_QUIRK
But if you prefer the quirk I dont mind implementing it, but please as
a follow-up patch, since there are 8 patches depending on this :)
Regards!
On Tue, May 20, 2014 at 4:21 PM, Alan Stern wrote:
> On Mon, 19 May 2014, Ricardo Ribalda Delgado wrote:
>
>> Instead of using magic numbers use #defines
>>
>> Signed-off-by: Ricardo Ribalda Delgado
>> ---
>> drivers/usb/gadget/net2280.c | 68
>> +++-
>> drivers/usb/gadget/net2280.h | 1 +
>> 2 files changed, 36 insertions(+), 33 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/net2280.c b/drivers/usb/gadget/net2280.c
>> index 8112d91..ba1fdd8 100644
>> --- a/drivers/usb/gadget/net2280.c
>> +++ b/drivers/usb/gadget/net2280.c
>> @@ -152,7 +152,7 @@ static inline void enable_pciirqenb(struct net2280_ep
>> *ep)
>> {
>> u32 tmp = readl(&ep->dev->regs->pciirqenb0);
>>
>> - if (ep->dev->pdev->vendor == 0x17cc)
>> + if (ep->dev->pdev->vendor == PCI_VENDOR_ID_PLX_LEGACY)
>
> You ought to be able to make this considerably smaller by using a
> quirk flag in ep->dev. The flag can indicate whether or not the
> controller uses the LEGACY interface.
>
> Alan Stern
>
--
Ricardo Ribalda
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/9] usb: gadget: net2280: Dont use magic numbers
On Tue, 20 May 2014, Ricardo Ribalda Delgado wrote: > Hello Alan > > In this case, there are only two family of devices supported by the > driver. Maybe quirks are too much. > > At the end: > > ep->dev->pdev->vendor == PCI_VENDOR_ID_PLX_LEGACY > > is not much bigger than > > ep->dev->quirk_flag & PLX_NET2280_QUIRK > > But if you prefer the quirk I dont mind implementing it, but please as > a follow-up patch, since there are 8 patches depending on this :) It's not a big deal. Up to you. 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
new driver Fresco Logic
Forgive my intrusion ... Im hoping to get a driver made for a Fresco Logic FL2000 USB to VGA adapter ... Device ID 1d5c:2000 Mike KC7NOA -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 04/10] clk: tegra: Initialize xusb clocks
On Thu, May 15, 2014 at 09:22:00PM +0200, Stephen Warren wrote: > On 05/14/2014 06:33 PM, Andrew Bresticker wrote: > > Initialize the XUSB-related clocks with appropriate parents and rates > > for both Tegra114 and Tegra124. > > These first 4 clock driver patches look plausible to me, although I > didn't look that hard! > > Peter or Mike, if they look OK to you, can you please pick them up for > 3.16 so that by the time we apply the rest of the patches in this series > for 3.17, the clock dependencies are already there? Thanks. Looks fine to me. Applied for 3.16, hopefully it will still make it. Cheers, Peter. -- 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: ASMedia AS2105 interface on USB 3.0 corrupts data
On 20.05.2014, Alan Stern wrote: > I have no idea if this will help, but you can try posting a usbmon > trace showing what happens when the device is plugged in and the errors > occur. Connected the drive and did some "dd if=/dev/zero..." to it. It failed within seconds. This is what "dmesg" shows right after that. [a whole bunch of the next two lines deleted here] [ 294.528690] xhci_hcd :02:00.0: xHCI xhci_drop_endpoint called with disabled ep 880222ed2000 [ 294.528702] xhci_hcd :02:00.0: xHCI xhci_drop_endpoint called with disabled ep 880222ed2040 [ 294.530087] sd 17:0:0:0: [sdb] Unhandled error code [ 294.530097] sd 17:0:0:0: [sdb] [ 294.530102] Result: hostbyte=DID_ERROR driverbyte=DRIVER_OK [ 294.530107] sd 17:0:0:0: [sdb] CDB: [ 294.530110] Write(10): 2a 00 00 00 14 a0 00 00 f0 00 [ 294.530128] end_request: I/O error, dev sdb, sector 5280 Right after that, the drive has been assigned to /dev/sdc. /dev/sdb does no longer exist. [htd@keera ~]$ lsblk NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT sda 8:00 238.5G 0 disk ├─sda1 8:10 500M 0 part /boot ├─sda2 8:2050G 0 part / ├─sda3 8:30 8G 0 part [SWAP] └─sda4 8:40 180G 0 part /home sdc 8:32 0 149.1G 0 disk └─sdc1 8:33 0 149.1G 0 part sr0 11:01 1024M 0 rom The usbmon dump of the whole process is here: http://www.fritha.org/usblog.zip While writing this mail, the USB disk is popping in and out on the other machine which it is connected to... What I know for sure (because I verified it): 1. The harddrive itself is fine, and so is the fs on it (before it is mounted) 2. The faulty behaviour described here only occurs using USB 3.0. UBS 2 and 1.1 are fine. 3. The faulty behaviour is the same on two other machines with different hardware. Thanks, Heinz. -- 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: ASMedia AS2105 interface on USB 3.0 corrupts data
On 20.05.2014, Heinz Diehl wrote: > The usbmon dump of the whole process is here: > http://www.fritha.org/usblog.zip And here's another one, while popping in and out. http://www.fritha.org/usblog2.zip -- 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 08/10 v2.1] usb: gadget: net2280: Code Cleanup
- Move logical continuations to end of line
- Improve spacing
Signed-off-by: Ricardo Ribalda Delgado
---
v2: Comments by Alan Stern
-Use octal notation istead of S_I
drivers/usb/gadget/net2280.c | 155 +--
drivers/usb/gadget/net2280.h | 4 +-
2 files changed, 78 insertions(+), 81 deletions(-)
diff --git a/drivers/usb/gadget/net2280.c b/drivers/usb/gadget/net2280.c
index d1d4f4f..d506c83 100644
--- a/drivers/usb/gadget/net2280.c
+++ b/drivers/usb/gadget/net2280.c
@@ -100,9 +100,9 @@ static bool use_dma_chaining;
static bool use_msi = true;
/* "modprobe net2280 use_dma=n" etc */
-module_param(use_dma, bool, S_IRUGO);
-module_param(use_dma_chaining, bool, S_IRUGO);
-module_param(use_msi, bool, S_IRUGO);
+module_param(use_dma, bool, 0444);
+module_param(use_dma_chaining, bool, 0444);
+module_param(use_msi, bool, 0444);
/* mode 0 == ep-{a,b,c,d} 1K fifo each
* mode 1 == ep-{a,b} 2K fifo each, ep-{c,d} unavailable
@@ -111,7 +111,7 @@ module_param(use_msi, bool, S_IRUGO);
static ushort fifo_mode;
/* "modprobe net2280 fifo_mode=1" etc */
-module_param (fifo_mode, ushort, 0644);
+module_param(fifo_mode, ushort, 0644);
/* enable_suspend -- When enabled, the driver will respond to
* USB suspend requests by powering down the NET2280. Otherwise,
@@ -121,7 +121,7 @@ module_param (fifo_mode, ushort, 0644);
static bool enable_suspend;
/* "modprobe net2280 enable_suspend=1" etc */
-module_param(enable_suspend, bool, S_IRUGO);
+module_param(enable_suspend, bool, 0444);
/* force full-speed operation */
static bool full_speed;
@@ -169,8 +169,8 @@ net2280_enable(struct usb_ep *_ep, const struct
usb_endpoint_descriptor *desc)
static const u32 ep_key[9] = { 1, 0, 1, 0, 1, 1, 0, 1, 0 };
ep = container_of(_ep, struct net2280_ep, ep);
- if (!_ep || !desc || ep->desc || _ep->name == ep0name
- || desc->bDescriptorType != USB_DT_ENDPOINT)
+ if (!_ep || !desc || ep->desc || _ep->name == ep0name ||
+ desc->bDescriptorType != USB_DT_ENDPOINT)
return -EINVAL;
dev = ep->dev;
if (!dev->driver || dev->gadget.speed == USB_SPEED_UNKNOWN)
@@ -220,9 +220,9 @@ net2280_enable(struct usb_ep *_ep, const struct
usb_endpoint_descriptor *desc)
tmp = (desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK);
if (tmp == USB_ENDPOINT_XFER_INT) {
/* erratum 0105 workaround prevents hs NYET */
- if (dev->chiprev == 0100
- && dev->gadget.speed == USB_SPEED_HIGH
- && !(desc->bEndpointAddress & USB_DIR_IN))
+ if (dev->chiprev == 0100 &&
+ dev->gadget.speed == USB_SPEED_HIGH &&
+ !(desc->bEndpointAddress & USB_DIR_IN))
writel(BIT(CLEAR_NAK_OUT_PACKETS_MODE),
&ep->regs->ep_rsp);
} else if (tmp == USB_ENDPOINT_XFER_BULK) {
@@ -402,8 +402,8 @@ static void ep_reset_228x(struct net2280_regs __iomem *regs,
BIT(DATA_PACKET_RECEIVED_INTERRUPT) |
BIT(DATA_PACKET_TRANSMITTED_INTERRUPT) |
BIT(DATA_OUT_PING_TOKEN_INTERRUPT) |
- BIT(DATA_IN_TOKEN_INTERRUPT)
- , &ep->regs->ep_stat);
+ BIT(DATA_IN_TOKEN_INTERRUPT),
+ &ep->regs->ep_stat);
/* fifo size is handled separately */
}
@@ -425,9 +425,9 @@ static void ep_reset_338x(struct net2280_regs __iomem *regs,
writel(BIT(DMA_ABORT_DONE_INTERRUPT) |
BIT(DMA_PAUSE_DONE_INTERRUPT) |
BIT(DMA_SCATTER_GATHER_DONE_INTERRUPT) |
- BIT(DMA_TRANSACTION_DONE_INTERRUPT)
- /* | BIT(DMA_ABORT) */
- , &ep->dma->dmastat);
+ BIT(DMA_TRANSACTION_DONE_INTERRUPT),
+ /* | BIT(DMA_ABORT), */
+ &ep->dma->dmastat);
dmastat = readl(&ep->dma->dmastat);
if (dmastat == 0x5002) {
@@ -618,15 +618,15 @@ static void out_flush(struct net2280_ep *ep)
statp = &ep->regs->ep_stat;
writel(BIT(DATA_OUT_PING_TOKEN_INTERRUPT) |
- BIT(DATA_PACKET_RECEIVED_INTERRUPT)
- , statp);
+ BIT(DATA_PACKET_RECEIVED_INTERRUPT),
+ statp);
writel(BIT(FIFO_FLUSH), statp);
/* Make sure that stap is written */
mb();
tmp = readl(statp);
- if (tmp & BIT(DATA_OUT_PING_TOKEN_INTERRUPT)
+ if (tmp & BIT(DATA_OUT_PING_TOKEN_INTERRUPT) &&
/* high speed did bulk NYET; fifo isn't filling */
- && ep->dev->gadget.speed == USB_SPEED_FULL) {
+ ep->dev->gadget.speed == USB_SPEED_FULL) {
unsignedusec;
usec = 50; /* 64 b
[PATCH 10/10] usb: gadget: net2280: Use quirks instead of pci id
Use of quirks improve readability and will be easier to add new devices
to this driver.
Suggested-by: Alan Stern
Signed-off-by: Ricardo Ribalda Delgado
---
drivers/usb/gadget/net2280.c | 83 +++-
drivers/usb/gadget/net2280.h | 6
2 files changed, 49 insertions(+), 40 deletions(-)
diff --git a/drivers/usb/gadget/net2280.c b/drivers/usb/gadget/net2280.c
index 9ced9ff..ce8bc86 100644
--- a/drivers/usb/gadget/net2280.c
+++ b/drivers/usb/gadget/net2280.c
@@ -147,7 +147,7 @@ static inline void enable_pciirqenb(struct net2280_ep *ep)
{
u32 tmp = readl(&ep->dev->regs->pciirqenb0);
- if (ep->dev->pdev->vendor == PCI_VENDOR_ID_PLX_LEGACY)
+ if (ep->dev->quirks & PLX_LEGACY)
tmp |= BIT(ep->num);
else
tmp |= BIT(ep_bit[ep->num]);
@@ -177,7 +177,7 @@ net2280_enable(struct usb_ep *_ep, const struct
usb_endpoint_descriptor *desc)
if ((desc->bEndpointAddress & 0x0f) == EP_DONTUSE)
return -EDOM;
- if (dev->pdev->vendor == PCI_VENDOR_ID_PLX) {
+ if (dev->quirks & PLX_SUPERSPEED) {
if ((desc->bEndpointAddress & 0x0f) >= 0x0c)
return -EDOM;
ep->is_in = !!usb_endpoint_dir_in(desc);
@@ -187,8 +187,7 @@ net2280_enable(struct usb_ep *_ep, const struct
usb_endpoint_descriptor *desc)
/* sanity check ep-e/ep-f since their fifos are small */
max = usb_endpoint_maxp(desc) & 0x1fff;
- if (ep->num > 4 && max > 64 &&
- (dev->pdev->vendor == PCI_VENDOR_ID_PLX_LEGACY))
+ if (ep->num > 4 && max > 64 && (dev->quirks & PLX_LEGACY))
return -ERANGE;
spin_lock_irqsave(&dev->lock, flags);
@@ -233,7 +232,7 @@ net2280_enable(struct usb_ep *_ep, const struct
usb_endpoint_descriptor *desc)
}
ep->is_iso = (tmp == USB_ENDPOINT_XFER_ISOC);
/* Enable this endpoint */
- if (dev->pdev->vendor == PCI_VENDOR_ID_PLX_LEGACY) {
+ if (dev->quirks & PLX_LEGACY) {
tmp <<= ENDPOINT_TYPE;
tmp |= desc->bEndpointAddress;
/* default full fifo lines */
@@ -263,7 +262,7 @@ net2280_enable(struct usb_ep *_ep, const struct
usb_endpoint_descriptor *desc)
/* for OUT transfers, block the rx fifo until a read is posted */
if (!ep->is_in)
writel(BIT(SET_NAK_OUT_PACKETS), &ep->regs->ep_rsp);
- else if (dev->pdev->device != 0x2280) {
+ else if (!(dev->quirks & PLX_2280)) {
/* Added for 2282, Don't use nak packets on an in endpoint,
* this was ignored on 2280
*/
@@ -279,7 +278,7 @@ net2280_enable(struct usb_ep *_ep, const struct
usb_endpoint_descriptor *desc)
tmp = BIT(DATA_PACKET_RECEIVED_INTERRUPT_ENABLE) |
BIT(DATA_PACKET_TRANSMITTED_INTERRUPT_ENABLE);
- if (dev->pdev->device == 0x2280)
+ if (dev->quirks & PLX_2280)
tmp |= readl(&ep->regs->ep_irqenb);
writel(tmp, &ep->regs->ep_irqenb);
} else {/* dma, per-request */
@@ -361,7 +360,7 @@ static void ep_reset_228x(struct net2280_regs __iomem *regs,
/* init to our chosen defaults, notably so that we NAK OUT
* packets until the driver queues a read (+note erratum 0112)
*/
- if (!ep->is_in || ep->dev->pdev->device == 0x2280) {
+ if (!ep->is_in || (ep->dev->quirks & PLX_2280)) {
tmp = BIT(SET_NAK_OUT_PACKETS_MODE) |
BIT(SET_NAK_OUT_PACKETS) |
BIT(CLEAR_EP_HIDE_STATUS_PHASE) |
@@ -381,7 +380,7 @@ static void ep_reset_228x(struct net2280_regs __iomem *regs,
writel(tmp, &ep->regs->ep_rsp);
/* scrub most status bits, and flush any fifo state */
- if (ep->dev->pdev->device == 0x2280)
+ if (ep->dev->quirks & PLX_2280)
tmp = BIT(FIFO_OVERFLOW) |
BIT(FIFO_UNDERFLOW);
else
@@ -468,7 +467,7 @@ static int net2280_disable(struct usb_ep *_ep)
spin_lock_irqsave(&ep->dev->lock, flags);
nuke(ep);
- if (ep->dev->pdev->vendor == PCI_VENDOR_ID_PLX)
+ if (ep->dev->quirks & PLX_SUPERSPEED)
ep_reset_338x(ep->dev->regs, ep);
else
ep_reset_228x(ep->dev->regs, ep);
@@ -742,7 +741,7 @@ static void fill_dma_desc(struct net2280_ep *ep,
if (ep->is_in)
dmacount |= BIT(DMA_DIRECTION);
if ((!ep->is_in && (dmacount % ep->ep.maxpacket) != 0) ||
- ep->dev->pdev->device != 0x2280)
+ !(ep->dev->quirks & PLX_2280))
dmacount |= BIT(END_OF_CHAIN);
req->valid = valid;
@@ -786,14 +785,14 @@ static void start_queue(struct net2280_ep *ep, u32
dmactl, u32 td_dma)
struct net2280_dma_regs __iomem *dma = e
[PATCH 06/10] usb: gadget: net2280: Refactor queues_show
Replace a long and ugly expresion with an already available function.
Signed-off-by: Ricardo Ribalda Delgado
---
drivers/usb/gadget/net2280.c | 10 +-
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/usb/gadget/net2280.c b/drivers/usb/gadget/net2280.c
index bd851de..c3205ec 100644
--- a/drivers/usb/gadget/net2280.c
+++ b/drivers/usb/gadget/net2280.c
@@ -1777,15 +1777,7 @@ static ssize_t queues_show(struct device *_dev, struct
device_attribute *attr,
"\n%s (ep%d%s-%s) max %04x %s fifo %d\n",
ep->ep.name, t & USB_ENDPOINT_NUMBER_MASK,
(t & USB_DIR_IN) ? "in" : "out",
- ({ char *val;
-switch (d->bmAttributes & 0x03) {
-case USB_ENDPOINT_XFER_BULK:
- val = "bulk"; break;
-case USB_ENDPOINT_XFER_INT:
- val = "intr"; break;
-default:
- val = "iso"; break;
-} val; }),
+ type_string(d->bmAttributes),
usb_endpoint_maxp (d) & 0x1fff,
ep->dma ? "dma" : "pio", ep->fifo_size
);
--
2.0.0.rc2
--
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 00/10] net2280: Support for PLX USB338x and cleanout code
The USB 3380 is a PCI Express Gen 2 to USB 3.0 SuperSpeed Peripheral Controller. It shares a lot functionatily with the net228x family. This series of patches includes some resend of previous patches. Please check the changelog for every patch. This time it has been placed under the tear line Ricardo Ribalda Delgado (10): usb: gadget: net2280: Add support for PLX USB338X usb: gadget: net2280: Dont use magic numbers usb: gadget: net2280: Use BIT() macro usb: gadget: net2280: Use true/false instead of 1/0 usb: gadget: net2280: Use module_pci_driver macro usb: gadget: net2280: Refactor queues_show usb: gadget: net2280: Pass checkpacth.pl test usb: gadget: net2280: Code Cleanup usb: gadget: net2280: Use pr_* function usb: gadget: net2280: Use quirks instead of pci id drivers/usb/gadget/Kconfig | 10 +- drivers/usb/gadget/net2280.c | 2752 -- drivers/usb/gadget/net2280.h | 285 +++-- include/linux/usb/usb338x.h | 199 +++ 4 files changed, 2235 insertions(+), 1011 deletions(-) create mode 100644 include/linux/usb/usb338x.h -- 2.0.0.rc2 -- 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 04/10] usb: gadget: net2280: Use true/false instead of 1/0
For bool variables
Signed-off-by: Ricardo Ribalda Delgado
---
drivers/usb/gadget/net2280.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/net2280.c b/drivers/usb/gadget/net2280.c
index 5b9368d..b43725a 100644
--- a/drivers/usb/gadget/net2280.c
+++ b/drivers/usb/gadget/net2280.c
@@ -95,9 +95,9 @@ static const char *const ep_name [] = {
* Some gadget drivers work better with the dma support here than others.
* These two parameters let you use PIO or more aggressive DMA.
*/
-static bool use_dma = 1;
-static bool use_dma_chaining = 0;
-static bool use_msi = 1;
+static bool use_dma = true;
+static bool use_dma_chaining;
+static bool use_msi = true;
/* "modprobe net2280 use_dma=n" etc */
module_param (use_dma, bool, S_IRUGO);
@@ -118,7 +118,7 @@ module_param (fifo_mode, ushort, 0644);
* USB suspend requests will be ignored. This is acceptable for
* self-powered devices
*/
-static bool enable_suspend = 0;
+static bool enable_suspend;
/* "modprobe net2280 enable_suspend=1" etc */
module_param (enable_suspend, bool, S_IRUGO);
--
2.0.0.rc2
--
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 03/10] usb: gadget: net2280: Use BIT() macro
Improves readability of the code
Signed-off-by: Ricardo Ribalda Delgado
---
drivers/usb/gadget/net2280.c | 572 +--
drivers/usb/gadget/net2280.h | 65 ++---
2 files changed, 318 insertions(+), 319 deletions(-)
diff --git a/drivers/usb/gadget/net2280.c b/drivers/usb/gadget/net2280.c
index ba1fdd8..5b9368d 100644
--- a/drivers/usb/gadget/net2280.c
+++ b/drivers/usb/gadget/net2280.c
@@ -144,8 +144,8 @@ static char *type_string (u8 bmAttributes)
#include "net2280.h"
-#define valid_bit cpu_to_le32 (1 << VALID_BIT)
-#define dma_done_iecpu_to_le32 (1 << DMA_DONE_INTERRUPT_ENABLE)
+#define valid_bit cpu_to_le32(BIT(VALID_BIT))
+#define dma_done_iecpu_to_le32(BIT(DMA_DONE_INTERRUPT_ENABLE))
/*-*/
static inline void enable_pciirqenb(struct net2280_ep *ep)
@@ -153,9 +153,9 @@ static inline void enable_pciirqenb(struct net2280_ep *ep)
u32 tmp = readl(&ep->dev->regs->pciirqenb0);
if (ep->dev->pdev->vendor == PCI_VENDOR_ID_PLX_LEGACY)
- tmp |= 1 << ep->num;
+ tmp |= BIT(ep->num);
else
- tmp |= 1 << ep_bit[ep->num];
+ tmp |= BIT(ep_bit[ep->num]);
writel(tmp, &ep->dev->regs->pciirqenb0);
return;
@@ -218,14 +218,14 @@ net2280_enable (struct usb_ep *_ep, const struct
usb_endpoint_descriptor *desc)
}
/* set type, direction, address; reset fifo counters */
- writel ((1 << FIFO_FLUSH), &ep->regs->ep_stat);
+ writel(BIT(FIFO_FLUSH), &ep->regs->ep_stat);
tmp = (desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK);
if (tmp == USB_ENDPOINT_XFER_INT) {
/* erratum 0105 workaround prevents hs NYET */
if (dev->chiprev == 0100
&& dev->gadget.speed == USB_SPEED_HIGH
&& !(desc->bEndpointAddress & USB_DIR_IN))
- writel ((1 << CLEAR_NAK_OUT_PACKETS_MODE),
+ writel(BIT(CLEAR_NAK_OUT_PACKETS_MODE),
&ep->regs->ep_rsp);
} else if (tmp == USB_ENDPOINT_XFER_BULK) {
/* catch some particularly blatant driver bugs */
@@ -243,18 +243,18 @@ net2280_enable (struct usb_ep *_ep, const struct
usb_endpoint_descriptor *desc)
tmp |= desc->bEndpointAddress;
/* default full fifo lines */
tmp |= (4 << ENDPOINT_BYTE_COUNT);
- tmp |= 1 << ENDPOINT_ENABLE;
+ tmp |= BIT(ENDPOINT_ENABLE);
ep->is_in = (tmp & USB_DIR_IN) != 0;
} else {
/* In Legacy mode, only OUT endpoints are used */
if (dev->enhanced_mode && ep->is_in) {
tmp <<= IN_ENDPOINT_TYPE;
- tmp |= (1 << IN_ENDPOINT_ENABLE);
+ tmp |= BIT(IN_ENDPOINT_ENABLE);
/* Not applicable to Legacy */
- tmp |= (1 << ENDPOINT_DIRECTION);
+ tmp |= BIT(ENDPOINT_DIRECTION);
} else {
tmp <<= OUT_ENDPOINT_TYPE;
- tmp |= (1 << OUT_ENDPOINT_ENABLE);
+ tmp |= BIT(OUT_ENDPOINT_ENABLE);
tmp |= (ep->is_in << ENDPOINT_DIRECTION);
}
@@ -267,13 +267,13 @@ net2280_enable (struct usb_ep *_ep, const struct
usb_endpoint_descriptor *desc)
/* for OUT transfers, block the rx fifo until a read is posted */
if (!ep->is_in)
- writel ((1 << SET_NAK_OUT_PACKETS), &ep->regs->ep_rsp);
+ writel(BIT(SET_NAK_OUT_PACKETS), &ep->regs->ep_rsp);
else if (dev->pdev->device != 0x2280) {
/* Added for 2282, Don't use nak packets on an in endpoint,
* this was ignored on 2280
*/
- writel ((1 << CLEAR_NAK_OUT_PACKETS)
- | (1 << CLEAR_NAK_OUT_PACKETS_MODE), &ep->regs->ep_rsp);
+ writel(BIT(CLEAR_NAK_OUT_PACKETS) |
+ BIT(CLEAR_NAK_OUT_PACKETS_MODE), &ep->regs->ep_rsp);
}
writel(tmp, &ep->cfg->ep_cfg);
@@ -282,13 +282,13 @@ net2280_enable (struct usb_ep *_ep, const struct
usb_endpoint_descriptor *desc)
if (!ep->dma) { /* pio, per-packet */
enable_pciirqenb(ep);
- tmp = (1 << DATA_PACKET_RECEIVED_INTERRUPT_ENABLE)
- | (1 << DATA_PACKET_TRANSMITTED_INTERRUPT_ENABLE);
+ tmp = BIT(DATA_PACKET_RECEIVED_INTERRUPT_ENABLE) |
+ BIT(DATA_PACKET_TRANSMITTED_INTERRUPT_ENABLE);
if (dev->pdev->device == 0x2280)
tmp |= readl (&ep->regs->ep_irqenb);
writel (tmp, &ep->regs->ep_irqenb);
} else {/* dma, per-request */
[PATCH 02/10] usb: gadget: net2280: Dont use magic numbers
Instead of using magic numbers use #defines
Signed-off-by: Ricardo Ribalda Delgado
---
drivers/usb/gadget/net2280.c | 68 +++-
drivers/usb/gadget/net2280.h | 1 +
2 files changed, 36 insertions(+), 33 deletions(-)
diff --git a/drivers/usb/gadget/net2280.c b/drivers/usb/gadget/net2280.c
index 8112d91..ba1fdd8 100644
--- a/drivers/usb/gadget/net2280.c
+++ b/drivers/usb/gadget/net2280.c
@@ -152,7 +152,7 @@ static inline void enable_pciirqenb(struct net2280_ep *ep)
{
u32 tmp = readl(&ep->dev->regs->pciirqenb0);
- if (ep->dev->pdev->vendor == 0x17cc)
+ if (ep->dev->pdev->vendor == PCI_VENDOR_ID_PLX_LEGACY)
tmp |= 1 << ep->num;
else
tmp |= 1 << ep_bit[ep->num];
@@ -182,7 +182,7 @@ net2280_enable (struct usb_ep *_ep, const struct
usb_endpoint_descriptor *desc)
if ((desc->bEndpointAddress & 0x0f) == EP_DONTUSE)
return -EDOM;
- if (dev->pdev->vendor == 0x10b5) {
+ if (dev->pdev->vendor == PCI_VENDOR_ID_PLX) {
if ((desc->bEndpointAddress & 0x0f) >= 0x0c)
return -EDOM;
ep->is_in = !!usb_endpoint_dir_in(desc);
@@ -192,7 +192,8 @@ net2280_enable (struct usb_ep *_ep, const struct
usb_endpoint_descriptor *desc)
/* sanity check ep-e/ep-f since their fifos are small */
max = usb_endpoint_maxp (desc) & 0x1fff;
- if (ep->num > 4 && max > 64 && (dev->pdev->vendor == 0x17cc))
+ if (ep->num > 4 && max > 64 &&
+ (dev->pdev->vendor == PCI_VENDOR_ID_PLX_LEGACY))
return -ERANGE;
spin_lock_irqsave (&dev->lock, flags);
@@ -237,7 +238,7 @@ net2280_enable (struct usb_ep *_ep, const struct
usb_endpoint_descriptor *desc)
}
ep->is_iso = (tmp == USB_ENDPOINT_XFER_ISOC) ? 1 : 0;
/* Enable this endpoint */
- if (dev->pdev->vendor == 0x17cc) {
+ if (dev->pdev->vendor == PCI_VENDOR_ID_PLX_LEGACY) {
tmp <<= ENDPOINT_TYPE;
tmp |= desc->bEndpointAddress;
/* default full fifo lines */
@@ -472,7 +473,7 @@ static int net2280_disable (struct usb_ep *_ep)
spin_lock_irqsave (&ep->dev->lock, flags);
nuke (ep);
- if (ep->dev->pdev->vendor == 0x10b5)
+ if (ep->dev->pdev->vendor == PCI_VENDOR_ID_PLX)
ep_reset_338x(ep->dev->regs, ep);
else
ep_reset_228x(ep->dev->regs, ep);
@@ -799,7 +800,7 @@ static void start_queue (struct net2280_ep *ep, u32 dmactl,
u32 td_dma)
writel (readl (&dma->dmastat), &dma->dmastat);
writel (td_dma, &dma->dmadesc);
- if (ep->dev->pdev->vendor == 0x10b5)
+ if (ep->dev->pdev->vendor == PCI_VENDOR_ID_PLX)
dmactl |= (0x01 << DMA_REQUEST_OUTSTANDING);
writel (dmactl, &dma->dmactl);
@@ -995,7 +996,7 @@ net2280_queue (struct usb_ep *_ep, struct usb_request
*_req, gfp_t gfp_flags)
/* DMA request while EP halted */
if (ep->dma &&
(readl(&ep->regs->ep_rsp) & (1 << CLEAR_ENDPOINT_HALT)) &&
- (dev->pdev->vendor == 0x10b5)) {
+ (dev->pdev->vendor == PCI_VENDOR_ID_PLX)) {
int valid = 1;
if (ep->is_in) {
int expect;
@@ -1126,7 +1127,7 @@ static void scan_dma_completions (struct net2280_ep *ep)
} else if (!ep->is_in
&& (req->req.length % ep->ep.maxpacket) != 0) {
tmp = readl (&ep->regs->ep_stat);
- if (ep->dev->pdev->vendor == 0x10b5)
+ if (ep->dev->pdev->vendor == PCI_VENDOR_ID_PLX)
return dma_done(ep, req, tmp, 0);
/* AVOID TROUBLE HERE by not issuing short reads from
@@ -1234,7 +1235,7 @@ static void abort_dma_338x(struct net2280_ep *ep)
static void abort_dma(struct net2280_ep *ep)
{
- if (ep->dev->pdev->vendor == 0x17cc)
+ if (ep->dev->pdev->vendor == PCI_VENDOR_ID_PLX_LEGACY)
return abort_dma_228x(ep);
return abort_dma_338x(ep);
}
@@ -1392,7 +1393,7 @@ net2280_set_halt_and_wedge(struct usb_ep *_ep, int value,
int wedged)
ep->wedged = 1;
} else {
clear_halt (ep);
- if (ep->dev->pdev->vendor == 0x10b5 &&
+ if (ep->dev->pdev->vendor == PCI_VENDOR_ID_PLX &&
!list_empty(&ep->queue) && ep->td_dma)
restart_dma(ep);
ep->wedged = 0;
@@ -2104,7 +2105,7 @@ static void usb_reset_338x(struct net2280 *dev)
static void usb_reset(struct net2280 *dev)
{
- if (dev->pdev->vendor == 0x17cc)
+ if (dev->pdev->vendor == PCI_VENDOR_ID_PLX_LEGACY)
return usb
[PATCH 05/10] usb: gadget: net2280: Use module_pci_driver macro
Signed-off-by: Ricardo Ribalda Delgado
---
drivers/usb/gadget/net2280.c | 19 +--
1 file changed, 5 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/gadget/net2280.c b/drivers/usb/gadget/net2280.c
index b43725a..bd851de 100644
--- a/drivers/usb/gadget/net2280.c
+++ b/drivers/usb/gadget/net2280.c
@@ -3588,6 +3588,9 @@ static int net2280_probe (struct pci_dev *pdev, const
struct pci_device_id *id)
void__iomem *base = NULL;
int retval, i;
+ if (!use_dma)
+ use_dma_chaining = 0;
+
/* alloc, and start init */
dev = kzalloc (sizeof *dev, GFP_KERNEL);
if (dev == NULL){
@@ -3833,20 +3836,8 @@ static struct pci_driver net2280_pci_driver = {
/* FIXME add power management support */
};
+module_pci_driver(net2280_pci_driver);
+
MODULE_DESCRIPTION (DRIVER_DESC);
MODULE_AUTHOR ("David Brownell");
MODULE_LICENSE ("GPL");
-
-static int __init init (void)
-{
- if (!use_dma)
- use_dma_chaining = 0;
- return pci_register_driver (&net2280_pci_driver);
-}
-module_init (init);
-
-static void __exit cleanup (void)
-{
- pci_unregister_driver (&net2280_pci_driver);
-}
-module_exit (cleanup);
--
2.0.0.rc2
--
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 09/10 v2.1] usb: gadget: net2280: Use pr_* function
Driver was using custom functions WARNING, ERROR, DEBUG, instead of
pr_err, pr_dgb...
New ep_* macros have been created that use standard pr_* functions.
Signed-off-by: Ricardo Ribalda Delgado
---
v2: Comment by Joe Perches and Alan Stern
-Use ep_* instead of custom DETDEV macro
drivers/usb/gadget/net2280.c | 121 +--
drivers/usb/gadget/net2280.h | 36 +
2 files changed, 71 insertions(+), 86 deletions(-)
diff --git a/drivers/usb/gadget/net2280.c b/drivers/usb/gadget/net2280.c
index d506c83..9ced9ff 100644
--- a/drivers/usb/gadget/net2280.c
+++ b/drivers/usb/gadget/net2280.c
@@ -42,9 +42,6 @@
* (at your option) any later version.
*/
-#undef DEBUG /* messages on error and most fault paths */
-#undef VERBOSE /* extra debug messages (success too) */
-
#include
#include
#include
@@ -210,7 +207,7 @@ net2280_enable(struct usb_ep *_ep, const struct
usb_endpoint_descriptor *desc)
* use it instead of troublesome (non-bulk) multi-packet DMA.
*/
if (ep->dma && (max % 4) != 0 && use_dma_chaining) {
- DEBUG(ep->dev, "%s, no dma for maxpacket %d\n",
+ ep_dbg(ep->dev, "%s, no dma for maxpacket %d\n",
ep->ep.name, ep->ep.maxpacket);
ep->dma = NULL;
}
@@ -303,7 +300,7 @@ net2280_enable(struct usb_ep *_ep, const struct
usb_endpoint_descriptor *desc)
}
tmp = desc->bEndpointAddress;
- DEBUG(dev, "enabled %s (ep%d%s-%s) %s max %04x\n",
+ ep_dbg(dev, "enabled %s (ep%d%s-%s) %s max %04x\n",
_ep->name, tmp & 0x0f, DIR_STRING(tmp),
type_string(desc->bmAttributes),
ep->dma ? "dma" : "pio", max);
@@ -431,7 +428,7 @@ static void ep_reset_338x(struct net2280_regs __iomem *regs,
dmastat = readl(&ep->dma->dmastat);
if (dmastat == 0x5002) {
- WARNING(ep->dev, "The dmastat return = %x!!\n",
+ ep_warn(ep->dev, "The dmastat return = %x!!\n",
dmastat);
writel(0x5a, &ep->dma->dmastat);
}
@@ -476,7 +473,7 @@ static int net2280_disable(struct usb_ep *_ep)
else
ep_reset_228x(ep->dev->regs, ep);
- VDEBUG(ep->dev, "disabled %s %s\n",
+ ep_vdbg(ep->dev, "disabled %s %s\n",
ep->dma ? "dma" : "pio", _ep->name);
/* synch memory views with the device */
@@ -572,7 +569,7 @@ static void write_fifo(struct net2280_ep *ep, struct
usb_request *req)
if (count > total) /* min() cannot be used on a bitfield */
count = total;
- VDEBUG(ep->dev, "write %s fifo (IN) %d bytes%s req %p\n",
+ ep_vdbg(ep->dev, "write %s fifo (IN) %d bytes%s req %p\n",
ep->ep.name, count,
(count != ep->ep.maxpacket) ? " (short)" : "",
req);
@@ -684,7 +681,7 @@ static int read_fifo(struct net2280_ep *ep, struct
net2280_request *req)
if (count > tmp) {
/* as with DMA, data overflow gets flushed */
if ((tmp % ep->ep.maxpacket) != 0) {
- ERROR(ep->dev,
+ ep_err(ep->dev,
"%s out fifo %d bytes, expected %d\n",
ep->ep.name, count, tmp);
req->req.status = -EOVERFLOW;
@@ -699,7 +696,7 @@ static int read_fifo(struct net2280_ep *ep, struct
net2280_request *req)
is_short = (count == 0) || ((count % ep->ep.maxpacket) != 0);
- VDEBUG(ep->dev, "read %s fifo (OUT) %d bytes%s%s%s req %p %d/%d\n",
+ ep_vdbg(ep->dev, "read %s fifo (OUT) %d bytes%s%s%s req %p %d/%d\n",
ep->ep.name, count, is_short ? " (short)" : "",
cleanup ? " flush" : "", prevent ? " nak" : "",
req, req->req.actual, req->req.length);
@@ -925,7 +922,7 @@ done(struct net2280_ep *ep, struct net2280_request *req,
int status)
usb_gadget_unmap_request(&dev->gadget, &req->req, ep->is_in);
if (status && status != -ESHUTDOWN)
- VDEBUG(dev, "complete %s req %p stat %d len %u/%u\n",
+ ep_vdbg(dev, "complete %s req %p stat %d len %u/%u\n",
ep->ep.name, &req->req, status,
req->req.actual, req->req.length);
@@ -978,7 +975,7 @@ net2280_queue(struct usb_ep *_ep, struct usb_request *_req,
gfp_t gfp_flags)
}
#if 0
- VDEBUG(dev, "%s queue req %p, len %d buf %p\n",
+ ep_vdbg(dev, "%s queue req %p, len %d buf %p\n",
_ep->name, _req, _req->length, _req->buf);
#endif
@@ -1012,7 +1009,7 @@ net2280_queue(struct usb_ep *_ep, struct usb_request
*_req, gfp_t gfp_flags)
if (ep->num == 0 && _req->length == 0) {
About ConfigFS and FunctionFS together
Hello Michal, Recently I'm working with configfs and libusbg. I have been also using FFS with it as it was merged in some recent kernel versions. While using I have experienced some issue while mounting it. Let me describe my it: $ cd /sys/kernel/config/usb_gadget $ mkdir g1 $ mkdir g1/functions/ffs.sdb At this point I'm able to mount using: $ mount sdb -t functionfs /tmp/sdb But if I make a mistake and write: $ mount sbd -t functionfs /tmp/sdb I get: mount: unknown filesystem type 'functionfs' Wow that was quite amazing for me, so I have checked few times /proc/filesystems and loaded modules and all looked fine. So I have used strace and found out that error returned from syscall was -ENODEV. I have started to look why such description is given to this error code. I found out that kernel returns -ENODEV error which when file system type has not been found. I look into code of ffs and found out that ffs_fs_mount() also returns this error if it was unable to acquire given device. I know that this error looks really suitable in this place but it really hurts discoverability in userspace because you don't know where did you make your typo in file system name or in dev name. I have run through few other fs implementations and found out that they all return -ENOENT if given device has not been found. Maybe FFS should follow this convention or maybe convention should be changed? -- Best regards, Krzysztof Opasiak Samsung R&D Institute Poland Samsung Electronics -- 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: ehci: Enable support for 64bit EHCI host controllers in arm64
On Tue, May 20, 2014 at 3:01 AM, Arnd Bergmann wrote: > On Monday 19 May 2014 16:42:18 Rob Herring wrote: >> On Mon, May 19, 2014 at 10:56 AM, Catalin Marinas >> wrote: >> > On Mon, May 19, 2014 at 10:44:51AM +0100, Arnd Bergmann wrote: >> > > On Monday 19 May 2014 10:03:40 Catalin Marinas wrote: >> > > > On Mon, May 19, 2014 at 09:32:43AM +0100, Arnd Bergmann wrote: > >> > > We definitely have to fix this very quickly, before people start building >> > > real arm64 systems and shipping them. >> > > >> > > We should not merge any hacks that attempt to work around the problem, >> > > but try to come to a conclusion how to handle them properly. >> > > My hope was that we could just always set the dma mask to whatever >> > > the DT says it should be to keep the burden from device drivers, >> > > unless they want to restrict it further (e.g. when the specific >> > > peripheral hardware has a bug that prevents us from using high addresses, >> > > even though the SoC in theory supports it everywhere). >> > >> > I agree. >> > >> > > Rob Herring argued that we should always mimic PCI and call >> > dma_set_mask() >> > > in drivers but default to a 32-bit mask otherwise, independent of whether >> > > the hardware can do more or less than that, IIRC. >> > >> > Can we not default to something built up from dma-ranges? Or 32-bit if >> > dma-ranges property is missing? >> > >> >> My reasoning was the information may not come from DT. For AHCI, the 32 vs. >> 64 bit capability is in a capability register and the DMA mask is set based >> on that. This information only exists with-in the driver. >> >> I think some sort of capability merging is needed here to merge a bus mask >> with a device mask. The current api doesn't seem to support this as >> adjusting the set mask is expected to fail if the requested mask is not >> supported. Here is how I see it working. Devices can have a default mask >> (32-bit typically). The default is simply the common case. Really the mask >> should only be set for DMA capable devices, but many drivers fail to set >> the mask. Devices can then enlarge or shrink what they support based on >> knowing the IP block features or getting capabilities from the IP block >> such as the AHCI case. This mask then needs to be adjusted (only shrinking) >> if the parent bus or platform has restrictions which is the part that >> should come from dma-ranges. > > Where do you think the shrinking should be done then? I'd like to see that > as part of the initial bus probe that Santosh just implemented to set the > offset. I think it would be reasonable to apply whatever the bus can do > there, but limit it to 32-bit. Before probe, we can only set a default unless we add a bus_mask or dma_mask from a parent device (i.e. a bus device). After driver probe would be too late as DMA allocations could occur during probe. So that basically leaves it to driver calls to dma_set_mask_and_coherent/dma_set_mask. We could do something like this for .set_dma_mask: bus_mask = of_get_dma_ranges_mask(); if (bus_mask) mask &= bus_mask; *dev->dma_mask = mask; There is an issue in how dma_set_coherent_mask works though. By default it can only check if the mask is valid, but cannot adjust the mask. As long as drivers use dma_set_mask or dma_set_mask_and_coherent, the above should work. This also adds an OF dependency into the DMA mapping code which I don't like too much. The alternative is storing the bus mask somewhere in struct device. Rob -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: About ConfigFS and FunctionFS together
On Tue, May 20 2014, Krzysztof Opasiak wrote: > I found out that kernel returns -ENODEV error which when file system > type has not been found. I look into code of ffs and found out that > ffs_fs_mount() also returns this error if it was unable to acquire given > device. I know that this error looks really suitable in this place but > it really hurts discoverability in userspace because you don't know > where did you make your typo in file system name or in dev name. I have > run through few other fs implementations and found out that they all > return -ENOENT if given device has not been found. > > Maybe FFS should follow this convention Probably, patches welcome. :) -- Best regards, _ _ .o. | Liege of Serenely Enlightened Majesty of o' \,=./ `o ..o | Computer Science, Michał “mina86” Nazarewicz(o o) ooo +--ooO--(_)--Ooo-- signature.asc Description: PGP signature
Re: [PATCH 02/10] xhci: 'noxhci_port_switch' kernel parameter
On Tue, May 20, 2014 at 2:51 AM, Takashi Iwai wrote: > At Tue, 20 May 2014 12:47:36 +0300, > Mathias Nyman wrote: >> >> On 05/20/2014 04:01 AM, Greg KH wrote: >> > On Thu, May 08, 2014 at 07:25:55PM +0300, Mathias Nyman wrote: >> >> From: Dan Williams >> >> >> >> Add a command line switch for disabling ehci port switchover. Useful >> >> for working around / debugging xhci incompatibilities where ehci >> >> operation is available. >> >> >> >> Reference: http://marc.info/?l=linux-usb&m=138920063001509&w=2 >> >> >> >> Cc: Sarah Sharp >> >> Cc: Mathias Nyman >> >> Cc: Holger Hans Peter Freyther >> >> Suggested-by: Alan Stern >> >> Signed-off-by: Dan Williams >> >> Signed-off-by: Mathias Nyman >> >> --- >> >> Documentation/kernel-parameters.txt | 3 +++ >> >> drivers/usb/host/pci-quirks.c | 15 +-- >> >> 2 files changed, 16 insertions(+), 2 deletions(-) >> >> >> >> diff --git a/Documentation/kernel-parameters.txt >> >> b/Documentation/kernel-parameters.txt >> >> index 4384217..fc3403114 100644 >> >> --- a/Documentation/kernel-parameters.txt >> >> +++ b/Documentation/kernel-parameters.txt >> >> @@ -2251,6 +2251,9 @@ bytes respectively. Such letter suffixes can also >> >> be entirely omitted. >> >> >> >>nox2apic[X86-64,APIC] Do not enable x2APIC mode. >> >> >> >> + noxhci_port_switch >> >> + [USB] Use EHCI instead of XHCI where available >> >> + >> > >> > Ugh, I really don't like new command line options. >> > >> > Especially one that isn't very well documented. Why would someone want >> > to enable this? What problem is it solving? Can we detect this >> > automatically and do it for the user? Is this just for prototype >> > hardware that has not shipped? What hardware needs this? >> > >> > I need a whole lot more documentation at the very least before I will >> > apply this. >> > >> >> On Intel hardware with both ehci and xhci controllers we can select if a >> usb2 port >> is controlled by ehci or xhci. This capability can be checked from Intel >> xhci pci >> config space. Xhci driver checks this on boot and switches over the >> supported ports. >> >> This is a feature in Intel Panther point and later chipsets, in shipped >> hardware. >> Its working quite well in most cases, but sometimes vendors claim they >> support >> switchover, but then forget to connect some wires, and the usb2 port ends up >> dead >> after switching. >> >> A recently found case is the Sony VAIO T-series. (I'll send you a different >> patch >> for that shortly) >> http://marc.info/?l=linux-usb&m=139993106029340&w=2 >> >> This is the extreme case that the usb2 ports appears completely dead. >> Other reasons are that some devices might work better under ehci than xhci, >> and users want to enforce the ehci opton. For powermanagement developers >> it's nice >> to disable switchover as it turns out some hardware are quirky with port >> switchover and suspend/resume. (might need to turn port back to ehci before >> suspending). >> >> I don't think we can detect this automatically. >> >> Dan, can you add more documentation to this patch? > > While we're at it: can we implement a bitmask instead? > > We've seen lots of HP laptops having Webcams or BT devices that don't > work XHCI but only with EHCI. For making them working properly, the > specific xhci ports have to be disabled. But, we don't want to kill > the whole XHCI at all. The single boolean option doesn't work for > such a case. > I'm not sure we want to make this more complex. Ideally this is just a stop-gap measure for users to workaround incompatibilities in xhci while the xhci fix is identified. The fact that it is a coarse hammer at least, in my mind, keeps more pressure on identifying the necessary xhci quirk/fix to make it as functional as ehci. We need that fix anyways for platforms with xhci ports without an ehci companion, so what purpose is served by letting users avoid xhci with finer granularity? -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/10] xhci: 'noxhci_port_switch' kernel parameter
At Tue, 20 May 2014 11:25:37 -0700, Dan Williams wrote: > > On Tue, May 20, 2014 at 2:51 AM, Takashi Iwai wrote: > > At Tue, 20 May 2014 12:47:36 +0300, > > Mathias Nyman wrote: > >> > >> On 05/20/2014 04:01 AM, Greg KH wrote: > >> > On Thu, May 08, 2014 at 07:25:55PM +0300, Mathias Nyman wrote: > >> >> From: Dan Williams > >> >> > >> >> Add a command line switch for disabling ehci port switchover. Useful > >> >> for working around / debugging xhci incompatibilities where ehci > >> >> operation is available. > >> >> > >> >> Reference: http://marc.info/?l=linux-usb&m=138920063001509&w=2 > >> >> > >> >> Cc: Sarah Sharp > >> >> Cc: Mathias Nyman > >> >> Cc: Holger Hans Peter Freyther > >> >> Suggested-by: Alan Stern > >> >> Signed-off-by: Dan Williams > >> >> Signed-off-by: Mathias Nyman > >> >> --- > >> >> Documentation/kernel-parameters.txt | 3 +++ > >> >> drivers/usb/host/pci-quirks.c | 15 +-- > >> >> 2 files changed, 16 insertions(+), 2 deletions(-) > >> >> > >> >> diff --git a/Documentation/kernel-parameters.txt > >> >> b/Documentation/kernel-parameters.txt > >> >> index 4384217..fc3403114 100644 > >> >> --- a/Documentation/kernel-parameters.txt > >> >> +++ b/Documentation/kernel-parameters.txt > >> >> @@ -2251,6 +2251,9 @@ bytes respectively. Such letter suffixes can also > >> >> be entirely omitted. > >> >> > >> >>nox2apic[X86-64,APIC] Do not enable x2APIC mode. > >> >> > >> >> + noxhci_port_switch > >> >> + [USB] Use EHCI instead of XHCI where available > >> >> + > >> > > >> > Ugh, I really don't like new command line options. > >> > > >> > Especially one that isn't very well documented. Why would someone want > >> > to enable this? What problem is it solving? Can we detect this > >> > automatically and do it for the user? Is this just for prototype > >> > hardware that has not shipped? What hardware needs this? > >> > > >> > I need a whole lot more documentation at the very least before I will > >> > apply this. > >> > > >> > >> On Intel hardware with both ehci and xhci controllers we can select if a > >> usb2 port > >> is controlled by ehci or xhci. This capability can be checked from Intel > >> xhci pci > >> config space. Xhci driver checks this on boot and switches over the > >> supported ports. > >> > >> This is a feature in Intel Panther point and later chipsets, in shipped > >> hardware. > >> Its working quite well in most cases, but sometimes vendors claim they > >> support > >> switchover, but then forget to connect some wires, and the usb2 port ends > >> up dead > >> after switching. > >> > >> A recently found case is the Sony VAIO T-series. (I'll send you a > >> different patch > >> for that shortly) > >> http://marc.info/?l=linux-usb&m=139993106029340&w=2 > >> > >> This is the extreme case that the usb2 ports appears completely dead. > >> Other reasons are that some devices might work better under ehci than xhci, > >> and users want to enforce the ehci opton. For powermanagement developers > >> it's nice > >> to disable switchover as it turns out some hardware are quirky with port > >> switchover and suspend/resume. (might need to turn port back to ehci before > >> suspending). > >> > >> I don't think we can detect this automatically. > >> > >> Dan, can you add more documentation to this patch? > > > > While we're at it: can we implement a bitmask instead? > > > > We've seen lots of HP laptops having Webcams or BT devices that don't > > work XHCI but only with EHCI. For making them working properly, the > > specific xhci ports have to be disabled. But, we don't want to kill > > the whole XHCI at all. The single boolean option doesn't work for > > such a case. > > > > I'm not sure we want to make this more complex. Ideally this is just > a stop-gap measure for users to workaround incompatibilities in xhci > while the xhci fix is identified. The fact that it is a coarse hammer > at least, in my mind, keeps more pressure on identifying the necessary > xhci quirk/fix to make it as functional as ehci. We need that fix > anyways for platforms with xhci ports without an ehci companion, so > what purpose is served by letting users avoid xhci with finer > granularity? There are cases where only certain ports suffer from the problem of XHCI. The finer granularity allows user to identify the affected ports, and at least, more-or-less working state. But the main purpose of this option is to give the pressure to XHCI developers, then I agree with it. But, in that case, it must be documented properly. Namely, this option is only for testing/debugging, and shouldn't be applied to any running system for real use as a workaround. Takashi -- 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 4/5] appledisplay: Convert /n to \n
On Thu, 2014-04-24 at 18:51 -0700, Joe Perches wrote: > Use a newline character appropriately. > > Signed-off-by: Joe Perches > --- Greg? Ping? You inadvertently added this "/n" use in commit 0a3fd536e685e0ceb646de1a43821bd11c0be75c Author: Greg Kroah-Hartman Date: Tue May 1 21:33:54 2012 -0700 USB: appledisplay.c: remove dbg() usage dbg() was a very old USB-specific macro that should no longer be used. This patch removes it from being used in the driver and uses dev_dbg() instead. > drivers/usb/misc/appledisplay.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/misc/appledisplay.c b/drivers/usb/misc/appledisplay.c > index ba6a5d6..f37c78d 100644 > --- a/drivers/usb/misc/appledisplay.c > +++ b/drivers/usb/misc/appledisplay.c > @@ -110,7 +110,7 @@ static void appledisplay_complete(struct urb *urb) > __func__, status); > return; > default: > - dev_dbg(dev, "%s - nonzero urb status received: %d/n", > + dev_dbg(dev, "%s - nonzero urb status received: %d\n", > __func__, status); > goto exit; > } -- 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: ehci: Enable support for 64bit EHCI host controllers in arm64
On Tuesday 20 May 2014 15:12:49 Catalin Marinas wrote: > On Tue, May 20, 2014 at 12:08:58PM +0100, Arnd Bergmann wrote: > > On Tuesday 20 May 2014 12:02:46 Catalin Marinas wrote: > > > On Mon, May 19, 2014 at 05:55:56PM +0100, Arnd Bergmann wrote: > > > > On Monday 19 May 2014 16:56:08 Catalin Marinas wrote: > > > > > On Mon, May 19, 2014 at 10:44:51AM +0100, Arnd Bergmann wrote: > > > > > > On Monday 19 May 2014 10:03:40 Catalin Marinas wrote: > > > > We probably want to default to 32-bit for arm32 in the absence of > > > > dma-ranges. > > > > For arm64, I'd prefer if we could always mandate dma-ranges to be > > > > present > > > > for each bus, just like we mandate ranges to be present. > > > > I hope it's not too late for that. > > > > > > > > dma_set_mask should definitely look at the dma-ranges properties, and > > > > the > > > > helper that Santosh just introduced should give us all the information > > > > we need. We just need to decide on the correct behavior. > > > > > > Last time I looked at Santosh's patches I thought the dma-ranges is per > > > device rather than per bus. We could make it per bus only and let the > > > device call dma_set_mask() explicitly if it wants to restrict it > > > further. > > > > Can you check again? I've read the code again yesterday to check this, > > and I concluded it was correctly doing this per bus. > > You are right, I missed the fact that of_dma_get_range() checks the > dma-ranges property in the node's parent. > > So what we need is setting the default dma mask based on the size > information in dma-ranges to something like this: > > mask = rounddown_pow_of_two(size) - 1; > dma_set_mask(dev, mask);/* or dma_set_mask_and_coherent() */ I don't think we should be calling dma_set_mask here, since that would just go and parse the same property again once we fix the implementation. Since this is a low-level helper, we can probably just assign the dma mask directly. > > > > > > While we currently don't have a set of swiotlb DMA ops on ARM32, we > > > > > > do > > > > > > have it on ARM64, and I think we should be using them properly. It > > > > > > should > > > > > > really not be hard to implement a proper dma_set_mask() function for > > > > > > ARM64 that gets is able to set up the swiotlb based on the > > > > > > dma-ranges > > > > > > properties and always returns success but leaves the mask unchanged. > > > > > > > > > > The swiotlb bounce buffer needs to be pre-allocated at boot, otherwise > > > > > we don't have any guarantees. Since we can't honour random masks > > > > > anyway, > > > > > we stick to ZONE_DMA which is currently in the 4G limit. But the > > > > > driver > > > > > calls dma_set_mask() too late for any further swiotlb setup. > > > > > > > > > > With IOMMU we can be more flexible around dma_set_mask(), can be done > > > > > at > > > > > run-time. > > > > > > > > What we can do with swiotlb is to check if the mask is smaller than > > > > ZONE_DMA. > > > > If it ever is, we have to fail dma_set_mask and hope the driver can fall > > > > back to PIO mode or it will have to fail its probe() function. > > > > > > dma_set_(coherent_)mask check swiotlb_dma_supported() which returns > > > false if io_tlb_end goes beyond the device mask. So we just need to > > > ensure that io_tlb is allocated within ZONE_DMA. > > > > Makes sense for dma_set_mask. Why do you do the same thing for > > coherent_mask? Shouldn't that check against ZONE_DMA instead? > > It depends on the meaning of coherent_dma_mask. As I understand it, > that's the dma mask use for dma_alloc_coherent() to return a memory > buffer that the device is able to access. I don't see it much different > from the dma_mask used by the streaming API. I guess some hardware could > have different masks here if they have cache coherency only for certain > memory ranges (and the coherent_dma_mask would be smaller than dma_mask). The point I was trying to make is a different one: For the streaming mapping, you can allow any mask as long as the swiotlb is able to create a bounce buffer. This does not work for the coherent mapping, because that by definition cannot use bounce buffers. > > > > For dma_set_coherent_mask(), we also have to fail any call that tries to > > > > set a mask larger than what the device hardware can do. Unlike that, > > > > dma_set_mask() can succeed with any mask, we just have to enable swiotlb > > > > if the mask that the driver wants is larger than what the hardware can > > > > do. > > > > > > Currently we can't satisfy any arbitrarily small dma mask even with > > > swiotlb since the bounce buffer is just guaranteed to be in ZONE_DMA. > > > Swiotlb allows for smaller masks but we need to reserve the io_tlb > > > buffer early during boot and at smaller addresses. For example, > > > swiotlb_alloc_coherent() first tries __get_free_pages(GFP_DMA) and if > > > the coherent_dma_mask isn't matched, it frees the pages and falls back > > > to the
Re: ffs-test.c Compilation failure
Commit [ac8dde11: “usb: gadget: f_fs: Add flags to descriptors block”]
which introduced a new descriptor format for FunctionFS removed the
usb_functionfs_descs_head structure, which is still used by ffs-test.
tool.
Convert ffs-test by converting it to use the new header format. For
testing kernels prior to 3.14 (when the new format was introduced) and
parsing of the legacy headers in the new kernels, provide a compilation
flag to make the tool use the old format.
Finally, include information as to when the legacy FunctionFS headers
format has been deprecated (which is also when the new one has been
introduced).
Reported-by: Lad, Prabhakar
Signed-off-by: Michal Nazarewicz
---
include/uapi/linux/usb/functionfs.h | 2 +-
tools/usb/Makefile | 6 +-
tools/usb/ffs-test.c| 20 ++--
3 files changed, 24 insertions(+), 4 deletions(-)
Sorry for the delay. This has been compile-tested only since I'm
technically on vacation in the middle of Pacific and won't be able to
properly test this for quite some time.
diff --git a/include/uapi/linux/usb/functionfs.h
b/include/uapi/linux/usb/functionfs.h
index 2a4b4a7..ecb3a31 100644
--- a/include/uapi/linux/usb/functionfs.h
+++ b/include/uapi/linux/usb/functionfs.h
@@ -53,7 +53,7 @@ struct usb_endpoint_descriptor_no_audio {
* structure. Any flags that are not recognised cause the whole block to be
* rejected with -ENOSYS.
*
- * Legacy descriptors format:
+ * Legacy descriptors format (deprecated as of 3.14):
*
* | off | name | type | description |
* |-+---+--+--|
diff --git a/tools/usb/Makefile b/tools/usb/Makefile
index acf2165..d576b3b 100644
--- a/tools/usb/Makefile
+++ b/tools/usb/Makefile
@@ -6,7 +6,11 @@ WARNINGS = -Wall -Wextra
CFLAGS = $(WARNINGS) -g -I../include
LDFLAGS = $(PTHREAD_LIBS)
-all: testusb ffs-test
+all: testusb ffs-test ffs-test-legacy
+
+ffs-test-legacy: ffs-test.c
+ $(CC) $(CFLAGS) -o $@ $^ $(LDFLAGS) -DUSE_LEGACY_DESC_HEAD
+
%: %.c
$(CC) $(CFLAGS) -o $@ $^ $(LDFLAGS)
diff --git a/tools/usb/ffs-test.c b/tools/usb/ffs-test.c
index fe1e66b..74b353d 100644
--- a/tools/usb/ffs-test.c
+++ b/tools/usb/ffs-test.c
@@ -1,5 +1,5 @@
/*
- * ffs-test.c.c -- user mode filesystem api for usb composite function
+ * ffs-test.c -- user mode filesystem api for usb composite function
*
* Copyright (C) 2010 Samsung Electronics
*Author: Michal Nazarewicz
@@ -21,6 +21,8 @@
/* $(CROSS_COMPILE)cc -Wall -Wextra -g -o ffs-test ffs-test.c -lpthread */
+/* Uncomment to make the tool use legacy FFS descriptor headers. */
+/* #define USE_LEGACY_DESC_HEAD */
#define _BSD_SOURCE /* for endian.h */
@@ -106,7 +108,15 @@ static void _msg(unsigned level, const char *fmt, ...)
/ Descriptors and Strings ***/
static const struct {
- struct usb_functionfs_descs_head header;
+ struct {
+ __le32 magic;
+ __le32 length;
+#ifndef USE_LEGACY_DESC_HEAD
+ __le32 flags;
+#endif
+ __le32 fs_count;
+ __le32 hs_count;
+ } __attribute__((packed)) header;
struct {
struct usb_interface_descriptor intf;
struct usb_endpoint_descriptor_no_audio sink;
@@ -114,7 +124,13 @@ static const struct {
} __attribute__((packed)) fs_descs, hs_descs;
} __attribute__((packed)) descriptors = {
.header = {
+#ifdef USE_LEGACY_DESC_HEAD
.magic = cpu_to_le32(FUNCTIONFS_DESCRIPTORS_MAGIC),
+#else
+ .magic = cpu_to_le32(FUNCTIONFS_DESCRIPTORS_MAGIC_V2),
+ .flags = cpu_to_le32(FUNCTIONFS_HAS_FS_DESC |
+FUNCTIONFS_HAS_HS_DESC),
+#endif
.length = cpu_to_le32(sizeof descriptors),
.fs_count = 3,
.hs_count = 3,
--
1.9.1.423.g4596e3a
--
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: ffs-test.c Compilation failure
Hi, On Tue, May 20, 2014 at 10:05:09AM -1000, Michal Nazarewicz wrote: > Commit [ac8dde11: “usb: gadget: f_fs: Add flags to descriptors block”] > which introduced a new descriptor format for FunctionFS removed the > usb_functionfs_descs_head structure, which is still used by ffs-test. > tool. > > Convert ffs-test by converting it to use the new header format. For > testing kernels prior to 3.14 (when the new format was introduced) and > parsing of the legacy headers in the new kernels, provide a compilation > flag to make the tool use the old format. > > Finally, include information as to when the legacy FunctionFS headers > format has been deprecated (which is also when the new one has been > introduced). > > Reported-by: Lad, Prabhakar > Signed-off-by: Michal Nazarewicz can you send as a proper patch with proper subject ? -- balbi signature.asc Description: Digital signature
[PATCH] iowarrior: Convert local dbg macro to dev_dbg
Use a more standard logging style.
Add terminating newlines to formats.
Remove __func__ as that can be added via dynamic debug.
Remove now unnecessary debug module parameter.
Remove the dbg macro too.
Signed-off-by: Joe Perches
---
compiled/untested, no hardware.
drivers/usb/misc/iowarrior.c | 38 +-
1 file changed, 17 insertions(+), 21 deletions(-)
diff --git a/drivers/usb/misc/iowarrior.c b/drivers/usb/misc/iowarrior.c
index 20bcfdd..c6bfd13 100644
--- a/drivers/usb/misc/iowarrior.c
+++ b/drivers/usb/misc/iowarrior.c
@@ -51,19 +51,12 @@
*/
#define MAX_WRITES_IN_FLIGHT 4
-/* Use our own dbg macro */
-#undef dbg
-#define dbg( format, arg... ) do { if( debug ) printk( KERN_DEBUG __FILE__ ":
" format "\n" , ## arg ); } while ( 0 )
-
MODULE_AUTHOR(DRIVER_AUTHOR);
MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_LICENSE("GPL");
/* Module parameters */
static DEFINE_MUTEX(iowarrior_mutex);
-static bool debug = 0;
-module_param(debug, bool, 0644);
-MODULE_PARM_DESC(debug, "debug=1 enables debugging messages");
static struct usb_driver iowarrior_driver;
static DEFINE_MUTEX(iowarrior_open_disc_lock);
@@ -235,8 +228,8 @@ static void iowarrior_write_callback(struct urb *urb)
if (status &&
!(status == -ENOENT ||
status == -ECONNRESET || status == -ESHUTDOWN)) {
- dbg("%s - nonzero write bulk status received: %d",
- __func__, status);
+ dev_dbg(&dev->interface->dev,
+ "nonzero write bulk status received: %d\n", status);
}
/* free up our allocated buffer */
usb_free_coherent(urb->dev, urb->transfer_buffer_length,
@@ -251,7 +244,7 @@ static void iowarrior_write_callback(struct urb *urb)
*/
static inline void iowarrior_delete(struct iowarrior *dev)
{
- dbg("%s - minor %d", __func__, dev->minor);
+ dev_dbg(&dev->interface->dev, "minor %d\n", dev->minor);
kfree(dev->int_in_buffer);
usb_free_urb(dev->int_in_urb);
kfree(dev->read_queue);
@@ -288,7 +281,8 @@ static ssize_t iowarrior_read(struct file *file, char
__user *buffer,
if (dev == NULL || !dev->present)
return -ENODEV;
- dbg("%s - minor %d, count = %zd", __func__, dev->minor, count);
+ dev_dbg(&dev->interface->dev, "minor %d, count = %zd\n",
+ dev->minor, count);
/* read count must be packet size (+ time stamp) */
if ((count != dev->report_size)
@@ -356,7 +350,8 @@ static ssize_t iowarrior_write(struct file *file,
retval = -ENODEV;
goto exit;
}
- dbg("%s - minor %d, count = %zd", __func__, dev->minor, count);
+ dev_dbg(&dev->interface->dev, "minor %d, count = %zd\n",
+ dev->minor, count);
/* if count is 0 we're already done */
if (count == 0) {
retval = 0;
@@ -418,14 +413,16 @@ static ssize_t iowarrior_write(struct file *file,
int_out_urb = usb_alloc_urb(0, GFP_KERNEL);
if (!int_out_urb) {
retval = -ENOMEM;
- dbg("%s Unable to allocate urb ", __func__);
+ dev_dbg(&dev->interface->dev,
+ "Unable to allocate urb\n");
goto error_no_urb;
}
buf = usb_alloc_coherent(dev->udev, dev->report_size,
GFP_KERNEL,
&int_out_urb->transfer_dma);
if (!buf) {
retval = -ENOMEM;
- dbg("%s Unable to allocate buffer ", __func__);
+ dev_dbg(&dev->interface->dev,
+ "Unable to allocate buffer\n");
goto error_no_buffer;
}
usb_fill_int_urb(int_out_urb, dev->udev,
@@ -441,8 +438,9 @@ static ssize_t iowarrior_write(struct file *file,
}
retval = usb_submit_urb(int_out_urb, GFP_KERNEL);
if (retval) {
- dbg("%s submit error %d for urb nr.%d", __func__,
- retval, atomic_read(&dev->write_busy));
+ dev_dbg(&dev->interface->dev,
+ "submit error %d for urb nr.%d\n",
+ retval, atomic_read(&dev->write_busy));
goto error;
}
/* submit was ok */
@@ -502,8 +500,8 @@ static long iowarrior_ioctl(struct file *file, unsigned int
cmd,
goto error_out;
}
- dbg("%s - minor %d, cmd 0x%.4x, arg %ld", __func__, dev->minor, cmd,
- arg);
+ dev_dbg(&dev->interface->dev, "minor %d, cmd 0x%.4x, arg %ld\n",
+ dev->minor, cmd, arg);
retval = 0;
io_res = 0;
@@ -601,8 +599,6 @@ static int iowarrior_open(struct inode *inode, struct file
*file)
int
Re: [PATCH 4/5] appledisplay: Convert /n to \n
On Tue, May 20, 2014 at 12:26:53PM -0700, Joe Perches wrote: > On Thu, 2014-04-24 at 18:51 -0700, Joe Perches wrote: > > Use a newline character appropriately. > > > > Signed-off-by: Joe Perches > > --- > > Greg? Ping? > > You inadvertently added this "/n" use in > commit 0a3fd536e685e0ceb646de1a43821bd11c0be75c > Author: Greg Kroah-Hartman > Date: Tue May 1 21:33:54 2012 -0700 > > USB: appledisplay.c: remove dbg() usage > > dbg() was a very old USB-specific macro that should no longer > be used. This patch removes it from being used in the driver > and uses dev_dbg() instead. Sorry, it's in my queue, just stalled due to traveling. It's not lost, don't worry. 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: new driver Fresco Logic
On Tue, May 20, 2014 at 08:07:43AM -0700, Michael Durkin wrote: > Forgive my intrusion ... > > Im hoping to get a driver made for a Fresco Logic FL2000 USB to VGA adapter > ... > > Device ID 1d5c:2000 What type of chip is in this device? If it is a USB 3 device, odds are that Linux doesn't support it yet as DeviceLink isn't providing the spec for the chip to anyone. Can you ask the company you bought the device from for how to get the specs for it? 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 02/10] xhci: 'noxhci_port_switch' kernel parameter
On Tue, May 20, 2014 at 11:25:37AM -0700, Dan Williams wrote: > On Tue, May 20, 2014 at 2:51 AM, Takashi Iwai wrote: > > At Tue, 20 May 2014 12:47:36 +0300, > > Mathias Nyman wrote: > >> > >> On 05/20/2014 04:01 AM, Greg KH wrote: > >> > On Thu, May 08, 2014 at 07:25:55PM +0300, Mathias Nyman wrote: > >> >> From: Dan Williams > >> >> > >> >> Add a command line switch for disabling ehci port switchover. Useful > >> >> for working around / debugging xhci incompatibilities where ehci > >> >> operation is available. > >> >> > >> >> Reference: http://marc.info/?l=linux-usb&m=138920063001509&w=2 > >> >> > >> >> Cc: Sarah Sharp > >> >> Cc: Mathias Nyman > >> >> Cc: Holger Hans Peter Freyther > >> >> Suggested-by: Alan Stern > >> >> Signed-off-by: Dan Williams > >> >> Signed-off-by: Mathias Nyman > >> >> --- > >> >> Documentation/kernel-parameters.txt | 3 +++ > >> >> drivers/usb/host/pci-quirks.c | 15 +-- > >> >> 2 files changed, 16 insertions(+), 2 deletions(-) > >> >> > >> >> diff --git a/Documentation/kernel-parameters.txt > >> >> b/Documentation/kernel-parameters.txt > >> >> index 4384217..fc3403114 100644 > >> >> --- a/Documentation/kernel-parameters.txt > >> >> +++ b/Documentation/kernel-parameters.txt > >> >> @@ -2251,6 +2251,9 @@ bytes respectively. Such letter suffixes can also > >> >> be entirely omitted. > >> >> > >> >>nox2apic[X86-64,APIC] Do not enable x2APIC mode. > >> >> > >> >> + noxhci_port_switch > >> >> + [USB] Use EHCI instead of XHCI where available > >> >> + > >> > > >> > Ugh, I really don't like new command line options. > >> > > >> > Especially one that isn't very well documented. Why would someone want > >> > to enable this? What problem is it solving? Can we detect this > >> > automatically and do it for the user? Is this just for prototype > >> > hardware that has not shipped? What hardware needs this? > >> > > >> > I need a whole lot more documentation at the very least before I will > >> > apply this. > >> > > >> > >> On Intel hardware with both ehci and xhci controllers we can select if a > >> usb2 port > >> is controlled by ehci or xhci. This capability can be checked from Intel > >> xhci pci > >> config space. Xhci driver checks this on boot and switches over the > >> supported ports. > >> > >> This is a feature in Intel Panther point and later chipsets, in shipped > >> hardware. > >> Its working quite well in most cases, but sometimes vendors claim they > >> support > >> switchover, but then forget to connect some wires, and the usb2 port ends > >> up dead > >> after switching. > >> > >> A recently found case is the Sony VAIO T-series. (I'll send you a > >> different patch > >> for that shortly) > >> http://marc.info/?l=linux-usb&m=139993106029340&w=2 > >> > >> This is the extreme case that the usb2 ports appears completely dead. > >> Other reasons are that some devices might work better under ehci than xhci, > >> and users want to enforce the ehci opton. For powermanagement developers > >> it's nice > >> to disable switchover as it turns out some hardware are quirky with port > >> switchover and suspend/resume. (might need to turn port back to ehci before > >> suspending). > >> > >> I don't think we can detect this automatically. > >> > >> Dan, can you add more documentation to this patch? > > > > While we're at it: can we implement a bitmask instead? > > > > We've seen lots of HP laptops having Webcams or BT devices that don't > > work XHCI but only with EHCI. For making them working properly, the > > specific xhci ports have to be disabled. But, we don't want to kill > > the whole XHCI at all. The single boolean option doesn't work for > > such a case. > > > > I'm not sure we want to make this more complex. Ideally this is just > a stop-gap measure for users to workaround incompatibilities in xhci > while the xhci fix is identified. We can't use a kernel command line option as a "stop-gap", sorry. Let's just fix the real problem here. 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] usb: usb5303: add support for reference clock specified in device tree
On Tue, May 20, 2014 at 02:18:30PM +0200, Marek Szyprowski wrote: > On 2014-05-20 13:57, Mark Brown wrote: > >> + else > >> + dev_err(dev, > >> + "unsupported reference clock rate > >> (%d)\n", > >> + rate); > >This looks like a switch statement. Should the driver not try to set > >the clock to a supported rate if it's not already at one rather than > >error out - it seems like a more constructive thing to do? > Hmm, the problem here is that you cannot determine the right rate. The rate > is > encoded on REF_SEL[1:0] pins (usually soldered to some resistors) and cannot > be > read via i2c commands. To set add support for rate setting, I would need to > add > one more property with correct ref clock rate. Is this okay? Sounds reasonable to me, yes. signature.asc Description: Digital signature
Re: new driver Fresco Logic
It can be used on USB3 or USB2 .. depending on max resolution ... I would be happy with USB2 800X600 resolution ... I got the device off of ebay ... but i could send an email to Fresco Logic (the maker of the IC) or to Orinco (who sells retail devices) I have already sent an email to the seller whom recanted on "open-source drivers with Linux" in the auction ... The print on the chip is FL2000 Fresco Logic ... and is also what is seen in win7 On Tue, May 20, 2014 at 1:31 PM, Greg KH wrote: > On Tue, May 20, 2014 at 08:07:43AM -0700, Michael Durkin wrote: >> Forgive my intrusion ... >> >> Im hoping to get a driver made for a Fresco Logic FL2000 USB to VGA adapter >> ... >> >> Device ID 1d5c:2000 > > What type of chip is in this device? If it is a USB 3 device, odds are > that Linux doesn't support it yet as DeviceLink isn't providing the spec > for the chip to anyone. Can you ask the company you bought the device > from for how to get the specs for it? > > 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 02/10] xhci: 'noxhci_port_switch' kernel parameter
On Tue, May 20, 2014 at 1:34 PM, Greg KH wrote: > On Tue, May 20, 2014 at 11:25:37AM -0700, Dan Williams wrote: >> On Tue, May 20, 2014 at 2:51 AM, Takashi Iwai wrote: >> > At Tue, 20 May 2014 12:47:36 +0300, >> > Mathias Nyman wrote: >> >> >> >> On 05/20/2014 04:01 AM, Greg KH wrote: >> >> > On Thu, May 08, 2014 at 07:25:55PM +0300, Mathias Nyman wrote: >> >> >> From: Dan Williams >> >> >> >> >> >> Add a command line switch for disabling ehci port switchover. Useful >> >> >> for working around / debugging xhci incompatibilities where ehci >> >> >> operation is available. >> >> >> >> >> >> Reference: http://marc.info/?l=linux-usb&m=138920063001509&w=2 >> >> >> >> >> >> Cc: Sarah Sharp >> >> >> Cc: Mathias Nyman >> >> >> Cc: Holger Hans Peter Freyther >> >> >> Suggested-by: Alan Stern >> >> >> Signed-off-by: Dan Williams >> >> >> Signed-off-by: Mathias Nyman >> >> >> --- >> >> >> Documentation/kernel-parameters.txt | 3 +++ >> >> >> drivers/usb/host/pci-quirks.c | 15 +-- >> >> >> 2 files changed, 16 insertions(+), 2 deletions(-) >> >> >> >> >> >> diff --git a/Documentation/kernel-parameters.txt >> >> >> b/Documentation/kernel-parameters.txt >> >> >> index 4384217..fc3403114 100644 >> >> >> --- a/Documentation/kernel-parameters.txt >> >> >> +++ b/Documentation/kernel-parameters.txt >> >> >> @@ -2251,6 +2251,9 @@ bytes respectively. Such letter suffixes can >> >> >> also be entirely omitted. >> >> >> >> >> >>nox2apic[X86-64,APIC] Do not enable x2APIC mode. >> >> >> >> >> >> + noxhci_port_switch >> >> >> + [USB] Use EHCI instead of XHCI where available >> >> >> + >> >> > >> >> > Ugh, I really don't like new command line options. >> >> > >> >> > Especially one that isn't very well documented. Why would someone want >> >> > to enable this? What problem is it solving? Can we detect this >> >> > automatically and do it for the user? Is this just for prototype >> >> > hardware that has not shipped? What hardware needs this? >> >> > >> >> > I need a whole lot more documentation at the very least before I will >> >> > apply this. >> >> > >> >> >> >> On Intel hardware with both ehci and xhci controllers we can select if a >> >> usb2 port >> >> is controlled by ehci or xhci. This capability can be checked from Intel >> >> xhci pci >> >> config space. Xhci driver checks this on boot and switches over the >> >> supported ports. >> >> >> >> This is a feature in Intel Panther point and later chipsets, in shipped >> >> hardware. >> >> Its working quite well in most cases, but sometimes vendors claim they >> >> support >> >> switchover, but then forget to connect some wires, and the usb2 port ends >> >> up dead >> >> after switching. >> >> >> >> A recently found case is the Sony VAIO T-series. (I'll send you a >> >> different patch >> >> for that shortly) >> >> http://marc.info/?l=linux-usb&m=139993106029340&w=2 >> >> >> >> This is the extreme case that the usb2 ports appears completely dead. >> >> Other reasons are that some devices might work better under ehci than >> >> xhci, >> >> and users want to enforce the ehci opton. For powermanagement developers >> >> it's nice >> >> to disable switchover as it turns out some hardware are quirky with port >> >> switchover and suspend/resume. (might need to turn port back to ehci >> >> before >> >> suspending). >> >> >> >> I don't think we can detect this automatically. >> >> >> >> Dan, can you add more documentation to this patch? >> > >> > While we're at it: can we implement a bitmask instead? >> > >> > We've seen lots of HP laptops having Webcams or BT devices that don't >> > work XHCI but only with EHCI. For making them working properly, the >> > specific xhci ports have to be disabled. But, we don't want to kill >> > the whole XHCI at all. The single boolean option doesn't work for >> > such a case. >> > >> >> I'm not sure we want to make this more complex. Ideally this is just >> a stop-gap measure for users to workaround incompatibilities in xhci >> while the xhci fix is identified. > > We can't use a kernel command line option as a "stop-gap", sorry. Let's > just fix the real problem here. > Greg, Sorry, I don't think it is fair to users to force them to re-compile their kernel to get their device to work. Granted, I'm new to USB development, but the rate of reports of endpoint devices that mess up and require quirks in the hcd-driver or usb-core seems un-ending to me. So, I don't think it is fair to expect that the tide of quirky devices will be stemmed in any reasonable amount of time. Having a "works with noxhci_port_switch" report from users is good data (hmm, I think a printk to tell users to file a report upstream if the option resolves their issue is needed). Ideally, we could just tell users to blacklist xhci and use ehci while we work on a fix for their problem, but this ehci port-switchover happens well in advance of the xhci driver loading. Ultimately, it's a userspace polic
Re: new driver Fresco Logic
Hi, On Wed, May 21, 2014 at 05:31:13AM +0900, Greg KH wrote: > On Tue, May 20, 2014 at 08:07:43AM -0700, Michael Durkin wrote: > > Forgive my intrusion ... > > > > Im hoping to get a driver made for a Fresco Logic FL2000 USB to VGA > > adapter ... > > > > Device ID 1d5c:2000 > > What type of chip is in this device? If it is a USB 3 device, odds are > that Linux doesn't support it yet as DeviceLink isn't providing the spec DisplayLink ? > for the chip to anyone. Can you ask the company you bought the device > from for how to get the specs for it? AFAICT this is a competing device. From what I could gather, Fresco Logic implemented this device from scratch. The good thing about it, is that it implements USB 3 A/V class, which means all we need is the USB IF A/V Class specification from [1] [1] http://www.usb.org/developers/docs/devclass_docs/USB_AV_Specification_Rev_1.0.zip cheers -- balbi signature.asc Description: Digital signature
Re: new driver Fresco Logic
On Tue, May 20, 2014 at 06:38:01PM -0500, Felipe Balbi wrote: > Hi, > > On Wed, May 21, 2014 at 05:31:13AM +0900, Greg KH wrote: > > On Tue, May 20, 2014 at 08:07:43AM -0700, Michael Durkin wrote: > > > Forgive my intrusion ... > > > > > > Im hoping to get a driver made for a Fresco Logic FL2000 USB to VGA > > > adapter ... > > > > > > Device ID 1d5c:2000 > > > > What type of chip is in this device? If it is a USB 3 device, odds are > > that Linux doesn't support it yet as DeviceLink isn't providing the spec > > DisplayLink ? Yes, sorry :) > > for the chip to anyone. Can you ask the company you bought the device > > from for how to get the specs for it? > > AFAICT this is a competing device. From what I could gather, Fresco > Logic implemented this device from scratch. The good thing about it, is > that it implements USB 3 A/V class, which means all we need is the USB > IF A/V Class specification from [1] > > [1] > http://www.usb.org/developers/docs/devclass_docs/USB_AV_Specification_Rev_1.0.zip Ah, good. I think someone is working on that. Michael, you might want to ask about this on the linux-me...@vger.kernel.org mailing list, the developers there should be able to help you out. 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 v9 09/19] usb: make usb_port flags atomic, rename did_runtime_put to child_usage
On Fri, May 16, 2014 at 11:32 AM, Alan Stern wrote:
> On Wed, 7 May 2014, Dan Williams wrote:
>
>> We want to manipulate ->did_runtime_put in usb_port_runtime_resume(),
>> but we don't want that to collide with other updates. Move usb_port
>> flags to new port-bitmap fields in usb_hub. "did_runtime_put" is renamed
>> "child_usage_bits" to reflect that it is strictly standing in for the
>> fact that usb_devices are not the device_model children of their parent
>> port.
>>
>> Signed-off-by: Dan Williams
>
>
>> @@ -4628,8 +4628,10 @@ static void hub_port_connect_change(struct usb_hub
>> *hub, int port1,
>> spin_lock_irq(&device_state_lock);
>> if (hdev->state == USB_STATE_NOTATTACHED)
>> status = -ENOTCONN;
>> - else
>> + else {
>> port_dev->child = udev;
>> + set_bit(port1, hub->child_usage_bits);
>> + }
>
> Doesn't this line belong in usb_new_device(), next to the
> pm_runtime_get_sync(&port_dev->dev) line? Or maybe does that line
> belong here?
I believe it belongs in usb_new_device() especially because that
routine initializes the runtime pm state of the device. Were the
usb_device a device-model child of its port then usb_new_device()
would already be implicitly modifying the port_dev usage count.
>> spin_unlock_irq(&device_state_lock);
>> mutex_unlock(&usb_port_peer_mutex);
>>
>> diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
>> index 048c797f394c..b3432deb8355 100644
>> --- a/drivers/usb/core/hub.h
>> +++ b/drivers/usb/core/hub.h
>> @@ -51,6 +51,10 @@ struct usb_hub {
>> device present */
>> unsigned long wakeup_bits[1]; /* ports that have signaled
>> remote wakeup */
>> + unsigned long power_bits[1]; /* ports that are powered */
>> + unsigned long child_usage_bits[1]; /* child pm_runtime
>> + active */
>
> I'd say rather /* ports powered on for children */
Done.
> Alan Stern
Thanks!
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 01/19] USB: mutual exclusion for resetting a hub and power-managing a port
From: Alan Stern
The USB core doesn't properly handle mutual exclusion between
resetting a hub and changing the power states of the hub's ports. We
need to avoid sending port-power requests to the hub while it is being
reset, because such requests cannot succeed.
This patch fixes the problem by keeping track of when a reset is in
progress. At such times, attempts to suspend (power-off) a port will
fail immediately with -EBUSY, and calls to usb_port_runtime_resume()
will update the power_is_on flag and return immediately. When the
reset is complete, hub_activate() will automatically restore each port
to the proper power state.
Signed-off-by: Alan Stern
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.c | 12
drivers/usb/core/hub.h |1 +
drivers/usb/core/port.c |6 ++
3 files changed, 19 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 090469ebfcff..e3bc03ee0928 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1276,12 +1276,22 @@ static void hub_quiesce(struct usb_hub *hub, enum
hub_quiescing_type type)
flush_work(&hub->tt.clear_work);
}
+static void hub_pm_barrier_for_all_ports(struct usb_hub *hub)
+{
+ int i;
+
+ for (i = 0; i < hub->hdev->maxchild; ++i)
+ pm_runtime_barrier(&hub->ports[i]->dev);
+}
+
/* caller has locked the hub device */
static int hub_pre_reset(struct usb_interface *intf)
{
struct usb_hub *hub = usb_get_intfdata(intf);
hub_quiesce(hub, HUB_PRE_RESET);
+ hub->in_reset = 1;
+ hub_pm_barrier_for_all_ports(hub);
return 0;
}
@@ -1290,6 +1300,8 @@ static int hub_post_reset(struct usb_interface *intf)
{
struct usb_hub *hub = usb_get_intfdata(intf);
+ hub->in_reset = 0;
+ hub_pm_barrier_for_all_ports(hub);
hub_activate(hub, HUB_POST_RESET);
return 0;
}
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 33bcb2c6f90a..f9b521e60128 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -66,6 +66,7 @@ struct usb_hub {
unsignedlimited_power:1;
unsignedquiescing:1;
unsigneddisconnected:1;
+ unsignedin_reset:1;
unsignedquirk_check_port_auto_suspend:1;
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 51542f852393..37647e080599 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -81,6 +81,10 @@ static int usb_port_runtime_resume(struct device *dev)
if (!hub)
return -EINVAL;
+ if (hub->in_reset) {
+ port_dev->power_is_on = 1;
+ return 0;
+ }
usb_autopm_get_interface(intf);
set_bit(port1, hub->busy_bits);
@@ -117,6 +121,8 @@ static int usb_port_runtime_suspend(struct device *dev)
if (!hub)
return -EINVAL;
+ if (hub->in_reset)
+ return -EBUSY;
if (dev_pm_qos_flags(&port_dev->dev, PM_QOS_FLAG_NO_POWER_OFF)
== PM_QOS_FLAGS_ALL)
--
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 v10 02/19] usb: disable port power control if not supported in wHubCharacteristics
A hub indicates whether it supports per-port power control via the
wHubCharacteristics field in its descriptor. If it is not supported
a hub will still emulate ClearPortPower(PORT_POWER) requests by
stopping the link state machine. However, since this does not save
power do not bother suspending.
This also consolidates support checks into a
hub_is_port_power_switchable() helper.
Acked-by: Alan Stern
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.c |8 ++--
drivers/usb/core/hub.h | 10 ++
drivers/usb/core/port.c | 13 -
3 files changed, 20 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index e3bc03ee0928..97e15ca3e21a 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -818,8 +818,6 @@ static unsigned hub_power_on(struct usb_hub *hub, bool
do_delay)
int port1;
unsigned pgood_delay = hub->descriptor->bPwrOn2PwrGood * 2;
unsigned delay;
- u16 wHubCharacteristics =
- le16_to_cpu(hub->descriptor->wHubCharacteristics);
/* Enable power on each port. Some hubs have reserved values
* of LPSM (> 2) in their descriptors, even though they are
@@ -827,7 +825,7 @@ static unsigned hub_power_on(struct usb_hub *hub, bool
do_delay)
* but only emulate it. In all cases, the ports won't work
* unless we send these messages to the hub.
*/
- if ((wHubCharacteristics & HUB_CHAR_LPSM) < 2)
+ if (hub_is_port_power_switchable(hub))
dev_dbg(hub->intfdev, "enabling power on all ports\n");
else
dev_dbg(hub->intfdev, "trying to enable port power on "
@@ -4419,8 +4417,6 @@ static void hub_port_connect_change(struct usb_hub *hub,
int port1,
struct usb_device *hdev = hub->hdev;
struct device *hub_dev = hub->intfdev;
struct usb_hcd *hcd = bus_to_hcd(hdev->bus);
- unsigned wHubCharacteristics =
- le16_to_cpu(hub->descriptor->wHubCharacteristics);
struct usb_device *udev;
int status, i;
unsigned unit_load;
@@ -4505,7 +4501,7 @@ static void hub_port_connect_change(struct usb_hub *hub,
int port1,
test_bit(port1, hub->removed_bits)) {
/* maybe switch power back on (e.g. root hub was reset) */
- if ((wHubCharacteristics & HUB_CHAR_LPSM) < 2
+ if (hub_is_port_power_switchable(hub)
&& !port_is_power_on(hub, portstatus))
set_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index f9b521e60128..4bd72dd303d5 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -112,6 +112,16 @@ extern int hub_port_debounce(struct usb_hub *hub, int
port1,
extern int usb_clear_port_feature(struct usb_device *hdev,
int port1, int feature);
+static inline bool hub_is_port_power_switchable(struct usb_hub *hub)
+{
+ __le16 hcs;
+
+ if (!hub)
+ return false;
+ hcs = hub->descriptor->wHubCharacteristics;
+ return (le16_to_cpu(hcs) & HUB_CHAR_LPSM) < HUB_CHAR_NO_LPSM;
+}
+
static inline int hub_port_debounce_be_connected(struct usb_hub *hub,
int port1)
{
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 37647e080599..168fa6ee3348 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -177,12 +177,15 @@ int usb_hub_create_port_device(struct usb_hub *hub, int
port1)
pm_runtime_set_active(&port_dev->dev);
- /* It would be dangerous if user space couldn't
-* prevent usb device from being powered off. So don't
-* enable port runtime pm if failed to expose port's pm qos.
+ /*
+* Do not enable port runtime pm if the hub does not support
+* power switching. Also, userspace must have final say of
+* whether a port is permitted to power-off. Do not enable
+* runtime pm if we fail to expose pm_qos_no_power_off.
*/
- if (!dev_pm_qos_expose_flags(&port_dev->dev,
- PM_QOS_FLAG_NO_POWER_OFF))
+ if (hub_is_port_power_switchable(hub)
+ && dev_pm_qos_expose_flags(&port_dev->dev,
+ PM_QOS_FLAG_NO_POWER_OFF) == 0)
pm_runtime_enable(&port_dev->dev);
device_enable_async_suspend(&port_dev->dev);
--
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 v10 05/19] usb: assign default peer ports for root hubs
Assume that the peer of a superspeed port is the port with the same id
on the shared_hcd root hub. This identification scheme is required of
external hubs by the USB3 spec [1]. However, for root hubs, tier mismatch
may be in effect [2]. Tier mismatch can only be enumerated via platform
firmware. For now, simply perform the nominal association.
A new lock 'usb_port_peer_mutex' is introduced to synchronize port
device add/remove with peer lookups. It protects peering against
changes to hcd->shared_hcd, hcd->self.root_hub, hdev->maxchild, and
port_dev->child pointers.
[1]: usb 3.1 section 10.3.3
[2]: xhci 1.1 appendix D
Cc: Alan Stern
[alan: usb_port_peer_mutex locking scheme]
Acked-by: Alan Stern
Signed-off-by: Dan Williams
---
drivers/usb/core/hcd.c | 43 +++-
drivers/usb/core/hub.c | 42 ++-
drivers/usb/core/hub.h |2 +
drivers/usb/core/port.c | 73 ---
drivers/usb/core/usb.h |1 +
5 files changed, 134 insertions(+), 27 deletions(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index adddc66c9e8d..0fcf6bc0a4ef 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2457,11 +2457,13 @@ struct usb_hcd *usb_create_shared_hcd(const struct
hc_driver *driver,
mutex_init(hcd->bandwidth_mutex);
dev_set_drvdata(dev, hcd);
} else {
+ mutex_lock(&usb_port_peer_mutex);
hcd->bandwidth_mutex = primary_hcd->bandwidth_mutex;
hcd->primary_hcd = primary_hcd;
primary_hcd->primary_hcd = primary_hcd;
hcd->shared_hcd = primary_hcd;
primary_hcd->shared_hcd = hcd;
+ mutex_unlock(&usb_port_peer_mutex);
}
kref_init(&hcd->kref);
@@ -2513,18 +2515,25 @@ EXPORT_SYMBOL_GPL(usb_create_hcd);
* deallocated.
*
* Make sure to only deallocate the bandwidth_mutex when the primary HCD is
- * freed. When hcd_release() is called for the non-primary HCD, set the
- * primary_hcd's shared_hcd pointer to null (since the non-primary HCD will be
- * freed shortly).
+ * freed. When hcd_release() is called for either hcd in a peer set
+ * invalidate the peer's ->shared_hcd and ->primary_hcd pointers to
+ * block new peering attempts
*/
-static void hcd_release (struct kref *kref)
+static void hcd_release(struct kref *kref)
{
struct usb_hcd *hcd = container_of (kref, struct usb_hcd, kref);
+ mutex_lock(&usb_port_peer_mutex);
if (usb_hcd_is_primary_hcd(hcd))
kfree(hcd->bandwidth_mutex);
- else
- hcd->shared_hcd->shared_hcd = NULL;
+ if (hcd->shared_hcd) {
+ struct usb_hcd *peer = hcd->shared_hcd;
+
+ peer->shared_hcd = NULL;
+ if (peer->primary_hcd == hcd)
+ peer->primary_hcd = NULL;
+ }
+ mutex_unlock(&usb_port_peer_mutex);
kfree(hcd);
}
@@ -2592,6 +2601,21 @@ static int usb_hcd_request_irqs(struct usb_hcd *hcd,
return 0;
}
+/*
+ * Before we free this root hub, flush in-flight peering attempts
+ * and disable peer lookups
+ */
+static void usb_put_invalidate_rhdev(struct usb_hcd *hcd)
+{
+ struct usb_device *rhdev;
+
+ mutex_lock(&usb_port_peer_mutex);
+ rhdev = hcd->self.root_hub;
+ hcd->self.root_hub = NULL;
+ mutex_unlock(&usb_port_peer_mutex);
+ usb_put_dev(rhdev);
+}
+
/**
* usb_add_hcd - finish generic HCD structure initialization and register
* @hcd: the usb_hcd structure to initialize
@@ -2652,7 +2676,9 @@ int usb_add_hcd(struct usb_hcd *hcd,
retval = -ENOMEM;
goto err_allocate_root_hub;
}
+ mutex_lock(&usb_port_peer_mutex);
hcd->self.root_hub = rhdev;
+ mutex_unlock(&usb_port_peer_mutex);
switch (hcd->speed) {
case HCD_USB11:
@@ -2761,7 +2787,7 @@ err_hcd_driver_start:
err_request_irq:
err_hcd_driver_setup:
err_set_rh_speed:
- usb_put_dev(hcd->self.root_hub);
+ usb_put_invalidate_rhdev(hcd);
err_allocate_root_hub:
usb_deregister_bus(&hcd->self);
err_register_bus:
@@ -2841,7 +2867,6 @@ void usb_remove_hcd(struct usb_hcd *hcd)
free_irq(hcd->irq, hcd);
}
- usb_put_dev(hcd->self.root_hub);
usb_deregister_bus(&hcd->self);
hcd_buffer_destroy(hcd);
if (hcd->remove_phy && hcd->phy) {
@@ -2849,6 +2874,8 @@ void usb_remove_hcd(struct usb_hcd *hcd)
usb_put_phy(hcd->phy);
hcd->phy = NULL;
}
+
+ usb_put_invalidate_rhdev(hcd);
}
EXPORT_SYMBOL_GPL(usb_remove_hcd);
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index b387ebfe25bc..7050560e0f9b 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -55,6 +55,9 @@ static DECLARE_WAIT_QUEUE_HEAD(khubd_wait);
static struct task_struct *khubd_task;
[PATCH v10 03/19] usb: rename usb_port device objects
The current port name "portX" is ambiguous. Before adding more port
messages rename ports to "-portX"
This is an ABI change, but the suspicion is that it will go unnoticed as
the port power control implementation has been broken since its
introduction. If however, someone was relying on the old name we can
add sysfs links from the old name to the new name.
Additionally, it unifies/simplifies port dev_printk messages and modifies
instances of:
dev_XXX(hub->intfdev, ..."port %d"...
dev_XXX(&hdev->dev, ..."port%d"...
into:
dev_XXX(&port_dev->dev, ...
Now that the names are unique usb_port devices it would be nice if they
could be included in /sys/bus/usb. However, it turns out that this
breaks 'lsusb -t'. For now, create a dummy port driver so that print
messages are prefixed "usb 1-1-port3" rather than the
subsystem-ambiguous " 1-1-port3".
Finally, it corrects an odd usage of sscanf("port%d") in usb-acpi.c.
Suggested-by: Alan Stern
Acked-by: Alan Stern
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.c | 273 ++-
drivers/usb/core/port.c | 10 +-
drivers/usb/core/usb-acpi.c | 60 +
drivers/usb/core/usb.h |4 -
4 files changed, 158 insertions(+), 189 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 97e15ca3e21a..143b2e68af02 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -412,30 +412,35 @@ static int set_port_feature(struct usb_device *hdev, int
port1, int feature)
NULL, 0, 1000);
}
+static char *to_led_name(int selector)
+{
+ switch (selector) {
+ case HUB_LED_AMBER:
+ return "amber";
+ case HUB_LED_GREEN:
+ return "green";
+ case HUB_LED_OFF:
+ return "off";
+ case HUB_LED_AUTO:
+ return "auto";
+ default:
+ return "??";
+ }
+}
+
/*
* USB 2.0 spec Section 11.24.2.7.1.10 and table 11-7
* for info about using port indicators
*/
-static void set_port_led(
- struct usb_hub *hub,
- int port1,
- int selector
-)
+static void set_port_led(struct usb_hub *hub, int port1, int selector)
{
- int status = set_port_feature(hub->hdev, (selector << 8) | port1,
+ struct usb_port *port_dev = hub->ports[port1 - 1];
+ int status;
+
+ status = set_port_feature(hub->hdev, (selector << 8) | port1,
USB_PORT_FEAT_INDICATOR);
- if (status < 0)
- dev_dbg (hub->intfdev,
- "port %d indicator %s status %d\n",
- port1,
- ({ char *s; switch (selector) {
- case HUB_LED_AMBER: s = "amber"; break;
- case HUB_LED_GREEN: s = "green"; break;
- case HUB_LED_OFF: s = "off"; break;
- case HUB_LED_AUTO: s = "auto"; break;
- default: s = "??"; break;
- } s; }),
- status);
+ dev_dbg(&port_dev->dev, "indicator %s status %d\n",
+ to_led_name(selector), status);
}
#defineLED_CYCLE_PERIOD((2*HZ)/3)
@@ -909,20 +914,20 @@ static int hub_usb3_port_disable(struct usb_hub *hub, int
port1)
msleep(HUB_DEBOUNCE_STEP);
}
if (total_time >= HUB_DEBOUNCE_TIMEOUT)
- dev_warn(hub->intfdev, "Could not disable port %d after %d
ms\n",
- port1, total_time);
+ dev_warn(&hub->ports[port1 - 1]->dev,
+ "Could not disable after %d ms\n", total_time);
return hub_set_port_link_state(hub, port1, USB_SS_PORT_LS_RX_DETECT);
}
static int hub_port_disable(struct usb_hub *hub, int port1, int set_state)
{
+ struct usb_port *port_dev = hub->ports[port1 - 1];
struct usb_device *hdev = hub->hdev;
int ret = 0;
- if (hub->ports[port1 - 1]->child && set_state)
- usb_set_device_state(hub->ports[port1 - 1]->child,
- USB_STATE_NOTATTACHED);
+ if (port_dev->child && set_state)
+ usb_set_device_state(port_dev->child, USB_STATE_NOTATTACHED);
if (!hub->error) {
if (hub_is_superspeed(hub->hdev))
ret = hub_usb3_port_disable(hub, port1);
@@ -931,8 +936,7 @@ static int hub_port_disable(struct usb_hub *hub, int port1,
int set_state)
USB_PORT_FEAT_ENABLE);
}
if (ret && ret != -ENODEV)
- dev_err(hub->intfdev, "cannot disable port %d (err = %d)\n",
- port1, ret);
+ dev_err(&port_dev->dev, "cannot disable (err = %d)\n", ret);
return ret;
}
@@ -943,7 +947,7 @@ static int hub_port_disable(struct usb_hub *hub, int port1,
int set_state)
*/
static void hub_port_logical_disc
[PATCH v10 09/19] usb: make usb_port flags atomic, rename did_runtime_put to child_usage
We want to manipulate ->did_runtime_put in usb_port_runtime_resume(),
but we don't want that to collide with other updates. Move usb_port
flags to new port-bitmap fields in usb_hub. "did_runtime_put" is renamed
"child_usage_bits" to reflect that it is strictly standing in for the
fact that usb_devices are not the device_model children of their parent
port.
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.c | 39 ---
drivers/usb/core/hub.h |7 +++
drivers/usb/core/port.c |4 ++--
3 files changed, 25 insertions(+), 25 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 7050560e0f9b..3d50f82faea9 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -751,16 +751,20 @@ int usb_hub_set_port_power(struct usb_device *hdev,
struct usb_hub *hub,
int port1, bool set)
{
int ret;
- struct usb_port *port_dev = hub->ports[port1 - 1];
if (set)
ret = set_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
else
ret = usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
- if (!ret)
- port_dev->power_is_on = set;
- return ret;
+ if (ret)
+ return ret;
+
+ if (set)
+ set_bit(port1, hub->power_bits);
+ else
+ clear_bit(port1, hub->power_bits);
+ return 0;
}
/**
@@ -839,7 +843,7 @@ static unsigned hub_power_on(struct usb_hub *hub, bool
do_delay)
dev_dbg(hub->intfdev, "trying to enable port power on "
"non-switchable hub\n");
for (port1 = 1; port1 <= hub->hdev->maxchild; port1++)
- if (hub->ports[port1 - 1]->power_is_on)
+ if (test_bit(port1, hub->power_bits))
set_port_feature(hub->hdev, port1, USB_PORT_FEAT_POWER);
else
usb_clear_port_feature(hub->hdev, port1,
@@ -1180,15 +1184,13 @@ static void hub_activate(struct usb_hub *hub, enum
hub_activation_type type)
set_bit(port1, hub->change_bits);
} else if (udev->persist_enabled) {
- struct usb_port *port_dev = hub->ports[port1 - 1];
-
#ifdef CONFIG_PM
udev->reset_resume = 1;
#endif
/* Don't set the change_bits when the device
* was powered off.
*/
- if (port_dev->power_is_on)
+ if (test_bit(port1, hub->power_bits))
set_bit(port1, hub->change_bits);
} else {
@@ -2096,16 +2098,15 @@ void usb_disconnect(struct usb_device **pdev)
usb_hcd_synchronize_unlinks(udev);
if (udev->parent) {
+ int port1 = udev->portnum;
struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent);
- struct usb_port *port_dev = hub->ports[udev->portnum - 1];
+ struct usb_port *port_dev = hub->ports[port1 - 1];
sysfs_remove_link(&udev->dev.kobj, "port");
sysfs_remove_link(&port_dev->dev.kobj, "device");
- if (!port_dev->did_runtime_put)
+ if (test_and_clear_bit(port1, hub->child_usage_bits))
pm_runtime_put(&port_dev->dev);
- else
- port_dev->did_runtime_put = false;
}
usb_remove_ep_devs(&udev->ep0);
@@ -2416,7 +2417,8 @@ int usb_new_device(struct usb_device *udev)
/* Create link files between child device and usb port device. */
if (udev->parent) {
struct usb_hub *hub = usb_hub_to_struct_hub(udev->parent);
- struct usb_port *port_dev = hub->ports[udev->portnum - 1];
+ int port1 = udev->portnum;
+ struct usb_port *port_dev = hub->ports[port1 - 1];
err = sysfs_create_link(&udev->dev.kobj,
&port_dev->dev.kobj, "port");
@@ -2430,7 +2432,8 @@ int usb_new_device(struct usb_device *udev)
goto fail;
}
- pm_runtime_get_sync(&port_dev->dev);
+ if (!test_and_set_bit(port1, hub->child_usage_bits))
+ pm_runtime_get_sync(&port_dev->dev);
}
(void) usb_create_ep_devs(&udev->dev, &udev->ep0, udev);
@@ -3100,10 +3103,9 @@ int usb_port_suspend(struct usb_device *udev,
pm_message_t msg)
usb_set_device_state(udev, USB_STATE_SUSPENDED);
}
- if (status == 0 && !udev->do_remote_wakeup && udev->persist_enabled) {
+ if (status == 0 && !udev->do_remote_wakeup && udev->persist_enabled
+ && test_and_clear_bit(port1, hub->child_usage_bits))
pm_runtime_put_sync(&port_dev->dev);
- port_dev->did_runtime_put = true;
-
[PATCH v10 08/19] usb: sysfs link peer ports
The usb topology after this change will have symlinks between usb3 ports
and their usb2 peers, for example:
usb2/2-1/2-1:1.0/2-1-port1/peer => ../../../../usb3/3-1/3-1:1.0/3-1-port1
usb2/2-1/2-1:1.0/2-1-port2/peer => ../../../../usb3/3-1/3-1:1.0/3-1-port2
usb2/2-1/2-1:1.0/2-1-port3/peer => ../../../../usb3/3-1/3-1:1.0/3-1-port3
usb2/2-1/2-1:1.0/2-1-port4/peer => ../../../../usb3/3-1/3-1:1.0/3-1-port4
usb2/2-0:1.0/usb2-port1/peer=> ../../../usb3/3-0:1.0/usb3-port1
usb2/2-0:1.0/usb2-port2/peer=> ../../../usb3/3-0:1.0/usb3-port2
usb2/2-0:1.0/usb2-port3/peer=> ../../../usb3/3-0:1.0/usb3-port3
usb2/2-0:1.0/usb2-port4/peer=> ../../../usb3/3-0:1.0/usb3-port4
usb3/3-1/3-1:1.0/usb3-1-port1/peer => ../../../../usb2/2-1/2-1:1.0/2-1-port1
usb3/3-1/3-1:1.0/usb3-1-port2/peer => ../../../../usb2/2-1/2-1:1.0/2-1-port2
usb3/3-1/3-1:1.0/usb3-1-port3/peer => ../../../../usb2/2-1/2-1:1.0/2-1-port3
usb3/3-1/3-1:1.0/usb3-1-port4/peer => ../../../../usb2/2-1/2-1:1.0/2-1-port4
usb3/3-0:1.0/usb3-port1/peer => ../../../usb2/2-0:1.0/usb2-port1
usb3/3-0:1.0/usb3-port2/peer => ../../../usb2/2-0:1.0/usb2-port2
usb3/3-0:1.0/usb3-port3/peer => ../../../usb2/2-0:1.0/usb2-port3
usb3/3-0:1.0/usb3-port4/peer => ../../../usb2/2-0:1.0/usb2-port4
Introduce link_peers_report() to notify on all link_peers() failure
cases.
Acked-by: Alan Stern
Signed-off-by: Dan Williams
---
drivers/usb/core/port.c | 39 ++-
1 files changed, 34 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index aea54e8dfe47..40c3ac173e9e 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -157,10 +157,12 @@ static struct device_driver usb_port_driver = {
.owner = THIS_MODULE,
};
-static void link_peers(struct usb_port *left, struct usb_port *right)
+static int link_peers(struct usb_port *left, struct usb_port *right)
{
+ int rc;
+
if (left->peer == right && right->peer == left)
- return;
+ return 0;
if (left->peer || right->peer) {
struct usb_port *lpeer = left->peer;
@@ -170,11 +172,36 @@ static void link_peers(struct usb_port *left, struct
usb_port *right)
dev_name(&left->dev), dev_name(&right->dev),
dev_name(&left->dev), lpeer,
dev_name(&right->dev), rpeer);
- return;
+ return -EBUSY;
+ }
+
+ rc = sysfs_create_link(&left->dev.kobj, &right->dev.kobj, "peer");
+ if (rc)
+ return rc;
+ rc = sysfs_create_link(&right->dev.kobj, &left->dev.kobj, "peer");
+ if (rc) {
+ sysfs_remove_link(&left->dev.kobj, "peer");
+ return rc;
}
left->peer = right;
right->peer = left;
+
+ return 0;
+}
+
+static void link_peers_report(struct usb_port *left, struct usb_port *right)
+{
+ int rc;
+
+ rc = link_peers(left, right);
+ if (rc == 0) {
+ dev_dbg(&left->dev, "peered to %s\n", dev_name(&right->dev));
+ } else {
+ dev_warn(&left->dev, "failed to peer to %s (%d)\n",
+ dev_name(&right->dev), rc);
+ pr_warn_once("usb: port power management may be unreliable\n");
+ }
}
static void unlink_peers(struct usb_port *left, struct usb_port *right)
@@ -183,7 +210,9 @@ static void unlink_peers(struct usb_port *left, struct
usb_port *right)
"%s and %s are not peers?\n",
dev_name(&left->dev), dev_name(&right->dev));
+ sysfs_remove_link(&left->dev.kobj, "peer");
right->peer = NULL;
+ sysfs_remove_link(&right->dev.kobj, "peer");
left->peer = NULL;
}
@@ -212,7 +241,7 @@ static int match_location(struct usb_device *peer_hdev,
void *p)
for (port1 = 1; port1 <= peer_hdev->maxchild; port1++) {
peer = peer_hub->ports[port1 - 1];
if (peer && peer->location == port_dev->location) {
- link_peers(port_dev, peer);
+ link_peers_report(port_dev, peer);
return 1; /* done */
}
}
@@ -275,7 +304,7 @@ static void find_and_link_peer(struct usb_hub *hub, int
port1)
*/
peer = peer_hub->ports[port1 - 1];
if (peer && peer->location == 0)
- link_peers(port_dev, peer);
+ link_peers_report(port_dev, peer);
}
int usb_hub_create_port_device(struct usb_hub *hub, int port1)
--
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 v10 11/19] usb: don't clear FEAT_C_ENABLE on usb_port_runtime_resume failure
Three reasons: 1/ It's an invalid operation on usb3 ports 2/ There's no guarantee of when / if a usb2 port has entered an error state relative to PORT_POWER request 3/ The port is active / powered at this point, so khubd will clear it as a matter of course Acked-by: Alan Stern Signed-off-by: Dan Williams --- drivers/usb/core/port.c |1 - 1 files changed, 0 insertions(+), 1 deletions(-) diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c index 827b0d38f73d..f41f0512307e 100644 --- a/drivers/usb/core/port.c +++ b/drivers/usb/core/port.c @@ -110,7 +110,6 @@ static int usb_port_runtime_resume(struct device *dev) if (retval < 0) dev_dbg(&port_dev->dev, "can't get reconnection after setting port power on, status %d\n", retval); - usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE); retval = 0; } -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 12/19] usb: usb3 ports do not support FEAT_C_ENABLE
The port pm_runtime implementation unconditionally clears FEAT_C_ENABLE after clearing PORT_POWER, but the bit is reserved on usb3 hub ports. We expect khubd to be prevented from running because the port state is not RPM_ACTIVE, so we need to clear any errors for usb2 ports. Acked-by: Alan Stern Signed-off-by: Dan Williams --- drivers/usb/core/port.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c index f41f0512307e..fb83c2c13920 100644 --- a/drivers/usb/core/port.c +++ b/drivers/usb/core/port.c @@ -142,7 +142,8 @@ static int usb_port_runtime_suspend(struct device *dev) set_bit(port1, hub->busy_bits); retval = usb_hub_set_port_power(hdev, hub, port1, false); usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_CONNECTION); - usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE); + if (!port_dev->is_superspeed) + usb_clear_port_feature(hdev, port1, USB_PORT_FEAT_C_ENABLE); clear_bit(port1, hub->busy_bits); usb_autopm_put_interface(intf); -- 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 v10 07/19] usb: find internal hub tier mismatch via acpi
ACPI identifies peer ports by setting their 'group_token' and
'group_position' _PLD data to the same value. If a platform has tier
mismatch [1] , ACPI can override the default (USB3 defined) peer port
association for internal hubs. External hubs follow the default peer
association scheme.
Location data is cached as an opaque cookie in usb_port_location data.
Note that we only consider the group_token and group_position attributes
from the _PLD data as ACPI specifies that group_token is a unique
identifier.
When we find port location data for a port then we assume that the
firmware will also describe its peer port. This allows the
implementation to only ever set the peer once. This leads to a question
about what happens when a pm runtime event occurs while the peer
associations are still resolving. Since we only ever set the peer
information once, a USB3 port needs to be prevented from suspending
while its ->peer pointer is NULL (implemented in a subsequent patch).
There is always the possibility that firmware mis-identifies the ports,
but there is not much the kernel can do in that case.
[1]: xhci 1.1 appendix D figure 131
[2]: acpi 5 section 6.1.8
[alan]: don't do default peering when acpi data present
Suggested-by: Alan Stern
Acked-by: Alan Stern
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.h |2 ++
drivers/usb/core/port.c | 56 ---
drivers/usb/core/usb-acpi.c | 41 ++-
drivers/usb/core/usb.h |6 +
4 files changed, 83 insertions(+), 22 deletions(-)
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index fcad5f9d12f0..048c797f394c 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -84,6 +84,7 @@ struct usb_hub {
* @port_owner: port's owner
* @peer: related usb2 and usb3 ports (share the same connector)
* @connect_type: port's connect type
+ * @location: opaque representation of platform connector location
* @portnum: port index num based one
* @power_is_on: port's power state
* @did_runtime_put: port has done pm_runtime_put().
@@ -94,6 +95,7 @@ struct usb_port {
struct usb_dev_state *port_owner;
struct usb_port *peer;
enum usb_port_connect_type connect_type;
+ usb_port_location_t location;
u8 portnum;
unsigned power_is_on:1;
unsigned did_runtime_put:1;
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 9b7496b52f2a..aea54e8dfe47 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -188,8 +188,42 @@ static void unlink_peers(struct usb_port *left, struct
usb_port *right)
}
/*
- * Set the default peer port for root hubs, or via the upstream peer
- * relationship for all other hubs
+ * For each usb hub device in the system check to see if it is in the
+ * peer domain of the given port_dev, and if it is check to see if it
+ * has a port that matches the given port by location
+ */
+static int match_location(struct usb_device *peer_hdev, void *p)
+{
+ int port1;
+ struct usb_hcd *hcd, *peer_hcd;
+ struct usb_port *port_dev = p, *peer;
+ struct usb_hub *peer_hub = usb_hub_to_struct_hub(peer_hdev);
+ struct usb_device *hdev = to_usb_device(port_dev->dev.parent->parent);
+
+ if (!peer_hub)
+ return 0;
+
+ hcd = bus_to_hcd(hdev->bus);
+ peer_hcd = bus_to_hcd(peer_hdev->bus);
+ /* peer_hcd is provisional until we verify it against the known peer */
+ if (peer_hcd != hcd->shared_hcd)
+ return 0;
+
+ for (port1 = 1; port1 <= peer_hdev->maxchild; port1++) {
+ peer = peer_hub->ports[port1 - 1];
+ if (peer && peer->location == port_dev->location) {
+ link_peers(port_dev, peer);
+ return 1; /* done */
+ }
+ }
+
+ return 0;
+}
+
+/*
+ * Find the peer port either via explicit platform firmware "location"
+ * data, the peer hcd for root hubs, or the upstream peer relationship
+ * for all other hubs.
*/
static void find_and_link_peer(struct usb_hub *hub, int port1)
{
@@ -198,7 +232,17 @@ static void find_and_link_peer(struct usb_hub *hub, int
port1)
struct usb_device *peer_hdev;
struct usb_hub *peer_hub;
- if (!hdev->parent) {
+ /*
+* If location data is available then we can only peer this port
+* by a location match, not the default peer (lest we create a
+* situation where we need to go back and undo a default peering
+* when the port is later peered by location data)
+*/
+ if (port_dev->location) {
+ /* we link the peer in match_location() if found */
+ usb_for_each_dev(port_dev, match_location);
+ return;
+ } else if (!hdev->parent) {
struct usb_hcd *hcd = bus_to_hcd(hdev->bus);
struct usb_hcd *peer_hcd = hcd->shared_hcd;
[PATCH v10 17/19] usb: resume child device when port is powered on
Unconditionally wake up the child device when the power session is
recovered.
This addresses the following scenarios:
1/ The device may need a reset on power-session loss, without this
change port power-on recovery exposes khubd to scenarios that
usb_port_resume() is set to handle. Prior to port power control the
only time a power session would be lost is during dpm_suspend of the
hub. In that scenario usb_port_resume() is guaranteed to be called
prior to khubd running for that port. With this change we wakeup the
child device as soon as possible (prior to khubd running again for this
port).
Although khubd has facilities to wake a child device it will only do
so if the portstatus / portchange indicates a suspend state. In the
case of port power control we are not coming from a hub-port-suspend
state. This implementation simply uses pm_request_resume() to wake the
device and relies on the port_dev->status_lock to prevent any collisions
between khubd and usb_port_resume().
2/ This mechanism rate limits port power toggling. The minimum port
power on/off period is now gated by the child device suspend/resume
latency. Empirically this mitigates devices downgrading their connection
on perceived instability of the host connection. This ratelimiting is
really only relevant to port power control testing, but it is a nice
side effect of closing the above race. Namely, the race of khubd for
the given port running while a usb_port_resume() event is pending.
3/ Going forward we are finding that power-session recovery requires
warm-resets (http://marc.info/?t=13865923293&r=1&w=2). This
mechanism allows for warm-resets to be requested at the same point in
the resume path for hub dpm_suspend power session losses, or port
rpm_suspend power session losses.
4/ If the device *was* disconnected the only time we'll know for sure is
after a failed resume, so it's necessary for usb_port_runtime_resume()
to expedite a usb_port_resume() to clean up the removed device. The
reasoning for this is "least surprise" for the user. Turning on a port
means that hotplug detection is again enabled for the port, it is
surprising that devices that were removed while the port was off are not
disconnected until they are attempted to be used. As a user "why would
I try to use a device I removed from the system?"
1, 2, and 4 are not a problem in the system dpm_resume() case because,
although the power-session is lost, khubd is frozen until after device
resume. For the rpm_resume() case pm_request_resume() is used to
request re-validation of the device, and if it happens to collide with a
khubd run we rely on the port_dev->status_lock to synchronize those
operations.
Besides testing, the primary scenario where this mechanism is expected
to be triggered is when the user changes the port power policy
(control/pm_qos_no_poweroff, or power/control). Each time power is
enabled want to revalidate the child device, where the revalidation is
handled by usb_port_resume().
Given that this arranges for port_dev->child to be de-referenced in
usb_port_runtime_resume() we need to make sure not to collide with
usb_disconnect() that frees the usb_device. To this end we hold the
port active with the "child_usage" reference across the disconnect
event. Subsequently, the need to access hub->child_usage_bits lead to
the creation of hub_disconnect_children() to remove any ambiguity of
which "hub" is being acted on in usb_disconnect() (prompted-by sharp
eyes from Alan).
Cc: Rafael J. Wysocki
Acked-by: Alan Stern
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.c | 42 +-
drivers/usb/core/port.c |9 -
2 files changed, 37 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index c2f20738f91f..df4a9f587502 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2039,6 +2039,18 @@ static void hub_free_dev(struct usb_device *udev)
hcd->driver->free_dev(hcd, udev);
}
+static void hub_disconnect_children(struct usb_device *udev)
+{
+ struct usb_hub *hub = usb_hub_to_struct_hub(udev);
+ int i;
+
+ /* Free up all the children before we remove this device */
+ for (i = 0; i < udev->maxchild; i++) {
+ if (hub->ports[i]->child)
+ usb_disconnect(&hub->ports[i]->child);
+ }
+}
+
/**
* usb_disconnect - disconnect a device (usbcore-internal)
* @pdev: pointer to device being disconnected
@@ -2057,9 +2069,10 @@ static void hub_free_dev(struct usb_device *udev)
*/
void usb_disconnect(struct usb_device **pdev)
{
- struct usb_device *udev = *pdev;
- struct usb_hub *hub = usb_hub_to_struct_hub(udev);
- int i;
+ struct usb_port *port_dev = NULL;
+ struct usb_device *udev = *pdev;
+ struct usb_hub *
[PATCH v10 13/19] usb: refactor port handling in hub_events()
In preparation for synchronizing port handling with pm_runtime
transitions refactor port handling into its own subroutine.
We expect that clearing some status flags will be required regardless of
the port state, so handle those first and group all non-trivial actions
at the bottom of the routine.
This also splits off the bottom half of hub_port_connect_change() into
hub_port_reconnect() in prepartion for introducing a port->status_lock.
hub_port_reconnect() will expect the port lock to not be held while
hub_port_connect_change() expects to enter with it held.
Other cleanups include:
1/ reflowing to 80 columns
2/ replacing redundant usages of 'hub->hdev' with 'hdev'
3/ consolidate clearing of ->change_bits() in hub_port_connect_change
4/ consolidate calls to usb_reset_device
Acked-by: Alan Stern
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.c | 374
1 files changed, 185 insertions(+), 189 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 2269f54da0a9..597bdc48e195 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4415,66 +4415,15 @@ hub_power_remaining (struct usb_hub *hub)
return remaining;
}
-/* Handle physical or logical connection change events.
- * This routine is called when:
- * a port connection-change occurs;
- * a port enable-change occurs (often caused by EMI);
- * usb_reset_and_verify_device() encounters changed descriptors (as from
- * a firmware download)
- * caller already locked the hub
- */
-static void hub_port_connect_change(struct usb_hub *hub, int port1,
- u16 portstatus, u16 portchange)
+static void hub_port_connect(struct usb_hub *hub, int port1, u16 portstatus,
+ u16 portchange)
{
+ int status, i;
+ unsigned unit_load;
struct usb_device *hdev = hub->hdev;
struct usb_hcd *hcd = bus_to_hcd(hdev->bus);
struct usb_port *port_dev = hub->ports[port1 - 1];
- struct usb_device *udev;
- int status, i;
- unsigned unit_load;
-
- dev_dbg(&port_dev->dev, "status %04x, change %04x, %s\n",
- portstatus, portchange, portspeed(hub, portstatus));
-
- if (hub->has_indicators) {
- set_port_led(hub, port1, HUB_LED_AUTO);
- hub->indicator[port1-1] = INDICATOR_AUTO;
- }
-
-#ifdef CONFIG_USB_OTG
- /* during HNP, don't repeat the debounce */
- if (hdev->bus->is_b_host)
- portchange &= ~(USB_PORT_STAT_C_CONNECTION |
- USB_PORT_STAT_C_ENABLE);
-#endif
-
- /* Try to resuscitate an existing device */
- udev = port_dev->child;
- if ((portstatus & USB_PORT_STAT_CONNECTION) && udev &&
- udev->state != USB_STATE_NOTATTACHED) {
- usb_lock_device(udev);
- if (portstatus & USB_PORT_STAT_ENABLE) {
- status = 0; /* Nothing to do */
-
-#ifdef CONFIG_PM_RUNTIME
- } else if (udev->state == USB_STATE_SUSPENDED &&
- udev->persist_enabled) {
- /* For a suspended device, treat this as a
-* remote wakeup event.
-*/
- status = usb_remote_wakeup(udev);
-#endif
-
- } else {
- status = -ENODEV; /* Don't resuscitate */
- }
- usb_unlock_device(udev);
-
- if (status == 0) {
- clear_bit(port1, hub->change_bits);
- return;
- }
- }
+ struct usb_device *udev = port_dev->child;
/* Disconnect any existing devices under this port */
if (udev) {
@@ -4483,7 +4432,6 @@ static void hub_port_connect_change(struct usb_hub *hub,
int port1,
usb_phy_notify_disconnect(hcd->phy, udev->speed);
usb_disconnect(&port_dev->child);
}
- clear_bit(port1, hub->change_bits);
/* We can forget about a "removed" device when there's a physical
* disconnect or the connect status changes.
@@ -4665,6 +4613,65 @@ done:
hub_port_disable(hub, port1, 1);
if (hcd->driver->relinquish_port && !hub->hdev->parent)
hcd->driver->relinquish_port(hcd, port1);
+
+}
+
+/* Handle physical or logical connection change events.
+ * This routine is called when:
+ * a port connection-change occurs;
+ * a port enable-change occurs (often caused by EMI);
+ * usb_reset_and_verify_device() encounters changed descriptors (as from
+ * a firmware download)
+ * caller already locked the hub
+ */
+static void hub_port_connect_change(struct usb_hub *hub, int port1,
+ u16 portstatus, u16 portchange)
+{
+ struct usb_port *port_dev = hub->ports[port1 - 1];
+
[PATCH v10 10/19] usb: block suspension of superspeed port while hispeed peer is active
ClearPortFeature(PORT_POWER) on a usb3 port places the port in either a
DSPORT.Powered-off-detect / DSPORT.Powered-off-reset loop, or the
DSPORT.Powered-off state. There is no way to ensure that RX
terminations will persist in this state, so it is possible a device will
degrade to its usb2 connection. Prevent this by blocking power-off of a
usb3 port while its usb2 peer is active, and powering on a usb3 port
before its usb2 peer.
By default the latency between peer power-on events is 0. In order for
the device to not see usb2 active while usb3 is still powering up inject
the hub recommended power_on_good delay. In support of satisfying the
power_on_good delay outside of hub_power_on() refactor the places where
the delay is consumed to call a new hub_power_on_good_delay() helper.
Finally, because this introduces several new checks for whether a port
is_superspeed, cache that disctinction at port creation so that we don't
need to keep looking up the parent hub device.
Acked-by: Alan Stern
[alan]: add a 'superspeed' flag to the port
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.c | 22 +++---
drivers/usb/core/hub.h | 15 ++
drivers/usb/core/port.c | 73 +++
3 files changed, 93 insertions(+), 17 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 3d50f82faea9..2269f54da0a9 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -36,11 +36,6 @@
#define USB_VENDOR_GENESYS_LOGIC 0x05e3
#define HUB_QUIRK_CHECK_PORT_AUTOSUSPEND 0x01
-static inline int hub_is_superspeed(struct usb_device *hdev)
-{
- return (hdev->descriptor.bDeviceProtocol == USB_HUB_PR_SS);
-}
-
/* Protect struct usb_device->state and ->children members
* Note: Both are also protected by ->dev.sem, except that ->state can
* change to USB_STATE_NOTATTACHED even when the semaphore isn't held. */
@@ -822,14 +817,9 @@ int usb_hub_clear_tt_buffer(struct urb *urb)
}
EXPORT_SYMBOL_GPL(usb_hub_clear_tt_buffer);
-/* If do_delay is false, return the number of milliseconds the caller
- * needs to delay.
- */
-static unsigned hub_power_on(struct usb_hub *hub, bool do_delay)
+static void hub_power_on(struct usb_hub *hub, bool do_delay)
{
int port1;
- unsigned pgood_delay = hub->descriptor->bPwrOn2PwrGood * 2;
- unsigned delay;
/* Enable power on each port. Some hubs have reserved values
* of LPSM (> 2) in their descriptors, even though they are
@@ -848,12 +838,8 @@ static unsigned hub_power_on(struct usb_hub *hub, bool
do_delay)
else
usb_clear_port_feature(hub->hdev, port1,
USB_PORT_FEAT_POWER);
-
- /* Wait at least 100 msec for power to become stable */
- delay = max(pgood_delay, (unsigned) 100);
if (do_delay)
- msleep(delay);
- return delay;
+ msleep(hub_power_on_good_delay(hub));
}
static int hub_hub_status(struct usb_hub *hub,
@@ -1057,7 +1043,9 @@ static void hub_activate(struct usb_hub *hub, enum
hub_activation_type type)
* for HUB_POST_RESET, but it's easier not to.
*/
if (type == HUB_INIT) {
- delay = hub_power_on(hub, false);
+ unsigned delay = hub_power_on_good_delay(hub);
+
+ hub_power_on(hub, false);
INIT_DELAYED_WORK(&hub->init_work, hub_init_func2);
queue_delayed_work(system_power_efficient_wq,
&hub->init_work,
diff --git a/drivers/usb/core/hub.h b/drivers/usb/core/hub.h
index 3ef1c2e435cc..906c355e0631 100644
--- a/drivers/usb/core/hub.h
+++ b/drivers/usb/core/hub.h
@@ -89,6 +89,7 @@ struct usb_hub {
* @connect_type: port's connect type
* @location: opaque representation of platform connector location
* @portnum: port index num based one
+ * @is_superspeed cache super-speed status
*/
struct usb_port {
struct usb_device *child;
@@ -98,6 +99,7 @@ struct usb_port {
enum usb_port_connect_type connect_type;
usb_port_location_t location;
u8 portnum;
+ unsigned int is_superspeed:1;
};
#define to_usb_port(_dev) \
@@ -125,6 +127,19 @@ static inline bool hub_is_port_power_switchable(struct
usb_hub *hub)
return (le16_to_cpu(hcs) & HUB_CHAR_LPSM) < HUB_CHAR_NO_LPSM;
}
+static inline int hub_is_superspeed(struct usb_device *hdev)
+{
+ return hdev->descriptor.bDeviceProtocol == USB_HUB_PR_SS;
+}
+
+static inline unsigned hub_power_on_good_delay(struct usb_hub *hub)
+{
+ unsigned delay = hub->descriptor->bPwrOn2PwrGood * 2;
+
+ /* Wait at least 100 msec for power to become stable */
+ return max(delay, 100U);
+}
+
static inline int hub_port_debounce_be_connected(struct usb_hub *hub,
int port1)
{
diff --git a/dr
[PATCH v10 16/19] usb: hub_handle_remote_wakeup() depends on CONFIG_PM_RUNTIME=y
Per Alan:
"You mean from within hub_handle_remote_wakeup()? That routine will
never get called if CONFIG_PM_RUNTIME isn't enabled, because khubd
never sees wakeup requests if they arise during system suspend.
In fact, that routine ought to go inside the "#ifdef CONFIG_PM_RUNTIME"
portion of hub.c, along with the other suspend/resume code."
Suggested-by: Alan Stern
Acked-by: Alan Stern
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.c | 90 ++--
drivers/usb/core/usb.h |5 ---
2 files changed, 49 insertions(+), 46 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index af68efdd48fc..c2f20738f91f 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -3348,6 +3348,55 @@ int usb_remote_wakeup(struct usb_device *udev)
return status;
}
+/* Returns 1 if there was a remote wakeup and a connect status change. */
+static int hub_handle_remote_wakeup(struct usb_hub *hub, unsigned int port,
+ u16 portstatus, u16 portchange)
+ __must_hold(&port_dev->status_lock)
+{
+ struct usb_port *port_dev = hub->ports[port - 1];
+ struct usb_device *hdev;
+ struct usb_device *udev;
+ int connect_change = 0;
+ int ret;
+
+ hdev = hub->hdev;
+ udev = port_dev->child;
+ if (!hub_is_superspeed(hdev)) {
+ if (!(portchange & USB_PORT_STAT_C_SUSPEND))
+ return 0;
+ usb_clear_port_feature(hdev, port, USB_PORT_FEAT_C_SUSPEND);
+ } else {
+ if (!udev || udev->state != USB_STATE_SUSPENDED ||
+(portstatus & USB_PORT_STAT_LINK_STATE) !=
+USB_SS_PORT_LS_U0)
+ return 0;
+ }
+
+ if (udev) {
+ /* TRSMRCY = 10 msec */
+ msleep(10);
+
+ usb_unlock_port(port_dev);
+ ret = usb_remote_wakeup(udev);
+ usb_lock_port(port_dev);
+ if (ret < 0)
+ connect_change = 1;
+ } else {
+ ret = -ENODEV;
+ hub_port_disable(hub, port, 1);
+ }
+ dev_dbg(&port_dev->dev, "resume, status %d\n", ret);
+ return connect_change;
+}
+
+#else
+
+static int hub_handle_remote_wakeup(struct usb_hub *hub, unsigned int port,
+ u16 portstatus, u16 portchange)
+{
+ return 0;
+}
+
#endif
static int check_ports_changed(struct usb_hub *hub)
@@ -4699,47 +4748,6 @@ static void hub_port_connect_change(struct usb_hub *hub,
int port1,
usb_lock_port(port_dev);
}
-/* Returns 1 if there was a remote wakeup and a connect status change. */
-static int hub_handle_remote_wakeup(struct usb_hub *hub, unsigned int port,
- u16 portstatus, u16 portchange)
- __must_hold(&port_dev->status_lock)
-{
- struct usb_port *port_dev = hub->ports[port - 1];
- struct usb_device *hdev;
- struct usb_device *udev;
- int connect_change = 0;
- int ret;
-
- hdev = hub->hdev;
- udev = port_dev->child;
- if (!hub_is_superspeed(hdev)) {
- if (!(portchange & USB_PORT_STAT_C_SUSPEND))
- return 0;
- usb_clear_port_feature(hdev, port, USB_PORT_FEAT_C_SUSPEND);
- } else {
- if (!udev || udev->state != USB_STATE_SUSPENDED ||
-(portstatus & USB_PORT_STAT_LINK_STATE) !=
-USB_SS_PORT_LS_U0)
- return 0;
- }
-
- if (udev) {
- /* TRSMRCY = 10 msec */
- msleep(10);
-
- usb_unlock_port(port_dev);
- ret = usb_remote_wakeup(udev);
- usb_lock_port(port_dev);
- if (ret < 0)
- connect_change = 1;
- } else {
- ret = -ENODEV;
- hub_port_disable(hub, port, 1);
- }
- dev_dbg(&port_dev->dev, "resume, status %d\n", ret);
- return connect_change;
-}
-
static void port_event(struct usb_hub *hub, int port1)
__must_hold(&port_dev->status_lock)
{
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 98dc08e13448..d9d08720c386 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -107,11 +107,6 @@ static inline int usb_autoresume_device(struct usb_device
*udev)
return 0;
}
-static inline int usb_remote_wakeup(struct usb_device *udev)
-{
- return 0;
-}
-
static inline int usb_set_usb2_hardware_lpm(struct usb_device *udev, int
enable)
{
return 0;
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v10 19/19] usb: documentation for usb port power off mechanisms
From: Lan Tianyu describe the mechanisms for controlling port power policy and discovering the port power state. Cc: Oliver Neukum [oliver]: fixes, clarification of wakeup vs port-power-control Signed-off-by: Lan Tianyu [sarah]: wordsmithing [djbw]: updates for peer port changes Signed-off-by: Dan Williams --- Documentation/usb/power-management.txt | 242 1 files changed, 240 insertions(+), 2 deletions(-) diff --git a/Documentation/usb/power-management.txt b/Documentation/usb/power-management.txt index 1392b61d6ebe..6c3e0f04f6cb 100644 --- a/Documentation/usb/power-management.txt +++ b/Documentation/usb/power-management.txt @@ -2,8 +2,27 @@ Alan Stern - October 28, 2010 - + Last-updated: February 2014 + + + Contents: + - + * What is Power Management? + * What is Remote Wakeup? + * When is a USB device idle? + * Forms of dynamic PM + * The user interface for dynamic PM + * Changing the default idle-delay time + * Warnings + * The driver interface for Power Management + * The driver interface for autosuspend and autoresume + * Other parts of the driver interface + * Mutual exclusion + * Interaction between dynamic PM and system PM + * xHCI hardware link PM + * USB Port Power Control + * User Interface for Port Power Control + * Suggested Userspace Port Power Policy What is Power Management? @@ -516,3 +535,222 @@ relevant attribute files is usb2_hardware_lpm. driver will enable hardware LPM for the device. You can write y/Y/1 or n/N/0 to the file to enable/disable USB2 hardware LPM manually. This is for test purpose mainly. + + + USB Port Power Control + -- + +In addition to suspending endpoint devices and enabling hardware +controlled link power management, the USB subsystem also has the +capability to disable power to ports under some conditions. Power is +controlled through Set/ClearPortFeature(PORT_POWER) requests to a hub. +In the case of a root or platform-internal hub the host controller +driver translates PORT_POWER requests into platform firmware (ACPI) +method calls to set the port power state. For more background see the +Linux Plumbers Conference 2012 slides [1] and video [2]: + +Upon receiving a ClearPortFeature(PORT_POWER) request a USB port is +logically off, and may trigger the actual loss of VBUS to the port [3]. +VBUS may be maintained in the case where a hub gangs multiple ports into +a shared power well causing power to remain until all ports in the gang +are turned off. VBUS may also be maintained by hub ports configured for +a charging application. In any event a logically off port will lose +connection with its device, not respond to hotplug events, and not +respond to remote wakeup events*. + +WARNING: turning off a port may result in the inability to hot add a device. +Please see "User Interface for Port Power Control" for details. + +As far as the effect on the device itself it is similar to what a device +goes through during system suspend, i.e. the power session is lost. Any +USB device or driver that misbehaves with system suspend will be +similarly affected by a port power cycle event. For this reason the +implementation shares the same device recovery path (and honors the same +quirks) as the system resume path for the hub. + +[1]: http://dl.dropbox.com/u/96820575/sarah-sharp-lpt-port-power-off2-mini.pdf +[2]: http://linuxplumbers.ubicast.tv/videos/usb-port-power-off-kerneluserspace-api/ +[3]: USB 3.1 Section 10.12 +* wakeup note: if a device is configured to send wakeup events the port + power control implementation will block poweroff attempts on that + port. + + + User Interface for Port Power Control + - + +The port power control mechanism uses the PM runtime system. Poweroff is +requested by clearing the power/pm_qos_no_power_off flag of the port device +(defaults to 1). If the port is disconnected it will immediately receive a +ClearPortFeature(PORT_POWER) request. Otherwise, it will honor the pm runtime +rules and require the attached child device and all descendants to be suspended. +This mechanism is dependent on the hub advertising port power switching in its +hub descriptor (wHubCharacteristics logical power switching mode field). + +Note, some interface devices/drivers do not support autosuspend. Userspace may +need to unbind the interface drivers before the usb_device will suspend. An +unbound interface device is suspended by default. When unbinding, be careful +to unbind interface drivers, not the driver of the parent usb device. Also, +leave hub interface drivers bound. If the driver for the usb device (not +interface) is unbound the kernel is no longer able to resume the device. If a
[PATCH v10 18/19] usb: force warm reset to break link re-connect livelock
Resuming a powered down port sometimes results in the port state being
stuck in the training sequence.
hub 3-0:1.0: debounce: port 1: total 2000ms stable 0ms status 0x2e0
port1: can't get reconnection after setting port power on, status -110
hub 3-0:1.0: port 1 status .02e0 after resume, -19
usb 3-1: can't resume, status -19
hub 3-0:1.0: logical disconnect on port 1
In the case above we wait for the port re-connect timeout of 2 seconds
and observe that the port status is USB_SS_PORT_LS_POLLING (although it
is likely toggling between this state and USB_SS_PORT_LS_RX_DETECT).
This is indicative of a case where the device is failing to progress the
link training state machine.
It is resolved by issuing a warm reset to get the hub and device link
state machines back in sync.
hub 3-0:1.0: debounce: port 1: total 2000ms stable 0ms status 0x2e0
usb usb3: port1 usb_port_runtime_resume requires warm reset
hub 3-0:1.0: port 1 not warm reset yet, waiting 50ms
usb 3-1: reset SuperSpeed USB device number 2 using xhci_hcd
After a reconnect timeout when we expect the device to be present, force
a warm reset of the device. Note that we can not simply look at the
link status to determine if a warm reset is required as any of the
training states USB_SS_PORT_LS_POLLING, USB_SS_PORT_LS_RX_DETECT, or
USB_SS_PORT_LS_COMP_MOD are valid states that do not indicate the need
for warm reset by themselves.
Cc: Alan Stern
Cc: Kukjin Kim
Cc: Vincent Palatin
Cc: Lan Tianyu
Cc: Ksenia Ragiadakou
Cc: Vivek Gautam
Cc: Douglas Anderson
Cc: Felipe Balbi
Cc: Sunil Joshi
Cc: Hans de Goede
Acked-by: Julius Werner
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.c | 23 ---
drivers/usb/core/hub.h |2 ++
drivers/usb/core/port.c | 21 -
3 files changed, 30 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index df4a9f587502..573faa097d03 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2570,10 +2570,12 @@ static int hub_port_reset(struct usb_hub *hub, int
port1,
/* Is a USB 3.0 port in the Inactive or Compliance Mode state?
* Port worm reset is required to recover
*/
-static bool hub_port_warm_reset_required(struct usb_hub *hub, u16 portstatus)
+static bool hub_port_warm_reset_required(struct usb_hub *hub, int port1,
+u16 portstatus)
{
return hub_is_superspeed(hub->hdev) &&
- (((portstatus & USB_PORT_STAT_LINK_STATE) ==
+ (test_bit(port1, hub->warm_reset_bits) ||
+ ((portstatus & USB_PORT_STAT_LINK_STATE) ==
USB_SS_PORT_LS_SS_INACTIVE) ||
((portstatus & USB_PORT_STAT_LINK_STATE) ==
USB_SS_PORT_LS_COMP_MOD)) ;
@@ -2613,7 +2615,7 @@ static int hub_port_wait_reset(struct usb_hub *hub, int
port1,
if ((portstatus & USB_PORT_STAT_RESET))
return -EBUSY;
- if (hub_port_warm_reset_required(hub, portstatus))
+ if (hub_port_warm_reset_required(hub, port1, portstatus))
return -ENOTCONN;
/* Device went away? */
@@ -2713,8 +2715,9 @@ static int hub_port_reset(struct usb_hub *hub, int port1,
if (status < 0)
goto done;
- if (hub_port_warm_reset_required(hub, portstatus))
+ if (hub_port_warm_reset_required(hub, port1, portstatus))
warm = true;
+ clear_bit(port1, hub->warm_reset_bits);
}
/* Reset the port */
@@ -2752,7 +2755,8 @@ static int hub_port_reset(struct usb_hub *hub, int port1,
&portstatus, &portchange) < 0)
goto done;
- if (!hub_port_warm_reset_required(hub, portstatus))
+ if (!hub_port_warm_reset_required(hub, port1,
+ portstatus))
goto done;
/*
@@ -2839,8 +2843,13 @@ static int check_port_resume_type(struct usb_device
*udev,
{
struct usb_port *port_dev = hub->ports[port1 - 1];
+ /* Is a warm reset needed to recover the connection? */
+ if (udev->reset_resume
+ && hub_port_warm_reset_required(hub, port1, portstatus)) {
+ /* pass */;
+ }
/* Is the device still present? */
- if (status || port_is_suspended(hub, portstatus) ||
+ else if (status || port_is_suspended(hub, portstatus) ||
!port_is_power_on(hub, portstatus) ||
!(portstatus & USB_PORT_STAT_CONNECTION)) {
if (status >= 0)
@@ -4848,7 +4857,7 @@ static void port_event(struct usb_hub *hub, int port1)
* Warm reset a USB3 protocol port if it's in
* SS.Inactive state.
*/
- if (hub_port_warm_reset_required(hub, portstatus)) {
+ if (hu
[PATCH v10 14/19] usb: synchronize port poweroff and khubd
If a port is powered-off, or in the process of being powered-off, prevent
khubd from operating on it. Otherwise, the following sequence of events
leading to an unintended disconnect may occur:
Events:
(0)
(1) hub 2-2:1.0: hub_resume
(2) hub 2-2:1.0: port 1: status 0301 change
(3) hub 2-2:1.0: state 7 ports 4 chg 0002 evt
(4) hub 2-2:1.0: port 1, power off status , change , 12 Mb/s
(5) usb 2-2.1: USB disconnect, device number 5
Description:
(1) hub is resumed before sending a ClearPortFeature request
(2) hub_activate() notices the port is connected and sets
hub->change_bits for the port
(3) hub_events() starts, but at the same time the port suspends
(4) hub_connect_change() sees the disabled port and triggers disconnect
Acked-by: Alan Stern
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.c | 21 -
1 files changed, 20 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 597bdc48e195..847271418cca 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -4786,6 +4786,10 @@ static void port_event(struct usb_hub *hub, int port1)
USB_PORT_FEAT_C_PORT_CONFIG_ERROR);
}
+ /* skip port actions that require the port to be powered on */
+ if (!pm_runtime_active(&port_dev->dev))
+ return;
+
if (hub_handle_remote_wakeup(hub, port1, portstatus, portchange))
connect_change = 1;
@@ -4912,11 +4916,26 @@ static void hub_events(void)
/* deal with port status changes */
for (i = 1; i <= hdev->maxchild; i++) {
+ struct usb_port *port_dev = hub->ports[i - 1];
+
if (!test_bit(i, hub->busy_bits)
&& (test_bit(i, hub->event_bits)
|| test_bit(i, hub->change_bits)
- || test_bit(i,
hub->wakeup_bits)))
+ || test_bit(i,
hub->wakeup_bits))) {
+ /*
+* The get_noresume and barrier ensure that if
+* the port was in the process of resuming, we
+* flush that work and keep the port active for
+* the duration of the port_event(). However,
+* if the port is runtime pm suspended
+* (powered-off), we leave it in that state, run
+* an abbreviated port_event(), and move on.
+*/
+ pm_runtime_get_noresume(&port_dev->dev);
+ pm_runtime_barrier(&port_dev->dev);
port_event(hub, i);
+ pm_runtime_put_sync(&port_dev->dev);
+ }
}
/* deal with hub status changes */
--
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: new driver Fresco Logic
Ok ill join them and see if linux-me...@vger.kernel.org are working on it ... i had no idea where to start ... Mike KC7NOA On Tue, May 20, 2014 at 5:25 PM, Greg KH wrote: > On Tue, May 20, 2014 at 06:38:01PM -0500, Felipe Balbi wrote: >> Hi, >> >> On Wed, May 21, 2014 at 05:31:13AM +0900, Greg KH wrote: >> > On Tue, May 20, 2014 at 08:07:43AM -0700, Michael Durkin wrote: >> > > Forgive my intrusion ... >> > > >> > > Im hoping to get a driver made for a Fresco Logic FL2000 USB to VGA >> > > adapter ... >> > > >> > > Device ID 1d5c:2000 >> > >> > What type of chip is in this device? If it is a USB 3 device, odds are >> > that Linux doesn't support it yet as DeviceLink isn't providing the spec >> >> DisplayLink ? > > Yes, sorry :) > >> > for the chip to anyone. Can you ask the company you bought the device >> > from for how to get the specs for it? >> >> AFAICT this is a competing device. From what I could gather, Fresco >> Logic implemented this device from scratch. The good thing about it, is >> that it implements USB 3 A/V class, which means all we need is the USB >> IF A/V Class specification from [1] >> >> [1] >> http://www.usb.org/developers/docs/devclass_docs/USB_AV_Specification_Rev_1.0.zip > > Ah, good. I think someone is working on that. Michael, you might want > to ask about this on the linux-me...@vger.kernel.org mailing list, the > developers there should be able to help you out. > > 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 v10 06/19] usb: assign usb3 external hub port peers
Given that root hub port peers are already established, external hub peer
ports can be determined by traversing the device topology:
1/ ascend to the parent hub and find the upstream port_dev
2/ walk ->peer to find the peer port
3/ descend to the peer hub via ->child
4/ find the port with the matching port id
Note that this assumes the port labeling scheme required by the
specification [1].
[1]: usb3 3.1 section 10.3.3
Acked-by: Alan Stern
Signed-off-by: Dan Williams
---
drivers/usb/core/port.c | 32
1 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index 5ecdbf31dfcb..9b7496b52f2a 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -187,15 +187,18 @@ static void unlink_peers(struct usb_port *left, struct
usb_port *right)
left->peer = NULL;
}
-/* set the default peer port for root hubs */
+/*
+ * Set the default peer port for root hubs, or via the upstream peer
+ * relationship for all other hubs
+ */
static void find_and_link_peer(struct usb_hub *hub, int port1)
{
struct usb_port *port_dev = hub->ports[port1 - 1], *peer;
struct usb_device *hdev = hub->hdev;
+ struct usb_device *peer_hdev;
+ struct usb_hub *peer_hub;
if (!hdev->parent) {
- struct usb_hub *peer_hub;
- struct usb_device *peer_hdev;
struct usb_hcd *hcd = bus_to_hcd(hdev->bus);
struct usb_hcd *peer_hcd = hcd->shared_hcd;
@@ -203,15 +206,28 @@ static void find_and_link_peer(struct usb_hub *hub, int
port1)
return;
peer_hdev = peer_hcd->self.root_hub;
- peer_hub = usb_hub_to_struct_hub(peer_hdev);
- if (!peer_hub || port1 > peer_hdev->maxchild)
+ } else {
+ struct usb_port *upstream;
+ struct usb_device *parent = hdev->parent;
+ struct usb_hub *parent_hub = usb_hub_to_struct_hub(parent);
+
+ if (!parent_hub)
return;
- peer = peer_hub->ports[port1 - 1];
+ upstream = parent_hub->ports[hdev->portnum - 1];
+ if (!upstream || !upstream->peer)
+ return;
- if (peer)
- link_peers(port_dev, peer);
+ peer_hdev = upstream->peer->child;
}
+
+ peer_hub = usb_hub_to_struct_hub(peer_hdev);
+ if (!peer_hub || port1 > peer_hdev->maxchild)
+ return;
+
+ peer = peer_hub->ports[port1 - 1];
+ if (peer)
+ link_peers(port_dev, peer);
}
int usb_hub_create_port_device(struct usb_hub *hub, int port1)
--
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 v10 04/19] usb: cleanup setting udev->removable from port_dev->connect_type
Once usb-acpi has set the port's connect type the usb_device's
->removable attribute can be set in the standard location
set_usb_port_removable().
This also changes behavior in the case where the firmware says that the
port connect type is unknown. In that case just use the default setting
determined from the hub descriptor.
Note, we no longer pass udev->portnum to acpi_find_child_device() in the
root hub case since:
1/ the usb-core sets this to zero
2/ acpi always expects zero
...just pass zero.
Suggested-by: Alan Stern
Acked-by: Alan Stern
Signed-off-by: Dan Williams
---
drivers/usb/core/hub.c | 22 +-
drivers/usb/core/usb-acpi.c | 34 ++
2 files changed, 23 insertions(+), 33 deletions(-)
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 143b2e68af02..b387ebfe25bc 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2305,6 +2305,22 @@ static void set_usb_port_removable(struct usb_device
*udev)
udev->removable = USB_DEVICE_REMOVABLE;
else
udev->removable = USB_DEVICE_FIXED;
+
+ /*
+* Platform firmware may have populated an alternative value for
+* removable. If the parent port has a known connect_type use
+* that instead.
+*/
+ switch (hub->ports[udev->portnum - 1]->connect_type) {
+ case USB_PORT_CONNECT_TYPE_HOT_PLUG:
+ udev->removable = USB_DEVICE_REMOVABLE;
+ break;
+ case USB_PORT_CONNECT_TYPE_HARD_WIRED:
+ udev->removable = USB_DEVICE_FIXED;
+ break;
+ default: /* use what was set above */
+ break;
+ }
}
/**
@@ -2374,11 +2390,7 @@ int usb_new_device(struct usb_device *udev)
device_enable_async_suspend(&udev->dev);
- /*
-* check whether the hub marks this port as non-removable. Do it
-* now so that platform-specific data can override it in
-* device_add()
-*/
+ /* check whether the hub or firmware marks this port as non-removable */
if (udev->parent)
set_usb_port_removable(udev);
diff --git a/drivers/usb/core/usb-acpi.c b/drivers/usb/core/usb-acpi.c
index f91ef0220066..d3e7e1b4125e 100644
--- a/drivers/usb/core/usb-acpi.c
+++ b/drivers/usb/core/usb-acpi.c
@@ -136,8 +136,8 @@ out:
static struct acpi_device *usb_acpi_find_companion(struct device *dev)
{
- int port1;
struct usb_device *udev;
+ struct acpi_device *adev;
acpi_handle *parent_handle;
/*
@@ -155,40 +155,18 @@ static struct acpi_device *usb_acpi_find_companion(struct
device *dev)
*/
if (is_usb_device(dev)) {
udev = to_usb_device(dev);
- port1 = udev->portnum;
- if (udev->parent) {
- struct usb_hub *hub;
-
- hub = usb_hub_to_struct_hub(udev->parent);
- /*
-* According usb port's connect type to set usb device's
-* removability.
-*/
- switch (hub->ports[port1 - 1]->connect_type) {
- case USB_PORT_CONNECT_TYPE_HOT_PLUG:
- udev->removable = USB_DEVICE_REMOVABLE;
- break;
- case USB_PORT_CONNECT_TYPE_HARD_WIRED:
- udev->removable = USB_DEVICE_FIXED;
- break;
- default:
- udev->removable = USB_DEVICE_REMOVABLE_UNKNOWN;
- break;
- }
-
+ if (udev->parent)
return NULL;
- }
- /* root hub's parent is the usb hcd. */
- return acpi_find_child_device(ACPI_COMPANION(dev->parent),
- port1, false);
+ /* root hub is only child (_ADR=0) under its parent, the HC */
+ adev = ACPI_COMPANION(dev->parent);
+ return acpi_find_child_device(adev, 0, false);
} else if (is_usb_port(dev)) {
struct usb_port *port_dev = to_usb_port(dev);
- struct acpi_device *adev = NULL;
+ int port1 = port_dev->portnum;
/* Get the struct usb_device point of port's hub */
udev = to_usb_device(dev->parent->parent);
- port1 = port_dev->portnum;
/*
* The root hub ports' parent is the root hub. The non-root-hub
--
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 v10 00/19] port power control reworks
Greg,
Here is the port power control rework for your consideration now that
Alan has finished acking it (see summary below). Patch 9 had one
outstanding minor question from Alan that I believe I have addressed.
Patch 16 has a checkpatch warning "WARNING: msleep < 20ms can sleep for
up to 20ms", but it's flagging old code that simply moved and a 20ms
timeout is acceptable.
Changes since v9 [1]:
* Reworked the forced wakeup mechanism in patch 17 to use
pm_request_resume()
* Random collection of fixlets and rewordings suggested by Alan.
=
This series addresses the following disconnect scenarios when attempting
to use port power-off.
1/ Superspeed devices downgrade to their hispeed connection when
rx-detection fails on the superspeed pins. Address this by
preventing superspeed port poweroff/suspension until the peer port is
suspended, and powering on the superspeed port before the hispeed
port is resumed. This depends on the ability to identify peer ports
(patches 5-8, and patch 10 implements the policy).
2/ khubd prematurely disconnects ports that are in the process of being
resumed or reset. After this series khubd will ignore ports in the
pm-runtime-suspended or suspending state, or force active ports in the
resuming state (patch 14). While it is running it holds a new port
status lock to synchronize the port status changes of
usb_port_{suspend|resume} and usb_reset_and_verify_device() (patch 15).
3/ Superspeed devices fail to reconnect after a 2 second timeout This
event has two causes:
3.1/ Repeated {Set|Clear}PortFeature(PORT_POWER) toggles results in
the the device switching to its hispeed connection (due to
perceived instability of the superspeed connection). Address
this by arranging for the child device to be woken up
and complete a resume cycle when the parent port resumes (patch 17).
3.2/ Devices may require a warm reset when recovering the power
session. When the child device is woken up per above and the
port timed out on reconnect, force a warm-reset during the
child's reset-resume (patch 18).
[1]: v9: http://marc.info/?l=linux-usb&m=139952735227163&w=2
---
ACK => Acked-by: Alan Stern
ack => Acked-by another reviewer
ACK [01/19] USB: mutual exclusion for resetting a hub and power-managing a port
ACK [02/19] usb: disable port power control if not supported in
wHubCharacteristics
ACK [03/19] usb: rename usb_port device objects
ACK [04/19] usb: cleanup setting udev->removable from port_dev->connect_type
ACK [05/19] usb: assign default peer ports for root hubs
ACK [06/19] usb: assign usb3 external hub port peers
ACK [07/19] usb: find internal hub tier mismatch via acpi
ACK [08/19] usb: sysfs link peer ports
[09/19] usb: make usb_port flags atomic, rename did_runtime_put to
child_usage
ACK [10/19] usb: block suspension of superspeed port while hispeed peer is
active
ACK [11/19] usb: don't clear FEAT_C_ENABLE on usb_port_runtime_resume failure
ACK [12/19] usb: usb3 ports do not support FEAT_C_ENABLE
ACK [13/19] usb: refactor port handling in hub_events()
ACK [14/19] usb: synchronize port poweroff and khubd
ACK [15/19] usb: introduce port status lock
ACK [16/19] usb: hub_handle_remote_wakeup() depends on CONFIG_PM_RUNTIME=y
ACK [17/19] usb: resume child device when port is powered on
ack [18/19] usb: force warm reset to break link re-connect livelock
[19/19] usb: documentation for usb port power off mechanisms
Alan Stern (1):
USB: mutual exclusion for resetting a hub and power-managing a port
Dan Williams (17):
usb: disable port power control if not supported in wHubCharacteristics
usb: rename usb_port device objects
usb: cleanup setting udev->removable from port_dev->connect_type
usb: assign default peer ports for root hubs
usb: assign usb3 external hub port peers
usb: find internal hub tier mismatch via acpi
usb: sysfs link peer ports
usb: make usb_port flags atomic, rename did_runtime_put to child_usage
usb: block suspension of superspeed port while hispeed peer is active
usb: don't clear FEAT_C_ENABLE on usb_port_runtime_resume failure
usb: usb3 ports do not support FEAT_C_ENABLE
usb: refactor port handling in hub_events()
usb: synchronize port poweroff and khubd
usb: introduce port status lock
usb: hub_handle_remote_wakeup() depends on CONFIG_PM_RUNTIME=y
usb: resume child device when port is powered on
usb: force warm reset to break link re-connect livelock
Lan Tianyu (1):
usb: documentation for usb port power off mechanisms
Documentation/usb/power-management.txt | 242
drivers/usb/core/hcd.c | 45 +
drivers/usb/core/hub.c | 965 +---
drivers/usb/core/hub.h | 43 +
drivers/usb/core/port.c| 310 +-
drivers/usb/core/usb-acpi.c
[PATCH v10 15/19] usb: introduce port status lock
In general we do not want khubd to act on port status changes that are
the result of in progress resets or USB runtime PM operations.
Specifically port power control testing has been able to trigger an
unintended disconnect in hub_port_connect_change(), paraphrasing:
if ((portstatus & USB_PORT_STAT_CONNECTION) && udev &&
udev->state != USB_STATE_NOTATTACHED) {
if (portstatus & USB_PORT_STAT_ENABLE) {
/* Nothing to do */
} else if (udev->state == USB_STATE_SUSPENDED &&
udev->persist_enabled) {
...
} else {
/* Don't resuscitate */;
}
}
...by falling to the "Don't resuscitate" path or missing
USB_PORT_STAT_CONNECTION because usb_port_resume() was in the middle of
modifying the port status.
So, we want a new lock to hold off khubd for a given port while the
child device is being suspended, resumed, or reset. The lock ordering
rules are now usb_lock_device() => usb_lock_port(). This is mandated by
the device core which may hold the device_lock on the usb_device before
invoking usb_port_{suspend|resume} which in turn take the status_lock on
the usb_port. We attempt to hold the status_lock for the duration of a
port_event() run, and drop/re-acquire it when needing to take the
device_lock. The lock is also dropped/re-acquired during
hub_port_reconnect().
This patch also deletes hub->busy_bits as all use cases are now covered
by port PM runtime synchronization or the port->status_lock and it
pushes down usb_device_lock() into usb_remote_wakeup().
Acked-by: Alan Stern
Signed-off-by: Dan Williams
---
drivers/usb/core/hcd.c |2 -
drivers/usb/core/hub.c | 97 +--
drivers/usb/core/hub.h |4 +-
drivers/usb/core/port.c |6 ---
4 files changed, 72 insertions(+), 37 deletions(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 0fcf6bc0a4ef..75b241a37539 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -2266,9 +2266,7 @@ static void hcd_resume_work(struct work_struct *work)
struct usb_hcd *hcd = container_of(work, struct usb_hcd, wakeup_work);
struct usb_device *udev = hcd->self.root_hub;
- usb_lock_device(udev);
usb_remote_wakeup(udev);
- usb_unlock_device(udev);
}
/**
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 847271418cca..af68efdd48fc 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -2781,6 +2781,20 @@ static int port_is_power_on(struct usb_hub *hub,
unsigned portstatus)
return ret;
}
+static void usb_lock_port(struct usb_port *port_dev)
+ __acquires(&port_dev->status_lock)
+{
+ mutex_lock(&port_dev->status_lock);
+ __acquire(&port_dev->status_lock);
+}
+
+static void usb_unlock_port(struct usb_port *port_dev)
+ __releases(&port_dev->status_lock)
+{
+ mutex_unlock(&port_dev->status_lock);
+ __release(&port_dev->status_lock);
+}
+
#ifdef CONFIG_PM
/* Check if a port is suspended(USB2.0 port) or in U3 state(USB3.0 port) */
@@ -3003,6 +3017,8 @@ int usb_port_suspend(struct usb_device *udev,
pm_message_t msg)
int status;
boolreally_suspend = true;
+ usb_lock_port(port_dev);
+
/* enable remote wakeup when appropriate; this lets the device
* wake up the upstream hub (including maybe the root hub).
*
@@ -3096,6 +3112,8 @@ int usb_port_suspend(struct usb_device *udev,
pm_message_t msg)
pm_runtime_put_sync(&port_dev->dev);
usb_mark_last_busy(hub->hdev);
+
+ usb_unlock_port(port_dev);
return status;
}
@@ -3244,13 +3262,13 @@ int usb_port_resume(struct usb_device *udev,
pm_message_t msg)
}
}
+ usb_lock_port(port_dev);
+
/* Skip the initial Clear-Suspend step for a remote wakeup */
status = hub_port_status(hub, port1, &portstatus, &portchange);
if (status == 0 && !port_is_suspended(hub, portstatus))
goto SuspendCleared;
- set_bit(port1, hub->busy_bits);
-
/* see 7.1.7.7; affects power usage, but not budgeting */
if (hub_is_superspeed(hub->hdev))
status = hub_set_port_link_state(hub, port1, USB_SS_PORT_LS_U0);
@@ -3289,8 +3307,6 @@ int usb_port_resume(struct usb_device *udev, pm_message_t
msg)
}
}
- clear_bit(port1, hub->busy_bits);
-
status = check_port_resume_type(udev,
hub, port1, status, portchange, portstatus);
if (status == 0)
@@ -3308,16 +3324,18 @@ int usb_port_resume(struct usb_device *udev,
pm_message_t msg)
usb_unlocked_enable_lpm(udev);
}
+ usb_unlock_port(port_dev);
+
return status;
}
#ifdef CONFIG_PM_RUNTIME
-/* caller has
Re: new driver Fresco Logic
Hi Greg, Mike has the udlfb module, the Fresco does not work with it. Hopefully he'll get a solution via the linux-media list as suggested. Regards Sid. On 20/05/14 21:31, Greg KH wrote: On Tue, May 20, 2014 at 08:07:43AM -0700, Michael Durkin wrote: Forgive my intrusion ... Im hoping to get a driver made for a Fresco Logic FL2000 USB to VGA adapter ... Device ID 1d5c:2000 What type of chip is in this device? If it is a USB 3 device, odds are that Linux doesn't support it yet as DeviceLink isn't providing the spec for the chip to anyone. Can you ask the company you bought the device from for how to get the specs for it? 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 -- Sid Boyce ... Hamradio License G3VBV, Licensed Private Pilot Emeritus IBM/Amdahl Mainframes and Sun/Fujitsu Servers Tech Support Senior Staff Specialist, Cricket Coach Microsoft Windows Free Zone - Linux used for all Computing Tasks -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/10] xhci: 'noxhci_port_switch' kernel parameter
On Tue, May 20, 2014 at 03:40:16PM -0700, Dan Williams wrote: > On Tue, May 20, 2014 at 1:34 PM, Greg KH wrote: > > On Tue, May 20, 2014 at 11:25:37AM -0700, Dan Williams wrote: > >> On Tue, May 20, 2014 at 2:51 AM, Takashi Iwai wrote: > >> > At Tue, 20 May 2014 12:47:36 +0300, > >> > Mathias Nyman wrote: > >> >> > >> >> On 05/20/2014 04:01 AM, Greg KH wrote: > >> >> > On Thu, May 08, 2014 at 07:25:55PM +0300, Mathias Nyman wrote: > >> >> >> From: Dan Williams > >> >> >> > >> >> >> Add a command line switch for disabling ehci port switchover. Useful > >> >> >> for working around / debugging xhci incompatibilities where ehci > >> >> >> operation is available. > >> >> >> > >> >> >> Reference: http://marc.info/?l=linux-usb&m=138920063001509&w=2 > >> >> >> > >> >> >> Cc: Sarah Sharp > >> >> >> Cc: Mathias Nyman > >> >> >> Cc: Holger Hans Peter Freyther > >> >> >> Suggested-by: Alan Stern > >> >> >> Signed-off-by: Dan Williams > >> >> >> Signed-off-by: Mathias Nyman > >> >> >> --- > >> >> >> Documentation/kernel-parameters.txt | 3 +++ > >> >> >> drivers/usb/host/pci-quirks.c | 15 +-- > >> >> >> 2 files changed, 16 insertions(+), 2 deletions(-) > >> >> >> > >> >> >> diff --git a/Documentation/kernel-parameters.txt > >> >> >> b/Documentation/kernel-parameters.txt > >> >> >> index 4384217..fc3403114 100644 > >> >> >> --- a/Documentation/kernel-parameters.txt > >> >> >> +++ b/Documentation/kernel-parameters.txt > >> >> >> @@ -2251,6 +2251,9 @@ bytes respectively. Such letter suffixes can > >> >> >> also be entirely omitted. > >> >> >> > >> >> >>nox2apic[X86-64,APIC] Do not enable x2APIC mode. > >> >> >> > >> >> >> + noxhci_port_switch > >> >> >> + [USB] Use EHCI instead of XHCI where available > >> >> >> + > >> >> > > >> >> > Ugh, I really don't like new command line options. > >> >> > > >> >> > Especially one that isn't very well documented. Why would someone > >> >> > want > >> >> > to enable this? What problem is it solving? Can we detect this > >> >> > automatically and do it for the user? Is this just for prototype > >> >> > hardware that has not shipped? What hardware needs this? > >> >> > > >> >> > I need a whole lot more documentation at the very least before I will > >> >> > apply this. > >> >> > > >> >> > >> >> On Intel hardware with both ehci and xhci controllers we can select if > >> >> a usb2 port > >> >> is controlled by ehci or xhci. This capability can be checked from > >> >> Intel xhci pci > >> >> config space. Xhci driver checks this on boot and switches over the > >> >> supported ports. > >> >> > >> >> This is a feature in Intel Panther point and later chipsets, in shipped > >> >> hardware. > >> >> Its working quite well in most cases, but sometimes vendors claim they > >> >> support > >> >> switchover, but then forget to connect some wires, and the usb2 port > >> >> ends up dead > >> >> after switching. > >> >> > >> >> A recently found case is the Sony VAIO T-series. (I'll send you a > >> >> different patch > >> >> for that shortly) > >> >> http://marc.info/?l=linux-usb&m=139993106029340&w=2 > >> >> > >> >> This is the extreme case that the usb2 ports appears completely dead. > >> >> Other reasons are that some devices might work better under ehci than > >> >> xhci, > >> >> and users want to enforce the ehci opton. For powermanagement > >> >> developers it's nice > >> >> to disable switchover as it turns out some hardware are quirky with port > >> >> switchover and suspend/resume. (might need to turn port back to ehci > >> >> before > >> >> suspending). > >> >> > >> >> I don't think we can detect this automatically. > >> >> > >> >> Dan, can you add more documentation to this patch? > >> > > >> > While we're at it: can we implement a bitmask instead? > >> > > >> > We've seen lots of HP laptops having Webcams or BT devices that don't > >> > work XHCI but only with EHCI. For making them working properly, the > >> > specific xhci ports have to be disabled. But, we don't want to kill > >> > the whole XHCI at all. The single boolean option doesn't work for > >> > such a case. > >> > > >> > >> I'm not sure we want to make this more complex. Ideally this is just > >> a stop-gap measure for users to workaround incompatibilities in xhci > >> while the xhci fix is identified. > > > > We can't use a kernel command line option as a "stop-gap", sorry. Let's > > just fix the real problem here. > > > > Greg, > > Sorry, I don't think it is fair to users to force them to re-compile > their kernel to get their device to work. I totally agree. > Granted, I'm new to USB > development, but the rate of reports of endpoint devices that mess up > and require quirks in the hcd-driver or usb-core seems un-ending to > me. So, I don't think it is fair to expect that the tide of quirky > devices will be stemmed in any reasonable amount of time. Having a > "works with noxhci_port_switch" report from users is good data (hmm, I > think a
Re: [PATCH net-next] net: cdc_ncm: fix 64bit division build error
Hi all, On Mon, 19 May 2014 09:21:09 +0200 Bjørn Mork wrote: > > The upper timer_interval limit is arbitrary and much higher > than anything usable in the real world. Reducing it from 15s > to ~4s to make the timer_interval fit in an u32 does not make > much difference. The limit is still outside the practical > bounds. > > This eliminates the need for a 64bit timer_interval, fixing a > build error related to 64bit division: > > drivers/built-in.o: In function `cdc_ncm_get_coalesce': > ak8975.c:(.text+0x1ac994): undefined reference to `__aeabi_uldivmod' > > Reported-by: Stephen Rothwell > Signed-off-by: Bjørn Mork I have applied this as a fix patch to linux-next today. -- Cheers, Stephen Rothwells...@canb.auug.org.au signature.asc Description: PGP signature
[PATCH 3/5] usb: gadget: m66592-udc: should not call gadget driver's .unbind
It has already been covered by udc core Signed-off-by: Peter Chen --- drivers/usb/gadget/m66592-udc.c |2 -- 1 files changed, 0 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/m66592-udc.c b/drivers/usb/gadget/m66592-udc.c index 5396b24..2dc37d1 100644 --- a/drivers/usb/gadget/m66592-udc.c +++ b/drivers/usb/gadget/m66592-udc.c @@ -1492,8 +1492,6 @@ static int m66592_udc_stop(struct usb_gadget *g, m66592_bclr(m66592, M66592_VBSE | M66592_URST, M66592_INTENB0); - driver->unbind(&m66592->gadget); - init_controller(m66592); disable_controller(m66592); -- 1.7.8 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 5/5] usb: gadget: omap_udc: should not call gadget driver's .unbind
It has already been covered by udc core
Signed-off-by: Peter Chen
---
drivers/usb/gadget/omap_udc.c |5 +
1 files changed, 1 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/gadget/omap_udc.c b/drivers/usb/gadget/omap_udc.c
index 2ae4f6d..e731373 100644
--- a/drivers/usb/gadget/omap_udc.c
+++ b/drivers/usb/gadget/omap_udc.c
@@ -2079,10 +2079,7 @@ static int omap_udc_start(struct usb_gadget *g,
&udc->gadget);
if (status < 0) {
ERR("can't bind to transceiver\n");
- if (driver->unbind) {
- driver->unbind(&udc->gadget);
- udc->driver = NULL;
- }
+ udc->driver = NULL;
goto done;
}
} else {
--
1.7.8
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/5] usb: gadget: fusb300_udc: should not call gadget driver's .unbind
It has already been covered by udc core
Signed-off-by: Peter Chen
---
drivers/usb/gadget/fusb300_udc.c |2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/fusb300_udc.c b/drivers/usb/gadget/fusb300_udc.c
index 90d2836..e882d41 100644
--- a/drivers/usb/gadget/fusb300_udc.c
+++ b/drivers/usb/gadget/fusb300_udc.c
@@ -1325,8 +1325,6 @@ static int fusb300_udc_stop(struct usb_gadget *g,
{
struct fusb300 *fusb300 = to_fusb300(g);
- driver->unbind(&fusb300->gadget);
-
init_controller(fusb300);
fusb300->driver = NULL;
--
1.7.8
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/5] usb: gadget: fsl_udc_core: should not call gadget driver's .unbind
It has already been covered by udc core
Signed-off-by: Peter Chen
---
drivers/usb/gadget/fsl_udc_core.c |1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/gadget/fsl_udc_core.c
b/drivers/usb/gadget/fsl_udc_core.c
index dd9ff29..0ae3705 100644
--- a/drivers/usb/gadget/fsl_udc_core.c
+++ b/drivers/usb/gadget/fsl_udc_core.c
@@ -1957,7 +1957,6 @@ static int fsl_udc_start(struct usb_gadget *g,
&udc_controller->gadget);
if (retval < 0) {
ERR("can't bind to transceiver\n");
- driver->unbind(&udc_controller->gadget);
udc_controller->driver = 0;
return retval;
}
--
1.7.8
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/5] usb: gadget: cleanup gadget driver .unbind usage at udc driver
Hi Felipe, This patch set cleans up unnecessary .unbind calling at udc driver, the udc core covers gadget driver's .unbind calling well. Peter Chen (5): usb: gadget: fsl_udc_core: should not call gadget driver's .unbind usb: gadget: fusb300_udc: should not call gadget driver's .unbind usb: gadget: m66592-udc: should not call gadget driver's .unbind usb: gadget: net2272: do not need to judge gadget driver's .unbind usb: gadget: omap_udc: should not call gadget driver's .unbind drivers/usb/gadget/fsl_udc_core.c |1 - drivers/usb/gadget/fusb300_udc.c |2 -- drivers/usb/gadget/m66592-udc.c |2 -- drivers/usb/gadget/net2272.c |2 +- drivers/usb/gadget/omap_udc.c |5 + 5 files changed, 2 insertions(+), 10 deletions(-) -- 1.7.8 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] usb: gadget: net2272: do not need to judge gadget driver's .unbind
It has already been covered by udc core, besides, we do not need unbind at .udc_start Signed-off-by: Peter Chen --- drivers/usb/gadget/net2272.c |2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/drivers/usb/gadget/net2272.c b/drivers/usb/gadget/net2272.c index ca15405..059cfe5 100644 --- a/drivers/usb/gadget/net2272.c +++ b/drivers/usb/gadget/net2272.c @@ -1453,7 +1453,7 @@ static int net2272_start(struct usb_gadget *_gadget, struct net2272 *dev; unsigned i; - if (!driver || !driver->unbind || !driver->setup || + if (!driver || !driver->setup || driver->max_speed != USB_SPEED_HIGH) return -EINVAL; -- 1.7.8 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH net-next] net: cdc_ncm: fix 64bit division build error
From: Bjørn Mork Date: Mon, 19 May 2014 09:21:09 +0200 > The upper timer_interval limit is arbitrary and much higher > than anything usable in the real world. Reducing it from 15s > to ~4s to make the timer_interval fit in an u32 does not make > much difference. The limit is still outside the practical > bounds. > > This eliminates the need for a 64bit timer_interval, fixing a > build error related to 64bit division: > > drivers/built-in.o: In function `cdc_ncm_get_coalesce': > ak8975.c:(.text+0x1ac994): undefined reference to `__aeabi_uldivmod' > > Reported-by: Stephen Rothwell > Signed-off-by: Bjørn Mork Applied, thank you. -- 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] usb: chipidea: using one inline function to cover queue work operations
The otg queue work include operations: one is disable interrupt,
another one is call kernel queue work API. Many codes do this
operation, using one inline function to instead of them.
Signed-off-by: Peter Chen
---
drivers/usb/chipidea/core.c|6 +--
drivers/usb/chipidea/otg.h |5 +++
drivers/usb/chipidea/otg_fsm.c | 69 +--
3 files changed, 30 insertions(+), 50 deletions(-)
diff --git a/drivers/usb/chipidea/core.c b/drivers/usb/chipidea/core.c
index 95b4dd7..619d13e 100644
--- a/drivers/usb/chipidea/core.c
+++ b/drivers/usb/chipidea/core.c
@@ -425,8 +425,7 @@ static irqreturn_t ci_irq(int irq, void *data)
ci->id_event = true;
/* Clear ID change irq status */
hw_write_otgsc(ci, OTGSC_IDIS, OTGSC_IDIS);
- disable_irq_nosync(ci->irq);
- queue_work(ci->wq, &ci->work);
+ ci_otg_queue_work(ci);
return IRQ_HANDLED;
}
@@ -438,8 +437,7 @@ static irqreturn_t ci_irq(int irq, void *data)
ci->b_sess_valid_event = true;
/* Clear BSV irq */
hw_write_otgsc(ci, OTGSC_BSVIS, OTGSC_BSVIS);
- disable_irq_nosync(ci->irq);
- queue_work(ci->wq, &ci->work);
+ ci_otg_queue_work(ci);
return IRQ_HANDLED;
}
diff --git a/drivers/usb/chipidea/otg.h b/drivers/usb/chipidea/otg.h
index 7349267..9ecb598 100644
--- a/drivers/usb/chipidea/otg.h
+++ b/drivers/usb/chipidea/otg.h
@@ -17,5 +17,10 @@ int ci_hdrc_otg_init(struct ci_hdrc *ci);
void ci_hdrc_otg_destroy(struct ci_hdrc *ci);
enum ci_role ci_otg_role(struct ci_hdrc *ci);
void ci_handle_vbus_change(struct ci_hdrc *ci);
+static inline void ci_otg_queue_work(struct ci_hdrc *ci)
+{
+ disable_irq_nosync(ci->irq);
+ queue_work(ci->wq, &ci->work);
+}
#endif /* __DRIVERS_USB_CHIPIDEA_OTG_H */
diff --git a/drivers/usb/chipidea/otg_fsm.c b/drivers/usb/chipidea/otg_fsm.c
index 8d4c33d..caaabc5 100644
--- a/drivers/usb/chipidea/otg_fsm.c
+++ b/drivers/usb/chipidea/otg_fsm.c
@@ -84,8 +84,7 @@ set_a_bus_req(struct device *dev, struct device_attribute
*attr,
ci->fsm.a_bus_req = 1;
}
- disable_irq_nosync(ci->irq);
- queue_work(ci->wq, &ci->work);
+ ci_otg_queue_work(ci);
mutex_unlock(&ci->fsm.lock);
return count;
@@ -125,8 +124,7 @@ set_a_bus_drop(struct device *dev, struct device_attribute
*attr,
ci->fsm.a_bus_req = 0;
}
- disable_irq_nosync(ci->irq);
- queue_work(ci->wq, &ci->work);
+ ci_otg_queue_work(ci);
mutex_unlock(&ci->fsm.lock);
return count;
@@ -165,8 +163,7 @@ set_b_bus_req(struct device *dev, struct device_attribute
*attr,
else if (buf[0] == '1')
ci->fsm.b_bus_req = 1;
- disable_irq_nosync(ci->irq);
- queue_work(ci->wq, &ci->work);
+ ci_otg_queue_work(ci);
mutex_unlock(&ci->fsm.lock);
return count;
@@ -186,8 +183,7 @@ set_a_clr_err(struct device *dev, struct device_attribute
*attr,
if (buf[0] == '1')
ci->fsm.a_clr_err = 1;
- disable_irq_nosync(ci->irq);
- queue_work(ci->wq, &ci->work);
+ ci_otg_queue_work(ci);
mutex_unlock(&ci->fsm.lock);
return count;
@@ -297,8 +293,7 @@ static void set_tmout_and_fsm(void *ptr, unsigned long
indicator)
set_tmout(ci, indicator);
- disable_irq_nosync(ci->irq);
- queue_work(ci->wq, &ci->work);
+ ci_otg_queue_work(ci);
}
static void a_wait_vfall_tmout_func(void *ptr, unsigned long indicator)
@@ -312,8 +307,7 @@ static void a_wait_vfall_tmout_func(void *ptr, unsigned
long indicator)
hw_write_otgsc(ci, OTGSC_DPIS, OTGSC_DPIS);
/* Enable data pulse irq */
hw_write_otgsc(ci, OTGSC_DPIE, OTGSC_DPIE);
- disable_irq_nosync(ci->irq);
- queue_work(ci->wq, &ci->work);
+ ci_otg_queue_work(ci);
}
static void b_ase0_brst_tmout_func(void *ptr, unsigned long indicator)
@@ -324,8 +318,7 @@ static void b_ase0_brst_tmout_func(void *ptr, unsigned long
indicator)
if (!hw_read_otgsc(ci, OTGSC_BSV))
ci->fsm.b_sess_vld = 0;
- disable_irq_nosync(ci->irq);
- queue_work(ci->wq, &ci->work);
+ ci_otg_queue_work(ci);
}
static void b_ssend_srp_tmout_func(void *ptr, unsigned long indicator)
@@ -335,10 +328,8 @@ static void b_ssend_srp_tmout_func(void *ptr, unsigned
long indicator)
set_tmout(ci, indicator);
/* only vbus fall below B_sess_vld in b_idle state */
- if (ci->transceiver->state == OTG_STATE_B_IDLE) {
- disable_irq_nosync(ci->irq);
- queue_work(ci->wq, &ci->work);
- }
+ if (ci->transceiver->state == OTG_STATE_B_IDLE)
+ ci_otg_queue_work(ci);
}
static void b_sess_vld_tmout_func(void *ptr, unsigned long indicator)
@@ -349,8
Re: [PATCH 02/10] xhci: 'noxhci_port_switch' kernel parameter
On Tue, May 20, 2014 at 5:27 PM, Greg KH wrote: >> Greg, >> >> Sorry, I don't think it is fair to users to force them to re-compile >> their kernel to get their device to work. > > I totally agree. > >> Granted, I'm new to USB >> development, but the rate of reports of endpoint devices that mess up >> and require quirks in the hcd-driver or usb-core seems un-ending to >> me. So, I don't think it is fair to expect that the tide of quirky >> devices will be stemmed in any reasonable amount of time. Having a >> "works with noxhci_port_switch" report from users is good data (hmm, I >> think a printk to tell users to file a report upstream if the option >> resolves their issue is needed). > > How about just adding a debugfs file instead? That way, once you fix > this, we can then remove it and no one will care. The only thing stopping me from saying "deal." is that this darn things is presently a pci quirk. So it happens well before the user has a chance to manually override it with a debugfs file. Let me look into delaying the quirk until the driver loads, because ideally the right interface for this is "blacklist xhci_hcd" in /etc/modprobe.conf. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 02/10] xhci: 'noxhci_port_switch' kernel parameter
On Tue, May 20, 2014 at 11:21:03PM -0700, Dan Williams wrote: > On Tue, May 20, 2014 at 5:27 PM, Greg KH wrote: > >> Greg, > >> > >> Sorry, I don't think it is fair to users to force them to re-compile > >> their kernel to get their device to work. > > > > I totally agree. > > > >> Granted, I'm new to USB > >> development, but the rate of reports of endpoint devices that mess up > >> and require quirks in the hcd-driver or usb-core seems un-ending to > >> me. So, I don't think it is fair to expect that the tide of quirky > >> devices will be stemmed in any reasonable amount of time. Having a > >> "works with noxhci_port_switch" report from users is good data (hmm, I > >> think a printk to tell users to file a report upstream if the option > >> resolves their issue is needed). > > > > How about just adding a debugfs file instead? That way, once you fix > > this, we can then remove it and no one will care. > > The only thing stopping me from saying "deal." is that this darn > things is presently a pci quirk. So it happens well before the user > has a chance to manually override it with a debugfs file. Then have the debugfs file disconnect the device and reconnect it. > Let me look into delaying the quirk until the driver loads, because > ideally the right interface for this is "blacklist xhci_hcd" in > /etc/modprobe.conf. Which really doesn't work for systems / distros that build the driver into 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
