diff mbox series

[net-next,13/15] net: phy: realtek: drop .read_page and .write_page for rtl822x series

Message ID 20231220155518.15692-14-kabel@kernel.org (mailing list archive)
State Changes Requested
Delegated to: Netdev Maintainers
Headers show
Series Realtek RTL822x PHY rework to c45 and SerDes interface switching | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1115 this patch: 1115
netdev/cc_maintainers warning 1 maintainers not CCed: edumazet@google.com
netdev/build_clang fail Errors and warnings before: 12 this patch: 12
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 1142 this patch: 1142
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 48 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Marek Behún Dec. 20, 2023, 3:55 p.m. UTC
Drop the .read_page() and .write_page() methods for rtl822x series.

The rtl822x driver methods are now reimplemented to only access clause
45 registers and these are the last methods that explicitly access
clause 22 registers.

If the underlying MDIO bus is clause 22, the paging mechanism is still
used internally in the .read_mmd() and .write_mmd() methods when
accessing registers in MMD 31.

Signed-off-by: Marek Behún <kabel@kernel.org>
---
 drivers/net/phy/realtek.c | 12 ------------
 1 file changed, 12 deletions(-)

Comments

Heiner Kallweit Dec. 20, 2023, 5:23 p.m. UTC | #1
On 20.12.2023 16:55, Marek Behún wrote:
> Drop the .read_page() and .write_page() methods for rtl822x series.
> 
> The rtl822x driver methods are now reimplemented to only access clause
> 45 registers and these are the last methods that explicitly access
> clause 22 registers.
> 
> If the underlying MDIO bus is clause 22, the paging mechanism is still
> used internally in the .read_mmd() and .write_mmd() methods when
> accessing registers in MMD 31.
> 
> Signed-off-by: Marek Behún <kabel@kernel.org>
> ---
>  drivers/net/phy/realtek.c | 12 ------------
>  1 file changed, 12 deletions(-)
> 
> diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
> index cf608d390aa5..e2f68ac4b005 100644
> --- a/drivers/net/phy/realtek.c
> +++ b/drivers/net/phy/realtek.c
> @@ -963,8 +963,6 @@ static struct phy_driver realtek_drvs[] = {
>  		.read_status	= rtl822x_read_status,
>  		.suspend	= genphy_c45_pma_suspend,
>  		.resume		= rtl822x_resume,
> -		.read_page	= rtl821x_read_page,
> -		.write_page	= rtl821x_write_page,
>  		.read_mmd	= rtlgen_read_mmd,
>  		.write_mmd	= rtlgen_write_mmd,
>  	}, {
> @@ -975,8 +973,6 @@ static struct phy_driver realtek_drvs[] = {
>  		.read_status	= rtl822x_read_status,
>  		.suspend	= genphy_c45_pma_suspend,
>  		.resume		= rtl822x_resume,
> -		.read_page	= rtl821x_read_page,
> -		.write_page	= rtl821x_write_page,
>  		.read_mmd	= rtlgen_read_mmd,
>  		.write_mmd	= rtlgen_write_mmd,
>  	}, {
> @@ -987,8 +983,6 @@ static struct phy_driver realtek_drvs[] = {
>  		.read_status    = rtl822x_read_status,
>  		.suspend	= genphy_c45_pma_suspend,
>  		.resume		= rtl822x_resume,
> -		.read_page      = rtl821x_read_page,
> -		.write_page     = rtl821x_write_page,
>  		.read_mmd	= rtlgen_read_mmd,
>  		.write_mmd	= rtlgen_write_mmd,
>  	}, {
> @@ -999,8 +993,6 @@ static struct phy_driver realtek_drvs[] = {
>  		.read_status    = rtl822x_read_status,
>  		.suspend	= genphy_c45_pma_suspend,
>  		.resume		= rtl822x_resume,
> -		.read_page      = rtl821x_read_page,
> -		.write_page     = rtl821x_write_page,
>  		.read_mmd	= rtlgen_read_mmd,
>  		.write_mmd	= rtlgen_write_mmd,
>  	}, {
> @@ -1011,8 +1003,6 @@ static struct phy_driver realtek_drvs[] = {
>  		.read_status    = rtl822x_read_status,
>  		.suspend	= genphy_c45_pma_suspend,
>  		.resume		= rtl822x_resume,
> -		.read_page      = rtl821x_read_page,
> -		.write_page     = rtl821x_write_page,
>  		.read_mmd	= rtlgen_read_mmd,
>  		.write_mmd	= rtlgen_write_mmd,
>  	}, {
> @@ -1023,8 +1013,6 @@ static struct phy_driver realtek_drvs[] = {
>  		.read_status    = rtl822x_read_status,
>  		.suspend	= genphy_c45_pma_suspend,
>  		.resume		= rtl822x_resume,
> -		.read_page      = rtl821x_read_page,
> -		.write_page     = rtl821x_write_page,
>  		.read_mmd	= rtlgen_read_mmd,
>  		.write_mmd	= rtlgen_write_mmd,
>  	}, {

Dropping the read_page/write_page hooks will be problematic,
because they are used by the PHY initialization in e.g.
rtl8125a_2_hw_phy_config().
Marek Behún Dec. 21, 2023, 10:21 a.m. UTC | #2
On Wed, 20 Dec 2023 18:23:21 +0100
Heiner Kallweit <hkallweit1@gmail.com> wrote:

> On 20.12.2023 16:55, Marek Behún wrote:
> > Drop the .read_page() and .write_page() methods for rtl822x series.
> > 
> > The rtl822x driver methods are now reimplemented to only access clause
> > 45 registers and these are the last methods that explicitly access
> > clause 22 registers.
> > 
> > If the underlying MDIO bus is clause 22, the paging mechanism is still
> > used internally in the .read_mmd() and .write_mmd() methods when
> > accessing registers in MMD 31.
> > 
> > Signed-off-by: Marek Behún <kabel@kernel.org>
> > ---
> >  drivers/net/phy/realtek.c | 12 ------------
> >  1 file changed, 12 deletions(-)
> > 
> > diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
> > index cf608d390aa5..e2f68ac4b005 100644
> > --- a/drivers/net/phy/realtek.c
> > +++ b/drivers/net/phy/realtek.c
> > @@ -963,8 +963,6 @@ static struct phy_driver realtek_drvs[] = {
> >  		.read_status	= rtl822x_read_status,
> >  		.suspend	= genphy_c45_pma_suspend,
> >  		.resume		= rtl822x_resume,
> > -		.read_page	= rtl821x_read_page,
> > -		.write_page	= rtl821x_write_page,
> >  		.read_mmd	= rtlgen_read_mmd,
> >  		.write_mmd	= rtlgen_write_mmd,
> >  	}, {
> > @@ -975,8 +973,6 @@ static struct phy_driver realtek_drvs[] = {
> >  		.read_status	= rtl822x_read_status,
> >  		.suspend	= genphy_c45_pma_suspend,
> >  		.resume		= rtl822x_resume,
> > -		.read_page	= rtl821x_read_page,
> > -		.write_page	= rtl821x_write_page,
> >  		.read_mmd	= rtlgen_read_mmd,
> >  		.write_mmd	= rtlgen_write_mmd,
> >  	}, {
> > @@ -987,8 +983,6 @@ static struct phy_driver realtek_drvs[] = {
> >  		.read_status    = rtl822x_read_status,
> >  		.suspend	= genphy_c45_pma_suspend,
> >  		.resume		= rtl822x_resume,
> > -		.read_page      = rtl821x_read_page,
> > -		.write_page     = rtl821x_write_page,
> >  		.read_mmd	= rtlgen_read_mmd,
> >  		.write_mmd	= rtlgen_write_mmd,
> >  	}, {
> > @@ -999,8 +993,6 @@ static struct phy_driver realtek_drvs[] = {
> >  		.read_status    = rtl822x_read_status,
> >  		.suspend	= genphy_c45_pma_suspend,
> >  		.resume		= rtl822x_resume,
> > -		.read_page      = rtl821x_read_page,
> > -		.write_page     = rtl821x_write_page,
> >  		.read_mmd	= rtlgen_read_mmd,
> >  		.write_mmd	= rtlgen_write_mmd,
> >  	}, {
> > @@ -1011,8 +1003,6 @@ static struct phy_driver realtek_drvs[] = {
> >  		.read_status    = rtl822x_read_status,
> >  		.suspend	= genphy_c45_pma_suspend,
> >  		.resume		= rtl822x_resume,
> > -		.read_page      = rtl821x_read_page,
> > -		.write_page     = rtl821x_write_page,
> >  		.read_mmd	= rtlgen_read_mmd,
> >  		.write_mmd	= rtlgen_write_mmd,
> >  	}, {
> > @@ -1023,8 +1013,6 @@ static struct phy_driver realtek_drvs[] = {
> >  		.read_status    = rtl822x_read_status,
> >  		.suspend	= genphy_c45_pma_suspend,
> >  		.resume		= rtl822x_resume,
> > -		.read_page      = rtl821x_read_page,
> > -		.write_page     = rtl821x_write_page,
> >  		.read_mmd	= rtlgen_read_mmd,
> >  		.write_mmd	= rtlgen_write_mmd,
> >  	}, {  
> 
> Dropping the read_page/write_page hooks will be problematic,
> because they are used by the PHY initialization in e.g.
> rtl8125a_2_hw_phy_config().

I see. 

Maybe it would be simpler to just remove it from this series.

Looking at all instances of paged access in r8169, most of them seem to
access the vendor 2 MMD registers. Also the person from Realtek says
that MMD registers are available also on 1gbps PHYs.

Looking at PHY specs for RTL8211 series, all of them (as old as 2009)
seem to document MMD access.

So I think we can safely add .read_mmd() and .write_mmd() methods to
all the PHYs in realtek.c that may be used by r8169, and then we can
change the relevant phy_read/write/modify_paged calls into
phy_read/write/modify_mmd in r8169 according to the formula.

(The relevant accesses being those where page is set to value >= 0xa00.)

What do you think?

Marek
diff mbox series

Patch

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index cf608d390aa5..e2f68ac4b005 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -963,8 +963,6 @@  static struct phy_driver realtek_drvs[] = {
 		.read_status	= rtl822x_read_status,
 		.suspend	= genphy_c45_pma_suspend,
 		.resume		= rtl822x_resume,
-		.read_page	= rtl821x_read_page,
-		.write_page	= rtl821x_write_page,
 		.read_mmd	= rtlgen_read_mmd,
 		.write_mmd	= rtlgen_write_mmd,
 	}, {
@@ -975,8 +973,6 @@  static struct phy_driver realtek_drvs[] = {
 		.read_status	= rtl822x_read_status,
 		.suspend	= genphy_c45_pma_suspend,
 		.resume		= rtl822x_resume,
-		.read_page	= rtl821x_read_page,
-		.write_page	= rtl821x_write_page,
 		.read_mmd	= rtlgen_read_mmd,
 		.write_mmd	= rtlgen_write_mmd,
 	}, {
@@ -987,8 +983,6 @@  static struct phy_driver realtek_drvs[] = {
 		.read_status    = rtl822x_read_status,
 		.suspend	= genphy_c45_pma_suspend,
 		.resume		= rtl822x_resume,
-		.read_page      = rtl821x_read_page,
-		.write_page     = rtl821x_write_page,
 		.read_mmd	= rtlgen_read_mmd,
 		.write_mmd	= rtlgen_write_mmd,
 	}, {
@@ -999,8 +993,6 @@  static struct phy_driver realtek_drvs[] = {
 		.read_status    = rtl822x_read_status,
 		.suspend	= genphy_c45_pma_suspend,
 		.resume		= rtl822x_resume,
-		.read_page      = rtl821x_read_page,
-		.write_page     = rtl821x_write_page,
 		.read_mmd	= rtlgen_read_mmd,
 		.write_mmd	= rtlgen_write_mmd,
 	}, {
@@ -1011,8 +1003,6 @@  static struct phy_driver realtek_drvs[] = {
 		.read_status    = rtl822x_read_status,
 		.suspend	= genphy_c45_pma_suspend,
 		.resume		= rtl822x_resume,
-		.read_page      = rtl821x_read_page,
-		.write_page     = rtl821x_write_page,
 		.read_mmd	= rtlgen_read_mmd,
 		.write_mmd	= rtlgen_write_mmd,
 	}, {
@@ -1023,8 +1013,6 @@  static struct phy_driver realtek_drvs[] = {
 		.read_status    = rtl822x_read_status,
 		.suspend	= genphy_c45_pma_suspend,
 		.resume		= rtl822x_resume,
-		.read_page      = rtl821x_read_page,
-		.write_page     = rtl821x_write_page,
 		.read_mmd	= rtlgen_read_mmd,
 		.write_mmd	= rtlgen_write_mmd,
 	}, {