Re: [PATCH] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
Hello!
On 1/13/2017 12:30 AM, Tony Lindgren wrote:
Commit fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
together with recent MUSB changes allowed USB and DMA on BeagleBone to idle
when no cable is connected. But looks like few corner case issues still
remain.
Looks like just by repluging USB cable about ten or so times on BeagleBone
Re-plugging.
when configured in USB peripheral mode we can get warnings and eventually
trigger an oops in cppi41 DMA:
WARNING: CPU: 0 PID: 14 at drivers/dma/cppi41.c:1154 cppi41_runtime_suspend+
x28/0x38 [cppi41]
...
WARNING: CPU: 0 PID: 14 at drivers/dma/cppi41.c:452
push_desc_queue+0x94/0x9c [cppi41]
...
Unable to handle kernel NULL pointer dereference at virtual
address 0104
pgd = c0004000
[0104] *pgd=
Internal error: Oops: 805 [#1] SMP ARM
...
[] (cppi41_runtime_resume [cppi41]) from []
(__rpm_callback+0xc0/0x214)
[] (__rpm_callback) from [] (rpm_callback+0x20/0x80)
[] (rpm_callback) from [] (rpm_resume+0x504/0x78c)
[] (rpm_resume) from [] (pm_runtime_work+0x60/0xa8)
[] (pm_runtime_work) from [] (process_one_work+0x2b4/0x808)
This is because of a race with PM runtime and cppi41_dma_issue_pending()
as reported by Alexandre Bailon in earlier
set of patches. Based on mailing list discussions we however came to the
conclusion that a different fix from Alexandre's fix is needed in order
to guarantee that DMA is really active when we try to use it.
To fix the issue, we need to add a driver specific flag as we otherwise
can have -EINPROGRESS state set by PM runtime and can't rely on
Runtime PM.
pm_runtime_active() to tell us when we can use the DMA.
For reference, this is also documented in Documentation/power/runtime_pm.txt
in the example at the end of the file as pointed out by Grygorii Strashko
.
So let's fix the issue by introducing atomic_t active that gets toggled
in runtime_suspend() and runtime_resume(). And then let's replace the
pm_runtime_active() checks with atomic_read().
Note that we may also get transactions queued while in -EINPROGRESS
state. So we need to check for new elements in cppi41_runtime_resume()
by replacing list_for_each_entry_safe() with the commonly used test for
while (!list_empty(&cdd->pending)).
Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
Cc: Andy Shevchenko
Cc: Bin Liu
Cc: Grygorii Strashko
Cc: Kevin Hilman
Cc: Patrick Titiano
Cc: Sergei Shtylyov
Reported-by: Alexandre Bailon
Signed-off-by: Tony Lindgren
---
drivers/dma/cppi41.c | 23 ++-
1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
[...]
@@ -320,7 +323,8 @@ static irqreturn_t cppi41_irq(int irq, void *data)
int error;
error = pm_runtime_get(cdd->ddev.dev);
- if (error < 0)
+ if (((error != -EINPROGRESS) && error < 0) ||
Hum, the innermost parens are inconsistent: around != but not around <.
[...]
MBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB ExpressCard makes kworker process utilise 72-75% CPU infinitely
On 12.01.2017 15:33, Christoph Anton Mitterer wrote: Hi. This is basically from: https://bugzilla.kernel.org/show_bug.cgi?id=108341 respectively https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=805903 the later in which Ben Hutchings recommended me to ping this list as well. I bought a USB3.0 ExpressCard from StarTech[0] which is apparently[1] based on the NEC uPD720200. Using a kernel 4.2.6 (see further below for the same on a more recent kernel version) on amd64, the System is Debian sid, the following happens when I plug the card: kernel logs show: Nov 21 17:15:22 heisenberg kernel: [ 102.387452] pci :01:00.0: [1033:0194] type 00 class 0x0c0330 Nov 21 17:15:22 heisenberg kernel: [ 102.387545] pci :01:00.0: reg 0x10: [mem0x-0x1fff 64bit] Nov 21 17:15:22 heisenberg kernel: [ 102.387723] pci :01:00.0: PME# supported from D0 D3hot Nov 21 17:15:22 heisenberg kernel: [ 102.394689] pci :01:00.0: BAR 0: assigned [mem0xf0d0-0xf0d01fff 64bit] Nov 21 17:15:22 heisenberg kernel: [ 102.394750] pci :01:00.0: enabling device ( -> 0002) Nov 21 17:15:22 heisenberg kernel: [ 102.395178] xhci_hcd :01:00.0: xHCI Host Controller Nov 21 17:15:22 heisenberg kernel: [ 102.395192] xhci_hcd :01:00.0: new USB bus registered,assigned bus number 5 Nov 21 17:15:22 heisenberg kernel: [ 102.395418] xhci_hcd :01:00.0: hcc params 0x014042cb hciversion 0x96 quirks 0x0004 Nov 21 17:15:22 heisenberg kernel: [ 102.395892] usb usb5: New USB device found, idVendor=1d6b, idProduct=0002 Nov 21 17:15:22 heisenberg kernel: [ 102.395896] usb usb5: New USB device strings: Mfr=3, Product=2, SerialNumber=1 Nov 21 17:15:22 heisenberg kernel: [ 102.395899] usb usb5: Product: xHCI Host Controller Nov 21 17:15:22 heisenberg kernel: [ 102.395902] usb usb5: Manufacturer: Linux 4.2.0-1-amd64 xhci-hcd Nov 21 17:15:22 heisenberg kernel: [ 102.395905] usb usb5: SerialNumber: :01:00.0 Nov 21 17:15:22 heisenberg kernel: [ 102.396308] hub 5-0:1.0: USB hub found Nov 21 17:15:22 heisenberg kernel: [ 102.396331] hub 5-0:1.0: 2 ports detected Nov 21 17:15:22 heisenberg kernel: [ 102.396591] xhci_hcd :01:00.0: xHCI Host Controller Nov 21 17:15:22 heisenberg kernel: [ 102.396599] xhci_hcd :01:00.0: new USB bus registered,assigned bus number 6 Nov 21 17:15:22 heisenberg kernel: [ 102.398835] usb usb6: We don't know the algorithms for LPM forthis host, disabling LPM. Nov 21 17:15:22 heisenberg kernel: [ 102.398883] usb usb6: New USB device found, idVendor=1d6b, idProduct=0003 Nov 21 17:15:22 heisenberg kernel: [ 102.398887] usb usb6: New USB device strings: Mfr=3, Product=2, SerialNumber=1 Nov 21 17:15:22 heisenberg kernel: [ 102.398890] usb usb6: Product: xHCI Host Controller Nov 21 17:15:22 heisenberg kernel: [ 102.398892] usb usb6: Manufacturer: Linux 4.2.0-1-amd64 xhci-hcd Nov 21 17:15:22 heisenberg kernel: [ 102.398895] usb usb6: SerialNumber: :01:00.0 Nov 21 17:15:22 heisenberg kernel: [ 102.399272] hub 6-0:1.0: USB hub found Nov 21 17:15:22 heisenberg kernel: [ 102.399294] hub 6-0:1.0: 2 ports detected and when removing: Nov 21 17:15:40 heisenberg kernel: [ 120.541515] xhci_hcd :01:00.0: remove, state 4 Nov 21 17:15:40 heisenberg kernel: [ 120.544671] usb usb6: USB disconnect, device number 1 Nov 21 17:15:40 heisenberg kernel: [ 120.546683] xhci_hcd :01:00.0: Host not halted after 16000 microseconds. Nov 21 17:15:40 heisenberg kernel: [ 120.547611] xhci_hcd :01:00.0: USB bus 6 deregistered Nov 21 17:15:40 heisenberg kernel: [ 120.547618] xhci_hcd :01:00.0: remove, state 4 Nov 21 17:15:40 heisenberg kernel: [ 120.547622] usb usb5: USB disconnect, device number 1 Nov 21 17:15:40 heisenberg kernel: [ 120.547735] xhci_hcd :01:00.0: USB bus 5 deregistered Now the problem is that immediately when I attach the card, a kworker process starts to utilise the CPU to always around 72%. And it never stops again until I shutdown; removing the card doesn't help. Does this kworker start to utilize the CPU immediately after a fresh boot when the card is inserted, or only after the first card removal? If after first removal then it could be related to this: https://www.spinics.net/lists/stable/msg155829.html -Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: host: xhci: plat: check hcc_params after add hcd
Hi,
On 13/01/17 05:18, William Wu wrote:
> From: William wu
>
> The commit 4ac53087d6d4 ("usb: xhci: plat: Create both
> HCDs before adding them") move add hcd to the end of
> probe, this cause hcc_params uninitiated, because xHCI
> driver sets hcc_params in xhci_gen_setup() called from
> usb_add_hcd().
>
> This patch checks the Maximum Primary Stream Array Size
> in the hcc_params register after add hcd.
>
> Signed-off-by: William wu
> ---
> drivers/usb/host/xhci-plat.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
> index ddfab30..52ce697 100644
> --- a/drivers/usb/host/xhci-plat.c
> +++ b/drivers/usb/host/xhci-plat.c
> @@ -232,9 +232,6 @@ static int xhci_plat_probe(struct platform_device *pdev)
> if (device_property_read_bool(&pdev->dev, "usb3-lpm-capable"))
> xhci->quirks |= XHCI_LPM_SUPPORT;
>
> - if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
> - xhci->shared_hcd->can_do_streams = 1;
> -
> hcd->usb_phy = devm_usb_get_phy_by_phandle(&pdev->dev, "usb-phy", 0);
> if (IS_ERR(hcd->usb_phy)) {
> ret = PTR_ERR(hcd->usb_phy);
> @@ -255,6 +252,9 @@ static int xhci_plat_probe(struct platform_device *pdev)
> if (ret)
> goto dealloc_usb2_hcd;
>
> + if (HCC_MAX_PSA(xhci->hcc_params) >= 4)
> + xhci->shared_hcd->can_do_streams = 1;
> +
xhci->hcc_params is initialized after the first usb_add_hcd().
Should this bit come before the usb_add_hcd(xhci->shared_hcd,..)?
You will also need to copy to v4.2+ . Thanks.
> return 0;
>
>
>
cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe 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 v14 09/10] usbip: exporting devices: chage to documenattion
On 01/13/2017 01:17 AM, Shuah Khan wrote: > On 12/26/2016 12:08 AM, Nobuo Iwata wrote: >> This patch adds function and usage of new connect operation, disconnect >> operation and application(vhci)-side daemon to README and manuals. > > This should be the first patch for the series. That would have saved > me lot of time. Please move this patch up. > >> >> At this point, the wording, 'server' and 'client' are ambiguous in >> several place. >> >> For existing attach command, the daemon runs device side machine and >> attach command is executed in application side machine. Then 'server' >> is used for device side and 'client' is for application side. >> >> For the new connect command, the daemon runs applications side machine >> and connect command is executed in device side machine. Now, 'server' >> and 'client' run in different machine than before. >> >> To avoid confusion, to represent things to be done in device side node >> by both command and daemon, words 'device-side' is used instead of >> 'server'. To represent things to be done is application side node by >> both command and daemon, 'applicationr-side' are used instead of >> 'client'. >> >> EXISTING) - invites devices from application(vhci)-side >> +--+ +--+ >> device--+ STUB | | application/VHCI | >> +--+ +--+ >> (server) (client) >> 1) # usbipd ... start daemon >> = = = >> 2) # usbip list --local >> 3) # usbip bind >> <--- list bound devices --- 4) # usbip list --remote >> <--- import a device -- 5) # usbip attach >> = = = >> X disconnected6) # usbip detach >> 7) usbip unbind >> >> NEW) - dedicates devices from device(stub)-side >> +--+ +--+ >> device--+ STUB | | application/VHCI | >> +--+ +--+ >> (client) (server) >> 1) # usbipa ... start daemon > > Make the left side server and right side client. I think you might be > using server and client network terminology. I would like to see server > as the system that has the device physically attached to. > > > I still want to see server as the one that is connected to the device. > It is just that in this new proposed model, server is exporting devices. > > Also does usbip run here in user mode. Can any user run uspip and initiate > export? If so, I don't think I can take this patch series. I don't like to > see non root being able to export and/or make attached devices public. > usbip connect (export operation) needs the same permissions as usbip bind because it uses the same kernel infrastructure. To export a device you need to bind a stub driver to it which you do using bind attribute in drivers directory which by default has permissions: --w--- 1 root root 4096 Dec 31 17:12 bind usbipa also has to run as a root to accept device which someone would like to system because it writes to attach attribute in vhci driver dir which is declared as follows: static DEVICE_ATTR(attach, S_IWUSR, NULL, store_attach); So summing up to export or to accept export on usbipa side both sides requires root permissions. Best regards, -- Krzysztof Opasiak Samsung R&D Institute Poland Samsung Electronics -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] HID: corsair: fix DMA to stack and info leaks
On Thu, 12 Jan 2017, Johan Hovold wrote: > These patches fix DMA buffers on stack and information leaks in the > corsair HID driver. > > Note that this series has only been compile tested. Adding Clément to CC, and applying to for-4.10/upstream-fixes branch. Thanks! -- Jiri Kosina SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v14 03/10] usbip: exporting devices: new connect operation
On 01/13/2017 01:06 AM, Shuah Khan wrote: > On 12/26/2016 12:08 AM, Nobuo Iwata wrote: >> Implementation of new connect operation. This is linked as a part of >> usbip command. With this patch, usbip command has following operations. >> >> bind >> unbind >> list (local/remote) >> attach >> detach >> port >> connect ... this patch > > Don't include existing commands. Just list the new one. Why do you call > this connect. Isn't it really export - connect isn't accurate. Call it > export/unexport. > >> >> In device side node, this binds a device internally, sends an export >> request and receives export response. The definition of the request and >> response are defined in original code: tools/usb/usbip/usbip_network.h >> but was not used. They are corresponding to NEW-3 in following diagram. >> >> EXISTING) - invites devices from application(vhci)-side >> +--+ +--+ >> device--+ STUB | | application/VHCI | >> +--+ +--+ >> (server) (client) >> 1) # usbipd ... start daemon >> = = = >> 2) # usbip list --local >> 3) # usbip bind >> <--- list bound devices --- 4) # usbip list --remote >> <--- import a device -- 5) # usbip attach >> = = = >> X disconnected6) # usbip detach >> 7) usbip unbind >> >> NEW) - dedicates devices from device(stub)-side >> +--+ +--+ >> device--+ STUB | | application/VHCI | >> +--+ +--+ >> (client) (server) > > Make the left side server and right side client. I think you might be > using server and client network terminology. I would like to see server > as the system that has the device physically attached to. > > > I still want to see server as the one that is connected to the device. > It is just that in this new proposed model, server is exporting devices. > > Also does usbip run here in user mode. Can any user run uspip and initiate > export? If so, I don't think I can take this patch series. I don't like to > see non root being able to export and/or make attached devices public. > By default it's not possible. See my response to patch 9. Best regards, -- Krzysztof Opasiak Samsung R&D Institute Poland Samsung Electronics -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
linux 4.9 breaks pctv452e
this OOPS always happens with 4.9. up to 4.9.3 all 4.8.x versions work fine. Disabling VMAP_STACK does not fix it. [9.043616] dvb-usb: found a 'Technotrend TT Connect S2-3600' in warm state. [9.043619] pctv452e: pctv452e_power_ctrl: 1 [9.051329] BUG: unable to handle kernel NULL pointer dereference at (null) [9.051342] IP: [] __mutex_lock_slowpath+0x76/0x130 [9.051350] PGD 0 [9.051355] Oops: 0002 [#1] PREEMPT SMP [9.051358] Modules linked in: dvb_usb_pctv452e(+) ttpci_eeprom dvb_usb ftdi_sio dvb_core usbserial rc_core snd_hda_codec_hdmi(+) x86_pkg_temp_thermal intel_powercla mp coretemp kvm_intel kvm irqbypass snd_hda_intel crct10dif_pclmul snd_hda_codec crc32_pclmul ghash_clmulni_intel snd_hda_core snd_hwdep aesni_intel aes_x86_64 snd_pcm lrw gf128mul glue_helper ablk_helper cryptd serio_raw snd_seq_midi snd_seq_midi_event snd_rawmidi joydev snd_seq snd_seq_device snd_timer snd soundcore sg mei_me mei sh pchp lpc_ich tpm_tis tpm_tis_core nfsd auth_rpcgss nfs_acl lockd grace sunrpc ip_tables x_tables autofs4 usb_storage hid_generic hid_logitech_hidpp hid_logitech_dj usbh id hid sd_mod nouveau e1000e psmouse ahci wmi ptp libahci ttm pps_core video [9.051451] CPU: 4 PID: 478 Comm: systemd-udevd Not tainted 4.9.0 #1 [9.051455] Hardware name: /DH87RL, BIOS RLH8710H.86A.0323.2013.1204.1726 12/04/2013 [9.051459] task: 88021156d880 task.stack: c9000171c000 [9.051462] RIP: 0010:[] [] __mutex_lock_slowpath+0x76/0x130 [9.051469] RSP: 0018:c9000171fa20 EFLAGS: 00010282 [9.051472] RAX: RBX: 8802003a7758 RCX: 0001 [9.051475] RDX: 0001 RSI: 81a771a2 RDI: 0001 [9.051479] RBP: c9000171fa68 R08: 00021b803ec5 R09: 0002 [9.051482] R10: 1329 R11: 880214cf1e00 R12: 8802003a775c [9.051486] R13: 88021156d880 R14: R15: 8802003a7760 [9.051490] FS: 7f6cf14748c0() GS:88021ed0() knlGS: [9.051494] CS: 0010 DS: ES: CR0: 80050033 [9.051497] CR2: CR3: 000214bc4000 CR4: 001406e0 [9.051500] Stack: [9.051503] 8802003a7760 88020018 c9000171fa90 [9.051509] 8802003a7758 88021260e900 88021273c000 8802003a7758 [9.051516] a0634f80 c9000171fa80 8179c507 8802003a7700 [9.051522] Call Trace: [9.051527] [] mutex_lock+0x17/0x30 [9.051531] [] pctv452e_power_ctrl+0x85/0x190 [dvb_usb_pctv452e] [9.051536] [] dvb_usb_device_power_ctrl+0x3f/0x50 [dvb_usb] [9.051541] [] dvb_usb_device_init+0x225/0x620 [dvb_usb] [9.051546] [] pctv452e_usb_probe+0x51/0x60 [dvb_usb_pctv452e] [9.051550] [] usb_probe_interface+0x159/0x2d0 [9.051555] [] driver_probe_device+0x223/0x430 [9.051559] [] __driver_attach+0xdf/0xf0 [9.051563] [] ? driver_probe_device+0x430/0x430 [9.051567] [] bus_for_each_dev+0x60/0xa0 [9.051571] [] driver_attach+0x1e/0x20 [9.051575] [] bus_add_driver+0x170/0x270 [9.051579] [] driver_register+0x60/0xe0 [9.051583] [] usb_register_driver+0x81/0x140 [9.051587] [] ? 0xa03aa000 [9.051591] [] pctv452e_usb_driver_init+0x1e/0x1000 [dvb_usb_pctv452e] [9.051596] [] do_one_initcall+0x3d/0x150 [9.051601] [] do_init_module+0x5f/0x1ef [9.051606] [] load_module+0x2384/0x2a40 [9.051610] [] ? ref_module+0x190/0x190 [9.051614] [] ? kernel_read_file+0x1a3/0x1c0 [9.051620] [] SYSC_finit_module+0xbc/0xf0 [9.051624] [] SyS_finit_module+0xe/0x10 [9.051628] [] entry_SYSCALL_64_fastpath+0x1e/0xad [9.051631] Code: e8 90 32 00 00 8b 03 83 f8 01 0f 84 b2 00 00 00 48 8b 43 10 4c 8d 7b 08 48 89 63 10 41 be ff ff ff ff 4c 89 3c 24 48 89 44 24 08 <48> 89 20 4c 89 6c 24 10 eb 1d 4c 89 e7 49 c7 45 08 02 00 00 00 [9.051693] RIP [] __mutex_lock_slowpath+0x76/0x130 [9.051698] RSP [9.051700] CR2: [9.056544] ---[ end trace d7fca26bbd239b35 ]--- -- Wolfgang -- To unsubscribe from this list: send the line "unsubscribe 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: linux 4.9 breaks pctv452e
On Fri, Jan 13, 2017 at 12:15:05PM +0100, Wolfgang Rohdewald wrote: > this OOPS always happens with 4.9. up to 4.9.3 > > all 4.8.x versions work fine. Ick. Any chance you can use 'git bisect' to determine which commit broke this for you? 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 v14 02/10] usbip: exporting devices: modifications to host side libraries
On 01/12/2017 10:14 PM, Shuah Khan wrote:
> On 12/26/2016 12:08 AM, Nobuo Iwata wrote:
>> Modifications to host driver wrapper and its related operations (i.e.
>> bind/unbind).
>
> Way too many changes in this one patch.
>
>>
>> usbip_get_device() method was not used. The implementation of the
>> method searches a bound devices list by list index. The position in the
>> list is meaningless for any operations so it is assumed not to be used
>> in future.
>>
>> For this patch set, a method to find a bound device in the bound
>> devices list by bus-id is needed. usbip_get_device() is re-implemented
>> as a function to find a bound device by bus-id.
>
> This is not an accurate description. You are really changing look ups using
> bus-id instead bus-num and whether usbip_get_device() is used or not is
> irrelevant.
>
> Please make changes to find bound device by bus-id in a separate patch.
> You will have to change usbip_get_device() anyway because you are changing
> .get_device() interface. Include exporting usbip_get_debice() in a separate
> patch.
>
>>
>> bind and unbind function are exported to reuse by new connect and
>> disconnect operation. Here, connect and disconnect is NEW-3 and NEW-4
>> respactively in diagram below.
>
> Please don't include exporting bind_device() and unbind_device() and
> changing their names in this patch. Send a separate patch for that.
>
> It makes sense to move the functions you are exporting bind_device(),
> unbind_device(), and usbip_get_device() to libsrc - usbip_common.c
> might be a good choice.
>
> The following isn't part of this patch, hence shouldn't be included in
> the change log.
>
>>
>> EXISTING) - invites devices from application(vhci)-side
>> +--+ +--+
>> device--+ STUB | | application/VHCI |
>> +--+ +--+
>> (server) (client)
>> 1) # usbipd ... start daemon
>> = = =
>> 2) # usbip list --local
>> 3) # usbip bind
>> <--- list bound devices --- 4) # usbip list --remote
>> <--- import a device -- 5) # usbip attach
>> = = =
>> X disconnected6) # usbip detach
>> 7) usbip unbind
>>
>> NEW) - dedicates devices from device(stub)-side
>> +--+ +--+
>> device--+ STUB | | application/VHCI |
>> +--+ +--+
>> (client) (server)
>> 1) # usbipa ... start daemon
>> = = =
>> 2) # usbip list --local
>> 3) # usbip connect--- export a device -->
>> = = =
>> 4) # usbip disconnect --- un-export a device --->
>>
>> Bind and unbind are done in connect and disconnect internally.
>>
>> Signed-off-by: Nobuo Iwata
>> ---
>> tools/usb/usbip/libsrc/usbip_host_common.c | 6 ++
>> tools/usb/usbip/libsrc/usbip_host_common.h | 8
>> tools/usb/usbip/src/usbip.h| 3 +++
>> tools/usb/usbip/src/usbip_bind.c | 4 ++--
>> tools/usb/usbip/src/usbip_unbind.c | 4 ++--
>> 5 files changed, 13 insertions(+), 12 deletions(-)
>>
>> diff --git a/tools/usb/usbip/libsrc/usbip_host_common.c
>> b/tools/usb/usbip/libsrc/usbip_host_common.c
>> index 9d41522..6a98d6c 100644
>> --- a/tools/usb/usbip/libsrc/usbip_host_common.c
>> +++ b/tools/usb/usbip/libsrc/usbip_host_common.c
>> @@ -256,17 +256,15 @@ int usbip_export_device(struct usbip_exported_device
>> *edev, int sockfd)
>> }
>>
>> struct usbip_exported_device *usbip_generic_get_device(
>> -struct usbip_host_driver *hdriver, int num)
>> +struct usbip_host_driver *hdriver, char *busid)
>> {
>> struct list_head *i;
>> struct usbip_exported_device *edev;
>> -int cnt = 0;
>>
>> list_for_each(i, &hdriver->edev_list) {
>> edev = list_entry(i, struct usbip_exported_device, node);
>> -if (num == cnt)
>> +if (!strncmp(busid, edev->udev.busid, SYSFS_BUS_ID_SIZE))
>
> Use strlen(edev->udev.busid) instead of SYSFS_BUS_ID_SIZE
>
For me using here strlen() is pointless. strncpy() is here to protect
against unterminated string in edev->udev.busid. In my humble opinion,
instead of using strncpy() all the time I would rather just ensure that
this string is \0 terminated in read_usb_device().
Best regards,
--
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [bug report] USB: opticon: switch to generic read implementation
On Thu, Jan 12, 2017 at 11:39:14PM +0300, Dan Carpenter wrote:
> Hello Johan Hovold,
>
> The patch 7a6ee2b02751: "USB: opticon: switch to generic read
> implementation" from Nov 18, 2012, leads to the following static
> checker warning:
>
> drivers/usb/serial/opticon.c:146 opticon_open()
> info: return a literal instead of 'res'
>
> drivers/usb/serial/opticon.c
>128 static int opticon_open(struct tty_struct *tty, struct
> usb_serial_port *port)
>129 {
>130 struct opticon_private *priv = usb_get_serial_port_data(port);
>131 unsigned long flags;
>132 int res;
>133
>134 spin_lock_irqsave(&priv->lock, flags);
>135 priv->rts = false;
>136 spin_unlock_irqrestore(&priv->lock, flags);
>137
>138 /* Clear RTS line */
>139 send_control_msg(port, CONTROL_RTS, 0);
>140
>141 /* clear the halt status of the endpoint */
>142 usb_clear_halt(port->serial->dev, port->read_urb->pipe);
>143
>144 res = usb_serial_generic_open(tty, port);
>145 if (!res)
>146 return res;
>
> I think this if (!ret) statement is reversed. In the original code we
> used to send RESEND_CTS_STATE on both the failure and success paths.
Indeed it is. Thanks for reporting this.
>147
>148 /* Request CTS line state, sometimes during opening the
> current
>149 * CTS state can be missed. */
>150 send_control_msg(port, RESEND_CTS_STATE, 1);
>151
>152 return res;
>153 }
Thanks,
Johan
--
To unsubscribe from this list: send the line "unsubscribe 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: serial: opticon: fix CTS retrieval at open
The opticon driver used a control request at open to trigger a CTS
status notification to be sent over the bulk-in pipe. When the driver
was converted to using the generic read implementation, an inverted test
prevented this request from being sent, something which could lead to
TIOCMGET reporting an incorrect CTS state.
Reported-by: Dan Carpenter
Fixes: 7a6ee2b02751 ("USB: opticon: switch to generic read
implementation")
Cc: stable
Signed-off-by: Johan Hovold
---
drivers/usb/serial/opticon.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/serial/opticon.c b/drivers/usb/serial/opticon.c
index 5ded6f524d59..b3c64f557d60 100644
--- a/drivers/usb/serial/opticon.c
+++ b/drivers/usb/serial/opticon.c
@@ -142,7 +142,7 @@ static int opticon_open(struct tty_struct *tty, struct
usb_serial_port *port)
usb_clear_halt(port->serial->dev, port->read_urb->pipe);
res = usb_serial_generic_open(tty, port);
- if (!res)
+ if (res)
return res;
/* Request CTS line state, sometimes during opening the current
--
2.10.2
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] USB: serial: opticon: fix CTS retrieval at open
On Fri, Jan 13, 2017 at 01:21:08PM +0100, Johan Hovold wrote:
> The opticon driver used a control request at open to trigger a CTS
> status notification to be sent over the bulk-in pipe. When the driver
> was converted to using the generic read implementation, an inverted test
> prevented this request from being sent, something which could lead to
> TIOCMGET reporting an incorrect CTS state.
>
> Reported-by: Dan Carpenter
> Fixes: 7a6ee2b02751 ("USB: opticon: switch to generic read
> implementation")
> Cc: stable
> Signed-off-by: Johan Hovold
> ---
> drivers/usb/serial/opticon.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: Greg Kroah-Hartman
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/37] usb: host: xhci: dynamically allocate devs array
On 29.12.2016 13:00, Felipe Balbi wrote: Instead of always defaulting to a 256-entry array, we can dynamically allocate devs based on what HW tells us it supports. Note that we can't, yet, purge MAX_HC_SLOTS completely because of struct xhci_device_context_array reliance on it. Signed-off-by: Felipe Balbi I'll postpone this to later, even if patch looks sane and everything I need to make sure the existing code doesn't depend on over-allocating -Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/37] usb: host: xhci: big cleanup series
Hi On 29.12.2016 13:00, Felipe Balbi wrote: Hi Mathias, Here's a resend of my previous series. This time I've added a few more patches which I have been working on and also cherry-pick three old patches which have been pending since May this year. Note, in particular, the work done for XHCI tracers which will help us a lot on debugging sessions. Note, also, that we're starting to save a lot of memory with XHCI after these patches. Only allocating memory for slots which we support, instead of default to static 256 entry arrays. There's still a lot of work to be done, but I suppose this is a step in the right direction. I've been running these patches for several weeks now using SKL most of the time. Tested with our tree of devices we have in the office. Mass Storage, mouse, keyboard, cameras, all still working fine. I'm working with this series. To get this forward and something to 4.11 I'll start picking the simpler and obvious patches. For the tracing part I know that Baolu was/is looking at xhci tracing rework as well, and it would be nice to get his input, well, ack or comments to the tracing patches. There might be some xhci specific weirdness -Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 06/37] usb: host: xhci: WARN on unexpected COMP_SUCCESS
On 29.12.2016 13:00, Felipe Balbi wrote: COMP_SUCCESS should only be asserted on a *true* sucessful transfer. Any other cases are bogus and we should aim to catch them. One easy way to get bug reports is to trigger a WARN_ONCE() on such cases. Signed-off-by: Felipe Balbi Skipping 6/37 and 7/37 Not sure they are really helpful, current solution still prints a message, WARN_ONCE will look scary, and we have plenty of real bugs to attend without this change. -Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 12/37] usb: host: xhci: major rewrite of process_ctrl_td()
On 29.12.2016 13:00, Felipe Balbi wrote: process_ctrl_td() is too complex and difficult to read. We can clean it up a lot and maintain same functionality as before. Note that while cleaning up, one comment was introduced to explain the special case - it wasn't clear before from code. Much of the changes are related to removal of duplicated work done in both process_ctrl_td() and finish_td(). By removing the duplicated code, we could remove a few local variables and shuffle things around so the implementation is more straight forward. Signed-off-by: Felipe Balbi --- Postponing this one, control transfers need more attention as we discovered that we use one TD for the whole control transfer, SETUP+DATA+STATUS stages, while xhci specs indicate each stage should be a separate TD. this affects setting the urb->actual_length, especially in error cases and short transfers -Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH RFC 2/7] net: phy: make phy_(read|write)_mmd() generic MMD accessors
Make phy_(read|write)_mmd() generic 802.3 clause 45 register accessors
for both Clause 22 and Clause 45 PHYs, using either the direct register
reading for Clause 45, or the indirect method for Clause 22 PHYs.
Allow this behaviour to be overriden by PHY drivers where necessary.
Signed-off-by: Russell King
---
drivers/net/phy/phy-core.c | 31 +++
include/linux/phy.h| 24
2 files changed, 47 insertions(+), 8 deletions(-)
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index b8d8276a3099..b50b3a64cf6a 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -69,11 +69,18 @@ EXPORT_SYMBOL(phy_read_mmd_indirect);
*/
int phy_read_mmd(struct phy_device *phydev, int devad, u32 regnum)
{
- if (!phydev->is_c45)
- return -EOPNOTSUPP;
+ if (regnum > (u16)~0 || devad > 32)
+ return -EINVAL;
- return mdiobus_read(phydev->mdio.bus, phydev->mdio.addr,
- MII_ADDR_C45 | (devad << 16) | (regnum & 0x));
+ if (phydev->drv->read_mmd)
+ return phydev->drv->read_mmd(phydev, devad, regnum);
+
+ if (phydev->is_c45) {
+ u32 addr = MII_ADDR_C45 | (devad << 16) | (regnum & 0x);
+ return mdiobus_read(phydev->mdio.bus, phydev->mdio.addr, addr);
+ }
+
+ return phy_read_mmd_indirect(phydev, regnum, devad);
}
EXPORT_SYMBOL(phy_read_mmd);
@@ -125,11 +132,19 @@ EXPORT_SYMBOL(phy_write_mmd_indirect);
*/
int phy_write_mmd(struct phy_device *phydev, int devad, u32 regnum, u16 val)
{
- if (!phydev->is_c45)
- return -EOPNOTSUPP;
+ if (regnum > (u16)~0 || devad > 32)
+ return -EINVAL;
- regnum = MII_ADDR_C45 | ((devad & 0x1f) << 16) | (regnum & 0x);
+ if (phydev->drv->read_mmd)
+ return phydev->drv->write_mmd(phydev, devad, regnum, val);
+
+ if (phydev->is_c45) {
+ u32 addr = MII_ADDR_C45 | (devad << 16) | (regnum & 0x);
+
+ return mdiobus_write(phydev->mdio.bus, phydev->mdio.addr,
+addr, val);
+ }
- return mdiobus_write(phydev->mdio.bus, phydev->mdio.addr, regnum, val);
+ return phy_write_mmd_indirect(phydev, regnum, devad, val);
}
EXPORT_SYMBOL(phy_write_mmd);
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 72acb0233b5f..54fa76efb096 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -583,6 +583,30 @@ struct phy_driver {
*/
void (*link_change_notify)(struct phy_device *dev);
+ /*
+* Phy specific driver override for reading a MMD register.
+* This function is optional for PHY specific drivers. When
+* not provided, the default MMD read function will be used
+* by phy_read_mmd(), which will use either a direct read for
+* Clause 45 PHYs or an indirect read for Clause 22 PHYs.
+* devnum is the MMD device number within the PHY device,
+* regnum is the register within the selected MMD device.
+*/
+ int (*read_mmd)(struct phy_device *dev, int devnum, u16 regnum);
+
+ /*
+* Phy specific driver override for writing a MMD register.
+* This function is optional for PHY specific drivers. When
+* not provided, the default MMD write function will be used
+* by phy_write_mmd(), which will use either a direct write for
+* Clause 45 PHYs, or an indirect write for Clause 22 PHYs.
+* devnum is the MMD device number within the PHY device,
+* regnum is the register within the selected MMD device.
+* val is the value to be written.
+*/
+ int (*write_mmd)(struct phy_device *dev, int devnum, u16 regnum,
+u16 val);
+
/* A function provided by a phy specific driver to override the
* the PHY driver framework support for reading a MMD register
* from the PHY. If not supported, return -1. This function is
--
2.7.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 RFC 3/7] net: lan78xx: update for phy_(read|write)_mmd_indirect() removal
lan78xx appears to use phylib in a rather weird way, accessing the PHY partly through phylib, and partly by makign direct accesses to it, including to the Clause 45 registers. As the indirect MMD accessors are going away, update this driver to use the plain phy_(read|write)_mmd() accessors instead. Signed-off-by: Russell King --- drivers/net/usb/lan78xx.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c index 08f8703e4d54..5e222d47f212 100644 --- a/drivers/net/usb/lan78xx.c +++ b/drivers/net/usb/lan78xx.c @@ -1951,10 +1951,10 @@ static int lan8835_fixup(struct phy_device *phydev) struct lan78xx_net *dev = netdev_priv(phydev->attached_dev); /* LED2/PME_N/IRQ_N/RGMII_ID pin to IRQ_N mode */ - buf = phy_read_mmd_indirect(phydev, 0x8010, 3); + buf = phy_read_mmd(phydev, MDIO_MMD_PCS, 0x8010); buf &= ~0x1800; buf |= 0x0800; - phy_write_mmd_indirect(phydev, 0x8010, 3, buf); + phy_write_mmd(phydev, MDIO_MMD_PCS, 0x8010, buf); /* RGMII MAC TXC Delay Enable */ ret = lan78xx_write_reg(dev, MAC_RGMII_ID, @@ -1974,11 +1974,11 @@ static int ksz9031rnx_fixup(struct phy_device *phydev) /* Micrel9301RNX PHY configuration */ /* RGMII Control Signal Pad Skew */ - phy_write_mmd_indirect(phydev, 4, 2, 0x0077); + phy_write_mmd(phydev, MDIO_MMD_WIS, 4, 0x0077); /* RGMII RX Data Pad Skew */ - phy_write_mmd_indirect(phydev, 5, 2, 0x); + phy_write_mmd(phydev, MDIO_MMD_WIS, 5, 0x); /* RGMII RX Clock Pad Skew */ - phy_write_mmd_indirect(phydev, 8, 2, 0x1FF); + phy_write_mmd(phydev, MDIO_MMD_WIS, 8, 0x1FF); dev->interface = PHY_INTERFACE_MODE_RGMII_RXID; -- 2.7.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 RFC 1/7] net: phy: move phy MMD accessors to phy-core.c
Move the phy_(read|write)__mmd() helpers out of line, they will become
our main MMD accessor functions, and so will be a little more complex.
This complexity doesn't belong in an inline function. Also move the
_indirect variants as well to keep like functionality together.
Signed-off-by: Russell King
---
drivers/net/phy/Makefile | 3 +-
drivers/net/phy/phy-core.c | 135 +
drivers/net/phy/phy.c | 85
include/linux/phy.h| 20 +--
4 files changed, 139 insertions(+), 104 deletions(-)
create mode 100644 drivers/net/phy/phy-core.c
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 356859ac7c18..06fa2e04ac7e 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -1,6 +1,7 @@
# Makefile for Linux PHY drivers and MDIO bus drivers
-libphy-y := phy.o phy_device.o mdio_bus.o mdio_device.o
+libphy-y := phy.o phy_device.o mdio_bus.o mdio_device.o \
+ phy-core.o
libphy-$(CONFIG_SWPHY) += swphy.o
libphy-$(CONFIG_LED_TRIGGER_PHY) += phy_led_triggers.o
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
new file mode 100644
index ..b8d8276a3099
--- /dev/null
+++ b/drivers/net/phy/phy-core.c
@@ -0,0 +1,135 @@
+/*
+ * Core PHY library, taken from phy.c
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+#include
+#include
+
+static inline void mmd_phy_indirect(struct mii_bus *bus, int prtad, int devad,
+ int addr)
+{
+ /* Write the desired MMD Devad */
+ bus->write(bus, addr, MII_MMD_CTRL, devad);
+
+ /* Write the desired MMD register address */
+ bus->write(bus, addr, MII_MMD_DATA, prtad);
+
+ /* Select the Function : DATA with no post increment */
+ bus->write(bus, addr, MII_MMD_CTRL, (devad | MII_MMD_CTRL_NOINCR));
+}
+
+/**
+ * phy_read_mmd_indirect - reads data from the MMD registers
+ * @phydev: The PHY device bus
+ * @prtad: MMD Address
+ * @devad: MMD DEVAD
+ *
+ * Description: it reads data from the MMD registers (clause 22 to access to
+ * clause 45) of the specified phy address.
+ * To read these register we have:
+ * 1) Write reg 13 // DEVAD
+ * 2) Write reg 14 // MMD Address
+ * 3) Write reg 13 // MMD Data Command for MMD DEVAD
+ * 3) Read reg 14 // Read MMD data
+ */
+int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
+{
+ struct phy_driver *phydrv = phydev->drv;
+ int addr = phydev->mdio.addr;
+ int value = -1;
+
+ if (!phydrv->read_mmd_indirect) {
+ struct mii_bus *bus = phydev->mdio.bus;
+
+ mutex_lock(&bus->mdio_lock);
+ mmd_phy_indirect(bus, prtad, devad, addr);
+
+ /* Read the content of the MMD's selected register */
+ value = bus->read(bus, addr, MII_MMD_DATA);
+ mutex_unlock(&bus->mdio_lock);
+ } else {
+ value = phydrv->read_mmd_indirect(phydev, prtad, devad, addr);
+ }
+ return value;
+}
+EXPORT_SYMBOL(phy_read_mmd_indirect);
+
+/**
+ * phy_read_mmd - Convenience function for reading a register
+ * from an MMD on a given PHY.
+ * @phydev: The phy_device struct
+ * @devad: The MMD to read from
+ * @regnum: The register on the MMD to read
+ *
+ * Same rules as for phy_read();
+ */
+int phy_read_mmd(struct phy_device *phydev, int devad, u32 regnum)
+{
+ if (!phydev->is_c45)
+ return -EOPNOTSUPP;
+
+ return mdiobus_read(phydev->mdio.bus, phydev->mdio.addr,
+ MII_ADDR_C45 | (devad << 16) | (regnum & 0x));
+}
+EXPORT_SYMBOL(phy_read_mmd);
+
+/**
+ * phy_write_mmd_indirect - writes data to the MMD registers
+ * @phydev: The PHY device
+ * @prtad: MMD Address
+ * @devad: MMD DEVAD
+ * @data: data to write in the MMD register
+ *
+ * Description: Write data from the MMD registers of the specified
+ * phy address.
+ * To write these register we have:
+ * 1) Write reg 13 // DEVAD
+ * 2) Write reg 14 // MMD Address
+ * 3) Write reg 13 // MMD Data Command for MMD DEVAD
+ * 3) Write reg 14 // Write MMD data
+ */
+void phy_write_mmd_indirect(struct phy_device *phydev, int prtad,
+ int devad, u32 data)
+{
+ struct phy_driver *phydrv = phydev->drv;
+ int addr = phydev->mdio.addr;
+
+ if (!phydrv->write_mmd_indirect) {
+ struct mii_bus *bus = phydev->mdio.bus;
+
+ mutex_lock(&bus->mdio_lock);
+ mmd_phy_indirect(bus, prtad, devad, addr);
+
+ /* Write the data into MMD's selected register */
+ bus->write(bus, addr, MII_MMD_DATA, data);
+
[PATCH RFC 4/7] net: phy: switch remaining users to phy_(read|write)_mmd()
Switch everyone over to using phy_read_mmd() and phy_write_mmd() now
that they are able to handle both Clause 22 indirect addressing and
Clause 45 direct addressing methods to the MMD registers.
Signed-off-by: Russell King
---
drivers/net/phy/bcm-phy-lib.c | 12
drivers/net/phy/dp83867.c | 18 --
drivers/net/phy/intel-xway.c | 26 +-
drivers/net/phy/microchip.c | 5 ++---
drivers/net/phy/phy.c | 25 ++---
drivers/net/phy/phy_device.c | 4 ++--
6 files changed, 39 insertions(+), 51 deletions(-)
diff --git a/drivers/net/phy/bcm-phy-lib.c b/drivers/net/phy/bcm-phy-lib.c
index ab9ad689617c..90be6ee42dfa 100644
--- a/drivers/net/phy/bcm-phy-lib.c
+++ b/drivers/net/phy/bcm-phy-lib.c
@@ -201,8 +201,7 @@ int bcm_phy_set_eee(struct phy_device *phydev, bool enable)
int val;
/* Enable EEE at PHY level */
- val = phy_read_mmd_indirect(phydev, BRCM_CL45VEN_EEE_CONTROL,
- MDIO_MMD_AN);
+ val = phy_read_mmd(phydev, MDIO_MMD_AN, BRCM_CL45VEN_EEE_CONTROL);
if (val < 0)
return val;
@@ -211,12 +210,10 @@ int bcm_phy_set_eee(struct phy_device *phydev, bool
enable)
else
val &= ~(LPI_FEATURE_EN | LPI_FEATURE_EN_DIG1000X);
- phy_write_mmd_indirect(phydev, BRCM_CL45VEN_EEE_CONTROL,
- MDIO_MMD_AN, (u32)val);
+ phy_write_mmd(phydev, MDIO_MMD_AN, BRCM_CL45VEN_EEE_CONTROL, (u32)val);
/* Advertise EEE */
- val = phy_read_mmd_indirect(phydev, BCM_CL45VEN_EEE_ADV,
- MDIO_MMD_AN);
+ val = phy_read_mmd(phydev, MDIO_MMD_AN, BCM_CL45VEN_EEE_ADV);
if (val < 0)
return val;
@@ -225,8 +222,7 @@ int bcm_phy_set_eee(struct phy_device *phydev, bool enable)
else
val &= ~(MDIO_AN_EEE_ADV_100TX | MDIO_AN_EEE_ADV_1000T);
- phy_write_mmd_indirect(phydev, BCM_CL45VEN_EEE_ADV,
- MDIO_MMD_AN, (u32)val);
+ phy_write_mmd(phydev, MDIO_MMD_AN, BCM_CL45VEN_EEE_ADV, (u32)val);
return 0;
}
diff --git a/drivers/net/phy/dp83867.c b/drivers/net/phy/dp83867.c
index 1b639242f9e2..43a5a22234f1 100644
--- a/drivers/net/phy/dp83867.c
+++ b/drivers/net/phy/dp83867.c
@@ -180,8 +180,7 @@ static int dp83867_config_init(struct phy_device *phydev)
if ((phydev->interface >= PHY_INTERFACE_MODE_RGMII_ID) &&
(phydev->interface <= PHY_INTERFACE_MODE_RGMII_RXID)) {
- val = phy_read_mmd_indirect(phydev, DP83867_RGMIICTL,
- DP83867_DEVADDR);
+ val = phy_read_mmd(phydev, DP83867_DEVADDR, DP83867_RGMIICTL);
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID)
val |= (DP83867_RGMII_TX_CLK_DELAY_EN |
DP83867_RGMII_RX_CLK_DELAY_EN);
@@ -192,25 +191,24 @@ static int dp83867_config_init(struct phy_device *phydev)
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID)
val |= DP83867_RGMII_RX_CLK_DELAY_EN;
- phy_write_mmd_indirect(phydev, DP83867_RGMIICTL,
- DP83867_DEVADDR, val);
+ phy_write_mmd(phydev, DP83867_DEVADDR, DP83867_RGMIICTL, val);
delay = (dp83867->rx_id_delay |
(dp83867->tx_id_delay <<
DP83867_RGMII_TX_CLK_DELAY_SHIFT));
- phy_write_mmd_indirect(phydev, DP83867_RGMIIDCTL,
- DP83867_DEVADDR, delay);
+ phy_write_mmd(phydev, DP83867_DEVADDR, DP83867_RGMIIDCTL,
+ delay);
if (dp83867->io_impedance >= 0) {
- val = phy_read_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
- DP83867_DEVADDR);
+ val = phy_read_mmd(phydev, DP83867_DEVADDR,
+ DP83867_IO_MUX_CFG);
val &= ~DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL;
val |= dp83867->io_impedance &
DP83867_IO_MUX_CFG_IO_IMPEDANCE_CTRL;
- phy_write_mmd_indirect(phydev, DP83867_IO_MUX_CFG,
- DP83867_DEVADDR, val);
+ phy_write_mmd(phydev, DP83867_DEVADDR,
+ DP83867_IO_MUX_CFG, val);
}
}
diff --git a/drivers/net/phy/intel-xway.c b/drivers/net/phy/intel-xway.c
index b1fd7bb0e4db..55f8c52dd2f1 100644
--- a/drivers/net/phy/intel-xway.c
+++ b/drivers/net/phy/intel-xway.c
@@ -166,13 +166,13 @@ static int xway_gphy_config_init(struct phy_device
*phydev)
/* Clear all pending interrupts */
phy_read(phydev, XWAY_MDIO_ISTAT);
- phy_write_mmd_indirect(phydev, XWAY_MMD_LEDCH, MDIO_MMD_VEN
[PATCH RFC 7/7] net: phy: clean up mmd_phy_indirect()
Make mmd_phy_indirect() use the same terminology as the rest of the
code, making clear what each address is - phy address, devad, and
register number.
While here, remove the "inline" from this static function, leaving
it to the compiler to decide whether to inline this function, and
get rid of unnecessary parens.
Signed-off-by: Russell King
---
drivers/net/phy/phy-core.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 80795ccd3fab..357a4d0d7641 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -9,17 +9,17 @@
#include
#include
-static inline void mmd_phy_indirect(struct mii_bus *bus, int prtad, int devad,
- int addr)
+static void mmd_phy_indirect(struct mii_bus *bus, int phy_addr, int devad,
+u16 regnum)
{
/* Write the desired MMD Devad */
- bus->write(bus, addr, MII_MMD_CTRL, devad);
+ bus->write(bus, phy_addr, MII_MMD_CTRL, devad);
/* Write the desired MMD register address */
- bus->write(bus, addr, MII_MMD_DATA, prtad);
+ bus->write(bus, phy_addr, MII_MMD_DATA, regnum);
/* Select the Function : DATA with no post increment */
- bus->write(bus, addr, MII_MMD_CTRL, (devad | MII_MMD_CTRL_NOINCR));
+ bus->write(bus, phy_addr, MII_MMD_CTRL, devad | MII_MMD_CTRL_NOINCR);
}
/**
@@ -49,7 +49,7 @@ int phy_read_mmd(struct phy_device *phydev, int devad, u32
regnum)
int phy_addr = phydev->mdio.addr;
mutex_lock(&bus->mdio_lock);
- mmd_phy_indirect(bus, regnum, devad, phy_addr);
+ mmd_phy_indirect(bus, phy_addr, devad, regnum);
/* Read the content of the MMD's selected register */
val = bus->read(bus, phy_addr, MII_MMD_DATA);
@@ -88,7 +88,7 @@ int phy_write_mmd(struct phy_device *phydev, int devad, u32
regnum, u16 val)
int phy_addr = phydev->mdio.addr;
mutex_lock(&bus->mdio_lock);
- mmd_phy_indirect(bus, regnum, devad, phy_addr);
+ mmd_phy_indirect(bus, phy_addr, devad, regnum);
/* Write the data into MMD's selected register */
bus->write(bus, phy_addr, MII_MMD_DATA, val);
--
2.7.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 RFC 6/7] net: phy: remove the indirect MMD read/write methods
Remove the indirect MMD read/write methods which are now no longer
necessary.
Signed-off-by: Russell King
---
drivers/net/phy/phy-core.c | 119 +
include/linux/phy.h| 18 ---
2 files changed, 35 insertions(+), 102 deletions(-)
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index b50b3a64cf6a..80795ccd3fab 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -23,102 +23,41 @@ static inline void mmd_phy_indirect(struct mii_bus *bus,
int prtad, int devad,
}
/**
- * phy_read_mmd_indirect - reads data from the MMD registers
- * @phydev: The PHY device bus
- * @prtad: MMD Address
- * @devad: MMD DEVAD
- *
- * Description: it reads data from the MMD registers (clause 22 to access to
- * clause 45) of the specified phy address.
- * To read these register we have:
- * 1) Write reg 13 // DEVAD
- * 2) Write reg 14 // MMD Address
- * 3) Write reg 13 // MMD Data Command for MMD DEVAD
- * 3) Read reg 14 // Read MMD data
- */
-int phy_read_mmd_indirect(struct phy_device *phydev, int prtad, int devad)
-{
- struct phy_driver *phydrv = phydev->drv;
- int addr = phydev->mdio.addr;
- int value = -1;
-
- if (!phydrv->read_mmd_indirect) {
- struct mii_bus *bus = phydev->mdio.bus;
-
- mutex_lock(&bus->mdio_lock);
- mmd_phy_indirect(bus, prtad, devad, addr);
-
- /* Read the content of the MMD's selected register */
- value = bus->read(bus, addr, MII_MMD_DATA);
- mutex_unlock(&bus->mdio_lock);
- } else {
- value = phydrv->read_mmd_indirect(phydev, prtad, devad, addr);
- }
- return value;
-}
-EXPORT_SYMBOL(phy_read_mmd_indirect);
-
-/**
* phy_read_mmd - Convenience function for reading a register
* from an MMD on a given PHY.
* @phydev: The phy_device struct
- * @devad: The MMD to read from
- * @regnum: The register on the MMD to read
+ * @devad: The MMD to read from (0..31)
+ * @regnum: The register on the MMD to read (0..65535)
*
* Same rules as for phy_read();
*/
int phy_read_mmd(struct phy_device *phydev, int devad, u32 regnum)
{
+ int val;
+
if (regnum > (u16)~0 || devad > 32)
return -EINVAL;
- if (phydev->drv->read_mmd)
- return phydev->drv->read_mmd(phydev, devad, regnum);
-
- if (phydev->is_c45) {
+ if (phydev->drv->read_mmd) {
+ val = phydev->drv->read_mmd(phydev, devad, regnum);
+ } else if (phydev->is_c45) {
u32 addr = MII_ADDR_C45 | (devad << 16) | (regnum & 0x);
- return mdiobus_read(phydev->mdio.bus, phydev->mdio.addr, addr);
- }
-
- return phy_read_mmd_indirect(phydev, regnum, devad);
-}
-EXPORT_SYMBOL(phy_read_mmd);
-
-/**
- * phy_write_mmd_indirect - writes data to the MMD registers
- * @phydev: The PHY device
- * @prtad: MMD Address
- * @devad: MMD DEVAD
- * @data: data to write in the MMD register
- *
- * Description: Write data from the MMD registers of the specified
- * phy address.
- * To write these register we have:
- * 1) Write reg 13 // DEVAD
- * 2) Write reg 14 // MMD Address
- * 3) Write reg 13 // MMD Data Command for MMD DEVAD
- * 3) Write reg 14 // Write MMD data
- */
-void phy_write_mmd_indirect(struct phy_device *phydev, int prtad,
- int devad, u32 data)
-{
- struct phy_driver *phydrv = phydev->drv;
- int addr = phydev->mdio.addr;
- if (!phydrv->write_mmd_indirect) {
+ val = mdiobus_read(phydev->mdio.bus, phydev->mdio.addr, addr);
+ } else {
struct mii_bus *bus = phydev->mdio.bus;
+ int phy_addr = phydev->mdio.addr;
mutex_lock(&bus->mdio_lock);
- mmd_phy_indirect(bus, prtad, devad, addr);
+ mmd_phy_indirect(bus, regnum, devad, phy_addr);
- /* Write the data into MMD's selected register */
- bus->write(bus, addr, MII_MMD_DATA, data);
+ /* Read the content of the MMD's selected register */
+ val = bus->read(bus, phy_addr, MII_MMD_DATA);
mutex_unlock(&bus->mdio_lock);
- } else {
- phydrv->write_mmd_indirect(phydev, prtad, devad, addr, data);
}
+ return val;
}
-EXPORT_SYMBOL(phy_write_mmd_indirect);
+EXPORT_SYMBOL(phy_read_mmd);
/**
* phy_write_mmd - Convenience function for writing a register
@@ -132,19 +71,31 @@ EXPORT_SYMBOL(phy_write_mmd_indirect);
*/
int phy_write_mmd(struct phy_device *phydev, int devad, u32 regnum, u16 val)
{
+ int ret;
+
if (regnum > (u16)~0 || devad > 32)
return -EINVAL;
- if (phydev->drv->read_mmd)
- return phydev->drv->write_mmd(phydev, devad, regnum, val);
-
- if (phydev->is_c45) {
+ if (phydev->drv->read_mmd) {
+ ret = phydev->drv->write_mmd(phydev
[PATCH RFC 5/7] net: phy: convert micrel to new read_mmd/write_mmd driver methods
Convert micrel to the new read_mmd/write_mmd driver methods. This
Clause 22 PHY does not support any MMD access method.
Signed-off-by: Russell King
---
drivers/net/phy/micrel.c | 13 ++---
1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index 9a77289109b7..8d6432c23a14 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -637,8 +637,7 @@ static int ksz8873mll_config_aneg(struct phy_device *phydev)
* MMD extended PHY registers.
*/
static int
-ksz9021_rd_mmd_phyreg(struct phy_device *phydev, int ptrad, int devnum,
- int regnum)
+ksz9021_rd_mmd_phyreg(struct phy_device *phydev, int devad, u16 regnum)
{
return -1;
}
@@ -646,10 +645,10 @@ ksz9021_rd_mmd_phyreg(struct phy_device *phydev, int
ptrad, int devnum,
/* This routine does nothing since the Micrel ksz9021 does not support
* standard IEEE MMD extended PHY registers.
*/
-static void
-ksz9021_wr_mmd_phyreg(struct phy_device *phydev, int ptrad, int devnum,
- int regnum, u32 val)
+static int
+ksz9021_wr_mmd_phyreg(struct phy_device *phydev, int devad, u16 regnum, u16
val)
{
+ return -1;
}
static int kszphy_get_sset_count(struct phy_device *phydev)
@@ -962,8 +961,8 @@ static struct phy_driver ksphy_driver[] = {
.get_stats = kszphy_get_stats,
.suspend= genphy_suspend,
.resume = genphy_resume,
- .read_mmd_indirect = ksz9021_rd_mmd_phyreg,
- .write_mmd_indirect = ksz9021_wr_mmd_phyreg,
+ .read_mmd = ksz9021_rd_mmd_phyreg,
+ .write_mmd = ksz9021_wr_mmd_phyreg,
}, {
.phy_id = PHY_ID_KSZ9031,
.phy_id_mask= MICREL_PHY_ID_MASK,
--
2.7.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 RFC 0/7] Clean up PHY MMD accessors
This series cleans up phylib's MMD accessors, so that we have a common way of accessing the Clause 45 register set. The current situation is far from ideal - we have phy_(read|write)_mmd() which accesses Clause 45 registers over Clause 45 accesses, and we have phy_(read|write)_mmd_indirect(), which accesses Clause 45 registers via Clause 22 register 13/14. Generic code uses the indirect methods to access standard Clause 45 features, and when we come to add Clause 45 PHY support to phylib, we would need to make these conditional upon the PHY type, or duplicate these functions. An alternative solution is to merge these accessors together, and select the appropriate access method depending upon the 802.3 clause that the PHY conforms with. The result is that we have a single set of phy_(read|write)_mmd() accessors. For cases which require special handling, we still allow PHY drivers to override all MMD accesses - except rather than just overriding the indirect accesses. This keeps existing overrides working. Combining the two also has another beneficial side effect - we get rid of similar functions that take arguments in different orders. The old direct accessors took the phy structure, devad and register number, whereas the indirect accessors took the phy structure, register number and devad in that order. Care must be taken when updating future drivers that the argument order is correct, and the function name is not merely replaced. This patch set is against 4.10-rc3 at present. drivers/net/phy/Makefile | 3 +- drivers/net/phy/bcm-phy-lib.c | 12 ++--- drivers/net/phy/dp83867.c | 18 +++ drivers/net/phy/intel-xway.c | 26 +- drivers/net/phy/micrel.c | 13 +++-- drivers/net/phy/microchip.c | 5 +- drivers/net/phy/phy-core.c| 101 ++ drivers/net/phy/phy.c | 110 -- drivers/net/phy/phy_device.c | 4 +- drivers/net/usb/lan78xx.c | 10 ++-- include/linux/phy.h | 56 + 11 files changed, 176 insertions(+), 182 deletions(-) create mode 100644 drivers/net/phy/phy-core.c -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe 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 v14 02/10] usbip: exporting devices: modifications to host side libraries
On 01/13/2017 04:55 AM, Krzysztof Opasiak wrote:
>
>
> On 01/12/2017 10:14 PM, Shuah Khan wrote:
>> On 12/26/2016 12:08 AM, Nobuo Iwata wrote:
>>> Modifications to host driver wrapper and its related operations (i.e.
>>> bind/unbind).
>>
>> Way too many changes in this one patch.
>>
>>>
>>> usbip_get_device() method was not used. The implementation of the
>>> method searches a bound devices list by list index. The position in the
>>> list is meaningless for any operations so it is assumed not to be used
>>> in future.
>>>
>>> For this patch set, a method to find a bound device in the bound
>>> devices list by bus-id is needed. usbip_get_device() is re-implemented
>>> as a function to find a bound device by bus-id.
>>
>> This is not an accurate description. You are really changing look ups using
>> bus-id instead bus-num and whether usbip_get_device() is used or not is
>> irrelevant.
>>
>> Please make changes to find bound device by bus-id in a separate patch.
>> You will have to change usbip_get_device() anyway because you are changing
>> .get_device() interface. Include exporting usbip_get_debice() in a separate
>> patch.
>>
>>>
>>> bind and unbind function are exported to reuse by new connect and
>>> disconnect operation. Here, connect and disconnect is NEW-3 and NEW-4
>>> respactively in diagram below.
>>
>> Please don't include exporting bind_device() and unbind_device() and
>> changing their names in this patch. Send a separate patch for that.
>>
>> It makes sense to move the functions you are exporting bind_device(),
>> unbind_device(), and usbip_get_device() to libsrc - usbip_common.c
>> might be a good choice.
>>
>> The following isn't part of this patch, hence shouldn't be included in
>> the change log.
>>
>>>
>>> EXISTING) - invites devices from application(vhci)-side
>>> +--+ +--+
>>> device--+ STUB | | application/VHCI |
>>> +--+ +--+
>>> (server) (client)
>>> 1) # usbipd ... start daemon
>>> = = =
>>> 2) # usbip list --local
>>> 3) # usbip bind
>>> <--- list bound devices --- 4) # usbip list --remote
>>> <--- import a device -- 5) # usbip attach
>>> = = =
>>> X disconnected6) # usbip detach
>>> 7) usbip unbind
>>>
>>> NEW) - dedicates devices from device(stub)-side
>>> +--+ +--+
>>> device--+ STUB | | application/VHCI |
>>> +--+ +--+
>>> (client) (server)
>>> 1) # usbipa ... start daemon
>>> = = =
>>> 2) # usbip list --local
>>> 3) # usbip connect--- export a device -->
>>> = = =
>>> 4) # usbip disconnect --- un-export a device --->
>>>
>>> Bind and unbind are done in connect and disconnect internally.
>>>
>>> Signed-off-by: Nobuo Iwata
>>> ---
>>> tools/usb/usbip/libsrc/usbip_host_common.c | 6 ++
>>> tools/usb/usbip/libsrc/usbip_host_common.h | 8
>>> tools/usb/usbip/src/usbip.h| 3 +++
>>> tools/usb/usbip/src/usbip_bind.c | 4 ++--
>>> tools/usb/usbip/src/usbip_unbind.c | 4 ++--
>>> 5 files changed, 13 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/tools/usb/usbip/libsrc/usbip_host_common.c
>>> b/tools/usb/usbip/libsrc/usbip_host_common.c
>>> index 9d41522..6a98d6c 100644
>>> --- a/tools/usb/usbip/libsrc/usbip_host_common.c
>>> +++ b/tools/usb/usbip/libsrc/usbip_host_common.c
>>> @@ -256,17 +256,15 @@ int usbip_export_device(struct usbip_exported_device
>>> *edev, int sockfd)
>>> }
>>>
>>> struct usbip_exported_device *usbip_generic_get_device(
>>> - struct usbip_host_driver *hdriver, int num)
>>> + struct usbip_host_driver *hdriver, char *busid)
>>> {
>>> struct list_head *i;
>>> struct usbip_exported_device *edev;
>>> - int cnt = 0;
>>>
>>> list_for_each(i, &hdriver->edev_list) {
>>> edev = list_entry(i, struct usbip_exported_device, node);
>>> - if (num == cnt)
>>> + if (!strncmp(busid, edev->udev.busid, SYSFS_BUS_ID_SIZE))
>>
>> Use strlen(edev->udev.busid) instead of SYSFS_BUS_ID_SIZE
>>
>
> For me using here strlen() is pointless. strncpy() is here to protect
> against unterminated string in edev->udev.busid. In my humble opinion,
> instead of using strncpy() all the time I would rather just ensure that
> this string is \0 terminated in read_usb_device().
>
> Best regards,
>
Yes my real concern here is non-null terminated strings and also that
edev->udev.busid can be trusted over busid. Suing edev->udev.busid
length would only compare stelne bytes as opposed to the max. 32.
Yes you do have to ensure we are dealing with null terminated st
Re: USB ExpressCard makes kworker process utilise 72-75% CPU infinitely
On Fri, 2017-01-13 at 11:43 +0200, Mathias Nyman wrote: > Does this kworker start to utilize the CPU immediately after a fresh > boot when the card is inserted, Yes. Once the card is inserted it immediately stops to consume the CPU at that high level, and it does not stop when the card is removed. > If after first removal then it could be related to this: > https://www.spinics.net/lists/stable/msg155829.html Well than it's probably another cause, isn't it? Thanks, Chris. smime.p7s Description: S/MIME cryptographic signature
Re: [PATCH] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
* Sergei Shtylyov [170113 01:25]: > > @@ -320,7 +323,8 @@ static irqreturn_t cppi41_irq(int irq, void *data) > > int error; > > > > error = pm_runtime_get(cdd->ddev.dev); > > - if (error < 0) > > + if (((error != -EINPROGRESS) && error < 0) || > >Hum, the innermost parens are inconsistent: around != but not around <. Oops just noticed I missed this one in the latest version, will update for the next revision. The typos I already fixed. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe 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] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
* Tony Lindgren [170112 16:04]:
> * Grygorii Strashko [170112 15:43]:
> > @@ -457,38 +449,36 @@ static void push_desc_queue(struct cppi41_channel *c)
> > cppi_writel(reg, cdd->qmgr_mem + QMGR_QUEUE_D(c->q_num));
> > }
> >
> > -static void pending_desc(struct cppi41_channel *c)
> > +static void cppi41_dma_issue_pending(struct dma_chan *chan)
> > {
> > + struct cppi41_channel *c = to_cpp41_chan(chan);
> > struct cppi41_dd *cdd = c->cdd;
> > + int error;
> > + struct cppi41_channel *_c;
> > unsigned long flags;
> >
> > spin_lock_irqsave(&cdd->lock, flags);
> > list_add_tail(&c->node, &cdd->pending);
> > - spin_unlock_irqrestore(&cdd->lock, flags);
> > -}
> > -
> > -static void cppi41_dma_issue_pending(struct dma_chan *chan)
> > -{
> > - struct cppi41_channel *c = to_cpp41_chan(chan);
> > - struct cppi41_dd *cdd = c->cdd;
> > - int error;
> >
> > error = pm_runtime_get(cdd->ddev.dev);
> > - if ((error != -EINPROGRESS) && error < 0) {
> > + if (error < 0) {
> > pm_runtime_put_noidle(cdd->ddev.dev);
> > dev_err(cdd->ddev.dev, "Failed to pm_runtime_get: %i\n",
> > error);
> > -
> > + spin_unlock_irqrestore(&cdd->lock, flags);
> > return;
> > }
> >
> > - if (likely(pm_runtime_active(cdd->ddev.dev)))
> > - push_desc_queue(c);
> > - else
> > - pending_desc(c);
> > + if (!cdd->is_suspended) {
> > + list_for_each_entry_safe(c, _c, &cdd->pending, node) {
> > + push_desc_queue(c);
> > + list_del(&c->node);
> > + };
> > + }
> >
> > pm_runtime_mark_last_busy(cdd->ddev.dev);
> > pm_runtime_put_autosuspend(cdd->ddev.dev);
> > + spin_lock_irqsave(&cdd->lock, flags);
> > }
>
> So always add to the queue no matter, then always flush the queue
> directly if active? Yeah that might work :)
>
> Don't we need spinlock in the list_for_each_entry_safe() to prevent
> it racing with runtime_resume() though?
I could not apply for me as looks like your mail server replaced tabs
with spaces it seems :(
But anyways here's your basic idea plugged into my recent patch and
it seems to work. I maybe have missed something though while reading
so please check.
The pm_runtime_get/mark_last_busy/put_autosuspend and WARN_ON() we
want to keep in cppi41_irq() at least for now :)
And I'm thinking we must also callcppi41_run_queue() with spinlock
held to prevent out of order triggering of the DMA transfers.
Does this look OK to you?
Regards,
Tony
8< ---
diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
index d5ba43a87a68..6ee593eb2acb 100644
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -153,6 +153,8 @@ struct cppi41_dd {
/* context for suspend/resume */
unsigned int dma_tdfdq;
+
+ bool is_suspended;
};
#define FIST_COMPLETION_QUEUE 93
@@ -316,11 +318,12 @@ static irqreturn_t cppi41_irq(int irq, void *data)
__iormb();
while (val) {
+ unsigned long flags;
u32 desc, len;
int error;
error = pm_runtime_get(cdd->ddev.dev);
- if (error < 0)
+ if ((error != -EINPROGRESS) && error < 0)
dev_err(cdd->ddev.dev, "%s pm runtime get:
%i\n",
__func__, error);
@@ -340,6 +343,11 @@ static irqreturn_t cppi41_irq(int irq, void *data)
else
len = pd_trans_len(c->desc->pd0);
+ /* This warning should never trigger */
+ spin_lock_irqsave(&cdd->lock, flags);
+ WARN_ON(cdd->is_suspended);
+ spin_unlock_irqrestore(&cdd->lock, flags);
+
c->residue = pd_trans_len(c->desc->pd6) - len;
dma_cookie_complete(&c->txd);
dmaengine_desc_get_callback_invoke(&c->txd, NULL);
@@ -457,20 +465,26 @@ static void push_desc_queue(struct cppi41_channel *c)
cppi_writel(reg, cdd->qmgr_mem + QMGR_QUEUE_D(c->q_num));
}
-static void pending_desc(struct cppi41_channel *c)
+/*
+ * Caller must hold cdd->lock to prevent push_desc_queue()
+ * getting called out of order. We have both cppi41_dma_issue_pending()
+ * and cppi41_runtime_resume() call this function.
+ */
+static void cppi41_run_queue(struct cppi41_dd *cdd)
{
- struct cppi41_dd *cdd = c->cdd;
- unsigned long flags;
+ struct cppi41_channel *c, *_c;
- spin_lock_irqsave(&cdd->lock, flags);
- list_add_tail(&c->node, &cdd->pending);
- spin_unlock_irqrestore(&cdd->lock, flags);
+ list_for_each_entry_safe(c,
Re: [PATCH] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
* Tony Lindgren [170113 08:27]:
> @@ -316,11 +318,12 @@ static irqreturn_t cppi41_irq(int irq, void *data)
> __iormb();
>
> while (val) {
> + unsigned long flags;
> u32 desc, len;
> int error;
>
> error = pm_runtime_get(cdd->ddev.dev);
> - if (error < 0)
> + if ((error != -EINPROGRESS) && error < 0)
> dev_err(cdd->ddev.dev, "%s pm runtime get:
> %i\n",
> __func__, error);
>
> @@ -340,6 +343,11 @@ static irqreturn_t cppi41_irq(int irq, void *data)
> else
> len = pd_trans_len(c->desc->pd0);
>
> + /* This warning should never trigger */
> + spin_lock_irqsave(&cdd->lock, flags);
> + WARN_ON(cdd->is_suspended);
> + spin_unlock_irqrestore(&cdd->lock, flags);
> +
> c->residue = pd_trans_len(c->desc->pd6) - len;
> dma_cookie_complete(&c->txd);
> dmaengine_desc_get_callback_invoke(&c->txd, NULL);
Hmm this check needs to be before cppi41_pop_desc() already as that does
cppi_readl(). And the spinlocks here don't help anything, we just want
to see a warning if we hit a bug somewhere else.
Regards,
Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
USB Touchscreen displaying cursor in Weston
I'm currently looking into an issue we are having with a USB touchscreen controller causing Weston to display a cursor when plugged in. The device is an "Elo TouchSystems 2216 AccuTouch USB Touchmonitor Interface", which is being handled by hid-generic. Weston (via libinput) is detecting this device as a mouse: Jan 12 12:38:36 GE00409729044C weston[387]: [12:38:36.674] input device 'EloTouchSystems,Inc Elo TouchSystems 2216 AccuTouch® USB Touchmonitor Interface', /dev/input/event5 is tagged by udev as: Mouse Jan 12 12:38:36 GE00409729044C weston[387]: [12:38:36.676] input device 'EloTouchSystems,Inc Elo TouchSystems 2216 AccuTouch® USB Touchmonitor Interface', /dev/input/event5 is a pointer caps Looking at the device capabilities and properties: # cat /sys/class/input/event5/device/capabilities/abs 100 3 # cat /sys/class/input/event5/device/capabilities/key 1 0 0 0 0 0 0 0 0 # cat /sys/class/input/event5/device/properties 0 I think I'm right in saying that this device is providing ABS_X, ABS_Y, ABS_MISC and BTN_LEFT (or BTN_MOUSE as they are the same code). So, looking at the code paths in libinput I believe this is being detected as a "VMware's USB mouse". >From what I understand, a touchscreen should be claiming BTN_TOUCH rather than >BTN_LEFT, which seems to be the case as a second device that I have access to >(Dialogue Technology Corp. PenMount USB) which is working. I see that the PenMount device has a routine that patches up some of the button mapping. Is that what I need to do for this device as well? Thanks in advance, Martyn -- To unsubscribe from this list: send the line "unsubscribe 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 v14 02/10] usbip: exporting devices: modifications to host side libraries
On 01/13/2017 04:46 PM, Shuah Khan wrote:
> On 01/13/2017 04:55 AM, Krzysztof Opasiak wrote:
>>
>>
>> On 01/12/2017 10:14 PM, Shuah Khan wrote:
>>> On 12/26/2016 12:08 AM, Nobuo Iwata wrote:
Modifications to host driver wrapper and its related operations (i.e.
bind/unbind).
>>>
>>> Way too many changes in this one patch.
>>>
usbip_get_device() method was not used. The implementation of the
method searches a bound devices list by list index. The position in the
list is meaningless for any operations so it is assumed not to be used
in future.
For this patch set, a method to find a bound device in the bound
devices list by bus-id is needed. usbip_get_device() is re-implemented
as a function to find a bound device by bus-id.
>>>
>>> This is not an accurate description. You are really changing look ups using
>>> bus-id instead bus-num and whether usbip_get_device() is used or not is
>>> irrelevant.
>>>
>>> Please make changes to find bound device by bus-id in a separate patch.
>>> You will have to change usbip_get_device() anyway because you are changing
>>> .get_device() interface. Include exporting usbip_get_debice() in a separate
>>> patch.
>>>
bind and unbind function are exported to reuse by new connect and
disconnect operation. Here, connect and disconnect is NEW-3 and NEW-4
respactively in diagram below.
>>>
>>> Please don't include exporting bind_device() and unbind_device() and
>>> changing their names in this patch. Send a separate patch for that.
>>>
>>> It makes sense to move the functions you are exporting bind_device(),
>>> unbind_device(), and usbip_get_device() to libsrc - usbip_common.c
>>> might be a good choice.
>>>
>>> The following isn't part of this patch, hence shouldn't be included in
>>> the change log.
>>>
EXISTING) - invites devices from application(vhci)-side
+--+ +--+
device--+ STUB | | application/VHCI |
+--+ +--+
(server) (client)
1) # usbipd ... start daemon
= = =
2) # usbip list --local
3) # usbip bind
<--- list bound devices --- 4) # usbip list --remote
<--- import a device -- 5) # usbip attach
= = =
X disconnected6) # usbip detach
7) usbip unbind
NEW) - dedicates devices from device(stub)-side
+--+ +--+
device--+ STUB | | application/VHCI |
+--+ +--+
(client) (server)
1) # usbipa ... start daemon
= = =
2) # usbip list --local
3) # usbip connect--- export a device -->
= = =
4) # usbip disconnect --- un-export a device --->
Bind and unbind are done in connect and disconnect internally.
Signed-off-by: Nobuo Iwata
---
tools/usb/usbip/libsrc/usbip_host_common.c | 6 ++
tools/usb/usbip/libsrc/usbip_host_common.h | 8
tools/usb/usbip/src/usbip.h| 3 +++
tools/usb/usbip/src/usbip_bind.c | 4 ++--
tools/usb/usbip/src/usbip_unbind.c | 4 ++--
5 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/tools/usb/usbip/libsrc/usbip_host_common.c
b/tools/usb/usbip/libsrc/usbip_host_common.c
index 9d41522..6a98d6c 100644
--- a/tools/usb/usbip/libsrc/usbip_host_common.c
+++ b/tools/usb/usbip/libsrc/usbip_host_common.c
@@ -256,17 +256,15 @@ int usbip_export_device(struct usbip_exported_device
*edev, int sockfd)
}
struct usbip_exported_device *usbip_generic_get_device(
- struct usbip_host_driver *hdriver, int num)
+ struct usbip_host_driver *hdriver, char *busid)
{
struct list_head *i;
struct usbip_exported_device *edev;
- int cnt = 0;
list_for_each(i, &hdriver->edev_list) {
edev = list_entry(i, struct usbip_exported_device, node);
- if (num == cnt)
+ if (!strncmp(busid, edev->udev.busid, SYSFS_BUS_ID_SIZE))
>>>
>>> Use strlen(edev->udev.busid) instead of SYSFS_BUS_ID_SIZE
>>>
>>
>> For me using here strlen() is pointless. strncpy() is here to protect
>> against unterminated string in edev->udev.busid. In my humble opinion,
>> instead of using strncpy() all the time I would rather just ensure that
>> this string is \0 terminated in read_usb_device().
>>
>> Best regards,
>>
>
> Yes my real concern here is non-null terminated strings and also that
> edev->udev.busid can be trusted over busid. Su
Re: [PATCH] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
On 01/13/2017 10:17 AM, Tony Lindgren wrote:
> * Tony Lindgren [170112 16:04]:
>> * Grygorii Strashko [170112 15:43]:
>>> @@ -457,38 +449,36 @@ static void push_desc_queue(struct cppi41_channel *c)
>>> cppi_writel(reg, cdd->qmgr_mem + QMGR_QUEUE_D(c->q_num));
>>> }
>>>
>>> -static void pending_desc(struct cppi41_channel *c)
>>> +static void cppi41_dma_issue_pending(struct dma_chan *chan)
>>> {
>>> + struct cppi41_channel *c = to_cpp41_chan(chan);
>>> struct cppi41_dd *cdd = c->cdd;
>>> + int error;
>>> + struct cppi41_channel *_c;
>>> unsigned long flags;
>>>
>>> spin_lock_irqsave(&cdd->lock, flags);
>>> list_add_tail(&c->node, &cdd->pending);
>>> - spin_unlock_irqrestore(&cdd->lock, flags);
>>> -}
>>> -
>>> -static void cppi41_dma_issue_pending(struct dma_chan *chan)
>>> -{
>>> - struct cppi41_channel *c = to_cpp41_chan(chan);
>>> - struct cppi41_dd *cdd = c->cdd;
>>> - int error;
>>>
>>> error = pm_runtime_get(cdd->ddev.dev);
>>> - if ((error != -EINPROGRESS) && error < 0) {
>>> + if (error < 0) {
>>> pm_runtime_put_noidle(cdd->ddev.dev);
>>> dev_err(cdd->ddev.dev, "Failed to pm_runtime_get: %i\n",
>>> error);
>>> -
>>> + spin_unlock_irqrestore(&cdd->lock, flags);
>>> return;
>>> }
>>>
>>> - if (likely(pm_runtime_active(cdd->ddev.dev)))
>>> - push_desc_queue(c);
>>> - else
>>> - pending_desc(c);
>>> + if (!cdd->is_suspended) {
>>> + list_for_each_entry_safe(c, _c, &cdd->pending, node) {
>>> + push_desc_queue(c);
>>> + list_del(&c->node);
>>> + };
>>> + }
>>>
>>> pm_runtime_mark_last_busy(cdd->ddev.dev);
>>> pm_runtime_put_autosuspend(cdd->ddev.dev);
>>> + spin_lock_irqsave(&cdd->lock, flags);
>>> }
>>
>> So always add to the queue no matter, then always flush the queue
>> directly if active? Yeah that might work :)
>>
>> Don't we need spinlock in the list_for_each_entry_safe() to prevent
>> it racing with runtime_resume() though?
>
> I could not apply for me as looks like your mail server replaced tabs
> with spaces it seems :(
Sorry.
>
> But anyways here's your basic idea plugged into my recent patch and
> it seems to work. I maybe have missed something though while reading
> so please check.
>
> The pm_runtime_get/mark_last_busy/put_autosuspend and WARN_ON() we
> want to keep in cppi41_irq() at least for now :)
As per my understanding and testing it looks like might be enough to
have just pm_runtime_mark_last_busy(cdd->ddev.dev); in cppi41_irq().
But it can be left as is and yes - over all idea is that irq should
not be triggered if device is Idle.
>
> And I'm thinking we must also callcppi41_run_queue() with spinlock
> held to prevent out of order triggering of the DMA transfers.
>
> Does this look OK to you?
>
I think yes. My current version is mostly similar to yours.
>
> 8< ---
> diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
> index d5ba43a87a68..6ee593eb2acb 100644
> --- a/drivers/dma/cppi41.c
> +++ b/drivers/dma/cppi41.c
> @@ -153,6 +153,8 @@ struct cppi41_dd {
>
> /* context for suspend/resume */
> unsigned int dma_tdfdq;
> +
> + bool is_suspended;
> };
>
> #define FIST_COMPLETION_QUEUE93
> @@ -316,11 +318,12 @@ static irqreturn_t cppi41_irq(int irq, void *data)
[..]
>
> pm_runtime_mark_last_busy(cdd->ddev.dev);
> pm_runtime_put_autosuspend(cdd->ddev.dev);
> @@ -1150,6 +1165,11 @@ static int __maybe_unused cppi41_resume(struct device
> *dev)
> static int __maybe_unused cppi41_runtime_suspend(struct device *dev)
> {
> struct cppi41_dd *cdd = dev_get_drvdata(dev);
> + unsigned long flags;
> +
> + spin_lock_irqsave(&cdd->lock, flags);
> + cdd->is_suspended = true;
> + spin_unlock_irqrestore(&cdd->lock, flags);
>
> WARN_ON(!list_empty(&cdd->pending));
Shouldn't we check list_empty() under spin lock?
>
> @@ -1159,14 +1179,11 @@ static int __maybe_unused
> cppi41_runtime_suspend(struct device *dev)
> static int __maybe_unused cppi41_runtime_resume(struct device *dev)
> {
> struct cppi41_dd *cdd = dev_get_drvdata(dev);
> - struct cppi41_channel *c, *_c;
> unsigned long flags;
>
> spin_lock_irqsave(&cdd->lock, flags);
> - list_for_each_entry_safe(c, _c, &cdd->pending, node) {
> - push_desc_queue(c);
> - list_del(&c->node);
> - }
> + cdd->is_suspended = false;
> + cppi41_run_queue(cdd);
> spin_unlock_irqrestore(&cdd->lock, flags);
>
> return 0;
>
--
regards,
-grygorii
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordo
Re: [PATCH v14 06/10] usbip: exporting devices: modifications to attach and detach
On 12/26/2016 08:08 AM, Nobuo Iwata wrote:
> Refactoring to attach and detach operation to reuse common portion to
> application(vhci)-side daemon.
>
> The new application(vhci)-side daemon executes same procedures as
> attach and detach. Most of common code to access vhci has been
> implemented in VHCI userspace wrapper(libsrc/vhci_driver.c) but left in
> attach and detach. With this patch, an accessor of vhci driver and
> tracking of vhci connections in attach and detach are moved to the VHCI
> userspace wrapper.
>
> Functions to create and delete port status file also moved to the
> wrapper. Timing to execute these function are corrected to keep
> consistent with attached status of vhci.
>
> Here, attach, detach and application(vhci)-side daemon is EXISTING-5,
> EXISTING-6 and NEW-1 respectively in diagram below.
>
> EXISTING) - invites devices from application(vhci)-side
> +--+ +--+
> device--+ STUB | | application/VHCI |
> +--+ +--+
> (server) (client)
> 1) # usbipd ... start daemon
> = = =
> 2) # usbip list --local
> 3) # usbip bind
> <--- list bound devices --- 4) # usbip list --remote
> <--- import a device -- 5) # usbip attach
> = = =
> X disconnected6) # usbip detach
> 7) usbip unbind
>
> NEW) - dedicates devices from device(stub)-side
> +--+ +--+
> device--+ STUB | | application/VHCI |
> +--+ +--+
> (client) (server)
> 1) # usbipa ... start daemon
> = = =
> 2) # usbip list --local
> 3) # usbip connect--- export a device -->
> = = =
> 4) # usbip disconnect --- un-export a device --->
>
> Bind and unbind are done in connect and disconnect internally.
>
> Signed-off-by: Nobuo Iwata
> ---
> tools/usb/usbip/libsrc/vhci_driver.c | 96 +++
> tools/usb/usbip/libsrc/vhci_driver.h | 3 +
> tools/usb/usbip/src/usbip_attach.c | 99 +---
> tools/usb/usbip/src/usbip_detach.c | 17 ++---
> 4 files changed, 124 insertions(+), 91 deletions(-)
>
> diff --git a/tools/usb/usbip/libsrc/vhci_driver.c
> b/tools/usb/usbip/libsrc/vhci_driver.c
> index ad92047..5843f43 100644
> --- a/tools/usb/usbip/libsrc/vhci_driver.c
> +++ b/tools/usb/usbip/libsrc/vhci_driver.c
> @@ -7,6 +7,8 @@
> #include
> #include
> #include
> +#include
> +#include
> #include "sysfs_utils.h"
>
> #undef PROGNAME
> @@ -215,6 +217,25 @@ static int read_record(int rhport, char *host, unsigned
> long host_len,
> return 0;
> }
>
> +#define OPEN_HC_MODE_FIRST 0
> +#define OPEN_HC_MODE_REOPEN 1
> +
> +static int open_hc_device(int mode)
> +{
> + if (mode == OPEN_HC_MODE_REOPEN)
> + udev_device_unref(vhci_driver->hc_device);
> +
> + vhci_driver->hc_device =
> + udev_device_new_from_subsystem_sysname(udev_context,
> +USBIP_VHCI_BUS_TYPE,
> +USBIP_VHCI_DRV_NAME);
> + if (!vhci_driver->hc_device) {
> + err("udev_device_new_from_subsystem_sysname failed");
> + return -1;
> + }
> + return 0;
> +}
> +
Please let mi cite my previous email:
Could you please elaborate why this function takes an argument and why
you are reopening vhci device while refreshing device list? there is
nothing about this in commit msg.
could you please answer?
> /* -- */
>
> int usbip_vhci_driver_open(void)
> @@ -227,28 +248,21 @@ int usbip_vhci_driver_open(void)
>
> vhci_driver = calloc(1, sizeof(struct usbip_vhci_driver));
>
> - /* will be freed in usbip_driver_close() */
> - vhci_driver->hc_device =
> - udev_device_new_from_subsystem_sysname(udev_context,
> -USBIP_VHCI_BUS_TYPE,
> -USBIP_VHCI_DRV_NAME);
> - if (!vhci_driver->hc_device) {
> - err("udev_device_new_from_subsystem_sysname failed");
> - goto err;
> - }
> + if (open_hc_device(OPEN_HC_MODE_FIRST))
> + goto err_free_driver;
>
> vhci_driver->nports = get_nports();
>
> dbg("available ports: %d", vhci_driver->nports);
>
> if (refresh_imported_device_list())
> - goto err;
> + goto err_unref_device;
>
> return 0;
>
> -err:
> +err_unref_device:
> udev_device_unref(vhci_driver->hc_device);
> -
> +err_free_driver:
> if (vhci_driv
Re: [PATCH] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
* Grygorii Strashko [170113 09:37]:
> On 01/13/2017 10:17 AM, Tony Lindgren wrote:
> > But anyways here's your basic idea plugged into my recent patch and
> > it seems to work. I maybe have missed something though while reading
> > so please check.
> >
> > The pm_runtime_get/mark_last_busy/put_autosuspend and WARN_ON() we
> > want to keep in cppi41_irq() at least for now :)
>
> As per my understanding and testing it looks like might be enough to
> have just pm_runtime_mark_last_busy(cdd->ddev.dev); in cppi41_irq().
> But it can be left as is and yes - over all idea is that irq should
> not be triggered if device is Idle.
OK yeah kicking the autoidle timeout is needed here.
> > And I'm thinking we must also callcppi41_run_queue() with spinlock
> > held to prevent out of order triggering of the DMA transfers.
> >
> > Does this look OK to you?
>
> I think yes. My current version is mostly similar to yours.
OK will update description and repost shortly.
> > @@ -1150,6 +1165,11 @@ static int __maybe_unused cppi41_resume(struct
> > device *dev)
> > static int __maybe_unused cppi41_runtime_suspend(struct device *dev)
> > {
> > struct cppi41_dd *cdd = dev_get_drvdata(dev);
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(&cdd->lock, flags);
> > + cdd->is_suspended = true;
> > + spin_unlock_irqrestore(&cdd->lock, flags);
> >
> > WARN_ON(!list_empty(&cdd->pending));
>
> Shouldn't we check list_empty() under spin lock?
Yeah let's do that.
Regards,
Tony
--
To unsubscribe from this list: send the line "unsubscribe 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 v14 00/10] usbip: exporting devices
On 12/26/2016 08:08 AM, Nobuo Iwata wrote: > Dear all, > > This series of patches adds exporting device operation to USB/IP. > I run through most of your series and answered with couple of comments. I'm not checking all patches in detail because I see that there is a lot of remarks from previous series which you didn't applied nor reply to them. Please apply those remarks or at least let's make some discussion about them. Best regards, -- Krzysztof Opasiak Samsung R&D Institute Poland Samsung Electronics -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v8 0/2] usbip: vhci number of ports extension
On 12/26/2016 08:18 AM, Nobuo Iwata wrote: > This series of patches extends number of ports limitaion in application > (vhci) side. > (...) > 5. Dependencies > > This series depends on 'usbip: exporting devices' patch set because > this includes changes to application side daemon which introduced the > patch set. You should definitely revers the dependency as this series is way simpler and requires much less discussion so in my humble opinion it has much bigger chances of getting quick into the kernel. Best regards, -- Krzysztof Opasiak Samsung R&D Institute Poland Samsung Electronics -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
Commit fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
together with recent MUSB changes allowed USB and DMA on BeagleBone to idle
when no cable is connected. But looks like few corner case issues still
remain.
Looks like just by re-plugging USB cable about ten or so times on BeagleBone
when configured in USB peripheral mode we can get warnings and eventually
trigger an oops in cppi41 DMA:
WARNING: CPU: 0 PID: 14 at drivers/dma/cppi41.c:1154 cppi41_runtime_suspend+
x28/0x38 [cppi41]
...
WARNING: CPU: 0 PID: 14 at drivers/dma/cppi41.c:452
push_desc_queue+0x94/0x9c [cppi41]
...
Unable to handle kernel NULL pointer dereference at virtual
address 0104
pgd = c0004000
[0104] *pgd=
Internal error: Oops: 805 [#1] SMP ARM
...
[] (cppi41_runtime_resume [cppi41]) from []
(__rpm_callback+0xc0/0x214)
[] (__rpm_callback) from [] (rpm_callback+0x20/0x80)
[] (rpm_callback) from [] (rpm_resume+0x504/0x78c)
[] (rpm_resume) from [] (pm_runtime_work+0x60/0xa8)
[] (pm_runtime_work) from [] (process_one_work+0x2b4/0x808)
This is because of a race with runtime PM and cppi41_dma_issue_pending()
as reported by Alexandre Bailon in earlier
set of patches. Based on mailing list discussions we however came to the
conclusion that a different fix from Alexandre's fix is needed in order
to guarantee that DMA is really active when we try to use it.
To fix the issue, we need to add a driver specific flag as we otherwise
can have -EINPROGRESS state set by runtime PM and can't rely on
pm_runtime_active() to tell us when we can use the DMA.
And we need to make sure the DMA transfers get triggered in the queued
order. So let's always queue the transfers, then flush the queue
from both cppi41_dma_issue_pending() and cppi41_runtime_resume()
as suggested by Grygorii Strashko in an
earlier example patch.
For reference, this is also documented in Documentation/power/runtime_pm.txt
in the example at the end of the file as pointed out by Grygorii Strashko
.
Based on earlier patches from Alexandre Bailon
and Grygorii Strashko modified based on
testing and what was discussed on the mailing lists.
Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
Cc: Andy Shevchenko
Cc: Bin Liu
Cc: Grygorii Strashko
Cc: Kevin Hilman
Cc: Patrick Titiano
Cc: Sergei Shtylyov
Reported-by: Alexandre Bailon
Signed-off-by: Tony Lindgren
---
Alexandre & Grygorii, can you guys please review and test?
Then if OK, I suggest you both reply with Tested-by/Reviewed-by plus
Signed-off-by as this patch contains contributions from both of you.
---
drivers/dma/cppi41.c | 45 +
1 file changed, 29 insertions(+), 16 deletions(-)
diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
--- a/drivers/dma/cppi41.c
+++ b/drivers/dma/cppi41.c
@@ -153,6 +153,8 @@ struct cppi41_dd {
/* context for suspend/resume */
unsigned int dma_tdfdq;
+
+ bool is_suspended;
};
#define FIST_COMPLETION_QUEUE 93
@@ -320,10 +322,13 @@ static irqreturn_t cppi41_irq(int irq, void *data)
int error;
error = pm_runtime_get(cdd->ddev.dev);
- if (error < 0)
+ if ((error != -EINPROGRESS) && (error < 0))
dev_err(cdd->ddev.dev, "%s pm runtime get:
%i\n",
__func__, error);
+ /* This warning should never trigger */
+ WARN_ON(cdd->is_suspended);
+
q_num = __fls(val);
val &= ~(1 << q_num);
q_num += 32 * i;
@@ -457,20 +462,26 @@ static void push_desc_queue(struct cppi41_channel *c)
cppi_writel(reg, cdd->qmgr_mem + QMGR_QUEUE_D(c->q_num));
}
-static void pending_desc(struct cppi41_channel *c)
+/*
+ * Caller must hold cdd->lock to prevent push_desc_queue()
+ * getting called out of order. We have both cppi41_dma_issue_pending()
+ * and cppi41_runtime_resume() call this function.
+ */
+static void cppi41_run_queue(struct cppi41_dd *cdd)
{
- struct cppi41_dd *cdd = c->cdd;
- unsigned long flags;
+ struct cppi41_channel *c, *_c;
- spin_lock_irqsave(&cdd->lock, flags);
- list_add_tail(&c->node, &cdd->pending);
- spin_unlock_irqrestore(&cdd->lock, flags);
+ list_for_each_entry_safe(c, _c, &cdd->pending, node) {
+ push_desc_queue(c);
+ list_del(&c->node);
+ }
}
static void cppi41_dma_issue_pending(struct dma_chan *chan)
{
struct cppi41_channel *c = to_cpp41_chan(chan);
struct cppi41_dd *cdd = c->cdd;
+ unsigned long flags;
int error;
error = pm_runtime_get(cdd->ddev.dev);
@@ -482,10 +493,11 @@ static void cppi41_dma_issue_pending(struct dma_chan
*chan)
return;
}
- if (likely(pm_runtime_active(cdd->ddev.dev)))
- push_desc_queue
Re: [PATCHv2] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
On 01/13/2017 12:01 PM, Tony Lindgren wrote:
> Commit fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
> together with recent MUSB changes allowed USB and DMA on BeagleBone to idle
> when no cable is connected. But looks like few corner case issues still
> remain.
>
> Looks like just by re-plugging USB cable about ten or so times on BeagleBone
> when configured in USB peripheral mode we can get warnings and eventually
> trigger an oops in cppi41 DMA:
>
> WARNING: CPU: 0 PID: 14 at drivers/dma/cppi41.c:1154 cppi41_runtime_suspend+
> x28/0x38 [cppi41]
> ...
>
> WARNING: CPU: 0 PID: 14 at drivers/dma/cppi41.c:452
> push_desc_queue+0x94/0x9c [cppi41]
> ...
>
> Unable to handle kernel NULL pointer dereference at virtual
> address 0104
> pgd = c0004000
> [0104] *pgd=
> Internal error: Oops: 805 [#1] SMP ARM
> ...
> [] (cppi41_runtime_resume [cppi41]) from []
> (__rpm_callback+0xc0/0x214)
> [] (__rpm_callback) from [] (rpm_callback+0x20/0x80)
> [] (rpm_callback) from [] (rpm_resume+0x504/0x78c)
> [] (rpm_resume) from [] (pm_runtime_work+0x60/0xa8)
> [] (pm_runtime_work) from []
> (process_one_work+0x2b4/0x808)
>
> This is because of a race with runtime PM and cppi41_dma_issue_pending()
> as reported by Alexandre Bailon in earlier
> set of patches. Based on mailing list discussions we however came to the
> conclusion that a different fix from Alexandre's fix is needed in order
> to guarantee that DMA is really active when we try to use it.
>
> To fix the issue, we need to add a driver specific flag as we otherwise
> can have -EINPROGRESS state set by runtime PM and can't rely on
> pm_runtime_active() to tell us when we can use the DMA.
>
> And we need to make sure the DMA transfers get triggered in the queued
> order. So let's always queue the transfers, then flush the queue
> from both cppi41_dma_issue_pending() and cppi41_runtime_resume()
> as suggested by Grygorii Strashko in an
> earlier example patch.
>
> For reference, this is also documented in Documentation/power/runtime_pm.txt
> in the example at the end of the file as pointed out by Grygorii Strashko
> .
>
> Based on earlier patches from Alexandre Bailon
> and Grygorii Strashko modified based on
> testing and what was discussed on the mailing lists.
>
> Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
> Cc: Andy Shevchenko
> Cc: Bin Liu
> Cc: Grygorii Strashko
> Cc: Kevin Hilman
> Cc: Patrick Titiano
> Cc: Sergei Shtylyov
> Reported-by: Alexandre Bailon
> Signed-off-by: Tony Lindgren
> ---
>
> Alexandre & Grygorii, can you guys please review and test?
I've just tested it on BBB. And it triggers warning from IRQ
root@am335x-evm:~# [ 242.280546] [ cut here ]
[ 242.285532] WARNING: CPU: 0 PID: 0 at drivers/dma/cppi41.c:330
cppi41_irq+0x228/0x260 [cppi41]
[ 242.335164] CPU: 0 PID: 0 Comm: swapper/0 Tainted: GW
4.10.0-rc3-00560-g51afc29 #133
[ 242.344601] Hardware name: Generic AM33XX (Flattened Device Tree)
[ 242.351084] [] (unwind_backtrace) from []
(show_stack+0x10/0x14)
[ 242.359269] [] (show_stack) from []
(dump_stack+0xac/0xe0)
[ 242.366919] [] (dump_stack) from [] (__warn+0xd8/0x104)
[ 242.374282] [] (__warn) from []
(warn_slowpath_null+0x20/0x28)
[ 242.382299] [] (warn_slowpath_null) from []
(cppi41_irq+0x228/0x260 [cppi41])
[ 242.391714] [] (cppi41_irq [cppi41]) from []
(__handle_irq_event_percpu+0x48/0x3b4)
[ 242.401727] [] (__handle_irq_event_percpu) from []
(handle_irq_event_percpu+0x1c/0x58)
[ 242.412017] [] (handle_irq_event_percpu) from []
(handle_irq_event+0x38/0x5c)
[ 242.421477] [] (handle_irq_event) from []
(handle_level_irq+0xc0/0x154)
[ 242.430378] [] (handle_level_irq) from []
(generic_handle_irq+0x20/0x34)
[ 242.439378] [] (generic_handle_irq) from []
(__handle_domain_irq+0x64/0xe0)
[ 242.448660] [] (__handle_domain_irq) from []
(__irq_svc+0x70/0x98)
[ 242.457117] [] (__irq_svc) from []
(arch_cpu_idle+0x20/0x3c)
[ 242.464935] [] (arch_cpu_idle) from []
(do_idle+0x164/0x218)
[ 242.472748] [] (do_idle) from []
(cpu_startup_entry+0x18/0x1c)
[ 242.480758] [] (cpu_startup_entry) from []
(start_kernel+0x344/0x3bc)
[ 242.489376] ---[ end trace 12c5b6488c1e8c75 ]---
[ 242.496525] usb 1-1.3: USB disconnect, device number 4
test sequence:
- plug usb hub + cd card reader
- plug usb stick in hub
37 mount /dev/sdb /media
38 ls /media/
39 rm /media/data
40 time dd if=/dev/zero of=/media/data bs=128k count=100
41 umount /media
- unplug USB stick - > Warning
^ The same sequence without Hub -- > no warnings
When I plug-unplug USB stick from Hub I can reproduce it pretty often :(
but it can be different issue :(
Patch by itself looks good
root@am335x-evm:~# [ 590.948093] usb 1-1: new high-speed USB device number 6
using musb-hdrc
[ 591.119731] usb 1-1: New USB device found, idVendor=058f, idProduct=6254
[ 591.126841] usb 1-1: New USB device strings: Mfr=0, Product=1,
Re: [PATCHv2] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
* Grygorii Strashko [170113 10:37]:
>
>
> On 01/13/2017 12:01 PM, Tony Lindgren wrote:
> > Commit fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
> > together with recent MUSB changes allowed USB and DMA on BeagleBone to idle
> > when no cable is connected. But looks like few corner case issues still
> > remain.
> >
> > Looks like just by re-plugging USB cable about ten or so times on BeagleBone
> > when configured in USB peripheral mode we can get warnings and eventually
> > trigger an oops in cppi41 DMA:
> >
> > WARNING: CPU: 0 PID: 14 at drivers/dma/cppi41.c:1154 cppi41_runtime_suspend+
> > x28/0x38 [cppi41]
> > ...
> >
> > WARNING: CPU: 0 PID: 14 at drivers/dma/cppi41.c:452
> > push_desc_queue+0x94/0x9c [cppi41]
> > ...
> >
> > Unable to handle kernel NULL pointer dereference at virtual
> > address 0104
> > pgd = c0004000
> > [0104] *pgd=
> > Internal error: Oops: 805 [#1] SMP ARM
> > ...
> > [] (cppi41_runtime_resume [cppi41]) from []
> > (__rpm_callback+0xc0/0x214)
> > [] (__rpm_callback) from [] (rpm_callback+0x20/0x80)
> > [] (rpm_callback) from [] (rpm_resume+0x504/0x78c)
> > [] (rpm_resume) from [] (pm_runtime_work+0x60/0xa8)
> > [] (pm_runtime_work) from []
> > (process_one_work+0x2b4/0x808)
> >
> > This is because of a race with runtime PM and cppi41_dma_issue_pending()
> > as reported by Alexandre Bailon in earlier
> > set of patches. Based on mailing list discussions we however came to the
> > conclusion that a different fix from Alexandre's fix is needed in order
> > to guarantee that DMA is really active when we try to use it.
> >
> > To fix the issue, we need to add a driver specific flag as we otherwise
> > can have -EINPROGRESS state set by runtime PM and can't rely on
> > pm_runtime_active() to tell us when we can use the DMA.
> >
> > And we need to make sure the DMA transfers get triggered in the queued
> > order. So let's always queue the transfers, then flush the queue
> > from both cppi41_dma_issue_pending() and cppi41_runtime_resume()
> > as suggested by Grygorii Strashko in an
> > earlier example patch.
> >
> > For reference, this is also documented in Documentation/power/runtime_pm.txt
> > in the example at the end of the file as pointed out by Grygorii Strashko
> > .
> >
> > Based on earlier patches from Alexandre Bailon
> > and Grygorii Strashko modified based on
> > testing and what was discussed on the mailing lists.
> >
> > Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
> > Cc: Andy Shevchenko
> > Cc: Bin Liu
> > Cc: Grygorii Strashko
> > Cc: Kevin Hilman
> > Cc: Patrick Titiano
> > Cc: Sergei Shtylyov
> > Reported-by: Alexandre Bailon
> > Signed-off-by: Tony Lindgren
> > ---
> >
> > Alexandre & Grygorii, can you guys please review and test?
>
> I've just tested it on BBB. And it triggers warning from IRQ
>
> root@am335x-evm:~# [ 242.280546] [ cut here ]
> [ 242.285532] WARNING: CPU: 0 PID: 0 at drivers/dma/cppi41.c:330
> cppi41_irq+0x228/0x260 [cppi41]
> [ 242.335164] CPU: 0 PID: 0 Comm: swapper/0 Tainted: GW
> 4.10.0-rc3-00560-g51afc29 #133
> [ 242.344601] Hardware name: Generic AM33XX (Flattened Device Tree)
> [ 242.351084] [] (unwind_backtrace) from []
> (show_stack+0x10/0x14)
> [ 242.359269] [] (show_stack) from []
> (dump_stack+0xac/0xe0)
> [ 242.366919] [] (dump_stack) from [] (__warn+0xd8/0x104)
> [ 242.374282] [] (__warn) from []
> (warn_slowpath_null+0x20/0x28)
> [ 242.382299] [] (warn_slowpath_null) from []
> (cppi41_irq+0x228/0x260 [cppi41])
> [ 242.391714] [] (cppi41_irq [cppi41]) from []
> (__handle_irq_event_percpu+0x48/0x3b4)
> [ 242.401727] [] (__handle_irq_event_percpu) from []
> (handle_irq_event_percpu+0x1c/0x58)
> [ 242.412017] [] (handle_irq_event_percpu) from []
> (handle_irq_event+0x38/0x5c)
> [ 242.421477] [] (handle_irq_event) from []
> (handle_level_irq+0xc0/0x154)
> [ 242.430378] [] (handle_level_irq) from []
> (generic_handle_irq+0x20/0x34)
> [ 242.439378] [] (generic_handle_irq) from []
> (__handle_domain_irq+0x64/0xe0)
> [ 242.448660] [] (__handle_domain_irq) from []
> (__irq_svc+0x70/0x98)
> [ 242.457117] [] (__irq_svc) from []
> (arch_cpu_idle+0x20/0x3c)
> [ 242.464935] [] (arch_cpu_idle) from []
> (do_idle+0x164/0x218)
> [ 242.472748] [] (do_idle) from []
> (cpu_startup_entry+0x18/0x1c)
> [ 242.480758] [] (cpu_startup_entry) from []
> (start_kernel+0x344/0x3bc)
> [ 242.489376] ---[ end trace 12c5b6488c1e8c75 ]---
> [ 242.496525] usb 1-1.3: USB disconnect, device number 4
>
> test sequence:
> - plug usb hub + cd card reader
> - plug usb stick in hub
> 37 mount /dev/sdb /media
>38 ls /media/
>39 rm /media/data
>40 time dd if=/dev/zero of=/media/data bs=128k count=100
>41 umount /media
> - unplug USB stick - > Warning
>
> ^ The same sequence without Hub -- > no warnings
>
> When I plug-unplug USB stick from Hub I can reproduce it pretty often :(
> but it ca
Re: [PATCHv2] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
On 01/13/2017 01:01 PM, Tony Lindgren wrote:
> * Grygorii Strashko [170113 10:37]:
>>
>>
>> On 01/13/2017 12:01 PM, Tony Lindgren wrote:
>>> Commit fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
>>> together with recent MUSB changes allowed USB and DMA on BeagleBone to idle
>>> when no cable is connected. But looks like few corner case issues still
>>> remain.
>>>
>>> Looks like just by re-plugging USB cable about ten or so times on BeagleBone
>>> when configured in USB peripheral mode we can get warnings and eventually
>>> trigger an oops in cppi41 DMA:
>>>
>>> WARNING: CPU: 0 PID: 14 at drivers/dma/cppi41.c:1154 cppi41_runtime_suspend+
>>> x28/0x38 [cppi41]
>>> ...
>>>
>>> WARNING: CPU: 0 PID: 14 at drivers/dma/cppi41.c:452
>>> push_desc_queue+0x94/0x9c [cppi41]
>>> ...
>>>
>>> Unable to handle kernel NULL pointer dereference at virtual
>>> address 0104
>>> pgd = c0004000
>>> [0104] *pgd=
>>> Internal error: Oops: 805 [#1] SMP ARM
>>> ...
>>> [] (cppi41_runtime_resume [cppi41]) from []
>>> (__rpm_callback+0xc0/0x214)
>>> [] (__rpm_callback) from [] (rpm_callback+0x20/0x80)
>>> [] (rpm_callback) from [] (rpm_resume+0x504/0x78c)
>>> [] (rpm_resume) from [] (pm_runtime_work+0x60/0xa8)
>>> [] (pm_runtime_work) from []
>>> (process_one_work+0x2b4/0x808)
>>>
>>> This is because of a race with runtime PM and cppi41_dma_issue_pending()
>>> as reported by Alexandre Bailon in earlier
>>> set of patches. Based on mailing list discussions we however came to the
>>> conclusion that a different fix from Alexandre's fix is needed in order
>>> to guarantee that DMA is really active when we try to use it.
>>>
>>> To fix the issue, we need to add a driver specific flag as we otherwise
>>> can have -EINPROGRESS state set by runtime PM and can't rely on
>>> pm_runtime_active() to tell us when we can use the DMA.
>>>
>>> And we need to make sure the DMA transfers get triggered in the queued
>>> order. So let's always queue the transfers, then flush the queue
>>> from both cppi41_dma_issue_pending() and cppi41_runtime_resume()
>>> as suggested by Grygorii Strashko in an
>>> earlier example patch.
>>>
>>> For reference, this is also documented in Documentation/power/runtime_pm.txt
>>> in the example at the end of the file as pointed out by Grygorii Strashko
>>> .
>>>
>>> Based on earlier patches from Alexandre Bailon
>>> and Grygorii Strashko modified based on
>>> testing and what was discussed on the mailing lists.
>>>
>>> Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
>>> Cc: Andy Shevchenko
>>> Cc: Bin Liu
>>> Cc: Grygorii Strashko
>>> Cc: Kevin Hilman
>>> Cc: Patrick Titiano
>>> Cc: Sergei Shtylyov
>>> Reported-by: Alexandre Bailon
>>> Signed-off-by: Tony Lindgren
>>> ---
>>>
>>> Alexandre & Grygorii, can you guys please review and test?
>>
>> I've just tested it on BBB. And it triggers warning from IRQ
>>
>> root@am335x-evm:~# [ 242.280546] [ cut here ]
>> [ 242.285532] WARNING: CPU: 0 PID: 0 at drivers/dma/cppi41.c:330
>> cppi41_irq+0x228/0x260 [cppi41]
>> [ 242.335164] CPU: 0 PID: 0 Comm: swapper/0 Tainted: GW
>> 4.10.0-rc3-00560-g51afc29 #133
>> [ 242.344601] Hardware name: Generic AM33XX (Flattened Device Tree)
>> [ 242.351084] [] (unwind_backtrace) from []
>> (show_stack+0x10/0x14)
>> [ 242.359269] [] (show_stack) from []
>> (dump_stack+0xac/0xe0)
>> [ 242.366919] [] (dump_stack) from []
>> (__warn+0xd8/0x104)
>> [ 242.374282] [] (__warn) from []
>> (warn_slowpath_null+0x20/0x28)
>> [ 242.382299] [] (warn_slowpath_null) from []
>> (cppi41_irq+0x228/0x260 [cppi41])
>> [ 242.391714] [] (cppi41_irq [cppi41]) from []
>> (__handle_irq_event_percpu+0x48/0x3b4)
>> [ 242.401727] [] (__handle_irq_event_percpu) from []
>> (handle_irq_event_percpu+0x1c/0x58)
>> [ 242.412017] [] (handle_irq_event_percpu) from []
>> (handle_irq_event+0x38/0x5c)
>> [ 242.421477] [] (handle_irq_event) from []
>> (handle_level_irq+0xc0/0x154)
>> [ 242.430378] [] (handle_level_irq) from []
>> (generic_handle_irq+0x20/0x34)
>> [ 242.439378] [] (generic_handle_irq) from []
>> (__handle_domain_irq+0x64/0xe0)
>> [ 242.448660] [] (__handle_domain_irq) from []
>> (__irq_svc+0x70/0x98)
>> [ 242.457117] [] (__irq_svc) from []
>> (arch_cpu_idle+0x20/0x3c)
>> [ 242.464935] [] (arch_cpu_idle) from []
>> (do_idle+0x164/0x218)
>> [ 242.472748] [] (do_idle) from []
>> (cpu_startup_entry+0x18/0x1c)
>> [ 242.480758] [] (cpu_startup_entry) from []
>> (start_kernel+0x344/0x3bc)
>> [ 242.489376] ---[ end trace 12c5b6488c1e8c75 ]---
>> [ 242.496525] usb 1-1.3: USB disconnect, device number 4
>>
>> test sequence:
>> - plug usb hub + cd card reader
>> - plug usb stick in hub
>> 37 mount /dev/sdb /media
>>38 ls /media/
>>39 rm /media/data
>>40 time dd if=/dev/zero of=/media/data bs=128k count=100
>>41 umount /media
>> - unplug USB stick - > Warning
>>
>> ^ The same sequence without Hub -- > no warni
Re: [PATCHv2] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
On 01/13/2017 01:53 PM, Grygorii Strashko wrote:
>
>
> On 01/13/2017 01:01 PM, Tony Lindgren wrote:
>> * Grygorii Strashko [170113 10:37]:
>>>
>>>
>>> On 01/13/2017 12:01 PM, Tony Lindgren wrote:
Commit fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
together with recent MUSB changes allowed USB and DMA on BeagleBone to idle
when no cable is connected. But looks like few corner case issues still
remain.
Looks like just by re-plugging USB cable about ten or so times on
BeagleBone
when configured in USB peripheral mode we can get warnings and eventually
trigger an oops in cppi41 DMA:
WARNING: CPU: 0 PID: 14 at drivers/dma/cppi41.c:1154
cppi41_runtime_suspend+
x28/0x38 [cppi41]
...
WARNING: CPU: 0 PID: 14 at drivers/dma/cppi41.c:452
push_desc_queue+0x94/0x9c [cppi41]
...
Unable to handle kernel NULL pointer dereference at virtual
address 0104
pgd = c0004000
[0104] *pgd=
Internal error: Oops: 805 [#1] SMP ARM
...
[] (cppi41_runtime_resume [cppi41]) from []
(__rpm_callback+0xc0/0x214)
[] (__rpm_callback) from [] (rpm_callback+0x20/0x80)
[] (rpm_callback) from [] (rpm_resume+0x504/0x78c)
[] (rpm_resume) from [] (pm_runtime_work+0x60/0xa8)
[] (pm_runtime_work) from []
(process_one_work+0x2b4/0x808)
This is because of a race with runtime PM and cppi41_dma_issue_pending()
as reported by Alexandre Bailon in earlier
set of patches. Based on mailing list discussions we however came to the
conclusion that a different fix from Alexandre's fix is needed in order
to guarantee that DMA is really active when we try to use it.
To fix the issue, we need to add a driver specific flag as we otherwise
can have -EINPROGRESS state set by runtime PM and can't rely on
pm_runtime_active() to tell us when we can use the DMA.
And we need to make sure the DMA transfers get triggered in the queued
order. So let's always queue the transfers, then flush the queue
from both cppi41_dma_issue_pending() and cppi41_runtime_resume()
as suggested by Grygorii Strashko in an
earlier example patch.
For reference, this is also documented in
Documentation/power/runtime_pm.txt
in the example at the end of the file as pointed out by Grygorii Strashko
.
Based on earlier patches from Alexandre Bailon
and Grygorii Strashko modified based on
testing and what was discussed on the mailing lists.
Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
Cc: Andy Shevchenko
Cc: Bin Liu
Cc: Grygorii Strashko
Cc: Kevin Hilman
Cc: Patrick Titiano
Cc: Sergei Shtylyov
Reported-by: Alexandre Bailon
Signed-off-by: Tony Lindgren
---
Alexandre & Grygorii, can you guys please review and test?
>>>
>>> I've just tested it on BBB. And it triggers warning from IRQ
>>>
>>> root@am335x-evm:~# [ 242.280546] [ cut here ]
>>> [ 242.285532] WARNING: CPU: 0 PID: 0 at drivers/dma/cppi41.c:330
>>> cppi41_irq+0x228/0x260 [cppi41]
>>> [ 242.335164] CPU: 0 PID: 0 Comm: swapper/0 Tainted: GW
>>> 4.10.0-rc3-00560-g51afc29 #133
>>> [ 242.344601] Hardware name: Generic AM33XX (Flattened Device Tree)
>>> [ 242.351084] [] (unwind_backtrace) from []
>>> (show_stack+0x10/0x14)
>>> [ 242.359269] [] (show_stack) from []
>>> (dump_stack+0xac/0xe0)
>>> [ 242.366919] [] (dump_stack) from []
>>> (__warn+0xd8/0x104)
>>> [ 242.374282] [] (__warn) from []
>>> (warn_slowpath_null+0x20/0x28)
>>> [ 242.382299] [] (warn_slowpath_null) from []
>>> (cppi41_irq+0x228/0x260 [cppi41])
>>> [ 242.391714] [] (cppi41_irq [cppi41]) from []
>>> (__handle_irq_event_percpu+0x48/0x3b4)
>>> [ 242.401727] [] (__handle_irq_event_percpu) from []
>>> (handle_irq_event_percpu+0x1c/0x58)
>>> [ 242.412017] [] (handle_irq_event_percpu) from []
>>> (handle_irq_event+0x38/0x5c)
>>> [ 242.421477] [] (handle_irq_event) from []
>>> (handle_level_irq+0xc0/0x154)
>>> [ 242.430378] [] (handle_level_irq) from []
>>> (generic_handle_irq+0x20/0x34)
>>> [ 242.439378] [] (generic_handle_irq) from []
>>> (__handle_domain_irq+0x64/0xe0)
>>> [ 242.448660] [] (__handle_domain_irq) from []
>>> (__irq_svc+0x70/0x98)
>>> [ 242.457117] [] (__irq_svc) from []
>>> (arch_cpu_idle+0x20/0x3c)
>>> [ 242.464935] [] (arch_cpu_idle) from []
>>> (do_idle+0x164/0x218)
>>> [ 242.472748] [] (do_idle) from []
>>> (cpu_startup_entry+0x18/0x1c)
>>> [ 242.480758] [] (cpu_startup_entry) from []
>>> (start_kernel+0x344/0x3bc)
>>> [ 242.489376] ---[ end trace 12c5b6488c1e8c75 ]---
>>> [ 242.496525] usb 1-1.3: USB disconnect, device number 4
>>>
>>> test sequence:
>>> - plug usb hub + cd card reader
>>> - plug usb stick in hub
>>> 37 mount /dev/sdb /media
>>>38 ls /media/
>>>
Re: [PATCH] Documentation: dt: dwc3: add reference to the usb-xhci properties
On Wed, Jan 11, 2017 at 03:59:42PM +0100, Martin Blumenstingl wrote: > dwc3 internally creates a usb-xhci device which means that all > properties documented in usb-xhci.txt are supported as well. > > Signed-off-by: Martin Blumenstingl > --- > Documentation/devicetree/bindings/usb/dwc3.txt | 4 > 1 file changed, 4 insertions(+) Acked-by: Rob Herring -- To unsubscribe from this list: send the line "unsubscribe 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: VL805 USB 3.0 does not see connected devices (only on x86_64) (x86 is ok)
kernel 4.9.3 still not working latest log: [ 196.984424] sd 7:0:0:0: [sdc] 31293440 512-byte logical blocks: (16.0 GB/14.9 GiB) [ 196.984564] sd 7:0:0:0: [sdc] Write Protect is off [ 196.984566] sd 7:0:0:0: [sdc] Mode Sense: 23 00 00 00 [ 196.984689] sd 7:0:0:0: [sdc] No Caching mode page found [ 196.984690] sd 7:0:0:0: [sdc] Assuming drive cache: write through [ 196.984696] device: '8:32': device_add [ 196.984714] PM: Adding info for No Bus:8:32 [ 196.984727] device: 'sdc': device_add [ 196.984747] PM: Adding info for No Bus:sdc [ 197.085658] sdc: sdc1 [ 197.085664] device: 'sdc1': device_add [ 197.085680] PM: Adding info for No Bus:sdc1 [ 197.086929] sd 7:0:0:0: [sdc] Attached SCSI removable disk [ 230.955006] FAT-fs (sdc1): Volume was not properly unmounted. Some data may be corrupt. Please run fsck. [ 234.422942] DMAR: DRHD: handling fault status reg 2 [ 234.422946] DMAR: [DMA Read] Request device [02:00.0] fault addr fffbb000 [fault reason 06] PTE Read access is not set [ 234.422950] xhci_hcd :02:00.0: WARNING: Host System Error [ 234.446078] xhci_hcd :02:00.0: Host not halted after 16000 microseconds. [ 270.447035] xhci_hcd :02:00.0: xHCI host not responding to stop endpoint command. [ 270.447040] xhci_hcd :02:00.0: Assuming host is dying, halting host. [ 270.470143] xhci_hcd :02:00.0: Host not halted after 16000 microseconds. [ 270.470153] xhci_hcd :02:00.0: Non-responsive xHCI host is not halting. [ 270.470154] xhci_hcd :02:00.0: Completing active URBs anyway. [ 270.470161] xhci_hcd :02:00.0: HC died; cleaning up [ 270.470265] sd 7:0:0:0: [sdc] tag#0 FAILED Result: hostbyte=DID_TIME_OUT driverbyte=DRIVER_OK [ 270.470268] sd 7:0:0:0: [sdc] tag#0 CDB: Read(10) 28 00 00 06 51 22 00 00 0e 00 [ 270.470269] blk_update_request: I/O error, dev sdc, sector 413986 [ 270.538994] sd 7:0:0:0: [sdc] tag#0 FAILED Result: hostbyte=DID_ERROR driverbyte=DRIVER_OK [ 270.538997] sd 7:0:0:0: [sdc] tag#0 CDB: Read(10) 28 00 00 06 51 23 00 00 0d 00 [ 270.538998] blk_update_request: I/O error, dev sdc, sector 413987 [ 270.986995] sd 7:0:0:0: [sdc] tag#0 FAILED Result: hostbyte=DID_ERROR driverbyte=DRIVER_OK [ 270.986997] sd 7:0:0:0: [sdc] tag#0 CDB: Read(10) 28 00 00 06 51 22 00 00 01 00 [ 270.986999] blk_update_request: I/O error, dev sdc, sector 413986 [ 270.987063] FAT-fs (sdc1): Directory bread(block 411938) failed [ 271.307002] sd 7:0:0:0: [sdc] tag#0 FAILED Result: hostbyte=DID_ERROR driverbyte=DRIVER_OK [ 271.307004] sd 7:0:0:0: [sdc] tag#0 CDB: Read(10) 28 00 00 06 51 23 00 00 01 00 [ 271.307005] blk_update_request: I/O error, dev sdc, sector 413987 [ 271.307062] FAT-fs (sdc1): Directory bread(block 411939) failed [ 271.307079] FAT-fs (sdc1): Directory bread(block 411940) failed [ 271.307084] FAT-fs (sdc1): Directory bread(block 411941) failed [ 271.307088] FAT-fs (sdc1): Directory bread(block 411942) failed [ 271.307092] FAT-fs (sdc1): Directory bread(block 411943) failed [ 271.307096] FAT-fs (sdc1): Directory bread(block 411944) failed [ 271.307099] FAT-fs (sdc1): Directory bread(block 411945) failed [ 271.307103] FAT-fs (sdc1): Directory bread(block 411946) failed [ 271.307106] FAT-fs (sdc1): Directory bread(block 411947) failed [ 271.307174] FAT-fs (sdc1): FAT read failed (blocknr 2446) [ 271.307206] FAT-fs (sdc1): FAT read failed (blocknr 2821) [ 271.307229] FAT-fs (sdc1): FAT read failed (blocknr 3093) [ 271.307279] FAT-fs (sdc1): FAT read failed (blocknr 17088) [ 271.307298] FAT-fs (sdc1): FAT read failed (blocknr 17089) [ 271.308434] FAT-fs (sdc1): FAT read failed (blocknr 2241) [ 282.002018] xhci_hcd :02:00.0: Stopped the command ring failed, maybe the host is dead [ 282.025084] xhci_hcd :02:00.0: Host not halted after 16000 microseconds. [ 282.025084] xhci_hcd :02:00.0: Abort command ring failed [ 282.025104] xhci_hcd :02:00.0: HC died; cleaning up [ 282.025108] usb 3-1: USB disconnect, device number 2 2017-01-10 13:06 GMT+03:00 c400 : > hi! > Using 4.9.2 kernel. Still no success with VL805 USB 3.0 > i can give a remote shell to test and reproduce the bug > > any help? > > 2016-09-23 0:47 GMT+03:00 c400 : >> btw >> i've tested a liitle better and the disk is visible, but write errors occurs >> after ejecting and inserting it is invisible again >> >> [129416.884960] sd 7:0:0:0: [sdc] 1250263728 512-byte logical blocks: >> (640 GB/596 GiB) >> [129416.885429] sd 7:0:0:0: [sdc] Write Protect is off >> [129416.885434] sd 7:0:0:0: [sdc] Mode Sense: 23 00 00 00 >> [129416.885798] sd 7:0:0:0: [sdc] No Caching mode page found >> [129416.885801] sd 7:0:0:0: [sdc] Assuming drive cache: write through >> [129416.885806] device: '8:32': device_add >> [129416.885820] PM: Adding info for No Bus:8:32 >> [129416.885831] device: 'sdc': device_add >> [129416.885847] PM: Adding info for No Bus:sdc >> [129416.897283] sdc: sdc1 >> [129416.897291] device: 'sdc1': device_add >> [129416.897309] PM: Adding info for No Bus:s
Re: [RFC PATCH 1/2] usb: host: add a generic platform USB roothub driver
On Wed, Jan 11, 2017 at 04:29:46PM +0100, Martin Blumenstingl wrote:
> Many SoC platforms have separate devices for the USB PHY which are
> registered through the generic PHY framework. These PHYs have to be
> enabled to make the USB controller actually work. They also have to be
> disabled again on shutdown/suspend.
>
> Currently (at least) the following HCI platform drivers are using custom
> code to obtain all PHYs via devicetree for the roothub/controller and
> disable/enable them when required:
> - ehci-platform.c has ehci_platform_power_{on,off}
> - xhci-mtk.c has xhci_mtk_phy_{init,exit,power_on,power_off}
> - ohci-platform.c has ohci_platform_power_{on,off}
>
> These drivers are not using the generic devicetree USB device bindings
> yet which were only introduced recently (documentation is available in
> devicetree/bindings/usb/usb-device.txt).
> With this new driver the usb2-phy and usb3-phy can be specified directly
> in the child-node of the corresponding port of the roothub via
> devicetree. This can be extended by not just parsing PHYs (some of the
> other drivers listed above are for example also parsing a list of clocks
> as well) when required.
>
> Signed-off-by: Martin Blumenstingl
> ---
> .../devicetree/bindings/usb/usb-roothub.txt| 41 ++
> drivers/usb/host/Kconfig | 3 +
> drivers/usb/host/Makefile | 2 +
> drivers/usb/host/platform-roothub.c| 146
> +
> drivers/usb/host/platform-roothub.h| 14 ++
> 5 files changed, 206 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/usb/usb-roothub.txt
> create mode 100644 drivers/usb/host/platform-roothub.c
> create mode 100644 drivers/usb/host/platform-roothub.h
>
> diff --git a/Documentation/devicetree/bindings/usb/usb-roothub.txt
> b/Documentation/devicetree/bindings/usb/usb-roothub.txt
> new file mode 100644
> index ..96e152d3901c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/usb-roothub.txt
> @@ -0,0 +1,41 @@
> +Generic USB root-hub Properties
> +
> +similar to the USB device bindings (documented in usb-device.txt from the
> +current directory) this provides support for configuring the root-hub.
> +
> +Required properties:
> +- compatible: should be at least one of "usb1d6b,3", "usb1d6b,2"
> +- reg: must be 0.
> +- address-cells: must be 1
> +- size-cells: must be 0
> +- a sub-node per port supports the following properties:
Make this another section with required and optional sections.
> + - reg: the port number on the root-hub (mandatory)
> + - phys: optional, from the *Generic PHY* bindings (mandatory needed when
> +phy-names is given)
> + - phy-names: optional, from the *Generic PHY* bindings; supported names
> +are "usb2-phy" or "usb3-phy"
> +
> +Example:
> + &usb1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + roothub@0 {
> + compatible = "usb1d6b,3", "usb1d6b,2";
Is this discoverable? IIRC, we had decided that ports on the root hub
are just children of the USB controller node (rather than
grandchildren). Why does that not work?
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0>;
> +
> + port@1 {
Wouldn't this normally be 0 and 1. This should probably be usb-port
rather than port to avoid OF graph overlap.
> + reg = <1>;
> + usb-phy = <&usb2_phy1>, <&usb3_phy1>;
s/usb-phy/phys/
> + phy-names = "usb2-phy", "usb3-phy";
> + };
> +
> + port@2 {
> + reg = <2>;
> + usb-phy = <&usb2_phy2>, <&usb3_phy2>;
> + phy-names = "usb2-phy", "usb3-phy";
> + };
> + };
> + }
--
To unsubscribe from this list: send the line "unsubscribe 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 1/2] usb: host: add a generic platform USB roothub driver
Hi Rob,
On Fri, Jan 13, 2017 at 9:08 PM, Rob Herring wrote:
> On Wed, Jan 11, 2017 at 04:29:46PM +0100, Martin Blumenstingl wrote:
>> Many SoC platforms have separate devices for the USB PHY which are
>> registered through the generic PHY framework. These PHYs have to be
>> enabled to make the USB controller actually work. They also have to be
>> disabled again on shutdown/suspend.
>>
>> Currently (at least) the following HCI platform drivers are using custom
>> code to obtain all PHYs via devicetree for the roothub/controller and
>> disable/enable them when required:
>> - ehci-platform.c has ehci_platform_power_{on,off}
>> - xhci-mtk.c has xhci_mtk_phy_{init,exit,power_on,power_off}
>> - ohci-platform.c has ohci_platform_power_{on,off}
>>
>> These drivers are not using the generic devicetree USB device bindings
>> yet which were only introduced recently (documentation is available in
>> devicetree/bindings/usb/usb-device.txt).
>> With this new driver the usb2-phy and usb3-phy can be specified directly
>> in the child-node of the corresponding port of the roothub via
>> devicetree. This can be extended by not just parsing PHYs (some of the
>> other drivers listed above are for example also parsing a list of clocks
>> as well) when required.
>>
>> Signed-off-by: Martin Blumenstingl
>> ---
>> .../devicetree/bindings/usb/usb-roothub.txt| 41 ++
>> drivers/usb/host/Kconfig | 3 +
>> drivers/usb/host/Makefile | 2 +
>> drivers/usb/host/platform-roothub.c| 146
>> +
>> drivers/usb/host/platform-roothub.h| 14 ++
>> 5 files changed, 206 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/usb/usb-roothub.txt
>> create mode 100644 drivers/usb/host/platform-roothub.c
>> create mode 100644 drivers/usb/host/platform-roothub.h
>>
>> diff --git a/Documentation/devicetree/bindings/usb/usb-roothub.txt
>> b/Documentation/devicetree/bindings/usb/usb-roothub.txt
>> new file mode 100644
>> index ..96e152d3901c
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/usb-roothub.txt
>> @@ -0,0 +1,41 @@
>> +Generic USB root-hub Properties
>> +
>> +similar to the USB device bindings (documented in usb-device.txt from the
>> +current directory) this provides support for configuring the root-hub.
>> +
>> +Required properties:
>> +- compatible: should be at least one of "usb1d6b,3", "usb1d6b,2"
>> +- reg: must be 0.
>> +- address-cells: must be 1
>> +- size-cells: must be 0
>
>> +- a sub-node per port supports the following properties:
>
> Make this another section with required and optional sections.
I can do that, but let's wait for the results if we want the PHYs to
be specified at the grand-child or child level
>> + - reg: the port number on the root-hub (mandatory)
>> + - phys: optional, from the *Generic PHY* bindings (mandatory needed when
>> +phy-names is given)
>> + - phy-names: optional, from the *Generic PHY* bindings; supported names
>> +are "usb2-phy" or "usb3-phy"
>> +
>> +Example:
>> + &usb1 {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + roothub@0 {
>> + compatible = "usb1d6b,3", "usb1d6b,2";
>
> Is this discoverable? IIRC, we had decided that ports on the root hub
> are just children of the USB controller node (rather than
> grandchildren). Why does that not work?
if I understand you correctly you are thinking of something like this:
&usb1 {
...cells...
port@1 {
reg = <1>;
phys = <&phy2>
}
port@2 {
reg = <2>;
phys = <&phy2>
}
}
in that case we need a way to differentiate between "actual device at
port 1" and "configuration for root-hub port 1".
in that example I also cannot specify a compatible string since I
don't know which device might be plugged into that port.
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + reg = <0>;
>> +
>> + port@1 {
>
> Wouldn't this normally be 0 and 1. This should probably be usb-port
> rather than port to avoid OF graph overlap.
the USB subsystem starts counting at 1, there can never be a "device
with device-number 0" - so I think we should stay with 1 and 2 for a
2-port roothub.
I'm fine with changing the name to "usb-port" though
>> + reg = <1>;
>> + usb-phy = <&usb2_phy1>, <&usb3_phy1>;
>
> s/usb-phy/phys/
thanks for spotting this
>> + phy-names = "usb2-phy", "usb3-phy";
>> + };
>> +
>> + port@2 {
>> + reg = <2>;
>> + usb-phy = <&usb2_phy2>, <&usb3_phy2>;
>> + phy-names = "usb2-phy", "usb3-phy";
>> + };
>> + };
>> + }
--
To unsubscribe from this list:
Re: [PATCH 4/4] ARM: dts: sun8i: add OTG function to Lichee Pi Zero
On Thu, Jan 12, 2017 at 06:39:38PM +0100, Maxime Ripard wrote:
> Hi Bin,
>
> On Thu, Jan 12, 2017 at 08:50:14AM -0600, Bin Liu wrote:
> > On Wed, Jan 11, 2017 at 10:06:38PM +0100, Maxime Ripard wrote:
> > > On Wed, Jan 11, 2017 at 02:08:11PM -0600, Bin Liu wrote:
> > > > On Thu, Jan 12, 2017 at 03:55:33AM +0800, Icenowy Zheng wrote:
> > > > >
> > > > >
> > > > > 11.01.2017, 04:24, "Bin Liu" :
> > > > > > On Tue, Jan 03, 2017 at 11:25:34PM +0800, Icenowy Zheng wrote:
> > > > > >> Lichee Pi Zero features a USB OTG port.
> > > > > >>
> > > > > >> Add support for it.
> > > > > >>
> > > > > >> Note: in order to use the Host mode, the board must be powered
> > > > > >> via the
> > > > > >> +5V and GND pins.
> > > > > >>
> > > > > >> Signed-off-by: Icenowy Zheng
> > > > > >> ---
> > > > > >> arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts | 10 ++
> > > > > >> 1 file changed, 10 insertions(+)
> > > > > >>
> > > > > >> diff --git a/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> > > > > >> b/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> > > > > >> index 0099affc6ce3..3d9168cbaeca 100644
> > > > > >> --- a/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> > > > > >> +++ b/arch/arm/boot/dts/sun8i-v3s-licheepi-zero.dts
> > > > > >> @@ -71,3 +71,13 @@
> > > > > >> pinctrl-names = "default";
> > > > > >> status = "okay";
> > > > > >> };
> > > > > >> +
> > > > > >> +&usb_otg {
> > > > > >> + dr_mode = "otg";
> > > > > >
> > > > > > Why not set this default mode in dtsi instead?
> > > > > >
> > > > > > Regards,
> > > > > > -Bin.
> > > > >
> > > > > There's possibly boards which do not have OTG functions.
> > > >
> > > > That is board specific.
> > >
> > > Exactly, and this is why it should be done in the board DT.
> >
> > I am just suggesting based on the common practice. If a .dtsi exists for
> > a family, the .dtsi describes the device and common properties for all
> > possible boards, and each board .dts adds or overrides its specific
> > implementation. Kernel has many devices/boards done in this way - define
> > the default dr_mode in .dtsi.
> >
> > In this case, I suggest to set the common dr_mode in .dtsi, then each
> > board .dts only overrides it if the implementation is different.
> >
> > >
> > > The controller in the Allwinner SoCs do not handle directly the ID pin
> > > and VBUS, but rather rely on a GPIO to do so.
> > >
> > > So boards with OTG will need setup anyway, at least to tell which
> > > GPIOs are used. There's no point in enforcing a default if it doesn't
> > > work by default.
> >
> > Then define a default which supposes to work for most boards.
> >
> > Why I suggest this, is because defining a default dr_mode which works
> > for most cases in dtsi could prevent a little surprise in MUSB function.
> > If someone designs a new board but forgets to define dr_mode in the new
> > board DT, the MUSB driver will default to org mode, which might not be
> > intended.
>
> The point is that there is no sensible default. Some boards don't have
> an ID pin and no VBUS (peripheral), some don't have an ID pin but VBUS
> (host), and some have an ID pin but no controllable VBUS, some have an
> ID pin and a controllable VBUS, but we have no idea which GPIOs are
> used.
>
> There's no way we can have something that works on most cases.
Ok, understood.
Regards,
-Bin.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: VL805 USB 3.0 does not see connected devices (only on x86_64) (x86 is ok)
On Fri, 13 Jan 2017, c400 wrote: > kernel 4.9.3 > still not working > latest log: > > [ 196.984424] sd 7:0:0:0: [sdc] 31293440 512-byte logical blocks: > (16.0 GB/14.9 GiB) > [ 196.984564] sd 7:0:0:0: [sdc] Write Protect is off > [ 196.984566] sd 7:0:0:0: [sdc] Mode Sense: 23 00 00 00 > [ 196.984689] sd 7:0:0:0: [sdc] No Caching mode page found > [ 196.984690] sd 7:0:0:0: [sdc] Assuming drive cache: write through > [ 196.984696] device: '8:32': device_add > [ 196.984714] PM: Adding info for No Bus:8:32 > [ 196.984727] device: 'sdc': device_add > [ 196.984747] PM: Adding info for No Bus:sdc > [ 197.085658] sdc: sdc1 > [ 197.085664] device: 'sdc1': device_add > [ 197.085680] PM: Adding info for No Bus:sdc1 > [ 197.086929] sd 7:0:0:0: [sdc] Attached SCSI removable disk > [ 230.955006] FAT-fs (sdc1): Volume was not properly unmounted. Some > data may be corrupt. Please run fsck. > [ 234.422942] DMAR: DRHD: handling fault status reg 2 > [ 234.422946] DMAR: [DMA Read] Request device [02:00.0] fault addr > fffbb000 [fault reason 06] PTE Read access is not set > [ 234.422950] xhci_hcd :02:00.0: WARNING: Host System Error > [ 234.446078] xhci_hcd :02:00.0: Host not halted after 16000 > microseconds. > [ 270.447035] xhci_hcd :02:00.0: xHCI host not responding to stop > endpoint command. > [ 270.447040] xhci_hcd :02:00.0: Assuming host is dying, halting host. > [ 270.470143] xhci_hcd :02:00.0: Host not halted after 16000 > microseconds. > [ 270.470153] xhci_hcd :02:00.0: Non-responsive xHCI host is not halting. > [ 270.470154] xhci_hcd :02:00.0: Completing active URBs anyway. > [ 270.470161] xhci_hcd :02:00.0: HC died; cleaning up > [ 270.470265] sd 7:0:0:0: [sdc] tag#0 FAILED Result: > hostbyte=DID_TIME_OUT driverbyte=DRIVER_OK > [ 270.470268] sd 7:0:0:0: [sdc] tag#0 CDB: Read(10) 28 00 00 06 51 22 > 00 00 0e 00 > [ 270.470269] blk_update_request: I/O error, dev sdc, sector 413986 > [ 270.538994] sd 7:0:0:0: [sdc] tag#0 FAILED Result: > hostbyte=DID_ERROR driverbyte=DRIVER_OK > [ 270.538997] sd 7:0:0:0: [sdc] tag#0 CDB: Read(10) 28 00 00 06 51 23 > 00 00 0d 00 > [ 270.538998] blk_update_request: I/O error, dev sdc, sector 413987 > [ 270.986995] sd 7:0:0:0: [sdc] tag#0 FAILED Result: > hostbyte=DID_ERROR driverbyte=DRIVER_OK > [ 270.986997] sd 7:0:0:0: [sdc] tag#0 CDB: Read(10) 28 00 00 06 51 22 > 00 00 01 00 > [ 270.986999] blk_update_request: I/O error, dev sdc, sector 413986 > [ 270.987063] FAT-fs (sdc1): Directory bread(block 411938) failed > [ 271.307002] sd 7:0:0:0: [sdc] tag#0 FAILED Result: > hostbyte=DID_ERROR driverbyte=DRIVER_OK > [ 271.307004] sd 7:0:0:0: [sdc] tag#0 CDB: Read(10) 28 00 00 06 51 23 > 00 00 01 00 > [ 271.307005] blk_update_request: I/O error, dev sdc, sector 413987 > [ 271.307062] FAT-fs (sdc1): Directory bread(block 411939) failed > [ 271.307079] FAT-fs (sdc1): Directory bread(block 411940) failed Please post a usbmon trace starting from just before you plug in the USB drive and continuing until the error occurs. (Instructions for usbmon are in the kernel source file Documentation/usb/usbmon.txt.) Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv2] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
On 01/13/2017 01:57 PM, Grygorii Strashko wrote:
>
>
> On 01/13/2017 01:53 PM, Grygorii Strashko wrote:
>>
>>
>> On 01/13/2017 01:01 PM, Tony Lindgren wrote:
>>> * Grygorii Strashko [170113 10:37]:
On 01/13/2017 12:01 PM, Tony Lindgren wrote:
> Commit fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
> together with recent MUSB changes allowed USB and DMA on BeagleBone to
> idle
> when no cable is connected. But looks like few corner case issues still
> remain.
>
> Looks like just by re-plugging USB cable about ten or so times on
> BeagleBone
> when configured in USB peripheral mode we can get warnings and eventually
> trigger an oops in cppi41 DMA:
>
> WARNING: CPU: 0 PID: 14 at drivers/dma/cppi41.c:1154
> cppi41_runtime_suspend+
> x28/0x38 [cppi41]
> ...
>
> WARNING: CPU: 0 PID: 14 at drivers/dma/cppi41.c:452
> push_desc_queue+0x94/0x9c [cppi41]
> ...
>
> Unable to handle kernel NULL pointer dereference at virtual
> address 0104
> pgd = c0004000
> [0104] *pgd=
> Internal error: Oops: 805 [#1] SMP ARM
> ...
> [] (cppi41_runtime_resume [cppi41]) from []
> (__rpm_callback+0xc0/0x214)
> [] (__rpm_callback) from [] (rpm_callback+0x20/0x80)
> [] (rpm_callback) from [] (rpm_resume+0x504/0x78c)
> [] (rpm_resume) from [] (pm_runtime_work+0x60/0xa8)
> [] (pm_runtime_work) from []
> (process_one_work+0x2b4/0x808)
>
> This is because of a race with runtime PM and cppi41_dma_issue_pending()
> as reported by Alexandre Bailon in earlier
> set of patches. Based on mailing list discussions we however came to the
> conclusion that a different fix from Alexandre's fix is needed in order
> to guarantee that DMA is really active when we try to use it.
>
> To fix the issue, we need to add a driver specific flag as we otherwise
> can have -EINPROGRESS state set by runtime PM and can't rely on
> pm_runtime_active() to tell us when we can use the DMA.
>
> And we need to make sure the DMA transfers get triggered in the queued
> order. So let's always queue the transfers, then flush the queue
> from both cppi41_dma_issue_pending() and cppi41_runtime_resume()
> as suggested by Grygorii Strashko in an
> earlier example patch.
>
> For reference, this is also documented in
> Documentation/power/runtime_pm.txt
> in the example at the end of the file as pointed out by Grygorii Strashko
> .
>
> Based on earlier patches from Alexandre Bailon
> and Grygorii Strashko modified based on
> testing and what was discussed on the mailing lists.
>
> Fixes: fdea2d09b997 ("dmaengine: cppi41: Add basic PM runtime support")
> Cc: Andy Shevchenko
> Cc: Bin Liu
> Cc: Grygorii Strashko
> Cc: Kevin Hilman
> Cc: Patrick Titiano
> Cc: Sergei Shtylyov
> Reported-by: Alexandre Bailon
> Signed-off-by: Tony Lindgren
> ---
>
> Alexandre & Grygorii, can you guys please review and test?
I've just tested it on BBB. And it triggers warning from IRQ
root@am335x-evm:~# [ 242.280546] [ cut here ]
[ 242.285532] WARNING: CPU: 0 PID: 0 at drivers/dma/cppi41.c:330
cppi41_irq+0x228/0x260 [cppi41]
[ 242.335164] CPU: 0 PID: 0 Comm: swapper/0 Tainted: GW
4.10.0-rc3-00560-g51afc29 #133
[ 242.344601] Hardware name: Generic AM33XX (Flattened Device Tree)
[ 242.351084] [] (unwind_backtrace) from []
(show_stack+0x10/0x14)
[ 242.359269] [] (show_stack) from []
(dump_stack+0xac/0xe0)
[ 242.366919] [] (dump_stack) from []
(__warn+0xd8/0x104)
[ 242.374282] [] (__warn) from []
(warn_slowpath_null+0x20/0x28)
[ 242.382299] [] (warn_slowpath_null) from []
(cppi41_irq+0x228/0x260 [cppi41])
[ 242.391714] [] (cppi41_irq [cppi41]) from []
(__handle_irq_event_percpu+0x48/0x3b4)
[ 242.401727] [] (__handle_irq_event_percpu) from []
(handle_irq_event_percpu+0x1c/0x58)
[ 242.412017] [] (handle_irq_event_percpu) from []
(handle_irq_event+0x38/0x5c)
[ 242.421477] [] (handle_irq_event) from []
(handle_level_irq+0xc0/0x154)
[ 242.430378] [] (handle_level_irq) from []
(generic_handle_irq+0x20/0x34)
[ 242.439378] [] (generic_handle_irq) from []
(__handle_domain_irq+0x64/0xe0)
[ 242.448660] [] (__handle_domain_irq) from []
(__irq_svc+0x70/0x98)
[ 242.457117] [] (__irq_svc) from []
(arch_cpu_idle+0x20/0x3c)
[ 242.464935] [] (arch_cpu_idle) from []
(do_idle+0x164/0x218)
[ 242.472748] [] (do_idle) from []
(cpu_startup_entry+0x18/0x1c)
[ 242.480758] [] (cpu_startup_entry) from []
(start_kernel+0x344/0x3bc)
[ 242.489376] ---[ end trace 12c5b6488c1e8c75 ]---
[ 242.496525] usb 1-
Re: [PATCHv2] dmaengine: cppi41: Fix oops in cppi41_runtime_resume
* Grygorii Strashko [170113 13:37]:
> > Simplified diff with fix on top of your patch:
> >
> > diff --git a/drivers/dma/cppi41.c b/drivers/dma/cppi41.c
> > index ce37a1a..9e9403a 100644
> > --- a/drivers/dma/cppi41.c
> > +++ b/drivers/dma/cppi41.c
> > @@ -319,12 +319,6 @@ static irqreturn_t cppi41_irq(int irq, void *data)
> >
> > while (val) {
> > u32 desc, len;
> > - int error;
> > -
> > - error = pm_runtime_get(cdd->ddev.dev);
> > - if ((error != -EINPROGRESS) && (error < 0))
> > - dev_err(cdd->ddev.dev, "%s pm runtime get:
> > %i\n",
> > - __func__, error);
> >
> > /* This warning should never trigger */
> > WARN_ON(cdd->is_suspended);
> > @@ -500,7 +494,6 @@ static void cppi41_dma_issue_pending(struct dma_chan
> > *chan)
> > spin_unlock_irqrestore(&cdd->lock, flags);
> >
> > pm_runtime_mark_last_busy(cdd->ddev.dev);
> > - pm_runtime_put_autosuspend(cdd->ddev.dev);
> > }
> >
> > static u32 get_host_pd0(u32 length)
> >
>
> Ok. this is incorrect in case of USB hub and will just hide the problem
> by incrementing PM runtime usage counter every time USB host is connected :(
Yeah if anything changes in those two nested loops, the pm_runtime counts
get unbalanced.
> Once USB hub is connected the PM runtime usage counter will became 1 and stay
> 1 after disconnection. Which means that some descriptor was pushed in Q, but
> IRQ
> was not triggered.
>
> Don't know how to proceed as I'm not so familiar with musb :(
I'm now playing with saving the queue manager value and kicking the
PM runtime if we have transfers in progress. Looks like the dma
registers are zero while there are transfers in progress, or at least
I have not yet found any hardware registers that would tell that.
Regards,
Tony
--
To unsubscribe from this list: send the line "unsubscribe 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] tools: usb usbip - update README USB/IP driver location
Update USB/IP driver location in README. Signed-off-by: Shuah Khan --- tools/usb/usbip/README | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/usb/usbip/README b/tools/usb/usbip/README index 831f49f..f349ef4 100644 --- a/tools/usb/usbip/README +++ b/tools/usb/usbip/README @@ -7,7 +7,7 @@ [Requirements] - USB/IP device drivers - Found in the staging directory of the Linux kernel. + Found in the drivers/usb/usbip directory of the Linux kernel. - libudev >= 2.0 libudev library -- 2.7.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
Standard USB Descriptor question
Hello everybody I have been stuck for ages trying to figure out something in the Standard USB Descriptor during the enumeration. It looks like it should should be quite simple but I am not getting it because maybe some mind block, maybe becuase the documentation is unclear, or maybe because I'm brain-dead. Here is the problem: I think the host starts out requesting the device descriptor This is okay and works no problem. At some point the host requests the configuration descriptor and I get stuck. The usb 2.0 spec. says: "When the host requests the configuration descriptor, all related interface and endpoint descriptors are returned" How is the host supposed to know how much data is coming in or how much data to request until it reads wTotalLength? I would assume the host would request the 9 bytes of the configuration descriptor, then make subsequent requests to get the interfaces and endpoints but for instance in the section on the interface descriptor the spec says " An interface descriptor is always returned as part of a configuration descriptor. Again how is this supposed happen if the host is only requesting 9 bytes in the configuration descriptor stage Bruce -- To unsubscribe from this list: send the line "unsubscribe 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: Standard USB Descriptor question
On Fri, Jan 13, 2017 at 07:24:10PM -0800, bruce m beach wrote: > Hello everybody > > I have been stuck for ages trying to figure out something > in the Standard USB Descriptor during the enumeration. It > looks like it should should be quite simple but I am not > getting it because maybe some mind block, maybe becuase the > documentation is unclear, or maybe because I'm brain-dead. > Here is the problem: > > I think the host starts out requesting the device descriptor > This is okay and works no problem. At some point the host > requests the configuration descriptor and I get stuck. The > usb 2.0 spec. says: > > "When the host requests the configuration descriptor, all > related interface and endpoint descriptors are returned" > > How is the host supposed to know how much data is coming in > or how much data to request until it reads wTotalLength? > > I would assume the host would request the 9 bytes of the > configuration descriptor, then make subsequent requests to > get the interfaces and endpoints but for instance in the > section on the interface descriptor the spec says > > " An interface descriptor is always returned as part of a > configuration descriptor. > > Again how is this supposed happen if the host is only > requesting 9 bytes in the configuration descriptor stage Have you looked at the data on the wire? The host asks for 9 bytes, and then it uses the information there to get the "real" size of the descriptor and then it asks for the full descriptor with all of the information in it. hope this helps, 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
