* [PATCH] staging:vme_user:fix the issue of using the wrong error code
@ 2023-12-11 15:54 Piro Yang
2023-12-11 16:01 ` Greg KH
0 siblings, 1 reply; 4+ messages in thread
From: Piro Yang @ 2023-12-11 15:54 UTC (permalink / raw
To: Greg KH; +Cc: Piro Yang, linux-staging, Linux Outreachy, linux-kernel
1. the error code of ENOSYS indicates Invalid system call number,
but there is not system call
2. unified the logging error message, when slave_set func is NULL
Signed-off-by: Piro Yang <piroyangg@gmail.com>
---
drivers/staging/vme_user/vme.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/vme_user/vme.c b/drivers/staging/vme_user/vme.c
index 5c416c31ec57..e9461a7a7ab8 100644
--- a/drivers/staging/vme_user/vme.c
+++ b/drivers/staging/vme_user/vme.c
@@ -340,8 +340,8 @@ int vme_slave_set(struct vme_resource *resource, int enabled,
image = list_entry(resource->entry, struct vme_slave_resource, list);
if (!bridge->slave_set) {
- dev_err(bridge->parent, "Function not supported\n");
- return -ENOSYS;
+ dev_err(bridge->parent, "%s not supported\n", __func__);
+ return -EINVAL;
}
if (!(((image->address_attr & aspace) == aspace) &&
--
2.25.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] staging:vme_user:fix the issue of using the wrong error code
2023-12-11 15:54 [PATCH] staging:vme_user:fix the issue of using the wrong error code Piro Yang
@ 2023-12-11 16:01 ` Greg KH
2023-12-11 16:13 ` piro yang
0 siblings, 1 reply; 4+ messages in thread
From: Greg KH @ 2023-12-11 16:01 UTC (permalink / raw
To: Piro Yang; +Cc: linux-staging, Linux Outreachy, linux-kernel
On Mon, Dec 11, 2023 at 11:54:53PM +0800, Piro Yang wrote:
> 1. the error code of ENOSYS indicates Invalid system call number,
> but there is not system call
>
> 2. unified the logging error message, when slave_set func is NULL
That is two different things, and as such, should be two different
changes, right?
Yes, it's picky, but that's what the staging driver code is for, to get
comfortable doing kernel development changes properly.
Also, are you sure this second change is correct:
>
> Signed-off-by: Piro Yang <piroyangg@gmail.com>
> ---
> drivers/staging/vme_user/vme.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/staging/vme_user/vme.c b/drivers/staging/vme_user/vme.c
> index 5c416c31ec57..e9461a7a7ab8 100644
> --- a/drivers/staging/vme_user/vme.c
> +++ b/drivers/staging/vme_user/vme.c
> @@ -340,8 +340,8 @@ int vme_slave_set(struct vme_resource *resource, int enabled,
> image = list_entry(resource->entry, struct vme_slave_resource, list);
>
> if (!bridge->slave_set) {
> - dev_err(bridge->parent, "Function not supported\n");
> - return -ENOSYS;
> + dev_err(bridge->parent, "%s not supported\n", __func__);
__func__ is not the same here as "Function", right? "Function" is the
functionality of the thing that is attempted here, so replacing that
word with the function name seems odd to me, don't you think?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] staging:vme_user:fix the issue of using the wrong error code
2023-12-11 16:01 ` Greg KH
@ 2023-12-11 16:13 ` piro yang
2023-12-11 16:37 ` piro yang
0 siblings, 1 reply; 4+ messages in thread
From: piro yang @ 2023-12-11 16:13 UTC (permalink / raw
To: Greg KH; +Cc: linux-staging, Linux Outreachy, linux-kernel
Greg KH <gregkh@linuxfoundation.org> 于2023年12月12日周二 00:01写道：
>
> On Mon, Dec 11, 2023 at 11:54:53PM +0800, Piro Yang wrote:
> > 1. the error code of ENOSYS indicates Invalid system call number,
> > but there is not system call
> >
> > 2. unified the logging error message, when slave_set func is NULL
>
> That is two different things, and as such, should be two different
> changes, right?
>
> Yes, it's picky, but that's what the staging driver code is for, to get
> comfortable doing kernel development changes properly.
>
> Also, are you sure this second change is correct:
yes, I'm sure .
the vme_slave_set function is diffierent
/*****************************vme_slave_set***********************/
int vme_slave_set(struct vme_resource *resource, int enabled,
unsigned long long vme_base, unsigned long long size,
dma_addr_t buf_base, u32 aspace, u32 cycle)
{
...
if (!bridge->slave_set) {
dev_err(bridge->parent, "Function not supported\n");
return -ENOSYS;
}
/******************other functions like below: *******************/
int vme_slave_get(struct vme_resource *resource, int *enabled,
unsigned long long *vme_base, unsigned long long *size,
dma_addr_t *buf_base, u32 *aspace, u32 *cycle)
{
....
if (!bridge->slave_get) {
dev_err(bridge->parent, "%s not supported\n", __func__);
return -EINVAL;
}
int vme_master_set(struct vme_resource *resource, int enabled,
unsigned long long vme_base, unsigned long long size,
u32 aspace, u32 cycle, u32 dwidth)
{
....
if (!bridge->master_set) {
dev_warn(bridge->parent, "%s not supported\n", __func__);
return -EINVAL;
}
int vme_master_get(struct vme_resource *resource, int *enabled,
unsigned long long *vme_base, unsigned long long *size,
u32 *aspace, u32 *cycle, u32 *dwidth)
{
....
if (!bridge->master_get) {
dev_warn(bridge->parent, "%s not supported\n", __func__);
return -EINVAL;
}
>
> >
> > Signed-off-by: Piro Yang <piroyangg@gmail.com>
> > ---
> > drivers/staging/vme_user/vme.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/vme_user/vme.c b/drivers/staging/vme_user/vme.c
> > index 5c416c31ec57..e9461a7a7ab8 100644
> > --- a/drivers/staging/vme_user/vme.c
> > +++ b/drivers/staging/vme_user/vme.c
> > @@ -340,8 +340,8 @@ int vme_slave_set(struct vme_resource *resource, int enabled,
> > image = list_entry(resource->entry, struct vme_slave_resource, list);
> >
> > if (!bridge->slave_set) {
> > - dev_err(bridge->parent, "Function not supported\n");
> > - return -ENOSYS;
> > + dev_err(bridge->parent, "%s not supported\n", __func__);
>
> __func__ is not the same here as "Function", right? "Function" is the
> functionality of the thing that is attempted here, so replacing that
> word with the function name seems odd to me, don't you think?
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] staging:vme_user:fix the issue of using the wrong error code
2023-12-11 16:13 ` piro yang
@ 2023-12-11 16:37 ` piro yang
0 siblings, 0 replies; 4+ messages in thread
From: piro yang @ 2023-12-11 16:37 UTC (permalink / raw
To: Greg KH; +Cc: linux-staging, Linux Outreachy, linux-kernel
piro yang <piroyangg@gmail.com> 于2023年12月12日周二 00:13写道：
>
> Greg KH <gregkh@linuxfoundation.org> 于2023年12月12日周二 00:01写道：
> >
> > On Mon, Dec 11, 2023 at 11:54:53PM +0800, Piro Yang wrote:
> > > 1. the error code of ENOSYS indicates Invalid system call number,
> > > but there is not system call
> > >
> > > 2. unified the logging error message, when slave_set func is NULL
> >
> > That is two different things, and as such, should be two different
> > changes, right?
> >
> > Yes, it's picky, but that's what the staging driver code is for, to get
> > comfortable doing kernel development changes properly.
> >
> > Also, are you sure this second change is correct:
>
> yes, I'm sure .
> the vme_slave_set function is diffierent
> /*****************************vme_slave_set***********************/
> int vme_slave_set(struct vme_resource *resource, int enabled,
> unsigned long long vme_base, unsigned long long size,
> dma_addr_t buf_base, u32 aspace, u32 cycle)
> {
> ...
> if (!bridge->slave_set) {
> dev_err(bridge->parent, "Function not supported\n");
> return -ENOSYS;
> }
>
> /******************other functions like below: *******************/
> int vme_slave_get(struct vme_resource *resource, int *enabled,
> unsigned long long *vme_base, unsigned long long *size,
> dma_addr_t *buf_base, u32 *aspace, u32 *cycle)
> {
> ....
> if (!bridge->slave_get) {
> dev_err(bridge->parent, "%s not supported\n", __func__);
> return -EINVAL;
> }
>
> int vme_master_set(struct vme_resource *resource, int enabled,
> unsigned long long vme_base, unsigned long long size,
> u32 aspace, u32 cycle, u32 dwidth)
> {
> ....
> if (!bridge->master_set) {
> dev_warn(bridge->parent, "%s not supported\n", __func__);
> return -EINVAL;
> }
>
> int vme_master_get(struct vme_resource *resource, int *enabled,
> unsigned long long *vme_base, unsigned long long *size,
> u32 *aspace, u32 *cycle, u32 *dwidth)
> {
> ....
> if (!bridge->master_get) {
> dev_warn(bridge->parent, "%s not supported\n", __func__);
> return -EINVAL;
> }
>
> >
> > >
> > > Signed-off-by: Piro Yang <piroyangg@gmail.com>
> > > ---
> > > drivers/staging/vme_user/vme.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/staging/vme_user/vme.c b/drivers/staging/vme_user/vme.c
> > > index 5c416c31ec57..e9461a7a7ab8 100644
> > > --- a/drivers/staging/vme_user/vme.c
> > > +++ b/drivers/staging/vme_user/vme.c
> > > @@ -340,8 +340,8 @@ int vme_slave_set(struct vme_resource *resource, int enabled,
> > > image = list_entry(resource->entry, struct vme_slave_resource, list);
> > >
> > > if (!bridge->slave_set) {
> > > - dev_err(bridge->parent, "Function not supported\n");
> > > - return -ENOSYS;
> > > + dev_err(bridge->parent, "%s not supported\n", __func__);
> >
> > __func__ is not the same here as "Function", right? "Function" is the
> > functionality of the thing that is attempted here, so replacing that
> > word with the function name seems odd to me, don't you think?
> >
yes, __func__ is not the same here as "Function".
but I think the logging error message format shoud be unified
with other functions.
like: vme_slave_get(){
.....
if (!bridge->slave_get) {
dev_err(bridge->parent, "%s not supported\n", __func__);
> > thanks,
> >
> > greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-12-11 16:37 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-11 15:54 [PATCH] staging:vme_user:fix the issue of using the wrong error code Piro Yang
2023-12-11 16:01 ` Greg KH
2023-12-11 16:13 ` piro yang
2023-12-11 16:37 ` piro yang
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 read-only IMAP folder(s) and NNTP newsgroup(s).