Question regarding pci_request_region()

[Johannes Thumshirn]

Hi,

I've some questions regarding pci_request_region(). Is there a similar function
that allows me to request the memory from a PCIe device, but not a whole BAR?

in drivers/mcb/mcb-pci.c I do a pci_request_region() for BAR 0. But I only need
the first 0x200 bytes. pci_request_region() locks the memory and thus probing of
mcb attached sub devices fails, as they can't do a request_mem on their part of
the PCI memory space until the mcb parser is done and releases BAR0.

Generally this is no problem when you build all drivers as modules, but once I
do build in drivers, probing fails. Before rewriting all mcb based drivers to
use deferred probing I wanted to ask if there is a more clean way to do this.

Thanks in advance,
Johannes

[Bjorn Helgaas]

On Tue, Oct 21, 2014 at 8:37 AM, Johannes Thumshirn

Post by Johannes Thumshirn
Hi,
I've some questions regarding pci_request_region(). Is there a similar function
that allows me to request the memory from a PCIe device, but not a whole BAR?

You can always do something like this:

start = pci_resource_start(dev, 0);
request_mem_region(start, 0x200, KBUILD_MODNAME);
ioremap(start, 0x200);

Post by Johannes Thumshirn
in drivers/mcb/mcb-pci.c I do a pci_request_region() for BAR 0. But I only need
the first 0x200 bytes. pci_request_region() locks the memory and thus probing of
mcb attached sub devices fails, as they can't do a request_mem on their part of
the PCI memory space until the mcb parser is done and releases BAR0.

The default assumption is that the driver bound to a PCI device is the
only user of the BARs. For example, if there is no driver bound to a
device, the PCI core assumes that it can reassign the BARs.

You can carve up a BAR among sub-devices, but you should make sure
that some driver remains bound to the PCI device as long as any
sub-driver is active. The top-level driver doesn't need to keep any
BARs mapped itself, but it should remain bound to the device.

It looks like mcb_pci_probe() disables the device before it returns.
That doesn't seem like a good idea if you have sub-devices that are
accessed via the device's BARs. At a minimum, pci_disable_device()
turns off bus mastering, and it may also disable IRQs

Post by Johannes Thumshirn
Generally this is no problem when you build all drivers as modules, but once I
do build in drivers, probing fails. Before rewriting all mcb based drivers to
use deferred probing I wanted to ask if there is a more clean way to do this.

Deferred probing sounds like a workaround for a deeper problem. It
seems like you basically have a PCI device that is effectively a
bridge to a MEN Chameleon Bus. That means than when you enumerate
such a device, the probe routine (mcb_pci_probe(), I think) should in
turn enumerate any MCB devices "behind" it, i.e., the sub-devices.

There should not be any ordering dependency between mcb_pci_probe()
and the module_init functions of the drivers. It is perfectly legal
for a driver to register before there are any devices for it to bind
to. If a driver like men_z135_uart.c initializes first, it should
register itself as an MCB driver, but its probe function won't be
called because there's no matching device yet. When mcb_pci_probe()
runs later to claim the MCB "bridge", it should enumerate the MCB
devices and add them. When the devices are added, we should check for
any already-registered drivers that match them and call their probe
functions at that time.

Bjorn

[Johannes Thumshirn]

Post by Bjorn Helgaas
On Tue, Oct 21, 2014 at 8:37 AM, Johannes Thumshirn

Post by Johannes Thumshirn
Hi,
I've some questions regarding pci_request_region(). Is there a similar function
that allows me to request the memory from a PCIe device, but not a whole BAR?

start = pci_resource_start(dev, 0);
request_mem_region(start, 0x200, KBUILD_MODNAME);
ioremap(start, 0x200);

Post by Johannes Thumshirn
in drivers/mcb/mcb-pci.c I do a pci_request_region() for BAR 0. But I only need
the first 0x200 bytes. pci_request_region() locks the memory and thus probing of
mcb attached sub devices fails, as they can't do a request_mem on their part of
the PCI memory space until the mcb parser is done and releases BAR0.

The default assumption is that the driver bound to a PCI device is the
only user of the BARs. For example, if there is no driver bound to a
device, the PCI core assumes that it can reassign the BARs.
You can carve up a BAR among sub-devices, but you should make sure
that some driver remains bound to the PCI device as long as any
sub-driver is active. The top-level driver doesn't need to keep any
BARs mapped itself, but it should remain bound to the device.
It looks like mcb_pci_probe() disables the device before it returns.
That doesn't seem like a good idea if you have sub-devices that are
accessed via the device's BARs. At a minimum, pci_disable_device()
turns off bus mastering, and it may also disable IRQs

Post by Johannes Thumshirn
Generally this is no problem when you build all drivers as modules, but once I
do build in drivers, probing fails. Before rewriting all mcb based drivers to
use deferred probing I wanted to ask if there is a more clean way to do this.

Deferred probing sounds like a workaround for a deeper problem. It
seems like you basically have a PCI device that is effectively a
bridge to a MEN Chameleon Bus. That means than when you enumerate
such a device, the probe routine (mcb_pci_probe(), I think) should in
turn enumerate any MCB devices "behind" it, i.e., the sub-devices.
There should not be any ordering dependency between mcb_pci_probe()
and the module_init functions of the drivers. It is perfectly legal
for a driver to register before there are any devices for it to bind
to. If a driver like men_z135_uart.c initializes first, it should
register itself as an MCB driver, but its probe function won't be
called because there's no matching device yet. When mcb_pci_probe()
runs later to claim the MCB "bridge", it should enumerate the MCB
devices and add them. When the devices are added, we should check for
any already-registered drivers that match them and call their probe
functions at that time.
Bjorn

Hi Bjorn,

Thanks for the info, I'll do a patch based on your suggestions.

Thanks,
Johannes