diff mbox

[2/4] hvc_dcc: don't ignore errors during initialization

Message ID 1435699387-32591-2-git-send-email-timur@codeaurora.org (mailing list archive)
State New, archived
Headers show

Commit Message

Timur Tabi June 30, 2015, 9:23 p.m. UTC 
hvc_instantiate() and hvc_alloc() return errors if they fail, so don't
ignore them.

Signed-off-by: Timur Tabi <timur@codeaurora.org>
---
 drivers/tty/hvc/hvc_dcc.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

Comments

Stephen Boyd July 1, 2015, 11:54 p.m. UTC | #1 
On 06/30/2015 02:23 PM, Timur Tabi wrote:
> hvc_instantiate() and hvc_alloc() return errors if they fail, so don't
> ignore them.
>
> Signed-off-by: Timur Tabi <timur@codeaurora.org>
> ---
>  drivers/tty/hvc/hvc_dcc.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/tty/hvc/hvc_dcc.c b/drivers/tty/hvc/hvc_dcc.c
> index 33657dc..f8b8cf2 100644
> --- a/drivers/tty/hvc/hvc_dcc.c
> +++ b/drivers/tty/hvc/hvc_dcc.c
> @@ -225,20 +225,29 @@ static const struct hv_ops hvc_dcc_get_put_ops = {
>  
>  static int __init hvc_dcc_console_init(void)
>  {
> +	int ret;
> +
> +	/* This always runs on boot core */
>  	if (!hvc_dcc_check())
>  		return -ENODEV;
>  
> -	hvc_instantiate(0, 0, &hvc_dcc_get_put_ops);
> -	return 0;
> +	/* Returns -1 if error */
> +	ret = hvc_instantiate(0, 0, &hvc_dcc_get_put_ops);
> +
> +	return ret < 0 ? -ENODEV : 0;

Why not just return the value that hvc_instantiate returns? And maybe we
should change those -1 in hvc_instantiate into -EPERM?

>  }
>  console_initcall(hvc_dcc_console_init);
>  
>  static int __init hvc_dcc_init(void)
>  {
> +	struct hvc_struct *p;
> +
> +	/* This can run on any core */
>  	if (!hvc_dcc_check())
>  		return -ENODEV;
>  
> -	hvc_alloc(0, 0, &hvc_dcc_get_put_ops, 128);
> -	return 0;
> +	p = hvc_alloc(0, 0, &hvc_dcc_get_put_ops, 128);
> +
> +	return IS_ERR(p) ? PTR_ERR(p) : 0;

return PTR_ERR_OR_ZERO()?

>  }
>  device_initcall(hvc_dcc_init);
Timur Tabi July 3, 2015, 2:46 p.m. UTC | #2 
On Jul 1, 2015, at 7:54 PM, Stephen Boyd wrote:

>> 
>> +	/* Returns -1 if error */
>> +	ret = hvc_instantiate(0, 0, &hvc_dcc_get_put_ops);
>> +
>> +	return ret < 0 ? -ENODEV : 0;
> 
> Why not just return the value that hvc_instantiate returns? And maybe we
> should change those -1 in hvc_instantiate into -EPERM?

Well, I didn't want to change common HVC code.  I would have to change all the other drivers that call hvc_instatiate as well.  I agree that returning -1 is bad, but fixing that is something that should be done in another patch.

>> 
>> -	hvc_alloc(0, 0, &hvc_dcc_get_put_ops, 128);
>> -	return 0;
>> +	p = hvc_alloc(0, 0, &hvc_dcc_get_put_ops, 128);
>> +
>> +	return IS_ERR(p) ? PTR_ERR(p) : 0;
> 
> return PTR_ERR_OR_ZERO()?

Yes, thanks.  I'll change it.
diff mbox

Patch

diff --git a/drivers/tty/hvc/hvc_dcc.c b/drivers/tty/hvc/hvc_dcc.c
index 33657dc..f8b8cf2 100644
--- a/drivers/tty/hvc/hvc_dcc.c
+++ b/drivers/tty/hvc/hvc_dcc.c
@@ -225,20 +225,29 @@  static const struct hv_ops hvc_dcc_get_put_ops = {
 
 static int __init hvc_dcc_console_init(void)
 {
+	int ret;
+
+	/* This always runs on boot core */
 	if (!hvc_dcc_check())
 		return -ENODEV;
 
-	hvc_instantiate(0, 0, &hvc_dcc_get_put_ops);
-	return 0;
+	/* Returns -1 if error */
+	ret = hvc_instantiate(0, 0, &hvc_dcc_get_put_ops);
+
+	return ret < 0 ? -ENODEV : 0;
 }
 console_initcall(hvc_dcc_console_init);
 
 static int __init hvc_dcc_init(void)
 {
+	struct hvc_struct *p;
+
+	/* This can run on any core */
 	if (!hvc_dcc_check())
 		return -ENODEV;
 
-	hvc_alloc(0, 0, &hvc_dcc_get_put_ops, 128);
-	return 0;
+	p = hvc_alloc(0, 0, &hvc_dcc_get_put_ops, 128);
+
+	return IS_ERR(p) ? PTR_ERR(p) : 0;
 }
 device_initcall(hvc_dcc_init);