[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 <pali at 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 at 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