Messages in this thread | | | Date | Mon, 26 Sep 2022 08:13:10 -0700 (PDT) | From | matthew.gerlach@linux ... | Subject | Re: [PATCH v2 4/6] fpga: dfl: add generic support for MSIX interrupts |
| |
On Fri, 23 Sep 2022, Andy Shevchenko wrote:
> On Fri, Sep 23, 2022 at 05:17:43AM -0700, matthew.gerlach@linux.intel.com wrote: >> From: Matthew Gerlach <matthew.gerlach@linux.intel.com> >> >> Define and use a DFHv1 parameter to add generic support for MSIX >> interrupts for DFL devices. > > ... > >> + if (fid != FEATURE_ID_AFU && fid != PORT_FEATURE_ID_ERROR && >> + fid != PORT_FEATURE_ID_UINT && fid != FME_FEATURE_ID_GLOBAL_ERR) { > >> + > > Unneeded blank line.
I will remove the blank line.
> >> + v = FIELD_GET(DFH_VERSION, readq(base)); >> + switch (v) { > > This v... > >> + case 0: >> + break; >> + >> + case 1: >> + v = readq(base + DFHv1_CSR_SIZE_GRP); > > Extra space. > > ...and this v are semantically different. It's quite hard to deduce their > semantics.
I was trying to be consistent with the existing code where the read was stored in a temporary variable, v, and the FIELD_GET would be used for the specific field. Will it be sufficiently clear if the v used above is changed to dfl_ver, and this use of v followed by FIELD_GET remains as is?
> >> + if (FIELD_GET(DFHv1_CSR_SIZE_GRP_HAS_PARAMS, v)) { >> + off = dfl_find_param(base + DFHv1_PARAM_HDR, ofst, >> + DFHv1_PARAM_ID_MSIX); > > I guess I have suggested to use temporary variable(s) here. > > void __iomem *dfhv1 = base + DFHv1...; > void __iomem *nth; > >> + if (off >= 0) { > > nth = dfhv1 + off; > >> + ibase = readl(base + DFHv1_PARAM_HDR + >> + off + DFHv1_PARAM_MSIX_STARTV); >> + inr = readl(base + DFHv1_PARAM_HDR + >> + off + DFHv1_PARAM_MSIX_NUMV); > > ibase = readl(nth + DFHv1_PARAM_MSIX_STARTV); > inr = readl(nth + 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; >> + } >> + } > > -- > With Best Regards, > Andy Shevchenko > > >
| |