Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
Date:   Tue, 5 Dec 2023 14:48:25 +0300
From:   Serge Semin <fancer.lancer@gmail.com>
To:     "Russell King (Oracle)" <linux@armlinux.org.uk>
Cc:     Andrew Lunn <andrew@lunn.ch>,
        Heiner Kallweit <hkallweit1@gmail.com>,
        Alexandre Torgue <alexandre.torgue@foss.st.com>,
        Jose Abreu <joabreu@synopsys.com>,
        Jose Abreu <Jose.Abreu@synopsys.com>,
        Maxime Chevallier <maxime.chevallier@bootlin.com>,
        Tomer Maimon <tmaimon77@gmail.com>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, openbmc@lists.ozlabs.org,
        netdev@vger.kernel.org, devicetree@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH net-next 05/16] net: pcs: xpcs: Move native device ID
 macro to linux/pcs/pcs-xpcs.h
Message-ID: <syr55e2c7izap6fc2yzmz6gyzcybmmxe3dyjoxencb2tylss2p@tpu2pfh33ked>
References: <20231205103559.9605-1-fancer.lancer@gmail.com>
 <20231205103559.9605-6-fancer.lancer@gmail.com>
 <ZW7/TrtSols1igy/@shell.armlinux.org.uk>
 <bas6l42vk2zzrwo22ss7fuganf4ekvhtvkb32duydjise7ui3o@o4f3rbcpokur>
 <ZW8ICvzwIHJhoV9U@shell.armlinux.org.uk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZW8ICvzwIHJhoV9U@shell.armlinux.org.uk>
Precedence: bulk

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!