Messages in this thread | | | Subject | Re: [PATCH] ARM: dts: am335x-sancloud-bbe: Fix PHY mode for ethernet | From | Grygorii Strashko <> | Date | Wed, 9 Oct 2019 12:43:14 +0300 |
| |
On 08/10/2019 20:02, Tony Lindgren wrote: > * Jeroen Hofstee <jhofstee@victronenergy.com> [191008 17:00]: >> Hi, >> >> On 10/8/19 6:51 PM, Tony Lindgren wrote: >>> * Jeroen Hofstee <jhofstee@victronenergy.com> [191008 16:43]: >>>> Hello Tony, >>>> >>>> On 10/8/19 6:14 PM, Tony Lindgren wrote: >>>>> * Jeroen Hofstee <jhofstee@victronenergy.com> [191008 16:03]: >>>>>> Hello Tony, >>>>>> >>>>>> On 10/8/19 4:23 PM, Tony Lindgren wrote: >>>>>>> * Grygorii Strashko <grygorii.strashko@ti.com> [191003 02:32]: >>>>>>>> On 03/10/2019 11:16, Jeroen Hofstee wrote: >>>>>>>>> Furthermore 4.19 is fine, so there is no need to include it in stable >>>>>>>>> and have a note to make sure also other patches are required etc. >>>>>>>> Hence all above patches went in 5.1 it would be correct to mention only >>>>>>>> 6d4cd041f0af net: phy: at803x: disable delay only for RGMII mode >>>>>>> Jeroen, can you please send an updated patch with the fixes >>>>>>> tag changed? >>>>>>> >>>>>> For completeness, there is no "Fixes tag" as you mentioned. >>>>>> The commit only refers to another commit which introduces >>>>>> a problem. >>>>> Well please add the fixes tag, that way this will get >>>>> properly applied to earlier stable kernels too :) >>>> But 4.19 is fine, this is an issue in 5.1 as in EOL... >>>> I really don't understand why I should waste time >>>> to figure out what happened exactly during the 5.1 >>>> release cycle... >>> Hmm so what's the issue with just adding the fixes tag Grygorii >>> suggested: >>> >>> 6d4cd041f0af ("net: phy: at803x: disable delay only for RGMII mode") >>> >>> No need to dig further? >> >> 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: "
-- Best regards, grygorii
| |