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.
