On Wed, Feb 22, 2023 at 11:05:10PM +0100, Marek Vasut wrote: > On 2/22/23 22:08, Vladimir Oltean wrote: > > Please summarize in the commit title what is the user-visible impact of > > the problem that is being fixed. Short and to the point. > > Can you suggest a Subject which is acceptable ? Nope. The thing is, I don't know what you're seeing, only you do. I can only review and comment if it's plausible or not. I'm sure you can come up with something. > > > Currently, the driver uses PORT read function on register P_XMII_CTRL_1 > > > to access the P_GMII_1GBIT_M, i.e. Is_1Gbps, bit. > > > > Provably false. The driver does do that, but not for KSZ87xx. > > The driver uses port read function with register value 0x56 instead of 0x06 > , which means the remapping happens twice, which provably breaks the driver > since commit Fixes below . The sentence is false in the context of ksz87xx, which is what is the implied context of this patch (see commit title written by yourself). The P_GMII_1GBIT_M field is not accessed, and that is a bug in itself. Also, the (lack of) access to the P_GMII_1GBIT_M field is not what causes the breakage that you see, but to other fields from that register. > > There is no call site other than ksz_set_xmii(). Please delete false > > information from the commit message. > > $ git grep P_XMII_CTRL_1 drivers/net/dsa/microchip/ > drivers/net/dsa/microchip/ksz_common.c: [P_XMII_CTRL_1] = 0x06, > drivers/net/dsa/microchip/ksz_common.c: [P_XMII_CTRL_1] = 0x0301, > drivers/net/dsa/microchip/ksz_common.c: ksz_pread8(dev, port, regs[P_XMII_CTRL_1], &data8); > drivers/net/dsa/microchip/ksz_common.c: ksz_pwrite8(dev, port, regs[P_XMII_CTRL_1], data8); > drivers/net/dsa/microchip/ksz_common.c: ksz_pread8(dev, port, regs[P_XMII_CTRL_1], &data8); > drivers/net/dsa/microchip/ksz_common.c: ksz_pread8(dev, port, regs[P_XMII_CTRL_1], &data8); > drivers/net/dsa/microchip/ksz_common.c: ksz_pread8(dev, port, regs[P_XMII_CTRL_1], &data8); > drivers/net/dsa/microchip/ksz_common.c: ksz_pwrite8(dev, port, regs[P_XMII_CTRL_1], data8); > drivers/net/dsa/microchip/ksz_common.h: P_XMII_CTRL_1, > > I count 6. So your response to 2 reviewers wasting their time to do a detailed analysis of the code paths that apply to the KSZ87xx model in particular, to tell you precisely why your commit message is incorrect is "git grep"? > OK, to make this simple, can you write a commit message which you consider > acceptable, to close this discussion ? Nope. The thing is, I'm sure you can, too. Maybe you need to take a break and think about this some more.
