* [PATCH char-misc-next 1/4] misc: mic: remove unneeded debug message
@ 2015-11-23 11:54 Sudip Mukherjee
2015-11-23 11:54 ` [PATCH char-misc-next 2/4] misc: mic: return error properly Sudip Mukherjee
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Sudip Mukherjee @ 2015-11-23 11:54 UTC (permalink / raw)
To: Sudeep Dutt, Greg Kroah-Hartman; +Cc: linux-kernel, Sudip Mukherjee
>From the error path we are printing an error message with dev_err(). No
need to print almost same message with dev_dbg().
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
drivers/misc/mic/host/mic_x100.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/drivers/misc/mic/host/mic_x100.c b/drivers/misc/mic/host/mic_x100.c
index 8118ac4..cd5208d 100644
--- a/drivers/misc/mic/host/mic_x100.c
+++ b/drivers/misc/mic/host/mic_x100.c
@@ -450,21 +450,21 @@ mic_x100_load_firmware(struct mic_device *mdev, const char *buf)
rc = mic_x100_get_boot_addr(mdev);
if (rc)
- goto error;
+ goto done;
/* load OS */
rc = request_firmware(&fw, mdev->cosm_dev->firmware, &mdev->pdev->dev);
if (rc < 0) {
dev_err(&mdev->pdev->dev,
"ramdisk request_firmware failed: %d %s\n",
rc, mdev->cosm_dev->firmware);
- goto error;
+ goto done;
}
if (mdev->bootaddr > mdev->aper.len - fw->size) {
rc = -EINVAL;
dev_err(&mdev->pdev->dev, "%s %d rc %d bootaddr 0x%x\n",
__func__, __LINE__, rc, mdev->bootaddr);
release_firmware(fw);
- goto error;
+ goto done;
}
memcpy_toio(mdev->aper.va + mdev->bootaddr, fw->data, fw->size);
mdev->ops->write_spad(mdev, MIC_X100_FW_SIZE, fw->size);
@@ -475,14 +475,13 @@ mic_x100_load_firmware(struct mic_device *mdev, const char *buf)
if (rc) {
dev_err(&mdev->pdev->dev, "%s %d rc %d\n",
__func__, __LINE__, rc);
- goto error;
+ goto done;
}
release_firmware(fw);
/* load ramdisk */
if (mdev->cosm_dev->ramdisk)
rc = mic_x100_load_ramdisk(mdev);
-error:
- dev_dbg(&mdev->pdev->dev, "%s %d rc %d\n", __func__, __LINE__, rc);
+
done:
return rc;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH char-misc-next 2/4] misc: mic: return error properly
2015-11-23 11:54 [PATCH char-misc-next 1/4] misc: mic: remove unneeded debug message Sudip Mukherjee
@ 2015-11-23 11:54 ` Sudip Mukherjee
2015-11-23 11:54 ` [PATCH char-misc-next 3/4] misc: mic: return error directly Sudip Mukherjee
2015-11-23 11:54 ` [PATCH char-misc-next 4/4] misc: mic: use common error path Sudip Mukherjee
2 siblings, 0 replies; 6+ messages in thread
From: Sudip Mukherjee @ 2015-11-23 11:54 UTC (permalink / raw)
To: Sudeep Dutt, Greg Kroah-Hartman; +Cc: linux-kernel, Sudip Mukherjee
If request_firmware() succeeds then rc becomes 0. After that if the test
for strcmp() fails then we were jumping to label done: and returning rc.
But rc being 0 we returned success whereas we have failed here and we
were supposed to return an error.
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
I am not sure if this was intentional. Another case might be that if the
string bootmode is "flash" then mic_x100_load_command_line() was
intentionally skipped and success was returned.
drivers/misc/mic/host/mic_x100.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/misc/mic/host/mic_x100.c b/drivers/misc/mic/host/mic_x100.c
index cd5208d..317e25f 100644
--- a/drivers/misc/mic/host/mic_x100.c
+++ b/drivers/misc/mic/host/mic_x100.c
@@ -468,8 +468,13 @@ mic_x100_load_firmware(struct mic_device *mdev, const char *buf)
}
memcpy_toio(mdev->aper.va + mdev->bootaddr, fw->data, fw->size);
mdev->ops->write_spad(mdev, MIC_X100_FW_SIZE, fw->size);
- if (!strcmp(mdev->cosm_dev->bootmode, "flash"))
+ if (!strcmp(mdev->cosm_dev->bootmode, "flash")) {
+ rc = -EINVAL;
+ dev_err(&mdev->pdev->dev, "%s %d rc %d\n",
+ __func__, __LINE__, rc);
+ release_firmware(fw);
goto done;
+ }
/* load command line */
rc = mic_x100_load_command_line(mdev, fw);
if (rc) {
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH char-misc-next 3/4] misc: mic: return error directly
2015-11-23 11:54 [PATCH char-misc-next 1/4] misc: mic: remove unneeded debug message Sudip Mukherjee
2015-11-23 11:54 ` [PATCH char-misc-next 2/4] misc: mic: return error properly Sudip Mukherjee
@ 2015-11-23 11:54 ` Sudip Mukherjee
2015-11-23 11:54 ` [PATCH char-misc-next 4/4] misc: mic: use common error path Sudip Mukherjee
2 siblings, 0 replies; 6+ messages in thread
From: Sudip Mukherjee @ 2015-11-23 11:54 UTC (permalink / raw)
To: Sudeep Dutt, Greg Kroah-Hartman; +Cc: linux-kernel, Sudip Mukherjee
Instead of jumping to a label and then returning from there lets return
directly.
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
drivers/misc/mic/host/mic_x100.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/misc/mic/host/mic_x100.c b/drivers/misc/mic/host/mic_x100.c
index 317e25f..37fa898 100644
--- a/drivers/misc/mic/host/mic_x100.c
+++ b/drivers/misc/mic/host/mic_x100.c
@@ -450,14 +450,14 @@ mic_x100_load_firmware(struct mic_device *mdev, const char *buf)
rc = mic_x100_get_boot_addr(mdev);
if (rc)
- goto done;
+ return rc;
/* load OS */
rc = request_firmware(&fw, mdev->cosm_dev->firmware, &mdev->pdev->dev);
if (rc < 0) {
dev_err(&mdev->pdev->dev,
"ramdisk request_firmware failed: %d %s\n",
rc, mdev->cosm_dev->firmware);
- goto done;
+ return rc;
}
if (mdev->bootaddr > mdev->aper.len - fw->size) {
rc = -EINVAL;
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH char-misc-next 4/4] misc: mic: use common error path
2015-11-23 11:54 [PATCH char-misc-next 1/4] misc: mic: remove unneeded debug message Sudip Mukherjee
2015-11-23 11:54 ` [PATCH char-misc-next 2/4] misc: mic: return error properly Sudip Mukherjee
2015-11-23 11:54 ` [PATCH char-misc-next 3/4] misc: mic: return error directly Sudip Mukherjee
@ 2015-11-23 11:54 ` Sudip Mukherjee
2015-12-12 2:46 ` Sudeep Dutt
2 siblings, 1 reply; 6+ messages in thread
From: Sudip Mukherjee @ 2015-11-23 11:54 UTC (permalink / raw)
To: Sudeep Dutt, Greg Kroah-Hartman; +Cc: linux-kernel, Sudip Mukherjee
Instead of calling release_firmware() on every error and then jumping
lets have a common release_firmware() in the error path.
This patch also fixes a memory leak where we missed release_firmware()
if mic_x100_load_command_line() fails.
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
drivers/misc/mic/host/mic_x100.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/misc/mic/host/mic_x100.c b/drivers/misc/mic/host/mic_x100.c
index 37fa898..82a973c 100644
--- a/drivers/misc/mic/host/mic_x100.c
+++ b/drivers/misc/mic/host/mic_x100.c
@@ -463,8 +463,7 @@ mic_x100_load_firmware(struct mic_device *mdev, const char *buf)
rc = -EINVAL;
dev_err(&mdev->pdev->dev, "%s %d rc %d bootaddr 0x%x\n",
__func__, __LINE__, rc, mdev->bootaddr);
- release_firmware(fw);
- goto done;
+ goto error;
}
memcpy_toio(mdev->aper.va + mdev->bootaddr, fw->data, fw->size);
mdev->ops->write_spad(mdev, MIC_X100_FW_SIZE, fw->size);
@@ -472,22 +471,24 @@ mic_x100_load_firmware(struct mic_device *mdev, const char *buf)
rc = -EINVAL;
dev_err(&mdev->pdev->dev, "%s %d rc %d\n",
__func__, __LINE__, rc);
- release_firmware(fw);
- goto done;
+ goto error;
}
/* load command line */
rc = mic_x100_load_command_line(mdev, fw);
if (rc) {
dev_err(&mdev->pdev->dev, "%s %d rc %d\n",
__func__, __LINE__, rc);
- goto done;
+ goto error;
}
release_firmware(fw);
/* load ramdisk */
if (mdev->cosm_dev->ramdisk)
rc = mic_x100_load_ramdisk(mdev);
-done:
+ return rc;
+
+error:
+ release_firmware(fw);
return rc;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH char-misc-next 4/4] misc: mic: use common error path
2015-11-23 11:54 ` [PATCH char-misc-next 4/4] misc: mic: use common error path Sudip Mukherjee
@ 2015-12-12 2:46 ` Sudeep Dutt
2016-01-06 12:06 ` Sudip Mukherjee
0 siblings, 1 reply; 6+ messages in thread
From: Sudeep Dutt @ 2015-12-12 2:46 UTC (permalink / raw)
To: Sudip Mukherjee
Cc: Sudeep Dutt, Greg Kroah-Hartman, linux-kernel, Dixit, Ashutosh
On Mon, 2015-11-23 at 17:24 +0530, Sudip Mukherjee wrote:
> Instead of calling release_firmware() on every error and then jumping
> lets have a common release_firmware() in the error path.
> This patch also fixes a memory leak where we missed release_firmware()
> if mic_x100_load_command_line() fails.
>
Thanks for this patch series Sudip. All 4 patches look good.
Reviewed-by: Sudeep Dutt <sudeep.dutt@intel.com>
> Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
> ---
> drivers/misc/mic/host/mic_x100.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/misc/mic/host/mic_x100.c b/drivers/misc/mic/host/mic_x100.c
> index 37fa898..82a973c 100644
> --- a/drivers/misc/mic/host/mic_x100.c
> +++ b/drivers/misc/mic/host/mic_x100.c
> @@ -463,8 +463,7 @@ mic_x100_load_firmware(struct mic_device *mdev, const char *buf)
> rc = -EINVAL;
> dev_err(&mdev->pdev->dev, "%s %d rc %d bootaddr 0x%x\n",
> __func__, __LINE__, rc, mdev->bootaddr);
> - release_firmware(fw);
> - goto done;
> + goto error;
> }
> memcpy_toio(mdev->aper.va + mdev->bootaddr, fw->data, fw->size);
> mdev->ops->write_spad(mdev, MIC_X100_FW_SIZE, fw->size);
> @@ -472,22 +471,24 @@ mic_x100_load_firmware(struct mic_device *mdev, const char *buf)
> rc = -EINVAL;
> dev_err(&mdev->pdev->dev, "%s %d rc %d\n",
> __func__, __LINE__, rc);
> - release_firmware(fw);
> - goto done;
> + goto error;
> }
> /* load command line */
> rc = mic_x100_load_command_line(mdev, fw);
> if (rc) {
> dev_err(&mdev->pdev->dev, "%s %d rc %d\n",
> __func__, __LINE__, rc);
> - goto done;
> + goto error;
> }
> release_firmware(fw);
> /* load ramdisk */
> if (mdev->cosm_dev->ramdisk)
> rc = mic_x100_load_ramdisk(mdev);
>
> -done:
> + return rc;
> +
> +error:
> + release_firmware(fw);
> return rc;
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH char-misc-next 4/4] misc: mic: use common error path
2015-12-12 2:46 ` Sudeep Dutt
@ 2016-01-06 12:06 ` Sudip Mukherjee
0 siblings, 0 replies; 6+ messages in thread
From: Sudip Mukherjee @ 2016-01-06 12:06 UTC (permalink / raw)
To: Sudeep Dutt; +Cc: Greg Kroah-Hartman, linux-kernel, Dixit, Ashutosh
On Fri, Dec 11, 2015 at 06:46:25PM -0800, Sudeep Dutt wrote:
> On Mon, 2015-11-23 at 17:24 +0530, Sudip Mukherjee wrote:
> > Instead of calling release_firmware() on every error and then jumping
> > lets have a common release_firmware() in the error path.
> > This patch also fixes a memory leak where we missed release_firmware()
> > if mic_x100_load_command_line() fails.
> >
>
> Thanks for this patch series Sudip. All 4 patches look good.
>
> Reviewed-by: Sudeep Dutt <sudeep.dutt@intel.com>
Hi Greg,
If your tree is not yet closed then a gentle ping for this series.
regards
sudip
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-01-06 12:06 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-11-23 11:54 [PATCH char-misc-next 1/4] misc: mic: remove unneeded debug message Sudip Mukherjee
2015-11-23 11:54 ` [PATCH char-misc-next 2/4] misc: mic: return error properly Sudip Mukherjee
2015-11-23 11:54 ` [PATCH char-misc-next 3/4] misc: mic: return error directly Sudip Mukherjee
2015-11-23 11:54 ` [PATCH char-misc-next 4/4] misc: mic: use common error path Sudip Mukherjee
2015-12-12 2:46 ` Sudeep Dutt
2016-01-06 12:06 ` Sudip Mukherjee
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox