* Re: [PATCH V2 3/3] net-next: dsa: add new driver for qca8xxx family
From: John Crispin @ 2016-09-14 15:47 UTC (permalink / raw)
To: David S. Miller, Andrew Lunn, Florian Fainelli
Cc: netdev, linux-kernel, qsdk-review
In-Reply-To: <1473849542-3298-4-git-send-email-john@phrozen.org>
On 14/09/2016 12:39, John Crispin wrote:
> +static void
> +qca8k_fdb_add(struct dsa_switch *ds, int port,
> + const struct switchdev_obj_port_fdb *fdb,
> + struct switchdev_trans *trans)
> +{
> + struct qca8k_priv *priv = qca8k_to_priv(ds);
> + u16 port_mask = BIT(port);
> + u16 vid = fdb->vid;
> +
> + if (!vid)
> + vid = 1;
> +
> + qca8k_fdb_write(priv, vid, port_mask, fdb->addr,
> + QCA8K_ATU_STATUS_STATIC);
> +
> + qca8k_fdb_access(priv, QCA8K_FDB_LOAD, -1);
> +}
> +
> +static int
> +qca8k_fdb_del(struct dsa_switch *ds, int port,
> + const struct switchdev_obj_port_fdb *fdb)
> +{
> + struct qca8k_priv *priv = qca8k_to_priv(ds);
> + u16 port_mask = BIT(port);
> +
> + qca8k_fdb_write(priv, fdb->vid, port_mask, fdb->addr, 0);
> +
> + return qca8k_fdb_access(priv, QCA8K_FDB_PURGE, -1);
> +}
> +
while adding MDB support i noticed that
1) the code above is still not properly locked
2) i can unify the FDB and MDB code
i will send a V3 with those 2 changes tomorrow.
John
^ permalink raw reply
* Re: stmmac/RTL8211F/Meson GXBB: TX throughput problems
From: Giuseppe CAVALLARO @ 2016-09-14 15:30 UTC (permalink / raw)
To: Martin Blumenstingl, netdev, linux-amlogic
Cc: Alexandre Torgue, Johnson Leung
In-Reply-To: <CAFBinCALSL_-izJ2tEsAjevAw90kGNTrQ3na3D1YV8f1dS1=Xg@mail.gmail.com>
Hello Martin
On 9/11/2016 10:39 PM, Martin Blumenstingl wrote:
> Hello,
>
> I have a device with a Meson GXBB SoC with an stmmac IP block.
> Gbit ethernet on my device is provided by a Realtek RTL8211F RGMII PHY.
> Similar issues were reported in #linux-amlogic by a user with an
> Odroid C2 board (= similar hardware).
>
> The symptoms are:
> Receiving data is plenty fast (I can max out my internet connection
> easily, and with iperf3 I get ~900Mbit/s).
> Transmitting data from the device is unfortunately very slow, traffic
> sometimes even stalls completely.
>
> I have attached the iperf results and the output of
> /sys/kernel/debug/stmmaceth/eth0/descriptors_status.
> Below you can find the ifconfig, netstat and stmmac dma_cap info
> (*after* I ran all tests).
>
> The "involved parties" are:
> - Meson GXBB specific network configuration registers (I have have
> double-checked them with the reference drivers: everything seems fine
> here)
> - stmmac: it seems that nobody else has reported these kind of issues
> so far, however I'd still like to hear where I should enable some
> debugging bits to rule out any stmmac bug
hmm I can also think that some configuration could impact!
For example, you could try disabling the scatter-gather or tx-cum
via ethtool and seeing if there is some benefit; so we could image
some problem on your HW or SYNP MAC integration for checksumming
on tx side.
Also you could check the AXI tuning and PBL value. To be honest
(thinking about your problem) I can actually suspect some related
problem on bus setup. So I suggest you to play with these value
(better if you ask for having values from HW validation on your side).
Otherwise the stmmac uses a default that cannot be good for your
platform. For example, sometime I have seen that PBL is better if
reduced to 8 instead of 32 and w/o 4xPBL...
> - RTL8211F PHY driver: unfortunately there are no public datasheets
> available so this is hard to debug. but I'm guessing that TX delay
> could cause similar issues, so this may be the cause as well.
as rule of thumb, I can only suggest you to see the RXDLY and TXDLY
and if you have (or need!) the resistor on PCB to have the 2ns of
extra delay. This can impact on RGMII case (1G).
Indeed, if this is true, I should expect some problem also when ping.
> Thanks for any input in advance!
welcome,
as Alex asked, pls provide us the output from ethtool -S eth0
Regards
Peppe
> Regards,
> Martin
>
>
> [root@alarm ~]# ifconfig eth0
> eth0: flags=4163<UP,BROADCAST,RUNNING,MULTICAST> mtu 1500
> inet 192.168.1.235 netmask 255.255.255.0 broadcast 192.168.1.255
> ether e2:aa:53:fc:f5:c5 txqueuelen 1000 (Ethernet)
> RX packets 1967602 bytes 2968750265 (2.7 GiB)
> RX errors 0 dropped 0 overruns 0 frame 0
> TX packets 101875 bytes 8548285 (8.1 MiB)
> TX errors 0 dropped 0 overruns 0 carrier 0 collisions 0
> device interrupt 18
>
> [root@alarm ~]# netstat -i
> Kernel Interface table
> Iface MTU RX-OK RX-ERR RX-DRP RX-OVR TX-OK TX-ERR TX-DRP TX-OVR Flg
> eth0 1500 1967801 0 0 0 101934 0 0 0 BMRU
>
> [root@alarm ~]# cat /sys/kernel/debug/stmmaceth/eth0/dma_cap
> ==============================
> DMA HW features
> ==============================
> 10/100 Mbps Y
> 1000 Mbps Y
> Half duple Y
> Hash Filter: Y
> Multiple MAC address registers: Y
> PCS (TBI/SGMII/RTBI PHY interfatces): N
> SMA (MDIO) Interface: Y
> PMT Remote wake up: Y
> PMT Magic Frame: Y
> RMON module: Y
> IEEE 1588-2002 Time Stamp: N
> IEEE 1588-2008 Advanced Time Stamp:N
> 802.3az - Energy-Efficient Ethernet (EEE) Y
> AV features: N
> Checksum Offload in TX: Y
> IP Checksum Offload (type1) in RX: N
> IP Checksum Offload (type2) in RX: Y
> RXFIFO > 2048bytes: Y
> Number of Additional RX channel: 0
> Number of Additional TX channel: 0
> Enhanced descriptors: N
>
^ permalink raw reply
* [PATCH stable] tipc: apply skb linearization commit to stable 4.4.x
From: Jon Maloy @ 2016-09-14 15:28 UTC (permalink / raw)
To: davem; +Cc: Jon Maloy, netdev, Paul Gortmaker, tipc-discussion
commit c7cad0d6f70cd upstream
(“tipc: move linearization of buffers to generic code”)
was applied to net-next in November 2015, and is present in kernel
versions from 4.5.x onwards.
We later discovered that this commit also fixes a serious bug, since
even L2 buffers may arrive non-linearized. Hence, in 4.4.x kernels we
often see debug printouts like this:
[880.688856] Dropping name table update (0) of {1651649891, 1819082752, 0} from <1.1.1> key=402710022
[880.688862] Dropping name table update (0) of {4029808599, 2711729614, 1639218685} from <1.1.1> key=18102394
[880.688865] Dropping name table update (0) of {134218495, 4278191616, 100669184} from <1.1.1> key=0
Those are symptoms of the binding table having received "corrupt"
publications read linearly from non-linear buffers. The above listed
commit solves this problem, and should be applied even to 4.4 kernels.
2.7.4
------------------------------------------------------------------------------
_______________________________________________
tipc-discussion mailing list
tipc-discussion@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tipc-discussion
^ permalink raw reply
* Re: [net v1] fib_rules: interface group matching
From: David Ahern @ 2016-09-14 15:25 UTC (permalink / raw)
To: Vincent Bernat; +Cc: David S. Miller, Nicolas Dichtel, Wilson Kok, netdev
In-Reply-To: <87a8fapl7l.fsf@zoro.exoscale.ch>
On 9/14/16 9:14 AM, Vincent Bernat wrote:
> I could just give more time to VRF. I also had some concerns over
> performance with the way Netfilter integration is done, but I understand
> that I could just stay away from POSTROUTING rules which is the only
> hook executed twice?
>
With the changes that were committed this past weekend, the VRF code is now setup where I can set a flag on a per VRF basis to disable the extra rx and tx processing - ie., no network taps, no netfilter, no qdisc, etc. Drops the overhead of VRF to ~3% maybe a bit less. I need to think about the user api a bit more and formalize the patch. Given my other commitments that probably won't happen until mid-October. But in terms of a building block, the overhead of VRF is continuing to drop.
^ permalink raw reply
* Re: [net v1] fib_rules: interface group matching
From: Vincent Bernat @ 2016-09-14 15:14 UTC (permalink / raw)
To: David Ahern; +Cc: David S. Miller, Nicolas Dichtel, Wilson Kok, netdev
In-Reply-To: <e8df55df-c22f-19a3-fabc-5fb592433706@cumulusnetworks.com>
❦ 14 septembre 2016 16:39 CEST, David Ahern <dsa@cumulusnetworks.com> :
>>>> When a user wants to assign a routing table to a group of incoming
>>>> interfaces, the current solutions are:
>>>>
>>>> - one IP rule for each interface (scalability problems)
>>>> - use of fwmark and devgroup matcher (don't work with internal route
>>>> lookups, used for example by RPF)
>>>> - use of VRF devices (more complex)
>>>
>>> Why do you believe that? A VRF is a formalized grouping of interfaces
>>> that includes an API for locally generated traffic to specify which
>>> VRF/group to use. And, with the l3mdev rule you only need 1 rule for
>>> all VRFs regardless of the number which is the best solution to the
>>> scalability problem of adding rules per device/group/VRF.
>>>
>>> What use case are trying to solve?
>>
>> Local processes have to be made aware of the VRF by binding to the
>> pseudo-device. Some processes may be tricked by LD_PRELOAD but some
>> won't (like stuff written in Go). Maybe I should just find a better way
>> to bind a process to a VRF without its cooperation.
>
> What API are you using for interface groups? How does an app tell the
> kernel to use interface group 1 versus group 2?
In my testbed, I have only one local application which is dnsmasq as a
DHCP server. It sends back the answer to the physical interface (with
sendmsg() and auxillary data). So it makes my argument a bit moot as the
situation is in fact worse without VRF. :-/
My testbed is here (with use of VRF, more recent commits just use plain
ip rules):
https://github.com/vincentbernat/network-lab/blob/d86e9ed658863ef0f51d7b853d0dc9f8b7427b21/lab-l3-hyperv/setup
I could just give more time to VRF. I also had some concerns over
performance with the way Netfilter integration is done, but I understand
that I could just stay away from POSTROUTING rules which is the only
hook executed twice?
--
All things that are, are with more spirit chased than enjoyed.
-- Shakespeare, "Merchant of Venice"
^ permalink raw reply
* [PATCH net-next v2 7/7] net: ethernet: mediatek: avoid race condition during the reset process
From: sean.wang @ 2016-09-14 15:13 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, keyhaede, objelf, Sean Wang
In-Reply-To: <1473866001-9805-1-git-send-email-sean.wang@mediatek.com>
From: Sean Wang <sean.wang@mediatek.com>
add the protection of the race condition between
the reset process and hardware access happening
on the related callbacks.
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 36 +++++++++++++++++++++++++++++
drivers/net/ethernet/mediatek/mtk_eth_soc.h | 3 ++-
2 files changed, 38 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index cde9cd6..b46b08a 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -145,6 +145,9 @@ static void mtk_phy_link_adjust(struct net_device *dev)
MAC_MCR_RX_EN | MAC_MCR_BACKOFF_EN |
MAC_MCR_BACKPR_EN;
+ if (unlikely(test_bit(MTK_RESETTING, &mac->hw->state)))
+ return;
+
switch (mac->phy_dev->speed) {
case SPEED_1000:
mcr |= MAC_MCR_SPEED_1000;
@@ -370,6 +373,9 @@ static int mtk_set_mac_address(struct net_device *dev, void *p)
if (ret)
return ret;
+ if (unlikely(test_bit(MTK_RESETTING, &mac->hw->state)))
+ return -EBUSY;
+
spin_lock_bh(&mac->hw->page_lock);
mtk_w32(mac->hw, (macaddr[0] << 8) | macaddr[1],
MTK_GDMA_MAC_ADRH(mac->id));
@@ -770,6 +776,9 @@ static int mtk_start_xmit(struct sk_buff *skb, struct net_device *dev)
*/
spin_lock(ð->page_lock);
+ if (unlikely(test_bit(MTK_RESETTING, ð->state)))
+ goto drop;
+
tx_num = mtk_cal_txd_req(skb);
if (unlikely(atomic_read(&ring->free_count) <= tx_num)) {
mtk_stop_queue(eth);
@@ -842,6 +851,9 @@ static int mtk_poll_rx(struct napi_struct *napi, int budget,
netdev = eth->netdev[mac];
+ if (unlikely(test_bit(MTK_RESETTING, ð->state)))
+ goto release_desc;
+
/* alloc new buffer */
new_data = napi_alloc_frag(ring->frag_size);
if (unlikely(!new_data)) {
@@ -1573,6 +1585,12 @@ static void mtk_pending_work(struct work_struct *work)
rtnl_lock();
+ dev_dbg(eth->dev, "[%s][%d] reset\n", __func__, __LINE__);
+
+ while (test_and_set_bit_lock(MTK_RESETTING, ð->state))
+ cpu_relax();
+
+ dev_dbg(eth->dev, "[%s][%d] mtk_stop starts\n", __func__, __LINE__);
/* stop all devices to make sure that dma is properly shut down */
for (i = 0; i < MTK_MAC_COUNT; i++) {
if (!eth->netdev[i])
@@ -1580,6 +1598,7 @@ static void mtk_pending_work(struct work_struct *work)
mtk_stop(eth->netdev[i]);
__set_bit(i, &restart);
}
+ dev_dbg(eth->dev, "[%s][%d] mtk_stop ends\n", __func__, __LINE__);
/* restart underlying hardware such as power, clock, pin mux
* and the connected phy
@@ -1612,6 +1631,11 @@ static void mtk_pending_work(struct work_struct *work)
dev_close(eth->netdev[i]);
}
}
+
+ dev_dbg(eth->dev, "[%s][%d] reset done\n", __func__, __LINE__);
+
+ clear_bit_unlock(MTK_RESETTING, ð->state);
+
rtnl_unlock();
}
@@ -1656,6 +1680,9 @@ static int mtk_get_settings(struct net_device *dev,
struct mtk_mac *mac = netdev_priv(dev);
int err;
+ if (unlikely(test_bit(MTK_RESETTING, &mac->hw->state)))
+ return -EBUSY;
+
err = phy_read_status(mac->phy_dev);
if (err)
return -ENODEV;
@@ -1706,6 +1733,9 @@ static int mtk_nway_reset(struct net_device *dev)
{
struct mtk_mac *mac = netdev_priv(dev);
+ if (unlikely(test_bit(MTK_RESETTING, &mac->hw->state)))
+ return -EBUSY;
+
return genphy_restart_aneg(mac->phy_dev);
}
@@ -1714,6 +1744,9 @@ static u32 mtk_get_link(struct net_device *dev)
struct mtk_mac *mac = netdev_priv(dev);
int err;
+ if (unlikely(test_bit(MTK_RESETTING, &mac->hw->state)))
+ return -EBUSY;
+
err = genphy_update_link(mac->phy_dev);
if (err)
return ethtool_op_get_link(dev);
@@ -1754,6 +1787,9 @@ static void mtk_get_ethtool_stats(struct net_device *dev,
unsigned int start;
int i;
+ if (unlikely(test_bit(MTK_RESETTING, &mac->hw->state)))
+ return;
+
if (netif_running(dev) && netif_device_present(dev)) {
if (spin_trylock(&hwstats->stats_lock)) {
mtk_stats_update_mac(mac);
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index 7efa00f..79954b4 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -336,7 +336,8 @@ enum mtk_clks_map {
};
enum mtk_dev_state {
- MTK_HW_INIT
+ MTK_HW_INIT,
+ MTK_RESETTING
};
/* struct mtk_tx_buf - This struct holds the pointers to the memory pointed at
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v2 2/7] net: ethernet: mediatek: add mtk_hw_deinit call as the opposite to mtk_hw_init call
From: sean.wang @ 2016-09-14 15:13 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, keyhaede, objelf, Sean Wang
In-Reply-To: <1473866001-9805-1-git-send-email-sean.wang@mediatek.com>
From: Sean Wang <sean.wang@mediatek.com>
grouping things related to the deinitialization of what
mtk_hw_init call does that help to be reused by the reset
process and the error path handling.
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index ca46e82..c71b0b3 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1477,6 +1477,16 @@ static int __init mtk_hw_init(struct mtk_eth *eth)
return 0;
}
+static int mtk_hw_deinit(struct mtk_eth *eth)
+{
+ clk_disable_unprepare(eth->clks[MTK_CLK_GP2]);
+ clk_disable_unprepare(eth->clks[MTK_CLK_GP1]);
+ clk_disable_unprepare(eth->clks[MTK_CLK_ESW]);
+ clk_disable_unprepare(eth->clks[MTK_CLK_ETHIF]);
+
+ return 0;
+}
+
static int __init mtk_init(struct net_device *dev)
{
struct mtk_mac *mac = netdev_priv(dev);
@@ -1923,10 +1933,7 @@ static int mtk_remove(struct platform_device *pdev)
mtk_stop(eth->netdev[i]);
}
- clk_disable_unprepare(eth->clks[MTK_CLK_ETHIF]);
- clk_disable_unprepare(eth->clks[MTK_CLK_ESW]);
- clk_disable_unprepare(eth->clks[MTK_CLK_GP1]);
- clk_disable_unprepare(eth->clks[MTK_CLK_GP2]);
+ mtk_hw_deinit(eth);
netif_napi_del(ð->tx_napi);
netif_napi_del(ð->rx_napi);
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v2 0/7] add enhancement into the existing reset flow
From: sean.wang @ 2016-09-14 15:13 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, keyhaede, objelf, Sean Wang
From: Sean Wang <sean.wang@mediatek.com>
Current driver only resets DMA used by descriptor rings which
can't guarantee it can recover all various kinds of fatal
errors, so the patch
1) tries to reset the underlying hardware resource from scratch on
Mediatek SoC required for ethernet running.
2) refactors code in order to the reusability of existing code.
3) considers handling for race condition between the reset flow and
callbacks registered into core driver called about hardware accessing.
4) introduces power domain usage to hardware setup which leads to have
cleanly and completely restore to the state as the initial.
Changes since v1:
- fix the build error with module built causing undefined symbol for
pinctrl_bind_pins, so using pinctrl_select_state instead accomplishes
the pin mux setup during the reset process.
Sean Wang (7):
net: ethernet: mediatek: refactoring mtk_hw_init to be reused
net: ethernet: mediatek: add mtk_hw_deinit call as the opposite to
mtk_hw_init call
net: ethernet: mediatek: cleanup error path inside mtk_hw_init
net: ethernet: mediatek: add controlling power domain the ethernet
belongs to
net: ethernet: mediatek: add the whole ethernet reset into the reset
process
net: ethernet: mediatek: add more resets for internal ethernet circuit
block
net: ethernet: mediatek: avoid race condition during the reset process
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 226 +++++++++++++++++++++-------
drivers/net/ethernet/mediatek/mtk_eth_soc.h | 15 +-
2 files changed, 186 insertions(+), 55 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH net-next v2 5/7] net: ethernet: mediatek: add the whole ethernet reset into the reset process
From: sean.wang @ 2016-09-14 15:13 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, keyhaede, objelf, Sean Wang
In-Reply-To: <1473866001-9805-1-git-send-email-sean.wang@mediatek.com>
From: Sean Wang <sean.wang@mediatek.com>
1) original driver only resets DMA used by descriptor rings
which can't guarantee it can recover all various kinds of fatal
errors, so the patch tries to reset the underlying hardware
resource from scratch on Mediatek SoC required for ethernet
running, including power, pin mux control, clock and internal
circuits on the ethernet in order to restore into the initial
state which the rebooted machine gives.
2) add state variable inside structure mtk_eth to help distinguish
mtk_hw_init is called between the initialization during boot time
or re-initialization during the reset process.
3) add ge_mode variable inside structure mtk_mac for restoring
the interface mode of the current setup for the target MAC.
4) remove __init attribute from mtk_hw_init definition
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 51 ++++++++++++++++++++++++-----
drivers/net/ethernet/mediatek/mtk_eth_soc.h | 8 +++++
2 files changed, 51 insertions(+), 8 deletions(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index fd5d064..a7e2e77 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -231,7 +231,7 @@ static int mtk_phy_connect(struct mtk_mac *mac)
{
struct mtk_eth *eth = mac->hw;
struct device_node *np;
- u32 val, ge_mode;
+ u32 val;
np = of_parse_phandle(mac->of_node, "phy-handle", 0);
if (!np && of_phy_is_fixed_link(mac->of_node))
@@ -245,18 +245,18 @@ static int mtk_phy_connect(struct mtk_mac *mac)
case PHY_INTERFACE_MODE_RGMII_RXID:
case PHY_INTERFACE_MODE_RGMII_ID:
case PHY_INTERFACE_MODE_RGMII:
- ge_mode = 0;
+ mac->ge_mode = 0;
break;
case PHY_INTERFACE_MODE_MII:
- ge_mode = 1;
+ mac->ge_mode = 1;
break;
case PHY_INTERFACE_MODE_REVMII:
- ge_mode = 2;
+ mac->ge_mode = 2;
break;
case PHY_INTERFACE_MODE_RMII:
if (!mac->id)
goto err_phy;
- ge_mode = 3;
+ mac->ge_mode = 3;
break;
default:
goto err_phy;
@@ -265,7 +265,7 @@ static int mtk_phy_connect(struct mtk_mac *mac)
/* put the gmac into the right mode */
regmap_read(eth->ethsys, ETHSYS_SYSCFG0, &val);
val &= ~SYSCFG0_GE_MODE(SYSCFG0_GE_MASK, mac->id);
- val |= SYSCFG0_GE_MODE(ge_mode, mac->id);
+ val |= SYSCFG0_GE_MODE(mac->ge_mode, mac->id);
regmap_write(eth->ethsys, ETHSYS_SYSCFG0, val);
mtk_phy_connect_node(eth, mac, np);
@@ -1414,9 +1414,12 @@ static int mtk_stop(struct net_device *dev)
return 0;
}
-static int __init mtk_hw_init(struct mtk_eth *eth)
+static int mtk_hw_init(struct mtk_eth *eth)
{
- int i;
+ int i, val;
+
+ if (test_and_set_bit(MTK_HW_INIT, ð->state))
+ return 0;
pm_runtime_enable(eth->dev);
pm_runtime_get_sync(eth->dev);
@@ -1432,6 +1435,15 @@ static int __init mtk_hw_init(struct mtk_eth *eth)
reset_control_deassert(eth->rstc);
usleep_range(10, 20);
+ regmap_read(eth->ethsys, ETHSYS_SYSCFG0, &val);
+ for (i = 0; i < MTK_MAC_COUNT; i++) {
+ if (!eth->mac[i])
+ continue;
+ val &= ~SYSCFG0_GE_MODE(SYSCFG0_GE_MASK, eth->mac[i]->id);
+ val |= SYSCFG0_GE_MODE(eth->mac[i]->ge_mode, eth->mac[i]->id);
+ }
+ regmap_write(eth->ethsys, ETHSYS_SYSCFG0, val);
+
/* Set GE2 driving and slew rate */
regmap_write(eth->pctl, GPIO_DRV_SEL10, 0xa00);
@@ -1483,6 +1495,9 @@ static int __init mtk_hw_init(struct mtk_eth *eth)
static int mtk_hw_deinit(struct mtk_eth *eth)
{
+ if (!test_and_clear_bit(MTK_HW_INIT, ð->state))
+ return 0;
+
clk_disable_unprepare(eth->clks[MTK_CLK_GP2]);
clk_disable_unprepare(eth->clks[MTK_CLK_GP1]);
clk_disable_unprepare(eth->clks[MTK_CLK_ESW]);
@@ -1557,6 +1572,26 @@ static void mtk_pending_work(struct work_struct *work)
__set_bit(i, &restart);
}
+ /* restart underlying hardware such as power, clock, pin mux
+ * and the connected phy
+ */
+ mtk_hw_deinit(eth);
+
+ if (eth->dev->pins)
+ pinctrl_select_state(eth->dev->pins->p,
+ eth->dev->pins->default_state);
+ mtk_hw_init(eth);
+
+ for (i = 0; i < MTK_MAC_COUNT; i++) {
+ if (!eth->mac[i] ||
+ of_phy_is_fixed_link(eth->mac[i]->of_node))
+ continue;
+ err = phy_init_hw(eth->mac[i]->phy_dev);
+ if (err)
+ dev_err(eth->dev, "%s: PHY init failed.\n",
+ eth->netdev[i]->name);
+ }
+
/* restart DMA and enable IRQs */
for (i = 0; i < MTK_MAC_COUNT; i++) {
if (!test_bit(i, &restart))
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index 0b984dc..388cbe7 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -330,6 +330,10 @@ enum mtk_clks_map {
MTK_CLK_MAX
};
+enum mtk_dev_state {
+ MTK_HW_INIT
+};
+
/* struct mtk_tx_buf - This struct holds the pointers to the memory pointed at
* by the TX descriptor s
* @skb: The SKB pointer of the packet being sent
@@ -413,6 +417,7 @@ struct mtk_rx_ring {
* @clks: clock array for all clocks required
* @mii_bus: If there is a bus we need to create an instance for it
* @pending_work: The workqueue used to reset the dma ring
+ * @state Initialization and runtime state of the device.
*/
struct mtk_eth {
@@ -441,11 +446,13 @@ struct mtk_eth {
struct mii_bus *mii_bus;
struct work_struct pending_work;
+ unsigned long state;
};
/* struct mtk_mac - the structure that holds the info about the MACs of the
* SoC
* @id: The number of the MAC
+ * @ge_mode: Interface mode kept for setup restoring
* @of_node: Our devicetree node
* @hw: Backpointer to our main datastruture
* @hw_stats: Packet statistics counter
@@ -453,6 +460,7 @@ struct mtk_eth {
*/
struct mtk_mac {
int id;
+ int ge_mode;
struct device_node *of_node;
struct mtk_eth *hw;
struct mtk_hw_stats *hw_stats;
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v2 4/7] net: ethernet: mediatek: add controlling power domain the ethernet belongs to
From: sean.wang @ 2016-09-14 15:13 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, keyhaede, objelf, Sean Wang
In-Reply-To: <1473866001-9805-1-git-send-email-sean.wang@mediatek.com>
From: Sean Wang <sean.wang@mediatek.com>
introduce power domain control which the digital circuit of
the ethernet belongs to inside the flow of hardware initialization
and deinitialization which helps the entire ethernet hardware block
could restart cleanly and completely as being back to the initial
state when the whole machine reboot.
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 917a49c6..fd5d064 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -18,6 +18,7 @@
#include <linux/mfd/syscon.h>
#include <linux/regmap.h>
#include <linux/clk.h>
+#include <linux/pm_runtime.h>
#include <linux/if_vlan.h>
#include <linux/reset.h>
#include <linux/tcp.h>
@@ -1417,6 +1418,9 @@ static int __init mtk_hw_init(struct mtk_eth *eth)
{
int i;
+ pm_runtime_enable(eth->dev);
+ pm_runtime_get_sync(eth->dev);
+
clk_prepare_enable(eth->clks[MTK_CLK_ETHIF]);
clk_prepare_enable(eth->clks[MTK_CLK_ESW]);
clk_prepare_enable(eth->clks[MTK_CLK_GP1]);
@@ -1484,6 +1488,9 @@ static int mtk_hw_deinit(struct mtk_eth *eth)
clk_disable_unprepare(eth->clks[MTK_CLK_ESW]);
clk_disable_unprepare(eth->clks[MTK_CLK_ETHIF]);
+ pm_runtime_put_sync(eth->dev);
+ pm_runtime_disable(eth->dev);
+
return 0;
}
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v2 1/7] net: ethernet: mediatek: refactoring mtk_hw_init to be reused
From: sean.wang @ 2016-09-14 15:13 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, keyhaede, objelf, Sean Wang
In-Reply-To: <1473866001-9805-1-git-send-email-sean.wang@mediatek.com>
From: Sean Wang <sean.wang@mediatek.com>
the existing mtk_hw_init includes hardware and software
initialization inside so that it is slightly hard to reuse
them for the process of the reset recovery, so some splitting
is made here for keeping hardware initializing relevant thing
and the else such as IRQ registration and MDIO initialization
what are all about to the interface of core driver moved to the
other proper place because they have no needs to register IRQ and
re-initialize structure again during the reset process.
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 62 ++++++++++++++++-------------
1 file changed, 34 insertions(+), 28 deletions(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index 66fd45a..ca46e82 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1415,7 +1415,12 @@ static int mtk_stop(struct net_device *dev)
static int __init mtk_hw_init(struct mtk_eth *eth)
{
- int err, i;
+ int i;
+
+ clk_prepare_enable(eth->clks[MTK_CLK_ETHIF]);
+ clk_prepare_enable(eth->clks[MTK_CLK_ESW]);
+ clk_prepare_enable(eth->clks[MTK_CLK_GP1]);
+ clk_prepare_enable(eth->clks[MTK_CLK_GP2]);
/* reset the frame engine */
reset_control_assert(eth->rstc);
@@ -1441,19 +1446,6 @@ static int __init mtk_hw_init(struct mtk_eth *eth)
/* Enable RX VLan Offloading */
mtk_w32(eth, 1, MTK_CDMP_EG_CTRL);
- err = devm_request_irq(eth->dev, eth->irq[1], mtk_handle_irq_tx, 0,
- dev_name(eth->dev), eth);
- if (err)
- return err;
- err = devm_request_irq(eth->dev, eth->irq[2], mtk_handle_irq_rx, 0,
- dev_name(eth->dev), eth);
- if (err)
- return err;
-
- err = mtk_mdio_init(eth);
- if (err)
- return err;
-
/* disable delay and normal interrupt */
mtk_w32(eth, 0, MTK_QDMA_DELAY_INT);
mtk_w32(eth, 0, MTK_PDMA_DELAY_INT);
@@ -1783,16 +1775,7 @@ static int mtk_add_mac(struct mtk_eth *eth, struct device_node *np)
eth->netdev[id]->features |= MTK_HW_FEATURES;
eth->netdev[id]->ethtool_ops = &mtk_ethtool_ops;
- err = register_netdev(eth->netdev[id]);
- if (err) {
- dev_err(eth->dev, "error bringing up device\n");
- goto free_netdev;
- }
eth->netdev[id]->irq = eth->irq[0];
- netif_info(eth, probe, eth->netdev[id],
- "mediatek frame engine at 0x%08lx, irq %d\n",
- eth->netdev[id]->base_addr, eth->irq[0]);
-
return 0;
free_netdev:
@@ -1862,11 +1845,6 @@ static int mtk_probe(struct platform_device *pdev)
}
}
- clk_prepare_enable(eth->clks[MTK_CLK_ETHIF]);
- clk_prepare_enable(eth->clks[MTK_CLK_ESW]);
- clk_prepare_enable(eth->clks[MTK_CLK_GP1]);
- clk_prepare_enable(eth->clks[MTK_CLK_GP2]);
-
eth->msg_enable = netif_msg_init(mtk_msg_level, MTK_DEFAULT_MSG_ENABLE);
INIT_WORK(ð->pending_work, mtk_pending_work);
@@ -1887,6 +1865,34 @@ static int mtk_probe(struct platform_device *pdev)
goto err_free_dev;
}
+ err = devm_request_irq(eth->dev, eth->irq[1], mtk_handle_irq_tx, 0,
+ dev_name(eth->dev), eth);
+ if (err)
+ goto err_free_dev;
+
+ err = devm_request_irq(eth->dev, eth->irq[2], mtk_handle_irq_rx, 0,
+ dev_name(eth->dev), eth);
+ if (err)
+ goto err_free_dev;
+
+ err = mtk_mdio_init(eth);
+ if (err)
+ goto err_free_dev;
+
+ for (i = 0; i < MTK_MAX_DEVS; i++) {
+ if (!eth->netdev[i])
+ continue;
+
+ err = register_netdev(eth->netdev[i]);
+ if (err) {
+ dev_err(eth->dev, "error bringing up device\n");
+ goto err_free_dev;
+ } else
+ netif_info(eth, probe, eth->netdev[i],
+ "mediatek frame engine at 0x%08lx, irq %d\n",
+ eth->netdev[i]->base_addr, eth->irq[0]);
+ }
+
/* we run 2 devices on the same DMA ring so we need a dummy device
* for NAPI to work
*/
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v2 6/7] net: ethernet: mediatek: add more resets for internal ethernet circuit block
From: sean.wang @ 2016-09-14 15:13 UTC (permalink / raw)
To: john, davem; +Cc: nbd, netdev, linux-mediatek, keyhaede, objelf, Sean Wang
In-Reply-To: <1473866001-9805-1-git-send-email-sean.wang@mediatek.com>
From: Sean Wang <sean.wang@mediatek.com>
struct mtk_eth has already contained struct regmap ethsys pointer
to the address range of the internal circuit reset, so we reuse it
to reset more internal blocks on ethernet hardware such as packet
processing engine (PPE) and frame engine (FE) instead of rstc which
deals with FE only.
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 27 +++++++++++++++------------
drivers/net/ethernet/mediatek/mtk_eth_soc.h | 6 +++++-
2 files changed, 20 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index a7e2e77..cde9cd6 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1414,6 +1414,19 @@ static int mtk_stop(struct net_device *dev)
return 0;
}
+static void ethsys_reset(struct mtk_eth *eth, u32 reset_bits)
+{
+ regmap_update_bits(eth->ethsys, ETHSYS_RSTCTRL,
+ reset_bits,
+ reset_bits);
+
+ usleep_range(1000, 1100);
+ regmap_update_bits(eth->ethsys, ETHSYS_RSTCTRL,
+ reset_bits,
+ ~reset_bits);
+ mdelay(10);
+}
+
static int mtk_hw_init(struct mtk_eth *eth)
{
int i, val;
@@ -1428,12 +1441,8 @@ static int mtk_hw_init(struct mtk_eth *eth)
clk_prepare_enable(eth->clks[MTK_CLK_ESW]);
clk_prepare_enable(eth->clks[MTK_CLK_GP1]);
clk_prepare_enable(eth->clks[MTK_CLK_GP2]);
-
- /* reset the frame engine */
- reset_control_assert(eth->rstc);
- usleep_range(10, 20);
- reset_control_deassert(eth->rstc);
- usleep_range(10, 20);
+ ethsys_reset(eth, RSTCTRL_FE);
+ ethsys_reset(eth, RSTCTRL_PPE);
regmap_read(eth->ethsys, ETHSYS_SYSCFG0, &val);
for (i = 0; i < MTK_MAC_COUNT; i++) {
@@ -1893,12 +1902,6 @@ static int mtk_probe(struct platform_device *pdev)
return PTR_ERR(eth->pctl);
}
- eth->rstc = devm_reset_control_get(&pdev->dev, "eth");
- if (IS_ERR(eth->rstc)) {
- dev_err(&pdev->dev, "no eth reset found\n");
- return PTR_ERR(eth->rstc);
- }
-
for (i = 0; i < 3; i++) {
eth->irq[i] = platform_get_irq(pdev, i);
if (eth->irq[i] < 0) {
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.h b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
index 388cbe7..7efa00f 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.h
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.h
@@ -266,6 +266,11 @@
#define SYSCFG0_GE_MASK 0x3
#define SYSCFG0_GE_MODE(x, y) (x << (12 + (y * 2)))
+/*ethernet reset control register*/
+#define ETHSYS_RSTCTRL 0x34
+#define RSTCTRL_FE BIT(6)
+#define RSTCTRL_PPE BIT(31)
+
struct mtk_rx_dma {
unsigned int rxd1;
unsigned int rxd2;
@@ -423,7 +428,6 @@ struct mtk_rx_ring {
struct mtk_eth {
struct device *dev;
void __iomem *base;
- struct reset_control *rstc;
spinlock_t page_lock;
spinlock_t irq_lock;
struct net_device dummy_dev;
--
1.9.1
^ permalink raw reply related
* [PATCH net-next v2 3/7] net: ethernet: mediatek: cleanup error path inside mtk_hw_init
From: sean.wang-NuS5LvNUpcJWk0Htik3J/w @ 2016-09-14 15:13 UTC (permalink / raw)
To: john-Pj+rj9U5foFAfugRpC6u6w, davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: nbd-p3rKhJxN3npAfugRpC6u6w, keyhaede-Re5JQEeQqe8AvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA, Sean Wang,
linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
objelf-Re5JQEeQqe8AvxtiuMwx3w
In-Reply-To: <1473866001-9805-1-git-send-email-sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
From: Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
This cleans up the error path inside mtk_hw_init call, causing it able
to exit appropriately when something fails and also includes refactoring
mtk_cleanup call to make the partial logic reusable on the error path.
Signed-off-by: Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
drivers/net/ethernet/mediatek/mtk_eth_soc.c | 34 ++++++++++++++++++++++++-----
1 file changed, 29 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
index c71b0b3..917a49c6 100644
--- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
+++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
@@ -1564,17 +1564,36 @@ static void mtk_pending_work(struct work_struct *work)
rtnl_unlock();
}
-static int mtk_cleanup(struct mtk_eth *eth)
+static int mtk_free_dev(struct mtk_eth *eth)
{
int i;
for (i = 0; i < MTK_MAC_COUNT; i++) {
if (!eth->netdev[i])
continue;
+ free_netdev(eth->netdev[i]);
+ }
+
+ return 0;
+}
+static int mtk_unreg_dev(struct mtk_eth *eth)
+{
+ int i;
+
+ for (i = 0; i < MTK_MAC_COUNT; i++) {
+ if (!eth->netdev[i])
+ continue;
unregister_netdev(eth->netdev[i]);
- free_netdev(eth->netdev[i]);
}
+
+ return 0;
+}
+
+static int mtk_cleanup(struct mtk_eth *eth)
+{
+ mtk_unreg_dev(eth);
+ mtk_free_dev(eth);
cancel_work_sync(ð->pending_work);
return 0;
@@ -1872,7 +1891,7 @@ static int mtk_probe(struct platform_device *pdev)
err = mtk_add_mac(eth, mac_np);
if (err)
- goto err_free_dev;
+ goto err_deinit_hw;
}
err = devm_request_irq(eth->dev, eth->irq[1], mtk_handle_irq_tx, 0,
@@ -1896,7 +1915,7 @@ static int mtk_probe(struct platform_device *pdev)
err = register_netdev(eth->netdev[i]);
if (err) {
dev_err(eth->dev, "error bringing up device\n");
- goto err_free_dev;
+ goto err_deinit_mdio;
} else
netif_info(eth, probe, eth->netdev[i],
"mediatek frame engine at 0x%08lx, irq %d\n",
@@ -1916,8 +1935,13 @@ static int mtk_probe(struct platform_device *pdev)
return 0;
+err_deinit_mdio:
+ mtk_mdio_cleanup(eth);
err_free_dev:
- mtk_cleanup(eth);
+ mtk_free_dev(eth);
+err_deinit_hw:
+ mtk_hw_deinit(eth);
+
return err;
}
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] net/mlx4_en: fix off by one in error handling
From: Tariq Toukan @ 2016-09-14 14:49 UTC (permalink / raw)
To: Sebastian Ott
Cc: Yishai Hadas, Tariq Toukan, netdev, linux-rdma, linux-kernel
In-Reply-To: <alpine.LFD.2.20.1609141548390.11022@schleppi>
On 14/09/2016 4:53 PM, Sebastian Ott wrote:
> Hello Tariq,
>
> On Wed, 14 Sep 2016, Tariq Toukan wrote:
>> On 14/09/2016 2:09 PM, Sebastian Ott wrote:
>>> If an error occurs in mlx4_init_eq_table the index used in the
>>> err_out_unmap label is one too big which results in a panic in
>>> mlx4_free_eq. This patch fixes the index in the error path.
>> You are right, but your change below does not cover all cases.
>> The full solution looks like this:
>>
>> @@ -1260,7 +1260,7 @@ int mlx4_init_eq_table(struct mlx4_dev *dev)
>> eq);
>> }
>> if (err)
>> - goto err_out_unmap;
>> + goto err_out_unmap_excluded;
> In this case a call to mlx4_create_eq failed. Do you really have to call
> mlx4_free_eq for this index again?
We agree on this part, that's why here we should goto the _excluded_ label.
For all other parts, we should not exclude the eq in the highest index,
and thus we goto the _non_excluded_ label.
> As far as I understood this code
> mlx4_create_eq cleans up when it fails and thus there is no need for an
> additional mlx4_free_eq call.
>
> Regards,
> Sebastian
>
Regards,
Tariq
^ permalink raw reply
* RE: [RFC 00/11] QLogic RDMA Driver (qedr) RFC
From: Amrani, Ram @ 2016-09-14 14:44 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, David Miller,
Yuval Mintz, Ariel Elior, Michal Kalderon, Rajesh Borundia,
linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, netdev
In-Reply-To: <20160913154000.GA25878-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/@public.gmane.org>
> Anything that is used with copy_to/from_user, ib_copy_to/from_udata,
> etc, etc must be in a include/uapi header.
>
> Any constant you might want to copy into your userspace provider must
> be in include/uapi.
>
I understand you mean something like https://www.spinics.net/lists/linux-rdma/msg40414.html ([RFC rdma-next 0/3] Move mlx5 vendor specific to UAPI directory)
> Avoid copying headers in your user space and use the standard kernel
> names to access your driver's uapi.
So the user library should be able to compile against these headers in their location and not hold identical copies.
What do you mean by "standard kernel names"?
Thanks,
Ram
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [net v1] fib_rules: interface group matching
From: David Ahern @ 2016-09-14 14:39 UTC (permalink / raw)
To: Vincent Bernat; +Cc: David S. Miller, Nicolas Dichtel, Wilson Kok, netdev
In-Reply-To: <87h99ipnhu.fsf@zoro.exoscale.ch>
On 9/14/16 8:25 AM, Vincent Bernat wrote:
> ❦ 14 septembre 2016 16:15 CEST, David Ahern <dsa@cumulusnetworks.com> :
>
>>> When a user wants to assign a routing table to a group of incoming
>>> interfaces, the current solutions are:
>>>
>>> - one IP rule for each interface (scalability problems)
>>> - use of fwmark and devgroup matcher (don't work with internal route
>>> lookups, used for example by RPF)
>>> - use of VRF devices (more complex)
>>
>> Why do you believe that? A VRF is a formalized grouping of interfaces
>> that includes an API for locally generated traffic to specify which
>> VRF/group to use. And, with the l3mdev rule you only need 1 rule for
>> all VRFs regardless of the number which is the best solution to the
>> scalability problem of adding rules per device/group/VRF.
>>
>> What use case are trying to solve?
>
> Local processes have to be made aware of the VRF by binding to the
> pseudo-device. Some processes may be tricked by LD_PRELOAD but some
> won't (like stuff written in Go). Maybe I should just find a better way
> to bind a process to a VRF without its cooperation.
>
What API are you using for interface groups? How does an app tell the kernel to use interface group 1 versus group 2?
LD_PRELOAD and overloading socket is an ad-hoc hack at best with many holes - as you have found.
We (Cumulus Linux) are using this cgroups patch:
http://www.mail-archive.com/netdev@vger.kernel.org/msg93408.html
I want something formal like the cgroups patch or even the first idea of adding a default sk_bound_dev_if to the task struct:
https://github.com/dsahern/linux/commit/b3e5ccc291505c8a503edb20ea2c2b5e86bed96f
Parent-to-child inheritance of the setting is a requirement as is the setting getting applied to all IPv4/v6 sockets without action by the process itself.
Still have some work to do to get a solution into the kernel.
^ permalink raw reply
* RE: [RFC v3 00/22] Landlock LSM: Unprivileged sandboxing
From: David Laight @ 2016-09-14 14:36 UTC (permalink / raw)
To: 'Mickaël Salaün', linux-kernel@vger.kernel.org
Cc: Alexei Starovoitov, Andy Lutomirski, Arnd Bergmann,
Casey Schaufler, Daniel Borkmann, Daniel Mack, David Drysdale,
David S . Miller, Elena Reshetova, Eric W . Biederman,
James Morris, Kees Cook, Paul Moore, Sargun Dhillon,
Serge E . Hallyn, Tejun Heo, Will Drewry,
kernel-hardening@lists.openwall.com,
"linux-api@vger.kernel.org" <linux-api@
In-Reply-To: <20160914072415.26021-1-mic@digikod.net>
From: Mickaël Salaün
> Sent: 14 September 2016 08:24
...
> ## Why does seccomp-filter is not enough?
>
> A seccomp filter can access to raw syscall arguments which means that it is not
> possible to filter according to pointed data as a file path. As demonstrated
> the first version of this patch series, filtering at the syscall level is
> complicated (e.g. need to take care of race conditions). This is mainly because
> the access control checkpoints of the kernel are not at this high-level but
> more underneath, at LSM hooks level. The LSM hooks are designed to handle this
> kind of checks. This series use this approach to leverage the ability of
> unprivileged users to limit themselves.
You cannot validate file path parameters during syscall entry.
It can only be done after the user buffer has been read into kernel memory.
(ie you must only access the buffer once.)
This has nothing to do with where the kernel does any access checks,
and everything to do with the fact that another thread/process can
modify the buffer after you have validated it.
David
^ permalink raw reply
* Re: [net v1] fib_rules: interface group matching
From: Vincent Bernat @ 2016-09-14 14:25 UTC (permalink / raw)
To: David Ahern; +Cc: David S. Miller, Nicolas Dichtel, Wilson Kok, netdev
In-Reply-To: <9540c014-78c5-9f9c-16d7-75a564f6c018@cumulusnetworks.com>
❦ 14 septembre 2016 16:15 CEST, David Ahern <dsa@cumulusnetworks.com> :
>> When a user wants to assign a routing table to a group of incoming
>> interfaces, the current solutions are:
>>
>> - one IP rule for each interface (scalability problems)
>> - use of fwmark and devgroup matcher (don't work with internal route
>> lookups, used for example by RPF)
>> - use of VRF devices (more complex)
>
> Why do you believe that? A VRF is a formalized grouping of interfaces
> that includes an API for locally generated traffic to specify which
> VRF/group to use. And, with the l3mdev rule you only need 1 rule for
> all VRFs regardless of the number which is the best solution to the
> scalability problem of adding rules per device/group/VRF.
>
> What use case are trying to solve?
Local processes have to be made aware of the VRF by binding to the
pseudo-device. Some processes may be tricked by LD_PRELOAD but some
won't (like stuff written in Go). Maybe I should just find a better way
to bind a process to a VRF without its cooperation.
--
Instrument your programs. Measure before making "efficiency" changes.
- The Elements of Programming Style (Kernighan & Plauger)
^ permalink raw reply
* Re: [PATCH 8/9] net: ethernet: ti: cpts: fix overflow check period
From: Richard Cochran @ 2016-09-14 14:25 UTC (permalink / raw)
To: Grygorii Strashko
Cc: David S. Miller, netdev, Mugunthan V N, Sekhar Nori, linux-kernel,
linux-omap, WingMan Kwok
In-Reply-To: <20160914130231.3035-9-grygorii.strashko@ti.com>
On Wed, Sep 14, 2016 at 04:02:30PM +0300, Grygorii Strashko wrote:
> @@ -427,9 +427,6 @@ static void cpts_calc_mult_shift(struct cpts *cpts)
> u64 ns;
> u64 frac;
>
> - if (cpts->cc_mult || cpts->cc.shift)
> - return;
> -
> freq = clk_get_rate(cpts->refclk);
>
> /* Calc the maximum number of seconds which we can run before
This hunk has nothing to do with $subject.
> @@ -442,11 +439,20 @@ static void cpts_calc_mult_shift(struct cpts *cpts)
> else if (maxsec > 600 && cpts->cc.mask > UINT_MAX)
> maxsec = 600;
>
> + /* Calc overflow check period (maxsec / 2) */
> + cpts->ov_check_period = (HZ * maxsec) / 2;
> + dev_info(cpts->dev, "cpts: overflow check period %lu\n",
> + cpts->ov_check_period);
> +
> + if (cpts->cc_mult || cpts->cc.shift)
> + return;
> +
> clocks_calc_mult_shift(&mult, &shift, freq, NSEC_PER_SEC, maxsec);
>
> cpts->cc_mult = mult;
> cpts->cc.mult = mult;
> cpts->cc.shift = shift;
> +
Nor does this.
Thanks,
Richard
> /* Check calculations and inform if not precise */
> frac = 0;
> ns = cyclecounter_cyc2ns(&cpts->cc, freq, cpts->cc.mask, &frac);
> diff --git a/drivers/net/ethernet/ti/cpts.h b/drivers/net/ethernet/ti/cpts.h
> index 47026ec..e0e4a62b 100644
> --- a/drivers/net/ethernet/ti/cpts.h
> +++ b/drivers/net/ethernet/ti/cpts.h
> @@ -97,9 +97,6 @@ enum {
> CPTS_EV_TX, /* Ethernet Transmit Event */
> };
>
> -/* This covers any input clock up to about 500 MHz. */
> -#define CPTS_OVERFLOW_PERIOD (HZ * 8)
> -
> #define CPTS_FIFO_DEPTH 16
> #define CPTS_MAX_EVENTS 32
>
> @@ -127,6 +124,7 @@ struct cpts {
> struct list_head events;
> struct list_head pool;
> struct cpts_event pool_data[CPTS_MAX_EVENTS];
> + unsigned long ov_check_period;
> };
>
> int cpts_rx_timestamp(struct cpts *cpts, struct sk_buff *skb);
> --
> 2.9.3
>
^ permalink raw reply
* Re: [PATCH 7/9] net: ethernet: ti: cpts: calc mult and shift from refclk freq
From: Richard Cochran @ 2016-09-14 14:22 UTC (permalink / raw)
To: Grygorii Strashko
Cc: David S. Miller, netdev, Mugunthan V N, Sekhar Nori, linux-kernel,
linux-omap, WingMan Kwok
In-Reply-To: <20160914130231.3035-8-grygorii.strashko@ti.com>
On Wed, Sep 14, 2016 at 04:02:29PM +0300, Grygorii Strashko wrote:
> @@ -35,6 +33,8 @@ Optional properties:
> For example in dra72x-evm, pcf gpio has to be
> driven low so that cpsw slave 0 and phy data
> lines are connected via mux.
> +- cpts_clock_mult : Numerator to convert input clock ticks into nanoseconds
> +- cpts_clock_shift : Denominator to convert input clock ticks into nanoseconds
You should explain to the reader how these will be calculated when the
properties are missing.
> diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
> index ff8bb85..8046a21 100644
> --- a/drivers/net/ethernet/ti/cpts.c
> +++ b/drivers/net/ethernet/ti/cpts.c
> @@ -418,18 +418,60 @@ void cpts_unregister(struct cpts *cpts)
> clk_disable(cpts->refclk);
> }
>
> +static void cpts_calc_mult_shift(struct cpts *cpts)
> +{
> + u64 maxsec;
> + u32 freq;
> + u32 mult;
> + u32 shift;
> + u64 ns;
> + u64 frac;
Why so many new lines? This isn't good style. Please combine
variables of the same type into one line and sort the lists
alphabetically.
> + if (cpts->cc_mult || cpts->cc.shift)
> + return;
> +
> + freq = clk_get_rate(cpts->refclk);
> +
> + /* Calc the maximum number of seconds which we can run before
> + * wrapping around.
> + */
> + maxsec = cpts->cc.mask;
> + do_div(maxsec, freq);
> + if (!maxsec)
> + maxsec = 1;
> + else if (maxsec > 600 && cpts->cc.mask > UINT_MAX)
> + maxsec = 600;
> +
> + clocks_calc_mult_shift(&mult, &shift, freq, NSEC_PER_SEC, maxsec);
> +
> + cpts->cc_mult = mult;
> + cpts->cc.mult = mult;
> + cpts->cc.shift = shift;
> + /* Check calculations and inform if not precise */
Contrary to this comment, you are not making any kind of judgment
about whether the calculations are precise or not.
> + frac = 0;
> + ns = cyclecounter_cyc2ns(&cpts->cc, freq, cpts->cc.mask, &frac);
> +
> + dev_info(cpts->dev,
> + "CPTS: ref_clk_freq:%u calc_mult:%u calc_shift:%u error:%lld nsec/sec\n",
> + freq, cpts->cc_mult, cpts->cc.shift, (ns - NSEC_PER_SEC));
> +}
> +
Thanks,
Richard
^ permalink raw reply
* Re: [net v1] fib_rules: interface group matching
From: David Ahern @ 2016-09-14 14:15 UTC (permalink / raw)
To: Vincent Bernat, David S. Miller, Nicolas Dichtel, Wilson Kok,
netdev
In-Reply-To: <20160914124025.13417-1-vincent@bernat.im>
On 9/14/16 6:40 AM, Vincent Bernat wrote:
> When a user wants to assign a routing table to a group of incoming
> interfaces, the current solutions are:
>
> - one IP rule for each interface (scalability problems)
> - use of fwmark and devgroup matcher (don't work with internal route
> lookups, used for example by RPF)
> - use of VRF devices (more complex)
Why do you believe that? A VRF is a formalized grouping of interfaces that includes an API for locally generated traffic to specify which VRF/group to use. And, with the l3mdev rule you only need 1 rule for all VRFs regardless of the number which is the best solution to the scalability problem of adding rules per device/group/VRF.
What use case are trying to solve?
^ permalink raw reply
* Re: [PATCH 6/9] net: ethernet: ti: cpts: clean up event list if event pool is empty
From: Richard Cochran @ 2016-09-14 14:14 UTC (permalink / raw)
To: Grygorii Strashko
Cc: David S. Miller, netdev, Mugunthan V N, Sekhar Nori, linux-kernel,
linux-omap, WingMan Kwok
In-Reply-To: <20160914130231.3035-7-grygorii.strashko@ti.com>
On Wed, Sep 14, 2016 at 04:02:28PM +0300, Grygorii Strashko wrote:
> From: WingMan Kwok <w-kwok2@ti.com>
>
> When a CPTS user does not exit gracefully by disabling cpts
> timestamping and leaving a joined multicast group, the system
> continues to receive and timestamps the ptp packets which eventually
> occupy all the event list entries. When this happns, the added code
> tries to remove some list entries which are expired.
>
> Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
> drivers/net/ethernet/ti/cpts.c | 30 ++++++++++++++++++++++++++++--
> 1 file changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
> index 970d4e2..ff8bb85 100644
> --- a/drivers/net/ethernet/ti/cpts.c
> +++ b/drivers/net/ethernet/ti/cpts.c
> @@ -57,22 +57,48 @@ static int cpts_fifo_pop(struct cpts *cpts, u32 *high, u32 *low)
> return -1;
> }
>
> +static int cpts_event_list_clean_up(struct cpts *cpts)
5 words, that is quite a mouth full. How about this instead?
static int cpts_purge_events(struct cpts *cpts);
> +{
> + struct list_head *this, *next;
> + struct cpts_event *event;
> + int removed = 0;
> +
> + list_for_each_safe(this, next, &cpts->events) {
> + event = list_entry(this, struct cpts_event, list);
> + if (event_expired(event)) {
> + list_del_init(&event->list);
> + list_add(&event->list, &cpts->pool);
> + ++removed;
> + }
> + }
> + return removed;
> +}
> +
> /*
> * Returns zero if matching event type was found.
> */
> static int cpts_fifo_read(struct cpts *cpts, int match)
> {
> int i, type = -1;
> + int removed;
No need for another variable, just change the return code above to
return removed ? 0 : -1;
and then you have ...
> u32 hi, lo;
> struct cpts_event *event;
>
> for (i = 0; i < CPTS_FIFO_DEPTH; i++) {
> if (cpts_fifo_pop(cpts, &hi, &lo))
> break;
> +
> if (list_empty(&cpts->pool)) {
> - pr_err("cpts: event pool is empty\n");
> - return -1;
> + removed = cpts_event_list_clean_up(cpts);
> + if (!removed) {
> + dev_err(cpts->dev,
> + "cpts: event pool is empty\n");
> + return -1;
> + }
if (cpts_purge_events(cpts)) {
dev_err(cpts->dev, "cpts: event pool empty\n");
return -1;
}
Notice how I avoided the ugly line break?
> + dev_dbg(cpts->dev,
> + "cpts: event pool cleaned up %d\n", removed);
> }
> +
> event = list_first_entry(&cpts->pool, struct cpts_event, list);
> event->tmo = jiffies + 2;
> event->high = hi;
> --
> 2.9.3
>
Thanks,
Richard
^ permalink raw reply
* Re: [RFC PATCH 9/9] ethernet: sun8i-emac: add pm_runtime support
From: LABBE Corentin @ 2016-09-14 14:03 UTC (permalink / raw)
To: Maxime Ripard
Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
wens-jdAy2FN1RRM, linux-I+IVW8TIWO2tmTQ+vhA3Yw,
davem-fT/PcQaiUtIeIZ0/mPfg9Q, netdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20160912204451.GO9449@lukather>
On Mon, Sep 12, 2016 at 10:44:51PM +0200, Maxime Ripard wrote:
> Hi,
>
> On Fri, Sep 09, 2016 at 02:45:17PM +0200, Corentin Labbe wrote:
> > This patch add pm_runtime support to sun8i-emac.
> > For the moment, only basic support is added, (the device is marked as
> > used when net/open)
> >
> > Signed-off-by: Corentin Labbe <clabbe.montjoie-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > ---
> > drivers/net/ethernet/allwinner/sun8i-emac.c | 62 ++++++++++++++++++++++++++++-
> > 1 file changed, 60 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/allwinner/sun8i-emac.c b/drivers/net/ethernet/allwinner/sun8i-emac.c
> > index 1c4bc80..cce886e 100644
> > --- a/drivers/net/ethernet/allwinner/sun8i-emac.c
> > +++ b/drivers/net/ethernet/allwinner/sun8i-emac.c
> > @@ -9,7 +9,6 @@
> > * - MAC filtering
> > * - Jumbo frame
> > * - features rx-all (NETIF_F_RXALL_BIT)
> > - * - PM runtime
> > */
> > #include <linux/bitops.h>
> > #include <linux/clk.h>
> > @@ -27,6 +26,7 @@
> > #include <linux/pinctrl/consumer.h>
> > #include <linux/pinctrl/pinctrl.h>
> > #include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
> > #include <linux/reset.h>
> > #include <linux/scatterlist.h>
> > #include <linux/skbuff.h>
> > @@ -1301,11 +1301,18 @@ static int sun8i_emac_open(struct net_device *ndev)
> > int err;
> > u32 v;
> >
> > + err = pm_runtime_get_sync(priv->dev);
> > + if (err) {
> > + pm_runtime_put_noidle(priv->dev);
> > + dev_err(priv->dev, "pm_runtime error: %d\n", err);
> > + return err;
> > + }
> > +
> > err = request_irq(priv->irq, sun8i_emac_dma_interrupt, 0,
> > dev_name(priv->dev), ndev);
> > if (err) {
> > dev_err(priv->dev, "Cannot request IRQ: %d\n", err);
> > - return err;
> > + goto err_runtime;
> > }
> >
> > /* Set interface mode (and configure internal PHY on H3) */
> > @@ -1395,6 +1402,8 @@ err_syscon:
> > sun8i_emac_unset_syscon(ndev);
> > err_irq:
> > free_irq(priv->irq, ndev);
> > +err_runtime:
> > + pm_runtime_put(priv->dev);
> > return err;
> > }
> >
> > @@ -1483,6 +1492,8 @@ static int sun8i_emac_stop(struct net_device *ndev)
> > dma_free_coherent(priv->dev, priv->nbdesc_tx * sizeof(struct dma_desc),
> > priv->dd_tx, priv->dd_tx_phy);
> >
> > + pm_runtime_put(priv->dev);
> > +
> > return 0;
> > }
> >
> > @@ -2210,6 +2221,8 @@ static int sun8i_emac_probe(struct platform_device *pdev)
> > goto probe_err;
> > }
> >
> > + pm_runtime_enable(priv->dev);
> > +
> > return 0;
> >
> > probe_err:
> > @@ -2221,6 +2234,8 @@ static int sun8i_emac_remove(struct platform_device *pdev)
> > {
> > struct net_device *ndev = platform_get_drvdata(pdev);
> >
> > + pm_runtime_disable(&pdev->dev);
> > +
> > unregister_netdev(ndev);
> > platform_set_drvdata(pdev, NULL);
> > free_netdev(ndev);
> > @@ -2228,6 +2243,47 @@ static int sun8i_emac_remove(struct platform_device *pdev)
> > return 0;
> > }
> >
> > +static int __maybe_unused sun8i_emac_suspend(struct platform_device *pdev, pm_message_t state)
> > +{
> > + struct net_device *ndev = platform_get_drvdata(pdev);
> > + struct sun8i_emac_priv *priv = netdev_priv(ndev);
> > +
> > + napi_disable(&priv->napi);
> > +
> > + if (netif_running(ndev))
> > + netif_device_detach(ndev);
> > +
> > + sun8i_emac_stop_tx(ndev);
> > + sun8i_emac_stop_rx(ndev);
> > +
> > + sun8i_emac_rx_clean(ndev);
> > + sun8i_emac_tx_clean(ndev);
> > +
> > + phy_stop(ndev->phydev);
> > +
> > + return 0;
> > +}
> > +
> > +static int __maybe_unused sun8i_emac_resume(struct platform_device *pdev)
> > +{
> > + struct net_device *ndev = platform_get_drvdata(pdev);
> > + struct sun8i_emac_priv *priv = netdev_priv(ndev);
> > +
> > + phy_start(ndev->phydev);
> > +
> > + sun8i_emac_start_tx(ndev);
> > + sun8i_emac_start_rx(ndev);
> > +
> > + if (netif_running(ndev))
> > + netif_device_attach(ndev);
> > +
> > + netif_start_queue(ndev);
> > +
> > + napi_enable(&priv->napi);
> > +
> > + return 0;
> > +}
>
> The main idea behind the runtime PM hooks is that they bring the
> device to a working state and shuts it down when it's not needed
> anymore.
>
I expect that the first part (all pm_runtime_xxx) of the patch bring that.
When the interface is not opened:
cat /sys/devices/platform/soc/1c30000.ethernet/power/runtime_status
suspended
> However, they shouldn't be called when the device is still in used, so
> all the mangling with NAPI, the phy and so on is irrelevant here, but
> the clocks, resets, for example, are.
>
I do the same as other ethernet driver for suspend/resume.
> > static const struct of_device_id sun8i_emac_of_match_table[] = {
> > { .compatible = "allwinner,sun8i-a83t-emac",
> > .data = &emac_variant_a83t },
> > @@ -2246,6 +2302,8 @@ static struct platform_driver sun8i_emac_driver = {
> > .name = "sun8i-emac",
> > .of_match_table = sun8i_emac_of_match_table,
> > },
> > + .suspend = sun8i_emac_suspend,
> > + .resume = sun8i_emac_resume,
>
> These are not the runtime PM hooks. How did you test that?
>
Anyway I didnt test suspend/resume so I will remove it until I successfully found how to hibernate my board.
Thanks
Regards
Corentin Labbe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 5/9] net: ethernet: ti: cpts: add return value to tx and rx timestamp funcitons
From: Richard Cochran @ 2016-09-14 14:00 UTC (permalink / raw)
To: Grygorii Strashko
Cc: David S. Miller, netdev, Mugunthan V N, Sekhar Nori, linux-kernel,
linux-omap, WingMan Kwok
In-Reply-To: <20160914130231.3035-6-grygorii.strashko@ti.com>
On Wed, Sep 14, 2016 at 04:02:27PM +0300, Grygorii Strashko wrote:
> From: WingMan Kwok <w-kwok2@ti.com>
>
> Added return values in tx and rx timestamp funcitons facilitate the
> possibililies of timestamping by CPSW modules other than CPTS, such as
> packet accelerator on Keystone 2 devices.
I'm sorry, this is totally bogus.
> Signed-off-by: WingMan Kwok <w-kwok2@ti.com>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
> drivers/net/ethernet/ti/cpts.c | 16 ++++++++++------
> drivers/net/ethernet/ti/cpts.h | 11 +++++++----
> 2 files changed, 17 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/cpts.c b/drivers/net/ethernet/ti/cpts.c
> index 1ee64c6..970d4e2 100644
> --- a/drivers/net/ethernet/ti/cpts.c
> +++ b/drivers/net/ethernet/ti/cpts.c
> @@ -302,34 +302,38 @@ static u64 cpts_find_ts(struct cpts *cpts, struct sk_buff *skb, int ev_type)
> return ns;
> }
>
> -void cpts_rx_timestamp(struct cpts *cpts, struct sk_buff *skb)
> +int cpts_rx_timestamp(struct cpts *cpts, struct sk_buff *skb)
> {
> u64 ns;
> struct skb_shared_hwtstamps *ssh;
>
> if (!cpts || !cpts->rx_enable)
> - return;
> + return -EPERM;
EPERM? Come on.
> ns = cpts_find_ts(cpts, skb, CPTS_EV_RX);
> if (!ns)
> - return;
> + return -ENOENT;
> ssh = skb_hwtstamps(skb);
> memset(ssh, 0, sizeof(*ssh));
> ssh->hwtstamp = ns_to_ktime(ns);
> +
> + return 0;
> }
>
> -void cpts_tx_timestamp(struct cpts *cpts, struct sk_buff *skb)
> +int cpts_tx_timestamp(struct cpts *cpts, struct sk_buff *skb)
> {
> u64 ns;
> struct skb_shared_hwtstamps ssh;
>
> if (!cpts || !(skb_shinfo(skb)->tx_flags & SKBTX_IN_PROGRESS))
> - return;
> + return -EPERM;
> ns = cpts_find_ts(cpts, skb, CPTS_EV_TX);
> if (!ns)
> - return;
> + return -ENOENT;
> memset(&ssh, 0, sizeof(ssh));
> ssh.hwtstamp = ns_to_ktime(ns);
> skb_tstamp_tx(skb, &ssh);
> +
> + return 0;
> }
>
> int cpts_register(struct cpts *cpts)
> diff --git a/drivers/net/ethernet/ti/cpts.h b/drivers/net/ethernet/ti/cpts.h
> index a865193..47026ec 100644
> --- a/drivers/net/ethernet/ti/cpts.h
> +++ b/drivers/net/ethernet/ti/cpts.h
> @@ -129,8 +129,8 @@ struct cpts {
> struct cpts_event pool_data[CPTS_MAX_EVENTS];
> };
>
> -void cpts_rx_timestamp(struct cpts *cpts, struct sk_buff *skb);
> -void cpts_tx_timestamp(struct cpts *cpts, struct sk_buff *skb);
> +int cpts_rx_timestamp(struct cpts *cpts, struct sk_buff *skb);
> +int cpts_tx_timestamp(struct cpts *cpts, struct sk_buff *skb);
> int cpts_register(struct cpts *cpts);
> void cpts_unregister(struct cpts *cpts);
> struct cpts *cpts_create(struct device *dev, void __iomem *regs,
> @@ -162,11 +162,14 @@ static inline bool cpts_is_tx_enabled(struct cpts *cpts)
> #else
> struct cpts;
>
> -static inline void cpts_rx_timestamp(struct cpts *cpts, struct sk_buff *skb)
> +static inline int cpts_rx_timestamp(struct cpts *cpts, struct sk_buff *skb)
> {
> + return -EOPNOTSUPP;
You are planning to check in the hot path if a compile time feature is
enabled?
Brilliant stuff.
> }
> -static inline void cpts_tx_timestamp(struct cpts *cpts, struct sk_buff *skb)
> +
> +static inline int cpts_tx_timestamp(struct cpts *cpts, struct sk_buff *skb)
> {
> + return -EOPNOTSUPP;
> }
>
> static inline
> --
> 2.9.3
>
Thanks,
Richard
^ permalink raw reply
* Re: [PATCH 4/9] net: ethernet: ti: cpts: move dt props parsing to cpts driver
From: Richard Cochran @ 2016-09-14 13:55 UTC (permalink / raw)
To: Grygorii Strashko
Cc: David S. Miller, netdev, Mugunthan V N, Sekhar Nori, linux-kernel,
linux-omap, WingMan Kwok
In-Reply-To: <20160914130231.3035-5-grygorii.strashko@ti.com>
On Wed, Sep 14, 2016 at 04:02:26PM +0300, Grygorii Strashko wrote:
> Move DT properties parsing into CPTS driver to simplify consumer's
> code and CPTS driver porting on other SoC in the future
> (like Keystone 2).
And just who is the consumer?
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
> ---
> drivers/net/ethernet/ti/cpsw.c | 16 +---------------
> drivers/net/ethernet/ti/cpsw.h | 2 --
> drivers/net/ethernet/ti/cpts.c | 29 ++++++++++++++++++++++++++---
> drivers/net/ethernet/ti/cpts.h | 5 +++--
> 4 files changed, 30 insertions(+), 22 deletions(-)
You have more (+) than (-). I wouldn't call that a simplification.
Thanks,
Richard
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox