PLX USB3380 (was: usbtest and gadget zero with PLX 3380)
On 22.04.2012 17:52, Thomas Knauth wrote: I am trying to develop a driver for the net2280 successor (USB 3380 from PLX)... What is the current state of this project? Is anyone still working on a driver for the PLX USB3380? For registered users PLXTech itself offers a driver download[1] which is a snapshot of Linux kernel version: 2.6.38.6-26.rc1.fc15.x86_64 (Linux OS used: Fedora 15). The code itself contains MODULE_LICENSE(GPL); So it sould be a legal base for the mainline. [1] http://www.plxtech.com/products/usbcontrollers/usb3380 (Linux SDK 1.3.1) Regards, Steffen Sledz -- DResearch Fahrzeugelektronik GmbH Otto-Schmirgal-Str. 3, 10319 Berlin, Germany Tel: +49 30 515932-237 mailto:sl...@dresearch-fe.de Fax: +49 30 515932-299 Geschäftsführer: Dr. Michael Weber, Werner Mögle; Amtsgericht Berlin Charlottenburg; HRB 130120 B; Ust.-IDNr. DE273952058 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/6] uas: make work list per-device
On Di, 2013-09-17 at 13:30 -0700, Christoph Hellwig wrote: On Fri, Sep 13, 2013 at 01:27:12PM +0200, Gerd Hoffmann wrote: Simplifies locking, we'll protect the list with the device spin lock. Also plugs races which can happen when two devices operate on the global list. While being at it rename the list head from list to work, preparing for the addition of a second list. Why do you even the list? The list was already there when I took over maintainance ... What would a ordered per-device workqueue not provide? Had no reason to look into replacing the list with something else so far. Why do you suggest using a workqueue instead? Note that the list is not used in a typical request workflow. Only in case queuing an usb urb failed the request is linked into the list and the worker will try to submit the usb urb again. cheers, Gerd -- To unsubscribe from this list: send the line unsubscribe 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/4] usb: musb: Call atomic_notifier_call_chain when status is changed
On Wednesday 18 September 2013 03:49:42 Felipe Balbi wrote:
On Tue, Sep 17, 2013 at 09:28:42PM +0200, Pali Rohár wrote:
On Tuesday 17 September 2013 18:08:35 Felipe Balbi wrote:
On Tue, Sep 17, 2013 at 06:05:15PM +0200, Pali Rohár wrote:
On Tuesday 17 September 2013 17:48:59 you wrote:
On Sun, Sep 08, 2013 at 10:50:36AM +0200, Pali Rohár wrote:
More power supply drivers depends on vbus events and
without it they not working. Power supply drivers
using usb_register_notifier, so to deliver events
it is needed to call atomic_notifier_call_chain.
So without atomic notifier power supply driver
isp1704 not retrieving vbus status and reporting
bogus values to userspace and also to board
platform data functions. Without proper data
charger drivers trying to charge battery also when
charger is disconnected or do not start charging
when wallcharger connects.
Atomic notifier in musb driver was used before v3.5
and was replaced with omap mailbox. This patch
adding atomic_notifier_call_chain call from
function omap_musb_set_mailbox.
Signed-off-by: Pali Rohár pali.ro...@gmail.com
---
drivers/usb/musb/omap2430.c |3 +++
drivers/usb/phy/phy-twl4030-usb.c |2 ++
2 files changed, 5 insertions(+)
diff --git a/drivers/usb/musb/omap2430.c
b/drivers/usb/musb/omap2430.c index f44e8b5..5c40252
100644 --- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -305,6 +305,9 @@ static void
omap_musb_set_mailbox(struct omap2430_glue *glue)
default:
dev_dbg(dev, ID float\n);
}
+
+ atomic_notifier_call_chain(musb-xceiv-notifier,
+ musb-xceiv-last_event, NULL);
let's add a wrapper for this:
static inline int usb_phy_notify(struct usb phy *x,
unsigned val, void *v) {
return atomic_notifier_call_chain(x-notifier, val,
v);
}
Where to add this wrapper? To omap2430.c? or some
include file?
linux/usb/phy.h
On Tuesday 17 September 2013 17:49:17 Felipe Balbi wrote:
On Sun, Sep 08, 2013 at 10:50:36AM +0200, Pali Rohár wrote:
diff --git a/drivers/usb/phy/phy-twl4030-usb.c
b/drivers/usb/phy/phy-twl4030-usb.c index
8f78d2d..efe6155 100644
--- a/drivers/usb/phy/phy-twl4030-usb.c
+++ b/drivers/usb/phy/phy-twl4030-usb.c
@@ -705,6 +705,8 @@ static int
twl4030_usb_probe(struct platform_device *pdev)
if (device_create_file(pdev-dev,
dev_attr_vbus))
dev_warn(pdev-dev, could not create sysfs
file\n);
+ ATOMIC_INIT_NOTIFIER_HEAD(twl-phy.notifier);
BTW, this is a bugfix, send separately.
What to send separately?
This full patch 1/4 is bugfix. And I did not understand
what you want. Maybe it could be easier for you to
apply this small 3+2 lines patch how you need.
This hunk here (initializaing notifier head) is a separate
bug fix and needs its own patch.
So will you do that? Or it is needed to resend this one line
hunk again in new email again?
new patch, new email
Guys, WHY ARE YOU SO STUPID AND ARROGANT?
Sorry but, need to copy full isolated patch/hunk from one mail to
another is hassling. So what you want from me? Do all those non
sense working only because yesterday you had bad day? Or what?
Everything needed with described information was in first mail.
Also second hunk has full isolated git diff output, so it is for
you really big problem to copy it? Or you did not see that patch?
I really did not understand what you wanted from me.
BEGINNING OF PATCH
This is bugfix and sending this patch separately from all other patches.
This patch is visibly isolated from all others and could be readable also
by disabled people. For other handicapped people I suggest to increase
font size and other text settings in program which view this patch.
For visually impaired people I suggest to use some text-to-speech software.
This is small 2 lines patch, diff starting after next visible break.
This patch initializing notifier head in tw4030 usb code which is missing.
Initialization code is needed for using any atomic_notifier_* functions.
Signed-off-by: Pali Rohár pali.ro...@gmail.com
===
BEGINNING OF DIFF
===
diff --git a/drivers/usb/phy/phy-twl4030-usb.c
b/drivers/usb/phy/phy-twl4030-usb.c
index 8f78d2d..efe6155 100644
--- a/drivers/usb/phy/phy-twl4030-usb.c
+++ b/drivers/usb/phy/phy-twl4030-usb.c
@@ -705,6 +705,8 @@ static int twl4030_usb_probe(struct platform_device *pdev)
if (device_create_file(pdev-dev,
Re: Fix style in s3c-hsotg.c
On Tue 2013-09-17 20:45:39, Felipe Balbi wrote:
On Wed, Sep 18, 2013 at 12:04:27AM +0200, Pavel Machek wrote:
On Tue 2013-09-17 10:42:30, Felipe Balbi wrote:
Hi,
On Mon, Sep 02, 2013 at 03:58:32PM +0200, Pavel Machek wrote:
Hi!
checkpatch.pl has some valid complaints about style in s3c-hsotg.c :
macro with if should be really enclosed in do {} while, and puts is
going to be slightly faster.
Here's suggested patch. I don't have the hardware, so it is completely
untested.
Signed-off-by: Pavel Machek, pa...@denx.de
this is not how you send a patch, please read
Documentation/SubmittingPatches
Have you considered possibility that this is how you nudge maintainer
into fix their coding style?
cute...
Seriously though, read that file, you're commit log has garbage in it
which shouldn't go to git history.
Run git log on SubmittingPatches.
Then, instead of telling me what to read, run checkpatch on your
files. You can either fix them yourself, or use my patch as a
basis. Note there's missing } or something, so it probably will not
compile, see the other mail. So you actually will have to modify that
patch. Stripping Hi! from it should not be that hard, neither should
be stripping note that patch is untested when you actually test
it. And as you are the maintainer, it is your job.
No, I'll not polish patch for hardware I don't have and have little
interest in. wanted to help you, but according to your first reply,
you do not really want help.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures)
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/6] usb: s3c-hsotg: fixes for proper functioning
Hello, This is a second version of set of patches fixing s3c-hsotg USB driver functioning. I've fixed issuses pointed by Felipe Balbi. For more information, please check the change log at the end of the mail. These patches add few fixes: - Fix protocol stall handling, by enqueue new ep0 request when stalled - Fix s3c_hsotg_write_fifo function to be usable in dedicated-fifo mode. Actually PTxFEmp/NPTxFEmp interrupts are enabled only in shared-fifo mode. - Fix endpoint interrput handling, by disabling interrupts for endpoints which has no requests enqueued. - Fixed DAINT register usage. Now only masked ep interrupts are handled. - Fix halt property updating. - Fix endpoint halt clearing, when it is not currently halted. Best regards Robert Baldyga Samsung RD Institute Poland Changelog: v2: - splitted patches to get one fix in each patch, as Felipe Balbi suggested - fixed typos v1: https://lkml.org/lkml/2013/9/12/324 - initial proposal Robert Baldyga (6): USB: gadget: s3c-hsotg: fix protocol stall handling USB: gadget: s3c-hsotg: fix s3c_hsotg_write_fifo function for dedicated fifo mode USB: gadget: s3c-hsotg: fix endpoint interrupts handling USB: gadget: s3c-hsotg: add DAINT masking USB: gadget: s3c-hsotg: fix halted property updating USB: gadget: s3c-hsotg: fix clear feature ENDPOINT_HALT drivers/usb/gadget/s3c-hsotg.c | 60 1 file changed, 48 insertions(+), 12 deletions(-) -- 1.7.9.5 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 6/6] USB: gadget: s3c-hsotg: fix clear feature ENDPOINT_HALT
All requests for endpoint are completed when it was halted and the halt was
cleared by CLEAR_FEATURE, but not when new state is same as previous.
Signed-off-by: Robert Baldyga r.bald...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/usb/gadget/s3c-hsotg.c | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index 23eec8f..84c302a 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -1097,6 +1097,7 @@ static int s3c_hsotg_process_req_feature(struct s3c_hsotg
*hsotg,
bool set = (ctrl-bRequest == USB_REQ_SET_FEATURE);
struct s3c_hsotg_ep *ep;
int ret;
+ bool halted;
dev_dbg(hsotg-dev, %s: %s_FEATURE\n,
__func__, set ? SET : CLEAR);
@@ -,6 +1112,8 @@ static int s3c_hsotg_process_req_feature(struct s3c_hsotg
*hsotg,
switch (le16_to_cpu(ctrl-wValue)) {
case USB_ENDPOINT_HALT:
+ halted = ep-halted;
+
s3c_hsotg_ep_sethalt(ep-ep, set);
ret = s3c_hsotg_send_reply(hsotg, ep0, NULL, 0);
@@ -1120,7 +1123,12 @@ static int s3c_hsotg_process_req_feature(struct
s3c_hsotg *hsotg,
return ret;
}
- if (!set) {
+ /*
+* we have to complete all requests for ep if it was
+* halted, and the halt was cleared by CLEAR_FEATURE
+*/
+
+ if (!set || halted) {
/*
* If we have request in progress,
* then complete it
--
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 v2 1/6] USB: gadget: s3c-hsotg: fix protocol stall handling
After normal handling of SetupDone interrupt, XferCompl interrupt occurs, and then we enqueue new setup request. But when ep0 is stalled, there is no XferCompl, so we have to enqueue setup request immediately after stalling ep. Otherwise incoming control requests won't be processed correctly. Signed-off-by: Robert Baldyga r.bald...@samsung.com Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com --- drivers/usb/gadget/s3c-hsotg.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c index dd5524c..c00ce27 100644 --- a/drivers/usb/gadget/s3c-hsotg.c +++ b/drivers/usb/gadget/s3c-hsotg.c @@ -1146,6 +1146,8 @@ static int s3c_hsotg_process_req_feature(struct s3c_hsotg *hsotg, return 1; } +static void s3c_hsotg_enqueue_setup(struct s3c_hsotg *hsotg); + /** * s3c_hsotg_process_control - process a control request * @hsotg: The device state @@ -1245,11 +1247,15 @@ static void s3c_hsotg_process_control(struct s3c_hsotg *hsotg, * don't believe we need to anything more to get the EP * to reply with a STALL packet */ + +/* + * complete won't be called, so we enqueue + * setup request here + */ +s3c_hsotg_enqueue_setup(hsotg); } } -static void s3c_hsotg_enqueue_setup(struct s3c_hsotg *hsotg); - /** * s3c_hsotg_complete_setup - completion of a setup transfer * @ep: The endpoint the request was on. -- 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 v2 3/6] USB: gadget: s3c-hsotg: fix endpoint interrupts handling
When s3c_hsotg_trytx is called for ep without enqueued request, interrupts
for this ep are disabled, to prevent interrupt flooding. Interrupts are
enabled when new request for this ep is starting.
Signed-off-by: Robert Baldyga r.bald...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/usb/gadget/s3c-hsotg.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index 4fe2f9e..3024685d 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -823,6 +823,9 @@ static void s3c_hsotg_start_req(struct s3c_hsotg *hsotg,
dev_dbg(hsotg-dev, %s: DxEPCTL=0x%08x\n,
__func__, readl(hsotg-regs + epctrl_reg));
+
+ /* enable ep interrupts */
+ s3c_hsotg_ctrl_epint(hsotg, hs_ep-index, hs_ep-dir_in, 1);
}
/**
@@ -1791,8 +1794,16 @@ static int s3c_hsotg_trytx(struct s3c_hsotg *hsotg,
{
struct s3c_hsotg_req *hs_req = hs_ep-req;
- if (!hs_ep-dir_in || !hs_req)
+ if (!hs_ep-dir_in || !hs_req) {
+ /**
+* if request is not enqueued, we disable interrupts
+* for endpoints, excepting ep0
+*/
+ if (hs_ep-index != 0)
+ s3c_hsotg_ctrl_epint(hsotg, hs_ep-index,
+hs_ep-dir_in, 0);
return 0;
+ }
if (hs_req-req.actual hs_req-req.length) {
dev_dbg(hsotg-dev, trying to write more for ep%d\n,
--
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 v2 4/6] USB: gadget: s3c-hsotg: add DAINT masking
In OEPInt/IEPInt interrupts handling added bitwise and of DAINT and
DAINTMSK, because we should handle masked interrupts only.
Signed-off-by: Robert Baldyga r.bald...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/usb/gadget/s3c-hsotg.c |8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index 3024685d..e32f868 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -2398,10 +2398,14 @@ irq_retry:
if (gintsts (GINTSTS_OEPInt | GINTSTS_IEPInt)) {
u32 daint = readl(hsotg-regs + DAINT);
- u32 daint_out = daint DAINT_OutEP_SHIFT;
- u32 daint_in = daint ~(daint_out DAINT_OutEP_SHIFT);
+ u32 daintmsk = readl(hsotg-regs + DAINTMSK);
+ u32 daint_out, daint_in;
int ep;
+ daint = daintmsk;
+ daint_out = daint DAINT_OutEP_SHIFT;
+ daint_in = daint ~(daint_out DAINT_OutEP_SHIFT);
+
dev_dbg(hsotg-dev, %s: daint=%08x\n, __func__, daint);
for (ep = 0; ep 15 daint_out; ep++, daint_out = 1) {
--
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 v2 2/6] USB: gadget: s3c-hsotg: fix s3c_hsotg_write_fifo function for dedicated fifo mode
In s3c_hsotg_write_fifo function PTxFEmp/NPTxFEmp interrupts are enabled
only in shared-fifo mode. In dedicated-fifo mode they should not be used
(when enabled then cause interrupt storm).
Signed-off-by: Robert Baldyga r.bald...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/usb/gadget/s3c-hsotg.c | 16 ++--
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index c00ce27..4fe2f9e 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -553,9 +553,11 @@ static int s3c_hsotg_write_fifo(struct s3c_hsotg *hsotg,
if (to_write hs_ep-ep.maxpacket) {
to_write = hs_ep-ep.maxpacket;
- s3c_hsotg_en_gsint(hsotg,
- periodic ? GINTSTS_PTxFEmp :
- GINTSTS_NPTxFEmp);
+ /* it's needed only when we do not use dedicated fifos */
+ if (!hsotg-dedicated_fifos)
+ s3c_hsotg_en_gsint(hsotg,
+ periodic ? GINTSTS_PTxFEmp :
+ GINTSTS_NPTxFEmp);
}
/* see if we can write data */
@@ -580,9 +582,11 @@ static int s3c_hsotg_write_fifo(struct s3c_hsotg *hsotg,
* is more room left.
*/
- s3c_hsotg_en_gsint(hsotg,
- periodic ? GINTSTS_PTxFEmp :
- GINTSTS_NPTxFEmp);
+ /* it's needed only when we do not use dedicated fifos */
+ if (!hsotg-dedicated_fifos)
+ s3c_hsotg_en_gsint(hsotg,
+ periodic ? GINTSTS_PTxFEmp :
+ GINTSTS_NPTxFEmp);
}
dev_dbg(hsotg-dev, write %d/%d, can_write %d, done %d\n,
--
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 v2 5/6] USB: gadget: s3c-hsotg: fix halted property updating
Property halted of s3c_hsotg_ep structure is actually initialised when ep
enabled, and changed when halt is set/cleared.
Signed-off-by: Robert Baldyga r.bald...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/usb/gadget/s3c-hsotg.c |3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index e32f868..23eec8f 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -2605,6 +2605,7 @@ static int s3c_hsotg_ep_enable(struct usb_ep *ep,
/* default, set to non-periodic */
hs_ep-periodic = 0;
+ hs_ep-halted = 0;
switch (desc-bmAttributes USB_ENDPOINT_XFERTYPE_MASK) {
case USB_ENDPOINT_XFER_ISOC:
@@ -2800,6 +2801,8 @@ static int s3c_hsotg_ep_sethalt(struct usb_ep *ep, int
value)
writel(epctl, hs-regs + epreg);
+ hs_ep-halted = value;
+
return 0;
}
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 06/10] ARM: dts: omap4: update omap-control-usb nodes
Split otghs_ctrl and USB2 PHY power-down into separate
omap-control-usb nodes. Get rid of ti,type property.
Also get rid of ti,has-mailbox property from usb_otg_hs
node and provide the ctrl-module phandle.
CC: Benoit Cousson bcous...@baylibre.com
Signed-off-by: Roger Quadros rog...@ti.com
---
arch/arm/boot/dts/omap4.dtsi | 20
1 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 22d9f2b..ca6400d 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -519,7 +519,7 @@
usb2_phy: usb2phy@4a0ad080 {
compatible = ti,omap-usb2;
reg = 0x4a0ad080 0x58;
- ctrl-module = omap_control_usb;
+ ctrl-module = omap_control_usb2phy;
};
};
@@ -643,12 +643,16 @@
};
};
- omap_control_usb: omap-control-usb@4a002300 {
- compatible = ti,omap-control-usb;
- reg = 0x4a002300 0x4,
- 0x4a00233c 0x4;
- reg-names = control_dev_conf, otghs_control;
- ti,type = 1;
+ omap_control_usb2phy: control-phy@4a002300 {
+ compatible = ti,control-phy-usb2;
+ reg = 0x4a002300 0x4;
+ reg-names = power;
+ };
+
+ omap_control_usbotg: control-phy@4a00233c {
+ compatible = ti,control-phy-otghs;
+ reg = 0x4a00233c 0x4;
+ reg-names = otghs_control;
};
usb_otg_hs: usb_otg_hs@4a0ab000 {
@@ -661,7 +665,7 @@
multipoint = 1;
num-eps = 16;
ram-bits = 12;
- ti,has-mailbox;
+ ctrl-module = omap_control_usbotg;
};
};
};
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 00/10] phy: omap-usb: Support multiple instances and new types
Hi, This patchset does the following: * Get rid of omap_control_usb platform data as we support DT only. * Restructure and add support for new PHY types. We now support the follwing four types TYPE_OTGHS - if it has otghs_control mailbox register (e.g. on OMAP4) TYPE_USB2 - if it has Power down bit in control_dev_conf register. e.g. USB2 PHY TYPE_PIPE3 - if it has DPLL and individual Rx Tx power control. e.g. USB3 PHY or SATA PHY TYPE_DRA7USB2 - if it has both power down and power aux registers. e.g. USB2 PHY on DRA7 * Get rid of ti,type DT property and introduce compatible strings for each type. * Have only one power control API omap_control_usb_phy_power() instead of a different one for each PHY type. * Get rid of omap_get_control_dev() so that we can support multiple instances of the control device. We take advantage of the fact that omap control USB device is only present on OMAP4 onwards and hence only supports DT boot. The users of control USB device can get a reference to it from the device node's phandle. Patches are based on v3.12-rc1. Smoke tested on OMAP4 panda with MUSB in gadget mode (g_zero). You can find the patches in branch usb-control-module in git tree git://github.com/rogerq/linux.git v7: - Rebased on v3.12-rc1 - Updated DT compatibility ID for better readability - Renamed TYPE_OMAP4 to TYPE_OTGHS, TYPE_USB3 to TYPE_PIPE3 and TYPE_DRA7 to TYPE_DRA7USB2 v6: - addressed review comment about usage of control_usb before allocation. v5: - fixed whitespace alignment issues. v4: - removed ti,has-mailbox from omap-usb binding document example. v3: - return -EINVAL instead of -ENODEV on probe failure. - pass device type data through of_device_id table. - get rid of ti,mailbox property and has_mailbox from musb platform data. v2: - get rid of ti,type property and introduce compatible strings for each device type. cheers, -roger --- Roger Quadros (10): usb: phy: omap-control: Get rid of platform data usb: phy: omap: Add new device types and remove omap_control_usb3_phy_power() usb: phy: omap-usb2: Don't use omap_get_control_dev() usb: phy: omap-usb3: Don't use omap_get_control_dev() usb: musb: omap2430: Don't use omap_get_control_dev() ARM: dts: omap4: update omap-control-usb nodes usb: phy: omap: get rid of omap_get_control_dev() ARM: dts: omap5: update omap-control-usb node usb: dwc3: omap: manage usb_otg_ss_refclk960m clock usb: phy: omap-usb2: Manage phy clock and not otg controller clock Documentation/devicetree/bindings/usb/omap-usb.txt | 38 ++-- arch/arm/boot/dts/omap4.dtsi | 20 ++- arch/arm/boot/dts/omap5.dtsi | 20 ++- drivers/usb/dwc3/dwc3-omap.c | 13 ++ drivers/usb/musb/omap2430.c| 25 ++- drivers/usb/phy/phy-omap-control.c | 208 ++- drivers/usb/phy/phy-omap-usb2.c| 30 ++- drivers/usb/phy/phy-omap-usb3.c| 32 +++- include/linux/usb/musb.h |2 - include/linux/usb/omap_control_usb.h | 33 ++-- 10 files changed, 236 insertions(+), 185 deletions(-) -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 10/10] usb: phy: omap-usb2: Manage phy clock and not otg controller clock
The OTG_SS controller driver manages the OTG_SS clock. The phy driver needs to manage the PHY's functional clock. So correct the clock name. Signed-off-by: Roger Quadros rog...@ti.com --- drivers/usb/phy/phy-omap-usb2.c |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/phy/phy-omap-usb2.c b/drivers/usb/phy/phy-omap-usb2.c index e7d4790..0fab250 100644 --- a/drivers/usb/phy/phy-omap-usb2.c +++ b/drivers/usb/phy/phy-omap-usb2.c @@ -179,9 +179,9 @@ static int omap_usb2_probe(struct platform_device *pdev) } clk_prepare(phy-wkupclk); - phy-optclk = devm_clk_get(phy-dev, usb_otg_ss_refclk960m); + phy-optclk = devm_clk_get(phy-dev, usb_phy_refclk960m); if (IS_ERR(phy-optclk)) - dev_vdbg(pdev-dev, unable to get refclk960m\n); + dev_dbg(pdev-dev, unable to get refclk960m\n); else clk_prepare(phy-optclk); -- 1.7.4.1 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 02/10] usb: phy: omap: Add new device types and remove omap_control_usb3_phy_power()
Add support for new device types and in the process rid of ti,type
device tree property. The correct type of device will be determined
from the compatible string instead.
Introduce a compatible string for each device type. At the moment
we support 4 types OTGHS, USB2, PIPE3 (e.g. USB3) and DRA7USB2.
Update DT binding information to reflect these changes.
Also get rid of omap_control_usb3_phy_power(). Just one function
i.e. omap_control_usb_phy_power() will now take care of all PHY types.
Signed-off-by: Roger Quadros rog...@ti.com
---
Documentation/devicetree/bindings/usb/omap-usb.txt | 30 ++--
drivers/usb/phy/phy-omap-control.c | 173 +++
drivers/usb/phy/phy-omap-usb3.c|6 +-
include/linux/usb/omap_control_usb.h | 24 ++--
4 files changed, 130 insertions(+), 103 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt
b/Documentation/devicetree/bindings/usb/omap-usb.txt
index 9088ab0..6d8ab9c 100644
--- a/Documentation/devicetree/bindings/usb/omap-usb.txt
+++ b/Documentation/devicetree/bindings/usb/omap-usb.txt
@@ -78,22 +78,22 @@ omap_dwc3 {
OMAP CONTROL USB
Required properties:
- - compatible: Should be ti,omap-control-usb
+ - compatible: Should be one of
+ ti,control-phy-otghs - if it has otghs_control mailbox register as on OMAP4.
+ ti,control-phy-usb2 - if it has Power down bit in control_dev_conf register
+ e.g. USB2_PHY on OMAP5.
+ ti,control-phy-pipe3 - if it has DPLL and individual Rx Tx power control
+ e.g. USB3 PHY and SATA PHY on OMAP5.
+ ti,control-phy-dra7usb2 - if it has power down register like USB2 PHY on
+ DRA7 platform.
- reg : Address and length of the register set for the device. It contains
- the address of control_dev_conf and otghs_control or phy_power_usb
- depending upon omap4 or omap5.
- - reg-names: The names of the register addresses corresponding to the
registers
- filled in reg.
- - ti,type: This is used to differentiate whether the control module has
- usb mailbox or usb3 phy power. omap4 has usb mailbox in control module to
- notify events to the musb core and omap5 has usb3 phy power register to
- power on usb3 phy. Should be 1 if it has mailbox and 2 if it has usb3
- phy power.
+ the address of otghs_control for control-phy-otghs or power register
+ for other types.
+ - reg-names: should be otghs_control control-phy-otghs and power for
+ other types.
omap_control_usb: omap-control-usb@4a002300 {
- compatible = ti,omap-control-usb;
- reg = 0x4a002300 0x4,
- 0x4a00233c 0x4;
- reg-names = control_dev_conf, otghs_control;
- ti,type = 1;
+ compatible = ti,control-phy-otghs;
+ reg = 0x4a00233c 0x4;
+ reg-names = otghs_control;
};
diff --git a/drivers/usb/phy/phy-omap-control.c
b/drivers/usb/phy/phy-omap-control.c
index 9772592..1c8a7c5 100644
--- a/drivers/usb/phy/phy-omap-control.c
+++ b/drivers/usb/phy/phy-omap-control.c
@@ -20,6 +20,7 @@
#include linux/platform_device.h
#include linux/slab.h
#include linux/of.h
+#include linux/of_device.h
#include linux/err.h
#include linux/io.h
#include linux/clk.h
@@ -46,61 +47,70 @@ struct device *omap_get_control_dev(void)
EXPORT_SYMBOL_GPL(omap_get_control_dev);
/**
- * omap_control_usb3_phy_power - power on/off the serializer using control
- * module
+ * omap_control_usb_phy_power - power on/off the phy using control module reg
* @dev: the control module device
- * @on: 0 to off and 1 to on based on powering on or off the PHY
- *
- * usb3 PHY driver should call this API to power on or off the PHY.
+ * @on: 0 or 1, based on powering on or off the PHY
*/
-void omap_control_usb3_phy_power(struct device *dev, bool on)
+void omap_control_usb_phy_power(struct device *dev, int on)
{
u32 val;
unsigned long rate;
- struct omap_control_usb *control_usb = dev_get_drvdata(dev);
+ struct omap_control_usb *control_usb;
- if (control_usb-type != OMAP_CTRL_DEV_TYPE2)
+ if (IS_ERR(dev) || !dev) {
+ pr_err(%s: invalid device\n, __func__);
return;
+ }
- rate = clk_get_rate(control_usb-sys_clk);
- rate = rate/100;
-
- val = readl(control_usb-phy_power);
-
- if (on) {
- val = ~(OMAP_CTRL_USB_PWRCTL_CLK_CMD_MASK |
- OMAP_CTRL_USB_PWRCTL_CLK_FREQ_MASK);
- val |= OMAP_CTRL_USB3_PHY_TX_RX_POWERON
- OMAP_CTRL_USB_PWRCTL_CLK_CMD_SHIFT;
- val |= rate OMAP_CTRL_USB_PWRCTL_CLK_FREQ_SHIFT;
- } else {
- val = ~OMAP_CTRL_USB_PWRCTL_CLK_CMD_MASK;
- val |= OMAP_CTRL_USB3_PHY_TX_RX_POWEROFF
- OMAP_CTRL_USB_PWRCTL_CLK_CMD_SHIFT;
+ control_usb = dev_get_drvdata(dev);
+ if (!control_usb) {
+ dev_err(dev, %s: invalid control usb
[PATCH v7 03/10] usb: phy: omap-usb2: Don't use omap_get_control_dev()
omap_get_control_dev() is being deprecated as it doesn't support
multiple instances. As control device is present only from OMAP4
onwards which supports DT only, we use phandles to get the
reference to the control device.
As we don't support non-DT boot, we just bail out on probe
if device node is not present.
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/usb/phy/phy-omap-usb2.c | 26 --
1 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/phy/phy-omap-usb2.c b/drivers/usb/phy/phy-omap-usb2.c
index d266861..e7d4790 100644
--- a/drivers/usb/phy/phy-omap-usb2.c
+++ b/drivers/usb/phy/phy-omap-usb2.c
@@ -28,6 +28,7 @@
#include linux/pm_runtime.h
#include linux/delay.h
#include linux/usb/omap_control_usb.h
+#include linux/of_platform.h
/**
* omap_usb2_set_comparator - links the comparator present in the sytem with
@@ -120,8 +121,14 @@ static int omap_usb2_suspend(struct usb_phy *x, int
suspend)
static int omap_usb2_probe(struct platform_device *pdev)
{
- struct omap_usb *phy;
- struct usb_otg *otg;
+ struct omap_usb *phy;
+ struct usb_otg *otg;
+ struct device_node *node = pdev-dev.of_node;
+ struct device_node *control_node;
+ struct platform_device *control_pdev;
+
+ if (!node)
+ return -EINVAL;
phy = devm_kzalloc(pdev-dev, sizeof(*phy), GFP_KERNEL);
if (!phy) {
@@ -143,11 +150,18 @@ static int omap_usb2_probe(struct platform_device *pdev)
phy-phy.otg= otg;
phy-phy.type = USB_PHY_TYPE_USB2;
- phy-control_dev = omap_get_control_dev();
- if (IS_ERR(phy-control_dev)) {
- dev_dbg(pdev-dev, Failed to get control device\n);
- return -ENODEV;
+ control_node = of_parse_phandle(node, ctrl-module, 0);
+ if (!control_node) {
+ dev_err(pdev-dev, Failed to get control device phandle\n);
+ return -EINVAL;
}
+ control_pdev = of_find_device_by_node(control_node);
+ if (!control_pdev) {
+ dev_err(pdev-dev, Failed to get control device\n);
+ return -EINVAL;
+ }
+
+ phy-control_dev = control_pdev-dev;
phy-is_suspended = 1;
omap_control_usb_phy_power(phy-control_dev, 0);
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 04/10] usb: phy: omap-usb3: Don't use omap_get_control_dev()
omap_get_control_dev() is being deprecated as it doesn't support
multiple instances. As control device is present only from OMAP4
onwards which supports DT only, we use phandles to get the
reference to the control device.
As we don't support non-DT boot, we just bail out on probe
if device node is not present.
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/usb/phy/phy-omap-usb3.c | 26 --
1 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/phy/phy-omap-usb3.c b/drivers/usb/phy/phy-omap-usb3.c
index 4a7f27c..4004f82 100644
--- a/drivers/usb/phy/phy-omap-usb3.c
+++ b/drivers/usb/phy/phy-omap-usb3.c
@@ -26,6 +26,7 @@
#include linux/pm_runtime.h
#include linux/delay.h
#include linux/usb/omap_control_usb.h
+#include linux/of_platform.h
#definePLL_STATUS 0x0004
#definePLL_GO 0x0008
@@ -196,8 +197,14 @@ static int omap_usb3_init(struct usb_phy *x)
static int omap_usb3_probe(struct platform_device *pdev)
{
- struct omap_usb *phy;
- struct resource *res;
+ struct omap_usb *phy;
+ struct resource *res;
+ struct device_node *node = pdev-dev.of_node;
+ struct device_node *control_node;
+ struct platform_device *control_pdev;
+
+ if (!node)
+ return -EINVAL;
phy = devm_kzalloc(pdev-dev, sizeof(*phy), GFP_KERNEL);
if (!phy) {
@@ -239,11 +246,18 @@ static int omap_usb3_probe(struct platform_device *pdev)
return -EINVAL;
}
- phy-control_dev = omap_get_control_dev();
- if (IS_ERR(phy-control_dev)) {
- dev_dbg(pdev-dev, Failed to get control device\n);
- return -ENODEV;
+ control_node = of_parse_phandle(node, ctrl-module, 0);
+ if (!control_node) {
+ dev_err(pdev-dev, Failed to get control device phandle\n);
+ return -EINVAL;
}
+ control_pdev = of_find_device_by_node(control_node);
+ if (!control_pdev) {
+ dev_err(pdev-dev, Failed to get control device\n);
+ return -EINVAL;
+ }
+
+ phy-control_dev = control_pdev-dev;
omap_control_usb_phy_power(phy-control_dev, 0);
usb_add_phy_dev(phy-phy);
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 01/10] usb: phy: omap-control: Get rid of platform data
omap-control device is present from OMAP4 onwards which
support device tree boots only. So get rid of platform data.
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/usb/phy/phy-omap-control.c | 12 +++-
include/linux/usb/omap_control_usb.h |4
2 files changed, 3 insertions(+), 13 deletions(-)
diff --git a/drivers/usb/phy/phy-omap-control.c
b/drivers/usb/phy/phy-omap-control.c
index a4dda8e..9772592 100644
--- a/drivers/usb/phy/phy-omap-control.c
+++ b/drivers/usb/phy/phy-omap-control.c
@@ -197,8 +197,6 @@ static int omap_control_usb_probe(struct platform_device
*pdev)
{
struct resource *res;
struct device_node *np = pdev-dev.of_node;
- struct omap_control_usb_platform_data *pdata =
- dev_get_platdata(pdev-dev);
control_usb = devm_kzalloc(pdev-dev, sizeof(*control_usb),
GFP_KERNEL);
@@ -207,14 +205,10 @@ static int omap_control_usb_probe(struct platform_device
*pdev)
return -ENOMEM;
}
- if (np) {
+ if (np)
of_property_read_u32(np, ti,type, control_usb-type);
- } else if (pdata) {
- control_usb-type = pdata-type;
- } else {
- dev_err(pdev-dev, no pdata present\n);
- return -EINVAL;
- }
+ else
+ return -EINVAL; /* We only support DT boot */
control_usb-dev= pdev-dev;
diff --git a/include/linux/usb/omap_control_usb.h
b/include/linux/usb/omap_control_usb.h
index 27b5b8c..e2416b4 100644
--- a/include/linux/usb/omap_control_usb.h
+++ b/include/linux/usb/omap_control_usb.h
@@ -31,10 +31,6 @@ struct omap_control_usb {
u32 type;
};
-struct omap_control_usb_platform_data {
- u8 type;
-};
-
enum omap_control_usb_mode {
USB_MODE_UNDEFINED = 0,
USB_MODE_HOST,
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 07/10] usb: phy: omap: get rid of omap_get_control_dev()
This function was preventing us from supporting multiple
instances. Get rid of it. Since we support DT boots only,
users can get the control device phandle from the DT node.
Signed-off-by: Roger Quadros rog...@ti.com
---
drivers/usb/phy/phy-omap-control.c | 31 ++-
include/linux/usb/omap_control_usb.h |5 -
2 files changed, 10 insertions(+), 26 deletions(-)
diff --git a/drivers/usb/phy/phy-omap-control.c
b/drivers/usb/phy/phy-omap-control.c
index 1c8a7c5..09c5ace 100644
--- a/drivers/usb/phy/phy-omap-control.c
+++ b/drivers/usb/phy/phy-omap-control.c
@@ -26,26 +26,6 @@
#include linux/clk.h
#include linux/usb/omap_control_usb.h
-static struct omap_control_usb *control_usb;
-
-/**
- * omap_get_control_dev - returns the device pointer for this control device
- *
- * This API should be called to get the device pointer for this control
- * module device. This device pointer should be used for called other
- * exported API's in this driver.
- *
- * To be used by PHY driver and glue driver.
- */
-struct device *omap_get_control_dev(void)
-{
- if (!control_usb)
- return ERR_PTR(-ENODEV);
-
- return control_usb-dev;
-}
-EXPORT_SYMBOL_GPL(omap_get_control_dev);
-
/**
* omap_control_usb_phy_power - power on/off the phy using control module reg
* @dev: the control module device
@@ -182,11 +162,19 @@ void omap_control_usb_set_mode(struct device *dev,
{
struct omap_control_usb *ctrl_usb;
- if (IS_ERR(dev) || control_usb-type != OMAP_CTRL_TYPE_OTGHS)
+ if (IS_ERR(dev) || !dev)
return;
ctrl_usb = dev_get_drvdata(dev);
+ if (!ctrl_usb) {
+ dev_err(dev, Invalid control usb device\n);
+ return;
+ }
+
+ if (ctrl_usb-type != OMAP_CTRL_TYPE_OTGHS)
+ return;
+
switch (mode) {
case USB_MODE_HOST:
omap_control_usb_host_mode(ctrl_usb);
@@ -237,6 +225,7 @@ static int omap_control_usb_probe(struct platform_device
*pdev)
{
struct resource *res;
const struct of_device_id *of_id;
+ struct omap_control_usb *control_usb;
of_id = of_match_device(of_match_ptr(omap_control_usb_id_table),
pdev-dev);
diff --git a/include/linux/usb/omap_control_usb.h
b/include/linux/usb/omap_control_usb.h
index 61b889a..596b019 100644
--- a/include/linux/usb/omap_control_usb.h
+++ b/include/linux/usb/omap_control_usb.h
@@ -65,15 +65,10 @@ enum omap_control_usb_mode {
#define OMAP_CTRL_USB2_PHY_PD BIT(28)
#if IS_ENABLED(CONFIG_OMAP_CONTROL_USB)
-extern struct device *omap_get_control_dev(void);
extern void omap_control_usb_phy_power(struct device *dev, int on);
extern void omap_control_usb_set_mode(struct device *dev,
enum omap_control_usb_mode mode);
#else
-static inline struct device *omap_get_control_dev(void)
-{
- return ERR_PTR(-ENODEV);
-}
static inline void omap_control_usb_phy_power(struct device *dev, int on)
{
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 05/10] usb: musb: omap2430: Don't use omap_get_control_dev()
omap_get_control_dev() is being deprecated as it doesn't support
multiple instances. As control device is present only from OMAP4
onwards which supports DT only, we use phandles to get the
reference to the control device.
Also get rid of ti,has-mailbox property as it is redundant and
we can determine that from whether ctrl-module property is present
or not. Get rid of has_mailbox from musb_hdrc_platform_data as well.
Signed-off-by: Roger Quadros rog...@ti.com
---
Documentation/devicetree/bindings/usb/omap-usb.txt |4 ---
drivers/usb/musb/omap2430.c| 25 +++
include/linux/usb/musb.h |2 -
3 files changed, 14 insertions(+), 17 deletions(-)
diff --git a/Documentation/devicetree/bindings/usb/omap-usb.txt
b/Documentation/devicetree/bindings/usb/omap-usb.txt
index 6d8ab9c..f67573c 100644
--- a/Documentation/devicetree/bindings/usb/omap-usb.txt
+++ b/Documentation/devicetree/bindings/usb/omap-usb.txt
@@ -3,9 +3,6 @@ OMAP GLUE AND OTHER OMAP SPECIFIC COMPONENTS
OMAP MUSB GLUE
- compatible : Should be ti,omap4-musb or ti,omap3-musb
- ti,hwmods : must be usb_otg_hs
- - ti,has-mailbox : to specify that omap uses an external mailbox
- (in control module) to communicate with the musb core during device connect
- and disconnect.
- multipoint : Should be 1 indicating the musb controller supports
multipoint. This is a MUSB configuration-specific setting.
- num-eps : Specifies the number of endpoints. This is also a
@@ -28,7 +25,6 @@ SOC specific device node entry
usb_otg_hs: usb_otg_hs@4a0ab000 {
compatible = ti,omap4-musb;
ti,hwmods = usb_otg_hs;
- ti,has-mailbox;
multipoint = 1;
num-eps = 16;
ram-bits = 12;
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 59d2245..4512a05 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -38,6 +38,7 @@
#include linux/delay.h
#include linux/usb/musb-omap.h
#include linux/usb/omap_control_usb.h
+#include linux/of_platform.h
#include musb_core.h
#include omap2430.h
@@ -509,8 +510,12 @@ static int omap2430_probe(struct platform_device *pdev)
glue-dev = pdev-dev;
glue-musb = musb;
glue-status= OMAP_MUSB_UNKNOWN;
+ glue-control_otghs = ERR_PTR(-ENODEV);
if (np) {
+ struct device_node *control_node;
+ struct platform_device *control_pdev;
+
pdata = devm_kzalloc(pdev-dev, sizeof(*pdata), GFP_KERNEL);
if (!pdata) {
dev_err(pdev-dev,
@@ -539,22 +544,20 @@ static int omap2430_probe(struct platform_device *pdev)
of_property_read_u32(np, ram-bits, (u32 *)config-ram_bits);
of_property_read_u32(np, power, (u32 *)pdata-power);
config-multipoint = of_property_read_bool(np, multipoint);
- pdata-has_mailbox = of_property_read_bool(np,
- ti,has-mailbox);
pdata-board_data = data;
pdata-config = config;
- }
- if (pdata-has_mailbox) {
- glue-control_otghs = omap_get_control_dev();
- if (IS_ERR(glue-control_otghs)) {
- dev_vdbg(pdev-dev, Failed to get control device\n);
- ret = PTR_ERR(glue-control_otghs);
- goto err2;
+ control_node = of_parse_phandle(np, ctrl-module, 0);
+ if (control_node) {
+ control_pdev = of_find_device_by_node(control_node);
+ if (!control_pdev) {
+ dev_err(pdev-dev, Failed to get control
device\n);
+ ret = -EINVAL;
+ goto err2;
+ }
+ glue-control_otghs = control_pdev-dev;
}
- } else {
- glue-control_otghs = ERR_PTR(-ENODEV);
}
pdata-platform_ops = omap2430_ops;
diff --git a/include/linux/usb/musb.h b/include/linux/usb/musb.h
index 053c268..eb50525 100644
--- a/include/linux/usb/musb.h
+++ b/include/linux/usb/musb.h
@@ -99,8 +99,6 @@ struct musb_hdrc_platform_data {
/* MUSB_HOST, MUSB_PERIPHERAL, or MUSB_OTG */
u8 mode;
- u8 has_mailbox:1;
-
/* for clk_get() */
const char *clock;
--
1.7.4.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] usb: musb: Call atomic_notifier_call_chain when status is changed
Hi! So will you do that? Or it is needed to resend this one line hunk again in new email again? new patch, new email Guys, WHY ARE YOU SO STUPID AND ARROGANT? Sorry but, need to copy full isolated patch/hunk from one mail to another is hassling. So what you want from me? Do all those non sense working only because yesterday you had bad day? Or what? ... Hi Pali, There is no need to be rude. Felipe asked you to do the split since he believes that the notifier chain call for musb xceiv and the twl-phy notifier head init should be done in two separate patches. Actually, there is need to be rude, because Felipe fails to act as maintainer. Instead of fixing bugs in his code, he bounces bugfix patches, points people to random READMEs and wastes everyones time. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USBNET: fix handling padding packet
On Tue, 2013-09-17 at 17:10 +0800, Ming Lei wrote: Commit 638c5115a7949(USBNET: support DMA SG) introduces DMA SG if the usb host controller is capable of building packet from discontinuous buffers, but missed handling padding packet when building DMA SG. This patch attachs the pre-allocated padding packet at the end of the sg list, so padding packet can be sent to device if drivers require that. Reported-by: David Laight david.lai...@aculab.com Cc: Oliver Neukum oli...@neukum.org Signed-off-by: Ming Lei ming@canonical.com Acked-by: Oliver Neukum oli...@neukum.org -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Bug] 0ac8:0321 Vimicro generic vc0321 Camera is not working and causes crashes since 3.2
Hi, On 09/17/2013 08:25 PM, Frank Dierich wrote: On 09/09/2013 02:09 PM, Hans de Goede wrote: Thanks for the bug report, looking at the bug reports, they all report an error of -71 which is EPROTO, which typically means something is wrong at the USB level. And nothing has changed for the driver in question between 3.1 and 3.2 , so I believe this regression is caused by changes to the usb sub-system, likely changes to the EHCI driver. I have tested the new 3.12.0-rc1 kernel and the regression is still present. It causes that Cheese crashes with a segmentation fault and I get the following errors [ 139.868628] gspca_main: ISOC data error: [21] len=0, status=-71 [ 139.904620] gspca_main: ISOC data error: [12] len=0, status=-71 [ 139.936595] gspca_main: ISOC data error: [9] len=0, status=-71 [ 139.968576] gspca_main: ISOC data error: [17] len=0, status=-71 [ 140.036571] gspca_main: ISOC data error: [16] len=0, status=-71 [ 140.037364] video_source:sr[2570]: segfault at 8 ip 7f0430d6868c sp 7f0406c02900 error 4 in libgstreamer-0.10.so.0.30.0[7f0430d15000+de000] [ 140.068533] gspca_main: ISOC data error: [24] len=0, status=-71 [ 140.104519] gspca_main: ISOC data error: [15] len=0, status=-71 [ 140.168474] gspca_main: ISOC data error: [20] len=0, status=-71 [ 140.200461] gspca_main: ISOC data error: [28] len=0, status=-71 Note I'm not claiming there is not problem, but since your testing indicated that things broken between 3.1 and 3.2, I'm pretty sure that the breakage is not caused by changes to the vimicro driver, as that driver was not changed between 3.1 and 3.2. As I already said in my previous mail, the best way forward with this is probably to bisect the problem, and then send a mail to linux-usb@vger.kernel.org about this. Please CC me on this mail. Regards, Hans -- To unsubscribe from this list: send the line unsubscribe 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/4] usb: musb: Call atomic_notifier_call_chain when status is changed
On Wed, Sep 18, 2013 at 3:30 PM, Pavel Machek pa...@ucw.cz wrote: Hi! So will you do that? Or it is needed to resend this one line hunk again in new email again? new patch, new email Guys, WHY ARE YOU SO STUPID AND ARROGANT? Sorry but, need to copy full isolated patch/hunk from one mail to another is hassling. So what you want from me? Do all those non sense working only because yesterday you had bad day? Or what? ... Hi Pali, There is no need to be rude. Felipe asked you to do the split since he believes that the notifier chain call for musb xceiv and the twl-phy notifier head init should be done in two separate patches. Actually, there is need to be rude, because Felipe fails to act as maintainer. Instead of fixing bugs in his code, he bounces bugfix patches, points people to random READMEs and wastes everyones time. Pavel I don't know what are you talking about (if that happened in another thread then I need more context). Felipe is not bouncing any bugfix but just asked to split the patch in two since the patch was solving two separate issues so is way better to have it in two separate patches for the reasons I explained before. So, as far as I can tell Felipe did exactly what I would expect from a maintainer. He took the time to review the patches sent to him and gave feedback. If the sender doesn't want to take his feedback into account and prefer to send pretty insulting emails instead that is his choice but I would say that is this not the greatest approach to get your code merged (to say the least). Best regards, Javier -- To unsubscribe from this list: send the line unsubscribe 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] USBNET: fix handling padding packet
Ming Lei ming@canonical.com writes:
Commit 638c5115a7949(USBNET: support DMA SG) introduces DMA SG
if the usb host controller is capable of building packet from
discontinuous buffers, but missed handling padding packet when
building DMA SG.
This patch attachs the pre-allocated padding packet at the
end of the sg list, so padding packet can be sent to device
if drivers require that.
Reported-by: David Laight david.lai...@aculab.com
Cc: Oliver Neukum oli...@neukum.org
Signed-off-by: Ming Lei ming@canonical.com
---
drivers/net/usb/usbnet.c | 27 +--
include/linux/usb/usbnet.h |1 +
2 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 7b331e6..4a9bed3 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1241,7 +1241,9 @@ static int build_dma_sg(const struct sk_buff *skb,
struct urb *urb)
if (num_sgs == 1)
return 0;
- urb-sg = kmalloc(num_sgs * sizeof(struct scatterlist), GFP_ATOMIC);
+ /* reserve one for zero packet */
+ urb-sg = kmalloc((num_sgs + 1) * sizeof(struct scatterlist),
+ GFP_ATOMIC);
if (!urb-sg)
return -ENOMEM;
@@ -1305,7 +1307,7 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
if (build_dma_sg(skb, urb) 0)
goto drop;
}
- entry-length = length = urb-transfer_buffer_length;
+ length = urb-transfer_buffer_length;
/* don't assume the hardware handles USB_ZERO_PACKET
* NOTE: strictly conforming cdc-ether devices should expect
@@ -1317,15 +1319,18 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
if (length % dev-maxpacket == 0) {
if (!(info-flags FLAG_SEND_ZLP)) {
if (!(info-flags FLAG_MULTI_PACKET)) {
- urb-transfer_buffer_length++;
- if (skb_tailroom(skb)) {
+ length++;
+ if (skb_tailroom(skb) !dev-can_dma_sg) {
skb-data[skb-len] = 0;
__skb_put(skb, 1);
- }
+ } else if (dev-can_dma_sg)
+ sg_set_buf(urb-sg[urb-num_sgs++],
+ dev-padding_pkt, 1);
}
} else
urb-transfer_flags |= URB_ZERO_PACKET;
}
+ entry-length = urb-transfer_buffer_length = length;
spin_lock_irqsave(dev-txq.lock, flags);
retval = usb_autopm_get_interface_async(dev-intf);
@@ -1509,6 +1514,7 @@ void usbnet_disconnect (struct usb_interface *intf)
usb_kill_urb(dev-interrupt);
usb_free_urb(dev-interrupt);
+ kfree(dev-padding_pkt);
free_netdev(net);
}
@@ -1679,9 +1685,16 @@ usbnet_probe (struct usb_interface *udev, const struct
usb_device_id *prod)
/* initialize max rx_qlen and tx_qlen */
usbnet_update_max_qlen(dev);
+ if (dev-can_dma_sg !(info-flags FLAG_SEND_ZLP)
+ !(info-flags FLAG_MULTI_PACKET)) {
+ dev-padding_pkt = kzalloc(1, GFP_KERNEL);
+ if (!dev-padding_pkt)
+ goto out4;
+ }
+
status = register_netdev (net);
if (status)
- goto out4;
+ goto out5;
netif_info(dev, probe, dev-net,
register '%s' at usb-%s-%s, %s, %pM\n,
udev-dev.driver-name,
@@ -1699,6 +1712,8 @@ usbnet_probe (struct usb_interface *udev, const struct
usb_device_id *prod)
return 0;
+out5:
+ kfree(dev-padding_pkt);
out4:
usb_free_urb(dev-interrupt);
out3:
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index 9cb2fe8..e303eef 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -42,6 +42,7 @@ struct usbnet {
struct usb_host_endpoint *status;
unsignedmaxpacket;
struct timer_list delay;
+ const char *padding_pkt;
/* protocol/interface state */
struct net_device *net;
The code handling the frame padding was already unecessarily complex
IMHO, and this does not improve it...
Are you really sure that the one driver/device using this really need
the padding byte? If you could just make FLAG_SEND_ZLP part of the
condition for enabling can_dma_sg, then all this extra complexity would
be unnecessary. As the comment in front of the padding code says:
strictly conforming cdc-ether devices should expect the ZLP here
There shouldn't be any problems requiring this conformance as a
precondition for allowing SG. The requirements are already strict.
I also believe it would be nice to move the initialisation of
Re: Fix style in s3c-hsotg.c
Hi,
On Wed, Sep 18, 2013 at 11:20:27AM +0200, Pavel Machek wrote:
On Tue 2013-09-17 20:45:39, Felipe Balbi wrote:
On Wed, Sep 18, 2013 at 12:04:27AM +0200, Pavel Machek wrote:
On Tue 2013-09-17 10:42:30, Felipe Balbi wrote:
Hi,
On Mon, Sep 02, 2013 at 03:58:32PM +0200, Pavel Machek wrote:
Hi!
checkpatch.pl has some valid complaints about style in s3c-hsotg.c :
macro with if should be really enclosed in do {} while, and puts is
going to be slightly faster.
Here's suggested patch. I don't have the hardware, so it is completely
untested.
Signed-off-by: Pavel Machek, pa...@denx.de
this is not how you send a patch, please read
Documentation/SubmittingPatches
Have you considered possibility that this is how you nudge maintainer
into fix their coding style?
cute...
Seriously though, read that file, you're commit log has garbage in it
which shouldn't go to git history.
Run git log on SubmittingPatches.
Then, instead of telling me what to read, run checkpatch on your
files. You can either fix them yourself, or use my patch as a
basis. Note there's missing } or something, so it probably will not
compile, see the other mail. So you actually will have to modify that
patch. Stripping Hi! from it should not be that hard, neither should
be stripping note that patch is untested when you actually test
it. And as you are the maintainer, it is your job.
you misunderstand the work of maintainers. Our work is not to fix you
crappy patches. If we start allowing crappy patches, we'd be fixing your
nonsense forever and linux wouldn't move.
No, I'll not polish patch for hardware I don't have and have little
interest in. wanted to help you, but according to your first reply,
you do not really want help.
that's your call. Now how about you stop being such a baby and go fix
your mistakes to start with ? Just because I'm the maintainer of the
gadget framework, doesn't mean I'm the maintainer of s3c-hsotc.c file.
Maintainer != author too, btw.
Anyway, I got much better stuff to do than babysitting grown ups.
cheers
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH v2 1/1] usb: gadget: zero: Add flexible auto remote wakeup test method
On Wed, Aug 07, 2013 at 05:18:13AM +0800, Peter Chen wrote: In order to increase test coverage, we can change the interval between two remote wakeups every time, and the interval can be any user defined value. This change will no affect current behavior if the user does not use two introduced module paramters. s/paramters/parameters other than that, patch looks good. -- balbi signature.asc Description: Digital signature
Re: [PATCH 1/4] usb: musb: Call atomic_notifier_call_chain when status is changed
Hi! So will you do that? Or it is needed to resend this one line hunk again in new email again? new patch, new email Guys, WHY ARE YOU SO STUPID AND ARROGANT? Sorry but, need to copy full isolated patch/hunk from one mail to another is hassling. So what you want from me? Do all those non sense working only because yesterday you had bad day? Or what? ... Actually, there is need to be rude, because Felipe fails to act as maintainer. Instead of fixing bugs in his code, he bounces bugfix patches, points people to random READMEs and wastes everyones time. I don't know what are you talking about (if that happened in another thread then I need more context). Felipe is not bouncing any bugfix Take a look here: https://lkml.org/lkml/2013/9/17/286 I clearly state that patch can not be tested as required for proper submission, but offer patch anyway. I get irrelevant boilerplate on patch format. but just asked to split the patch in two since the patch was solving two separate issues so is way better to have it in two separate patches for the reasons I explained before. So, as far as I can tell Felipe did exactly what I would expect from a maintainer. He took the time to review the patches sent to him and I'd expect maintainer to, well, maintain code. It means actually fixing bugs in his code, when he's pointed at them. gave feedback. If the sender doesn't want to take his feedback into account and prefer to send pretty insulting emails instead that is his choice but I would say that is this not the greatest approach to get your code merged (to say the least). Clearly not. But Pali found bug in code Felipe should maintain. Instead of thank you for bug report, I applied this one line of your code to fix it, Pali got new patch, new email for his efforts. That is how you train dogs, not how you should treat kernel contributors. Now, it is possible that Felipe just has problems with english, as he called me piece of wood in https://lkml.org/lkml/2013/9/17/476 , but he appears more arogant than usual over email. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: dwc3: core: clarify usb-phy array binding
Hi,
On Wed, Aug 28, 2013 at 05:01:51PM +0100, Mark Rutland wrote:
So it's not physically possible for someone to just wire up a single phy
to the device, either USB2-only or USB3?
of course it is :-) In fact, TI has done it. But it causes a whole bunch
of other problems to support that sort of model. For one, in some
systems, a clock generated by the USB3 PHY is backfed into the IP and if
USB3 PHY isn't running, the dwc3 IP won't start.
That sounds like a mess. But unless I've misunderstood, that doesn't
well, it is :-)
sound like a general problem with having one phy, but rather an
integration issue on a specific system? Presumably in that case as long
as the phy was brought up before poking the rest of the IP, the unit
would function correctly.
right, but what 'brings up' the PHY is usb_phy_init(). If we don't add
usb3phy to DTS or skip usb3phy in case maximum-speed superspeed, then
we're screwed :-)
I even wrote a patch making USB3 PHY optional, but didn't push it
exactly because it broke some other systems and I can't guarantee users
won't mess up their DTS/pdata.
Does that mean that their dts or pdata are wrong at the moment, but
they're spared because the driver bails out due to a missing phy? If
their pdata's wrong, that should be fixable relatively easily, but if
the dt is wrong then I'm not sure how we can support those systems
sensibly. That sounds like an ideal candidate for a dt quirks
mechanism...
hmm, the idea of the patch was:
switch (maximum-speed) {
case SUPER:
grab_and_initialize_usb3_phy();
grab_and_initialize_usb2_phy();
break;
case HIGH:
case FULL:
case LOW:
grab_and_initialize_usb2_phy();
break;
}
now, imagine someone wants to run his dwc3 in highspeed mode, we
wouldn't initialize USB3 PHY which could cause the whole IP to break.
I tried poking around on device's registers to see if there was any way
to detect that the IP needs somethin back from USB3 PHY, but couldn't
find anything.
Since we can't know how the IP was integrated, it's best to leave it
alone and require NOP xceiv to be used.
You can describe the USB2-only case in the dt by only listing the first
phy (though the driver won't support it as it expects both to be
present), but it's impossible to describe that you've wired up a single
phy that's USB3 capable.
you might be right there...
Would it be possible to have something like (an optional) usb-phy-names?
If not present, we assume the current situation, if present then we use
it to figure out which phys are present. Maybe I'm missing something
that makes that impossible?
you're missing the point regarding the IP possibly needing something
back from the PHY (e.g. a clock which PHY generates).
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH 1/4] usb: musb: Call atomic_notifier_call_chain when status is changed
Hi! gave feedback. If the sender doesn't want to take his feedback into account and prefer to send pretty insulting emails instead that is his choice but I would say that is this not the greatest approach to get your code merged (to say the least). Clearly not. But Pali found bug in code Felipe should maintain. Instead of thank you for bug report, I applied this one line of your code to fix it, Pali got new patch, new email for his efforts. That is how you train dogs, not how you should treat kernel contributors. Now, it is possible that Felipe just has problems with english, as he called me piece of wood in https://lkml.org/lkml/2013/9/17/476 , but he appears more arogant than usual over email. Actually he called me piece of wood with garbage in it. I guess I have right to be offended. I'm baby in the next email. Hmm. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] usb: musb: Call atomic_notifier_call_chain when status is changed
On Wed, Sep 18, 2013 at 4:22 PM, Pavel Machek pa...@ucw.cz wrote: Hi! So will you do that? Or it is needed to resend this one line hunk again in new email again? new patch, new email Guys, WHY ARE YOU SO STUPID AND ARROGANT? Sorry but, need to copy full isolated patch/hunk from one mail to another is hassling. So what you want from me? Do all those non sense working only because yesterday you had bad day? Or what? ... Actually, there is need to be rude, because Felipe fails to act as maintainer. Instead of fixing bugs in his code, he bounces bugfix patches, points people to random READMEs and wastes everyones time. I don't know what are you talking about (if that happened in another thread then I need more context). Felipe is not bouncing any bugfix Take a look here: https://lkml.org/lkml/2013/9/17/286 I clearly state that patch can not be tested as required for proper submission, but offer patch anyway. I get irrelevant boilerplate on patch format. Felipe didn't complain about you not being to be able to test the patch (most of the times compile tested if enough) what he said was: Seriously though, read that file, you're commit log has garbage in it which shouldn't go to git history. which is totally true, if you want to comment things that don't have to go to the backlog you can't comment between the --- after your s-o-b and before the first diff. That's were you should puts comments like Hi! or Here's suggested patch. I don't have the hardware, so it is completely untested. but just asked to split the patch in two since the patch was solving two separate issues so is way better to have it in two separate patches for the reasons I explained before. So, as far as I can tell Felipe did exactly what I would expect from a maintainer. He took the time to review the patches sent to him and I'd expect maintainer to, well, maintain code. It means actually fixing bugs in his code, when he's pointed at them. gave feedback. If the sender doesn't want to take his feedback into account and prefer to send pretty insulting emails instead that is his choice but I would say that is this not the greatest approach to get your code merged (to say the least). Clearly not. But Pali found bug in code Felipe should maintain. Instead of thank you for bug report, I applied this one line of your code to fix it, Pali got new patch, new email for his efforts. That is how you train dogs, not how you should treat kernel contributors. No, you misunderstood the role of the maintainers. Maintainers should be custodian of a part of the kernel but not responsible for every single line of code on their sub-systems. If a piece of code is buggy then the people using and that take care of that should be fixing and maintainers should review and suggest improvements to the patches. As long as a piece of code keep compiling then it is harmless even if is buggy and nobody cares about it. If it is so broken that it doesn't even compile then the maintainer can decide to just drop it since no one else seems to care about it. If someone finds a bug on a piece of code is because that people care about that functionality. Maintainers are really busy people and can jump and fix any random bug that someone finds on a piece of code just because it is the subsystem they maintainer neither they have to blindly merge any crappy patch just because they don't have time (or interest) in fixing a reported bug on a piece of code. Now, it is possible that Felipe just has problems with english, as he called me piece of wood in https://lkml.org/lkml/2013/9/17/476 , but he appears more arogant than usual over email. Clearly he meant your commit log has garbage instead of you're, that's a typo. Pavel But neither Felipe needs someone to defend him nor I have time to keep arguing with you. Have nice day! Javier -- To unsubscribe from this list: send the line unsubscribe 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] USBNET: fix handling padding packet
On Wed, Sep 18, 2013 at 9:59 PM, Bjørn Mork bj...@mork.no wrote:
Ming Lei ming@canonical.com writes:
Commit 638c5115a7949(USBNET: support DMA SG) introduces DMA SG
if the usb host controller is capable of building packet from
discontinuous buffers, but missed handling padding packet when
building DMA SG.
This patch attachs the pre-allocated padding packet at the
end of the sg list, so padding packet can be sent to device
if drivers require that.
Reported-by: David Laight david.lai...@aculab.com
Cc: Oliver Neukum oli...@neukum.org
Signed-off-by: Ming Lei ming@canonical.com
---
drivers/net/usb/usbnet.c | 27 +--
include/linux/usb/usbnet.h |1 +
2 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 7b331e6..4a9bed3 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1241,7 +1241,9 @@ static int build_dma_sg(const struct sk_buff *skb,
struct urb *urb)
if (num_sgs == 1)
return 0;
- urb-sg = kmalloc(num_sgs * sizeof(struct scatterlist), GFP_ATOMIC);
+ /* reserve one for zero packet */
+ urb-sg = kmalloc((num_sgs + 1) * sizeof(struct scatterlist),
+ GFP_ATOMIC);
if (!urb-sg)
return -ENOMEM;
@@ -1305,7 +1307,7 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
if (build_dma_sg(skb, urb) 0)
goto drop;
}
- entry-length = length = urb-transfer_buffer_length;
+ length = urb-transfer_buffer_length;
/* don't assume the hardware handles USB_ZERO_PACKET
* NOTE: strictly conforming cdc-ether devices should expect
@@ -1317,15 +1319,18 @@ netdev_tx_t usbnet_start_xmit (struct sk_buff *skb,
if (length % dev-maxpacket == 0) {
if (!(info-flags FLAG_SEND_ZLP)) {
if (!(info-flags FLAG_MULTI_PACKET)) {
- urb-transfer_buffer_length++;
- if (skb_tailroom(skb)) {
+ length++;
+ if (skb_tailroom(skb) !dev-can_dma_sg) {
skb-data[skb-len] = 0;
__skb_put(skb, 1);
- }
+ } else if (dev-can_dma_sg)
+ sg_set_buf(urb-sg[urb-num_sgs++],
+ dev-padding_pkt, 1);
}
} else
urb-transfer_flags |= URB_ZERO_PACKET;
}
+ entry-length = urb-transfer_buffer_length = length;
spin_lock_irqsave(dev-txq.lock, flags);
retval = usb_autopm_get_interface_async(dev-intf);
@@ -1509,6 +1514,7 @@ void usbnet_disconnect (struct usb_interface *intf)
usb_kill_urb(dev-interrupt);
usb_free_urb(dev-interrupt);
+ kfree(dev-padding_pkt);
free_netdev(net);
}
@@ -1679,9 +1685,16 @@ usbnet_probe (struct usb_interface *udev, const
struct usb_device_id *prod)
/* initialize max rx_qlen and tx_qlen */
usbnet_update_max_qlen(dev);
+ if (dev-can_dma_sg !(info-flags FLAG_SEND_ZLP)
+ !(info-flags FLAG_MULTI_PACKET)) {
+ dev-padding_pkt = kzalloc(1, GFP_KERNEL);
+ if (!dev-padding_pkt)
+ goto out4;
+ }
+
status = register_netdev (net);
if (status)
- goto out4;
+ goto out5;
netif_info(dev, probe, dev-net,
register '%s' at usb-%s-%s, %s, %pM\n,
udev-dev.driver-name,
@@ -1699,6 +1712,8 @@ usbnet_probe (struct usb_interface *udev, const struct
usb_device_id *prod)
return 0;
+out5:
+ kfree(dev-padding_pkt);
out4:
usb_free_urb(dev-interrupt);
out3:
diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
index 9cb2fe8..e303eef 100644
--- a/include/linux/usb/usbnet.h
+++ b/include/linux/usb/usbnet.h
@@ -42,6 +42,7 @@ struct usbnet {
struct usb_host_endpoint *status;
unsignedmaxpacket;
struct timer_list delay;
+ const char *padding_pkt;
/* protocol/interface state */
struct net_device *net;
The code handling the frame padding was already unecessarily complex
IMHO, and this does not improve it...
Are you really sure that the one driver/device using this really need
the padding byte? If you could just make FLAG_SEND_ZLP part of the
condition for enabling can_dma_sg, then all this extra complexity would
be unnecessary. As the comment in front of the padding code says:
At least for the effected driver of ax88179, the padding packet is needed
since the driver does set the padding flag in the header, see
ax88179_tx_fixup().
strictly conforming cdc-ether devices should expect
RE: [PATCH] USBNET: fix handling padding packet
I also believe it would be nice to move the initialisation of can_dma_sg
away from the minidriver and into usbnet_probe. It's confusing that
this field is used uninitialized (well, defaulting to zero) in all but
one minidriver. It would much nicer if the logic was more like
usbnet_probe:
if (...)
dev-can_dma_sg = 1;
minidriver_bind:
if (dev-can_dma_sg) {
dev-net-features |= NETIF_F_SG | NETIF_F_TSO;
dev-net-hw_features |= NETIF_F_SG | NETIF_F_TSO;
}
Actually it would probably be nicer if the minidriver set
a flag to indicate that it could support fragmented skb
(a lot can't because of the way they add trailers)
and also provided the length of the header it will add.
The usbnet code could then allocate the header space.
If scatter-gather dma is available (a host feature) then
the header can be allocated outside the skb data area
to avoid having to copy the entire skb data.
The check for ZLP avoidance could then be done once only
(not sure how the info would be passed to teh minidriver apart
from adding more additional parameters to teh tx_fixup function).
I did a quick scan of the minidrivers, some of them don't
seem to have the correct checks for fragmented skb (etc).
Most of them only add a header.
David
Re: [PATCH 1/4] usb: musb: Call atomic_notifier_call_chain when status is changed
On Wednesday 18 September 2013 15:57:13 Javier Martinez Canillas wrote: to split the patch in two since the patch was solving two separate issues My patch does not solving *two* issues. It is *one* regression and both parts of patch are needed for fixing it. Read commit message again. It does not make sense to split patch fixing kernel regression into more one line patches - or please clarify why. -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: [PATCH] USBNET: fix handling padding packet
From: Bjørn Mork bj...@mork.no Date: Wed, 18 Sep 2013 17:52:42 +0200 Ming Lei ming@canonical.com writes: There is no reason to forbid DMA SG for one driver which requires padding, right? Yes there is: Added complexity for everybody, based on a combination of features which just does not make any sense. No modern device should need the padding. No old device will be able to use the SG feature as implemented. You only enable it on USB3, don't you? If this feature is restricted to USB3 capable devices, then it most certainly can be restricted to ZLP capable devices with absolutely no difference in the resulting set of supported devices. I completely agree. -- To unsubscribe from this list: send the line unsubscribe 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/4] usb: musb: Call atomic_notifier_call_chain when status is changed
Hi, On Wed, Sep 18, 2013 at 04:35:37PM +0200, Pavel Machek wrote: Hi! gave feedback. If the sender doesn't want to take his feedback into account and prefer to send pretty insulting emails instead that is his choice but I would say that is this not the greatest approach to get your code merged (to say the least). Clearly not. But Pali found bug in code Felipe should maintain. Instead of thank you for bug report, I applied this one line of your code to fix it, Pali got new patch, new email for his efforts. That is how you train dogs, not how you should treat kernel contributors. Now, it is possible that Felipe just has problems with english, as he called me piece of wood in https://lkml.org/lkml/2013/9/17/476 , but he appears more arogant than usual over email. Actually he called me piece of wood with garbage in it. I guess I have right to be offended. I'm baby in the next email. Hmm. what a baby. Grow up dude, just grow up. I said your commit log has garbage which shouldn't be there (there was a typo you're instead of your). -- balbi signature.asc Description: Digital signature
Re: [PATCH] USBNET: fix handling padding packet
Ming Lei ming@canonical.com writes:
Are you really sure that the one driver/device using this really need
the padding byte? If you could just make FLAG_SEND_ZLP part of the
condition for enabling can_dma_sg, then all this extra complexity would
be unnecessary. As the comment in front of the padding code says:
At least for the effected driver of ax88179, the padding packet is needed
since the driver does set the padding flag in the header, see
ax88179_tx_fixup().
Yes, I noticed that the driver doesn't set the flag. I just didn't put
too much into that. I don't think that necessarily means that the
padding is needed. It probably just means that the driver worked with
the default padding enabled, so the author left it there.
This flag should really have been inverted. ZLP should be the default
for all new usbnet drivers.
Why don't you test it on the device you tested the SG patch with? I am
pretty sure it works just fine using proper ZLP transfer termination.
strictly conforming cdc-ether devices should expect the ZLP here
There shouldn't be any problems requiring this conformance as a
precondition for allowing SG. The requirements are already strict.
There is no reason to forbid DMA SG for one driver which requires
padding, right?
Yes there is: Added complexity for everybody, based on a combination of
features which just does not make any sense.
No modern device should need the padding. No old device will be able to
use the SG feature as implemented. You only enable it on USB3, don't
you? If this feature is restricted to USB3 capable devices, then it most
certainly can be restricted to ZLP capable devices with absolutely no
difference in the resulting set of supported devices.
Anyway, if you want to keep the padding for SG then maybe this will work
and allow you to drop the extra struct usbnet field and allocation:
if (skb_tailroom(skb) !dev-can_dma_sg) {
skb-data[skb-len] = 0;
__skb_put(skb, 1);
} else if (dev-can_dma_sg) {
sg_set_buf(urb-sg[urb-num_sgs++], skb-data,
1);
}
I.e. cheat and use the skb-data buffer twice, if that is allowed? The
actual value of the padding byte should not matter, I believe?
Bjørn
--
To unsubscribe from this list: send the line unsubscribe 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/4] usb: musb: Call atomic_notifier_call_chain when status is changed
On Wed, Sep 18, 2013 at 05:56:12PM +0200, Pali Rohár wrote: On Wednesday 18 September 2013 15:57:13 Javier Martinez Canillas wrote: to split the patch in two since the patch was solving two separate issues My patch does not solving *two* issues. It is *one* regression and both parts of patch are needed for fixing it. Read commit message again. It does not make sense to split patch fixing kernel regression into more one line patches - or please clarify why. issue 1) twl4030-usb.c doesn't initialize atomic notifier head issue 2) musb doesn't call notifier chain do you need a drawing ? -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: dwc3: core: clarify usb-phy array binding
On Wed, Sep 18, 2013 at 03:21:18PM +0100, Felipe Balbi wrote:
Hi,
On Wed, Aug 28, 2013 at 05:01:51PM +0100, Mark Rutland wrote:
So it's not physically possible for someone to just wire up a single phy
to the device, either USB2-only or USB3?
of course it is :-) In fact, TI has done it. But it causes a whole bunch
of other problems to support that sort of model. For one, in some
systems, a clock generated by the USB3 PHY is backfed into the IP and if
USB3 PHY isn't running, the dwc3 IP won't start.
That sounds like a mess. But unless I've misunderstood, that doesn't
well, it is :-)
sound like a general problem with having one phy, but rather an
integration issue on a specific system? Presumably in that case as long
as the phy was brought up before poking the rest of the IP, the unit
would function correctly.
right, but what 'brings up' the PHY is usb_phy_init(). If we don't add
usb3phy to DTS or skip usb3phy in case maximum-speed superspeed, then
we're screwed :-)
:(
I even wrote a patch making USB3 PHY optional, but didn't push it
exactly because it broke some other systems and I can't guarantee users
won't mess up their DTS/pdata.
Does that mean that their dts or pdata are wrong at the moment, but
they're spared because the driver bails out due to a missing phy? If
their pdata's wrong, that should be fixable relatively easily, but if
the dt is wrong then I'm not sure how we can support those systems
sensibly. That sounds like an ideal candidate for a dt quirks
mechanism...
hmm, the idea of the patch was:
switch (maximum-speed) {
case SUPER:
grab_and_initialize_usb3_phy();
grab_and_initialize_usb2_phy();
break;
case HIGH:
case FULL:
case LOW:
grab_and_initialize_usb2_phy();
break;
}
now, imagine someone wants to run his dwc3 in highspeed mode, we
wouldn't initialize USB3 PHY which could cause the whole IP to break.
When you say wants to run it in highspeed mode, you mean that they want
this configured at run-time, or they deliberately omit a phy when
describing the hardware in the DT?
For the former I appreciate that it's a problem, but for the latter I'd
argue that's their fault. As far as I can see we initialise both PHYs in
the probe path and never uninitialise them, so the only problem would be
if someone's trying to be too clever. As we never check the return value
of usb_phy_init, they can still attempt to work around our best
efforts...
I appreciate that we should not break existing DTs. More on that below.
I tried poking around on device's registers to see if there was any way
to detect that the IP needs somethin back from USB3 PHY, but couldn't
find anything.
Since we can't know how the IP was integrated, it's best to leave it
alone and require NOP xceiv to be used.
Agreed for the existing systems, but I still think we can work around
this for new DTs...
You can describe the USB2-only case in the dt by only listing the first
phy (though the driver won't support it as it expects both to be
present), but it's impossible to describe that you've wired up a single
phy that's USB3 capable.
you might be right there...
Would it be possible to have something like (an optional) usb-phy-names?
If not present, we assume the current situation, if present then we use
it to figure out which phys are present. Maybe I'm missing something
that makes that impossible?
you're missing the point regarding the IP possibly needing something
back from the PHY (e.g. a clock which PHY generates).
I'm not sure why that detracts from the usb-phy-names idea -- if not
present, we stick with the current behaviour and require both PHYs or
fail early. No existing dts suddently explode, though none gain new
functionality either.
If someone's explicitly placed usb-phy-names, we know that they're
up-to-date on the lastest binding, something like:
- usb-phy: A list of phandles to PHYs. If usb-phy-names is not
present, the USB2/HS PHY should be first, followed by the
USB3/SS PHY. If usb-phy-names is present, it defines the
order of PHYs.
- usb-phy-names: The names of PHYs described in the usb-phy property.
Valid values are usb2 and usb3. Should be used iff
a subset of PHYs are connected.
Compatibility note: The DWC3 IP can be integrated in
such a way as to require outputs from particular PHYs
for *any* level of operation. This cannot be detected
by the OS, and on such systems all required PHYs must
be described.
Given that, if they list fewer PHYs than present and their system really
requires a particular PHY, we can quite happily allow their system to
explode in the knowledge it's their fault, not ours :)
If
Re: [PATCH 1/4] usb: musb: Call atomic_notifier_call_chain when status is changed
On Wednesday 18 September 2013 18:36:49 Felipe Balbi wrote: On Wed, Sep 18, 2013 at 05:56:12PM +0200, Pali Rohár wrote: On Wednesday 18 September 2013 15:57:13 Javier Martinez Canillas wrote: to split the patch in two since the patch was solving two separate issues My patch does not solving *two* issues. It is *one* regression and both parts of patch are needed for fixing it. Read commit message again. It does not make sense to split patch fixing kernel regression into more one line patches - or please clarify why. issue 1) twl4030-usb.c doesn't initialize atomic notifier head issue 2) musb doesn't call notifier chain do you need a drawing ? Regression 1) power supply drivers not working since v3.5 Look at firsts emails. Do you really have problem to do what *you* need with 1 line patch which I sent? Or what what you want? -- Pali Rohár pali.ro...@gmail.com signature.asc Description: This is a digitally signed message part.
Re: [PATCH] usb: gadget: Add UDC driver for Aeroflex Gaisler GRUSBDC
Hi,
On Mon, Aug 12, 2013 at 04:05:10PM +0200, Andreas Larsson wrote:
diff --git a/drivers/usb/gadget/gr_udc.c b/drivers/usb/gadget/gr_udc.c
new file mode 100644
index 000..37a6c08
--- /dev/null
+++ b/drivers/usb/gadget/gr_udc.c
@@ -0,0 +1,2268 @@
+/*
+ * USB Peripheral Controller driver for Aeroflex Gaisler GRUSBDC.
+ *
+ * 2013 (c) Aeroflex Gaisler AB
+ *
+ * This driver supports GRUSBDC USB Device Controller cores available in the
+ * GRLIB VHDL IP core library.
+ *
+ * Full documentation of the GRUSBDC core can be found here:
+ * http://www.gaisler.com/products/grlib/grip.pdf
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ * Contributors:
+ * - Andreas Larsson andr...@gaisler.com
+ * - Marko Isomaki
+ */
+
+/*
+ * A GRUSBDC core can have up to 16 IN endpoints and 16 OUT endpoints each
+ * individually configurable to any of the four USB transfer types. This
driver
+ * only supports cores in DMA mode.
+ */
+
+#include linux/kernel.h
+#include linux/module.h
+#include linux/slab.h
+#include linux/spinlock.h
+#include linux/errno.h
+#include linux/init.h
+#include linux/list.h
+#include linux/interrupt.h
+#include linux/device.h
+#include linux/usb/ch9.h
+#include linux/usb/gadget.h
+#include linux/dma-mapping.h
+#include linux/dmapool.h
+#include linux/debugfs.h
+#include linux/seq_file.h
+
+#include asm/byteorder.h
+#include asm/irq.h
linux/irq.h
+#include linux/of_platform.h
+#include linux/of_irq.h
+#include linux/of_address.h
+
+/* #define VERBOSE_DEBUG */
we don't want this, we want verbose debug to be selectable on Kconfig,
which already is ;-)
+#include gr_udc.h
+
+#define DRIVER_NAME gr_udc
+#define DRIVER_DESC Aeroflex Gaisler GRUSBDC USB Peripheral
Controller
+
+static const char driver_name[] = DRIVER_NAME;
+static const char driver_desc[] = DRIVER_DESC;
+
+#define gr_read32(x) (ioread32be((x)))
+#define gr_write32(x, v) (iowrite32be((v), (x)))
+
+/* USB speed and corresponding string calculated from status register value
*/
+#define GR_SPEED(status) \
+ ((status GR_STATUS_SP) ? USB_SPEED_FULL : USB_SPEED_HIGH)
+#define GR_SPEED_STR(status) usb_speed_string(GR_SPEED(status))
+
+/* Size of hardware buffer calculated from epctrl register value */
+#define GR_BUFFER_SIZE(epctrl) \
+ epctrl) GR_EPCTRL_BUFSZ_MASK) GR_EPCTRL_BUFSZ_POS) * \
+ GR_EPCTRL_BUFSZ_SCALER)
+
+/* -- */
+/* Debug printout functionality */
+
+static const char * const gr_modestring[] = {control, iso, bulk,
int};
+
+static const char *gr_ep0state_string(enum gr_ep0state state)
+{
+ static const char *const names[] = {
+ [GR_EP0_DISCONNECT] = disconnect,
+ [GR_EP0_SETUP] = setup,
+ [GR_EP0_IDATA] = idata,
+ [GR_EP0_ODATA] = odata,
+ [GR_EP0_ISTATUS] = istatus,
+ [GR_EP0_OSTATUS] = ostatus,
+ [GR_EP0_STALL] = stall,
+ [GR_EP0_SUSPEND] = suspend,
+ };
+
+ if (state 0 || state = ARRAY_SIZE(names))
+ return UNKNOWN;
+
+ return names[state];
+}
+
+#ifdef VERBOSE_DEBUG
+
+#define BPRINTF(buf, left, fmt, args...) \
+ do {\
+ int ret = snprintf(buf, left, fmt, ## args);\
+ buf += ret; \
+ left -= ret;\
+ } while (0)
nack, use dev_vdbg() instead.
+static void gr_dbgprint_request(const char *str, struct gr_ep *ep,
+ struct gr_request *req)
+{
+ char buffer[100];
NAK^1000
use kernel facilities instead. printk() and all its friends already
print to a ring buffer.
+ u8 *data = (u8 *)req-req.buf;
you don't need to cast void pointers
+static void gr_finish_request(struct gr_ep *ep, struct gr_request *req,
+ int status)
+{
+ struct gr_udc *dev;
+
+ list_del_init(req-queue);
+
+ if (likely(req-req.status == -EINPROGRESS))
+ req-req.status = status;
+ else
+ status = req-req.status;
+
+ dev = ep-dev;
+ usb_gadget_unmap_request(dev-gadget, req-req, ep-is_in);
+ gr_free_dma_desc_chain(dev, req);
+
+ if (ep-is_in) /* For OUT, actual gets updated by the work handler */
+ req-req.actual = req-req.length;
+
+ if (!status) {
+ if (ep-is_in)
+ gr_dbgprint_request(SENT, ep, req);
+ else
+ gr_dbgprint_request(RECV, ep, req);
+
Re: PLX USB3380 (was: usbtest and gadget zero with PLX 3380)
On Wed, Sep 18, 2013 at 09:31:55AM +0200, Steffen Sledz wrote: On 22.04.2012 17:52, Thomas Knauth wrote: I am trying to develop a driver for the net2280 successor (USB 3380 from PLX)... What is the current state of this project? Is anyone still working on a driver for the PLX USB3380? For registered users PLXTech itself offers a driver download[1] which is a snapshot of Linux kernel version: 2.6.38.6-26.rc1.fc15.x86_64 (Linux OS used: Fedora 15). The code itself contains MODULE_LICENSE(GPL); So it sould be a legal base for the mainline. [1] http://www.plxtech.com/products/usbcontrollers/usb3380 (Linux SDK 1.3.1) I offered to write the driver for them if they'd just give me a few boards to test against, didn't get a reply :-p -- balbi signature.asc Description: Digital signature
Re: [PATCH] USBNET: fix handling padding packet
On Wed, Sep 18, 2013 at 11:52 PM, Bjørn Mork bj...@mork.no wrote:
Ming Lei ming@canonical.com writes:
Are you really sure that the one driver/device using this really need
the padding byte? If you could just make FLAG_SEND_ZLP part of the
condition for enabling can_dma_sg, then all this extra complexity would
be unnecessary. As the comment in front of the padding code says:
At least for the effected driver of ax88179, the padding packet is needed
since the driver does set the padding flag in the header, see
ax88179_tx_fixup().
Yes, I noticed that the driver doesn't set the flag. I just didn't put
too much into that. I don't think that necessarily means that the
padding is needed. It probably just means that the driver worked with
the default padding enabled, so the author left it there.
This flag should really have been inverted. ZLP should be the default
for all new usbnet drivers.
In theory, it is, but who knows the reality.
Why don't you test it on the device you tested the SG patch with? I am
pretty sure it works just fine using proper ZLP transfer termination.
I should have planned to test it, but didn't know how to build TCP TSO
to make the whole frame length plus 8 dividable by 1024.
Could you or other guys share how to build such packet so that I can
do the test?
Once we can confirm the padding isn't needed, we can remove the
padding flag. But if the padding flag is still there, this patch should be
dropped.
strictly conforming cdc-ether devices should expect the ZLP here
There shouldn't be any problems requiring this conformance as a
precondition for allowing SG. The requirements are already strict.
There is no reason to forbid DMA SG for one driver which requires
padding, right?
Yes there is: Added complexity for everybody, based on a combination of
features which just does not make any sense.
No modern device should need the padding. No old device will be able to
use the SG feature as implemented. You only enable it on USB3, don't
you? If this feature is restricted to USB3 capable devices, then it most
certainly can be restricted to ZLP capable devices with absolutely no
difference in the resulting set of supported devices.
See above, if we can prove that padding isn't needed, we can remove
the padding, then the patch can be dropped. If we can't, the patch is needed.
Anyway, if you want to keep the padding for SG then maybe this will work
and allow you to drop the extra struct usbnet field and allocation:
if (skb_tailroom(skb) !dev-can_dma_sg) {
skb-data[skb-len] = 0;
__skb_put(skb, 1);
} else if (dev-can_dma_sg) {
sg_set_buf(urb-sg[urb-num_sgs++], skb-data,
1);
}
I.e. cheat and use the skb-data buffer twice, if that is allowed? The
actual value of the padding byte should not matter, I believe?
If so, we can remove the assignment of zero to skb-data[skb-len],
but probably it might cause regression, that is why I wouldn't like to
do that. Also looks introducing one extra dev-padding_frame doesn't
cause much complexity.
Thanks
--
Ming Lei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH usb 1/2] usb: musb: Add missing ATOMIC_INIT_NOTIFIER_HEAD
twl-phy.notifier is not initalized Signed-off-by: Pali Rohár pali.ro...@gmail.com diff --git a/drivers/usb/phy/phy-twl4030-usb.c b/drivers/usb/phy/phy-twl4030-usb.c index 8f78d2d..efe6155 100644 --- a/drivers/usb/phy/phy-twl4030-usb.c +++ b/drivers/usb/phy/phy-twl4030-usb.c @@ -705,6 +705,8 @@ static int twl4030_usb_probe(struct platform_device *pdev) if (device_create_file(pdev-dev, dev_attr_vbus)) dev_warn(pdev-dev, could not create sysfs file\n); + ATOMIC_INIT_NOTIFIER_HEAD(twl-phy.notifier); + /* Our job is to use irqs and status from the power module * to keep the transceiver disabled when nothing's connected. * signature.asc Description: This is a digitally signed message part.
Re: [PATCH 1/4] usb: musb: Call atomic_notifier_call_chain when status is changed
On Wed, Sep 18, 2013 at 06:43:49PM +0200, Pali Rohár wrote: On Wednesday 18 September 2013 18:36:49 Felipe Balbi wrote: On Wed, Sep 18, 2013 at 05:56:12PM +0200, Pali Rohár wrote: On Wednesday 18 September 2013 15:57:13 Javier Martinez Canillas wrote: to split the patch in two since the patch was solving two separate issues My patch does not solving *two* issues. It is *one* regression and both parts of patch are needed for fixing it. Read commit message again. It does not make sense to split patch fixing kernel regression into more one line patches - or please clarify why. issue 1) twl4030-usb.c doesn't initialize atomic notifier head issue 2) musb doesn't call notifier chain do you need a drawing ? Regression 1) power supply drivers not working since v3.5 Look at firsts emails. Do you really have problem to do what *you* need with 1 line patch which I sent? Or what what you want? if you had already resent patches the way I requested, they'd already be applied. Instead you chose to troll the people who are trying to helping out. You have two issues being fixed in the same patch and that's not acceptable. -- balbi signature.asc Description: Digital signature
Re: [PATCH] usb: dwc3: core: clarify usb-phy array binding
Hi,
On Wed, Sep 18, 2013 at 05:46:10PM +0100, Mark Rutland wrote:
I even wrote a patch making USB3 PHY optional, but didn't push it
exactly because it broke some other systems and I can't guarantee users
won't mess up their DTS/pdata.
Does that mean that their dts or pdata are wrong at the moment, but
they're spared because the driver bails out due to a missing phy? If
their pdata's wrong, that should be fixable relatively easily, but if
the dt is wrong then I'm not sure how we can support those systems
sensibly. That sounds like an ideal candidate for a dt quirks
mechanism...
hmm, the idea of the patch was:
switch (maximum-speed) {
case SUPER:
grab_and_initialize_usb3_phy();
grab_and_initialize_usb2_phy();
break;
case HIGH:
case FULL:
case LOW:
grab_and_initialize_usb2_phy();
break;
}
now, imagine someone wants to run his dwc3 in highspeed mode, we
wouldn't initialize USB3 PHY which could cause the whole IP to break.
When you say wants to run it in highspeed mode, you mean that they want
this configured at run-time, or they deliberately omit a phy when
describing the hardware in the DT?
it doesn't really matter, I guess. TI's DRA7xx platforms, for instance,
don't even provide a USB3 PHY and now way to connect one (since PIPE3
interface isn't brought out of the chip).
Granted the problem is bigger in superspeed capable integration where we
want to run at highspeed for e.g. test purposes.
For the former I appreciate that it's a problem, but for the latter I'd
argue that's their fault. As far as I can see we initialise both PHYs in
true, we can look at it that way.
the probe path and never uninitialise them, so the only problem would be
if someone's trying to be too clever. As we never check the return value
of usb_phy_init, they can still attempt to work around our best
efforts...
I appreciate that we should not break existing DTs. More on that below.
k
I tried poking around on device's registers to see if there was any way
to detect that the IP needs somethin back from USB3 PHY, but couldn't
find anything.
Since we can't know how the IP was integrated, it's best to leave it
alone and require NOP xceiv to be used.
Agreed for the existing systems, but I still think we can work around
this for new DTs...
perhaps, but wouldn't that be a quirk ? If it's a quirk, perhaps we'd be
better off adding a quirk flag to skip USB?PHY in some platforms. Then
we could:
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 474162e..5d39e0e 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -387,16 +387,29 @@ static int dwc3_probe(struct platform_device *pdev)
if (node) {
dwc-maximum_speed = of_usb_get_maximum_speed(node);
- dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev, usb-phy, 0);
- dwc-usb3_phy = devm_usb_get_phy_by_phandle(dev, usb-phy, 1);
+ if (of_device_is_compatible(node, ti,dra7xxx-dwc3))
+ dwc-quirks |= DWC3_SKIP_USB3PHY;
+
+ if (!dwc-quirks DWC3_SKIP_USB2_PHY)
+ dwc-usb2_phy = devm_usb_get_phy_by_phandle(dev,
+ usb-phy, 0);
+
+ if (!dwc-quirks DWC3_SKIP_USB3_PHY)
+ dwc-usb3_phy = devm_usb_get_phy_by_phandle(dev,
+ usb-phy, 1);
dwc-needs_fifo_resize = of_property_read_bool(node,
tx-fifo-resize);
dwc-dr_mode = of_usb_get_dr_mode(node);
+
} else if (pdata) {
dwc-maximum_speed = pdata-maximum_speed;
+ dwc-quirks = pdata-quirks;
- dwc-usb2_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB2);
- dwc-usb3_phy = devm_usb_get_phy(dev, USB_PHY_TYPE_USB3);
+ if (!dwc-quirks DWC3_SKIP_USB2_PHY)
+ dwc-usb2_phy = devm_usb_get_phy(dev,
USB_PHY_TYPE_USB2);
+
+ if (!dwc-quirks DWC3_SKIP_USB3_PHY)
+ dwc-usb3_phy = devm_usb_get_phy(dev,
USB_PHY_TYPE_USB3);
dwc-needs_fifo_resize = pdata-tx_fifo_resize;
dwc-dr_mode = pdata-dr_mode;
@@ -732,6 +745,9 @@ static const struct dev_pm_ops dwc3_dev_pm_ops = {
#ifdef CONFIG_OF
static const struct of_device_id of_dwc3_match[] = {
{
+ .compatible = ti,dra7xxx-dwc3,
+ },
+ {
.compatible = snps,dwc3
},
{
You can describe the USB2-only case in the dt by only listing the
first
phy (though the driver won't support it as it expects both to be
present), but it's impossible to describe that you've wired up a
single
phy that's USB3 capable.
you might be right there...
Would it be possible to have something like (an
[PATCH usb 2/2] usb: musb: Call atomic_notifier_call_chain when status is changed
More power supply drivers depends on vbus events and without it they not working. Power supply drivers using usb_register_notifier, so to deliver events it is needed to call atomic_notifier_call_chain. So without atomic notifier power supply driver isp1704 not retrieving vbus status and reporting bogus values to userspace and also to board platform data functions. Without proper data charger drivers trying to charge battery also when charger is disconnected or do not start charging when wallcharger connects. Atomic notifier in musb driver was used before v3.5 and was replaced with omap mailbox. This patch adding atomic_notifier_call_chain call from function omap_musb_set_mailbox. Signed-off-by: Pali Rohár pali.ro...@gmail.com diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c index f44e8b5..5c40252 100644 --- a/drivers/usb/musb/omap2430.c +++ b/drivers/usb/musb/omap2430.c @@ -305,6 +305,9 @@ static void omap_musb_set_mailbox(struct omap2430_glue *glue) default: dev_dbg(dev, ID float\n); } + + atomic_notifier_call_chain(musb-xceiv-notifier, + musb-xceiv-last_event, NULL); } signature.asc Description: This is a digitally signed message part.
Re: [PATCH] USBNET: fix handling padding packet
On Wed, 2013-09-18 at 17:52 +0200, Bjørn Mork wrote:
No modern device should need the padding. No old device will be able to
use the SG feature as implemented. You only enable it on USB3, don't
On XHCI.
you? If this feature is restricted to USB3 capable devices, then it most
certainly can be restricted to ZLP capable devices with absolutely no
difference in the resulting set of supported devices.
No, USB 3.0 uses no companion controllers, so you can have devices
of any speed connected to it.
Anyway, if you want to keep the padding for SG then maybe this will work
and allow you to drop the extra struct usbnet field and allocation:
if (skb_tailroom(skb) !dev-can_dma_sg) {
skb-data[skb-len] = 0;
__skb_put(skb, 1);
} else if (dev-can_dma_sg) {
sg_set_buf(urb-sg[urb-num_sgs++], skb-data,
1);
}
I.e. cheat and use the skb-data buffer twice, if that is allowed? The
actual value of the padding byte should not matter, I believe?
That makes me immediately suspect a violation of the DMA rules.
Regards
Oliver
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USBNET: fix handling padding packet
Ming Lei ming@canonical.com writes: On Wed, Sep 18, 2013 at 11:52 PM, Bjørn Mork bj...@mork.no wrote: Why don't you test it on the device you tested the SG patch with? I am pretty sure it works just fine using proper ZLP transfer termination. I should have planned to test it, but didn't know how to build TCP TSO to make the whole frame length plus 8 dividable by 1024. You could test this without enabling TSO and just send IP packets of the appriopriate size, taking any additional framing into account Bjørn -- To unsubscribe from this list: send the line unsubscribe 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] USBNET: fix handling padding packet
Oliver Neukum oneu...@suse.de wrote:
On Wed, 2013-09-18 at 17:52 +0200, Bjørn Mork wrote:
No modern device should need the padding. No old device will be able
to
use the SG feature as implemented. You only enable it on USB3, don't
On XHCI.
you? If this feature is restricted to USB3 capable devices, then it
most
certainly can be restricted to ZLP capable devices with absolutely no
difference in the resulting set of supported devices.
No, USB 3.0 uses no companion controllers, so you can have devices
of any speed connected to it.
Ah, right. I don't own such modern hardware, but I should have known this
anyway.
This still doesn't change the fact that the driver is brand new for brand new
devices. I believe we should assume such devices will support ZLPs unless we
have documentation stating anything else.
Anyway, if you want to keep the padding for SG then maybe this will
work
and allow you to drop the extra struct usbnet field and allocation:
if (skb_tailroom(skb) !dev-can_dma_sg) {
skb-data[skb-len] = 0;
__skb_put(skb, 1);
} else if (dev-can_dma_sg) {
sg_set_buf(urb-sg[urb-num_sgs++],
skb-data, 1);
}
I.e. cheat and use the skb-data buffer twice, if that is allowed?
The
actual value of the padding byte should not matter, I believe?
That makes me immediately suspect a violation of the DMA rules.
Sounds likely. And it's an ugly hack in any case. Probably not a good idea.
Just one of the many random thoughts I should have kept to myself :-)
Bjørn
--
To unsubscribe from this list: send the line unsubscribe 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: WARNING/BUG in dwc3 driver in 3.11 kernel
From: Paul Zimmerman Sent: Wednesday, September 18, 2013 3:13 PM I'm getting the following from the dwc3 driver using the dwc3-pci glue layer, when modprobing the driver. It's not fatal, the driver continues to work afterwards. Looks like request_threaded_irq() is getting called with irqs disabled, and it can sleep. I am unable to test with 3.12-rc1, because my VGA console doesn't work with that kernel. After further investigation, it looks like this was fixed in June by b0d7ffd44ba9cd2dfbf299674418193a5f9ed21a usb: dwc3: gadget: don't request IRQs in atomic, but somehow it didn't make it to Linus for 3.11 -- Paul -- To unsubscribe from this list: send the line unsubscribe 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: WARNING/BUG in dwc3 driver in 3.11 kernel
From: Felipe Balbi [mailto:ba...@ti.com] Sent: Wednesday, September 18, 2013 6:58 PM On Wed, Sep 18, 2013 at 10:39:34PM +, Paul Zimmerman wrote: From: Paul Zimmerman Sent: Wednesday, September 18, 2013 3:13 PM I'm getting the following from the dwc3 driver using the dwc3-pci glue layer, when modprobing the driver. It's not fatal, the driver continues to work afterwards. Looks like request_threaded_irq() is getting called with irqs disabled, and it can sleep. I am unable to test with 3.12-rc1, because my VGA console doesn't work with that kernel. After further investigation, it looks like this was fixed in June by b0d7ffd44ba9cd2dfbf299674418193a5f9ed21a usb: dwc3: gadget: don't request IRQs in atomic, but somehow it didn't make it to Linus for 3.11 Do you need that in v3.11 stable ? I can backport the patch and ask Greg to apply to his stable tree. Just let me know ;-) Sure. The patch was marked for stable 3.10, so I think it belongs in 3.11 too. I don't know if it made it to 3.10.x. -- Paul -- To unsubscribe from this list: send the line unsubscribe 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/1] usb: gadget: zero: Add flexible auto remote wakeup test method
In order to increase test coverage, we can change the interval between
two remote wakeups every time, and the interval can be any user defined
value. This change will no affect current behavior if the user does not
use two introduced module parameters.
Signed-off-by: Peter Chen peter.c...@freescale.com
---
Chagnes for v3:
- Change typo: s/paramters/parameters
Changes for v2:
- Change some typo and description
drivers/usb/gadget/zero.c | 25 +++--
1 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/zero.c b/drivers/usb/gadget/zero.c
index 0deb9d6..21da094 100644
--- a/drivers/usb/gadget/zero.c
+++ b/drivers/usb/gadget/zero.c
@@ -95,6 +95,18 @@ unsigned autoresume = DEFAULT_AUTORESUME;
module_param(autoresume, uint, S_IRUGO);
MODULE_PARM_DESC(autoresume, zero, or seconds before remote wakeup);
+/* Maximum Autoresume time */
+unsigned max_autoresume;
+module_param(max_autoresume, uint, S_IRUGO);
+MODULE_PARM_DESC(max_autoresume, maximum seconds before remote wakeup);
+
+/* Interval between two remote wakeups */
+unsigned autoresume_interval_ms;
+module_param(autoresume_interval_ms, uint, S_IRUGO);
+MODULE_PARM_DESC(autoresume_interval_ms,
+ milliseconds to increase successive wakup delays);
+
+static unsigned autoresume_step_ms;
/*-*/
static struct usb_device_descriptor device_desc = {
@@ -183,8 +195,16 @@ static void zero_suspend(struct usb_composite_dev *cdev)
return;
if (autoresume) {
- mod_timer(autoresume_timer, jiffies + (HZ * autoresume));
- DBG(cdev, suspend, wakeup in %d seconds\n, autoresume);
+ if (max_autoresume
+ (autoresume_step_ms max_autoresume * 1000))
+ autoresume_step_ms = autoresume * 1000;
+
+ mod_timer(autoresume_timer, jiffies +
+ msecs_to_jiffies(autoresume_step_ms));
+ DBG(cdev, suspend, wakeup in %d milliseconds\n,
+ autoresume_step_ms);
+
+ autoresume_step_ms += autoresume_interval_ms;
} else
DBG(cdev, %s\n, __func__);
}
@@ -316,6 +336,7 @@ static int __init zero_bind(struct usb_composite_dev *cdev)
if (autoresume) {
sourcesink_driver.bmAttributes |= USB_CONFIG_ATT_WAKEUP;
loopback_driver.bmAttributes |= USB_CONFIG_ATT_WAKEUP;
+ autoresume_step_ms = autoresume * 1000;
}
/* support OTG systems */
--
1.7.1
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
