Skip to content

cpu/sam0_common: remove unneeded GCLK_SLOW setup in i2c driver#10839

Merged
MrKevinWeiss merged 1 commit into
RIOT-OS:masterfrom
dylad:pr/sam0/remove_gclk_slow
Mar 28, 2019
Merged

cpu/sam0_common: remove unneeded GCLK_SLOW setup in i2c driver#10839
MrKevinWeiss merged 1 commit into
RIOT-OS:masterfrom
dylad:pr/sam0/remove_gclk_slow

Conversation

@dylad

@dylad dylad commented Jan 22, 2019

Copy link
Copy Markdown
Member

Contribution description

This PR removes the SERCOMx_GLCK_SLOW clock setup from the I2C driver.
According to SAML21, SAMD21 and SAML10 datasheets, this clock is used for SMBus timeouts features but RIOT doesn't use them at all.
Look for GCLK_SERCOM_SLOW within the datasheet for more information.
In addition, this clock should be feed with 32kHz clock source not xx MHz (which is the case now)

Testing procedure

Pick the SAM0 board you want, play with any I2C sensors.

Issues/PRs references

Found this while adding I2C support for SAML10/SAML11 in #10653

@dylad dylad added Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR TF: I2C Marks issues and PRs related to the work of the I²C rework task force Area: cpu Area: CPU/MCU ports labels Jan 22, 2019
@dylad dylad requested review from MrKevinWeiss and aabadie January 22, 2019 08:42
@MrKevinWeiss

Copy link
Copy Markdown
Contributor

It makes sense but I will need some time before I can really test it.

@dylad

dylad commented Jan 23, 2019

Copy link
Copy Markdown
Member Author

It makes sense but I will need some time before I can really test it.

Thanks @MrKevinWeiss ! This is not an urgent PR. This can be done after the release.

@dylad dylad added this to the Release 2019.04 milestone Feb 2, 2019
@fedepell

Copy link
Copy Markdown
Contributor

I've tested this on a SAML21 based custom board that has some EEPROM chips on I2C and all seems fine

@dylad

dylad commented Mar 27, 2019

Copy link
Copy Markdown
Member Author

Thanks for testing @fedepell !

@MrKevinWeiss

Copy link
Copy Markdown
Contributor

Uh oh, I can test when I get back. Sorry I forgot about it.

@MrKevinWeiss

Copy link
Copy Markdown
Contributor

I can get to it on Thursday or Friday!

@dylad

dylad commented Mar 27, 2019

Copy link
Copy Markdown
Member Author

@MrKevinWeiss this would be perfect !

@MrKevinWeiss MrKevinWeiss added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Mar 28, 2019

@MrKevinWeiss MrKevinWeiss left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on the samr30-xpro with the scope and it shows 100kHz, the datasheets indicated that it isn't needed for this application. It appears it got merged in with initial commits.

ACK!.

@MrKevinWeiss MrKevinWeiss merged commit 15c2a48 into RIOT-OS:master Mar 28, 2019
@dylad

dylad commented Mar 28, 2019

Copy link
Copy Markdown
Member Author

Thanks for merging this !

@dylad dylad deleted the pr/sam0/remove_gclk_slow branch March 28, 2019 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: cpu Area: CPU/MCU ports CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer TF: I2C Marks issues and PRs related to the work of the I²C rework task force Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants