| Message ID | 20230119102547.26698-2-jonathanh@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [V3,1/2] usb: gadget: u_ether: Improve print in gether_setup_name_default() | expand |
On Thu, Jan 19, 2023 at 10:25:47AM +0000, Jon Hunter wrote: > The function gether_setup_name_default() is called by various USB > ethernet gadget drivers. Calling this function will select a random > host and device MAC addresses. A properly working driver should be > silent, so don't warn the user about default MAC addresses selection > happening and convert the warnings into debug messages. > > Signed-off-by: Jon Hunter <jonathanh@nvidia.com> > --- > V2 -> V3: Updated commit message. > V1 -> V2: Changed print to debug instead of info. > > drivers/usb/gadget/function/u_ether.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c > index be8e7b448933..8feb40e38137 100644 > --- a/drivers/usb/gadget/function/u_ether.c > +++ b/drivers/usb/gadget/function/u_ether.c > @@ -845,13 +845,13 @@ struct net_device *gether_setup_name_default(const char *netname) > snprintf(net->name, sizeof(net->name), "%s%%d", netname); > > eth_random_addr(dev->dev_mac); > - pr_warn("using random self ethernet address %pM\n", dev->dev_mac); > + pr_debug("using random self ethernet address %pM\n", dev->dev_mac); > > /* by default we always have a random MAC address */ > net->addr_assign_type = NET_ADDR_RANDOM; > > eth_random_addr(dev->host_mac); > - pr_warn("using random host ethernet address %pM\n", dev->host_mac); > + pr_debug("using random host ethernet address %pM\n", dev->host_mac); In looking at this further, you are right, the structures are not initialized properly yet. So move these to a different function entirely, they should be in gether_register_netdev() instead, after the device is registered with the system. thanks, greg k-h
On 19/01/2023 10:46, Greg Kroah-Hartman wrote: ... > In looking at this further, you are right, the structures are not > initialized properly yet. > > So move these to a different function entirely, they should be in > gether_register_netdev() instead, after the device is registered with > the system. I wonder if this is even necessary. Looking at gether_register_netdev() it already has ... status = register_netdev(net); if (status < 0) { dev_dbg(&g->dev, "register_netdev failed, %d\n", status); return status; } else { INFO(dev, "HOST MAC %pM\n", dev->host_mac); INFO(dev, "MAC %pM\n", dev->dev_mac); ... } Any objection to just removing completely from gether_setup_name_default()? If we move to gether_register_netdev() then I believe we also have to deal with gether_register_netdev(). Jon
On Thu, Jan 19, 2023 at 01:38:15PM +0000, Jon Hunter wrote: > > On 19/01/2023 10:46, Greg Kroah-Hartman wrote: > > ... > > > In looking at this further, you are right, the structures are not > > initialized properly yet. > > > > So move these to a different function entirely, they should be in > > gether_register_netdev() instead, after the device is registered with > > the system. > > > I wonder if this is even necessary. Looking at gether_register_netdev() it > already has ... > > status = register_netdev(net); > if (status < 0) { > dev_dbg(&g->dev, "register_netdev failed, %d\n", status); > return status; > } else { > INFO(dev, "HOST MAC %pM\n", dev->host_mac); > INFO(dev, "MAC %pM\n", dev->dev_mac); Those need to be moved to be dev_info() and then I'll stop complaining :) Or dev_dbg() if you want to make it debug only. > ... > } > > > Any objection to just removing completely from gether_setup_name_default()? Nope! Remove them and change the above lines and then all should be good. thanks, greg k-h
diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c index be8e7b448933..8feb40e38137 100644 --- a/drivers/usb/gadget/function/u_ether.c +++ b/drivers/usb/gadget/function/u_ether.c @@ -845,13 +845,13 @@ struct net_device *gether_setup_name_default(const char *netname) snprintf(net->name, sizeof(net->name), "%s%%d", netname); eth_random_addr(dev->dev_mac); - pr_warn("using random self ethernet address %pM\n", dev->dev_mac); + pr_debug("using random self ethernet address %pM\n", dev->dev_mac); /* by default we always have a random MAC address */ net->addr_assign_type = NET_ADDR_RANDOM; eth_random_addr(dev->host_mac); - pr_warn("using random host ethernet address %pM\n", dev->host_mac); + pr_debug("using random host ethernet address %pM\n", dev->host_mac); net->netdev_ops = ð_netdev_ops;
The function gether_setup_name_default() is called by various USB ethernet gadget drivers. Calling this function will select a random host and device MAC addresses. A properly working driver should be silent, so don't warn the user about default MAC addresses selection happening and convert the warnings into debug messages. Signed-off-by: Jon Hunter <jonathanh@nvidia.com> --- V2 -> V3: Updated commit message. V1 -> V2: Changed print to debug instead of info. drivers/usb/gadget/function/u_ether.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)