[PATCH 1/2] dt-bindings: i2c: designware: allow fine tuning tuning waveform from device tree

[PATCH 1/2] dt-bindings: i2c: designware: allow fine tuning tuning waveform from device tree

[Aahil Awatramani]

The Synopsys i2c driver allows a user to override the parameters
controlling the waveform using ACPI; this is useful for fine tuning when
needed to make spec compliance. Extend this support to the device tree to
allow non-ACPI platforms the same capabilities.

Signed-off-by: Aahil Awatramani <aahila@google.com>
---
 Documentation/devicetree/bindings/i2c/i2c.txt | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
index fc3dd7ec0445..3c33c36571f9 100644
--- a/Documentation/devicetree/bindings/i2c/i2c.txt
+++ b/Documentation/devicetree/bindings/i2c/i2c.txt
@@ -45,6 +45,24 @@ wants to support one of the below features, it should adapt these bindings.
 	Number of nanoseconds the SCL signal takes to rise; t(r) in the I2C
 	specification.
 
+- i2c-scl-ss-hcnt
+	HCNT value for standard speed mode in I2C Controller.
+
+- i2c-scl-ss-lcnt
+	LCNT value for standard speed mode in I2C Controller.
+
+- i2c-scl-fs-hcnt
+	HCNT value for fast speed mode in I2C Controller.
+
+- i2c-scl-fs-lcnt
+	LCNT value for fast speed mode in I2C Controller.
+
+- i2c-scl-hs-hcnt
+	HCNT value for high speed mode in I2C Controller.
+
+- i2c-scl-hs-lcnt
+	LCNT value for high speed mode in I2C Controller.
+
 - i2c-sda-falling-time-ns
 	Number of nanoseconds the SDA signal takes to fall; t(f) in the I2C
 	specification.
-- 
2.43.0.429.g432eaa2c6b-goog

[PATCH 2/2] i2c: designware: allow fine tuning tuning waveform from device tree

[Aahil Awatramani]

The Synopsys i2c driver allows a user to override the parameters
controlling the waveform using ACPI; this is useful for fine tuning when
needed to make spec compliance. Extend this support to the device tree to
allow non-ACPI platforms the same capabilities.

Signed-off-by: Aahil Awatramani <aahila@google.com>
---
 drivers/i2c/busses/i2c-designware-platdrv.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 855b698e99c0..09c464874fdb 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -132,6 +132,17 @@ static int mscc_twi_set_sda_hold_time(struct dw_i2c_dev *dev)
 	return 0;
 }
 
+static void i2c_parse_cnt(struct device *dev, char *prop_name, u16 *cnt)
+{
+	u32 tmp_cnt;
+	int ret;
+
+	ret = device_property_read_u32(dev, prop_name, &tmp_cnt);
+	if (ret)
+		return;
+	*cnt = tmp_cnt;
+}
+
 static int dw_i2c_of_configure(struct platform_device *pdev)
 {
 	struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
@@ -146,6 +157,15 @@ static int dw_i2c_of_configure(struct platform_device *pdev)
 		break;
 	}
 
+	i2c_parse_cnt(&pdev->dev, "i2c-scl-ss-hcnt", &dev->ss_hcnt);
+	i2c_parse_cnt(&pdev->dev, "i2c-scl-ss-lcnt", &dev->ss_lcnt);
+	i2c_parse_cnt(&pdev->dev, "i2c-scl-fs-hcnt", &dev->fs_hcnt);
+	i2c_parse_cnt(&pdev->dev, "i2c-scl-fs-lcnt", &dev->fs_lcnt);
+	i2c_parse_cnt(&pdev->dev, "i2c-scl-fp-hcnt", &dev->fp_hcnt);
+	i2c_parse_cnt(&pdev->dev, "i2c-scl-fp-lcnt", &dev->fp_lcnt);
+	i2c_parse_cnt(&pdev->dev, "i2c-scl-hs-hcnt", &dev->hs_hcnt);
+	i2c_parse_cnt(&pdev->dev, "i2c-scl-hs-lcnt", &dev->hs_lcnt);
+
 	return 0;
 }
 
-- 
2.43.0.429.g432eaa2c6b-goog

Re: [PATCH 1/2] dt-bindings: i2c: designware: allow fine tuning tuning waveform from device tree

[Krzysztof Kozlowski]

On 01/02/2024 05:46, Aahil Awatramani wrote:
> The Synopsys i2c driver allows a user to override the parameters

Driver? That already suggests it is not suitable for binding...

> controlling the waveform using ACPI; this is useful for fine tuning when
> needed to make spec compliance. Extend this support to the device tree to
> allow non-ACPI platforms the same capabilities.
> 
> Signed-off-by: Aahil Awatramani <aahila@google.com>
> ---
>  Documentation/devicetree/bindings/i2c/i2c.txt | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/i2c.txt b/Documentation/devicetree/bindings/i2c/i2c.txt
> index fc3dd7ec0445..3c33c36571f9 100644
> --- a/Documentation/devicetree/bindings/i2c/i2c.txt
> +++ b/Documentation/devicetree/bindings/i2c/i2c.txt
> @@ -45,6 +45,24 @@ wants to support one of the below features, it should adapt these bindings.
>  	Number of nanoseconds the SCL signal takes to rise; t(r) in the I2C
>  	specification.
>  
> +- i2c-scl-ss-hcnt
> +	HCNT value for standard speed mode in I2C Controller.

I have no clue what is HCNT, so I expect you to help us understand what
is this and why this is suitable for DT. Your reference to driver
suggests it is not suitable for DT.

Best regards,
Krzysztof

Re: [PATCH 2/2] i2c: designware: allow fine tuning tuning waveform from device tree

[Krzysztof Kozlowski]

On 01/02/2024 05:46, Aahil Awatramani wrote:
>  
> +static void i2c_parse_cnt(struct device *dev, char *prop_name, u16 *cnt)
> +{
> +	u32 tmp_cnt;
> +	int ret;
> +
> +	ret = device_property_read_u32(dev, prop_name, &tmp_cnt);
> +	if (ret)
> +		return;
> +	*cnt = tmp_cnt;
> +}
> +
>  static int dw_i2c_of_configure(struct platform_device *pdev)
>  {
>  	struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
> @@ -146,6 +157,15 @@ static int dw_i2c_of_configure(struct platform_device *pdev)
>  		break;
>  	}
>  
> +	i2c_parse_cnt(&pdev->dev, "i2c-scl-ss-hcnt", &dev->ss_hcnt);

Please post your DTS and post results of testing it against bindings.

Best regards,
Krzysztof

Re: [PATCH 1/2] dt-bindings: i2c: designware: allow fine tuning tuning waveform from device tree

[Andy Shevchenko]

On Thu, Feb 01, 2024 at 04:46:22AM +0000, Aahil Awatramani wrote:
> The Synopsys i2c driver allows a user to override the parameters
> controlling the waveform using ACPI; this is useful for fine tuning when
> needed to make spec compliance. Extend this support to the device tree to
> allow non-ACPI platforms the same capabilities.

DT has different counters from which one should derive these ones.
Can you explain why existing bindings may _not_ be utilised?

Without this very justification, NAK.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/2] dt-bindings: i2c: designware: allow fine tuning tuning waveform from device tree

[Aahil Awatramani]

Thank you Andy and Krzysztof,

I think you are right and I can use the other DT counter, I am
currently trying to test if they work for me.

Best,
Aahil

On Thu, Feb 1, 2024 at 3:49 AM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Thu, Feb 01, 2024 at 04:46:22AM +0000, Aahil Awatramani wrote:
> > The Synopsys i2c driver allows a user to override the parameters
> > controlling the waveform using ACPI; this is useful for fine tuning when
> > needed to make spec compliance. Extend this support to the device tree to
> > allow non-ACPI platforms the same capabilities.
>
> DT has different counters from which one should derive these ones.
> Can you explain why existing bindings may _not_ be utilised?
>
> Without this very justification, NAK.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>