From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Implement bus recovery by reinitializing the hardware to reset the bus
state and generating 9 clock cycles (and a stop condition) to release
the SDA line.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
v1->v2
- Used single register read to check SDA/SCL lines
---
 drivers/i2c/busses/i2c-riic.c | 100 ++++++++++++++++++++++++++++++----
 1 file changed, 90 insertions(+), 10 deletions(-)

diff --git a/drivers/i2c/busses/i2c-riic.c b/drivers/i2c/busses/i2c-riic.c
index 586092454bb2..d93c371a22de 100644
--- a/drivers/i2c/busses/i2c-riic.c
+++ b/drivers/i2c/busses/i2c-riic.c
@@ -50,6 +50,7 @@
 
 #define ICCR1_ICE	BIT(7)
 #define ICCR1_IICRST	BIT(6)
+#define ICCR1_CLO	BIT(5)
 #define ICCR1_SOWP	BIT(4)
 #define ICCR1_SCLI	BIT(1)
 #define ICCR1_SDAI	BIT(0)
@@ -68,6 +69,7 @@
 #define ICMR3_ACKBT	BIT(3)
 
 #define ICFER_FMPE	BIT(7)
+#define ICFER_MALE	BIT(1)
 
 #define ICIER_TIE	BIT(7)
 #define ICIER_TEIE	BIT(6)
@@ -81,6 +83,8 @@
 
 #define RIIC_INIT_MSG	-1
 
+#define RIIC_RECOVERY_CLK_CNT	9
+
 enum riic_reg_list {
 	RIIC_ICCR1 = 0,
 	RIIC_ICCR2,
@@ -150,13 +154,16 @@ static int riic_bus_barrier(struct riic_dev *riic)
 	ret = readb_poll_timeout(riic->base + riic->info->regs[RIIC_ICCR2], val,
 				 !(val & ICCR2_BBSY), 10, riic->adapter.timeout);
 	if (ret)
-		return -EBUSY;
+		goto i2c_recover;
 
 	if ((riic_readb(riic, RIIC_ICCR1) & (ICCR1_SDAI | ICCR1_SCLI)) !=
 	     (ICCR1_SDAI | ICCR1_SCLI))
-		return -EBUSY;
+		goto i2c_recover;
 
 	return 0;
+
+i2c_recover:
+	return i2c_recover_bus(&riic->adapter);
 }
 
 static int riic_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num)
@@ -332,7 +339,7 @@ static const struct i2c_algorithm riic_algo = {
 	.functionality = riic_func,
 };
 
-static int riic_init_hw(struct riic_dev *riic)
+static int riic_init_hw(struct riic_dev *riic, bool recover)
 {
 	int ret;
 	unsigned long rate;
@@ -414,9 +421,11 @@ static int riic_init_hw(struct riic_dev *riic)
 		 rate / total_ticks, ((brl + 3) * 100) / (brl + brh + 6),
 		 t->scl_fall_ns / ns_per_tick, t->scl_rise_ns / ns_per_tick, cks, brl, brh);
 
-	ret = pm_runtime_resume_and_get(dev);
-	if (ret)
-		return ret;
+	if (!recover) {
+		ret = pm_runtime_resume_and_get(dev);
+		if (ret)
+			return ret;
+	}
 
 	/* Changing the order of accessing IICRST and ICE may break things! */
 	riic_writeb(riic, ICCR1_IICRST | ICCR1_SOWP, RIIC_ICCR1);
@@ -434,8 +443,74 @@ static int riic_init_hw(struct riic_dev *riic)
 
 	riic_clear_set_bit(riic, ICCR1_IICRST, 0, RIIC_ICCR1);
 
-	pm_runtime_mark_last_busy(dev);
-	pm_runtime_put_autosuspend(dev);
+	if (!recover) {
+		pm_runtime_mark_last_busy(dev);
+		pm_runtime_put_autosuspend(dev);
+	}
+	return 0;
+}
+
+static int riic_recover_bus(struct i2c_adapter *adap)
+{
+	struct riic_dev *riic = i2c_get_adapdata(adap);
+	struct device *dev = riic->adapter.dev.parent;
+	int ret;
+	u8 val;
+
+	ret = riic_init_hw(riic, true);
+	if (ret)
+		return -EINVAL;
+
+	/* output extra SCL clock cycles with master arbitration-lost detection disabled */
+	riic_clear_set_bit(riic, ICFER_MALE, 0, RIIC_ICFER);
+
+	for (unsigned int i = 0; i < RIIC_RECOVERY_CLK_CNT; i++) {
+		riic_clear_set_bit(riic, 0, ICCR1_CLO, RIIC_ICCR1);
+		ret = readb_poll_timeout(riic->base + riic->info->regs[RIIC_ICCR1], val,
+					 !(val & ICCR1_CLO), 0, 100);
+		if (ret) {
+			dev_err(dev, "SCL clock cycle timeout\n");
+			return ret;
+		}
+	}
+
+	/*
+	 * The last clock cycle may have driven the SDA line high, so add a
+	 * short delay to allow the line to stabilize before checking the status.
+	 */
+	udelay(5);
+
+	/*
+	 * If an incomplete byte write occurs, the SDA line may remain low
+	 * even after 9 clock pulses, indicating the bus is not released.
+	 * To resolve this, send an additional clock pulse to simulate a STOP
+	 * condition and ensure proper bus release.
+	 */
+	if ((riic_readb(riic, RIIC_ICCR1) & (ICCR1_SDAI | ICCR1_SCLI)) !=
+	    (ICCR1_SDAI | ICCR1_SCLI)) {
+		riic_clear_set_bit(riic, 0, ICCR1_CLO, RIIC_ICCR1);
+		ret = readb_poll_timeout(riic->base + riic->info->regs[RIIC_ICCR1], val,
+					 !(val & ICCR1_CLO), 0, 100);
+		if (ret) {
+			dev_err(dev, "SCL clock cycle timeout occurred while issuing the STOP condition\n");
+			return ret;
+		}
+		/* delay to make sure SDA line goes back HIGH again */
+		udelay(5);
+	}
+
+	/* clear any flags set */
+	riic_writeb(riic, 0, RIIC_ICSR2);
+	/* read back register to confirm writes */
+	riic_readb(riic, RIIC_ICSR2);
+
+	/* restore back ICFER_MALE */
+	riic_clear_set_bit(riic, 0, ICFER_MALE, RIIC_ICFER);
+
+	if ((riic_readb(riic, RIIC_ICCR1) & (ICCR1_SDAI | ICCR1_SCLI)) !=
+	    (ICCR1_SDAI | ICCR1_SCLI))
+		return -EINVAL;
+
 	return 0;
 }
 
@@ -447,6 +522,10 @@ static const struct riic_irq_desc riic_irqs[] = {
 	{ .res_num = 5, .isr = riic_tend_isr, .name = "riic-nack" },
 };
 
+static struct i2c_bus_recovery_info riic_bri = {
+	.recover_bus = riic_recover_bus,
+};
+
 static int riic_i2c_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -491,6 +570,7 @@ static int riic_i2c_probe(struct platform_device *pdev)
 	strscpy(adap->name, "Renesas RIIC adapter", sizeof(adap->name));
 	adap->owner = THIS_MODULE;
 	adap->algo = &riic_algo;
+	adap->bus_recovery_info = &riic_bri;
 	adap->dev.parent = dev;
 	adap->dev.of_node = dev->of_node;
 
@@ -503,7 +583,7 @@ static int riic_i2c_probe(struct platform_device *pdev)
 	pm_runtime_use_autosuspend(dev);
 	pm_runtime_enable(dev);
 
-	ret = riic_init_hw(riic);
+	ret = riic_init_hw(riic, false);
 	if (ret)
 		goto out;
 
@@ -611,7 +691,7 @@ static int riic_i2c_resume(struct device *dev)
 	if (ret)
 		return ret;
 
-	ret = riic_init_hw(riic);
+	ret = riic_init_hw(riic, false);
 	if (ret) {
 		/*
 		 * In case this happens there is no way to recover from this
-- 
2.43.0

On 18.12.2024 02:16, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Implement bus recovery by reinitializing the hardware to reset the bus
> state and generating 9 clock cycles (and a stop condition) to release
> the SDA line.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> v1->v2
> - Used single register read to check SDA/SCL lines
> ---
>  drivers/i2c/busses/i2c-riic.c | 100 ++++++++++++++++++++++++++++++----
>  1 file changed, 90 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-riic.c b/drivers/i2c/busses/i2c-riic.c
> index 586092454bb2..d93c371a22de 100644
> --- a/drivers/i2c/busses/i2c-riic.c
> +++ b/drivers/i2c/busses/i2c-riic.c
> @@ -50,6 +50,7 @@
>  

[ ... ]

> +static int riic_recover_bus(struct i2c_adapter *adap)
> +{
> +	struct riic_dev *riic = i2c_get_adapdata(adap);
> +	struct device *dev = riic->adapter.dev.parent;
> +	int ret;
> +	u8 val;
> +
> +	ret = riic_init_hw(riic, true);
> +	if (ret)
> +		return -EINVAL;

Any reason for not returning directly the ret? It is true the
riic_init_hw() can, ATM, return only -EINVAL in case of failure.

Thank you,
Claudiu

On Sat, Dec 21, 2024 at 9:13 AM Claudiu Beznea <claudiu.beznea@tuxon.dev> wrote:
>
>
>
> On 18.12.2024 02:16, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Implement bus recovery by reinitializing the hardware to reset the bus
> > state and generating 9 clock cycles (and a stop condition) to release
> > the SDA line.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > v1->v2
> > - Used single register read to check SDA/SCL lines
> > ---
> >  drivers/i2c/busses/i2c-riic.c | 100 ++++++++++++++++++++++++++++++----
> >  1 file changed, 90 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/i2c/busses/i2c-riic.c b/drivers/i2c/busses/i2c-riic.c
> > index 586092454bb2..d93c371a22de 100644
> > --- a/drivers/i2c/busses/i2c-riic.c
> > +++ b/drivers/i2c/busses/i2c-riic.c
> > @@ -50,6 +50,7 @@
> >
>
> [ ... ]
>
> > +static int riic_recover_bus(struct i2c_adapter *adap)
> > +{
> > +     struct riic_dev *riic = i2c_get_adapdata(adap);
> > +     struct device *dev = riic->adapter.dev.parent;
> > +     int ret;
> > +     u8 val;
> > +
> > +     ret = riic_init_hw(riic, true);
> > +     if (ret)
> > +             return -EINVAL;
>
> Any reason for not returning directly the ret? It is true the
> riic_init_hw() can, ATM, return only -EINVAL in case of failure.
>
No reason, I will fix that to return ret directly on failure.

Cheers,
Prabhakar

On Wed, Dec 18, 2024 at 12:16:18AM +0000, Prabhakar wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Implement bus recovery by reinitializing the hardware to reset the bus
> state and generating 9 clock cycles (and a stop condition) to release
> the SDA line.
> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

I need to ask a high level question first: why don't you use the general
scl_recovery algorithm? We have stuff like get/set_scl/sda as well as
(un)prepare_recovery. Won't that do?

Hi Wolfram,

On Fri, Dec 20, 2024 at 9:16 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
> On Wed, Dec 18, 2024 at 12:16:18AM +0000, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> >
> > Implement bus recovery by reinitializing the hardware to reset the bus
> > state and generating 9 clock cycles (and a stop condition) to release
> > the SDA line.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> I need to ask a high level question first: why don't you use the general
> scl_recovery algorithm? We have stuff like get/set_scl/sda as well as
> (un)prepare_recovery. Won't that do?
>
On the RZ/G2L and RZ/G3S there is a restriction for forcing the SDA/SCL states:

● Write:
0: Changes the RIICnSCL/RIICnSDA pin output to a low level.
1: Changes the RIICnSCL/RIICnSDA pin in a high-impedance state.
(High level output is achieved through an external pull-up resistor.)

So using the generic algorithm may be platform dependent as it would
only work on platforms which have external pull-up resistor on SDA/SCL
pins. So to overcome this and make recovery possible on the platforms
I choose the RIIC feature to output clock cycles as required.

Cheers,
Prabhakar

> On the RZ/G2L and RZ/G3S there is a restriction for forcing the SDA/SCL states:
> 
> ● Write:
> 0: Changes the RIICnSCL/RIICnSDA pin output to a low level.
> 1: Changes the RIICnSCL/RIICnSDA pin in a high-impedance state.
> (High level output is achieved through an external pull-up resistor.)
> 
> So using the generic algorithm may be platform dependent as it would
> only work on platforms which have external pull-up resistor on SDA/SCL
> pins. So to overcome this and make recovery possible on the platforms
> I choose the RIIC feature to output clock cycles as required.

I would be super-surprised if this is really a restriction and not a
very precise documentation. In other words, I am quite sure that there
is no difference between the bit forcing SCL high (via high-impedance)
and the internal RIIC handling when it needs SCL high. Most I2C busses
are open-drain anyhow.

Or is it confirmed by hardware engineers that RIIC is able to support
push/pull-busses but only this bit cannot?

Hi Wolfram,

On Sun, Dec 22, 2024 at 12:44 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
>
> > On the RZ/G2L and RZ/G3S there is a restriction for forcing the SDA/SCL states:
> >
> > ● Write:
> > 0: Changes the RIICnSCL/RIICnSDA pin output to a low level.
> > 1: Changes the RIICnSCL/RIICnSDA pin in a high-impedance state.
> > (High level output is achieved through an external pull-up resistor.)
> >
> > So using the generic algorithm may be platform dependent as it would
> > only work on platforms which have external pull-up resistor on SDA/SCL
> > pins. So to overcome this and make recovery possible on the platforms
> > I choose the RIIC feature to output clock cycles as required.
>
> I would be super-surprised if this is really a restriction and not a
> very precise documentation. In other words, I am quite sure that there
> is no difference between the bit forcing SCL high (via high-impedance)
> and the internal RIIC handling when it needs SCL high. Most I2C busses
> are open-drain anyhow.
>
> Or is it confirmed by hardware engineers that RIIC is able to support
> push/pull-busses but only this bit cannot?
>
Based on the feedback from the HW engineer the restriction is valid
see attached image (i2c-pullup.png). The SCL and SDA are Schmitt
input/open-drain output pins for both master and slave operations.
Because the output is open drain, an external pull-up resistor is
required.

Assuming there is an external pull-up resistor for all the platforms I
implemented the I2C bus recovery using the generic recovery algorithm
and I'm seeing issues, as the required number of clock pulses are not
being triggered (Note, the i2c clock frequency is 400000Hz where the
below tests are run).

I'm not sure if the below restriction is causing an issue:
--------------------
The SDAO and SCLO bits directly control the SDAn and SCLn signals
output from the RIIC. When writing to these bits, also write 0b to the
SOWP bit. Setting these bits results in input to the RIIC by the input
buffer. When slave mode is selected, a start condition might be
detected and the bus might be released, depending on the bit settings.
Do not rewrite these bits during a start condition, stop condition,
restart condition, or during transmission or reception. Operation
after rewriting under the specified conditions is not guaranteed. When
reading these bits, the state of signals output from the RIIC can be
read.

Ive run the below sequence and attached the images where the recovery
is failing, during recovery I see there aren't reliable clock pulses
see the attached images i2c-error-*.png

$ echo 0x68 > incomplete_address_phase;i2cget -y -f 3 0x68 8
See images  i2c-error-0.png and i2c-error-1.png ---> In here we can
see the i2c error is injected and the SDA line goes low and for
recovery we can see only 6 clock pulses were forced

$ i2cget -y -f 3 0x68 8 #trigger recovery again by reading
See images  i2c-error-2.png --->In here we can see only 3 clock pulses
were forced

$ i2cget -y -f 3 0x68 8  #trigger recovery again by reading
See images  i2c-error-3.png ---> This is where the i2c has recovered
successfully and we can see proper 9 clock pulses

Below is the I2C recovery code using the generic algorithm:
diff --git a/drivers/i2c/busses/i2c-riic.c b/drivers/i2c/busses/i2c-riic.c
index 850f640d8fd4..3b9cb26ede3d 100644
--- a/drivers/i2c/busses/i2c-riic.c
+++ b/drivers/i2c/busses/i2c-riic.c
@@ -54,6 +54,8 @@
 #define ICCR1_IICRST   BIT(6)
 #define ICCR1_CLO      BIT(5)
 #define ICCR1_SOWP     BIT(4)
+#define ICCR1_SCLO     BIT(3)
+#define ICCR1_SDAO     BIT(2)
 #define ICCR1_SCLI     BIT(1)
 #define ICCR1_SDAI     BIT(0)

@@ -181,7 +183,7 @@ static int riic_xfer(struct i2c_adapter *adap,
struct i2c_msg msgs[], int num)
                return ret;

        riic->err = riic_bus_barrier(riic);
-       if (riic->err)
+       if (riic->err < 0)
                goto out;

        reinit_completion(&riic->msg_done);
@@ -515,6 +517,51 @@ static int riic_recover_bus(struct i2c_adapter *adap)
        return 0;
 }

+static int riic_get_scl(struct i2c_adapter *adap)
+{
+       struct riic_dev *riic = i2c_get_adapdata(adap);
+
+       return !!(riic_readb(riic, RIIC_ICCR1) & ICCR1_SCLI);
+}
+
+static void riic_set_scl(struct i2c_adapter *adap, int val)
+{
+       struct riic_dev *riic = i2c_get_adapdata(adap);
+
+       if (val)
+               riic_clear_set_bit(riic, ICCR1_SOWP, ICCR1_SCLO, RIIC_ICCR1);
+       else
+               riic_clear_set_bit(riic, ICCR1_SOWP | ICCR1_SCLO, 0,
RIIC_ICCR1);
+
+       riic_clear_set_bit(riic, 0, ICCR1_SOWP, RIIC_ICCR1);
+}
+
+static void riic_set_sda(struct i2c_adapter *adap, int val)
+{
+       struct riic_dev *riic = i2c_get_adapdata(adap);
+
+       if (val)
+               riic_clear_set_bit(riic, ICCR1_SOWP, ICCR1_SDAO, RIIC_ICCR1);
+       else
+               riic_clear_set_bit(riic, ICCR1_SOWP | ICCR1_SDAO, 0,
RIIC_ICCR1);
+
+       riic_clear_set_bit(riic, 0, ICCR1_SOWP, RIIC_ICCR1);
+}
+
+static int riic_get_bus_free(struct i2c_adapter *adap)
+{
+       struct riic_dev *riic = i2c_get_adapdata(adap);
+
+       udelay(5);
+
+       /* Check if the bus is busy or SDA is not high */
+       if ((riic_readb(riic, RIIC_ICCR2) & ICCR2_BBSY) ||
+           !(riic_readb(riic, RIIC_ICCR1) & ICCR1_SDAI))
+               return -EBUSY;
+
+       return 1;
+}
+
 static const struct riic_irq_desc riic_irqs[] = {
        { .res_num = 0, .isr = riic_tend_isr, .name = "riic-tend" },
        { .res_num = 1, .isr = riic_rdrf_isr, .name = "riic-rdrf" },
@@ -524,7 +571,11 @@ static const struct riic_irq_desc riic_irqs[] = {
 };

 static struct i2c_bus_recovery_info riic_bri = {
-       .recover_bus = riic_recover_bus,
+       .get_scl = riic_get_scl,
+       .set_scl = riic_set_scl,
+       .set_sda = riic_set_sda,
+       .get_bus_free = riic_get_bus_free,
+       .recover_bus = i2c_generic_scl_recovery,
 };


 static int riic_i2c_probe(struct platform_device *pdev)

Hi Prabhakar,

finally some time for this!

> Based on the feedback from the HW engineer the restriction is valid
> see attached image (i2c-pullup.png). The SCL and SDA are Schmitt
> input/open-drain output pins for both master and slave operations.
> Because the output is open drain, an external pull-up resistor is
> required.

That confirms what I was saying. It is required. There is no difference
between setting the bit manually and the IP core doing it internally.

> Assuming there is an external pull-up resistor for all the platforms I
> implemented the I2C bus recovery using the generic recovery algorithm
> and I'm seeing issues, as the required number of clock pulses are not
> being triggered (Note, the i2c clock frequency is 400000Hz where the
> below tests are run).

So, my take is to check further why this is the case. The code looks
mostly good, except for bus_free:

> +static int riic_get_bus_free(struct i2c_adapter *adap)
> +{
> +       struct riic_dev *riic = i2c_get_adapdata(adap);
> +
> +       udelay(5);

I wonder about the udelay here. Both, why this is necessary and where
the value comes from.

> +
> +       /* Check if the bus is busy or SDA is not high */
> +       if ((riic_readb(riic, RIIC_ICCR2) & ICCR2_BBSY) ||
> +           !(riic_readb(riic, RIIC_ICCR1) & ICCR1_SDAI))

And maybe if we can't skip reading SDAI here?

> +               return -EBUSY;
> +
> +       return 1;
> +}

Have you already played with these options? If you didn't and don't have
time to do so, I can also check it. I luckily got a G3S meanwhile.

Happy hacking,

   Wolfram

> Have you already played with these options? If you didn't and don't have
> time to do so, I can also check it. I luckily got a G3S meanwhile.

If the BBSY bit does not work the way we need it, maybe it is simpler
and better to just implement get_sda() and drop bus_free()?

Hi Wolfram,

On Sun, Dec 22, 2024 at 12:44 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
>
>
> > On the RZ/G2L and RZ/G3S there is a restriction for forcing the SDA/SCL states:
> >
> > ● Write:
> > 0: Changes the RIICnSCL/RIICnSDA pin output to a low level.
> > 1: Changes the RIICnSCL/RIICnSDA pin in a high-impedance state.
> > (High level output is achieved through an external pull-up resistor.)
> >
> > So using the generic algorithm may be platform dependent as it would
> > only work on platforms which have external pull-up resistor on SDA/SCL
> > pins. So to overcome this and make recovery possible on the platforms
> > I choose the RIIC feature to output clock cycles as required.
>
> I would be super-surprised if this is really a restriction and not a
> very precise documentation. In other words, I am quite sure that there
> is no difference between the bit forcing SCL high (via high-impedance)
> and the internal RIIC handling when it needs SCL high. Most I2C busses
> are open-drain anyhow.
>
I had asked this previously to the HW engineers about the requirement
(as this restriction is not mentioned in the RZ/V2H(P) SoC, Ive seen
it for RZ/A series RZ/G2L family and RZ/G3S only) before the start of
the I2C recovery work but haven't got a response yet. Ive pinged them
again and I'll wait for their feedback.

Cheers,
Prabhakar

Hi,

On Mon, Dec 23, 2024 at 06:35:28AM +0000, Lad, Prabhakar wrote:
> On Sun, Dec 22, 2024 at 12:44 PM Wolfram Sang
> > > ● Write:
> > > 0: Changes the RIICnSCL/RIICnSDA pin output to a low level.
> > > 1: Changes the RIICnSCL/RIICnSDA pin in a high-impedance state.
> > > (High level output is achieved through an external pull-up resistor.)
> > >
> > > So using the generic algorithm may be platform dependent as it would
> > > only work on platforms which have external pull-up resistor on SDA/SCL
> > > pins. So to overcome this and make recovery possible on the platforms
> > > I choose the RIIC feature to output clock cycles as required.
> >
> > I would be super-surprised if this is really a restriction and not a
> > very precise documentation. In other words, I am quite sure that there
> > is no difference between the bit forcing SCL high (via high-impedance)
> > and the internal RIIC handling when it needs SCL high. Most I2C busses
> > are open-drain anyhow.
> >
> I had asked this previously to the HW engineers about the requirement
> (as this restriction is not mentioned in the RZ/V2H(P) SoC, Ive seen
> it for RZ/A series RZ/G2L family and RZ/G3S only) before the start of
> the I2C recovery work but haven't got a response yet. Ive pinged them
> again and I'll wait for their feedback.

Wolfram has commented on a very valid point, on a standard i2c
specification.

I'd like to merge this once all the hardware questions are
answered.

Please, do follow up on this.

Thanks,
Andi

> I'd like to merge this once all the hardware questions are
> answered.

Maybe we can apply patches 1-8 already, so they don't reappear on the
list? IMHO they make sense independently of the bus recovery patch.

Hi Wolfram,

On Fri, Dec 27, 2024 at 12:27:38PM +0100, Wolfram Sang wrote:
> > I'd like to merge this once all the hardware questions are
> > answered.
> 
> Maybe we can apply patches 1-8 already, so they don't reappear on the
> list? IMHO they make sense independently of the bus recovery patch.

yes, this is what I already suggested as well in 0/8. Indeed
Prabhakar sent already patch 1-8 that I just checked and applied
(locally, still).

Thanks for your reviews and test here and thanks Prabhakar for
following on the reviews.

Andi