[PATCH v4 0/2] Compute HS HCNT and LCNT based on HW parameters

[PATCH v4 0/2] Compute HS HCNT and LCNT based on HW parameters

[Michael Wu]

In commit 35eba185fd1a ("i2c: designware: Calculate SCL timing parameter
for High Speed Mode") the SCL high period count and low period count for
high speed mode are calculated based on fixed tHIGH = 160 and tLOW = 120.
However, the set of two fixed values is only applicable to the combination
of hardware parameters IC_CAP_LOADING is 400 and IC_CLK_FREQ_OPTIMIZATION
is true. Outside of this combination, the SCL frequency may not reach
3.4 MHz because the fixed tHIGH and tLOW are not small enough.

Since there are no any registers controlling these two hardware parameters,
their values can only be declared through the device tree.

v4:
- yaml: re-formatting two properties' description
- yaml: enumeriate bus-capacitance-pf
- yaml: extand an existing example
- driver: modify the commit description
- driver: rename "bus-capacitance-pf" to "bus-capacitance-pF"

v3:
- add vendor prefix on new property name
- read new properties in i2c_dw_fw_parse_and_configure() directly
- in i2c_dw_set_timings_master() check dev->bus_capacitance_pf and then decide
  t_high and t_low

v2:
- provide more hardware information in dt-bindings
- rename "bus-loading" to "bus-capacitance-pf"
- call new i2c_dw_fw_parse_hw_params() in i2c_dw_fw_parse_and_configure() to
  parse hardware parameters from the device tree.

Michael Wu (2):
  dt-bindings: i2c: snps,designware-i2c: declare bus capacitance and clk
    freq optimized
  i2c: dwsignware: determine HS tHIGH and tLOW based on HW parameters

 .../bindings/i2c/snps,designware-i2c.yaml     | 18 +++++++++++++++
 drivers/i2c/busses/i2c-designware-common.c    |  5 ++++
 drivers/i2c/busses/i2c-designware-core.h      |  6 +++++
 drivers/i2c/busses/i2c-designware-master.c    | 23 +++++++++++++++++--
 4 files changed, 50 insertions(+), 2 deletions(-)

-- 
2.43.0

[PATCH v4 1/2] dt-bindings: i2c: snps,designware-i2c: declare bus capacitance and clk freq optimized

[Michael Wu]

Since there are no registers controlling the hardware parameters
IC_CAP_LOADING and IC_CLK_FREQ_OPTIMIZATION, their values can only be
declared in the device tree.

snps,bus-capacitance-pf indicates the bus capacitance in picofarads (pF).
It affects the high and low pulse width of SCL line in high speed mode.
The legal values for this property are 100 and 400 only, and default
value is 100. This property corresponds to IC_CAP_LOADING.

snps,clk-freq-optimized indicates whether the hardware reduce its
internal clock frequency by reducing the internal latency required to
generate the high period and low period of SCL line. This property
corresponds to IC_CLK_FREQ_OPTIMIZATION.

The driver can calculate the high period count and low period count of
SCL line for high speed mode based on these two properties.

Signed-off-by: Michael Wu <michael.wu@kneron.us>
---
 .../bindings/i2c/snps,designware-i2c.yaml      | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
index 60035a787e5c..e5d05263c45a 100644
--- a/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/snps,designware-i2c.yaml
@@ -97,6 +97,22 @@ properties:
       - const: tx
       - const: rx
 
+  snps,bus-capacitance-pf:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      This property indicates the bus capacitance in picofarads (pF).
+      This value is used to compute the tHIGH and tLOW periods for high speed
+      mode.
+    enum: [100, 400]
+    default: 100
+
+  snps,clk-freq-optimized:
+    description:
+      This property indicates whether the hardware reduce its clock frequency
+      by reducing the internal latency required to generate the high period and
+      low period of SCL line.
+    type: boolean
+
 unevaluatedProperties: false
 
 required:
@@ -121,6 +137,8 @@ examples:
       i2c-sda-hold-time-ns = <300>;
       i2c-sda-falling-time-ns = <300>;
       i2c-scl-falling-time-ns = <300>;
+      snps,bus-capacitance-pf = <400>;
+      snps,clk-freq-optimized;
     };
   - |
     i2c@2000 {
-- 
2.43.0

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

[Michael Wu]

In commit 35eba185fd1a ("i2c: designware: Calculate SCL timing parameter
for High Speed Mode") the SCL high period count and low period count for
high speed mode are calculated based on fixed tHIGH = 160 and tLOW = 120.
However, the set of two fixed values is only applicable to the combination
of hardware parameters IC_CAP_LOADING is 400 and IC_CLK_FREQ_OPTIMIZATION
is true. Outside of this combination, the SCL frequency may not reach
3.4 MHz because the fixed tHIGH and tLOW are not small enough.

If IC_CAP_LOADING is 400, it means the bus capacitance is 400pF;
Otherwise, 100 pF. If IC_CLK_FREQ_OPTIMIZATION is true, it means that the
hardware reduces its internal clock frequency by reducing the internal
latency required to generate the high period and low period of the SCL line.

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 SCL high period count and low period
count for high speed mode, two hardware parameters IC_CAP_LOADING and
IC_CLK_FREQ_OPTIMIZATION must be considered together. Since there're no
registers controlliing these these two hardware parameters, users can
declare them in the device tree so that the driver can obtain them.

Signed-off-by: Michael Wu <michael.wu@kneron.us>
---
 drivers/i2c/busses/i2c-designware-common.c |  5 +++++
 drivers/i2c/busses/i2c-designware-core.h   |  6 ++++++
 drivers/i2c/busses/i2c-designware-master.c | 23 ++++++++++++++++++++--
 3 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 080204182bb5..ff3d83269ee7 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -380,6 +380,11 @@ int i2c_dw_fw_parse_and_configure(struct dw_i2c_dev *dev)
 
 	i2c_parse_fw_timings(device, t, false);
 
+	if (device_property_read_u32(device, "snps,bus-capacitance-pf", &dev->bus_capacitance_pF))
+		dev->bus_capacitance_pF = 100;
+
+	dev->clk_freq_optimized = device_property_read_bool(device, "snps,clk-freq-optimized");
+
 	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..d09e6ffb793d 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 picofarads
+ * @clk_freq_optimized: if this value is true, it means the hardware reduces
+ *	its internal clock frequency by reducing the internal latency required
+ *	to generate the high period and low period of 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..b56f33297d5d 100644
--- a/drivers/i2c/busses/i2c-designware-master.c
+++ b/drivers/i2c/busses/i2c-designware-master.c
@@ -154,12 +154,31 @@ 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;
+
+			/*
+			 * The legal values stated in the databook for bus
+			 * capacitance are only 100pF and 400pF.
+			 * If dev->bus_capacitance_pF is greater than or equals
+			 * to 400, t_high and t_low are assumed to be
+			 * appropriate values for 400pF, otherwise 100pF.
+			 */
+			if (dev->bus_capacitance_pF >= 400) {
+				/* assume bus capacitance is 400pF */
+				t_high = dev->clk_freq_optimized ? 160 : 120;
+				t_low = 320;
+			} else {
+				/* assume bus capacitance is 100pF */
+				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 +186,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 */
 		}
-- 
2.43.0

Re: [PATCH v4 2/2] i2c: dwsignware: determine HS tHIGH and tLOW based on HW parameters

[Andy Shevchenko]

On Thu, Oct 03, 2024 at 07:15:24PM +0800, Michael Wu wrote:
> In commit 35eba185fd1a ("i2c: designware: Calculate SCL timing parameter
> for High Speed Mode") the SCL high period count and low period count for
> high speed mode are calculated based on fixed tHIGH = 160 and tLOW = 120.
> However, the set of two fixed values is only applicable to the combination
> of hardware parameters IC_CAP_LOADING is 400 and IC_CLK_FREQ_OPTIMIZATION
> is true. Outside of this combination, the SCL frequency may not reach
> 3.4 MHz because the fixed tHIGH and tLOW are not small enough.
> 
> If IC_CAP_LOADING is 400, it means the bus capacitance is 400pF;
> Otherwise, 100 pF. If IC_CLK_FREQ_OPTIMIZATION is true, it means that the
> hardware reduces its internal clock frequency by reducing the internal
> latency required to generate the high period and low period of the SCL line.
> 
> 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 SCL high period count and low period
> count for high speed mode, two hardware parameters IC_CAP_LOADING and
> IC_CLK_FREQ_OPTIMIZATION must be considered together. Since there're no
> registers controlliing these these two hardware parameters, users can
> declare them in the device tree so that the driver can obtain them.

As long as DT schema (new properties) is accepted, this LGTM now,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v4 1/2] dt-bindings: i2c: snps,designware-i2c: declare bus capacitance and clk freq optimized

[Krzysztof Kozlowski]

On 03/10/2024 13:15, Michael Wu wrote:
> Since there are no registers controlling the hardware parameters
> IC_CAP_LOADING and IC_CLK_FREQ_OPTIMIZATION, their values can only be
> declared in the device tree.
> 
> snps,bus-capacitance-pf indicates the bus capacitance in picofarads (pF).
> It affects the high and low pulse width of SCL line in high speed mode.
> The legal values for this property are 100 and 400 only, and default
> value is 100. This property corresponds to IC_CAP_LOADING.
> 
> snps,clk-freq-optimized indicates whether the hardware reduce its
> internal clock frequency by reducing the internal latency required to
> generate the high period and low period of SCL line. This property
> corresponds to IC_CLK_FREQ_OPTIMIZATION.
> 
> The driver can calculate the high period count and low period count of
> SCL line for high speed mode based on these two properties.
> 
> Signed-off-by: Michael Wu <michael.wu@kneron.us>
> ---


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>


---

<form letter>
This is an automated instruction, just in case, because many review tags
are being ignored. If you know the process, you can skip it (please do
not feel offended by me posting it here - no bad intentions intended).
If you do not know the process, here is a short explanation:

Please add Acked-by/Reviewed-by/Tested-by tags when posting new
versions, under or above your Signed-off-by tag. Tag is "received", when
provided in a message replied to you on the mailing list. Tools like b4
can help here. However, there's no need to repost patches *only* to add
the tags. The upstream maintainer will do that for tags received on the
version they apply.

https://elixir.bootlin.com/linux/v6.5-rc3/source/Documentation/process/submitting-patches.rst#L577
</form letter>

Best regards,
Krzysztof

Re: [PATCH v4 2/2] i2c: dwsignware: determine HS tHIGH and tLOW based on HW parameters

[Jarkko Nikula]

On 10/3/24 2:41 PM, Andy Shevchenko wrote:
> On Thu, Oct 03, 2024 at 07:15:24PM +0800, Michael Wu wrote:
>> In commit 35eba185fd1a ("i2c: designware: Calculate SCL timing parameter
>> for High Speed Mode") the SCL high period count and low period count for
>> high speed mode are calculated based on fixed tHIGH = 160 and tLOW = 120.
>> However, the set of two fixed values is only applicable to the combination
>> of hardware parameters IC_CAP_LOADING is 400 and IC_CLK_FREQ_OPTIMIZATION
>> is true. Outside of this combination, the SCL frequency may not reach
>> 3.4 MHz because the fixed tHIGH and tLOW are not small enough.
>>
>> If IC_CAP_LOADING is 400, it means the bus capacitance is 400pF;
>> Otherwise, 100 pF. If IC_CLK_FREQ_OPTIMIZATION is true, it means that the
>> hardware reduces its internal clock frequency by reducing the internal
>> latency required to generate the high period and low period of the SCL line.
>>
>> 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 SCL high period count and low period
>> count for high speed mode, two hardware parameters IC_CAP_LOADING and
>> IC_CLK_FREQ_OPTIMIZATION must be considered together. Since there're no
>> registers controlliing these these two hardware parameters, users can
>> declare them in the device tree so that the driver can obtain them.
> 
> As long as DT schema (new properties) is accepted, this LGTM now,
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

RE: [PATCH v4 0/2] Compute HS HCNT and LCNT based on HW parameters

[Michael Wu]

On 03.10.23, Michael Wu wrote:
> In commit 35eba185fd1a ("i2c: designware: Calculate SCL timing parameter
> for High Speed Mode") the SCL high period count and low period count for
> high speed mode are calculated based on fixed tHIGH = 160 and tLOW = 120.
> However, the set of two fixed values is only applicable to the combination
> of hardware parameters IC_CAP_LOADING is 400 and
> IC_CLK_FREQ_OPTIMIZATION
> is true. Outside of this combination, the SCL frequency may not reach
> 3.4 MHz because the fixed tHIGH and tLOW are not small enough.
> 
> Since there are no any registers controlling these two hardware parameters,
> their values can only be declared through the device tree.
> 
> v4:
> - yaml: re-formatting two properties' description
> - yaml: enumeriate bus-capacitance-pf
> - yaml: extand an existing example
> - driver: modify the commit description
> - driver: rename "bus-capacitance-pf" to "bus-capacitance-pF"
> 
> v3:
> - add vendor prefix on new property name
> - read new properties in i2c_dw_fw_parse_and_configure() directly
> - in i2c_dw_set_timings_master() check dev->bus_capacitance_pf and then
> decide
>   t_high and t_low
> 
> v2:
> - provide more hardware information in dt-bindings
> - rename "bus-loading" to "bus-capacitance-pf"
> - call new i2c_dw_fw_parse_hw_params() in i2c_dw_fw_parse_and_configure()
> to
>   parse hardware parameters from the device tree.
> 
> Michael Wu (2):
>   dt-bindings: i2c: snps,designware-i2c: declare bus capacitance and clk
>     freq optimized
>   i2c: dwsignware: determine HS tHIGH and tLOW based on HW parameters
> 
>  .../bindings/i2c/snps,designware-i2c.yaml     | 18 +++++++++++++++
>  drivers/i2c/busses/i2c-designware-common.c    |  5 ++++
>  drivers/i2c/busses/i2c-designware-core.h      |  6 +++++
>  drivers/i2c/busses/i2c-designware-master.c    | 23 +++++++++++++++++--
>  4 files changed, 50 insertions(+), 2 deletions(-)
> 
> --
> 2.43.0

Hi Andi,

Please forgive me for my presumption. Regarding this patch, will it be
merged? If not, please kindly provide any suggestions I can make to
improve it.

Sincerely yours,
Michael Wu

Re: [PATCH v4 0/2] Compute HS HCNT and LCNT based on HW parameters

[Andi Shyti]

Hi Michael,

> Michael Wu (2):
>   dt-bindings: i2c: snps,designware-i2c: declare bus capacitance and clk
>     freq optimized
>   i2c: dwsignware: determine HS tHIGH and tLOW based on HW parameters

merged to i2c/i2c-host.

Thanks,
Andi