Skip to contentSkip to conversation

firewire updates for 6.6

GIT PULL New Takashi Sakamoto 3 participants 4 messages Sep 9, 2023 – Sep 10, 2023 Firewire

lore
Share & tools
lore

Takashi Sakamoto

author maintainer
#0 lore raw

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(-)
#1 lore raw
Quoted 4 lines · Takashi Sakamoto

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

author maintainer
#2 lore raw

Hi Linus,

Quoted 49 lines · Linus Torvalds
Quoted 4 lines · Takashi Sakamoto

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.

Quoted 8 lines

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/[email protected]/

Thanks

Takashi Sakamoto

#3 lore raw

PATCH 1/1 firewire: dissolve one menu indirection

In reply to Takashi Sakamoto

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.

---
This patch applies on top of fd416616d0995f4947dd0a7a8c2ac4d9f916a9d0.

 drivers/firewire/Kconfig | 32 +++++++++++---------------------
 1 file changed, 11 insertions(+), 21 deletions(-)
drivers/firewire/Kconfig +11 -21
index be1a9e685782..6b27944059bc 100644
@@ -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
Signature · 1 line
2.42.0