diff mbox series

[5/9] i2c: riic: Make use of devres helper to request deasserted reset line

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

Commit Message

Prabhakar Dec. 13, 2024, 5:58 p.m. UTC 
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>
---
 drivers/i2c/busses/i2c-riic.c | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

Comments

Geert Uytterhoeven Dec. 16, 2024, 3:56 p.m. UTC | #1 
Hi Prabhakar,

On Fri, Dec 13, 2024 at 6:58 PM 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>

Thanks for your patch!

> --- a/drivers/i2c/busses/i2c-riic.c
> +++ b/drivers/i2c/busses/i2c-riic.c
> @@ -447,18 +442,10 @@ static int riic_i2c_probe(struct platform_device *pdev)
>                 return dev_err_probe(dev, PTR_ERR(riic->clk),
>                                      "missing controller clock");
>
> -       riic->rstc = devm_reset_control_get_optional_exclusive(dev, NULL);
> +       riic->rstc = devm_reset_control_get_optional_exclusive_deasserted(dev, NULL);
>         if (IS_ERR(riic->rstc))
>                 return dev_err_probe(dev, PTR_ERR(riic->rstc),
> -                                    "Error: missing reset ctrl\n");
> -
> -       ret = reset_control_deassert(riic->rstc);
> -       if (ret)
> -               return ret;
> -
> -       ret = devm_add_action_or_reset(dev, riic_reset_control_assert, riic->rstc);
> -       if (ret)
> -               return ret;
> +                                    "Failed to acquire deasserted reset control\n");

"failed", as all other error messages start with a lower-case character.
Perhaps drop the " control" from the message?

>
>         for (i = 0; i < ARRAY_SIZE(riic_irqs); i++) {
>                 ret = platform_get_irq(pdev, riic_irqs[i].res_num);

Gr{oetje,eeting}s,

                        Geert
Prabhakar Dec. 16, 2024, 6:17 p.m. UTC | #2 
Hi Geert,

Thank you for the review.

On Mon, Dec 16, 2024 at 3:56 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Fri, Dec 13, 2024 at 6:58 PM 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>
>
> Thanks for your patch!
>
> > --- a/drivers/i2c/busses/i2c-riic.c
> > +++ b/drivers/i2c/busses/i2c-riic.c
> > @@ -447,18 +442,10 @@ static int riic_i2c_probe(struct platform_device *pdev)
> >                 return dev_err_probe(dev, PTR_ERR(riic->clk),
> >                                      "missing controller clock");
> >
> > -       riic->rstc = devm_reset_control_get_optional_exclusive(dev, NULL);
> > +       riic->rstc = devm_reset_control_get_optional_exclusive_deasserted(dev, NULL);
> >         if (IS_ERR(riic->rstc))
> >                 return dev_err_probe(dev, PTR_ERR(riic->rstc),
> > -                                    "Error: missing reset ctrl\n");
> > -
> > -       ret = reset_control_deassert(riic->rstc);
> > -       if (ret)
> > -               return ret;
> > -
> > -       ret = devm_add_action_or_reset(dev, riic_reset_control_assert, riic->rstc);
> > -       if (ret)
> > -               return ret;
> > +                                    "Failed to acquire deasserted reset control\n");
>
> "failed", as all other error messages start with a lower-case character.
> Perhaps drop the " control" from the message?
>
Sure, I will update the error message using `failed` and drop the `control`.

Cheers,
Prabhakar
diff mbox series

Patch

diff --git a/drivers/i2c/busses/i2c-riic.c b/drivers/i2c/busses/i2c-riic.c
index 487fa26264b0..032cf0b1368e 100644
--- a/drivers/i2c/busses/i2c-riic.c
+++ b/drivers/i2c/busses/i2c-riic.c
@@ -422,11 +422,6 @@  static struct riic_irq_desc riic_irqs[] = {
 	{ .res_num = 5, .isr = riic_tend_isr, .name = "riic-nack" },
 };
 
-static void riic_reset_control_assert(void *data)
-{
-	reset_control_assert(data);
-}
-
 static int riic_i2c_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -447,18 +442,10 @@  static int riic_i2c_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, PTR_ERR(riic->clk),
 				     "missing controller clock");
 
-	riic->rstc = devm_reset_control_get_optional_exclusive(dev, NULL);
+	riic->rstc = devm_reset_control_get_optional_exclusive_deasserted(dev, NULL);
 	if (IS_ERR(riic->rstc))
 		return dev_err_probe(dev, PTR_ERR(riic->rstc),
-				     "Error: missing reset ctrl\n");
-
-	ret = reset_control_deassert(riic->rstc);
-	if (ret)
-		return ret;
-
-	ret = devm_add_action_or_reset(dev, riic_reset_control_assert, riic->rstc);
-	if (ret)
-		return ret;
+				     "Failed to acquire deasserted reset control\n");
 
 	for (i = 0; i < ARRAY_SIZE(riic_irqs); i++) {
 		ret = platform_get_irq(pdev, riic_irqs[i].res_num);