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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 18 Mar 2023 10:40:46 -0400 From: Sean Anderson <seanga2@...il.com> To: Simon Horman <simon.horman@...igine.com> Cc: "David S . Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH net-next v3 9/9] net: sunhme: Consolidate common probe tasks 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@...il.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
Powered by blists - more mailing lists