Re: change MSI/MSI-X APIs

[Kengo NAKAHARA <k-nakahara%iij.ad.jp@localhost>]

Hi

On 2015/05/11 23:18, Christos Zoulas wrote:
> In article <555048FD.6020001%iij.ad.jp@localhost>,
> Kengo NAKAHARA  <k-nakahara%iij.ad.jp@localhost> wrote:
>> I received feedback from some device driver authors. They point out
>> establish, disestablish and release APIs should be unified for INTx,
>> MSI and MSI-X. So, I would change the APIs as below:
>>
>> before                  | after (unify to)
>> -------------------------+-------------------------
>> pci_intr_establish()    | 
>> pci_msi_establish()     | pci_intr_establish()
>> pci_msix_establish()    |
>> -------------------------+-------------------------
>> pci_intr_disestablish() | 
>> pci_msi_disestablish()  | pci_intr_disestablish()
>> pci_msix_disestablish() |
>> -------------------------+-------------------------
>> pci_intx_release()      | 
>> pci_msi_release()       | pci_intr_release()
>> pci_msix_release()      |
>> # In contrast, alloc APIs are not changed.
>>
>> Here is the above modification diff.
>>    http://www.netbsd.org/~knakahara/unify-msi-apis/unify-msi-apis.diff
>>
>> If there is no objection, I commit it in a few days.
>>
>> Could you comment this modification?
> 
> I like it, but why can't we merge the allocations too?
> 
> Can't we have a:
> 
> 	int pci_intr_map(struct pci_attach_args *pa, pci_intr_handle_t *ih);
> 
> for the drivers that have only one interrupt, and:
> 
> 	int pci_intr_vector_map(struct pci_attach_args *pa,
> 	    pci_intr_handle_t **ihp, size_t *nih);
> 
> for drivers that can handle more than one?

Because some device drivers must select INTx, MSI, or MSI-X depending
on using devices; e.g. some devices have errata about MSI-X,
so the device driver cannot use MSI-X even if the devices have
MSI-X capability. In addition, some other the device drivers should
use MSI-X and INTx only since some devices have errata about MSI.
In a similar way, other device drivers should use MSI-X and MSI only.

These selection logic codes can be done only in the device drivers
codes, therefore it is required for the device drivers to be able to
select interrupt types by allocation APIs.

> Also can we add the disestablish/free code in the driver? What are the
> steps to undo the interrupt allocation/establishment?

Here is the example code of if_wm:
    http://www.netbsd.org/~knakahara/unify-msi-apis/if_wm-example.diff

This diff includes pci_intr_disestablish()/pci_intr_release() and
pci_{intx,msi,msix}_alloc()/pci_intr_establish().
How about this sample code?


Thanks,

-- 
//////////////////////////////////////////////////////////////////////
Internet Initiative Japan Inc.

Device Engineering Section,
Core Product Development Department,
Product Division,
Technology Unit

Kengo NAKAHARA <k-nakahara%iij.ad.jp@localhost>