| Message ID | 1508777005-20938-1-git-send-email-steven.hill@cavium.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
| Series | ethernet: cavium: octeon: Switch to using netdev_info(). | expand |
From: "Steven J. Hill" <steven.hill@cavium.com> Date: Mon, 23 Oct 2017 11:43:25 -0500 > @@ -705,14 +705,15 @@ static int octeon_mgmt_ioctl_hwtstamp(struct net_device *netdev, > u64 clock_comp = (NSEC_PER_SEC << 32) / octeon_get_io_clock_rate(); > if (!ptp.s.ptp_en) > cvmx_write_csr(CVMX_MIO_PTP_CLOCK_COMP, clock_comp); > - pr_info("PTP Clock: Using sclk reference at %lld Hz\n", > + netdev_info(netdev, > + "PTP Clock: Using sclk reference at %lld Hz\n", Please get the argument alignment correct when you make changes like this. For a function call, the arguments on the second and subsequent line of arguments must start exactly at the first column after the openning parenthesis of the call. You must use the appropriate number of TAB then SPACE characters necessary to achieve this. Thank you.
Hi Steven,
[auto build test WARNING on net-next/master]
[also build test WARNING on v4.14-rc6]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Steven-J-Hill/ethernet-cavium-octeon-Switch-to-using-netdev_info/20171024-071910
config: mips-cavium_octeon_defconfig (attached as .config)
compiler: mips64-linux-gnuabi64-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=mips
All warnings (new ones prefixed by >>):
drivers/net/ethernet/cavium/octeon/octeon_mgmt.c: In function 'octeon_mgmt_adjust_link':
>> drivers/net/ethernet/cavium/octeon/octeon_mgmt.c:929:5: warning: suggest explicit braces to avoid ambiguous 'else' [-Wparentheses]
if (link_changed != 0)
^
vim +/else +929 drivers/net/ethernet/cavium/octeon/octeon_mgmt.c
896
897 static void octeon_mgmt_adjust_link(struct net_device *netdev)
898 {
899 struct octeon_mgmt *p = netdev_priv(netdev);
900 struct phy_device *phydev = netdev->phydev;
901 unsigned long flags;
902 int link_changed = 0;
903
904 if (!phydev)
905 return;
906
907 spin_lock_irqsave(&p->lock, flags);
908
909
910 if (!phydev->link && p->last_link)
911 link_changed = -1;
912
913 if (phydev->link &&
914 (p->last_duplex != phydev->duplex ||
915 p->last_link != phydev->link ||
916 p->last_speed != phydev->speed)) {
917 octeon_mgmt_disable_link(p);
918 link_changed = 1;
919 octeon_mgmt_update_link(p);
920 octeon_mgmt_enable_link(p);
921 }
922
923 p->last_link = phydev->link;
924 p->last_speed = phydev->speed;
925 p->last_duplex = phydev->duplex;
926
927 spin_unlock_irqrestore(&p->lock, flags);
928
> 929 if (link_changed != 0)
930 if (link_changed > 0)
931 netdev_info(netdev, "Link is up - %d/%s\n",
932 phydev->speed, phydev->duplex == DUPLEX_FULL ? "Full" : "Half");
933 else
934 netdev_info(netdev, "Link is down\n");
935 }
936
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Wed, 2017-10-25 at 14:41 +0800, kbuild test robot wrote: > Hi Steven, > > [auto build test WARNING on net-next/master] > [also build test WARNING on v4.14-rc6] > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] > > url: https://github.com/0day-ci/linux/commits/Steven-J-Hill/ethernet-cavium-octeon-Switch-to-using-netdev_info/20171024-071910 > config: mips-cavium_octeon_defconfig (attached as .config) > compiler: mips64-linux-gnuabi64-gcc (Debian 6.1.1-9) 6.1.1 20160705 > reproduce: > wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > make.cross ARCH=mips > > All warnings (new ones prefixed by >>): > > drivers/net/ethernet/cavium/octeon/octeon_mgmt.c: In function 'octeon_mgmt_adjust_link': > > > drivers/net/ethernet/cavium/octeon/octeon_mgmt.c:929:5: warning: suggest explicit braces to avoid ambiguous 'else' [-Wparentheses] > > if (link_changed != 0) > ^ > > vim +/else +929 drivers/net/ethernet/cavium/octeon/octeon_mgmt.c > > 896 > 897 static void octeon_mgmt_adjust_link(struct net_device *netdev) > 898 { > 899 struct octeon_mgmt *p = netdev_priv(netdev); > 900 struct phy_device *phydev = netdev->phydev; > 901 unsigned long flags; > 902 int link_changed = 0; > 903 > 904 if (!phydev) > 905 return; > 906 > 907 spin_lock_irqsave(&p->lock, flags); > 908 > 909 > 910 if (!phydev->link && p->last_link) > 911 link_changed = -1; > 912 > 913 if (phydev->link && > 914 (p->last_duplex != phydev->duplex || > 915 p->last_link != phydev->link || > 916 p->last_speed != phydev->speed)) { > 917 octeon_mgmt_disable_link(p); > 918 link_changed = 1; > 919 octeon_mgmt_update_link(p); > 920 octeon_mgmt_enable_link(p); > 921 } > 922 > 923 p->last_link = phydev->link; > 924 p->last_speed = phydev->speed; > 925 p->last_duplex = phydev->duplex; > 926 > 927 spin_unlock_irqrestore(&p->lock, flags); > 928 > > 929 if (link_changed != 0) > 930 if (link_changed > 0) > 931 netdev_info(netdev, "Link is up - %d/%s\n", > 932 phydev->speed, phydev->duplex == DUPLEX_FULL ? "Full" : "Half"); > 933 else > 934 netdev_info(netdev, "Link is down\n"); > 935 } > 936 I think this would be better as if (!phydev_link) { if (p->last_link) link_changed = -1; } else if (p->last_duplex != phydev->duplex || p->last_link != phydev->link || p->last_speed != phydev->speed) { link_changed = 1; octeon_mgnt_disable_link(p); octeon_mgnt_update_link(p); octeon_mgnt_enable_link(p); } ... if (link_changed > 0) netdev_info(netdev, "Link is up - %d/%s\n", phydev->speed, phydev->duplex == DUPLEX_FULL ? "Full" : "Half"); else if (link_changed < 0) netdev_info(netdev, "Link is down\n");
diff --git a/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c b/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c index 2887bca..3bb8fbd 100644 --- a/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c +++ b/drivers/net/ethernet/cavium/octeon/octeon_mgmt.c @@ -705,14 +705,15 @@ static int octeon_mgmt_ioctl_hwtstamp(struct net_device *netdev, u64 clock_comp = (NSEC_PER_SEC << 32) / octeon_get_io_clock_rate(); if (!ptp.s.ptp_en) cvmx_write_csr(CVMX_MIO_PTP_CLOCK_COMP, clock_comp); - pr_info("PTP Clock: Using sclk reference at %lld Hz\n", + netdev_info(netdev, + "PTP Clock: Using sclk reference at %lld Hz\n", (NSEC_PER_SEC << 32) / clock_comp); } else { /* The clock is already programmed to use a GPIO */ u64 clock_comp = cvmx_read_csr(CVMX_MIO_PTP_CLOCK_COMP); - pr_info("PTP Clock: Using GPIO %d at %lld Hz\n", - ptp.s.ext_clk_in, - (NSEC_PER_SEC << 32) / clock_comp); + netdev_info(netdev, + "PTP Clock: Using GPIO %d at %lld Hz\n", + ptp.s.ext_clk_in, (NSEC_PER_SEC << 32) / clock_comp); } /* Enable the clock if it wasn't done already */ @@ -925,16 +926,12 @@ static void octeon_mgmt_adjust_link(struct net_device *netdev) spin_unlock_irqrestore(&p->lock, flags); - if (link_changed != 0) { - if (link_changed > 0) { - pr_info("%s: Link is up - %d/%s\n", netdev->name, - phydev->speed, - phydev->duplex == DUPLEX_FULL ? - "Full" : "Half"); - } else { - pr_info("%s: Link is down\n", netdev->name); - } - } + if (link_changed != 0) + if (link_changed > 0) + netdev_info(netdev, "Link is up - %d/%s\n", + phydev->speed, phydev->duplex == DUPLEX_FULL ? "Full" : "Half"); + else + netdev_info(netdev, "Link is down\n"); } static int octeon_mgmt_init_phy(struct net_device *netdev)