| Message ID | 20240927042230.277144-3-michael.wu@kneron.us |
|---|---|
| State | New |
| Headers | show |
| Series | Compute HS HCNT and LCNT based on HW parameters | expand |
On Fri, Sep 27, 2024 at 12:22:17PM +0800, Michael Wu wrote: > In commit 35eba185fd1a ("i2c: designware: Calculate SCL timing parameter > for High Speed Mode") hs_hcnt and hs_lcnt are calculated based on fixed > tHIGH = 160 and tLOW = 120. However, the set of these fixed values only > applies to the combination of hardware parameters IC_CAP_LOADING = 400pF > and IC_CLK_FREQ_OPTIMIZATION = 1. Outside of this combination, if these > fixed tHIGH = 160 and tLOW = 120 are still used, the calculated hs_hcnt > and hs_lcnt may not be small enough, making it impossible for the SCL > frequency to reach 3.4 MHz. > > Section 3.15.4.5 in DesignWare DW_apb_i2b Databook v2.03 says that when > IC_CLK_FREQ_OPTIMIZATION = 0, > > MIN_SCL_HIGHtime = 60 ns for 3.4 Mbps, bus loading = 100pF > = 120 ns for 3.4 Mbps, bus loading = 400pF > MIN_SCL_LOWtime = 160 ns for 3.4 Mbps, bus loading = 100pF > = 320 ns for 3.4 Mbps, bus loading = 400pF > > and section 3.15.4.6 says that when IC_CLK_FREQ_OPTIMIZATION = 1, > > MIN_SCL_HIGHtime = 60 ns for 3.4 Mbps, bus loading = 100pF > = 160 ns for 3.4 Mbps, bus loading = 400pF > MIN_SCL_LOWtime = 120 ns for 3.4 Mbps, bus loading = 100pF > = 320 ns for 3.4 Mbps, bus loading = 400pF > > In order to calculate more accurate hs_hcnt amd hs_lcnt, two hardware > parameters IC_CAP_LOADING and IC_CLK_FREQ_OPTIMIZATION must be > considered together. > > Signed-off-by: Michael Wu <michael.wu@kneron.us> > --- > drivers/i2c/busses/i2c-designware-common.c | 16 ++++++++++++++++ > drivers/i2c/busses/i2c-designware-core.h | 6 ++++++ > drivers/i2c/busses/i2c-designware-master.c | 14 ++++++++++++-- > 3 files changed, 34 insertions(+), 2 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c > index 080204182bb5..907f049f09f8 100644 > --- a/drivers/i2c/busses/i2c-designware-common.c > +++ b/drivers/i2c/busses/i2c-designware-common.c > @@ -372,6 +372,20 @@ static void i2c_dw_adjust_bus_speed(struct dw_i2c_dev *dev) > t->bus_freq_hz = I2C_MAX_FAST_MODE_FREQ; > } > > +static void i2c_dw_fw_parse_hw_params(struct dw_i2c_dev *dev) > +{ > + struct device *device = dev->dev; > + int ret; > + > + ret = device_property_read_u32(device, "bus-capacitance-pf", &dev->bus_capacitance_pf); > + if (ret || dev->bus_capacitance_pf < 400) Your binding said nothing about some limits here. Something is not complete. You might be missing constraints in the binding. > + dev->bus_capacitance_pf = 100; > + else > + dev->bus_capacitance_pf = 400; > + > + dev->clk_freq_optimized = device_property_read_bool(device, "clk-freq-optimized"); Best regards, Krzysztof
On Fri, Sep 27, 2024 at 12:22:17PM +0800, Michael Wu wrote: > In commit 35eba185fd1a ("i2c: designware: Calculate SCL timing parameter > for High Speed Mode") hs_hcnt and hs_lcnt are calculated based on fixed > tHIGH = 160 and tLOW = 120. However, the set of these fixed values only > applies to the combination of hardware parameters IC_CAP_LOADING = 400pF > and IC_CLK_FREQ_OPTIMIZATION = 1. Outside of this combination, if these > fixed tHIGH = 160 and tLOW = 120 are still used, the calculated hs_hcnt > and hs_lcnt may not be small enough, making it impossible for the SCL > frequency to reach 3.4 MHz. > > Section 3.15.4.5 in DesignWare DW_apb_i2b Databook v2.03 says that when > IC_CLK_FREQ_OPTIMIZATION = 0, > > MIN_SCL_HIGHtime = 60 ns for 3.4 Mbps, bus loading = 100pF > = 120 ns for 3.4 Mbps, bus loading = 400pF > MIN_SCL_LOWtime = 160 ns for 3.4 Mbps, bus loading = 100pF > = 320 ns for 3.4 Mbps, bus loading = 400pF > > and section 3.15.4.6 says that when IC_CLK_FREQ_OPTIMIZATION = 1, > > MIN_SCL_HIGHtime = 60 ns for 3.4 Mbps, bus loading = 100pF > = 160 ns for 3.4 Mbps, bus loading = 400pF > MIN_SCL_LOWtime = 120 ns for 3.4 Mbps, bus loading = 100pF > = 320 ns for 3.4 Mbps, bus loading = 400pF > > In order to calculate more accurate hs_hcnt amd hs_lcnt, two hardware > parameters IC_CAP_LOADING and IC_CLK_FREQ_OPTIMIZATION must be > considered together. ... > +static void i2c_dw_fw_parse_hw_params(struct dw_i2c_dev *dev) Separate function is an overkill. Just inline its contents... ... > + ret = device_property_read_u32(device, "bus-capacitance-pf", &dev->bus_capacitance_pf); > + if (ret || dev->bus_capacitance_pf < 400) > + dev->bus_capacitance_pf = 100; > + else > + dev->bus_capacitance_pf = 400; > + > + dev->clk_freq_optimized = device_property_read_bool(device, "clk-freq-optimized"); ... > i2c_parse_fw_timings(device, t, false); > > + i2c_dw_fw_parse_hw_params(dev); ...here. > i2c_dw_adjust_bus_speed(dev); ... > + * @bus_capacitance_pf: bus capacitance in picofarad picofarads ... > + u32 t_high, t_low; > + > + if (dev->bus_capacitance_pf == 400) { > + t_high = dev->clk_freq_optimized ? 160 : 120; > + t_low = 320; > + } else { Yeah, this has no protection against wrong values in the DT/ACPI. So, perhaps } else if (...) { and assign defaults or bail out with an error? > + t_high = 60; > + t_low = dev->clk_freq_optimized ? 120 : 160; > + }
diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c index 080204182bb5..907f049f09f8 100644 --- a/drivers/i2c/busses/i2c-designware-common.c +++ b/drivers/i2c/busses/i2c-designware-common.c @@ -372,6 +372,20 @@ static void i2c_dw_adjust_bus_speed(struct dw_i2c_dev *dev) t->bus_freq_hz = I2C_MAX_FAST_MODE_FREQ; } +static void i2c_dw_fw_parse_hw_params(struct dw_i2c_dev *dev) +{ + struct device *device = dev->dev; + int ret; + + ret = device_property_read_u32(device, "bus-capacitance-pf", &dev->bus_capacitance_pf); + if (ret || dev->bus_capacitance_pf < 400) + dev->bus_capacitance_pf = 100; + else + dev->bus_capacitance_pf = 400; + + dev->clk_freq_optimized = device_property_read_bool(device, "clk-freq-optimized"); +} + int i2c_dw_fw_parse_and_configure(struct dw_i2c_dev *dev) { struct i2c_timings *t = &dev->timings; @@ -380,6 +394,8 @@ int i2c_dw_fw_parse_and_configure(struct dw_i2c_dev *dev) i2c_parse_fw_timings(device, t, false); + i2c_dw_fw_parse_hw_params(dev); + i2c_dw_adjust_bus_speed(dev); if (is_of_node(fwnode)) diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h index 1ac2afd03a0a..893c2c53648a 100644 --- a/drivers/i2c/busses/i2c-designware-core.h +++ b/drivers/i2c/busses/i2c-designware-core.h @@ -240,6 +240,10 @@ struct reset_control; * @set_sda_hold_time: callback to retrieve IP specific SDA hold timing * @mode: operation mode - DW_IC_MASTER or DW_IC_SLAVE * @rinfo: I²C GPIO recovery information + * @bus_capacitance_pf: bus capacitance in picofarad + * @clk_freq_optimized: indicate whether the hardware input clock frequency is + * reduced by reducing the internal latency required to generate the high + * period and low period of the SCL line * * HCNT and LCNT parameters can be used if the platform knows more accurate * values than the one computed based only on the input clock frequency. @@ -297,6 +301,8 @@ struct dw_i2c_dev { int (*set_sda_hold_time)(struct dw_i2c_dev *dev); int mode; struct i2c_bus_recovery_info rinfo; + u32 bus_capacitance_pf; + bool clk_freq_optimized; }; #define ACCESS_INTR_MASK BIT(0) diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c index e46f1b22c360..b18da66da6a8 100644 --- a/drivers/i2c/busses/i2c-designware-master.c +++ b/drivers/i2c/busses/i2c-designware-master.c @@ -154,12 +154,22 @@ static int i2c_dw_set_timings_master(struct dw_i2c_dev *dev) dev->hs_hcnt = 0; dev->hs_lcnt = 0; } else if (!dev->hs_hcnt || !dev->hs_lcnt) { + u32 t_high, t_low; + + if (dev->bus_capacitance_pf == 400) { + t_high = dev->clk_freq_optimized ? 160 : 120; + t_low = 320; + } else { + t_high = 60; + t_low = dev->clk_freq_optimized ? 120 : 160; + } + ic_clk = i2c_dw_clk_rate(dev); dev->hs_hcnt = i2c_dw_scl_hcnt(dev, DW_IC_HS_SCL_HCNT, ic_clk, - 160, /* tHIGH = 160 ns */ + t_high, sda_falling_time, 0, /* DW default */ 0); /* No offset */ @@ -167,7 +177,7 @@ static int i2c_dw_set_timings_master(struct dw_i2c_dev *dev) i2c_dw_scl_lcnt(dev, DW_IC_HS_SCL_LCNT, ic_clk, - 320, /* tLOW = 320 ns */ + t_low, scl_falling_time, 0); /* No offset */ }
In commit 35eba185fd1a ("i2c: designware: Calculate SCL timing parameter for High Speed Mode") hs_hcnt and hs_lcnt are calculated based on fixed tHIGH = 160 and tLOW = 120. However, the set of these fixed values only applies to the combination of hardware parameters IC_CAP_LOADING = 400pF and IC_CLK_FREQ_OPTIMIZATION = 1. Outside of this combination, if these fixed tHIGH = 160 and tLOW = 120 are still used, the calculated hs_hcnt and hs_lcnt may not be small enough, making it impossible for the SCL frequency to reach 3.4 MHz. Section 3.15.4.5 in DesignWare DW_apb_i2b Databook v2.03 says that when IC_CLK_FREQ_OPTIMIZATION = 0, MIN_SCL_HIGHtime = 60 ns for 3.4 Mbps, bus loading = 100pF = 120 ns for 3.4 Mbps, bus loading = 400pF MIN_SCL_LOWtime = 160 ns for 3.4 Mbps, bus loading = 100pF = 320 ns for 3.4 Mbps, bus loading = 400pF and section 3.15.4.6 says that when IC_CLK_FREQ_OPTIMIZATION = 1, MIN_SCL_HIGHtime = 60 ns for 3.4 Mbps, bus loading = 100pF = 160 ns for 3.4 Mbps, bus loading = 400pF MIN_SCL_LOWtime = 120 ns for 3.4 Mbps, bus loading = 100pF = 320 ns for 3.4 Mbps, bus loading = 400pF In order to calculate more accurate hs_hcnt amd hs_lcnt, two hardware parameters IC_CAP_LOADING and IC_CLK_FREQ_OPTIMIZATION must be considered together. Signed-off-by: Michael Wu <michael.wu@kneron.us> --- drivers/i2c/busses/i2c-designware-common.c | 16 ++++++++++++++++ drivers/i2c/busses/i2c-designware-core.h | 6 ++++++ drivers/i2c/busses/i2c-designware-master.c | 14 ++++++++++++-- 3 files changed, 34 insertions(+), 2 deletions(-)