Hello Grygorri, On 10/9/19 11:43 AM, Grygorii Strashko wrote: > > >>> Grygorii doesn't suggest to add a fixes tag, just to change the >>> referenced >>> commit to another. Obviously I would like to understand why another >>> commit >>> should be referenced. And then I should read and parse the response, >>> so there >>> is no special reason, just time... >> >> OK sure. Well once you guys have the commit figured out, let me >> know what to apply. And we know Grygorii is mostly right based >> on his history of comments so best to not ignore that :) > > Sry, but I do not think my request is somehow special. > Yes, your patch is correct by itself, but commit description is not: > 1) commit cd28d1d6e52e: ("net: phy: at803x: Disable phy delay for > RGMII mode") which you've mentioned is A BUG > and should not be merged first of all (which you can find out by > reading corresponding thread). > > just try checkout that commit and apply your patch on top - networking > should not work. > > But it was merged and not reverted - instead two more patches were > applied to fix regression. > > 2) Those commits are defined final behavior (which i again explained > above) and that new behavior hardly can > be called "the bug in the at803x driver" as, unfortunately, there were > no common conclusion how default values for > RX/TX delay should be handled vs phy-mode = "rgmii-txid"/"rgmii-rxid". > Originally many PHY driver kept them default (as per boot > strapping/bootloader configuration), but now > some driver (including at803x) started disabling RX delay if > "rgmii-txid" or TX delay if "rgmii-rxid". > > Hence, pls update commit message and add proper fixes tag. smth like: > "Now after commit 6d4cd041f0af net: phy: at803x: disable delay only > for RGMII mode the driver will forcibly disable > RGMII RX delay if phy-mode = "rgmii-txid" is specified in DT which > will break networking on .. > > Hence change .. to ensure ... > Fixes: " > > Yes, that part is clear to me, and I am not saying you are incorrect, only that I would like to understand why above commit pops up (since it makes no sense to me as well, my own commit). And the reason is rather silly, I guess... I fixed it on master, thereafter checked the 5.1 branch, while keeping the fixed dtb... :( Let me check that and I will send a v2. Likely tomorrow (I am not near the device at the moment). Regards, Jeroen