Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
In-Reply-To: <20180905010100.GN29121@lunn.ch>
References: <20180905001535.19168-1-mdf@kernel.org> <20180905010100.GN29121@lunn.ch>
From:   Moritz Fischer <moritz.fischer@ettus.com>
Date:   Tue, 4 Sep 2018 20:27:43 -0700
Message-ID: <CAAtXAHdu1a5dTP8rd7+Oc2--5F-nU5E6M796KZKd8croYcLVoQ@mail.gmail.com>
Subject: Re: [RFC/PATCH] net: nixge: Add PHYLINK support
To:     Andrew Lunn <andrew@lunn.ch>
Cc:     netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Alex Williams <alex.williams@ni.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Hi Andrew,

On Tue, Sep 4, 2018 at 6:01 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> 3) I'm again not sure about the 'select PHYLINK', wouldn't
>>    wanna break the build again...
>
> Hi Moritz
>
> I think it is safe. PHYLINK has no stated dependencies on OF. But i
> suspect it currently is pretty useless without OF.

Ok, great. Thanks!

>> @@ -1286,7 +1329,13 @@ static int nixge_probe(struct platform_device *pdev)
>>       priv->coalesce_count_rx = XAXIDMA_DFT_RX_THRESHOLD;
>>       priv->coalesce_count_tx = XAXIDMA_DFT_TX_THRESHOLD;
>>
>> -     err = nixge_mdio_setup(priv, pdev->dev.of_node);
>> +     mn = of_get_child_by_name(pdev->dev.of_node, "mdio");
>> +     if (!mn) {
>> +             dev_warn(&pdev->dev, "No \"mdio\" subnode found, defaulting to legacy\n");
>> +             mn = pdev->dev.of_node;
>> +     }
>> +
>> +     err = nixge_mdio_setup(priv, mn);
>
> I would suggest making this a patch of its own.

Yeah, will do.
> Also, do you need the legacy behaviour? If there are no boards out in
> the wild which this will break, just make the change.

Well all users basically use devicetree overlays that we distribute
with the FPGA images
as part of a filesystem update. So I suppose it wouldn't break anyone,
yet... It would certainly
make the code easier to read.

> Please also update the device tree binding documentation.

Yeah will do.

Thanks for the feedback,

Moritz