| Message ID | 20250123070935.1810110-1-18255117159@163.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Krzysztof Wilczyński |
| Headers | show |
| Series | [RESEND] PCI: cadence: Add configuration space capability search API | expand |
On Thu, Jan 23, 2025 at 03:09:35PM +0800, Hans Zhang wrote: > Add configuration space capability search API using struct cdns_pcie* > pointer. > > Similar patches below have been merged. > commit 5b0841fa653f ("PCI: dwc: Add extended configuration space capability > search API") > commit 7a6854f6874f ("PCI: dwc: Move config space capability search API") Similar patches being merged doesn't sound like a proper reason for having a feature. Please provide details regarding why this is required. Assuming that the intent for introducing this feature is to use it later, it will be a good idea to post the patch for that as well in the same series. > > Signed-off-by: Hans Zhang <18255117159@163.com> > --- > drivers/pci/controller/cadence/pcie-cadence.c | 80 +++++++++++++++++++ > drivers/pci/controller/cadence/pcie-cadence.h | 3 + > 2 files changed, 83 insertions(+) [...] Regards, Siddharth.
On 2025/1/23 15:40, Siddharth Vadapalli wrote: > On Thu, Jan 23, 2025 at 03:09:35PM +0800, Hans Zhang wrote: >> Add configuration space capability search API using struct cdns_pcie* >> pointer. >> >> Similar patches below have been merged. >> commit 5b0841fa653f ("PCI: dwc: Add extended configuration space capability >> search API") >> commit 7a6854f6874f ("PCI: dwc: Move config space capability search API") > > Similar patches being merged doesn't sound like a proper reason for > having a feature. Please provide details regarding why this is required. > Assuming that the intent for introducing this feature is to use it > later, it will be a good idea to post the patch for that as well in the > same series. > Hi Siddharth, For our SOC platform, the offset of some capability needs to be found during the initialization process, which I think should be put into the cadence public code eg: For API: cdns_pcie_find_capability Need to find PCI Express, then set link speed, retrain link, MaxPayload, MaxReadReq, Enable Relaxed Ordering. For API: cdns_pcie_find_ext_capability Need to find the Secondary PCIe Capability and set the GEN3 preset value. Find the Physical Layer 16.0 GT/s and set the GEN4 preset value. Development board based on our SOC, Radxa Orinon O6. https://radxa.com/products/orion/o6/ Our controller driver currently has no plans for upstream and needs to wait for notification from the boss. Best regards Hans
On Thu, Jan 23, 2025 at 04:15:12PM +0800, Hans Zhang wrote: > On 2025/1/23 15:40, Siddharth Vadapalli wrote: > > On Thu, Jan 23, 2025 at 03:09:35PM +0800, Hans Zhang wrote: > > > Add configuration space capability search API using struct cdns_pcie* > > > pointer. > > > > > > Similar patches below have been merged. > > > commit 5b0841fa653f ("PCI: dwc: Add extended configuration space capability > > > search API") > > > commit 7a6854f6874f ("PCI: dwc: Move config space capability search API") > > > > Similar patches being merged doesn't sound like a proper reason for > > having a feature. Please provide details regarding why this is required. > > Assuming that the intent for introducing this feature is to use it > > later, it will be a good idea to post the patch for that as well in the > > same series. > > For our SOC platform, the offset of some capability needs to be found during > the initialization process, which I think should be put into the cadence > public code > > eg: > > For API: cdns_pcie_find_capability > Need to find PCI Express, then set link speed, retrain link, MaxPayload, > MaxReadReq, Enable Relaxed Ordering. > > For API: cdns_pcie_find_ext_capability > Need to find the Secondary PCIe Capability and set the GEN3 preset value. > Find the Physical Layer 16.0 GT/s and set the GEN4 preset value. > > Development board based on our SOC, Radxa Orinon O6. > https://radxa.com/products/orion/o6/ > > Our controller driver currently has no plans for upstream and needs to wait > for notification from the boss. If/when you upstream code that needs this interface, include this patch as part of the series. As Siddharth pointed out, we avoid merging code that has no upstream users. Bjorn
On 2025/1/24 01:08, Bjorn Helgaas wrote: > On Thu, Jan 23, 2025 at 04:15:12PM +0800, Hans Zhang wrote: >> On 2025/1/23 15:40, Siddharth Vadapalli wrote: >>> On Thu, Jan 23, 2025 at 03:09:35PM +0800, Hans Zhang wrote: >>>> Add configuration space capability search API using struct cdns_pcie* >>>> pointer. >>>> >>>> Similar patches below have been merged. >>>> commit 5b0841fa653f ("PCI: dwc: Add extended configuration space capability >>>> search API") >>>> commit 7a6854f6874f ("PCI: dwc: Move config space capability search API") >>> >>> Similar patches being merged doesn't sound like a proper reason for >>> having a feature. Please provide details regarding why this is required. >>> Assuming that the intent for introducing this feature is to use it >>> later, it will be a good idea to post the patch for that as well in the >>> same series. >> >> For our SOC platform, the offset of some capability needs to be found during >> the initialization process, which I think should be put into the cadence >> public code >> >> eg: >> >> For API: cdns_pcie_find_capability >> Need to find PCI Express, then set link speed, retrain link, MaxPayload, >> MaxReadReq, Enable Relaxed Ordering. >> >> For API: cdns_pcie_find_ext_capability >> Need to find the Secondary PCIe Capability and set the GEN3 preset value. >> Find the Physical Layer 16.0 GT/s and set the GEN4 preset value. >> >> Development board based on our SOC, Radxa Orinon O6. >> https://radxa.com/products/orion/o6/ >> >> Our controller driver currently has no plans for upstream and needs to wait >> for notification from the boss. > > If/when you upstream code that needs this interface, include this > patch as part of the series. As Siddharth pointed out, we avoid > merging code that has no upstream users. > > Bjorn I got it. Thank you Siddharth and Bjorn. Best regards Hans
diff --git a/drivers/pci/controller/cadence/pcie-cadence.c b/drivers/pci/controller/cadence/pcie-cadence.c index 204e045aed8c..ebb4a0130145 100644 --- a/drivers/pci/controller/cadence/pcie-cadence.c +++ b/drivers/pci/controller/cadence/pcie-cadence.c @@ -8,6 +8,86 @@ #include "pcie-cadence.h" +/* + * These interfaces resemble the pci_find_*capability() interfaces, but these + * are for configuring host controllers, which are bridges *to* PCI devices but + * are not PCI devices themselves. + */ +static u8 __cdns_pcie_find_next_cap(struct cdns_pcie *pcie, u8 cap_ptr, + u8 cap) +{ + u8 cap_id, next_cap_ptr; + u16 reg; + + if (!cap_ptr) + return 0; + + reg = cdns_pcie_readl(pcie, cap_ptr); + cap_id = (reg & 0x00ff); + + if (cap_id > PCI_CAP_ID_MAX) + return 0; + + if (cap_id == cap) + return cap_ptr; + + next_cap_ptr = (reg & 0xff00) >> 8; + return __cdns_pcie_find_next_cap(pcie, next_cap_ptr, cap); +} + +u8 cdns_pcie_find_capability(struct cdns_pcie *pcie, u8 cap) +{ + u8 next_cap_ptr; + u16 reg; + + reg = cdns_pcie_readl(pcie, PCI_CAPABILITY_LIST); + next_cap_ptr = (reg & 0x00ff); + + return __cdns_pcie_find_next_cap(pcie, next_cap_ptr, cap); +} +EXPORT_SYMBOL_GPL(cdns_pcie_find_capability); + +static u16 cdns_pcie_find_next_ext_capability(struct cdns_pcie *pcie, + u16 start, u8 cap) +{ + u32 header; + int ttl; + int pos = PCI_CFG_SPACE_SIZE; + + /* minimum 8 bytes per capability */ + ttl = (PCI_CFG_SPACE_EXP_SIZE - PCI_CFG_SPACE_SIZE) / 8; + + if (start) + pos = start; + + header = cdns_pcie_readl(pcie, pos); + /* + * If we have no capabilities, this is indicated by cap ID, + * cap version and next pointer all being 0. + */ + if (header == 0) + return 0; + + while (ttl-- > 0) { + if (PCI_EXT_CAP_ID(header) == cap && pos != start) + return pos; + + pos = PCI_EXT_CAP_NEXT(header); + if (pos < PCI_CFG_SPACE_SIZE) + break; + + header = cdns_pcie_readl(pcie, pos); + } + + return 0; +} + +u16 cdns_pcie_find_ext_capability(struct cdns_pcie *pcie, u8 cap) +{ + return cdns_pcie_find_next_ext_capability(pcie, 0, cap); +} +EXPORT_SYMBOL_GPL(cdns_pcie_find_ext_capability); + void cdns_pcie_detect_quiet_min_delay_set(struct cdns_pcie *pcie) { u32 delay = 0x3; diff --git a/drivers/pci/controller/cadence/pcie-cadence.h b/drivers/pci/controller/cadence/pcie-cadence.h index f5eeff834ec1..6f4981fccb94 100644 --- a/drivers/pci/controller/cadence/pcie-cadence.h +++ b/drivers/pci/controller/cadence/pcie-cadence.h @@ -557,6 +557,9 @@ static inline int cdns_pcie_ep_setup(struct cdns_pcie_ep *ep) } #endif +u8 cdns_pcie_find_capability(struct cdns_pcie *pcie, u8 cap); +u16 cdns_pcie_find_ext_capability(struct cdns_pcie *pcie, u8 cap); + void cdns_pcie_detect_quiet_min_delay_set(struct cdns_pcie *pcie); void cdns_pcie_set_outbound_region(struct cdns_pcie *pcie, u8 busnr, u8 fn,
Add configuration space capability search API using struct cdns_pcie* pointer. Similar patches below have been merged. commit 5b0841fa653f ("PCI: dwc: Add extended configuration space capability search API") commit 7a6854f6874f ("PCI: dwc: Move config space capability search API") Signed-off-by: Hans Zhang <18255117159@163.com> --- drivers/pci/controller/cadence/pcie-cadence.c | 80 +++++++++++++++++++ drivers/pci/controller/cadence/pcie-cadence.h | 3 + 2 files changed, 83 insertions(+)