| Message ID | 20210709134229.2510349-5-trix@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fpga: fpga-mgr: move compat_id from fpga_mgr to dfl | expand |
Hi,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[cannot apply to v5.13 next-20210709]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/trix-redhat-com/fpga-fpga-mgr-move-compat_id-from-fpga_mgr-to-dfl/20210709-214433
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git f55966571d5eb2876a11e48e798b4592fa1ffbb7
config: i386-randconfig-a016-20210709 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/afee0ef49cd3ec87aedcae556893cdea33cf7f1f
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review trix-redhat-com/fpga-fpga-mgr-move-compat_id-from-fpga_mgr-to-dfl/20210709-214433
git checkout afee0ef49cd3ec87aedcae556893cdea33cf7f1f
# save the attached .config to linux build tree
make W=1 ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "fme_mgr_get_compat_id" [drivers/fpga/dfl-fme-region.ko] undefined!
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
> Subject: [PATCH v2 3/4] fpga: dfl: implement the compat_id_show region op > > From: Tom Rix <trix@redhat.com> > > Make sure dfl will work as previously when compat_id is removed > from struct fpga_manager. Store and pass the compat_id values > internal to dfl. > > Signed-off-by: Tom Rix <trix@redhat.com> > --- > drivers/fpga/dfl-fme-mgr.c | 16 +++++++++++++--- > drivers/fpga/dfl-fme-region.c | 14 ++++++++++++++ > drivers/fpga/dfl.h | 14 ++++++++++++++ > 3 files changed, 41 insertions(+), 3 deletions(-) > > diff --git a/drivers/fpga/dfl-fme-mgr.c b/drivers/fpga/dfl-fme-mgr.c > index d5861d13b3069..cd0b9157ea6e5 100644 > --- a/drivers/fpga/dfl-fme-mgr.c > +++ b/drivers/fpga/dfl-fme-mgr.c > @@ -22,6 +22,7 @@ > #include <linux/io-64-nonatomic-lo-hi.h> > #include <linux/fpga/fpga-mgr.h> > > +#include "dfl.h" > #include "dfl-fme-pr.h" > > /* FME Partial Reconfiguration Sub Feature Register Set */ > @@ -70,6 +71,7 @@ > struct fme_mgr_priv { > void __iomem *ioaddr; > u64 pr_error; > + struct dfl_compat_id compat_id; > }; > > static u64 pr_error_to_mgr_status(u64 err) > @@ -272,13 +274,21 @@ static const struct fpga_manager_ops fme_mgr_ops > = { > .status = fme_mgr_status, > }; > > -static void fme_mgr_get_compat_id(void __iomem *fme_pr, > - struct fpga_compat_id *id) > +static void _fme_mgr_get_compat_id(void __iomem *fme_pr, > + struct dfl_compat_id *id) > { > id->id_l = readq(fme_pr + FME_PR_INTFC_ID_L); > id->id_h = readq(fme_pr + FME_PR_INTFC_ID_H); > } > > +void fme_mgr_get_compat_id(struct fpga_manager *mgr, > + struct dfl_compat_id *id) > +{ > + struct fme_mgr_priv *priv = mgr->priv; > + *id = priv->compat_id; > +} > +EXPORT_SYMBOL_GPL(fme_mgr_get_compat_id); > + > static int fme_mgr_probe(struct platform_device *pdev) > { > struct dfl_fme_mgr_pdata *pdata = dev_get_platdata(&pdev->dev); > @@ -306,7 +316,7 @@ static int fme_mgr_probe(struct platform_device *pdev) > if (!compat_id) > return -ENOMEM; > > - fme_mgr_get_compat_id(priv->ioaddr, compat_id); > + _fme_mgr_get_compat_id(priv->ioaddr, &priv->compat_id); > > mgr = devm_fpga_mgr_create(dev, "DFL FME FPGA Manager", > &fme_mgr_ops, priv); > diff --git a/drivers/fpga/dfl-fme-region.c b/drivers/fpga/dfl-fme-region.c > index ca7277d3d30a9..d21eacbf2469f 100644 > --- a/drivers/fpga/dfl-fme-region.c > +++ b/drivers/fpga/dfl-fme-region.c > @@ -17,6 +17,7 @@ > #include <linux/fpga/fpga-mgr.h> > #include <linux/fpga/fpga-region.h> > > +#include "dfl.h" > #include "dfl-fme-pr.h" > > static int fme_region_get_bridges(struct fpga_region *region) > @@ -27,8 +28,21 @@ static int fme_region_get_bridges(struct fpga_region > *region) > return fpga_bridge_get_to_list(dev, region->info, ®ion->bridge_list); > } > > +static ssize_t fme_region_compat_id_show(struct fpga_region *region, char > *buf) > +{ > + struct fpga_manager *mgr = region->mgr; > + struct dfl_compat_id compat_id; > + > + fme_mgr_get_compat_id(mgr, &compat_id); It's better to have a common interface, otherwise this region driver depends on one specific mgr driver FPGA_DFL_FME_MGR. Ideally this region driver can be reused with a new fpga mgr driver. Compat id can be per-region or shared one from fpga-mgr. In this hardware, all regions share the same compat_id from fpga-mgr. I think reuse fpga-mgr compatibility id can be done via fpga-mgr data structure or some common API exposed by fpga-mgr. In the first implementation, we added it to fpga-mgr data structure which has less code. Thanks Hao > + > + return sysfs_emit(buf, "%016llx%016llx\n", > + (unsigned long long)compat_id.id_h, > + (unsigned long long)compat_id.id_l); > +} > + > static const struct fpga_region_ops fme_fpga_region_ops = { > .get_bridges = fme_region_get_bridges, > + .compat_id_show = fme_region_compat_id_show, > }; > > static int fme_region_probe(struct platform_device *pdev) > diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h > index 2b82c96ba56c7..a83fd11b390fc 100644 > --- a/drivers/fpga/dfl.h > +++ b/drivers/fpga/dfl.h > @@ -169,6 +169,20 @@ > #define PORT_UINT_CAP_INT_NUM GENMASK_ULL(11, 0) /* Interrupts > num */ > #define PORT_UINT_CAP_FST_VECT GENMASK_ULL(23, 12) /* First Vector > */ > > +/** > + * struct dfl_compat_id - id for compatibility check > + * > + * @id_h: high 64bit of the compat_id > + * @id_l: low 64bit of the compat_id > + */ > +struct dfl_compat_id { > + u64 id_h; > + u64 id_l; > +}; > + > +void fme_mgr_get_compat_id(struct fpga_manager *mgr, > + struct dfl_compat_id *id); > + > /** > * struct dfl_fpga_port_ops - port ops > * > -- > 2.26.3
diff --git a/drivers/fpga/dfl-fme-mgr.c b/drivers/fpga/dfl-fme-mgr.c index d5861d13b3069..cd0b9157ea6e5 100644 --- a/drivers/fpga/dfl-fme-mgr.c +++ b/drivers/fpga/dfl-fme-mgr.c @@ -22,6 +22,7 @@ #include <linux/io-64-nonatomic-lo-hi.h> #include <linux/fpga/fpga-mgr.h> +#include "dfl.h" #include "dfl-fme-pr.h" /* FME Partial Reconfiguration Sub Feature Register Set */ @@ -70,6 +71,7 @@ struct fme_mgr_priv { void __iomem *ioaddr; u64 pr_error; + struct dfl_compat_id compat_id; }; static u64 pr_error_to_mgr_status(u64 err) @@ -272,13 +274,21 @@ static const struct fpga_manager_ops fme_mgr_ops = { .status = fme_mgr_status, }; -static void fme_mgr_get_compat_id(void __iomem *fme_pr, - struct fpga_compat_id *id) +static void _fme_mgr_get_compat_id(void __iomem *fme_pr, + struct dfl_compat_id *id) { id->id_l = readq(fme_pr + FME_PR_INTFC_ID_L); id->id_h = readq(fme_pr + FME_PR_INTFC_ID_H); } +void fme_mgr_get_compat_id(struct fpga_manager *mgr, + struct dfl_compat_id *id) +{ + struct fme_mgr_priv *priv = mgr->priv; + *id = priv->compat_id; +} +EXPORT_SYMBOL_GPL(fme_mgr_get_compat_id); + static int fme_mgr_probe(struct platform_device *pdev) { struct dfl_fme_mgr_pdata *pdata = dev_get_platdata(&pdev->dev); @@ -306,7 +316,7 @@ static int fme_mgr_probe(struct platform_device *pdev) if (!compat_id) return -ENOMEM; - fme_mgr_get_compat_id(priv->ioaddr, compat_id); + _fme_mgr_get_compat_id(priv->ioaddr, &priv->compat_id); mgr = devm_fpga_mgr_create(dev, "DFL FME FPGA Manager", &fme_mgr_ops, priv); diff --git a/drivers/fpga/dfl-fme-region.c b/drivers/fpga/dfl-fme-region.c index ca7277d3d30a9..d21eacbf2469f 100644 --- a/drivers/fpga/dfl-fme-region.c +++ b/drivers/fpga/dfl-fme-region.c @@ -17,6 +17,7 @@ #include <linux/fpga/fpga-mgr.h> #include <linux/fpga/fpga-region.h> +#include "dfl.h" #include "dfl-fme-pr.h" static int fme_region_get_bridges(struct fpga_region *region) @@ -27,8 +28,21 @@ static int fme_region_get_bridges(struct fpga_region *region) return fpga_bridge_get_to_list(dev, region->info, ®ion->bridge_list); } +static ssize_t fme_region_compat_id_show(struct fpga_region *region, char *buf) +{ + struct fpga_manager *mgr = region->mgr; + struct dfl_compat_id compat_id; + + fme_mgr_get_compat_id(mgr, &compat_id); + + return sysfs_emit(buf, "%016llx%016llx\n", + (unsigned long long)compat_id.id_h, + (unsigned long long)compat_id.id_l); +} + static const struct fpga_region_ops fme_fpga_region_ops = { .get_bridges = fme_region_get_bridges, + .compat_id_show = fme_region_compat_id_show, }; static int fme_region_probe(struct platform_device *pdev) diff --git a/drivers/fpga/dfl.h b/drivers/fpga/dfl.h index 2b82c96ba56c7..a83fd11b390fc 100644 --- a/drivers/fpga/dfl.h +++ b/drivers/fpga/dfl.h @@ -169,6 +169,20 @@ #define PORT_UINT_CAP_INT_NUM GENMASK_ULL(11, 0) /* Interrupts num */ #define PORT_UINT_CAP_FST_VECT GENMASK_ULL(23, 12) /* First Vector */ +/** + * struct dfl_compat_id - id for compatibility check + * + * @id_h: high 64bit of the compat_id + * @id_l: low 64bit of the compat_id + */ +struct dfl_compat_id { + u64 id_h; + u64 id_l; +}; + +void fme_mgr_get_compat_id(struct fpga_manager *mgr, + struct dfl_compat_id *id); + /** * struct dfl_fpga_port_ops - port ops *