Re: [PATCH v6 3/9] PCI: rzg3s-host: Drop nop instructions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] pci: rzg3s: dropped LNKCTL2 write leaves TLS out of sync and breaks retraining
--

commit 82580733205489082c60a7965e3e79b44492b4e4
Author: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>

PCI: rzg3s-host: Drop nop instructions

On the RZ/G3S PCIe IP variant, access to the PCIe Type 1 registers requires
setting the PCI_PERM.CFG_HWINIT_EN bit. Since this bit is not set on the
code paths where rzg3s_pcie_set_max_link_speed() is called, the writes to
PCI_EXP_LNKCTL2.TLS are nops and are dropped by this commit.

> diff --git a/drivers/pci/controller/pcie-rzg3s-host.c b/drivers/pci/controller/pcie-rzg3s-host.c
> index 0ef49bb5ab1aa..362aa923978f7 100644
> --- a/drivers/pci/controller/pcie-rzg3s-host.c
> +++ b/drivers/pci/controller/pcie-rzg3s-host.c
> @@ -1144,11 +1144,6 @@ static int rzg3s_pcie_set_max_link_speed(struct rzg3s_pcie_host *host)
>  	    remote_supported_link_speeds != max_supported_link_speeds)
>  		return 0;
>  
> -	/* Set target Link speed */
> -	rzg3s_pcie_update_bits(host->pcie, pcie_cap + PCI_EXP_LNKCTL2,
> -			       PCI_EXP_LNKCTL2_TLS,
> -			       FIELD_PREP(PCI_EXP_LNKCTL2_TLS, link_speed));
> -

[Severity: Medium]
Does dropping this LNKCTL2 update leave the standard capability register out
of sync with the hardware link state configured via RZG3S_PCI_PCCTRL2?

If LNKCTL2 is not updated, lspci and generic bandwidth control logic might
read incorrect values.

Furthermore, if LNKCTL2 and potentially other standard capability registers
are locked behind CFG_HWINIT_EN, does this mean that standard PCI core
functions (like pcie_set_target_speed() or ASPM configuration) will silently
fail when the PCI core attempts to write to the Root Port's LNKCTL2 via
pci_generic_config_write?

drivers/pci/controller/pcie-rzg3s-host.c:
static struct pci_ops rzg3s_pcie_root_ops = {
	.read		= pci_generic_config_read,
	.write		= pci_generic_config_write,
	.map_bus	= rzg3s_pcie_root_map_bus,
};

Should the driver temporarily assert CFG_HWINIT_EN to perform this update and
provide a custom config write wrapper instead of dropping the write entirely?

>  	/* Request link speed change */
>  	rzg3s_pcie_update_bits(host->axi, RZG3S_PCI_PCCTRL2,
>  			       RZG3S_PCI_PCCTRL2_LS_CHG_REQ |

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007053645.1391545-1-claudiu.beznea@kernel.org?part=3