[PATCH v2 0/3] staging: vme: Restructuring menuconfig and tree

[PATCH v2 0/3] staging: vme: Restructuring menuconfig and tree

[Bruno Moreira-Guedes]

[-- Attachment #1: Type: text/plain, Size: 1438 bytes --]

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).

Signed-off-by: Bruno Moreira-Guedes <codeagain@codeagain.dev>

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


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 1/3] staging: vme: Adjusted VME_USER in Kconfig

[Bruno Moreira-Guedes]

[-- Attachment #1: Type: text/plain, Size: 2354 bytes --]

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

Signed-off-by: Bruno Moreira-Guedes <codeagain@codeagain.dev>
---
 drivers/staging/Kconfig | 2 ++
 drivers/vme/Kconfig     | 2 --
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index 932acb4e8cbc..0545850eb2ff 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -88,4 +88,6 @@ source "drivers/staging/qlge/Kconfig"
 
 source "drivers/staging/wfx/Kconfig"
 
+source "drivers/staging/vme/devices/Kconfig"
+
 endif # STAGING
diff --git a/drivers/vme/Kconfig b/drivers/vme/Kconfig
index 936392ca3c8c..c13dd9d2a604 100644
--- a/drivers/vme/Kconfig
+++ b/drivers/vme/Kconfig
@@ -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


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 2/3] staging: vme: Fix missing `depends on` at KConfig

[Bruno Moreira-Guedes]

[-- Attachment #1: Type: text/plain, Size: 1869 bytes --]

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/3fbc5325e94b9ae0666a1f5a56a4e5372bfcea1d.camel@codeagain.dev/T/#t
[2] Tested both as module and as built-in.

Signed-off-by: Bruno Moreira-Guedes <codeagain@codeagain.dev>
---
 drivers/staging/vme/devices/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vme/devices/Kconfig b/drivers/staging/vme/devices/Kconfig
index 5651bb16b28b..e8b4461bf27f 100644
--- a/drivers/staging/vme/devices/Kconfig
+++ b/drivers/staging/vme/devices/Kconfig
@@ -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


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[PATCH v2 3/3] staging: vme: "drivers/staging/vme" tree cleanup

[Bruno Moreira-Guedes]

[-- Attachment #1: Type: text/plain, Size: 4544 bytes --]

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

Signed-off-by: Bruno Moreira-Guedes <codeagain@codeagain.dev>
---
 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%)

diff --git a/MAINTAINERS b/MAINTAINERS
index fd768d43e048..88423218e5e6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21014,7 +21014,7 @@ L:	linux-kernel@vger.kernel.org
 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*
 
diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index 0545850eb2ff..9f11592336f8 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -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
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index 3ffb35ccfae2..1dffa02121ee 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -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/
diff --git a/drivers/staging/vme/Makefile b/drivers/staging/vme/Makefile
deleted file mode 100644
index cf2f686ccffe..000000000000
--- a/drivers/staging/vme/Makefile
+++ /dev/null
@@ -1,2 +0,0 @@
-# SPDX-License-Identifier: GPL-2.0
-obj-y				+= devices/
diff --git a/drivers/staging/vme/devices/Kconfig b/drivers/staging/vme_user/Kconfig
similarity index 100%
rename from drivers/staging/vme/devices/Kconfig
rename to drivers/staging/vme_user/Kconfig
diff --git a/drivers/staging/vme/devices/Makefile b/drivers/staging/vme_user/Makefile
similarity index 100%
rename from drivers/staging/vme/devices/Makefile
rename to drivers/staging/vme_user/Makefile
diff --git a/drivers/staging/vme/devices/vme_user.c b/drivers/staging/vme_user/vme_user.c
similarity index 100%
rename from drivers/staging/vme/devices/vme_user.c
rename to drivers/staging/vme_user/vme_user.c
diff --git a/drivers/staging/vme/devices/vme_user.h b/drivers/staging/vme_user/vme_user.h
similarity index 100%
rename from drivers/staging/vme/devices/vme_user.h
rename to drivers/staging/vme_user/vme_user.h
-- 
2.35.1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 0/3] staging: vme: Restructuring menuconfig and tree

[Bruno Moreira-Guedes]

[-- Attachment #1: Type: text/plain, Size: 2036 bytes --]

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

On Tue, Apr 12, 2022 at 03:53:46AM -0300, Bruno Moreira-Guedes wrote:
> 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).
> 
> Signed-off-by: Bruno Moreira-Guedes <codeagain@codeagain.dev>
> 
> Bruno Moreira-Guedes (3):
>   staging: vme: Adjusted VME_USER in Kconfig
This is fine as far as I noticed.

>   staging: vme: Fix missing `depends on` at KConfig
This one refers to VME_BRIDGE somewhere, when it's actually VME_BUS.
I'll fix it.

>   staging: vme: "drivers/staging/vme" tree cleanup
Some changes aren't properly mentioned in the commit message here.

> 
>  MAINTAINERS                                          | 2 +-
I missed to mention this update on MAINTAINERS, due to changes in the
directory structure.

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


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 1/3] staging: vme: Adjusted VME_USER in Kconfig

[Greg Kroah-Hartman]

On Tue, Apr 12, 2022 at 03:55:21AM -0300, Bruno Moreira-Guedes wrote:
> 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.


Changelog goes below the --- line please.

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

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

Re: [PATCH v2 0/3] staging: vme: Restructuring menuconfig and tree

[Alison Schofield]

On Tue, Apr 12, 2022 at 03:53:46AM -0300, Bruno Moreira-Guedes wrote:
> 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).

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

> 
> Signed-off-by: Bruno Moreira-Guedes <codeagain@codeagain.dev>
> 
> 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
> 

Re: [PATCH v2 1/3] staging: vme: Adjusted VME_USER in Kconfig

[Alison Schofield]

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'.  

> 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

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.

> 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
> 
> Signed-off-by: Bruno Moreira-Guedes <codeagain@codeagain.dev>
> ---
>  drivers/staging/Kconfig | 2 ++
>  drivers/vme/Kconfig     | 2 --
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
> index 932acb4e8cbc..0545850eb2ff 100644
> --- a/drivers/staging/Kconfig
> +++ b/drivers/staging/Kconfig
> @@ -88,4 +88,6 @@ source "drivers/staging/qlge/Kconfig"
>  
>  source "drivers/staging/wfx/Kconfig"
>  
> +source "drivers/staging/vme/devices/Kconfig"
> +
>  endif # STAGING
> diff --git a/drivers/vme/Kconfig b/drivers/vme/Kconfig
> index 936392ca3c8c..c13dd9d2a604 100644
> --- a/drivers/vme/Kconfig
> +++ b/drivers/vme/Kconfig
> @@ -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
> 

Re: [PATCH v2 2/3] staging: vme: Fix missing `depends on` at KConfig

[Alison Schofield]

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

> [PATCH v2 2/3] staging: vme: Fix missing `depends on` at KConfig

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

That's all!

snip

Re: [PATCH v2 3/3] staging: vme: "drivers/staging/vme" tree cleanup

[Alison Schofield]

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

> [PATCH v2 3/3] staging: vme: "drivers/staging/vme" tree cleanup

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
>