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

Adding support for custom SOF ISR for CFG_TUD_VENDOR #2220

Closed
wants to merge 2 commits into from

Conversation

Rocky04
Copy link
Contributor

@Rocky04 Rocky04 commented Aug 14, 2023

  • Opens the driver SOF which is executed in the ISR to be changed by the vendor if the CFG_TUD_VENDOR configuration is used as a device driver
  • A vendor driver should have full control over over all its aspects

A draft for a generic SOF callback which is executed in the main loop via tud_task() can be found here: #2213

- The previous check is likely not needed
@hathach
Copy link
Owner

hathach commented Aug 14, 2023

.sof in usbd driver is only available for driver that can be managed by usbd, e.g audio with feedback enpoint is enabled, sof is enabled, and disable along with the interface. I don't see the point to add sof_isr for vendor. Vendor manage SOF in application and should use the generic tud_sof_enable()

@hathach hathach closed this Aug 14, 2023
@Rocky04
Copy link
Contributor Author

Rocky04 commented Aug 14, 2023

How is a vendor driver not managed by usbd? I don't get it. If that is the case it makes absolutely no sense for me that there is a CFG_TUD_VENDOR driver present in there.

I agree that there should be a function to enable the SOF of the MCU but besides that...

@hathach
Copy link
Owner

hathach commented Aug 14, 2023

SOF interrupt is not always enabled since it interrupt 1 every 1ms which hurt performance and ampere. It will only enabled when needed. When will vendor eanble sof, and when will it disable sof interrupt ? Read my above explanation for audio sof for feedback endpoint.

@Rocky04
Copy link
Contributor Author

Rocky04 commented Aug 14, 2023

Again, yes the control of that is missing. The code I use so far is much older and rely on a symbol (USE_SOF) where it's always on or off. Indeed a dynamic control of that is the preferred way and for this reason I'm currently adding it to the PR #2213.

It also doesn't matter if it hurts performance when it's needed. Of course it should be off when it's not currently needed. What's the preferred way if an implementation wants to have it's own audio driver with endpoint control? I would imagine in that case the vendor driver should be used instead of altering the audio driver code of TinyUSB. If that is needed for audio why not for video? There can be just video and in a real time stream audio and video even needs to be synchronized.

I'm still irritated why you think a vendor driver isn't controlled by usbd.

@hathach
Copy link
Owner

hathach commented Aug 14, 2023

I'm still irritated why you think a vendor driver isn't controlled by usbd.

English is not my first language, and I don't like to read and type much text. vendor sof is not auto managed by usdb, application turn it on and off by itself and stay permanently after that. Audio sof is only enabled when needed in run time by usbd.

@Rocky04
Copy link
Contributor Author

Rocky04 commented Aug 14, 2023

English is not my first langue either...

At first I thought I know how a vendor defined device should look like. Due to your feedback I'm not sure anymore. 😟

I was expecting that each device should have at least one standard class from TinyUSB defined in the tusb_config.h config file. So if no predefined class matches the use case the vendor class should be used instead. But if this isn't the case I don't get why there is a vendor class to begin with if that should not be used for a custom implementation. 😕

So if the internal classes are off the table, the driver can only be loaded by an application driver via usbd_app_driver_get_cb(). But to me it seems that this isn't fully implemented yet. The value for _app_driver_count is set to zero and _app_driver is set to NULL while where isn't anything which updates these. I would expect that usbd is owned by TinyUSB and so that a normal implementation should not alter the code there. Meaning I would assume TinyUSB can be used for a vendor defined application without needing to touch one of its core files (usbd). So usbd_app_driver_get_cb() updates the driver count as well as the driver structure, all outside TinyUSB.

Either way, sadly I haven't found a single example in the framework how the internal vendor class can be used or how to implement a custom application driver. So I would really appreciative if you can show me how TinyUSB is supposed to be used for a custom vendor define implementation which don't uses any of the common standard classes of TinyUSB.

@hathach
Copy link
Owner

hathach commented Aug 14, 2023

  • webusb example use vendor class
  • if there is any double, just check the code. It explains better than me

@Rocky04
Copy link
Contributor Author

Rocky04 commented Aug 14, 2023

Ok, thank you.... ❤️ That helped to understand how you want it to be used.

@Rocky04 Rocky04 deleted the patch-7 branch August 15, 2023 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants