On Wed, Sep 14, 2011 at 6:15 AM, Benjamin Herrenschmidt
<benh@xxxxxxxxxxxxxxxxxxx> wrote:
> On Tue, 2011-09-13 at 23:55 -0500, Jon Mason wrote:
>> Rework the "performance" MPS option to configure the device MPS with the
>> smaller of the device MPSS or the bridge MPS (which is assumed to be
>> properly configured at this point to the largest allowable MPS based on
>> its parent bus).
>>
>> Also, rework the MRRS setting to report an inability to set the MRRS to
>> a valid setting.
>>
>> Finally, rework the pcie_bus_configure_settings to not include an MPS
>> suggestion from the root port and simply use the root port MPSS as a
>> starting point (thus removing the need to pass around the info and
>> making the call simplier).
>>
>> Signed-off-by: Jon Mason <mason@xxxxxxxx>
>> ---
>
> A comment here:
>
>> static int pcie_bus_configure_set(struct pci_dev *dev, void *data)
>> @@ -1456,13 +1456,15 @@ static int pcie_bus_configure_set(struct pci_dev *dev, void *data)
>> * parents then children fashion. If this changes, then this code will not
>> * work as designed.
>> */
>> -void pcie_bus_configure_settings(struct pci_bus *bus, u8 mpss)
>> +void pcie_bus_configure_settings(struct pci_bus *bus)
>> {
>> - u8 smpss = mpss;
>> + u8 smpss;
>>
>> - if (!pci_is_pcie(bus->self))
>> + if (!bus || !bus->self || !pci_is_pcie(bus->self))
>> return;
>
> So basically this says I shouldn't call this for my top level bus (my
> root bus) but I must go down one level to pass the bus that is below the
> root complex right ?
>
> IE, my arch code can't just do
>
> pcie_bus_configure_settings(host->bus);
>
> "host" is my PCI host bridge data structure which contains a "bus"
> pointer to the top-level bus, which has no bus->self
>
> But instead needs to iterate all children of host->bus (in case there
> are several RC's off the top level bus) and call
> pcie_bus_configure_settings() on each of them.
>
> I find that a bit awkward but if it's how that is intended, then
> I'll modify my code accordingly.
The issue with calling pcie_bus_configure_settings directly with the
root bus is the "safe" option. Doing so would make the MPS uniform
from the root bus level instead of the root port level, thus making
the system-wide MPS the smallest of the root complex instead of the
smallest on the root port fabric. However, since every arch is most
likely going to have to iterate over the children of the root bus, it
might make sense to have a helper function do that.
>
> Cheers,
> Ben.
>
>> + smpss = bus->self->pcie_mpss;
>> +
>> if (pcie_bus_config == PCIE_BUS_SAFE) {
>> pcie_find_smpss(bus->self, &smpss);
>> pci_walk_bus(bus, pcie_find_smpss, &smpss);
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index 8c230cb..6db35bf 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -618,7 +618,7 @@ struct pci_driver {
>> /* these external functions are only available when PCI support is enabled */
>> #ifdef CONFIG_PCI
>>
>> -extern void pcie_bus_configure_settings(struct pci_bus *bus, u8 smpss);
>> +extern void pcie_bus_configure_settings(struct pci_bus *bus);
>>
>> enum pcie_bus_config_types {
>> PCIE_BUS_PERFORMANCE,
>
>
>
--
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
