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

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