piro yang <piroyangg@xxxxxxxxx> 于2023年12月12日周二 00:13写道：
>
> Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> 于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@xxxxxxxxx>
> > > ---
> > > 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
