diff mbox series

[v2,2/2] i2c: dwsignware: determine HS tHIGH and tLOW based on HW parameters

Message ID 20240927042230.277144-3-michael.wu@kneron.us
State New
Headers show
Series Compute HS HCNT and LCNT based on HW parameters | expand

Commit Message

Michael Wu Sept. 27, 2024, 4:22 a.m. UTC
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(-)

Comments

Krzysztof Kozlowski Sept. 27, 2024, 8:36 a.m. UTC | #1
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
Andy Shevchenko Sept. 27, 2024, 9:44 a.m. UTC | #2
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 mbox series

Patch

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 */
 		}