Re: [PATCH v3 1/2] leds: trigger: netdev: display only supported link speed attribute

[Christian Marangi]

On Sun, Dec 17, 2023 at 06:23:05AM +0800, kernel test robot wrote:
> Hi Christian,
> 
> kernel test robot noticed the following build errors:
> 
> [auto build test ERROR on lee-leds/for-leds-next]
> [cannot apply to net-next/main net/main linus/master v6.7-rc5 next-20231215]
> [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#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Christian-Marangi/docs-ABI-sysfs-class-led-trigger-netdev-Document-now-hidable-link_/20231214-202215
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/lee/leds.git for-leds-next
> patch link:    https://lore.kernel.org/r/20231214122041.17439-1-ansuelsmth%40gmail.com
> patch subject: [PATCH v3 1/2] leds: trigger: netdev: display only supported link speed attribute
> config: i386-randconfig-011-20231217 (https://download.01.org/0day-ci/archive/20231217/202312170606.NWH5SzQD-lkp@xxxxxxxxx/config)
> compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231217/202312170606.NWH5SzQD-lkp@xxxxxxxxx/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@xxxxxxxxx>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202312170606.NWH5SzQD-lkp@xxxxxxxxx/
> 
> All errors (new ones prefixed by >>):
> 
>    ld: drivers/leds/trigger/ledtrig-netdev.o: in function `get_device_state':
> >> drivers/leds/trigger/ledtrig-netdev.c:230: undefined reference to `phy_speeds'
> 
> 
> vim +230 drivers/leds/trigger/ledtrig-netdev.c
> 
>    211	
>    212	static void get_device_state(struct led_netdev_data *trigger_data)
>    213	{
>    214		struct ethtool_link_ksettings cmd;
>    215		int speeds_num;
>    216	
>    217		trigger_data->carrier_link_up = netif_carrier_ok(trigger_data->net_dev);
>    218	
>    219		if (__ethtool_get_link_ksettings(trigger_data->net_dev, &cmd))
>    220			return;
>    221	
>    222		if (trigger_data->carrier_link_up) {
>    223			trigger_data->link_speed = cmd.base.speed;
>    224			trigger_data->duplex = cmd.base.duplex;
>    225		}
>    226	
>    227		/* Have a local copy of the link speed supported to not rtnl lock every time
>    228		 * Modes are refreshed on any change event to handle mode changes
>    229		 */
>  > 230		speeds_num = phy_speeds(trigger_data->supported_link_speeds,
>    231					ARRAY_SIZE(trigger_data->supported_link_speeds),
>    232					cmd.link_modes.supported);
>    233		trigger_data->supported_link_speeds_num = speeds_num;
>    234	}
>    235	

Ugh didn't think that LEDs netdev trigger doesn't have a dependency on
PHY...

Andrew any idea about this?

I can see 2 solution (or maybe 3???):

- Add the dependency for PHY
- Move phy_speeds net_utils.c (with the settings table moved there)
- Implement a custom function in ledtrig-netdev.c

It's sad since the phy_speed was just what we needed to implement this
ins a ""clean way"".

-- 
	Ansuel