| Message ID | 20150708073238.389781888@linuxfoundation.org |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | David Miller |
| Headers | show |
08.07.2015 10:35, Greg Kroah-Hartman пишет: > 4.1-stable review patch. If anyone has any objections, please let me know. > > ------------------ > > From: Stas Sergeev <stsp@list.ru> > > [ Upstream commit 538761b794c1542f1c6e31eadd9d7aae118889f7 ] > > The commit 898b2970e2c9 ("mvneta: implement SGMII-based in-band link state > signaling") > changed mvneta_adjust_link() so that it does not clear the auto-negotiation > bits in MVNETA_GMAC_AUTONEG_CONFIG register. This was necessary for > auto-negotiation mode to work. > Unfortunately I haven't checked if these bits are ever initialized. > It appears they are not. > This patch adds the missing initialization of the auto-negotiation bits > in the MVNETA_GMAC_AUTONEG_CONFIG register. > It fixes the following regression: > https://www.mail-archive.com/netdev@vger.kernel.org/msg67928.html > > Since the patch was tested to fix a regression, it should be applied to > stable tree. > > Tested-by: Arnaud Ebalard <arno@natisbad.org> > > CC: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > CC: Florian Fainelli <f.fainelli@gmail.com> > CC: netdev@vger.kernel.org > CC: linux-kernel@vger.kernel.org > CC: stable@vger.kernel.org > > Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net> > Signed-off-by: David S. Miller <davem@davemloft.net> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > --- > drivers/net/ethernet/marvell/mvneta.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > --- a/drivers/net/ethernet/marvell/mvneta.c > +++ b/drivers/net/ethernet/marvell/mvneta.c > @@ -1013,6 +1013,12 @@ static void mvneta_defaults_set(struct m > val = mvreg_read(pp, MVNETA_GMAC_CLOCK_DIVIDER); > val |= MVNETA_GMAC_1MS_CLOCK_ENABLE; > mvreg_write(pp, MVNETA_GMAC_CLOCK_DIVIDER, val); > + } else { > + val = mvreg_read(pp, MVNETA_GMAC_AUTONEG_CONFIG); > + val &= ~(MVNETA_GMAC_INBAND_AN_ENABLE | > + MVNETA_GMAC_AN_SPEED_EN | > + MVNETA_GMAC_AN_DUPLEX_EN); > + mvreg_write(pp, MVNETA_GMAC_AUTONEG_CONFIG, val); > } > > mvneta_set_ucast_table(pp, -1); Hi Greg. Another problem was reported: https://lkml.org/lkml/2015/7/8/865 So, while the above patch is correct and fixes what it should, the original patch has more problems to deal with. Maybe for stable it would be better to just revert the whole thing? I guess for stable I'll be posting the reverts instead of the above fix. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jul 08, 2015 at 08:10:46PM +0300, Stas Sergeev wrote: > 08.07.2015 10:35, Greg Kroah-Hartman пишет: > >4.1-stable review patch. If anyone has any objections, please let me know. > > > >------------------ > > > >From: Stas Sergeev <stsp@list.ru> > > > >[ Upstream commit 538761b794c1542f1c6e31eadd9d7aae118889f7 ] > > > >The commit 898b2970e2c9 ("mvneta: implement SGMII-based in-band link state > >signaling") > >changed mvneta_adjust_link() so that it does not clear the auto-negotiation > >bits in MVNETA_GMAC_AUTONEG_CONFIG register. This was necessary for > >auto-negotiation mode to work. > >Unfortunately I haven't checked if these bits are ever initialized. > >It appears they are not. > >This patch adds the missing initialization of the auto-negotiation bits > >in the MVNETA_GMAC_AUTONEG_CONFIG register. > >It fixes the following regression: > >https://www.mail-archive.com/netdev@vger.kernel.org/msg67928.html > > > >Since the patch was tested to fix a regression, it should be applied to > >stable tree. > > > >Tested-by: Arnaud Ebalard <arno@natisbad.org> > > > >CC: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > >CC: Florian Fainelli <f.fainelli@gmail.com> > >CC: netdev@vger.kernel.org > >CC: linux-kernel@vger.kernel.org > >CC: stable@vger.kernel.org > > > >Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net> > >Signed-off-by: David S. Miller <davem@davemloft.net> > >Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > >--- > > drivers/net/ethernet/marvell/mvneta.c | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > >--- a/drivers/net/ethernet/marvell/mvneta.c > >+++ b/drivers/net/ethernet/marvell/mvneta.c > >@@ -1013,6 +1013,12 @@ static void mvneta_defaults_set(struct m > > val = mvreg_read(pp, MVNETA_GMAC_CLOCK_DIVIDER); > > val |= MVNETA_GMAC_1MS_CLOCK_ENABLE; > > mvreg_write(pp, MVNETA_GMAC_CLOCK_DIVIDER, val); > >+ } else { > >+ val = mvreg_read(pp, MVNETA_GMAC_AUTONEG_CONFIG); > >+ val &= ~(MVNETA_GMAC_INBAND_AN_ENABLE | > >+ MVNETA_GMAC_AN_SPEED_EN | > >+ MVNETA_GMAC_AN_DUPLEX_EN); > >+ mvreg_write(pp, MVNETA_GMAC_AUTONEG_CONFIG, val); > > } > > mvneta_set_ucast_table(pp, -1); > Hi Greg. > > Another problem was reported: > https://lkml.org/lkml/2015/7/8/865 > > So, while the above patch is correct and fixes what > it should, the original patch has more problems to deal > with. Maybe for stable it would be better to just revert > the whole thing? No, you will have to fix this in Linus's tree, right? So I'll take the patch that you get into there when that happens, I don't want to diverge from what is in that tree. > I guess for stable I'll be posting the reverts instead of > the above fix. If you are reverting patches in Linus's tree, I'll take that as well. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
08.07.2015 20:36, Greg Kroah-Hartman пишет: > On Wed, Jul 08, 2015 at 08:10:46PM +0300, Stas Sergeev wrote: >> 08.07.2015 10:35, Greg Kroah-Hartman пишет: >>> 4.1-stable review patch. If anyone has any objections, please let me know. >>> >>> ------------------ >>> >>> From: Stas Sergeev <stsp@list.ru> >>> >>> [ Upstream commit 538761b794c1542f1c6e31eadd9d7aae118889f7 ] >>> >>> The commit 898b2970e2c9 ("mvneta: implement SGMII-based in-band link state >>> signaling") >>> changed mvneta_adjust_link() so that it does not clear the auto-negotiation >>> bits in MVNETA_GMAC_AUTONEG_CONFIG register. This was necessary for >>> auto-negotiation mode to work. >>> Unfortunately I haven't checked if these bits are ever initialized. >>> It appears they are not. >>> This patch adds the missing initialization of the auto-negotiation bits >>> in the MVNETA_GMAC_AUTONEG_CONFIG register. >>> It fixes the following regression: >>> https://www.mail-archive.com/netdev@vger.kernel.org/msg67928.html >>> >>> Since the patch was tested to fix a regression, it should be applied to >>> stable tree. >>> >>> Tested-by: Arnaud Ebalard <arno@natisbad.org> >>> >>> CC: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> >>> CC: Florian Fainelli <f.fainelli@gmail.com> >>> CC: netdev@vger.kernel.org >>> CC: linux-kernel@vger.kernel.org >>> CC: stable@vger.kernel.org >>> >>> Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net> >>> Signed-off-by: David S. Miller <davem@davemloft.net> >>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> >>> --- >>> drivers/net/ethernet/marvell/mvneta.c | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> --- a/drivers/net/ethernet/marvell/mvneta.c >>> +++ b/drivers/net/ethernet/marvell/mvneta.c >>> @@ -1013,6 +1013,12 @@ static void mvneta_defaults_set(struct m >>> val = mvreg_read(pp, MVNETA_GMAC_CLOCK_DIVIDER); >>> val |= MVNETA_GMAC_1MS_CLOCK_ENABLE; >>> mvreg_write(pp, MVNETA_GMAC_CLOCK_DIVIDER, val); >>> + } else { >>> + val = mvreg_read(pp, MVNETA_GMAC_AUTONEG_CONFIG); >>> + val &= ~(MVNETA_GMAC_INBAND_AN_ENABLE | >>> + MVNETA_GMAC_AN_SPEED_EN | >>> + MVNETA_GMAC_AN_DUPLEX_EN); >>> + mvreg_write(pp, MVNETA_GMAC_AUTONEG_CONFIG, val); >>> } >>> mvneta_set_ucast_table(pp, -1); >> Hi Greg. >> >> Another problem was reported: >> https://lkml.org/lkml/2015/7/8/865 >> >> So, while the above patch is correct and fixes what >> it should, the original patch has more problems to deal >> with. Maybe for stable it would be better to just revert >> the whole thing? > No, you will have to fix this in Linus's tree, right? So I'll take the > patch that you get into there when that happens, I don't want to diverge > from what is in that tree. For Linus tree I am planning a new DT property to explicitly enable the inband status. I don't see any quick fix suitable for -stable, and new DT property will likely not be quickly accepted. If you don't want a revert, then the stable will likely have that regression for quite long, that's the warning. OTOH, the revert will remove the support for my board, so I won't be able to even test it, which is also not perfect. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, Stas Sergeev <stsp@list.ru> writes: >>> Another problem was reported: >>> https://lkml.org/lkml/2015/7/8/865 >>> >>> So, while the above patch is correct and fixes what >>> it should, the original patch has more problems to deal >>> with. Maybe for stable it would be better to just revert >>> the whole thing? >> No, you will have to fix this in Linus's tree, right? So I'll take the >> patch that you get into there when that happens, I don't want to diverge >> from what is in that tree. > For Linus tree I am planning a new DT property to explicitly > enable the inband status. I don't see any quick fix suitable for > -stable, and new DT property will likely not be quickly accepted. > If you don't want a revert, then the stable will likely have that > regression for quite long, that's the warning. I do not think the problem is to have a revert in -stable, it's more having in in Linus tree *first* ;-) > OTOH, the revert will remove the support for my board, so I > won't be able to even test it, which is also not perfect. ATM, the priority is more on fixing the regressions the initial patch caused *for existing boards*. There were at least three boards which got hit by first regression during 4.1-rc and a new one on the table now that 4.1 is out. I understand your reluctance to revert the patch that made mvneta work for your custom board but it's unfair for others that are hit by the regressions it causes and have to spend time bisecting/fixing it. Anyway, if you come w/ a fix, I can commit to test it on the boards I have. Cheers, a+ -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
08.07.2015 22:36, Arnaud Ebalard пишет: > Hi, > > Stas Sergeev <stsp@list.ru> writes: > >>>> Another problem was reported: >>>> https://lkml.org/lkml/2015/7/8/865 >>>> >>>> So, while the above patch is correct and fixes what >>>> it should, the original patch has more problems to deal >>>> with. Maybe for stable it would be better to just revert >>>> the whole thing? >>> No, you will have to fix this in Linus's tree, right? So I'll take the >>> patch that you get into there when that happens, I don't want to diverge >>> from what is in that tree. >> For Linus tree I am planning a new DT property to explicitly >> enable the inband status. I don't see any quick fix suitable for >> -stable, and new DT property will likely not be quickly accepted. >> If you don't want a revert, then the stable will likely have that >> regression for quite long, that's the warning. > I do not think the problem is to have a revert in -stable, it's more > having in in Linus tree *first* ;-) > >> OTOH, the revert will remove the support for my board, so I >> won't be able to even test it, which is also not perfect. > ATM, the priority is more on fixing the regressions the initial patch > caused *for existing boards*. There were at least three boards which got > hit by first regression during 4.1-rc That one is fixed, so doesn't count. > and a new one on the table now > that 4.1 is out. For that we don't know the impact yet. I asked Sebastien Rannou about what HW he has connected via sgmii link and why does he use a fixed-link. If it is just some strange HW that does not generate the inband status where it should, perhaps it is not such a big deal to rush reverting it from Linus tree. > I understand your reluctance to revert the patch that > made mvneta work for your custom board but it's unfair for others that > are hit by the regressions it causes and have to spend time > bisecting/fixing it. I am not reluctant for a revert, I in fact _propose_ the revert for -stable. As for mainline - yes, I'd really rather just do a proper fix there, as there is probably not a big deal to wait just for a little longer till the proper fix is discussed. But since Greg have spoken against the divergence, I am currently in an undecided state. I guess I'll code the fix first, then will see. Hope the news will be tomorrow. > Anyway, if you come w/ a fix, I can commit to test it on the boards I > have. Thanks, I'll keep you CCed. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Jul 08, 2015 at 09:36:46PM +0300, Stas Sergeev wrote: > 08.07.2015 20:36, Greg Kroah-Hartman пишет: > >On Wed, Jul 08, 2015 at 08:10:46PM +0300, Stas Sergeev wrote: > >>08.07.2015 10:35, Greg Kroah-Hartman пишет: > >>>4.1-stable review patch. If anyone has any objections, please let me know. > >>> > >>>------------------ > >>> > >>>From: Stas Sergeev <stsp@list.ru> > >>> > >>>[ Upstream commit 538761b794c1542f1c6e31eadd9d7aae118889f7 ] > >>> > >>>The commit 898b2970e2c9 ("mvneta: implement SGMII-based in-band link state > >>>signaling") > >>>changed mvneta_adjust_link() so that it does not clear the auto-negotiation > >>>bits in MVNETA_GMAC_AUTONEG_CONFIG register. This was necessary for > >>>auto-negotiation mode to work. > >>>Unfortunately I haven't checked if these bits are ever initialized. > >>>It appears they are not. > >>>This patch adds the missing initialization of the auto-negotiation bits > >>>in the MVNETA_GMAC_AUTONEG_CONFIG register. > >>>It fixes the following regression: > >>>https://www.mail-archive.com/netdev@vger.kernel.org/msg67928.html > >>> > >>>Since the patch was tested to fix a regression, it should be applied to > >>>stable tree. > >>> > >>>Tested-by: Arnaud Ebalard <arno@natisbad.org> > >>> > >>>CC: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > >>>CC: Florian Fainelli <f.fainelli@gmail.com> > >>>CC: netdev@vger.kernel.org > >>>CC: linux-kernel@vger.kernel.org > >>>CC: stable@vger.kernel.org > >>> > >>>Signed-off-by: Stas Sergeev <stsp@users.sourceforge.net> > >>>Signed-off-by: David S. Miller <davem@davemloft.net> > >>>Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > >>>--- > >>> drivers/net/ethernet/marvell/mvneta.c | 6 ++++++ > >>> 1 file changed, 6 insertions(+) > >>> > >>>--- a/drivers/net/ethernet/marvell/mvneta.c > >>>+++ b/drivers/net/ethernet/marvell/mvneta.c > >>>@@ -1013,6 +1013,12 @@ static void mvneta_defaults_set(struct m > >>> val = mvreg_read(pp, MVNETA_GMAC_CLOCK_DIVIDER); > >>> val |= MVNETA_GMAC_1MS_CLOCK_ENABLE; > >>> mvreg_write(pp, MVNETA_GMAC_CLOCK_DIVIDER, val); > >>>+ } else { > >>>+ val = mvreg_read(pp, MVNETA_GMAC_AUTONEG_CONFIG); > >>>+ val &= ~(MVNETA_GMAC_INBAND_AN_ENABLE | > >>>+ MVNETA_GMAC_AN_SPEED_EN | > >>>+ MVNETA_GMAC_AN_DUPLEX_EN); > >>>+ mvreg_write(pp, MVNETA_GMAC_AUTONEG_CONFIG, val); > >>> } > >>> mvneta_set_ucast_table(pp, -1); > >>Hi Greg. > >> > >>Another problem was reported: > >>https://lkml.org/lkml/2015/7/8/865 > >> > >>So, while the above patch is correct and fixes what > >>it should, the original patch has more problems to deal > >>with. Maybe for stable it would be better to just revert > >>the whole thing? > >No, you will have to fix this in Linus's tree, right? So I'll take the > >patch that you get into there when that happens, I don't want to diverge > >from what is in that tree. > For Linus tree I am planning a new DT property to explicitly > enable the inband status. I don't see any quick fix suitable for > -stable, and new DT property will likely not be quickly accepted. > If you don't want a revert, then the stable will likely have that > regression for quite long, that's the warning. That's fine, we will be in sync with Linus's tree, so all is fine, and it might spur people to get the thing fixed properly faster :) thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- a/drivers/net/ethernet/marvell/mvneta.c +++ b/drivers/net/ethernet/marvell/mvneta.c @@ -1013,6 +1013,12 @@ static void mvneta_defaults_set(struct m val = mvreg_read(pp, MVNETA_GMAC_CLOCK_DIVIDER); val |= MVNETA_GMAC_1MS_CLOCK_ENABLE; mvreg_write(pp, MVNETA_GMAC_CLOCK_DIVIDER, val); + } else { + val = mvreg_read(pp, MVNETA_GMAC_AUTONEG_CONFIG); + val &= ~(MVNETA_GMAC_INBAND_AN_ENABLE | + MVNETA_GMAC_AN_SPEED_EN | + MVNETA_GMAC_AN_DUPLEX_EN); + mvreg_write(pp, MVNETA_GMAC_AUTONEG_CONFIG, val); } mvneta_set_ucast_table(pp, -1);