I2C issue at 1 MHz clock.

[SBKrogh]

When running the ATTiny402 at a clock frequency of 1 MHz, the I2C CLK will drop down to ~ 1.9 kHz.
I have isolated the issue to the twi.c - void TWI_MasterSetBaud(uint32_t frequency) .

If the frequency, in the equation below, is lower than ~83 kHz, the equation will result in a negative value but due to the uint32_t baud will be assigned a large value.

uint32_t baud = (F_CPU / frequency - F_CPU / 1000 / 1000 * t_rise / 1000 - 10) / 2;

I have looked through the ATTiny402 datasheet and it seems the equation given at p. 350 f_scl is not in compliance with the above equation.
http://ww1.microchip.com/downloads/en/DeviceDoc/ATtiny202-402-AVR-MCU-with-Core-Independent-Peripherals_and-picoPower-40001969A.pdf

---

To people who need a fast fix.
This can be "fixed" by directly writing the register
TWI0.MBAUD = 1; // Increase I2C clk to ~83 kHz @ 1 MHz internal clock
at the end of void setup(), but this is also an ugly solution - fast but ugly ;)

[SpenceKonde]

uuuuuuggghhhh

Actually that equation is exactly the same as the one in the datasheet. To get that equation from the one in the book, take the two terms out of the parens giving F_CPU/2Freq -5 - F_CPU*Trise/2.multiply each term by 2 and divide whole thing by 2 to yield

(F_CPU / frequency - F_CPU*t_rise - 10) / 2
The equation in the book expects tRise to be in seconds, not nanoseconds. 1 second is 1 billion nanoseconds. But we need to avoid any intermediate getting too close to either 0 or overflowing, Hence hideous games with 1000.

The problem appears to be immediately before it, where the rise time and frequency is set like so:

  uint16_t t_rise;

  if (frequency < 200000) {
    frequency = 100000;
    t_rise    = 1000;

  } else if (frequency < 800000) {
    frequency = 400000;
    t_rise    = 300;

  } else if (frequency < 1200000) {
    frequency = 1000000;
    t_rise    = 120;

  } else {
    frequency = 100000;
    t_rise    = 1000;
  }

  uint32_t baud = (F_CPU / frequency - F_CPU / 1000 / 1000 * t_rise / 1000 - 10) / 2;
  TWI0.MBAUD = (uint8_t)baud;

Why the person who wrote this implemenetation stored it as a uint32 on one line, despite being a local variable that gets written to an 8-bit register on the next lline, which in turn is the last line of the function is a bit mystifying.

I would interpret the negative =value of baud

[SpenceKonde]

For fucks sake... Wire.h provides a Wire.setClock. Which takes any value you can dream of. Then passes it to that function, which sorts it into one of three boxes, none of which are compatible with 1 MHz operation.

[SpenceKonde]

Huh, so I2C has never been in spec on any of these parts at any speed. Obviously when it calculates a baud rate tat is non negative, though, ithas a better shot at working. They all fail the Tlow test described further down the datasheet.

[SpenceKonde]

(they were using the wrong limiting condition to calculate the BAUD value)

Okay fuck trying to calculate it on the fly, that's a godawful calculation to try to do in integer math while remaining cognizant of the fact that you['re running it on an 8-bit AVR. I'll still give more choice to the user than than the 3 buckets they used... .

[SpenceKonde]

The code that is now checked in has what I think is a fix, I have not tested it with hardware - can you test it? Ideally at a range of speeds? I really ought to have a set of test rigs for I2C and others, but.... there are many things I should have b uilt for supporting core development.

[SBKrogh]

That was fast and sorry for the missing replies. Hope it'll work.
I'll test it today or over the weekend when I can find the time needed for it.

Will a step sizes of 10 kHz be sufficient from 10kHz to 100 kHz?

[SpenceKonde]

There are 6 buckets you get sorted into, based on the frequency you request. I was not able to get the formulas to give useful results, because some of the parameters require measuring the electrical properties of the network of devices you have ti connected to, or pulling numbers out of your ass. What is the rise time? it looks to me like BAUD sets the time that it spends at highs and lows, but not the time that it spends transitioning, which alters the actual frequency

50 kHz, (half speed standard) 100kHz (full speed standard), 200 kHz (half speed fast mode(, 400 kHz (full fast mode),. and, if either FMPEN is set, or master is not yet enabled there's a sub-FM+ and full speed FM+ option.

I actually just pushed a very important fix >.> Make sure you have the fix I just pushed!

(frequency < 80000 = 50 kHz
frequency <= 150000 = 100kHz
frequency <= 300000 = 200 kHz
frequency <= 600000 || or already enabled with FMPEN not set so no FM+ speeds are available - 400kHz
frequency< 900000 750k FastMode+
frequency > 900000 ~ 1 MHz FastMode+

All of the calculations are done with hand-calculated linearizations of that nasty formula. modified to generate values that consistently met the low time constraint as well.

[SpenceKonde]

1 speed per bucket is fine, but I'd love to see some tests at higher system clocks rto we know I haven'rt broken that.

[SBKrogh]

Would you please specify the page with the low time constraint?

Version: 2.3.1
So I tried the below dummy code.
Either I'm missing something or the Wire.setClock(uint32_t) still has some issues.

#include <Wire.h>

const int SDA_I2C = 2;              // I2C data pin
const int SCL_I2C = 3;              // I2C clk pin

void setup() {
  Wire.begin();
  
  pinMode(SDA_I2C,    INPUT);
  pinMode(SCL_I2C,    INPUT);
  digitalWrite(SDA_I2C, HIGH);
  digitalWrite(SCL_I2C, HIGH);
  
}

void loop() {
  
  Wire.setClock((uint32_t)80000);
  delay(10);
  Wire.beginTransmission((uint8_t)1);//reading); // transmit to device #8
  Wire.write(0x1);              // sends one byte
  Wire.endTransmission();
  delay(100);

  Wire.setClock((uint32_t)150000);
  delay(10);
  Wire.beginTransmission((uint8_t)2);//reading); // transmit to device #8
  Wire.write(0x1);              // sends one byte
  Wire.endTransmission();
  delay(100);

  Wire.setClock((uint32_t)300000);
  delay(10);
  Wire.beginTransmission((uint8_t)3);//reading); // transmit to device #8
  Wire.write(0x1);              // sends one byte
  Wire.endTransmission();
  delay(100);

  Wire.setClock((uint32_t)600000);
  delay(10);
  Wire.beginTransmission((uint8_t)4);//reading); // transmit to device #8
  Wire.write(0x1);              // sends one byte
  Wire.endTransmission();
  delay(100);

  Wire.setClock((uint32_t)900000);
  delay(10);
  Wire.beginTransmission((uint8_t)5);//reading); // transmit to device #8
  Wire.write(0x1);              // sends one byte
  Wire.endTransmission();
  delay(100);
  Wire.setClock((uint32_t)1000000);
  delay(10);
  Wire.beginTransmission((uint8_t)6);//reading); // transmit to device #8
  Wire.write(0x1);              // sends one byte
  Wire.endTransmission();
  delay(100);
}

Main Clock 20 MHz
input || I2C_clk
80000 => 98 kHz
150000 => 98 kHz
300000 => 300 kHz
600000 => 300 kHz
900000 => 495 kHz
1000000 => 507 kHz

Main Clock 5 MHz
input || I2C_clk
80000 => 101 kHz
150000 => 101 kHz
300000 => 331 kHz
600000 => 331 kHz
900000 => 9.58 kHz
1000000 => 9.58 kHz

Main Clock 1 MHz
input || I2C_clk
80000 => 1.94 kHz
150000 => 1.94 kHz
300000 => 1.94 kHz
600000 => 1.94 kHz
900000 => 1.94 kHz
1000000 => 1.94 kHz

[SpenceKonde]

The lowest speed buicklet is when you specify LESS than 80000, so that hasn't been tested.

Also, you haven't tested anything that's actually within the non-full-speed FM+ mode. 600000 gets given fast mode 400khz. and 900000 gets given full speed fast mode.. or in theory it does, that doesn't look full speed rto mwe

. And there is a minimum clock frequency below which I wasn't able to solve the equations. I have it as 4 MHz can only to halved fast mode, 8 MHz required for 400 khz, and 10 and 12 for the .775 MHz and 1 MHz options.  Iit sis supposed to pick the fastest that the child can accommodate in these cases.What a nightmare. 

What values is TWI0.MBAUD set to after "setting it" when it's coming out to sub-10 kHz values?

[SBKrogh]

Not sure what to write for testing what you ask for i.e non-full-speed FM+ mode.

Main Clock 1 MHz
input || I2C_clk
70000 => 1.94 kHz
TWI0.MBAUD is 255;

Main Clock 5 MHz
70000 => 101 kHz

Main Clock 20 MHz
70000 => 98 kHz

[Modac]

I tested the SCL frequency of version 2.3.1 and the newest fix (including #403).
For version 2.3.1 I can confirm that the speeds are kind of broken.

For the new fix I compiled this table of CPU frequencies, desired SCL frequencies and the actual measured frequencies.
The desired frequency was set with setClock() immediately after begin().
If another frequency was measured when FM+ was enabled before begin() these are the values in the brackets.

F_CPU/TWI Freq	50 kHz	100 kHz	200 kHz	400 kHz	750 kHz	1 MHz
20 MHz	~49 kHz	~96 kHz	~156 kHz	~420 kHz	~420(595) kHz	~420(675) kHz
16 MHz	~49 kHz	~97 kHz	~157 kHz	~420 kHz	~420(620) kHz	~420(670) kHz
10 MHz	~49 kHz	~97 kHz	~157 kHz	~420 kHz	~420(630) kHz	~420 kHz
8 MHz	~50 kHz	~97 kHz	~158 kHz	~425 kHz	~425 kHz	~425 kHz
5 MHz	~49 kHz	~96 kHz	~157 kHz	~157 kHz	~157 kHz	~157 kHz
4 MHz	~50 kHz	~99 kHz	~162 kHz	~162 kHz	~162 kHz	~162 kHz
1 MHz	~51 kHz	~51 kHz	~51 kHz	~51 kHz	~51 kHz	~51 kHz

It seems to be working as intended. The only thing I think is a bit out of place is that the frequency at (10Mhz, 1Mhz) doesn't change when FM+ is enabled even though it does change at (10Mhz, 750kHz).

[SpenceKonde]

Wow @Modac - you win the award here,. Thank you!

Best of all, plugging in a constant rise time (as opposed to the worst-case-that-complies-with-standard one used in the old calculations) and using the formula in the datasheet--- if the rise time in your setup was 300ns or so, exactly the speeds you recorded would be predicted. Seeing the values you observed calculated with physically reasonable assumption of constant Trise is comforting.

I've adjusted the algorithm slightly here - I applied basic algebra to the formulas to make them less convoluted (no practical impact as all m,ath is done at compile time not run time). Was surprised that the "1 MHz" wasn;t faster; adjusted that so it comes out closer to 1 MHz, particularly when F_CPU >= 20MHz. I also fixed the case you cited at at 10 MHz with FMP+ bit set at full speed not at least giving you FMP at partial speed.

It also now guards against BAUD < 3. rather just catching BAUD < 1 -BAUD of 1 or 2 is changed to 3 (required by some parts per datasheet, and in practice probably all). If BAUD would ever be set to less than 1, the user requested a speed that the hardware isn't clocked fast enough to provide, so we do the fastest non-FMP mode that their hardware can provide for them.

Also fixed Wire.cpp so that it won't let you call Wire.swap(1) or Wire.pins() with pins other than the default ones on parts that don't have alternate pin mappings - if the argument is both constant and we know they're asking for the impossible, we error on compile. If they camouflage it such that we don't know they're passing bogus values to it at compile time, they will still get the true/false return value. As if anyone checks them.

[Modac]

I'm glad I can help.
Yeah, I totally forgot to mention my testing setup. It's basically just the Attiny412 as the master, a Pro Micro as the slave connected via jumper wires and 10k pullups. That's why the "fast" speeds are relatively low.
As a final few tests I ran the same test as in my previous comment, but also tested the speeds with 2k pullups instead of 10k, just for the fun of it.

setClock() after begin(), FM+ enabled in brackets, 10k pullups:

F_CPU/TWI Freq	50 kHz	100 kHz	200 kHz	400 kHz	750 kHz	1 MHz
20 MHz	~49 kHz	~96 kHz	~156 kHz	~420 kHz	~420(595) kHz	~420(775) kHz
16 MHz	~49 kHz	~97 kHz	~157 kHz	~420 kHz	~420(620) kHz	~420(730) kHz
10 MHz	~49 kHz	~97 kHz	~157 kHz	~420 kHz	~420(510) kHz	~420(510) kHz
8 MHz	~50 kHz	~97 kHz	~159 kHz	~425 kHz	~425 kHz	~425 kHz
5 MHz	~49 kHz	~96 kHz	~157 kHz	~157 kHz	~157 kHz	~157 kHz
4 MHz	~50 kHz	~99 kHz	~162 kHz	~162 kHz	~162 kHz	~162 kHz
1 MHz	~51 kHz	~51 kHz	~51 kHz	~51 kHz	~51 kHz	~51 kHz

FM+ enabled, setClock() after begin(), 2k pullups:

F_CPU/TWI Freq	50 kHz	100 kHz	200 kHz	400 kHz	750 kHz	1 MHz
20 MHz	~50 kHz	~98 kHz	~162 kHz	~465 kHz	~690 kHz	~950 kHz
16 MHz	~50 kHz	~99 kHz	~164 kHz	~475 kHz	~730 kHz	~890 kHz
10 MHz	~50 kHz	~99 kHz	~164 kHz	~480 kHz	~590 kHz	~590 kHz
8 MHz	~50 kHz	~100 kHz	~165 kHz	~475 kHz	~475 kHz	~475 kHz
5 MHz	~50 kHz	~100 kHz	~167 kHz	~167 kHz	~167 kHz	~167 kHz
4 MHz	~51 kHz	~101 kHz	~169 kHz	~169 kHz	~169 kHz	~169 kHz
1 MHz	~51 kHz	~51 kHz	~51 kHz	~51 kHz	~51 kHz	~51 kHz

So as you mentioned the (10Mhz, 1Mhz) case is now fixed and the 1Mhz speeds got closer to the desired 1Mhz. With 2k pullups it even reached about 950kHz at 20Mhz for my setup.
All in all a pretty solid implementation now, in my opinion.