[GIT PULL] firewire updates for 6.6

[GIT PULL] firewire updates for 6.6

[Takashi Sakamoto]

Hi Linus,

Please pull firewire updates for v6.6.

The following changes since commit 2dde18cd1d8fac735875f2e4987f11817cc0bc2c:

  Linux 6.5 (2023-08-27 14:49:51 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/ieee1394/linux1394.git tags/firewire-updates-6.6

for you to fetch changes up to fd416616d0995f4947dd0a7a8c2ac4d9f916a9d0:

  firewire: allow deactivating the IEEE1394 menuconfig section at once (2023-09-07 19:11:17 +0900)

----------------------------------------------------------------
firewire updates for 6.6

The pull request includes a slight change of configuration entry in Kconfig.

In the second half of 6.6 merge window, Jan Engelhardt sent the change. It
allows any front ends of Kconfig to deactivate FireWire subsystem at a
clip. I would usually nack such late change and postpone it next merge
window, while it includes no impact to existent CI build in the world since
it just a hint for the front ends. I would like to include the change in
the time as an exception.

----------------------------------------------------------------
Jan Engelhardt (1):
      firewire: allow deactivating the IEEE1394 menuconfig section at once

 drivers/firewire/Kconfig | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

Re: [GIT PULL] firewire updates for 6.6

[Linus Torvalds]

On Fri, 8 Sept 2023 at 20:35, Takashi Sakamoto <o-takashi@sakamocchi.jp> wrote:
>
> In the second half of 6.6 merge window, Jan Engelhardt sent the change. It
> allows any front ends of Kconfig to deactivate FireWire subsystem at a
> clip.

I pulled this, but after looking at it, I unpulled it again.

We *already* had this. Saying 'N' to the existing FIREWIRE option
would disable all of the firewire stack, since the rest then just has

        depends on FIREWIRE

on it.

The only exception is the firewire sniffing side (FIREWIRE_NOSY),
which technically doesn't need the firewire stack to exist or to work.

The other thing this adds is a

        depends on PCI || COMPILE_TEST

for the firewire subsystem, which makes sense since the controllers
all depend on PCI even if the code itself doesn't care (thus the
COMPILE_TEST) part.

Anyway, both of those changes are fine by me - but adding a new config
option, and bothering users that want firewire support with TWO
questions about "do you want firewire" is just annoying and frankly
just stupid.

I have said this five hundred times before, but I guess I'll say it
five hundred times again (the Proclaimers even wrote a song about it):
we don't make the config options worse, and we don't ask people stupid
things.

So no.

The actual core limitations I'd be ok with: just add that

        depends on PCI || COMPILE_TEST

to the *existing* FIREWIRE config, and add a

        depends on FIREWIRE

to FIREWIRE_NOSY for all I care. That potentiall y*removes* annoying
questions, not adds them.

And if people want to change the existing menu to a menuconfig
(*keeping* the existing FIREWIRE config option, not adding a new one),
that's fine too.

But this "let's add yet another mindless option to ask users" is _not_
acceptable.

              Linus

[PATCH] firewire: dissolve one menu indirection

[Jan Engelhardt]

Assume you are in the "Device Drivers" menu in menuconfig, and the
cursor is on "IEEE 1394 (FireWire) support". To deactivate everything
FireWire-related, the following keystrokes are needed:

ENTER N DOWN N RIGHT ENTER

The plan of v6.5-1-gfd416616d099 was to reduce this to just

N

by making "IEEE 1394" a menuconfig item. That did not resonate, so
try a middle way which reduces it to

N DOWN N

without introducing a new config option, by dissolving one level of
menus.

Signed-off-by: Jan Engelhardt <jengelh@inai.de>
---
This patch applies on top of fd416616d0995f4947dd0a7a8c2ac4d9f916a9d0.

 drivers/firewire/Kconfig | 32 +++++++++++---------------------
 1 file changed, 11 insertions(+), 21 deletions(-)

diff --git a/drivers/firewire/Kconfig b/drivers/firewire/Kconfig
index be1a9e685782..6b27944059bc 100644
--- a/drivers/firewire/Kconfig
+++ b/drivers/firewire/Kconfig
@@ -1,22 +1,10 @@
 # SPDX-License-Identifier: GPL-2.0-only
-menuconfig FIREWIRE_SUPPORT
-	bool "IEEE 1394 (FireWire) support"
-	default y
+menuconfig FIREWIRE
+	tristate "FireWire driver stack"
+	select CRC_ITU_T
 	depends on PCI || COMPILE_TEST
 	# firewire-core does not depend on PCI but is
 	# not useful without PCI controller driver
-	help
-	  Support for FireWire.
-
-	  The answer to this question will not directly affect the
-	  kernel: saying N will just cause the configurator to skip all
-	  the questions about FireWire.
-
-if FIREWIRE_SUPPORT
-
-config FIREWIRE
-	tristate "FireWire driver stack"
-	select CRC_ITU_T
 	help
 	  This is the new-generation IEEE 1394 (FireWire) driver stack
 	  a.k.a. Juju, a new implementation designed for robustness and
@@ -28,9 +16,11 @@ config FIREWIRE
 	  To compile this driver as a module, say M here: the module will be
 	  called firewire-core.
 
+if FIREWIRE
+
 config FIREWIRE_KUNIT_UAPI_TEST
 	tristate "KUnit tests for layout of structure in UAPI" if !KUNIT_ALL_TESTS
-	depends on FIREWIRE && KUNIT
+	depends on KUNIT
 	default KUNIT_ALL_TESTS
 	help
 	  This builds the KUnit tests whether structures exposed to user
@@ -46,7 +36,7 @@ config FIREWIRE_KUNIT_UAPI_TEST
 
 config FIREWIRE_OHCI
 	tristate "OHCI-1394 controllers"
-	depends on PCI && FIREWIRE && MMU
+	depends on PCI && MMU
 	help
 	  Enable this driver if you have a FireWire controller based
 	  on the OHCI specification.  For all practical purposes, this
@@ -57,7 +47,7 @@ config FIREWIRE_OHCI
 
 config FIREWIRE_SBP2
 	tristate "Storage devices (SBP-2 protocol)"
-	depends on FIREWIRE && SCSI
+	depends on SCSI
 	help
 	  This option enables you to use SBP-2 devices connected to a
 	  FireWire bus.  SBP-2 devices include storage devices like
@@ -72,7 +62,7 @@ config FIREWIRE_SBP2
 
 config FIREWIRE_NET
 	tristate "IP networking over 1394"
-	depends on FIREWIRE && INET
+	depends on INET
 	help
 	  This enables IPv4/IPv6 over IEEE 1394, providing IP connectivity
 	  with other implementations of RFC 2734/3146 as found on several
@@ -81,6 +71,8 @@ config FIREWIRE_NET
 	  To compile this driver as a module, say M here:  The module will be
 	  called firewire-net.
 
+endif # FIREWIRE
+
 config FIREWIRE_NOSY
 	tristate "Nosy - a FireWire traffic sniffer for PCILynx cards"
 	depends on PCI
@@ -105,5 +97,3 @@ config FIREWIRE_NOSY
 	  nosy-dump, can be found in tools/firewire/ of the kernel sources.
 
 	  If unsure, say N.
-
-endif # FIREWIRE_SUPPORT
-- 
2.42.0

Re: [GIT PULL] firewire updates for 6.6

[Takashi Sakamoto]

Hi Linus,

On Sat, Sep 09, 2023 at 11:28:14AM -0700, Linus Torvalds wrote:
> On Fri, 8 Sept 2023 at 20:35, Takashi Sakamoto <o-takashi@sakamocchi.jp> wrote:
> >
> > In the second half of 6.6 merge window, Jan Engelhardt sent the change. It
> > allows any front ends of Kconfig to deactivate FireWire subsystem at a
> > clip.
> 
> I pulled this, but after looking at it, I unpulled it again.
> 
> We *already* had this. Saying 'N' to the existing FIREWIRE option
> would disable all of the firewire stack, since the rest then just has
> 
>         depends on FIREWIRE
> 
> on it.
> 
> The only exception is the firewire sniffing side (FIREWIRE_NOSY),
> which technically doesn't need the firewire stack to exist or to work.
>
> The other thing this adds is a
> 
>         depends on PCI || COMPILE_TEST
> 
> for the firewire subsystem, which makes sense since the controllers
> all depend on PCI even if the code itself doesn't care (thus the
> COMPILE_TEST) part.
> 
> Anyway, both of those changes are fine by me - but adding a new config
> option, and bothering users that want firewire support with TWO
> questions about "do you want firewire" is just annoying and frankly
> just stupid.
> 
> I have said this five hundred times before, but I guess I'll say it
> five hundred times again (the Proclaimers even wrote a song about it):
> we don't make the config options worse, and we don't ask people stupid
> things.
> 
> So no.
> 
> The actual core limitations I'd be ok with: just add that
> 
>         depends on PCI || COMPILE_TEST
> 
> to the *existing* FIREWIRE config, and add a
> 
>         depends on FIREWIRE
> 
> to FIREWIRE_NOSY for all I care. That potentiall y*removes* annoying
> questions, not adds them.
> 
> And if people want to change the existing menu to a menuconfig
> (*keeping* the existing FIREWIRE config option, not adding a new one),
> that's fine too.
> 
> But this "let's add yet another mindless option to ask users" is _not_
> acceptable.
> 
>               Linus

Indeed, the additional option would be annoying to users. I figure out
that It is reasonable to avoid the change and stop 'history repeats'.

Ideally, core functions of FireWire subsystem in firewire-core can be
built without dependency on PCI subsystem, but actually it depends, as
we can see in comment of the menu option. The nosy driver is for Texus
Instruments PCILynx device and should depends on PCI subsystem. We can
see apparent difference between these two cases of dependency on PCI
subsystem. I doubt the loss of direct dependency on PCI subsystem in nosy,
at present.

Anyway, the requester has already sent another patch[1]. I postpone it
and continue discussion with him for next merge window.

[1] https://lore.kernel.org/lkml/20230909221248.2598-1-jengelh@inai.de/


Thanks

Takashi Sakamoto