Do we need to have the defines for inline functions? I thought in that case we could just use the original names directly?

long line.

Is there are reason this is not an inline function?

Given the two blocks from malloc() down are the same apart from "devfn", can they be one internal function or two as the lower half is another copy from linux_pci_attach_device() if I am not mistaken?

I was looking at that for linux_pci_attach_device() yesterday as for the dev the release function doesn't exist and it's highly confusing how the pci or the dev get allocated and the naming of things. So I am happy the less duplicated "alloc" code we have.

Not need to do, if you don't want to, just a suggestion in this case.

I am still pondering if this will work in all cases; I'll go and try to try/check tonight.

Nope, this was a copy paste from drm-kmod but this should be directly using the linux name. Will change that.

Guess it could be. Will change.

Yeah I can do a generic alloc function.

Why will it not work in some case ?

Because from what I can see on network drivers pci_dev_get() and pci_dev_put() are reference operations, I'd assume around the device and not direct operations. I also see pdev accesses after pci_dev_put() so it cannot be freed at that point:

iwlwifi/pcie/trans.c::iwl_trans_pcie_removal_wk()
        pci_dev_put(pdev);
        pci_stop_and_remove_bus_device(pdev);

It's the only call I have there so I had ignored it so far.

I assume the above just works for you in your special case.

I further assume the normal implementation would be similar to device_get/put():

static __inline void /* XXX-BZ returns something? */
pci_dev_get(struct pci_dev *pdev)
{
    if (pdev != NULL)
        device_get(&pdev->dev);
}

static __inline void
pci_dev_put(struct pci_dev *pdev)
{
        if (pdev != NULL)
           device_put(&pdev->dev);
}

I believe to achieve what you seem to want/need you'd want to do the following:

Rename this function to

static void
linuxkpi_pci_release_direct(struct device *dev)
(
... fix/cast dev to pdev
}

and move it up before your two new linuxkpi_pci_get_* functions or a new alloc function; there for the struct pci_dev *dev you want to set:

pdev->dev.release = linuxkpi_pci_release_direct;

But that means that you probably also have to set the parent and init the kobj in your two get_* or alloc functions above as does linux_pci_attach_device(). That means embracing the struct device which embedded in pci_dev as first argument rather than working around it. And you don't have to use it much more than that I'd assume.

I haven't tried this; only put it down from the top of my head.

PS: yes, linux_pci_attach_device() also lacks a release function for devres; this is how I got to figure that out yesterday.

Yeah I had a look at what linux is doing and you're right, it's just device_put.
I'll see at how to fix that.

Well no it's needed as we have the same functions defined in pcivar.h so I'll just rename them to linuxkpi_

This doesn't look right. I think you need a temporary uint32_t variable before storing into uint16_t.

Ditto below.

This breaks for ARI devices. In particular, PCI_FUNC() is broken, it truncates the function address.

Arguably it is more an issue with PCI_FUNC() macro.

Yes this only work on little-endian.

Same.

What's ARI devices ?

Right ok, will fix.
This is the only part not tested as it's only used by the radeon and I don't have such device.

ARI is PCIe cap which indicates that instead of using the second byte of the device address for slot:func, whole second byte is dedicated to the function number. This way device might have 255 siblings instead of 31, which is useful/needed for things like SRIOV.

These definitions do no longer match what Linux defines them to be.
Are you sure this is OK?

See the mlx4 driver for example, how it assumes how the current macros work.

static int extended_func_num(struct pci_dev *pdev)
{
        return PCI_SLOT(pdev->devfn) * 8 + PCI_FUNC(pdev->devfn);
}

It doesn't matter as we have setup the devfn with the new macro.

This does not work with ARI.

Ok, how so ?

This does not work with non-ARI devices.

I see, you changed PCI_SLOT()/PCI_FUNC() definitions. But now PCI_SLOT() would break
for non-ARI.

With this KPI, I believe you must use PCIB_ARI_ENABLED() to get it right.

Is there a reason we do lose the & 0x07 on the (func) ?

Can we make the macros arguments for both of these also pdev instead dev to avoid possible confusion?

return (pbus->domain);

It is now initialised in lkpifill_pci_dev() so should be done for all paths, so we can simply return it.

For ARI devices. slot is zero and function takes all lower byte. So these definitions are compatible both with usual (legacy bleh) addressing and with ARI.

True, will change.

So this is still not done ?

Style: 4 spaces indent for continuation line.

I actually do not understand this function. What devfn argument is for ?

I think you should use PCI_GET_ID() there instead of PCI_DEVFN.

Style: space around binary op (' | ').

I suggest to add assert that returned device indeed has slot/function number as requested. This should simplify further debugging for some cases.