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]
Message-ID: <CAAtXAHertUY3L9dQABb-YkreSYUKh254frHvUEBVJinT4TdneQ@mail.gmail.com>
Date:   Wed, 15 Feb 2017 13:04:38 -0800
From:   Moritz Fischer <mdf@...nel.org>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        netdev@...r.kernel.org, Nicolas Ferre <nicolas.ferre@...rochip.com>
Subject: Re: [PATCH] net: ethernet: cadence: Add fixed-link functionality

Hi Florian,

thanks for the quick reply.

On Wed, Feb 15, 2017 at 12:57 PM, Florian Fainelli <f.fainelli@...il.com> wrote:
> On 02/15/2017 12:44 PM, mdf@...nel.org wrote:
>> From: Moritz Fischer <mdf@...nel.org>
>>
>> This allows 'fixed-link' direct MAC connections to be declared
>> in devicetree.
>>
>> Signed-off-by: Moritz Fischer <mdf@...nel.org>
>> Cc: Nicolas Ferre <nicolas.ferre@...rochip.com>
>> ---
>>  drivers/net/ethernet/cadence/macb.c | 61 ++++++++++++++++++++++++++++++++++---
>>  drivers/net/ethernet/cadence/macb.h |  1 +
>>  2 files changed, 57 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/cadence/macb.c b/drivers/net/ethernet/cadence/macb.c
>> index 30606b1..af443a8 100644
>> --- a/drivers/net/ethernet/cadence/macb.c
>> +++ b/drivers/net/ethernet/cadence/macb.c
>> @@ -469,6 +469,34 @@ static int macb_mii_probe(struct net_device *dev)
>>       return 0;
>>  }
>>
>> +static int macb_fixed_init(struct macb *bp)
>> +{
>> +     struct phy_device *phydev;
>> +
>> +     phydev = of_phy_connect(bp->dev, bp->phy_node,
>> +                             &macb_handle_link_change, 0,
>> +                             bp->phy_interface);
>> +     if (!phydev)
>> +             return -ENODEV;
>> +
>> +     /* mask with MAC supported features */
>> +     if (macb_is_gem(bp) && bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE)
>> +             phydev->supported &= PHY_GBIT_FEATURES;
>> +     else
>> +             phydev->supported &= PHY_BASIC_FEATURES;
>> +
>> +     if (bp->caps & MACB_CAPS_NO_GIGABIT_HALF)
>> +             phydev->supported &= ~SUPPORTED_1000baseT_Half;
>> +
>> +     phydev->advertising = phydev->supported;
>> +
>> +     bp->link = 0;
>> +     bp->speed = 0;
>> +     bp->duplex = -1;
>> +
>> +     return 0;
>> +}
>
> This is nearly identical to macb_mii_probe(), can you try to re-use what
> is done there and just extract the phy_find_first() part which is
> different here?

Yeah. It probably still needs some work.
>
>> +
>>  static int macb_mii_init(struct macb *bp)
>>  {
>>       struct macb_platform_data *pdata;
>> @@ -3245,6 +3273,7 @@ static int macb_probe(struct platform_device *pdev)
>>       const char *mac;
>>       struct macb *bp;
>>       int err;
>> +     bool fixed_link = false;
>>
>>       regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>>       mem = devm_ioremap_resource(&pdev->dev, regs);
>> @@ -3342,8 +3371,18 @@ static int macb_probe(struct platform_device *pdev)
>>               macb_get_hwaddr(bp);
>>
>>       /* Power up the PHY if there is a GPIO reset */
>> -     phy_node =  of_get_next_available_child(np, NULL);
>> -     if (phy_node) {
>> +     phy_node = of_parse_phandle(np, "phy-handle", 0);
>> +     if (!phy_node && of_phy_is_fixed_link(np)) {
>> +             err = of_phy_register_fixed_link(np);
>> +             if (err < 0) {
>> +                     dev_err(&pdev->dev, "broken fixed-link specification");
>> +                     goto failed_phy;
>> +             }
>> +             /* in case of a fixed PHY, the DT node is the ethernet MAC */
>> +             phy_node = of_node_get(np);
>> +             bp->phy_node = phy_node;
>> +             fixed_link = true;
>> +     } else {
>>               int gpio = of_get_named_gpio(phy_node, "reset-gpios", 0);
>>
>>               if (gpio_is_valid(gpio)) {
>> @@ -3369,7 +3408,10 @@ static int macb_probe(struct platform_device *pdev)
>>       if (err)
>>               goto err_out_free_netdev;
>>
>> -     err = macb_mii_init(bp);
>> +     if (!fixed_link)
>> +             err = macb_mii_init(bp);
>> +     else
>> +             err = macb_fixed_init(bp);
>>       if (err)
>>               goto err_out_free_netdev;
>>
>> @@ -3400,6 +3442,9 @@ static int macb_probe(struct platform_device *pdev)
>>       if (bp->reset_gpio)
>>               gpiod_set_value(bp->reset_gpio, 0);
>>
>> +failed_phy:
>> +     of_node_put(phy_node);
>> +
>>  err_out_free_netdev:
>>       free_netdev(dev);
>>
>> @@ -3423,9 +3468,14 @@ static int macb_remove(struct platform_device *pdev)
>>               bp = netdev_priv(dev);
>>               if (dev->phydev)
>>                       phy_disconnect(dev->phydev);
>> -             mdiobus_unregister(bp->mii_bus);
>> +
>> +             if (!bp->phy_node)
>> +                     mdiobus_unregister(bp->mii_bus);
>> +
>>               dev->phydev = NULL;
>> -             mdiobus_free(bp->mii_bus);
>> +
>> +             if (!bp->phy_node)
>> +                     mdiobus_free(bp->mii_bus);
>
> Humm, I'd have to read the code a bit more, but conceptually, you could
> declare child MDIO device nodes (e.g: Ethernet switch) and a fixed-link
> that describes how you are connecting to that Ethernet switch. The MDIO
> devices require the MDIO bus to be available, so I don't think you can
> treat fixed-link as mutually exclusive with an absence of PHY nodes.

Huh, yeah. In my case the switch configuration is done over SPI.
Probably someone
out there will have hardware that does MDIO, possible.
I'll do some more research on my end. In my understanding (which might
be wrong),
the fixed-link was the alternative to a PHY hooked up with MDIO.

If you find something in your research, feel free to keep me posted ;-)

Cheers,
Moritz

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ