| Message ID | 8f2f4c517f1664f362badc2a0f9af8e3531cba87.1558105715.git.jan.kundrat@cesnet.cz (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Commit | cc538ca4308372e81b824be08561c466b1d73b72 |
| Headers | show |
| Series | ARM: mvebu_v7_defconfig: fix Ethernet on Clearfog | expand |
Hi Jan Kundrát, > Compared to kernel 5.1, patches merged for 5.2 added support for A38x' > PHY guarded by a config option which was not enabled by default. As a > result, there was no eth1 and eth2 on a Solid Run Clearfog Base. > > Ensure that A38x PHY is enabled on mvebu. > Applied on mvebu/arm-5.3 Thanks, Gregory > Signed-off-by: Jan Kundrát <jan.kundrat@cesnet.cz> > Cc: Baruch Siach <baruch@tkos.co.il> > Cc: Gregory CLEMENT <gregory.clement@bootlin.com> > Cc: Russell King <rmk+kernel@armlinux.org.uk> > Cc: David S. Miller <davem@davemloft.net> > Cc: Maxime Chevallier <maxime.chevallier@bootlin.com> > --- > arch/arm/configs/mvebu_v7_defconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/configs/mvebu_v7_defconfig b/arch/arm/configs/mvebu_v7_defconfig > index 48f7b4277b8d..3ac2e84fdeaa 100644 > --- a/arch/arm/configs/mvebu_v7_defconfig > +++ b/arch/arm/configs/mvebu_v7_defconfig > @@ -131,6 +131,7 @@ CONFIG_MV_XOR=y > # CONFIG_IOMMU_SUPPORT is not set > CONFIG_MEMORY=y > CONFIG_PWM=y > +CONFIG_PHY_MVEBU_A38X_COMPHY=y > CONFIG_EXT4_FS=y > CONFIG_ISO9660_FS=y > CONFIG_JOLIET=y > -- > 2.21.0 > >
> Applied on mvebu/arm-5.3
To me this looks like a 5.2 material because it fixes a regression that got
introduced within the 5.2 cycle. Or did I get the process wrong and this is
perhaps just a first step?
Cheers,
Jan
Jan Kundrát <jan.kundrat@cesnet.cz> writes: >> Applied on mvebu/arm-5.3 > > To me this looks like a 5.2 material because it fixes a regression that got > introduced within the 5.2 cycle. Or did I get the process wrong and this is > perhaps just a first step? Well, this is just about configuration, I don't consider this is something that is a candidate for a fix. If there is a regression, then, it is maybe located in the Kconfig dependency. Of course I can change my mind with good arguments :) Gregory > > Cheers, > Jan
> Well, this is just about configuration, I don't consider this is > something that is a candidate for a fix. > > If there is a regression, then, it is maybe located in the Kconfig > dependency. > > Of course I can change my mind with good arguments :) Hi Gregory, I agree that it's just a config bug, but it's also something which can silently produce broken systems. If this is not fixed, people building their 5.2 kernels will not have working network on Clearfog unless they take an extra action. For example, a Buildroot defconfig that's been available for quite some time (and which uses just `mvebu_v7_defconfig` for kernel) suddenly becomes broken. Isn't the whole point of the -rc release to find *and* fix bugs early? This trivial patch does not introduce any new or untested code. I made a choice to test a pre-release kernel, I hit a bug -- no big deal. I found the root cause, I sent a trivial fix upstream, and now I'm told by a maintainer that they will let the next kernel version, which is about seven -rc releases away, be released without a fully functioning network, I am surprised by that. I would have understood this better if we were at the final -rc stage, but during the merge window? Or is that perhaps a misunderstanding and you're planning to send this in time after -rc1? Cheers, Jan
On sobota 18. května 2019 0:50:28 CEST, Jan Kundrát wrote: >> Well, this is just about configuration, I don't consider this is >> something that is a candidate for a fix. >> >> If there is a regression, then, it is maybe located in the Kconfig >> dependency. >> >> Of course I can change my mind with good arguments :) > > Hi Gregory, > I agree that it's just a config bug, but it's also something > which can silently produce broken systems. If this is not fixed, > people building their 5.2 kernels will not have working network > on Clearfog unless they take an extra action. For example, a > Buildroot defconfig that's been available for quite some time > (and which uses just `mvebu_v7_defconfig` for kernel) suddenly > becomes broken. > > Isn't the whole point of the -rc release to find *and* fix bugs > early? This trivial patch does not introduce any new or untested > code. I made a choice to test a pre-release kernel, I hit a bug > -- no big deal. I found the root cause, I sent a trivial fix > upstream, and now I'm told by a maintainer that they will let > the next kernel version, which is about seven -rc releases away, > be released without a fully functioning network, I am surprised > by that. I would have understood this better if we were at the > final -rc stage, but during the merge window? Or is that perhaps > a misunderstanding and you're planning to send this in time > after -rc1? Hi Gregory, was I successful in persuading you that this patch should be included in the 5.2 tree, so that Clearfog Base has all three Ethernet interfaces? With kind regards, Jan
Jan Kundrát <jan.kundrat@cesnet.cz> writes: > On sobota 18. května 2019 0:50:28 CEST, Jan Kundrát wrote: >>> Well, this is just about configuration, I don't consider this is >>> something that is a candidate for a fix. >>> >>> If there is a regression, then, it is maybe located in the Kconfig >>> dependency. >>> >>> Of course I can change my mind with good arguments :) >> >> Hi Gregory, >> I agree that it's just a config bug, but it's also something >> which can silently produce broken systems. If this is not fixed, >> people building their 5.2 kernels will not have working network >> on Clearfog unless they take an extra action. For example, a >> Buildroot defconfig that's been available for quite some time >> (and which uses just `mvebu_v7_defconfig` for kernel) suddenly >> becomes broken. >> >> Isn't the whole point of the -rc release to find *and* fix bugs >> early? This trivial patch does not introduce any new or untested >> code. I made a choice to test a pre-release kernel, I hit a bug >> -- no big deal. I found the root cause, I sent a trivial fix >> upstream, and now I'm told by a maintainer that they will let >> the next kernel version, which is about seven -rc releases away, >> be released without a fully functioning network, I am surprised >> by that. I would have understood this better if we were at the >> final -rc stage, but during the merge window? Or is that perhaps >> a misunderstanding and you're planning to send this in time >> after -rc1? > > Hi Gregory, > was I successful in persuading you that this patch should be included in > the 5.2 tree, so that Clearfog Base has all three Ethernet interfaces? Finally I moved the commit from mvebu/arm to mvebu/fixes. I still think the problem is at driver level, but I didn't take enough time to find where and we didn't have any feedback from the author of the initila patch. So let's try to push it to fixes, I will do the pull request for arm-soc before end of the week. Gregory > > With kind regards, > Jan
On úterý 11. června 2019 13:28:32 CEST, Gregory CLEMENT wrote: > Jan Kundrát <jan.kundrat@cesnet.cz> writes: > >> On sobota 18. května 2019 0:50:28 CEST, Jan Kundrát wrote: ... > > Finally I moved the commit from mvebu/arm to mvebu/fixes. I still think > the problem is at driver level, but I didn't take enough time to find > where and we didn't have any feedback from the author of the initila > patch. > > So let's try to push it to fixes, I will do the pull request for arm-soc > before end of the week. Thanks! Jan
On Tue, Jun 11, 2019 at 01:28:32PM +0200, Gregory CLEMENT wrote: > Jan Kundrát <jan.kundrat@cesnet.cz> writes: > > > On sobota 18. května 2019 0:50:28 CEST, Jan Kundrát wrote: > >>> Well, this is just about configuration, I don't consider this is > >>> something that is a candidate for a fix. > >>> > >>> If there is a regression, then, it is maybe located in the Kconfig > >>> dependency. > >>> > >>> Of course I can change my mind with good arguments :) > >> > >> Hi Gregory, > >> I agree that it's just a config bug, but it's also something > >> which can silently produce broken systems. If this is not fixed, > >> people building their 5.2 kernels will not have working network > >> on Clearfog unless they take an extra action. For example, a > >> Buildroot defconfig that's been available for quite some time > >> (and which uses just `mvebu_v7_defconfig` for kernel) suddenly > >> becomes broken. > >> > >> Isn't the whole point of the -rc release to find *and* fix bugs > >> early? This trivial patch does not introduce any new or untested > >> code. I made a choice to test a pre-release kernel, I hit a bug > >> -- no big deal. I found the root cause, I sent a trivial fix > >> upstream, and now I'm told by a maintainer that they will let > >> the next kernel version, which is about seven -rc releases away, > >> be released without a fully functioning network, I am surprised > >> by that. I would have understood this better if we were at the > >> final -rc stage, but during the merge window? Or is that perhaps > >> a misunderstanding and you're planning to send this in time > >> after -rc1? > > > > Hi Gregory, > > was I successful in persuading you that this patch should be included in > > the 5.2 tree, so that Clearfog Base has all three Ethernet interfaces? > > Finally I moved the commit from mvebu/arm to mvebu/fixes. I still think > the problem is at driver level, but I didn't take enough time to find > where and we didn't have any feedback from the author of the initila > patch. I don't see that there's much that I need to say, and I'm at a total loss to work out why you think it's a problem at driver level. Why do you think it's appropriate for mvneta to know whether the a38x comphy driver is configured for the current kernel or not, given that mvneta is not exclusively used on Armada 38x systems? We're already doing the best we can do with ignoring the comphy if not present; the only case that we defer probe is when devm_of_phy_get() returns -EPROBE_DEFER, which could mean "the comphy driver is a module but is not loaded yet" or "the comphy driver has not been probed yet" - we can't ignore those. > So let's try to push it to fixes, I will do the pull request for arm-soc > before end of the week. That's the correct solution, and it should also have a Fixes: tag on it. Unfortunately, keeping the defconfigs up to date is quite a hard problem unless you have lots of computing power to build and boot all the defconfigs on all platforms (I don't.)
Hello Russell King, > On Tue, Jun 11, 2019 at 01:28:32PM +0200, Gregory CLEMENT wrote: >> Jan Kundrát <jan.kundrat@cesnet.cz> writes: >> >> > On sobota 18. května 2019 0:50:28 CEST, Jan Kundrát wrote: >> >>> Well, this is just about configuration, I don't consider this is >> >>> something that is a candidate for a fix. >> >>> >> >>> If there is a regression, then, it is maybe located in the Kconfig >> >>> dependency. >> >>> >> >>> Of course I can change my mind with good arguments :) >> >> >> >> Hi Gregory, >> >> I agree that it's just a config bug, but it's also something >> >> which can silently produce broken systems. If this is not fixed, >> >> people building their 5.2 kernels will not have working network >> >> on Clearfog unless they take an extra action. For example, a >> >> Buildroot defconfig that's been available for quite some time >> >> (and which uses just `mvebu_v7_defconfig` for kernel) suddenly >> >> becomes broken. >> >> >> >> Isn't the whole point of the -rc release to find *and* fix bugs >> >> early? This trivial patch does not introduce any new or untested >> >> code. I made a choice to test a pre-release kernel, I hit a bug >> >> -- no big deal. I found the root cause, I sent a trivial fix >> >> upstream, and now I'm told by a maintainer that they will let >> >> the next kernel version, which is about seven -rc releases away, >> >> be released without a fully functioning network, I am surprised >> >> by that. I would have understood this better if we were at the >> >> final -rc stage, but during the merge window? Or is that perhaps >> >> a misunderstanding and you're planning to send this in time >> >> after -rc1? >> > >> > Hi Gregory, >> > was I successful in persuading you that this patch should be included in >> > the 5.2 tree, so that Clearfog Base has all three Ethernet interfaces? >> >> Finally I moved the commit from mvebu/arm to mvebu/fixes. I still think >> the problem is at driver level, but I didn't take enough time to find >> where and we didn't have any feedback from the author of the initila >> patch. > > I don't see that there's much that I need to say, and I'm at a total > loss to work out why you think it's a problem at driver level. > > Why do you think it's appropriate for mvneta to know whether the a38x > comphy driver is configured for the current kernel or not, given that > mvneta is not exclusively used on Armada 38x systems? > > We're already doing the best we can do with ignoring the comphy if > not present; the only case that we defer probe is when > devm_of_phy_get() returns -EPROBE_DEFER, which could mean "the comphy > driver is a module but is not loaded yet" or "the comphy driver has > not been probed yet" - we can't ignore those. > >> So let's try to push it to fixes, I will do the pull request for arm-soc >> before end of the week. > > That's the correct solution, and it should also have a Fixes: tag on > it. Unfortunately, keeping the defconfigs up to date is quite a hard > problem unless you have lots of computing power to build and boot all > the defconfigs on all platforms (I don't.) Thanks for the explanation and confirmation. As the issue appeared only in 5.2-rc1 then it won't be backported and for my point of view the main interest of Fixes is for the backport, that's I though that not having a Fixes tag was an issue. But I don't mind adding a Fixes: tag. Jan, do you find which exact commit cause this regression? Thanks, Gregory > > -- > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ > FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up > According to speedtest.net: 11.9Mbps down 500kbps up
On úterý 11. června 2019 17:04:38 CEST, Gregory CLEMENT wrote:
> Jan, do you find which exact commit cause this regression?
I have not bisected, but I think that it is due to
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a10c1c8191e04c21769656c2ca8e1c69a6218954
No guarantees there, though.
Cheers,
Jan
On Tue, Jun 11, 2019 at 05:12:45PM +0200, Jan Kundrát wrote: > On úterý 11. června 2019 17:04:38 CEST, Gregory CLEMENT wrote: > > Jan, do you find which exact commit cause this regression? > > I have not bisected, but I think that it is due to https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a10c1c8191e04c21769656c2ca8e1c69a6218954 > > No guarantees there, though. Correct, which means it does need backporting to 5.1, since that was introduced in the 5.1 merge window.
>> I have not bisected, but I think that it is due to >> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a10c1c8191e04c21769656c2ca8e1c69a6218954 >> >> No guarantees there, though. > > Correct, which means it does need backporting to 5.1, since that was > introduced in the 5.1 merge window. ...which means that I made a mistake during my initial report. If it's in 5.1 already, then 5.1 could not have worked for me. Sorry for confusion, it's quite possible that I compared 5.2-rcX with 5.0.y. Jan
Jan Kundrát <jan.kundrat@cesnet.cz> writes: >>> I have not bisected, but I think that it is due to >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a10c1c8191e04c21769656c2ca8e1c69a6218954 >>> >>> No guarantees there, though. >> >> Correct, which means it does need backporting to 5.1, since that was >> introduced in the 5.1 merge window. > > ...which means that I made a mistake during my initial report. If it's in > 5.1 already, then 5.1 could not have worked for me. Sorry for confusion, > it's quite possible that I compared 5.2-rcX with 5.0.y. So I added the Fixes tag and correct the commit log, mentioning 5.1 instead of 5.2. The commit is in mvebu/fixes and for-next on git://git.infradead.org/linux-mvebu.git and will be part of the Fixes pull request I will do at the end of the week. Gregory > > Jan > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff --git a/arch/arm/configs/mvebu_v7_defconfig b/arch/arm/configs/mvebu_v7_defconfig index 48f7b4277b8d..3ac2e84fdeaa 100644 --- a/arch/arm/configs/mvebu_v7_defconfig +++ b/arch/arm/configs/mvebu_v7_defconfig @@ -131,6 +131,7 @@ CONFIG_MV_XOR=y # CONFIG_IOMMU_SUPPORT is not set CONFIG_MEMORY=y CONFIG_PWM=y +CONFIG_PHY_MVEBU_A38X_COMPHY=y CONFIG_EXT4_FS=y CONFIG_ISO9660_FS=y CONFIG_JOLIET=y
Compared to kernel 5.1, patches merged for 5.2 added support for A38x' PHY guarded by a config option which was not enabled by default. As a result, there was no eth1 and eth2 on a Solid Run Clearfog Base. Ensure that A38x PHY is enabled on mvebu. Signed-off-by: Jan Kundrát <jan.kundrat@cesnet.cz> Cc: Baruch Siach <baruch@tkos.co.il> Cc: Gregory CLEMENT <gregory.clement@bootlin.com> Cc: Russell King <rmk+kernel@armlinux.org.uk> Cc: David S. Miller <davem@davemloft.net> Cc: Maxime Chevallier <maxime.chevallier@bootlin.com> --- arch/arm/configs/mvebu_v7_defconfig | 1 + 1 file changed, 1 insertion(+)