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

2016-08-08 Thread Stefan Roese

Hi George,

On 05.08.2016 18:09, George McCollister wrote:

On Tue, Jun 28, 2016 at 8:44 AM, Stefan Roese  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 
Cc: Bin Meng 
Cc: Simon Glass 
Cc: Heiko Schocher 
---
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  0xea51

-/* 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 VCH0x  /* 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 
  *
+ * SMBus block read/write support added by Stefan Roese:
+ * Copyright (C) 2016 Stefan Roese 
+ *
  * SPDX-License-Identifier: GPL-2.0+
  */

 #include 
 #include 
 #include 
+#include 
 #include 
+#ifdef CONFIG_NORTHBRIDGE_INTEL_IVYBRIDGE
 #include 
+#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_DONE0x80
+#define SMBHSTSTS_INUSE_STS0x40
+#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_BUSY0x01
+
+/* I801 Host Control register bits */
+#define SMBHSTCNT_INTREN   0x01
+#define SMBHSTCNT_KILL 0x02
+#define SMBHSTCNT_LAST_BYTE0x20
+#define SMBHSTCNT_START0x40
+#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 

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

2016-08-07 Thread Stefan Roese

Hi Simon,

On 06.08.2016 05:36, Simon Glass wrote:

On 5 August 2016 at 01:18, Stefan Roese  wrote:

On 05.08.2016 09:10, Heiko Schocher wrote:


Hello Bin,

Am 05.08.2016 um 07:46 schrieb Bin Meng:


Simon, Stefan,

On Tue, Jul 26, 2016 at 8:13 PM, Stefan Roese  wrote:


Hi Simon,

On 25.07.2016 04:07, Simon Glass wrote:



On 28 June 2016 at 07:44, Stefan Roese  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 
Cc: Bin Meng 
Cc: Simon Glass 
Cc: Heiko Schocher 
---
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?




Yes this breaks booting on link. Something odd is going on because the
call to set up I2C in ivybridge's print_cpuinfo() returns a very
strange error -726376.




Hmmm, very strange.


But I then enabled CONFIG_CMD_I2C and it boots. However 'i2c probe'
produces a lot of errors like this:

ERROR: len=0 on read
smbus_block_read (107): dev=0x3b offs=0x0 len=0x1
smbus_block_read (136): count=0 (len=1)




A general question:

Is the SMBus controller on Ivybridge also exported as PCI device? If
yes, can't we just use the PCI code as done for BayTrail for this
platform as well? And get rid of the platform specific stuff this
way?

Could you send me the output of "pci 0 long" on this platform?



Do you plan to get this I2C merged in this release? If so, please work
this out .. I don't feel comfortable to apply this at present.



Full Ack.



I really would like to see this SMBus support upstream. As other
patches depend on this. Unfortunately I can't test on the Ivybridge
platform. I talked with Simon on #irc some days ago and he
"volunteered" (thanks again) to fix / debug this Ivybridge problem
on his board - perhaps by moving to a PCI based probing there as
well.

Simon, did you find the time to dig into this? Please let me know if
there is something that I can do to help / assist you here.

Thanks,
Stefan


I found one problem, which is that intel_i2c_bind() writes to BSS
before it is available. That fixes the crash. I sent a few patches for
that and something else I found.


Great, thanks!


Also I don't think your code in intel_i2c_probe() is very different
from the ivybridge code. SMB_BASE is the same as PCI_BASE_ADDRESS_4.
So perhaps just drop the ivybridge code?


Thats what I meant with PCI based probing. I will send a v2 of this
patch out on Monday and will drop the Ivybrige code in this version.
I'll also fix the crash below (thanks to George for spotting a
problem here).

Thanks,
Stefan


Here's the output you asked for.

=> pci 0 long
Scanning PCI devices on bus 0

Found PCI device 00.00.00:
  vendor ID =   0x8086
  device ID =   0x0154
  command register ID = 0x0006
  status register = 0x2090
  revision ID = 0x09
  class code =  0x06 (Bridge device)
  sub class code =  0x00
  programming interface =   0x00
  cache line =  0x00
  latency time =0x00
  header type = 0x00
  BIST =0x00
  base address 0 =  0x
  base address 1 =  0x
  base address 2 =  0x
  base address 3 =  0x
  base address 4 =  0x
  base address 5 =  0x
  cardBus CIS pointer = 0x
  sub system vendor ID =0x
  sub system ID =   0x
  expansion ROM base address =  0x
  interrupt line =  0x00
  interrupt pin =   0x00
  min Grant =   0x00
  max Latency = 0x00

Found PCI device 00.02.00:
  vendor ID =   0x8086
  device ID =   0x0166
  command register ID = 0x0007
  status register = 0x0090
  revision ID = 0x09
  class code =  0x03 (Display controller)
  sub class code =  0x00
  programming interface =   0x00
  cache line =  0x00
  latency time =0x00
  header type = 0x00
  BIST =0x00
  base address 0 =  0xe004
  base address 1 =  0x
  base address 2 =  0xd00c
  base address 3 =  0x
  base address 4 = 

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

2016-08-05 Thread Simon Glass
Hi Stefan,

On 5 August 2016 at 01:18, Stefan Roese  wrote:
> On 05.08.2016 09:10, Heiko Schocher wrote:
>>
>> Hello Bin,
>>
>> Am 05.08.2016 um 07:46 schrieb Bin Meng:
>>>
>>> Simon, Stefan,
>>>
>>> On Tue, Jul 26, 2016 at 8:13 PM, Stefan Roese  wrote:

 Hi Simon,

 On 25.07.2016 04:07, Simon Glass wrote:
>
>
> On 28 June 2016 at 07:44, Stefan Roese  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 
>> Cc: Bin Meng 
>> Cc: Simon Glass 
>> Cc: Heiko Schocher 
>> ---
>> 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?
>
>
>
> Yes this breaks booting on link. Something odd is going on because the
> call to set up I2C in ivybridge's print_cpuinfo() returns a very
> strange error -726376.



 Hmmm, very strange.

> But I then enabled CONFIG_CMD_I2C and it boots. However 'i2c probe'
> produces a lot of errors like this:
>
> ERROR: len=0 on read
> smbus_block_read (107): dev=0x3b offs=0x0 len=0x1
> smbus_block_read (136): count=0 (len=1)



 A general question:

 Is the SMBus controller on Ivybridge also exported as PCI device? If
 yes, can't we just use the PCI code as done for BayTrail for this
 platform as well? And get rid of the platform specific stuff this
 way?

 Could you send me the output of "pci 0 long" on this platform?

>>>
>>> Do you plan to get this I2C merged in this release? If so, please work
>>> this out .. I don't feel comfortable to apply this at present.
>>
>>
>> Full Ack.
>
>
> I really would like to see this SMBus support upstream. As other
> patches depend on this. Unfortunately I can't test on the Ivybridge
> platform. I talked with Simon on #irc some days ago and he
> "volunteered" (thanks again) to fix / debug this Ivybridge problem
> on his board - perhaps by moving to a PCI based probing there as
> well.
>
> Simon, did you find the time to dig into this? Please let me know if
> there is something that I can do to help / assist you here.
>
> Thanks,
> Stefan

I found one problem, which is that intel_i2c_bind() writes to BSS
before it is available. That fixes the crash. I sent a few patches for
that and something else I found.

Also I don't think your code in intel_i2c_probe() is very different
from the ivybridge code. SMB_BASE is the same as PCI_BASE_ADDRESS_4.
So perhaps just drop the ivybridge code?

Here's the output you asked for.

=> pci 0 long
Scanning PCI devices on bus 0

Found PCI device 00.00.00:
  vendor ID =   0x8086
  device ID =   0x0154
  command register ID = 0x0006
  status register = 0x2090
  revision ID = 0x09
  class code =  0x06 (Bridge device)
  sub class code =  0x00
  programming interface =   0x00
  cache line =  0x00
  latency time =0x00
  header type = 0x00
  BIST =0x00
  base address 0 =  0x
  base address 1 =  0x
  base address 2 =  0x
  base address 3 =  0x
  base address 4 =  0x
  base address 5 =  0x
  cardBus CIS pointer = 0x
  sub system vendor ID =0x
  sub system ID =   0x
  expansion ROM base address =  0x
  interrupt line =  0x00
  interrupt pin =   0x00
  min Grant =   0x00
  max Latency = 0x00

Found PCI device 00.02.00:
  vendor ID =   0x8086
  device ID =   0x0166
  command register ID = 0x0007
  status register = 0x0090
  revision ID = 0x09
  class code =  0x03 (Display controller)
  sub class code =  0x00
  programming interface =   0x00
  cache line =  0x00
  latency time =0x00
  header type = 0x00
  BIST =0x00
  base address 0 =  0xe004
  base address 1 =  0x
  base address 2 

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

2016-08-05 Thread George McCollister
On Tue, Jun 28, 2016 at 8:44 AM, Stefan Roese  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 
> Cc: Bin Meng 
> Cc: Simon Glass 
> Cc: Heiko Schocher 
> ---
> 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  0xea51
>
> -/* 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 VCH0x  /* 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 
>   *
> + * SMBus block read/write support added by Stefan Roese:
> + * Copyright (C) 2016 Stefan Roese 
> + *
>   * SPDX-License-Identifier: GPL-2.0+
>   */
>
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
> +#ifdef CONFIG_NORTHBRIDGE_INTEL_IVYBRIDGE
>  #include 
> +#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_DONE0x80
> +#define SMBHSTSTS_INUSE_STS0x40
> +#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_BUSY0x01
> +
> +/* I801 Host Control register bits */
> +#define SMBHSTCNT_INTREN   0x01
> +#define SMBHSTCNT_KILL 0x02
> +#define SMBHSTCNT_LAST_BYTE0x20
> +#define SMBHSTCNT_START0x40
> +#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);
> +

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

2016-08-05 Thread Stefan Roese

On 05.08.2016 09:10, Heiko Schocher wrote:

Hello Bin,

Am 05.08.2016 um 07:46 schrieb Bin Meng:

Simon, Stefan,

On Tue, Jul 26, 2016 at 8:13 PM, Stefan Roese  wrote:

Hi Simon,

On 25.07.2016 04:07, Simon Glass wrote:


On 28 June 2016 at 07:44, Stefan Roese  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 
Cc: Bin Meng 
Cc: Simon Glass 
Cc: Heiko Schocher 
---
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?



Yes this breaks booting on link. Something odd is going on because the
call to set up I2C in ivybridge's print_cpuinfo() returns a very
strange error -726376.



Hmmm, very strange.


But I then enabled CONFIG_CMD_I2C and it boots. However 'i2c probe'
produces a lot of errors like this:

ERROR: len=0 on read
smbus_block_read (107): dev=0x3b offs=0x0 len=0x1
smbus_block_read (136): count=0 (len=1)



A general question:

Is the SMBus controller on Ivybridge also exported as PCI device? If
yes, can't we just use the PCI code as done for BayTrail for this
platform as well? And get rid of the platform specific stuff this
way?

Could you send me the output of "pci 0 long" on this platform?



Do you plan to get this I2C merged in this release? If so, please work
this out .. I don't feel comfortable to apply this at present.


Full Ack.


I really would like to see this SMBus support upstream. As other
patches depend on this. Unfortunately I can't test on the Ivybridge
platform. I talked with Simon on #irc some days ago and he
"volunteered" (thanks again) to fix / debug this Ivybridge problem
on his board - perhaps by moving to a PCI based probing there as
well.

Simon, did you find the time to dig into this? Please let me know if
there is something that I can do to help / assist you here.

Thanks,
Stefan
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2016-08-05 Thread Heiko Schocher

Hello Bin,

Am 05.08.2016 um 07:46 schrieb Bin Meng:

Simon, Stefan,

On Tue, Jul 26, 2016 at 8:13 PM, Stefan Roese  wrote:

Hi Simon,

On 25.07.2016 04:07, Simon Glass wrote:


On 28 June 2016 at 07:44, Stefan Roese  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 
Cc: Bin Meng 
Cc: Simon Glass 
Cc: Heiko Schocher 
---
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?



Yes this breaks booting on link. Something odd is going on because the
call to set up I2C in ivybridge's print_cpuinfo() returns a very
strange error -726376.



Hmmm, very strange.


But I then enabled CONFIG_CMD_I2C and it boots. However 'i2c probe'
produces a lot of errors like this:

ERROR: len=0 on read
smbus_block_read (107): dev=0x3b offs=0x0 len=0x1
smbus_block_read (136): count=0 (len=1)



A general question:

Is the SMBus controller on Ivybridge also exported as PCI device? If
yes, can't we just use the PCI code as done for BayTrail for this
platform as well? And get rid of the platform specific stuff this
way?

Could you send me the output of "pci 0 long" on this platform?



Do you plan to get this I2C merged in this release? If so, please work
this out .. I don't feel comfortable to apply this at present.


Full Ack.

bye,
Heiko
--
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2016-08-04 Thread Bin Meng
Simon, Stefan,

On Tue, Jul 26, 2016 at 8:13 PM, Stefan Roese  wrote:
> Hi Simon,
>
> On 25.07.2016 04:07, Simon Glass wrote:
>>
>> On 28 June 2016 at 07:44, Stefan Roese  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 
>>> Cc: Bin Meng 
>>> Cc: Simon Glass 
>>> Cc: Heiko Schocher 
>>> ---
>>> 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?
>>
>>
>> Yes this breaks booting on link. Something odd is going on because the
>> call to set up I2C in ivybridge's print_cpuinfo() returns a very
>> strange error -726376.
>
>
> Hmmm, very strange.
>
>> But I then enabled CONFIG_CMD_I2C and it boots. However 'i2c probe'
>> produces a lot of errors like this:
>>
>> ERROR: len=0 on read
>> smbus_block_read (107): dev=0x3b offs=0x0 len=0x1
>> smbus_block_read (136): count=0 (len=1)
>
>
> A general question:
>
> Is the SMBus controller on Ivybridge also exported as PCI device? If
> yes, can't we just use the PCI code as done for BayTrail for this
> platform as well? And get rid of the platform specific stuff this
> way?
>
> Could you send me the output of "pci 0 long" on this platform?
>

Do you plan to get this I2C merged in this release? If so, please work
this out .. I don't feel comfortable to apply this at present.

Regards,
Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2016-07-26 Thread Stefan Roese

Hi Simon,

On 25.07.2016 04:07, Simon Glass wrote:

On 28 June 2016 at 07:44, Stefan Roese  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 
Cc: Bin Meng 
Cc: Simon Glass 
Cc: Heiko Schocher 
---
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?


Yes this breaks booting on link. Something odd is going on because the
call to set up I2C in ivybridge's print_cpuinfo() returns a very
strange error -726376.


Hmmm, very strange.


But I then enabled CONFIG_CMD_I2C and it boots. However 'i2c probe'
produces a lot of errors like this:

ERROR: len=0 on read
smbus_block_read (107): dev=0x3b offs=0x0 len=0x1
smbus_block_read (136): count=0 (len=1)


A general question:

Is the SMBus controller on Ivybridge also exported as PCI device? If
yes, can't we just use the PCI code as done for BayTrail for this
platform as well? And get rid of the platform specific stuff this
way?

Could you send me the output of "pci 0 long" on this platform?

Thanks,
Stefan
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2016-07-25 Thread Stefan Roese

Hi Simon,

On 25.07.2016 04:07, Simon Glass wrote:

On 28 June 2016 at 07:44, Stefan Roese  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 
Cc: Bin Meng 
Cc: Simon Glass 
Cc: Heiko Schocher 
---
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?


Yes this breaks booting on link. Something odd is going on because the
call to set up I2C in ivybridge's print_cpuinfo() returns a very
strange error -726376.

But I then enabled CONFIG_CMD_I2C and it boots. However 'i2c probe'
produces a lot of errors like this:

ERROR: len=0 on read
smbus_block_read (107): dev=0x3b offs=0x0 len=0x1
smbus_block_read (136): count=0 (len=1)


Thanks for checking. I'll try to find some time this week to look into
this.

Thanks,
Stefan
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2016-07-24 Thread Simon Glass
Hi Stefan,

On 28 June 2016 at 07:44, Stefan Roese  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 
> Cc: Bin Meng 
> Cc: Simon Glass 
> Cc: Heiko Schocher 
> ---
> 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?

Yes this breaks booting on link. Something odd is going on because the
call to set up I2C in ivybridge's print_cpuinfo() returns a very
strange error -726376.

But I then enabled CONFIG_CMD_I2C and it boots. However 'i2c probe'
produces a lot of errors like this:

ERROR: len=0 on read
smbus_block_read (107): dev=0x3b offs=0x0 len=0x1
smbus_block_read (136): count=0 (len=1)

>
> 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  0xea51
>
> -/* 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 VCH0x  /* 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 
>   *
> + * SMBus block read/write support added by Stefan Roese:
> + * Copyright (C) 2016 Stefan Roese 
> + *
>   * SPDX-License-Identifier: GPL-2.0+
>   */
>
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
> +#ifdef CONFIG_NORTHBRIDGE_INTEL_IVYBRIDGE

We should avoid machine-specific #ifdefs in the driver.

>  #include 
> +#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_DONE0x80
> +#define SMBHSTSTS_INUSE_STS0x40
> +#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_BUSY0x01
> +
> +/* I801 Host Control register bits */
> +#define SMBHSTCNT_INTREN   0x01
> +#define SMBHSTCNT_KILL 0x02
> +#define SMBHSTCNT_LAST_BYTE0x20
> +#define SMBHSTCNT_START0x40
> +#define SMBHSTCNT_PEC_EN   0x80/* ICH3 and later */
> +
> +/* Auxiliary control register bits, ICH4+ only */
> +#define SMBAUXCTL_CRC  1
> +#define SMBAUXCTL_E32B 2
> +
> 

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

2016-07-18 Thread Bin Meng
Simon, Heiko,

On Tue, Jul 12, 2016 at 1:22 PM, Bin Meng  wrote:
> Hi Simon,
>
> On Tue, Jun 28, 2016 at 9:44 PM, Stefan Roese  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 
>> Cc: Bin Meng 
>> Cc: Simon Glass 
>> Cc: Heiko Schocher 
>> ---
>> 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?
>>
>
> Can you test this patch on Ivybridge to see if it breaks anything? Is
> SMBus used on Ivybridge for the memory initialization (eg: reading
> SPD?)
>

Do you have any review comments before it's applied? I believe this
needs to be tested on Ivybridge at least.

Regards,
Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2016-07-11 Thread Bin Meng
Hi Simon,

On Tue, Jun 28, 2016 at 9:44 PM, Stefan Roese  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 
> Cc: Bin Meng 
> Cc: Simon Glass 
> Cc: Heiko Schocher 
> ---
> 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?
>

Can you test this patch on Ivybridge to see if it breaks anything? Is
SMBus used on Ivybridge for the memory initialization (eg: reading
SPD?)

Regards,
Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


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

2016-06-28 Thread Stefan Roese
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 
Cc: Bin Meng 
Cc: Simon Glass 
Cc: Heiko Schocher 
---
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  0xea51
 
-/* 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 VCH0x  /* 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 
  *
+ * SMBus block read/write support added by Stefan Roese:
+ * Copyright (C) 2016 Stefan Roese 
+ *
  * SPDX-License-Identifier: GPL-2.0+
  */
 
 #include 
 #include 
 #include 
+#include 
 #include 
+#ifdef CONFIG_NORTHBRIDGE_INTEL_IVYBRIDGE
 #include 
+#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_DONE0x80
+#define SMBHSTSTS_INUSE_STS0x40
+#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_BUSY0x01
+
+/* I801 Host Control register bits */
+#define SMBHSTCNT_INTREN   0x01
+#define SMBHSTCNT_KILL 0x02
+#define SMBHSTCNT_LAST_BYTE0x20
+#define SMBHSTCNT_START0x40
+#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) <