Re: [PATCH 40/40] drm/amdgpu: Correctly disable the I2C IP block

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jun 14, 2021 at 1:47 PM Luben Tuikov <luben.tuikov@xxxxxxx> wrote:
>
> On long transfers to the EEPROM device,
> i.e. write, it is observed that the driver aborts
> the transfer.
>
> The reason for this is that the driver isn't
> patient enough--the IC_STATUS register's contents
> is 0x27, which is MST_ACTIVITY | TFE | TFNF |
> ACTIVITY. That is, while the transmission FIFO is
> empty, we, the I2C master device, are still
> driving the bus.
>
> Implement the correct procedure to disable
> the block, as described in the DesignWare I2C
> Databook, section 3.8.3 Disabling DW_apb_i2c on
> page 56. Now there are no premature aborts on long
> data transfers.
>
> Cc: Alexander Deucher <Alexander.Deucher@xxxxxxx>
> Cc: Andrey Grodzovsky <Andrey.Grodzovsky@xxxxxxx>
> Signed-off-by: Luben Tuikov <luben.tuikov@xxxxxxx>

Acked-by: Alex Deucher <alexander.deucher@xxxxxxx>

> ---
>  drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c | 80 +++++++++++++++++-----
>  1 file changed, 62 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c b/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c
> index 751ea2517c4380..7d74d6204d8d0a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c
> +++ b/drivers/gpu/drm/amd/amdgpu/smu_v11_0_i2c.c
> @@ -54,12 +54,48 @@ static void smu_v11_0_i2c_set_clock_gating(struct i2c_adapter *control, bool en)
>         WREG32_SOC15(SMUIO, 0, mmSMUIO_PWRMGT, reg);
>  }
>
> +/* The T_I2C_POLL_US is defined as follows:
> + *
> + * "Define a timer interval (t_i2c_poll) equal to 10 times the
> + *  signalling period for the highest I2C transfer speed used in the
> + *  system and supported by DW_apb_i2c. For instance, if the highest
> + *  I2C data transfer mode is 400 kb/s, then t_i2c_poll is 25 us."  --
> + * DesignWare DW_apb_i2c Databook, Version 1.21a, section 3.8.3.1,
> + * page 56, with grammar and syntax corrections.
> + *
> + * Vcc for our device is at 1.8V which puts it at 400 kHz,
> + * see Atmel AT24CM02 datasheet, section 8.3 DC Characteristics table, page 14.
> + *
> + * The procedure to disable the IP block is described in section
> + * 3.8.3 Disabling DW_apb_i2c on page 56.
> + */
> +#define I2C_SPEED_MODE_FAST     2
> +#define T_I2C_POLL_US           25
> +#define I2C_MAX_T_POLL_COUNT    1000
>
> -static void smu_v11_0_i2c_enable(struct i2c_adapter *control, bool enable)
> +static int smu_v11_0_i2c_enable(struct i2c_adapter *control, bool enable)
>  {
>         struct amdgpu_device *adev = to_amdgpu_device(control);
>
>         WREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_ENABLE, enable ? 1 : 0);
> +
> +       if (!enable) {
> +               int ii;
> +
> +               for (ii = I2C_MAX_T_POLL_COUNT; ii > 0; ii--) {
> +                       u32 en_stat = RREG32_SOC15(SMUIO,
> +                                                  0,
> +                                                  mmCKSVII2C_IC_ENABLE_STATUS);
> +                       if (REG_GET_FIELD(en_stat, CKSVII2C_IC_ENABLE_STATUS, IC_EN))
> +                               udelay(T_I2C_POLL_US);
> +                       else
> +                               return I2C_OK;
> +               }
> +
> +               return I2C_ABORT;
> +       }
> +
> +       return I2C_OK;
>  }
>
>  static void smu_v11_0_i2c_clear_status(struct i2c_adapter *control)
> @@ -81,8 +117,13 @@ static void smu_v11_0_i2c_configure(struct i2c_adapter *control)
>         reg = REG_SET_FIELD(reg, CKSVII2C_IC_CON, IC_RESTART_EN, 1);
>         reg = REG_SET_FIELD(reg, CKSVII2C_IC_CON, IC_10BITADDR_MASTER, 0);
>         reg = REG_SET_FIELD(reg, CKSVII2C_IC_CON, IC_10BITADDR_SLAVE, 0);
> -       /* Standard mode */
> -       reg = REG_SET_FIELD(reg, CKSVII2C_IC_CON, IC_MAX_SPEED_MODE, 2);
> +       /* The values of IC_MAX_SPEED_MODE are,
> +        * 1: standard mode, 0 - 100 Kb/s,
> +        * 2: fast mode, <= 400 Kb/s, or fast mode plus, <= 1000 Kb/s,
> +        * 3: high speed mode, <= 3.4 Mb/s.
> +        */
> +       reg = REG_SET_FIELD(reg, CKSVII2C_IC_CON, IC_MAX_SPEED_MODE,
> +                           I2C_SPEED_MODE_FAST);
>         reg = REG_SET_FIELD(reg, CKSVII2C_IC_CON, IC_MASTER_MODE, 1);
>
>         WREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_CON, reg);
> @@ -404,7 +445,6 @@ static void smu_v11_0_i2c_abort(struct i2c_adapter *control)
>         DRM_DEBUG_DRIVER("I2C_Abort() Done.");
>  }
>
> -
>  static bool smu_v11_0_i2c_activity_done(struct i2c_adapter *control)
>  {
>         struct amdgpu_device *adev = to_amdgpu_device(control);
> @@ -416,7 +456,6 @@ static bool smu_v11_0_i2c_activity_done(struct i2c_adapter *control)
>         reg_ic_enable_status = RREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_ENABLE_STATUS);
>         reg_ic_enable = RREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_ENABLE);
>
> -
>         if ((REG_GET_FIELD(reg_ic_enable, CKSVII2C_IC_ENABLE, ENABLE) == 0) &&
>             (REG_GET_FIELD(reg_ic_enable_status, CKSVII2C_IC_ENABLE_STATUS, IC_EN) == 1)) {
>                 /*
> @@ -446,6 +485,8 @@ static bool smu_v11_0_i2c_activity_done(struct i2c_adapter *control)
>
>  static void smu_v11_0_i2c_init(struct i2c_adapter *control)
>  {
> +       int res;
> +
>         /* Disable clock gating */
>         smu_v11_0_i2c_set_clock_gating(control, false);
>
> @@ -453,7 +494,9 @@ static void smu_v11_0_i2c_init(struct i2c_adapter *control)
>                 DRM_WARN("I2C busy !");
>
>         /* Disable I2C */
> -       smu_v11_0_i2c_enable(control, false);
> +       res = smu_v11_0_i2c_enable(control, false);
> +       if (res != I2C_OK)
> +               smu_v11_0_i2c_abort(control);
>
>         /* Configure I2C to operate as master and in standard mode */
>         smu_v11_0_i2c_configure(control);
> @@ -466,21 +509,22 @@ static void smu_v11_0_i2c_init(struct i2c_adapter *control)
>  static void smu_v11_0_i2c_fini(struct i2c_adapter *control)
>  {
>         struct amdgpu_device *adev = to_amdgpu_device(control);
> -       uint32_t reg_ic_enable_status, reg_ic_enable;
> +       u32 status, enable, en_stat;
> +       int res;
>
> -       smu_v11_0_i2c_enable(control, false);
> +       res = smu_v11_0_i2c_enable(control, false);
> +       if (res != I2C_OK) {
> +               status  = RREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_STATUS);
> +               enable  = RREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_ENABLE);
> +               en_stat = RREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_ENABLE_STATUS);
>
> -       /* Double check if disabled, else force abort */
> -       reg_ic_enable_status = RREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_ENABLE_STATUS);
> -       reg_ic_enable = RREG32_SOC15(SMUIO, 0, mmCKSVII2C_IC_ENABLE);
> -
> -       if ((REG_GET_FIELD(reg_ic_enable, CKSVII2C_IC_ENABLE, ENABLE) == 0) &&
> -           (REG_GET_FIELD(reg_ic_enable_status,
> -                          CKSVII2C_IC_ENABLE_STATUS, IC_EN) == 1)) {
> -               /*
> -                * Nobody is using I2C engine, but engine remains active because
> -                * someone missed to send STOP
> +               /* Nobody is using the I2C engine, yet it remains
> +                * active, possibly because someone missed to send
> +                * STOP.
>                  */
> +               DRM_DEBUG_DRIVER("Aborting from fini: status:0x%08x "
> +                                "enable:0x%08x enable_stat:0x%08x",
> +                                status, enable, en_stat);
>                 smu_v11_0_i2c_abort(control);
>         }
>
> --
> 2.32.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux