Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v4 2/5] net: ethernet: cpsw: add optional third memory region for CONTROL module
From: Daniel Mack @ 2013-08-23 16:21 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: netdev, bcousson, nsekhar, davem, ujhelyi.m, mugunthanvnm,
	vaibhav.bedia, d-gerlach, linux-arm-kernel, linux-omap,
	devicetree
In-Reply-To: <521778DB.7040402@cogentembedded.com>

On 23.08.2013 16:59, Sergei Shtylyov wrote:
> On 23-08-2013 18:16, Daniel Mack wrote:

>> +	priv->gmii_sel_reg = devm_ioremap_resource(&pdev->dev, res);
>> +	if (IS_ERR(priv->gmii_sel_reg)) {
>> +		dev_err(priv->dev, "unable to map control i/o region\n");
> 
>     You didn't actually seem to heed my words about error message.

Well yes I did, but only in the check for platform_get_resource(). As
the comment says - we pass on if that memory region is not given, but if
it is given, it also has to be valid.

> And don't  you want to do:
> 
> 	res = PTR_ERR(priv->gmii_sel_reg);


Erm, of course. Sorry for that.


Daniel

^ permalink raw reply
* pull request: wireless 2013-08-23
From: John W. Linville @ 2013-08-23 16:20 UTC (permalink / raw)
  To: davem; +Cc: linux-wireless, netdev, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 11214 bytes --]

Dave,

This is one more set of fixes intended for the 3.11 stream...

For the mac80211 bits, Johannes says:

"I have three more patches for the 3.11 stream: Felix's fix for the
fairly visible brcmsmac crash, a fix from Simon for an IBSS join bug I
found and a fix for a channel context bug in IBSS I'd introduced."

Along with those...

Sujith Manoharan makes a minor change to not use a PLL hang workaroun
for AR9550.  This one-liner fixes a couple of bugs reported in the Red Hat
bugzilla.

Helmut Schaa addresses an ath9k_htc bug that mangles frame headers
during Tx.  This fix is small, tested by the bug reported and isolated
to ath9k_htc.

Stanislaw Gruszka reverts a recent iwl4965 change that broke rfkill
notification to user space.

Please let me know if there are problems!

Thanks,

John

---

The following changes since commit 4a5a8aa6c966eafc106543bd955ae388230420e5:

  ipv4: expose IPV4_DEVCONF (2013-08-22 20:30:15 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless.git for-davem

for you to fetch changes up to 81ca2ff9450c2e983ee416eca17ebe6fa9c3da4f:

  Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless into for-davem (2013-08-23 11:47:48 -0400)

----------------------------------------------------------------

Felix Fietkau (1):
      mac80211: add a flag to indicate CCK support for HT clients

Helmut Schaa (1):
      ath9k_htc: Restore skb headroom when returning skb to mac80211

Johannes Berg (1):
      mac80211: add missing channel context release

John W. Linville (2):
      Merge branch 'for-john' of git://git.kernel.org/.../jberg/mac80211
      Merge branch 'master' of git://git.kernel.org/.../linville/wireless into for-davem

Simon Wunderlich (1):
      mac80211: ibss: fix ignored channel parameter

Stanislaw Gruszka (1):
      iwl4965: fix rfkill set state regression

Sujith Manoharan (1):
      ath9k: Enable PLL fix only for AR9340/AR9330

 drivers/net/wireless/ath/ath9k/htc_drv_txrx.c | 10 ++++++++
 drivers/net/wireless/ath/ath9k/init.c         |  3 ++-
 drivers/net/wireless/ath/ath9k/main.c         |  3 +--
 drivers/net/wireless/ath/carl9170/main.c      |  3 ++-
 drivers/net/wireless/iwlegacy/4965-mac.c      |  2 +-
 drivers/net/wireless/rt2x00/rt2800lib.c       |  3 ++-
 include/net/mac80211.h                        |  1 +
 net/mac80211/ibss.c                           | 34 +++++++++++++++++++++++----
 net/mac80211/rc80211_minstrel_ht.c            |  3 +++
 9 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
index e602c95..c028df7 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
@@ -448,6 +448,7 @@ static void ath9k_htc_tx_process(struct ath9k_htc_priv *priv,
 	struct ieee80211_conf *cur_conf = &priv->hw->conf;
 	bool txok;
 	int slot;
+	int hdrlen, padsize;
 
 	slot = strip_drv_header(priv, skb);
 	if (slot < 0) {
@@ -504,6 +505,15 @@ send_mac80211:
 
 	ath9k_htc_tx_clear_slot(priv, slot);
 
+	/* Remove padding before handing frame back to mac80211 */
+	hdrlen = ieee80211_get_hdrlen_from_skb(skb);
+
+	padsize = hdrlen & 3;
+	if (padsize && skb->len > hdrlen + padsize) {
+		memmove(skb->data + padsize, skb->data, hdrlen);
+		skb_pull(skb, padsize);
+	}
+
 	/* Send status to mac80211 */
 	ieee80211_tx_status(priv->hw, skb);
 }
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 16f8b20..026a2a0 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -802,7 +802,8 @@ void ath9k_set_hw_capab(struct ath_softc *sc, struct ieee80211_hw *hw)
 		IEEE80211_HW_PS_NULLFUNC_STACK |
 		IEEE80211_HW_SPECTRUM_MGMT |
 		IEEE80211_HW_REPORTS_TX_ACK_STATUS |
-		IEEE80211_HW_SUPPORTS_RC_TABLE;
+		IEEE80211_HW_SUPPORTS_RC_TABLE |
+		IEEE80211_HW_SUPPORTS_HT_CCK_RATES;
 
 	if (sc->sc_ah->caps.hw_caps & ATH9K_HW_CAP_HT) {
 		hw->flags |= IEEE80211_HW_AMPDU_AGGREGATION;
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 1737a3e..cb5a655 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -173,8 +173,7 @@ static void ath_restart_work(struct ath_softc *sc)
 {
 	ieee80211_queue_delayed_work(sc->hw, &sc->tx_complete_work, 0);
 
-	if (AR_SREV_9340(sc->sc_ah) || AR_SREV_9485(sc->sc_ah) ||
-	    AR_SREV_9550(sc->sc_ah))
+	if (AR_SREV_9340(sc->sc_ah) || AR_SREV_9330(sc->sc_ah))
 		ieee80211_queue_delayed_work(sc->hw, &sc->hw_pll_work,
 				     msecs_to_jiffies(ATH_PLL_WORK_INTERVAL));
 
diff --git a/drivers/net/wireless/ath/carl9170/main.c b/drivers/net/wireless/ath/carl9170/main.c
index 4a33c6e..349fa22 100644
--- a/drivers/net/wireless/ath/carl9170/main.c
+++ b/drivers/net/wireless/ath/carl9170/main.c
@@ -1860,7 +1860,8 @@ void *carl9170_alloc(size_t priv_size)
 		     IEEE80211_HW_PS_NULLFUNC_STACK |
 		     IEEE80211_HW_NEED_DTIM_BEFORE_ASSOC |
 		     IEEE80211_HW_SUPPORTS_RC_TABLE |
-		     IEEE80211_HW_SIGNAL_DBM;
+		     IEEE80211_HW_SIGNAL_DBM |
+		     IEEE80211_HW_SUPPORTS_HT_CCK_RATES;
 
 	if (!modparam_noht) {
 		/*
diff --git a/drivers/net/wireless/iwlegacy/4965-mac.c b/drivers/net/wireless/iwlegacy/4965-mac.c
index f2ed62e..7acf5ee 100644
--- a/drivers/net/wireless/iwlegacy/4965-mac.c
+++ b/drivers/net/wireless/iwlegacy/4965-mac.c
@@ -4464,9 +4464,9 @@ il4965_irq_tasklet(struct il_priv *il)
 			set_bit(S_RFKILL, &il->status);
 		} else {
 			clear_bit(S_RFKILL, &il->status);
-			wiphy_rfkill_set_hw_state(il->hw->wiphy, hw_rf_kill);
 			il_force_reset(il, true);
 		}
+		wiphy_rfkill_set_hw_state(il->hw->wiphy, hw_rf_kill);
 
 		handled |= CSR_INT_BIT_RF_KILL;
 	}
diff --git a/drivers/net/wireless/rt2x00/rt2800lib.c b/drivers/net/wireless/rt2x00/rt2800lib.c
index 1f80ea5..1b41c8e 100644
--- a/drivers/net/wireless/rt2x00/rt2800lib.c
+++ b/drivers/net/wireless/rt2x00/rt2800lib.c
@@ -6133,7 +6133,8 @@ static int rt2800_probe_hw_mode(struct rt2x00_dev *rt2x00dev)
 	    IEEE80211_HW_SUPPORTS_PS |
 	    IEEE80211_HW_PS_NULLFUNC_STACK |
 	    IEEE80211_HW_AMPDU_AGGREGATION |
-	    IEEE80211_HW_REPORTS_TX_ACK_STATUS;
+	    IEEE80211_HW_REPORTS_TX_ACK_STATUS |
+	    IEEE80211_HW_SUPPORTS_HT_CCK_RATES;
 
 	/*
 	 * Don't set IEEE80211_HW_HOST_BROADCAST_PS_BUFFERING for USB devices
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 5b7a3da..551ba6a 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1499,6 +1499,7 @@ enum ieee80211_hw_flags {
 	IEEE80211_HW_SUPPORTS_RC_TABLE			= 1<<24,
 	IEEE80211_HW_P2P_DEV_ADDR_FOR_INTF		= 1<<25,
 	IEEE80211_HW_TIMING_BEACON_ONLY			= 1<<26,
+	IEEE80211_HW_SUPPORTS_HT_CCK_RATES		= 1<<27,
 };
 
 /**
diff --git a/net/mac80211/ibss.c b/net/mac80211/ibss.c
index ea7b9c2..2d45643 100644
--- a/net/mac80211/ibss.c
+++ b/net/mac80211/ibss.c
@@ -36,7 +36,7 @@
 
 static void __ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
 				      const u8 *bssid, const int beacon_int,
-				      struct ieee80211_channel *chan,
+				      struct cfg80211_chan_def *req_chandef,
 				      const u32 basic_rates,
 				      const u16 capability, u64 tsf,
 				      bool creator)
@@ -51,6 +51,7 @@ static void __ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
 	u32 bss_change;
 	u8 supp_rates[IEEE80211_MAX_SUPP_RATES];
 	struct cfg80211_chan_def chandef;
+	struct ieee80211_channel *chan;
 	struct beacon_data *presp;
 	int frame_len;
 
@@ -81,7 +82,9 @@ static void __ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
 
 	sdata->drop_unencrypted = capability & WLAN_CAPABILITY_PRIVACY ? 1 : 0;
 
-	chandef = ifibss->chandef;
+	/* make a copy of the chandef, it could be modified below. */
+	chandef = *req_chandef;
+	chan = chandef.chan;
 	if (!cfg80211_reg_can_beacon(local->hw.wiphy, &chandef)) {
 		chandef.width = NL80211_CHAN_WIDTH_20;
 		chandef.center_freq1 = chan->center_freq;
@@ -259,10 +262,12 @@ static void ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
 	struct cfg80211_bss *cbss =
 		container_of((void *)bss, struct cfg80211_bss, priv);
 	struct ieee80211_supported_band *sband;
+	struct cfg80211_chan_def chandef;
 	u32 basic_rates;
 	int i, j;
 	u16 beacon_int = cbss->beacon_interval;
 	const struct cfg80211_bss_ies *ies;
+	enum nl80211_channel_type chan_type;
 	u64 tsf;
 
 	sdata_assert_lock(sdata);
@@ -270,6 +275,26 @@ static void ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
 	if (beacon_int < 10)
 		beacon_int = 10;
 
+	switch (sdata->u.ibss.chandef.width) {
+	case NL80211_CHAN_WIDTH_20_NOHT:
+	case NL80211_CHAN_WIDTH_20:
+	case NL80211_CHAN_WIDTH_40:
+		chan_type = cfg80211_get_chandef_type(&sdata->u.ibss.chandef);
+		cfg80211_chandef_create(&chandef, cbss->channel, chan_type);
+		break;
+	case NL80211_CHAN_WIDTH_5:
+	case NL80211_CHAN_WIDTH_10:
+		cfg80211_chandef_create(&chandef, cbss->channel,
+					NL80211_CHAN_WIDTH_20_NOHT);
+		chandef.width = sdata->u.ibss.chandef.width;
+		break;
+	default:
+		/* fall back to 20 MHz for unsupported modes */
+		cfg80211_chandef_create(&chandef, cbss->channel,
+					NL80211_CHAN_WIDTH_20_NOHT);
+		break;
+	}
+
 	sband = sdata->local->hw.wiphy->bands[cbss->channel->band];
 
 	basic_rates = 0;
@@ -294,7 +319,7 @@ static void ieee80211_sta_join_ibss(struct ieee80211_sub_if_data *sdata,
 
 	__ieee80211_sta_join_ibss(sdata, cbss->bssid,
 				  beacon_int,
-				  cbss->channel,
+				  &chandef,
 				  basic_rates,
 				  cbss->capability,
 				  tsf, false);
@@ -736,7 +761,7 @@ static void ieee80211_sta_create_ibss(struct ieee80211_sub_if_data *sdata)
 		sdata->drop_unencrypted = 0;
 
 	__ieee80211_sta_join_ibss(sdata, bssid, sdata->vif.bss_conf.beacon_int,
-				  ifibss->chandef.chan, ifibss->basic_rates,
+				  &ifibss->chandef, ifibss->basic_rates,
 				  capability, 0, true);
 }
 
@@ -1138,6 +1163,7 @@ int ieee80211_ibss_leave(struct ieee80211_sub_if_data *sdata)
 	clear_bit(SDATA_STATE_OFFCHANNEL_BEACON_STOPPED, &sdata->state);
 	ieee80211_bss_info_change_notify(sdata, BSS_CHANGED_BEACON_ENABLED |
 						BSS_CHANGED_IBSS);
+	ieee80211_vif_release_channel(sdata);
 	synchronize_rcu();
 	kfree(presp);
 
diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index f5aed96..f3bbea1 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -828,6 +828,9 @@ minstrel_ht_update_cck(struct minstrel_priv *mp, struct minstrel_ht_sta *mi,
 	if (sband->band != IEEE80211_BAND_2GHZ)
 		return;
 
+	if (!(mp->hw->flags & IEEE80211_HW_SUPPORTS_HT_CCK_RATES))
+		return;
+
 	mi->cck_supported = 0;
 	mi->cck_supported_short = 0;
 	for (i = 0; i < 4; i++) {
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply related
* Re: netem: the reorder discussion
From: Teco Boot @ 2013-08-23 16:18 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Stephen Hemminger, Ferry Huberts, Netem, netdev@vger.kernel.org,
	core-users@pf.itd.nrl.navy.mil
In-Reply-To: <1377273838.8828.11.camel@edumazet-glaptop>


Op 23 aug. 2013 om 18:03 heeft Eric Dumazet <eric.dumazet@gmail.com> het volgende geschreven:

> On Fri, 2013-08-23 at 17:53 +0200, Teco Boot wrote:
> 
>> Questions that pops out of of my head: What is more important,
>> understandable configuration or keep existing behavior for unmodified
>> configuration? Do we want to reorder, even with configured rate?
>> 
>> My preference: make in understandable, document well and make all
>> sensible options available.
> 
> The day 'rate' support was added, documentation became obsolete.
> 
> Its actually hard to have both rate and delay + jitters + reorders or
> not.
> 
> If you can fix this properly, patches are welcome, but I suspect you
> wanted to reintroduce the thing that made netem unusable for us.

No, for sure I do not want to make it unusable for you. Or anybody. The opposite is true. 
I don't like the "look in souce" documentation. Or parameters with hard to understand side effects. 

^ permalink raw reply
* Re: [PATCH] netfilter: avoid array overflow in nf_register_hook
From: Eric Dumazet @ 2013-08-23 16:05 UTC (permalink / raw)
  To: Dong Fang
  Cc: pablo, kaber, kadlec, davem, netfilter-devel, netfilter, coreteam,
	netdev
In-Reply-To: <1377313495-16060-1-git-send-email-yp.fangdong@gmail.com>

On Fri, 2013-08-23 at 23:04 -0400, Dong Fang wrote:
> This patch fix the array overflow in nf_register_hook function
> 
> Signed-off-by: Dong Fang <yp.fangdong@gmail.com>
> ---
>  net/netfilter/core.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/net/netfilter/core.c b/net/netfilter/core.c
> index 2217363..819eee1 100644
> --- a/net/netfilter/core.c
> +++ b/net/netfilter/core.c
> @@ -68,8 +68,11 @@ static DEFINE_MUTEX(nf_hook_mutex);
>  int nf_register_hook(struct nf_hook_ops *reg)
>  {
>  	struct nf_hook_ops *elem;
> -	int err;
> +	int err = -EINVAL;
>  
> +	if (reg->pf >= NFPROTO_NUMPROTO || reg->hooknum >= NF_MAX_HOOKS)
> +		return err;
> +

Thats a bug in the caller.

We want a hard crash so that the poor netfilter guy who made this error
can fix the bug before sending us a buggy patch.



^ permalink raw reply
* Re: netem: the reorder discussion
From: Eric Dumazet @ 2013-08-23 16:03 UTC (permalink / raw)
  To: Teco Boot
  Cc: Stephen Hemminger, Ferry Huberts, Netem, netdev@vger.kernel.org,
	core-users@pf.itd.nrl.navy.mil
In-Reply-To: <31E9BEA1-3DE2-4DA7-9CEE-6B7C9C99F5EB@inf-net.nl>

On Fri, 2013-08-23 at 17:53 +0200, Teco Boot wrote:

> Questions that pops out of of my head: What is more important,
> understandable configuration or keep existing behavior for unmodified
> configuration? Do we want to reorder, even with configured rate?
> 
> My preference: make in understandable, document well and make all
> sensible options available. 

The day 'rate' support was added, documentation became obsolete.

Its actually hard to have both rate and delay + jitters + reorders or
not.

If you can fix this properly, patches are welcome, but I suspect you
wanted to reintroduce the thing that made netem unusable for us.

^ permalink raw reply
* Re: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Santosh Shilimkar @ 2013-08-23 15:54 UTC (permalink / raw)
  To: Benoit Cousson
  Cc: Daniel Mack, netdev, nsekhar, sergei.shtylyov, davem, ujhelyi.m,
	mugunthanvnm, vaibhav.bedia, d-gerlach, linux-arm-kernel,
	linux-omap, devicetree
In-Reply-To: <52177E29.6040103@baylibre.com>

On Friday 23 August 2013 11:22 AM, Benoit Cousson wrote:
> Hi Santosh,
> 
> [...]
> 
>>> +static const struct of_device_id cpsw_of_mtable[] = {
>>> +    {
>>> +        .compatible    = "ti,am3352-cpsw",
>> I didn't notice this earlier, but can't you use the IP version
>> as a compatible instead of using a SOC name. Whats really SOC specific
>> on this IP ? Sorry i have missed any earlier discussion on this but
>> this approach doesn't seem good. Its like adding SOC checks in the
>> driver subsystem.
> 
> Hehe, welcome to the club!
> I keep arguing about that as well :-)
> 
> I think we should create a Facebook group: "against-the-soc-version-in-the-compatible-string".
> 
Indeed !!

Regards,
Santosh

^ permalink raw reply
* Re: netem: the reorder discussion
From: Teco Boot @ 2013-08-23 15:53 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: Eric Dumazet, Ferry Huberts, Netem, netdev@vger.kernel.org,
	core-users@pf.itd.nrl.navy.mil
In-Reply-To: <20130823084450.6baf2835@nehalam.linuxnetplumber.net>


Op 23 aug. 2013, om 17:44 heeft Stephen Hemminger <stephen@networkplumber.org> het volgende geschreven:

> On Fri, 23 Aug 2013 08:12:36 -0700
> Eric Dumazet <eric.dumazet@gmail.com> wrote:
> 
>> On Fri, 2013-08-23 at 16:16 +0200, Teco Boot wrote:
>>> I had reorder problems with netem qdisc. I make use of the NRL CORE
>>> network emulator, which sets up virtual routers and links, build with
>>> network namespaces and netem. Typical CORE config for a n1<-->n2 link
>>> with rate (1024Kbps), delay (50ms) and no jitter:
>>>  qdisc tbf 1: dev n1.eth0.222 root refcnt 2 rate 1024Kbit burst 2999b
>>> lat 488.6ms 
>>>  qdisc netem 10: dev n1.eth0.222 parent 1:1 limit 1000 delay 50.0ms
>>>  qdisc tbf 1: dev n2.eth0.222 root refcnt 2 rate 1024Kbit burst 2999b
>>> lat 488.6ms 
>>>  qdisc netem 10: dev n2.eth0.222 parent 1:1 limit 1000 delay 50.0ms
>>> 
>>> I added jitter to this bi-directional link, e.g. 20ms. Now the delay
>>> for each packet is 50ms +/- 20ms is 30ms to 70ms.
>>> However, this has some unexpected results: packets may be reordered.
>>> That is because the actual delay is calculated for each packet. Some
>>> packets have a larger delay, some have a smaller. If the inter-packet
>>> spacing is smaller than the time differences set by netem (up to 2x
>>> configured jitter), packets are reordered. In this example with
>>> +/20ms, this is the case with packet rate larger than 25pps.
>>> Reordering has bad effects on transport protocol throughputs.
>>> Reordering is less common on the Internet, so I don't want to emulate
>>> such. I don't say there is no reordering, I just say I don't want this
>>> netem behavior.
>>> 
>>> The netem guide mentions this unexpected results. It was caused by a
>>> change in version 1.1 (2.6.15). For people like me that do not want
>>> this, there is a work-around mentioned. However, this doesn't work
>>> anymore since somewhere before 2.6.31.
>>> http://www.linuxfoundation.org/collaborate/workgroups/networking/netem
>>> https://lists.linux-foundation.org/pipermail/netem/2011-April/001507.html
>>> 
>>> So what to do? The recent patch from Eric Dumazet eliminates
>>> jitter_reordering when netem rate is configured. Maybe not the clean
>>> approach, but acceptable. Ferry his patch eliminates
>>> jitter_reordering. Some people may use reorder by jitter, so this
>>> "feature" should not get removed. So Ferry his patch is not accepted.
>>> On the other hand, people like me are very confused by current
>>> behavior.
>>> 
>>> What we could do is keep existing features and describe what netem
>>> currently does. That is:
>>> - netem with delay and jitter may reorder packets, if inter packet
>>> spacing is smaller than jitter
>>> - reordering caused by delay and jitter can be turned off by using
>>> netem rate. rate can be set to very high value is no shaping is
>>> wanted.
>> 
>> As long as one can define the expected behavior, you can add whatever
>> new netem parameter.
>> 
>> One could envision adding flow separation (skb->sk or rxhashing), so
>> that each flow can have his own local queue, to guarantee no reorders
>> per flow _if_ this is needed, even if per flow delays/jitter is/are
>> configured.
>> 
>> We also use netem to test on large scale, where the reordering stuff
>> needs fixes in transport stacks (And yes, we are working on TCP stack
>> to permit higher levels of reorders)
> 
> I am happy with any solution that is:
>  * allows both always ordering and reordering based on random jitter of delay
>  * documented
> 
> I do get worried that people's tests get different results because of
> netem behavior changes. Researchers like to have repeatable results.

Questions that pops out of of my head: What is more important, understandable configuration or keep existing behavior for unmodified configuration? Do we want to reorder, even with configured rate?

My preference: make in understandable, document well and make all sensible options available. 

Teco

^ permalink raw reply
* Re: [PATCH RESEND net-next 06/16] sfc: Introduce and use MCDI_DECLARE_BUF macro
From: Ben Hutchings @ 2013-08-23 15:52 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, linux-net-drivers
In-Reply-To: <1377253570.2586.15.camel@deadeye.wl.decadent.org.uk>

On Fri, 2013-08-23 at 11:26 +0100, Ben Hutchings wrote:
> On Thu, 2013-08-22 at 22:02 -0700, David Miller wrote:
> > From: Ben Hutchings <bhutchings@solarflare.com>
> > Date: Thu, 22 Aug 2013 20:26:49 +0100
> > 
> > > MCDI_DECLARE_BUF declares a variable as an MCDI buffer of the
> > > requested length, adding any necessary padding.
> > > 
> > > Signed-off-by: Ben Hutchings <bhutchings@solarflare.com>
> > 
> > There really isn't a guarentee that a u8 array is going to
> > aligned appropriately to write dwords there using casts like
> > you do.
> 
> Yes, but this has been wrong for years.
> 
> > Since you're here macro'izing the declaration, take the time to fix it
> > to specify the necessary alignment as well.
> [...]
> 
> My intent was to separate refactoring changes with no immediate effect
> (#6-9) from functional changes (#10-11), making them easier to review.
> The alignment gets fixed by #11.

Does this explanation resolve your concerns or do you still want me to
squash a fix into this commit?

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

^ permalink raw reply
* Re: netem: the reorder discussion
From: Stephen Hemminger @ 2013-08-23 15:44 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Teco Boot, Ferry Huberts, Netem, netdev@vger.kernel.org,
	core-users@pf.itd.nrl.navy.mil
In-Reply-To: <1377270756.8828.5.camel@edumazet-glaptop>

On Fri, 23 Aug 2013 08:12:36 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:

> On Fri, 2013-08-23 at 16:16 +0200, Teco Boot wrote:
> > I had reorder problems with netem qdisc. I make use of the NRL CORE
> > network emulator, which sets up virtual routers and links, build with
> > network namespaces and netem. Typical CORE config for a n1<-->n2 link
> > with rate (1024Kbps), delay (50ms) and no jitter:
> >   qdisc tbf 1: dev n1.eth0.222 root refcnt 2 rate 1024Kbit burst 2999b
> > lat 488.6ms 
> >   qdisc netem 10: dev n1.eth0.222 parent 1:1 limit 1000 delay 50.0ms
> >   qdisc tbf 1: dev n2.eth0.222 root refcnt 2 rate 1024Kbit burst 2999b
> > lat 488.6ms 
> >   qdisc netem 10: dev n2.eth0.222 parent 1:1 limit 1000 delay 50.0ms
> > 
> > I added jitter to this bi-directional link, e.g. 20ms. Now the delay
> > for each packet is 50ms +/- 20ms is 30ms to 70ms.
> > However, this has some unexpected results: packets may be reordered.
> > That is because the actual delay is calculated for each packet. Some
> > packets have a larger delay, some have a smaller. If the inter-packet
> > spacing is smaller than the time differences set by netem (up to 2x
> > configured jitter), packets are reordered. In this example with
> > +/20ms, this is the case with packet rate larger than 25pps.
> > Reordering has bad effects on transport protocol throughputs.
> > Reordering is less common on the Internet, so I don't want to emulate
> > such. I don't say there is no reordering, I just say I don't want this
> > netem behavior.
> > 
> > The netem guide mentions this unexpected results. It was caused by a
> > change in version 1.1 (2.6.15). For people like me that do not want
> > this, there is a work-around mentioned. However, this doesn't work
> > anymore since somewhere before 2.6.31.
> > http://www.linuxfoundation.org/collaborate/workgroups/networking/netem
> > https://lists.linux-foundation.org/pipermail/netem/2011-April/001507.html
> > 
> > So what to do? The recent patch from Eric Dumazet eliminates
> > jitter_reordering when netem rate is configured. Maybe not the clean
> > approach, but acceptable. Ferry his patch eliminates
> > jitter_reordering. Some people may use reorder by jitter, so this
> > "feature" should not get removed. So Ferry his patch is not accepted.
> > On the other hand, people like me are very confused by current
> > behavior.
> > 
> > What we could do is keep existing features and describe what netem
> > currently does. That is:
> >  - netem with delay and jitter may reorder packets, if inter packet
> > spacing is smaller than jitter
> >  - reordering caused by delay and jitter can be turned off by using
> > netem rate. rate can be set to very high value is no shaping is
> > wanted.
> 
> As long as one can define the expected behavior, you can add whatever
> new netem parameter.
> 
> One could envision adding flow separation (skb->sk or rxhashing), so
> that each flow can have his own local queue, to guarantee no reorders
> per flow _if_ this is needed, even if per flow delays/jitter is/are
> configured.
> 
> We also use netem to test on large scale, where the reordering stuff
> needs fixes in transport stacks (And yes, we are working on TCP stack
> to permit higher levels of reorders)

I am happy with any solution that is:
  * allows both always ordering and reordering based on random jitter of delay
  * documented

I do get worried that people's tests get different results because of
netem behavior changes. Researchers like to have repeatable results.

^ permalink raw reply
* Re: [PATCH net-next] net: tcp_probe: allow more advanced ingress filtering by mark
From: Daniel Borkmann @ 2013-08-23 15:28 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: davem, netdev
In-Reply-To: <1377270813.8828.6.camel@edumazet-glaptop>

On 08/23/2013 05:13 PM, Eric Dumazet wrote:
> On Fri, 2013-08-23 at 16:16 +0200, Daniel Borkmann wrote:
>> Currently, the tcp_probe snooper can either filter packets by a given
>> port (handed to the module via module parameter e.g. port=80) or lets
>> all TCP traffic pass (port=0, default). When a port is specified, the
>> port number is tested against the sk's source/destination port. Thus,
>> if one of them matches, the information will be further processed for
>> the log.
>>
>> As this is quite limited, allow for more advanced filtering possibilities
>> which can facilitate debugging/analysis with the help of the tcp_probe
>> snooper. Therefore, similarly as added to BPF machine in commit 7e75f93e
>> ("pkt_sched: ingress socket filter by mark"), add the possibility to
>> use skb->mark as a filter.
>>
>> If the mark is not being used otherwise, this allows ingress filtering
>> by flow (e.g. in order to track updates from only a single flow, or a
>> subset of all flows for a given port) and other things such as dynamic
>> logging and reconfiguration without removing/re-inserting the tcp_probe
>> module, etc. Simple example:
>>
>>    insmod net/ipv4/tcp_probe.ko fwmark=8888 full=1
>>    ...
>>    iptables -A INPUT -i eth4 -t mangle -p tcp --dport 22 \
>>             --sport 60952 -j MARK --set-mark 8888
>>    [... sampling interval ...]
>>    iptables -D INPUT -i eth4 -t mangle -p tcp --dport 22 \
>>             --sport 60952 -j MARK --set-mark 8888
>>
>> The current option to filter by a given port is still being preserved. A
>> similar approach could be done for the sctp_probe module as a follow-up.
>>
>> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
>> ---
>
> Well, why not adding ability to load/use a BPF filter instead ? ;)

Hehe. ;-)

I think this patch is minimal and least intrusive and for observing flows
the skb->mark would provide an easy job for us of quickly configuring and
altering a filter during runtime through iptables or other tools.

If we would tell the user to load some BPF bytecode first through tool <xyz>,
we would need an additional interface of injecting this into tcp_probe, and
the ability to atomically change a filter at run-time. Likely, that the
biggest use-case would be to filter particular flows in order to observe
TCP params, I think the whole BPF machinery would be sort of overkill for
that, although you know I like playing with BPF. ;-)

So I think these 15 lines of code from this patch are less intrusive and
more clean than the alternative.

^ permalink raw reply
* Re: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Benoit Cousson @ 2013-08-23 15:22 UTC (permalink / raw)
  To: Santosh Shilimkar
  Cc: Daniel Mack, netdev, nsekhar, sergei.shtylyov, davem, ujhelyi.m,
	mugunthanvnm, vaibhav.bedia, d-gerlach, linux-arm-kernel,
	linux-omap, devicetree
In-Reply-To: <52177052.1030308@ti.com>

Hi Santosh,

[...]

>> +static const struct of_device_id cpsw_of_mtable[] = {
>> +	{
>> +		.compatible	= "ti,am3352-cpsw",
> I didn't notice this earlier, but can't you use the IP version
> as a compatible instead of using a SOC name. Whats really SOC specific
> on this IP ? Sorry i have missed any earlier discussion on this but
> this approach doesn't seem good. Its like adding SOC checks in the
> driver subsystem.

Hehe, welcome to the club!
I keep arguing about that as well :-)

I think we should create a Facebook group: 
"against-the-soc-version-in-the-compatible-string".

Regards,
Benoit

^ permalink raw reply
* Re: [PATCH net-next] net: tcp_probe: allow more advanced ingress filtering by mark
From: Eric Dumazet @ 2013-08-23 15:13 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: davem, netdev
In-Reply-To: <1377267393-24604-1-git-send-email-dborkman@redhat.com>

On Fri, 2013-08-23 at 16:16 +0200, Daniel Borkmann wrote:
> Currently, the tcp_probe snooper can either filter packets by a given
> port (handed to the module via module parameter e.g. port=80) or lets
> all TCP traffic pass (port=0, default). When a port is specified, the
> port number is tested against the sk's source/destination port. Thus,
> if one of them matches, the information will be further processed for
> the log.
> 
> As this is quite limited, allow for more advanced filtering possibilities
> which can facilitate debugging/analysis with the help of the tcp_probe
> snooper. Therefore, similarly as added to BPF machine in commit 7e75f93e
> ("pkt_sched: ingress socket filter by mark"), add the possibility to
> use skb->mark as a filter.
> 
> If the mark is not being used otherwise, this allows ingress filtering
> by flow (e.g. in order to track updates from only a single flow, or a
> subset of all flows for a given port) and other things such as dynamic
> logging and reconfiguration without removing/re-inserting the tcp_probe
> module, etc. Simple example:
> 
>   insmod net/ipv4/tcp_probe.ko fwmark=8888 full=1
>   ...
>   iptables -A INPUT -i eth4 -t mangle -p tcp --dport 22 \
>            --sport 60952 -j MARK --set-mark 8888
>   [... sampling interval ...]
>   iptables -D INPUT -i eth4 -t mangle -p tcp --dport 22 \
>            --sport 60952 -j MARK --set-mark 8888
> 
> The current option to filter by a given port is still being preserved. A
> similar approach could be done for the sctp_probe module as a follow-up.
> 
> Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
> ---

Well, why not adding ability to load/use a BPF filter instead ? ;)

^ permalink raw reply
* Re: netem: the reorder discussion
From: Eric Dumazet @ 2013-08-23 15:12 UTC (permalink / raw)
  To: Teco Boot
  Cc: Ferry Huberts, Netem, netdev@vger.kernel.org,
	core-users@pf.itd.nrl.navy.mil
In-Reply-To: <766825FB-5295-4C5C-ABA0-CC520EFE2EA6@inf-net.nl>

On Fri, 2013-08-23 at 16:16 +0200, Teco Boot wrote:
> I had reorder problems with netem qdisc. I make use of the NRL CORE
> network emulator, which sets up virtual routers and links, build with
> network namespaces and netem. Typical CORE config for a n1<-->n2 link
> with rate (1024Kbps), delay (50ms) and no jitter:
>   qdisc tbf 1: dev n1.eth0.222 root refcnt 2 rate 1024Kbit burst 2999b
> lat 488.6ms 
>   qdisc netem 10: dev n1.eth0.222 parent 1:1 limit 1000 delay 50.0ms
>   qdisc tbf 1: dev n2.eth0.222 root refcnt 2 rate 1024Kbit burst 2999b
> lat 488.6ms 
>   qdisc netem 10: dev n2.eth0.222 parent 1:1 limit 1000 delay 50.0ms
> 
> I added jitter to this bi-directional link, e.g. 20ms. Now the delay
> for each packet is 50ms +/- 20ms is 30ms to 70ms.
> However, this has some unexpected results: packets may be reordered.
> That is because the actual delay is calculated for each packet. Some
> packets have a larger delay, some have a smaller. If the inter-packet
> spacing is smaller than the time differences set by netem (up to 2x
> configured jitter), packets are reordered. In this example with
> +/20ms, this is the case with packet rate larger than 25pps.
> Reordering has bad effects on transport protocol throughputs.
> Reordering is less common on the Internet, so I don't want to emulate
> such. I don't say there is no reordering, I just say I don't want this
> netem behavior.
> 
> The netem guide mentions this unexpected results. It was caused by a
> change in version 1.1 (2.6.15). For people like me that do not want
> this, there is a work-around mentioned. However, this doesn't work
> anymore since somewhere before 2.6.31.
> http://www.linuxfoundation.org/collaborate/workgroups/networking/netem
> https://lists.linux-foundation.org/pipermail/netem/2011-April/001507.html
> 
> So what to do? The recent patch from Eric Dumazet eliminates
> jitter_reordering when netem rate is configured. Maybe not the clean
> approach, but acceptable. Ferry his patch eliminates
> jitter_reordering. Some people may use reorder by jitter, so this
> "feature" should not get removed. So Ferry his patch is not accepted.
> On the other hand, people like me are very confused by current
> behavior.
> 
> What we could do is keep existing features and describe what netem
> currently does. That is:
>  - netem with delay and jitter may reorder packets, if inter packet
> spacing is smaller than jitter
>  - reordering caused by delay and jitter can be turned off by using
> netem rate. rate can be set to very high value is no shaping is
> wanted.

As long as one can define the expected behavior, you can add whatever
new netem parameter.

One could envision adding flow separation (skb->sk or rxhashing), so
that each flow can have his own local queue, to guarantee no reorders
per flow _if_ this is needed, even if per flow delays/jitter is/are
configured.

We also use netem to test on large scale, where the reordering stuff
needs fixes in transport stacks (And yes, we are working on TCP stack
to permit higher levels of reorders)

^ permalink raw reply
* Re: [PATCH net-next 08/10] net: libertas: inherit addr_assign_type along with dev_addr
From: John W. Linville @ 2013-08-23 14:52 UTC (permalink / raw)
  To: Bjørn Mork; +Cc: netdev, libertas-dev, linux-wireless
In-Reply-To: <1377250513-3662-9-git-send-email-bjorn@mork.no>

On Fri, Aug 23, 2013 at 11:35:11AM +0200, Bjørn Mork wrote:
> Cc: "John W. Linville" <linville@tuxdriver.com>
> Signed-off-by: Bjørn Mork <bjorn@mork.no>
> ---
>  drivers/net/wireless/libertas/mesh.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/libertas/mesh.c b/drivers/net/wireless/libertas/mesh.c
> index efae07e..6fef746 100644
> --- a/drivers/net/wireless/libertas/mesh.c
> +++ b/drivers/net/wireless/libertas/mesh.c
> @@ -1017,7 +1017,7 @@ static int lbs_add_mesh(struct lbs_private *priv)
>  
>  	mesh_dev->netdev_ops = &mesh_netdev_ops;
>  	mesh_dev->ethtool_ops = &lbs_ethtool_ops;
> -	memcpy(mesh_dev->dev_addr, priv->dev->dev_addr, ETH_ALEN);
> +	eth_hw_addr_inherit(mesh_dev, priv->dev);
>  
>  	SET_NETDEV_DEV(priv->mesh_dev, priv->dev->dev.parent);
>  
> -- 
> 1.7.10.4
> 
> 
Acked-by: "John W. Linville" <linville@tuxdriver.com>

-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

^ permalink raw reply
* Re: [PATCH v4 2/5] net: ethernet: cpsw: add optional third memory region for CONTROL module
From: Sergei Shtylyov @ 2013-08-23 14:59 UTC (permalink / raw)
  To: Daniel Mack
  Cc: netdev, bcousson, nsekhar, davem, ujhelyi.m, mugunthanvnm,
	vaibhav.bedia, d-gerlach, linux-arm-kernel, linux-omap,
	devicetree
In-Reply-To: <1377267365-24057-3-git-send-email-zonque@gmail.com>

On 23-08-2013 18:16, Daniel Mack wrote:

> At least the AM33xx SoC has a control module register to configure
> details such as the hardware ethernet interface mode.

> I'm not sure whether all SoCs which feature the cpsw block have such a
> register, so that third memory region is considered optional for now.

> Signed-off-by: Daniel Mack <zonque@gmail.com>
[...]

> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> index 849af52..7a25ff4 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
[...]
> @@ -1999,6 +2000,21 @@ static int cpsw_probe(struct platform_device *pdev)
>   		goto clean_runtime_disable_ret;
>   	}
>
> +	/* If the control memory region is unspecified, continue without it.
> +	 * If it is specified, but we're unable to reserve it, bail.
> +	 */
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
> +	if (!res) {
> +		dev_info(priv->dev, "error getting control i/o resource\n");
> +		goto no_gmii_sel;
> +	}
> +	priv->gmii_sel_reg = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(priv->gmii_sel_reg)) {
> +		dev_err(priv->dev, "unable to map control i/o region\n");

    You didn't actually seem to heed my words about error message. And don't 
you want to do:

	res = PTR_ERR(priv->gmii_sel_reg);

> +		goto clean_runtime_disable_ret;
> +	}
> +
> +no_gmii_sel:
>   	memset(&dma_params, 0, sizeof(dma_params));
>   	memset(&ale_params, 0, sizeof(ale_params));

WBR, Sergei



^ permalink raw reply
* Re: [PATCH net-next 06/10] net: airo: inherit addr_assign_type along with dev_addr
From: John W. Linville @ 2013-08-23 14:53 UTC (permalink / raw)
  To: Bjørn Mork
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1377250513-3662-7-git-send-email-bjorn-yOkvZcmFvRU@public.gmane.org>

On Fri, Aug 23, 2013 at 11:35:09AM +0200, Bjørn Mork wrote:
> Cc: "John W. Linville" <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
> Signed-off-by: Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org>
> ---
>  drivers/net/wireless/airo.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/airo.c b/drivers/net/wireless/airo.c
> index d0adbaf..7fe1964 100644
> --- a/drivers/net/wireless/airo.c
> +++ b/drivers/net/wireless/airo.c
> @@ -2693,7 +2693,7 @@ static struct net_device *init_wifidev(struct airo_info *ai,
>  	dev->base_addr = ethdev->base_addr;
>  	dev->wireless_data = ethdev->wireless_data;
>  	SET_NETDEV_DEV(dev, ethdev->dev.parent);
> -	memcpy(dev->dev_addr, ethdev->dev_addr, dev->addr_len);
> +	eth_hw_addr_inherit(dev, ethdev);
>  	err = register_netdev(dev);
>  	if (err<0) {
>  		free_netdev(dev);
> -- 
> 1.7.10.4

Acked-by: "John W. Linville" <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>

-- 
John W. Linville		Someday the world will need a hero, and you
linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org			might be all we have.  Be ready.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" 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 v2 nf-next] netfilter: conntrack: remove the central spinlock
From: Jesper Dangaard Brouer @ 2013-08-23 14:42 UTC (permalink / raw)
  To: Pablo Neira Ayuso
  Cc: Eric Dumazet, netfilter-devel, netdev, Tom Herbert,
	Patrick McHardy
In-Reply-To: <20130527123656.GA16212@localhost>

Hi Pablo,

On Mon, 27 May 2013 14:36:56 +0200
Pablo Neira Ayuso <pablo@netfilter.org> wrote:

> On Mon, May 27, 2013 at 02:33:46PM +0200, Jesper Dangaard Brouer wrote:
> > On Fri, 24 May 2013 06:51:36 -0700
> > Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > 
> > > On Fri, 2013-05-24 at 15:16 +0200, Jesper Dangaard Brouer wrote:
> > [...cut...]
> > > > I'm amazed, this patch will actually make it a viable choice to load
> > > > the conntrack modules on a DDoS based filtering box, and use the
> > > > conntracks to protect against ACK and SYN+ACK attacks.
> > > > 
> > > > Simply by not accepting the ACK or SYN+ACK to create a conntrack
> > > > entry. Via the command:
> > > >  sysctl -w net/netfilter/nf_conntrack_tcp_loose=0
> > > > 
> > > > A quick test show; now I can run a LISTEN process on the port, and
> > > > handle an SYN+ACK attack of approx 2580Kpps (and the same for ACK
> > > > attacks), while running a LISTEN process on the port.
> > > > 
> > [...]
> > > > 
> > > 
> > > Wow, this is very interesting !
> > > 
> > > Did you test the thing when expectations are possible ? (say ftp
> > > module loaded)
> > 
> > Nope. I'm not sure how to create a test case, that causes an
> > expectation to be created.
> 
> This is still in my queue, I didn't forget about this. I need to find
> some spare time to give this a test with expectations enabled and also
> with conntrackd/state-sync.

What about this patch, what is the status?
Is it still on you queue, or did it get applied without me noticing?


Link for people wanting to read-up on thread:
 http://thread.gmane.org/gmane.linux.network/268758/
-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Sr. Network Kernel Developer at Red Hat
  Author of http://www.iptv-analyzer.org
  LinkedIn: http://www.linkedin.com/in/brouer

^ permalink raw reply
* Re: [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Santosh Shilimkar @ 2013-08-23 14:23 UTC (permalink / raw)
  To: Daniel Mack
  Cc: netdev, bcousson, nsekhar, sergei.shtylyov, davem, ujhelyi.m,
	mugunthanvnm, vaibhav.bedia, d-gerlach, linux-arm-kernel,
	linux-omap, devicetree
In-Reply-To: <1377267365-24057-4-git-send-email-zonque@gmail.com>

On Friday 23 August 2013 10:16 AM, Daniel Mack wrote:
> In order to support features that are specific to the AM335x IP, we have
> to add hardware types and another compatible string.
> 
> Signed-off-by: Daniel Mack <zonque@gmail.com>
> ---
>  Documentation/devicetree/bindings/net/cpsw.txt |  3 ++-
>  drivers/net/ethernet/ti/cpsw.c                 | 32 ++++++++++++++++++++------
>  drivers/net/ethernet/ti/cpsw.h                 |  1 +
>  3 files changed, 28 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
> index 4e5ca54..b717458 100644
> --- a/Documentation/devicetree/bindings/net/cpsw.txt
> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
> @@ -2,7 +2,8 @@ TI SoC Ethernet Switch Controller Device Tree Bindings
>  ------------------------------------------------------
>  
>  Required properties:
> -- compatible		: Should be "ti,cpsw"
> +- compatible		: Should be "ti,cpsw" for generic cpsw support, or
> +			  "ti,am3352-cpsw" for AM3352 SoCs
>  - reg			: physical base address and size of the cpsw
>  			  registers map.
>  			  An optional third memory region can be supplied if
> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> index 7a25ff4..73c44cb6 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
> @@ -155,6 +155,11 @@ do {								\
>  		((priv->data.dual_emac) ? priv->emac_port :	\
>  		priv->data.active_slave)
>  
> +enum {
> +	CPSW_TYPE_GENERIC,
> +	CPSW_TYPE_AM33XX
> +};
> +
>  static int debug_level;
>  module_param(debug_level, int, 0);
>  MODULE_PARM_DESC(debug_level, "cpsw debug level (NETIF_MSG bits)");
> @@ -1692,17 +1697,36 @@ static void cpsw_slave_init(struct cpsw_slave *slave, struct cpsw_priv *priv,
>  	slave->port_vlan = data->dual_emac_res_vlan;
>  }
>  
> +static const struct of_device_id cpsw_of_mtable[] = {
> +	{
> +		.compatible	= "ti,am3352-cpsw",
I didn't notice this earlier, but can't you use the IP version
as a compatible instead of using a SOC name. Whats really SOC specific
on this IP ? Sorry i have missed any earlier discussion on this but
this approach doesn't seem good. Its like adding SOC checks in the
driver subsystem.

Regards,
Santosh



^ permalink raw reply
* netem: the reorder discussion
From: Teco Boot @ 2013-08-23 14:16 UTC (permalink / raw)
  To: Eric Dumazet, Ferry Huberts
  Cc: Netem, netdev@vger.kernel.org, core-users@pf.itd.nrl.navy.mil

I had reorder problems with netem qdisc. I make use of the NRL CORE network emulator, which sets up virtual routers and links, build with network namespaces and netem. Typical CORE config for a n1<-->n2 link with rate (1024Kbps), delay (50ms) and no jitter:
  qdisc tbf 1: dev n1.eth0.222 root refcnt 2 rate 1024Kbit burst 2999b lat 488.6ms 
  qdisc netem 10: dev n1.eth0.222 parent 1:1 limit 1000 delay 50.0ms
  qdisc tbf 1: dev n2.eth0.222 root refcnt 2 rate 1024Kbit burst 2999b lat 488.6ms 
  qdisc netem 10: dev n2.eth0.222 parent 1:1 limit 1000 delay 50.0ms

I added jitter to this bi-directional link, e.g. 20ms. Now the delay for each packet is 50ms +/- 20ms is 30ms to 70ms.
However, this has some unexpected results: packets may be reordered. That is because the actual delay is calculated for each packet. Some packets have a larger delay, some have a smaller. If the inter-packet spacing is smaller than the time differences set by netem (up to 2x configured jitter), packets are reordered. In this example with +/20ms, this is the case with packet rate larger than 25pps. Reordering has bad effects on transport protocol throughputs. Reordering is less common on the Internet, so I don't want to emulate such. I don't say there is no reordering, I just say I don't want this netem behavior.

The netem guide mentions this unexpected results. It was caused by a change in version 1.1 (2.6.15). For people like me that do not want this, there is a work-around mentioned. However, this doesn't work anymore since somewhere before 2.6.31.
http://www.linuxfoundation.org/collaborate/workgroups/networking/netem
https://lists.linux-foundation.org/pipermail/netem/2011-April/001507.html

So what to do? The recent patch from Eric Dumazet eliminates jitter_reordering when netem rate is configured. Maybe not the clean approach, but acceptable. Ferry his patch eliminates jitter_reordering. Some people may use reorder by jitter, so this "feature" should not get removed. So Ferry his patch is not accepted. On the other hand, people like me are very confused by current behavior.

What we could do is keep existing features and describe what netem currently does. That is:
 - netem with delay and jitter may reorder packets, if inter packet spacing is smaller than jitter
 - reordering caused by delay and jitter can be turned off by using netem rate. rate can be set to very high value is no shaping is wanted.

Are we OK with this approach?
Adjust the linux-foundation & man page info? Who can do this?

Teco

^ permalink raw reply
* [PATCH net-next] net: tcp_probe: allow more advanced ingress filtering by mark
From: Daniel Borkmann @ 2013-08-23 14:16 UTC (permalink / raw)
  To: davem; +Cc: netdev

Currently, the tcp_probe snooper can either filter packets by a given
port (handed to the module via module parameter e.g. port=80) or lets
all TCP traffic pass (port=0, default). When a port is specified, the
port number is tested against the sk's source/destination port. Thus,
if one of them matches, the information will be further processed for
the log.

As this is quite limited, allow for more advanced filtering possibilities
which can facilitate debugging/analysis with the help of the tcp_probe
snooper. Therefore, similarly as added to BPF machine in commit 7e75f93e
("pkt_sched: ingress socket filter by mark"), add the possibility to
use skb->mark as a filter.

If the mark is not being used otherwise, this allows ingress filtering
by flow (e.g. in order to track updates from only a single flow, or a
subset of all flows for a given port) and other things such as dynamic
logging and reconfiguration without removing/re-inserting the tcp_probe
module, etc. Simple example:

  insmod net/ipv4/tcp_probe.ko fwmark=8888 full=1
  ...
  iptables -A INPUT -i eth4 -t mangle -p tcp --dport 22 \
           --sport 60952 -j MARK --set-mark 8888
  [... sampling interval ...]
  iptables -D INPUT -i eth4 -t mangle -p tcp --dport 22 \
           --sport 60952 -j MARK --set-mark 8888

The current option to filter by a given port is still being preserved. A
similar approach could be done for the sctp_probe module as a follow-up.

Signed-off-by: Daniel Borkmann <dborkman@redhat.com>
---
 net/ipv4/tcp_probe.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/tcp_probe.c b/net/ipv4/tcp_probe.c
index 301a3ef..622a437 100644
--- a/net/ipv4/tcp_probe.c
+++ b/net/ipv4/tcp_probe.c
@@ -46,6 +46,10 @@ static unsigned int bufsize __read_mostly = 4096;
 MODULE_PARM_DESC(bufsize, "Log buffer size in packets (4096)");
 module_param(bufsize, uint, 0);
 
+static unsigned int fwmark __read_mostly = 0;
+MODULE_PARM_DESC(fwmark, "skb mark to match (0=no mark)");
+module_param(fwmark, uint, 0);
+
 static int full __read_mostly;
 MODULE_PARM_DESC(full, "Full log (1=every ack packet received,  0=only cwnd changes)");
 module_param(full, int, 0);
@@ -124,9 +128,11 @@ static int jtcp_rcv_established(struct sock *sk, struct sk_buff *skb,
 	const struct tcp_sock *tp = tcp_sk(sk);
 	const struct inet_sock *inet = inet_sk(sk);
 
-	/* Only update if port matches */
-	if ((port == 0 || ntohs(inet->inet_dport) == port ||
-	     ntohs(inet->inet_sport) == port) &&
+	/* Only update if port or skb mark matches */
+	if (((port == 0 && fwmark == 0) ||
+	     ntohs(inet->inet_dport) == port ||
+	     ntohs(inet->inet_sport) == port ||
+	     (fwmark > 0 && skb->mark == fwmark)) &&
 	    (full || tp->snd_cwnd != tcp_probe.lastcwnd)) {
 
 		spin_lock(&tcp_probe.lock);
@@ -284,7 +290,8 @@ static __init int tcpprobe_init(void)
 	if (ret)
 		goto err1;
 
-	pr_info("probe registered (port=%d) bufsize=%u\n", port, bufsize);
+	pr_info("probe registered (port=%d/fwmark=%u) bufsize=%u\n",
+		port, fwmark, bufsize);
 	return 0;
  err1:
 	remove_proc_entry(procname, init_net.proc_net);
-- 
1.7.11.7

^ permalink raw reply related
* [PATCH v4 5/5] ARM: dts: am33xx: adopt to cpsw changes
From: Daniel Mack @ 2013-08-23 14:16 UTC (permalink / raw)
  To: netdev
  Cc: bcousson, nsekhar, sergei.shtylyov, davem, ujhelyi.m,
	mugunthanvnm, vaibhav.bedia, d-gerlach, linux-arm-kernel,
	linux-omap, devicetree, Daniel Mack
In-Reply-To: <1377267365-24057-1-git-send-email-zonque@gmail.com>

This third memory region just denotes one single register in the CONTROL
module block. The driver uses that in order to set the correct physical
ethernet interface modes.

Also update the compatible string to make use of the am335x specific
features of the cpsw driver.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 arch/arm/boot/dts/am33xx.dtsi | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm/boot/dts/am33xx.dtsi b/arch/arm/boot/dts/am33xx.dtsi
index a785b95..c7b41ae 100644
--- a/arch/arm/boot/dts/am33xx.dtsi
+++ b/arch/arm/boot/dts/am33xx.dtsi
@@ -446,7 +446,7 @@
 		};
 
 		mac: ethernet@4a100000 {
-			compatible = "ti,cpsw";
+			compatible = "ti,am3352-cpsw", "ti,cpsw";
 			ti,hwmods = "cpgmac0";
 			cpdma_channels = <8>;
 			ale_entries = <1024>;
@@ -459,7 +459,8 @@
 			cpts_clock_mult = <0x80000000>;
 			cpts_clock_shift = <29>;
 			reg = <0x4a100000 0x800
-			       0x4a101200 0x100>;
+			       0x4a101200 0x100
+			       0x44e10650 0x4>;
 			#address-cells = <1>;
 			#size-cells = <1>;
 			interrupt-parent = <&intc>;
-- 
1.8.3.1

^ permalink raw reply related
* [PATCH v4 4/5] net: ethernet: cpsw: add support for hardware interface mode config
From: Daniel Mack @ 2013-08-23 14:16 UTC (permalink / raw)
  To: netdev
  Cc: bcousson, nsekhar, sergei.shtylyov, davem, ujhelyi.m,
	mugunthanvnm, vaibhav.bedia, d-gerlach, linux-arm-kernel,
	linux-omap, devicetree, Daniel Mack
In-Reply-To: <1377267365-24057-1-git-send-email-zonque@gmail.com>

The cpsw currently lacks code to properly set up the hardware interface
mode on AM33xx. Other platforms might be equally affected.

Usually, the bootloader will configure the control module register, so
probably that's why such support wasn't needed in the past. In suspend
mode though, this register is modified, and so it needs reprogramming
after resume.

This patch adds code that makes use of the previously added and optional
support for passing the control mode register, and configures the
correct register bits when the slave is opened.

The AM33xx also has a bit for each slave to configure the RMII reference
clock direction. Setting it is now supported by a per-slave DT property.

This code path introducted by this patch is currently exclusive for
am33xx.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 Documentation/devicetree/bindings/net/cpsw.txt |  2 +
 drivers/net/ethernet/ti/cpsw.c                 | 61 ++++++++++++++++++++++++++
 drivers/net/ethernet/ti/cpsw.h                 |  1 +
 3 files changed, 64 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
index b717458..0895a51 100644
--- a/Documentation/devicetree/bindings/net/cpsw.txt
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -34,6 +34,8 @@ Required properties:
 - phy_id		: Specifies slave phy id
 - phy-mode		: The interface between the SoC and the PHY (a string
 			  that of_get_phy_mode() can understand)
+- ti,rmii-clock-ext	: If present, the driver will configure the RMII
+			  interface to external clock usage
 - mac-address		: Specifies slave MAC address
 
 Optional properties:
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 73c44cb6..86b8f7a 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -138,6 +138,13 @@ do {								\
 #define CPSW_CMINTMAX_INTVL	(1000 / CPSW_CMINTMIN_CNT)
 #define CPSW_CMINTMIN_INTVL	((1000 / CPSW_CMINTMAX_CNT) + 1)
 
+#define AM33XX_GMII_SEL_MODE_MII	(0)
+#define AM33XX_GMII_SEL_MODE_RMII	(1)
+#define AM33XX_GMII_SEL_MODE_RGMII	(2)
+
+#define AM33XX_GMII_SEL_RMII2_IO_CLK_EN	BIT(7)
+#define AM33XX_GMII_SEL_RMII1_IO_CLK_EN	BIT(6)
+
 #define cpsw_enable_irq(priv)	\
 	do {			\
 		u32 i;		\
@@ -980,6 +987,56 @@ static inline void cpsw_add_dual_emac_def_ale_entries(
 		priv->host_port, ALE_VLAN, slave->port_vlan);
 }
 
+static void cpsw_set_phy_interface_mode(struct cpsw_slave *slave,
+					struct cpsw_priv *priv)
+{
+	u32 reg, mask, mode = 0;
+
+	switch (priv->data.hw_type) {
+	case CPSW_TYPE_AM33XX:
+		if (!priv->gmii_sel_reg)
+			break;
+
+		reg = readl(priv->gmii_sel_reg);
+
+		if (slave->phy) {
+			switch (slave->phy->interface) {
+			case PHY_INTERFACE_MODE_MII:
+			default:
+				mode = AM33XX_GMII_SEL_MODE_MII;
+				break;
+			case PHY_INTERFACE_MODE_RMII:
+				mode = AM33XX_GMII_SEL_MODE_RMII;
+				break;
+			case PHY_INTERFACE_MODE_RGMII:
+				mode = AM33XX_GMII_SEL_MODE_RGMII;
+				break;
+			};
+		}
+
+		mask = 0x3 << (slave->slave_num * 2) |
+		       BIT(slave->slave_num + 6);
+		mode <<= slave->slave_num * 2;
+
+		if (slave->data->rmii_clock_external) {
+			if (slave->slave_num == 0)
+				mode |= AM33XX_GMII_SEL_RMII1_IO_CLK_EN;
+			else
+				mode |= AM33XX_GMII_SEL_RMII2_IO_CLK_EN;
+		}
+
+		reg &= ~mask;
+		reg |= mode;
+
+		writel(reg, priv->gmii_sel_reg);
+		break;
+
+	default:
+		break;
+
+	}
+}
+
 static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
 {
 	char name[32];
@@ -1028,6 +1085,8 @@ static void cpsw_slave_open(struct cpsw_slave *slave, struct cpsw_priv *priv)
 			 slave->phy->phy_id);
 		phy_start(slave->phy);
 	}
+
+	cpsw_set_phy_interface_mode(slave, priv);
 }
 
 static inline void cpsw_add_default_vlan(struct cpsw_priv *priv)
@@ -1823,6 +1882,8 @@ static int cpsw_probe_dt(struct cpsw_platform_data *data,
 			memcpy(slave_data->mac_addr, mac_addr, ETH_ALEN);
 
 		slave_data->phy_if = of_get_phy_mode(slave_node);
+		if (of_find_property(slave_node, "ti,rmii-clock-ext", NULL))
+			slave_data->rmii_clock_external = true;
 
 		if (data->dual_emac) {
 			if (of_property_read_u32(slave_node, "dual_emac_res_vlan",
diff --git a/drivers/net/ethernet/ti/cpsw.h b/drivers/net/ethernet/ti/cpsw.h
index 96c374a..3baa2350 100644
--- a/drivers/net/ethernet/ti/cpsw.h
+++ b/drivers/net/ethernet/ti/cpsw.h
@@ -19,6 +19,7 @@
 struct cpsw_slave_data {
 	char		phy_id[MII_BUS_ID_SIZE];
 	int		phy_if;
+	bool		rmii_clock_external;
 	u8		mac_addr[ETH_ALEN];
 	u16		dual_emac_res_vlan;	/* Reserved VLAN for DualEMAC */
 };
-- 
1.8.3.1

^ permalink raw reply related
* [PATCH v4 3/5] net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
From: Daniel Mack @ 2013-08-23 14:16 UTC (permalink / raw)
  To: netdev
  Cc: bcousson, nsekhar, sergei.shtylyov, davem, ujhelyi.m,
	mugunthanvnm, vaibhav.bedia, d-gerlach, linux-arm-kernel,
	linux-omap, devicetree, Daniel Mack
In-Reply-To: <1377267365-24057-1-git-send-email-zonque@gmail.com>

In order to support features that are specific to the AM335x IP, we have
to add hardware types and another compatible string.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 Documentation/devicetree/bindings/net/cpsw.txt |  3 ++-
 drivers/net/ethernet/ti/cpsw.c                 | 32 ++++++++++++++++++++------
 drivers/net/ethernet/ti/cpsw.h                 |  1 +
 3 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
index 4e5ca54..b717458 100644
--- a/Documentation/devicetree/bindings/net/cpsw.txt
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -2,7 +2,8 @@ TI SoC Ethernet Switch Controller Device Tree Bindings
 ------------------------------------------------------
 
 Required properties:
-- compatible		: Should be "ti,cpsw"
+- compatible		: Should be "ti,cpsw" for generic cpsw support, or
+			  "ti,am3352-cpsw" for AM3352 SoCs
 - reg			: physical base address and size of the cpsw
 			  registers map.
 			  An optional third memory region can be supplied if
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 7a25ff4..73c44cb6 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -155,6 +155,11 @@ do {								\
 		((priv->data.dual_emac) ? priv->emac_port :	\
 		priv->data.active_slave)
 
+enum {
+	CPSW_TYPE_GENERIC,
+	CPSW_TYPE_AM33XX
+};
+
 static int debug_level;
 module_param(debug_level, int, 0);
 MODULE_PARM_DESC(debug_level, "cpsw debug level (NETIF_MSG bits)");
@@ -1692,17 +1697,36 @@ static void cpsw_slave_init(struct cpsw_slave *slave, struct cpsw_priv *priv,
 	slave->port_vlan = data->dual_emac_res_vlan;
 }
 
+static const struct of_device_id cpsw_of_mtable[] = {
+	{
+		.compatible	= "ti,am3352-cpsw",
+		.data		= (void *) CPSW_TYPE_AM33XX
+	}, {
+		.compatible	= "ti,cpsw",
+		.data		= (void *) CPSW_TYPE_GENERIC
+	},
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, cpsw_of_mtable);
+
 static int cpsw_probe_dt(struct cpsw_platform_data *data,
 			 struct platform_device *pdev)
 {
 	struct device_node *node = pdev->dev.of_node;
+	const struct of_device_id *match;
 	struct device_node *slave_node;
+	unsigned long match_data;
 	int i = 0, ret;
 	u32 prop;
 
-	if (!node)
+	match = of_match_device(cpsw_of_mtable, &pdev->dev);
+
+	if (!node || !match)
 		return -EINVAL;
 
+	match_data = (unsigned long) match->data;
+	data->hw_type = match_data;
+
 	if (of_property_read_u32(node, "slaves", &prop)) {
 		pr_err("Missing slaves property in the DT.\n");
 		return -EINVAL;
@@ -2228,12 +2252,6 @@ static const struct dev_pm_ops cpsw_pm_ops = {
 	.resume		= cpsw_resume,
 };
 
-static const struct of_device_id cpsw_of_mtable[] = {
-	{ .compatible = "ti,cpsw", },
-	{ /* sentinel */ },
-};
-MODULE_DEVICE_TABLE(of, cpsw_of_mtable);
-
 static struct platform_driver cpsw_driver = {
 	.driver = {
 		.name	 = "cpsw",
diff --git a/drivers/net/ethernet/ti/cpsw.h b/drivers/net/ethernet/ti/cpsw.h
index eb3e101..96c374a 100644
--- a/drivers/net/ethernet/ti/cpsw.h
+++ b/drivers/net/ethernet/ti/cpsw.h
@@ -37,6 +37,7 @@ struct cpsw_platform_data {
 	u32	mac_control;	/* Mac control register */
 	u16	default_vlan;	/* Def VLAN for ALE lookup in VLAN aware mode*/
 	bool	dual_emac;	/* Enable Dual EMAC mode */
+	u32	hw_type;	/* hardware type as specified in 'compatible' */
 };
 
 #endif /* __CPSW_H__ */
-- 
1.8.3.1

^ permalink raw reply related
* [PATCH v4 0/5] cpsw: support for control module register
From: Daniel Mack @ 2013-08-23 14:16 UTC (permalink / raw)
  To: netdev
  Cc: bcousson, nsekhar, sergei.shtylyov, davem, ujhelyi.m,
	mugunthanvnm, vaibhav.bedia, d-gerlach, linux-arm-kernel,
	linux-omap, devicetree, Daniel Mack

v3 -> v4:
	* use IS_ERR() to check for failed devm_ioremap_resource()
	  calls (reported by Sergei Shtylyov)

v2 -> v3:
	* swap "ti,am3352-cpsw" and "ti,cpsw" to work around a matching
	  bug (reported by Sekhar)

v1 -> v2:
	* combine devm_request_mem_region() and devm_ioremap() and use
	  devm_ioremap_resource() (reported by Sergei Shtylyov)
	* fix multi-line comment style (reported by Sergei Shtylyov)
	* fix ti,rmii-clock-ext property name (reported by Sekhar)
	* rebased to net-next (reported by Mugunthan V N, David Miller)
	* add a new compatible type, and handle AM33xx specific
	  registers that way (reported by Sekhar)
	* move gmii_sel_reg modifications to the open routine
	  (reported by Mugunthan V N)


Daniel Mack (5):
  net: ethernet: cpsw: switch to devres allocations
  net: ethernet: cpsw: add optional third memory region for CONTROL
    module
  net: ethernet: cpsw: introduce ti,am3352-cpsw compatible string
  net: ethernet: cpsw: add support for hardware interface mode config
  ARM: dts: am33xx: adopt to cpsw changes

 Documentation/devicetree/bindings/net/cpsw.txt |  10 +-
 arch/arm/boot/dts/am33xx.dtsi                  |   5 +-
 drivers/net/ethernet/ti/cpsw.c                 | 254 ++++++++++++++-----------
 drivers/net/ethernet/ti/cpsw.h                 |   2 +
 4 files changed, 159 insertions(+), 112 deletions(-)

-- 
1.8.3.1

^ permalink raw reply
* [PATCH v4 2/5] net: ethernet: cpsw: add optional third memory region for CONTROL module
From: Daniel Mack @ 2013-08-23 14:16 UTC (permalink / raw)
  To: netdev
  Cc: bcousson, nsekhar, sergei.shtylyov, davem, ujhelyi.m,
	mugunthanvnm, vaibhav.bedia, d-gerlach, linux-arm-kernel,
	linux-omap, devicetree, Daniel Mack
In-Reply-To: <1377267365-24057-1-git-send-email-zonque@gmail.com>

At least the AM33xx SoC has a control module register to configure
details such as the hardware ethernet interface mode.

I'm not sure whether all SoCs which feature the cpsw block have such a
register, so that third memory region is considered optional for now.

Signed-off-by: Daniel Mack <zonque@gmail.com>
---
 Documentation/devicetree/bindings/net/cpsw.txt |  5 ++++-
 drivers/net/ethernet/ti/cpsw.c                 | 16 ++++++++++++++++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
index 05d660e..4e5ca54 100644
--- a/Documentation/devicetree/bindings/net/cpsw.txt
+++ b/Documentation/devicetree/bindings/net/cpsw.txt
@@ -4,7 +4,10 @@ TI SoC Ethernet Switch Controller Device Tree Bindings
 Required properties:
 - compatible		: Should be "ti,cpsw"
 - reg			: physical base address and size of the cpsw
-			  registers map
+			  registers map.
+			  An optional third memory region can be supplied if
+			  the platform has a control module register to
+			  configure phy interface details
 - interrupts		: property with a value describing the interrupt
 			  number
 - interrupt-parent	: The parent interrupt controller
diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
index 849af52..7a25ff4 100644
--- a/drivers/net/ethernet/ti/cpsw.c
+++ b/drivers/net/ethernet/ti/cpsw.c
@@ -372,6 +372,7 @@ struct cpsw_priv {
 	struct cpsw_platform_data	data;
 	struct cpsw_ss_regs __iomem	*regs;
 	struct cpsw_wr_regs __iomem	*wr_regs;
+	u32 __iomem			*gmii_sel_reg;
 	u8 __iomem			*hw_stats;
 	struct cpsw_host_regs __iomem	*host_port_regs;
 	u32				msg_enable;
@@ -1999,6 +2000,21 @@ static int cpsw_probe(struct platform_device *pdev)
 		goto clean_runtime_disable_ret;
 	}
 
+	/* If the control memory region is unspecified, continue without it.
+	 * If it is specified, but we're unable to reserve it, bail.
+	 */
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 2);
+	if (!res) {
+		dev_info(priv->dev, "error getting control i/o resource\n");
+		goto no_gmii_sel;
+	}
+	priv->gmii_sel_reg = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(priv->gmii_sel_reg)) {
+		dev_err(priv->dev, "unable to map control i/o region\n");
+		goto clean_runtime_disable_ret;
+	}
+
+no_gmii_sel:
 	memset(&dma_params, 0, sizeof(dma_params));
 	memset(&ale_params, 0, sizeof(ale_params));
 
-- 
1.8.3.1


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