RFC: USB: Use maximum-speed DTS parameters #15140

finikorg · 2019-04-03T07:16:34Z

Add better HS support

zephyrbot · 2019-04-03T07:18:19Z

All checks are passing now.

Review history of this comment for details about previous failed status.
Note that some checks might have not completed yet.

aurel32 · 2019-04-03T07:58:38Z

At the moment maximum-speed is a string making it impossible to use
with preprocessor macros.

The initial version of the PR didn't use strings. It has been switched to match the linux bindings. See:
#9062 (review)

carlescufi · 2019-04-03T08:44:32Z

@aurel32 understood, thanks for that.
@erwango can you also weigh in here?

@finikorg @jfischer-phytec-iot I think it might be better to stay with strings given @galak's original comment

carlescufi · 2019-04-03T08:46:53Z

include/dt-bindings/usb/usb.h

+#ifndef ZEPHYR_INCLUDE_DT_BINDINGS_USB_H_
+#define ZEPHYR_INCLUDE_DT_BINDINGS_USB_H_
+
+#define USB_MAXIMUM_SPEED_LOW_SPEED	0


I would move those to usb_dc.h or similar and then have some code that translates the string into a variable containing one of the values. I would do this directly in PR #14977

we need this kind of stuff
b441bba68a64a3ba8a6f389b4845b8878d632921

And especially here 6098067af34951b0ef7bf2ab1dae2721213f49a1. This is just the example, should be changed to every place we define endpoints.

galak

The DTS proper maximum-speed should be a string, it should NOT be a int. This matches dts binding specs from Linux for this property.

finikorg · 2019-04-03T14:34:29Z

The DTS proper maximum-speed should be a string, it should NOT be a int. This matches dts binding specs from Linux for this property.

How can we use strings for preprocessor macros? Maybe we use then Kconfig for that?

finikorg · 2019-04-03T14:48:52Z

@nashif @galak
Maybe our dts parser script can create out of strings like DT_USB_MAXIMUM_SPEED = "high-speed" something like

#define DT_USB_MAXIMUM_SPEED_HIGH_SPEED 1

jfischer-no · 2019-04-04T22:57:11Z

include/usb/usb_device.h

@@ -74,6 +75,17 @@ extern "C" {

 #define MAX_PACKET_SIZE0    64        /**< maximum packet size for EP 0 */

+#if defined(DT_USBHS_MAXIMUM_SPEED) && \


I think doing it this way is not good, readability suffers and worse, we need to touch it again to support more then one device controller, e.g. a SoC may have one FS controller and one HS controller. That why I want to check it explicit for at least HS and FS.

To support several device controllers we need to rewrite a lot anyway, so it is probably better to touch less code. And we do compare against one global DT_USBHS_MAXIMUM_SPEED which does not depends on the controller. Probably that check should be done in usb_dc_*** function.

3c750df make usb device controller check

carlescufi · 2019-05-22T09:26:32Z

Will be reworked

Fix handling for sam boards. Replace definitions like DT_USBHS_MAXIMUM_SPEED with DT_USB_MAXIMUM_SPEED which is more correct, since HS implies that speed is High Speed and similar to definitions for other boards. Signed-off-by: Andrei Emeltchenko <[email protected]>

Use maximum speed enum making it consistent with the rest of the code and making code faster. Signed-off-by: Andrei Emeltchenko <[email protected]>

Define MPS for endpoints based on maximum speed DTS parameter. Signed-off-by: Andrei Emeltchenko <[email protected]>

Use defined USB_MAX_BULK_MPS for endpoint wMaxPacketSize. Signed-off-by: Andrei Emeltchenko <[email protected]>

Use macros since we need to use them in the preprocessor conditionals. Signed-off-by: Andrei Emeltchenko <[email protected]>

For the devices defining DT_USB_MAXIMUM_SPEED_ENUM and equal to DT_USB_MAXIMUM_SPEED_HIGH_SPEED return Device Qualifier. Signed-off-by: Andrei Emeltchenko <[email protected]>

Use USB_MAX_BULK_MPS instead of hardcoded TEST_BULK_EP_MPS and check size for High Speed. Signed-off-by: Andrei Emeltchenko <[email protected]>

stale

carlescufi · 2019-06-05T09:55:09Z

@jfischer-phytec-iot can you re-review this one?

galak · 2019-09-19T17:24:26Z

@finikorg is this still relevant? If not please close or update the PR.

finikorg · 2019-09-20T05:54:38Z

Closing for now

finikorg requested review from ydamigos, aurel32 and anangl April 3, 2019 07:16

finikorg requested review from erwango, galak and loicpoulain as code owners April 3, 2019 07:16

finikorg requested a review from jfischer-no as a code owner April 3, 2019 07:26

finikorg mentioned this pull request Apr 3, 2019

USB: Fix Coverity issues #14977

Closed

carlescufi added the area: USB Universal Serial Bus label Apr 3, 2019

carlescufi previously requested changes Apr 3, 2019

View reviewed changes

jfischer-no mentioned this pull request Apr 3, 2019

USB: check MPS depending on USB speed and endpoint type (Fix Coverity issues #14977) #15147

Closed

finikorg force-pushed the usb_hs branch from b441bba to 6098067 Compare April 3, 2019 13:34

galak previously requested changes Apr 3, 2019

View reviewed changes

jfischer-no previously requested changes Apr 4, 2019

View reviewed changes

finikorg added 6 commits May 22, 2019 16:58

drivers: usb: usb_dc_stm32: Use maximum-speed enum

cd5d591

Use maximum speed enum making it consistent with the rest of the code and making code faster. Signed-off-by: Andrei Emeltchenko <[email protected]>

usb: Define MPS based on maximum speed parameter

7064b4b

Define MPS for endpoints based on maximum speed DTS parameter. Signed-off-by: Andrei Emeltchenko <[email protected]>

tests: usb: Use USB_MAX_BULK_MPS in tests

2ada502

Use defined USB_MAX_BULK_MPS for endpoint wMaxPacketSize. Signed-off-by: Andrei Emeltchenko <[email protected]>

usb: dt-bindings: Change enum to macros

2f8c804

Use macros since we need to use them in the preprocessor conditionals. Signed-off-by: Andrei Emeltchenko <[email protected]>

usb: device: Return Device Qualifier for HS devices

ab6258c

For the devices defining DT_USB_MAXIMUM_SPEED_ENUM and equal to DT_USB_MAXIMUM_SPEED_HIGH_SPEED return Device Qualifier. Signed-off-by: Andrei Emeltchenko <[email protected]>

finikorg force-pushed the usb_hs branch from 6098067 to ab6258c Compare May 23, 2019 08:12

finikorg requested review from MaureenHelm and nashif as code owners May 23, 2019 08:12

finikorg changed the title ~~USB: Make maximum-speed DTS parameter usable~~ WIP: RFC: USB: Use maximum-speed DTS parameters May 23, 2019

tests: usb: desc_sections: Use standard MPS and check size

b9f1b77

Use USB_MAX_BULK_MPS instead of hardcoded TEST_BULK_EP_MPS and check size for High Speed. Signed-off-by: Andrei Emeltchenko <[email protected]>

finikorg changed the title ~~WIP: RFC: USB: Use maximum-speed DTS parameters~~ RFC: USB: Use maximum-speed DTS parameters Jun 5, 2019

finikorg closed this Sep 20, 2019

finikorg deleted the usb_hs branch September 6, 2021 14:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RFC: USB: Use maximum-speed DTS parameters #15140

RFC: USB: Use maximum-speed DTS parameters #15140

finikorg commented Apr 3, 2019 •

edited

Loading

zephyrbot commented Apr 3, 2019 •

edited

Loading

aurel32 commented Apr 3, 2019

carlescufi commented Apr 3, 2019

carlescufi Apr 3, 2019

finikorg Apr 3, 2019

finikorg Apr 3, 2019

galak left a comment

finikorg commented Apr 3, 2019 •

edited

Loading

finikorg commented Apr 3, 2019 •

edited

Loading

jfischer-no Apr 4, 2019

finikorg Apr 5, 2019

finikorg Apr 5, 2019

carlescufi commented May 22, 2019

carlescufi commented Jun 5, 2019

galak commented Sep 19, 2019

finikorg commented Sep 20, 2019

		@@ -74,6 +75,17 @@ extern "C" {

		#define MAX_PACKET_SIZE0 64 /*< maximum packet size for EP 0 /

		#if defined(DT_USBHS_MAXIMUM_SPEED) && \

RFC: USB: Use maximum-speed DTS parameters #15140

RFC: USB: Use maximum-speed DTS parameters #15140

Conversation

finikorg commented Apr 3, 2019 • edited Loading

zephyrbot commented Apr 3, 2019 • edited Loading

aurel32 commented Apr 3, 2019

carlescufi commented Apr 3, 2019

carlescufi Apr 3, 2019

Choose a reason for hiding this comment

finikorg Apr 3, 2019

Choose a reason for hiding this comment

finikorg Apr 3, 2019

Choose a reason for hiding this comment

galak left a comment

Choose a reason for hiding this comment

finikorg commented Apr 3, 2019 • edited Loading

finikorg commented Apr 3, 2019 • edited Loading

jfischer-no Apr 4, 2019

Choose a reason for hiding this comment

finikorg Apr 5, 2019

Choose a reason for hiding this comment

finikorg Apr 5, 2019

Choose a reason for hiding this comment

carlescufi commented May 22, 2019

carlescufi commented Jun 5, 2019

galak commented Sep 19, 2019

finikorg commented Sep 20, 2019

finikorg commented Apr 3, 2019 •

edited

Loading

zephyrbot commented Apr 3, 2019 •

edited

Loading

finikorg commented Apr 3, 2019 •

edited

Loading

finikorg commented Apr 3, 2019 •

edited

Loading