linux-i2c.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 1/7] i2c: designware: Get rid of PCI driver specifics in common code
@ 2020-05-19 12:50 Andy Shevchenko
  2020-05-19 12:50 ` [PATCH v2 2/7] i2c: designware: Include proper headers in i2c-desingware-core.h Andy Shevchenko
                   ` (7 more replies)
  0 siblings, 8 replies; 11+ messages in thread
From: Andy Shevchenko @ 2020-05-19 12:50 UTC (permalink / raw)
  To: Jarkko Nikula, Mika Westerberg, linux-i2c, Wolfram Sang; +Cc: Andy Shevchenko

Do not spread PCI specifics over common code. It seems to be a layering
violation which can be easily avoided. Refactor PCI driver and drop
PCI specifics from common code.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: new patch
 drivers/i2c/busses/i2c-designware-core.h   |  1 -
 drivers/i2c/busses/i2c-designware-pcidrv.c | 24 +++++++++++++---------
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 1674caf27745..6202f9ee953d 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -232,7 +232,6 @@ struct dw_i2c_dev {
 	struct reset_control	*rst;
 	struct i2c_client		*slave;
 	u32			(*get_clk_rate_khz) (struct dw_i2c_dev *dev);
-	struct dw_pci_controller *controller;
 	int			cmd_err;
 	struct i2c_msg		*msgs;
 	int			msgs_num;
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index c762e5a11e44..c8808e5855b4 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -48,10 +48,10 @@ struct dw_pci_controller {
 	u32 bus_num;
 	u32 tx_fifo_depth;
 	u32 rx_fifo_depth;
-	u32 clk_khz;
 	u32 flags;
 	struct dw_scl_sda_cfg *scl_sda_cfg;
 	int (*setup)(struct pci_dev *pdev, struct dw_pci_controller *c);
+	u32 (*get_clk_rate_khz)(struct dw_i2c_dev *dev);
 };
 
 /* Merrifield HCNT/LCNT/SDA hold time */
@@ -80,6 +80,11 @@ static struct dw_scl_sda_cfg hsw_config = {
 	.sda_hold = 0x9,
 };
 
+static u32 mfld_get_clk_rate_khz(struct dw_i2c_dev *dev)
+{
+	return 25000;
+}
+
 static int mfld_setup(struct pci_dev *pdev, struct dw_pci_controller *c)
 {
 	struct dw_i2c_dev *dev = dev_get_drvdata(&pdev->dev);
@@ -120,13 +125,18 @@ static int mrfld_setup(struct pci_dev *pdev, struct dw_pci_controller *c)
 	return -ENODEV;
 }
 
+static u32 ehl_get_clk_rate_khz(struct dw_i2c_dev *dev)
+{
+	return 100000;
+}
+
 static struct dw_pci_controller dw_pci_controllers[] = {
 	[medfield] = {
 		.bus_num = -1,
 		.tx_fifo_depth = 32,
 		.rx_fifo_depth = 32,
-		.clk_khz      = 25000,
 		.setup = mfld_setup,
+		.get_clk_rate_khz = mfld_get_clk_rate_khz,
 	},
 	[merrifield] = {
 		.bus_num = -1,
@@ -158,7 +168,7 @@ static struct dw_pci_controller dw_pci_controllers[] = {
 		.bus_num = -1,
 		.tx_fifo_depth = 32,
 		.rx_fifo_depth = 32,
-		.clk_khz = 100000,
+		.get_clk_rate_khz = ehl_get_clk_rate_khz,
 	},
 };
 
@@ -188,11 +198,6 @@ static int i2c_dw_pci_resume(struct device *dev)
 static UNIVERSAL_DEV_PM_OPS(i2c_dw_pm_ops, i2c_dw_pci_suspend,
 			    i2c_dw_pci_resume, NULL);
 
-static u32 i2c_dw_get_clk_rate_khz(struct dw_i2c_dev *dev)
-{
-	return dev->controller->clk_khz;
-}
-
 static int i2c_dw_pci_probe(struct pci_dev *pdev,
 			    const struct pci_device_id *id)
 {
@@ -233,8 +238,7 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 	if (r < 0)
 		return r;
 
-	dev->controller = controller;
-	dev->get_clk_rate_khz = i2c_dw_get_clk_rate_khz;
+	dev->get_clk_rate_khz = controller->get_clk_rate_khz;
 	dev->timings.bus_freq_hz = I2C_MAX_FAST_MODE_FREQ;
 	dev->base = pcim_iomap_table(pdev)[0];
 	dev->dev = &pdev->dev;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 2/7] i2c: designware: Include proper headers in i2c-desingware-core.h
  2020-05-19 12:50 [PATCH v2 1/7] i2c: designware: Get rid of PCI driver specifics in common code Andy Shevchenko
@ 2020-05-19 12:50 ` Andy Shevchenko
  2020-05-19 12:50 ` [PATCH v2 3/7] i2c: designware: Move i2c_dw_validate_speed() helper to a common code Andy Shevchenko
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2020-05-19 12:50 UTC (permalink / raw)
  To: Jarkko Nikula, Mika Westerberg, linux-i2c, Wolfram Sang; +Cc: Andy Shevchenko

This header is a user of some generic ones, include them respectively.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: new patch
 drivers/i2c/busses/i2c-designware-core.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 6202f9ee953d..94d9ad15133a 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -9,7 +9,13 @@
  * Copyright (C) 2009 Provigent Ltd.
  */
 
+#include <linux/bits.h>
+#include <linux/compiler_types.h>
+#include <linux/completion.h>
+#include <linux/dev_printk.h>
+#include <linux/errno.h>
 #include <linux/i2c.h>
+#include <linux/types.h>
 
 #define DW_IC_DEFAULT_FUNCTIONALITY (I2C_FUNC_I2C |			\
 					I2C_FUNC_SMBUS_BYTE |		\
@@ -170,6 +176,9 @@
 					 DW_IC_TX_ABRT_TXDATA_NOACK | \
 					 DW_IC_TX_ABRT_GCALL_NOACK)
 
+struct clk;
+struct device;
+struct reset_control;
 
 /**
  * struct dw_i2c_dev - private i2c-designware data
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 3/7] i2c: designware: Move i2c_dw_validate_speed() helper to a common code
  2020-05-19 12:50 [PATCH v2 1/7] i2c: designware: Get rid of PCI driver specifics in common code Andy Shevchenko
  2020-05-19 12:50 ` [PATCH v2 2/7] i2c: designware: Include proper headers in i2c-desingware-core.h Andy Shevchenko
@ 2020-05-19 12:50 ` Andy Shevchenko
  2020-05-19 12:50 ` [PATCH v2 4/7] i2c: designware: Drop unneeded condition in i2c_dw_validate_speed() Andy Shevchenko
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2020-05-19 12:50 UTC (permalink / raw)
  To: Jarkko Nikula, Mika Westerberg, linux-i2c, Wolfram Sang; +Cc: Andy Shevchenko

In order to export array supported speed for wider use, move it
to a header along with i2c_dw_validate_speed() helper moved to
a common code.

No functional changes intended.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: move it to common code instead of being separate (Jarkko)
 drivers/i2c/busses/i2c-designware-common.c  | 24 +++++++++++++++++
 drivers/i2c/busses/i2c-designware-core.h    |  9 +++++++
 drivers/i2c/busses/i2c-designware-platdrv.c | 29 ++++-----------------
 3 files changed, 38 insertions(+), 24 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index c70c6fc09ee3..9f06567be54a 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -116,6 +116,30 @@ int i2c_dw_set_reg_access(struct dw_i2c_dev *dev)
 	return 0;
 }
 
+int i2c_dw_validate_speed(struct dw_i2c_dev *dev)
+{
+	struct i2c_timings *t = &dev->timings;
+	unsigned int i;
+
+	/*
+	 * Only standard mode at 100kHz, fast mode at 400kHz,
+	 * fast mode plus at 1MHz and high speed mode at 3.4MHz are supported.
+	 */
+	for (i = 0; i < ARRAY_SIZE(i2c_dw_supported_speeds); i++) {
+		if (t->bus_freq_hz == i2c_dw_supported_speeds[i])
+			break;
+	}
+	if (i == ARRAY_SIZE(i2c_dw_supported_speeds)) {
+		dev_err(dev->dev,
+			"%d Hz is unsupported, only 100kHz, 400kHz, 1MHz and 3.4MHz are supported\n",
+			t->bus_freq_hz);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(i2c_dw_validate_speed);
+
 u32 i2c_dw_scl_hcnt(u32 ic_clk, u32 tSYMBOL, u32 tf, int cond, int offset)
 {
 	/*
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 94d9ad15133a..77c8aa422268 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -359,3 +359,12 @@ extern int i2c_dw_probe_lock_support(struct dw_i2c_dev *dev);
 #else
 static inline int i2c_dw_probe_lock_support(struct dw_i2c_dev *dev) { return 0; }
 #endif
+
+static __maybe_unused const u32 i2c_dw_supported_speeds[] = {
+	I2C_MAX_HIGH_SPEED_MODE_FREQ,
+	I2C_MAX_FAST_MODE_PLUS_FREQ,
+	I2C_MAX_FAST_MODE_FREQ,
+	I2C_MAX_STANDARD_MODE_FREQ,
+};
+
+int i2c_dw_validate_speed(struct dw_i2c_dev *dev);
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 7e228ff91ad4..a502750c89b7 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -192,13 +192,6 @@ static void dw_i2c_plat_pm_cleanup(struct dw_i2c_dev *dev)
 		pm_runtime_put_noidle(dev->dev);
 }
 
-static const u32 supported_speeds[] = {
-	I2C_MAX_HIGH_SPEED_MODE_FREQ,
-	I2C_MAX_FAST_MODE_PLUS_FREQ,
-	I2C_MAX_FAST_MODE_FREQ,
-	I2C_MAX_STANDARD_MODE_FREQ,
-};
-
 static int dw_i2c_plat_probe(struct platform_device *pdev)
 {
 	struct dw_i2c_platform_data *pdata = dev_get_platdata(&pdev->dev);
@@ -241,11 +234,11 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 	 * Some DSTDs use a non standard speed, round down to the lowest
 	 * standard speed.
 	 */
-	for (i = 0; i < ARRAY_SIZE(supported_speeds); i++) {
-		if (acpi_speed >= supported_speeds[i])
+	for (i = 0; i < ARRAY_SIZE(i2c_dw_supported_speeds); i++) {
+		if (acpi_speed >= i2c_dw_supported_speeds[i])
 			break;
 	}
-	acpi_speed = i < ARRAY_SIZE(supported_speeds) ? supported_speeds[i] : 0;
+	acpi_speed = i < ARRAY_SIZE(i2c_dw_supported_speeds) ? i2c_dw_supported_speeds[i] : 0;
 
 	/*
 	 * Find bus speed from the "clock-frequency" device property, ACPI
@@ -266,21 +259,9 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 	if (has_acpi_companion(&pdev->dev))
 		dw_i2c_acpi_configure(pdev);
 
-	/*
-	 * Only standard mode at 100kHz, fast mode at 400kHz,
-	 * fast mode plus at 1MHz and high speed mode at 3.4MHz are supported.
-	 */
-	for (i = 0; i < ARRAY_SIZE(supported_speeds); i++) {
-		if (t->bus_freq_hz == supported_speeds[i])
-			break;
-	}
-	if (i == ARRAY_SIZE(supported_speeds)) {
-		dev_err(&pdev->dev,
-			"%d Hz is unsupported, only 100kHz, 400kHz, 1MHz and 3.4MHz are supported\n",
-			t->bus_freq_hz);
-		ret = -EINVAL;
+	ret = i2c_dw_validate_speed(dev);
+	if (ret)
 		goto exit_reset;
-	}
 
 	ret = i2c_dw_probe_lock_support(dev);
 	if (ret)
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 4/7] i2c: designware: Drop unneeded condition in i2c_dw_validate_speed()
  2020-05-19 12:50 [PATCH v2 1/7] i2c: designware: Get rid of PCI driver specifics in common code Andy Shevchenko
  2020-05-19 12:50 ` [PATCH v2 2/7] i2c: designware: Include proper headers in i2c-desingware-core.h Andy Shevchenko
  2020-05-19 12:50 ` [PATCH v2 3/7] i2c: designware: Move i2c_dw_validate_speed() helper to a common code Andy Shevchenko
@ 2020-05-19 12:50 ` Andy Shevchenko
  2020-05-19 12:50 ` [PATCH v2 5/7] i2c: designware: Move ACPI parts into common module Andy Shevchenko
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2020-05-19 12:50 UTC (permalink / raw)
  To: Jarkko Nikula, Mika Westerberg, linux-i2c, Wolfram Sang; +Cc: Andy Shevchenko

We may bailout directly from the loop instead of breaking it and
testing a loop counter. This also gives advantages such as decreased
indentation level along with dropped unneeded condition.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: new patch
 drivers/i2c/busses/i2c-designware-common.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 9f06567be54a..2fd5372b1237 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -127,16 +127,14 @@ int i2c_dw_validate_speed(struct dw_i2c_dev *dev)
 	 */
 	for (i = 0; i < ARRAY_SIZE(i2c_dw_supported_speeds); i++) {
 		if (t->bus_freq_hz == i2c_dw_supported_speeds[i])
-			break;
-	}
-	if (i == ARRAY_SIZE(i2c_dw_supported_speeds)) {
-		dev_err(dev->dev,
-			"%d Hz is unsupported, only 100kHz, 400kHz, 1MHz and 3.4MHz are supported\n",
-			t->bus_freq_hz);
-		return -EINVAL;
+			return 0;
 	}
 
-	return 0;
+	dev_err(dev->dev,
+		"%d Hz is unsupported, only 100kHz, 400kHz, 1MHz and 3.4MHz are supported\n",
+		t->bus_freq_hz);
+
+	return -EINVAL;
 }
 EXPORT_SYMBOL_GPL(i2c_dw_validate_speed);
 
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 5/7] i2c: designware: Move ACPI parts into common module
  2020-05-19 12:50 [PATCH v2 1/7] i2c: designware: Get rid of PCI driver specifics in common code Andy Shevchenko
                   ` (2 preceding siblings ...)
  2020-05-19 12:50 ` [PATCH v2 4/7] i2c: designware: Drop unneeded condition in i2c_dw_validate_speed() Andy Shevchenko
@ 2020-05-19 12:50 ` Andy Shevchenko
  2020-05-19 12:50 ` [PATCH v2 6/7] i2c: designware: Read counters from ACPI for PCI driver Andy Shevchenko
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2020-05-19 12:50 UTC (permalink / raw)
  To: Jarkko Nikula, Mika Westerberg, linux-i2c, Wolfram Sang; +Cc: Andy Shevchenko

For possible code reuse in the future, move ACPI parts into common module.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: moved to a common code instead of being a separate (Jarkko)
 drivers/i2c/busses/i2c-designware-common.c  | 135 +++++++++++++++++++-
 drivers/i2c/busses/i2c-designware-core.h    |  15 ++-
 drivers/i2c/busses/i2c-designware-platdrv.c | 111 +---------------
 3 files changed, 142 insertions(+), 119 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index 2fd5372b1237..e1697ed8b54a 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -8,17 +8,21 @@
  * Copyright (C) 2007 MontaVista Software Inc.
  * Copyright (C) 2009 Provigent Ltd.
  */
+#include <linux/acpi.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
-#include <linux/export.h>
-#include <linux/errno.h>
+#include <linux/device.h>
 #include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/export.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
+#include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
 #include <linux/swab.h>
+#include <linux/types.h>
 
 #include "i2c-designware-core.h"
 
@@ -116,6 +120,13 @@ int i2c_dw_set_reg_access(struct dw_i2c_dev *dev)
 	return 0;
 }
 
+static const u32 supported_speeds[] = {
+	I2C_MAX_HIGH_SPEED_MODE_FREQ,
+	I2C_MAX_FAST_MODE_PLUS_FREQ,
+	I2C_MAX_FAST_MODE_FREQ,
+	I2C_MAX_STANDARD_MODE_FREQ,
+};
+
 int i2c_dw_validate_speed(struct dw_i2c_dev *dev)
 {
 	struct i2c_timings *t = &dev->timings;
@@ -125,8 +136,8 @@ int i2c_dw_validate_speed(struct dw_i2c_dev *dev)
 	 * Only standard mode at 100kHz, fast mode at 400kHz,
 	 * fast mode plus at 1MHz and high speed mode at 3.4MHz are supported.
 	 */
-	for (i = 0; i < ARRAY_SIZE(i2c_dw_supported_speeds); i++) {
-		if (t->bus_freq_hz == i2c_dw_supported_speeds[i])
+	for (i = 0; i < ARRAY_SIZE(supported_speeds); i++) {
+		if (t->bus_freq_hz == supported_speeds[i])
 			return 0;
 	}
 
@@ -138,6 +149,122 @@ int i2c_dw_validate_speed(struct dw_i2c_dev *dev)
 }
 EXPORT_SYMBOL_GPL(i2c_dw_validate_speed);
 
+#ifdef CONFIG_ACPI
+
+#include <linux/dmi.h>
+
+/*
+ * The HCNT/LCNT information coming from ACPI should be the most accurate
+ * for given platform. However, some systems get it wrong. On such systems
+ * we get better results by calculating those based on the input clock.
+ */
+static const struct dmi_system_id i2c_dw_no_acpi_params[] = {
+	{
+		.ident = "Dell Inspiron 7348",
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 7348"),
+		},
+	},
+	{}
+};
+
+static void i2c_dw_acpi_params(struct device *device, char method[],
+			       u16 *hcnt, u16 *lcnt, u32 *sda_hold)
+{
+	struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER };
+	acpi_handle handle = ACPI_HANDLE(device);
+	union acpi_object *obj;
+
+	if (dmi_check_system(i2c_dw_no_acpi_params))
+		return;
+
+	if (ACPI_FAILURE(acpi_evaluate_object(handle, method, NULL, &buf)))
+		return;
+
+	obj = (union acpi_object *)buf.pointer;
+	if (obj->type == ACPI_TYPE_PACKAGE && obj->package.count == 3) {
+		const union acpi_object *objs = obj->package.elements;
+
+		*hcnt = (u16)objs[0].integer.value;
+		*lcnt = (u16)objs[1].integer.value;
+		*sda_hold = (u32)objs[2].integer.value;
+	}
+
+	kfree(buf.pointer);
+}
+
+int i2c_dw_acpi_configure(struct device *device)
+{
+	struct dw_i2c_dev *dev = dev_get_drvdata(device);
+	struct i2c_timings *t = &dev->timings;
+	u32 ss_ht = 0, fp_ht = 0, hs_ht = 0, fs_ht = 0;
+
+	dev->tx_fifo_depth = 32;
+	dev->rx_fifo_depth = 32;
+
+	/*
+	 * Try to get SDA hold time and *CNT values from an ACPI method for
+	 * selected speed modes.
+	 */
+	i2c_dw_acpi_params(device, "SSCN", &dev->ss_hcnt, &dev->ss_lcnt, &ss_ht);
+	i2c_dw_acpi_params(device, "FPCN", &dev->fp_hcnt, &dev->fp_lcnt, &fp_ht);
+	i2c_dw_acpi_params(device, "HSCN", &dev->hs_hcnt, &dev->hs_lcnt, &hs_ht);
+	i2c_dw_acpi_params(device, "FMCN", &dev->fs_hcnt, &dev->fs_lcnt, &fs_ht);
+
+	switch (t->bus_freq_hz) {
+	case I2C_MAX_STANDARD_MODE_FREQ:
+		dev->sda_hold_time = ss_ht;
+		break;
+	case I2C_MAX_FAST_MODE_PLUS_FREQ:
+		dev->sda_hold_time = fp_ht;
+		break;
+	case I2C_MAX_HIGH_SPEED_MODE_FREQ:
+		dev->sda_hold_time = hs_ht;
+		break;
+	case I2C_MAX_FAST_MODE_FREQ:
+	default:
+		dev->sda_hold_time = fs_ht;
+		break;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(i2c_dw_acpi_configure);
+
+void i2c_dw_acpi_adjust_bus_speed(struct device *device)
+{
+	struct dw_i2c_dev *dev = dev_get_drvdata(device);
+	struct i2c_timings *t = &dev->timings;
+	u32 acpi_speed;
+	int i;
+
+	acpi_speed = i2c_acpi_find_bus_speed(device);
+	/*
+	 * Some DSTDs use a non standard speed, round down to the lowest
+	 * standard speed.
+	 */
+	for (i = 0; i < ARRAY_SIZE(supported_speeds); i++) {
+		if (acpi_speed >= supported_speeds[i])
+			break;
+	}
+	acpi_speed = i < ARRAY_SIZE(supported_speeds) ? supported_speeds[i] : 0;
+
+	/*
+	 * Find bus speed from the "clock-frequency" device property, ACPI
+	 * or by using fast mode if neither is set.
+	 */
+	if (acpi_speed && t->bus_freq_hz)
+		t->bus_freq_hz = min(t->bus_freq_hz, acpi_speed);
+	else if (acpi_speed || t->bus_freq_hz)
+		t->bus_freq_hz = max(t->bus_freq_hz, acpi_speed);
+	else
+		t->bus_freq_hz = I2C_MAX_FAST_MODE_FREQ;
+}
+EXPORT_SYMBOL_GPL(i2c_dw_acpi_adjust_bus_speed);
+
+#endif	/* CONFIG_ACPI */
+
 u32 i2c_dw_scl_hcnt(u32 ic_clk, u32 tSYMBOL, u32 tf, int cond, int offset)
 {
 	/*
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 77c8aa422268..150de5e5c31b 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -360,11 +360,12 @@ extern int i2c_dw_probe_lock_support(struct dw_i2c_dev *dev);
 static inline int i2c_dw_probe_lock_support(struct dw_i2c_dev *dev) { return 0; }
 #endif
 
-static __maybe_unused const u32 i2c_dw_supported_speeds[] = {
-	I2C_MAX_HIGH_SPEED_MODE_FREQ,
-	I2C_MAX_FAST_MODE_PLUS_FREQ,
-	I2C_MAX_FAST_MODE_FREQ,
-	I2C_MAX_STANDARD_MODE_FREQ,
-};
-
 int i2c_dw_validate_speed(struct dw_i2c_dev *dev);
+
+#if IS_ENABLED(CONFIG_ACPI)
+int i2c_dw_acpi_configure(struct device *device);
+void i2c_dw_acpi_adjust_bus_speed(struct device *device);
+#else
+static inline int i2c_dw_acpi_configure(struct device *device) { return -ENODEV; }
+static inline void i2c_dw_acpi_adjust_bus_speed(struct device *device) {}
+#endif
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index a502750c89b7..9be9118f7a8d 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -12,7 +12,6 @@
 #include <linux/clk-provider.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
-#include <linux/dmi.h>
 #include <linux/err.h>
 #include <linux/errno.h>
 #include <linux/i2c.h>
@@ -39,84 +38,6 @@ static u32 i2c_dw_get_clk_rate_khz(struct dw_i2c_dev *dev)
 }
 
 #ifdef CONFIG_ACPI
-/*
- * The HCNT/LCNT information coming from ACPI should be the most accurate
- * for given platform. However, some systems get it wrong. On such systems
- * we get better results by calculating those based on the input clock.
- */
-static const struct dmi_system_id dw_i2c_no_acpi_params[] = {
-	{
-		.ident = "Dell Inspiron 7348",
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "Inspiron 7348"),
-		},
-	},
-	{ }
-};
-
-static void dw_i2c_acpi_params(struct platform_device *pdev, char method[],
-			       u16 *hcnt, u16 *lcnt, u32 *sda_hold)
-{
-	struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER };
-	acpi_handle handle = ACPI_HANDLE(&pdev->dev);
-	union acpi_object *obj;
-
-	if (dmi_check_system(dw_i2c_no_acpi_params))
-		return;
-
-	if (ACPI_FAILURE(acpi_evaluate_object(handle, method, NULL, &buf)))
-		return;
-
-	obj = (union acpi_object *)buf.pointer;
-	if (obj->type == ACPI_TYPE_PACKAGE && obj->package.count == 3) {
-		const union acpi_object *objs = obj->package.elements;
-
-		*hcnt = (u16)objs[0].integer.value;
-		*lcnt = (u16)objs[1].integer.value;
-		*sda_hold = (u32)objs[2].integer.value;
-	}
-
-	kfree(buf.pointer);
-}
-
-static int dw_i2c_acpi_configure(struct platform_device *pdev)
-{
-	struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
-	struct i2c_timings *t = &dev->timings;
-	u32 ss_ht = 0, fp_ht = 0, hs_ht = 0, fs_ht = 0;
-
-	dev->tx_fifo_depth = 32;
-	dev->rx_fifo_depth = 32;
-
-	/*
-	 * Try to get SDA hold time and *CNT values from an ACPI method for
-	 * selected speed modes.
-	 */
-	dw_i2c_acpi_params(pdev, "SSCN", &dev->ss_hcnt, &dev->ss_lcnt, &ss_ht);
-	dw_i2c_acpi_params(pdev, "FPCN", &dev->fp_hcnt, &dev->fp_lcnt, &fp_ht);
-	dw_i2c_acpi_params(pdev, "HSCN", &dev->hs_hcnt, &dev->hs_lcnt, &hs_ht);
-	dw_i2c_acpi_params(pdev, "FMCN", &dev->fs_hcnt, &dev->fs_lcnt, &fs_ht);
-
-	switch (t->bus_freq_hz) {
-	case I2C_MAX_STANDARD_MODE_FREQ:
-		dev->sda_hold_time = ss_ht;
-		break;
-	case I2C_MAX_FAST_MODE_PLUS_FREQ:
-		dev->sda_hold_time = fp_ht;
-		break;
-	case I2C_MAX_HIGH_SPEED_MODE_FREQ:
-		dev->sda_hold_time = hs_ht;
-		break;
-	case I2C_MAX_FAST_MODE_FREQ:
-	default:
-		dev->sda_hold_time = fs_ht;
-		break;
-	}
-
-	return 0;
-}
-
 static const struct acpi_device_id dw_i2c_acpi_match[] = {
 	{ "INT33C2", 0 },
 	{ "INT33C3", 0 },
@@ -134,11 +55,6 @@ static const struct acpi_device_id dw_i2c_acpi_match[] = {
 	{ }
 };
 MODULE_DEVICE_TABLE(acpi, dw_i2c_acpi_match);
-#else
-static inline int dw_i2c_acpi_configure(struct platform_device *pdev)
-{
-	return -ENODEV;
-}
 #endif
 
 #ifdef CONFIG_OF
@@ -198,8 +114,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 	struct i2c_adapter *adap;
 	struct dw_i2c_dev *dev;
 	struct i2c_timings *t;
-	u32 acpi_speed;
-	int i, irq, ret;
+	int irq, ret;
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
@@ -229,27 +144,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 	else
 		i2c_parse_fw_timings(&pdev->dev, t, false);
 
-	acpi_speed = i2c_acpi_find_bus_speed(&pdev->dev);
-	/*
-	 * Some DSTDs use a non standard speed, round down to the lowest
-	 * standard speed.
-	 */
-	for (i = 0; i < ARRAY_SIZE(i2c_dw_supported_speeds); i++) {
-		if (acpi_speed >= i2c_dw_supported_speeds[i])
-			break;
-	}
-	acpi_speed = i < ARRAY_SIZE(i2c_dw_supported_speeds) ? i2c_dw_supported_speeds[i] : 0;
-
-	/*
-	 * Find bus speed from the "clock-frequency" device property, ACPI
-	 * or by using fast mode if neither is set.
-	 */
-	if (acpi_speed && t->bus_freq_hz)
-		t->bus_freq_hz = min(t->bus_freq_hz, acpi_speed);
-	else if (acpi_speed || t->bus_freq_hz)
-		t->bus_freq_hz = max(t->bus_freq_hz, acpi_speed);
-	else
-		t->bus_freq_hz = I2C_MAX_FAST_MODE_FREQ;
+	i2c_dw_acpi_adjust_bus_speed(&pdev->dev);
 
 	dev->flags |= (uintptr_t)device_get_match_data(&pdev->dev);
 
@@ -257,7 +152,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 		dw_i2c_of_configure(pdev);
 
 	if (has_acpi_companion(&pdev->dev))
-		dw_i2c_acpi_configure(pdev);
+		i2c_dw_acpi_configure(&pdev->dev);
 
 	ret = i2c_dw_validate_speed(dev);
 	if (ret)
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 6/7] i2c: designware: Read counters from ACPI for PCI driver
  2020-05-19 12:50 [PATCH v2 1/7] i2c: designware: Get rid of PCI driver specifics in common code Andy Shevchenko
                   ` (3 preceding siblings ...)
  2020-05-19 12:50 ` [PATCH v2 5/7] i2c: designware: Move ACPI parts into common module Andy Shevchenko
@ 2020-05-19 12:50 ` Andy Shevchenko
  2020-05-19 12:50 ` [PATCH v2 7/7] i2c: designware: Drop hard coded FIFO depth assignment Andy Shevchenko
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2020-05-19 12:50 UTC (permalink / raw)
  To: Jarkko Nikula, Mika Westerberg, linux-i2c, Wolfram Sang; +Cc: Andy Shevchenko

PCI devices may have been backed with ACPI handle which supplies
an additional information to the drivers, such as counters.

Call for ACPI configuration from PCI driver in order to utilize counters
provided by ACPI.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: also call i2c_dw_acpi_adjust_bus_speed()
 drivers/i2c/busses/i2c-designware-pcidrv.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index c8808e5855b4..3664d76bb976 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -255,6 +255,17 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 		}
 	}
 
+	i2c_dw_acpi_adjust_bus_speed(&pdev->dev);
+
+	if (has_acpi_companion(&pdev->dev))
+		i2c_dw_acpi_configure(&pdev->dev);
+
+	r = i2c_dw_validate_speed(dev);
+	if (r) {
+		pci_free_irq_vectors(pdev);
+		return r;
+	}
+
 	i2c_dw_configure(dev);
 
 	if (controller->scl_sda_cfg) {
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 7/7] i2c: designware: Drop hard coded FIFO depth assignment
  2020-05-19 12:50 [PATCH v2 1/7] i2c: designware: Get rid of PCI driver specifics in common code Andy Shevchenko
                   ` (4 preceding siblings ...)
  2020-05-19 12:50 ` [PATCH v2 6/7] i2c: designware: Read counters from ACPI for PCI driver Andy Shevchenko
@ 2020-05-19 12:50 ` Andy Shevchenko
  2020-05-20 13:55 ` [PATCH v2 1/7] i2c: designware: Get rid of PCI driver specifics in common code Jarkko Nikula
  2020-05-22 14:55 ` Wolfram Sang
  7 siblings, 0 replies; 11+ messages in thread
From: Andy Shevchenko @ 2020-05-19 12:50 UTC (permalink / raw)
  To: Jarkko Nikula, Mika Westerberg, linux-i2c, Wolfram Sang; +Cc: Andy Shevchenko

It's not clear why the commit fe20ff5c7e9c
  ("i2c-designware: Add support for Designware core behind PCI devices.")
followed by commit b61b14154b19
  ("i2c-designware: add support for Intel Lynxpoint")
chose to hard code FIFO depth size. The FIFO depth on all hardware,
I have tested on, can be nicely detected automatically.

Thus, we may safely drop hard coded FIFO sizes from the driver.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
v2: new patch
 drivers/i2c/busses/i2c-designware-common.c |  3 ---
 drivers/i2c/busses/i2c-designware-pcidrv.c | 17 -----------------
 2 files changed, 20 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-common.c b/drivers/i2c/busses/i2c-designware-common.c
index e1697ed8b54a..ed302342f8db 100644
--- a/drivers/i2c/busses/i2c-designware-common.c
+++ b/drivers/i2c/busses/i2c-designware-common.c
@@ -200,9 +200,6 @@ int i2c_dw_acpi_configure(struct device *device)
 	struct i2c_timings *t = &dev->timings;
 	u32 ss_ht = 0, fp_ht = 0, hs_ht = 0, fs_ht = 0;
 
-	dev->tx_fifo_depth = 32;
-	dev->rx_fifo_depth = 32;
-
 	/*
 	 * Try to get SDA hold time and *CNT values from an ACPI method for
 	 * selected speed modes.
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 3664d76bb976..11a5e4751eab 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -46,8 +46,6 @@ struct dw_scl_sda_cfg {
 
 struct dw_pci_controller {
 	u32 bus_num;
-	u32 tx_fifo_depth;
-	u32 rx_fifo_depth;
 	u32 flags;
 	struct dw_scl_sda_cfg *scl_sda_cfg;
 	int (*setup)(struct pci_dev *pdev, struct dw_pci_controller *c);
@@ -133,41 +131,29 @@ static u32 ehl_get_clk_rate_khz(struct dw_i2c_dev *dev)
 static struct dw_pci_controller dw_pci_controllers[] = {
 	[medfield] = {
 		.bus_num = -1,
-		.tx_fifo_depth = 32,
-		.rx_fifo_depth = 32,
 		.setup = mfld_setup,
 		.get_clk_rate_khz = mfld_get_clk_rate_khz,
 	},
 	[merrifield] = {
 		.bus_num = -1,
-		.tx_fifo_depth = 64,
-		.rx_fifo_depth = 64,
 		.scl_sda_cfg = &mrfld_config,
 		.setup = mrfld_setup,
 	},
 	[baytrail] = {
 		.bus_num = -1,
-		.tx_fifo_depth = 32,
-		.rx_fifo_depth = 32,
 		.scl_sda_cfg = &byt_config,
 	},
 	[haswell] = {
 		.bus_num = -1,
-		.tx_fifo_depth = 32,
-		.rx_fifo_depth = 32,
 		.scl_sda_cfg = &hsw_config,
 	},
 	[cherrytrail] = {
 		.bus_num = -1,
-		.tx_fifo_depth = 32,
-		.rx_fifo_depth = 32,
 		.flags = MODEL_CHERRYTRAIL,
 		.scl_sda_cfg = &byt_config,
 	},
 	[elkhartlake] = {
 		.bus_num = -1,
-		.tx_fifo_depth = 32,
-		.rx_fifo_depth = 32,
 		.get_clk_rate_khz = ehl_get_clk_rate_khz,
 	},
 };
@@ -277,9 +263,6 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 		dev->sda_hold_time = cfg->sda_hold;
 	}
 
-	dev->tx_fifo_depth = controller->tx_fifo_depth;
-	dev->rx_fifo_depth = controller->rx_fifo_depth;
-
 	adap = &dev->adapter;
 	adap->owner = THIS_MODULE;
 	adap->class = 0;
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/7] i2c: designware: Get rid of PCI driver specifics in common code
  2020-05-19 12:50 [PATCH v2 1/7] i2c: designware: Get rid of PCI driver specifics in common code Andy Shevchenko
                   ` (5 preceding siblings ...)
  2020-05-19 12:50 ` [PATCH v2 7/7] i2c: designware: Drop hard coded FIFO depth assignment Andy Shevchenko
@ 2020-05-20 13:55 ` Jarkko Nikula
  2020-05-22 10:58   ` Andy Shevchenko
  2020-05-22 14:55 ` Wolfram Sang
  7 siblings, 1 reply; 11+ messages in thread
From: Jarkko Nikula @ 2020-05-20 13:55 UTC (permalink / raw)
  To: Andy Shevchenko, Mika Westerberg, linux-i2c, Wolfram Sang

On 5/19/20 3:50 PM, Andy Shevchenko wrote:
> Do not spread PCI specifics over common code. It seems to be a layering
> violation which can be easily avoided. Refactor PCI driver and drop
> PCI specifics from common code.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
> v2: new patch
>   drivers/i2c/busses/i2c-designware-core.h   |  1 -
>   drivers/i2c/busses/i2c-designware-pcidrv.c | 24 +++++++++++++---------
>   2 files changed, 14 insertions(+), 11 deletions(-)
> 
For all patches 1-7/7:

Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/7] i2c: designware: Get rid of PCI driver specifics in common code
  2020-05-20 13:55 ` [PATCH v2 1/7] i2c: designware: Get rid of PCI driver specifics in common code Jarkko Nikula
@ 2020-05-22 10:58   ` Andy Shevchenko
  2020-05-22 14:18     ` Wolfram Sang
  0 siblings, 1 reply; 11+ messages in thread
From: Andy Shevchenko @ 2020-05-22 10:58 UTC (permalink / raw)
  To: Jarkko Nikula; +Cc: Mika Westerberg, linux-i2c, Wolfram Sang

On Wed, May 20, 2020 at 04:55:39PM +0300, Jarkko Nikula wrote:
> On 5/19/20 3:50 PM, Andy Shevchenko wrote:
> > Do not spread PCI specifics over common code. It seems to be a layering
> > violation which can be easily avoided. Refactor PCI driver and drop
> > PCI specifics from common code.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> > v2: new patch
> >   drivers/i2c/busses/i2c-designware-core.h   |  1 -
> >   drivers/i2c/busses/i2c-designware-pcidrv.c | 24 +++++++++++++---------
> >   2 files changed, 14 insertions(+), 11 deletions(-)
> > 
> For all patches 1-7/7:
> 
> Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

Thank you, Jarkko!

Wolfram, do we have a chance to get this into v5.8-rc1?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/7] i2c: designware: Get rid of PCI driver specifics in common code
  2020-05-22 10:58   ` Andy Shevchenko
@ 2020-05-22 14:18     ` Wolfram Sang
  0 siblings, 0 replies; 11+ messages in thread
From: Wolfram Sang @ 2020-05-22 14:18 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Jarkko Nikula, Mika Westerberg, linux-i2c

[-- Attachment #1: Type: text/plain, Size: 169 bytes --]


> > Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>
> 
> Thank you, Jarkko!
> 
> Wolfram, do we have a chance to get this into v5.8-rc1?

I think so.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 1/7] i2c: designware: Get rid of PCI driver specifics in common code
  2020-05-19 12:50 [PATCH v2 1/7] i2c: designware: Get rid of PCI driver specifics in common code Andy Shevchenko
                   ` (6 preceding siblings ...)
  2020-05-20 13:55 ` [PATCH v2 1/7] i2c: designware: Get rid of PCI driver specifics in common code Jarkko Nikula
@ 2020-05-22 14:55 ` Wolfram Sang
  7 siblings, 0 replies; 11+ messages in thread
From: Wolfram Sang @ 2020-05-22 14:55 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: Jarkko Nikula, Mika Westerberg, linux-i2c

[-- Attachment #1: Type: text/plain, Size: 355 bytes --]

On Tue, May 19, 2020 at 03:50:37PM +0300, Andy Shevchenko wrote:
> Do not spread PCI specifics over common code. It seems to be a layering
> violation which can be easily avoided. Refactor PCI driver and drop
> PCI specifics from common code.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Applied to for-next, thanks!


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-05-22 14:55 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-19 12:50 [PATCH v2 1/7] i2c: designware: Get rid of PCI driver specifics in common code Andy Shevchenko
2020-05-19 12:50 ` [PATCH v2 2/7] i2c: designware: Include proper headers in i2c-desingware-core.h Andy Shevchenko
2020-05-19 12:50 ` [PATCH v2 3/7] i2c: designware: Move i2c_dw_validate_speed() helper to a common code Andy Shevchenko
2020-05-19 12:50 ` [PATCH v2 4/7] i2c: designware: Drop unneeded condition in i2c_dw_validate_speed() Andy Shevchenko
2020-05-19 12:50 ` [PATCH v2 5/7] i2c: designware: Move ACPI parts into common module Andy Shevchenko
2020-05-19 12:50 ` [PATCH v2 6/7] i2c: designware: Read counters from ACPI for PCI driver Andy Shevchenko
2020-05-19 12:50 ` [PATCH v2 7/7] i2c: designware: Drop hard coded FIFO depth assignment Andy Shevchenko
2020-05-20 13:55 ` [PATCH v2 1/7] i2c: designware: Get rid of PCI driver specifics in common code Jarkko Nikula
2020-05-22 10:58   ` Andy Shevchenko
2020-05-22 14:18     ` Wolfram Sang
2020-05-22 14:55 ` Wolfram Sang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).