Hi,

On Wed, Jan 31, 2018 at 08:09:12AM +0530, Sundar Subramaniyan wrote:

Hi Paul,
On Tue, Jan 30, 2018 at 9:55 PM, Paul Sokolovsky
<[1]paul.sokolovsky@...> wrote:


A good clarification thus would be:

1. bytes_ret is always not-NULL.
2. There can always be short writes, number of bytes written is
returned in *bytes_ret, caller must retry with remaining data as
needed.

Note that this doesn't call for updates to existing drivers - they can
keep accepting bytes_ret as NULL, and loop/block inside. But clients
of this API definitely need to be updated, because otherwise they
simply won't work with some drivers.

I think the same too. For now, new drivers should handle both the methods
to be coherent with the API definition. If ret_bytes is NULL, then it
shall to
a blocked write taking care of fragmentation of the packet internally
within
the driver. Otherwise if it is != NULL, it can let the upper layer do the
fragmentation
and shall *not* block-wait for the completion of the current write on the
USB bus,
since we have write callback for notification purpose.

I think we do not have now this requirement not to block-wait. When
working with sending fragmented packets you would prefer to block until all
fragments are sent.

Best regards
Andrei Emeltchenko
_______________________________________________
Zephyr-devel mailing list
Zephyr-devel@...
https://lists.zephyrproject.org/mailman/listinfo/zephyr-devel

Hi Paul,
On Tue, Jan 30, 2018 at 9:55 PM, Paul Sokolovsky
<[1]paul.sokolovsky@...> wrote:
 

A good clarification thus would be:

1. bytes_ret is always not-NULL.
2. There can always be short writes, number of bytes written is
returned in *bytes_ret, caller must retry with remaining data as
needed.

Note that this doesn't call for updates to existing drivers - they can
keep accepting bytes_ret as NULL, and loop/block inside. But clients
of this API definitely need to be updated, because otherwise they
simply won't work with some drivers.

I think the same too. For now, new drivers should handle both the methods
to be coherent with the API definition. If ret_bytes is NULL, then it
shall to
a blocked write taking care of fragmentation of the packet internally
within
the driver. Otherwise if it is != NULL, it can let the upper layer do the
fragmentation
and shall *not* block-wait for the completion of the current write on the
USB bus,
since we have write callback for notification purpose.

I'd suggest to look at this as a generic problem, not specific to
USB subsystem (e.g., there's the same issue in networking
subsystem, etc.), and consider the known best practices dealing
with it. For example, POSIX, when dealing with streams-of-bytes,

Although you raise a good point, I'm not convinced that this applies
here. I think that what Sundar wants to know is whether this
particular API call, usb_write(), was originally designed to be able
to buffer data and fragment it into smaller USB-sized packets or
instead it should mirror the state of whatever hardware FIFO it is
using below, returning a number of bytes written different from the
ones the caller asked to send. Beyond that there's the issue of the
API call being blocking or not, which seems unclear given the
different ways it is used in Zephyr. To sum up, I thin he's trying to
clarify the original intentions of the API designer in order to adapt
the driver he's working on to it, not looking to modify the API
itself.

Thanks,

Carles

Here's what bt_usb does:
https://github.com/zephyrproject-rtos/zephyr/blob/master/samples/bluetooth/hci_usb/src/main.c#L665

When I looked at other drivers to see how they are implemented, the
designware USB driver seems to assume
the application/class drivers always support the "write in parts" method.
I'm not blaming the driver or it's
implementation but I just wanted to point out that there are drivers which
do not support handling chunked
write within them.

Another class driver, netusb does it a little differently by writing in
parts, but this too assumes the usb_write()
to be synchronous.

Here's what it does:
https://github.com/zephyrproject-rtos/zephyr/blob/master/subsys/usb/class/netusb/netusb.c#L189

One more thing to consider is the write callbacks. In most cases, the
application/class driver is the one that registers
the Bulk/Interrupt endpoint callbacks and the device stack has no way of
knowing when to perform the next write, even
if it wants to handle this in the USB device stack.

My question now is, what is the ideal place to take care of truncation and
chunk handling as per the USB device stack design?
Why the driver needs to handle the multi-part write and synchronization?

+ Andrei

 

Carles

 

From: zephyr-devel-bounces@...
[mailto:zephyr-devel-bounces@...] On Behalf Of Sundar
Subramaniyan
Sent: 30 January 2018 13:18
To: zephyr-devel@...
Subject: [Zephyr-devel] USB: Question about usb_write() API

 

Hi,

While developing a USB device controller driver for the Nordic nRF52840
SoC,
I ran into the problem of handling chunked/partial write() while coding
the usb_dc_ep_write() function.

From what I could gather, the usb_write() API documentation states that
the application calling
this function may choose to write all the data buffer at once even if it
is larger than the maximum
packet size supported by the USB endpoint to which we're writing to.
Let's call this method "write all at once".

If the application or the class driver can manage to write in parts, then
it must use the "bytes_ret" argument
to know how much data is written on the bus so as to know if there's
anymore data remaining to be written in
the subsequent call. Let's call this method "write in parts".

In my case, I'm using bt_usb and it tries to write all at once - i.e.
there's no handling of remaining data.
This mandates the USB device controller driver to take care of the writing
in parts within the driver itself.
While this can be done in a simple loop if the driver talks synchronously
to the USBD HW and do a poll and
a yield or use a semaphore till the HW completes the DMA and there's room
to write more.

Here's what bt_usb does:
[1]https://github.com/zephyrproject-rtos/zephyr/blob/master/samples/bluetooth/hci_usb/src/main.c#L665

When I looked at other drivers to see how they are implemented, the
designware USB driver seems to assume
the application/class drivers always support the "write in parts" method.
I'm not blaming the driver or it's
implementation but I just wanted to point out that there are drivers which
do not support handling chunked
write within them.

Here's the DW driver implementation:
[2]https://github.com/zephyrproject-rtos/zephyr/blob/master/drivers/usb/device/usb_dc_dw.c#L1004
[3]https://github.com/zephyrproject-rtos/zephyr/blob/master/drivers/usb/device/usb_dc_dw.c#L423

Another class driver, netusb does it a little differently by writing in
parts, but this too assumes the usb_write()
to be synchronous.

Here's what it does:
[4]https://github.com/zephyrproject-rtos/zephyr/blob/master/subsys/usb/class/netusb/netusb.c#L189

One more thing to consider is the write callbacks. In most cases, the
application/class driver is the one that registers
the Bulk/Interrupt endpoint callbacks and the device stack has no way of
knowing when to perform the next write, even
if it wants to handle this in the USB device stack.

My question now is, what is the ideal place to take care of truncation and
chunk handling as per the USB device stack design?
Why the driver needs to handle the multi-part write and synchronization?

I understand that there are not going to be multiple USBD controllers on a
given chip and taking care of this in the
driver won't cost much from memory/CPU cycles point of view but from USB
DC driver implementation perspective
this might save a lot of time if the stack can abstract them.