[2/3] net: phy: DP83822: enable rgmii/rmmi mode according to phy interface mode

RMII/RGMII mode can be enable from dp83822 straps, and also writing bit 9 and
bit 5 of register 0x17 - RMII and Status Register (RCSR).
When phy_interface_is_rgmii rgmii mode must be enabled, same for
contrary, this prevents malconfigurations of hw straps. If bit 9 is 0
the RMII/MII mode is determinated by bit 5.

References:
 - https://www.ti.com/lit/gpn/dp83822i p66

Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
Co-developed-by: Michael Trimarchi <michael@amarulasolutions.com>
Suggested-by: Alberto Bianchi <alberto.bianchi@amarulasolutions.com>
Tested-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
---
 drivers/net/phy/dp83822.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)
  

On Sun, May 22, 2022 at 03:38:12PM +0200, Michael Trimarchi wrote:
> RMII/RGMII mode can be enable from dp83822 straps, and also writing bit 9 and
> bit 5 of register 0x17 - RMII and Status Register (RCSR).
> When phy_interface_is_rgmii rgmii mode must be enabled, same for
> contrary, this prevents malconfigurations of hw straps. If bit 9 is 0
> the RMII/MII mode is determinated by bit 5.
> 
> References:
>  - https://www.ti.com/lit/gpn/dp83822i p66
> 
> Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
> Co-developed-by: Michael Trimarchi <michael@amarulasolutions.com>
> Suggested-by: Alberto Bianchi <alberto.bianchi@amarulasolutions.com>
> Tested-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
> ---
>  drivers/net/phy/dp83822.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
> index c344b8ffaf3c..89887a05f23d 100644
> --- a/drivers/net/phy/dp83822.c
> +++ b/drivers/net/phy/dp83822.c
> @@ -94,7 +94,9 @@
>  #define DP83822_WOL_INDICATION_SEL BIT(8)
>  #define DP83822_WOL_CLR_INDICATION BIT(11)
>  
> -/* RSCR bits */
> +/* RCSR bits */
> +#define DP83822_RMII_MODE_EN	BIT(5)
> +#define DP83822_RGMII_MODE_EN	BIT(9)
>  #define DP83822_RX_CLK_SHIFT	BIT(12)
>  #define DP83822_TX_CLK_SHIFT	BIT(11)
>  
> @@ -403,6 +405,17 @@ static int dp83822_config_init(struct phy_device *phydev)
>  			if (err)
>  				return err;
>  		}
> +
> +		phy_set_bits_mmd(phydev, DP83822_DEVADDR,
> +					MII_DP83822_RCSR, DP83822_RGMII_MODE_EN);

Hi Michael,
Can you clarify this peace of code pls:

----------------------------------------------------------------------------------------
> +	} else {
> +		value = DP83822_RGMII_MODE_EN;
> +

if phy mode is rmii, value is != rmii?

> +	        if (phydev->interface == PHY_INTERFACE_MODE_RMII)
> +			value != DP83822_RMII_MODE_EN;

This clear rgmii_en and rmii_en and after set value

> +
> +		phy_modify_mmd(phydev, DP83822_DEVADDR, MII_DP83822_RCSR,
> +				DP83822_RGMII_MODE_EN | DP83822_RMII_MODE_EN, value);
>  	}
-----------------------------------------------------------------------------------------

I have some doubt about this. I'm confuse sorry.

Thanks,
Tommaso

>  
>  	if (dp83822->fx_enabled) {
> -- 
> 2.25.1
>
  

Hi

On Tue, May 24, 2022 at 9:56 AM Tommaso Merciai
<tommaso.merciai@amarulasolutions.com> wrote:
>
> On Sun, May 22, 2022 at 03:38:12PM +0200, Michael Trimarchi wrote:
> > RMII/RGMII mode can be enable from dp83822 straps, and also writing bit 9 and
> > bit 5 of register 0x17 - RMII and Status Register (RCSR).
> > When phy_interface_is_rgmii rgmii mode must be enabled, same for
> > contrary, this prevents malconfigurations of hw straps. If bit 9 is 0
> > the RMII/MII mode is determinated by bit 5.
> >
> > References:
> >  - https://www.ti.com/lit/gpn/dp83822i p66
> >
> > Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
> > Co-developed-by: Michael Trimarchi <michael@amarulasolutions.com>
> > Suggested-by: Alberto Bianchi <alberto.bianchi@amarulasolutions.com>
> > Tested-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
> > ---
> >  drivers/net/phy/dp83822.c | 15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
> > index c344b8ffaf3c..89887a05f23d 100644
> > --- a/drivers/net/phy/dp83822.c
> > +++ b/drivers/net/phy/dp83822.c
> > @@ -94,7 +94,9 @@
> >  #define DP83822_WOL_INDICATION_SEL BIT(8)
> >  #define DP83822_WOL_CLR_INDICATION BIT(11)
> >
> > -/* RSCR bits */
> > +/* RCSR bits */
> > +#define DP83822_RMII_MODE_EN BIT(5)
> > +#define DP83822_RGMII_MODE_EN        BIT(9)
> >  #define DP83822_RX_CLK_SHIFT BIT(12)
> >  #define DP83822_TX_CLK_SHIFT BIT(11)
> >
> > @@ -403,6 +405,17 @@ static int dp83822_config_init(struct phy_device *phydev)
> >                       if (err)
> >                               return err;
> >               }
> > +
> > +             phy_set_bits_mmd(phydev, DP83822_DEVADDR,
> > +                                     MII_DP83822_RCSR, DP83822_RGMII_MODE_EN);
>
> Hi Michael,
> Can you clarify this peace of code pls:
>
> ----------------------------------------------------------------------------------------
> > +     } else {
> > +             value = DP83822_RGMII_MODE_EN;
> > +
>
This drop in v2

> if phy mode is rmii, value is != rmii?
>
> > +             if (phydev->interface == PHY_INTERFACE_MODE_RMII)
> > +                     value != DP83822_RMII_MODE_EN;
>
> This clear rgmii_en and rmii_en and after set value

As I said this shoule be
if (phydev->interface == PHY_INTERFACE_MODE_RMII)
      value = DP83822_RMII_MODE_EN;
>
> > +
> > +             phy_modify_mmd(phydev, DP83822_DEVADDR, MII_DP83822_RCSR,
> > +                             DP83822_RGMII_MODE_EN | DP83822_RMII_MODE_EN, value);
> >       }

Michael

> -----------------------------------------------------------------------------------------
>
> I have some doubt about this. I'm confuse sorry.
>
> Thanks,
> Tommaso
>
> >
> >       if (dp83822->fx_enabled) {
> > --
> > 2.25.1
> >
>
> --
> Tommaso Merciai
> Embedded Linux Engineer
> tommaso.merciai@amarulasolutions.com
> __________________________________
>
> Amarula Solutions SRL
> Via Le Canevare 30, 31100 Treviso, Veneto, IT
> T. +39 042 243 5310
> info@amarulasolutions.com
> www.amarulasolutions.com
  

On Tue, May 24, 2022 at 10:19:27AM +0200, Michael Nazzareno Trimarchi wrote:
> Hi
> 
> On Tue, May 24, 2022 at 9:56 AM Tommaso Merciai
> <tommaso.merciai@amarulasolutions.com> wrote:
> >
> > On Sun, May 22, 2022 at 03:38:12PM +0200, Michael Trimarchi wrote:
> > > RMII/RGMII mode can be enable from dp83822 straps, and also writing bit 9 and
> > > bit 5 of register 0x17 - RMII and Status Register (RCSR).
> > > When phy_interface_is_rgmii rgmii mode must be enabled, same for
> > > contrary, this prevents malconfigurations of hw straps. If bit 9 is 0
> > > the RMII/MII mode is determinated by bit 5.
> > >
> > > References:
> > >  - https://www.ti.com/lit/gpn/dp83822i p66
> > >
> > > Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
> > > Co-developed-by: Michael Trimarchi <michael@amarulasolutions.com>
> > > Suggested-by: Alberto Bianchi <alberto.bianchi@amarulasolutions.com>
> > > Tested-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
> > > ---
> > >  drivers/net/phy/dp83822.c | 15 ++++++++++++++-
> > >  1 file changed, 14 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
> > > index c344b8ffaf3c..89887a05f23d 100644
> > > --- a/drivers/net/phy/dp83822.c
> > > +++ b/drivers/net/phy/dp83822.c
> > > @@ -94,7 +94,9 @@
> > >  #define DP83822_WOL_INDICATION_SEL BIT(8)
> > >  #define DP83822_WOL_CLR_INDICATION BIT(11)
> > >
> > > -/* RSCR bits */
> > > +/* RCSR bits */
> > > +#define DP83822_RMII_MODE_EN BIT(5)
> > > +#define DP83822_RGMII_MODE_EN        BIT(9)
> > >  #define DP83822_RX_CLK_SHIFT BIT(12)
> > >  #define DP83822_TX_CLK_SHIFT BIT(11)
> > >
> > > @@ -403,6 +405,17 @@ static int dp83822_config_init(struct phy_device *phydev)
> > >                       if (err)
> > >                               return err;
> > >               }
> > > +
> > > +             phy_set_bits_mmd(phydev, DP83822_DEVADDR,
> > > +                                     MII_DP83822_RCSR, DP83822_RGMII_MODE_EN);
> >
> > Hi Michael,
> > Can you clarify this peace of code pls:
> >
> > ----------------------------------------------------------------------------------------
> > > +     } else {
> > > +             value = DP83822_RGMII_MODE_EN;
> > > +
> >
> This drop in v2
> 
> > if phy mode is rmii, value is != rmii?
> >
> > > +             if (phydev->interface == PHY_INTERFACE_MODE_RMII)
> > > +                     value != DP83822_RMII_MODE_EN;
> >
> > This clear rgmii_en and rmii_en and after set value
> 
> As I said this shoule be
> if (phydev->interface == PHY_INTERFACE_MODE_RMII)
>       value = DP83822_RMII_MODE_EN;

Sorry Michael,
I forgot. v2 Need some rework or can I review it?

Thanks,
Tommaso

> >
> > > +
> > > +             phy_modify_mmd(phydev, DP83822_DEVADDR, MII_DP83822_RCSR,
> > > +                             DP83822_RGMII_MODE_EN | DP83822_RMII_MODE_EN, value);
> > >       }
> 
> Michael
> 
> > -----------------------------------------------------------------------------------------
> >
> > I have some doubt about this. I'm confuse sorry.
> >
> > Thanks,
> > Tommaso
> >
> > >
> > >       if (dp83822->fx_enabled) {
> > > --
> > > 2.25.1
> > >
> >
> > --
> > Tommaso Merciai
> > Embedded Linux Engineer
> > tommaso.merciai@amarulasolutions.com
> > __________________________________
> >
> > Amarula Solutions SRL
> > Via Le Canevare 30, 31100 Treviso, Veneto, IT
> > T. +39 042 243 5310
> > info@amarulasolutions.com
> > www.amarulasolutions.com
> 
> 
> 
> -- 
> Michael Nazzareno Trimarchi
> Co-Founder & Chief Executive Officer
> M. +39 347 913 2170
> michael@amarulasolutions.com
> __________________________________
> 
> Amarula Solutions BV
> Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> T. +31 (0)85 111 9172
> info@amarulasolutions.com
> www.amarulasolutions.com
  

Hi

Yes thrre is this error of |= to =


Michael

On Tue, May 24, 2022 at 10:23 AM Tommaso Merciai
<tommaso.merciai@amarulasolutions.com> wrote:
>
> On Tue, May 24, 2022 at 10:19:27AM +0200, Michael Nazzareno Trimarchi wrote:
> > Hi
> >
> > On Tue, May 24, 2022 at 9:56 AM Tommaso Merciai
> > <tommaso.merciai@amarulasolutions.com> wrote:
> > >
> > > On Sun, May 22, 2022 at 03:38:12PM +0200, Michael Trimarchi wrote:
> > > > RMII/RGMII mode can be enable from dp83822 straps, and also writing bit 9 and
> > > > bit 5 of register 0x17 - RMII and Status Register (RCSR).
> > > > When phy_interface_is_rgmii rgmii mode must be enabled, same for
> > > > contrary, this prevents malconfigurations of hw straps. If bit 9 is 0
> > > > the RMII/MII mode is determinated by bit 5.
> > > >
> > > > References:
> > > >  - https://www.ti.com/lit/gpn/dp83822i p66
> > > >
> > > > Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
> > > > Co-developed-by: Michael Trimarchi <michael@amarulasolutions.com>
> > > > Suggested-by: Alberto Bianchi <alberto.bianchi@amarulasolutions.com>
> > > > Tested-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
> > > > ---
> > > >  drivers/net/phy/dp83822.c | 15 ++++++++++++++-
> > > >  1 file changed, 14 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
> > > > index c344b8ffaf3c..89887a05f23d 100644
> > > > --- a/drivers/net/phy/dp83822.c
> > > > +++ b/drivers/net/phy/dp83822.c
> > > > @@ -94,7 +94,9 @@
> > > >  #define DP83822_WOL_INDICATION_SEL BIT(8)
> > > >  #define DP83822_WOL_CLR_INDICATION BIT(11)
> > > >
> > > > -/* RSCR bits */
> > > > +/* RCSR bits */
> > > > +#define DP83822_RMII_MODE_EN BIT(5)
> > > > +#define DP83822_RGMII_MODE_EN        BIT(9)
> > > >  #define DP83822_RX_CLK_SHIFT BIT(12)
> > > >  #define DP83822_TX_CLK_SHIFT BIT(11)
> > > >
> > > > @@ -403,6 +405,17 @@ static int dp83822_config_init(struct phy_device *phydev)
> > > >                       if (err)
> > > >                               return err;
> > > >               }
> > > > +
> > > > +             phy_set_bits_mmd(phydev, DP83822_DEVADDR,
> > > > +                                     MII_DP83822_RCSR, DP83822_RGMII_MODE_EN);
> > >
> > > Hi Michael,
> > > Can you clarify this peace of code pls:
> > >
> > > ----------------------------------------------------------------------------------------
> > > > +     } else {
> > > > +             value = DP83822_RGMII_MODE_EN;
> > > > +
> > >
> > This drop in v2
> >
> > > if phy mode is rmii, value is != rmii?
> > >
> > > > +             if (phydev->interface == PHY_INTERFACE_MODE_RMII)
> > > > +                     value != DP83822_RMII_MODE_EN;
> > >
> > > This clear rgmii_en and rmii_en and after set value
> >
> > As I said this shoule be
> > if (phydev->interface == PHY_INTERFACE_MODE_RMII)
> >       value = DP83822_RMII_MODE_EN;
>
> Sorry Michael,
> I forgot. v2 Need some rework or can I review it?
>
> Thanks,
> Tommaso
>
> > >
> > > > +
> > > > +             phy_modify_mmd(phydev, DP83822_DEVADDR, MII_DP83822_RCSR,
> > > > +                             DP83822_RGMII_MODE_EN | DP83822_RMII_MODE_EN, value);
> > > >       }
> >
> > Michael
> >
> > > -----------------------------------------------------------------------------------------
> > >
> > > I have some doubt about this. I'm confuse sorry.
> > >
> > > Thanks,
> > > Tommaso
> > >
> > > >
> > > >       if (dp83822->fx_enabled) {
> > > > --
> > > > 2.25.1
> > > >
> > >
> > > --
> > > Tommaso Merciai
> > > Embedded Linux Engineer
> > > tommaso.merciai@amarulasolutions.com
> > > __________________________________
> > >
> > > Amarula Solutions SRL
> > > Via Le Canevare 30, 31100 Treviso, Veneto, IT
> > > T. +39 042 243 5310
> > > info@amarulasolutions.com
> > > www.amarulasolutions.com
> >
> >
> >
> > --
> > Michael Nazzareno Trimarchi
> > Co-Founder & Chief Executive Officer
> > M. +39 347 913 2170
> > michael@amarulasolutions.com
> > __________________________________
> >
> > Amarula Solutions BV
> > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> > T. +31 (0)85 111 9172
> > info@amarulasolutions.com
> > www.amarulasolutions.com
>
> --
> Tommaso Merciai
> Embedded Linux Engineer
> tommaso.merciai@amarulasolutions.com
> __________________________________
>
> Amarula Solutions SRL
> Via Le Canevare 30, 31100 Treviso, Veneto, IT
> T. +39 042 243 5310
> info@amarulasolutions.com
> www.amarulasolutions.com
  

On Tue, May 24, 2022 at 10:24:45AM +0200, Michael Nazzareno Trimarchi wrote:
> Hi
> 
> Yes thrre is this error of |= to =

Thanks for your time Michael,
I'll wait v3.

Tommaso

> 
> 
> Michael
> 
> On Tue, May 24, 2022 at 10:23 AM Tommaso Merciai
> <tommaso.merciai@amarulasolutions.com> wrote:
> >
> > On Tue, May 24, 2022 at 10:19:27AM +0200, Michael Nazzareno Trimarchi wrote:
> > > Hi
> > >
> > > On Tue, May 24, 2022 at 9:56 AM Tommaso Merciai
> > > <tommaso.merciai@amarulasolutions.com> wrote:
> > > >
> > > > On Sun, May 22, 2022 at 03:38:12PM +0200, Michael Trimarchi wrote:
> > > > > RMII/RGMII mode can be enable from dp83822 straps, and also writing bit 9 and
> > > > > bit 5 of register 0x17 - RMII and Status Register (RCSR).
> > > > > When phy_interface_is_rgmii rgmii mode must be enabled, same for
> > > > > contrary, this prevents malconfigurations of hw straps. If bit 9 is 0
> > > > > the RMII/MII mode is determinated by bit 5.
> > > > >
> > > > > References:
> > > > >  - https://www.ti.com/lit/gpn/dp83822i p66
> > > > >
> > > > > Signed-off-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
> > > > > Co-developed-by: Michael Trimarchi <michael@amarulasolutions.com>
> > > > > Suggested-by: Alberto Bianchi <alberto.bianchi@amarulasolutions.com>
> > > > > Tested-by: Tommaso Merciai <tommaso.merciai@amarulasolutions.com>
> > > > > ---
> > > > >  drivers/net/phy/dp83822.c | 15 ++++++++++++++-
> > > > >  1 file changed, 14 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/net/phy/dp83822.c b/drivers/net/phy/dp83822.c
> > > > > index c344b8ffaf3c..89887a05f23d 100644
> > > > > --- a/drivers/net/phy/dp83822.c
> > > > > +++ b/drivers/net/phy/dp83822.c
> > > > > @@ -94,7 +94,9 @@
> > > > >  #define DP83822_WOL_INDICATION_SEL BIT(8)
> > > > >  #define DP83822_WOL_CLR_INDICATION BIT(11)
> > > > >
> > > > > -/* RSCR bits */
> > > > > +/* RCSR bits */
> > > > > +#define DP83822_RMII_MODE_EN BIT(5)
> > > > > +#define DP83822_RGMII_MODE_EN        BIT(9)
> > > > >  #define DP83822_RX_CLK_SHIFT BIT(12)
> > > > >  #define DP83822_TX_CLK_SHIFT BIT(11)
> > > > >
> > > > > @@ -403,6 +405,17 @@ static int dp83822_config_init(struct phy_device *phydev)
> > > > >                       if (err)
> > > > >                               return err;
> > > > >               }
> > > > > +
> > > > > +             phy_set_bits_mmd(phydev, DP83822_DEVADDR,
> > > > > +                                     MII_DP83822_RCSR, DP83822_RGMII_MODE_EN);
> > > >
> > > > Hi Michael,
> > > > Can you clarify this peace of code pls:
> > > >
> > > > ----------------------------------------------------------------------------------------
> > > > > +     } else {
> > > > > +             value = DP83822_RGMII_MODE_EN;
> > > > > +
> > > >
> > > This drop in v2
> > >
> > > > if phy mode is rmii, value is != rmii?
> > > >
> > > > > +             if (phydev->interface == PHY_INTERFACE_MODE_RMII)
> > > > > +                     value != DP83822_RMII_MODE_EN;
> > > >
> > > > This clear rgmii_en and rmii_en and after set value
> > >
> > > As I said this shoule be
> > > if (phydev->interface == PHY_INTERFACE_MODE_RMII)
> > >       value = DP83822_RMII_MODE_EN;
> >
> > Sorry Michael,
> > I forgot. v2 Need some rework or can I review it?
> >
> > Thanks,
> > Tommaso
> >
> > > >
> > > > > +
> > > > > +             phy_modify_mmd(phydev, DP83822_DEVADDR, MII_DP83822_RCSR,
> > > > > +                             DP83822_RGMII_MODE_EN | DP83822_RMII_MODE_EN, value);
> > > > >       }
> > >
> > > Michael
> > >
> > > > -----------------------------------------------------------------------------------------
> > > >
> > > > I have some doubt about this. I'm confuse sorry.
> > > >
> > > > Thanks,
> > > > Tommaso
> > > >
> > > > >
> > > > >       if (dp83822->fx_enabled) {
> > > > > --
> > > > > 2.25.1
> > > > >
> > > >
> > > > --
> > > > Tommaso Merciai
> > > > Embedded Linux Engineer
> > > > tommaso.merciai@amarulasolutions.com
> > > > __________________________________
> > > >
> > > > Amarula Solutions SRL
> > > > Via Le Canevare 30, 31100 Treviso, Veneto, IT
> > > > T. +39 042 243 5310
> > > > info@amarulasolutions.com
> > > > www.amarulasolutions.com
> > >
> > >
> > >
> > > --
> > > Michael Nazzareno Trimarchi
> > > Co-Founder & Chief Executive Officer
> > > M. +39 347 913 2170
> > > michael@amarulasolutions.com
> > > __________________________________
> > >
> > > Amarula Solutions BV
> > > Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> > > T. +31 (0)85 111 9172
> > > info@amarulasolutions.com
> > > www.amarulasolutions.com
> >
> > --
> > Tommaso Merciai
> > Embedded Linux Engineer
> > tommaso.merciai@amarulasolutions.com
> > __________________________________
> >
> > Amarula Solutions SRL
> > Via Le Canevare 30, 31100 Treviso, Veneto, IT
> > T. +39 042 243 5310
> > info@amarulasolutions.com
> > www.amarulasolutions.com
> 
> 
> 
> -- 
> Michael Nazzareno Trimarchi
> Co-Founder & Chief Executive Officer
> M. +39 347 913 2170
> michael@amarulasolutions.com
> __________________________________
> 
> Amarula Solutions BV
> Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> T. +31 (0)85 111 9172
> info@amarulasolutions.com
> www.amarulasolutions.com