| Message ID | 1442529694-1792-4-git-send-email-ddaney.cavm@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
On Thu, Sep 17, 2015 at 11:41:34PM +0100, David Daney wrote: > From: David Daney <david.daney@cavium.com> > > The config space for external PCIe root complexes on some Cavium > ThunderX SoCs is very similar to CAM and ECAM, but differs in the > shift values that have to be applied to the bus and devfn numbers to > compose that address window offset. These root complexes also have > the interesting property that there is no root bridge, so the standard > manner of limiting scanning to only the first device doesn't work. We > can use the standard pci-host-generic driver if we make a minor > addition to handle these differences, so we... > > Add a mapping function for ThunderX PCIe root complexes with a bus > shift of 24 and devfn shift of 16. Ignore accesses for devices other > than the first device on the primary bus. > > Document the whole thing in devicetree/bindings/pci/host-generic-pci.txt > > Signed-off-by: David Daney <david.daney@cavium.com> > --- > .../devicetree/bindings/pci/host-generic-pci.txt | 8 +++--- > drivers/pci/host/pci-host-generic.c | 29 ++++++++++++++++++++++ > 2 files changed, 34 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.txt b/Documentation/devicetree/bindings/pci/host-generic-pci.txt > index 105a968..a5aed0f 100644 > --- a/Documentation/devicetree/bindings/pci/host-generic-pci.txt > +++ b/Documentation/devicetree/bindings/pci/host-generic-pci.txt > @@ -14,9 +14,11 @@ tree bindings communicated in pci.txt: > > Properties of the host controller node: > > -- compatible : Must be "pci-host-cam-generic" or "pci-host-ecam-generic" > - depending on the layout of configuration space (CAM vs > - ECAM respectively). > +- compatible : One of the following with bus:devfn:reg mapped to the > + PCI config space address window in the bit positions shown: > + "pci-host-cam-generic" -- 'CAM' bits 16:8:0 > + "pci-host-ecam-generic" -- 'ECAM' bits 20:12:0 > + "cavium,pci-host-thunder-pem" -- bits 24:16:0 To me that's ECAM left shifted by 4. Is not it just a matter of defining config space base address (ie reg property) differently ? Lorenzo > > - device_type : Must be "pci". > > diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c > index e364232..e1d8d5b 100644 > --- a/drivers/pci/host/pci-host-generic.c > +++ b/drivers/pci/host/pci-host-generic.c > @@ -91,6 +91,32 @@ static struct gen_pci_cfg_bus_ops gen_pci_cfg_ecam_bus_ops = { > } > }; > > +static void __iomem *gen_pci_map_cfg_bus_thunder_pem(struct pci_bus *bus, > + unsigned int devfn, > + int where) > +{ > + struct gen_pci *pci = bus->sysdata; > + resource_size_t idx = bus->number - pci->cfg.bus_range->start; > + > + /* > + * Thunder PEM is a PCIe RC, but without a root bridge. On > + * the primary bus, ignore accesses for devices other than > + * the first device. > + */ > + if (idx == 0 && (devfn & ~7u)) > + return NULL; > + return pci->cfg.win[idx] + ((devfn << 16) | where); > +} > + > +static struct gen_pci_cfg_bus_ops gen_pci_cfg_thunder_pem_bus_ops = { > + .bus_shift = 24, > + .ops = { > + .map_bus = gen_pci_map_cfg_bus_thunder_pem, > + .read = pci_generic_config_read, > + .write = pci_generic_config_write, > + } > +}; > + > static const struct of_device_id gen_pci_of_match[] = { > { .compatible = "pci-host-cam-generic", > .data = &gen_pci_cfg_cam_bus_ops }, > @@ -98,6 +124,9 @@ static const struct of_device_id gen_pci_of_match[] = { > { .compatible = "pci-host-ecam-generic", > .data = &gen_pci_cfg_ecam_bus_ops }, > > + { .compatible = "cavium,pci-host-thunder-pem", > + .data = &gen_pci_cfg_thunder_pem_bus_ops }, > + > { }, > }; > MODULE_DEVICE_TABLE(of, gen_pci_of_match); > -- > 1.9.1 > > -- > 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
On 09/22/2015 09:05 AM, Lorenzo Pieralisi wrote: > On Thu, Sep 17, 2015 at 11:41:34PM +0100, David Daney wrote: >> From: David Daney <david.daney@cavium.com> >> >> The config space for external PCIe root complexes on some Cavium >> ThunderX SoCs is very similar to CAM and ECAM, but differs in the >> shift values that have to be applied to the bus and devfn numbers to >> compose that address window offset. These root complexes also have >> the interesting property that there is no root bridge, so the standard >> manner of limiting scanning to only the first device doesn't work. We >> can use the standard pci-host-generic driver if we make a minor >> addition to handle these differences, so we... >> >> Add a mapping function for ThunderX PCIe root complexes with a bus >> shift of 24 and devfn shift of 16. Ignore accesses for devices other >> than the first device on the primary bus. >> >> Document the whole thing in devicetree/bindings/pci/host-generic-pci.txt >> >> Signed-off-by: David Daney <david.daney@cavium.com> >> --- >> .../devicetree/bindings/pci/host-generic-pci.txt | 8 +++--- >> drivers/pci/host/pci-host-generic.c | 29 ++++++++++++++++++++++ >> 2 files changed, 34 insertions(+), 3 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.txt b/Documentation/devicetree/bindings/pci/host-generic-pci.txt >> index 105a968..a5aed0f 100644 >> --- a/Documentation/devicetree/bindings/pci/host-generic-pci.txt >> +++ b/Documentation/devicetree/bindings/pci/host-generic-pci.txt >> @@ -14,9 +14,11 @@ tree bindings communicated in pci.txt: >> >> Properties of the host controller node: >> >> -- compatible : Must be "pci-host-cam-generic" or "pci-host-ecam-generic" >> - depending on the layout of configuration space (CAM vs >> - ECAM respectively). >> +- compatible : One of the following with bus:devfn:reg mapped to the >> + PCI config space address window in the bit positions shown: >> + "pci-host-cam-generic" -- 'CAM' bits 16:8:0 >> + "pci-host-ecam-generic" -- 'ECAM' bits 20:12:0 >> + "cavium,pci-host-thunder-pem" -- bits 24:16:0 > > To me that's ECAM left shifted by 4. Is not it just a matter of defining > config space base address (ie reg property) differently ? No. That's like saying ECAM is just CAM shifted by 4, can't we just specify the "reg" differently. The "reg" property describes the base of the config space access window. CAM/ECAM/cavium,pci-host-thunder-pem describe how to generate offsets from that base, The offset calculation requires information that is not present in the "reg" property, it is enumerated in the various "compatible" values listed above. David Daney > > Lorenzo > >> >> - device_type : Must be "pci". >> >> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c >> index e364232..e1d8d5b 100644 >> --- a/drivers/pci/host/pci-host-generic.c >> +++ b/drivers/pci/host/pci-host-generic.c >> @@ -91,6 +91,32 @@ static struct gen_pci_cfg_bus_ops gen_pci_cfg_ecam_bus_ops = { >> } >> }; >> >> +static void __iomem *gen_pci_map_cfg_bus_thunder_pem(struct pci_bus *bus, >> + unsigned int devfn, >> + int where) >> +{ >> + struct gen_pci *pci = bus->sysdata; >> + resource_size_t idx = bus->number - pci->cfg.bus_range->start; >> + >> + /* >> + * Thunder PEM is a PCIe RC, but without a root bridge. On >> + * the primary bus, ignore accesses for devices other than >> + * the first device. >> + */ >> + if (idx == 0 && (devfn & ~7u)) >> + return NULL; >> + return pci->cfg.win[idx] + ((devfn << 16) | where); >> +} >> + >> +static struct gen_pci_cfg_bus_ops gen_pci_cfg_thunder_pem_bus_ops = { >> + .bus_shift = 24, >> + .ops = { >> + .map_bus = gen_pci_map_cfg_bus_thunder_pem, >> + .read = pci_generic_config_read, >> + .write = pci_generic_config_write, >> + } >> +}; >> + >> static const struct of_device_id gen_pci_of_match[] = { >> { .compatible = "pci-host-cam-generic", >> .data = &gen_pci_cfg_cam_bus_ops }, >> @@ -98,6 +124,9 @@ static const struct of_device_id gen_pci_of_match[] = { >> { .compatible = "pci-host-ecam-generic", >> .data = &gen_pci_cfg_ecam_bus_ops }, >> >> + { .compatible = "cavium,pci-host-thunder-pem", >> + .data = &gen_pci_cfg_thunder_pem_bus_ops }, >> + >> { }, >> }; >> MODULE_DEVICE_TABLE(of, gen_pci_of_match); >> -- >> 1.9.1 >> >> -- >> 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
On Tue, Sep 22, 2015 at 05:13:45PM +0100, David Daney wrote: > On 09/22/2015 09:05 AM, Lorenzo Pieralisi wrote: > > On Thu, Sep 17, 2015 at 11:41:34PM +0100, David Daney wrote: [...] > >> Properties of the host controller node: > >> > >> -- compatible : Must be "pci-host-cam-generic" or "pci-host-ecam-generic" > >> - depending on the layout of configuration space (CAM vs > >> - ECAM respectively). > >> +- compatible : One of the following with bus:devfn:reg mapped to the > >> + PCI config space address window in the bit positions shown: > >> + "pci-host-cam-generic" -- 'CAM' bits 16:8:0 > >> + "pci-host-ecam-generic" -- 'ECAM' bits 20:12:0 > >> + "cavium,pci-host-thunder-pem" -- bits 24:16:0 > > > > To me that's ECAM left shifted by 4. Is not it just a matter of defining > > config space base address (ie reg property) differently ? > > No. > > That's like saying ECAM is just CAM shifted by 4, can't we just specify > the "reg" differently. Ok, sorry I was not clear, apologies. I meant to ask if it is something that can be configured in your platform and apparently it is not, I worded it wrongly. > The "reg" property describes the base of the config space access window. > CAM/ECAM/cavium,pci-host-thunder-pem describe how to generate offsets > from that base, The offset calculation requires information that is not > present in the "reg" property, it is enumerated in the various > "compatible" values listed above. Agreed, this also forces you to map in more virtual address space for a given bus, right ? I am also a bit worried about how we can make this work with ACPI, where MCFG accessors take the bus and devfn shifts for granted (ie ECAM specs), so I asked. Thanks, Lorenzo > > David Daney > > > > > Lorenzo > > > >> > >> - device_type : Must be "pci". > >> > >> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c > >> index e364232..e1d8d5b 100644 > >> --- a/drivers/pci/host/pci-host-generic.c > >> +++ b/drivers/pci/host/pci-host-generic.c > >> @@ -91,6 +91,32 @@ static struct gen_pci_cfg_bus_ops gen_pci_cfg_ecam_bus_ops = { > >> } > >> }; > >> > >> +static void __iomem *gen_pci_map_cfg_bus_thunder_pem(struct pci_bus *bus, > >> + unsigned int devfn, > >> + int where) > >> +{ > >> + struct gen_pci *pci = bus->sysdata; > >> + resource_size_t idx = bus->number - pci->cfg.bus_range->start; > >> + > >> + /* > >> + * Thunder PEM is a PCIe RC, but without a root bridge. On > >> + * the primary bus, ignore accesses for devices other than > >> + * the first device. > >> + */ > >> + if (idx == 0 && (devfn & ~7u)) > >> + return NULL; > >> + return pci->cfg.win[idx] + ((devfn << 16) | where); > >> +} > >> + > >> +static struct gen_pci_cfg_bus_ops gen_pci_cfg_thunder_pem_bus_ops = { > >> + .bus_shift = 24, > >> + .ops = { > >> + .map_bus = gen_pci_map_cfg_bus_thunder_pem, > >> + .read = pci_generic_config_read, > >> + .write = pci_generic_config_write, > >> + } > >> +}; > >> + > >> static const struct of_device_id gen_pci_of_match[] = { > >> { .compatible = "pci-host-cam-generic", > >> .data = &gen_pci_cfg_cam_bus_ops }, > >> @@ -98,6 +124,9 @@ static const struct of_device_id gen_pci_of_match[] = { > >> { .compatible = "pci-host-ecam-generic", > >> .data = &gen_pci_cfg_ecam_bus_ops }, > >> > >> + { .compatible = "cavium,pci-host-thunder-pem", > >> + .data = &gen_pci_cfg_thunder_pem_bus_ops }, > >> + > >> { }, > >> }; > >> MODULE_DEVICE_TABLE(of, gen_pci_of_match); > >> -- > >> 1.9.1 > >> > >> -- > >> 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
On 09/22/2015 09:40 AM, Lorenzo Pieralisi wrote: > On Tue, Sep 22, 2015 at 05:13:45PM +0100, David Daney wrote: >> On 09/22/2015 09:05 AM, Lorenzo Pieralisi wrote: >>> On Thu, Sep 17, 2015 at 11:41:34PM +0100, David Daney wrote: > > [...] > >>>> Properties of the host controller node: >>>> >>>> -- compatible : Must be "pci-host-cam-generic" or "pci-host-ecam-generic" >>>> - depending on the layout of configuration space (CAM vs >>>> - ECAM respectively). >>>> +- compatible : One of the following with bus:devfn:reg mapped to the >>>> + PCI config space address window in the bit positions shown: >>>> + "pci-host-cam-generic" -- 'CAM' bits 16:8:0 >>>> + "pci-host-ecam-generic" -- 'ECAM' bits 20:12:0 >>>> + "cavium,pci-host-thunder-pem" -- bits 24:16:0 >>> >>> To me that's ECAM left shifted by 4. Is not it just a matter of defining >>> config space base address (ie reg property) differently ? >> >> No. >> >> That's like saying ECAM is just CAM shifted by 4, can't we just specify >> the "reg" differently. > > Ok, sorry I was not clear, apologies. I meant to ask if it is something > that can be configured in your platform and apparently it is not, No, the hardware cannot be adjusted. > I worded it wrongly. OK, I guess I also misunderstood what you were suggesting. > >> The "reg" property describes the base of the config space access window. >> CAM/ECAM/cavium,pci-host-thunder-pem describe how to generate offsets >> from that base, The offset calculation requires information that is not >> present in the "reg" property, it is enumerated in the various >> "compatible" values listed above. > > Agreed, this also forces you to map in more virtual address space for > a given bus, right ? Yes. It takes a full 32-bits of address space to have access to all PCI buses on the root complex. > I am also a bit worried about how we can make this > work with ACPI, where MCFG accessors take the bus and devfn shifts for > granted (ie ECAM specs), so I asked. If ACPI is not flexible enough to describe actual hardware, it will either have to be enhanced, or not used. > > Thanks, > Lorenzo > >> >> David Daney >> >>> >>> Lorenzo >>> >>>> >>>> - device_type : Must be "pci". >>>> >>>> diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c >>>> index e364232..e1d8d5b 100644 >>>> --- a/drivers/pci/host/pci-host-generic.c >>>> +++ b/drivers/pci/host/pci-host-generic.c >>>> @@ -91,6 +91,32 @@ static struct gen_pci_cfg_bus_ops gen_pci_cfg_ecam_bus_ops = { >>>> } >>>> }; >>>> >>>> +static void __iomem *gen_pci_map_cfg_bus_thunder_pem(struct pci_bus *bus, >>>> + unsigned int devfn, >>>> + int where) >>>> +{ >>>> + struct gen_pci *pci = bus->sysdata; >>>> + resource_size_t idx = bus->number - pci->cfg.bus_range->start; >>>> + >>>> + /* >>>> + * Thunder PEM is a PCIe RC, but without a root bridge. On >>>> + * the primary bus, ignore accesses for devices other than >>>> + * the first device. >>>> + */ >>>> + if (idx == 0 && (devfn & ~7u)) >>>> + return NULL; >>>> + return pci->cfg.win[idx] + ((devfn << 16) | where); >>>> +} >>>> + >>>> +static struct gen_pci_cfg_bus_ops gen_pci_cfg_thunder_pem_bus_ops = { >>>> + .bus_shift = 24, >>>> + .ops = { >>>> + .map_bus = gen_pci_map_cfg_bus_thunder_pem, >>>> + .read = pci_generic_config_read, >>>> + .write = pci_generic_config_write, >>>> + } >>>> +}; >>>> + >>>> static const struct of_device_id gen_pci_of_match[] = { >>>> { .compatible = "pci-host-cam-generic", >>>> .data = &gen_pci_cfg_cam_bus_ops }, >>>> @@ -98,6 +124,9 @@ static const struct of_device_id gen_pci_of_match[] = { >>>> { .compatible = "pci-host-ecam-generic", >>>> .data = &gen_pci_cfg_ecam_bus_ops }, >>>> >>>> + { .compatible = "cavium,pci-host-thunder-pem", >>>> + .data = &gen_pci_cfg_thunder_pem_bus_ops }, >>>> + >>>> { }, >>>> }; >>>> MODULE_DEVICE_TABLE(of, gen_pci_of_match); >>>> -- >>>> 1.9.1 >>>> >>>> -- >>>> 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
On Thu, Sep 17, 2015 at 11:41:34PM +0100, David Daney wrote: > From: David Daney <david.daney@cavium.com> > > The config space for external PCIe root complexes on some Cavium > ThunderX SoCs is very similar to CAM and ECAM, but differs in the > shift values that have to be applied to the bus and devfn numbers to > compose that address window offset. These root complexes also have > the interesting property that there is no root bridge, so the standard > manner of limiting scanning to only the first device doesn't work. We > can use the standard pci-host-generic driver if we make a minor > addition to handle these differences, so we... > > Add a mapping function for ThunderX PCIe root complexes with a bus > shift of 24 and devfn shift of 16. Ignore accesses for devices other > than the first device on the primary bus. > > Document the whole thing in devicetree/bindings/pci/host-generic-pci.txt > > Signed-off-by: David Daney <david.daney@cavium.com> > --- > .../devicetree/bindings/pci/host-generic-pci.txt | 8 +++--- > drivers/pci/host/pci-host-generic.c | 29 ++++++++++++++++++++++ > 2 files changed, 34 insertions(+), 3 deletions(-) Thanks, this looks better now: Acked-by: Will Deacon <will.deacon@arm.com> Will -- 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
On 09/22/2015 11:52 AM, Will Deacon wrote: > On Thu, Sep 17, 2015 at 11:41:34PM +0100, David Daney wrote: >> From: David Daney <david.daney@cavium.com> >> >> The config space for external PCIe root complexes on some Cavium >> ThunderX SoCs is very similar to CAM and ECAM, but differs in the >> shift values that have to be applied to the bus and devfn numbers to >> compose that address window offset. These root complexes also have >> the interesting property that there is no root bridge, so the standard >> manner of limiting scanning to only the first device doesn't work. We >> can use the standard pci-host-generic driver if we make a minor >> addition to handle these differences, so we... >> >> Add a mapping function for ThunderX PCIe root complexes with a bus >> shift of 24 and devfn shift of 16. Ignore accesses for devices other >> than the first device on the primary bus. >> >> Document the whole thing in devicetree/bindings/pci/host-generic-pci.txt >> >> Signed-off-by: David Daney <david.daney@cavium.com> >> --- >> .../devicetree/bindings/pci/host-generic-pci.txt | 8 +++--- >> drivers/pci/host/pci-host-generic.c | 29 ++++++++++++++++++++++ >> 2 files changed, 34 insertions(+), 3 deletions(-) > > Thanks, this looks better now: > > Acked-by: Will Deacon <will.deacon@arm.com> > Thanks Will. Because patches 1/3 and 2/3 will be reworked, I will re-send this as a stand-alone patch. David Daney > Will > -- 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
diff --git a/Documentation/devicetree/bindings/pci/host-generic-pci.txt b/Documentation/devicetree/bindings/pci/host-generic-pci.txt index 105a968..a5aed0f 100644 --- a/Documentation/devicetree/bindings/pci/host-generic-pci.txt +++ b/Documentation/devicetree/bindings/pci/host-generic-pci.txt @@ -14,9 +14,11 @@ tree bindings communicated in pci.txt: Properties of the host controller node: -- compatible : Must be "pci-host-cam-generic" or "pci-host-ecam-generic" - depending on the layout of configuration space (CAM vs - ECAM respectively). +- compatible : One of the following with bus:devfn:reg mapped to the + PCI config space address window in the bit positions shown: + "pci-host-cam-generic" -- 'CAM' bits 16:8:0 + "pci-host-ecam-generic" -- 'ECAM' bits 20:12:0 + "cavium,pci-host-thunder-pem" -- bits 24:16:0 - device_type : Must be "pci". diff --git a/drivers/pci/host/pci-host-generic.c b/drivers/pci/host/pci-host-generic.c index e364232..e1d8d5b 100644 --- a/drivers/pci/host/pci-host-generic.c +++ b/drivers/pci/host/pci-host-generic.c @@ -91,6 +91,32 @@ static struct gen_pci_cfg_bus_ops gen_pci_cfg_ecam_bus_ops = { } }; +static void __iomem *gen_pci_map_cfg_bus_thunder_pem(struct pci_bus *bus, + unsigned int devfn, + int where) +{ + struct gen_pci *pci = bus->sysdata; + resource_size_t idx = bus->number - pci->cfg.bus_range->start; + + /* + * Thunder PEM is a PCIe RC, but without a root bridge. On + * the primary bus, ignore accesses for devices other than + * the first device. + */ + if (idx == 0 && (devfn & ~7u)) + return NULL; + return pci->cfg.win[idx] + ((devfn << 16) | where); +} + +static struct gen_pci_cfg_bus_ops gen_pci_cfg_thunder_pem_bus_ops = { + .bus_shift = 24, + .ops = { + .map_bus = gen_pci_map_cfg_bus_thunder_pem, + .read = pci_generic_config_read, + .write = pci_generic_config_write, + } +}; + static const struct of_device_id gen_pci_of_match[] = { { .compatible = "pci-host-cam-generic", .data = &gen_pci_cfg_cam_bus_ops }, @@ -98,6 +124,9 @@ static const struct of_device_id gen_pci_of_match[] = { { .compatible = "pci-host-ecam-generic", .data = &gen_pci_cfg_ecam_bus_ops }, + { .compatible = "cavium,pci-host-thunder-pem", + .data = &gen_pci_cfg_thunder_pem_bus_ops }, + { }, }; MODULE_DEVICE_TABLE(of, gen_pci_of_match);