Patchwork [3/4] pci: mvebu: emulate an empty capability list

login
register
mail settings
Submitter Thomas Petazzoni
Date May 22, 2013, 1:12 p.m.
Message ID <1369228358-32580-4-git-send-email-thomas.petazzoni@free-electrons.com>
Download mbox | patch
Permalink /patch/245615/
State Not Applicable
Headers show

Comments

Thomas Petazzoni - May 22, 2013, 1:12 p.m.
Currently, our PCI-to-PCI bridge emulation doesn't emulate a proper
capability list, which leads 'lspci -v' to show:

  Capabilities: [fc] <chain broken>

In order to fix this, this commit improve the PCI-to-PCI bridge
emulation to emulate an empty capability list. It might be later
extended to expose things like the PCI Express Capability header, but
an empty capability list is sufficient for now.

lspci -v now shows the much nicer:

  Capabilities: [40] #00 [0000]

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 drivers/pci/host/pci-mvebu.c |   10 ++++++++++
 1 file changed, 10 insertions(+)
Bjorn Helgaas - May 22, 2013, 2:39 p.m.
On Wed, May 22, 2013 at 7:12 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Currently, our PCI-to-PCI bridge emulation doesn't emulate a proper
> capability list, which leads 'lspci -v' to show:
>
>   Capabilities: [fc] <chain broken>
>
> In order to fix this, this commit improve the PCI-to-PCI bridge

s/improve/improves/

> emulation to emulate an empty capability list. It might be later
> extended to expose things like the PCI Express Capability header, but
> an empty capability list is sufficient for now.
>
> lspci -v now shows the much nicer:
>
>   Capabilities: [40] #00 [0000]

It'd be even better if you could make the Capabilities List bit in the
Device Status register be zero.  Then lspci wouldn't even try to read
the Capabilities Pointer at 0x34, and you wouldn't have to deal with
reads of 0x40.

> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  drivers/pci/host/pci-mvebu.c |   10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
> index c21ca84..c887598 100644
> --- a/drivers/pci/host/pci-mvebu.c
> +++ b/drivers/pci/host/pci-mvebu.c
> @@ -440,6 +440,16 @@ static int mvebu_sw_pci_bridge_read(struct mvebu_pcie_port *port,
>                 *value = 0;
>                 break;
>
> +       case PCI_CAPABILITY_LIST:
> +               /* Offset of the capability list */
> +               *value = 0x40;
> +               break;
> +
> +       case 0x40:
> +               /* We have no element in our capability list */
> +               *value = 0;
> +               break;
> +
>         default:
>                 *value = 0xffffffff;
>                 return PCIBIOS_BAD_REGISTER_NUMBER;
> --
> 1.7.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Thomas Petazzoni - May 22, 2013, 2:55 p.m.
Dear Bjorn Helgaas,

On Wed, 22 May 2013 08:39:03 -0600, Bjorn Helgaas wrote:

> > emulation to emulate an empty capability list. It might be later
> > extended to expose things like the PCI Express Capability header, but
> > an empty capability list is sufficient for now.
> >
> > lspci -v now shows the much nicer:
> >
> >   Capabilities: [40] #00 [0000]
> 
> It'd be even better if you could make the Capabilities List bit in the
> Device Status register be zero.  Then lspci wouldn't even try to read
> the Capabilities Pointer at 0x34, and you wouldn't have to deal with
> reads of 0x40.

The Device Status register is emulated with an initial value of zero.
Then, whenever the Linux PCI core writes into it, those writes are
preserved. So to me, it looks like the Linux PCI core might be setting
this Capabilities List bit, and later re-reads it and finds it to be
set to 1.

Should this bit be emulated as a read-only bit (it's not made explicit
for this particular bit in the PCI-to-PCI bridge specification that I
have in front of me) ? Or even the entire Device Status register ?

Thanks for your comments!

Thomas

Patch

diff --git a/drivers/pci/host/pci-mvebu.c b/drivers/pci/host/pci-mvebu.c
index c21ca84..c887598 100644
--- a/drivers/pci/host/pci-mvebu.c
+++ b/drivers/pci/host/pci-mvebu.c
@@ -440,6 +440,16 @@  static int mvebu_sw_pci_bridge_read(struct mvebu_pcie_port *port,
 		*value = 0;
 		break;
 
+	case PCI_CAPABILITY_LIST:
+		/* Offset of the capability list */
+		*value = 0x40;
+		break;
+
+	case 0x40:
+		/* We have no element in our capability list */
+		*value = 0;
+		break;
+
 	default:
 		*value = 0xffffffff;
 		return PCIBIOS_BAD_REGISTER_NUMBER;