Messages in this thread Patch in this message | | | Date | Wed, 29 Mar 2023 14:44:34 +0530 | Subject | Re: [Patch v4 10/10] PCI: tegra194: add interconnect support in Tegra234 | From | Sumit Gupta <> |
| |
On 28/03/23 23:23, Bjorn Helgaas wrote: > External email: Use caution opening links or attachments > > > Capitalize subject line please, to match pcie-tegra194.c history. > > On Mon, Mar 27, 2023 at 09:44:26PM +0530, Sumit Gupta wrote: >> Add support to request DRAM bandwidth with Memory Interconnect >> in Tegra234 SoC. The DRAM BW required for different modes depends >> on speed (Gen-1/2/3/4) and width/lanes (x1/x2/x4/x8). >> >> Suggested-by: Manikanta Maddireddy <mmaddireddy@nvidia.com> >> Signed-off-by: Sumit Gupta <sumitg@nvidia.com> >> --- >> drivers/pci/controller/dwc/pcie-tegra194.c | 40 +++++++++++++++++----- >> 1 file changed, 32 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/pci/controller/dwc/pcie-tegra194.c b/drivers/pci/controller/dwc/pcie-tegra194.c >> index 09825b4a075e..d2513c9d3feb 100644 >> --- a/drivers/pci/controller/dwc/pcie-tegra194.c >> +++ b/drivers/pci/controller/dwc/pcie-tegra194.c >> @@ -15,6 +15,7 @@ >> #include <linux/gpio.h> >> #include <linux/gpio/consumer.h> >> #include <linux/interrupt.h> >> +#include <linux/interconnect.h> > > Almost alphabetized, swap interrupt.h and interconnect.h. > Ok, will swap.
>> #include <linux/iopoll.h> >> #include <linux/kernel.h> >> #include <linux/module.h> >> @@ -287,6 +288,7 @@ struct tegra_pcie_dw { >> unsigned int pex_rst_irq; >> int ep_state; >> long link_status; >> + struct icc_path *icc_path; >> }; >> >> static inline struct tegra_pcie_dw *to_tegra_pcie(struct dw_pcie *pci) >> @@ -309,6 +311,24 @@ struct tegra_pcie_soc { >> enum dw_pcie_device_mode mode; >> }; >> >> +static void tegra_pcie_icc_set(struct tegra_pcie_dw *pcie) >> +{ >> + struct dw_pcie *pci = &pcie->pci; >> + u32 val, speed, width; >> + >> + val = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA); >> + >> + speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, val); >> + width = FIELD_GET(PCI_EXP_LNKSTA_NLW, val); >> + >> + val = width * (PCIE_SPEED2MBS_ENC(pcie_link_speed[speed]) / BITS_PER_BYTE); >> + >> + if (icc_set_bw(pcie->icc_path, MBps_to_icc(val), 0)) >> + dev_err(pcie->dev, "can't set bw[%u]\n", val); >> + >> + clk_set_rate(pcie->core_clk, pcie_gen_freq[speed - 1]); > > Array bounds violation; PCI_EXP_LNKSTA_CLS is 0x000f, so possible > speed (CLS) values are 0..0xf and "speed - 1" values are -1..0xe. > > pcie_gen_freq[] is of size 4 (valid indices 0..3). > > I see that you're just *moving* this code, but might as well fix it. > Thank you for the review. Will include the below change in the same patch. Please let me know if any issue.
- clk_set_rate(pcie->core_clk, pcie_gen_freq[speed - 1]); + if (speed && (speed <= ARRAY_SIZE(pcie_gen_freq))) + clk_set_rate(pcie->core_clk, pcie_gen_freq[speed - 1]); + else + clk_set_rate(pcie->core_clk, pcie_gen_freq[0]);
Thank you, Sumit Gupta
>> +} >> + >> static void apply_bad_link_workaround(struct dw_pcie_rp *pp) >> { >> struct dw_pcie *pci = to_dw_pcie_from_pp(pp); >> @@ -452,14 +472,12 @@ static irqreturn_t tegra_pcie_ep_irq_thread(int irq, void *arg) >> struct tegra_pcie_dw *pcie = arg; >> struct dw_pcie_ep *ep = &pcie->pci.ep; >> struct dw_pcie *pci = &pcie->pci; >> - u32 val, speed; >> + u32 val; >> >> if (test_and_clear_bit(0, &pcie->link_status)) >> dw_pcie_ep_linkup(ep); >> >> - speed = dw_pcie_readw_dbi(pci, pcie->pcie_cap_base + PCI_EXP_LNKSTA) & >> - PCI_EXP_LNKSTA_CLS; >> - clk_set_rate(pcie->core_clk, pcie_gen_freq[speed - 1]); >> + tegra_pcie_icc_set(pcie);
| |