From: Bartosz Golaszewski <[email protected]>
On sa8775p platforms, there's a SGMII SerDes PHY between the MAC and
external PHY that we need to enable and configure.
Signed-off-by: Bartosz Golaszewski <[email protected]>
---
.../stmicro/stmmac/dwmac-qcom-ethqos.c | 37 +++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
index 8ed05f29fe8b..3438b6229351 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
@@ -6,6 +6,7 @@
#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <linux/phy.h>
+#include <linux/phy/phy.h>
#include <linux/property.h>
#include "stmmac.h"
@@ -93,6 +94,7 @@ struct qcom_ethqos {
unsigned int rgmii_clk_rate;
struct clk *rgmii_clk;
+ struct phy *serdes_phy;
unsigned int speed;
const struct ethqos_emac_por *por;
@@ -566,6 +568,30 @@ static void ethqos_fix_mac_speed(void *priv, unsigned int speed)
ethqos_configure(ethqos);
}
+static int qcom_ethqos_serdes_powerup(struct net_device *ndev, void *priv)
+{
+ struct qcom_ethqos *ethqos = priv;
+ int ret;
+
+ ret = phy_set_speed(ethqos->serdes_phy, ethqos->speed);
+ if (ret)
+ return ret;
+
+ ret = phy_init(ethqos->serdes_phy);
+ if (ret)
+ return ret;
+
+ return phy_power_on(ethqos->serdes_phy);
+}
+
+static void qcom_ethqos_serdes_powerdown(struct net_device *ndev, void *priv)
+{
+ struct qcom_ethqos *ethqos = priv;
+
+ phy_power_off(ethqos->serdes_phy);
+ phy_exit(ethqos->serdes_phy);
+}
+
static int ethqos_clks_config(void *priv, bool enabled)
{
struct qcom_ethqos *ethqos = priv;
@@ -651,6 +677,12 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
if (ret)
goto out_config_dt;
+ ethqos->serdes_phy = devm_phy_optional_get(dev, "serdes");
+ if (IS_ERR(ethqos->serdes_phy)) {
+ ret = PTR_ERR(ethqos->serdes_phy);
+ goto out_config_dt;
+ }
+
ethqos->speed = SPEED_1000;
ethqos_update_rgmii_clk(ethqos, SPEED_1000);
ethqos_set_func_clk_en(ethqos);
@@ -666,6 +698,11 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
if (of_device_is_compatible(np, "qcom,qcs404-ethqos"))
plat_dat->rx_clk_runs_in_lpi = 1;
+ if (ethqos->serdes_phy) {
+ plat_dat->serdes_powerup = qcom_ethqos_serdes_powerup;
+ plat_dat->serdes_powerdown = qcom_ethqos_serdes_powerdown;
+ }
+
ret = stmmac_dvr_probe(dev, plat_dat, &stmmac_res);
if (ret)
goto out_config_dt;
--
2.39.2
On Mon, Jun 12, 2023 at 11:23:41AM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <[email protected]>
>
> On sa8775p platforms, there's a SGMII SerDes PHY between the MAC and
> external PHY that we need to enable and configure.
>
> Signed-off-by: Bartosz Golaszewski <[email protected]>
> ---
> .../stmicro/stmmac/dwmac-qcom-ethqos.c | 37 +++++++++++++++++++
> 1 file changed, 37 insertions(+)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> index 8ed05f29fe8b..3438b6229351 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> @@ -6,6 +6,7 @@
> #include <linux/of_device.h>
> #include <linux/platform_device.h>
> #include <linux/phy.h>
> +#include <linux/phy/phy.h>
> #include <linux/property.h>
>
> #include "stmmac.h"
> @@ -93,6 +94,7 @@ struct qcom_ethqos {
>
> unsigned int rgmii_clk_rate;
> struct clk *rgmii_clk;
> + struct phy *serdes_phy;
> unsigned int speed;
>
> const struct ethqos_emac_por *por;
> @@ -566,6 +568,30 @@ static void ethqos_fix_mac_speed(void *priv, unsigned int speed)
> ethqos_configure(ethqos);
> }
>
> +static int qcom_ethqos_serdes_powerup(struct net_device *ndev, void *priv)
> +{
> + struct qcom_ethqos *ethqos = priv;
> + int ret;
> +
> + ret = phy_set_speed(ethqos->serdes_phy, ethqos->speed);
> + if (ret)
> + return ret;
> +
> + ret = phy_init(ethqos->serdes_phy);
> + if (ret)
> + return ret;
> +
> + return phy_power_on(ethqos->serdes_phy);
The docs say (phy.rst):
The general order of calls should be::
[devm_][of_]phy_get()
phy_init()
phy_power_on()
[phy_set_mode[_ext]()]
...
phy_power_off()
phy_exit()
[[of_]phy_put()]
Some PHY drivers may not implement :c:func:`phy_init` or :c:func:`phy_power_on`,
but controllers should always call these functions to be compatible with other
PHYs. Some PHYs may require :c:func:`phy_set_mode <phy_set_mode_ext>`, while
others may use a default mode (typically configured via devicetree or other
firmware). For compatibility, you should always call this function if you know
what mode you will be using. Generally, this function should be called after
:c:func:`phy_power_on`, although some PHY drivers may allow it at any time.
Not really dictating you need to do that order, but if possible I think
calling phy_set_speed after init + power_on is more generic. Not sure if
that plays nice with the phy driver in this series or not.
Otherwise, I think this looks good.
> +}
> +
> +static void qcom_ethqos_serdes_powerdown(struct net_device *ndev, void *priv)
> +{
> + struct qcom_ethqos *ethqos = priv;
> +
> + phy_power_off(ethqos->serdes_phy);
> + phy_exit(ethqos->serdes_phy);
> +}
> +
> static int ethqos_clks_config(void *priv, bool enabled)
> {
> struct qcom_ethqos *ethqos = priv;
> @@ -651,6 +677,12 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
> if (ret)
> goto out_config_dt;
>
> + ethqos->serdes_phy = devm_phy_optional_get(dev, "serdes");
> + if (IS_ERR(ethqos->serdes_phy)) {
> + ret = PTR_ERR(ethqos->serdes_phy);
> + goto out_config_dt;
> + }
> +
> ethqos->speed = SPEED_1000;
> ethqos_update_rgmii_clk(ethqos, SPEED_1000);
> ethqos_set_func_clk_en(ethqos);
> @@ -666,6 +698,11 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
> if (of_device_is_compatible(np, "qcom,qcs404-ethqos"))
> plat_dat->rx_clk_runs_in_lpi = 1;
>
> + if (ethqos->serdes_phy) {
> + plat_dat->serdes_powerup = qcom_ethqos_serdes_powerup;
> + plat_dat->serdes_powerdown = qcom_ethqos_serdes_powerdown;
> + }
> +
> ret = stmmac_dvr_probe(dev, plat_dat, &stmmac_res);
> if (ret)
> goto out_config_dt;
> --
> 2.39.2
>
On Mon, Jun 12, 2023 at 10:33 PM Andrew Halaney <[email protected]> wrote:
>
> On Mon, Jun 12, 2023 at 11:23:41AM +0200, Bartosz Golaszewski wrote:
> > From: Bartosz Golaszewski <[email protected]>
> >
> > On sa8775p platforms, there's a SGMII SerDes PHY between the MAC and
> > external PHY that we need to enable and configure.
> >
> > Signed-off-by: Bartosz Golaszewski <[email protected]>
> > ---
> > .../stmicro/stmmac/dwmac-qcom-ethqos.c | 37 +++++++++++++++++++
> > 1 file changed, 37 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> > index 8ed05f29fe8b..3438b6229351 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-qcom-ethqos.c
> > @@ -6,6 +6,7 @@
> > #include <linux/of_device.h>
> > #include <linux/platform_device.h>
> > #include <linux/phy.h>
> > +#include <linux/phy/phy.h>
> > #include <linux/property.h>
> >
> > #include "stmmac.h"
> > @@ -93,6 +94,7 @@ struct qcom_ethqos {
> >
> > unsigned int rgmii_clk_rate;
> > struct clk *rgmii_clk;
> > + struct phy *serdes_phy;
> > unsigned int speed;
> >
> > const struct ethqos_emac_por *por;
> > @@ -566,6 +568,30 @@ static void ethqos_fix_mac_speed(void *priv, unsigned int speed)
> > ethqos_configure(ethqos);
> > }
> >
> > +static int qcom_ethqos_serdes_powerup(struct net_device *ndev, void *priv)
> > +{
> > + struct qcom_ethqos *ethqos = priv;
> > + int ret;
> > +
> > + ret = phy_set_speed(ethqos->serdes_phy, ethqos->speed);
> > + if (ret)
> > + return ret;
> > +
> > + ret = phy_init(ethqos->serdes_phy);
> > + if (ret)
> > + return ret;
> > +
> > + return phy_power_on(ethqos->serdes_phy);
>
> The docs say (phy.rst):
>
> The general order of calls should be::
>
> [devm_][of_]phy_get()
> phy_init()
> phy_power_on()
> [phy_set_mode[_ext]()]
> ...
> phy_power_off()
> phy_exit()
> [[of_]phy_put()]
>
> Some PHY drivers may not implement :c:func:`phy_init` or :c:func:`phy_power_on`,
> but controllers should always call these functions to be compatible with other
> PHYs. Some PHYs may require :c:func:`phy_set_mode <phy_set_mode_ext>`, while
> others may use a default mode (typically configured via devicetree or other
> firmware). For compatibility, you should always call this function if you know
> what mode you will be using. Generally, this function should be called after
> :c:func:`phy_power_on`, although some PHY drivers may allow it at any time.
>
> Not really dictating you need to do that order, but if possible I think
> calling phy_set_speed after init + power_on is more generic. Not sure if
> that plays nice with the phy driver in this series or not.
>
> Otherwise, I think this looks good.
>
I had to rework the PHY driver code a bit for this order to work but
it'll be good now in v2.
Thanks!
Bart
> > +}
> > +
> > +static void qcom_ethqos_serdes_powerdown(struct net_device *ndev, void *priv)
> > +{
> > + struct qcom_ethqos *ethqos = priv;
> > +
> > + phy_power_off(ethqos->serdes_phy);
> > + phy_exit(ethqos->serdes_phy);
> > +}
> > +
> > static int ethqos_clks_config(void *priv, bool enabled)
> > {
> > struct qcom_ethqos *ethqos = priv;
> > @@ -651,6 +677,12 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
> > if (ret)
> > goto out_config_dt;
> >
> > + ethqos->serdes_phy = devm_phy_optional_get(dev, "serdes");
> > + if (IS_ERR(ethqos->serdes_phy)) {
> > + ret = PTR_ERR(ethqos->serdes_phy);
> > + goto out_config_dt;
> > + }
> > +
> > ethqos->speed = SPEED_1000;
> > ethqos_update_rgmii_clk(ethqos, SPEED_1000);
> > ethqos_set_func_clk_en(ethqos);
> > @@ -666,6 +698,11 @@ static int qcom_ethqos_probe(struct platform_device *pdev)
> > if (of_device_is_compatible(np, "qcom,qcs404-ethqos"))
> > plat_dat->rx_clk_runs_in_lpi = 1;
> >
> > + if (ethqos->serdes_phy) {
> > + plat_dat->serdes_powerup = qcom_ethqos_serdes_powerup;
> > + plat_dat->serdes_powerdown = qcom_ethqos_serdes_powerdown;
> > + }
> > +
> > ret = stmmac_dvr_probe(dev, plat_dat, &stmmac_res);
> > if (ret)
> > goto out_config_dt;
> > --
> > 2.39.2
> >
>