According to Rockchip sources, there is grave danger in enabling 5.0
GT/s speed for this core. Add a comment documenting that danger and
discouraging end-users from forcing higher speed through DT changes.

Link: https://lore.kernel.org/all/ffd05070-9879-4468-94e3-b88968b4c21b@rock-chips.com/
Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
---
 drivers/pci/controller/pcie-rockchip-host.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
index ee1822ca01db..7e6ff76466b7 100644
--- a/drivers/pci/controller/pcie-rockchip-host.c
+++ b/drivers/pci/controller/pcie-rockchip-host.c
@@ -332,6 +332,11 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip)
 		/*
 		 * Enable retrain for gen2. This should be configured only after
 		 * gen1 finished.
+		 *
+		 * According to Rockchip this path is dangerous and may lead to
+		 * catastrophic failure. Even if the odds are small, users are
+		 * still discouraged to engage the corresponding DT option.
+		 *
 		 */
 		status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL2);
 		status &= ~PCI_EXP_LNKCTL2_TLS;
-- 
2.49.0

Hello Geraldo,

On Saturday, November 15, 2025 10:10 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> According to Rockchip sources, there is grave danger in enabling 5.0
> GT/s speed for this core. Add a comment documenting that danger and
> discouraging end-users from forcing higher speed through DT changes.
> 
> Link: https://lore.kernel.org/all/ffd05070-9879-4468-94e3-b88968b4c21b@rock-chips.com/
> Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
> Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> ---
>  drivers/pci/controller/pcie-rockchip-host.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
> index ee1822ca01db..7e6ff76466b7 100644
> --- a/drivers/pci/controller/pcie-rockchip-host.c
> +++ b/drivers/pci/controller/pcie-rockchip-host.c
> @@ -332,6 +332,11 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip)
>  		/*
>  		 * Enable retrain for gen2. This should be configured only after
>  		 * gen1 finished.
> +		 *
> +		 * According to Rockchip this path is dangerous and may lead to
> +		 * catastrophic failure. Even if the odds are small, users are
> +		 * still discouraged to engage the corresponding DT option.
> +		 *
>  		 */
>  		status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL2);
>  		status &= ~PCI_EXP_LNKCTL2_TLS;

Looking good to me, thanks for this patch!  There's no need
to emit warnings here, because they'd be emitted already in
the rockchip_pcie_parse_dt() function.

Please feel free to include

Reviewed-by: Dragan Simic <dsimic@manjaro.org>

On Sat, Nov 15, 2025 at 10:30:49AM +0100, Dragan Simic wrote:
> Hello Geraldo,
> 
> On Saturday, November 15, 2025 10:10 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> > According to Rockchip sources, there is grave danger in enabling 5.0
> > GT/s speed for this core. Add a comment documenting that danger and
> > discouraging end-users from forcing higher speed through DT changes.
> > 
> > Link: https://lore.kernel.org/all/ffd05070-9879-4468-94e3-b88968b4c21b@rock-chips.com/
> > Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
> > Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> > ---
> >  drivers/pci/controller/pcie-rockchip-host.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
> > index ee1822ca01db..7e6ff76466b7 100644
> > --- a/drivers/pci/controller/pcie-rockchip-host.c
> > +++ b/drivers/pci/controller/pcie-rockchip-host.c
> > @@ -332,6 +332,11 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip)
> >  		/*
> >  		 * Enable retrain for gen2. This should be configured only after
> >  		 * gen1 finished.
> > +		 *
> > +		 * According to Rockchip this path is dangerous and may lead to
> > +		 * catastrophic failure. Even if the odds are small, users are
> > +		 * still discouraged to engage the corresponding DT option.
> > +		 *
> >  		 */
> >  		status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL2);
> >  		status &= ~PCI_EXP_LNKCTL2_TLS;
> 
> Looking good to me, thanks for this patch!  There's no need
> to emit warnings here, because they'd be emitted already in
> the rockchip_pcie_parse_dt() function.
> 
> Please feel free to include
> 
> Reviewed-by: Dragan Simic <dsimic@manjaro.org>
>

I disagree, I think the comment stands.

Even if we reduce to one line, ex:

+ May cause damage

Regards,
Geraldo Nascimento

On Saturday, November 15, 2025 10:51 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> On Sat, Nov 15, 2025 at 10:30:49AM +0100, Dragan Simic wrote:
> > On Saturday, November 15, 2025 10:10 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> > > According to Rockchip sources, there is grave danger in enabling 5.0
> > > GT/s speed for this core. Add a comment documenting that danger and
> > > discouraging end-users from forcing higher speed through DT changes.
> > > 
> > > Link: https://lore.kernel.org/all/ffd05070-9879-4468-94e3-b88968b4c21b@rock-chips.com/
> > > Reported-by: Shawn Lin <shawn.lin@rock-chips.com>
> > > Signed-off-by: Geraldo Nascimento <geraldogabriel@gmail.com>
> > > ---
> > >  drivers/pci/controller/pcie-rockchip-host.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > > 
> > > diff --git a/drivers/pci/controller/pcie-rockchip-host.c b/drivers/pci/controller/pcie-rockchip-host.c
> > > index ee1822ca01db..7e6ff76466b7 100644
> > > --- a/drivers/pci/controller/pcie-rockchip-host.c
> > > +++ b/drivers/pci/controller/pcie-rockchip-host.c
> > > @@ -332,6 +332,11 @@ static int rockchip_pcie_host_init_port(struct rockchip_pcie *rockchip)
> > >  		/*
> > >  		 * Enable retrain for gen2. This should be configured only after
> > >  		 * gen1 finished.
> > > +		 *
> > > +		 * According to Rockchip this path is dangerous and may lead to
> > > +		 * catastrophic failure. Even if the odds are small, users are
> > > +		 * still discouraged to engage the corresponding DT option.
> > > +		 *
> > >  		 */
> > >  		status = rockchip_pcie_read(rockchip, PCIE_RC_CONFIG_CR + PCI_EXP_LNKCTL2);
> > >  		status &= ~PCI_EXP_LNKCTL2_TLS;
> > 
> > Looking good to me, thanks for this patch!  There's no need
> > to emit warnings here, because they'd be emitted already in
> > the rockchip_pcie_parse_dt() function.
> > 
> > Please feel free to include
> > 
> > Reviewed-by: Dragan Simic <dsimic@manjaro.org>
> >
> 
> I disagree, I think the comment stands.
> 
> Even if we reduce to one line, ex:
> 
> + May cause damage

Ah, perhaps I wasn't clear enough, so let me clarify a bit.  The
comment you added is fine, I just referred to no need for emitting
a warning at that point, because it would be emitted already.

On Sat, Nov 15, 2025 at 11:01:21AM +0100, Dragan Simic wrote:
> On Saturday, November 15, 2025 10:51 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> > On Sat, Nov 15, 2025 at 10:30:49AM +0100, Dragan Simic wrote:
> > > Looking good to me, thanks for this patch!  There's no need
> > > to emit warnings here, because they'd be emitted already in
> > > the rockchip_pcie_parse_dt() function.
> > > 
> > > Please feel free to include
> > > 
> > > Reviewed-by: Dragan Simic <dsimic@manjaro.org>
> > >
> > 
> > I disagree, I think the comment stands.
> > 
> > Even if we reduce to one line, ex:
> > 
> > + May cause damage
> 
> Ah, perhaps I wasn't clear enough, so let me clarify a bit.  The
> comment you added is fine, I just referred to no need for emitting
> a warning at that point, because it would be emitted already.

OK, I get it now so I think it's time to send v2 with all that in mind
:)

Thanks,
Geraldo Nascimento

On Saturday, November 15, 2025 11:04 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> On Sat, Nov 15, 2025 at 11:01:21AM +0100, Dragan Simic wrote:
> > On Saturday, November 15, 2025 10:51 CET, Geraldo Nascimento <geraldogabriel@gmail.com> wrote:
> > > On Sat, Nov 15, 2025 at 10:30:49AM +0100, Dragan Simic wrote:
> > > > Looking good to me, thanks for this patch!  There's no need
> > > > to emit warnings here, because they'd be emitted already in
> > > > the rockchip_pcie_parse_dt() function.
> > > > 
> > > > Please feel free to include
> > > > 
> > > > Reviewed-by: Dragan Simic <dsimic@manjaro.org>
> > > >
> > > 
> > > I disagree, I think the comment stands.
> > > 
> > > Even if we reduce to one line, ex:
> > > 
> > > + May cause damage
> > 
> > Ah, perhaps I wasn't clear enough, so let me clarify a bit.  The
> > comment you added is fine, I just referred to no need for emitting
> > a warning at that point, because it would be emitted already.
> 
> OK, I get it now so I think it's time to send v2 with all that in mind
> :)

Please, wait a day or two before sending the v2, because that's
standard procedure.  Sending more than one version in the same day
is highly discouraged, because it doesn't give enough time to people
for reviewing, and may cause people to look at a wrong version.

On Sat, Nov 15, 2025 at 11:09:42AM +0100, Dragan Simic wrote:
> Please, wait a day or two before sending the v2, because that's
> standard procedure.  Sending more than one version in the same day
> is highly discouraged, because it doesn't give enough time to people
> for reviewing, and may cause people to look at a wrong version.

Yeah, you're right, sorry.