2021-12-07 19:22:10

by Andy Shevchenko

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

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 <[email protected]>
---
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



2021-12-07 19:22:13

by Andy Shevchenko

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

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 <[email protected]>
---
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


2021-12-07 19:22:16

by Andy Shevchenko

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

Use temporary variable for struct device to make code neater.

Signed-off-by: Andy Shevchenko <[email protected]>
---
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


2021-12-07 19:22:19

by Andy Shevchenko

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

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 <[email protected]>
---
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


2021-12-07 19:22:22

by Andy Shevchenko

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

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

Signed-off-by: Andy Shevchenko <[email protected]>
---
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


2021-12-07 19:22:24

by Andy Shevchenko

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

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

Signed-off-by: Andy Shevchenko <[email protected]>
---
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


2021-12-07 19:22:25

by Andy Shevchenko

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

Use __maybe_unused for PM functions instead of ifdeffery.

Signed-off-by: Andy Shevchenko <[email protected]>
---
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


2021-12-07 19:22:29

by Andy Shevchenko

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

From: Lakshmi Sowjanya D <[email protected]>

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 <[email protected]>
Signed-off-by: Andy Shevchenko <[email protected]>
---
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


2021-12-07 19:22:31

by Andy Shevchenko

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

For better maintenance group MODULE_*() macros together.

Signed-off-by: Andy Shevchenko <[email protected]>
---
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 <[email protected]>");
MODULE_DESCRIPTION("Synopsys DesignWare PCI I2C bus adapter");
MODULE_LICENSE("GPL");
--
2.33.0


2021-12-07 19:22:32

by Andy Shevchenko

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

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 <[email protected]>
---
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


2021-12-08 12:21:13

by Jarkko Nikula

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

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

2021-12-08 12:29:09

by Jarkko Nikula

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

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 <[email protected]>
> ---
> 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

2021-12-08 12:29:57

by Jarkko Nikula

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

On 12/7/21 21:21, Andy Shevchenko wrote:
> From: Lakshmi Sowjanya D <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> 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 <[email protected]>

2021-12-08 12:30:06

by Jarkko Nikula

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

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 <[email protected]>
> ---
> drivers/i2c/busses/i2c-designware-pcidrv.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
Acked-by: Jarkko Nikula <[email protected]>

2021-12-08 12:30:36

by Jarkko Nikula

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

On 12/7/21 21:21, Andy Shevchenko wrote:
> For better maintenance group MODULE_*() macros together.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> drivers/i2c/busses/i2c-designware-pcidrv.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
Acked-by: Jarkko Nikula <[email protected]>

2021-12-08 12:31:26

by Jarkko Nikula

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

On 12/7/21 21:21, Andy Shevchenko wrote:
> Use __maybe_unused for PM functions instead of ifdeffery.
>
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> 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 <[email protected]>

2021-12-13 18:05:40

by Andy Shevchenko

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

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 <[email protected]>
> > ---
> > 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



2021-12-15 13:50:30

by Shah, Nehal-bakulchandra

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

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 <[email protected]>
>>> ---
>>> 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