* [PATCH 1/4] PCI: dwc: Add new feature to skip core initialization
2019-11-13 9:08 [PATCH 0/4] Add support to defer core initialization Vidya Sagar
@ 2019-11-13 9:08 ` Vidya Sagar
2019-11-27 8:14 ` Kishon Vijay Abraham I
` (2 more replies)
2019-11-13 9:08 ` [PATCH 2/4] PCI: endpoint: Add notification for core init completion Vidya Sagar
` (3 subsequent siblings)
4 siblings, 3 replies; 23+ messages in thread
From: Vidya Sagar @ 2019-11-13 9:08 UTC (permalink / raw)
To: jingoohan1, gustavo.pimentel, lorenzo.pieralisi, andrew.murray,
bhelgaas, kishon, thierry.reding
Cc: Jisheng.Zhang, jonathanh, linux-pci, linux-kernel, kthota,
mmaddireddy, vidyas, sagar.tv
Add a new feature 'skip_core_init' that can be set by platform drivers
of devices that do not have their core registers available until reference
clock from host is available (Ex:- Tegra194) to indicate DesignWare
endpoint mode sub-system to not perform core registers initialization.
Existing dw_pcie_ep_init() is refactored and all the code that touches
registers is extracted to form a new API dw_pcie_ep_init_complete() that
can be called later by platform drivers setting 'skip_core_init' to '1'.
Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
.../pci/controller/dwc/pcie-designware-ep.c | 72 +++++++++++--------
drivers/pci/controller/dwc/pcie-designware.h | 6 ++
include/linux/pci-epc.h | 1 +
3 files changed, 51 insertions(+), 28 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 3dd2e2697294..06f4379be8a3 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -492,19 +492,53 @@ static unsigned int dw_pcie_ep_find_ext_capability(struct dw_pcie *pci, int cap)
return 0;
}
-int dw_pcie_ep_init(struct dw_pcie_ep *ep)
+int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
{
+ struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+ unsigned int offset;
+ unsigned int nbars;
+ u8 hdr_type;
+ u32 reg;
int i;
+
+ hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE);
+ if (hdr_type != PCI_HEADER_TYPE_NORMAL) {
+ dev_err(pci->dev,
+ "PCIe controller is not set to EP mode (hdr_type:0x%x)!\n",
+ hdr_type);
+ return -EIO;
+ }
+
+ ep->msi_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
+
+ ep->msix_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSIX);
+
+ offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
+ if (offset) {
+ reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
+ nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
+ PCI_REBAR_CTRL_NBAR_SHIFT;
+
+ dw_pcie_dbi_ro_wr_en(pci);
+ for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
+ dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
+ dw_pcie_dbi_ro_wr_dis(pci);
+ }
+
+ dw_pcie_setup(pci);
+
+ return 0;
+}
+
+int dw_pcie_ep_init(struct dw_pcie_ep *ep)
+{
int ret;
- u32 reg;
void *addr;
- u8 hdr_type;
- unsigned int nbars;
- unsigned int offset;
struct pci_epc *epc;
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
struct device *dev = pci->dev;
struct device_node *np = dev->of_node;
+ const struct pci_epc_features *epc_features;
if (!pci->dbi_base || !pci->dbi_base2) {
dev_err(dev, "dbi_base/dbi_base2 is not populated\n");
@@ -563,13 +597,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
if (ep->ops->ep_init)
ep->ops->ep_init(ep);
- hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE);
- if (hdr_type != PCI_HEADER_TYPE_NORMAL) {
- dev_err(pci->dev, "PCIe controller is not set to EP mode (hdr_type:0x%x)!\n",
- hdr_type);
- return -EIO;
- }
-
ret = of_property_read_u8(np, "max-functions", &epc->max_functions);
if (ret < 0)
epc->max_functions = 1;
@@ -587,23 +614,12 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
dev_err(dev, "Failed to reserve memory for MSI/MSI-X\n");
return -ENOMEM;
}
- ep->msi_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
- ep->msix_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSIX);
-
- offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
- if (offset) {
- reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
- nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
- PCI_REBAR_CTRL_NBAR_SHIFT;
-
- dw_pcie_dbi_ro_wr_en(pci);
- for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
- dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
- dw_pcie_dbi_ro_wr_dis(pci);
+ if (ep->ops->get_features) {
+ epc_features = ep->ops->get_features(ep);
+ if (epc_features->skip_core_init)
+ return 0;
}
- dw_pcie_setup(pci);
-
- return 0;
+ return dw_pcie_ep_init_complete(ep);
}
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 5accdd6bc388..340783e9032e 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -399,6 +399,7 @@ static inline int dw_pcie_allocate_domains(struct pcie_port *pp)
#ifdef CONFIG_PCIE_DW_EP
void dw_pcie_ep_linkup(struct dw_pcie_ep *ep);
int dw_pcie_ep_init(struct dw_pcie_ep *ep);
+int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep);
void dw_pcie_ep_exit(struct dw_pcie_ep *ep);
int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no);
int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
@@ -416,6 +417,11 @@ static inline int dw_pcie_ep_init(struct dw_pcie_ep *ep)
return 0;
}
+static inline int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
+{
+ return 0;
+}
+
static inline void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
{
}
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index 36644ccd32ac..241e6a6f39fb 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -121,6 +121,7 @@ struct pci_epc_features {
u8 bar_fixed_64bit;
u64 bar_fixed_size[PCI_STD_NUM_BARS];
size_t align;
+ bool skip_core_init;
};
#define to_pci_epc(device) container_of((device), struct pci_epc, dev)
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 1/4] PCI: dwc: Add new feature to skip core initialization
2019-11-13 9:08 ` [PATCH 1/4] PCI: dwc: Add new feature to skip " Vidya Sagar
@ 2019-11-27 8:14 ` Kishon Vijay Abraham I
2019-11-27 8:40 ` Vidya Sagar
2019-11-27 9:48 ` Christoph Hellwig
2019-12-05 10:04 ` Kishon Vijay Abraham I
2 siblings, 1 reply; 23+ messages in thread
From: Kishon Vijay Abraham I @ 2019-11-27 8:14 UTC (permalink / raw)
To: Vidya Sagar, jingoohan1, gustavo.pimentel, lorenzo.pieralisi,
andrew.murray, bhelgaas, thierry.reding
Cc: Jisheng.Zhang, jonathanh, linux-pci, linux-kernel, kthota,
mmaddireddy, sagar.tv
Hi,
On 13/11/19 2:38 PM, Vidya Sagar wrote:
> Add a new feature 'skip_core_init' that can be set by platform drivers
> of devices that do not have their core registers available until reference
> clock from host is available (Ex:- Tegra194) to indicate DesignWare
> endpoint mode sub-system to not perform core registers initialization.
> Existing dw_pcie_ep_init() is refactored and all the code that touches
> registers is extracted to form a new API dw_pcie_ep_init_complete() that
> can be called later by platform drivers setting 'skip_core_init' to '1'.
>
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> ---
> .../pci/controller/dwc/pcie-designware-ep.c | 72 +++++++++++--------
> drivers/pci/controller/dwc/pcie-designware.h | 6 ++
> include/linux/pci-epc.h | 1 +
> 3 files changed, 51 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 3dd2e2697294..06f4379be8a3 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -492,19 +492,53 @@ static unsigned int dw_pcie_ep_find_ext_capability(struct dw_pcie *pci, int cap)
> return 0;
> }
>
> -int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> +int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
> {
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> + unsigned int offset;
> + unsigned int nbars;
> + u8 hdr_type;
> + u32 reg;
> int i;
> +
> + hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE);
> + if (hdr_type != PCI_HEADER_TYPE_NORMAL) {
> + dev_err(pci->dev,
> + "PCIe controller is not set to EP mode (hdr_type:0x%x)!\n",
> + hdr_type);
> + return -EIO;
> + }
> +
> + ep->msi_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
> +
> + ep->msix_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSIX);
> +
> + offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
> + if (offset) {
> + reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
> + nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
> + PCI_REBAR_CTRL_NBAR_SHIFT;
> +
> + dw_pcie_dbi_ro_wr_en(pci);
> + for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
> + dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
> + dw_pcie_dbi_ro_wr_dis(pci);
> + }
> +
> + dw_pcie_setup(pci);
> +
> + return 0;
> +}
> +
> +int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> +{
> int ret;
> - u32 reg;
> void *addr;
> - u8 hdr_type;
> - unsigned int nbars;
> - unsigned int offset;
> struct pci_epc *epc;
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> struct device *dev = pci->dev;
> struct device_node *np = dev->of_node;
> + const struct pci_epc_features *epc_features;
>
> if (!pci->dbi_base || !pci->dbi_base2) {
> dev_err(dev, "dbi_base/dbi_base2 is not populated\n");
> @@ -563,13 +597,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> if (ep->ops->ep_init)
> ep->ops->ep_init(ep);
>
> - hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE);
> - if (hdr_type != PCI_HEADER_TYPE_NORMAL) {
> - dev_err(pci->dev, "PCIe controller is not set to EP mode (hdr_type:0x%x)!\n",
> - hdr_type);
> - return -EIO;
> - }
> -
> ret = of_property_read_u8(np, "max-functions", &epc->max_functions);
> if (ret < 0)
> epc->max_functions = 1;
> @@ -587,23 +614,12 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> dev_err(dev, "Failed to reserve memory for MSI/MSI-X\n");
> return -ENOMEM;
> }
> - ep->msi_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
>
> - ep->msix_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSIX);
> -
> - offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
> - if (offset) {
> - reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
> - nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
> - PCI_REBAR_CTRL_NBAR_SHIFT;
> -
> - dw_pcie_dbi_ro_wr_en(pci);
> - for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
> - dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
> - dw_pcie_dbi_ro_wr_dis(pci);
> + if (ep->ops->get_features) {
> + epc_features = ep->ops->get_features(ep);
> + if (epc_features->skip_core_init)
> + return 0;
> }
>
> - dw_pcie_setup(pci);
> -
> - return 0;
> + return dw_pcie_ep_init_complete(ep);
> }
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 5accdd6bc388..340783e9032e 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -399,6 +399,7 @@ static inline int dw_pcie_allocate_domains(struct pcie_port *pp)
> #ifdef CONFIG_PCIE_DW_EP
> void dw_pcie_ep_linkup(struct dw_pcie_ep *ep);
> int dw_pcie_ep_init(struct dw_pcie_ep *ep);
> +int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep);
> void dw_pcie_ep_exit(struct dw_pcie_ep *ep);
> int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no);
> int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
> @@ -416,6 +417,11 @@ static inline int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> return 0;
> }
>
> +static inline int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
> +{
> + return 0;
> +}
> +
> static inline void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
> {
> }
> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> index 36644ccd32ac..241e6a6f39fb 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -121,6 +121,7 @@ struct pci_epc_features {
> u8 bar_fixed_64bit;
> u64 bar_fixed_size[PCI_STD_NUM_BARS];
> size_t align;
> + bool skip_core_init;
This looks more like a designware specific change. Why is it added to the core
pci_epc_features?
Thanks
Kishon
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/4] PCI: dwc: Add new feature to skip core initialization
2019-11-27 8:14 ` Kishon Vijay Abraham I
@ 2019-11-27 8:40 ` Vidya Sagar
2019-11-27 9:18 ` Kishon Vijay Abraham I
0 siblings, 1 reply; 23+ messages in thread
From: Vidya Sagar @ 2019-11-27 8:40 UTC (permalink / raw)
To: Kishon Vijay Abraham I, jingoohan1, gustavo.pimentel,
lorenzo.pieralisi, andrew.murray, bhelgaas, thierry.reding
Cc: Jisheng.Zhang, jonathanh, linux-pci, linux-kernel, kthota,
mmaddireddy, sagar.tv
On 11/27/2019 1:44 PM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On 13/11/19 2:38 PM, Vidya Sagar wrote:
>> Add a new feature 'skip_core_init' that can be set by platform drivers
>> of devices that do not have their core registers available until reference
>> clock from host is available (Ex:- Tegra194) to indicate DesignWare
>> endpoint mode sub-system to not perform core registers initialization.
>> Existing dw_pcie_ep_init() is refactored and all the code that touches
>> registers is extracted to form a new API dw_pcie_ep_init_complete() that
>> can be called later by platform drivers setting 'skip_core_init' to '1'.
>>
>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>> ---
>> .../pci/controller/dwc/pcie-designware-ep.c | 72 +++++++++++--------
>> drivers/pci/controller/dwc/pcie-designware.h | 6 ++
>> include/linux/pci-epc.h | 1 +
>> 3 files changed, 51 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
>> index 3dd2e2697294..06f4379be8a3 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
>> @@ -492,19 +492,53 @@ static unsigned int dw_pcie_ep_find_ext_capability(struct dw_pcie *pci, int cap)
>> return 0;
>> }
>>
>> -int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>> +int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
>> {
>> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>> + unsigned int offset;
>> + unsigned int nbars;
>> + u8 hdr_type;
>> + u32 reg;
>> int i;
>> +
>> + hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE);
>> + if (hdr_type != PCI_HEADER_TYPE_NORMAL) {
>> + dev_err(pci->dev,
>> + "PCIe controller is not set to EP mode (hdr_type:0x%x)!\n",
>> + hdr_type);
>> + return -EIO;
>> + }
>> +
>> + ep->msi_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
>> +
>> + ep->msix_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSIX);
>> +
>> + offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
>> + if (offset) {
>> + reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
>> + nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
>> + PCI_REBAR_CTRL_NBAR_SHIFT;
>> +
>> + dw_pcie_dbi_ro_wr_en(pci);
>> + for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
>> + dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
>> + dw_pcie_dbi_ro_wr_dis(pci);
>> + }
>> +
>> + dw_pcie_setup(pci);
>> +
>> + return 0;
>> +}
>> +
>> +int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>> +{
>> int ret;
>> - u32 reg;
>> void *addr;
>> - u8 hdr_type;
>> - unsigned int nbars;
>> - unsigned int offset;
>> struct pci_epc *epc;
>> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>> struct device *dev = pci->dev;
>> struct device_node *np = dev->of_node;
>> + const struct pci_epc_features *epc_features;
>>
>> if (!pci->dbi_base || !pci->dbi_base2) {
>> dev_err(dev, "dbi_base/dbi_base2 is not populated\n");
>> @@ -563,13 +597,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>> if (ep->ops->ep_init)
>> ep->ops->ep_init(ep);
>>
>> - hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE);
>> - if (hdr_type != PCI_HEADER_TYPE_NORMAL) {
>> - dev_err(pci->dev, "PCIe controller is not set to EP mode (hdr_type:0x%x)!\n",
>> - hdr_type);
>> - return -EIO;
>> - }
>> -
>> ret = of_property_read_u8(np, "max-functions", &epc->max_functions);
>> if (ret < 0)
>> epc->max_functions = 1;
>> @@ -587,23 +614,12 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>> dev_err(dev, "Failed to reserve memory for MSI/MSI-X\n");
>> return -ENOMEM;
>> }
>> - ep->msi_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
>>
>> - ep->msix_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSIX);
>> -
>> - offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
>> - if (offset) {
>> - reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
>> - nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
>> - PCI_REBAR_CTRL_NBAR_SHIFT;
>> -
>> - dw_pcie_dbi_ro_wr_en(pci);
>> - for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
>> - dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
>> - dw_pcie_dbi_ro_wr_dis(pci);
>> + if (ep->ops->get_features) {
>> + epc_features = ep->ops->get_features(ep);
>> + if (epc_features->skip_core_init)
>> + return 0;
>> }
>>
>> - dw_pcie_setup(pci);
>> -
>> - return 0;
>> + return dw_pcie_ep_init_complete(ep);
>> }
>> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
>> index 5accdd6bc388..340783e9032e 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware.h
>> +++ b/drivers/pci/controller/dwc/pcie-designware.h
>> @@ -399,6 +399,7 @@ static inline int dw_pcie_allocate_domains(struct pcie_port *pp)
>> #ifdef CONFIG_PCIE_DW_EP
>> void dw_pcie_ep_linkup(struct dw_pcie_ep *ep);
>> int dw_pcie_ep_init(struct dw_pcie_ep *ep);
>> +int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep);
>> void dw_pcie_ep_exit(struct dw_pcie_ep *ep);
>> int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no);
>> int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
>> @@ -416,6 +417,11 @@ static inline int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>> return 0;
>> }
>>
>> +static inline int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
>> +{
>> + return 0;
>> +}
>> +
>> static inline void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
>> {
>> }
>> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
>> index 36644ccd32ac..241e6a6f39fb 100644
>> --- a/include/linux/pci-epc.h
>> +++ b/include/linux/pci-epc.h
>> @@ -121,6 +121,7 @@ struct pci_epc_features {
>> u8 bar_fixed_64bit;
>> u64 bar_fixed_size[PCI_STD_NUM_BARS];
>> size_t align;
>> + bool skip_core_init;
>
> This looks more like a designware specific change. Why is it added to the core
> pci_epc_features?
Although the changes are done in DesignWare core (as Tegra194 uses DesignWare IP),
core not being available for programming before REFCLK from host is available, seemed
like a very generic case to me, so I added this as part of core features it self.
Thanks,
Vidya Sagar
>
> Thanks
> Kishon
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/4] PCI: dwc: Add new feature to skip core initialization
2019-11-27 8:40 ` Vidya Sagar
@ 2019-11-27 9:18 ` Kishon Vijay Abraham I
0 siblings, 0 replies; 23+ messages in thread
From: Kishon Vijay Abraham I @ 2019-11-27 9:18 UTC (permalink / raw)
To: Vidya Sagar, jingoohan1, gustavo.pimentel, lorenzo.pieralisi,
andrew.murray, bhelgaas, thierry.reding
Cc: Jisheng.Zhang, jonathanh, linux-pci, linux-kernel, kthota,
mmaddireddy, sagar.tv
Hi,
On 27/11/19 2:10 PM, Vidya Sagar wrote:
> On 11/27/2019 1:44 PM, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On 13/11/19 2:38 PM, Vidya Sagar wrote:
>>> Add a new feature 'skip_core_init' that can be set by platform drivers
>>> of devices that do not have their core registers available until reference
>>> clock from host is available (Ex:- Tegra194) to indicate DesignWare
>>> endpoint mode sub-system to not perform core registers initialization.
>>> Existing dw_pcie_ep_init() is refactored and all the code that touches
>>> registers is extracted to form a new API dw_pcie_ep_init_complete() that
>>> can be called later by platform drivers setting 'skip_core_init' to '1'.
>>>
>>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>>> ---
>>> .../pci/controller/dwc/pcie-designware-ep.c | 72 +++++++++++--------
>>> drivers/pci/controller/dwc/pcie-designware.h | 6 ++
>>> include/linux/pci-epc.h | 1 +
>>> 3 files changed, 51 insertions(+), 28 deletions(-)
>>>
>>> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c
>>> b/drivers/pci/controller/dwc/pcie-designware-ep.c
>>> index 3dd2e2697294..06f4379be8a3 100644
>>> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
>>> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
>>> @@ -492,19 +492,53 @@ static unsigned int
>>> dw_pcie_ep_find_ext_capability(struct dw_pcie *pci, int cap)
>>> return 0;
>>> }
>>> -int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>>> +int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
>>> {
>>> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>>> + unsigned int offset;
>>> + unsigned int nbars;
>>> + u8 hdr_type;
>>> + u32 reg;
>>> int i;
>>> +
>>> + hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE);
>>> + if (hdr_type != PCI_HEADER_TYPE_NORMAL) {
>>> + dev_err(pci->dev,
>>> + "PCIe controller is not set to EP mode (hdr_type:0x%x)!\n",
>>> + hdr_type);
>>> + return -EIO;
>>> + }
>>> +
>>> + ep->msi_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
>>> +
>>> + ep->msix_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSIX);
>>> +
>>> + offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
>>> + if (offset) {
>>> + reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
>>> + nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
>>> + PCI_REBAR_CTRL_NBAR_SHIFT;
>>> +
>>> + dw_pcie_dbi_ro_wr_en(pci);
>>> + for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
>>> + dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
>>> + dw_pcie_dbi_ro_wr_dis(pci);
>>> + }
>>> +
>>> + dw_pcie_setup(pci);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>>> +{
>>> int ret;
>>> - u32 reg;
>>> void *addr;
>>> - u8 hdr_type;
>>> - unsigned int nbars;
>>> - unsigned int offset;
>>> struct pci_epc *epc;
>>> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>>> struct device *dev = pci->dev;
>>> struct device_node *np = dev->of_node;
>>> + const struct pci_epc_features *epc_features;
>>> if (!pci->dbi_base || !pci->dbi_base2) {
>>> dev_err(dev, "dbi_base/dbi_base2 is not populated\n");
>>> @@ -563,13 +597,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>>> if (ep->ops->ep_init)
>>> ep->ops->ep_init(ep);
>>> - hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE);
>>> - if (hdr_type != PCI_HEADER_TYPE_NORMAL) {
>>> - dev_err(pci->dev, "PCIe controller is not set to EP mode
>>> (hdr_type:0x%x)!\n",
>>> - hdr_type);
>>> - return -EIO;
>>> - }
>>> -
>>> ret = of_property_read_u8(np, "max-functions", &epc->max_functions);
>>> if (ret < 0)
>>> epc->max_functions = 1;
>>> @@ -587,23 +614,12 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>>> dev_err(dev, "Failed to reserve memory for MSI/MSI-X\n");
>>> return -ENOMEM;
>>> }
>>> - ep->msi_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
>>> - ep->msix_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSIX);
>>> -
>>> - offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
>>> - if (offset) {
>>> - reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
>>> - nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
>>> - PCI_REBAR_CTRL_NBAR_SHIFT;
>>> -
>>> - dw_pcie_dbi_ro_wr_en(pci);
>>> - for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
>>> - dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
>>> - dw_pcie_dbi_ro_wr_dis(pci);
>>> + if (ep->ops->get_features) {
>>> + epc_features = ep->ops->get_features(ep);
>>> + if (epc_features->skip_core_init)
>>> + return 0;
>>> }
>>> - dw_pcie_setup(pci);
>>> -
>>> - return 0;
>>> + return dw_pcie_ep_init_complete(ep);
>>> }
>>> diff --git a/drivers/pci/controller/dwc/pcie-designware.h
>>> b/drivers/pci/controller/dwc/pcie-designware.h
>>> index 5accdd6bc388..340783e9032e 100644
>>> --- a/drivers/pci/controller/dwc/pcie-designware.h
>>> +++ b/drivers/pci/controller/dwc/pcie-designware.h
>>> @@ -399,6 +399,7 @@ static inline int dw_pcie_allocate_domains(struct
>>> pcie_port *pp)
>>> #ifdef CONFIG_PCIE_DW_EP
>>> void dw_pcie_ep_linkup(struct dw_pcie_ep *ep);
>>> int dw_pcie_ep_init(struct dw_pcie_ep *ep);
>>> +int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep);
>>> void dw_pcie_ep_exit(struct dw_pcie_ep *ep);
>>> int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no);
>>> int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
>>> @@ -416,6 +417,11 @@ static inline int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>>> return 0;
>>> }
>>> +static inline int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
>>> +{
>>> + return 0;
>>> +}
>>> +
>>> static inline void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
>>> {
>>> }
>>> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
>>> index 36644ccd32ac..241e6a6f39fb 100644
>>> --- a/include/linux/pci-epc.h
>>> +++ b/include/linux/pci-epc.h
>>> @@ -121,6 +121,7 @@ struct pci_epc_features {
>>> u8 bar_fixed_64bit;
>>> u64 bar_fixed_size[PCI_STD_NUM_BARS];
>>> size_t align;
>>> + bool skip_core_init;
>>
>> This looks more like a designware specific change. Why is it added to the core
>> pci_epc_features?
> Although the changes are done in DesignWare core (as Tegra194 uses DesignWare IP),
> core not being available for programming before REFCLK from host is available,
> seemed
> like a very generic case to me, so I added this as part of core features it self.
right, I think you can name the epc_feature as core_init_notifier instead of
skip_core_init (similar to linkup_notifier?) and add that as a first patch.
Then you can use the epc_features in epf_test and designware in subsequent patches.
Thanks
Kishon
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/4] PCI: dwc: Add new feature to skip core initialization
2019-11-13 9:08 ` [PATCH 1/4] PCI: dwc: Add new feature to skip " Vidya Sagar
2019-11-27 8:14 ` Kishon Vijay Abraham I
@ 2019-11-27 9:48 ` Christoph Hellwig
2019-11-29 14:40 ` Vidya Sagar
2019-12-05 10:04 ` Kishon Vijay Abraham I
2 siblings, 1 reply; 23+ messages in thread
From: Christoph Hellwig @ 2019-11-27 9:48 UTC (permalink / raw)
To: Vidya Sagar
Cc: jingoohan1, gustavo.pimentel, lorenzo.pieralisi, andrew.murray,
bhelgaas, kishon, thierry.reding, Jisheng.Zhang, jonathanh,
linux-pci, linux-kernel, kthota, mmaddireddy, sagar.tv
On Wed, Nov 13, 2019 at 02:38:48PM +0530, Vidya Sagar wrote:
> + if (ep->ops->get_features) {
> + epc_features = ep->ops->get_features(ep);
> + if (epc_features->skip_core_init)
> + return 0;
> }
>
> + return dw_pcie_ep_init_complete(ep);
This calling convention is strange. Just split the early part of
dw_pcie_ep_init into an dw_pcie_ep_early and either add a tiny
wrapper like:
int dw_pcie_ep_init(struct dw_pcie_ep *ep)
{
int error;
error = dw_pcie_ep_init_early(ep);
if (error)
return error;
return dw_pcie_ep_init_late(ep);
}
or just open code that in the few callers. That keeps the calling
conventions much simpler and avoids relying on a callback and flag.
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/4] PCI: dwc: Add new feature to skip core initialization
2019-11-27 9:48 ` Christoph Hellwig
@ 2019-11-29 14:40 ` Vidya Sagar
2019-12-05 9:59 ` Vidya Sagar
0 siblings, 1 reply; 23+ messages in thread
From: Vidya Sagar @ 2019-11-29 14:40 UTC (permalink / raw)
To: Christoph Hellwig
Cc: jingoohan1, gustavo.pimentel, lorenzo.pieralisi, andrew.murray,
bhelgaas, kishon, thierry.reding, Jisheng.Zhang, jonathanh,
linux-pci, linux-kernel, kthota, mmaddireddy, sagar.tv
On 11/27/2019 3:18 PM, Christoph Hellwig wrote:
> On Wed, Nov 13, 2019 at 02:38:48PM +0530, Vidya Sagar wrote:
>> + if (ep->ops->get_features) {
>> + epc_features = ep->ops->get_features(ep);
>> + if (epc_features->skip_core_init)
>> + return 0;
>> }
>>
>> + return dw_pcie_ep_init_complete(ep);
>
> This calling convention is strange. Just split the early part of
> dw_pcie_ep_init into an dw_pcie_ep_early and either add a tiny
> wrapper like:
>
> int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> {
> int error;
>
> error = dw_pcie_ep_init_early(ep);
> if (error)
> return error;
> return dw_pcie_ep_init_late(ep);
> }
>
> or just open code that in the few callers. That keeps the calling
> conventions much simpler and avoids relying on a callback and flag.
I'm not sure if I got this right. I think in any case, code that is going to be
part of dw_pcie_ep_init_late() needs to depend on callback and flag right?
I mean, unless it is confirmed (by calling the get_features() callback and
checking whether or not the core is available for programming) dw_pcie_ep_init_late()
can't be called right?
Please let me know if I'm missing something here.
- Vidya Sagar
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/4] PCI: dwc: Add new feature to skip core initialization
2019-11-29 14:40 ` Vidya Sagar
@ 2019-12-05 9:59 ` Vidya Sagar
0 siblings, 0 replies; 23+ messages in thread
From: Vidya Sagar @ 2019-12-05 9:59 UTC (permalink / raw)
To: Christoph Hellwig
Cc: jingoohan1, gustavo.pimentel, lorenzo.pieralisi, andrew.murray,
bhelgaas, kishon, thierry.reding, Jisheng.Zhang, jonathanh,
linux-pci, linux-kernel, kthota, mmaddireddy, sagar.tv
On 11/29/2019 8:10 PM, Vidya Sagar wrote:
Hi Christoph,
Could you please let me know what am I missing here?
Thanks,
Vidya Sagar
> On 11/27/2019 3:18 PM, Christoph Hellwig wrote:
>> On Wed, Nov 13, 2019 at 02:38:48PM +0530, Vidya Sagar wrote:
>>> + if (ep->ops->get_features) {
>>> + epc_features = ep->ops->get_features(ep);
>>> + if (epc_features->skip_core_init)
>>> + return 0;
>>> }
>>> + return dw_pcie_ep_init_complete(ep);
>>
>> This calling convention is strange. Just split the early part of
>> dw_pcie_ep_init into an dw_pcie_ep_early and either add a tiny
>> wrapper like:
>>
>> int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>> {
>> int error;
>>
>> error = dw_pcie_ep_init_early(ep);
>> if (error)
>> return error;
>> return dw_pcie_ep_init_late(ep);
>> }
>>
>> or just open code that in the few callers. That keeps the calling
>> conventions much simpler and avoids relying on a callback and flag.
> I'm not sure if I got this right. I think in any case, code that is going to be
> part of dw_pcie_ep_init_late() needs to depend on callback and flag right?
> I mean, unless it is confirmed (by calling the get_features() callback and
> checking whether or not the core is available for programming) dw_pcie_ep_init_late()
> can't be called right?
> Please let me know if I'm missing something here.
>
> - Vidya Sagar
>>
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/4] PCI: dwc: Add new feature to skip core initialization
2019-11-13 9:08 ` [PATCH 1/4] PCI: dwc: Add new feature to skip " Vidya Sagar
2019-11-27 8:14 ` Kishon Vijay Abraham I
2019-11-27 9:48 ` Christoph Hellwig
@ 2019-12-05 10:04 ` Kishon Vijay Abraham I
2020-01-03 9:40 ` Vidya Sagar
2 siblings, 1 reply; 23+ messages in thread
From: Kishon Vijay Abraham I @ 2019-12-05 10:04 UTC (permalink / raw)
To: Vidya Sagar, jingoohan1, gustavo.pimentel, lorenzo.pieralisi,
andrew.murray, bhelgaas, thierry.reding
Cc: Jisheng.Zhang, jonathanh, linux-pci, linux-kernel, kthota,
mmaddireddy, sagar.tv
On 13/11/19 2:38 pm, Vidya Sagar wrote:
> Add a new feature 'skip_core_init' that can be set by platform drivers
> of devices that do not have their core registers available until reference
> clock from host is available (Ex:- Tegra194) to indicate DesignWare
> endpoint mode sub-system to not perform core registers initialization.
> Existing dw_pcie_ep_init() is refactored and all the code that touches
> registers is extracted to form a new API dw_pcie_ep_init_complete() that
> can be called later by platform drivers setting 'skip_core_init' to '1'.
No. pci_epc_features should only use constant values. This is used by
function drivers to know the controller capabilities.
Thanks
Kishon
>
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> ---
> .../pci/controller/dwc/pcie-designware-ep.c | 72 +++++++++++--------
> drivers/pci/controller/dwc/pcie-designware.h | 6 ++
> include/linux/pci-epc.h | 1 +
> 3 files changed, 51 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 3dd2e2697294..06f4379be8a3 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -492,19 +492,53 @@ static unsigned int dw_pcie_ep_find_ext_capability(struct dw_pcie *pci, int cap)
> return 0;
> }
>
> -int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> +int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
> {
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> + unsigned int offset;
> + unsigned int nbars;
> + u8 hdr_type;
> + u32 reg;
> int i;
> +
> + hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE);
> + if (hdr_type != PCI_HEADER_TYPE_NORMAL) {
> + dev_err(pci->dev,
> + "PCIe controller is not set to EP mode (hdr_type:0x%x)!\n",
> + hdr_type);
> + return -EIO;
> + }
> +
> + ep->msi_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
> +
> + ep->msix_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSIX);
> +
> + offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
> + if (offset) {
> + reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
> + nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
> + PCI_REBAR_CTRL_NBAR_SHIFT;
> +
> + dw_pcie_dbi_ro_wr_en(pci);
> + for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
> + dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
> + dw_pcie_dbi_ro_wr_dis(pci);
> + }
> +
> + dw_pcie_setup(pci);
> +
> + return 0;
> +}
> +
> +int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> +{
> int ret;
> - u32 reg;
> void *addr;
> - u8 hdr_type;
> - unsigned int nbars;
> - unsigned int offset;
> struct pci_epc *epc;
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> struct device *dev = pci->dev;
> struct device_node *np = dev->of_node;
> + const struct pci_epc_features *epc_features;
>
> if (!pci->dbi_base || !pci->dbi_base2) {
> dev_err(dev, "dbi_base/dbi_base2 is not populated\n");
> @@ -563,13 +597,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> if (ep->ops->ep_init)
> ep->ops->ep_init(ep);
>
> - hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE);
> - if (hdr_type != PCI_HEADER_TYPE_NORMAL) {
> - dev_err(pci->dev, "PCIe controller is not set to EP mode (hdr_type:0x%x)!\n",
> - hdr_type);
> - return -EIO;
> - }
> -
> ret = of_property_read_u8(np, "max-functions", &epc->max_functions);
> if (ret < 0)
> epc->max_functions = 1;
> @@ -587,23 +614,12 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> dev_err(dev, "Failed to reserve memory for MSI/MSI-X\n");
> return -ENOMEM;
> }
> - ep->msi_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
>
> - ep->msix_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSIX);
> -
> - offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
> - if (offset) {
> - reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
> - nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
> - PCI_REBAR_CTRL_NBAR_SHIFT;
> -
> - dw_pcie_dbi_ro_wr_en(pci);
> - for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
> - dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
> - dw_pcie_dbi_ro_wr_dis(pci);
> + if (ep->ops->get_features) {
> + epc_features = ep->ops->get_features(ep);
> + if (epc_features->skip_core_init)
> + return 0;
> }
>
> - dw_pcie_setup(pci);
> -
> - return 0;
> + return dw_pcie_ep_init_complete(ep);
> }
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 5accdd6bc388..340783e9032e 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -399,6 +399,7 @@ static inline int dw_pcie_allocate_domains(struct pcie_port *pp)
> #ifdef CONFIG_PCIE_DW_EP
> void dw_pcie_ep_linkup(struct dw_pcie_ep *ep);
> int dw_pcie_ep_init(struct dw_pcie_ep *ep);
> +int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep);
> void dw_pcie_ep_exit(struct dw_pcie_ep *ep);
> int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no);
> int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
> @@ -416,6 +417,11 @@ static inline int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> return 0;
> }
>
> +static inline int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
> +{
> + return 0;
> +}
> +
> static inline void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
> {
> }
> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> index 36644ccd32ac..241e6a6f39fb 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -121,6 +121,7 @@ struct pci_epc_features {
> u8 bar_fixed_64bit;
> u64 bar_fixed_size[PCI_STD_NUM_BARS];
> size_t align;
> + bool skip_core_init;
> };
>
> #define to_pci_epc(device) container_of((device), struct pci_epc, dev)
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 1/4] PCI: dwc: Add new feature to skip core initialization
2019-12-05 10:04 ` Kishon Vijay Abraham I
@ 2020-01-03 9:40 ` Vidya Sagar
0 siblings, 0 replies; 23+ messages in thread
From: Vidya Sagar @ 2020-01-03 9:40 UTC (permalink / raw)
To: Kishon Vijay Abraham I, jingoohan1, gustavo.pimentel,
lorenzo.pieralisi, andrew.murray, bhelgaas, thierry.reding
Cc: Jisheng.Zhang, jonathanh, linux-pci, linux-kernel, kthota,
mmaddireddy, sagar.tv
On 12/5/2019 3:34 PM, Kishon Vijay Abraham I wrote:
>
>
> On 13/11/19 2:38 pm, Vidya Sagar wrote:
>> Add a new feature 'skip_core_init' that can be set by platform drivers
>> of devices that do not have their core registers available until reference
>> clock from host is available (Ex:- Tegra194) to indicate DesignWare
>> endpoint mode sub-system to not perform core registers initialization.
>> Existing dw_pcie_ep_init() is refactored and all the code that touches
>> registers is extracted to form a new API dw_pcie_ep_init_complete() that
>> can be called later by platform drivers setting 'skip_core_init' to '1'.
>
> No. pci_epc_features should only use constant values. This is used by function drivers to know the controller capabilities.
Yes. I'm going to set EPC features as constant values in pcie-tegra194.c driver.
I'm going to rewrite this commit message in the next patch.
>
> Thanks
> Kishon
>
>>
>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>> ---
>> .../pci/controller/dwc/pcie-designware-ep.c | 72 +++++++++++--------
>> drivers/pci/controller/dwc/pcie-designware.h | 6 ++
>> include/linux/pci-epc.h | 1 +
>> 3 files changed, 51 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
>> index 3dd2e2697294..06f4379be8a3 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
>> @@ -492,19 +492,53 @@ static unsigned int dw_pcie_ep_find_ext_capability(struct dw_pcie *pci, int cap)
>> return 0;
>> }
>> -int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>> +int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
>> {
>> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>> + unsigned int offset;
>> + unsigned int nbars;
>> + u8 hdr_type;
>> + u32 reg;
>> int i;
>> +
>> + hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE);
>> + if (hdr_type != PCI_HEADER_TYPE_NORMAL) {
>> + dev_err(pci->dev,
>> + "PCIe controller is not set to EP mode (hdr_type:0x%x)!\n",
>> + hdr_type);
>> + return -EIO;
>> + }
>> +
>> + ep->msi_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
>> +
>> + ep->msix_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSIX);
>> +
>> + offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
>> + if (offset) {
>> + reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
>> + nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
>> + PCI_REBAR_CTRL_NBAR_SHIFT;
>> +
>> + dw_pcie_dbi_ro_wr_en(pci);
>> + for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
>> + dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
>> + dw_pcie_dbi_ro_wr_dis(pci);
>> + }
>> +
>> + dw_pcie_setup(pci);
>> +
>> + return 0;
>> +}
>> +
>> +int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>> +{
>> int ret;
>> - u32 reg;
>> void *addr;
>> - u8 hdr_type;
>> - unsigned int nbars;
>> - unsigned int offset;
>> struct pci_epc *epc;
>> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>> struct device *dev = pci->dev;
>> struct device_node *np = dev->of_node;
>> + const struct pci_epc_features *epc_features;
>> if (!pci->dbi_base || !pci->dbi_base2) {
>> dev_err(dev, "dbi_base/dbi_base2 is not populated\n");
>> @@ -563,13 +597,6 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>> if (ep->ops->ep_init)
>> ep->ops->ep_init(ep);
>> - hdr_type = dw_pcie_readb_dbi(pci, PCI_HEADER_TYPE);
>> - if (hdr_type != PCI_HEADER_TYPE_NORMAL) {
>> - dev_err(pci->dev, "PCIe controller is not set to EP mode (hdr_type:0x%x)!\n",
>> - hdr_type);
>> - return -EIO;
>> - }
>> -
>> ret = of_property_read_u8(np, "max-functions", &epc->max_functions);
>> if (ret < 0)
>> epc->max_functions = 1;
>> @@ -587,23 +614,12 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>> dev_err(dev, "Failed to reserve memory for MSI/MSI-X\n");
>> return -ENOMEM;
>> }
>> - ep->msi_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
>> - ep->msix_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSIX);
>> -
>> - offset = dw_pcie_ep_find_ext_capability(pci, PCI_EXT_CAP_ID_REBAR);
>> - if (offset) {
>> - reg = dw_pcie_readl_dbi(pci, offset + PCI_REBAR_CTRL);
>> - nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
>> - PCI_REBAR_CTRL_NBAR_SHIFT;
>> -
>> - dw_pcie_dbi_ro_wr_en(pci);
>> - for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
>> - dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
>> - dw_pcie_dbi_ro_wr_dis(pci);
>> + if (ep->ops->get_features) {
>> + epc_features = ep->ops->get_features(ep);
>> + if (epc_features->skip_core_init)
>> + return 0;
>> }
>> - dw_pcie_setup(pci);
>> -
>> - return 0;
>> + return dw_pcie_ep_init_complete(ep);
>> }
>> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
>> index 5accdd6bc388..340783e9032e 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware.h
>> +++ b/drivers/pci/controller/dwc/pcie-designware.h
>> @@ -399,6 +399,7 @@ static inline int dw_pcie_allocate_domains(struct pcie_port *pp)
>> #ifdef CONFIG_PCIE_DW_EP
>> void dw_pcie_ep_linkup(struct dw_pcie_ep *ep);
>> int dw_pcie_ep_init(struct dw_pcie_ep *ep);
>> +int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep);
>> void dw_pcie_ep_exit(struct dw_pcie_ep *ep);
>> int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no);
>> int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
>> @@ -416,6 +417,11 @@ static inline int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>> return 0;
>> }
>> +static inline int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
>> +{
>> + return 0;
>> +}
>> +
>> static inline void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
>> {
>> }
>> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
>> index 36644ccd32ac..241e6a6f39fb 100644
>> --- a/include/linux/pci-epc.h
>> +++ b/include/linux/pci-epc.h
>> @@ -121,6 +121,7 @@ struct pci_epc_features {
>> u8 bar_fixed_64bit;
>> u64 bar_fixed_size[PCI_STD_NUM_BARS];
>> size_t align;
>> + bool skip_core_init;
>> };
>> #define to_pci_epc(device) container_of((device), struct pci_epc, dev)
>>
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 2/4] PCI: endpoint: Add notification for core init completion
2019-11-13 9:08 [PATCH 0/4] Add support to defer core initialization Vidya Sagar
2019-11-13 9:08 ` [PATCH 1/4] PCI: dwc: Add new feature to skip " Vidya Sagar
@ 2019-11-13 9:08 ` Vidya Sagar
2019-11-27 8:18 ` Kishon Vijay Abraham I
2019-11-13 9:08 ` [PATCH 3/4] PCI: dwc: Add API to notify core initialization completion Vidya Sagar
` (2 subsequent siblings)
4 siblings, 1 reply; 23+ messages in thread
From: Vidya Sagar @ 2019-11-13 9:08 UTC (permalink / raw)
To: jingoohan1, gustavo.pimentel, lorenzo.pieralisi, andrew.murray,
bhelgaas, kishon, thierry.reding
Cc: Jisheng.Zhang, jonathanh, linux-pci, linux-kernel, kthota,
mmaddireddy, vidyas, sagar.tv
Add support to send notifications to EPF from EPC once the core
registers initialization is complete.
Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
drivers/pci/endpoint/pci-epc-core.c | 19 ++++++++++++++++++-
include/linux/pci-epc.h | 1 +
include/linux/pci-epf.h | 5 +++++
3 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index 2f6436599fcb..fcc3f7fb19c0 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -542,10 +542,27 @@ void pci_epc_linkup(struct pci_epc *epc)
if (!epc || IS_ERR(epc))
return;
- atomic_notifier_call_chain(&epc->notifier, 0, NULL);
+ atomic_notifier_call_chain(&epc->notifier, LINK_UP, NULL);
}
EXPORT_SYMBOL_GPL(pci_epc_linkup);
+/**
+ * pci_epc_init_notify() - Notify the EPF device that EPC device's core
+ * initialization is completed.
+ * @epc: the EPC device whose core initialization is completeds
+ *
+ * Invoke to Notify the EPF device that the EPC device's initialization
+ * is completed.
+ */
+void pci_epc_init_notify(struct pci_epc *epc)
+{
+ if (!epc || IS_ERR(epc))
+ return;
+
+ atomic_notifier_call_chain(&epc->notifier, CORE_INIT, NULL);
+}
+EXPORT_SYMBOL_GPL(pci_epc_init_notify);
+
/**
* pci_epc_destroy() - destroy the EPC device
* @epc: the EPC device that has to be destroyed
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index 241e6a6f39fb..c670543e42f9 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -160,6 +160,7 @@ void devm_pci_epc_destroy(struct device *dev, struct pci_epc *epc);
void pci_epc_destroy(struct pci_epc *epc);
int pci_epc_add_epf(struct pci_epc *epc, struct pci_epf *epf);
void pci_epc_linkup(struct pci_epc *epc);
+void pci_epc_init_notify(struct pci_epc *epc);
void pci_epc_remove_epf(struct pci_epc *epc, struct pci_epf *epf);
int pci_epc_write_header(struct pci_epc *epc, u8 func_no,
struct pci_epf_header *hdr);
diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
index 4993f7f6439b..3cb65ac1648c 100644
--- a/include/linux/pci-epf.h
+++ b/include/linux/pci-epf.h
@@ -15,6 +15,11 @@
struct pci_epf;
+enum pci_notify_event {
+ CORE_INIT,
+ LINK_UP,
+};
+
enum pci_barno {
BAR_0,
BAR_1,
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 2/4] PCI: endpoint: Add notification for core init completion
2019-11-13 9:08 ` [PATCH 2/4] PCI: endpoint: Add notification for core init completion Vidya Sagar
@ 2019-11-27 8:18 ` Kishon Vijay Abraham I
2019-11-27 8:22 ` Kishon Vijay Abraham I
0 siblings, 1 reply; 23+ messages in thread
From: Kishon Vijay Abraham I @ 2019-11-27 8:18 UTC (permalink / raw)
To: Vidya Sagar, jingoohan1, gustavo.pimentel, lorenzo.pieralisi,
andrew.murray, bhelgaas, thierry.reding
Cc: Jisheng.Zhang, jonathanh, linux-pci, linux-kernel, kthota,
mmaddireddy, sagar.tv
Hi,
On 13/11/19 2:38 PM, Vidya Sagar wrote:
> Add support to send notifications to EPF from EPC once the core
> registers initialization is complete.
>
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> ---
> drivers/pci/endpoint/pci-epc-core.c | 19 ++++++++++++++++++-
> include/linux/pci-epc.h | 1 +
> include/linux/pci-epf.h | 5 +++++
> 3 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index 2f6436599fcb..fcc3f7fb19c0 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -542,10 +542,27 @@ void pci_epc_linkup(struct pci_epc *epc)
> if (!epc || IS_ERR(epc))
> return;
>
> - atomic_notifier_call_chain(&epc->notifier, 0, NULL);
> + atomic_notifier_call_chain(&epc->notifier, LINK_UP, NULL);
> }
> EXPORT_SYMBOL_GPL(pci_epc_linkup);
Is this based on upstream kernel? or did you create this after applying [1]?
[1] -> https://lkml.org/lkml/2019/6/4/633
Thanks
Kishon
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 2/4] PCI: endpoint: Add notification for core init completion
2019-11-27 8:18 ` Kishon Vijay Abraham I
@ 2019-11-27 8:22 ` Kishon Vijay Abraham I
0 siblings, 0 replies; 23+ messages in thread
From: Kishon Vijay Abraham I @ 2019-11-27 8:22 UTC (permalink / raw)
To: Vidya Sagar, jingoohan1, gustavo.pimentel, lorenzo.pieralisi,
andrew.murray, bhelgaas, thierry.reding
Cc: Jisheng.Zhang, jonathanh, linux-pci, linux-kernel, kthota,
mmaddireddy, sagar.tv
Hi,
On 27/11/19 1:48 PM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On 13/11/19 2:38 PM, Vidya Sagar wrote:
>> Add support to send notifications to EPF from EPC once the core
>> registers initialization is complete.
>>
>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>> ---
>> drivers/pci/endpoint/pci-epc-core.c | 19 ++++++++++++++++++-
>> include/linux/pci-epc.h | 1 +
>> include/linux/pci-epf.h | 5 +++++
>> 3 files changed, 24 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
>> index 2f6436599fcb..fcc3f7fb19c0 100644
>> --- a/drivers/pci/endpoint/pci-epc-core.c
>> +++ b/drivers/pci/endpoint/pci-epc-core.c
>> @@ -542,10 +542,27 @@ void pci_epc_linkup(struct pci_epc *epc)
>> if (!epc || IS_ERR(epc))
>> return;
>>
>> - atomic_notifier_call_chain(&epc->notifier, 0, NULL);
>> + atomic_notifier_call_chain(&epc->notifier, LINK_UP, NULL);
>> }
>> EXPORT_SYMBOL_GPL(pci_epc_linkup);
>
> Is this based on upstream kernel? or did you create this after applying [1]?
never mind, you've mentioned this depends on my other series in your cover letter.
Thanks
Kishon
^ permalink raw reply [flat|nested] 23+ messages in thread
* [PATCH 3/4] PCI: dwc: Add API to notify core initialization completion
2019-11-13 9:08 [PATCH 0/4] Add support to defer core initialization Vidya Sagar
2019-11-13 9:08 ` [PATCH 1/4] PCI: dwc: Add new feature to skip " Vidya Sagar
2019-11-13 9:08 ` [PATCH 2/4] PCI: endpoint: Add notification for core init completion Vidya Sagar
@ 2019-11-13 9:08 ` Vidya Sagar
2019-11-13 9:08 ` [PATCH 4/4] PCI: pci-epf-test: Add support to defer core initialization Vidya Sagar
2019-11-18 6:55 ` [PATCH 0/4] " Vidya Sagar
4 siblings, 0 replies; 23+ messages in thread
From: Vidya Sagar @ 2019-11-13 9:08 UTC (permalink / raw)
To: jingoohan1, gustavo.pimentel, lorenzo.pieralisi, andrew.murray,
bhelgaas, kishon, thierry.reding
Cc: Jisheng.Zhang, jonathanh, linux-pci, linux-kernel, kthota,
mmaddireddy, vidyas, sagar.tv
Add a new API dw_pcie_ep_init_notify() to let platform drivers
call it when the core is available for initialization.
Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
drivers/pci/controller/dwc/pcie-designware-ep.c | 7 +++++++
drivers/pci/controller/dwc/pcie-designware.h | 5 +++++
2 files changed, 12 insertions(+)
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 06f4379be8a3..1355e1d4d426 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -19,6 +19,13 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
pci_epc_linkup(epc);
}
+void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep)
+{
+ struct pci_epc *epc = ep->epc;
+
+ pci_epc_init_notify(epc);
+}
+
static void __dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar,
int flags)
{
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 340783e9032e..f62c5bae6b2d 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -400,6 +400,7 @@ static inline int dw_pcie_allocate_domains(struct pcie_port *pp)
void dw_pcie_ep_linkup(struct dw_pcie_ep *ep);
int dw_pcie_ep_init(struct dw_pcie_ep *ep);
int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep);
+void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep);
void dw_pcie_ep_exit(struct dw_pcie_ep *ep);
int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no);
int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
@@ -422,6 +423,10 @@ static inline int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
return 0;
}
+static inline void dw_pcie_ep_init_notify(struct dw_pcie_ep *ep)
+{
+}
+
static inline void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
{
}
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH 4/4] PCI: pci-epf-test: Add support to defer core initialization
2019-11-13 9:08 [PATCH 0/4] Add support to defer core initialization Vidya Sagar
` (2 preceding siblings ...)
2019-11-13 9:08 ` [PATCH 3/4] PCI: dwc: Add API to notify core initialization completion Vidya Sagar
@ 2019-11-13 9:08 ` Vidya Sagar
2019-11-27 9:20 ` Kishon Vijay Abraham I
2019-11-18 6:55 ` [PATCH 0/4] " Vidya Sagar
4 siblings, 1 reply; 23+ messages in thread
From: Vidya Sagar @ 2019-11-13 9:08 UTC (permalink / raw)
To: jingoohan1, gustavo.pimentel, lorenzo.pieralisi, andrew.murray,
bhelgaas, kishon, thierry.reding
Cc: Jisheng.Zhang, jonathanh, linux-pci, linux-kernel, kthota,
mmaddireddy, vidyas, sagar.tv
Add support to defer core initialization and to receive a notifier
when core is ready to accommodate platforms where core is not for
initialization untile reference clock from host is available.
Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
drivers/pci/endpoint/functions/pci-epf-test.c | 114 ++++++++++++------
1 file changed, 77 insertions(+), 37 deletions(-)
diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index bddff15052cc..068024fab544 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -360,18 +360,6 @@ static void pci_epf_test_cmd_handler(struct work_struct *work)
msecs_to_jiffies(1));
}
-static int pci_epf_test_notifier(struct notifier_block *nb, unsigned long val,
- void *data)
-{
- struct pci_epf *epf = container_of(nb, struct pci_epf, nb);
- struct pci_epf_test *epf_test = epf_get_drvdata(epf);
-
- queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
- msecs_to_jiffies(1));
-
- return NOTIFY_OK;
-}
-
static void pci_epf_test_unbind(struct pci_epf *epf)
{
struct pci_epf_test *epf_test = epf_get_drvdata(epf);
@@ -428,6 +416,78 @@ static int pci_epf_test_set_bar(struct pci_epf *epf)
return 0;
}
+static int pci_epf_test_core_init(struct pci_epf *epf)
+{
+ struct pci_epf_header *header = epf->header;
+ const struct pci_epc_features *epc_features;
+ struct pci_epc *epc = epf->epc;
+ struct device *dev = &epf->dev;
+ bool msix_capable = false;
+ bool msi_capable = true;
+ int ret;
+
+ epc_features = pci_epc_get_features(epc, epf->func_no);
+ if (epc_features) {
+ msix_capable = epc_features->msix_capable;
+ msi_capable = epc_features->msi_capable;
+ }
+
+ ret = pci_epc_write_header(epc, epf->func_no, header);
+ if (ret) {
+ dev_err(dev, "Configuration header write failed\n");
+ return ret;
+ }
+
+ ret = pci_epf_test_set_bar(epf);
+ if (ret)
+ 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 (msix_capable) {
+ ret = pci_epc_set_msix(epc, epf->func_no, epf->msix_interrupts);
+ if (ret) {
+ dev_err(dev, "MSI-X configuration failed\n");
+ return ret;
+ }
+ }
+
+ return 0;
+}
+
+static int pci_epf_test_notifier(struct notifier_block *nb, unsigned long val,
+ void *data)
+{
+ struct pci_epf *epf = container_of(nb, struct pci_epf, nb);
+ struct pci_epf_test *epf_test = epf_get_drvdata(epf);
+ int ret;
+
+ switch (val) {
+ case CORE_INIT:
+ ret = pci_epf_test_core_init(epf);
+ if (ret)
+ return NOTIFY_BAD;
+ break;
+
+ case LINK_UP:
+ queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
+ msecs_to_jiffies(1));
+ break;
+
+ default:
+ dev_err(&epf->dev, "Invalid EPF test notifier event\n");
+ return NOTIFY_BAD;
+ }
+
+ return NOTIFY_OK;
+}
+
static int pci_epf_test_alloc_space(struct pci_epf *epf)
{
struct pci_epf_test *epf_test = epf_get_drvdata(epf);
@@ -496,12 +556,11 @@ 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 skip_core_init = false;
bool msix_capable = false;
bool msi_capable = true;
@@ -511,6 +570,7 @@ static int pci_epf_test_bind(struct pci_epf *epf)
epc_features = pci_epc_get_features(epc, epf->func_no);
if (epc_features) {
linkup_notifier = epc_features->linkup_notifier;
+ skip_core_init = epc_features->skip_core_init;
msix_capable = epc_features->msix_capable;
msi_capable = epc_features->msi_capable;
test_reg_bar = pci_epc_get_first_free_bar(epc_features);
@@ -520,34 +580,14 @@ static int pci_epf_test_bind(struct pci_epf *epf)
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) {
- dev_err(dev, "Configuration header write failed\n");
- return ret;
- }
-
ret = pci_epf_test_alloc_space(epf);
if (ret)
return ret;
- ret = pci_epf_test_set_bar(epf);
- if (ret)
- 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 (msix_capable) {
- ret = pci_epc_set_msix(epc, epf->func_no, epf->msix_interrupts);
- if (ret) {
- dev_err(dev, "MSI-X configuration failed\n");
+ if (!skip_core_init) {
+ ret = pci_epf_test_core_init(epf);
+ if (ret)
return ret;
- }
}
if (linkup_notifier) {
--
2.17.1
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH 4/4] PCI: pci-epf-test: Add support to defer core initialization
2019-11-13 9:08 ` [PATCH 4/4] PCI: pci-epf-test: Add support to defer core initialization Vidya Sagar
@ 2019-11-27 9:20 ` Kishon Vijay Abraham I
2019-12-01 14:29 ` Vidya Sagar
0 siblings, 1 reply; 23+ messages in thread
From: Kishon Vijay Abraham I @ 2019-11-27 9:20 UTC (permalink / raw)
To: Vidya Sagar, jingoohan1, gustavo.pimentel, lorenzo.pieralisi,
andrew.murray, bhelgaas, thierry.reding
Cc: Jisheng.Zhang, jonathanh, linux-pci, linux-kernel, kthota,
mmaddireddy, sagar.tv
Hi,
On 13/11/19 2:38 PM, Vidya Sagar wrote:
> Add support to defer core initialization and to receive a notifier
> when core is ready to accommodate platforms where core is not for
> initialization untile reference clock from host is available.
>
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> ---
> drivers/pci/endpoint/functions/pci-epf-test.c | 114 ++++++++++++------
> 1 file changed, 77 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index bddff15052cc..068024fab544 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -360,18 +360,6 @@ static void pci_epf_test_cmd_handler(struct work_struct *work)
> msecs_to_jiffies(1));
> }
>
> -static int pci_epf_test_notifier(struct notifier_block *nb, unsigned long val,
> - void *data)
> -{
> - struct pci_epf *epf = container_of(nb, struct pci_epf, nb);
> - struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> -
> - queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
> - msecs_to_jiffies(1));
> -
> - return NOTIFY_OK;
> -}
> -
> static void pci_epf_test_unbind(struct pci_epf *epf)
> {
> struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> @@ -428,6 +416,78 @@ static int pci_epf_test_set_bar(struct pci_epf *epf)
> return 0;
> }
>
> +static int pci_epf_test_core_init(struct pci_epf *epf)
> +{
> + struct pci_epf_header *header = epf->header;
> + const struct pci_epc_features *epc_features;
> + struct pci_epc *epc = epf->epc;
> + struct device *dev = &epf->dev;
> + bool msix_capable = false;
> + bool msi_capable = true;
> + int ret;
> +
> + epc_features = pci_epc_get_features(epc, epf->func_no);
> + if (epc_features) {
> + msix_capable = epc_features->msix_capable;
> + msi_capable = epc_features->msi_capable;
> + }
> +
> + ret = pci_epc_write_header(epc, epf->func_no, header);
> + if (ret) {
> + dev_err(dev, "Configuration header write failed\n");
> + return ret;
> + }
> +
> + ret = pci_epf_test_set_bar(epf);
> + if (ret)
> + 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 (msix_capable) {
> + ret = pci_epc_set_msix(epc, epf->func_no, epf->msix_interrupts);
> + if (ret) {
> + dev_err(dev, "MSI-X configuration failed\n");
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int pci_epf_test_notifier(struct notifier_block *nb, unsigned long val,
> + void *data)
> +{
> + struct pci_epf *epf = container_of(nb, struct pci_epf, nb);
> + struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> + int ret;
> +
> + switch (val) {
> + case CORE_INIT:
> + ret = pci_epf_test_core_init(epf);
> + if (ret)
> + return NOTIFY_BAD;
> + break;
> +
> + case LINK_UP:
> + queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
> + msecs_to_jiffies(1));
> + break;
> +
> + default:
> + dev_err(&epf->dev, "Invalid EPF test notifier event\n");
> + return NOTIFY_BAD;
> + }
> +
> + return NOTIFY_OK;
> +}
> +
> static int pci_epf_test_alloc_space(struct pci_epf *epf)
> {
> struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> @@ -496,12 +556,11 @@ 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 skip_core_init = false;
> bool msix_capable = false;
> bool msi_capable = true;
>
> @@ -511,6 +570,7 @@ static int pci_epf_test_bind(struct pci_epf *epf)
> epc_features = pci_epc_get_features(epc, epf->func_no);
> if (epc_features) {
> linkup_notifier = epc_features->linkup_notifier;
> + skip_core_init = epc_features->skip_core_init;
> msix_capable = epc_features->msix_capable;
> msi_capable = epc_features->msi_capable;
Are these used anywhere in this function?
> test_reg_bar = pci_epc_get_first_free_bar(epc_features);
> @@ -520,34 +580,14 @@ static int pci_epf_test_bind(struct pci_epf *epf)
> 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) {
> - dev_err(dev, "Configuration header write failed\n");
> - return ret;
> - }
> -
> ret = pci_epf_test_alloc_space(epf);
> if (ret)
> return ret;
>
> - ret = pci_epf_test_set_bar(epf);
> - if (ret)
> - 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 (msix_capable) {
> - ret = pci_epc_set_msix(epc, epf->func_no, epf->msix_interrupts);
> - if (ret) {
> - dev_err(dev, "MSI-X configuration failed\n");
> + if (!skip_core_init) {
> + ret = pci_epf_test_core_init(epf);
> + if (ret)
> return ret;
> - }
> }
>
> if (linkup_notifier) {
This could as well be moved to pci_epf_test_core_init().
Thanks
Kishon
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/4] PCI: pci-epf-test: Add support to defer core initialization
2019-11-27 9:20 ` Kishon Vijay Abraham I
@ 2019-12-01 14:29 ` Vidya Sagar
2019-12-05 11:22 ` Kishon Vijay Abraham I
0 siblings, 1 reply; 23+ messages in thread
From: Vidya Sagar @ 2019-12-01 14:29 UTC (permalink / raw)
To: Kishon Vijay Abraham I, jingoohan1, gustavo.pimentel,
lorenzo.pieralisi, andrew.murray, bhelgaas, thierry.reding
Cc: Jisheng.Zhang, jonathanh, linux-pci, linux-kernel, kthota,
mmaddireddy, sagar.tv
On 11/27/2019 2:50 PM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On 13/11/19 2:38 PM, Vidya Sagar wrote:
>> Add support to defer core initialization and to receive a notifier
>> when core is ready to accommodate platforms where core is not for
>> initialization untile reference clock from host is available.
>>
>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>> ---
>> drivers/pci/endpoint/functions/pci-epf-test.c | 114 ++++++++++++------
>> 1 file changed, 77 insertions(+), 37 deletions(-)
>>
>> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
>> index bddff15052cc..068024fab544 100644
>> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
>> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
>> @@ -360,18 +360,6 @@ static void pci_epf_test_cmd_handler(struct work_struct *work)
>> msecs_to_jiffies(1));
>> }
>>
>> -static int pci_epf_test_notifier(struct notifier_block *nb, unsigned long val,
>> - void *data)
>> -{
>> - struct pci_epf *epf = container_of(nb, struct pci_epf, nb);
>> - struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>> -
>> - queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
>> - msecs_to_jiffies(1));
>> -
>> - return NOTIFY_OK;
>> -}
>> -
>> static void pci_epf_test_unbind(struct pci_epf *epf)
>> {
>> struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>> @@ -428,6 +416,78 @@ static int pci_epf_test_set_bar(struct pci_epf *epf)
>> return 0;
>> }
>>
>> +static int pci_epf_test_core_init(struct pci_epf *epf)
>> +{
>> + struct pci_epf_header *header = epf->header;
>> + const struct pci_epc_features *epc_features;
>> + struct pci_epc *epc = epf->epc;
>> + struct device *dev = &epf->dev;
>> + bool msix_capable = false;
>> + bool msi_capable = true;
>> + int ret;
>> +
>> + epc_features = pci_epc_get_features(epc, epf->func_no);
>> + if (epc_features) {
>> + msix_capable = epc_features->msix_capable;
>> + msi_capable = epc_features->msi_capable;
>> + }
>> +
>> + ret = pci_epc_write_header(epc, epf->func_no, header);
>> + if (ret) {
>> + dev_err(dev, "Configuration header write failed\n");
>> + return ret;
>> + }
>> +
>> + ret = pci_epf_test_set_bar(epf);
>> + if (ret)
>> + 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 (msix_capable) {
>> + ret = pci_epc_set_msix(epc, epf->func_no, epf->msix_interrupts);
>> + if (ret) {
>> + dev_err(dev, "MSI-X configuration failed\n");
>> + return ret;
>> + }
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int pci_epf_test_notifier(struct notifier_block *nb, unsigned long val,
>> + void *data)
>> +{
>> + struct pci_epf *epf = container_of(nb, struct pci_epf, nb);
>> + struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>> + int ret;
>> +
>> + switch (val) {
>> + case CORE_INIT:
>> + ret = pci_epf_test_core_init(epf);
>> + if (ret)
>> + return NOTIFY_BAD;
>> + break;
>> +
>> + case LINK_UP:
>> + queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
>> + msecs_to_jiffies(1));
>> + break;
>> +
>> + default:
>> + dev_err(&epf->dev, "Invalid EPF test notifier event\n");
>> + return NOTIFY_BAD;
>> + }
>> +
>> + return NOTIFY_OK;
>> +}
>> +
>> static int pci_epf_test_alloc_space(struct pci_epf *epf)
>> {
>> struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>> @@ -496,12 +556,11 @@ 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 skip_core_init = false;
>> bool msix_capable = false;
>> bool msi_capable = true;
>>
>> @@ -511,6 +570,7 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>> epc_features = pci_epc_get_features(epc, epf->func_no);
>> if (epc_features) {
>> linkup_notifier = epc_features->linkup_notifier;
>> + skip_core_init = epc_features->skip_core_init;
>> msix_capable = epc_features->msix_capable;
>> msi_capable = epc_features->msi_capable;
>
> Are these used anywhere in this function?
Nope. I'll remove them.
>> test_reg_bar = pci_epc_get_first_free_bar(epc_features);
>> @@ -520,34 +580,14 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>> 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) {
>> - dev_err(dev, "Configuration header write failed\n");
>> - return ret;
>> - }
>> -
>> ret = pci_epf_test_alloc_space(epf);
>> if (ret)
>> return ret;
>>
>> - ret = pci_epf_test_set_bar(epf);
>> - if (ret)
>> - 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 (msix_capable) {
>> - ret = pci_epc_set_msix(epc, epf->func_no, epf->msix_interrupts);
>> - if (ret) {
>> - dev_err(dev, "MSI-X configuration failed\n");
>> + if (!skip_core_init) {
>> + ret = pci_epf_test_core_init(epf);
>> + if (ret)
>> return ret;
>> - }
>> }
>>
>> if (linkup_notifier) {
>
> This could as well be moved to pci_epf_test_core_init().
Yes, but I would like to keep only the code that touches hardware in pci_epf_test_core_init()
to minimize the time it takes to execute it. Is there any strong reason to move it? if not,
I would prefer to leave it here in this function itself.
>
> Thanks
> Kishon
>
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/4] PCI: pci-epf-test: Add support to defer core initialization
2019-12-01 14:29 ` Vidya Sagar
@ 2019-12-05 11:22 ` Kishon Vijay Abraham I
2020-01-03 9:40 ` Vidya Sagar
0 siblings, 1 reply; 23+ messages in thread
From: Kishon Vijay Abraham I @ 2019-12-05 11:22 UTC (permalink / raw)
To: Vidya Sagar, jingoohan1, gustavo.pimentel, lorenzo.pieralisi,
andrew.murray, bhelgaas, thierry.reding
Cc: Jisheng.Zhang, jonathanh, linux-pci, linux-kernel, kthota,
mmaddireddy, sagar.tv
Hi,
On 01/12/19 7:59 pm, Vidya Sagar wrote:
> On 11/27/2019 2:50 PM, Kishon Vijay Abraham I wrote:
>> Hi,
>>
>> On 13/11/19 2:38 PM, Vidya Sagar wrote:
>>> Add support to defer core initialization and to receive a notifier
>>> when core is ready to accommodate platforms where core is not for
>>> initialization untile reference clock from host is available.
>>>
>>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>>> ---
>>> drivers/pci/endpoint/functions/pci-epf-test.c | 114 ++++++++++++------
>>> 1 file changed, 77 insertions(+), 37 deletions(-)
>>>
>>> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c
>>> b/drivers/pci/endpoint/functions/pci-epf-test.c
>>> index bddff15052cc..068024fab544 100644
>>> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
>>> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
>>> @@ -360,18 +360,6 @@ static void pci_epf_test_cmd_handler(struct
>>> work_struct *work)
>>> msecs_to_jiffies(1));
>>> }
>>> -static int pci_epf_test_notifier(struct notifier_block *nb, unsigned
>>> long val,
>>> - void *data)
>>> -{
>>> - struct pci_epf *epf = container_of(nb, struct pci_epf, nb);
>>> - struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>>> -
>>> - queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
>>> - msecs_to_jiffies(1));
>>> -
>>> - return NOTIFY_OK;
>>> -}
>>> -
>>> static void pci_epf_test_unbind(struct pci_epf *epf)
>>> {
>>> struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>>> @@ -428,6 +416,78 @@ static int pci_epf_test_set_bar(struct pci_epf
>>> *epf)
>>> return 0;
>>> }
>>> +static int pci_epf_test_core_init(struct pci_epf *epf)
>>> +{
>>> + struct pci_epf_header *header = epf->header;
>>> + const struct pci_epc_features *epc_features;
>>> + struct pci_epc *epc = epf->epc;
>>> + struct device *dev = &epf->dev;
>>> + bool msix_capable = false;
>>> + bool msi_capable = true;
>>> + int ret;
>>> +
>>> + epc_features = pci_epc_get_features(epc, epf->func_no);
>>> + if (epc_features) {
>>> + msix_capable = epc_features->msix_capable;
>>> + msi_capable = epc_features->msi_capable;
>>> + }
>>> +
>>> + ret = pci_epc_write_header(epc, epf->func_no, header);
>>> + if (ret) {
>>> + dev_err(dev, "Configuration header write failed\n");
>>> + return ret;
>>> + }
>>> +
>>> + ret = pci_epf_test_set_bar(epf);
>>> + if (ret)
>>> + 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 (msix_capable) {
>>> + ret = pci_epc_set_msix(epc, epf->func_no,
>>> epf->msix_interrupts);
>>> + if (ret) {
>>> + dev_err(dev, "MSI-X configuration failed\n");
>>> + return ret;
>>> + }
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int pci_epf_test_notifier(struct notifier_block *nb, unsigned
>>> long val,
>>> + void *data)
>>> +{
>>> + struct pci_epf *epf = container_of(nb, struct pci_epf, nb);
>>> + struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>>> + int ret;
>>> +
>>> + switch (val) {
>>> + case CORE_INIT:
>>> + ret = pci_epf_test_core_init(epf);
>>> + if (ret)
>>> + return NOTIFY_BAD;
>>> + break;
>>> +
>>> + case LINK_UP:
>>> + queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
>>> + msecs_to_jiffies(1));
>>> + break;
>>> +
>>> + default:
>>> + dev_err(&epf->dev, "Invalid EPF test notifier event\n");
>>> + return NOTIFY_BAD;
>>> + }
>>> +
>>> + return NOTIFY_OK;
>>> +}
>>> +
>>> static int pci_epf_test_alloc_space(struct pci_epf *epf)
>>> {
>>> struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>>> @@ -496,12 +556,11 @@ 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 skip_core_init = false;
>>> bool msix_capable = false;
>>> bool msi_capable = true;
>>> @@ -511,6 +570,7 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>>> epc_features = pci_epc_get_features(epc, epf->func_no);
>>> if (epc_features) {
>>> linkup_notifier = epc_features->linkup_notifier;
>>> + skip_core_init = epc_features->skip_core_init;
>>> msix_capable = epc_features->msix_capable;
>>> msi_capable = epc_features->msi_capable;
>>
>> Are these used anywhere in this function?
> Nope. I'll remove them.
>
>>> test_reg_bar = pci_epc_get_first_free_bar(epc_features);
>>> @@ -520,34 +580,14 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>>> 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) {
>>> - dev_err(dev, "Configuration header write failed\n");
>>> - return ret;
>>> - }
>>> -
>>> ret = pci_epf_test_alloc_space(epf);
>>> if (ret)
>>> return ret;
>>> - ret = pci_epf_test_set_bar(epf);
>>> - if (ret)
>>> - 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 (msix_capable) {
>>> - ret = pci_epc_set_msix(epc, epf->func_no,
>>> epf->msix_interrupts);
>>> - if (ret) {
>>> - dev_err(dev, "MSI-X configuration failed\n");
>>> + if (!skip_core_init) {
>>> + ret = pci_epf_test_core_init(epf);
>>> + if (ret)
>>> return ret;
>>> - }
>>> }
>>> if (linkup_notifier) {
>>
>> This could as well be moved to pci_epf_test_core_init().
> Yes, but I would like to keep only the code that touches hardware in
> pci_epf_test_core_init()
> to minimize the time it takes to execute it. Is there any strong reason
> to move it? if not,
> I would prefer to leave it here in this function itself.
There is no point in scheduling a work to check for commands from host
when the EP itself is not initialized.
Thanks
Kishon
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 4/4] PCI: pci-epf-test: Add support to defer core initialization
2019-12-05 11:22 ` Kishon Vijay Abraham I
@ 2020-01-03 9:40 ` Vidya Sagar
0 siblings, 0 replies; 23+ messages in thread
From: Vidya Sagar @ 2020-01-03 9:40 UTC (permalink / raw)
To: Kishon Vijay Abraham I, jingoohan1, gustavo.pimentel,
lorenzo.pieralisi, andrew.murray, bhelgaas, thierry.reding
Cc: Jisheng.Zhang, jonathanh, linux-pci, linux-kernel, kthota,
mmaddireddy, sagar.tv
On 12/5/2019 4:52 PM, Kishon Vijay Abraham I wrote:
> Hi,
>
> On 01/12/19 7:59 pm, Vidya Sagar wrote:
>> On 11/27/2019 2:50 PM, Kishon Vijay Abraham I wrote:
>>> Hi,
>>>
>>> On 13/11/19 2:38 PM, Vidya Sagar wrote:
>>>> Add support to defer core initialization and to receive a notifier
>>>> when core is ready to accommodate platforms where core is not for
>>>> initialization untile reference clock from host is available.
>>>>
>>>> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
>>>> ---
>>>> drivers/pci/endpoint/functions/pci-epf-test.c | 114 ++++++++++++------
>>>> 1 file changed, 77 insertions(+), 37 deletions(-)
>>>>
>>>> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
>>>> index bddff15052cc..068024fab544 100644
>>>> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
>>>> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
>>>> @@ -360,18 +360,6 @@ static void pci_epf_test_cmd_handler(struct work_struct *work)
>>>> msecs_to_jiffies(1));
>>>> }
>>>> -static int pci_epf_test_notifier(struct notifier_block *nb, unsigned long val,
>>>> - void *data)
>>>> -{
>>>> - struct pci_epf *epf = container_of(nb, struct pci_epf, nb);
>>>> - struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>>>> -
>>>> - queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
>>>> - msecs_to_jiffies(1));
>>>> -
>>>> - return NOTIFY_OK;
>>>> -}
>>>> -
>>>> static void pci_epf_test_unbind(struct pci_epf *epf)
>>>> {
>>>> struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>>>> @@ -428,6 +416,78 @@ static int pci_epf_test_set_bar(struct pci_epf *epf)
>>>> return 0;
>>>> }
>>>> +static int pci_epf_test_core_init(struct pci_epf *epf)
>>>> +{
>>>> + struct pci_epf_header *header = epf->header;
>>>> + const struct pci_epc_features *epc_features;
>>>> + struct pci_epc *epc = epf->epc;
>>>> + struct device *dev = &epf->dev;
>>>> + bool msix_capable = false;
>>>> + bool msi_capable = true;
>>>> + int ret;
>>>> +
>>>> + epc_features = pci_epc_get_features(epc, epf->func_no);
>>>> + if (epc_features) {
>>>> + msix_capable = epc_features->msix_capable;
>>>> + msi_capable = epc_features->msi_capable;
>>>> + }
>>>> +
>>>> + ret = pci_epc_write_header(epc, epf->func_no, header);
>>>> + if (ret) {
>>>> + dev_err(dev, "Configuration header write failed\n");
>>>> + return ret;
>>>> + }
>>>> +
>>>> + ret = pci_epf_test_set_bar(epf);
>>>> + if (ret)
>>>> + 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 (msix_capable) {
>>>> + ret = pci_epc_set_msix(epc, epf->func_no, epf->msix_interrupts);
>>>> + if (ret) {
>>>> + dev_err(dev, "MSI-X configuration failed\n");
>>>> + return ret;
>>>> + }
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +
>>>> +static int pci_epf_test_notifier(struct notifier_block *nb, unsigned long val,
>>>> + void *data)
>>>> +{
>>>> + struct pci_epf *epf = container_of(nb, struct pci_epf, nb);
>>>> + struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>>>> + int ret;
>>>> +
>>>> + switch (val) {
>>>> + case CORE_INIT:
>>>> + ret = pci_epf_test_core_init(epf);
>>>> + if (ret)
>>>> + return NOTIFY_BAD;
>>>> + break;
>>>> +
>>>> + case LINK_UP:
>>>> + queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
>>>> + msecs_to_jiffies(1));
>>>> + break;
>>>> +
>>>> + default:
>>>> + dev_err(&epf->dev, "Invalid EPF test notifier event\n");
>>>> + return NOTIFY_BAD;
>>>> + }
>>>> +
>>>> + return NOTIFY_OK;
>>>> +}
>>>> +
>>>> static int pci_epf_test_alloc_space(struct pci_epf *epf)
>>>> {
>>>> struct pci_epf_test *epf_test = epf_get_drvdata(epf);
>>>> @@ -496,12 +556,11 @@ 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 skip_core_init = false;
>>>> bool msix_capable = false;
>>>> bool msi_capable = true;
>>>> @@ -511,6 +570,7 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>>>> epc_features = pci_epc_get_features(epc, epf->func_no);
>>>> if (epc_features) {
>>>> linkup_notifier = epc_features->linkup_notifier;
>>>> + skip_core_init = epc_features->skip_core_init;
>>>> msix_capable = epc_features->msix_capable;
>>>> msi_capable = epc_features->msi_capable;
>>>
>>> Are these used anywhere in this function?
>> Nope. I'll remove them.
>>
>>>> test_reg_bar = pci_epc_get_first_free_bar(epc_features);
>>>> @@ -520,34 +580,14 @@ static int pci_epf_test_bind(struct pci_epf *epf)
>>>> 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) {
>>>> - dev_err(dev, "Configuration header write failed\n");
>>>> - return ret;
>>>> - }
>>>> -
>>>> ret = pci_epf_test_alloc_space(epf);
>>>> if (ret)
>>>> return ret;
>>>> - ret = pci_epf_test_set_bar(epf);
>>>> - if (ret)
>>>> - 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 (msix_capable) {
>>>> - ret = pci_epc_set_msix(epc, epf->func_no, epf->msix_interrupts);
>>>> - if (ret) {
>>>> - dev_err(dev, "MSI-X configuration failed\n");
>>>> + if (!skip_core_init) {
>>>> + ret = pci_epf_test_core_init(epf);
>>>> + if (ret)
>>>> return ret;
>>>> - }
>>>> }
>>>> if (linkup_notifier) {
>>>
>>> This could as well be moved to pci_epf_test_core_init().
>> Yes, but I would like to keep only the code that touches hardware in pci_epf_test_core_init()
>> to minimize the time it takes to execute it. Is there any strong reason to move it? if not,
>> I would prefer to leave it here in this function itself.
>
> There is no point in scheduling a work to check for commands from host when the EP itself is not initialized.
True. But, since this is more of preparatory work, I thought we should just have it done here itself.
Main reason being, once PERST is perceived, endpoint can't take too much initializing its core. So, I want to
keep that part as minimalistic as possible.
- Vidya Sagar
>
> Thanks
> Kishon
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/4] Add support to defer core initialization
2019-11-13 9:08 [PATCH 0/4] Add support to defer core initialization Vidya Sagar
` (3 preceding siblings ...)
2019-11-13 9:08 ` [PATCH 4/4] PCI: pci-epf-test: Add support to defer core initialization Vidya Sagar
@ 2019-11-18 6:55 ` Vidya Sagar
2019-11-18 16:43 ` Jingoo Han
4 siblings, 1 reply; 23+ messages in thread
From: Vidya Sagar @ 2019-11-18 6:55 UTC (permalink / raw)
To: jingoohan1, gustavo.pimentel, lorenzo.pieralisi, andrew.murray,
bhelgaas, kishon, thierry.reding
Cc: Jisheng.Zhang, jonathanh, linux-pci, linux-kernel, kthota,
mmaddireddy, sagar.tv
On 11/13/2019 2:38 PM, Vidya Sagar wrote:
> EPC/DesignWare core endpoint subsystems assume that the core registers are
> available always for SW to initialize. But, that may not be the case always.
> For example, Tegra194 hardware has the core running on a clock that is derived
> from reference clock that is coming into the endpoint system from host.
> Hence core is made available asynchronously based on when host system is going
> for enumeration of devices. To accommodate this kind of hardwares, support is
> required to defer the core initialization until the respective platform driver
> informs the EPC/DWC endpoint sub-systems that the core is indeed available for
> initiaization. This patch series is attempting to add precisely that.
> This series is based on Kishon's patch that adds notification mechanism
> support from EPC to EPF @ http://patchwork.ozlabs.org/patch/1109884/
>
> Vidya Sagar (4):
> PCI: dwc: Add new feature to skip core initialization
> PCI: endpoint: Add notification for core init completion
> PCI: dwc: Add API to notify core initialization completion
> PCI: pci-epf-test: Add support to defer core initialization
>
> .../pci/controller/dwc/pcie-designware-ep.c | 79 +++++++-----
> drivers/pci/controller/dwc/pcie-designware.h | 11 ++
> drivers/pci/endpoint/functions/pci-epf-test.c | 114 ++++++++++++------
> drivers/pci/endpoint/pci-epc-core.c | 19 ++-
> include/linux/pci-epc.h | 2 +
> include/linux/pci-epf.h | 5 +
> 6 files changed, 164 insertions(+), 66 deletions(-)
>
Hi Kishon / Gustavo / Jingoo,
Could you please take a look at this patch series?
- Vidya Sagar
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/4] Add support to defer core initialization
2019-11-18 6:55 ` [PATCH 0/4] " Vidya Sagar
@ 2019-11-18 16:43 ` Jingoo Han
2019-11-25 4:33 ` Vidya Sagar
0 siblings, 1 reply; 23+ messages in thread
From: Jingoo Han @ 2019-11-18 16:43 UTC (permalink / raw)
To: Vidya Sagar, gustavo.pimentel, lorenzo.pieralisi, andrew.murray,
bhelgaas, kishon, thierry.reding
Cc: Jisheng.Zhang, jonathanh, linux-pci, linux-kernel, kthota,
mmaddireddy, sagar.tv, Han Jingoo
On 11/18/19, 1:55 AM, Vidya Sagar wrote:
>
> On 11/13/2019 2:38 PM, Vidya Sagar wrote:
> > EPC/DesignWare core endpoint subsystems assume that the core registers are
> > available always for SW to initialize. But, that may not be the case always.
> > For example, Tegra194 hardware has the core running on a clock that is derived
> > from reference clock that is coming into the endpoint system from host.
> > Hence core is made available asynchronously based on when host system is going
> > for enumeration of devices. To accommodate this kind of hardwares, support is
> > required to defer the core initialization until the respective platform driver
> > informs the EPC/DWC endpoint sub-systems that the core is indeed available for
> > initiaization. This patch series is attempting to add precisely that.
> > This series is based on Kishon's patch that adds notification mechanism
> > support from EPC to EPF @ http://patchwork.ozlabs.org/patch/1109884/
> >
> > Vidya Sagar (4):
> > PCI: dwc: Add new feature to skip core initialization
> > PCI: endpoint: Add notification for core init completion
> > PCI: dwc: Add API to notify core initialization completion
> > PCI: pci-epf-test: Add support to defer core initialization
> >
> > .../pci/controller/dwc/pcie-designware-ep.c | 79 +++++++-----
> > drivers/pci/controller/dwc/pcie-designware.h | 11 ++
> > drivers/pci/endpoint/functions/pci-epf-test.c | 114 ++++++++++++------
> > drivers/pci/endpoint/pci-epc-core.c | 19 ++-
> > include/linux/pci-epc.h | 2 +
> > include/linux/pci-epf.h | 5 +
> > 6 files changed, 164 insertions(+), 66 deletions(-)
> >
>
> Hi Kishon / Gustavo / Jingoo,
> Could you please take a look at this patch series?
You need a Ack from Kishon, because he made EP code.
> - Vidya Sagar
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/4] Add support to defer core initialization
2019-11-18 16:43 ` Jingoo Han
@ 2019-11-25 4:33 ` Vidya Sagar
2019-11-25 4:45 ` Kishon Vijay Abraham I
0 siblings, 1 reply; 23+ messages in thread
From: Vidya Sagar @ 2019-11-25 4:33 UTC (permalink / raw)
To: kishon
Cc: Jingoo Han, gustavo.pimentel, lorenzo.pieralisi, andrew.murray,
bhelgaas, thierry.reding, Jisheng.Zhang, jonathanh, linux-pci,
linux-kernel, kthota, mmaddireddy, sagar.tv
On 11/18/2019 10:13 PM, Jingoo Han wrote:
>
>
> On 11/18/19, 1:55 AM, Vidya Sagar wrote:
>>
>> On 11/13/2019 2:38 PM, Vidya Sagar wrote:
>>> EPC/DesignWare core endpoint subsystems assume that the core registers are
>>> available always for SW to initialize. But, that may not be the case always.
>>> For example, Tegra194 hardware has the core running on a clock that is derived
>>> from reference clock that is coming into the endpoint system from host.
>>> Hence core is made available asynchronously based on when host system is going
>>> for enumeration of devices. To accommodate this kind of hardwares, support is
>>> required to defer the core initialization until the respective platform driver
>>> informs the EPC/DWC endpoint sub-systems that the core is indeed available for
>>> initiaization. This patch series is attempting to add precisely that.
>>> This series is based on Kishon's patch that adds notification mechanism
>>> support from EPC to EPF @ http://patchwork.ozlabs.org/patch/1109884/
>>>
>>> Vidya Sagar (4):
>>> PCI: dwc: Add new feature to skip core initialization
>>> PCI: endpoint: Add notification for core init completion
>>> PCI: dwc: Add API to notify core initialization completion
>>> PCI: pci-epf-test: Add support to defer core initialization
>>>
>>> .../pci/controller/dwc/pcie-designware-ep.c | 79 +++++++-----
>>> drivers/pci/controller/dwc/pcie-designware.h | 11 ++
>>> drivers/pci/endpoint/functions/pci-epf-test.c | 114 ++++++++++++------
>>> drivers/pci/endpoint/pci-epc-core.c | 19 ++-
>>> include/linux/pci-epc.h | 2 +
>>> include/linux/pci-epf.h | 5 +
>>> 6 files changed, 164 insertions(+), 66 deletions(-)
>>>
>>
>> Hi Kishon / Gustavo / Jingoo,
>> Could you please take a look at this patch series?
>
> You need a Ack from Kishon, because he made EP code.
Hi Kishon,
Could you please find time to review this series?
- Vidya Sagar
>
>
>> - Vidya Sagar
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH 0/4] Add support to defer core initialization
2019-11-25 4:33 ` Vidya Sagar
@ 2019-11-25 4:45 ` Kishon Vijay Abraham I
0 siblings, 0 replies; 23+ messages in thread
From: Kishon Vijay Abraham I @ 2019-11-25 4:45 UTC (permalink / raw)
To: Vidya Sagar
Cc: Jingoo Han, gustavo.pimentel, lorenzo.pieralisi, andrew.murray,
bhelgaas, thierry.reding, Jisheng.Zhang, jonathanh, linux-pci,
linux-kernel, kthota, mmaddireddy, sagar.tv
Hi,
On 25/11/19 10:03 AM, Vidya Sagar wrote:
> On 11/18/2019 10:13 PM, Jingoo Han wrote:
>>
>>
>> On 11/18/19, 1:55 AM, Vidya Sagar wrote:
>>>
>>> On 11/13/2019 2:38 PM, Vidya Sagar wrote:
>>>> EPC/DesignWare core endpoint subsystems assume that the core registers are
>>>> available always for SW to initialize. But, that may not be the case always.
>>>> For example, Tegra194 hardware has the core running on a clock that is derived
>>>> from reference clock that is coming into the endpoint system from host.
>>>> Hence core is made available asynchronously based on when host system is going
>>>> for enumeration of devices. To accommodate this kind of hardwares, support is
>>>> required to defer the core initialization until the respective platform driver
>>>> informs the EPC/DWC endpoint sub-systems that the core is indeed available for
>>>> initiaization. This patch series is attempting to add precisely that.
>>>> This series is based on Kishon's patch that adds notification mechanism
>>>> support from EPC to EPF @ http://patchwork.ozlabs.org/patch/1109884/
>>>>
>>>> Vidya Sagar (4):
>>>> PCI: dwc: Add new feature to skip core initialization
>>>> PCI: endpoint: Add notification for core init completion
>>>> PCI: dwc: Add API to notify core initialization completion
>>>> PCI: pci-epf-test: Add support to defer core initialization
>>>>
>>>> .../pci/controller/dwc/pcie-designware-ep.c | 79 +++++++-----
>>>> drivers/pci/controller/dwc/pcie-designware.h | 11 ++
>>>> drivers/pci/endpoint/functions/pci-epf-test.c | 114 ++++++++++++------
>>>> drivers/pci/endpoint/pci-epc-core.c | 19 ++-
>>>> include/linux/pci-epc.h | 2 +
>>>> include/linux/pci-epf.h | 5 +
>>>> 6 files changed, 164 insertions(+), 66 deletions(-)
>>>>
>>>
>>> Hi Kishon / Gustavo / Jingoo,
>>> Could you please take a look at this patch series?
>>
>> You need a Ack from Kishon, because he made EP code.
> Hi Kishon,
> Could you please find time to review this series?
I'll review it this week. Sorry for the delay.
-Kishon
^ permalink raw reply [flat|nested] 23+ messages in thread