On Fri, 23 Sep 2022, Ilpo Järvinen wrote:
On Fri, 23 Sep 2022, matthew.gerlach@xxxxxxxxxxxxxxx wrote:From: Matthew Gerlach <matthew.gerlach@xxxxxxxxxxxxxxx> Define and use a DFHv1 parameter to add generic support for MSIX interrupts for DFL devices. Signed-off-by: Matthew Gerlach <matthew.gerlach@xxxxxxxxxxxxxxx> --- v2: fix kernel doc clarify use of DFH_VERSION field --- drivers/fpga/dfl.c | 60 +++++++++++++++++++++++++++++++++++++++++---- include/linux/dfl.h | 14 +++++++++++ 2 files changed, 69 insertions(+), 5 deletions(-) diff --git a/drivers/fpga/dfl.c b/drivers/fpga/dfl.c index 1132f3c10440..dfd3f563c92d 100644 --- a/drivers/fpga/dfl.c +++ b/drivers/fpga/dfl.c @@ -941,23 +941,22 @@ static int parse_feature_irqs(struct build_feature_devs_info *binfo, void __iomem *base = binfo->ioaddr + ofst; unsigned int i, ibase, inr = 0; enum dfl_id_type type; - int virq; + int virq, off; u64 v; type = 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 + * current version, DFHv0, only provides mmio resources information for * each feature in DFL Header, no field for interrupt resources. * Interrupt resource information is provided by specific mmio * registers of each private feature which supports interrupt. So in * order to parse and assign irq resources, DFL framework has to look * into specific capability registers of these private features. * - * Once future DFL version supports generic interrupt resource - * information in common DFL headers, the generic interrupt parsing - * code will be added. But in order to be compatible to old version + * DFHv1 supports generic interrupt resource information in DFHv1 + * parameter blocks. But in order to be compatible to old version * DFL, the driver may still fall back to these quirks. */ if (type == PORT_ID) { @@ -981,6 +980,36 @@ static int parse_feature_irqs(struct build_feature_devs_info *binfo, } } + if (fid != FEATURE_ID_AFU && fid != PORT_FEATURE_ID_ERROR && + fid != PORT_FEATURE_ID_UINT && fid != FME_FEATURE_ID_GLOBAL_ERR) { + + v = FIELD_GET(DFH_VERSION, readq(base));I'd call this variable version (or ver) if you want to store it but it would also fit to switch () line so that no extra variable is needed.
I will change the v to dfh_ver to be clearer. I want to store the value because it is used in the default case in the error message. The error message helps to debug broken FPGA images.
+ switch (v) { + case 0: + break; + + case 1: + v = readq(base + DFHv1_CSR_SIZE_GRP);Extra space.
I will remove extra space.
+ if (FIELD_GET(DFHv1_CSR_SIZE_GRP_HAS_PARAMS, v)) { + off = dfl_find_param(base + DFHv1_PARAM_HDR, ofst, + DFHv1_PARAM_ID_MSIX); + if (off >= 0) {I'd reverse these 2 conditions and break when there's nothing to do.
I'm not sure what you mean by reversing these conditions because a DFHv1 may or may not have parameters (the first condition), and a DFHv1 may have
parameters but may not have a MSI-X parameter (the second condition).
+ ibase = readl(base + DFHv1_PARAM_HDR + + off + DFHv1_PARAM_MSIX_STARTV); + inr = readl(base + DFHv1_PARAM_HDR + + off + DFHv1_PARAM_MSIX_NUMV); + dev_dbg(binfo->dev, "start %d num %d fid 0x%x\n", + ibase, inr, fid); + } + } + break; + + default: + dev_warn(binfo->dev, "unexpected DFH version %lld\n", v); + break; + } + } + if (!inr) { *irq_base = 0; *nr_irqs = 0;-- i.
