Hi Lorenzo,
The Endpoint controller driver uses features member in 'struct pci_epc'
to advertise the list of supported features to the endpoint function
driver.
There are a few shortcomings with this approach.
*) Certain endpoint controllers support fixed size BAR (e.g. TI's
AM654 uses Designware configuration with fixed size BAR). The
size of each BARs cannot be passed to the endpoint function
driver.
*) Too many macros for handling EPC features.
(EPC_FEATURE_NO_LINKUP_NOTIFIER, EPC_FEATURE_BAR_MASK,
EPC_FEATURE_MSIX_AVAILABLE, EPC_FEATURE_SET_BAR,
EPC_FEATURE_GET_BAR)
*) Endpoint controllers are directly modifying struct pci_epc
members. (I have plans to move struct pci_epc to
drivers/pci/endpoint so that pci_epc members are referenced
only by endpoint core).
To overcome the above shortcomings, introduced pci_epc_get_features()
API, pci_epc_features structure and a ->get_features() callback.
Also added a patch to set BAR flags in pci_epf_alloc_space and
remove it from pci-epf-test function driver.
Changes from v1:
*) Fixed helper function to return '0' (or BAR_0) for any incorrect
values in reserved BAR.
*) Do not set_bar or alloc space for BARs if the BARs are reserved
*) Fix incorrect check of epc_features in pci_epf_test_bind
Tested on TI's DRA7xx platform and AM654 platform. Support for PCIe
in AM654 platform will be posted shortly.
Kishon Vijay Abraham I (15):
PCI: endpoint: Add new pci_epc_ops to get EPC features
PCI: dwc: Add ->get_features() callback function in dw_pcie_ep_ops
PCI: designware-plat: Populate ->get_features() dw_pcie_ep_ops
PCI: pci-dra7xx: Populate ->get_features() dw_pcie_ep_ops
PCI: rockchip: Populate ->get_features() dw_pcie_ep_ops
PCI: cadence: Populate ->get_features() cdns_pcie_epc_ops
PCI: endpoint: Add helper to get first unreserved BAR
PCI: endpoint: Fix pci_epf_alloc_space to set correct MEM TYPE flags
PCI: pci-epf-test: Remove setting epf_bar flags in function driver
PCI: pci-epf-test: Do not allocate next BARs memory if current BAR is
64Bit
PCI: pci-epf-test: Use pci_epc_get_features to get EPC features
PCI: cadence: Remove pci_epf_linkup from Cadence EP driver
PCI: rockchip: Remove pci_epf_linkup from Rockchip EP driver
PCI: designware-plat: Remove setting epc->features in Designware plat
EP driver
PCI: endpoint: Remove features member in struct pci_epc
drivers/pci/controller/dwc/pci-dra7xx.c | 13 +++
.../pci/controller/dwc/pcie-designware-ep.c | 12 +++
.../pci/controller/dwc/pcie-designware-plat.c | 17 +++-
drivers/pci/controller/dwc/pcie-designware.h | 1 +
drivers/pci/controller/pcie-cadence-ep.c | 25 ++---
drivers/pci/controller/pcie-rockchip-ep.c | 16 +++-
drivers/pci/endpoint/functions/pci-epf-test.c | 93 ++++++++++++-------
drivers/pci/endpoint/pci-epc-core.c | 53 +++++++++++
drivers/pci/endpoint/pci-epf-core.c | 4 +-
include/linux/pci-epc.h | 31 +++++--
10 files changed, 201 insertions(+), 64 deletions(-)
--
2.17.1
Add a new pci_epc_ops ->get_features() to get the features
supported by EPC. Since EPC can provide different features to
different functions, the ->get_features() ops takes _func_no_ as
an argument.
Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
drivers/pci/endpoint/pci-epc-core.c | 30 +++++++++++++++++++++++++++++
include/linux/pci-epc.h | 22 +++++++++++++++++++++
2 files changed, 52 insertions(+)
diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index 094dcc3203b8..5a099479d9ab 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -83,6 +83,36 @@ struct pci_epc *pci_epc_get(const char *epc_name)
}
EXPORT_SYMBOL_GPL(pci_epc_get);
+/**
+ * pci_epc_get_features() - get the features supported by EPC
+ * @epc: the features supported by *this* EPC device will be returned
+ * @func_no: the features supported by the EPC device specific to the
+ * endpoint function with func_no will be returned
+ *
+ * Invoke to get the features provided by the EPC which may be
+ * specific to an endpoint function. Returns pci_epc_features on success
+ * and NULL for any failures.
+ */
+const struct pci_epc_features *pci_epc_get_features(struct pci_epc *epc,
+ u8 func_no)
+{
+ const struct pci_epc_features *epc_features;
+ unsigned long flags;
+
+ if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions)
+ return NULL;
+
+ if (!epc->ops->get_features)
+ return NULL;
+
+ spin_lock_irqsave(&epc->lock, flags);
+ epc_features = epc->ops->get_features(epc, func_no);
+ spin_unlock_irqrestore(&epc->lock, flags);
+
+ return epc_features;
+}
+EXPORT_SYMBOL_GPL(pci_epc_get_features);
+
/**
* pci_epc_stop() - stop the PCI link
* @epc: the link of the EPC device that has to be stopped
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index 37dab8116901..79fbcf94e14d 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -59,6 +59,8 @@ struct pci_epc_ops {
enum pci_epc_irq_type type, u16 interrupt_num);
int (*start)(struct pci_epc *epc);
void (*stop)(struct pci_epc *epc);
+ const struct pci_epc_features* (*get_features)(struct pci_epc *epc,
+ u8 func_no);
struct module *owner;
};
@@ -100,6 +102,24 @@ struct pci_epc {
unsigned int features;
};
+/**
+ * struct pci_epc_features - features supported by a EPC device per function
+ * @linkup_notifier: indicate if the EPC device can notify EPF driver on link up
+ * @msi_capable: indicate if the endpoint function has MSI capability
+ * @msix_capable: indicate if the endpoint function has MSI-X capability
+ * @reserved_bar: bitmap to indicate reserved BAR unavailable to function driver
+ * @bar_fixed_64bit: bitmap to indicate fixed 64bit BARs
+ * @bar_fixed_size: Array specifying the size supported by each BAR
+ */
+struct pci_epc_features {
+ unsigned int linkup_notifier : 1;
+ unsigned int msi_capable : 1;
+ unsigned int msix_capable : 1;
+ u8 reserved_bar;
+ u8 bar_fixed_64bit;
+ u64 bar_fixed_size[BAR_5 + 1];
+};
+
#define EPC_FEATURE_NO_LINKUP_NOTIFIER BIT(0)
#define EPC_FEATURE_BAR_MASK (BIT(1) | BIT(2) | BIT(3))
#define EPC_FEATURE_MSIX_AVAILABLE BIT(4)
@@ -158,6 +178,8 @@ int pci_epc_raise_irq(struct pci_epc *epc, u8 func_no,
enum pci_epc_irq_type type, u16 interrupt_num);
int pci_epc_start(struct pci_epc *epc);
void pci_epc_stop(struct pci_epc *epc);
+const struct pci_epc_features *pci_epc_get_features(struct pci_epc *epc,
+ u8 func_no);
struct pci_epc *pci_epc_get(const char *epc_name);
void pci_epc_put(struct pci_epc *epc);
--
2.17.1
Populate ->get_features() dw_pcie_ep_ops to return the EPC features
supported by DRA7xx PCIe endpoint controller.
Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
drivers/pci/controller/dwc/pci-dra7xx.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/pci/controller/dwc/pci-dra7xx.c b/drivers/pci/controller/dwc/pci-dra7xx.c
index a32d6dde7a57..15620cfa617b 100644
--- a/drivers/pci/controller/dwc/pci-dra7xx.c
+++ b/drivers/pci/controller/dwc/pci-dra7xx.c
@@ -389,9 +389,22 @@ static int dra7xx_pcie_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
return 0;
}
+static const struct pci_epc_features dra7xx_pcie_epc_features = {
+ .linkup_notifier = true,
+ .msi_capable = true,
+ .msix_capable = false,
+};
+
+static const struct pci_epc_features*
+dra7xx_pcie_get_features(struct dw_pcie_ep *ep)
+{
+ return &dra7xx_pcie_epc_features;
+}
+
static struct dw_pcie_ep_ops pcie_ep_ops = {
.ep_init = dra7xx_pcie_ep_init,
.raise_irq = dra7xx_pcie_raise_irq,
+ .get_features = dra7xx_pcie_get_features,
};
static int __init dra7xx_add_pcie_ep(struct dra7xx_pcie *dra7xx,
--
2.17.1
Populate ->get_features() dw_pcie_ep_ops to return the EPC features
supported by Designware PCIe endpoint controller.
Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
drivers/pci/controller/dwc/pcie-designware-plat.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-designware-plat.c b/drivers/pci/controller/dwc/pcie-designware-plat.c
index c12bf794d69c..bd0516afc86f 100644
--- a/drivers/pci/controller/dwc/pcie-designware-plat.c
+++ b/drivers/pci/controller/dwc/pcie-designware-plat.c
@@ -100,9 +100,22 @@ static int dw_plat_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
return 0;
}
+static const struct pci_epc_features dw_plat_pcie_epc_features = {
+ .linkup_notifier = false,
+ .msi_capable = true,
+ .msix_capable = true,
+};
+
+static const struct pci_epc_features*
+dw_plat_pcie_get_features(struct dw_pcie_ep *ep)
+{
+ return &dw_plat_pcie_epc_features;
+}
+
static struct dw_pcie_ep_ops pcie_ep_ops = {
.ep_init = dw_plat_pcie_ep_init,
.raise_irq = dw_plat_pcie_ep_raise_irq,
+ .get_features = dw_plat_pcie_get_features,
};
static int dw_plat_add_pcie_port(struct dw_plat_pcie *dw_plat_pcie,
--
2.17.1
Add a helper function pci_epc_get_first_free_bar(), to get the first
unreserved BAR that can be used for endpoint function.
Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
drivers/pci/endpoint/pci-epc-core.c | 23 +++++++++++++++++++++++
include/linux/pci-epc.h | 2 ++
2 files changed, 25 insertions(+)
diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index 5a099479d9ab..e4712a0f249c 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -83,6 +83,29 @@ struct pci_epc *pci_epc_get(const char *epc_name)
}
EXPORT_SYMBOL_GPL(pci_epc_get);
+/**
+ * pci_epc_get_first_free_bar() - helper to get first unreserved BAR
+ * @epc_features: pci_epc_features structure that holds the reserved bar bitmap
+ *
+ * Invoke to get the first unreserved BAR that can be used for endpoint
+ * function. For any incorrect value in reserved_bar return '0'.
+ */
+unsigned int pci_epc_get_first_free_bar(const struct pci_epc_features
+ *epc_features)
+{
+ int free_bar;
+
+ if (!epc_features)
+ return 0;
+
+ free_bar = ffz(epc_features->reserved_bar);
+ if (free_bar > 5)
+ return 0;
+
+ return free_bar;
+}
+EXPORT_SYMBOL_GPL(pci_epc_get_first_free_bar);
+
/**
* pci_epc_get_features() - get the features supported by EPC
* @epc: the features supported by *this* EPC device will be returned
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index 79fbcf94e14d..94e1ecff98ce 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -180,6 +180,8 @@ int pci_epc_start(struct pci_epc *epc);
void pci_epc_stop(struct pci_epc *epc);
const struct pci_epc_features *pci_epc_get_features(struct pci_epc *epc,
u8 func_no);
+unsigned int pci_epc_get_first_free_bar(const struct pci_epc_features
+ *epc_features);
struct pci_epc *pci_epc_get(const char *epc_name);
void pci_epc_put(struct pci_epc *epc);
--
2.17.1
Populate ->get_features() dw_pcie_ep_ops to return the EPC features
supported by Rockchip PCIe endpoint controller.
Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
drivers/pci/controller/pcie-rockchip-ep.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index b8163c56a142..9b60ad323ac7 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -505,6 +505,18 @@ static int rockchip_pcie_ep_start(struct pci_epc *epc)
return 0;
}
+static const struct pci_epc_features rockchip_pcie_epc_features = {
+ .linkup_notifier = false,
+ .msi_capable = true,
+ .msix_capable = false,
+};
+
+static const struct pci_epc_features*
+rockchip_pcie_ep_get_features(struct pci_epc *epc, u8 func_no)
+{
+ return &rockchip_pcie_epc_features;
+}
+
static const struct pci_epc_ops rockchip_pcie_epc_ops = {
.write_header = rockchip_pcie_ep_write_header,
.set_bar = rockchip_pcie_ep_set_bar,
@@ -515,6 +527,7 @@ static const struct pci_epc_ops rockchip_pcie_epc_ops = {
.get_msi = rockchip_pcie_ep_get_msi,
.raise_irq = rockchip_pcie_ep_raise_irq,
.start = rockchip_pcie_ep_start,
+ .get_features = rockchip_pcie_ep_get_features,
};
static int rockchip_pcie_parse_ep_dt(struct rockchip_pcie *rockchip,
--
2.17.1
Each platform using Designware PCIe core can support different set of
endpoint features. Add a new callback function ->get_features() in
dw_pcie_ep_ops so that each platform using Designware PCIe core can
advertise its supported features to the endpoint function driver.
Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
drivers/pci/controller/dwc/pcie-designware-ep.c | 12 ++++++++++++
drivers/pci/controller/dwc/pcie-designware.h | 1 +
2 files changed, 13 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index a543c45c7224..7a2925a16ab8 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -355,6 +355,17 @@ static int dw_pcie_ep_start(struct pci_epc *epc)
return pci->ops->start_link(pci);
}
+static const struct pci_epc_features*
+dw_pcie_ep_get_features(struct pci_epc *epc, u8 func_no)
+{
+ struct dw_pcie_ep *ep = epc_get_drvdata(epc);
+
+ if (!ep->ops->get_features)
+ return NULL;
+
+ return ep->ops->get_features(ep);
+}
+
static const struct pci_epc_ops epc_ops = {
.write_header = dw_pcie_ep_write_header,
.set_bar = dw_pcie_ep_set_bar,
@@ -368,6 +379,7 @@ static const struct pci_epc_ops epc_ops = {
.raise_irq = dw_pcie_ep_raise_irq,
.start = dw_pcie_ep_start,
.stop = dw_pcie_ep_stop,
+ .get_features = dw_pcie_ep_get_features,
};
int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no)
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 9943d8c68335..1f56e6ae34ff 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -192,6 +192,7 @@ struct dw_pcie_ep_ops {
void (*ep_init)(struct dw_pcie_ep *ep);
int (*raise_irq)(struct dw_pcie_ep *ep, u8 func_no,
enum pci_epc_irq_type type, u16 interrupt_num);
+ const struct pci_epc_features* (*get_features)(struct dw_pcie_ep *ep);
};
struct dw_pcie_ep {
--
2.17.1
Populate ->get_features() dw_pcie_ep_ops to return the EPC features
supported by Cadence PCIe endpoint controller.
Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
drivers/pci/controller/pcie-cadence-ep.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/pci/controller/pcie-cadence-ep.c b/drivers/pci/controller/pcie-cadence-ep.c
index c3a088910f48..14c2545bb17e 100644
--- a/drivers/pci/controller/pcie-cadence-ep.c
+++ b/drivers/pci/controller/pcie-cadence-ep.c
@@ -411,6 +411,18 @@ static int cdns_pcie_ep_start(struct pci_epc *epc)
return 0;
}
+static const struct pci_epc_features cdns_pcie_epc_features = {
+ .linkup_notifier = false,
+ .msi_capable = true,
+ .msix_capable = false,
+};
+
+static const struct pci_epc_features*
+cdns_pcie_ep_get_features(struct pci_epc *epc, u8 func_no)
+{
+ return &cdns_pcie_epc_features;
+}
+
static const struct pci_epc_ops cdns_pcie_epc_ops = {
.write_header = cdns_pcie_ep_write_header,
.set_bar = cdns_pcie_ep_set_bar,
@@ -421,6 +433,7 @@ static const struct pci_epc_ops cdns_pcie_epc_ops = {
.get_msi = cdns_pcie_ep_get_msi,
.raise_irq = cdns_pcie_ep_raise_irq,
.start = cdns_pcie_ep_start,
+ .get_features = cdns_pcie_ep_get_features,
};
static const struct of_device_id cdns_pcie_ep_of_match[] = {
--
2.17.1
Now that pci-epf-test uses get_features callback and
dw_plat_pcie_epc_features in Designware plat EP driver already indicates
it doesn't support linkup notification and is MSIX capable, remove setting
epc->features which is not used anymore by the endpoint function driver.
Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
drivers/pci/controller/dwc/pcie-designware-plat.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-plat.c b/drivers/pci/controller/dwc/pcie-designware-plat.c
index bd0516afc86f..3be87126aef3 100644
--- a/drivers/pci/controller/dwc/pcie-designware-plat.c
+++ b/drivers/pci/controller/dwc/pcie-designware-plat.c
@@ -70,14 +70,10 @@ static const struct dw_pcie_ops dw_pcie_ops = {
static void dw_plat_pcie_ep_init(struct dw_pcie_ep *ep)
{
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
- struct pci_epc *epc = ep->epc;
enum pci_barno bar;
for (bar = BAR_0; bar <= BAR_5; bar++)
dw_pcie_ep_reset_bar(pci, bar);
-
- epc->features |= EPC_FEATURE_NO_LINKUP_NOTIFIER;
- epc->features |= EPC_FEATURE_MSIX_AVAILABLE;
}
static int dw_plat_pcie_ep_raise_irq(struct dw_pcie_ep *ep, u8 func_no,
--
2.17.1
pci_epf_alloc_space() sets the MEM TYPE flags to indicate a 32-bit
Base Address Register irrespective of the size. Fix it here to indicate
64-bit BAR if the size is > 2GB.
Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
drivers/pci/endpoint/pci-epf-core.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
index 825fa24427a3..8bfdcd291196 100644
--- a/drivers/pci/endpoint/pci-epf-core.c
+++ b/drivers/pci/endpoint/pci-epf-core.c
@@ -131,7 +131,9 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar)
epf->bar[bar].phys_addr = phys_addr;
epf->bar[bar].size = size;
epf->bar[bar].barno = bar;
- epf->bar[bar].flags = PCI_BASE_ADDRESS_SPACE_MEMORY;
+ epf->bar[bar].flags |= upper_32_bits(size) ?
+ PCI_BASE_ADDRESS_MEM_TYPE_64 :
+ PCI_BASE_ADDRESS_MEM_TYPE_32;
return space;
}
--
2.17.1
It's useless to allocate memory for next BAR if the current BAR is a
64Bit BAR. Stop allocating memory for the next BAR, if the current
BARs flag indicates this is a 64Bit BAR.
Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
drivers/pci/endpoint/functions/pci-epf-test.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 44cc31343a80..ade296180383 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -429,6 +429,7 @@ static int pci_epf_test_alloc_space(struct pci_epf *epf)
{
struct pci_epf_test *epf_test = epf_get_drvdata(epf);
struct device *dev = &epf->dev;
+ struct pci_epf_bar *epf_bar;
void *base;
int bar;
enum pci_barno test_reg_bar = epf_test->test_reg_bar;
@@ -442,6 +443,7 @@ static int pci_epf_test_alloc_space(struct pci_epf *epf)
epf_test->reg[test_reg_bar] = base;
for (bar = BAR_0; bar <= BAR_5; bar++) {
+ epf_bar = &epf->bar[bar];
if (bar == test_reg_bar)
continue;
base = pci_epf_alloc_space(epf, bar_size[bar], bar);
@@ -449,6 +451,8 @@ static int pci_epf_test_alloc_space(struct pci_epf *epf)
dev_err(dev, "Failed to allocate space for BAR%d\n",
bar);
epf_test->reg[bar] = base;
+ if (epf_bar->flags & PCI_BASE_ADDRESS_MEM_TYPE_64)
+ bar++;
}
return 0;
--
2.17.1
Now that pci_epf_alloc_space() sets BAR MEM TYPE flags as 64Bit or
32Bit based on size, remove setting it in function driver.
Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
drivers/pci/endpoint/functions/pci-epf-test.c | 4 ----
1 file changed, 4 deletions(-)
diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 3e86fa3c7da3..44cc31343a80 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -406,10 +406,6 @@ static int pci_epf_test_set_bar(struct pci_epf *epf)
for (bar = BAR_0; bar <= BAR_5; bar++) {
epf_bar = &epf->bar[bar];
- epf_bar->flags |= upper_32_bits(epf_bar->size) ?
- PCI_BASE_ADDRESS_MEM_TYPE_64 :
- PCI_BASE_ADDRESS_MEM_TYPE_32;
-
ret = pci_epc_set_bar(epc, epf->func_no, epf_bar);
if (ret) {
pci_epf_free_space(epf, epf_test->reg[bar], bar);
--
2.17.1
Since EPC features are now implemented using pci_epc_features and
all the EPC drivers are moved to using pci_epc_features, remove
features member in struct pci_epc and all the helper macros for
configuring the features.
Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
include/linux/pci-epc.h | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index 94e1ecff98ce..c3ffa3917f88 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -99,7 +99,6 @@ struct pci_epc {
struct config_group *group;
/* spinlock to protect against concurrent access of EP controller */
spinlock_t lock;
- unsigned int features;
};
/**
@@ -120,14 +119,6 @@ struct pci_epc_features {
u64 bar_fixed_size[BAR_5 + 1];
};
-#define EPC_FEATURE_NO_LINKUP_NOTIFIER BIT(0)
-#define EPC_FEATURE_BAR_MASK (BIT(1) | BIT(2) | BIT(3))
-#define EPC_FEATURE_MSIX_AVAILABLE BIT(4)
-#define EPC_FEATURE_SET_BAR(features, bar) \
- (features |= (EPC_FEATURE_BAR_MASK & (bar << 1)))
-#define EPC_FEATURE_GET_BAR(features) \
- ((features & EPC_FEATURE_BAR_MASK) >> 1)
-
#define to_pci_epc(device) container_of((device), struct pci_epc, dev)
#define pci_epc_create(dev, ops) \
--
2.17.1
pci_epf_linkup is intended to be invoked if the EPC supports linkup
notification. Now that pci-epf-test uses get_features callback, which
indicates Cadence EP driver doesn't support linkup notification, remove
pci_epf_linkup from Cadence EP driver.
Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
drivers/pci/controller/pcie-cadence-ep.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/drivers/pci/controller/pcie-cadence-ep.c b/drivers/pci/controller/pcie-cadence-ep.c
index 14c2545bb17e..def7820cb824 100644
--- a/drivers/pci/controller/pcie-cadence-ep.c
+++ b/drivers/pci/controller/pcie-cadence-ep.c
@@ -396,18 +396,6 @@ static int cdns_pcie_ep_start(struct pci_epc *epc)
cfg |= BIT(epf->func_no);
cdns_pcie_writel(pcie, CDNS_PCIE_LM_EP_FUNC_CFG, cfg);
- /*
- * The PCIe links are automatically established by the controller
- * once for all at powerup: the software can neither start nor stop
- * those links later at runtime.
- *
- * Then we only have to notify the EP core that our links are already
- * established. However we don't call directly pci_epc_linkup() because
- * we've already locked the epc->lock.
- */
- list_for_each_entry(epf, &epc->pci_epf, list)
- pci_epf_linkup(epf);
-
return 0;
}
--
2.17.1
Use pci_epc_get_features to get EPC features such as linkup
notifier support, MSI/MSIX capable, BAR configuration etc and use it
for configuring pci-epf-test. Since these features are now obtained
directly from EPC driver, remove pci_epf_test_data which was initially
added to have EPC features in endpoint function driver.
Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
drivers/pci/endpoint/functions/pci-epf-test.c | 87 ++++++++++++-------
1 file changed, 54 insertions(+), 33 deletions(-)
diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index ade296180383..2e0fb231ce0c 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -47,9 +47,8 @@ struct pci_epf_test {
void *reg[6];
struct pci_epf *epf;
enum pci_barno test_reg_bar;
- bool linkup_notifier;
- bool msix_available;
struct delayed_work cmd_handler;
+ const struct pci_epc_features *epc_features;
};
struct pci_epf_test_reg {
@@ -71,11 +70,6 @@ static struct pci_epf_header test_header = {
.interrupt_pin = PCI_INTERRUPT_INTA,
};
-struct pci_epf_test_data {
- enum pci_barno test_reg_bar;
- bool linkup_notifier;
-};
-
static size_t bar_size[] = { 512, 512, 1024, 16384, 131072, 1048576 };
static int pci_epf_test_copy(struct pci_epf_test *epf_test)
@@ -402,10 +396,16 @@ static int pci_epf_test_set_bar(struct pci_epf *epf)
struct device *dev = &epf->dev;
struct pci_epf_test *epf_test = epf_get_drvdata(epf);
enum pci_barno test_reg_bar = epf_test->test_reg_bar;
+ const struct pci_epc_features *epc_features;
+
+ epc_features = epf_test->epc_features;
for (bar = BAR_0; bar <= BAR_5; bar++) {
epf_bar = &epf->bar[bar];
+ if (!!(epc_features->reserved_bar & (1 << bar)))
+ continue;
+
ret = pci_epc_set_bar(epc, epf->func_no, epf_bar);
if (ret) {
pci_epf_free_space(epf, epf_test->reg[bar], bar);
@@ -433,6 +433,9 @@ static int pci_epf_test_alloc_space(struct pci_epf *epf)
void *base;
int bar;
enum pci_barno test_reg_bar = epf_test->test_reg_bar;
+ const struct pci_epc_features *epc_features;
+
+ epc_features = epf_test->epc_features;
base = pci_epf_alloc_space(epf, sizeof(struct pci_epf_test_reg),
test_reg_bar);
@@ -446,6 +449,10 @@ static int pci_epf_test_alloc_space(struct pci_epf *epf)
epf_bar = &epf->bar[bar];
if (bar == test_reg_bar)
continue;
+
+ if (!!(epc_features->reserved_bar & (1 << bar)))
+ continue;
+
base = pci_epf_alloc_space(epf, bar_size[bar], bar);
if (!base)
dev_err(dev, "Failed to allocate space for BAR%d\n",
@@ -458,25 +465,50 @@ static int pci_epf_test_alloc_space(struct pci_epf *epf)
return 0;
}
+static void pci_epf_configure_bar(struct pci_epf *epf,
+ const struct pci_epc_features *epc_features)
+{
+ struct pci_epf_bar *epf_bar;
+ bool bar_fixed_64bit;
+ int i;
+
+ for (i = BAR_0; i <= BAR_5; i++) {
+ epf_bar = &epf->bar[i];
+ bar_fixed_64bit = !!(epc_features->bar_fixed_64bit & (1 << i));
+ if (bar_fixed_64bit)
+ epf_bar->flags |= PCI_BASE_ADDRESS_MEM_TYPE_64;
+ if (epc_features->bar_fixed_size[i])
+ bar_size[i] = epc_features->bar_fixed_size[i];
+ }
+}
+
static int pci_epf_test_bind(struct pci_epf *epf)
{
int ret;
struct pci_epf_test *epf_test = epf_get_drvdata(epf);
struct pci_epf_header *header = epf->header;
+ const struct pci_epc_features *epc_features;
+ enum pci_barno test_reg_bar = BAR_0;
struct pci_epc *epc = epf->epc;
struct device *dev = &epf->dev;
+ bool linkup_notifier = false;
+ bool msix_capable = false;
+ bool msi_capable = true;
if (WARN_ON_ONCE(!epc))
return -EINVAL;
- if (epc->features & EPC_FEATURE_NO_LINKUP_NOTIFIER)
- epf_test->linkup_notifier = false;
- else
- epf_test->linkup_notifier = true;
-
- epf_test->msix_available = epc->features & EPC_FEATURE_MSIX_AVAILABLE;
+ epc_features = pci_epc_get_features(epc, epf->func_no);
+ if (epc_features) {
+ linkup_notifier = epc_features->linkup_notifier;
+ msix_capable = epc_features->msix_capable;
+ msi_capable = epc_features->msi_capable;
+ test_reg_bar = pci_epc_get_first_free_bar(epc_features);
+ pci_epf_configure_bar(epf, epc_features);
+ }
- epf_test->test_reg_bar = EPC_FEATURE_GET_BAR(epc->features);
+ epf_test->test_reg_bar = test_reg_bar;
+ epf_test->epc_features = epc_features;
ret = pci_epc_write_header(epc, epf->func_no, header);
if (ret) {
@@ -492,13 +524,15 @@ static int pci_epf_test_bind(struct pci_epf *epf)
if (ret)
return ret;
- ret = pci_epc_set_msi(epc, epf->func_no, epf->msi_interrupts);
- if (ret) {
- dev_err(dev, "MSI configuration failed\n");
- return ret;
+ if (msi_capable) {
+ ret = pci_epc_set_msi(epc, epf->func_no, epf->msi_interrupts);
+ if (ret) {
+ dev_err(dev, "MSI configuration failed\n");
+ return ret;
+ }
}
- if (epf_test->msix_available) {
+ if (msix_capable) {
ret = pci_epc_set_msix(epc, epf->func_no, epf->msix_interrupts);
if (ret) {
dev_err(dev, "MSI-X configuration failed\n");
@@ -506,7 +540,7 @@ static int pci_epf_test_bind(struct pci_epf *epf)
}
}
- if (!epf_test->linkup_notifier)
+ if (!linkup_notifier)
queue_work(kpcitest_workqueue, &epf_test->cmd_handler.work);
return 0;
@@ -523,17 +557,6 @@ static int pci_epf_test_probe(struct pci_epf *epf)
{
struct pci_epf_test *epf_test;
struct device *dev = &epf->dev;
- const struct pci_epf_device_id *match;
- struct pci_epf_test_data *data;
- enum pci_barno test_reg_bar = BAR_0;
- bool linkup_notifier = true;
-
- match = pci_epf_match_device(pci_epf_test_ids, epf);
- data = (struct pci_epf_test_data *)match->driver_data;
- if (data) {
- test_reg_bar = data->test_reg_bar;
- linkup_notifier = data->linkup_notifier;
- }
epf_test = devm_kzalloc(dev, sizeof(*epf_test), GFP_KERNEL);
if (!epf_test)
@@ -541,8 +564,6 @@ static int pci_epf_test_probe(struct pci_epf *epf)
epf->header = &test_header;
epf_test->epf = epf;
- epf_test->test_reg_bar = test_reg_bar;
- epf_test->linkup_notifier = linkup_notifier;
INIT_DELAYED_WORK(&epf_test->cmd_handler, pci_epf_test_cmd_handler);
--
2.17.1
pci_epf_linkup is intended to be invoked if the EPC supports linkup
notification. Now that pci-epf-test uses get_features callback, which
indicates Rockchip EP driver doesn't support linkup notification, remove
pci_epf_linkup from Rockchip EP driver.
Signed-off-by: Kishon Vijay Abraham I <[email protected]>
---
drivers/pci/controller/pcie-rockchip-ep.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/pci/controller/pcie-rockchip-ep.c b/drivers/pci/controller/pcie-rockchip-ep.c
index 9b60ad323ac7..a5d799e2dff2 100644
--- a/drivers/pci/controller/pcie-rockchip-ep.c
+++ b/drivers/pci/controller/pcie-rockchip-ep.c
@@ -499,9 +499,6 @@ static int rockchip_pcie_ep_start(struct pci_epc *epc)
rockchip_pcie_write(rockchip, cfg, PCIE_CORE_PHY_FUNC_CFG);
- list_for_each_entry(epf, &epc->pci_epf, list)
- pci_epf_linkup(epf);
-
return 0;
}
--
2.17.1
On 2019/1/14 19:14, Kishon Vijay Abraham I wrote:
> Hi Lorenzo,
>
> The Endpoint controller driver uses features member in 'struct pci_epc'
> to advertise the list of supported features to the endpoint function
> driver.
>
> There are a few shortcomings with this approach.
> *) Certain endpoint controllers support fixed size BAR (e.g. TI's
> AM654 uses Designware configuration with fixed size BAR). The
> size of each BARs cannot be passed to the endpoint function
> driver.
> *) Too many macros for handling EPC features.
> (EPC_FEATURE_NO_LINKUP_NOTIFIER, EPC_FEATURE_BAR_MASK,
> EPC_FEATURE_MSIX_AVAILABLE, EPC_FEATURE_SET_BAR,
> EPC_FEATURE_GET_BAR)
> *) Endpoint controllers are directly modifying struct pci_epc
> members. (I have plans to move struct pci_epc to
> drivers/pci/endpoint so that pci_epc members are referenced
> only by endpoint core).
>
> To overcome the above shortcomings, introduced pci_epc_get_features()
> API, pci_epc_features structure and a ->get_features() callback.
>
> Also added a patch to set BAR flags in pci_epf_alloc_space and
> remove it from pci-epf-test function driver.
>
> Changes from v1:
> *) Fixed helper function to return '0' (or BAR_0) for any incorrect
> values in reserved BAR.
> *) Do not set_bar or alloc space for BARs if the BARs are reserved
> *) Fix incorrect check of epc_features in pci_epf_test_bind
>
> Tested on TI's DRA7xx platform and AM654 platform. Support for PCIe
> in AM654 platform will be posted shortly.
>
----8<-----
> drivers/pci/controller/pcie-rockchip-ep.c | 16 +++-
Acked-by: Shawn Lin <[email protected]>
Hi,
On 14/01/2019 11:14, Kishon Vijay Abraham I wrote:
> Hi Lorenzo,
>
> The Endpoint controller driver uses features member in 'struct pci_epc'
> to advertise the list of supported features to the endpoint function
> driver.
>
> There are a few shortcomings with this approach.
> *) Certain endpoint controllers support fixed size BAR (e.g. TI's
> AM654 uses Designware configuration with fixed size BAR). The
> size of each BARs cannot be passed to the endpoint function
> driver.
> *) Too many macros for handling EPC features.
> (EPC_FEATURE_NO_LINKUP_NOTIFIER, EPC_FEATURE_BAR_MASK,
> EPC_FEATURE_MSIX_AVAILABLE, EPC_FEATURE_SET_BAR,
> EPC_FEATURE_GET_BAR)
> *) Endpoint controllers are directly modifying struct pci_epc
> members. (I have plans to move struct pci_epc to
> drivers/pci/endpoint so that pci_epc members are referenced
> only by endpoint core).
>
> To overcome the above shortcomings, introduced pci_epc_get_features()
> API, pci_epc_features structure and a ->get_features() callback.
>
> Also added a patch to set BAR flags in pci_epf_alloc_space and
> remove it from pci-epf-test function driver.
>
> Changes from v1:
> *) Fixed helper function to return '0' (or BAR_0) for any incorrect
> values in reserved BAR.
> *) Do not set_bar or alloc space for BARs if the BARs are reserved
> *) Fix incorrect check of epc_features in pci_epf_test_bind
>
> Tested on TI's DRA7xx platform and AM654 platform. Support for PCIe
> in AM654 platform will be posted shortly.
>
> Kishon Vijay Abraham I (15):
> PCI: endpoint: Add new pci_epc_ops to get EPC features
> PCI: dwc: Add ->get_features() callback function in dw_pcie_ep_ops
> PCI: designware-plat: Populate ->get_features() dw_pcie_ep_ops
> PCI: pci-dra7xx: Populate ->get_features() dw_pcie_ep_ops
> PCI: rockchip: Populate ->get_features() dw_pcie_ep_ops
> PCI: cadence: Populate ->get_features() cdns_pcie_epc_ops
> PCI: endpoint: Add helper to get first unreserved BAR
> PCI: endpoint: Fix pci_epf_alloc_space to set correct MEM TYPE flags
> PCI: pci-epf-test: Remove setting epf_bar flags in function driver
> PCI: pci-epf-test: Do not allocate next BARs memory if current BAR is
> 64Bit
> PCI: pci-epf-test: Use pci_epc_get_features to get EPC features
> PCI: cadence: Remove pci_epf_linkup from Cadence EP driver
> PCI: rockchip: Remove pci_epf_linkup from Rockchip EP driver
> PCI: designware-plat: Remove setting epc->features in Designware plat
> EP driver
> PCI: endpoint: Remove features member in struct pci_epc
>
> drivers/pci/controller/dwc/pci-dra7xx.c | 13 +++
> .../pci/controller/dwc/pcie-designware-ep.c | 12 +++
> .../pci/controller/dwc/pcie-designware-plat.c | 17 +++-
> drivers/pci/controller/dwc/pcie-designware.h | 1 +
> drivers/pci/controller/pcie-cadence-ep.c | 25 ++---
> drivers/pci/controller/pcie-rockchip-ep.c | 16 +++-
> drivers/pci/endpoint/functions/pci-epf-test.c | 93 ++++++++++++-------
> drivers/pci/endpoint/pci-epc-core.c | 53 +++++++++++
> drivers/pci/endpoint/pci-epf-core.c | 4 +-
> include/linux/pci-epc.h | 31 +++++--
> 10 files changed, 201 insertions(+), 64 deletions(-)
>
Sorry for the delay, I had a problem with my setup.
Tested-by: Gustavo Pimentel <[email protected]>
On 11/02/19 3:49 PM, Gustavo Pimentel wrote:
> Hi,
>
> On 14/01/2019 11:14, Kishon Vijay Abraham I wrote:
>> Hi Lorenzo,
>>
>> The Endpoint controller driver uses features member in 'struct pci_epc'
>> to advertise the list of supported features to the endpoint function
>> driver.
>>
>> There are a few shortcomings with this approach.
>> *) Certain endpoint controllers support fixed size BAR (e.g. TI's
>> AM654 uses Designware configuration with fixed size BAR). The
>> size of each BARs cannot be passed to the endpoint function
>> driver.
>> *) Too many macros for handling EPC features.
>> (EPC_FEATURE_NO_LINKUP_NOTIFIER, EPC_FEATURE_BAR_MASK,
>> EPC_FEATURE_MSIX_AVAILABLE, EPC_FEATURE_SET_BAR,
>> EPC_FEATURE_GET_BAR)
>> *) Endpoint controllers are directly modifying struct pci_epc
>> members. (I have plans to move struct pci_epc to
>> drivers/pci/endpoint so that pci_epc members are referenced
>> only by endpoint core).
>>
>> To overcome the above shortcomings, introduced pci_epc_get_features()
>> API, pci_epc_features structure and a ->get_features() callback.
>>
>> Also added a patch to set BAR flags in pci_epf_alloc_space and
>> remove it from pci-epf-test function driver.
>>
>> Changes from v1:
>> *) Fixed helper function to return '0' (or BAR_0) for any incorrect
>> values in reserved BAR.
>> *) Do not set_bar or alloc space for BARs if the BARs are reserved
>> *) Fix incorrect check of epc_features in pci_epf_test_bind
>>
>> Tested on TI's DRA7xx platform and AM654 platform. Support for PCIe
>> in AM654 platform will be posted shortly.
>>
>> Kishon Vijay Abraham I (15):
>> PCI: endpoint: Add new pci_epc_ops to get EPC features
>> PCI: dwc: Add ->get_features() callback function in dw_pcie_ep_ops
>> PCI: designware-plat: Populate ->get_features() dw_pcie_ep_ops
>> PCI: pci-dra7xx: Populate ->get_features() dw_pcie_ep_ops
>> PCI: rockchip: Populate ->get_features() dw_pcie_ep_ops
>> PCI: cadence: Populate ->get_features() cdns_pcie_epc_ops
>> PCI: endpoint: Add helper to get first unreserved BAR
>> PCI: endpoint: Fix pci_epf_alloc_space to set correct MEM TYPE flags
>> PCI: pci-epf-test: Remove setting epf_bar flags in function driver
>> PCI: pci-epf-test: Do not allocate next BARs memory if current BAR is
>> 64Bit
>> PCI: pci-epf-test: Use pci_epc_get_features to get EPC features
>> PCI: cadence: Remove pci_epf_linkup from Cadence EP driver
>> PCI: rockchip: Remove pci_epf_linkup from Rockchip EP driver
>> PCI: designware-plat: Remove setting epc->features in Designware plat
>> EP driver
>> PCI: endpoint: Remove features member in struct pci_epc
>>
>> drivers/pci/controller/dwc/pci-dra7xx.c | 13 +++
>> .../pci/controller/dwc/pcie-designware-ep.c | 12 +++
>> .../pci/controller/dwc/pcie-designware-plat.c | 17 +++-
>> drivers/pci/controller/dwc/pcie-designware.h | 1 +
>> drivers/pci/controller/pcie-cadence-ep.c | 25 ++---
>> drivers/pci/controller/pcie-rockchip-ep.c | 16 +++-
>> drivers/pci/endpoint/functions/pci-epf-test.c | 93 ++++++++++++-------
>> drivers/pci/endpoint/pci-epc-core.c | 53 +++++++++++
>> drivers/pci/endpoint/pci-epf-core.c | 4 +-
>> include/linux/pci-epc.h | 31 +++++--
>> 10 files changed, 201 insertions(+), 64 deletions(-)
>>
>
> Sorry for the delay, I had a problem with my setup.
>
> Tested-by: Gustavo Pimentel <[email protected]>
Thank you Gustavo!
-Kishon
On Mon, Jan 14, 2019 at 04:45:06PM +0530, Kishon Vijay Abraham I wrote:
> pci_epf_alloc_space() sets the MEM TYPE flags to indicate a 32-bit
> Base Address Register irrespective of the size. Fix it here to indicate
> 64-bit BAR if the size is > 2GB.
>
> Signed-off-by: Kishon Vijay Abraham I <[email protected]>
> ---
> drivers/pci/endpoint/pci-epf-core.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
This looks like a fix and should me marked as such. Does it work
as as standalone patch if it gets backported ?
Lorenzo
> diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
> index 825fa24427a3..8bfdcd291196 100644
> --- a/drivers/pci/endpoint/pci-epf-core.c
> +++ b/drivers/pci/endpoint/pci-epf-core.c
> @@ -131,7 +131,9 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar)
> epf->bar[bar].phys_addr = phys_addr;
> epf->bar[bar].size = size;
> epf->bar[bar].barno = bar;
> - epf->bar[bar].flags = PCI_BASE_ADDRESS_SPACE_MEMORY;
> + epf->bar[bar].flags |= upper_32_bits(size) ?
> + PCI_BASE_ADDRESS_MEM_TYPE_64 :
> + PCI_BASE_ADDRESS_MEM_TYPE_32;
>
> return space;
> }
> --
> 2.17.1
>
Hi Lorenzo,
On 11/02/19 11:07 PM, Lorenzo Pieralisi wrote:
> On Mon, Jan 14, 2019 at 04:45:06PM +0530, Kishon Vijay Abraham I wrote:
>> pci_epf_alloc_space() sets the MEM TYPE flags to indicate a 32-bit
>> Base Address Register irrespective of the size. Fix it here to indicate
>> 64-bit BAR if the size is > 2GB.
>>
>> Signed-off-by: Kishon Vijay Abraham I <[email protected]>
>> ---
>> drivers/pci/endpoint/pci-epf-core.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> This looks like a fix and should me marked as such. Does it work
> as as standalone patch if it gets backported ?
Yeah, it should work. But the current users doesn't allocate > 2GB and some
EPC drivers configure their registers based on size. So nothing is broken
without this patch as such.
Thanks
Kishon
On Wed, Feb 13, 2019 at 07:17:14PM +0530, Kishon Vijay Abraham I wrote:
> Hi Lorenzo,
>
> On 11/02/19 11:07 PM, Lorenzo Pieralisi wrote:
> > On Mon, Jan 14, 2019 at 04:45:06PM +0530, Kishon Vijay Abraham I wrote:
> >> pci_epf_alloc_space() sets the MEM TYPE flags to indicate a 32-bit
> >> Base Address Register irrespective of the size. Fix it here to indicate
> >> 64-bit BAR if the size is > 2GB.
> >>
> >> Signed-off-by: Kishon Vijay Abraham I <[email protected]>
> >> ---
> >> drivers/pci/endpoint/pci-epf-core.c | 4 +++-
> >> 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > This looks like a fix and should me marked as such. Does it work
> > as as standalone patch if it gets backported ?
>
> Yeah, it should work. But the current users doesn't allocate > 2GB and some
> EPC drivers configure their registers based on size. So nothing is broken
> without this patch as such.
I suspect you mean 4GB (here and the commit log), right ? I am checking
the commit logs, aiming at merging the patches.
Lorenzo
Hi Lorenzo,
On 14/02/19 9:59 PM, Lorenzo Pieralisi wrote:
> On Wed, Feb 13, 2019 at 07:17:14PM +0530, Kishon Vijay Abraham I wrote:
>> Hi Lorenzo,
>>
>> On 11/02/19 11:07 PM, Lorenzo Pieralisi wrote:
>>> On Mon, Jan 14, 2019 at 04:45:06PM +0530, Kishon Vijay Abraham I wrote:
>>>> pci_epf_alloc_space() sets the MEM TYPE flags to indicate a 32-bit
>>>> Base Address Register irrespective of the size. Fix it here to indicate
>>>> 64-bit BAR if the size is > 2GB.
>>>>
>>>> Signed-off-by: Kishon Vijay Abraham I <[email protected]>
>>>> ---
>>>> drivers/pci/endpoint/pci-epf-core.c | 4 +++-
>>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> This looks like a fix and should me marked as such. Does it work
>>> as as standalone patch if it gets backported ?
>>
>> Yeah, it should work. But the current users doesn't allocate > 2GB and some
>> EPC drivers configure their registers based on size. So nothing is broken
>> without this patch as such.
>
> I suspect you mean 4GB (here and the commit log), right ? I am checking
> the commit logs, aiming at merging the patches.
A 32bit BAR register can support a 'size' of only up to 2GB. Though it can hold
a memory address of up to 4GB.
This is also mentioned in the PCI Local Bus Specification.
"A 32-bit register can be implemented to support a single memory size that is a
power of 2 from 16 bytes to 2 GB"
Thanks
Kishon
On Fri, Feb 15, 2019 at 11:49:12AM +0530, Kishon Vijay Abraham I wrote:
> Hi Lorenzo,
>
> On 14/02/19 9:59 PM, Lorenzo Pieralisi wrote:
> > On Wed, Feb 13, 2019 at 07:17:14PM +0530, Kishon Vijay Abraham I wrote:
> >> Hi Lorenzo,
> >>
> >> On 11/02/19 11:07 PM, Lorenzo Pieralisi wrote:
> >>> On Mon, Jan 14, 2019 at 04:45:06PM +0530, Kishon Vijay Abraham I wrote:
> >>>> pci_epf_alloc_space() sets the MEM TYPE flags to indicate a 32-bit
> >>>> Base Address Register irrespective of the size. Fix it here to indicate
> >>>> 64-bit BAR if the size is > 2GB.
> >>>>
> >>>> Signed-off-by: Kishon Vijay Abraham I <[email protected]>
> >>>> ---
> >>>> drivers/pci/endpoint/pci-epf-core.c | 4 +++-
> >>>> 1 file changed, 3 insertions(+), 1 deletion(-)
> >>>
> >>> This looks like a fix and should me marked as such. Does it work
> >>> as as standalone patch if it gets backported ?
> >>
> >> Yeah, it should work. But the current users doesn't allocate > 2GB and some
> >> EPC drivers configure their registers based on size. So nothing is broken
> >> without this patch as such.
> >
> > I suspect you mean 4GB (here and the commit log), right ? I am checking
> > the commit logs, aiming at merging the patches.
>
> A 32bit BAR register can support a 'size' of only up to 2GB. Though it
> can hold a memory address of up to 4GB.
>
> This is also mentioned in the PCI Local Bus Specification. "A 32-bit
> register can be implemented to support a single memory size that is a
> power of 2 from 16 bytes to 2 GB"
Very true - sorry for the noise.
Lorenz,o
On Mon, Jan 14, 2019 at 04:44:58PM +0530, Kishon Vijay Abraham I wrote:
> Hi Lorenzo,
>
> The Endpoint controller driver uses features member in 'struct pci_epc'
> to advertise the list of supported features to the endpoint function
> driver.
>
> There are a few shortcomings with this approach.
> *) Certain endpoint controllers support fixed size BAR (e.g. TI's
> AM654 uses Designware configuration with fixed size BAR). The
> size of each BARs cannot be passed to the endpoint function
> driver.
> *) Too many macros for handling EPC features.
> (EPC_FEATURE_NO_LINKUP_NOTIFIER, EPC_FEATURE_BAR_MASK,
> EPC_FEATURE_MSIX_AVAILABLE, EPC_FEATURE_SET_BAR,
> EPC_FEATURE_GET_BAR)
> *) Endpoint controllers are directly modifying struct pci_epc
> members. (I have plans to move struct pci_epc to
> drivers/pci/endpoint so that pci_epc members are referenced
> only by endpoint core).
>
> To overcome the above shortcomings, introduced pci_epc_get_features()
> API, pci_epc_features structure and a ->get_features() callback.
>
> Also added a patch to set BAR flags in pci_epf_alloc_space and
> remove it from pci-epf-test function driver.
>
> Changes from v1:
> *) Fixed helper function to return '0' (or BAR_0) for any incorrect
> values in reserved BAR.
> *) Do not set_bar or alloc space for BARs if the BARs are reserved
> *) Fix incorrect check of epc_features in pci_epf_test_bind
>
> Tested on TI's DRA7xx platform and AM654 platform. Support for PCIe
> in AM654 platform will be posted shortly.
>
> Kishon Vijay Abraham I (15):
> PCI: endpoint: Add new pci_epc_ops to get EPC features
> PCI: dwc: Add ->get_features() callback function in dw_pcie_ep_ops
> PCI: designware-plat: Populate ->get_features() dw_pcie_ep_ops
> PCI: pci-dra7xx: Populate ->get_features() dw_pcie_ep_ops
> PCI: rockchip: Populate ->get_features() dw_pcie_ep_ops
> PCI: cadence: Populate ->get_features() cdns_pcie_epc_ops
> PCI: endpoint: Add helper to get first unreserved BAR
> PCI: endpoint: Fix pci_epf_alloc_space to set correct MEM TYPE flags
> PCI: pci-epf-test: Remove setting epf_bar flags in function driver
> PCI: pci-epf-test: Do not allocate next BARs memory if current BAR is
> 64Bit
> PCI: pci-epf-test: Use pci_epc_get_features to get EPC features
> PCI: cadence: Remove pci_epf_linkup from Cadence EP driver
> PCI: rockchip: Remove pci_epf_linkup from Rockchip EP driver
> PCI: designware-plat: Remove setting epc->features in Designware plat
> EP driver
> PCI: endpoint: Remove features member in struct pci_epc
>
> drivers/pci/controller/dwc/pci-dra7xx.c | 13 +++
> .../pci/controller/dwc/pcie-designware-ep.c | 12 +++
> .../pci/controller/dwc/pcie-designware-plat.c | 17 +++-
> drivers/pci/controller/dwc/pcie-designware.h | 1 +
> drivers/pci/controller/pcie-cadence-ep.c | 25 ++---
> drivers/pci/controller/pcie-rockchip-ep.c | 16 +++-
> drivers/pci/endpoint/functions/pci-epf-test.c | 93 ++++++++++++-------
> drivers/pci/endpoint/pci-epc-core.c | 53 +++++++++++
> drivers/pci/endpoint/pci-epf-core.c | 4 +-
> include/linux/pci-epc.h | 31 +++++--
> 10 files changed, 201 insertions(+), 64 deletions(-)
Applied to pci/endpoint for v5.1, thanks !
Lorenzo