Re: WARNING: CPU: 0 PID: 0 at ../drivers/usb/dwc2/gadget.c:300 dwc2_hsotg_init_fifo+0x34/0x1b4

2018-11-06 Thread Minas Harutyunyan
Hi Maynard,

On 11/7/2018 2:07 AM, Maynard CABIENTE wrote:
> Hi John,
> 
> Sorry re-sending as it was detected as spam before (due to HTML subpart).
> 
> I’m not certain if you are the correct person that maintains the 
> drivers/usb/dwc2 for Linux kernel 4.14. I noticed that 4.20 is a different 
> person.
> 
> We are using an Altera Cyclone V SoC FPGA on our board with linux kernel 
> 4.14.44 and enabling the USB gadget for HID (keyboard and mouse) and mass 
> storage (2 interfaces). The USB will always be configured for USB gadget (and 
> never host). At times, we are encountering the kernel warning as seen below. 
> It can happen a few times and it may cause our board to reboot. This mostly 
> happens when a USB reset occurs and the (HID and mass storage) gadget is in a 
> connected state.
> 
> Here is my understanding of the issue.
> 
> When in connected state and a USB reset has occurred, the irq handler 
> dwc2_hstog_irq() will report the disconnection to the individual gadget 
> functions (e.g. hid and mass storage). Each hid gadget function (keyboard and 
> mouse) will call hidg_disable() and will in turn disable the USB endpoints 
> and clear the respective bits in the FIFO map. Each mass storage function 
> will also call fsg_disable() and will in turn disable the USB endpoints and 
> clear the bits in the FIFO map. The function 
> dwc2_hsotg_core_init_disconnected() will be called afterwards and it assumes 
> that all the USB endpoints have been disabled already and FIFO map is zeroed 
> out.
> 
> What I notice here is that there is a race condition that can happen where 
> the mass storage gadget function can be delayed in disabling the USB 
> endpoints prior to the call to dwc2_hsotg_core_init_disconnected(), which 
> results to the kernel warning message:
> 
> [  135.400063] WARNING: CPU: 0 PID: 0 at ../drivers/usb/dwc2/gadget.c:300 
> dwc2_hsotg_init_fifo+0x34/0x1b4
> [  135.409327] Modules linked in: usb_f_mass_storage usb_f_hid libcomposite
> [  135.416020] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.14.44-kx4+ #8
> [  135.422433] Hardware name: Altera SOCFPGA
> [  135.426450] [] (unwind_backtrace) from [] 
> (show_stack+0x20/0x24)
> [  135.434170] [] (show_stack) from [] 
> (dump_stack+0xa4/0xd8)
> [  135.441375] [] (dump_stack) from [] (__warn+0xe0/0x110)
> [  135.448318] [] (__warn) from [] 
> (warn_slowpath_null+0x30/0x38)
> [  135.455866] [] (warn_slowpath_null) from [] 
> (dwc2_hsotg_init_fifo+0x34/0x1b4)
> [  135.464714] [] (dwc2_hsotg_init_fifo) from [] 
> (dwc2_hsotg_core_init_disconnected+0xb8/0x4ac)
> [  135.474856] [] (dwc2_hsotg_core_init_disconnected) from 
> [] (dwc2_hsotg_irq.part.4+0x100/0x9a4)
> [  135.485168] [] (dwc2_hsotg_irq.part.4) from [] 
> (dwc2_hsotg_irq+0x30/0x3c)
> [  135.493671] [] (dwc2_hsotg_irq) from [] 
> (__handle_irq_event_percpu+0xb0/0x2bc)
> [  135.502602] [] (__handle_irq_event_percpu) from [] 
> (handle_irq_event_percpu+0x2c/0x68)
> [  135.512223] [] (handle_irq_event_percpu) from [] 
> (handle_irq_event+0x6c/0x90)
> [  135.521066] [] (handle_irq_event) from [] 
> (handle_fasteoi_irq+0xb4/0x158)
> [  135.529562] [] (handle_fasteoi_irq) from [] 
> (generic_handle_irq+0x28/0x38)
> [  135.538145] [] (generic_handle_irq) from [] 
> (__handle_domain_irq+0x9c/0xc4)
> [  135.546815] [] (__handle_domain_irq) from [] 
> (gic_handle_irq+0x5c/0x88)
> [  135.555140] [] (gic_handle_irq) from [] 
> (__irq_svc+0x70/0x98)
> [  135.562591] Exception stack(0xc0c01f00 to 0xc0c01f48)
> 
> I am hoping that you will be able to help me determine a possible solution to 
> this kernel warning. I tried to put a delay to the call to 
> dwc2_hsotg_core_init_disconnected() in dwc2_hstog_irq() by using a workqueue 
> but there are other issues that popped up. I appreciate any help I can get to 
> resolve this issue.

This warn message popup due to on disconnect not all EP's are disabled 
and FIFO released in fifo_map. To solve this issue I submitted patches:
[PATCH v2] usb: dwc2: Disable all EP's on disconnect
and fix to above patch:
[PATCH] usb: dwc2: Fix ep disable spinlock flow.

Please apply these patches and test. Let me know if this fix your issue.

Thanks,
Minas


> 
> Regards,
> Maynard
> 
> 
> 
> 
> Ce message, ainsi que tous les fichiers joints à ce message, peuvent contenir 
> des informations sensibles et/ ou confidentielles ne devant pas être 
> divulguées. Si vous n'êtes pas le destinataire de ce message (ou que vous 
> recevez ce message par erreur), nous vous remercions de le notifier 
> immédiatement à son expéditeur, et de détruire ce message. Toute copie, 
> divulgation, modification, utilisation ou diffusion, non autorisée, directe 
> ou indirecte, de tout ou partie de ce message, est strictement interdite.
> 
> 
> This e-mail, and any document attached hereby, may contain confidential 
> and/or privileged information. If you are not the intended recipient (or have 
> received this e-mail in error) please notify the sender 

Re: [PATCH 1/3] usb: dwc3: Add reference clock properties

2018-11-06 Thread Felipe Balbi

Hi,

Thinh Nguyen  writes:
>> Thinh Nguyen  writes:
>>> Add two new device properties to program the reference clock period and
>>> to enable low power management using the reference clock. This allows a
>>> higher demand to go in low power for Audio Device Class devices. This
>>> feature is currently only valid for DWC_usb31 peripheral controller
>>> v1.80a and higher.
>>>
>>> Set "snps,refclk-period-ns" to program the reference clock period. The
>>> valid input periods are as follow:
>>>  +-+-+
>>>  | Period (ns) | Freq (MHz)  |
>>>  +-+-+
>>>  | 25  | 39.7/40 |
>>>  | 41  | 24.4|
>>>  | 50  | 20  |
>>>  | 52  | 19.2|
>>>  | 58  | 17.2|
>>>  | 62  | 16.1|
>>>  +-+-+
>>>
>>> Set "snps,refclk-lpm" to enable low power scheduling of isochronous
>>> transfers by running SOF/ITP counters using the reference clock. Both
>>> "snps,dis_u2_susphy_quirk" and "snps,dis_enblslpm_quirk" must not be
>>> set for this feature to be enabled.
>>>
>>> Signed-off-by: Thinh Nguyen 
>>> ---
>>>  Documentation/devicetree/bindings/usb/dwc3.txt | 18 ++
>>>  1 file changed, 18 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt 
>>> b/Documentation/devicetree/bindings/usb/dwc3.txt
>>> index 636630fb92d7..712b344c3a31 100644
>>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
>>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
>>> @@ -95,6 +95,24 @@ Optional properties:
>>> this and tx-thr-num-pkt-prd to a valid, non-zero value
>>> 1-16 (DWC_usb31 programming guide section 1.2.3) to
>>> enable periodic ESS TX threshold.
>>> + - snps,refclk-period-ns: set to program the reference clock period. The 
>>> valid
>>> +   input periods are as follow:
>>> +   +-+-+
>>> +   | Period (ns) | Freq (MHz)  |
>>> +   +-+-+
>>> +   | 25  | 39.7/40 |
>>> +   | 41  | 24.4|
>>> +   | 50  | 20  |
>>> +   | 52  | 19.2|
>>> +   | 58  | 17.2|
>>> +   | 62  | 16.1|
>>> +   +-+-+
>>> + - snps,enable-refclk-lpm: set to enable low power scheduling of 
>>> isochronous
>>> +   transfers by running SOF/ITP counters using the
>>> +   reference clock. Only valid for DWC_usb31 peripheral
>>> +   controller v1.80a and higher. Both
>>> +   "snps,dis_u2_susphy_quirk" and
>>> +   "snps,dis_enblslpm_quirk" must not be set.
>> sounds like you should rely on clk API here. Then on driver call
>> clk_get_rate() to computer whatever you need to compute.
>>
> There's nothing to compute here. We can simply enable this feature with
> "snps, enable-refclk-lpm" and the controller will use the default refclk
> settings.

Right, right. What I'm saying, though, is that we have a clock API for
describing a clock. So why wouldn't we rely on that API for this? I
think both of these new properties can be replaced with standard clock
API properties:

clocks = <>, ..., <_clk>
clock-names = "clock1", , "lpm";

Then dwc3 core could, simply, check if we have a clock named "lpm" and
if there is, use NSECS_PER_SEC / clk_get_rate() to get its period and
write it to the register that needs the information.

-- 
balbi


signature.asc
Description: PGP signature


Re: USB Bluetooth dongle stop response with timeout error

2018-11-06 Thread Morikazu Fumita

On 31/10/2018 4:49 PM, Oliver Neukum wrote:

On Mi, 2018-10-31 at 12:32 +0800, Morikazu Fumita wrote:

My test procedure is below (assuming Bluetooth devices are already paired).

1. Adding a network bridge for PAN using "brctl".
2. Link the bridge up.
3. Run "hciconfig hci0 up" to power the USB Bluetooth dongle up.
4. Register "nap" service and the network bridge to
"org.bluez.NetworkServer1"
via d-bus using a Python script.
5. Bluez makes "bnep0" virtual network interface automatically.
6. Connect to the PAN-NAP server from the client. Network is working
fine at
this point.
(Note: The hang does not happen even if turning promiscuous mode off at
this
point by running "ip link set bnep0 promisc off")
7. Disconnect PAN from the client or make the "bnep0" virtual network
adapter
down by running "ip link set bnep0 down".
8. The hang happens with "Bluetooth: hci0: command 0x tx timeout"
errors.
9. No response from the USB Bluetooth dongle anymore.
For example, running "discoverable on" from "bluetoothctl" fails with error
message of "Failed to set discoverable on: org.bluez.Error.Failed".
The timeout error is logged in "dmesg" as well.

   From this fact, I believe this issue is related to Bluez but not to USB.
What do you think?

Hi,

you are using a slightly less common HC, are you not?

Anyway, it seems to me that you can narrow this down a bit further.
You are using bridging. You can try to remove that.

And after that you can take an usbmon trace right as you give the
command that crashes the device. The procedure is described in the
kernel's Documentation directory. That will allow to determine
which exact command is the problem.

HTH
Oliver


Hello Oliver,

I got rid of the network bridge but the timeout error still happens so I 
can rule out the bridge now.


I also got USB packet dump and found that the error is happening 
regardless of HCI commands.


One example is below. I just inserted the USB dongle when I got the 
following errors.

[  145.046503] Bluetooth: hci0: command 0x1002 tx timeout
[  147.086503] Bluetooth: hci0: command 0x0c52 tx timeout
[  149.121499] Bluetooth: hci0: command 0x0c45 tx timeout
[  151.166503] Bluetooth: hci0: command 0x0c58 tx timeout

Please find the USB packet dump attached.
In frame no. 161, the host sent HCI command 0x1002 (Read Local Supported 
Commands).
Then the USB dongle tried to respond to that from frame no. 163 but it 
did not sent all fragment packets. It seems to stop response in the middle.

Finally, above "0x1002 tx timeout" happened.

As for frame no. 165, the host sent 0x0c52 (Write Extended Inquiry 
Response) but there was no response to it then above "0x0c52 tx timeout" 
happened.


This is just one example. The USB dongle can be successfully inserted 
and working for a while but suddenly stops response to HCI commands like 
this example.

It seems there is no specific HCI commands to cause the problem.

I do not find out what is the trigger of it yet.
Do you have any thoughts from this point?

Here is log from inserting the USB dongle.
[  141.811511] usb 1-1: new full-speed USB device number 2 using dwc2
[  142.001509] usb 1-1: device descriptor read/64, error -71
[  142.373525] usb 1-1: New USB device found, idVendor=0a12, 
idProduct=0001, bcdDevice=88.91
[  142.377642] usb 1-1: New USB device strings: Mfr=0, Product=2, 
SerialNumber=0

[  142.381204] usb 1-1: Product: CSR8510 A10
[  142.385718] device: '1-1': device_add
[  142.385829] bus: 'usb': add device 1-1
[  142.385866] PM: Adding info for usb:1-1
[  142.386066] bus: 'usb': driver_probe_device: matched device 1-1 with 
driver usb

[  142.386081] bus: 'usb': really_probe: probing driver usb with device 1-1
[  142.386101] usb 1-1: no default pinctrl state
[  142.386128] devices_kset: Moving 1-1 to end of list
[  142.392785] device: '1-1:1.0': device_add
[  142.392837] bus: 'usb': add device 1-1:1.0
[  142.392860] PM: Adding info for usb:1-1:1.0
[  142.392997] device: 'ep_81': device_add
[  142.393068] PM: Adding info for No Bus:ep_81
[  142.393087] device: 'ep_02': device_add
[  142.393138] PM: Adding info for No Bus:ep_02
[  142.393153] device: 'ep_82': device_add
[  142.393202] PM: Adding info for No Bus:ep_82
[  142.393212] device: '1-1:1.1': device_add
[  142.393259] bus: 'usb': add device 1-1:1.1
[  142.393280] PM: Adding info for usb:1-1:1.1
[  142.393380] device: 'ep_03': device_add
[  142.393434] PM: Adding info for No Bus:ep_03
[  142.393448] device: 'ep_83': device_add
[  142.393495] PM: Adding info for No Bus:ep_83
[  142.393512] driver: 'usb': driver_bound: bound to device '1-1'
[  142.393577] bus: 'usb': really_probe: bound device 1-1 to driver usb
[  142.393630] device: 'ep_00': device_add
[  142.393694] PM: Adding info for No Bus:ep_00
[  142.724485] Bluetooth: Core ver 2.22
[  142.726310] device class 'bluetooth': registering
[  142.726384] NET: Registered protocol family 31
[  142.729838] Bluetooth: HCI device and connection manager initialized
[  

Re: [PATCH 1/3] usb: dwc3: Add reference clock properties

2018-11-06 Thread Thinh Nguyen
Hi Felipe,

On 11/6/2018 3:26 AM, Felipe Balbi wrote:
> hi,
>
> Thinh Nguyen  writes:
>> Add two new device properties to program the reference clock period and
>> to enable low power management using the reference clock. This allows a
>> higher demand to go in low power for Audio Device Class devices. This
>> feature is currently only valid for DWC_usb31 peripheral controller
>> v1.80a and higher.
>>
>> Set "snps,refclk-period-ns" to program the reference clock period. The
>> valid input periods are as follow:
>>  +-+-+
>>  | Period (ns) | Freq (MHz)  |
>>  +-+-+
>>  | 25  | 39.7/40 |
>>  | 41  | 24.4|
>>  | 50  | 20  |
>>  | 52  | 19.2|
>>  | 58  | 17.2|
>>  | 62  | 16.1|
>>  +-+-+
>>
>> Set "snps,refclk-lpm" to enable low power scheduling of isochronous
>> transfers by running SOF/ITP counters using the reference clock. Both
>> "snps,dis_u2_susphy_quirk" and "snps,dis_enblslpm_quirk" must not be
>> set for this feature to be enabled.
>>
>> Signed-off-by: Thinh Nguyen 
>> ---
>>  Documentation/devicetree/bindings/usb/dwc3.txt | 18 ++
>>  1 file changed, 18 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt 
>> b/Documentation/devicetree/bindings/usb/dwc3.txt
>> index 636630fb92d7..712b344c3a31 100644
>> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
>> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
>> @@ -95,6 +95,24 @@ Optional properties:
>>  this and tx-thr-num-pkt-prd to a valid, non-zero value
>>  1-16 (DWC_usb31 programming guide section 1.2.3) to
>>  enable periodic ESS TX threshold.
>> + - snps,refclk-period-ns: set to program the reference clock period. The 
>> valid
>> +input periods are as follow:
>> ++-+-+
>> +| Period (ns) | Freq (MHz)  |
>> ++-+-+
>> +| 25  | 39.7/40 |
>> +| 41  | 24.4|
>> +| 50  | 20  |
>> +| 52  | 19.2|
>> +| 58  | 17.2|
>> +| 62  | 16.1|
>> ++-+-+
>> + - snps,enable-refclk-lpm: set to enable low power scheduling of isochronous
>> +transfers by running SOF/ITP counters using the
>> +reference clock. Only valid for DWC_usb31 peripheral
>> +controller v1.80a and higher. Both
>> +"snps,dis_u2_susphy_quirk" and
>> +"snps,dis_enblslpm_quirk" must not be set.
> sounds like you should rely on clk API here. Then on driver call
> clk_get_rate() to computer whatever you need to compute.
>
There's nothing to compute here. We can simply enable this feature with
"snps, enable-refclk-lpm" and the controller will use the default refclk
settings.

Thinh



Re: [PATCH] usb: dwc3: debugfs: Dump internal states

2018-11-06 Thread Thinh Nguyen
Hi Felipe,

On 11/5/2018 11:35 PM, Felipe Balbi wrote:
> Hi,
>
> Thinh Nguyen  writes:
 +static int dwc3_internal_states_show(struct seq_file *s, void *unused)
 +{
 +  struct dwc3 *dwc = s->private;
 +  unsigned intcurrent_mode;
 +  unsigned long   flags;
 +  u32 reg;
 +
 +  spin_lock_irqsave(>lock, flags);
 +  reg = dwc3_readl(dwc->regs, DWC3_GSTS);
 +  current_mode = DWC3_GSTS_CURMOD(reg);
 +
 +  reg = dwc3_readl(dwc->regs, DWC3_GDBGBMU);
 +  spin_unlock_irqrestore(>lock, flags);
 +
 +  seq_printf(s, "GDBGBMU = 0x%08x\n", reg);
>>> shouldn't the print be done with locks held?
>> I think the lock for the prints should be held at a higher level.
>> Otherwise, I don't think it will make a difference using dwc->lock.
> what happens if this gets preempted when you release the lock and
> a different thread writes to internal_states and reads the result before
> $this thread?

I see.

>
 +static ssize_t dwc3_internal_states_write(struct file *file,
 +  const char __user *ubuf, size_t count, loff_t *ppos)
>>> why is this necessary? Seems like it would be nicer to create a
>>> directory structure if the current operating mode is host so that we
>>> don't need to write anything.
>> Can you clarify more about the directory structure? Actually, I was
>> wondering what's the best way to do this also. The reason I want to
>> write to it is because the LSP selection for host is quite large (2^14).
> right, turn each of those into a directory of its own. If you don't want
> to or can't disclose proper names for those directories, just call them
> lsp0, lsp1, lsp2, and so on. Then a read of the file under lsp1
> directory would write and read the correct registers.
>
> Everything remains read-only.

This means that there will be 16384 + 256 files create for host. It also
means that we need to kmalloc at least (16384 + 256) * (size of each
selection) so that each file knows what to print. On top of that, when
we change mode of operation (e.g. device->host), then we need to
create/destroy all these files. Is this the way to do it?

Thanks,

Thinh



WARNING: CPU: 0 PID: 0 at ../drivers/usb/dwc2/gadget.c:300 dwc2_hsotg_init_fifo+0x34/0x1b4

2018-11-06 Thread Maynard CABIENTE
Hi John,

Sorry re-sending as it was detected as spam before (due to HTML subpart).

I’m not certain if you are the correct person that maintains the 
drivers/usb/dwc2 for Linux kernel 4.14. I noticed that 4.20 is a different 
person.

We are using an Altera Cyclone V SoC FPGA on our board with linux kernel 
4.14.44 and enabling the USB gadget for HID (keyboard and mouse) and mass 
storage (2 interfaces). The USB will always be configured for USB gadget (and 
never host). At times, we are encountering the kernel warning as seen below. It 
can happen a few times and it may cause our board to reboot. This mostly 
happens when a USB reset occurs and the (HID and mass storage) gadget is in a 
connected state.

Here is my understanding of the issue.

When in connected state and a USB reset has occurred, the irq handler 
dwc2_hstog_irq() will report the disconnection to the individual gadget 
functions (e.g. hid and mass storage). Each hid gadget function (keyboard and 
mouse) will call hidg_disable() and will in turn disable the USB endpoints and 
clear the respective bits in the FIFO map. Each mass storage function will also 
call fsg_disable() and will in turn disable the USB endpoints and clear the 
bits in the FIFO map. The function dwc2_hsotg_core_init_disconnected() will be 
called afterwards and it assumes that all the USB endpoints have been disabled 
already and FIFO map is zeroed out.

What I notice here is that there is a race condition that can happen where the 
mass storage gadget function can be delayed in disabling the USB endpoints 
prior to the call to dwc2_hsotg_core_init_disconnected(), which results to the 
kernel warning message:

[  135.400063] WARNING: CPU: 0 PID: 0 at ../drivers/usb/dwc2/gadget.c:300 
dwc2_hsotg_init_fifo+0x34/0x1b4
[  135.409327] Modules linked in: usb_f_mass_storage usb_f_hid libcomposite
[  135.416020] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.14.44-kx4+ #8
[  135.422433] Hardware name: Altera SOCFPGA
[  135.426450] [] (unwind_backtrace) from [] 
(show_stack+0x20/0x24)
[  135.434170] [] (show_stack) from [] 
(dump_stack+0xa4/0xd8)
[  135.441375] [] (dump_stack) from [] (__warn+0xe0/0x110)
[  135.448318] [] (__warn) from [] 
(warn_slowpath_null+0x30/0x38)
[  135.455866] [] (warn_slowpath_null) from [] 
(dwc2_hsotg_init_fifo+0x34/0x1b4)
[  135.464714] [] (dwc2_hsotg_init_fifo) from [] 
(dwc2_hsotg_core_init_disconnected+0xb8/0x4ac)
[  135.474856] [] (dwc2_hsotg_core_init_disconnected) from 
[] (dwc2_hsotg_irq.part.4+0x100/0x9a4)
[  135.485168] [] (dwc2_hsotg_irq.part.4) from [] 
(dwc2_hsotg_irq+0x30/0x3c)
[  135.493671] [] (dwc2_hsotg_irq) from [] 
(__handle_irq_event_percpu+0xb0/0x2bc)
[  135.502602] [] (__handle_irq_event_percpu) from [] 
(handle_irq_event_percpu+0x2c/0x68)
[  135.512223] [] (handle_irq_event_percpu) from [] 
(handle_irq_event+0x6c/0x90)
[  135.521066] [] (handle_irq_event) from [] 
(handle_fasteoi_irq+0xb4/0x158)
[  135.529562] [] (handle_fasteoi_irq) from [] 
(generic_handle_irq+0x28/0x38)
[  135.538145] [] (generic_handle_irq) from [] 
(__handle_domain_irq+0x9c/0xc4)
[  135.546815] [] (__handle_domain_irq) from [] 
(gic_handle_irq+0x5c/0x88)
[  135.555140] [] (gic_handle_irq) from [] 
(__irq_svc+0x70/0x98)
[  135.562591] Exception stack(0xc0c01f00 to 0xc0c01f48)

I am hoping that you will be able to help me determine a possible solution to 
this kernel warning. I tried to put a delay to the call to 
dwc2_hsotg_core_init_disconnected() in dwc2_hstog_irq() by using a workqueue 
but there are other issues that popped up. I appreciate any help I can get to 
resolve this issue.

Regards,
Maynard




Ce message, ainsi que tous les fichiers joints à ce message, peuvent contenir 
des informations sensibles et/ ou confidentielles ne devant pas être 
divulguées. Si vous n'êtes pas le destinataire de ce message (ou que vous 
recevez ce message par erreur), nous vous remercions de le notifier 
immédiatement à son expéditeur, et de détruire ce message. Toute copie, 
divulgation, modification, utilisation ou diffusion, non autorisée, directe ou 
indirecte, de tout ou partie de ce message, est strictement interdite.


This e-mail, and any document attached hereby, may contain confidential and/or 
privileged information. If you are not the intended recipient (or have received 
this e-mail in error) please notify the sender immediately and destroy this 
e-mail. Any unauthorized, direct or indirect, copying, disclosure, distribution 
or other use of the material or parts thereof is strictly forbidden.


[PATCH] usb: gadget: f_fs: Allow scatter-gather buffers

2018-11-06 Thread Andrzej Pietrasiewicz
Some protocols implemented in userspace with FunctionFS might require large
buffers, e.g. 64kB or more. Currently the said memory is allocated with
kmalloc, which might fail should system memory be highly fragmented.

On the other hand, some UDC hardware allows scatter-gather operation and
this patch takes advantage of this capability: if the requested buffer
is larger than PAGE_SIZE and the UDC allows scatter-gather operation, then
the buffer is allocated with vmalloc and a scatterlist describing it is
created and passed to usb request.

Signed-off-by: Andrzej Pietrasiewicz 
---

@Felipe: This time I used your correct address.

 drivers/usb/gadget/function/f_fs.c | 92 +++---
 1 file changed, 87 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c 
b/drivers/usb/gadget/function/f_fs.c
index 3ada83d..e83fa63 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -219,6 +220,8 @@ struct ffs_io_data {
 
struct usb_ep *ep;
struct usb_request *req;
+   struct sg_table sgt;
+   bool use_sg;
 
struct ffs_data *ffs;
 };
@@ -750,6 +753,70 @@ static ssize_t ffs_copy_to_iter(void *data, int data_len, 
struct iov_iter *iter)
return ret;
 }
 
+/*
+ * allocate a virtually contiguous buffer and create a scatterlist describing 
it
+ * @sg_table   - pointer to a place to be filled with sg_table contents
+ * @size   - required buffer size
+ */
+static void *ffs_build_sg_list(struct sg_table *sg_table, size_t size)
+{
+   struct page **pages;
+   void *vaddr;
+   unsigned long ptr;
+   unsigned int n_pages;
+   int i;
+
+   vaddr = vmalloc(size);
+   if (!vaddr)
+   return NULL;
+
+   n_pages =  (PAGE_ALIGN((unsigned long)vaddr + size) -
+   ((unsigned long)vaddr & PAGE_MASK))
+   >> PAGE_SHIFT;
+   pages = kvmalloc_array(n_pages, sizeof(struct page *), GFP_KERNEL);
+   if (!pages) {
+   vfree(vaddr);
+
+   return NULL;
+   }
+   for (i = 0, ptr = (unsigned long)vaddr & PAGE_MASK; i < n_pages;
+   ++i, ptr += PAGE_SIZE)
+   pages[i] = vmalloc_to_page((void *)ptr);
+
+   if (sg_alloc_table_from_pages(sg_table, pages, n_pages,
+   ((unsigned long)vaddr) & ~PAGE_MASK, size, GFP_KERNEL)) {
+   kvfree(pages);
+   vfree(vaddr);
+
+   return NULL;
+   }
+   kvfree(pages);
+
+   return vaddr;
+}
+
+static inline void *ffs_alloc_buffer(struct ffs_io_data *io_data,
+   size_t data_len)
+{
+   if (io_data->use_sg)
+   return ffs_build_sg_list(_data->sgt, data_len);
+
+   return kmalloc(data_len, GFP_KERNEL);
+}
+
+static inline void ffs_free_buffer(struct ffs_io_data *io_data)
+{
+   if (!io_data->buf)
+   return;
+
+   if (io_data->use_sg) {
+   sg_free_table(_data->sgt);
+   vfree(io_data->buf);
+   } else {
+   kfree(io_data->buf);
+   }
+}
+
 static void ffs_user_copy_worker(struct work_struct *work)
 {
struct ffs_io_data *io_data = container_of(work, struct ffs_io_data,
@@ -777,7 +844,7 @@ static void ffs_user_copy_worker(struct work_struct *work)
 
if (io_data->read)
kfree(io_data->to_free);
-   kfree(io_data->buf);
+   ffs_free_buffer(io_data);
kfree(io_data);
 }
 
@@ -933,6 +1000,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct 
ffs_io_data *io_data)
 * earlier
 */
gadget = epfile->ffs->gadget;
+   io_data->use_sg = gadget->sg_supported && data_len > PAGE_SIZE;
 
spin_lock_irq(>ffs->eps_lock);
/* In the meantime, endpoint got disabled or changed. */
@@ -949,7 +1017,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct 
ffs_io_data *io_data)
data_len = usb_ep_align_maybe(gadget, ep->ep, data_len);
spin_unlock_irq(>ffs->eps_lock);
 
-   data = kmalloc(data_len, GFP_KERNEL);
+   data = ffs_alloc_buffer(io_data, data_len);
if (unlikely(!data)) {
ret = -ENOMEM;
goto error_mutex;
@@ -989,9 +1057,17 @@ static ssize_t ffs_epfile_io(struct file *file, struct 
ffs_io_data *io_data)
bool interrupted = false;
 
req = ep->req;
-   req->buf  = data;
+   if (io_data->use_sg) {
+   req->buf  = NULL;
+   req->sg = io_data->sgt.sgl;
+   req->num_sgs = io_data->sgt.nents;
+   } else {
+   req->buf  = data;
+   }
req->length   = data_len;
 
+   io_data->buf = 

[PATCH] usb: gadget: f_fs: Allow scatter-gather buffers

2018-11-06 Thread Andrzej Pietrasiewicz
Some protocols implemented in userspace with FunctionFS might require large
buffers, e.g. 64kB or more. Currently the said memory is allocated with
kmalloc, which might fail should system memory be highly fragmented.

On the other hand, some UDC hardware allows scatter-gather operation and
this patch takes advantage of this capability: if the requested buffer
is larger than PAGE_SIZE and the UDC allows scatter-gather operation, then
the buffer is allocated with vmalloc and a scatterlist describing it is
created and passed to usb request.

Signed-off-by: Andrzej Pietrasiewicz 
---
 drivers/usb/gadget/function/f_fs.c | 92 +++---
 1 file changed, 87 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/gadget/function/f_fs.c 
b/drivers/usb/gadget/function/f_fs.c
index 3ada83d..e83fa63 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -219,6 +220,8 @@ struct ffs_io_data {
 
struct usb_ep *ep;
struct usb_request *req;
+   struct sg_table sgt;
+   bool use_sg;
 
struct ffs_data *ffs;
 };
@@ -750,6 +753,70 @@ static ssize_t ffs_copy_to_iter(void *data, int data_len, 
struct iov_iter *iter)
return ret;
 }
 
+/*
+ * allocate a virtually contiguous buffer and create a scatterlist describing 
it
+ * @sg_table   - pointer to a place to be filled with sg_table contents
+ * @size   - required buffer size
+ */
+static void *ffs_build_sg_list(struct sg_table *sg_table, size_t size)
+{
+   struct page **pages;
+   void *vaddr;
+   unsigned long ptr;
+   unsigned int n_pages;
+   int i;
+
+   vaddr = vmalloc(size);
+   if (!vaddr)
+   return NULL;
+
+   n_pages =  (PAGE_ALIGN((unsigned long)vaddr + size) -
+   ((unsigned long)vaddr & PAGE_MASK))
+   >> PAGE_SHIFT;
+   pages = kvmalloc_array(n_pages, sizeof(struct page *), GFP_KERNEL);
+   if (!pages) {
+   vfree(vaddr);
+
+   return NULL;
+   }
+   for (i = 0, ptr = (unsigned long)vaddr & PAGE_MASK; i < n_pages;
+   ++i, ptr += PAGE_SIZE)
+   pages[i] = vmalloc_to_page((void *)ptr);
+
+   if (sg_alloc_table_from_pages(sg_table, pages, n_pages,
+   ((unsigned long)vaddr) & ~PAGE_MASK, size, GFP_KERNEL)) {
+   kvfree(pages);
+   vfree(vaddr);
+
+   return NULL;
+   }
+   kvfree(pages);
+
+   return vaddr;
+}
+
+static inline void *ffs_alloc_buffer(struct ffs_io_data *io_data,
+   size_t data_len)
+{
+   if (io_data->use_sg)
+   return ffs_build_sg_list(_data->sgt, data_len);
+
+   return kmalloc(data_len, GFP_KERNEL);
+}
+
+static inline void ffs_free_buffer(struct ffs_io_data *io_data)
+{
+   if (!io_data->buf)
+   return;
+
+   if (io_data->use_sg) {
+   sg_free_table(_data->sgt);
+   vfree(io_data->buf);
+   } else {
+   kfree(io_data->buf);
+   }
+}
+
 static void ffs_user_copy_worker(struct work_struct *work)
 {
struct ffs_io_data *io_data = container_of(work, struct ffs_io_data,
@@ -777,7 +844,7 @@ static void ffs_user_copy_worker(struct work_struct *work)
 
if (io_data->read)
kfree(io_data->to_free);
-   kfree(io_data->buf);
+   ffs_free_buffer(io_data);
kfree(io_data);
 }
 
@@ -933,6 +1000,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct 
ffs_io_data *io_data)
 * earlier
 */
gadget = epfile->ffs->gadget;
+   io_data->use_sg = gadget->sg_supported && data_len > PAGE_SIZE;
 
spin_lock_irq(>ffs->eps_lock);
/* In the meantime, endpoint got disabled or changed. */
@@ -949,7 +1017,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct 
ffs_io_data *io_data)
data_len = usb_ep_align_maybe(gadget, ep->ep, data_len);
spin_unlock_irq(>ffs->eps_lock);
 
-   data = kmalloc(data_len, GFP_KERNEL);
+   data = ffs_alloc_buffer(io_data, data_len);
if (unlikely(!data)) {
ret = -ENOMEM;
goto error_mutex;
@@ -989,9 +1057,17 @@ static ssize_t ffs_epfile_io(struct file *file, struct 
ffs_io_data *io_data)
bool interrupted = false;
 
req = ep->req;
-   req->buf  = data;
+   if (io_data->use_sg) {
+   req->buf  = NULL;
+   req->sg = io_data->sgt.sgl;
+   req->num_sgs = io_data->sgt.nents;
+   } else {
+   req->buf  = data;
+   }
req->length   = data_len;
 
+   io_data->buf = data;
+
req->context  = 
 

Re: [PATCH 1/3] usb: dwc3: Add reference clock properties

2018-11-06 Thread Felipe Balbi

hi,

Thinh Nguyen  writes:
> Add two new device properties to program the reference clock period and
> to enable low power management using the reference clock. This allows a
> higher demand to go in low power for Audio Device Class devices. This
> feature is currently only valid for DWC_usb31 peripheral controller
> v1.80a and higher.
>
> Set "snps,refclk-period-ns" to program the reference clock period. The
> valid input periods are as follow:
>  +-+-+
>  | Period (ns) | Freq (MHz)  |
>  +-+-+
>  | 25  | 39.7/40 |
>  | 41  | 24.4|
>  | 50  | 20  |
>  | 52  | 19.2|
>  | 58  | 17.2|
>  | 62  | 16.1|
>  +-+-+
>
> Set "snps,refclk-lpm" to enable low power scheduling of isochronous
> transfers by running SOF/ITP counters using the reference clock. Both
> "snps,dis_u2_susphy_quirk" and "snps,dis_enblslpm_quirk" must not be
> set for this feature to be enabled.
>
> Signed-off-by: Thinh Nguyen 
> ---
>  Documentation/devicetree/bindings/usb/dwc3.txt | 18 ++
>  1 file changed, 18 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt 
> b/Documentation/devicetree/bindings/usb/dwc3.txt
> index 636630fb92d7..712b344c3a31 100644
> --- a/Documentation/devicetree/bindings/usb/dwc3.txt
> +++ b/Documentation/devicetree/bindings/usb/dwc3.txt
> @@ -95,6 +95,24 @@ Optional properties:
>   this and tx-thr-num-pkt-prd to a valid, non-zero value
>   1-16 (DWC_usb31 programming guide section 1.2.3) to
>   enable periodic ESS TX threshold.
> + - snps,refclk-period-ns: set to program the reference clock period. The 
> valid
> + input periods are as follow:
> + +-+-+
> + | Period (ns) | Freq (MHz)  |
> + +-+-+
> + | 25  | 39.7/40 |
> + | 41  | 24.4|
> + | 50  | 20  |
> + | 52  | 19.2|
> + | 58  | 17.2|
> + | 62  | 16.1|
> + +-+-+
> + - snps,enable-refclk-lpm: set to enable low power scheduling of isochronous
> + transfers by running SOF/ITP counters using the
> + reference clock. Only valid for DWC_usb31 peripheral
> + controller v1.80a and higher. Both
> + "snps,dis_u2_susphy_quirk" and
> + "snps,dis_enblslpm_quirk" must not be set.

sounds like you should rely on clk API here. Then on driver call
clk_get_rate() to computer whatever you need to compute.

-- 
balbi


signature.asc
Description: PGP signature


Re: [PATCH 2/3] spi: add FTDI MPSSE SPI controller driver

2018-11-06 Thread Anatolij Gustschin
On Tue, 6 Nov 2018 13:07:13 +0800
kbuild test robot l...@intel.com wrote:
...
>from drivers/spi/spi-ftdi-mpsse.c:11:
>>> arch/x86/include/uapi/asm/ptrace-abi.h:20:13: error: expected identifier 
>>> before numeric constant  
>#define CS  13
>^
>>> drivers/spi/spi-ftdi-mpsse.c:27:2: note: in expansion of macro 'CS'  
> CS = BIT(3),
> ^~

I didn't see this while my build-testing, sorry. Will fix it in next
patch version.

Thanks,
Anatolij