Re: [PATCH] PCI: dwc: endpoint: Fix advertized resizable BAR size

[Bjorn Helgaas <helgaas@kernel.org>]

On Thu, Feb 29, 2024 at 02:13:13PM +0100, Niklas Cassel wrote:
> The commit message in commit fc9a77040b04 ("PCI: designware-ep: Configure
> Resizable BAR cap to advertise the smallest size") claims that it modifies
> the Resizable BAR capability to only advertize support for 1 MB size BARs.
> 
> However, the commit writes all zeroes to PCI_REBAR_CAP (the register which
> contains the possible BAR sizes that a BAR be resized to).
> 
> According to the spec, it is illegal to not have a bit set in
> PCI_REBAR_CAP, and 1 MB is the smallest size allowed.
> 
> Set bit 4 in PCI_REBAR_CAP, so that we actually advertize support for a
> 1 MB BAR size.

s/advertize/advertise/ in subject, commit log, comment

I assume this probably fixes some user-visible problem?  If so, would
be nice to include sample symptom, e.g., dmesg log, bug report, error
message, etc.

> Fixes: fc9a77040b04 ("PCI: designware-ep: Configure Resizable BAR cap to advertise the smallest size")
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
> I'm working on a series that adds proper Resizable BAR support, but it is
> taking longer than expected, so I'm sending this fix first.
> 
>  drivers/pci/controller/dwc/pcie-designware-ep.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 5befed2dc02b..bb759a7b5fc7 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -627,8 +627,13 @@ int dw_pcie_ep_init_complete(struct dw_pcie_ep *ep)
>  		nbars = (reg & PCI_REBAR_CTRL_NBAR_MASK) >>
>  			PCI_REBAR_CTRL_NBAR_SHIFT;
>  
> +		/*
> +		 * The PCI Express Base Specification require us to support at
> +		 * least one size in the range from 1 MB to 512 GB. Advertize
> +		 * support for 1 MB BAR size only.

Please include the spec revision and section.  I think "PCIe r6.0, sec
7.8.6.2" would be sufficient.

> +		 */
>  		for (i = 0; i < nbars; i++, offset += PCI_REBAR_CTRL)
> -			dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, 0x0);
> +			dw_pcie_writel_dbi(pci, offset + PCI_REBAR_CAP, BIT(4));
>  	}
>  
>  	/*
> -- 
> 2.44.0
>