public inbox for linuxppc-dev@ozlabs.org
 help / color / mirror / Atom feed
From: Jori Koolstra <jkoolstra@xs4all.nl>
To: Haren Myneni <haren@linux.ibm.com>, Greg KH <gregkh@linuxfoundation.org>
Cc: Madhavan Srinivasan <maddy@linux.ibm.com>,
	Michael Ellerman <mpe@ellerman.id.au>,
	Nicholas Piggin <npiggin@gmail.com>,
	"Christophe Leroy (CS GROUP)" <chleroy@kernel.org>,
	Srikar Dronamraju <srikar@linux.ibm.com>,
	Jonathan Greental <yonatan02greental@gmail.com>,
	Kees Cook <kees@kernel.org>,
	Shrikanth Hegde <sshegde@linux.ibm.com>,
	"open list:LINUX FOR POWERPC (32-BIT AND 64-BIT)"
	<linuxppc-dev@lists.ozlabs.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2] powerpc: vas-api: constify dynamic struct class in coproc api register
Date: Wed, 1 Apr 2026 19:14:16 +0200 (CEST)	[thread overview]
Message-ID: <1967941740.854847.1775063656203@kpc.webmail.kpnmail.nl> (raw)
In-Reply-To: <b0a99a20f5f56ad8291d91b209d8ad5f7515bb23.camel@linux.ibm.com>

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.


      reply	other threads:[~2026-04-01 17:15 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-08 21:46 [PATCH v2] powerpc: vas-api: constify dynamic struct class in coproc api register Jori Koolstra
2026-03-09  6:01 ` Greg KH
2026-03-09 10:35   ` Jori Koolstra
2026-03-09 10:46   ` Jori Koolstra
2026-03-09 10:48   ` Jori Koolstra
2026-03-11  8:23     ` Greg KH
2026-03-11 23:27       ` Haren Myneni
2026-04-01 17:14         ` Jori Koolstra [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=1967941740.854847.1775063656203@kpc.webmail.kpnmail.nl \
    --to=jkoolstra@xs4all.nl \
    --cc=chleroy@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=haren@linux.ibm.com \
    --cc=kees@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=maddy@linux.ibm.com \
    --cc=mpe@ellerman.id.au \
    --cc=npiggin@gmail.com \
    --cc=srikar@linux.ibm.com \
    --cc=sshegde@linux.ibm.com \
    --cc=yonatan02greental@gmail.com \
    /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