| Message ID | 875xsuknvt.wl-kuninori.morimoto.gx@renesas.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/1] i2c: rcar: tidyup priv->devtype handling on rcar_i2c_probe() | expand |
Hi Morimoto-san, On Thu, Jul 25, 2024 at 8:05 AM Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> wrote: > rcar_i2c_probe() has priv->devtype operation, but handling (A) and (C) > in same place is more easy-to-understand ((A)(C) and (B) are independent). > > (A) if (priv->devtype < I2C_RCAR_GEN3) { > ... > } > > (B) ... > > (C) if (priv->devtype >= I2C_RCAR_GEN3) { > ... > } > > Let's merge it with if-else > > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> Thanks for your patch! > --- a/drivers/i2c/busses/i2c-rcar.c > +++ b/drivers/i2c/busses/i2c-rcar.c > @@ -1168,7 +1163,10 @@ static int rcar_i2c_probe(struct platform_device *pdev) > if (of_property_read_bool(dev->of_node, "smbus")) > priv->flags |= ID_P_HOST_NOTIFY; > > - if (priv->devtype >= I2C_RCAR_GEN3) { > + if (priv->devtype < I2C_RCAR_GEN3) { > + irqflags |= IRQF_NO_THREAD; > + irqhandler = rcar_i2c_gen2_irq; > + } else { > priv->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL); > if (IS_ERR(priv->rstc)) { > ret = PTR_ERR(priv->rstc); This hunk won't apply as-is, as a comment was added in commit ea5ea84c9d3570dc ("i2c: rcar: ensure Gen3+ reset does not disturb local targets") in v6.10. Hence please rebase your patch. For the logic: Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
diff --git a/drivers/i2c/busses/i2c-rcar.c b/drivers/i2c/busses/i2c-rcar.c index 828aa2ea0fe4..66aca5c239d5 100644 --- a/drivers/i2c/busses/i2c-rcar.c +++ b/drivers/i2c/busses/i2c-rcar.c @@ -1154,11 +1154,6 @@ static int rcar_i2c_probe(struct platform_device *pdev) rcar_i2c_write(priv, ICSAR, 0); /* Gen2: must be 0 if not using slave */ - if (priv->devtype < I2C_RCAR_GEN3) { - irqflags |= IRQF_NO_THREAD; - irqhandler = rcar_i2c_gen2_irq; - } - /* Stay always active when multi-master to keep arbitration working */ if (of_property_read_bool(dev->of_node, "multi-master")) priv->flags |= ID_P_PM_BLOCKED; @@ -1168,7 +1163,10 @@ static int rcar_i2c_probe(struct platform_device *pdev) if (of_property_read_bool(dev->of_node, "smbus")) priv->flags |= ID_P_HOST_NOTIFY; - if (priv->devtype >= I2C_RCAR_GEN3) { + if (priv->devtype < I2C_RCAR_GEN3) { + irqflags |= IRQF_NO_THREAD; + irqhandler = rcar_i2c_gen2_irq; + } else { priv->rstc = devm_reset_control_get_exclusive(&pdev->dev, NULL); if (IS_ERR(priv->rstc)) { ret = PTR_ERR(priv->rstc);
rcar_i2c_probe() has priv->devtype operation, but handling (A) and (C) in same place is more easy-to-understand ((A)(C) and (B) are independent). (A) if (priv->devtype < I2C_RCAR_GEN3) { ... } (B) ... (C) if (priv->devtype >= I2C_RCAR_GEN3) { ... } Let's merge it with if-else Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com> --- drivers/i2c/busses/i2c-rcar.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)