My bad, the earlier email went out to soon.

> Op 09-03-2026 07:01 CET schreef Greg KH <[email protected]>:
> > 
> >  arch/powerpc/platforms/book3s/vas-api.c | 34 +++++++++++++++++++------
> >  1 file changed, 26 insertions(+), 8 deletions(-)
> > 
> > diff --git a/arch/powerpc/platforms/book3s/vas-api.c 
> > b/arch/powerpc/platforms/book3s/vas-api.c
> > index ea4ffa63f043..e377981fd533 100644
> > --- a/arch/powerpc/platforms/book3s/vas-api.c
> > +++ b/arch/powerpc/platforms/book3s/vas-api.c
> > @@ -45,7 +45,7 @@ static struct coproc_dev {
> >     struct device *device;
> >     char *name;
> >     dev_t devt;
> > -   struct class *class;
> > +   const struct class *class;
> >     enum vas_cop_type cop_type;
> >     const struct vas_user_win_ops *vops;
> >  } coproc_device;
> > @@ -599,6 +599,21 @@ static struct file_operations coproc_fops = {
> >     .unlocked_ioctl = coproc_ioctl,
> >  };
> >  
> > +static const struct class nx_gzip_class = {
> > +   .name           = "nx-gzip",
> > +   .devnode        = coproc_devnode
> > +};
> > +
> > +static const struct class* cop_to_class(enum vas_cop_type cop)
> > +{
> > +   switch (cop) {
> > +   case VAS_COP_TYPE_GZIP:         return &nx_gzip_class;
> > +   default:
> > +           pr_err("No device class defined for cop type %d\n", cop);
> > +           return NULL;
> > +   }
> > +}
> > +
> >  /*
> >   * Supporting only nx-gzip coprocessor type now, but this API code
> >   * extended to other coprocessor types later.
> > @@ -609,6 +624,10 @@ int vas_register_coproc_api(struct module *mod, enum 
> > vas_cop_type cop_type,
> >  {
> >     int rc = -EINVAL;
> >     dev_t devno;
> > +   const struct class* class = cop_to_class(cop_type);
> > +
> > +   if (!class)
> > +           return rc;
> 
> How can this happen?
> 
> This feels odd, are different types of devices being registered here?  I
> don't see where VAS_COP_TYPE_GZIP was being tested in the original code,
> why add this additional logic?
> 

My line of thought is this:

There is a function vas_register_coproc_api() that does some kind of registering
for different coprocessor types. It has the following comment above:

/*
 * Supporting only nx-gzip coprocessor type now, but this API code
 * extended to other coprocessor types later.
 */

If you look at where this function is eventually triggered it is indeed only 
ever
passed VAS_COP_TYPE_GZIP. For instance, line 1238 of 
drivers/crypto/nx/nx-common-series.c
has the call

        ret = vas_register_api_pseries(THIS_MODULE, VAS_COP_TYPE_GZIP,
                                       "nx-gzip");

(which immediately calls vas_register_coproc_api())

It also passes a hard-coded name for the device ("nx-gzip"), and this name is 
also
used as the device class name. Now, this is a problem if we want to get rid of
class_create(). Somehow "nx-gzip" needs to get linked to the appropriate const 
struct
class. I figured it is better to use the cop_type, and a cop_to_class() 
function to
set this link. However, since the other co-processor types are not implemented 
(yet)
it would seem silly to already assign struct classes for these, hence the NULL 
return.
It is meant to signal: not implemented. Then again, if ever a new co-processor 
was added
you must update the cop_to_class()... but at least this is my line of thought.

There is also a function static char *cop_to_str(int cop) that strangely takes
an int instead of an enum vas_cop_type, and also misses an option I think.

> thanks,
> 
> greg k-h

Thanks,
Jori

Reply via email to