On 07/25/2012 05:12 AM, Don Dutile wrote:
> On 07/24/2012 12:31 PM, Jiang Liu wrote:
>> +int pci_pcie_capability_read_word(struct pci_dev *dev, int pos, u16 *val)
>> +{
>> + int ret = 0;
>> +
>> + *val = 0;
>> + if (pos& 1)
>> + return -EINVAL;
>> +
>> + if (pci_pcie_capability_reg_implemented(dev, pos)) {
>> + ret = pci_read_config_word(dev, pci_pcie_cap(dev) + pos, val);
>> + /*
>> + * Reset *val to 0 if pci_read_config_word() fails, it may
>> + * have been written as 0xFFFF if hardware error happens
>> + * during pci_read_config_word().
>> + */
>> + if (ret)
>> + *val = 0;
>> + } else if (pos == PCI_EXP_SLTSTA&&
>> + pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM) {
>> + *val = PCI_EXP_SLTSTA_PDS;
>> + }
> Don't you want the above if check done 1st, and not the
> pci_pcie_capability_reg_implemented(dev, pos) check ?
> Isn't PCI_EXP_SLTCTL an implemented register, looking at this snippet?:
>> + switch (pos) {
> <snip>
>> + case PCI_EXP_SLTCAP:
>> + case PCI_EXP_SLTCTL:
>> + case PCI_EXP_SLTSTA:
>> + return pci_pcie_cap_has_sltctl(dev);
> and the above function is:
>
>> +static inline bool pci_pcie_cap_has_sltctl(const struct pci_dev *dev)
>> +{
>> + int type = pci_pcie_type(dev);
>> +
>> + return pci_pcie_cap_version(dev)> 1 ||
>> + type == PCI_EXP_TYPE_ROOT_PORT ||
>> + (type == PCI_EXP_TYPE_DOWNSTREAM&&
>> + dev->pcie_flags_reg& PCI_EXP_FLAGS_SLOT);
>> +}
>
> or is PCI_EXP_FLAGS_SLOT not set when it should be, then the first condition
> fails, and this function forces the val to 1b ?
Hi Don,
Yes, that's the purpose. PCIe spec v2/v3 defines that hardware should return
a value with bit PCI_EXP_SLTSTA_PDS set if PCI_EXP_SLTSTA register is not implemented.
So for PCIe v1 hardwares, we try to behave in the same way as v2/v3.
Regards!
Gerry
--
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
