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

[greiman]

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]

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

[greiman]

Doesn't matter, too many problems for me to use R4.

[alranel]

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

[alranel]

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

[bperrybap]

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]

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 😞 .