On Thu, 29 Dec 2022, Andy Shevchenko wrote:
On Wed, Dec 28, 2022 at 10:16:23AM -0800, matthew.gerlach@xxxxxxxxxxxxxxx wrote:
From: Matthew Gerlach <matthew.gerlach@xxxxxxxxxxxxxxx>
Version 1 of the Device Feature Header (DFH) definition adds
functionality to the DFL bus.
A DFHv1 header may have one or more parameter blocks that
further describes the HW to SW. Add support to the DFL bus
Single space is enough.
Yes, single space is enough. Two spaces after a period is hard habit for
me break. I will update in v9.
to parse the MSI-X parameter.
The location of a feature's register set 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.
I'm wondering what DFL states for.
I will define DFL in the commit message like DFH in the next revision.
...
+/**
+ * dfh_get_u64_param_vals() - get array of u64 param values for given parameter id
+ * @dfl_dev: dfl device
+ * @param: id of dfl parameter
+ * @pval: location of parameter data destination
+ * @nvals: number of u64 elements of parameter data
+ *
+ * Return: pointer to start of parameter block, PTR_ERR otherwise
+ */
+u64 *dfh_get_u64_param_vals(struct dfl_device *dfl_dev, int param_id, u64 *pval, int nvals)
+{
+ u64 *param = find_param(dfl_dev->params, dfl_dev->param_size, param_id);
+ u64 next;
+ int i;
+
+ if (!param)
+ return ERR_PTR(-ENOENT);
+
+ next = FIELD_GET(DFHv1_PARAM_HDR_NEXT_OFFSET, *param);
+
+ if (nvals >= next)
+ return ERR_PTR(-ENOMEM);
ENODATA ?
ENODATA does seem to be more accurate than ENOMEM in this case.
+ for (i = 0; i < nvals; i++)
+ *pval++ = param[i + 1];
memcpy() ?
Using memcpy() will make code cleaner.
+ return param;
+}
...
+ finfo = kzalloc(struct_size(finfo, params, dfh_psize/sizeof(u64)), GFP_KERNEL);
' / ' (mind the spaces)
Yes, I will mind the spaces.
Also, perhaps better to use sizeof(*params) or what is the member of that
structure. So it will be more robust against possible changes.
params is the name of the structure member that is the trailing array.
if (!finfo)
return -ENOMEM;
--
With Best Regards,
Andy Shevchenko
Thanks for the feedback,
Matthew Gerlach
