[PATCH v1 0/6] console: unify return codes from ->setup() hook

[PATCH v1 0/6] console: unify return codes from ->setup() hook

[Andy Shevchenko]

Some of the console providers treat error code, returned by ->setup() hook,
differently. Here is the unification of the behaviour.

The drivers checked by one of the below criteria:
1/ the driver has explicit struct console .setup assignment
2/ the driver has assigned callback to the setup member

All such drivers were read in order to see if there is any problematic return
codes, and fixed accordingly which is this series in the result.

Andy Shevchenko (6):
  mips: Return proper error code from console ->setup() hook
  serial: sunsab: Return proper error code from console ->setup() hook
  serial: sunzilog: Return proper error code from console ->setup() hook
  tty: hvc: Return proper error code from console ->setup() hook
  console: Propagate error code from console ->setup()
  console: Fix trivia typo 'change' -> 'chance'

 arch/mips/fw/arc/arc_con.c    | 4 +++-
 drivers/tty/hvc/hvsi.c        | 2 +-
 drivers/tty/serial/sunsab.c   | 2 +-
 drivers/tty/serial/sunzilog.c | 2 +-
 kernel/printk/printk.c        | 8 ++++----
 5 files changed, 10 insertions(+), 8 deletions(-)

-- 
2.27.0

[PATCH v1 1/6] mips: Return proper error code from console ->setup() hook

[Andy Shevchenko]

For unifying console ->setup() handling, which is pure documented,
return error code, rather than non-zero arbitrary number.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
---
 arch/mips/fw/arc/arc_con.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/mips/fw/arc/arc_con.c b/arch/mips/fw/arc/arc_con.c
index 365e3913231e..7fdce236b298 100644
--- a/arch/mips/fw/arc/arc_con.c
+++ b/arch/mips/fw/arc/arc_con.c
@@ -28,7 +28,9 @@ static void prom_console_write(struct console *co, const char *s,
 
 static int prom_console_setup(struct console *co, char *options)
 {
-	return !(prom_flags & PROM_FLAG_USE_AS_CONSOLE);
+	if (prom_flags & PROM_FLAG_USE_AS_CONSOLE)
+		return 0;
+	return -ENODEV;
 }
 
 static struct console arc_cons = {
-- 
2.27.0

Re: [PATCH v1 1/6] mips: Return proper error code from console ->setup() hook

[Thomas Bogendoerfer]

On Thu, Jun 18, 2020 at 07:47:46PM +0300, Andy Shevchenko wrote:
> For unifying console ->setup() handling, which is pure documented,
> return error code, rather than non-zero arbitrary number.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>
> ---
>  arch/mips/fw/arc/arc_con.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Acked-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de>

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

Re: [PATCH v1 1/6] mips: Return proper error code from console ->setup() hook

[Jiri Slaby]

On 18. 06. 20, 18:47, Andy Shevchenko wrote:
> For unifying console ->setup() handling, which is pure documented,

Did you mean s/pure/poorly/ in all the patches? Or purely? But then where?

> return error code, rather than non-zero arbitrary number.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>

thanks,
-- 
js
suse labs

Re: [PATCH v1 1/6] mips: Return proper error code from console ->setup() hook

[Andy Shevchenko]

On Mon, Jun 22, 2020 at 5:30 PM Jiri Slaby <jslaby@suse.com> wrote:
>
> On 18. 06. 20, 18:47, Andy Shevchenko wrote:
> > For unifying console ->setup() handling, which is pure documented,
>
> Did you mean s/pure/poorly/ in all the patches? Or purely? But then where?

Poorly.

-- 
With Best Regards,
Andy Shevchenko

[PATCH v1 2/6] serial: sunsab: Return proper error code from console ->setup() hook

[Andy Shevchenko]

For unifying console ->setup() handling, which is pure documented,
return error code, rather than non-zero arbitrary number.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: "David S. Miller" <davem@davemloft.net>
---
 drivers/tty/serial/sunsab.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/sunsab.c b/drivers/tty/serial/sunsab.c
index 1eb703c980e0..bab551f46963 100644
--- a/drivers/tty/serial/sunsab.c
+++ b/drivers/tty/serial/sunsab.c
@@ -886,7 +886,7 @@ static int sunsab_console_setup(struct console *con, char *options)
 	 * though...
 	 */
 	if (up->port.type != PORT_SUNSAB)
-		return -1;
+		return -EINVAL;
 
 	printk("Console: ttyS%d (SAB82532)\n",
 	       (sunsab_reg.minor - 64) + con->index);
-- 
2.27.0

Re: [PATCH v1 2/6] serial: sunsab: Return proper error code from console ->setup() hook

[David Miller]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: Thu, 18 Jun 2020 19:47:47 +0300

> For unifying console ->setup() handling, which is pure documented,
> return error code, rather than non-zero arbitrary number.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: "David S. Miller" <davem@davemloft.net>

Acked-by: David S. Miller <davem@davemloft.net>

[PATCH v1 3/6] serial: sunzilog: Return proper error code from console ->setup() hook

[Andy Shevchenko]

For unifying console ->setup() handling, which is pure documented,
return error code, rather than non-zero arbitrary number.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: "David S. Miller" <davem@davemloft.net>
---
 drivers/tty/serial/sunzilog.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/serial/sunzilog.c b/drivers/tty/serial/sunzilog.c
index 103ab8c556e7..7ea06bbc6197 100644
--- a/drivers/tty/serial/sunzilog.c
+++ b/drivers/tty/serial/sunzilog.c
@@ -1221,7 +1221,7 @@ static int __init sunzilog_console_setup(struct console *con, char *options)
 	int baud, brg;
 
 	if (up->port.type != PORT_SUNZILOG)
-		return -1;
+		return -EINVAL;
 
 	printk(KERN_INFO "Console: ttyS%d (SunZilog zs%d)\n",
 	       (sunzilog_reg.minor - 64) + con->index, con->index);
-- 
2.27.0

Re: [PATCH v1 3/6] serial: sunzilog: Return proper error code from console ->setup() hook

[David Miller]

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: Thu, 18 Jun 2020 19:47:48 +0300

> For unifying console ->setup() handling, which is pure documented,
> return error code, rather than non-zero arbitrary number.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Cc: "David S. Miller" <davem@davemloft.net>

Acked-by: David S. Miller <davem@davemloft.net>

[PATCH v1 4/6] tty: hvc: Return proper error code from console ->setup() hook

[Andy Shevchenko]

For unifying console ->setup() handling, which is pure documented,
return error code, rather than non-zero arbitrary number.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/tty/hvc/hvsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/hvc/hvsi.c b/drivers/tty/hvc/hvsi.c
index 66f95f758be0..e8c58f9bd263 100644
--- a/drivers/tty/hvc/hvsi.c
+++ b/drivers/tty/hvc/hvsi.c
@@ -1128,7 +1128,7 @@ static int __init hvsi_console_setup(struct console *console, char *options)
 	int ret;
 
 	if (console->index < 0 || console->index >= hvsi_count)
-		return -1;
+		return -EINVAL;
 	hp = &hvsi_ports[console->index];
 
 	/* give the FSP a chance to change the baud rate when we re-open */
-- 
2.27.0

[PATCH v1 5/6] console: Propagate error code from console ->setup()

[Andy Shevchenko]

Since console ->setup() hook returns meaningful error codes,
propagate it to the caller of try_enable_new_console().

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 kernel/printk/printk.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 8c14835be46c..aaea3ad182e1 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2668,7 +2668,7 @@ early_param("keep_bootcon", keep_bootcon_setup);
 static int try_enable_new_console(struct console *newcon, bool user_specified)
 {
 	struct console_cmdline *c;
-	int i;
+	int i, err;
 
 	for (i = 0, c = console_cmdline;
 	     i < MAX_CMDLINECONSOLES && c->name[0];
@@ -2691,8 +2691,8 @@ static int try_enable_new_console(struct console *newcon, bool user_specified)
 				return 0;
 
 			if (newcon->setup &&
-			    newcon->setup(newcon, c->options) != 0)
-				return -EIO;
+			    (err = newcon->setup(newcon, c->options)) != 0)
+				return err;
 		}
 		newcon->flags |= CON_ENABLED;
 		if (i == preferred_console) {
-- 
2.27.0

Re: [PATCH v1 5/6] console: Propagate error code from console ->setup()

[Benjamin Herrenschmidt]

On Thu, 2020-06-18 at 19:47 +0300, Andy Shevchenko wrote:
> Since console ->setup() hook returns meaningful error codes,
> propagate it to the caller of try_enable_new_console().
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> ---A
>  kernel/printk/printk.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index 8c14835be46c..aaea3ad182e1 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2668,7 +2668,7 @@ early_param("keep_bootcon", keep_bootcon_setup);
>  static int try_enable_new_console(struct console *newcon, bool user_specified)
>  {
>  	struct console_cmdline *c;
> -	int i;
> +	int i, err;
>  
>  	for (i = 0, c = console_cmdline;
>  	     i < MAX_CMDLINECONSOLES && c->name[0];
> @@ -2691,8 +2691,8 @@ static int try_enable_new_console(struct console *newcon, bool user_specified)
>  				return 0;
>  
>  			if (newcon->setup &&
> -			    newcon->setup(newcon, c->options) != 0)
> -				return -EIO;
> +			    (err = newcon->setup(newcon, c->options)) != 0)
> +				return err;
>  		}
>  		newcon->flags |= CON_ENABLED;
>  		if (i == preferred_console) {

[PATCH v1 6/6] console: Fix trivia typo 'change' -> 'chance'

[Andy Shevchenko]

I bet the word 'chance' has to be used in 'had a chance to be called',
but, alas, I'm not native speaker...

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---
 kernel/printk/printk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index aaea3ad182e1..6623e975675a 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -2705,7 +2705,7 @@ static int try_enable_new_console(struct console *newcon, bool user_specified)
 	/*
 	 * Some consoles, such as pstore and netconsole, can be enabled even
 	 * without matching. Accept the pre-enabled consoles only when match()
-	 * and setup() had a change to be called.
+	 * and setup() had a chance to be called.
 	 */
 	if (newcon->flags & CON_ENABLED && c->user_specified ==	user_specified)
 		return 0;
-- 
2.27.0

Re: [PATCH v1 6/6] console: Fix trivia typo 'change' -> 'chance'

[Benjamin Herrenschmidt]

On Thu, 2020-06-18 at 19:47 +0300, Andy Shevchenko wrote:
> I bet the word 'chance' has to be used in 'had a chance to be called',
> but, alas, I'm not native speaker...

Yup, typo :)

> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> 

Acked-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>

> ---
>  kernel/printk/printk.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
> index aaea3ad182e1..6623e975675a 100644
> --- a/kernel/printk/printk.c
> +++ b/kernel/printk/printk.c
> @@ -2705,7 +2705,7 @@ static int try_enable_new_console(struct console *newcon, bool user_specified)
>  	/*
>  	 * Some consoles, such as pstore and netconsole, can be enabled even
>  	 * without matching. Accept the pre-enabled consoles only when match()
> -	 * and setup() had a change to be called.
> +	 * and setup() had a chance to be called.
>  	 */
>  	if (newcon->flags & CON_ENABLED && c->user_specified ==	user_specified)
>  		return 0;

Re: [PATCH v1 0/6] console: unify return codes from ->setup() hook

[Sergey Senozhatsky]

On (20/06/18 19:47), Andy Shevchenko wrote:
> Some of the console providers treat error code, returned by ->setup() hook,
> differently. Here is the unification of the behaviour.
> 
> The drivers checked by one of the below criteria:
> 1/ the driver has explicit struct console .setup assignment
> 2/ the driver has assigned callback to the setup member
> 
> All such drivers were read in order to see if there is any problematic return
> codes, and fixed accordingly which is this series in the result.
> 
> Andy Shevchenko (6):
>   mips: Return proper error code from console ->setup() hook
>   serial: sunsab: Return proper error code from console ->setup() hook
>   serial: sunzilog: Return proper error code from console ->setup() hook
>   tty: hvc: Return proper error code from console ->setup() hook
>   console: Propagate error code from console ->setup()
>   console: Fix trivia typo 'change' -> 'chance'
> 
>  arch/mips/fw/arc/arc_con.c    | 4 +++-
>  drivers/tty/hvc/hvsi.c        | 2 +-
>  drivers/tty/serial/sunsab.c   | 2 +-
>  drivers/tty/serial/sunzilog.c | 2 +-
>  kernel/printk/printk.c        | 8 ++++----
>  5 files changed, 10 insertions(+), 8 deletions(-)

Looks good to me. I'd also, probably, unify the naming. I can grep
71 foo_console_setup() and only 1 foo_setup_console().

---
 drivers/tty/hvc/hvc_xen.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
index 5ef08905fe05..2a0e51a20e34 100644
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -603,7 +603,7 @@ static void xen_hvm_early_write(uint32_t vtermno, const char *str, int len) { }
 #endif
 
 #ifdef CONFIG_EARLY_PRINTK
-static int __init xenboot_setup_console(struct console *console, char *string)
+static int __init xenboot_console_setup(struct console *console, char *string)
 {
 	static struct xencons_info xenboot;
 
@@ -647,7 +647,7 @@ static void xenboot_write_console(struct console *console, const char *string,
 struct console xenboot_console = {
 	.name		= "xenboot",
 	.write		= xenboot_write_console,
-	.setup		= xenboot_setup_console,
+	.setup		= xenboot_console_setup,
 	.flags		= CON_PRINTBUFFER | CON_BOOT | CON_ANYTIME,
 	.index		= -1,
 };
-- 
2.27.0

Re: [PATCH v1 0/6] console: unify return codes from ->setup() hook

[Petr Mladek]

On Thu 2020-06-18 19:47:45, Andy Shevchenko wrote:
> Some of the console providers treat error code, returned by ->setup() hook,
> differently. Here is the unification of the behaviour.
> 
> The drivers checked by one of the below criteria:
> 1/ the driver has explicit struct console .setup assignment
> 2/ the driver has assigned callback to the setup member
> 
> All such drivers were read in order to see if there is any problematic return
> codes, and fixed accordingly which is this series in the result.
> 
> Andy Shevchenko (6):
>   mips: Return proper error code from console ->setup() hook
>   serial: sunsab: Return proper error code from console ->setup() hook
>   serial: sunzilog: Return proper error code from console ->setup() hook
>   tty: hvc: Return proper error code from console ->setup() hook
>   console: Propagate error code from console ->setup()
>   console: Fix trivia typo 'change' -> 'chance'

For the entire patchset:

Reviewed-by: Petr Mladek <pmladek@suse.com>

I am going to push it the following week via printk tree unless
anybody complains.

Best Regards,
Petr

Re: [PATCH v1 0/6] console: unify return codes from ->setup() hook

[Andy Shevchenko]

On Fri, Jun 19, 2020 at 11:39:18AM +0200, Petr Mladek wrote:
> On Thu 2020-06-18 19:47:45, Andy Shevchenko wrote:
> > Some of the console providers treat error code, returned by ->setup() hook,
> > differently. Here is the unification of the behaviour.
> > 
> > The drivers checked by one of the below criteria:
> > 1/ the driver has explicit struct console .setup assignment
> > 2/ the driver has assigned callback to the setup member
> > 
> > All such drivers were read in order to see if there is any problematic return
> > codes, and fixed accordingly which is this series in the result.
> > 
> > Andy Shevchenko (6):
> >   mips: Return proper error code from console ->setup() hook
> >   serial: sunsab: Return proper error code from console ->setup() hook
> >   serial: sunzilog: Return proper error code from console ->setup() hook
> >   tty: hvc: Return proper error code from console ->setup() hook
> >   console: Propagate error code from console ->setup()
> >   console: Fix trivia typo 'change' -> 'chance'
> 
> For the entire patchset:
> 
> Reviewed-by: Petr Mladek <pmladek@suse.com>
> 
> I am going to push it the following week via printk tree unless
> anybody complains.

Thanks, Petr, I guess you may also incorporate Sergey's patch he proposed. Sergey, are you going to submit it officially?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 0/6] console: unify return codes from ->setup() hook

[Sergey Senozhatsky]

On (20/06/19 12:47), Andy Shevchenko wrote:
> On Fri, Jun 19, 2020 at 11:39:18AM +0200, Petr Mladek wrote:
> > On Thu 2020-06-18 19:47:45, Andy Shevchenko wrote:
[..]
> > I am going to push it the following week via printk tree unless
> > anybody complains.
> 
> Thanks, Petr, I guess you may also incorporate Sergey's patch
> he proposed. Sergey, are you going to submit it officially?

I can send a patch, unless Petr has objections.

	-ss

Re: [PATCH v1 0/6] console: unify return codes from ->setup() hook

[Petr Mladek]

On Fri 2020-06-19 19:21:32, Sergey Senozhatsky wrote:
> On (20/06/19 12:47), Andy Shevchenko wrote:
> > On Fri, Jun 19, 2020 at 11:39:18AM +0200, Petr Mladek wrote:
> > > On Thu 2020-06-18 19:47:45, Andy Shevchenko wrote:
> [..]
> > > I am going to push it the following week via printk tree unless
> > > anybody complains.
> > 
> > Thanks, Petr, I guess you may also incorporate Sergey's patch
> > he proposed. Sergey, are you going to submit it officially?
> 
> I can send a patch, unless Petr has objections.

Go ahead :-)

Best Regards,
Petr

Re: [PATCH v1 0/6] console: unify return codes from ->setup() hook

[Sergey Senozhatsky]

On (20/06/19 13:26), Petr Mladek wrote:
> On Fri 2020-06-19 19:21:32, Sergey Senozhatsky wrote:
> > On (20/06/19 12:47), Andy Shevchenko wrote:
> > > On Fri, Jun 19, 2020 at 11:39:18AM +0200, Petr Mladek wrote:
> > > > On Thu 2020-06-18 19:47:45, Andy Shevchenko wrote:
> > [..]
> > > > I am going to push it the following week via printk tree unless
> > > > anybody complains.
> > > 
> > > Thanks, Petr, I guess you may also incorporate Sergey's patch
> > > he proposed. Sergey, are you going to submit it officially?
> > 
> > I can send a patch, unless Petr has objections.
> 
> Go ahead :-)

Oops, I just sent it, but I forgot to specify in-reply-to message-id :(

	-ss

Re: [PATCH v1 0/6] console: unify return codes from ->setup() hook

[Petr Mladek]

On Thu 2020-06-18 19:47:45, Andy Shevchenko wrote:
> Some of the console providers treat error code, returned by ->setup() hook,
> differently. Here is the unification of the behaviour.
> 
> The drivers checked by one of the below criteria:
> 1/ the driver has explicit struct console .setup assignment
> 2/ the driver has assigned callback to the setup member
> 
> All such drivers were read in order to see if there is any problematic return
> codes, and fixed accordingly which is this series in the result.
> 
> Andy Shevchenko (6):
>   mips: Return proper error code from console ->setup() hook
>   serial: sunsab: Return proper error code from console ->setup() hook
>   serial: sunzilog: Return proper error code from console ->setup() hook
>   tty: hvc: Return proper error code from console ->setup() hook
>   console: Propagate error code from console ->setup()
>   console: Fix trivia typo 'change' -> 'chance'

The entire patchset has been pushed into printk/linux.git,
branch for-5.9-console-return-codes.

I have done "s/pure/poorly/" to fix the typo in commit messages
reported by Jiri Slaby.

Best Regards,
Petr