From: Xu Yilun <yilun.xu@linux.intel.com>
To: Peter Colberg <peter.colberg@intel.com>
Cc: Wu Hao <hao.wu@intel.com>, Tom Rix <trix@redhat.com>,
Moritz Fischer <mdf@kernel.org>, Xu Yilun <yilun.xu@intel.com>,
linux-fpga@vger.kernel.org, linux-kernel@vger.kernel.org,
Russ Weight <russ.weight@linux.dev>,
Marco Pagani <marpagan@redhat.com>,
Matthew Gerlach <matthew.gerlach@linux.intel.com>,
Russ Weight <russell.h.weight@intel.com>
Subject: Re: [RFC PATCH v2 9/9] fpga: dfl: fix kernel warning on port release/assign for SRIOV
Date: Tue, 23 Apr 2024 23:36:02 +0800 [thread overview]
Message-ID: <ZifVYiMgssOFjM17@yilunxu-OptiPlex-7050> (raw)
In-Reply-To: <20240409233942.828440-10-peter.colberg@intel.com>
On Tue, Apr 09, 2024 at 07:39:42PM -0400, Peter Colberg wrote:
> From: Xu Yilun <yilun.xu@intel.com>
>
> DFL ports are registered as platform devices in PF mode. The port device
> should be removed from the host when the user wants to configure the
> port as a VF and pass through to a virtual machine. The FME device
> ioctls DFL_FPGA_FME_PORT_RELEASE/ASSIGN are designed for this purpose.
>
> In the previous implementation, the port platform device is not completely
> destroyed on port release: it is removed from the system by
> platform_device_del(), but the platform device instance is retained.
> When the port assign ioctl is called, the platform device is added back by
> platform_device_add(), which conflicts with this comment of device_add():
> "Do not call this routine more than once for any device structure", and
> will cause a kernel warning at runtime.
>
> This patch tries to completely unregister the port platform device on
> release and registers a new one on assign. But the main work is to remove
> the dependency on struct dfl_feature_platform_data for many internal DFL
> APIs. This structure holds many DFL enumeration infos for feature devices.
> Many DFL APIs are expected to work with these info even when the port
> platform device is unregistered. But with the change the platform_data will
> be freed in this case. So this patch introduces a new structure
> dfl_feature_dev_data for these APIs, which acts similarly to the previous
> dfl_feature_platform_data. The dfl_feature_platform_data then only needs a
> pointer to dfl_feature_dev_data to make the feature device driver work.
>
> Signed-off-by: Xu Yilun <yilun.xu@intel.com>
> Signed-off-by: Russ Weight <russell.h.weight@intel.com>
> Signed-off-by: Peter Colberg <peter.colberg@intel.com>
> ---
> v2:
> - Split monolithic patch into series at request of maintainer
> - Substitute binfo->type for removed function feature_dev_id_type() in
> parse_feature_irqs().
> - Return ERR_PTR(-ENOMEM) on !feature->params in
> binfo_create_feature_dev_data().
> - Reorder cdev as first member of struct dfl_feature_platform_data
> such that container_of() to obtain pdata evaluates to a no-op.
> - Align kernel-doc function name for __dfl_fpga_cdev_find_port_data().
> ---
> drivers/fpga/dfl-afu-main.c | 9 +-
> drivers/fpga/dfl-fme-br.c | 24 +-
> drivers/fpga/dfl-fme-main.c | 6 +-
> drivers/fpga/dfl.c | 430 +++++++++++++++++-------------------
> drivers/fpga/dfl.h | 86 +++++---
> 5 files changed, 281 insertions(+), 274 deletions(-)
>
> diff --git a/drivers/fpga/dfl-afu-main.c b/drivers/fpga/dfl-afu-main.c
> index 42928cc7e42b..ead03b7aea70 100644
> --- a/drivers/fpga/dfl-afu-main.c
> +++ b/drivers/fpga/dfl-afu-main.c
> @@ -143,9 +143,8 @@ static int port_reset(struct platform_device *pdev)
> return ret;
> }
>
> -static int port_get_id(struct platform_device *pdev)
> +static int port_get_id(struct dfl_feature_dev_data *fdata)
> {
> - struct dfl_feature_dev_data *fdata = to_dfl_feature_dev_data(&pdev->dev);
> void __iomem *base;
>
> base = dfl_get_feature_ioaddr_by_id(fdata, PORT_FEATURE_ID_HEADER);
> @@ -156,7 +155,8 @@ static int port_get_id(struct platform_device *pdev)
> static ssize_t
> id_show(struct device *dev, struct device_attribute *attr, char *buf)
> {
> - int id = port_get_id(to_platform_device(dev));
> + struct dfl_feature_dev_data *fdata = to_dfl_feature_dev_data(dev);
> + int id = port_get_id(fdata);
My quick idea is we go with these steps:
1. refactor struct dfl_feature_platform_data then replace all dev/pdev
arguments with pdata when necessary.
2. factor out fdata from pdata, add fdata helpers.
3. massive pdata->fdata replacement.
4. delete all unused pdata helpers.
Please check if it is possible.
Thanks,
Yilun
next prev parent reply other threads:[~2024-04-23 15:41 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-09 23:39 [RFC PATCH v2 0/9] fpga: dfl: fix kernel warning on port release/assign for SRIOV Peter Colberg
2024-04-09 23:39 ` [RFC PATCH v2 1/9] fpga: dfl: alias dfl_feature_dev_data to dfl_feature_platform_data Peter Colberg
2024-04-09 23:39 ` [RFC PATCH v2 2/9] fpga: dfl: migrate AFU DMA region management driver to dfl_feature_dev_data Peter Colberg
2024-04-23 9:01 ` Xu Yilun
2024-04-09 23:39 ` [RFC PATCH v2 3/9] fpga: dfl: migrate AFU MMIO " Peter Colberg
2024-04-09 23:56 ` Colberg, Peter
2024-04-23 14:22 ` Xu Yilun
2024-04-09 23:39 ` [RFC PATCH v2 4/9] fpga: dfl: migrate FPGA Management Engine " Peter Colberg
2024-04-23 9:38 ` Xu Yilun
2024-04-09 23:39 ` [RFC PATCH v2 5/9] fpga: dfl: migrate FME partial reconfiguration " Peter Colberg
2024-04-09 23:39 ` [RFC PATCH v2 6/9] fpga: dfl: migrate Accelerated Function Unit " Peter Colberg
2024-04-23 14:31 ` Xu Yilun
2024-04-09 23:39 ` [RFC PATCH v2 7/9] fpga: dfl: migrate DFL support header " Peter Colberg
2024-04-09 23:39 ` [RFC PATCH v2 8/9] fpga: dfl: migrate dfl_get_feature_by_id() " Peter Colberg
2024-04-23 15:16 ` Xu Yilun
2024-04-09 23:39 ` [RFC PATCH v2 9/9] fpga: dfl: fix kernel warning on port release/assign for SRIOV Peter Colberg
2024-04-23 15:36 ` Xu Yilun [this message]
2024-04-23 8:27 ` [RFC PATCH v2 0/9] " Xu Yilun
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=ZifVYiMgssOFjM17@yilunxu-OptiPlex-7050 \
--to=yilun.xu@linux.intel.com \
--cc=hao.wu@intel.com \
--cc=linux-fpga@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=marpagan@redhat.com \
--cc=matthew.gerlach@linux.intel.com \
--cc=mdf@kernel.org \
--cc=peter.colberg@intel.com \
--cc=russ.weight@linux.dev \
--cc=russell.h.weight@intel.com \
--cc=trix@redhat.com \
--cc=yilun.xu@intel.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).