* [PATCH v2] i2c: add i2c bus driver for amd navi gpu
@ 2020-12-08 12:37 Sanjay R Mehta
2020-12-08 12:47 ` Krzysztof Kozlowski
0 siblings, 1 reply; 4+ messages in thread
From: Sanjay R Mehta @ 2020-12-08 12:37 UTC (permalink / raw)
To: wsa+renesas, jarkko.nikula, andriy.shevchenko, jdelvare,
Sergey.Semin, krzk, kblaiech, loic.poulain, rppt,
bjorn.andersson, linux, vadimp, tali.perry1
Cc: linux-i2c, Sanjay R Mehta, Nehal Bakulchandra Shah
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 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 | 345 +++++++++++++++++++++++++++
4 files changed, 362 insertions(+)
create mode 100644 drivers/i2c/busses/i2c-amdgpu-navi.c
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..3922b8aebc26
--- /dev/null
+++ b/drivers/i2c/busses/i2c-amdgpu-navi.c
@@ -0,0 +1,345 @@
+// 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;
+};
+
+static int amdgpu_i2c_read(void *context, unsigned int reg, unsigned int *val)
+{
+ struct amdgpu_i2c_dev *i2cd = context;
+
+ *val = readl_relaxed(i2cd->regs + reg);
+
+ return 0;
+}
+
+static int amdgpu_i2c_write(void *context, unsigned int reg, unsigned int val)
+{
+ struct amdgpu_i2c_dev *i2cd = context;
+
+ writel_relaxed(val, i2cd->regs + reg);
+
+ return 0;
+}
+
+struct regmap_config map_cfg = {
+ .reg_bits = 32,
+ .val_bits = 32,
+ .reg_stride = 4,
+ .disable_locking = true,
+ .reg_read = amdgpu_i2c_read,
+ .reg_write = amdgpu_i2c_write,
+ .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, ®);
+ 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(i2cd->dev, NULL, i2cd, &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");
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] i2c: add i2c bus driver for amd navi gpu
2020-12-08 12:37 [PATCH v2] i2c: add i2c bus driver for amd navi gpu Sanjay R Mehta
@ 2020-12-08 12:47 ` Krzysztof Kozlowski
2020-12-08 13:02 ` Sanjay R Mehta
0 siblings, 1 reply; 4+ messages in thread
From: Krzysztof Kozlowski @ 2020-12-08 12:47 UTC (permalink / raw)
To: Sanjay R Mehta
Cc: wsa+renesas, jarkko.nikula, andriy.shevchenko, jdelvare,
Sergey.Semin, kblaiech, loic.poulain, rppt, bjorn.andersson,
linux, vadimp, tali.perry1, linux-i2c, Nehal Bakulchandra Shah
On Tue, Dec 08, 2020 at 06:37:41AM -0600, Sanjay R Mehta wrote:
> 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 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 | 345 +++++++++++++++++++++++++++
> 4 files changed, 362 insertions(+)
> create mode 100644 drivers/i2c/busses/i2c-amdgpu-navi.c
>
> 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..3922b8aebc26
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-amdgpu-navi.c
> @@ -0,0 +1,345 @@
> +// 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;
> +};
> +
> +static int amdgpu_i2c_read(void *context, unsigned int reg, unsigned int *val)
> +{
> + struct amdgpu_i2c_dev *i2cd = context;
> +
> + *val = readl_relaxed(i2cd->regs + reg);
It's quite confusing calling it "i2c_read" function. What is more
important - why do you need it? It's a simple MMIO on PCI, so why regmap
MMIO cannot be used?
Best regards,
Krzysztof
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] i2c: add i2c bus driver for amd navi gpu
2020-12-08 12:47 ` Krzysztof Kozlowski
@ 2020-12-08 13:02 ` Sanjay R Mehta
2020-12-08 16:48 ` Sanjay R Mehta
0 siblings, 1 reply; 4+ messages in thread
From: Sanjay R Mehta @ 2020-12-08 13:02 UTC (permalink / raw)
To: Krzysztof Kozlowski, Sanjay R Mehta
Cc: wsa+renesas, jarkko.nikula, andriy.shevchenko, jdelvare,
Sergey.Semin, kblaiech, loic.poulain, rppt, bjorn.andersson,
linux, vadimp, tali.perry1, linux-i2c, Nehal Bakulchandra Shah
On 12/8/2020 6:17 PM, Krzysztof Kozlowski wrote:
> [CAUTION: External Email]
>
> On Tue, Dec 08, 2020 at 06:37:41AM -0600, Sanjay R Mehta wrote:
>> 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 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 | 345 +++++++++++++++++++++++++++
>> 4 files changed, 362 insertions(+)
>> create mode 100644 drivers/i2c/busses/i2c-amdgpu-navi.c
>>
>> 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..3922b8aebc26
>> --- /dev/null
>> +++ b/drivers/i2c/busses/i2c-amdgpu-navi.c
>> @@ -0,0 +1,345 @@
>> +// 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;
>> +};
>> +
>> +static int amdgpu_i2c_read(void *context, unsigned int reg, unsigned int *val)
>> +{
>> + struct amdgpu_i2c_dev *i2cd = context;
>> +
>> + *val = readl_relaxed(i2cd->regs + reg);
>
> It's quite confusing calling it "i2c_read" function. What is more
> important - why do you need it? It's a simple MMIO on PCI, so why regmap
> MMIO cannot be used?
>
Thanks Krzysztof.
I am new to using regmap based API's and had referred to designware code for this.
(https://elixir.bootlin.com/linux/latest/source/drivers/i2c/busses/i2c-designware-common.c#L61)
Any specific API you recommend me to use or any driver you want me to refer will be helpful.
Thanks & Regards,
Sanjay
> Best regards,
> Krzysztof
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] i2c: add i2c bus driver for amd navi gpu
2020-12-08 13:02 ` Sanjay R Mehta
@ 2020-12-08 16:48 ` Sanjay R Mehta
0 siblings, 0 replies; 4+ messages in thread
From: Sanjay R Mehta @ 2020-12-08 16:48 UTC (permalink / raw)
To: Krzysztof Kozlowski, Sanjay R Mehta
Cc: wsa+renesas, jarkko.nikula, andriy.shevchenko, jdelvare,
Sergey.Semin, kblaiech, loic.poulain, rppt, bjorn.andersson,
linux, vadimp, tali.perry1, linux-i2c, Nehal Bakulchandra Shah
On 12/8/2020 6:32 PM, Sanjay R Mehta wrote:
>
>
> On 12/8/2020 6:17 PM, Krzysztof Kozlowski wrote:
>> [CAUTION: External Email]
>>
>> On Tue, Dec 08, 2020 at 06:37:41AM -0600, Sanjay R Mehta wrote:
>>> 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 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 | 345 +++++++++++++++++++++++++++
>>> 4 files changed, 362 insertions(+)
>>> create mode 100644 drivers/i2c/busses/i2c-amdgpu-navi.c
>>>
>>> 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..3922b8aebc26
>>> --- /dev/null
>>> +++ b/drivers/i2c/busses/i2c-amdgpu-navi.c
>>> @@ -0,0 +1,345 @@
>>> +// 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;
>>> +};
>>> +
>>> +static int amdgpu_i2c_read(void *context, unsigned int reg, unsigned int *val)
>>> +{
>>> + struct amdgpu_i2c_dev *i2cd = context;
>>> +
>>> + *val = readl_relaxed(i2cd->regs + reg);
>>
>> It's quite confusing calling it "i2c_read" function. What is more
>> important - why do you need it? It's a simple MMIO on PCI, so why regmap
>> MMIO cannot be used?
>>
Hi Krzysztof,
As suggested have made the changes in the code to use regmap MMIO and it works fine :).
will send v3 patch with this change.
Thanks,
Sanjay
> Thanks Krzysztof.
>
> I am new to using regmap based API's and had referred to designware code for this.
> (https://elixir.bootlin.com/linux/latest/source/drivers/i2c/busses/i2c-designware-common.c#L61)
>
> Any specific API you recommend me to use or any driver you want me to refer will be helpful.
>
> Thanks & Regards,
> Sanjay
>
>> Best regards,
>> Krzysztof
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-12-08 16:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-08 12:37 [PATCH v2] i2c: add i2c bus driver for amd navi gpu Sanjay R Mehta
2020-12-08 12:47 ` Krzysztof Kozlowski
2020-12-08 13:02 ` Sanjay R Mehta
2020-12-08 16:48 ` Sanjay R Mehta
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.