Generalize USB pullup attach/detach support #885
Assignees
Labels
Comments
|
On Malyan M200 variants which have the pullup (almost all boards have the pad, not all have the transistor), the pin is active high. Simply setting it high did not work to get reenumeration to work, I had to do the toggle (I tried enough variations of this to know).
… On Jan 23, 2020, at 9:45 AM, Matthijs Kooijman ***@***.***> wrote:
I previously commented about this in the jump-to-bootloader, but I think this is something that warrants a bit more discussion, and can be solved separately as well. See #710 (comment) <#710 (comment)> This is getting a bit of a long post, but I'm also trying to capture my thoughts and, while writing this, moving towards a proposal as well.
USB needs a pullup on the D+ line (or D- for low speed), which indicates the "attachment" status (i.e. pullup present signals the USB host that the device is present, while lack of a pullup detaches from the bus) and allows discriminating between low speed and fullspeed (or higher).
There are various ways this pullup can be implemented in the circuit. In particular, I've seen:
A fixed pullup
A pullup that can be switched on or off using a transistor, which can be default on or off.
A pullup builtin to some MCUs.
Control of this pullup happens in the USBD_reenumerate() function, which currently handles two cases:
When USB_DISC_PIN is defined by the variant (e.g. on the maple mini), the pullup is assumed to be disabled by default, and can be enabled by making the defined pin OUTPUT LOW.
When USB_DISC_PIN is not defined, it is assumed that there is an external fixed pullup resistor. To force re-enumeration, the DP pin itself is briefly put into output low mode, which overrides the pullup and looks like an USB detach to the host (though it is a bit of a hack and probably not entirely standards-compliant).
These two cases seem to be somehwat mismatched: When USB_DISC_PIN is defined, the function actually only attaches to the USB bus (which causes enumeration to happen), while otherwise the function actually detaches and reattaches (which causes reenumeration to happen). So maybe for the USB_DISC_PIN case, it should be driven output high (or input/floating) for a short while before switching to low, so it actually causes reenumeration if already enumerated (e.g. when called later after startup).
Note that the internal pullup and default-on pullup configurations are not currently handled.
More generally, you might want to have an API that actually just supports "Attach" and "Detach" operations, so any caller can decide what they need. However, this is problematic because:
In the fixed pullup configuration, detaching is really a hack that should probably not last for more than a few ms.
In the internal pullup configuration, the USB core automatically controls the pullup state (in particular, on the F401 we're using, the pullup is enabled automatically when USB device mode is enabled and VBUS is detected). It can be disabled using the SDIS (soft disconnect) bit, so some control is still available.
In the different configurations, the default / startup state is different, so different sequences of attach/detach might be required.
So, it is probably better to not support separate attach/detach, but instead have USB_reenumerate() (possibly under a different name) just handle all usecases that need to be handled. Looking at those, I can see:
Startup, which needs to trigger USB attachment. At first glance, it seems this only needs work for the external switched pullup case (just enable the pullup), but for the fixed pullup and default on configurations, the pullup stays on through a reset, so this actually needs to disable the pullup before enabling it. For the internal configuration, nothing is needed (the pullup will be enabled as part of USB initialization). Note that this is the only usecase present in the current code (i.e. the only call to USBD_reenumerate() is at startup).
Jumping to the bootloader, which needs to detach from USB and then reattach to allow the bootloader to provide its own descriptors. This case is added by #710 <#710>. In the current implementation, the bootloader happens after a reset, which makes this case essentially identical to the normal startup case.
Re-enumerating during runtime. I can imagine one wants to, as part of their sketch, force re-enumeration (e.g. to change device type or configuration) without resetting the MCU. This is pretty similar to the startup case, except that it might need to explicitly disable an external pullup, since no MCU reset has done so.
Detaching from the USB bus for a longer period of time. One might want to disable USB connectivity based on some trigger, user interaction, etc. This is something that is not feasible with fixed pullups, but is possible in the other configurations.
Looking at the above, it seems that the first three cases are pretty identical (the third one needs some extra work in case no reset happened beforehand, but that extra work should not hurt if a reset happened, so maybe best to just always do that). The fourth usecase is different, but also not currently supported. Hence, USBD_reenumerate() could be made to support the first three cases, and then we could (if we need it later) add USBD_detach() or something to support the fourth case.
This means that the contract of USBD_reenumerate() can be:
When internal pullups are used, do nothing
Otherwise, when pullups are enabled, disable them for a few ms
Then (re-)enable pullups
This function should then be called from the startup/USB code, it is not intended to be called from user code directly.
This supports the first three usecases (though it might need some extra work with internal pullups for the third usecase, reenumerating during runtime, when that is implemented).
Specifying the configuration
Currently, only two configurations are handled, based on the USB_DISC_PIN macro: External fixed pullup, and external switched pullup that is detached by default and attaches on OUTPUT LOW. I do not really like the USB_DISC_PIN macro name here (it suggests "disconnect pin", while the pin really connects rather than disconnects). Also, it supports only one specific external pullup configuration. I would propose generalizing this by using a few macros. In particular, here's what I would add to the variant.h template:
// If the board has external USB pullup (on DP/DP depending on speed)
// that can be controlled using a GPIO pin, define these.
// - If the the pullup is disabled (detached) by default, define
// USB_ATTACH_PIN to the pin that, when written to USB_ATTACH_LEVEL,
// attaches the pullup.
// - If the the pullup is enabled (attached) by default, define
// USB_DETACH_PIN to the pin that, when written to USB_DETACH_LEVEL,
// detaches the pullup.
//#define USB_ATTACH_PIN x
//#define USB_ATTACH_LEVEL LOW
//#define USB_DETACH_PIN x
//#define USB_ATTACH_LEVEL LOW
I think this could also be used to simplify the MALYANM200 variant, which currently has a customized USBD_reenumerate() function because, I think, it has an active-high attach pin (Maybe @xC0000005 <https://github.com/xC0000005> can confirm that the pullup is indeed disabled by default and enabled by making PB9 high?).
Then, we also need to handle the internal pullup case. Ideally, we would autodetect that internal pullups are supported. We could check for the USB_OTG_DCTL_SDIS macro, which is the Soft Disconnect bit - if you can soft disconnect, that probably means that pullups are builtin. Note sure if this catches all of them, though.
If there are no internal pullups and USB_ATTACH_PIN/USB_DETACH_PIN are not defined, we can fall back to the DP-pin-low hack. Alternatively, we could say that such a hack should not be a default and thus must be explicitly enabled for a board, but I guess that does not add so much (we can always add a macro to disable it later when needed).
Summarizing
It's become a bit of a wall of text, but unless I missed something above, I think this should be something we can implement now. In particular, this needs:
Changes to the template variant.h to document the new macros
Changes to USBD_reenumerate() to:
Support different external switchable pullup configurations
Disable the external switchable pullup and wait a bit, just like with the fixed pullup hack.
Detect internal pullups and do nothing.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#885?email_source=notifications&email_token=AHVGS4NOGAXE25VLOYXVHWDQ7HJTDA5CNFSM4KK2MLL2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4IIKCGJQ>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHVGS4NUJHH2F6V3YNO234LQ7HJTDANCNFSM4KK2MLLQ>.
|
matthijskooijman
added a commit
to 3devo/Arduino_Core_STM32
that referenced
this issue
Jan 23, 2020
Previously, when no external pullup control was available, the core would apply a hack to override an external fixed pullup. However, on chips that have internal pullups, these will be automatically controlled, so this is not needed at all. This probably needs to be more configurable by board, but this is enough to for the specific board we are working with now. See stm32duino#885
|
@matthijskooijman |
fpistm
added
enhancement
@xC0000005, Do you know in more detail how this is wired? I guess there is a transistor to switch the USB pullup on and off? Does the transistor gate have a pulldown/pullup too (i.e. what is its default state)? And a high signal enables the pullup, low disables it, right?
Ok, so that suggests to me that the pullup is on by default and re-enumeration can be done by making the pin low for a while? That would mean that going from INPUT to LOW back to INPUT would make this work (not necessarily HIGH -> LOW -> HIGH)? |
|
Yes, the code in the M200 variant does this right - I switch it high (just to be sure), pull low (to signal) and push it high, just as you suggested. V2 of the printer doesn’t include the pullup (and has some reported issues with enumeration).
… On Mar 18, 2020, at 11:47 AM, Matthijs Kooijman ***@***.***> wrote:
On Malyan M200 variants which have the pullup (almost all boards have the pad, not all have the transistor), the pin is active high.
@xC0000005 <https://github.com/xC0000005>, Do you know in more detail how this is wired? I guess there is a transistor to switch the USB pullup on and off? Does the transistor gate have a pulldown/pullup too (i.e. what is its default state)? And a high signal enables the pullup, low disables it, right?
Simply setting it high did not work to get reenumeration to work, I had to do the toggle (I tried enough variations of this to know).
Ok, so that suggests to me that the pullup is on by default and re-enumeration can be done by making the pin low for a while? That would mean that going from INPUT to LOW back to INPUT would make this work (not necessarily HIGH -> LOW -> HIGH)?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <#885 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHVGS4JU7Z3KBYH4453VIWLRIEJLZANCNFSM4KK2MLLQ>.
|
I'm trying to figure out some more details, since I'm hoping to remove the custom code in the M200 variant and just have some generic code. Easiest would be INPUT -> LOW -> INPUT (Since then the same code can be used for the DP-LOW-hack as well), so I'm trying to figure out what options there are. Good point about the V2 board, I forgot to ask that in my previous comment. You say it does not include the pullup. I presume it does have a D+ pullup, but you probably mean that they do not have a pullup on the transistor gate? Or is there no transistor at all? |
matthijskooijman
added a commit
to 3devo/Arduino_Core_STM32
that referenced
this issue
Mar 19, 2020
Previously, variants could define USB_DISC_PIN when they had an USB attach pullup that was disabled by default and needed a pin to be written LOW to enable it. Other hardware configurations could only overwrite the USBD_reenumerate function, like the M200 board did. This commit makes the pullup configuration more flexible. By defining the appropriate macros, enabled-by-default and disabled-by-default pullups are both supported. The output level of the pin can also be configured. Finally, for disabled-by-default, the output mode for the enabled state can also be configured (INPUT to leave the pin floating and let any external transistor gate pullup enable the USB pullup, or OUTPUT to keep the pin as OUTPUT an actively drive the transistor gate). The new macros also take PinName constants rather than pin numbers. This allows merging the code that controls an external pullup with the code that writes directly to the USB DP pin to fake a disabled pullup (this code only knows the pin name of the DP pin, not the number). Finally, for CPUs that have internal USB pullup (as indicated by the presence of a SDIS config bit), the write-to-DP-trick is now not performed (since the pullup is automatically managed by the USB hardware already). This fixes stm32duino#885, also see that issue for discussion leading up to this change.
matthijskooijman
added a commit
to 3devo/Arduino_Core_STM32
that referenced
this issue
Mar 19, 2020
Previously, variants could define USB_DISC_PIN when they had an USB attach pullup that was disabled by default and needed a pin to be written LOW to enable it. Other hardware configurations could only overwrite the USBD_reenumerate function, like the M200 board did. This commit makes the pullup configuration more flexible. By defining the appropriate macros, enabled-by-default and disabled-by-default pullups are both supported. The output level of the pin can also be configured. Finally, for disabled-by-default, the output mode for the enabled state can also be configured (INPUT to leave the pin floating and let any external transistor gate pullup enable the USB pullup, or OUTPUT to keep the pin as OUTPUT an actively drive the transistor gate). The new macros also take PinName constants rather than pin numbers. This allows merging the code that controls an external pullup with the code that writes directly to the USB DP pin to fake a disabled pullup (this code only knows the pin name of the DP pin, not the number). Finally, for CPUs that have internal USB pullup (as indicated by the presence of a SDIS config bit), the write-to-DP-trick is now not performed (since the pullup is automatically managed by the USB hardware already). This fixes stm32duino#885, also see that issue for discussion leading up to this change.
|
I implemented this, see #997. Compared to my plan above, I:
|
matthijskooijman
added a commit
to 3devo/Arduino_Core_STM32
that referenced
this issue
Mar 19, 2020
Previously, variants could define USB_DISC_PIN when they had an USB attach pullup that was disabled by default and needed a pin to be written LOW to enable it. Other hardware configurations could only overwrite the USBD_reenumerate function, like the M200 board did. This commit makes the pullup configuration more flexible. By defining the appropriate macros, enabled-by-default and disabled-by-default pullups are both supported. The output level of the pin can also be configured. Finally, for disabled-by-default, the output mode for the enabled state can also be configured (INPUT to leave the pin floating and let any external transistor gate pullup enable the USB pullup, or OUTPUT to keep the pin as OUTPUT an actively drive the transistor gate). The new macros also take PinName constants rather than pin numbers. This allows merging the code that controls an external pullup with the code that writes directly to the USB DP pin to fake a disabled pullup (this code only knows the pin name of the DP pin, not the number). Finally, for CPUs that have internal USB pullup (as indicated by the presence of a SDIS config bit), the write-to-DP-trick is now not performed (since the pullup is automatically managed by the USB hardware already). This fixes stm32duino#885, also see that issue for discussion leading up to this change.
matthijskooijman
added a commit
to 3devo/Arduino_Core_STM32
that referenced
this issue
Mar 19, 2020
Previously, variants could define USB_DISC_PIN when they had an USB attach pullup that was disabled by default and needed a pin to be written LOW to enable it. Other hardware configurations could only overwrite the USBD_reenumerate function, like the M200 board did. This commit makes the pullup configuration more flexible. By defining the appropriate macros, enabled-by-default and disabled-by-default pullups are both supported. The output level of the pin can also be configured. Finally, for disabled-by-default, the output mode for the enabled state can also be configured (INPUT to leave the pin floating and let any external transistor gate pullup enable the USB pullup, or OUTPUT to keep the pin as OUTPUT an actively drive the transistor gate). The new macros also take PinName constants rather than pin numbers. This allows merging the code that controls an external pullup with the code that writes directly to the USB DP pin to fake a disabled pullup (this code only knows the pin name of the DP pin, not the number). Finally, for CPUs that have internal USB pullup (as indicated by the presence of a SDIS config bit), the write-to-DP-trick is now not performed (since the pullup is automatically managed by the USB hardware already). This fixes stm32duino#885, also see that issue for discussion leading up to this change.
matthijskooijman
added a commit
to 3devo/Arduino_Core_STM32
that referenced
this issue
Mar 19, 2020
Previously, variants could define USB_DISC_PIN when they had an USB attach pullup that was disabled by default and needed a pin to be written LOW to enable it. Other hardware configurations could only overwrite the USBD_reenumerate function, like the M200 board did. This commit makes the pullup configuration more flexible. By defining the appropriate macros, enabled-by-default and disabled-by-default pullups are both supported. The output level of the pin can also be configured. Finally, for disabled-by-default, the output mode for the enabled state can also be configured (INPUT to leave the pin floating and let any external transistor gate pullup enable the USB pullup, or OUTPUT to keep the pin as OUTPUT an actively drive the transistor gate). The new macros also take PinName constants rather than pin numbers. This allows merging the code that controls an external pullup with the code that writes directly to the USB DP pin to fake a disabled pullup (this code only knows the pin name of the DP pin, not the number). Finally, for CPUs that have internal USB pullup (as indicated by the presence of a SDIS config bit), the write-to-DP-trick is now not performed (since the pullup is automatically managed by the USB hardware already). This fixes stm32duino#885, also see that issue for discussion leading up to this change.
matthijskooijman
added a commit
to 3devo/Arduino_Core_STM32
that referenced
this issue
Mar 20, 2020
Previously, variants could define USB_DISC_PIN when they had an USB attach pullup that was disabled by default and needed a pin to be written LOW to enable it. Other hardware configurations could only overwrite the USBD_reenumerate function, like the M200 board did. This commit makes the pullup configuration more flexible. By defining the appropriate macros, enabled-by-default and disabled-by-default pullups are both supported. The output level of the pin can also be configured. Finally, for disabled-by-default, the output mode for the enabled state can also be configured (INPUT to leave the pin floating and let any external transistor gate pullup enable the USB pullup, or OUTPUT to keep the pin as OUTPUT an actively drive the transistor gate). The new macros also take PinName constants rather than pin numbers. This allows merging the code that controls an external pullup with the code that writes directly to the USB DP pin to fake a disabled pullup (this code only knows the pin name of the DP pin, not the number). Finally, for CPUs that have internal USB pullup (as indicated by the presence of a SDIS config bit), the write-to-DP-trick is now not performed (since the pullup is automatically managed by the USB hardware already). This fixes stm32duino#885, also see that issue for discussion leading up to this change.
matthijskooijman
added a commit
to 3devo/Arduino_Core_STM32
that referenced
this issue
Mar 23, 2020
Previously, variants could define USB_DISC_PIN when they had an USB attach pullup that was disabled by default and needed a pin to be written LOW to enable it. Other hardware configurations could only overwrite the USBD_reenumerate function, like the M200 board did. This commit makes the pullup configuration more flexible. By defining the appropriate macros, enabled-by-default and disabled-by-default pullups are both supported. The output level of the pin can also be configured. The code that controls an external pullup is now merged with the code that applies the D+ output trick for fixed pullups, since the behaviour is almost identical (except for reverting to INPUT mode for the D+ pin instead of inverting the output value). Finally, for CPUs that have internal USB pullup (as indicated by the presence of a SDIS or DPPU config bit), the write-to-DP-trick is now not performed, but the internal pullup is toggled instead. This fixes stm32duino#885, also see that issue for discussion leading up to this change.
matthijskooijman
added a commit
to 3devo/Arduino_Core_STM32
that referenced
this issue
Mar 28, 2020
Previously, variants could define USB_DISC_PIN when they had an USB attach pullup that was disabled by default and needed a pin to be written LOW to enable it. Other hardware configurations could only overwrite the USBD_reenumerate function, like the M200 board did. This commit makes the pullup configuration more flexible. By defining the appropriate macros, enabled-by-default and disabled-by-default pullups are both supported. The output level of the pin can also be configured. The code that controls an external pullup is now merged with the code that applies the D+ output trick for fixed pullups, since the behaviour is almost identical (except for reverting to INPUT mode for the D+ pin instead of inverting the output value). Finally, for CPUs that have internal USB pullup (as indicated by the presence of a SDIS or DPPU config bit), the write-to-DP-trick is now not performed, but the internal pullup is toggled instead. This fixes stm32duino#885, also see that issue for discussion leading up to this change.
fpistm
pushed a commit
that referenced
this issue
Mar 30, 2020
Previously, variants could define USB_DISC_PIN when they had an USB attach pullup that was disabled by default and needed a pin to be written LOW to enable it. Other hardware configurations could only overwrite the USBD_reenumerate function, like the M200 board did. This commit makes the pullup configuration more flexible. By defining the appropriate macros, enabled-by-default and disabled-by-default pullups are both supported. The output level of the pin can also be configured. The code that controls an external pullup is now merged with the code that applies the D+ output trick for fixed pullups, since the behaviour is almost identical (except for reverting to INPUT mode for the D+ pin instead of inverting the output value). Finally, for CPUs that have internal USB pullup (as indicated by the presence of a SDIS or DPPU config bit), the write-to-DP-trick is now not performed, but the internal pullup is toggled instead. This fixes #885, also see that issue for discussion leading up to this change.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
I previously commented about this in the jump-to-bootloader, but I think this is something that warrants a bit more discussion, and can be solved separately as well. See #710 (comment) This is getting a bit of a long post, but I'm also trying to capture my thoughts and, while writing this, moving towards a proposal as well.
USB needs a pullup on the D+ line (or D- for low speed), which indicates the "attachment" status (i.e. pullup present signals the USB host that the device is present, while lack of a pullup detaches from the bus) and allows discriminating between low speed and fullspeed (or higher).
There are various ways this pullup can be implemented in the circuit. In particular, I've seen:
Control of this pullup happens in the
USBD_reenumerate()function, which currently handles two cases:These two cases seem to be somehwat mismatched: When USB_DISC_PIN is defined, the function actually only attaches to the USB bus (which causes enumeration to happen), while otherwise the function actually detaches and reattaches (which causes reenumeration to happen). So maybe for the USB_DISC_PIN case, it should be driven output high (or input/floating) for a short while before switching to low, so it actually causes reenumeration if already enumerated (e.g. when called later after startup).
Note that the internal pullup and default-on pullup configurations are not currently handled.
More generally, you might want to have an API that actually just supports "Attach" and "Detach" operations, so any caller can decide what they need. However, this is problematic because:
So, it is probably better to not support separate attach/detach, but instead have
USB_reenumerate()(possibly under a different name) just handle all usecases that need to be handled. Looking at those, I can see:USBD_reenumerate()is at startup).Looking at the above, it seems that the first three cases are pretty identical (the third one needs some extra work in case no reset happened beforehand, but that extra work should not hurt if a reset happened, so maybe best to just always do that). The fourth usecase is different, but also not currently supported. Hence,
USBD_reenumerate()could be made to support the first three cases, and then we could (if we need it later) addUSBD_detach()or something to support the fourth case.This means that the contract of
USBD_reenumerate()can be:This function should then be called from the startup/USB code, it is not intended to be called from user code directly.
This supports the first three usecases (though it might need some extra work with internal pullups for the third usecase, reenumerating during runtime, when that is implemented).
Specifying the configuration
Currently, only two configurations are handled, based on the
USB_DISC_PINmacro: External fixed pullup, and external switched pullup that is detached by default and attaches on OUTPUT LOW. I do not really like theUSB_DISC_PINmacro name here (it suggests "disconnect pin", while the pin really connects rather than disconnects). Also, it supports only one specific external pullup configuration. I would propose generalizing this by using a few macros. In particular, here's what I would add to thevariant.htemplate:I think this could also be used to simplify the MALYANM200 variant, which currently has a customized
USBD_reenumerate()function because, I think, it has an active-high attach pin (Maybe @xC0000005 can confirm that the pullup is indeed disabled by default and enabled by making PB9 high?).Then, we also need to handle the internal pullup case. Ideally, we would autodetect that internal pullups are supported. We could check for the
USB_OTG_DCTL_SDISmacro, which is the Soft Disconnect bit - if you can soft disconnect, that probably means that pullups are builtin. Note sure if this catches all of them, though.If there are no internal pullups and
USB_ATTACH_PIN/USB_DETACH_PINare not defined, we can fall back to the DP-pin-low hack. Alternatively, we could say that such a hack should not be a default and thus must be explicitly enabled for a board, but I guess that does not add so much (we can always add a macro to disable it later when needed).Summarizing
It's become a bit of a wall of text, but unless I missed something above, I think this should be something we can implement now. In particular, this needs:
USBD_reenumerate()to:The text was updated successfully, but these errors were encountered: