[PATCH] net: zynq: Add support for PHY configuration in SGMII mode

[Sean Anderson]

On 11/21/21 2:16 PM, Ramon Fried wrote:
> On Fri, Nov 19, 2021 at 7:01 PM Sean Anderson <sean.anderson at 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 at 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