Messages in this thread | | | Date | Tue, 26 Apr 2022 10:46:06 -0500 | Subject | Re: [PATCH 2/2] hwmon (occ): Retry for checksum failure | From | Eddie James <> |
| |
On 4/25/22 04:10, David Laight wrote: > From: Guenter Roeck >> Sent: 24 April 2022 18:18 >> >> On Mon, Mar 21, 2022 at 10:31:12AM -0500, Eddie James wrote: >>> Due to the OCC communication design with a shared SRAM area, >>> checkum errors are expected due to corrupted buffer from OCC >>> communications with other system components. Therefore, retry >>> the command twice in the event of a checksum failure. >>> >>> Signed-off-by: Eddie James <eajames@linux.ibm.com> >> I assume this will be applied together with patch 1 of the series. >> >> Acked-by: Guenter Roeck <linux@roeck-us.net> >> >> Guenter >> >>> --- >>> drivers/hwmon/occ/p9_sbe.c | 28 ++++++++++++++++++---------- >>> 1 file changed, 18 insertions(+), 10 deletions(-) >>> >>> diff --git a/drivers/hwmon/occ/p9_sbe.c b/drivers/hwmon/occ/p9_sbe.c >>> index 49b13cc01073..7f4c3f979c54 100644 >>> --- a/drivers/hwmon/occ/p9_sbe.c >>> +++ b/drivers/hwmon/occ/p9_sbe.c >>> @@ -84,17 +84,25 @@ static int p9_sbe_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len) >>> 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, len, resp, &resp_len); >>> - if (rc < 0) { >>> - if (resp_len) { >>> - if (p9_sbe_occ_save_ffdc(ctx, resp, resp_len)) >>> - sysfs_notify(&occ->bus_dev->kobj, NULL, >>> - bin_attr_ffdc.attr.name); >>> + int tries = 0; >>> + >>> + do { > Why not use a for() loop? > >>> + rc = fsi_occ_submit(ctx->sbe, cmd, len, resp, &resp_len); >>> + if (rc < 0) { >>> + if (resp_len) { >>> + if (p9_sbe_occ_save_ffdc(ctx, resp, resp_len)) >>> + sysfs_notify(&occ->bus_dev->kobj, NULL, >>> + bin_attr_ffdc.attr.name); >>> + >>> + return rc; >>> + } else if (rc != -EBADE) { >>> + return rc; >>> + } > No need for else after return. > >>> + /* retry twice for checksum failures */ >>> + } else { >>> + break; > I'd break on the success path after testing (rc >= 0). > Saves a level of indent. > >>> } >>> - >>> - return rc; >>> - } >>> + } while (++tries < 3); >>> >>> switch (resp->return_status) { >>> case OCC_RESP_CMD_IN_PRG: > Probably end up with something like: > for (tries = 0; tries < 3; tries++) { > rc = ...; > if (rc >= 0) > break; > if (resp_len) { > ... > return rc; > } > if (rc != -EBADE) > return rc; > } > > David
Thanks David, Ack.
> > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK > Registration No: 1397386 (Wales) >
| |