You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/07/17 15:30:23 UTC
[GitHub] [incubator-nuttx] v01d opened a new issue #1416: nRF52 radio adaptation
v01d opened a new issue #1416:
URL: https://github.com/apache/incubator-nuttx/issues/1416
While I'm in the process of writing a BLE link-layer for NuttX (to be tested on NRF52832), I'm looking at the RADIO peripheral implementation in nrf52_radio and I would like to modify it since I believe its design is not really appropriate for a low-level peripheral interface. This is issue is to get feedback from other users of nrf52 arch before doing the change.
1. The RADIO peripheral interface is currently written as it were a driver supporting multiple instances of the RADIO (ie RADIO0, RADIO1). This makes no sense since, to my knowledge, there is no nRF52 part having more than one RADIO.
2. The interface takes care of avoiding concurrent access to peripheral via mutex and also registers its own ISR routine for handling read/write complete events, using an internal locking mutex (ie, read/write operations are blocking). This is typically done by the driver using this peripheral interface.
I would like to simplify nrf52_radio to remove all logic which assumes another RADIO instance. This means to simplify interface to be based on direct function calls and not calls via "ops" struct. Functions would still receive an instance of the internal structure which holds state. Although I suspect there's really no need to do so, since the state is usually handled by the invoking driver.
The second modification would be to remove internal mutex use and ISR registration and have read/write functions simply return and let the driver using this interface handle the ISR. This is important since for BLE I would need to be able to cancel ongoing operations if a timer expires and it is also not appropriate for the RADIO peripheral interface to handle this, since it would cram a lot of unrelated logic into a very low-level code.
I know @btashton and @raiden00pl worked on nRF52, not sure if anyone else did.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] raiden00pl commented on issue #1416: nRF52 radio adaptation
Posted by GitBox <gi...@apache.org>.
raiden00pl commented on issue #1416:
URL: https://github.com/apache/incubator-nuttx/issues/1416#issuecomment-660445400
> The RADIO peripheral interface is currently written as it were a driver supporting multiple instances of the RADIO (ie RADIO0, RADIO1). This makes no sense since, to my knowledge, there is no nRF52 part having more than one RADIO.
It's done on purpose. For now there is no multi-radio chips, but in the future who knows?
> I would like to simplify nrf52_radio to remove all logic which assumes another RADIO instance. This means to simplify interface to be based on direct function calls and not calls via "ops" struct.
Don't like this. I don't see any added value in the proposed solution and more - it is less modular.
> The second modification would be to remove internal mutex use and ISR registration and have read/write functions simply return and let the driver using this interface handle the ISR.
Remember that RADIO can be also used to implement simple proprietary protocols. The default interrupt logic can be useful in this case.
It may be better to provide a configuration option to disable default interrupts and expose the interrupts interface.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on issue #1416: nRF52 radio adaptation
Posted by GitBox <gi...@apache.org>.
v01d commented on issue #1416:
URL: https://github.com/apache/incubator-nuttx/issues/1416#issuecomment-660493212
> Don't like this. I don't see any added value in the proposed solution and more - it is less modular.
I'd say there's unnecessary complexity right now. Anyway, I can live with it.
> Remember that RADIO can be also used to implement simple proprietary protocols. The default interrupt logic can be useful in this case.
> It may be better to provide a configuration option to disable default interrupts and expose the interrupts interface.
I'm aware of other use cases but It would be best for the RADIO peripheral interface to not really implement behaviour on its own that conflicts with a given communication protocol. For BLE I will need to get tight into hardware, and the way the read/write methods are written right now (blocking on a mutex, triggered by the ISR) doesn't really allow much to be done differently one layer above. For any other protocol I would write the appropriate driver doing these things at that layer.
I will continue working on BLE and see exactly what are the changes I would need to introduce to the RADIO interface. I will open a draft PR to show how it would look. Maybe this helps to think the best way to handle this.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d commented on issue #1416: nRF52 radio adaptation
Posted by GitBox <gi...@apache.org>.
v01d commented on issue #1416:
URL: https://github.com/apache/incubator-nuttx/issues/1416#issuecomment-661066694
Great! I'll move forward then with this.
Having a driver which implements some kind of RAW protocol using RADIO peripheral would not be hard anyway.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] raiden00pl commented on issue #1416: nRF52 radio adaptation
Posted by GitBox <gi...@apache.org>.
raiden00pl commented on issue #1416:
URL: https://github.com/apache/incubator-nuttx/issues/1416#issuecomment-660822721
I thought about it again, and I agree with you on the interrupt interface. The default interrupt routine for the RADIO doesn't make sense as it is strictly dependent on the implemented protocol. So I agree with the second proposed modification.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] v01d closed issue #1416: nRF52 radio adaptation
Posted by GitBox <gi...@apache.org>.
v01d closed issue #1416:
URL: https://github.com/apache/incubator-nuttx/issues/1416
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org