Re: [PATCH 11/13] PCI: brcmstb: Put max_burst_size setting in cfg_data

[Florian Fainelli <florian.fainelli@broadcom.com>]

On 9/11/26 16:35, Jim Quinlan wrote:
> Rather than ascertaining the max_burst_size setting by comparing different
> SoC IDs, specify the setting in the config_data structure for each SoC.
> Also add a comment on each SoC's encoding of this field.
> 
> Note: Previously, the max burst setting used for the 4908 Soc was 2.
> However, it appears that the setting of 2 is illegal for this chip.  Set it
> to 0 as (a) this is the safest (smallest) choice and (b) the 2 was probably
> interpreted by the HW as a 0 anyway.
> 
> Signed-off-by: Jim Quinlan <james.quinlan@broadcom.com>
> ---
>   drivers/pci/controller/pcie-brcmstb.c | 29 ++++++++++++---------------
>   1 file changed, 13 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 047783e33ae4..8bca98e6f5fa 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -317,6 +317,7 @@ struct pcie_cfg_data {
>   	u32 flags;
>   	u32 quirks;
>   	u8 num_inbound_wins;
> +	u8 burst_setting;
>   	int (*perst_set)(struct brcm_pcie *pcie, u32 val);
>   	int (*bridge_sw_init_set)(struct brcm_pcie *pcie, u32 val);
>   	int (*post_setup)(struct brcm_pcie *pcie);
> @@ -1153,7 +1154,7 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
>   	void __iomem *base = pcie->base;
>   	struct pci_host_bridge *bridge;
>   	struct resource_entry *entry;
> -	u32 tmp, burst, num_lanes, num_lanes_cap;
> +	u32 tmp, num_lanes, num_lanes_cap;
>   	u8 num_out_wins = 0;
>   	int num_inbound_wins = 0;
>   	int memc, ret;
> @@ -1188,20 +1189,6 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
>   	/* Wait for SerDes to be stable */
>   	usleep_range(100, 200);
>   
> -	/*
> -	 * SCB_MAX_BURST_SIZE is a two bit field.  For GENERIC chips it
> -	 * is encoded as 0=128, 1=256, 2=512, 3=Rsvd, for BCM7278 it
> -	 * is encoded as 0=Rsvd, 1=128, 2=256, 3=512.
> -	 */
> -	if (BFLAG(pcie, IS_BMIPS))
> -		burst = 0x1; /* 256 bytes */
> -	else if (pcie->cfg->soc_base == BCM2711)
> -		burst = 0x0; /* 128 bytes */
> -	else if (pcie->cfg->soc_base == BCM7278)
> -		burst = 0x3; /* 512 bytes */
> -	else
> -		burst = 0x2; /* 512 bytes */
> -
>   	/*
>   	 * Set SCB_MAX_BURST_SIZE, CFG_READ_UR_MODE, SCB_ACCESS_EN,
>   	 * RCB_MPS_MODE, RCB_64B_MODE
> @@ -1209,7 +1196,8 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
>   	tmp = readl(base + PCIE_MISC_MISC_CTRL);
>   	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_SCB_ACCESS_EN_MASK);
>   	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_CFG_READ_UR_MODE_MASK);
> -	u32p_replace_bits(&tmp, burst, PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK);
> +	u32p_replace_bits(&tmp, pcie->cfg->burst_setting,
> +			  PCIE_MISC_MISC_CTRL_MAX_BURST_SIZE_MASK);
>   	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_PCIE_RCB_MPS_MODE_MASK);
>   	u32p_replace_bits(&tmp, 1, PCIE_MISC_MISC_CTRL_PCIE_RCB_64B_MODE_MASK);
>   	writel(tmp, base + PCIE_MISC_MISC_CTRL);
> @@ -1961,6 +1949,7 @@ static const struct pcie_cfg_data generic_cfg = {
>   	.perst_set	= brcm_pcie_perst_set_generic,
>   	.bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_generic,
>   	.num_inbound_wins = 3,
> +	.burst_setting	= 0x2, /* 0=128B, 1=256B, 2=512B, 3=Rsvd */

Not sure this one is much of an improvement, especially if we end up 
repeating the comment, also there is inconsistent decimal versus 
hexadecimal.
-- 
Florian