Hi Stefan, On Mon, Mar 21, 2016 at 5:03 PM, Stefan Roese <s...@denx.de> wrote: > Hi Bin, > > On 21.03.2016 09:54, Bin Meng wrote: >> On Fri, Mar 18, 2016 at 3:54 PM, Stefan Roese <s...@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 <s...@denx.de> >>> Cc: Simon Glass <s...@chromium.org> >>> Cc: Bin Meng <bmeng...@gmail.com> >>> Cc: Marek Vasut <ma...@denx.de> >>> Cc: Heiko Schocher <h...@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. >
Great. So looks we may consolidate one usage for such both PCI and SoC devices, like the one used in drivers/net/designware.c? Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot