Re: uhci_hcd: Possible corruption of DMA pool uhci-td_pool
On 12/11/2013 08:41 PM, Alan Stern wrote: On Wed, 11 Dec 2013, Eugene Shatokhin wrote: Hi, On ROSA Linux with kernel 3.10.21 with DMA debug options enabled, the kernel sometimes issues a warning about DMA pool corruption (see the log below). That happens sometimes, when the system boots or resumes from hibernation with Samson C01U USB microphone attached. The affected DMA pool is 'uhci-td_pool', uhci_alloc_td() from drivers/usb/host/uhci-hcd.c makes the relevant dma_pool_alloc() calls. Any ideas about how to find what causes this and how to fix it? This is not an easy sort of thing to track down... Here is the relevant part of the system log: [ 22.264332] usb 2-1: new full-speed USB device number 2 using uhci_hcd [ 22.450609] usb 2-1: New USB device found, idVendor=17a0, idProduct=0001 [ 22.450626] usb 2-1: New USB device strings: Mfr=1, Product=2, SerialNumber=0 [ 22.450639] usb 2-1: Product: Samson C01U [ 22.450649] usb 2-1: Manufacturer: Samson Technologies ... [ 280.703483] retire_capture_urb: 4494 callbacks suppressed [ 284.961087] uhci_hcd :00:1d.1: dma_pool_alloc uhci_td, efb7b060 (corruped) [ 284.961087] : 00 06 00 00 af 00 00 03 a7 a7 a7 a7 a7 a7 a7 a7 [ 284.961087] 0010: a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 [ 284.961087] 0020: a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 [ 284.961087] retire_capture_urb: 4343 callbacks suppressed [ 284.961087] uhci_hcd :00:1d.1: dma_pool_alloc uhci_td, efb7b5d0 (corruped) [ 284.961087] : 00 06 00 00 af 00 00 03 a7 a7 a7 a7 a7 a7 a7 a7 [ 284.961087] 0010: a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 [ 284.961087] 0020: a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 a7 [ 284.961087] cannot submit urb (err = -27) [ 284.961087] cannot submit urb (err = -27) [ 284.961087] cannot submit urb (err = -27) [ 284.961087] cannot submit urb (err = -27) [ 284.961087] cannot submit urb (err = -27) [ 284.961087] cannot submit urb (err = -27) [ 284.961087] cannot submit urb (err = -27) [ 284.961087] cannot submit urb (err = -27) 0xa7 is POOL_POISON_FREED. The memory pages to be allocated from the pool should be filled with such bytes. Each time I observed this problem, the first 8 bytes of the listed memory area were overwritten, with different data each time. It kind of looks like a hardware bug. Still, it's hard to say. Can you test the current 3.13-rc kernel? There have been a few recent changes in this area that might have an effect. I tested 3.13-rc3 - the problem has not shown up so far. Regards, Eugene -- Eugene Shatokhin, ROSA Laboratory. www.rosalab.com -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 3/3] usb/xhci-plat: remove unnecessary #ifdef checks for CONFIG_PM_SLEEP
On 13 December 2013 06:18, David Cohen david.a.co...@linux.intel.com wrote:
From: Santosh Shilimkar santosh.shilim...@ti.com
Drivers using SET_*_PM_OPS() no longer need to #ifdef for CONFIG_PM_*
So, let's remove the unnecessary #ifdef's.
Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
Signed-off-by: David Cohen david.a.co...@linux.intel.com
---
drivers/usb/host/xhci-plat.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index d9c169f470d3..b1d93c344e04 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -197,7 +197,6 @@ static int xhci_plat_remove(struct platform_device *dev)
return 0;
}
-#ifdef CONFIG_PM
I think you can solve this in another way.
As a start, I would suggest to change above to CONFIG_PM_SLEEP
static int xhci_plat_suspend(struct device *dev)
{
struct usb_hcd *hcd = dev_get_drvdata(dev);
@@ -217,10 +216,6 @@ static int xhci_plat_resume(struct device *dev)
static const struct dev_pm_ops xhci_plat_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(xhci_plat_suspend, xhci_plat_resume)
};
-#define DEV_PM_OPS (xhci_plat_pm_ops)
-#else
-#define DEV_PM_OPS NULL
-#endif /* CONFIG_PM */
Remove the use of DEV_PM_OPS define. Instead use below macro and
outside #ifdef CONFIG_PM_SLEEP.
static SIMPLE_DEV_PM_OPS(xhci_plat_pm_ops, xhci_plat_suspend,
xhci_plat_resume)
That should do the trick I believe, without the need for changing the
existing SET_SYSTEM_SLEEP_PM_OPS macro in patch1.
Kind regards
Ulf Hansson
#ifdef CONFIG_OF
static const struct of_device_id usb_xhci_of_match[] = {
@@ -235,7 +230,7 @@ static struct platform_driver usb_xhci_driver = {
.remove = xhci_plat_remove,
.driver = {
.name = xhci-hcd,
- .pm = DEV_PM_OPS,
+ .pm = xhci_plat_pm_ops,
.of_match_table = of_match_ptr(usb_xhci_of_match),
},
};
--
1.8.4.2
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line unsubscribe 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 v5 1/9] phy: add phy_get_bus_width()/phy_set_bus_width() calls
On Thursday 12 December 2013 11:48 PM, Felipe Balbi wrote: Hi, On Thu, Dec 12, 2013 at 08:26:02AM -0500, Matt Porter wrote: This adds a pair of APIs that allows the generic PHY subsystem to provide information on the PHY bus width. The PHY provider driver may use phy_set_bus_width() to set the bus width that the PHY supports. The controller driver may then use phy_get_bus_width() to fetch the PHY bus width in order to properly configure the controller. Signed-off-by: Matt Porter mpor...@linaro.org Kishon, I need your Acked-by here, if you're ok with the change. here it goes.. Acked-by: Kishon Vijay Abraham I kis...@ti.com -- To unsubscribe from this list: send the line unsubscribe 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: usb: musb: ux500: harden checks for platform data
Hello Lee Jones,
The patch 5f6091a023ca: usb: musb: ux500: harden checks for platform
data from May 15, 2013, leads to the following
static checker warning: drivers/usb/musb/ux500_dma.c:335
ux500_dma_controller_start()
error: potential NULL dereference 'param_array'.
drivers/usb/musb/ux500_dma.c
313 param_array = data ? data-dma_rx_param_array : NULL;
^
314 chan_names = (char **)iep_chan_names;
315
316 for (dir = 0; dir 2; dir++) {
317 for (ch_num = 0;
318 ch_num UX500_MUSB_DMA_NUM_RX_TX_CHANNELS;
319 ch_num++) {
320 ux500_channel = channel_array[ch_num];
321 ux500_channel-controller = controller;
322 ux500_channel-ch_num = ch_num;
323 ux500_channel-is_tx = is_tx;
324
325 dma_channel = (ux500_channel-channel);
326 dma_channel-private_data = ux500_channel;
327 dma_channel-status = MUSB_DMA_STATUS_FREE;
328 dma_channel-max_len = SZ_16M;
329
330 ux500_channel-dma_chan =
331 dma_request_slave_channel(dev,
chan_names[ch_num]);
332
333 if (!ux500_channel-dma_chan)
334 ux500_channel-dma_chan =
335 dma_request_channel(mask,
336 data ?
337
data-dma_filter :
338 NULL,
339
param_array[ch_num]);
^
It's not clear if param_array[] is NULL or not here.
340
341 if (!ux500_channel-dma_chan) {
342 ERR(Dma pipe allocation error dir=%d
ch=%d\n,
343 dir, ch_num);
344
345 /* Release already allocated channels */
346 ux500_dma_controller_stop(controller);
347
348 return -EBUSY;
349 }
350
351 }
352
353 /* Prepare the loop for TX channels */
354 channel_array = controller-tx_channel;
355 param_array = data ? data-dma_tx_param_array : NULL;
^^
356 chan_names = (char **)oep_chan_names;
357 is_tx = 1;
358 }
359
regards,
dan carpenter
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] USB: gadget: add maxpacket_limit field to struct usb_ep
On 12/12/2013 08:14 PM, Paul Zimmerman wrote: From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Robert Baldyga Sent: Thursday, December 12, 2013 12:51 AM This patch adds maxpacket_limit to struct usb_ep. This field contains maximum value of maxpacket supported by driver, and is set in driver probe. This value should be used by autoconfig() function, because value of field maxpacket is set to value from endpoint descriptor when endpoint becomes enabled. So when autoconfig() function will be called again for this endpoint, maxpacket value will contain wMaxPacketSize from descriptior instead of maximum packet size for this endpoint. Can you describe what the exact problem is that this patch is trying to solve? Under what circumstances does the problem show up? And perhaps a pointer to any previous email thread where this was discussed? Thanks. Hello Paul, This problem shows up when you unload gadget which uses, for example, maxpacket=64 for its endpoints, and then you want to load gadget which want to use greater maxpacket value (for example maxpacket=512). In described case autoconfig() function will not be able to return endpoints used by previous gadget, because their maxpacket field is set to 64, and it seems to be maximum supported value. Using additional field maxpacket_limit to store original value set by UDC driver solves this problem, because autoconfig can see maximum packet value handled by endpoint irrespectively to maxpacket value used by previously loaded gadget. Best regards Robert Baldyga Samsung RD Institute Poland -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/1] AX88179_178A: Add FLAG_HW_IPALIGN to determine whether reserving NET_IP_ALIGN bytes for an SKB.
From: fre...@asix.com.tw
...
- skb = __netdev_alloc_skb_ip_align(dev-net, size, flags);
+ if (dev-driver_info-flags FLAG_HW_IPALIGN)
+ skb = __netdev_alloc_skb(dev-net, size, flags);
+ else
+ skb = __netdev_alloc_skb_ip_align(dev-net, size, flags);
Given the definition:
static inline struct sk_buff *__netdev_alloc_skb_ip_align(struct net_device
*dev,
unsigned int length, gfp_t gfp)
{
struct sk_buff *skb = __netdev_alloc_skb(dev, length + NET_IP_ALIGN,
gfp);
if (NET_IP_ALIGN skb)
skb_reserve(skb, NET_IP_ALIGN);
return skb;
}
It really ought to be possible to code that without an extra conditional.
David
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/1] usb: musb: ux500_dma: fix potential NULL dereference error
static checker warning: drivers/usb/musb/ux500_dma.c:335
ux500_dma_controller_start()
error: potential NULL dereference 'param_array'.
Reported-by: Dan Carpenter dan.carpen...@oracle.com
Signed-off-by: Lee Jones lee.jo...@linaro.org
---
drivers/usb/musb/ux500_dma.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/musb/ux500_dma.c b/drivers/usb/musb/ux500_dma.c
index 3700e97..9aad00f 100644
--- a/drivers/usb/musb/ux500_dma.c
+++ b/drivers/usb/musb/ux500_dma.c
@@ -336,7 +336,9 @@ static int ux500_dma_controller_start(struct
ux500_dma_controller *controller)
data ?
data-dma_filter :
NULL,
-
param_array[ch_num]);
+ param_array ?
+ param_array[ch_num]
:
+ NULL);
if (!ux500_channel-dma_chan) {
ERR(Dma pipe allocation error dir=%d ch=%d\n,
--
1.8.3.2
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 5/9] usb: gadget: s3c-hsotg: use generic phy_init()/phy_exit() support
On Thursday 12 December 2013 06:56 PM, Matt Porter wrote: If a generic phy is present, call phy_init()/phy_exit(). This supports generic phys that must be soft reset before power on. Signed-off-by: Matt Porter mpor...@linaro.org Acked-by: Kishon Vijay Abraham I kis...@ti.com --- drivers/usb/gadget/s3c-hsotg.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c index 7c5d8bd..e9683c2 100644 --- a/drivers/usb/gadget/s3c-hsotg.c +++ b/drivers/usb/gadget/s3c-hsotg.c @@ -3621,6 +3621,9 @@ static int s3c_hsotg_probe(struct platform_device *pdev) goto err_supplies; } + if (hsotg-phy) + phy_init(hsotg-phy); + /* usb phy enable */ s3c_hsotg_phy_enable(hsotg); @@ -3714,6 +3717,8 @@ static int s3c_hsotg_remove(struct platform_device *pdev) } s3c_hsotg_phy_disable(hsotg); + if (hsotg-phy) + phy_exit(hsotg-phy); clk_disable_unprepare(hsotg-clk); return 0; -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 6/9] usb: gadget: s3c-hsotg: get phy bus width from phy subsystem
On Thursday 12 December 2013 06:56 PM, Matt Porter wrote:
Adds support for querying the phy bus width from the generic phy
subsystem. Configure UTMI bus width in GUSBCFG based on this value.
Signed-off-by: Matt Porter mpor...@linaro.org
Acked-by: Kishon Vijay Abraham I kis...@ti.com
---
drivers/usb/gadget/s3c-hsotg.c | 14 +-
drivers/usb/gadget/s3c-hsotg.h | 1 +
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index e9683c2..168aaa9 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -144,6 +144,7 @@ struct s3c_hsotg_ep {
* @regs: The memory area mapped for accessing registers.
* @irq: The IRQ number we are using
* @supplies: Definition of USB power supplies
+ * @phyif: PHY interface width
* @dedicated_fifos: Set if the hardware has dedicated IN-EP fifos.
* @num_of_eps: Number of available EPs (excluding EP0)
* @debug_root: root directrory for debugfs.
@@ -171,6 +172,7 @@ struct s3c_hsotg {
struct regulator_bulk_data supplies[ARRAY_SIZE(s3c_hsotg_supply_names)];
+ u32 phyif;
unsigned intdedicated_fifos:1;
unsigned char num_of_eps;
@@ -2276,7 +2278,7 @@ static void s3c_hsotg_core_init(struct s3c_hsotg *hsotg)
*/
/* set the PLL on, remove the HNP/SRP and set the PHY */
- writel(GUSBCFG_PHYIf16 | GUSBCFG_TOutCal(7) |
+ writel(hsotg-phyif | GUSBCFG_TOutCal(7) |
(0x5 10), hsotg-regs + GUSBCFG);
s3c_hsotg_init_fifo(hsotg);
@@ -3621,6 +3623,16 @@ static int s3c_hsotg_probe(struct platform_device
*pdev)
goto err_supplies;
}
+ /* Set default UTMI width */
+ hsotg-phyif = GUSBCFG_PHYIf16;
+
+ /*
+ * If using the generic PHY framework, check if the PHY bus
+ * width is 8-bit and set the phyif appropriately.
+ */
+ if (hsotg-phy (phy_get_bus_width(phy) == 8))
+ hsotg-phyif = GUSBCFG_PHYIf8;
+
if (hsotg-phy)
phy_init(hsotg-phy);
diff --git a/drivers/usb/gadget/s3c-hsotg.h b/drivers/usb/gadget/s3c-hsotg.h
index d650b12..85f549f 100644
--- a/drivers/usb/gadget/s3c-hsotg.h
+++ b/drivers/usb/gadget/s3c-hsotg.h
@@ -55,6 +55,7 @@
#define GUSBCFG_HNPCap (1 9)
#define GUSBCFG_SRPCap (1 8)
#define GUSBCFG_PHYIf16 (1 3)
+#define GUSBCFG_PHYIf8 (0 3)
#define GUSBCFG_TOutCal_MASK (0x7 0)
#define GUSBCFG_TOutCal_SHIFT(0)
#define GUSBCFG_TOutCal_LIMIT(0x7)
--
To unsubscribe from this list: send the line unsubscribe 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: usb: gadget: nokia: convert to new interface of f_phonet
Hello Andrzej Pietrasiewicz,
The patch 83167f12da05: usb: gadget: nokia: convert to new interface
of f_phonet from May 23, 2013, leads to the following Smatch
warnings:
drivers/usb/gadget/nokia.c:209 nokia_bind_config() error: potential NULL
dereference 'f_obex2'.
drivers/usb/gadget/nokia.c:211 nokia_bind_config() error: potential NULL
dereference 'f_obex1'.
drivers/usb/gadget/nokia.c:213 nokia_bind_config() error: potential NULL
dereference 'f_phonet'.
Let me walk you throug the code for the first warning.
drivers/usb/gadget/nokia.c
115 static struct usb_function_instance *fi_acm;
116 static struct usb_function_instance *fi_ecm;
117 static struct usb_function_instance *fi_obex1;
118 static struct usb_function_instance *fi_obex2;
119 static struct usb_function_instance *fi_phonet;
120
121 static int __init nokia_bind_config(struct usb_configuration *c)
122 {
123 struct usb_function *f_acm;
124 struct usb_function *f_phonet = NULL;
125 struct usb_function *f_obex1 = NULL;
126 struct usb_function *f_ecm;
127 struct usb_function *f_obex2 = NULL;
128 int status = 0;
129 int obex1_stat = 0;
130 int obex2_stat = 0;
131 int phonet_stat = 0;
We need to trace three variables to understand the first warning
message: fi_obex2, f_obex2, and obex2_stat.
Lets start with the assumption that fi_obex2 is an error pointer at
the start of the function.
f_obex2 is NULL.
obex2_stat is zero.
132
133 if (!IS_ERR(fi_phonet)) {
134 f_phonet = usb_get_function(fi_phonet);
135 if (IS_ERR(f_phonet))
136 pr_debug(could not get phonet function\n);
137 }
138
139 if (!IS_ERR(fi_obex1)) {
140 f_obex1 = usb_get_function(fi_obex1);
141 if (IS_ERR(f_obex1))
142 pr_debug(could not get obex function 0\n);
It's funny that the human readable messages start counting from zero as
in obex function 0 and obex function 1 but the computer code starts
counting from 1 as in fi_obex1 and fi_obex2. Normally you would
expect it to be the other way around. Also this is not consistent with
the rest of the driver.
143 }
144
145 if (!IS_ERR(fi_obex2)) {
146 f_obex2 = usb_get_function(fi_obex2);
147 if (IS_ERR(f_obex2))
148 pr_debug(could not get obex function 1\n);
149 }
fi_obex2 is an error pointer.
f_obex2 is NULL.
obex2_stat is zero.
150
151 f_acm = usb_get_function(fi_acm);
152 if (IS_ERR(f_acm)) {
153 status = PTR_ERR(f_acm);
154 goto err_get_acm;
155 }
156
157 f_ecm = usb_get_function(fi_ecm);
158 if (IS_ERR(f_ecm)) {
159 status = PTR_ERR(f_ecm);
160 goto err_get_ecm;
161 }
162
163 if (!IS_ERR_OR_NULL(f_phonet)) {
164 phonet_stat = usb_add_function(c, f_phonet);
165 if (phonet_stat)
166 pr_debug(could not add phonet function\n);
167 }
168
169 if (!IS_ERR_OR_NULL(f_obex1)) {
170 obex1_stat = usb_add_function(c, f_obex1);
171 if (obex1_stat)
172 pr_debug(could not add obex function 0\n);
173 }
174
175 if (!IS_ERR_OR_NULL(f_obex2)) {
176 obex2_stat = usb_add_function(c, f_obex2);
177 if (obex2_stat)
178 pr_debug(could not add obex function 1\n);
179 }
f_obex2 is NULL so the condition is false.
obex2_stat is zero.
180
181 status = usb_add_function(c, f_acm);
182 if (status)
183 goto err_conf;
Let's assume we hit this goto.
184
185 status = usb_add_function(c, f_ecm);
186 if (status) {
187 pr_debug(could not bind ecm config %d\n, status);
188 goto err_ecm;
189 }
190 if (c == nokia_config_500ma_driver) {
191 f_acm_cfg1 = f_acm;
192 f_ecm_cfg1 = f_ecm;
193 f_phonet_cfg1 = f_phonet;
194 f_obex1_cfg1 = f_obex1;
195 f_obex2_cfg1 = f_obex2;
196 } else {
197 f_acm_cfg2 = f_acm;
198 f_ecm_cfg2 = f_ecm;
199 f_phonet_cfg2 = f_phonet;
200 f_obex1_cfg2 = f_obex1;
201 f_obex2_cfg2 = f_obex2;
Btw, we are assigning an ERR_PTR to the global here. It means we need
to constantly check it, which the code does, but it's messy. The driver
[PATCH v3] USB: gadget: add maxpacket_limit field to struct usb_ep
This patch adds maxpacket_limit to struct usb_ep. This field contains
maximum value of maxpacket supported by driver, and is set in driver probe.
This value should be used by autoconfig() function, because value of field
maxpacket is set to value from endpoint descriptor when endpoint becomes
enabled. So when autoconfig() function will be called again for this endpoint,
maxpacket value will contain wMaxPacketSize from descriptior instead of
maximum packet size for this endpoint.
For this reason this patch adds new field maxpacket_limit which contains
value of maximum packet size (which defines maximum endpoint capabilities).
This value is used in ep_matches() function used by autoconfig().
Value of maxpacket_limit should be set in UDC driver probe function, using
usb_ep_set_maxpacket_limit() function, defined in gadget.h. This function
set choosen value to both maxpacket_limit and maxpacket fields.
This patch modifies UDC drivers by adding support for maxpacket_limit.
Signed-off-by: Robert Baldyga r.bald...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
Hello,
This is third version of this patch. I added support for few UDC drivers.
Best regards
Robert Baldyga
Samsung RD Institute Poland
Changelog:
v3:
- add support for: drivers/usb/dwc3, drivers/usb/musb,
drivers/usb/renesas_usbhs, drivers/usb/gadget/mv_u3d_core,
drivers/usb/gadget/fsl_qe_udc, drivers/usb/gadget/net2272.c,
drivers/usb/gadget/net2280.c and drivers/usb/chipidea
v2: http://www.spinics.net/lists/linux-usb/msg99330.html
- add documentation for maxpacket_limit field in struct usb_ep and for
usb_ep_set_maxpacket_limit() function
- add maxpacket_limit support for dummy-hdc UDC driver
v1: http://www.spinics.net/lists/linux-usb/msg99305.html
drivers/usb/chipidea/udc.c |4 ++--
drivers/usb/dwc3/gadget.c |4 ++--
drivers/usb/gadget/amd5536udc.c| 15 +--
drivers/usb/gadget/at91_udc.c | 16
drivers/usb/gadget/atmel_usba_udc.c|5 +++--
drivers/usb/gadget/bcm63xx_udc.c |4 ++--
drivers/usb/gadget/dummy_hcd.c |2 +-
drivers/usb/gadget/epautoconf.c|6 +++---
drivers/usb/gadget/fotg210-udc.c |3 ++-
drivers/usb/gadget/fsl_qe_udc.c|2 +-
drivers/usb/gadget/fsl_udc_core.c |5 +++--
drivers/usb/gadget/fusb300_udc.c |4 ++--
drivers/usb/gadget/goku_udc.c |4 ++--
drivers/usb/gadget/lpc32xx_udc.c |2 +-
drivers/usb/gadget/m66592-udc.c|4 ++--
drivers/usb/gadget/mv_u3d_core.c |4 ++--
drivers/usb/gadget/mv_udc_core.c |4 ++--
drivers/usb/gadget/net2272.c |4 ++--
drivers/usb/gadget/net2280.c |8
drivers/usb/gadget/omap_udc.c |3 ++-
drivers/usb/gadget/pch_udc.c |6 +++---
drivers/usb/gadget/pxa25x_udc.c|1 +
drivers/usb/gadget/pxa27x_udc.c|5 -
drivers/usb/gadget/r8a66597-udc.c |4 ++--
drivers/usb/gadget/s3c-hsotg.c |2 +-
drivers/usb/gadget/s3c-hsudc.c |2 +-
drivers/usb/gadget/s3c2410_udc.c |1 +
drivers/usb/musb/musb_gadget.c |6 +++---
drivers/usb/renesas_usbhs/mod_gadget.c |4 ++--
include/linux/usb/gadget.h | 19 +++
30 files changed, 92 insertions(+), 61 deletions(-)
diff --git a/drivers/usb/chipidea/udc.c b/drivers/usb/chipidea/udc.c
index b34c819..77e4a17 100644
--- a/drivers/usb/chipidea/udc.c
+++ b/drivers/usb/chipidea/udc.c
@@ -1566,7 +1566,7 @@ static int init_eps(struct ci_hdrc *ci)
* eps, maxP is set by epautoconfig() called
* by gadget layer
*/
- hwep-ep.maxpacket = (unsigned short)~0;
+ usb_ep_set_maxpacket_limit(hwep-ep, (unsigned
short)~0);
INIT_LIST_HEAD(hwep-qh.queue);
hwep-qh.ptr = dma_pool_alloc(ci-qh_pool, GFP_KERNEL,
@@ -1586,7 +1586,7 @@ static int init_eps(struct ci_hdrc *ci)
else
ci-ep0in = hwep;
- hwep-ep.maxpacket = CTRL_PAYLOAD_MAX;
+ usb_ep_set_maxpacket_limit(hwep-ep,
CTRL_PAYLOAD_MAX);
continue;
}
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 5452c0f..9988195 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -1653,7 +1653,7 @@ static int dwc3_gadget_init_hw_endpoints(struct dwc3 *dwc,
dev_vdbg(dwc-dev, initializing %s\n, dep-name);
if (epnum == 0 || epnum == 1) {
- dep-endpoint.maxpacket = 512;
+ usb_ep_set_maxpacket_limit(dep-endpoint, 512);
dep-endpoint.maxburst = 1;
Re: some question about period scheduleing
hi 2013/12/2 Alan Stern st...@rowland.harvard.edu: On Sun, 1 Dec 2013, vichy wrote: hi Alan: 2013/12/1 Alan Stern st...@rowland.harvard.edu: On Fri, 29 Nov 2013, vichy wrote: hi all: My connection like below ehci -- high speed hub - full speed device I have some questions about period scheduling. 1. when creating qh for full speed device, why we set EHCI_TUNE_RL_TT. Are you asking why EHCI_TUNE_RL_TT is equal to 0? I don't know -- it looks like a mistake. Do you find that changing it to 3 makes any difference? Yes, when I change EHCI_TUNE_RL_TT as not 0. The transaction can keep going. But if EHCI_TUNE_RL_TT is 0, the transfer doesn't work? No. the transaction will stop since device return Nak. I copy the usb traffic log for your reference. usually device will not return NAK in setup token. but in my case, it did. Thanks for your kind help, attachment: Device.Nak.png
Re: WG: [PATCH] usb: musb: omap2430: fix occasional musb breakage on boot
On Thu, Dec 12, 2013 at 11:29 PM, Andreas Naumann d...@andin.de wrote:
Hi Grazvydas,
Von: Grazvydas Ignotas [mailto:nota...@gmail.com]
Gesendet: Donnerstag, 12. Dezember 2013 01:21
An: linux-usb@vger.kernel.org
Cc: Felipe Balbi; linux-o...@vger.kernel.org; Naumann Andreas; Grazvydas
Ignotas; sta...@vger.kernel.org
Betreff: [PATCH] usb: musb: omap2430: fix occasional musb breakage on boot
This is a hard to reproduce problem which leads to non-functional
USB-OTG port in 0.1%-1% of all boots. Tracked it down to commit
e25bec160158abe86c omap2+: save and restore OTG_INTERFSEL,
which introduces save/restore of OTG_INTERFSEL over suspend.
Since the resume function is also called early in driver init, it uses a
non-initialized value (which is 0 and a non-supported setting in DM37xx
for INTERFSEL). Shortly after the correct value is set. Apparently this
works most time, but not always.
Fix it by not writing the value on runtime resume if it is 0
(0 should never be saved in the context as it's invalid value,
so we use it as an indicator that context hasn't been saved yet).
This issue was originally found by Andreas Naumann:
http://marc.info/?l=linux-usbm=138562574719654w=2
Reported-and-bisected-by: Andreas Naumann anaum...@ultratronik.de
Signed-off-by: Grazvydas Ignotas nota...@gmail.com
Cc: sta...@vger.kernel.org
---
This is a regression from 3.2, so should go to -rc and stable, IMO.
It's really annoying issue if you want to have a stable OTG behavior,
I've burned quite a lot of time on it myself over a year ago and gave up
eventually. Good thing Andreas finally found it, many thanks to him!
drivers/usb/musb/omap2430.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 2a408cd..737b3da 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -672,7 +672,8 @@ static int omap2430_runtime_resume(struct device *dev)
if (musb) {
omap2430_low_level_init(musb);
- musb_writel(musb-mregs, OTG_INTERFSEL,
+ if (musb-context.otg_interfsel != 0)
+ musb_writel(musb-mregs, OTG_INTERFSEL,
musb-context.otg_interfsel);
phy_power_on(musb-phy);
}
Oh, easy way out. I like it but I've also been thinking about your comment
on my original post, which was that initializing otg_interfsel to the PHYSEL
bits only might be dangerous because we cant be sure that there are other
bits in the register.
However, isnt assuming that 0 is invalid on all OMAPs just as dangerous?
Well I was trying to do a minimal fix so that it could be suitable for
merging to stable kernels. But yes you're right, I've just checked
OMAP4 TRM and 0 is actually valid value there..
After thinking about my patch again, I would propose to change otg_interfsel
into otg_physel and read-modify-write only those bits in resume() as you
suggested in your first answer. That way I could discard the problematic
first read in probe() while leaving other bits untouched. If you agree I
post a patch for this tomorrow.
Hmm I don't know about that, this would be inconsistent with what all
other OMAP drivers do. Maybe we should do what musb_core.c does just
to be consistent and add a similar comment. Only the static variable
could be avoided in favor of struct omap2430_glue member.
Gražvydas
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
gadgetfs USB2.0 Chapter 9 Tests: Test after Addressed State fails
Hello! I am new to this group and hope you can help me out. I am on an embedded board with an ARM cortex a5 and using gadgetfs. An Application that is derived from the usb.c example is running on it while the board is connected to a PC with the USB 2.0 Command Verifier. I am quite new to development with usb. When I run all the Chapter 9 tests, the Device Descriptor Test - Configured State and the Device Descriptor Test - Addressed State are passed while the Interface Association Descriptor Test and all the following tests fail. I found out that in Interface Association Descriptor Test the open() of the endpoint fails as well as the read() of the filedescriptor after handling the USB_REQ_SET_CONFIGURATION in this line: /* ... ack (a write would stall) */ status = read (fd, status, 0); if (status) perror (\t\tack SET_CONFIGURATION); return; The print out of the error message is Identifier removed. I noticed that when I leave out all the Adressed state Tests, my Tests would all pass. As I mentioned, I'm new to USB developing and maybe there's no way to achieve Chapter 9 conformity with GadgetFS or there's a known bug? Thanks for any additional info! -- To unsubscribe from this list: send the line unsubscribe 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: usb: gadget: nokia: convert to new interface of f_phonet
Hello Dan Carpenter,
Thank you for your comments.
W dniu 13.12.2013 12:22, Dan Carpenter pisze:
Hello Andrzej Pietrasiewicz,
The patch 83167f12da05: usb: gadget: nokia: convert to new interface
of f_phonet from May 23, 2013, leads to the following Smatch
warnings:
drivers/usb/gadget/nokia.c:209 nokia_bind_config() error: potential NULL
dereference 'f_obex2'.
drivers/usb/gadget/nokia.c:211 nokia_bind_config() error: potential NULL
dereference 'f_obex1'.
drivers/usb/gadget/nokia.c:213 nokia_bind_config() error: potential NULL
dereference 'f_phonet'.
Let me walk you throug the code for the first warning.
drivers/usb/gadget/nokia.c
115 static struct usb_function_instance *fi_acm;
116 static struct usb_function_instance *fi_ecm;
117 static struct usb_function_instance *fi_obex1;
118 static struct usb_function_instance *fi_obex2;
119 static struct usb_function_instance *fi_phonet;
120
121 static int __init nokia_bind_config(struct usb_configuration *c)
122 {
123 struct usb_function *f_acm;
124 struct usb_function *f_phonet = NULL;
125 struct usb_function *f_obex1 = NULL;
126 struct usb_function *f_ecm;
127 struct usb_function *f_obex2 = NULL;
128 int status = 0;
129 int obex1_stat = 0;
130 int obex2_stat = 0;
131 int phonet_stat = 0;
We need to trace three variables to understand the first warning
message: fi_obex2, f_obex2, and obex2_stat.
Lets start with the assumption that fi_obex2 is an error pointer at
the start of the function.
f_obex2 is NULL.
obex2_stat is zero.
132
133 if (!IS_ERR(fi_phonet)) {
134 f_phonet = usb_get_function(fi_phonet);
135 if (IS_ERR(f_phonet))
136 pr_debug(could not get phonet function\n);
137 }
138
139 if (!IS_ERR(fi_obex1)) {
140 f_obex1 = usb_get_function(fi_obex1);
141 if (IS_ERR(f_obex1))
142 pr_debug(could not get obex function 0\n);
It's funny that the human readable messages start counting from zero as
in obex function 0 and obex function 1 but the computer code starts
counting from 1 as in fi_obex1 and fi_obex2. Normally you would
expect it to be the other way around. Also this is not consistent with
the rest of the driver.
143 }
144
145 if (!IS_ERR(fi_obex2)) {
146 f_obex2 = usb_get_function(fi_obex2);
147 if (IS_ERR(f_obex2))
148 pr_debug(could not get obex function 1\n);
149 }
fi_obex2 is an error pointer.
f_obex2 is NULL.
obex2_stat is zero.
150
151 f_acm = usb_get_function(fi_acm);
152 if (IS_ERR(f_acm)) {
153 status = PTR_ERR(f_acm);
154 goto err_get_acm;
155 }
156
157 f_ecm = usb_get_function(fi_ecm);
158 if (IS_ERR(f_ecm)) {
159 status = PTR_ERR(f_ecm);
160 goto err_get_ecm;
161 }
162
163 if (!IS_ERR_OR_NULL(f_phonet)) {
164 phonet_stat = usb_add_function(c, f_phonet);
165 if (phonet_stat)
166 pr_debug(could not add phonet function\n);
167 }
168
169 if (!IS_ERR_OR_NULL(f_obex1)) {
170 obex1_stat = usb_add_function(c, f_obex1);
171 if (obex1_stat)
172 pr_debug(could not add obex function 0\n);
173 }
174
175 if (!IS_ERR_OR_NULL(f_obex2)) {
176 obex2_stat = usb_add_function(c, f_obex2);
177 if (obex2_stat)
178 pr_debug(could not add obex function 1\n);
179 }
f_obex2 is NULL so the condition is false.
obex2_stat is zero.
180
181 status = usb_add_function(c, f_acm);
182 if (status)
183 goto err_conf;
Let's assume we hit this goto.
184
185 status = usb_add_function(c, f_ecm);
186 if (status) {
187 pr_debug(could not bind ecm config %d\n, status);
188 goto err_ecm;
189 }
190 if (c == nokia_config_500ma_driver) {
191 f_acm_cfg1 = f_acm;
192 f_ecm_cfg1 = f_ecm;
193 f_phonet_cfg1 = f_phonet;
194 f_obex1_cfg1 = f_obex1;
195 f_obex2_cfg1 = f_obex2;
196 } else {
197 f_acm_cfg2 = f_acm;
198 f_ecm_cfg2 = f_ecm;
199 f_phonet_cfg2 = f_phonet;
200 f_obex1_cfg2 = f_obex1;
201
RE: gadgetfs USB2.0 Chapter 9 Tests: Test after Addressed State fails
Hi Macro, We have observed same issue and reason for this issue is reset_config which triggers complete USB disconnect from F_FS. For SET_CONFIG(Config#0) there is no need to do USB Disconnect. This seems to be bottleneck issue for USB compliance. I believe this issue should be addressed by GadgetFS driver. Regards, Roshan -Original Message- From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-ow...@vger.kernel.org] On Behalf Of Marco Johannes Sent: Friday, December 13, 2013 6:56 PM To: linux-usb@vger.kernel.org Subject: gadgetfs USB2.0 Chapter 9 Tests: Test after Addressed State fails Hello! I am new to this group and hope you can help me out. I am on an embedded board with an ARM cortex a5 and using gadgetfs. An Application that is derived from the usb.c example is running on it while the board is connected to a PC with the USB 2.0 Command Verifier. I am quite new to development with usb. When I run all the Chapter 9 tests, the Device Descriptor Test - Configured State and the Device Descriptor Test - Addressed State are passed while the Interface Association Descriptor Test and all the following tests fail. I found out that in Interface Association Descriptor Test the open() of the endpoint fails as well as the read() of the filedescriptor after handling the USB_REQ_SET_CONFIGURATION in this line: /* ... ack (a write would stall) */ status = read (fd, status, 0); if (status) perror (\t\tack SET_CONFIGURATION); return; The print out of the error message is Identifier removed. I noticed that when I leave out all the Adressed state Tests, my Tests would all pass. As I mentioned, I'm new to USB developing and maybe there's no way to achieve Chapter 9 conformity with GadgetFS or there's a known bug? Thanks for any additional info! -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: xhci_hcd debugging status, please?
On 2013-12-10 16:03, Oliver Neukum wrote: Thanks, but some of the messages look quite hardcoded. Is there a patch to get rid of them? More patches concerning dynamic debugging are in the queue for 3.14 It looks like on your system it is by default switched on. You can switch it off at boot time in the kernel command line as documented in Documentation/dynamic-debug-howto.txt Default debugging is a bit unusual. Can we please change this so that my dmesg is left at peace by default? (unless really needed...) Kind regards, Udo -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
next_string_id in Kernel 3.10
Hi, Recently, we encountered one issue in Kernel 3.10(On Android 4.4) where USB enumeration is falling after many connect/disconnect. The reason for this issue is, we started using usb_gstrings_attach for string update in configuration and cdev-next_string_id is never set to 0 in disconnect case. I could see the commit 88af8b which has this fix in composite_dev_cleanup. But when we use Android framework, this is never called because all USB driver unbind happens through android_disable. In Kernel 3.4.5, usb string in configuration gets updated once during boot (bind_config) but in Kernel 3.10, it happens every connect event (usb_gstrings_attach), if uses f_fs.c So we don't want to call composite_dev_cleanup (consider android case) then cdev-next_string_id has to be clear somewhere else. Do you think this should be handle by function driver (ACM/MTP/F_FS) or this can be fixed in composite.c itself. Can this be fixed in reset_config than composite_dev_cleanup?? Regards, Roshan -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usb: gadget: nokia: fix error recovery path for optional functions
In the nokia gadget some USB functions (obex 1 and 2, phonet) are optional.
If at the start of nokia_bind_config e.g. fi_phonet is an error pointer,
which can happen because we don't fail the bind process if
usb_get_function_instance() fails for fi_phonet, then f_phonet is NULL, and
phonet_stat = usb_add_function(c, f_phonet);
is never called and phonet_stat remains 0.
If, in these circumstances, we hit the err_conf label then !phonet_stat
evaluates to true and we try usb_remove_function() with its second
parameter being f_phonet which is NULL and it causes NULL pointer
dereference.
This patch changes the initial values of (obex1|obex2|phonet)_stat to a
nonzero value so that if the err_conf label is hit while the respective
functions have not been acquired the usb_remove_function() is not called
for those functions.
Signed-off-by: Andrzej Pietrasiewicz andrze...@samsung.com
---
drivers/usb/gadget/nokia.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/nokia.c b/drivers/usb/gadget/nokia.c
index 0a8099a..3ab3861 100644
--- a/drivers/usb/gadget/nokia.c
+++ b/drivers/usb/gadget/nokia.c
@@ -126,9 +126,9 @@ static int __init nokia_bind_config(struct
usb_configuration *c)
struct usb_function *f_ecm;
struct usb_function *f_obex2 = NULL;
int status = 0;
- int obex1_stat = 0;
- int obex2_stat = 0;
- int phonet_stat = 0;
+ int obex1_stat = -1;
+ int obex2_stat = -1;
+ int phonet_stat = -1;
if (!IS_ERR(fi_phonet)) {
f_phonet = usb_get_function(fi_phonet);
--
1.7.0.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/2] phy: core: Add devm_of_phy_get to phy-core
From: Kamil Debski k.deb...@samsung.com
Adding devm_of_phy_get will allow to get phys by supplying the
device_node instead of by name.
Signed-off-by: Kamil Debski k.deb...@samsung.com
---
drivers/phy/phy-core.c | 31 +++
include/linux/phy/phy.h |2 ++
2 files changed, 33 insertions(+)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index f0dbd42..661f7ab 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -446,6 +446,37 @@ struct phy *devm_phy_get(struct device *dev, const char
*string)
EXPORT_SYMBOL_GPL(devm_phy_get);
/**
+ * devm_of_phy_get() - lookup and obtain a reference to a phy.
+ * @dev: device that requests this phy
+ * @np: node containing the phy
+ * @index: the index of the phy
+ *
+ * Gets the phy using phy_get(), and associates a device with it using
+ * devres. On driver detach, release function is invoked on the devres data,
+ * then, devres data is freed.
+ */
+struct phy *devm_of_phy_get(struct device *dev, struct device_node *np,
+ int index)
+{
+ struct phy **ptr, *phy;
+
+ ptr = devres_alloc(devm_phy_release, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
+
+ phy = of_phy_get(np, index);
+ if (!IS_ERR(phy)) {
+ *ptr = phy;
+ devres_add(dev, ptr);
+ } else {
+ devres_free(ptr);
+ }
+
+ return phy;
+}
+EXPORT_SYMBOL_GPL(devm_of_phy_get);
+
+/**
* phy_create() - create a new phy
* @dev: device that is creating the new phy
* @ops: function pointers for performing phy operations
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 169f572..db36d81 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -129,6 +129,8 @@ int phy_power_on(struct phy *phy);
int phy_power_off(struct phy *phy);
struct phy *phy_get(struct device *dev, const char *string);
struct phy *devm_phy_get(struct device *dev, const char *string);
+struct phy *devm_of_phy_get(struct device *dev, struct device_node *np,
+ int index);
void phy_put(struct phy *phy);
void devm_phy_put(struct device *dev, struct phy *phy);
struct phy *of_phy_get(struct device_node *np, int index);
--
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/2] phy: core: Add an exported of_phy_get function
From: Kamil Debski k.deb...@samsung.com
Previously the of_phy_get function took a struct device * and
was declared static. It was impossible to call it from
another driver and thus it was impossible to get phy defined
for a given node. The old function was renamed to _of_phy_get
and was left for internal use. of_phy_get function was added
and it was exported. The function enables to get a phy for
a given device tree node.
Signed-off-by: Kamil Debski k.deb...@samsung.com
---
drivers/phy/phy-core.c | 41 -
include/linux/phy/phy.h |1 +
2 files changed, 33 insertions(+), 9 deletions(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 03cf8fb..f0dbd42 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -240,8 +240,8 @@ out:
EXPORT_SYMBOL_GPL(phy_power_off);
/**
- * of_phy_get() - lookup and obtain a reference to a phy by phandle
- * @dev: device that requests this phy
+ * _of_phy_get() - lookup and obtain a reference to a phy by phandle
+ * @np: device_node for which to get the phy
* @index: the index of the phy
*
* Returns the phy associated with the given phandle value,
@@ -250,20 +250,17 @@ EXPORT_SYMBOL_GPL(phy_power_off);
* not yet loaded. This function uses of_xlate call back function provided
* while registering the phy_provider to find the phy instance.
*/
-static struct phy *of_phy_get(struct device *dev, int index)
+static struct phy *_of_phy_get(struct device_node *np, int index)
{
int ret;
struct phy_provider *phy_provider;
struct phy *phy = NULL;
struct of_phandle_args args;
- ret = of_parse_phandle_with_args(dev-of_node, phys, #phy-cells,
+ ret = of_parse_phandle_with_args(np, phys, #phy-cells,
index, args);
- if (ret) {
- dev_dbg(dev, failed to get phy in %s node\n,
- dev-of_node-full_name);
+ if (ret)
return ERR_PTR(-ENODEV);
- }
mutex_lock(phy_provider_mutex);
phy_provider = of_phy_provider_lookup(args.np);
@@ -283,6 +280,32 @@ err0:
}
/**
+ * of_phy_get() - lookup and obtain a reference to a phy using a device_node.
+ * @np: device_node for which to get the phy
+ * @index: the index of the phy
+ *
+ * Returns the phy driver, after getting a refcount to it; or
+ * -ENODEV if there is no such phy. The caller is responsible for
+ * calling phy_put() to release that count.
+ */
+struct phy *of_phy_get(struct device_node *np, int index)
+{
+ struct phy *phy = NULL;
+
+ phy = _of_phy_get(np, index);
+ if (IS_ERR(phy))
+ return phy;
+
+ if (!try_module_get(phy-ops-owner))
+ return ERR_PTR(-EPROBE_DEFER);
+
+ get_device(phy-dev);
+
+ return phy;
+}
+EXPORT_SYMBOL_GPL(of_phy_get);
+
+/**
* phy_put() - release the PHY
* @phy: the phy returned by phy_get()
*
@@ -370,7 +393,7 @@ struct phy *phy_get(struct device *dev, const char *string)
if (dev-of_node) {
index = of_property_match_string(dev-of_node, phy-names,
string);
- phy = of_phy_get(dev, index);
+ phy = _of_phy_get(dev-of_node, index);
if (IS_ERR(phy)) {
dev_err(dev, unable to find phy\n);
return phy;
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 6d72269..169f572 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -131,6 +131,7 @@ struct phy *phy_get(struct device *dev, const char *string);
struct phy *devm_phy_get(struct device *dev, const char *string);
void phy_put(struct phy *phy);
void devm_phy_put(struct device *dev, struct phy *phy);
+struct phy *of_phy_get(struct device_node *np, int index);
struct phy *of_phy_simple_xlate(struct device *dev,
struct of_phandle_args *args);
struct phy *phy_create(struct device *dev, const struct phy_ops *ops,
--
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/2] phy: core: Add devm_of_phy_get to phy-core
Adding devm_of_phy_get will allow to get phys by supplying the
device_node instead of by name.
Signed-off-by: Kamil Debski k.deb...@samsung.com
---
It seems that my git send-email is playing up and sent the previous emails
without from. This is a resend. Sorry for any confusion.
---
drivers/phy/phy-core.c | 31 +++
include/linux/phy/phy.h |2 ++
2 files changed, 33 insertions(+)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index f0dbd42..661f7ab 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -446,6 +446,37 @@ struct phy *devm_phy_get(struct device *dev, const char
*string)
EXPORT_SYMBOL_GPL(devm_phy_get);
/**
+ * devm_of_phy_get() - lookup and obtain a reference to a phy.
+ * @dev: device that requests this phy
+ * @np: node containing the phy
+ * @index: the index of the phy
+ *
+ * Gets the phy using phy_get(), and associates a device with it using
+ * devres. On driver detach, release function is invoked on the devres data,
+ * then, devres data is freed.
+ */
+struct phy *devm_of_phy_get(struct device *dev, struct device_node *np,
+ int index)
+{
+ struct phy **ptr, *phy;
+
+ ptr = devres_alloc(devm_phy_release, sizeof(*ptr), GFP_KERNEL);
+ if (!ptr)
+ return ERR_PTR(-ENOMEM);
+
+ phy = of_phy_get(np, index);
+ if (!IS_ERR(phy)) {
+ *ptr = phy;
+ devres_add(dev, ptr);
+ } else {
+ devres_free(ptr);
+ }
+
+ return phy;
+}
+EXPORT_SYMBOL_GPL(devm_of_phy_get);
+
+/**
* phy_create() - create a new phy
* @dev: device that is creating the new phy
* @ops: function pointers for performing phy operations
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 169f572..db36d81 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -129,6 +129,8 @@ int phy_power_on(struct phy *phy);
int phy_power_off(struct phy *phy);
struct phy *phy_get(struct device *dev, const char *string);
struct phy *devm_phy_get(struct device *dev, const char *string);
+struct phy *devm_of_phy_get(struct device *dev, struct device_node *np,
+ int index);
void phy_put(struct phy *phy);
void devm_phy_put(struct device *dev, struct phy *phy);
struct phy *of_phy_get(struct device_node *np, int index);
--
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 RESEND v2 1/2] phy: core: Add an exported of_phy_get function
Previously the of_phy_get function took a struct device * and
was declared static. It was impossible to call it from
another driver and thus it was impossible to get phy defined
for a given node. The old function was renamed to _of_phy_get
and was left for internal use. of_phy_get function was added
and it was exported. The function enables to get a phy for
a given device tree node.
Signed-off-by: Kamil Debski k.deb...@samsung.com
---
It seems that my git send-email is playing up and sent the previous emails
without from. This is a resend. Sorry for any confusion.
---
drivers/phy/phy-core.c | 41 -
include/linux/phy/phy.h |1 +
2 files changed, 33 insertions(+), 9 deletions(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 03cf8fb..f0dbd42 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -240,8 +240,8 @@ out:
EXPORT_SYMBOL_GPL(phy_power_off);
/**
- * of_phy_get() - lookup and obtain a reference to a phy by phandle
- * @dev: device that requests this phy
+ * _of_phy_get() - lookup and obtain a reference to a phy by phandle
+ * @np: device_node for which to get the phy
* @index: the index of the phy
*
* Returns the phy associated with the given phandle value,
@@ -250,20 +250,17 @@ EXPORT_SYMBOL_GPL(phy_power_off);
* not yet loaded. This function uses of_xlate call back function provided
* while registering the phy_provider to find the phy instance.
*/
-static struct phy *of_phy_get(struct device *dev, int index)
+static struct phy *_of_phy_get(struct device_node *np, int index)
{
int ret;
struct phy_provider *phy_provider;
struct phy *phy = NULL;
struct of_phandle_args args;
- ret = of_parse_phandle_with_args(dev-of_node, phys, #phy-cells,
+ ret = of_parse_phandle_with_args(np, phys, #phy-cells,
index, args);
- if (ret) {
- dev_dbg(dev, failed to get phy in %s node\n,
- dev-of_node-full_name);
+ if (ret)
return ERR_PTR(-ENODEV);
- }
mutex_lock(phy_provider_mutex);
phy_provider = of_phy_provider_lookup(args.np);
@@ -283,6 +280,32 @@ err0:
}
/**
+ * of_phy_get() - lookup and obtain a reference to a phy using a device_node.
+ * @np: device_node for which to get the phy
+ * @index: the index of the phy
+ *
+ * Returns the phy driver, after getting a refcount to it; or
+ * -ENODEV if there is no such phy. The caller is responsible for
+ * calling phy_put() to release that count.
+ */
+struct phy *of_phy_get(struct device_node *np, int index)
+{
+ struct phy *phy = NULL;
+
+ phy = _of_phy_get(np, index);
+ if (IS_ERR(phy))
+ return phy;
+
+ if (!try_module_get(phy-ops-owner))
+ return ERR_PTR(-EPROBE_DEFER);
+
+ get_device(phy-dev);
+
+ return phy;
+}
+EXPORT_SYMBOL_GPL(of_phy_get);
+
+/**
* phy_put() - release the PHY
* @phy: the phy returned by phy_get()
*
@@ -370,7 +393,7 @@ struct phy *phy_get(struct device *dev, const char *string)
if (dev-of_node) {
index = of_property_match_string(dev-of_node, phy-names,
string);
- phy = of_phy_get(dev, index);
+ phy = _of_phy_get(dev-of_node, index);
if (IS_ERR(phy)) {
dev_err(dev, unable to find phy\n);
return phy;
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 6d72269..169f572 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -131,6 +131,7 @@ struct phy *phy_get(struct device *dev, const char *string);
struct phy *devm_phy_get(struct device *dev, const char *string);
void phy_put(struct phy *phy);
void devm_phy_put(struct device *dev, struct phy *phy);
+struct phy *of_phy_get(struct device_node *np, int index);
struct phy *of_phy_simple_xlate(struct device *dev,
struct of_phandle_args *args);
struct phy *phy_create(struct device *dev, const struct phy_ops *ops,
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH 1/9] phy: core: Change the way of_phy_get is called
Hi,
From: Kishon Vijay Abraham I [mailto:kis...@ti.com]
Sent: Monday, December 09, 2013 8:23 AM
On Friday 06 December 2013 04:22 PM, Kamil Debski wrote:
Hi,
From: Kishon Vijay Abraham I [mailto:kis...@ti.com]
Sent: Friday, December 06, 2013 6:31 AM
Hi,
On Thursday 05 December 2013 05:59 PM, Kamil Debski wrote:
Previously the of_phy_get function took a struct device * and was
declared static. It was impossible to call it from another driver
and thus it was impossible to get phy defined
It was never intended to be called from other drivers. What's up
with
the wrapper of of_phy_get, phy_get()/devm_phy_get()? Why isn't that
enough?
Implementing support for multiple phys in the ehci driver is a bit
tricky.
Especially when we want to do it right. Please have a look at this
part of the dts file:
+ehci@1258 {
+compatible = samsung,exynos4210-ehci;
+reg = 0x1258 0x2;
+interrupts = 0 70 0;
+clocks = clock 304, clock 305;
+clock-names = usbhost, otg;
+status = disabled;
+#address-cells = 1;
+#size-cells = 0;
+port@0 {
+reg = 0;
+phys = usb2phy 1;
+phy-names = host;
+status = disabled;
+};
+port@1 {
+reg = 1;
+phys = usb2phy 2;
+phy-names = hsic0;
+status = disabled;
+};
+port@2 {
+reg = 2;
+phys = usb2phy 3;
+phy-names = hsic1;
+status = disabled;
+};
+};
With the above we have a clear specification of ports and their
respective phys. But to do this properly the ehci driver has to
iterate over port nodes. It is much easier to use devm_of_phy_get by
giving the node as its argument.
I see. There are a couple of more things we do in the wrapper that gets
missed while exporting of_phy_get (get_device and try_module_get). You
might want to re-work that one.
Thank you for the review. I have just sent an updated version of the core
patches.
Best wishes,
--
Kamil Debski
Samsung RD Institute Poland
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] hso: fix problem with wrong status code sent by OPTION GTM601 during RING indication
Hi,
Am 02.10.2013 um 09:00 schrieb Dr. H. Nikolaus Schaller:
Hi Jan,
we are using a GTM601 modem (Firmware 1.7) for a while and have spotted an
issue that under some conditions the modem sends a packed wIndex over USB
that is rejected by the hso driver making troubles afterwards. Not rejecting
makes
it work fine.
BR,
Nikolaus Schaller
---
From f5c7e15b61f2ce4fe3105ff914f6bfaf5d74af0d Mon Sep 17 00:00:00 2001
From: H. Nikolaus Schaller h...@goldelico.com
Date: Thu, 15 Nov 2012 14:40:57 +0100
Subject: [PATCH 1/1] hso: fix problem with wrong status code sent by OPTION
GTM601 during RING indication
It has been observed that the GTM601 with 1.7 firmware sometimes sends a value
wIndex that has bit 0x04 set instead of being reset as the code expects. So we
mask it for the error check.
See http://lists.goldelico.com/pipermail/gta04-owner/2012-February/001643.html
Signed-off-by: NeilBrown ne...@suse.de
Signed-off-by: H. Nikolaus Schaller h...@goldelico.de
---
drivers/net/usb/hso.c |3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/drivers/net/usb/hso.c b/drivers/net/usb/hso.c
index cba1d46..d146e26 100644
--- a/drivers/net/usb/hso.c
+++ b/drivers/net/usb/hso.c
@@ -1503,7 +1503,8 @@ static void tiocmget_intr_callback(struct urb *urb)
if (serial_state_notification-bmRequestType != BM_REQUEST_TYPE ||
serial_state_notification-bNotification != B_NOTIFICATION ||
le16_to_cpu(serial_state_notification-wValue) != W_VALUE ||
- le16_to_cpu(serial_state_notification-wIndex) != W_INDEX ||
+ (le16_to_cpu(serial_state_notification-wIndex) ~0x4) !=
+ W_INDEX ||
le16_to_cpu(serial_state_notification-wLength) != W_LENGTH) {
dev_warn(usb-dev,
hso received invalid serial state notification\n);
--
1.7.7.4
I have found this (better) proposal by OPTION, but wonder what did happen to it.
It neither shows in mainline 3.13-rc nor linux-next:
https://lkml.org/lkml/2013/10/9/263
BR,
Nikolaus
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND v2 1/2] phy: core: Add an exported of_phy_get function
Hi,
On Friday 13 December 2013 07:32 PM, Kamil Debski wrote:
Previously the of_phy_get function took a struct device * and
was declared static. It was impossible to call it from
another driver and thus it was impossible to get phy defined
for a given node. The old function was renamed to _of_phy_get
and was left for internal use. of_phy_get function was added
and it was exported. The function enables to get a phy for
a given device tree node.
Signed-off-by: Kamil Debski k.deb...@samsung.com
---
It seems that my git send-email is playing up and sent the previous emails
without from. This is a resend. Sorry for any confusion.
---
drivers/phy/phy-core.c | 41 -
include/linux/phy/phy.h |1 +
2 files changed, 33 insertions(+), 9 deletions(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c
index 03cf8fb..f0dbd42 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -240,8 +240,8 @@ out:
EXPORT_SYMBOL_GPL(phy_power_off);
/**
- * of_phy_get() - lookup and obtain a reference to a phy by phandle
- * @dev: device that requests this phy
+ * _of_phy_get() - lookup and obtain a reference to a phy by phandle
+ * @np: device_node for which to get the phy
* @index: the index of the phy
*
* Returns the phy associated with the given phandle value,
@@ -250,20 +250,17 @@ EXPORT_SYMBOL_GPL(phy_power_off);
* not yet loaded. This function uses of_xlate call back function provided
* while registering the phy_provider to find the phy instance.
*/
-static struct phy *of_phy_get(struct device *dev, int index)
+static struct phy *_of_phy_get(struct device_node *np, int index)
{
int ret;
struct phy_provider *phy_provider;
struct phy *phy = NULL;
struct of_phandle_args args;
- ret = of_parse_phandle_with_args(dev-of_node, phys, #phy-cells,
+ ret = of_parse_phandle_with_args(np, phys, #phy-cells,
index, args);
- if (ret) {
- dev_dbg(dev, failed to get phy in %s node\n,
- dev-of_node-full_name);
+ if (ret)
return ERR_PTR(-ENODEV);
- }
mutex_lock(phy_provider_mutex);
phy_provider = of_phy_provider_lookup(args.np);
@@ -283,6 +280,32 @@ err0:
}
/**
+ * of_phy_get() - lookup and obtain a reference to a phy using a device_node.
+ * @np: device_node for which to get the phy
+ * @index: the index of the phy
Would be better if the user can get the PHY by string instead of index.
+ *
+ * Returns the phy driver, after getting a refcount to it; or
+ * -ENODEV if there is no such phy. The caller is responsible for
+ * calling phy_put() to release that count.
+ */
+struct phy *of_phy_get(struct device_node *np, int index)
+{
+ struct phy *phy = NULL;
+
+ phy = _of_phy_get(np, index);
+ if (IS_ERR(phy))
dev_err here?
Thanks
Kishon
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Bug#704242: Driver for PL-2303 HX not working
Hello together, is there anything new for the PL-2303 HX? It would be fine if it could work like in the past. Of course for Linux there are good alternatives. Here is a new one with the CP2102 suffering easy supply voltage: http://www.ebay.de/itm/110954294607 Best regards Karsten Am 24.06.2013 21:00, schrieb Greg KH: On Mon, Jun 24, 2013 at 12:38:17PM -0400, Aric Fedida wrote: I will gladly ship you one. Give me address details, and I'll go to the post office to send it. I'll send it off-list, thanks. Personally, I have resolved to abandon the PL2303 chip, and move to FTDI instead. But for the sake of other Linux users who might buy that adapter by mistake, I am willing to donate the hardware to an able kernel hacker :-) Yes, I recommend the ftdi devices as well, the specs are availble, and the driver seems to work better because of it. thanks, greg k-h -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH RESEND v2 1/2] phy: core: Add an exported of_phy_get function
Hi Kishon,
From: Kishon Vijay Abraham I [mailto:kis...@ti.com]
Sent: Friday, December 13, 2013 3:45 PM
Hi,
On Friday 13 December 2013 07:32 PM, Kamil Debski wrote:
Previously the of_phy_get function took a struct device * and was
declared static. It was impossible to call it from another driver and
thus it was impossible to get phy defined for a given node. The old
function was renamed to _of_phy_get and was left for internal use.
of_phy_get function was added and it was exported. The function
enables to get a phy for a given device tree node.
Signed-off-by: Kamil Debski k.deb...@samsung.com
---
It seems that my git send-email is playing up and sent the previous
emails without from. This is a resend. Sorry for any confusion.
---
drivers/phy/phy-core.c | 41 -
include/linux/phy/phy.h |1 +
2 files changed, 33 insertions(+), 9 deletions(-)
diff --git a/drivers/phy/phy-core.c b/drivers/phy/phy-core.c index
03cf8fb..f0dbd42 100644
--- a/drivers/phy/phy-core.c
+++ b/drivers/phy/phy-core.c
@@ -240,8 +240,8 @@ out:
EXPORT_SYMBOL_GPL(phy_power_off);
/**
- * of_phy_get() - lookup and obtain a reference to a phy by phandle
- * @dev: device that requests this phy
+ * _of_phy_get() - lookup and obtain a reference to a phy by phandle
+ * @np: device_node for which to get the phy
* @index: the index of the phy
*
* Returns the phy associated with the given phandle value, @@
-250,20 +250,17 @@ EXPORT_SYMBOL_GPL(phy_power_off);
* not yet loaded. This function uses of_xlate call back function
provided
* while registering the phy_provider to find the phy instance.
*/
-static struct phy *of_phy_get(struct device *dev, int index)
+static struct phy *_of_phy_get(struct device_node *np, int index)
{
int ret;
struct phy_provider *phy_provider;
struct phy *phy = NULL;
struct of_phandle_args args;
- ret = of_parse_phandle_with_args(dev-of_node, phys, #phy-
cells,
+ ret = of_parse_phandle_with_args(np, phys, #phy-cells,
index, args);
- if (ret) {
- dev_dbg(dev, failed to get phy in %s node\n,
- dev-of_node-full_name);
+ if (ret)
return ERR_PTR(-ENODEV);
- }
mutex_lock(phy_provider_mutex);
phy_provider = of_phy_provider_lookup(args.np); @@ -283,6 +280,32
@@
err0:
}
/**
+ * of_phy_get() - lookup and obtain a reference to a phy using a
device_node.
+ * @np: device_node for which to get the phy
+ * @index: the index of the phy
Would be better if the user can get the PHY by string instead of index.
Ok, this sounds doable. I will add of_phy_get_by_name analogous to the clk
framework.
+ *
+ * Returns the phy driver, after getting a refcount to it; or
+ * -ENODEV if there is no such phy. The caller is responsible for
+ * calling phy_put() to release that count.
+ */
+struct phy *of_phy_get(struct device_node *np, int index) {
+ struct phy *phy = NULL;
+
+ phy = _of_phy_get(np, index);
+ if (IS_ERR(phy))
dev_err here?
dev_err requires a dev, so it is not possible with the arguments used.
And we do not want to add any extra parameters to keep the function call
analogous to other of_*_get (as for example of_clk_get).
Best wishes,
--
Kamil Debski
Samsung RD Institute Poland
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 3/3] usb/xhci-plat: remove unnecessary #ifdef checks for CONFIG_PM_SLEEP
On Fri, Dec 13, 2013 at 09:19:34AM +0100, Ulf Hansson wrote:
On 13 December 2013 06:18, David Cohen david.a.co...@linux.intel.com wrote:
From: Santosh Shilimkar santosh.shilim...@ti.com
Drivers using SET_*_PM_OPS() no longer need to #ifdef for CONFIG_PM_*
So, let's remove the unnecessary #ifdef's.
Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com
Signed-off-by: David Cohen david.a.co...@linux.intel.com
---
drivers/usb/host/xhci-plat.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index d9c169f470d3..b1d93c344e04 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -197,7 +197,6 @@ static int xhci_plat_remove(struct platform_device *dev)
return 0;
}
-#ifdef CONFIG_PM
I think you can solve this in another way.
As a start, I would suggest to change above to CONFIG_PM_SLEEP
static int xhci_plat_suspend(struct device *dev)
{
struct usb_hcd *hcd = dev_get_drvdata(dev);
@@ -217,10 +216,6 @@ static int xhci_plat_resume(struct device *dev)
static const struct dev_pm_ops xhci_plat_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(xhci_plat_suspend, xhci_plat_resume)
};
-#define DEV_PM_OPS (xhci_plat_pm_ops)
-#else
-#define DEV_PM_OPS NULL
-#endif /* CONFIG_PM */
Remove the use of DEV_PM_OPS define. Instead use below macro and
outside #ifdef CONFIG_PM_SLEEP.
static SIMPLE_DEV_PM_OPS(xhci_plat_pm_ops, xhci_plat_suspend,
xhci_plat_resume)
That should do the trick I believe, without the need for changing the
existing SET_SYSTEM_SLEEP_PM_OPS macro in patch1.
Yes, it does. That matches my suggestion in this e-mail:
http://marc.info/?l=linux-kernelm=138690490016503w=2
But I personally don't like #ifdef's. In this case, SET_*_PM_OPS()
macros handle #ifdef's when setting the callbacks. My intention is to
extend it to callbacks' implementation too making the code cleaner
Br, David Cohen
Kind regards
Ulf Hansson
#ifdef CONFIG_OF
static const struct of_device_id usb_xhci_of_match[] = {
@@ -235,7 +230,7 @@ static struct platform_driver usb_xhci_driver = {
.remove = xhci_plat_remove,
.driver = {
.name = xhci-hcd,
- .pm = DEV_PM_OPS,
+ .pm = xhci_plat_pm_ops,
.of_match_table = of_match_ptr(usb_xhci_of_match),
},
};
--
1.8.4.2
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH v2 1/7] usb: dwc3: keystone: add basic PM support
Hello -Original Message- From: Shilimkar, Santosh Sent: Thursday, December 12, 2013 7:29 PM To: Balbi, Felipe Cc: Linux USB Mailing List; kgene@samsung.com; Linux ARM Kernel Mailing List; linux-samsung-...@vger.kernel.org; Linux OMAP Mailing List; Kwok, WingMan Subject: Re: [PATCH v2 1/7] usb: dwc3: keystone: add basic PM support On Thursday 12 December 2013 04:45 PM, Felipe Balbi wrote: A bare-minimum PM implementation which will server as building block for more complex s/server/serve ;-) PM implementation in the future. At the least will not leave clocks on unnecessarily when e.g. a user write mem to /sys/power/state. Signed-off-by: Felipe Balbi ba...@ti.com --- improve error path a little bit. We will test this out. Thanks for the patch Felipe. I have tested the patch and the keystone usb driver continues to function, though I can't test suspend at this time as the rest of the system does not that functionality yet. Thanks WingMan -- To unsubscribe from this list: send the line unsubscribe 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: WG: [PATCH] usb: musb: omap2430: fix occasional musb breakage on boot
On 13.12.2013 13:34, Grazvydas Ignotas wrote:
On Thu, Dec 12, 2013 at 11:29 PM, Andreas Naumann d...@andin.de wrote:
Hi Grazvydas,
Von: Grazvydas Ignotas [mailto:nota...@gmail.com]
Gesendet: Donnerstag, 12. Dezember 2013 01:21
An: linux-usb@vger.kernel.org
Cc: Felipe Balbi; linux-o...@vger.kernel.org; Naumann Andreas; Grazvydas
Ignotas; sta...@vger.kernel.org
Betreff: [PATCH] usb: musb: omap2430: fix occasional musb breakage on boot
This is a hard to reproduce problem which leads to non-functional
USB-OTG port in 0.1%-1% of all boots. Tracked it down to commit
e25bec160158abe86c omap2+: save and restore OTG_INTERFSEL,
which introduces save/restore of OTG_INTERFSEL over suspend.
Since the resume function is also called early in driver init, it uses a
non-initialized value (which is 0 and a non-supported setting in DM37xx
for INTERFSEL). Shortly after the correct value is set. Apparently this
works most time, but not always.
Fix it by not writing the value on runtime resume if it is 0
(0 should never be saved in the context as it's invalid value,
so we use it as an indicator that context hasn't been saved yet).
This issue was originally found by Andreas Naumann:
http://marc.info/?l=linux-usbm=138562574719654w=2
Reported-and-bisected-by: Andreas Naumann anaum...@ultratronik.de
Signed-off-by: Grazvydas Ignotas nota...@gmail.com
Cc: sta...@vger.kernel.org
---
This is a regression from 3.2, so should go to -rc and stable, IMO.
It's really annoying issue if you want to have a stable OTG behavior,
I've burned quite a lot of time on it myself over a year ago and gave up
eventually. Good thing Andreas finally found it, many thanks to him!
drivers/usb/musb/omap2430.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
index 2a408cd..737b3da 100644
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -672,7 +672,8 @@ static int omap2430_runtime_resume(struct device *dev)
if (musb) {
omap2430_low_level_init(musb);
- musb_writel(musb-mregs, OTG_INTERFSEL,
+ if (musb-context.otg_interfsel != 0)
+ musb_writel(musb-mregs, OTG_INTERFSEL,
musb-context.otg_interfsel);
phy_power_on(musb-phy);
}
Oh, easy way out. I like it but I've also been thinking about your comment
on my original post, which was that initializing otg_interfsel to the PHYSEL
bits only might be dangerous because we cant be sure that there are other
bits in the register.
However, isnt assuming that 0 is invalid on all OMAPs just as dangerous?
Well I was trying to do a minimal fix so that it could be suitable for
merging to stable kernels. But yes you're right, I've just checked
OMAP4 TRM and 0 is actually valid value there..
After thinking about my patch again, I would propose to change otg_interfsel
into otg_physel and read-modify-write only those bits in resume() as you
suggested in your first answer. That way I could discard the problematic
first read in probe() while leaving other bits untouched. If you agree I
post a patch for this tomorrow.
Hmm I don't know about that, this would be inconsistent with what all
other OMAP drivers do. Maybe we should do what musb_core.c does just
Ok, thats cool.
to be consistent and add a similar comment. Only the static variable
could be avoided in favor of struct omap2430_glue member.
Whats wrong with the static?
cheers,
Andreas
Gražvydas
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: XHCI - USB3 HDD not recognised
On Fri, Dec 13, 2013 at 02:31:47PM +, Chris Clayton wrote: Hi, Hi Chris, Thanks for the bug report! Firstly, I'm not subscribed, so please cc me on any replies. I see the problem I describe below on 3.12.[0..5] and on the current 3.13 development kernel, including a kernel pulled from Linus' tree just a few minutes ago. The diagnostics below and the config file attached are from 3.12.5. I can easily repeat on 3.13 if that would be more useful. My Fujitsu Lifebook AH531 laptop has an expresscard slot and I bought an expresscard USB3.0 card. Has this expresscard worked on any older kernels? Or did you only try the new card on 3.12 and 3.13? When I insert the card, two new usb devices are added to the output of lsusb: Bus 004 Device 001: ID 1d6b:0003 Linux Foundation 3.0 root hub Bus 003 Device 001: ID 1d6b:0002 Linux Foundation 2.0 root hub and the related new output from dmesg is: [ 3139.294483] pci :02:00.0: [1b73:1100] type 00 class 0x0c0330 [ 3139.294582] pci :02:00.0: reg 0x10: [mem 0x-0x 64bit] [ 3139.294656] pci :02:00.0: reg 0x18: [mem 0x-0x0fff 64bit] [ 3139.294729] pci :02:00.0: reg 0x20: [mem 0x-0x0fff 64bit] [ 3139.294955] pci :02:00.0: supports D1 [ 3139.294958] pci :02:00.0: PME# supported from D0 D1 D3hot [ 3139.295098] pci :02:00.0: System wakeup disabled by ACPI [ 3139.302185] pci :02:00.0: BAR 0: assigned [mem 0xf0d0-0xf0d0 64bit] [ 3139.302243] pci :02:00.0: BAR 2: assigned [mem 0xf0d1-0xf0d10fff 64bit] [ 3139.302294] pci :02:00.0: BAR 4: assigned [mem 0xf0d11000-0xf0d11fff 64bit] [ 3139.302374] pci :02:00.0: no hotplug settings from platform [ 3139.302415] pcieport :00:1c.3: driver skip pci_set_master, fix it! [ 3139.302431] pci :02:00.0: enabling device ( - 0002) [ 3139.315124] xhci_hcd :02:00.0: xHCI Host Controller [ 3139.315139] xhci_hcd :02:00.0: new USB bus registered, assigned bus number 3 [ 3139.316242] usb usb3: New USB device found, idVendor=1d6b, idProduct=0002 [ 3139.316250] usb usb3: New USB device strings: Mfr=3, Product=2, SerialNumber=1 [ 3139.316254] usb usb3: Product: xHCI Host Controller [ 3139.316258] usb usb3: Manufacturer: Linux 3.12.5 xhci_hcd [ 3139.316261] usb usb3: SerialNumber: :02:00.0 [ 3139.316548] hub 3-0:1.0: USB hub found [ 3139.316738] hub 3-0:1.0: 4 ports detected [ 3139.317301] xhci_hcd :02:00.0: xHCI Host Controller [ 3139.317309] xhci_hcd :02:00.0: new USB bus registered, assigned bus number 4 [ 3139.317631] usb usb4: New USB device found, idVendor=1d6b, idProduct=0003 [ 3139.317637] usb usb4: New USB device strings: Mfr=3, Product=2, SerialNumber=1 [ 3139.317641] usb usb4: Product: xHCI Host Controller [ 3139.317644] usb usb4: Manufacturer: Linux 3.12.5 xhci_hcd [ 3139.317647] usb usb4: SerialNumber: :02:00.0 [ 3139.317896] hub 4-0:1.0: USB hub found [ 3139.318049] hub 4-0:1.0: 4 ports detected [snip] If I plug a USB2 device into the card, everything works fine. If however, I plug a USB3 HDD into the card, the drive appears not to be recognised. Is this your only USB 3.0 device? It would be useful to know if this is a device-specific issue, so if you have any other USB 3.0 devices, please try them as well. lsusb does not show the device at all and the only additions to the output from dmesg is one or more instances of: [ 3704.319656] xhci_hcd :02:00.0: no hotplug settings from platform Hmm, I think that message only appears when the PCI device is hot-plugged. It's odd that it would appear when you plug something into the USB 3.0 port. Are you sure you didn't bump the expresscard and cause a hot-remove? (Bumping the expresscard should have no impact on whether the USB 3.0 device is recognized or not.) The HDD works fine if plugged into a USB2 port. Let me know If I can provide any additional diagnostics, test patches, etc. Can you turn on CONFIG_USB_DEBUG on 3.13 (if it's not already enabled), and also run these two commands as root: cd /sys/kernel/debug/dynamic_debug echo -n 'module xhci_hcd =p' control That should enable xHCI driver debugging. Then hot-plug your expresscard, plug in your USB 3.0 device into the USB 3.0 port, and send me the resulting dmesg, starting from xHCI driver initialization. That will allow me to figure out whether the xHCI driver is receiving a port status change event for the USB 3.0 device hot-plug. Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: usb mass storage bug
On Thu, 12 Dec 2013, Felipe Balbi wrote:
Felipe:
Is the usb_enumerate_device_otg() routine really correct? It looks
like it will try to enable HNP whenever CONFIG_USB_OTG is set, even if
the root hub isn't OTG-capable.
The routine does this:
/* enable HNP before suspend, it's simpler */
if (port1 == bus-otg_port)
bus-b_hnp_enable = 1;
err = usb_control_msg(udev,
usb_sndctrlpipe(udev, 0),
USB_REQ_SET_FEATURE, 0,
bus-b_hnp_enable
? USB_DEVICE_B_HNP_ENABLE
: USB_DEVICE_A_ALT_HNP_SUPPORT,
0, NULL, 0, USB_CTRL_SET_TIMEOUT);
if (err 0) {
/* OTG MESSAGE: report errors here,
* customize to match your product.
*/
dev_info(udev-dev,
can't set HNP mode: %d\n,
err);
bus-b_hnp_enable = 0;
}
Maybe the usb_control_msg and the error check should be inside the
first if statement?
I don't think so, see below:
| static int usb_enumerate_device_otg(struct usb_device *udev)
| {
| int err = 0;
|
| #ifdef CONFIG_USB_OTG
first, we know OTG is enabled.
| /*
| * OTG-aware devices on OTG-capable root hubs may be able to use SRP,
| * to wake us after we've powered off VBUS; and HNP, switching roles
| * host to peripheral. The OTG descriptor helps figure this out.
| */
| if (!udev-bus-is_b_host
| udev-config
| udev-parent == udev-bus-root_hub) {
| struct usb_otg_descriptor *desc = NULL;
| struct usb_bus *bus = udev-bus;
|
| /* descriptor may appear anywhere in config */
| if (__usb_get_extra_descriptor (udev-rawdescriptors[0],
|
le16_to_cpu(udev-config[0].desc.wTotalLength),
| USB_DT_OTG, (void **) desc) == 0) {
| if (desc-bmAttributes USB_OTG_HNP) {
this check ensures the port supports HNP
No, it doesn't. It ensures that the _device_ supports HNP. It has
nothing to do with the host port.
| unsignedport1 = udev-portnum;
|
| dev_info(udev-dev,
| Dual-Role OTG device on %sHNP port\n,
| (port1 == bus-otg_port)
| ? : non-);
This is where the host port is tested.
|
| /* enable HNP before suspend, it's simpler */
| if (port1 == bus-otg_port)
| bus-b_hnp_enable = 1;
And here.
this check tells is just to help you choose between enabling HNP on a B
device, or telling the device that another root hub port supports HNP
| err = usb_control_msg(udev,
| usb_sndctrlpipe(udev, 0),
| USB_REQ_SET_FEATURE, 0,
| bus-b_hnp_enable
| ? USB_DEVICE_B_HNP_ENABLE
| : USB_DEVICE_A_ALT_HNP_SUPPORT,
so, based on this, we're just enabling HNP on the device, but HNP won't
kick in until the root hub port is suspended.
What if bus-otg_port is not equal to port1? Either the host supports
OTG on a different port or doesn't support it at all on this bus. Is
there any reason for enabling HNP on the device in those cases?
The point is that if this message fails (for example, if err is -EPIPE)
then device enumeration fails. Even if there was no reason to send
this message in the first place.
Alan Stern
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 6/9] usb: gadget: s3c-hsotg: get phy bus width from phy subsystem
Adds support for querying the phy bus width from the generic phy
subsystem. Configure UTMI bus width in GUSBCFG based on this value.
Signed-off-by: Matt Porter mpor...@linaro.org
Acked-by: Kishon Vijay Abraham I kis...@ti.com
---
drivers/usb/gadget/s3c-hsotg.c | 14 +-
drivers/usb/gadget/s3c-hsotg.h | 1 +
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index e9683c2..168aaa9 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -144,6 +144,7 @@ struct s3c_hsotg_ep {
* @regs: The memory area mapped for accessing registers.
* @irq: The IRQ number we are using
* @supplies: Definition of USB power supplies
+ * @phyif: PHY interface width
* @dedicated_fifos: Set if the hardware has dedicated IN-EP fifos.
* @num_of_eps: Number of available EPs (excluding EP0)
* @debug_root: root directrory for debugfs.
@@ -171,6 +172,7 @@ struct s3c_hsotg {
struct regulator_bulk_data supplies[ARRAY_SIZE(s3c_hsotg_supply_names)];
+ u32 phyif;
unsigned intdedicated_fifos:1;
unsigned char num_of_eps;
@@ -2276,7 +2278,7 @@ static void s3c_hsotg_core_init(struct s3c_hsotg *hsotg)
*/
/* set the PLL on, remove the HNP/SRP and set the PHY */
- writel(GUSBCFG_PHYIf16 | GUSBCFG_TOutCal(7) |
+ writel(hsotg-phyif | GUSBCFG_TOutCal(7) |
(0x5 10), hsotg-regs + GUSBCFG);
s3c_hsotg_init_fifo(hsotg);
@@ -3621,6 +3623,16 @@ static int s3c_hsotg_probe(struct platform_device *pdev)
goto err_supplies;
}
+ /* Set default UTMI width */
+ hsotg-phyif = GUSBCFG_PHYIf16;
+
+ /*
+* If using the generic PHY framework, check if the PHY bus
+* width is 8-bit and set the phyif appropriately.
+*/
+ if (hsotg-phy (phy_get_bus_width(phy) == 8))
+ hsotg-phyif = GUSBCFG_PHYIf8;
+
if (hsotg-phy)
phy_init(hsotg-phy);
diff --git a/drivers/usb/gadget/s3c-hsotg.h b/drivers/usb/gadget/s3c-hsotg.h
index d650b12..85f549f 100644
--- a/drivers/usb/gadget/s3c-hsotg.h
+++ b/drivers/usb/gadget/s3c-hsotg.h
@@ -55,6 +55,7 @@
#define GUSBCFG_HNPCap (1 9)
#define GUSBCFG_SRPCap (1 8)
#define GUSBCFG_PHYIf16(1 3)
+#define GUSBCFG_PHYIf8 (0 3)
#define GUSBCFG_TOutCal_MASK (0x7 0)
#define GUSBCFG_TOutCal_SHIFT (0)
#define GUSBCFG_TOutCal_LIMIT (0x7)
--
1.8.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 8/9] phy: add Broadcom Kona USB2 PHY driver
Add a driver for the internal Broadcom Kona USB 2.0 PHY found
on the BCM281xx family of SoCs.
Signed-off-by: Matt Porter mpor...@linaro.org
---
drivers/phy/Kconfig | 6 ++
drivers/phy/Makefile| 1 +
drivers/phy/phy-bcm-kona-usb2.c | 158
3 files changed, 165 insertions(+)
create mode 100644 drivers/phy/phy-bcm-kona-usb2.c
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index a344f3d..2e87fa8 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -51,4 +51,10 @@ config PHY_EXYNOS_DP_VIDEO
help
Support for Display Port PHY found on Samsung EXYNOS SoCs.
+config BCM_KONA_USB2_PHY
+ tristate Broadcom Kona USB2 PHY Driver
+ depends on GENERIC_PHY
+ help
+ Enable this to support the Broadcom Kona USB 2.0 PHY.
+
endmenu
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index d0caae9..c447f1a 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -3,6 +3,7 @@
#
obj-$(CONFIG_GENERIC_PHY) += phy-core.o
+obj-$(CONFIG_BCM_KONA_USB2_PHY)+= phy-bcm-kona-usb2.o
obj-$(CONFIG_PHY_EXYNOS_DP_VIDEO) += phy-exynos-dp-video.o
obj-$(CONFIG_PHY_EXYNOS_MIPI_VIDEO)+= phy-exynos-mipi-video.o
obj-$(CONFIG_OMAP_USB2)+= phy-omap-usb2.o
diff --git a/drivers/phy/phy-bcm-kona-usb2.c b/drivers/phy/phy-bcm-kona-usb2.c
new file mode 100644
index 000..0046781
--- /dev/null
+++ b/drivers/phy/phy-bcm-kona-usb2.c
@@ -0,0 +1,158 @@
+/*
+ * phy-bcm-kona-usb2.c - Broadcom Kona USB2 Phy Driver
+ *
+ * Copyright (C) 2013 Linaro Limited
+ * Matt Porter mpor...@linaro.org
+ *
+ * This software is licensed under the terms of the GNU General Public
+ * License version 2, as published by the Free Software Foundation, and
+ * may be copied, distributed, and modified under those terms.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include linux/clk.h
+#include linux/delay.h
+#include linux/err.h
+#include linux/io.h
+#include linux/module.h
+#include linux/of.h
+#include linux/phy/phy.h
+#include linux/platform_device.h
+
+#define OTGCTL (0)
+#define OTGCTL_OTGSTAT2BIT(31)
+#define OTGCTL_OTGSTAT1BIT(30)
+#define OTGCTL_PRST_N_SW BIT(11)
+#define OTGCTL_HRESET_NBIT(10)
+#define OTGCTL_UTMI_LINE_STATE1BIT(9)
+#define OTGCTL_UTMI_LINE_STATE0BIT(8)
+
+#define P1CTL (8)
+#define P1CTL_SOFT_RESET BIT(1)
+#define P1CTL_NON_DRIVING BIT(0)
+
+struct bcm_kona_usb {
+ void __iomem *regs;
+};
+
+static void bcm_kona_usb_phy_power(struct bcm_kona_usb *phy, int on)
+{
+ u32 val;
+
+ val = readl(phy-regs + OTGCTL);
+ if (on) {
+ /* Configure and power PHY */
+ val = ~(OTGCTL_OTGSTAT2 | OTGCTL_OTGSTAT1 |
+OTGCTL_UTMI_LINE_STATE1 | OTGCTL_UTMI_LINE_STATE0);
+ val |= OTGCTL_PRST_N_SW | OTGCTL_HRESET_N;
+ } else {
+ val = ~(OTGCTL_PRST_N_SW | OTGCTL_HRESET_N);
+ }
+ writel(val, phy-regs + OTGCTL);
+}
+
+static int bcm_kona_usb_phy_init(struct phy *gphy)
+{
+ struct bcm_kona_usb *phy = phy_get_drvdata(gphy);
+ u32 val;
+
+ /* Soft reset PHY */
+ val = readl(phy-regs + P1CTL);
+ val = ~P1CTL_NON_DRIVING;
+ val |= P1CTL_SOFT_RESET;
+ writel(val, phy-regs + P1CTL);
+ writel(val ~P1CTL_SOFT_RESET, phy-regs + P1CTL);
+ /* Reset needs to be asserted for 2ms */
+ mdelay(2);
+ writel(val | P1CTL_SOFT_RESET, phy-regs + P1CTL);
+
+ return 0;
+}
+
+static int bcm_kona_usb_phy_power_on(struct phy *gphy)
+{
+ struct bcm_kona_usb *phy = phy_get_drvdata(gphy);
+
+ bcm_kona_usb_phy_power(phy, 1);
+
+ return 0;
+}
+
+static int bcm_kona_usb_phy_power_off(struct phy *gphy)
+{
+ struct bcm_kona_usb *phy = phy_get_drvdata(gphy);
+
+ bcm_kona_usb_phy_power(phy, 0);
+
+ return 0;
+}
+
+static struct phy_ops ops = {
+ .init = bcm_kona_usb_phy_init,
+ .power_on = bcm_kona_usb_phy_power_on,
+ .power_off = bcm_kona_usb_phy_power_off,
+ .owner = THIS_MODULE,
+};
+
+static int bcm_kona_usb2_probe(struct platform_device *pdev)
+{
+ struct device *dev = pdev-dev;
+ struct bcm_kona_usb *phy;
+ struct resource *res;
+ struct phy *gphy;
+ struct phy_provider *phy_provider;
+
+ phy = devm_kzalloc(dev, sizeof(*phy), GFP_KERNEL);
+ if (!phy)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ phy-regs = devm_ioremap_resource(pdev-dev, res);
+ if (IS_ERR(phy-regs))
+ return
Re: some question about period scheduleing
On Fri, 13 Dec 2013, vichy wrote: hi 2013/12/2 Alan Stern st...@rowland.harvard.edu: On Sun, 1 Dec 2013, vichy wrote: hi Alan: 2013/12/1 Alan Stern st...@rowland.harvard.edu: On Fri, 29 Nov 2013, vichy wrote: hi all: My connection like below ehci -- high speed hub - full speed device I have some questions about period scheduling. 1. when creating qh for full speed device, why we set EHCI_TUNE_RL_TT. Are you asking why EHCI_TUNE_RL_TT is equal to 0? I don't know -- it looks like a mistake. Do you find that changing it to 3 makes any difference? Yes, when I change EHCI_TUNE_RL_TT as not 0. The transaction can keep going. But if EHCI_TUNE_RL_TT is 0, the transfer doesn't work? No. the transaction will stop since device return Nak. I copy the usb traffic log for your reference. The device did not return NAK. The NAK was sent by the high-speed hub the device was attached to. usually device will not return NAK in setup token. but in my case, it did. When EHCI_TUNE_RL_TT is set to 0, the controller should never stop retrying the transfer. That's what 0 means here -- 3 means stop (temporarily) after 3 attempts, but 0 means never stop. This sounds like a bug in your EHCI hardware. What type of controller are you using? Anyway, I don't mind changing EHCI_TUNE_RL_TT to 3. Would you like to submit a patch? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 9/9] ARM: dts: add usb udc support to bcm281xx
Adds USB OTG/PHY and clock support to BCM281xx and enables
UDC support on the bcm11351-brt and bcm28155-ap boards.
Signed-off-by: Matt Porter mpor...@linaro.org
Reviewed-by: Markus Mayer markus.ma...@linaro.org
Reviewed-by: Tim Kryger tim.kry...@linaro.org
---
arch/arm/boot/dts/bcm11351-brt.dts | 6 ++
arch/arm/boot/dts/bcm11351.dtsi| 18 ++
arch/arm/boot/dts/bcm28155-ap.dts | 8
3 files changed, 32 insertions(+)
diff --git a/arch/arm/boot/dts/bcm11351-brt.dts
b/arch/arm/boot/dts/bcm11351-brt.dts
index 23cd16d..396b704 100644
--- a/arch/arm/boot/dts/bcm11351-brt.dts
+++ b/arch/arm/boot/dts/bcm11351-brt.dts
@@ -44,5 +44,11 @@
status = okay;
};
+ usbotg: usb@3f12 {
+ status = okay;
+ };
+ usbphy: usb-phy@3f13 {
+ status = okay;
+ };
};
diff --git a/arch/arm/boot/dts/bcm11351.dtsi b/arch/arm/boot/dts/bcm11351.dtsi
index 1246885..0fbb455 100644
--- a/arch/arm/boot/dts/bcm11351.dtsi
+++ b/arch/arm/boot/dts/bcm11351.dtsi
@@ -243,4 +243,22 @@
#clock-cells = 0;
};
};
+
+ usbotg: usb@3f12 {
+ compatible = snps,dwc2;
+ reg = 0x3f12 0x1;
+ interrupts = GIC_SPI 47 IRQ_TYPE_LEVEL_HIGH;
+ clocks = usb_otg_ahb_clk;
+ clock-names = otg;
+ phys = usbphy;
+ phy-names = usb2-phy;
+ status = disabled;
+ };
+
+ usbphy: usb-phy@3f13 {
+ compatible = brcm,kona-usb2-phy;
+ reg = 0x3f13 0x28;
+ #phy-cells = 0;
+ status = disabled;
+ };
};
diff --git a/arch/arm/boot/dts/bcm28155-ap.dts
b/arch/arm/boot/dts/bcm28155-ap.dts
index 08e47c2..a3bc436 100644
--- a/arch/arm/boot/dts/bcm28155-ap.dts
+++ b/arch/arm/boot/dts/bcm28155-ap.dts
@@ -43,4 +43,12 @@
cd-gpios = gpio 14 0;
status = okay;
};
+
+ usbotg: usb@3f12 {
+ status = okay;
+ };
+
+ usbphy: usb-phy@3f13 {
+ status = okay;
+ };
};
--
1.8.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 5/9] usb: gadget: s3c-hsotg: use generic phy_init()/phy_exit() support
If a generic phy is present, call phy_init()/phy_exit(). This supports generic phys that must be soft reset before power on. Signed-off-by: Matt Porter mpor...@linaro.org Acked-by: Kishon Vijay Abraham I kis...@ti.com --- drivers/usb/gadget/s3c-hsotg.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c index 7c5d8bd..e9683c2 100644 --- a/drivers/usb/gadget/s3c-hsotg.c +++ b/drivers/usb/gadget/s3c-hsotg.c @@ -3621,6 +3621,9 @@ static int s3c_hsotg_probe(struct platform_device *pdev) goto err_supplies; } + if (hsotg-phy) + phy_init(hsotg-phy); + /* usb phy enable */ s3c_hsotg_phy_enable(hsotg); @@ -3714,6 +3717,8 @@ static int s3c_hsotg_remove(struct platform_device *pdev) } s3c_hsotg_phy_disable(hsotg); + if (hsotg-phy) + phy_exit(hsotg-phy); clk_disable_unprepare(hsotg-clk); return 0; -- 1.8.4 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 7/9] phy: add Broadcom Kona USB2 PHY DT binding
Add a binding that describes the Broadcom Kona USB2 PHY found
on the BCM281xx family of SoCs.
Signed-off-by: Matt Porter mpor...@linaro.org
Acked-by: Kishon Vijay Abraham I kis...@ti.com
---
Documentation/devicetree/bindings/phy/bcm-phy.txt | 15 +++
1 file changed, 15 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/bcm-phy.txt
diff --git a/Documentation/devicetree/bindings/phy/bcm-phy.txt
b/Documentation/devicetree/bindings/phy/bcm-phy.txt
new file mode 100644
index 000..3dc8b3d
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/bcm-phy.txt
@@ -0,0 +1,15 @@
+BROADCOM KONA USB2 PHY
+
+Required properties:
+ - compatible: brcm,kona-usb2-phy
+ - reg: offset and length of the PHY registers
+ - #phy-cells: must be 0
+Refer to phy/phy-bindings.txt for the generic PHY binding properties
+
+Example:
+
+ usbphy: usb-phy@3f13 {
+ compatible = brcm,kona-usb2-phy;
+ reg = 0x3f13 0x28;
+ #phy-cells = 0;
+ };
--
1.8.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 0/9] USB Device Controller support for BCM281xx
Changes since v5: - tweak s3c-hsotg Kconfig help message to be more generic Changes since v4: - phy_set/get_bus_width now use an int for bus_width Changes since v3: - Rebased on 3.13-rc3 - Move struct phy bus_width attribute back into struct phy_attrs - Fix build issue on !GENERIC_PHY - Update dwc2 binding to reflect optional phy properties - Rename bcm-kona-phy.txt binding to bcm-phy.txt - Reorder bcm kona phy includes and use bitops - phy-names changed to usb2-phy to match updated s3c-hsotg generic phy-ification series Changes since v2: - Rebased on 3.13-rc1 - Fix braces in phy_get_bus_width()/phy_set_bus_width() - Drop generic phy conversion to use the same support from the Exynos generic phy conversion series - Modify dts support to match the device phy name required in the v3 Exynos generic phy conversion - Add s3c-hsotg phy_init/phy_exit support - Fix typo on reg property in kona phy binding - Replace phy driver reg struct with offset defines - Move phy soft reset to phy driver init - Fix dts node names to match ePAPR conventions Changes since v1: - Convert USB phy driver to generic phy subsystem - Add phy bus width apis - Drop dwc2 phy bus width DT property in favor of querying the phy provider for bus width - Add generic phy/clock properties to dwc2 DT binding - Add generic phy subsystem support to s3c-hsotg with the existing usb phy and pdata phy methods as a fallback - Split bindings out to separate patches to match the latest DT binding review guidelines This series adds USB Device Controller support for the Broadcom BCM281xx family of parts. BCM281xx contains a DWC2 OTG block and s3c-hsotg is used to support UDC operation. Part 1 adds phy bus width support to the generic phy subsystem Parts 2-6 allows s3c-hsotg to build on non-Samsung platforms, supports the dwc2 binding, adds phy_init/phy_exit support, and supports fetching phy bus width using the generic phy layer. Parts 7-8 add a generic phy binding and driver for the BCM Kona USB PHY. Part 9 adds the DT nodes to enable UDC support on both BCM281xx boards in the kernel. This series depends on: - Update Kona drivers to use clocks v4 series https://lkml.org/lkml/2013/12/5/508 - Add new Exynos USB 2.0 PHY driver v4 series https://lkml.org/lkml/2013/12/5/166 Matt Porter (9): phy: add phy_get_bus_width()/phy_set_bus_width() calls staging: dwc2: update DT binding to add generic clock/phy properties usb: gadget: s3c-hsotg: enable build for other platforms usb: gadget: s3c-hsotg: add snps,dwc2 compatible string usb: gadget: s3c-hsotg: use generic phy_init()/phy_exit() support usb: gadget: s3c-hsotg: get phy bus width from phy subsystem phy: add Broadcom Kona USB2 PHY DT binding phy: add Broadcom Kona USB2 PHY driver ARM: dts: add usb udc support to bcm281xx Documentation/devicetree/bindings/phy/bcm-phy.txt | 15 ++ Documentation/devicetree/bindings/staging/dwc2.txt | 12 ++ arch/arm/boot/dts/bcm11351-brt.dts | 6 + arch/arm/boot/dts/bcm11351.dtsi| 18 +++ arch/arm/boot/dts/bcm28155-ap.dts | 8 ++ drivers/phy/Kconfig| 6 + drivers/phy/Makefile | 1 + drivers/phy/phy-bcm-kona-usb2.c| 158 + drivers/usb/gadget/Kconfig | 7 +- drivers/usb/gadget/s3c-hsotg.c | 22 ++- drivers/usb/gadget/s3c-hsotg.h | 1 + include/linux/phy/phy.h| 28 12 files changed, 275 insertions(+), 7 deletions(-) create mode 100644 Documentation/devicetree/bindings/phy/bcm-phy.txt create mode 100644 drivers/phy/phy-bcm-kona-usb2.c -- 1.8.4 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 3/9] usb: gadget: s3c-hsotg: enable build for other platforms
Remove unused Samsung-specific machine include and Kconfig
dependency on S3C.
Signed-off-by: Matt Porter mpor...@linaro.org
Reviewed-by: Markus Mayer markus.ma...@linaro.org
Reviewed-by: Tim Kryger tim.kry...@linaro.org
---
drivers/usb/gadget/Kconfig | 7 +++
drivers/usb/gadget/s3c-hsotg.c | 2 --
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
index a91e642..181e760 100644
--- a/drivers/usb/gadget/Kconfig
+++ b/drivers/usb/gadget/Kconfig
@@ -294,11 +294,10 @@ config USB_PXA27X
gadget drivers to also be dynamically linked.
config USB_S3C_HSOTG
- tristate S3C HS/OtG USB Device controller
- depends on S3C_DEV_USB_HSOTG
+ tristate Designware/S3C HS/OtG USB Device controller
help
- The Samsung S3C64XX USB2.0 high-speed gadget controller
- integrated into the S3C64XX series SoC.
+ The Designware USB2.0 high-speed gadget controller
+ integrated into many SoCs.
config USB_S3C2410
tristate S3C2410 USB Device Controller
diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index 33eb690..8ceb5ef 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -37,8 +37,6 @@
#include linux/usb/phy.h
#include linux/platform_data/s3c-hsotg.h
-#include mach/map.h
-
#include s3c-hsotg.h
static const char * const s3c_hsotg_supply_names[] = {
--
1.8.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 1/9] phy: add phy_get_bus_width()/phy_set_bus_width() calls
This adds a pair of APIs that allows the generic PHY subsystem to
provide information on the PHY bus width. The PHY provider driver may
use phy_set_bus_width() to set the bus width that the PHY supports.
The controller driver may then use phy_get_bus_width() to fetch the
PHY bus width in order to properly configure the controller.
Signed-off-by: Matt Porter mpor...@linaro.org
Acked-by: Kishon Vijay Abraham I kis...@ti.com
---
include/linux/phy/phy.h | 28
1 file changed, 28 insertions(+)
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index 6d72269..e273e5a 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -38,6 +38,14 @@ struct phy_ops {
};
/**
+ * struct phy_attrs - represents phy attributes
+ * @bus_width: Data path width implemented by PHY
+ */
+struct phy_attrs {
+ u32 bus_width;
+};
+
+/**
* struct phy - represents the phy device
* @dev: phy device
* @id: id of the phy device
@@ -46,6 +54,7 @@ struct phy_ops {
* @mutex: mutex to protect phy_ops
* @init_count: used to protect when the PHY is used by multiple consumers
* @power_count: used to protect when the PHY is used by multiple consumers
+ * @phy_attrs: used to specify PHY specific attributes
*/
struct phy {
struct device dev;
@@ -55,6 +64,7 @@ struct phy {
struct mutexmutex;
int init_count;
int power_count;
+ struct phy_attrsattrs;
};
/**
@@ -127,6 +137,14 @@ int phy_init(struct phy *phy);
int phy_exit(struct phy *phy);
int phy_power_on(struct phy *phy);
int phy_power_off(struct phy *phy);
+static inline int phy_get_bus_width(struct phy *phy)
+{
+ return phy-attrs.bus_width;
+}
+static inline void phy_set_bus_width(struct phy *phy, int bus_width)
+{
+ phy-attrs.bus_width = bus_width;
+}
struct phy *phy_get(struct device *dev, const char *string);
struct phy *devm_phy_get(struct device *dev, const char *string);
void phy_put(struct phy *phy);
@@ -199,6 +217,16 @@ static inline int phy_power_off(struct phy *phy)
return -ENOSYS;
}
+static inline int phy_get_bus_width(struct phy *phy)
+{
+ return -ENOSYS;
+}
+
+static inline void phy_set_bus_width(struct phy *phy, int bus_width)
+{
+ return;
+}
+
static inline struct phy *phy_get(struct device *dev, const char *string)
{
return ERR_PTR(-ENOSYS);
--
1.8.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 4/9] usb: gadget: s3c-hsotg: add snps,dwc2 compatible string
Enable support for the dwc2 binding.
Signed-off-by: Matt Porter mpor...@linaro.org
---
drivers/usb/gadget/s3c-hsotg.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/usb/gadget/s3c-hsotg.c b/drivers/usb/gadget/s3c-hsotg.c
index 8ceb5ef..7c5d8bd 100644
--- a/drivers/usb/gadget/s3c-hsotg.c
+++ b/drivers/usb/gadget/s3c-hsotg.c
@@ -3727,6 +3727,7 @@ static int s3c_hsotg_remove(struct platform_device *pdev)
#ifdef CONFIG_OF
static const struct of_device_id s3c_hsotg_of_ids[] = {
{ .compatible = samsung,s3c6400-hsotg, },
+ { .compatible = snps,dwc2, },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, s3c_hsotg_of_ids);
--
1.8.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 2/9] staging: dwc2: update DT binding to add generic clock/phy properties
dwc2/s3c-hsotg require a single clock to be specified and optionally a generic phy. On the s3c-hsotg driver old style USB phy support is present as a fallback so the generic phy properties are optional. Signed-off-by: Matt Porter mpor...@linaro.org Acked-by: Kishon Vijay Abraham I kis...@ti.com --- Documentation/devicetree/bindings/staging/dwc2.txt | 12 1 file changed, 12 insertions(+) diff --git a/Documentation/devicetree/bindings/staging/dwc2.txt b/Documentation/devicetree/bindings/staging/dwc2.txt index 1a1b7cf..a1753ed 100644 --- a/Documentation/devicetree/bindings/staging/dwc2.txt +++ b/Documentation/devicetree/bindings/staging/dwc2.txt @@ -5,6 +5,14 @@ Required properties: - compatible : snps,dwc2 - reg : Should contain 1 register range (address and length) - interrupts : Should contain 1 interrupt +- clocks: clock provider specifier +- clock-names: shall be otg +Refer to clk/clock-bindings.txt for generic clock consumer properties + +Optional properties: +- phys: phy provider specifier +- phy-names: shall be device +Refer to phy/phy-bindings.txt for generic phy consumer properties Example: @@ -12,4 +20,8 @@ Example: compatible = ralink,rt3050-usb, snps,dwc2; reg = 0x101c 4; interrupts = 18; + clocks = usb_otg_ahb_clk; + clock-names = otg; + phys = usbphy; + phy-names = usb2-phy; }; -- 1.8.4 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: xhci_hcd and Canon Lide 110 not playing well together
On Tue, May 28, 2013 at 07:40:57PM +0200, Holger Hans Peter Freyther wrote: Good Evening, Is there a timeline when you think this could be fixed? I tried with 3.10.x and 3.12.5 and the symptoms remain the same. The first time it is working, the second time the scanning does not start. The scanner is still working fine on other machines (with the identical cable). any ideas? holger -- To unsubscribe from this list: send the line unsubscribe 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: core: Add warm reset while reset-resuming SuperSpeed HUBs
On Thu, Dec 12, 2013 at 11:05:04AM -0500, Alan Stern wrote: On Wed, 11 Dec 2013, Julius Werner wrote: ...although, the spec says that it does not wait for the port resets to complete. As far as I can see re-issuing a warm reset and waiting is the only way to guarantee the core times the recovery. Presumably the portstatus debounce in hub_activate() mitigates this, but that 100ms is less than a full reset timeout. It's definitely not just a timing issue for us. I can't reproduce all the same cases as Vikas, but when I attach a USB analyzer to the ones I do see the host controller doesn't even start sending a reset. The xHCI spec requires that when the xHCI host is reset, a USB reset is driven down the USB 3.0 ports. If hot reset fails, the port may migrate to warm reset. See table 32 in the xHCI spec, in the definition of HCRST. It sounds like this host doesn't drive a USB reset down USB 3.0 ports at all on host controller reset? Oh, interesting, I hadn't seen that yet. So I guess the spec itself is fine if it were followed to the letter. I did some more tests about this on my Exynos machine: when I put a device to autosuspend (U3) and manually poke the xHC reset bit, I do see an automatic warm reset on the analyzer and the ports manage to retrain to U0. But after a system suspend/resume which calls xhci_reset() in the process, there is no reset on the wire. I also noticed that it doesn't drive a reset (even after manual poking) when there is no device connected on the other end of the analyzer. So this might be our problem: maybe these host controllers (Synopsys DesignWare) issue the spec-mandated warm reset only on ports where they think there is a device attached. But after a system suspend/resume (where the whole IP block on the SoC was powered down), the host controller cannot know that there is still a device with an active power session attached, and therefore doesn't drive the reset on its own. Ok, that makes some sense. I could see why host controllers wouldn't want to drive reset on an unconnected port. Even though this is a host controller bug, we still have to deal with it somehow. I guess we could move the code into xhci_plat_resume() and hide it behind a quirk to lessen the impact. But since reset_resume is not a common case for most host controllers, it's hard to say if this is DesignWare specific or a more widespread implementation mistake. I was going to suggest something along these lines too. This seems to be a bug in xHCI. Therefore the fix belongs in xhci-hcd, not in the hub driver. I agree. Is there a chance that the Synopsys DesignWare will be a PCI device instead of a platform device? If so, it would be better to put the code into xhci_resume instead of xhci_plat_resume. That also allows you to only issue the warm reset when the register restore state command fails, after the xhci_reset call. Also, I assume that other systems with the Synopsys DesignWare IP will experience this issue? I know of at least two other chipsets that will include that IP, and it would be good to find a way to trigger on the Synopsys IP, rather than off xHCI PCI vendor and device ID. Otherwise we'll be adding PCI IDs to the xHCI driver quirks for many many kernels to come. I'm actually leaning towards enabling the check for warm reset broadly. It seems like it wouldn't hurt to issue a warm reset on the USB 3.0 ports if they're in compliance, poll, or rx.detect. So, let's enable this broadly in xhci_resume, mark the patch for stable, but ask for the backport to be delayed until 3.13.3 is out, to allow for more testing. If anyone complains of xHCI behavior changes, we'll change the code to add a quirk. Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: core: Add warm reset while reset-resuming SuperSpeed HUBs
Hi Sarah, On Fri, Dec 13, 2013 at 09:48:15AM -0800, Sarah Sharp wrote: On Thu, Dec 12, 2013 at 11:05:04AM -0500, Alan Stern wrote: On Wed, 11 Dec 2013, Julius Werner wrote: ...although, the spec says that it does not wait for the port resets to complete. As far as I can see re-issuing a warm reset and waiting is the only way to guarantee the core times the recovery. Presumably the portstatus debounce in hub_activate() mitigates this, but that 100ms is less than a full reset timeout. It's definitely not just a timing issue for us. I can't reproduce all the same cases as Vikas, but when I attach a USB analyzer to the ones I do see the host controller doesn't even start sending a reset. The xHCI spec requires that when the xHCI host is reset, a USB reset is driven down the USB 3.0 ports. If hot reset fails, the port may migrate to warm reset. See table 32 in the xHCI spec, in the definition of HCRST. It sounds like this host doesn't drive a USB reset down USB 3.0 ports at all on host controller reset? Oh, interesting, I hadn't seen that yet. So I guess the spec itself is fine if it were followed to the letter. I did some more tests about this on my Exynos machine: when I put a device to autosuspend (U3) and manually poke the xHC reset bit, I do see an automatic warm reset on the analyzer and the ports manage to retrain to U0. But after a system suspend/resume which calls xhci_reset() in the process, there is no reset on the wire. I also noticed that it doesn't drive a reset (even after manual poking) when there is no device connected on the other end of the analyzer. So this might be our problem: maybe these host controllers (Synopsys DesignWare) issue the spec-mandated warm reset only on ports where they think there is a device attached. But after a system suspend/resume (where the whole IP block on the SoC was powered down), the host controller cannot know that there is still a device with an active power session attached, and therefore doesn't drive the reset on its own. Ok, that makes some sense. I could see why host controllers wouldn't want to drive reset on an unconnected port. Even though this is a host controller bug, we still have to deal with it somehow. I guess we could move the code into xhci_plat_resume() and hide it behind a quirk to lessen the impact. But since reset_resume is not a common case for most host controllers, it's hard to say if this is DesignWare specific or a more widespread implementation mistake. I was going to suggest something along these lines too. This seems to be a bug in xHCI. Therefore the fix belongs in xhci-hcd, not in the hub driver. I agree. Is there a chance that the Synopsys DesignWare will be a PCI device instead of a platform device? If so, it would be better to put the code into xhci_resume instead of xhci_plat_resume. That also allows DWC3 on Intel Baytrail and Merrifield is PCI device. Br, David -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: core: Add warm reset while reset-resuming SuperSpeed HUBs
On Fri, Dec 13, 2013 at 9:48 AM, Sarah Sharp sarah.a.sh...@linux.intel.com wrote: On Thu, Dec 12, 2013 at 11:05:04AM -0500, Alan Stern wrote: On Wed, 11 Dec 2013, Julius Werner wrote: ...although, the spec says that it does not wait for the port resets to complete. As far as I can see re-issuing a warm reset and waiting is the only way to guarantee the core times the recovery. Presumably the portstatus debounce in hub_activate() mitigates this, but that 100ms is less than a full reset timeout. It's definitely not just a timing issue for us. I can't reproduce all the same cases as Vikas, but when I attach a USB analyzer to the ones I do see the host controller doesn't even start sending a reset. The xHCI spec requires that when the xHCI host is reset, a USB reset is driven down the USB 3.0 ports. If hot reset fails, the port may migrate to warm reset. See table 32 in the xHCI spec, in the definition of HCRST. It sounds like this host doesn't drive a USB reset down USB 3.0 ports at all on host controller reset? Oh, interesting, I hadn't seen that yet. So I guess the spec itself is fine if it were followed to the letter. I did some more tests about this on my Exynos machine: when I put a device to autosuspend (U3) and manually poke the xHC reset bit, I do see an automatic warm reset on the analyzer and the ports manage to retrain to U0. But after a system suspend/resume which calls xhci_reset() in the process, there is no reset on the wire. I also noticed that it doesn't drive a reset (even after manual poking) when there is no device connected on the other end of the analyzer. So this might be our problem: maybe these host controllers (Synopsys DesignWare) issue the spec-mandated warm reset only on ports where they think there is a device attached. But after a system suspend/resume (where the whole IP block on the SoC was powered down), the host controller cannot know that there is still a device with an active power session attached, and therefore doesn't drive the reset on its own. Ok, that makes some sense. I could see why host controllers wouldn't want to drive reset on an unconnected port. Even though this is a host controller bug, we still have to deal with it somehow. I guess we could move the code into xhci_plat_resume() and hide it behind a quirk to lessen the impact. But since reset_resume is not a common case for most host controllers, it's hard to say if this is DesignWare specific or a more widespread implementation mistake. I was going to suggest something along these lines too. This seems to be a bug in xHCI. Therefore the fix belongs in xhci-hcd, not in the hub driver. I agree. Is there a chance that the Synopsys DesignWare will be a PCI device instead of a platform device? If so, it would be better to put the code into xhci_resume instead of xhci_plat_resume. That also allows you to only issue the warm reset when the register restore state command fails, after the xhci_reset call. Also, I assume that other systems with the Synopsys DesignWare IP will experience this issue? I know of at least two other chipsets that will include that IP, and it would be good to find a way to trigger on the Synopsys IP, rather than off xHCI PCI vendor and device ID. Otherwise we'll be adding PCI IDs to the xHCI driver quirks for many many kernels to come. I'm actually leaning towards enabling the check for warm reset broadly. It seems like it wouldn't hurt to issue a warm reset on the USB 3.0 ports if they're in compliance, poll, or rx.detect. So, let's enable this broadly in xhci_resume, mark the patch for stable, but ask for the backport to be delayed until 3.13.3 is out, to allow for more testing. If anyone complains of xHCI behavior changes, we'll change the code to add a quirk. Is there a clean way to make this per-port rather than globally at xhci_resume()? I am looking to hook into this for port power recovery as Tianyu's testing encountered warm reset required conditions at runtime_resume. I'm still on the hunt for a solid reproducer, but it indicates this is a more general quirk with power session recovery. -- To unsubscribe from this list: send the line unsubscribe 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: [usb-storage] UAS hangs khubd on USB disconnect
On Thu, Dec 12, 2013 at 05:04:31PM -0500, Alan Stern wrote: On Wed, 11 Dec 2013, Sarah Sharp wrote: Hi Hans, I've been testing the UAS code you sent a pull request for against 3.13-rc1, and I've run into a rather nasty issue with USB disconnect. I ran some tests with a USB 3.0 storage device under xHCI. The disk has three 10GB partitions: ext3 (sdb1), ext4 (sdb2), and fat32 (sdb4). There was a btrfs partition on sdb3, but I deleted it. If I start to play a movie on the ext4 partition, and then yank the USB cable, the uas driver is unbound from the device. It looks like something goes wrong in the SCSI layer shortly after that, causing an oops in sysfs_remove_group(). I did a little testing. It turns out this WARN (not an oops) is the result of recent changes to sysfs, combined with the peculiar way the SCSI layer handles targets. In the new kernel, when you call device_del for some object, the object's directory and everything beneath it get removed from sysfs. This wasn't true in the past. When a USB drive is unplugged, almost everything below it gets unregistered. But not the SCSI target -- it remains registered until the number of reap references drops to 0. This doesn't happen until all the devices beneath it are released, which happens when all the open file references are closed and the filesystem is unmounted. So scsi_target_reap_usercontext ends up calling device_del for the target after everything else has been removed from sysfs. As part of normal device_del processing, attribute groups get removed. In particular the power/ subdirectory is removed from the target's sysfs directory. But the sysfs directories are long gone by this time, so sysfs_remove_group complains that it was asked to remove a non-existent subdirectory. Given the way things work now, I suspect these warnings are truly harmless. We could simply get rid of the WARN in sysfs_remove_group. The alternative is to call device_del for SCSI targets earlier on, such as when their hosts are unregistered. I don't know how James would feel about this approach. It would be difficult because targets use their own reference counts instead of relying on the usual device refcounting mechanism. Thanks for looking into this. I think just getting rid of the WARN would be sufficient. Can you make a patch for that? The patch still won't help with the UAS issues with scsi_init_shared_tag_map though. Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: core: Add warm reset while reset-resuming SuperSpeed HUBs
On Fri, 13 Dec 2013, Dan Williams wrote: I'm actually leaning towards enabling the check for warm reset broadly. It seems like it wouldn't hurt to issue a warm reset on the USB 3.0 ports if they're in compliance, poll, or rx.detect. So, let's enable this broadly in xhci_resume, mark the patch for stable, but ask for the backport to be delayed until 3.13.3 is out, to allow for more testing. If anyone complains of xHCI behavior changes, we'll change the code to add a quirk. Is there a clean way to make this per-port rather than globally at xhci_resume()? I am looking to hook into this for port power recovery as Tianyu's testing encountered warm reset required conditions at runtime_resume. I'm still on the hunt for a solid reproducer, but it indicates this is a more general quirk with power session recovery. There's no reason you can't do per-port testing inside xhci_resume (assuming you know what to test for) as well as putting a warm reset in the port-power handler of xhci_hub_control. Of course, doing simultaneous warm resets on multiple ports will use less time than resetting each port individually, in sequence. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [usb-storage] UAS hangs khubd on USB disconnect
On Fri, 13 Dec 2013, Sarah Sharp wrote: Given the way things work now, I suspect these warnings are truly harmless. We could simply get rid of the WARN in sysfs_remove_group. The alternative is to call device_del for SCSI targets earlier on, such as when their hosts are unregistered. I don't know how James would feel about this approach. It would be difficult because targets use their own reference counts instead of relying on the usual device refcounting mechanism. Thanks for looking into this. I think just getting rid of the WARN would be sufficient. Can you make a patch for that? Easily. The downside is that there would no longer be any warning when someone tries to remove a wrong subdirectory by mistake. The patch still won't help with the UAS issues with scsi_init_shared_tag_map though. I wasn't clear on the reason for that problem. Does it also arise from late device_del for scsi_target? I could try to change the way that works, if anybody (Hans?) would like to test it. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: core: Add warm reset while reset-resuming SuperSpeed HUBs
On Fri, Dec 13, 2013 at 10:15 AM, Alan Stern st...@rowland.harvard.edu wrote: On Fri, 13 Dec 2013, Dan Williams wrote: I'm actually leaning towards enabling the check for warm reset broadly. It seems like it wouldn't hurt to issue a warm reset on the USB 3.0 ports if they're in compliance, poll, or rx.detect. So, let's enable this broadly in xhci_resume, mark the patch for stable, but ask for the backport to be delayed until 3.13.3 is out, to allow for more testing. If anyone complains of xHCI behavior changes, we'll change the code to add a quirk. Is there a clean way to make this per-port rather than globally at xhci_resume()? I am looking to hook into this for port power recovery as Tianyu's testing encountered warm reset required conditions at runtime_resume. I'm still on the hunt for a solid reproducer, but it indicates this is a more general quirk with power session recovery. There's no reason you can't do per-port testing inside xhci_resume (assuming you know what to test for) as well as putting a warm reset in the port-power handler of xhci_hub_control. I'm just uneasy putting the recovery there as we lose the context of why the port was powered-on. For example we don't want to pre-empt/duplicate a reset in xhci_hub_control() that is already specified in hub_events(). Of course, doing simultaneous warm resets on multiple ports will use less time than resetting each port individually, in sequence. For the hub resume case, yes. For pm_runtime_resume of an individual port I believe it needs to be synchronous. -- To unsubscribe from this list: send the line unsubscribe 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: core: Add warm reset while reset-resuming SuperSpeed HUBs
On Fri, Dec 13, 2013 at 10:00:28AM -0800, David Cohen wrote: Hi Sarah, On Fri, Dec 13, 2013 at 09:48:15AM -0800, Sarah Sharp wrote: On Thu, Dec 12, 2013 at 11:05:04AM -0500, Alan Stern wrote: On Wed, 11 Dec 2013, Julius Werner wrote: ...although, the spec says that it does not wait for the port resets to complete. As far as I can see re-issuing a warm reset and waiting is the only way to guarantee the core times the recovery. Presumably the portstatus debounce in hub_activate() mitigates this, but that 100ms is less than a full reset timeout. It's definitely not just a timing issue for us. I can't reproduce all the same cases as Vikas, but when I attach a USB analyzer to the ones I do see the host controller doesn't even start sending a reset. The xHCI spec requires that when the xHCI host is reset, a USB reset is driven down the USB 3.0 ports. If hot reset fails, the port may migrate to warm reset. See table 32 in the xHCI spec, in the definition of HCRST. It sounds like this host doesn't drive a USB reset down USB 3.0 ports at all on host controller reset? Oh, interesting, I hadn't seen that yet. So I guess the spec itself is fine if it were followed to the letter. I did some more tests about this on my Exynos machine: when I put a device to autosuspend (U3) and manually poke the xHC reset bit, I do see an automatic warm reset on the analyzer and the ports manage to retrain to U0. But after a system suspend/resume which calls xhci_reset() in the process, there is no reset on the wire. I also noticed that it doesn't drive a reset (even after manual poking) when there is no device connected on the other end of the analyzer. So this might be our problem: maybe these host controllers (Synopsys DesignWare) issue the spec-mandated warm reset only on ports where they think there is a device attached. But after a system suspend/resume (where the whole IP block on the SoC was powered down), the host controller cannot know that there is still a device with an active power session attached, and therefore doesn't drive the reset on its own. Ok, that makes some sense. I could see why host controllers wouldn't want to drive reset on an unconnected port. Even though this is a host controller bug, we still have to deal with it somehow. I guess we could move the code into xhci_plat_resume() and hide it behind a quirk to lessen the impact. But since reset_resume is not a common case for most host controllers, it's hard to say if this is DesignWare specific or a more widespread implementation mistake. I was going to suggest something along these lines too. This seems to be a bug in xHCI. Therefore the fix belongs in xhci-hcd, not in the hub driver. I agree. Is there a chance that the Synopsys DesignWare will be a PCI device instead of a platform device? If so, it would be better to put the code into xhci_resume instead of xhci_plat_resume. That also allows DWC3 on Intel Baytrail and Merrifield is PCI device. But it actually registers xHCI's platform device to probe it. So, nevermind. Br, David -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [usb-storage] UAS hangs khubd on USB disconnect
Hello, guys. (cc'ing Greg) On Fri, Dec 13, 2013 at 01:19:36PM -0500, Alan Stern wrote: On Fri, 13 Dec 2013, Sarah Sharp wrote: Given the way things work now, I suspect these warnings are truly harmless. We could simply get rid of the WARN in sysfs_remove_group. The alternative is to call device_del for SCSI targets earlier on, such as when their hosts are unregistered. I don't know how James would feel about this approach. It would be difficult because targets use their own reference counts instead of relying on the usual device refcounting mechanism. Thanks for looking into this. I think just getting rid of the WARN would be sufficient. Can you make a patch for that? Easily. The downside is that there would no longer be any warning when someone tries to remove a wrong subdirectory by mistake. The patch still won't help with the UAS issues with scsi_init_shared_tag_map though. I wasn't clear on the reason for that problem. Does it also arise from late device_del for scsi_target? I could try to change the way that works, if anybody (Hans?) would like to test it. While the recent sysfs changes made this issue more visible, Greg wants to make sure that devices are removed from leaf up in all cases and keep the warning to ensure that. Would there be a way fix SCSI removal ordering? Thanks. -- tejun -- To unsubscribe from this list: send the line unsubscribe 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: [usb-storage] UAS hangs khubd on USB disconnect
On Fri, Dec 13, 2013 at 01:19:36PM -0500, Alan Stern wrote: On Fri, 13 Dec 2013, Sarah Sharp wrote: Given the way things work now, I suspect these warnings are truly harmless. We could simply get rid of the WARN in sysfs_remove_group. The alternative is to call device_del for SCSI targets earlier on, such as when their hosts are unregistered. I don't know how James would feel about this approach. It would be difficult because targets use their own reference counts instead of relying on the usual device refcounting mechanism. Thanks for looking into this. I think just getting rid of the WARN would be sufficient. Can you make a patch for that? Easily. The downside is that there would no longer be any warning when someone tries to remove a wrong subdirectory by mistake. The patch still won't help with the UAS issues with scsi_init_shared_tag_map though. I wasn't clear on the reason for that problem. Does it also arise from late device_del for scsi_target? I could try to change the way that works, if anybody (Hans?) would like to test it. I can certainly test it with my UAS device as well. I don't know if the issue arises from the late device_del. Looking at Hans' stack trace, the BUG in blk_free_tags gets triggered when the scsi_host is released before the block_queue release. So I don't think moving the scsi_target delete sooner would help? I really don't know anything about the SCSI or block layer though. I can confirm that simply removing the BUG() call in blk_free_tags allows the partitions on the UAS device to be mounted after it was hot-removed in the middle of video playback. Hans, maybe in order to get an answer to your question[1], you should submit a patch to the block layer maintainer, Jens Axboe? Sarah Sharp [1] http://www.spinics.net/lists/linux-scsi/msg70002.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: [usb-storage] UAS hangs khubd on USB disconnect
On Fri, 2013-12-13 at 13:33 -0500, Tejun Heo wrote: Hello, guys. (cc'ing Greg) On Fri, Dec 13, 2013 at 01:19:36PM -0500, Alan Stern wrote: On Fri, 13 Dec 2013, Sarah Sharp wrote: Given the way things work now, I suspect these warnings are truly harmless. We could simply get rid of the WARN in sysfs_remove_group. The alternative is to call device_del for SCSI targets earlier on, such as when their hosts are unregistered. I don't know how James would feel about this approach. It would be difficult because targets use their own reference counts instead of relying on the usual device refcounting mechanism. Thanks for looking into this. I think just getting rid of the WARN would be sufficient. Can you make a patch for that? Easily. The downside is that there would no longer be any warning when someone tries to remove a wrong subdirectory by mistake. The patch still won't help with the UAS issues with scsi_init_shared_tag_map though. I wasn't clear on the reason for that problem. Does it also arise from late device_del for scsi_target? I could try to change the way that works, if anybody (Hans?) would like to test it. While the recent sysfs changes made this issue more visible, Greg wants to make sure that devices are removed from leaf up in all cases and keep the warning to ensure that. Would there be a way fix SCSI removal ordering? Could someone analyse the actual problem? We're quite careful even on host remove to iterate and remove all the devices, then targets, then host (and allied transport objects). Which removal is inverted? James -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] xhci: Use TRB_CYCLE instead of the constant 0x1
Hi David,
All right, I'm finally starting to catch up on my patch queue. :)
Sorry for the delay.
In general this looks like a good patch, thanks for submitting it.
However, there's a couple things I need you to fix.
On Mon, Nov 04, 2013 at 11:36:00AM -, David Laight wrote:
In many cases the constant 1 is used for values that eventually
get written to the hardware ring. Replace all of these with the
symbolic constant TRB_CYCLE.
This makes it clear that the ring-cycle_state value is used when
writing to the actual ring itself.
Always use ^= TRB_CYCLE to invert the bit.
---
The first is that you forgot to include a Signed-off-by line here.
Running your patch through scripts/checkpatch.pl will help you catch
these kinds of errors.
The second is that this patch triggers some checkpatch warnings because
some of your changes caused the lines to go over 80 characters:
sarah@xanatos:~/git/kernels/xhci$ git commit -v
WARNING: line over 80 characters
#10: FILE: drivers/usb/host/xhci-mem.c:601:
+ xhci_ring_alloc(xhci, 2, TRB_CYCLE, TYPE_STREAM,
mem_flags);
WARNING: line over 80 characters
#19: FILE: drivers/usb/host/xhci-mem.c:909:
+ dev-eps[0].ring = xhci_ring_alloc(xhci, 2, TRB_CYCLE, TYPE_CTRL,
flags);
WARNING: line over 80 characters
#37: FILE: drivers/usb/host/xhci-mem.c:2304:
+ xhci-cmd_ring = xhci_ring_alloc(xhci, 1, TRB_CYCLE, TYPE_COMMAND,
flags);
WARNING: line over 80 characters
#46: FILE: drivers/usb/host/xhci-mem.c:2348:
+ xhci-event_ring = xhci_ring_alloc(xhci, ERST_NUM_SEGS, TRB_CYCLE,
TYPE_EVENT,
total: 0 errors, 4 warnings, 112 lines checked
Your patch has style problems, please review.
Can you please break those lines at 80 characters?
Thanks,
Sarah Sharp
drivers/usb/host/xhci-mem.c | 10 +-
drivers/usb/host/xhci-ring.c | 16
drivers/usb/host/xhci.c | 2 +-
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 9b5d1c3..5b2d835 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -597,7 +597,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct
xhci_hcd *xhci,
*/
for (cur_stream = 1; cur_stream num_streams; cur_stream++) {
stream_info-stream_rings[cur_stream] =
- xhci_ring_alloc(xhci, 2, 1, TYPE_STREAM, mem_flags);
+ xhci_ring_alloc(xhci, 2, TRB_CYCLE, TYPE_STREAM,
mem_flags);
cur_ring = stream_info-stream_rings[cur_stream];
if (!cur_ring)
goto cleanup_rings;
@@ -906,7 +906,7 @@ int xhci_alloc_virt_device(struct xhci_hcd *xhci, int
slot_id,
}
/* Allocate endpoint 0 ring */
- dev-eps[0].ring = xhci_ring_alloc(xhci, 2, 1, TYPE_CTRL, flags);
+ dev-eps[0].ring = xhci_ring_alloc(xhci, 2, TRB_CYCLE, TYPE_CTRL,
flags);
if (!dev-eps[0].ring)
goto fail;
@@ -1326,7 +1326,7 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
type = usb_endpoint_type(ep-desc);
/* Set up the endpoint ring */
virt_dev-eps[ep_index].new_ring =
- xhci_ring_alloc(xhci, 2, 1, type, mem_flags);
+ xhci_ring_alloc(xhci, 2, TRB_CYCLE, type, mem_flags);
if (!virt_dev-eps[ep_index].new_ring) {
/* Attempt to use the ring cache */
if (virt_dev-num_rings_cached == 0)
@@ -2311,7 +2311,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
goto fail;
/* Set up the command ring to have one segments for now. */
- xhci-cmd_ring = xhci_ring_alloc(xhci, 1, 1, TYPE_COMMAND, flags);
+ xhci-cmd_ring = xhci_ring_alloc(xhci, 1, TRB_CYCLE, TYPE_COMMAND,
flags);
if (!xhci-cmd_ring)
goto fail;
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
@@ -2355,7 +2355,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
* the event ring segment table (ERST). Section 4.9.3.
*/
xhci_dbg_trace(xhci, trace_xhci_dbg_init, // Allocating event ring);
- xhci-event_ring = xhci_ring_alloc(xhci, ERST_NUM_SEGS, 1, TYPE_EVENT,
+ xhci-event_ring = xhci_ring_alloc(xhci, ERST_NUM_SEGS, TRB_CYCLE,
TYPE_EVENT,
flags);
if (!xhci-event_ring)
goto fail;
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index d3f4a9a..408978b 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -178,7 +178,7 @@ static void inc_deq(struct xhci_hcd *xhci, struct
xhci_ring *ring)
if (ring-type == TYPE_EVENT
last_trb_on_last_seg(xhci, ring,
ring-deq_seg, ring-dequeue)) {
- ring-cycle_state = (ring-cycle_state ? 0 : 1);
+
Re: [PATCH] xhci: Use TRB_CYCLE instead of the constant 0x1
On Fri, Dec 13, 2013 at 11:47:56AM -0800, Sarah Sharp wrote:
Hi David,
All right, I'm finally starting to catch up on my patch queue. :)
Sorry for the delay.
In general this looks like a good patch, thanks for submitting it.
However, there's a couple things I need you to fix.
Ah, I just saw your patch v2. Nevermind.
On Mon, Nov 04, 2013 at 11:36:00AM -, David Laight wrote:
In many cases the constant 1 is used for values that eventually
get written to the hardware ring. Replace all of these with the
symbolic constant TRB_CYCLE.
This makes it clear that the ring-cycle_state value is used when
writing to the actual ring itself.
Always use ^= TRB_CYCLE to invert the bit.
---
The first is that you forgot to include a Signed-off-by line here.
Running your patch through scripts/checkpatch.pl will help you catch
these kinds of errors.
The second is that this patch triggers some checkpatch warnings because
some of your changes caused the lines to go over 80 characters:
sarah@xanatos:~/git/kernels/xhci$ git commit -v
WARNING: line over 80 characters
#10: FILE: drivers/usb/host/xhci-mem.c:601:
+ xhci_ring_alloc(xhci, 2, TRB_CYCLE, TYPE_STREAM,
mem_flags);
WARNING: line over 80 characters
#19: FILE: drivers/usb/host/xhci-mem.c:909:
+ dev-eps[0].ring = xhci_ring_alloc(xhci, 2, TRB_CYCLE, TYPE_CTRL,
flags);
WARNING: line over 80 characters
#37: FILE: drivers/usb/host/xhci-mem.c:2304:
+ xhci-cmd_ring = xhci_ring_alloc(xhci, 1, TRB_CYCLE, TYPE_COMMAND,
flags);
WARNING: line over 80 characters
#46: FILE: drivers/usb/host/xhci-mem.c:2348:
+ xhci-event_ring = xhci_ring_alloc(xhci, ERST_NUM_SEGS, TRB_CYCLE,
TYPE_EVENT,
total: 0 errors, 4 warnings, 112 lines checked
Your patch has style problems, please review.
Can you please break those lines at 80 characters?
Thanks,
Sarah Sharp
drivers/usb/host/xhci-mem.c | 10 +-
drivers/usb/host/xhci-ring.c | 16
drivers/usb/host/xhci.c | 2 +-
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 9b5d1c3..5b2d835 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -597,7 +597,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct
xhci_hcd *xhci,
*/
for (cur_stream = 1; cur_stream num_streams; cur_stream++) {
stream_info-stream_rings[cur_stream] =
- xhci_ring_alloc(xhci, 2, 1, TYPE_STREAM, mem_flags);
+ xhci_ring_alloc(xhci, 2, TRB_CYCLE, TYPE_STREAM,
mem_flags);
cur_ring = stream_info-stream_rings[cur_stream];
if (!cur_ring)
goto cleanup_rings;
@@ -906,7 +906,7 @@ int xhci_alloc_virt_device(struct xhci_hcd *xhci, int
slot_id,
}
/* Allocate endpoint 0 ring */
- dev-eps[0].ring = xhci_ring_alloc(xhci, 2, 1, TYPE_CTRL, flags);
+ dev-eps[0].ring = xhci_ring_alloc(xhci, 2, TRB_CYCLE, TYPE_CTRL,
flags);
if (!dev-eps[0].ring)
goto fail;
@@ -1326,7 +1326,7 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
type = usb_endpoint_type(ep-desc);
/* Set up the endpoint ring */
virt_dev-eps[ep_index].new_ring =
- xhci_ring_alloc(xhci, 2, 1, type, mem_flags);
+ xhci_ring_alloc(xhci, 2, TRB_CYCLE, type, mem_flags);
if (!virt_dev-eps[ep_index].new_ring) {
/* Attempt to use the ring cache */
if (virt_dev-num_rings_cached == 0)
@@ -2311,7 +2311,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
goto fail;
/* Set up the command ring to have one segments for now. */
- xhci-cmd_ring = xhci_ring_alloc(xhci, 1, 1, TYPE_COMMAND, flags);
+ xhci-cmd_ring = xhci_ring_alloc(xhci, 1, TRB_CYCLE, TYPE_COMMAND,
flags);
if (!xhci-cmd_ring)
goto fail;
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
@@ -2355,7 +2355,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
* the event ring segment table (ERST). Section 4.9.3.
*/
xhci_dbg_trace(xhci, trace_xhci_dbg_init, // Allocating event ring);
- xhci-event_ring = xhci_ring_alloc(xhci, ERST_NUM_SEGS, 1, TYPE_EVENT,
+ xhci-event_ring = xhci_ring_alloc(xhci, ERST_NUM_SEGS, TRB_CYCLE,
TYPE_EVENT,
flags);
if (!xhci-event_ring)
goto fail;
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index d3f4a9a..408978b 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -178,7 +178,7 @@ static void inc_deq(struct xhci_hcd *xhci, struct
xhci_ring *ring)
if (ring-type == TYPE_EVENT
last_trb_on_last_seg(xhci, ring,
ring-deq_seg,
Re: [PATCH v3] usb: gadget: Add UDC driver for Aeroflex Gaisler GRUSBDC
Hi,
On Fri, Dec 13, 2013 at 08:48:30AM +0100, Andreas Larsson wrote:
On Wed, Dec 04, 2013 at 09:13:58AM +0100, Andreas Larsson wrote:
+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 bit by bit */
+ 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);
+ }
+
+ /* Prevent changes to ep-queue during callback */
+ ep-callback = 1;
+ if (req == dev-ep0reqo !status) {
+ if (req-setup)
+ gr_ep0_setup(dev, req);
+ else
+ dev_err(dev-dev,
+ Unexpected non setup packet on ep0in\n);
+ } else if (req-req.complete) {
+ unsigned long flags;
+
+ /*
+* Complete should be called with interrupts disabled according
+* to the contract of struct usb_request
+*/
+ local_irq_save(flags);
sorry but this driver isn't ready for inclusion. local_irq_save() is a
pretty good hint that there's something wrong in the driver. Consider
the fact that local_irq_save() will disable preemption even when
CONFIG_PREEMPT_FULL is enabled and you have a bit a problem.
This connection between local_irq_save and CONFIG_PREEMPT_RT_FULL was
unknown to me. Sure, I can disable interrupts right at spin lock
time.
that's better.
Also, the way you're using thread IRQs is quite wrong. I can't let that
pass and get merged upstream, sorry.
What is quite wrong? What is it that I need to fix?
Ideally the hardirq handler should be usually to actually check if
$this_device generated the IRQ, that should involve reading a IRQSTATUS
register of some sort.
Sure, check that IRQs are actually enabled, but you also need to read
STATUS register before waking the thread up.
--
balbi
signature.asc
Description: Digital signature
Re: [PATCH] xhci: Allocate the td array and urb_priv together.
On Mon, Nov 04, 2013 at 11:32:41AM -, David Laight wrote:
Replace the array of pointers to transfer descriptors with
the transfer descriptors themselves.
This saves a kzalloc() call on every transfer and some memory
indirections.
The only possible downside is for isochronous tranfers with 64 td
when the allocate is 8+4096 bytes (on 64bit systems) so requires
an additional page.
Looks like this patch is missing a Signed-off-by as well, and I don't
see a v2 patch from you. Can you fix this and resubmit it? The patch
itself looks fine.
Sarah Sharp
---
drivers/usb/host/xhci-mem.c | 4 +---
drivers/usb/host/xhci-ring.c | 22 ++
drivers/usb/host/xhci.c | 24 ++--
drivers/usb/host/xhci.h | 2 +-
4 files changed, 18 insertions(+), 34 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 79cdcc2..9b5d1c3 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1675,10 +1675,8 @@ struct xhci_command *xhci_alloc_command(struct
xhci_hcd *xhci,
void xhci_urb_free_priv(struct xhci_hcd *xhci, struct urb_priv *urb_priv)
{
- if (urb_priv) {
- kfree(urb_priv-td[0]);
+ if (urb_priv)
kfree(urb_priv);
- }
}
void xhci_free_command(struct xhci_hcd *xhci,
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index e38abc2..d3f4a9a 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3007,7 +3007,7 @@ static int prepare_transfer(struct xhci_hcd *xhci,
return ret;
urb_priv = urb-hcpriv;
- td = urb_priv-td[td_index];
+ td = urb_priv-td[td_index];
INIT_LIST_HEAD(td-td_list);
INIT_LIST_HEAD(td-cancelled_td_list);
@@ -3024,8 +3024,6 @@ static int prepare_transfer(struct xhci_hcd *xhci,
td-start_seg = ep_ring-enq_seg;
td-first_trb = ep_ring-enqueue;
- urb_priv-td[td_index] = td;
-
return 0;
}
@@ -3216,7 +3214,7 @@ static int queue_bulk_sg_tx(struct xhci_hcd *xhci,
gfp_t mem_flags,
return trb_buff_len;
urb_priv = urb-hcpriv;
- td = urb_priv-td[0];
+ td = urb_priv-td[0];
/*
* Don't give the first TRB to the hardware (by toggling the cycle bit)
@@ -3387,7 +3385,7 @@ int xhci_queue_bulk_tx(struct xhci_hcd *xhci, gfp_t
mem_flags,
return ret;
urb_priv = urb-hcpriv;
- td = urb_priv-td[0];
+ td = urb_priv-td[0];
/*
* Don't give the first TRB to the hardware (by toggling the cycle bit)
@@ -3517,7 +3515,7 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t
mem_flags,
return ret;
urb_priv = urb-hcpriv;
- td = urb_priv-td[0];
+ td = urb_priv-td[0];
/*
* Don't give the first TRB to the hardware (by toggling the cycle bit)
@@ -3729,7 +3727,7 @@ static int xhci_queue_isoc_tx(struct xhci_hcd *xhci,
gfp_t mem_flags,
goto cleanup;
}
- td = urb_priv-td[i];
+ td = urb_priv-td[i];
for (j = 0; j trbs_per_td; j++) {
u32 remainder = 0;
field = 0;
@@ -3829,20 +3827,20 @@ cleanup:
/* Clean up a partially enqueued isoc transfer. */
for (i--; i = 0; i--)
- list_del_init(urb_priv-td[i]-td_list);
+ list_del_init(urb_priv-td[i].td_list);
/* Use the first TD as a temporary variable to turn the TDs we've queued
* into No-ops with a software-owned cycle bit. That way the hardware
* won't accidentally start executing bogus TDs when we partially
* overwrite them. td-first_trb and td-start_seg are already set.
*/
- urb_priv-td[0]-last_trb = ep_ring-enqueue;
+ urb_priv-td[0].last_trb = ep_ring-enqueue;
/* Every TRB except the first last will have its cycle bit flipped. */
- td_to_noop(xhci, ep_ring, urb_priv-td[0], true);
+ td_to_noop(xhci, ep_ring, urb_priv-td[0], true);
/* Reset the ring enqueue back to the first TRB and its cycle bit. */
- ep_ring-enqueue = urb_priv-td[0]-first_trb;
- ep_ring-enq_seg = urb_priv-td[0]-start_seg;
+ ep_ring-enqueue = urb_priv-td[0].first_trb;
+ ep_ring-enq_seg = urb_priv-td[0].start_seg;
ep_ring-cycle_state = start_cycle;
ep_ring-num_trbs_free = ep_ring-num_trbs_free_temp;
usb_hcd_unlink_urb_from_ep(bus_to_hcd(urb-dev-bus), urb);
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 6e0d886..0969f74 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -1248,12 +1248,11 @@ static int xhci_check_maxpacket(struct xhci_hcd
*xhci, unsigned int slot_id,
int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags)
{
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
- struct xhci_td
Re: [PATCH v2 1/7] usb: dwc3: keystone: add basic PM support
On Fri, Dec 13, 2013 at 10:04:38AM -0600, Kwok, WingMan wrote: Hello -Original Message- From: Shilimkar, Santosh Sent: Thursday, December 12, 2013 7:29 PM To: Balbi, Felipe Cc: Linux USB Mailing List; kgene@samsung.com; Linux ARM Kernel Mailing List; linux-samsung-...@vger.kernel.org; Linux OMAP Mailing List; Kwok, WingMan Subject: Re: [PATCH v2 1/7] usb: dwc3: keystone: add basic PM support On Thursday 12 December 2013 04:45 PM, Felipe Balbi wrote: A bare-minimum PM implementation which will server as building block for more complex s/server/serve ;-) PM implementation in the future. At the least will not leave clocks on unnecessarily when e.g. a user write mem to /sys/power/state. Signed-off-by: Felipe Balbi ba...@ti.com --- improve error path a little bit. We will test this out. Thanks for the patch Felipe. I have tested the patch and the keystone usb driver continues to function, though I can't test suspend at this time as the rest of the system does not that functionality yet. Thanks, should I add your Tested-by ? -- balbi signature.asc Description: Digital signature
Re: [PATCH 7/7] usb: dwc3: exynos: add pm_runtime support
On Fri, Dec 13, 2013 at 02:01:32PM +0900, Anton Tikhomirov wrote:
Hi Felipe,
-static int dwc3_exynos_suspend(struct device *dev)
+static int __dwc3_exynos_suspend(struct dwc3_exynos *exynos)
{
- struct dwc3_exynos *exynos = dev_get_drvdata(dev);
-
clk_disable(exynos-clk);
return 0;
}
+static int __dwc3_exynos_resume(struct dwc3_exynos *exynos)
+{
+ return clk_enable(exynos-clk);
+}
+
+static int dwc3_exynos_suspend(struct device *dev)
+{
+ struct dwc3_exynos *exynos = dev_get_drvdata(dev);
+
+ return __dwc3_exynos_suspend(exynos);
If dwc3-exynos is runtime suspended, the clock will be disabled
second time here (unbalanced clk_enable/clk_disable).
I don't get what you mean but there is something that probably needs
fixing, I guess below makes it better:
diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
index c93919a..1e5720a 100644
--- a/drivers/usb/dwc3/dwc3-exynos.c
+++ b/drivers/usb/dwc3/dwc3-exynos.c
@@ -218,6 +218,9 @@ static int dwc3_exynos_suspend(struct device *dev)
{
struct dwc3_exynos *exynos = dev_get_drvdata(dev);
+ if (pm_runtime_suspended(dev))
+ return 0;
+
return __dwc3_exynos_suspend(exynos);
}
Is that what you meant ?
--
balbi
signature.asc
Description: Digital signature
Re: gadgetfs USB2.0 Chapter 9 Tests: Test after Addressed State fails
hi, On Fri, Dec 13, 2013 at 01:36:03PM +, roshan.jhal...@broadcom.com wrote: Hi Macro, We have observed same issue and reason for this issue is reset_config which triggers complete USB disconnect from F_FS. For SET_CONFIG(Config#0) there is no need to do USB Disconnect. This seems to be bottleneck issue for USB compliance. I believe this issue should be addressed by GadgetFS driver. patches are welcome :-) -- balbi signature.asc Description: Digital signature
Re: [usb-storage] UAS hangs khubd on USB disconnect
On Fri, 2013-12-13 at 11:18 -0800, James Bottomley wrote: On Fri, 2013-12-13 at 13:33 -0500, Tejun Heo wrote: Hello, guys. (cc'ing Greg) On Fri, Dec 13, 2013 at 01:19:36PM -0500, Alan Stern wrote: On Fri, 13 Dec 2013, Sarah Sharp wrote: Given the way things work now, I suspect these warnings are truly harmless. We could simply get rid of the WARN in sysfs_remove_group. The alternative is to call device_del for SCSI targets earlier on, such as when their hosts are unregistered. I don't know how James would feel about this approach. It would be difficult because targets use their own reference counts instead of relying on the usual device refcounting mechanism. Thanks for looking into this. I think just getting rid of the WARN would be sufficient. Can you make a patch for that? Easily. The downside is that there would no longer be any warning when someone tries to remove a wrong subdirectory by mistake. The patch still won't help with the UAS issues with scsi_init_shared_tag_map though. I wasn't clear on the reason for that problem. Does it also arise from late device_del for scsi_target? I could try to change the way that works, if anybody (Hans?) would like to test it. While the recent sysfs changes made this issue more visible, Greg wants to make sure that devices are removed from leaf up in all cases and keep the warning to ensure that. Would there be a way fix SCSI removal ordering? Could someone analyse the actual problem? We're quite careful even on host remove to iterate and remove all the devices, then targets, then host (and allied transport objects). Which removal is inverted? Actually, I think I have this figured out. There's a thinko in one of the scsi_target_reap() cases. The original (and still existing) problem with targets is that nothing creates them and nothing destroys them, so, while we could rely on the refcounting of the device model to preserve the actual target object, we had no idea when to remove it from visibility. That was the job of the reap reference, to track visibility. It looks like the reap on device last put is occurring too late. I think we should reap immediately after doing the sdev device_del, so does this fix the warn on? (I'm not sure because no-one has actually posted a backtrace, but it sounds like this is the problem). James --- diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index 8ff62c2..98d4eb3 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -399,8 +399,6 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work) /* NULL queue means the device can't be used */ sdev-request_queue = NULL; - scsi_target_reap(scsi_target(sdev)); - kfree(sdev-inquiry); kfree(sdev); @@ -1044,6 +1042,8 @@ void __scsi_remove_device(struct scsi_device *sdev) } else put_device(sdev-sdev_dev); + scsi_target_reap(scsi_target(sdev)); + /* * Stop accepting new requests and wait until all queuecommand() and * scsi_run_queue() invocations have finished before tearing down the -- To unsubscribe from this list: send the line unsubscribe 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: [usb-storage] UAS hangs khubd on USB disconnect
On Fri, 13 Dec 2013, James Bottomley wrote: I wasn't clear on the reason for that problem. Does it also arise from late device_del for scsi_target? I could try to change the way that works, if anybody (Hans?) would like to test it. While the recent sysfs changes made this issue more visible, Greg wants to make sure that devices are removed from leaf up in all cases and keep the warning to ensure that. Would there be a way fix SCSI removal ordering? Could someone analyse the actual problem? We're quite careful even on host remove to iterate and remove all the devices, then targets, then host (and allied transport objects). Which removal is inverted? The scsi_host is removed before the scsi_target. The reason is that scsi_remove_host() calls device_del(shost-shost_gendev) directly, but scsi_target_reap_usercontext() doesn't call device_del(starget-dev) until it gets invoked (indirectly) from scsi_device_dev_release_usercontext(), by way of scsi_target_reap(). Thus, the host gets removed from visibility at the appropriate time, but the target remains until all the scsi_devices beneath it are not only removed from visibility but also released (their refcounts drop to 0). This can occur much later if, for example, a scsi_device holds a mounted filesystem. The scsi_host and scsi_device are removed when the underlying USB device is unplugged. But the scsi_device isn't released, and hence the scsi_target isn't removed, until the filesystem is unmounted. Broadly speaking, the correct approach would be to call scsi_target_reap() from __scsi_remove_device() instead of from scsi_device_dev_release_usercontext(). Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: xhci: Add 2nd memory barrier to giveback_first_trb()
On Wed, Nov 13, 2013 at 10:24:00AM -, David Laight wrote: Greg KH wrote: On Tue, Nov 12, 2013 at 01:58:05PM -, David Laight wrote: There needs to be a wmb() barrier between the write to the cycle bit in the first TRB and the write to the doorbell register. Since it isn't needed in the other places the doobell is rung (because the ring contents haven't been changed) add it to giveback_first_trb() rather than somewhere later. Signed-off-by: David Laight david.lai...@aculab.com --- This patch will only apply cleanly if my earlier patch that affects the previous line has also been applied. drivers/usb/host/xhci-ring.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 35dfed0..8bce4c3 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -3136,6 +3136,7 @@ static void giveback_first_trb(struct xhci_hcd *xhci, int slot_id, */ wmb(); start_trb-field[3] ^= cpu_to_le32(TRB_CYCLE); + wmb(); I don't understand, why is this needed? field[] isn't being used elsewhere at this point in time, is it? When adding barriers, you need to also add a big comment as to exactly why this is needed, in the code itself, which this patch does not do, so we can't take it (well, I will not take it, Sarah might, but then I'll complain to her...) The sequence is: 1) Write all the ring entries, but leave the first with its old TRB_CYCLE value. 2) The first wmb() in giveback_first_trb() ensures that all the ring entries are written before we... 3) Invert the TRB_CYCLE bit on the first TRB, this allows the hardware to process that entry. 4) The extra wmb() ensures that the ring entry write happens before... 5) We write to the doorbell register telling the hardware it can read the first ring entry. Without the extra wmb() the hardware could read the ring entry before the cpu has inverted its TRB_CYCLE - so the TD wouldn't be processed until the next URB setup is completed. All right, you've convinced me that the extra wmb() is necessary. Can you please add the above explanation as a function comment above giveback_first_trb() and re-submit this patch? Given the size of the function that writes to the doorbell register this is probably unlikely. OTOH the doorbell register address and value to write could be cached in the ep_ring making it a very short (and inlined) function. The reason we haven't run into this is probably because the xHCI driver has only been thoroughly tested on x86 platforms. PCI memory writes aren't reordered on that platform, so wmb is a no-op. Given that no one has complained about this bug that has been around since the xHCI driver was first submitted, I'm disinclined to queue this for stable. Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] usb: xhci: Use TRB_CYCLE instead of the constant 0x1
On Mon, Nov 11, 2013 at 04:15:26PM -, David Laight wrote:
This makes it clear that the ring-cycle_state value is used when
writing to the actual ring itself.
Always use ^= TRB_CYCLE to invert the bit.
Signed-off-by: David Laight david.lai...@aculab.com
---
Changes for v2:
1 Adjusted so that it should apply to HEAD.
(One of the ?: had been changed to ^=.)
2 Added Signed-off line.
All right, this is closer. However, it still contains the checkpatch.pl
warnings about lines over 80-characters:
sarah@xanatos:~/git/kernels/xhci$ git am -s ~/Maildir.fetchmail/.to-apply
Applying: usb: xhci: Use TRB_CYCLE instead of the constant 0x1
WARNING: line over 80 characters
#10: FILE: drivers/usb/host/xhci-mem.c:601:
+ xhci_ring_alloc(xhci, 2, TRB_CYCLE, TYPE_STREAM,
mem_flags);
WARNING: line over 80 characters
#19: FILE: drivers/usb/host/xhci-mem.c:909:
+ dev-eps[0].ring = xhci_ring_alloc(xhci, 2, TRB_CYCLE, TYPE_CTRL,
flags);
WARNING: line over 80 characters
#37: FILE: drivers/usb/host/xhci-mem.c:2306:
+ xhci-cmd_ring = xhci_ring_alloc(xhci, 1, TRB_CYCLE, TYPE_COMMAND,
flags);
WARNING: line over 80 characters
#46: FILE: drivers/usb/host/xhci-mem.c:2350:
+ xhci-event_ring = xhci_ring_alloc(xhci, ERST_NUM_SEGS, TRB_CYCLE,
TYPE_EVENT,
total: 0 errors, 4 warnings, 112 lines checked
Your patch has style problems, please review.
Can you please break these changed lines at 80-characters, recheck the
patch with checkpatch.pl, and resubmit?
Thanks,
Sarah Sharp
drivers/usb/host/xhci-mem.c | 10 +-
drivers/usb/host/xhci-ring.c | 16
drivers/usb/host/xhci.c | 2 +-
3 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 9b5d1c3..5b2d835 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -597,7 +597,7 @@ struct xhci_stream_info *xhci_alloc_stream_info(struct
xhci_hcd *xhci,
*/
for (cur_stream = 1; cur_stream num_streams; cur_stream++) {
stream_info-stream_rings[cur_stream] =
- xhci_ring_alloc(xhci, 2, 1, TYPE_STREAM, mem_flags);
+ xhci_ring_alloc(xhci, 2, TRB_CYCLE, TYPE_STREAM,
mem_flags);
cur_ring = stream_info-stream_rings[cur_stream];
if (!cur_ring)
goto cleanup_rings;
@@ -906,7 +906,7 @@ int xhci_alloc_virt_device(struct xhci_hcd *xhci, int
slot_id,
}
/* Allocate endpoint 0 ring */
- dev-eps[0].ring = xhci_ring_alloc(xhci, 2, 1, TYPE_CTRL, flags);
+ dev-eps[0].ring = xhci_ring_alloc(xhci, 2, TRB_CYCLE, TYPE_CTRL,
flags);
if (!dev-eps[0].ring)
goto fail;
@@ -1326,7 +1326,7 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
type = usb_endpoint_type(ep-desc);
/* Set up the endpoint ring */
virt_dev-eps[ep_index].new_ring =
- xhci_ring_alloc(xhci, 2, 1, type, mem_flags);
+ xhci_ring_alloc(xhci, 2, TRB_CYCLE, type, mem_flags);
if (!virt_dev-eps[ep_index].new_ring) {
/* Attempt to use the ring cache */
if (virt_dev-num_rings_cached == 0)
@@ -2311,7 +2311,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
goto fail;
/* Set up the command ring to have one segments for now. */
- xhci-cmd_ring = xhci_ring_alloc(xhci, 1, 1, TYPE_COMMAND, flags);
+ xhci-cmd_ring = xhci_ring_alloc(xhci, 1, TRB_CYCLE, TYPE_COMMAND,
flags);
if (!xhci-cmd_ring)
goto fail;
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
@@ -2355,7 +2355,7 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
* the event ring segment table (ERST). Section 4.9.3.
*/
xhci_dbg_trace(xhci, trace_xhci_dbg_init, // Allocating event ring);
- xhci-event_ring = xhci_ring_alloc(xhci, ERST_NUM_SEGS, 1, TYPE_EVENT,
+ xhci-event_ring = xhci_ring_alloc(xhci, ERST_NUM_SEGS, TRB_CYCLE,
TYPE_EVENT,
flags);
if (!xhci-event_ring)
goto fail;
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index d3f4a9a..408978b 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -178,7 +178,7 @@ static void inc_deq(struct xhci_hcd *xhci, struct
xhci_ring *ring)
if (ring-type == TYPE_EVENT
last_trb_on_last_seg(xhci, ring,
ring-deq_seg, ring-dequeue)) {
- ring-cycle_state ^= 1;
+ ring-cycle_state ^= TRB_CYCLE;
}
ring-deq_seg = ring-deq_seg-next;
ring-dequeue = ring-deq_seg-trbs;
@@ -257,7 +257,7 @@ static void inc_enq(struct xhci_hcd *xhci, struct
xhci_ring *ring,
Re: [PATCH v6 08/15] usb: phy-mxs: Add implementation of nofity_suspend and notify_resume
On Fri, Dec 13, 2013 at 02:31:42PM +0800, Peter Chen wrote:
On Fri, Dec 13, 2013 at 12:32 PM, Felipe Balbi ba...@ti.com wrote:
On Fri, Dec 13, 2013 at 09:23:38AM +0800, Peter Chen wrote:
Implementation of notify_suspend and notify_resume will be different
according to mxs_phy_data-flags.
Signed-off-by: Peter Chen peter.c...@freescale.com
---
drivers/usb/phy/phy-mxs-usb.c | 55
++---
1 files changed, 51 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/phy/phy-mxs-usb.c b/drivers/usb/phy/phy-mxs-usb.c
index 0ef930a..e3df53f 100644
--- a/drivers/usb/phy/phy-mxs-usb.c
+++ b/drivers/usb/phy/phy-mxs-usb.c
@@ -166,8 +166,8 @@ static int mxs_phy_suspend(struct usb_phy *x, int
suspend)
static int mxs_phy_on_connect(struct usb_phy *phy,
enum usb_device_speed speed)
{
- dev_dbg(phy-dev, %s speed device has connected\n,
- (speed == USB_SPEED_HIGH) ? high : non-high);
+ dev_dbg(phy-dev, %s device has connected\n,
+ (speed == USB_SPEED_HIGH) ? HS : FS/LS);
unrelated.
@@ -179,8 +179,8 @@ static int mxs_phy_on_connect(struct usb_phy *phy,
static int mxs_phy_on_disconnect(struct usb_phy *phy,
enum usb_device_speed speed)
{
- dev_dbg(phy-dev, %s speed device has disconnected\n,
- (speed == USB_SPEED_HIGH) ? high : non-high);
+ dev_dbg(phy-dev, %s device has disconnected\n,
+ (speed == USB_SPEED_HIGH) ? HS : FS/LS);
unrelated.
Marek suggested using that string, I will added it at another patch.
@@ -189,6 +189,48 @@ static int mxs_phy_on_disconnect(struct usb_phy *phy,
return 0;
}
+static int mxs_phy_on_suspend(struct usb_phy *phy,
+ enum usb_device_speed speed)
+{
+ struct mxs_phy *mxs_phy = to_mxs_phy(phy);
+
+ dev_dbg(phy-dev, %s device has suspended\n,
+ (speed == USB_SPEED_HIGH) ? HS : FS/LS);
+
+ /* delay 4ms to wait bus entering idle */
+ usleep_range(4000, 5000);
+
+ if (mxs_phy-data-flags MXS_PHY_ABNORMAL_IN_SUSPEND) {
+ writel_relaxed(0x, phy-io_priv + HW_USBPHY_PWD);
+ writel_relaxed(0, phy-io_priv + HW_USBPHY_PWD);
+ }
+
+ if (speed == USB_SPEED_HIGH)
+ writel_relaxed(BM_USBPHY_CTRL_ENHOSTDISCONDETECT,
+ phy-io_priv + HW_USBPHY_CTRL_CLR);
why only on HS ? So if !HS and !ABNORMAL, this is no-op.
+static int mxs_phy_on_resume(struct usb_phy *phy,
+ enum usb_device_speed speed)
+{
+ dev_dbg(phy-dev, %s device has resumed\n,
+ (speed == USB_SPEED_HIGH) ? HS : FS/LS);
+
+ if (speed == USB_SPEED_HIGH) {
+ /* Make sure the device has switched to High-Speed mode */
+ udelay(500);
+ writel_relaxed(BM_USBPHY_CTRL_ENHOSTDISCONDETECT,
+ phy-io_priv + HW_USBPHY_CTRL_SET);
+ }
likewise, if !HS it's a no-op.
Correct, this operation is only needed for HS.
@@ -235,6 +277,11 @@ static int mxs_phy_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, mxs_phy);
+ if (mxs_phy-data-flags MXS_PHY_SENDING_SOF_TOO_FAST) {
+ mxs_phy-phy.notify_suspend = mxs_phy_on_suspend;
+ mxs_phy-phy.notify_resume = mxs_phy_on_resume;
+ }
hmm, and seems like you only need notify_* on a buggy device. Sorry
Peter but you don't have enough arguments to make me agree with this
(and previous) patch.
You gotta find a better way to handle this using normal phy
suspend/resume calls.
Like I explained at previous patch, it needs to be notified during
ehci suspend/resume.
I admit it is a SoC bug, but all SoCs have bugs, hmm.
Software needs the solution to workaround it which breaks the standard USB
spec.
Then I think what you need is a real notification mechanism. usbcore
already notifies about buses and devices being added and removed,
perhaps you can convince Greg to accept suspend/resume notifications.
With that, you can (conditionally) make this driver listen to usbcore
notifications. That'll be more work, but I guess it's best in the long
run as we won't need to keep on adding callbacks to the USB PHY
structure just because another buggy device showed up on the market.
--
balbi
signature.asc
Description: Digital signature
RE: [PATCH v2 1/7] usb: dwc3: keystone: add basic PM support
-Original Message- From: Balbi, Felipe Sent: Friday, December 13, 2013 2:55 PM To: Kwok, WingMan Cc: Shilimkar, Santosh; Balbi, Felipe; Linux USB Mailing List; kgene@samsung.com; Linux ARM Kernel Mailing List; linux-samsung- s...@vger.kernel.org; Linux OMAP Mailing List Subject: Re: [PATCH v2 1/7] usb: dwc3: keystone: add basic PM support On Fri, Dec 13, 2013 at 10:04:38AM -0600, Kwok, WingMan wrote: Hello -Original Message- From: Shilimkar, Santosh Sent: Thursday, December 12, 2013 7:29 PM To: Balbi, Felipe Cc: Linux USB Mailing List; kgene@samsung.com; Linux ARM Kernel Mailing List; linux-samsung-...@vger.kernel.org; Linux OMAP Mailing List; Kwok, WingMan Subject: Re: [PATCH v2 1/7] usb: dwc3: keystone: add basic PM support On Thursday 12 December 2013 04:45 PM, Felipe Balbi wrote: A bare-minimum PM implementation which will server as building block for more complex s/server/serve ;-) PM implementation in the future. At the least will not leave clocks on unnecessarily when e.g. a user write mem to /sys/power/state. Signed-off-by: Felipe Balbi ba...@ti.com --- improve error path a little bit. We will test this out. Thanks for the patch Felipe. I have tested the patch and the keystone usb driver continues to function, though I can't test suspend at this time as the rest of the system does not that functionality yet. Thanks, should I add your Tested-by ? Yes please. Thanks WingMan -- To unsubscribe from this list: send the line unsubscribe 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 7/7] usb: dwc3: exynos: add pm_runtime support
On Fri, Dec 13, 2013 at 01:56:18PM -0600, Felipe Balbi wrote:
On Fri, Dec 13, 2013 at 02:01:32PM +0900, Anton Tikhomirov wrote:
Hi Felipe,
-static int dwc3_exynos_suspend(struct device *dev)
+static int __dwc3_exynos_suspend(struct dwc3_exynos *exynos)
{
- struct dwc3_exynos *exynos = dev_get_drvdata(dev);
-
clk_disable(exynos-clk);
return 0;
}
+static int __dwc3_exynos_resume(struct dwc3_exynos *exynos)
+{
+ return clk_enable(exynos-clk);
+}
+
+static int dwc3_exynos_suspend(struct device *dev)
+{
+ struct dwc3_exynos *exynos = dev_get_drvdata(dev);
+
+ return __dwc3_exynos_suspend(exynos);
If dwc3-exynos is runtime suspended, the clock will be disabled
second time here (unbalanced clk_enable/clk_disable).
I don't get what you mean but there is something that probably needs
fixing, I guess below makes it better:
diff --git a/drivers/usb/dwc3/dwc3-exynos.c b/drivers/usb/dwc3/dwc3-exynos.c
index c93919a..1e5720a 100644
--- a/drivers/usb/dwc3/dwc3-exynos.c
+++ b/drivers/usb/dwc3/dwc3-exynos.c
@@ -218,6 +218,9 @@ static int dwc3_exynos_suspend(struct device *dev)
{
struct dwc3_exynos *exynos = dev_get_drvdata(dev);
+ if (pm_runtime_suspended(dev))
+ return 0;
+
return __dwc3_exynos_suspend(exynos);
}
Is that what you meant ?
note, however, that this is *not* a case where we would fall today. See
that we pm_runtime_get() in probe and only pm_runtime_put() during
remove. So there would never be a case where we would try system suspend
while device was already runtime suspended.
I have fixed all patches in my testing/next branch anyway, just to make
sure we're idiot-proof when it comes to implementing real runtime pm
later on :-)
cheers
--
balbi
signature.asc
Description: Digital signature
Re: [usb-storage] UAS hangs khubd on USB disconnect
Hi James, On 12/13/2013 09:03 PM, James Bottomley wrote: On Fri, 2013-12-13 at 11:18 -0800, James Bottomley wrote: On Fri, 2013-12-13 at 13:33 -0500, Tejun Heo wrote: Hello, guys. (cc'ing Greg) On Fri, Dec 13, 2013 at 01:19:36PM -0500, Alan Stern wrote: On Fri, 13 Dec 2013, Sarah Sharp wrote: Given the way things work now, I suspect these warnings are truly harmless. We could simply get rid of the WARN in sysfs_remove_group The alternative is to call device_del for SCSI targets earlier on, such as when their hosts are unregistered. I don't know how James would feel about this approach. It would be difficult because targets use their own reference counts instead of relying on the usual device refcounting mechanism. Thanks for looking into this. I think just getting rid of the WARN would be sufficient. Can you make a patch for that? Easily. The downside is that there would no longer be any warning when someone tries to remove a wrong subdirectory by mistake. The patch still won't help with the UAS issues with scsi_init_shared_tag_map though. I wasn't clear on the reason for that problem. Does it also arise from late device_del for scsi_target? I could try to change the way that works, if anybody (Hans?) would like to test it. While the recent sysfs changes made this issue more visible, Greg wants to make sure that devices are removed from leaf up in all cases and keep the warning to ensure that. Would there be a way fix SCSI removal ordering? Could someone analyse the actual problem? We're quite careful even on host remove to iterate and remove all the devices, then targets, then host (and allied transport objects). Which removal is inverted? Actually, I think I have this figured out. There's a thinko in one of the scsi_target_reap() cases. The original (and still existing) problem with targets is that nothing creates them and nothing destroys them, so, while we could rely on the refcounting of the device model to preserve the actual target object, we had no idea when to remove it from visibility. That was the job of the reap reference, to track visibility. It looks like the reap on device last put is occurring too late. I think we should reap immediately after doing the sdev device_del, so does this fix the warn on? (I'm not sure because no-one has actually posted a backtrace, but it sounds like this is the problem). Thanks I'll give this patch a try. As for backtraces I've posted some (partial) backtraces as well as reproduction instructions here: http://www.spinics.net/lists/linux-scsi/msg70002.html 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 v2 1/7] usb: dwc3: keystone: add basic PM support
On Fri, Dec 13, 2013 at 02:18:42PM -0600, Kwok, WingMan wrote: -Original Message- From: Balbi, Felipe Sent: Friday, December 13, 2013 2:55 PM To: Kwok, WingMan Cc: Shilimkar, Santosh; Balbi, Felipe; Linux USB Mailing List; kgene@samsung.com; Linux ARM Kernel Mailing List; linux-samsung- s...@vger.kernel.org; Linux OMAP Mailing List Subject: Re: [PATCH v2 1/7] usb: dwc3: keystone: add basic PM support On Fri, Dec 13, 2013 at 10:04:38AM -0600, Kwok, WingMan wrote: Hello -Original Message- From: Shilimkar, Santosh Sent: Thursday, December 12, 2013 7:29 PM To: Balbi, Felipe Cc: Linux USB Mailing List; kgene@samsung.com; Linux ARM Kernel Mailing List; linux-samsung-...@vger.kernel.org; Linux OMAP Mailing List; Kwok, WingMan Subject: Re: [PATCH v2 1/7] usb: dwc3: keystone: add basic PM support On Thursday 12 December 2013 04:45 PM, Felipe Balbi wrote: A bare-minimum PM implementation which will server as building block for more complex s/server/serve ;-) PM implementation in the future. At the least will not leave clocks on unnecessarily when e.g. a user write mem to /sys/power/state. Signed-off-by: Felipe Balbi ba...@ti.com --- improve error path a little bit. We will test this out. Thanks for the patch Felipe. I have tested the patch and the keystone usb driver continues to function, though I can't test suspend at this time as the rest of the system does not that functionality yet. Thanks, should I add your Tested-by ? Yes please. you need to reply with Tested-by: Your Name your.em...@domain.com just to make it all official. Sorry -- balbi signature.asc Description: Digital signature
Re: [usb-storage] UAS hangs khubd on USB disconnect
On Fri, 13 Dec 2013, James Bottomley wrote: Actually, I think I have this figured out. There's a thinko in one of the scsi_target_reap() cases. The original (and still existing) problem with targets is that nothing creates them and nothing destroys them, so, while we could rely on the refcounting of the device model to preserve the actual target object, we had no idea when to remove it from visibility. That was the job of the reap reference, to track visibility. It looks like the reap on device last put is occurring too late. I think we should reap immediately after doing the sdev device_del, so does this fix the warn on? (I'm not sure because no-one has actually posted a backtrace, but it sounds like this is the problem). James --- diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index 8ff62c2..98d4eb3 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -399,8 +399,6 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work) /* NULL queue means the device can't be used */ sdev-request_queue = NULL; - scsi_target_reap(scsi_target(sdev)); - kfree(sdev-inquiry); kfree(sdev); @@ -1044,6 +1042,8 @@ void __scsi_remove_device(struct scsi_device *sdev) } else put_device(sdev-sdev_dev); + scsi_target_reap(scsi_target(sdev)); + /* * Stop accepting new requests and wait until all queuecommand() and * scsi_run_queue() invocations have finished before tearing down the This is not right. The problem is that you don't keep track explicitly of the number of references to a target; you rely implicitly on starget-devices being non-empty. starget-reap_ref is only a count of local operations that should block removal. Consider, for example, what would happen if there is more than one LUN. What if one of them is removed while the other remains? A more invasive change is needed. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [usb-storage] UAS hangs khubd on USB disconnect
On Fri, Dec 13, 2013 at 12:03:19PM -0800, James Bottomley wrote: Actually, I think I have this figured out. There's a thinko in one of the scsi_target_reap() cases. The original (and still existing) problem with targets is that nothing creates them and nothing destroys them, so, while we could rely on the refcounting of the device model to preserve the actual target object, we had no idea when to remove it from visibility. That was the job of the reap reference, to track visibility. It looks like the reap on device last put is occurring too late. I think we should reap immediately after doing the sdev device_del, so does this fix the warn on? (I'm not sure because no-one has actually posted a backtrace, but it sounds like this is the problem). I can confirm that this patch fixes both the sysfs warning, and the issue with USB storage disconnect during video playback. I did trigger a new (possibly unrelated?) mutex deadlock warning. dmesg is attached. Sarah Sharp --- diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index 8ff62c2..98d4eb3 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -399,8 +399,6 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work) /* NULL queue means the device can't be used */ sdev-request_queue = NULL; - scsi_target_reap(scsi_target(sdev)); - kfree(sdev-inquiry); kfree(sdev); @@ -1044,6 +1042,8 @@ void __scsi_remove_device(struct scsi_device *sdev) } else put_device(sdev-sdev_dev); + scsi_target_reap(scsi_target(sdev)); + /* * Stop accepting new requests and wait until all queuecommand() and * scsi_run_queue() invocations have finished before tearing down the Dec 13 13:02:02 xanatos kernel: [7.029300] usb usb4: bus auto-suspend, wakeup 1 Dec 13 13:02:02 xanatos kernel: [7.040327] input: SynPS/2 Synaptics TouchPad as /devices/platform/i8042/serio1/input/input11 Dec 13 13:02:02 xanatos kernel: [7.112065] btusb 3-1.4:1.0: usb_probe_interface Dec 13 13:02:02 xanatos kernel: [7.112070] btusb 3-1.4:1.0: usb_probe_interface - got id Dec 13 13:02:02 xanatos kernel: [7.122731] usbcore: registered new interface driver btusb Dec 13 13:02:02 xanatos kernel: [7.167710] Linux video capture interface: v2.00 Dec 13 13:02:02 xanatos kernel: [7.235181] uvcvideo 3-1.6:1.0: usb_probe_interface Dec 13 13:02:02 xanatos kernel: [7.235187] uvcvideo 3-1.6:1.0: usb_probe_interface - got id Dec 13 13:02:02 xanatos kernel: [7.235293] uvcvideo: Found UVC 1.00 device Integrated Camera (04f2:b2ea) Dec 13 13:02:02 xanatos kernel: [7.242661] input: Integrated Camera as /devices/pci:00/:00:1a.0/usb3/3-1/3-1.6/3-1.6:1.0/input/input20 Dec 13 13:02:02 xanatos kernel: [7.244470] usbcore: registered new interface driver uvcvideo Dec 13 13:02:02 xanatos kernel: [7.244473] USB Video Class driver (1.1.1) Dec 13 13:02:03 xanatos kernel: [8.044806] bio: create slab bio-2 at 2 Dec 13 13:02:03 xanatos kernel: [8.261355] Adding 4085756k swap on /dev/mapper/cryptswap1. Priority:-1 extents:1 across:4085756k SSFS Dec 13 13:02:03 xanatos kernel: [8.407323] e1000e :00:19.0: irq 44 for MSI/MSI-X Dec 13 13:02:03 xanatos kernel: [8.510442] e1000e :00:19.0: irq 44 for MSI/MSI-X Dec 13 13:02:03 xanatos kernel: [8.510945] IPv6: ADDRCONF(NETDEV_UP): eth1: link is not ready Dec 13 13:02:03 xanatos kernel: [8.516037] iwlwifi :03:00.0: L1 Enabled; Disabling L0S Dec 13 13:02:03 xanatos kernel: [8.517364] iwlwifi :03:00.0: Radio type=0x1-0x2-0x0 Dec 13 13:02:03 xanatos kernel: [8.785685] iwlwifi :03:00.0: L1 Enabled; Disabling L0S Dec 13 13:02:03 xanatos kernel: [8.792724] iwlwifi :03:00.0: Radio type=0x1-0x2-0x0 Dec 13 13:02:04 xanatos kernel: [8.876409] IPv6: ADDRCONF(NETDEV_UP): wlan1: link is not ready Dec 13 13:02:04 xanatos kernel: [9.787586] usb 3-1.6: usb auto-suspend, wakeup 0 Dec 13 13:02:05 xanatos kernel: [9.910341] psmouse serio2: alps: Unknown ALPS touchpad: E7=10 00 64, EC=10 00 64 Dec 13 13:02:06 xanatos kernel: [ 11.169530] psmouse serio2: trackpoint: IBM TrackPoint firmware: 0x0e, buttons: 3/3 Dec 13 13:02:06 xanatos kernel: [ 11.375342] input: TPPS/2 IBM TrackPoint as /devices/platform/i8042/serio1/serio2/input/input19 Dec 13 13:02:07 xanatos kernel: [ 11.917809] e1000e: eth1 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: Rx/Tx Dec 13 13:02:07 xanatos kernel: [ 11.917863] IPv6: ADDRCONF(NETDEV_CHANGE): eth1: link becomes ready Dec 13 13:02:12 xanatos kernel: [ 17.125215] Dec 13 13:02:12 xanatos kernel: [ 17.125218] == Dec 13 13:02:12 xanatos kernel: [ 17.125219] [ INFO: possible circular locking dependency detected ] Dec 13 13:02:12 xanatos kernel: [ 17.125221] 3.13.0-rc1+ #140 Not tainted Dec 13 13:02:12
Re: [usb-storage] UAS hangs khubd on USB disconnect
On Fri, 2013-12-13 at 16:06 -0500, Alan Stern wrote: On Fri, 13 Dec 2013, James Bottomley wrote: Actually, I think I have this figured out. There's a thinko in one of the scsi_target_reap() cases. The original (and still existing) problem with targets is that nothing creates them and nothing destroys them, so, while we could rely on the refcounting of the device model to preserve the actual target object, we had no idea when to remove it from visibility. That was the job of the reap reference, to track visibility. It looks like the reap on device last put is occurring too late. I think we should reap immediately after doing the sdev device_del, so does this fix the warn on? (I'm not sure because no-one has actually posted a backtrace, but it sounds like this is the problem). James --- diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index 8ff62c2..98d4eb3 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -399,8 +399,6 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work) /* NULL queue means the device can't be used */ sdev-request_queue = NULL; - scsi_target_reap(scsi_target(sdev)); - kfree(sdev-inquiry); kfree(sdev); @@ -1044,6 +1042,8 @@ void __scsi_remove_device(struct scsi_device *sdev) } else put_device(sdev-sdev_dev); + scsi_target_reap(scsi_target(sdev)); + /* * Stop accepting new requests and wait until all queuecommand() and * scsi_run_queue() invocations have finished before tearing down the This is not right. The problem is that you don't keep track explicitly of the number of references to a target; you rely implicitly on starget-devices being non-empty. starget-reap_ref is only a count of local operations that should block removal. No, it was supposed explicitly to be a visibility counter to answer the question when can we delete the target. It's incremented every time we add a device to the target (and when we do an operation that may remove one to keep an atomic context before we blow it away) and decremented every time we remove one. Consider, for example, what would happen if there is more than one LUN. What if one of them is removed while the other remains? Then the reap reference remains above zero and the target stays. A more invasive change is needed. I think you might be right in that we need to kill the list_empty check, but I think that should be it. James -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [usb-storage] UAS hangs khubd on USB disconnect
Hi, On 12/13/2013 09:03 PM, James Bottomley wrote: snip Actually, I think I have this figured out. There's a thinko in one of the scsi_target_reap() cases. The original (and still existing) problem with targets is that nothing creates them and nothing destroys them, so, while we could rely on the refcounting of the device model to preserve the actual target object, we had no idea when to remove it from visibility. That was the job of the reap reference, to track visibility. It looks like the reap on device last put is occurring too late. I think we should reap immediately after doing the sdev device_del, so does this fix the warn on? (I'm not sure because no-one has actually posted a backtrace, but it sounds like this is the problem). James --- diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index 8ff62c2..98d4eb3 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -399,8 +399,6 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work) /* NULL queue means the device can't be used */ sdev-request_queue = NULL; - scsi_target_reap(scsi_target(sdev)); - kfree(sdev-inquiry); kfree(sdev); @@ -1044,6 +1042,8 @@ void __scsi_remove_device(struct scsi_device *sdev) } else put_device(sdev-sdev_dev); + scsi_target_reap(scsi_target(sdev)); + /* * Stop accepting new requests and wait until all queuecommand() and * scsi_run_queue() invocations have finished before tearing down the I've given this patch a try and it fixes the blk-tag.c: 89 BUG() I was seeing. As for the other patch you (James) have send for that problem: diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index 8ff62c2..98d4eb3 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -399,8 +399,6 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work) /* NULL queue means the device can't be used */ sdev-request_queue = NULL; - scsi_target_reap(scsi_target(sdev)); - kfree(sdev-inquiry); kfree(sdev); @@ -1044,6 +1042,8 @@ void __scsi_remove_device(struct scsi_device *sdev) } else put_device(sdev-sdev_dev); + scsi_target_reap(scsi_target(sdev)); + /* * Stop accepting new requests and wait until all queuecommand() and * scsi_run_queue() invocations have finished before tearing down the That too fixes the blk-tag.c: 89 BUG() I was seeing. Either patch by itself seems to be enough to fix this issue for me. Thanks 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 v2 1/7] usb: dwc3: keystone: add basic PM support
-Original Message- From: Balbi, Felipe Sent: Friday, December 13, 2013 3:23 PM To: Kwok, WingMan Cc: Balbi, Felipe; Shilimkar, Santosh; Linux USB Mailing List; kgene@samsung.com; Linux ARM Kernel Mailing List; linux-samsung- s...@vger.kernel.org; Linux OMAP Mailing List Subject: Re: [PATCH v2 1/7] usb: dwc3: keystone: add basic PM support On Fri, Dec 13, 2013 at 02:18:42PM -0600, Kwok, WingMan wrote: -Original Message- From: Balbi, Felipe Sent: Friday, December 13, 2013 2:55 PM To: Kwok, WingMan Cc: Shilimkar, Santosh; Balbi, Felipe; Linux USB Mailing List; kgene@samsung.com; Linux ARM Kernel Mailing List; linux- samsung- s...@vger.kernel.org; Linux OMAP Mailing List Subject: Re: [PATCH v2 1/7] usb: dwc3: keystone: add basic PM support On Fri, Dec 13, 2013 at 10:04:38AM -0600, Kwok, WingMan wrote: Hello -Original Message- From: Shilimkar, Santosh Sent: Thursday, December 12, 2013 7:29 PM To: Balbi, Felipe Cc: Linux USB Mailing List; kgene@samsung.com; Linux ARM Kernel Mailing List; linux-samsung-...@vger.kernel.org; Linux OMAP Mailing List; Kwok, WingMan Subject: Re: [PATCH v2 1/7] usb: dwc3: keystone: add basic PM support On Thursday 12 December 2013 04:45 PM, Felipe Balbi wrote: A bare-minimum PM implementation which will server as building block for more complex s/server/serve ;-) PM implementation in the future. At the least will not leave clocks on unnecessarily when e.g. a user write mem to /sys/power/state. Signed-off-by: Felipe Balbi ba...@ti.com --- improve error path a little bit. We will test this out. Thanks for the patch Felipe. I have tested the patch and the keystone usb driver continues to function, though I can't test suspend at this time as the rest of the system does not that functionality yet. Thanks, should I add your Tested-by ? Yes please. you need to reply with Tested-by: Your Name your.em...@domain.com just to make it all official. Sorry Yes, you can add Tested-by: WingMan Kwok w-kw...@ti.com -- To unsubscribe from this list: send the line unsubscribe 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] usbtest: Fix BOS control test for USB 2.01 devices.
Commit c952a8ba7136505cd1ca01735cc748ddc08c7d2f usb: usbtest: add a test case to support bos for queue control will cause USB 2.01 and USB 2.10 devices with a BOS descriptor to fail case 15 of the control test. The Link PM errata (released in 2007, updated in 2011) says: The value of the bcdUSB field in the standard USB 2.0 Device Descriptor is used to indicate that the device supports the request to read the BOS Descriptor (i.e. GetDescriptor(BOS)). Devices that support the BOS descriptor must have a bcdUSB value of 0201H or larger. The current code says that non-SuperSpeed devices *must* return -EPIPE, as this comment shows: /* sign of this variable means: * -: tested code must return this (negative) error code * +: tested code may return this (negative too) error code */ int expected = 0; This means the test will fail with USB 2.01 and USB 2.10 devices that provide a BOS descriptor. Change it to only require a stall response if the USB device bcdUSB is less than 2.01. Signed-off-by: Sarah Sharp sarah.a.sh...@linux.intel.com Cc: Huang Rui ray.hu...@amd.com --- drivers/usb/misc/usbtest.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c index bff058ea222e..446ff55e3c58 100644 --- a/drivers/usb/misc/usbtest.c +++ b/drivers/usb/misc/usbtest.c @@ -1224,7 +1224,7 @@ test_ctrl_queue(struct usbtest_dev *dev, struct usbtest_param *param) len = le16_to_cpu(udev-bos-desc-wTotalLength); else len = sizeof(struct usb_bos_descriptor); - if (udev-speed != USB_SPEED_SUPER) + if (le16_to_cpu(udev-descriptor.bcdUSB) 0x0201) expected = -EPIPE; break; default: -- 1.8.3.3 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: host: xhci: Move suspend ops under PM_SLEEP to avoid warning
On Friday 13 December 2013 12:23 AM, David Cohen wrote: On Thu, Dec 12, 2013 at 07:25:55PM -0800, David Cohen wrote: On Thu, Dec 12, 2013 at 09:01:04PM -0500, Santosh Shilimkar wrote: On Thursday 12 December 2013 08:51 PM, David Cohen wrote: On Thu, Dec 12, 2013 at 08:06:24PM -0500, Santosh Shilimkar wrote: Otherwise you get below build warnings drivers/usb/host/xhci-plat.c:201:12: warning: ‘xhci_plat_suspend’ defined but not used [-Wunused-function] drivers/usb/host/xhci-plat.c:209:12: warning: ‘xhci_plat_resume’ defined but not used [-Wunused-function] Cc: Sarah Sharp sarah.a.sh...@linux.intel.com Cc: Greg Kroah-Hartman gre...@linuxfoundation.org Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com --- drivers/usb/host/xhci-plat.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c index d9c169f..4875be5 100644 --- a/drivers/usb/host/xhci-plat.c +++ b/drivers/usb/host/xhci-plat.c @@ -197,7 +197,7 @@ static int xhci_plat_remove(struct platform_device *dev) return 0; } -#ifdef CONFIG_PM +#ifdef CONFIG_PM_SLEEP Can't you just remove these #ifdefs altogether? xhci_plat_pm_ops is set using SET_SYSTEM_SLEEP_PM_OPS() macro which already handles '#ifdef CONFIG_PM_SLEEP' case. It does handle the difference but the hooks implemented would show-up un-used warning if you remove the #ifdefs. drivers/usb/host/xhci-plat.c:200:12: warning: ‘xhci_plat_suspend’ defined but not used [-Wunused-function] drivers/usb/host/xhci-plat.c:208:12: warning: ‘xhci_plat_resume’ defined but not used [-Wunused-function] So you need to wrap them under the PM_SLEEP check. Yeah... it's not smart enought :) But you could still remove the #else condition and the ugly DEV_PM_OPS macro. Since this patch is not urgent, I sent a RFC proposing smarter SET_*_PM_OPS(). I included your patch (a bit different) here: https://patchwork.kernel.org/patch/3337961/ Thats fine by me if you can get your RFC through. Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/7] usb: dwc3: keystone: add basic PM support
On Thursday 12 December 2013 07:43 PM, Felipe Balbi wrote: On Thu, Dec 12, 2013 at 07:29:24PM -0500, Santosh Shilimkar wrote: On Thursday 12 December 2013 04:45 PM, Felipe Balbi wrote: A bare-minimum PM implementation which will server as building block for more complex s/server/serve ;-) hah! :-) will fix in my branch. PM implementation in the future. At the least will not leave clocks on unnecessarily when e.g. a user write mem to /sys/power/state. Signed-off-by: Felipe Balbi ba...@ti.com --- improve error path a little bit. We will test this out. Thanks for the patch Felipe. I see Wingman already tested the patch. Feel free add, my ack if you need one... Acked-by: Santosh Shilimkar santosh.shilim...@ti.com -- To unsubscribe from this list: send the line unsubscribe 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: [PATH] xhci: fix array index out of the bounds in function last_trb() and last_trb_on_last_seg().
On Tue, Nov 12, 2013 at 08:46:27AM +, Wang, Lin X wrote:
Hi Sarah,
Hi Lin,
Sorry for taking so long to respond to this patch.
Yes, I think this is a bug, but the end result is harmless. Please send
an updated version of the patch to me, and Cc the linux-usb mailing
list.
I found a potential array index out of bounds issue in xhci driver, according
to my understanding, the number of TRBs inside a segment is
TRBS_PER_SEGMENT which is 64 in current driver,
here seg-trbs[TRBS_PER_SEGMENT] means the 65th element in TRB array which
cross the border of seg-trbs[]. Is this a bug which should be fixed?
[85767.775733] address: trb = 88023ee267f0
[85767.775734] address: seg- seg-trbs[TRBS_PER_SEGMENT-1] =
88023ee267f0 // for a array with size equal to TRBS_PER_SEGMENT,
seg-trbs[TRBS_PER_SEGMENT-1]should be the last element
in this segment, seg-trbs[TRBS_PER_SEGMENT] as the last element in an array
doesn't make sense.
[85767.775735] address: seg-trbs[TRBS_PER_SEGMENT] = 88023ee26800 // if
segments in segment_pool are not contiguous, seg-trbs[TRBS_PER_SEGMENT-1] +
16 would not equal to seg-trbs[TRBS_PER_SEGMENT],
here might be a coincidence, or return
trb == seg-trbs[TRBS_PER_SEGMENT] will never be true.
8-8
This patch fix array index out of the bounds in function ast_trb() and
last_trb_on_last_seg().
This should be last_trb() instead of ast_trb(). Also, please line-wrap
your commit message bodies to at least 80 characters, except when
copy-pasting dmesg or warning output. Finally, this patch doesn't apply
to my for-usb-next-queue branch. Please rebase it.
More comments below.
Signed-off-by: Lin Wang lin.x.w...@intel.com
---
drivers/usb/host/xhci-ring.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 1e2f3f4..7dc8a24 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -98,21 +98,21 @@ static bool last_trb_on_last_seg(struct xhci_hcd *xhci,
struct xhci_ring *ring,
struct xhci_segment *seg, union xhci_trb *trb)
{
if (ring == xhci-event_ring)
- return (trb == seg-trbs[TRBS_PER_SEGMENT])
+ return (trb == seg-trbs[TRBS_PER_SEGMENT - 1])
(seg-next == xhci-event_ring-first_seg);
else
return le32_to_cpu(trb-link.control) LINK_TOGGLE;
}
-/* Is this TRB a link TRB or was the last TRB the last TRB in this event ring
- * segment? I.e. would the updated event TRB pointer step off the end of the
- * event seg?
+/* Is this TRB a link TRB or was the last TRB in this event ring segment?
+ * I.e. would the updated event TRB pointer step off the end of the event
+ * seg?
*/
static int last_trb(struct xhci_hcd *xhci, struct xhci_ring *ring,
struct xhci_segment *seg, union xhci_trb *trb)
{
if (ring == xhci-event_ring)
- return trb == seg-trbs[TRBS_PER_SEGMENT];
+ return trb == seg-trbs[TRBS_PER_SEGMENT - 1];
else
return TRB_TYPE_LINK_LE32(trb-link.control);
}
--
1.7.9.5
I drilled down into the behavior of the functions that call both
last_trb and last_trb_on_last_seg, and without your patch, they still
work as desired.
last_trb and last_trb_on_last_seg are called in:
next_trb()
- called from xhci_find_new_dequeue_state, td_to_noop,
xhci_cmd_to_noop, process_isoc_td, and process_bulk_intr_td. These
functions are never called for event rings, so the code you changed
never runs.
update_ring_for_set_deq_completion()
prepare_ring()
inc_enq()
- not called for event rings.
inc_deq()
- is called for event rings.
static void inc_deq(struct xhci_hcd *xhci, struct xhci_ring *ring)
{
unsigned long long addr;
ring-deq_updates++;
/*
* If this is not event ring, and the dequeue pointer
* is not on a link TRB, there is one more usable TRB
*/
if (ring-type != TYPE_EVENT
!last_trb(xhci, ring, ring-deq_seg, ring-dequeue))
ring-num_trbs_free++;
do {
/*
* Update the dequeue pointer further if that was a link TRB or
* we're at the end of an event ring segment (which doesn't have
* link TRBS)
*/
if (last_trb(xhci, ring, ring-deq_seg, ring-dequeue)) {
if (ring-type == TYPE_EVENT
last_trb_on_last_seg(xhci, ring,
ring-deq_seg, ring-dequeue)) {
ring-cycle_state ^= 1;
}
ring-deq_seg = ring-deq_seg-next;
Re: UAS/xHC changes for streams
On Thu, Dec 12, 2013 at 06:37:28PM +, Ashwini Pahuja wrote: Hi Sarah, I am planning to use the UAS/streams feature under Linux, I know there were a lot of patches submitted by Hans in the last few weeks. Do you have any git repository from where I can pull the latest kernel with these changes? https://git.kernel.org/cgit/linux/kernel/git/sarah/xhci.git/log/?h=for-usb-next-streams I'm not going to merge the UAS driver until this bug is fixed: http://marc.info/?l=linux-scsim=138685401118908w=2 The HEAD commit does fix it, but it's still under discussion: http://marc.info/?l=linux-scsim=138696954025093w=2 First, I will try these patches on a TI xHC card and a Pluggable USB3.0 UASP capable device, the streams work fine under Windows 8 with these Host/device. I'm using the Plugable UAS device as well. My purpose of using streams is to validate the stream feature on our xHC controller. Broadcom has an xHCI host controller? That's the first I've heard from that vendor. Let me know how your testing goes. If you find any issues in either the xHCI driver or the UAS driver, please let us know. BTW, do you know any other xHC cards/devices in the market which support streams? Intel xHCI hosts. I believe some NEC hosts as well. I'm not sure about other vendors. Obviously you already know TI supports streams. Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [usb-storage] UAS hangs khubd on USB disconnect
On Fri, 13 Dec 2013, James Bottomley wrote:
On Fri, 2013-12-13 at 16:06 -0500, Alan Stern wrote:
On Fri, 13 Dec 2013, James Bottomley wrote:
Actually, I think I have this figured out. There's a thinko in one of
the scsi_target_reap() cases. The original (and still existing) problem
with targets is that nothing creates them and nothing destroys them, so,
while we could rely on the refcounting of the device model to preserve
the actual target object, we had no idea when to remove it from
visibility. That was the job of the reap reference, to track
visibility. It looks like the reap on device last put is occurring too
late. I think we should reap immediately after doing the sdev
device_del, so does this fix the warn on? (I'm not sure because no-one
has actually posted a backtrace, but it sounds like this is the
problem).
James
---
diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
index 8ff62c2..98d4eb3 100644
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -399,8 +399,6 @@ static void
scsi_device_dev_release_usercontext(struct work_struct *work)
/* NULL queue means the device can't be used */
sdev-request_queue = NULL;
- scsi_target_reap(scsi_target(sdev));
-
kfree(sdev-inquiry);
kfree(sdev);
@@ -1044,6 +1042,8 @@ void __scsi_remove_device(struct scsi_device *sdev)
} else
put_device(sdev-sdev_dev);
+ scsi_target_reap(scsi_target(sdev));
+
/*
* Stop accepting new requests and wait until all queuecommand() and
* scsi_run_queue() invocations have finished before tearing down the
This is not right. The problem is that you don't keep track explicitly
of the number of references to a target; you rely implicitly on
starget-devices being non-empty. starget-reap_ref is only a count of
local operations that should block removal.
No, it was supposed explicitly to be a visibility counter to answer the
question when can we delete the target. It's incremented every time we
add a device to the target (and when we do an operation that may remove
one to keep an atomic context before we blow it away) and decremented
every time we remove one.
Sorry, but you're wrong. starget-reap_ref is _not_ incremented every
time we add a device to the target. That's one of the things we need to
fix.
Consider, for example, what would happen if there is more than one LUN.
What if one of them is removed while the other remains?
Then the reap reference remains above zero and the target stays.
A more invasive change is needed.
I think you might be right in that we need to kill the list_empty check,
but I think that should be it.
That, plus a one or two other things. Look over the patch below.
Alan Stern
Index: usb-3.13/drivers/scsi/scsi_scan.c
===
--- usb-3.13.orig/drivers/scsi/scsi_scan.c
+++ usb-3.13/drivers/scsi/scsi_scan.c
@@ -334,6 +334,7 @@ static void scsi_target_dev_release(stru
struct device *parent = dev-parent;
struct scsi_target *starget = to_scsi_target(dev);
+ WARN_ON(!list_empty(starget-devices));
kfree(starget);
put_device(parent);
}
@@ -481,7 +482,7 @@ void scsi_target_reap(struct scsi_target
spin_lock_irqsave(shost-host_lock, flags);
state = starget-state;
- if (--starget-reap_ref == 0 list_empty(starget-devices)) {
+ if (--starget-reap_ref == 0) {
empty = 1;
starget-state = STARGET_DEL;
}
Index: usb-3.13/drivers/scsi/scsi_sysfs.c
===
--- usb-3.13.orig/drivers/scsi/scsi_sysfs.c
+++ usb-3.13/drivers/scsi/scsi_sysfs.c
@@ -369,17 +369,13 @@ static void scsi_device_dev_release_user
{
struct scsi_device *sdev;
struct device *parent;
- struct scsi_target *starget;
struct list_head *this, *tmp;
unsigned long flags;
sdev = container_of(work, struct scsi_device, ew.work);
-
parent = sdev-sdev_gendev.parent;
- starget = to_scsi_target(parent);
spin_lock_irqsave(sdev-host-host_lock, flags);
- starget-reap_ref++;
list_del(sdev-siblings);
list_del(sdev-same_target_siblings);
list_del(sdev-starved_entry);
@@ -399,13 +395,10 @@ static void scsi_device_dev_release_user
/* NULL queue means the device can't be used */
sdev-request_queue = NULL;
- scsi_target_reap(scsi_target(sdev));
-
kfree(sdev-inquiry);
kfree(sdev);
- if (parent)
- put_device(parent);
+ put_device(parent);
}
static void scsi_device_dev_release(struct device *dev)
@@ -1044,6 +1037,8 @@ void __scsi_remove_device(struct scsi_de
} else
put_device(sdev-sdev_dev);
+
Re: [PATCH] usb: host: xhci: Move suspend ops under PM_SLEEP to avoid warning
On Fri, Dec 13, 2013 at 05:55:20PM -0500, Santosh Shilimkar wrote: On Friday 13 December 2013 12:23 AM, David Cohen wrote: On Thu, Dec 12, 2013 at 07:25:55PM -0800, David Cohen wrote: On Thu, Dec 12, 2013 at 09:01:04PM -0500, Santosh Shilimkar wrote: On Thursday 12 December 2013 08:51 PM, David Cohen wrote: On Thu, Dec 12, 2013 at 08:06:24PM -0500, Santosh Shilimkar wrote: Otherwise you get below build warnings drivers/usb/host/xhci-plat.c:201:12: warning: ‘xhci_plat_suspend’ defined but not used [-Wunused-function] drivers/usb/host/xhci-plat.c:209:12: warning: ‘xhci_plat_resume’ defined but not used [-Wunused-function] Cc: Sarah Sharp sarah.a.sh...@linux.intel.com Cc: Greg Kroah-Hartman gre...@linuxfoundation.org Signed-off-by: Santosh Shilimkar santosh.shilim...@ti.com --- drivers/usb/host/xhci-plat.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c index d9c169f..4875be5 100644 --- a/drivers/usb/host/xhci-plat.c +++ b/drivers/usb/host/xhci-plat.c @@ -197,7 +197,7 @@ static int xhci_plat_remove(struct platform_device *dev) return 0; } -#ifdef CONFIG_PM +#ifdef CONFIG_PM_SLEEP Can't you just remove these #ifdefs altogether? xhci_plat_pm_ops is set using SET_SYSTEM_SLEEP_PM_OPS() macro which already handles '#ifdef CONFIG_PM_SLEEP' case. It does handle the difference but the hooks implemented would show-up un-used warning if you remove the #ifdefs. drivers/usb/host/xhci-plat.c:200:12: warning: ‘xhci_plat_suspend’ defined but not used [-Wunused-function] drivers/usb/host/xhci-plat.c:208:12: warning: ‘xhci_plat_resume’ defined but not used [-Wunused-function] So you need to wrap them under the PM_SLEEP check. Yeah... it's not smart enought :) But you could still remove the #else condition and the ugly DEV_PM_OPS macro. Since this patch is not urgent, I sent a RFC proposing smarter SET_*_PM_OPS(). I included your patch (a bit different) here: https://patchwork.kernel.org/patch/3337961/ Thats fine by me if you can get your RFC through. Thanks. I'll keep trying in this case. Br, David Regards, Santosh -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [usb-storage] UAS hangs khubd on USB disconnect
On Fri, 2013-12-13 at 19:48 -0500, Alan Stern wrote: On Fri, 13 Dec 2013, James Bottomley wrote: On Fri, 2013-12-13 at 16:06 -0500, Alan Stern wrote: On Fri, 13 Dec 2013, James Bottomley wrote: Actually, I think I have this figured out. There's a thinko in one of the scsi_target_reap() cases. The original (and still existing) problem with targets is that nothing creates them and nothing destroys them, so, while we could rely on the refcounting of the device model to preserve the actual target object, we had no idea when to remove it from visibility. That was the job of the reap reference, to track visibility. It looks like the reap on device last put is occurring too late. I think we should reap immediately after doing the sdev device_del, so does this fix the warn on? (I'm not sure because no-one has actually posted a backtrace, but it sounds like this is the problem). James --- diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index 8ff62c2..98d4eb3 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -399,8 +399,6 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work) /* NULL queue means the device can't be used */ sdev-request_queue = NULL; - scsi_target_reap(scsi_target(sdev)); - kfree(sdev-inquiry); kfree(sdev); @@ -1044,6 +1042,8 @@ void __scsi_remove_device(struct scsi_device *sdev) } else put_device(sdev-sdev_dev); + scsi_target_reap(scsi_target(sdev)); + /* * Stop accepting new requests and wait until all queuecommand() and * scsi_run_queue() invocations have finished before tearing down the This is not right. The problem is that you don't keep track explicitly of the number of references to a target; you rely implicitly on starget-devices being non-empty. starget-reap_ref is only a count of local operations that should block removal. No, it was supposed explicitly to be a visibility counter to answer the question when can we delete the target. It's incremented every time we add a device to the target (and when we do an operation that may remove one to keep an atomic context before we blow it away) and decremented every time we remove one. Sorry, but you're wrong. starget-reap_ref is _not_ incremented every time we add a device to the target. That's one of the things we need to fix. Well, then we would have a pretty astonishing cockup in the code. The found case of scsi_alloc_target increments the reference each time it's called, so scsi_add_device() definitely behaves like this. I suppose it's possible the list_empty() check is covering a miscount in some of the other probing routines, but that would mean we have stale targets for a lot of our use cases. I'll audit the code. James -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [usb-storage] UAS hangs khubd on USB disconnect
On Fri, 13 Dec 2013, James Bottomley wrote: Sorry, but you're wrong. starget-reap_ref is _not_ incremented every time we add a device to the target. That's one of the things we need to fix. Well, then we would have a pretty astonishing cockup in the code. The found case of scsi_alloc_target increments the reference each time it's called, so scsi_add_device() definitely behaves like this. You forgot that __scsi_add_device() calls scsi_target_reap() at the end. So the reference count is incremented and then decremented again. It's easy enough to check that the scsi_probe_and_add_lun pathway doesn't elevate the refcount. Print out the value of starget-reap_ref just after __scsi_add_device() calls scsi_alloc_target() and just before it calls scsi_target_reap(). I suppose it's possible the list_empty() check is covering a miscount in some of the other probing routines, but that would mean we have stale targets for a lot of our use cases. I'll audit the code. That's probably right; whenever a target has more than one LUN we must end up leaking the target. In the common case of one LUN it works out, because the list is empty by the time the scsi_device is released. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC] fix our current target reap infrastructure.
This patch eliminates the reap_ref and replaces it with a proper kref.
On last put of this kref, the target is removed from visibility in
sysfs. The final call to scsi_target_reap() for the device is done from
__scsi_remove_device() and only if the device was made visible. This
ensures that the target disappears as soon as the last device is gone
rather than waiting until final release of the device (which is often
too long).
---
diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index 307a811..d966e36 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -371,6 +371,31 @@ static struct scsi_target *__scsi_find_target(struct
device *parent,
}
/**
+ * scsi_target_reap_ref_release - remove target from visibility
+ * @kref: the reap_ref in the target being released
+ *
+ * Called on last put of reap_ref, which is the indication that no device
+ * under this target is visible anymore, so render the target invisible in
+ * sysfs. Note: we have to be in user context here because the target reaps
+ * should be done in places where the scsi device visibility is being removed.
+ */
+static void scsi_target_reap_ref_release(struct kref *kref)
+{
+ struct scsi_target *starget
+ = container_of(kref, struct scsi_target, reap_ref);
+
+ transport_remove_device(starget-dev);
+ device_del(starget-dev);
+ starget-state = STARGET_DEL;
+ scsi_target_destroy(starget);
+}
+
+static void scsi_target_reap_ref_put(struct scsi_target *starget)
+{
+ kref_put(starget-reap_ref, scsi_target_reap_ref_release);
+}
+
+/**
* scsi_alloc_target - allocate a new or find an existing target
* @parent:parent of the target (need not be a scsi host)
* @channel: target channel number (zero if no channels)
@@ -401,7 +426,7 @@ static struct scsi_target *scsi_alloc_target(struct device
*parent,
}
dev = starget-dev;
device_initialize(dev);
- starget-reap_ref = 1;
+ kref_init(starget-reap_ref);
dev-parent = get_device(parent);
dev_set_name(dev, target%d:%d:%d, shost-host_no, channel, id);
dev-bus = scsi_bus_type;
@@ -441,29 +466,26 @@ static struct scsi_target *scsi_alloc_target(struct
device *parent,
return starget;
found:
- found_target-reap_ref++;
+ kref_get(found_target-reap_ref);
spin_unlock_irqrestore(shost-host_lock, flags);
if (found_target-state != STARGET_DEL) {
put_device(dev);
return found_target;
}
- /* Unfortunately, we found a dying target; need to
-* wait until it's dead before we can get a new one */
+ /*
+* Unfortunately, we found a dying target; need to wait until it's
+* dead before we can get a new one. There is an anomaly here. We
+* *should* call scsi_target_reap() to balance the kref_get() of the
+* reap_ref above. However, since the target is in state STARGET_DEL,
+* it's already invisible and the reap_ref is irrelevant. If we call
+* scsi_target_reap() we might spuriously do another device_del() on
+* an already invisible target.
+*/
put_device(found_target-dev);
flush_scheduled_work();
goto retry;
}
-static void scsi_target_reap_usercontext(struct work_struct *work)
-{
- struct scsi_target *starget =
- container_of(work, struct scsi_target, ew.work);
-
- transport_remove_device(starget-dev);
- device_del(starget-dev);
- scsi_target_destroy(starget);
-}
-
/**
* scsi_target_reap - check to see if target is in use and destroy if not
* @starget: target to be checked
@@ -474,28 +496,11 @@ static void scsi_target_reap_usercontext(struct
work_struct *work)
*/
void scsi_target_reap(struct scsi_target *starget)
{
- struct Scsi_Host *shost = dev_to_shost(starget-dev.parent);
- unsigned long flags;
- enum scsi_target_state state;
- int empty = 0;
-
- spin_lock_irqsave(shost-host_lock, flags);
- state = starget-state;
- if (--starget-reap_ref == 0 list_empty(starget-devices)) {
- empty = 1;
- starget-state = STARGET_DEL;
- }
- spin_unlock_irqrestore(shost-host_lock, flags);
-
- if (!empty)
- return;
-
- BUG_ON(state == STARGET_DEL);
- if (state == STARGET_CREATED)
+ BUG_ON(starget-state == STARGET_DEL);
+ if (starget-state == STARGET_CREATED)
scsi_target_destroy(starget);
else
- execute_in_process_context(scsi_target_reap_usercontext,
- starget-ew);
+ scsi_target_reap_ref_put(starget);
}
/**
@@ -1532,6 +1537,10 @@ struct scsi_device *__scsi_add_device(struct Scsi_Host
*shost, uint channel,
}
mutex_unlock(shost-scan_mutex);
scsi_autopm_put_target(starget);
+ /*
+* paired with
Re: [RFC] fix our current target reap infrastructure.
On Fri, 13 Dec 2013, James Bottomley wrote:
This patch eliminates the reap_ref and replaces it with a proper kref.
On last put of this kref, the target is removed from visibility in
sysfs. The final call to scsi_target_reap() for the device is done from
__scsi_remove_device() and only if the device was made visible. This
ensures that the target disappears as soon as the last device is gone
rather than waiting until final release of the device (which is often
too long).
@@ -474,28 +496,11 @@ static void scsi_target_reap_usercontext(struct
work_struct *work)
*/
void scsi_target_reap(struct scsi_target *starget)
{
- struct Scsi_Host *shost = dev_to_shost(starget-dev.parent);
- unsigned long flags;
- enum scsi_target_state state;
- int empty = 0;
-
- spin_lock_irqsave(shost-host_lock, flags);
- state = starget-state;
- if (--starget-reap_ref == 0 list_empty(starget-devices)) {
- empty = 1;
- starget-state = STARGET_DEL;
- }
- spin_unlock_irqrestore(shost-host_lock, flags);
-
- if (!empty)
- return;
-
- BUG_ON(state == STARGET_DEL);
- if (state == STARGET_CREATED)
+ BUG_ON(starget-state == STARGET_DEL);
+ if (starget-state == STARGET_CREATED)
scsi_target_destroy(starget);
else
- execute_in_process_context(scsi_target_reap_usercontext,
-starget-ew);
+ scsi_target_reap_ref_put(starget);
The refcount test and state change race with scsi_alloc_target().
Maybe the race won't occur in practice, but to be safe you should hold
shost-host_lock throughout that time interval, as the original code
here does.
This means the kref approach won't work so easily. You might as well
leave reap_ref as an ordinary int.
@@ -393,7 +393,6 @@ static void scsi_device_dev_release_usercontext(struct
work_struct *work)
starget = to_scsi_target(parent);
spin_lock_irqsave(sdev-host-host_lock, flags);
- starget-reap_ref++;
list_del(sdev-siblings);
list_del(sdev-same_target_siblings);
list_del(sdev-starved_entry);
starget is now an unused local variable. It can be eliminated.
Alan Stern
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 3/3] usb: musb: fix setting JZ4740 gadget periphal mode on reset
JZ4740 USB Device Controller is not OTG compatible and does not have DEVCTL
register in silicon.
During ethernet-over-usb transactions, on reset, musb driver tries to
read from DEVCTL and consequently sets device as host (A-Device)
instead of peripheral (B-Device), which makes it a composite device to
the USB gadget driver.
This induces a kernel panic during power down where the USB gadget
driver does a null pointer dereference when trying to access the
composite device configuration.
On reset, do not rely on DEVCTL value for setting gadget peripheral
mode: hardcode it instead to B-Device.
Signed-off-by: Apelete Seketeli apel...@seketeli.net
---
drivers/usb/musb/musb_gadget.c |9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
index 32fb057..b4bea7a 100644
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -2119,6 +2119,14 @@ __acquires(musb-lock)
/* Normal reset, as B-Device;
* or else after HNP, as A-Device
*/
+#if defined(CONFIG_USB_MUSB_JZ4740) || defined(CONFIG_USB_MUSB_JZ4740_MODULE)
+ /* JZ4740 UDC Controller is not OTG compatible and does not
+* have DEVCTL register in silicon: do not rely on devctl for
+* setting peripheral mode.
+*/
+ musb-xceiv-state = OTG_STATE_B_PERIPHERAL;
+ musb-g.is_a_peripheral = 0;
+#else
if (devctl MUSB_DEVCTL_BDEVICE) {
musb-xceiv-state = OTG_STATE_B_PERIPHERAL;
musb-g.is_a_peripheral = 0;
@@ -2126,6 +2134,7 @@ __acquires(musb-lock)
musb-xceiv-state = OTG_STATE_A_PERIPHERAL;
musb-g.is_a_peripheral = 1;
}
+#endif
/* start with default limits on VBUS power draw */
(void) musb_gadget_vbus_draw(musb-g, 8);
--
1.7.10.4
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/3] Add USB support for Ingenic JZ4740
Hello, Following the fix I submitted a few weeks ago, here is a set of patches that add USB support for the Ingenic JZ4740 MIPS SoC. The JZ4740 is found in the Ben NanoNote handheld computer which is built by the Qi-Hardware community. Even though Ben NanoNote is already supported in the kernel, we were relying on an out-of-tree gadget driver to make use of the JZ4740 USB Device Controller. The patches that come as a follow-up of this message are an attempt to provide USB support through an musb glue layer. Changes were rebased on top of Felipe Balbi's USB Subsystem master branch, built and tested on device successfully. The following changes since commit 9bdff34517bc49d8e98558659e077ea9f9df3d60: Merge branch 'next' are available in the git repository at: git://seketeli.fr/~apelete/linux-usb.git musb-jz4740 Apelete Seketeli (3): mips: qi_lb60: add defconfig for Ben NanoNote usb: musb: add support for JZ4740 usb device controller usb: musb: fix setting JZ4740 gadget periphal mode on reset arch/mips/configs/qi_lb60_defconfig | 188 + arch/mips/include/asm/mach-jz4740/platform.h |1 + arch/mips/jz4740/board-qi_lb60.c |1 + arch/mips/jz4740/platform.c | 55 +-- drivers/usb/musb/Kconfig |8 +- drivers/usb/musb/Makefile|1 + drivers/usb/musb/jz4740.c| 229 ++ drivers/usb/musb/musb_core.c | 14 ++ drivers/usb/musb/musb_gadget.c |9 + 9 files changed, 489 insertions(+), 17 deletions(-) create mode 100644 arch/mips/configs/qi_lb60_defconfig create mode 100644 drivers/usb/musb/jz4740.c -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/3] mips: qi_lb60: add defconfig for Ben NanoNote
Add defconfig for the Ben NanoNote handheld computer which is built around QI_LB60 board and Ingenic JZ4740 MIPS SoC. Signed-off-by: Apelete Seketeli apel...@seketeli.net --- arch/mips/configs/qi_lb60_defconfig | 188 +++ 1 file changed, 188 insertions(+) create mode 100644 arch/mips/configs/qi_lb60_defconfig diff --git a/arch/mips/configs/qi_lb60_defconfig b/arch/mips/configs/qi_lb60_defconfig new file mode 100644 index 000..2b96547 --- /dev/null +++ b/arch/mips/configs/qi_lb60_defconfig @@ -0,0 +1,188 @@ +CONFIG_MACH_JZ4740=y +# CONFIG_COMPACTION is not set +# CONFIG_CROSS_MEMORY_ATTACH is not set +CONFIG_HZ_100=y +CONFIG_PREEMPT=y +# CONFIG_SECCOMP is not set +# CONFIG_LOCALVERSION_AUTO is not set +CONFIG_SYSVIPC=y +CONFIG_LOG_BUF_SHIFT=14 +CONFIG_SYSCTL_SYSCALL=y +CONFIG_KALLSYMS_ALL=y +CONFIG_EMBEDDED=y +# CONFIG_VM_EVENT_COUNTERS is not set +# CONFIG_COMPAT_BRK is not set +CONFIG_SLAB=y +CONFIG_MODULES=y +CONFIG_MODULE_UNLOAD=y +# CONFIG_BLK_DEV_BSG is not set +CONFIG_PARTITION_ADVANCED=y +# CONFIG_EFI_PARTITION is not set +# CONFIG_IOSCHED_CFQ is not set +# CONFIG_CORE_DUMP_DEFAULT_ELF_HEADERS is not set +CONFIG_NET=y +CONFIG_PACKET=y +CONFIG_UNIX=y +CONFIG_INET=y +CONFIG_IP_MULTICAST=y +CONFIG_IP_ADVANCED_ROUTER=y +CONFIG_IP_MULTIPLE_TABLES=y +CONFIG_IP_ROUTE_MULTIPATH=y +CONFIG_IP_ROUTE_VERBOSE=y +CONFIG_IP_MROUTE=y +CONFIG_IP_MROUTE_MULTIPLE_TABLES=y +# CONFIG_INET_XFRM_MODE_TRANSPORT is not set +# CONFIG_INET_XFRM_MODE_TUNNEL is not set +# CONFIG_INET_XFRM_MODE_BEET is not set +# CONFIG_INET_LRO is not set +# CONFIG_INET_DIAG is not set +CONFIG_TCP_CONG_ADVANCED=y +# CONFIG_TCP_CONG_BIC is not set +# CONFIG_TCP_CONG_CUBIC is not set +CONFIG_TCP_CONG_WESTWOOD=y +# CONFIG_TCP_CONG_HTCP is not set +# CONFIG_IPV6 is not set +CONFIG_UEVENT_HELPER_PATH=/sbin/hotplug +# CONFIG_FIRMWARE_IN_KERNEL is not set +CONFIG_MTD=y +CONFIG_MTD_BLOCK=y +CONFIG_MTD_NAND=y +CONFIG_MTD_NAND_JZ4740=y +CONFIG_MTD_UBI=y +CONFIG_NETDEVICES=y +# CONFIG_WLAN is not set +# CONFIG_INPUT_MOUSEDEV is not set +CONFIG_INPUT_EVDEV=y +# CONFIG_KEYBOARD_ATKBD is not set +CONFIG_KEYBOARD_GPIO=y +CONFIG_KEYBOARD_MATRIX=y +# CONFIG_INPUT_MOUSE is not set +CONFIG_INPUT_MISC=y +# CONFIG_SERIO is not set +CONFIG_LEGACY_PTY_COUNT=2 +# CONFIG_DEVKMEM is not set +CONFIG_SERIAL_8250=y +CONFIG_SERIAL_8250_CONSOLE=y +# CONFIG_SERIAL_8250_DMA is not set +CONFIG_SERIAL_8250_NR_UARTS=2 +CONFIG_SERIAL_8250_RUNTIME_UARTS=2 +# CONFIG_HW_RANDOM is not set +CONFIG_SPI=y +CONFIG_SPI_GPIO=y +CONFIG_POWER_SUPPLY=y +CONFIG_BATTERY_JZ4740=y +CONFIG_CHARGER_GPIO=y +# CONFIG_HWMON is not set +CONFIG_MFD_JZ4740_ADC=y +CONFIG_REGULATOR=y +CONFIG_REGULATOR_FIXED_VOLTAGE=y +CONFIG_FB=y +CONFIG_FB_JZ4740=y +CONFIG_BACKLIGHT_LCD_SUPPORT=y +CONFIG_LCD_CLASS_DEVICE=y +# CONFIG_BACKLIGHT_CLASS_DEVICE is not set +# CONFIG_VGA_CONSOLE is not set +CONFIG_FRAMEBUFFER_CONSOLE=y +CONFIG_LOGO=y +# CONFIG_LOGO_LINUX_MONO is not set +# CONFIG_LOGO_LINUX_VGA16 is not set +# CONFIG_LOGO_LINUX_CLUT224 is not set +CONFIG_SOUND=y +CONFIG_SND=y +# CONFIG_SND_SUPPORT_OLD_API is not set +# CONFIG_SND_VERBOSE_PROCFS is not set +# CONFIG_SND_DRIVERS is not set +# CONFIG_SND_SPI is not set +# CONFIG_SND_MIPS is not set +CONFIG_SND_SOC=y +CONFIG_SND_JZ4740_SOC=y +CONFIG_SND_JZ4740_SOC_QI_LB60=y +CONFIG_USB=y +CONFIG_USB_OTG_BLACKLIST_HUB=y +CONFIG_USB_MUSB_HDRC=y +CONFIG_USB_MUSB_GADGET=y +CONFIG_USB_MUSB_JZ4740=y +CONFIG_NOP_USB_XCEIV=y +CONFIG_USB_GADGET=y +CONFIG_USB_GADGET_DEBUG=y +CONFIG_USB_ETH=y +# CONFIG_USB_ETH_RNDIS is not set +CONFIG_MMC=y +CONFIG_MMC_UNSAFE_RESUME=y +# CONFIG_MMC_BLOCK_BOUNCE is not set +CONFIG_MMC_JZ4740=y +CONFIG_RTC_CLASS=y +CONFIG_RTC_DRV_JZ4740=y +CONFIG_DMADEVICES=y +CONFIG_DMA_JZ4740=y +CONFIG_PWM=y +CONFIG_PWM_JZ4740=y +CONFIG_EXT2_FS=y +CONFIG_EXT3_FS=y +# CONFIG_EXT3_DEFAULTS_TO_ORDERED is not set +# CONFIG_EXT3_FS_XATTR is not set +# CONFIG_DNOTIFY is not set +CONFIG_VFAT_FS=y +CONFIG_PROC_KCORE=y +# CONFIG_PROC_PAGE_MONITOR is not set +CONFIG_TMPFS=y +CONFIG_JFFS2_FS=y +CONFIG_JFFS2_SUMMARY=y +CONFIG_JFFS2_COMPRESSION_OPTIONS=y +# CONFIG_JFFS2_ZLIB is not set +CONFIG_UBIFS_FS=y +CONFIG_UBIFS_FS_ADVANCED_COMPR=y +# CONFIG_NETWORK_FILESYSTEMS is not set +CONFIG_NLS_CODEPAGE_437=y +CONFIG_NLS_CODEPAGE_737=y +CONFIG_NLS_CODEPAGE_775=y +CONFIG_NLS_CODEPAGE_850=y +CONFIG_NLS_CODEPAGE_852=y +CONFIG_NLS_CODEPAGE_855=y +CONFIG_NLS_CODEPAGE_857=y +CONFIG_NLS_CODEPAGE_860=y +CONFIG_NLS_CODEPAGE_861=y +CONFIG_NLS_CODEPAGE_862=y +CONFIG_NLS_CODEPAGE_863=y +CONFIG_NLS_CODEPAGE_864=y +CONFIG_NLS_CODEPAGE_865=y +CONFIG_NLS_CODEPAGE_866=y +CONFIG_NLS_CODEPAGE_869=y +CONFIG_NLS_CODEPAGE_936=y +CONFIG_NLS_CODEPAGE_950=y +CONFIG_NLS_CODEPAGE_932=y +CONFIG_NLS_CODEPAGE_949=y +CONFIG_NLS_CODEPAGE_874=y +CONFIG_NLS_ISO8859_8=y +CONFIG_NLS_CODEPAGE_1250=y +CONFIG_NLS_CODEPAGE_1251=y +CONFIG_NLS_ASCII=y +CONFIG_NLS_ISO8859_1=y +CONFIG_NLS_ISO8859_2=y +CONFIG_NLS_ISO8859_3=y +CONFIG_NLS_ISO8859_4=y +CONFIG_NLS_ISO8859_5=y
[PATCH 2/3] usb: musb: add support for JZ4740 usb device controller
Add support for Ingenic JZ4740 USB Device Controller through a
specific musb glue layer.
The platform data already available in tree for that USB Device
Controller was previously used by an out-of-tree USB gadget driver
which was not relying on the musb driver and was written by Ingenic
and the Qi-Hardware community.
JZ4740 UDC not being OTG compatible and missing some hardware
registers, this musb glue layer is written from scratch to be used in
gadget mode only and take silicon design specifics into account.
Signed-off-by: Apelete Seketeli apel...@seketeli.net
---
arch/mips/include/asm/mach-jz4740/platform.h |1 +
arch/mips/jz4740/board-qi_lb60.c |1 +
arch/mips/jz4740/platform.c | 55 +--
drivers/usb/musb/Kconfig |8 +-
drivers/usb/musb/Makefile|1 +
drivers/usb/musb/jz4740.c| 229 ++
drivers/usb/musb/musb_core.c | 14 ++
7 files changed, 292 insertions(+), 17 deletions(-)
create mode 100644 drivers/usb/musb/jz4740.c
diff --git a/arch/mips/include/asm/mach-jz4740/platform.h
b/arch/mips/include/asm/mach-jz4740/platform.h
index 05988c2..069b43a 100644
--- a/arch/mips/include/asm/mach-jz4740/platform.h
+++ b/arch/mips/include/asm/mach-jz4740/platform.h
@@ -21,6 +21,7 @@
extern struct platform_device jz4740_usb_ohci_device;
extern struct platform_device jz4740_udc_device;
+extern struct platform_device jz4740_udc_xceiv_device;
extern struct platform_device jz4740_mmc_device;
extern struct platform_device jz4740_rtc_device;
extern struct platform_device jz4740_i2c_device;
diff --git a/arch/mips/jz4740/board-qi_lb60.c b/arch/mips/jz4740/board-qi_lb60.c
index 8a5ec0e..c01900e 100644
--- a/arch/mips/jz4740/board-qi_lb60.c
+++ b/arch/mips/jz4740/board-qi_lb60.c
@@ -427,6 +427,7 @@ static struct platform_device qi_lb60_audio_device = {
static struct platform_device *jz_platform_devices[] __initdata = {
jz4740_udc_device,
+ jz4740_udc_xceiv_device,
jz4740_mmc_device,
jz4740_nand_device,
qi_lb60_keypad,
diff --git a/arch/mips/jz4740/platform.c b/arch/mips/jz4740/platform.c
index df65677..c55ff0a 100644
--- a/arch/mips/jz4740/platform.c
+++ b/arch/mips/jz4740/platform.c
@@ -21,6 +21,8 @@
#include linux/dma-mapping.h
+#include linux/usb/musb.h
+
#include asm/mach-jz4740/platform.h
#include asm/mach-jz4740/base.h
#include asm/mach-jz4740/irq.h
@@ -56,29 +58,50 @@ struct platform_device jz4740_usb_ohci_device = {
.resource = jz4740_usb_ohci_resources,
};
-/* UDC (USB gadget controller) */
-static struct resource jz4740_usb_gdt_resources[] = {
- {
- .start = JZ4740_UDC_BASE_ADDR,
- .end= JZ4740_UDC_BASE_ADDR + 0x1000 - 1,
- .flags = IORESOURCE_MEM,
+/* USB Device Controller */
+struct platform_device jz4740_udc_xceiv_device = {
+ .name = usb_phy_gen_xceiv,
+ .id = 0,
+};
+
+static struct musb_hdrc_config jz4740_udc_config = {
+ /* Silicon does not implement USB OTG. */
+ .multipoint = 0,
+ /* Max EPs scanned, driver will decide which EP can be used. */
+ .num_eps= 4,
+ /* RAMbits needed to configure EPs from table */
+ .ram_bits = 9,
+};
+
+static struct musb_hdrc_platform_data jz4740_udc_platform_data = {
+ .mode = MUSB_PERIPHERAL,
+ .config = jz4740_udc_config,
+};
+
+static struct resource jz4740_udc_resources[] = {
+ [0] = {
+ .start = JZ4740_UDC_BASE_ADDR,
+ .end = JZ4740_UDC_BASE_ADDR + 0x1 - 1,
+ .flags = IORESOURCE_MEM,
},
- {
- .start = JZ4740_IRQ_UDC,
- .end= JZ4740_IRQ_UDC,
- .flags = IORESOURCE_IRQ,
+ [1] = {
+ .start = JZ4740_IRQ_UDC,
+ .end = JZ4740_IRQ_UDC,
+ .flags = IORESOURCE_IRQ,
+ .name = mc,
},
};
struct platform_device jz4740_udc_device = {
- .name = jz-udc,
- .id = -1,
- .dev = {
- .dma_mask = jz4740_udc_device.dev.coherent_dma_mask,
+ .name = musb-jz4740,
+ .id = -1,
+ .dev = {
+ .dma_mask = jz4740_udc_device.dev.coherent_dma_mask,
.coherent_dma_mask = DMA_BIT_MASK(32),
+ .platform_data = jz4740_udc_platform_data,
},
- .num_resources = ARRAY_SIZE(jz4740_usb_gdt_resources),
- .resource = jz4740_usb_gdt_resources,
+ .num_resources = ARRAY_SIZE(jz4740_udc_resources),
+ .resource = jz4740_udc_resources,
};
/* MMC/SD controller */
diff --git a/drivers/usb/musb/Kconfig b/drivers/usb/musb/Kconfig
index 57dfc0c..a45ed15 100644
--- a/drivers/usb/musb/Kconfig
+++ b/drivers/usb/musb/Kconfig
@@ -93,6 +93,12 @@ config USB_MUSB_BLACKFIN
config USB_MUSB_UX500
tristate Ux500 platforms
