staging: vme: Restructuring menuconfig and tree

This patch series modify the vme_user driver's place in menuconfig (1/3), fixes a missing depends on line in a Kconfig file (2/3), and rearrages the directory tree for the driver allowing a more straightforward comprehension of its contents (3/3).

Bruno Moreira-Guedes (3):
staging: vme: Adjusted VME_USER in Kconfig
staging: vme: Fix missing depends on at KConfig
staging: vme: "drivers/staging/vme" tree cleanup

 MAINTAINERS                                          | 2 +-
 drivers/staging/Kconfig                              | 2 ++
 drivers/staging/Makefile                             | 2 +-
 drivers/staging/vme/Makefile                         | 2 --
 drivers/staging/{vme/devices => vme_user}/Kconfig    | 2 +-
 drivers/staging/{vme/devices => vme_user}/Makefile   | 0
 drivers/staging/{vme/devices => vme_user}/vme_user.c | 0
 drivers/staging/{vme/devices => vme_user}/vme_user.h | 0
 drivers/vme/Kconfig                                  | 2 --
 9 files changed, 5 insertions(+), 7 deletions(-)
 delete mode 100644 drivers/staging/vme/Makefile
 rename drivers/staging/{vme/devices => vme_user}/Kconfig (93%)
 rename drivers/staging/{vme/devices => vme_user}/Makefile (100%)
 rename drivers/staging/{vme/devices => vme_user}/vme_user.c (100%)
 rename drivers/staging/{vme/devices => vme_user}/vme_user.h (100%)

2.35.1

Currently, the VME_USER driver is in the staging tree Kconfig, unlike other VME drivers already moved to the main portions of the kernel tree. Its configuration is, however, nested into the VME_BUS config option, which might be misleading.

Since the staging tree "[...] is used to hold stand-alone[1] drivers and filesystem that are not ready to be merged into the main portion of the Linux kernel tree [...]"[1], IMHO all staging drivers should appear nested into the Main Menu -> Device Drivers -> Staging Drivers to make sure the user don't pick it without being fully aware of its staging status as it could be the case in Menu -> Device Drivers -> VME bridge support (the current location).

With this change menuconfig users will clearly know this is not a driver in the main portion of the kernel tree and decide whether to build it or not with that clearly in mind.

This change goes into the same direction of commit 4b4cdf3979c3 ("STAGING: Move staging drivers back to staging-specific menu")

CHANGELOG
v1:
- Sourced "drivers/staging/vme/devices/Kconfig" in "drivers/staging/Kconfig" and unsourced in "drivers/vme/Kconfig".

v2:
- Turned into a patchset to (1) fix missing "depends on" missing entry in config VME_USER at "drivers/staging/vme/devices/Kconfig", affecting the behavior of this patch, and (2) fix unnecessary remaining files from the time the whole vme subtree was in staging.

REFERENCES
[1] https://lkml.org/lkml/2009/3/18/314

---
 drivers/staging/Kconfig | 2 ++
 drivers/vme/Kconfig     | 2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

index 932acb4e8cbc..0545850eb2ff 100644
@@ -88,4 +88,6 @@ source "drivers/staging/qlge/Kconfig"
 
 source "drivers/staging/wfx/Kconfig"
 
+source "drivers/staging/vme/devices/Kconfig"
+
 endif # STAGING

index 936392ca3c8c..c13dd9d2a604 100644
@@ -15,6 +15,4 @@ source "drivers/vme/bridges/Kconfig"
 
 source "drivers/vme/boards/Kconfig"
 
-source "drivers/staging/vme/devices/Kconfig"
-
 endif # VME

2.35.1

Changelog goes below the --- line please.

Never reference lkml.org, we have no control over it and it goes down a lot. Please use lore.kernel.org links, and put the link in the body of the text, no need for footnotes.

thanks,

greg k-h

On Tue, Apr 12, 2022 at 03:55:21AM -0300, Bruno Moreira-Guedes wrote:

'Adjusted' is unecessarily vague. In the commit msg try to say exactly what the patch did. Write in the imperative tense, like you are issuing commands.

Something like 'Move VME staging driver to staging Kconfig' , or 'Source vme driver from staging Kconfig'.

Skip the 'IMHO'. It's all your HO and if you state anything incorrectly, your reviewers should catch it. If you are truly making a wild guess about something, either look into it further, or not it in a question below the '---'.

This commit message is otherwise very well written. It explains the why of this patch nicely.

Again, for this set, perhaps the changelog in the cover letter only.

Originally, the KConfig file for VME_USER ("drivers/staging/vme/devices/Kconfig") was sourced at "drivers/vme/boards/KConfig". It misses a depends on line for VME_BUS, which went unnoticeable for a menuconfig user who wouldn't be able to select it through this interface without selecting the CONFIG_VME_BUS option because it was nested on its menu.

A patch to move appropriately this driver to the "Staging" submenu[1], however, unveiled this missing depends on line, since it moved the source line for VME_USER's KConfig to somewhere else, allowing it to be compiled without the support for VME_BRIDGE. While it compiles fine and seems to initialize the driver fine[2], it seems to make no sense having VME_USER without VME_BUS.

This patches addresses it by adding the VME_BUS to the depends on line at "drivers/staging/vme/devices/Kconfig".

CHANGELOG:
v2:
- Added this patch to the patchset

REFERENCES AND NOTES
[1] First commit in this patch, originally submitted at
https://lore.kernel.org/linux-staging/[email protected]/T/#t
[2] Tested both as module and as built-in.

---
 drivers/staging/vme/devices/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

index 5651bb16b28b..e8b4461bf27f 100644
@@ -3,7 +3,7 @@ comment "VME Device Drivers"
 
 config VME_USER
 	tristate "VME user space access driver"
-	depends on STAGING
+	depends on STAGING && VME_BUS
 	help
 	  If you say Y here you want to be able to access a limited number of
 	  VME windows in a manner at least semi-compatible with the interface

2.35.1

Be direct in the commit msg. Perhaps -
'Add 'depends on' VME_BUS to Kconfig'

That's all!

snip

In <db3b9e990e75> ("Staging: VME: move VME drivers out of staging") the vme code, board and bridge drivers were moved out of the staging tree, remaining only the VME user device driver.

Since this driver is the only one remaining in staging, such multi-level struct confuses more than helps. The current structure is as follows:

- drivers/staging/vme/

                      +Makefile
                      +devices/
                               +Kconfig
                               +Makefile
                               +vme_user.c
                               +vme_user.h

The root Makefile has the only function of calling the other Makefile into the devices/ subdirectory. This latter only compiles the vme_user driver, since there is no other.

This patch allows a more straightforward understanding of this driver's contents by removing the unnecessary Makefile from the 'vme/' subdir, moving the contents of 'vme/devices' straight into 'vme/', and renaming 'vme/' to 'vme_user' (the driver name). It also adjusts the Kconfig and the Makefile from drivers/staging to properly reflect the new structure.

CHANGELOG
v2:
- Added this patch to the patchset

---
 MAINTAINERS                                          | 2 +-
 drivers/staging/Kconfig                              | 2 +-
 drivers/staging/Makefile                             | 2 +-
 drivers/staging/vme/Makefile                         | 2 --
 drivers/staging/{vme/devices => vme_user}/Kconfig    | 0
 drivers/staging/{vme/devices => vme_user}/Makefile   | 0
 drivers/staging/{vme/devices => vme_user}/vme_user.c | 0
 drivers/staging/{vme/devices => vme_user}/vme_user.h | 0
 8 files changed, 3 insertions(+), 5 deletions(-)
 delete mode 100644 drivers/staging/vme/Makefile
 rename drivers/staging/{vme/devices => vme_user}/Kconfig (100%)
 rename drivers/staging/{vme/devices => vme_user}/Makefile (100%)
 rename drivers/staging/{vme/devices => vme_user}/vme_user.c (100%)
 rename drivers/staging/{vme/devices => vme_user}/vme_user.h (100%)

index fd768d43e048..88423218e5e6 100644
@@ -21014,7 +21014,7 @@ L:	[email protected]
 S:	Maintained
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/char-misc.git
 F:	Documentation/driver-api/vme.rst
-F:	drivers/staging/vme/
+F:	drivers/staging/vme_user/
 F:	drivers/vme/
 F:	include/linux/vme*
 

index 0545850eb2ff..9f11592336f8 100644
@@ -88,6 +88,6 @@ source "drivers/staging/qlge/Kconfig"
 
 source "drivers/staging/wfx/Kconfig"
 
-source "drivers/staging/vme/devices/Kconfig"
+source "drivers/staging/vme_user/Kconfig"
 
 endif # STAGING

index 3ffb35ccfae2..1dffa02121ee 100644
@@ -14,7 +14,7 @@ obj-$(CONFIG_OCTEON_ETHERNET)	+= octeon/
 obj-$(CONFIG_OCTEON_USB)	+= octeon-usb/
 obj-$(CONFIG_VT6655)		+= vt6655/
 obj-$(CONFIG_VT6656)		+= vt6656/
-obj-$(CONFIG_VME_BUS)		+= vme/
+obj-$(CONFIG_VME_BUS)		+= vme_user/
 obj-$(CONFIG_IIO)		+= iio/
 obj-$(CONFIG_FB_SM750)		+= sm750fb/
 obj-$(CONFIG_USB_EMXX)		+= emxx_udc/

deleted file mode 100644
index cf2f686ccffe..000000000000
@@ -1,2 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0
-obj-y				+= devices/

similarity index 100%
rename from drivers/staging/vme/devices/Kconfig
rename to drivers/staging/vme_user/Kconfig

similarity index 100%
rename from drivers/staging/vme/devices/Makefile
rename to drivers/staging/vme_user/Makefile

similarity index 100%
rename from drivers/staging/vme/devices/vme_user.c
rename to drivers/staging/vme_user/vme_user.c

similarity index 100%
rename from drivers/staging/vme/devices/vme_user.h
rename to drivers/staging/vme_user/vme_user.h

2.35.1

Only the commit msg again ;)

staging: vme: Move drivers/vme to drivers/vme_user

I think that's the whole point here.

That's all!

snip

Please, disconsider some small errors in the commit message. I have sent the patch files generated befor updating some information, so two of the patches will be updated

This is fine as far as I noticed.

This one refers to VME_BRIDGE somewhere, when it's actually VME_BUS. I'll fix it.

Some changes aren't properly mentioned in the commit message here.

I missed to mention this update on MAINTAINERS, due to changes in the directory structure.

For a patchset it (typically) is OK to put the changelog in the cover letter. See the first patch tutorial for how to format that. That give you a little less editing to do for each individual patch.

I do see that you responded to this cover letter with clarifications. That is fine. Just be aware that it will probably stop reviewers from continuing their review, because you've already alerted them a v3 is in the works. I'm going to go ahead and comment on a few format things in this set, so maybe you can tidy them up in v3.

Alison