On Thu, Oct 31, 2013 at 08:44:36PM -0600, Bjorn Helgaas wrote:
> You obviously understand all this, so sorry for the repetition; I
> guess I'm just trying to get it clear in my own mind :)
Ditto, it is tricky. When I first looked at that code I thought it was
trying to do something else..
> > How about this instead:
> >
> > if (!io) {
> > /* Disable the IO window by setting limit < base */
> > pci_write_config_word(bridge, PCI_IO_BASE, 0x00f0);
> > pci_read_config_word(bridge, PCI_IO_BASE, &io);
> > }
> > /* Bridges without IO support must wire the IO register to 0 */
> > if (io)
> > b_res[0].flags |= IORESOURCE_IO;
>
> If an I/O window happened to be configured to the 4K region at 0x0,
> this disables it. An I/O window configured anywhere else is left
> enabled. Previously the only effect of the function was to set bits
> in b_res[].flags; with this change it would also sometimes disable an
> I/O window. That seems worse to me. Am I just misunderstanding the
> problem you're solving?
No, you've got it right. The problem I had is completely solved by
PATCH 1/2 PCI: mvebu - The bridge should obey the MEM and IO command
bits
But the code looks odd to me.
It is certainly correct as written if the check is only ever called
with PCI_COMMAND_IO cleared.
However, I am having trouble convincing myself of this. Notably
pci_setup_bridge does not seem to make that assumption.
If PCI_COMMAND_IO is set, it can just assume IORESOURCE_IO and skip
the testing of the PCI_IO_BASE register.
Why 0xf0f0 anyhow? :)
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
