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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 13 Jun 2023 09:58:44 +0200
From: Bartosz Golaszewski <brgl@...ev.pl>
To: Andrew Halaney <ahalaney@...hat.com>
Cc: Vinod Koul <vkoul@...nel.org>, Bhupesh Sharma <bhupesh.sharma@...aro.org>, 
	Andy Gross <agross@...nel.org>, Bjorn Andersson <andersson@...nel.org>, 
	Konrad Dybcio <konrad.dybcio@...aro.org>, "David S . Miller" <davem@...emloft.net>, 
	Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	Rob Herring <robh+dt@...nel.org>, 
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>, 
	Kishon Vijay Abraham I <kishon@...nel.org>, Giuseppe Cavallaro <peppe.cavallaro@...com>, 
	Alexandre Torgue <alexandre.torgue@...s.st.com>, Jose Abreu <joabreu@...opsys.com>, 
	netdev@...r.kernel.org, linux-arm-msm@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-phy@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org, 
	linux-stm32@...md-mailman.stormreply.com, 
	Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
Subject: 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 <ahalaney@...hat.com> wrote:
>
> On Mon, Jun 12, 2023 at 11:23:42AM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <bartosz.golaszewski@...aro.org>
> >
> > On sa8775p there's no RGMII clock so make it optional in the driver.
> >
> > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@...aro.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
> >
>