Message ID | 20200824170904.60832-1-andriy.shevchenko@linux.intel.com |
---|---|
State | Accepted |
Delegated to: | David Miller |
Headers | show |
Series | [v1] net: phy: leds: Deduplicate link LED trigger registration | expand |
On Mon, Aug 24, 2020 at 08:09:04PM +0300, Andy Shevchenko wrote: > Refactor phy_led_trigger_register() and deduplicate its functionality > when registering LED trigger for link. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Hi Andy Please take a look at https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html Andrew
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Date: Mon, 24 Aug 2020 20:09:04 +0300 > Refactor phy_led_trigger_register() and deduplicate its functionality > when registering LED trigger for link. > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> This doesn't compile: CC [M] drivers/net/phy/phy_led_triggers.o drivers/net/phy/phy_led_triggers.c: In function ‘phy_led_triggers_register’: drivers/net/phy/phy_led_triggers.c:102:38: error: passing argument 2 of ‘phy_led_trigger_register’ from incompatible pointer type [-Werror=incompatible-pointer-types] 102 | err = phy_led_trigger_register(phy, &phy->led_link_trigger, 0, "link"); | ^~~~~~~~~~~~~~~~~~~~~~ | | | struct phy_led_trigger ** drivers/net/phy/phy_led_triggers.c:68:33: note: expected ‘struct phy_led_trigger *’ but argument is of type ‘struct phy_led_trigger **’ 68 | struct phy_led_trigger *plt, | ~~~~~~~~~~~~~~~~~~~~~~~~^~~
Hi Andy, I love your patch! Yet something to improve: [auto build test ERROR on linus/master] [also build test ERROR on v5.9-rc2 next-20200825] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Andy-Shevchenko/net-phy-leds-Deduplicate-link-LED-trigger-registration/20200825-011159 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git d012a7190fc1fd72ed48911e77ca97ba4521bccd config: x86_64-randconfig-a003-20200826 (attached as .config) compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 77e5a195f818b9ace91f7b12ab948b21d7918238) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): >> drivers/net/phy/phy_led_triggers.c:102:38: error: incompatible pointer types passing 'struct phy_led_trigger **' to parameter of type 'struct phy_led_trigger *'; remove & [-Werror,-Wincompatible-pointer-types] err = phy_led_trigger_register(phy, &phy->led_link_trigger, 0, "link"); ^~~~~~~~~~~~~~~~~~~~~~ drivers/net/phy/phy_led_triggers.c:68:33: note: passing argument to parameter 'plt' here struct phy_led_trigger *plt, ^ 1 error generated. # https://github.com/0day-ci/linux/commit/13a3d33899e0fe9212a1625d7c9daff466272f28 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Andy-Shevchenko/net-phy-leds-Deduplicate-link-LED-trigger-registration/20200825-011159 git checkout 13a3d33899e0fe9212a1625d7c9daff466272f28 vim +102 drivers/net/phy/phy_led_triggers.c 83 84 int phy_led_triggers_register(struct phy_device *phy) 85 { 86 int i, err; 87 unsigned int speeds[50]; 88 89 phy->phy_num_led_triggers = phy_supported_speeds(phy, speeds, 90 ARRAY_SIZE(speeds)); 91 if (!phy->phy_num_led_triggers) 92 return 0; 93 94 phy->led_link_trigger = devm_kzalloc(&phy->mdio.dev, 95 sizeof(*phy->led_link_trigger), 96 GFP_KERNEL); 97 if (!phy->led_link_trigger) { 98 err = -ENOMEM; 99 goto out_clear; 100 } 101 > 102 err = phy_led_trigger_register(phy, &phy->led_link_trigger, 0, "link"); 103 if (err) 104 goto out_free_link; 105 106 phy->phy_led_triggers = devm_kcalloc(&phy->mdio.dev, 107 phy->phy_num_led_triggers, 108 sizeof(struct phy_led_trigger), 109 GFP_KERNEL); 110 if (!phy->phy_led_triggers) { 111 err = -ENOMEM; 112 goto out_unreg_link; 113 } 114 115 for (i = 0; i < phy->phy_num_led_triggers; i++) { 116 err = phy_led_trigger_register(phy, &phy->phy_led_triggers[i], 117 speeds[i], phy_speed_to_str(speeds[i])); 118 if (err) 119 goto out_unreg; 120 } 121 122 phy->last_triggered = NULL; 123 phy_led_trigger_change_speed(phy); 124 125 return 0; 126 out_unreg: 127 while (i--) 128 phy_led_trigger_unregister(&phy->phy_led_triggers[i]); 129 devm_kfree(&phy->mdio.dev, phy->phy_led_triggers); 130 out_unreg_link: 131 phy_led_trigger_unregister(phy->led_link_trigger); 132 out_free_link: 133 devm_kfree(&phy->mdio.dev, phy->led_link_trigger); 134 phy->led_link_trigger = NULL; 135 out_clear: 136 phy->phy_num_led_triggers = 0; 137 return err; 138 } 139 EXPORT_SYMBOL_GPL(phy_led_triggers_register); 140 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Tue, Aug 25, 2020 at 07:40:15AM -0700, David Miller wrote: > From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > Date: Mon, 24 Aug 2020 20:09:04 +0300 > > > Refactor phy_led_trigger_register() and deduplicate its functionality > > when registering LED trigger for link. > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> My bad... Missed LED_CLASS and saw even something has been built in drivers/net/ but apparently wasn't enough to get this one compiled. v2 will fix that. Thanks! > This doesn't compile: > > CC [M] drivers/net/phy/phy_led_triggers.o > drivers/net/phy/phy_led_triggers.c: In function ‘phy_led_triggers_register’: > drivers/net/phy/phy_led_triggers.c:102:38: error: passing argument 2 of ‘phy_led_trigger_register’ from incompatible pointer type [-Werror=incompatible-pointer-types] > 102 | err = phy_led_trigger_register(phy, &phy->led_link_trigger, 0, "link"); > | ^~~~~~~~~~~~~~~~~~~~~~ > | | > | struct phy_led_trigger ** > drivers/net/phy/phy_led_triggers.c:68:33: note: expected ‘struct phy_led_trigger *’ but argument is of type ‘struct phy_led_trigger **’ > 68 | struct phy_led_trigger *plt, > | ~~~~~~~~~~~~~~~~~~~~~~~~^~~ >
On Mon, Aug 24, 2020 at 07:45:58PM +0200, Andrew Lunn wrote: > On Mon, Aug 24, 2020 at 08:09:04PM +0300, Andy Shevchenko wrote: > > Refactor phy_led_trigger_register() and deduplicate its functionality > > when registering LED trigger for link. > > > > Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > > Reviewed-by: Andrew Lunn <andrew@lunn.ch> Thanks! > Hi Andy > > Please take a look at https://www.kernel.org/doc/html/latest/networking/netdev-FAQ.html Anything particular? I suspect you want me to put net-next prefix...
diff --git a/drivers/net/phy/phy_led_triggers.c b/drivers/net/phy/phy_led_triggers.c index 59a94e07e7c5..9a92e471400e 100644 --- a/drivers/net/phy/phy_led_triggers.c +++ b/drivers/net/phy/phy_led_triggers.c @@ -66,11 +66,11 @@ static void phy_led_trigger_format_name(struct phy_device *phy, char *buf, static int phy_led_trigger_register(struct phy_device *phy, struct phy_led_trigger *plt, - unsigned int speed) + unsigned int speed, + const char *suffix) { plt->speed = speed; - phy_led_trigger_format_name(phy, plt->name, sizeof(plt->name), - phy_speed_to_str(speed)); + phy_led_trigger_format_name(phy, plt->name, sizeof(plt->name), suffix); plt->trigger.name = plt->name; return led_trigger_register(&plt->trigger); @@ -99,12 +99,7 @@ int phy_led_triggers_register(struct phy_device *phy) goto out_clear; } - phy_led_trigger_format_name(phy, phy->led_link_trigger->name, - sizeof(phy->led_link_trigger->name), - "link"); - phy->led_link_trigger->trigger.name = phy->led_link_trigger->name; - - err = led_trigger_register(&phy->led_link_trigger->trigger); + err = phy_led_trigger_register(phy, &phy->led_link_trigger, 0, "link"); if (err) goto out_free_link; @@ -119,7 +114,7 @@ int phy_led_triggers_register(struct phy_device *phy) for (i = 0; i < phy->phy_num_led_triggers; i++) { err = phy_led_trigger_register(phy, &phy->phy_led_triggers[i], - speeds[i]); + speeds[i], phy_speed_to_str(speeds[i])); if (err) goto out_unreg; }
Refactor phy_led_trigger_register() and deduplicate its functionality when registering LED trigger for link. Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- drivers/net/phy/phy_led_triggers.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-)