* [PATCH] PCI: keystone: Link training retries initiation @ 2019-10-07 8:41 Yurii Monakov 2019-12-16 11:08 ` Andrew Murray 0 siblings, 1 reply; 4+ messages in thread From: Yurii Monakov @ 2019-10-07 8:41 UTC (permalink / raw) To: linux-pci; +Cc: m-karicheri2, andrew.murray ks_pcie_stop_link function never cleared LTSSM_EN_VAL bit so link training was never triggered more than once after startup. In configurations where link can be unstable during early boot, for example, under low temperature, it will never be established. Signed-off-by: Yurii Monakov <monakov.y@gmail.com> --- drivers/pci/controller/dwc/pci-keystone.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c index f19de60ac991..ea8e7ebd8c4f 100644 --- a/drivers/pci/controller/dwc/pci-keystone.c +++ b/drivers/pci/controller/dwc/pci-keystone.c @@ -510,7 +510,7 @@ static void ks_pcie_stop_link(struct dw_pcie *pci) /* Disable Link training */ val = ks_pcie_app_readl(ks_pcie, CMD_STATUS); val &= ~LTSSM_EN_VAL; - ks_pcie_app_writel(ks_pcie, CMD_STATUS, LTSSM_EN_VAL | val); + ks_pcie_app_writel(ks_pcie, CMD_STATUS, val); } static int ks_pcie_start_link(struct dw_pcie *pci) -- 2.17.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] PCI: keystone: Link training retries initiation 2019-10-07 8:41 [PATCH] PCI: keystone: Link training retries initiation Yurii Monakov @ 2019-12-16 11:08 ` Andrew Murray 2019-12-17 7:40 ` Yurii Monakov 0 siblings, 1 reply; 4+ messages in thread From: Andrew Murray @ 2019-12-16 11:08 UTC (permalink / raw) To: Yurii Monakov; +Cc: linux-pci, m-karicheri2 On Mon, Oct 07, 2019 at 11:41:59AM +0300, Yurii Monakov wrote: > ks_pcie_stop_link function never cleared LTSSM_EN_VAL bit so > link training was never triggered more than once after startup. > In configurations where link can be unstable during early boot, > for example, under low temperature, it will never be established. > > Signed-off-by: Yurii Monakov <monakov.y@gmail.com> > --- > drivers/pci/controller/dwc/pci-keystone.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c > index f19de60ac991..ea8e7ebd8c4f 100644 > --- a/drivers/pci/controller/dwc/pci-keystone.c > +++ b/drivers/pci/controller/dwc/pci-keystone.c > @@ -510,7 +510,7 @@ static void ks_pcie_stop_link(struct dw_pcie *pci) > /* Disable Link training */ > val = ks_pcie_app_readl(ks_pcie, CMD_STATUS); > val &= ~LTSSM_EN_VAL; > - ks_pcie_app_writel(ks_pcie, CMD_STATUS, LTSSM_EN_VAL | val); Oh yeah, that doesn't look right to me. Good spot. Thanks for this! > + ks_pcie_app_writel(ks_pcie, CMD_STATUS, val); As far as I can work out, this bug existed from the beginning - can you please resend with this tag? Fixes: 0c4ffcfe1fbc ("PCI: keystone: Add TI Keystone PCIe driver") Can you also update the commit subject to emphasise it's a bug fix, e.g. PCI: keystone: Fix ... or similar. Thanks, Andrew Murray > } > > static int ks_pcie_start_link(struct dw_pcie *pci) > -- > 2.17.1 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] PCI: keystone: Link training retries initiation 2019-12-16 11:08 ` Andrew Murray @ 2019-12-17 7:40 ` Yurii Monakov 2019-12-17 9:40 ` Andrew Murray 0 siblings, 1 reply; 4+ messages in thread From: Yurii Monakov @ 2019-12-17 7:40 UTC (permalink / raw) To: Andrew Murray; +Cc: linux-pci, m-karicheri2 On Mon, 16 Dec 2019 11:08:22 +0000, Andrew Murray <andrew.murray@arm.com> wrote: > On Mon, Oct 07, 2019 at 11:41:59AM +0300, Yurii Monakov wrote: > > ks_pcie_stop_link function never cleared LTSSM_EN_VAL bit so > > link training was never triggered more than once after startup. > > In configurations where link can be unstable during early boot, > > for example, under low temperature, it will never be established. > > > > Signed-off-by: Yurii Monakov <monakov.y@gmail.com> > > --- > > drivers/pci/controller/dwc/pci-keystone.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/pci/controller/dwc/pci-keystone.c > > b/drivers/pci/controller/dwc/pci-keystone.c index > > f19de60ac991..ea8e7ebd8c4f 100644 --- > > a/drivers/pci/controller/dwc/pci-keystone.c +++ > > b/drivers/pci/controller/dwc/pci-keystone.c @@ -510,7 +510,7 @@ > > static void ks_pcie_stop_link(struct dw_pcie *pci) /* Disable Link > > training */ val = ks_pcie_app_readl(ks_pcie, CMD_STATUS); > > val &= ~LTSSM_EN_VAL; > > - ks_pcie_app_writel(ks_pcie, CMD_STATUS, LTSSM_EN_VAL | > > val); > > Oh yeah, that doesn't look right to me. Good spot. Thanks for this! > > > + ks_pcie_app_writel(ks_pcie, CMD_STATUS, val); > > As far as I can work out, this bug existed from the beginning - can > you please resend with this tag? > > Fixes: 0c4ffcfe1fbc ("PCI: keystone: Add TI Keystone PCIe driver") I have to add this line prior to 'Signed-off-by' tag? > Can you also update the commit subject to emphasise it's a bug fix, > e.g. PCI: keystone: Fix ... or similar. Do you mean that I have to create new patch from scratch and send it again? Best Regards, Yurii Monakov PS. I'm new to LKLM, so sorry for dumb questions. > > Thanks, > > Andrew Murray > > > } > > > > static int ks_pcie_start_link(struct dw_pcie *pci) > > -- > > 2.17.1 > > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] PCI: keystone: Link training retries initiation 2019-12-17 7:40 ` Yurii Monakov @ 2019-12-17 9:40 ` Andrew Murray 0 siblings, 0 replies; 4+ messages in thread From: Andrew Murray @ 2019-12-17 9:40 UTC (permalink / raw) To: Yurii Monakov; +Cc: linux-pci, m-karicheri2 On Tue, Dec 17, 2019 at 10:40:42AM +0300, Yurii Monakov wrote: > On Mon, 16 Dec 2019 11:08:22 +0000, Andrew Murray <andrew.murray@arm.com> wrote: > > > On Mon, Oct 07, 2019 at 11:41:59AM +0300, Yurii Monakov wrote: > > > ks_pcie_stop_link function never cleared LTSSM_EN_VAL bit so > > > link training was never triggered more than once after startup. > > > In configurations where link can be unstable during early boot, > > > for example, under low temperature, it will never be established. > > > > > > Signed-off-by: Yurii Monakov <monakov.y@gmail.com> > > > --- > > > drivers/pci/controller/dwc/pci-keystone.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/pci/controller/dwc/pci-keystone.c > > > b/drivers/pci/controller/dwc/pci-keystone.c index > > > f19de60ac991..ea8e7ebd8c4f 100644 --- > > > a/drivers/pci/controller/dwc/pci-keystone.c +++ > > > b/drivers/pci/controller/dwc/pci-keystone.c @@ -510,7 +510,7 @@ > > > static void ks_pcie_stop_link(struct dw_pcie *pci) /* Disable Link > > > training */ val = ks_pcie_app_readl(ks_pcie, CMD_STATUS); > > > val &= ~LTSSM_EN_VAL; > > > - ks_pcie_app_writel(ks_pcie, CMD_STATUS, LTSSM_EN_VAL | > > > val); > > > > Oh yeah, that doesn't look right to me. Good spot. Thanks for this! > > > > > + ks_pcie_app_writel(ks_pcie, CMD_STATUS, val); > > > > As far as I can work out, this bug existed from the beginning - can > > you please resend with this tag? > > > > Fixes: 0c4ffcfe1fbc ("PCI: keystone: Add TI Keystone PCIe driver") > I have to add this line prior to 'Signed-off-by' tag? Yes please. Please also see this [1] email for more details on the suggested order of tags for future reference (the tags are generally in chronological order. [1] https://www.spinics.net/lists/linux-pci/msg65938.html > > > Can you also update the commit subject to emphasise it's a bug fix, > > e.g. PCI: keystone: Fix ... or similar. > Do you mean that I have to create new patch from scratch and send it again? Yes please create a new patch and send it again (such that the subject starts with [PATCH v2]. Please make sure the patch will be able to apply cleanly ontop of: https://git.kernel.org/pub/scm/linux/kernel/git/lpieralisi/pci.git/log/?h=pci/dwc Also as this fix will be beneficial for older kernels it can be applied to the -stable release, you can do that by adding: CC: stable@vger.kernel.org After the Signed-off-by tag, and including that address on CC. See [2] for more details. [2] https://www.kernel.org/doc/html/v4.10/process/stable-kernel-rules.html > > Best Regards, > Yurii Monakov > > PS. I'm new to LKLM, so sorry for dumb questions. Welcome! Please - ask as many as you like. You can also learn a lot by looking at what other people do, so always check git history and other patches on the mailing lists to get an idea of the way things are done. Thanks, Andrew Murray > > > > > Thanks, > > > > Andrew Murray > > > > > } > > > > > > static int ks_pcie_start_link(struct dw_pcie *pci) > > > -- > > > 2.17.1 > > > > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-12-17 9:40 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-10-07 8:41 [PATCH] PCI: keystone: Link training retries initiation Yurii Monakov 2019-12-16 11:08 ` Andrew Murray 2019-12-17 7:40 ` Yurii Monakov 2019-12-17 9:40 ` Andrew Murray
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).