linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] staging: vme: Adjusted VME_USER in Kconfig
@ 2022-04-01  5:00 Bruno Moreira-Guedes
  2022-04-01  6:08 ` reg Kroah-Hartman
  0 siblings, 1 reply; 8+ messages in thread
From: Bruno Moreira-Guedes @ 2022-04-01  5:00 UTC (permalink / raw)
  To: reg Kroah-Hartman, Martyn Welch, Manohar Vanga, linux-staging,
	linux-kernel, outreachy
  Cc: Bruno Moreira-Guedes

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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] staging: vme: Adjusted VME_USER in Kconfig
  2022-04-01  5:00 [PATCH] staging: vme: Adjusted VME_USER in Kconfig Bruno Moreira-Guedes
@ 2022-04-01  6:08 ` reg Kroah-Hartman
  2022-04-01  6:10   ` reg Kroah-Hartman
  0 siblings, 1 reply; 8+ messages in thread
From: reg Kroah-Hartman @ 2022-04-01  6:08 UTC (permalink / raw)
  To: Bruno Moreira-Guedes
  Cc: Martyn Welch, Manohar Vanga, linux-staging, linux-kernel, outreachy

On Fri, Apr 01, 2022 at 02:00:45AM -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")
> 
> [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
> 
> 

The problem with this change is that you just changed the initialization
order of the drivers if they are built into the kernel.  Are you sure
that you can initialize a vme device driver before the vme bridge and
bus code is run?  I don't know if that will work properly, which is why
the Kconfig entries are in the order they currently are in (we preserved
the link order.)

It's not an obvious thing at all, sorry, but build order defines link
order, which defines the order in which things are initialized in the
kernel.

So I can't take this change unless you are able to prove that it still
works properly on the hardware that these drivers control.  Do you have
this hardware to test this change with?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] staging: vme: Adjusted VME_USER in Kconfig
  2022-04-01  6:08 ` reg Kroah-Hartman
@ 2022-04-01  6:10   ` reg Kroah-Hartman
  2022-04-01 18:21     ` Bruno
  2022-04-01 19:26     ` Bruno Moreira-Guedes
  0 siblings, 2 replies; 8+ messages in thread
From: reg Kroah-Hartman @ 2022-04-01  6:10 UTC (permalink / raw)
  To: Bruno Moreira-Guedes
  Cc: Martyn Welch, Manohar Vanga, linux-staging, linux-kernel, outreachy

On Fri, Apr 01, 2022 at 08:08:17AM +0200, reg Kroah-Hartman wrote:
> On Fri, Apr 01, 2022 at 02:00:45AM -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")
> > 
> > [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
> > 
> > 
> 
> The problem with this change is that you just changed the initialization
> order of the drivers if they are built into the kernel.  Are you sure
> that you can initialize a vme device driver before the vme bridge and
> bus code is run?  I don't know if that will work properly, which is why
> the Kconfig entries are in the order they currently are in (we preserved
> the link order.)
> 
> It's not an obvious thing at all, sorry, but build order defines link
> order, which defines the order in which things are initialized in the
> kernel.
> 
> So I can't take this change unless you are able to prove that it still
> works properly on the hardware that these drivers control.  Do you have
> this hardware to test this change with?

Oh wait, it's the Makefile order that controls this, not the Kconfig
order.  Sorry for the noise here, it's still early...

So this change _should_ be fine, but it would be good if you could prove
it still works with some build tests.  How did you test this change?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] staging: vme: Adjusted VME_USER in Kconfig
  2022-04-01  6:10   ` reg Kroah-Hartman
@ 2022-04-01 18:21     ` Bruno
  2022-04-03 11:05       ` Greg Kroah-Hartman
  2022-04-01 19:26     ` Bruno Moreira-Guedes
  1 sibling, 1 reply; 8+ messages in thread
From: Bruno @ 2022-04-01 18:21 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Martyn Welch, Manohar Vanga, linux-staging, linux-kernel, outreachy

On Fri, 2022-04-01 at 08:10 +0200, reg Kroah-Hartman wrote:
> On Fri, Apr 01, 2022 at 08:08:17AM +0200, reg Kroah-Hartman wrote:
> > On Fri, Apr 01, 2022 at 02:00:45AM -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")
> > > 
> > > [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
> > > 
> > > 
> > 
> > The problem with this change is that you just changed the
> > initialization
> > order of the drivers if they are built into the kernel.  Are you
> > sure
> > that you can initialize a vme device driver before the vme bridge
> > and
> > bus code is run?  I don't know if that will work properly, which
> > is why
> > the Kconfig entries are in the order they currently are in (we
> > preserved
> > the link order.)
> > 
> > It's not an obvious thing at all, sorry, but build order defines
> > link
> > order, which defines the order in which things are initialized in
> > the
> > kernel.
> > 
> > So I can't take this change unless you are able to prove that it
> > still
> > works properly on the hardware that these drivers control.  Do you
> > have
> > this hardware to test this change with?
> 
> Oh wait, it's the Makefile order that controls this, not the Kconfig
> order.  Sorry for the noise here, it's still early...

No worries, your previous message was quite helpful to make realize
some scenarios I wasn't considering at first. I did a more throrough
inspection of how this patch impacts everything thanks to this
observations.

I don't have the hardware so indeed I'm avoiding changes that would
need it to be tested, and as far as I'm properly aware my patch just
changes the places of things in the config targets. Build is protected
from such changes through some Makefile validations such as in
drivers/staging/Makefile:
| obj-$(CONFIG_VME_BUS)           += vme/

> 
> So this change _should_ be fine, but it would be good if you could
> prove it still works with some build tests.  How did you test this
> change?

At first I ran menuconfig and tested if it was still properly setting
CONFIG_VME_USER. Then I built with CONFIG_VME_USER=m and
CONFIG_VME_USER=n to check if it would build the module.

After your first e-mail I realized I didn't account for
CONFIG_VME_USER=y on my tests. I have now successfuly built with this
option too. Are those enough tests for this situation?

> 
> thanks,
> 
> greg k-h
> 

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?

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.
| [177666.601166] vme_user: VME User Space Access Driver
| [177666.602111] vme_user: No cards, skipping registration modprobe:
| ERROR: could not insert 'vme_user': No such device

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:

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

-- 
Sincerely,
Bruno | Pronouns: they/them/theirs
IRC: CodeAgain

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] staging: vme: Adjusted VME_USER in Kconfig
  2022-04-01  6:10   ` reg Kroah-Hartman
  2022-04-01 18:21     ` Bruno
@ 2022-04-01 19:26     ` Bruno Moreira-Guedes
  1 sibling, 0 replies; 8+ messages in thread
From: Bruno Moreira-Guedes @ 2022-04-01 19:26 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Martyn Welch, Manohar Vanga, linux-staging, linux-kernel, outreachy

On Fri, 2022-04-01 at 08:10 +0200, reg Kroah-Hartman wrote:
> On Fri, Apr 01, 2022 at 08:08:17AM +0200, reg Kroah-Hartman wrote:
> > On Fri, Apr 01, 2022 at 02:00:45AM -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")
> > > 
> > > [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
> > > 
> > > 
> > 
> > The problem with this change is that you just changed the
> > initialization
> > order of the drivers if they are built into the kernel.  Are you
> > sure
> > that you can initialize a vme device driver before the vme bridge
> > and
> > bus code is run?  I don't know if that will work properly, which
> > is why
> > the Kconfig entries are in the order they currently are in (we
> > preserved
> > the link order.)
> > 
> > It's not an obvious thing at all, sorry, but build order defines
> > link
> > order, which defines the order in which things are initialized in
> > the
> > kernel.
> > 
> > So I can't take this change unless you are able to prove that it
> > still
> > works properly on the hardware that these drivers control.  Do you
> > have
> > this hardware to test this change with?
> 
> Oh wait, it's the Makefile order that controls this, not the Kconfig
> order.  Sorry for the noise here, it's still early...

No problem, your previous message was quite helpful to make realize
some scenarios I wasn't considering at first. I did a more throrough
inspection of how this patch impacts everything thanks to this
observations.

I don't have the hardware so indeed I'm avoiding changes that would
need it to be tested, and as far as I'm properly aware my patch just
changes the places of things in the config targets. Build is protected
from such changes through some Makefile validations such as in
drivers/staging/Makefile:
| obj-$(CONFIG_VME_BUS)           += vme/

> 
> So this change _should_ be fine, but it would be good if you could
> prove
> it still works with some build tests.  How did you test this change?

At first I ran menuconfig and tested if it was still properly setting
CONFIG_VME_USER. Then I built with CONFIG_VME_USER=m and
CONFIG_VME_USER=n to check if it would build the module.

After your first e-mail I realized I didn't account for
CONFIG_VME_USER=y on my tests. I have now successfuly built with this
option too. Are those enough tests for this situation?

> 
> thanks,
> 
> greg k-h

While testing and checking, 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?

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.
| [177666.601166] vme_user: VME User Space Access Driver
| [177666.602111] vme_user: No cards, skipping registration modprobe:
| ERROR: could not insert 'vme_user': No such device

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:

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

NOTE: This message was sent earlier but due to some deliverability
issues didn't make to destinations. I apologize in advance if it ends
up duplicating.

-- 
Sincerely,
Bruno | Pronouns: they/them/theirs
IRC: CodeAgain

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] staging: vme: Adjusted VME_USER in Kconfig
  2022-04-01 18:21     ` Bruno
@ 2022-04-03 11:05       ` Greg Kroah-Hartman
  2022-04-12 15:14         ` Bruno Moreira-Guedes
  0 siblings, 1 reply; 8+ messages in thread
From: Greg Kroah-Hartman @ 2022-04-03 11:05 UTC (permalink / raw)
  To: Bruno; +Cc: Martyn Welch, Manohar Vanga, linux-staging, linux-kernel, outreachy

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] staging: vme: Adjusted VME_USER in Kconfig
  2022-04-03 11:05       ` Greg Kroah-Hartman
@ 2022-04-12 15:14         ` Bruno Moreira-Guedes
  2022-04-12 15:54           ` Greg Kroah-Hartman
  0 siblings, 1 reply; 8+ messages in thread
From: Bruno Moreira-Guedes @ 2022-04-12 15:14 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Martyn Welch, Manohar Vanga, linux-staging, linux-kernel, outreachy

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

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

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] staging: vme: Adjusted VME_USER in Kconfig
  2022-04-12 15:14         ` Bruno Moreira-Guedes
@ 2022-04-12 15:54           ` Greg Kroah-Hartman
  0 siblings, 0 replies; 8+ messages in thread
From: Greg Kroah-Hartman @ 2022-04-12 15:54 UTC (permalink / raw)
  To: Bruno Moreira-Guedes
  Cc: Martyn Welch, Manohar Vanga, linux-staging, linux-kernel, outreachy

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2022-04-12 15:56 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-01  5:00 [PATCH] staging: vme: Adjusted VME_USER in Kconfig Bruno Moreira-Guedes
2022-04-01  6:08 ` reg Kroah-Hartman
2022-04-01  6:10   ` reg Kroah-Hartman
2022-04-01 18:21     ` Bruno
2022-04-03 11:05       ` Greg Kroah-Hartman
2022-04-12 15:14         ` Bruno Moreira-Guedes
2022-04-12 15:54           ` Greg Kroah-Hartman
2022-04-01 19:26     ` Bruno Moreira-Guedes

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).