The rtl8211f supports downshift and before commit 5502b218e001
("net: phy: use phy_resolve_aneg_linkmode in genphy_read_status")
the read-back of register MII_CTRL1000 was used to detect the
negotiated link speed.
The code added in commit d445dff2df60 ("net: phy: realtek: read
actual speed to detect downshift") is working fine also for this
phy and it's trivial re-using it to restore the downshift
detection on rtl8211f.
Add the phy specific read_status() pointing to the existing
function rtlgen_read_status().
Signed-off-by: Antonio Borneo <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
To: Andrew Lunn <[email protected]>
To: Heiner Kallweit <[email protected]>
To: Russell King <[email protected]>
To: "David S. Miller" <[email protected]>
To: Jakub Kicinski <[email protected]>
To: [email protected]
To: Yonglong Liu <[email protected]>
To: Willy Liu <[email protected]>
Cc: [email protected]
Cc: Salil Mehta <[email protected]>
Cc: [email protected]
Cc: [email protected]
In-Reply-To: <[email protected]>
V1 => V2
move from a generic implementation affecting every phy
to a rtl8211f specific implementation
---
drivers/net/phy/realtek.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 575580d3ffe0..8ff8a4edc173 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -621,6 +621,7 @@ static struct phy_driver realtek_drvs[] = {
PHY_ID_MATCH_EXACT(0x001cc916),
.name = "RTL8211F Gigabit Ethernet",
.config_init = &rtl8211f_config_init,
+ .read_status = rtlgen_read_status,
.ack_interrupt = &rtl8211f_ack_interrupt,
.config_intr = &rtl8211f_config_intr,
.suspend = genphy_suspend,
base-commit: 9bd2702d292cb7b565b09e949d30288ab7a26d51
--
2.29.2
Am 24.11.2020 um 22:59 schrieb Antonio Borneo:
> The rtl8211f supports downshift and before commit 5502b218e001
> ("net: phy: use phy_resolve_aneg_linkmode in genphy_read_status")
> the read-back of register MII_CTRL1000 was used to detect the
> negotiated link speed.
> The code added in commit d445dff2df60 ("net: phy: realtek: read
> actual speed to detect downshift") is working fine also for this
> phy and it's trivial re-using it to restore the downshift
> detection on rtl8211f.
>
> Add the phy specific read_status() pointing to the existing
> function rtlgen_read_status().
>
> Signed-off-by: Antonio Borneo <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]
> ---
> To: Andrew Lunn <[email protected]>
> To: Heiner Kallweit <[email protected]>
> To: Russell King <[email protected]>
> To: "David S. Miller" <[email protected]>
> To: Jakub Kicinski <[email protected]>
> To: [email protected]
> To: Yonglong Liu <[email protected]>
> To: Willy Liu <[email protected]>
> Cc: [email protected]
> Cc: Salil Mehta <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> In-Reply-To: <[email protected]>
>
> V1 => V2
> move from a generic implementation affecting every phy
> to a rtl8211f specific implementation
> ---
> drivers/net/phy/realtek.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
> index 575580d3ffe0..8ff8a4edc173 100644
> --- a/drivers/net/phy/realtek.c
> +++ b/drivers/net/phy/realtek.c
> @@ -621,6 +621,7 @@ static struct phy_driver realtek_drvs[] = {
> PHY_ID_MATCH_EXACT(0x001cc916),
> .name = "RTL8211F Gigabit Ethernet",
> .config_init = &rtl8211f_config_init,
> + .read_status = rtlgen_read_status,
> .ack_interrupt = &rtl8211f_ack_interrupt,
> .config_intr = &rtl8211f_config_intr,
> .suspend = genphy_suspend,
>
> base-commit: 9bd2702d292cb7b565b09e949d30288ab7a26d51
>
Pefect would be to make this a fix for 5502b218e001,
but rtlgen_read_status() was added one year after this change.
Marking the change that added rtlgen_read_status() as "Fixes"
would be technically ok, but as it's not actually broken not
everybody may be happy with this.
Having said that I'd be fine with treating this as an improvement,
downshift should be a rare case.
Am 24.11.2020 um 23:33 schrieb Antonio Borneo:
> On Tue, 2020-11-24 at 23:22 +0100, Heiner Kallweit wrote:
>> Am 24.11.2020 um 22:59 schrieb Antonio Borneo:
>>> The rtl8211f supports downshift and before commit 5502b218e001
>>> ("net: phy: use phy_resolve_aneg_linkmode in genphy_read_status")
>>> the read-back of register MII_CTRL1000 was used to detect the
>>> negotiated link speed.
>>> The code added in commit d445dff2df60 ("net: phy: realtek: read
>>> actual speed to detect downshift") is working fine also for this
>>> phy and it's trivial re-using it to restore the downshift
>>> detection on rtl8211f.
>>>
>>> Add the phy specific read_status() pointing to the existing
>>> function rtlgen_read_status().
>>>
>>> Signed-off-by: Antonio Borneo <[email protected]>
>>> Link: https://lore.kernel.org/r/[email protected]
>>> ---
>>> To: Andrew Lunn <[email protected]>
>>> To: Heiner Kallweit <[email protected]>
>>> To: Russell King <[email protected]>
>>> To: "David S. Miller" <[email protected]>
>>> To: Jakub Kicinski <[email protected]>
>>> To: [email protected]
>>> To: Yonglong Liu <[email protected]>
>>> To: Willy Liu <[email protected]>
>>> Cc: [email protected]
>>> Cc: Salil Mehta <[email protected]>
>>> Cc: [email protected]
>>> Cc: [email protected]
>>> In-Reply-To: <[email protected]>
>>>
>>> V1 => V2
>>> move from a generic implementation affecting every phy
>>> to a rtl8211f specific implementation
>>> ---
>>> drivers/net/phy/realtek.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
>>> index 575580d3ffe0..8ff8a4edc173 100644
>>> --- a/drivers/net/phy/realtek.c
>>> +++ b/drivers/net/phy/realtek.c
>>> @@ -621,6 +621,7 @@ static struct phy_driver realtek_drvs[] = {
>>> PHY_ID_MATCH_EXACT(0x001cc916),
>>> .name = "RTL8211F Gigabit Ethernet",
>>> .config_init = &rtl8211f_config_init,
>>> + .read_status = rtlgen_read_status,
>>> .ack_interrupt = &rtl8211f_ack_interrupt,
>>> .config_intr = &rtl8211f_config_intr,
>>> .suspend = genphy_suspend,
>>>
>>> base-commit: 9bd2702d292cb7b565b09e949d30288ab7a26d51
>>>
>>
>> Pefect would be to make this a fix for 5502b218e001,
>> but rtlgen_read_status() was added one year after this change.
>> Marking the change that added rtlgen_read_status() as "Fixes"
>> would be technically ok, but as it's not actually broken not
>> everybody may be happy with this.
>> Having said that I'd be fine with treating this as an improvement,
>> downshift should be a rare case.
>
> Correct! Being the commit that adds rtlgen_read_status() an improvement,
> should not be backported, so this patch is not marked anymore as a fix!
> Plus, this does not fix 5502b218e001 in the general case, but limited to
> one specific phy, making the 'fixes' label less relevant.
> Anyway, the commit message reports all the ingredients for a backport.
>
> By the way, I have incorrectly sent this based on netdev, but it's not a
> fix anymore! Should I rebase it on netdev-next and resend?
>
For this small change it shouldn't make a difference whether it's based
on net or net-next. I don't think anything has changed here. But better
check whether patch applies cleanly on net-next. Patch should have been
annotated as [PATCH net-next], but I think a re-send isn't needed as
Jakub can see it based on this communication.
> Antonio
>
On Tue, 2020-11-24 at 23:22 +0100, Heiner Kallweit wrote:
> Am 24.11.2020 um 22:59 schrieb Antonio Borneo:
> > The rtl8211f supports downshift and before commit 5502b218e001
> > ("net: phy: use phy_resolve_aneg_linkmode in genphy_read_status")
> > the read-back of register MII_CTRL1000 was used to detect the
> > negotiated link speed.
> > The code added in commit d445dff2df60 ("net: phy: realtek: read
> > actual speed to detect downshift") is working fine also for this
> > phy and it's trivial re-using it to restore the downshift
> > detection on rtl8211f.
> >
> > Add the phy specific read_status() pointing to the existing
> > function rtlgen_read_status().
> >
> > Signed-off-by: Antonio Borneo <[email protected]>
> > Link: https://lore.kernel.org/r/[email protected]
> > ---
> > To: Andrew Lunn <[email protected]>
> > To: Heiner Kallweit <[email protected]>
> > To: Russell King <[email protected]>
> > To: "David S. Miller" <[email protected]>
> > To: Jakub Kicinski <[email protected]>
> > To: [email protected]
> > To: Yonglong Liu <[email protected]>
> > To: Willy Liu <[email protected]>
> > Cc: [email protected]
> > Cc: Salil Mehta <[email protected]>
> > Cc: [email protected]
> > Cc: [email protected]
> > In-Reply-To: <[email protected]>
> >
> > V1 => V2
> > move from a generic implementation affecting every phy
> > to a rtl8211f specific implementation
> > ---
> > drivers/net/phy/realtek.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
> > index 575580d3ffe0..8ff8a4edc173 100644
> > --- a/drivers/net/phy/realtek.c
> > +++ b/drivers/net/phy/realtek.c
> > @@ -621,6 +621,7 @@ static struct phy_driver realtek_drvs[] = {
> > PHY_ID_MATCH_EXACT(0x001cc916),
> > .name = "RTL8211F Gigabit Ethernet",
> > .config_init = &rtl8211f_config_init,
> > + .read_status = rtlgen_read_status,
> > .ack_interrupt = &rtl8211f_ack_interrupt,
> > .config_intr = &rtl8211f_config_intr,
> > .suspend = genphy_suspend,
> >
> > base-commit: 9bd2702d292cb7b565b09e949d30288ab7a26d51
> >
>
> Pefect would be to make this a fix for 5502b218e001,
> but rtlgen_read_status() was added one year after this change.
> Marking the change that added rtlgen_read_status() as "Fixes"
> would be technically ok, but as it's not actually broken not
> everybody may be happy with this.
> Having said that I'd be fine with treating this as an improvement,
> downshift should be a rare case.
Correct! Being the commit that adds rtlgen_read_status() an improvement,
should not be backported, so this patch is not marked anymore as a fix!
Plus, this does not fix 5502b218e001 in the general case, but limited to
one specific phy, making the 'fixes' label less relevant.
Anyway, the commit message reports all the ingredients for a backport.
By the way, I have incorrectly sent this based on netdev, but it's not a
fix anymore! Should I rebase it on netdev-next and resend?
Antonio