Received: by 10.223.176.5 with SMTP id f5csp441425wra; Sat, 3 Feb 2018 02:56:41 -0800 (PST) X-Google-Smtp-Source: AH8x225oKLJ+Qh3k8/XcFnEAbNAtvSlBQZGw5LR0FgOm9NC1heVuX/ilUDG4l0kQodp3Q9QxS+D7 X-Received: by 10.99.67.197 with SMTP id q188mr32790600pga.255.1517655401684; Sat, 03 Feb 2018 02:56:41 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1517655401; cv=none; d=google.com; s=arc-20160816; b=lwU5sApB5SLg/VA6q1s5skmDGJp6ywt8hmHtj97AJXUhqoW4ORx93SNdN8g7rEFItP s8R8ZEjyHvjvhax66VRGc2wC6sv6PJpa4d2kQeSepuC3F95b6dMjE4suwWwlPPXZb0he tk5CNWsce6+KNMqOxeTy2Q4qC1jYPR+4s/t70lLq75gq87xIXxKFX6YM6ggDtNmQagPu Yl4cnthkz5jsCrwi5+UDmijrAlyCpc1jr4zTVCvSPGuiuL44gY598dwRr5iGOSo0dAm0 HRhfqvJxs7HqVAwmUF4UmRObhLmi4QWP/kWtUrLfqnKmW0Q26GLcEFpONh0ECNSf29RK vuFg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date:arc-authentication-results; bh=/v6dYrEtsFjHw4QeR0hRVAktQCm+wr4aVoPcBv5UHsA=; b=ZMFlk58OV/efS5FUlvouZ49Mlg3Dpxc5f9V/Vcd+jQozlO7hHu137Z7jQpZK7KIOZ5 S45twnbjn+p9qy/r/t9UTWNEwNjEYCpXpRIzSwzzsHXS1XnIsAxLUl0vSlkc0IoPMi44 SVuLk9H4LC7+MBalbQeRf4PYRetsFTy68LkTGzct9oQ9b6f45QpUh2OUQfomPLSsuOfz dIYDXD0b9lKs1zFQynv8/7QAciUihrkgaMrTQgVWAm4B3p7kKx2mGVa/DgqgFPLMjNsU nskjdubPlpGiX3s78G3TbctkibU5kFTkDbsmRlbl/iq66slAfgRJIan9yrbHRdY+z5Rk ffUw== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id z15si2815589pgv.91.2018.02.03.02.56.26; Sat, 03 Feb 2018 02:56:41 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751770AbeBCKlo (ORCPT + 99 others); Sat, 3 Feb 2018 05:41:44 -0500 Received: from mail-wm0-f65.google.com ([74.125.82.65]:52179 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750821AbeBCKld (ORCPT ); Sat, 3 Feb 2018 05:41:33 -0500 Received: by mail-wm0-f65.google.com with SMTP id r71so17447772wmd.1 for ; Sat, 03 Feb 2018 02:41:32 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=/v6dYrEtsFjHw4QeR0hRVAktQCm+wr4aVoPcBv5UHsA=; b=tmp/IaaYHFO2st/crTvb6YAg+042t9sFREKulgbhWXgLAH/p1d5x08zGqGdlY8AvTb fNaD+umIwmRBVahnuHDrBrU3L8YJybd0pdyD6ZL4zGoFY1Syn0bVKgJUoBzBrw9DCUen 1jqnTAzD3g0tr5E4Q38+iHJR5PvvVAT2V2srj+rTMTWVoDjivsLdascIIwKIL05IhYkv KXHlh1t8Bn4gfv9mS/+eDPVdnEGQLjqWe7eEPJ7rmvFx0WLoskLt3EftecP3+EhKznxq MD7yMFUKY+EQ96zHHCN/D1KY99OjpzgZzgV8NCJwSZ1B3dVS+4Qfbqma6cJjx39x1Pu1 MxAw== X-Gm-Message-State: AKwxytfkvqDQtm+4omtX2gaoqdvEx/XpyQwy1qHYCjIS+DvdPLBsu+xV M5f8B5LhtlERbMpwWn3/NdmRUg== X-Received: by 10.28.235.17 with SMTP id j17mr33179667wmh.52.1517654490988; Sat, 03 Feb 2018 02:41:30 -0800 (PST) Received: from localhost ([2001:67c:1810:f055:68bc:1a13:fb0:44de]) by smtp.gmail.com with ESMTPSA id j126sm5261663wmj.1.2018.02.03.02.41.26 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sat, 03 Feb 2018 02:41:30 -0800 (PST) Date: Sat, 3 Feb 2018 11:41:24 +0100 From: Moritz Fischer To: "Luebbers, Enno" Cc: Wu Hao , Alan Tull , Moritz Fischer , linux-fpga@vger.kernel.org, linux-kernel , linux-api@vger.kernel.org, "Kang, Luwei" , "Zhang, Yi Z" , Tim Whisonant , Shiva Rao , Christopher Rauer , Xiao Guangrong Subject: Re: [PATCH v3 14/21] fpga: dfl: add fpga manager platform driver for FME Message-ID: <20180203104124.luniynyrr6iwxozd@derp-derp.local> References: <1511764948-20972-1-git-send-email-hao.wu@intel.com> <1511764948-20972-15-git-send-email-hao.wu@intel.com> <20180202094213.GB17015@hao-dev> <20180203002626.GA51125@eluebber-mac02.jf.intel.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="baaqpsedxowg56qp" Content-Disposition: inline In-Reply-To: <20180203002626.GA51125@eluebber-mac02.jf.intel.com> User-Agent: NeoMutt/20171027 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --baaqpsedxowg56qp Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Hi Hao, On Fri, Feb 02, 2018 at 04:26:26PM -0800, Luebbers, Enno wrote: > Hi Hao, Alan, >=20 > On Fri, Feb 02, 2018 at 05:42:13PM +0800, Wu Hao wrote: > > On Thu, Feb 01, 2018 at 04:00:36PM -0600, Alan Tull wrote: > > > On Mon, Nov 27, 2017 at 12:42 AM, Wu Hao wrote: > > >=20 > > > Hi Hao, > > >=20 > > > A few comments below. Besides that, looks good. > > >=20 > > > > This patch adds fpga manager driver for FPGA Management Engine (FME= ). It > > > > implements fpga_manager_ops for FPGA Partial Reconfiguration functi= on. > > > > > > > > Signed-off-by: Tim Whisonant > > > > Signed-off-by: Enno Luebbers > > > > Signed-off-by: Shiva Rao > > > > Signed-off-by: Christopher Rauer > > > > Signed-off-by: Kang Luwei > > > > Signed-off-by: Xiao Guangrong > > > > Signed-off-by: Wu Hao > > > > ---- > > > > v3: rename driver to dfl-fpga-fme-mgr > > > > implemented status callback for fpga manager > > > > rebased due to fpga api changes > > > > --- > > > > .../ABI/testing/sysfs-platform-fpga-dfl-fme-mgr | 8 + > > > > drivers/fpga/Kconfig | 6 + > > > > drivers/fpga/Makefile | 1 + > > > > drivers/fpga/fpga-dfl-fme-mgr.c | 318 +++++++++= ++++++++++++ > > > > drivers/fpga/fpga-dfl.h | 39 ++- > > > > 5 files changed, 371 insertions(+), 1 deletion(-) > > > > create mode 100644 Documentation/ABI/testing/sysfs-platform-fpga-d= fl-fme-mgr > > > > create mode 100644 drivers/fpga/fpga-dfl-fme-mgr.c > > > > > > > > diff --git a/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-= mgr b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr > > > > new file mode 100644 > > > > index 0000000..2d4f917 > > > > --- /dev/null > > > > +++ b/Documentation/ABI/testing/sysfs-platform-fpga-dfl-fme-mgr > > > > @@ -0,0 +1,8 @@ > > > > +What: /sys/bus/platform/devices/fpga-dfl-fme-mgr.0/interf= ace_id > > > > +Date: November 2017 > > > > +KernelVersion: 4.15 > > > > +Contact: Wu Hao > > > > +Description: Read-only. It returns interface id of partial recon= figuration > > > > + hardware. Userspace could use this information to c= heck if > > > > + current hardware is compatible with given image bef= ore FPGA > > > > + programming. > > >=20 > > > I'm a little confused by this. I can understand that the PR bitstream > > > has a dependency on the FPGA's static image, but I don't understand > > > the dependency of the bistream on the hardware that is used to program > > > the bitstream to the FPGA. > >=20 > > Sorry for the confusion, the interface_id is used to indicate the versi= on of > > the hardware for partial reconfiguration (it's part of the static image= of > > the FPGA device). Will improve the description on this. I'm not sure userland should be making the call on whether what you're trying to load is compatible or not. Isn't there a way to check this in your PR reconfiguration handler in-kernel? > >=20 >=20 > The interface_id expresses the compatibility of the static region with PR > bitstreams generated for it. It changes every time a new static region is > generated. >=20 > Would it make more sense to have the interface_id exposed as part of the = FME > device (which represents the static region)? I'm not sure - it kind of al= so > makes sense here, where you would have all the information in one place (= if the > interface_id matches, I can use this component to program a bitstream). >=20 > Sorry for my limited understanding of the infrastructure - would this same > "fpga-dfl-fme-mgr.0" be used for PR if we had multiple PR regions? In tha= t case > it would need to expose multiple interface_ids (or we'd have to track both > interface IDs and an identifier for the target PR region). >=20 > > >=20 > > > > diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig > > > > index 57da904..0171ecb 100644 > > > > --- a/drivers/fpga/Kconfig > > > > +++ b/drivers/fpga/Kconfig > > > > @@ -150,6 +150,12 @@ config FPGA_DFL_FME > > > > FPGA platform level management features. There shall be 1= FME > > > > per DFL based FPGA device. > > > > > > > > +config FPGA_DFL_FME_MGR > > > > + tristate "FPGA DFL FME Manager Driver" > > > > + depends on FPGA_DFL_FME > > > > + help > > > > + Say Y to enable FPGA Manager driver for FPGA Management E= ngine. > > > > + > > > > config INTEL_FPGA_DFL_PCI > > > > tristate "Intel FPGA DFL PCIe Device Driver" > > > > depends on PCI && FPGA_DFL > > > > diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile > > > > index cc75bb3..6378580 100644 > > > > --- a/drivers/fpga/Makefile > > > > +++ b/drivers/fpga/Makefile > > > > @@ -31,6 +31,7 @@ obj-$(CONFIG_OF_FPGA_REGION) +=3D of-fpg= a-region.o > > > > # FPGA Device Feature List Support > > > > obj-$(CONFIG_FPGA_DFL) +=3D fpga-dfl.o > > > > obj-$(CONFIG_FPGA_DFL_FME) +=3D fpga-dfl-fme.o > > > > +obj-$(CONFIG_FPGA_DFL_FME_MGR) +=3D fpga-dfl-fme-mgr.o > > > > > > > > fpga-dfl-fme-objs :=3D dfl-fme-main.o dfl-fme-pr.o > > > > > > > > diff --git a/drivers/fpga/fpga-dfl-fme-mgr.c b/drivers/fpga/fpga-df= l-fme-mgr.c > > > > new file mode 100644 > > > > index 0000000..70356ce > > > > --- /dev/null > > > > +++ b/drivers/fpga/fpga-dfl-fme-mgr.c > > > > @@ -0,0 +1,318 @@ > > > > +/* > > > > + * FPGA Manager Driver for FPGA Management Engine (FME) > > > > + * > > > > + * Copyright (C) 2017 Intel Corporation, Inc. > > > > + * > > > > + * Authors: > > > > + * Kang Luwei > > > > + * Xiao Guangrong > > > > + * Wu Hao > > > > + * Joseph Grecco > > > > + * Enno Luebbers > > > > + * Tim Whisonant > > > > + * Ananda Ravuri > > > > + * Christopher Rauer > > > > + * Henry Mitchel > > > > + * > > > > + * This work is licensed under the terms of the GNU GPL version 2. > > > > + * SPDX-License-Identifier: GPL-2.0 > > > > + */ > > > > + > > > > +#include > > > > +#include > > > > +#include > > > > + > > > > +#include "fpga-dfl.h" > > > > +#include "dfl-fme.h" > > > > + > > > > +#define PR_WAIT_TIMEOUT 8000000 > > > > +#define PR_HOST_STATUS_IDLE 0 > > > > + > > > > +struct fme_mgr_priv { > > > > + void __iomem *ioaddr; > > > > + u64 pr_error; > > > > +}; > > > > + > > > > +static ssize_t interface_id_show(struct device *dev, > > > > + struct device_attribute *attr, cha= r *buf) > > > > +{ > > > > + struct fpga_manager *mgr =3D dev_get_drvdata(dev); > > > > + struct fme_mgr_priv *priv =3D mgr->priv; > > > > + u64 intfc_id_l, intfc_id_h; > > > > + > > > > + intfc_id_l =3D readq(priv->ioaddr + FME_PR_INTFC_ID_L); > > > > + intfc_id_h =3D readq(priv->ioaddr + FME_PR_INTFC_ID_H); > > > > + > > > > + return scnprintf(buf, PAGE_SIZE, "%016llx%016llx\n", > > > > + (unsigned long long)intfc_id_h, > > > > + (unsigned long long)intfc_id_l); > > > > +} > > > > +static DEVICE_ATTR_RO(interface_id); > > > > + > > > > +static const struct attribute *fme_mgr_attrs[] =3D { > > > > + &dev_attr_interface_id.attr, > > > > + NULL, > > > > +}; > > > > + > > > > +static u64 pr_error_to_mgr_status(u64 err) > > > > +{ > > > > + u64 status =3D 0; > > > > + > > > > + if (err & FME_PR_ERR_OPERATION_ERR) > > > > + status |=3D FPGA_MGR_STATUS_OPERATION_ERR; > > > > + if (err & FME_PR_ERR_CRC_ERR) > > > > + status |=3D FPGA_MGR_STATUS_CRC_ERR; > > > > + if (err & FME_PR_ERR_INCOMPATIBLE_BS) > > > > + status |=3D FPGA_MGR_STATUS_INCOMPATIBLE_IMAGE_ERR; > > > > + if (err & FME_PR_ERR_PROTOCOL_ERR) > > > > + status |=3D FPGA_MGR_STATUS_IP_PROTOCOL_ERR; > > > > + if (err & FME_PR_ERR_FIFO_OVERFLOW) > > > > + status |=3D FPGA_MGR_STATUS_FIFO_OVERFLOW_ERR; > > > > + > > > > + return status; > > > > +} > > > > + > > > > +static u64 fme_mgr_pr_error_handle(void __iomem *fme_pr) > > > > +{ > > > > + u64 pr_status, pr_error; > > > > + > > > > + pr_status =3D readq(fme_pr + FME_PR_STS); > > > > + if (!(pr_status & FME_PR_STS_PR_STS)) > > > > + return 0; > > > > + > > > > + pr_error =3D readq(fme_pr + FME_PR_ERR); > > > > + writeq(pr_error, fme_pr + FME_PR_ERR); > > > > + > > > > + return pr_error; > > > > +} > > > > + > > > > +static int fme_mgr_write_init(struct fpga_manager *mgr, > > > > + struct fpga_image_info *info, > > > > + const char *buf, size_t count) > > > > +{ > > > > + struct device *dev =3D &mgr->dev; > > > > + struct fme_mgr_priv *priv =3D mgr->priv; > > > > + void __iomem *fme_pr =3D priv->ioaddr; > > > > + u64 pr_ctrl, pr_status; > > > > + > > > > + if (!(info->flags & FPGA_MGR_PARTIAL_RECONFIG)) { > > > > + dev_err(dev, "only support partial reconfiguration.= \n"); > > >=20 > > > supports > > >=20 > > > > + return -EINVAL; > > > > + } > > > > + > > > > + dev_dbg(dev, "resetting PR before initiated PR\n"); > > > > + > > > > + pr_ctrl =3D readq(fme_pr + FME_PR_CTRL); > > > > + pr_ctrl |=3D FME_PR_CTRL_PR_RST; > > > > + writeq(pr_ctrl, fme_pr + FME_PR_CTRL); > > > > + > > > > + if (readq_poll_timeout(fme_pr + FME_PR_CTRL, pr_ctrl, > > > > + pr_ctrl & FME_PR_CTRL_PR_RSTACK, 1, > > > > + PR_WAIT_TIMEOUT)) { > > > > + dev_err(dev, "maximum PR timeout\n"); > > >=20 > > > We have two dev_err's with the same "maximum PR timeout" so the user > > > would not be able to see at which point the timeout occurred. > > >=20 > > > I suggest to get rid of the word 'maximum' for both. This one could > > > be 'reset ack timeout" or something similar. > >=20 > > Sure, will switch to a more specific message per your suggestion. Thank= s. > >=20 > > >=20 > > > > + return -ETIMEDOUT; > > > > + } > > > > + > > > > + pr_ctrl =3D readq(fme_pr + FME_PR_CTRL); > > > > + pr_ctrl &=3D ~FME_PR_CTRL_PR_RST; > > > > + writeq(pr_ctrl, fme_pr + FME_PR_CTRL); > > > > + > > > > + dev_dbg(dev, > > > > + "waiting for PR resource in HW to be initialized an= d ready\n"); > > > > + > > > > + if (readq_poll_timeout(fme_pr + FME_PR_STS, pr_status, > > > > + (pr_status & FME_PR_STS_PR_STS) =3D= =3D > > > > + FME_PR_STS_PR_STS_IDLE, 1, PR_WAIT_T= IMEOUT)) { > > > > + dev_err(dev, "maximum PR timeout\n"); > > >=20 > > > "PR_STS timeout"? Or something better. > > >=20 > > > > + priv->pr_error =3D fme_mgr_pr_error_handle(fme_pr); > > > > + return -ETIMEDOUT; > > > > + } > > > > + > > > > + dev_dbg(dev, "check and clear previous PR error\n"); > > > > + priv->pr_error =3D fme_mgr_pr_error_handle(fme_pr); > > > > + if (priv->pr_error) > > > > + dev_dbg(dev, "previous PR error detected %llx\n", > > > > + (unsigned long long)priv->pr_error); > > > > + > > > > + dev_dbg(dev, "set PR port ID\n"); > > > > + > > > > + pr_ctrl =3D readq(fme_pr + FME_PR_CTRL); > > > > + pr_ctrl &=3D ~FME_PR_CTRL_PR_RGN_ID; > > > > + pr_ctrl |=3D FIELD_PREP(FME_PR_CTRL_PR_RGN_ID, info->region= _id); > > > > + writeq(pr_ctrl, fme_pr + FME_PR_CTRL); > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static int fme_mgr_write(struct fpga_manager *mgr, > > > > + const char *buf, size_t count) > > > > +{ > > > > + struct device *dev =3D &mgr->dev; > > > > + struct fme_mgr_priv *priv =3D mgr->priv; > > > > + void __iomem *fme_pr =3D priv->ioaddr; > > > > + u64 pr_ctrl, pr_status, pr_data; > > > > + int delay =3D 0, pr_credit, i =3D 0; > > > > + > > > > + dev_dbg(dev, "start request\n"); > > > > + > > > > + pr_ctrl =3D readq(fme_pr + FME_PR_CTRL); > > > > + pr_ctrl |=3D FME_PR_CTRL_PR_START; > > > > + writeq(pr_ctrl, fme_pr + FME_PR_CTRL); > > > > + > > > > + dev_dbg(dev, "pushing data from bitstream to HW\n"); > > > > + > > > > + pr_status =3D readq(fme_pr + FME_PR_STS); > > > > + pr_credit =3D FIELD_GET(FME_PR_STS_PR_CREDIT, pr_status); > > > > + > > > > + while (count > 0) { > > > > + while (pr_credit <=3D 1) { > > > > + if (delay++ > PR_WAIT_TIMEOUT) { > > > > + dev_err(dev, "maximum try\n"); > > >=20 > > > It looks like this is waiting for an entry in a queue and timing out > > > here. Could you add a some comments for pr_credit above and this > > > loop? Also a better error message, perhaps "PR credit timeout"? > >=20 > > Driver needs to read the PR credit to know if it could push PR data > > to hardware or not. I will add more description here on this PR credit, > > and use "PR credit timeout" as error message. > >=20 > > >=20 > > > > + return -ETIMEDOUT; > > > > + } > > > > + udelay(1); > > > > + > > > > + pr_status =3D readq(fme_pr + FME_PR_STS); > > > > + pr_credit =3D FIELD_GET(FME_PR_STS_PR_CREDI= T, pr_status); > > > > + } > > > > + > > > > + if (count >=3D 4) { > > > > + pr_data =3D 0; > > > > + pr_data |=3D FIELD_PREP(FME_PR_DATA_PR_DATA= _RAW, > > > > + *(((u32 *)buf) + i)); > > > > + writeq(pr_data, fme_pr + FME_PR_DATA); > > > > + count -=3D 4; > > > > + pr_credit--; > > > > + i++; > > > > + } else { > > > > + WARN_ON(1); > > > > + return -EINVAL; > > > > + } > > > > + } > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static int fme_mgr_write_complete(struct fpga_manager *mgr, > > > > + struct fpga_image_info *info) > > > > +{ > > > > + struct device *dev =3D &mgr->dev; > > > > + struct fme_mgr_priv *priv =3D mgr->priv; > > > > + void __iomem *fme_pr =3D priv->ioaddr; > > > > + u64 pr_ctrl; > > > > + > > > > + pr_ctrl =3D readq(fme_pr + FME_PR_CTRL); > > > > + pr_ctrl |=3D FME_PR_CTRL_PR_COMPLETE; > > > > + writeq(pr_ctrl, fme_pr + FME_PR_CTRL); > > > > + > > > > + dev_dbg(dev, "green bitstream push complete\n"); > > > > + dev_dbg(dev, "waiting for HW to release PR resource\n"); > > > > + > > > > + if (readq_poll_timeout(fme_pr + FME_PR_CTRL, pr_ctrl, > > > > + !(pr_ctrl & FME_PR_CTRL_PR_START), 1, > > > > + PR_WAIT_TIMEOUT)) { > > > > + dev_err(dev, "maximum try.\n"); > > >=20 > > > Some message specific to here also, please. > > >=20 > > > > + return -ETIMEDOUT; > > > > + } > > > > + > > > > + dev_dbg(dev, "PR operation complete, checking status\n"); > > > > + priv->pr_error =3D fme_mgr_pr_error_handle(fme_pr); > > > > + if (priv->pr_error) { > > > > + dev_dbg(dev, "PR error detected %llx\n", > > > > + (unsigned long long)priv->pr_error); > > > > + return -EIO; > > > > + } > > > > + > > > > + dev_dbg(dev, "PR done successfully\n"); > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static enum fpga_mgr_states fme_mgr_state(struct fpga_manager *mgr) > > > > +{ > > > > + return FPGA_MGR_STATE_UNKNOWN; > > > > +} > > > > + > > > > +static u64 fme_mgr_status(struct fpga_manager *mgr) > > > > +{ > > > > + struct fme_mgr_priv *priv =3D mgr->priv; > > > > + > > > > + return pr_error_to_mgr_status(priv->pr_error); > > > > +} > > > > + > > > > +static const struct fpga_manager_ops fme_mgr_ops =3D { > > > > + .write_init =3D fme_mgr_write_init, > > > > + .write =3D fme_mgr_write, > > > > + .write_complete =3D fme_mgr_write_complete, > > > > + .state =3D fme_mgr_state, > > > > + .status =3D fme_mgr_status, > > > > +}; > > > > + > > > > +static int fme_mgr_probe(struct platform_device *pdev) > > > > +{ > > > > + struct device *dev =3D &pdev->dev; > > > > + struct fme_mgr_priv *priv; > > > > + struct fpga_manager *mgr; > > > > + struct resource *res; > > > > + int ret; > > > > + > > > > + priv =3D devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); > > > > + if (!priv) > > > > + return -ENOMEM; > > > > + > > > > + res =3D platform_get_resource(pdev, IORESOURCE_MEM, 0); > > > > + priv->ioaddr =3D devm_ioremap(dev, res->start, resource_siz= e(res)); > > > > + if (IS_ERR(priv->ioaddr)) > > > > + return PTR_ERR(priv->ioaddr); > > > > + > > > > + ret =3D sysfs_create_files(&pdev->dev.kobj, fme_mgr_attrs); > > > > + if (ret) > > > > + return ret; > > > > + > > > > + mgr =3D devm_kzalloc(dev, sizeof(*mgr), GFP_KERNEL); > > > > + if (!mgr) > > > > + goto sysfs_remove_exit; > > > > + > > > > + mgr->name =3D "DFL FPGA Manager"; > > > > + mgr->mops =3D &fme_mgr_ops; > > > > + mgr->priv =3D priv; > > > > + mgr->parent =3D dev; > > > > + platform_set_drvdata(pdev, mgr); > > > > + > > > > + ret =3D fpga_mgr_register(mgr); > > > > + if (ret) { > > > > + dev_err(dev, "unable to register FPGA manager\n"); > > > > + goto sysfs_remove_exit; > > > > + } > > > > + > > > > + return 0; > > > > + > > > > +sysfs_remove_exit: > > > > + sysfs_remove_files(&pdev->dev.kobj, fme_mgr_attrs); > > > > + return ret; > > > > +} > > > > + > > > > +static int fme_mgr_remove(struct platform_device *pdev) > > > > +{ > > > > + struct fpga_manager *mgr =3D platform_get_drvdata(pdev); > > > > + > > > > + fpga_mgr_unregister(mgr); > > > > + sysfs_remove_files(&pdev->dev.kobj, fme_mgr_attrs); > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +static struct platform_driver fme_mgr_driver =3D { > > > > + .driver =3D { > > > > + .name =3D FPGA_DFL_FME_MGR, > > > > + }, > > > > + .probe =3D fme_mgr_probe, > > > > + .remove =3D fme_mgr_remove, > > > > +}; > > > > + > > > > +module_platform_driver(fme_mgr_driver); > > > > + > > > > +MODULE_DESCRIPTION("FPGA Manager for FPGA Management Engine"); > > > > +MODULE_AUTHOR("Intel Corporation"); > > > > +MODULE_LICENSE("GPL v2"); > > > > +MODULE_ALIAS("platform:fpga-dfl-fme-mgr"); > > > > diff --git a/drivers/fpga/fpga-dfl.h b/drivers/fpga/fpga-dfl.h > > > > index e5a1094..d45eb82 100644 > > > > --- a/drivers/fpga/fpga-dfl.h > > > > +++ b/drivers/fpga/fpga-dfl.h > > > > @@ -130,7 +130,44 @@ > > > > > > > > /* FME Partial Reconfiguration Sub Feature Register Set */ > > > > #define FME_PR_DFH DFH > > > > -#define FME_PR_SIZE DFH_SIZE > > > > +#define FME_PR_CTRL 0x8 > > > > +#define FME_PR_STS 0x10 > > > > +#define FME_PR_DATA 0x18 > > > > +#define FME_PR_ERR 0x20 > > > > +#define FME_PR_INTFC_ID_H 0xA8 > > > > +#define FME_PR_INTFC_ID_L 0xB0 > > > > +#define FME_PR_SIZE 0xB8 > > > > + > > > > +/* FME PR Control Register Bitfield */ > > > > +#define FME_PR_CTRL_PR_RST BIT(0) /* Reset PR engine */ > > > > +#define FME_PR_CTRL_PR_RSTACK BIT(4) /* Ack for PR engine reset = */ > > > > +#define FME_PR_CTRL_PR_RGN_ID GENMASK_ULL(9, 7) /* PR Regio= n ID */ > > > > +#define FME_PR_CTRL_PR_START BIT(12) /* Start to request for PR = service */ > > > > +#define FME_PR_CTRL_PR_COMPLETE BIT(13) /* PR data push com= plete notification */ > > > > + > > > > +/* FME PR Status Register Bitfield */ > > > > +/* Number of available entries in HW queue inside the PR engine. */ > > > > +#define FME_PR_STS_PR_CREDIT GENMASK_ULL(8, 0) > > > > +#define FME_PR_STS_PR_STS BIT(16) /* PR operation status */ > > > > +#define FME_PR_STS_PR_STS_IDLE 0 > > > > +#define FME_PR_STS_PR_CTRLR_STS GENMASK_ULL(22, 20) /* = Controller status */ > > > > +#define FME_PR_STS_PR_HOST_STS GENMASK_ULL(27, 24) /* PR host = status */ > > > > + > > > > +/* FME PR Data Register Bitfield */ > > > > +/* PR data from the raw-binary file. */ > > > > +#define FME_PR_DATA_PR_DATA_RAW GENMASK_ULL(32, 0) > > > > + > > > > +/* FME PR Error Register */ > > > > +/* Previous PR Operation errors detected. */ > > > > +#define FME_PR_ERR_OPERATION_ERR BIT(0) > > > > +/* CRC error detected. */ > > > > +#define FME_PR_ERR_CRC_ERR BIT(1) > > > > +/* Incompatible PR bitstream detected. */ > > > > +#define FME_PR_ERR_INCOMPATIBLE_BS BIT(2) > > > > +/* PR data push protocol violated. */ > > > > +#define FME_PR_ERR_PROTOCOL_ERR BIT(3) > > > > +/* PR data fifo overflow error detected */ > > > > +#define FME_PR_ERR_FIFO_OVERFLOW BIT(4) > > > > > > > > /* FME HSSI Sub Feature Register Set */ > > > > #define FME_HSSI_DFH DFH > > >=20 > > > I see fpga-dfl.h as enumeration code which is separate from any driver > > > implementation specifics other than what's required for the DFL > > > enumeration scheme. These PR engine #defines should move to a .h > > > that is dedicated to this specific PR hardware device. If someone else > > > adds a different PR device to the framework, their PR driver would > > > also have its own .h. Same for any other modules that aren't central > > > to DFL enumeration. > >=20 > > Sure, will move PR related register to a separated header file. > > DFL enumeration related registers, will still be kept in fpga-dfl.h. > >=20 > > Thanks > > Hao > >=20 > > >=20 > > > Thanks, > > > Alan > > >=20 > > > > -- > > > > 1.8.3.1 > > > > Thanks, Moritz --baaqpsedxowg56qp Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAEBCAAdFiEEowQ4eJSlIZpNWnl2UVwKRFcJcNgFAlp1kdAACgkQUVwKRFcJ cNhgvggAxmGY16qldCFyq7Pbn8/0EX9Xrxo7iK2Qk+kQ4ppwV3yh8u5fwl6I8DCX YF7EDUzgRqzvOb30GRg/kkpjBnhSmyosi24RCxIZZ4/V7+8Iq9VdeODjym+Z7YAK XIy8klDBPcazumjtNxRMHJlN87F/e84R2xlPM7kjf//J1/dtaqHQAOM0d1ljkQU4 bMuNvtnHN8XTOh2buJGPf7KqgYoCRuWhEkrD3ersXqWR3uokrbqtFEh8uSV5OiAI 9kmoAaVs9eoD3uhCbnvDbXt25GeNKHbPJw8BSINccAI0ku4Wgu7fjSh/uXIWGtJG B9lftVO42501jkX2zUJxpU15fP94Yw== =REsw -----END PGP SIGNATURE----- --baaqpsedxowg56qp--