Subject: [PATCH v2 2/8] PCI: qcom-ep: Add support for D-state change notification

Add support to pass D-state change notification to Endpoint
function driver.

Signed-off-by: Krishna chaitanya chundru <[email protected]>
---
drivers/pci/controller/dwc/pcie-qcom-ep.c | 6 ++++++
1 file changed, 6 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
index 1435f51..e75aec4 100644
--- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
+++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
@@ -561,6 +561,7 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data)
struct device *dev = pci->dev;
u32 status = readl_relaxed(pcie_ep->parf + PARF_INT_ALL_STATUS);
u32 mask = readl_relaxed(pcie_ep->parf + PARF_INT_ALL_MASK);
+ pci_power_t state;
u32 dstate, val;

writel_relaxed(status, pcie_ep->parf + PARF_INT_ALL_CLEAR);
@@ -583,11 +584,16 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data)
dstate = dw_pcie_readl_dbi(pci, DBI_CON_STATUS) &
DBI_CON_STATUS_POWER_STATE_MASK;
dev_dbg(dev, "Received D%d state event\n", dstate);
+ state = dstate;
if (dstate == 3) {
val = readl_relaxed(pcie_ep->parf + PARF_PM_CTRL);
val |= PARF_PM_CTRL_REQ_EXIT_L1;
writel_relaxed(val, pcie_ep->parf + PARF_PM_CTRL);
+ state = PCI_D3hot;
+ if (gpiod_get_value(pcie_ep->reset))
+ state = PCI_D3cold;
}
+ pci_epc_dstate_change(pci->ep.epc, state);
} else if (FIELD_GET(PARF_INT_ALL_LINK_UP, status)) {
dev_dbg(dev, "Received Linkup event. Enumeration complete!\n");
dw_pcie_ep_linkup(&pci->ep);
--
2.7.4



2023-07-07 05:59:58

by Manivannan Sadhasivam

[permalink] [raw]
Subject: Re: [PATCH v2 2/8] PCI: qcom-ep: Add support for D-state change notification

On Fri, Jun 30, 2023 at 04:22:05PM +0530, Krishna chaitanya chundru wrote:
> Add support to pass D-state change notification to Endpoint
> function driver.
>
> Signed-off-by: Krishna chaitanya chundru <[email protected]>

Few comments below.

> ---
> drivers/pci/controller/dwc/pcie-qcom-ep.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> index 1435f51..e75aec4 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
> @@ -561,6 +561,7 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data)
> struct device *dev = pci->dev;
> u32 status = readl_relaxed(pcie_ep->parf + PARF_INT_ALL_STATUS);
> u32 mask = readl_relaxed(pcie_ep->parf + PARF_INT_ALL_MASK);
> + pci_power_t state;
> u32 dstate, val;
>
> writel_relaxed(status, pcie_ep->parf + PARF_INT_ALL_CLEAR);
> @@ -583,11 +584,16 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data)
> dstate = dw_pcie_readl_dbi(pci, DBI_CON_STATUS) &
> DBI_CON_STATUS_POWER_STATE_MASK;
> dev_dbg(dev, "Received D%d state event\n", dstate);

We should also change this debug output to make use of pci_power_name() based on
the "state" variable and move it just above pci_epc_dstate_change(). But this
should be a separate patch.

> + state = dstate;
> if (dstate == 3) {
> val = readl_relaxed(pcie_ep->parf + PARF_PM_CTRL);
> val |= PARF_PM_CTRL_REQ_EXIT_L1;
> writel_relaxed(val, pcie_ep->parf + PARF_PM_CTRL);
> + state = PCI_D3hot;
> + if (gpiod_get_value(pcie_ep->reset))
> + state = PCI_D3cold;
> }
> + pci_epc_dstate_change(pci->ep.epc, state);

Need to change this API as per my comment on patch 1.

- Mani

> } else if (FIELD_GET(PARF_INT_ALL_LINK_UP, status)) {
> dev_dbg(dev, "Received Linkup event. Enumeration complete!\n");
> dw_pcie_ep_linkup(&pci->ep);
> --
> 2.7.4
>

--
மணிவண்ணன் சதாசிவம்

Subject: Re: [PATCH v2 2/8] PCI: qcom-ep: Add support for D-state change notification


On 7/7/2023 11:21 AM, Manivannan Sadhasivam wrote:
> On Fri, Jun 30, 2023 at 04:22:05PM +0530, Krishna chaitanya chundru wrote:
>> Add support to pass D-state change notification to Endpoint
>> function driver.
>>
>> Signed-off-by: Krishna chaitanya chundru <[email protected]>
> Few comments below.
>
>> ---
>> drivers/pci/controller/dwc/pcie-qcom-ep.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-qcom-ep.c b/drivers/pci/controller/dwc/pcie-qcom-ep.c
>> index 1435f51..e75aec4 100644
>> --- a/drivers/pci/controller/dwc/pcie-qcom-ep.c
>> +++ b/drivers/pci/controller/dwc/pcie-qcom-ep.c
>> @@ -561,6 +561,7 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data)
>> struct device *dev = pci->dev;
>> u32 status = readl_relaxed(pcie_ep->parf + PARF_INT_ALL_STATUS);
>> u32 mask = readl_relaxed(pcie_ep->parf + PARF_INT_ALL_MASK);
>> + pci_power_t state;
>> u32 dstate, val;
>>
>> writel_relaxed(status, pcie_ep->parf + PARF_INT_ALL_CLEAR);
>> @@ -583,11 +584,16 @@ static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data)
>> dstate = dw_pcie_readl_dbi(pci, DBI_CON_STATUS) &
>> DBI_CON_STATUS_POWER_STATE_MASK;
>> dev_dbg(dev, "Received D%d state event\n", dstate);
> We should also change this debug output to make use of pci_power_name() based on
> the "state" variable and move it just above pci_epc_dstate_change(). But this
> should be a separate patch.
I will add a seperate patch in the same series
>
>> + state = dstate;
>> if (dstate == 3) {
>> val = readl_relaxed(pcie_ep->parf + PARF_PM_CTRL);
>> val |= PARF_PM_CTRL_REQ_EXIT_L1;
>> writel_relaxed(val, pcie_ep->parf + PARF_PM_CTRL);
>> + state = PCI_D3hot;
>> + if (gpiod_get_value(pcie_ep->reset))
>> + state = PCI_D3cold;
>> }
>> + pci_epc_dstate_change(pci->ep.epc, state);
> Need to change this API as per my comment on patch 1.
>
> - Mani

sure will change it as you suggested.

- KC

>
>> } else if (FIELD_GET(PARF_INT_ALL_LINK_UP, status)) {
>> dev_dbg(dev, "Received Linkup event. Enumeration complete!\n");
>> dw_pcie_ep_linkup(&pci->ep);
>> --
>> 2.7.4
>>

2023-07-13 21:23:38

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH v2 2/8] PCI: qcom-ep: Add support for D-state change notification

Hi Krishna,

kernel test robot noticed the following build warnings:

[auto build test WARNING on pci/next]
[also build test WARNING on pci/for-linus linus/master v6.5-rc1 next-20230713]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Krishna-chaitanya-chundru/PCI-endpoint-Add-dstate-change-notifier-support/20230630-190228
base: https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git next
patch link: https://lore.kernel.org/r/1688122331-25478-3-git-send-email-quic_krichai%40quicinc.com
patch subject: [PATCH v2 2/8] PCI: qcom-ep: Add support for D-state change notification
config: arm-randconfig-r081-20230713 (https://download.01.org/0day-ci/archive/20230714/[email protected]/config)
compiler: arm-linux-gnueabi-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230714/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

sparse warnings: (new ones prefixed by >>)
>> drivers/pci/controller/dwc/pcie-qcom-ep.c:587:23: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted pci_power_t [usertype] state @@ got unsigned int [assigned] [usertype] dstate @@
drivers/pci/controller/dwc/pcie-qcom-ep.c:587:23: sparse: expected restricted pci_power_t [usertype] state
drivers/pci/controller/dwc/pcie-qcom-ep.c:587:23: sparse: got unsigned int [assigned] [usertype] dstate

vim +587 drivers/pci/controller/dwc/pcie-qcom-ep.c

555
556 /* TODO: Notify clients about PCIe state change */
557 static irqreturn_t qcom_pcie_ep_global_irq_thread(int irq, void *data)
558 {
559 struct qcom_pcie_ep *pcie_ep = data;
560 struct dw_pcie *pci = &pcie_ep->pci;
561 struct device *dev = pci->dev;
562 u32 status = readl_relaxed(pcie_ep->parf + PARF_INT_ALL_STATUS);
563 u32 mask = readl_relaxed(pcie_ep->parf + PARF_INT_ALL_MASK);
564 pci_power_t state;
565 u32 dstate, val;
566
567 writel_relaxed(status, pcie_ep->parf + PARF_INT_ALL_CLEAR);
568 status &= mask;
569
570 if (FIELD_GET(PARF_INT_ALL_LINK_DOWN, status)) {
571 dev_dbg(dev, "Received Linkdown event\n");
572 pcie_ep->link_status = QCOM_PCIE_EP_LINK_DOWN;
573 pci_epc_linkdown(pci->ep.epc);
574 } else if (FIELD_GET(PARF_INT_ALL_BME, status)) {
575 dev_dbg(dev, "Received BME event. Link is enabled!\n");
576 pcie_ep->link_status = QCOM_PCIE_EP_LINK_ENABLED;
577 pci_epc_bme_notify(pci->ep.epc);
578 } else if (FIELD_GET(PARF_INT_ALL_PM_TURNOFF, status)) {
579 dev_dbg(dev, "Received PM Turn-off event! Entering L23\n");
580 val = readl_relaxed(pcie_ep->parf + PARF_PM_CTRL);
581 val |= PARF_PM_CTRL_READY_ENTR_L23;
582 writel_relaxed(val, pcie_ep->parf + PARF_PM_CTRL);
583 } else if (FIELD_GET(PARF_INT_ALL_DSTATE_CHANGE, status)) {
584 dstate = dw_pcie_readl_dbi(pci, DBI_CON_STATUS) &
585 DBI_CON_STATUS_POWER_STATE_MASK;
586 dev_dbg(dev, "Received D%d state event\n", dstate);
> 587 state = dstate;
588 if (dstate == 3) {
589 val = readl_relaxed(pcie_ep->parf + PARF_PM_CTRL);
590 val |= PARF_PM_CTRL_REQ_EXIT_L1;
591 writel_relaxed(val, pcie_ep->parf + PARF_PM_CTRL);
592 state = PCI_D3hot;
593 if (gpiod_get_value(pcie_ep->reset))
594 state = PCI_D3cold;
595 }
596 pci_epc_dstate_change(pci->ep.epc, state);
597 } else if (FIELD_GET(PARF_INT_ALL_LINK_UP, status)) {
598 dev_dbg(dev, "Received Linkup event. Enumeration complete!\n");
599 dw_pcie_ep_linkup(&pci->ep);
600 pcie_ep->link_status = QCOM_PCIE_EP_LINK_UP;
601 } else {
602 dev_dbg(dev, "Received unknown event: %d\n", status);
603 }
604
605 return IRQ_HANDLED;
606 }
607

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki