Re: [PATCH v4 11/12] PCI: brcmstb: Change field name from 'type' to 'model'

[Manivannan Sadhasivam]

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.

- Mani

> Signed-off-by: Jim Quinlan <james.quinlan@xxxxxxxxxxxx>
> Reviewed-by: Florian Fainelli <florian.fainelli@xxxxxxxxxxxx>
> ---
>  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
> 



-- 
மணிவண்ணன் சதாசிவம்