* [PATCH] bcma,ssb: simplify dependency handling for bcma and ssb drivers
@ 2023-12-18 11:58 Lukas Bulwahn
2023-12-18 13:17 ` Johannes Berg
0 siblings, 1 reply; 6+ messages in thread
From: Lukas Bulwahn @ 2023-12-18 11:58 UTC (permalink / raw)
To: Rafał Miłecki, David S . Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Kalle Valo, Larry Finger,
Arend van Spriel, Franky Lin, Hante Meuleman, Michael Buesch,
linux-wireless, netdev, b43-dev, brcm80211-dev-list.pdl,
SHA-cyfmac-dev-list
Cc: kernel-janitors, linux-kernel, Lukas Bulwahn
The files, drivers/bcma/Kconfig and drivers/ssb/Kconfig, define two helper
config options BCMA_POSSIBLE and SSB_POSSIBLE. Both options are defined
identical:
config {BCMA_POSSIBLE,SSB_POSSIBLE}
bool
depends on HAS_IOMEM && HAS_DMA
default y
While this kind of duplication might still be acceptable in order to have
both sections work independently of each other, it really gets strange when
looking how they are then used in expression where both of those configs
appear. E.g., config B43's dependency is:
(BCMA_POSSIBLE || SSB_POSSIBLE) && MAC80211 && HAS_DMA
Note that BCMA_POSSIBLE and SSB_POSSIBLE identical and already have HAS_DMA
as condition, so that is then also another duplication.
Another example is the choice Supported bus types in B43, which already
depends on B43 and hence, we know that HAS_IOMEM && HAS_DMA holds, so all
stated dependencies in the choice are true in all cases.
Given this whole confusion around the use of these two symbols, just remove
them and replace them with the expression they intend to abbreviate.
Signed-off-by: Lukas Bulwahn <lukas.bulwahn@gmail.com>
---
drivers/bcma/Kconfig | 7 +------
drivers/net/ethernet/broadcom/Kconfig | 4 ++--
drivers/net/wireless/broadcom/b43/Kconfig | 5 +----
drivers/net/wireless/broadcom/b43legacy/Kconfig | 2 +-
drivers/net/wireless/broadcom/brcm80211/Kconfig | 3 +--
drivers/ssb/Kconfig | 7 +------
6 files changed, 7 insertions(+), 21 deletions(-)
diff --git a/drivers/bcma/Kconfig b/drivers/bcma/Kconfig
index b9558ff20830..26bb2a28c7db 100644
--- a/drivers/bcma/Kconfig
+++ b/drivers/bcma/Kconfig
@@ -1,12 +1,7 @@
# SPDX-License-Identifier: GPL-2.0
-config BCMA_POSSIBLE
- bool
- depends on HAS_IOMEM && HAS_DMA
- default y
-
menuconfig BCMA
tristate "Broadcom specific AMBA"
- depends on BCMA_POSSIBLE
+ depends on HAS_IOMEM && HAS_DMA
help
Bus driver for Broadcom specific Advanced Microcontroller Bus
Architecture.
diff --git a/drivers/net/ethernet/broadcom/Kconfig b/drivers/net/ethernet/broadcom/Kconfig
index 75ca3ddda1f5..8abbdb88459c 100644
--- a/drivers/net/ethernet/broadcom/Kconfig
+++ b/drivers/net/ethernet/broadcom/Kconfig
@@ -6,7 +6,7 @@
config NET_VENDOR_BROADCOM
bool "Broadcom devices"
default y
- depends on (SSB_POSSIBLE && HAS_DMA) || PCI || BCM63XX || \
+ depends on (HAS_IOMEM && HAS_DMA) || PCI || BCM63XX || \
SIBYTE_SB1xxx_SOC
help
If you have a network (Ethernet) chipset belonging to this class,
@@ -21,7 +21,7 @@ if NET_VENDOR_BROADCOM
config B44
tristate "Broadcom 440x/47xx ethernet support"
- depends on SSB_POSSIBLE && HAS_DMA
+ depends on HAS_IOMEM && HAS_DMA
select SSB
select MII
select PHYLIB
diff --git a/drivers/net/wireless/broadcom/b43/Kconfig b/drivers/net/wireless/broadcom/b43/Kconfig
index 4559549b80fe..f53eaa8b11cd 100644
--- a/drivers/net/wireless/broadcom/b43/Kconfig
+++ b/drivers/net/wireless/broadcom/b43/Kconfig
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
config B43
tristate "Broadcom 43xx wireless support (mac80211 stack)"
- depends on (BCMA_POSSIBLE || SSB_POSSIBLE) && MAC80211 && HAS_DMA
+ depends on HAS_IOMEM && HAS_DMA && MAC80211
select BCMA if B43_BCMA
select SSB if B43_SSB
select FW_LOADER
@@ -42,18 +42,15 @@ choice
config B43_BUSES_BCMA_AND_SSB
bool "BCMA and SSB"
- depends on BCMA_POSSIBLE && SSB_POSSIBLE
select B43_BCMA
select B43_SSB
config B43_BUSES_BCMA
bool "BCMA only"
- depends on BCMA_POSSIBLE
select B43_BCMA
config B43_BUSES_SSB
bool "SSB only"
- depends on SSB_POSSIBLE
select B43_SSB
endchoice
diff --git a/drivers/net/wireless/broadcom/b43legacy/Kconfig b/drivers/net/wireless/broadcom/b43legacy/Kconfig
index e4da34ec4f5b..ff11c63b5248 100644
--- a/drivers/net/wireless/broadcom/b43legacy/Kconfig
+++ b/drivers/net/wireless/broadcom/b43legacy/Kconfig
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
config B43LEGACY
tristate "Broadcom 43xx-legacy wireless support (mac80211 stack)"
- depends on SSB_POSSIBLE && MAC80211 && HAS_DMA
+ depends on HAS_IOMEM && HAS_DMA && MAC80211
select SSB
select FW_LOADER
help
diff --git a/drivers/net/wireless/broadcom/brcm80211/Kconfig b/drivers/net/wireless/broadcom/brcm80211/Kconfig
index 3a1a35b5672f..2e1db48201ff 100644
--- a/drivers/net/wireless/broadcom/brcm80211/Kconfig
+++ b/drivers/net/wireless/broadcom/brcm80211/Kconfig
@@ -4,8 +4,7 @@ config BRCMUTIL
config BRCMSMAC
tristate "Broadcom IEEE802.11n PCIe SoftMAC WLAN driver"
- depends on MAC80211
- depends on BCMA_POSSIBLE
+ depends on HAS_IOMEM && HAS_DMA && MAC80211
select BCMA
select BRCMUTIL
select FW_LOADER
diff --git a/drivers/ssb/Kconfig b/drivers/ssb/Kconfig
index 1cf1a98952fa..0a6d5a60b5a8 100644
--- a/drivers/ssb/Kconfig
+++ b/drivers/ssb/Kconfig
@@ -1,12 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
-config SSB_POSSIBLE
- bool
- depends on HAS_IOMEM && HAS_DMA
- default y
-
menuconfig SSB
tristate "Sonics Silicon Backplane support"
- depends on SSB_POSSIBLE
+ depends on HAS_IOMEM && HAS_DMA
help
Support for the Sonics Silicon Backplane bus.
You only need to enable this option, if you are
--
2.17.1
_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] bcma,ssb: simplify dependency handling for bcma and ssb drivers
2023-12-18 11:58 [PATCH] bcma,ssb: simplify dependency handling for bcma and ssb drivers Lukas Bulwahn
@ 2023-12-18 13:17 ` Johannes Berg
2023-12-18 14:59 ` Kalle Valo
2023-12-18 14:59 ` Lukas Bulwahn
0 siblings, 2 replies; 6+ messages in thread
From: Johannes Berg @ 2023-12-18 13:17 UTC (permalink / raw)
To: Lukas Bulwahn, Rafał Miłecki, David S . Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Kalle Valo,
Larry Finger, Arend van Spriel, Franky Lin, Hante Meuleman,
Michael Buesch, linux-wireless, netdev, b43-dev,
brcm80211-dev-list.pdl, SHA-cyfmac-dev-list
Cc: kernel-janitors, linux-kernel
On Mon, 2023-12-18 at 12:58 +0100, Lukas Bulwahn wrote:
Dunno, I'm not super involved with this but ...
> +++ b/drivers/bcma/Kconfig
> @@ -1,12 +1,7 @@
> # SPDX-License-Identifier: GPL-2.0
> -config BCMA_POSSIBLE
> - bool
> - depends on HAS_IOMEM && HAS_DMA
> - default y
> -
> menuconfig BCMA
> tristate "Broadcom specific AMBA"
> - depends on BCMA_POSSIBLE
> + depends on HAS_IOMEM && HAS_DMA
[...]
> config BRCMSMAC
> tristate "Broadcom IEEE802.11n PCIe SoftMAC WLAN driver"
> - depends on MAC80211
> - depends on BCMA_POSSIBLE
> + depends on HAS_IOMEM && HAS_DMA && MAC80211
> select BCMA
to me it kind of seems more obvious for example in this case to say
"depend on BCMA_POSSIBLE and select BCMA" rather than open-coding the
BCMA dependencies both here and in BCMA? Now granted, they're rather
unlikely to _change_, but it still seems more obvious?
johannes
_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bcma,ssb: simplify dependency handling for bcma and ssb drivers
2023-12-18 13:17 ` Johannes Berg
@ 2023-12-18 14:59 ` Kalle Valo
2023-12-18 15:03 ` Lukas Bulwahn
2023-12-18 14:59 ` Lukas Bulwahn
1 sibling, 1 reply; 6+ messages in thread
From: Kalle Valo @ 2023-12-18 14:59 UTC (permalink / raw)
To: Johannes Berg
Cc: brcm80211-dev-list.pdl, Arend van Spriel, kernel-janitors,
linux-wireless, Franky Lin, Hante Meuleman, linux-kernel,
Eric Dumazet, netdev, SHA-cyfmac-dev-list, b43-dev,
Michael Buesch, Jakub Kicinski, Lukas Bulwahn, Paolo Abeni,
David S . Miller, Larry Finger
Johannes Berg <johannes@sipsolutions.net> writes:
> On Mon, 2023-12-18 at 12:58 +0100, Lukas Bulwahn wrote:
>
> Dunno, I'm not super involved with this but ...
>
>> +++ b/drivers/bcma/Kconfig
>> @@ -1,12 +1,7 @@
>> # SPDX-License-Identifier: GPL-2.0
>> -config BCMA_POSSIBLE
>> - bool
>> - depends on HAS_IOMEM && HAS_DMA
>> - default y
>> -
>> menuconfig BCMA
>> tristate "Broadcom specific AMBA"
>> - depends on BCMA_POSSIBLE
>> + depends on HAS_IOMEM && HAS_DMA
>
> [...]
>> config BRCMSMAC
>> tristate "Broadcom IEEE802.11n PCIe SoftMAC WLAN driver"
>> - depends on MAC80211
>> - depends on BCMA_POSSIBLE
>> + depends on HAS_IOMEM && HAS_DMA && MAC80211
>> select BCMA
>
> to me it kind of seems more obvious for example in this case to say
> "depend on BCMA_POSSIBLE and select BCMA" rather than open-coding the
> BCMA dependencies both here and in BCMA? Now granted, they're rather
> unlikely to _change_, but it still seems more obvious?
I was thinking the same. Lukas, is there a specific reason why you want
to change this or this just something you noticed by chance?
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bcma,ssb: simplify dependency handling for bcma and ssb drivers
2023-12-18 14:59 ` Kalle Valo
@ 2023-12-18 15:03 ` Lukas Bulwahn
2023-12-18 16:16 ` Michael Büsch
0 siblings, 1 reply; 6+ messages in thread
From: Lukas Bulwahn @ 2023-12-18 15:03 UTC (permalink / raw)
To: Kalle Valo
Cc: brcm80211-dev-list.pdl, Arend van Spriel, kernel-janitors,
linux-wireless, Franky Lin, Hante Meuleman, linux-kernel,
Eric Dumazet, SHA-cyfmac-dev-list, b43-dev, Michael Buesch,
netdev, Jakub Kicinski, Johannes Berg, Paolo Abeni,
David S . Miller, Larry Finger
On Mon, Dec 18, 2023 at 3:59 PM Kalle Valo <kvalo@kernel.org> wrote:
>
> Johannes Berg <johannes@sipsolutions.net> writes:
>
> > On Mon, 2023-12-18 at 12:58 +0100, Lukas Bulwahn wrote:
> >
> > Dunno, I'm not super involved with this but ...
> >
> >> +++ b/drivers/bcma/Kconfig
> >> @@ -1,12 +1,7 @@
> >> # SPDX-License-Identifier: GPL-2.0
> >> -config BCMA_POSSIBLE
> >> - bool
> >> - depends on HAS_IOMEM && HAS_DMA
> >> - default y
> >> -
> >> menuconfig BCMA
> >> tristate "Broadcom specific AMBA"
> >> - depends on BCMA_POSSIBLE
> >> + depends on HAS_IOMEM && HAS_DMA
> >
> > [...]
> >> config BRCMSMAC
> >> tristate "Broadcom IEEE802.11n PCIe SoftMAC WLAN driver"
> >> - depends on MAC80211
> >> - depends on BCMA_POSSIBLE
> >> + depends on HAS_IOMEM && HAS_DMA && MAC80211
> >> select BCMA
> >
> > to me it kind of seems more obvious for example in this case to say
> > "depend on BCMA_POSSIBLE and select BCMA" rather than open-coding the
> > BCMA dependencies both here and in BCMA? Now granted, they're rather
> > unlikely to _change_, but it still seems more obvious?
>
> I was thinking the same. Lukas, is there a specific reason why you want
> to change this or this just something you noticed by chance?
>
I just noticed this by chance---well, I was wondering what these
config symbols were doing in my kernel build configuration (they are
actually in every config). While reading through the code, I was
confused on what the dependencies were trying to tell me, as the
config symbols and conditions seemed to repeat over and over in
different places.
I thought it was worth a clean up and this was the patch I came up
with in the end.
Lukas
_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bcma,ssb: simplify dependency handling for bcma and ssb drivers
2023-12-18 15:03 ` Lukas Bulwahn
@ 2023-12-18 16:16 ` Michael Büsch
0 siblings, 0 replies; 6+ messages in thread
From: Michael Büsch @ 2023-12-18 16:16 UTC (permalink / raw)
To: Lukas Bulwahn
Cc: brcm80211-dev-list.pdl, Arend van Spriel, kernel-janitors,
linux-wireless, Kalle Valo, Franky Lin, Hante Meuleman,
linux-kernel, Eric Dumazet, SHA-cyfmac-dev-list, b43-dev, netdev,
Jakub Kicinski, Johannes Berg, Paolo Abeni, David S . Miller,
Larry Finger
[-- Attachment #1.1: Type: text/plain, Size: 1185 bytes --]
Hi Lukas,
thanks for your patch.
On Mon, 18 Dec 2023 16:03:54 +0100
Lukas Bulwahn <lukas.bulwahn@gmail.com> wrote:
> While reading through the code, I was
> confused on what the dependencies were trying to tell me, as the
> config symbols and conditions seemed to repeat over and over in
> different places.
The {SSB,BCMA}_POSSIBLE constants are defining the conditions under
which it is possible to 'select' SSB/BCMA.
SSB and BCMA are usually 'select'ed rather than depended on, for better
user experience while configuring.
> I thought it was worth a clean up and this was the patch I came up
> with in the end.
IMO this does not clean up or simplify the code.
It rather makes it more complicated to maintain.
The idea behind the POSSIBLE constants it to _not_ spread the
conditions all across the drivers. That has significant advantages, if
the condition changes.
I also don't see the redundancy in the resulting dependency conditions
as a bad thing. It's better if every option explicitly defines its
dependencies rather than expecting something else to depend on it.
That's fragile.
NAK from me.
--
Michael Büsch
https://bues.ch/
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
[-- Attachment #2: Type: text/plain, Size: 149 bytes --]
_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] bcma,ssb: simplify dependency handling for bcma and ssb drivers
2023-12-18 13:17 ` Johannes Berg
2023-12-18 14:59 ` Kalle Valo
@ 2023-12-18 14:59 ` Lukas Bulwahn
1 sibling, 0 replies; 6+ messages in thread
From: Lukas Bulwahn @ 2023-12-18 14:59 UTC (permalink / raw)
To: Johannes Berg
Cc: brcm80211-dev-list.pdl, Arend van Spriel, kernel-janitors,
linux-wireless, Kalle Valo, Franky Lin, Hante Meuleman,
linux-kernel, Eric Dumazet, SHA-cyfmac-dev-list, b43-dev,
Michael Buesch, netdev, Jakub Kicinski, Paolo Abeni,
David S . Miller, Larry Finger
On Mon, Dec 18, 2023 at 2:18 PM Johannes Berg <johannes@sipsolutions.net> wrote:
>
> On Mon, 2023-12-18 at 12:58 +0100, Lukas Bulwahn wrote:
>
> Dunno, I'm not super involved with this but ...
>
> > +++ b/drivers/bcma/Kconfig
> > @@ -1,12 +1,7 @@
> > # SPDX-License-Identifier: GPL-2.0
> > -config BCMA_POSSIBLE
> > - bool
> > - depends on HAS_IOMEM && HAS_DMA
> > - default y
> > -
> > menuconfig BCMA
> > tristate "Broadcom specific AMBA"
> > - depends on BCMA_POSSIBLE
> > + depends on HAS_IOMEM && HAS_DMA
>
> [...]
> > config BRCMSMAC
> > tristate "Broadcom IEEE802.11n PCIe SoftMAC WLAN driver"
> > - depends on MAC80211
> > - depends on BCMA_POSSIBLE
> > + depends on HAS_IOMEM && HAS_DMA && MAC80211
> > select BCMA
>
> to me it kind of seems more obvious for example in this case to say
> "depend on BCMA_POSSIBLE and select BCMA" rather than open-coding the
> BCMA dependencies both here and in BCMA? Now granted, they're rather
> unlikely to _change_, but it still seems more obvious?
>
Okay, I see. Well, if that kind of pattern is the preference, then the
code as-is makes sense. The pattern just starts to become obscure when
the dependencies of multiple drivers are the same and we start writing
"BCMA_POSSIBLE || SSB_POSSIBLE", but the dependencies are the same
anyway.
Let us see what others think.
Lukas
_______________________________________________
b43-dev mailing list
b43-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/b43-dev
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-12-18 16:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-18 11:58 [PATCH] bcma,ssb: simplify dependency handling for bcma and ssb drivers Lukas Bulwahn
2023-12-18 13:17 ` Johannes Berg
2023-12-18 14:59 ` Kalle Valo
2023-12-18 15:03 ` Lukas Bulwahn
2023-12-18 16:16 ` Michael Büsch
2023-12-18 14:59 ` Lukas Bulwahn
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 NNTP newsgroup(s).