2022-04-02 16:18:31

by Bru Moreira-Guedes

[permalink] [raw]
Subject: [PATCH] staging: vme: Adjusted VME_USER in 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
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")

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

Signed-off-by: Bruno Moreira-Guedes <[email protected]>
---
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


2022-04-04 01:01:45

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] staging: vme: Adjusted VME_USER in Kconfig

You sent this twice?

Anyway...

On Fri, Apr 01, 2022 at 03:21:50PM -0300, Bruno wrote:
> With my tests in my, I have found two other things that I think are
> remarkable to mention. First one is a missing `depends on` line for
> `VME_BRIDGE` in drivers/staging/vme/devices/Kconfig, not visible
> because they were in the same tree, but now unveiled. I'm fixing it,
> do you think it's best to add it in the same patch?

Make that a second patch, and resend it as part of a patch series since
your first patch here is gone from my queue.

> Finally, not directly related with the patch, yet remarkable, I
> happened to notice something. When probing the vme_user module
> (compiled with CONFIG_VME_USER=m), I naturally get the following
> messages on my log and command output for `modprobe vme_user`:
> | [177666.590400] vme_user: module is from the staging directory, the
> quality is unknown, you have been warned.

That is expected.

> While this is completely expected, the message about the code from
> staging directory does not appear when compiled with
> CONFIG_VME_USER=y, as shows a `grep -i vme` on the console log:

That is because you built the driver into the tree, so there is nothing
to cause the taint code to run as there is no module loader involved.

It's expected and works the same for all staging drivers. Try it
yourself with a different one to verify this.

> | [0.000000] Linux version 5.17.0lsa-t-vme_user=y-13483-gfeb94431c35c-
> dirty (bruno@AN5Bruno) (gcc (GCC) 11.2.0, GNU ld (GNU Binutils) 2.38)
> #7 SMP PREEMPT_DYNAMIC Fri Apr 1 14:33:16 -03 2022
> | [1.974450] vme_user: VME User Space Access Driver
> | [ 1.975405] vme_user: No cards, skipping registration
>
> Do you think it would be interesting for a future patch to provide
> some output when drivers from the staging tree are present in the
> running kernel image?

If you can figure out how to do so, that would be interesting to see.

thanks,

greg k-h

2022-04-12 21:38:14

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH] staging: vme: Adjusted VME_USER in Kconfig

On Tue, Apr 12, 2022 at 12:14:32PM -0300, Bruno Moreira-Guedes wrote:
> On Sun, Apr 03, 2022 at 01:05:44PM +0200, Greg Kroah-Hartman wrote:
> >
> >On Fri, Apr 01, 2022 at 03:21:50PM -0300, Bruno wrote:
> >> With my tests in my, I have found two other things that I think are
> >> remarkable to mention. First one is a missing `depends on` line for
> >> `VME_BRIDGE` in drivers/staging/vme/devices/Kconfig, not visible
> >> because they were in the same tree, but now unveiled. I'm fixing it,
> >> do you think it's best to add it in the same patch?
> >
> > Make that a second patch, and resend it as part of a patch series since
> > your first patch here is gone from my queue.
>
> This patch is already sent, so I'll trim most of this message to avoid
> duplicating the discussions. There's only one thing I'd like some input
> first, if you don't mind.
>
> >> Do you think it would be interesting for a future patch to provide
> >> some output when drivers from the staging tree are present in the
> >> running kernel image?
> >
> > If you can figure out how to do so, that would be interesting to see.
> I think I might have figured out. In "include/modules.h" and
> "include/init.h" I happened to notice the driver initialization is
> handled by some macros. After some inspection through gcc -E and looking
> how they are defined, I figured out a scenario (when MODULE is not
> defined) where the module_init() macro is defined as (among other
> things) an inline initcall function that wraps the driver initialization
> function.
>
> So I thought about implementing it by creating a -DSTAGING flag in
> drivers/staging/Makefile, and then using this macro to make an #ifdef
> STAGING to add a similar inline wrapping function, except that in this
> case the function makes a pr_warning() before calling the initialization
> function.
>
> Do you think it would be a good way of solving that?

Yes, that would be a possible way, try it and see!

greg k-h

2022-04-12 23:04:09

by Bru Moreira-Guedes

[permalink] [raw]
Subject: Re: [PATCH] staging: vme: Adjusted VME_USER in Kconfig

On Sun, Apr 03, 2022 at 01:05:44PM +0200, Greg Kroah-Hartman wrote:
>
>On Fri, Apr 01, 2022 at 03:21:50PM -0300, Bruno wrote:
>> With my tests in my, I have found two other things that I think are
>> remarkable to mention. First one is a missing `depends on` line for
>> `VME_BRIDGE` in drivers/staging/vme/devices/Kconfig, not visible
>> because they were in the same tree, but now unveiled. I'm fixing it,
>> do you think it's best to add it in the same patch?
>
> Make that a second patch, and resend it as part of a patch series since
> your first patch here is gone from my queue.

This patch is already sent, so I'll trim most of this message to avoid
duplicating the discussions. There's only one thing I'd like some input
first, if you don't mind.

>> Do you think it would be interesting for a future patch to provide
>> some output when drivers from the staging tree are present in the
>> running kernel image?
>
> If you can figure out how to do so, that would be interesting to see.
I think I might have figured out. In "include/modules.h" and
"include/init.h" I happened to notice the driver initialization is
handled by some macros. After some inspection through gcc -E and looking
how they are defined, I figured out a scenario (when MODULE is not
defined) where the module_init() macro is defined as (among other
things) an inline initcall function that wraps the driver initialization
function.

So I thought about implementing it by creating a -DSTAGING flag in
drivers/staging/Makefile, and then using this macro to make an #ifdef
STAGING to add a similar inline wrapping function, except that in this
case the function makes a pr_warning() before calling the initialization
function.

Do you think it would be a good way of solving that?

>
> thanks,
>
> greg k-h

Sincerely,
Bruno


Attachments:
(No filename) (1.81 kB)
signature.asc (235.00 B)
Download all attachments