On Fri, 30 Dec 2022, Xu Yilun wrote:
On 2022-12-28 at 10:16:23 -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
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.
Signed-off-by: Matthew Gerlach <matthew.gerlach@xxxxxxxxxxxxxxx>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
---
v8: use struct_size() from overflow.h
add dfh_get_u64_param_vals()
Could you help check my comments?
https://lore.kernel.org/linux-fpga/alpine.DEB.2.22.394.2212211421210.570436@rhweight-WRK1/T/#md86e3836130ebacd3c088f5c512ba741aac8a4d1
Sorry I missed your earlier comments. I have since responded to them
specificially.
[...]
+static u64 *find_param(u64 *params, resource_size_t max, int param_id)
+{
+ u64 *end = params + max / sizeof(u64);
+ u64 v, next;
+
+ while (params < end) {
+ v = *params;
+ if (param_id == FIELD_GET(DFHv1_PARAM_HDR_ID, v))
+ return params;
+
+ next = FIELD_GET(DFHv1_PARAM_HDR_NEXT_OFFSET, v);
+ params += next;
+ if (FIELD_GET(DFHv1_PARAM_HDR_NEXT_EOP, v))
+ break;
+ }
+
+ return NULL;
+}
+
+/**
+ * dfh_find_param() - find parameter block for the given parameter id
+ * @dfl_dev: dfl device
+ * @param: id of dfl parameter
+ *
+ * Return: pointer to start of parameter block, NULL otherwise.
+ */
+u64 *dfh_find_param(struct dfl_device *dfl_dev, int param_id)
+{
+ return find_param(dfl_dev->params, dfl_dev->param_size, param_id);
+}
+EXPORT_SYMBOL_GPL(dfh_find_param);
Didn't find where to use it?
I understand. Don't export a function unless there is a comsumer for it.
+
+/**
+ * dfh_get_u64_param_vals() - get array of u64 param values for given parameter id
There is no rule to say one u64 for each property in the parameter block.
So I don't see the reason for DFL core to provide u64 array for the API,
And the size of the parameter block is decided by HW, why make the user
input the value?
As we discussed before, dfl core doesn't try to look into the parameter
block. So please just provide the const void *data & data_size for drivers.
This is the most common way to represent a data block.
I will move the parameter parsing helper function to the driver itself.
Thanks for the feedback,
Matthew Gerlach
Thanks,
Yilun
+ * @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);
+
+ for (i = 0; i < nvals; i++)
+ *pval++ = param[i + 1];
+
+ return param;
+}
+EXPORT_SYMBOL_GPL(dfh_get_u64_param_vals);
+
