| Message ID | 20200209003748.882-1-xypron.glpk@gmx.de |
|---|---|
| State | New |
| Headers | show |
| Series | [1/1] net: designware: speed should be in a debug message | expand |
On Sun, Feb 9, 2020 at 8:38 AM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > > The network connection speed is a debug information. So we should use > debug() and not printf(). > > Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> > --- > drivers/net/designware.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/designware.c b/drivers/net/designware.c > index 19fc34f771..a89f4bedf1 100644 > --- a/drivers/net/designware.c > +++ b/drivers/net/designware.c > @@ -255,9 +255,9 @@ static int dw_adjust_link(struct dw_eth_dev *priv, struct eth_mac_regs *mac_p, > > writel(conf, &mac_p->conf); > > - printf("Speed: %d, %s duplex%s\n", phydev->speed, > - (phydev->duplex) ? "full" : "half", > - (phydev->port == PORT_FIBRE) ? ", fiber mode" : ""); > + debug("Speed: %d, %s duplex%s\n", phydev->speed, > + (phydev->duplex) ? "full" : "half", > + (phydev->port == PORT_FIBRE) ? ", fiber mode" : ""); Maybe this was intentional as I see such in the MACB driver as well. Leaving this to Joe and original author of this driver to comment. Regards, Bin
On 2/9/20 3:59 AM, Bin Meng wrote: > On Sun, Feb 9, 2020 at 8:38 AM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: >> >> The network connection speed is a debug information. So we should use >> debug() and not printf(). >> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> >> --- >> drivers/net/designware.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/net/designware.c b/drivers/net/designware.c >> index 19fc34f771..a89f4bedf1 100644 >> --- a/drivers/net/designware.c >> +++ b/drivers/net/designware.c >> @@ -255,9 +255,9 @@ static int dw_adjust_link(struct dw_eth_dev *priv, struct eth_mac_regs *mac_p, >> >> writel(conf, &mac_p->conf); >> >> - printf("Speed: %d, %s duplex%s\n", phydev->speed, >> - (phydev->duplex) ? "full" : "half", >> - (phydev->port == PORT_FIBRE) ? ", fiber mode" : ""); >> + debug("Speed: %d, %s duplex%s\n", phydev->speed, >> + (phydev->duplex) ? "full" : "half", >> + (phydev->port == PORT_FIBRE) ? ", fiber mode" : ""); > > Maybe this was intentional as I see such in the MACB driver as well. > Leaving this to Joe and original author of this driver to comment. Thanks for reviewing. In my syslog driver https://lists.denx.de/pipermail/u-boot/2020-February/399551.html I got this message everytime that I initialized the network. Best regards Heinrich
Hi Heinrich, On Sun, Feb 9, 2020 at 11:58 AM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > > On 2/9/20 3:59 AM, Bin Meng wrote: > > On Sun, Feb 9, 2020 at 8:38 AM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: > >> > >> The network connection speed is a debug information. So we should use > >> debug() and not printf(). > >> > >> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> > >> --- > >> drivers/net/designware.c | 6 +++--- > >> 1 file changed, 3 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/net/designware.c b/drivers/net/designware.c > >> index 19fc34f771..a89f4bedf1 100644 > >> --- a/drivers/net/designware.c > >> +++ b/drivers/net/designware.c > >> @@ -255,9 +255,9 @@ static int dw_adjust_link(struct dw_eth_dev *priv, struct eth_mac_regs *mac_p, > >> > >> writel(conf, &mac_p->conf); > >> > >> - printf("Speed: %d, %s duplex%s\n", phydev->speed, > >> - (phydev->duplex) ? "full" : "half", > >> - (phydev->port == PORT_FIBRE) ? ", fiber mode" : ""); > >> + debug("Speed: %d, %s duplex%s\n", phydev->speed, > >> + (phydev->duplex) ? "full" : "half", > >> + (phydev->port == PORT_FIBRE) ? ", fiber mode" : ""); > > > > Maybe this was intentional as I see such in the MACB driver as well. > > Leaving this to Joe and original author of this driver to comment. > > Thanks for reviewing. > > In my syslog driver > https://lists.denx.de/pipermail/u-boot/2020-February/399551.html > I got this message everytime that I initialized the network. Yeah, so we probably need set up a rule for network drivers? Regards, Bin
On 2/10/20 3:41 AM, Bin Meng wrote: > Hi Heinrich, > > On Sun, Feb 9, 2020 at 11:58 AM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: >> >> On 2/9/20 3:59 AM, Bin Meng wrote: >>> On Sun, Feb 9, 2020 at 8:38 AM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote: >>>> >>>> The network connection speed is a debug information. So we should use >>>> debug() and not printf(). >>>> >>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> >>>> --- >>>> drivers/net/designware.c | 6 +++--- >>>> 1 file changed, 3 insertions(+), 3 deletions(-) >>>> >>>> diff --git a/drivers/net/designware.c b/drivers/net/designware.c >>>> index 19fc34f771..a89f4bedf1 100644 >>>> --- a/drivers/net/designware.c >>>> +++ b/drivers/net/designware.c >>>> @@ -255,9 +255,9 @@ static int dw_adjust_link(struct dw_eth_dev *priv, struct eth_mac_regs *mac_p, >>>> >>>> writel(conf, &mac_p->conf); >>>> >>>> - printf("Speed: %d, %s duplex%s\n", phydev->speed, >>>> - (phydev->duplex) ? "full" : "half", >>>> - (phydev->port == PORT_FIBRE) ? ", fiber mode" : ""); >>>> + debug("Speed: %d, %s duplex%s\n", phydev->speed, >>>> + (phydev->duplex) ? "full" : "half", >>>> + (phydev->port == PORT_FIBRE) ? ", fiber mode" : ""); >>> >>> Maybe this was intentional as I see such in the MACB driver as well. >>> Leaving this to Joe and original author of this driver to comment. >> >> Thanks for reviewing. >> >> In my syslog driver >> https://lists.denx.de/pipermail/u-boot/2020-February/399551.html >> I got this message everytime that I initialized the network. > > Yeah, so we probably need set up a rule for network drivers? I personally prefer drivers only to report errors. I have proposed a patch to make log_*() functions fall back to printf() or debug() for CONFIG_LOG_n. [PATCH 1/1] log: output for CONFIG_LOG=n https://lists.denx.de/pipermail/u-boot/2020-February/399593.html So after that patch prossibly log_info() or log_debug() would be the most adequate choice because it leaves determining the noisiness to the user. Best regards Heinrich
diff --git a/drivers/net/designware.c b/drivers/net/designware.c index 19fc34f771..a89f4bedf1 100644 --- a/drivers/net/designware.c +++ b/drivers/net/designware.c @@ -255,9 +255,9 @@ static int dw_adjust_link(struct dw_eth_dev *priv, struct eth_mac_regs *mac_p, writel(conf, &mac_p->conf); - printf("Speed: %d, %s duplex%s\n", phydev->speed, - (phydev->duplex) ? "full" : "half", - (phydev->port == PORT_FIBRE) ? ", fiber mode" : ""); + debug("Speed: %d, %s duplex%s\n", phydev->speed, + (phydev->duplex) ? "full" : "half", + (phydev->port == PORT_FIBRE) ? ", fiber mode" : ""); return 0; }
The network connection speed is a debug information. So we should use debug() and not printf(). Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de> --- drivers/net/designware.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) -- 2.20.1