| Message ID | 20230106161759.66019-1-jonathanh@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | usb: gadget: u_ether: Don't warn in gether_setup_name_default() | expand |
Hi, Hasn't there been a similar patch already? W dniu 6.01.2023 o 17:17, Jon Hunter pisze: > The function gether_setup_name_default() is called by various USB > ethernet gadget drivers. This function always generates the MAC address > for the ethernet gadget device and always prints a warning when > generating the MAC address. Given that these messages are expected, make > these prints informational instead of warnings. > > Signed-off-by: Jon Hunter <jonathanh@nvidia.com> > --- > 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 8f12f3f8f6ee..c19d66cd1446 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 %s ethernet address\n", "self"); > + pr_info("using random %s ethernet address\n", "self"); As far as I can tell this function is called by all Ethernet gadgets, and using random Ethernet addresses is the default behavior for all of them, including legacy gadgets. Why to inform about the default situation happening? So in fact maybe it would be better to eliminate the pr_warn() altogether, instead of replacing it with pr_info()? If the user does not care to explicitly set some non-default address(es), why would she care to know that a randomly selected address has been chosen? Note that learning _what_ specific address has been chosen is perfectly doable without these pr_info() calls. Regards, Andrzej > > /* 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 %s ethernet address\n", "host"); > + pr_info("using random %s ethernet address\n", "host"); > > net->netdev_ops = ð_netdev_ops; >
On 10/01/2023 16:31, Andrzej Pietrasiewicz wrote: > Hi, > > Hasn't there been a similar patch already? There could be, but I was not aware. Do you happen to have a link to it? > W dniu 6.01.2023 o 17:17, Jon Hunter pisze: >> The function gether_setup_name_default() is called by various USB >> ethernet gadget drivers. This function always generates the MAC address >> for the ethernet gadget device and always prints a warning when >> generating the MAC address. Given that these messages are expected, make >> these prints informational instead of warnings. >> >> Signed-off-by: Jon Hunter <jonathanh@nvidia.com> >> --- >> 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 8f12f3f8f6ee..c19d66cd1446 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 %s ethernet address\n", "self"); >> + pr_info("using random %s ethernet address\n", "self"); > > As far as I can tell this function is called by all Ethernet gadgets, > and using random Ethernet addresses is the default behavior for all of > them, > including legacy gadgets. Why to inform about the default situation > happening? > So in fact maybe it would be better to eliminate the pr_warn() altogether, > instead of replacing it with pr_info()? If the user does not care to > explicitly set some non-default address(es), why would she care to know > that a randomly selected address has been chosen? Note that learning > _what_ specific address has been chosen is perfectly doable without > these pr_info() calls. That would be fine with me. This print has been there for a long time and so I figured people wanted some sort of message. I would be happy to remove. Jon
Hi, W dniu 11.01.2023 o 11:35, Jon Hunter pisze: > > On 10/01/2023 16:31, Andrzej Pietrasiewicz wrote: >> Hi, >> >> Hasn't there been a similar patch already? > > There could be, but I was not aware. Do you happen to have a link to it? Nope, I only rely on my memory, which can obviously be faulty. > >> W dniu 6.01.2023 o 17:17, Jon Hunter pisze: >>> The function gether_setup_name_default() is called by various USB >>> ethernet gadget drivers. This function always generates the MAC address >>> for the ethernet gadget device and always prints a warning when >>> generating the MAC address. Given that these messages are expected, make >>> these prints informational instead of warnings. >>> >>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com> >>> --- >>> 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 8f12f3f8f6ee..c19d66cd1446 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 %s ethernet address\n", "self"); >>> + pr_info("using random %s ethernet address\n", "self"); >> >> As far as I can tell this function is called by all Ethernet gadgets, >> and using random Ethernet addresses is the default behavior for all of them, >> including legacy gadgets. Why to inform about the default situation happening? >> So in fact maybe it would be better to eliminate the pr_warn() altogether, >> instead of replacing it with pr_info()? If the user does not care to >> explicitly set some non-default address(es), why would she care to know >> that a randomly selected address has been chosen? Note that learning >> _what_ specific address has been chosen is perfectly doable without >> these pr_info() calls. > > > That would be fine with me. This print has been there for a long time and so I > figured people wanted some sort of message. I would be happy to remove. It has. Maybe it's been a mistake? A properly working driver should be silent. In case of Ethernet gadgets using random MAC addresses is a proper (and default) behavior. I think it's worth trying to just remove pr_warn(). Or, at least, change to pr_debug(), because these messages look more like debugging messages no regular user should be interested in. There's also the maintainer who can have his say. @Greg? As a side note, there's quite a lot of cleanup needed in gadgets' messaging. Regards, Andrzej > > Jon >
diff --git a/drivers/usb/gadget/function/u_ether.c b/drivers/usb/gadget/function/u_ether.c index 8f12f3f8f6ee..c19d66cd1446 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 %s ethernet address\n", "self"); + pr_info("using random %s ethernet address\n", "self"); /* 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 %s ethernet address\n", "host"); + pr_info("using random %s ethernet address\n", "host"); net->netdev_ops = ð_netdev_ops;
The function gether_setup_name_default() is called by various USB ethernet gadget drivers. This function always generates the MAC address for the ethernet gadget device and always prints a warning when generating the MAC address. Given that these messages are expected, make these prints informational instead of warnings. Signed-off-by: Jon Hunter <jonathanh@nvidia.com> --- drivers/usb/gadget/function/u_ether.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)