On Mon, 20 Mar 2023 17:16:15 -0500 Andrew Halaney wrote: > static int ethqos_rgmii_macro_init(struct qcom_ethqos *ethqos) > { > + int phy_mode; > + int phase_shift; nit: invert the order, we like variable declaration lines longest to shortest > + /* Determine if the PHY adds a 2 ns TX delay or the MAC handles it */ > + phy_mode = device_get_phy_mode(ðqos->pdev->dev); > + if (phy_mode == PHY_INTERFACE_MODE_RGMII_ID || phy_mode == PHY_INTERFACE_MODE_RGMII_TXID) Let's try to stick to 80 chars where reasonable, this would be easier to read as 2 lines. > + phase_shift = 0; > + else > + phase_shift = RGMII_CONFIG2_TX_CLK_PHASE_SHIFT_EN; > + > /* Disable loopback mode */ > rgmii_updatel(ethqos, RGMII_CONFIG2_TX_TO_RX_LOOPBACK_EN, > 0, RGMII_IO_MACRO_CONFIG2); > @@ -300,9 +310,9 @@ static int ethqos_rgmii_macro_init(struct qcom_ethqos *ethqos) > RGMII_CONFIG_PROG_SWAP, RGMII_IO_MACRO_CONFIG); > rgmii_updatel(ethqos, RGMII_CONFIG2_DATA_DIVIDE_CLK_SEL, > 0, RGMII_IO_MACRO_CONFIG2); > + > rgmii_updatel(ethqos, RGMII_CONFIG2_TX_CLK_PHASE_SHIFT_EN, > - RGMII_CONFIG2_TX_CLK_PHASE_SHIFT_EN, > - RGMII_IO_MACRO_CONFIG2); > + phase_shift, RGMII_IO_MACRO_CONFIG2); here and in couple more places looks like indentation got broken? continuation line should start under the opening bracket + 1.