All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael Büsch" <m@bues.ch>
To: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Cc: brcm80211-dev-list.pdl@broadcom.com,
	Arend van Spriel <aspriel@gmail.com>,
	kernel-janitors@vger.kernel.org, linux-wireless@vger.kernel.org,
	Kalle Valo <kvalo@kernel.org>,
	Franky Lin <franky.lin@broadcom.com>,
	Hante Meuleman <hante.meuleman@broadcom.com>,
	linux-kernel@vger.kernel.org, Eric Dumazet <edumazet@google.com>,
	SHA-cyfmac-dev-list@infineon.com, b43-dev@lists.infradead.org,
	netdev@vger.kernel.org, Jakub Kicinski <kuba@kernel.org>,
	Johannes Berg <johannes@sipsolutions.net>,
	Paolo Abeni <pabeni@redhat.com>,
	"David S . Miller" <davem@davemloft.net>,
	Larry Finger <Larry.Finger@lwfinger.net>
Subject: Re: [PATCH] bcma,ssb: simplify dependency handling for bcma and ssb drivers
Date: Mon, 18 Dec 2023 17:16:29 +0100	[thread overview]
Message-ID: <20231218171629.5cf95fd3@barney> (raw)
In-Reply-To: <CAKXUXMxh3rM8da9kJG_=Sy8fQqqf7f8xXaHDHPLvpvRiYg1e5w@mail.gmail.com>


[-- 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

WARNING: multiple messages have this Message-ID (diff)
From: "Michael Büsch" <m@bues.ch>
To: Lukas Bulwahn <lukas.bulwahn@gmail.com>
Cc: "Kalle Valo" <kvalo@kernel.org>,
	"Johannes Berg" <johannes@sipsolutions.net>,
	"Rafał Miłecki" <zajec5@gmail.com>,
	"David S . Miller" <davem@davemloft.net>,
	"Eric Dumazet" <edumazet@google.com>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Larry Finger" <Larry.Finger@lwfinger.net>,
	"Arend van Spriel" <aspriel@gmail.com>,
	"Franky Lin" <franky.lin@broadcom.com>,
	"Hante Meuleman" <hante.meuleman@broadcom.com>,
	linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
	b43-dev@lists.infradead.org, brcm80211-dev-list.pdl@broadcom.com,
	SHA-cyfmac-dev-list@infineon.com,
	kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] bcma,ssb: simplify dependency handling for bcma and ssb drivers
Date: Mon, 18 Dec 2023 17:16:29 +0100	[thread overview]
Message-ID: <20231218171629.5cf95fd3@barney> (raw)
In-Reply-To: <CAKXUXMxh3rM8da9kJG_=Sy8fQqqf7f8xXaHDHPLvpvRiYg1e5w@mail.gmail.com>

[-- Attachment #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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2023-12-18 16:17 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-18 11:58 [PATCH] bcma,ssb: simplify dependency handling for bcma and ssb drivers Lukas Bulwahn
2023-12-18 11:58 ` Lukas Bulwahn
2023-12-18 13:17 ` Johannes Berg
2023-12-18 13:17   ` Johannes Berg
2023-12-18 14:59   ` Kalle Valo
2023-12-18 14:59     ` Kalle Valo
2023-12-18 15:03     ` Lukas Bulwahn
2023-12-18 15:03       ` Lukas Bulwahn
2023-12-18 16:16       ` Michael Büsch [this message]
2023-12-18 16:16         ` Michael Büsch
2023-12-18 14:59   ` Lukas Bulwahn
2023-12-18 14:59     ` Lukas Bulwahn

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=20231218171629.5cf95fd3@barney \
    --to=m@bues.ch \
    --cc=Larry.Finger@lwfinger.net \
    --cc=SHA-cyfmac-dev-list@infineon.com \
    --cc=aspriel@gmail.com \
    --cc=b43-dev@lists.infradead.org \
    --cc=brcm80211-dev-list.pdl@broadcom.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=franky.lin@broadcom.com \
    --cc=hante.meuleman@broadcom.com \
    --cc=johannes@sipsolutions.net \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=kuba@kernel.org \
    --cc=kvalo@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=lukas.bulwahn@gmail.com \
    --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 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.