On Wed, Jun 26, 2019 at 5:31 AM Rasmus Villemoes <rasmus.villemoes@xxxxxxxxx> wrote: > > On 24/06/2019 19.26, Willem de Bruijn wrote: > > On Mon, Jun 24, 2019 at 4:34 AM Rasmus Villemoes > > <rasmus.villemoes@xxxxxxxxx> wrote: > >> > >> CONFIG_CAN_LEDS is deprecated. When trying to use the generic netdev > >> trigger as suggested, there's a small inconsistency with the link > >> property: The LED is on initially, stays on when the device is brought > >> up, and then turns off (as expected) when the device is brought down. > >> > >> Make sure the LED always reflects the state of the CAN device. > >> > >> Signed-off-by: Rasmus Villemoes <rasmus.villemoes@xxxxxxxxx> > > > > Should this target net? > > No, I think this should go through the CAN tree. Perhaps I've > misunderstood when to use the net-next prefix - is that only for things > that should be applied directly to the net-next tree? If so, sorry. I don't see consistent behavior on the list, so this is probably fine. It would probably help to target can (for fixes) or can-next (for new features). Let me reframe the question: should this target can, instead of can-next? > > Regardless of CONFIG_CAN_LEDS deprecation, > > this is already not initialized properly if that CONFIG is disabled > > and a can_led_event call at device probe is a noop. > > I'm not sure I understand this part. The CONFIG_CAN_LEDS support for > showing the state of the interface is implemented via hooking into the > ndo_open/ndo_stop callbacks, and does not look at or touch the > __LINK_STATE_NOCARRIER bit at all. > > Other than via the netdev LED trigger I don't think one can even observe > the slightly odd initial state of the __LINK_STATE_NOCARRIER bit for CAN > devices, it's still incorrect, though I guess that's moot in practice. > which is why I framed this as a fix purely to allow the netdev > trigger to be a closer drop-in replacement for CONFIG_CAN_LEDS. So the entire CONFIG_CAN_LEDS code is to be removed? What exactly is this netdev trigger replacement, if not can_led_event? Sorry, I probably miss some context.
