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

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_ 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 , IBM Corp.
  */
 
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
 #undef DEBUG
 
 #include 
@@ -43,6 +45,7 @@ struct hvc_opal_priv {
hv_protocol_t   proto;  /* Raw data or HVSI packets */
struct hvsi_privhvsi;   /* 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 = _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, , 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) {