mbox series

[v2,0/9] Add support for I2C bus recovery for riic driver

Message ID 20241218001618.488946-1-prabhakar.mahadev-lad.rj@bp.renesas.com
Headers show
Series Add support for I2C bus recovery for riic driver | expand

Message

Lad, Prabhakar Dec. 18, 2024, 12:16 a.m. UTC
From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Hi All,

This patch series introduces support for I2C bus recovery in the RIIC
driver, which is utilized in RZ series SoCs. The addition of bus recovery
functionality enhances the reliability of the I2C interface by allowing it
to recover from error conditions that might leave the bus in an unusable
state.

Alongside the bus recovery implementation, the series includes several
cleanup and improvement patches that simplify and modernize the driver
code. These include replacing `dev_err` calls with `dev_err_probe`,
consistent usage of the `BIT` and `GENMASK` macros, leveraging devres
helpers for reset management, and improving code readability by marking
static data as `const`.

v1->v2
- Fixed review comments and collected RB tags from Geert

v1:
https://lore.kernel.org/all/20241213175828.909987-1-prabhakar.mahadev-lad.rj@bp.renesas.com/

Cheers,
Prabhakar

Lad Prabhakar (9):
  i2c: riic: Replace dev_err with dev_err_probe in probe function
  i2c: riic: Use local `dev` pointer in `dev_err_probe()`
  i2c: riic: Use BIT macro consistently
  i2c: riic: Use GENMASK() macro for bitmask definitions
  i2c: riic: Make use of devres helper to request deasserted reset line
  i2c: riic: Mark riic_irqs array as const
  i2c: riic: Use predefined macro and simplify clock tick calculation
  i2c: riic: Add `riic_bus_barrier()` to check bus availability
  i2c: riic: Implement bus recovery

 drivers/i2c/busses/i2c-riic.c | 222 ++++++++++++++++++++++++----------
 1 file changed, 155 insertions(+), 67 deletions(-)

Comments

Geert Uytterhoeven Dec. 18, 2024, 1:30 p.m. UTC | #1
On Wed, Dec 18, 2024 at 1:16 AM Prabhakar <prabhakar.csengg@gmail.com> wrote:
> From: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
>
> Simplify the `riic_i2c_probe()` function by using the
> `devm_reset_control_get_optional_exclusive_deasserted()` API to request a
> deasserted reset line. This eliminates the need to manually deassert the
> reset control and the additional cleanup.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> v1->v2
> - Updated error message

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert
Wolfram Sang Dec. 20, 2024, 12:02 p.m. UTC | #2
> This patch series introduces support for I2C bus recovery in the RIIC
> driver, which is utilized in RZ series SoCs. The addition of bus recovery
> functionality enhances the reliability of the I2C interface by allowing it
> to recover from error conditions that might leave the bus in an unusable
> state.
> 
> Alongside the bus recovery implementation, the series includes several
> cleanup and improvement patches that simplify and modernize the driver
> code. These include replacing `dev_err` calls with `dev_err_probe`,
> consistent usage of the `BIT` and `GENMASK` macros, leveraging devres
> helpers for reset management, and improving code readability by marking
> static data as `const`.

I am planning to review and test this series later today.
Wolfram Sang Dec. 20, 2024, 9:28 p.m. UTC | #3
On Fri, Dec 20, 2024 at 01:02:51PM +0100, Wolfram Sang wrote:
> 
> > This patch series introduces support for I2C bus recovery in the RIIC
> > driver, which is utilized in RZ series SoCs. The addition of bus recovery
> > functionality enhances the reliability of the I2C interface by allowing it
> > to recover from error conditions that might leave the bus in an unusable
> > state.
> > 
> > Alongside the bus recovery implementation, the series includes several
> > cleanup and improvement patches that simplify and modernize the driver
> > code. These include replacing `dev_err` calls with `dev_err_probe`,
> > consistent usage of the `BIT` and `GENMASK` macros, leveraging devres
> > helpers for reset management, and improving code readability by marking
> > static data as `const`.
> 
> I am planning to review and test this series later today.

Thanks for this series! Regarding the cleanups, rhe driver is indeed in
a better shape afterwards. Good work. Patch 9 still needs discussion but
for patches 1-8:

Tested-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

On a RZ/G3S, doing bus scans and some transfers with checksumming.
Claudiu Beznea Dec. 21, 2024, 9:42 a.m. UTC | #4
Hi, Prabhakar,

On 18.12.2024 02:16, Prabhakar wrote:
> Lad Prabhakar (9):
>   i2c: riic: Replace dev_err with dev_err_probe in probe function
>   i2c: riic: Use local `dev` pointer in `dev_err_probe()`
>   i2c: riic: Use BIT macro consistently
>   i2c: riic: Use GENMASK() macro for bitmask definitions
>   i2c: riic: Make use of devres helper to request deasserted reset line
>   i2c: riic: Mark riic_irqs array as const
>   i2c: riic: Use predefined macro and simplify clock tick calculation
>   i2c: riic: Add `riic_bus_barrier()` to check bus availability
>   i2c: riic: Implement bus recovery

I've tested this series on RZ/G3S checking the already enabled
functionalities and suspend to RAM. All good. Please add:

Tested-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
Reviewed-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

Thank you,
Claudiu