On Mon, Mar 09, 2026 at 11:48:57AM +0100, Jori Koolstra wrote:
> 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.
> */
I guess "later" never happened :(
> 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.
Ok, I'll defer to the maintainer here, as it's their code, and seems to
not follow the "normal" style of handling classes.
thanks,
greg k-h
