Messages in this thread | | | From | Joel Stanley <> | Date | Fri, 10 Feb 2017 16:01:28 +1030 | Subject | Re: [PATCH linux v7 3/6] hwmon: occ: Add I2C transport implementation for SCOM operations |
| |
On Wed, Feb 8, 2017 at 9:40 AM, <eajames@linux.vnet.ibm.com> wrote: > From: "Edward A. James" <eajames@us.ibm.com> > > Add functions to send SCOM operations over I2C bus. The BMC can > communicate with the Power8 host processor over I2C, but needs to use SCOM > operations in order to access the OCC register space.
This doesn't need to be separate from the p8_occ_i2c.c file. You can remove a layer of function calls by merging this in and having these be your getscom putscom bus_ops callbacks.
> Signed-off-by: Edward A. James <eajames@us.ibm.com> > Signed-off-by: Andrew Jeffery <andrew@aj.id.au> > --- > drivers/hwmon/occ/occ_scom_i2c.c | 77 ++++++++++++++++++++++++++++++++++++++++ > drivers/hwmon/occ/occ_scom_i2c.h | 26 ++++++++++++++ > 2 files changed, 103 insertions(+) > create mode 100644 drivers/hwmon/occ/occ_scom_i2c.c > create mode 100644 drivers/hwmon/occ/occ_scom_i2c.h > > diff --git a/drivers/hwmon/occ/occ_scom_i2c.c b/drivers/hwmon/occ/occ_scom_i2c.c > new file mode 100644 > index 0000000..74bd6ff > --- /dev/null > +++ b/drivers/hwmon/occ/occ_scom_i2c.c > @@ -0,0 +1,77 @@
> + > +int occ_i2c_getscom(void *bus, u32 address, u64 *data) > +{ > + ssize_t rc; > + u64 buf;
If you add endianness annotations sparse can check your types are consistent. The warning looks like this:
make C=2 drivers/hwmon/occ/occ_scom_i2c.o drivers/hwmon/occ/occ_scom_i2c.c:48:17: warning: cast to restricted __be64
Which tells you it expects the type you pass to be64_to_cpu to be __be64.
> + struct i2c_client *client = bus; > + struct i2c_msg msgs[2]; > + > + msgs[0].addr = client->addr; > + msgs[0].flags = client->flags & I2C_M_TEN; > + msgs[0].len = sizeof(u32); > + msgs[0].buf = (char *)&address; > + > + msgs[1].addr = client->addr; > + msgs[1].flags = client->flags & I2C_M_TEN; > + msgs[1].flags |= I2C_M_RD;
I first thought you had made a mistake here. Instead you could do:
msgs[1].flags = client->flags & I2C_M_TEN | I2C_M_RD;
> + msgs[1].len = sizeof(u64); > + msgs[1].buf = (char *)&buf; > + > + rc = i2c_transfer(client->adapter, msgs, 2); > + if (rc < 0) > + return rc; > +
| |