-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Add common hardware alocator for nRF5x TWI and SPI instances #4491
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just minor style comments
@@ -87,6 +89,11 @@ typedef struct { | |||
nrf_drv_spis_t slave; | |||
} sdk_driver_instances_t; | |||
|
|||
typedef struct { | |||
uint8_t hw_idx; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 spaces here?
@@ -219,6 +239,18 @@ static nrf_drv_spis_event_handler_t const m_slave_event_handlers[SPIS_COUNT] = { | |||
#endif | |||
}; | |||
|
|||
static sdk_driver_instances_t const * get_driver_pattern(uint8_t hw_idx) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new line {
int8_t idx = instance_hw_idx_get(HW_RESOURCE_SPI); | ||
|
||
if (idx < 0) | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if () {
style fix
This fix the issue 4357: NRF52 doesn't support simultaneous use of I2C and SPI. Now this resources are reserved by each of hal driver using shared_hw module.
@0xc0170 Could you rereview? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How was this tested?
Intention is to have different policies for peripheral types with shares the same resources. | ||
Thanks to that, maximum numbers of peripheral instances will be available. | ||
*/ | ||
static const uint8_t m_serch_policy[] = {SERCH_ASCCENDING, SERCH_DESCCENDING}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo - ascending /descending?
static uint8_t m_reservation = 0; | ||
|
||
|
||
int8_t instance_hw_idx_get(hw_resource_type_t peripheral_type) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{
newline
{ | ||
MBED_ASSERT(idx < SHARED_INSTANCES_CNT); | ||
|
||
m_reservation &= (1 << idx); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldnt you negate the number to return it to 0 ? As the above is setting it to 0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes sure!
@pan- want to review this one? |
Please see my comments at #4357 This change appears to be a serious regression. Even when assigning a single I2C bus (no SPI), "i2c->write(address, NULL, 0)" is returning 0 for any value of address. |
@nvlsianpu could you please let us know whether you are able to reproduce the problems? |
As this has serious bug - after discussion we provide fix in quite different way. |
Description
Add Support for sharing resources between SPI and TWI instancess.
This fix the issue #4357: NRF52 doesn't support simultaneous use of I2C and SPI.
Now this resources are reserved by each of hal driver using shared_hw module.
Status
READY