| Message ID | 20250201121420.32316-1-wahrenst@gmx.net (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Krzysztof Wilczyński |
| Headers | show |
| Series | PCI: brcmstb: Adjust message if L23 cannot be entered | expand |
On 2/1/25 04:14, Stefan Wahren wrote: > The entering of L23 lower-power state is optional, because the > connected endpoint might doesn't support it. So pcie-brcmstb shouldn't > print an error if it fails. > > Signed-off-by: Stefan Wahren <wahrenst@gmx.net> Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
On Sat, Feb 01, 2025 at 01:14:20PM +0100, Stefan Wahren wrote: > The entering of L23 lower-power state is optional, because the > connected endpoint might doesn't support it. So pcie-brcmstb shouldn't > print an error if it fails. > Which part of the PCIe spec states that the L23 Ready state is optional? - Mani > Signed-off-by: Stefan Wahren <wahrenst@gmx.net> > --- > drivers/pci/controller/pcie-brcmstb.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c > index e733a27dc8df..9e7c5349c6c2 100644 > --- a/drivers/pci/controller/pcie-brcmstb.c > +++ b/drivers/pci/controller/pcie-brcmstb.c > @@ -1399,7 +1399,10 @@ static void brcm_pcie_remove_bus(struct pci_bus *bus) > pcie->sr = NULL; > } > > -/* L23 is a low-power PCIe link state */ > +/* > + * Try to enter L23 ( low-power PCIe link state ) > + * This might fail if connected endpoint doesn't support it. > + */ > static void brcm_pcie_enter_l23(struct brcm_pcie *pcie) > { > void __iomem *base = pcie->base; > @@ -1422,7 +1425,7 @@ static void brcm_pcie_enter_l23(struct brcm_pcie *pcie) > } > > if (!l23) > - dev_err(pcie->dev, "failed to enter low-power link state\n"); > + dev_dbg(pcie->dev, "Unable to enter low-power link state\n"); > } > > static int brcm_phy_cntl(struct brcm_pcie *pcie, const int start) > -- > 2.34.1 > >
Hi Mani, Am 08.02.25 um 11:27 schrieb Manivannan Sadhasivam: > On Sat, Feb 01, 2025 at 01:14:20PM +0100, Stefan Wahren wrote: >> The entering of L23 lower-power state is optional, because the >> connected endpoint might doesn't support it. So pcie-brcmstb shouldn't >> print an error if it fails. >> > Which part of the PCIe spec states that the L23 Ready state is optional? tbh i don't have access to the PCIe spec, so my statement based on this comment [1]. Thanks [1] - https://lore.kernel.org/all/CADQZjwcjLMTLZB1tzp+PYKK9rm=Ke7a-KoGKMKb9zoWodcRK-A@mail.gmail.com/ > > - Mani > >> Signed-off-by: Stefan Wahren <wahrenst@gmx.net> >> --- >> drivers/pci/controller/pcie-brcmstb.c | 7 +++++-- >> 1 file changed, 5 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c >> index e733a27dc8df..9e7c5349c6c2 100644 >> --- a/drivers/pci/controller/pcie-brcmstb.c >> +++ b/drivers/pci/controller/pcie-brcmstb.c >> @@ -1399,7 +1399,10 @@ static void brcm_pcie_remove_bus(struct pci_bus *bus) >> pcie->sr = NULL; >> } >> >> -/* L23 is a low-power PCIe link state */ >> +/* >> + * Try to enter L23 ( low-power PCIe link state ) >> + * This might fail if connected endpoint doesn't support it. >> + */ >> static void brcm_pcie_enter_l23(struct brcm_pcie *pcie) >> { >> void __iomem *base = pcie->base; >> @@ -1422,7 +1425,7 @@ static void brcm_pcie_enter_l23(struct brcm_pcie *pcie) >> } >> >> if (!l23) >> - dev_err(pcie->dev, "failed to enter low-power link state\n"); >> + dev_dbg(pcie->dev, "Unable to enter low-power link state\n"); >> } >> >> static int brcm_phy_cntl(struct brcm_pcie *pcie, const int start) >> -- >> 2.34.1 >> >>
Hi Mani, Am 08.02.25 um 14:22 schrieb Stefan Wahren: > Hi Mani, > > Am 08.02.25 um 11:27 schrieb Manivannan Sadhasivam: >> On Sat, Feb 01, 2025 at 01:14:20PM +0100, Stefan Wahren wrote: >>> The entering of L23 lower-power state is optional, because the >>> connected endpoint might doesn't support it. So pcie-brcmstb shouldn't >>> print an error if it fails. >>> >> Which part of the PCIe spec states that the L23 Ready state is optional? > tbh i don't have access to the PCIe spec, so my statement based on > this comment [1]. Please tell, how can we proceed here? In case L23 is required by the specs the driver also need adjustment. Best regards > > Thanks > > [1] - > https://lore.kernel.org/all/CADQZjwcjLMTLZB1tzp+PYKK9rm=Ke7a-KoGKMKb9zoWodcRK-A@mail.gmail.com/ >> >> - Mani >>
On Thu, Feb 13, 2025 at 07:59:38PM +0100, Stefan Wahren wrote: > Hi Mani, > > Am 08.02.25 um 14:22 schrieb Stefan Wahren: > > Hi Mani, > > > > Am 08.02.25 um 11:27 schrieb Manivannan Sadhasivam: > > > On Sat, Feb 01, 2025 at 01:14:20PM +0100, Stefan Wahren wrote: > > > > The entering of L23 lower-power state is optional, because the > > > > connected endpoint might doesn't support it. So pcie-brcmstb shouldn't > > > > print an error if it fails. > > > > > > > Which part of the PCIe spec states that the L23 Ready state is optional? > > tbh i don't have access to the PCIe spec, so my statement based on > > this comment [1]. > Please tell, how can we proceed here? In case L23 is required by the > specs the driver also need adjustment. > I don't think the spec mentions that the L23 Ready state is optional. Atleast, I cannot find the reference. In that case, I do not see a reason to drop the error. - Mani
diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c index e733a27dc8df..9e7c5349c6c2 100644 --- a/drivers/pci/controller/pcie-brcmstb.c +++ b/drivers/pci/controller/pcie-brcmstb.c @@ -1399,7 +1399,10 @@ static void brcm_pcie_remove_bus(struct pci_bus *bus) pcie->sr = NULL; } -/* L23 is a low-power PCIe link state */ +/* + * Try to enter L23 ( low-power PCIe link state ) + * This might fail if connected endpoint doesn't support it. + */ static void brcm_pcie_enter_l23(struct brcm_pcie *pcie) { void __iomem *base = pcie->base; @@ -1422,7 +1425,7 @@ static void brcm_pcie_enter_l23(struct brcm_pcie *pcie) } if (!l23) - dev_err(pcie->dev, "failed to enter low-power link state\n"); + dev_dbg(pcie->dev, "Unable to enter low-power link state\n"); } static int brcm_phy_cntl(struct brcm_pcie *pcie, const int start)
The entering of L23 lower-power state is optional, because the connected endpoint might doesn't support it. So pcie-brcmstb shouldn't print an error if it fails. Signed-off-by: Stefan Wahren <wahrenst@gmx.net> --- drivers/pci/controller/pcie-brcmstb.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) -- 2.34.1