[PATCH v1 01/11] i2c: Introduce common module to instantiate CCGx UCSI

[PATCH v1 01/11] i2c: Introduce common module to instantiate CCGx UCSI

[Andy Shevchenko]

Introduce a common module to provide an API to instantiate UCSI device
for Cypress CCGx Type-C controller. Individual bus drivers need to select
this one on demand.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/Kconfig         |  7 +++++++
 drivers/i2c/busses/Makefile        |  3 +++
 drivers/i2c/busses/i2c-ccgx-ucsi.c | 27 +++++++++++++++++++++++++++
 drivers/i2c/busses/i2c-ccgx-ucsi.h | 11 +++++++++++
 4 files changed, 48 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-ccgx-ucsi.c
 create mode 100644 drivers/i2c/busses/i2c-ccgx-ucsi.h

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index df89cb809330..0fb2caf7498c 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -9,6 +9,13 @@ menu "I2C Hardware Bus support"
 comment "PC SMBus host controller drivers"
 	depends on PCI
 
+config I2C_CCGX_UCSI
+	tristate
+	help
+	  A common module to provide an API to instantiate UCSI device
+	  for Cypress CCGx Type-C controller. Individual bus drivers
+	  need to select this one on demand.
+
 config I2C_ALI1535
 	tristate "ALI 1535"
 	depends on PCI
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 1d00dce77098..79405cb5d600 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -6,6 +6,9 @@
 # ACPI drivers
 obj-$(CONFIG_I2C_SCMI)		+= i2c-scmi.o
 
+# Auxiliary I2C/SMBus modules
+obj-$(CONFIG_I2C_CCGX_UCSI)	+= i2c-ccgx-ucsi.o
+
 # PC SMBus host controller drivers
 obj-$(CONFIG_I2C_ALI1535)	+= i2c-ali1535.o
 obj-$(CONFIG_I2C_ALI1563)	+= i2c-ali1563.o
diff --git a/drivers/i2c/busses/i2c-ccgx-ucsi.c b/drivers/i2c/busses/i2c-ccgx-ucsi.c
new file mode 100644
index 000000000000..141c3d1ef752
--- /dev/null
+++ b/drivers/i2c/busses/i2c-ccgx-ucsi.c
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Instantiate UCSI device for Cypress CCGx Type-C controller.
+ * Derived from i2c-designware-pcidrv.c and i2c-nvidia-gpu.c.
+ */
+
+#include <linux/i2c.h>
+#include <linux/export.h>
+#include <linux/string.h>
+
+#include "i2c-ccgx-ucsi.h"
+
+struct software_node;
+
+struct i2c_client *i2c_new_ccgx_ucsi(struct i2c_adapter *adapter, int irq,
+				     const struct software_node *swnode)
+{
+	struct i2c_board_info info = {};
+
+	strscpy(info.type, "ccgx-ucsi", sizeof(info.type));
+	info.addr = 0x08;
+	info.irq = irq;
+	info.swnode = swnode;
+
+	return i2c_new_client_device(adapter, &info);
+}
+EXPORT_SYMBOL_GPL(i2c_new_ccgx_ucsi);
diff --git a/drivers/i2c/busses/i2c-ccgx-ucsi.h b/drivers/i2c/busses/i2c-ccgx-ucsi.h
new file mode 100644
index 000000000000..739ac7a4b117
--- /dev/null
+++ b/drivers/i2c/busses/i2c-ccgx-ucsi.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef __I2C_CCGX_UCSI_H_
+#define __I2C_CCGX_UCSI_H_
+
+struct i2c_adapter;
+struct i2c_client;
+struct software_node;
+
+struct i2c_client *i2c_new_ccgx_ucsi(struct i2c_adapter *adapter, int irq,
+				     const struct software_node *swnode);
+#endif /* __I2C_CCGX_UCSI_H_ */
-- 
2.33.0

[PATCH v1 02/11] i2c: designware-pci: Switch to use i2c_new_ccgx_ucsi()

[Andy Shevchenko]

Instead of open coded variant switch to use i2c_new_ccgx_ucsi().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/Kconfig                 |  1 +
 drivers/i2c/busses/i2c-designware-pcidrv.c | 30 ++++------------------
 2 files changed, 6 insertions(+), 25 deletions(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 0fb2caf7498c..451ddec12dba 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -577,6 +577,7 @@ config I2C_DESIGNWARE_PCI
 	tristate "Synopsys DesignWare PCI"
 	depends on PCI
 	select I2C_DESIGNWARE_CORE
+	select I2C_CCGX_UCSI
 	help
 	  If you say yes to this option, support will be included for the
 	  Synopsys DesignWare I2C adapter. Only master mode is supported.
diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 0f409a4c2da0..2952eca87b86 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -24,6 +24,7 @@
 #include <linux/slab.h>
 
 #include "i2c-designware-core.h"
+#include "i2c-ccgx-ucsi.h"
 
 #define DRIVER_NAME "i2c-designware-pci"
 #define AMD_CLK_RATE_HZ	100000
@@ -118,26 +119,6 @@ static int mfld_setup(struct pci_dev *pdev, struct dw_pci_controller *c)
 	return -ENODEV;
 }
 
- /*
-  * TODO find a better way how to deduplicate instantiation
-  * of USB PD slave device from nVidia GPU driver.
-  */
-static int navi_amd_register_client(struct dw_i2c_dev *dev)
-{
-	struct i2c_board_info	info;
-
-	memset(&info, 0, sizeof(struct i2c_board_info));
-	strscpy(info.type, "ccgx-ucsi", I2C_NAME_SIZE);
-	info.addr = 0x08;
-	info.irq = dev->irq;
-
-	dev->slave = i2c_new_client_device(&dev->adapter, &info);
-	if (IS_ERR(dev->slave))
-		return PTR_ERR(dev->slave);
-
-	return 0;
-}
-
 static int navi_amd_setup(struct pci_dev *pdev, struct dw_pci_controller *c)
 {
 	struct dw_i2c_dev *dev = dev_get_drvdata(&pdev->dev);
@@ -324,11 +305,10 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 	}
 
 	if ((dev->flags & MODEL_MASK) == MODEL_AMD_NAVI_GPU) {
-		r = navi_amd_register_client(dev);
-		if (r) {
-			dev_err(dev->dev, "register client failed with %d\n", r);
-			return r;
-		}
+		dev->slave = i2c_new_ccgx_ucsi(&dev->adapter, dev->irq, NULL);
+		if (IS_ERR(dev->slave))
+			return dev_err_probe(&pdev->dev, PTR_ERR(dev->slave),
+					     "register UCSI failed\n");
 	}
 
 	pm_runtime_set_autosuspend_delay(&pdev->dev, 1000);
-- 
2.33.0

[PATCH v1 03/11] i2c: designware-pci: Use temporary variable for struct device

[Andy Shevchenko]

Use temporary variable for struct device to make code neater.

While at it, rename variable of struct dw_i2c_dev pointer to i_dev.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-pcidrv.c | 99 +++++++++++-----------
 1 file changed, 50 insertions(+), 49 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 2952eca87b86..f91b352f448a 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -52,7 +52,7 @@ struct dw_pci_controller {
 	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);
+	u32 (*get_clk_rate_khz)(struct dw_i2c_dev *i_dev);
 };
 
 /* Merrifield HCNT/LCNT/SDA hold time */
@@ -88,23 +88,23 @@ static struct dw_scl_sda_cfg navi_amd_config = {
 	.sda_hold = 0x9,
 };
 
-static u32 mfld_get_clk_rate_khz(struct dw_i2c_dev *dev)
+static u32 mfld_get_clk_rate_khz(struct dw_i2c_dev *i_dev)
 {
 	return 25000;
 }
 
-static u32 navi_amd_get_clk_rate_khz(struct dw_i2c_dev *dev)
+static u32 navi_amd_get_clk_rate_khz(struct dw_i2c_dev *i_dev)
 {
 	return AMD_CLK_RATE_HZ;
 }
 
 static int mfld_setup(struct pci_dev *pdev, struct dw_pci_controller *c)
 {
-	struct dw_i2c_dev *dev = dev_get_drvdata(&pdev->dev);
+	struct dw_i2c_dev *i_dev = dev_get_drvdata(&pdev->dev);
 
 	switch (pdev->device) {
 	case 0x0817:
-		dev->timings.bus_freq_hz = I2C_MAX_STANDARD_MODE_FREQ;
+		i_dev->timings.bus_freq_hz = I2C_MAX_STANDARD_MODE_FREQ;
 		fallthrough;
 	case 0x0818:
 	case 0x0819:
@@ -121,10 +121,10 @@ static int mfld_setup(struct pci_dev *pdev, struct dw_pci_controller *c)
 
 static int navi_amd_setup(struct pci_dev *pdev, struct dw_pci_controller *c)
 {
-	struct dw_i2c_dev *dev = dev_get_drvdata(&pdev->dev);
+	struct dw_i2c_dev *i_dev = dev_get_drvdata(&pdev->dev);
 
-	dev->flags |= MODEL_AMD_NAVI_GPU;
-	dev->timings.bus_freq_hz = I2C_MAX_STANDARD_MODE_FREQ;
+	i_dev->flags |= MODEL_AMD_NAVI_GPU;
+	i_dev->timings.bus_freq_hz = I2C_MAX_STANDARD_MODE_FREQ;
 	return 0;
 }
 
@@ -216,14 +216,15 @@ static UNIVERSAL_DEV_PM_OPS(i2c_dw_pm_ops, i2c_dw_pci_suspend,
 static int i2c_dw_pci_probe(struct pci_dev *pdev,
 			    const struct pci_device_id *id)
 {
-	struct dw_i2c_dev *dev;
+	struct device *dev = &pdev->dev;
 	struct i2c_adapter *adap;
 	int r;
 	struct dw_pci_controller *controller;
 	struct dw_scl_sda_cfg *cfg;
+	struct dw_i2c_dev *i_dev;
 
 	if (id->driver_data >= ARRAY_SIZE(dw_pci_controllers)) {
-		dev_err(&pdev->dev, "%s: invalid driver data %ld\n", __func__,
+		dev_err(dev, "%s: invalid driver data %ld\n", __func__,
 			id->driver_data);
 		return -EINVAL;
 	}
@@ -232,7 +233,7 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 
 	r = pcim_enable_device(pdev);
 	if (r) {
-		dev_err(&pdev->dev, "Failed to enable I2C PCI device (%d)\n",
+		dev_err(dev, "Failed to enable I2C PCI device (%d)\n",
 			r);
 		return r;
 	}
@@ -241,26 +242,26 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 
 	r = pcim_iomap_regions(pdev, 1 << 0, pci_name(pdev));
 	if (r) {
-		dev_err(&pdev->dev, "I/O memory remapping failed\n");
+		dev_err(dev, "I/O memory remapping failed\n");
 		return r;
 	}
 
-	dev = devm_kzalloc(&pdev->dev, sizeof(struct dw_i2c_dev), GFP_KERNEL);
-	if (!dev)
+	i_dev = devm_kzalloc(dev, sizeof(*i_dev), GFP_KERNEL);
+	if (!i_dev)
 		return -ENOMEM;
 
 	r = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
 	if (r < 0)
 		return r;
 
-	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;
-	dev->irq = pci_irq_vector(pdev, 0);
-	dev->flags |= controller->flags;
+	i_dev->get_clk_rate_khz = controller->get_clk_rate_khz;
+	i_dev->timings.bus_freq_hz = I2C_MAX_FAST_MODE_FREQ;
+	i_dev->base = pcim_iomap_table(pdev)[0];
+	i_dev->dev = dev;
+	i_dev->irq = pci_irq_vector(pdev, 0);
+	i_dev->flags |= controller->flags;
 
-	pci_set_drvdata(pdev, dev);
+	pci_set_drvdata(pdev, i_dev);
 
 	if (controller->setup) {
 		r = controller->setup(pdev, controller);
@@ -270,65 +271,65 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 		}
 	}
 
-	i2c_dw_adjust_bus_speed(dev);
+	i2c_dw_adjust_bus_speed(i_dev);
 
-	if (has_acpi_companion(&pdev->dev))
-		i2c_dw_acpi_configure(&pdev->dev);
+	if (has_acpi_companion(dev))
+		i2c_dw_acpi_configure(dev);
 
-	r = i2c_dw_validate_speed(dev);
+	r = i2c_dw_validate_speed(i_dev);
 	if (r) {
 		pci_free_irq_vectors(pdev);
 		return r;
 	}
 
-	i2c_dw_configure(dev);
+	i2c_dw_configure(i_dev);
 
 	if (controller->scl_sda_cfg) {
 		cfg = controller->scl_sda_cfg;
-		dev->ss_hcnt = cfg->ss_hcnt;
-		dev->fs_hcnt = cfg->fs_hcnt;
-		dev->ss_lcnt = cfg->ss_lcnt;
-		dev->fs_lcnt = cfg->fs_lcnt;
-		dev->sda_hold_time = cfg->sda_hold;
+		i_dev->ss_hcnt = cfg->ss_hcnt;
+		i_dev->fs_hcnt = cfg->fs_hcnt;
+		i_dev->ss_lcnt = cfg->ss_lcnt;
+		i_dev->fs_lcnt = cfg->fs_lcnt;
+		i_dev->sda_hold_time = cfg->sda_hold;
 	}
 
-	adap = &dev->adapter;
+	adap = &i_dev->adapter;
 	adap->owner = THIS_MODULE;
 	adap->class = 0;
-	ACPI_COMPANION_SET(&adap->dev, ACPI_COMPANION(&pdev->dev));
+	ACPI_COMPANION_SET(&adap->dev, ACPI_COMPANION(dev));
 	adap->nr = controller->bus_num;
 
-	r = i2c_dw_probe(dev);
+	r = i2c_dw_probe(i_dev);
 	if (r) {
 		pci_free_irq_vectors(pdev);
 		return r;
 	}
 
-	if ((dev->flags & MODEL_MASK) == MODEL_AMD_NAVI_GPU) {
-		dev->slave = i2c_new_ccgx_ucsi(&dev->adapter, dev->irq, NULL);
-		if (IS_ERR(dev->slave))
-			return dev_err_probe(&pdev->dev, PTR_ERR(dev->slave),
-					     "register UCSI failed\n");
+	if ((i_dev->flags & MODEL_MASK) == MODEL_AMD_NAVI_GPU) {
+		i_dev->slave = i2c_new_ccgx_ucsi(&i_dev->adapter, i_dev->irq, NULL);
+		if (IS_ERR(i_dev->slave))
+			return dev_err_probe(dev, PTR_ERR(i_dev->slave), "register UCSI failed\n");
 	}
 
-	pm_runtime_set_autosuspend_delay(&pdev->dev, 1000);
-	pm_runtime_use_autosuspend(&pdev->dev);
-	pm_runtime_put_autosuspend(&pdev->dev);
-	pm_runtime_allow(&pdev->dev);
+	pm_runtime_set_autosuspend_delay(dev, 1000);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_put_autosuspend(dev);
+	pm_runtime_allow(dev);
 
 	return 0;
 }
 
 static void i2c_dw_pci_remove(struct pci_dev *pdev)
 {
-	struct dw_i2c_dev *dev = pci_get_drvdata(pdev);
+	struct dw_i2c_dev *i_dev = pci_get_drvdata(pdev);
+	struct device *dev = &pdev->dev;
 
-	dev->disable(dev);
-	pm_runtime_forbid(&pdev->dev);
-	pm_runtime_get_noresume(&pdev->dev);
+	i_dev->disable(i_dev);
+	pm_runtime_forbid(dev);
+	pm_runtime_get_noresume(dev);
 
-	i2c_del_adapter(&dev->adapter);
-	devm_free_irq(&pdev->dev, dev->irq, dev);
+	i2c_del_adapter(&i_dev->adapter);
+	devm_free_irq(dev, i_dev->irq, i_dev);
 	pci_free_irq_vectors(pdev);
 }
 
-- 
2.33.0

[PATCH v1 04/11] i2c: designware-pci: Convert to use dev_err_probe()

[Andy Shevchenko]

It's fine to call dev_err_probe() in ->probe() when error code is known.
Convert the driver to use dev_err_probe().

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

diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index f91b352f448a..e6b4b1a468da 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -223,28 +223,20 @@ static int i2c_dw_pci_probe(struct pci_dev *pdev,
 	struct dw_scl_sda_cfg *cfg;
 	struct dw_i2c_dev *i_dev;
 
-	if (id->driver_data >= ARRAY_SIZE(dw_pci_controllers)) {
-		dev_err(dev, "%s: invalid driver data %ld\n", __func__,
-			id->driver_data);
-		return -EINVAL;
-	}
+	if (id->driver_data >= ARRAY_SIZE(dw_pci_controllers))
+		return dev_err_probe(dev, -EINVAL, "invalid driver data %ld\n", id->driver_data);
 
 	controller = &dw_pci_controllers[id->driver_data];
 
 	r = pcim_enable_device(pdev);
-	if (r) {
-		dev_err(dev, "Failed to enable I2C PCI device (%d)\n",
-			r);
-		return r;
-	}
+	if (r)
+		return dev_err_probe(dev, r, "Failed to enable I2C PCI device\n");
 
 	pci_set_master(pdev);
 
 	r = pcim_iomap_regions(pdev, 1 << 0, pci_name(pdev));
-	if (r) {
-		dev_err(dev, "I/O memory remapping failed\n");
-		return r;
-	}
+	if (r)
+		return dev_err_probe(dev, r, "I/O memory remapping failed\n");
 
 	i_dev = devm_kzalloc(dev, sizeof(*i_dev), GFP_KERNEL);
 	if (!i_dev)
-- 
2.33.0

[PATCH v1 05/11] i2c: designware-pci: use __maybe_unused for PM functions

[Andy Shevchenko]

Use __maybe_unused for PM functions instead of ifdeffery.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-pcidrv.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index e6b4b1a468da..66dc765b6834 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -187,8 +187,7 @@ static struct dw_pci_controller dw_pci_controllers[] = {
 	},
 };
 
-#ifdef CONFIG_PM
-static int i2c_dw_pci_suspend(struct device *dev)
+static int __maybe_unused i2c_dw_pci_suspend(struct device *dev)
 {
 	struct dw_i2c_dev *i_dev = dev_get_drvdata(dev);
 
@@ -198,7 +197,7 @@ static int i2c_dw_pci_suspend(struct device *dev)
 	return 0;
 }
 
-static int i2c_dw_pci_resume(struct device *dev)
+static int __maybe_unused i2c_dw_pci_resume(struct device *dev)
 {
 	struct dw_i2c_dev *i_dev = dev_get_drvdata(dev);
 	int ret;
@@ -208,7 +207,6 @@ static int i2c_dw_pci_resume(struct device *dev)
 
 	return ret;
 }
-#endif
 
 static UNIVERSAL_DEV_PM_OPS(i2c_dw_pm_ops, i2c_dw_pci_suspend,
 			    i2c_dw_pci_resume, NULL);
-- 
2.33.0

[PATCH v1 06/11] i2c: designware-pci: Fix to change data types of hcnt and lcnt parameters

[Andy Shevchenko]

From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>

The data type of hcnt and lcnt in the struct dw_i2c_dev is of type u16.
It's better to have same data type in struct dw_scl_sda_cfg as well.

Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-pcidrv.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 66dc765b6834..a0ea71e71886 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -40,10 +40,10 @@ enum dw_pci_ctl_id_t {
 };
 
 struct dw_scl_sda_cfg {
-	u32 ss_hcnt;
-	u32 fs_hcnt;
-	u32 ss_lcnt;
-	u32 fs_lcnt;
+	u16 ss_hcnt;
+	u16 fs_hcnt;
+	u16 ss_lcnt;
+	u16 fs_lcnt;
 	u32 sda_hold;
 };
 
-- 
2.33.0

[PATCH v1 07/11] i2c: designware-pci: Group MODULE_*() macros

[Andy Shevchenko]

For better maintenance group MODULE_*() macros together.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/i2c-designware-pcidrv.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index a0ea71e71886..87bdf7acb612 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -323,9 +323,6 @@ static void i2c_dw_pci_remove(struct pci_dev *pdev)
 	pci_free_irq_vectors(pdev);
 }
 
-/* work with hotplug and coldplug */
-MODULE_ALIAS("i2c_designware-pci");
-
 static const struct pci_device_id i2_designware_pci_ids[] = {
 	/* Medfield */
 	{ PCI_VDEVICE(INTEL, 0x0817), medfield },
@@ -382,9 +379,11 @@ static struct pci_driver dw_i2c_driver = {
 		.pm     = &i2c_dw_pm_ops,
 	},
 };
-
 module_pci_driver(dw_i2c_driver);
 
+/* work with hotplug and coldplug */
+MODULE_ALIAS("i2c_designware-pci");
+
 MODULE_AUTHOR("Baruch Siach <baruch@tkos.co.il>");
 MODULE_DESCRIPTION("Synopsys DesignWare PCI I2C bus adapter");
 MODULE_LICENSE("GPL");
-- 
2.33.0

[PATCH v1 08/11] i2c: designware-pci: Add a note about struct dw_scl_sda_cfg usage

[Andy Shevchenko]

Add a note about struct dw_scl_sda_cfg usage to discourage people
of using this structure on new platforms. Instead they should try
hard to put the needed information into firmware descriptions.

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

diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
index 87bdf7acb612..c1b57895e8ab 100644
--- a/drivers/i2c/busses/i2c-designware-pcidrv.c
+++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
@@ -39,6 +39,13 @@ enum dw_pci_ctl_id_t {
 	navi_amd,
 };
 
+/*
+ * This is a legacy structure to describe the hardware counters
+ * to configure signal timings on the bus. For Device Tree platforms
+ * one should use the respective properties and for ACPI there is
+ * a set of ACPI methods that provide these counters. No new
+ * platform should use this structure.
+ */
 struct dw_scl_sda_cfg {
 	u16 ss_hcnt;
 	u16 fs_hcnt;
-- 
2.33.0

[PATCH v1 09/11] i2c: nvidia-gpu: Switch to use i2c_new_ccgx_ucsi()

[Andy Shevchenko]

Instead of open coded variant switch to use i2c_new_ccgx_ucsi().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/Kconfig          |  1 +
 drivers/i2c/busses/i2c-nvidia-gpu.c | 26 ++++++--------------------
 2 files changed, 7 insertions(+), 20 deletions(-)

diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 451ddec12dba..c2d29d7cdff3 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -252,6 +252,7 @@ config I2C_NFORCE2_S4985
 config I2C_NVIDIA_GPU
 	tristate "NVIDIA GPU I2C controller"
 	depends on PCI
+	select I2C_CCGX_UCSI
 	help
 	  If you say yes to this option, support will be included for the
 	  NVIDIA GPU I2C controller which is used to communicate with the GPU's
diff --git a/drivers/i2c/busses/i2c-nvidia-gpu.c b/drivers/i2c/busses/i2c-nvidia-gpu.c
index b5055a3cbd93..8117c3674209 100644
--- a/drivers/i2c/busses/i2c-nvidia-gpu.c
+++ b/drivers/i2c/busses/i2c-nvidia-gpu.c
@@ -17,6 +17,8 @@
 
 #include <asm/unaligned.h>
 
+#include "i2c-ccgx-ucsi.h"
+
 /* I2C definitions */
 #define I2C_MST_CNTL				0x00
 #define I2C_MST_CNTL_GEN_START			BIT(0)
@@ -266,23 +268,6 @@ static const struct software_node ccgx_node = {
 	.properties = ccgx_props,
 };
 
-static int gpu_populate_client(struct gpu_i2c_dev *i2cd, int irq)
-{
-	i2cd->gpu_ccgx_ucsi = devm_kzalloc(i2cd->dev,
-					   sizeof(*i2cd->gpu_ccgx_ucsi),
-					   GFP_KERNEL);
-	if (!i2cd->gpu_ccgx_ucsi)
-		return -ENOMEM;
-
-	strlcpy(i2cd->gpu_ccgx_ucsi->type, "ccgx-ucsi",
-		sizeof(i2cd->gpu_ccgx_ucsi->type));
-	i2cd->gpu_ccgx_ucsi->addr = 0x8;
-	i2cd->gpu_ccgx_ucsi->irq = irq;
-	i2cd->gpu_ccgx_ucsi->swnode = &ccgx_node;
-	i2cd->ccgx_client = i2c_new_client_device(&i2cd->adapter, i2cd->gpu_ccgx_ucsi);
-	return PTR_ERR_OR_ZERO(i2cd->ccgx_client);
-}
-
 static int gpu_i2c_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
 	struct gpu_i2c_dev *i2cd;
@@ -328,9 +313,10 @@ static int gpu_i2c_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (status < 0)
 		goto free_irq_vectors;
 
-	status = gpu_populate_client(i2cd, pdev->irq);
-	if (status < 0) {
-		dev_err(&pdev->dev, "gpu_populate_client failed %d\n", status);
+	i2cd->ccgx_client = i2c_new_ccgx_ucsi(&i2cd->adapter, pdev->irq, &ccgx_node);
+	if (IS_ERR(i2cd->ccgx_client)) {
+		status = dev_err_probe(&pdev->dev, PTR_ERR(i2cd->ccgx_client),
+				       "register UCSI failed\n");
 		goto del_adapter;
 	}
 
-- 
2.33.0

[PATCH v1 10/11] i2c: nvidia-gpu: Use temporary variable for struct device

[Andy Shevchenko]

Use temporary variable for struct device to make code neater.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/i2c-nvidia-gpu.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/i2c/busses/i2c-nvidia-gpu.c b/drivers/i2c/busses/i2c-nvidia-gpu.c
index 8117c3674209..a82be377146e 100644
--- a/drivers/i2c/busses/i2c-nvidia-gpu.c
+++ b/drivers/i2c/busses/i2c-nvidia-gpu.c
@@ -270,19 +270,20 @@ static const struct software_node ccgx_node = {
 
 static int gpu_i2c_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
+	struct device *dev = &pdev->dev;
 	struct gpu_i2c_dev *i2cd;
 	int status;
 
-	i2cd = devm_kzalloc(&pdev->dev, sizeof(*i2cd), GFP_KERNEL);
+	i2cd = devm_kzalloc(dev, sizeof(*i2cd), GFP_KERNEL);
 	if (!i2cd)
 		return -ENOMEM;
 
-	i2cd->dev = &pdev->dev;
-	dev_set_drvdata(&pdev->dev, i2cd);
+	i2cd->dev = dev;
+	dev_set_drvdata(dev, i2cd);
 
 	status = pcim_enable_device(pdev);
 	if (status < 0) {
-		dev_err(&pdev->dev, "pcim_enable_device failed %d\n", status);
+		dev_err(dev, "pcim_enable_device failed %d\n", status);
 		return status;
 	}
 
@@ -290,13 +291,13 @@ static int gpu_i2c_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 
 	i2cd->regs = pcim_iomap(pdev, 0, 0);
 	if (!i2cd->regs) {
-		dev_err(&pdev->dev, "pcim_iomap failed\n");
+		dev_err(dev, "pcim_iomap failed\n");
 		return -ENOMEM;
 	}
 
 	status = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSI);
 	if (status < 0) {
-		dev_err(&pdev->dev, "pci_alloc_irq_vectors err %d\n", status);
+		dev_err(dev, "pci_alloc_irq_vectors err %d\n", status);
 		return status;
 	}
 
@@ -308,22 +309,21 @@ static int gpu_i2c_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		sizeof(i2cd->adapter.name));
 	i2cd->adapter.algo = &gpu_i2c_algorithm;
 	i2cd->adapter.quirks = &gpu_i2c_quirks;
-	i2cd->adapter.dev.parent = &pdev->dev;
+	i2cd->adapter.dev.parent = dev;
 	status = i2c_add_adapter(&i2cd->adapter);
 	if (status < 0)
 		goto free_irq_vectors;
 
 	i2cd->ccgx_client = i2c_new_ccgx_ucsi(&i2cd->adapter, pdev->irq, &ccgx_node);
 	if (IS_ERR(i2cd->ccgx_client)) {
-		status = dev_err_probe(&pdev->dev, PTR_ERR(i2cd->ccgx_client),
-				       "register UCSI failed\n");
+		status = dev_err_probe(dev, PTR_ERR(i2cd->ccgx_client), "register UCSI failed\n");
 		goto del_adapter;
 	}
 
-	pm_runtime_set_autosuspend_delay(&pdev->dev, 3000);
-	pm_runtime_use_autosuspend(&pdev->dev);
-	pm_runtime_put_autosuspend(&pdev->dev);
-	pm_runtime_allow(&pdev->dev);
+	pm_runtime_set_autosuspend_delay(dev, 3000);
+	pm_runtime_use_autosuspend(dev);
+	pm_runtime_put_autosuspend(dev);
+	pm_runtime_allow(dev);
 
 	return 0;
 
@@ -336,7 +336,7 @@ static int gpu_i2c_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 
 static void gpu_i2c_remove(struct pci_dev *pdev)
 {
-	struct gpu_i2c_dev *i2cd = dev_get_drvdata(&pdev->dev);
+	struct gpu_i2c_dev *i2cd = pci_get_drvdata(pdev);
 
 	pm_runtime_get_noresume(i2cd->dev);
 	i2c_del_adapter(&i2cd->adapter);
-- 
2.33.0

[PATCH v1 11/11] i2c: nvidia-gpu: Convert to use dev_err_probe()

[Andy Shevchenko]

It's fine to call dev_err_probe() in ->probe() when error code is known.
Convert the driver to use dev_err_probe().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/busses/i2c-nvidia-gpu.c | 18 ++++++------------
 1 file changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/i2c/busses/i2c-nvidia-gpu.c b/drivers/i2c/busses/i2c-nvidia-gpu.c
index a82be377146e..6920c1b9a126 100644
--- a/drivers/i2c/busses/i2c-nvidia-gpu.c
+++ b/drivers/i2c/busses/i2c-nvidia-gpu.c
@@ -282,24 +282,18 @@ static int gpu_i2c_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	dev_set_drvdata(dev, i2cd);
 
 	status = pcim_enable_device(pdev);
-	if (status < 0) {
-		dev_err(dev, "pcim_enable_device failed %d\n", status);
-		return status;
-	}
+	if (status < 0)
+		return dev_err_probe(dev, status, "pcim_enable_device failed\n");
 
 	pci_set_master(pdev);
 
 	i2cd->regs = pcim_iomap(pdev, 0, 0);
-	if (!i2cd->regs) {
-		dev_err(dev, "pcim_iomap failed\n");
-		return -ENOMEM;
-	}
+	if (!i2cd->regs)
+		return dev_err_probe(dev, -ENOMEM, "pcim_iomap failed\n");
 
 	status = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSI);
-	if (status < 0) {
-		dev_err(dev, "pci_alloc_irq_vectors err %d\n", status);
-		return status;
-	}
+	if (status < 0)
+		return dev_err_probe(dev, status, "pci_alloc_irq_vectors err\n");
 
 	gpu_enable_i2c_bus(i2cd);
 
-- 
2.33.0

Re: [PATCH v1 04/11] i2c: designware-pci: Convert to use dev_err_probe()

[Jarkko Nikula]

On 12/7/21 21:21, Andy Shevchenko wrote:
> +	if (id->driver_data >= ARRAY_SIZE(dw_pci_controllers))
> +		return dev_err_probe(dev, -EINVAL, "invalid driver data %ld\n", id->driver_data);
>   
I know checkpatch.pl doesn't complain this but for my taste readability 
would be a bit better if error causing id->driver_data is split into 
another line.

Jarkko

Re: [PATCH v1 01/11] i2c: Introduce common module to instantiate CCGx UCSI

[Jarkko Nikula]

On 12/7/21 21:21, Andy Shevchenko wrote:
> Introduce a common module to provide an API to instantiate UCSI device
> for Cypress CCGx Type-C controller. Individual bus drivers need to select
> this one on demand.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>   drivers/i2c/busses/Kconfig         |  7 +++++++
>   drivers/i2c/busses/Makefile        |  3 +++
>   drivers/i2c/busses/i2c-ccgx-ucsi.c | 27 +++++++++++++++++++++++++++
>   drivers/i2c/busses/i2c-ccgx-ucsi.h | 11 +++++++++++
>   4 files changed, 48 insertions(+)
>   create mode 100644 drivers/i2c/busses/i2c-ccgx-ucsi.c
>   create mode 100644 drivers/i2c/busses/i2c-ccgx-ucsi.h
> 
I've mixed feelings about this set. I'd either put patches 3-8 first 
since e.g. 6/11 and 8/11 are fixing existing issues or even better to 
split CCGx UCSI stuff into another set.

Jarkko

Re: [PATCH v1 06/11] i2c: designware-pci: Fix to change data types of hcnt and lcnt parameters

[Jarkko Nikula]

On 12/7/21 21:21, Andy Shevchenko wrote:
> From: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> 
> The data type of hcnt and lcnt in the struct dw_i2c_dev is of type u16.
> It's better to have same data type in struct dw_scl_sda_cfg as well.
> 
> Signed-off-by: Lakshmi Sowjanya D <lakshmi.sowjanya.d@intel.com>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>   drivers/i2c/busses/i2c-designware-pcidrv.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
> index 66dc765b6834..a0ea71e71886 100644
> --- a/drivers/i2c/busses/i2c-designware-pcidrv.c
> +++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
> @@ -40,10 +40,10 @@ enum dw_pci_ctl_id_t {
>   };
>   
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

Re: [PATCH v1 08/11] i2c: designware-pci: Add a note about struct dw_scl_sda_cfg usage

[Jarkko Nikula]

On 12/7/21 21:21, Andy Shevchenko wrote:
> Add a note about struct dw_scl_sda_cfg usage to discourage people
> of using this structure on new platforms. Instead they should try
> hard to put the needed information into firmware descriptions.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>   drivers/i2c/busses/i2c-designware-pcidrv.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

Re: [PATCH v1 07/11] i2c: designware-pci: Group MODULE_*() macros

[Jarkko Nikula]

On 12/7/21 21:21, Andy Shevchenko wrote:
> For better maintenance group MODULE_*() macros together.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>   drivers/i2c/busses/i2c-designware-pcidrv.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
> 
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

Re: [PATCH v1 05/11] i2c: designware-pci: use __maybe_unused for PM functions

[Jarkko Nikula]

On 12/7/21 21:21, Andy Shevchenko wrote:
> Use __maybe_unused for PM functions instead of ifdeffery.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>   drivers/i2c/busses/i2c-designware-pcidrv.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/i2c/busses/i2c-designware-pcidrv.c b/drivers/i2c/busses/i2c-designware-pcidrv.c
> index e6b4b1a468da..66dc765b6834 100644
> --- a/drivers/i2c/busses/i2c-designware-pcidrv.c
> +++ b/drivers/i2c/busses/i2c-designware-pcidrv.c
> @@ -187,8 +187,7 @@ static struct dw_pci_controller dw_pci_controllers[] = {
>   	},
>   };
>   
Acked-by: Jarkko Nikula <jarkko.nikula@linux.intel.com>

Re: [PATCH v1 03/11] i2c: designware-pci: Use temporary variable for struct device

[Jarkko Nikula]

On 12/7/21 21:21, Andy Shevchenko wrote:
> Use temporary variable for struct device to make code neater.
> 
> While at it, rename variable of struct dw_i2c_dev pointer to i_dev.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>   drivers/i2c/busses/i2c-designware-pcidrv.c | 99 +++++++++++-----------
>   1 file changed, 50 insertions(+), 49 deletions(-)
> 
I think struct dw_i2c_dev *dev to *i_dev renaming would be better to do 
consistently through the all drivers/i2c/busses/i2c-designware-* or 
leave as is?

Jarkko

Re: [PATCH v1 01/11] i2c: Introduce common module to instantiate CCGx UCSI

[Andy Shevchenko]

On Wed, Dec 08, 2021 at 02:29:04PM +0200, Jarkko Nikula wrote:
> On 12/7/21 21:21, Andy Shevchenko wrote:
> > Introduce a common module to provide an API to instantiate UCSI device
> > for Cypress CCGx Type-C controller. Individual bus drivers need to select
> > this one on demand.
> > 
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> > ---
> >   drivers/i2c/busses/Kconfig         |  7 +++++++
> >   drivers/i2c/busses/Makefile        |  3 +++
> >   drivers/i2c/busses/i2c-ccgx-ucsi.c | 27 +++++++++++++++++++++++++++
> >   drivers/i2c/busses/i2c-ccgx-ucsi.h | 11 +++++++++++
> >   4 files changed, 48 insertions(+)
> >   create mode 100644 drivers/i2c/busses/i2c-ccgx-ucsi.c
> >   create mode 100644 drivers/i2c/busses/i2c-ccgx-ucsi.h
> > 
> I've mixed feelings about this set. I'd either put patches 3-8 first since
> e.g. 6/11 and 8/11 are fixing existing issues or even better to split CCGx
> UCSI stuff into another set.

I have sent v2 with DesignWare patches only and no conversion part included.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 01/11] i2c: Introduce common module to instantiate CCGx UCSI

[Shah, Nehal-bakulchandra]

Hi Andy,

On 12/13/2021 11:30 PM, Andy Shevchenko wrote:
> On Wed, Dec 08, 2021 at 02:29:04PM +0200, Jarkko Nikula wrote:
>> On 12/7/21 21:21, Andy Shevchenko wrote:
>>> Introduce a common module to provide an API to instantiate UCSI device
>>> for Cypress CCGx Type-C controller. Individual bus drivers need to select
>>> this one on demand.
>>>
>>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>>> ---
>>>    drivers/i2c/busses/Kconfig         |  7 +++++++
>>>    drivers/i2c/busses/Makefile        |  3 +++
>>>    drivers/i2c/busses/i2c-ccgx-ucsi.c | 27 +++++++++++++++++++++++++++
>>>    drivers/i2c/busses/i2c-ccgx-ucsi.h | 11 +++++++++++
>>>    4 files changed, 48 insertions(+)
>>>    create mode 100644 drivers/i2c/busses/i2c-ccgx-ucsi.c
>>>    create mode 100644 drivers/i2c/busses/i2c-ccgx-ucsi.h
>>>
>> I've mixed feelings about this set. I'd either put patches 3-8 first since
>> e.g. 6/11 and 8/11 are fixing existing issues or even better to split CCGx
>> UCSI stuff into another set.
> 
> I have sent v2 with DesignWare patches only and no conversion part included.
> 

It will be good we can take this patch also in this series. This is more 
nicer and cleaner solution. That said, we are working futuristic 
platform where CCGX is connected over system  i2c of our platform i.e 
AMDI0010 so in this case from designware platform i2c driver we will 
have to probe the CCGX driver.

Regards
Nehal