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

Rewrite stm32f4 setup with svd generated registers #18

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mitchmindtree
Copy link
Contributor

@thalesfragoso this is the patch I mentioned on matrix. Curiously,
@therealprof didn't need it, and you mentioned the current setup has
worked for you in the past as well. Just to clarify, the board I'm
testing is STM32F407G-DISC1 - but I imagine behaviour should be the same
across F407s?

As you mentioned it would need either a critical section or access to
the &RCC and &SYSCFG passed as arguments to the function directly.

I think I'd opt for the latter approach, and not call this from inside
Eth::new, but rather have a separate configure_rcc (or something
like this) that the user can call separately before Eth::new is
called. If it's a separate function, the user can do

configure_rcc(&p.RCC, &p.SYSCFG);

before they set the RCC clock speeds and call freeze. If we really
wanted to make sure this was called prior to Eth::new, we could do so
by returning some type from configure_rcc that must be passed as an
argument to Eth::new.

I'll double check the behaviour of master compared to this patch again
tomorrow once I'm back at work on this. I thought I'd open this in the
meantime in case you were curious!

@thalesfragoso this is the patch I mentioned on matrix. Curiously,
@therealprof didn't need it, and you mentioned the current setup has
worked for you in the past as well. Just to clarify, the board I'm
testing is STM32F407G-DISC1 - but I imagine behaviour should be the same
across F407s?

As you mentioned it would need either a critical section or access to
the `&RCC` and `&SYSCFG` passed as arguments to the function directly.

I think I'd opt for the latter approach, and not call this from inside
`Eth::new`, but rather have a separate `configure_rcc` (or something
like this) that the user can call separately before `Eth::new` is
called. If it's a separate function, the user can do

```rust
configure_rcc(&p.RCC, &p.SYSCFG);
```

before they set the RCC clock speeds and call `freeze`. If we really
wanted to make sure this was called prior to `Eth::new`, we could do so
by returning some type from `configure_rcc` that must be passed as an
argument to `Eth::new`.

I'll double check the behaviour of `master` compared to this patch again
tomorrow once I'm back at work on this. I thought I'd open this in the
meantime in case you were curious!
@thalesfragoso
Copy link
Member

You should use modify instead of write, that can be messing with your other configurations. About going away from bit-banding, I would rather not, could you check the errata for your revision ? Maybe we can get some clue about what is going on.

@mitchmindtree
Copy link
Contributor Author

could you check the errata for your revision

Interestingly, there are quite a few entries related to Ethernet in the errata for the STM32F407. That said, it appears all of the known issues have been resolved prior to my revision (which appears to be revision 4). You can find the errata here.

I haven't yet had a chance to go through the bit const declarations, or to dig deeper into why the bb version is not working in my case, but hoping to get a chance to some point soon.

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