@@ -111,11 +111,6 @@ int amd_mp2_read(struct pci_dev *dev, struct i2c_read_config read_cfg)
if (read_cfg.length <= 32) {
i2c_cmd_base.s.mem_type = use_c2pmsg;
- if (!privdata->eventval.buf) {
- dev_err(ndev_dev(privdata), "%s no mem for buf received\n",
- __func__);
- return -ENOMEM;
- }
privdata->eventval.buf = (u32 *)read_cfg.buf;
dev_dbg(ndev_dev(privdata), "%s buf: %llx\n", __func__,
(u64)privdata->eventval.buf);
@@ -164,7 +159,8 @@ 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;
+ int i;
+ int buf_len;
privdata->requested = true;
dev_dbg(ndev_dev(privdata), "%s addr: %x id: %d\n", __func__,
@@ -198,7 +194,8 @@ int amd_mp2_write(struct pci_dev *dev, struct i2c_write_config write_cfg)
if (write_cfg.length <= 32) {
i2c_cmd_base.s.mem_type = use_c2pmsg;
- for (i = 0; i < ((write_cfg.length + 3) / 4); i++) {
+ buf_len = (write_cfg.length + 3) / 4;
+ for (i = 0; i < buf_len; i++) {
writel(write_cfg.buf[i],
privdata->mmio + (AMD_C2P_MSG2 + i * 4));
}
@@ -241,15 +238,17 @@ 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.r.status;
- int res = privdata->eventval.r.response;
- int len = privdata->eventval.r.length;
+ int i;
+ int buf_len;
+ 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++) {
+ buf_len = (len + 3) / 4;
+ for (i = 0; i < buf_len; i++) {
readdata = readl(privdata->mmio +
(AMD_C2P_MSG2 + i * 4));
privdata->eventval.buf[i] = readdata;
@@ -284,12 +283,12 @@ static irqreturn_t amd_mp2_irq_isr(int irq, void *dev)
val = readl(privdata->mmio + AMD_P2C_MSG1);
if (val != 0) {
writel(0, privdata->mmio + AMD_P2C_MSG_INTEN);
- privdata->eventval.ul = val;
+ 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.ul = val;
+ privdata->eventval.base.ul = val;
}
}
@@ -163,16 +163,18 @@ enum status_type {
i2C_bus_notinitialized
};
-union i2c_event_base {
- 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 */
+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;
};
@@ -204,9 +206,9 @@ struct i2c_read_config {
// struct to send/receive data b/w pci and i2c drivers
struct amd_i2c_pci_ops {
- int (*read_complete)(union i2c_event_base event, void *dev_ctx);
- int (*write_complete)(union i2c_event_base event, void *dev_ctx);
- int (*connect_complete)(union i2c_event_base event, void *dev_ctx);
+ 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 {
@@ -222,7 +224,7 @@ struct amd_mp2_dev {
struct dentry *debugfs_dir;
struct dentry *debugfs_info;
void __iomem *mmio;
- union i2c_event_base eventval;
+ struct i2c_event eventval;
enum i2c_cmd reqcmd;
struct i2c_connect_config connect_cfg;
struct i2c_read_config read_cfg;
@@ -71,34 +71,37 @@ struct amd_i2c_dev {
};
-static int i2c_amd_read_completion(union i2c_event_base event, void *dev_ctx)
+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;
+ int i;
+ int buf_len;
- if (event.r.status == i2c_readcomplete_event) {
- if (event.r.length <= 32) {
+ 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.r.length);
+ (unsigned char *)event.buf, event.base.r.length);
- for (i = 0; i < ((event.r.length + 3) / 4); i++)
+ buf_len = (event.base.r.length + 3) / 4;
+ for (i = 0; i < buf_len; 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.r.length);
+ 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.r.length + 3) / 4); i++)
+ buf_len = (event.base.r.length + 3) / 4;
+ for (i = 0; i < buf_len; i++)
pr_devel("%s:%s readdata:%x\n",
DRIVER_NAME, __func__, ((unsigned int *)
commond->read_cfg.buf)[i]);
@@ -110,21 +113,21 @@ static int i2c_amd_read_completion(union i2c_event_base event, void *dev_ctx)
return 0;
}
-static int i2c_amd_write_completion(union i2c_event_base event, void *dev_ctx)
+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.r.status == i2c_writecomplete_event)
+ if (event.base.r.status == i2c_writecomplete_event)
complete(&i2c_dev->msg_complete);
return 0;
}
-static int i2c_amd_connect_completion(union i2c_event_base event, void *dev_ctx)
+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.r.status == i2c_busenable_complete)
+ if (event.base.r.status == i2c_busenable_complete)
complete(&i2c_dev->msg_complete);
return 0;
@@ -168,7 +171,7 @@ 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;
+ int i;
unsigned long timeout;
struct i2c_msg *pmsg;
unsigned char *dma_buf = NULL;
"Unable to handle kernel paging request" occurs at this line of code: privdata->eventval.buf[i] = readdata; The pointer "buf" in union "i2c_event_base" will change its value as soon as any union member gets modified. We can observe this behaviour in amd_mp2_irq_isr(). This makes buf points to the wrong place, and causes the trouble we saw when it gets accessed. Put pointer inside union is not a good idea, so refactor the structure to make sure the pointer and the union do not overlap. Also remove the NULL check for eventval.buf, as its value comes from read_cfg.buf. Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> --- drivers/i2c/busses/i2c-amd-pci-mp2.c | 27 ++++++++++++------------- drivers/i2c/busses/i2c-amd-pci-mp2.h | 30 +++++++++++++++------------- drivers/i2c/busses/i2c-amd-platdrv.c | 29 +++++++++++++++------------ 3 files changed, 45 insertions(+), 41 deletions(-)