Linux-LEDs Archive mirror
 help / color / mirror / Atom feed
From: Heiner Kallweit <hkallweit1@gmail.com>
To: Arnd Bergmann <arnd@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>
Cc: Arnd Bergmann <arnd@arndb.de>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	"linux-leds@vger.kernel.org" <linux-leds@vger.kernel.org>,
	Lee Jones <lee@kernel.org>
Subject: Re: [PATCH] r8169: fix building with CONFIG_LEDS_CLASS=m
Date: Wed, 3 Jan 2024 12:33:53 +0100	[thread overview]
Message-ID: <5b0a6150-8043-4de7-980f-54020a3e7981@gmail.com> (raw)
In-Reply-To: <20240103102630.3770242-1-arnd@kernel.org>

On 03.01.2024 11:26, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> When r8169 is built-in but the LED support is a loadable module, the
> new code to drive the LED now causes a link failure:
> 
> ld: drivers/net/ethernet/realtek/r8169_leds.o: in function `rtl8168_init_leds':
> r8169_leds.c:(.text+0x36c): undefined reference to `devm_led_classdev_register_ext'
> 
> Add a Kconfig dependency to prevent the broken configuration but still
> allow having the network code built-in as long as CONFIG_LEDS_TRIGGER_NETDEV
> is disabled, regardless of CONFIG_LEDS_CLASS.
> 
The proposed change is more of a workaround IMO. A proper fix (in LED subsystem)
has been submitted, but it's not reviewed/applied yet. And I don't think building
r8169 should depend on support for an optional feature.
This fix would also allow to remove Kconfig dependencies similar to the one
proposed here from other drivers. Link to submitted fix:

https://lore.kernel.org/linux-leds/0f6f432b-c650-4bb8-a1b5-fe3372804d52@gmail.com/T/#u

> Fixes: 18764b883e15 ("r8169: add support for LED's on RTL8168/RTL8101")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/net/ethernet/realtek/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/net/ethernet/realtek/Kconfig b/drivers/net/ethernet/realtek/Kconfig
> index 93d9df55b361..fd3f18b328de 100644
> --- a/drivers/net/ethernet/realtek/Kconfig
> +++ b/drivers/net/ethernet/realtek/Kconfig
> @@ -98,6 +98,7 @@ config 8139_OLD_RX_RESET
>  config R8169
>  	tristate "Realtek 8169/8168/8101/8125 ethernet support"
>  	depends on PCI
> +	depends on LEDS_CLASS || !LEDS_TRIGGER_NETDEV
>  	select FW_LOADER
>  	select CRC32
>  	select PHYLIB


       reply	other threads:[~2024-01-03 11:33 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20240103102630.3770242-1-arnd@kernel.org>
2024-01-03 11:33 ` Heiner Kallweit [this message]
2024-01-03 12:45   ` [PATCH] r8169: fix building with CONFIG_LEDS_CLASS=m Arnd Bergmann
2024-01-03 13:56     ` Heiner Kallweit

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=5b0a6150-8043-4de7-980f-54020a3e7981@gmail.com \
    --to=hkallweit1@gmail.com \
    --cc=arnd@arndb.de \
    --cc=arnd@kernel.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=lee@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-leds@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).