Re: [PATCH v7 3/4] fpga: dfl: add basic support for DFHv1

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 2587 bytes --]

On Tue, 20 Dec 2022, matthew.gerlach@linux.intel.com wrote:

> From: Matthew Gerlach <matthew.gerlach@linux.intel.com>
> 
> 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@linux.intel.com>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
> v7: no change
> 
> v6: move MSI_X parameter definitions to drivers/fpga/dfl.h
> 
> v5: update field names
>     fix find_param/dfh_get_psize
>     clean up mmio_res assignments
>     use u64* instead of void*
>     use FIELD_GET instead of masking
> 
> 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 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 data for the given parameter id
> + * @dfl_dev: dfl device
> + * @param: id of dfl parameter
> + *
> + * Return: pointer to parameter header on success, 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);

BTW, should there be a way for the caller to ensure the parameter is long 
enough?

All callers probably want to ensure the length of the parameter is valid 
so it would perhaps make sense to add a parameter for the required 
(minimum) length?


-- 
 i.