I2C fix: Idle_delay added in between commands, SPI slave Fix: Format function modified and SPISLAVE enabled #3611
Conversation
|
@pradeep-gr Please fill in the template for pull requests to introduce this changeset. It helps to review , to have an overview . Thanks! (You can edit the message above). |
|
|
||
| #define I2C_API_STATUS_SUCCESS 0 | ||
| #define PAD_REG_ADRS_BYTE_SIZE 4 | ||
|
|
||
| #define SEND_COMMAND(cmd) while(!I2C_FIFO_EMPTY); wait_us(1); obj->membase->CMD_REG = cmd; |
There was a problem hiding this comment.
@0xc0170 Do we have a policy for using the wait* api in the hal? Is it fine to use or do we typically just busy loop instead?
There was a problem hiding this comment.
It is used in HAL for some targets .
I wonder why this timeout is needed .
|
Please split this commit into 3 separate commits, they are not related, thus is hard to review and integrate. headlines could be:
For instance, I don't understand why wait is being added, the commit message shall include the reasoning, as it's not clear what it fixes and why 1 us delay for a command. How was this tested? |
|
Hi Pradeep, as Martin mentioned, it is very helpful to get a description of the change (overview of what is changed and the reason) and group logically separate changes into separate commits. We just want to do a quick review of each one separately to check for known gotchas or other potential issues and collaborate with you efficiently. We really appreciate your expertise and help on this. |
|
We can split that commit as its one, should be quick. However I would like to get more details for this commit to be able to understand it better. Can you please @pradeep-gr provide some details (I asked some questions above) |
|
@0xc0170 @maclobdell |
|
/morph test |
|
Note: for the integration we shall provide better commit message |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
OutputAll builds and test passed! |
Description
I2C fix: Idle_delay added in between commands, SPI slave Fix: Format function modified and SPISLAVE enabled
Status
READY