Fix external and internal usb pullup #1048
Conversation
|
Works for me |
These boards were broken in commit e1d409f Refactor USB pullup handling. Before that commit, all boards without external controllable pullups were assumed to have fixed external pullups and use the DP write trick. Since that commit, boards that have internal pullups are assumed to *not* have any external pullups and the internal pullups are automatically used. It turns out there exist some boards that have internal pullups in the chips, but also have an external fixed pullup. This can probably be considered a hardware bug, but since the boards exist, we should support them. This commit allows variants to define USBD_FIXED_PULLUP to explicitly state they have a fixed pullup on the D+ line. This will cause the D+ output trick to be applied even when internal pullups are present, fixing these boards. This define is only needed on these specific boards, but it can also be defined on boards with a fixed external pullup without internal pullups. This problem was prompted by the "Black F407VE" board, which has the problematic pullup. All other F4 boards were checked and one other was found to have the pullup, all others seem ok. None of the other series have been checked, so there might still be board broken. See also STM32-base/STM32-base.github.io#26 for some additional inventarisation of this problem. This fixes stm32duino#1029.
While reviewing some of these boards, it was not immediately clear to me what boards they referred to (especially with the relatively unbranded boards from aliexpress). Just in case this helps anyone else, this adds some urls with more info on those boards I found from the git history and github issues.
This board also has internal pullups, so the external one is not actually needed (and will even violate the USB spec when both are used together). This commit disabled the external pullups, but leaves the defines in comments, as future documentation. See also stm32duino#1029.
matthijskooijman
force-pushed
the
fix-external-and-internal-usb-pullup
branch
from
91a2a02 to
d3fdc1f
Compare
|
Heh, I had actually thought I fixed the indentation of that comment, didn't realize astyle had another opinion about it :-p Anyway, thanks for the fix, I've squashed it into my original commit. @stas2z Thanks for testing! |
Tested on:
|
|
Thanks for the extensive testing! |
|
Welcome. |
|
@matthijskooijman on the CoreBoard F401RC the pullup is not permanent but driven by PD2 |
Yeah, that's what I already said (or at least tried to say :-P). Regardless, that pullup is still problematic when used together with the internal pullup, so this PR changes the code to not use the external pullup anymore. If you could confirm that USB still works on this board, that would be awesome! |
|
@matthijskooijman I don't want to impact my install too much. Is changing these files enough to test? |
Yeah, I think it should be. |
|
@matthijskooijman I tried it succesfully. I hope the test is significant. Here's what I did:
|
|
Awesome, thanks! |
This fixes boards that have external and internal USB pullups, which were broken by #997, as reported in #1029.
This needs a new define in the variant for these broken boards. I added this for two boards for which I could confirm that they had an external pullup. @stas2z, you have a black-based board, could you test this PR on your board?
I found that the CoreBoard F401RC had a switchable external pullup, but also internal pullups, so I changed that one to no longer use the external pullup and just the internal pullup. @mrguen, I think you have one of these boards, could you maybe test this PR on that board?
I tested this on a custom STM32F401 board, where I added an external pullup for testing. I also tested on an unmodified Blue pill, which still works as expected.