All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	AngeloGioacchino Del Regno
	<angelogioacchino.delregno@collabora.com>,
	"Chester A. Unal" <chester.a.unal@arinc9.com>,
	Daniel Golle <daniel@makrotopia.org>,
	"David S. Miller" <davem@davemloft.net>,
	DENG Qingfang <dqfext@gmail.com>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org,
	Matthias Brugger <matthias.bgg@gmail.com>,
	netdev@vger.kernel.org, Paolo Abeni <pabeni@redhat.com>,
	Sean Wang <sean.wang@mediatek.com>,
	Simon Horman <horms@kernel.org>
Subject: Re: [PATCH net-next v3 1/3] net: phylink: provide phylink_mac_implements_lpi()
Date: Mon, 10 Feb 2025 14:26:19 +0000	[thread overview]
Message-ID: <Z6oMi8rTeuwHHDt8@shell.armlinux.org.uk> (raw)
In-Reply-To: <Z6oHxrtBHAvaMqd3@shell.armlinux.org.uk>

On Mon, Feb 10, 2025 at 02:05:58PM +0000, Russell King (Oracle) wrote:
> On Mon, Feb 10, 2025 at 03:20:54PM +0200, Vladimir Oltean wrote:
> > On Mon, Feb 10, 2025 at 10:36:44AM +0000, Russell King (Oracle) wrote:
> > > diff --git a/include/linux/phylink.h b/include/linux/phylink.h
> > > index 898b00451bbf..0de78673172d 100644
> > > --- a/include/linux/phylink.h
> > > +++ b/include/linux/phylink.h
> > > @@ -737,6 +737,18 @@ static inline int phylink_get_link_timer_ns(phy_interface_t interface)
> > >  	}
> > >  }
> > >  
> > > +/**
> > > + * phylink_mac_implements_lpi() - determine if MAC implements LPI ops
> > > + * @ops: phylink_mac_ops structure
> > > + *
> > > + * Returns true if the phylink MAC operations structure indicates that the
> > > + * LPI operations have been implemented, false otherwise.
> > 
> > This is something that I only noticed for v3 because I wanted to leave a
> > review tag, so I first checked the status in patchwork, but there it says:
> > 
> > include/linux/phylink.h:749: warning: No description found for return value of 'phylink_mac_implements_lpi'
> > 
> > I am aware of this conversation from November where you raised the point
> > about tooling being able to accept the syntax without the colon as well:
> > https://lore.kernel.org/netdev/87v7wjffo6.fsf@trenco.lwn.net/
> > 
> > but it looks like it didn't go anywhere, with Jon still preferring the
> > strict syntax for now, and no follow-up that I can see. So, the current
> > conventions are not these, and you haven't specifically said anywhere
> > that you are deliberately ignoring them.
> 
> It was explained in this email as part of that thread:
> 
> https://lore.kernel.org/netdev/ZzjHH-L-ylLe0YhU@shell.armlinux.org.uk/
> 
> The reason is that it goes against natural grammar. The only time that
> "Returns:" would make sense in grammar is when listing with e.g. a
> bulleted list, where the part before the colon doesn't have to be a
> complete sentence.
> 
> This is why it's going to be an uphill battle - grammatically it is
> wrong, and thus it doesn't flow when thinking about documenting the
> return value.
> 
> If we want to go to a bulleted list, then it will be natural to add
> the colon.
> 
> I'm not going to explain to this level of detail in every email, and
> because of the grammatical nature of this, it's going to be very
> difficult to use a form that goes against proper grammar.

Also note that it follows the style already present in that file
with one exception (which is one of the few cases I remembered to use
the new format.)

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!


  reply	other threads:[~2025-02-10 14:28 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-10 10:36 [PATCH net-next v3 0/3] net: dsa: add support for phylink managed EEE Russell King (Oracle)
2025-02-10 10:36 ` [PATCH net-next v3 1/3] net: phylink: provide phylink_mac_implements_lpi() Russell King (Oracle)
2025-02-10 13:20   ` Vladimir Oltean
2025-02-10 14:05     ` Russell King (Oracle)
2025-02-10 14:26       ` Russell King (Oracle) [this message]
2025-02-13  3:40   ` patchwork-bot+netdevbpf
2025-02-10 10:36 ` [PATCH net-next v3 2/3] net: dsa: allow use of phylink managed EEE support Russell King (Oracle)
2025-02-10 10:36 ` [PATCH net-next v3 3/3] net: dsa: mt7530: convert to phylink managed EEE Russell King (Oracle)
2025-02-10 13:28 ` [PATCH net-next v3 0/3] net: dsa: add support for " Vladimir Oltean

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=Z6oMi8rTeuwHHDt8@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=andrew@lunn.ch \
    --cc=angelogioacchino.delregno@collabora.com \
    --cc=chester.a.unal@arinc9.com \
    --cc=daniel@makrotopia.org \
    --cc=davem@davemloft.net \
    --cc=dqfext@gmail.com \
    --cc=edumazet@google.com \
    --cc=hkallweit1@gmail.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=matthias.bgg@gmail.com \
    --cc=netdev@vger.kernel.org \
    --cc=olteanv@gmail.com \
    --cc=pabeni@redhat.com \
    --cc=sean.wang@mediatek.com \
    /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.