| Message ID | 20230314003613.3874089-10-seanga2@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: sunhme: Probe/IRQ cleanups | expand |
On Mon, Mar 13, 2023 at 08:36:13PM -0400, Sean Anderson wrote: > Most of the second half of the PCI/SBUS probe functions are the same. > Consolidate them into a common function. > > Signed-off-by: Sean Anderson <seanga2@gmail.com> > --- > > (no changes since v1) > > drivers/net/ethernet/sun/sunhme.c | 183 ++++++++++++------------------ > 1 file changed, 71 insertions(+), 112 deletions(-) > > diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c > index a59b998062d9..a384b162c46d 100644 > --- a/drivers/net/ethernet/sun/sunhme.c > +++ b/drivers/net/ethernet/sun/sunhme.c > @@ -2430,6 +2430,71 @@ static void happy_meal_addr_init(struct happy_meal *hp, > } > } > > +static int happy_meal_common_probe(struct happy_meal *hp, > + struct device_node *dp, int minor_rev) > +{ > + struct net_device *dev = hp->dev; > + int err; > + > +#ifdef CONFIG_SPARC > + hp->hm_revision = of_getintprop_default(dp, "hm-rev", 0xff); > + if (hp->hm_revision == 0xff) > + hp->hm_revision = 0xc0 | minor_rev; > +#else > + /* works with this on non-sparc hosts */ > + hp->hm_revision = 0x20; > +#endif ... > +#if defined(CONFIG_SBUS) && defined(CONFIG_PCI) > + /* Hook up SBUS register/descriptor accessors. */ > + hp->read_desc32 = sbus_hme_read_desc32; > + hp->write_txd = sbus_hme_write_txd; > + hp->write_rxd = sbus_hme_write_rxd; > + hp->read32 = sbus_hme_read32; > + hp->write32 = sbus_hme_write32; > +#endif This looks correct for the SBUS case. But I'm not sure about the PCIE case. gcc 12 tells me when compiling with sparc allmodconfig that the following functions are now unused. pci_hme_read_desc32 pci_hme_write_txd pci_hme_write_rxd pci_hme_read32 pci_hme_write32 > + > + /* Grrr, Happy Meal comes up by default not advertising > + * full duplex 100baseT capabilities, fix this. b > + */ > + spin_lock_irq(&hp->happy_lock); > + happy_meal_set_initial_advertisement(hp); > + spin_unlock_irq(&hp->happy_lock); > + > + err = devm_register_netdev(hp->dma_dev, dev); > + if (err) > + dev_err(hp->dma_dev, "Cannot register net device, aborting.\n"); > + return err; > +} > + > #ifdef CONFIG_SBUS > static int happy_meal_sbus_probe_one(struct platform_device *op, int is_qfe) > { > @@ -2511,70 +2576,18 @@ static int happy_meal_sbus_probe_one(struct platform_device *op, int is_qfe) > goto err_out_clear_quattro; > } > > - hp->hm_revision = of_getintprop_default(dp, "hm-rev", 0xff); > - if (hp->hm_revision == 0xff) > - hp->hm_revision = 0xa0; It's not clear to me that the same value will be set by the call to happy_meal_common_probe(hp, dp, 0); where the logic is: #ifdef CONFIG_SPARC hp->hm_revision = of_getintprop_default(dp, "hm-rev", 0xff); if (hp->hm_revision == 0xff) hp->hm_revision = 0xc0 | minor_rev; #else /* works with this on non-sparc hosts */ hp->hm_revision = 0x20; #endif I am assuming that the SPARC logic is run. But another question: is it strictly true that SBUS means SPARC? ... > - > -#if defined(CONFIG_SBUS) && defined(CONFIG_PCI) > - /* Hook up SBUS register/descriptor accessors. */ > - hp->read_desc32 = sbus_hme_read_desc32; > - hp->write_txd = sbus_hme_write_txd; > - hp->write_rxd = sbus_hme_write_rxd; > - hp->read32 = sbus_hme_read32; > - hp->write32 = sbus_hme_write32; > -#endif ... > @@ -2689,21 +2702,6 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, > hp->bigmacregs = (hpreg_base + 0x6000UL); > hp->tcvregs = (hpreg_base + 0x7000UL); > > -#ifdef CONFIG_SPARC > - hp->hm_revision = of_getintprop_default(dp, "hm-rev", 0xff); > - if (hp->hm_revision == 0xff) > - hp->hm_revision = 0xc0 | (pdev->revision & 0x0f); > -#else > - /* works with this on non-sparc hosts */ > - hp->hm_revision = 0x20; > -#endif ... > -#if defined(CONFIG_SBUS) && defined(CONFIG_PCI) > - /* Hook up PCI register/descriptor accessors. */ > - hp->read_desc32 = pci_hme_read_desc32; > - hp->write_txd = pci_hme_write_txd; > - hp->write_rxd = pci_hme_write_rxd; > - hp->read32 = pci_hme_read32; > - hp->write32 = pci_hme_write32; > -#endif ...
On 3/18/23 09:53, Simon Horman wrote: > On Mon, Mar 13, 2023 at 08:36:13PM -0400, Sean Anderson wrote: >> Most of the second half of the PCI/SBUS probe functions are the same. >> Consolidate them into a common function. >> >> Signed-off-by: Sean Anderson <seanga2@gmail.com> >> --- >> >> (no changes since v1) >> >> drivers/net/ethernet/sun/sunhme.c | 183 ++++++++++++------------------ >> 1 file changed, 71 insertions(+), 112 deletions(-) >> >> diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c >> index a59b998062d9..a384b162c46d 100644 >> --- a/drivers/net/ethernet/sun/sunhme.c >> +++ b/drivers/net/ethernet/sun/sunhme.c >> @@ -2430,6 +2430,71 @@ static void happy_meal_addr_init(struct happy_meal *hp, >> } >> } >> >> +static int happy_meal_common_probe(struct happy_meal *hp, >> + struct device_node *dp, int minor_rev) >> +{ >> + struct net_device *dev = hp->dev; >> + int err; >> + >> +#ifdef CONFIG_SPARC >> + hp->hm_revision = of_getintprop_default(dp, "hm-rev", 0xff); >> + if (hp->hm_revision == 0xff) >> + hp->hm_revision = 0xc0 | minor_rev; >> +#else >> + /* works with this on non-sparc hosts */ >> + hp->hm_revision = 0x20; >> +#endif > > ... > >> +#if defined(CONFIG_SBUS) && defined(CONFIG_PCI) >> + /* Hook up SBUS register/descriptor accessors. */ >> + hp->read_desc32 = sbus_hme_read_desc32; >> + hp->write_txd = sbus_hme_write_txd; >> + hp->write_rxd = sbus_hme_write_rxd; >> + hp->read32 = sbus_hme_read32; >> + hp->write32 = sbus_hme_write32; >> +#endif > > This looks correct for the SBUS case. > But I'm not sure about the PCIE case. > > gcc 12 tells me when compiling with sparc allmodconfig that the following > functions are now unused. > > pci_hme_read_desc32 > pci_hme_write_txd > pci_hme_write_rxd > pci_hme_read32 > pci_hme_write32 Oh, looks like I missed that these were different while refactoring. That said, I haven't seen any issues here... >> + >> + /* Grrr, Happy Meal comes up by default not advertising >> + * full duplex 100baseT capabilities, fix this. > b >> + */ >> + spin_lock_irq(&hp->happy_lock); >> + happy_meal_set_initial_advertisement(hp); >> + spin_unlock_irq(&hp->happy_lock); >> + >> + err = devm_register_netdev(hp->dma_dev, dev); >> + if (err) >> + dev_err(hp->dma_dev, "Cannot register net device, aborting.\n"); >> + return err; >> +} >> + >> #ifdef CONFIG_SBUS >> static int happy_meal_sbus_probe_one(struct platform_device *op, int is_qfe) >> { > >> @@ -2511,70 +2576,18 @@ static int happy_meal_sbus_probe_one(struct platform_device *op, int is_qfe) >> goto err_out_clear_quattro; >> } >> >> - hp->hm_revision = of_getintprop_default(dp, "hm-rev", 0xff); >> - if (hp->hm_revision == 0xff) >> - hp->hm_revision = 0xa0; > > It's not clear to me that the same value will be set by the call to > happy_meal_common_probe(hp, dp, 0); where the logic is: > > #ifdef CONFIG_SPARC > hp->hm_revision = of_getintprop_default(dp, "hm-rev", 0xff); > if (hp->hm_revision == 0xff) > hp->hm_revision = 0xc0 | minor_rev; OK, so maybe this should be xor, with sbus passing in 0x30. > #else > /* works with this on non-sparc hosts */ > hp->hm_revision = 0x20; > #endif > > I am assuming that the SPARC logic is run. > But another question: is it strictly true that SBUS means SPARC? Yes. >> - >> -#if defined(CONFIG_SBUS) && defined(CONFIG_PCI) >> - /* Hook up SBUS register/descriptor accessors. */ >> - hp->read_desc32 = sbus_hme_read_desc32; >> - hp->write_txd = sbus_hme_write_txd; >> - hp->write_rxd = sbus_hme_write_rxd; >> - hp->read32 = sbus_hme_read32; >> - hp->write32 = sbus_hme_write32; >> -#endif > > ... > >> @@ -2689,21 +2702,6 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, >> hp->bigmacregs = (hpreg_base + 0x6000UL); >> hp->tcvregs = (hpreg_base + 0x7000UL); >> >> -#ifdef CONFIG_SPARC >> - hp->hm_revision = of_getintprop_default(dp, "hm-rev", 0xff); >> - if (hp->hm_revision == 0xff) >> - hp->hm_revision = 0xc0 | (pdev->revision & 0x0f); >> -#else >> - /* works with this on non-sparc hosts */ >> - hp->hm_revision = 0x20; >> -#endif > > ... > >> -#if defined(CONFIG_SBUS) && defined(CONFIG_PCI) >> - /* Hook up PCI register/descriptor accessors. */ >> - hp->read_desc32 = pci_hme_read_desc32; >> - hp->write_txd = pci_hme_write_txd; >> - hp->write_rxd = pci_hme_write_rxd; >> - hp->read32 = pci_hme_read32; >> - hp->write32 = pci_hme_write32; >> -#endif > > ... --Sean
On Sat, Mar 18, 2023 at 10:40:46AM -0400, Sean Anderson wrote: > On 3/18/23 09:53, Simon Horman wrote: > > On Mon, Mar 13, 2023 at 08:36:13PM -0400, Sean Anderson wrote: > > > Most of the second half of the PCI/SBUS probe functions are the same. > > > Consolidate them into a common function. > > > > > > Signed-off-by: Sean Anderson <seanga2@gmail.com> ... > > > @@ -2511,70 +2576,18 @@ static int happy_meal_sbus_probe_one(struct platform_device *op, int is_qfe) > > > goto err_out_clear_quattro; > > > } > > > - hp->hm_revision = of_getintprop_default(dp, "hm-rev", 0xff); > > > - if (hp->hm_revision == 0xff) > > > - hp->hm_revision = 0xa0; > > > > It's not clear to me that the same value will be set by the call to > > happy_meal_common_probe(hp, dp, 0); where the logic is: > > > > #ifdef CONFIG_SPARC > > hp->hm_revision = of_getintprop_default(dp, "hm-rev", 0xff); > > if (hp->hm_revision == 0xff) > > hp->hm_revision = 0xc0 | minor_rev; > > OK, so maybe this should be xor, with sbus passing in 0x30. Maybe moving the math to the caller makes things easier. I'm unsure. > > #else > > /* works with this on non-sparc hosts */ > > hp->hm_revision = 0x20; > > #endif > > > > I am assuming that the SPARC logic is run. > > But another question: is it strictly true that SBUS means SPARC? > > Yes. Thanks, got it.
diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c index a59b998062d9..a384b162c46d 100644 --- a/drivers/net/ethernet/sun/sunhme.c +++ b/drivers/net/ethernet/sun/sunhme.c @@ -2430,6 +2430,71 @@ static void happy_meal_addr_init(struct happy_meal *hp, } } +static int happy_meal_common_probe(struct happy_meal *hp, + struct device_node *dp, int minor_rev) +{ + struct net_device *dev = hp->dev; + int err; + +#ifdef CONFIG_SPARC + hp->hm_revision = of_getintprop_default(dp, "hm-rev", 0xff); + if (hp->hm_revision == 0xff) + hp->hm_revision = 0xc0 | minor_rev; +#else + /* works with this on non-sparc hosts */ + hp->hm_revision = 0x20; +#endif + + /* Now enable the feature flags we can. */ + if (hp->hm_revision == 0x20 || hp->hm_revision == 0x21) + hp->happy_flags |= HFLAG_20_21; + else if (hp->hm_revision != 0xa0) + hp->happy_flags |= HFLAG_NOT_A0; + + hp->happy_block = dmam_alloc_coherent(hp->dma_dev, PAGE_SIZE, + &hp->hblock_dvma, GFP_KERNEL); + if (!hp->happy_block) + return -ENOMEM; + + /* Force check of the link first time we are brought up. */ + hp->linkcheck = 0; + + /* Force timer state to 'asleep' with count of zero. */ + hp->timer_state = asleep; + hp->timer_ticks = 0; + + timer_setup(&hp->happy_timer, happy_meal_timer, 0); + + dev->netdev_ops = &hme_netdev_ops; + dev->watchdog_timeo = 5 * HZ; + dev->ethtool_ops = &hme_ethtool_ops; + + /* Happy Meal can do it all... */ + dev->hw_features = NETIF_F_SG | NETIF_F_HW_CSUM; + dev->features |= dev->hw_features | NETIF_F_RXCSUM; + +#if defined(CONFIG_SBUS) && defined(CONFIG_PCI) + /* Hook up SBUS register/descriptor accessors. */ + hp->read_desc32 = sbus_hme_read_desc32; + hp->write_txd = sbus_hme_write_txd; + hp->write_rxd = sbus_hme_write_rxd; + hp->read32 = sbus_hme_read32; + hp->write32 = sbus_hme_write32; +#endif + + /* Grrr, Happy Meal comes up by default not advertising + * full duplex 100baseT capabilities, fix this. + */ + spin_lock_irq(&hp->happy_lock); + happy_meal_set_initial_advertisement(hp); + spin_unlock_irq(&hp->happy_lock); + + err = devm_register_netdev(hp->dma_dev, dev); + if (err) + dev_err(hp->dma_dev, "Cannot register net device, aborting.\n"); + return err; +} + #ifdef CONFIG_SBUS static int happy_meal_sbus_probe_one(struct platform_device *op, int is_qfe) { @@ -2511,70 +2576,18 @@ static int happy_meal_sbus_probe_one(struct platform_device *op, int is_qfe) goto err_out_clear_quattro; } - hp->hm_revision = of_getintprop_default(dp, "hm-rev", 0xff); - if (hp->hm_revision == 0xff) - hp->hm_revision = 0xa0; - - /* Now enable the feature flags we can. */ - if (hp->hm_revision == 0x20 || hp->hm_revision == 0x21) - hp->happy_flags = HFLAG_20_21; - else if (hp->hm_revision != 0xa0) - hp->happy_flags = HFLAG_NOT_A0; - if (qp != NULL) hp->happy_flags |= HFLAG_QUATTRO; + hp->irq = op->archdata.irqs[0]; + /* Get the supported DVMA burst sizes from our Happy SBUS. */ hp->happy_bursts = of_getintprop_default(sbus_dp, "burst-sizes", 0x00); - hp->happy_block = dmam_alloc_coherent(&op->dev, PAGE_SIZE, - &hp->hblock_dvma, GFP_KERNEL); - if (!hp->happy_block) { - err = -ENOMEM; + err = happy_meal_common_probe(hp, dp, 0); + if (err) goto err_out_clear_quattro; - } - - /* Force check of the link first time we are brought up. */ - hp->linkcheck = 0; - - /* Force timer state to 'asleep' with count of zero. */ - hp->timer_state = asleep; - hp->timer_ticks = 0; - - timer_setup(&hp->happy_timer, happy_meal_timer, 0); - - dev->netdev_ops = &hme_netdev_ops; - dev->watchdog_timeo = 5*HZ; - dev->ethtool_ops = &hme_ethtool_ops; - - /* Happy Meal can do it all... */ - dev->hw_features = NETIF_F_SG | NETIF_F_HW_CSUM; - dev->features |= dev->hw_features | NETIF_F_RXCSUM; - - hp->irq = op->archdata.irqs[0]; - -#if defined(CONFIG_SBUS) && defined(CONFIG_PCI) - /* Hook up SBUS register/descriptor accessors. */ - hp->read_desc32 = sbus_hme_read_desc32; - hp->write_txd = sbus_hme_write_txd; - hp->write_rxd = sbus_hme_write_rxd; - hp->read32 = sbus_hme_read32; - hp->write32 = sbus_hme_write32; -#endif - - /* Grrr, Happy Meal comes up by default not advertising - * full duplex 100baseT capabilities, fix this. - */ - spin_lock_irq(&hp->happy_lock); - happy_meal_set_initial_advertisement(hp); - spin_unlock_irq(&hp->happy_lock); - - err = devm_register_netdev(&op->dev, dev); - if (err) { - dev_err(&op->dev, "Cannot register net device, aborting.\n"); - goto err_out_clear_quattro; - } platform_set_drvdata(op, hp); @@ -2689,21 +2702,6 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, hp->bigmacregs = (hpreg_base + 0x6000UL); hp->tcvregs = (hpreg_base + 0x7000UL); -#ifdef CONFIG_SPARC - hp->hm_revision = of_getintprop_default(dp, "hm-rev", 0xff); - if (hp->hm_revision == 0xff) - hp->hm_revision = 0xc0 | (pdev->revision & 0x0f); -#else - /* works with this on non-sparc hosts */ - hp->hm_revision = 0x20; -#endif - - /* Now enable the feature flags we can. */ - if (hp->hm_revision == 0x20 || hp->hm_revision == 0x21) - hp->happy_flags = HFLAG_20_21; - else if (hp->hm_revision != 0xa0 && hp->hm_revision != 0xc0) - hp->happy_flags = HFLAG_NOT_A0; - if (qp != NULL) hp->happy_flags |= HFLAG_QUATTRO; @@ -2714,50 +2712,11 @@ static int happy_meal_pci_probe(struct pci_dev *pdev, /* Assume PCI happy meals can handle all burst sizes. */ hp->happy_bursts = DMA_BURSTBITS; #endif - - hp->happy_block = dmam_alloc_coherent(&pdev->dev, PAGE_SIZE, - &hp->hblock_dvma, GFP_KERNEL); - if (!hp->happy_block) { - err = -ENOMEM; - goto err_out_clear_quattro; - } - - hp->linkcheck = 0; - hp->timer_state = asleep; - hp->timer_ticks = 0; - - timer_setup(&hp->happy_timer, happy_meal_timer, 0); - hp->irq = pdev->irq; - dev->netdev_ops = &hme_netdev_ops; - dev->watchdog_timeo = 5*HZ; - dev->ethtool_ops = &hme_ethtool_ops; - /* Happy Meal can do it all... */ - dev->hw_features = NETIF_F_SG | NETIF_F_HW_CSUM; - dev->features |= dev->hw_features | NETIF_F_RXCSUM; - -#if defined(CONFIG_SBUS) && defined(CONFIG_PCI) - /* Hook up PCI register/descriptor accessors. */ - hp->read_desc32 = pci_hme_read_desc32; - hp->write_txd = pci_hme_write_txd; - hp->write_rxd = pci_hme_write_rxd; - hp->read32 = pci_hme_read32; - hp->write32 = pci_hme_write32; -#endif - - /* Grrr, Happy Meal comes up by default not advertising - * full duplex 100baseT capabilities, fix this. - */ - spin_lock_irq(&hp->happy_lock); - happy_meal_set_initial_advertisement(hp); - spin_unlock_irq(&hp->happy_lock); - - err = devm_register_netdev(&pdev->dev, dev); - if (err) { - dev_err(&pdev->dev, "Cannot register net device, aborting.\n"); + err = happy_meal_common_probe(hp, dp, pdev->revision & 0x0f); + if (err) goto err_out_clear_quattro; - } pci_set_drvdata(pdev, hp);
Most of the second half of the PCI/SBUS probe functions are the same. Consolidate them into a common function. Signed-off-by: Sean Anderson <seanga2@gmail.com> --- (no changes since v1) drivers/net/ethernet/sun/sunhme.c | 183 ++++++++++++------------------ 1 file changed, 71 insertions(+), 112 deletions(-)