From: piro yang <piroyangg@gmail.com>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: linux-staging@lists.linux.dev,
Linux Outreachy <outreachy@lists.linux.dev>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] staging:vme_user:fix the issue of using the wrong error code
Date: Tue, 12 Dec 2023 00:37:16 +0800 [thread overview]
Message-ID: <CALaNatM_2UKo5jS=9FWjCCrj0dA-01JDndc+Q2RF=1LTCbHfZQ@mail.gmail.com> (raw)
In-Reply-To: <CALaNatNpBJ6a-hv3MiV7NzuJ7wYLwHRCs3ppKM5O-UCvTSeiGg@mail.gmail.com>
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
prev parent reply other threads:[~2023-12-11 16:37 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
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 message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CALaNatM_2UKo5jS=9FWjCCrj0dA-01JDndc+Q2RF=1LTCbHfZQ@mail.gmail.com' \
--to=piroyangg@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=outreachy@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).