[PATCH 4.1 11/56] mvneta: add forgotten initialization of autonegotiation bits

[PATCH 4.1 11/56] mvneta: add forgotten initialization of autonegotiation bits

[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);

Re: [PATCH 4.1 11/56] mvneta: add forgotten initialization of autonegotiation bits

[Stas Sergeev]

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.

Re: [PATCH 4.1 11/56] mvneta: add forgotten initialization of autonegotiation bits

[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.

> 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

Re: [PATCH 4.1 11/56] mvneta: add forgotten initialization of autonegotiation bits

[Stas Sergeev]

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.

Re: [PATCH 4.1 11/56] mvneta: add forgotten initialization of autonegotiation bits

[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 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+

Re: [PATCH 4.1 11/56] mvneta: add forgotten initialization of autonegotiation bits

[Stas Sergeev]

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.

Re: [PATCH 4.1 11/56] mvneta: add forgotten initialization of autonegotiation bits

[Greg Kroah-Hartman]

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