[U-Boot] [PATCH] i2c: intel_i2c: SMBus driver PCI addition (e.g. BayTrail)
George McCollister
george.mccollister at gmail.com
Fri Aug 5 18:09:19 CEST 2016
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?
> + debug("%s (%d): count=%d (len=%d)\n", __func__, __LINE__, count, len);
> + if (count == 0) {
> + debug("ERROR: len=0 on read\n");
> + return -EIO;
> + }
> +
> + if (count < len) {
> + debug("ERROR: too few bytes read\n");
> + return -EIO;
> + }
> +
> + /* Read all available bytes from buffer */
> + for (i = 0; i < count; i++)
> + buf_temp[i] = inb(base + SMBBLKDAT);
No overrun check, if count is > 32 buffer is over run. In my case the
data returned by the PCA9554 is interpreted as count and an overrun
occurs.
> +
> + memcpy(buffer, buf_temp, len);
> +
> + /* Return results of transaction */
> + if (!(inb(base + SMBHSTSTAT) & SMBHSTSTS_INTR))
> + return -EIO;
> +
> + return 0;
> +}
> +
> +static int smbus_block_write(u32 base, u8 dev, u8 *buffer,
> + int offset, int len)
> +{
> + int i;
>
> -int intel_i2c_xfer(struct udevice *bus, struct i2c_msg *msg, int nmsgs)
> + 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 */
> + /* Set the device I'm talking too */
> + outb(((dev & 0x7f) << 1) & ~0x01, base + SMBXMITADD);
> + /* Set the command/address... */
> + outb(offset, base + SMBHSTCMD);
> + /* Set up for a block write */
> + 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);
> +
> + /* Write count in DAT0 register */
> + outb(len, base + SMBHSTDAT0);
> +
> + /* Write data bytes... */
> + for (i = 0; i < len; i++)
> + outb(*buffer++, base + SMBBLKDAT);
> +
> + /* Start the command */
> + outb((inb(base + SMBHSTCTL) | SMBHSTCNT_START), base + SMBHSTCTL);
> +
> + /* Poll for transaction completion */
> + if (smbus_wait_until_done(base) < 0) {
> + printf("SMBUS write transaction timeout (dev=0x%x)\n", dev);
> + return -ETIMEDOUT;
> + }
> +
> + /* Return results of transaction */
> + if (!(inb(base + SMBHSTSTAT) & SMBHSTSTS_INTR))
> + return -EIO;
> +
> + return 0;
> +}
> +
> +static int intel_i2c_xfer(struct udevice *bus, struct i2c_msg *msg, int nmsgs)
> {
> - return -ENOSYS;
> + struct intel_i2c *i2c = dev_get_priv(bus);
> + struct i2c_msg *dmsg, *omsg, dummy;
> +
> + debug("i2c_xfer: %d messages\n", nmsgs);
> +
> + memset(&dummy, 0, sizeof(struct i2c_msg));
> +
> + /*
> + * We expect either two messages (one with an offset and one with the
> + * actucal data) or one message (just data)
> + */
> + if (nmsgs > 2 || nmsgs == 0) {
> + debug("%s: Only one or two messages are supported", __func__);
> + return -EIO;
> + }
> +
> + omsg = nmsgs == 1 ? &dummy : msg;
> + dmsg = nmsgs == 1 ? msg : msg + 1;
> +
> + if (dmsg->flags & I2C_M_RD)
> + return smbus_block_read(i2c->base, dmsg->addr, &dmsg->buf[0],
> + omsg->buf[0], dmsg->len);
> + else
> + return smbus_block_write(i2c->base, dmsg->addr, &dmsg->buf[1],
> + dmsg->buf[0], dmsg->len - 1);
> }
>
> -int intel_i2c_probe_chip(struct udevice *bus, uint chip_addr, uint chip_flags)
> +static int intel_i2c_probe_chip(struct udevice *bus, uint chip_addr,
> + uint chip_flags)
> {
> - return -ENOSYS;
> + struct intel_i2c *i2c = dev_get_priv(bus);
> + u8 buf[4];
> +
> + return smbus_block_read(i2c->base, chip_addr, buf, 0, 1);
> }
>
> -int intel_i2c_set_bus_speed(struct udevice *bus, unsigned int speed)
> +static int intel_i2c_set_bus_speed(struct udevice *bus, unsigned int speed)
> {
> return 0;
> }
>
> static int intel_i2c_probe(struct udevice *dev)
> {
> + struct intel_i2c *priv = dev_get_priv(dev);
> + u32 base;
> +
> +#ifdef CONFIG_NORTHBRIDGE_INTEL_IVYBRIDGE
> + /* Handle ivybridge setup code */
> + priv->base = SMBUS_IO_BASE;
> + base = SMBUS_IO_BASE;
> +
> /*
> - * So far this is just setup code for ivybridge SMbus. When we have
> - * a full I2C driver this may need to be moved, generalised or made
> - * dependant on a particular compatible string.
> - *
> * Set SMBus I/O base
> */
> dm_pci_write_config32(dev, SMB_BASE,
> SMBUS_IO_BASE | PCI_BASE_ADDRESS_SPACE_IO);
>
> - /* Set SMBus enable. */
> - dm_pci_write_config8(dev, HOSTC, HST_EN);
> -
> /* Set SMBus I/O space enable. */
> dm_pci_write_config16(dev, PCI_COMMAND, PCI_COMMAND_IO);
>
> @@ -50,6 +268,44 @@ static int intel_i2c_probe(struct udevice *dev)
> /* Clear any lingering errors, so transactions can run. */
> outb(inb(SMBUS_IO_BASE + SMBHSTSTAT), SMBUS_IO_BASE + SMBHSTSTAT);
> debug("SMBus controller enabled\n");
> +#else
> + /* Save base address from PCI BAR */
> + priv->base = (u32)dm_pci_map_bar(dev, PCI_BASE_ADDRESS_4,
> + PCI_REGION_IO);
> + base = priv->base;
> +#endif
> +
> + /* Set SMBus enable. */
> + dm_pci_write_config8(dev, HOSTC, HST_EN);
> +
> + /* Disable interrupts */
> + outb(inb(base + SMBHSTCTL) & ~SMBHSTCNT_INTREN, base + SMBHSTCTL);
> +
> + /* Set 32-byte data buffer mode */
> + outb(inb(base + SMBAUXCTL) | SMBAUXCTL_E32B, base + SMBAUXCTL);
> +
> + return 0;
> +}
> +
> +static int intel_i2c_bind(struct udevice *dev)
> +{
> + static int num_cards;
> + char name[20];
> +
> + /* Create a unique device name for PCI type devices */
> + if (device_is_on_pci_bus(dev)) {
> + /*
> + * ToDo:
> + * Setting req_seq in the driver is probably not recommended.
> + * But without a DT alias the number is not configured. And
> + * using this driver is impossible for PCIe I2C devices.
> + * This can be removed, once a better (correct) way for this
> + * is found and implemented.
> + */
> + dev->req_seq = num_cards;
> + sprintf(name, "intel_i2c#%u", num_cards++);
> + device_set_name(dev, name);
> + }
>
> return 0;
> }
> @@ -71,5 +327,15 @@ U_BOOT_DRIVER(intel_i2c) = {
> .of_match = intel_i2c_ids,
> .per_child_auto_alloc_size = sizeof(struct dm_i2c_chip),
> .ops = &intel_i2c_ops,
> + .priv_auto_alloc_size = sizeof(struct intel_i2c),
> + .bind = intel_i2c_bind,
> .probe = intel_i2c_probe,
> };
> +
> +static struct pci_device_id intel_smbus_pci_supported[] = {
> + /* Intel BayTrail SMBus on the PCI bus */
> + { PCI_VDEVICE(INTEL, 0x0f12) },
> + {},
> +};
> +
> +U_BOOT_PCI_DEVICE(intel_i2c, intel_smbus_pci_supported);
> --
> 2.9.0
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
More information about the U-Boot
mailing list