| Message ID | 20211207033811.518981-2-joel@jms.id.au |
|---|---|
| State | New |
| Headers | show |
| Series | fsi: scom: Error handling fixes | expand |
On 12/6/21 21:38, Joel Stanley wrote: > SCOM error handling is made complex by trying to pass around two bits of > information: the function return code, and a status parameter that > represents the CFAM error status register. > > The commit f72ddbe1d7b7 ("fsi: scom: Remove retries") removed the > "hidden" retries in the SCOM driver, in preference of allowing the > calling code (userspace or driver) to decide how to handle a failed > SCOM. However it introduced a bug by attempting to be smart about the > return codes that were "errors" and which were ok to fall through to the > status register parsing. > > We get the following errors: > > - EINVAL or ENXIO, for indirect scoms where the value is invalid > - EINVAL, where the size or address is incorrect > - EIO or ETIMEOUT, where FSI write failed (aspeed master) > - EAGAIN, where the master detected a crc error (GPIO master only) > - EBUSY, where the bus is disabled (GPIO master in external mode) > > In all of these cases we should fail the SCOM read/write and return the > error. > > Thanks to Dan Carpenter for the detailed bug report. Reviewed-by: Eddie James <eajames@linux.ibm.com> > > Fixes: f72ddbe1d7b7 ("fsi: scom: Remove retries") > Link: https://lists.ozlabs.org/pipermail/linux-fsi/2021-November/000235.html > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Signed-off-by: Joel Stanley <joel@jms.id.au> > --- > drivers/fsi/fsi-scom.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/fsi/fsi-scom.c b/drivers/fsi/fsi-scom.c > index da1486bb6a14..3b427f7e9027 100644 > --- a/drivers/fsi/fsi-scom.c > +++ b/drivers/fsi/fsi-scom.c > @@ -289,7 +289,7 @@ static int put_scom(struct scom_device *scom, uint64_t value, > int rc; > > rc = raw_put_scom(scom, value, addr, &status); > - if (rc == -ENODEV) > + if (rc) > return rc; > > rc = handle_fsi2pib_status(scom, status); > @@ -308,7 +308,7 @@ static int get_scom(struct scom_device *scom, uint64_t *value, > int rc; > > rc = raw_get_scom(scom, value, addr, &status); > - if (rc == -ENODEV) > + if (rc) > return rc; > > rc = handle_fsi2pib_status(scom, status);
diff --git a/drivers/fsi/fsi-scom.c b/drivers/fsi/fsi-scom.c index da1486bb6a14..3b427f7e9027 100644 --- a/drivers/fsi/fsi-scom.c +++ b/drivers/fsi/fsi-scom.c @@ -289,7 +289,7 @@ static int put_scom(struct scom_device *scom, uint64_t value, int rc; rc = raw_put_scom(scom, value, addr, &status); - if (rc == -ENODEV) + if (rc) return rc; rc = handle_fsi2pib_status(scom, status); @@ -308,7 +308,7 @@ static int get_scom(struct scom_device *scom, uint64_t *value, int rc; rc = raw_get_scom(scom, value, addr, &status); - if (rc == -ENODEV) + if (rc) return rc; rc = handle_fsi2pib_status(scom, status);
SCOM error handling is made complex by trying to pass around two bits of information: the function return code, and a status parameter that represents the CFAM error status register. The commit f72ddbe1d7b7 ("fsi: scom: Remove retries") removed the "hidden" retries in the SCOM driver, in preference of allowing the calling code (userspace or driver) to decide how to handle a failed SCOM. However it introduced a bug by attempting to be smart about the return codes that were "errors" and which were ok to fall through to the status register parsing. We get the following errors: - EINVAL or ENXIO, for indirect scoms where the value is invalid - EINVAL, where the size or address is incorrect - EIO or ETIMEOUT, where FSI write failed (aspeed master) - EAGAIN, where the master detected a crc error (GPIO master only) - EBUSY, where the bus is disabled (GPIO master in external mode) In all of these cases we should fail the SCOM read/write and return the error. Thanks to Dan Carpenter for the detailed bug report. Fixes: f72ddbe1d7b7 ("fsi: scom: Remove retries") Link: https://lists.ozlabs.org/pipermail/linux-fsi/2021-November/000235.html Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Joel Stanley <joel@jms.id.au> --- drivers/fsi/fsi-scom.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)