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

[Jim Quinlan]

On Thu, Sep 17, 2026 at 1:05 PM Florian Fainelli
<florian.fainelli@xxxxxxxxxxxx> wrote:
>
> 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@xxxxxxxxxxxx>
> > ---
> >   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,
The new code is simpler and does not do any "if" conditions like
"pcie->cfg->soc_base == BCM2711".  This is the commit series' major goal.

Additionally, the existing comment lacks sufficient information for someone
to change a specific setting. Of the nine comment lines in the commit, only four
lines are duplicates. Also keep in mind that I plan to add several new SoCs
in future commits which may require yet more interpretations of this field,
requiring more "else if" clauses.

> also there is inconsistent decimal versus hexadecimal.
Ack, will fix along with s/Resv/Rsvd/ for consistency.

Jim Quinlan
Broadcom CM/STB
> --
> Florian

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature