[PATCH v1 6/8] PCI: brcmstb: Don't conflate the reset rescal with phy ctrl

[Jim Quinlan]

On Tue, Jul 2, 2024 at 9:10 AM Stanimir Varbanov <svarbanov at suse.de> wrote:
>
>
>
> On 6/28/24 23:54, Jim Quinlan wrote:
> > We've been assuming that if an SOC has a "rescal" reset controller that we
> > should automatically invoke brcm_phy_cntl(...).  This will not be true in
> > future SOCs, so we create a bool "has_phy" and adjust the cfg_data
> > appropriately (we need to give 7216 its own cfg_data structure instead of
> > sharing one).
> >
> > Signed-off-by: Jim Quinlan <james.quinlan at broadcom.com>
> > ---
> >  drivers/pci/controller/pcie-brcmstb.c | 17 ++++++++++++++---
> >  1 file changed, 14 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> > index 4e0848e1311f..e740e2966a5c 100644
> > --- a/drivers/pci/controller/pcie-brcmstb.c
> > +++ b/drivers/pci/controller/pcie-brcmstb.c
> > @@ -227,6 +227,7 @@ enum pcie_type {
> >  struct pcie_cfg_data {
> >       const int *offsets;
> >       const enum pcie_type type;
> > +     const bool has_phy;
> >       void (*perst_set)(struct brcm_pcie *pcie, u32 val);
> >       void (*bridge_sw_init_set)(struct brcm_pcie *pcie, u32 val);
> >  };
> > @@ -277,6 +278,7 @@ struct brcm_pcie {
> >       void                    (*bridge_sw_init_set)(struct brcm_pcie *pcie, u32 val);
> >       struct subdev_regulators *sr;
> >       bool                    ep_wakeup_capable;
> > +     bool                    has_phy;
> >  };
> >
> >  static inline bool is_bmips(const struct brcm_pcie *pcie)
> > @@ -1316,12 +1318,12 @@ static int brcm_phy_cntl(struct brcm_pcie *pcie, const int start)
> >
> >  static inline int brcm_phy_start(struct brcm_pcie *pcie)
> >  {
> > -     return pcie->rescal ? brcm_phy_cntl(pcie, 1) : 0;
> > +     return pcie->has_phy ? brcm_phy_cntl(pcie, 1) : 0;
> >  }
> >
> >  static inline int brcm_phy_stop(struct brcm_pcie *pcie)
> >  {
> > -     return pcie->rescal ? brcm_phy_cntl(pcie, 0) : 0;
> > +     return pcie->has_phy ? brcm_phy_cntl(pcie, 0) : 0;
> >  }
> >
> >  static void brcm_pcie_turn_off(struct brcm_pcie *pcie)
> > @@ -1564,12 +1566,20 @@ static const struct pcie_cfg_data bcm2711_cfg = {
> >       .bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_generic,
> >  };
> >
> > +static const struct pcie_cfg_data bcm7216_cfg = {
> > +     .offsets        = pcie_offset_bcm7278,
> > +     .type           = BCM7278,
>
> This "type" field is confusing, maybe it would be good to rename it to
> "family"? For example BCM72XX family.

Hi Stanimir,

I'm open for another name but "family" would present problems with Broadcom STB.
For example, we call 7216b0 a "family" as there are a number of
derivative products based off
of this general design.  Second, having something like "BCM72XX" won't work;
we have 7211 which is something altogether different from the 7216.
Note that we only
introduce a new "type" when we need to; if the behavior is the same as
a previously declared
"type" we do not introduce new ones.

But if you wanted to change "type" to "model" then I have no problem with that.

Regards,
Jim Quinlan
Broadcom STB/CM

>
> > +     .perst_set      = brcm_pcie_perst_set_7278,
> > +     .bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_7278,
> > +     .has_phy        = true,
> > +};
> > +
> >  static const struct of_device_id brcm_pcie_match[] = {
> >       { .compatible = "brcm,bcm2711-pcie", .data = &bcm2711_cfg },
> >       { .compatible = "brcm,bcm4908-pcie", .data = &bcm4908_cfg },
> >       { .compatible = "brcm,bcm7211-pcie", .data = &generic_cfg },
> >       { .compatible = "brcm,bcm7278-pcie", .data = &bcm7278_cfg },
> > -     { .compatible = "brcm,bcm7216-pcie", .data = &bcm7278_cfg },
> > +     { .compatible = "brcm,bcm7216-pcie", .data = &bcm7216_cfg },
> >       { .compatible = "brcm,bcm7445-pcie", .data = &generic_cfg },
> >       { .compatible = "brcm,bcm7435-pcie", .data = &bcm7435_cfg },
> >       { .compatible = "brcm,bcm7425-pcie", .data = &bcm7425_cfg },
> > @@ -1617,6 +1627,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
> >       pcie->type = data->type;
> >       pcie->perst_set = data->perst_set;
> >       pcie->bridge_sw_init_set = data->bridge_sw_init_set;
> > +     pcie->has_phy = data->has_phy;
> >
> >       pcie->base = devm_platform_ioremap_resource(pdev, 0);
> >       if (IS_ERR(pcie->base))
>
> ~Stan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/pkcs7-signature
Size: 4210 bytes
Desc: S/MIME Cryptographic Signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20240702/295abf2a/attachment.p7s>