Hi Wolfram,

On Thu, Jun 28, 2018 at 10:45 PM Wolfram Sang
<wsa+rene...@sang-engineering.com> wrote:
> On Gen3, we can only do RXDMA once per transfer reliably. For that, we
> must reset the device, then we can have RXDMA once. This patch
> implements this. When there is no reset controller or the reset fails,
> RXDMA will be blocked completely. Otherwise, it will be disabled after
> the first RXDMA transfer. Based on a commit from the BSP by Hiromitsu
> Yamasaki, yet completely refactored to handle multiple read messages
> within one transfer.
>
> Signed-off-by: Wolfram Sang <wsa+rene...@sang-engineering.com>

Thanks for your patch!

> --- a/drivers/i2c/busses/i2c-rcar.c
> +++ b/drivers/i2c/busses/i2c-rcar.c

> @@ -751,6 +780,16 @@ static int rcar_i2c_master_xfer(struct i2c_adapter *adap,
>
>         pm_runtime_get_sync(dev);
>
> +       /* Gen3 needs a reset before allowing RXDMA once */
> +       if (priv->devtype == I2C_RCAR_GEN3) {

So on R-Car Gen3 the device is always reset, even if no reads will be done?
Or is that something you cannot check for at this point?

> +               priv->flags |= ID_P_NO_RXDMA;
> +               if (!IS_ERR(priv->rstc)) {
> +                       ret = rcar_i2c_do_reset(priv);
> +                       if (ret == 0)
> +                               priv->flags &= ~ID_P_NO_RXDMA;
> +               }
> +       }
> +
>         rcar_i2c_init(priv);
>
>         ret = rcar_i2c_bus_barrier(priv);
> @@ -921,6 +960,15 @@ static int rcar_i2c_probe(struct platform_device *pdev)
>         if (ret < 0)
>                 goto out_pm_put;
>
> +       if (priv->devtype == I2C_RCAR_GEN3) {
> +               priv->rstc = devm_reset_control_get_exclusive(&pdev->dev, 
> NULL);
> +               if (!IS_ERR(priv->rstc)) {
> +                       ret = reset_control_status(priv->rstc);

Why this call and check? To check if .status() is implemented (it always is
on R-Car Gen3, if CONFIG_RESET_CONTROLLER is enabled), and to avoid the
timeout in rcar_i2c_do_reset() on every transfer?

> +                       if (ret < 0)
> +                               priv->rstc = ERR_PTR(-ENOTSUPP);
> +               }
> +       }

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

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Reply via email to