[PATCH] net: zynq: Add support for PHY configuration in SGMII mode
SGMII configuration depends on proper GT setting that's why when node has phys property call PSGTR driver to configure it properly.
Signed-off-by: Michal Simek michal.simek@xilinx.com ---
drivers/net/zynq_gem.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 5cbe8d28304b..fece077066df 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -12,6 +12,7 @@ #include <common.h> #include <cpu_func.h> #include <dm.h> +#include <generic-phy.h> #include <log.h> #include <net.h> #include <netdev.h> @@ -716,6 +717,21 @@ static int zynq_gem_probe(struct udevice *dev) struct zynq_gem_priv *priv = dev_get_priv(dev); int ret;
+ if (priv->interface == PHY_INTERFACE_MODE_SGMII) { + struct phy phy; + + ret = generic_phy_get_by_index(dev, 0, &phy); + if (!ret) { + ret = generic_phy_init(&phy); + if (ret) + return ret; + + ret = generic_phy_power_on(&phy); + if (ret) + return ret; + } + } + ret = zynq_gem_reset_init(dev); if (ret) return ret;
On 11/18/21 7:09 AM, Michal Simek wrote:> SGMII configuration depends on proper GT setting that's why when node has
phys property call PSGTR driver to configure it properly.
Signed-off-by: Michal Simek michal.simek@xilinx.com
drivers/net/zynq_gem.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 5cbe8d28304b..fece077066df 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -12,6 +12,7 @@ #include <common.h> #include <cpu_func.h> #include <dm.h> +#include <generic-phy.h> #include <log.h> #include <net.h> #include <netdev.h> @@ -716,6 +717,21 @@ static int zynq_gem_probe(struct udevice *dev) struct zynq_gem_priv *priv = dev_get_priv(dev); int ret;
- if (priv->interface == PHY_INTERFACE_MODE_SGMII) {
struct phy phy;ret = generic_phy_get_by_index(dev, 0, &phy);
You should check the return value here. See f56db163ad ("usb: host: xhci-dwc3: Add generic PHY support") for an example.
if (!ret) {ret = generic_phy_init(&phy);if (ret)return ret;ret = generic_phy_power_on(&phy);if (ret)return ret;}- }
- ret = zynq_gem_reset_init(dev); if (ret) return ret;
Otherwise LGTM
On Fri, Nov 19, 2021 at 7:01 PM Sean Anderson sean.anderson@seco.com wrote:
On 11/18/21 7:09 AM, Michal Simek wrote:> SGMII configuration depends on proper GT setting that's why when node has
phys property call PSGTR driver to configure it properly.
Signed-off-by: Michal Simek michal.simek@xilinx.com
drivers/net/zynq_gem.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 5cbe8d28304b..fece077066df 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -12,6 +12,7 @@ #include <common.h> #include <cpu_func.h> #include <dm.h> +#include <generic-phy.h> #include <log.h> #include <net.h> #include <netdev.h> @@ -716,6 +717,21 @@ static int zynq_gem_probe(struct udevice *dev) struct zynq_gem_priv *priv = dev_get_priv(dev); int ret;
if (priv->interface == PHY_INTERFACE_MODE_SGMII) {struct phy phy;ret = generic_phy_get_by_index(dev, 0, &phy);You should check the return value here. See f56db163ad ("usb: host: xhci-dwc3: Add generic PHY support") for an example.
He does check, he just doesn't print an error message. what am I missing here ?
if (!ret) {ret = generic_phy_init(&phy);if (ret)return ret;ret = generic_phy_power_on(&phy);if (ret)return ret;}}ret = zynq_gem_reset_init(dev); if (ret) return ret;Otherwise LGTM
On 11/21/21 2:16 PM, Ramon Fried wrote:
On Fri, Nov 19, 2021 at 7:01 PM Sean Anderson sean.anderson@seco.com wrote:
On 11/18/21 7:09 AM, Michal Simek wrote:> SGMII configuration depends on proper GT setting that's why when node has
phys property call PSGTR driver to configure it properly.
Signed-off-by: Michal Simek michal.simek@xilinx.com
drivers/net/zynq_gem.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 5cbe8d28304b..fece077066df 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -12,6 +12,7 @@ #include <common.h> #include <cpu_func.h> #include <dm.h> +#include <generic-phy.h> #include <log.h> #include <net.h> #include <netdev.h> @@ -716,6 +717,21 @@ static int zynq_gem_probe(struct udevice *dev) struct zynq_gem_priv *priv = dev_get_priv(dev); int ret;
if (priv->interface == PHY_INTERFACE_MODE_SGMII) {struct phy phy;ret = generic_phy_get_by_index(dev, 0, &phy);You should check the return value here. See f56db163ad ("usb: host: xhci-dwc3: Add generic PHY support") for an example.
He does check, he just doesn't print an error message. what am I missing here ?
The correct behavior here is something like
ret = generic_phy_get_by_index(dev, 0, &phy); if (!ret) { ... } else if (ret != -ENOENT) { dev_dbg(dev, "could not get phy (err %d)\n", ret); return ret; }
which will give an error message if e.g. the reference is malformed, or the phy driver isn't compiled in
--Sean
if (!ret) {ret = generic_phy_init(&phy);if (ret)return ret;ret = generic_phy_power_on(&phy);if (ret)return ret;}}ret = zynq_gem_reset_init(dev); if (ret) return ret;Otherwise LGTM
On 11/21/21 21:58, Sean Anderson wrote:
On 11/21/21 2:16 PM, Ramon Fried wrote:
On Fri, Nov 19, 2021 at 7:01 PM Sean Anderson sean.anderson@seco.com wrote:
On 11/18/21 7:09 AM, Michal Simek wrote:> SGMII configuration depends on proper GT setting that's why when node has
phys property call PSGTR driver to configure it properly.
Signed-off-by: Michal Simek michal.simek@xilinx.com
drivers/net/zynq_gem.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
diff --git a/drivers/net/zynq_gem.c b/drivers/net/zynq_gem.c index 5cbe8d28304b..fece077066df 100644 --- a/drivers/net/zynq_gem.c +++ b/drivers/net/zynq_gem.c @@ -12,6 +12,7 @@ #include <common.h> #include <cpu_func.h> #include <dm.h> +#include <generic-phy.h> #include <log.h> #include <net.h> #include <netdev.h> @@ -716,6 +717,21 @@ static int zynq_gem_probe(struct udevice *dev) struct zynq_gem_priv *priv = dev_get_priv(dev); int ret;
+ if (priv->interface == PHY_INTERFACE_MODE_SGMII) { + struct phy phy;
+ ret = generic_phy_get_by_index(dev, 0, &phy);
You should check the return value here. See f56db163ad ("usb: host: xhci-dwc3: Add generic PHY support") for an example.
He does check, he just doesn't print an error message. what am I missing here ?
The correct behavior here is something like
ret = generic_phy_get_by_index(dev, 0, &phy); if (!ret) { ... } else if (ret != -ENOENT) { dev_dbg(dev, "could not get phy (err %d)\n", ret); return ret; }
which will give an error message if e.g. the reference is malformed, or the phy driver isn't compiled in
v2 sent with this added. Also I have tested it on real board.
Thanks, Michal
participants (4)
-
Michal Simek -
Ramon Fried -
Sean Anderson -
Sean Anderson