Re: [PATCH net-next 3/3] net: stmmac: Add RK3566/RK3568 SoC support

Hi Peter, Heiko,

On Wed, 2021-04-14 at 13:15 +0200, Heiko Stübner wrote:

Am Mittwoch, 14. April 2021, 13:03:25 CEST schrieb Peter Geis:

quoted

On Tue, Apr 13, 2021 at 7:37 PM Ezequiel Garcia [off-list ref] wrote:

quoted

quoted

quoted

+static void rk3566_set_to_rmii(struct rk_priv_data *bsp_priv)
+{
+       struct device *dev = &bsp_priv->pdev->dev;
+
+       if (IS_ERR(bsp_priv->grf)) {
+               dev_err(dev, "%s: Missing rockchip,grf property\n", __func__);
+               return;
+       }
+
+       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
+                    RK3568_GMAC_PHY_INTF_SEL_RMII);
+}
+
+static void rk3568_set_to_rgmii(struct rk_priv_data *bsp_priv,
+                               int tx_delay, int rx_delay)
+{
+       struct device *dev = &bsp_priv->pdev->dev;
+
+       if (IS_ERR(bsp_priv->grf)) {
+               dev_err(dev, "Missing rockchip,grf property\n");
+               return;
+       }
+
+       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON1,
+                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
+                    RK3568_GMAC_RXCLK_DLY_ENABLE |
+                    RK3568_GMAC_TXCLK_DLY_ENABLE);
+
+       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC0_CON0,
+                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
+                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));
+
+       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON1,
+                    RK3568_GMAC_PHY_INTF_SEL_RGMII |
+                    RK3568_GMAC_RXCLK_DLY_ENABLE |
+                    RK3568_GMAC_TXCLK_DLY_ENABLE);
+
+       regmap_write(bsp_priv->grf, RK3568_GRF_GMAC1_CON0,
+                    RK3568_GMAC_CLK_RX_DL_CFG(rx_delay) |
+                    RK3568_GMAC_CLK_TX_DL_CFG(tx_delay));

Since there are two GMACs on the rk3568, and either, or, or both may
be enabled in various configurations, we should only configure the
controller we are currently operating.

Perhaps we should have match data (such as reg = <0>, or against the
address) to identify the individual controllers.

Hmm, "reg" will be used by the actual mmio address of the controller,
so matching against that should be the way I guess.

We're already doing something similar for dsi:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c#n1170

I have to admit, I'm not a fan of hardcoding the registers in the kernel.

David Wu solved this in the downstream kernel by using bus_id,
which parses the devicetree "ethernet@0" node, i.e.:

  plat->bus_id = of_alias_get_id(np, "ethernet");

I'm inclined for this solution. Maybe Jose can suggest how to approach it?

Thanks!
Ezequiel