Re: [PATCH v2 1/4] PCI: rockchip: limit RK3399 to 2.5 GT/s to prevent damage

Geraldo Nascimento
2025-12-24T02:24:37Z

Hello Mani, Dragan, Diederik, and all,

I'm currently facing banking issues in Brazil which means I'm
refraining from fetching my email through POP3 with mutt to do proper
kernel development.
I usually fetch my mail through POP3 and have the server delete it, to
save space on my free tier.
But I found out I have a 20-year old bank account open that I had
forgot about, and with this bureaucracy hell I can't have the server
delete the messages for now.
I'll send v3 with more proper and illustrative wording once I get this done.

Thanks,
Geraldo Nascimento

On Thu, Dec 18, 2025 at 7:13 AM Dragan Simic <dsimic@manjaro.org> wrote:
>
> Hello Diederik,
>
> On Thursday, December 18, 2025 11:01 CET, "Diederik de Haas" <diederik@cknow-tech.com> wrote:
> > On Thu Dec 18, 2025 at 10:47 AM CET, Dragan Simic wrote:
> > > On Thursday, December 18, 2025 09:05 CET, Manivannan Sadhasivam <mani@kernel.org> wrote:
> > >> On Mon, Nov 17, 2025 at 06:47:05PM -0300, Geraldo Nascimento wrote:
> > >> > Shawn Lin from Rockchip has reiterated that there may be danger in using
> > >> > their PCIe with 5.0 GT/s speeds. Warn the user if they make a DT change
> > >> > from the default and drive at 2.5 GT/s only, even if the DT
> > >> > max-link-speed property is invalid or inexistent.
> > >> >
> > >> > This change is corroborated by RK3399 official datasheet [1], which
> > >> > says maximum link speed for this platform is 2.5 GT/s.
> > >> >
> > >> > [1] https://opensource.rock-chips.com/images/d/d7/Rockchip_RK3399_Datasheet_V2.1-20200323.pdf
> > >> >
> > >> > Fixes: 956cd99b35a8 ("PCI: rockchip: Separate common code from RC driver")
> > >> > Link: https://lore.kernel.org/all/ffd05070-9879-4468-94e3-b88968b4c21b@rock-chips.com/
> > >> > Cc: stable@vger.kernel.org
> > >> > Reported-by: Dragan Simic <dsimic@manjaro.org>
> > >> > Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
> > >> > Reviewed-by: Dragan Simic <dsimic@manjaro.org>
> > >> > Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> > >> > ---
> > >> > drivers/pci/controller/pcie-rockchip.c | 10 ++++++++--
> > >> > 1 file changed, 8 insertions(+), 2 deletions(-)
> > >> >
> > >> > diff --git a/drivers/pci/controller/pcie-rockchip.c b/drivers/pci/controller/pcie-rockchip.c
> > >> > index 0f88da378805..992ccf4b139e 100644
> > >> > --- a/drivers/pci/controller/pcie-rockchip.c
> > >> > +++ b/drivers/pci/controller/pcie-rockchip.c
> > >> > @@ -66,8 +66,14 @@ int rockchip_pcie_parse_dt(struct rockchip_pcie *rockchip)
> > >> > }
> > >> >
> > >> > rockchip->link_gen = of_pci_get_max_link_speed(node);
> > >> > - if (rockchip->link_gen < 0 || rockchip->link_gen > 2)
> > >> > - rockchip->link_gen = 2;
> > >> > + if (rockchip->link_gen < 0 || rockchip->link_gen > 2) {
> > >> > + rockchip->link_gen = 1;
> > >> > + dev_warn(dev, "invalid max-link-speed, set to 2.5 GT/s\n");
> > >> > + }
> > >> > + else if (rockchip->link_gen == 2) {
> > >> > + rockchip->link_gen = 1;
> > >> > + dev_warn(dev, "5.0 GT/s is dangerous, set to 2.5 GT/s\n");
> > >>
> > >> What does 'danger' really mean here? Link instability or something else?
> > >> Error messages should be precise and not fearmongering.
> > >
> > > I agree that the original wording is a bit suboptimal, and I'd suggest
> > > to Geraldo that the produced warning message is changed to
> > >
> > > "5.0 GT/s may cause data corruption, limited to to 2.5 GT/s\n"
> > >
> > > or something similar, to better reflect the actual underlying issue.
> >
> > s/limited to to/therefore limit speed to/ ?
>
> That would work well in a book or an article, while slightly terse
> wording is usually preferred in the messages produced by the kernel,
> or in log messages in general. Such an approach compacts as much
> information as possible in as few words as possible, while still
> remaining (mostly) grammatically correct.