Message ID | 20241021062732.5592-4-kfting@nuvoton.com |
---|---|
State | Under Review |
Delegated to: | Andi Shyti |
Headers | show |
Series | i2c: npcm: read/write operation, checkpatch | expand |
Hi Tyrone, ... > - /* 100KHz and below: */ > - if (bus_freq_hz <= I2C_MAX_STANDARD_MODE_FREQ) { > - sclfrq = src_clk_khz / (bus_freq_khz * 4); > - > - if (sclfrq < SCLFRQ_MIN || sclfrq > SCLFRQ_MAX) > - return -EDOM; > - > - if (src_clk_khz >= 40000) > - hldt = 17; > - else if (src_clk_khz >= 12500) > - hldt = 15; > - else > - hldt = 7; > - } > - > - /* 400KHz: */ > - else if (bus_freq_hz <= I2C_MAX_FAST_MODE_FREQ) { > - sclfrq = 0; > + switch (bus_freq_hz) { > + case I2C_MAX_STANDARD_MODE_FREQ: > + smb_timing = smb_timing_100khz; > + table_size = ARRAY_SIZE(smb_timing_100khz); > + break; > + case I2C_MAX_FAST_MODE_FREQ: > + smb_timing = smb_timing_400khz; > + table_size = ARRAY_SIZE(smb_timing_400khz); > fast_mode = I2CCTL3_400K_MODE; > - > - if (src_clk_khz < 7500) > - /* 400KHZ cannot be supported for core clock < 7.5MHz */ > - return -EDOM; > - > - else if (src_clk_khz >= 50000) { > - k1 = 80; > - k2 = 48; > - hldt = 12; > - dbnct = 7; > - } > - > - /* Master or Slave with frequency > 25MHz */ > - else if (src_clk_khz > 25000) { > - hldt = clk_coef(src_clk_khz, 300) + 7; > - k1 = clk_coef(src_clk_khz, 1600); > - k2 = clk_coef(src_clk_khz, 900); > - } > - } > - > - /* 1MHz: */ > - else if (bus_freq_hz <= I2C_MAX_FAST_MODE_PLUS_FREQ) { > - sclfrq = 0; > + break; > + case I2C_MAX_FAST_MODE_PLUS_FREQ: > + smb_timing = smb_timing_1000khz; > + table_size = ARRAY_SIZE(smb_timing_1000khz); > fast_mode = I2CCTL3_400K_MODE; > - > - /* 1MHZ cannot be supported for core clock < 24 MHz */ > - if (src_clk_khz < 24000) > - return -EDOM; > - > - k1 = clk_coef(src_clk_khz, 620); > - k2 = clk_coef(src_clk_khz, 380); > - > - /* Core clk > 40 MHz */ > - if (src_clk_khz > 40000) { > - /* > - * Set HLDT: > - * SDA hold time: (HLDT-7) * T(CLK) >= 120 > - * HLDT = 120/T(CLK) + 7 = 120 * FREQ(CLK) + 7 > - */ > - hldt = clk_coef(src_clk_khz, 120) + 7; > - } else { > - hldt = 7; > - dbnct = 2; > - } > + break; > + default: > + return -EINVAL; There is here a slight change of behaiour which is not mentioned in the commit log. Before the user could set a bus_freq_hz which had to be <= I2C_MAX_..._MODE_FREQ, while now it has to be precisely that. Do we want to check what the user has set in the DTS? (Or am I missing something?) Thanks, Andi
Hi Andi: Thank you for your comments. Andi Shyti <andi.shyti@kernel.org> 於 2024年10月24日 週四 下午6:20寫道: > > Hi Tyrone, > > ... > > > - /* 100KHz and below: */ > > - if (bus_freq_hz <= I2C_MAX_STANDARD_MODE_FREQ) { > > - sclfrq = src_clk_khz / (bus_freq_khz * 4); > > - > > - if (sclfrq < SCLFRQ_MIN || sclfrq > SCLFRQ_MAX) > > - return -EDOM; > > - > > - if (src_clk_khz >= 40000) > > - hldt = 17; > > - else if (src_clk_khz >= 12500) > > - hldt = 15; > > - else > > - hldt = 7; > > - } > > - > > - /* 400KHz: */ > > - else if (bus_freq_hz <= I2C_MAX_FAST_MODE_FREQ) { > > - sclfrq = 0; > > + switch (bus_freq_hz) { > > + case I2C_MAX_STANDARD_MODE_FREQ: > > + smb_timing = smb_timing_100khz; > > + table_size = ARRAY_SIZE(smb_timing_100khz); > > + break; > > + case I2C_MAX_FAST_MODE_FREQ: > > + smb_timing = smb_timing_400khz; > > + table_size = ARRAY_SIZE(smb_timing_400khz); > > fast_mode = I2CCTL3_400K_MODE; > > - > > - if (src_clk_khz < 7500) > > - /* 400KHZ cannot be supported for core clock < 7.5MHz */ > > - return -EDOM; > > - > > - else if (src_clk_khz >= 50000) { > > - k1 = 80; > > - k2 = 48; > > - hldt = 12; > > - dbnct = 7; > > - } > > - > > - /* Master or Slave with frequency > 25MHz */ > > - else if (src_clk_khz > 25000) { > > - hldt = clk_coef(src_clk_khz, 300) + 7; > > - k1 = clk_coef(src_clk_khz, 1600); > > - k2 = clk_coef(src_clk_khz, 900); > > - } > > - } > > - > > - /* 1MHz: */ > > - else if (bus_freq_hz <= I2C_MAX_FAST_MODE_PLUS_FREQ) { > > - sclfrq = 0; > > + break; > > + case I2C_MAX_FAST_MODE_PLUS_FREQ: > > + smb_timing = smb_timing_1000khz; > > + table_size = ARRAY_SIZE(smb_timing_1000khz); > > fast_mode = I2CCTL3_400K_MODE; > > - > > - /* 1MHZ cannot be supported for core clock < 24 MHz */ > > - if (src_clk_khz < 24000) > > - return -EDOM; > > - > > - k1 = clk_coef(src_clk_khz, 620); > > - k2 = clk_coef(src_clk_khz, 380); > > - > > - /* Core clk > 40 MHz */ > > - if (src_clk_khz > 40000) { > > - /* > > - * Set HLDT: > > - * SDA hold time: (HLDT-7) * T(CLK) >= 120 > > - * HLDT = 120/T(CLK) + 7 = 120 * FREQ(CLK) + 7 > > - */ > > - hldt = clk_coef(src_clk_khz, 120) + 7; > > - } else { > > - hldt = 7; > > - dbnct = 2; > > - } > > + break; > > + default: > > + return -EINVAL; > > There is here a slight change of behaiour which is not mentioned > in the commit log. Before the user could set a bus_freq_hz which > had to be <= I2C_MAX_..._MODE_FREQ, while now it has to be > precisely that. > > Do we want to check what the user has set in the DTS? The driver checks the bus frequency the user sets in the DTS. Please refer to the links: 1. https://github.com/torvalds/linux/blob/master/drivers/i2c/busses/i2c-npcm7xx.c#L1995 2. https://github.com/torvalds/linux/blob/master/drivers/i2c/busses/i2c-npcm7xx.c#L2002 > > (Or am I missing something?) > > Thanks, > Andi Thank you. Regards, Tyrone
Hi Andi: May I have your comments about my feedback on these patches? Tyrone Ting <warp5tw@gmail.com> 於 2024年10月25日 週五 上午9:46寫道: > > Hi Andi: > > Thank you for your comments. > > Andi Shyti <andi.shyti@kernel.org> 於 2024年10月24日 週四 下午6:20寫道: > > > > Hi Tyrone, > > > > ... > > > > > - /* 100KHz and below: */ > > > - if (bus_freq_hz <= I2C_MAX_STANDARD_MODE_FREQ) { > > > - sclfrq = src_clk_khz / (bus_freq_khz * 4); > > > - > > > - if (sclfrq < SCLFRQ_MIN || sclfrq > SCLFRQ_MAX) > > > - return -EDOM; > > > - > > > - if (src_clk_khz >= 40000) > > > - hldt = 17; > > > - else if (src_clk_khz >= 12500) > > > - hldt = 15; > > > - else > > > - hldt = 7; > > > - } > > > - > > > - /* 400KHz: */ > > > - else if (bus_freq_hz <= I2C_MAX_FAST_MODE_FREQ) { > > > - sclfrq = 0; > > > + switch (bus_freq_hz) { > > > + case I2C_MAX_STANDARD_MODE_FREQ: > > > + smb_timing = smb_timing_100khz; > > > + table_size = ARRAY_SIZE(smb_timing_100khz); > > > + break; > > > + case I2C_MAX_FAST_MODE_FREQ: > > > + smb_timing = smb_timing_400khz; > > > + table_size = ARRAY_SIZE(smb_timing_400khz); > > > fast_mode = I2CCTL3_400K_MODE; > > > - > > > - if (src_clk_khz < 7500) > > > - /* 400KHZ cannot be supported for core clock < 7.5MHz */ > > > - return -EDOM; > > > - > > > - else if (src_clk_khz >= 50000) { > > > - k1 = 80; > > > - k2 = 48; > > > - hldt = 12; > > > - dbnct = 7; > > > - } > > > - > > > - /* Master or Slave with frequency > 25MHz */ > > > - else if (src_clk_khz > 25000) { > > > - hldt = clk_coef(src_clk_khz, 300) + 7; > > > - k1 = clk_coef(src_clk_khz, 1600); > > > - k2 = clk_coef(src_clk_khz, 900); > > > - } > > > - } > > > - > > > - /* 1MHz: */ > > > - else if (bus_freq_hz <= I2C_MAX_FAST_MODE_PLUS_FREQ) { > > > - sclfrq = 0; > > > + break; > > > + case I2C_MAX_FAST_MODE_PLUS_FREQ: > > > + smb_timing = smb_timing_1000khz; > > > + table_size = ARRAY_SIZE(smb_timing_1000khz); > > > fast_mode = I2CCTL3_400K_MODE; > > > - > > > - /* 1MHZ cannot be supported for core clock < 24 MHz */ > > > - if (src_clk_khz < 24000) > > > - return -EDOM; > > > - > > > - k1 = clk_coef(src_clk_khz, 620); > > > - k2 = clk_coef(src_clk_khz, 380); > > > - > > > - /* Core clk > 40 MHz */ > > > - if (src_clk_khz > 40000) { > > > - /* > > > - * Set HLDT: > > > - * SDA hold time: (HLDT-7) * T(CLK) >= 120 > > > - * HLDT = 120/T(CLK) + 7 = 120 * FREQ(CLK) + 7 > > > - */ > > > - hldt = clk_coef(src_clk_khz, 120) + 7; > > > - } else { > > > - hldt = 7; > > > - dbnct = 2; > > > - } > > > + break; > > > + default: > > > + return -EINVAL; > > > > There is here a slight change of behaiour which is not mentioned > > in the commit log. Before the user could set a bus_freq_hz which > > had to be <= I2C_MAX_..._MODE_FREQ, while now it has to be > > precisely that. > > > > Do we want to check what the user has set in the DTS? > > The driver checks the bus frequency the user sets in the DTS. > > Please refer to the links: > 1. https://github.com/torvalds/linux/blob/master/drivers/i2c/busses/i2c-npcm7xx.c#L1995 > 2. https://github.com/torvalds/linux/blob/master/drivers/i2c/busses/i2c-npcm7xx.c#L2002 > > > > > (Or am I missing something?) > > > > Thanks, > > Andi > > Thank you. > > Regards, > Tyrone Thank you. Regards, Tyrone
Hi Tyrone, ... > > > - /* 100KHz and below: */ > > > - if (bus_freq_hz <= I2C_MAX_STANDARD_MODE_FREQ) { > > > - sclfrq = src_clk_khz / (bus_freq_khz * 4); > > > - > > > - if (sclfrq < SCLFRQ_MIN || sclfrq > SCLFRQ_MAX) > > > - return -EDOM; > > > - > > > - if (src_clk_khz >= 40000) > > > - hldt = 17; > > > - else if (src_clk_khz >= 12500) > > > - hldt = 15; > > > - else > > > - hldt = 7; > > > - } > > > - > > > - /* 400KHz: */ > > > - else if (bus_freq_hz <= I2C_MAX_FAST_MODE_FREQ) { > > > - sclfrq = 0; > > > + switch (bus_freq_hz) { > > > + case I2C_MAX_STANDARD_MODE_FREQ: > > > + smb_timing = smb_timing_100khz; > > > + table_size = ARRAY_SIZE(smb_timing_100khz); > > > + break; > > > + case I2C_MAX_FAST_MODE_FREQ: > > > + smb_timing = smb_timing_400khz; > > > + table_size = ARRAY_SIZE(smb_timing_400khz); > > > fast_mode = I2CCTL3_400K_MODE; > > > - > > > - if (src_clk_khz < 7500) > > > - /* 400KHZ cannot be supported for core clock < 7.5MHz */ > > > - return -EDOM; > > > - > > > - else if (src_clk_khz >= 50000) { > > > - k1 = 80; > > > - k2 = 48; > > > - hldt = 12; > > > - dbnct = 7; > > > - } > > > - > > > - /* Master or Slave with frequency > 25MHz */ > > > - else if (src_clk_khz > 25000) { > > > - hldt = clk_coef(src_clk_khz, 300) + 7; > > > - k1 = clk_coef(src_clk_khz, 1600); > > > - k2 = clk_coef(src_clk_khz, 900); > > > - } > > > - } > > > - > > > - /* 1MHz: */ > > > - else if (bus_freq_hz <= I2C_MAX_FAST_MODE_PLUS_FREQ) { > > > - sclfrq = 0; > > > + break; > > > + case I2C_MAX_FAST_MODE_PLUS_FREQ: > > > + smb_timing = smb_timing_1000khz; > > > + table_size = ARRAY_SIZE(smb_timing_1000khz); > > > fast_mode = I2CCTL3_400K_MODE; > > > - > > > - /* 1MHZ cannot be supported for core clock < 24 MHz */ > > > - if (src_clk_khz < 24000) > > > - return -EDOM; > > > - > > > - k1 = clk_coef(src_clk_khz, 620); > > > - k2 = clk_coef(src_clk_khz, 380); > > > - > > > - /* Core clk > 40 MHz */ > > > - if (src_clk_khz > 40000) { > > > - /* > > > - * Set HLDT: > > > - * SDA hold time: (HLDT-7) * T(CLK) >= 120 > > > - * HLDT = 120/T(CLK) + 7 = 120 * FREQ(CLK) + 7 > > > - */ > > > - hldt = clk_coef(src_clk_khz, 120) + 7; > > > - } else { > > > - hldt = 7; > > > - dbnct = 2; > > > - } > > > + break; > > > + default: > > > + return -EINVAL; > > > > There is here a slight change of behaiour which is not mentioned > > in the commit log. Before the user could set a bus_freq_hz which > > had to be <= I2C_MAX_..._MODE_FREQ, while now it has to be > > precisely that. > > > > Do we want to check what the user has set in the DTS? > > The driver checks the bus frequency the user sets in the DTS. yes, but before it was checking the value within a range, while now it's checking the exact value. The difference is that now if you don't set the exact value you get EINVAL, not before. Andi
Hi Andi, > > > > > - /* 100KHz and below: */ > > > > - if (bus_freq_hz <= I2C_MAX_STANDARD_MODE_FREQ) { > > > > - sclfrq = src_clk_khz / (bus_freq_khz * 4); > > > > - > > > > - if (sclfrq < SCLFRQ_MIN || sclfrq > SCLFRQ_MAX) > > > > - return -EDOM; > > > > - > > > > - if (src_clk_khz >= 40000) > > > > - hldt = 17; > > > > - else if (src_clk_khz >= 12500) > > > > - hldt = 15; > > > > - else > > > > - hldt = 7; > > > > - } > > > > - > > > > - /* 400KHz: */ > > > > - else if (bus_freq_hz <= I2C_MAX_FAST_MODE_FREQ) { > > > > - sclfrq = 0; > > > > + switch (bus_freq_hz) { > > > > + case I2C_MAX_STANDARD_MODE_FREQ: > > > > + smb_timing = smb_timing_100khz; > > > > + table_size = ARRAY_SIZE(smb_timing_100khz); > > > > + break; > > > > + case I2C_MAX_FAST_MODE_FREQ: > > > > + smb_timing = smb_timing_400khz; > > > > + table_size = ARRAY_SIZE(smb_timing_400khz); > > > > fast_mode = I2CCTL3_400K_MODE; > > > > - > > > > - if (src_clk_khz < 7500) > > > > - /* 400KHZ cannot be supported for core clock < 7.5MHz */ > > > > - return -EDOM; > > > > - > > > > - else if (src_clk_khz >= 50000) { > > > > - k1 = 80; > > > > - k2 = 48; > > > > - hldt = 12; > > > > - dbnct = 7; > > > > - } > > > > - > > > > - /* Master or Slave with frequency > 25MHz */ > > > > - else if (src_clk_khz > 25000) { > > > > - hldt = clk_coef(src_clk_khz, 300) + 7; > > > > - k1 = clk_coef(src_clk_khz, 1600); > > > > - k2 = clk_coef(src_clk_khz, 900); > > > > - } > > > > - } > > > > - > > > > - /* 1MHz: */ > > > > - else if (bus_freq_hz <= I2C_MAX_FAST_MODE_PLUS_FREQ) { > > > > - sclfrq = 0; > > > > + break; > > > > + case I2C_MAX_FAST_MODE_PLUS_FREQ: > > > > + smb_timing = smb_timing_1000khz; > > > > + table_size = ARRAY_SIZE(smb_timing_1000khz); > > > > fast_mode = I2CCTL3_400K_MODE; > > > > - > > > > - /* 1MHZ cannot be supported for core clock < 24 MHz */ > > > > - if (src_clk_khz < 24000) > > > > - return -EDOM; > > > > - > > > > - k1 = clk_coef(src_clk_khz, 620); > > > > - k2 = clk_coef(src_clk_khz, 380); > > > > - > > > > - /* Core clk > 40 MHz */ > > > > - if (src_clk_khz > 40000) { > > > > - /* > > > > - * Set HLDT: > > > > - * SDA hold time: (HLDT-7) * T(CLK) >= 120 > > > > - * HLDT = 120/T(CLK) + 7 = 120 * FREQ(CLK) + 7 > > > > - */ > > > > - hldt = clk_coef(src_clk_khz, 120) + 7; > > > > - } else { > > > > - hldt = 7; > > > > - dbnct = 2; > > > > - } > > > > + break; > > > > + default: > > > > + return -EINVAL; > > > > > > There is here a slight change of behaiour which is not mentioned > > > in the commit log. Before the user could set a bus_freq_hz which > > > had to be <= I2C_MAX_..._MODE_FREQ, while now it has to be > > > precisely that. > > > > > > Do we want to check what the user has set in the DTS? > > > > The driver checks the bus frequency the user sets in the DTS. > > yes, but before it was checking the value within a range, while > now it's checking the exact value. > > The difference is that now if you don't set the exact value you > get EINVAL, not before. > > Andi Previously the driver was rounding numbers down. The driver has settings for 100, 400, 1000 KHz. but what happens if the user asks for 200KHz? Some of the coefficients were calculated according to the equations, and some were hard-coded values per setting. We don't want to support this mix. We prefer the users to ask for numbers that are one of the three supported values and block unknown input values. Thanks , Tali
diff --git a/drivers/i2c/busses/i2c-npcm7xx.c b/drivers/i2c/busses/i2c-npcm7xx.c index a9a9b21f1f0b..7c13f9f6014a 100644 --- a/drivers/i2c/busses/i2c-npcm7xx.c +++ b/drivers/i2c/busses/i2c-npcm7xx.c @@ -263,6 +263,265 @@ static const int npcm_i2caddr[I2C_NUM_OWN_ADDR] = { #define I2C_FREQ_MIN_HZ 10000 #define I2C_FREQ_MAX_HZ I2C_MAX_FAST_MODE_PLUS_FREQ +struct smb_timing_t { + u32 core_clk; + u8 hldt; + u8 dbcnt; + u16 sclfrq; + u8 scllt; + u8 sclht; + bool fast_mode; +}; + +static struct smb_timing_t smb_timing_100khz[] = { + { + .core_clk = 100000000, .hldt = 0x2A, .dbcnt = 0x4, + .sclfrq = 0xFB, .scllt = 0x0, .sclht = 0x0, + .fast_mode = false, + }, + { + .core_clk = 62500000, .hldt = 0x2A, .dbcnt = 0x1, + .sclfrq = 0x9D, .scllt = 0x0, .sclht = 0x0, + .fast_mode = false, + }, + { + .core_clk = 50000000, .hldt = 0x2A, .dbcnt = 0x1, + .sclfrq = 0x7E, .scllt = 0x0, .sclht = 0x0, + .fast_mode = false, + }, + { + .core_clk = 48000000, .hldt = 0x2A, .dbcnt = 0x1, + .sclfrq = 0x79, .scllt = 0x0, .sclht = 0x0, + .fast_mode = false, + }, + { + .core_clk = 40000000, .hldt = 0x2A, .dbcnt = 0x1, + .sclfrq = 0x65, .scllt = 0x0, .sclht = 0x0, + .fast_mode = false, + }, + { + .core_clk = 30000000, .hldt = 0x2A, .dbcnt = 0x1, + .sclfrq = 0x4C, .scllt = 0x0, .sclht = 0x0, + .fast_mode = false, + }, + { + .core_clk = 29000000, .hldt = 0x2A, .dbcnt = 0x1, + .sclfrq = 0x49, .scllt = 0x0, .sclht = 0x0, + .fast_mode = false, + }, + { + .core_clk = 26000000, .hldt = 0x2A, .dbcnt = 0x1, + .sclfrq = 0x42, .scllt = 0x0, .sclht = 0x0, + .fast_mode = false, + }, + { + .core_clk = 25000000, .hldt = 0x2A, .dbcnt = 0x1, + .sclfrq = 0x3F, .scllt = 0x0, .sclht = 0x0, + .fast_mode = false, + }, + { + .core_clk = 24000000, .hldt = 0x2A, .dbcnt = 0x1, + .sclfrq = 0x3D, .scllt = 0x0, .sclht = 0x0, + .fast_mode = false, + }, + { + .core_clk = 20000000, .hldt = 0x2A, .dbcnt = 0x1, + .sclfrq = 0x33, .scllt = 0x0, .sclht = 0x0, + .fast_mode = false, + }, + { + .core_clk = 16180000, .hldt = 0x2A, .dbcnt = 0x1, + .sclfrq = 0x29, .scllt = 0x0, .sclht = 0x0, + .fast_mode = false, + }, + { + .core_clk = 15000000, .hldt = 0x23, .dbcnt = 0x1, + .sclfrq = 0x26, .scllt = 0x0, .sclht = 0x0, + .fast_mode = false, + }, + { + .core_clk = 13000000, .hldt = 0x1D, .dbcnt = 0x1, + .sclfrq = 0x21, .scllt = 0x0, .sclht = 0x0, + .fast_mode = false, + }, + { + .core_clk = 12000000, .hldt = 0x1B, .dbcnt = 0x1, + .sclfrq = 0x1F, .scllt = 0x0, .sclht = 0x0, + .fast_mode = false, + }, + { + .core_clk = 10000000, .hldt = 0x18, .dbcnt = 0x1, + .sclfrq = 0x1A, .scllt = 0x0, .sclht = 0x0, + .fast_mode = false, + }, + { + .core_clk = 9000000, .hldt = 0x16, .dbcnt = 0x1, + .sclfrq = 0x17, .scllt = 0x0, .sclht = 0x0, + .fast_mode = false, + }, + { + .core_clk = 8090000, .hldt = 0x14, .dbcnt = 0x1, + .sclfrq = 0x15, .scllt = 0x0, .sclht = 0x0, + .fast_mode = false, + }, + { + .core_clk = 7500000, .hldt = 0x7, .dbcnt = 0x1, + .sclfrq = 0x13, .scllt = 0x0, .sclht = 0x0, + .fast_mode = false, + }, + { + .core_clk = 6500000, .hldt = 0xE, .dbcnt = 0x1, + .sclfrq = 0x11, .scllt = 0x0, .sclht = 0x0, + .fast_mode = false, + }, + { + .core_clk = 4000000, .hldt = 0x9, .dbcnt = 0x1, + .sclfrq = 0xB, .scllt = 0x0, .sclht = 0x0, + .fast_mode = false, + }, +}; + +static struct smb_timing_t smb_timing_400khz[] = { + { + .core_clk = 100000000, .hldt = 0x2A, .dbcnt = 0x3, + .sclfrq = 0x0, .scllt = 0x47, .sclht = 0x35, + .fast_mode = true, + }, + { + .core_clk = 62500000, .hldt = 0x2A, .dbcnt = 0x2, + .sclfrq = 0x0, .scllt = 0x2C, .sclht = 0x22, + .fast_mode = true, + }, + { + .core_clk = 50000000, .hldt = 0x21, .dbcnt = 0x1, + .sclfrq = 0x0, .scllt = 0x24, .sclht = 0x1B, + .fast_mode = true, + }, + { + .core_clk = 48000000, .hldt = 0x1E, .dbcnt = 0x1, + .sclfrq = 0x0, .scllt = 0x24, .sclht = 0x19, + .fast_mode = true, + }, + { + .core_clk = 40000000, .hldt = 0x1B, .dbcnt = 0x1, + .sclfrq = 0x0, .scllt = 0x1E, .sclht = 0x14, + .fast_mode = true, + }, + { + .core_clk = 33000000, .hldt = 0x15, .dbcnt = 0x1, + .sclfrq = 0x0, .scllt = 0x19, .sclht = 0x11, + .fast_mode = true, + }, + { + .core_clk = 30000000, .hldt = 0x15, .dbcnt = 0x1, + .sclfrq = 0x0, .scllt = 0x19, .sclht = 0xD, + .fast_mode = true, + }, + { + .core_clk = 29000000, .hldt = 0x11, .dbcnt = 0x1, + .sclfrq = 0x0, .scllt = 0x15, .sclht = 0x10, + .fast_mode = true, + }, + { + .core_clk = 26000000, .hldt = 0x10, .dbcnt = 0x1, + .sclfrq = 0x0, .scllt = 0x13, .sclht = 0xE, + .fast_mode = true, + }, + { + .core_clk = 25000000, .hldt = 0xF, .dbcnt = 0x1, + .sclfrq = 0x0, .scllt = 0x13, .sclht = 0xD, + .fast_mode = true, + }, + { + .core_clk = 24000000, .hldt = 0xD, .dbcnt = 0x1, + .sclfrq = 0x0, .scllt = 0x12, .sclht = 0xD, + .fast_mode = true, + }, + { + .core_clk = 20000000, .hldt = 0xB, .dbcnt = 0x1, + .sclfrq = 0x0, .scllt = 0xF, .sclht = 0xA, + .fast_mode = true, + }, + { + .core_clk = 16180000, .hldt = 0xA, .dbcnt = 0x1, + .sclfrq = 0x0, .scllt = 0xC, .sclht = 0x9, + .fast_mode = true, + }, + { + .core_clk = 15000000, .hldt = 0x9, .dbcnt = 0x1, + .sclfrq = 0x0, .scllt = 0xB, .sclht = 0x8, + .fast_mode = true, + }, + { + .core_clk = 13000000, .hldt = 0x7, .dbcnt = 0x1, + .sclfrq = 0x0, .scllt = 0xA, .sclht = 0x7, + .fast_mode = true, + }, + { + .core_clk = 12000000, .hldt = 0x7, .dbcnt = 0x1, + .sclfrq = 0x0, .scllt = 0xA, .sclht = 0x6, + .fast_mode = true, + }, + { + .core_clk = 10000000, .hldt = 0x6, .dbcnt = 0x1, + .sclfrq = 0x0, .scllt = 0x8, .sclht = 0x5, + .fast_mode = true, + }, +}; + +static struct smb_timing_t smb_timing_1000khz[] = { + { + .core_clk = 100000000, .hldt = 0x15, .dbcnt = 0x4, + .sclfrq = 0x0, .scllt = 0x1C, .sclht = 0x15, + .fast_mode = true, + }, + { + .core_clk = 62500000, .hldt = 0xF, .dbcnt = 0x3, + .sclfrq = 0x0, .scllt = 0x11, .sclht = 0xE, + .fast_mode = true, + }, + { + .core_clk = 50000000, .hldt = 0xA, .dbcnt = 0x2, + .sclfrq = 0x0, .scllt = 0xE, .sclht = 0xB, + .fast_mode = true, + }, + { + .core_clk = 48000000, .hldt = 0x9, .dbcnt = 0x2, + .sclfrq = 0x0, .scllt = 0xD, .sclht = 0xB, + .fast_mode = true, + }, + { + .core_clk = 41000000, .hldt = 0x9, .dbcnt = 0x2, + .sclfrq = 0x0, .scllt = 0xC, .sclht = 0x9, + .fast_mode = true, + }, + { + .core_clk = 40000000, .hldt = 0x8, .dbcnt = 0x2, + .sclfrq = 0x0, .scllt = 0xB, .sclht = 0x9, + .fast_mode = true, + }, + { + .core_clk = 33000000, .hldt = 0x7, .dbcnt = 0x1, + .sclfrq = 0x0, .scllt = 0xA, .sclht = 0x7, + .fast_mode = true, + }, + { + .core_clk = 25000000, .hldt = 0x4, .dbcnt = 0x1, + .sclfrq = 0x0, .scllt = 0x7, .sclht = 0x6, + .fast_mode = true, + }, + { + .core_clk = 24000000, .hldt = 0x7, .dbcnt = 0x1, + .sclfrq = 0x0, .scllt = 0x8, .sclht = 0x5, + .fast_mode = true, + }, + { + .core_clk = 20000000, .hldt = 0x4, .dbcnt = 0x1, + .sclfrq = 0x0, .scllt = 0x6, .sclht = 0x4, + .fast_mode = true, + }, +}; + struct npcm_i2c_data { u8 fifo_size; u32 segctl_init_val; @@ -1805,102 +2064,45 @@ static void npcm_i2c_recovery_init(struct i2c_adapter *_adap) */ static int npcm_i2c_init_clk(struct npcm_i2c *bus, u32 bus_freq_hz) { - u32 k1 = 0; - u32 k2 = 0; - u8 dbnct = 0; - u32 sclfrq = 0; - u8 hldt = 7; + struct smb_timing_t *smb_timing; + u8 scl_table_cnt = 0, table_size = 0; u8 fast_mode = 0; - u32 src_clk_khz; - u32 bus_freq_khz; - src_clk_khz = bus->apb_clk / 1000; - bus_freq_khz = bus_freq_hz / 1000; bus->bus_freq = bus_freq_hz; - /* 100KHz and below: */ - if (bus_freq_hz <= I2C_MAX_STANDARD_MODE_FREQ) { - sclfrq = src_clk_khz / (bus_freq_khz * 4); - - if (sclfrq < SCLFRQ_MIN || sclfrq > SCLFRQ_MAX) - return -EDOM; - - if (src_clk_khz >= 40000) - hldt = 17; - else if (src_clk_khz >= 12500) - hldt = 15; - else - hldt = 7; - } - - /* 400KHz: */ - else if (bus_freq_hz <= I2C_MAX_FAST_MODE_FREQ) { - sclfrq = 0; + switch (bus_freq_hz) { + case I2C_MAX_STANDARD_MODE_FREQ: + smb_timing = smb_timing_100khz; + table_size = ARRAY_SIZE(smb_timing_100khz); + break; + case I2C_MAX_FAST_MODE_FREQ: + smb_timing = smb_timing_400khz; + table_size = ARRAY_SIZE(smb_timing_400khz); fast_mode = I2CCTL3_400K_MODE; - - if (src_clk_khz < 7500) - /* 400KHZ cannot be supported for core clock < 7.5MHz */ - return -EDOM; - - else if (src_clk_khz >= 50000) { - k1 = 80; - k2 = 48; - hldt = 12; - dbnct = 7; - } - - /* Master or Slave with frequency > 25MHz */ - else if (src_clk_khz > 25000) { - hldt = clk_coef(src_clk_khz, 300) + 7; - k1 = clk_coef(src_clk_khz, 1600); - k2 = clk_coef(src_clk_khz, 900); - } - } - - /* 1MHz: */ - else if (bus_freq_hz <= I2C_MAX_FAST_MODE_PLUS_FREQ) { - sclfrq = 0; + break; + case I2C_MAX_FAST_MODE_PLUS_FREQ: + smb_timing = smb_timing_1000khz; + table_size = ARRAY_SIZE(smb_timing_1000khz); fast_mode = I2CCTL3_400K_MODE; - - /* 1MHZ cannot be supported for core clock < 24 MHz */ - if (src_clk_khz < 24000) - return -EDOM; - - k1 = clk_coef(src_clk_khz, 620); - k2 = clk_coef(src_clk_khz, 380); - - /* Core clk > 40 MHz */ - if (src_clk_khz > 40000) { - /* - * Set HLDT: - * SDA hold time: (HLDT-7) * T(CLK) >= 120 - * HLDT = 120/T(CLK) + 7 = 120 * FREQ(CLK) + 7 - */ - hldt = clk_coef(src_clk_khz, 120) + 7; - } else { - hldt = 7; - dbnct = 2; - } + break; + default: + return -EINVAL; } - /* Frequency larger than 1 MHz is not supported */ - else - return -EINVAL; + for (scl_table_cnt = 0; scl_table_cnt < table_size; scl_table_cnt++) + if (bus->apb_clk >= smb_timing[scl_table_cnt].core_clk) + break; - if (bus_freq_hz >= I2C_MAX_FAST_MODE_FREQ) { - k1 = round_up(k1, 2); - k2 = round_up(k2 + 1, 2); - if (k1 < SCLFRQ_MIN || k1 > SCLFRQ_MAX || - k2 < SCLFRQ_MIN || k2 > SCLFRQ_MAX) - return -EDOM; - } + if (scl_table_cnt == table_size) + return -EINVAL; /* write sclfrq value. bits [6:0] are in I2CCTL2 reg */ - iowrite8(FIELD_PREP(I2CCTL2_SCLFRQ6_0, sclfrq & 0x7F), + iowrite8(FIELD_PREP(I2CCTL2_SCLFRQ6_0, smb_timing[scl_table_cnt].sclfrq & 0x7F), bus->reg + NPCM_I2CCTL2); /* bits [8:7] are in I2CCTL3 reg */ - iowrite8(fast_mode | FIELD_PREP(I2CCTL3_SCLFRQ8_7, (sclfrq >> 7) & 0x3), + iowrite8(FIELD_PREP(I2CCTL3_SCLFRQ8_7, (smb_timing[scl_table_cnt].sclfrq >> 7) & 0x3) | + fast_mode, bus->reg + NPCM_I2CCTL3); /* Select Bank 0 to access NPCM_I2CCTL4/NPCM_I2CCTL5 */ @@ -1912,13 +2114,13 @@ static int npcm_i2c_init_clk(struct npcm_i2c *bus, u32 bus_freq_hz) * k1 = 2 * SCLLT7-0 -> Low Time = k1 / 2 * k2 = 2 * SCLLT7-0 -> High Time = k2 / 2 */ - iowrite8(k1 / 2, bus->reg + NPCM_I2CSCLLT); - iowrite8(k2 / 2, bus->reg + NPCM_I2CSCLHT); + iowrite8(smb_timing[scl_table_cnt].scllt, bus->reg + NPCM_I2CSCLLT); + iowrite8(smb_timing[scl_table_cnt].sclht, bus->reg + NPCM_I2CSCLHT); - iowrite8(dbnct, bus->reg + NPCM_I2CCTL5); + iowrite8(smb_timing[scl_table_cnt].dbcnt, bus->reg + NPCM_I2CCTL5); } - iowrite8(hldt, bus->reg + NPCM_I2CCTL4); + iowrite8(smb_timing[scl_table_cnt].hldt, bus->reg + NPCM_I2CCTL4); /* Return to Bank 1, and stay there by default: */ npcm_i2c_select_bank(bus, I2C_BANK_1);