PCI: correctly check availability of PCIe Link Cap/Status/Control registers

Commit Message

Jiang Liu Aug. 27, 2013, 4:44 p.m. UTC

From: Jiang Liu <[e-mail]>

PCIe Base Spec 1.0 Section 5.8 states that "The PCI Express Capabilities,
Device Capabilities, Device Status/Control, Link Capabilities and
Link Status/Control registers are required for all PCI Express devices."

And PCIe Base Spec 3.0 Section 7.8 states that "The Link Capabilities,
Link Status, and Link Control registers are required for all Root Ports,
Switch Ports, Bridges, and Endpoints that are not Root Complex Integrated
Endpoints."

Changeset 1b6b8ce2ac372e "PCI: only save/restore existent registers in the
PCIe capability" introduces pcie_cap_has_lnkctl(), which assumes:
1) PCIe root port, endpoint and legacy endpoint with 1.0 PCIe Capability
   implement Link Cap/Status/Control registers.
2) All PCIe devices with 2.0 PCIe Capability implemnet Link Cap/Status/
   Control registers.

The above assumptions don't conform to PCIe base specifications, so change
pcie_cap_has_lnkctl() to follow PCIe specifications.

Signed-off-by: Jiang Liu <[e-mail]>
---
Hi Bjorn,
	Yuval has a PCIe 1.0 swither, so triggers the bug.  I'm not sure
why Yu Zhao has implemented pcie_cap_has_linctl() in that way, any special
consideration here?

Hi Yuval,
	Could you please help to test this patch? Due to hardware resource
constraints, I have just compiled the patch on my laptop without testing.

Thanks!

---
 drivers/pci/access.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

Comments

Bjorn Helgaas Aug. 27, 2013, 5:07 p.m. UTC | #1

On Tue, Aug 27, 2013 at 10:44 AM, Jiang Liu <[e-mail]> wrote:
> From: Jiang Liu <[e-mail]>
>
> PCIe Base Spec 1.0 Section 5.8 states that "The PCI Express Capabilities,
> Device Capabilities, Device Status/Control, Link Capabilities and
> Link Status/Control registers are required for all PCI Express devices."

And spec 1.1 amended that (in sec 7.8, not 5.8) to say that the link
registers are not required for Root Complex Integrated Endpoints.
Integrated endpoints and event collectors don't have links, so even if
some RC-integrated devices do implement link registers per spec 1.0, I
don't think there's any point in allowing access to them.

> And PCIe Base Spec 3.0 Section 7.8 states that "The Link Capabilities,
> Link Status, and Link Control registers are required for all Root Ports,
> Switch Ports, Bridges, and Endpoints that are not Root Complex Integrated
> Endpoints."
>
> Changeset 1b6b8ce2ac372e "PCI: only save/restore existent registers in the
> PCIe capability" introduces pcie_cap_has_lnkctl(), which assumes:
> 1) PCIe root port, endpoint and legacy endpoint with 1.0 PCIe Capability
>    implement Link Cap/Status/Control registers.
> 2) All PCIe devices with 2.0 PCIe Capability implemnet Link Cap/Status/
>    Control registers.
>
> The above assumptions don't conform to PCIe base specifications, so change
> pcie_cap_has_lnkctl() to follow PCIe specifications.
>
> Signed-off-by: Jiang Liu <[e-mail]>
> ---
> Hi Bjorn,
>         Yuval has a PCIe 1.0 swither, so triggers the bug.  I'm not sure
> why Yu Zhao has implemented pcie_cap_has_linctl() in that way, any special
> consideration here?
>
> Hi Yuval,
>         Could you please help to test this patch? Due to hardware resource
> constraints, I have just compiled the patch on my laptop without testing.
>
> Thanks!
>
> ---
>  drivers/pci/access.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/access.c b/drivers/pci/access.c
> index 1cc2366..23ff366 100644
> --- a/drivers/pci/access.c
> +++ b/drivers/pci/access.c
> @@ -484,10 +484,14 @@ static inline bool pcie_cap_has_lnkctl(const struct pci_dev *dev)
>  {
>         int type = pci_pcie_type(dev);
>
> -       return pcie_cap_version(dev) > 1 ||
> -              type == PCI_EXP_TYPE_ROOT_PORT ||
> -              type == PCI_EXP_TYPE_ENDPOINT ||
> -              type == PCI_EXP_TYPE_LEG_END;
> +       return  pcie_cap_version(dev) == 1 ||
> +               type == PCI_EXP_TYPE_ENDPOINT ||
> +               type == PCI_EXP_TYPE_LEG_END ||
> +               type == PCI_EXP_TYPE_ROOT_PORT ||
> +               type == PCI_EXP_TYPE_UPSTREAM ||
> +               type == PCI_EXP_TYPE_DOWNSTREAM ||
> +               type == PCI_EXP_TYPE_PCI_BRIDGE ||
> +               type == PCI_EXP_TYPE_PCIE_BRIDGE;
>  }
>
>  static inline bool pcie_cap_has_sltctl(const struct pci_dev *dev)
> --
> 1.8.1.2
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Jacob Keller Aug. 27, 2013, 6:02 p.m. UTC | #2

On Tue, 2013-08-27 at 11:07 -0600, Bjorn Helgaas wrote:
> On Tue, Aug 27, 2013 at 10:44 AM, Jiang Liu <[e-mail]> wrote:

> > From: Jiang Liu <[e-mail]>

> >

> > PCIe Base Spec 1.0 Section 5.8 states that "The PCI Express Capabilities,

> > Device Capabilities, Device Status/Control, Link Capabilities and

> > Link Status/Control registers are required for all PCI Express devices."

> 

> And spec 1.1 amended that (in sec 7.8, not 5.8) to say that the link

> registers are not required for Root Complex Integrated Endpoints.

> Integrated endpoints and event collectors don't have links, so even if

> some RC-integrated devices do implement link registers per spec 1.0, I

> don't think there's any point in allowing access to them.

> 


Does this mean that pcie_get_minimum_link should be modified to check to
make sure the device has a linkstatus, and only keep track of values for
which there is a minimum?

Thanks,
Jake

> > And PCIe Base Spec 3.0 Section 7.8 states that "The Link Capabilities,

> > Link Status, and Link Control registers are required for all Root Ports,

> > Switch Ports, Bridges, and Endpoints that are not Root Complex Integrated

> > Endpoints."

> >

> > Changeset 1b6b8ce2ac372e "PCI: only save/restore existent registers in the

> > PCIe capability" introduces pcie_cap_has_lnkctl(), which assumes:

> > 1) PCIe root port, endpoint and legacy endpoint with 1.0 PCIe Capability

> >    implement Link Cap/Status/Control registers.

> > 2) All PCIe devices with 2.0 PCIe Capability implemnet Link Cap/Status/

> >    Control registers.

> >

> > The above assumptions don't conform to PCIe base specifications, so change

> > pcie_cap_has_lnkctl() to follow PCIe specifications.

> >

> > Signed-off-by: Jiang Liu <[e-mail]>

> > ---

> > Hi Bjorn,

> >         Yuval has a PCIe 1.0 swither, so triggers the bug.  I'm not sure

> > why Yu Zhao has implemented pcie_cap_has_linctl() in that way, any special

> > consideration here?

> >

> > Hi Yuval,

> >         Could you please help to test this patch? Due to hardware resource

> > constraints, I have just compiled the patch on my laptop without testing.

> >

> > Thanks!

> >

> > ---

> >  drivers/pci/access.c | 12 ++++++++----

> >  1 file changed, 8 insertions(+), 4 deletions(-)

> >

> > diff --git a/drivers/pci/access.c b/drivers/pci/access.c

> > index 1cc2366..23ff366 100644

> > --- a/drivers/pci/access.c

> > +++ b/drivers/pci/access.c

> > @@ -484,10 +484,14 @@ static inline bool pcie_cap_has_lnkctl(const struct pci_dev *dev)

> >  {

> >         int type = pci_pcie_type(dev);

> >

> > -       return pcie_cap_version(dev) > 1 ||

> > -              type == PCI_EXP_TYPE_ROOT_PORT ||

> > -              type == PCI_EXP_TYPE_ENDPOINT ||

> > -              type == PCI_EXP_TYPE_LEG_END;

> > +       return  pcie_cap_version(dev) == 1 ||

> > +               type == PCI_EXP_TYPE_ENDPOINT ||

> > +               type == PCI_EXP_TYPE_LEG_END ||

> > +               type == PCI_EXP_TYPE_ROOT_PORT ||

> > +               type == PCI_EXP_TYPE_UPSTREAM ||

> > +               type == PCI_EXP_TYPE_DOWNSTREAM ||

> > +               type == PCI_EXP_TYPE_PCI_BRIDGE ||

> > +               type == PCI_EXP_TYPE_PCIE_BRIDGE;

> >  }

> >

> >  static inline bool pcie_cap_has_sltctl(const struct pci_dev *dev)

> > --

> > 1.8.1.2

> >

Bjorn Helgaas Aug. 27, 2013, 6:11 p.m. UTC | #3

On Tue, Aug 27, 2013 at 12:02 PM, Keller, Jacob E
<[e-mail]> wrote:
> On Tue, 2013-08-27 at 11:07 -0600, Bjorn Helgaas wrote:
>> On Tue, Aug 27, 2013 at 10:44 AM, Jiang Liu <[e-mail]> wrote:
>> > From: Jiang Liu <[e-mail]>
>> >
>> > PCIe Base Spec 1.0 Section 5.8 states that "The PCI Express Capabilities,
>> > Device Capabilities, Device Status/Control, Link Capabilities and
>> > Link Status/Control registers are required for all PCI Express devices."
>>
>> And spec 1.1 amended that (in sec 7.8, not 5.8) to say that the link
>> registers are not required for Root Complex Integrated Endpoints.
>> Integrated endpoints and event collectors don't have links, so even if
>> some RC-integrated devices do implement link registers per spec 1.0, I
>> don't think there's any point in allowing access to them.
>>
>
> Does this mean that pcie_get_minimum_link should be modified to check to
> make sure the device has a linkstatus, and only keep track of values for
> which there is a minimum?

That doesn't seem necessary to me.  pcie_get_minimum_link() is only
looking at bridge devices (Downstream Ports, Upstream Ports, and Root
Ports).  All of those are required to implement the Link Status
register, so I think checking would be unnecessary complication.

The RC-integrated devices can't be bridges, so pcie_get_minimum_link()
should never even iterate over any of them.

>> > And PCIe Base Spec 3.0 Section 7.8 states that "The Link Capabilities,
>> > Link Status, and Link Control registers are required for all Root Ports,
>> > Switch Ports, Bridges, and Endpoints that are not Root Complex Integrated
>> > Endpoints."
>> >
>> > Changeset 1b6b8ce2ac372e "PCI: only save/restore existent registers in the
>> > PCIe capability" introduces pcie_cap_has_lnkctl(), which assumes:
>> > 1) PCIe root port, endpoint and legacy endpoint with 1.0 PCIe Capability
>> >    implement Link Cap/Status/Control registers.
>> > 2) All PCIe devices with 2.0 PCIe Capability implemnet Link Cap/Status/
>> >    Control registers.
>> >
>> > The above assumptions don't conform to PCIe base specifications, so change
>> > pcie_cap_has_lnkctl() to follow PCIe specifications.
>> >
>> > Signed-off-by: Jiang Liu <[e-mail]>
>> > ---
>> > Hi Bjorn,
>> >         Yuval has a PCIe 1.0 swither, so triggers the bug.  I'm not sure
>> > why Yu Zhao has implemented pcie_cap_has_linctl() in that way, any special
>> > consideration here?
>> >
>> > Hi Yuval,
>> >         Could you please help to test this patch? Due to hardware resource
>> > constraints, I have just compiled the patch on my laptop without testing.
>> >
>> > Thanks!
>> >
>> > ---
>> >  drivers/pci/access.c | 12 ++++++++----
>> >  1 file changed, 8 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/drivers/pci/access.c b/drivers/pci/access.c
>> > index 1cc2366..23ff366 100644
>> > --- a/drivers/pci/access.c
>> > +++ b/drivers/pci/access.c
>> > @@ -484,10 +484,14 @@ static inline bool pcie_cap_has_lnkctl(const struct pci_dev *dev)
>> >  {
>> >         int type = pci_pcie_type(dev);
>> >
>> > -       return pcie_cap_version(dev) > 1 ||
>> > -              type == PCI_EXP_TYPE_ROOT_PORT ||
>> > -              type == PCI_EXP_TYPE_ENDPOINT ||
>> > -              type == PCI_EXP_TYPE_LEG_END;
>> > +       return  pcie_cap_version(dev) == 1 ||
>> > +               type == PCI_EXP_TYPE_ENDPOINT ||
>> > +               type == PCI_EXP_TYPE_LEG_END ||
>> > +               type == PCI_EXP_TYPE_ROOT_PORT ||
>> > +               type == PCI_EXP_TYPE_UPSTREAM ||
>> > +               type == PCI_EXP_TYPE_DOWNSTREAM ||
>> > +               type == PCI_EXP_TYPE_PCI_BRIDGE ||
>> > +               type == PCI_EXP_TYPE_PCIE_BRIDGE;
>> >  }
>> >
>> >  static inline bool pcie_cap_has_sltctl(const struct pci_dev *dev)
>> > --
>> > 1.8.1.2
>> >
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Yuval Mintz Aug. 27, 2013, 10:28 p.m. UTC | #4

> On Tue, Aug 27, 2013 at 12:02 PM, Keller, Jacob E
> >> > From: Jiang Liu <[e-mail]>
> >> > ---
> >> > Hi Bjorn,
> >> >         Yuval has a PCIe 1.0 swither, so triggers the bug.  I'm not sure
> >> > why Yu Zhao has implemented pcie_cap_has_linctl() in that way, any
> special
> >> > consideration here?
> >> >
> >> > Hi Yuval,
> >> >         Could you please help to test this patch? Due to hardware resource
> >> > constraints, I have just compiled the patch on my laptop without testing.
> >> >

Hi,

Don't know whether your fix is correct or not (I'm leaving that to you PCI guys),
but it solved my issue - width now returns as x4. Thanks.

Tested-by: Yuval Mintz <[e-mail]>

> >> > Thanks!
> >> >
> >> > ---
> >> >  drivers/pci/access.c | 12 ++++++++----
> >> >  1 file changed, 8 insertions(+), 4 deletions(-)
> >> >
> >> > diff --git a/drivers/pci/access.c b/drivers/pci/access.c
> >> > index 1cc2366..23ff366 100644
> >> > --- a/drivers/pci/access.c
> >> > +++ b/drivers/pci/access.c
> >> > @@ -484,10 +484,14 @@ static inline bool pcie_cap_has_lnkctl(const
> struct pci_dev *dev)
> >> >  {
> >> >         int type = pci_pcie_type(dev);
> >> >
> >> > -       return pcie_cap_version(dev) > 1 ||
> >> > -              type == PCI_EXP_TYPE_ROOT_PORT ||
> >> > -              type == PCI_EXP_TYPE_ENDPOINT ||
> >> > -              type == PCI_EXP_TYPE_LEG_END;
> >> > +       return  pcie_cap_version(dev) == 1 ||
> >> > +               type == PCI_EXP_TYPE_ENDPOINT ||
> >> > +               type == PCI_EXP_TYPE_LEG_END ||
> >> > +               type == PCI_EXP_TYPE_ROOT_PORT ||
> >> > +               type == PCI_EXP_TYPE_UPSTREAM ||
> >> > +               type == PCI_EXP_TYPE_DOWNSTREAM ||
> >> > +               type == PCI_EXP_TYPE_PCI_BRIDGE ||
> >> > +               type == PCI_EXP_TYPE_PCIE_BRIDGE;
> >> >  }
> >> >
> >> >  static inline bool pcie_cap_has_sltctl(const struct pci_dev *dev)
> >> > --
> >> > 1.8.1.2
> >> >
> >
> >


--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Jiang Liu Aug. 28, 2013, 12:08 a.m. UTC | #5

On 08/28/2013 01:07 AM, Bjorn Helgaas wrote:
> On Tue, Aug 27, 2013 at 10:44 AM, Jiang Liu <[e-mail]> wrote:
>> From: Jiang Liu <[e-mail]>
>>
>> PCIe Base Spec 1.0 Section 5.8 states that "The PCI Express Capabilities,
>> Device Capabilities, Device Status/Control, Link Capabilities and
>> Link Status/Control registers are required for all PCI Express devices."
> 
> And spec 1.1 amended that (in sec 7.8, not 5.8) to say that the link
> registers are not required for Root Complex Integrated Endpoints.
> Integrated endpoints and event collectors don't have links, so even if
> some RC-integrated devices do implement link registers per spec 1.0, I
> don't think there's any point in allowing access to them.
Hi Bjorn,
	Sorry, I haven't noticed that PCIe Base spec 1.1 has modified
the definition without changing the PCIe capability version. So seems
we should remove the "pcie_cap_version(dev) == 1" check and just
verify PCIe express device types. Which form do you prefer?

!(type == PCI_EXP_TYPE_RC_END || type == PCI_EXP_TYPE_RC_EC)
or
type == PCI_EXP_TYPE_ENDPOINT ||
type == PCI_EXP_TYPE_LEG_END ||
type == PCI_EXP_TYPE_ROOT_PORT ||
type == PCI_EXP_TYPE_UPSTREAM ||
type == PCI_EXP_TYPE_DOWNSTREAM ||
type == PCI_EXP_TYPE_PCI_BRIDGE ||
type == PCI_EXP_TYPE_PCIE_BRIDGE;

Regards!
Gerry

> 
>> And PCIe Base Spec 3.0 Section 7.8 states that "The Link Capabilities,
>> Link Status, and Link Control registers are required for all Root Ports,
>> Switch Ports, Bridges, and Endpoints that are not Root Complex Integrated
>> Endpoints."
>>
>> Changeset 1b6b8ce2ac372e "PCI: only save/restore existent registers in the
>> PCIe capability" introduces pcie_cap_has_lnkctl(), which assumes:
>> 1) PCIe root port, endpoint and legacy endpoint with 1.0 PCIe Capability
>>    implement Link Cap/Status/Control registers.
>> 2) All PCIe devices with 2.0 PCIe Capability implemnet Link Cap/Status/
>>    Control registers.
>>
>> The above assumptions don't conform to PCIe base specifications, so change
>> pcie_cap_has_lnkctl() to follow PCIe specifications.
>>
>> Signed-off-by: Jiang Liu <[e-mail]>
>> ---
>> Hi Bjorn,
>>         Yuval has a PCIe 1.0 swither, so triggers the bug.  I'm not sure
>> why Yu Zhao has implemented pcie_cap_has_linctl() in that way, any special
>> consideration here?
>>
>> Hi Yuval,
>>         Could you please help to test this patch? Due to hardware resource
>> constraints, I have just compiled the patch on my laptop without testing.
>>
>> Thanks!
>>
>> ---
>>  drivers/pci/access.c | 12 ++++++++----
>>  1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/pci/access.c b/drivers/pci/access.c
>> index 1cc2366..23ff366 100644
>> --- a/drivers/pci/access.c
>> +++ b/drivers/pci/access.c
>> @@ -484,10 +484,14 @@ static inline bool pcie_cap_has_lnkctl(const struct pci_dev *dev)
>>  {
>>         int type = pci_pcie_type(dev);
>>
>> -       return pcie_cap_version(dev) > 1 ||
>> -              type == PCI_EXP_TYPE_ROOT_PORT ||
>> -              type == PCI_EXP_TYPE_ENDPOINT ||
>> -              type == PCI_EXP_TYPE_LEG_END;
>> +       return  pcie_cap_version(dev) == 1 ||
>> +               type == PCI_EXP_TYPE_ENDPOINT ||
>> +               type == PCI_EXP_TYPE_LEG_END ||
>> +               type == PCI_EXP_TYPE_ROOT_PORT ||
>> +               type == PCI_EXP_TYPE_UPSTREAM ||
>> +               type == PCI_EXP_TYPE_DOWNSTREAM ||
>> +               type == PCI_EXP_TYPE_PCI_BRIDGE ||
>> +               type == PCI_EXP_TYPE_PCIE_BRIDGE;
>>  }
>>
>>  static inline bool pcie_cap_has_sltctl(const struct pci_dev *dev)
>> --
>> 1.8.1.2
>>

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Bjorn Helgaas Aug. 28, 2013, 12:58 p.m. UTC | #6

On Tue, Aug 27, 2013 at 6:08 PM, Jiang Liu <[e-mail]> wrote:
> On 08/28/2013 01:07 AM, Bjorn Helgaas wrote:
>> On Tue, Aug 27, 2013 at 10:44 AM, Jiang Liu <[e-mail]> wrote:
>>> From: Jiang Liu <[e-mail]>
>>>
>>> PCIe Base Spec 1.0 Section 5.8 states that "The PCI Express Capabilities,
>>> Device Capabilities, Device Status/Control, Link Capabilities and
>>> Link Status/Control registers are required for all PCI Express devices."
>>
>> And spec 1.1 amended that (in sec 7.8, not 5.8) to say that the link
>> registers are not required for Root Complex Integrated Endpoints.
>> Integrated endpoints and event collectors don't have links, so even if
>> some RC-integrated devices do implement link registers per spec 1.0, I
>> don't think there's any point in allowing access to them.
> Hi Bjorn,
>         Sorry, I haven't noticed that PCIe Base spec 1.1 has modified
> the definition without changing the PCIe capability version. So seems
> we should remove the "pcie_cap_version(dev) == 1" check and just
> verify PCIe express device types. Which form do you prefer?

The "pcie_cap_version(dev) > 1" test is based on the spec r3.0
language to the effect that "For Functions that do not implement the
[Device, Link, Slot, Root] registers, these spaces must be hardwired
to 0b."  That means that for v2 capabilities, we don't need to check
the device type at all.  I think we should keep that check so the
structure of pcie_cap_has_lnkctl() remains similar to
pcie_cap_has_sltctl() and pcie_cap_has_rtctl().

It might be more obvious what we're doing if we restructured them all
in the form of:

    if (pcie_cap_version(dev) == 1)
        return type == PCI_EXP_TYPE_ROOT_PORT ||
                type == PCI_EXP_TYPE_ENDPOINT ||
                ...;
    return true;

That would make it more clear that for v1 capabilities, we have to
make extra checks, and for v2 capabilities, we rely on the r3.0 spec
language.

> !(type == PCI_EXP_TYPE_RC_END || type == PCI_EXP_TYPE_RC_EC)
> or
> type == PCI_EXP_TYPE_ENDPOINT ||
> type == PCI_EXP_TYPE_LEG_END ||
> type == PCI_EXP_TYPE_ROOT_PORT ||
> type == PCI_EXP_TYPE_UPSTREAM ||
> type == PCI_EXP_TYPE_DOWNSTREAM ||
> type == PCI_EXP_TYPE_PCI_BRIDGE ||
> type == PCI_EXP_TYPE_PCIE_BRIDGE;

I don't really care which of these styles we use.  Either way seems
future-proof, since no new device type should ever be added with a v1
capability.  Yours has the advantage of being stated in the positive,
which is a good aid to understanding.

>>> And PCIe Base Spec 3.0 Section 7.8 states that "The Link Capabilities,
>>> Link Status, and Link Control registers are required for all Root Ports,
>>> Switch Ports, Bridges, and Endpoints that are not Root Complex Integrated
>>> Endpoints."
>>>
>>> Changeset 1b6b8ce2ac372e "PCI: only save/restore existent registers in the
>>> PCIe capability" introduces pcie_cap_has_lnkctl(), which assumes:
>>> 1) PCIe root port, endpoint and legacy endpoint with 1.0 PCIe Capability
>>>    implement Link Cap/Status/Control registers.
>>> 2) All PCIe devices with 2.0 PCIe Capability implemnet Link Cap/Status/
>>>    Control registers.
>>>
>>> The above assumptions don't conform to PCIe base specifications, so change
>>> pcie_cap_has_lnkctl() to follow PCIe specifications.
>>>
>>> Signed-off-by: Jiang Liu <[e-mail]>
>>> ---
>>> Hi Bjorn,
>>>         Yuval has a PCIe 1.0 swither, so triggers the bug.  I'm not sure
>>> why Yu Zhao has implemented pcie_cap_has_linctl() in that way, any special
>>> consideration here?
>>>
>>> Hi Yuval,
>>>         Could you please help to test this patch? Due to hardware resource
>>> constraints, I have just compiled the patch on my laptop without testing.
>>>
>>> Thanks!
>>>
>>> ---
>>>  drivers/pci/access.c | 12 ++++++++----
>>>  1 file changed, 8 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/pci/access.c b/drivers/pci/access.c
>>> index 1cc2366..23ff366 100644
>>> --- a/drivers/pci/access.c
>>> +++ b/drivers/pci/access.c
>>> @@ -484,10 +484,14 @@ static inline bool pcie_cap_has_lnkctl(const struct pci_dev *dev)
>>>  {
>>>         int type = pci_pcie_type(dev);
>>>
>>> -       return pcie_cap_version(dev) > 1 ||
>>> -              type == PCI_EXP_TYPE_ROOT_PORT ||
>>> -              type == PCI_EXP_TYPE_ENDPOINT ||
>>> -              type == PCI_EXP_TYPE_LEG_END;
>>> +       return  pcie_cap_version(dev) == 1 ||
>>> +               type == PCI_EXP_TYPE_ENDPOINT ||
>>> +               type == PCI_EXP_TYPE_LEG_END ||
>>> +               type == PCI_EXP_TYPE_ROOT_PORT ||
>>> +               type == PCI_EXP_TYPE_UPSTREAM ||
>>> +               type == PCI_EXP_TYPE_DOWNSTREAM ||
>>> +               type == PCI_EXP_TYPE_PCI_BRIDGE ||
>>> +               type == PCI_EXP_TYPE_PCIE_BRIDGE;
>>>  }
>>>
>>>  static inline bool pcie_cap_has_sltctl(const struct pci_dev *dev)
>>> --
>>> 1.8.1.2
>>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/pci/access.c b/drivers/pci/access.c
index 1cc2366..23ff366 100644
--- a/drivers/pci/access.c
+++ b/drivers/pci/access.c
@@ -484,10 +484,14 @@  static inline bool pcie_cap_has_lnkctl(const struct pci_dev *dev)
 {
 	int type = pci_pcie_type(dev);
 
-	return pcie_cap_version(dev) > 1 ||
-	       type == PCI_EXP_TYPE_ROOT_PORT ||
-	       type == PCI_EXP_TYPE_ENDPOINT ||
-	       type == PCI_EXP_TYPE_LEG_END;
+	return	pcie_cap_version(dev) == 1 ||
+		type == PCI_EXP_TYPE_ENDPOINT ||
+		type == PCI_EXP_TYPE_LEG_END ||
+		type == PCI_EXP_TYPE_ROOT_PORT ||
+		type == PCI_EXP_TYPE_UPSTREAM ||
+		type == PCI_EXP_TYPE_DOWNSTREAM ||
+		type == PCI_EXP_TYPE_PCI_BRIDGE ||
+		type == PCI_EXP_TYPE_PCIE_BRIDGE;
 }
 
 static inline bool pcie_cap_has_sltctl(const struct pci_dev *dev)