| Message ID | E1tgO70-003ilF-1x@rmk-PC.armlinux.org.uk (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | net: dsa: add support for phylink managed EEE | expand |
On Fri, Feb 07, 2025 at 01:09:38PM +0000, Russell King (Oracle) wrote: > In order to allow DSA drivers to use phylink managed EEE, changes are > necessary to the DSA .set_eee() and .get_eee() methods. Where drivers > make use of phylink managed EEE, these should just pass the method on > to their phylink implementation without calling the DSA specific > operations. > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > --- > net/dsa/user.c | 21 +++++++++++++-------- > 1 file changed, 13 insertions(+), 8 deletions(-) > > diff --git a/net/dsa/user.c b/net/dsa/user.c > index 291ab1b4acc4..2e0a51c1b750 100644 > --- a/net/dsa/user.c > +++ b/net/dsa/user.c > @@ -1243,16 +1243,21 @@ static int dsa_user_set_eee(struct net_device *dev, struct ethtool_keee *e) > if (!ds->ops->support_eee || !ds->ops->support_eee(ds, dp->index)) > return -EOPNOTSUPP; > > - /* Port's PHY and MAC both need to be EEE capable */ > - if (!dev->phydev) > - return -ENODEV; > + /* If the port is using phylink managed EEE, then get_mac_eee is > + * unnecessary. You thanked me for spotting that this should have been set_mac_eee() in the comment, but you didn't update it. https://lore.kernel.org/netdev/Z4bC77mwoeypDAdH@shell.armlinux.org.uk/ > + */ > + if (!phylink_mac_implements_lpi(ds->phylink_mac_ops)) { > + /* Port's PHY and MAC both need to be EEE capable */ > + if (!dev->phydev) > + return -ENODEV; > > - if (!ds->ops->set_mac_eee) > - return -EOPNOTSUPP; > + if (!ds->ops->set_mac_eee) > + return -EOPNOTSUPP; > > - ret = ds->ops->set_mac_eee(ds, dp->index, e); > - if (ret) > - return ret; > + ret = ds->ops->set_mac_eee(ds, dp->index, e); > + if (ret) > + return ret; > + } > > return phylink_ethtool_set_eee(dp->pl, e); > } > -- > 2.30.2 >
On Fri, Feb 07, 2025 at 05:19:59PM +0200, Vladimir Oltean wrote: > On Fri, Feb 07, 2025 at 01:09:38PM +0000, Russell King (Oracle) wrote: > > In order to allow DSA drivers to use phylink managed EEE, changes are > > necessary to the DSA .set_eee() and .get_eee() methods. Where drivers > > make use of phylink managed EEE, these should just pass the method on > > to their phylink implementation without calling the DSA specific > > operations. > > > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> > > --- > > net/dsa/user.c | 21 +++++++++++++-------- > > 1 file changed, 13 insertions(+), 8 deletions(-) > > > > diff --git a/net/dsa/user.c b/net/dsa/user.c > > index 291ab1b4acc4..2e0a51c1b750 100644 > > --- a/net/dsa/user.c > > +++ b/net/dsa/user.c > > @@ -1243,16 +1243,21 @@ static int dsa_user_set_eee(struct net_device *dev, struct ethtool_keee *e) > > if (!ds->ops->support_eee || !ds->ops->support_eee(ds, dp->index)) > > return -EOPNOTSUPP; > > > > - /* Port's PHY and MAC both need to be EEE capable */ > > - if (!dev->phydev) > > - return -ENODEV; > > + /* If the port is using phylink managed EEE, then get_mac_eee is > > + * unnecessary. > > You thanked me for spotting that this should have been set_mac_eee() in > the comment, but you didn't update it. > https://lore.kernel.org/netdev/Z4bC77mwoeypDAdH@shell.armlinux.org.uk/ Bah - actually I _did_ update the patch, but in a different tree: http://git.armlinux.org.uk/cgit/linux-arm.git/commit/?h=net-queue&id=2ee2b1d5f0f8cee7dd748cfae9c20af429d5a4c2 I'm now looking at this wondering what updates were made in each tree and therefore what I need to pass across between the two trees...
On Fri, Feb 07, 2025 at 03:33:46PM +0000, Russell King (Oracle) wrote:
> Bah - actually I _did_ update the patch, but in a different tree:
I'm glad there's a reasonable explanation for what appears to be an
oversight on your part, and I wish to make nothing more out of the event
in itself.
I just want you to know that it was really hard for me, looking back in
that thread to make sure I'm not misremembering that I asked for this
change, to get reminded how quick you were to jump to an insulting and
sarcastic conclusion in that same reply. It was a shit reaction and it
really didn't sit well with me then, and it still doesn't sit any better
re-reading it 3 weeks later either.
I don't like it that the more subtle hints apparently pass you by, like
me leaving a link to your entire reply, so I have to tell you straight,
even though I like that even less. I want to respect you and I'm very
careful with my words around you, Russell, but I need to know that you're
mindful of your own reactions too, because otherwise it makes me feel
that we're on an unequal footing.
Whenever you make a sarcastic comment about how people don't do what you
expect them to, as if the only possible explanation is that they must
not have read what you said, because if they did they must be dumb or
something, you just put yourself up on a pedestal where you're above
making mistakes and above misunderstanding or not thinking things
through - which is such an obtuse way of looking at life. And so, when
you make mistakes like all humans do, it's really hard for me to react
in a way that isn't just giving you back a little bit of your own venom.
But I've tried that before, and it led nowhere, so now I'm trying to be
kind, which to my desperation seems to be treated neutrally/not seen,
and it's just too much for me.
You know how with Andrew, whenever he says something on the mailing
lists that you disagree with, you never make a sarcastic comment
about it? I want that too, and not because I dare to compare myself
with Andrew technically, but because the way in which you treat a
human should not be based solely on your respect for their technical
prowess, and you shouldn't get away with reactions that stink because
you're good technically.
On Fri, Feb 07, 2025 at 11:38:23PM +0200, Vladimir Oltean wrote: > On Fri, Feb 07, 2025 at 03:33:46PM +0000, Russell King (Oracle) wrote: > > Bah - actually I _did_ update the patch, but in a different tree: > > I'm glad there's a reasonable explanation for what appears to be an > oversight on your part, and I wish to make nothing more out of the event > in itself. > > I just want you to know that it was really hard for me, looking back in > that thread to make sure I'm not misremembering that I asked for this > change, to get reminded how quick you were to jump to an insulting and > sarcastic conclusion in that same reply. It was a shit reaction and it > really didn't sit well with me then, and it still doesn't sit any better > re-reading it 3 weeks later either. It was not intended to be insulting and sarcastic. It was based on a general observation from a whole raft of patch series that has led me to the conclusion that cover messages are basically completely ignored. Many times, I've posed questions in cover messages... and have _never_ got a response to those questions. I'd even started experimenting putting stuff in the cover messages to provoke some kind of a response... and got nothing. I had got to the point of wondering why I'm bothering to write expansive and detailed cover messages. Everything was pointing towards cover messages being wholesale ignored - and it was a complete waste of time writing them. Having got to that point, to then get asked, basically, "why are you including this patch" when I already said in the cover message for the _RFC_ series (requesting comments - obviously not a submission) that the 10th patch was preparatory for DSA, and would be included along with a user when it's submitted - that all suggested to me that, yet again, the cover message hadn't been read. You've said that you do read them (thanks) but I think you're probably the only one, and I still question their value given the lack of engagement with cover messages. This is a serious point. What use are cover messages on patch series beyond providing an anchor to tie a patch series to and provide a diffstat? Given my observations over the last year, I don't think including very much else adds any value. Certainly not asking pertinent questions. Certainly not describing the patches in the series. Certainly not describing the overall rationale, nor (apparently) the plan for the patches. You may have noticed that I'm no longer putting as much effort into cover messages - and this is precisely why. I honestly don't see that there's any point in spending much time on cover messages anymore.
On Fri, Feb 07, 2025 at 10:24:27PM +0000, Russell King (Oracle) wrote: > You may have noticed that I'm no longer putting as much effort into > cover messages - and this is precisely why. I honestly don't see > that there's any point in spending much time on cover messages anymore. I have things to do and places to be, and I'm sure you do too, so I won't drag this out more than I need to. I'll just give you one self-contained example of what bothers me, from this very thread. I'm not even expecting you to change all of a sudden, I know it's hard and takes time and a deeper understanding of what is happening. First of all, if "I tend not to read cover messages" isn't sarcastic when you've make it pretty obvious so many times that this is important to you, then I don't know what sarcasm is. So, your message starts off from the very beginning by not acknowledging the way in which your reaction is perceived. Then, your message goes off to justify your reaction by essentially reiterating what you've publicly said elsewhere about not being noticed, making things sew with no Singer sewing machines required, and what not. Apart from the fact that you seem to build certain expectations of others' reactions in your mind and behave bizarrely when they react differently to how you expect (or not at all), my personal problem is that you've responded to a complaint which was really hard for me to verbalize by talking about you, again. Everything I've said in the previous email, about you expecting more respect and attention than you are willing to give, still stands. You are an important and valued member of the community, but you don't have to be so hard on people when they don't rise to your expectations. Your inflexibility makes it difficult at least for me to find a place around you.
On Mon, Feb 10, 2025 at 02:37:01PM +0200, Vladimir Oltean wrote: > On Fri, Feb 07, 2025 at 10:24:27PM +0000, Russell King (Oracle) wrote: > > You may have noticed that I'm no longer putting as much effort into > > cover messages - and this is precisely why. I honestly don't see > > that there's any point in spending much time on cover messages anymore. > > I have things to do and places to be, and I'm sure you do too, so > I won't drag this out more than I need to. I'll just give you one > self-contained example of what bothers me, from this very thread. > I'm not even expecting you to change all of a sudden, I know it's hard > and takes time and a deeper understanding of what is happening. > > First of all, if "I tend not to read cover messages" isn't sarcastic > when you've make it pretty obvious so many times that this is important > to you, then I don't know what sarcasm is. Sorry, but I disagree with you on this. I'm getting tired of having to tip-toe around issues like this, I'm not going to spend anymore time on this thread.
diff --git a/net/dsa/user.c b/net/dsa/user.c index 291ab1b4acc4..2e0a51c1b750 100644 --- a/net/dsa/user.c +++ b/net/dsa/user.c @@ -1243,16 +1243,21 @@ static int dsa_user_set_eee(struct net_device *dev, struct ethtool_keee *e) if (!ds->ops->support_eee || !ds->ops->support_eee(ds, dp->index)) return -EOPNOTSUPP; - /* Port's PHY and MAC both need to be EEE capable */ - if (!dev->phydev) - return -ENODEV; + /* If the port is using phylink managed EEE, then get_mac_eee is + * unnecessary. + */ + if (!phylink_mac_implements_lpi(ds->phylink_mac_ops)) { + /* Port's PHY and MAC both need to be EEE capable */ + if (!dev->phydev) + return -ENODEV; - if (!ds->ops->set_mac_eee) - return -EOPNOTSUPP; + if (!ds->ops->set_mac_eee) + return -EOPNOTSUPP; - ret = ds->ops->set_mac_eee(ds, dp->index, e); - if (ret) - return ret; + ret = ds->ops->set_mac_eee(ds, dp->index, e); + if (ret) + return ret; + } return phylink_ethtool_set_eee(dp->pl, e); }
In order to allow DSA drivers to use phylink managed EEE, changes are necessary to the DSA .set_eee() and .get_eee() methods. Where drivers make use of phylink managed EEE, these should just pass the method on to their phylink implementation without calling the DSA specific operations. Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> --- net/dsa/user.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-)