On 28.10.2024 16:31:25, Ming Yu wrote:

quoted

quoted

quoted

quoted

quoted

quoted

quoted

The Linux USB stack can receive bulk messages longer than the max packet size.

[Ming] Since NCT6694's bulk pipe endpoint size is 128 bytes for this MFD device.
The core will divide packet 256 bytes for high speed USB device, but
it is exceeds
the hardware limitation, so I am dividing it manually.

You say the endpoint descriptor is correctly reporting it's max packet
size of 128, but the Linux USB will send packets of 256 bytes?

[Ming] The endpoint descriptor is correctly reporting it's max packet
size of 256, but the Linux USB may send more than 256 (max is 512)
https://elixir.bootlin.com/linux/v6.11.5/source/drivers/usb/host/xhci-mem.c#L1446

AFAIK according to the USB-2.0 spec the maximum packet size for
high-speed bulk transfers is fixed set to 512 bytes. Does this mean that
your device is a non-compliant USB device?

We will reduce the endpoint size of other interfaces to ensure that MFD device
meets the USB2.0 spec. In other words, I will remove the code for manual
unpacking in the next patch.

I was not talking about the driver, but your USB device. According to
the USB2.0 spec, the packet size is fixed to 512 for high-speed bulk
transfers. So your device must be able to handle 512 byte transfers or
it's a non-compliant USB device.

I understand. Therefore, the USB device's firmware will be modified to support
bulk pipe size of 512 bytes to comply with the USB 2.0 spec.

Then you don't need manual segmentation of bulk transfers anymore!

quoted

quoted

quoted

quoted

quoted

quoted

quoted

quoted

+     for (i = 0, len = length; len > 0; i++, len -= packet_len) {
+             if (len > nct6694->maxp)
+                     packet_len = nct6694->maxp;
+             else
+                     packet_len = len;
+
+             ret = usb_bulk_msg(udev, usb_rcvbulkpipe(udev, BULK_IN_ENDPOINT),
+                                nct6694->rx_buffer + nct6694->maxp * i,
+                                packet_len, &rx_len, nct6694->timeout);
+             if (ret)
+                     goto err;
+     }
+
+     for (i = 0; i < rd_len; i++)
+             buf[i] = nct6694->rx_buffer[i + rd_idx];

memcpy()?

Or why don't you directly receive data into the provided buffer? Copying
of the data doesn't make it faster.

On the other hand, receiving directly into the target buffer means the
target buffer must not live on the stack.

[Ming] Okay! I'll change it to memcpy().

fine!

quoted

This is my perspective: the data is uniformly received by the rx_bffer held
by the MFD device. does it need to be changed?

My question is: Why do you first receive into the nct6694->rx_buffer and
then memcpy() to the buffer provided by the caller, why don't you
directly receive into the memory provided by the caller?

[Ming] Due to the bulk pipe maximum packet size limitation, I think consistently
using the MFD'd dynamically allocated buffer to submit URBs will better
manage USB-related operations

The non-compliant max packet size limitation is unrelated to the
question which RX or TX buffer to use.

I think these two USB functions can be easily called using the buffer
dynamically
allocated by the MFD. However, if they transfer data directly to the
target buffer,
they must ensure that it is not located on the stack.

You have a high coupling between the MFD driver and the individual
drivers anyways, so why not directly use the dynamically allocated
buffer provided by the caller and get rid of the memcpy()?

Okay! I will provide a function to request and free buffer for child devices,
and update the caller's variables to use these two functions in the next patch.

I don't see a need to provide dedicated function to allocate and free
the buffers. The caller can allocate them as part of their private data,
or allocate them during probe().

regards,
Marc

-- 
Pengutronix e.K.                 | Marc Kleine-Budde          |
Embedded Linux                   | https://www.pengutronix.de |
Vertretung Nürnberg              | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax:   +49-5121-206917-9   |