| Message ID | 1451920655-10798-1-git-send-email-Suravee.Suthikulpanit@amd.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Mon, Jan 04, 2016 at 09:17:35AM -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@linux.intel.com> > Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> > Tested-by: Loc Ho <lho@apm.com> > --- > > Note: This has been tested on AMD Seattle RevB for both DT and ACPI. > > Changes from V3 (https://lkml.org/lkml/2015/12/22/596): > * Add i2c_dw_plat_prepare_clk() per Andy's suggestion > * Add tested-by Loc Ho. The changes from V3 are big enough that I'd appreciate a new Tested-by tag.
Hi, >> 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@linux.intel.com> >> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> >> Tested-by: Loc Ho <lho@apm.com> >> --- >> >> Note: This has been tested on AMD Seattle RevB for both DT and ACPI. >> >> Changes from V3 (https://lkml.org/lkml/2015/12/22/596): >> * Add i2c_dw_plat_prepare_clk() per Andy's suggestion >> * Add tested-by Loc Ho. > > The changes from V3 are big enough that I'd appreciate a new Tested-by > tag. > Will let you know when I have moment to give this a try on X-Gene. -Loc -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi All, >> 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@linux.intel.com> >> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> >> Tested-by: Loc Ho <lho@apm.com> >> --- >> >> Note: This has been tested on AMD Seattle RevB for both DT and ACPI. >> >> Changes from V3 (https://lkml.org/lkml/2015/12/22/596): >> * Add i2c_dw_plat_prepare_clk() per Andy's suggestion >> * Add tested-by Loc Ho. > > The changes from V3 are big enough that I'd appreciate a new Tested-by > tag. I had tested this via this mixes of test cases: a. NO APD driver + this patch ==> HCNT/LCNT as expected b. with APD driver + this patch ==> HCNT/LCNT as expected c. with APD driver + this patch + double the frequency APD driver ==> HCNT/LCNT as expected d. with APD driver + this patch + double the frequency APD driver + comment out the ACPI parameter retrieve ==> HCNT/LCNT changed as expected Therefore, you can add my - Tested-by: Loc Ho <lho@apm.com> -Loc -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On 01/06/2016 06:31 PM, Loc Ho wrote: > Hi All, > >>> 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@linux.intel.com> >>> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> >>> Tested-by: Loc Ho <lho@apm.com> >>> --- >>> >>> Note: This has been tested on AMD Seattle RevB for both DT and ACPI. >>> >>> Changes from V3 (https://lkml.org/lkml/2015/12/22/596): >>> * Add i2c_dw_plat_prepare_clk() per Andy's suggestion >>> * Add tested-by Loc Ho. >> >> The changes from V3 are big enough that I'd appreciate a new Tested-by >> tag. > > I had tested this via this mixes of test cases: > > a. NO APD driver + this patch ==> HCNT/LCNT as expected > b. with APD driver + this patch ==> HCNT/LCNT as expected > c. with APD driver + this patch + double the frequency APD driver ==> > HCNT/LCNT as expected > d. with APD driver + this patch + double the frequency APD driver + > comment out the ACPI parameter retrieve ==> HCNT/LCNT changed as > expected > > Therefore, you can add my - Tested-by: Loc Ho <lho@apm.com> > > -Loc > Thanks Loc, Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 04, 2016 at 09:17:35AM -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@linux.intel.com> > Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> > Tested-by: Loc Ho <lho@apm.com> Applied to for-next, thanks! Also thanks to all reviewers and testers.
On 1/10/2016 2:38 AM, Wolfram Sang wrote: > On Mon, Jan 04, 2016 at 09:17:35AM -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@linux.intel.com> >> Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> >> Tested-by: Loc Ho <lho@apm.com> > > Applied to for-next, thanks! Also thanks to all reviewers and testers. > Thank you, Suravee -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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..965512c 100644 --- a/drivers/i2c/busses/i2c-designware-platdrv.c +++ b/drivers/i2c/busses/i2c-designware-platdrv.c @@ -128,6 +128,18 @@ static inline int dw_i2c_acpi_configure(struct platform_device *pdev) } #endif +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) + return clk_prepare_enable(i_dev->clk); + + clk_disable_unprepare(i_dev->clk); + return 0; +} + static int dw_i2c_plat_probe(struct platform_device *pdev) { struct dw_i2c_dev *dev; @@ -205,16 +217,13 @@ 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 (!i2c_dw_plat_prepare_clk(dev, true)) { + dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz; - 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 (!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 +306,7 @@ 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); + i2c_dw_plat_prepare_clk(i_dev, false); return 0; } @@ -307,7 +316,7 @@ 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); + i2c_dw_plat_prepare_clk(i_dev, true); if (!i_dev->pm_runtime_disabled) i2c_dw_init(i_dev);