All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Zhangfei Gao <zhangfei.gao@linaro.org>
Cc: Arnd Bergmann <arnd@arndb.de>,
	jonathan.cameron@huawei.com, kenneth-lee-2012@foxmail.com,
	Wangzhou <wangzhou1@hisilicon.com>,
	linux-accelerators@lists.ozlabs.org,
	linux-kernel@vger.kernel.org,
	Kenneth Lee <liguozhu@hisilicon.com>,
	Zaibo Xu <xuzaibo@huawei.com>
Subject: Re: [PATCH v2 2/2] uacce: add uacce driver
Date: Wed, 28 Aug 2019 17:22:01 +0200	[thread overview]
Message-ID: <20190828152201.GA10163@kroah.com> (raw)
In-Reply-To: <1566998876-31770-3-git-send-email-zhangfei.gao@linaro.org>

On Wed, Aug 28, 2019 at 09:27:56PM +0800, Zhangfei Gao wrote:
> +struct uacce {
> +	const char *drv_name;
> +	const char *algs;
> +	const char *api_ver;
> +	unsigned int flags;
> +	unsigned long qf_pg_start[UACCE_QFRT_MAX];
> +	struct uacce_ops *ops;
> +	struct device *pdev;
> +	bool is_vf;
> +	u32 dev_id;
> +	struct cdev cdev;
> +	struct device dev;
> +	void *priv;
> +	atomic_t state;
> +	int prot;
> +	struct mutex q_lock;
> +	struct list_head qs;
> +};

At a quick glance, this problem really stood out to me.  You CAN NOT
have two different objects within a structure that have different
lifetime rules and reference counts.  You do that here with both a
'struct cdev' and a 'struct device'.  Pick one or the other, but never
both.

I would recommend using a 'struct device' and then a 'struct cdev *'.
That way you get the advantage of using the driver model properly, and
then just adding your char device node pointer to "the side" which
interacts with this device.

Then you might want to call this "struct uacce_device" :)

thanks,

greg k-h

  reply	other threads:[~2019-08-28 15:22 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-28 13:27 [PATCH v2 0/2] Add uacce module for Accelerator Zhangfei Gao
2019-08-28 13:27 ` [PATCH v2 1/2] uacce: Add documents for uacce Zhangfei Gao
2019-08-28 13:27 ` [PATCH v2 2/2] uacce: add uacce driver Zhangfei Gao
2019-08-28 15:22   ` Greg Kroah-Hartman [this message]
2019-08-29  9:05     ` zhangfei
2019-08-29  9:54       ` Greg Kroah-Hartman
2019-08-30  8:48         ` zhangfei
2019-08-30 14:54         ` zhangfei
2019-09-02  3:44           ` zhangfei
2019-09-02  8:52             ` Greg Kroah-Hartman

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=20190828152201.GA10163@kroah.com \
    --to=gregkh@linuxfoundation.org \
    --cc=arnd@arndb.de \
    --cc=jonathan.cameron@huawei.com \
    --cc=kenneth-lee-2012@foxmail.com \
    --cc=liguozhu@hisilicon.com \
    --cc=linux-accelerators@lists.ozlabs.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=wangzhou1@hisilicon.com \
    --cc=xuzaibo@huawei.com \
    --cc=zhangfei.gao@linaro.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.