There may be different reasons for the transfer to be cancelled. Don't
blindly free the command without checking its status. We may still need
to properly respond to cancelled command. Check and only free the
command on endpoint disable.
Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Link: https://lore.kernel.org/r/6c4ae2b4c2a9037bdcb6f909e173a94b11f04657.1733876548.git.Thinh.Nguyen@synopsys.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Don't just wait for the data write completion and execute the target
command. We need to verify if the request completed successfully and not
just sending invalid data. The verification is done in the write request
completion routine. Queue the same work of the command to execute the
target_execute_cmd() on data write.
Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Link: https://lore.kernel.org/r/9f6b1c6946cf49eeba0173e405678b9b7786636b.1733876548.git.Thinh.Nguyen@synopsys.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To properly respond to host sending more commands than the number of
streams the device advertises, the device needs to be able to reject the
command with a response. Allocate an extra request to handle 1 more
command than the number of streams.
Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Link: https://lore.kernel.org/r/256f2ec8f5e042ab692d9593144fa75f3d3ce94b.1733876548.git.Thinh.Nguyen@synopsys.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Resubmit command on completion to fetch more commands and service them
in parallel. Increase the number of work in a queue. Each work will be
for each command allowing them to be processed concurrently. Also, set
them to be unbounded by cpu to improve performance.
Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Link: https://lore.kernel.org/r/933cf7191b672bf4cfbea4df19af1b08dc1baca9.1733876548.git.Thinh.Nguyen@synopsys.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
We can handle multiple commands concurently. Each command services a
stream id. At the moment, the driver will handle 32 outstanding streams,
which is equivalent to 32 commands. Make sure to allocate a matching
number of commands to the number of streams.
Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Link: https://lore.kernel.org/r/2d806120dcc10c88fef21865b7bc1d2b6604fe42.1733876548.git.Thinh.Nguyen@synopsys.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Host can assign stream ID value greater than number of streams
allocated. The tcm function needs to keep track of which stream is
available to assign the stream ID. This patch doesn't track that, but at
least it makes sure that there's no Oops if the host send tag with a
value greater than the number of supported streams.
Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Link: https://lore.kernel.org/r/d57b7dfd228199cef811206b1b83ec649f742752.1733876548.git.Thinh.Nguyen@synopsys.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Only allocate up to UASP_SS_EP_COMP_NUM_STREAMS number of session tags.
We should not be using more than UASP_SS_EP_COMP_NUM_STREAMS of tags due
to the number of commands limit we imposed. Each command uses a unique
tag. Any more than that is unnecessary. By limiting it, we can detect an
issue in our driver immediately should we run out of session tags.
Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Link: https://lore.kernel.org/r/017016ffcab2f3c284d863fc42483b83dbd21b35.1733876548.git.Thinh.Nguyen@synopsys.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Currently the default bMaxBurst is 0. Set default bMaxBurst to 15 (i.e.
16 bursts) to Data IN and OUT endpoints to improve performance. It
should be fine for a controller that supports less than 16 bursts. It
should be able to negotiate properly with the host at packet level for
the end of burst.
If the controller can't handle a burst of 16, and high performance isn't
important, the user can use BOT protocol from mass_storage gadget driver
instead.
Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Link: https://lore.kernel.org/r/9d6265db4d138e542f281988362bc4392b034d43.1733876548.git.Thinh.Nguyen@synopsys.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Some old builds of Microsoft Windows 10 UASP class driver reject UASP
device with stream count of 2^4. To keep compatibility with both Linux
and Windows, let's increase the stream count to 2^5. Also, internal
tests show that stream count of 2^5 increases performance slightly.
Signed-off-by: Thinh Nguyen <Thinh.Nguyen@synopsys.com>
Link: https://lore.kernel.org/r/23bf7f5cb04da691fd6ba0a77babee9ad3195f44.1733876548.git.Thinh.Nguyen@synopsys.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit addresses an issue related to below kernel panic where
panic_on_warn is enabled. It is caused by the unnecessary use of WARN_ON
in functionsfs_bind, which easily leads to the following scenarios.
1.adb_write in adbd 2. UDC write via configfs
================= =====================
->usb_ffs_open_thread() ->UDC write
->open_functionfs() ->configfs_write_iter()
->adb_open() ->gadget_dev_desc_UDC_store()
->adb_write() ->usb_gadget_register_driver_owner
->driver_register()
->StartMonitor() ->bus_add_driver()
->adb_read() ->gadget_bind_driver()
<times-out without BIND event> ->configfs_composite_bind()
->usb_add_function()
->open_functionfs() ->ffs_func_bind()
->adb_open() ->functionfs_bind()
<ffs->state !=FFS_ACTIVE>
The adb_open, adb_read, and adb_write operations are invoked from the
daemon, but trying to bind the function is a process that is invoked by
UDC write through configfs, which opens up the possibility of a race
condition between the two paths. In this race scenario, the kernel panic
occurs due to the WARN_ON from functionfs_bind when panic_on_warn is
enabled. This commit fixes the kernel panic by removing the unnecessary
WARN_ON.
Kernel panic - not syncing: kernel: panic_on_warn set ...
[ 14.542395] Call trace:
[ 14.542464] ffs_func_bind+0x1c8/0x14a8
[ 14.542468] usb_add_function+0xcc/0x1f0
[ 14.542473] configfs_composite_bind+0x468/0x588
[ 14.542478] gadget_bind_driver+0x108/0x27c
[ 14.542483] really_probe+0x190/0x374
[ 14.542488] __driver_probe_device+0xa0/0x12c
[ 14.542492] driver_probe_device+0x3c/0x220
[ 14.542498] __driver_attach+0x11c/0x1fc
[ 14.542502] bus_for_each_dev+0x104/0x160
[ 14.542506] driver_attach+0x24/0x34
[ 14.542510] bus_add_driver+0x154/0x270
[ 14.542514] driver_register+0x68/0x104
[ 14.542518] usb_gadget_register_driver_owner+0x48/0xf4
[ 14.542523] gadget_dev_desc_UDC_store+0xf8/0x144
[ 14.542526] configfs_write_iter+0xf0/0x138
Fixes: ddf8abd259 ("USB: f_fs: the FunctionFS driver")
Cc: stable <stable@kernel.org>
Signed-off-by: Akash M <akash.m5@samsung.com>
Link: https://lore.kernel.org/r/20241219125221.1679-1-akash.m5@samsung.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Considering that in some extreme cases, when performing the
unbinding operation, gserial_disconnect has cleared gser->ioport,
which triggers gadget reconfiguration, and then calls gs_read_complete,
resulting in access to a null pointer. Therefore, ep is disabled before
gserial_disconnect sets port to null to prevent this from happening.
Call trace:
gs_read_complete+0x58/0x240
usb_gadget_giveback_request+0x40/0x160
dwc3_remove_requests+0x170/0x484
dwc3_ep0_out_start+0xb0/0x1d4
__dwc3_gadget_start+0x25c/0x720
kretprobe_trampoline.cfi_jt+0x0/0x8
kretprobe_trampoline.cfi_jt+0x0/0x8
udc_bind_to_driver+0x1d8/0x300
usb_gadget_probe_driver+0xa8/0x1dc
gadget_dev_desc_UDC_store+0x13c/0x188
configfs_write_iter+0x160/0x1f4
vfs_write+0x2d0/0x40c
ksys_write+0x7c/0xf0
__arm64_sys_write+0x20/0x30
invoke_syscall+0x60/0x150
el0_svc_common+0x8c/0xf8
do_el0_svc+0x28/0xa0
el0_svc+0x24/0x84
Fixes: c1dca562be ("usb gadget: split out serial core")
Cc: stable <stable@kernel.org>
Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Lianqin Hu <hulianqin@vivo.com>
Link: https://lore.kernel.org/r/TYUPR06MB621733B5AC690DBDF80A0DCCD2042@TYUPR06MB6217.apcprd06.prod.outlook.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Currently afunc_bind sets std_ac_if_desc.bNumEndpoints to 1 if
controls (mute/volume) are enabled. During next afunc_bind call,
bNumEndpoints would be unchanged and incorrectly set to 1 even
if the controls aren't enabled.
Fix this by resetting the value of bNumEndpoints to 0 on every
afunc_bind call.
Fixes: eaf6cbe099 ("usb: gadget: f_uac2: add volume and mute support")
Cc: stable <stable@kernel.org>
Signed-off-by: Prashanth K <quic_prashk@quicinc.com>
Link: https://lore.kernel.org/r/20241211115915.159864-1-quic_prashk@quicinc.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The meaning of this value is already used in several places,
but with constant values and comments to explain it separately.
It's better to have a central place to do this then use the macro
in those places for better readability.
Signed-off-by: Dingyan Li <18500469033@163.com>
Reviewed-by: Alan Stern <stern@rowland.harvard.edu>
Link: https://lore.kernel.org/r/20241030083858.46907-1-18500469033@163.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Considering that in some extreme cases,
when u_serial driver is accessed by multiple threads,
Thread A is executing the open operation and calling the gs_open,
Thread B is executing the disconnect operation and calling the
gserial_disconnect function,The port->port_usb pointer will be set to NULL.
E.g.
Thread A Thread B
gs_open() gadget_unbind_driver()
gs_start_io() composite_disconnect()
gs_start_rx() gserial_disconnect()
... ...
spin_unlock(&port->port_lock)
status = usb_ep_queue() spin_lock(&port->port_lock)
spin_lock(&port->port_lock) port->port_usb = NULL
gs_free_requests(port->port_usb->in) spin_unlock(&port->port_lock)
Crash
This causes thread A to access a null pointer (port->port_usb is null)
when calling the gs_free_requests function, causing a crash.
If port_usb is NULL, the release request will be skipped as it
will be done by gserial_disconnect.
So add a null pointer check to gs_start_io before attempting
to access the value of the pointer port->port_usb.
Call trace:
gs_start_io+0x164/0x25c
gs_open+0x108/0x13c
tty_open+0x314/0x638
chrdev_open+0x1b8/0x258
do_dentry_open+0x2c4/0x700
vfs_open+0x2c/0x3c
path_openat+0xa64/0xc60
do_filp_open+0xb8/0x164
do_sys_openat2+0x84/0xf0
__arm64_sys_openat+0x70/0x9c
invoke_syscall+0x58/0x114
el0_svc_common+0x80/0xe0
do_el0_svc+0x1c/0x28
el0_svc+0x38/0x68
Fixes: c1dca562be ("usb gadget: split out serial core")
Cc: stable@vger.kernel.org
Suggested-by: Prashanth K <quic_prashk@quicinc.com>
Signed-off-by: Lianqin Hu <hulianqin@vivo.com>
Acked-by: Prashanth K <quic_prashk@quicinc.com>
Link: https://lore.kernel.org/r/TYUPR06MB62178DC3473F9E1A537DCD02D2362@TYUPR06MB6217.apcprd06.prod.outlook.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The UMP Function Block info m1.0 field (represented by is_midi1 sysfs
entry) is an enumeration from 0 to 2, while the midi2 gadget driver
incorrectly copies it to the corresponding snd_ump_block_info.flags
bits as-is. This made the wrong bit flags set when m1.0 = 2.
This patch corrects the wrong interpretation of is_midi1 bits.
Fixes: 29ee7a4ddd ("usb: gadget: midi2: Add configfs support")
Cc: stable@vger.kernel.org
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://lore.kernel.org/r/20241127070213.8232-1-tiwai@suse.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
After an initial range change on the insigned int alt being > 1
the only possible values for alt are 0 or 1. Therefore the else
statement for values other than 0 or 1 is redundant and can be
removed. Replace the else if (all == 1) check with just an else.
Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
Reviewed-by: Takashi Iwai <tiwai@suse.de>
Link: https://lore.kernel.org/5f54ffd0-b5fe-4203-a626-c166becad362@gmail.com
Link: https://lore.kernel.org/r/20241107133348.22762-1-colin.i.king@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
If the streamoff call was triggered by some previous disconnect
or userspace application shutdown the uvc_function_setup_continue
should not be called and the state should not be overwritten.
For this situation the set_alt(0) was never called and the streaming ep
has no USB_GADGET_DELAYED_STATUS pending.
Since the state then was already updated before we also omit the state
update.
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Link: https://lore.kernel.org/r/20240403-uvc_request_length_by_interval-v7-9-e224bb1035f0@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This patch is adding trace events for each request that is being
enqueued into the hw and will be completed. This way it is possible
to track the fill status of the gadget hardware and find potential
issues.
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Link: https://lore.kernel.org/r/20240403-uvc_request_length_by_interval-v7-8-e224bb1035f0@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
We set the minimum amount of v4l2 buffers that is possibly be pending
to UVCG_STREAMING_MIN_BUFFERS which is two. This way the driver will
always have at least one frame pending to be encoded while the other
is being enqueued in the hardware.
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Link: https://lore.kernel.org/r/20240403-uvc_request_length_by_interval-v7-7-e224bb1035f0@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Compressed formats generate content depending amount of data that is set
in the vb2 buffer by the payload_size. When streaming those formats it
is better to scatter that smaller data over all requests. This patch is
doing that by introducing the calculated req_payload_size which is
updated by each frame. It the uses this amount of data to fill the
isoc requests instead of the video->req_size.
For uncompressed formats it will not make a difference since the payload
size will be equal to the imagesize. Therefore the code will have no
effecta as req_payload_size will be equal to req_size.
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Link: https://lore.kernel.org/r/20240403-uvc_request_length_by_interval-v7-6-e224bb1035f0@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This patch is removing the initial imprecise and limited calculation of
requests needed to be used from the queue_setup callback. It instead
introduces the uvc_video_prep_requests function which is called
immediately before the request allocation.
With the information of the usb frame interval length it is possible to
calculate the number of requests needed during one frame duration.
Based on the calculated number of requests and the imagesize we
calculate the actual size per request. This calculation has the benefit
that the frame data is equally distributed over all allocated requests.
When the req_size is not in the range for the actually configured
max_req_size configured for the overall bandwidth we fallback
to use the max_req_size instead.
Since this calculations are only important for isoc transfers we just
use max_request_size for bulk and skip it.
As video->req_size will be recalculated on every video_enable resetting
it to 0 is not necessary anymore.
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Link: https://lore.kernel.org/r/20240403-uvc_request_length_by_interval-v7-5-e224bb1035f0@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The uvc gadget driver is lacking the information which frame interval
was set by the host. We add this information by implementing the g_parm
and s_parm callbacks.
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Link: https://lore.kernel.org/r/20240403-uvc_request_length_by_interval-v7-4-e224bb1035f0@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
We install an kthread with pfifo priority that is iterating over all
prepared requests and keeps the isoc queue busy. This way it will be
scheduled with the same priority as the interrupt handler.
As the kthread is triggered with video_enable it will immediately
queue some zero length requests into the hw if there is no buffer data
available. It also watches the level of needed zero length requests in
the hardware not to fall under the UVCG_REQ_MAX_ZERO_COUNT threshold.
This way we can drop the function uvc_video_ep_queue_initial_requests
entirely.
By using the kthread to do the actual request handling the interrupt
handler will not be running into the time consuming and eventually
locking work of actually enqueueing the requests back into its own
pipeline. This work can now even be scheduled on another cpu.
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Link: https://lore.kernel.org/r/20240403-uvc_request_length_by_interval-v7-3-e224bb1035f0@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The complete handler will at least be called after 16 requests have
completed, but will still handle all finisher requests. Since we have
to maintain a costant filling in the isoc queue we ensure this by
adding zero length requests.
By counting the amount enqueued requests we can ensure that the queue is
never underrun and only need to get active if the queue is running
critical. This patch is setting 32 as the critical level, which
is twice the request amount that is needed to create interrupts.
To properly solve the amount of zero length requests that needs to
be held in the hardware after one interrupt needs to be measured
and depends on the runtime of the first enqueue run after the interrupt
triggered. For now we just use twice the amount of requests between an
interrupt.
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Link: https://lore.kernel.org/r/20240403-uvc_request_length_by_interval-v7-2-e224bb1035f0@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Since the req_free list will updated if enqueuing one request was not
possible it will be added back to the free list. With every available
free request in the queue it is a valid case for the pump worker to use
it and continue the pending bufferdata into requests for the req_ready
list.
Fixes: 6acba0345b ("usb:gadget:uvc Do not use worker thread to pump isoc usb requests")
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Link: https://lore.kernel.org/r/20240403-uvc_request_length_by_interval-v7-1-e224bb1035f0@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Add support for frame-based frame format, which can be used to support
multiple formats like H264 or H265, in addition to MJPEG and YUV frames.
The frame-based format is set to H264 by default, but it can be updated
to other formats by modifying the GUID through the guid configfs
attribute. Different structures are used for all three formats, as
H264 has a different structure compared to MJPEG and uncompressed
formats. These structures will be passed to the frame make function
based on the active format, using a common frame structure with
additional parameters needed only for frame-based formats. These
parameters are handled at runtime in the UVC driver.
Signed-off-by: Akash Kumar <quic_akakum@quicinc.com>
Link: https://lore.kernel.org/r/20240927152138.31416-1-quic_akakum@quicinc.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The configfs store callback should return the number of bytes consumed
not the total number of bytes we actually stored. These could differ if
for example the passed in string had a newline we did not store.
If the returned value does not match the number of bytes written the
writer might assume a failure or keep trying to write the remaining bytes.
For example the following command will hang trying to write the final
newline over and over again (tested on bash 2.05b):
echo foo > function_name
Fixes: 993a44fa85 ("usb: gadget: f_uac2: allow changing interface name via configfs")
Cc: stable <stable@kernel.org>
Signed-off-by: Kevin Groeneveld <kgroeneveld@lenbrook.com>
Link: https://lore.kernel.org/r/20241006232637.4267-1-kgroeneveld@lenbrook.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Remove the extra semicolon after the
do {} while (0) in UVC_COPY_DESCRIPTOR macro.
Fix the following checkpatch.pl warning
WARNING: do {} while (0) macros should not be semicolon terminated
+#define UVC_COPY_DESCRIPTOR(mem, dst, desc) \
+ do { \
+ memcpy(mem, desc, (desc)->bLength); \
+ *(dst)++ = mem; \
+ mem += (desc)->bLength; \
+ } while (0);
Signed-off-by: Abhishek Tamboli <abhishektamboli9@gmail.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Link: https://lore.kernel.org/r/20241013142511.9946-1-abhishektamboli9@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The function strcpy() is depreciated and potentially unsafe. It performs
no bounds checking on the destination buffer. This could result in
linear overflows beyond the end of the buffer, leading to all kinds of
misbehaviors. The safe replacement is strscpy() [1].
this fixes checkpatch warning:
WARNING: Prefer strscpy over strcpy
Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strcpy [1]
Signed-off-by: Abdul Rahim <abdul.rahim@myyahoo.com>
Reviewed-by: Kees Cook <kees@kernel.org>
Link: https://lore.kernel.org/r/20240914231756.503521-1-abdul.rahim@myyahoo.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
asm/unaligned.h is always an include of asm-generic/unaligned.h;
might as well move that thing to linux/unaligned.h and include
that - there's nothing arch-specific in that header.
auto-generated by the following:
for i in `git grep -l -w asm/unaligned.h`; do
sed -i -e "s/asm\/unaligned.h/linux\/unaligned.h/" $i
done
for i in `git grep -l -w asm-generic/unaligned.h`; do
sed -i -e "s/asm-generic\/unaligned.h/linux\/unaligned.h/" $i
done
git mv include/asm-generic/unaligned.h include/linux/unaligned.h
git mv tools/include/asm-generic/unaligned.h tools/include/linux/unaligned.h
sed -i -e "/unaligned.h/d" include/asm-generic/Kbuild
sed -i -e "s/__ASM_GENERIC/__LINUX/" include/linux/unaligned.h tools/include/linux/unaligned.h
no_llseek had been defined to NULL two years ago, in commit 868941b144
("fs: remove no_llseek")
To quote that commit,
At -rc1 we'll need do a mechanical removal of no_llseek -
git grep -l -w no_llseek | grep -v porting.rst | while read i; do
sed -i '/\<no_llseek\>/d' $i
done
would do it.
Unfortunately, that hadn't been done. Linus, could you do that now, so
that we could finally put that thing to rest? All instances are of the
form
.llseek = no_llseek,
so it's obviously safe.
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
We move the func_utils.h header to include/linux/usb to be
able to compile function drivers outside of the
drivers/usb/gadget/function directory.
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Link: https://lore.kernel.org/r/20240116-ml-topic-u9p-v12-1-9a27de5160e0@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Replace kthread_create/wake_up_process() with kthread_run()
to simplify the code.
Acked-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Hongbo Li <lihongbo22@huawei.com>
Link: https://lore.kernel.org/r/20240903014249.3098082-1-lihongbo22@huawei.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The bInterfaceProtocol is hardcoded to USB_CDC_ACM_PROTO_AT_V25TER. This
will lead to problems with ModemManger which will gladly try to probe
that port as a modem if the gadget also has a network function.
ModemManager will try to send AT commands to the ACM port. Make the
bInterfaceProtocol configurable. For this, track the number of instances
and only allow write to the property if there are no intances (yet).
This will also set bFunctionProtocol to the same value, see commit
5c8db070b4 ("USB: Change acm_iad_descriptor bFunctionProtocol to
USB_CDC_ACM_PROTO_AT_V25TER") for more details.
Signed-off-by: Michael Walle <mwalle@kernel.org>
Link: https://lore.kernel.org/r/20240825180446.3757073-1-mwalle@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Since commit "6acba0345b68 usb:gadget:uvc Do not use worker thread to pump
isoc usb requests", pump work could only be queued in uvc_video_complete()
and uvc_v4l2_qbuf(). If VIDIOC_QBUF is executed before VIDIOC_STREAMON,
we can only depend on uvc_video_complete() to queue pump work. However,
this requires some free requests in req_ready list. If req_ready list is
empty all the time, pump work will never be queued and video datas will
never be pumped to usb controller. Actually, this situation could happen
when run uvc-gadget with static image:
$ ./uvc-gadget -i 1080p.jpg uvc.0
When capture image from this device, the user app will always block there.
The issue is uvc driver has queued video buffer before streamon, but the
req_ready list is empty all the time after streamon. This will queue pump
work in uvcg_video_enable() to fill some request to req_ready list so the
uvc device could work properly.
Fixes: 6acba0345b ("usb:gadget:uvc Do not use worker thread to pump isoc usb requests")
Cc: stable@vger.kernel.org
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
Link: https://lore.kernel.org/r/20240814112537.2608949-1-xu.yang_2@nxp.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
While supporting GET_REPORT is a mandatory request per the HID
specification the current implementation of the GET_REPORT request responds
to the USB Host with an empty reply of the request length. However, some
USB Hosts will request the contents of feature reports via the GET_REPORT
request. In addition, some proprietary HID 'protocols' will expect
different data, for the same report ID, to be to become available in the
feature report by sending a preceding SET_REPORT to the USB Device that
defines what data is to be presented when that feature report is
subsequently retrieved via GET_REPORT (with a very fast < 5ms turn around
between the SET_REPORT and the GET_REPORT).
There are two other patch sets already submitted for adding GET_REPORT
support. The first [1] allows for pre-priming a list of reports via IOCTLs
which then allows the USB Host to perform the request, with no further
userspace interaction possible during the GET_REPORT request. And another
[2] which allows for a single report to be setup by userspace via IOCTL,
which will be fetched and returned by the kernel for subsequent GET_REPORT
requests by the USB Host, also with no further userspace interaction
possible.
This patch, while loosely based on both the patch sets, differs by allowing
the option for userspace to respond to each GET_REPORT request by setting
up a poll to notify userspace that a new GET_REPORT request has arrived. To
support this, two extra IOCTLs are supplied. The first of which is used to
retrieve the report ID of the GET_REPORT request (in the case of having
non-zero report IDs in the HID descriptor). The second IOCTL allows for
storing report responses in a list for responding to requests.
The report responses are stored in a list (it will be either added if it
does not exist or updated if it exists already). A flag (userspace_req) can
be set to whether subsequent requests notify userspace or not.
Basic operation when a GET_REPORT request arrives from USB Host:
- If the report ID exists in the list and it is set for immediate return
(i.e. userspace_req == false) then response is sent immediately,
userspace is not notified
- The report ID does not exist, or exists but is set to notify userspace
(i.e. userspace_req == true) then notify userspace via poll:
- If userspace responds, and either adds or update the response in
the list and respond to the host with the contents
- If userspace does not respond within the fixed timeout (2500ms)
but the report has been set prevously, then send 'old' report
contents
- If userspace does not respond within the fixed timeout (2500ms)
and the report does not exist in the list then send an empty
report
Note that userspace could 'prime' the report list at any other time.
While this patch allows for flexibility in how the system responds to
requests, and therefore the HID 'protocols' that could be supported, a
drawback is the time it takes to service the requests and therefore the
maximum throughput that would be achievable. The USB HID Specification
v1.11 itself states that GET_REPORT is not intended for periodic data
polling, so this limitation is not severe.
Testing on an iMX8M Nano Ultra Lite with a heavy multi-core CPU loading
showed that userspace can typically respond to the GET_REPORT request
within 1200ms - which is well within the 5000ms most operating systems seem
to allow, and within the 2500ms set by this patch.
[1] https://lore.kernel.org/all/20220805070507.123151-2-sunil@amarulasolutions.com/
[2] https://lore.kernel.org/all/20220726005824.2817646-1-vi@endrift.com/
Signed-off-by: David Sands <david.sands@biamp.com>
Signed-off-by: Chris Wulff <chris.wulff@biamp.com>
Link: https://lore.kernel.org/r/20240817142850.1311460-2-crwulff@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Fix potential dereferencing of ERR_PTR() in find_format_by_pix()
and uvc_v4l2_enum_format().
Fix the following smatch errors:
drivers/usb/gadget/function/uvc_v4l2.c:124 find_format_by_pix()
error: 'fmtdesc' dereferencing possible ERR_PTR()
drivers/usb/gadget/function/uvc_v4l2.c:392 uvc_v4l2_enum_format()
error: 'fmtdesc' dereferencing possible ERR_PTR()
Also, fix similar issue in uvc_v4l2_try_format() for potential
dereferencing of ERR_PTR().
Signed-off-by: Abhishek Tamboli <abhishektamboli9@gmail.com>
Link: https://lore.kernel.org/r/20240815102202.594812-1-abhishektamboli9@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Add the ability for the USB FunctionFS (FFS) gadget driver to be able
to create Device Firmware Upgrade (DFU) functional descriptors. [1]
This patch allows implementation of DFU in userspace using the
FFS gadget. The DFU protocol uses the control pipe (ep0) for all
messaging so only the addition of the DFU functional descriptor
is needed in the kernel driver.
The DFU functional descriptor is written to the ep0 file along with
any other descriptors during FFS setup. DFU requires an interface
descriptor followed by the DFU functional descriptor.
This patch includes documentation of the added descriptor for DFU
and conversion of some existing documentation to kernel-doc format
so that it can be included in the generated docs.
An implementation of DFU 1.1 that implements just the runtime descriptor
using the FunctionFS gadget (with rebooting into u-boot for DFU mode)
has been tested on an i.MX8 Nano.
An implementation of DFU 1.1 that implements both runtime and DFU mode
using the FunctionFS gadget has been tested on Xilinx Zynq UltraScale+.
Note that for the best performance of firmware update file transfers, the
userspace program should respond as quick as possible to the setup packets.
[1] https://www.usb.org/sites/default/files/DFU_1.1.pdf
Signed-off-by: David Sands <david.sands@biamp.com>
Co-developed-by: Chris Wulff <crwulff@gmail.com>
Signed-off-by: Chris Wulff <crwulff@gmail.com>
Link: https://lore.kernel.org/r/20240811000004.1395888-2-crwulff@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Convert array initializers to C99 syntax by adding the '=' after
each designated initializer.
Fix the following smatch warnings:
drivers/usb/gadget/function/u_audio.c:1117:20:
warning: obsolete array initializer, use C99 syntax
drivers/usb/gadget/function/u_audio.c:1124:28:
warning: obsolete array initializer, use C99 syntax
drivers/usb/gadget/function/u_audio.c:1131:19:
warning: obsolete array initializer, use C99 syntax
drivers/usb/gadget/function/u_audio.c:1138:27:
warning: obsolete array initializer, use C99 syntax
drivers/usb/gadget/function/u_audio.c:1145:25:
warning: obsolete array initializer, use C99 syntax
Also, fix two checkpatch.pl warnings:
WARNING: please, no spaces at the start of a line
+ [UAC_FBACK_CTRL] = {$
WARNING: please, no spaces at the start of a line
+ [UAC_MUTE_CTRL] = {$
Signed-off-by: Abhishek Tamboli <abhishektamboli9@gmail.com>
Reviewed-by: Ricardo B. Marliere <ricardo@marliere.net>
Link: https://lore.kernel.org/r/20240801190209.500373-1-abhishektamboli9@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The ``alt`` parameter was used as a way to differentiate between
f->disable() and f->set_alt(). As the code paths diverge quite a bit,
pull out the f->disable() code from ffs_func_set_alt(), everything will
become clearer and less error prone. No change in functionality
intended.
Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
Link: https://lore.kernel.org/r/20240802140428.2000312-3-tudor.ambarus@linaro.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The blamed commit made ffs_func_disable() always return -EINVAL as the
method calls ffs_func_set_alt() with the ``alt`` argument being
``(unsigned)-1``, which is always greater than MAX_ALT_SETTINGS.
Use the MAX_ALT_SETTINGS check just in the f->set_alt() code path,
f->disable() doesn't care about the ``alt`` parameter.
Make a surgical fix, but really the f->disable() code shall be pulled
out from ffs_func_set_alt(), the code will become clearer. A patch will
follow.
Note that ffs_func_disable() always returning -EINVAL made pixel6 crash
on USB disconnect.
Fixes: 2f550553e2 ("usb: gadget: f_fs: Add the missing get_alt callback")
Cc: stable <stable@kernel.org>
Reported-by: William McVicker <willmcvicker@google.com>
Signed-off-by: Tudor Ambarus <tudor.ambarus@linaro.org>
Link: https://lore.kernel.org/r/20240802140428.2000312-2-tudor.ambarus@linaro.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Add icount support for the transmitting and receiving characters. This
will make the tty LED trigger work with the ttyGS devices.
Signed-off-by: Michael Walle <mwalle@kernel.org>
Link: https://lore.kernel.org/r/20240730193840.2580358-1-mwalle@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Upstream commit aba3a8d01d ("usb: gadget: u_serial: add suspend
resume callbacks") added started_delayed flag, so that new ports
which are opened after USB suspend can start IO while resuming.
But if the port was already opened, and gadget suspend kicks in
afterwards, start_delayed will never be set. This causes resume
to bail out before calling gs_start_io(). Fix this by setting
start_delayed during suspend.
Fixes: aba3a8d01d ("usb: gadget: u_serial: add suspend resume callbacks")
Cc: stable@vger.kernel.org
Signed-off-by: Prashanth K <quic_prashk@quicinc.com>
Link: https://lore.kernel.org/r/20240730125754.576326-1-quic_prashk@quicinc.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
These functions can fail if descriptors are malformed, or missing,
for the selected USB speed.
Fixes: eb9fecb9e6 ("usb: gadget: f_uac2: split out audio core")
Fixes: 24f779dac8 ("usb: gadget: f_uac2/u_audio: add feedback endpoint support")
Cc: stable@vger.kernel.org
Signed-off-by: Chris Wulff <crwulff@gmail.com>
Link: https://lore.kernel.org/r/20240721192314.3532697-2-crwulff@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
When the block number 0xff is given to Function Block Discovery
message, the device should return the information of all Function
Blocks, but currently the gadget driver treats it as an error.
Implement the proper behavior for the block 0xff instead.
Fixes: 8b645922b2 ("usb: gadget: Add support for USB MIDI 2.0 function driver")
Cc: stable@vger.kernel.org
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://lore.kernel.org/r/20240717095102.10493-1-tiwai@suse.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Most writes to configfs handle an optional newline, but do not require
it. By using the number of bytes written as the limit for scnprintf()
it is guaranteed that the final character in the buffer will be
overwritten.
This is expected if it is a newline but is undesirable when a string is
written "as-is" (as libusbgx does, for example).
Update the store function to strip an optional newline, matching the
behaviour of usb_string_copy().
Signed-off-by: John Keeping <jkeeping@inmusicbrands.com>
Link: https://lore.kernel.org/r/20240708142553.3995022-1-jkeeping@inmusicbrands.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The MIDI2 gadget driver handled the default MIDI protocol version
incorrectly due to the confusion of the protocol version passed via
configfs (either 1 or 2) and UMP protocol bits (0x100 / 0x200).
As a consequence, the default protocol always resulted in MIDI1.
This patch addresses the misunderstanding of the protocol handling.
Fixes: 29ee7a4ddd ("usb: gadget: midi2: Add configfs support")
Cc: stable <stable@kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://lore.kernel.org/r/20240708095719.25627-1-tiwai@suse.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This reverts commit f49449fbc2.
This commit breaks u_ether on some setups (at least Merrifield). The fix
"usb: gadget: u_ether: Re-attach netif device to mirror detachment" party
restores u-ether. However the netif usb: remains up even usb is switched
from device to host mode. This creates problems for user space as the
interface remains in the routing table while not realy present and network
managers (connman) not detecting a network change.
Various attempts to find the root cause were unsuccesful up to now. Therefore
revert until a solution is found.
Link: https://lore.kernel.org/linux-usb/20231006141231.7220-1-hgajjar@de.adit-jv.com/
Reported-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Reported-by: Ferry Toth <fntoth@gmail.com>
Fixes: f49449fbc2 ("usb: gadget: u_ether: Replace netif_stop_queue with netif_device_detach")
Cc: stable@vger.kernel.org
Signed-off-by: Ferry Toth <fntoth@gmail.com>
Link: https://lore.kernel.org/r/20240620204832.24518-3-ftoth@exalondelft.nl
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
printer_read() and printer_write() guard against the race
against disable() by checking the dev->interface flag,
which in turn is guarded by a spinlock.
These functions, however, drop the lock on multiple occasions.
This means that the test has to be redone after reacquiring
the lock and before doing IO.
Add the tests.
This also addresses CVE-2024-25741
Fixes: 7f2ca14d2f ("usb: gadget: function: printer: Interface is disabled and returns error")
Cc: stable <stable@kernel.org>
Signed-off-by: Oliver Neukum <oneukum@suse.com>
Link: https://lore.kernel.org/r/20240620114039.5767-1-oneukum@suse.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
We need to treat super speed plus as super speed, not the default,
which is full speed.
Signed-off-by: Oliver Neukum <oneukum@suse.com>
Cc: stable <stable@kernel.org>
Link: https://lore.kernel.org/r/20240620093800.28901-1-oneukum@suse.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-----BEGIN PGP SIGNATURE-----
iQFSBAABCAA8FiEEq68RxlopcLEwq+PEeb4+QwBBGIYFAmY39LkeHHRvcnZhbGRz
QGxpbnV4LWZvdW5kYXRpb24ub3JnAAoJEHm+PkMAQRiGaB8H+wWcwT1O7K2SQx1a
sHDYJSXfbjgShS7Wt+gbDNvhGJqG/5FkpHyBmBKUdElZs8QMf21GAbIfJvzcNThA
g+FakIMjfhNH/dr6TJXHWbb2NZV67exDbbuaQDyMhNhIZqil4BH/OEPbLYIdquvm
/HDGFB/whVoHVC+Ug8esEOL3Uy+rizBDlsT/K4ZgpOC3TuUmlRpptSVACwjcCGXM
PFKyyTIr0ZnKcz3+4yTEtpDFm6e/ujxMwM66GBCgmPaZ3lJp5ruY3kGpBTmSyoqV
Rh3CQlIhBo8gwlAx/yQXfxEZomhxT6DfRpc6s/1W0J1UZN/j2pYy056OYyckG32c
uQ9ujxQ=
=M0wP
-----END PGP SIGNATURE-----
Merge 6.9-rc7 into usb-next
We want the USB fixes in here as well, and resolve a merge conflict in
drivers/usb/dwc3/core.c
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
If the USB driver passes a pointer into the TRB buffer for creq, this
buffer can be overwritten with the status response as soon as the event
is queued. This can make the final check return USB_GADGET_DELAYED_STATUS
when it shouldn't. Instead use the stored wLength.
Fixes: 4d644abf25 ("usb: gadget: f_fs: Only return delayed status when len is 0")
Cc: stable <stable@kernel.org>
Signed-off-by: Chris Wulff <chris.wulff@biamp.com>
Link: https://lore.kernel.org/r/CO1PR17MB5419BD664264A558B2395E28E1112@CO1PR17MB5419.namprd17.prod.outlook.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit fixes uvc gadget support on 32-bit platforms.
Commit 0df28607c5 ("usb: gadget: uvc: Generalise helper functions for
reuse") introduced a helper function __uvcg_iter_item_entries() to aid
with parsing lists of items on configfs attributes stores. This function
is a generalization of another very similar function, which used a
stack-allocated temporary buffer of fixed size for each item in the list
and used the sizeof() operator to check for potential buffer overruns.
The new function was changed to allocate the now variably sized temp
buffer on heap, but wasn't properly updated to also check for max buffer
size using the computed size instead of sizeof() operator.
As a result, the maximum item size was 7 (plus null terminator) on
64-bit platforms, and 3 on 32-bit ones. While 7 is accidentally just
barely enough, 3 is definitely too small for some of UVC configfs
attributes. For example, dwFrameInteval, specified in 100ns units,
usually has 6-digit item values, e.g. 166666 for 60fps.
Cc: stable@vger.kernel.org
Fixes: 0df28607c5 ("usb: gadget: uvc: Generalise helper functions for reuse")
Signed-off-by: Ivan Avdeev <me@provod.works>
Link: https://lore.kernel.org/r/20240413150124.1062026-1-me@provod.works
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
FFS based applications can utilize the aio_cancel() callback to dequeue
pending USB requests submitted to the UDC. There is a scenario where the
FFS application issues an AIO cancel call, while the UDC is handling a
soft disconnect. For a DWC3 based implementation, the callstack looks
like the following:
DWC3 Gadget FFS Application
dwc3_gadget_soft_disconnect() ...
--> dwc3_stop_active_transfers()
--> dwc3_gadget_giveback(-ESHUTDOWN)
--> ffs_epfile_async_io_complete() ffs_aio_cancel()
--> usb_ep_free_request() --> usb_ep_dequeue()
There is currently no locking implemented between the AIO completion
handler and AIO cancel, so the issue occurs if the completion routine is
running in parallel to an AIO cancel call coming from the FFS application.
As the completion call frees the USB request (io_data->req) the FFS
application is also referencing it for the usb_ep_dequeue() call. This can
lead to accessing a stale/hanging pointer.
commit b566d38857 ("usb: gadget: f_fs: use io_data->status consistently")
relocated the usb_ep_free_request() into ffs_epfile_async_io_complete().
However, in order to properly implement locking to mitigate this issue, the
spinlock can't be added to ffs_epfile_async_io_complete(), as
usb_ep_dequeue() (if successfully dequeuing a USB request) will call the
function driver's completion handler in the same context. Hence, leading
into a deadlock.
Fix this issue by moving the usb_ep_free_request() back to
ffs_user_copy_worker(), and ensuring that it explicitly sets io_data->req
to NULL after freeing it within the ffs->eps_lock. This resolves the race
condition above, as the ffs_aio_cancel() routine will not continue
attempting to dequeue a request that has already been freed, or the
ffs_user_copy_work() not freeing the USB request until the AIO cancel is
done referencing it.
This fix depends on
commit b566d38857 ("usb: gadget: f_fs: use io_data->status
consistently")
Fixes: 2e4c7553cd ("usb: gadget: f_fs: add aio support")
Cc: stable <stable@kernel.org> # b566d38857 ("usb: gadget: f_fs: use io_data->status consistently")
Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
Link: https://lore.kernel.org/r/20240409014059.6740-1-quic_wcheng@quicinc.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
ida_alloc() and ida_free() should be preferred to the deprecated
ida_simple_get() and ida_simple_remove().
Note that the upper limit of ida_simple_get() is exclusive, but the one of
ida_alloc_max() is inclusive. So a -1 has been added when needed.
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Link: https://lore.kernel.org/r/7cd361e2b377a5373968fa7deee4169229992a1e.1713107386.git.christophe.jaillet@wanadoo.fr
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
When ncm function is working and then stop usb0 interface for link down,
eth_stop() is called. At this piont, accidentally if usb transport error
should happen in usb_ep_enable(), 'in_ep' and/or 'out_ep' may not be enabled.
After that, ncm_disable() is called to disable for ncm unbind
but gether_disconnect() is never called since 'in_ep' is not enabled.
As the result, ncm object is released in ncm unbind
but 'dev->port_usb' associated to 'ncm->port' is not NULL.
And when ncm bind again to recover netdev, ncm object is reallocated
but usb0 interface is already associated to previous released ncm object.
Therefore, once usb0 interface is up and eth_start_xmit() is called,
released ncm object is dereferrenced and it might cause use-after-free memory.
[function unlink via configfs]
usb0: eth_stop dev->port_usb=ffffff9b179c3200
--> error happens in usb_ep_enable().
NCM: ncm_disable: ncm=ffffff9b179c3200
--> no gether_disconnect() since ncm->port.in_ep->enabled is false.
NCM: ncm_unbind: ncm unbind ncm=ffffff9b179c3200
NCM: ncm_free: ncm free ncm=ffffff9b179c3200 <-- released ncm
[function link via configfs]
NCM: ncm_alloc: ncm alloc ncm=ffffff9ac4f8a000
NCM: ncm_bind: ncm bind ncm=ffffff9ac4f8a000
NCM: ncm_set_alt: ncm=ffffff9ac4f8a000 alt=0
usb0: eth_open dev->port_usb=ffffff9b179c3200 <-- previous released ncm
usb0: eth_start dev->port_usb=ffffff9b179c3200 <--
eth_start_xmit()
--> dev->wrap()
Unable to handle kernel paging request at virtual address dead00000000014f
This patch addresses the issue by checking if 'ncm->netdev' is not NULL at
ncm_disable() to call gether_disconnect() to deassociate 'dev->port_usb'.
It's more reasonable to check 'ncm->netdev' to call gether_connect/disconnect
rather than check 'ncm->port.in_ep->enabled' since it might not be enabled
but the gether connection might be established.
Signed-off-by: Norihiko Hama <Norihiko.Hama@alpsalpine.com>
Cc: stable <stable@kernel.org>
Link: https://lore.kernel.org/r/20240327023550.51214-1-Norihiko.Hama@alpsalpine.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Instead of bailing when fences have already been installed on the
DMABUF, wait for them (with a timeout) when doing a blocking operation.
This fixes the issue where userspace would submit a DMABUF with fences
already installed, with the (correct) expectation that it would just
work.
Fixes: 7b07a2a7ca ("usb: gadget: functionfs: Add DMABUF import interface")
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Link: https://lore.kernel.org/r/20240402110951.16376-3-paul@crapouillou.net
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
A "read" fence was installed when the DMABUF was to be written to,
and a "write" fence was installed when the DMABUF was to be read from.
Besides, dma_resv_usage_rw() should only be used when waiting for
fences.
Fixes: 7b07a2a7ca ("usb: gadget: functionfs: Add DMABUF import interface")
Signed-off-by: Paul Cercueil <paul@crapouillou.net>
Link: https://lore.kernel.org/r/20240402110951.16376-2-paul@crapouillou.net
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The size given to strscpy() is not consistent with the destination buffer
that is used. The size is related to 'driver' and the buffer is
'mixername'.
sizeof(card->mixername) is 80 and sizeof(card->driver) is 16, so in
theory this could lead to unneeded string truncation.
In practice, this is not the case because g_audio_setup() has only 2
callers. 'card_name' is either "UAC1_Gadget" or "UAC2_Gadget".
Anyway, using the correct size is cleaner and more future proof.
In order to be less verbose, use the new 2-argument version of strscpy()
which computes auto-magically the size of the destination.
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Link: https://lore.kernel.org/r/bf8a9353319566624f653531b80e5caf3d346ba1.1711176700.git.christophe.jaillet@wanadoo.fr
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
The Apple CarLife iAP gadget has a descriptor in userspace with two
alternate settings. The host sends the set_alt request to configure
alt_setting 0 or 1, and this is verified by the subsequent get_alt
request.
This patch implements and sets the get_alt callback. Without the
get_alt callback, composite.c abruptly concludes the
USB_REQ_GET/SET_INTERFACE request, assuming only one alt setting
for the endpoint.
unlike the uvc and ncm, f_fs gadget is fully implemented in userspace,
and driver just reset the eps and generate the event. so no additional
adaptaion associated with this change is not required in set_alt callback
Signed-off-by: Hardik Gajjar <hgajjar@de.adit-jv.com>
Link: https://lore.kernel.org/r/20240301124708.120394-1-hgajjar@de.adit-jv.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
When setting the guid via configfs it is possible to test if
its value is one of the kernel supported ones by calling
uvc_format_by_guid on it. If the result is NULL, we know the
guid is unsupported and can be ignored.
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Link: https://lore.kernel.org/r/20240221-uvc-gadget-configfs-guid-v1-1-f0678ca62ebb@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
When setting uncompressed formats, the values of bytesperline and
sizeimage can already be determined by using the v4l2_fill_pixfmt helper
function. We change the try_fmt function to use the helper instead.
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Link: https://lore.kernel.org/r/20240221-uvc-gadget-uncompressed-v1-1-f55e97287cae@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Here is the big set of USB and Thunderbolt changes for 6.9-rc1. Lots of
tiny changes and forward progress to support new hardware and better
support for existing devices. Included in here are:
- Thunderbolt (i.e. USB4) updates for newer hardware and uses as more
people start to use the hardware
- default USB authentication mode Kconfig and documentation update to
make it more obvious what is going on
- USB typec updates and enhancements
- usual dwc3 driver updates
- usual xhci driver updates
- function USB (i.e. gadget) driver updates and additions
- new device ids for lots of drivers
- loads of other small updates, full details in the shortlog
All of these, including a "last minute regression fix" have been in
linux-next with no reported issues.
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-----BEGIN PGP SIGNATURE-----
iG0EABECAC0WIQT0tgzFv3jCIUoxPcsxR9QN2y37KQUCZfwpzA8cZ3JlZ0Brcm9h
aC5jb20ACgkQMUfUDdst+ymS9QCdEuF6KJFLOrDrGS4NbZNSUPIVF6oAn350r4NX
CMZah37Dfr1VDCOOV4gQ
=HACL
-----END PGP SIGNATURE-----
Merge tag 'usb-6.9-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb
Pull USB / Thunderbolt updates from Greg KH:
"Here is the big set of USB and Thunderbolt changes for 6.9-rc1. Lots
of tiny changes and forward progress to support new hardware and
better support for existing devices. Included in here are:
- Thunderbolt (i.e. USB4) updates for newer hardware and uses as more
people start to use the hardware
- default USB authentication mode Kconfig and documentation update to
make it more obvious what is going on
- USB typec updates and enhancements
- usual dwc3 driver updates
- usual xhci driver updates
- function USB (i.e. gadget) driver updates and additions
- new device ids for lots of drivers
- loads of other small updates, full details in the shortlog
All of these, including a "last minute regression fix" have been in
linux-next with no reported issues"
* tag 'usb-6.9-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb: (185 commits)
usb: usb-acpi: Fix oops due to freeing uninitialized pld pointer
usb: gadget: net2272: Use irqflags in the call to net2272_probe_fin
usb: gadget: tegra-xudc: Fix USB3 PHY retrieval logic
phy: tegra: xusb: Add API to retrieve the port number of phy
USB: gadget: pxa27x_udc: Remove unused of_gpio.h
usb: gadget/snps_udc_plat: Remove unused of_gpio.h
usb: ohci-pxa27x: Remove unused of_gpio.h
usb: sl811-hcd: only defined function checkdone if QUIRK2 is defined
usb: Clarify expected behavior of dev_bin_attrs_are_visible()
xhci: Allow RPM on the USB controller (1022:43f7) by default
usb: isp1760: remove SLAB_MEM_SPREAD flag usage
usb: misc: onboard_hub: use pointer consistently in the probe function
usb: gadget: fsl: Increase size of name buffer for endpoints
usb: gadget: fsl: Add of device table to enable module autoloading
usb: typec: tcpm: add support to set tcpc connector orientatition
usb: typec: tcpci: add generic tcpci fallback compatible
dt-bindings: usb: typec-tcpci: add tcpci fallback binding
usb: gadget: fsl-udc: Replace custom log wrappers by dev_{err,warn,dbg,vdbg}
usb: core: Set connect_type of ports based on DT node
dt-bindings: usb: Add downstream facing ports to realtek binding
...
While connecting to a Linux host with CDC_NCM_NTB_DEF_SIZE_TX
set to 65536, it has been observed that we receive short packets,
which come at interval of 5-10 seconds sometimes and have block
length zero but still contain 1-2 valid datagrams present.
According to the NCM spec:
"If wBlockLength = 0x0000, the block is terminated by a
short packet. In this case, the USB transfer must still
be shorter than dwNtbInMaxSize or dwNtbOutMaxSize. If
exactly dwNtbInMaxSize or dwNtbOutMaxSize bytes are sent,
and the size is a multiple of wMaxPacketSize for the
given pipe, then no ZLP shall be sent.
wBlockLength= 0x0000 must be used with extreme care, because
of the possibility that the host and device may get out of
sync, and because of test issues.
wBlockLength = 0x0000 allows the sender to reduce latency by
starting to send a very large NTB, and then shortening it when
the sender discovers that there’s not sufficient data to justify
sending a large NTB"
However, there is a potential issue with the current implementation,
as it checks for the occurrence of multiple NTBs in a single
giveback by verifying if the leftover bytes to be processed is zero
or not. If the block length reads zero, we would process the same
NTB infintely because the leftover bytes is never zero and it leads
to a crash. Fix this by bailing out if block length reads zero.
Cc: stable@vger.kernel.org
Fixes: 427694cfaa ("usb: gadget: ncm: Handle decoding of multiple NTB's in unwrap call")
Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
Reviewed-by: Maciej Żenczykowski <maze@google.com>
Link: https://lore.kernel.org/r/20240228115441.2105585-1-quic_kriskura@quicinc.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
We need it here for the USB fixes, and it resolves a merge conflict as
reported in linux-next in drivers/usb/roles/class.c
Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Since commit aed65af1cc ("drivers: make device_type const"), the driver
core can properly handle constant struct device_type. Move the gadget_type
variable to be a constant structure as well, placing it into read-only
memory which can not be modified at runtime.
Signed-off-by: Ricardo B. Marliere <ricardo@marliere.net>
Link: https://lore.kernel.org/r/20240218-device_cleanup-usb-v1-3-77423c4da262@marliere.net
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
It is observed sometimes when tethering is used over NCM with Windows 11
as host, at some instances, the gadget_giveback has one byte appended at
the end of a proper NTB. When the NTB is parsed, unwrap call looks for
any leftover bytes in SKB provided by u_ether and if there are any pending
bytes, it treats them as a separate NTB and parses it. But in case the
second NTB (as per unwrap call) is faulty/corrupt, all the datagrams that
were parsed properly in the first NTB and saved in rx_list are dropped.
Adding a few custom traces showed the following:
[002] d..1 7828.532866: dwc3_gadget_giveback: ep1out:
req 000000003868811a length 1025/16384 zsI ==> 0
[002] d..1 7828.532867: ncm_unwrap_ntb: K: ncm_unwrap_ntb toprocess: 1025
[002] d..1 7828.532867: ncm_unwrap_ntb: K: ncm_unwrap_ntb nth: 1751999342
[002] d..1 7828.532868: ncm_unwrap_ntb: K: ncm_unwrap_ntb seq: 0xce67
[002] d..1 7828.532868: ncm_unwrap_ntb: K: ncm_unwrap_ntb blk_len: 0x400
[002] d..1 7828.532868: ncm_unwrap_ntb: K: ncm_unwrap_ntb ndp_len: 0x10
[002] d..1 7828.532869: ncm_unwrap_ntb: K: Parsed NTB with 1 frames
In this case, the giveback is of 1025 bytes and block length is 1024.
The rest 1 byte (which is 0x00) won't be parsed resulting in drop of
all datagrams in rx_list.
Same is case with packets of size 2048:
[002] d..1 7828.557948: dwc3_gadget_giveback: ep1out:
req 0000000011dfd96e length 2049/16384 zsI ==> 0
[002] d..1 7828.557949: ncm_unwrap_ntb: K: ncm_unwrap_ntb nth: 1751999342
[002] d..1 7828.557950: ncm_unwrap_ntb: K: ncm_unwrap_ntb blk_len: 0x800
Lecroy shows one byte coming in extra confirming that the byte is coming
in from PC:
Transfer 2959 - Bytes Transferred(1025) Timestamp((18.524 843 590)
- Transaction 8391 - Data(1025 bytes) Timestamp(18.524 843 590)
--- Packet 4063861
Data(1024 bytes)
Duration(2.117us) Idle(14.700ns) Timestamp(18.524 843 590)
--- Packet 4063863
Data(1 byte)
Duration(66.160ns) Time(282.000ns) Timestamp(18.524 845 722)
According to Windows driver, no ZLP is needed if wBlockLength is non-zero,
because the non-zero wBlockLength has already told the function side the
size of transfer to be expected. However, there are in-market NCM devices
that rely on ZLP as long as the wBlockLength is multiple of wMaxPacketSize.
To deal with such devices, it pads an extra 0 at end so the transfer is no
longer multiple of wMaxPacketSize.
Cc: <stable@vger.kernel.org>
Fixes: 9f6ce4240a ("usb: gadget: f_ncm.c added")
Signed-off-by: Krishna Kurapati <quic_kriskura@quicinc.com>
Reviewed-by: Maciej Żenczykowski <maze@google.com>
Link: https://lore.kernel.org/r/20240205074650.200304-1-quic_kriskura@quicinc.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
If an frame was transmitted incomplete to the host, we set the
UVC_STREAM_ERR bit in the header for the last request that is going
to be queued. This way the host will know that it should drop the
frame instead of trying to display the corrupted content.
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Link: https://lore.kernel.org/r/20240214-uvc-error-tag-v1-2-37659a3877fe@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
If the request that was missed was zero bytes long, it
is likely that the overall transferred frame was not affected.
So don't flag the frame incomplete in that case.
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
Link: https://lore.kernel.org/r/20240214-uvc-error-tag-v1-1-37659a3877fe@pengutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
