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!
