On 1/21/19 12:36 PM, Heiner Kallweit wrote:
> On 21.01.2019 17:35, Andrew Lunn wrote:
>> On Sun, Jan 20, 2019 at 10:01:15AM +0100, Heiner Kallweit wrote:
>>> The state machine is a no-op before phy_start() has been called.
>>> Therefore let's enable it in phy_start() only. In phy_start()
>>> let's call phy_start_machine() instead of phy_trigger_machine().
>>> phy_start_machine is an alias for phy_trigger_machine but it makes
>>> clearer that we start the state machine here instead of just
>>> triggering a run.
>>
>> Hi Heiner
>>
>> Documentation/networking/phy.txt has a section "Doing it all yourself"
>> It would be good to review that, and make sure that documentation is
>> still valid. I'm not sure any MAC driver actually does do it all
>> itself. So it might be worth reviewing the whole document and making
>> updates to remove parts of the text.
>>
> Right. I figured out that I have update phy.txt anyway because I
> recently removed phy_stop_interrupts which is referenced in the
> documentation. OK if we leave the patch series as is and I submit
> the documentation update as a separate patch?
I think you need to be careful here and not break what is allowed in the
"Doing it all yourself" section. The amd-xgbe driver makes use of this
functionality and does not use phy_start()/phy_stop(). Specifically, it
does:
get_phy_device();
phy_device_register();
phy_attach_direct();
At which point it uses phy_start_aneg(), phy_read(), phy_write(),
phy_read_status() and phy_aneg_done().
I'm not sure what other drivers out there that make use of this support
within phylib.
Btw, I did notice this revert that was applied that eliminated a warning
that I started seeing in 5.0, so that is good:
d9f903f6af3d ("net: phy: fix too strict check in phy_start_aneg")
Thanks,
Tom
>
>> Andrew
>>
>>
> Heiner
>
