The braille_unregister_console() already tests for console to have
CON_BRL flag. No need to repeat this in _braille_unregister_console().
However, we need to check for that flag at the beginning of the function
to avoid any regressions in the callers.
Signed-off-by: Andy Shevchenko <[email protected]>
---
v2: new patch
drivers/accessibility/braille/braille_console.c | 4 ++--
kernel/printk/braille.c | 5 +----
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/accessibility/braille/braille_console.c b/drivers/accessibility/braille/braille_console.c
index 1339c586bf64..f0ffccfb6bb7 100644
--- a/drivers/accessibility/braille/braille_console.c
+++ b/drivers/accessibility/braille/braille_console.c
@@ -369,10 +369,10 @@ int braille_register_console(struct console *console, int index,
int braille_unregister_console(struct console *console)
{
- if (braille_co != console)
- return -EINVAL;
if (!(console->flags & CON_BRL))
return 0;
+ if (braille_co != console)
+ return -EINVAL;
unregister_keyboard_notifier(&keyboard_notifier_block);
unregister_vt_notifier(&vt_notifier_block);
braille_co = NULL;
diff --git a/kernel/printk/braille.c b/kernel/printk/braille.c
index 17a9591e54ff..2ec42173890f 100644
--- a/kernel/printk/braille.c
+++ b/kernel/printk/braille.c
@@ -51,8 +51,5 @@ _braille_register_console(struct console *console, struct console_cmdline *c)
int
_braille_unregister_console(struct console *console)
{
- if (console->flags & CON_BRL)
- return braille_unregister_console(console);
-
- return 0;
+ return braille_unregister_console(console);
}
--
2.24.1
Some consoles might require special operations on unregistering. For example,
serial console, when registered in the kernel, keeps power on for entire time,
until it gets unregistered. For such cases to have a balance we would provide
->exit() callback.
Signed-off-by: Andy Shevchenko <[email protected]>
---
v2: new patch
include/linux/console.h | 1 +
kernel/printk/printk.c | 3 +++
2 files changed, 4 insertions(+)
diff --git a/include/linux/console.h b/include/linux/console.h
index f33016b3a401..54759ad0c36b 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -148,6 +148,7 @@ struct console {
struct tty_driver *(*device)(struct console *, int *);
void (*unblank)(void);
int (*setup)(struct console *, char *);
+ void (*exit)(struct console *);
int (*match)(struct console *, char *name, int idx, char *options);
short flags;
short index;
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index da6a9bdf76b6..0319bb698d05 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2850,6 +2850,9 @@ int unregister_console(struct console *console)
if (console_drivers != NULL && console->flags & CON_CONSDEV)
console_drivers->flags |= CON_CONSDEV;
+ if (console->exit)
+ console->exit(console);
+
console->flags &= ~CON_ENABLED;
console_unlock();
console_sysfs_notify();
--
2.24.1
On (20/01/24 17:57), Andy Shevchenko wrote:
[..]
> +++ b/include/linux/console.h
> @@ -148,6 +148,7 @@ struct console {
> struct tty_driver *(*device)(struct console *, int *);
> void (*unblank)(void);
> int (*setup)(struct console *, char *);
> + void (*exit)(struct console *);
> int (*match)(struct console *, char *name, int idx, char *options);
> short flags;
> short index;
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index da6a9bdf76b6..0319bb698d05 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2850,6 +2850,9 @@ int unregister_console(struct console *console)
> if (console_drivers != NULL && console->flags & CON_CONSDEV)
> console_drivers->flags |= CON_CONSDEV;
>
> + if (console->exit)
> + console->exit(console);
> +
> console->flags &= ~CON_ENABLED;
> console_unlock();
> console_sysfs_notify();
Do you need to call ->exit() under console_lock?
-ss
On (20/01/24 17:57), Andy Shevchenko wrote:
[..]
> +++ b/drivers/accessibility/braille/braille_console.c
> @@ -369,10 +369,10 @@ int braille_register_console(struct console *console, int index,
>
> int braille_unregister_console(struct console *console)
> {
> - if (braille_co != console)
> - return -EINVAL;
> if (!(console->flags & CON_BRL))
> return 0;
> + if (braille_co != console)
> + return -EINVAL;
> unregister_keyboard_notifier(&keyboard_notifier_block);
> unregister_vt_notifier(&vt_notifier_block);
> braille_co = NULL;
> diff --git a/kernel/printk/braille.c b/kernel/printk/braille.c
> index 17a9591e54ff..2ec42173890f 100644
> --- a/kernel/printk/braille.c
> +++ b/kernel/printk/braille.c
> @@ -51,8 +51,5 @@ _braille_register_console(struct console *console, struct console_cmdline *c)
> int
> _braille_unregister_console(struct console *console)
> {
> - if (console->flags & CON_BRL)
> - return braille_unregister_console(console);
> -
> - return 0;
> + return braille_unregister_console(console);
> }
Hmm, I don't know. This moves sort of important code from common upper
layer down to particular driver implementation. Should there be another
driver/super-braille.c it must test CON_BRL flag as well.
Because printk invokes _braille_unregister_console() unconditionally,
and _braille_unregister_console() unconditionally calls into the driver.
I guess we can remove CON_BRL from braille_unregister_console() instead,
because printk tests it.
-ss
On Mon, Jan 27, 2020 at 11:41:14AM +0900, Sergey Senozhatsky wrote:
> On (20/01/24 17:57), Andy Shevchenko wrote:
> [..]
> > +++ b/drivers/accessibility/braille/braille_console.c
> > @@ -369,10 +369,10 @@ int braille_register_console(struct console *console, int index,
> >
> > int braille_unregister_console(struct console *console)
> > {
> > - if (braille_co != console)
> > - return -EINVAL;
> > if (!(console->flags & CON_BRL))
> > return 0;
> > + if (braille_co != console)
> > + return -EINVAL;
> > unregister_keyboard_notifier(&keyboard_notifier_block);
> > unregister_vt_notifier(&vt_notifier_block);
> > braille_co = NULL;
> > diff --git a/kernel/printk/braille.c b/kernel/printk/braille.c
> > index 17a9591e54ff..2ec42173890f 100644
> > --- a/kernel/printk/braille.c
> > +++ b/kernel/printk/braille.c
> > @@ -51,8 +51,5 @@ _braille_register_console(struct console *console, struct console_cmdline *c)
> > int
> > _braille_unregister_console(struct console *console)
> > {
> > - if (console->flags & CON_BRL)
> > - return braille_unregister_console(console);
> > -
> > - return 0;
> > + return braille_unregister_console(console);
> > }
>
> Hmm, I don't know. This moves sort of important code from common upper
> layer down to particular driver implementation. Should there be another
> driver/super-braille.c it must test CON_BRL flag as well.
> Because printk invokes _braille_unregister_console() unconditionally,
> and _braille_unregister_console() unconditionally calls into the driver.
>
> I guess we can remove CON_BRL from braille_unregister_console() instead,
> because printk tests it.
Let me do that way, thanks!
--
With Best Regards,
Andy Shevchenko
On Mon, Jan 27, 2020 at 11:22:20AM +0900, Sergey Senozhatsky wrote:
> On (20/01/24 17:57), Andy Shevchenko wrote:
> [..]
> > +++ b/include/linux/console.h
> > @@ -148,6 +148,7 @@ struct console {
> > struct tty_driver *(*device)(struct console *, int *);
> > void (*unblank)(void);
> > int (*setup)(struct console *, char *);
> > + void (*exit)(struct console *);
> > int (*match)(struct console *, char *name, int idx, char *options);
> > short flags;
> > short index;
> > diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> > index da6a9bdf76b6..0319bb698d05 100644
> > --- a/kernel/printk/printk.c
> > +++ b/kernel/printk/printk.c
> > @@ -2850,6 +2850,9 @@ int unregister_console(struct console *console)
> > if (console_drivers != NULL && console->flags & CON_CONSDEV)
> > console_drivers->flags |= CON_CONSDEV;
> >
> > + if (console->exit)
> > + console->exit(console);
> > +
> > console->flags &= ~CON_ENABLED;
> > console_unlock();
> > console_sysfs_notify();
>
> Do you need to call ->exit() under console_lock?
I think we don't need that lock to be held.
--
With Best Regards,
Andy Shevchenko