Re: [PATCH u-boot-marvell 6/8] arm: mvebu: turris_omnia: Extract code for disabling sata/pcie

[Stefan Roese]

On 4/22/22 11:20, Pali Rohár wrote:

On Wednesday 02 March 2022 13:38:48 Marek Behún wrote:

On Wed,  2 Mar 2022 12:47:56 +0100
Pali Rohár <p...@kernel.org> wrote:

Move code for disabling sata and pcie DT nodes to own functions, so this
code can be called from other places in follow up patches.

Signed-off-by: Pali Rohár <p...@kernel.org>
---
  board/CZ.NIC/turris_omnia/turris_omnia.c | 96 +++++++++++++-----------
  1 file changed, 52 insertions(+), 44 deletions(-)

diff --git a/board/CZ.NIC/turris_omnia/turris_omnia.c 
b/board/CZ.NIC/turris_omnia/turris_omnia.c
index 6a057ea7dd70..57b797db4a8e 100644
--- a/board/CZ.NIC/turris_omnia/turris_omnia.c
+++ b/board/CZ.NIC/turris_omnia/turris_omnia.c
@@ -518,11 +518,54 @@ void spl_board_init(void)

#if IS_ENABLED(CONFIG_OF_BOARD_FIXUP) || IS_ENABLED(CONFIG_OF_BOARD_SETUP) -static void fixup_serdes_0_nodes(void *blob)

+static void disable_sata_node(void *blob)
  {
-       bool mode_sata;
        int node;

+ fdt_for_each_node_by_compatible(node, blob, -1, "marvell,armada-380-ahci") {

+               if (!fdtdec_get_is_enabled(blob, node))
+                       continue;
+
+               if (fdt_status_disabled(blob, node) < 0)
+                       printf("Cannot disable SATA DT node!\n");
+               else
+                       debug("Disabled SATA DT node\n");
+
+               break;
+       }
+}
+
+static void disable_pcie_node(void *blob, int port)
+{
+       int node;
+
+       fdt_for_each_node_by_compatible(node, blob, -1, 
"marvell,armada-370-pcie") {
+               int port_node;
+
+               if (!fdtdec_get_is_enabled(blob, node))
+                       continue;
+
+               fdt_for_each_subnode (port_node, blob, node) {
+                       if (!fdtdec_get_is_enabled(blob, port_node))
+                               continue;
+
+                       if (fdtdec_get_int(blob, port_node, 
"marvell,pcie-port", -1) != port)
+                               continue;
+
+                       if (fdt_status_disabled(blob, port_node) < 0)
+                               printf("Cannot disable PCIe port %d DT 
node!\n", port);
+                       else
+                               debug("Disabled PCIe port %d DT node\n", port);
+
+                       return;
+               }
+       }
+}
+
+static void fixup_msata_port_nodes(void *blob)
+{
+       bool mode_sata;
+
        /*
         * Determine if SerDes 0 is configured to SATA mode.
         * We do this instead of calling omnia_detect_sata() to avoid another
@@ -541,47 +584,12 @@ static void fixup_serdes_0_nodes(void *blob)
                return;
        }

- /* If mSATA card is not present, disable SATA DT node */

        if (!mode_sata) {
-               fdt_for_each_node_by_compatible(node, blob, -1,
-                                               "marvell,armada-380-ahci") {
-                       if (!fdtdec_get_is_enabled(blob, node))
-                               continue;
-
-                       if (fdt_status_disabled(blob, node) < 0)
-                               printf("Cannot disable SATA DT node!\n");
-                       else
-                               debug("Disabled SATA DT node\n");
-
-                       break;
-               }
-
-               return;
-       }
-
-       /* Otherwise disable PCIe port 0 DT node (MiniPCIe / mSATA port) */
-       fdt_for_each_node_by_compatible(node, blob, -1,
-                                       "marvell,armada-370-pcie") {
-               int port;
-
-               if (!fdtdec_get_is_enabled(blob, node))
-                       continue;
-
-               fdt_for_each_subnode (port, blob, node) {
-                       if (!fdtdec_get_is_enabled(blob, port))
-                               continue;
-
-                       if (fdtdec_get_int(blob, port, "marvell,pcie-port",
-                                          -1) != 0)
-                               continue;
-
-                       if (fdt_status_disabled(blob, port) < 0)
-                               printf("Cannot disable PCIe port 0 DT node!\n");
-                       else
-                               debug("Disabled PCIe port 0 DT node\n");
-
-                       return;
-               }
+               /* If mSATA card is not present, disable SATA DT node */
+               disable_sata_node(blob);
+       } else {
+               /* Otherwise disable PCIe port 0 DT node (MiniPCIe / mSATA 
port) */
+               disable_pcie_node(blob, 0);
        }

If I am looking at this correctly, there is only one statement both in
if and in else clause. Can we drop the {} parentheses?

Marek

Due to comment above which belongs to the code, I put there {} parentheses.

Agreed. I am also in favor of putting parentheses on multi-line
statements (not depending on single- or multi-statement).

Thanks,
Stefan