lkml.org 
[lkml]   [2016]   [Jul]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 07/13] pci: Provide sensible irq vector alloc/free routines
    On Wed, Jul 06, 2016 at 10:05:45AM +0200, Alexander Gordeev wrote:
    > > + pci_enable_msi, pci_enable_msi_range, pci_enable_msi_exact, pci_disable_msi,
    > > + pci_msi_vec_count, pci_enable_msix_range, pci_enable_msix_exact,
    > > + pci_disable_msix, pci_msix_vec_count
    >
    > Description of these functions can be removed when all drivers migrated
    > to the new API. Also implementation descriptions + examples would still
    > be needed AFAICT.

    I diagreed - if we deprecated functions the only thing that should
    be mentioned is a "don't use these".

    > This function's code almost matches the existing pci_enable_msix_range()
    > so pci_enable_msix_range() should be reworked instead IMHO.

    That's what earlier versions of the code did. However due to the
    fact that we want to avoid over-allocating the msix_vectors array
    (minor) and get the vectors count of the affinity mask right (major,
    as pointed out by you last time) I had to move the allocations inside
    the helpers that loop around the atctual enablement. I didn't want
    to change the function to a different version of the algorithm just
    before removing them relatively soon. But given that strong preference
    for changing these simple functions instead of duplicating them I've
    changed that patch to do that now.

    > We do not need to keep msix_entry array, since it only needed for
    > pci_irq_vector() function. But the same info could be retrieved from
    > msi_desc::irq.

    Indeed. Avoiding this allocation makes these interfaces quite a bit
    simpler. It requires a few prep patches, but I think it's definitively
    worth, so the next version will avoid the need for the msix_entry array.

    > > + /* use legacy irq if allowed */
    > > + if (min_vecs == 1)
    > > + return 1;
    > > + return -ENOSPC;
    >
    > The original error code (in vecs) would be overridden with -ENOSPC here.

    Ok, fixed.

    > > + WARN_ON_ONCE(!dev->msi_enabled && nr > 0);
    > > + return dev->irq + nr;
    >
    > I think this function should check irq number existence and return the
    > vector number or -EINVAL;

    Ok, fixed.

    > > + unsigned int flags)
    > > +{
    > > + if (min_vecs > 1)
    > > + return -ENOSPC;
    >
    > In case CONFIG_PCI_MSI is unset min_vecs > 1 is -EINVAL;

    Ok, fixed.

    \
     
     \ /
      Last update: 2016-07-10 06:21    [W:4.242 / U:0.896 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site