Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
From:   "Zhang, Tianfei" <tianfei.zhang@intel.com>
To:     "Wu, Hao" <hao.wu@intel.com>, "trix@redhat.com" <trix@redhat.com>,
        "mdf@kernel.org" <mdf@kernel.org>,
        "Xu, Yilun" <yilun.xu@intel.com>,
        "linux-fpga@vger.kernel.org" <linux-fpga@vger.kernel.org>,
        "linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
CC:     "corbet@lwn.net" <corbet@lwn.net>
Subject: RE: [PATCH v3] fpga: dfl: check feature type before parse irq info
Thread-Topic: [PATCH v3] fpga: dfl: check feature type before parse irq info
Thread-Index: AQHYNO3Mz7yyGRAOxUOWMPv4ER/dsqzC2COAgAAQnVA=
Date:   Thu, 17 Mar 2022 02:50:28 +0000
Message-ID: <BN9PR11MB5483AD216BC179029E279E0BE3129@BN9PR11MB5483.namprd11.prod.outlook.com>
References: <20220311021121.1504442-1-tianfei.zhang@intel.com>
 <BYAPR11MB3816A2AD9C59AC3A1B6494C485129@BYAPR11MB3816.namprd11.prod.outlook.com>
In-Reply-To: <BYAPR11MB3816A2AD9C59AC3A1B6494C485129@BYAPR11MB3816.namprd11.prod.outlook.com>
Accept-Language: zh-CN, en-US
Content-Language: en-US
dlp-product: dlpe-windows
dlp-version: 11.6.401.20
dlp-reaction: no-action
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?v6t0fegptKE2L2iv9YZKklFK+EeU2ngyM5v7sdNMm/mYgDFeMIWzLq7NsLTz?=
 =?us-ascii?Q?hWC/HNvW06WHc2ODoRJUmFy7LIYLNV3IvARHP9mgAh63/+J2RO7v4PU2kReA?=
 =?us-ascii?Q?uRfx6NWn1Acus02QTpBPzneu8eGz5XDivNlXCHo/ZlhP8yJzoK2WVXjx9xrC?=
 =?us-ascii?Q?gV/LzoKAILy/G1Pr9ZjaKU1RTg3yzd5i0u47hb0Ue5f5v7IQJzkzg1jDaOto?=
 =?us-ascii?Q?G6B5aTwF5F1Pcdl8nPo0Y34soS5vJDtj1G+YyY1NfdKRybdtfL+sm01CUFBS?=
 =?us-ascii?Q?liNK2mg+ppUa6X5eTM8i2y5kwEJhZ02HwzETq1/dklT2Ls7RUs+cjqS+Xrug?=
 =?us-ascii?Q?Aze4e+qrIvO4AZmaasPpvxxqq+akISPkLVyhP2vMGES94rGHZalNer3kChRO?=
 =?us-ascii?Q?5w94idu2dThwlAoOgKrhXljEroWyvKEE97LAp+U31vvsup9peMAftuJRrh60?=
 =?us-ascii?Q?7nZsK0GN71g2zD5quk6/65DUimUqsTfG+iKXbX6PBevFKiu8/4/FIYbgq7z8?=
 =?us-ascii?Q?XcWk7KZSpu/UOhYhUniYTUQUeu/3H+EI+Af/+0/tE9f29tqde8DRCKvaeT6S?=
 =?us-ascii?Q?Oy8pZt5aYchnc5RVWg/FnKXyWSKzQ1zZtL/1XHGTCc8gtKyjPIgaOFkpIpmc?=
 =?us-ascii?Q?bBhaagN31XqhCdwxc2QeqNZ6s/HRqFooPgPl4GH4FJ4WYPgqnd9Bxw/f6D1I?=
 =?us-ascii?Q?xVVa5LImkPbVqQlljReORWiQo80sDfTWZlisf+HOhv5GYeT6Gz6R8/DCJzOh?=
 =?us-ascii?Q?j88KWCyqqQRbrxUQyL3imIR+Q0Elb2BPf3FUsi2Tr5HR3Vc4AbNRg5Ag+9fj?=
 =?us-ascii?Q?3smWFMJbFFndsSK0WHzxpEPYQRKlz3npu/z2o+XpaZkVlZz/M1HzIe3YlTXM?=
 =?us-ascii?Q?xhfBTWnjMRXZVADEjzrDNzFiB4PmHpw5gRB5/ejCyTjN1XrGmwbgNvE6NKN8?=
 =?us-ascii?Q?xAcEeWWECSu2p6iUObaC/F4jqHlWfZcXTudIzcwL7VK1eIniyrm8YP6EvhsA?=
 =?us-ascii?Q?aFCkLKS0pZOYFzJuwvkuktSEHr2DzsDZqnElmA1XZx6EMWSIe0urvmLDY0vh?=
 =?us-ascii?Q?CvsbY3EwZjyIbcIISVE90fxtVYoB1LwutDc0D81ABVvg1FTwJ8prKymtmvvc?=
 =?us-ascii?Q?NoPbP8benI7ElP6ECNvb9ZpCg4T2536UPs2sRERoLndNEz/7cCCTqO7DUmGE?=
 =?us-ascii?Q?ah36fZQ4qtzKJ0vK3S0DOEKc4kfi4dJMY+K9mmX+hkC3o18heSOM0/HLc3BK?=
 =?us-ascii?Q?OiNVQKFPjBoon/k9PdL33dW7i+bdzkPfVgPYFtqumLI0/fbPjEqwfODyukuP?=
 =?us-ascii?Q?aYr0qTvMv/VVRnyxDMcG7GSKXwWJVWN7Wc5y+oGoFQFiML6/EGaLkj6mWVy1?=
 =?us-ascii?Q?/3TgA7RX9TWfHNg/PP0o4GrwWezB3Nf6F866Sm714cBi9Ef9wXBw+Q4k2qpb?=
 =?us-ascii?Q?D4g9G98dfT/wxrtoKeJEuvwXZ4hJySgH?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN9PR11MB5483.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9e70ca9e-1dbb-4c06-473b-08da07c0e564
X-MS-Exchange-CrossTenant-originalarrivaltime: 17 Mar 2022 02:50:28.9078
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: J3pUDyf8BG2WHJfqop3cjwOvJDeV5CLhYmBK15FhYV4GkCP9h72t+SwZqlvWVomLfB/+a/QEiHOPocwEAi1V1A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR11MB2583
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-3.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
	DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,RDNS_NONE,SPF_HELO_NONE,T_SCC_BODY_TEXT_LINE
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
	lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


> -----Original Message-----
> From: Wu, Hao <hao.wu@intel.com>
> Sent: Thursday, March 17, 2022 9:48 AM
> To: Zhang, Tianfei <tianfei.zhang@intel.com>; trix@redhat.com;
> mdf@kernel.org; Xu, Yilun <yilun.xu@intel.com>; linux-fpga@vger.kernel.or=
g;
> linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org
> Cc: corbet@lwn.net
> Subject: RE: [PATCH v3] fpga: dfl: check feature type before parse irq in=
fo
>=20
> > -----Original Message-----
> > From: Zhang, Tianfei <tianfei.zhang@intel.com>
> > Sent: Friday, March 11, 2022 10:11 AM
> > To: Wu, Hao <hao.wu@intel.com>; trix@redhat.com; mdf@kernel.org; Xu,
> > Yilun <yilun.xu@intel.com>; linux-fpga@vger.kernel.org;
> > linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org
> > Cc: corbet@lwn.net; Zhang, Tianfei <tianfei.zhang@intel.com>
> > Subject: [PATCH v3] fpga: dfl: check feature type before parse irq
> > info
> >
> > The feature ID of "Port User Interrupt" and the "PMCI Subsystem" are
> > identical, 0x12, but one is for FME, other is for Port. It should
> > check the feature type While parsing the irq info in
> > parse_feature_irqs().
> >
>=20
> What about this
> "Previously the feature IDs defined are unique, no matter which feature t=
ype.
> But currently we want to extend its usage to have a per-type feature ID s=
pace,
> so this patch adds feature type checking as well just before look into fe=
ature ID
> for different features which have irq info."

This commit looks good for me.

>=20
> > ---
> > v3: Remove "Fixes" in commit log with Hao's comment, this is a
> >     extension not a bug fix.
> >
> > v2: add DFL Feature ID Registry in documentation.
> >
> > Signed-off-by: Tianfei Zhang <tianfei.zhang@intel.com>
>=20
> Move up your signed off line before these changelogs.
>=20
> > ---
> >  Documentation/fpga/dfl.rst | 11 +++++++++++
> >  drivers/fpga/dfl.c         | 38 ++++++++++++++++++++++----------------
> >  2 files changed, 33 insertions(+), 16 deletions(-)
> >
> > diff --git a/Documentation/fpga/dfl.rst b/Documentation/fpga/dfl.rst
> > index ef9eec71f6f3..14f342bb85e4 100644
> > --- a/Documentation/fpga/dfl.rst
> > +++ b/Documentation/fpga/dfl.rst
> > @@ -502,6 +502,17 @@ Developer only needs to provide a sub feature
> > driver with matched feature id.
> >  FME Partial Reconfiguration Sub Feature driver (see
> > drivers/fpga/dfl-fme-pr.c)  could be a reference.
> >
> > +Individual DFL drivers are bound DFL devices based on Feature Type
> > +and
> > Feature ID.
> > +The definition of Feature Type and Feature ID can be found:
> > +
> > +https://github.com/OPAE/linux-dfl-feature-id/blob/master/dfl-feature-
> > +ids.rst
> > +
> > +If you want to add a new feature ID for FPGA DFL feature device, we
> > recommend that
>=20
> "recommend" or "must"

I think it is "must" if you want to use linux dfl driver.

>=20
> > +use a pull request to reserve a feature ID for DFL. Here is the DFL
> > +Feature ID
> > +Registry:
> > +
> > +https://github.com/OPAE/linux-dfl-feature-id
> > +
>=20
> Actually we don't have to put all details in kernel doc, and feature ID i=
s used
> inside the device, it could be used in linux or windows or other OS.
> What about putting a README file in your repo to describe the detailed pr=
ocess,
> not in kernel doc. And we can just put a link to that README here.
> Then split this one into another patch? How do you think?

I think the process is that submit a patch/pull request to " https://github=
.com/OPAE/linux-dfl-feature-id" for review if you have a new FeatureID.

>=20
> >  Location of DFLs on a PCI Device
> >  =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D
> >  The original method for finding a DFL on a PCI device assumed the
> > start of the diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c
> > index 599bb21d86af..6bff39ff21a0 100644
> > --- a/drivers/fpga/dfl.c
> > +++ b/drivers/fpga/dfl.c
> > @@ -940,9 +940,12 @@ static int parse_feature_irqs(struct
> > build_feature_devs_info *binfo,  {
> >  	void __iomem *base =3D binfo->ioaddr + ofst;
> >  	unsigned int i, ibase, inr =3D 0;
> > +	enum dfl_id_type type;
> >  	int virq;
> >  	u64 v;
> >
> > +	type =3D feature_dev_id_type(binfo->feature_dev);
> > +
> >  	/*
> >  	 * Ideally DFL framework should only read info from DFL header, but
> >  	 * current version DFL only provides mmio resources information for
> > @@ -957,22 +960,25 @@ static int parse_feature_irqs(struct
> > build_feature_devs_info *binfo,
> >  	 * code will be added. But in order to be compatible to old version
> >  	 * DFL, the driver may still fall back to these quirks.
> >  	 */
> > -	switch (fid) {
> > -	case PORT_FEATURE_ID_UINT:
> > -		v =3D readq(base + PORT_UINT_CAP);
> > -		ibase =3D FIELD_GET(PORT_UINT_CAP_FST_VECT, v);
> > -		inr =3D FIELD_GET(PORT_UINT_CAP_INT_NUM, v);
> > -		break;
> > -	case PORT_FEATURE_ID_ERROR:
> > -		v =3D readq(base + PORT_ERROR_CAP);
> > -		ibase =3D FIELD_GET(PORT_ERROR_CAP_INT_VECT, v);
> > -		inr =3D FIELD_GET(PORT_ERROR_CAP_SUPP_INT, v);
> > -		break;
> > -	case FME_FEATURE_ID_GLOBAL_ERR:
> > -		v =3D readq(base + FME_ERROR_CAP);
> > -		ibase =3D FIELD_GET(FME_ERROR_CAP_INT_VECT, v);
> > -		inr =3D FIELD_GET(FME_ERROR_CAP_SUPP_INT, v);
> > -		break;
> > +	if (type =3D=3D PORT_ID) {
> > +		switch (fid) {
> > +		case PORT_FEATURE_ID_UINT:
> > +			v =3D readq(base + PORT_UINT_CAP);
> > +			ibase =3D FIELD_GET(PORT_UINT_CAP_FST_VECT, v);
> > +			inr =3D FIELD_GET(PORT_UINT_CAP_INT_NUM, v);
> > +			break;
> > +		case PORT_FEATURE_ID_ERROR:
> > +			v =3D readq(base + PORT_ERROR_CAP);
> > +			ibase =3D FIELD_GET(PORT_ERROR_CAP_INT_VECT, v);
> > +			inr =3D FIELD_GET(PORT_ERROR_CAP_SUPP_INT, v);
> > +			break;
> > +		}
> > +	} else if (type =3D=3D FME_ID) {
> > +		if (fid =3D=3D FME_FEATURE_ID_GLOBAL_ERR) {
> > +			v =3D readq(base + FME_ERROR_CAP);
> > +			ibase =3D FIELD_GET(FME_ERROR_CAP_INT_VECT, v);
> > +			inr =3D FIELD_GET(FME_ERROR_CAP_SUPP_INT, v);
> > +		}
> >  	}
> >
> >  	if (!inr) {
> > --
> > 2.26.2