* [PATCH] net/phy: fix DP83865 10 Mbps HDX loopback disable function @ 2019-09-18 14:03 Peter Mamonov 2019-09-18 14:19 ` Andrew Lunn 0 siblings, 1 reply; 5+ messages in thread From: Peter Mamonov @ 2019-09-18 14:03 UTC (permalink / raw) To: andrew; +Cc: Peter Mamonov, Florian Fainelli, netdev, linux-kernel According to the DP83865 datasheet "The 10 Mbps HDX loopback can be disabled in the expanded memory register 0x1C0.1." The driver erroneously used bit 0 instead of bit 1. Signed-off-by: Peter Mamonov <pmamonov@gmail.com> --- drivers/net/phy/national.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/net/phy/national.c b/drivers/net/phy/national.c index 2addf1d3f619..4892e785dbf3 100644 --- a/drivers/net/phy/national.c +++ b/drivers/net/phy/national.c @@ -110,11 +110,14 @@ static void ns_giga_speed_fallback(struct phy_device *phydev, int mode) static void ns_10_base_t_hdx_loopack(struct phy_device *phydev, int disable) { + u16 lb_dis = 1 << 1; + if (disable) - ns_exp_write(phydev, 0x1c0, ns_exp_read(phydev, 0x1c0) | 1); + ns_exp_write(phydev, 0x1c0, + ns_exp_read(phydev, 0x1c0) | lb_dis); else ns_exp_write(phydev, 0x1c0, - ns_exp_read(phydev, 0x1c0) & 0xfffe); + ns_exp_read(phydev, 0x1c0) & ~lb_dis); pr_debug("10BASE-T HDX loopback %s\n", (ns_exp_read(phydev, 0x1c0) & 0x0001) ? "off" : "on"); -- 2.23.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] net/phy: fix DP83865 10 Mbps HDX loopback disable function 2019-09-18 14:03 [PATCH] net/phy: fix DP83865 10 Mbps HDX loopback disable function Peter Mamonov @ 2019-09-18 14:19 ` Andrew Lunn 2019-09-18 14:48 ` [PATCH v1] " Peter Mamonov 0 siblings, 1 reply; 5+ messages in thread From: Andrew Lunn @ 2019-09-18 14:19 UTC (permalink / raw) To: Peter Mamonov; +Cc: Florian Fainelli, netdev, linux-kernel On Wed, Sep 18, 2019 at 05:03:40PM +0300, Peter Mamonov wrote: > According to the DP83865 datasheet "The 10 Mbps HDX loopback can be > disabled in the expanded memory register 0x1C0.1." The driver erroneously > used bit 0 instead of bit 1. > > Signed-off-by: Peter Mamonov <pmamonov@gmail.com> > --- > drivers/net/phy/national.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/phy/national.c b/drivers/net/phy/national.c > index 2addf1d3f619..4892e785dbf3 100644 > --- a/drivers/net/phy/national.c > +++ b/drivers/net/phy/national.c > @@ -110,11 +110,14 @@ static void ns_giga_speed_fallback(struct phy_device *phydev, int mode) > > static void ns_10_base_t_hdx_loopack(struct phy_device *phydev, int disable) > { > + u16 lb_dis = 1 << 1; Hi Peter Please use the BIT() macro. > + > if (disable) > - ns_exp_write(phydev, 0x1c0, ns_exp_read(phydev, 0x1c0) | 1); > + ns_exp_write(phydev, 0x1c0, > + ns_exp_read(phydev, 0x1c0) | lb_dis); > else > ns_exp_write(phydev, 0x1c0, > - ns_exp_read(phydev, 0x1c0) & 0xfffe); > + ns_exp_read(phydev, 0x1c0) & ~lb_dis); > > pr_debug("10BASE-T HDX loopback %s\n", > (ns_exp_read(phydev, 0x1c0) & 0x0001) ? "off" : "on"); Isn't this also wrong? Andrew ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v1] net/phy: fix DP83865 10 Mbps HDX loopback disable function 2019-09-18 14:19 ` Andrew Lunn @ 2019-09-18 14:48 ` Peter Mamonov 2019-09-18 15:26 ` Andrew Lunn 0 siblings, 1 reply; 5+ messages in thread From: Peter Mamonov @ 2019-09-18 14:48 UTC (permalink / raw) To: andrew; +Cc: Peter Mamonov, Florian Fainelli, netdev, linux-kernel According to the DP83865 datasheet "The 10 Mbps HDX loopback can be disabled in the expanded memory register 0x1C0.1." The driver erroneously used bit 0 instead of bit 1. Signed-off-by: Peter Mamonov <pmamonov@gmail.com> --- drivers/net/phy/national.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/net/phy/national.c b/drivers/net/phy/national.c index 2addf1d3f619..3aa910b3dc89 100644 --- a/drivers/net/phy/national.c +++ b/drivers/net/phy/national.c @@ -110,14 +110,17 @@ static void ns_giga_speed_fallback(struct phy_device *phydev, int mode) static void ns_10_base_t_hdx_loopack(struct phy_device *phydev, int disable) { + u16 lb_dis = BIT(1); + if (disable) - ns_exp_write(phydev, 0x1c0, ns_exp_read(phydev, 0x1c0) | 1); + ns_exp_write(phydev, 0x1c0, + ns_exp_read(phydev, 0x1c0) | lb_dis); else ns_exp_write(phydev, 0x1c0, - ns_exp_read(phydev, 0x1c0) & 0xfffe); + ns_exp_read(phydev, 0x1c0) & ~lb_dis); pr_debug("10BASE-T HDX loopback %s\n", - (ns_exp_read(phydev, 0x1c0) & 0x0001) ? "off" : "on"); + (ns_exp_read(phydev, 0x1c0) & lb_dis) ? "off" : "on"); } static int ns_config_init(struct phy_device *phydev) -- 2.23.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v1] net/phy: fix DP83865 10 Mbps HDX loopback disable function 2019-09-18 14:48 ` [PATCH v1] " Peter Mamonov @ 2019-09-18 15:26 ` Andrew Lunn 2019-09-18 15:40 ` Peter Mamonov 0 siblings, 1 reply; 5+ messages in thread From: Andrew Lunn @ 2019-09-18 15:26 UTC (permalink / raw) To: Peter Mamonov; +Cc: Florian Fainelli, netdev, linux-kernel On Wed, Sep 18, 2019 at 05:48:25PM +0300, Peter Mamonov wrote: > According to the DP83865 datasheet "The 10 Mbps HDX loopback can be > disabled in the expanded memory register 0x1C0.1." The driver erroneously > used bit 0 instead of bit 1. Hi Peter This is version 2, not 1. Or if you want to start counting from 0, it would be good to put v0 in your first patch :-) It is also normal to put in the commit message what changed from the previous version. This is a fix. So please add a Fixes: tag, with the hash of the commit which introduced the problem. And since this is a fix, it should be against DaveM net tree, and you indicate this in the subject line with [PATCH net v3]. Thanks Andrew > > Signed-off-by: Peter Mamonov <pmamonov@gmail.com> > --- > drivers/net/phy/national.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/phy/national.c b/drivers/net/phy/national.c > index 2addf1d3f619..3aa910b3dc89 100644 > --- a/drivers/net/phy/national.c > +++ b/drivers/net/phy/national.c > @@ -110,14 +110,17 @@ static void ns_giga_speed_fallback(struct phy_device *phydev, int mode) > > static void ns_10_base_t_hdx_loopack(struct phy_device *phydev, int disable) > { > + u16 lb_dis = BIT(1); > + > if (disable) > - ns_exp_write(phydev, 0x1c0, ns_exp_read(phydev, 0x1c0) | 1); > + ns_exp_write(phydev, 0x1c0, > + ns_exp_read(phydev, 0x1c0) | lb_dis); > else > ns_exp_write(phydev, 0x1c0, > - ns_exp_read(phydev, 0x1c0) & 0xfffe); > + ns_exp_read(phydev, 0x1c0) & ~lb_dis); > > pr_debug("10BASE-T HDX loopback %s\n", > - (ns_exp_read(phydev, 0x1c0) & 0x0001) ? "off" : "on"); > + (ns_exp_read(phydev, 0x1c0) & lb_dis) ? "off" : "on"); > } > > static int ns_config_init(struct phy_device *phydev) > -- > 2.23.0 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v1] net/phy: fix DP83865 10 Mbps HDX loopback disable function 2019-09-18 15:26 ` Andrew Lunn @ 2019-09-18 15:40 ` Peter Mamonov 0 siblings, 0 replies; 5+ messages in thread From: Peter Mamonov @ 2019-09-18 15:40 UTC (permalink / raw) To: Andrew Lunn; +Cc: Florian Fainelli, netdev, linux-kernel Hi Andrew, On Wed, Sep 18, 2019 at 05:26:46PM +0200, Andrew Lunn wrote: > On Wed, Sep 18, 2019 at 05:48:25PM +0300, Peter Mamonov wrote: > > According to the DP83865 datasheet "The 10 Mbps HDX loopback can be > > disabled in the expanded memory register 0x1C0.1." The driver erroneously > > used bit 0 instead of bit 1. > > Hi Peter > > This is version 2, not 1. Or if you want to start counting from 0, it > would be good to put v0 in your first patch :-) > > It is also normal to put in the commit message what changed from the > previous version. > > This is a fix. So please add a Fixes: tag, with the hash of the commit > which introduced the problem. > > And since this is a fix, it should be against DaveM net tree, and you > indicate this in the subject line with [PATCH net v3]. Thanks for the tips! Will submit new version soon. Regards, Peter > > Thanks > Andrew > > > > > Signed-off-by: Peter Mamonov <pmamonov@gmail.com> > > --- > > drivers/net/phy/national.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/net/phy/national.c b/drivers/net/phy/national.c > > index 2addf1d3f619..3aa910b3dc89 100644 > > --- a/drivers/net/phy/national.c > > +++ b/drivers/net/phy/national.c > > @@ -110,14 +110,17 @@ static void ns_giga_speed_fallback(struct phy_device *phydev, int mode) > > > > static void ns_10_base_t_hdx_loopack(struct phy_device *phydev, int disable) > > { > > + u16 lb_dis = BIT(1); > > + > > if (disable) > > - ns_exp_write(phydev, 0x1c0, ns_exp_read(phydev, 0x1c0) | 1); > > + ns_exp_write(phydev, 0x1c0, > > + ns_exp_read(phydev, 0x1c0) | lb_dis); > > else > > ns_exp_write(phydev, 0x1c0, > > - ns_exp_read(phydev, 0x1c0) & 0xfffe); > > + ns_exp_read(phydev, 0x1c0) & ~lb_dis); > > > > pr_debug("10BASE-T HDX loopback %s\n", > > - (ns_exp_read(phydev, 0x1c0) & 0x0001) ? "off" : "on"); > > + (ns_exp_read(phydev, 0x1c0) & lb_dis) ? "off" : "on"); > > } > > > > static int ns_config_init(struct phy_device *phydev) > > -- > > 2.23.0 > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-09-18 15:40 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-09-18 14:03 [PATCH] net/phy: fix DP83865 10 Mbps HDX loopback disable function Peter Mamonov 2019-09-18 14:19 ` Andrew Lunn 2019-09-18 14:48 ` [PATCH v1] " Peter Mamonov 2019-09-18 15:26 ` Andrew Lunn 2019-09-18 15:40 ` Peter Mamonov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).