| Message ID | 20210716151850.28973-3-eajames@linux.ibm.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | OCC: fsi and hwmon: Set sequence number in submit interface | expand |
On Fri, Jul 16, 2021 at 10:18:49AM -0500, Eddie James wrote: > Checksumming of the request and sequence numbering is now done in the > OCC interface driver in order to keep unique sequence numbers. So > remove those in the hwmon driver. > > Signed-off-by: Eddie James <eajames@linux.ibm.com> Acked-by: Guenter Roeck <linux@roeck-us.net> > --- > drivers/hwmon/occ/common.c | 30 ++++++++++++------------------ > drivers/hwmon/occ/common.h | 3 +-- > drivers/hwmon/occ/p8_i2c.c | 15 +++++++++------ > drivers/hwmon/occ/p9_sbe.c | 4 ++-- > 4 files changed, 24 insertions(+), 28 deletions(-) > > diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c > index 0d68a78be980..fc298268c89e 100644 > --- a/drivers/hwmon/occ/common.c > +++ b/drivers/hwmon/occ/common.c > @@ -132,22 +132,20 @@ struct extended_sensor { > static int occ_poll(struct occ *occ) > { > int rc; > - u16 checksum = occ->poll_cmd_data + occ->seq_no + 1; > - u8 cmd[8]; > + u8 cmd[7]; > struct occ_poll_response_header *header; > > /* big endian */ > - cmd[0] = occ->seq_no++; /* sequence number */ > + cmd[0] = 0; /* sequence number */ > cmd[1] = 0; /* cmd type */ > cmd[2] = 0; /* data length msb */ > cmd[3] = 1; /* data length lsb */ > cmd[4] = occ->poll_cmd_data; /* data */ > - cmd[5] = checksum >> 8; /* checksum msb */ > - cmd[6] = checksum & 0xFF; /* checksum lsb */ > - cmd[7] = 0; > + cmd[5] = 0; /* checksum msb */ > + cmd[6] = 0; /* checksum lsb */ > > /* mutex should already be locked if necessary */ > - rc = occ->send_cmd(occ, cmd); > + rc = occ->send_cmd(occ, cmd, sizeof(cmd)); > if (rc) { > occ->last_error = rc; > if (occ->error_count++ > OCC_ERROR_COUNT_THRESHOLD) > @@ -184,25 +182,23 @@ static int occ_set_user_power_cap(struct occ *occ, u16 user_power_cap) > { > int rc; > u8 cmd[8]; > - u16 checksum = 0x24; > __be16 user_power_cap_be = cpu_to_be16(user_power_cap); > > - cmd[0] = 0; > - cmd[1] = 0x22; > - cmd[2] = 0; > - cmd[3] = 2; > + cmd[0] = 0; /* sequence number */ > + cmd[1] = 0x22; /* cmd type */ > + cmd[2] = 0; /* data length msb */ > + cmd[3] = 2; /* data length lsb */ > > memcpy(&cmd[4], &user_power_cap_be, 2); > > - checksum += cmd[4] + cmd[5]; > - cmd[6] = checksum >> 8; > - cmd[7] = checksum & 0xFF; > + cmd[6] = 0; /* checksum msb */ > + cmd[7] = 0; /* checksum lsb */ > > rc = mutex_lock_interruptible(&occ->lock); > if (rc) > return rc; > > - rc = occ->send_cmd(occ, cmd); > + rc = occ->send_cmd(occ, cmd, sizeof(cmd)); > > mutex_unlock(&occ->lock); > > @@ -1151,8 +1147,6 @@ int occ_setup(struct occ *occ, const char *name) > { > int rc; > > - /* start with 1 to avoid false match with zero-initialized SRAM buffer */ > - occ->seq_no = 1; > mutex_init(&occ->lock); > occ->groups[0] = &occ->group; > > diff --git a/drivers/hwmon/occ/common.h b/drivers/hwmon/occ/common.h > index e6df719770e8..5020117be740 100644 > --- a/drivers/hwmon/occ/common.h > +++ b/drivers/hwmon/occ/common.h > @@ -95,9 +95,8 @@ struct occ { > struct occ_sensors sensors; > > int powr_sample_time_us; /* average power sample time */ > - u8 seq_no; > u8 poll_cmd_data; /* to perform OCC poll command */ > - int (*send_cmd)(struct occ *occ, u8 *cmd); > + int (*send_cmd)(struct occ *occ, u8 *cmd, size_t len); > > unsigned long next_update; > struct mutex lock; /* lock OCC access */ > diff --git a/drivers/hwmon/occ/p8_i2c.c b/drivers/hwmon/occ/p8_i2c.c > index 0cf8588be35a..22af189eafa6 100644 > --- a/drivers/hwmon/occ/p8_i2c.c > +++ b/drivers/hwmon/occ/p8_i2c.c > @@ -97,18 +97,21 @@ static int p8_i2c_occ_putscom_u32(struct i2c_client *client, u32 address, > } > > static int p8_i2c_occ_putscom_be(struct i2c_client *client, u32 address, > - u8 *data) > + u8 *data, size_t len) > { > - __be32 data0, data1; > + __be32 data0 = 0, data1 = 0; > > - memcpy(&data0, data, 4); > - memcpy(&data1, data + 4, 4); > + memcpy(&data0, data, min(len, 4UL)); > + if (len > 4UL) { > + len -= 4; > + memcpy(&data1, data + 4, min(len, 4UL)); > + } > > return p8_i2c_occ_putscom_u32(client, address, be32_to_cpu(data0), > be32_to_cpu(data1)); > } > > -static int p8_i2c_occ_send_cmd(struct occ *occ, u8 *cmd) > +static int p8_i2c_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len) > { > int i, rc; > unsigned long start; > @@ -127,7 +130,7 @@ static int p8_i2c_occ_send_cmd(struct occ *occ, u8 *cmd) > return rc; > > /* write command (expected to already be BE), we need bus-endian... */ > - rc = p8_i2c_occ_putscom_be(client, OCB_DATA3, cmd); > + rc = p8_i2c_occ_putscom_be(client, OCB_DATA3, cmd, len); > if (rc) > return rc; > > diff --git a/drivers/hwmon/occ/p9_sbe.c b/drivers/hwmon/occ/p9_sbe.c > index f6387cc0b754..9709f2b9c052 100644 > --- a/drivers/hwmon/occ/p9_sbe.c > +++ b/drivers/hwmon/occ/p9_sbe.c > @@ -16,14 +16,14 @@ struct p9_sbe_occ { > > #define to_p9_sbe_occ(x) container_of((x), struct p9_sbe_occ, occ) > > -static int p9_sbe_occ_send_cmd(struct occ *occ, u8 *cmd) > +static int p9_sbe_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len) > { > struct occ_response *resp = &occ->resp; > struct p9_sbe_occ *ctx = to_p9_sbe_occ(occ); > size_t resp_len = sizeof(*resp); > int rc; > > - rc = fsi_occ_submit(ctx->sbe, cmd, 8, resp, &resp_len); > + rc = fsi_occ_submit(ctx->sbe, cmd, len, resp, &resp_len); > if (rc < 0) > return rc; >
Hi Eddie,
I love your patch! Perhaps something to improve:
[auto build test WARNING on hwmon/hwmon-next]
[also build test WARNING on linux/master linus/master v5.14-rc1 next-20210716]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Eddie-James/OCC-fsi-and-hwmon-Set-sequence-number-in-submit-interface/20210718-103535
base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: arm-randconfig-r033-20210718 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 5d5b08761f944d5b9822d582378333cc4b36a0a7)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm cross compiling tool for clang build
# apt-get install binutils-arm-linux-gnueabi
# https://github.com/0day-ci/linux/commit/5e8ecc23325ef0310d83a4520071aae18418f88a
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Eddie-James/OCC-fsi-and-hwmon-Set-sequence-number-in-submit-interface/20210718-103535
git checkout 5e8ecc23325ef0310d83a4520071aae18418f88a
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/hwmon/occ/p8_i2c.c:104:23: warning: comparison of distinct pointer types ('typeof (len) *' (aka 'unsigned int *') and 'typeof (4UL) *' (aka 'unsigned long *')) [-Wcompare-distinct-pointer-types]
memcpy(&data0, data, min(len, 4UL));
^~~~~~~~~~~~~
include/linux/minmax.h:45:19: note: expanded from macro 'min'
#define min(x, y) __careful_cmp(x, y, <)
^~~~~~~~~~~~~~~~~~~~~~
include/linux/minmax.h:36:24: note: expanded from macro '__careful_cmp'
__builtin_choose_expr(__safe_cmp(x, y), \
^~~~~~~~~~~~~~~~
include/linux/minmax.h:26:4: note: expanded from macro '__safe_cmp'
(__typecheck(x, y) && __no_side_effects(x, y))
^~~~~~~~~~~~~~~~~
include/linux/minmax.h:20:28: note: expanded from macro '__typecheck'
(!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
drivers/hwmon/occ/p8_i2c.c:107:28: warning: comparison of distinct pointer types ('typeof (len) *' (aka 'unsigned int *') and 'typeof (4UL) *' (aka 'unsigned long *')) [-Wcompare-distinct-pointer-types]
memcpy(&data1, data + 4, min(len, 4UL));
^~~~~~~~~~~~~
include/linux/minmax.h:45:19: note: expanded from macro 'min'
#define min(x, y) __careful_cmp(x, y, <)
^~~~~~~~~~~~~~~~~~~~~~
include/linux/minmax.h:36:24: note: expanded from macro '__careful_cmp'
__builtin_choose_expr(__safe_cmp(x, y), \
^~~~~~~~~~~~~~~~
include/linux/minmax.h:26:4: note: expanded from macro '__safe_cmp'
(__typecheck(x, y) && __no_side_effects(x, y))
^~~~~~~~~~~~~~~~~
include/linux/minmax.h:20:28: note: expanded from macro '__typecheck'
(!!(sizeof((typeof(x) *)1 == (typeof(y) *)1)))
~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~
2 warnings generated.
vim +104 drivers/hwmon/occ/p8_i2c.c
98
99 static int p8_i2c_occ_putscom_be(struct i2c_client *client, u32 address,
100 u8 *data, size_t len)
101 {
102 __be32 data0 = 0, data1 = 0;
103
> 104 memcpy(&data0, data, min(len, 4UL));
105 if (len > 4UL) {
106 len -= 4;
107 memcpy(&data1, data + 4, min(len, 4UL));
108 }
109
110 return p8_i2c_occ_putscom_u32(client, address, be32_to_cpu(data0),
111 be32_to_cpu(data1));
112 }
113
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Eddie,
I love your patch! Perhaps something to improve:
[auto build test WARNING on hwmon/hwmon-next]
[also build test WARNING on linux/master linus/master v5.14-rc1 next-20210716]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Eddie-James/OCC-fsi-and-hwmon-Set-sequence-number-in-submit-interface/20210718-103535
base: https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: i386-randconfig-s001-20210718 (attached as .config)
compiler: gcc-10 (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.3-341-g8af24329-dirty
# https://github.com/0day-ci/linux/commit/5e8ecc23325ef0310d83a4520071aae18418f88a
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Eddie-James/OCC-fsi-and-hwmon-Set-sequence-number-in-submit-interface/20210718-103535
git checkout 5e8ecc23325ef0310d83a4520071aae18418f88a
# save the attached .config to linux build tree
make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=i386 SHELL=/bin/bash drivers/hwmon/occ/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
>> drivers/hwmon/occ/p8_i2c.c:104:30: sparse: sparse: incompatible types in comparison expression (different type sizes):
>> drivers/hwmon/occ/p8_i2c.c:104:30: sparse: unsigned int *
>> drivers/hwmon/occ/p8_i2c.c:104:30: sparse: unsigned long *
drivers/hwmon/occ/p8_i2c.c:107:42: sparse: sparse: incompatible types in comparison expression (different type sizes):
drivers/hwmon/occ/p8_i2c.c:107:42: sparse: unsigned int *
drivers/hwmon/occ/p8_i2c.c:107:42: sparse: unsigned long *
vim +104 drivers/hwmon/occ/p8_i2c.c
98
99 static int p8_i2c_occ_putscom_be(struct i2c_client *client, u32 address,
100 u8 *data, size_t len)
101 {
102 __be32 data0 = 0, data1 = 0;
103
> 104 memcpy(&data0, data, min(len, 4UL));
105 if (len > 4UL) {
106 len -= 4;
107 memcpy(&data1, data + 4, min(len, 4UL));
108 }
109
110 return p8_i2c_occ_putscom_u32(client, address, be32_to_cpu(data0),
111 be32_to_cpu(data1));
112 }
113
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Fri, 16 Jul 2021 at 15:19, Eddie James <eajames@linux.ibm.com> wrote: > > Checksumming of the request and sequence numbering is now done in the > OCC interface driver in order to keep unique sequence numbers. So > remove those in the hwmon driver. > > Signed-off-by: Eddie James <eajames@linux.ibm.com> > --- > drivers/hwmon/occ/common.c | 30 ++++++++++++------------------ > drivers/hwmon/occ/common.h | 3 +-- > drivers/hwmon/occ/p8_i2c.c | 15 +++++++++------ > drivers/hwmon/occ/p9_sbe.c | 4 ++-- > 4 files changed, 24 insertions(+), 28 deletions(-) > > diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c > index 0d68a78be980..fc298268c89e 100644 > --- a/drivers/hwmon/occ/common.c > +++ b/drivers/hwmon/occ/common.c > @@ -132,22 +132,20 @@ struct extended_sensor { > static int occ_poll(struct occ *occ) > { > int rc; > - u16 checksum = occ->poll_cmd_data + occ->seq_no + 1; > - u8 cmd[8]; > + u8 cmd[7]; The shortening of the command seems unrelated? If you leave it at 8 then you avoid the special casing below. Is there any downside to sending the extra 0 byte at the end? > struct occ_poll_response_header *header; > > /* big endian */ > - cmd[0] = occ->seq_no++; /* sequence number */ > + cmd[0] = 0; /* sequence number */ > cmd[1] = 0; /* cmd type */ > cmd[2] = 0; /* data length msb */ > cmd[3] = 1; /* data length lsb */ > cmd[4] = occ->poll_cmd_data; /* data */ > - cmd[5] = checksum >> 8; /* checksum msb */ > - cmd[6] = checksum & 0xFF; /* checksum lsb */ > - cmd[7] = 0; > + cmd[5] = 0; /* checksum msb */ > + cmd[6] = 0; /* checksum lsb */ > --- a/drivers/hwmon/occ/p8_i2c.c> +++ b/drivers/hwmon/occ/p8_i2c.c > @@ -97,18 +97,21 @@ static int p8_i2c_occ_putscom_u32(struct i2c_client *client, u32 address, > } > > static int p8_i2c_occ_putscom_be(struct i2c_client *client, u32 address, > - u8 *data) > + u8 *data, size_t len) > { > - __be32 data0, data1; > + __be32 data0 = 0, data1 = 0; > > - memcpy(&data0, data, 4); > - memcpy(&data1, data + 4, 4); > + memcpy(&data0, data, min(len, 4UL)); The UL here seems unnecessary (and dropping it should fix your 0day bot warnings). But I think it would be simpler to go back to a fixed length of 8. > + if (len > 4UL) { > + len -= 4; > + memcpy(&data1, data + 4, min(len, 4UL)); > + } > > return p8_i2c_occ_putscom_u32(client, address, be32_to_cpu(data0), > be32_to_cpu(data1)); > }
On Wed, 2021-07-21 at 02:43 +0000, Joel Stanley wrote: > On Fri, 16 Jul 2021 at 15:19, Eddie James <eajames@linux.ibm.com> > wrote: > > Checksumming of the request and sequence numbering is now done in > > the > > OCC interface driver in order to keep unique sequence numbers. So > > remove those in the hwmon driver. > > > > Signed-off-by: Eddie James <eajames@linux.ibm.com> > > --- > > drivers/hwmon/occ/common.c | 30 ++++++++++++------------------ > > drivers/hwmon/occ/common.h | 3 +-- > > drivers/hwmon/occ/p8_i2c.c | 15 +++++++++------ > > drivers/hwmon/occ/p9_sbe.c | 4 ++-- > > 4 files changed, 24 insertions(+), 28 deletions(-) > > > > diff --git a/drivers/hwmon/occ/common.c > > b/drivers/hwmon/occ/common.c > > index 0d68a78be980..fc298268c89e 100644 > > --- a/drivers/hwmon/occ/common.c > > +++ b/drivers/hwmon/occ/common.c > > @@ -132,22 +132,20 @@ struct extended_sensor { > > static int occ_poll(struct occ *occ) > > { > > int rc; > > - u16 checksum = occ->poll_cmd_data + occ->seq_no + 1; > > - u8 cmd[8]; > > + u8 cmd[7]; > > The shortening of the command seems unrelated? > > If you leave it at 8 then you avoid the special casing below. Is > there > any downside to sending the extra 0 byte at the end? Yes, it would break the checksumming unfortunately. The checksum is calculated and added at the last two bytes, so if you send more than your command actually is, the checksum will be in the wrong spot. > > > struct occ_poll_response_header *header; > > > > /* big endian */ > > - cmd[0] = occ->seq_no++; /* sequence number */ > > + cmd[0] = 0; /* sequence number */ > > cmd[1] = 0; /* cmd type */ > > cmd[2] = 0; /* data length msb */ > > cmd[3] = 1; /* data length lsb */ > > cmd[4] = occ->poll_cmd_data; /* data */ > > - cmd[5] = checksum >> 8; /* checksum msb */ > > - cmd[6] = checksum & 0xFF; /* checksum lsb */ > > - cmd[7] = 0; > > + cmd[5] = 0; /* checksum msb */ > > + cmd[6] = 0; /* checksum lsb */ > > --- a/drivers/hwmon/occ/p8_i2c.c> +++ b/drivers/hwmon/occ/p8_i2c.c > > @@ -97,18 +97,21 @@ static int p8_i2c_occ_putscom_u32(struct > > i2c_client *client, u32 address, > > } > > > > static int p8_i2c_occ_putscom_be(struct i2c_client *client, u32 > > address, > > - u8 *data) > > + u8 *data, size_t len) > > { > > - __be32 data0, data1; > > + __be32 data0 = 0, data1 = 0; > > > > - memcpy(&data0, data, 4); > > - memcpy(&data1, data + 4, 4); > > + memcpy(&data0, data, min(len, 4UL)); > > The UL here seems unnecessary (and dropping it should fix your 0day > bot warnings). Yea, I think I just need min_t Thanks for the review! Eddie > > But I think it would be simpler to go back to a fixed length of 8. > > > + if (len > 4UL) { > > + len -= 4; > > + memcpy(&data1, data + 4, min(len, 4UL)); > > + } > > > > return p8_i2c_occ_putscom_u32(client, address, > > be32_to_cpu(data0), > > be32_to_cpu(data1)); > > }
diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c index 0d68a78be980..fc298268c89e 100644 --- a/drivers/hwmon/occ/common.c +++ b/drivers/hwmon/occ/common.c @@ -132,22 +132,20 @@ struct extended_sensor { static int occ_poll(struct occ *occ) { int rc; - u16 checksum = occ->poll_cmd_data + occ->seq_no + 1; - u8 cmd[8]; + u8 cmd[7]; struct occ_poll_response_header *header; /* big endian */ - cmd[0] = occ->seq_no++; /* sequence number */ + cmd[0] = 0; /* sequence number */ cmd[1] = 0; /* cmd type */ cmd[2] = 0; /* data length msb */ cmd[3] = 1; /* data length lsb */ cmd[4] = occ->poll_cmd_data; /* data */ - cmd[5] = checksum >> 8; /* checksum msb */ - cmd[6] = checksum & 0xFF; /* checksum lsb */ - cmd[7] = 0; + cmd[5] = 0; /* checksum msb */ + cmd[6] = 0; /* checksum lsb */ /* mutex should already be locked if necessary */ - rc = occ->send_cmd(occ, cmd); + rc = occ->send_cmd(occ, cmd, sizeof(cmd)); if (rc) { occ->last_error = rc; if (occ->error_count++ > OCC_ERROR_COUNT_THRESHOLD) @@ -184,25 +182,23 @@ static int occ_set_user_power_cap(struct occ *occ, u16 user_power_cap) { int rc; u8 cmd[8]; - u16 checksum = 0x24; __be16 user_power_cap_be = cpu_to_be16(user_power_cap); - cmd[0] = 0; - cmd[1] = 0x22; - cmd[2] = 0; - cmd[3] = 2; + cmd[0] = 0; /* sequence number */ + cmd[1] = 0x22; /* cmd type */ + cmd[2] = 0; /* data length msb */ + cmd[3] = 2; /* data length lsb */ memcpy(&cmd[4], &user_power_cap_be, 2); - checksum += cmd[4] + cmd[5]; - cmd[6] = checksum >> 8; - cmd[7] = checksum & 0xFF; + cmd[6] = 0; /* checksum msb */ + cmd[7] = 0; /* checksum lsb */ rc = mutex_lock_interruptible(&occ->lock); if (rc) return rc; - rc = occ->send_cmd(occ, cmd); + rc = occ->send_cmd(occ, cmd, sizeof(cmd)); mutex_unlock(&occ->lock); @@ -1151,8 +1147,6 @@ int occ_setup(struct occ *occ, const char *name) { int rc; - /* start with 1 to avoid false match with zero-initialized SRAM buffer */ - occ->seq_no = 1; mutex_init(&occ->lock); occ->groups[0] = &occ->group; diff --git a/drivers/hwmon/occ/common.h b/drivers/hwmon/occ/common.h index e6df719770e8..5020117be740 100644 --- a/drivers/hwmon/occ/common.h +++ b/drivers/hwmon/occ/common.h @@ -95,9 +95,8 @@ struct occ { struct occ_sensors sensors; int powr_sample_time_us; /* average power sample time */ - u8 seq_no; u8 poll_cmd_data; /* to perform OCC poll command */ - int (*send_cmd)(struct occ *occ, u8 *cmd); + int (*send_cmd)(struct occ *occ, u8 *cmd, size_t len); unsigned long next_update; struct mutex lock; /* lock OCC access */ diff --git a/drivers/hwmon/occ/p8_i2c.c b/drivers/hwmon/occ/p8_i2c.c index 0cf8588be35a..22af189eafa6 100644 --- a/drivers/hwmon/occ/p8_i2c.c +++ b/drivers/hwmon/occ/p8_i2c.c @@ -97,18 +97,21 @@ static int p8_i2c_occ_putscom_u32(struct i2c_client *client, u32 address, } static int p8_i2c_occ_putscom_be(struct i2c_client *client, u32 address, - u8 *data) + u8 *data, size_t len) { - __be32 data0, data1; + __be32 data0 = 0, data1 = 0; - memcpy(&data0, data, 4); - memcpy(&data1, data + 4, 4); + memcpy(&data0, data, min(len, 4UL)); + if (len > 4UL) { + len -= 4; + memcpy(&data1, data + 4, min(len, 4UL)); + } return p8_i2c_occ_putscom_u32(client, address, be32_to_cpu(data0), be32_to_cpu(data1)); } -static int p8_i2c_occ_send_cmd(struct occ *occ, u8 *cmd) +static int p8_i2c_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len) { int i, rc; unsigned long start; @@ -127,7 +130,7 @@ static int p8_i2c_occ_send_cmd(struct occ *occ, u8 *cmd) return rc; /* write command (expected to already be BE), we need bus-endian... */ - rc = p8_i2c_occ_putscom_be(client, OCB_DATA3, cmd); + rc = p8_i2c_occ_putscom_be(client, OCB_DATA3, cmd, len); if (rc) return rc; diff --git a/drivers/hwmon/occ/p9_sbe.c b/drivers/hwmon/occ/p9_sbe.c index f6387cc0b754..9709f2b9c052 100644 --- a/drivers/hwmon/occ/p9_sbe.c +++ b/drivers/hwmon/occ/p9_sbe.c @@ -16,14 +16,14 @@ struct p9_sbe_occ { #define to_p9_sbe_occ(x) container_of((x), struct p9_sbe_occ, occ) -static int p9_sbe_occ_send_cmd(struct occ *occ, u8 *cmd) +static int p9_sbe_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len) { struct occ_response *resp = &occ->resp; struct p9_sbe_occ *ctx = to_p9_sbe_occ(occ); size_t resp_len = sizeof(*resp); int rc; - rc = fsi_occ_submit(ctx->sbe, cmd, 8, resp, &resp_len); + rc = fsi_occ_submit(ctx->sbe, cmd, len, resp, &resp_len); if (rc < 0) return rc;
Checksumming of the request and sequence numbering is now done in the OCC interface driver in order to keep unique sequence numbers. So remove those in the hwmon driver. Signed-off-by: Eddie James <eajames@linux.ibm.com> --- drivers/hwmon/occ/common.c | 30 ++++++++++++------------------ drivers/hwmon/occ/common.h | 3 +-- drivers/hwmon/occ/p8_i2c.c | 15 +++++++++------ drivers/hwmon/occ/p9_sbe.c | 4 ++-- 4 files changed, 24 insertions(+), 28 deletions(-)