diff mbox series

[v3] i2c: add i2c bus driver for amd navi gpu

Message ID 1607446104-58597-1-git-send-email-sanju.mehta@amd.com
State Changes Requested
Headers show
Series [v3] i2c: add i2c bus driver for amd navi gpu | expand

Commit Message

Sanjay R Mehta Dec. 8, 2020, 4:48 p.m. UTC
From: Sanjay R Mehta <Sanju.Mehta@amd.com>

Latest amdgpu card has USB Type-C interface. There is a Type-C controller
which can be accessed over I2C.

This driver adds I2C bus driver to communicate with Type-C controller. I2C
client driver will be part of USB Type-C UCSI driver.

Signed-off-by: Sanjay R Mehta <Sanju.Mehta@amd.com>
Signed-off-by: Nehal Bakulchandra Shah <Nehal-Bakulchandra.Shah@amd.com>
---

Changes in v3:
- made changes to use regmap mmio as suggested by Krzysztof Kozlowski.

Changes in v2:

- converted the code to use regmap, read_poll_timeout and made some cosmetic
  changes as suggested by Andy Shevchenko.
---
 MAINTAINERS                          |   7 +
 drivers/i2c/busses/Kconfig           |   9 +
 drivers/i2c/busses/Makefile          |   1 +
 drivers/i2c/busses/i2c-amdgpu-navi.c | 325 +++++++++++++++++++++++++++
 4 files changed, 342 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-amdgpu-navi.c

Comments

Andy Shevchenko Dec. 8, 2020, 6:07 p.m. UTC | #1
On Tue, Dec 08, 2020 at 10:48:24AM -0600, Sanjay R Mehta wrote:
> From: Sanjay R Mehta <Sanju.Mehta@amd.com>

Please, don't send new versions too often. Give a chance others to review.

> Latest amdgpu card has USB Type-C interface. There is a Type-C controller
> which can be accessed over I2C.

This is confusing. Does it mean Type-C controller is being controlled via I²C
or this is a bridge I²C-to-Type-C (of course I barely believe in the latter but
who knows)?

> This driver adds I2C bus driver to communicate with Type-C controller. I2C
> client driver will be part of USB Type-C UCSI driver.

Is this IP is from Synopsys? Can you clarify it here?

...

>  drivers/i2c/busses/i2c-amdgpu-navi.c | 325 +++++++++++++++++++++++++++

If the answer is yes to the above Q, I would rather see this file to be in
i2c-designware namespace (and going further perhaps even part of its PCI
glue driver (with corresponding configuration option taken into account).

...

> +#define AMD_UCSI_INTR_EN	0xD
> +#define AMD_UCSI_INTR_REG	0x474
> +#define AMD_MASTERCFG_MASK	GENMASK(15, 0)

So, this sounds like a custom register space on top of Synopsys' one, correct?

...

> +struct amdgpu_i2c_dev {

> +	void __iomem		*regs;
> +	struct regmap		*map;
> +	struct device		*dev;
> +	u32			master_cfg;
> +	u32			slave_adr;
> +	u32			tx_fifo_depth;
> +	u32			rx_fifo_depth;
> +	u16			ss_hcnt;
> +	u16			ss_lcnt;
> +	struct i2c_adapter	adapter;

Can you rather embed struct dw_i2c_dev here instead of all above?

> +	struct i2c_board_info	*gpu_ccgx_ucsi;
> +	struct i2c_client	*ccgx_client;
> +};

...

> +static void amdgpu_configure_i2c_bus(struct amdgpu_i2c_dev *i2cd)
> +{
> +	u16 icon;
> +	u32 reg;
> +
> +	/* First disable the controller */
> +	regmap_write(i2cd->map, DW_IC_ENABLE, 0);
> +	i2cd->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE | DW_IC_CON_RESTART_EN |
> +			   DW_IC_CON_SPEED_STD;
> +
> +	/* clear all the interrupts */
> +	regmap_read(i2cd->map, DW_IC_CLR_INTR, &reg);
> +	regmap_write(i2cd->map, DW_IC_INTR_MASK, 0);
> +
> +	icon = i2cd->master_cfg & AMD_MASTERCFG_MASK;
> +	icon &= ~BIT(3);
> +	icon &= ~DW_IC_CON_10BITADDR_MASTER;
> +	icon = icon | DW_IC_CON_SPEED_STD;
> +	/* configure the master */
> +	regmap_write(i2cd->map, DW_IC_CON, icon);
> +	/* configure the FIFO */
> +	i2cd->tx_fifo_depth = 32;
> +	i2cd->rx_fifo_depth = 32;
> +	regmap_write(i2cd->map, DW_IC_TX_TL, i2cd->tx_fifo_depth);
> +	regmap_write(i2cd->map, DW_IC_RX_TL, i2cd->rx_fifo_depth);
> +
> +	/* setup 100k Speed */
> +	i2cd->ss_hcnt = 430;
> +	i2cd->ss_lcnt = 570;
> +	regmap_write(i2cd->map, DW_IC_SS_SCL_HCNT, i2cd->ss_hcnt);
> +	regmap_write(i2cd->map, DW_IC_SS_SCL_LCNT, i2cd->ss_lcnt);
> +	/* setup the slave address */
> +	i2cd->slave_adr = 0x08;
> +	regmap_write(i2cd->map, DW_IC_TAR, i2cd->slave_adr);
> +
> +	/* Now Enable the controller */
> +	regmap_write(i2cd->map, DW_IC_ENABLE, 1);
> +}

Can you use existing code from i2c-designware-common.c?

(For instance, i2c_dw_scl_hcnt() / i2c_dw_scl_lcnt() for counters,
 i2c_dw_set_fifo_size() for FIFO depth)

...

> +static int amdgpu_i2c_check_activity(struct amdgpu_i2c_dev *i2cd)
> +{
> +	u32 val;
> +	int ret;
> +
> +	ret = regmap_read_poll_timeout(i2cd->map, DW_IC_STATUS, val,
> +				       !(val & DW_IC_STATUS_ACTIVITY),
> +				       1100, 20000);
> +	if (ret) {
> +		dev_err(i2cd->dev, "i2c timeout error %x\n", val);
> +		return -ETIMEDOUT;
> +	}
> +
> +	regmap_read(i2cd->map, DW_IC_STATUS, &val);
> +	if (val & DW_IC_STATUS_ACTIVITY)
> +		return -ETIMEDOUT;
> +
> +	return 0;
> +}

Isn't it a dup of i2c_dw_wait_bus_not_busy()?

...

> +/* Polling based xfer routine */
> +static int amdgpu_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
> +{
> +	struct amdgpu_i2c_dev *i2cd = i2c_get_adapdata(adap);
> +	int i, j, len, k;
> +	int cmd = 0;
> +	int status;
> +	u8 *buf;
> +	u32 val;
> +
> +	amdgpu_configure_i2c_bus(i2cd);
> +
> +	for (i = 0; i < num; i++) {
> +		buf = msgs[i].buf;
> +		len = msgs[i].len;
> +
> +		if (!(msgs[i].flags & I2C_M_RD))
> +			regmap_write(i2cd->map, DW_IC_TX_TL, len - 1);
> +
> +		for (j = len; j > 0; j--) {
> +			if (i == num - 1 && j == 1)

> +				cmd |= BIT(9);

What is this magic for?

> +
> +			if (msgs[i].flags & I2C_M_RD) {
> +				regmap_write(i2cd->map, DW_IC_DATA_CMD, 0x100);
> +				regmap_write(i2cd->map, DW_IC_DATA_CMD, 0x100 | cmd);
> +				if (cmd) {
> +					regmap_write(i2cd->map, DW_IC_TX_TL, 2 * (len - 1));
> +					regmap_write(i2cd->map, DW_IC_RX_TL, 2 * (len - 1));
> +					status = amdgpu_i2c_status(i2cd);
> +					if (status)
> +						return -ETIMEDOUT;
> +
> +					for (k = 0; k < len; k++) {
> +						regmap_read(i2cd->map, DW_IC_DATA_CMD, &val);
> +						buf[k] = val;
> +					}
> +					status = amdgpu_i2c_check_stopbit(i2cd);
> +					if (status)
> +						return -ETIMEDOUT;
> +				}
> +			} else {
> +				regmap_write(i2cd->map, DW_IC_DATA_CMD, *buf++ | cmd);
> +				usleep_range(10000, 11000);
> +			}
> +		}
> +		status = amdgpu_i2c_check_stopbit(i2cd);
> +		if (status)
> +			return -ETIMEDOUT;
> +	}
> +
> +	return 0;
> +}

...

> +static u32 amdgpu_i2c_functionality(struct i2c_adapter *adap)
> +{
> +	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
> +}
> +
> +static const struct i2c_algorithm amdgpu_i2c_algorithm = {
> +	.master_xfer	= amdgpu_i2c_master_xfer,
> +	.functionality	= amdgpu_i2c_functionality,
> +};

Can you utilize i2c_dw_func() along with functionality member of struct
dw_i2c_dev?

...

> +static const struct pci_device_id amdgpu_i2c_ids[] = {
> +		{PCI_VDEVICE(ATI, 0x7314)},
> +		{PCI_VDEVICE(ATI, 0x73A4)},
> +		{PCI_VDEVICE(ATI, 0x73C4)},
> +		{PCI_VDEVICE(ATI, 0x73E4)},

Two issues with indentation:
- extra TAB
- missed surrounding spaces inside {} of each record.

> +		{ }
> +};
> +MODULE_DEVICE_TABLE(pci, amdgpu_i2c_ids);

Move it closer to the actual user of the table.

...

> +static int amdgpu_populate_client(struct amdgpu_i2c_dev *i2cd, int irq)
> +{
> +	i2cd->gpu_ccgx_ucsi = devm_kzalloc(i2cd->dev,
> +					   sizeof(*i2cd->gpu_ccgx_ucsi),
> +					   GFP_KERNEL);

Do you really need this to be alive for entire lifetime of the device
bound to the driver?

> +	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->ccgx_client = i2c_new_client_device(&i2cd->adapter, i2cd->gpu_ccgx_ucsi);
> +	if (!i2cd->ccgx_client)
> +		return -ENODEV;
> +
> +	return 0;

Read documentation: "This returns ... or an ERR_PTR to describe the error."

> +}

...

> +	pci_set_master(pdev);

Why?

...

> +	status = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);

Do you know what type of IRQ your hardware handles?

> +	if (status < 0) {
> +		dev_err(&pdev->dev, "pci_alloc_irq_vectors err %d\n", status);
> +		return status;
> +	}
> +
> +	irq = pci_irq_vector(pdev, 0);

...

> +	/* Enable ucsi interrupt */

Please, spell abbreviation capitalized everywhere, like UCSI in this case.

...

> +static void amdgpu_i2c_remove(struct pci_dev *pdev)
> +{
> +	struct amdgpu_i2c_dev *i2cd = dev_get_drvdata(&pdev->dev);
> +
> +	regmap_write(i2cd->map, AMD_UCSI_INTR_REG, 0);
> +	i2c_del_adapter(&i2cd->adapter);
> +	pci_free_irq_vectors(pdev);

Here is the thing. You better not interleave devm_*() / pci_m*() / etc with
regular calls because here you have an ordering issues.

OTOH it may be grouped like all managed followed by the rest which is regular
calls. And yes, here I'm talking about ->probe().

> +}
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 190c7fa2ea01..93894459a4c8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -8119,6 +8119,13 @@  L:	linux-acpi@vger.kernel.org
 S:	Maintained
 F:	drivers/i2c/i2c-core-acpi.c
 
+I2C CONTROLLER DRIVER FOR AMD GPU
+M:	Nehal Bakulchandra Shah <Nehal-Bakulchandra.Shah@amd.com>
+M:	Sanjay R Mehta <sanju.mehta@amd.com>
+L:	linux-i2c@vger.kernel.org
+S:	Maintained
+F:	drivers/i2c/busses/i2c-amdgpu-navi.*
+
 I2C CONTROLLER DRIVER FOR NVIDIA GPU
 M:	Ajay Gupta <ajayg@nvidia.com>
 L:	linux-i2c@vger.kernel.org
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 293e7a0760e7..0ff369c0f41f 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -88,6 +88,15 @@  config I2C_AMD_MP2
 	  This driver can also be built as modules.  If so, the modules will
 	  be called i2c-amd-mp2-pci and i2c-amd-mp2-plat.
 
+config I2C_AMDGPU_NAVI
+	tristate "AMDGPU NAVI I2C controller"
+	depends on PCI
+	help
+	  If you say yes to this option, support will be included for the
+	  NAVI I2C controller which is used to communicate with the GPU's
+	  Type-C controller. This driver can also be built as a module called
+	  i2c-amdgpu-navi.
+
 config I2C_HIX5HD2
 	tristate "Hix5hd2 high-speed I2C driver"
 	depends on ARCH_HISI || ARCH_HIX5HD2 || COMPILE_TEST
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 19aff0e45cb5..f599473a8ad9 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -13,6 +13,7 @@  obj-$(CONFIG_I2C_ALI15X3)	+= i2c-ali15x3.o
 obj-$(CONFIG_I2C_AMD756)	+= i2c-amd756.o
 obj-$(CONFIG_I2C_AMD756_S4882)	+= i2c-amd756-s4882.o
 obj-$(CONFIG_I2C_AMD8111)	+= i2c-amd8111.o
+obj-$(CONFIG_I2C_AMDGPU_NAVI)	+= i2c-amdgpu-navi.o
 obj-$(CONFIG_I2C_CHT_WC)	+= i2c-cht-wc.o
 obj-$(CONFIG_I2C_I801)		+= i2c-i801.o
 obj-$(CONFIG_I2C_ISCH)		+= i2c-isch.o
diff --git a/drivers/i2c/busses/i2c-amdgpu-navi.c b/drivers/i2c/busses/i2c-amdgpu-navi.c
new file mode 100644
index 000000000000..d129b691bc23
--- /dev/null
+++ b/drivers/i2c/busses/i2c-amdgpu-navi.c
@@ -0,0 +1,325 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+//
+// AMD I2C Controller Driver for Navi GPU's
+//
+// Copyright (c) 2020, Advanced Micro Devices, Inc.
+//
+// Authors:
+//	Nehal Bakulchandra Shah <Nehal-Bakulchandra.Shah@amd.com>
+//	Sanjay R Mehta <Sanju.Mehta@amd.com>
+
+#include <linux/bits.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/platform_device.h>
+#include <linux/pm.h>
+#include <linux/regmap.h>
+#include <asm/unaligned.h>
+#include "i2c-designware-core.h"
+
+#define AMD_UCSI_INTR_EN	0xD
+#define AMD_UCSI_INTR_REG	0x474
+#define AMD_MASTERCFG_MASK	GENMASK(15, 0)
+
+struct amdgpu_i2c_dev {
+	void __iomem		*regs;
+	struct regmap		*map;
+	struct device		*dev;
+	u32			master_cfg;
+	u32			slave_adr;
+	u32			tx_fifo_depth;
+	u32			rx_fifo_depth;
+	u16			ss_hcnt;
+	u16			ss_lcnt;
+	struct i2c_adapter	adapter;
+	struct i2c_board_info	*gpu_ccgx_ucsi;
+	struct i2c_client	*ccgx_client;
+};
+
+struct regmap_config map_cfg = {
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+	.disable_locking = true,
+	.max_register = AMD_UCSI_INTR_REG,
+};
+
+static void amdgpu_configure_i2c_bus(struct amdgpu_i2c_dev *i2cd)
+{
+	u16 icon;
+	u32 reg;
+
+	/* First disable the controller */
+	regmap_write(i2cd->map, DW_IC_ENABLE, 0);
+	i2cd->master_cfg = DW_IC_CON_MASTER | DW_IC_CON_SLAVE_DISABLE | DW_IC_CON_RESTART_EN |
+			   DW_IC_CON_SPEED_STD;
+
+	/* clear all the interrupts */
+	regmap_read(i2cd->map, DW_IC_CLR_INTR, &reg);
+	regmap_write(i2cd->map, DW_IC_INTR_MASK, 0);
+
+	icon = i2cd->master_cfg & AMD_MASTERCFG_MASK;
+	icon &= ~BIT(3);
+	icon &= ~DW_IC_CON_10BITADDR_MASTER;
+	icon = icon | DW_IC_CON_SPEED_STD;
+	/* configure the master */
+	regmap_write(i2cd->map, DW_IC_CON, icon);
+	/* configure the FIFO */
+	i2cd->tx_fifo_depth = 32;
+	i2cd->rx_fifo_depth = 32;
+	regmap_write(i2cd->map, DW_IC_TX_TL, i2cd->tx_fifo_depth);
+	regmap_write(i2cd->map, DW_IC_RX_TL, i2cd->rx_fifo_depth);
+
+	/* setup 100k Speed */
+	i2cd->ss_hcnt = 430;
+	i2cd->ss_lcnt = 570;
+	regmap_write(i2cd->map, DW_IC_SS_SCL_HCNT, i2cd->ss_hcnt);
+	regmap_write(i2cd->map, DW_IC_SS_SCL_LCNT, i2cd->ss_lcnt);
+	/* setup the slave address */
+	i2cd->slave_adr = 0x08;
+	regmap_write(i2cd->map, DW_IC_TAR, i2cd->slave_adr);
+
+	/* Now Enable the controller */
+	regmap_write(i2cd->map, DW_IC_ENABLE, 1);
+}
+
+static int amdgpu_i2c_check_activity(struct amdgpu_i2c_dev *i2cd)
+{
+	u32 val;
+	int ret;
+
+	ret = regmap_read_poll_timeout(i2cd->map, DW_IC_STATUS, val,
+				       !(val & DW_IC_STATUS_ACTIVITY),
+				       1100, 20000);
+	if (ret) {
+		dev_err(i2cd->dev, "i2c timeout error %x\n", val);
+		return -ETIMEDOUT;
+	}
+
+	regmap_read(i2cd->map, DW_IC_STATUS, &val);
+	if (val & DW_IC_STATUS_ACTIVITY)
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
+static int amdgpu_i2c_check_stopbit(struct amdgpu_i2c_dev *i2cd)
+{
+	u32 val;
+	int ret;
+
+	ret = regmap_read_poll_timeout(i2cd->map, DW_IC_INTR_STAT, val,
+				       !(val & DW_IC_INTR_STOP_DET),
+				       1100, 20000);
+	if (ret) {
+		dev_err(i2cd->dev, "i2c timeout error %x\n", val);
+		return -ETIMEDOUT;
+	}
+
+	regmap_read(i2cd->map, DW_IC_CLR_INTR, &val);
+	if (val & DW_IC_INTR_STOP_DET)
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
+static int amdgpu_i2c_status(struct amdgpu_i2c_dev *i2cd)
+{
+	int status;
+
+	status = amdgpu_i2c_check_activity(i2cd);
+	if (status)
+		return -ETIMEDOUT;
+
+	status = amdgpu_i2c_check_stopbit(i2cd);
+	if (status)
+		return -ETIMEDOUT;
+
+	return status;
+}
+
+/* Polling based xfer routine */
+static int amdgpu_i2c_master_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
+{
+	struct amdgpu_i2c_dev *i2cd = i2c_get_adapdata(adap);
+	int i, j, len, k;
+	int cmd = 0;
+	int status;
+	u8 *buf;
+	u32 val;
+
+	amdgpu_configure_i2c_bus(i2cd);
+
+	for (i = 0; i < num; i++) {
+		buf = msgs[i].buf;
+		len = msgs[i].len;
+
+		if (!(msgs[i].flags & I2C_M_RD))
+			regmap_write(i2cd->map, DW_IC_TX_TL, len - 1);
+
+		for (j = len; j > 0; j--) {
+			if (i == num - 1 && j == 1)
+				cmd |= BIT(9);
+
+			if (msgs[i].flags & I2C_M_RD) {
+				regmap_write(i2cd->map, DW_IC_DATA_CMD, 0x100);
+				regmap_write(i2cd->map, DW_IC_DATA_CMD, 0x100 | cmd);
+				if (cmd) {
+					regmap_write(i2cd->map, DW_IC_TX_TL, 2 * (len - 1));
+					regmap_write(i2cd->map, DW_IC_RX_TL, 2 * (len - 1));
+					status = amdgpu_i2c_status(i2cd);
+					if (status)
+						return -ETIMEDOUT;
+
+					for (k = 0; k < len; k++) {
+						regmap_read(i2cd->map, DW_IC_DATA_CMD, &val);
+						buf[k] = val;
+					}
+					status = amdgpu_i2c_check_stopbit(i2cd);
+					if (status)
+						return -ETIMEDOUT;
+				}
+			} else {
+				regmap_write(i2cd->map, DW_IC_DATA_CMD, *buf++ | cmd);
+				usleep_range(10000, 11000);
+			}
+		}
+		status = amdgpu_i2c_check_stopbit(i2cd);
+		if (status)
+			return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static u32 amdgpu_i2c_functionality(struct i2c_adapter *adap)
+{
+	return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+static const struct i2c_algorithm amdgpu_i2c_algorithm = {
+	.master_xfer	= amdgpu_i2c_master_xfer,
+	.functionality	= amdgpu_i2c_functionality,
+};
+
+static const struct pci_device_id amdgpu_i2c_ids[] = {
+		{PCI_VDEVICE(ATI, 0x7314)},
+		{PCI_VDEVICE(ATI, 0x73A4)},
+		{PCI_VDEVICE(ATI, 0x73C4)},
+		{PCI_VDEVICE(ATI, 0x73E4)},
+		{ }
+};
+MODULE_DEVICE_TABLE(pci, amdgpu_i2c_ids);
+
+static int amdgpu_populate_client(struct amdgpu_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->ccgx_client = i2c_new_client_device(&i2cd->adapter, i2cd->gpu_ccgx_ucsi);
+	if (!i2cd->ccgx_client)
+		return -ENODEV;
+
+	return 0;
+}
+
+static int amdgpu_i2c_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+	struct amdgpu_i2c_dev *i2cd;
+	int status;
+	int irq;
+
+	i2cd = devm_kzalloc(&pdev->dev, sizeof(*i2cd), GFP_KERNEL);
+	if (!i2cd)
+		return -ENOMEM;
+
+	i2cd->dev = &pdev->dev;
+	dev_set_drvdata(&pdev->dev, i2cd);
+
+	status = pcim_enable_device(pdev);
+	if (status < 0) {
+		dev_err(&pdev->dev, "pcim_enable_device failed %d\n", status);
+		return status;
+	}
+
+	pci_set_master(pdev);
+
+	i2cd->regs = pcim_iomap(pdev, 0, 0);
+	if (!i2cd->regs) {
+		dev_err(&pdev->dev, "pcim_iomap failed\n");
+		return -ENOMEM;
+	}
+
+	status = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_ALL_TYPES);
+	if (status < 0) {
+		dev_err(&pdev->dev, "pci_alloc_irq_vectors err %d\n", status);
+		return status;
+	}
+
+	irq = pci_irq_vector(pdev, 0);
+
+	i2cd->map = devm_regmap_init_mmio(&pdev->dev, i2cd->regs, &map_cfg);
+	if (IS_ERR(i2cd->map)) {
+		dev_err(i2cd->dev, "Failed to init the registers map\n");
+		return PTR_ERR(i2cd->map);
+	}
+
+	/* Enable ucsi interrupt */
+	regmap_write(i2cd->map, AMD_UCSI_INTR_REG, AMD_UCSI_INTR_EN);
+	amdgpu_configure_i2c_bus(i2cd);
+	i2c_set_adapdata(&i2cd->adapter, i2cd);
+	i2cd->adapter.owner = THIS_MODULE;
+
+	strlcpy(i2cd->adapter.name, "AMDGPU NAVI I2C adapter", sizeof(i2cd->adapter.name));
+	i2cd->adapter.algo = &amdgpu_i2c_algorithm;
+	i2cd->adapter.dev.parent = &pdev->dev;
+
+	status = i2c_add_adapter(&i2cd->adapter);
+	if (status < 0)
+		goto free_irq_vectors;
+
+	status = amdgpu_populate_client(i2cd, irq);
+	if (status < 0) {
+		dev_err(&pdev->dev, "amdgpu_populate_client failed %d\n", status);
+		goto del_adapter;
+	}
+
+	return 0;
+
+del_adapter:
+	i2c_del_adapter(&i2cd->adapter);
+free_irq_vectors:
+	pci_clear_master(pdev);
+	pci_free_irq_vectors(pdev);
+	return status;
+}
+
+static void amdgpu_i2c_remove(struct pci_dev *pdev)
+{
+	struct amdgpu_i2c_dev *i2cd = dev_get_drvdata(&pdev->dev);
+
+	regmap_write(i2cd->map, AMD_UCSI_INTR_REG, 0);
+	i2c_del_adapter(&i2cd->adapter);
+	pci_free_irq_vectors(pdev);
+}
+
+static struct pci_driver amdgpu_i2c_driver = {
+	.name		= "i2c-amdgpu-navi",
+	.id_table	= amdgpu_i2c_ids,
+	.probe		= amdgpu_i2c_probe,
+	.remove		= amdgpu_i2c_remove,
+};
+module_pci_driver(amdgpu_i2c_driver);
+
+MODULE_DESCRIPTION("AMD I2C Controller Driver for Navi GPUs");
+MODULE_LICENSE("GPL");