Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

samples: usb: move legacy USB code to legacy directory and cleanup samples #80985

Closed

Conversation

jfischer-no
Copy link
Collaborator

@jfischer-no jfischer-no commented Nov 6, 2024

Move legacy USB code to legacy directory and cleanup samples.
WIP
depends on #81308 #79794

Use new USB device stack.

Signed-off-by: Johann Fischer <[email protected]>
Use new USB device stack.

Signed-off-by: Johann Fischer <[email protected]>
Use new USB device stack.

Signed-off-by: Johann Fischer <[email protected]>
Remove netusb code and use new USB device stack.

Signed-off-by: Johann Fischer <[email protected]>
Remove netusb code, use new USB device stack and CDC NCM implementation.

Signed-off-by: Johann Fischer <[email protected]>
Use new USB device stack.

Signed-off-by: Johann Fischer <[email protected]>
Use new USB device stack.

Signed-off-by: Johann Fischer <[email protected]>
USB device stack is not used in this sample, remove configuration file
overlay.

Signed-off-by: Johann Fischer <[email protected]>
Use new USB device stack.

Signed-off-by: Johann Fischer <[email protected]>
Rework sample to use new USB device stack.

Signed-off-by: Johann Fischer <[email protected]>
This function is used for testing purposes only, there is no real use
for it in a user application. Remove in favor of implementation in new
device stack.
Also remove the part of the documentation that depends on loopback.
Documentation on how to implement your own USB device function using the
new USB device support will follow during the documentation rework.

Signed-off-by: Johann Fischer <[email protected]>
@carlescufi
Copy link
Member

carlescufi commented Feb 5, 2025

@henrikbrixandersen @kartben @nashif we seem to have a problem here preventing us from deprecating the old USB stack: the wio_terminal board uses an IC that has not been ported to the new stack (Atmel SAM0) and enables USB by default, which would then trigger deprecation warnings building any sample with that board. So I think we have 3 options moving forward:

  1. Port the Atmel SAM0 IC to the new stack (but no maintainer has volunteered)
  2. Remove the board altogether from Zephyr
  3. Do not enable USB at all by default in this board, and then enable it only with the legacy samples (which, once @nashif has provided a patch, we will be able to run while ignoring deprecation warnings)
  4. Move samples and boards to the new stack, but not deprecate the old stack yet. This gives us more time to deal with outliers like this one

@kartben
Copy link
Collaborator

kartben commented Feb 5, 2025

@henrikbrixandersen @kartben @nashif we seem to have a problem here preventing us from deprecating the old USB stack: the wio_terminal board uses an IC that has not been ported to the new stack (Atmel SAM0) and enables USB by default, which would then trigger deprecation warnings building any sample with that board. So I think we have 3 options moving forward:

  1. Port the Atmel SAM0 IC to the new stack (but no maintainer has volunteered)
  2. Remove the board altogether from Zephyr
  3. Do not enable USB at all by default in this board, and then enable it only with the legacy samples (which, once @nashif has provided a patch, we will be able to run while ignoring deprecation warnings)
  4. Move samples and boards to the new stack, but not deprecate the old stack yet. This gives us more time to deal with outliers like this one

Mmmh option 2 is certainly a big no from me, and I'm a bit shocked you even consider this to be an option TBH :)

Option 4 sounds like something that impacts MANY boards, a lot more than just Wio Terminal. 21, actually, judging by the upcoming "filter by supported HW feature" addition to the board catalog :) (https://builds.zephyrproject.io/zephyr/pr/79754/docs/boards/index.html then select atmel_sam and atmel_sam0 family, and usb as "Supported Hardware Capabilities"). I don't think we can just stop offering examples of how to do USB Device with these boards, and even if we would, there is a lot of changes that would need to be implemented in various places of the documentation since e.g. https://docs.zephyrproject.org/latest/boards/adafruit/trinket_m0/doc/index.html#supported-features just lists "USB device" as being supported, so surely one would expect to be able to run USB device samples on it.

@carlescufi
Copy link
Member

carlescufi commented Feb 5, 2025

Mmmh option 2 is certainly a big no from me, and I'm a bit shocked you even consider this to be an option TBH :)

I had to write it, for completness ;)

Option 4 sounds like something that impacts MANY boards, a lot more than just Wio Terminal. 21, actually, judging by the upcoming "filter by supported HW feature" addition to the board catalog :)

No, actually, because the other boards do not enable USB by default, and so this would not be an issue: in the legacy samples we would enable the legacy USB stack and that would work just fine with those SAM0/SAM boards. To be clear, the issue with the wio_terminal is that it enables the legacy USB stack by default in its defconfig file, because it enables CDC ACM by default.

@kartben I think the simplest would be 3. in my opinion. But 4 could also be a good option for 4.1, since we are unlikely to be able to fully transition to the new stack by then.

@henrikbrixandersen
Copy link
Member

henrikbrixandersen commented Feb 5, 2025

Mmmh option 2 is certainly a big no from me, and I'm a bit shocked you even consider this to be an option TBH :)

I had to write it, for completness ;)

Option 4 sounds like something that impacts MANY boards, a lot more than just Wio Terminal. 21, actually, judging by the upcoming "filter by supported HW feature" addition to the board catalog :)

No, actually, because the other boards do not enable USB by default, and so this would not be an issue: in the legacy samples we would enable the legacy USB stack and that would work just fine with those SAM0/SAM boards. To be clear, the issue with the wio_terminal is that it enables the legacy USB stack by default in its defconfig file, because it enables CDC ACM by default.

@kartben I think the simplest would be 3. in my opinion. But 4 could also be a good option for 4.1, since we are unlikely to be able to fully transition to the new stack by then.

As already stated in an earlier comment on this PR (#80985 (review)) I do not think "device_next" is ready becoming the default USB device stack in Zephyr v4.1.

There's the issue with lack of "vendor" testing (tested it on a random selection of development boards, three of the new USB device drivers failed to enumerate on MS Windows), the lack of support for a number of platforms (I know some people find it acceptable to "leave these behind"), the list of open bug reports - and especially the vastly increased RAM/ROM footprint as reported in #83309. These are all mentioned in #42066 but not yet handled with 9 days until v4.1 feature freeze.

@fabiobaltieri
Copy link
Member

Switching this just before release does not strike me as a great idea regardless of the how ready we think it is. Would make more sense to switch default at the beginning of the cycle and then we'd have the whole integration period for the vendor to test and stabilize.

Regardless, switching knowing that some platforms are missing entirely does not sound great either, unless you can make an argument that those platforms are not really used or something like that.

@jfischer-no
Copy link
Collaborator Author

There's the issue with lack of "vendor" testing (tested it on a random selection of development boards, three of the new USB device drivers failed to enumerate on MS Windows), the lack of support for a number of platforms (I know some people find it acceptable to "leave these behind"),

There was enough time for this platform to move. Also, I did not see you helping, but I did not expect you to.

the list of open bug reports - and especially the vastly increased RAM/ROM footprint as reported in #83309. These are all mentioned in #42066 but not yet handled with 9 days until v4.1 feature freeze.

There is a bug list for the new device stack? Where is this list? The increased resource usage is something you have to live with and accept. There are few things that can be improved, but this is a new design that cannot be compared to legacy device support. Likewise, it is accepted that the resource consumption of the Zephyr RTOS increases with each release.

@kartben
Copy link
Collaborator

kartben commented Feb 5, 2025

There was enough time for this platform to move.

I'm not sure I agree here. Define "enough time", and in particular I'm not sure it was clearly communicated that every driver had to move - and by when. I might have missed announcements in TSC meetings or on devel mailing list but simply adding an item to a GitHub issue checklist saying that "stragglers may be left behind" is likely not enough of a notice IMO.

[...] Likewise, it is accepted that the resource consumption of the Zephyr RTOS increases with each release.

Says who? I really don't think that blanket statement reflects reality, or that users just expect Zephyr to inevitably grow. And Zephyr's footprint has actually remained pretty stable over the years.

https://stats.zephyrproject.org/testing/d/SSxr5P6Gk/footprint-tracking?orgId=1&var-board=frdm_k64f&var-application=footprints&var-feature=default&var-type=rom&from=now-5y&to=now

https://stats.zephyrproject.org/testing/d/SSxr5P6Gk/footprint-tracking?orgId=1&var-board=frdm_k64f&var-application=footprints&var-feature=default&var-type=ram&from=now-5y&to=now

@carlescufi
Copy link
Member

Having read all the comments, here's my two cents. I have to agree it is too late to transition at this point, so instead I would focus on the 4.2 development cycle and warn maintainers early that they need to transition their drivers (Atmel ones in particular) to avoid being left behind. I was convinced I had sent an email about this to the devel@ mailing list some time ago, but I can't find it so I was wrong.

I will send an email about this.

@jfischer-no
Copy link
Collaborator Author

There was enough time for this platform to move.

I'm not sure I agree here. Define "enough time", and in particular I'm not sure it was clearly communicated that every driver had to move - and by when. I might have missed announcements in TSC meetings or on devel mailing list but simply adding an item to a GitHub issue checklist saying that "stragglers may be left behind" is likely not enough of a notice IMO.

There is an open issue for each driver. This was on Arch WG agenda long time ago, and not once. Finally, the drivers will not be removed now, they will still be there for 2 or 3 releases.

[...] Likewise, it is accepted that the resource consumption of the Zephyr RTOS increases with each release.
Says who? I really don't think that blanket statement reflects reality, or that users just expect Zephyr to inevitably grow. And Zephyr's footprint has actually remained pretty stable over the years.

https://stats.zephyrproject.org/testing/d/SSxr5P6Gk/footprint-tracking?orgId=1&var-board=frdm_k64f&var-application=footprints&var-feature=default&var-type=rom&from=now-5y&to=now

https://stats.zephyrproject.org/testing/d/SSxr5P6Gk/footprint-tracking?orgId=1&var-board=frdm_k64f&var-application=footprints&var-feature=default&var-type=ram&from=now-5y&to=now

Maybe look at something that can be used in the real world,
https://stats.zephyrproject.org/testing/d/SSxr5P6Gk/footprint-tracking?orgId=1&var-board=frdm_k64f&var-application=echo_server&var-feature=default&var-type=rom&from=now-5y&to=now

https://stats.zephyrproject.org/testing/d/SSxr5P6Gk/footprint-tracking?orgId=1&var-board=frdm_k64f&var-application=echo_client&var-feature=default&var-type=rom&from=now-5y&to=now

even something trivial like bt_peripheral grows
https://stats.zephyrproject.org/testing/d/SSxr5P6Gk/footprint-tracking?orgId=1&var-board=nrf52840dk&var-application=bt_peripheral&var-feature=default&var-type=rom&from=now-5y&to=now

Or compare LVGL resource usage from version to version.

@carlescufi
Copy link
Member

There is an open issue for each driver. This was on Arch WG agenda long time ago, and not once. Finally, the drivers will not be removed now, they will still be there for 2 or 3 releases.

All of this is correct, and I guess that's why I had assumed that there was enough visibility about it. The tracking issue has been featured both in the agenda and minutes multiple times, and we had pinged everyone we thought could work on these drivers. But it is true that we did not broadcast it via the mailing lists, which we should've done.

@jfischer-no jfischer-no closed this Feb 5, 2025
@jfischer-no jfischer-no deleted the pr-usb-reorg-samples branch February 5, 2025 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Samples Samples area: USB Universal Serial Bus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants