Re: [PATCH net-next v3 12/12] net: stmmac: dwmac-qcom-ethqos: Add EMAC3 support

Paolo Abeni
2023-04-06T08:58:04Z

Hi,

On Mon, 2023-04-03 at 11:52 -0500, Andrew Halaney wrote:
> @@ -327,9 +370,17 @@ static int ethqos_rgmii_macro_init(struct qcom_ethqos *ethqos)
> RGMII_CONFIG2_RX_PROG_SWAP,
> RGMII_IO_MACRO_CONFIG2);
>
> - /* Set PRG_RCLK_DLY to 57 for 1.8 ns delay */
> - rgmii_updatel(ethqos, SDCC_DDR_CONFIG_PRG_RCLK_DLY,
> - 57, SDCC_HC_REG_DDR_CONFIG);
> + /* PRG_RCLK_DLY = TCXO period * TCXO_CYCLES_CNT / 2 * RX delay ns,
> + * in practice this becomes PRG_RCLK_DLY = 52 * 4 / 2 * RX delay ns
> + */
> + if (ethqos->has_emac3)
> + /* 0.9 ns */
> + rgmii_updatel(ethqos, SDCC_DDR_CONFIG_PRG_RCLK_DLY,
> + 115, SDCC_HC_REG_DDR_CONFIG);
> + else
> + /* 1.8 ns */
> + rgmii_updatel(ethqos, SDCC_DDR_CONFIG_PRG_RCLK_DLY,
> + 57, SDCC_HC_REG_DDR_CONFIG);

The only (very minor) comment I have is that AFAIK the preferred style
for the above block is:

if (ethqos->has_emac3) {
/* 0.9 ns */
rgmii_updatel(ethqos, SDCC_DDR_CONFIG_PRG_RCLK_DLY,
115, SDCC_HC_REG_DDR_CONFIG);
} else {
...

due to the comment presence this should be threaded as a multi-line statement.
(even if checkpatch does not complain).

Cheers,

Paolo

> SDCC_HC_REG_DDR_CONFIG);
> @@ -355,8 +406,15 @@ static int ethqos_rgmii_macro_init(struct qcom_ethqos *ethqos)
> BIT(6), RGMII_IO_MACRO_CONFIG);
> rgmii_updatel(ethqos, RGMII_CONFIG2_RSVD_CONFIG15,
> 0, RGMII_IO_MACRO_CONFIG2);
> - rgmii_updatel(ethqos, RGMII_CONFIG2_RX_PROG_SWAP,
> - 0, RGMII_IO_MACRO_CONFIG2);
> +
> + if (ethqos->has_emac3)
> + rgmii_updatel(ethqos, RGMII_CONFIG2_RX_PROG_SWAP,
> + RGMII_CONFIG2_RX_PROG_SWAP,
> + RGMII_IO_MACRO_CONFIG2);
> + else
> + rgmii_updatel(ethqos, RGMII_CONFIG2_RX_PROG_SWAP,
> + 0, RGMII_IO_MACRO_CONFIG2);
> +
> /* Write 0x5 to PRG_RCLK_DLY_CODE */
> rgmii_updatel(ethqos, SDCC_DDR_CONFIG_EXT_PRG_RCLK_DLY_CODE,
> (BIT(29) | BIT(27)), SDCC_HC_REG_DDR_CONFIG);
> @@ -389,8 +447,13 @@ static int ethqos_rgmii_macro_init(struct qcom_ethqos *ethqos)
> RGMII_IO_MACRO_CONFIG);
> rgmii_updatel(ethqos, RGMII_CONFIG2_RSVD_CONFIG15,
> 0, RGMII_IO_MACRO_CONFIG2);
> - rgmii_updatel(ethqos, RGMII_CONFIG2_RX_PROG_SWAP,
> - 0, RGMII_IO_MACRO_CONFIG2);
> + if (ethqos->has_emac3)
> + rgmii_updatel(ethqos, RGMII_CONFIG2_RX_PROG_SWAP,
> + RGMII_CONFIG2_RX_PROG_SWAP,
> + RGMII_IO_MACRO_CONFIG2);
> + else
> + rgmii_updatel(ethqos, RGMII_CONFIG2_RX_PROG_SWAP,
> + 0, RGMII_IO_MACRO_CONFIG2);
> /* Write 0x5 to PRG_RCLK_DLY_CODE */
> rgmii_updatel(ethqos, SDCC_DDR_CONFIG_EXT_PRG_RCLK_DLY_CODE,
> (BIT(29) | BIT(27)), SDCC_HC_REG_DDR_CONFIG);
> @@ -433,6 +496,17 @@ static int ethqos_configure(struct qcom_ethqos *ethqos)
> rgmii_updatel(ethqos, SDCC_DLL_CONFIG_PDN,
> SDCC_DLL_CONFIG_PDN, SDCC_HC_REG_DLL_CONFIG);
>
> + if (ethqos->has_emac3) {
> + if (ethqos->speed == SPEED_1000) {
> + rgmii_writel(ethqos, 0x1800000, SDCC_TEST_CTL);
> + rgmii_writel(ethqos, 0x2C010800, SDCC_USR_CTL);
> + rgmii_writel(ethqos, 0xA001, SDCC_HC_REG_DLL_CONFIG2);
> + } else {
> + rgmii_writel(ethqos, 0x40010800, SDCC_USR_CTL);
> + rgmii_writel(ethqos, 0xA001, SDCC_HC_REG_DLL_CONFIG2);
> + }
> + }
> +
> /* Clear DLL_RST */
> rgmii_updatel(ethqos, SDCC_DLL_CONFIG_DLL_RST, 0,
> SDCC_HC_REG_DLL_CONFIG);
> @@ -452,7 +526,9 @@ static int ethqos_configure(struct qcom_ethqos *ethqos)
> SDCC_HC_REG_DLL_CONFIG);
>
> /* Set USR_CTL bit 26 with mask of 3 bits */
> - rgmii_updatel(ethqos, GENMASK(26, 24), BIT(26), SDCC_USR_CTL);
> + if (!ethqos->has_emac3)
> + rgmii_updatel(ethqos, GENMASK(26, 24), BIT(26),
> + SDCC_USR_CTL);
>
> /* wait for DLL LOCK */
> do {
> @@ -547,6 +623,7 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
> ethqos->por = data->por;
> ethqos->num_por = data->num_por;
> ethqos->rgmii_config_loopback_en = data->rgmii_config_loopback_en;
> + ethqos->has_emac3 = data->has_emac3;
>
> ethqos->rgmii_clk = devm_clk_get(&pdev->dev, "rgmii");
> if (IS_ERR(ethqos->rgmii_clk)) {
> @@ -566,6 +643,7 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
> plat_dat->fix_mac_speed = ethqos_fix_mac_speed;
> plat_dat->dump_debug_regs = rgmii_dump;
> plat_dat->has_gmac4 = 1;
> + plat_dat->dwmac4_addrs = &data->dwmac4_addrs;
> plat_dat->pmt = 1;
> plat_dat->tso_en = of_property_read_bool(np, "snps,tso");
> if (of_device_is_compatible(np, "qcom,qcs404-ethqos"))
> @@ -603,6 +681,7 @@ static int qcom_ethqos_remove(struct platform_device *pdev)
>
> static const struct of_device_id qcom_ethqos_match[] = {
> { .compatible = "qcom,qcs404-ethqos", .data = &emac_v2_3_0_data},
> + { .compatible = "qcom,sc8280xp-ethqos", .data = &emac_v3_0_0_data},
> { .compatible = "qcom,sm8150-ethqos", .data = &emac_v2_1_0_data},
> { }
> };