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

matthew.gerlach
2022-11-01T22:38:34Z

On Sat, 29 Oct 2022, matthew.gerlach@linux.intel.com wrote:

>
>>
>>> if (!finfo)
>>> return -ENOMEM;
>>>
>>> + if (dfh_psize > 0) {
>>> + memcpy_fromio(finfo->params,
>>> + binfo->ioaddr + ofst + DFHv1_PARAM_HDR,
>>> dfh_psize);
>>> + finfo->param_size = dfh_psize;
>>> + }
>>> +
>>> finfo->fid = fid;
>>> finfo->revision = revision;
>>> - finfo->mmio_res.start = binfo->start + ofst;
>>> - finfo->mmio_res.end = finfo->mmio_res.start + size - 1;
>>> + finfo->dfh_version = dfh_ver;
>>> finfo->mmio_res.flags = IORESOURCE_MEM;
>>> - finfo->irq_base = irq_base;
>>> - finfo->nr_irqs = nr_irqs;
>>> + if (dfh_ver == 1) {
>>> + v = readq(binfo->ioaddr + ofst + DFHv1_CSR_ADDR);
>>> + if (v & DFHv1_CSR_ADDR_REL)
>>> + finfo->mmio_res.start = v & ~DFHv1_CSR_ADDR_REL;
>>> + else
>>> + finfo->mmio_res.start = binfo->start + ofst +
>>> + FIELD_GET(DFHv1_CSR_ADDR_MASK,
>>> v);
>>> +
>>> + v = readq(binfo->ioaddr + ofst + DFHv1_CSR_SIZE_GRP);
>>> + finfo->mmio_res.end = finfo->mmio_res.start +
>>> + FIELD_GET(DFHv1_CSR_SIZE_GRP_SIZE, v) -
>>> 1;
>>
>> So for dflv1, no feature header resource for dfl_device, is it a problem
>> for dfl_uio? Does userspace driver need the raw feature header?
> These are two very good questions. The dfl_uio driver question is
> particularly relevent because user space is looking at the GUIDs.
>

In the case of dfl_uio driver, user space will definitely want to look at
the feature header for the GUID and the parameters. Since DFHv1 can have
the DFH header and the feature registers in non-contiguous memory
locations, a resource for the dfl_device will be required. In earlier
revisions of this patch set, a second resource was added called csr_res
pointing to the feature's register while mmio_res pointed at the header.
Do we just need better names or do we need an array of named resources?

>>
>>> + } else {
>>> + finfo->mmio_res.start = binfo->start + ofst;
>>> + finfo->mmio_res.end = finfo->mmio_res.start + size - 1;
>>> + }
>>> +
>>> + ret = parse_feature_irqs(binfo, ofst, finfo);
>>> + if (ret) {
>>> + kfree(finfo);
>>> + return ret;
>>> + }
>>>