On Mon, Aug 24, 2020 at 03:30:18PM -0400, Jim Quinlan wrote:
> From: Jim Quinlan <jquinlan@xxxxxxxxxxxx>
>
> The PERST# bit was moved to a different register in 7278-type STB chips.
> In addition, the polarity of the bit was also changed; for other chips
> writing a 1 specified assert; for 7278-type chips, writing a 0 specifies
> assert.
>
> Of course, PERST# is a PCIe asserted-low signal.
>
> Signed-off-by: Jim Quinlan <jquinlan@xxxxxxxxxxxx>
> Acked-by: Florian Fainelli <f.fainelli@xxxxxxxxx>
> ---
> drivers/pci/controller/pcie-brcmstb.c | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> index 3d588ab7a6dd..acf2239b0251 100644
> --- a/drivers/pci/controller/pcie-brcmstb.c
> +++ b/drivers/pci/controller/pcie-brcmstb.c
> @@ -83,6 +83,7 @@
>
> #define PCIE_MISC_PCIE_CTRL 0x4064
> #define PCIE_MISC_PCIE_CTRL_PCIE_L23_REQUEST_MASK 0x1
> +#define PCIE_MISC_PCIE_CTRL_PCIE_PERSTB_MASK 0x4
>
> #define PCIE_MISC_PCIE_STATUS 0x4068
> #define PCIE_MISC_PCIE_STATUS_PCIE_PORT_MASK 0x80
> @@ -684,9 +685,16 @@ static inline void brcm_pcie_perst_set(struct brcm_pcie *pcie, u32 val)
> {
> u32 tmp;
>
> - tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1(pcie));
> - u32p_replace_bits(&tmp, val, PCIE_RGR1_SW_INIT_1_PERST_MASK);
> - writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1(pcie));
> + if (pcie->type == BCM7278) {
> + /* Perst bit has moved and assert value is 0 */
> + tmp = readl(pcie->base + PCIE_MISC_PCIE_CTRL);
> + u32p_replace_bits(&tmp, !val, PCIE_MISC_PCIE_CTRL_PCIE_PERSTB_MASK);
> + writel(tmp, pcie->base + PCIE_MISC_PCIE_CTRL);
> + } else {
> + tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1(pcie));
> + u32p_replace_bits(&tmp, val, PCIE_RGR1_SW_INIT_1_PERST_MASK);
> + writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1(pcie));
Humm, now we have a mixture of a code path based on the chip and
variables to abstract the register details. Just do a function per chip.
I have some notion to abstract out the PERST# handling from the host
bridges. We have several cases of GPIO based handling and random
assertion times. So having an ops function here will move in that
direction.
> + }
> }
>
> static inline int brcm_pcie_get_rc_bar2_size_and_offset(struct brcm_pcie *pcie,
> @@ -771,7 +779,10 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
>
> /* Reset the bridge */
> brcm_pcie_bridge_sw_init_set(pcie, 1);
> - brcm_pcie_perst_set(pcie, 1);
If these 2 functions are always called together, then you just need 1
per chip function.
> +
> + /* BCM7278 fails when PERST# is set here */
> + if (pcie->type != BCM7278)
> + brcm_pcie_perst_set(pcie, 1);
>
> usleep_range(100, 200);
>
> --
> 2.17.1
>
