Skip to content

Wire::setClock() only allows exact values of 100kHz, 400kHz, and 1 MHz #68

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

Open
greiman opened this issue Jul 25, 2023 · 6 comments
Open
Labels
type: imperfection Perceived defect in any part of project

Comments

@greiman
Copy link

greiman commented Jul 25, 2023

The case statements in void TwoWire::setClock(uint32_t freq) are at odds with Wire for Uno R3.

Uno R3 allows a range of clock values.

//   Probably R3 should be range checked.
void TwoWire::setClock(uint32_t clock)
{
  twi_setFrequency(clock);
}
void twi_setFrequency(uint32_t frequency)
{
  TWBR = ((F_CPU / frequency) - 16) / 2;
}

The R4 boards only allow values in this enum:

/** Communication speed options */
typedef enum e_i2c_master_rate
{
    I2C_MASTER_RATE_STANDARD = 100000, ///< 100 kHz
    I2C_MASTER_RATE_FAST     = 400000, ///< 400 kHz
    I2C_MASTER_RATE_FASTPLUS = 1000000 ///< 1 MHz
} i2c_master_rate_t;

The I2C standard suggest you should allow the best match possible to the requested clock.

The I2C clock can be 0 Hz to 100 kHz, 0 Hz to 400 kHz, 0 Hz to 1 MHz and 0 Hz to 3.4 MHz, depending on the mode. This means that an I2C-bus running at less than 10 kHz is not SMBus compliant since the SMBus devices may time-out.

@aentinger
Copy link
Contributor

Since we are using the FSP driver code here I suggest to raise the issue over at the fsp respository.

@greiman

This comment was marked as off-topic.

@alranel
Copy link
Contributor

alranel commented Aug 2, 2023

I would keep this open until we have an upstream fix, since the issue is reflected in our API

@alranel
Copy link
Contributor

alranel commented Aug 2, 2023

I reported this upstream as per @aentinger's advice: renesas/fsp#282

@bperrybap
Copy link

One of things that was done in the chipkit core is to extend the WIRE API a bit.
On that core, setClock() returns the actual clock frequency (in Hz), which can often be different from the requested rate and that core also has a getClock() to get the current actual clock bit rate in Hz. (not the last rate requested)
These are useful to help determine what clock rate is actually being used.
I implemented the code for this on that core, so I am familiar with all the details.

@aentinger
Copy link
Contributor

I believe we'd be open to accept contributions to solve this issue, even if they bypass the FSP layer, like I've already done for SPI (see #45). Otherwise we've got to defer this until Renesas fixes it upstream 🤷 . It's a bandwidth problem 😞 .

@per1234 per1234 added the type: imperfection Perceived defect in any part of project label Aug 8, 2023
pennam pushed a commit to pennam/ArduinoCore-renesas that referenced this issue Aug 9, 2023
[Arduino FreeRTOS] Add support for Uno R4 WiFi and Minima
cristidragomir97 pushed a commit to cristidragomir97/ArduinoCore-renesas that referenced this issue May 20, 2024
[Arduino FreeRTOS] Add support for Uno R4 WiFi and Minima

Former-commit-id: bda4a18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: imperfection Perceived defect in any part of project
Projects
None yet
Development

No branches or pull requests

5 participants