Re: [PATCH 21/40] drm/amdgpu: I2C EEPROM full memory addressing

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

 



On Tue, Jun 8, 2021 at 5:40 PM Luben Tuikov <luben.tuikov@xxxxxxx> wrote:
>
> * "eeprom_addr" is now 32-bit wide.
> * Remove "slave_addr" from the I2C EEPROM driver
>   interface. The I2C EEPROM Device Type Identifier
>   is fixed at 1010b, and the rest of the bits
>   of the Device Address Byte/Device Select Code,
>   are memory address bits, where the first three
>   of those bits are the hardware selection bits.
>   All this is now a 19-bit address and passed
>   as "eeprom_addr". This abstracts the I2C bus
>   for EEPROM devices for this I2C EEPROM driver.
>   Now clients only pass the 19-bit EEPROM memory
>   address, to the I2C EEPROM driver, as the 32-bit
>   "eeprom_addr", from which they want to read from
>   or write to.
>
> Cc: Jean Delvare <jdelvare@xxxxxxx>
> Cc: Alexander Deucher <Alexander.Deucher@xxxxxxx>
> Cc: Andrey Grodzovsky <Andrey.Grodzovsky@xxxxxxx>
> Cc: Lijo Lazar <Lijo.Lazar@xxxxxxx>
> Cc: Stanley Yang <Stanley.Yang@xxxxxxx>
> Cc: Hawking Zhang <Hawking.Zhang@xxxxxxx>
> Signed-off-by: Luben Tuikov <luben.tuikov@xxxxxxx>

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

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c | 88 +++++++++++++++++-----
>  drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h |  4 +-
>  2 files changed, 72 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
> index 94aeda1c7f8ca0..a5a87affedabf1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.c
> @@ -24,7 +24,7 @@
>  #include "amdgpu_eeprom.h"
>  #include "amdgpu.h"
>
> -/* AT24CM02 has a 256-byte write page size.
> +/* AT24CM02 and M24M02-R have a 256-byte write page size.
>   */
>  #define EEPROM_PAGE_BITS   8
>  #define EEPROM_PAGE_SIZE   (1U << EEPROM_PAGE_BITS)
> @@ -32,20 +32,72 @@
>
>  #define EEPROM_OFFSET_SIZE 2
>
> -static int __amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap,
> -                               u16 slave_addr, u16 eeprom_addr,
> +/* EEPROM memory addresses are 19-bits long, which can
> + * be partitioned into 3, 8, 8 bits, for a total of 19.
> + * The upper 3 bits are sent as part of the 7-bit
> + * "Device Type Identifier"--an I2C concept, which for EEPROM devices
> + * is hard-coded as 1010b, indicating that it is an EEPROM
> + * device--this is the wire format, followed by the upper
> + * 3 bits of the 19-bit address, followed by the direction,
> + * followed by two bytes holding the rest of the 16-bits of
> + * the EEPROM memory address. The format on the wire for EEPROM
> + * devices is: 1010XYZD, A15:A8, A7:A0,
> + * Where D is the direction and sequenced out by the hardware.
> + * Bits XYZ are memory address bits 18, 17 and 16.
> + * These bits are compared to how pins 1-3 of the part are connected,
> + * depending on the size of the part, more on that later.
> + *
> + * Note that of this wire format, a client is in control
> + * of, and needs to specify only XYZ, A15:A8, A7:0, bits,
> + * which is exactly the EEPROM memory address, or offset,
> + * in order to address up to 8 EEPROM devices on the I2C bus.
> + *
> + * For instance, a 2-Mbit I2C EEPROM part, addresses all its bytes,
> + * using an 18-bit address, bit 17 to 0 and thus would use all but one bit of
> + * the 19 bits previously mentioned. The designer would then not connect
> + * pins 1 and 2, and pin 3 usually named "A_2" or "E2", would be connected to
> + * either Vcc or GND. This would allow for up to two 2-Mbit parts on
> + * the same bus, where one would be addressable with bit 18 as 1, and
> + * the other with bit 18 of the address as 0.
> + *
> + * For a 2-Mbit part, bit 18 is usually known as the "Chip Enable" or
> + * "Hardware Address Bit". This bit is compared to the load on pin 3
> + * of the device, described above, and if there is a match, then this
> + * device responds to the command. This way, you can connect two
> + * 2-Mbit EEPROM devices on the same bus, but see one contiguous
> + * memory from 0 to 7FFFFh, where address 0 to 3FFFF is in the device
> + * whose pin 3 is connected to GND, and address 40000 to 7FFFFh is in
> + * the 2nd device, whose pin 3 is connected to Vcc.
> + *
> + * This addressing you encode in the 32-bit "eeprom_addr" below,
> + * namely the 19-bits "XYZ,A15:A0", as a single 19-bit address. For
> + * instance, eeprom_addr = 0x6DA01, is 110_1101_1010_0000_0001, where
> + * XYZ=110b, and A15:A0=DA01h. The XYZ bits become part of the device
> + * address, and the rest of the address bits are sent as the memory
> + * address bytes.
> + *
> + * That is, for an I2C EEPROM driver everything is controlled by
> + * the "eeprom_addr".
> + *
> + * P.S. If you need to write, lock and read the Identification Page,
> + * (M24M02-DR device only, which we do not use), change the "7" to
> + * "0xF" in the macro below, and let the client set bit 20 to 1 in
> + * "eeprom_addr", and set A10 to 0 to write into it, and A10 and A1 to
> + * 1 to lock it permanently.
> + */
> +#define MAKE_I2C_ADDR(_aa) ((0xA << 3) | (((_aa) >> 16) & 7))
> +
> +static int __amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, u32 eeprom_addr,
>                                 u8 *eeprom_buf, u16 buf_size, bool read)
>  {
>         u8 eeprom_offset_buf[EEPROM_OFFSET_SIZE];
>         struct i2c_msg msgs[] = {
>                 {
> -                       .addr = slave_addr,
>                         .flags = 0,
>                         .len = EEPROM_OFFSET_SIZE,
>                         .buf = eeprom_offset_buf,
>                 },
>                 {
> -                       .addr = slave_addr,
>                         .flags = read ? I2C_M_RD : 0,
>                 },
>         };
> @@ -58,6 +110,8 @@ static int __amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap,
>               buf_size -= len, eeprom_addr += len, eeprom_buf += len) {
>                 /* Set the EEPROM address we want to write to/read from.
>                  */
> +               msgs[0].addr = MAKE_I2C_ADDR(eeprom_addr);
> +               msgs[1].addr = msgs[0].addr;
>                 msgs[0].buf[0] = (eeprom_addr >> 8) & 0xff;
>                 msgs[0].buf[1] = eeprom_addr & 0xff;
>
> @@ -71,7 +125,7 @@ static int __amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap,
>                          * over).
>                          *
>                          * As per the AT24CM02 EEPROM spec, after
> -                        * writing into a page, the I2C driver MUST
> +                        * writing into a page, the I2C driver should
>                          * terminate the transfer, i.e. in
>                          * "i2c_transfer()" below, with a STOP
>                          * condition, so that the self-timed write
> @@ -91,17 +145,20 @@ static int __amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap,
>                 msgs[1].len = len;
>                 msgs[1].buf = eeprom_buf;
>
> +               /* This constitutes a START-STOP transaction.
> +                */
>                 r = i2c_transfer(i2c_adap, msgs, ARRAY_SIZE(msgs));
>                 if (r < ARRAY_SIZE(msgs))
>                         break;
>
>                 if (!read) {
> -                       /* According to the AT24CM02 EEPROM spec the
> -                        * length of the self-writing cycle, tWR, is
> -                        * 10 ms.
> +                       /* According to EEPROM specs the length of the
> +                        * self-writing cycle, tWR (tW), is 10 ms.
>                          *
> -                        * TODO Improve to wait for first ACK for slave address after
> -                        * internal write cycle done.
> +                        * TODO: Use polling on ACK, aka Acknowledge
> +                        * Polling, to minimize waiting for the
> +                        * internal write cycle to complete, as it is
> +                        * usually smaller than tWR (tW).
>                          */
>                         msleep(10);
>                 }
> @@ -113,7 +170,6 @@ static int __amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap,
>  /**
>   * 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
> @@ -121,8 +177,7 @@ static int __amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap,
>   *
>   * 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,
> +int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, u32 eeprom_addr,
>                        u8 *eeprom_buf, u16 buf_size, bool read)
>  {
>         const struct i2c_adapter_quirks *quirks = i2c_adap->quirks;
> @@ -136,7 +191,7 @@ int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap,
>                 limit = quirks->max_write_len;
>
>         if (limit == 0) {
> -               return __amdgpu_eeprom_xfer(i2c_adap, slave_addr, eeprom_addr,
> +               return __amdgpu_eeprom_xfer(i2c_adap, eeprom_addr,
>                                             eeprom_buf, buf_size, read);
>         } else if (limit <= EEPROM_OFFSET_SIZE) {
>                 dev_err_ratelimited(&i2c_adap->dev,
> @@ -157,8 +212,7 @@ int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap,
>                       buf_size -= ps, eeprom_addr += ps, eeprom_buf += ps) {
>                         ps = min(limit, buf_size);
>
> -                       r = __amdgpu_eeprom_xfer(i2c_adap,
> -                                                slave_addr, eeprom_addr,
> +                       r = __amdgpu_eeprom_xfer(i2c_adap, eeprom_addr,
>                                                  eeprom_buf, ps, read);
>                         if (r < 0)
>                                 return r;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h
> index 9301e5678910ad..417472be2712e6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_eeprom.h
> @@ -26,9 +26,7 @@
>
>  #include <linux/i2c.h>
>
> -int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap,
> -                      u16 slave_addr, u16 eeprom_addr,
> +int amdgpu_eeprom_xfer(struct i2c_adapter *i2c_adap, u32 eeprom_addr,
>                        u8 *eeprom_buf, u16 bytes, bool read);
>
> -
>  #endif
> --
> 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