[U-Boot] [PATCH 6/6] i2c: designware_i2c: Add support for PCI(e) based I2C cores (x86)

[Stefan Roese <sr@denx.de>]

Hi Bin,

On 21.03.2016 09:54, Bin Meng wrote:
> On Fri, Mar 18, 2016 at 3:54 PM, Stefan Roese <sr@denx.de> wrote:
>> This patch adds support for the PCI(e) based I2C cores. Which can be
>> found for example on the Intel Bay Trail SoC. It has 7 I2C controllers
>> implemented as PCI devices.
>>
>> This patch also adds the fixed values for the timing registers for
>> BayTrail which are taken from the Linux designware I2C driver.
>>
>> Signed-off-by: Stefan Roese <sr@denx.de>
>> Cc: Simon Glass <sjg@chromium.org>
>> Cc: Bin Meng <bmeng.cn@gmail.com>
>> Cc: Marek Vasut <marex@denx.de>
>> Cc: Heiko Schocher <hs@denx.de>
>> ---
>>   drivers/i2c/designware_i2c.c | 111 +++++++++++++++++++++++++++++++++++++++----
>>   1 file changed, 101 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/i2c/designware_i2c.c b/drivers/i2c/designware_i2c.c
>> index 4e5340d..f7f2eba 100644
>> --- a/drivers/i2c/designware_i2c.c
>> +++ b/drivers/i2c/designware_i2c.c
>> @@ -8,11 +8,32 @@
>>   #include <common.h>
>>   #include <dm.h>
>>   #include <i2c.h>
>> +#include <pci.h>
>>   #include <asm/io.h>
>>   #include "designware_i2c.h"
>>
>> +struct dw_scl_sda_cfg {
>> +       u32 ss_hcnt;
>> +       u32 fs_hcnt;
>> +       u32 ss_lcnt;
>> +       u32 fs_lcnt;
>> +       u32 sda_hold;
>> +};
>> +
>> +#ifdef CONFIG_X86
>> +/* BayTrail HCNT/LCNT/SDA hold time */
>> +static struct dw_scl_sda_cfg byt_config = {
>> +       .ss_hcnt = 0x200,
>> +       .fs_hcnt = 0x55,
>> +       .ss_lcnt = 0x200,
>> +       .fs_lcnt = 0x99,
>> +       .sda_hold = 0x6,
>> +};
>> +#endif
>> +
>>   struct dw_i2c {
>>          struct i2c_regs *regs;
>> +       struct dw_scl_sda_cfg *scl_sda_cfg;
>>   };
>>
>>   static void dw_i2c_enable(struct i2c_regs *i2c_base, bool enable)
>> @@ -42,6 +63,7 @@ static void dw_i2c_enable(struct i2c_regs *i2c_base, bool enable)
>>    * Set the i2c speed.
>>    */
>>   static unsigned int __dw_i2c_set_bus_speed(struct i2c_regs *i2c_base,
>> +                                          struct dw_scl_sda_cfg *scl_sda_cfg,
>>                                             unsigned int speed)
>>   {
>>          unsigned int cntl;
>> @@ -61,34 +83,55 @@ static unsigned int __dw_i2c_set_bus_speed(struct i2c_regs *i2c_base,
>>          cntl = (readl(&i2c_base->ic_con) & (~IC_CON_SPD_MSK));
>>
>>          switch (i2c_spd) {
>> +#ifndef CONFIG_X86 /* No High-speed for BayTrail yet */
>>          case IC_SPEED_MODE_MAX:
>> -               cntl |= IC_CON_SPD_HS;
>> -               hcnt = (IC_CLK * MIN_HS_SCL_HIGHTIME) / NANO_TO_MICRO;
>> +               cntl |= IC_CON_SPD_SS;
>> +               if (scl_sda_cfg) {
>> +                       hcnt = scl_sda_cfg->fs_hcnt;
>> +                       lcnt = scl_sda_cfg->fs_lcnt;
>> +               } else {
>> +                       hcnt = (IC_CLK * MIN_HS_SCL_HIGHTIME) / NANO_TO_MICRO;
>> +                       lcnt = (IC_CLK * MIN_HS_SCL_LOWTIME) / NANO_TO_MICRO;
>> +               }
>>                  writel(hcnt, &i2c_base->ic_hs_scl_hcnt);
>> -               lcnt = (IC_CLK * MIN_HS_SCL_LOWTIME) / NANO_TO_MICRO;
>>                  writel(lcnt, &i2c_base->ic_hs_scl_lcnt);
>>                  break;
>> +#endif
>>
>>          case IC_SPEED_MODE_STANDARD:
>>                  cntl |= IC_CON_SPD_SS;
>> -               hcnt = (IC_CLK * MIN_SS_SCL_HIGHTIME) / NANO_TO_MICRO;
>> +               if (scl_sda_cfg) {
>> +                       hcnt = scl_sda_cfg->ss_hcnt;
>> +                       lcnt = scl_sda_cfg->ss_lcnt;
>> +               } else {
>> +                       hcnt = (IC_CLK * MIN_SS_SCL_HIGHTIME) / NANO_TO_MICRO;
>> +                       lcnt = (IC_CLK * MIN_SS_SCL_LOWTIME) / NANO_TO_MICRO;
>> +               }
>>                  writel(hcnt, &i2c_base->ic_ss_scl_hcnt);
>> -               lcnt = (IC_CLK * MIN_SS_SCL_LOWTIME) / NANO_TO_MICRO;
>>                  writel(lcnt, &i2c_base->ic_ss_scl_lcnt);
>>                  break;
>>
>>          case IC_SPEED_MODE_FAST:
>>          default:
>>                  cntl |= IC_CON_SPD_FS;
>> -               hcnt = (IC_CLK * MIN_FS_SCL_HIGHTIME) / NANO_TO_MICRO;
>> +               if (scl_sda_cfg) {
>> +                       hcnt = scl_sda_cfg->fs_hcnt;
>> +                       lcnt = scl_sda_cfg->fs_lcnt;
>> +               } else {
>> +                       hcnt = (IC_CLK * MIN_FS_SCL_HIGHTIME) / NANO_TO_MICRO;
>> +                       lcnt = (IC_CLK * MIN_FS_SCL_LOWTIME) / NANO_TO_MICRO;
>> +               }
>>                  writel(hcnt, &i2c_base->ic_fs_scl_hcnt);
>> -               lcnt = (IC_CLK * MIN_FS_SCL_LOWTIME) / NANO_TO_MICRO;
>>                  writel(lcnt, &i2c_base->ic_fs_scl_lcnt);
>>                  break;
>>          }
>>
>>          writel(cntl, &i2c_base->ic_con);
>>
>> +       /* Configure SDA Hold Time if required */
>> +       if (scl_sda_cfg)
>> +               writel(scl_sda_cfg->sda_hold, &i2c_base->ic_sda_hold);
>> +
>>          /* Enable back i2c now speed set */
>>          dw_i2c_enable(i2c_base, 1);
>>
>> @@ -315,7 +358,7 @@ static void __dw_i2c_init(struct i2c_regs *i2c_base, int speed, int slaveaddr)
>>          writel(IC_TX_TL, &i2c_base->ic_tx_tl);
>>          writel(IC_STOP_DET, &i2c_base->ic_intr_mask);
>>   #ifndef CONFIG_DM_I2C
>> -       __dw_i2c_set_bus_speed(i2c_base, speed);
>> +       __dw_i2c_set_bus_speed(i2c_base, NULL, speed);
>>          writel(slaveaddr, &i2c_base->ic_sar);
>>   #endif
>>
>> @@ -356,7 +399,7 @@ static unsigned int dw_i2c_set_bus_speed(struct i2c_adapter *adap,
>>                                           unsigned int speed)
>>   {
>>          adap->speed = speed;
>> -       return __dw_i2c_set_bus_speed(i2c_get_base(adap), speed);
>> +       return __dw_i2c_set_bus_speed(i2c_get_base(adap), NULL, speed);
>>   }
>>
>>   static void dw_i2c_init(struct i2c_adapter *adap, int speed, int slaveaddr)
>> @@ -451,7 +494,7 @@ static int designware_i2c_set_bus_speed(struct udevice *bus, unsigned int speed)
>>   {
>>          struct dw_i2c *i2c = dev_get_priv(bus);
>>
>> -       return __dw_i2c_set_bus_speed(i2c->regs, speed);
>> +       return __dw_i2c_set_bus_speed(i2c->regs, i2c->scl_sda_cfg, speed);
>>   }
>>
>>   static int designware_i2c_probe_chip(struct udevice *bus, uint chip_addr,
>> @@ -476,14 +519,45 @@ static int designware_i2c_probe(struct udevice *bus)
>>   {
>>          struct dw_i2c *priv = dev_get_priv(bus);
>>
>> +#ifdef CONFIG_X86
>> +       /* Save base address from PCI BAR */
>> +       priv->regs = (struct i2c_regs *)
>> +               dm_pci_map_bar(bus, PCI_BASE_ADDRESS_0, PCI_REGION_MEM);
>> +       /* Use BayTrail specific timing values */
>> +       priv->scl_sda_cfg = &byt_config;
>> +#else
> 
> How about:
> 
>      if (device_is_on_pci_bus(dev)) {
>      do the PCI I2C stuff here;
>      }

I've tried this but it generated compilation errors on socfpga, as the
dm_pci_xxx functions are not available there. So it definitely needs
some #ifdef here. I could go with your suggestion and use
#if CONFIG_DM_PCI as well.
 
> See driver/net/designware.c for example.
> 
>>          /* Save base address from device-tree */
>>          priv->regs = (struct i2c_regs *)dev_get_addr(bus);
>> +#endif
>>
>>          __dw_i2c_init(priv->regs, 0, 0);
>>
>>          return 0;
>>   }
>>
>> +static int designware_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, "i2c_designware#%u", num_cards++);
>> +               device_set_name(dev, name);
>> +       }
> 
> I believe the above should be wrapped by #ifdef CONFIG_DM_PCI #endif,
> otherwise it won't build when DM_PCI is not enabled.

It does build and work on socfpga without CONFIG_DM_PCI enabled. I've
double-checked this. The only problem is the dm_pci_xxx() in
designware_i2c_probe() as I mentioned above.

Thanks,
Stefan