All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Klaus Kudielka <klaus.kudielka@gmail.com>,
	"David S . Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>, netdev <netdev@vger.kernel.org>
Subject: Re: [PATCH] leds: triggers: netdev: add a check, whether device is up
Date: Sat, 4 Nov 2023 17:12:32 +0000	[thread overview]
Message-ID: <ZUZ7gAXs0/gDRbIc@shell.armlinux.org.uk> (raw)
In-Reply-To: <196db01b-40ff-44ed-8e45-1b855940417f@lunn.ch>

On Sat, Nov 04, 2023 at 05:41:54PM +0100, Andrew Lunn wrote:
> [Changes the Cc: list. Dropping LED people, adding a few netdev
> people]
> 
> On Sat, Nov 04, 2023 at 04:27:45PM +0100, Klaus Kudielka wrote:
> > After booting, the device is down, but netdev trigger reports "link" active.
> > This looks wrong to me.
> > 
> > I can then "ip link set eth2 up", and the "link" goes away - as I
> > would have expected it to be from the beginning.
> 
> Thanks for the details.
> 
> A brain dump...
> 
> You do see a lot of MAC drivers doing a netif_carrier_off() in there
> probe function. That suggests the carrier is on by default. I doubt we
> can change that, we would break all the drivers which assume the
> carrier is on by default, probably virtual devices and some real
> devices.

Note that one of the things that phylink will do is call
netif_carrier_off() when phylink_start() is called to ensure that the
netdev state matches its internal state.

> Often the MAC and PHY are connected in the open() callback, when using
> phylib. So that is too late.  phylink_create() is however mostly used
> in the probe function. So it could set the carrier to off by default.
> Russell, what do you think?

I was going to ask whether that would be a good idea - since it means
that the carrier is in the right state at probe time as well as after
phylink_start() has been called.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

      reply	other threads:[~2023-11-04 17:12 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-04 12:58 [PATCH] leds: triggers: netdev: add a check, whether device is up Klaus Kudielka
2023-11-04 14:29 ` Andrew Lunn
2023-11-04 15:27   ` Klaus Kudielka
2023-11-04 16:32     ` Klaus Kudielka
2023-11-04 16:46       ` Andrew Lunn
2023-11-04 17:42         ` Russell King (Oracle)
2023-11-04 19:46           ` Klaus Kudielka
2023-11-04 16:41     ` Andrew Lunn
2023-11-04 17:12       ` Russell King (Oracle) [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZUZ7gAXs0/gDRbIc@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=klaus.kudielka@gmail.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.