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

["Marek Behún" <marek.behun@nic.cz>]

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