[U-Boot] [PATCH] i2c: intel_i2c: SMBus driver PCI addition (e.g. BayTrail)

Stefan Roese sr at denx.de
Tue Aug 9 07:32:56 CEST 2016


Hi George,

On 05.08.2016 18:09, George McCollister wrote:
> On Tue, Jun 28, 2016 at 8:44 AM, Stefan Roese <sr at denx.de> wrote:
>> This patch adds support for the SMBus block read/write functionality.
>> Other protocols like the SMBus quick command need to get added
>> if this is needed.
>>
>> This patch also removed the SMBus related defines from the Ivybridge
>> pch.h header. As they are integrated in this driver and should be
>> used from here. This change is added in this patch to avoid compile
>> breakage to keep the source git bisectable.
>>
>> Tested on a congatec BayTrail board to configure the SMSC2513 USB
>> hub.
>>
>> Signed-off-by: Stefan Roese <sr at denx.de>
>> Cc: Bin Meng <bmeng.cn at gmail.com>
>> Cc: Simon Glass <sjg at chromium.org>
>> Cc: Heiko Schocher <hs at denx.de>
>> ---
>> Simon, I'm not sure if this change breaks your Ivybridge targets
>> using the probe part of this driver. Could you please let me
>> know if this works? Or let me know what needs changes here?
>>
>> Thanks,
>> Stefan
>>
>>  arch/x86/include/asm/arch-ivybridge/pch.h |  26 ---
>>  drivers/i2c/intel_i2c.c                   | 290 ++++++++++++++++++++++++++++--
>>  2 files changed, 278 insertions(+), 38 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/arch-ivybridge/pch.h b/arch/x86/include/asm/arch-ivybridge/pch.h
>> index 4725250..9c51f63 100644
>> --- a/arch/x86/include/asm/arch-ivybridge/pch.h
>> +++ b/arch/x86/include/asm/arch-ivybridge/pch.h
>> @@ -134,32 +134,6 @@
>>  #define SATA_IOBP_SP0G3IR      0xea000151
>>  #define SATA_IOBP_SP1G3IR      0xea000051
>>
>> -/* PCI Configuration Space (D31:F3): SMBus */
>> -#define PCH_SMBUS_DEV          PCI_BDF(0, 0x1f, 3)
>> -#define SMB_BASE               0x20
>> -#define HOSTC                  0x40
>> -#define SMB_RCV_SLVA           0x09
>> -
>> -/* HOSTC bits */
>> -#define I2C_EN                 (1 << 2)
>> -#define SMB_SMI_EN             (1 << 1)
>> -#define HST_EN                 (1 << 0)
>> -
>> -/* SMBus I/O bits. */
>> -#define SMBHSTSTAT             0x0
>> -#define SMBHSTCTL              0x2
>> -#define SMBHSTCMD              0x3
>> -#define SMBXMITADD             0x4
>> -#define SMBHSTDAT0             0x5
>> -#define SMBHSTDAT1             0x6
>> -#define SMBBLKDAT              0x7
>> -#define SMBTRNSADD             0x9
>> -#define SMBSLVDATA             0xa
>> -#define SMLINK_PIN_CTL         0xe
>> -#define SMBUS_PIN_CTL          0xf
>> -
>> -#define SMBUS_TIMEOUT          (10 * 1000 * 100)
>> -
>>  #define VCH            0x0000  /* 32bit */
>>  #define VCAP1          0x0004  /* 32bit */
>>  #define VCAP2          0x0008  /* 32bit */
>> diff --git a/drivers/i2c/intel_i2c.c b/drivers/i2c/intel_i2c.c
>> index 3d777ff..8b63916 100644
>> --- a/drivers/i2c/intel_i2c.c
>> +++ b/drivers/i2c/intel_i2c.c
>> @@ -2,45 +2,263 @@
>>   * Copyright (c) 2015 Google, Inc
>>   * Written by Simon Glass <sjg at chromium.org>
>>   *
>> + * SMBus block read/write support added by Stefan Roese:
>> + * Copyright (C) 2016 Stefan Roese <sr at denx.de>
>> + *
>>   * SPDX-License-Identifier:     GPL-2.0+
>>   */
>>
>>  #include <common.h>
>>  #include <dm.h>
>>  #include <i2c.h>
>> +#include <pci.h>
>>  #include <asm/io.h>
>> +#ifdef CONFIG_NORTHBRIDGE_INTEL_IVYBRIDGE
>>  #include <asm/arch/pch.h>
>> +#endif
>> +
>> +/* PCI Configuration Space (D31:F3): SMBus */
>> +#define SMB_BASE               0x20
>> +#define HOSTC                  0x40
>> +#define  HST_EN                        (1 << 0)
>> +#define SMB_RCV_SLVA           0x09
>> +
>> +/* SMBus I/O bits. */
>> +#define SMBHSTSTAT             0x0
>> +#define SMBHSTCTL              0x2
>> +#define SMBHSTCMD              0x3
>> +#define SMBXMITADD             0x4
>> +#define SMBHSTDAT0             0x5
>> +#define SMBHSTDAT1             0x6
>> +#define SMBBLKDAT              0x7
>> +#define SMBTRNSADD             0x9
>> +#define SMBSLVDATA             0xa
>> +#define SMBAUXCTL              0xd
>> +#define SMLINK_PIN_CTL         0xe
>> +#define SMBUS_PIN_CTL          0xf
>> +
>> +/* I801 Hosts Status register bits */
>> +#define SMBHSTSTS_BYTE_DONE    0x80
>> +#define SMBHSTSTS_INUSE_STS    0x40
>> +#define SMBHSTSTS_SMBALERT_STS 0x20
>> +#define SMBHSTSTS_FAILED       0x10
>> +#define SMBHSTSTS_BUS_ERR      0x08
>> +#define SMBHSTSTS_DEV_ERR      0x04
>> +#define SMBHSTSTS_INTR         0x02
>> +#define SMBHSTSTS_HOST_BUSY    0x01
>> +
>> +/* I801 Host Control register bits */
>> +#define SMBHSTCNT_INTREN       0x01
>> +#define SMBHSTCNT_KILL         0x02
>> +#define SMBHSTCNT_LAST_BYTE    0x20
>> +#define SMBHSTCNT_START                0x40
>> +#define SMBHSTCNT_PEC_EN       0x80    /* ICH3 and later */
>> +
>> +/* Auxiliary control register bits, ICH4+ only */
>> +#define SMBAUXCTL_CRC          1
>> +#define SMBAUXCTL_E32B         2
>> +
>> +#define SMBUS_TIMEOUT  100     /* 100 ms */
>> +
>> +struct intel_i2c {
>> +       u32 base;
>> +       int running;
>> +};
>> +
>> +static int smbus_wait_until_ready(u32 base)
>> +{
>> +       unsigned long ts;
>> +       u8 byte;
>> +
>> +       ts = get_timer(0);
>> +       do {
>> +               byte = inb(base + SMBHSTSTAT);
>> +               if (!(byte & 1))
>> +                       return 0;
>> +       } while (get_timer(ts) < SMBUS_TIMEOUT);
>> +
>> +       return -ETIMEDOUT;
>> +}
>> +
>> +static int smbus_wait_until_done(u32 base)
>> +{
>> +       unsigned long ts;
>> +       u8 byte;
>> +
>> +       ts = get_timer(0);
>> +       do {
>> +               byte = inb(base + SMBHSTSTAT);
>> +               if (!((byte & 1) || (byte & ~((1 << 6) | (1 << 0))) == 0))
>> +                       return 0;
>> +       } while (get_timer(ts) < SMBUS_TIMEOUT);
>> +
>> +       return -ETIMEDOUT;
>> +}
>> +
>> +static int smbus_block_read(u32 base, u8 dev, u8 *buffer,
>> +                           int offset, int len)
>> +{
>> +       u8 buf_temp[32];
>> +       int count;
>> +       int i;
>> +
>> +       debug("%s (%d): dev=0x%x offs=0x%x len=0x%x\n",
>> +             __func__, __LINE__, dev, offset, len);
>> +       if (smbus_wait_until_ready(base) < 0)
>> +               return -ETIMEDOUT;
>> +
>> +       /* Setup transaction */
>> +
>> +       /* Reset the data buffer index */
>> +       inb(base + SMBHSTCTL);
>> +
>> +       /* Set the device I'm talking too */
>> +       outb(((dev & 0x7f) << 1) | 1, base + SMBXMITADD);
>> +       /* Set the command/address... */
>> +       outb(offset & 0xff, base + SMBHSTCMD);
>> +       /* Set up for a block read */
>> +       outb((inb(base + SMBHSTCTL) & (~(0x7) << 2)) | (0x5 << 2),
>> +            (base + SMBHSTCTL));
>> +       /* Clear any lingering errors, so the transaction will run */
>> +       outb(inb(base + SMBHSTSTAT), base + SMBHSTSTAT);
>> +
>> +       /* Start the command */
>> +       outb((inb(base + SMBHSTCTL) | SMBHSTCNT_START), base + SMBHSTCTL);
>> +
>> +       /* Poll for transaction completion */
>> +       if (smbus_wait_until_done(base) < 0) {
>> +               printf("SMBUS read transaction timeout (dev=0x%x)\n", dev);
>> +               return -ETIMEDOUT;
>> +       }
>> +
>> +       count = inb(base + SMBHSTDAT0);
>
> I think always doing a block read is a bit problematic. I've tested
> the driver (on Baytrail) with several SMBus devices.
>
> This example is a PCA9554 8-Bit I2C AND SMBus I/O Expander (datasheet
> http://www.ti.com/lit/ds/symlink/pca9554.pdf):
>
> i2c md 20 0 5
> i2c_xfer: 2 messages
> smbus_block_read (108): dev=0x20 offs=0x0 len=0x5
> smbus_block_read (139): count=127 (len=5)
> Invalid Opcode (Undefined Opcode)
> EIP: 0010:[<80000001>] EFLAGS: 00010a92
> Original EIP :[<049a0001>]
> EAX: 00000000 EBX: 7f7ffe7f ECX: 00000000 EDX: 0000b000
> ESI: 7f7f7f7f EDI: 7f7f7f7f EBP: 7f7f7f7f ESP: 7b34c3b0
> DS: 0018 ES: 0018 FS: 0020 GS: 0018 SS: 0018
> CR0: 00000033 CR2: 00000000 CR3: 00000000 CR4: 00000600
> DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
> DR6: ffff0ff0 DR7: 00000400
> Stack:
>    0x7b34c3f0 : 0x7b36bd60
>    0x7b34c3ec : 0x7b7f7f7f
>    0x7b34c3e8 : 0x7f7f7f7f
>    0x7b34c3e4 : 0x7f7f7f7f
>    0x7b34c3e0 : 0x7f7f7f7f
>    0x7b34c3dc : 0x7f7f7f7f
>    0x7b34c3d8 : 0x7f7f7f7f
>    0x7b34c3d4 : 0x7f7f7f7f
>    0x7b34c3d0 : 0x7f7f7f7f
>    0x7b34c3cc : 0x7f7f7f7f
>    0x7b34c3c8 : 0x7f7f7f7f
>    0x7b34c3c4 : 0x7f7f7f7f
>    0x7b34c3c0 : 0x7f7f7f7f
>    0x7b34c3bc : 0x7f7f7f7f
>    0x7b34c3b8 : 0x7f7f7f7f
>    0x7b34c3b4 : 0x7f7f7f7f
> --->0x7b34c3b0 : 0x7f7f7f7f
>    0x7b34c3ac : 0x00010a92
>    0x7b34c3a8 : 0x00000010
>    0x7b34c3a4 : 0x80000001
> ### ERROR ### Please RESET the board ###
>
> This device always returns the data instead of providing a proper
> count. In order for this driver to be usable with this part it needs
> to support a byte read. Any ideas how we could support both byte and
> block read?

The USB device I need to communicate with only supports block read
write operations. Thats why I added those functions. I would prefer to
get v2 of this SMBus support upstream first. And then work on
supporting other SMBus operations (like byte read).

Simon, do you have an idea on how to support byte and block SMBus
operations? The easy way would be to add a Kconfig option to
select the OP at compile time. But a runtime selection would be
much better.

Thanks,
Stefan


More information about the U-Boot mailing list