Re: [PATCH 06/16] usb: musb: dsps: rename ti81xx_driver_data to am33xx_driver_data
Hi Sebastian, On 7/22/2013 11:39 PM, Sebastian Andrzej Siewior wrote: This patch renames the type struct from ti81xx_driver_data to am33xx_driver_data since it is not used for ti81xx anymore. The EOI member is also removed since the am33xx SoC does not have such register. The interrupt is acknowledged by writting into the stat register. AM335X TRM Section 16.6.5 and 16.7.5 describes about EOI registers. Its at offset 0x24. Or is it that the interrupts are acknowledged even without writing to eoi register? -- -George -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] TX throttling bug-fixing patch of AX88179_178A
On Mon, 2013-07-22 at 20:47 +0100, Ben Hutchings wrote: On Mon, 2013-07-22 at 11:47 -0700, Eric Dumazet wrote: On Mon, 2013-07-22 at 19:38 +0100, Ben Hutchings wrote: On Mon, 2013-07-22 at 11:29 -0700, Grant Grundler wrote: On Mon, Jul 22, 2013 at 10:07 AM, Eric Dumazet eric.duma...@gmail.com wrote: ... I guess that if a driver does not advertise NETIF_F_SG, this skb_linearize() call is not needed : All frames reaching your xmit function should already be linear As Ben Hutchings pointed out, hw_features is still setting this...but I'm not sure how that matters. ax88179_set_features() doesn't allow setting SG or TSO features. But I expect it would be not too difficult to add such that ethtool could set those features after boot. [...] It already can. That's what putting feature flags in hw_features does. My original concern, that inspired this patch, was to remove SG support, as this driver does not have SG support at all. Linearize a full TSO packet needs order-5 allocations, thats likely to fail and lead to very slow TCP performance, because it will only rely on retransmits. The driver could set gso_max_size to reduce that problem. But I rather doubt that TSO followed by skb_linearize() significantly improves throughput or CPU-efficiency. (If the device has a 1G link but is connected to the host through a USB 2.0 port, then USB is the bottleneck and TSO could improve throughput a few percent. But that's a silly configuration.) The real solution would be for someone to add SG support to the usbnet core. Trying to support 1GbE with only linear skbs is not a great idea... and it can only be a matter of time before there is USB ultra speed (or whatever comes after 'super') with 10GbE devices... This sounds a good idea. Is anybody working on adding SG to usbnet ? -- 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
a small patch that fixes the ohci warn irq nobody cared on shutdown.
Hi:
Here's a small patch that fixes the ohci shutdown.
The patch is against v3.4.54.
From cc16ef2f15300201cc3f680b7df20ecded28daa4 Mon Sep 17 00:00:00 2001
From: Cai Zhiyong caizhiy...@huawei.com
Date: Tue, 23 Jul 2013 12:17:01 +0800
Subject: [PATCH] USB: ohci_usb warn irq nobody cared on shutdown
When ohci-hcd is shutting down, call ohci_usb_reset reset ohci-hcd,
the root hub generate an interrupt, but ohci-rh_state is OHCI_RH_HALTED,
and ohci_irq ignore the interrupt, the kernel trigger warning irq nobody
cared.
This patch disable ohci interrupt before reset ohci.
The patch is tested at the arm cortex-a9 demo board.
Signed-off-by: Cai Zhiyong caizhiy...@huawei.com
---
drivers/usb/host/ohci-hcd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 235171f..3dfeed1 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -895,8 +895,8 @@ static void ohci_stop (struct usb_hcd *hcd)
if (quirk_nec(ohci))
flush_work_sync(ohci-nec_work);
- ohci_usb_reset (ohci);
ohci_writel (ohci, OHCI_INTR_MIE, ohci-regs-intrdisable);
+ ohci_usb_reset (ohci);
free_irq(hcd-irq, hcd);
hcd-irq = 0;
--
1.8.1.5
Looking forward to your feedback.
Best regards,
Cai Zhiyong.
http://www.huawei.com
本邮件及其附件含有华为公司的保密信息，仅限于发送给上面地址中列出的个人或群组。禁
止任何其他人以任何形式使用（包括但不限于全部或部分地泄露、复制、或散发）本邮件中
的信息。如果您错收了本邮件，请您立即电话或邮件通知发件人并删除本邮件！
This e-mail and its attachments contain confidential information from HUAWEI,
which
is intended only for the person or entity whose address is listed above. Any
use of the
information contained herein in any way (including, but not limited to, total
or partial
disclosure, reproduction, or dissemination) by persons other than the intended
recipient(s) is prohibited. If you receive this e-mail in error, please notify
the sender by
phone or email immediately and delete it!
N�Р骒r��yb�X�肚�v�^�)藓{.n�+�伐�{焙柒��^n�r■�z���h�ㄨ��Ⅷ �G���h�(�茛j���m赇z罐��帼f＂�h���~�m�
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
Hi Alan,
On Monday 22 of July 2013 10:44:39 Alan Stern wrote:
On Mon, 22 Jul 2013, Kishon Vijay Abraham I wrote:
The PHY and the controller it is attached to are both physical
devices.
The connection between them is hardwired by the system
manufacturer and cannot be changed by software.
PHYs are generally described by fixed system-specific board
files or by Device Tree information. Are they ever discovered
dynamically?
No. They are created just like any other platform devices are created.
Okay. Are PHYs _always_ platform devices?
They can be i2c, spi or any other device types as well.
Is the same true for the controllers attached to the PHYs?
If not -- if both a PHY and a controller are discovered
dynamically -- how does the kernel know whether they are
connected to each other?
No differences here. Both PHY and controller will have dt information
or hwmod data using which platform devices will be created.
The kernel needs to know which controller is attached to which
PHY. Currently this information is represented by name or ID
strings embedded in platform data.
right. It's embedded in the platform data of the controller.
It must also be embedded in the PHY's platform data somehow.
Otherwise, how would the kernel know which PHY to use?
By using a PHY lookup as Stephen and I suggested in our previous replies.
Without any extra data in platform data. (I have even posted a code
example.)
The PHY's driver (the supplier) uses the platform data to
construct a platform_device structure that represents the PHY.
Currently the driver assigns static labels (corresponding to the label
used in the platform data of the controller).
Until this is done, the controller's driver (the client) cannot
use the PHY.
right.
Since there is no parent-child relation between the PHY and the
controller, there is no guarantee that the PHY's driver will be
ready when the controller's driver wants to use it. A deferred
probe may be needed.
right.
The issue (or one of the issues) in this discussion is that
Greg does not like the idea of using names or IDs to associate
PHYs with controllers, because they are too prone to
duplications or other errors. Pointers are more reliable.
But pointers to what? Since the only data known to be
available to both the PHY driver and controller driver is the
platform data, the obvious answer is a pointer to platform data
(either for the PHY or for the controller, or maybe both).
hmm.. it's not going to be simple though as the platform device for
the PHY and controller can be created in entirely different places.
e.g., in some cases the PHY device is a child of some mfd core device
(the device will be created in drivers/mfd) and the controller driver
(usually) is created in board file. I guess then we have to come up
with something to share a pointer in two different files.
The ability for two different source files to share a pointer to a data
item defined in a third source file has been around since long before
the C language was invented. :-)
In this case, it doesn't matter where the platform_device structures
are created or where the driver source code is. Let's take a simple
example. Suppose the system design includes a PHY named foo. Then
the board file could contain:
struct phy_info { ... } phy_foo;
EXPORT_SYMBOL_GPL(phy_foo);
and a header file would contain:
extern struct phy_info phy_foo;
The PHY supplier could then call phy_create(phy_foo), and the PHY
client could call phy_find(phy_foo). Or something like that; make up
your own structure tags and function names.
It's still possible to have conflicts, but now two PHYs with the same
name (or a misspelled name somewhere) will cause an error at link time.
This is incorrect, sorry. First of all it's a layering violation - you
export random driver-specific symbols from one driver to another. Then
imagine 4 SoCs - A, B, C, D. There are two PHY types PHY1 and PHY2 and
there are two types of consumer drivers (e.g. USB host controllers). Now
consider following mapping:
SoC PHY consumer
A PHY1HOST1
B PHY1HOST2
C PHY2HOST1
D PHY2HOST2
So we have to be able to use any of the PHYs with any of the host drivers.
This means you would have to export symbol with the same name from both
PHY drivers, which obviously would not work in this case, because having
both drivers enabled (in a multiplatform aware configuration) would lead
to linking conflict.
Best regards,
Tomasz
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usb: dwc3: ep0: don't change to configured state too early
Hi,
On Mon, Jul 22, 2013 at 10:55:28AM -0400, Alan Stern wrote:
On Mon, 22 Jul 2013, Felipe Balbi wrote:
before changing to configured state, we need
to wait until gadget driver has had a chance
to process the request.
In case of USB_GADGET_DELAYED_STATUS, that means
we need to defer usb_gadget_set_state() until
the upcoming usb_ep_queue().
Reported-by: Alan Stern st...@rowland.harvard.edu
Signed-off-by: Felipe Balbi ba...@ti.com
---
drivers/usb/dwc3/ep0.c | 13 +++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 007651c..7fa93f4 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -148,6 +148,7 @@ static int __dwc3_gadget_ep0_queue(struct dwc3_ep *dep,
direction = !dwc-ep0_expect_in;
dwc-delayed_status = false;
+ usb_gadget_set_state(dwc-gadget, USB_STATE_CONFIGURED);
Isn't this overkill? Do you really want to call usb_gadget_set_state()
every time the gadget driver queues a transfer on ep0?
Or am I missing an important part of the context?
heh, you're missing context, that will only be called when we had
delayed status flag set:
| static int __dwc3_gadget_ep0_queue(struct dwc3_ep *dep,
| struct dwc3_request *req)
| {
[ ... ]
| /*
|* In case gadget driver asked us to delay the STATUS phase,
|* handle it here.
|*/
| if (dwc-delayed_status) {
| unsigneddirection;
|
| direction = !dwc-ep0_expect_in;
| dwc-delayed_status = false;
| usb_gadget_set_state(dwc-gadget, USB_STATE_CONFIGURED);
|
| if (dwc-ep0state == EP0_STATUS_PHASE)
| __dwc3_ep0_do_control_status(dwc, dwc-eps[direction]);
| else
| dev_dbg(dwc-dev, too early for delayed status\n);
|
| return 0;
| }
[ ... ]
| return 0;
| }
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH] usb: gadget: udc-core: move sysfs_notify() to a workqueue
Hi,
On Mon, Jul 22, 2013 at 10:53:50AM -0400, Alan Stern wrote:
On Mon, 22 Jul 2013, Felipe Balbi wrote:
indeed. Added a flush_work() call to usb_del_gadget_udc()
Also, what happens if two state transitions occur before the work queue
gets around to executing the work routine?
do we need to care about that at all ? It's a queue anyway, transitions
will still be notified in order.
The queue in workqueue doesn't refer to the individual calls for a
particular work_struct. If you call schedule_work(gadget-work) twice
in quick succession, the work routine is likely to get executed only
once.
I don't know if that would be a problem in this case -- the effect
would be that some state transitions would appear to be skipped over.
If nobody cares about anything but the current state, this would be
okay.
I don't think anyone should care about the state transitions themselves,
only current state. Unless we want to have some tool which makes sure
device transitions through all states properly...
@@ -311,6 +320,7 @@ found:
kobject_uevent(udc-dev.kobj, KOBJ_REMOVE);
device_unregister(udc-dev);
device_unregister(gadget-dev);
+ flush_work(gadget-work);
Shouldn't the flush_work() call come before device_unregister()? You
don't want to call sysfs_notify() on an unregistered device.
aaa, that's true. I was considering the possibility of someone adding a
usb_gadget_set_state() call on their -release() method, but I guess
that would be a much bigger bug then the one I ended up causing :-)
Here's a newer version:
commit 766ed104b6f420dc7587a63dc1679f78176d082e
Author: Felipe Balbi ba...@ti.com
Date: Wed Jul 17 11:09:49 2013 +0300
usb: gadget: udc-core: move sysfs_notify() to a workqueue
usb_gadget_set_state() will call sysfs_notify()
which might sleep. Some users might want to call
usb_gadget_set_state() from the very IRQ handler
which actually changes the gadget state.
Instead of having every UDC driver add their own
workqueue for such a simple notification, we're
adding it generically to our struct usb_gadget,
so the details are hidden from all UDC drivers.
Signed-off-by: Felipe Balbi ba...@ti.com
diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
index ffd8fa5..eed8503 100644
--- a/drivers/usb/gadget/udc-core.c
+++ b/drivers/usb/gadget/udc-core.c
@@ -23,6 +23,7 @@
#include linux/list.h
#include linux/err.h
#include linux/dma-mapping.h
+#include linux/workqueue.h
#include linux/usb/ch9.h
#include linux/usb/gadget.h
@@ -101,11 +102,18 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
/* - */
+static void usb_gadget_state_work(struct work_struct *work)
+{
+ struct usb_gadget *gadget = work_to_gadget(work);
+
+ sysfs_notify(gadget-dev.kobj, NULL, status);
+}
+
void usb_gadget_set_state(struct usb_gadget *gadget,
enum usb_device_state state)
{
gadget-state = state;
- sysfs_notify(gadget-dev.kobj, NULL, status);
+ schedule_work(gadget-work);
}
EXPORT_SYMBOL_GPL(usb_gadget_set_state);
@@ -192,6 +200,7 @@ int usb_add_gadget_udc_release(struct device *parent,
struct usb_gadget *gadget,
goto err1;
dev_set_name(gadget-dev, gadget);
+ INIT_WORK(gadget-work, usb_gadget_state_work);
gadget-dev.parent = parent;
dma_set_coherent_mask(gadget-dev, parent-coherent_dma_mask);
@@ -309,6 +318,7 @@ found:
usb_gadget_remove_driver(udc);
kobject_uevent(udc-dev.kobj, KOBJ_REMOVE);
+ flush_work(gadget-work);
device_unregister(udc-dev);
device_unregister(gadget-dev);
}
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index f1b0dca..942ef5e 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -22,6 +22,7 @@
#include linux/slab.h
#include linux/scatterlist.h
#include linux/types.h
+#include linux/workqueue.h
#include linux/usb/ch9.h
struct usb_ep;
@@ -475,6 +476,7 @@ struct usb_gadget_ops {
/**
* struct usb_gadget - represents a usb slave device
+ * @work: (internal use) Workqueue to be used for sysfs_notify()
* @ops: Function pointers used to access hardware-specific operations.
* @ep0: Endpoint zero, used when reading or writing responses to
* driver setup() requests
@@ -520,6 +522,7 @@ struct usb_gadget_ops {
* device is acting as a B-Peripheral (so is_a_peripheral is false).
*/
struct usb_gadget {
+ struct work_struct work;
/* readonly to gadget driver */
const struct usb_gadget_ops *ops;
struct usb_ep *ep0;
@@ -538,6 +541,7 @@ struct usb_gadget {
unsignedout_epnum;
unsignedin_epnum;
};
+#define work_to_gadget(w) (container_of((w), struct usb_gadget, work))
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
[Fixed address of devicetree mailing list and added more people on CC.]
For reference, full thread can be found under following link:
http://thread.gmane.org/gmane.linux.ports.arm.kernel/252813
Best regards,
Tomasz
On Tuesday 23 of July 2013 09:29:32 Tomasz Figa wrote:
Hi Alan,
On Monday 22 of July 2013 10:44:39 Alan Stern wrote:
On Mon, 22 Jul 2013, Kishon Vijay Abraham I wrote:
The PHY and the controller it is attached to are both
physical
devices.
The connection between them is hardwired by the system
manufacturer and cannot be changed by software.
PHYs are generally described by fixed system-specific
board
files or by Device Tree information. Are they ever
discovered
dynamically?
No. They are created just like any other platform devices are
created.
Okay. Are PHYs _always_ platform devices?
They can be i2c, spi or any other device types as well.
Is the same true for the controllers attached to the PHYs?
If not -- if both a PHY and a controller are discovered
dynamically -- how does the kernel know whether they are
connected to each other?
No differences here. Both PHY and controller will have dt
information
or hwmod data using which platform devices will be created.
The kernel needs to know which controller is attached to
which
PHY. Currently this information is represented by name or
ID
strings embedded in platform data.
right. It's embedded in the platform data of the controller.
It must also be embedded in the PHY's platform data somehow.
Otherwise, how would the kernel know which PHY to use?
By using a PHY lookup as Stephen and I suggested in our previous
replies. Without any extra data in platform data. (I have even posted a
code example.)
The PHY's driver (the supplier) uses the platform data to
construct a platform_device structure that represents the
PHY.
Currently the driver assigns static labels (corresponding to the
label
used in the platform data of the controller).
Until this is done, the controller's driver (the client)
cannot
use the PHY.
right.
Since there is no parent-child relation between the PHY
and the
controller, there is no guarantee that the PHY's driver
will be
ready when the controller's driver wants to use it. A
deferred
probe may be needed.
right.
The issue (or one of the issues) in this discussion is
that
Greg does not like the idea of using names or IDs to
associate
PHYs with controllers, because they are too prone to
duplications or other errors. Pointers are more reliable.
But pointers to what? Since the only data known to be
available to both the PHY driver and controller driver is
the
platform data, the obvious answer is a pointer to platform
data
(either for the PHY or for the controller, or maybe both).
hmm.. it's not going to be simple though as the platform device for
the PHY and controller can be created in entirely different places.
e.g., in some cases the PHY device is a child of some mfd core
device
(the device will be created in drivers/mfd) and the controller
driver
(usually) is created in board file. I guess then we have to come up
with something to share a pointer in two different files.
The ability for two different source files to share a pointer to a
data
item defined in a third source file has been around since long before
the C language was invented. :-)
In this case, it doesn't matter where the platform_device structures
are created or where the driver source code is. Let's take a simple
example. Suppose the system design includes a PHY named foo. Then
the board file could contain:
struct phy_info { ... } phy_foo;
EXPORT_SYMBOL_GPL(phy_foo);
and a header file would contain:
extern struct phy_info phy_foo;
The PHY supplier could then call phy_create(phy_foo), and the PHY
client could call phy_find(phy_foo). Or something like that; make up
your own structure tags and function names.
It's still possible to have conflicts, but now two PHYs with the same
name (or a misspelled name somewhere) will cause an error at link
time.
This is incorrect, sorry. First of all it's a layering violation - you
export random driver-specific symbols from one driver to another. Then
imagine 4 SoCs - A, B, C, D. There are two PHY types PHY1 and PHY2 and
there are two types of consumer drivers (e.g. USB host controllers). Now
consider following mapping:
SoC PHY consumer
A PHY1HOST1
B PHY1HOST2
C PHY2HOST1
D PHY2HOST2
Re: [PATCH v3 1/1] Intel xhci: refactor EHCI/xHCI port switching
On 07/22/2013 06:23 PM, Alan Stern wrote:
On Mon, 22 Jul 2013, Mathias Nyman wrote:
Make the Linux xHCI driver automatically try to switchover the EHCI ports to
xHCI when an Intel xHCI host is detected, and it also finds an Intel EHCI host.
This means we will no longer have to add Intel xHCI hosts to a quirks list when
the PCI device IDs change. Simply continuing to add new Intel xHCI PCI device
IDs to the quirks list is not sustainable.
During suspend ports may be swicthed back to EHCI by BIOS and not properly
restored to xHCI at resume. Previously both EHCI and xHCI resume functions
switched ports back to XHCI, but it's enough to do it in xHCI only
because the hub driver doesn't start running again until after both hosts are
resumed.
Signed-off-by: Mathias Nymanmathias.ny...@linux.intel.com
Acked-by: Alan Sternst...@rowland.harvard.edu
It is noticeable that this code:
diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index b9848e4..90f927f 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -921,8 +895,16 @@ static void quirk_usb_handoff_xhci(struct pci_dev *pdev)
writel(val, base + ext_cap_offset + XHCI_LEGACY_CONTROL_OFFSET);
hc_init:
- if (usb_is_intel_switchable_xhci(pdev))
- usb_enable_xhci_ports(pdev);
+ if (pdev-vendor == PCI_VENDOR_ID_INTEL) {
+ struct pci_dev *companion = NULL;
+ for_each_pci_dev(companion) {
+ if (companion-class == PCI_CLASS_SERIAL_USB_EHCI
+ companion-vendor == PCI_VENDOR_ID_INTEL) {
+ usb_enable_intel_xhci_ports(pdev);
+ break;
+ }
+ }
+ }
and this code:
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index cc24e39..0e83863 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -250,13 +250,23 @@ static int xhci_pci_resume(struct usb_hcd *hcd, bool
hibernated)
...
- if (usb_is_intel_switchable_xhci(pdev))
- usb_enable_xhci_ports(pdev);
+
+ if (pdev-vendor == PCI_VENDOR_ID_INTEL) {
+ struct pci_dev *companion = NULL;
+ for_each_pci_dev(companion) {
+ if (companion-class == PCI_CLASS_SERIAL_USB_EHCI
+ companion-vendor == PCI_VENDOR_ID_INTEL) {
+ usb_enable_intel_xhci_ports(pdev);
+ break;
+ }
+ }
+ }
are identical. Can you have xhci-pci.c call pci-quirks.c, to get rid
of the duplication?
Ah, sure.
The EHCI pci walk can be moved to be a part of
usb_enable_intel_xhci_ports().
I'll fix it.
-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 6/6] USB: OHCI: make ohci-s3c2410 a separate driver
Hi Manjunath, Please see some comments inline. On Monday 22 of July 2013 14:49:30 Manjunath Goudar wrote: Separate the Samsung OHCI S3C host controller driver from ohci-hcd host code so that it can be built as a separate driver module. This work is part of enabling multi-platform kernels on ARM; it would be nice to have in 3.12. Signed-off-by: Manjunath Goudar manjunath.gou...@linaro.org Acked-by: Alan Stern st...@rowland.harvard.edu Cc: Arnd Bergmann a...@arndb.de Cc: Greg KH g...@kroah.com Cc: linux-usb@vger.kernel.org V2: -Set non-standard fields in ohci_s3c2410_hc_driver manually, rather than relying on an expanded struct ohci_driver_overrides. -Save orig_ohci_hub_control and orig_ohci_hub_status_data rather than relying on ohci_hub_control and hub_status_data being exported. V3: -Kconfig wrong parentheses discription fixed. -ohci_setup() has been removed because it is called in .reset member of the ohci_hc_driver structure. V4: - Removed extra space before the '='. - Moved /* forward definitions */ line before the declarations of functions. --- drivers/usb/host/Kconfig|8 +++ drivers/usb/host/Makefile |1 + drivers/usb/host/ohci-hcd.c | 18 -- drivers/usb/host/ohci-s3c2410.c | 128 +-- 4 files changed, 66 insertions(+), 89 deletions(-) diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig index 693560a..795d14d 100644 --- a/drivers/usb/host/Kconfig +++ b/drivers/usb/host/Kconfig @@ -390,6 +390,14 @@ config USB_OHCI_HCD_SPEAR Enables support for the on-chip OHCI controller on ST SPEAr chips. +config USB_OHCI_HCD_S3C +tristate Support for S3C on-chip OHCI USB controller +depends on USB_OHCI_HCD (ARCH_S3C24XX || ARCH_S3C64XX) +default y +---help--- + Enables support for the on-chip OHCI controller on + S3C chips. + Please keep the name s3c2410 for consistency. Having all the names come from the first SoC with this hardware is somewhat deterministic as opposed to some other naming methods, e.g. ohci-exynos2, while later on a different SoC from Exynos 2 shows up that needs a different driver. config USB_OHCI_HCD_AT91 tristate Support for Atmel on-chip OHCI USB controller depends on USB_OHCI_HCD ARCH_AT91 diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile index a70b044..9dffe81 100644 --- a/drivers/usb/host/Makefile +++ b/drivers/usb/host/Makefile @@ -51,6 +51,7 @@ obj-$(CONFIG_USB_OHCI_HCD_OMAP1)+= ohci-omap.o obj-$(CONFIG_USB_OHCI_HCD_OMAP3) += ohci-omap3.o obj-$(CONFIG_USB_OHCI_HCD_SPEAR) += ohci-spear.o obj-$(CONFIG_USB_OHCI_HCD_AT91) += ohci-at91.o +obj-$(CONFIG_USB_OHCI_HCD_S3C) += ohci-s3c2410.o obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o obj-$(CONFIG_USB_FHCI_HCD) += fhci.o diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c index b48c892..b69a49e 100644 --- a/drivers/usb/host/ohci-hcd.c +++ b/drivers/usb/host/ohci-hcd.c @@ -1177,11 +1177,6 @@ MODULE_LICENSE (GPL); #define SA_DRIVERohci_hcd_sa_driver #endif -#if defined(CONFIG_ARCH_S3C24XX) || defined(CONFIG_ARCH_S3C64XX) -#include ohci-s3c2410.c -#define S3C2410_PLATFORM_DRIVER ohci_hcd_s3c2410_driver -#endif - #if defined(CONFIG_PXA27x) || defined(CONFIG_PXA3xx) #include ohci-pxa27x.c #define PLATFORM_DRIVER ohci_hcd_pxa27x_driver @@ -1293,12 +1288,6 @@ static int __init ohci_hcd_mod_init(void) goto error_tmio; #endif -#ifdef S3C2410_PLATFORM_DRIVER - retval = platform_driver_register(S3C2410_PLATFORM_DRIVER); - if (retval 0) - goto error_s3c2410; -#endif - #ifdef EP93XX_PLATFORM_DRIVER retval = platform_driver_register(EP93XX_PLATFORM_DRIVER); if (retval 0) @@ -1332,10 +1321,6 @@ static int __init ohci_hcd_mod_init(void) platform_driver_unregister(EP93XX_PLATFORM_DRIVER); error_ep93xx: #endif -#ifdef S3C2410_PLATFORM_DRIVER - platform_driver_unregister(S3C2410_PLATFORM_DRIVER); - error_s3c2410: -#endif #ifdef TMIO_OHCI_DRIVER platform_driver_unregister(TMIO_OHCI_DRIVER); error_tmio: @@ -1382,9 +1367,6 @@ static void __exit ohci_hcd_mod_exit(void) #ifdef EP93XX_PLATFORM_DRIVER platform_driver_unregister(EP93XX_PLATFORM_DRIVER); #endif -#ifdef S3C2410_PLATFORM_DRIVER - platform_driver_unregister(S3C2410_PLATFORM_DRIVER); -#endif #ifdef TMIO_OHCI_DRIVER platform_driver_unregister(TMIO_OHCI_DRIVER); #endif diff --git a/drivers/usb/host/ohci-s3c2410.c b/drivers/usb/host/ohci-s3c2410.c index e125770..48b5948 100644 --- a/drivers/usb/host/ohci-s3c2410.c +++ b/drivers/usb/host/ohci-s3c2410.c @@ -19,19 +19,36 @@ * This file is licenced under the GPL. */ -#include linux/platform_device.h #include linux/clk.h
Re: [PROBLEM/BUG] mouse polling at only half rate on ohci_hcd
On Sun, 21 Jul 2013, sgtcapslock wrote: I took Alan's excellent advice and read a good bit of that book last night. Definitely some good authors there! After pondering Alan's diagnosis for a bit, I went to inspect the usbhid driver code, and wound up creating a patch which works! I've tested three different gaming mice, and they now all poll properly using the ohci_hcd driver when the usbhid driver uses two URBs to receive input data: (output from the evhz utility) event3: latest hz = 1000 (average hz = 1000) event3: latest hz = 1000 (average hz = 1000) event3: latest hz = 1000 (average hz = 1000) event3: latest hz = 1000 (average hz = 1000) event3: latest hz = 1000 (average hz = 1000) I probably need a lot more time to research things and make sure that I did this the proper way. I'm a bit scared to submit a patch for the first time, so I'd like it to be right. Jiri, can I send you some private e-mail to ask for your advice about all that? Hi, as Greg mentioned already -- normally there shouldn't be any reason for private e-mail, Ccing proper mailinglists is always a good idea to get as much feedback as possible; feel free to drop me an e-mail. Looking forward to your patches, -- Jiri Kosina SUSE Labs -- 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 v4 1/1] Intel xhci: refactor EHCI/xHCI port switching
Make the Linux xHCI driver automatically try to switchover the EHCI ports to
xHCI when an Intel xHCI host is detected, and it also finds an Intel EHCI host.
This means we will no longer have to add Intel xHCI hosts to a quirks list when
the PCI device IDs change. Simply continuing to add new Intel xHCI PCI device
IDs to the quirks list is not sustainable.
During suspend ports may be swicthed back to EHCI by BIOS and not properly
restored to xHCI at resume. Previously both EHCI and xHCI resume functions
switched ports back to XHCI, but it's enough to do it in xHCI only
because the hub driver doesn't start running again until after both hosts are
resumed.
Signed-off-by: Mathias Nyman mathias.ny...@linux.intel.com
---
drivers/usb/host/ehci-pci.c | 42 ---
drivers/usb/host/pci-quirks.c | 48 +++-
drivers/usb/host/pci-quirks.h |3 +-
drivers/usb/host/xhci-pci.c | 14 ++-
4 files changed, 27 insertions(+), 80 deletions(-)
diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
index 595d210..6bd299e 100644
--- a/drivers/usb/host/ehci-pci.c
+++ b/drivers/usb/host/ehci-pci.c
@@ -315,53 +315,11 @@ done:
* Also they depend on separate root hub suspend/resume.
*/
-static bool usb_is_intel_switchable_ehci(struct pci_dev *pdev)
-{
- return pdev-class == PCI_CLASS_SERIAL_USB_EHCI
- pdev-vendor == PCI_VENDOR_ID_INTEL
- (pdev-device == 0x1E26 ||
-pdev-device == 0x8C2D ||
-pdev-device == 0x8C26 ||
-pdev-device == 0x9C26);
-}
-
-static void ehci_enable_xhci_companion(void)
-{
- struct pci_dev *companion = NULL;
-
- /* The xHCI and EHCI controllers are not on the same PCI slot */
- for_each_pci_dev(companion) {
- if (!usb_is_intel_switchable_xhci(companion))
- continue;
- usb_enable_xhci_ports(companion);
- return;
- }
-}
-
static int ehci_pci_resume(struct usb_hcd *hcd, bool hibernated)
{
struct ehci_hcd *ehci = hcd_to_ehci(hcd);
struct pci_dev *pdev = to_pci_dev(hcd-self.controller);
- /* The BIOS on systems with the Intel Panther Point chipset may or may
-* not support xHCI natively. That means that during system resume, it
-* may switch the ports back to EHCI so that users can use their
-* keyboard to select a kernel from GRUB after resume from hibernate.
-*
-* The BIOS is supposed to remember whether the OS had xHCI ports
-* enabled before resume, and switch the ports back to xHCI when the
-* BIOS/OS semaphore is written, but we all know we can't trust BIOS
-* writers.
-*
-* Unconditionally switch the ports back to xHCI after a system resume.
-* We can't tell whether the EHCI or xHCI controller will be resumed
-* first, so we have to do the port switchover in both drivers. Writing
-* a '1' to the port switchover registers should have no effect if the
-* port was already switched over.
-*/
- if (usb_is_intel_switchable_ehci(pdev))
- ehci_enable_xhci_companion();
-
if (ehci_resume(hcd, hibernated) != 0)
(void) ehci_pci_reinit(ehci, pdev);
return 0;
diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index b9848e4..2c76ef1 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -735,32 +735,6 @@ static int handshake(void __iomem *ptr, u32 mask, u32 done,
return -ETIMEDOUT;
}
-#define PCI_DEVICE_ID_INTEL_LYNX_POINT_XHCI0x8C31
-#define PCI_DEVICE_ID_INTEL_LYNX_POINT_LP_XHCI 0x9C31
-
-bool usb_is_intel_ppt_switchable_xhci(struct pci_dev *pdev)
-{
- return pdev-class == PCI_CLASS_SERIAL_USB_XHCI
- pdev-vendor == PCI_VENDOR_ID_INTEL
- pdev-device == PCI_DEVICE_ID_INTEL_PANTHERPOINT_XHCI;
-}
-
-/* The Intel Lynx Point chipset also has switchable ports. */
-bool usb_is_intel_lpt_switchable_xhci(struct pci_dev *pdev)
-{
- return pdev-class == PCI_CLASS_SERIAL_USB_XHCI
- pdev-vendor == PCI_VENDOR_ID_INTEL
- (pdev-device == PCI_DEVICE_ID_INTEL_LYNX_POINT_XHCI ||
-pdev-device == PCI_DEVICE_ID_INTEL_LYNX_POINT_LP_XHCI);
-}
-
-bool usb_is_intel_switchable_xhci(struct pci_dev *pdev)
-{
- return usb_is_intel_ppt_switchable_xhci(pdev) ||
- usb_is_intel_lpt_switchable_xhci(pdev);
-}
-EXPORT_SYMBOL_GPL(usb_is_intel_switchable_xhci);
-
/*
* Intel's Panther Point chipset has two host controllers (EHCI and xHCI) that
* share some number of ports. These ports can be switched between either
@@ -779,9 +753,23 @@ EXPORT_SYMBOL_GPL(usb_is_intel_switchable_xhci);
* terminations before switching the USB 2.0 wires over, so that USB 3.0
* devices connect at
Re: [PATCH 06/16] usb: musb: dsps: rename ti81xx_driver_data to am33xx_driver_data
On 07/23/2013 08:04 AM, George Cherian wrote: Hi Sebastian, On 7/22/2013 11:39 PM, Sebastian Andrzej Siewior wrote: This patch renames the type struct from ti81xx_driver_data to am33xx_driver_data since it is not used for ti81xx anymore. The EOI member is also removed since the am33xx SoC does not have such register. The interrupt is acknowledged by writting into the stat register. AM335X TRM Section 16.6.5 and 16.7.5 describes about EOI registers. Its at offset 0x24. Or is it that the interrupts are acknowledged even without writing to eoi register? I have here Literature Number: SPRUH73H October 2011 Revised April 2013 which calls itself AM335x ARM® CortexTM-A8 Microprocessors (MPUs) Technical Reference Manual. This document ends with 16.5 that means there is no chapter 16.6 or 16.7. The next one is 17. 16.5.2 and 16.5.3 describes the available registers of USB[01]_CTRL REGISTERS which is where the EOI register is accessed. I see here 20h USB0IRQMSTAT 28h USB0IRQSTATRAW0 so no 0x24 at least in my document. Sebastian -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/16] usb: musb: dsps: rename ti81xx_driver_data to am33xx_driver_data
On 7/23/2013 2:39 PM, Sebastian Andrzej Siewior wrote: On 07/23/2013 08:04 AM, George Cherian wrote: Hi Sebastian, On 7/22/2013 11:39 PM, Sebastian Andrzej Siewior wrote: This patch renames the type struct from ti81xx_driver_data to am33xx_driver_data since it is not used for ti81xx anymore. The EOI member is also removed since the am33xx SoC does not have such register. The interrupt is acknowledged by writting into the stat register. AM335X TRM Section 16.6.5 and 16.7.5 describes about EOI registers. Its at offset 0x24. Or is it that the interrupts are acknowledged even without writing to eoi register? I have here Literature Number: SPRUH73H October 2011 Revised April 2013 which calls itself AM335x ARM® CortexTM-A8 Microprocessors (MPUs) Technical Reference Manual. Looks like I have an old TRM with me, which has EOI register at 24h. I need to update my TRM. ;) This document ends with 16.5 that means there is no chapter 16.6 or 16.7. The next one is 17. 16.5.2 and 16.5.3 describes the available registers of USB[01]_CTRL REGISTERS which is where the EOI register is accessed. I see here 20h USB0IRQMSTAT 28h USB0IRQSTATRAW0 so no 0x24 at least in my document. Sebastian -- -George -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 05/16] usb: musb: dsps: use proper child nodes
On 07/23/2013 08:46 AM, George Cherian wrote:
diff --git a/arch/arm/boot/dts/am335x-evm.dts
b/arch/arm/boot/dts/am335x-evm.dts
index 3aee1a4..a3a642a 100644
--- a/arch/arm/boot/dts/am335x-evm.dts
+++ b/arch/arm/boot/dts/am335x-evm.dts
@@ -171,6 +171,26 @@
};
};
+musb: usb@4740 {
+status = okay;
+
+phy@47401300 {
+status = okay;
+};
+
+phy@47401b00 {
+status = okay;
+};
+
+usb@47401000 {
+status = no;
+};
Any reason usb0 is disabled for am33xx evm? Just for testing?
No, that is an error and will be corrected.
Please try to reduce the email in reply to sane size / context. Here
the reader notices the file you refer to and your comment. Quoting the
whole patch eats up more resources for processing plus the reader might
overlook additional comments (he most likely will search for at least
one line).
Sebastian
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 6/6] USB: ehci-omap: Implement suspend/resume
On 07/22/2013 06:18 PM, Alan Stern wrote:
On Mon, 22 Jul 2013, Roger Quadros wrote:
Right, I understand it now. How does the below code look?
+static int omap_ehci_suspend(struct device *dev)
+{
+ struct usb_hcd *hcd = dev_get_drvdata(dev);
+ bool do_wakeup = device_may_wakeup(dev);
+ int ret;
+
+ dev_dbg(dev, %s may_wakeup %d\n, __func__, do_wakeup);
+
+ if (pm_runtime_status_suspended(dev)) {
You need to do this only when do_wakeup is false.
Right. Missed that.
+ pm_runtime_get_sync(dev);
+ ehci_resume(hcd, false);
+ ret = ehci_suspend(hcd, do_wakeup);
+ pm_runtime_put_sync(dev);
It would be better to call pm_runtime_resume(dev) at the start instead
of pm_runtime_get_sync(), and eliminate the last two lines. Then the
ehci_suspend() below could be moved outside the if statement.
Why do I find pm_runtime_* so confusing ;)
Alternatively, if you are able to turn off the wakeup setting without
going through an entire resume/suspend cycle, that would be preferable.
As we don't rely on the EHCI controller's interrupt any more after we
power it down, maybe ehci_resume/suspend cycle is not required at all on OMAP,
even if the wakeup setting is disabled during system suspend.
As the wakeup is controlled entirely by pad wakeup, all we need to do is make
sure
the IO pad wakeup is configured correctly based on do_wakeup. How this is done
is still in transition but it might be turn out to be as simple as
irq_set_irq_wake()
So IMHO, for ehci-omap this should suffice
static int omap_ehci_suspend(struct device *dev)
{
struct usb_hcd *hcd = dev_get_drvdata(dev);
bool do_wakeup = device_may_wakeup(dev);
int ret = 0;
if (!pm_runtime_status_suspended(dev))
ret = ehci_suspend(hcd, do_wakeup);
/* Not tested yet */
irq_set_irq_wake(hcd-irq, do_wakeup);
return ret;
}
What do you think?
As the OMAP IO pad wakeup management code [1] is still in transition, I'll wait
till
that gets settled and then resend this series.
cheers,
-roger
[1] - http://thread.gmane.org/gmane.linux.ports.arm.omap/99010
--
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/3] net/usb/r815x: replace USB buffer from stack to DMA-able
Some USB buffers use stack which may not be DMA-able.
Use the buffers from kmalloc to replace those one.
Signed-off-by: Hayes Wang hayesw...@realtek.com
---
drivers/net/usb/r815x.c | 44 +++-
1 file changed, 27 insertions(+), 17 deletions(-)
diff --git a/drivers/net/usb/r815x.c b/drivers/net/usb/r815x.c
index 8523922..e9b99ba 100644
--- a/drivers/net/usb/r815x.c
+++ b/drivers/net/usb/r815x.c
@@ -24,34 +24,43 @@
static int pla_read_word(struct usb_device *udev, u16 index)
{
- int data, ret;
+ int ret;
u8 shift = index 2;
- __le32 ocp_data;
+ __le32 *tmp;
+
+ tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+ if (!tmp)
+ return -ENOMEM;
index = ~3;
ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
RTL815x_REQ_GET_REGS, RTL815x_REQT_READ,
- index, MCU_TYPE_PLA, ocp_data, sizeof(ocp_data),
- 500);
+ index, MCU_TYPE_PLA, tmp, sizeof(*tmp), 500);
if (ret 0)
- return ret;
+ goto out2;
- data = __le32_to_cpu(ocp_data);
- data = (shift * 8);
- data = 0x;
+ ret = __le32_to_cpu(*tmp);
+ ret = (shift * 8);
+ ret = 0x;
- return data;
+out2:
+ kfree(tmp);
+ return ret;
}
static int pla_write_word(struct usb_device *udev, u16 index, u32 data)
{
- __le32 ocp_data;
+ __le32 *tmp;
u32 mask = 0x;
u16 byen = BYTE_EN_WORD;
u8 shift = index 2;
int ret;
+ tmp = kmalloc(sizeof(*tmp), GFP_KERNEL);
+ if (!tmp)
+ return -ENOMEM;
+
data = mask;
if (shift) {
@@ -63,19 +72,20 @@ static int pla_write_word(struct usb_device *udev, u16
index, u32 data)
ret = usb_control_msg(udev, usb_rcvctrlpipe(udev, 0),
RTL815x_REQ_GET_REGS, RTL815x_REQT_READ,
- index, MCU_TYPE_PLA, ocp_data, sizeof(ocp_data),
- 500);
+ index, MCU_TYPE_PLA, tmp, sizeof(*tmp), 500);
if (ret 0)
- return ret;
+ goto out3;
- data |= __le32_to_cpu(ocp_data) ~mask;
- ocp_data = __cpu_to_le32(data);
+ data |= __le32_to_cpu(*tmp) ~mask;
+ *tmp = __cpu_to_le32(data);
ret = usb_control_msg(udev, usb_sndctrlpipe(udev, 0),
RTL815x_REQ_SET_REGS, RTL815x_REQT_WRITE,
- index, MCU_TYPE_PLA | byen, ocp_data,
- sizeof(ocp_data), 500);
+ index, MCU_TYPE_PLA | byen, tmp, sizeof(*tmp),
+ 500);
+out3:
+ kfree(tmp);
return ret;
}
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/3] net/usb/r8152: make sure the USB buffer is DMA-able
Allocate the required memory before calling usb_control_msg. And
the additional memory copy is necessary.
Signed-off-by: Hayes Wang hayesw...@realtek.com
---
drivers/net/usb/r8152.c | 60 -
1 file changed, 35 insertions(+), 25 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index ee13f9e..ef033ab 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -344,17 +344,41 @@ static const int multicast_filter_limit = 32;
static
int get_registers(struct r8152 *tp, u16 value, u16 index, u16 size, void *data)
{
- return usb_control_msg(tp-udev, usb_rcvctrlpipe(tp-udev, 0),
+ int ret;
+ void *tmp;
+
+ tmp = kmalloc(size, GFP_KERNEL);
+ if (!tmp)
+ return -ENOMEM;
+
+ ret = usb_control_msg(tp-udev, usb_rcvctrlpipe(tp-udev, 0),
RTL8152_REQ_GET_REGS, RTL8152_REQT_READ,
- value, index, data, size, 500);
+ value, index, tmp, size, 500);
+
+ memcpy(data, tmp, size);
+ kfree(tmp);
+
+ return ret;
}
static
int set_registers(struct r8152 *tp, u16 value, u16 index, u16 size, void *data)
{
- return usb_control_msg(tp-udev, usb_sndctrlpipe(tp-udev, 0),
+ int ret;
+ void *tmp;
+
+ tmp = kmalloc(size, GFP_KERNEL);
+ if (!tmp)
+ return -ENOMEM;
+
+ memcpy(tmp, data, size);
+
+ ret = usb_control_msg(tp-udev, usb_sndctrlpipe(tp-udev, 0),
RTL8152_REQ_SET_REGS, RTL8152_REQT_WRITE,
- value, index, data, size, 500);
+ value, index, tmp, size, 500);
+
+ kfree(tmp);
+ return ret;
}
static int generic_ocp_read(struct r8152 *tp, u16 index, u16 size,
@@ -685,21 +709,14 @@ static void ocp_reg_write(struct r8152 *tp, u16 addr, u16
data)
static inline void set_ethernet_addr(struct r8152 *tp)
{
struct net_device *dev = tp-netdev;
- u8 *node_id;
+ u8 node_id[8] = {0};
- node_id = kmalloc(sizeof(u8) * 8, GFP_KERNEL);
- if (!node_id) {
- netif_err(tp, probe, dev, out of memory);
- return;
- }
-
- if (pla_ocp_read(tp, PLA_IDR, sizeof(u8) * 8, node_id) 0)
+ if (pla_ocp_read(tp, PLA_IDR, sizeof(node_id), node_id) 0)
netif_notice(tp, probe, dev, inet addr fail\n);
else {
memcpy(dev-dev_addr, node_id, dev-addr_len);
memcpy(dev-perm_addr, dev-dev_addr, dev-addr_len);
}
- kfree(node_id);
}
static int rtl8152_set_mac_address(struct net_device *netdev, void *p)
@@ -882,15 +899,10 @@ static void rtl8152_set_rx_mode(struct net_device *netdev)
static void _rtl8152_set_rx_mode(struct net_device *netdev)
{
struct r8152 *tp = netdev_priv(netdev);
- u32 tmp, *mc_filter;/* Multicast hash filter */
+ u32 mc_filter[2]; /* Multicast hash filter */
+ __le32 tmp[2];
u32 ocp_data;
- mc_filter = kmalloc(sizeof(u32) * 2, GFP_KERNEL);
- if (!mc_filter) {
- netif_err(tp, link, netdev, out of memory);
- return;
- }
-
clear_bit(RTL8152_SET_RX_MODE, tp-flags);
netif_stop_queue(netdev);
ocp_data = ocp_read_dword(tp, MCU_TYPE_PLA, PLA_RCR);
@@ -918,14 +930,12 @@ static void _rtl8152_set_rx_mode(struct net_device
*netdev)
}
}
- tmp = mc_filter[0];
- mc_filter[0] = __cpu_to_le32(swab32(mc_filter[1]));
- mc_filter[1] = __cpu_to_le32(swab32(tmp));
+ tmp[0] = __cpu_to_le32(swab32(mc_filter[1]));
+ tmp[1] = __cpu_to_le32(swab32(mc_filter[0]));
- pla_ocp_write(tp, PLA_MAR, BYTE_EN_DWORD, sizeof(u32) * 2, mc_filter);
+ pla_ocp_write(tp, PLA_MAR, BYTE_EN_DWORD, sizeof(tmp), tmp);
ocp_write_dword(tp, MCU_TYPE_PLA, PLA_RCR, ocp_data);
netif_wake_queue(netdev);
- kfree(mc_filter);
}
static netdev_tx_t rtl8152_start_xmit(struct sk_buff *skb,
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] net/usb/r8152: adjust relative ocp function
- fix the conversion between cpu and __le32
- replace some pla_ocp and usb_ocp functions with generic_ocp function
Signed-off-by: Hayes Wang hayesw...@realtek.com
---
drivers/net/usb/r8152.c | 66 +
1 file changed, 23 insertions(+), 43 deletions(-)
diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c
index ef033ab..11c51f2 100644
--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -514,37 +514,31 @@ int usb_ocp_write(struct r8152 *tp, u16 index, u16
byteen, u16 size, void *data)
static u32 ocp_read_dword(struct r8152 *tp, u16 type, u16 index)
{
- u32 data;
+ __le32 data;
- if (type == MCU_TYPE_PLA)
- pla_ocp_read(tp, index, sizeof(data), data);
- else
- usb_ocp_read(tp, index, sizeof(data), data);
+ generic_ocp_read(tp, index, sizeof(data), data, type);
return __le32_to_cpu(data);
}
static void ocp_write_dword(struct r8152 *tp, u16 type, u16 index, u32 data)
{
- if (type == MCU_TYPE_PLA)
- pla_ocp_write(tp, index, BYTE_EN_DWORD, sizeof(data), data);
- else
- usb_ocp_write(tp, index, BYTE_EN_DWORD, sizeof(data), data);
+ __le32 tmp = __cpu_to_le32(data);
+
+ generic_ocp_write(tp, index, BYTE_EN_DWORD, sizeof(tmp), tmp, type);
}
static u16 ocp_read_word(struct r8152 *tp, u16 type, u16 index)
{
u32 data;
+ __le32 tmp;
u8 shift = index 2;
index = ~3;
- if (type == MCU_TYPE_PLA)
- pla_ocp_read(tp, index, sizeof(data), data);
- else
- usb_ocp_read(tp, index, sizeof(data), data);
+ generic_ocp_read(tp, index, sizeof(tmp), tmp, type);
- data = __le32_to_cpu(data);
+ data = __le32_to_cpu(tmp);
data = (shift * 8);
data = 0x;
@@ -553,7 +547,8 @@ static u16 ocp_read_word(struct r8152 *tp, u16 type, u16
index)
static void ocp_write_word(struct r8152 *tp, u16 type, u16 index, u32 data)
{
- u32 tmp, mask = 0x;
+ u32 mask = 0x;
+ __le32 tmp;
u16 byen = BYTE_EN_WORD;
u8 shift = index 2;
@@ -566,34 +561,25 @@ static void ocp_write_word(struct r8152 *tp, u16 type,
u16 index, u32 data)
index = ~3;
}
- if (type == MCU_TYPE_PLA)
- pla_ocp_read(tp, index, sizeof(tmp), tmp);
- else
- usb_ocp_read(tp, index, sizeof(tmp), tmp);
+ generic_ocp_read(tp, index, sizeof(tmp), tmp, type);
- tmp = __le32_to_cpu(tmp) ~mask;
- tmp |= data;
- tmp = __cpu_to_le32(tmp);
+ data |= __le32_to_cpu(tmp) ~mask;
+ tmp = __cpu_to_le32(data);
- if (type == MCU_TYPE_PLA)
- pla_ocp_write(tp, index, byen, sizeof(tmp), tmp);
- else
- usb_ocp_write(tp, index, byen, sizeof(tmp), tmp);
+ generic_ocp_write(tp, index, byen, sizeof(tmp), tmp, type);
}
static u8 ocp_read_byte(struct r8152 *tp, u16 type, u16 index)
{
u32 data;
+ __le32 tmp;
u8 shift = index 3;
index = ~3;
- if (type == MCU_TYPE_PLA)
- pla_ocp_read(tp, index, sizeof(data), data);
- else
- usb_ocp_read(tp, index, sizeof(data), data);
+ generic_ocp_read(tp, index, sizeof(tmp), tmp, type);
- data = __le32_to_cpu(data);
+ data = __le32_to_cpu(tmp);
data = (shift * 8);
data = 0xff;
@@ -602,7 +588,8 @@ static u8 ocp_read_byte(struct r8152 *tp, u16 type, u16
index)
static void ocp_write_byte(struct r8152 *tp, u16 type, u16 index, u32 data)
{
- u32 tmp, mask = 0xff;
+ u32 mask = 0xff;
+ __le32 tmp;
u16 byen = BYTE_EN_BYTE;
u8 shift = index 3;
@@ -615,19 +602,12 @@ static void ocp_write_byte(struct r8152 *tp, u16 type,
u16 index, u32 data)
index = ~3;
}
- if (type == MCU_TYPE_PLA)
- pla_ocp_read(tp, index, sizeof(tmp), tmp);
- else
- usb_ocp_read(tp, index, sizeof(tmp), tmp);
+ generic_ocp_read(tp, index, sizeof(tmp), tmp, type);
- tmp = __le32_to_cpu(tmp) ~mask;
- tmp |= data;
- tmp = __cpu_to_le32(tmp);
+ data |= __le32_to_cpu(tmp) ~mask;
+ tmp = __cpu_to_le32(data);
- if (type == MCU_TYPE_PLA)
- pla_ocp_write(tp, index, byen, sizeof(tmp), tmp);
- else
- usb_ocp_write(tp, index, byen, sizeof(tmp), tmp);
+ generic_ocp_write(tp, index, byen, sizeof(tmp), tmp, type);
}
static void r8152_mdio_write(struct r8152 *tp, u32 reg_addr, u32 value)
--
1.8.3.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PROBLEM/BUG] mouse polling at only half rate on ohci_hcd
On 07/23/2013 03:26 AM, Jiri Kosina wrote:
Hi,
as Greg mentioned already -- normally there shouldn't be any reason for
private e-mail, Ccing proper mailinglists is always a good idea to get as
much feedback as possible; feel free to drop me an e-mail.
Hello,
Ahh, apologies to both of you... I posted a response to this thread earlier
and it looks like I somehow didn't cc: either of you. :( I'll post again
what I posted just earlier:
Hello,
I'd like to post the incomplete patch I've written so far and see if
anybody has anything to say about it, and perhaps see if anyone can help
me with a couple little problems I'm having trouble with.
It's taken me a while to come back to this because I've been busy trying
to make sure that the code I've written so far isn't a complete mess.
I decided to post it in this thread since it already discusses the
problem - but for anyone who hasn't read the prior posts, here is a
summary:
On some OHCI controllers, the usbhid driver needs a second input URB to
prevent data being lost when one URB is being read on a frame boundary
and incoming interrupt data is learned about on the next frame. I'm
completely new to programming anything for the kernel and I'm trying to
write a patch to fix this.
Here's some thoughts about what I've coded so far:
It occurs to me that this only happens on some controllers. So, I
thought it'd be nice if I could find a way to test for the necessity of
a second (or maybe even a third? who knows) input URB instead of just
giving every usbhid device two of them.
wrt that:
1. Am I on the right track thinking like that?
2. Does the USB stuff in the kernel already have a way to test this?
3. If not, I'm really stumped about how I can do this, so any advice
would be great. :p
Aside from that - in hid_start_in(), would it have been better if I'd
used goto and a label instead of breaking from the for loop? I wouldn't
have to test if (rc == 0) to clear that HID_NO_BANDWIDTH bit that way,
but it seemed more messy.
Please let me know if I've made even the slightest mistake so I can
learn from this!
Regards,
Josef
--
diff -uprN -X linux-3.10-vanilla/Documentation/dontdiff
linux-3.10-vanilla/drivers/hid/usbhid/hid-core.c
linux-3.10-dev/drivers/hid/usbhid/hid-core.c
--- linux-3.10-vanilla/drivers/hid/usbhid/hid-core.c2013-06-30
17:13:29.0 -0500
+++ linux-3.10-dev/drivers/hid/usbhid/hid-core.c2013-07-22
16:00:24.785950521 -0500
@@ -80,20 +80,26 @@ static int hid_start_in(struct hid_devic
unsigned long flags;
int rc = 0;
struct usbhid_device *usbhid = hid-driver_data;
+ int i;
spin_lock_irqsave(usbhid-lock, flags);
if (hid-open 0
!test_bit(HID_DISCONNECTED, usbhid-iofl)
!test_bit(HID_SUSPENDED, usbhid-iofl)
!test_and_set_bit(HID_IN_RUNNING, usbhid-iofl)) {
- rc = usb_submit_urb(usbhid-urbin, GFP_ATOMIC);
- if (rc != 0) {
- clear_bit(HID_IN_RUNNING, usbhid-iofl);
- if (rc == -ENOSPC)
- set_bit(HID_NO_BANDWIDTH, usbhid-iofl);
- } else {
- clear_bit(HID_NO_BANDWIDTH, usbhid-iofl);
+ for (i = 0; i usbhid-n_inurbs; i++) {
+ rc = usb_submit_urb(usbhid-inurbs[i], GFP_ATOMIC);
+ if (rc != 0) {
+ clear_bit(HID_IN_RUNNING, usbhid-iofl);
+ if (rc == -ENOSPC)
+ set_bit(HID_NO_BANDWIDTH,
usbhid-iofl);
+
+ break;
+ }
}
+
+ if (rc == 0)
+ clear_bit(HID_NO_BANDWIDTH, usbhid-iofl);
}
spin_unlock_irqrestore(usbhid-lock, flags);
return rc;
@@ -120,7 +126,7 @@ static void hid_reset(struct work_struct
if (test_bit(HID_CLEAR_HALT, usbhid-iofl)) {
dev_dbg(usbhid-intf-dev, clear halt\n);
- rc = usb_clear_halt(hid_to_usb_dev(hid), usbhid-urbin-pipe);
+ rc = usb_clear_halt(hid_to_usb_dev(hid),
usbhid-inurbs[0]-pipe);
clear_bit(HID_CLEAR_HALT, usbhid-iofl);
hid_start_in(hid);
}
@@ -780,6 +786,7 @@ done:
void usbhid_close(struct hid_device *hid)
{
struct usbhid_device *usbhid = hid-driver_data;
+ int i;
mutex_lock(hid_open_mut);
@@ -791,7 +798,10 @@ void usbhid_close(struct hid_device *hid
if (!--hid-open) {
spin_unlock_irq(usbhid-lock);
hid_cancel_delayed_stuff(usbhid);
- usb_kill_urb(usbhid-urbin);
+
+ for (i = 0; i usbhid-n_inurbs; i++)
+ usb_kill_urb(usbhid-inurbs[i]);
+
usbhid-intf-needs_remote_wakeup = 0;
} else {
Re: [PATCH V2] usb: Add Device Tree support to XHCI Platform driver
Hello.
On 23-07-2013 3:04, Al Cooper wrote:
Add Device Tree match table to xhci-plat.c. Add DT bindings document.
Signed-off-by: Al Cooper alcoop...@gmail.com
[...]
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index d718134..4c4823a 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
[...]
@@ -212,11 +213,17 @@ static int xhci_plat_remove(struct platform_device *dev)
return 0;
}
+static const struct of_device_id usb_xhci_of_match[] = {
+ { .compatible = usb-xhci },
+ {},
+};
+
static struct platform_driver usb_xhci_driver = {
.probe = xhci_plat_probe,
.remove = xhci_plat_remove,
.driver = {
.name = xhci-hcd,
+ .of_match_table = of_match_ptr(usb_xhci_of_match),
Since you're using of_match_ptr(), you could enclose usb_xhci_of_match[]
into #ifdef CONFIG_OF to save several bytes when it's not enabled...
WBR, Sergei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/8] clk: mux: Add support for read-only muxes.
Hello.
On 23-07-2013 3:49, Tomasz Figa wrote:
Some platforms have read-only clock muxes that are preconfigured at
reset and cannot be changed at runtime. This patch extends mux clock
driver to allow handling such read-only muxes by adding new
CLK_MUX_READ_ONLY mux flag.
Signed-off-by: Tomasz Figa tomasz.f...@gmail.com
[...]
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 1ec14a7..9487b96 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -327,8 +327,10 @@ struct clk_mux {
#define CLK_MUX_INDEX_ONE BIT(0)
#define CLK_MUX_INDEX_BIT BIT(1)
#define CLK_MUX_HIWORD_MASK BIT(2)
+#define CLK_MUX_READ_ONLY BIT(3) /* mux setting cannot be changed */
Please align BIT(3) with the above BIT() invocations.
WBR, Sergei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/8] clk: mux: Add support for read-only muxes.
Hi Sergei,
On Tuesday 23 of July 2013 15:22:44 Sergei Shtylyov wrote:
Hello.
On 23-07-2013 3:49, Tomasz Figa wrote:
Some platforms have read-only clock muxes that are preconfigured at
reset and cannot be changed at runtime. This patch extends mux clock
driver to allow handling such read-only muxes by adding new
CLK_MUX_READ_ONLY mux flag.
Signed-off-by: Tomasz Figa tomasz.f...@gmail.com
[...]
diff --git a/include/linux/clk-provider.h
b/include/linux/clk-provider.h
index 1ec14a7..9487b96 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -327,8 +327,10 @@ struct clk_mux {
#define CLK_MUX_INDEX_ONE BIT(0)
#define CLK_MUX_INDEX_BIT BIT(1)
#define CLK_MUX_HIWORD_MASK BIT(2)
+#define CLK_MUX_READ_ONLY BIT(3) /* mux setting cannot be changed */
Please align BIT(3) with the above BIT() invocations.
Different indentation was intended here to fit the comment, like in case of
generic flags. IMHO remaining flags should be changed to this way as well,
but this is probably material for another patch.
Best regards,
Tomasz
--
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
Weird Serial number with Fitbit Base Station dongle
Hi, This is probably a minor issue, but when plugin the Fitbit base station dongle, I'm getting some weird serial number in dmesg. Can something be done here? Kind regards Laurent Bigonville [ 9993.722197] usb 1-1.2: new full-speed USB device number 7 using ehci-pci [ 9993.820617] usb 1-1.2: New USB device found, idVendor=2687, idProduct=fb01 [ 9993.820622] usb 1-1.2: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 9993.820625] usb 1-1.2: Product: Fitbit Base Station [ 9993.820627] usb 1-1.2: Manufacturer: Fitbit Inc. [ 9993.820629] usb 1-1.2: SerialNumber: \xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf [ 9993.824615] hid-generic 0003:2687:FB01.0008: hiddev0,hidraw1: USB HID v1.11 Device [Fitbit Inc. Fitbit Base Station] on usb-:00:1a.0-1.2/input0 [ 9993.827453] hid-generic 0003:2687:FB01.0009: hiddev0,hidraw2: USB HID v1.11 Device [Fitbit Inc. Fitbit Base Station] on usb-:00:1a.0-1.2/input1 Bus 001 Device 007: ID 2687:fb01 Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass0 (Defined at Interface level) bDeviceSubClass 0 bDeviceProtocol 0 bMaxPacketSize032 idVendor 0x2687 idProduct 0xfb01 bcdDevice1.00 iManufacturer 1 Fitbit Inc. iProduct2 Fitbit Base Station iSerial 3 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 73 bNumInterfaces 2 bConfigurationValue 1 iConfiguration 0 bmAttributes 0xa0 (Bus Powered) Remote Wakeup MaxPower 50mA Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 2 bInterfaceClass 3 Human Interface Device bInterfaceSubClass 0 No Subclass bInterfaceProtocol 0 None iInterface 0 HID Device Descriptor: bLength 9 bDescriptorType33 bcdHID 1.11 bCountryCode0 Not supported bNumDescriptors 1 bDescriptorType34 Report wDescriptorLength 54 Report Descriptors: ** UNAVAILABLE ** Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x81 EP 1 IN bmAttributes3 Transfer TypeInterrupt Synch Type None Usage Type Data wMaxPacketSize 0x0020 1x 32 bytes bInterval 2 Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x01 EP 1 OUT bmAttributes3 Transfer TypeInterrupt Synch Type None Usage Type Data wMaxPacketSize 0x0020 1x 32 bytes bInterval 2 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber1 bAlternateSetting 0 bNumEndpoints 2 bInterfaceClass 3 Human Interface Device bInterfaceSubClass 0 No Subclass bInterfaceProtocol 0 None iInterface 0 HID Device Descriptor: bLength 9 bDescriptorType33 bcdHID 1.11 bCountryCode0 Not supported bNumDescriptors 1 bDescriptorType34 Report wDescriptorLength 54 Report Descriptors: ** UNAVAILABLE ** Endpoint Descriptor: bLength 7 bDescriptorType 5 bEndpointAddress 0x82 EP 2 IN bmAttributes3 Transfer TypeInterrupt Synch Type None Usage Type
[PATCH v3 4/5] staging: ozwpan: Convert macro to function.
From: Joe Perches j...@perches.com
Replace macro with inline function.
Signed-off-by: Joe Perches j...@perches.com
Signed-off-by: Rupesh Gujare rupesh.guj...@atmel.com
---
drivers/staging/ozwpan/ozurbparanoia.h |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/ozwpan/ozurbparanoia.h
b/drivers/staging/ozwpan/ozurbparanoia.h
index 00f5a3a..5080ea7 100644
--- a/drivers/staging/ozwpan/ozurbparanoia.h
+++ b/drivers/staging/ozwpan/ozurbparanoia.h
@@ -10,8 +10,8 @@
void oz_remember_urb(struct urb *urb);
int oz_forget_urb(struct urb *urb);
#else
-#define oz_remember_urb(__x)
-#define oz_forget_urb(__x) 0
+static inline void oz_remember_urb(struct urb *urb) {}
+static inline int oz_forget_urb(struct urb *urb) { return 0; }
#endif /* WANT_URB_PARANOIA */
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/5] staging: ozwpan: Replace debug macro
This is v3 of this original patch series here:- http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2013-June/039280.html v3 adds commit log for each patch as suggested by Joe here:- http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2013-July/039361.html Joe Perches (5): staging: ozwpan: Remove extra debug logs. staging: ozwpan: Replace oz_trace with oz_dbg staging: ozwpan: Remove old debug macro. staging: ozwpan: Convert macro to function. staging: ozwpan: Rename Kbuild to Makefile drivers/staging/ozwpan/Kbuild | 18 -- drivers/staging/ozwpan/Makefile| 16 ++ drivers/staging/ozwpan/ozcdev.c| 52 +++--- drivers/staging/ozwpan/ozconfig.h | 26 --- drivers/staging/ozwpan/ozdbg.h | 54 ++ drivers/staging/ozwpan/ozeltbuf.c | 32 ++-- drivers/staging/ozwpan/ozhcd.c | 296 +++- drivers/staging/ozwpan/ozmain.c|7 +- drivers/staging/ozwpan/ozpd.c | 67 drivers/staging/ozwpan/ozproto.c | 60 +++ drivers/staging/ozwpan/ozproto.h |2 +- drivers/staging/ozwpan/oztrace.c | 36 drivers/staging/ozwpan/oztrace.h | 35 drivers/staging/ozwpan/ozurbparanoia.c | 14 +- drivers/staging/ozwpan/ozurbparanoia.h |4 +- drivers/staging/ozwpan/ozusbsvc.c | 25 +-- drivers/staging/ozwpan/ozusbsvc1.c | 19 +- 17 files changed, 348 insertions(+), 415 deletions(-) delete mode 100644 drivers/staging/ozwpan/Kbuild create mode 100644 drivers/staging/ozwpan/Makefile delete mode 100644 drivers/staging/ozwpan/ozconfig.h create mode 100644 drivers/staging/ozwpan/ozdbg.h delete mode 100644 drivers/staging/ozwpan/oztrace.c delete mode 100644 drivers/staging/ozwpan/oztrace.h -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 5/5] staging: ozwpan: Rename Kbuild to Makefile
From: Joe Perches j...@perches.com Rename Kbuild to usual Makefile, consistent with Kernel build structure. Signed-off-by: Joe Perches j...@perches.com Signed-off-by: Rupesh Gujare rupesh.guj...@atmel.com --- drivers/staging/ozwpan/Kbuild | 16 drivers/staging/ozwpan/Makefile | 16 2 files changed, 16 insertions(+), 16 deletions(-) delete mode 100644 drivers/staging/ozwpan/Kbuild create mode 100644 drivers/staging/ozwpan/Makefile diff --git a/drivers/staging/ozwpan/Kbuild b/drivers/staging/ozwpan/Kbuild deleted file mode 100644 index 29529c1..000 --- a/drivers/staging/ozwpan/Kbuild +++ /dev/null @@ -1,16 +0,0 @@ -# - -# Copyright (c) 2011 Ozmo Inc -# Released under the GNU General Public License Version 2 (GPLv2). -# - - -obj-$(CONFIG_USB_WPAN_HCD) += ozwpan.o -ozwpan-y := \ - ozmain.o \ - ozpd.o \ - ozusbsvc.o \ - ozusbsvc1.o \ - ozhcd.o \ - ozeltbuf.o \ - ozproto.o \ - ozcdev.o \ - ozurbparanoia.o diff --git a/drivers/staging/ozwpan/Makefile b/drivers/staging/ozwpan/Makefile new file mode 100644 index 000..29529c1 --- /dev/null +++ b/drivers/staging/ozwpan/Makefile @@ -0,0 +1,16 @@ +# - +# Copyright (c) 2011 Ozmo Inc +# Released under the GNU General Public License Version 2 (GPLv2). +# - + +obj-$(CONFIG_USB_WPAN_HCD) += ozwpan.o +ozwpan-y := \ + ozmain.o \ + ozpd.o \ + ozusbsvc.o \ + ozusbsvc1.o \ + ozhcd.o \ + ozeltbuf.o \ + ozproto.o \ + ozcdev.o \ + ozurbparanoia.o -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/5] staging: ozwpan: Remove extra debug logs.
From: Joe Perches j...@perches.com
Remove unnecessary debug logs. Most of these logs
print function name at the start of function, which
are not really required.
Signed-off-by: Joe Perches j...@perches.com
Signed-off-by: Rupesh Gujare rupesh.guj...@atmel.com
---
drivers/staging/ozwpan/ozcdev.c |2 --
drivers/staging/ozwpan/ozhcd.c | 28
2 files changed, 8 insertions(+), 22 deletions(-)
diff --git a/drivers/staging/ozwpan/ozcdev.c b/drivers/staging/ozwpan/ozcdev.c
index 374fdc3..284c26d 100644
--- a/drivers/staging/ozwpan/ozcdev.c
+++ b/drivers/staging/ozwpan/ozcdev.c
@@ -73,7 +73,6 @@ static void oz_cdev_release_ctx(struct oz_serial_ctx *ctx)
static int oz_cdev_open(struct inode *inode, struct file *filp)
{
struct oz_cdev *dev;
- oz_trace(oz_cdev_open()\n);
oz_trace(major = %d minor = %d\n, imajor(inode), iminor(inode));
dev = container_of(inode-i_cdev, struct oz_cdev, cdev);
filp-private_data = dev;
@@ -84,7 +83,6 @@ static int oz_cdev_open(struct inode *inode, struct file
*filp)
*/
static int oz_cdev_release(struct inode *inode, struct file *filp)
{
- oz_trace(oz_cdev_release()\n);
return 0;
}
/*--
diff --git a/drivers/staging/ozwpan/ozhcd.c b/drivers/staging/ozwpan/ozhcd.c
index d68d63a..9d1bd44 100644
--- a/drivers/staging/ozwpan/ozhcd.c
+++ b/drivers/staging/ozwpan/ozhcd.c
@@ -394,7 +394,6 @@ static void oz_complete_urb(struct usb_hcd *hcd, struct urb
*urb,
*/
static void oz_ep_free(struct oz_port *port, struct oz_endpoint *ep)
{
- oz_trace(oz_ep_free()\n);
if (port) {
struct list_head list;
struct oz_hcd *ozhcd = port-ozhcd;
@@ -631,7 +630,6 @@ void *oz_hcd_pd_arrived(void *hpd)
void *hport = NULL;
struct oz_hcd *ozhcd = NULL;
struct oz_endpoint *ep;
- oz_trace(oz_hcd_pd_arrived()\n);
ozhcd = oz_hcd_claim();
if (ozhcd == NULL)
return NULL;
@@ -691,7 +689,6 @@ void oz_hcd_pd_departed(void *hport)
void *hpd;
struct oz_endpoint *ep = NULL;
- oz_trace(oz_hcd_pd_departed()\n);
if (port == NULL) {
oz_trace(oz_hcd_pd_departed() port = 0\n);
return;
@@ -1696,7 +1693,6 @@ static void oz_hcd_clear_orphanage(struct oz_hcd *ozhcd,
int status)
*/
static int oz_hcd_start(struct usb_hcd *hcd)
{
- oz_trace(oz_hcd_start()\n);
hcd-power_budget = 200;
hcd-state = HC_STATE_RUNNING;
hcd-uses_new_polling = 1;
@@ -1707,14 +1703,12 @@ static int oz_hcd_start(struct usb_hcd *hcd)
*/
static void oz_hcd_stop(struct usb_hcd *hcd)
{
- oz_trace(oz_hcd_stop()\n);
}
/*--
* Context: unknown
*/
static void oz_hcd_shutdown(struct usb_hcd *hcd)
{
- oz_trace(oz_hcd_shutdown()\n);
}
/*--
* Called to queue an urb for the device.
@@ -1844,7 +1838,6 @@ static int oz_hcd_urb_dequeue(struct usb_hcd *hcd, struct
urb *urb, int status)
static void oz_hcd_endpoint_disable(struct usb_hcd *hcd,
struct usb_host_endpoint *ep)
{
- oz_trace(oz_hcd_endpoint_disable\n);
}
/*--
* Context: unknown
@@ -1852,7 +1845,6 @@ static void oz_hcd_endpoint_disable(struct usb_hcd *hcd,
static void oz_hcd_endpoint_reset(struct usb_hcd *hcd,
struct usb_host_endpoint *ep)
{
- oz_trace(oz_hcd_endpoint_reset\n);
}
/*--
* Context: unknown
@@ -1872,7 +1864,6 @@ static int oz_hcd_hub_status_data(struct usb_hcd *hcd,
char *buf)
struct oz_hcd *ozhcd = oz_hcd_private(hcd);
int i;
- oz_trace2(OZ_TRACE_HUB, oz_hcd_hub_status_data()\n);
buf[0] = 0;
spin_lock_bh(ozhcd-hcd_lock);
@@ -1892,7 +1883,6 @@ static int oz_hcd_hub_status_data(struct usb_hcd *hcd,
char *buf)
static void oz_get_hub_descriptor(struct usb_hcd *hcd,
struct usb_hub_descriptor *desc)
{
- oz_trace2(OZ_TRACE_HUB, GetHubDescriptor\n);
memset(desc, 0, sizeof(*desc));
desc-bDescriptorType = 0x29;
desc-bDescLength = 9;
@@ -1911,7 +1901,7 @@ static int oz_set_port_feature(struct usb_hcd *hcd, u16
wvalue, u16 windex)
struct oz_hcd *ozhcd = oz_hcd_private(hcd);
unsigned set_bits = 0;
unsigned clear_bits = 0;
- oz_trace2(OZ_TRACE_HUB, SetPortFeature\n);
+
if ((port_id 1) || (port_id OZ_NB_PORTS))
return -EPIPE;
port = ozhcd-ports[port_id-1];
@@ -1986,7 +1976,7 @@ static int oz_clear_port_feature(struct usb_hcd *hcd, u16
wvalue, u16 windex)
u8
[PATCH v3 3/5] staging: ozwpan: Remove old debug macro.
From: Joe Perches j...@perches.com
Remove old oz_trace oz_trace2 macro related header files.
Signed-off-by: Joe Perches j...@perches.com
Signed-off-by: Rupesh Gujare rupesh.guj...@atmel.com
---
drivers/staging/ozwpan/Kbuild |6 ++
drivers/staging/ozwpan/ozcdev.c|2 --
drivers/staging/ozwpan/ozconfig.h | 26 ---
drivers/staging/ozwpan/ozeltbuf.c |2 --
drivers/staging/ozwpan/ozhcd.c |7 +--
drivers/staging/ozwpan/ozmain.c|7 +--
drivers/staging/ozwpan/ozpd.c |6 ++
drivers/staging/ozwpan/ozproto.c |4 ++--
drivers/staging/ozwpan/oztrace.c | 36
drivers/staging/ozwpan/oztrace.h | 35 ---
drivers/staging/ozwpan/ozurbparanoia.c |5 +++--
drivers/staging/ozwpan/ozusbsvc.c |4 ++--
drivers/staging/ozwpan/ozusbsvc1.c |2 --
13 files changed, 17 insertions(+), 125 deletions(-)
delete mode 100644 drivers/staging/ozwpan/ozconfig.h
delete mode 100644 drivers/staging/ozwpan/oztrace.c
delete mode 100644 drivers/staging/ozwpan/oztrace.h
diff --git a/drivers/staging/ozwpan/Kbuild b/drivers/staging/ozwpan/Kbuild
index 1766a26..29529c1 100644
--- a/drivers/staging/ozwpan/Kbuild
+++ b/drivers/staging/ozwpan/Kbuild
@@ -2,6 +2,7 @@
# Copyright (c) 2011 Ozmo Inc
# Released under the GNU General Public License Version 2 (GPLv2).
# -
+
obj-$(CONFIG_USB_WPAN_HCD) += ozwpan.o
ozwpan-y := \
ozmain.o \
@@ -12,7 +13,4 @@ ozwpan-y := \
ozeltbuf.o \
ozproto.o \
ozcdev.o \
- ozurbparanoia.o \
- oztrace.o
-
-
+ ozurbparanoia.o
diff --git a/drivers/staging/ozwpan/ozcdev.c b/drivers/staging/ozwpan/ozcdev.c
index 87c3131..3e29760 100644
--- a/drivers/staging/ozwpan/ozcdev.c
+++ b/drivers/staging/ozwpan/ozcdev.c
@@ -11,10 +11,8 @@
#include linux/etherdevice.h
#include linux/poll.h
#include linux/sched.h
-#include ozconfig.h
#include ozdbg.h
#include ozprotocol.h
-#include oztrace.h
#include ozappif.h
#include ozeltbuf.h
#include ozpd.h
diff --git a/drivers/staging/ozwpan/ozconfig.h
b/drivers/staging/ozwpan/ozconfig.h
deleted file mode 100644
index 087c322..000
--- a/drivers/staging/ozwpan/ozconfig.h
+++ /dev/null
@@ -1,26 +0,0 @@
-/*
-
- * Copyright (c) 2011 Ozmo Inc
- * Released under the GNU General Public License Version 2 (GPLv2).
- *
---*/
-#ifndef _OZCONFIG_H
-#define _OZCONFIG_H
-
-/* #define WANT_TRACE */
-#ifdef WANT_TRACE
-#define WANT_VERBOSE_TRACE
-#endif /* #ifdef WANT_TRACE */
-/* #define WANT_URB_PARANOIA */
-
-/* #define WANT_PRE_2_6_39 */
-
-/* These defines determine what verbose trace is displayed. */
-#ifdef WANT_VERBOSE_TRACE
-/* #define WANT_TRACE_STREAM */
-/* #define WANT_TRACE_URB */
-/* #define WANT_TRACE_CTRL_DETAIL */
-#define WANT_TRACE_HUB
-/* #define WANT_TRACE_RX_FRAMES */
-/* #define WANT_TRACE_TX_FRAMES */
-#endif /* WANT_VERBOSE_TRACE */
-
-#endif /* _OZCONFIG_H */
diff --git a/drivers/staging/ozwpan/ozeltbuf.c
b/drivers/staging/ozwpan/ozeltbuf.c
index bf280aa..5e98aeb 100644
--- a/drivers/staging/ozwpan/ozeltbuf.c
+++ b/drivers/staging/ozwpan/ozeltbuf.c
@@ -6,12 +6,10 @@
#include linux/init.h
#include linux/module.h
#include linux/netdevice.h
-#include ozconfig.h
#include ozdbg.h
#include ozprotocol.h
#include ozeltbuf.h
#include ozpd.h
-#include oztrace.h
/*--
*/
diff --git a/drivers/staging/ozwpan/ozhcd.c b/drivers/staging/ozwpan/ozhcd.c
index fece2da..6b16bfc 100644
--- a/drivers/staging/ozwpan/ozhcd.c
+++ b/drivers/staging/ozwpan/ozhcd.c
@@ -32,9 +32,7 @@
#include linux/usb/hcd.h
#include asm/unaligned.h
#include ozdbg.h
-#include ozconfig.h
#include ozusbif.h
-#include oztrace.h
#include ozurbparanoia.h
#include ozhcd.h
/*--
@@ -797,7 +795,6 @@ void oz_hcd_get_desc_cnf(void *hport, u8 req_id, int
status, const u8 *desc,
/*--
* Context: softirq
*/
-#ifdef WANT_TRACE
static void oz_display_conf_type(u8 t)
{
switch (t) {
@@ -836,9 +833,7 @@ static void oz_display_conf_type(u8 t)
break;
}
}
-#else
-#define oz_display_conf_type(__x)
-#endif /* WANT_TRACE */
+
/*--
* Context: softirq
*/
diff --git a/drivers/staging/ozwpan/ozmain.c b/drivers/staging/ozwpan/ozmain.c
index 51fe9e9..e26d6be 100644
--- a/drivers/staging/ozwpan/ozmain.c
+++ b/drivers/staging/ozwpan/ozmain.c
@@ -3,6 +3,7 @@
* Released under the GNU General Public
EHCI driver breaks at 6 endpoints
Hello, We are attempting to create a system that will support 8 full speed USB devices each sending a 64-byte transfer every 1ms via interrupt endpoints. When we connect 5 full speed USB devices our USB host use 24% of the CPU, but when we connect a 6th device, the CPU goes to 100%. Has anyone else seen this type of CPU jump or know what could be causing it? Devices CPU Total Throughput 1 7% 64 KB/s 2 11% 128 KB/s 3 15% 192 KB/s 4 19% 256 KB/s 5 24% 320 KB/s 6 99% (should be 384 KB/s, but the iMX535 misses sending IN tokens in some SOF periods) We are using a Freescale iMX535 board running at 800 MHz as the USB host. One of the iMX535 EHCI root controllers is connected to a high speed USB hub which has two ports each connected to another high speed USB hub (giving us 8 ports for USB devices). Each hub has multi-TT support. The iMX535 is running Linux 2.6.35, and our test application uses libUSB asynchronous I/O (v1.0.16). The libUSB callback function do not process the data; rather the function only resubmits the transfer. We only have the EHCI driver enabled. CONFIG_USB_EHCI_ROOT_HUB_TT=y CONFIG_USB_EHCI_TT_NEWSCHED=y # CONFIG_USB_ARCH_HAS_OHCI is not set Thanks for the help. -Nate -- 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: option: add D-Link DWM-152/C1 and DWM-156/C1
Adding support for D-Link DWM-152/C1 and DWM-156/C1 devices.
DWM-152/C1:
T: Bus=01 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#= 6 Spd=480 MxCh= 0
D: Ver= 2.00 Cls=00(ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1
P: Vendor=07d1 ProdID=3e01 Rev= 0.00
S: Product=USB Configuration
S: SerialNumber=1234567890ABCDEF
C:* #Ifs= 5 Cfg#= 1 Atr=e0 MxPwr=500mA
I:* If#= 0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=option
E: Ad=81(I) Atr=03(Int.) MxPS= 64 Ivl=2ms
E: Ad=82(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= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option
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=ff(vend.) Sub=ff Prot=ff Driver=option
E: Ad=85(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=04(O) Atr=02(Bulk) MxPS= 512 Ivl=4ms
I:* If#= 4 Alt= 0 #EPs= 2 Cls=08(stor.) Sub=06 Prot=50 Driver=usb-storage
E: Ad=05(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=86(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
DWM-156/C1:
T: Bus=01 Lev=01 Prnt=01 Port=00 Cnt=01 Dev#= 8 Spd=480 MxCh= 0
D: Ver= 2.00 Cls=00(ifc ) Sub=00 Prot=00 MxPS=64 #Cfgs= 1
P: Vendor=07d1 ProdID=3e02 Rev= 0.00
S: Product=DataCard Device
S: SerialNumber=1234567890ABCDEF
C:* #Ifs= 5 Cfg#= 1 Atr=e0 MxPwr=500mA
I:* If#= 0 Alt= 0 #EPs= 3 Cls=ff(vend.) Sub=ff Prot=ff Driver=option
E: Ad=81(I) Atr=03(Int.) MxPS= 64 Ivl=2ms
E: Ad=82(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= 2 Cls=ff(vend.) Sub=ff Prot=ff Driver=option
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=ff(vend.) Sub=ff Prot=ff Driver=option
E: Ad=85(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=04(O) Atr=02(Bulk) MxPS= 512 Ivl=4ms
I:* If#= 4 Alt= 0 #EPs= 2 Cls=08(stor.) Sub=06 Prot=50 Driver=usb-storage
E: Ad=05(O) Atr=02(Bulk) MxPS= 512 Ivl=0ms
E: Ad=86(I) Atr=02(Bulk) MxPS= 512 Ivl=0ms
Signed-off-by: Alexandr Ivanov alexandr@gmail.com
---
drivers/usb/serial/option.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/usb/serial/option.c b/drivers/usb/serial/option.c
index 5dd857d..d62b455 100644
--- a/drivers/usb/serial/option.c
+++ b/drivers/usb/serial/option.c
@@ -1339,6 +1339,8 @@ static const struct usb_device_id option_ids[] = {
{ USB_DEVICE_AND_INTERFACE_INFO(0x2001, 0x7d02, 0xff, 0x00, 0x00) },
{ USB_DEVICE_AND_INTERFACE_INFO(0x2001, 0x7d03, 0xff, 0x02, 0x01) },
{ USB_DEVICE_AND_INTERFACE_INFO(0x2001, 0x7d03, 0xff, 0x00, 0x00) },
+ { USB_DEVICE_AND_INTERFACE_INFO(0x07d1, 0x3e01, 0xff, 0xff, 0xff) },
/* D-Link DWM-152/C1 */
+ { USB_DEVICE_AND_INTERFACE_INFO(0x07d1, 0x3e02, 0xff, 0xff, 0xff) },
/* D-Link DWM-156/C1 */
{ } /* Terminating entry */
};
MODULE_DEVICE_TABLE(usb, option_ids);
--
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 2/2] usb: dwc3: ep0: don't change to configured state too early
On Tue, 23 Jul 2013, Felipe Balbi wrote:
@@ -148,6 +148,7 @@ static int __dwc3_gadget_ep0_queue(struct dwc3_ep
*dep,
direction = !dwc-ep0_expect_in;
dwc-delayed_status = false;
+ usb_gadget_set_state(dwc-gadget, USB_STATE_CONFIGURED);
Isn't this overkill? Do you really want to call usb_gadget_set_state()
every time the gadget driver queues a transfer on ep0?
Or am I missing an important part of the context?
heh, you're missing context, that will only be called when we had
delayed status flag set:
| static int __dwc3_gadget_ep0_queue(struct dwc3_ep *dep,
| struct dwc3_request *req)
| {
[ ... ]
| /*
| * In case gadget driver asked us to delay the STATUS phase,
| * handle it here.
| */
| if (dwc-delayed_status) {
| unsigneddirection;
|
| direction = !dwc-ep0_expect_in;
| dwc-delayed_status = false;
| usb_gadget_set_state(dwc-gadget, USB_STATE_CONFIGURED);
I see. Doesn't the mass-storage gadget also use delayed status when
going into the _un_configured state?
Alan Stern
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: gadget: udc-core: move sysfs_notify() to a workqueue
On Tue, 23 Jul 2013, Felipe Balbi wrote:
Here's a newer version:
commit 766ed104b6f420dc7587a63dc1679f78176d082e
Author: Felipe Balbi ba...@ti.com
Date: Wed Jul 17 11:09:49 2013 +0300
usb: gadget: udc-core: move sysfs_notify() to a workqueue
usb_gadget_set_state() will call sysfs_notify()
which might sleep. Some users might want to call
usb_gadget_set_state() from the very IRQ handler
which actually changes the gadget state.
Instead of having every UDC driver add their own
workqueue for such a simple notification, we're
adding it generically to our struct usb_gadget,
so the details are hidden from all UDC drivers.
Signed-off-by: Felipe Balbi ba...@ti.com
diff --git a/drivers/usb/gadget/udc-core.c b/drivers/usb/gadget/udc-core.c
index ffd8fa5..eed8503 100644
--- a/drivers/usb/gadget/udc-core.c
+++ b/drivers/usb/gadget/udc-core.c
@@ -23,6 +23,7 @@
#include linux/list.h
#include linux/err.h
#include linux/dma-mapping.h
+#include linux/workqueue.h
#include linux/usb/ch9.h
#include linux/usb/gadget.h
@@ -101,11 +102,18 @@ EXPORT_SYMBOL_GPL(usb_gadget_unmap_request);
/* -
*/
+static void usb_gadget_state_work(struct work_struct *work)
+{
+ struct usb_gadget *gadget = work_to_gadget(work);
+
+ sysfs_notify(gadget-dev.kobj, NULL, status);
+}
+
void usb_gadget_set_state(struct usb_gadget *gadget,
enum usb_device_state state)
{
gadget-state = state;
- sysfs_notify(gadget-dev.kobj, NULL, status);
+ schedule_work(gadget-work);
}
EXPORT_SYMBOL_GPL(usb_gadget_set_state);
@@ -192,6 +200,7 @@ int usb_add_gadget_udc_release(struct device *parent,
struct usb_gadget *gadget,
goto err1;
dev_set_name(gadget-dev, gadget);
+ INIT_WORK(gadget-work, usb_gadget_state_work);
gadget-dev.parent = parent;
dma_set_coherent_mask(gadget-dev, parent-coherent_dma_mask);
@@ -309,6 +318,7 @@ found:
usb_gadget_remove_driver(udc);
kobject_uevent(udc-dev.kobj, KOBJ_REMOVE);
+ flush_work(gadget-work);
device_unregister(udc-dev);
device_unregister(gadget-dev);
}
diff --git a/include/linux/usb/gadget.h b/include/linux/usb/gadget.h
index f1b0dca..942ef5e 100644
--- a/include/linux/usb/gadget.h
+++ b/include/linux/usb/gadget.h
@@ -22,6 +22,7 @@
#include linux/slab.h
#include linux/scatterlist.h
#include linux/types.h
+#include linux/workqueue.h
#include linux/usb/ch9.h
struct usb_ep;
@@ -475,6 +476,7 @@ struct usb_gadget_ops {
/**
* struct usb_gadget - represents a usb slave device
+ * @work: (internal use) Workqueue to be used for sysfs_notify()
* @ops: Function pointers used to access hardware-specific operations.
* @ep0: Endpoint zero, used when reading or writing responses to
* driver setup() requests
@@ -520,6 +522,7 @@ struct usb_gadget_ops {
* device is acting as a B-Peripheral (so is_a_peripheral is false).
*/
struct usb_gadget {
+ struct work_struct work;
/* readonly to gadget driver */
const struct usb_gadget_ops *ops;
struct usb_ep *ep0;
@@ -538,6 +541,7 @@ struct usb_gadget {
unsignedout_epnum;
unsignedin_epnum;
};
+#define work_to_gadget(w)(container_of((w), struct usb_gadget, work))
static inline void set_gadget_data(struct usb_gadget *gadget, void *data)
{ dev_set_drvdata(gadget-dev, data); }
Acked-by: Alan Stern st...@rowland.harvard.edu
--
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 6/6] USB: ehci-omap: Implement suspend/resume
On Tue, 23 Jul 2013, Roger Quadros wrote:
+ pm_runtime_get_sync(dev);
+ ehci_resume(hcd, false);
+ ret = ehci_suspend(hcd, do_wakeup);
+ pm_runtime_put_sync(dev);
It would be better to call pm_runtime_resume(dev) at the start instead
of pm_runtime_get_sync(), and eliminate the last two lines. Then the
ehci_suspend() below could be moved outside the if statement.
Why do I find pm_runtime_* so confusing ;)
It tries to provide every service anyone might want, and ends up being
confusingly large.
In this case, though, the reasoning is simple. We know that after the
system resumes, the device will be back in the active state. Hence
there's no point in calling pm_runtime_put_sync here, other than to
balance the _get_sync above. Since there's no danger of another thread
trying to do a runtime suspend at this point, you don't need to
increment the usage counter. Therefore pm_runtime_resume is good
enough.
Alternatively, if you are able to turn off the wakeup setting without
going through an entire resume/suspend cycle, that would be preferable.
As we don't rely on the EHCI controller's interrupt any more after we
power it down, maybe ehci_resume/suspend cycle is not required at all on OMAP,
even if the wakeup setting is disabled during system suspend.
As the wakeup is controlled entirely by pad wakeup, all we need to do is make
sure
the IO pad wakeup is configured correctly based on do_wakeup. How this is done
is still in transition but it might be turn out to be as simple as
irq_set_irq_wake()
So IMHO, for ehci-omap this should suffice
static int omap_ehci_suspend(struct device *dev)
{
struct usb_hcd *hcd = dev_get_drvdata(dev);
bool do_wakeup = device_may_wakeup(dev);
int ret = 0;
if (!pm_runtime_status_suspended(dev))
ret = ehci_suspend(hcd, do_wakeup);
/* Not tested yet */
irq_set_irq_wake(hcd-irq, do_wakeup);
return ret;
}
What do you think?
Nice and simple. It looks good.
As the OMAP IO pad wakeup management code [1] is still in transition, I'll
wait till
that gets settled and then resend this series.
Okay.
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: Weird Serial number with Fitbit Base Station dongle
On Tue, 23 Jul 2013, Laurent Bigonville wrote: Hi, This is probably a minor issue, but when plugin the Fitbit base station dongle, I'm getting some weird serial number in dmesg. Can something be done here? Kind regards Laurent Bigonville [ 9993.722197] usb 1-1.2: new full-speed USB device number 7 using ehci-pci [ 9993.820617] usb 1-1.2: New USB device found, idVendor=2687, idProduct=fb01 [ 9993.820622] usb 1-1.2: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 9993.820625] usb 1-1.2: Product: Fitbit Base Station [ 9993.820627] usb 1-1.2: Manufacturer: Fitbit Inc. [ 9993.820629] usb 1-1.2: SerialNumber: \xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf It may be that, weird as this looks, it is actually correct. Can you post a usbmon trace showing what happens when the dongle is first plugged? Instructions are in the kernel source file Documentation/usb/usbmon.txt. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/1] Intel xhci: refactor EHCI/xHCI port switching
On Tue, 23 Jul 2013, Mathias Nyman wrote:
Make the Linux xHCI driver automatically try to switchover the EHCI ports to
xHCI when an Intel xHCI host is detected, and it also finds an Intel EHCI
host.
This means we will no longer have to add Intel xHCI hosts to a quirks list
when
the PCI device IDs change. Simply continuing to add new Intel xHCI PCI device
IDs to the quirks list is not sustainable.
During suspend ports may be swicthed back to EHCI by BIOS and not properly
restored to xHCI at resume. Previously both EHCI and xHCI resume functions
switched ports back to XHCI, but it's enough to do it in xHCI only
because the hub driver doesn't start running again until after both hosts are
resumed.
Signed-off-by: Mathias Nyman mathias.ny...@linux.intel.com
-void usb_enable_xhci_ports(struct pci_dev *xhci_pdev)
+void usb_enable_intel_xhci_ports(struct pci_dev *xhci_pdev)
{
u32 ports_available;
+ boolehci_found = false;
+ struct pci_dev *companion = NULL;
+
+ /* make sure an intel EHCI controller exists */
+ for_each_pci_dev(companion) {
+ if (companion-class == PCI_CLASS_SERIAL_USB_EHCI
+ companion-vendor == PCI_VENDOR_ID_INTEL) {
+ ehci_found = true;
+ break;
+ }
+ }
+
+ if (!ehci_found)
+ return;
/* Don't switchover the ports if the user hasn't compiled the xHCI
* driver. Otherwise they will see dead USB ports that don't power
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -250,13 +250,15 @@ static int xhci_pci_resume(struct usb_hcd *hcd, bool
hibernated)
* writers.
*
* Unconditionally switch the ports back to xHCI after a system resume.
- * We can't tell whether the EHCI or xHCI controller will be resumed
- * first, so we have to do the port switchover in both drivers. Writing
- * a '1' to the port switchover registers should have no effect if the
- * port was already switched over.
+ * It should not matter whether the EHCI or xHCI controller is
+ * resumed first. It's enough to do the switchover in xHCI because
+ * USB core won't notice anything as the hub driver doesn't start
+ * running again until after all the devices (including both EHCI and
+ * xHCI host controllers) have been resumed.
*/
- if (usb_is_intel_switchable_xhci(pdev))
- usb_enable_xhci_ports(pdev);
+
+ if (pdev-vendor == PCI_VENDOR_ID_INTEL)
+ usb_enable_intel_xhci_ports(pdev);
Short and sweet; I like it!
Alan Stern
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH V3] usb: Add Device Tree support to XHCI Platform driver
Add Device Tree match table to xhci-plat.c. Add DT bindings document.
Signed-off-by: Al Cooper alcoop...@gmail.com
---
Documentation/devicetree/bindings/usb/usb-xhci.txt | 14 ++
drivers/usb/host/xhci-plat.c | 9 +
2 files changed, 23 insertions(+)
create mode 100644 Documentation/devicetree/bindings/usb/usb-xhci.txt
diff --git a/Documentation/devicetree/bindings/usb/usb-xhci.txt
b/Documentation/devicetree/bindings/usb/usb-xhci.txt
new file mode 100644
index 000..654cf3d
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/usb-xhci.txt
@@ -0,0 +1,14 @@
+USB XHCI controllers
+
+Required properties:
+ - compatible: should be usb-xhci.
+ - reg: should contain address and length of the standard XHCI
+register set for the device.
+ - interrupts: one XHCI interrupt should be described here.
+
+Example:
+ xhci@f0931000 {
+ compatible = usb-xhci;
+ reg = 0xf0931000 0x8c8;
+ interrupts = 0x0 0x4e 0x0;
+ };
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index d718134..d70f607 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -14,6 +14,7 @@
#include linux/platform_device.h
#include linux/module.h
#include linux/slab.h
+#include linux/of.h
#include xhci.h
@@ -212,11 +213,19 @@ static int xhci_plat_remove(struct platform_device *dev)
return 0;
}
+#ifdef CONFIG_OF
+static const struct of_device_id usb_xhci_of_match[] = {
+ { .compatible = usb-xhci },
+ {},
+};
+#endif
+
static struct platform_driver usb_xhci_driver = {
.probe = xhci_plat_probe,
.remove = xhci_plat_remove,
.driver = {
.name = xhci-hcd,
+ .of_match_table = of_match_ptr(usb_xhci_of_match),
},
};
MODULE_ALIAS(platform:xhci-hcd);
--
1.8.1.3
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, 23 Jul 2013, Tomasz Figa wrote:
On Tuesday 23 of July 2013 09:29:32 Tomasz Figa wrote:
Hi Alan,
Thanks for helping to clarify the issues here.
Okay. Are PHYs _always_ platform devices?
They can be i2c, spi or any other device types as well.
In those other cases, presumably there is no platform data associated
with the PHY since it isn't a platform device. Then how does the
kernel know which controller is attached to the PHY? Is this spelled
out in platform data associated with the PHY's i2c/spi/whatever parent?
PHY. Currently this information is represented by name or
ID
strings embedded in platform data.
right. It's embedded in the platform data of the controller.
It must also be embedded in the PHY's platform data somehow.
Otherwise, how would the kernel know which PHY to use?
By using a PHY lookup as Stephen and I suggested in our previous
replies. Without any extra data in platform data. (I have even posted a
code example.)
I don't understand, because I don't know what a PHY lookup does.
In this case, it doesn't matter where the platform_device structures
are created or where the driver source code is. Let's take a simple
example. Suppose the system design includes a PHY named foo. Then
the board file could contain:
struct phy_info { ... } phy_foo;
EXPORT_SYMBOL_GPL(phy_foo);
and a header file would contain:
extern struct phy_info phy_foo;
The PHY supplier could then call phy_create(phy_foo), and the PHY
client could call phy_find(phy_foo). Or something like that; make up
your own structure tags and function names.
It's still possible to have conflicts, but now two PHYs with the same
name (or a misspelled name somewhere) will cause an error at link
time.
This is incorrect, sorry. First of all it's a layering violation - you
export random driver-specific symbols from one driver to another. Then
No, that's not what I said. Neither the PHY driver nor the controller
driver exports anything to the other. Instead, both drivers use data
exported by the board file.
imagine 4 SoCs - A, B, C, D. There are two PHY types PHY1 and PHY2 and
there are two types of consumer drivers (e.g. USB host controllers). Now
consider following mapping:
SoC PHY consumer
A PHY1HOST1
B PHY1HOST2
C PHY2HOST1
D PHY2HOST2
So we have to be able to use any of the PHYs with any of the host
drivers. This means you would have to export symbol with the same name
from both PHY drivers, which obviously would not work in this case,
because having both drivers enabled (in a multiplatform aware
configuration) would lead to linking conflict.
You're right; the scheme was too simple. Instead, the board file must
export two types of data structures, one for PHYs and one for
controllers. Like this:
struct phy_info {
/* Info for the controller attached to this PHY */
struct controller_info *hinfo;
};
struct controller_info {
/* Info for the PHY which this controller is attached to */
struct phy_info *pinfo;
};
The board file for SoC A would contain:
struct phy_info phy1 = {host1);
EXPORT_SYMBOL(phy1);
struct controller_info host1 = {phy1};
EXPORT_SYMBOL(host1);
The board file for SoC B would contain:
struct phy_info phy1 = {host2);
EXPORT_SYMBOL(phy1);
struct controller_info host2 = {phy1};
EXPORT_SYMBOL(host2);
And so on. This explicitly gives the connection between PHYs and
controllers. The PHY providers would use phy1 or phy2, and the PHY
consumers would use host1 or host2.
Alan Stern
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3] usb: Add Device Tree support to XHCI Platform driver
Hello.
On 07/23/2013 06:35 PM, Matthijs Kooijman wrote:
@@ -212,11 +213,19 @@ static int xhci_plat_remove(struct platform_device *dev)
return 0;
}
+#ifdef CONFIG_OF
+static const struct of_device_id usb_xhci_of_match[] = {
+ { .compatible = usb-xhci },
+ {},
+};
I wonder if we also need:
MODULE_DEVICE_TABLE(of, usb_xhci_of_match);
+#endif
+
static struct platform_driver usb_xhci_driver = {
.probe = xhci_plat_probe,
.remove = xhci_plat_remove,
.driver = {
.name = xhci-hcd,
+ .of_match_table = of_match_ptr(usb_xhci_of_match),
},
};
MODULE_ALIAS(platform:xhci-hcd);
This looks like it wouldn't compile without CONFIG_OF?
No, of_match_ptr() is #defined to NULL when CONFIG_OF=n.
Gr.
Matthijs
WBR, Sergei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tuesday 23 of July 2013 10:37:05 Alan Stern wrote:
On Tue, 23 Jul 2013, Tomasz Figa wrote:
On Tuesday 23 of July 2013 09:29:32 Tomasz Figa wrote:
Hi Alan,
Thanks for helping to clarify the issues here.
Okay. Are PHYs _always_ platform devices?
They can be i2c, spi or any other device types as well.
In those other cases, presumably there is no platform data associated
with the PHY since it isn't a platform device. Then how does the
kernel know which controller is attached to the PHY? Is this spelled
out in platform data associated with the PHY's i2c/spi/whatever parent?
PHY. Currently this information is represented by name or
ID
strings embedded in platform data.
right. It's embedded in the platform data of the controller.
It must also be embedded in the PHY's platform data somehow.
Otherwise, how would the kernel know which PHY to use?
By using a PHY lookup as Stephen and I suggested in our previous
replies. Without any extra data in platform data. (I have even posted
a
code example.)
I don't understand, because I don't know what a PHY lookup does.
I have provided a code example in [1]. Feel free to ask questions about
those code snippets.
[1] http://thread.gmane.org/gmane.linux.ports.arm.kernel/252813/focus=20889
In this case, it doesn't matter where the platform_device
structures
are created or where the driver source code is. Let's take a
simple
example. Suppose the system design includes a PHY named foo.
Then
the board file could contain:
struct phy_info { ... } phy_foo;
EXPORT_SYMBOL_GPL(phy_foo);
and a header file would contain:
extern struct phy_info phy_foo;
The PHY supplier could then call phy_create(phy_foo), and the PHY
client could call phy_find(phy_foo). Or something like that; make
up
your own structure tags and function names.
It's still possible to have conflicts, but now two PHYs with the
same
name (or a misspelled name somewhere) will cause an error at link
time.
This is incorrect, sorry. First of all it's a layering violation -
you
export random driver-specific symbols from one driver to another.
Then
No, that's not what I said. Neither the PHY driver nor the controller
driver exports anything to the other. Instead, both drivers use data
exported by the board file.
It's still a random, driver-specific global symbol exported from board file
to drivers.
imagine 4 SoCs - A, B, C, D. There are two PHY types PHY1 and PHY2
and
there are two types of consumer drivers (e.g. USB host controllers).
Now
consider following mapping:
SoC PHY consumer
A PHY1HOST1
B PHY1HOST2
C PHY2HOST1
D PHY2HOST2
So we have to be able to use any of the PHYs with any of the host
drivers. This means you would have to export symbol with the same
name
from both PHY drivers, which obviously would not work in this case,
because having both drivers enabled (in a multiplatform aware
configuration) would lead to linking conflict.
You're right; the scheme was too simple. Instead, the board file must
export two types of data structures, one for PHYs and one for
controllers. Like this:
struct phy_info {
/* Info for the controller attached to this PHY */
struct controller_info *hinfo;
};
struct controller_info {
/* Info for the PHY which this controller is attached to */
struct phy_info *pinfo;
};
The board file for SoC A would contain:
struct phy_info phy1 = {host1);
EXPORT_SYMBOL(phy1);
struct controller_info host1 = {phy1};
EXPORT_SYMBOL(host1);
The board file for SoC B would contain:
struct phy_info phy1 = {host2);
EXPORT_SYMBOL(phy1);
struct controller_info host2 = {phy1};
EXPORT_SYMBOL(host2);
And so on. This explicitly gives the connection between PHYs and
controllers. The PHY providers would use phy1 or phy2, and the PHY
consumers would use host1 or host2.
This could work assuming that only one SoC and one board is supported in
single kernel image. However it's not the case.
We've used to support multiple boards since a long time already and now for
selected platforms we even support multiplatform, i.e. multiple SoCs in
single zImage. Such solution will not work.
Best regards,
Tomasz
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
Hi,
On Tuesday 23 July 2013 08:07 PM, Alan Stern wrote:
On Tue, 23 Jul 2013, Tomasz Figa wrote:
On Tuesday 23 of July 2013 09:29:32 Tomasz Figa wrote:
Hi Alan,
Thanks for helping to clarify the issues here.
Okay. Are PHYs _always_ platform devices?
They can be i2c, spi or any other device types as well.
In those other cases, presumably there is no platform data associated
with the PHY since it isn't a platform device. Then how does the
kernel know which controller is attached to the PHY? Is this spelled
out in platform data associated with the PHY's i2c/spi/whatever parent?
Yes. I think we could use i2c_board_info for passing platform data.
PHY. Currently this information is represented by name or
ID
strings embedded in platform data.
right. It's embedded in the platform data of the controller.
It must also be embedded in the PHY's platform data somehow.
Otherwise, how would the kernel know which PHY to use?
By using a PHY lookup as Stephen and I suggested in our previous
replies. Without any extra data in platform data. (I have even posted a
code example.)
I don't understand, because I don't know what a PHY lookup does.
It is how the PHY framework finds a PHY, when the controller (say USB)requests
a PHY from the PHY framework.
In this case, it doesn't matter where the platform_device structures
are created or where the driver source code is. Let's take a simple
example. Suppose the system design includes a PHY named foo. Then
the board file could contain:
struct phy_info { ... } phy_foo;
EXPORT_SYMBOL_GPL(phy_foo);
and a header file would contain:
extern struct phy_info phy_foo;
The PHY supplier could then call phy_create(phy_foo), and the PHY
client could call phy_find(phy_foo). Or something like that; make up
your own structure tags and function names.
It's still possible to have conflicts, but now two PHYs with the same
name (or a misspelled name somewhere) will cause an error at link
time.
This is incorrect, sorry. First of all it's a layering violation - you
export random driver-specific symbols from one driver to another. Then
No, that's not what I said. Neither the PHY driver nor the controller
driver exports anything to the other. Instead, both drivers use data
exported by the board file.
I think instead we can use the same data while creating the platform data of
the controller and the PHY.
The PHY driver while creating the PHY (using PHY framework) will also pass the
*data* it actually got from the platform data to the framework.
The PHY user driver (USB), while requesting for the PHY (from the PHY
framework) will pass the *data* it got from its platform data.
The PHY framework can do a comparison of the *data* pointers it has and return
the appropriate PHY to the controller.
imagine 4 SoCs - A, B, C, D. There are two PHY types PHY1 and PHY2 and
there are two types of consumer drivers (e.g. USB host controllers). Now
consider following mapping:
SoC PHY consumer
A PHY1HOST1
B PHY1HOST2
C PHY2HOST1
D PHY2HOST2
So we have to be able to use any of the PHYs with any of the host
drivers. This means you would have to export symbol with the same name
from both PHY drivers, which obviously would not work in this case,
because having both drivers enabled (in a multiplatform aware
configuration) would lead to linking conflict.
You're right; the scheme was too simple. Instead, the board file must
export two types of data structures, one for PHYs and one for
controllers. Like this:
struct phy_info {
/* Info for the controller attached to this PHY */
struct controller_info *hinfo;
};
struct controller_info {
/* Info for the PHY which this controller is attached to */
struct phy_info *pinfo;
};
The board file for SoC A would contain:
struct phy_info phy1 = {host1);
EXPORT_SYMBOL(phy1);
struct controller_info host1 = {phy1};
EXPORT_SYMBOL(host1);
The board file for SoC B would contain:
struct phy_info phy1 = {host2);
EXPORT_SYMBOL(phy1);
struct controller_info host2 = {phy1};
EXPORT_SYMBOL(host2);
I meant something like this
struct phy_info {
const char *name;
};
struct phy_platform_data {
.
.
struct phy_info *info;
};
struct usb_controller_platform_data {
.
.
struct phy_info *info;
};
struct phy_info phy_info;
While creating the phy device
struct phy_platform_data phy_data;
phy_data.info = info;
platform_device_add_data(pdev, phy_data, sizeof(*phy_data))
platform_device_add();
While creating the controller device
struct usb_controller_platform_data controller_data;
controller_data.info = info;
platform_device_add_data(pdev, controller_data,
sizeof(*controller_data))
platform_device_add();
Then modify PHY framework API phy create
phy_create((struct device *dev, const struct phy_ops
Re: EHCI driver breaks at 6 endpoints
On Tue, Jul 23, 2013 at 01:33:25PM +, Stoddard, Nate (GE Healthcare) wrote: Hello, We are attempting to create a system that will support 8 full speed USB devices each sending a 64-byte transfer every 1ms via interrupt endpoints. When we connect 5 full speed USB devices our USB host use 24% of the CPU, but when we connect a 6th device, the CPU goes to 100%. Has anyone else seen this type of CPU jump or know what could be causing it? Devices CPU Total Throughput 1 7% 64 KB/s 211% 128 KB/s 315% 192 KB/s 419% 256 KB/s 524% 320 KB/s 699% (should be 384 KB/s, but the iMX535 misses sending IN tokens in some SOF periods) We are using a Freescale iMX535 board running at 800 MHz as the USB host. One of the iMX535 EHCI root controllers is connected to a high speed USB hub which has two ports each connected to another high speed USB hub (giving us 8 ports for USB devices). Each hub has multi-TT support. The iMX535 is running Linux 2.6.35, and our test application uses libUSB asynchronous I/O (v1.0.16). The libUSB callback function do not process the data; rather the function only resubmits the transfer. As you are using an old, obsolete, and unsupported-by-the-community kernel version, I am going to have to point you to the vendor that provided you with this kernel (that you are paying for), and ask that you get support from them, as there's really nothing we can do for you here. And that hardware is not the best. What happens if you plug the same number of devices into a real EHCI host controller (i.e. a PCI one on a PC)? The EHCI driver should handle this just fine, odds are the issues are down in the iMX535 driver, or possibly, the silicon for that chip as I don't think it was designed to support that many devices (read the data sheet for specifics, but I thought I saw that years ago...) Best of luck, 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: EHCI driver breaks at 6 endpoints
On Tue, Jul 23, 2013 at 10:33 AM, Stoddard, Nate (GE Healthcare) nate.stodd...@med.ge.com wrote: Hello, We are attempting to create a system that will support 8 full speed USB devices each sending a 64-byte transfer every 1ms via interrupt endpoints. When we connect 5 full speed USB devices our USB host use 24% of the CPU, but when we connect a 6th device, the CPU goes to 100%. Has anyone else seen this type of CPU jump or know what could be causing it? Devices CPU Total Throughput 1 7% 64 KB/s 2 11% 128 KB/s 3 15% 192 KB/s 4 19% 256 KB/s 5 24% 320 KB/s 6 99% (should be 384 KB/s, but the iMX535 misses sending IN tokens in some SOF periods) We are using a Freescale iMX535 board running at 800 MHz as the USB host. One of the iMX535 EHCI root controllers is connected to a high speed USB hub which has two ports each connected to another high speed USB hub (giving us 8 ports for USB devices). Each hub has multi-TT support. The iMX535 is running Linux 2.6.35, and our test application uses Please try it on a 3.10.2 kernel and let us know the result. Regards, Fabio Estevam -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] usb: dwc3: ep0: don't change to configured state too early
Hi,
On Tue, Jul 23, 2013 at 10:06:15AM -0400, Alan Stern wrote:
@@ -148,6 +148,7 @@ static int __dwc3_gadget_ep0_queue(struct dwc3_ep
*dep,
direction = !dwc-ep0_expect_in;
dwc-delayed_status = false;
+ usb_gadget_set_state(dwc-gadget,
USB_STATE_CONFIGURED);
Isn't this overkill? Do you really want to call usb_gadget_set_state()
every time the gadget driver queues a transfer on ep0?
Or am I missing an important part of the context?
heh, you're missing context, that will only be called when we had
delayed status flag set:
| static int __dwc3_gadget_ep0_queue(struct dwc3_ep *dep,
| struct dwc3_request *req)
| {
[ ... ]
| /*
|* In case gadget driver asked us to delay the STATUS phase,
|* handle it here.
|*/
| if (dwc-delayed_status) {
| unsigneddirection;
|
| direction = !dwc-ep0_expect_in;
| dwc-delayed_status = false;
| usb_gadget_set_state(dwc-gadget, USB_STATE_CONFIGURED);
I see. Doesn't the mass-storage gadget also use delayed status when
going into the _un_configured state?
no it doesn't, we bail out early if config number is zero, look at
composite.c and you'll see in case of configuration zero, we just make
sure to disable all endpoints and don't call -set_alt():
| static void reset_config(struct usb_composite_dev *cdev)
| {
| struct usb_function *f;
|
| DBG(cdev, reset config\n);
|
| list_for_each_entry(f, cdev-config-functions, list) {
| if (f-disable)
| f-disable(f);
|
| bitmap_zero(f-endpoints, 32);
| }
| cdev-config = NULL;
| }
|
| static int set_config(struct usb_composite_dev *cdev,
| const struct usb_ctrlrequest *ctrl, unsigned number)
| {
| struct usb_gadget *gadget = cdev-gadget;
| struct usb_configuration *c = NULL;
| int result = -EINVAL;
| unsignedpower = gadget_is_otg(gadget) ? 8 : 100;
| int tmp;
|
| if (number) {
| list_for_each_entry(c, cdev-configs, list) {
| if (c-bConfigurationValue == number) {
| /*
|* We disable the FDs of the previous
|* configuration only if the new configuration
|* is a valid one
|*/
| if (cdev-config)
| reset_config(cdev);
| result = 0;
| break;
| }
| }
| if (result 0)
| goto done;
| } else { /* Zero configuration value - need to reset the config */
| if (cdev-config)
| reset_config(cdev);
| result = 0;
| }
|
| INFO(cdev, %s config #%d: %s\n,
|usb_speed_string(gadget-speed),
|number, c ? c-label : unconfigured);
|
| if (!c)
| goto done;
[ ... ]
| done:
| usb_gadget_vbus_draw(gadget, power);
| if (result = 0 cdev-delayed_status)
| result = USB_GADGET_DELAYED_STATUS;
| return result;
| }
|
| int
| composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
| {
| struct usb_composite_dev*cdev = get_gadget_data(gadget);
| struct usb_request *req = cdev-req;
| int value = -EOPNOTSUPP;
| int status = 0;
| u16 w_index = le16_to_cpu(ctrl-wIndex);
| u8 intf = w_index 0xFF;
| u16 w_value = le16_to_cpu(ctrl-wValue);
| u16 w_length = le16_to_cpu(ctrl-wLength);
| struct usb_function *f = NULL;
| u8 endp;
[ ... ]
| switch (ctrl-bRequest) {
[ ... ]
| /* any number of configs can work */
| case USB_REQ_SET_CONFIGURATION:
| if (ctrl-bRequestType != 0)
| goto unknown;
| if (gadget_is_otg(gadget)) {
| if (gadget-a_hnp_support)
| DBG(cdev, HNP available\n);
| else if (gadget-a_alt_hnp_support)
| DBG(cdev, HNP on another port\n);
| else
| VDBG(cdev, HNP inactive\n);
| }
| spin_lock(cdev-lock);
| value = set_config(cdev, ctrl, w_value);
| spin_unlock(cdev-lock);
| break;
[ ...]
| }
|
| /* respond with data
[PATCH V2 1/2] USB: OHCI: make ohci-ep93xx a separate driver
Separate the OHCI EP93XX host controller driver from ohci-hcd
host code so that it can be built as a separate driver module.
This work is part of enabling multi-platform kernels on ARM.
Signed-off-by: Manjunath Goudar manjunath.gou...@linaro.org
Cc: Arnd Bergmann a...@arndb.de
Cc: Alan Stern st...@rowland.harvard.edu
Cc: Greg KH g...@kroah.com
Cc: linux-usb@vger.kernel.org
V2:
-ohci_hcd_init() statements are removed,
because by default it is called in ohci_setup().
---
drivers/usb/host/Kconfig |8
drivers/usb/host/Makefile |1 +
drivers/usb/host/ohci-ep93xx.c | 81 +---
drivers/usb/host/ohci-hcd.c| 19 --
4 files changed, 44 insertions(+), 65 deletions(-)
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index f7f7823..cdfaa04 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -414,6 +414,14 @@ config USB_OHCI_HCD_DA8XX
Enables support for the on-chip OHCI controller on
DA8xx/OMAP-L1x chips.
+config USB_OHCI_HCD_EP93XX
+ tristate Support for EP93XX on-chip OHCI USB controller
+ depends on USB_OHCI_HCD ARCH_EP93XX
+ default y
+ ---help---
+ Enables support for the on-chip OHCI controller on
+ EP93XX chips.
+
config USB_OHCI_HCD_AT91
tristate Support for Atmel on-chip OHCI USB controller
depends on USB_OHCI_HCD ARCH_AT91
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index f8d59371..3fee3ea 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -54,6 +54,7 @@ obj-$(CONFIG_USB_OHCI_HCD_AT91) += ohci-at91.o
obj-$(CONFIG_USB_OHCI_HCD_S3C) += ohci-s3c2410.o
obj-$(CONFIG_USB_OHCI_HCD_LPC32XX) += ohci-nxp.o
obj-$(CONFIG_USB_OHCI_HCD_DA8XX) += ohci-da8xx.o
+obj-$(CONFIG_USB_OHCI_HCD_EP93XX) += ohci-ep93xx.o
obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o
obj-$(CONFIG_USB_FHCI_HCD) += fhci.o
diff --git a/drivers/usb/host/ohci-ep93xx.c b/drivers/usb/host/ohci-ep93xx.c
index f0aaa48..30b0ffe 100644
--- a/drivers/usb/host/ohci-ep93xx.c
+++ b/drivers/usb/host/ohci-ep93xx.c
@@ -25,8 +25,21 @@
#include linux/clk.h
#include linux/device.h
-#include linux/signal.h
+#include linux/io.h
+#include linux/kernel.h
+#include linux/module.h
#include linux/platform_device.h
+#include linux/signal.h
+#include linux/usb.h
+#include linux/usb/hcd.h
+
+#include ohci.h
+
+#define DRIVER_DESC OHCI EP93xx driver
+
+static const char hcd_name[] = ohci-ep93xx;
+
+static struct hc_driver __read_mostly ohci_ep93xx_hc_driver;
static struct clk *usb_host_clock;
@@ -45,6 +58,7 @@ static int usb_hcd_ep93xx_probe(const struct hc_driver
*driver,
{
int retval;
struct usb_hcd *hcd;
+ struct ohci_hcd *ohci;
if (pdev-resource[1].flags != IORESOURCE_IRQ) {
dev_dbg(pdev-dev, resource[1] is not IORESOURCE_IRQ\n);
@@ -79,8 +93,6 @@ static int usb_hcd_ep93xx_probe(const struct hc_driver
*driver,
ep93xx_start_hc(pdev-dev);
- ohci_hcd_init(hcd_to_ohci(hcd));
-
retval = usb_add_hcd(hcd, pdev-resource[1].start, 0);
if (retval == 0)
return retval;
@@ -107,48 +119,6 @@ static void usb_hcd_ep93xx_remove(struct usb_hcd *hcd,
usb_put_hcd(hcd);
}
-static int ohci_ep93xx_start(struct usb_hcd *hcd)
-{
- struct ohci_hcd *ohci = hcd_to_ohci(hcd);
- int ret;
-
- if ((ret = ohci_init(ohci)) 0)
- return ret;
-
- if ((ret = ohci_run(ohci)) 0) {
- dev_err(hcd-self.controller, can't start %s\n,
- hcd-self.bus_name);
- ohci_stop(hcd);
- return ret;
- }
-
- return 0;
-}
-
-static struct hc_driver ohci_ep93xx_hc_driver = {
- .description= hcd_name,
- .product_desc = EP93xx OHCI,
- .hcd_priv_size = sizeof(struct ohci_hcd),
- .irq= ohci_irq,
- .flags = HCD_USB11 | HCD_MEMORY,
- .start = ohci_ep93xx_start,
- .stop = ohci_stop,
- .shutdown = ohci_shutdown,
- .urb_enqueue= ohci_urb_enqueue,
- .urb_dequeue= ohci_urb_dequeue,
- .endpoint_disable = ohci_endpoint_disable,
- .get_frame_number = ohci_get_frame,
- .hub_status_data= ohci_hub_status_data,
- .hub_control= ohci_hub_control,
-#ifdef CONFIG_PM
- .bus_suspend= ohci_bus_suspend,
- .bus_resume = ohci_bus_resume,
-#endif
- .start_port_reset = ohci_start_port_reset,
-};
-
-extern int usb_disabled(void);
-
static int ohci_hcd_ep93xx_drv_probe(struct platform_device *pdev)
{
int ret;
@@ -206,7 +176,6 @@ static int ohci_hcd_ep93xx_drv_resume(struct
platform_device *pdev)
}
#endif
-
static struct platform_driver
[PATCH V2 2/2] USB: OHCI: make ohci-pxa27x a separate driver
Separate the OHCI pxa27x/pxa3xx host controller driver from
ohci-hcd host code so that it can be built as a separate driver
module. This work is part of enabling multi-platform kernels on
ARM.
Signed-off-by: Manjunath Goudar manjunath.gou...@linaro.org
Cc: Arnd Bergmann a...@arndb.de
Cc: Greg KH g...@kroah.com
Cc: Alan Stern st...@rowland.harvard.edu
Cc: linux-usb@vger.kernel.org
V2:
-Changed ohci_hcd and pxa27x_ohci struct variable names.
1 ohci_hcd struct variable name is ohci.
2 pxa27x_ohci struct variable name is pxa_ohci.
---
drivers/usb/host/Kconfig |8 ++
drivers/usb/host/Makefile |1 +
drivers/usb/host/ohci-hcd.c|5 -
drivers/usb/host/ohci-pxa27x.c | 240 ++--
4 files changed, 114 insertions(+), 140 deletions(-)
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index cdfaa04..0d7ee36 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -422,6 +422,14 @@ config USB_OHCI_HCD_EP93XX
Enables support for the on-chip OHCI controller on
EP93XX chips.
+config USB_OHCI_HCD_PXA27X
+ tristate Support for PXA27X/PXA3XX on-chip OHCI USB controller
+ depends on USB_OHCI_HCD (PXA27x || PXA3xx)
+ default y
+ ---help---
+ Enables support for the on-chip OHCI controller on
+ PXA27x/PXA3xx chips.
+
config USB_OHCI_HCD_AT91
tristate Support for Atmel on-chip OHCI USB controller
depends on USB_OHCI_HCD ARCH_AT91
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 3fee3ea..8b7fa89 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_USB_OHCI_HCD_S3C)+= ohci-s3c2410.o
obj-$(CONFIG_USB_OHCI_HCD_LPC32XX) += ohci-nxp.o
obj-$(CONFIG_USB_OHCI_HCD_DA8XX) += ohci-da8xx.o
obj-$(CONFIG_USB_OHCI_HCD_EP93XX) += ohci-ep93xx.o
+obj-$(CONFIG_USB_OHCI_HCD_PXA27X) += ohci-pxa27x.o
obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o
obj-$(CONFIG_USB_FHCI_HCD) += fhci.o
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 3f46cff..f601dde 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -1184,11 +1184,6 @@ MODULE_LICENSE (GPL);
#define SA_DRIVER ohci_hcd_sa_driver
#endif
-#if defined(CONFIG_PXA27x) || defined(CONFIG_PXA3xx)
-#include ohci-pxa27x.c
-#define PLATFORM_DRIVERohci_hcd_pxa27x_driver
-#endif
-
#ifdef CONFIG_USB_OHCI_HCD_PPC_OF
#include ohci-ppc-of.c
#define OF_PLATFORM_DRIVER ohci_hcd_ppc_of_driver
diff --git a/drivers/usb/host/ohci-pxa27x.c b/drivers/usb/host/ohci-pxa27x.c
index 5fb91f1..3d97c63 100644
--- a/drivers/usb/host/ohci-pxa27x.c
+++ b/drivers/usb/host/ohci-pxa27x.c
@@ -19,15 +19,26 @@
* This file is licenced under the GPL.
*/
-#include linux/device.h
-#include linux/signal.h
-#include linux/platform_device.h
#include linux/clk.h
+#include linux/device.h
+#include linux/io.h
+#include linux/kernel.h
+#include linux/module.h
#include linux/of_platform.h
#include linux/of_gpio.h
-#include mach/hardware.h
#include linux/platform_data/usb-ohci-pxa27x.h
#include linux/platform_data/usb-pxa3xx-ulpi.h
+#include linux/platform_device.h
+#include linux/signal.h
+#include linux/usb.h
+#include linux/usb/hcd.h
+#include linux/usb/otg.h
+
+#include mach/hardware.h
+
+#include ohci.h
+
+#define DRIVER_DESC OHCI PXA27x/PXA3x driver
/*
* UHC: USB Host Controller (OHCI-like) register definitions
@@ -101,11 +112,11 @@
#define PXA_UHC_MAX_PORTNUM3
-struct pxa27x_ohci {
- /* must be 1st member here for hcd_to_ohci() to work */
- struct ohci_hcd ohci;
+static const char hcd_name[] = ohci-pxa27x;
+
+static struct hc_driver __read_mostly ohci_pxa27x_hc_driver;
- struct device *dev;
+struct pxa27x_ohci {
struct clk *clk;
void __iomem*mmio_base;
};
@@ -122,10 +133,10 @@ struct pxa27x_ohci {
PMM_PERPORT_MODE -- PMM per port switching mode
Ports are powered individually.
*/
-static int pxa27x_ohci_select_pmm(struct pxa27x_ohci *ohci, int mode)
+static int pxa27x_ohci_select_pmm(struct pxa27x_ohci *pxa_ohci, int mode)
{
- uint32_t uhcrhda = __raw_readl(ohci-mmio_base + UHCRHDA);
- uint32_t uhcrhdb = __raw_readl(ohci-mmio_base + UHCRHDB);
+ uint32_t uhcrhda = __raw_readl(pxa_ohci-mmio_base + UHCRHDA);
+ uint32_t uhcrhdb = __raw_readl(pxa_ohci-mmio_base + UHCRHDB);
switch (mode) {
case PMM_NPS_MODE:
@@ -149,20 +160,18 @@ static int pxa27x_ohci_select_pmm(struct pxa27x_ohci
*ohci, int mode)
uhcrhda |= RH_A_NPS;
}
- __raw_writel(uhcrhda, ohci-mmio_base + UHCRHDA);
- __raw_writel(uhcrhdb, ohci-mmio_base + UHCRHDB);
+ __raw_writel(uhcrhda, pxa_ohci-mmio_base + UHCRHDA);
+ __raw_writel(uhcrhdb, pxa_ohci-mmio_base + UHCRHDB);
return 0;
}
-extern int usb_disabled(void);
-
Re: a small patch that fixes the ohci warn irq nobody cared on shutdown.
On Tue, Jul 23, 2013 at 07:13:02AM +, Caizhiyong wrote: Hi: Here's a small patch that fixes the ohci shutdown. The patch is against v3.4.54. 3.4 is quite old, and we can't add new patches to it that are not also in Linus's tree. Can you verify that 3.11-rc2 also needs this patch, and redo it against that tree so that we can accept it? From cc16ef2f15300201cc3f680b7df20ecded28daa4 Mon Sep 17 00:00:00 2001 From: Cai Zhiyong caizhiy...@huawei.com Date: Tue, 23 Jul 2013 12:17:01 +0800 Subject: [PATCH] USB: ohci_usb warn irq nobody cared on shutdown Also, please don't send a patch which would require me to edit it by hand in order to apply it (I handle thousands of patches a month). Just use 'git send-email' to send it, which will properly create the needed format. For more details about this, please read Documentation/SubmittingPatches. 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 V3] usb: Add Device Tree support to XHCI Platform driver
This compiles without CONFIG_OF because of_match_ptr() assigns NULL if
CONFIG_OF is not defined.
On Tue, Jul 23, 2013 at 10:35 AM, Matthijs Kooijman matth...@stdin.nl wrote:
Hi Al,
@@ -212,11 +213,19 @@ static int xhci_plat_remove(struct platform_device
*dev)
return 0;
}
+#ifdef CONFIG_OF
+static const struct of_device_id usb_xhci_of_match[] = {
+ { .compatible = usb-xhci },
+ {},
+};
+#endif
+
static struct platform_driver usb_xhci_driver = {
.probe = xhci_plat_probe,
.remove = xhci_plat_remove,
.driver = {
.name = xhci-hcd,
+ .of_match_table = of_match_ptr(usb_xhci_of_match),
},
};
MODULE_ALIAS(platform:xhci-hcd);
This looks like it wouldn't compile without CONFIG_OF?
Gr.
Matthijs
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V3] usb: Add Device Tree support to XHCI Platform driver
Hi Alan, This compiles without CONFIG_OF because of_match_ptr() assigns NULL if CONFIG_OF is not defined. Ah, I guess that makes sense :-) Apologies for the noise... Gr. Matthijs -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, Jul 23, 2013 at 08:48:24PM +0530, Kishon Vijay Abraham I wrote:
Hi,
On Tuesday 23 July 2013 08:07 PM, Alan Stern wrote:
On Tue, 23 Jul 2013, Tomasz Figa wrote:
On Tuesday 23 of July 2013 09:29:32 Tomasz Figa wrote:
Hi Alan,
Thanks for helping to clarify the issues here.
Okay. Are PHYs _always_ platform devices?
They can be i2c, spi or any other device types as well.
In those other cases, presumably there is no platform data associated
with the PHY since it isn't a platform device. Then how does the
kernel know which controller is attached to the PHY? Is this spelled
out in platform data associated with the PHY's i2c/spi/whatever parent?
Yes. I think we could use i2c_board_info for passing platform data.
PHY. Currently this information is represented by name or
ID
strings embedded in platform data.
right. It's embedded in the platform data of the controller.
It must also be embedded in the PHY's platform data somehow.
Otherwise, how would the kernel know which PHY to use?
By using a PHY lookup as Stephen and I suggested in our previous
replies. Without any extra data in platform data. (I have even posted a
code example.)
I don't understand, because I don't know what a PHY lookup does.
It is how the PHY framework finds a PHY, when the controller (say USB)requests
a PHY from the PHY framework.
In this case, it doesn't matter where the platform_device structures
are created or where the driver source code is. Let's take a simple
example. Suppose the system design includes a PHY named foo. Then
the board file could contain:
struct phy_info { ... } phy_foo;
EXPORT_SYMBOL_GPL(phy_foo);
and a header file would contain:
extern struct phy_info phy_foo;
The PHY supplier could then call phy_create(phy_foo), and the PHY
client could call phy_find(phy_foo). Or something like that; make up
your own structure tags and function names.
It's still possible to have conflicts, but now two PHYs with the same
name (or a misspelled name somewhere) will cause an error at link
time.
This is incorrect, sorry. First of all it's a layering violation - you
export random driver-specific symbols from one driver to another. Then
No, that's not what I said. Neither the PHY driver nor the controller
driver exports anything to the other. Instead, both drivers use data
exported by the board file.
I think instead we can use the same data while creating the platform data of
the controller and the PHY.
The PHY driver while creating the PHY (using PHY framework) will also pass the
*data* it actually got from the platform data to the framework.
The PHY user driver (USB), while requesting for the PHY (from the PHY
framework) will pass the *data* it got from its platform data.
The PHY framework can do a comparison of the *data* pointers it has and return
the appropriate PHY to the controller.
imagine 4 SoCs - A, B, C, D. There are two PHY types PHY1 and PHY2 and
there are two types of consumer drivers (e.g. USB host controllers). Now
consider following mapping:
SoC PHY consumer
A PHY1HOST1
B PHY1HOST2
C PHY2HOST1
D PHY2HOST2
So we have to be able to use any of the PHYs with any of the host
drivers. This means you would have to export symbol with the same name
from both PHY drivers, which obviously would not work in this case,
because having both drivers enabled (in a multiplatform aware
configuration) would lead to linking conflict.
You're right; the scheme was too simple. Instead, the board file must
export two types of data structures, one for PHYs and one for
controllers. Like this:
struct phy_info {
/* Info for the controller attached to this PHY */
struct controller_info *hinfo;
};
struct controller_info {
/* Info for the PHY which this controller is attached to */
struct phy_info *pinfo;
};
The board file for SoC A would contain:
struct phy_info phy1 = {host1);
EXPORT_SYMBOL(phy1);
struct controller_info host1 = {phy1};
EXPORT_SYMBOL(host1);
The board file for SoC B would contain:
struct phy_info phy1 = {host2);
EXPORT_SYMBOL(phy1);
struct controller_info host2 = {phy1};
EXPORT_SYMBOL(host2);
I meant something like this
struct phy_info {
const char *name;
};
struct phy_platform_data {
.
.
struct phy_info *info;
};
struct usb_controller_platform_data {
.
.
struct phy_info *info;
};
struct phy_info phy_info;
While creating the phy device
struct phy_platform_data phy_data;
phy_data.info = info;
platform_device_add_data(pdev, phy_data, sizeof(*phy_data))
platform_device_add();
While creating the controller device
struct usb_controller_platform_data controller_data;
controller_data.info = info;
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
Hi Greg,
On Tuesday 23 July 2013 09:48 PM, Greg KH wrote:
On Tue, Jul 23, 2013 at 08:48:24PM +0530, Kishon Vijay Abraham I wrote:
Hi,
On Tuesday 23 July 2013 08:07 PM, Alan Stern wrote:
On Tue, 23 Jul 2013, Tomasz Figa wrote:
On Tuesday 23 of July 2013 09:29:32 Tomasz Figa wrote:
Hi Alan,
Thanks for helping to clarify the issues here.
Okay. Are PHYs _always_ platform devices?
They can be i2c, spi or any other device types as well.
In those other cases, presumably there is no platform data associated
with the PHY since it isn't a platform device. Then how does the
kernel know which controller is attached to the PHY? Is this spelled
out in platform data associated with the PHY's i2c/spi/whatever parent?
.
.
snip
.
.
static struct phy *phy_lookup(void *priv) {
.
.
if (phy-priv==priv) //instead of string comparison, we'll use
pointer
return phy;
}
PHY driver should be like
phy_create((dev, ops, pdata-info);
The controller driver would do
phy_get(dev, NULL, pdata-info);
Now the PHY framework will check for a match of *priv* pointer and return
the PHY.
I think this should be possible?
Ick, no. Why can't you just pass the pointer to the phy itself? If you
had a priv pointer to search from, then you could have just passed the
original phy pointer in the first place, right?
The issue is that a string name is not going to scale at all, as it
requires hard-coded information that will change over time (as the
existing clock interface is already showing.)
Please just pass the real phy pointer around, that's what it is there
for. Your board binding logic/code should be able to handle this, as
it somehow was going to do the same thing with a name.
The problem is the board file won't have the *phy* pointer. *phy* pointer is
created at a much later time when the phy driver is probed.
Thanks
Kishon
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, Jul 23, 2013 at 09:58:34PM +0530, Kishon Vijay Abraham I wrote:
Hi Greg,
On Tuesday 23 July 2013 09:48 PM, Greg KH wrote:
On Tue, Jul 23, 2013 at 08:48:24PM +0530, Kishon Vijay Abraham I wrote:
Hi,
On Tuesday 23 July 2013 08:07 PM, Alan Stern wrote:
On Tue, 23 Jul 2013, Tomasz Figa wrote:
On Tuesday 23 of July 2013 09:29:32 Tomasz Figa wrote:
Hi Alan,
Thanks for helping to clarify the issues here.
Okay. Are PHYs _always_ platform devices?
They can be i2c, spi or any other device types as well.
In those other cases, presumably there is no platform data associated
with the PHY since it isn't a platform device. Then how does the
kernel know which controller is attached to the PHY? Is this spelled
out in platform data associated with the PHY's i2c/spi/whatever parent?
.
.
snip
.
.
static struct phy *phy_lookup(void *priv) {
.
.
if (phy-priv==priv) //instead of string comparison, we'll use
pointer
return phy;
}
PHY driver should be like
phy_create((dev, ops, pdata-info);
The controller driver would do
phy_get(dev, NULL, pdata-info);
Now the PHY framework will check for a match of *priv* pointer and return
the PHY.
I think this should be possible?
Ick, no. Why can't you just pass the pointer to the phy itself? If you
had a priv pointer to search from, then you could have just passed the
original phy pointer in the first place, right?
The issue is that a string name is not going to scale at all, as it
requires hard-coded information that will change over time (as the
existing clock interface is already showing.)
Please just pass the real phy pointer around, that's what it is there
for. Your board binding logic/code should be able to handle this, as
it somehow was going to do the same thing with a name.
The problem is the board file won't have the *phy* pointer. *phy* pointer is
created at a much later time when the phy driver is probed.
Ok, then save it then, as no one could have used it before then, right?
All I don't want to see is any get by name/void * functions in the
api, as that way is fragile and will break, as people have already
shown.
Just pass the real pointer around. If that is somehow a problem, then
something larger is a problem with how board devices are tied together :)
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 01/15] drivers: phy: add generic PHY framework
On Tuesday 23 of July 2013 09:18:46 Greg KH wrote:
On Tue, Jul 23, 2013 at 08:48:24PM +0530, Kishon Vijay Abraham I wrote:
Hi,
On Tuesday 23 July 2013 08:07 PM, Alan Stern wrote:
On Tue, 23 Jul 2013, Tomasz Figa wrote:
On Tuesday 23 of July 2013 09:29:32 Tomasz Figa wrote:
Hi Alan,
Thanks for helping to clarify the issues here.
Okay. Are PHYs _always_ platform devices?
They can be i2c, spi or any other device types as well.
In those other cases, presumably there is no platform data associated
with the PHY since it isn't a platform device. Then how does the
kernel know which controller is attached to the PHY? Is this spelled
out in platform data associated with the PHY's i2c/spi/whatever
parent?
Yes. I think we could use i2c_board_info for passing platform data.
PHY. Currently this information is represented by name or
ID
strings embedded in platform data.
right. It's embedded in the platform data of the controller.
It must also be embedded in the PHY's platform data somehow.
Otherwise, how would the kernel know which PHY to use?
By using a PHY lookup as Stephen and I suggested in our previous
replies. Without any extra data in platform data. (I have even
posted a
code example.)
I don't understand, because I don't know what a PHY lookup does.
It is how the PHY framework finds a PHY, when the controller (say
USB)requests a PHY from the PHY framework.
In this case, it doesn't matter where the platform_device
structures
are created or where the driver source code is. Let's take a
simple
example. Suppose the system design includes a PHY named foo.
Then
the board file could contain:
struct phy_info { ... } phy_foo;
EXPORT_SYMBOL_GPL(phy_foo);
and a header file would contain:
extern struct phy_info phy_foo;
The PHY supplier could then call phy_create(phy_foo), and the PHY
client could call phy_find(phy_foo). Or something like that;
make up
your own structure tags and function names.
It's still possible to have conflicts, but now two PHYs with the
same
name (or a misspelled name somewhere) will cause an error at link
time.
This is incorrect, sorry. First of all it's a layering violation -
you
export random driver-specific symbols from one driver to another.
Then
No, that's not what I said. Neither the PHY driver nor the
controller
driver exports anything to the other. Instead, both drivers use data
exported by the board file.
I think instead we can use the same data while creating the platform
data of the controller and the PHY.
The PHY driver while creating the PHY (using PHY framework) will also
pass the *data* it actually got from the platform data to the
framework. The PHY user driver (USB), while requesting for the PHY
(from the PHY framework) will pass the *data* it got from its platform
data.
The PHY framework can do a comparison of the *data* pointers it has and
return the appropriate PHY to the controller.
imagine 4 SoCs - A, B, C, D. There are two PHY types PHY1 and PHY2
and
there are two types of consumer drivers (e.g. USB host
controllers). Now
consider following mapping:
SoC PHY consumer
A PHY1HOST1
B PHY1HOST2
C PHY2HOST1
D PHY2HOST2
So we have to be able to use any of the PHYs with any of the host
drivers. This means you would have to export symbol with the same
name
from both PHY drivers, which obviously would not work in this case,
because having both drivers enabled (in a multiplatform aware
configuration) would lead to linking conflict.
You're right; the scheme was too simple. Instead, the board file
must
export two types of data structures, one for PHYs and one for
controllers. Like this:
struct phy_info {
/* Info for the controller attached to this PHY */
struct controller_info *hinfo;
};
struct controller_info {
/* Info for the PHY which this controller is attached to */
struct phy_info *pinfo;
};
The board file for SoC A would contain:
struct phy_info phy1 = {host1);
EXPORT_SYMBOL(phy1);
struct controller_info host1 = {phy1};
EXPORT_SYMBOL(host1);
The board file for SoC B would contain:
struct phy_info phy1 = {host2);
EXPORT_SYMBOL(phy1);
struct controller_info host2 = {phy1};
EXPORT_SYMBOL(host2);
I meant something like this
struct phy_info {
const char *name;
};
struct phy_platform_data {
.
.
struct phy_info *info;
};
struct usb_controller_platform_data {
.
.
struct phy_info *info;
};
struct phy_info phy_info;
While creating the phy device
struct phy_platform_data phy_data;
Re: [PATCH 2/2] usb: dwc3: ep0: don't change to configured state too early
On Tue, 23 Jul 2013, Felipe Balbi wrote: I see. Doesn't the mass-storage gadget also use delayed status when going into the _un_configured state? no it doesn't, we bail out early if config number is zero, look at composite.c and you'll see in case of configuration zero, we just make sure to disable all endpoints and don't call -set_alt(): This is my fault, for not being more familiar with the composite framework, and no doubt I will regret asking this, but... Doesn't that approach leave the function drivers sitting around with all their resources still allocated? When do those resources get deallocated? When the gadget disconnects from the host? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 15/16] dmaengine: add transfered member to dma_async_tx_descriptor
On 07/22/2013 08:52 PM, Sergei Shtylyov wrote:
@@ -416,6 +418,7 @@ struct dma_async_tx_descriptor {
dma_cookie_t (*tx_submit)(struct dma_async_tx_descriptor *tx);
dma_async_tx_callback callback;
void *callback_param;
+unsigned int transfered;
Correct grammar is transferred.
Thanks, corrected.
WBR, Sergei
Sebastian
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 14/16] usb: musb: dsps: add MUSB_DEVCTL_SESSION back after removal
On 07/23/2013 07:28 PM, Bin Liu wrote: Hi, Hi, On Mon, Jul 22, 2013 at 1:10 PM, Sebastian Andrzej Siewior bige...@linutronix.de mailto:bige...@linutronix.de wrote: This patch adds the MUSB_DEVCTL_SESSION back after it has been removed. If it is missing then the host session is not recognized. This bit is added initially added in musb_start() and removed after the first device disconnect. AFAIK, after the device is disconnected, the OTG state machine will go back to B_IDLE/A_IDLE state. SESSION is not needed in this case. Okay. In OTG mode, when no device is plugged, the ID pin is floating, you can never hold the host session in this case, even set the SESSION bit. The SESSION bit will be cleared by the controller after 100ms. In my testing the bit remains set. How is the bit supposed to come back after I connect a host device? Regards, -Bin. Sebastian -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, Jul 23, 2013 at 06:50:29PM +0200, Tomasz Figa wrote:
Ick, no. Why can't you just pass the pointer to the phy itself? If you
had a priv pointer to search from, then you could have just passed the
original phy pointer in the first place, right?
IMHO it would be better if you provided some code example, but let's try to
check if I understood you correctly.
It's not my code that I want to have added, so I don't have to write
examples, I just get to complain about the existing stuff :)
8
[Board file]
static struct phy my_phy;
static struct platform_device phy_pdev = {
/* ... */
.platform_data = my_phy;
/* ... */
};
static struct platform_device phy_pdev = {
/* ... */
.platform_data = my_phy;
/* ... */
};
[Provider driver]
struct phy *phy = pdev-dev.platform_data;
ret = phy_create(phy);
[Consumer driver]
struct phy *phy = pdev-dev.platform_data;
ret = phy_get(pdev-dev, phy);
8
Is this what you mean?
No. Well, kind of. What's wrong with using the platform data structure
unique to the board to have the pointer?
For example (just randomly picking one), the ata-pxa driver would change
include/linux/platform_data/ata-pxa.h to have a phy pointer in it:
struct phy;
struct pata_pxa_pdata {
/* PXA DMA DREQ0:2 pin */
uint32_tdma_dreq;
/* Register shift */
uint32_treg_shift;
/* IRQ flags */
uint32_tirq_flags;
/* PHY */
struct phy *phy;
};
Then, when you create the platform, set the phy* pointer with a call to
phy_create(). Then you can use that pointer wherever that plaform data
is available (i.e. whereever platform_data is at).
The issue is that a string name is not going to scale at all, as it
requires hard-coded information that will change over time (as the
existing clock interface is already showing.)
I fully agree that a simple, single string will not scale even in some, not
so uncommon cases, but there is already a lot of existing lookup solutions
over the kernel and so there is no point in introducing another one.
I'm trying to get _rid_ of lookup solutions and just use a real
pointer, as you should. I'll go tackle those other ones after this one
is taken care of, to show how the others should be handled as well.
Please just pass the real phy pointer around, that's what it is there
for. Your board binding logic/code should be able to handle this, as
it somehow was going to do the same thing with a name.
It's technically correct, but quality of this solution isn't really nice,
because it's a layering violation (at least if I understood what you mean).
This is because you need to have full definition of struct phy in board file
and a structure that is used as private data in PHY core comes from
platform code.
No, just a pointer, you don't need the full structure until you get to
some .c code that actually manipulates the phy itself, for all other
places, you are just dealing with a pointer and a structure you never
reference.
Does that make more sense?
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 01/15] drivers: phy: add generic PHY framework
On Tue, Jul 23, 2013 at 10:37:05AM -0400, Alan Stern wrote: On Tue, 23 Jul 2013, Tomasz Figa wrote: Okay. Are PHYs _always_ platform devices? They can be i2c, spi or any other device types as well. In those other cases, presumably there is no platform data associated with the PHY since it isn't a platform device. Then how does the kernel know which controller is attached to the PHY? Is this spelled out in platform data associated with the PHY's i2c/spi/whatever parent? Platform data is nothing to do with the platform bus - it's board specific data (ie, data for the platform) and can be done with any device. signature.asc Description: Digital signature
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, Jul 23, 2013 at 10:37:11AM -0700, Greg KH wrote: On Tue, Jul 23, 2013 at 06:50:29PM +0200, Tomasz Figa wrote: I fully agree that a simple, single string will not scale even in some, not so uncommon cases, but there is already a lot of existing lookup solutions over the kernel and so there is no point in introducing another one. I'm trying to get _rid_ of lookup solutions and just use a real pointer, as you should. I'll go tackle those other ones after this one is taken care of, to show how the others should be handled as well. What are the problems you are seeing with doing things with lookups? Having to write platform data for everything gets old fast and the code duplication is pretty tedious... signature.asc Description: Digital signature
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tuesday 23 of July 2013 10:37:11 Greg KH wrote:
On Tue, Jul 23, 2013 at 06:50:29PM +0200, Tomasz Figa wrote:
Ick, no. Why can't you just pass the pointer to the phy itself? If
you
had a priv pointer to search from, then you could have just passed
the
original phy pointer in the first place, right?
IMHO it would be better if you provided some code example, but let's
try to check if I understood you correctly.
It's not my code that I want to have added, so I don't have to write
examples, I just get to complain about the existing stuff :)
Still, I think that some small code snippets illustrating the idea are
really helpful.
8
[Board file]
static struct phy my_phy;
static struct platform_device phy_pdev = {
/* ... */
.platform_data = my_phy;
/* ... */
};
static struct platform_device phy_pdev = {
/* ... */
.platform_data = my_phy;
/* ... */
};
[Provider driver]
struct phy *phy = pdev-dev.platform_data;
ret = phy_create(phy);
[Consumer driver]
struct phy *phy = pdev-dev.platform_data;
ret = phy_get(pdev-dev, phy);
---
-8
Is this what you mean?
No. Well, kind of. What's wrong with using the platform data structure
unique to the board to have the pointer?
For example (just randomly picking one), the ata-pxa driver would change
include/linux/platform_data/ata-pxa.h to have a phy pointer in it:
struct phy;
struct pata_pxa_pdata {
/* PXA DMA DREQ0:2 pin */
uint32_tdma_dreq;
/* Register shift */
uint32_treg_shift;
/* IRQ flags */
uint32_tirq_flags;
/* PHY */
struct phy *phy;
};
Then, when you create the platform, set the phy* pointer with a call to
phy_create(). Then you can use that pointer wherever that plaform data
is available (i.e. whereever platform_data is at).
Hmm? So, do you suggest to call phy_create() from board file? What phy_ops
struct and other hardware parameters would it take?
The issue is that a string name is not going to scale at all, as it
requires hard-coded information that will change over time (as the
existing clock interface is already showing.)
I fully agree that a simple, single string will not scale even in some,
not so uncommon cases, but there is already a lot of existing lookup
solutions over the kernel and so there is no point in introducing
another one.
I'm trying to get _rid_ of lookup solutions and just use a real
pointer, as you should. I'll go tackle those other ones after this one
is taken care of, to show how the others should be handled as well.
There was a reason for introducing lookup solutions. The reason was that in
board file there is no way to get a pointer to something that is going to be
created much later in time. We don't do time travel ;-).
Please just pass the real phy pointer around, that's what it is
there
for. Your board binding logic/code should be able to handle this,
as
it somehow was going to do the same thing with a name.
It's technically correct, but quality of this solution isn't really
nice, because it's a layering violation (at least if I understood what
you mean). This is because you need to have full definition of struct
phy in board file and a structure that is used as private data in PHY
core comes from platform code.
No, just a pointer, you don't need the full structure until you get to
some .c code that actually manipulates the phy itself, for all other
places, you are just dealing with a pointer and a structure you never
reference.
Does that make more sense?
Well, to the point that I think I now understood your suggestion.
Unfortunately the suggestion alone isn't really something that can be done,
considering how driver core and generic frameworks work.
Best regards,
Tomasz
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, Jul 23, 2013 at 06:44:56PM +0100, Mark Brown wrote: On Tue, Jul 23, 2013 at 10:37:11AM -0700, Greg KH wrote: On Tue, Jul 23, 2013 at 06:50:29PM +0200, Tomasz Figa wrote: I fully agree that a simple, single string will not scale even in some, not so uncommon cases, but there is already a lot of existing lookup solutions over the kernel and so there is no point in introducing another one. I'm trying to get _rid_ of lookup solutions and just use a real pointer, as you should. I'll go tackle those other ones after this one is taken care of, to show how the others should be handled as well. What are the problems you are seeing with doing things with lookups? You don't know the id of the device you are looking up, due to multiple devices being in the system (dynamic ids, look back earlier in this thread for details about that.) Having to write platform data for everything gets old fast and the code duplication is pretty tedious... Adding a single pointer is tedious? Where is the name that you are going to lookup going to come from? That code doesn't write itself... thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 19/27] drivers/usb/phy: don't check resource with devm_ioremap_resource
devm_ioremap_resource does sanity checks on the given resource. No need to
duplicate this in the driver.
Signed-off-by: Wolfram Sang w...@the-dreams.de
---
Please apply via the subsystem-tree.
drivers/usb/phy/phy-rcar-usb.c |5 -
1 file changed, 5 deletions(-)
diff --git a/drivers/usb/phy/phy-rcar-usb.c b/drivers/usb/phy/phy-rcar-usb.c
index ae90940..deb7f97 100644
--- a/drivers/usb/phy/phy-rcar-usb.c
+++ b/drivers/usb/phy/phy-rcar-usb.c
@@ -190,11 +190,6 @@ static int rcar_usb_phy_probe(struct platform_device *pdev)
}
res0 = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (!res0) {
- dev_err(dev, Not enough platform resources\n);
- return -EINVAL;
- }
-
reg0 = devm_ioremap_resource(dev, res0);
if (IS_ERR(reg0))
return PTR_ERR(reg0);
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, Jul 23, 2013 at 07:48:11PM +0200, Tomasz Figa wrote:
On Tuesday 23 of July 2013 10:37:11 Greg KH wrote:
On Tue, Jul 23, 2013 at 06:50:29PM +0200, Tomasz Figa wrote:
Ick, no. Why can't you just pass the pointer to the phy itself? If
you
had a priv pointer to search from, then you could have just passed
the
original phy pointer in the first place, right?
IMHO it would be better if you provided some code example, but let's
try to check if I understood you correctly.
It's not my code that I want to have added, so I don't have to write
examples, I just get to complain about the existing stuff :)
Still, I think that some small code snippets illustrating the idea are
really helpful.
8
[Board file]
static struct phy my_phy;
static struct platform_device phy_pdev = {
/* ... */
.platform_data = my_phy;
/* ... */
};
static struct platform_device phy_pdev = {
/* ... */
.platform_data = my_phy;
/* ... */
};
[Provider driver]
struct phy *phy = pdev-dev.platform_data;
ret = phy_create(phy);
[Consumer driver]
struct phy *phy = pdev-dev.platform_data;
ret = phy_get(pdev-dev, phy);
---
-8
Is this what you mean?
No. Well, kind of. What's wrong with using the platform data structure
unique to the board to have the pointer?
For example (just randomly picking one), the ata-pxa driver would change
include/linux/platform_data/ata-pxa.h to have a phy pointer in it:
struct phy;
struct pata_pxa_pdata {
/* PXA DMA DREQ0:2 pin */
uint32_tdma_dreq;
/* Register shift */
uint32_treg_shift;
/* IRQ flags */
uint32_tirq_flags;
/* PHY */
struct phy *phy;
};
Then, when you create the platform, set the phy* pointer with a call to
phy_create(). Then you can use that pointer wherever that plaform data
is available (i.e. whereever platform_data is at).
Hmm? So, do you suggest to call phy_create() from board file? What phy_ops
struct and other hardware parameters would it take?
The issue is that a string name is not going to scale at all, as it
requires hard-coded information that will change over time (as the
existing clock interface is already showing.)
I fully agree that a simple, single string will not scale even in some,
not so uncommon cases, but there is already a lot of existing lookup
solutions over the kernel and so there is no point in introducing
another one.
I'm trying to get _rid_ of lookup solutions and just use a real
pointer, as you should. I'll go tackle those other ones after this one
is taken care of, to show how the others should be handled as well.
There was a reason for introducing lookup solutions. The reason was that in
board file there is no way to get a pointer to something that is going to be
created much later in time. We don't do time travel ;-).
Please just pass the real phy pointer around, that's what it is
there
for. Your board binding logic/code should be able to handle this,
as
it somehow was going to do the same thing with a name.
It's technically correct, but quality of this solution isn't really
nice, because it's a layering violation (at least if I understood what
you mean). This is because you need to have full definition of struct
phy in board file and a structure that is used as private data in PHY
core comes from platform code.
No, just a pointer, you don't need the full structure until you get to
some .c code that actually manipulates the phy itself, for all other
places, you are just dealing with a pointer and a structure you never
reference.
Does that make more sense?
Well, to the point that I think I now understood your suggestion.
Unfortunately the suggestion alone isn't really something that can be done,
considering how driver core and generic frameworks work.
Ok, given that I seem to be totally confused as to exactly how the
board-specific frameworks work, I'll take your word for it.
But again, I will not accept lookup by name type solutions, when the
name is dynamic and will change. Because you are using a name, you
can deal with a pointer, putting it _somewhere_ in your board-specific
data structures, as you are going to need to store it anyway (hint, you
had to get that name from somewhere, right?)
And maybe the way that these generic frameworks are created is wrong,
given that you don't feel that a generic pointer can be passed to the
needed devices. That seems like a huge problem, one that has already
been pointed out is causing issues with other subsystems.
So maybe they need to be fixed?
thanks,
greg k-h
--
To
Re: [PATCH v2 01/42] ARM: at91: move at91_pmc.h to include/linux/clk/at91.h
On 15:37 Wed 17 Jul , Boris BREZILLON wrote: This patch moves at91_pmc.h header from machine specific directory (arch/arm/mach-at91/include/mach/at91_pmc.h) to clk include directory (include/linux/clk/at91.h). please the at91_pmc.h in the name so we known the IP that we use and if we change it we will not have to rename it in the futur otherwise Acked-by: Jean-Christophe PLAGNIOL-VILLARD plagn...@jcrosoft.com Best Regards, J. We need this to avoid reference to machine specific headers in clk drivers. Signed-off-by: Boris BREZILLON b.brezil...@overkiz.com --- arch/arm/mach-at91/at91rm9200.c|2 +- arch/arm/mach-at91/at91sam9260.c |2 +- arch/arm/mach-at91/at91sam9261.c |2 +- arch/arm/mach-at91/at91sam9263.c |2 +- arch/arm/mach-at91/at91sam9g45.c |2 +- arch/arm/mach-at91/at91sam9n12.c |2 +- arch/arm/mach-at91/at91sam9rl.c|2 +- arch/arm/mach-at91/at91sam9x5.c|2 +- arch/arm/mach-at91/clock.c |2 +- arch/arm/mach-at91/pm.c|2 +- arch/arm/mach-at91/pm_slowclock.S |2 +- arch/arm/mach-at91/sama5d3.c |2 +- arch/arm/mach-at91/setup.c |2 +- drivers/usb/gadget/atmel_usba_udc.c|2 +- .../mach/at91_pmc.h = include/linux/clk/at91.h|2 +- 15 files changed, 15 insertions(+), 15 deletions(-) rename arch/arm/mach-at91/include/mach/at91_pmc.h = include/linux/clk/at91.h (99%) diff --git a/arch/arm/mach-at91/at91rm9200.c b/arch/arm/mach-at91/at91rm9200.c index 4aad93d..8de5b02 100644 --- a/arch/arm/mach-at91/at91rm9200.c +++ b/arch/arm/mach-at91/at91rm9200.c @@ -12,13 +12,13 @@ #include linux/module.h #include linux/reboot.h +#include linux/clk/at91.h #include asm/irq.h #include asm/mach/arch.h #include asm/mach/map.h #include asm/system_misc.h #include mach/at91rm9200.h -#include mach/at91_pmc.h #include mach/at91_st.h #include mach/cpu.h diff --git a/arch/arm/mach-at91/at91sam9260.c b/arch/arm/mach-at91/at91sam9260.c index 5de6074..db9d89a 100644 --- a/arch/arm/mach-at91/at91sam9260.c +++ b/arch/arm/mach-at91/at91sam9260.c @@ -11,6 +11,7 @@ */ #include linux/module.h +#include linux/clk/at91.h #include asm/proc-fns.h #include asm/irq.h @@ -20,7 +21,6 @@ #include mach/cpu.h #include mach/at91_dbgu.h #include mach/at91sam9260.h -#include mach/at91_pmc.h #include at91_aic.h #include at91_rstc.h diff --git a/arch/arm/mach-at91/at91sam9261.c b/arch/arm/mach-at91/at91sam9261.c index 0e07932..a4123bd 100644 --- a/arch/arm/mach-at91/at91sam9261.c +++ b/arch/arm/mach-at91/at91sam9261.c @@ -11,6 +11,7 @@ */ #include linux/module.h +#include linux/clk/at91.h #include asm/proc-fns.h #include asm/irq.h @@ -19,7 +20,6 @@ #include asm/system_misc.h #include mach/cpu.h #include mach/at91sam9261.h -#include mach/at91_pmc.h #include at91_aic.h #include at91_rstc.h diff --git a/arch/arm/mach-at91/at91sam9263.c b/arch/arm/mach-at91/at91sam9263.c index 6ce7d18..e0a1a68 100644 --- a/arch/arm/mach-at91/at91sam9263.c +++ b/arch/arm/mach-at91/at91sam9263.c @@ -11,6 +11,7 @@ */ #include linux/module.h +#include linux/clk/at91.h #include asm/proc-fns.h #include asm/irq.h @@ -18,7 +19,6 @@ #include asm/mach/map.h #include asm/system_misc.h #include mach/at91sam9263.h -#include mach/at91_pmc.h #include at91_aic.h #include at91_rstc.h diff --git a/arch/arm/mach-at91/at91sam9g45.c b/arch/arm/mach-at91/at91sam9g45.c index 474ee04..29ba2ca 100644 --- a/arch/arm/mach-at91/at91sam9g45.c +++ b/arch/arm/mach-at91/at91sam9g45.c @@ -12,13 +12,13 @@ #include linux/module.h #include linux/dma-mapping.h +#include linux/clk/at91.h #include asm/irq.h #include asm/mach/arch.h #include asm/mach/map.h #include asm/system_misc.h #include mach/at91sam9g45.h -#include mach/at91_pmc.h #include mach/cpu.h #include at91_aic.h diff --git a/arch/arm/mach-at91/at91sam9n12.c b/arch/arm/mach-at91/at91sam9n12.c index c7d670d..c270503 100644 --- a/arch/arm/mach-at91/at91sam9n12.c +++ b/arch/arm/mach-at91/at91sam9n12.c @@ -8,12 +8,12 @@ #include linux/module.h #include linux/dma-mapping.h +#include linux/clk/at91.h #include asm/irq.h #include asm/mach/arch.h #include asm/mach/map.h #include mach/at91sam9n12.h -#include mach/at91_pmc.h #include mach/cpu.h #include board.h diff --git a/arch/arm/mach-at91/at91sam9rl.c b/arch/arm/mach-at91/at91sam9rl.c index d4ec0d9..2694bd1 100644 --- a/arch/arm/mach-at91/at91sam9rl.c +++ b/arch/arm/mach-at91/at91sam9rl.c @@ -10,6 +10,7 @@ */ #include linux/module.h +#include linux/clk/at91.h #include asm/proc-fns.h #include asm/irq.h
Re: FUSB200 xhci issue
On Tuesday, July 23, 2013 06:59:46 AM Oleksij Rempel wrote: Am 22.07.2013 23:23, schrieb Christian Lamparter: On Monday, July 22, 2013 10:47:41 PM Oleksij Rempel wrote: Am 22.07.2013 21:54, schrieb Christian Lamparter: Hello! On Monday, July 22, 2013 05:21:54 PM Oleksij Rempel wrote: i'm one of ath9k_htc devs. Currently i'm working on usb_suspend issue of this adapters. Looks like ar9271 and ar7010 have FUSB200, and i accidentally discovered that 9170 have it too. Are there any issue with usb-suspend + xhci controllers by you? Did you some how specially handled it? No, I haven't heard any complains about xhci + suspend. In fact, it's working fine with the NEC xhci I have. I also have a AR9271 and AR7010, so if you want I could try if they survive a suspend +resume cycle when attached. But, I do have a bug-report from someone else who has/had? problems with carl9170 and xhci. If you want, you can get the details from: carl9170 A-MPDU transmit problem: http://comments.gmane.org/gmane.linux.kernel.wireless.general/104597 The likely cause is related to Intel's xhci silicon (Ivy Bridge is affected, but I don't know about Haswell): http://permalink.gmane.org/gmane.linux.kernel.wireless.general/104602 Same situation is here - i have problem on Ivy Bridge. (Note: I don't have any Ivy Bridge system. Just Sandy Bridge and AMD's new A6-1450 Temash and both xhci work. So I can't do any proper comparisons like you.) Steps to reproduce: - plug adapter. Module and firmware will be loaded - make sure usb autosupend is enabled. By default it is not! Use powertop or directly sysfs to enable autosuspend for this device - rmmod and wait some seconds until adapter is suspended and then modprobe ath9k_htc first packet which is bigger as 64Byte will kill EP4 FIFO. Size register will report wrong size.. bigger as FIFO can handle. And only first ~40 readet bytes will be actually OK.. all the rest of packet will be trashed. This is what happens with a WN721 (ar9271): [17619.597905] usbcore: deregistering interface driver ath9k_htc [17619.679549] usb 2-2: ath9k_htc: USB layer deinitialized [17619.679602] ath9k_htc: Driver unloaded suspend resume [17667.543024] usb 2-2: reset high-speed USB device number 3 using xhci_hcd [17667.572168] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with disabled ep 88031aa77600 [17667.572174] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with disabled ep 88031aa77640 [17667.572177] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with disabled ep 88031aa77680 [17667.572180] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with disabled ep 88031aa776c0 [17667.572183] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with disabled ep 88031aa77700 [17667.572185] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with disabled ep 88031aa77740 [17667.573826] usb 2-2: ath9k_htc: Firmware htc_9271.fw requested [17667.573873] usbcore: registered new interface driver ath9k_htc [17668.038200] usb 2-2: ath9k_htc: Transferred FW: htc_9271.fw, size: 51272 [17668.273249] ath9k_htc 2-2:1.0: ath9k_htc: HTC initialized with 33 credits The driver loads, but there's no wlanX interface, no phyX interface and the driver can't be unloaded due to Error: Module ath9k_htc is in use. So, it is exactly this bug. After firmware was loaded ath9k trying to set some registers. Since command buffer is trashed it will write some nonsense to registers and some times in wrong to wrong addresses. I have some patches to do crc check of commands, to easy detect corrupted ones. You reproduced it on NEC xhci? Is it possible to reproduce it in carl9170? Ok: That's dmesg of your scenario: [ 809.995966] usbcore: deregistering interface driver carl9170 suspend resume [ 826.365596] usb 2-2: reset high-speed USB device number 2 using xhci_hcd [ 826.431154] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with disabled ep 88045af2b900 [ 826.431159] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with disabled ep 88045af2b940 [ 826.431162] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with disabled ep 88045af2b980 [ 826.431164] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with disabled ep 88045af2b9c0 [ 826.432257] usbcore: registered new interface driver carl9170 [ 826.433717] usb 2-2: driver API: 1.9.8 2013-03-23 [1-1] ... [ 826.816110] usb 2-2: Atheros AR9170 is registered as 'phy3' carl9170 is able to recover and the stick is working after autosuspend cycle. How about carl9170 A-MPDU transmit problem? This was already isolated. The AMPDU transmit problem only happens with Ivy Bridge's xhci. Regards Christian -- 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
Re: [PATCH 2/6] USB: OHCI: make ohci-omap a separate driver
On Mon, 22 Jul 2013, Manjunath Goudar wrote: Separate the TI OHCI OMAP1/2 host controller driver from ohci-hcd host code so that it can be built as a separate driver module. This work is part of enabling multi-platform kernels on ARM; it would be nice to have in 3.11. Signed-off-by: Manjunath Goudar manjunath.gou...@linaro.org Cc: Felipe Balbi ba...@ti.com Cc: Arnd Bergmann a...@arndb.de Cc: Greg KH g...@kroah.com Cc: Alan Stern st...@rowland.harvard.edu Cc: linux-usb@vger.kernel.org V2: -omap_ohci_clock_power(0) called in usb_hcd_omap_remove(). -Removed ohci_setup() call from usb_hcd_omap_probe(). -host_enabled and host_initialized variables aren't used for anything thats what removed. V3: -rewritten if (config-otg || config-rwc) block statements into two separate 'if blocks' to handle below scenarios 1. config-otg set scenario. 2. if any of these (config-otg, config-rwc) are set, this scenario should be handled only after ohci_setup() V4: -usb_remove_hcd() function is required a valid clock that is what omap_ohci_clock_power(0) is called after hcd shutdown. @@ -369,11 +367,6 @@ static int usb_hcd_omap_probe (const struct hc_driver *driver, if (retval) goto err3; - host_initialized = 1; - - if (!host_enabled) - omap_ohci_clock_power(0); - return 0; err3: iounmap(hcd-regs); I suspect there's a mistake here, and the omap_ohci_clock_power() call perhaps should be moved after the err3: label. But that mistake (if it is a mistake) was present in the original code, and this patch shouldn't change it. Acked-by: Alan Stern st...@rowland.harvard.edu -- 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/6] USB: OHCI: make ohci-omap3 a separate driver
On Mon, 22 Jul 2013, Manjunath Goudar wrote:
Separate the TI OHCI OMAP3 host controller driver from ohci-hcd
host code so that it can be built as a separate driver module.
This work is part of enabling multi-platform kernels on ARM;
it would be nice to have in 3.11.
Signed-off-by: Manjunath Goudar manjunath.gou...@linaro.org
Cc: Anand Gadiyar gadi...@ti.com
Cc: Felipe Balbi ba...@ti.com
Cc: Arnd Bergmann a...@arndb.de
Cc: Greg KH g...@kroah.com
Cc: Alan Stern st...@rowland.harvard.edu
Cc: linux-usb@vger.kernel.org
V2:
-ohci_setup() removed because it is called in .reset member
of the ohci_hc_driver structure.
-The improper multi-line commenting style written in proper way.
('*' characters aligned in vertically).
V3:
-RemoteWakeupConnected setting has been removed.
V4:
-V3 modification revert back, only ohci-regs setting write()
function has been removed because ohci-regs doesn't get set until
usb_add_hcd.
Acked-by: Alan Stern st...@rowland.harvard.edu
--
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 5/6] USB: OHCI: make ohci-at91 a separate driver
On Mon, 22 Jul 2013, Manjunath Goudar wrote:
Separate the TI OHCI Atmel host controller driver from ohci-hcd
host code so that it can be built as a separate driver module.
This work is part of enabling multi-platform kernels on ARM;
it would be nice to have in 3.12.
Signed-off-by: Manjunath Goudar manjunath.gou...@linaro.org
Cc: Arnd Bergmann a...@arndb.de
Cc: Alan Stern st...@rowland.harvard.edu
Cc: Greg KH g...@kroah.com
Cc: linux-usb@vger.kernel.org
V2:
-Set non-standard fields in ohci_at91_hc_driver manually, rather than
relying on an expanded struct ohci_driver_overrides.
-Save orig_ohci_hub_control and orig_ohci_hub_status_data rather than
relying on ohci_hub_control and hub_status_data being exported.
-ohci_setup() has been removed because it is called in .reset member
of the ohci_hc_driver structure.
V3:
-The ohci_restart() function is not required in current scenario,
only discarding connection state of integrated transceivers is sufficient,
for this directly handling ohci-hc_control.
V4:
- Removed extra space after tristate.
- Removed extra space between function name and '(' characters.
- MODULE_ALIAS line moved to last statement of ohci-at91 file.
Acked-by: Alan Stern st...@rowland.harvard.edu
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V2] USB: EHCI: make ehci-w90X900 a separate driver
On Mon, 22 Jul 2013, Manjunath Goudar wrote: Separate the W90X900(W90P910) on-chip host controller driver from ehci-hcd host code so that it can be built as a separate driver module. This work is part of enabling multi-platform kernels on ARM; however, note that other changes are still needed before W90X900(W90P910) can be booted with a multi-platform kernel and an ehci driver that only works on one of them. With the infrastructure added by Alan Stern in patch 3e0232039 USB: EHCI: prepare to make ehci-hcd a library module, we can avoid this problem by turning a bus glue into a separate module, as we do here for the w90X900 bus glue. Signed-off-by: Manjunath Goudar manjunath.gou...@linaro.org Acked-by: Arnd Bergmann a...@arndb.de Cc: Greg KH g...@kroah.com Cc: Alan Stern st...@rowland.harvard.edu Cc: Wan ZongShun mcuos@gmail.com Cc: linux-usb@vger.kernel.org Cc: linux-ker...@vger.kernel.org V2: -Arranged #include's in alphabetical order. -Replaced w90p910 by w90x900 because it is supports all series of w90x900. +MODULE_DESCRIPTION(DRIVER_DESC); MODULE_AUTHOR(Wan ZongShun mcuos@gmail.com); -MODULE_DESCRIPTION(w90p910 usb ehci driver!); -MODULE_LICENSE(GPL); MODULE_ALIAS(platform:w90p910-ehci); +MODULE_LICENSE(GPL v2); Are you sure the license is supposed to be changed? Is that okay with Wan ZongShun? 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] usb: gadget: at91_udc: Check gpio lookup results
This resolves the following valid build warning: drivers/usb/gadget/at91_udc.c:1685:34: warning: 'flags' may be used uninitialized in this function [-Wmaybe-uninitialized] I switched from ? : to !! mostly to save from wrapping the lines while I was at it. Signed-off-by: Olof Johansson o...@lixom.net --- Felipe, this would be nice to see fixed for 3.11 but I'd argue that it's been here long enough to not really be needed for -stable. drivers/usb/gadget/at91_udc.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/at91_udc.c b/drivers/usb/gadget/at91_udc.c index 073b938..f3dbcd0 100644 --- a/drivers/usb/gadget/at91_udc.c +++ b/drivers/usb/gadget/at91_udc.c @@ -1682,12 +1682,20 @@ static void at91udc_of_init(struct at91_udc *udc, board-vbus_pin = of_get_named_gpio_flags(np, atmel,vbus-gpio, 0, flags); - board-vbus_active_low = (flags OF_GPIO_ACTIVE_LOW) ? 1 : 0; + if (board-vbus_pin 0) + pr_err(%s: Failed to get atmel,vbus-gpio property\n, + np-full_name); + else + board-vbus_active_low = !!(flags OF_GPIO_ACTIVE_LOW); board-pullup_pin = of_get_named_gpio_flags(np, atmel,pullup-gpio, 0, flags); - board-pullup_active_low = (flags OF_GPIO_ACTIVE_LOW) ? 1 : 0; + if (board-pullup_pin 0) + pr_err(%s: Failed to get atmel,pullup-gpio property\n, + np-full_name); + else + board-pullup_active_low = !!(flags OF_GPIO_ACTIVE_LOW); } static int at91udc_probe(struct platform_device *pdev) -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 14/16] usb: musb: dsps: add MUSB_DEVCTL_SESSION back after removal
Hi Sebastian, On Tue, Jul 23, 2013 at 12:31 PM, Sebastian Andrzej Siewior bige...@linutronix.de wrote: On 07/23/2013 07:28 PM, Bin Liu wrote: Hi, Hi, On Mon, Jul 22, 2013 at 1:10 PM, Sebastian Andrzej Siewior bige...@linutronix.de mailto:bige...@linutronix.de wrote: This patch adds the MUSB_DEVCTL_SESSION back after it has been removed. If it is missing then the host session is not recognized. This bit is added initially added in musb_start() and removed after the first device disconnect. AFAIK, after the device is disconnected, the OTG state machine will go back to B_IDLE/A_IDLE state. SESSION is not needed in this case. Okay. In OTG mode, when no device is plugged, the ID pin is floating, you can never hold the host session in this case, even set the SESSION bit. The SESSION bit will be cleared by the controller after 100ms. In my testing the bit remains set. How is the bit supposed to come back after I connect a host device? The bit remains even when no device is plugged and ID ping is float? what platform do you use to test it? 'a host device'? you meant a usb device? By the otg specs, the session will not automatically start. The user/app has to issue the command, either by SRP or HNP, or something else. In TI 3.2 kernel, there is workaround in otg_timer() to _toggle_ the SESSION bit to detect if ID pin is grounded, which means a USB device is connected. Regards, -Bin. Regards, -Bin. Sebastian -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: xhci: Mark two functions __maybe_unused
Resolves the following build warnings:
drivers/usb/host/xhci.c:332:13: warning: 'xhci_msix_sync_irqs' defined but not
used [-Wunused-function]
drivers/usb/host/xhci.c:3901:12: warning: 'xhci_change_max_exit_latency'
defined but not used [-Wunused-function]
These functions are not always used, and since they're marked static
they will produce build warnings:
- xhci_msix_sync_irqs is only used with CONFIG_PCI.
- xhci_change_max_exit_latency is a little more complicated with
dependencies on CONFIG_PM and CONFIG_PM_RUNTIME.
Instead of building a bigger maze of ifdefs in this code, I've just
marked both with __maybe_unused.
Signed-off-by: Olof Johansson o...@lixom.net
---
Sarah,
I guess taste might differ on ifdef vs __maybe_unused, let me know this
is not to your liking.
Thanks,
-Olof
drivers/usb/host/xhci.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 2c49f00..87ae8cd 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -329,7 +329,7 @@ static void xhci_cleanup_msix(struct xhci_hcd *xhci)
return;
}
-static void xhci_msix_sync_irqs(struct xhci_hcd *xhci)
+static void __maybe_unused xhci_msix_sync_irqs(struct xhci_hcd *xhci)
{
int i;
@@ -3898,7 +3898,7 @@ int xhci_find_raw_port_number(struct usb_hcd *hcd, int
port1)
* Issue an Evaluate Context command to change the Maximum Exit Latency in the
* slot context. If that succeeds, store the new MEL in the xhci_virt_device.
*/
-static int xhci_change_max_exit_latency(struct xhci_hcd *xhci,
+static int __maybe_unused xhci_change_max_exit_latency(struct xhci_hcd *xhci,
struct usb_device *udev, u16 max_exit_latency)
{
struct xhci_virt_device *virt_dev;
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/16] usb: musb: dsps: rename ti81xx_driver_data to am33xx_driver_data
(re-send, due to last delivery failure to the mailing list...) On Tue, Jul 23, 2013 at 1:23 PM, Bin Liu binml...@gmail.com wrote: Hi Sebastian, On Mon, Jul 22, 2013 at 1:09 PM, Sebastian Andrzej Siewior bige...@linutronix.de wrote: This patch renames the type struct from ti81xx_driver_data to am33xx_driver_data since it is not used for ti81xx anymore. The EOI member is also removed since the am33xx SoC does not have such register. The interrupt is acknowledged by writting into the stat register. I guess the EOI register is removed from the TRM because AM33xx does not use it, there is no need to write to it to acknowledge. It does not hurt to write to it though since the register still exists, it just does nothing, I guess. But I am not sure if it is a good idea to remove eoi from the musb_dsps driver. If the intension is to merge the support for other SoC, such as AM35xx, AM18xx, then EOI handling might be still needed. I just don't know how those devices use EOI. Regards, -Bin. -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, Jul 23, 2013 at 11:01:10AM -0700, Greg KH wrote: On Tue, Jul 23, 2013 at 06:44:56PM +0100, Mark Brown wrote: What are the problems you are seeing with doing things with lookups? You don't know the id of the device you are looking up, due to multiple devices being in the system (dynamic ids, look back earlier in this thread for details about that.) I got copied in very late so don't have most of the thread I'm afraid, I did try looking at web archives but didn't see a clear problem statement. In any case this is why the APIs doing lookups do the lookups in the context of the requesting device - devices ask for whatever name they use locally. Having to write platform data for everything gets old fast and the code duplication is pretty tedious... Adding a single pointer is tedious? Where is the name that you are going to lookup going to come from? That code doesn't write itself... It's adding platform data in the first place that gets tedious - and of course there's also DT and ACPI to worry about, it's not just a case of platform data and then you're done. Pushing the lookup into library code means that drivers don't have to worry about any of this stuff. For most of the APIs doing this there is a clear and unambiguous name in the hardware that can be used (and for hardware process reasons is unlikely to get changed). The major exception to this is the clock API since it is relatively rare to have clear, segregated IP level information for IPs baked into larger chips. The other APIs tend to be establishing chip to chip links. signature.asc Description: Digital signature
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, 23 Jul 2013, Tomasz Figa wrote:
IMHO it would be better if you provided some code example, but let's try to
check if I understood you correctly.
8
[Board file]
static struct phy my_phy;
static struct platform_device phy_pdev = {
/* ... */
.platform_data = my_phy;
/* ... */
};
static struct platform_device phy_pdev = {
This should be controller_pdev, not phy_pdev, yes?
/* ... */
.platform_data = my_phy;
/* ... */
};
[Provider driver]
struct phy *phy = pdev-dev.platform_data;
ret = phy_create(phy);
[Consumer driver]
struct phy *phy = pdev-dev.platform_data;
ret = phy_get(pdev-dev, phy);
Or even just phy_get(pdev-dev), because phy_get() could be smart
enough to to set phy = dev-platform_data.
8
Is this what you mean?
That's what I was going to suggest too. The struct phy is defined in
the board file, which already knows about all the PHYs that exist in
the system. (Or perhaps it is allocated dynamically, so that when many
board files are present in the same kernel, only the entries listed in
the board file for the current system get created.) Then the
structure's address is stored in the platform data and made available
to both the provider and the consumer.
Even though the struct phy is defined (or allocated) in the board file,
its contents don't get filled in until the PHY driver provides the
details.
It's technically correct, but quality of this solution isn't really nice,
because it's a layering violation (at least if I understood what you mean).
This is because you need to have full definition of struct phy in board file
and a structure that is used as private data in PHY core comes from
platform code.
You don't have to have a full definition in the board file. Just a
partial definition -- most of the contents can be filled in later, when
the PHY driver is ready to store the private data.
It's not a layering violation for one region of the kernel to store
private data in a structure defined by another part of the kernel.
This happens all the time (e.g., dev_set_drvdata).
Alan Stern
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, Jul 23, 2013 at 08:31:05PM +0100, Mark Brown wrote: You don't know the id of the device you are looking up, due to multiple devices being in the system (dynamic ids, look back earlier in this thread for details about that.) I got copied in very late so don't have most of the thread I'm afraid, I did try looking at web archives but didn't see a clear problem statement. In any case this is why the APIs doing lookups do the lookups in the context of the requesting device - devices ask for whatever name they use locally. What do you mean by locally? The problem with the api was that the phy core wanted a id and a name to create a phy, and then later other code was doing a lookup based on the name and id (mushed together), because it knew that this device was the one it wanted. Just like the clock api, which, for multiple devices, has proven to cause problems. I don't want to see us accept an api that we know has issues in it now, I'd rather us fix it up properly. Subsystems should be able to create ids how ever they want to, and not rely on the code calling them to specify the names of the devices that way, otherwise the api is just too fragile. I think, that if you create a device, then just carry around the pointer to that device (in this case a phy) and pass it to whatever other code needs it. No need to do lookups on known names or anything else, just normal pointers, with no problems for multiple devices, busses, or naming issues. Having to write platform data for everything gets old fast and the code duplication is pretty tedious... Adding a single pointer is tedious? Where is the name that you are going to lookup going to come from? That code doesn't write itself... It's adding platform data in the first place that gets tedious - and of course there's also DT and ACPI to worry about, it's not just a case of platform data and then you're done. Pushing the lookup into library code means that drivers don't have to worry about any of this stuff. I agree, so just pass around the pointer to the phy and all is good. No need to worry about DT or ACPI or anything else. For most of the APIs doing this there is a clear and unambiguous name in the hardware that can be used (and for hardware process reasons is unlikely to get changed). The major exception to this is the clock API since it is relatively rare to have clear, segregated IP level information for IPs baked into larger chips. The other APIs tend to be establishing chip to chip links. The clock api is having problems with multiple names due to dynamic devices from what I was told. I want to prevent the PHY interface from having that same issue. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tuesday 23 of July 2013 12:44:23 Greg KH wrote:
On Tue, Jul 23, 2013 at 08:31:05PM +0100, Mark Brown wrote:
You don't know the id of the device you are looking up, due to
multiple devices being in the system (dynamic ids, look back earlier
in
this thread for details about that.)
I got copied in very late so don't have most of the thread I'm afraid,
I did try looking at web archives but didn't see a clear problem
statement. In any case this is why the APIs doing lookups do the
lookups in the context of the requesting device - devices ask for
whatever name they use locally.
What do you mean by locally?
The problem with the api was that the phy core wanted a id and a name to
create a phy, and then later other code was doing a lookup based on
the name and id (mushed together), because it knew that this device
was the one it wanted.
Just like the clock api, which, for multiple devices, has proven to
cause problems. I don't want to see us accept an api that we know has
issues in it now, I'd rather us fix it up properly.
Subsystems should be able to create ids how ever they want to, and not
rely on the code calling them to specify the names of the devices that
way, otherwise the api is just too fragile.
I think, that if you create a device, then just carry around the pointer
to that device (in this case a phy) and pass it to whatever other code
needs it. No need to do lookups on known names or anything else,
just normal pointers, with no problems for multiple devices, busses, or
naming issues.
PHY object is not a device, it is something that a device driver creates
(one or more instances of) when it is being probed. You don't have a clean
way to export this PHY object to other driver, other than keeping this PHY
on a list inside PHY core with some well-known ID (e.g. device name +
consumer port name/index, like in regulator core) and then to use this
well-known ID inside consumer driver as a lookup key passed to phy_get();
Actually I think for PHY case, exactly the same way as used for regulators
might be completely fine:
1. Each PHY would have some kind of platform, non-unique name, that is
just used to print some messages (like the platform/board name of a
regulator).
2. Each PHY would have an array of consumers. Consumer specifier would
consist of consumer device name and consumer port name - just like in
regulator subsystem.
3. PHY driver receives an array of, let's say, phy_init_data inside its
platform data that it would use to register its PHYs.
4. Consumer drivers would have constant consumer port names and wouldn't
receive any information about PHYs from platform code.
Code example:
[Board file]
static const struct phy_consumer_data usb_20_phy0_consumers[] = {
{
.devname = foo-ehci,
.port = usbphy,
},
};
static const struct phy_consumer_data usb_20_phy1_consumers[] = {
{
.devname = foo-otg,
.port = otgphy,
},
};
static const struct phy_init_data my_phys[] = {
{
.name = USB 2.0 PHY 0,
.consumers = usb_20_phy0_consumers,
.num_consumers = ARRAY_SIZE(usb_20_phy0_consumers),
},
{
.name = USB 2.0 PHY 1,
.consumers = usb_20_phy1_consumers,
.num_consumers = ARRAY_SIZE(usb_20_phy1_consumers),
},
{ }
};
static const struct platform_device usb_phy_pdev = {
.name = foo-usbphy,
.id = -1,
.dev = {
.platform_data = my_phys,
},
};
[PHY driver]
static int foo_usbphy_probe(pdev)
{
struct foo_usbphy *foo;
struct phy_init_data *init_data = pdev-dev.platform_data;
/* ... */
// for each PHY in init_data {
phy_register(foo-phy[i], init_data[i]);
// }
/* ... */
}
[EHCI driver]
static int foo_ehci_probe(pdev)
{
struct phy *phy;
/* ... */
phy = phy_get(pdev-dev, usbphy);
/* ... */
}
[OTG driver]
static int foo_otg_probe(pdev)
{
struct phy *phy;
/* ... */
phy = phy_get(pdev-dev, otgphy);
/* ... */
}
Having to write platform data for everything gets old fast and the
code
duplication is pretty tedious...
Adding a single pointer is tedious? Where is the name that you
are
going to lookup going to come from? That code doesn't write
itself...
It's adding platform data in the first place that gets tedious - and
of
course there's also DT and ACPI to worry about, it's not just a case
of
platform data and then you're done. Pushing the lookup into library
code means that drivers don't have to worry about any of this stuff.
I agree, so just pass around the pointer to the phy and all is good. No
need to worry about DT or ACPI or anything else.
With Device Tree we don't have board files anymore. How would
Re: Audio I/O parameters
On Fri, Jul 19, 2013 at 4:17 PM, Alan Stern st...@rowland.harvard.edu wrote: On Fri, 19 Jul 2013, James Stone wrote: The questions now are: Why are the same requests sent over and over again? Why does the ALSA driver attempt to set the clock frequency while the clock is actively in use? Has this behavior changed since the 3.5 kernel? Well, I think these requests may correspond to the lights flashing on and off on the front of the device. When starting the device in 3.5 at 256 frames/period (duplex), the lights flash on and off 2 times, in the current patched 3.8 version I have been using, the device lights flash on and off 4 times before starting jack with exactly the same settings - so it seems for some reason, the requests are going through multiple times on the 3.8 kernel but not on 3.5. I will send a 3.5 usbmon trace of a successful start off list (plus the same for 3.8?) if it would be useful. I don't know -- it's up to Clemens. Alan Stern Hi Alan, Just tried a few old kernels, and it seems that the bug I am experiencing was introduced at the start of 3.7 - kernel 3.6.11 is fine, and all the 3.7 series kernels are broken. So it seems it is not the updated ehci usb code that is directly responsible for the realtime audio problem. I have been trying the kernels from: http://kernel.ubuntu.com/~kernel-ppa/mainline/. Any suggestions on how to further zoom in on the culprit commit? James -- 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: Audio I/O parameters
On Tue, 23 Jul 2013, James Stone wrote: Hi Alan, Just tried a few old kernels, and it seems that the bug I am experiencing was introduced at the start of 3.7 - kernel 3.6.11 is Note that 3.6.11 came out _after_ 3.7, not before. fine, and all the 3.7 series kernels are broken. So it seems it is not the updated ehci usb code that is directly responsible for the realtime audio problem. I have been trying the kernels from: http://kernel.ubuntu.com/~kernel-ppa/mainline/. Any suggestions on how to further zoom in on the culprit commit? The usual approach is git bisection. You can find plenty of references for how to do it on Google, but it is time consuming and needs plenty of disk space as well as a reasonably fast machine. 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] usb: gadget: mv_u3d_core: fix violation of locking discipline in mv_u3d_ep_disable()
mv_u3d_nuke() expects to be calles with ep-u3d-lock held,
because mv_u3d_done() does. But mv_u3d_ep_disable() calls it
without lock that can lead to unpleasant consequences.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Alexey Khoroshilov khoroshi...@ispras.ru
---
drivers/usb/gadget/mv_u3d_core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/gadget/mv_u3d_core.c b/drivers/usb/gadget/mv_u3d_core.c
index 07fdb3e..650847d 100644
--- a/drivers/usb/gadget/mv_u3d_core.c
+++ b/drivers/usb/gadget/mv_u3d_core.c
@@ -645,6 +645,7 @@ static int mv_u3d_ep_disable(struct usb_ep *_ep)
struct mv_u3d_ep *ep;
struct mv_u3d_ep_context *ep_context;
u32 epxcr, direction;
+ unsigned long flags;
if (!_ep)
return -EINVAL;
@@ -661,7 +662,9 @@ static int mv_u3d_ep_disable(struct usb_ep *_ep)
direction = mv_u3d_ep_dir(ep);
/* nuke all pending requests (does flush) */
+ spin_lock_irqsave(u3d-lock, flags);
mv_u3d_nuke(ep, -ESHUTDOWN);
+ spin_unlock_irqrestore(u3d-lock, flags);
/* Disable the endpoint for Rx or Tx and reset the endpoint type */
if (direction == MV_U3D_EP_DIR_OUT) {
--
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 01/15] drivers: phy: add generic PHY framework
On Tuesday 23 of July 2013 15:36:00 Alan Stern wrote:
On Tue, 23 Jul 2013, Tomasz Figa wrote:
IMHO it would be better if you provided some code example, but let's
try to check if I understood you correctly.
8---
-
[Board file]
static struct phy my_phy;
static struct platform_device phy_pdev = {
/* ... */
.platform_data = my_phy;
/* ... */
};
static struct platform_device phy_pdev = {
This should be controller_pdev, not phy_pdev, yes?
Right. A copy-pasto.
/* ... */
.platform_data = my_phy;
/* ... */
};
[Provider driver]
struct phy *phy = pdev-dev.platform_data;
ret = phy_create(phy);
[Consumer driver]
struct phy *phy = pdev-dev.platform_data;
ret = phy_get(pdev-dev, phy);
Or even just phy_get(pdev-dev), because phy_get() could be smart
enough to to set phy = dev-platform_data.
Unless you need more than one PHY in this driver...
--
--8
Is this what you mean?
That's what I was going to suggest too. The struct phy is defined in
the board file, which already knows about all the PHYs that exist in
the system. (Or perhaps it is allocated dynamically, so that when many
board files are present in the same kernel, only the entries listed in
the board file for the current system get created.)
Well, such dynamic allocation is a must. We don't accept non-multiplatform
aware code anymore, not even saying about multiboard.
Then the
structure's address is stored in the platform data and made available
to both the provider and the consumer.
Yes, technically this can work. You would still have to perform some kind
of synchronization to make sure that the PHY bound to this structure is
actually present. This is again technically doable (e.g. a list of
registered struct phys inside PHY core).
Even though the struct phy is defined (or allocated) in the board file,
its contents don't get filled in until the PHY driver provides the
details.
You can't assure this. Board file is free to do whatever it wants with
this struct. A clean solution would prevent this.
It's technically correct, but quality of this solution isn't really
nice, because it's a layering violation (at least if I understood
what you mean). This is because you need to have full definition of
struct phy in board file and a structure that is used as private data
in PHY core comes from platform code.
You don't have to have a full definition in the board file. Just a
partial definition -- most of the contents can be filled in later, when
the PHY driver is ready to store the private data.
It's not a layering violation for one region of the kernel to store
private data in a structure defined by another part of the kernel.
This happens all the time (e.g., dev_set_drvdata).
Not really. The phy struct is something that _is_ private data of PHY
subsystem, not something that can store private data of PHY subsystem
(sure it can store private data of particular PHY driver, but that's
another story) and only PHY subsystem should have access to its contents.
By the way, we need to consider other cases here as well, for example it
would be nice to have a single phy_get() function that works for both non-
DT and DT cases to make the consumer driver not have to worry whether it's
being probed from DT or not.
I'd suggest simply reusing the lookup method of regulator framework, just
as I suggested here:
http://thread.gmane.org/gmane.linux.ports.arm.kernel/252813/focus=101661
Best regards,
Tomasz
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tuesday 23 of July 2013 11:04:14 Greg KH wrote:
On Tue, Jul 23, 2013 at 07:48:11PM +0200, Tomasz Figa wrote:
On Tuesday 23 of July 2013 10:37:11 Greg KH wrote:
On Tue, Jul 23, 2013 at 06:50:29PM +0200, Tomasz Figa wrote:
Ick, no. Why can't you just pass the pointer to the phy itself?
If
you
had a priv pointer to search from, then you could have just
passed
the
original phy pointer in the first place, right?
IMHO it would be better if you provided some code example, but
let's
try to check if I understood you correctly.
It's not my code that I want to have added, so I don't have to write
examples, I just get to complain about the existing stuff :)
Still, I think that some small code snippets illustrating the idea are
really helpful.
8---
-
[Board file]
static struct phy my_phy;
static struct platform_device phy_pdev = {
/* ... */
.platform_data = my_phy;
/* ... */
};
static struct platform_device phy_pdev = {
/* ... */
.platform_data = my_phy;
/* ... */
};
[Provider driver]
struct phy *phy = pdev-dev.platform_data;
ret = phy_create(phy);
[Consumer driver]
struct phy *phy = pdev-dev.platform_data;
ret = phy_get(pdev-dev, phy);
--
-
-8
Is this what you mean?
No. Well, kind of. What's wrong with using the platform data
structure unique to the board to have the pointer?
For example (just randomly picking one), the ata-pxa driver would
change include/linux/platform_data/ata-pxa.h to have a phy pointer
in it:
struct phy;
struct pata_pxa_pdata {
/* PXA DMA DREQ0:2 pin */
uint32_tdma_dreq;
/* Register shift */
uint32_treg_shift;
/* IRQ flags */
uint32_tirq_flags;
/* PHY */
struct phy *phy;
};
Then, when you create the platform, set the phy* pointer with a call
to
phy_create(). Then you can use that pointer wherever that plaform
data
is available (i.e. whereever platform_data is at).
Hmm? So, do you suggest to call phy_create() from board file? What
phy_ops struct and other hardware parameters would it take?
The issue is that a string name is not going to scale at all,
as it
requires hard-coded information that will change over time (as
the
existing clock interface is already showing.)
I fully agree that a simple, single string will not scale even in
some,
not so uncommon cases, but there is already a lot of existing
lookup
solutions over the kernel and so there is no point in introducing
another one.
I'm trying to get _rid_ of lookup solutions and just use a real
pointer, as you should. I'll go tackle those other ones after this
one
is taken care of, to show how the others should be handled as well.
There was a reason for introducing lookup solutions. The reason was
that in board file there is no way to get a pointer to something that
is going to be created much later in time. We don't do time travel
;-).
Please just pass the real phy pointer around, that's what it
is
there
for. Your board binding logic/code should be able to handle
this,
as
it somehow was going to do the same thing with a name.
It's technically correct, but quality of this solution isn't
really
nice, because it's a layering violation (at least if I understood
what
you mean). This is because you need to have full definition of
struct
phy in board file and a structure that is used as private data in
PHY
core comes from platform code.
No, just a pointer, you don't need the full structure until you
get to some .c code that actually manipulates the phy itself, for
all other places, you are just dealing with a pointer and a
structure you never reference.
Does that make more sense?
Well, to the point that I think I now understood your suggestion.
Unfortunately the suggestion alone isn't really something that can be
done, considering how driver core and generic frameworks work.
Ok, given that I seem to be totally confused as to exactly how the
board-specific frameworks work, I'll take your word for it.
Well, they are working in a way that keeps separation of layers, making
things clean. Platform code should not (well, there might exist some in
tree hacks, but this should not be propagated) used to exchange data
between drivers, but rather to specify board specific parameters for
generic drivers. If drivers need to cooperate, there must be a dedicated
interface for
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, Jul 23, 2013 at 10:07:52PM +0200, Tomasz Figa wrote:
On Tuesday 23 of July 2013 12:44:23 Greg KH wrote:
On Tue, Jul 23, 2013 at 08:31:05PM +0100, Mark Brown wrote:
You don't know the id of the device you are looking up, due to
multiple devices being in the system (dynamic ids, look back earlier
in
this thread for details about that.)
I got copied in very late so don't have most of the thread I'm afraid,
I did try looking at web archives but didn't see a clear problem
statement. In any case this is why the APIs doing lookups do the
lookups in the context of the requesting device - devices ask for
whatever name they use locally.
What do you mean by locally?
The problem with the api was that the phy core wanted a id and a name to
create a phy, and then later other code was doing a lookup based on
the name and id (mushed together), because it knew that this device
was the one it wanted.
Just like the clock api, which, for multiple devices, has proven to
cause problems. I don't want to see us accept an api that we know has
issues in it now, I'd rather us fix it up properly.
Subsystems should be able to create ids how ever they want to, and not
rely on the code calling them to specify the names of the devices that
way, otherwise the api is just too fragile.
I think, that if you create a device, then just carry around the pointer
to that device (in this case a phy) and pass it to whatever other code
needs it. No need to do lookups on known names or anything else,
just normal pointers, with no problems for multiple devices, busses, or
naming issues.
PHY object is not a device, it is something that a device driver creates
(one or more instances of) when it is being probed.
But you created a 'struct device' for it, so I think of it as a device
be it virtual or real :)
You don't have a clean way to export this PHY object to other driver,
other than keeping this PHY on a list inside PHY core with some
well-known ID (e.g. device name + consumer port name/index, like in
regulator core) and then to use this well-known ID inside consumer
driver as a lookup key passed to phy_get();
Actually I think for PHY case, exactly the same way as used for
regulators might be completely fine:
1. Each PHY would have some kind of platform, non-unique name, that is
just used to print some messages (like the platform/board name of a
regulator).
2. Each PHY would have an array of consumers. Consumer specifier would
consist of consumer device name and consumer port name - just like in
regulator subsystem.
3. PHY driver receives an array of, let's say, phy_init_data inside its
platform data that it would use to register its PHYs.
4. Consumer drivers would have constant consumer port names and wouldn't
receive any information about PHYs from platform code.
Code example:
[Board file]
static const struct phy_consumer_data usb_20_phy0_consumers[] = {
{
.devname = foo-ehci,
.port = usbphy,
},
};
static const struct phy_consumer_data usb_20_phy1_consumers[] = {
{
.devname = foo-otg,
.port = otgphy,
},
};
static const struct phy_init_data my_phys[] = {
{
.name = USB 2.0 PHY 0,
.consumers = usb_20_phy0_consumers,
.num_consumers = ARRAY_SIZE(usb_20_phy0_consumers),
},
{
.name = USB 2.0 PHY 1,
.consumers = usb_20_phy1_consumers,
.num_consumers = ARRAY_SIZE(usb_20_phy1_consumers),
},
{ }
};
static const struct platform_device usb_phy_pdev = {
.name = foo-usbphy,
.id = -1,
.dev = {
.platform_data = my_phys,
},
};
[PHY driver]
static int foo_usbphy_probe(pdev)
{
struct foo_usbphy *foo;
struct phy_init_data *init_data = pdev-dev.platform_data;
/* ... */
// for each PHY in init_data {
phy_register(foo-phy[i], init_data[i]);
// }
/* ... */
}
[EHCI driver]
static int foo_ehci_probe(pdev)
{
struct phy *phy;
/* ... */
phy = phy_get(pdev-dev, usbphy);
/* ... */
}
[OTG driver]
static int foo_otg_probe(pdev)
{
struct phy *phy;
/* ... */
phy = phy_get(pdev-dev, otgphy);
/* ... */
}
That's not so bad, as long as you let the phy core use whatever name it
wants for the device when it registers it with sysfs. Use the name you
are requesting as a tag or some such hint as to what the phy can be
looked up by.
Good luck handling duplicate tags :)
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 01/15] drivers: phy: add generic PHY framework
On Tue, 23 Jul 2013, Tomasz Figa wrote: That's what I was going to suggest too. The struct phy is defined in the board file, which already knows about all the PHYs that exist in the system. (Or perhaps it is allocated dynamically, so that when many board files are present in the same kernel, only the entries listed in the board file for the current system get created.) Well, such dynamic allocation is a must. We don't accept non-multiplatform aware code anymore, not even saying about multiboard. Then the structure's address is stored in the platform data and made available to both the provider and the consumer. Yes, technically this can work. You would still have to perform some kind of synchronization to make sure that the PHY bound to this structure is actually present. This is again technically doable (e.g. a list of registered struct phys inside PHY core). The synchronization takes place inside phy_get. If phy_create hasn't been called for this structure by the time phy_get runs, phy_get will return an error. Even though the struct phy is defined (or allocated) in the board file, its contents don't get filled in until the PHY driver provides the details. You can't assure this. Board file is free to do whatever it wants with this struct. A clean solution would prevent this. I'm not sure what you mean here. Of course I can't prevent a board file from messing up a data structure. I can't prevent it from causing memory access violations either; in fact, I can't prevent any bugs in other people's code. Besides, why do you say the board file is free to do whatever it wants with the struct phy? Currently the struct phy is created by the PHY provider and the PHY core, right? It's not even mentioned in the board file. It's technically correct, but quality of this solution isn't really nice, because it's a layering violation (at least if I understood what you mean). This is because you need to have full definition of struct phy in board file and a structure that is used as private data in PHY core comes from platform code. You don't have to have a full definition in the board file. Just a partial definition -- most of the contents can be filled in later, when the PHY driver is ready to store the private data. It's not a layering violation for one region of the kernel to store private data in a structure defined by another part of the kernel. This happens all the time (e.g., dev_set_drvdata). Not really. The phy struct is something that _is_ private data of PHY subsystem, not something that can store private data of PHY subsystem (sure it can store private data of particular PHY driver, but that's another story) and only PHY subsystem should have access to its contents. If you want to keep the phy struct completely separate from the board file, there's an easy way to do it. Let's say the board file knows about N different PHYs in the system. Then you define an array of N pointers to phys: struct phy *(phy_address[N]); In the platform data for both PHY j and its controller, store phy_address[j]. The PHY provider passes this cookie to phy_create: cookie = pdev-dev.platform_data; ret = phy_create(phy, cookie); and phy_create simply stores: *cookie = phy. The PHY consumer does much the same the same thing: cookie = pdev-dev.platform_data; phy = phy_get(cookie); phy_get returns *cookie if it isn't NULL, or an ERR_PTR otherwise. By the way, we need to consider other cases here as well, for example it would be nice to have a single phy_get() function that works for both non- DT and DT cases to make the consumer driver not have to worry whether it's being probed from DT or not. You ought to be able to adapt this scheme to work with DT. Maybe by having multiple phy_address arrays. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tuesday 23 of July 2013 16:53:55 Alan Stern wrote: On Tue, 23 Jul 2013, Tomasz Figa wrote: That's what I was going to suggest too. The struct phy is defined in the board file, which already knows about all the PHYs that exist in the system. (Or perhaps it is allocated dynamically, so that when many board files are present in the same kernel, only the entries listed in the board file for the current system get created.) Well, such dynamic allocation is a must. We don't accept non-multiplatform aware code anymore, not even saying about multiboard. Then the structure's address is stored in the platform data and made available to both the provider and the consumer. Yes, technically this can work. You would still have to perform some kind of synchronization to make sure that the PHY bound to this structure is actually present. This is again technically doable (e.g. a list of registered struct phys inside PHY core). The synchronization takes place inside phy_get. If phy_create hasn't been called for this structure by the time phy_get runs, phy_get will return an error. Yes, this is the solution that I had in mind when saying that this is doable. Even though the struct phy is defined (or allocated) in the board file, its contents don't get filled in until the PHY driver provides the details. You can't assure this. Board file is free to do whatever it wants with this struct. A clean solution would prevent this. I'm not sure what you mean here. Of course I can't prevent a board file from messing up a data structure. I can't prevent it from causing memory access violations either; in fact, I can't prevent any bugs in other people's code. Besides, why do you say the board file is free to do whatever it wants with the struct phy? Currently the struct phy is created by the PHY provider and the PHY core, right? It's not even mentioned in the board file. I mean, if you have a struct type of which full declaration is available for some code, this code can access any memeber of it without any hacks, which is not something that we want to have in board files. The phy struct should be opaque for them. It's technically correct, but quality of this solution isn't really nice, because it's a layering violation (at least if I understood what you mean). This is because you need to have full definition of struct phy in board file and a structure that is used as private data in PHY core comes from platform code. You don't have to have a full definition in the board file. Just a partial definition -- most of the contents can be filled in later, when the PHY driver is ready to store the private data. It's not a layering violation for one region of the kernel to store private data in a structure defined by another part of the kernel. This happens all the time (e.g., dev_set_drvdata). Not really. The phy struct is something that _is_ private data of PHY subsystem, not something that can store private data of PHY subsystem (sure it can store private data of particular PHY driver, but that's another story) and only PHY subsystem should have access to its contents. If you want to keep the phy struct completely separate from the board file, there's an easy way to do it. Let's say the board file knows about N different PHYs in the system. Then you define an array of N pointers to phys: struct phy *(phy_address[N]); In the platform data for both PHY j and its controller, store phy_address[j]. The PHY provider passes this cookie to phy_create: cookie = pdev-dev.platform_data; ret = phy_create(phy, cookie); and phy_create simply stores: *cookie = phy. The PHY consumer does much the same the same thing: cookie = pdev-dev.platform_data; phy = phy_get(cookie); phy_get returns *cookie if it isn't NULL, or an ERR_PTR otherwise. OK, this can work. Again, just technically, because it's rather ugly. By the way, we need to consider other cases here as well, for example it would be nice to have a single phy_get() function that works for both non- DT and DT cases to make the consumer driver not have to worry whether it's being probed from DT or not. You ought to be able to adapt this scheme to work with DT. Maybe by having multiple phy_address arrays. Where would you want to have those phy_address arrays stored? There are no board files when booting with DT. Not even saying that you don't need to use any hacky schemes like this when you have DT that nicely specifies relations between devices. Anyway, board file should not be considered as a method to exchange data between drivers. It should be used only to pass data from it to drivers, not the other way. Ideally all data in a board file should be marked as const and __init and dropped after system initialization. Best regards, Tomasz -- To
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tuesday 23 of July 2013 13:50:07 Greg KH wrote:
On Tue, Jul 23, 2013 at 10:07:52PM +0200, Tomasz Figa wrote:
On Tuesday 23 of July 2013 12:44:23 Greg KH wrote:
On Tue, Jul 23, 2013 at 08:31:05PM +0100, Mark Brown wrote:
You don't know the id of the device you are looking up, due to
multiple devices being in the system (dynamic ids, look back
earlier
in
this thread for details about that.)
I got copied in very late so don't have most of the thread I'm
afraid,
I did try looking at web archives but didn't see a clear problem
statement. In any case this is why the APIs doing lookups do the
lookups in the context of the requesting device - devices ask for
whatever name they use locally.
What do you mean by locally?
The problem with the api was that the phy core wanted a id and a
name to create a phy, and then later other code was doing a
lookup based on the name and id (mushed together), because it
knew that this device was the one it wanted.
Just like the clock api, which, for multiple devices, has proven to
cause problems. I don't want to see us accept an api that we know
has
issues in it now, I'd rather us fix it up properly.
Subsystems should be able to create ids how ever they want to, and
not
rely on the code calling them to specify the names of the devices
that
way, otherwise the api is just too fragile.
I think, that if you create a device, then just carry around the
pointer to that device (in this case a phy) and pass it to whatever
other code needs it. No need to do lookups on known names or
anything else, just normal pointers, with no problems for multiple
devices, busses, or naming issues.
PHY object is not a device, it is something that a device driver
creates (one or more instances of) when it is being probed.
But you created a 'struct device' for it, so I think of it as a device
be it virtual or real :)
Keep in mind that those virtual devices are created by PHY driver bound to
a real device and one real device can have multiple virtual devices behind
it.
You don't have a clean way to export this PHY object to other driver,
other than keeping this PHY on a list inside PHY core with some
well-known ID (e.g. device name + consumer port name/index, like in
regulator core) and then to use this well-known ID inside consumer
driver as a lookup key passed to phy_get();
Actually I think for PHY case, exactly the same way as used for
regulators might be completely fine:
1. Each PHY would have some kind of platform, non-unique name, that is
just used to print some messages (like the platform/board name of a
regulator).
2. Each PHY would have an array of consumers. Consumer specifier would
consist of consumer device name and consumer port name - just like in
regulator subsystem.
3. PHY driver receives an array of, let's say, phy_init_data inside
its
platform data that it would use to register its PHYs.
4. Consumer drivers would have constant consumer port names and
wouldn't receive any information about PHYs from platform code.
Code example:
[Board file]
static const struct phy_consumer_data usb_20_phy0_consumers[] = {
{
.devname = foo-ehci,
.port = usbphy,
},
};
static const struct phy_consumer_data usb_20_phy1_consumers[] = {
{
.devname = foo-otg,
.port = otgphy,
},
};
static const struct phy_init_data my_phys[] = {
{
.name = USB 2.0 PHY 0,
.consumers = usb_20_phy0_consumers,
.num_consumers = ARRAY_SIZE(usb_20_phy0_consumers),
},
{
.name = USB 2.0 PHY 1,
.consumers = usb_20_phy1_consumers,
.num_consumers = ARRAY_SIZE(usb_20_phy1_consumers),
},
{ }
};
static const struct platform_device usb_phy_pdev = {
.name = foo-usbphy,
.id = -1,
.dev = {
.platform_data = my_phys,
},
};
[PHY driver]
static int foo_usbphy_probe(pdev)
{
struct foo_usbphy *foo;
struct phy_init_data *init_data = pdev-dev.platform_data;
/* ... */
// for each PHY in init_data {
phy_register(foo-phy[i], init_data[i]);
// }
/* ... */
}
[EHCI driver]
static int foo_ehci_probe(pdev)
{
struct phy *phy;
/* ... */
phy = phy_get(pdev-dev, usbphy);
/* ... */
}
[OTG driver]
static int foo_otg_probe(pdev)
{
struct phy *phy;
/* ... */
phy = phy_get(pdev-dev, otgphy);
/* ... */
}
That's not so bad, as long as you let the phy core use whatever name it
wants for the device when it registers it with sysfs.
Yes, in regulator core
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, 23 Jul 2013, Tomasz Figa wrote: If you want to keep the phy struct completely separate from the board file, there's an easy way to do it. Let's say the board file knows about N different PHYs in the system. Then you define an array of N pointers to phys: struct phy *(phy_address[N]); In the platform data for both PHY j and its controller, store phy_address[j]. The PHY provider passes this cookie to phy_create: cookie = pdev-dev.platform_data; ret = phy_create(phy, cookie); and phy_create simply stores: *cookie = phy. The PHY consumer does much the same the same thing: cookie = pdev-dev.platform_data; phy = phy_get(cookie); phy_get returns *cookie if it isn't NULL, or an ERR_PTR otherwise. OK, this can work. Again, just technically, because it's rather ugly. There's no reason the phy_address things have to be arrays. A separate individual pointer for each PHY would work just as well. Where would you want to have those phy_address arrays stored? There are no board files when booting with DT. Not even saying that you don't need to use any hacky schemes like this when you have DT that nicely specifies relations between devices. If everybody agrees DT has a nice scheme for specifying relations between devices, why not use that same scheme in the PHY core? Anyway, board file should not be considered as a method to exchange data between drivers. It should be used only to pass data from it to drivers, not the other way. Ideally all data in a board file should be marked as const and __init and dropped after system initialization. The phy_address things don't have to be defined or allocated in the board file; they could be set up along with the platform data. In any case, this was simply meant to be a suggestion to show that it is relatively easy to do what you need without using name or ID strings. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/15] drivers: phy: add generic PHY framework
On Tue, Jul 23, 2013 at 11:05:48PM +0200, Tomasz Figa wrote: That's not so bad, as long as you let the phy core use whatever name it wants for the device when it registers it with sysfs. Yes, in regulator core consumer names are completely separated from this. Regulator core simply assigns a sequential integer ID to each regulator and registers /sys/class/regulator/regulator.ID for each regulator. Yes, that's fine. Use the name you are requesting as a tag or some such hint as to what the phy can be looked up by. Good luck handling duplicate tags :) The tag alone is not a key. Lookup key consists of two components, consumer device name and consumer tag. What kind of duplicate tags can be a problem here? Ok, I didn't realize it looked at both parts, that makes sense, 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 v4] staging: usbip: replace pr_warning() with dev_warn().
On Thu, Jun 27, 2013 at 03:34:52PM +0530, navin patidar wrote:
dev_warn() is preferred over pr_warning().
container_of() is used to get usb_driver pointer from usbip_device container
(stub_device or vhci_device), to get device structure required for dev_warn().
Signed-off-by: navin patidar nav...@cdac.in
---
drivers/staging/usbip/usbip_event.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/usbip/usbip_event.c
b/drivers/staging/usbip/usbip_event.c
index 82123be..1f3a571 100644
--- a/drivers/staging/usbip/usbip_event.c
+++ b/drivers/staging/usbip/usbip_event.c
@@ -21,6 +21,8 @@
#include linux/export.h
#include usbip_common.h
+#include stub.h
+#include vhci.h
static int event_handler(struct usbip_device *ud)
{
@@ -85,7 +87,14 @@ int usbip_start_eh(struct usbip_device *ud)
ud-eh = kthread_run(event_handler_loop, ud, usbip_eh);
if (IS_ERR(ud-eh)) {
- pr_warning(Unable to start control thread\n);
+ struct device *dev;
+
+ if (ud-side == USBIP_STUB)
+ dev = container_of(ud, struct stub_device,
ud)-udev-dev;
+ else
+ dev = container_of(ud, struct vhci_device,
ud)-udev-dev;
Putting '' in front of container_of seems odd, are you sure it's needed
here? If ud is a pointer, everything else should just work properly
without that.
Can you please fix this up?
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 3/3] staging: dwc2: Don't touch the dma_mask when dma is disabled
On Fri, Jul 19, 2013 at 11:34:24AM +0200, Matthijs Kooijman wrote: There was some code that cleared the dma_mask when dma was disabled in the driver. Given that clearing the mask doesn't actually tell the usb core we're not using dma, and a previous commit explicitely sets the hcd-self.uses_dma value, it seems these values are unneeded and can only potentially cause problems (when reloading a module, for example). Signed-off-by: Matthijs Kooijman matth...@stdin.nl Acked-by: Paul Zimmerman paul.zimmer...@synopsys.com --- drivers/staging/dwc2/hcd.c | 3 --- 1 file changed, 3 deletions(-) This patch no longer applies, as I've caught up with Paul's pending patches. Please refresh it against the next linux-next release and resend 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] staging: dwc2: add microframe scheduler from downstream Pi kernel
On Wed, Jul 17, 2013 at 12:35:46PM -0700, Paul Zimmerman wrote: The transfer scheduler in the dwc2 driver is pretty basic, not to mention buggy. It works fairly well with just a couple of devices plugged in, but if you add, say, multiple devices with periodic endpoints, the scheduler breaks down and can't even enumerate all the devices. To fix this, import the microframe scheduler patch from the driver in the downstream Raspberry Pi kernel, which is based on the Synopsys vendor driver. That patch was done by the guys from raspberrypi.org, including at least Gordon Hollingsworth and popcornmix. I have added a driver parameter for this, enabled by default, in case anyone has problems with it and needs to disable it. I don't think we should add a DT binding for that, though, since I plan to remove the option once any bugs are fixed. Signed-off-by: Paul Zimmerman pa...@synopsys.com I'm guessing there will be a new version of this, so I'll drop this one from my queue. Please feel free to resend an updated version. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 4/5] Intel xhci: refactor EHCI/xHCI port switching
From: Mathias Nyman mathias.ny...@linux.intel.com
Make the Linux xHCI driver automatically try to switchover the EHCI ports to
xHCI when an Intel xHCI host is detected, and it also finds an Intel EHCI host.
This means we will no longer have to add Intel xHCI hosts to a quirks list when
the PCI device IDs change. Simply continuing to add new Intel xHCI PCI device
IDs to the quirks list is not sustainable.
During suspend ports may be swicthed back to EHCI by BIOS and not properly
restored to xHCI at resume. Previously both EHCI and xHCI resume functions
switched ports back to XHCI, but it's enough to do it in xHCI only
because the hub driver doesn't start running again until after both hosts are
resumed.
Signed-off-by: Mathias Nyman mathias.ny...@linux.intel.com
Signed-off-by: Sarah Sharp sarah.a.sh...@linux.intel.com
---
drivers/usb/host/ehci-pci.c | 42 ---
drivers/usb/host/pci-quirks.c | 48 +++-
drivers/usb/host/pci-quirks.h |3 +-
drivers/usb/host/xhci-pci.c | 14 ++-
4 files changed, 27 insertions(+), 80 deletions(-)
diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
index 595d210..6bd299e 100644
--- a/drivers/usb/host/ehci-pci.c
+++ b/drivers/usb/host/ehci-pci.c
@@ -315,53 +315,11 @@ done:
* Also they depend on separate root hub suspend/resume.
*/
-static bool usb_is_intel_switchable_ehci(struct pci_dev *pdev)
-{
- return pdev-class == PCI_CLASS_SERIAL_USB_EHCI
- pdev-vendor == PCI_VENDOR_ID_INTEL
- (pdev-device == 0x1E26 ||
-pdev-device == 0x8C2D ||
-pdev-device == 0x8C26 ||
-pdev-device == 0x9C26);
-}
-
-static void ehci_enable_xhci_companion(void)
-{
- struct pci_dev *companion = NULL;
-
- /* The xHCI and EHCI controllers are not on the same PCI slot */
- for_each_pci_dev(companion) {
- if (!usb_is_intel_switchable_xhci(companion))
- continue;
- usb_enable_xhci_ports(companion);
- return;
- }
-}
-
static int ehci_pci_resume(struct usb_hcd *hcd, bool hibernated)
{
struct ehci_hcd *ehci = hcd_to_ehci(hcd);
struct pci_dev *pdev = to_pci_dev(hcd-self.controller);
- /* The BIOS on systems with the Intel Panther Point chipset may or may
-* not support xHCI natively. That means that during system resume, it
-* may switch the ports back to EHCI so that users can use their
-* keyboard to select a kernel from GRUB after resume from hibernate.
-*
-* The BIOS is supposed to remember whether the OS had xHCI ports
-* enabled before resume, and switch the ports back to xHCI when the
-* BIOS/OS semaphore is written, but we all know we can't trust BIOS
-* writers.
-*
-* Unconditionally switch the ports back to xHCI after a system resume.
-* We can't tell whether the EHCI or xHCI controller will be resumed
-* first, so we have to do the port switchover in both drivers. Writing
-* a '1' to the port switchover registers should have no effect if the
-* port was already switched over.
-*/
- if (usb_is_intel_switchable_ehci(pdev))
- ehci_enable_xhci_companion();
-
if (ehci_resume(hcd, hibernated) != 0)
(void) ehci_pci_reinit(ehci, pdev);
return 0;
diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c
index b9848e4..2c76ef1 100644
--- a/drivers/usb/host/pci-quirks.c
+++ b/drivers/usb/host/pci-quirks.c
@@ -735,32 +735,6 @@ static int handshake(void __iomem *ptr, u32 mask, u32 done,
return -ETIMEDOUT;
}
-#define PCI_DEVICE_ID_INTEL_LYNX_POINT_XHCI0x8C31
-#define PCI_DEVICE_ID_INTEL_LYNX_POINT_LP_XHCI 0x9C31
-
-bool usb_is_intel_ppt_switchable_xhci(struct pci_dev *pdev)
-{
- return pdev-class == PCI_CLASS_SERIAL_USB_XHCI
- pdev-vendor == PCI_VENDOR_ID_INTEL
- pdev-device == PCI_DEVICE_ID_INTEL_PANTHERPOINT_XHCI;
-}
-
-/* The Intel Lynx Point chipset also has switchable ports. */
-bool usb_is_intel_lpt_switchable_xhci(struct pci_dev *pdev)
-{
- return pdev-class == PCI_CLASS_SERIAL_USB_XHCI
- pdev-vendor == PCI_VENDOR_ID_INTEL
- (pdev-device == PCI_DEVICE_ID_INTEL_LYNX_POINT_XHCI ||
-pdev-device == PCI_DEVICE_ID_INTEL_LYNX_POINT_LP_XHCI);
-}
-
-bool usb_is_intel_switchable_xhci(struct pci_dev *pdev)
-{
- return usb_is_intel_ppt_switchable_xhci(pdev) ||
- usb_is_intel_lpt_switchable_xhci(pdev);
-}
-EXPORT_SYMBOL_GPL(usb_is_intel_switchable_xhci);
-
/*
* Intel's Panther Point chipset has two host controllers (EHCI and xHCI) that
* share some number of ports. These ports can be switched between either
@@ -779,9 +753,23 @@
[PATCH 5/5] xhci: Correct misplaced newlines
From: Joe Perches j...@perches.com
Logging messages end in newlines, not have
them put in the middle of messages.
Signed-off-by: Joe Perches j...@perches.com
Signed-off-by: Sarah Sharp sarah.a.sh...@linux.intel.com
---
drivers/usb/host/xhci.c | 10 +-
1 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 2c49f00..87b5e65 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3075,8 +3075,8 @@ static u32 xhci_calculate_no_streams_bitmask(struct
xhci_hcd *xhci,
/* Are streams already being freed for the endpoint? */
if (ep_state EP_GETTING_NO_STREAMS) {
xhci_warn(xhci, WARN Can't disable streams for
- endpoint 0x%x\n,
- streams are being disabled already.,
+ endpoint 0x%x,
+ streams are being disabled already\n,
eps[i]-desc.bEndpointAddress);
return 0;
}
@@ -3084,8 +3084,8 @@ static u32 xhci_calculate_no_streams_bitmask(struct
xhci_hcd *xhci,
if (!(ep_state EP_HAS_STREAMS)
!(ep_state EP_GETTING_STREAMS)) {
xhci_warn(xhci, WARN Can't disable streams for
- endpoint 0x%x\n,
- streams are already disabled!,
+ endpoint 0x%x,
+ streams are already disabled!\n,
eps[i]-desc.bEndpointAddress);
xhci_warn(xhci, WARN xhci_free_streams() called
with non-streams endpoint\n);
@@ -4353,7 +4353,7 @@ static u16 xhci_get_timeout_no_hub_lpm(struct usb_device
*udev,
state_name, sel);
else
dev_dbg(udev-dev, Device-initiated %s disabled
- due to long PEL %llu\n ms,
+ due to long PEL %llu ms\n,
state_name, pel);
return USB3_LPM_DISABLED;
}
--
1.7.9
--
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
[Pull Request] xhci: Features for 3.12
The following changes since commit 3b2f64d00c46e1e4e9bd0bb9bb12619adac27a4b: Linux 3.11-rc2 (2013-07-21 12:05:29 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci.git for-usb-next-2013-07-23 for you to fetch changes up to 03e64e967180181510de06eecae3be44e648b692: xhci: Correct misplaced newlines (2013-07-23 14:50:29 -0700) xhci: Features for 3.12 In the spirit of let's stop gossiping around the water cooler and get to work, here's some xHCI patches for 3.12. They include a patch for suspend/resume support for xhci platform hosts, two patches to support showing USB 2.1 link status, and a patch to future-proof the Intel EHCI to xHCI port switchover. Sarah Sharp Joe Perches (1): xhci: Correct misplaced newlines Mathias Nyman (1): Intel xhci: refactor EHCI/xHCI port switching Sarah Sharp (2): xhci: Refactor port status into a new function. xhci: Report USB 2.1 link status for L1 Vikas Sajjan (1): usb: xhci: add the suspend/resume functionality drivers/usb/host/ehci-pci.c | 42 drivers/usb/host/pci-quirks.c | 48 -- drivers/usb/host/pci-quirks.h |3 +- drivers/usb/host/xhci-hub.c | 221 - drivers/usb/host/xhci-pci.c | 14 ++- drivers/usb/host/xhci-plat.c | 26 + drivers/usb/host/xhci.c | 10 +- 7 files changed, 187 insertions(+), 177 deletions(-) -- 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] xhci: Refactor port status into a new function.
The hub control function is *way* too long. Refactor it into a new
function, and document the side effects of calling that function.
Signed-off-by: Sarah Sharp sarah.a.sh...@linux.intel.com
---
drivers/usb/host/xhci-hub.c | 210 ---
1 files changed, 119 insertions(+), 91 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 1d35459..83fc636 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -534,6 +534,118 @@ void xhci_del_comp_mod_timer(struct xhci_hcd *xhci, u32
status, u16 wIndex)
}
}
+/*
+ * Converts a raw xHCI port status into the format that external USB 2.0 or USB
+ * 3.0 hubs use.
+ *
+ * Possible side effects:
+ * - Mark a port as being done with device resume,
+ *and ring the endpoint doorbells.
+ * - Stop the Synopsys redriver Compliance Mode polling.
+ */
+static u32 xhci_get_port_status(struct usb_hcd *hcd,
+ struct xhci_bus_state *bus_state,
+ __le32 __iomem **port_array,
+ u16 wIndex, u32 raw_port_status)
+{
+ struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+ u32 status = 0;
+ int slot_id;
+
+ /* wPortChange bits */
+ if (raw_port_status PORT_CSC)
+ status |= USB_PORT_STAT_C_CONNECTION 16;
+ if (raw_port_status PORT_PEC)
+ status |= USB_PORT_STAT_C_ENABLE 16;
+ if ((raw_port_status PORT_OCC))
+ status |= USB_PORT_STAT_C_OVERCURRENT 16;
+ if ((raw_port_status PORT_RC))
+ status |= USB_PORT_STAT_C_RESET 16;
+ /* USB3.0 only */
+ if (hcd-speed == HCD_USB3) {
+ if ((raw_port_status PORT_PLC))
+ status |= USB_PORT_STAT_C_LINK_STATE 16;
+ if ((raw_port_status PORT_WRC))
+ status |= USB_PORT_STAT_C_BH_RESET 16;
+ }
+
+ if (hcd-speed != HCD_USB3) {
+ if ((raw_port_status PORT_PLS_MASK) == XDEV_U3
+(raw_port_status PORT_POWER))
+ status |= USB_PORT_STAT_SUSPEND;
+ }
+ if ((raw_port_status PORT_PLS_MASK) == XDEV_RESUME
+ !DEV_SUPERSPEED(raw_port_status)) {
+ if ((raw_port_status PORT_RESET) ||
+ !(raw_port_status PORT_PE))
+ return 0x;
+ if (time_after_eq(jiffies,
+ bus_state-resume_done[wIndex])) {
+ xhci_dbg(xhci, Resume USB2 port %d\n,
+ wIndex + 1);
+ bus_state-resume_done[wIndex] = 0;
+ clear_bit(wIndex, bus_state-resuming_ports);
+ xhci_set_link_state(xhci, port_array, wIndex,
+ XDEV_U0);
+ xhci_dbg(xhci, set port %d resume\n,
+ wIndex + 1);
+ slot_id = xhci_find_slot_id_by_port(hcd, xhci,
+ wIndex + 1);
+ if (!slot_id) {
+ xhci_dbg(xhci, slot_id is zero\n);
+ return 0x;
+ }
+ xhci_ring_device(xhci, slot_id);
+ bus_state-port_c_suspend |= 1 wIndex;
+ bus_state-suspended_ports = ~(1 wIndex);
+ } else {
+ /*
+* The resume has been signaling for less than
+* 20ms. Report the port status as SUSPEND,
+* let the usbcore check port status again
+* and clear resume signaling later.
+*/
+ status |= USB_PORT_STAT_SUSPEND;
+ }
+ }
+ if ((raw_port_status PORT_PLS_MASK) == XDEV_U0
+(raw_port_status PORT_POWER)
+(bus_state-suspended_ports (1 wIndex))) {
+ bus_state-suspended_ports = ~(1 wIndex);
+ if (hcd-speed != HCD_USB3)
+ bus_state-port_c_suspend |= 1 wIndex;
+ }
+ if (raw_port_status PORT_CONNECT) {
+ status |= USB_PORT_STAT_CONNECTION;
+ status |= xhci_port_speed(raw_port_status);
+ }
+ if (raw_port_status PORT_PE)
+ status |= USB_PORT_STAT_ENABLE;
+ if (raw_port_status PORT_OC)
+ status |= USB_PORT_STAT_OVERCURRENT;
+ if (raw_port_status PORT_RESET)
+ status |= USB_PORT_STAT_RESET;
+ if (raw_port_status PORT_POWER) {
+ if (hcd-speed == HCD_USB3)
+ status |= USB_SS_PORT_STAT_POWER;
+ else
+ status |= USB_PORT_STAT_POWER;
+ }
+ /* Update Port Link State
[PATCH 1/5] usb: xhci: add the suspend/resume functionality
From: Vikas Sajjan vikas.saj...@linaro.org
Adds power management support to xHCI platform driver.
This patch facilitates the transition of xHCI host controller
between S0 and S3/S4 power states, during suspend/resume cycles.
Signed-off-by: Abhilash Kesavan a.kesa...@samsung.com
Signed-off-by: Vikas C Sajjan vikas.saj...@linaro.org
CC: Doug Anderson diand...@chromium.org
Signed-off-by: Felipe Balbi ba...@ti.com
Signed-off-by: Sarah Sharp sarah.a.sh...@linux.intel.com
---
drivers/usb/host/xhci-plat.c | 26 ++
1 files changed, 26 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index 51e22bf..412fe8d 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -186,11 +186,37 @@ static int xhci_plat_remove(struct platform_device *dev)
return 0;
}
+#ifdef CONFIG_PM
+static int xhci_plat_suspend(struct device *dev)
+{
+ struct usb_hcd *hcd = dev_get_drvdata(dev);
+ struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+
+ return xhci_suspend(xhci);
+}
+
+static int xhci_plat_resume(struct device *dev)
+{
+ struct usb_hcd *hcd = dev_get_drvdata(dev);
+ struct xhci_hcd *xhci = hcd_to_xhci(hcd);
+
+ return xhci_resume(xhci, 0);
+}
+
+static const struct dev_pm_ops xhci_plat_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(xhci_plat_suspend, xhci_plat_resume)
+};
+#define DEV_PM_OPS (xhci_plat_pm_ops)
+#else
+#define DEV_PM_OPS NULL
+#endif /* CONFIG_PM */
+
static struct platform_driver usb_xhci_driver = {
.probe = xhci_plat_probe,
.remove = xhci_plat_remove,
.driver = {
.name = xhci-hcd,
+ .pm = DEV_PM_OPS,
},
};
MODULE_ALIAS(platform:xhci-hcd);
--
1.7.9
--
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 3/5] xhci: Report USB 2.1 link status for L1
USB 2.1 devices can go into a lower power link state, L1. When they are
active, they are in the L0 state. The L1 transition can be purely
driven by software, or some USB host controllers (including some xHCI
1.0 hosts) allow the host hardware to track idleness and automatically
place a port into L1.
The USB 2.1 Link Power Management ECN gives a way for USB 2.1 hubs that
support LPM to report that a port is in L1. The port status bit 5 will
be set when the port is in L1. The xHCI host reports the root port as
being in 'U2' when the devices is in L1, and as being in 'U0' when the
port is active (in L0).
Translate the xHCI USB 2.1 link status into the format external hubs
use, and pass the L1 status up to the USB core and tools like lsusb.
Signed-off-by: Sarah Sharp sarah.a.sh...@linux.intel.com
---
drivers/usb/host/xhci-hub.c | 15 ---
1 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 83fc636..93f3fdf 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -461,8 +461,15 @@ void xhci_test_and_clear_bit(struct xhci_hcd *xhci, __le32
__iomem **port_array,
}
}
+/* Updates Link Status for USB 2.1 port */
+static void xhci_hub_report_usb2_link_state(u32 *status, u32 status_reg)
+{
+ if ((status_reg PORT_PLS_MASK) == XDEV_U2)
+ *status |= USB_PORT_STAT_L1;
+}
+
/* Updates Link Status for super Speed port */
-static void xhci_hub_report_link_state(u32 *status, u32 status_reg)
+static void xhci_hub_report_usb3_link_state(u32 *status, u32 status_reg)
{
u32 pls = status_reg PORT_PLS_MASK;
@@ -631,14 +638,16 @@ static u32 xhci_get_port_status(struct usb_hcd *hcd,
else
status |= USB_PORT_STAT_POWER;
}
- /* Update Port Link State for super speed ports*/
+ /* Update Port Link State */
if (hcd-speed == HCD_USB3) {
- xhci_hub_report_link_state(status, raw_port_status);
+ xhci_hub_report_usb3_link_state(status, raw_port_status);
/*
* Verify if all USB3 Ports Have entered U0 already.
* Delete Compliance Mode Timer if so.
*/
xhci_del_comp_mod_timer(xhci, raw_port_status, wIndex);
+ } else {
+ xhci_hub_report_usb2_link_state(status, raw_port_status);
}
if (bus_state-port_c_suspend (1 wIndex))
status |= 1 USB_PORT_FEAT_C_SUSPEND;
--
1.7.9
--
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/7] staging: usbip: add option for usbipd to save its process id.
On Mon, Jul 08, 2013 at 07:43:30PM -0600, Anthony Foiani wrote: Anthony Foiani anthony.foi...@gmail.com writes: Introduce option -P / --pid to request that usbipd save its PID to a file while running. I've already spotted one problem with this patch: + write_pid_file(pid_file); rc = do_standalone_mode(daemonize); + remove_pid_file(pid_file); I need to write the PID *after* the daemon(3) call. (I suspect I did all my testing in foreground mode, so didn't notice this until I actually tried to use it on my project...) So there will be a v2, but I'll probably wait for more feedback before doing an official respin. I've applied the first 3 patches, as they seemed simple enough :) 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 5/7] staging: usbip: set server port via option.
On Mon, Jul 08, 2013 at 07:47:20PM -0600, Anthony Foiani wrote: Anthony Foiani anthony.foi...@gmail.com writes: Add an option -p / --port to specify the TCP port to listen on. This is unfortunate, as port is also used to indicate which usbip port is in use by a particular connection. So I might change this to -t / --tcp-port in v2. Yes, please don't confuse people. 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: Weird Serial number with Fitbit Base Station dongle
Le Tue, 23 Jul 2013 10:16:00 -0400 (EDT), Alan Stern st...@rowland.harvard.edu a écrit : On Tue, 23 Jul 2013, Laurent Bigonville wrote: Hi, This is probably a minor issue, but when plugin the Fitbit base station dongle, I'm getting some weird serial number in dmesg. Can something be done here? Kind regards Laurent Bigonville [ 9993.722197] usb 1-1.2: new full-speed USB device number 7 using ehci-pci [ 9993.820617] usb 1-1.2: New USB device found, idVendor=2687, idProduct=fb01 [ 9993.820622] usb 1-1.2: New USB device strings: Mfr=1, Product=2, SerialNumber=3 [ 9993.820625] usb 1-1.2: Product: Fitbit Base Station [ 9993.820627] usb 1-1.2: Manufacturer: Fitbit Inc. [ 9993.820629] usb 1-1.2: SerialNumber: \xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf\xffef\xffbf\xffbf\xffbf\xffbf It may be that, weird as this looks, it is actually correct. Can you post a usbmon trace showing what happens when the dongle is first plugged? Instructions are in the kernel source file Documentation/usb/usbmon.txt. I hope this is enough: 8801b840fbc0 3460461486 S Ci:7:001:0 s a3 00 0001 0004 4 8801b840fbc0 3460461497 C Ci:7:001:0 0 4 = 0001 8801b840fbc0 3460461500 S Ci:7:001:0 s a3 00 0002 0004 4 8801b840fbc0 3460461503 C Ci:7:001:0 0 4 = 0001 8801b840fbc0 3460461505 S Ci:7:001:0 s a3 00 0003 0004 4 8801b840fbc0 3460461507 C Ci:7:001:0 0 4 = 0001 8801b840fbc0 3460461508 S Ci:7:001:0 s a3 00 0004 0004 4 8801b840fbc0 3460461510 C Ci:7:001:0 0 4 = 0001 8801b840fbc0 3460461511 S Ci:7:001:0 s a3 00 0005 0004 4 8801b840fbc0 3460461513 C Ci:7:001:0 0 4 = 0705 8801b840fbc0 3460461515 S Ci:7:001:0 s a3 00 0006 0004 4 8801b840fbc0 3460461517 C Ci:7:001:0 0 4 = 0001 8801b533d680 3460461518 S Ii:7:001:1 -115:2048 4 8801b533d680 3460465524 C Ii:7:001:1 0:2048 1 = 20 8801b533d680 3460465531 S Ii:7:001:1 -115:2048 4 880170d62e00 3460465611 S Ci:7:001:0 s a3 00 0005 0004 4 880170d62e00 3460465653 C Ci:7:001:0 0 4 = 03050400 880170d62e00 3460465655 S Co:7:001:0 s 23 01 0012 0005 0 880170d62e00 3460465660 C Co:7:001:0 0 0 880170d62e00 3460481441 S Ci:7:001:0 s a3 00 0005 0004 4 880170d62e00 3460481446 C Ci:7:001:0 0 4 = 0305 880170d62e00 3460481449 S Ci:7:004:0 s 80 00 0002 2 880170d62e00 3460481592 C Ci:7:004:0 0 2 = 0300 880170d62e00 3460481696 S Co:7:004:0 s 00 01 0001 0 880170d62e00 3460481842 C Co:7:004:0 0 0 880170d62e00 3460481922 S Ci:7:004:0 s a3 00 0001 0004 4 880170d62e00 3460482114 C Ci:7:004:0 0 4 = 0001 880170d62e00 3460482198 S Ci:7:004:0 s a3 00 0002 0004 4 880170d62e00 3460482368 C Ci:7:004:0 0 4 = 01010100 880170d62e00 3460482454 S Co:7:004:0 s 23 01 0010 0002 0 880170d62e00 3460482613 C Co:7:004:0 0 0 880170d62e00 3460482692 S Ci:7:004:0 s a3 00 0003 0004 4 880170d62e00 3460482869 C Ci:7:004:0 0 4 = 0001 880170d62e00 3460482895 S Ci:7:004:0 s a3 00 0004 0004 4 880170d62e00 3460483105 C Ci:7:004:0 0 4 = 0001 8801b485c500 3460585479 S Ii:7:004:1 -115:2048 1 880170d62e00 3460585496 S Ci:7:004:0 s a3 00 0002 0004 4 880170d62e00 3460585554 C Ci:7:004:0 0 4 = 0101 880170d62e00 3460585604 S Co:7:004:0 s 23 03 0004 0002 0 880170d62e00 3460585669 C Co:7:004:0 0 0 8801b76a1840 3460601424 S Ci:7:004:0 s a3 00 0002 0004 4 8801b76a1840 3460601635 C Ci:7:004:0 0 4 = 1101 8801b840fbc0 3460617472 S Ci:7:004:0 s a3 00 0002 0004 4 8801b840fbc0 3460617537 C Ci:7:004:0 0 4 = 03011000 8801b840fbc0 3460673477 S Co:7:004:0 s 23 01 0014 0002 0 8801b840fbc0 3460673546 C Co:7:004:0 0 0 8801b840fbc0 3460673572 S Ci:7:000:0 s 80 06 0100 0040 64 8801b840fbc0 3460674092 C Ci:7:000:0 0 18 = 12010002 0020 872601fb 00010102 0301 8801b840fbc0 3460674158 S Co:7:004:0 s 23 03 0004 0002 0 8801b840fbc0 3460674347 C Co:7:004:0 0 0 8801b840fbc0 3460689449 S Ci:7:004:0 s a3 00 0002 0004 4 8801b840fbc0 3460689590 C Ci:7:004:0 0 4 = 1101 8801b840fbc0 3460705380 S Ci:7:004:0 s a3 00 0002
Re: [Pull Request] xhci: Features for 3.12
On Tue, Jul 23, 2013 at 03:01:32PM -0700, Sarah Sharp wrote: The following changes since commit 3b2f64d00c46e1e4e9bd0bb9bb12619adac27a4b: Linux 3.11-rc2 (2013-07-21 12:05:29 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/sarah/xhci.git for-usb-next-2013-07-23 That's just a branch, not a signed tag? Why not a signed tag? 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
