Re: [PATCH 13/26] net: stmmac: dwmac-qcom-ethqos: make the rgmii clock optional

On Mon, Jun 12, 2023 at 10:40 PM Andrew Halaney [off-list ref] wrote:

On Mon, Jun 12, 2023 at 11:23:42AM +0200, Bartosz Golaszewski wrote:

quoted

From: Bartosz Golaszewski <redacted>

On sa8775p there's no RGMII clock so make it optional in the driver.

Signed-off-by: Bartosz Golaszewski <redacted>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
index 3438b6229351..252dca400071 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c

@@ -663,7 +663,7 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
      ethqos->rgmii_config_loopback_en = data->rgmii_config_loopback_en;
      ethqos->has_emac3 = data->has_emac3;

-     ethqos->rgmii_clk = devm_clk_get(dev, "rgmii");
+     ethqos->rgmii_clk = devm_clk_get_optional(dev, "rgmii");

This makes it optional for older platforms too, but as far as I know on
those platforms it is mandatory.

This can be enforced in dt-binding checks, but should we also enforce
that in the driver still? Honestly I feel like yes, but there's probably
some precedent maintainers follow on this front that I don't know of.

While my gut feeling is that enforcing the clock list on the DT
binding lever is enough, we can also do a different thing: rename the
clock from rgmii_clk to link_clk or something similar and just
determine the name based on the HW variant ("rgmii" or "phyaux"). Or
even get the clock by its index? this way we could fold the next patch
in the series into this one and simplify the code.

Bart

quoted

      if (IS_ERR(ethqos->rgmii_clk)) {
              ret = PTR_ERR(ethqos->rgmii_clk);
              goto out_config_dt;
--
2.39.2

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy