[v2] i2c:amd I2C Driver based on PCI Interface for upcoming, platform

From: Nehal-bakulchandra Shah <Nehal-bakulchandra.Shah@amd.com>

This contains two drivers.
1)i2c-amd-platdrv: This is based on I2C framework of
linux kernel. So any i2c read write call or commands
to this driver is routed to PCI Interface driver.
2) i2c-amd-platdrv: This driver is responsible to
talk with Mp2 and does all C2P/P2C communication
or reading/writing from DRAM in case of more
data.

Reviewed-by: S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>
Reviewed-by: Sandeep Singh <sandeep.singh@amd.com>
Signed-off-by: Nehal-bakulchandra Shah <Nehal-bakulchandra.Shah@amd.com>
---
Changes since v1:(https://www.spinics.net/lists/linux-i2c/msg34650.html)
-> Add fix for IOMMU
-> Add depedency of ACPI
-> Add locks to avoid the crash

 drivers/i2c/busses/Kconfig           |  10 +
 drivers/i2c/busses/Makefile          |   2 +
 drivers/i2c/busses/i2c-amd-pci-mp2.c | 626 +++++++++++++++++++++++++++++++++++
 drivers/i2c/busses/i2c-amd-pci-mp2.h | 253 ++++++++++++++
 drivers/i2c/busses/i2c-amd-platdrv.c | 324 ++++++++++++++++++
 5 files changed, 1215 insertions(+)
 create mode 100644 drivers/i2c/busses/i2c-amd-pci-mp2.c
 create mode 100644 drivers/i2c/busses/i2c-amd-pci-mp2.h
 create mode 100644 drivers/i2c/busses/i2c-amd-platdrv.c

A few comments inline, I didn't look all that long and now I have
other things to attend to...

On 2018-08-23 12:32, Shah, Nehal-bakulchandra wrote:
> From: Nehal-bakulchandra Shah <Nehal-bakulchandra.Shah@amd.com>
> 
> This contains two drivers.
> 1)i2c-amd-platdrv: This is based on I2C framework of
> linux kernel. So any i2c read write call or commands
> to this driver is routed to PCI Interface driver.
> 2) i2c-amd-platdrv: This driver is responsible to
> talk with Mp2 and does all C2P/P2C communication
> or reading/writing from DRAM in case of more
> data.
> 
> Reviewed-by: S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>
> Reviewed-by: Sandeep Singh <sandeep.singh@amd.com>
> Signed-off-by: Nehal-bakulchandra Shah <Nehal-bakulchandra.Shah@amd.com>
> ---
> Changes since v1:(https://www.spinics.net/lists/linux-i2c/msg34650.html)
> -> Add fix for IOMMU
> -> Add depedency of ACPI
> -> Add locks to avoid the crash
> 
>  drivers/i2c/busses/Kconfig           |  10 +
>  drivers/i2c/busses/Makefile          |   2 +
>  drivers/i2c/busses/i2c-amd-pci-mp2.c | 626 +++++++++++++++++++++++++++++++++++
>  drivers/i2c/busses/i2c-amd-pci-mp2.h | 253 ++++++++++++++
>  drivers/i2c/busses/i2c-amd-platdrv.c | 324 ++++++++++++++++++
>  5 files changed, 1215 insertions(+)
>  create mode 100644 drivers/i2c/busses/i2c-amd-pci-mp2.c
>  create mode 100644 drivers/i2c/busses/i2c-amd-pci-mp2.h
>  create mode 100644 drivers/i2c/busses/i2c-amd-platdrv.c
> 
> diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> index 8d21b98..ff68424 100644
> --- a/drivers/i2c/busses/Kconfig
> +++ b/drivers/i2c/busses/Kconfig
> @@ -77,6 +77,16 @@ config I2C_AMD8111
>  	  This driver can also be built as a module.  If so, the module
>  	  will be called i2c-amd8111.
>  
> +config I2C_AMD_MP2
> +	tristate "AMD MP2"
> +	depends on ACPI && PCI
> +	help
> +	  If you say yes to this option, support will be included for mp2
> +	  I2C interface.
> +
> +	  This driver can also be built as a module.  If so, the module
> +	  will be called i2c-amd-platdrv.
> +
>  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 189e34b..b01e46b 100644
> --- a/drivers/i2c/busses/Makefile
> +++ b/drivers/i2c/busses/Makefile
> @@ -113,6 +113,8 @@ obj-$(CONFIG_I2C_XLR)		+= i2c-xlr.o
>  obj-$(CONFIG_I2C_XLP9XX)	+= i2c-xlp9xx.o
>  obj-$(CONFIG_I2C_RCAR)		+= i2c-rcar.o
>  obj-$(CONFIG_I2C_ZX2967)	+= i2c-zx2967.o
> +obj-$(CONFIG_I2C_AMD_MP2)   	+= i2c-amd-pci-mp2.o
> +obj-$(CONFIG_I2C_AMD_MP2)   	+= i2c-amd-platdrv.o
>  
>  # External I2C/SMBus adapter drivers
>  obj-$(CONFIG_I2C_DIOLAN_U2C)	+= i2c-diolan-u2c.o
> diff --git a/drivers/i2c/busses/i2c-amd-pci-mp2.c b/drivers/i2c/busses/i2c-amd-pci-mp2.c
> new file mode 100644
> index 0000000..5eaec81
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-amd-pci-mp2.c
> @@ -0,0 +1,626 @@
> +// SPDX-License-Identifier: GPL-2.0

GPL-2.0 OR BSD-3-Clause
Goes for all new files in the patch.

> +/*
> + * This file is provided under a dual BSD/GPLv2 license.  When using or
> + *   redistributing this file, you may do so under either license.
> + *
> + *   GPL LICENSE SUMMARY
> + *
> + *   Copyright (C) 2018 Advanced Micro Devices, Inc. All Rights Reserved.
> + *
> + *   This program is free software; you can redistribute it and/or modify
> + *   it under the terms of version 2 of the GNU General Public License as
> + *   published by the Free Software Foundation.
> + *
> + *   BSD LICENSE
> + *
> + *   Copyright (C) 2018 Advanced Micro Devices, Inc. All Rights Reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copy
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of AMD Corporation nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * AMD PCIe MP2 Communication Driver
> + * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> + */

Lose all the license boilerplate gunk, that's what the SPDX id is for.
Goes for all new files in the patch.

> +
> +#include <linux/debugfs.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +#include <linux/slab.h>
> +
> +#include "i2c-amd-pci-mp2.h"
> +
> +#define DRIVER_NAME	"pcie_mp2_amd"
> +#define DRIVER_DESC	"AMD(R) PCI-E MP2 Communication Driver"
> +#define DRIVER_VER	"1.0"
> +
> +MODULE_DESCRIPTION(DRIVER_DESC);
> +MODULE_VERSION(DRIVER_VER);
> +MODULE_LICENSE("Dual BSD/GPL");
> +MODULE_AUTHOR("Shyam Sundar S K <Shyam-sundar.S-k@amd.com>");
> +
> +static const struct file_operations amd_mp2_debugfs_info;
> +static struct dentry *debugfs_dir;
> +
> +int amd_mp2_connect(struct pci_dev *dev,
> +		    struct i2c_connect_config connect_cfg)
> +{
> +	struct amd_mp2_dev *privdata = pci_get_drvdata(dev);
> +	union i2c_cmd_base i2c_cmd_base;
> +	unsigned  long  flags;
> +
> +	raw_spin_lock_irqsave(&privdata->lock, flags);
> +	dev_dbg(ndev_dev(privdata), "%s addr: %x id: %d\n", __func__,
> +		connect_cfg.dev_addr, connect_cfg.bus_id);
> +
> +	i2c_cmd_base.ul = 0;
> +	i2c_cmd_base.s.i2c_cmd = i2c_enable;
> +	i2c_cmd_base.s.bus_id = connect_cfg.bus_id;
> +	i2c_cmd_base.s.i2c_speed = connect_cfg.i2c_speed;
> +
> +	if (i2c_cmd_base.s.bus_id == i2c_bus_1) {
> +		writel(i2c_cmd_base.ul, privdata->mmio + AMD_C2P_MSG1);
> +	} else if (i2c_cmd_base.s.bus_id == i2c_bus_0) {
> +		writel(i2c_cmd_base.ul, privdata->mmio + AMD_C2P_MSG0);
> +	} else {
> +		dev_err(ndev_dev(privdata), "%s Invalid bus id\n", __func__);
> +		return -EINVAL;
> +	}
> +	raw_spin_unlock_irqrestore(&privdata->lock, flags);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(amd_mp2_connect);
> +
> +int amd_mp2_read(struct pci_dev *dev, struct i2c_read_config read_cfg)
> +{
> +	struct amd_mp2_dev *privdata = pci_get_drvdata(dev);
> +	union i2c_cmd_base i2c_cmd_base;
> +
> +	dev_dbg(ndev_dev(privdata), "%s addr: %x id: %d\n", __func__,
> +		read_cfg.dev_addr, read_cfg.bus_id);
> +
> +	privdata->requested = true;
> +	i2c_cmd_base.ul = 0;
> +	i2c_cmd_base.s.i2c_cmd = i2c_read;
> +	i2c_cmd_base.s.dev_addr = read_cfg.dev_addr;
> +	i2c_cmd_base.s.length = read_cfg.length;
> +	i2c_cmd_base.s.bus_id = read_cfg.bus_id;
> +
> +	if (read_cfg.length <= 32) {
> +		i2c_cmd_base.s.mem_type = use_c2pmsg;
> +		privdata->eventval.buf = (u32 *)read_cfg.buf;
> +		if (!privdata->eventval.buf) {
> +			dev_err(ndev_dev(privdata), "%s no mem for buf received\n",
> +				__func__);
> +			return -ENOMEM;
> +		}
> +		dev_dbg(ndev_dev(privdata), "%s buf: %llx\n", __func__,
> +			(u64)privdata->eventval.buf);
> +	} else {
> +		i2c_cmd_base.s.mem_type = use_dram;
> +		privdata->read_cfg.phy_addr = read_cfg.phy_addr;
> +		privdata->read_cfg.buf = read_cfg.buf;
> +		write64((u64)privdata->read_cfg.phy_addr,
> +			privdata->mmio + AMD_C2P_MSG2);
> +	}
> +
> +	switch (read_cfg.i2c_speed) {
> +	case 0:
> +		i2c_cmd_base.s.i2c_speed = speed100k;
> +		break;
> +	case 1:
> +		i2c_cmd_base.s.i2c_speed = speed400k;
> +		break;
> +	case 2:
> +		i2c_cmd_base.s.i2c_speed = speed1000k;
> +		break;
> +	case 3:
> +		i2c_cmd_base.s.i2c_speed = speed1400k;
> +		break;
> +	case 4:
> +		i2c_cmd_base.s.i2c_speed = speed3400k;
> +		break;
> +	default:
> +		dev_err(ndev_dev(privdata), "Invalid ConnectionSpeed\n");
> +	}
> +
> +	if (i2c_cmd_base.s.bus_id == i2c_bus_1) {
> +		writel(i2c_cmd_base.ul, privdata->mmio + AMD_C2P_MSG1);
> +	} else if (i2c_cmd_base.s.bus_id == i2c_bus_0) {
> +		writel(i2c_cmd_base.ul, privdata->mmio + AMD_C2P_MSG0);
> +	} else {
> +		dev_err(ndev_dev(privdata), "%s Invalid bus id\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(amd_mp2_read);
> +
> +int amd_mp2_write(struct pci_dev *dev, struct i2c_write_config write_cfg)
> +{
> +	struct amd_mp2_dev *privdata = pci_get_drvdata(dev);
> +	union i2c_cmd_base i2c_cmd_base;
> +	int i = 0;
> +
> +	privdata->requested = true;
> +	dev_dbg(ndev_dev(privdata), "%s addr: %x id: %d\n", __func__,
> +		write_cfg.dev_addr, write_cfg.bus_id);
> +
> +	i2c_cmd_base.ul = 0;
> +	i2c_cmd_base.s.i2c_cmd = i2c_write;
> +	i2c_cmd_base.s.dev_addr = write_cfg.dev_addr;
> +	i2c_cmd_base.s.length = write_cfg.length;
> +	i2c_cmd_base.s.bus_id = write_cfg.bus_id;
> +
> +	switch (write_cfg.i2c_speed) {
> +	case 0:
> +		i2c_cmd_base.s.i2c_speed = speed100k;
> +		break;
> +	case 1:
> +		i2c_cmd_base.s.i2c_speed = speed400k;
> +		break;
> +	case 2:
> +		i2c_cmd_base.s.i2c_speed = speed1000k;
> +		break;
> +	case 3:
> +		i2c_cmd_base.s.i2c_speed = speed1400k;
> +		break;
> +	case 4:
> +		i2c_cmd_base.s.i2c_speed = speed3400k;
> +		break;
> +	default:
> +		dev_err(ndev_dev(privdata), "Invalid ConnectionSpeed\n");
> +	}
> +
> +	if (write_cfg.length <= 32) {
> +		i2c_cmd_base.s.mem_type = use_c2pmsg;
> +		for (i = 0; i < ((write_cfg.length + 3) / 4); i++) {
> +			writel(write_cfg.buf[i],
> +			       privdata->mmio + (AMD_C2P_MSG2 + i * 4));
> +		}
> +	} else {
> +		i2c_cmd_base.s.mem_type = use_dram;
> +		privdata->write_cfg.phy_addr = write_cfg.phy_addr;
> +		write64((u64)privdata->write_cfg.phy_addr,
> +			privdata->mmio + AMD_C2P_MSG2);
> +	}
> +
> +	if (i2c_cmd_base.s.bus_id == i2c_bus_1) {
> +		writel(i2c_cmd_base.ul, privdata->mmio + AMD_C2P_MSG1);
> +	} else if (i2c_cmd_base.s.bus_id == i2c_bus_0) {
> +		writel(i2c_cmd_base.ul, privdata->mmio + AMD_C2P_MSG0);
> +	} else {
> +		dev_err(ndev_dev(privdata), "%s Invalid bus id\n", __func__);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(amd_mp2_write);
> +
> +int amd_i2c_register_cb(struct pci_dev *dev, const struct amd_i2c_pci_ops *ops,
> +			void *dev_ctx)
> +{
> +	struct amd_mp2_dev *privdata = pci_get_drvdata(dev);
> +
> +	privdata->ops = ops;
> +	privdata->i2c_dev_ctx = dev_ctx;
> +
> +	if (!privdata->ops || !privdata->i2c_dev_ctx)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(amd_i2c_register_cb);
> +
> +static void amd_mp2_pci_work(struct work_struct *work)
> +{
> +	struct amd_mp2_dev *privdata = mp2_dev(work);
> +	u32 readdata = 0;
> +	int i = 0;
> +	int sts = privdata->eventval.base.r.status;
> +	int res = privdata->eventval.base.r.response;
> +	int len = privdata->eventval.base.r.length;
> +
> +	if (res == command_success && sts == i2c_readcomplete_event) {
> +		if (privdata->ops->read_complete) {
> +			if (len <= 32) {
> +				for (i = 0; i < ((len + 3) / 4); i++) {
> +					readdata = readl(privdata->mmio +
> +							(AMD_C2P_MSG2 + i * 4));
> +					privdata->eventval.buf[i] = readdata;
> +				}
> +				privdata->ops->read_complete(privdata->eventval,
> +						privdata->i2c_dev_ctx);
> +			} else {
> +				privdata->ops->read_complete(privdata->eventval,
> +						privdata->i2c_dev_ctx);
> +			}
> +		}
> +	} else if (res == command_success && sts == i2c_writecomplete_event) {
> +		if (privdata->ops->write_complete)
> +			privdata->ops->write_complete(privdata->eventval,
> +					privdata->i2c_dev_ctx);
> +	} else if (res == command_success && sts == i2c_busenable_complete) {
> +		if (privdata->ops->connect_complete)
> +			privdata->ops->connect_complete(privdata->eventval,
> +					privdata->i2c_dev_ctx);
> +	} else {
> +		dev_err(ndev_dev(privdata), "ERROR!!nothing to be handled !\n");
> +	}
> +}
> +
> +static irqreturn_t amd_mp2_irq_isr(int irq, void *dev)
> +{
> +	struct amd_mp2_dev *privdata = dev;
> +	u32 val = 0;
> +	unsigned long  flags;
> +
> +	raw_spin_lock_irqsave(&privdata->lock, flags);
> +	val = readl(privdata->mmio + AMD_P2C_MSG1);
> +	if (val != 0) {
> +		writel(0, privdata->mmio + AMD_P2C_MSG_INTEN);
> +		privdata->eventval.base.ul = val;
> +	} else {
> +		val = readl(privdata->mmio + AMD_P2C_MSG2);
> +		if (val != 0) {
> +			writel(0, privdata->mmio + AMD_P2C_MSG_INTEN);
> +			privdata->eventval.base.ul = val;
> +		}
> +	}
> +
> +	raw_spin_unlock_irqrestore(&privdata->lock, flags);
> +	if (!privdata->ops)
> +		return IRQ_NONE;
> +
> +	if (!privdata->requested)
> +		return IRQ_HANDLED;
> +
> +	privdata->requested = false;
> +	schedule_delayed_work(&privdata->work, 0);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static ssize_t amd_mp2_debugfs_read(struct file *filp, char __user *ubuf,
> +				    size_t count, loff_t *offp)
> +{
> +	struct amd_mp2_dev *privdata;
> +	void __iomem *mmio;
> +	u8 *buf;
> +	size_t buf_size;
> +	ssize_t ret, off;
> +	union {
> +		u64 v64;
> +		u32 v32;
> +		u16 v16;
> +	} u;

What is this union for? Seems totally pointless to me when the only
thing you ever use from it is u.v32...

Cheers,
Peter

> +
> +	privdata = filp->private_data;
> +	mmio = privdata->mmio;
> +	buf_size = min(count, 0x800ul);
> +	buf = kmalloc(buf_size, GFP_KERNEL);
> +
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	off = 0;
> +	off += scnprintf(buf + off, buf_size - off,
> +			"Mp2 Device Information:\n");
> +
> +	off += scnprintf(buf + off, buf_size - off,
> +			"========================\n");
> +	off += scnprintf(buf + off, buf_size - off,
> +			"\tMP2 C2P Message Register Dump:\n\n");
> +	u.v32 = readl(privdata->mmio + AMD_C2P_MSG0);
> +	off += scnprintf(buf + off, buf_size - off,
> +			"AMD_C2P_MSG0 -\t\t\t%#06x\n", u.v32);
> +
> +	u.v32 = readl(privdata->mmio + AMD_C2P_MSG1);
> +	off += scnprintf(buf + off, buf_size - off,
> +			"AMD_C2P_MSG1 -\t\t\t%#06x\n", u.v32);
> +
> +	u.v32 = readl(privdata->mmio + AMD_C2P_MSG2);
> +	off += scnprintf(buf + off, buf_size - off,
> +			"AMD_C2P_MSG2 -\t\t\t%#06x\n", u.v32);
> +
> +	u.v32 = readl(privdata->mmio + AMD_C2P_MSG3);
> +	off += scnprintf(buf + off, buf_size - off,
> +			"AMD_C2P_MSG3 -\t\t\t%#06x\n", u.v32);
> +
> +	u.v32 = readl(privdata->mmio + AMD_C2P_MSG4);
> +	off += scnprintf(buf + off, buf_size - off,
> +			"AMD_C2P_MSG4 -\t\t\t%#06x\n", u.v32);
> +
> +	u.v32 = readl(privdata->mmio + AMD_C2P_MSG5);
> +	off += scnprintf(buf + off, buf_size - off,
> +			"AMD_C2P_MSG5 -\t\t\t%#06x\n", u.v32);
> +
> +	u.v32 = readl(privdata->mmio + AMD_C2P_MSG6);
> +	off += scnprintf(buf + off, buf_size - off,
> +			"AMD_C2P_MSG6 -\t\t\t%#06x\n", u.v32);
> +
> +	u.v32 = readl(privdata->mmio + AMD_C2P_MSG7);
> +	off += scnprintf(buf + off, buf_size - off,
> +			"AMD_C2P_MSG7 -\t\t\t%#06x\n", u.v32);
> +
> +	u.v32 = readl(privdata->mmio + AMD_C2P_MSG8);
> +	off += scnprintf(buf + off, buf_size - off,
> +			"AMD_C2P_MSG8 -\t\t\t%#06x\n", u.v32);
> +
> +	u.v32 = readl(privdata->mmio + AMD_C2P_MSG9);
> +	off += scnprintf(buf + off, buf_size - off,
> +			"AMD_C2P_MSG9 -\t\t\t%#06x\n", u.v32);
> +
> +	off += scnprintf(buf + off, buf_size - off,
> +			"\n\tMP2 P2C Message Register Dump:\n\n");
> +
> +	u.v32 = readl(privdata->mmio + AMD_P2C_MSG1);
> +	off += scnprintf(buf + off, buf_size - off,
> +			"AMD_P2C_MSG1 -\t\t\t%#06x\n", u.v32);
> +
> +	u.v32 = readl(privdata->mmio + AMD_P2C_MSG2);
> +	off += scnprintf(buf + off, buf_size - off,
> +			"AMD_P2C_MSG2 -\t\t\t%#06x\n", u.v32);
> +
> +	u.v32 = readl(privdata->mmio + AMD_P2C_MSG_INTEN);
> +	off += scnprintf(buf + off, buf_size - off,
> +			"AMD_P2C_MSG_INTEN -\t\t%#06x\n", u.v32);
> +
> +	u.v32 = readl(privdata->mmio + AMD_P2C_MSG_INTSTS);
> +	off += scnprintf(buf + off, buf_size - off,
> +			"AMD_P2C_MSG_INTSTS -\t\t%#06x\n", u.v32);
> +
> +	ret = simple_read_from_buffer(ubuf, count, offp, buf, off);
> +	kfree(buf);
> +	return ret;
> +}
> +
> +static void amd_mp2_init_debugfs(struct amd_mp2_dev *privdata)
> +{
> +	if (!debugfs_dir) {
> +		privdata->debugfs_dir = NULL;
> +		privdata->debugfs_info = NULL;
> +	} else {
> +		privdata->debugfs_dir = debugfs_create_dir(ndev_name(privdata),
> +							   debugfs_dir);
> +		if (!privdata->debugfs_dir) {
> +			privdata->debugfs_info = NULL;
> +		} else {
> +			privdata->debugfs_info = debugfs_create_file
> +				("info", 0400, privdata->debugfs_dir,
> +					privdata, &amd_mp2_debugfs_info);
> +		}
> +	}
> +}
> +
> +static void amd_mp2_deinit_debugfs(struct amd_mp2_dev *privdata)
> +{
> +	debugfs_remove_recursive(privdata->debugfs_dir);
> +}
> +
> +static void amd_mp2_clear_reg(struct amd_mp2_dev *privdata)
> +{
> +	int reg = 0;
> +
> +	for (reg = AMD_C2P_MSG0; reg <= AMD_C2P_MSG9; reg += 4)
> +		writel(0, privdata->mmio + reg);
> +
> +	for (reg = AMD_P2C_MSG0; reg <= AMD_P2C_MSG2; reg += 4)
> +		writel(0, privdata->mmio + reg);
> +}
> +
> +static int amd_mp2_pci_init(struct amd_mp2_dev *privdata, struct pci_dev *pdev)
> +{
> +	int rc;
> +	int bar_index = 2;
> +	resource_size_t size, base;
> +
> +	pci_set_drvdata(pdev, privdata);
> +
> +	rc = pci_enable_device(pdev);
> +	if (rc)
> +		goto err_pci_enable;
> +
> +	rc = pci_request_regions(pdev, DRIVER_NAME);
> +	if (rc)
> +		goto err_pci_regions;
> +
> +	pci_set_master(pdev);
> +
> +	rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(64));
> +	if (rc) {
> +		rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
> +		if (rc)
> +			goto err_dma_mask;
> +		dev_warn(ndev_dev(privdata), "Cannot DMA highmem\n");
> +	}
> +
> +	rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64));
> +	if (rc) {
> +		rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
> +		if (rc)
> +			goto err_dma_mask;
> +		dev_warn(ndev_dev(privdata), "Cannot DMA consistent highmem\n");
> +	}
> +
> +	base = pci_resource_start(pdev, bar_index);
> +	size = pci_resource_len(pdev, bar_index);
> +	dev_dbg(ndev_dev(privdata), "Base addr:%llx size:%llx\n", base, size);
> +
> +	privdata->mmio = ioremap(base, size);
> +	if (!privdata->mmio) {
> +		rc = -EIO;
> +		goto err_dma_mask;
> +	}
> +
> +	/* Try to set up intx irq */
> +	pci_intx(pdev, 1);
> +	privdata->eventval.buf = NULL;
> +	privdata->requested = false;
> +	raw_spin_lock_init(&privdata->lock);
> +	rc = request_irq(pdev->irq, amd_mp2_irq_isr, IRQF_SHARED, "mp2_irq_isr",
> +			 privdata);
> +	if (rc)
> +		goto err_intx_request;
> +
> +	return 0;
> +
> +err_intx_request:
> +	return rc;
> +err_dma_mask:
> +	pci_clear_master(pdev);
> +	pci_release_regions(pdev);
> +err_pci_regions:
> +	pci_disable_device(pdev);
> +err_pci_enable:
> +	pci_set_drvdata(pdev, NULL);
> +	return rc;
> +}
> +
> +static void amd_mp2_pci_deinit(struct amd_mp2_dev *privdata)
> +{
> +	struct pci_dev *pdev = ndev_pdev(privdata);
> +
> +	pci_iounmap(pdev, privdata->mmio);
> +
> +	pci_clear_master(pdev);
> +	pci_release_regions(pdev);
> +	pci_disable_device(pdev);
> +	pci_set_drvdata(pdev, NULL);
> +}
> +
> +static int amd_mp2_pci_probe(struct pci_dev *pdev,
> +			     const struct pci_device_id *id)
> +{
> +	struct amd_mp2_dev *privdata;
> +	int rc;
> +
> +	dev_info(&pdev->dev, "MP2 device found [%04x:%04x] (rev %x)\n",
> +		 (int)pdev->vendor, (int)pdev->device, (int)pdev->revision);
> +
> +	privdata = kzalloc(sizeof(*privdata), GFP_KERNEL);
> +	privdata->pdev = pdev;
> +
> +	if (!privdata) {
> +		rc = -ENOMEM;
> +		goto err_dev;
> +	}
> +
> +	rc = amd_mp2_pci_init(privdata, pdev);
> +	if (rc)
> +		goto err_pci_init;
> +	dev_dbg(&pdev->dev, "pci init done.\n");
> +
> +	INIT_DELAYED_WORK(&privdata->work, amd_mp2_pci_work);
> +
> +	amd_mp2_init_debugfs(privdata);
> +	dev_info(&pdev->dev, "MP2 device registered.\n");
> +	return 0;
> +
> +err_pci_init:
> +	kfree(privdata);
> +err_dev:
> +	dev_err(&pdev->dev, "Memory Allocation Failed\n");
> +	return rc;
> +}
> +
> +static void amd_mp2_pci_remove(struct pci_dev *pdev)
> +{
> +	struct amd_mp2_dev *privdata = pci_get_drvdata(pdev);
> +
> +	amd_mp2_deinit_debugfs(privdata);
> +	amd_mp2_clear_reg(privdata);
> +	cancel_delayed_work_sync(&privdata->work);
> +	free_irq(pdev->irq, privdata);
> +	pci_intx(pdev, 0);
> +	amd_mp2_pci_deinit(privdata);
> +	kfree(privdata);
> +}
> +
> +static const struct file_operations amd_mp2_debugfs_info = {
> +	.owner = THIS_MODULE,
> +	.open = simple_open,
> +	.read = amd_mp2_debugfs_read,
> +};
> +
> +static const struct pci_device_id amd_mp2_pci_tbl[] = {
> +	{PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_MP2)},
> +	{0}
> +};
> +MODULE_DEVICE_TABLE(pci, amd_mp2_pci_tbl);
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int amd_mp2_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg)
> +{
> +	struct amd_mp2_dev *privdata = pci_get_drvdata(pdev);
> +
> +	if (!privdata)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
> +static int amd_mp2_pci_device_resume(struct pci_dev *pdev)
> +{
> +	struct amd_mp2_dev *privdata = pci_get_drvdata(pdev);
> +
> +	if (!privdata)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +#endif
> +
> +static struct pci_driver amd_mp2_pci_driver = {
> +	.name		= DRIVER_NAME,
> +	.id_table	= amd_mp2_pci_tbl,
> +	.probe		= amd_mp2_pci_probe,
> +	.remove		= amd_mp2_pci_remove,
> +#ifdef CONFIG_PM_SLEEP
> +	.suspend		= amd_mp2_pci_device_suspend,
> +	.resume			= amd_mp2_pci_device_resume,
> +#endif
> +};
> +
> +static int __init amd_mp2_pci_driver_init(void)
> +{
> +	pr_info("%s: %s Version: %s\n", DRIVER_NAME, DRIVER_DESC, DRIVER_VER);
> +
> +	if (debugfs_initialized())
> +		debugfs_dir = debugfs_create_dir(KBUILD_MODNAME, NULL);
> +
> +	return pci_register_driver(&amd_mp2_pci_driver);
> +}
> +module_init(amd_mp2_pci_driver_init);
> +
> +static void __exit amd_mp2_pci_driver_exit(void)
> +{
> +	pci_unregister_driver(&amd_mp2_pci_driver);
> +	debugfs_remove_recursive(debugfs_dir);
> +}
> +module_exit(amd_mp2_pci_driver_exit);
> diff --git a/drivers/i2c/busses/i2c-amd-pci-mp2.h b/drivers/i2c/busses/i2c-amd-pci-mp2.h
> new file mode 100644
> index 0000000..da77b9f
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-amd-pci-mp2.h
> @@ -0,0 +1,253 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * This file is provided under a dual BSD/GPLv2 license.  When using or
> + *   redistributing this file, you may do so under either license.
> + *
> + *   GPL LICENSE SUMMARY
> + *
> + *   Copyright (C) 2018 Advanced Micro Devices, Inc. All Rights Reserved.
> + *
> + *   This program is free software; you can redistribute it and/or modify
> + *   it under the terms of version 2 of the GNU General Public License as
> + *   published by the Free Software Foundation.
> + *
> + *   BSD LICENSE
> + *
> + *   Copyright (C) 2018 Advanced Micro Devices, Inc. All Rights Reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copy
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of AMD Corporation nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> + * AMD PCIe MP2 Communication Interface Driver
> + * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> + */
> +
> +#ifndef I2C_AMD_PCI_MP2_H
> +#define I2C_AMD_PCI_MP2_H
> +
> +#include <linux/pci.h>
> +
> +#define PCI_DEVICE_ID_AMD_MP2	0x15E6
> +
> +#define write64 _write64
> +static inline void _write64(u64 val, void __iomem *mmio)
> +{
> +	writel(val, mmio);
> +	writel(val >> 32, mmio + sizeof(u32));
> +}
> +
> +#define read64 _read64
> +static inline u64 _read64(void __iomem *mmio)
> +{
> +	u64 low, high;
> +
> +	low = readl(mmio);
> +	high = readl(mmio + sizeof(u32));
> +	return low | (high << 32);
> +}
> +
> +enum {
> +	/* MP2 C2P Message Registers */
> +	AMD_C2P_MSG0 = 0x10500, /*MP2 Message for I2C0*/
> +	AMD_C2P_MSG1 = 0x10504, /*MP2 Message for I2C1*/
> +	AMD_C2P_MSG2 = 0x10508, /*DRAM Address Lo / Data 0*/
> +	AMD_C2P_MSG3 = 0x1050c, /*DRAM Address HI / Data 1*/
> +	AMD_C2P_MSG4 = 0x10510, /*Data 2*/
> +	AMD_C2P_MSG5 = 0x10514, /*Data 3*/
> +	AMD_C2P_MSG6 = 0x10518, /*Data 4*/
> +	AMD_C2P_MSG7 = 0x1051c, /*Data 5*/
> +	AMD_C2P_MSG8 = 0x10520, /*Data 6*/
> +	AMD_C2P_MSG9 = 0x10524, /*Data 7*/
> +
> +	/* MP2 P2C Message Registers */
> +	AMD_P2C_MSG0 = 0x10680, /*Do not use*/
> +	AMD_P2C_MSG1 = 0x10684, /*I2c0 int reg*/
> +	AMD_P2C_MSG2 = 0x10688, /*I2c1 int reg*/
> +	AMD_P2C_MSG3 = 0x1068C, /*MP2 debug info*/
> +	AMD_P2C_MSG_INTEN = 0x10690, /*MP2 int gen register*/
> +	AMD_P2C_MSG_INTSTS = 0x10694, /*Interrupt sts*/
> +};
> +
> +/* Command register data structures */
> +
> +enum i2c_cmd {
> +	i2c_read,
> +	i2c_write,
> +	i2c_enable,
> +	i2c_disable,
> +	number_of_sensor_discovered,
> +	is_mp2_active,
> +	invalid_cmd = 0xF,
> +};
> +
> +enum i2c_bus_index {
> +	i2c_bus_0 = 0,
> +	i2c_bus_1 = 1,
> +	i2c_bus_max
> +};
> +
> +enum speed_enum {
> +	speed100k = 0,
> +	speed400k = 1,
> +	speed1000k = 2,
> +	speed1400k = 3,
> +	speed3400k = 4
> +};
> +
> +enum mem_type {
> +	use_dram = 0,
> +	use_c2pmsg = 1,
> +};
> +
> +union i2c_cmd_base {
> +	u32 ul;
> +	struct {
> +		enum i2c_cmd i2c_cmd : 4; /*!< bit: 0..3 i2c R/W command */
> +		enum i2c_bus_index bus_id : 4; /*!< bit: 4..7 i2c bus index */
> +		u32 dev_addr : 8; /*!< bit: 8..15 device address or Bus Speed*/
> +		u32 length : 12; /*!< bit: 16..29 read/write length */
> +		enum speed_enum i2c_speed : 3; /*!< bit: 30 register address*/
> +		enum mem_type mem_type : 1; /*!< bit: 15 mem type*/
> +	} s; /*!< Structure used for bit access */
> +};
> +
> +/* Response register data structures */
> +
> +/*Response - Response of SFI*/
> +enum response_type {
> +	invalid_response = 0,
> +	command_success = 1,
> +	command_failed = 2,
> +};
> +
> +/*Status - Command ID to indicate a command*/
> +enum status_type {
> +	i2c_readcomplete_event = 0,
> +	i2c_readfail_event = 1,
> +	i2c_writecomplete_event = 2,
> +	i2c_writefail_event = 3,
> +	i2c_busenable_complete = 4,
> +	i2c_busenable_failed = 5,
> +	i2c_busdisable_complete = 6,
> +	i2c_busdisable_failed = 7,
> +	invalid_data_length = 8,
> +	invalid_slave_address = 9,
> +	invalid_i2cbus_id = 10,
> +	invalid_dram_addr = 11,
> +	invalid_command = 12,
> +	mp2_active = 13,
> +	numberof_sensors_discovered_resp = 14,
> +	i2C_bus_notinitialized
> +};
> +
> +struct i2c_event {
> +	union {
> +		u32 ul;
> +		struct {
> +			enum response_type response : 2; /*!< bit: 0..1 I2C res type */
> +			enum status_type status : 5; /*!< bit: 2..6 status_type */
> +			enum mem_type mem_type : 1; /*!< bit: 7 0-DRAM;1- C2PMsg o/p */
> +			enum i2c_bus_index bus_id : 4; /*!< bit: 8..11 I2C Bus ID */
> +			u32 length : 12; /*!< bit:16..29 length */
> +			u32 slave_addr : 8; /*!< bit: 15 debug msg include in p2c msg */
> +		} r; /*!< Structure used for bit access */
> +	} base;
> +	u32 *buf;
> +};
> +
> +/* data structures for communication with I2c*/
> +
> +struct i2c_connect_config {
> +	enum i2c_bus_index bus_id;
> +	u64 i2c_speed;
> +	u16 dev_addr;
> +};
> +
> +struct i2c_write_config {
> +	enum i2c_bus_index bus_id;
> +	u64 i2c_speed;
> +	u16 dev_addr;
> +	u32 length;
> +	phys_addr_t phy_addr;
> +	u32 *buf;
> +};
> +
> +struct i2c_read_config {
> +	enum i2c_bus_index bus_id;
> +	u64 i2c_speed;
> +	u16 dev_addr;
> +	u32 length;
> +	phys_addr_t phy_addr;
> +	u8 *buf;
> +};
> +
> +// struct to send/receive data b/w pci and i2c drivers
> +struct amd_i2c_pci_ops {
> +	int (*read_complete)(struct i2c_event event, void *dev_ctx);
> +	int (*write_complete)(struct i2c_event event, void *dev_ctx);
> +	int (*connect_complete)(struct i2c_event event, void *dev_ctx);
> +};
> +
> +struct amd_i2c_common {
> +	struct i2c_connect_config connect_cfg;
> +	struct i2c_read_config read_cfg;
> +	struct i2c_write_config write_cfg;
> +	const struct amd_i2c_pci_ops *ops;
> +	struct pci_dev *pdev;
> +};
> +
> +struct amd_mp2_dev {
> +	struct pci_dev *pdev;
> +	struct dentry *debugfs_dir;
> +	struct dentry *debugfs_info;
> +	void __iomem *mmio;
> +	struct i2c_event eventval;
> +	enum i2c_cmd reqcmd;
> +	struct i2c_connect_config connect_cfg;
> +	struct i2c_read_config read_cfg;
> +	struct i2c_write_config write_cfg;
> +	union i2c_cmd_base i2c_cmd_base;
> +	const struct amd_i2c_pci_ops *ops;
> +	struct delayed_work work;
> +	void *i2c_dev_ctx;
> +	bool	requested;
> +	raw_spinlock_t lock;
> +};
> +
> +int amd_mp2_read(struct pci_dev *pdev, struct i2c_read_config read_cfg);
> +int amd_mp2_write(struct pci_dev *pdev,
> +		  struct i2c_write_config write_cfg);
> +int amd_mp2_connect(struct pci_dev *pdev,
> +		    struct i2c_connect_config connect_cfg);
> +int amd_i2c_register_cb(struct pci_dev *pdev, const struct amd_i2c_pci_ops *ops,
> +			void *dev_ctx);
> +
> +#define ndev_pdev(ndev) ((ndev)->pdev)
> +#define ndev_name(ndev) pci_name(ndev_pdev(ndev))
> +#define ndev_dev(ndev) (&ndev_pdev(ndev)->dev)
> +#define mp2_dev(__work) container_of(__work, struct amd_mp2_dev, work.work)
> +
> +#endif
> diff --git a/drivers/i2c/busses/i2c-amd-platdrv.c b/drivers/i2c/busses/i2c-amd-platdrv.c
> new file mode 100644
> index 0000000..9965b26
> --- /dev/null
> +++ b/drivers/i2c/busses/i2c-amd-platdrv.c
> @@ -0,0 +1,324 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + *
> + * This file is provided under a dual BSD/GPLv2 license.  When using or
> + *   redistributing this file, you may do so under either license.
> + *
> + *   GPL LICENSE SUMMARY
> + *
> + *   Copyright (C) 2018 Advanced Micro Devices, Inc. All Rights Reserved.
> + *
> + *   This program is free software; you can redistribute it and/or modify
> + *   it under the terms of version 2 of the GNU General Public License as
> + *   published by the Free Software Foundation.
> + *
> + *   BSD LICENSE
> + *
> + *   Copyright (C) 2018 Advanced Micro Devices, Inc. All Rights Reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copy
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of AMD Corporation nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + *
> +
> + * AMD I2C Platform Driver
> + * Author: Nehal Bakulchandra Shah <Nehal-bakulchandra.shah@amd.com>
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/types.h>
> +#include <linux/slab.h>
> +#include <linux/i2c.h>
> +#include <linux/platform_device.h>
> +#include <linux/acpi.h>
> +#include <linux/delay.h>
> +#include "i2c-amd-pci-mp2.h"
> +#include <linux/dma-mapping.h>
> +#define  DRIVER_NAME "AMD-I2C-PLATDRV"
> +
> +struct amd_i2c_dev {
> +	struct platform_device *pdev;
> +	struct i2c_adapter adapter;
> +	struct amd_i2c_common i2c_common;
> +	struct completion msg_complete;
> +	struct i2c_msg *msg_buf;
> +	bool is_configured;
> +	u8 bus_id;
> +
> +};
> +
> +static int i2c_amd_read_completion(struct i2c_event event, void *dev_ctx)
> +{
> +	struct amd_i2c_dev *i2c_dev = (struct amd_i2c_dev *)dev_ctx;
> +	struct amd_i2c_common *commond = &i2c_dev->i2c_common;
> +	int i = 0;
> +
> +	if (event.base.r.status == i2c_readcomplete_event) {
> +		if (event.base.r.length <= 32) {
> +			pr_devel(" in %s i2c_dev->msg_buf :%p\n",
> +				 __func__, i2c_dev->msg_buf);
> +
> +			memcpy(i2c_dev->msg_buf->buf,
> +			       (unsigned char *)event.buf, event.base.r.length);
> +
> +			for (i = 0; i < ((event.base.r.length + 3) / 4); i++)
> +				pr_devel("%s:%s readdata:%x\n",
> +					 DRIVER_NAME, __func__, event.buf[i]);
> +
> +		} else {
> +			memcpy(i2c_dev->msg_buf->buf,
> +			       (unsigned char *)commond->read_cfg.buf,
> +				event.base.r.length);
> +			pr_devel("%s:%s virt:%llx phy_addr:%llx\n",
> +				 DRIVER_NAME, __func__,
> +				(u64)commond->read_cfg.buf,
> +				(u64)commond->read_cfg.phy_addr);
> +
> +			for (i = 0; i < ((event.base.r.length + 3) / 4); i++)
> +				pr_devel("%s:%s readdata:%x\n",
> +					 DRIVER_NAME, __func__, ((unsigned int *)
> +					commond->read_cfg.buf)[i]);
> +		}
> +
> +		complete(&i2c_dev->msg_complete);
> +	}
> +
> +	return 0;
> +}
> +
> +static int i2c_amd_write_completion(struct i2c_event event, void *dev_ctx)
> +{
> +	struct amd_i2c_dev *i2c_dev = (struct amd_i2c_dev *)dev_ctx;
> +
> +	if (event.base.r.status == i2c_writecomplete_event)
> +		complete(&i2c_dev->msg_complete);
> +
> +	return 0;
> +}
> +
> +static int i2c_amd_connect_completion(struct i2c_event event, void *dev_ctx)
> +{
> +	struct amd_i2c_dev *i2c_dev = (struct amd_i2c_dev *)dev_ctx;
> +
> +	if (event.base.r.status == i2c_busenable_complete)
> +		complete(&i2c_dev->msg_complete);
> +
> +	return 0;
> +}
> +
> +static const struct amd_i2c_pci_ops data_handler = {
> +		.read_complete = i2c_amd_read_completion,
> +		.write_complete = i2c_amd_write_completion,
> +		.connect_complete = i2c_amd_connect_completion,
> +};
> +
> +static int i2c_amd_pci_configure(struct amd_i2c_dev *i2c_dev, int slaveaddr)
> +{
> +	struct amd_i2c_common *i2c_common = &i2c_dev->i2c_common;
> +	int ret;
> +
> +	amd_i2c_register_cb(i2c_common->pdev, &data_handler, (void *)i2c_dev);
> +	i2c_common->connect_cfg.bus_id = i2c_dev->bus_id;
> +	i2c_common->connect_cfg.dev_addr = slaveaddr;
> +	i2c_common->connect_cfg.i2c_speed = speed400k;
> +
> +	ret = amd_mp2_connect(i2c_common->pdev, i2c_common->connect_cfg);
> +	if (ret)
> +		return -1;
> +
> +	mdelay(100);
> +
> +	i2c_common->write_cfg.bus_id = i2c_dev->bus_id;
> +	i2c_common->write_cfg.dev_addr = slaveaddr;
> +	i2c_common->write_cfg.i2c_speed = speed400k;
> +
> +	i2c_common->read_cfg.bus_id = i2c_dev->bus_id;
> +	i2c_common->read_cfg.dev_addr = slaveaddr;
> +	i2c_common->read_cfg.i2c_speed = speed400k;
> +
> +	return 0;
> +}
> +
> +static int i2c_amd_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
> +{
> +	struct amd_i2c_dev *dev = i2c_get_adapdata(adap);
> +	struct amd_i2c_common *i2c_common = &dev->i2c_common;
> +
> +	int i = 0;
> +	unsigned long timeout;
> +	struct i2c_msg *pmsg;
> +	unsigned char *buf;
> +
> +	dma_addr_t phys;
> +
> +	reinit_completion(&dev->msg_complete);
> +	if (dev->is_configured == 0) {
> +		i2c_amd_pci_configure(dev, msgs->addr);
> +		timeout = wait_for_completion_timeout(&dev->msg_complete, 50);
> +		dev->is_configured = 1;
> +	}
> +
> +	for (i = 0; i < num; i++) {
> +		pmsg = &msgs[i];
> +		if (pmsg->flags & I2C_M_RD) {
> +			if (pmsg->len <= 32) {
> +				buf = kzalloc(pmsg->len, GFP_KERNEL);
> +				if (!buf)
> +					return -ENOMEM;
> +				i2c_common->read_cfg.buf = buf;
> +				i2c_common->read_cfg.length = pmsg->len;
> +				i2c_common->read_cfg.phy_addr =
> +							virt_to_phys(buf);
> +			} else {
> +				buf = (u8 *)dma_alloc_coherent(&i2c_common->pdev->dev,
> +					pmsg->len, &phys, GFP_KERNEL);
> +				i2c_common->read_cfg.buf = buf;
> +				i2c_common->read_cfg.length = pmsg->len;
> +				i2c_common->read_cfg.phy_addr = phys;
> +			}
> +			dev->msg_buf = pmsg;
> +			amd_mp2_read(i2c_common->pdev,
> +				     i2c_common->read_cfg);
> +			timeout = wait_for_completion_timeout
> +					(&dev->msg_complete, 50);
> +			if (pmsg->len <= 32)
> +				kfree(buf);
> +			else
> +				dma_free_coherent(&i2c_common->pdev->dev,
> +						  pmsg->len, buf, phys);
> +		} else {
> +			i2c_common->write_cfg.buf = (unsigned int *)pmsg->buf;
> +			i2c_common->write_cfg.length = pmsg->len;
> +			amd_mp2_write(i2c_common->pdev,
> +				      i2c_common->write_cfg);
> +
> +			timeout = wait_for_completion_timeout
> +						(&dev->msg_complete, 50);
> +		}
> +	}
> +	return num;
> +}
> +
> +static u32 i2c_amd_func(struct i2c_adapter *a)
> +{
> +	return I2C_FUNC_I2C;
> +}
> +
> +static const struct i2c_algorithm i2c_amd_algorithm = {
> +	.master_xfer = i2c_amd_xfer,
> +	.functionality = i2c_amd_func,
> +};
> +
> +static int i2c_amd_probe(struct platform_device *pdev)
> +{
> +	int ret;
> +	struct amd_i2c_dev *i2c_dev;
> +	struct device *dev = &pdev->dev;
> +	acpi_handle handle = ACPI_HANDLE(&pdev->dev);
> +	struct acpi_device *adev;
> +	const char *uid = NULL;
> +
> +	i2c_dev = devm_kzalloc(dev, sizeof(*i2c_dev), GFP_KERNEL);
> +	if (!i2c_dev)
> +		return -ENOMEM;
> +
> +	i2c_dev->pdev = pdev;
> +
> +	if (!acpi_bus_get_device(handle, &adev)) {
> +		pr_err(" i2c0  pdev->id=%s\n", adev->pnp.unique_id);
> +		uid = adev->pnp.unique_id;
> +	}
> +
> +	if (strcmp(uid, "0") == 0) {
> +		pr_err(" bus id is 0\n");
> +		i2c_dev->bus_id = 0;
> +	}
> +
> +	pr_devel(" i2c1  pdev->id=%s\n", uid);
> +	if (strcmp(uid, "1") == 0) {
> +		pr_err(" bus id is 1\n");
> +		i2c_dev->bus_id = 1;
> +	}
> +	/* setup i2c adapter description */
> +	i2c_dev->adapter.owner = THIS_MODULE;
> +	i2c_dev->adapter.algo = &i2c_amd_algorithm;
> +	i2c_dev->adapter.dev.parent = dev;
> +	i2c_dev->adapter.algo_data = i2c_dev;
> +	ACPI_COMPANION_SET(&i2c_dev->adapter.dev, ACPI_COMPANION(&pdev->dev));
> +	i2c_dev->adapter.dev.of_node = dev->of_node;
> +	snprintf(i2c_dev->adapter.name, sizeof(i2c_dev->adapter.name), "%s-%s",
> +		 "i2c_dev-i2c", dev_name(pdev->dev.parent));
> +
> +	i2c_dev->i2c_common.pdev = pci_get_device(PCI_VENDOR_ID_AMD,
> +						  PCI_DEVICE_ID_AMD_MP2, NULL);
> +
> +	if (!i2c_dev->i2c_common.pdev) {
> +		pr_err("%s Could not find pdev in i2c\n", __func__);
> +		return -EINVAL;
> +	}
> +	platform_set_drvdata(pdev, i2c_dev);
> +
> +	i2c_set_adapdata(&i2c_dev->adapter, i2c_dev);
> +
> +	init_completion(&i2c_dev->msg_complete);
> +	/* and finally attach to i2c layer */
> +	ret = i2c_add_adapter(&i2c_dev->adapter);
> +
> +	if (ret < 0)
> +		pr_err(" i2c add adpater failed =%d", ret);
> +
> +	return ret;
> +}
> +
> +static int i2c_amd_remove(struct platform_device *pdev)
> +{
> +	struct amd_i2c_dev *i2c_dev = platform_get_drvdata(pdev);
> +
> +	i2c_del_adapter(&i2c_dev->adapter);
> +
> +	return 0;
> +}
> +
> +static const struct acpi_device_id i2c_amd_acpi_match[] = {
> +		{ "AMDI0011" },
> +		{ },
> +};
> +
> +static struct platform_driver amd_i2c_plat_driver = {
> +		.probe = i2c_amd_probe,
> +		.remove = i2c_amd_remove,
> +		.driver = {
> +				.name = "i2c_amd_platdrv",
> +				.acpi_match_table = ACPI_PTR
> +						(i2c_amd_acpi_match),
> +		},
> +};
> +
> +module_platform_driver(amd_i2c_plat_driver);
> +
> +MODULE_AUTHOR("Nehal Shah <nehal-bakulchandra.shah@amd.com>");
> +MODULE_DESCRIPTION("AMD I2C Platform Driver");
> +MODULE_LICENSE("Dual BSD/GPL");

On Thu, 23 Aug 2018 14:20:07 +0200
Peter Rosin <peda@axentia.se> wrote:

> A few comments inline, I didn't look all that long and now I have
> other things to attend to...

Hi,

A quick lunch time drive by review from me...

No in any way a thorough review but I was curious so might as well
let you know what I noticed!

Jonathan

> 
> On 2018-08-23 12:32, Shah, Nehal-bakulchandra wrote:
> > From: Nehal-bakulchandra Shah <Nehal-bakulchandra.Shah@amd.com>
> > 
> > This contains two drivers.
> > 1)i2c-amd-platdrv: This is based on I2C framework of
> > linux kernel. So any i2c read write call or commands
> > to this driver is routed to PCI Interface driver.
> > 2) i2c-amd-platdrv: This driver is responsible to
> > talk with Mp2 and does all C2P/P2C communication
> > or reading/writing from DRAM in case of more
> > data.
> > 
> > Reviewed-by: S-k, Shyam-sundar <Shyam-sundar.S-k@amd.com>
> > Reviewed-by: Sandeep Singh <sandeep.singh@amd.com>
> > Signed-off-by: Nehal-bakulchandra Shah <Nehal-bakulchandra.Shah@amd.com>
> > ---
> > Changes since v1:(https://www.spinics.net/lists/linux-i2c/msg34650.html)  
> > -> Add fix for IOMMU
> > -> Add depedency of ACPI
> > -> Add locks to avoid the crash  
> > 
> >  drivers/i2c/busses/Kconfig           |  10 +
> >  drivers/i2c/busses/Makefile          |   2 +
> >  drivers/i2c/busses/i2c-amd-pci-mp2.c | 626 +++++++++++++++++++++++++++++++++++
> >  drivers/i2c/busses/i2c-amd-pci-mp2.h | 253 ++++++++++++++
> >  drivers/i2c/busses/i2c-amd-platdrv.c | 324 ++++++++++++++++++
> >  5 files changed, 1215 insertions(+)
> >  create mode 100644 drivers/i2c/busses/i2c-amd-pci-mp2.c
> >  create mode 100644 drivers/i2c/busses/i2c-amd-pci-mp2.h
> >  create mode 100644 drivers/i2c/busses/i2c-amd-platdrv.c
> > 
> > diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
> > index 8d21b98..ff68424 100644
> > --- a/drivers/i2c/busses/Kconfig
> > +++ b/drivers/i2c/busses/Kconfig
> > @@ -77,6 +77,16 @@ config I2C_AMD8111
> >  	  This driver can also be built as a module.  If so, the module
> >  	  will be called i2c-amd8111.
> >  
> > +config I2C_AMD_MP2
> > +	tristate "AMD MP2"
> > +	depends on ACPI && PCI
> > +	help
> > +	  If you say yes to this option, support will be included for mp2
> > +	  I2C interface.
> > +
> > +	  This driver can also be built as a module.  If so, the module
> > +	  will be called i2c-amd-platdrv.
> > +
> >  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 189e34b..b01e46b 100644
> > --- a/drivers/i2c/busses/Makefile
> > +++ b/drivers/i2c/busses/Makefile
> > @@ -113,6 +113,8 @@ obj-$(CONFIG_I2C_XLR)		+= i2c-xlr.o
> >  obj-$(CONFIG_I2C_XLP9XX)	+= i2c-xlp9xx.o
> >  obj-$(CONFIG_I2C_RCAR)		+= i2c-rcar.o
> >  obj-$(CONFIG_I2C_ZX2967)	+= i2c-zx2967.o
> > +obj-$(CONFIG_I2C_AMD_MP2)   	+= i2c-amd-pci-mp2.o
> > +obj-$(CONFIG_I2C_AMD_MP2)   	+= i2c-amd-platdrv.o
> >  
> >  # External I2C/SMBus adapter drivers
> >  obj-$(CONFIG_I2C_DIOLAN_U2C)	+= i2c-diolan-u2c.o
> > diff --git a/drivers/i2c/busses/i2c-amd-pci-mp2.c b/drivers/i2c/busses/i2c-amd-pci-mp2.c
> > new file mode 100644
> > index 0000000..5eaec81
> > --- /dev/null
> > +++ b/drivers/i2c/busses/i2c-amd-pci-mp2.c
> > @@ -0,0 +1,626 @@
> > +// SPDX-License-Identifier: GPL-2.0  
> 
> GPL-2.0 OR BSD-3-Clause
> Goes for all new files in the patch.
> 
> > +/*
> > + * This file is provided under a dual BSD/GPLv2 license.  When using or
> > + *   redistributing this file, you may do so under either license.
> > + *
> > + *   GPL LICENSE SUMMARY
> > + *
> > + *   Copyright (C) 2018 Advanced Micro Devices, Inc. All Rights Reserved.
> > + *
> > + *   This program is free software; you can redistribute it and/or modify
> > + *   it under the terms of version 2 of the GNU General Public License as
> > + *   published by the Free Software Foundation.
> > + *
> > + *   BSD LICENSE
> > + *
> > + *   Copyright (C) 2018 Advanced Micro Devices, Inc. All Rights Reserved.
> > + *
> > + *   Redistribution and use in source and binary forms, with or without
> > + *   modification, are permitted provided that the following conditions
> > + *   are met:
> > + *
> > + *     * Redistributions of source code must retain the above copyright
> > + *       notice, this list of conditions and the following disclaimer.
> > + *     * Redistributions in binary form must reproduce the above copy
> > + *       notice, this list of conditions and the following disclaimer in
> > + *       the documentation and/or other materials provided with the
> > + *       distribution.
> > + *     * Neither the name of AMD Corporation nor the names of its
> > + *       contributors may be used to endorse or promote products derived
> > + *       from this software without specific prior written permission.
> > + *
> > + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> > + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> > + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> > + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> > + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> > + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> > + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> > + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> > + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> > + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> > + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> > + *
> > + * AMD PCIe MP2 Communication Driver
> > + * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> > + */  
> 
> Lose all the license boilerplate gunk, that's what the SPDX id is for.
> Goes for all new files in the patch.
> 
> > +
> > +#include <linux/debugfs.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/module.h>
> > +#include <linux/pci.h>
> > +#include <linux/slab.h>
> > +
> > +#include "i2c-amd-pci-mp2.h"
> > +
> > +#define DRIVER_NAME	"pcie_mp2_amd"
> > +#define DRIVER_DESC	"AMD(R) PCI-E MP2 Communication Driver"
> > +#define DRIVER_VER	"1.0"
> > +
> > +MODULE_DESCRIPTION(DRIVER_DESC);
> > +MODULE_VERSION(DRIVER_VER);
> > +MODULE_LICENSE("Dual BSD/GPL");
> > +MODULE_AUTHOR("Shyam Sundar S K <Shyam-sundar.S-k@amd.com>");
> > +
> > +static const struct file_operations amd_mp2_debugfs_info;
> > +static struct dentry *debugfs_dir;
> > +
> > +int amd_mp2_connect(struct pci_dev *dev,
> > +		    struct i2c_connect_config connect_cfg)
> > +{
> > +	struct amd_mp2_dev *privdata = pci_get_drvdata(dev);
> > +	union i2c_cmd_base i2c_cmd_base;
> > +	unsigned  long  flags;
> > +
> > +	raw_spin_lock_irqsave(&privdata->lock, flags);
> > +	dev_dbg(ndev_dev(privdata), "%s addr: %x id: %d\n", __func__,
> > +		connect_cfg.dev_addr, connect_cfg.bus_id);
> > +
> > +	i2c_cmd_base.ul = 0;
> > +	i2c_cmd_base.s.i2c_cmd = i2c_enable;
> > +	i2c_cmd_base.s.bus_id = connect_cfg.bus_id;
> > +	i2c_cmd_base.s.i2c_speed = connect_cfg.i2c_speed;
> > +
> > +	if (i2c_cmd_base.s.bus_id == i2c_bus_1) {
> > +		writel(i2c_cmd_base.ul, privdata->mmio + AMD_C2P_MSG1);
> > +	} else if (i2c_cmd_base.s.bus_id == i2c_bus_0) {
> > +		writel(i2c_cmd_base.ul, privdata->mmio + AMD_C2P_MSG0);
> > +	} else {
> > +		dev_err(ndev_dev(privdata), "%s Invalid bus id\n", __func__);
> > +		return -EINVAL;
> > +	}
> > +	raw_spin_unlock_irqrestore(&privdata->lock, flags);
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(amd_mp2_connect);
> > +
> > +int amd_mp2_read(struct pci_dev *dev, struct i2c_read_config read_cfg)
> > +{
> > +	struct amd_mp2_dev *privdata = pci_get_drvdata(dev);
> > +	union i2c_cmd_base i2c_cmd_base;
> > +
> > +	dev_dbg(ndev_dev(privdata), "%s addr: %x id: %d\n", __func__,
> > +		read_cfg.dev_addr, read_cfg.bus_id);
> > +
> > +	privdata->requested = true;
> > +	i2c_cmd_base.ul = 0;
> > +	i2c_cmd_base.s.i2c_cmd = i2c_read;
> > +	i2c_cmd_base.s.dev_addr = read_cfg.dev_addr;
> > +	i2c_cmd_base.s.length = read_cfg.length;
> > +	i2c_cmd_base.s.bus_id = read_cfg.bus_id;
> > +
> > +	if (read_cfg.length <= 32) {
> > +		i2c_cmd_base.s.mem_type = use_c2pmsg;
> > +		privdata->eventval.buf = (u32 *)read_cfg.buf;
> > +		if (!privdata->eventval.buf) {
> > +			dev_err(ndev_dev(privdata), "%s no mem for buf received\n",
> > +				__func__);
> > +			return -ENOMEM;
> > +		}
> > +		dev_dbg(ndev_dev(privdata), "%s buf: %llx\n", __func__,
> > +			(u64)privdata->eventval.buf);
> > +	} else {
> > +		i2c_cmd_base.s.mem_type = use_dram;
> > +		privdata->read_cfg.phy_addr = read_cfg.phy_addr;
> > +		privdata->read_cfg.buf = read_cfg.buf;
> > +		write64((u64)privdata->read_cfg.phy_addr,
> > +			privdata->mmio + AMD_C2P_MSG2);
> > +	}
> > +
> > +	switch (read_cfg.i2c_speed) {
> > +	case 0:
> > +		i2c_cmd_base.s.i2c_speed = speed100k;
> > +		break;
> > +	case 1:
> > +		i2c_cmd_base.s.i2c_speed = speed400k;
> > +		break;
> > +	case 2:
> > +		i2c_cmd_base.s.i2c_speed = speed1000k;
> > +		break;
> > +	case 3:
> > +		i2c_cmd_base.s.i2c_speed = speed1400k;
> > +		break;
> > +	case 4:
> > +		i2c_cmd_base.s.i2c_speed = speed3400k;
> > +		break;

Looks like a case for a look up table to me..
Though looking closer it seems to be a case of 
i2c_cmd_base.s.i2c_speed = read_cfg.i2c_speed;

> > +	default:
> > +		dev_err(ndev_dev(privdata), "Invalid ConnectionSpeed\n");
> > +	}
> > +
> > +	if (i2c_cmd_base.s.bus_id == i2c_bus_1) {
> > +		writel(i2c_cmd_base.ul, privdata->mmio + AMD_C2P_MSG1);
> > +	} else if (i2c_cmd_base.s.bus_id == i2c_bus_0) {
> > +		writel(i2c_cmd_base.ul, privdata->mmio + AMD_C2P_MSG0);
> > +	} else {
> > +		dev_err(ndev_dev(privdata), "%s Invalid bus id\n", __func__);
> > +		return -EINVAL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(amd_mp2_read);
> > +
> > +int amd_mp2_write(struct pci_dev *dev, struct i2c_write_config write_cfg)
> > +{
> > +	struct amd_mp2_dev *privdata = pci_get_drvdata(dev);
> > +	union i2c_cmd_base i2c_cmd_base;
> > +	int i = 0;
> > +
> > +	privdata->requested = true;
> > +	dev_dbg(ndev_dev(privdata), "%s addr: %x id: %d\n", __func__,
> > +		write_cfg.dev_addr, write_cfg.bus_id);
> > +
> > +	i2c_cmd_base.ul = 0;
> > +	i2c_cmd_base.s.i2c_cmd = i2c_write;
> > +	i2c_cmd_base.s.dev_addr = write_cfg.dev_addr;
> > +	i2c_cmd_base.s.length = write_cfg.length;
> > +	i2c_cmd_base.s.bus_id = write_cfg.bus_id;
> > +
> > +	switch (write_cfg.i2c_speed) {
> > +	case 0:
> > +		i2c_cmd_base.s.i2c_speed = speed100k;
> > +		break;
> > +	case 1:
> > +		i2c_cmd_base.s.i2c_speed = speed400k;
> > +		break;
> > +	case 2:
> > +		i2c_cmd_base.s.i2c_speed = speed1000k;
> > +		break;
> > +	case 3:
> > +		i2c_cmd_base.s.i2c_speed = speed1400k;
> > +		break;
> > +	case 4:
> > +		i2c_cmd_base.s.i2c_speed = speed3400k;
> > +		break;
> > +	default:
> > +		dev_err(ndev_dev(privdata), "Invalid ConnectionSpeed\n");
> > +	}
> > +
> > +	if (write_cfg.length <= 32) {
> > +		i2c_cmd_base.s.mem_type = use_c2pmsg;
> > +		for (i = 0; i < ((write_cfg.length + 3) / 4); i++) {
> > +			writel(write_cfg.buf[i],
> > +			       privdata->mmio + (AMD_C2P_MSG2 + i * 4));
> > +		}
> > +	} else {
> > +		i2c_cmd_base.s.mem_type = use_dram;
> > +		privdata->write_cfg.phy_addr = write_cfg.phy_addr;
> > +		write64((u64)privdata->write_cfg.phy_addr,
> > +			privdata->mmio + AMD_C2P_MSG2);
> > +	}
> > +
> > +	if (i2c_cmd_base.s.bus_id == i2c_bus_1) {
> > +		writel(i2c_cmd_base.ul, privdata->mmio + AMD_C2P_MSG1);
> > +	} else if (i2c_cmd_base.s.bus_id == i2c_bus_0) {
> > +		writel(i2c_cmd_base.ul, privdata->mmio + AMD_C2P_MSG0);
> > +	} else {
> > +		dev_err(ndev_dev(privdata), "%s Invalid bus id\n", __func__);
> > +		return -EINVAL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(amd_mp2_write);
> > +
> > +int amd_i2c_register_cb(struct pci_dev *dev, const struct amd_i2c_pci_ops *ops,
> > +			void *dev_ctx)
> > +{
> > +	struct amd_mp2_dev *privdata = pci_get_drvdata(dev);
> > +
> > +	privdata->ops = ops;
> > +	privdata->i2c_dev_ctx = dev_ctx;
> > +
> > +	if (!privdata->ops || !privdata->i2c_dev_ctx)
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(amd_i2c_register_cb);
> > +
> > +static void amd_mp2_pci_work(struct work_struct *work)
> > +{
> > +	struct amd_mp2_dev *privdata = mp2_dev(work);
> > +	u32 readdata = 0;
> > +	int i = 0;
> > +	int sts = privdata->eventval.base.r.status;
> > +	int res = privdata->eventval.base.r.response;
> > +	int len = privdata->eventval.base.r.length;
> > +
> > +	if (res == command_success && sts == i2c_readcomplete_event) {
> > +		if (privdata->ops->read_complete) {
> > +			if (len <= 32) {
> > +				for (i = 0; i < ((len + 3) / 4); i++) {
> > +					readdata = readl(privdata->mmio +
> > +							(AMD_C2P_MSG2 + i * 4));
> > +					privdata->eventval.buf[i] = readdata;
> > +				}
> > +				privdata->ops->read_complete(privdata->eventval,
> > +						privdata->i2c_dev_ctx);
> > +			} else {
> > +				privdata->ops->read_complete(privdata->eventval,
> > +						privdata->i2c_dev_ctx);
> > +			}
> > +		}
> > +	} else if (res == command_success && sts == i2c_writecomplete_event) {
> > +		if (privdata->ops->write_complete)
> > +			privdata->ops->write_complete(privdata->eventval,
> > +					privdata->i2c_dev_ctx);
> > +	} else if (res == command_success && sts == i2c_busenable_complete) {
> > +		if (privdata->ops->connect_complete)
> > +			privdata->ops->connect_complete(privdata->eventval,
> > +					privdata->i2c_dev_ctx);
> > +	} else {
> > +		dev_err(ndev_dev(privdata), "ERROR!!nothing to be handled !\n");
> > +	}
> > +}
> > +
> > +static irqreturn_t amd_mp2_irq_isr(int irq, void *dev)
> > +{
> > +	struct amd_mp2_dev *privdata = dev;
> > +	u32 val = 0;
> > +	unsigned long  flags;
> > +
> > +	raw_spin_lock_irqsave(&privdata->lock, flags);
> > +	val = readl(privdata->mmio + AMD_P2C_MSG1);
> > +	if (val != 0) {
> > +		writel(0, privdata->mmio + AMD_P2C_MSG_INTEN);
> > +		privdata->eventval.base.ul = val;
> > +	} else {
> > +		val = readl(privdata->mmio + AMD_P2C_MSG2);
> > +		if (val != 0) {
> > +			writel(0, privdata->mmio + AMD_P2C_MSG_INTEN);
> > +			privdata->eventval.base.ul = val;
> > +		}
> > +	}
> > +
> > +	raw_spin_unlock_irqrestore(&privdata->lock, flags);
> > +	if (!privdata->ops)
> > +		return IRQ_NONE;
> > +
> > +	if (!privdata->requested)
> > +		return IRQ_HANDLED;
> > +
> > +	privdata->requested = false;
> > +	schedule_delayed_work(&privdata->work, 0);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static ssize_t amd_mp2_debugfs_read(struct file *filp, char __user *ubuf,
> > +				    size_t count, loff_t *offp)
> > +{
> > +	struct amd_mp2_dev *privdata;
> > +	void __iomem *mmio;
> > +	u8 *buf;
> > +	size_t buf_size;
> > +	ssize_t ret, off;
> > +	union {
> > +		u64 v64;
> > +		u32 v32;
> > +		u16 v16;
> > +	} u;  
> 
> What is this union for? Seems totally pointless to me when the only
> thing you ever use from it is u.v32...
> 
> Cheers,
> Peter
> 
> > +
> > +	privdata = filp->private_data;
> > +	mmio = privdata->mmio;
> > +	buf_size = min(count, 0x800ul);
> > +	buf = kmalloc(buf_size, GFP_KERNEL);
> > +
> > +	if (!buf)
> > +		return -ENOMEM;
> > +
> > +	off = 0;
> > +	off += scnprintf(buf + off, buf_size - off,
> > +			"Mp2 Device Information:\n");
> > +
> > +	off += scnprintf(buf + off, buf_size - off,
> > +			"========================\n");
> > +	off += scnprintf(buf + off, buf_size - off,
> > +			"\tMP2 C2P Message Register Dump:\n\n");
> > +	u.v32 = readl(privdata->mmio + AMD_C2P_MSG0);
> > +	off += scnprintf(buf + off, buf_size - off,
> > +			"AMD_C2P_MSG0 -\t\t\t%#06x\n", u.v32);
> > +
> > +	u.v32 = readl(privdata->mmio + AMD_C2P_MSG1);
> > +	off += scnprintf(buf + off, buf_size - off,
> > +			"AMD_C2P_MSG1 -\t\t\t%#06x\n", u.v32);
> > +
> > +	u.v32 = readl(privdata->mmio + AMD_C2P_MSG2);
> > +	off += scnprintf(buf + off, buf_size - off,
> > +			"AMD_C2P_MSG2 -\t\t\t%#06x\n", u.v32);
> > +
> > +	u.v32 = readl(privdata->mmio + AMD_C2P_MSG3);
> > +	off += scnprintf(buf + off, buf_size - off,
> > +			"AMD_C2P_MSG3 -\t\t\t%#06x\n", u.v32);
> > +
> > +	u.v32 = readl(privdata->mmio + AMD_C2P_MSG4);
> > +	off += scnprintf(buf + off, buf_size - off,
> > +			"AMD_C2P_MSG4 -\t\t\t%#06x\n", u.v32);
> > +
> > +	u.v32 = readl(privdata->mmio + AMD_C2P_MSG5);
> > +	off += scnprintf(buf + off, buf_size - off,
> > +			"AMD_C2P_MSG5 -\t\t\t%#06x\n", u.v32);
> > +
> > +	u.v32 = readl(privdata->mmio + AMD_C2P_MSG6);
> > +	off += scnprintf(buf + off, buf_size - off,
> > +			"AMD_C2P_MSG6 -\t\t\t%#06x\n", u.v32);
> > +
> > +	u.v32 = readl(privdata->mmio + AMD_C2P_MSG7);
> > +	off += scnprintf(buf + off, buf_size - off,
> > +			"AMD_C2P_MSG7 -\t\t\t%#06x\n", u.v32);
> > +
> > +	u.v32 = readl(privdata->mmio + AMD_C2P_MSG8);
> > +	off += scnprintf(buf + off, buf_size - off,
> > +			"AMD_C2P_MSG8 -\t\t\t%#06x\n", u.v32);
> > +
> > +	u.v32 = readl(privdata->mmio + AMD_C2P_MSG9);
> > +	off += scnprintf(buf + off, buf_size - off,
> > +			"AMD_C2P_MSG9 -\t\t\t%#06x\n", u.v32);
> > +
> > +	off += scnprintf(buf + off, buf_size - off,
> > +			"\n\tMP2 P2C Message Register Dump:\n\n");
> > +
> > +	u.v32 = readl(privdata->mmio + AMD_P2C_MSG1);
> > +	off += scnprintf(buf + off, buf_size - off,
> > +			"AMD_P2C_MSG1 -\t\t\t%#06x\n", u.v32);
> > +
> > +	u.v32 = readl(privdata->mmio + AMD_P2C_MSG2);
> > +	off += scnprintf(buf + off, buf_size - off,
> > +			"AMD_P2C_MSG2 -\t\t\t%#06x\n", u.v32);
> > +
> > +	u.v32 = readl(privdata->mmio + AMD_P2C_MSG_INTEN);
> > +	off += scnprintf(buf + off, buf_size - off,
> > +			"AMD_P2C_MSG_INTEN -\t\t%#06x\n", u.v32);
> > +
> > +	u.v32 = readl(privdata->mmio + AMD_P2C_MSG_INTSTS);
> > +	off += scnprintf(buf + off, buf_size - off,
> > +			"AMD_P2C_MSG_INTSTS -\t\t%#06x\n", u.v32);
> > +
> > +	ret = simple_read_from_buffer(ubuf, count, offp, buf, off);
> > +	kfree(buf);
> > +	return ret;
> > +}
> > +
> > +static void amd_mp2_init_debugfs(struct amd_mp2_dev *privdata)
> > +{
> > +	if (!debugfs_dir) {
> > +		privdata->debugfs_dir = NULL;
> > +		privdata->debugfs_info = NULL;
> > +	} else {
> > +		privdata->debugfs_dir = debugfs_create_dir(ndev_name(privdata),
> > +							   debugfs_dir);
> > +		if (!privdata->debugfs_dir) {
> > +			privdata->debugfs_info = NULL;
> > +		} else {
> > +			privdata->debugfs_info = debugfs_create_file
> > +				("info", 0400, privdata->debugfs_dir,
> > +					privdata, &amd_mp2_debugfs_info);
> > +		}
> > +	}
> > +}
> > +
> > +static void amd_mp2_deinit_debugfs(struct amd_mp2_dev *privdata)
> > +{
> > +	debugfs_remove_recursive(privdata->debugfs_dir);
> > +}
> > +
> > +static void amd_mp2_clear_reg(struct amd_mp2_dev *privdata)
> > +{
> > +	int reg = 0;
> > +
> > +	for (reg = AMD_C2P_MSG0; reg <= AMD_C2P_MSG9; reg += 4)
> > +		writel(0, privdata->mmio + reg);
> > +
> > +	for (reg = AMD_P2C_MSG0; reg <= AMD_P2C_MSG2; reg += 4)
> > +		writel(0, privdata->mmio + reg);
> > +}
> > +
> > +static int amd_mp2_pci_init(struct amd_mp2_dev *privdata, struct pci_dev *pdev)
> > +{
> > +	int rc;
> > +	int bar_index = 2;
> > +	resource_size_t size, base;
> > +
> > +	pci_set_drvdata(pdev, privdata);
> > +
> > +	rc = pci_enable_device(pdev);
> > +	if (rc)
> > +		goto err_pci_enable;
> > +
> > +	rc = pci_request_regions(pdev, DRIVER_NAME);
> > +	if (rc)
> > +		goto err_pci_regions;
> > +
> > +	pci_set_master(pdev);
> > +
> > +	rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(64));
> > +	if (rc) {
> > +		rc = pci_set_dma_mask(pdev, DMA_BIT_MASK(32));
> > +		if (rc)
> > +			goto err_dma_mask;
> > +		dev_warn(ndev_dev(privdata), "Cannot DMA highmem\n");
> > +	}
> > +
> > +	rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(64));
> > +	if (rc) {
> > +		rc = pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32));
> > +		if (rc)
> > +			goto err_dma_mask;
> > +		dev_warn(ndev_dev(privdata), "Cannot DMA consistent highmem\n");
> > +	}
> > +
> > +	base = pci_resource_start(pdev, bar_index);
> > +	size = pci_resource_len(pdev, bar_index);
> > +	dev_dbg(ndev_dev(privdata), "Base addr:%llx size:%llx\n", base, size);
> > +
> > +	privdata->mmio = ioremap(base, size);
> > +	if (!privdata->mmio) {
> > +		rc = -EIO;
> > +		goto err_dma_mask;
> > +	}
> > +
> > +	/* Try to set up intx irq */
> > +	pci_intx(pdev, 1);
> > +	privdata->eventval.buf = NULL;
> > +	privdata->requested = false;
> > +	raw_spin_lock_init(&privdata->lock);
> > +	rc = request_irq(pdev->irq, amd_mp2_irq_isr, IRQF_SHARED, "mp2_irq_isr",
> > +			 privdata);
> > +	if (rc)
> > +		goto err_intx_request;
> > +
> > +	return 0;
> > +
> > +err_intx_request:
> > +	return rc;
> > +err_dma_mask:
> > +	pci_clear_master(pdev);
> > +	pci_release_regions(pdev);
> > +err_pci_regions:
> > +	pci_disable_device(pdev);
> > +err_pci_enable:
> > +	pci_set_drvdata(pdev, NULL);
> > +	return rc;
> > +}
> > +
> > +static void amd_mp2_pci_deinit(struct amd_mp2_dev *privdata)
> > +{
> > +	struct pci_dev *pdev = ndev_pdev(privdata);
> > +
> > +	pci_iounmap(pdev, privdata->mmio);
> > +
> > +	pci_clear_master(pdev);
> > +	pci_release_regions(pdev);
> > +	pci_disable_device(pdev);
> > +	pci_set_drvdata(pdev, NULL);
> > +}
> > +
> > +static int amd_mp2_pci_probe(struct pci_dev *pdev,
> > +			     const struct pci_device_id *id)
> > +{
> > +	struct amd_mp2_dev *privdata;
> > +	int rc;
> > +
> > +	dev_info(&pdev->dev, "MP2 device found [%04x:%04x] (rev %x)\n",
> > +		 (int)pdev->vendor, (int)pdev->device, (int)pdev->revision);
> > +
> > +	privdata = kzalloc(sizeof(*privdata), GFP_KERNEL);
> > +	privdata->pdev = pdev;
> > +
> > +	if (!privdata) {
> > +		rc = -ENOMEM;
> > +		goto err_dev;
> > +	}
> > +
> > +	rc = amd_mp2_pci_init(privdata, pdev);
> > +	if (rc)
> > +		goto err_pci_init;
> > +	dev_dbg(&pdev->dev, "pci init done.\n");
> > +
> > +	INIT_DELAYED_WORK(&privdata->work, amd_mp2_pci_work);
> > +
> > +	amd_mp2_init_debugfs(privdata);
> > +	dev_info(&pdev->dev, "MP2 device registered.\n");
> > +	return 0;
> > +
> > +err_pci_init:
> > +	kfree(privdata);
> > +err_dev:
> > +	dev_err(&pdev->dev, "Memory Allocation Failed\n");
> > +	return rc;
> > +}
> > +
> > +static void amd_mp2_pci_remove(struct pci_dev *pdev)
> > +{
> > +	struct amd_mp2_dev *privdata = pci_get_drvdata(pdev);
> > +
> > +	amd_mp2_deinit_debugfs(privdata);
> > +	amd_mp2_clear_reg(privdata);
> > +	cancel_delayed_work_sync(&privdata->work);
> > +	free_irq(pdev->irq, privdata);
> > +	pci_intx(pdev, 0);
> > +	amd_mp2_pci_deinit(privdata);
> > +	kfree(privdata);
> > +}
> > +
> > +static const struct file_operations amd_mp2_debugfs_info = {
> > +	.owner = THIS_MODULE,
> > +	.open = simple_open,
> > +	.read = amd_mp2_debugfs_read,
> > +};
> > +
> > +static const struct pci_device_id amd_mp2_pci_tbl[] = {
> > +	{PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_MP2)},
> > +	{0}
> > +};
> > +MODULE_DEVICE_TABLE(pci, amd_mp2_pci_tbl);
> > +
> > +#ifdef CONFIG_PM_SLEEP
> > +static int amd_mp2_pci_device_suspend(struct pci_dev *pdev, pm_message_t mesg)
> > +{
> > +	struct amd_mp2_dev *privdata = pci_get_drvdata(pdev);
> > +
> > +	if (!privdata)
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> > +
> > +static int amd_mp2_pci_device_resume(struct pci_dev *pdev)
> > +{
> > +	struct amd_mp2_dev *privdata = pci_get_drvdata(pdev);
> > +
> > +	if (!privdata)
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> > +#endif
> > +
> > +static struct pci_driver amd_mp2_pci_driver = {
> > +	.name		= DRIVER_NAME,
> > +	.id_table	= amd_mp2_pci_tbl,
> > +	.probe		= amd_mp2_pci_probe,
> > +	.remove		= amd_mp2_pci_remove,
> > +#ifdef CONFIG_PM_SLEEP
> > +	.suspend		= amd_mp2_pci_device_suspend,
> > +	.resume			= amd_mp2_pci_device_resume,
> > +#endif
> > +};
> > +
> > +static int __init amd_mp2_pci_driver_init(void)
> > +{
> > +	pr_info("%s: %s Version: %s\n", DRIVER_NAME, DRIVER_DESC, DRIVER_VER);
> > +
> > +	if (debugfs_initialized())
> > +		debugfs_dir = debugfs_create_dir(KBUILD_MODNAME, NULL);
> > +
> > +	return pci_register_driver(&amd_mp2_pci_driver);
> > +}
> > +module_init(amd_mp2_pci_driver_init);
> > +
> > +static void __exit amd_mp2_pci_driver_exit(void)
> > +{
> > +	pci_unregister_driver(&amd_mp2_pci_driver);
> > +	debugfs_remove_recursive(debugfs_dir);
> > +}
> > +module_exit(amd_mp2_pci_driver_exit);
> > diff --git a/drivers/i2c/busses/i2c-amd-pci-mp2.h b/drivers/i2c/busses/i2c-amd-pci-mp2.h
> > new file mode 100644
> > index 0000000..da77b9f
> > --- /dev/null
> > +++ b/drivers/i2c/busses/i2c-amd-pci-mp2.h
> > @@ -0,0 +1,253 @@
> > +/* SPDX-License-Identifier: GPL-2.0
> > + *
> > + * This file is provided under a dual BSD/GPLv2 license.  When using or
> > + *   redistributing this file, you may do so under either license.
> > + *
> > + *   GPL LICENSE SUMMARY
> > + *
> > + *   Copyright (C) 2018 Advanced Micro Devices, Inc. All Rights Reserved.
> > + *
> > + *   This program is free software; you can redistribute it and/or modify
> > + *   it under the terms of version 2 of the GNU General Public License as
> > + *   published by the Free Software Foundation.
> > + *
> > + *   BSD LICENSE
> > + *
> > + *   Copyright (C) 2018 Advanced Micro Devices, Inc. All Rights Reserved.
> > + *
> > + *   Redistribution and use in source and binary forms, with or without
> > + *   modification, are permitted provided that the following conditions
> > + *   are met:
> > + *
> > + *     * Redistributions of source code must retain the above copyright
> > + *       notice, this list of conditions and the following disclaimer.
> > + *     * Redistributions in binary form must reproduce the above copy
> > + *       notice, this list of conditions and the following disclaimer in
> > + *       the documentation and/or other materials provided with the
> > + *       distribution.
> > + *     * Neither the name of AMD Corporation nor the names of its
> > + *       contributors may be used to endorse or promote products derived
> > + *       from this software without specific prior written permission.
> > + *
> > + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> > + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> > + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> > + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> > + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> > + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> > + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> > + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> > + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> > + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> > + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> > + *
> > + * AMD PCIe MP2 Communication Interface Driver
> > + * Author: Shyam Sundar S K <Shyam-sundar.S-k@amd.com>
> > + */
> > +
> > +#ifndef I2C_AMD_PCI_MP2_H
> > +#define I2C_AMD_PCI_MP2_H
> > +
> > +#include <linux/pci.h>
> > +
> > +#define PCI_DEVICE_ID_AMD_MP2	0x15E6
> > +
> > +#define write64 _write64
> > +static inline void _write64(u64 val, void __iomem *mmio)
> > +{
> > +	writel(val, mmio);
> > +	writel(val >> 32, mmio + sizeof(u32));
> > +}
> > +
> > +#define read64 _read64
> > +static inline u64 _read64(void __iomem *mmio)
> > +{
> > +	u64 low, high;
> > +
> > +	low = readl(mmio);
> > +	high = readl(mmio + sizeof(u32));
> > +	return low | (high << 32);
> > +}
> > +
> > +enum {
> > +	/* MP2 C2P Message Registers */
> > +	AMD_C2P_MSG0 = 0x10500, /*MP2 Message for I2C0*/
> > +	AMD_C2P_MSG1 = 0x10504, /*MP2 Message for I2C1*/
> > +	AMD_C2P_MSG2 = 0x10508, /*DRAM Address Lo / Data 0*/
> > +	AMD_C2P_MSG3 = 0x1050c, /*DRAM Address HI / Data 1*/
> > +	AMD_C2P_MSG4 = 0x10510, /*Data 2*/
> > +	AMD_C2P_MSG5 = 0x10514, /*Data 3*/
> > +	AMD_C2P_MSG6 = 0x10518, /*Data 4*/
> > +	AMD_C2P_MSG7 = 0x1051c, /*Data 5*/
> > +	AMD_C2P_MSG8 = 0x10520, /*Data 6*/
> > +	AMD_C2P_MSG9 = 0x10524, /*Data 7*/
> > +
> > +	/* MP2 P2C Message Registers */
> > +	AMD_P2C_MSG0 = 0x10680, /*Do not use*/
> > +	AMD_P2C_MSG1 = 0x10684, /*I2c0 int reg*/
> > +	AMD_P2C_MSG2 = 0x10688, /*I2c1 int reg*/
> > +	AMD_P2C_MSG3 = 0x1068C, /*MP2 debug info*/
> > +	AMD_P2C_MSG_INTEN = 0x10690, /*MP2 int gen register*/
> > +	AMD_P2C_MSG_INTSTS = 0x10694, /*Interrupt sts*/
> > +};
> > +
> > +/* Command register data structures */
> > +
> > +enum i2c_cmd {
> > +	i2c_read,
> > +	i2c_write,
> > +	i2c_enable,
> > +	i2c_disable,
> > +	number_of_sensor_discovered,
> > +	is_mp2_active,
> > +	invalid_cmd = 0xF,
> > +};
> > +
> > +enum i2c_bus_index {
> > +	i2c_bus_0 = 0,
> > +	i2c_bus_1 = 1,
> > +	i2c_bus_max
> > +};
> > +
> > +enum speed_enum {
> > +	speed100k = 0,
> > +	speed400k = 1,
> > +	speed1000k = 2,
> > +	speed1400k = 3,
> > +	speed3400k = 4
> > +};
> > +
> > +enum mem_type {
> > +	use_dram = 0,
> > +	use_c2pmsg = 1,
> > +};
> > +
> > +union i2c_cmd_base {
> > +	u32 ul;
> > +	struct {
> > +		enum i2c_cmd i2c_cmd : 4; /*!< bit: 0..3 i2c R/W command */
> > +		enum i2c_bus_index bus_id : 4; /*!< bit: 4..7 i2c bus index */
> > +		u32 dev_addr : 8; /*!< bit: 8..15 device address or Bus Speed*/
> > +		u32 length : 12; /*!< bit: 16..29 read/write length */
> > +		enum speed_enum i2c_speed : 3; /*!< bit: 30 register address*/
> > +		enum mem_type mem_type : 1; /*!< bit: 15 mem type*/
> > +	} s; /*!< Structure used for bit access */
> > +};
> > +
> > +/* Response register data structures */
> > +
> > +/*Response - Response of SFI*/
> > +enum response_type {
> > +	invalid_response = 0,
> > +	command_success = 1,
> > +	command_failed = 2,
> > +};
> > +
> > +/*Status - Command ID to indicate a command*/
> > +enum status_type {
> > +	i2c_readcomplete_event = 0,
> > +	i2c_readfail_event = 1,
> > +	i2c_writecomplete_event = 2,
> > +	i2c_writefail_event = 3,
> > +	i2c_busenable_complete = 4,
> > +	i2c_busenable_failed = 5,
> > +	i2c_busdisable_complete = 6,
> > +	i2c_busdisable_failed = 7,
> > +	invalid_data_length = 8,
> > +	invalid_slave_address = 9,
> > +	invalid_i2cbus_id = 10,
> > +	invalid_dram_addr = 11,
> > +	invalid_command = 12,
> > +	mp2_active = 13,
> > +	numberof_sensors_discovered_resp = 14,
> > +	i2C_bus_notinitialized
> > +};
> > +
> > +struct i2c_event {
> > +	union {
> > +		u32 ul;
> > +		struct {
> > +			enum response_type response : 2; /*!< bit: 0..1 I2C res type */
> > +			enum status_type status : 5; /*!< bit: 2..6 status_type */
> > +			enum mem_type mem_type : 1; /*!< bit: 7 0-DRAM;1- C2PMsg o/p */
> > +			enum i2c_bus_index bus_id : 4; /*!< bit: 8..11 I2C Bus ID */
> > +			u32 length : 12; /*!< bit:16..29 length */
> > +			u32 slave_addr : 8; /*!< bit: 15 debug msg include in p2c msg */
> > +		} r; /*!< Structure used for bit access */

kernel-doc style would be nicer.

> > +	} base;
> > +	u32 *buf;
> > +};
> > +
> > +/* data structures for communication with I2c*/
> > +
> > +struct i2c_connect_config {
> > +	enum i2c_bus_index bus_id;
> > +	u64 i2c_speed;
> > +	u16 dev_addr;
> > +};
> > +
> > +struct i2c_write_config {
> > +	enum i2c_bus_index bus_id;
> > +	u64 i2c_speed;
> > +	u16 dev_addr;
> > +	u32 length;
> > +	phys_addr_t phy_addr;
> > +	u32 *buf;
> > +};
> > +
> > +struct i2c_read_config {
> > +	enum i2c_bus_index bus_id;
> > +	u64 i2c_speed;
> > +	u16 dev_addr;
> > +	u32 length;
> > +	phys_addr_t phy_addr;
> > +	u8 *buf;
> > +};
> > +
> > +// struct to send/receive data b/w pci and i2c drivers

Non kernel style comment.

> > +struct amd_i2c_pci_ops {
> > +	int (*read_complete)(struct i2c_event event, void *dev_ctx);
> > +	int (*write_complete)(struct i2c_event event, void *dev_ctx);
> > +	int (*connect_complete)(struct i2c_event event, void *dev_ctx);
> > +};
> > +
> > +struct amd_i2c_common {
> > +	struct i2c_connect_config connect_cfg;
> > +	struct i2c_read_config read_cfg;
> > +	struct i2c_write_config write_cfg;
> > +	const struct amd_i2c_pci_ops *ops;
> > +	struct pci_dev *pdev;
> > +};
> > +
> > +struct amd_mp2_dev {
> > +	struct pci_dev *pdev;
> > +	struct dentry *debugfs_dir;
> > +	struct dentry *debugfs_info;
> > +	void __iomem *mmio;
> > +	struct i2c_event eventval;
> > +	enum i2c_cmd reqcmd;
> > +	struct i2c_connect_config connect_cfg;
> > +	struct i2c_read_config read_cfg;
> > +	struct i2c_write_config write_cfg;
> > +	union i2c_cmd_base i2c_cmd_base;
> > +	const struct amd_i2c_pci_ops *ops;
> > +	struct delayed_work work;
> > +	void *i2c_dev_ctx;
> > +	bool	requested;

Odd spacing.

> > +	raw_spinlock_t lock;
> > +};
> > +
> > +int amd_mp2_read(struct pci_dev *pdev, struct i2c_read_config read_cfg);
> > +int amd_mp2_write(struct pci_dev *pdev,
> > +		  struct i2c_write_config write_cfg);
> > +int amd_mp2_connect(struct pci_dev *pdev,
> > +		    struct i2c_connect_config connect_cfg);
> > +int amd_i2c_register_cb(struct pci_dev *pdev, const struct amd_i2c_pci_ops *ops,
> > +			void *dev_ctx);
> > +
> > +#define ndev_pdev(ndev) ((ndev)->pdev)
> > +#define ndev_name(ndev) pci_name(ndev_pdev(ndev))
> > +#define ndev_dev(ndev) (&ndev_pdev(ndev)->dev)
> > +#define mp2_dev(__work) container_of(__work, struct amd_mp2_dev, work.work)
> > +
> > +#endif
> > diff --git a/drivers/i2c/busses/i2c-amd-platdrv.c b/drivers/i2c/busses/i2c-amd-platdrv.c
> > new file mode 100644
> > index 0000000..9965b26
> > --- /dev/null
> > +++ b/drivers/i2c/busses/i2c-amd-platdrv.c
> > @@ -0,0 +1,324 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + *
> > + * This file is provided under a dual BSD/GPLv2 license.  When using or
> > + *   redistributing this file, you may do so under either license.
> > + *
> > + *   GPL LICENSE SUMMARY
> > + *
> > + *   Copyright (C) 2018 Advanced Micro Devices, Inc. All Rights Reserved.
> > + *
> > + *   This program is free software; you can redistribute it and/or modify
> > + *   it under the terms of version 2 of the GNU General Public License as
> > + *   published by the Free Software Foundation.
> > + *
> > + *   BSD LICENSE
> > + *
> > + *   Copyright (C) 2018 Advanced Micro Devices, Inc. All Rights Reserved.
> > + *
> > + *   Redistribution and use in source and binary forms, with or without
> > + *   modification, are permitted provided that the following conditions
> > + *   are met:
> > + *
> > + *     * Redistributions of source code must retain the above copyright
> > + *       notice, this list of conditions and the following disclaimer.
> > + *     * Redistributions in binary form must reproduce the above copy
> > + *       notice, this list of conditions and the following disclaimer in
> > + *       the documentation and/or other materials provided with the
> > + *       distribution.
> > + *     * Neither the name of AMD Corporation nor the names of its
> > + *       contributors may be used to endorse or promote products derived
> > + *       from this software without specific prior written permission.
> > + *
> > + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> > + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> > + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> > + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> > + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> > + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> > + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> > + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> > + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> > + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> > + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> > + *
> > +
> > + * AMD I2C Platform Driver
> > + * Author: Nehal Bakulchandra Shah <Nehal-bakulchandra.shah@amd.com>
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/types.h>
> > +#include <linux/slab.h>
> > +#include <linux/i2c.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/acpi.h>
> > +#include <linux/delay.h>
> > +#include "i2c-amd-pci-mp2.h"

Ordering is a bit odd here.

> > +#include <linux/dma-mapping.h>
> > +#define  DRIVER_NAME "AMD-I2C-PLATDRV"
> > +
> > +struct amd_i2c_dev {
> > +	struct platform_device *pdev;
> > +	struct i2c_adapter adapter;
> > +	struct amd_i2c_common i2c_common;
> > +	struct completion msg_complete;
> > +	struct i2c_msg *msg_buf;
> > +	bool is_configured;
> > +	u8 bus_id;
> > +
No need for this blank line.
> > +};
> > +
> > +static int i2c_amd_read_completion(struct i2c_event event, void *dev_ctx)
> > +{
> > +	struct amd_i2c_dev *i2c_dev = (struct amd_i2c_dev *)dev_ctx;
> > +	struct amd_i2c_common *commond = &i2c_dev->i2c_common;
> > +	int i = 0;
> > +
> > +	if (event.base.r.status == i2c_readcomplete_event) {
> > +		if (event.base.r.length <= 32) {
> > +			pr_devel(" in %s i2c_dev->msg_buf :%p\n",
> > +				 __func__, i2c_dev->msg_buf);
> > +

Inconsistent ordering with the pr_devel before this one and after memcpy in
the block below.

> > +			memcpy(i2c_dev->msg_buf->buf,
> > +			       (unsigned char *)event.buf, event.base.r.length);
> > +
> > +			for (i = 0; i < ((event.base.r.length + 3) / 4); i++)
> > +				pr_devel("%s:%s readdata:%x\n",
> > +					 DRIVER_NAME, __func__, event.buf[i]);
> > +
> > +		} else {
> > +			memcpy(i2c_dev->msg_buf->buf,
> > +			       (unsigned char *)commond->read_cfg.buf,
> > +				event.base.r.length);
> > +			pr_devel("%s:%s virt:%llx phy_addr:%llx\n",
> > +				 DRIVER_NAME, __func__,
> > +				(u64)commond->read_cfg.buf,
> > +				(u64)commond->read_cfg.phy_addr);
> > +
> > +			for (i = 0; i < ((event.base.r.length + 3) / 4); i++)
> > +				pr_devel("%s:%s readdata:%x\n",
> > +					 DRIVER_NAME, __func__, ((unsigned int *)
> > +					commond->read_cfg.buf)[i]);
> > +		}
> > +
> > +		complete(&i2c_dev->msg_complete);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int i2c_amd_write_completion(struct i2c_event event, void *dev_ctx)
> > +{
> > +	struct amd_i2c_dev *i2c_dev = (struct amd_i2c_dev *)dev_ctx;
> > +
> > +	if (event.base.r.status == i2c_writecomplete_event)
> > +		complete(&i2c_dev->msg_complete);
> > +
> > +	return 0;
> > +}
> > +
> > +static int i2c_amd_connect_completion(struct i2c_event event, void *dev_ctx)
> > +{
> > +	struct amd_i2c_dev *i2c_dev = (struct amd_i2c_dev *)dev_ctx;
No need to cast void * explicitly.
> > +
> > +	if (event.base.r.status == i2c_busenable_complete)
> > +		complete(&i2c_dev->msg_complete);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct amd_i2c_pci_ops data_handler = {
> > +		.read_complete = i2c_amd_read_completion,
> > +		.write_complete = i2c_amd_write_completion,
> > +		.connect_complete = i2c_amd_connect_completion,
> > +};
> > +
> > +static int i2c_amd_pci_configure(struct amd_i2c_dev *i2c_dev, int slaveaddr)
> > +{
> > +	struct amd_i2c_common *i2c_common = &i2c_dev->i2c_common;
> > +	int ret;
> > +
> > +	amd_i2c_register_cb(i2c_common->pdev, &data_handler, (void *)i2c_dev);
> > +	i2c_common->connect_cfg.bus_id = i2c_dev->bus_id;
> > +	i2c_common->connect_cfg.dev_addr = slaveaddr;
> > +	i2c_common->connect_cfg.i2c_speed = speed400k;
> > +
> > +	ret = amd_mp2_connect(i2c_common->pdev, i2c_common->connect_cfg);
> > +	if (ret)
> > +		return -1;

More informative error value perhaps?

> > +
> > +	mdelay(100);
> > +
> > +	i2c_common->write_cfg.bus_id = i2c_dev->bus_id;
> > +	i2c_common->write_cfg.dev_addr = slaveaddr;
> > +	i2c_common->write_cfg.i2c_speed = speed400k;
> > +
> > +	i2c_common->read_cfg.bus_id = i2c_dev->bus_id;
> > +	i2c_common->read_cfg.dev_addr = slaveaddr;
> > +	i2c_common->read_cfg.i2c_speed = speed400k;
> > +
> > +	return 0;
> > +}
> > +
> > +static int i2c_amd_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs, int num)
> > +{
> > +	struct amd_i2c_dev *dev = i2c_get_adapdata(adap);
> > +	struct amd_i2c_common *i2c_common = &dev->i2c_common;
> > +
> > +	int i = 0;

Looks to be always set below.

> > +	unsigned long timeout;
> > +	struct i2c_msg *pmsg;
> > +	unsigned char *buf;
> > +
> > +	dma_addr_t phys;
> > +
> > +	reinit_completion(&dev->msg_complete);
> > +	if (dev->is_configured == 0) {
> > +		i2c_amd_pci_configure(dev, msgs->addr);
> > +		timeout = wait_for_completion_timeout(&dev->msg_complete, 50);
> > +		dev->is_configured = 1;
> > +	}
> > +
> > +	for (i = 0; i < num; i++) {
> > +		pmsg = &msgs[i];
> > +		if (pmsg->flags & I2C_M_RD) {
> > +			if (pmsg->len <= 32) {
> > +				buf = kzalloc(pmsg->len, GFP_KERNEL);
> > +				if (!buf)
> > +					return -ENOMEM;

Perhaps consider using the same buf array for all such cases? Can you
put one in the amd_i2c_dev structure for example and avoid constant
allocate then free cycles.

If you were to do that you could keep a preset up version of this
around and simply select it an set the length each time?

> > +				i2c_common->read_cfg.buf = buf;
> > +				i2c_common->read_cfg.length = pmsg->len;
> > +				i2c_common->read_cfg.phy_addr =
> > +							virt_to_phys(buf);
> > +			} else {
> > +				buf = (u8 *)dma_alloc_coherent(&i2c_common->pdev->dev,
No need to cast form a void *

Interesting that you use kzalloc above but not the zeroing form here... 
Might be fine but probably needs a comment to say why.

> > +					pmsg->len, &phys, GFP_KERNEL);
> > +				i2c_common->read_cfg.buf = buf;
> > +				i2c_common->read_cfg.length = pmsg->len;
> > +				i2c_common->read_cfg.phy_addr = phys;
> > +			}
> > +			dev->msg_buf = pmsg;
> > +			amd_mp2_read(i2c_common->pdev,
> > +				     i2c_common->read_cfg);
> > +			timeout = wait_for_completion_timeout
> > +					(&dev->msg_complete, 50);
> > +			if (pmsg->len <= 32)
> > +				kfree(buf);
> > +			else
> > +				dma_free_coherent(&i2c_common->pdev->dev,
> > +						  pmsg->len, buf, phys);

This might be more readable if you had separate read and write functions to
reduce the depth of indentation a few levels?

> > +		} else {
> > +			i2c_common->write_cfg.buf = (unsigned int *)pmsg->buf;
> > +			i2c_common->write_cfg.length = pmsg->len;
> > +			amd_mp2_write(i2c_common->pdev,
> > +				      i2c_common->write_cfg);
> > +
> > +			timeout = wait_for_completion_timeout
> > +						(&dev->msg_complete, 50);
> > +		}
> > +	}
> > +	return num;
> > +}
> > +
> > +static u32 i2c_amd_func(struct i2c_adapter *a)
> > +{
> > +	return I2C_FUNC_I2C;
> > +}
> > +
> > +static const struct i2c_algorithm i2c_amd_algorithm = {
> > +	.master_xfer = i2c_amd_xfer,
> > +	.functionality = i2c_amd_func,
> > +};
> > +
> > +static int i2c_amd_probe(struct platform_device *pdev)
> > +{
> > +	int ret;
> > +	struct amd_i2c_dev *i2c_dev;
> > +	struct device *dev = &pdev->dev;
> > +	acpi_handle handle = ACPI_HANDLE(&pdev->dev);
> > +	struct acpi_device *adev;
> > +	const char *uid = NULL;
> > +
> > +	i2c_dev = devm_kzalloc(dev, sizeof(*i2c_dev), GFP_KERNEL);
> > +	if (!i2c_dev)
> > +		return -ENOMEM;
> > +
> > +	i2c_dev->pdev = pdev;
> > +
> > +	if (!acpi_bus_get_device(handle, &adev)) {
> > +		pr_err(" i2c0  pdev->id=%s\n", adev->pnp.unique_id);
> > +		uid = adev->pnp.unique_id;
> > +	}
> > +
> > +	if (strcmp(uid, "0") == 0) {
> > +		pr_err(" bus id is 0\n");
> > +		i2c_dev->bus_id = 0;
> > +	}
> > +
> > +	pr_devel(" i2c1  pdev->id=%s\n", uid);
> > +	if (strcmp(uid, "1") == 0) {
> > +		pr_err(" bus id is 1\n");
> > +		i2c_dev->bus_id = 1;
> > +	}
> > +	/* setup i2c adapter description */
> > +	i2c_dev->adapter.owner = THIS_MODULE;
> > +	i2c_dev->adapter.algo = &i2c_amd_algorithm;
> > +	i2c_dev->adapter.dev.parent = dev;
> > +	i2c_dev->adapter.algo_data = i2c_dev;
> > +	ACPI_COMPANION_SET(&i2c_dev->adapter.dev, ACPI_COMPANION(&pdev->dev));
> > +	i2c_dev->adapter.dev.of_node = dev->of_node;
> > +	snprintf(i2c_dev->adapter.name, sizeof(i2c_dev->adapter.name), "%s-%s",
> > +		 "i2c_dev-i2c", dev_name(pdev->dev.parent));
> > +
> > +	i2c_dev->i2c_common.pdev = pci_get_device(PCI_VENDOR_ID_AMD,
> > +						  PCI_DEVICE_ID_AMD_MP2, NULL);
> > +
> > +	if (!i2c_dev->i2c_common.pdev) {
> > +		pr_err("%s Could not find pdev in i2c\n", __func__);
> > +		return -EINVAL;
> > +	}
> > +	platform_set_drvdata(pdev, i2c_dev);
> > +
> > +	i2c_set_adapdata(&i2c_dev->adapter, i2c_dev);
> > +
> > +	init_completion(&i2c_dev->msg_complete);
> > +	/* and finally attach to i2c layer */
> > +	ret = i2c_add_adapter(&i2c_dev->adapter);
> > +
> > +	if (ret < 0)
> > +		pr_err(" i2c add adpater failed =%d", ret);
> > +
> > +	return ret;
> > +}
> > +
> > +static int i2c_amd_remove(struct platform_device *pdev)
> > +{
> > +	struct amd_i2c_dev *i2c_dev = platform_get_drvdata(pdev);
> > +
> > +	i2c_del_adapter(&i2c_dev->adapter);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct acpi_device_id i2c_amd_acpi_match[] = {
> > +		{ "AMDI0011" },
> > +		{ },
> > +};
> > +
> > +static struct platform_driver amd_i2c_plat_driver = {
> > +		.probe = i2c_amd_probe,
> > +		.remove = i2c_amd_remove,
> > +		.driver = {
> > +				.name = "i2c_amd_platdrv",
> > +				.acpi_match_table = ACPI_PTR
> > +						(i2c_amd_acpi_match),
> > +		},
> > +};
> > +
> > +module_platform_driver(amd_i2c_plat_driver);
> > +
> > +MODULE_AUTHOR("Nehal Shah <nehal-bakulchandra.shah@amd.com>");
> > +MODULE_DESCRIPTION("AMD I2C Platform Driver");
> > +MODULE_LICENSE("Dual BSD/GPL");

Hi Nehal-bakulchandra,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on v4.18 next-20180822]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Shah-Nehal-bakulchandra/i2c-amd-I2C-Driver-based-on-PCI-Interface-for-upcoming-platform/20180823-232746
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: i386-allmodconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   In file included from include/linux/printk.h:332:0,
                    from include/linux/kernel.h:14,
                    from include/linux/list.h:9,
                    from include/linux/wait.h:7,
                    from include/linux/wait_bit.h:8,
                    from include/linux/fs.h:6,
                    from include/linux/debugfs.h:15,
                    from drivers/i2c/busses/i2c-amd-pci-mp2.c:48:
   drivers/i2c/busses/i2c-amd-pci-mp2.c: In function 'amd_mp2_read':
>> drivers/i2c/busses/i2c-amd-pci-mp2.c:121:4: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
       (u64)privdata->eventval.buf);
       ^
   include/linux/dynamic_debug.h:136:9: note: in definition of macro 'dynamic_dev_dbg'
          ##__VA_ARGS__);  \
            ^~~~~~~~~~~
>> drivers/i2c/busses/i2c-amd-pci-mp2.c:120:3: note: in expansion of macro 'dev_dbg'
      dev_dbg(ndev_dev(privdata), "%s buf: %llx\n", __func__,
      ^~~~~~~
   In file included from include/linux/list.h:9:0,
                    from include/linux/wait.h:7,
                    from include/linux/wait_bit.h:8,
                    from include/linux/fs.h:6,
                    from include/linux/debugfs.h:15,
                    from drivers/i2c/busses/i2c-amd-pci-mp2.c:48:
   drivers/i2c/busses/i2c-amd-pci-mp2.c: In function 'amd_mp2_debugfs_read':
   include/linux/kernel.h:812:29: warning: comparison of distinct pointer types lacks a cast
      (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
                                ^
   include/linux/kernel.h:826:4: note: in expansion of macro '__typecheck'
      (__typecheck(x, y) && __no_side_effects(x, y))
       ^~~~~~~~~~~
   include/linux/kernel.h:836:24: note: in expansion of macro '__safe_cmp'
     __builtin_choose_expr(__safe_cmp(x, y), \
                           ^~~~~~~~~~
   include/linux/kernel.h:845:19: note: in expansion of macro '__careful_cmp'
    #define min(x, y) __careful_cmp(x, y, <)
                      ^~~~~~~~~~~~~
>> drivers/i2c/busses/i2c-amd-pci-mp2.c:325:13: note: in expansion of macro 'min'
     buf_size = min(count, 0x800ul);
                ^~~
   In file included from include/linux/printk.h:332:0,
                    from include/linux/kernel.h:14,
                    from include/linux/list.h:9,
                    from include/linux/wait.h:7,
                    from include/linux/wait_bit.h:8,
                    from include/linux/fs.h:6,
                    from include/linux/debugfs.h:15,
                    from drivers/i2c/busses/i2c-amd-pci-mp2.c:48:
   drivers/i2c/busses/i2c-amd-pci-mp2.c: In function 'amd_mp2_pci_init':
>> drivers/i2c/busses/i2c-amd-pci-mp2.c:473:30: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 4 has type 'resource_size_t {aka unsigned int}' [-Wformat=]
     dev_dbg(ndev_dev(privdata), "Base addr:%llx size:%llx\n", base, size);
                                 ^
   include/linux/dynamic_debug.h:135:39: note: in definition of macro 'dynamic_dev_dbg'
      __dynamic_dev_dbg(&descriptor, dev, fmt, \
                                          ^~~
   drivers/i2c/busses/i2c-amd-pci-mp2.c:473:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(ndev_dev(privdata), "Base addr:%llx size:%llx\n", base, size);
     ^~~~~~~
   drivers/i2c/busses/i2c-amd-pci-mp2.c:473:30: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 5 has type 'resource_size_t {aka unsigned int}' [-Wformat=]
     dev_dbg(ndev_dev(privdata), "Base addr:%llx size:%llx\n", base, size);
                                 ^
   include/linux/dynamic_debug.h:135:39: note: in definition of macro 'dynamic_dev_dbg'
      __dynamic_dev_dbg(&descriptor, dev, fmt, \
                                          ^~~
   drivers/i2c/busses/i2c-amd-pci-mp2.c:473:2: note: in expansion of macro 'dev_dbg'
     dev_dbg(ndev_dev(privdata), "Base addr:%llx size:%llx\n", base, size);
     ^~~~~~~
--
   In file included from include/linux/kernel.h:14:0,
                    from drivers/i2c/busses/i2c-amd-platdrv.c:50:
   drivers/i2c/busses/i2c-amd-platdrv.c: In function 'i2c_amd_read_completion':
>> drivers/i2c/busses/i2c-amd-platdrv.c:97:5: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
        (u64)commond->read_cfg.buf,
        ^
   include/linux/printk.h:323:35: note: in definition of macro 'pr_devel'
     printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__)
                                      ^~~~~~~~~~~

vim +121 drivers/i2c/busses/i2c-amd-pci-mp2.c

  > 48	#include <linux/debugfs.h>
    49	#include <linux/interrupt.h>
    50	#include <linux/module.h>
    51	#include <linux/pci.h>
    52	#include <linux/slab.h>
    53	
    54	#include "i2c-amd-pci-mp2.h"
    55	
    56	#define DRIVER_NAME	"pcie_mp2_amd"
    57	#define DRIVER_DESC	"AMD(R) PCI-E MP2 Communication Driver"
    58	#define DRIVER_VER	"1.0"
    59	
    60	MODULE_DESCRIPTION(DRIVER_DESC);
    61	MODULE_VERSION(DRIVER_VER);
    62	MODULE_LICENSE("Dual BSD/GPL");
    63	MODULE_AUTHOR("Shyam Sundar S K <Shyam-sundar.S-k@amd.com>");
    64	
    65	static const struct file_operations amd_mp2_debugfs_info;
    66	static struct dentry *debugfs_dir;
    67	
    68	int amd_mp2_connect(struct pci_dev *dev,
    69			    struct i2c_connect_config connect_cfg)
    70	{
    71		struct amd_mp2_dev *privdata = pci_get_drvdata(dev);
    72		union i2c_cmd_base i2c_cmd_base;
    73		unsigned  long  flags;
    74	
    75		raw_spin_lock_irqsave(&privdata->lock, flags);
    76		dev_dbg(ndev_dev(privdata), "%s addr: %x id: %d\n", __func__,
    77			connect_cfg.dev_addr, connect_cfg.bus_id);
    78	
    79		i2c_cmd_base.ul = 0;
    80		i2c_cmd_base.s.i2c_cmd = i2c_enable;
    81		i2c_cmd_base.s.bus_id = connect_cfg.bus_id;
    82		i2c_cmd_base.s.i2c_speed = connect_cfg.i2c_speed;
    83	
    84		if (i2c_cmd_base.s.bus_id == i2c_bus_1) {
    85			writel(i2c_cmd_base.ul, privdata->mmio + AMD_C2P_MSG1);
    86		} else if (i2c_cmd_base.s.bus_id == i2c_bus_0) {
    87			writel(i2c_cmd_base.ul, privdata->mmio + AMD_C2P_MSG0);
    88		} else {
    89			dev_err(ndev_dev(privdata), "%s Invalid bus id\n", __func__);
    90			return -EINVAL;
    91		}
    92		raw_spin_unlock_irqrestore(&privdata->lock, flags);
    93		return 0;
    94	}
    95	EXPORT_SYMBOL_GPL(amd_mp2_connect);
    96	
    97	int amd_mp2_read(struct pci_dev *dev, struct i2c_read_config read_cfg)
    98	{
    99		struct amd_mp2_dev *privdata = pci_get_drvdata(dev);
   100		union i2c_cmd_base i2c_cmd_base;
   101	
   102		dev_dbg(ndev_dev(privdata), "%s addr: %x id: %d\n", __func__,
   103			read_cfg.dev_addr, read_cfg.bus_id);
   104	
   105		privdata->requested = true;
   106		i2c_cmd_base.ul = 0;
   107		i2c_cmd_base.s.i2c_cmd = i2c_read;
   108		i2c_cmd_base.s.dev_addr = read_cfg.dev_addr;
   109		i2c_cmd_base.s.length = read_cfg.length;
   110		i2c_cmd_base.s.bus_id = read_cfg.bus_id;
   111	
   112		if (read_cfg.length <= 32) {
   113			i2c_cmd_base.s.mem_type = use_c2pmsg;
   114			privdata->eventval.buf = (u32 *)read_cfg.buf;
   115			if (!privdata->eventval.buf) {
   116				dev_err(ndev_dev(privdata), "%s no mem for buf received\n",
   117					__func__);
   118				return -ENOMEM;
   119			}
 > 120			dev_dbg(ndev_dev(privdata), "%s buf: %llx\n", __func__,
 > 121				(u64)privdata->eventval.buf);
   122		} else {
   123			i2c_cmd_base.s.mem_type = use_dram;
   124			privdata->read_cfg.phy_addr = read_cfg.phy_addr;
   125			privdata->read_cfg.buf = read_cfg.buf;
   126			write64((u64)privdata->read_cfg.phy_addr,
   127				privdata->mmio + AMD_C2P_MSG2);
   128		}
   129	
   130		switch (read_cfg.i2c_speed) {
   131		case 0:
   132			i2c_cmd_base.s.i2c_speed = speed100k;
   133			break;
   134		case 1:
   135			i2c_cmd_base.s.i2c_speed = speed400k;
   136			break;
   137		case 2:
   138			i2c_cmd_base.s.i2c_speed = speed1000k;
   139			break;
   140		case 3:
   141			i2c_cmd_base.s.i2c_speed = speed1400k;
   142			break;
   143		case 4:
   144			i2c_cmd_base.s.i2c_speed = speed3400k;
   145			break;
   146		default:
   147			dev_err(ndev_dev(privdata), "Invalid ConnectionSpeed\n");
   148		}
   149	
   150		if (i2c_cmd_base.s.bus_id == i2c_bus_1) {
   151			writel(i2c_cmd_base.ul, privdata->mmio + AMD_C2P_MSG1);
   152		} else if (i2c_cmd_base.s.bus_id == i2c_bus_0) {
   153			writel(i2c_cmd_base.ul, privdata->mmio + AMD_C2P_MSG0);
   154		} else {
   155			dev_err(ndev_dev(privdata), "%s Invalid bus id\n", __func__);
   156			return -EINVAL;
   157		}
   158	
   159		return 0;
   160	}
   161	EXPORT_SYMBOL_GPL(amd_mp2_read);
   162	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Hi Nehal-bakulchandra,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on wsa/i2c/for-next]
[also build test WARNING on v4.18 next-20180822]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Shah-Nehal-bakulchandra/i2c-amd-I2C-Driver-based-on-PCI-Interface-for-upcoming-platform/20180823-232746
base:   https://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-next
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

   drivers/i2c//busses/i2c-amd-pci-mp2.c: In function 'amd_mp2_read':
   drivers/i2c//busses/i2c-amd-pci-mp2.c:120:82: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
      dev_dbg(ndev_dev(privdata), "%s buf: %llx\n", __func__,
                                                                                     ^
   drivers/i2c//busses/i2c-amd-pci-mp2.c: In function 'amd_mp2_debugfs_read':
>> drivers/i2c//busses/i2c-amd-pci-mp2.c:325:66: warning: comparison of distinct pointer types lacks a cast
     buf_size = min(count, 0x800ul);
                                                                     ^ 
   drivers/i2c//busses/i2c-amd-pci-mp2.c: In function 'amd_mp2_pci_init':
   drivers/i2c//busses/i2c-amd-pci-mp2.c:473:53: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 4 has type 'resource_size_t {aka unsigned int}' [-Wformat=]
     dev_dbg(ndev_dev(privdata), "Base addr:%llx size:%llx\n", base, size);
                                                        ^~~~~~~~~~~~~~~~~~~         
   drivers/i2c//busses/i2c-amd-pci-mp2.c:473:53: warning: format '%llx' expects argument of type 'long long unsigned int', but argument 5 has type 'resource_size_t {aka unsigned int}' [-Wformat=]
smatch warnings:
drivers/i2c/busses/i2c-amd-pci-mp2.c:93 amd_mp2_connect() warn: inconsistent returns 'spin_lock:&privdata->lock'.
  Locked on:   line 90
  Unlocked on: line 93
drivers/i2c/busses/i2c-amd-pci-mp2.c:93 amd_mp2_connect() warn: inconsistent returns 'irqsave:flags'.
  Locked on:   line 90
  Unlocked on: line 93
drivers/i2c/busses/i2c-amd-pci-mp2.c:120 amd_mp2_read() warn: argument 5 to %llx specifier is cast from pointer
drivers/i2c/busses/i2c-amd-pci-mp2.c:527 amd_mp2_pci_probe() error: potential null dereference 'privdata'.  (kzalloc returns null)
drivers/i2c/busses/i2c-amd-pci-mp2.c:529 amd_mp2_pci_probe() warn: variable dereferenced before check 'privdata' (see line 527)

# https://github.com/0day-ci/linux/commit/da2ae4a10759e5b6bfc7fb3afd91d08e66cf19c0
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout da2ae4a10759e5b6bfc7fb3afd91d08e66cf19c0
vim +93 drivers/i2c/busses/i2c-amd-pci-mp2.c

da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   67  
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   68  int amd_mp2_connect(struct pci_dev *dev,
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   69  		    struct i2c_connect_config connect_cfg)
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   70  {
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   71  	struct amd_mp2_dev *privdata = pci_get_drvdata(dev);
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   72  	union i2c_cmd_base i2c_cmd_base;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   73  	unsigned  long  flags;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   74  
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   75  	raw_spin_lock_irqsave(&privdata->lock, flags);
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   76  	dev_dbg(ndev_dev(privdata), "%s addr: %x id: %d\n", __func__,
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   77  		connect_cfg.dev_addr, connect_cfg.bus_id);
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   78  
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   79  	i2c_cmd_base.ul = 0;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   80  	i2c_cmd_base.s.i2c_cmd = i2c_enable;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   81  	i2c_cmd_base.s.bus_id = connect_cfg.bus_id;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   82  	i2c_cmd_base.s.i2c_speed = connect_cfg.i2c_speed;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   83  
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   84  	if (i2c_cmd_base.s.bus_id == i2c_bus_1) {
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   85  		writel(i2c_cmd_base.ul, privdata->mmio + AMD_C2P_MSG1);
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   86  	} else if (i2c_cmd_base.s.bus_id == i2c_bus_0) {
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   87  		writel(i2c_cmd_base.ul, privdata->mmio + AMD_C2P_MSG0);
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   88  	} else {
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   89  		dev_err(ndev_dev(privdata), "%s Invalid bus id\n", __func__);
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   90  		return -EINVAL;
                                                                 ^^^^^^^^^^^^^
Need to unlock.

da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   91  	}
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   92  	raw_spin_unlock_irqrestore(&privdata->lock, flags);
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  @93  	return 0;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   94  }
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   95  EXPORT_SYMBOL_GPL(amd_mp2_connect);
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   96  
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   97  int amd_mp2_read(struct pci_dev *dev, struct i2c_read_config read_cfg)
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   98  {
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23   99  	struct amd_mp2_dev *privdata = pci_get_drvdata(dev);
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  100  	union i2c_cmd_base i2c_cmd_base;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  101  
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  102  	dev_dbg(ndev_dev(privdata), "%s addr: %x id: %d\n", __func__,
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  103  		read_cfg.dev_addr, read_cfg.bus_id);
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  104  
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  105  	privdata->requested = true;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  106  	i2c_cmd_base.ul = 0;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  107  	i2c_cmd_base.s.i2c_cmd = i2c_read;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  108  	i2c_cmd_base.s.dev_addr = read_cfg.dev_addr;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  109  	i2c_cmd_base.s.length = read_cfg.length;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  110  	i2c_cmd_base.s.bus_id = read_cfg.bus_id;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  111  
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  112  	if (read_cfg.length <= 32) {
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  113  		i2c_cmd_base.s.mem_type = use_c2pmsg;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  114  		privdata->eventval.buf = (u32 *)read_cfg.buf;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  115  		if (!privdata->eventval.buf) {
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  116  			dev_err(ndev_dev(privdata), "%s no mem for buf received\n",
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  117  				__func__);
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  118  			return -ENOMEM;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  119  		}
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23 @120  		dev_dbg(ndev_dev(privdata), "%s buf: %llx\n", __func__,
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  121  			(u64)privdata->eventval.buf);
                                                                             ^^^^^^^^^^^^^^^^^^^^^^
This is a pointer so we want to hide that in the dmesg unless people
have enough privileges or whatever.

da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  122  	} else {
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  123  		i2c_cmd_base.s.mem_type = use_dram;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  124  		privdata->read_cfg.phy_addr = read_cfg.phy_addr;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  125  		privdata->read_cfg.buf = read_cfg.buf;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  126  		write64((u64)privdata->read_cfg.phy_addr,
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  127  			privdata->mmio + AMD_C2P_MSG2);
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  128  	}
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  129  
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  130  	switch (read_cfg.i2c_speed) {
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  131  	case 0:
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  132  		i2c_cmd_base.s.i2c_speed = speed100k;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  133  		break;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  134  	case 1:
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  135  		i2c_cmd_base.s.i2c_speed = speed400k;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  136  		break;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  137  	case 2:
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  138  		i2c_cmd_base.s.i2c_speed = speed1000k;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  139  		break;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  140  	case 3:
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  141  		i2c_cmd_base.s.i2c_speed = speed1400k;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  142  		break;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  143  	case 4:
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  144  		i2c_cmd_base.s.i2c_speed = speed3400k;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  145  		break;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  146  	default:
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  147  		dev_err(ndev_dev(privdata), "Invalid ConnectionSpeed\n");
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  148  	}
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  149  
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  150  	if (i2c_cmd_base.s.bus_id == i2c_bus_1) {
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  151  		writel(i2c_cmd_base.ul, privdata->mmio + AMD_C2P_MSG1);
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  152  	} else if (i2c_cmd_base.s.bus_id == i2c_bus_0) {
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  153  		writel(i2c_cmd_base.ul, privdata->mmio + AMD_C2P_MSG0);
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  154  	} else {
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  155  		dev_err(ndev_dev(privdata), "%s Invalid bus id\n", __func__);
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  156  		return -EINVAL;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  157  	}
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  158  
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  159  	return 0;
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  160  }
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  161  EXPORT_SYMBOL_GPL(amd_mp2_read);
da2ae4a1 Nehal-bakulchandra Shah 2018-08-23  162  

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation