On Fri, 21 Oct 2022, Ilpo Järvinen wrote:
On Thu, 20 Oct 2022, matthew.gerlach@xxxxxxxxxxxxxxx wrote:
From: Matthew Gerlach <matthew.gerlach@xxxxxxxxxxxxxxx>
Add generic support for MSI-X interrupts for DFL devices.
The location of a feature's registers is explicitly
described in DFHv1 and can be relative to the base of the DFHv1
or an absolute address. Parse the location and pass the information
to DFL driver.
Signed-off-by: Matthew Gerlach <matthew.gerlach@xxxxxxxxxxxxxxx>
---
v4: s/MSIX/MSI_X
move kernel doc to implementation
use structure assignment
fix decode of absolute address
clean up comment in parse_feature_irqs
remove use of csr_res
v3: remove unneeded blank line
use clearer variable name
pass finfo into parse_feature_irqs()
refactor code for better indentation
use switch statement for irq parsing
squash in code parsing register location
v2: fix kernel doc
clarify use of DFH_VERSION field
---
+static int dfh_get_psize(void __iomem *dfh_base, resource_size_t max)
+{
+ int size = 0;
+ u64 v, next;
+
+ if (!FIELD_GET(DFHv1_CSR_SIZE_GRP_HAS_PARAMS,
+ readq(dfh_base + DFHv1_CSR_SIZE_GRP)))
+ return 0;
+
+ while (size + DFHv1_PARAM_HDR < max) {
+ v = readq(dfh_base + DFHv1_PARAM_HDR + size);
+
+ next = FIELD_GET(DFHv1_PARAM_HDR_NEXT_OFFSET, v);
+ if (!(next & ~DFHv1_PARAM_HDR_NEXT_MASK))
In general, try to not use inverse logic for defining masks. However here,
just change DFHv1_PARAM_HDR_NEXT_OFFSET to not include any extra bits
(no rsvd nor eop) and you no longer need this extra masking.
I agree that defining the fields better and using FIELD_GET would make
this code cleaner.
+ return -EINVAL;
+
+ size += next & ~DFHv1_PARAM_HDR_NEXT_MASK;
...Then you can drop this anding too.
+
+ if (next & DFHv1_PARAM_HDR_NEXT_EOL)
Your docs say EOP, but here you use EOL.
Thanks for catching the inconsistency.
Change DFHv1_PARAM_HDR_NEXT_EOL such that this is extracted directly from
v.
--
i.
