[PATCH 0/3] i2c: mux: gpio: Add 'transition-delay-ms' property

[PATCH 0/3] i2c: mux: gpio: Add 'transition-delay-ms' property

[Bastien Curutchet]

Hi all,

The i2c-gpio-mux can be used to describe a multiplexer built upon
several i2c isolators having an enable pin (such as LTC4310):

 +---------------+                     +------+  +------+
 | +-----------+ |                     | dev  |  | dev  |
 | | GPIO_EN_A |-|-----------|         +------+  +------+
 | +-----------+ |     +-----+---+         |         |
 |               |  |--| isol. A |---------+---------+
 |     +-----+   |  |  +---------+
 | SOC | I2C |---|--|
 |     +-----+   |  |  +---------+
 |               |  |--| isol. B |------+---------+---------+
 | +-----------+ |     +-----+---+      |         |         |
 | | GPIO_EN_B |-|-----------|      +------+  +------+  +------+
 | +-----------+ |                  | dev  |  | dev  |  | dev  |
 +---------------+                  +------+  +------+  +------+

These isolators often need some time between their enable pin's
assertion and the first i2c transfer. If the first i2c transfer
happens before this enabling time is reached, transfer fails.

There is no available option to configure such a time in the
i2c-gpio-mux driver.

Add a optional property in the bindings called 'transition-delay-ms'.
If present, driver waits for this delay every time a new bus is
selected, i.e. before returning from the bus_select() callback.

Bastien Curutchet (3):
  dt-bindings: i2c: gpio: Add 'transition-delay-ms' property
  i2c: mux: gpio: Re-order #include to match alphabetic order
  i2c: mux: gpio: Add support for the 'transition-delay-ms' property

 .../devicetree/bindings/i2c/i2c-mux-gpio.yaml      |  3 +++
 drivers/i2c/muxes/i2c-mux-gpio.c                   | 14 ++++++++++----
 include/linux/platform_data/i2c-mux-gpio.h         |  2 ++
 3 files changed, 15 insertions(+), 4 deletions(-)

-- 
2.44.0

[PATCH 1/3] dt-bindings: i2c: gpio: Add 'transition-delay-ms' property

[Bastien Curutchet]

The i2c-gpio-mux can be used to describe a multiplexer built upon
several i2c isolators having an enable pin (such as LTC4310). These
isolators can need some time between their enable pin's assertion and
the first i2c transfer.

Add a 'transition-delay-ms' property that indicates the delay to be
respected before doing the first i2c transfer.

Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
---
 Documentation/devicetree/bindings/i2c/i2c-mux-gpio.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.yaml b/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.yaml
index f34cc7ad5a00..5bca58b78359 100644
--- a/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.yaml
+++ b/Documentation/devicetree/bindings/i2c/i2c-mux-gpio.yaml
@@ -57,6 +57,9 @@ properties:
       last value used.
     $ref: /schemas/types.yaml#/definitions/uint32
 
+  transition-delay-ms:
+    description: Delay to wait before doing any transfer when a new bus gets selected.
+
 allOf:
   - $ref: i2c-mux.yaml
 
-- 
2.44.0

Re: [PATCH 1/3] dt-bindings: i2c: gpio: Add 'transition-delay-ms' property

[Krzysztof Kozlowski]

On 27/05/2024 13:39, Bastien Curutchet wrote:
> The i2c-gpio-mux can be used to describe a multiplexer built upon
> several i2c isolators having an enable pin (such as LTC4310). These
> isolators can need some time between their enable pin's assertion and
> the first i2c transfer.
> 
> Add a 'transition-delay-ms' property that indicates the delay to be
> respected before doing the first i2c transfer.
> 

That's quite limited hardware description, comparing to cover letter.
Please provide full description here, not in cover letter. This is the
binding, so the hardware part.

Anyway, this does not look like property of mux itself. If there is no
isolator, the mux would work fine, right?

Then why you are not adding this property to every possible bus and I2C
controller? I2C isolator could be placed there as well.

So just like RC binding, that's not a property of I2C mux. Maybe this
fits usage of GPIO RC / delay binding.

Best regards,
Krzysztof

Re: [PATCH 1/3] dt-bindings: i2c: gpio: Add 'transition-delay-ms' property

[Bastien Curutchet]

Hi Krzysztof,

On 5/27/24 16:38, Krzysztof Kozlowski wrote:
> On 27/05/2024 13:39, Bastien Curutchet wrote:
>> The i2c-gpio-mux can be used to describe a multiplexer built upon
>> several i2c isolators having an enable pin (such as LTC4310). These
>> isolators can need some time between their enable pin's assertion and
>> the first i2c transfer.
>>
>> Add a 'transition-delay-ms' property that indicates the delay to be
>> respected before doing the first i2c transfer.
>>
> 
> That's quite limited hardware description, comparing to cover letter.
> Please provide full description here, not in cover letter. This is the
> binding, so the hardware part.

Ok, I'll add details in next iteration.

> 
> Anyway, this does not look like property of mux itself. If there is no
> isolator, the mux would work fine, right?
> 
In the case I'm thinking about, there is no mux at all on the hardware, 
only two isolators. Each of them have several devices behind. I use the 
i2c-gpio-mux to drive the isolators enable pins to always enable only 
one of the isolators at a time.

> Then why you are not adding this property to every possible bus and I2C
> controller? I2C isolator could be placed there as well.
> 
I actually thought about adding a description of I2C isolators because 
my real use case is only one I2C isolator on a I2C bus. The isolator has 
an enable pin that I want to drive low when the bus is unused to save 
power.
But I didn't find a proper way to describe it. I think a property for 
I2C controllers is not ideal to describe the GPIO, the transition-delay 
and the fact that there could be devices in front of the isolator and/or 
devices behind it (see below)
 

                                                      +------------+ 

                                                      |   GPIO     | 

                                                      | controller | 

                                                      +------------+ 

   +------------+                                           | 

   |    I2C     |------+-------------+                      | 

   | controller |      |             |                      | 

   +------------+  +---+---+  +------+------+               | 

                   | dev A |  |    I2C    EN|---------------+ 

                   +-------+  |  isolator   | 

                              +------+------+ 

                                     | 

                                     +-------+-----------+------- 

                                             |           | 

                                         +---+---+   +---+---+ 

                                         | dev B |   | dev C | 

                                         +-------+   +-------+ 


So I started to describe it as a device itself but then I realized that 
I was doing something very similar to the i2c-gpio-mux description 
that's why I finally submitted this patch series.

> So just like RC binding, that's not a property of I2C mux. Maybe this
> fits usage of GPIO RC / delay binding.
> 

IMHO that’s more of a MUX property than an RC binding because every MUX 
needs some time to switch from one bus to another. It’s just that for 
the vast majority of MUXes, this time is so small that it doesn’t need 
to be described.


Best regards,
Bastien

Re: [PATCH 1/3] dt-bindings: i2c: gpio: Add 'transition-delay-ms' property

[Krzysztof Kozlowski]

On 28/05/2024 08:59, Bastien Curutchet wrote:
> 
>> So just like RC binding, that's not a property of I2C mux. Maybe this
>> fits usage of GPIO RC / delay binding.
>>
> 
> IMHO that’s more of a MUX property than an RC binding because every MUX 
> needs some time to switch from one bus to another. It’s just that for 
> the vast majority of MUXes, this time is so small that it doesn’t need 
> to be described.


Hm, true, it's fine then.

However this probably should be microseconds not milliseconds to allow
greater granularity, just like gpio-delay.

Best regards,
Krzysztof

[PATCH 2/3] i2c: mux: gpio: Re-order #include to match alphabetic order

[Bastien Curutchet]

The #includes don't match alphabetic order.

Re-order #includes to match the alphabetic order before adding a new
one.

Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
---
 drivers/i2c/muxes/i2c-mux-gpio.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index d6bbb8b68333..c61e9d9ea695 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -5,16 +5,16 @@
  * Peter Korsgaard <peter.korsgaard@barco.com>
  */
 
+#include <linux/bits.h>
+#include <linux/gpio/consumer.h>
+#include <linux/gpio/driver.h>
 #include <linux/i2c.h>
 #include <linux/i2c-mux.h>
+#include <linux/module.h>
 #include <linux/overflow.h>
 #include <linux/platform_data/i2c-mux-gpio.h>
 #include <linux/platform_device.h>
-#include <linux/module.h>
 #include <linux/slab.h>
-#include <linux/bits.h>
-#include <linux/gpio/consumer.h>
-#include <linux/gpio/driver.h>
 
 struct gpiomux {
 	struct i2c_mux_gpio_platform_data data;
-- 
2.44.0

[PATCH 3/3] i2c: mux: gpio: Add support for the 'transition-delay-ms' property

[Bastien Curutchet]

Some hardware need some time to switch from a bus to another. This can
cause the first transfers following the selection of a bus to fail.
There is no way to configure this kind of waiting time in the driver.

Add support for the 'transition-delay-ms' device-tree property. When set,
the i2c_mux_gpio_select() applies a delay before returning, leaving
enough time to the hardware to switch to the new bus.

Signed-off-by: Bastien Curutchet <bastien.curutchet@bootlin.com>
---
 drivers/i2c/muxes/i2c-mux-gpio.c           | 6 ++++++
 include/linux/platform_data/i2c-mux-gpio.h | 2 ++
 2 files changed, 8 insertions(+)

diff --git a/drivers/i2c/muxes/i2c-mux-gpio.c b/drivers/i2c/muxes/i2c-mux-gpio.c
index c61e9d9ea695..d2cbbaa09485 100644
--- a/drivers/i2c/muxes/i2c-mux-gpio.c
+++ b/drivers/i2c/muxes/i2c-mux-gpio.c
@@ -6,6 +6,7 @@
  */
 
 #include <linux/bits.h>
+#include <linux/delay.h>
 #include <linux/gpio/consumer.h>
 #include <linux/gpio/driver.h>
 #include <linux/i2c.h>
@@ -37,6 +38,9 @@ static int i2c_mux_gpio_select(struct i2c_mux_core *muxc, u32 chan)
 
 	i2c_mux_gpio_set(mux, chan);
 
+	if (mux->data.transition_delay)
+		mdelay(mux->data.transition_delay);
+
 	return 0;
 }
 
@@ -116,6 +120,8 @@ static int i2c_mux_gpio_probe_fw(struct gpiomux *mux,
 	if (device_property_read_u32(dev, "idle-state", &mux->data.idle))
 		mux->data.idle = I2C_MUX_GPIO_NO_IDLE;
 
+	device_property_read_u32(dev, "transition-delay-ms", &mux->data.transition_delay);
+
 	return 0;
 }
 
diff --git a/include/linux/platform_data/i2c-mux-gpio.h b/include/linux/platform_data/i2c-mux-gpio.h
index 816a4cd3ccb5..c449f714d32b 100644
--- a/include/linux/platform_data/i2c-mux-gpio.h
+++ b/include/linux/platform_data/i2c-mux-gpio.h
@@ -19,6 +19,7 @@
  *	position
  * @n_values: Number of multiplexer positions (busses to instantiate)
  * @idle: Bitmask to write to MUX when idle or GPIO_I2CMUX_NO_IDLE if not used
+ * @transition_delay: Delay to wait when a new bus is selected
  */
 struct i2c_mux_gpio_platform_data {
 	int parent;
@@ -26,6 +27,7 @@ struct i2c_mux_gpio_platform_data {
 	const unsigned *values;
 	int n_values;
 	unsigned idle;
+	int transition_delay;
 };
 
 #endif /* _LINUX_I2C_MUX_GPIO_H */
-- 
2.44.0