Re: [PATCH 2/6] pci: pcie_dw_meson: Use dw_pcie_link_set_max_link_width()

On 17/06/2025 10:16, Marek Vasut wrote:
> Use dw_pcie_link_set_max_link_width() instead of local implementation
> of the same functionality. This does change the behavior slightly, as
> the dw_pcie_link_set_max_link_width() implementation also programs the
> LNKCAP register MLW, this should however be correct and is now aligned
> with Linux kernel behavior.
> 
> Signed-off-by: Marek Vasut <[email protected]>
> ---
> Cc: Casey Connolly <[email protected]>
> Cc: Christian Marangi <[email protected]>
> Cc: Daniel Schwierzeck <[email protected]>
> Cc: Jiaxun Yang <[email protected]>
> Cc: John Crispin <[email protected]>
> Cc: Kever Yang <[email protected]>
> Cc: Neil Armstrong <[email protected]>
> Cc: Nobuhiro Iwamatsu <[email protected]>
> Cc: Philipp Tomsich <[email protected]>
> Cc: Siddharth Vadapalli <[email protected]>
> Cc: Simon Glass <[email protected]>
> Cc: Sumit Garg <[email protected]>
> Cc: Tom Rini <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> ---
>   drivers/pci/pcie_dw_meson.c | 6 +-----
>   1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/pcie_dw_meson.c b/drivers/pci/pcie_dw_meson.c
> index bb78e7874b1..483b07ce078 100644
> --- a/drivers/pci/pcie_dw_meson.c
> +++ b/drivers/pci/pcie_dw_meson.c
> @@ -115,13 +115,9 @@ static void meson_pcie_configure(struct meson_pcie *priv)
>   	val &= ~PORT_LINK_FAST_LINK_MODE;
>   	val |= PORT_LINK_DLL_LINK_EN;
>   	val &= ~PORT_LINK_MODE_MASK;
> -	val |= PORT_LINK_MODE_1_LANES;
>   	writel(val, priv->dw.dbi_base + PCIE_PORT_LINK_CONTROL);
>   
> -	val = readl(priv->dw.dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL);
> -	val &= ~PORT_LOGIC_LINK_WIDTH_MASK;
> -	val |= PORT_LOGIC_LINK_WIDTH_1_LANES;
> -	writel(val, priv->dw.dbi_base + PCIE_LINK_WIDTH_SPEED_CONTROL);
> +	dw_pcie_link_set_max_link_width(&priv->dw, 1);

This also changes PCI_EXP_LNKCAP but I guess it's also done like this
on Linux when calling dw_pcie_host_init.

I'll need to test it, but AFAIK it looks fine:
Reviewed-by: Neil Armstrong <[email protected]>

>   
>   	dw_pcie_dbi_write_enable(&priv->dw, false);
>   }