Hi Bin, On Mon, 25 Nov 2019 at 22:09, Bin Meng <bmeng...@gmail.com> wrote: > > Hi Simon, > > > On Mon, Nov 25, 2019 at 12:11 PM Simon Glass <s...@chromium.org> wrote: > > > > This is hacked into the driver at present. It seems better to have it as > > a separate driver that uses the base driver. Create a new file and put > > the X86 code into it. > > > > Actually the Baytrail settings should really come from the device tree. > > > > Note that 'has_max_speed' is added as well. This is currently always false > > but since only Baytrail provides the config, it does not affect operation > > for other devices. > > > > Signed-off-by: Simon Glass <s...@chromium.org> > > Reviewed-by: Heiko Schocher <h...@denx.de> > > --- > > > > Changes in v5: None > > Changes in v4: > > - Add a comment about the speed logic in __dw_i2c_set_bus_speed() > > - Add a comment in the commit message about why has_max_speed is added > > - Drop unwanted debug printf("bad\n") > > - Fix indentation nit > > - Rename new file to designware_i2c_pci.c > > > > Changes in v3: None > > Changes in v2: None > > > > drivers/i2c/Makefile | 3 + > > drivers/i2c/designware_i2c.c | 106 +++++-------------------------- > > drivers/i2c/designware_i2c.h | 35 ++++++++++ > > drivers/i2c/designware_i2c_pci.c | 79 +++++++++++++++++++++++ > > 4 files changed, 134 insertions(+), 89 deletions(-) > > create mode 100644 drivers/i2c/designware_i2c_pci.c > > > > diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile > > index c2f75d8755..f5a471f887 100644 > > --- a/drivers/i2c/Makefile > > +++ b/drivers/i2c/Makefile > > @@ -14,6 +14,9 @@ obj-$(CONFIG_SYS_I2C_AT91) += at91_i2c.o > > obj-$(CONFIG_SYS_I2C_CADENCE) += i2c-cdns.o > > obj-$(CONFIG_SYS_I2C_DAVINCI) += davinci_i2c.o > > obj-$(CONFIG_SYS_I2C_DW) += designware_i2c.o > > +ifdef CONFIG_DM_PCI > > +obj-$(CONFIG_SYS_I2C_DW) += designware_i2c_pci.o > > +endif > > obj-$(CONFIG_SYS_I2C_FSL) += fsl_i2c.o > > obj-$(CONFIG_SYS_I2C_IHS) += ihs_i2c.o > > obj-$(CONFIG_SYS_I2C_INTEL) += intel_i2c.o > > diff --git a/drivers/i2c/designware_i2c.c b/drivers/i2c/designware_i2c.c > > index 6daa90e744..99b7d09bb2 100644 > > --- a/drivers/i2c/designware_i2c.c > > +++ b/drivers/i2c/designware_i2c.c > > @@ -13,34 +13,6 @@ > > #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; > > - struct reset_ctl_bulk resets; > > -#if CONFIG_IS_ENABLED(CLK) > > - struct clk clk; > > -#endif > > -}; > > - > > #ifdef CONFIG_SYS_I2C_DW_ENABLE_STATUS_UNSUPPORTED > > static int dw_i2c_enable(struct i2c_regs *i2c_base, bool enable) > > { > > @@ -90,7 +62,9 @@ static unsigned int __dw_i2c_set_bus_speed(struct > > i2c_regs *i2c_base, > > unsigned int ena; > > int i2c_spd; > > > > - if (speed >= I2C_MAX_SPEED) > > + /* Allow max speed if there is no config , or the config allows it > > */ > > nits: remove the space before ,
OK done. > > > + if (speed >= I2C_MAX_SPEED && > > + (!scl_sda_cfg || scl_sda_cfg->has_max_speed)) > > So with this change, for Baytrail, this new logic does not match the old > codes. > > Baytrail has the config, but its config does not have the > has_max_speed, so the if () logic evaluates to false. From my reading of it, that is correct. The mode_max code is behind an #ifdef in the old code. > > > i2c_spd = IC_SPEED_MODE_MAX; > > else if (speed >= I2C_FAST_SPEED) > > i2c_spd = IC_SPEED_MODE_FAST; > > @@ -106,7 +80,6 @@ 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_SS; > > if (scl_sda_cfg) { > > @@ -119,7 +92,6 @@ static unsigned int __dw_i2c_set_bus_speed(struct > > i2c_regs *i2c_base, > > writel(hcnt, &i2c_base->ic_hs_scl_hcnt); > > writel(lcnt, &i2c_base->ic_hs_scl_lcnt); > > break; > > -#endif [..] Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot