[PATCH] tty/hvc_opal: simplify if-if to if-else

[PATCH] tty/hvc_opal: simplify if-if to if-else

[Wan Jiabing]

Use if and else instead of if(A) and if (!A).

Signed-off-by: Wan Jiabing <wanjiabing@vivo.com>
---
 drivers/tty/hvc/hvc_opal.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/hvc/hvc_opal.c b/drivers/tty/hvc/hvc_opal.c
index 84776bc641e6..2dafa0964c2a 100644
--- a/drivers/tty/hvc/hvc_opal.c
+++ b/drivers/tty/hvc/hvc_opal.c
@@ -344,14 +344,15 @@ void __init hvc_opal_init_early(void)
 		opal = of_find_node_by_path("/ibm,opal/consoles");
 		if (opal)
 			pr_devel("hvc_opal: Found consoles in new location\n");
-		if (!opal) {
+		else {
 			opal = of_find_node_by_path("/ibm,opal");
 			if (opal)
 				pr_devel("hvc_opal: "
 					 "Found consoles in old location\n");
+			else
+				return;
 		}
-		if (!opal)
-			return;
+
 		for_each_child_of_node(opal, np) {
 			if (of_node_name_eq(np, "serial")) {
 				stdout_node = np;
-- 
2.35.1

Re: [PATCH] tty/hvc_opal: simplify if-if to if-else

[Joe Perches]

On Sun, 2022-04-24 at 17:25 +0800, Wan Jiabing wrote:
> Use if and else instead of if(A) and if (!A).
[]
> diff --git a/drivers/tty/hvc/hvc_opal.c b/drivers/tty/hvc/hvc_opal.c
[]
> @@ -344,14 +344,15 @@ void __init hvc_opal_init_early(void)
>  		opal = of_find_node_by_path("/ibm,opal/consoles");
>  		if (opal)
>  			pr_devel("hvc_opal: Found consoles in new location\n");
> -		if (!opal) {
> +		else {
>  			opal = of_find_node_by_path("/ibm,opal");
>  			if (opal)
>  				pr_devel("hvc_opal: "
>  					 "Found consoles in old location\n");
> +			else
> +				return;

A few things:

o add {} braces to first block before else
o see about using pr_fmt to prefix the pr_<level> output
o reverse the test and unindent the pr_devel

			if (!opal)
				return;
			pr_devel("...);

Maybe a few more just to quiet checkpatch noise.  Something like:
---
 drivers/tty/hvc/hvc_opal.c | 58 +++++++++++++++++++++++++---------------------
 1 file changed, 31 insertions(+), 27 deletions(-)

diff --git a/drivers/tty/hvc/hvc_opal.c b/drivers/tty/hvc/hvc_opal.c
index 84776bc641e6b..a42d5697ae198 100644
--- a/drivers/tty/hvc/hvc_opal.c
+++ b/drivers/tty/hvc/hvc_opal.c
@@ -5,6 +5,8 @@
  * Copyright 2011 Benjamin Herrenschmidt <benh@kernel.crashing.org>, IBM Corp.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #undef DEBUG
 
 #include <linux/types.h>
@@ -43,6 +45,7 @@ struct hvc_opal_priv {
 	hv_protocol_t		proto;	/* Raw data or HVSI packets */
 	struct hvsi_priv	hvsi;	/* HVSI specific data */
 };
+
 static struct hvc_opal_priv *hvc_opal_privs[MAX_NR_HVC_CONSOLES];
 
 /* For early boot console */
@@ -124,7 +127,7 @@ static int hvc_opal_hvsi_tiocmget(struct hvc_struct *hp)
 }
 
 static int hvc_opal_hvsi_tiocmset(struct hvc_struct *hp, unsigned int set,
-				unsigned int clear)
+				  unsigned int clear)
 {
 	struct hvc_opal_priv *pv = hvc_opal_privs[hp->vtermno];
 
@@ -167,8 +170,7 @@ static int hvc_opal_probe(struct platform_device *dev)
 		proto = HV_PROTOCOL_HVSI;
 		ops = &hvc_opal_hvsi_ops;
 	} else {
-		pr_err("hvc_opal: Unknown protocol for %pOF\n",
-		       dev->dev.of_node);
+		pr_err("Unknown protocol for %pOF\n", dev->dev.of_node);
 		return -ENXIO;
 	}
 
@@ -195,15 +197,16 @@ static int hvc_opal_probe(struct platform_device *dev)
 				     termno, 0);
 		}
 
-		/* Instanciate now to establish a mapping index==vtermno */
+		/* Instantiate now to establish a mapping index==vtermno */
 		hvc_instantiate(termno, termno, ops);
 	} else {
-		pr_err("hvc_opal: Device %pOF has duplicate terminal number #%d\n",
+		pr_err("Device %pOF has duplicate terminal number #%d\n",
 		       dev->dev.of_node, termno);
 		return -ENXIO;
 	}
 
-	pr_info("hvc%d: %s protocol on %pOF%s\n", termno,
+	pr_info("hvc%d: %s protocol on %pOF%s\n",
+		termno,
 		proto == HV_PROTOCOL_RAW ? "raw" : "hvsi",
 		dev->dev.of_node,
 		boot ? " (boot console)" : "");
@@ -211,13 +214,13 @@ static int hvc_opal_probe(struct platform_device *dev)
 	irq = irq_of_parse_and_map(dev->dev.of_node, 0);
 	if (!irq) {
 		pr_info("hvc%d: No interrupts property, using OPAL event\n",
-				termno);
+			termno);
 		irq = opal_event_request(ilog2(OPAL_EVENT_CONSOLE_INPUT));
 	}
 
 	if (!irq) {
-		pr_err("hvc_opal: Unable to map interrupt for device %pOF\n",
-			dev->dev.of_node);
+		pr_err("Unable to map interrupt for device %pOF\n",
+		       dev->dev.of_node);
 		return irq;
 	}
 
@@ -275,7 +278,7 @@ static void udbg_opal_putc(char c)
 		udbg_opal_putc('\r');
 
 	do {
-		switch(hvc_opal_boot_priv.proto) {
+		switch (hvc_opal_boot_priv.proto) {
 		case HV_PROTOCOL_RAW:
 			count = opal_put_chars(termno, &c, 1);
 			break;
@@ -288,7 +291,7 @@ static void udbg_opal_putc(char c)
 		 * when there aren't any interrupts.
 		 */
 		opal_flush_console(termno);
-	} while(count == 0 || count == -EAGAIN);
+	} while (count == 0 || count == -EAGAIN);
 }
 
 static int udbg_opal_getc_poll(void)
@@ -297,7 +300,7 @@ static int udbg_opal_getc_poll(void)
 	int rc = 0;
 	char c;
 
-	switch(hvc_opal_boot_priv.proto) {
+	switch (hvc_opal_boot_priv.proto) {
 	case HV_PROTOCOL_RAW:
 		rc = opal_get_chars(termno, &c, 1);
 		break;
@@ -313,6 +316,7 @@ static int udbg_opal_getc_poll(void)
 static int udbg_opal_getc(void)
 {
 	int ch;
+
 	for (;;) {
 		ch = udbg_opal_getc_poll();
 		if (ch != -1)
@@ -342,16 +346,14 @@ void __init hvc_opal_init_early(void)
 		 * path, so we hard wire it
 		 */
 		opal = of_find_node_by_path("/ibm,opal/consoles");
-		if (opal)
-			pr_devel("hvc_opal: Found consoles in new location\n");
-		if (!opal) {
+		if (opal) {
+			pr_devel("Found consoles in new location\n");
+		} else {
 			opal = of_find_node_by_path("/ibm,opal");
-			if (opal)
-				pr_devel("hvc_opal: "
-					 "Found consoles in old location\n");
+			if (!opal)
+				return;
+			pr_devel("Found consoles in old location\n");
 		}
-		if (!opal)
-			return;
 		for_each_child_of_node(opal, np) {
 			if (of_node_name_eq(np, "serial")) {
 				stdout_node = np;
@@ -359,9 +361,9 @@ void __init hvc_opal_init_early(void)
 			}
 		}
 		of_node_put(opal);
+		if (!stdout_node)
+			return;
 	}
-	if (!stdout_node)
-		return;
 	termno = of_get_property(stdout_node, "reg", NULL);
 	index = termno ? be32_to_cpup(termno) : 0;
 	if (index >= MAX_NR_HVC_CONSOLES)
@@ -372,9 +374,8 @@ void __init hvc_opal_init_early(void)
 	if (of_device_is_compatible(stdout_node, "ibm,opal-console-raw")) {
 		hvc_opal_boot_priv.proto = HV_PROTOCOL_RAW;
 		ops = &hvc_opal_raw_ops;
-		pr_devel("hvc_opal: Found RAW console\n");
-	}
-	else if (of_device_is_compatible(stdout_node,"ibm,opal-console-hvsi")) {
+		pr_devel("Found RAW console\n");
+	} else if (of_device_is_compatible(stdout_node, "ibm,opal-console-hvsi")) {
 		hvc_opal_boot_priv.proto = HV_PROTOCOL_HVSI;
 		ops = &hvc_opal_hvsi_ops;
 		hvsilib_init(&hvc_opal_boot_priv.hvsi,
@@ -382,9 +383,10 @@ void __init hvc_opal_init_early(void)
 			     index, 1);
 		/* HVSI, perform the handshake now */
 		hvsilib_establish(&hvc_opal_boot_priv.hvsi);
-		pr_devel("hvc_opal: Found HVSI console\n");
-	} else
+		pr_devel("Found HVSI console\n");
+	} else {
 		goto out;
+	}
 	hvc_opal_boot_termno = index;
 	udbg_init_opal_common();
 	add_preferred_console("hvc", index, NULL);
@@ -397,6 +399,7 @@ void __init hvc_opal_init_early(void)
 void __init udbg_init_debug_opal_raw(void)
 {
 	u32 index = CONFIG_PPC_EARLY_DEBUG_OPAL_VTERMNO;
+
 	hvc_opal_privs[index] = &hvc_opal_boot_priv;
 	hvc_opal_boot_priv.proto = HV_PROTOCOL_RAW;
 	hvc_opal_boot_termno = index;
@@ -408,6 +411,7 @@ void __init udbg_init_debug_opal_raw(void)
 void __init udbg_init_debug_opal_hvsi(void)
 {
 	u32 index = CONFIG_PPC_EARLY_DEBUG_OPAL_VTERMNO;
+
 	hvc_opal_privs[index] = &hvc_opal_boot_priv;
 	hvc_opal_boot_termno = index;
 	udbg_init_opal_common();

Re: [PATCH] tty/hvc_opal: simplify if-if to if-else

[Jiri Slaby]

On 24. 04. 22, 11:25, Wan Jiabing wrote:
> Use if and else instead of if(A) and if (!A).
> 
> Signed-off-by: Wan Jiabing <wanjiabing@vivo.com>
> ---
>   drivers/tty/hvc/hvc_opal.c | 7 ++++---
>   1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/hvc/hvc_opal.c b/drivers/tty/hvc/hvc_opal.c
> index 84776bc641e6..2dafa0964c2a 100644
> --- a/drivers/tty/hvc/hvc_opal.c
> +++ b/drivers/tty/hvc/hvc_opal.c
> @@ -344,14 +344,15 @@ void __init hvc_opal_init_early(void)
>   		opal = of_find_node_by_path("/ibm,opal/consoles");
>   		if (opal)
>   			pr_devel("hvc_opal: Found consoles in new location\n");
> -		if (!opal) {
> +		else {

This looks good, except missing braces as noted by Joe.

>   			opal = of_find_node_by_path("/ibm,opal");
>   			if (opal)
>   				pr_devel("hvc_opal: "
>   					 "Found consoles in old location\n");
> +			else
> +				return;

I am not sure this return is more obvious than the previous one. Rather 
the opposite, IMO.

>   		}
> -		if (!opal)
> -			return;
> +
>   		for_each_child_of_node(opal, np) {
>   			if (of_node_name_eq(np, "serial")) {
>   				stdout_node = np;

thanks,
-- 
js
suse labs