| Message ID | 1462898601-5429-1-git-send-email-harvey.hunt@imgtec.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Tue, 10 May 2016 17:43:21 +0100 Harvey Hunt <harvey.hunt@imgtec.com> wrote: > For ethernet devices, net_device.name will be eth%d before > register_netdev() is called. Don't print the net_device name until > the format string is replaced. > > Cc: Robert Jarzmik <robert.jarzmik@free.fr> > Cc: Barry Song <Baohua.Song@csr.com> > Cc: Marcel Ziswiler <marcel@ziswiler.com> > Cc: netdev@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > > Signed-off-by: Harvey Hunt <harvey.hunt@imgtec.com> > --- > I've tested this patch on a board that has a DM9000, but haven't > tested the other two network devices. > > drivers/net/ethernet/davicom/dm9000.c | 3 +-- > drivers/net/ethernet/micrel/ks8695net.c | 3 +-- > drivers/net/ethernet/netx-eth.c | 3 +-- > 3 files changed, 3 insertions(+), 6 deletions(-) > > diff --git a/drivers/net/ethernet/davicom/dm9000.c b/drivers/net/ethernet/davicom/dm9000.c > index 48d9194..89dcaa1 100644 > --- a/drivers/net/ethernet/davicom/dm9000.c > +++ b/drivers/net/ethernet/davicom/dm9000.c > @@ -1686,8 +1686,7 @@ dm9000_probe(struct platform_device *pdev) > } > > if (!is_valid_ether_addr(ndev->dev_addr)) { > - dev_warn(db->dev, "%s: Invalid ethernet MAC address. Please " > - "set using ifconfig\n", ndev->name); > + dev_warn(db->dev, "Invalid ethernet MAC address. Please set using ifconfig\n"); ifconfig is deprecated, therefore please don't tell users to use it.
From: Harvey Hunt <harvey.hunt@imgtec.com> Date: Tue, 10 May 2016 17:43:21 +0100 > @@ -1686,8 +1686,7 @@ dm9000_probe(struct platform_device *pdev) > } > > if (!is_valid_ether_addr(ndev->dev_addr)) { > - dev_warn(db->dev, "%s: Invalid ethernet MAC address. Please " > - "set using ifconfig\n", ndev->name); > + dev_warn(db->dev, "Invalid ethernet MAC address. Please set using ifconfig\n"); > > eth_hw_addr_random(ndev); > mac_src = "random"; If we don't print the netdev name, it's harder for the user to see which adapter has the problem. Therefore, it is better if you save some boolean state into a local variable here, then print the warning right after register_netdev(). Likewise for the rest of your changes too.
Hi David, On 12/05/16 00:26, David Miller wrote: > From: Harvey Hunt <harvey.hunt@imgtec.com> > Date: Tue, 10 May 2016 17:43:21 +0100 > >> @@ -1686,8 +1686,7 @@ dm9000_probe(struct platform_device *pdev) >> } >> >> if (!is_valid_ether_addr(ndev->dev_addr)) { >> - dev_warn(db->dev, "%s: Invalid ethernet MAC address. Please " >> - "set using ifconfig\n", ndev->name); >> + dev_warn(db->dev, "Invalid ethernet MAC address. Please set using ifconfig\n"); >> >> eth_hw_addr_random(ndev); >> mac_src = "random"; > > If we don't print the netdev name, it's harder for the user to see which > adapter has the problem. > > Therefore, it is better if you save some boolean state into a local variable > here, then print the warning right after register_netdev(). > > Likewise for the rest of your changes too. > Okay, I'll do that for v2. Thanks, Harvey
Hi Stephen, On 10/05/16 19:31, Stephen Hemminger wrote: > On Tue, 10 May 2016 17:43:21 +0100 > Harvey Hunt <harvey.hunt@imgtec.com> wrote: > >> For ethernet devices, net_device.name will be eth%d before >> register_netdev() is called. Don't print the net_device name until >> the format string is replaced. >> >> Cc: Robert Jarzmik <robert.jarzmik@free.fr> >> Cc: Barry Song <Baohua.Song@csr.com> >> Cc: Marcel Ziswiler <marcel@ziswiler.com> >> Cc: netdev@vger.kernel.org >> Cc: linux-kernel@vger.kernel.org >> >> Signed-off-by: Harvey Hunt <harvey.hunt@imgtec.com> >> --- >> I've tested this patch on a board that has a DM9000, but haven't >> tested the other two network devices. >> >> drivers/net/ethernet/davicom/dm9000.c | 3 +-- >> drivers/net/ethernet/micrel/ks8695net.c | 3 +-- >> drivers/net/ethernet/netx-eth.c | 3 +-- >> 3 files changed, 3 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/net/ethernet/davicom/dm9000.c b/drivers/net/ethernet/davicom/dm9000.c >> index 48d9194..89dcaa1 100644 >> --- a/drivers/net/ethernet/davicom/dm9000.c >> +++ b/drivers/net/ethernet/davicom/dm9000.c >> @@ -1686,8 +1686,7 @@ dm9000_probe(struct platform_device *pdev) >> } >> >> if (!is_valid_ether_addr(ndev->dev_addr)) { >> - dev_warn(db->dev, "%s: Invalid ethernet MAC address. Please " >> - "set using ifconfig\n", ndev->name); >> + dev_warn(db->dev, "Invalid ethernet MAC address. Please set using ifconfig\n"); > > ifconfig is deprecated, therefore please don't tell users to use it. > I'll update it to print "Please set using ip". Thanks, Harvey
diff --git a/drivers/net/ethernet/davicom/dm9000.c b/drivers/net/ethernet/davicom/dm9000.c index 48d9194..89dcaa1 100644 --- a/drivers/net/ethernet/davicom/dm9000.c +++ b/drivers/net/ethernet/davicom/dm9000.c @@ -1686,8 +1686,7 @@ dm9000_probe(struct platform_device *pdev) } if (!is_valid_ether_addr(ndev->dev_addr)) { - dev_warn(db->dev, "%s: Invalid ethernet MAC address. Please " - "set using ifconfig\n", ndev->name); + dev_warn(db->dev, "Invalid ethernet MAC address. Please set using ifconfig\n"); eth_hw_addr_random(ndev); mac_src = "random"; diff --git a/drivers/net/ethernet/micrel/ks8695net.c b/drivers/net/ethernet/micrel/ks8695net.c index a8522d8..2e2ea61 100644 --- a/drivers/net/ethernet/micrel/ks8695net.c +++ b/drivers/net/ethernet/micrel/ks8695net.c @@ -1456,8 +1456,7 @@ ks8695_probe(struct platform_device *pdev) ndev->dev_addr[5] = maclow & 0xFF; if (!is_valid_ether_addr(ndev->dev_addr)) - dev_warn(ksp->dev, "%s: Invalid ethernet MAC address. Please " - "set using ifconfig\n", ndev->name); + dev_warn(ksp->dev, "Invalid ethernet MAC address. Please set using ifconfig\n"); /* In order to be efficient memory-wise, we allocate both * rings in one go. diff --git a/drivers/net/ethernet/netx-eth.c b/drivers/net/ethernet/netx-eth.c index 9fbc302..c2e2151 100644 --- a/drivers/net/ethernet/netx-eth.c +++ b/drivers/net/ethernet/netx-eth.c @@ -358,8 +358,7 @@ static int netx_eth_enable(struct net_device *ndev) xc_start(priv->xc); if (!is_valid_ether_addr(ndev->dev_addr)) - printk("%s: Invalid ethernet MAC address. Please " - "set using ifconfig\n", ndev->name); + printk("Invalid ethernet MAC address. Please set using ifconfig\n"); for (i=2; i<=18; i++) pfifo_push(EMPTY_PTR_FIFO(priv->id),
For ethernet devices, net_device.name will be eth%d before register_netdev() is called. Don't print the net_device name until the format string is replaced. Cc: Robert Jarzmik <robert.jarzmik@free.fr> Cc: Barry Song <Baohua.Song@csr.com> Cc: Marcel Ziswiler <marcel@ziswiler.com> Cc: netdev@vger.kernel.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Harvey Hunt <harvey.hunt@imgtec.com> --- I've tested this patch on a board that has a DM9000, but haven't tested the other two network devices. drivers/net/ethernet/davicom/dm9000.c | 3 +-- drivers/net/ethernet/micrel/ks8695net.c | 3 +-- drivers/net/ethernet/netx-eth.c | 3 +-- 3 files changed, 3 insertions(+), 6 deletions(-)