diff mbox

i2c: designware: Do not require clock when SSCN and FFCN are provided

Message ID 1450219138-5868-1-git-send-email-Suravee.Suthikulpanit@amd.com
State New
Headers show

Commit Message

Suthikulpanit, Suravee Dec. 15, 2015, 10:38 p.m. UTC
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, this patch removes the clock requirement when SSCN and FFCN
are provided.

Signed-off-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>

---

Note: This has been tested on AMD Seattle RevB for both DT and ACPI.

 drivers/i2c/busses/i2c-designware-core.c    |  5 +++--
 drivers/i2c/busses/i2c-designware-platdrv.c | 30 +++++++++++++++++++++++------
 2 files changed, 27 insertions(+), 8 deletions(-)

-- 
2.5.0

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Suthikulpanit, Suravee Dec. 16, 2015, 2:44 p.m. UTC | #1
On 12/16/2015 08:28 AM, Mika Westerberg wrote:
> On Wed, Dec 16, 2015 at 08:11:12AM -0600, Suravee Suthikulpanit wrote:

>>> The clk framework should work fine if the returned clock is NULL (which

>>> I think is your case).

>>>

>>> The driver gates clocks when the device is suspended and on Intel LPSS

>>> there actually is a clock that gets gated.

>>>

>>>> [..]

>>>> @@ -203,13 +223,11 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)

>>>>   		dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |

>>>>   			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);

>>

>> Actually, if we don't provide the clock (which is the case for ACPI), this

>> would also return and cause the probing to fail.

>

> Indeed it seems that when you have CONFIG_COMMON_CLK selected the clock

> framework starts returning errors if the clock is not found.

>

> Since we need the clock for Intel LPSS I2C host controllers (and they

> may have *CNT methods), I think you just need to provide the clock for

> AMD I2C host controller in similar way than we do in

> drivers/acpi/acpi_lpss.c.


I am trying to avoid having to hard-coded clock frequency value in the 
driver. Would it be alright to not return w/ error, and just do the 
following?

	dev->clk = devm_clk_get(&pdev->dev, NULL);
	if (!IS_ERR(dev->clk))
		dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz;

This should work for the Intel case when clock is also provided.

Thanks,
Suravee
--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index 8c48b27..ec458ec 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -281,7 +281,7 @@  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 input_clock_khz = 0;
 	u32 hcnt, lcnt;
 	u32 reg;
 	u32 sda_falling_time, scl_falling_time;
@@ -295,7 +295,8 @@  int i2c_dw_init(struct dw_i2c_dev *dev)
 		}
 	}
 
-	input_clock_khz = dev->get_clk_rate_khz(dev);
+	if (dev->get_clk_rate_khz)
+		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)) {
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 809579e..57f623b 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -127,6 +127,26 @@  static inline int dw_i2c_acpi_configure(struct platform_device *pdev)
 }
 #endif
 
+static int dw_i2c_configure_clk(struct platform_device *pdev,
+				struct dw_i2c_dev *dev)
+{
+	/* For ACPI, if SSCN and FMCN is provided, we should not
+	 * need the clock value.
+	 */
+	if (has_acpi_companion(&pdev->dev) &&
+	    dev->ss_hcnt && dev->ss_lcnt &&
+	    dev->fs_hcnt && dev->fs_lcnt)
+		return 0;
+
+	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);
+
+	return 0;
+}
+
 static int dw_i2c_plat_probe(struct platform_device *pdev)
 {
 	struct dw_i2c_dev *dev;
@@ -203,13 +223,11 @@  static int dw_i2c_plat_probe(struct platform_device *pdev)
 		dev->master_cfg =  DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE |
 			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);
+	r = dw_i2c_configure_clk(pdev, dev);
+	if (r)
+		return r;
 
-	if (!dev->sda_hold_time && ht) {
+	if (!dev->sda_hold_time && ht && dev->get_clk_rate_khz) {
 		u32 ic_clk = dev->get_clk_rate_khz(dev);
 
 		dev->sda_hold_time = div_u64((u64)ic_clk * ht + 500000,