| Message ID | 20230612092355.87937-14-brgl@bgdev.pl (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm64: qcom: sa8775p-ride: enable the first ethernet port | expand |
On Mon, Jun 12, 2023 at 11:23:42AM +0200, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > On sa8775p there's no RGMII clock so make it optional in the driver. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > --- > 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. > if (IS_ERR(ethqos->rgmii_clk)) { > ret = PTR_ERR(ethqos->rgmii_clk); > goto out_config_dt; > -- > 2.39.2 >
On Mon, Jun 12, 2023 at 10:40 PM Andrew Halaney <ahalaney@redhat.com> wrote: > > On Mon, Jun 12, 2023 at 11:23:42AM +0200, Bartosz Golaszewski wrote: > > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > > > On sa8775p there's no RGMII clock so make it optional in the driver. > > > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org> > > --- > > 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 > > > if (IS_ERR(ethqos->rgmii_clk)) { > > ret = PTR_ERR(ethqos->rgmii_clk); > > goto out_config_dt; > > -- > > 2.39.2 > > >
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"); if (IS_ERR(ethqos->rgmii_clk)) { ret = PTR_ERR(ethqos->rgmii_clk); goto out_config_dt;