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

Pali Rohár
2022-04-22T09:21:13Z

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.