From: Baruch Siach <baruch@tkos.co.il> To: "Pali Rohár" <pali@kernel.org> Cc: Andy Gross <agross@kernel.org>, Bjorn Andersson <bjorn.andersson@linaro.org>, Selvam Sathappan Periakaruppan <speriaka@codeaurora.org>, Kathiravan T <kathirav@codeaurora.org>, Bjorn Helgaas <bhelgaas@google.com>, Rob Herring <robh+dt@kernel.org>, Thierry Reding <thierry.reding@gmail.com>, Jonathan Hunter <jonathanh@nvidia.com>, Jingoo Han <jingoohan1@gmail.com>, Gustavo Pimentel <gustavo.pimentel@synopsys.com>, Robert Marko <robert.marko@sartura.hr>, Bryan O'Donoghue <pure.logic@nexus-software.ie>, linux-pci@vger.kernel.org, linux-arm-msm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-tegra@vger.kernel.org, Stanimir Varbanov <svarbanov@mm-sol.com> Subject: Re: [PATCH v6 2/3] PCI: qcom: Define slot capabilities using PCI_EXP_SLTCAP_* Date: Thu, 09 Jun 2022 06:27:34 +0300 [thread overview] Message-ID: <87r13ymrf2.fsf@tarshish> (raw) In-Reply-To: <20220608230008.acp6lwu6xjin62ql@pali> Hi Pali, On Thu, Jun 09 2022, Pali Rohár wrote: > On Monday 07 February 2022 16:51:25 Baruch Siach wrote: >> From: Baruch Siach <baruch.siach@siklu.com> >> >> The PCIE_CAP_LINK1_VAL macro actually defines slot capabilities. Use >> PCI_EXP_SLTCAP_* macros to spell its value, and rename it to better >> describe its meaning. >> >> Signed-off-by: Baruch Siach <baruch.siach@siklu.com> >> --- >> drivers/pci/controller/dwc/pcie-qcom.c | 15 +++++++++++++-- >> 1 file changed, 13 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c >> index c19cd506ed3f..01e58b057d2a 100644 >> --- a/drivers/pci/controller/dwc/pcie-qcom.c >> +++ b/drivers/pci/controller/dwc/pcie-qcom.c >> @@ -69,7 +69,18 @@ >> #define PCIE20_AXI_MSTR_RESP_COMP_CTRL1 0x81c >> #define CFG_BRIDGE_SB_INIT BIT(0) >> >> -#define PCIE_CAP_LINK1_VAL 0x2FD7F >> +#define PCIE_CAP_SLOT_POWER_LIMIT_VAL 0x7D00 >> +#define PCIE_CAP_SLOT_POWER_LIMIT_SCALE 0x8000 > > Hello! > > Please do not use hardcoded values for slot power limit value and scale > numbers. There are macros PCI_EXP_SLTCAP_SPLV and PCI_EXP_SLTCAP_SPLS > for composing mask: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/pci_regs.h?h=v5.19-rc1#n593 > Which could be used together with FIELD_PREP(). See e.g. aardvark commit: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0d5b8c298545c827ca9f2461b2655277ce0aef79 Thanks for the tip. > And the important information: Slot power limit is board specific and > depends on how power supply and power regulators are designed. So slot > power limit **cannot** be hardcoded in driver. Instead this value should > be read from device tree file for the current board. > > There is a new kernel function of_pci_get_slot_power_limit() which reads > it and compose PCIe slot power limit value and scale numbers. See: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/of.c?h=v5.19-rc1#n631 The 'slot-power-limit-milliwatt' property appears to be undocumented as of v5.19-rc1. This patch should make no functional change. I guess we should keep the default hard-coded driver value for compatibility with existing DTs with no 'slot-power-limit-milliwatt'. Thanks, baruch >> +#define PCIE_CAP_SLOT_VAL (PCI_EXP_SLTCAP_ABP | \ >> + PCI_EXP_SLTCAP_PCP | \ >> + PCI_EXP_SLTCAP_MRLSP | \ >> + PCI_EXP_SLTCAP_AIP | \ >> + PCI_EXP_SLTCAP_PIP | \ >> + PCI_EXP_SLTCAP_HPS | \ >> + PCI_EXP_SLTCAP_HPC | \ >> + PCI_EXP_SLTCAP_EIP | \ >> + PCIE_CAP_SLOT_POWER_LIMIT_VAL | \ >> + PCIE_CAP_SLOT_POWER_LIMIT_SCALE) >> >> #define PCIE20_PARF_Q2A_FLUSH 0x1AC >> >> @@ -1111,7 +1122,7 @@ static int qcom_pcie_init_2_3_3(struct qcom_pcie *pcie) >> >> writel(PCI_COMMAND_MASTER, pci->dbi_base + PCI_COMMAND); >> writel(DBI_RO_WR_EN, pci->dbi_base + PCIE20_MISC_CONTROL_1_REG); >> - writel(PCIE_CAP_LINK1_VAL, pci->dbi_base + offset + PCI_EXP_SLTCAP); >> + writel(PCIE_CAP_SLOT_VAL, pci->dbi_base + offset + PCI_EXP_SLTCAP); >> >> val = readl(pci->dbi_base + offset + PCI_EXP_LNKCAP); >> val &= ~PCI_EXP_LNKCAP_ASPMS; >> -- >> 2.34.1 >> -- ~. .~ Tk Open Systems =}------------------------------------------------ooO--U--Ooo------------{= - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
WARNING: multiple messages have this Message-ID (diff)
From: Baruch Siach <baruch@tkos.co.il> To: "Pali Rohár" <pali@kernel.org> Cc: Andy Gross <agross@kernel.org>, Bjorn Andersson <bjorn.andersson@linaro.org>, Selvam Sathappan Periakaruppan <speriaka@codeaurora.org>, Kathiravan T <kathirav@codeaurora.org>, Bjorn Helgaas <bhelgaas@google.com>, Rob Herring <robh+dt@kernel.org>, Thierry Reding <thierry.reding@gmail.com>, Jonathan Hunter <jonathanh@nvidia.com>, Jingoo Han <jingoohan1@gmail.com>, Gustavo Pimentel <gustavo.pimentel@synopsys.com>, Robert Marko <robert.marko@sartura.hr>, Bryan O'Donoghue <pure.logic@nexus-software.ie>, linux-pci@vger.kernel.org, linux-arm-msm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-tegra@vger.kernel.org, Stanimir Varbanov <svarbanov@mm-sol.com> Subject: Re: [PATCH v6 2/3] PCI: qcom: Define slot capabilities using PCI_EXP_SLTCAP_* Date: Thu, 09 Jun 2022 06:27:34 +0300 [thread overview] Message-ID: <87r13ymrf2.fsf@tarshish> (raw) In-Reply-To: <20220608230008.acp6lwu6xjin62ql@pali> Hi Pali, On Thu, Jun 09 2022, Pali Rohár wrote: > On Monday 07 February 2022 16:51:25 Baruch Siach wrote: >> From: Baruch Siach <baruch.siach@siklu.com> >> >> The PCIE_CAP_LINK1_VAL macro actually defines slot capabilities. Use >> PCI_EXP_SLTCAP_* macros to spell its value, and rename it to better >> describe its meaning. >> >> Signed-off-by: Baruch Siach <baruch.siach@siklu.com> >> --- >> drivers/pci/controller/dwc/pcie-qcom.c | 15 +++++++++++++-- >> 1 file changed, 13 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c >> index c19cd506ed3f..01e58b057d2a 100644 >> --- a/drivers/pci/controller/dwc/pcie-qcom.c >> +++ b/drivers/pci/controller/dwc/pcie-qcom.c >> @@ -69,7 +69,18 @@ >> #define PCIE20_AXI_MSTR_RESP_COMP_CTRL1 0x81c >> #define CFG_BRIDGE_SB_INIT BIT(0) >> >> -#define PCIE_CAP_LINK1_VAL 0x2FD7F >> +#define PCIE_CAP_SLOT_POWER_LIMIT_VAL 0x7D00 >> +#define PCIE_CAP_SLOT_POWER_LIMIT_SCALE 0x8000 > > Hello! > > Please do not use hardcoded values for slot power limit value and scale > numbers. There are macros PCI_EXP_SLTCAP_SPLV and PCI_EXP_SLTCAP_SPLS > for composing mask: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/pci_regs.h?h=v5.19-rc1#n593 > Which could be used together with FIELD_PREP(). See e.g. aardvark commit: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=0d5b8c298545c827ca9f2461b2655277ce0aef79 Thanks for the tip. > And the important information: Slot power limit is board specific and > depends on how power supply and power regulators are designed. So slot > power limit **cannot** be hardcoded in driver. Instead this value should > be read from device tree file for the current board. > > There is a new kernel function of_pci_get_slot_power_limit() which reads > it and compose PCIe slot power limit value and scale numbers. See: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/pci/of.c?h=v5.19-rc1#n631 The 'slot-power-limit-milliwatt' property appears to be undocumented as of v5.19-rc1. This patch should make no functional change. I guess we should keep the default hard-coded driver value for compatibility with existing DTs with no 'slot-power-limit-milliwatt'. Thanks, baruch >> +#define PCIE_CAP_SLOT_VAL (PCI_EXP_SLTCAP_ABP | \ >> + PCI_EXP_SLTCAP_PCP | \ >> + PCI_EXP_SLTCAP_MRLSP | \ >> + PCI_EXP_SLTCAP_AIP | \ >> + PCI_EXP_SLTCAP_PIP | \ >> + PCI_EXP_SLTCAP_HPS | \ >> + PCI_EXP_SLTCAP_HPC | \ >> + PCI_EXP_SLTCAP_EIP | \ >> + PCIE_CAP_SLOT_POWER_LIMIT_VAL | \ >> + PCIE_CAP_SLOT_POWER_LIMIT_SCALE) >> >> #define PCIE20_PARF_Q2A_FLUSH 0x1AC >> >> @@ -1111,7 +1122,7 @@ static int qcom_pcie_init_2_3_3(struct qcom_pcie *pcie) >> >> writel(PCI_COMMAND_MASTER, pci->dbi_base + PCI_COMMAND); >> writel(DBI_RO_WR_EN, pci->dbi_base + PCIE20_MISC_CONTROL_1_REG); >> - writel(PCIE_CAP_LINK1_VAL, pci->dbi_base + offset + PCI_EXP_SLTCAP); >> + writel(PCIE_CAP_SLOT_VAL, pci->dbi_base + offset + PCI_EXP_SLTCAP); >> >> val = readl(pci->dbi_base + offset + PCI_EXP_LNKCAP); >> val &= ~PCI_EXP_LNKCAP_ASPMS; >> -- >> 2.34.1 >> -- ~. .~ Tk Open Systems =}------------------------------------------------ooO--U--Ooo------------{= - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il - _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2022-06-09 3:44 UTC|newest] Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-02-07 14:51 [PATCH v6 0/3] PCI: IPQ6018 platform support Baruch Siach 2022-02-07 14:51 ` Baruch Siach 2022-02-07 14:51 ` [PATCH v6 1/3] PCI: dwc: tegra: move GEN3_RELATED DBI register to common header Baruch Siach 2022-02-07 14:51 ` Baruch Siach 2022-02-07 14:51 ` [PATCH v6 2/3] PCI: qcom: Define slot capabilities using PCI_EXP_SLTCAP_* Baruch Siach 2022-02-07 14:51 ` Baruch Siach 2022-06-08 23:00 ` Pali Rohár 2022-06-08 23:00 ` Pali Rohár 2022-06-09 3:27 ` Baruch Siach [this message] 2022-06-09 3:27 ` Baruch Siach 2022-06-09 8:47 ` Pali Rohár 2022-06-09 8:47 ` Pali Rohár 2022-02-07 14:51 ` [PATCH v6 3/3] PCI: qcom: Add IPQ60xx support Baruch Siach 2022-02-07 14:51 ` Baruch Siach 2022-02-11 16:06 ` [PATCH v6 0/3] PCI: IPQ6018 platform support Lorenzo Pieralisi 2022-02-11 16:06 ` Lorenzo Pieralisi 2022-03-15 13:04 ` Robert Marko 2022-03-15 13:04 ` Robert Marko 2022-03-15 13:20 ` Baruch Siach 2022-03-15 13:20 ` Baruch Siach 2022-03-15 13:41 ` Robert Marko 2022-03-15 13:41 ` Robert Marko 2022-04-12 16:12 ` Lorenzo Pieralisi 2022-04-12 16:12 ` Lorenzo Pieralisi 2022-05-11 14:03 ` Lorenzo Pieralisi 2022-05-11 14:03 ` Lorenzo Pieralisi 2022-06-07 13:12 ` Robert Marko 2022-06-07 13:12 ` Robert Marko 2022-06-08 20:24 ` Bjorn Helgaas 2022-06-08 20:24 ` Bjorn Helgaas 2022-06-09 13:10 ` Baruch Siach 2022-06-09 13:10 ` Baruch Siach
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=87r13ymrf2.fsf@tarshish \ --to=baruch@tkos.co.il \ --cc=agross@kernel.org \ --cc=bhelgaas@google.com \ --cc=bjorn.andersson@linaro.org \ --cc=gustavo.pimentel@synopsys.com \ --cc=jingoohan1@gmail.com \ --cc=jonathanh@nvidia.com \ --cc=kathirav@codeaurora.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-pci@vger.kernel.org \ --cc=linux-tegra@vger.kernel.org \ --cc=pali@kernel.org \ --cc=pure.logic@nexus-software.ie \ --cc=robert.marko@sartura.hr \ --cc=robh+dt@kernel.org \ --cc=speriaka@codeaurora.org \ --cc=svarbanov@mm-sol.com \ --cc=thierry.reding@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.