[PATCH v1 1/4] i2c: designware: Split out i2c_dw_validate_speed() helper

[PATCH v1 1/4] i2c: designware: Split out i2c_dw_validate_speed() helper

[Andy Shevchenko]

In order to export array supported speed for wider use,
split out them along with i2c_dw_validate_speed() helper.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-core.h    | 31 +++++++++++++++++++++
 drivers/i2c/busses/i2c-designware-platdrv.c | 29 ++++---------------
 2 files changed, 36 insertions(+), 24 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 1674caf277451..626959573f894 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -9,6 +9,7 @@
  * Copyright (C) 2009 Provigent Ltd.
  */
 
+#include <linux/errno.h>
 #include <linux/i2c.h>
 
 #define DW_IC_DEFAULT_FUNCTIONALITY (I2C_FUNC_I2C |			\
@@ -351,3 +352,33 @@ 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 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,
+};
+
+static inline 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;
+}
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 01db634461b60..d6c03d7179c7a 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

[PATCH v1 2/4] i2c: designware: Split out OF parts into separate module

[Andy Shevchenko]

For better maintenance and possible code reuse in the future,
split out OF parts into a separate module.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/Makefile                 |  3 ++
 drivers/i2c/busses/i2c-designware-core.h    |  6 +++
 drivers/i2c/busses/i2c-designware-of.c      | 50 ++++++++++++++++++
 drivers/i2c/busses/i2c-designware-platdrv.c | 56 ++++-----------------
 4 files changed, 68 insertions(+), 47 deletions(-)
 create mode 100644 drivers/i2c/busses/i2c-designware-of.c

diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index a33aa107a05d2..cadcff3aad814 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -50,6 +50,9 @@ obj-$(CONFIG_I2C_CPM)		+= i2c-cpm.o
 obj-$(CONFIG_I2C_DAVINCI)	+= i2c-davinci.o
 obj-$(CONFIG_I2C_DESIGNWARE_CORE)	+= i2c-designware-core.o
 i2c-designware-core-objs := i2c-designware-common.o
+ifeq ($(CONFIG_OF),y)
+i2c-designware-core-objs += i2c-designware-of.o
+endif
 i2c-designware-core-objs += i2c-designware-master.o
 ifeq ($(CONFIG_I2C_DESIGNWARE_SLAVE),y)
 i2c-designware-core-objs += i2c-designware-slave.o
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index 626959573f894..b3d7c04ffe1ae 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -382,3 +382,9 @@ static inline int i2c_dw_validate_speed(struct dw_i2c_dev *dev)
 
 	return 0;
 }
+
+#if IS_ENABLED(CONFIG_OF)
+int i2c_dw_of_configure(struct device *device);
+#else
+static inline int i2c_dw_of_configure(struct device *device) { return -ENODEV; }
+#endif
diff --git a/drivers/i2c/busses/i2c-designware-of.c b/drivers/i2c/busses/i2c-designware-of.c
new file mode 100644
index 0000000000000..7434bce286b98
--- /dev/null
+++ b/drivers/i2c/busses/i2c-designware-of.c
@@ -0,0 +1,50 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Synopsys DesignWare I2C adapter driver.
+ *
+ * Based on the TI DAVINCI I2C adapter driver.
+ *
+ * Copyright (C) 2006 Texas Instruments.
+ * Copyright (C) 2007 MontaVista Software Inc.
+ * Copyright (C) 2009 Provigent Ltd.
+ */
+#include <linux/bits.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/export.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/types.h>
+
+#include "i2c-designware-core.h"
+
+#define MSCC_ICPU_CFG_TWI_DELAY		0x0
+#define MSCC_ICPU_CFG_TWI_DELAY_ENABLE	BIT(0)
+#define MSCC_ICPU_CFG_TWI_SPIKE_FILTER	0x4
+
+static int mscc_twi_set_sda_hold_time(struct dw_i2c_dev *dev)
+{
+	writel((dev->sda_hold_time << 1) | MSCC_ICPU_CFG_TWI_DELAY_ENABLE,
+	       dev->ext + MSCC_ICPU_CFG_TWI_DELAY);
+
+	return 0;
+}
+
+int i2c_dw_of_configure(struct device *device)
+{
+	struct platform_device *pdev = to_platform_device(device);
+	struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
+
+	switch (dev->flags & MODEL_MASK) {
+	case MODEL_MSCC_OCELOT:
+		dev->ext = devm_platform_ioremap_resource(pdev, 1);
+		if (!IS_ERR(dev->ext))
+			dev->set_sda_hold_time = mscc_twi_set_sda_hold_time;
+		break;
+	default:
+		break;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(i2c_dw_of_configure);
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index d6c03d7179c7a..6780d636bac73 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -17,10 +17,8 @@
 #include <linux/errno.h>
 #include <linux/i2c.h>
 #include <linux/interrupt.h>
-#include <linux/io.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
-#include <linux/of.h>
 #include <linux/platform_data/i2c-designware.h>
 #include <linux/platform_device.h>
 #include <linux/pm.h>
@@ -141,49 +139,6 @@ static inline int dw_i2c_acpi_configure(struct platform_device *pdev)
 }
 #endif
 
-#ifdef CONFIG_OF
-#define MSCC_ICPU_CFG_TWI_DELAY		0x0
-#define MSCC_ICPU_CFG_TWI_DELAY_ENABLE	BIT(0)
-#define MSCC_ICPU_CFG_TWI_SPIKE_FILTER	0x4
-
-static int mscc_twi_set_sda_hold_time(struct dw_i2c_dev *dev)
-{
-	writel((dev->sda_hold_time << 1) | MSCC_ICPU_CFG_TWI_DELAY_ENABLE,
-	       dev->ext + MSCC_ICPU_CFG_TWI_DELAY);
-
-	return 0;
-}
-
-static int dw_i2c_of_configure(struct platform_device *pdev)
-{
-	struct dw_i2c_dev *dev = platform_get_drvdata(pdev);
-
-	switch (dev->flags & MODEL_MASK) {
-	case MODEL_MSCC_OCELOT:
-		dev->ext = devm_platform_ioremap_resource(pdev, 1);
-		if (!IS_ERR(dev->ext))
-			dev->set_sda_hold_time = mscc_twi_set_sda_hold_time;
-		break;
-	default:
-		break;
-	}
-
-	return 0;
-}
-
-static const struct of_device_id dw_i2c_of_match[] = {
-	{ .compatible = "snps,designware-i2c", },
-	{ .compatible = "mscc,ocelot-i2c", .data = (void *)MODEL_MSCC_OCELOT },
-	{},
-};
-MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
-#else
-static inline int dw_i2c_of_configure(struct platform_device *pdev)
-{
-	return -ENODEV;
-}
-#endif
-
 static void dw_i2c_plat_pm_cleanup(struct dw_i2c_dev *dev)
 {
 	pm_runtime_disable(dev->dev);
@@ -254,7 +209,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 	dev->flags |= (uintptr_t)device_get_match_data(&pdev->dev);
 
 	if (pdev->dev.of_node)
-		dw_i2c_of_configure(pdev);
+		i2c_dw_of_configure(&pdev->dev);
 
 	if (has_acpi_companion(&pdev->dev))
 		dw_i2c_acpi_configure(pdev);
@@ -419,6 +374,13 @@ static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
 #define DW_I2C_DEV_PMOPS NULL
 #endif
 
+static const struct of_device_id dw_i2c_of_match[] = {
+	{ .compatible = "snps,designware-i2c", },
+	{ .compatible = "mscc,ocelot-i2c", .data = (void *)MODEL_MSCC_OCELOT },
+	{}
+};
+MODULE_DEVICE_TABLE(of, dw_i2c_of_match);
+
 /* Work with hotplug and coldplug */
 MODULE_ALIAS("platform:i2c_designware");
 
@@ -427,7 +389,7 @@ static struct platform_driver dw_i2c_driver = {
 	.remove = dw_i2c_plat_remove,
 	.driver		= {
 		.name	= "i2c_designware",
-		.of_match_table = of_match_ptr(dw_i2c_of_match),
+		.of_match_table = dw_i2c_of_match,
 		.acpi_match_table = ACPI_PTR(dw_i2c_acpi_match),
 		.pm	= DW_I2C_DEV_PMOPS,
 	},
-- 
2.26.2

[PATCH v1 3/4] i2c: designware: Split out ACPI parts into separate module

[Andy Shevchenko]

For better maintenance and possible code reuse in the future,
split out ACPI parts into a separate module.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/Makefile                 |   3 +
 drivers/i2c/busses/i2c-designware-acpi.c    | 130 +++++++++++++++++
 drivers/i2c/busses/i2c-designware-core.h    |   8 ++
 drivers/i2c/busses/i2c-designware-platdrv.c | 151 +++-----------------
 4 files changed, 163 insertions(+), 129 deletions(-)
 create mode 100644 drivers/i2c/busses/i2c-designware-acpi.c

diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index cadcff3aad814..177a4266dc1e3 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -50,6 +50,9 @@ obj-$(CONFIG_I2C_CPM)		+= i2c-cpm.o
 obj-$(CONFIG_I2C_DAVINCI)	+= i2c-davinci.o
 obj-$(CONFIG_I2C_DESIGNWARE_CORE)	+= i2c-designware-core.o
 i2c-designware-core-objs := i2c-designware-common.o
+ifeq ($(CONFIG_ACPI),y)
+i2c-designware-core-objs += i2c-designware-acpi.o
+endif
 ifeq ($(CONFIG_OF),y)
 i2c-designware-core-objs += i2c-designware-of.o
 endif
diff --git a/drivers/i2c/busses/i2c-designware-acpi.c b/drivers/i2c/busses/i2c-designware-acpi.c
new file mode 100644
index 0000000000000..78b62546cea8a
--- /dev/null
+++ b/drivers/i2c/busses/i2c-designware-acpi.c
@@ -0,0 +1,130 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Synopsys DesignWare I2C adapter driver.
+ *
+ * Based on the TI DAVINCI I2C adapter driver.
+ *
+ * Copyright (C) 2006 Texas Instruments.
+ * Copyright (C) 2007 MontaVista Software Inc.
+ * Copyright (C) 2009 Provigent Ltd.
+ */
+#include <linux/acpi.h>
+#include <linux/device.h>
+#include <linux/dmi.h>
+#include <linux/export.h>
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+
+#include "i2c-designware-core.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(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;
+}
+EXPORT_SYMBOL_GPL(i2c_dw_acpi_adjust_bus_speed);
diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
index b3d7c04ffe1ae..140c0b3d49686 100644
--- a/drivers/i2c/busses/i2c-designware-core.h
+++ b/drivers/i2c/busses/i2c-designware-core.h
@@ -383,6 +383,14 @@ static inline int i2c_dw_validate_speed(struct dw_i2c_dev *dev)
 	return 0;
 }
 
+#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
+
 #if IS_ENABLED(CONFIG_OF)
 int i2c_dw_of_configure(struct device *device);
 #else
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 6780d636bac73..deada90e67071 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>
@@ -36,109 +35,6 @@ static u32 i2c_dw_get_clk_rate_khz(struct dw_i2c_dev *dev)
 	return clk_get_rate(dev->clk)/1000;
 }
 
-#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 },
-	{ "INT3432", 0 },
-	{ "INT3433", 0 },
-	{ "80860F41", ACCESS_NO_IRQ_SUSPEND },
-	{ "808622C1", ACCESS_NO_IRQ_SUSPEND | MODEL_CHERRYTRAIL },
-	{ "AMD0010", ACCESS_INTR_MASK },
-	{ "AMDI0010", ACCESS_INTR_MASK },
-	{ "AMDI0510", 0 },
-	{ "APMC0D0F", 0 },
-	{ "HISI02A1", 0 },
-	{ "HISI02A2", 0 },
-	{ "HISI02A3", 0 },
-	{ }
-};
-MODULE_DEVICE_TABLE(acpi, dw_i2c_acpi_match);
-#else
-static inline int dw_i2c_acpi_configure(struct platform_device *pdev)
-{
-	return -ENODEV;
-}
-#endif
-
 static void dw_i2c_plat_pm_cleanup(struct dw_i2c_dev *dev)
 {
 	pm_runtime_disable(dev->dev);
@@ -153,8 +49,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)
@@ -184,27 +79,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);
 
@@ -212,7 +87,7 @@ static int dw_i2c_plat_probe(struct platform_device *pdev)
 		i2c_dw_of_configure(&pdev->dev);
 
 	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)
@@ -374,6 +249,24 @@ static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
 #define DW_I2C_DEV_PMOPS NULL
 #endif
 
+static const struct acpi_device_id dw_i2c_acpi_match[] = {
+	{ "INT33C2", 0 },
+	{ "INT33C3", 0 },
+	{ "INT3432", 0 },
+	{ "INT3433", 0 },
+	{ "80860F41", ACCESS_NO_IRQ_SUSPEND },
+	{ "808622C1", ACCESS_NO_IRQ_SUSPEND | MODEL_CHERRYTRAIL },
+	{ "AMD0010", ACCESS_INTR_MASK },
+	{ "AMDI0010", ACCESS_INTR_MASK },
+	{ "AMDI0510", 0 },
+	{ "APMC0D0F", 0 },
+	{ "HISI02A1", 0 },
+	{ "HISI02A2", 0 },
+	{ "HISI02A3", 0 },
+	{}
+};
+MODULE_DEVICE_TABLE(acpi, dw_i2c_acpi_match);
+
 static const struct of_device_id dw_i2c_of_match[] = {
 	{ .compatible = "snps,designware-i2c", },
 	{ .compatible = "mscc,ocelot-i2c", .data = (void *)MODEL_MSCC_OCELOT },
@@ -390,7 +283,7 @@ static struct platform_driver dw_i2c_driver = {
 	.driver		= {
 		.name	= "i2c_designware",
 		.of_match_table = dw_i2c_of_match,
-		.acpi_match_table = ACPI_PTR(dw_i2c_acpi_match),
+		.acpi_match_table = dw_i2c_acpi_match,
 		.pm	= DW_I2C_DEV_PMOPS,
 	},
 };
-- 
2.26.2

Re: [PATCH v1 3/4] i2c: designware: Split out ACPI parts into separate module

[Jarkko Nikula]

On 5/7/20 4:51 PM, Andy Shevchenko wrote:
> For better maintenance and possible code reuse in the future,
> split out ACPI parts into a separate module.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>   drivers/i2c/busses/Makefile                 |   3 +
>   drivers/i2c/busses/i2c-designware-acpi.c    | 130 +++++++++++++++++
>   drivers/i2c/busses/i2c-designware-core.h    |   8 ++
>   drivers/i2c/busses/i2c-designware-platdrv.c | 151 +++-----------------
>   4 files changed, 163 insertions(+), 129 deletions(-)
>   create mode 100644 drivers/i2c/busses/i2c-designware-acpi.c
> 
Comment to both of this and patch 2/4:

I'm not so fan of introducing even more i2c-designware modules. No any 
other drivers have so many files as i2c-designware in 
drivers/i2c/busses/*.c. Feeling a bit of hall of shame because of it :-)

What's the rationale here? Currently i2c-designware-platdrv.c is 512 
lines of code so it's not too hard to maintain in my opinion.

-- 
Jarkko

Re: [PATCH v1 3/4] i2c: designware: Split out ACPI parts into separate module

[Andy Shevchenko]

On Mon, May 11, 2020 at 04:31:18PM +0300, Jarkko Nikula wrote:
> On 5/7/20 4:51 PM, Andy Shevchenko wrote:
> > For better maintenance and possible code reuse in the future,
> > split out ACPI parts into a separate module.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >   drivers/i2c/busses/Makefile                 |   3 +
> >   drivers/i2c/busses/i2c-designware-acpi.c    | 130 +++++++++++++++++
> >   drivers/i2c/busses/i2c-designware-core.h    |   8 ++
> >   drivers/i2c/busses/i2c-designware-platdrv.c | 151 +++-----------------
> >   4 files changed, 163 insertions(+), 129 deletions(-)
> >   create mode 100644 drivers/i2c/busses/i2c-designware-acpi.c
> > 
> Comment to both of this and patch 2/4:
> 
> I'm not so fan of introducing even more i2c-designware modules. No any other
> drivers have so many files as i2c-designware in drivers/i2c/busses/*.c.
> Feeling a bit of hall of shame because of it :-)
> 
> What's the rationale here? Currently i2c-designware-platdrv.c is 512 lines
> of code so it's not too hard to maintain in my opinion.

How to use same functionality in PCI and platform driver? Are you implying that
the common.c is good enough? I would move there, of course.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v1 4/4] i2c: designware: Read counters from ACPI for PCI driver

[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>
---
 drivers/i2c/busses/i2c-designware-pcidrv.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index c762e5a11e44e..96191925f71ec 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -251,6 +251,15 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 		}
 	}
 
+	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

Re: [PATCH v1 1/4] i2c: designware: Split out i2c_dw_validate_speed() helper

[Jarkko Nikula]

On 5/7/20 4:51 PM, Andy Shevchenko wrote:
> In order to export array supported speed for wider use,
> split out them along with i2c_dw_validate_speed() helper.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>   drivers/i2c/busses/i2c-designware-core.h    | 31 +++++++++++++++++++++
>   drivers/i2c/busses/i2c-designware-platdrv.c | 29 ++++---------------
>   2 files changed, 36 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
> index 1674caf277451..626959573f894 100644
> --- a/drivers/i2c/busses/i2c-designware-core.h
> +++ b/drivers/i2c/busses/i2c-designware-core.h
> @@ -9,6 +9,7 @@
>    * Copyright (C) 2009 Provigent Ltd.
>    */
>   
> +#include <linux/errno.h>
>   #include <linux/i2c.h>
>   
>   #define DW_IC_DEFAULT_FUNCTIONALITY (I2C_FUNC_I2C |			\
> @@ -351,3 +352,33 @@ 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 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,
> +};
> +
> +static inline 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;
> +}

This header is included by every i2c-designware-*.c file and this inline 
function is not tiny. Would it be better to have this in 
i2c-designware-common.c instead?

-- 
Jarkko

Re: [PATCH v1 1/4] i2c: designware: Split out i2c_dw_validate_speed() helper

[Andy Shevchenko]

On Mon, May 11, 2020 at 04:11:57PM +0300, Jarkko Nikula wrote:
> On 5/7/20 4:51 PM, Andy Shevchenko wrote:
> > In order to export array supported speed for wider use,
> > split out them along with i2c_dw_validate_speed() helper.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >   drivers/i2c/busses/i2c-designware-core.h    | 31 +++++++++++++++++++++
> >   drivers/i2c/busses/i2c-designware-platdrv.c | 29 ++++---------------
> >   2 files changed, 36 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
> > index 1674caf277451..626959573f894 100644
> > --- a/drivers/i2c/busses/i2c-designware-core.h
> > +++ b/drivers/i2c/busses/i2c-designware-core.h
> > @@ -9,6 +9,7 @@
> >    * Copyright (C) 2009 Provigent Ltd.
> >    */
> > +#include <linux/errno.h>
> >   #include <linux/i2c.h>
> >   #define DW_IC_DEFAULT_FUNCTIONALITY (I2C_FUNC_I2C |			\
> > @@ -351,3 +352,33 @@ 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 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,
> > +};
> > +
> > +static inline 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;
> > +}
> 
> This header is included by every i2c-designware-*.c file and this inline
> function is not tiny. Would it be better to have this in
> i2c-designware-common.c instead?

Yes, but then we will need to export i2c_dw_supported_speeds as well as its
array size.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/4] i2c: designware: Split out i2c_dw_validate_speed() helper

[Jarkko Nikula]

On 5/11/20 4:42 PM, Andy Shevchenko wrote:
> On Mon, May 11, 2020 at 04:11:57PM +0300, Jarkko Nikula wrote:
>> On 5/7/20 4:51 PM, Andy Shevchenko wrote:
>>> In order to export array supported speed for wider use,
>>> split out them along with i2c_dw_validate_speed() helper.
>>>
>>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>>> ---
>>>    drivers/i2c/busses/i2c-designware-core.h    | 31 +++++++++++++++++++++
>>>    drivers/i2c/busses/i2c-designware-platdrv.c | 29 ++++---------------
>>>    2 files changed, 36 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/drivers/i2c/busses/i2c-designware-core.h b/drivers/i2c/busses/i2c-designware-core.h
>>> index 1674caf277451..626959573f894 100644
>>> --- a/drivers/i2c/busses/i2c-designware-core.h
>>> +++ b/drivers/i2c/busses/i2c-designware-core.h
>>> @@ -9,6 +9,7 @@
>>>     * Copyright (C) 2009 Provigent Ltd.
>>>     */
>>> +#include <linux/errno.h>
>>>    #include <linux/i2c.h>
>>>    #define DW_IC_DEFAULT_FUNCTIONALITY (I2C_FUNC_I2C |			\
>>> @@ -351,3 +352,33 @@ 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 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,
>>> +};
>>> +
>>> +static inline 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;
>>> +}
>>
>> This header is included by every i2c-designware-*.c file and this inline
>> function is not tiny. Would it be better to have this in
>> i2c-designware-common.c instead?
> 
> Yes, but then we will need to export i2c_dw_supported_speeds as well as its
> array size.
> 
Would that not be needed if you move ACPI parts also into 
i2c-designware-common.c?

-- 
Jarkko

Re: [PATCH v1 1/4] i2c: designware: Split out i2c_dw_validate_speed() helper

[Andy Shevchenko]

On Tue, May 12, 2020 at 06:12:32PM +0300, Jarkko Nikula wrote:
> On 5/11/20 4:42 PM, Andy Shevchenko wrote:
> > On Mon, May 11, 2020 at 04:11:57PM +0300, Jarkko Nikula wrote:
> > > On 5/7/20 4:51 PM, Andy Shevchenko wrote:
> > > > In order to export array supported speed for wider use,
> > > > split out them along with i2c_dw_validate_speed() helper.

...

> > > This header is included by every i2c-designware-*.c file and this inline
> > > function is not tiny. Would it be better to have this in
> > > i2c-designware-common.c instead?
> > 
> > Yes, but then we will need to export i2c_dw_supported_speeds as well as its
> > array size.
> > 
> Would that not be needed if you move ACPI parts also into
> i2c-designware-common.c?

Yes, in the intermediate patch. I'm about to send v2, where you may see how it
looks like.

-- 
With Best Regards,
Andy Shevchenko