* [PATCH] misc: aspeed-lpc-ctrl: Correct return values
@ 2019-01-23 23:06 Vijay Khemka
2019-01-24 8:15 ` Greg Kroah-Hartman
0 siblings, 1 reply; 9+ messages in thread
From: Vijay Khemka @ 2019-01-23 23:06 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Joel Stanley, Andrew Jeffery,
linux-arm-kernel, linux-aspeed, linux-kernel
Cc: openbmc @ lists . ozlabs . org, vijaykhemka
Corrected some of return values with appropriate meanings.
Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
---
drivers/misc/aspeed-lpc-ctrl.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/misc/aspeed-lpc-ctrl.c b/drivers/misc/aspeed-lpc-ctrl.c
index 332210e06e98..97ae341109d5 100644
--- a/drivers/misc/aspeed-lpc-ctrl.c
+++ b/drivers/misc/aspeed-lpc-ctrl.c
@@ -68,7 +68,6 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
unsigned long param)
{
struct aspeed_lpc_ctrl *lpc_ctrl = file_aspeed_lpc_ctrl(file);
- struct device *dev = file->private_data;
void __user *p = (void __user *)param;
struct aspeed_lpc_ctrl_mapping map;
u32 addr;
@@ -93,8 +92,8 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
/* If memory-region is not described in device tree */
if (!lpc_ctrl->mem_size) {
- dev_err(dev, "Didn't find reserved memory\n");
- return -EINVAL;
+ pr_err("aspeed_lpc_ctrl: ioctl: Didn't find reserved memory\n");
+ return -ENXIO;
}
map.size = lpc_ctrl->mem_size;
@@ -134,16 +133,16 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
if (map.window_type == ASPEED_LPC_CTRL_WINDOW_FLASH) {
if (!lpc_ctrl->pnor_size) {
- dev_err(dev, "Didn't find host pnor flash\n");
- return -EINVAL;
+ pr_err("aspeed_lpc_ctrl: ioctl: Didn't find host pnor flash\n");
+ return -ENXIO;
}
addr = lpc_ctrl->pnor_base;
size = lpc_ctrl->pnor_size;
} else if (map.window_type == ASPEED_LPC_CTRL_WINDOW_MEMORY) {
/* If memory-region is not described in device tree */
if (!lpc_ctrl->mem_size) {
- dev_err(dev, "Didn't find reserved memory\n");
- return -EINVAL;
+ pr_err("aspeed_lpc_ctrl: ioctl: Didn't find reserved memory\n");
+ return -ENXIO;
}
addr = lpc_ctrl->mem_base;
size = lpc_ctrl->mem_size;
@@ -239,7 +238,7 @@ static int aspeed_lpc_ctrl_probe(struct platform_device *pdev)
of_node_put(node);
if (rc) {
dev_err(dev, "Couldn't address to resource for reserved memory\n");
- return -ENOMEM;
+ return -ENXIO;
}
lpc_ctrl->mem_size = resource_size(&resm);
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] misc: aspeed-lpc-ctrl: Correct return values
2019-01-23 23:06 [PATCH] misc: aspeed-lpc-ctrl: Correct return values Vijay Khemka
@ 2019-01-24 8:15 ` Greg Kroah-Hartman
2019-01-24 19:29 ` Vijay Khemka
0 siblings, 1 reply; 9+ messages in thread
From: Greg Kroah-Hartman @ 2019-01-24 8:15 UTC (permalink / raw)
To: Vijay Khemka
Cc: Arnd Bergmann, linux-aspeed, Andrew Jeffery,
openbmc @ lists . ozlabs . org, linux-kernel, Joel Stanley,
linux-arm-kernel
On Wed, Jan 23, 2019 at 03:06:34PM -0800, Vijay Khemka wrote:
> Corrected some of return values with appropriate meanings.
>
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> ---
> drivers/misc/aspeed-lpc-ctrl.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/misc/aspeed-lpc-ctrl.c b/drivers/misc/aspeed-lpc-ctrl.c
> index 332210e06e98..97ae341109d5 100644
> --- a/drivers/misc/aspeed-lpc-ctrl.c
> +++ b/drivers/misc/aspeed-lpc-ctrl.c
> @@ -68,7 +68,6 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
> unsigned long param)
> {
> struct aspeed_lpc_ctrl *lpc_ctrl = file_aspeed_lpc_ctrl(file);
> - struct device *dev = file->private_data;
> void __user *p = (void __user *)param;
> struct aspeed_lpc_ctrl_mapping map;
> u32 addr;
> @@ -93,8 +92,8 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
>
> /* If memory-region is not described in device tree */
> if (!lpc_ctrl->mem_size) {
> - dev_err(dev, "Didn't find reserved memory\n");
> - return -EINVAL;
> + pr_err("aspeed_lpc_ctrl: ioctl: Didn't find reserved memory\n");
Why did you change from dev_err() to pr_err()? You just lost a lot of
information that the user previously was getting from dev_err() :(
> + return -ENXIO;
> }
>
> map.size = lpc_ctrl->mem_size;
> @@ -134,16 +133,16 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
>
> if (map.window_type == ASPEED_LPC_CTRL_WINDOW_FLASH) {
> if (!lpc_ctrl->pnor_size) {
> - dev_err(dev, "Didn't find host pnor flash\n");
> - return -EINVAL;
> + pr_err("aspeed_lpc_ctrl: ioctl: Didn't find host pnor flash\n");
Again, don't do that :(
thanks,
greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] misc: aspeed-lpc-ctrl: Correct return values
2019-01-24 8:15 ` Greg Kroah-Hartman
@ 2019-01-24 19:29 ` Vijay Khemka
2019-02-11 5:22 ` Andrew Jeffery
0 siblings, 1 reply; 9+ messages in thread
From: Vijay Khemka @ 2019-01-24 19:29 UTC (permalink / raw)
To: Greg Kroah-Hartman, Andrew Jeffery
Cc: Arnd Bergmann, linux-aspeed, Andrew Jeffery,
openbmc @ lists . ozlabs . org, linux-kernel, Joel Stanley,
linux-arm-kernel
On 1/24/19, 12:16 AM, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org> wrote:
On Wed, Jan 23, 2019 at 03:06:34PM -0800, Vijay Khemka wrote:
> Corrected some of return values with appropriate meanings.
>
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> ---
> drivers/misc/aspeed-lpc-ctrl.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/misc/aspeed-lpc-ctrl.c b/drivers/misc/aspeed-lpc-ctrl.c
> index 332210e06e98..97ae341109d5 100644
> --- a/drivers/misc/aspeed-lpc-ctrl.c
> +++ b/drivers/misc/aspeed-lpc-ctrl.c
> @@ -68,7 +68,6 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
> unsigned long param)
> {
> struct aspeed_lpc_ctrl *lpc_ctrl = file_aspeed_lpc_ctrl(file);
> - struct device *dev = file->private_data;
> void __user *p = (void __user *)param;
> struct aspeed_lpc_ctrl_mapping map;
> u32 addr;
> @@ -93,8 +92,8 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
>
> /* If memory-region is not described in device tree */
> if (!lpc_ctrl->mem_size) {
> - dev_err(dev, "Didn't find reserved memory\n");
> - return -EINVAL;
> + pr_err("aspeed_lpc_ctrl: ioctl: Didn't find reserved memory\n");
Why did you change from dev_err() to pr_err()? You just lost a lot of
information that the user previously was getting from dev_err() :(
I did this as per review comment from Andrew Jeffery, as we don't want to put this error for driver. It has to be handled by userspace. But I am still reporting some error here.
> + return -ENXIO;
> }
>
> map.size = lpc_ctrl->mem_size;
> @@ -134,16 +133,16 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
>
> if (map.window_type == ASPEED_LPC_CTRL_WINDOW_FLASH) {
> if (!lpc_ctrl->pnor_size) {
> - dev_err(dev, "Didn't find host pnor flash\n");
> - return -EINVAL;
> + pr_err("aspeed_lpc_ctrl: ioctl: Didn't find host pnor flash\n");
Again, don't do that :(
thanks,
greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] misc: aspeed-lpc-ctrl: Correct return values
2019-01-24 19:29 ` Vijay Khemka
@ 2019-02-11 5:22 ` Andrew Jeffery
2019-02-11 20:13 ` Vijay Khemka
0 siblings, 1 reply; 9+ messages in thread
From: Andrew Jeffery @ 2019-02-11 5:22 UTC (permalink / raw)
To: Vijay Khemka, Greg Kroah-Hartman
Cc: Arnd Bergmann, linux-aspeed, openbmc @ lists . ozlabs . org,
linux-kernel, Joel Stanley, linux-arm-kernel
On Fri, 25 Jan 2019, at 05:59, Vijay Khemka wrote:
>
>
> On 1/24/19, 12:16 AM, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org> wrote:
>
> On Wed, Jan 23, 2019 at 03:06:34PM -0800, Vijay Khemka wrote:
> > Corrected some of return values with appropriate meanings.
> >
> > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> > ---
> > drivers/misc/aspeed-lpc-ctrl.c | 15 +++++++--------
> > 1 file changed, 7 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/misc/aspeed-lpc-ctrl.c b/drivers/misc/aspeed-
> lpc-ctrl.c
> > index 332210e06e98..97ae341109d5 100644
> > --- a/drivers/misc/aspeed-lpc-ctrl.c
> > +++ b/drivers/misc/aspeed-lpc-ctrl.c
> > @@ -68,7 +68,6 @@ static long aspeed_lpc_ctrl_ioctl(struct file
> *file, unsigned int cmd,
> > unsigned long param)
> > {
> > struct aspeed_lpc_ctrl *lpc_ctrl = file_aspeed_lpc_ctrl(file);
> > - struct device *dev = file->private_data;
> > void __user *p = (void __user *)param;
> > struct aspeed_lpc_ctrl_mapping map;
> > u32 addr;
> > @@ -93,8 +92,8 @@ static long aspeed_lpc_ctrl_ioctl(struct file
> *file, unsigned int cmd,
> >
> > /* If memory-region is not described in device tree */
> > if (!lpc_ctrl->mem_size) {
> > - dev_err(dev, "Didn't find reserved memory\n");
> > - return -EINVAL;
> > + pr_err("aspeed_lpc_ctrl: ioctl: Didn't find reserved memory\n");
>
> Why did you change from dev_err() to pr_err()? You just lost a lot of
> information that the user previously was getting from dev_err() :(
>
> I did this as per review comment from Andrew Jeffery, as we don't want
> to put this error for driver. It has to be handled by userspace. But I
> am still reporting some error here.
Sorry? What I was trying to suggest was removing the logging altogether and
just returning the error code. Not simply changing how the logging is done.
Andrew
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] misc: aspeed-lpc-ctrl: Correct return values
2019-02-11 5:22 ` Andrew Jeffery
@ 2019-02-11 20:13 ` Vijay Khemka
0 siblings, 0 replies; 9+ messages in thread
From: Vijay Khemka @ 2019-02-11 20:13 UTC (permalink / raw)
To: Andrew Jeffery, Greg Kroah-Hartman
Cc: Arnd Bergmann, linux-aspeed, openbmc @ lists . ozlabs . org,
linux-kernel, Joel Stanley, linux-arm-kernel
On 2/10/19, 9:22 PM, "Andrew Jeffery" <andrew@aj.id.au> wrote:
On Fri, 25 Jan 2019, at 05:59, Vijay Khemka wrote:
>
>
> On 1/24/19, 12:16 AM, "Greg Kroah-Hartman" <gregkh@linuxfoundation.org> wrote:
>
> On Wed, Jan 23, 2019 at 03:06:34PM -0800, Vijay Khemka wrote:
> > Corrected some of return values with appropriate meanings.
> >
> > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> > ---
> > drivers/misc/aspeed-lpc-ctrl.c | 15 +++++++--------
> > 1 file changed, 7 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/misc/aspeed-lpc-ctrl.c b/drivers/misc/aspeed-
> lpc-ctrl.c
> > index 332210e06e98..97ae341109d5 100644
> > --- a/drivers/misc/aspeed-lpc-ctrl.c
> > +++ b/drivers/misc/aspeed-lpc-ctrl.c
> > @@ -68,7 +68,6 @@ static long aspeed_lpc_ctrl_ioctl(struct file
> *file, unsigned int cmd,
> > unsigned long param)
> > {
> > struct aspeed_lpc_ctrl *lpc_ctrl = file_aspeed_lpc_ctrl(file);
> > - struct device *dev = file->private_data;
> > void __user *p = (void __user *)param;
> > struct aspeed_lpc_ctrl_mapping map;
> > u32 addr;
> > @@ -93,8 +92,8 @@ static long aspeed_lpc_ctrl_ioctl(struct file
> *file, unsigned int cmd,
> >
> > /* If memory-region is not described in device tree */
> > if (!lpc_ctrl->mem_size) {
> > - dev_err(dev, "Didn't find reserved memory\n");
> > - return -EINVAL;
> > + pr_err("aspeed_lpc_ctrl: ioctl: Didn't find reserved memory\n");
>
> Why did you change from dev_err() to pr_err()? You just lost a lot of
> information that the user previously was getting from dev_err() :(
>
> I did this as per review comment from Andrew Jeffery, as we don't want
> to put this error for driver. It has to be handled by userspace. But I
> am still reporting some error here.
Sorry? What I was trying to suggest was removing the logging altogether and
just returning the error code. Not simply changing how the logging is done.
No issue, I can remove logging, will just return error code. Will send new patch with update.
Andrew
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] misc: aspeed-lpc-ctrl: Correct return values
@ 2019-05-01 22:38 Vijay Khemka
2019-05-02 6:40 ` Greg Kroah-Hartman
0 siblings, 1 reply; 9+ messages in thread
From: Vijay Khemka @ 2019-05-01 22:38 UTC (permalink / raw)
To: Arnd Bergmann, Greg Kroah-Hartman, Joel Stanley, Andrew Jeffery,
linux-arm-kernel, linux-aspeed, linux-kernel
Cc: sdasari, vijaykhemka
Corrected some of return values with appropriate meanings.
Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
---
drivers/misc/aspeed-lpc-ctrl.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/misc/aspeed-lpc-ctrl.c b/drivers/misc/aspeed-lpc-ctrl.c
index 332210e06e98..97ae341109d5 100644
--- a/drivers/misc/aspeed-lpc-ctrl.c
+++ b/drivers/misc/aspeed-lpc-ctrl.c
@@ -68,7 +68,6 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
unsigned long param)
{
struct aspeed_lpc_ctrl *lpc_ctrl = file_aspeed_lpc_ctrl(file);
- struct device *dev = file->private_data;
void __user *p = (void __user *)param;
struct aspeed_lpc_ctrl_mapping map;
u32 addr;
@@ -93,8 +92,8 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
/* If memory-region is not described in device tree */
if (!lpc_ctrl->mem_size) {
- dev_err(dev, "Didn't find reserved memory\n");
- return -EINVAL;
+ pr_err("aspeed_lpc_ctrl: ioctl: Didn't find reserved memory\n");
+ return -ENXIO;
}
map.size = lpc_ctrl->mem_size;
@@ -134,16 +133,16 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
if (map.window_type == ASPEED_LPC_CTRL_WINDOW_FLASH) {
if (!lpc_ctrl->pnor_size) {
- dev_err(dev, "Didn't find host pnor flash\n");
- return -EINVAL;
+ pr_err("aspeed_lpc_ctrl: ioctl: Didn't find host pnor flash\n");
+ return -ENXIO;
}
addr = lpc_ctrl->pnor_base;
size = lpc_ctrl->pnor_size;
} else if (map.window_type == ASPEED_LPC_CTRL_WINDOW_MEMORY) {
/* If memory-region is not described in device tree */
if (!lpc_ctrl->mem_size) {
- dev_err(dev, "Didn't find reserved memory\n");
- return -EINVAL;
+ pr_err("aspeed_lpc_ctrl: ioctl: Didn't find reserved memory\n");
+ return -ENXIO;
}
addr = lpc_ctrl->mem_base;
size = lpc_ctrl->mem_size;
@@ -239,7 +238,7 @@ static int aspeed_lpc_ctrl_probe(struct platform_device *pdev)
of_node_put(node);
if (rc) {
dev_err(dev, "Couldn't address to resource for reserved memory\n");
- return -ENOMEM;
+ return -ENXIO;
}
lpc_ctrl->mem_size = resource_size(&resm);
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] misc: aspeed-lpc-ctrl: Correct return values
2019-05-01 22:38 Vijay Khemka
@ 2019-05-02 6:40 ` Greg Kroah-Hartman
2019-05-02 6:49 ` Andrew Jeffery
0 siblings, 1 reply; 9+ messages in thread
From: Greg Kroah-Hartman @ 2019-05-02 6:40 UTC (permalink / raw)
To: Vijay Khemka
Cc: sdasari, Arnd Bergmann, linux-aspeed, Andrew Jeffery,
linux-kernel, Joel Stanley, linux-arm-kernel
On Wed, May 01, 2019 at 03:38:36PM -0700, Vijay Khemka wrote:
> Corrected some of return values with appropriate meanings.
>
> Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> ---
> drivers/misc/aspeed-lpc-ctrl.c | 15 +++++++--------
> 1 file changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/misc/aspeed-lpc-ctrl.c b/drivers/misc/aspeed-lpc-ctrl.c
> index 332210e06e98..97ae341109d5 100644
> --- a/drivers/misc/aspeed-lpc-ctrl.c
> +++ b/drivers/misc/aspeed-lpc-ctrl.c
> @@ -68,7 +68,6 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
> unsigned long param)
> {
> struct aspeed_lpc_ctrl *lpc_ctrl = file_aspeed_lpc_ctrl(file);
> - struct device *dev = file->private_data;
> void __user *p = (void __user *)param;
> struct aspeed_lpc_ctrl_mapping map;
> u32 addr;
This change is not reflected in your changelog text :(
Please fix up, or break this up into multiple patches.
greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] misc: aspeed-lpc-ctrl: Correct return values
2019-05-02 6:40 ` Greg Kroah-Hartman
@ 2019-05-02 6:49 ` Andrew Jeffery
2019-05-03 17:55 ` Vijay Khemka
0 siblings, 1 reply; 9+ messages in thread
From: Andrew Jeffery @ 2019-05-02 6:49 UTC (permalink / raw)
To: Greg Kroah-Hartman, Vijay Khemka
Cc: sdasari, Arnd Bergmann, linux-aspeed, linux-kernel, Joel Stanley,
linux-arm-kernel
On Thu, 2 May 2019, at 16:10, Greg Kroah-Hartman wrote:
> On Wed, May 01, 2019 at 03:38:36PM -0700, Vijay Khemka wrote:
> > Corrected some of return values with appropriate meanings.
> >
> > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> > ---
> > drivers/misc/aspeed-lpc-ctrl.c | 15 +++++++--------
> > 1 file changed, 7 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/misc/aspeed-lpc-ctrl.c b/drivers/misc/aspeed-lpc-ctrl.c
> > index 332210e06e98..97ae341109d5 100644
> > --- a/drivers/misc/aspeed-lpc-ctrl.c
> > +++ b/drivers/misc/aspeed-lpc-ctrl.c
> > @@ -68,7 +68,6 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
> > unsigned long param)
> > {
> > struct aspeed_lpc_ctrl *lpc_ctrl = file_aspeed_lpc_ctrl(file);
> > - struct device *dev = file->private_data;
> > void __user *p = (void __user *)param;
> > struct aspeed_lpc_ctrl_mapping map;
> > u32 addr;
>
> This change is not reflected in your changelog text :(
>
> Please fix up, or break this up into multiple patches.
The return value fixes should also be squashed into the patch that introduced those lines
given it hasn't yet been applied.
Further, IIRC I previously suggested removing the dev_err()s entirely, not just switching
them to pr_err(). Returning an error code is enough IMO, there's no need to pollute the
kernel logs with application-level errors. Or make them dev_dbg().
Andrew
>
> greg k-h
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] misc: aspeed-lpc-ctrl: Correct return values
2019-05-02 6:49 ` Andrew Jeffery
@ 2019-05-03 17:55 ` Vijay Khemka
0 siblings, 0 replies; 9+ messages in thread
From: Vijay Khemka @ 2019-05-03 17:55 UTC (permalink / raw)
To: Andrew Jeffery, Greg Kroah-Hartman
Cc: Sai Dasari, Arnd Bergmann, linux-aspeed, linux-kernel,
Joel Stanley, linux-arm-kernel
On 5/1/19, 11:49 PM, "Andrew Jeffery" <andrew@aj.id.au> wrote:
On Thu, 2 May 2019, at 16:10, Greg Kroah-Hartman wrote:
> On Wed, May 01, 2019 at 03:38:36PM -0700, Vijay Khemka wrote:
> > Corrected some of return values with appropriate meanings.
> >
> > Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
> > ---
> > drivers/misc/aspeed-lpc-ctrl.c | 15 +++++++--------
> > 1 file changed, 7 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/misc/aspeed-lpc-ctrl.c b/drivers/misc/aspeed-lpc-ctrl.c
> > index 332210e06e98..97ae341109d5 100644
> > --- a/drivers/misc/aspeed-lpc-ctrl.c
> > +++ b/drivers/misc/aspeed-lpc-ctrl.c
> > @@ -68,7 +68,6 @@ static long aspeed_lpc_ctrl_ioctl(struct file *file, unsigned int cmd,
> > unsigned long param)
> > {
> > struct aspeed_lpc_ctrl *lpc_ctrl = file_aspeed_lpc_ctrl(file);
> > - struct device *dev = file->private_data;
> > void __user *p = (void __user *)param;
> > struct aspeed_lpc_ctrl_mapping map;
> > u32 addr;
>
> This change is not reflected in your changelog text :(
>
> Please fix up, or break this up into multiple patches.
The return value fixes should also be squashed into the patch that introduced those lines
given it hasn't yet been applied.
Further, IIRC I previously suggested removing the dev_err()s entirely, not just switching
them to pr_err(). Returning an error code is enough IMO, there's no need to pollute the
kernel logs with application-level errors. Or make them dev_dbg().
Alright, I will replace with dev_dbg(), information can still be extracted by user with debug.
Andrew
>
> greg k-h
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-05-03 17:55 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-23 23:06 [PATCH] misc: aspeed-lpc-ctrl: Correct return values Vijay Khemka
2019-01-24 8:15 ` Greg Kroah-Hartman
2019-01-24 19:29 ` Vijay Khemka
2019-02-11 5:22 ` Andrew Jeffery
2019-02-11 20:13 ` Vijay Khemka
2019-05-01 22:38 Vijay Khemka
2019-05-02 6:40 ` Greg Kroah-Hartman
2019-05-02 6:49 ` Andrew Jeffery
2019-05-03 17:55 ` Vijay Khemka
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).