-
Notifications
You must be signed in to change notification settings - Fork 39
Add setClockDivider. Fix polarity. #30
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
Conversation
We can't put bitOrder in the config structure because the HAL defines am_hal_iom_config_t. So instead we pass it by argument to the two initialize functions. Not elegant but functional.
One big pitfall was the lack of return AP3_OK from initialize. The lack of it did not throw an error but caused very weird behavior. Where else do we not have return values?
Note that transfer16 uses bitOrder to indicate byte order. I think this is an error but I'm not sure what you were intending. If transfer16() is a new unique function to Apollo3 I would suggest adding a bool called byteOrder that is passed in rather than relying on global var. |
Thanks for the PR, looks like a good functionality boost. Here's where Right now this PR breaks "Wire" because both Wire and SPI use the "ap3_iomaster" class, but Wire does not provide the 'bit order' constructor argument. Does the bit order bit apply to both I2C and SPI? If it only applies to SPI then I'd prefer to keep that isolated to the SPI library for now - and ultimately it would be good to get HAL support for the bit order. If not then we can add 'bit order' to the Wire library. |
Looks like the API for SPILSB is available, so let's use that in the SPI library and keep "iomaster" and "Wire" untouched. How does that sound? |
// void setClockDivider(uint8_t uc_div); | ||
void setClockDivider(uint8_t div); | ||
|
||
void _transfer(void *buf_out = NULL, void *buf_in = NULL, size_t count = 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.
Not taking this change since '_transfer' is meant to be for internal use of the SPI ibrary
@@ -104,22 +114,22 @@ class SPIClass : public IOMaster { | |||
|
|||
void setBitOrder(BitOrder order); | |||
void setDataMode(uint8_t uc_mode); | |||
// void setClockDivider(uint8_t uc_div); | |||
void setClockDivider(uint8_t div); |
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.
Got it
// // _p_sercom->setBaudrateSPI(div); | ||
// // } | ||
// } | ||
void SPIClass::setClockDivider(uint8_t div) |
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.
Got these changes
I just took the meat of these changes and committed them with 74fb873. (Did this because there were many changes to master since this PR and I also did not want the bitOrder percolating up into the IOMaster code.) Closing. Thanks! |
One big pitfall was the lack of return AP3_OK from initialize(). The lack of it did not throw an error but caused very weird behavior. Where else do we not have return values?
I've got a request into Ambiq for HAL change to expose SPI LSB/MSB. For now, it's a hard coded register address that should work across all 6 IOM instances.