| Message ID | 20200519203419.12369-5-james.quinlan@broadcom.com |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: brcmstb: enable PCIe for STB chips | expand |
Hi Jim, On Tue, 2020-05-19 at 16:34 -0400, Jim Quinlan wrote: > From: Jim Quinlan <jquinlan@broadcom.com> > > Add in compatibility strings and code for three Broadcom STB chips. > Some of the register locations, shifts, and masks are different > for certain chips, requiring the use of different constants based > on of_id. > > We would like to add the following at this time to the match list > but we need to wait until the end of this patchset so that > everything works. > > { .compatible = "brcm,bcm7211-pcie", .data = &generic_cfg }, > { .compatible = "brcm,bcm7278-pcie", .data = &bcm7278_cfg }, > { .compatible = "brcm,bcm7216-pcie", .data = &bcm7278_cfg }, > { .compatible = "brcm,bcm7445-pcie", .data = &generic_cfg }, > > Signed-off-by: Jim Quinlan <jquinlan@broadcom.com> > --- > drivers/pci/controller/pcie-brcmstb.c | 103 +++++++++++++++++++++++--- > 1 file changed, 91 insertions(+), 12 deletions(-) > > diff --git a/drivers/pci/controller/pcie-brcmstb.c > b/drivers/pci/controller/pcie-brcmstb.c > index 73020b4ff090..c1cf4ea7d3d9 100644 > --- a/drivers/pci/controller/pcie-brcmstb.c > +++ b/drivers/pci/controller/pcie-brcmstb.c > @@ -120,9 +120,8 @@ > #define PCIE_EXT_SLOT_SHIFT 15 > #define PCIE_EXT_FUNC_SHIFT 12 > > -#define PCIE_RGR1_SW_INIT_1 0x9210 > #define PCIE_RGR1_SW_INIT_1_PERST_MASK 0x1 > -#define PCIE_RGR1_SW_INIT_1_INIT_MASK 0x2 > +#define PCIE_RGR1_SW_INIT_1_PERST_SHIFT 0x0 > > /* PCIe parameters */ > #define BRCM_NUM_PCIE_OUT_WINS 0x4 > @@ -152,6 +151,69 @@ > #define SSC_STATUS_SSC_MASK 0x400 > #define SSC_STATUS_PLL_LOCK_MASK 0x800 > > +#define IDX_ADDR(pcie) \ > + (pcie->reg_offsets[EXT_CFG_INDEX]) > +#define DATA_ADDR(pcie) \ > + (pcie->reg_offsets[EXT_CFG_DATA]) > +#define PCIE_RGR1_SW_INIT_1(pcie) \ > + (pcie->reg_offsets[RGR1_SW_INIT_1]) > + > +enum { > + RGR1_SW_INIT_1, > + EXT_CFG_INDEX, > + EXT_CFG_DATA, > +}; > + > +enum { > + RGR1_SW_INIT_1_INIT_MASK, > + RGR1_SW_INIT_1_INIT_SHIFT, > +}; > + > +enum pcie_type { > + GENERIC, > + BCM7278, > +}; > + > +struct pcie_cfg_data { > + const int *reg_field_info; > + const int *offsets; > + const enum pcie_type type; > +}; > + > +static const int pcie_reg_field_info[] = { > + [RGR1_SW_INIT_1_INIT_MASK] = 0x2, > + [RGR1_SW_INIT_1_INIT_SHIFT] = 0x1, > +}; > + > +static const int pcie_reg_field_info_bcm7278[] = { > + [RGR1_SW_INIT_1_INIT_MASK] = 0x1, > + [RGR1_SW_INIT_1_INIT_SHIFT] = 0x0, > +}; > + > +static const int pcie_offsets[] = { > + [RGR1_SW_INIT_1] = 0x9210, > + [EXT_CFG_INDEX] = 0x9000, > + [EXT_CFG_DATA] = 0x9004, > +}; > + > +static const struct pcie_cfg_data generic_cfg = { > + .reg_field_info = pcie_reg_field_info, > + .offsets = pcie_offsets, > + .type = GENERIC, > +}; > + > +static const int pcie_offset_bcm7278[] = { > + [RGR1_SW_INIT_1] = 0xc010, > + [EXT_CFG_INDEX] = 0x9000, > + [EXT_CFG_DATA] = 0x9004, > +}; > + > +static const struct pcie_cfg_data bcm7278_cfg = { > + .reg_field_info = pcie_reg_field_info_bcm7278, > + .offsets = pcie_offset_bcm7278, > + .type = BCM7278, > +}; It's not essential, but if v2 is due I'd suggest factoring out the bcm2728 specific structures above, and moving them to patch #15. This will keep a clearer division between the patch introducing the infrastructure and the one adding the support for a new device. > + > struct brcm_msi { > struct device *dev; > void __iomem *base; > @@ -176,6 +238,9 @@ struct brcm_pcie { > int gen; > u64 msi_target_addr; > struct brcm_msi *msi; > + const int *reg_offsets; > + const int *reg_field_info; > + enum pcie_type type; > }; > > /* > @@ -602,20 +667,21 @@ static struct pci_ops brcm_pcie_ops = { > > static inline void brcm_pcie_bridge_sw_init_set(struct brcm_pcie *pcie, u32 > val) > { > - u32 tmp; > + u32 tmp, mask = pcie->reg_field_info[RGR1_SW_INIT_1_INIT_MASK]; > + u32 shift = pcie->reg_field_info[RGR1_SW_INIT_1_INIT_SHIFT]; I don't think you need shift here, IIUC u32p_replace_bits() will take care of all the masking and shifting internally, moreover, you'd be able to drop the shift entry from reg_field_info. > - tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1); > - u32p_replace_bits(&tmp, val, PCIE_RGR1_SW_INIT_1_INIT_MASK); > - writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1); > + tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1(pcie)); > + tmp = (tmp & ~mask) | ((val << shift) & mask); > + writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1(pcie)); > } Regards, Nicolas
On Wed, May 20, 2020 at 7:51 AM Nicolas Saenz Julienne <nsaenzjulienne@suse.de> wrote: > > Hi Jim, > > On Tue, 2020-05-19 at 16:34 -0400, Jim Quinlan wrote: > > From: Jim Quinlan <jquinlan@broadcom.com> > > > > Add in compatibility strings and code for three Broadcom STB chips. > > Some of the register locations, shifts, and masks are different > > for certain chips, requiring the use of different constants based > > on of_id. > > > > We would like to add the following at this time to the match list > > but we need to wait until the end of this patchset so that > > everything works. > > > > { .compatible = "brcm,bcm7211-pcie", .data = &generic_cfg }, > > { .compatible = "brcm,bcm7278-pcie", .data = &bcm7278_cfg }, > > { .compatible = "brcm,bcm7216-pcie", .data = &bcm7278_cfg }, > > { .compatible = "brcm,bcm7445-pcie", .data = &generic_cfg }, > > > > Signed-off-by: Jim Quinlan <jquinlan@broadcom.com> > > --- > > drivers/pci/controller/pcie-brcmstb.c | 103 +++++++++++++++++++++++--- > > 1 file changed, 91 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/pci/controller/pcie-brcmstb.c > > b/drivers/pci/controller/pcie-brcmstb.c > > index 73020b4ff090..c1cf4ea7d3d9 100644 > > --- a/drivers/pci/controller/pcie-brcmstb.c > > +++ b/drivers/pci/controller/pcie-brcmstb.c > > @@ -120,9 +120,8 @@ > > #define PCIE_EXT_SLOT_SHIFT 15 > > #define PCIE_EXT_FUNC_SHIFT 12 > > > > -#define PCIE_RGR1_SW_INIT_1 0x9210 > > #define PCIE_RGR1_SW_INIT_1_PERST_MASK 0x1 > > -#define PCIE_RGR1_SW_INIT_1_INIT_MASK 0x2 > > +#define PCIE_RGR1_SW_INIT_1_PERST_SHIFT 0x0 > > > > /* PCIe parameters */ > > #define BRCM_NUM_PCIE_OUT_WINS 0x4 > > @@ -152,6 +151,69 @@ > > #define SSC_STATUS_SSC_MASK 0x400 > > #define SSC_STATUS_PLL_LOCK_MASK 0x800 > > > > +#define IDX_ADDR(pcie) \ > > + (pcie->reg_offsets[EXT_CFG_INDEX]) > > +#define DATA_ADDR(pcie) \ > > + (pcie->reg_offsets[EXT_CFG_DATA]) > > +#define PCIE_RGR1_SW_INIT_1(pcie) \ > > + (pcie->reg_offsets[RGR1_SW_INIT_1]) > > + > > +enum { > > + RGR1_SW_INIT_1, > > + EXT_CFG_INDEX, > > + EXT_CFG_DATA, > > +}; > > + > > +enum { > > + RGR1_SW_INIT_1_INIT_MASK, > > + RGR1_SW_INIT_1_INIT_SHIFT, > > +}; > > + > > +enum pcie_type { > > + GENERIC, > > + BCM7278, > > +}; > > + > > +struct pcie_cfg_data { > > + const int *reg_field_info; > > + const int *offsets; > > + const enum pcie_type type; > > +}; > > + > > +static const int pcie_reg_field_info[] = { > > + [RGR1_SW_INIT_1_INIT_MASK] = 0x2, > > + [RGR1_SW_INIT_1_INIT_SHIFT] = 0x1, > > +}; > > + > > +static const int pcie_reg_field_info_bcm7278[] = { > > + [RGR1_SW_INIT_1_INIT_MASK] = 0x1, > > + [RGR1_SW_INIT_1_INIT_SHIFT] = 0x0, > > +}; > > + > > +static const int pcie_offsets[] = { > > + [RGR1_SW_INIT_1] = 0x9210, > > + [EXT_CFG_INDEX] = 0x9000, > > + [EXT_CFG_DATA] = 0x9004, > > +}; > > + > > +static const struct pcie_cfg_data generic_cfg = { > > + .reg_field_info = pcie_reg_field_info, > > + .offsets = pcie_offsets, > > + .type = GENERIC, > > +}; > > + > > +static const int pcie_offset_bcm7278[] = { > > + [RGR1_SW_INIT_1] = 0xc010, > > + [EXT_CFG_INDEX] = 0x9000, > > + [EXT_CFG_DATA] = 0x9004, > > +}; > > + > > +static const struct pcie_cfg_data bcm7278_cfg = { > > + .reg_field_info = pcie_reg_field_info_bcm7278, > > + .offsets = pcie_offset_bcm7278, > > + .type = BCM7278, > > +}; > > It's not essential, but if v2 is due I'd suggest factoring out the bcm2728 > specific structures above, and moving them to patch #15. This will keep a > clearer division between the patch introducing the infrastructure and the one > adding the support for a new device. The problem is that one of the commits needs the 7278 type so it has to be declared earlier. > > > + > > struct brcm_msi { > > struct device *dev; > > void __iomem *base; > > @@ -176,6 +238,9 @@ struct brcm_pcie { > > int gen; > > u64 msi_target_addr; > > struct brcm_msi *msi; > > + const int *reg_offsets; > > + const int *reg_field_info; > > + enum pcie_type type; > > }; > > > > /* > > @@ -602,20 +667,21 @@ static struct pci_ops brcm_pcie_ops = { > > > > static inline void brcm_pcie_bridge_sw_init_set(struct brcm_pcie *pcie, u32 > > val) > > { > > - u32 tmp; > > + u32 tmp, mask = pcie->reg_field_info[RGR1_SW_INIT_1_INIT_MASK]; > > + u32 shift = pcie->reg_field_info[RGR1_SW_INIT_1_INIT_SHIFT]; > > I don't think you need shift here, IIUC u32p_replace_bits() will take care of > all the masking and shifting internally, moreover, you'd be able to drop the > shift entry from reg_field_info. Got it. > > > - tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1); > > - u32p_replace_bits(&tmp, val, PCIE_RGR1_SW_INIT_1_INIT_MASK); > > - writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1); > > + tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1(pcie)); > > + tmp = (tmp & ~mask) | ((val << shift) & mask); > > + writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1(pcie)); > > } > > Regards, > Nicolas > Thanks! Jim
On Wed, 2020-05-20 at 10:30 -0400, Jim Quinlan wrote: > On Wed, May 20, 2020 at 7:51 AM Nicolas Saenz Julienne > <nsaenzjulienne@suse.de> wrote: [...] > > > + > > > +static const struct pcie_cfg_data bcm7278_cfg = { > > > + .reg_field_info = pcie_reg_field_info_bcm7278, > > > + .offsets = pcie_offset_bcm7278, > > > + .type = BCM7278, > > > +}; > > > > It's not essential, but if v2 is due I'd suggest factoring out the bcm2728 > > specific structures above, and moving them to patch #15. This will keep a > > clearer division between the patch introducing the infrastructure and the > > one > > adding the support for a new device. > The problem is that one of the commits needs the 7278 type so it has > to be declared earlier. Fair enough. > > > + > > > struct brcm_msi { > > > struct device *dev; > > > void __iomem *base; > > > @@ -176,6 +238,9 @@ struct brcm_pcie { > > > int gen; > > > u64 msi_target_addr; > > > struct brcm_msi *msi; > > > + const int *reg_offsets; > > > + const int *reg_field_info; > > > + enum pcie_type type; > > > }; > > > > > > /* > > > @@ -602,20 +667,21 @@ static struct pci_ops brcm_pcie_ops = { > > > > > > static inline void brcm_pcie_bridge_sw_init_set(struct brcm_pcie *pcie, > > > u32 > > > val) > > > { > > > - u32 tmp; > > > + u32 tmp, mask = pcie->reg_field_info[RGR1_SW_INIT_1_INIT_MASK]; > > > + u32 shift = pcie->reg_field_info[RGR1_SW_INIT_1_INIT_SHIFT]; > > > > I don't think you need shift here, IIUC u32p_replace_bits() will take care > > of > > all the masking and shifting internally, moreover, you'd be able to drop the > > shift entry from reg_field_info. > Got it. > > > - tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1); > > > - u32p_replace_bits(&tmp, val, PCIE_RGR1_SW_INIT_1_INIT_MASK); > > > - writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1); > > > + tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1(pcie)); > > > + tmp = (tmp & ~mask) | ((val << shift) & mask); > > > + writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1(pcie)); > > > } > > > > Regards, > > Nicolas > > > Thanks! > Jim
On Wed, May 20, 2020 at 7:51 AM Nicolas Saenz Julienne <nsaenzjulienne@suse.de> wrote: > > Hi Jim, > > On Tue, 2020-05-19 at 16:34 -0400, Jim Quinlan wrote: > > From: Jim Quinlan <jquinlan@broadcom.com> > > > > Add in compatibility strings and code for three Broadcom STB chips. > > Some of the register locations, shifts, and masks are different > > for certain chips, requiring the use of different constants based > > on of_id. > > > > We would like to add the following at this time to the match list > > but we need to wait until the end of this patchset so that > > everything works. > > > > { .compatible = "brcm,bcm7211-pcie", .data = &generic_cfg }, > > { .compatible = "brcm,bcm7278-pcie", .data = &bcm7278_cfg }, > > { .compatible = "brcm,bcm7216-pcie", .data = &bcm7278_cfg }, > > { .compatible = "brcm,bcm7445-pcie", .data = &generic_cfg }, > > > > Signed-off-by: Jim Quinlan <jquinlan@broadcom.com> > > --- > > drivers/pci/controller/pcie-brcmstb.c | 103 +++++++++++++++++++++++--- > > 1 file changed, 91 insertions(+), 12 deletions(-) > > > > diff --git a/drivers/pci/controller/pcie-brcmstb.c > > b/drivers/pci/controller/pcie-brcmstb.c > > index 73020b4ff090..c1cf4ea7d3d9 100644 > > --- a/drivers/pci/controller/pcie-brcmstb.c > > +++ b/drivers/pci/controller/pcie-brcmstb.c > > @@ -120,9 +120,8 @@ > > #define PCIE_EXT_SLOT_SHIFT 15 > > #define PCIE_EXT_FUNC_SHIFT 12 > > > > -#define PCIE_RGR1_SW_INIT_1 0x9210 > > #define PCIE_RGR1_SW_INIT_1_PERST_MASK 0x1 > > -#define PCIE_RGR1_SW_INIT_1_INIT_MASK 0x2 > > +#define PCIE_RGR1_SW_INIT_1_PERST_SHIFT 0x0 > > > > /* PCIe parameters */ > > #define BRCM_NUM_PCIE_OUT_WINS 0x4 > > @@ -152,6 +151,69 @@ > > #define SSC_STATUS_SSC_MASK 0x400 > > #define SSC_STATUS_PLL_LOCK_MASK 0x800 > > > > +#define IDX_ADDR(pcie) \ > > + (pcie->reg_offsets[EXT_CFG_INDEX]) > > +#define DATA_ADDR(pcie) \ > > + (pcie->reg_offsets[EXT_CFG_DATA]) > > +#define PCIE_RGR1_SW_INIT_1(pcie) \ > > + (pcie->reg_offsets[RGR1_SW_INIT_1]) > > + > > +enum { > > + RGR1_SW_INIT_1, > > + EXT_CFG_INDEX, > > + EXT_CFG_DATA, > > +}; > > + > > +enum { > > + RGR1_SW_INIT_1_INIT_MASK, > > + RGR1_SW_INIT_1_INIT_SHIFT, > > +}; > > + > > +enum pcie_type { > > + GENERIC, > > + BCM7278, > > +}; > > + > > +struct pcie_cfg_data { > > + const int *reg_field_info; > > + const int *offsets; > > + const enum pcie_type type; > > +}; > > + > > +static const int pcie_reg_field_info[] = { > > + [RGR1_SW_INIT_1_INIT_MASK] = 0x2, > > + [RGR1_SW_INIT_1_INIT_SHIFT] = 0x1, > > +}; > > + > > +static const int pcie_reg_field_info_bcm7278[] = { > > + [RGR1_SW_INIT_1_INIT_MASK] = 0x1, > > + [RGR1_SW_INIT_1_INIT_SHIFT] = 0x0, > > +}; > > + > > +static const int pcie_offsets[] = { > > + [RGR1_SW_INIT_1] = 0x9210, > > + [EXT_CFG_INDEX] = 0x9000, > > + [EXT_CFG_DATA] = 0x9004, > > +}; > > + > > +static const struct pcie_cfg_data generic_cfg = { > > + .reg_field_info = pcie_reg_field_info, > > + .offsets = pcie_offsets, > > + .type = GENERIC, > > +}; > > + > > +static const int pcie_offset_bcm7278[] = { > > + [RGR1_SW_INIT_1] = 0xc010, > > + [EXT_CFG_INDEX] = 0x9000, > > + [EXT_CFG_DATA] = 0x9004, > > +}; > > + > > +static const struct pcie_cfg_data bcm7278_cfg = { > > + .reg_field_info = pcie_reg_field_info_bcm7278, > > + .offsets = pcie_offset_bcm7278, > > + .type = BCM7278, > > +}; > > It's not essential, but if v2 is due I'd suggest factoring out the bcm2728 > specific structures above, and moving them to patch #15. This will keep a > clearer division between the patch introducing the infrastructure and the one > adding the support for a new device. > > > + > > struct brcm_msi { > > struct device *dev; > > void __iomem *base; > > @@ -176,6 +238,9 @@ struct brcm_pcie { > > int gen; > > u64 msi_target_addr; > > struct brcm_msi *msi; > > + const int *reg_offsets; > > + const int *reg_field_info; > > + enum pcie_type type; > > }; > > > > /* > > @@ -602,20 +667,21 @@ static struct pci_ops brcm_pcie_ops = { > > > > static inline void brcm_pcie_bridge_sw_init_set(struct brcm_pcie *pcie, u32 > > val) > > { > > - u32 tmp; > > + u32 tmp, mask = pcie->reg_field_info[RGR1_SW_INIT_1_INIT_MASK]; > > + u32 shift = pcie->reg_field_info[RGR1_SW_INIT_1_INIT_SHIFT]; > > I don't think you need shift here, IIUC u32p_replace_bits() will take care of > all the masking and shifting internally, moreover, you'd be able to drop the > shift entry from reg_field_info. I believe that u32p_replace_bits requires at least one of the value or mask to be compile time constants to work and we don't have that here. Regards, Jim > > > - tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1); > > - u32p_replace_bits(&tmp, val, PCIE_RGR1_SW_INIT_1_INIT_MASK); > > - writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1); > > + tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1(pcie)); > > + tmp = (tmp & ~mask) | ((val << shift) & mask); > > + writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1(pcie)); > > } > > Regards, > Nicolas >
On Thu, 2020-05-21 at 15:35 -0400, Jim Quinlan wrote: > On Wed, May 20, 2020 at 7:51 AM Nicolas Saenz Julienne [...] > > > /* > > > @@ -602,20 +667,21 @@ static struct pci_ops brcm_pcie_ops = { > > > > > > static inline void brcm_pcie_bridge_sw_init_set(struct brcm_pcie *pcie, > > > u32 > > > val) > > > { > > > - u32 tmp; > > > + u32 tmp, mask = pcie->reg_field_info[RGR1_SW_INIT_1_INIT_MASK]; > > > + u32 shift = pcie->reg_field_info[RGR1_SW_INIT_1_INIT_SHIFT]; > > > > I don't think you need shift here, IIUC u32p_replace_bits() will take care > > of > > all the masking and shifting internally, moreover, you'd be able to drop the > > shift entry from reg_field_info. > I believe that u32p_replace_bits requires at least one of the value or > mask to be compile time constants to work and we don't have that here. Of course, sorry for the noise then. Regards, Nicolas
diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c index 73020b4ff090..c1cf4ea7d3d9 100644 --- a/drivers/pci/controller/pcie-brcmstb.c +++ b/drivers/pci/controller/pcie-brcmstb.c @@ -120,9 +120,8 @@ #define PCIE_EXT_SLOT_SHIFT 15 #define PCIE_EXT_FUNC_SHIFT 12 -#define PCIE_RGR1_SW_INIT_1 0x9210 #define PCIE_RGR1_SW_INIT_1_PERST_MASK 0x1 -#define PCIE_RGR1_SW_INIT_1_INIT_MASK 0x2 +#define PCIE_RGR1_SW_INIT_1_PERST_SHIFT 0x0 /* PCIe parameters */ #define BRCM_NUM_PCIE_OUT_WINS 0x4 @@ -152,6 +151,69 @@ #define SSC_STATUS_SSC_MASK 0x400 #define SSC_STATUS_PLL_LOCK_MASK 0x800 +#define IDX_ADDR(pcie) \ + (pcie->reg_offsets[EXT_CFG_INDEX]) +#define DATA_ADDR(pcie) \ + (pcie->reg_offsets[EXT_CFG_DATA]) +#define PCIE_RGR1_SW_INIT_1(pcie) \ + (pcie->reg_offsets[RGR1_SW_INIT_1]) + +enum { + RGR1_SW_INIT_1, + EXT_CFG_INDEX, + EXT_CFG_DATA, +}; + +enum { + RGR1_SW_INIT_1_INIT_MASK, + RGR1_SW_INIT_1_INIT_SHIFT, +}; + +enum pcie_type { + GENERIC, + BCM7278, +}; + +struct pcie_cfg_data { + const int *reg_field_info; + const int *offsets; + const enum pcie_type type; +}; + +static const int pcie_reg_field_info[] = { + [RGR1_SW_INIT_1_INIT_MASK] = 0x2, + [RGR1_SW_INIT_1_INIT_SHIFT] = 0x1, +}; + +static const int pcie_reg_field_info_bcm7278[] = { + [RGR1_SW_INIT_1_INIT_MASK] = 0x1, + [RGR1_SW_INIT_1_INIT_SHIFT] = 0x0, +}; + +static const int pcie_offsets[] = { + [RGR1_SW_INIT_1] = 0x9210, + [EXT_CFG_INDEX] = 0x9000, + [EXT_CFG_DATA] = 0x9004, +}; + +static const struct pcie_cfg_data generic_cfg = { + .reg_field_info = pcie_reg_field_info, + .offsets = pcie_offsets, + .type = GENERIC, +}; + +static const int pcie_offset_bcm7278[] = { + [RGR1_SW_INIT_1] = 0xc010, + [EXT_CFG_INDEX] = 0x9000, + [EXT_CFG_DATA] = 0x9004, +}; + +static const struct pcie_cfg_data bcm7278_cfg = { + .reg_field_info = pcie_reg_field_info_bcm7278, + .offsets = pcie_offset_bcm7278, + .type = BCM7278, +}; + struct brcm_msi { struct device *dev; void __iomem *base; @@ -176,6 +238,9 @@ struct brcm_pcie { int gen; u64 msi_target_addr; struct brcm_msi *msi; + const int *reg_offsets; + const int *reg_field_info; + enum pcie_type type; }; /* @@ -602,20 +667,21 @@ static struct pci_ops brcm_pcie_ops = { static inline void brcm_pcie_bridge_sw_init_set(struct brcm_pcie *pcie, u32 val) { - u32 tmp; + u32 tmp, mask = pcie->reg_field_info[RGR1_SW_INIT_1_INIT_MASK]; + u32 shift = pcie->reg_field_info[RGR1_SW_INIT_1_INIT_SHIFT]; - tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1); - u32p_replace_bits(&tmp, val, PCIE_RGR1_SW_INIT_1_INIT_MASK); - writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1); + tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1(pcie)); + tmp = (tmp & ~mask) | ((val << shift) & mask); + writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1(pcie)); } static inline void brcm_pcie_perst_set(struct brcm_pcie *pcie, u32 val) { u32 tmp; - tmp = readl(pcie->base + PCIE_RGR1_SW_INIT_1); + 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); + writel(tmp, pcie->base + PCIE_RGR1_SW_INIT_1(pcie)); } static inline int brcm_pcie_get_rc_bar2_size_and_offset(struct brcm_pcie *pcie, @@ -924,10 +990,17 @@ static int brcm_pcie_remove(struct platform_device *pdev) return 0; } +static const struct of_device_id brcm_pcie_match[] = { + { .compatible = "brcm,bcm2711-pcie", .data = &generic_cfg }, + {}, +}; + static int brcm_pcie_probe(struct platform_device *pdev) { struct device_node *np = pdev->dev.of_node, *msi_np; struct pci_host_bridge *bridge; + const struct pcie_cfg_data *data; + const struct of_device_id *of_id; struct brcm_pcie *pcie; struct pci_bus *child; struct resource *res; @@ -937,9 +1010,19 @@ static int brcm_pcie_probe(struct platform_device *pdev) if (!bridge) return -ENOMEM; + of_id = of_match_node(brcm_pcie_match, np); + if (!of_id) { + pr_err("failed to look up compatible string\n"); + return -EINVAL; + } + data = of_id->data; + pcie = pci_host_bridge_priv(bridge); pcie->dev = &pdev->dev; pcie->np = np; + pcie->reg_offsets = data->offsets; + pcie->reg_field_info = data->reg_field_info; + pcie->type = data->type; res = platform_get_resource(pdev, IORESOURCE_MEM, 0); pcie->base = devm_ioremap_resource(&pdev->dev, res); @@ -1005,10 +1088,6 @@ static int brcm_pcie_probe(struct platform_device *pdev) return ret; } -static const struct of_device_id brcm_pcie_match[] = { - { .compatible = "brcm,bcm2711-pcie" }, - {}, -}; MODULE_DEVICE_TABLE(of, brcm_pcie_match); static struct platform_driver brcm_pcie_driver = {