lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+-6iNxWz1aJr2rHJKSpXaD1raUij-P_Xo9a6MvWJLae1_m7oQ@mail.gmail.com>
Date: Thu, 25 Jul 2024 16:38:12 -0400
From: Jim Quinlan <james.quinlan@...adcom.com>
To: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
Cc: linux-pci@...r.kernel.org, Nicolas Saenz Julienne <nsaenz@...nel.org>, 
	Bjorn Helgaas <bhelgaas@...gle.com>, Lorenzo Pieralisi <lorenzo.pieralisi@....com>, 
	Cyril Brulebois <kibi@...ian.org>, Stanimir Varbanov <svarbanov@...e.de>, 
	Krzysztof Kozlowski <krzk@...nel.org>, bcm-kernel-feedback-list@...adcom.com, 
	jim2101024@...il.com, Florian Fainelli <florian.fainelli@...adcom.com>, 
	Lorenzo Pieralisi <lpieralisi@...nel.org>, Krzysztof Wilczyński <kw@...ux.com>, 
	Rob Herring <robh@...nel.org>, 
	"moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" <linux-rpi-kernel@...ts.infradead.org>, 
	"moderated list:BROADCOM BCM2711/BCM2835 ARM ARCHITECTURE" <linux-arm-kernel@...ts.infradead.org>, 
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v4 11/12] PCI: brcmstb: Change field name from 'type' to 'model'

On Thu, Jul 25, 2024 at 12:58 AM Manivannan Sadhasivam
<manivannan.sadhasivam@...aro.org> wrote:
>
> On Tue, Jul 16, 2024 at 05:31:26PM -0400, Jim Quinlan wrote:
> > The 'type' field used in the driver to discern SoC differences is confusing
> > so change it to the more apt 'model'.  We considered using 'family' but
> > this conflicts with Broadcom's conception of a family; for example, 7216a0
> > and 7216b0 chips are both considered separate families as each has multiple
> > derivative product chips based on the original design.
> >
>
> TBH, 'model' is also confusing :) Why can't you just use 'soc' as you are
> referrring to the SoC name.

Hello,

Well, the "model" we assign is not necessarily the same as the SoC.
If a new SoC has the same characteristics as a previous "model", we
will not create a new model but rather use the existing one.   For example,
the bcm7216_cfg structure, which is for the 7216 SoC uses the model "BCM7278".

I agree that this is not crystal clear but using SoC could be
considered misleading.

Regards,
Jim Quinlan
Broadcom STB/CM

>
> - Mani
>
> > Signed-off-by: Jim Quinlan <james.quinlan@...adcom.com>
> > Reviewed-by: Florian Fainelli <florian.fainelli@...adcom.com>
> > ---
> >  drivers/pci/controller/pcie-brcmstb.c | 42 +++++++++++++--------------
> >  1 file changed, 21 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c
> > index 2fe1f2a26697..fa5616a56383 100644
> > --- a/drivers/pci/controller/pcie-brcmstb.c
> > +++ b/drivers/pci/controller/pcie-brcmstb.c
> > @@ -211,7 +211,7 @@ enum {
> >       PCIE_INTR2_CPU_BASE,
> >  };
> >
> > -enum pcie_type {
> > +enum pcie_model {
> >       GENERIC,
> >       BCM7425,
> >       BCM7435,
> > @@ -229,7 +229,7 @@ struct rc_bar {
> >
> >  struct pcie_cfg_data {
> >       const int *offsets;
> > -     const enum pcie_type type;
> > +     const enum pcie_model model;
> >       const bool has_phy;
> >       unsigned int num_inbound;
> >       int (*perst_set)(struct brcm_pcie *pcie, u32 val);
> > @@ -270,7 +270,7 @@ struct brcm_pcie {
> >       u64                     msi_target_addr;
> >       struct brcm_msi         *msi;
> >       const int               *reg_offsets;
> > -     enum pcie_type          type;
> > +     enum pcie_model         model;
> >       struct reset_control    *rescal;
> >       struct reset_control    *perst_reset;
> >       struct reset_control    *bridge;
> > @@ -288,7 +288,7 @@ struct brcm_pcie {
> >
> >  static inline bool is_bmips(const struct brcm_pcie *pcie)
> >  {
> > -     return pcie->type == BCM7435 || pcie->type == BCM7425;
> > +     return pcie->model == BCM7435 || pcie->model == BCM7425;
> >  }
> >
> >  /*
> > @@ -852,7 +852,7 @@ static int brcm_pcie_get_inbound_wins(struct brcm_pcie *pcie,
> >        * security considerations, and is not implemented in our modern
> >        * SoCs.
> >        */
> > -     if (pcie->type != BCM7712)
> > +     if (pcie->model != BCM7712)
> >               set_bar(b++, &n, 0, 0, 0);
> >
> >       resource_list_for_each_entry(entry, &bridge->dma_ranges) {
> > @@ -869,7 +869,7 @@ static int brcm_pcie_get_inbound_wins(struct brcm_pcie *pcie,
> >                * That being said, each BARs size must still be a power of
> >                * two.
> >                */
> > -             if (pcie->type == BCM7712)
> > +             if (pcie->model == BCM7712)
> >                       set_bar(b++, &n, size, cpu_beg, pcie_beg);
> >
> >               if (n > pcie->num_inbound)
> > @@ -886,7 +886,7 @@ static int brcm_pcie_get_inbound_wins(struct brcm_pcie *pcie,
> >        * that enables multiple memory controllers.  As such, it can return
> >        * now w/o doing special configuration.
> >        */
> > -     if (pcie->type == BCM7712)
> > +     if (pcie->model == BCM7712)
> >               return n;
> >
> >       ret = of_property_read_variable_u64_array(pcie->np, "brcm,scb-sizes", pcie->memc_size, 1,
> > @@ -1008,7 +1008,7 @@ static void set_inbound_win_registers(struct brcm_pcie *pcie, const struct rc_ba
> >                * 7712:
> >                *     All of their BARs need to be set.
> >                */
> > -             if (pcie->type == BCM7712) {
> > +             if (pcie->model == BCM7712) {
> >                       /* BUS remap register settings */
> >                       reg_offset = brcm_ubus_reg_offset(i);
> >                       tmp = lower_32_bits(cpu_addr) & ~0xfff;
> > @@ -1036,7 +1036,7 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
> >               return ret;
> >
> >       /* Ensure that PERST# is asserted; some bootloaders may deassert it. */
> > -     if (pcie->type == BCM2711) {
> > +     if (pcie->model == BCM2711) {
> >               ret = pcie->perst_set(pcie, 1);
> >               if (ret) {
> >                       pcie->bridge_sw_init_set(pcie, 0);
> > @@ -1067,9 +1067,9 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie)
> >        */
> >       if (is_bmips(pcie))
> >               burst = 0x1; /* 256 bytes */
> > -     else if (pcie->type == BCM2711)
> > +     else if (pcie->model == BCM2711)
> >               burst = 0x0; /* 128 bytes */
> > -     else if (pcie->type == BCM7278)
> > +     else if (pcie->model == BCM7278)
> >               burst = 0x3; /* 512 bytes */
> >       else
> >               burst = 0x2; /* 512 bytes */
> > @@ -1666,7 +1666,7 @@ static const int pcie_offsets_bmips_7425[] = {
> >
> >  static const struct pcie_cfg_data generic_cfg = {
> >       .offsets        = pcie_offsets,
> > -     .type           = GENERIC,
> > +     .model          = GENERIC,
> >       .perst_set      = brcm_pcie_perst_set_generic,
> >       .bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_generic,
> >       .num_inbound    = 3,
> > @@ -1674,7 +1674,7 @@ static const struct pcie_cfg_data generic_cfg = {
> >
> >  static const struct pcie_cfg_data bcm7425_cfg = {
> >       .offsets        = pcie_offsets_bmips_7425,
> > -     .type           = BCM7425,
> > +     .model          = BCM7425,
> >       .perst_set      = brcm_pcie_perst_set_generic,
> >       .bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_generic,
> >       .num_inbound    = 3,
> > @@ -1682,7 +1682,7 @@ static const struct pcie_cfg_data bcm7425_cfg = {
> >
> >  static const struct pcie_cfg_data bcm7435_cfg = {
> >       .offsets        = pcie_offsets,
> > -     .type           = BCM7435,
> > +     .model          = BCM7435,
> >       .perst_set      = brcm_pcie_perst_set_generic,
> >       .bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_generic,
> >       .num_inbound    = 3,
> > @@ -1690,7 +1690,7 @@ static const struct pcie_cfg_data bcm7435_cfg = {
> >
> >  static const struct pcie_cfg_data bcm4908_cfg = {
> >       .offsets        = pcie_offsets,
> > -     .type           = BCM4908,
> > +     .model          = BCM4908,
> >       .perst_set      = brcm_pcie_perst_set_4908,
> >       .bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_generic,
> >       .num_inbound    = 3,
> > @@ -1706,7 +1706,7 @@ static const int pcie_offset_bcm7278[] = {
> >
> >  static const struct pcie_cfg_data bcm7278_cfg = {
> >       .offsets        = pcie_offset_bcm7278,
> > -     .type           = BCM7278,
> > +     .model          = BCM7278,
> >       .perst_set      = brcm_pcie_perst_set_7278,
> >       .bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_7278,
> >       .num_inbound    = 3,
> > @@ -1714,7 +1714,7 @@ static const struct pcie_cfg_data bcm7278_cfg = {
> >
> >  static const struct pcie_cfg_data bcm2711_cfg = {
> >       .offsets        = pcie_offsets,
> > -     .type           = BCM2711,
> > +     .model          = BCM2711,
> >       .perst_set      = brcm_pcie_perst_set_generic,
> >       .bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_generic,
> >       .num_inbound    = 3,
> > @@ -1722,7 +1722,7 @@ static const struct pcie_cfg_data bcm2711_cfg = {
> >
> >  static const struct pcie_cfg_data bcm7216_cfg = {
> >       .offsets        = pcie_offset_bcm7278,
> > -     .type           = BCM7278,
> > +     .model          = BCM7278,
> >       .perst_set      = brcm_pcie_perst_set_7278,
> >       .bridge_sw_init_set = brcm_pcie_bridge_sw_init_set_7278,
> >       .has_phy        = true,
> > @@ -1779,7 +1779,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
> >       pcie->dev = &pdev->dev;
> >       pcie->np = np;
> >       pcie->reg_offsets = data->offsets;
> > -     pcie->type = data->type;
> > +     pcie->model = data->model;
> >       pcie->perst_set = data->perst_set;
> >       pcie->bridge_sw_init_set = data->bridge_sw_init_set;
> >       pcie->has_phy = data->has_phy;
> > @@ -1848,7 +1848,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
> >               goto fail;
> >
> >       pcie->hw_rev = readl(pcie->base + PCIE_MISC_REVISION);
> > -     if (pcie->type == BCM4908 && pcie->hw_rev >= BRCM_PCIE_HW_REV_3_20) {
> > +     if (pcie->model == BCM4908 && pcie->hw_rev >= BRCM_PCIE_HW_REV_3_20) {
> >               dev_err(pcie->dev, "hardware revision with unsupported PERST# setup\n");
> >               ret = -ENODEV;
> >               goto fail;
> > @@ -1863,7 +1863,7 @@ static int brcm_pcie_probe(struct platform_device *pdev)
> >               }
> >       }
> >
> > -     bridge->ops = pcie->type == BCM7425 ? &brcm7425_pcie_ops : &brcm_pcie_ops;
> > +     bridge->ops = pcie->model == BCM7425 ? &brcm7425_pcie_ops : &brcm_pcie_ops;
> >       bridge->sysdata = pcie;
> >
> >       platform_set_drvdata(pdev, pcie);
> > --
> > 2.17.1
> >
>
>
>
> --
> மணிவண்ணன் சதாசிவம்

Download attachment "smime.p7s" of type "application/pkcs7-signature" (4210 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ