[PATCH 20/40] drm/amdgpu: EEPROM respects I2C quirks

[Alex Deucher]

On Tue, Jun 8, 2021 at 5:40 PM Luben Tuikov <luben.tuikov at amd.com> wrote:
>
> Consult the i2c_adapter.quirks table for
> the maximum read/write data length per bus
> transaction. Do not exceed this transaction
> limit.
>
> Cc: Jean Delvare <jdelvare at suse.de>
> Cc: Alexander Deucher <Alexander.Deucher at amd.com>
> Cc: Andrey Grodzovsky <Andrey.Grodzovsky at amd.com>
> Cc: Lijo Lazar <Lijo.Lazar at amd.com>
> Cc: Stanley Yang <Stanley.Yang at amd.com>
> Cc: Hawking Zhang <Hawking.Zhang at amd.com>
> Signed-off-by: Luben Tuikov <luben.tuikov at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c | 80 +++++++++++++++++-----
>  1 file changed, 64 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
> index 7fdb5bd2fc8bc8..94aeda1c7f8ca0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
> @@ -32,20 +32,9 @@
>
>  #define EEPROM_OFFSET_SIZE 2
>
> -/**
> - * amdgpu_eeprom_xfer -- Read/write from/to an I2C EEPROM device
> - * @i2c_adap: pointer to the I2C adapter to use
> - * @slave_addr: I2C address of the slave device
> - * @eeprom_addr: EEPROM address from which to read/write
> - * @eeprom_buf: pointer to data buffer to read into/write from
> - * @buf_size: the size of @eeprom_buf
> - * @read: True if reading from the EEPROM, false if writing
> - *
> - * Returns the number of bytes read/written; -errno on error.
> - */
> -int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap,
> -                      u16 slave_addr, u16 eeprom_addr,
> -                      u8 *eeprom_buf, u16 buf_size, bool read)
> +static int __amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap,
> +                               u16 slave_addr, u16 eeprom_addr,
> +                               u8 *eeprom_buf, u16 buf_size, bool read)
>  {
>         u8 eeprom_offset_buf[EEPROM_OFFSET_SIZE];
>         struct i2c_msg msgs[] = {
> @@ -65,8 +54,8 @@ int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap,
>         u16 len;
>
>         r = 0;
> -       for (len = 0; buf_size > 0;
> -            buf_size -= len, eeprom_addr += len, eeprom_buf += len) {
> +       for ( ; buf_size > 0;
> +             buf_size -= len, eeprom_addr += len, eeprom_buf += len) {
>                 /* Set the EEPROM address we want to write to/read from.
>                  */
>                 msgs[0].buf[0] = (eeprom_addr >> 8) & 0xff;
> @@ -120,3 +109,62 @@ int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap,
>
>         return r < 0 ? r : eeprom_buf - p;
>  }
> +
> +/**
> + * amdgpu_eeprom_xfer -- Read/write from/to an I2C EEPROM device
> + * @i2c_adap: pointer to the I2C adapter to use
> + * @slave_addr: I2C address of the slave device
> + * @eeprom_addr: EEPROM address from which to read/write
> + * @eeprom_buf: pointer to data buffer to read into/write from
> + * @buf_size: the size of @eeprom_buf
> + * @read: True if reading from the EEPROM, false if writing
> + *
> + * Returns the number of bytes read/written; -errno on error.
> + */
> +int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap,
> +                      u16 slave_addr, u16 eeprom_addr,
> +                      u8 *eeprom_buf, u16 buf_size, bool read)
> +{
> +       const struct i2c_adapter_quirks *quirks = i2c_adap->quirks;
> +       u16 limit;
> +
> +       if (!quirks)
> +               limit = 0;
> +       else if (read)
> +               limit = quirks->max_read_len;
> +       else
> +               limit = quirks->max_write_len;
> +
> +       if (limit == 0) {
> +               return __amdgpu_eeprom_xfer(i2c_adap, slave_addr, eeprom_addr,
> +                                           eeprom_buf, buf_size, read);
> +       } else if (limit <= EEPROM_OFFSET_SIZE) {
> +               dev_err_ratelimited(&i2c_adap->dev,
> +                                   "maddr:0x%04X size:0x%02X:quirk max_%s_len must be > %d",
> +                                   eeprom_addr, buf_size,
> +                                   read ? "read" : "write", EEPROM_OFFSET_SIZE);
> +               return -EINVAL;

I presume we handle this case properly at higher levels (i.e., split
up EEPROM updates into smaller transactions)?

Alex


> +       } else {
> +               u16 ps; /* Partial size */
> +               int res = 0, r;
> +
> +               /* The "limit" includes all data bytes sent/received,
> +                * which would include the EEPROM_OFFSET_SIZE bytes.
> +                * Account for them here.
> +                */
> +               limit -= EEPROM_OFFSET_SIZE;
> +               for ( ; buf_size > 0;
> +                     buf_size -= ps, eeprom_addr += ps, eeprom_buf += ps) {
> +                       ps = min(limit, buf_size);
> +
> +                       r = __amdgpu_eeprom_xfer(i2c_adap,
> +                                                slave_addr, eeprom_addr,
> +                                                eeprom_buf, ps, read);
> +                       if (r < 0)
> +                               return r;
> +                       res += r;
> +               }
> +
> +               return res;
> +       }
> +}
> --
> 2.32.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx