Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next v3 0/2] net: phy: at803x: Update delays for RGMII modes
From: Vinod Koul @ 2019-02-19  6:17 UTC (permalink / raw)
  To: David S Miller
  Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, netdev, Niklas Cassel,
	Andrew Lunn, Florian Fainelli, Nori, Sekhar, Peter Ujfalusi,
	Marc Gonzalez

Peter[1] reported that patch cd28d1d6e52e: ("net: phy: at803x: Disable
phy delay for RGMII mode") caused regression on am335x-evmsk board.
This board expects the Phy delay to be enabled but specified RGMII mode
which refers to delays being disabled. So fix this by disabling delay only
for RGMII mode and enable for RGMII_ID and RGMII_TXID/RXID modes.

While at it, as pointed by Dave, don't inline the helpers.

[1]: https://www.spinics.net/lists/netdev/msg550749.html

Vinod Koul (2):
  net: phy: at803x: don't inline helpers
  net: phy: at803x: disable delay only for RGMII mode

 drivers/net/phy/at803x.c | 51 ++++++++++++++++++++++++++++++----------
 1 file changed, 38 insertions(+), 13 deletions(-)

-- 
2.20.1


^ permalink raw reply

* [PATCH net-next v3 1/2] net: phy: at803x: don't inline helpers
From: Vinod Koul @ 2019-02-19  6:17 UTC (permalink / raw)
  To: David S Miller
  Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, netdev, Niklas Cassel,
	Andrew Lunn, Florian Fainelli, Nori, Sekhar, Peter Ujfalusi,
	Marc Gonzalez
In-Reply-To: <20190219061800.31025-1-vkoul@kernel.org>

Some helpers were inlined, but makes more sense to allow compiler
to do the right optimizations instead, so remove inline for
at803x_disable_rx_delay() and at803x_disable_tx_delay()

Reviewed-by: Niklas Cassel <niklas.cassel@linaro.org>
Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/net/phy/at803x.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index 8ff12938ab47..c6e7d800fd7a 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -110,13 +110,13 @@ static int at803x_debug_reg_mask(struct phy_device *phydev, u16 reg,
 	return phy_write(phydev, AT803X_DEBUG_DATA, val);
 }
 
-static inline int at803x_disable_rx_delay(struct phy_device *phydev)
+static int at803x_disable_rx_delay(struct phy_device *phydev)
 {
 	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
 				     AT803X_DEBUG_RX_CLK_DLY_EN, 0);
 }
 
-static inline int at803x_disable_tx_delay(struct phy_device *phydev)
+static int at803x_disable_tx_delay(struct phy_device *phydev)
 {
 	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_5,
 				     AT803X_DEBUG_TX_CLK_DLY_EN, 0);
-- 
2.20.1


^ permalink raw reply related

* [PATCH net-next v3 2/2] net: phy: at803x: disable delay only for RGMII mode
From: Vinod Koul @ 2019-02-19  6:18 UTC (permalink / raw)
  To: David S Miller
  Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, netdev, Niklas Cassel,
	Andrew Lunn, Florian Fainelli, Nori, Sekhar, Peter Ujfalusi,
	Marc Gonzalez, Peter Ujfalusi
In-Reply-To: <20190219061800.31025-1-vkoul@kernel.org>

Per "Documentation/devicetree/bindings/net/ethernet.txt" RGMII mode
should not have delay in PHY whereas RGMII_ID and RGMII_RXID/RGMII_TXID
can have delay in phy.

So disable the delay only for RGMII mode and enable for other modes.
Also treat the default case as disabled delays.

Fixes: cd28d1d6e52e: ("net: phy: at803x: Disable phy delay for RGMII mode")
Reported-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
Reviewed-by: Niklas Cassel <niklas.cassel@linaro.org>
Tested-by: Peter Ujfalusi <peter.ujflausi@ti.com>
Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/net/phy/at803x.c | 47 ++++++++++++++++++++++++++++++----------
 1 file changed, 36 insertions(+), 11 deletions(-)

diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c
index c6e7d800fd7a..dc1b13f7fc12 100644
--- a/drivers/net/phy/at803x.c
+++ b/drivers/net/phy/at803x.c
@@ -110,6 +110,18 @@ static int at803x_debug_reg_mask(struct phy_device *phydev, u16 reg,
 	return phy_write(phydev, AT803X_DEBUG_DATA, val);
 }
 
+static int at803x_enable_rx_delay(struct phy_device *phydev)
+{
+	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0, 0,
+				     AT803X_DEBUG_RX_CLK_DLY_EN);
+}
+
+static int at803x_enable_tx_delay(struct phy_device *phydev)
+{
+	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_5, 0,
+				     AT803X_DEBUG_TX_CLK_DLY_EN);
+}
+
 static int at803x_disable_rx_delay(struct phy_device *phydev)
 {
 	return at803x_debug_reg_mask(phydev, AT803X_DEBUG_REG_0,
@@ -255,23 +267,36 @@ static int at803x_config_init(struct phy_device *phydev)
 	if (ret < 0)
 		return ret;
 
-	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
-			phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
-			phydev->interface == PHY_INTERFACE_MODE_RGMII) {
-		ret = at803x_disable_rx_delay(phydev);
+	/* The hardware register default is RX and TX delay enabled, so lets
+	 * first disable the RX and TX delays in phy and enable them based
+	 * on the mode selected
+	 */
+	ret = at803x_disable_rx_delay(phydev);
+	if (ret < 0)
+		return ret;
+	ret = at803x_disable_tx_delay(phydev);
+	if (ret < 0)
+		return ret;
+
+	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
+	    phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID) {
+		/* If RGMII_ID or RGMII_RXID are specified enable RX delay,
+		 * otherwise keep it disabled
+		 */
+		ret = at803x_enable_rx_delay(phydev);
 		if (ret < 0)
 			return ret;
 	}
 
-	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
-			phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
-			phydev->interface == PHY_INTERFACE_MODE_RGMII) {
-		ret = at803x_disable_tx_delay(phydev);
-		if (ret < 0)
-			return ret;
+	if (phydev->interface == PHY_INTERFACE_MODE_RGMII_ID ||
+	    phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID) {
+		/* If RGMII_ID or RGMII_TXID are specified enable TX delay,
+		 * otherwise keep it disabled
+		 */
+		ret = at803x_enable_tx_delay(phydev);
 	}
 
-	return 0;
+	return ret;
 }
 
 static int at803x_ack_interrupt(struct phy_device *phydev)
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH RFC 0/5] net/sched: validate the control action with all the other parameters
From: Cong Wang @ 2019-02-19  6:42 UTC (permalink / raw)
  To: Davide Caratti
  Cc: Jamal Hadi Salim, Jiri Pirko, David S. Miller, Vlad Buslov,
	Paolo Abeni, Linux Kernel Network Developers
In-Reply-To: <cover.1550271080.git.dcaratti@redhat.com>

On Fri, Feb 15, 2019 at 3:06 PM Davide Caratti <dcaratti@redhat.com> wrote:
>
> currently, the kernel checks for bad values of the control action in
> tcf_action_init_1(), after a successful call to the action's init()
> function. This causes three bad behaviors:

Yeah, I have been complaining about this for a long time,
although slightly differently. The problem here is the lack of
"copy" in RCU mechanism, which makes it nearly impossible
to rollback to the previous state of an action on failure path
of an update, which also makes RCU readers reading a partially
updated action too.

Before I fix the "copy" part, your fixes make sense to me. There
might be some other way to expose the action-specific tcfa_action
opcode, but it might not be better than yours.

BTW, please fold these bad behaviors into each appropriate
patch, and keep the cover letter as an overview of the whole
patchset rather than showing any details.

[...]

> all these three problems can be fixed if we validate the control action
> in the init() function, in the same way as we are already doing for all
> the other parameters.
>
> - patch 1 is a temporary fix for problem 2), but it's reverted at the
>   end of the series

Please drop patch 1, it is very unlikely only patch 1 will be backported,
I think the whole patchset should be, therefore we have no reason
to carry a temporary fix here.

Thanks.

^ permalink raw reply

* Re: [PATCH net] vhost: correctly check the return value of translate_desc() in log_used()
From: Jason Wang @ 2019-02-19  6:50 UTC (permalink / raw)
  To: David Miller; +Cc: mst, kvm, virtualization, netdev, linux-kernel, stephen
In-Reply-To: <20190215.100356.251698752340585300.davem@davemloft.net>


On 2019/2/16 上午2:03, David Miller wrote:
> From: Jason Wang<jasowang@redhat.com>
> Date: Fri, 15 Feb 2019 15:53:24 +0800
>
>> When fail, translate_desc() returns negative value, otherwise the
>> number of iovs. So we should fail when the return value is negative
>> instead of a blindly check against zero.
>>
>> Reported-by: Stephen Hemminger<stephen@networkplumber.org>
>> Fixes: cc5e71075947 ("vhost: log dirty page correctly")
>> Signed-off-by: Jason Wang<jasowang@redhat.com>
> Jason, please put the Fixes tag first.
>
> Thank you.


Ok. Will post V2.

Thanks


^ permalink raw reply

* Re: [PATCH net] vhost: correctly check the return value of translate_desc() in log_used()
From: Jason Wang @ 2019-02-19  6:51 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: mst, kvm, virtualization, netdev, linux-kernel
In-Reply-To: <20190215084502.23feed0e@shemminger-XPS-13-9360>


On 2019/2/16 上午12:45, Stephen Hemminger wrote:
> On Fri, 15 Feb 2019 15:53:24 +0800
> Jason Wang <jasowang@redhat.com> wrote:
>
>> When fail, translate_desc() returns negative value, otherwise the
>> number of iovs. So we should fail when the return value is negative
>> instead of a blindly check against zero.
>>
>> Reported-by: Stephen Hemminger <stephen@networkplumber.org>
>> Fixes: cc5e71075947 ("vhost: log dirty page correctly")
>> Signed-off-by: Jason Wang <jasowang@redhat.com>
> Looks good. It is best to put the Addresses-Coverity-Id tag on these kind
> of bug fixes so that the automated tools see it.


Ok. Will do this in V2.

Thanks



^ permalink raw reply

* [PATCH net V2] vhost: correctly check the return value of translate_desc() in log_used()
From: Jason Wang @ 2019-02-19  6:53 UTC (permalink / raw)
  To: mst, jasowang; +Cc: kvm, virtualization, netdev, linux-kernel

When fail, translate_desc() returns negative value, otherwise the
number of iovs. So we should fail when the return value is negative
instead of a blindly check against zero.

Detected by CoverityScan, CID# 1442593:  Control flow issues  (DEADCODE)

Fixes: cc5e71075947 ("vhost: log dirty page correctly")
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Reported-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/vhost/vhost.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 24a129fcdd61..a2e5dc7716e2 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -1788,7 +1788,7 @@ static int log_used(struct vhost_virtqueue *vq, u64 used_offset, u64 len)
 
 	ret = translate_desc(vq, (uintptr_t)vq->used + used_offset,
 			     len, iov, 64, VHOST_ACCESS_WO);
-	if (ret)
+	if (ret < 0)
 		return ret;
 
 	for (i = 0; i < ret; i++) {
-- 
2.17.1


^ permalink raw reply related

* [PATCH net-next v2] net: dsa: qca8k: Enable delay for RGMII_ID mode
From: Vinod Koul @ 2019-02-19  6:59 UTC (permalink / raw)
  To: David S Miller
  Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, netdev, Niklas Cassel,
	Andrew Lunn, Florian Fainelli, Michal Vokáč

RGMII_ID specifies that we should have internal delay, so resurrect the
delay addition routine but under the RGMII_ID mode.

Fixes: 40269aa9f40a ("net: dsa: qca8k: disable delay for RGMII mode")
Tested-by: Michal Vokáč <michal.vokac@ysoft.com>
Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
 drivers/net/dsa/qca8k.c | 12 ++++++++++++
 drivers/net/dsa/qca8k.h |  1 +
 2 files changed, 13 insertions(+)

diff --git a/drivers/net/dsa/qca8k.c b/drivers/net/dsa/qca8k.c
index a4b6cda38016..195a8a87b984 100644
--- a/drivers/net/dsa/qca8k.c
+++ b/drivers/net/dsa/qca8k.c
@@ -443,6 +443,18 @@ qca8k_set_pad_ctrl(struct qca8k_priv *priv, int port, int mode)
 		val = QCA8K_PORT_PAD_RGMII_EN;
 		qca8k_write(priv, reg, val);
 		break;
+	case PHY_INTERFACE_MODE_RGMII_ID:
+		/* RGMII_ID needs internal delay. This is enabled through
+		 * PORT5_PAD_CTRL for all ports, rather than individual port
+		 * registers
+		 */
+		qca8k_write(priv, reg,
+			    QCA8K_PORT_PAD_RGMII_EN |
+			    QCA8K_PORT_PAD_RGMII_TX_DELAY(QCA8K_MAX_DELAY) |
+			    QCA8K_PORT_PAD_RGMII_RX_DELAY(QCA8K_MAX_DELAY));
+		qca8k_write(priv, QCA8K_REG_PORT5_PAD_CTRL,
+			    QCA8K_PORT_PAD_RGMII_RX_DELAY_EN);
+		break;
 	case PHY_INTERFACE_MODE_SGMII:
 		qca8k_write(priv, reg, QCA8K_PORT_PAD_SGMII_EN);
 		break;
diff --git a/drivers/net/dsa/qca8k.h b/drivers/net/dsa/qca8k.h
index 613fe5c50236..d146e54c8a6c 100644
--- a/drivers/net/dsa/qca8k.h
+++ b/drivers/net/dsa/qca8k.h
@@ -40,6 +40,7 @@
 						((0x8 + (x & 0x3)) << 22)
 #define   QCA8K_PORT_PAD_RGMII_RX_DELAY(x)		\
 						((0x10 + (x & 0x3)) << 20)
+#define   QCA8K_MAX_DELAY				3
 #define   QCA8K_PORT_PAD_RGMII_RX_DELAY_EN		BIT(24)
 #define   QCA8K_PORT_PAD_SGMII_EN			BIT(7)
 #define QCA8K_REG_MODULE_EN				0x030
-- 
2.20.1


^ permalink raw reply related

* Re: [RFC PATCH net-next v3 07/21] ethtool: implement EVENT notifications
From: Michal Kubecek @ 2019-02-19  7:02 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: netdev, David Miller, Jakub Kicinski, Jiri Pirko, linux-kernel
In-Reply-To: <20190218234608.GI14879@lunn.ch>

On Tue, Feb 19, 2019 at 12:46:08AM +0100, Andrew Lunn wrote:
> On Mon, Feb 18, 2019 at 07:21:59PM +0100, Michal Kubecek wrote:
> > Three types of netlink notifications are introduced:
> > 
> >   - ETHA_EVENT_NEWDEV to notify about newly registered network devices
> >   - ETHA_EVENT_DELDEV to notify about unregistered network devices
> >   - ETHA_EVENT_RENAMEDEV to notify about renamed network device
> > 
> > The notifications are triggered by NETDEV_REGISTER, NETDEV_UNREGISTER and
> > NETDEV_CHANGENAME notifiers.
> > 
> > These notifications are intended for applications and daemons monitoring
> > ethtool events to allow updating the list of existing devices without
> > having to open another socket for rtnetlink.
> 
> Hi Michal
> 
> Does ETHA_EVENT_RENAMEDEV actually contain enough information to avoid
> needing a rtnetlink lookup? If i understand the code correctly, all
> you have is the new name. You don't know the old name?
> 
> Having said that, i don't see an easy way to get access to the old
> name when handling the NETDEV_CHANGENAME.

We don't have the old name and without modifying the NETDEV_CHANGENAME
notifier to pass it, there is probably no way to get it. But the ethtool
notification also contains ifindex so that userspace applications which
want to track the configuration can remember both and identify the
renamed device by ifindex.

Michal

^ permalink raw reply

* [tipc-discussion][net 0/1] tipc: fix race condition between sleep/wakeup
From: Tung Nguyen @ 2019-02-19  7:03 UTC (permalink / raw)
  To: davem, netdev; +Cc: tipc-discussion

Fix issue of hung sendto() in user space.

Tung Nguyen (1):
  tipc: fix race condition causing hung sendto

 net/tipc/socket.c | 4 ++++
 1 file changed, 4 insertions(+)
 mode change 100644 => 100755 net/tipc/socket.c

-- 
2.17.1


^ permalink raw reply

* [tipc-discussion][net 1/1] tipc: fix race condition causing hung sendto
From: Tung Nguyen @ 2019-02-19  7:03 UTC (permalink / raw)
  To: davem, netdev; +Cc: tipc-discussion
In-Reply-To: <20190219070310.23888-1-tung.q.nguyen@dektech.com.au>

When sending multicast messages via blocking socket,
if sending link is congested (tsk->cong_link_cnt is set to 1),
the sending thread will be put into sleeping state. However,
tipc_sk_filter_rcv() is called under socket spin lock but
tipc_wait_for_cond() is not. So, there is no guarantee that
the setting of tsk->cong_link_cnt to 0 in tipc_sk_proto_rcv() in
CPU-1 will be perceived by CPU-0. If that is the case, the sending
thread in CPU-0 after being waken up, will continue to see
tsk->cong_link_cnt as 1 and put the sending thread into sleeping
state again. The sending thread will sleep forever.

CPU-0                                | CPU-1
tipc_wait_for_cond()                 |
{                                    |
 // condition_ = !tsk->cong_link_cnt |
 while ((rc_ = !(condition_))) {     |
  ...                                |
  release_sock(sk_);                 |
  wait_woken();                      |
                                     | if (!sock_owned_by_user(sk))
                                     |  tipc_sk_filter_rcv()
                                     |  {
                                     |   ...
                                     |   tipc_sk_proto_rcv()
                                     |   {
                                     |    ...
                                     |    tsk->cong_link_cnt--;
                                     |    ...
                                     |    sk->sk_write_space(sk);
                                     |    ...
                                     |   }
                                     |   ...
                                     |  }
  sched_annotate_sleep();            |
  lock_sock(sk_);                    |
  remove_wait_queue();               |
 }                                   |
}                                    |

This commit fixes it by adding memory barrier to tipc_sk_proto_rcv()
and tipc_wait_for_cond().

Acked-by: Jon Maloy <jon.maloy@ericsson.com>
Signed-off-by: Tung Nguyen <tung.q.nguyen@dektech.com.au>
---
 net/tipc/socket.c | 4 ++++
 1 file changed, 4 insertions(+)
 mode change 100644 => 100755 net/tipc/socket.c

diff --git a/net/tipc/socket.c b/net/tipc/socket.c
old mode 100644
new mode 100755
index 1217c90a363b..d8f054d45941
--- a/net/tipc/socket.c
+++ b/net/tipc/socket.c
@@ -383,6 +383,8 @@ static int tipc_sk_sock_err(struct socket *sock, long *timeout)
 	int rc_;							       \
 									       \
 	while ((rc_ = !(condition_))) {					       \
+		/* coupled with smp_wmb() in tipc_sk_proto_rcv() */            \
+		smp_rmb();                                                     \
 		DEFINE_WAIT_FUNC(wait_, woken_wake_function);	               \
 		sk_ = (sock_)->sk;					       \
 		rc_ = tipc_sk_sock_err((sock_), timeo_);		       \
@@ -1982,6 +1984,8 @@ static void tipc_sk_proto_rcv(struct sock *sk,
 		return;
 	case SOCK_WAKEUP:
 		tipc_dest_del(&tsk->cong_links, msg_orignode(hdr), 0);
+		/* coupled with smp_rmb() in tipc_wait_for_cond() */
+		smp_wmb();
 		tsk->cong_link_cnt--;
 		wakeup = true;
 		break;
-- 
2.17.1


^ permalink raw reply related

* [PATCH] phy/marvell: prevent unneeded page switching
From: Frank de Brabander @ 2019-02-19  7:21 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, Heiner Kallweit; +Cc: Frank de Brabander, netdev

marvell_read_status() no longer first switches to the fiber page, instead
looks at the already active register page first. In case the link is down,
switches to the other (fiber/copper) register page.

Unneeded register page switching of the Marvell 88E1510 PHY can cause
the ethernet driver to register a link down & up event.

Signed-off-by: Frank de Brabander <debrabander@gmail.com>
---
 drivers/net/phy/marvell.c | 59 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 38 insertions(+), 21 deletions(-)

diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index abb7876..31de2db 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -194,6 +194,11 @@ static int marvell_read_page(struct phy_device *phydev)
 	return __phy_read(phydev, MII_MARVELL_PHY_PAGE);
 }
 
+static int marvell_get_page(struct phy_device *phydev)
+{
+	return phy_read(phydev, MII_MARVELL_PHY_PAGE);
+}
+
 static int marvell_write_page(struct phy_device *phydev, int page)
 {
 	return __phy_write(phydev, MII_MARVELL_PHY_PAGE, page);
@@ -1245,47 +1250,59 @@ static int marvell_read_status_page(struct phy_device *phydev, int page)
  * Some Marvell's phys have two modes: fiber and copper.
  * Both need status checked.
  * Description:
- *   First, check the fiber link and status.
- *   If the fiber link is down, check the copper link and status which
- *   will be the default value if both link are down.
+ *   First, check the link of the currently selected page. Unneeded page
+ *   switches should be prevented, they can trigger the ethernet driver to
+ *   register a link down event.
+ *   If the first checked link type is down, continue with checking the other
+ *   link type. This is needed for platforms that have a fiber and copper link
+ *   connected to the same phy.
  */
 static int marvell_read_status(struct phy_device *phydev)
 {
 	int err;
+	int page;
 
-	/* Check the fiber mode first */
 	if (linkmode_test_bit(ETHTOOL_LINK_MODE_FIBRE_BIT,
 			      phydev->supported) &&
 	    phydev->interface != PHY_INTERFACE_MODE_SGMII) {
-		err = marvell_set_page(phydev, MII_MARVELL_FIBER_PAGE);
-		if (err < 0)
-			goto error;
+		page = marvell_get_page(phydev);
+		if (page < 0)
+			return page;
+
+		if (page != MII_MARVELL_FIBER_PAGE &&
+		    page != MII_MARVELL_COPPER_PAGE) {
+			page = MII_MARVELL_FIBER_PAGE;
 
-		err = marvell_read_status_page(phydev, MII_MARVELL_FIBER_PAGE);
+			err = marvell_set_page(phydev, page);
+			if (err < 0)
+				return err;
+		}
+
+		err = marvell_read_status_page(phydev, page);
 		if (err < 0)
-			goto error;
+			return err;
 
-		/* If the fiber link is up, it is the selected and
-		 * used link. In this case, we need to stay in the
-		 * fiber page. Please to be careful about that, avoid
-		 * to restore Copper page in other functions which
-		 * could break the behaviour for some fiber phy like
+		/* If the link is up, it is the selected and
+		 * used link. In this case, we need to stay in that
+		 * page. Please to be careful about that, avoid
+		 * to restore the page in other functions which
+		 * could break the behaviour for some phy's like
 		 * 88E1512.
 		 */
 		if (phydev->link)
 			return 0;
 
-		/* If fiber link is down, check and save copper mode state */
-		err = marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
+		page = (page == MII_MARVELL_FIBER_PAGE) ?
+			MII_MARVELL_COPPER_PAGE : MII_MARVELL_FIBER_PAGE;
+
+		err = marvell_set_page(phydev, page);
 		if (err < 0)
-			goto error;
+			return err;
+
+		return marvell_read_status_page(phydev, page);
 	}
 
 	return marvell_read_status_page(phydev, MII_MARVELL_COPPER_PAGE);
-
-error:
-	marvell_set_page(phydev, MII_MARVELL_COPPER_PAGE);
-	return err;
 }
 
 /* marvell_suspend
-- 
2.7.4


^ permalink raw reply related

* [PATCH] net: ns83820: drop pointless static qualifier in ns83820_probe_phy()
From: Mao Wenan @ 2019-02-19  8:06 UTC (permalink / raw)
  To: kernel-janitors, netdev, john.fastabend, hawk, jakub.kicinski,
	daniel, ast

There is no need to have the 'int first' static
since new value always be assigned before use it.
The old codes of first dereferencing this variable have
been commented out. So it is useless with 'static int first'.
/*
if (!first) {
 ...
}
*/
first = 1;

Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 drivers/net/ethernet/natsemi/ns83820.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/natsemi/ns83820.c b/drivers/net/ethernet/natsemi/ns83820.c
index 958fced4dacf..fb064df3a1a6 100644
--- a/drivers/net/ethernet/natsemi/ns83820.c
+++ b/drivers/net/ethernet/natsemi/ns83820.c
@@ -1869,7 +1869,7 @@ static unsigned ns83820_mii_write_reg(struct ns83820 *dev, unsigned phy, unsigne
 static void ns83820_probe_phy(struct net_device *ndev)
 {
 	struct ns83820 *dev = PRIV(ndev);
-	static int first;
+	int first;
 	int i;
 #define MII_PHYIDR1	0x02
 #define MII_PHYIDR2	0x03
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH] net: ns83820: drop pointless static qualifier in ns83820_probe_phy()
From: Julia Lawall @ 2019-02-19  8:01 UTC (permalink / raw)
  To: Mao Wenan
  Cc: kernel-janitors, netdev, john.fastabend, hawk, jakub.kicinski,
	daniel, ast
In-Reply-To: <20190219080659.89732-1-maowenan@huawei.com>



On Tue, 19 Feb 2019, Mao Wenan wrote:

> There is no need to have the 'int first' static
> since new value always be assigned before use it.
> The old codes of first dereferencing this variable have
> been commented out. So it is useless with 'static int first'.
> /*
> if (!first) {
>  ...
> }
> */
> first = 1;

Do you need the variable at all?

In the code, the commenting out is actually done with #if 0.  It could be
good for the commit log to reflect the code more accurately.

Can the #if 0 and code be removed?  Apparently it predates git.

A commented out printk below also predates git.

julia

>
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
>  drivers/net/ethernet/natsemi/ns83820.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/natsemi/ns83820.c b/drivers/net/ethernet/natsemi/ns83820.c
> index 958fced4dacf..fb064df3a1a6 100644
> --- a/drivers/net/ethernet/natsemi/ns83820.c
> +++ b/drivers/net/ethernet/natsemi/ns83820.c
> @@ -1869,7 +1869,7 @@ static unsigned ns83820_mii_write_reg(struct ns83820 *dev, unsigned phy, unsigne
>  static void ns83820_probe_phy(struct net_device *ndev)
>  {
>  	struct ns83820 *dev = PRIV(ndev);
> -	static int first;
> +	int first;
>  	int i;
>  #define MII_PHYIDR1	0x02
>  #define MII_PHYIDR2	0x03
> --
> 2.20.1
>
>

^ permalink raw reply

* Re: [PATCH 0/2] ARM: dts: am335x-evm/evmsk: Fix PHY mode for ethernet
From: Peter Ujfalusi @ 2019-02-19  8:02 UTC (permalink / raw)
  To: Tony Lindgren
  Cc: bcousson, linux-omap, devicetree, linux-arm-kernel, nsekhar,
	grygorii.strashko, vkoul, netdev, f.fainelli, marc.w.gonzalez,
	niklas.cassel
In-Reply-To: <20190218162613.GH15711@atomide.com>

Hi Tony,

On 18/02/2019 18.26, Tony Lindgren wrote:
> * Peter Ujfalusi <peter.ujfalusi@ti.com> [190218 16:22]:
>>
>>
>> On 18/02/2019 16.44, Tony Lindgren wrote:
>>> * Peter Ujfalusi <peter.ujfalusi@ti.com> [190218 14:36]:
>>>> Hi,
>>>>
>>>> cd28d1d6e52e: ("net: phy: at803x: Disable phy delay for RGMII mode") broke the
>>>> ethernet networking on evmsk (and most likely on the evm as well):
>>>> https://patchwork.ozlabs.org/patch/1028527/
>>>>
>>>> v1 patch to fix the situation:
>>>> https://patchwork.ozlabs.org/patch/1040617/
>>>>
>>>> It turned out that the at803x driver is actually broken and need to be fixed
>>>> along with the DT data.
>>>>
>>>> The following series is proposed to fix the driver:
>>>> https://patchwork.ozlabs.org/project/netdev/list/?series=92611
>>>>
>>>> but the PHT mode needs to be switched to rgmii-id from rgmii-txid:
>>>> The rx delay is enabled by default and the driver never disabled it so when
>>>> asking rgmii-txid it actually got rgmii-id.
>>>>
>>>> The patch can be backported to stable, I have tested that it is not causing
>>>> regression with the old, broken driver.
>>>
>>> Can the dts changes be merged before the driver changes or
>>> does it cause the phy to stop working?
>>
>> The phy is not working atm, but this change will not cause regression
>> even if it is merged first.
> 
> OK so sounds like these are OK to wait for v5.1 merge window
> then as the dts changes alone won't fix anything?

I think it would be better to send these to 5.0 to avoid the regression
siting in linux-next.

The patches are actually doing to the correct thing (we need rgmii-id,
not rmgii-txid for these boards).
It was just a matter of luck that it worked with rgmii-txid as the
driver was broken and did not disabled the rxid when it should have.

Tested the patch on top of mainline
(b5372fe5dc84235dbe04998efdede3c4daa866a9) was the topmost commit after
the rc7 tag.

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

^ permalink raw reply

* [PATCH net-next] net: rose: add missing dev_put() on error in rose_bind
From: YueHaibing @ 2019-02-19  8:06 UTC (permalink / raw)
  To: ralf, davem; +Cc: linux-kernel, netdev, linux-hams, YueHaibing

when capable check failed, dev_put should
be call before return -EACCES.

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
 net/rose/af_rose.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/rose/af_rose.c b/net/rose/af_rose.c
index d00a0ef..c96f63f 100644
--- a/net/rose/af_rose.c
+++ b/net/rose/af_rose.c
@@ -689,8 +689,10 @@ static int rose_bind(struct socket *sock, struct sockaddr *uaddr, int addr_len)
 		rose->source_call = user->call;
 		ax25_uid_put(user);
 	} else {
-		if (ax25_uid_policy && !capable(CAP_NET_BIND_SERVICE))
+		if (ax25_uid_policy && !capable(CAP_NET_BIND_SERVICE)) {
+			dev_put(dev);
 			return -EACCES;
+		}
 		rose->source_call   = *source;
 	}
 
-- 
2.7.0



^ permalink raw reply related

* Re: [PATCH net-next v3 0/2] net: phy: at803x: Update delays for RGMII modes
From: Peter Ujfalusi @ 2019-02-19  8:09 UTC (permalink / raw)
  To: Vinod Koul, David S Miller
  Cc: linux-arm-msm, Bjorn Andersson, netdev, Niklas Cassel,
	Andrew Lunn, Florian Fainelli, Nori, Sekhar, Marc Gonzalez
In-Reply-To: <20190219061800.31025-1-vkoul@kernel.org>

Hi Vinod,

On 19/02/2019 8.17, Vinod Koul wrote:
> Peter[1] reported that patch cd28d1d6e52e: ("net: phy: at803x: Disable
> phy delay for RGMII mode") caused regression on am335x-evmsk board.
> This board expects the Phy delay to be enabled but specified RGMII mode
> which refers to delays being disabled. So fix this by disabling delay only
> for RGMII mode and enable for RGMII_ID and RGMII_TXID/RXID modes.
> 
> While at it, as pointed by Dave, don't inline the helpers.
> 
> [1]: https://www.spinics.net/lists/netdev/msg550749.html

Not sure what was changed since v2, but with
https://patchwork.ozlabs.org/project/netdev/list/?series=92672
ethernet is working on am335x-evmsk. thank you:

Tested-by: Peter Ujfalusi <peter.ujflausi@ti.com>

> Vinod Koul (2):
>   net: phy: at803x: don't inline helpers
>   net: phy: at803x: disable delay only for RGMII mode
> 
>  drivers/net/phy/at803x.c | 51 ++++++++++++++++++++++++++++++----------
>  1 file changed, 38 insertions(+), 13 deletions(-)
> 

- Péter

Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

^ permalink raw reply

* Re: [PATCH] net: ns83820: drop pointless static qualifier in ns83820_probe_phy()
From: maowenan @ 2019-02-19  8:09 UTC (permalink / raw)
  To: Julia Lawall
  Cc: kernel-janitors, netdev, john.fastabend, hawk, jakub.kicinski,
	daniel, ast
In-Reply-To: <alpine.DEB.2.21.1902190857300.2570@hadrien>



On 2019/2/19 16:01, Julia Lawall wrote:
> 
> 
> On Tue, 19 Feb 2019, Mao Wenan wrote:
> 
>> There is no need to have the 'int first' static
>> since new value always be assigned before use it.
>> The old codes of first dereferencing this variable have
>> been commented out. So it is useless with 'static int first'.
>> /*
>> if (!first) {
>>  ...
>> }
>> */
>> first = 1;
> 
> Do you need the variable at all?
> 
> In the code, the commenting out is actually done with #if 0.  It could be
> good for the commit log to reflect the code more accurately.
> 
OK, the original codes are using #if 0 to comment out, but when I generate patch
the "#if 0" has been removed, so I use "/*...*/" to reflect that.

> Can the #if 0 and code be removed?  Apparently it predates git.
> 
> A commented out printk below also predates git.
> 
I think all the "#if 0" comment out codes and printk codes can be removed, shall I send
v2 to do this?

> julia
> 
>>
>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
>> ---
>>  drivers/net/ethernet/natsemi/ns83820.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/natsemi/ns83820.c b/drivers/net/ethernet/natsemi/ns83820.c
>> index 958fced4dacf..fb064df3a1a6 100644
>> --- a/drivers/net/ethernet/natsemi/ns83820.c
>> +++ b/drivers/net/ethernet/natsemi/ns83820.c
>> @@ -1869,7 +1869,7 @@ static unsigned ns83820_mii_write_reg(struct ns83820 *dev, unsigned phy, unsigne
>>  static void ns83820_probe_phy(struct net_device *ndev)
>>  {
>>  	struct ns83820 *dev = PRIV(ndev);
>> -	static int first;
>> +	int first;
>>  	int i;
>>  #define MII_PHYIDR1	0x02
>>  #define MII_PHYIDR2	0x03
>> --
>> 2.20.1
>>
>>
> 
> .
> 


^ permalink raw reply

* Re: [PATCH] net: ns83820: drop pointless static qualifier in ns83820_probe_phy()
From: Julia Lawall @ 2019-02-19  8:15 UTC (permalink / raw)
  To: maowenan
  Cc: kernel-janitors, netdev, john.fastabend, hawk, jakub.kicinski,
	daniel, ast
In-Reply-To: <578c4208-956d-b905-6a50-5ad04620a349@huawei.com>



On Tue, 19 Feb 2019, maowenan wrote:

>
>
> On 2019/2/19 16:01, Julia Lawall wrote:
> >
> >
> > On Tue, 19 Feb 2019, Mao Wenan wrote:
> >
> >> There is no need to have the 'int first' static
> >> since new value always be assigned before use it.
> >> The old codes of first dereferencing this variable have
> >> been commented out. So it is useless with 'static int first'.
> >> /*
> >> if (!first) {
> >>  ...
> >> }
> >> */
> >> first = 1;
> >
> > Do you need the variable at all?
> >
> > In the code, the commenting out is actually done with #if 0.  It could be
> > good for the commit log to reflect the code more accurately.
> >
> OK, the original codes are using #if 0 to comment out, but when I generate patch
> the "#if 0" has been removed, so I use "/*...*/" to reflect that.

I don't understand the above comment.  The proposed patch doesn't have any
impact on the #if 0 lines.

>
> > Can the #if 0 and code be removed?  Apparently it predates git.
> >
> > A commented out printk below also predates git.
> >
> I think all the "#if 0" comment out codes and printk codes can be removed, shall I send
> v2 to do this?

It seems reasonable to me.

julia

> > julia
> >
> >>
> >> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> >> ---
> >>  drivers/net/ethernet/natsemi/ns83820.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/net/ethernet/natsemi/ns83820.c b/drivers/net/ethernet/natsemi/ns83820.c
> >> index 958fced4dacf..fb064df3a1a6 100644
> >> --- a/drivers/net/ethernet/natsemi/ns83820.c
> >> +++ b/drivers/net/ethernet/natsemi/ns83820.c
> >> @@ -1869,7 +1869,7 @@ static unsigned ns83820_mii_write_reg(struct ns83820 *dev, unsigned phy, unsigne
> >>  static void ns83820_probe_phy(struct net_device *ndev)
> >>  {
> >>  	struct ns83820 *dev = PRIV(ndev);
> >> -	static int first;
> >> +	int first;
> >>  	int i;
> >>  #define MII_PHYIDR1	0x02
> >>  #define MII_PHYIDR2	0x03
> >> --
> >> 2.20.1
> >>
> >>
> >
> > .
> >
>
>

^ permalink raw reply

* Re: [PATCH] net/mlx4_en: fix spelling mistake: "quiting" -> "quitting"
From: Tariq Toukan @ 2019-02-19  8:20 UTC (permalink / raw)
  To: David Miller, colin.king@canonical.com
  Cc: Tariq Toukan, netdev@vger.kernel.org, linux-rdma@vger.kernel.org,
	kernel-janitors@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20190218.120836.1419708231583986821.davem@davemloft.net>



On 2/18/2019 10:08 PM, David Miller wrote:
> From: Colin King <colin.king@canonical.com>
> Date: Sun, 17 Feb 2019 23:03:31 +0000
> 
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> There is a spelling mistake in a en_err error message. Fix it.
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> 
> Applied, thanks Colin.
> 
> And I agree that this doesn't really deserve a Fixes: tag.
> 
> Fixes: tags should really be for changes that introduce truly
> functional bugs.
> 
> And that could even be applied in this case _iff_ the string
> was essential in some way for userland tools which parse the
> output or similar.  But that is not the case here.
> 

Thanks for the clarification.

> Anyways, thanks.
> 

^ permalink raw reply

* Regarding CLNP driver
From: Shreeya Patel @ 2019-02-19  8:22 UTC (permalink / raw)
  To: netdev@vger.kernel.org; +Cc: Moustafa Ali, Mohamed El-Dessouki

Hi All,

My goal is to have the CLNP ( Connectionless network protocol ) driver
working on the latest Ubuntu version / any Linux distros.
But I see that CLNP driver was developed by a group of people during
2.6 kernel and now it doesn't exist anymore.
Here is the source code...
https://github.com/crazoes/clnp

I tried to build the driver on latest kernel of latest Ubuntu version
by having 2.6 kernel's header files but as we all know it is not
possible to have so old module working on the latest version.

I also tried to downgrade the kernel on Ubuntu 18.04 to 2.6 kernel but
again I knew this was not going to work and it showed me an error of
KERNEL TOO OLD during boot time.

Is there any way possible to have CLNP driver working on the latest
Ubuntu release or any other Linux distro's release?

Thanks




^ permalink raw reply

* [PATCH v2] net: ns83820: drop pointless static qualifier in ns83820_probe_phy()
From: Mao Wenan @ 2019-02-19  8:42 UTC (permalink / raw)
  To: kernel-janitors, netdev, john.fastabend, hawk, jakub.kicinski,
	daniel, ast

There is no need to have the 'int first' static
since new value always be assigned before use it.

This patch cleans up the codes in ns83820_probe_phy() as well.

Signed-off-by: Mao Wenan <maowenan@huawei.com>
---
 v1->v2: delete unused codes.
 drivers/net/ethernet/natsemi/ns83820.c | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/drivers/net/ethernet/natsemi/ns83820.c b/drivers/net/ethernet/natsemi/ns83820.c
index 958fced4dacf..37694f6843fe 100644
--- a/drivers/net/ethernet/natsemi/ns83820.c
+++ b/drivers/net/ethernet/natsemi/ns83820.c
@@ -1869,23 +1869,11 @@ static unsigned ns83820_mii_write_reg(struct ns83820 *dev, unsigned phy, unsigne
 static void ns83820_probe_phy(struct net_device *ndev)
 {
 	struct ns83820 *dev = PRIV(ndev);
-	static int first;
+	int first;
 	int i;
 #define MII_PHYIDR1	0x02
 #define MII_PHYIDR2	0x03
 
-#if 0
-	if (!first) {
-		unsigned tmp;
-		ns83820_mii_read_reg(dev, 1, 0x09);
-		ns83820_mii_write_reg(dev, 1, 0x10, 0x0d3e);
-
-		tmp = ns83820_mii_read_reg(dev, 1, 0x00);
-		ns83820_mii_write_reg(dev, 1, 0x00, tmp | 0x8000);
-		udelay(1300);
-		ns83820_mii_read_reg(dev, 1, 0x09);
-	}
-#endif
 	first = 1;
 
 	for (i=1; i<2; i++) {
@@ -1894,9 +1882,6 @@ static void ns83820_probe_phy(struct net_device *ndev)
 		a = ns83820_mii_read_reg(dev, i, MII_PHYIDR1);
 		b = ns83820_mii_read_reg(dev, i, MII_PHYIDR2);
 
-		//printk("%s: phy %d: 0x%04x 0x%04x\n",
-		//	ndev->name, i, a, b);
-
 		for (j=0; j<0x16; j+=4) {
 			dprintk("%s: [0x%02x] %04x %04x %04x %04x\n",
 				ndev->name, j,
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH v2] net: ns83820: drop pointless static qualifier in ns83820_probe_phy()
From: Julia Lawall @ 2019-02-19  8:31 UTC (permalink / raw)
  To: Mao Wenan
  Cc: kernel-janitors, netdev, john.fastabend, hawk, jakub.kicinski,
	daniel, ast
In-Reply-To: <20190219084214.96202-1-maowenan@huawei.com>



On Tue, 19 Feb 2019, Mao Wenan wrote:

> There is no need to have the 'int first' static
> since new value always be assigned before use it.
>
> This patch cleans up the codes in ns83820_probe_phy() as well.

As mentioned previously, there is no use of first in the function at all,
other than setting it to 1.  So it can be just dropped completely.

"codes" is also a word that I am particularly not fond of.  Just "code" is
fine.

"cleans up" is not very specific about what is going on, and why.

julia

>
> Signed-off-by: Mao Wenan <maowenan@huawei.com>
> ---
>  v1->v2: delete unused codes.
>  drivers/net/ethernet/natsemi/ns83820.c | 17 +----------------
>  1 file changed, 1 insertion(+), 16 deletions(-)
>
> diff --git a/drivers/net/ethernet/natsemi/ns83820.c b/drivers/net/ethernet/natsemi/ns83820.c
> index 958fced4dacf..37694f6843fe 100644
> --- a/drivers/net/ethernet/natsemi/ns83820.c
> +++ b/drivers/net/ethernet/natsemi/ns83820.c
> @@ -1869,23 +1869,11 @@ static unsigned ns83820_mii_write_reg(struct ns83820 *dev, unsigned phy, unsigne
>  static void ns83820_probe_phy(struct net_device *ndev)
>  {
>  	struct ns83820 *dev = PRIV(ndev);
> -	static int first;
> +	int first;
>  	int i;
>  #define MII_PHYIDR1	0x02
>  #define MII_PHYIDR2	0x03
>
> -#if 0
> -	if (!first) {
> -		unsigned tmp;
> -		ns83820_mii_read_reg(dev, 1, 0x09);
> -		ns83820_mii_write_reg(dev, 1, 0x10, 0x0d3e);
> -
> -		tmp = ns83820_mii_read_reg(dev, 1, 0x00);
> -		ns83820_mii_write_reg(dev, 1, 0x00, tmp | 0x8000);
> -		udelay(1300);
> -		ns83820_mii_read_reg(dev, 1, 0x09);
> -	}
> -#endif
>  	first = 1;
>
>  	for (i=1; i<2; i++) {
> @@ -1894,9 +1882,6 @@ static void ns83820_probe_phy(struct net_device *ndev)
>  		a = ns83820_mii_read_reg(dev, i, MII_PHYIDR1);
>  		b = ns83820_mii_read_reg(dev, i, MII_PHYIDR2);
>
> -		//printk("%s: phy %d: 0x%04x 0x%04x\n",
> -		//	ndev->name, i, a, b);
> -
>  		for (j=0; j<0x16; j+=4) {
>  			dprintk("%s: [0x%02x] %04x %04x %04x %04x\n",
>  				ndev->name, j,
> --
> 2.20.1
>
>

^ permalink raw reply

* Re: [PATCH v2] net: ns83820: drop pointless static qualifier in ns83820_probe_phy()
From: maowenan @ 2019-02-19  8:39 UTC (permalink / raw)
  To: Julia Lawall
  Cc: kernel-janitors, netdev, john.fastabend, hawk, jakub.kicinski,
	daniel, ast
In-Reply-To: <alpine.DEB.2.21.1902190929250.2570@hadrien>



On 2019/2/19 16:31, Julia Lawall wrote:
> 
> 
> On Tue, 19 Feb 2019, Mao Wenan wrote:
> 
>> There is no need to have the 'int first' static
>> since new value always be assigned before use it.
>>
>> This patch cleans up the codes in ns83820_probe_phy() as well.
> 
> As mentioned previously, there is no use of first in the function at all,
> other than setting it to 1.  So it can be just dropped completely.
> 
OK, thank you for your comments.

> "codes" is also a word that I am particularly not fond of.  Just "code" is
> fine.
> 
> "cleans up" is not very specific about what is going on, and why.
> 

> julia
> 
>>
>> Signed-off-by: Mao Wenan <maowenan@huawei.com>
>> ---
>>  v1->v2: delete unused codes.
>>  drivers/net/ethernet/natsemi/ns83820.c | 17 +----------------
>>  1 file changed, 1 insertion(+), 16 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/natsemi/ns83820.c b/drivers/net/ethernet/natsemi/ns83820.c
>> index 958fced4dacf..37694f6843fe 100644
>> --- a/drivers/net/ethernet/natsemi/ns83820.c
>> +++ b/drivers/net/ethernet/natsemi/ns83820.c
>> @@ -1869,23 +1869,11 @@ static unsigned ns83820_mii_write_reg(struct ns83820 *dev, unsigned phy, unsigne
>>  static void ns83820_probe_phy(struct net_device *ndev)
>>  {
>>  	struct ns83820 *dev = PRIV(ndev);
>> -	static int first;
>> +	int first;
>>  	int i;
>>  #define MII_PHYIDR1	0x02
>>  #define MII_PHYIDR2	0x03
>>
>> -#if 0
>> -	if (!first) {
>> -		unsigned tmp;
>> -		ns83820_mii_read_reg(dev, 1, 0x09);
>> -		ns83820_mii_write_reg(dev, 1, 0x10, 0x0d3e);
>> -
>> -		tmp = ns83820_mii_read_reg(dev, 1, 0x00);
>> -		ns83820_mii_write_reg(dev, 1, 0x00, tmp | 0x8000);
>> -		udelay(1300);
>> -		ns83820_mii_read_reg(dev, 1, 0x09);
>> -	}
>> -#endif
>>  	first = 1;
>>
>>  	for (i=1; i<2; i++) {
>> @@ -1894,9 +1882,6 @@ static void ns83820_probe_phy(struct net_device *ndev)
>>  		a = ns83820_mii_read_reg(dev, i, MII_PHYIDR1);
>>  		b = ns83820_mii_read_reg(dev, i, MII_PHYIDR2);
>>
>> -		//printk("%s: phy %d: 0x%04x 0x%04x\n",
>> -		//	ndev->name, i, a, b);
>> -
>>  		for (j=0; j<0x16; j+=4) {
>>  			dprintk("%s: [0x%02x] %04x %04x %04x %04x\n",
>>  				ndev->name, j,
>> --
>> 2.20.1
>>
>>
> 
> .
> 


^ permalink raw reply

* Re: stmmac / meson8b-dwmac
From: Jose Abreu @ 2019-02-19  8:47 UTC (permalink / raw)
  To: Simon Huelck, Jose Abreu, Martin Blumenstingl
  Cc: Emiliano Ingrassia, Gpeppe.cavallaro, alexandre.torgue,
	linux-amlogic, netdev
In-Reply-To: <cd0b3dec-af3f-af69-50b7-6ca6f7256900@gmx.de>

Hi Simon,

On 2/18/2019 6:05 PM, Simon Huelck wrote:
> disabling EEE doesnt help ( did it via the entry in the .dtb / .dts ),
> the results are the same. I can confirm the LPI counters are zero or one
> only after the test.....

It's interesting to see that you have a lot of RX packets but few
RX interrupts. This can either be due to mis-configuration of
interrupt flags or RX Watchdog.

1. For interrupt flags you should be using LEVEL triggered IRQ.
2. For RX Watchdog you can try disabling it by adding the
following in your platform wrapper driver (which will be
"dwmac-meson8b.c", at probe):
	"plat_data->force_sf_dma_mode = 1;" and
	"plat_data->riwt_off = 1;"

Thanks,
Jose Miguel Abreu

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox