| Message ID | 20170712042304.19745-4-cyrilbur@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Wed, 2017-07-12 at 14:22 +1000, Cyril Bur wrote: > While this driver expects to interact asynchronously, OPAL is well > within its rights to return OPAL_SUCCESS to indicate that the operation > completed without the need for a callback. We shouldn't treat > OPAL_SUCCESS as an error rather we should wrap up and return promptly to > the caller. > > Signed-off-by: Cyril Bur <cyrilbur@gmail.com> > --- > I'll note here that currently no OPAL exists that will return > OPAL_SUCCESS so there isn't the possibility of a bug today. It would help if you mentioned OPAL_SUCCESS to the async call. So effectively what we expected to be an asynchronous call with callback, but OPAL returned immediately with success. Balbir Singh.
On Mon, 2017-07-17 at 18:50 +1000, Balbir Singh wrote: > On Wed, 2017-07-12 at 14:22 +1000, Cyril Bur wrote: > > While this driver expects to interact asynchronously, OPAL is well > > within its rights to return OPAL_SUCCESS to indicate that the operation > > completed without the need for a callback. We shouldn't treat > > OPAL_SUCCESS as an error rather we should wrap up and return promptly to > > the caller. > > > > Signed-off-by: Cyril Bur <cyrilbur@gmail.com> > > --- > > I'll note here that currently no OPAL exists that will return > > OPAL_SUCCESS so there isn't the possibility of a bug today. > > It would help if you mentioned OPAL_SUCCESS to the async call. So effectively > what we expected to be an asynchronous call with callback, but OPAL returned > immediately with success. > Ah my favourite problems, commit message. Thanks, Cyril > Balbir Singh. >
diff --git a/drivers/mtd/devices/powernv_flash.c b/drivers/mtd/devices/powernv_flash.c index 7b41af06f4fe..d50b5f200f73 100644 --- a/drivers/mtd/devices/powernv_flash.c +++ b/drivers/mtd/devices/powernv_flash.c @@ -66,9 +66,8 @@ static int powernv_flash_async_op(struct mtd_info *mtd, enum flash_op op, if (token < 0) { if (token != -ERESTARTSYS) dev_err(dev, "Failed to get an async token\n"); - - rc = token; - goto out; + mutex_unlock(&info->lock); + return token; } switch (op) { @@ -87,23 +86,25 @@ static int powernv_flash_async_op(struct mtd_info *mtd, enum flash_op op, goto out; } + if (rc == OPAL_SUCCESS) + goto out_success; + if (rc != OPAL_ASYNC_COMPLETION) { dev_err(dev, "opal_flash_async_op(op=%d) failed (rc %d)\n", op, rc); - opal_async_release_token(token); rc = -EIO; goto out; } rc = opal_async_wait_response(token, &msg); - opal_async_release_token(token); - mutex_unlock(&info->lock); if (rc) { dev_err(dev, "opal async wait failed (rc %d)\n", rc); - return -EIO; + rc = -EIO; + goto out; } rc = opal_get_async_rc(msg); +out_success: if (rc == OPAL_SUCCESS) { rc = 0; if (retlen) @@ -112,8 +113,8 @@ static int powernv_flash_async_op(struct mtd_info *mtd, enum flash_op op, rc = -EIO; } - return rc; out: + opal_async_release_token(token); mutex_unlock(&info->lock); return rc; }
While this driver expects to interact asynchronously, OPAL is well within its rights to return OPAL_SUCCESS to indicate that the operation completed without the need for a callback. We shouldn't treat OPAL_SUCCESS as an error rather we should wrap up and return promptly to the caller. Signed-off-by: Cyril Bur <cyrilbur@gmail.com> --- I'll note here that currently no OPAL exists that will return OPAL_SUCCESS so there isn't the possibility of a bug today. drivers/mtd/devices/powernv_flash.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)