[PATCH v2] powerpc: vas-api: constify dynamic struct class in coproc api register

Jori Koolstra posted 1 patch 1 month ago
arch/powerpc/platforms/book3s/vas-api.c | 34 +++++++++++++++++++------
1 file changed, 26 insertions(+), 8 deletions(-)
[PATCH v2] powerpc: vas-api: constify dynamic struct class in coproc api register
Posted by Jori Koolstra 1 month ago
The class_create() call has been deprecated in favor of class_register()
as the driver core now allows for a struct class to be in read-only
memory.

In vas_register_coproc_api() the dynamic allocation of the struct class
corresonding to the coprocessor type (right now only nx-gzip) is
replaced by calling

	static const struct class* cop_to_class(enum vas_cop_type cop)

which links the coprocessor type to the appropriate static const struct
class.

Compile tested only.

Link: https://lore.kernel.org/all/2023040244-duffel-pushpin-f738@gregkh/

Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
---
v2: undo whitespace removal

 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;
 
 	rc = alloc_chrdev_region(&coproc_device.devt, 1, 1, name);
 	if (rc) {
@@ -619,13 +638,12 @@ int vas_register_coproc_api(struct module *mod, enum vas_cop_type cop_type,
 	pr_devel("%s device allocated, dev [%i,%i]\n", name,
 			MAJOR(coproc_device.devt), MINOR(coproc_device.devt));
 
-	coproc_device.class = class_create(name);
-	if (IS_ERR(coproc_device.class)) {
-		rc = PTR_ERR(coproc_device.class);
-		pr_err("Unable to create %s class %d\n", name, rc);
+	rc = class_register(class);
+	if (rc) {
+		pr_err("Unable to register %s class %d\n", name, rc);
 		goto err_class;
 	}
-	coproc_device.class->devnode = coproc_devnode;
+	coproc_device.class = class;
 	coproc_device.cop_type = cop_type;
 	coproc_device.vops = vops;
 
@@ -654,7 +672,7 @@ int vas_register_coproc_api(struct module *mod, enum vas_cop_type cop_type,
 err:
 	cdev_del(&coproc_device.cdev);
 err_cdev:
-	class_destroy(coproc_device.class);
+	class_unregister(coproc_device.class);
 err_class:
 	unregister_chrdev_region(coproc_device.devt, 1);
 	return rc;
@@ -668,6 +686,6 @@ void vas_unregister_coproc_api(void)
 	devno = MKDEV(MAJOR(coproc_device.devt), 0);
 	device_destroy(coproc_device.class, devno);
 
-	class_destroy(coproc_device.class);
+	class_unregister(coproc_device.class);
 	unregister_chrdev_region(coproc_device.devt, 1);
 }

base-commit: d466c332e106fe666d1e2f5a24d08e308bebbfa1
-- 
2.53.0
Re: [PATCH v2] powerpc: vas-api: constify dynamic struct class in coproc api register
Posted by Greg KH 1 month ago
On Sun, Mar 08, 2026 at 10:46:31PM +0100, Jori Koolstra wrote:
> The class_create() call has been deprecated in favor of class_register()
> as the driver core now allows for a struct class to be in read-only
> memory.
> 
> In vas_register_coproc_api() the dynamic allocation of the struct class
> corresonding to the coprocessor type (right now only nx-gzip) is
> replaced by calling
> 
> 	static const struct class* cop_to_class(enum vas_cop_type cop)
> 
> which links the coprocessor type to the appropriate static const struct
> class.
> 
> Compile tested only.
> 
> Link: https://lore.kernel.org/all/2023040244-duffel-pushpin-f738@gregkh/
> 
> Suggested-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
> ---
> v2: undo whitespace removal
> 
>  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?

thanks,

greg k-h
Re: [PATCH v2] powerpc: vas-api: constify dynamic struct class in coproc api register
Posted by Jori Koolstra 1 month ago
My bad, the earlier email went out to soon.

> Op 09-03-2026 07:01 CET schreef Greg KH <gregkh@linuxfoundation.org>:
> >  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.
Re: [PATCH v2] powerpc: vas-api: constify dynamic struct class in coproc api register
Posted by Greg KH 4 weeks, 1 day ago
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 <gregkh@linuxfoundation.org>:
> > >  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
Re: [PATCH v2] powerpc: vas-api: constify dynamic struct class in coproc api register
Posted by Haren Myneni 4 weeks ago
On Wed, 2026-03-11 at 09:23 +0100, Greg KH wrote:
> 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
> > > <gregkh@linuxfoundation.org>:
> > > >  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 :(

Yes, VAS is virtual accelerator switchboard which can support multiple
accelerator types. Hence added vas_register_api* interface for
different accelerators. But no plans to add other accelerators on
powerpc right now. So can we use only VAS_COP_TYPE_GZIP class and can
visit later. 


diff --git a/arch/powerpc/platforms/powernv/vas-window.c
b/arch/powerpc/platforms/powernv/vas-window.c
index 9f093176b8db..360fdd8f200e 100644
--- a/arch/powerpc/platforms/powernv/vas-window.c
+++ b/arch/powerpc/platforms/powernv/vas-window.c
@@ -1459,6 +1459,8 @@ static const struct vas_user_win_ops vops =  {
 int vas_register_api_powernv(struct module *mod, enum vas_cop_type
cop_type,
                             const char *name)
 {
+       if (cop_type != VAS_COP_TYPE_GZIP)
+               return -ENOTSUPP;
 
        return vas_register_coproc_api(mod, cop_type, name, &vops);
 }
diff --git a/arch/powerpc/platforms/pseries/vas.c
b/arch/powerpc/platforms/pseries/vas.c
index 9a28f2899716..19e11056e6aa 100644
--- a/arch/powerpc/platforms/pseries/vas.c
+++ b/arch/powerpc/platforms/pseries/vas.c
@@ -574,6 +574,9 @@ int vas_register_api_pseries(struct module *mod,
enum vas_cop_type cop_type,
        if (!copypaste_feat)
                return -ENOTSUPP;
 
+       if (cop_type != VAS_COP_TYPE_GZIP)
+               return -ENOTSUPP;
+
        return vas_register_coproc_api(mod, cop_type, name,
&vops_pseries);
 }
 EXPORT_SYMBOL_GPL(vas_register_api_pseries);

Thanks
Haren
Re: [PATCH v2] powerpc: vas-api: constify dynamic struct class in coproc api register
Posted by Jori Koolstra 1 week ago
Hi Haren,

> Op 12-03-2026 00:27 CET schreef Haren Myneni <haren@linux.ibm.com>:
> 
>  
> On Wed, 2026-03-11 at 09:23 +0100, Greg KH wrote:
> > 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
> > > > <gregkh@linuxfoundation.org>:
> > > > >  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 :(
> 
> Yes, VAS is virtual accelerator switchboard which can support multiple
> accelerator types. Hence added vas_register_api* interface for
> different accelerators. But no plans to add other accelerators on
> powerpc right now. So can we use only VAS_COP_TYPE_GZIP class and can
> visit later. 
> 

Are you OK with the way

   static const struct class* cop_to_class(enum vas_cop_type cop)

is used to connect the vas_cop_type to the proper device class? Or would you
prefer a different solution?

Thanks,
Jori.
Re: [PATCH v2] powerpc: vas-api: constify dynamic struct class in coproc api register
Posted by Jori Koolstra 1 month ago
My bad, the earlier email went out to soon.

> Op 09-03-2026 07:01 CET schreef Greg KH <gregkh@linuxfoundation.org>:
> > 
> >  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
Re: [PATCH v2] powerpc: vas-api: constify dynamic struct class in coproc api register
Posted by Jori Koolstra 1 month ago
> Op 09-03-2026 07:01 CET schreef Greg KH <gregkh@linuxfoundation.org>:
> 
> > 
> > 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, and this name is also used as thedevice
class name. Now, this is

There is 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