devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/6] i2c: nomadik: support >=1MHz & Mobileye EyeQ6H platform
@ 2024-10-09 14:01 Théo Lebrun
  2024-10-09 14:01 ` [PATCH v3 1/6] dt-bindings: i2c: nomadik: add mobileye,eyeq6h-i2c bindings Théo Lebrun
                   ` (7 more replies)
  0 siblings, 8 replies; 12+ messages in thread
From: Théo Lebrun @ 2024-10-09 14:01 UTC (permalink / raw)
  To: Linus Walleij, Andi Shyti, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-kernel, linux-i2c, devicetree, linux-kernel,
	Vladimir Kondratiev, Grégory Clement, Thomas Petazzoni,
	Tawfik Bayouk, Théo Lebrun

Hi,

Three patches are about adding Mobileye EyeQ6H support to the Nomadik
I2C controller driver, in the same vein as was done a few months ago
for EyeQ5.
 - dt-bindings:
    - [PATCH 1/6]: add new compatible.
 - driver:
    - [PATCH 3/6]: we switch from using a match table, before adding the
      new EyeQ6H compatible.
    - [PATCH 4/6]: adding EyeQ6H is only about creating a new match
      table entry thanks to the previous patch; we reuse
      the .has_32b_bus flag created for EyeQ5.

Three patches are about supporting higher speeds (fast-plus and
high-speed).
 - dt-bindings:
    - [PATCH 2/6]: fix them, they indicated only a max of 400kHz.
 - driver:
    - [PATCH 5/6]: fix computation of the bus rate clock divider (BRCR).
      For a bus rate of 400kHz, it would pick >400kHz. Now pick <400kHz.
    - [PATCH 6/6]: support high SM (speed-mode) values.
      This is not much work.

It works on EyeQ6H HW just fine. 1MHz has been tested but not 3.4MHz
because HW doesn't support it. The theory is there, and BRCR
computation has been checked to be valid with 3.4MHz clocks.

DTS patches targeting EyeQ5 & EyeQ6H devicetrees are not provided
because they depend on the platform's clock series [0].

Have a nice day,
Théo

[0]: https://lore.kernel.org/lkml/20241007-mbly-clk-v5-0-e9d8994269cb@bootlin.com/

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
Changes in v3:
- [PATCH 5/6] "i2c: nomadik: fix BRCR computation":
  - Use DIV_ROUND_UP() for BRCR calculation.
  - Improve code comment.
  - Add small note in commit message about DIV_ROUND_UP() usage.
- For all patches but [PATCH 5/6], apply Reviewed-by: Linus Walleij.
- Link to v2: https://lore.kernel.org/r/20241009-mbly-i2c-v2-0-ac9230a8dac5@bootlin.com

Changes in v2:
- dt-bindings: change `clock-frequency: maximum` from 400kHz to 3.4MHz
  in a separate patch.
- dt-bindings: use enum to list compatibles.
- Switch away from of_device_is_compatible() in probe. Use a match table
  with flags inside .data, using of_match_device() from probe.
  This is done as a separate commit before adding EyeQ6H support.
- Link to v1: https://lore.kernel.org/r/20241008-mbly-i2c-v1-0-a06c1317a2f7@bootlin.com

---
Théo Lebrun (6):
      dt-bindings: i2c: nomadik: add mobileye,eyeq6h-i2c bindings
      dt-bindings: i2c: nomadik: support 400kHz < clock-frequency <= 3.4MHz
      i2c: nomadik: switch from of_device_is_compatible() to of_match_device()
      i2c: nomadik: support Mobileye EyeQ6H I2C controller
      i2c: nomadik: fix BRCR computation
      i2c: nomadik: support >=1MHz speed modes

 .../devicetree/bindings/i2c/st,nomadik-i2c.yaml    | 13 ++--
 drivers/i2c/busses/i2c-nomadik.c                   | 89 +++++++++++++---------
 2 files changed, 61 insertions(+), 41 deletions(-)
---
base-commit: 6f1cfa7816af8b3286140f1b0476200d5e914eb9
change-id: 20241007-mbly-i2c-267c9d482b90

Best regards,
-- 
Théo Lebrun <theo.lebrun@bootlin.com>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3 1/6] dt-bindings: i2c: nomadik: add mobileye,eyeq6h-i2c bindings
  2024-10-09 14:01 [PATCH v3 0/6] i2c: nomadik: support >=1MHz & Mobileye EyeQ6H platform Théo Lebrun
@ 2024-10-09 14:01 ` Théo Lebrun
  2024-10-09 21:17   ` Rob Herring (Arm)
  2024-10-09 14:01 ` [PATCH v3 2/6] dt-bindings: i2c: nomadik: support 400kHz < clock-frequency <= 3.4MHz Théo Lebrun
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 12+ messages in thread
From: Théo Lebrun @ 2024-10-09 14:01 UTC (permalink / raw)
  To: Linus Walleij, Andi Shyti, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-kernel, linux-i2c, devicetree, linux-kernel,
	Vladimir Kondratiev, Grégory Clement, Thomas Petazzoni,
	Tawfik Bayouk, Théo Lebrun

After EyeQ5, it is time for Mobileye EyeQ6H to reuse the Nomadik I2C
controller. Add a specific compatible because its HW integration is
slightly different from EyeQ5.

Do NOT add an example as it looks like EyeQ5 from a DT standpoint
(without the mobileye,olb property).

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml b/Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml
index 44c54b162bb10741ec7aac70d165403c28176eba..7e84465c20094b799697a71a66c66d144d621f46 100644
--- a/Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml
@@ -22,6 +22,7 @@ select:
         enum:
           - st,nomadik-i2c
           - mobileye,eyeq5-i2c
+          - mobileye,eyeq6h-i2c
   required:
     - compatible
 
@@ -29,15 +30,15 @@ properties:
   compatible:
     oneOf:
       - items:
-          - const: st,nomadik-i2c
+          - enum:
+              - st,nomadik-i2c
+              - mobileye,eyeq5-i2c
+              - mobileye,eyeq6h-i2c
           - const: arm,primecell
       - items:
           - const: stericsson,db8500-i2c
           - const: st,nomadik-i2c
           - const: arm,primecell
-      - items:
-          - const: mobileye,eyeq5-i2c
-          - const: arm,primecell
 
   reg:
     maxItems: 1
@@ -54,7 +55,7 @@ properties:
       - items:
           - const: mclk
           - const: apb_pclk
-      # Clock name in DB8500 or EyeQ5
+      # Clock name in DB8500 or EyeQ
       - items:
           - const: i2cclk
           - const: apb_pclk

-- 
2.46.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 2/6] dt-bindings: i2c: nomadik: support 400kHz < clock-frequency <= 3.4MHz
  2024-10-09 14:01 [PATCH v3 0/6] i2c: nomadik: support >=1MHz & Mobileye EyeQ6H platform Théo Lebrun
  2024-10-09 14:01 ` [PATCH v3 1/6] dt-bindings: i2c: nomadik: add mobileye,eyeq6h-i2c bindings Théo Lebrun
@ 2024-10-09 14:01 ` Théo Lebrun
  2024-10-09 21:17   ` Rob Herring (Arm)
  2024-10-09 14:01 ` [PATCH v3 3/6] i2c: nomadik: switch from of_device_is_compatible() to of_match_device() Théo Lebrun
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 12+ messages in thread
From: Théo Lebrun @ 2024-10-09 14:01 UTC (permalink / raw)
  To: Linus Walleij, Andi Shyti, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-kernel, linux-i2c, devicetree, linux-kernel,
	Vladimir Kondratiev, Grégory Clement, Thomas Petazzoni,
	Tawfik Bayouk, Théo Lebrun

Hardware is not limited to 400kHz, its documentation does mention how to
configure it for high-speed (a specific Speed-Mode enum value and
a different bus rate clock divider register to be used).

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml b/Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml
index 7e84465c20094b799697a71a66c66d144d621f46..012402debfeb244b85dcecdc0411a77ada4494df 100644
--- a/Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml
+++ b/Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml
@@ -68,7 +68,7 @@ properties:
 
   clock-frequency:
     minimum: 1
-    maximum: 400000
+    maximum: 3400000
 
   mobileye,olb:
     $ref: /schemas/types.yaml#/definitions/phandle-array

-- 
2.46.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 3/6] i2c: nomadik: switch from of_device_is_compatible() to of_match_device()
  2024-10-09 14:01 [PATCH v3 0/6] i2c: nomadik: support >=1MHz & Mobileye EyeQ6H platform Théo Lebrun
  2024-10-09 14:01 ` [PATCH v3 1/6] dt-bindings: i2c: nomadik: add mobileye,eyeq6h-i2c bindings Théo Lebrun
  2024-10-09 14:01 ` [PATCH v3 2/6] dt-bindings: i2c: nomadik: support 400kHz < clock-frequency <= 3.4MHz Théo Lebrun
@ 2024-10-09 14:01 ` Théo Lebrun
  2024-10-09 14:01 ` [PATCH v3 4/6] i2c: nomadik: support Mobileye EyeQ6H I2C controller Théo Lebrun
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Théo Lebrun @ 2024-10-09 14:01 UTC (permalink / raw)
  To: Linus Walleij, Andi Shyti, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-kernel, linux-i2c, devicetree, linux-kernel,
	Vladimir Kondratiev, Grégory Clement, Thomas Petazzoni,
	Tawfik Bayouk, Théo Lebrun

Compatible-specific behavior is implemented using a if-condition on the
return value from of_device_is_compatible(), from probe. It does not
scale well when compatible number increases. Switch to using a match
table and a call to of_match_device().

We DO NOT attach a .of_match_table field to our amba driver, as we do
not use the table to match our driver to devices.

Sort probe variable declarations in reverse christmas tree to try and
introduce some logic into the ordering.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/i2c/busses/i2c-nomadik.c | 37 ++++++++++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index ad0f02acdb1215a1c04729f97bb14a4d93f88456..c40328d1bca6cdefc61906cf9160f8411e37922a 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -26,6 +26,7 @@
 #include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_device.h>
 #include <linux/pinctrl/consumer.h>
 #include <linux/pm_runtime.h>
 #include <linux/regmap.h>
@@ -1046,8 +1047,6 @@ static int nmk_i2c_eyeq5_probe(struct nmk_i2c_dev *priv)
 	struct regmap *olb;
 	unsigned int id;
 
-	priv->has_32b_bus = true;
-
 	olb = syscon_regmap_lookup_by_phandle_args(np, "mobileye,olb", 1, &id);
 	if (IS_ERR(olb))
 		return PTR_ERR(olb);
@@ -1068,15 +1067,35 @@ static int nmk_i2c_eyeq5_probe(struct nmk_i2c_dev *priv)
 	return 0;
 }
 
+#define NMK_I2C_EYEQ_FLAG_32B_BUS	BIT(0)
+#define NMK_I2C_EYEQ_FLAG_IS_EYEQ5	BIT(1)
+
+static const struct of_device_id nmk_i2c_eyeq_match_table[] = {
+	{
+		.compatible = "mobileye,eyeq5-i2c",
+		.data = (void *)(NMK_I2C_EYEQ_FLAG_32B_BUS | NMK_I2C_EYEQ_FLAG_IS_EYEQ5),
+	},
+};
+
 static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 {
-	int ret = 0;
-	struct nmk_i2c_dev *priv;
-	struct device_node *np = adev->dev.of_node;
-	struct device *dev = &adev->dev;
-	struct i2c_adapter *adap;
 	struct i2c_vendor_data *vendor = id->data;
 	u32 max_fifo_threshold = (vendor->fifodepth / 2) - 1;
+	struct device_node *np = adev->dev.of_node;
+	const struct of_device_id *match;
+	struct device *dev = &adev->dev;
+	unsigned long match_flags = 0;
+	struct nmk_i2c_dev *priv;
+	struct i2c_adapter *adap;
+	int ret = 0;
+
+	/*
+	 * We do not want to attach a .of_match_table to our amba driver.
+	 * Do not convert to device_get_match_data().
+	 */
+	match = of_match_device(nmk_i2c_eyeq_match_table, dev);
+	if (match)
+		match_flags = (unsigned long)match->data;
 
 	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 	if (!priv)
@@ -1084,10 +1103,10 @@ static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)
 
 	priv->vendor = vendor;
 	priv->adev = adev;
-	priv->has_32b_bus = false;
+	priv->has_32b_bus = match_flags & NMK_I2C_EYEQ_FLAG_32B_BUS;
 	nmk_i2c_of_probe(np, priv);
 
-	if (of_device_is_compatible(np, "mobileye,eyeq5-i2c")) {
+	if (match_flags & NMK_I2C_EYEQ_FLAG_IS_EYEQ5) {
 		ret = nmk_i2c_eyeq5_probe(priv);
 		if (ret)
 			return dev_err_probe(dev, ret, "failed OLB lookup\n");

-- 
2.46.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 4/6] i2c: nomadik: support Mobileye EyeQ6H I2C controller
  2024-10-09 14:01 [PATCH v3 0/6] i2c: nomadik: support >=1MHz & Mobileye EyeQ6H platform Théo Lebrun
                   ` (2 preceding siblings ...)
  2024-10-09 14:01 ` [PATCH v3 3/6] i2c: nomadik: switch from of_device_is_compatible() to of_match_device() Théo Lebrun
@ 2024-10-09 14:01 ` Théo Lebrun
  2024-10-09 14:01 ` [PATCH v3 5/6] i2c: nomadik: fix BRCR computation Théo Lebrun
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 12+ messages in thread
From: Théo Lebrun @ 2024-10-09 14:01 UTC (permalink / raw)
  To: Linus Walleij, Andi Shyti, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-kernel, linux-i2c, devicetree, linux-kernel,
	Vladimir Kondratiev, Grégory Clement, Thomas Petazzoni,
	Tawfik Bayouk, Théo Lebrun

Add EyeQ6H support to the nmk-i2c AMBA driver. It shares the same quirk
as EyeQ5: the memory bus only supports 32-bit accesses. Avoid writeb()
and readb() by reusing the same `priv->has_32b_bus` flag.

It does NOT need to write speed-mode specific value into a register;
therefore it does not depend on the mobileye,olb DT property.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/i2c/busses/i2c-nomadik.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index c40328d1bca6cdefc61906cf9160f8411e37922a..8f52ae4d6285af2dd2b3dc7070672757e831a019 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -6,10 +6,10 @@
  * I2C master mode controller driver, used in Nomadik 8815
  * and Ux500 platforms.
  *
- * The Mobileye EyeQ5 platform is also supported; it uses
+ * The Mobileye EyeQ5 and EyeQ6H platforms are also supported; they use
  * the same Ux500/DB8500 IP block with two quirks:
  *  - The memory bus only supports 32-bit accesses.
- *  - A register must be configured for the I2C speed mode;
+ *  - (only EyeQ5) A register must be configured for the I2C speed mode;
  *    it is located in a shared register region called OLB.
  *
  * Author: Srinidhi Kasagar <srinidhi.kasagar@stericsson.com>
@@ -1075,6 +1075,10 @@ static const struct of_device_id nmk_i2c_eyeq_match_table[] = {
 		.compatible = "mobileye,eyeq5-i2c",
 		.data = (void *)(NMK_I2C_EYEQ_FLAG_32B_BUS | NMK_I2C_EYEQ_FLAG_IS_EYEQ5),
 	},
+	{
+		.compatible = "mobileye,eyeq6h-i2c",
+		.data = (void *)NMK_I2C_EYEQ_FLAG_32B_BUS,
+	},
 };
 
 static int nmk_i2c_probe(struct amba_device *adev, const struct amba_id *id)

-- 
2.46.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 5/6] i2c: nomadik: fix BRCR computation
  2024-10-09 14:01 [PATCH v3 0/6] i2c: nomadik: support >=1MHz & Mobileye EyeQ6H platform Théo Lebrun
                   ` (3 preceding siblings ...)
  2024-10-09 14:01 ` [PATCH v3 4/6] i2c: nomadik: support Mobileye EyeQ6H I2C controller Théo Lebrun
@ 2024-10-09 14:01 ` Théo Lebrun
  2024-10-09 15:52   ` Linus Walleij
  2024-10-09 14:01 ` [PATCH v3 6/6] i2c: nomadik: support >=1MHz speed modes Théo Lebrun
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 12+ messages in thread
From: Théo Lebrun @ 2024-10-09 14:01 UTC (permalink / raw)
  To: Linus Walleij, Andi Shyti, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-kernel, linux-i2c, devicetree, linux-kernel,
	Vladimir Kondratiev, Grégory Clement, Thomas Petazzoni,
	Tawfik Bayouk, Théo Lebrun

Current BRCR computation is:

    brcr = floor(i2cclk / (clkfreq * div))

With brcr: "baud rate counter", an internal clock divider,
 and i2cclk: input clock rate (24MHz, 38.4MHz or 48MHz),
 and clkfreq: desired bus rate,
 and div: speed-mode dependent divider (2 for standard, 3 otherwise).

Assume i2cclk=48MHz, clkfreq=3.4MHz, div=3,
  then brcr = floor(48MHz / (3.4MHz * 3)) = 4
   and resulting bus rate = 48MHz / (4 * 3) = 4MHz

Assume i2cclk=38.4MHz, clkfreq=1.0MHz, div=3,
  then brcr = floor(38.4MHz / (1.0MHz * 3)) = 12
   and resulting bus rate = 38.4MHz / (12 * 3) = 1066kHz

The current computation means we always pick the smallest divider that
gives a bus rate above target. We should instead pick the largest
divider that gives a bus rate below target, using:

    brcr = floor(i2cclk / (clkfreq * div)) + 1

If we redo the above examples:

Assume i2cclk=48MHz, clkfreq=3.4MHz, div=3,
  then brcr = floor(48MHz / (3.4MHz * 3)) + 1 = 5
   and resulting bus rate = 48MHz / (5 * 3) = 3.2MHz

Assume i2cclk=38.4MHz, clkfreq=1.0MHz, div=3,
  then brcr = floor(38.4MHz / (1.0MHz * 3)) + 1 = 13
   and resulting bus rate = 38.4MHz / (13 * 3) = 985kHz

In kernel C code, floor(x)   is DIV_ROUND_DOWN() and,
                  floor(x)+1 is DIV_ROUND_UP().

This is much less of an issue with slower bus rates (ie those currently
supported), because the gap from one divider to the next is much
smaller. It however keeps us from always using bus rates superior to
the target.

This fix is required for later on supporting faster bus rates:
I2C_FREQ_MODE_FAST_PLUS (1MHz) and I2C_FREQ_MODE_HIGH_SPEED (3.4MHz).

Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/i2c/busses/i2c-nomadik.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 8f52ae4d6285af2dd2b3dc7070672757e831a019..90e3390cd66de5b3aa47d94de7fb30e1d67ac70e 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -454,9 +454,13 @@ static void setup_i2c_controller(struct nmk_i2c_dev *priv)
 	 * operation, and the other is for std, fast mode, fast mode
 	 * plus operation. Currently we do not supprt high speed mode
 	 * so set brcr1 to 0.
+	 *
+	 * BRCR is a clock divider amount. Pick highest value that
+	 * leads to rate strictly below target. Eg when asking for
+	 * 400kHz you want a bus rate <=400kHz (and not >=400kHz).
 	 */
 	brcr1 = FIELD_PREP(I2C_BRCR_BRCNT1, 0);
-	brcr2 = FIELD_PREP(I2C_BRCR_BRCNT2, i2c_clk / (priv->clk_freq * div));
+	brcr2 = FIELD_PREP(I2C_BRCR_BRCNT2, DIV_ROUND_UP(i2c_clk, priv->clk_freq * div));
 
 	/* set the baud rate counter register */
 	writel((brcr1 | brcr2), priv->virtbase + I2C_BRCR);

-- 
2.46.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 6/6] i2c: nomadik: support >=1MHz speed modes
  2024-10-09 14:01 [PATCH v3 0/6] i2c: nomadik: support >=1MHz & Mobileye EyeQ6H platform Théo Lebrun
                   ` (4 preceding siblings ...)
  2024-10-09 14:01 ` [PATCH v3 5/6] i2c: nomadik: fix BRCR computation Théo Lebrun
@ 2024-10-09 14:01 ` Théo Lebrun
  2024-10-31 16:14 ` [PATCH v3 0/6] i2c: nomadik: support >=1MHz & Mobileye EyeQ6H platform Théo Lebrun
  2024-11-18 23:17 ` Andi Shyti
  7 siblings, 0 replies; 12+ messages in thread
From: Théo Lebrun @ 2024-10-09 14:01 UTC (permalink / raw)
  To: Linus Walleij, Andi Shyti, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley
  Cc: linux-arm-kernel, linux-i2c, devicetree, linux-kernel,
	Vladimir Kondratiev, Grégory Clement, Thomas Petazzoni,
	Tawfik Bayouk, Théo Lebrun

 - BRCR value must go into the BRCR1 field when in high-speed mode.
   It goes into BRCR2 otherwise.

 - Remove fallback to standard mode if priv->sm > I2C_FREQ_MODE_FAST.

 - Set SM properly in probe; previously it only checked STANDARD versus
   FAST. Now we set STANDARD, FAST, FAST_PLUS or HIGH_SPEED.

 - Remove all comment sections saying we only support low-speeds.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 drivers/i2c/busses/i2c-nomadik.c | 40 ++++++++++++++++------------------------
 1 file changed, 16 insertions(+), 24 deletions(-)

diff --git a/drivers/i2c/busses/i2c-nomadik.c b/drivers/i2c/busses/i2c-nomadik.c
index 90e3390cd66de5b3aa47d94de7fb30e1d67ac70e..efb33802804fdde82fe6d38d4f89d0b898f1667e 100644
--- a/drivers/i2c/busses/i2c-nomadik.c
+++ b/drivers/i2c/busses/i2c-nomadik.c
@@ -397,7 +397,7 @@ static u32 load_i2c_mcr_reg(struct nmk_i2c_dev *priv, u16 flags)
  */
 static void setup_i2c_controller(struct nmk_i2c_dev *priv)
 {
-	u32 brcr1, brcr2;
+	u32 brcr;
 	u32 i2c_clk, div;
 	u32 ns;
 	u16 slsu;
@@ -444,7 +444,7 @@ static void setup_i2c_controller(struct nmk_i2c_dev *priv)
 	/*
 	 * The spec says, in case of std. mode the divider is
 	 * 2 whereas it is 3 for fast and fastplus mode of
-	 * operation. TODO - high speed support.
+	 * operation.
 	 */
 	div = (priv->clk_freq > I2C_MAX_STANDARD_MODE_FREQ) ? 3 : 2;
 
@@ -452,34 +452,23 @@ static void setup_i2c_controller(struct nmk_i2c_dev *priv)
 	 * generate the mask for baud rate counters. The controller
 	 * has two baud rate counters. One is used for High speed
 	 * operation, and the other is for std, fast mode, fast mode
-	 * plus operation. Currently we do not supprt high speed mode
-	 * so set brcr1 to 0.
+	 * plus operation.
 	 *
 	 * BRCR is a clock divider amount. Pick highest value that
 	 * leads to rate strictly below target. Eg when asking for
 	 * 400kHz you want a bus rate <=400kHz (and not >=400kHz).
 	 */
-	brcr1 = FIELD_PREP(I2C_BRCR_BRCNT1, 0);
-	brcr2 = FIELD_PREP(I2C_BRCR_BRCNT2, DIV_ROUND_UP(i2c_clk, priv->clk_freq * div));
+	brcr = DIV_ROUND_UP(i2c_clk, priv->clk_freq * div);
+
+	if (priv->sm == I2C_FREQ_MODE_HIGH_SPEED)
+		brcr = FIELD_PREP(I2C_BRCR_BRCNT1, brcr);
+	else
+		brcr = FIELD_PREP(I2C_BRCR_BRCNT2, brcr);
 
 	/* set the baud rate counter register */
-	writel((brcr1 | brcr2), priv->virtbase + I2C_BRCR);
+	writel(brcr, priv->virtbase + I2C_BRCR);
 
-	/*
-	 * set the speed mode. Currently we support
-	 * only standard and fast mode of operation
-	 * TODO - support for fast mode plus (up to 1Mb/s)
-	 * and high speed (up to 3.4 Mb/s)
-	 */
-	if (priv->sm > I2C_FREQ_MODE_FAST) {
-		dev_err(&priv->adev->dev,
-			"do not support this mode defaulting to std. mode\n");
-		brcr2 = FIELD_PREP(I2C_BRCR_BRCNT2,
-				   i2c_clk / (I2C_MAX_STANDARD_MODE_FREQ * 2));
-		writel((brcr1 | brcr2), priv->virtbase + I2C_BRCR);
-		writel(FIELD_PREP(I2C_CR_SM, I2C_FREQ_MODE_STANDARD),
-		       priv->virtbase + I2C_CR);
-	}
+	/* set the speed mode */
 	writel(FIELD_PREP(I2C_CR_SM, priv->sm), priv->virtbase + I2C_CR);
 
 	/* set the Tx and Rx FIFO threshold */
@@ -1020,11 +1009,14 @@ static void nmk_i2c_of_probe(struct device_node *np,
 	if (of_property_read_u32(np, "clock-frequency", &priv->clk_freq))
 		priv->clk_freq = I2C_MAX_STANDARD_MODE_FREQ;
 
-	/* This driver only supports 'standard' and 'fast' modes of operation. */
 	if (priv->clk_freq <= I2C_MAX_STANDARD_MODE_FREQ)
 		priv->sm = I2C_FREQ_MODE_STANDARD;
-	else
+	else if (priv->clk_freq <= I2C_MAX_FAST_MODE_FREQ)
 		priv->sm = I2C_FREQ_MODE_FAST;
+	else if (priv->clk_freq <= I2C_MAX_FAST_MODE_PLUS_FREQ)
+		priv->sm = I2C_FREQ_MODE_FAST_PLUS;
+	else
+		priv->sm = I2C_FREQ_MODE_HIGH_SPEED;
 	priv->tft = 1; /* Tx FIFO threshold */
 	priv->rft = 8; /* Rx FIFO threshold */
 

-- 
2.46.2


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 5/6] i2c: nomadik: fix BRCR computation
  2024-10-09 14:01 ` [PATCH v3 5/6] i2c: nomadik: fix BRCR computation Théo Lebrun
@ 2024-10-09 15:52   ` Linus Walleij
  0 siblings, 0 replies; 12+ messages in thread
From: Linus Walleij @ 2024-10-09 15:52 UTC (permalink / raw)
  To: Théo Lebrun
  Cc: Andi Shyti, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-arm-kernel, linux-i2c, devicetree, linux-kernel,
	Vladimir Kondratiev, Grégory Clement, Thomas Petazzoni,
	Tawfik Bayouk

On Wed, Oct 9, 2024 at 4:01 PM Théo Lebrun <theo.lebrun@bootlin.com> wrote:

> Current BRCR computation is:
>
>     brcr = floor(i2cclk / (clkfreq * div))
>
> With brcr: "baud rate counter", an internal clock divider,
>  and i2cclk: input clock rate (24MHz, 38.4MHz or 48MHz),
>  and clkfreq: desired bus rate,
>  and div: speed-mode dependent divider (2 for standard, 3 otherwise).
>
> Assume i2cclk=48MHz, clkfreq=3.4MHz, div=3,
>   then brcr = floor(48MHz / (3.4MHz * 3)) = 4
>    and resulting bus rate = 48MHz / (4 * 3) = 4MHz
>
> Assume i2cclk=38.4MHz, clkfreq=1.0MHz, div=3,
>   then brcr = floor(38.4MHz / (1.0MHz * 3)) = 12
>    and resulting bus rate = 38.4MHz / (12 * 3) = 1066kHz
>
> The current computation means we always pick the smallest divider that
> gives a bus rate above target. We should instead pick the largest
> divider that gives a bus rate below target, using:
>
>     brcr = floor(i2cclk / (clkfreq * div)) + 1
>
> If we redo the above examples:
>
> Assume i2cclk=48MHz, clkfreq=3.4MHz, div=3,
>   then brcr = floor(48MHz / (3.4MHz * 3)) + 1 = 5
>    and resulting bus rate = 48MHz / (5 * 3) = 3.2MHz
>
> Assume i2cclk=38.4MHz, clkfreq=1.0MHz, div=3,
>   then brcr = floor(38.4MHz / (1.0MHz * 3)) + 1 = 13
>    and resulting bus rate = 38.4MHz / (13 * 3) = 985kHz
>
> In kernel C code, floor(x)   is DIV_ROUND_DOWN() and,
>                   floor(x)+1 is DIV_ROUND_UP().
>
> This is much less of an issue with slower bus rates (ie those currently
> supported), because the gap from one divider to the next is much
> smaller. It however keeps us from always using bus rates superior to
> the target.
>
> This fix is required for later on supporting faster bus rates:
> I2C_FREQ_MODE_FAST_PLUS (1MHz) and I2C_FREQ_MODE_HIGH_SPEED (3.4MHz).
>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>

Excellent, thanks for iterating!
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 1/6] dt-bindings: i2c: nomadik: add mobileye,eyeq6h-i2c bindings
  2024-10-09 14:01 ` [PATCH v3 1/6] dt-bindings: i2c: nomadik: add mobileye,eyeq6h-i2c bindings Théo Lebrun
@ 2024-10-09 21:17   ` Rob Herring (Arm)
  0 siblings, 0 replies; 12+ messages in thread
From: Rob Herring (Arm) @ 2024-10-09 21:17 UTC (permalink / raw)
  To: Théo Lebrun
  Cc: Grégory Clement, Krzysztof Kozlowski, linux-i2c,
	Linus Walleij, Vladimir Kondratiev, linux-kernel,
	Thomas Petazzoni, Conor Dooley, devicetree, Tawfik Bayouk,
	linux-arm-kernel, Andi Shyti


On Wed, 09 Oct 2024 16:01:07 +0200, Théo Lebrun wrote:
> After EyeQ5, it is time for Mobileye EyeQ6H to reuse the Nomadik I2C
> controller. Add a specific compatible because its HW integration is
> slightly different from EyeQ5.
> 
> Do NOT add an example as it looks like EyeQ5 from a DT standpoint
> (without the mobileye,olb property).
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 2/6] dt-bindings: i2c: nomadik: support 400kHz < clock-frequency <= 3.4MHz
  2024-10-09 14:01 ` [PATCH v3 2/6] dt-bindings: i2c: nomadik: support 400kHz < clock-frequency <= 3.4MHz Théo Lebrun
@ 2024-10-09 21:17   ` Rob Herring (Arm)
  0 siblings, 0 replies; 12+ messages in thread
From: Rob Herring (Arm) @ 2024-10-09 21:17 UTC (permalink / raw)
  To: Théo Lebrun
  Cc: Grégory Clement, linux-i2c, Thomas Petazzoni, Andi Shyti,
	Krzysztof Kozlowski, Vladimir Kondratiev, linux-kernel,
	linux-arm-kernel, devicetree, Linus Walleij, Tawfik Bayouk,
	Conor Dooley


On Wed, 09 Oct 2024 16:01:08 +0200, Théo Lebrun wrote:
> Hardware is not limited to 400kHz, its documentation does mention how to
> configure it for high-speed (a specific Speed-Mode enum value and
> a different bus rate clock divider register to be used).
> 
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  Documentation/devicetree/bindings/i2c/st,nomadik-i2c.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 0/6] i2c: nomadik: support >=1MHz & Mobileye EyeQ6H platform
  2024-10-09 14:01 [PATCH v3 0/6] i2c: nomadik: support >=1MHz & Mobileye EyeQ6H platform Théo Lebrun
                   ` (5 preceding siblings ...)
  2024-10-09 14:01 ` [PATCH v3 6/6] i2c: nomadik: support >=1MHz speed modes Théo Lebrun
@ 2024-10-31 16:14 ` Théo Lebrun
  2024-11-18 23:17 ` Andi Shyti
  7 siblings, 0 replies; 12+ messages in thread
From: Théo Lebrun @ 2024-10-31 16:14 UTC (permalink / raw)
  To: Théo Lebrun, Linus Walleij, Andi Shyti, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley
  Cc: linux-arm-kernel, linux-i2c, devicetree, linux-kernel,
	Vladimir Kondratiev, Grégory Clement, Thomas Petazzoni,
	Tawfik Bayouk

Hi Andi,

On Wed Oct 9, 2024 at 4:01 PM CEST, Théo Lebrun wrote:
> Three patches are about adding Mobileye EyeQ6H support to the Nomadik
> I2C controller driver, in the same vein as was done a few months ago
> for EyeQ5.
[...]
> Three patches are about supporting higher speeds (fast-plus and
> high-speed).

I come in checking. What's your state of mind about that series?

Rob has given his Reviewed-By on both dt-bindings patches, and Linus
gave his on all patches. I can send a new revision taking the three
Reviewed-By trailers from this V3 if you like.

Also, i2c-host-next is free of conflicts on Nomadik files.

Thanks!

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3 0/6] i2c: nomadik: support >=1MHz & Mobileye EyeQ6H platform
  2024-10-09 14:01 [PATCH v3 0/6] i2c: nomadik: support >=1MHz & Mobileye EyeQ6H platform Théo Lebrun
                   ` (6 preceding siblings ...)
  2024-10-31 16:14 ` [PATCH v3 0/6] i2c: nomadik: support >=1MHz & Mobileye EyeQ6H platform Théo Lebrun
@ 2024-11-18 23:17 ` Andi Shyti
  7 siblings, 0 replies; 12+ messages in thread
From: Andi Shyti @ 2024-11-18 23:17 UTC (permalink / raw)
  To: Théo Lebrun
  Cc: Linus Walleij, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	linux-arm-kernel, linux-i2c, devicetree, linux-kernel,
	Vladimir Kondratiev, Grégory Clement, Thomas Petazzoni,
	Tawfik Bayouk

Hi Theo,

> Théo Lebrun (6):
>       dt-bindings: i2c: nomadik: add mobileye,eyeq6h-i2c bindings
>       dt-bindings: i2c: nomadik: support 400kHz < clock-frequency <= 3.4MHz
>       i2c: nomadik: switch from of_device_is_compatible() to of_match_device()
>       i2c: nomadik: support Mobileye EyeQ6H I2C controller
>       i2c: nomadik: fix BRCR computation
>       i2c: nomadik: support >=1MHz speed modes

merged to i2c/i2c-host.

Thanks,
Andi

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2024-11-18 23:17 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-09 14:01 [PATCH v3 0/6] i2c: nomadik: support >=1MHz & Mobileye EyeQ6H platform Théo Lebrun
2024-10-09 14:01 ` [PATCH v3 1/6] dt-bindings: i2c: nomadik: add mobileye,eyeq6h-i2c bindings Théo Lebrun
2024-10-09 21:17   ` Rob Herring (Arm)
2024-10-09 14:01 ` [PATCH v3 2/6] dt-bindings: i2c: nomadik: support 400kHz < clock-frequency <= 3.4MHz Théo Lebrun
2024-10-09 21:17   ` Rob Herring (Arm)
2024-10-09 14:01 ` [PATCH v3 3/6] i2c: nomadik: switch from of_device_is_compatible() to of_match_device() Théo Lebrun
2024-10-09 14:01 ` [PATCH v3 4/6] i2c: nomadik: support Mobileye EyeQ6H I2C controller Théo Lebrun
2024-10-09 14:01 ` [PATCH v3 5/6] i2c: nomadik: fix BRCR computation Théo Lebrun
2024-10-09 15:52   ` Linus Walleij
2024-10-09 14:01 ` [PATCH v3 6/6] i2c: nomadik: support >=1MHz speed modes Théo Lebrun
2024-10-31 16:14 ` [PATCH v3 0/6] i2c: nomadik: support >=1MHz & Mobileye EyeQ6H platform Théo Lebrun
2024-11-18 23:17 ` Andi Shyti

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).