| Message ID | 20220522133813.119729-2-michael@amarulasolutions.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
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
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); + } else { + value = DP83822_RGMII_MODE_EN; + + 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); } if (dp83822->fx_enabled) {