Message-ID: <syr55e2c7izap6fc2yzmz6gyzcybmmxe3dyjoxencb2tylss2p@tpu2pfh33ked>
Date: Tue, 5 Dec 2023 14:48:25 +0300
From: Serge Semin <fancer.lancer@...il.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>, 
	Alexandre Torgue <alexandre.torgue@...s.st.com>, Jose Abreu <joabreu@...opsys.com>, 
	Jose Abreu <Jose.Abreu@...opsys.com>, Maxime Chevallier <maxime.chevallier@...tlin.com>, 
	Tomer Maimon <tmaimon77@...il.com>, Rob Herring <robh+dt@...nel.org>, 
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, openbmc@...ts.ozlabs.org, 
	netdev@...r.kernel.org, devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next 05/16] net: pcs: xpcs: Move native device ID
 macro to linux/pcs/pcs-xpcs.h

On Tue, Dec 05, 2023 at 11:22:50AM +0000, Russell King (Oracle) wrote:
> On Tue, Dec 05, 2023 at 02:14:34PM +0300, Serge Semin wrote:
> > On Tue, Dec 05, 2023 at 10:45:34AM +0000, Russell King (Oracle) wrote:
> > > On Tue, Dec 05, 2023 at 01:35:26PM +0300, Serge Semin wrote:
> > > > Generic MDIO-device driver will support setting a custom device ID for the
> > > > particular MDIO-device.
> > > 
> > > Why future tense? I don't see anything later in this patch set adding
> > > this.
> > 
> > After the next patch is applied
> > [PATCH net-next 10/16] net: pcs: xpcs: Add generic DW XPCS MDIO-device support
> > the DW XPCS driver _will_ support setting custom IDs based on the
> > platform data and the DT compatibles.
> 

> What is confusing is that the sentence makes it sound like it's some
> generic driver that can be used for any PCS, whereas in reality it is
> _this_ XPCS driver which is not generic.
> 
> "This driver will support setting a custom device ID in a future patch."
> or explicitly state the summary line of the patch concerned so one can
> refer to it. Future references are difficult to find whether they're in
> email and especially once they're merged into git.

Ok. I'll convert the patch log to be less confusing. As I already said
to Vladimir writing sometimes overcomplicated messages my eternal
problem.

> 
> > It can be used for instance to
> > fix the already available SJ1105 and SJ1110 MDIO bus implementations,
> > so instead of substituting the XPCS IDs on the PHYSID CSR reads the
> > driver could just pass the device ID and PMA ID via the device
> > platform data.
> > 
> > If my patch log text looks unclear anyway, just say so. I'll change it
> > accordingly. I guess it would be enough to say that moving is required
> > just to collect all the IDs in a single place.
> 

> You need to adjust your attitude - I did exactly that. There was
> something which I didn't understand, so I raised the issue. Sorry
> for spotting a problem, but do you always get arsey when a reviewer
> picks up on something wrong? If that's your attitude, then for this
> entire series: NAK.

I'm sorry if what I wrote sounded like I was arsey. I didn't mean it
at all, really. By this sentence:

> I guess it would be enough to say that moving is required
> just to collect all the IDs in a single place.

I meant that _I_ should have just stated in the log message that
moving was required to collect all the IDs in a single place. The
rest of the text was redundant and caused confusion what you pointed
out to.

-Serge(y)

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

Powered by blists - more mailing lists