On Tue, 2015-12-22 at 15:35 -0600, Suravee Suthikulpanit wrote: > The current driver uses input clock source frequency to calculate > values for [SS|FS]_[HC|LC] registers. However, when booting ACPI, we > do not > currently have a good way to provide the frequency information. > Instead, we can leverage the SSCN and FFCN ACPI methods, which can be > used > to directly provide these values. So, the clock information should > no longer be required during probing. > > However, since clk can be invalid, additional checks must be done > where > we are making use of it. > > Signed-off-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx> > Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@xxxxxxx> > --- > Note: This has been tested on AMD Seattle RevB for both DT and ACPI. > > Changes from V2 (https://lkml.org/lkml/2015/12/22/584): > * Add the i2c_dw_clk_rate from Mika. > * Add check if get_clk_rate_khz is set before setting > sda_hold_time > > Changes from V1 (https://lkml.org/lkml/2015/12/15/792): > In v1, I disregarded the clock if SSCN and FMCN are provided, > assuming that it was not needed. That was incorrect assumption, > and is now fixed in v2. > > drivers/i2c/busses/i2c-designware-core.c | 22 +++++++++++++++-- > ----- > drivers/i2c/busses/i2c-designware-platdrv.c | 24 ++++++++++++------- > ----- > 2 files changed, 27 insertions(+), 19 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-designware-core.c > b/drivers/i2c/busses/i2c-designware-core.c > index 8c48b27..25dccd8 100644 > --- a/drivers/i2c/busses/i2c-designware-core.c > +++ b/drivers/i2c/busses/i2c-designware-core.c > @@ -271,6 +271,17 @@ static void __i2c_dw_enable(struct dw_i2c_dev > *dev, bool enable) > enable ? "en" : "dis"); > } > > +static unsigned long i2c_dw_clk_rate(struct dw_i2c_dev *dev) > +{ > + /* > + * Clock is not necessary if we got LCNT/HCNT values > directly from > + * the platform code. > + */ > + if (WARN_ON_ONCE(!dev->get_clk_rate_khz)) > + return 0; > + return dev->get_clk_rate_khz(dev); > +} > + > /** > * i2c_dw_init() - initialize the designware i2c master hardware > * @dev: device private data > @@ -281,7 +292,6 @@ static void __i2c_dw_enable(struct dw_i2c_dev > *dev, bool enable) > */ > int i2c_dw_init(struct dw_i2c_dev *dev) > { > - u32 input_clock_khz; > u32 hcnt, lcnt; > u32 reg; > u32 sda_falling_time, scl_falling_time; > @@ -295,8 +305,6 @@ int i2c_dw_init(struct dw_i2c_dev *dev) > } > } > > - input_clock_khz = dev->get_clk_rate_khz(dev); > - > reg = dw_readl(dev, DW_IC_COMP_TYPE); > if (reg == ___constant_swab32(DW_IC_COMP_TYPE_VALUE)) { > /* Configure register endianess access */ > @@ -325,12 +333,12 @@ int i2c_dw_init(struct dw_i2c_dev *dev) > hcnt = dev->ss_hcnt; > lcnt = dev->ss_lcnt; > } else { > - hcnt = i2c_dw_scl_hcnt(input_clock_khz, > + hcnt = i2c_dw_scl_hcnt(i2c_dw_clk_rate(dev), > 4000, /* tHD;STA = > tHIGH = 4.0 us */ > sda_falling_time, > 0, /* 0: DW default, > 1: Ideal */ > 0); /* No offset */ > - lcnt = i2c_dw_scl_lcnt(input_clock_khz, > + lcnt = i2c_dw_scl_lcnt(i2c_dw_clk_rate(dev), > 4700, /* tLOW = 4.7 > us */ > scl_falling_time, > 0); /* No offset */ > @@ -344,12 +352,12 @@ int i2c_dw_init(struct dw_i2c_dev *dev) > hcnt = dev->fs_hcnt; > lcnt = dev->fs_lcnt; > } else { > - hcnt = i2c_dw_scl_hcnt(input_clock_khz, > + hcnt = i2c_dw_scl_hcnt(i2c_dw_clk_rate(dev), > 600, /* tHD;STA = > tHIGH = 0.6 us */ > sda_falling_time, > 0, /* 0: DW default, > 1: Ideal */ > 0); /* No offset */ > - lcnt = i2c_dw_scl_lcnt(input_clock_khz, > + lcnt = i2c_dw_scl_lcnt(i2c_dw_clk_rate(dev), > 1300, /* tLOW = 1.3 > us */ > scl_falling_time, > 0); /* No offset */ > diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c > b/drivers/i2c/busses/i2c-designware-platdrv.c > index 8ffc36b..04edd09 100644 > --- a/drivers/i2c/busses/i2c-designware-platdrv.c > +++ b/drivers/i2c/busses/i2c-designware-platdrv.c Can we introduce static int i2c_dw_plat_prepare_clk(struct dw_i2c_dev *i_dev, bool prepare) { if (IS_ERR(i_dev->clk)) return PTR_ERR(i_dev->clk); if (prepare) /* REMOVEME: Yes, you have to check return value and this is one benefit of this change */ return clk_prepare_enable(i_dev->clk); clk_disable_unprepare(i_dev->clk); return 0; } and… > @@ -205,16 +205,14 @@ static int dw_i2c_plat_probe(struct > platform_device *pdev) > DW_IC_CON_RESTART_EN | DW_IC_CON_SPEED_FAST; > > dev->clk = devm_clk_get(&pdev->dev, NULL); > - dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz; > - if (IS_ERR(dev->clk)) > - return PTR_ERR(dev->clk); > - clk_prepare_enable(dev->clk); > - > - if (!dev->sda_hold_time && ht) { > - u32 ic_clk = dev->get_clk_rate_khz(dev); > - > - dev->sda_hold_time = div_u64((u64)ic_clk * ht + > 500000, > - 1000000); > + if (!IS_ERR(dev->clk)) { > + dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz; > + clk_prepare_enable(dev->clk); if (!i2c_dw_plat_prepare_clk(dev, true)) { dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz; … > + > + if (!dev->sda_hold_time && ht) > + dev->sda_hold_time = div_u64( > + (u64)dev->get_clk_rate_khz(dev) * ht > + 500000, > + 1000000); > } > > if (!dev->tx_fifo_depth) { > @@ -297,7 +295,8 @@ static int dw_i2c_plat_suspend(struct device > *dev) > struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev); > > i2c_dw_disable(i_dev); > - clk_disable_unprepare(i_dev->clk); > + if (!IS_ERR(i_dev->clk)) > + clk_disable_unprepare(i_dev->clk); i2c_dw_plat_prepare_clk(i_dev, false); > > return 0; > } > @@ -307,7 +306,8 @@ static int dw_i2c_plat_resume(struct device *dev) > struct platform_device *pdev = to_platform_device(dev); > struct dw_i2c_dev *i_dev = platform_get_drvdata(pdev); > > - clk_prepare_enable(i_dev->clk); > + if (!IS_ERR(i_dev->clk)) > + clk_prepare_enable(i_dev->clk); i2c_dw_plat_prepare_clk(i_dev, true); > > if (!i_dev->pm_runtime_disabled) > i2c_dw_init(i_dev); -- Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> Intel Finland Oy -- To unsubscribe from this list: send the line "unsubscribe linux-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
