From: Stas Sergeev <stsp@list.ru>
To: Florian Fainelli <f.fainelli@gmail.com>, netdev@vger.kernel.org
Cc: Linux kernel <linux-kernel@vger.kernel.org>,
Stas Sergeev <stsp@users.sourceforge.net>
Subject: Re: [PATCH 1/2] add fixed_phy_update_state() - update state of fixed_phy
Date: Sat, 04 Apr 2015 00:25:27 +0300 [thread overview]
Message-ID: <551F0547.8000202@list.ru> (raw)
In-Reply-To: <551EE936.4010700@gmail.com>
03.04.2015 22:25, Florian Fainelli пишет:
> On 01/04/15 10:30, Stas Sergeev wrote:
>> - The callback needs to be registered before of_phy_connect() to
>> avoid running with outdated state, but of_phy_connect() returns the
>> phy_device pointer, which is needed to register the callback. Registering
>> it before of_phy_connect() will therefore require a hack to get the
>> pointer earlier.
In fact, this can't even be done: registering it before of_phy_connect()
is an asking for troubles with NULL deref.
>> Overall, this addition makes the subsequent patch that implements
>> SGMII link status for mvneta, much cleaner.
> Agreed, now that we have that, we should probably just remove the
> ability to have a fixed link update callback since it suffers from all
> the deficiencies you outlined above, and is creating some overhead by
> polling the hardware.
In fact, there is still a bit of a problem.
You certainly want to set the initial state, so that on of_phy_connect()
time it is already valid. For this I still use of_phy_find_device()...
This, however, differs from registering the callback:
- initial state is set only once; callback should be registered before
_every_ of_phy_connect() (but it can't be registered before because
of the NULL, so only after)
- initial state can be set anywhere anytime, not necessary any near
of_phy_connect().
So the problem is mostly solved, but of_phy_find_device() is still
there. To get rid of this, I guess some addition to of_mdio may be
needed as well. Maybe optionally passing the status directly to
of_phy_connect()? Not sure.
next prev parent reply other threads:[~2015-04-03 21:25 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-01 17:28 [PATCH v3 0/2] mvneta: SGMII-based in-band link state signaling Stas Sergeev
2015-04-01 17:30 ` [PATCH 1/2] add fixed_phy_update_state() - update state of fixed_phy Stas Sergeev
2015-04-03 19:25 ` Florian Fainelli
2015-04-03 21:25 ` Stas Sergeev [this message]
2015-04-01 17:32 ` [PATCH 2/2] mvneta: implement SGMII-based in-band link state signaling Stas Sergeev
2015-04-03 19:08 ` [PATCH v3 0/2] mvneta: " David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=551F0547.8000202@list.ru \
--to=stsp@list.ru \
--cc=f.fainelli@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=stsp@users.sourceforge.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.