| Message ID | 20200708093018.28474-9-kishon@ti.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Lorenzo Pieralisi |
| Headers | show |
| Series | Add PCIe support to TI's J721E SoC | expand |
On Wed, Jul 08, 2020 at 03:00:12PM +0530, Kishon Vijay Abraham I wrote: > Commit 1b79c5284439 ("PCI: cadence: Add host driver for Cadence PCIe > controller") in order to update Vendor ID, directly wrote to > PCI_VENDOR_ID register. However PCI_VENDOR_ID in root port configuration > space is read-only register and writing to it will have no effect. > Use local management register to configure Vendor ID and Subsystem Vendor > ID. > > Fixes: 1b79c5284439 ("PCI: cadence: Add host driver for Cadence PCIe controller") > Reviewed-by: Rob Herring <robh@kernel.org> > Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> > --- > drivers/pci/controller/cadence/pcie-cadence-host.c | 9 +++++++-- > 1 file changed, 7 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c > index 10127ea71b83..8935f7a37e5a 100644 > --- a/drivers/pci/controller/cadence/pcie-cadence-host.c > +++ b/drivers/pci/controller/cadence/pcie-cadence-host.c > @@ -82,6 +82,7 @@ static int cdns_pcie_host_init_root_port(struct cdns_pcie_rc *rc) > { > struct cdns_pcie *pcie = &rc->pcie; > u32 value, ctrl; > + u32 id; > > /* > * Set the root complex BAR configuration register: > @@ -101,8 +102,12 @@ static int cdns_pcie_host_init_root_port(struct cdns_pcie_rc *rc) > cdns_pcie_writel(pcie, CDNS_PCIE_LM_RC_BAR_CFG, value); > > /* Set root port configuration space */ > - if (rc->vendor_id != 0xffff) > - cdns_pcie_rp_writew(pcie, PCI_VENDOR_ID, rc->vendor_id); If this is read-only, then... > + if (rc->vendor_id != 0xffff) { > + id = CDNS_PCIE_LM_ID_VENDOR(rc->vendor_id) | > + CDNS_PCIE_LM_ID_SUBSYS(rc->vendor_id); > + cdns_pcie_writel(pcie, CDNS_PCIE_LM_ID, id); > + } > + > if (rc->device_id != 0xffff) > cdns_pcie_rp_writew(pcie, PCI_DEVICE_ID, rc->device_id); ...isn't this read-only too? > > -- > 2.17.1 >
Hi Rob, On 7/10/2020 3:15 AM, Rob Herring wrote: > On Wed, Jul 08, 2020 at 03:00:12PM +0530, Kishon Vijay Abraham I wrote: >> Commit 1b79c5284439 ("PCI: cadence: Add host driver for Cadence PCIe >> controller") in order to update Vendor ID, directly wrote to >> PCI_VENDOR_ID register. However PCI_VENDOR_ID in root port configuration >> space is read-only register and writing to it will have no effect. >> Use local management register to configure Vendor ID and Subsystem Vendor >> ID. >> >> Fixes: 1b79c5284439 ("PCI: cadence: Add host driver for Cadence PCIe controller") >> Reviewed-by: Rob Herring <robh@kernel.org> >> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> >> --- >> drivers/pci/controller/cadence/pcie-cadence-host.c | 9 +++++++-- >> 1 file changed, 7 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c >> index 10127ea71b83..8935f7a37e5a 100644 >> --- a/drivers/pci/controller/cadence/pcie-cadence-host.c >> +++ b/drivers/pci/controller/cadence/pcie-cadence-host.c >> @@ -82,6 +82,7 @@ static int cdns_pcie_host_init_root_port(struct cdns_pcie_rc *rc) >> { >> struct cdns_pcie *pcie = &rc->pcie; >> u32 value, ctrl; >> + u32 id; >> >> /* >> * Set the root complex BAR configuration register: >> @@ -101,8 +102,12 @@ static int cdns_pcie_host_init_root_port(struct cdns_pcie_rc *rc) >> cdns_pcie_writel(pcie, CDNS_PCIE_LM_RC_BAR_CFG, value); >> >> /* Set root port configuration space */ >> - if (rc->vendor_id != 0xffff) >> - cdns_pcie_rp_writew(pcie, PCI_VENDOR_ID, rc->vendor_id); > > If this is read-only, then... > >> + if (rc->vendor_id != 0xffff) { >> + id = CDNS_PCIE_LM_ID_VENDOR(rc->vendor_id) | >> + CDNS_PCIE_LM_ID_SUBSYS(rc->vendor_id); >> + cdns_pcie_writel(pcie, CDNS_PCIE_LM_ID, id); >> + } >> + >> if (rc->device_id != 0xffff) >> cdns_pcie_rp_writew(pcie, PCI_DEVICE_ID, rc->device_id); > > ...isn't this read-only too? Apparently no. There is local management register only for vendor ID and subsystem vendorID. It could be because this same IP is used EP mode and in case of multi-function device, different deviceID could be required for each independent function whereas vendor ID should be same, they could have given a single local management register for vendorID and not for deviceID. Thanks Kishon
diff --git a/drivers/pci/controller/cadence/pcie-cadence-host.c b/drivers/pci/controller/cadence/pcie-cadence-host.c index 10127ea71b83..8935f7a37e5a 100644 --- a/drivers/pci/controller/cadence/pcie-cadence-host.c +++ b/drivers/pci/controller/cadence/pcie-cadence-host.c @@ -82,6 +82,7 @@ static int cdns_pcie_host_init_root_port(struct cdns_pcie_rc *rc) { struct cdns_pcie *pcie = &rc->pcie; u32 value, ctrl; + u32 id; /* * Set the root complex BAR configuration register: @@ -101,8 +102,12 @@ static int cdns_pcie_host_init_root_port(struct cdns_pcie_rc *rc) cdns_pcie_writel(pcie, CDNS_PCIE_LM_RC_BAR_CFG, value); /* Set root port configuration space */ - if (rc->vendor_id != 0xffff) - cdns_pcie_rp_writew(pcie, PCI_VENDOR_ID, rc->vendor_id); + if (rc->vendor_id != 0xffff) { + id = CDNS_PCIE_LM_ID_VENDOR(rc->vendor_id) | + CDNS_PCIE_LM_ID_SUBSYS(rc->vendor_id); + cdns_pcie_writel(pcie, CDNS_PCIE_LM_ID, id); + } + if (rc->device_id != 0xffff) cdns_pcie_rp_writew(pcie, PCI_DEVICE_ID, rc->device_id);
Commit 1b79c5284439 ("PCI: cadence: Add host driver for Cadence PCIe controller") in order to update Vendor ID, directly wrote to PCI_VENDOR_ID register. However PCI_VENDOR_ID in root port configuration space is read-only register and writing to it will have no effect. Use local management register to configure Vendor ID and Subsystem Vendor ID. Fixes: 1b79c5284439 ("PCI: cadence: Add host driver for Cadence PCIe controller") Reviewed-by: Rob Herring <robh@kernel.org> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com> --- drivers/pci/controller/cadence/pcie-cadence-host.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)