* Re: [BUG] 3.19-rc1 net: less interrupt masking in NAPI
From: Oded Gabbay @ 2015-01-10 21:43 UTC (permalink / raw)
To: Eric Dumazet
Cc: David S. Miller, Eric Dumazet, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, Willem de Bruijn, Bridgman, John,
Elifaz, Dana
In-Reply-To: <54B199EB.1080004@amd.com>
On 01/10/2015 11:30 PM, Oded Gabbay wrote:
>
>
> On 01/10/2015 10:58 PM, Eric Dumazet wrote:
>> On Sat, 2015-01-10 at 22:39 +0200, Oded Gabbay wrote:
>>> Hi,
>>>
>>> Commit d75b1ade567ffab085e8adbbdacf0092d10cd09c breaks my "Qualcomm Atheros
>>> AR8161 Gigabit Ethernet (rev 10)" Ethernet controller, which is handled by
>>> the alx network driver.
>>>
>>> ogabbay@odedg-ubuntu:~$ lspci -s 01:00.0 -k
>>> 01:00.0 Ethernet controller:
>>> Qualcomm Atheros AR8161 Gigabit Ethernet (rev 10)
>>> Subsystem: Qualcomm Atheros Device 1071
>>> Kernel driver in use: alx
>>>
>>> I have this controller on a mobile platform of AMD APU Kaveri, which I use
>>> to test amdkfd, and from 3.19-rc1 the network stopped working when trying to
>>> transfer files through scp or nfs.
>>>
>>> I bisected the kernel (from 3.18.0 to 3.19-rc1) and reached this commit.
>>>
>>> Here is the log of the bisect:
>>>
>>> git bisect start
>>> # bad: [97bf6af1f928216fd6c5a66e8a57bfa95a659672] Linux 3.19-rc1
>>> git bisect bad 97bf6af1f928216fd6c5a66e8a57bfa95a659672
>>>
>>> # good: [b2776bf7149bddd1f4161f14f79520f17fc1d71d] Linux 3.18
>>> git bisect good b2776bf7149bddd1f4161f14f79520f17fc1d71d
>>>
>>> # bad: [70e71ca0af244f48a5dcf56dc435243792e3a495] Merge
>>> git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next
>>> git bisect bad 70e71ca0af244f48a5dcf56dc435243792e3a495
>>>
>>> # good: [e28870f9b3e92cd3570925089c6bb789c2603bc4] Merge tag
>>> 'backlight-for-linus-3.19' of
>>> git://git.kernel.org/pub/scm/linux/kernel/git/lee/backlight
>>> git bisect good e28870f9b3e92cd3570925089c6bb789c2603bc4
>>>
>>> # bad: [450fa21942fe2c37f0c9f52d1a33bbc081eee288] sh_eth: Remove redundant
>>> alignment adjustment
>>> git bisect bad 450fa21942fe2c37f0c9f52d1a33bbc081eee288
>>>
>>> # bad: [5c8d19da950861d0482abc0ac3481acca34b008f] e100e: use
>>> netdev_rss_key_fill() helper
>>> git bisect bad 5c8d19da950861d0482abc0ac3481acca34b008f
>>>
>>> # good: [bf515fb11ab539c76d04f0e3c5216ed41f41d81f] Merge tag
>>> 'mac80211-next-for-john-2014-11-04' of
>>> git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next
>>> git bisect good bf515fb11ab539c76d04f0e3c5216ed41f41d81f
>>>
>>> # bad: [2c99cd914d4fed9160d98849c9dd38034616768e] Merge branch 'amd-xgbe-next'
>>> git bisect bad 2c99cd914d4fed9160d98849c9dd38034616768e
>>>
>>> # good: [3d762a0f0ab9cb4a6b5993db3ce56c92f9f90ab2] net: dsa: Add support for
>>> reading switch registers with ethtool
>>> git bisect good 3d762a0f0ab9cb4a6b5993db3ce56c92f9f90ab2
>>>
>>> # bad: [8ce0c8254f15229aa99fc6c04141f28c446e5f8c] Merge branch 'master' of
>>> git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next
>>> git bisect bad 8ce0c8254f15229aa99fc6c04141f28c446e5f8c
>>>
>>> # good: [f0c65567b3c1b23f79e8a49139580a3872a68d1f] Merge branch
>>> 'sunvnet-multi-tx-queue'
>>> git bisect good f0c65567b3c1b23f79e8a49139580a3872a68d1f
>>>
>>> # bad: [547f2735c20023d7b50a791b1b17cacb652e9237] Merge branch 'mlx4-next'
>>> git bisect bad 547f2735c20023d7b50a791b1b17cacb652e9237
>>>
>>> # good: [4cdb1e2e3d3495423db558d3bb7ed11d66aabce7] net: shrink struct
>>> softnet_data
>>> git bisect good 4cdb1e2e3d3495423db558d3bb7ed11d66aabce7
>>>
>>> # bad: [0a98455666ec87378148a1dde97f1ce5baf75a64] net/mlx4_core: Protect
>>> port type setting by mutex
>>> git bisect bad 0a98455666ec87378148a1dde97f1ce5baf75a64
>>>
>>> # bad: [6e8066999800d90d52af5c84ac49ebf683d14cdc] net/mlx4_core: Prevent VF
>>> from changing port configuration
>>> git bisect bad 6e8066999800d90d52af5c84ac49ebf683d14cdc
>>>
>>> # bad: [d75b1ade567ffab085e8adbbdacf0092d10cd09c] net: less interrupt
>>> masking in NAPI
>>> git bisect bad d75b1ade567ffab085e8adbbdacf0092d10cd09c
>>>
>>> # first bad commit: [d75b1ade567ffab085e8adbbdacf0092d10cd09c]
>>> net: less interrupt masking in NAPI
>>>
>>> Could you please solve this issue as it renders my board quite useless.
>>>
>>
>> Thanks for the report and bisection !
>>
>> Could you try following fix ?
>>
>> diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
>> index e398eda07298..209c40765e0d 100644
>> --- a/drivers/net/ethernet/atheros/alx/main.c
>> +++ b/drivers/net/ethernet/atheros/alx/main.c
>> @@ -272,7 +272,7 @@ static int alx_poll(struct napi_struct *napi, int budget)
>> alx_clean_rx_irq(alx, budget);
>>
>> if (!complete)
>> - return 1;
>> + return budget;
>>
>> napi_complete(&alx->napi);
>>
>>
>>
>>
>>
> Yes, no problem.
> I will update on the result.
>
> Oded
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
Hi Eric,
Your patch fixed the problem.
Thanks for the quick help!
Oded
^ permalink raw reply
* Re: [BUG] 3.19-rc1 net: less interrupt masking in NAPI
From: Eric Dumazet @ 2015-01-10 21:50 UTC (permalink / raw)
To: Oded Gabbay, Johannes Berg
Cc: David S. Miller, Eric Dumazet, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, Willem de Bruijn, Bridgman, John,
Elifaz, Dana
In-Reply-To: <54B199EB.1080004@amd.com>
On Sat, 2015-01-10 at 23:30 +0200, Oded Gabbay wrote:
> Yes, no problem.
> I will update on the result.
Please try this more complete patch, solving the TX pressure problem as
well, and not lying about NAPI budget. thanks !
diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
index e398eda07298..5f05b387c0a7 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -184,15 +184,16 @@ static void alx_schedule_reset(struct alx_priv *alx)
schedule_work(&alx->reset_wk);
}
-static bool alx_clean_rx_irq(struct alx_priv *alx, int budget)
+static int alx_clean_rx_irq(struct alx_priv *alx, int budget)
{
struct alx_rx_queue *rxq = &alx->rxq;
struct alx_rrd *rrd;
struct alx_buffer *rxb;
struct sk_buff *skb;
u16 length, rfd_cleaned = 0;
+ int work = 0;
- while (budget > 0) {
+ while (work < budget) {
rrd = &rxq->rrd[rxq->rrd_read_idx];
if (!(rrd->word3 & cpu_to_le32(1 << RRD_UPDATED_SHIFT)))
break;
@@ -243,7 +244,7 @@ static bool alx_clean_rx_irq(struct alx_priv *alx, int budget)
}
napi_gro_receive(&alx->napi, skb);
- budget--;
+ work++;
next_pkt:
if (++rxq->read_idx == alx->rx_ringsz)
@@ -258,21 +259,22 @@ next_pkt:
if (rfd_cleaned)
alx_refill_rx_ring(alx, GFP_ATOMIC);
- return budget > 0;
+ return work;
}
static int alx_poll(struct napi_struct *napi, int budget)
{
struct alx_priv *alx = container_of(napi, struct alx_priv, napi);
struct alx_hw *hw = &alx->hw;
- bool complete = true;
unsigned long flags;
+ bool tx_complete;
+ int work;
- complete = alx_clean_tx_irq(alx) &&
- alx_clean_rx_irq(alx, budget);
+ tx_complete = alx_clean_tx_irq(alx);
+ work = alx_clean_rx_irq(alx, budget);
- if (!complete)
- return 1;
+ if (!tx_complete || work == budget)
+ return budget;
napi_complete(&alx->napi);
@@ -284,7 +286,7 @@ static int alx_poll(struct napi_struct *napi, int budget)
alx_post_write(hw);
- return 0;
+ return work;
}
static irqreturn_t alx_intr_handle(struct alx_priv *alx, u32 intr)
^ permalink raw reply related
* Re: Re: [PATCH] net: less interrupt masking in NAPI
From: Oded Gabbay @ 2015-01-10 20:27 UTC (permalink / raw)
To: davem, eric.dumazet; +Cc: netdev, willemb
In-Reply-To: <20141103.122538.387451917276174830.davem@davemloft.net>
On 2014/11/4 1:25, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@xxxxxxxxx>
> Date: Sun, 02 Nov 2014 06:19:33 -0800
>
>> From: Eric Dumazet <edumazet@xxxxxxxxxx>
>>
>> net_rx_action() can mask irqs a single time to transfert sd->poll_list
>> into a private list, for a very short duration.
>>
>> Then, napi_complete() can avoid masking irqs again,
>> and net_rx_action() only needs to mask irq again in slow path.
>>
>> This patch removes 2 couples of irq mask/unmask per typical NAPI run,
>> more if multiple napi were triggered.
>>
>> Note this also allows to give control back to caller (do_softirq())
>> more often, so that other softirq handlers can be called a bit earlier,
>> or ksoftirqd can be wakeup earlier under pressure.
>>
>> This was developed while testing an alternative to RX interrupt
>> mitigation to reduce latencies while keeping or improving GRO
>> aggregation on fast NIC.
>>
>> Idea is to test napi->gro_list at the end of a napi->poll() and
>> reschedule one NAPI poll, but after servicing a full round of
>> softirqs (timers, TX, rcu, ...). This will be allowed only if softirq
>> is currently serviced by idle task or ksoftirqd, and resched not needed.
>>
>> Signed-off-by: Eric Dumazet <edumazet@xxxxxxxxxx>
>
> Also applied, thanks Eric.
Hi,
Unfortunately, this patch breaks my "Qualcomm Atheros AR8161 Gigabit
Ethernet (rev 10)" Ethernet controller, which is handled by the alx
network driver.
ogabbay@odedg-ubuntu:~$ lspci -s 01:00.0 -k
01:00.0 Ethernet controller:
Qualcomm Atheros AR8161 Gigabit Ethernet (rev 10)
Subsystem: Qualcomm Atheros Device 1071
Kernel driver in use: alx
I have this controller on a mobile platform which I use to test amdkfd
on Kaveri AMD APU and from 3.19-rc1, the network stopped working when
trying to transfer files through scp or nfs.
I bisected the kernel (from 3.18.0 to 3.19-rc1) and reached this patch.
Here is the log of the bisect:
git bisect start
# bad: [97bf6af1f928216fd6c5a66e8a57bfa95a659672] Linux 3.19-rc1
git bisect bad 97bf6af1f928216fd6c5a66e8a57bfa95a659672
# good: [b2776bf7149bddd1f4161f14f79520f17fc1d71d] Linux 3.18
git bisect good b2776bf7149bddd1f4161f14f79520f17fc1d71d
# bad: [70e71ca0af244f48a5dcf56dc435243792e3a495] Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next
git bisect bad 70e71ca0af244f48a5dcf56dc435243792e3a495
# good: [e28870f9b3e92cd3570925089c6bb789c2603bc4] Merge tag 'backlight-for-linus-3.19' of git://git.kernel.org/pub/scm/linux/kernel/git/lee/backlight
git bisect good e28870f9b3e92cd3570925089c6bb789c2603bc4
# bad: [450fa21942fe2c37f0c9f52d1a33bbc081eee288] sh_eth: Remove redundant alignment adjustment
git bisect bad 450fa21942fe2c37f0c9f52d1a33bbc081eee288
# bad: [5c8d19da950861d0482abc0ac3481acca34b008f] e100e: use netdev_rss_key_fill() helper
git bisect bad 5c8d19da950861d0482abc0ac3481acca34b008f
# good: [bf515fb11ab539c76d04f0e3c5216ed41f41d81f] Merge tag 'mac80211-next-for-john-2014-11-04' of git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next
git bisect good bf515fb11ab539c76d04f0e3c5216ed41f41d81f
# bad: [2c99cd914d4fed9160d98849c9dd38034616768e] Merge branch 'amd-xgbe-next'
git bisect bad 2c99cd914d4fed9160d98849c9dd38034616768e
# good: [3d762a0f0ab9cb4a6b5993db3ce56c92f9f90ab2] net: dsa: Add support for reading switch registers with ethtool
git bisect good 3d762a0f0ab9cb4a6b5993db3ce56c92f9f90ab2
# bad: [8ce0c8254f15229aa99fc6c04141f28c446e5f8c] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/net-next
git bisect bad 8ce0c8254f15229aa99fc6c04141f28c446e5f8c
# good: [f0c65567b3c1b23f79e8a49139580a3872a68d1f] Merge branch 'sunvnet-multi-tx-queue'
git bisect good f0c65567b3c1b23f79e8a49139580a3872a68d1f
# bad: [547f2735c20023d7b50a791b1b17cacb652e9237] Merge branch 'mlx4-next'
git bisect bad 547f2735c20023d7b50a791b1b17cacb652e9237
# good: [4cdb1e2e3d3495423db558d3bb7ed11d66aabce7] net: shrink struct softnet_data
git bisect good 4cdb1e2e3d3495423db558d3bb7ed11d66aabce7
# bad: [0a98455666ec87378148a1dde97f1ce5baf75a64] net/mlx4_core: Protect port type setting by mutex
git bisect bad 0a98455666ec87378148a1dde97f1ce5baf75a64
# bad: [6e8066999800d90d52af5c84ac49ebf683d14cdc] net/mlx4_core: Prevent VF from changing port configuration
git bisect bad 6e8066999800d90d52af5c84ac49ebf683d14cdc
# bad: [d75b1ade567ffab085e8adbbdacf0092d10cd09c] net: less interrupt masking in NAPI
git bisect bad d75b1ade567ffab085e8adbbdacf0092d10cd09c
# first bad commit: [d75b1ade567ffab085e8adbbdacf0092d10cd09c]
net: less interrupt masking in NAPI
Could you please solve this issue as it renders my board quite useless.
If you need more info, please ask.
Thanks,
Oded
^ permalink raw reply
* Re: [BUG] 3.19-rc1 net: less interrupt masking in NAPI
From: Oded Gabbay @ 2015-01-10 22:05 UTC (permalink / raw)
To: Eric Dumazet, Johannes Berg
Cc: David S. Miller, Eric Dumazet, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, Willem de Bruijn, Bridgman, John,
Elifaz, Dana
In-Reply-To: <1420926614.5947.89.camel@edumazet-glaptop2.roam.corp.google.com>
On 01/10/2015 11:50 PM, Eric Dumazet wrote:
> On Sat, 2015-01-10 at 23:30 +0200, Oded Gabbay wrote:
>
>> Yes, no problem.
>> I will update on the result.
>
> Please try this more complete patch, solving the TX pressure problem as
> well, and not lying about NAPI budget. thanks !
>
>
> diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
> index e398eda07298..5f05b387c0a7 100644
> --- a/drivers/net/ethernet/atheros/alx/main.c
> +++ b/drivers/net/ethernet/atheros/alx/main.c
> @@ -184,15 +184,16 @@ static void alx_schedule_reset(struct alx_priv *alx)
> schedule_work(&alx->reset_wk);
> }
>
> -static bool alx_clean_rx_irq(struct alx_priv *alx, int budget)
> +static int alx_clean_rx_irq(struct alx_priv *alx, int budget)
> {
> struct alx_rx_queue *rxq = &alx->rxq;
> struct alx_rrd *rrd;
> struct alx_buffer *rxb;
> struct sk_buff *skb;
> u16 length, rfd_cleaned = 0;
> + int work = 0;
>
> - while (budget > 0) {
> + while (work < budget) {
> rrd = &rxq->rrd[rxq->rrd_read_idx];
> if (!(rrd->word3 & cpu_to_le32(1 << RRD_UPDATED_SHIFT)))
> break;
> @@ -243,7 +244,7 @@ static bool alx_clean_rx_irq(struct alx_priv *alx, int budget)
> }
>
> napi_gro_receive(&alx->napi, skb);
> - budget--;
> + work++;
>
> next_pkt:
> if (++rxq->read_idx == alx->rx_ringsz)
> @@ -258,21 +259,22 @@ next_pkt:
> if (rfd_cleaned)
> alx_refill_rx_ring(alx, GFP_ATOMIC);
>
> - return budget > 0;
> + return work;
> }
>
> static int alx_poll(struct napi_struct *napi, int budget)
> {
> struct alx_priv *alx = container_of(napi, struct alx_priv, napi);
> struct alx_hw *hw = &alx->hw;
> - bool complete = true;
> unsigned long flags;
> + bool tx_complete;
> + int work;
>
> - complete = alx_clean_tx_irq(alx) &&
> - alx_clean_rx_irq(alx, budget);
> + tx_complete = alx_clean_tx_irq(alx);
> + work = alx_clean_rx_irq(alx, budget);
>
> - if (!complete)
> - return 1;
> + if (!tx_complete || work == budget)
> + return budget;
>
> napi_complete(&alx->napi);
>
> @@ -284,7 +286,7 @@ static int alx_poll(struct napi_struct *napi, int budget)
>
> alx_post_write(hw);
>
> - return 0;
> + return work;
> }
>
> static irqreturn_t alx_intr_handle(struct alx_priv *alx, u32 intr)
>
>
Hi,
Checked it and its working.
Thanks again.
Please note that I only use this Ethernet controller for NFS and SCP, it is
not used for development of networking software of any kind, so maybe a more
extensive testing is needed.
Oded
^ permalink raw reply
* Re: [BUG] 3.19-rc1 net: less interrupt masking in NAPI
From: Oded Gabbay @ 2015-01-10 22:08 UTC (permalink / raw)
To: Eric Dumazet, Johannes Berg
Cc: David S. Miller, Eric Dumazet, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org, Willem de Bruijn, Bridgman, John,
Elifaz, Dana
In-Reply-To: <54B1A22C.2020301@amd.com>
On 01/11/2015 12:05 AM, Oded Gabbay wrote:
>
>
> On 01/10/2015 11:50 PM, Eric Dumazet wrote:
>> On Sat, 2015-01-10 at 23:30 +0200, Oded Gabbay wrote:
>>
>>> Yes, no problem.
>>> I will update on the result.
>>
>> Please try this more complete patch, solving the TX pressure problem as
>> well, and not lying about NAPI budget. thanks !
>>
>>
>> diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
>> index e398eda07298..5f05b387c0a7 100644
>> --- a/drivers/net/ethernet/atheros/alx/main.c
>> +++ b/drivers/net/ethernet/atheros/alx/main.c
>> @@ -184,15 +184,16 @@ static void alx_schedule_reset(struct alx_priv *alx)
>> schedule_work(&alx->reset_wk);
>> }
>>
>> -static bool alx_clean_rx_irq(struct alx_priv *alx, int budget)
>> +static int alx_clean_rx_irq(struct alx_priv *alx, int budget)
>> {
>> struct alx_rx_queue *rxq = &alx->rxq;
>> struct alx_rrd *rrd;
>> struct alx_buffer *rxb;
>> struct sk_buff *skb;
>> u16 length, rfd_cleaned = 0;
>> + int work = 0;
>>
>> - while (budget > 0) {
>> + while (work < budget) {
>> rrd = &rxq->rrd[rxq->rrd_read_idx];
>> if (!(rrd->word3 & cpu_to_le32(1 << RRD_UPDATED_SHIFT)))
>> break;
>> @@ -243,7 +244,7 @@ static bool alx_clean_rx_irq(struct alx_priv *alx, int budget)
>> }
>>
>> napi_gro_receive(&alx->napi, skb);
>> - budget--;
>> + work++;
>>
>> next_pkt:
>> if (++rxq->read_idx == alx->rx_ringsz)
>> @@ -258,21 +259,22 @@ next_pkt:
>> if (rfd_cleaned)
>> alx_refill_rx_ring(alx, GFP_ATOMIC);
>>
>> - return budget > 0;
>> + return work;
>> }
>>
>> static int alx_poll(struct napi_struct *napi, int budget)
>> {
>> struct alx_priv *alx = container_of(napi, struct alx_priv, napi);
>> struct alx_hw *hw = &alx->hw;
>> - bool complete = true;
>> unsigned long flags;
>> + bool tx_complete;
>> + int work;
>>
>> - complete = alx_clean_tx_irq(alx) &&
>> - alx_clean_rx_irq(alx, budget);
>> + tx_complete = alx_clean_tx_irq(alx);
>> + work = alx_clean_rx_irq(alx, budget);
>>
>> - if (!complete)
>> - return 1;
>> + if (!tx_complete || work == budget)
>> + return budget;
>>
>> napi_complete(&alx->napi);
>>
>> @@ -284,7 +286,7 @@ static int alx_poll(struct napi_struct *napi, int budget)
>>
>> alx_post_write(hw);
>>
>> - return 0;
>> + return work;
>> }
>>
>> static irqreturn_t alx_intr_handle(struct alx_priv *alx, u32 intr)
>>
>>
> Hi,
> Checked it and its working.
> Thanks again.
> Please note that I only use this Ethernet controller for NFS and SCP, it is
> not used for development of networking software of any kind, so maybe a more
> extensive testing is needed.
>
> Oded
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
But in any case, you can add:
Tested-by: Oded Gabbay <oded.gabbay@amd.com>
Oded
^ permalink raw reply
* /proc/net/dev regression
From: Carlos R. Mafra @ 2015-01-10 23:25 UTC (permalink / raw)
To: LKML; +Cc: Hauke Mehrtens, John W. Linville, netdev
I use a dockapp called 'wmnet' [1] to monitor the speed of
my internet connection and after the kernel v3.18 it does no
longer work properly (it still doesn't work in v3.19-rc3)
I bisected the problem and the culprit is this commit:
commit 6e094bd805a9b6ad2f5421125db8f604a166616c
Author: Rafał Miłecki <zajec5@gmail.com>
Date: Fri Sep 5 00:18:48 2014 +0200
bcma: move code for core registration into separate function
This cleans code a bit and will us to register cores in other places as
well. The only difference with this patch is using "core_index" for
setting device name.
The problem is caused by the different name that my wireless connection
receives after this patch:
wlp3s0b1 (after the patch)
wlp3s0 (before the patch)
because it affects the display of information in the file /proc/net/dev.
Before the patch the fields are all aligned:
[mafra@linux-g29b:wmnet]$ cat net_dev_good.txt
Inter-| Receive | Transmit
face |bytes packets errs drop fifo frame compressed multicast|bytes packets errs drop fifo colls carrier compressed
lo: 35916 332 0 0 0 0 0 0 35916 332 0 0 0 0 0 0
wlp3s0: 6406428 5794 0 0 0 0 0 0 426813 3778 0 0 0 0 0 0
but after the patch the fields are misaligned:
[mafra@linux-g29b:wmnet]$ cat net_dev_bad.txt
Inter-| Receive | Transmit
face |bytes packets errs drop fifo frame compressed multicast|bytes packets errs drop fifo colls carrier compressed
lo: 600 8 0 0 0 0 0 0 600 8 0 0 0 0 0 0
wlp3s0b1: 9266848 7298 0 0 0 0 0 0 372229 4030 0 0 0 0 0 0
And for some reason this change confuses 'wmnet'. Reading the source code
of 'wmnet' I found that it reads the packets as follows,
totalpackets_in = strtoul(&buffer[15], NULL, 10);
I am not sure if 'wmnet' could do this better (any suggestions?),
but the fact is that it was working before and now it is not.
Any help is greatly appreciated.
[1] wmnet can be found in http://repo.or.cz/w/dockapps.git
^ permalink raw reply
* Re: /proc/net/dev regression
From: Al Viro @ 2015-01-11 0:27 UTC (permalink / raw)
To: Carlos R. Mafra; +Cc: LKML, Hauke Mehrtens, John W. Linville, netdev
In-Reply-To: <20150110232518.GA3212@linux-g29b.site>
On Sat, Jan 10, 2015 at 11:25:18PM +0000, Carlos R. Mafra wrote:
> [mafra@linux-g29b:wmnet]$ cat net_dev_bad.txt
> Inter-| Receive | Transmit
> face |bytes packets errs drop fifo frame compressed multicast|bytes packets errs drop fifo colls carrier compressed
> lo: 600 8 0 0 0 0 0 0 600 8 0 0 0 0 0 0
> wlp3s0b1: 9266848 7298 0 0 0 0 0 0 372229 4030 0 0 0 0 0 0
>
> And for some reason this change confuses 'wmnet'. Reading the source code
> of 'wmnet' I found that it reads the packets as follows,
>
> totalpackets_in = strtoul(&buffer[15], NULL, 10);
>
> I am not sure if 'wmnet' could do this better (any suggestions?),
*snort*
well, yes - it's called scanf(). And if one is really, really nervous
about the overhead of <gasp> parsing a bunch of integers (as if fopen/
fgets/fclose alone won't cost enough to make constantly calling that
sucker a bad idea), just use ptr + <something> - 6 instead of
&buffer[<something>] in there. That thing has just found where the
colon was (and replaced it with NUL), so dealing with "the first field
turned out to be too long and shifted everything past it" isn't hard.
> but the fact is that it was working before and now it is not.
True. Mind you, the real issue is that this code expects the interface
names to be never longer than 6 characters, but then /proc/net/dev layout
strongly suggests that. Hell knows; it is a regression and it does
break real-world userland code. The only way to avoid that, AFAICS, is
to prohibit interface names longer than 6 chars ;-/
Lovely combination of crappy ABI (procfs file layout), crappy userland
code relying on details of said ABI out of sheer laziness and triggering
kernel change producing bloody long interface names...
Incidentally, sufficiently long interface name will produce other fun issues
for a docked app - it simply won't fit into 64x64 square on screen ;-)
^ permalink raw reply
* Re: /proc/net/dev regression
From: Carlos R. Mafra @ 2015-01-11 0:58 UTC (permalink / raw)
To: Al Viro; +Cc: LKML, Hauke Mehrtens, John W. Linville, netdev
In-Reply-To: <20150111002706.GC22149@ZenIV.linux.org.uk>
On Sun, 11 Jan 2015 at 0:27:06 +0000, Al Viro wrote:
> On Sat, Jan 10, 2015 at 11:25:18PM +0000, Carlos R. Mafra wrote:
> > [mafra@linux-g29b:wmnet]$ cat net_dev_bad.txt
> > Inter-| Receive | Transmit
> > face |bytes packets errs drop fifo frame compressed multicast|bytes packets errs drop fifo colls carrier compressed
> > lo: 600 8 0 0 0 0 0 0 600 8 0 0 0 0 0 0
> > wlp3s0b1: 9266848 7298 0 0 0 0 0 0 372229 4030 0 0 0 0 0 0
> >
> > And for some reason this change confuses 'wmnet'. Reading the source code
> > of 'wmnet' I found that it reads the packets as follows,
> >
> > totalpackets_in = strtoul(&buffer[15], NULL, 10);
> >
> > I am not sure if 'wmnet' could do this better (any suggestions?),
>
> *snort*
>
> well, yes - it's called scanf(). And if one is really, really nervous
> about the overhead of <gasp> parsing a bunch of integers (as if fopen/
> fgets/fclose alone won't cost enough to make constantly calling that
> sucker a bad idea), just use ptr + <something> - 6 instead of
> &buffer[<something>] in there. That thing has just found where the
> colon was (and replaced it with NUL), so dealing with "the first field
> turned out to be too long and shifted everything past it" isn't hard.
Alright! Thank you.
> > but the fact is that it was working before and now it is not.
>
> True. Mind you, the real issue is that this code expects the interface
> names to be never longer than 6 characters, but then /proc/net/dev layout
> strongly suggests that. Hell knows; it is a regression and it does
> break real-world userland code. The only way to avoid that, AFAICS, is
> to prohibit interface names longer than 6 chars ;-/
>
> Lovely combination of crappy ABI (procfs file layout), crappy userland
> code relying on details of said ABI out of sheer laziness and triggering
> kernel change producing bloody long interface names...
I won't mind just changing the crappy and fragile wmnet code and moving on.
I have already lost the 2 hours bisecting this anyway.
Thank you very much for your advice.
^ permalink raw reply
* Re: /proc/net/dev regression
From: Al Viro @ 2015-01-11 1:00 UTC (permalink / raw)
To: Carlos R. Mafra; +Cc: LKML, Hauke Mehrtens, John W. Linville, netdev
In-Reply-To: <20150111002706.GC22149@ZenIV.linux.org.uk>
On Sun, Jan 11, 2015 at 12:27:06AM +0000, Al Viro wrote:
> On Sat, Jan 10, 2015 at 11:25:18PM +0000, Carlos R. Mafra wrote:
> > [mafra@linux-g29b:wmnet]$ cat net_dev_bad.txt
> > Inter-| Receive | Transmit
> > face |bytes packets errs drop fifo frame compressed multicast|bytes packets errs drop fifo colls carrier compressed
> > lo: 600 8 0 0 0 0 0 0 600 8 0 0 0 0 0 0
> > wlp3s0b1: 9266848 7298 0 0 0 0 0 0 372229 4030 0 0 0 0 0 0
> >
> > And for some reason this change confuses 'wmnet'. Reading the source code
> > of 'wmnet' I found that it reads the packets as follows,
> >
> > totalpackets_in = strtoul(&buffer[15], NULL, 10);
> >
> > I am not sure if 'wmnet' could do this better (any suggestions?),
>
> *snort*
>
> well, yes - it's called scanf(). And if one is really, really nervous
> about the overhead of <gasp> parsing a bunch of integers (as if fopen/
> fgets/fclose alone won't cost enough to make constantly calling that
> sucker a bad idea), just use ptr + <something> - 6 instead of
> &buffer[<something>] in there. That thing has just found where the
> colon was (and replaced it with NUL), so dealing with "the first field
> turned out to be too long and shifted everything past it" isn't hard.
>
> > but the fact is that it was working before and now it is not.
>
> True. Mind you, the real issue is that this code expects the interface
> names to be never longer than 6 characters, but then /proc/net/dev layout
> strongly suggests that. Hell knows; it is a regression and it does
> break real-world userland code. The only way to avoid that, AFAICS, is
> to prohibit interface names longer than 6 chars ;-/
>
> Lovely combination of crappy ABI (procfs file layout), crappy userland
> code relying on details of said ABI out of sheer laziness and triggering
> kernel change producing bloody long interface names...
>
> Incidentally, sufficiently long interface name will produce other fun issues
> for a docked app - it simply won't fit into 64x64 square on screen ;-)
Mind you, assuming that columns will align is obviously broken - the producing
side of that thing is
seq_printf(seq, "%6s: %7llu %7llu %4llu %4llu %4llu %5llu %10llu %9llu "
"%8llu %7llu %4llu %4llu %4llu %5llu %7llu %10llu\n",
dev->name, stats->rx_bytes, stats->rx_packets,
stats->rx_errors,
stats->rx_dropped + stats->rx_missed_errors,
stats->rx_fifo_errors,
stats->rx_length_errors + stats->rx_over_errors +
stats->rx_crc_errors + stats->rx_frame_errors,
stats->rx_compressed, stats->multicast,
stats->tx_bytes, stats->tx_packets,
stats->tx_errors, stats->tx_dropped,
stats->tx_fifo_errors, stats->collisions,
stats->tx_carrier_errors +
stats->tx_aborted_errors +
stats->tx_window_errors +
stats->tx_heartbeat_errors,
stats->tx_compressed);
To start with, expecting the ->rx_bytes to remain a 7-digit number is somewhat,
er, odd. Long interace names be damned, the columns will not stay aligned,
no matter what. Unless your interface vanishes as soon as it has sent
or received 10 megabytes, that is...
^ permalink raw reply
* Re: /proc/net/dev regression
From: Carlos R. Mafra @ 2015-01-11 1:33 UTC (permalink / raw)
To: Al Viro; +Cc: LKML, Hauke Mehrtens, John W. Linville, netdev
In-Reply-To: <20150111010036.GD22149@ZenIV.linux.org.uk>
On Sun, 11 Jan 2015 at 1:00:36 +0000, Al Viro wrote:
> On Sun, Jan 11, 2015 at 12:27:06AM +0000, Al Viro wrote:
> > On Sat, Jan 10, 2015 at 11:25:18PM +0000, Carlos R. Mafra wrote:
> > > [mafra@linux-g29b:wmnet]$ cat net_dev_bad.txt
> > > Inter-| Receive | Transmit
> > > face |bytes packets errs drop fifo frame compressed multicast|bytes packets errs drop fifo colls carrier compressed
> > > lo: 600 8 0 0 0 0 0 0 600 8 0 0 0 0 0 0
> > > wlp3s0b1: 9266848 7298 0 0 0 0 0 0 372229 4030 0 0 0 0 0 0
> > >
> > > And for some reason this change confuses 'wmnet'. Reading the source code
> > > of 'wmnet' I found that it reads the packets as follows,
> > >
> > > totalpackets_in = strtoul(&buffer[15], NULL, 10);
> > >
> > > I am not sure if 'wmnet' could do this better (any suggestions?),
> >
> > *snort*
> >
> > well, yes - it's called scanf(). And if one is really, really nervous
> > about the overhead of <gasp> parsing a bunch of integers (as if fopen/
> > fgets/fclose alone won't cost enough to make constantly calling that
> > sucker a bad idea), just use ptr + <something> - 6 instead of
> > &buffer[<something>] in there. That thing has just found where the
> > colon was (and replaced it with NUL), so dealing with "the first field
> > turned out to be too long and shifted everything past it" isn't hard.
> >
> > > but the fact is that it was working before and now it is not.
> >
> > True. Mind you, the real issue is that this code expects the interface
> > names to be never longer than 6 characters, but then /proc/net/dev layout
> > strongly suggests that. Hell knows; it is a regression and it does
> > break real-world userland code. The only way to avoid that, AFAICS, is
> > to prohibit interface names longer than 6 chars ;-/
> >
> > Lovely combination of crappy ABI (procfs file layout), crappy userland
> > code relying on details of said ABI out of sheer laziness and triggering
> > kernel change producing bloody long interface names...
> >
> > Incidentally, sufficiently long interface name will produce other fun issues
> > for a docked app - it simply won't fit into 64x64 square on screen ;-)
>
> Mind you, assuming that columns will align is obviously broken - the producing
> side of that thing is
> seq_printf(seq, "%6s: %7llu %7llu %4llu %4llu %4llu %5llu %10llu %9llu "
> "%8llu %7llu %4llu %4llu %4llu %5llu %7llu %10llu\n",
> dev->name, stats->rx_bytes, stats->rx_packets,
> stats->rx_errors,
> stats->rx_dropped + stats->rx_missed_errors,
> stats->rx_fifo_errors,
> stats->rx_length_errors + stats->rx_over_errors +
> stats->rx_crc_errors + stats->rx_frame_errors,
> stats->rx_compressed, stats->multicast,
> stats->tx_bytes, stats->tx_packets,
> stats->tx_errors, stats->tx_dropped,
> stats->tx_fifo_errors, stats->collisions,
> stats->tx_carrier_errors +
> stats->tx_aborted_errors +
> stats->tx_window_errors +
> stats->tx_heartbeat_errors,
> stats->tx_compressed);
> To start with, expecting the ->rx_bytes to remain a 7-digit number is somewhat,
> er, odd. Long interace names be damned, the columns will not stay aligned,
> no matter what. Unless your interface vanishes as soon as it has sent
> or received 10 megabytes, that is...
I think the problem with wmnet is not that it was expecting the fields
to be aligned because it never had problems before (when definitely more
than 10 megabytes were received, wmnet is crappy but not _that_ crappy).
I think the problem really was here,
totalbytes_in = strtoul(&buffer[7], NULL, 10);
After the patch the device name is 8 characters long and &buffer[7]
overlaps with the name instead of reading the bytes. Before the
patch is was fine because the call to strtoul() seems correct in the
sense that it would read everything until the NULL. So more than 10
megabytes was still ok.
So I guess I was wrong when suggesting that the problem was the
alignment.
^ permalink raw reply
* Re: /proc/net/dev regression
From: Al Viro @ 2015-01-11 1:39 UTC (permalink / raw)
To: Carlos R. Mafra; +Cc: LKML, Hauke Mehrtens, John W. Linville, netdev
In-Reply-To: <20150111013335.GA5753@linux-g29b.site>
On Sun, Jan 11, 2015 at 01:33:35AM +0000, Carlos R. Mafra wrote:
> I think the problem with wmnet is not that it was expecting the fields
> to be aligned because it never had problems before (when definitely more
> than 10 megabytes were received, wmnet is crappy but not _that_ crappy).
>
> I think the problem really was here,
>
> totalbytes_in = strtoul(&buffer[7], NULL, 10);
>
> After the patch the device name is 8 characters long and &buffer[7]
> overlaps with the name instead of reading the bytes. Before the
> patch is was fine because the call to strtoul() seems correct in the
> sense that it would read everything until the NULL. So more than 10
> megabytes was still ok.
>
> So I guess I was wrong when suggesting that the problem was the
> alignment.
Several lines below there's this:
totalpackets_out = strtoul(&buffer[74], NULL, 10);
if (totalpackets_out != lastpackets_out) {
totalbytes_out = strtoul(&buffer[66], NULL, 10);
diffpackets_out += totalpackets_out - lastpackets_out;
diffbytes_out += totalbytes_out - lastbytes_out;
lastpackets_out = totalpackets_out;
lastbytes_out = totalbytes_out;
tx = True;
}
So I'm afraid it *is* that crappy. This function really should use scanf();
note that updateStats_ipchains() in the same file does just that (well,
fgets()+sscanf() for fsck knows what reason). And I'd be careful with all
those %d, actually - it's not _that_ hard to get more than 4Gb sent.
scanf formats really ought to match the kernel-side (seq_)printf one...
^ permalink raw reply
* RE: [PATCH v4] can: Convert to runtime_pm
From: Appana Durga Kedareswara Rao @ 2015-01-11 5:34 UTC (permalink / raw)
To: Marc Kleine-Budde
Cc: linux-can@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Soren Brinkmann,
grant.likely@linaro.org, wg@grandegger.com
In-Reply-To: <54AD267C.4060004@pengutronix.de>
Hi Marc,
> -----Original Message-----
> From: Marc Kleine-Budde [mailto:mkl@pengutronix.de]
> Sent: Wednesday, January 07, 2015 5:59 PM
> To: Appana Durga Kedareswara Rao; wg@grandegger.com; Michal Simek;
> grant.likely@linaro.org
> Cc: linux-can@vger.kernel.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Appana Durga Kedareswara Rao; Soren Brinkmann
> Subject: Re: [PATCH v4] can: Convert to runtime_pm
>
> On 12/23/2014 01:25 PM, Kedareswara rao Appana wrote:
> > Instead of enabling/disabling clocks at several locations in the
> > driver, use the runtime_pm framework. This consolidates the actions
> > for runtime PM in the appropriate callbacks and makes the driver more
> > readable and mantainable.
> >
> > Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> > Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
> > ---
> > Chnages for v4:
> > - Updated with the review comments.
> > Changes for v3:
> > - Converted the driver to use runtime_pm.
> > Changes for v2:
> > - Removed the struct platform_device* from suspend/resume
> > as suggest by Lothar.
> >
> > drivers/net/can/xilinx_can.c | 123
> > +++++++++++++++++++++++++-----------------
> > 1 files changed, 74 insertions(+), 49 deletions(-)
> >
> > diff --git a/drivers/net/can/xilinx_can.c
> > b/drivers/net/can/xilinx_can.c index 6c67643..c71f683 100644
> > --- a/drivers/net/can/xilinx_can.c
> > +++ b/drivers/net/can/xilinx_can.c
> > @@ -32,6 +32,7 @@
> > #include <linux/can/dev.h>
> > #include <linux/can/error.h>
> > #include <linux/can/led.h>
> > +#include <linux/pm_runtime.h>
> >
> > #define DRIVER_NAME "xilinx_can"
> >
> > @@ -138,7 +139,7 @@ struct xcan_priv {
> > u32 (*read_reg)(const struct xcan_priv *priv, enum xcan_reg reg);
> > void (*write_reg)(const struct xcan_priv *priv, enum xcan_reg reg,
> > u32 val);
> > - struct net_device *dev;
> > + struct device *dev;
> > void __iomem *reg_base;
> > unsigned long irq_flags;
> > struct clk *bus_clk;
> > @@ -842,6 +843,13 @@ static int xcan_open(struct net_device *ndev)
> > struct xcan_priv *priv = netdev_priv(ndev);
> > int ret;
> >
> > + ret = pm_runtime_get_sync(priv->dev);
> > + if (ret < 0) {
> > + netdev_err(ndev, "%s: pm_runtime_get failed\r(%d)\n\r",
> > + __func__, ret);
> > + return ret;
> > + }
> > +
> > ret = request_irq(ndev->irq, xcan_interrupt, priv->irq_flags,
> > ndev->name, ndev);
> > if (ret < 0) {
> > @@ -849,29 +857,17 @@ static int xcan_open(struct net_device *ndev)
> > goto err;
> > }
> >
> > - ret = clk_prepare_enable(priv->can_clk);
> > - if (ret) {
> > - netdev_err(ndev, "unable to enable device clock\n");
> > - goto err_irq;
> > - }
> > -
> > - ret = clk_prepare_enable(priv->bus_clk);
> > - if (ret) {
> > - netdev_err(ndev, "unable to enable bus clock\n");
> > - goto err_can_clk;
> > - }
> > -
> > /* Set chip into reset mode */
> > ret = set_reset_mode(ndev);
> > if (ret < 0) {
> > netdev_err(ndev, "mode resetting failed!\n");
> > - goto err_bus_clk;
> > + goto err_irq;
> > }
> >
> > /* Common open */
> > ret = open_candev(ndev);
> > if (ret)
> > - goto err_bus_clk;
> > + goto err_irq;
> >
> > ret = xcan_chip_start(ndev);
> > if (ret < 0) {
> > @@ -887,13 +883,11 @@ static int xcan_open(struct net_device *ndev)
> >
> > err_candev:
> > close_candev(ndev);
> > -err_bus_clk:
> > - clk_disable_unprepare(priv->bus_clk);
> > -err_can_clk:
> > - clk_disable_unprepare(priv->can_clk);
> > err_irq:
> > free_irq(ndev->irq, ndev);
> > err:
> > + pm_runtime_put(priv->dev);
> > +
> > return ret;
> > }
> >
> > @@ -910,12 +904,11 @@ static int xcan_close(struct net_device *ndev)
> > netif_stop_queue(ndev);
> > napi_disable(&priv->napi);
> > xcan_chip_stop(ndev);
> > - clk_disable_unprepare(priv->bus_clk);
> > - clk_disable_unprepare(priv->can_clk);
> > free_irq(ndev->irq, ndev);
> > close_candev(ndev);
> >
> > can_led_event(ndev, CAN_LED_EVENT_STOP);
> > + pm_runtime_put(priv->dev);
> >
> > return 0;
> > }
> > @@ -934,27 +927,20 @@ static int xcan_get_berr_counter(const struct
> net_device *ndev,
> > struct xcan_priv *priv = netdev_priv(ndev);
> > int ret;
> >
> > - ret = clk_prepare_enable(priv->can_clk);
> > - if (ret)
> > - goto err;
> > -
> > - ret = clk_prepare_enable(priv->bus_clk);
> > - if (ret)
> > - goto err_clk;
> > + ret = pm_runtime_get_sync(priv->dev);
> > + if (ret < 0) {
> > + netdev_err(ndev, "%s: pm_runtime_get failed\r(%d)\n\r",
> > + __func__, ret);
>
> Please remove the \r from the error messages.
>
Ok
> > + return ret;
> > + }
> >
> > bec->txerr = priv->read_reg(priv, XCAN_ECR_OFFSET) &
> XCAN_ECR_TEC_MASK;
> > bec->rxerr = ((priv->read_reg(priv, XCAN_ECR_OFFSET) &
> > XCAN_ECR_REC_MASK) >> XCAN_ESR_REC_SHIFT);
> >
> > - clk_disable_unprepare(priv->bus_clk);
> > - clk_disable_unprepare(priv->can_clk);
> > + pm_runtime_put(priv->dev);
> >
> > return 0;
> > -
> > -err_clk:
> > - clk_disable_unprepare(priv->can_clk);
> > -err:
> > - return ret;
> > }
> >
> >
> > @@ -967,15 +953,45 @@ static const struct net_device_ops
> > xcan_netdev_ops = {
> >
> > /**
> > * xcan_suspend - Suspend method for the driver
> > - * @dev: Address of the platform_device structure
> > + * @dev: Address of the device structure
> > *
> > * Put the driver into low power mode.
> > - * Return: 0 always
> > + * Return: 0 on success and failure value on error
> > */
> > static int __maybe_unused xcan_suspend(struct device *dev) {
> > - struct platform_device *pdev = dev_get_drvdata(dev);
> > - struct net_device *ndev = platform_get_drvdata(pdev);
> > + if (!device_may_wakeup(dev))
> > + return pm_runtime_force_suspend(dev);
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * xcan_resume - Resume from suspend
> > + * @dev: Address of the device structure
> > + *
> > + * Resume operation after suspend.
> > + * Return: 0 on success and failure value on error */ static int
> > +__maybe_unused xcan_resume(struct device *dev) {
> > + if (!device_may_wakeup(dev))
> > + return pm_runtime_force_resume(dev);
> > +
> > + return 0;
> > +
> > +}
> > +
> > +/**
> > + * xcan_runtime_suspend - Runtime suspend method for the driver
> > + * @dev: Address of the device structure
> > + *
> > + * Put the driver into low power mode.
> > + * Return: 0 always
> > + */
> > +static int __maybe_unused xcan_runtime_suspend(struct device *dev) {
> > + struct net_device *ndev = dev_get_drvdata(dev);
> > struct xcan_priv *priv = netdev_priv(ndev);
> >
> > if (netif_running(ndev)) {
> > @@ -993,16 +1009,15 @@ static int __maybe_unused xcan_suspend(struct
> > device *dev) }
> >
> > /**
> > - * xcan_resume - Resume from suspend
> > - * @dev: Address of the platformdevice structure
> > + * xcan_runtime_resume - Runtime resume from suspend
> > + * @dev: Address of the device structure
> > *
> > * Resume operation after suspend.
> > * Return: 0 on success and failure value on error
> > */
> > -static int __maybe_unused xcan_resume(struct device *dev)
> > +static int __maybe_unused xcan_runtime_resume(struct device *dev)
> > {
> > - struct platform_device *pdev = dev_get_drvdata(dev);
> > - struct net_device *ndev = platform_get_drvdata(pdev);
> > + struct net_device *ndev = dev_get_drvdata(dev);
> > struct xcan_priv *priv = netdev_priv(ndev);
> > int ret;
>
> Some more context:
>
> > ret = clk_enable(priv->bus_clk);
> > if (ret) {
> > dev_err(dev, "Cannot enable clock.\n");
> > return ret;
> > }
> > ret = clk_enable(priv->can_clk);
> > if (ret) {
> > dev_err(dev, "Cannot enable clock.\n");
> > clk_disable_unprepare(priv->bus_clk);
>
> This disable_unprepare looks wrong, should be a disable only.
>
Ok
> > return ret;
> > }
> >
> > priv->write_reg(priv, XCAN_MSR_OFFSET, 0);
> > priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_CEN_MASK);
> >
> > if (netif_running(ndev)) {
> > priv->can.state = CAN_STATE_ERROR_ACTIVE;
>
> What happens if the device was not in ACTIVE state prior to the
> runtime_suspend?
>
I am not sure about the state of CAN at this point of time.
I just followed what other drivers are following for run time suspend :).
> > netif_device_attach(ndev);
> > netif_start_queue(ndev);
> > }
> >
> > return 0;
> > }
>
>
> >
> > @@ -1020,9 +1035,9 @@ static int __maybe_unused xcan_resume(struct
> > device *dev)
> >
> > priv->write_reg(priv, XCAN_MSR_OFFSET, 0);
> > priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_CEN_MASK);
> > - priv->can.state = CAN_STATE_ERROR_ACTIVE;
> >
> > if (netif_running(ndev)) {
> > + priv->can.state = CAN_STATE_ERROR_ACTIVE;
> > netif_device_attach(ndev);
> > netif_start_queue(ndev);
> > }
> > @@ -1030,7 +1045,10 @@ static int __maybe_unused xcan_resume(struct
> device *dev)
> > return 0;
> > }
> >
> > -static SIMPLE_DEV_PM_OPS(xcan_dev_pm_ops, xcan_suspend,
> xcan_resume);
> > +static const struct dev_pm_ops xcan_dev_pm_ops = {
> > + SET_SYSTEM_SLEEP_PM_OPS(xcan_suspend, xcan_resume)
> > + SET_PM_RUNTIME_PM_OPS(xcan_runtime_suspend,
> xcan_runtime_resume,
> > +NULL) };
> >
> > /**
> > * xcan_probe - Platform registration call @@ -1071,7 +1089,7 @@
> > static int xcan_probe(struct platform_device *pdev)
> > return -ENOMEM;
> >
> > priv = netdev_priv(ndev);
> > - priv->dev = ndev;
> > + priv->dev = &pdev->dev;
> > priv->can.bittiming_const = &xcan_bittiming_const;
> > priv->can.do_set_mode = xcan_do_set_mode;
> > priv->can.do_get_berr_counter = xcan_get_berr_counter; @@ -
> 1137,15
> > +1155,22 @@ static int xcan_probe(struct platform_device *pdev)
> >
> > netif_napi_add(ndev, &priv->napi, xcan_rx_poll, rx_max);
> >
> > + pm_runtime_set_active(&pdev->dev);
> > + pm_runtime_irq_safe(&pdev->dev);
> > + pm_runtime_enable(&pdev->dev);
> > + pm_runtime_get_sync(&pdev->dev);
> Check error values?
Ok
> > +
> > ret = register_candev(ndev);
> > if (ret) {
> > dev_err(&pdev->dev, "fail to register failed (err=%d)\n",
> ret);
> > + pm_runtime_put(priv->dev);
>
> Please move the pm_runtime_put into the common error exit path.
>
Ok
> > goto err_unprepare_disable_busclk;
> > }
> >
> > devm_can_led_init(ndev);
> > - clk_disable_unprepare(priv->bus_clk);
> > - clk_disable_unprepare(priv->can_clk);
> > +
> > + pm_runtime_put(&pdev->dev);
> > +
> > netdev_dbg(ndev, "reg_base=0x%p irq=%d clock=%d, tx fifo
> depth:%d\n",
> > priv->reg_base, ndev->irq, priv->can.clock.freq,
> > priv->tx_max);
> >
>
> I think you have to convert the _remove() function, too. Have a look at the
> gpio-zynq.c driver:
>
> > static int zynq_gpio_remove(struct platform_device *pdev) {
> > struct zynq_gpio *gpio = platform_get_drvdata(pdev);
> >
> > pm_runtime_get_sync(&pdev->dev);
>
> However I don't understand why the get_sync() is here. Maybe Sören can
> help?
I converted the remove function to use the run-time PM and .
Below is the remove code snippet.
ret = pm_runtime_get_sync(&pdev->dev);
if (ret < 0) {
netdev_err(ndev, "%s: pm_runtime_get failed(%d)\n",
__func__, ret);
return ret;
}
if (set_reset_mode(ndev) < 0)
netdev_err(ndev, "mode resetting failed!\n");
unregister_candev(ndev);
netif_napi_del(&priv->napi);
free_candev(ndev);
pm_runtime_disable(&pdev->dev);
Observed the below things in the /sys/kernel/debug/clk/clk_summary.
Modprobe ifconfig up ifconfig down rmmod Modprobe ifconfig up ifconfig down rmmod Modprobe ifconfig up ifconfig down rmmod
Clk_prepare count: 1 1 1 1 2 2 2 2 3 3 3 3
Clk_enable count: 0 1 0 1 2 2 2 2 3 3 3 3
Regards,
Kedar.
>
> > gpiochip_remove(&gpio->chip);
> > clk_disable_unprepare(gpio->clk);
> > device_set_wakeup_capable(&pdev->dev, 0);
> > return 0;
> > }
>
> regards,
> Marc
>
> --
> Pengutronix e.K. | Marc Kleine-Budde |
> Industrial Linux Solutions | Phone: +49-231-2826-924 |
> Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
> Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.
^ permalink raw reply
* Re: [PATCH RESEND 2/2] wlcore: align member-assigns in a structure-copy block
From: Eliad Peller @ 2015-01-11 10:22 UTC (permalink / raw)
To: Kalle Valo
Cc: Giel van Schijndel, LKML, John W. Linville, Arik Nemtsov,
open list:TI WILINK WIRELES..., open list:NETWORKING DRIVERS
In-Reply-To: <87vbkfga32.fsf-HodKDYzPHsUD5k0oWYwrnHL1okKdlPRT@public.gmane.org>
On Fri, Jan 9, 2015 at 7:03 PM, Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
> Giel van Schijndel <me-sZ9Uef1cvPWHXe+LvDLADg@public.gmane.org> writes:
>
>> This highlights the differences (e.g. the bug fixed in the previous
>> commit).
>>
>> Signed-off-by: Giel van Schijndel <me-sZ9Uef1cvPWHXe+LvDLADg@public.gmane.org>
>> ---
>> drivers/net/wireless/ti/wlcore/acx.c | 22 +++++++++++-----------
>> 1 file changed, 11 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ti/wlcore/acx.c b/drivers/net/wireless/ti/wlcore/acx.c
>> index f28fa3b..93a2fa8 100644
>> --- a/drivers/net/wireless/ti/wlcore/acx.c
>> +++ b/drivers/net/wireless/ti/wlcore/acx.c
>> @@ -1715,17 +1715,17 @@ int wl12xx_acx_config_hangover(struct wl1271 *wl)
>> goto out;
>> }
>>
>> - acx->recover_time = cpu_to_le32(conf->recover_time);
>> - acx->hangover_period = conf->hangover_period;
>> - acx->dynamic_mode = conf->dynamic_mode;
>> - acx->early_termination_mode = conf->early_termination_mode;
>> - acx->max_period = conf->max_period;
>> - acx->min_period = conf->min_period;
>> - acx->increase_delta = conf->increase_delta;
>> - acx->decrease_delta = conf->decrease_delta;
>> - acx->quiet_time = conf->quiet_time;
>> - acx->increase_time = conf->increase_time;
>> - acx->window_size = conf->window_size;
>> + acx->recover_time = cpu_to_le32(conf->recover_time);
>> + acx->hangover_period = conf->hangover_period;
>> + acx->dynamic_mode = conf->dynamic_mode;
>> + acx->early_termination_mode = conf->early_termination_mode;
>> + acx->max_period = conf->max_period;
>> + acx->min_period = conf->min_period;
>> + acx->increase_delta = conf->increase_delta;
>> + acx->decrease_delta = conf->decrease_delta;
>> + acx->quiet_time = conf->quiet_time;
>> + acx->increase_time = conf->increase_time;
>> + acx->window_size = conf->window_size;
>
> I would like to get an ACK from one of the wlcore developers if I should
> apply this (or not).
>
I don't have a strong opinion here.
However, it looks pretty much redundant to take a random blob (which
was just fixed by a correct patch) and re-indent it.
The rest of the file doesn't follow this style, so i don't see a good
reason to apply it here.
I agree such indentation have some benefit, but it won't help with the
more common use case (of copy-paste error) of copying the wrong field
(i.e. D->a = S->b instead of D->a = S->a).
For these cases the macros suggested by Arend and Johannes will do the
trick. However i usually dislike such macros, as they tend to break
some IDE features (e.g. auto completion).
Maybe we can come up with some nice spatch to catch these cases.
Just my 2c.
Eliad.
--
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 1/3] rtlwifi: btcoexist: Remove some unused functions
From: Rickard Strandqvist @ 2015-01-11 11:56 UTC (permalink / raw)
To: Larry Finger
Cc: Kalle Valo, Masanari Iida, Sachin Kamat,
linux-wireless@vger.kernel.org, Network Development,
Linux Kernel Mailing List
In-Reply-To: <54B15D57.5060709@lwfinger.net>
2015-01-10 18:11 GMT+01:00 Larry Finger <Larry.Finger@lwfinger.net>:
> On 01/10/2015 10:24 AM, Rickard Strandqvist wrote:
>>
>> Removes some functions that are not used anywhere:
>> ex_halbtc8821a1ant_periodical() ex_halbtc8821a1ant_pnp_notify()
>> ex_halbtc8821a1ant_halt_notify() ex_halbtc8821a1ant_bt_info_notify()
>> ex_halbtc8821a1ant_special_packet_notify()
>> ex_halbtc8821a1ant_connect_notify()
>> ex_halbtc8821a1ant_scan_notify() ex_halbtc8821a1ant_lps_notify()
>> ex_halbtc8821a1ant_ips_notify() ex_halbtc8821a1ant_display_coex_info()
>> ex_halbtc8821a1ant_init_coex_dm() ex_halbtc8821a1ant_init_hwconfig()
>>
>> This was partially found by using a static code analysis program called
>> cppcheck.
>>
>> Signed-off-by: Rickard Strandqvist
>> <rickard_strandqvist@spectrumdigital.se>
>
>
> NACK!!!!!!!!!
>
> I told you to stay away from these routines until I finish my update. Not
> only might you remove some functions that I will be needing later, but you
> run the risk of merge complications.
>
> Kalle: Please ignore EVERYTHING from this person. Obviously, he is incapable
> of understanding even the simplest instructions.
>
> Larry
>
>
>> ---
>> .../wireless/rtlwifi/btcoexist/halbtc8821a1ant.c | 707
>> --------------------
>> .../wireless/rtlwifi/btcoexist/halbtc8821a1ant.h | 14 -
>> 2 files changed, 721 deletions(-)
>>
>> diff --git a/drivers/net/wireless/rtlwifi/btcoexist/halbtc8821a1ant.c
>> b/drivers/net/wireless/rtlwifi/btcoexist/halbtc8821a1ant.c
>> index b72e537..a86e6b6 100644
>> --- a/drivers/net/wireless/rtlwifi/btcoexist/halbtc8821a1ant.c
>> +++ b/drivers/net/wireless/rtlwifi/btcoexist/halbtc8821a1ant.c
>> @@ -2213,435 +2213,6 @@ static void halbtc8821a1ant_init_hw_config(struct
>> btc_coexist *btcoexist,
>> /*============================================================*/
>> /* extern function start with EXhalbtc8821a1ant_*/
>> /*============================================================*/
>> -void ex_halbtc8821a1ant_init_hwconfig(struct btc_coexist *btcoexist)
>> -{
>> - halbtc8821a1ant_init_hw_config(btcoexist, true);
>> -}
>> -
>> -void ex_halbtc8821a1ant_init_coex_dm(struct btc_coexist *btcoexist)
>> -{
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_INIT,
>> - "[BTCoex], Coex Mechanism Init!!\n");
>> -
>> - btcoexist->stop_coex_dm = false;
>> -
>> - halbtc8821a1ant_init_coex_dm(btcoexist);
>> -
>> - halbtc8821a1ant_query_bt_info(btcoexist);
>> -}
>> -
>> -void ex_halbtc8821a1ant_display_coex_info(struct btc_coexist *btcoexist)
>> -{
>> - struct btc_board_info *board_info = &btcoexist->board_info;
>> - struct btc_stack_info *stack_info = &btcoexist->stack_info;
>> - struct btc_bt_link_info *bt_link_info = &btcoexist->bt_link_info;
>> - struct rtl_priv *rtlpriv = btcoexist->adapter;
>> - u8 u1_tmp[4], i, bt_info_ext, ps_tdma_case = 0;
>> - u16 u2_tmp[4];
>> - u32 u4_tmp[4];
>> - bool roam = false, scan = false, link = false, wifi_under_5g =
>> false;
>> - bool bt_hs_on = false, wifi_busy = false;
>> - long wifi_rssi = 0, bt_hs_rssi = 0;
>> - u32 wifi_bw, wifi_traffic_dir;
>> - u8 wifi_dot11_chnl, wifi_hs_chnl;
>> - u32 fw_ver = 0, bt_patch_ver = 0;
>> -
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n ============[BT Coexist info]============");
>> -
>> - if (btcoexist->manual_control) {
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n ============[Under Manual
>> Control]============");
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n
>> ==========================================");
>> - }
>> - if (btcoexist->stop_coex_dm) {
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n ============[Coex is
>> STOPPED]============");
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n
>> ==========================================");
>> - }
>> -
>> - if (!board_info->bt_exist) {
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG, "\r\n BT not
>> exists !!!");
>> - return;
>> - }
>> -
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %d/ %d/ %d",
>> - "Ant PG Num/ Ant Mech/ Ant Pos:",
>> - board_info->pg_ant_num,
>> - board_info->btdm_ant_num,
>> - board_info->btdm_ant_pos);
>> -
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %s / %d", "BT stack/ hci ext ver",
>> - ((stack_info->profile_notified) ? "Yes" : "No"),
>> - stack_info->hci_version);
>> -
>> - btcoexist->btc_get(btcoexist, BTC_GET_U4_BT_PATCH_VER,
>> - &bt_patch_ver);
>> - btcoexist->btc_get(btcoexist, BTC_GET_U4_WIFI_FW_VER, &fw_ver);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %d_%x/ 0x%x/ 0x%x(%d)",
>> - "CoexVer/ FwVer/ PatchVer",
>> - glcoex_ver_date_8821a_1ant,
>> - glcoex_ver_8821a_1ant,
>> - fw_ver, bt_patch_ver,
>> - bt_patch_ver);
>> -
>> - btcoexist->btc_get(btcoexist, BTC_GET_BL_HS_OPERATION,
>> - &bt_hs_on);
>> - btcoexist->btc_get(btcoexist, BTC_GET_U1_WIFI_DOT11_CHNL,
>> - &wifi_dot11_chnl);
>> - btcoexist->btc_get(btcoexist, BTC_GET_U1_WIFI_HS_CHNL,
>> - &wifi_hs_chnl);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %d / %d(%d)",
>> - "Dot11 channel / HsChnl(HsMode)",
>> - wifi_dot11_chnl, wifi_hs_chnl, bt_hs_on);
>> -
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %02x %02x %02x ",
>> - "H2C Wifi inform bt chnl Info",
>> - coex_dm->wifi_chnl_info[0], coex_dm->wifi_chnl_info[1],
>> - coex_dm->wifi_chnl_info[2]);
>> -
>> - btcoexist->btc_get(btcoexist, BTC_GET_S4_WIFI_RSSI, &wifi_rssi);
>> - btcoexist->btc_get(btcoexist, BTC_GET_S4_HS_RSSI, &bt_hs_rssi);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %d/ %d", "Wifi rssi/ HS rssi",
>> - (int)wifi_rssi, (int)bt_hs_rssi);
>> -
>> - btcoexist->btc_get(btcoexist, BTC_GET_BL_WIFI_SCAN, &scan);
>> - btcoexist->btc_get(btcoexist, BTC_GET_BL_WIFI_LINK, &link);
>> - btcoexist->btc_get(btcoexist, BTC_GET_BL_WIFI_ROAM, &roam);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %d/ %d/ %d ", "Wifi link/ roam/ scan",
>> - link, roam, scan);
>> -
>> - btcoexist->btc_get(btcoexist, BTC_GET_BL_WIFI_UNDER_5G,
>> - &wifi_under_5g);
>> - btcoexist->btc_get(btcoexist, BTC_GET_U4_WIFI_BW,
>> - &wifi_bw);
>> - btcoexist->btc_get(btcoexist, BTC_GET_BL_WIFI_BUSY,
>> - &wifi_busy);
>> - btcoexist->btc_get(btcoexist, BTC_GET_U4_WIFI_TRAFFIC_DIRECTION,
>> - &wifi_traffic_dir);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %s / %s/ %s ", "Wifi status",
>> - (wifi_under_5g ? "5G" : "2.4G"),
>> - ((BTC_WIFI_BW_LEGACY == wifi_bw) ? "Legacy" :
>> - (((BTC_WIFI_BW_HT40 == wifi_bw) ? "HT40" : "HT20"))),
>> - ((!wifi_busy) ? "idle" :
>> - ((BTC_WIFI_TRAFFIC_TX == wifi_traffic_dir) ?
>> - "uplink" : "downlink")));
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = [%s/ %d/ %d] ", "BT [status/ rssi/
>> retryCnt]",
>> - ((btcoexist->bt_info.bt_disabled) ? ("disabled") :
>> - ((coex_sta->c2h_bt_inquiry_page) ? ("inquiry/page
>> scan") :
>> - ((BT_8821A_1ANT_BT_STATUS_NON_CONNECTED_IDLE ==
>> - coex_dm->bt_status) ?
>> - "non-connected idle" :
>> - ((BT_8821A_1ANT_BT_STATUS_CONNECTED_IDLE ==
>> - coex_dm->bt_status) ?
>> - "connected-idle" : "busy")))),
>> - coex_sta->bt_rssi, coex_sta->bt_retry_cnt);
>> -
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %d / %d / %d / %d", "SCO/HID/PAN/A2DP",
>> - bt_link_info->sco_exist,
>> - bt_link_info->hid_exist,
>> - bt_link_info->pan_exist,
>> - bt_link_info->a2dp_exist);
>> - btcoexist->btc_disp_dbg_msg(btcoexist, BTC_DBG_DISP_BT_LINK_INFO);
>> -
>> - bt_info_ext = coex_sta->bt_info_ext;
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %s",
>> - "BT Info A2DP rate",
>> - (bt_info_ext&BIT0) ?
>> - "Basic rate" : "EDR rate");
>> -
>> - for (i = 0; i < BT_INFO_SRC_8821A_1ANT_MAX; i++) {
>> - if (coex_sta->bt_info_c2h_cnt[i]) {
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %02x %02x %02x %02x %02x
>> %02x %02x(%d)",
>> - glbt_info_src_8821a_1ant[i],
>> - coex_sta->bt_info_c2h[i][0],
>> - coex_sta->bt_info_c2h[i][1],
>> - coex_sta->bt_info_c2h[i][2],
>> - coex_sta->bt_info_c2h[i][3],
>> - coex_sta->bt_info_c2h[i][4],
>> - coex_sta->bt_info_c2h[i][5],
>> - coex_sta->bt_info_c2h[i][6],
>> - coex_sta->bt_info_c2h_cnt[i]);
>> - }
>> - }
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %s/%s, (0x%x/0x%x)",
>> - "PS state, IPS/LPS, (lps/rpwm)",
>> - ((coex_sta->under_ips ? "IPS ON" : "IPS OFF")),
>> - ((coex_sta->under_Lps ? "LPS ON" : "LPS OFF")),
>> - btcoexist->bt_info.lps_val,
>> - btcoexist->bt_info.rpwm_val);
>> - btcoexist->btc_disp_dbg_msg(btcoexist,
>> BTC_DBG_DISP_FW_PWR_MODE_CMD);
>> -
>> - if (!btcoexist->manual_control) {
>> - /* Sw mechanism*/
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s", "============[Sw
>> mechanism]============");
>> -
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %d", "SM[LowPenaltyRA]",
>> - coex_dm->cur_low_penalty_ra);
>> -
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %s/ %s/ %d ",
>> - "DelBA/ BtCtrlAgg/ AggSize",
>> - (btcoexist->bt_info.reject_agg_pkt ? "Yes" :
>> "No"),
>> - (btcoexist->bt_info.bt_ctrl_buf_size ? "Yes" :
>> "No"),
>> - btcoexist->bt_info.agg_buf_size);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = 0x%x ", "Rate Mask",
>> - btcoexist->bt_info.ra_mask);
>> -
>> - /* Fw mechanism*/
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG, "\r\n %-35s",
>> - "============[Fw mechanism]============");
>> -
>> - ps_tdma_case = coex_dm->cur_ps_tdma;
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %02x %02x %02x %02x %02x case-%d
>> (auto:%d)",
>> - "PS TDMA",
>> - coex_dm->ps_tdma_para[0],
>> - coex_dm->ps_tdma_para[1],
>> - coex_dm->ps_tdma_para[2],
>> - coex_dm->ps_tdma_para[3],
>> - coex_dm->ps_tdma_para[4],
>> - ps_tdma_case,
>> - coex_dm->auto_tdma_adjust);
>> -
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = 0x%x ",
>> - "Latest error condition(should be 0)",
>> - coex_dm->error_condition);
>> -
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %d ", "IgnWlanAct",
>> - coex_dm->cur_ignore_wlan_act);
>> - }
>> -
>> - /* Hw setting*/
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s", "============[Hw setting]============");
>> -
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = 0x%x/0x%x/0x%x/0x%x",
>> - "backup ARFR1/ARFR2/RL/AMaxTime",
>> - coex_dm->backup_arfr_cnt1,
>> - coex_dm->backup_arfr_cnt2,
>> - coex_dm->backup_retry_limit,
>> - coex_dm->backup_ampdu_max_time);
>> -
>> - u4_tmp[0] = btcoexist->btc_read_4byte(btcoexist, 0x430);
>> - u4_tmp[1] = btcoexist->btc_read_4byte(btcoexist, 0x434);
>> - u2_tmp[0] = btcoexist->btc_read_2byte(btcoexist, 0x42a);
>> - u1_tmp[0] = btcoexist->btc_read_1byte(btcoexist, 0x456);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = 0x%x/0x%x/0x%x/0x%x",
>> - "0x430/0x434/0x42a/0x456",
>> - u4_tmp[0], u4_tmp[1], u2_tmp[0], u1_tmp[0]);
>> -
>> - u1_tmp[0] = btcoexist->btc_read_1byte(btcoexist, 0x778);
>> - u4_tmp[0] = btcoexist->btc_read_4byte(btcoexist, 0xc58);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = 0x%x/ 0x%x", "0x778/ 0xc58[29:25]",
>> - u1_tmp[0], (u4_tmp[0]&0x3e000000) >> 25);
>> -
>> - u1_tmp[0] = btcoexist->btc_read_1byte(btcoexist, 0x8db);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = 0x%x", "0x8db[6:5]",
>> - ((u1_tmp[0]&0x60)>>5));
>> -
>> - u1_tmp[0] = btcoexist->btc_read_1byte(btcoexist, 0x975);
>> - u4_tmp[0] = btcoexist->btc_read_4byte(btcoexist, 0xcb4);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = 0x%x/ 0x%x/ 0x%x",
>> - "0xcb4[29:28]/0xcb4[7:0]/0x974[9:8]",
>> - (u4_tmp[0] & 0x30000000)>>28,
>> - u4_tmp[0] & 0xff,
>> - u1_tmp[0] & 0x3);
>> -
>> - u1_tmp[0] = btcoexist->btc_read_1byte(btcoexist, 0x40);
>> - u4_tmp[0] = btcoexist->btc_read_4byte(btcoexist, 0x4c);
>> - u1_tmp[1] = btcoexist->btc_read_1byte(btcoexist, 0x64);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = 0x%x/ 0x%x/ 0x%x",
>> - "0x40/0x4c[24:23]/0x64[0]",
>> - u1_tmp[0], ((u4_tmp[0]&0x01800000)>>23),
>> u1_tmp[1]&0x1);
>> -
>> - u4_tmp[0] = btcoexist->btc_read_4byte(btcoexist, 0x550);
>> - u1_tmp[0] = btcoexist->btc_read_1byte(btcoexist, 0x522);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = 0x%x/ 0x%x", "0x550(bcn ctrl)/0x522",
>> - u4_tmp[0], u1_tmp[0]);
>> -
>> - u4_tmp[0] = btcoexist->btc_read_4byte(btcoexist, 0xc50);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = 0x%x", "0xc50(dig)",
>> - u4_tmp[0]&0xff);
>> -
>> - u4_tmp[0] = btcoexist->btc_read_4byte(btcoexist, 0xf48);
>> - u1_tmp[0] = btcoexist->btc_read_1byte(btcoexist, 0xa5d);
>> - u1_tmp[1] = btcoexist->btc_read_1byte(btcoexist, 0xa5c);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = 0x%x/ 0x%x", "OFDM-FA/ CCK-FA",
>> - u4_tmp[0], (u1_tmp[0]<<8) + u1_tmp[1]);
>> -
>> - u4_tmp[0] = btcoexist->btc_read_4byte(btcoexist, 0x6c0);
>> - u4_tmp[1] = btcoexist->btc_read_4byte(btcoexist, 0x6c4);
>> - u4_tmp[2] = btcoexist->btc_read_4byte(btcoexist, 0x6c8);
>> - u1_tmp[0] = btcoexist->btc_read_1byte(btcoexist, 0x6cc);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = 0x%x/ 0x%x/ 0x%x/ 0x%x",
>> - "0x6c0/0x6c4/0x6c8/0x6cc(coexTable)",
>> - u4_tmp[0], u4_tmp[1], u4_tmp[2], u1_tmp[0]);
>> -
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %d/ %d", "0x770(high-pri rx/tx)",
>> - coex_sta->high_priority_rx,
>> coex_sta->high_priority_tx);
>> - RT_TRACE(rtlpriv, COMP_INIT, DBG_DMESG,
>> - "\r\n %-35s = %d/ %d", "0x774(low-pri rx/tx)",
>> - coex_sta->low_priority_rx, coex_sta->low_priority_tx);
>> -#if (BT_AUTO_REPORT_ONLY_8821A_1ANT == 1)
>> - halbtc8821a1ant_monitor_bt_ctr(btcoexist);
>> -#endif
>> - btcoexist->btc_disp_dbg_msg(btcoexist,
>> BTC_DBG_DISP_COEX_STATISTICS);
>> -}
>> -
>> -void ex_halbtc8821a1ant_ips_notify(struct btc_coexist *btcoexist, u8
>> type)
>> -{
>> - if (btcoexist->manual_control || btcoexist->stop_coex_dm)
>> - return;
>> -
>> - if (BTC_IPS_ENTER == type) {
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], IPS ENTER notify\n");
>> - coex_sta->under_ips = true;
>> - halbtc8821a1ant_set_ant_path(btcoexist,
>> - BTC_ANT_PATH_BT, false,
>> true);
>> - /*set PTA control*/
>> - halbtc8821a1ant_ps_tdma(btcoexist, NORMAL_EXEC, false, 8);
>> - halbtc8821a1ant_coex_table_with_type(btcoexist,
>> - NORMAL_EXEC, 0);
>> - } else if (BTC_IPS_LEAVE == type) {
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], IPS LEAVE notify\n");
>> - coex_sta->under_ips = false;
>> -
>> - halbtc8821a1ant_run_coexist_mechanism(btcoexist);
>> - }
>> -}
>> -
>> -void ex_halbtc8821a1ant_lps_notify(struct btc_coexist *btcoexist, u8
>> type)
>> -{
>> - if (btcoexist->manual_control || btcoexist->stop_coex_dm)
>> - return;
>> -
>> - if (BTC_LPS_ENABLE == type) {
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], LPS ENABLE notify\n");
>> - coex_sta->under_Lps = true;
>> - } else if (BTC_LPS_DISABLE == type) {
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], LPS DISABLE notify\n");
>> - coex_sta->under_Lps = false;
>> - }
>> -}
>> -
>> -void ex_halbtc8821a1ant_scan_notify(struct btc_coexist *btcoexist, u8
>> type)
>> -{
>> - bool wifi_connected = false, bt_hs_on = false;
>> -
>> - if (btcoexist->manual_control ||
>> - btcoexist->stop_coex_dm ||
>> - btcoexist->bt_info.bt_disabled)
>> - return;
>> -
>> - btcoexist->btc_get(btcoexist,
>> - BTC_GET_BL_HS_OPERATION, &bt_hs_on);
>> - btcoexist->btc_get(btcoexist,
>> - BTC_GET_BL_WIFI_CONNECTED, &wifi_connected);
>> -
>> - halbtc8821a1ant_query_bt_info(btcoexist);
>> -
>> - if (coex_sta->c2h_bt_inquiry_page) {
>> - halbtc8821a1ant_action_bt_inquiry(btcoexist);
>> - return;
>> - } else if (bt_hs_on) {
>> - halbtc8821a1ant_action_hs(btcoexist);
>> - return;
>> - }
>> -
>> - if (BTC_SCAN_START == type) {
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], SCAN START notify\n");
>> - if (!wifi_connected) {
>> - /* non-connected scan*/
>> - btc8821a1ant_act_wifi_not_conn_scan(btcoexist);
>> - } else {
>> - /* wifi is connected*/
>> -
>> halbtc8821a1ant_action_wifi_connected_scan(btcoexist);
>> - }
>> - } else if (BTC_SCAN_FINISH == type) {
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], SCAN FINISH notify\n");
>> - if (!wifi_connected) {
>> - /* non-connected scan*/
>> -
>> halbtc8821a1ant_action_wifi_not_connected(btcoexist);
>> - } else {
>> - halbtc8821a1ant_action_wifi_connected(btcoexist);
>> - }
>> - }
>> -}
>> -
>> -void ex_halbtc8821a1ant_connect_notify(struct btc_coexist *btcoexist, u8
>> type)
>> -{
>> - bool wifi_connected = false, bt_hs_on = false;
>> -
>> - if (btcoexist->manual_control ||
>> - btcoexist->stop_coex_dm ||
>> - btcoexist->bt_info.bt_disabled)
>> - return;
>> -
>> - btcoexist->btc_get(btcoexist, BTC_GET_BL_HS_OPERATION, &bt_hs_on);
>> - if (coex_sta->c2h_bt_inquiry_page) {
>> - halbtc8821a1ant_action_bt_inquiry(btcoexist);
>> - return;
>> - } else if (bt_hs_on) {
>> - halbtc8821a1ant_action_hs(btcoexist);
>> - return;
>> - }
>> -
>> - if (BTC_ASSOCIATE_START == type) {
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], CONNECT START notify\n");
>> - btc8821a1ant_act_wifi_not_conn_scan(btcoexist);
>> - } else if (BTC_ASSOCIATE_FINISH == type) {
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], CONNECT FINISH notify\n");
>> -
>> - btcoexist->btc_get(btcoexist,
>> - BTC_GET_BL_WIFI_CONNECTED, &wifi_connected);
>> - if (!wifi_connected) {
>> - /* non-connected scan*/
>> -
>> halbtc8821a1ant_action_wifi_not_connected(btcoexist);
>> - } else {
>> - halbtc8821a1ant_action_wifi_connected(btcoexist);
>> - }
>> - }
>> -}
>>
>> void ex_halbtc8821a1ant_media_status_notify(struct btc_coexist
>> *btcoexist,
>> u8 type)
>> @@ -2690,281 +2261,3 @@ void ex_halbtc8821a1ant_media_status_notify(struct
>> btc_coexist *btcoexist,
>> btcoexist->btc_fill_h2c(btcoexist, 0x66, 3, h2c_parameter);
>> }
>>
>> -void ex_halbtc8821a1ant_special_packet_notify(struct btc_coexist
>> *btcoexist,
>> - u8 type)
>> -{
>> - bool bt_hs_on = false;
>> -
>> - if (btcoexist->manual_control ||
>> - btcoexist->stop_coex_dm ||
>> - btcoexist->bt_info.bt_disabled)
>> - return;
>> -
>> - coex_sta->special_pkt_period_cnt = 0;
>> -
>> - btcoexist->btc_get(btcoexist, BTC_GET_BL_HS_OPERATION, &bt_hs_on);
>> - if (coex_sta->c2h_bt_inquiry_page) {
>> - halbtc8821a1ant_action_bt_inquiry(btcoexist);
>> - return;
>> - } else if (bt_hs_on) {
>> - halbtc8821a1ant_action_hs(btcoexist);
>> - return;
>> - }
>> -
>> - if (BTC_PACKET_DHCP == type ||
>> - BTC_PACKET_EAPOL == type) {
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], special Packet(%d) notify\n", type);
>> - btc8821a1ant_act_wifi_conn_sp_pkt(btcoexist);
>> - }
>> -}
>> -
>> -void ex_halbtc8821a1ant_bt_info_notify(struct btc_coexist *btcoexist,
>> - u8 *tmp_buf, u8 length)
>> -{
>> - u8 bt_info = 0;
>> - u8 i, rsp_source = 0;
>> - bool wifi_connected = false;
>> - bool bt_busy = false;
>> - bool wifi_under_5g = false;
>> -
>> - coex_sta->c2h_bt_info_req_sent = false;
>> -
>> - btcoexist->btc_get(btcoexist,
>> - BTC_GET_BL_WIFI_UNDER_5G, &wifi_under_5g);
>> -
>> - rsp_source = tmp_buf[0]&0xf;
>> - if (rsp_source >= BT_INFO_SRC_8821A_1ANT_MAX)
>> - rsp_source = BT_INFO_SRC_8821A_1ANT_WIFI_FW;
>> - coex_sta->bt_info_c2h_cnt[rsp_source]++;
>> -
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], Bt info[%d], length = %d, hex data = [",
>> - rsp_source, length);
>> - for (i = 0; i < length; i++) {
>> - coex_sta->bt_info_c2h[rsp_source][i] = tmp_buf[i];
>> - if (i == 1)
>> - bt_info = tmp_buf[i];
>> - if (i == length-1) {
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "0x%02x]\n", tmp_buf[i]);
>> - } else {
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "0x%02x, ", tmp_buf[i]);
>> - }
>> - }
>> -
>> - if (BT_INFO_SRC_8821A_1ANT_WIFI_FW != rsp_source) {
>> - coex_sta->bt_retry_cnt = /* [3:0]*/
>> - coex_sta->bt_info_c2h[rsp_source][2]&0xf;
>> -
>> - coex_sta->bt_rssi =
>> - coex_sta->bt_info_c2h[rsp_source][3]*2+10;
>> -
>> - coex_sta->bt_info_ext =
>> - coex_sta->bt_info_c2h[rsp_source][4];
>> -
>> - /* Here we need to resend some wifi info to BT*/
>> - /* because bt is reset and loss of the info.*/
>> - if (coex_sta->bt_info_ext & BIT1) {
>> - BTC_PRINT(BTC_MSG_ALGORITHM, ALGO_TRACE,
>> - "[BTCoex], BT ext info bit1 check, send
>> wifi BW&Chnl to BT!!\n");
>> - btcoexist->btc_get(btcoexist,
>> - BTC_GET_BL_WIFI_CONNECTED,
>> - &wifi_connected);
>> - if (wifi_connected) {
>> -
>> ex_halbtc8821a1ant_media_status_notify(btcoexist,
>> -
>> BTC_MEDIA_CONNECT);
>> - } else {
>> -
>> ex_halbtc8821a1ant_media_status_notify(btcoexist,
>> -
>> BTC_MEDIA_DISCONNECT);
>> - }
>> - }
>> -
>> - if ((coex_sta->bt_info_ext & BIT3) && !wifi_under_5g) {
>> - if (!btcoexist->manual_control &&
>> - !btcoexist->stop_coex_dm) {
>> - BTC_PRINT(BTC_MSG_ALGORITHM, ALGO_TRACE,
>> - "[BTCoex], BT ext info bit3
>> check, set BT NOT to ignore Wlan active!!\n");
>> - halbtc8821a1ant_ignore_wlan_act(btcoexist,
>> -
>> FORCE_EXEC,
>> - false);
>> - }
>> - }
>> -#if (BT_AUTO_REPORT_ONLY_8821A_1ANT == 0)
>> - if (!(coex_sta->bt_info_ext & BIT4)) {
>> - BTC_PRINT(BTC_MSG_ALGORITHM, ALGO_TRACE,
>> - "[BTCoex], BT ext info bit4 check, set
>> BT to enable Auto Report!!\n");
>> - halbtc8821a1ant_bt_auto_report(btcoexist,
>> - FORCE_EXEC, true);
>> - }
>> -#endif
>> - }
>> -
>> - /* check BIT2 first ==> check if bt is under inquiry or page
>> scan*/
>> - if (bt_info & BT_INFO_8821A_1ANT_B_INQ_PAGE)
>> - coex_sta->c2h_bt_inquiry_page = true;
>> - else
>> - coex_sta->c2h_bt_inquiry_page = false;
>> -
>> - /* set link exist status*/
>> - if (!(bt_info&BT_INFO_8821A_1ANT_B_CONNECTION)) {
>> - coex_sta->bt_link_exist = false;
>> - coex_sta->pan_exist = false;
>> - coex_sta->a2dp_exist = false;
>> - coex_sta->hid_exist = false;
>> - coex_sta->sco_exist = false;
>> - } else {
>> - /* connection exists*/
>> - coex_sta->bt_link_exist = true;
>> - if (bt_info & BT_INFO_8821A_1ANT_B_FTP)
>> - coex_sta->pan_exist = true;
>> - else
>> - coex_sta->pan_exist = false;
>> - if (bt_info & BT_INFO_8821A_1ANT_B_A2DP)
>> - coex_sta->a2dp_exist = true;
>> - else
>> - coex_sta->a2dp_exist = false;
>> - if (bt_info & BT_INFO_8821A_1ANT_B_HID)
>> - coex_sta->hid_exist = true;
>> - else
>> - coex_sta->hid_exist = false;
>> - if (bt_info & BT_INFO_8821A_1ANT_B_SCO_ESCO)
>> - coex_sta->sco_exist = true;
>> - else
>> - coex_sta->sco_exist = false;
>> - }
>> -
>> - halbtc8821a1ant_update_bt_link_info(btcoexist);
>> -
>> - if (!(bt_info&BT_INFO_8821A_1ANT_B_CONNECTION)) {
>> - coex_dm->bt_status =
>> BT_8821A_1ANT_BT_STATUS_NON_CONNECTED_IDLE;
>> - BTC_PRINT(BTC_MSG_ALGORITHM, ALGO_TRACE,
>> - "[BTCoex], BtInfoNotify(), BT Non-Connected
>> idle!!!\n");
>> - } else if (bt_info == BT_INFO_8821A_1ANT_B_CONNECTION) {
>> - /* connection exists but no busy*/
>> - coex_dm->bt_status =
>> BT_8821A_1ANT_BT_STATUS_CONNECTED_IDLE;
>> - BTC_PRINT(BTC_MSG_ALGORITHM, ALGO_TRACE,
>> - "[BTCoex], BtInfoNotify(), BT
>> Connected-idle!!!\n");
>> - } else if ((bt_info&BT_INFO_8821A_1ANT_B_SCO_ESCO) ||
>> - (bt_info&BT_INFO_8821A_1ANT_B_SCO_BUSY)) {
>> - coex_dm->bt_status = BT_8821A_1ANT_BT_STATUS_SCO_BUSY;
>> - BTC_PRINT(BTC_MSG_ALGORITHM, ALGO_TRACE,
>> - "[BTCoex], BtInfoNotify(), BT SCO busy!!!\n");
>> - } else if (bt_info&BT_INFO_8821A_1ANT_B_ACL_BUSY) {
>> - if (BT_8821A_1ANT_BT_STATUS_ACL_BUSY !=
>> coex_dm->bt_status)
>> - coex_dm->auto_tdma_adjust = false;
>> - coex_dm->bt_status = BT_8821A_1ANT_BT_STATUS_ACL_BUSY;
>> - BTC_PRINT(BTC_MSG_ALGORITHM, ALGO_TRACE,
>> - "[BTCoex], BtInfoNotify(), BT ACL busy!!!\n");
>> - } else {
>> - coex_dm->bt_status = BT_8821A_1ANT_BT_STATUS_MAX;
>> - BTC_PRINT(BTC_MSG_ALGORITHM, ALGO_TRACE,
>> - "[BTCoex], BtInfoNotify(), BT Non-Defined
>> state!!!\n");
>> - }
>> -
>> - if ((BT_8821A_1ANT_BT_STATUS_ACL_BUSY == coex_dm->bt_status) ||
>> - (BT_8821A_1ANT_BT_STATUS_SCO_BUSY == coex_dm->bt_status) ||
>> - (BT_8821A_1ANT_BT_STATUS_ACL_SCO_BUSY == coex_dm->bt_status))
>> - bt_busy = true;
>> - else
>> - bt_busy = false;
>> - btcoexist->btc_set(btcoexist,
>> - BTC_SET_BL_BT_TRAFFIC_BUSY, &bt_busy);
>> -
>> - halbtc8821a1ant_run_coexist_mechanism(btcoexist);
>> -}
>> -
>> -void ex_halbtc8821a1ant_halt_notify(struct btc_coexist *btcoexist)
>> -{
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], Halt notify\n");
>> -
>> - btcoexist->stop_coex_dm = true;
>> -
>> - halbtc8821a1ant_set_ant_path(btcoexist,
>> - BTC_ANT_PATH_BT, false, true);
>> - halbtc8821a1ant_ignore_wlan_act(btcoexist, FORCE_EXEC, true);
>> -
>> - halbtc8821a1ant_power_save_state(btcoexist,
>> - BTC_PS_WIFI_NATIVE, 0x0, 0x0);
>> - halbtc8821a1ant_ps_tdma(btcoexist, FORCE_EXEC, false, 0);
>> -
>> - ex_halbtc8821a1ant_media_status_notify(btcoexist,
>> - BTC_MEDIA_DISCONNECT);
>> -}
>> -
>> -void ex_halbtc8821a1ant_pnp_notify(struct btc_coexist *btcoexist, u8
>> pnp_state)
>> -{
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], Pnp notify\n");
>> -
>> - if (BTC_WIFI_PNP_SLEEP == pnp_state) {
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], Pnp notify to SLEEP\n");
>> - btcoexist->stop_coex_dm = true;
>> - halbtc8821a1ant_ignore_wlan_act(btcoexist, FORCE_EXEC,
>> true);
>> - halbtc8821a1ant_power_save_state(btcoexist,
>> BTC_PS_WIFI_NATIVE,
>> - 0x0, 0x0);
>> - halbtc8821a1ant_ps_tdma(btcoexist, NORMAL_EXEC, false, 9);
>> - } else if (BTC_WIFI_PNP_WAKE_UP == pnp_state) {
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_NOTIFY,
>> - "[BTCoex], Pnp notify to WAKE UP\n");
>> - btcoexist->stop_coex_dm = false;
>> - halbtc8821a1ant_init_hw_config(btcoexist, false);
>> - halbtc8821a1ant_init_coex_dm(btcoexist);
>> - halbtc8821a1ant_query_bt_info(btcoexist);
>> - }
>> -}
>> -
>> -void
>> -ex_halbtc8821a1ant_periodical(
>> - struct btc_coexist *btcoexist) {
>> - static u8 dis_ver_info_cnt;
>> - u32 fw_ver = 0, bt_patch_ver = 0;
>> - struct btc_board_info *board_info = &btcoexist->board_info;
>> - struct btc_stack_info *stack_info = &btcoexist->stack_info;
>> -
>> - BTC_PRINT(BTC_MSG_ALGORITHM, ALGO_TRACE,
>> - "[BTCoex],
>> ==========================Periodical===========================\n");
>> -
>> - if (dis_ver_info_cnt <= 5) {
>> - dis_ver_info_cnt += 1;
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_INIT,
>> - "[BTCoex],
>> ****************************************************************\n");
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_INIT,
>> - "[BTCoex], Ant PG Num/ Ant Mech/ Ant Pos = %d/
>> %d/ %d\n",
>> - board_info->pg_ant_num,
>> - board_info->btdm_ant_num,
>> - board_info->btdm_ant_pos);
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_INIT,
>> - "[BTCoex], BT stack/ hci ext ver = %s / %d\n",
>> - ((stack_info->profile_notified) ? "Yes" : "No"),
>> - stack_info->hci_version);
>> - btcoexist->btc_get(btcoexist, BTC_GET_U4_BT_PATCH_VER,
>> - &bt_patch_ver);
>> - btcoexist->btc_get(btcoexist, BTC_GET_U4_WIFI_FW_VER,
>> &fw_ver);
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_INIT,
>> - "[BTCoex], CoexVer/ FwVer/ PatchVer = %d_%x/
>> 0x%x/ 0x%x(%d)\n",
>> - glcoex_ver_date_8821a_1ant,
>> - glcoex_ver_8821a_1ant,
>> - fw_ver, bt_patch_ver,
>> - bt_patch_ver);
>> - BTC_PRINT(BTC_MSG_INTERFACE, INTF_INIT,
>> - "[BTCoex],
>> ****************************************************************\n");
>> - }
>> -
>> -#if (BT_AUTO_REPORT_ONLY_8821A_1ANT == 0)
>> - halbtc8821a1ant_query_bt_info(btcoexist);
>> - halbtc8821a1ant_monitor_bt_ctr(btcoexist);
>> - btc8821a1ant_mon_bt_en_dis(btcoexist);
>> -#else
>> - if (halbtc8821a1ant_Is_wifi_status_changed(btcoexist) ||
>> - coex_dm->auto_tdma_adjust) {
>> - if (coex_sta->special_pkt_period_cnt > 2)
>> - halbtc8821a1ant_run_coexist_mechanism(btcoexist);
>> - }
>> -
>> - coex_sta->special_pkt_period_cnt++;
>> -#endif
>> -}
>> diff --git a/drivers/net/wireless/rtlwifi/btcoexist/halbtc8821a1ant.h
>> b/drivers/net/wireless/rtlwifi/btcoexist/halbtc8821a1ant.h
>> index 20e9048..c3eab15 100644
>> --- a/drivers/net/wireless/rtlwifi/btcoexist/halbtc8821a1ant.h
>> +++ b/drivers/net/wireless/rtlwifi/btcoexist/halbtc8821a1ant.h
>> @@ -168,21 +168,7 @@ struct coex_sta_8821a_1ant {
>> * The following is interface which will notify coex module.
>> *===========================================
>> */
>> -void ex_halbtc8821a1ant_init_hwconfig(struct btc_coexist *btcoexist);
>> -void ex_halbtc8821a1ant_init_coex_dm(struct btc_coexist *btcoexist);
>> -void ex_halbtc8821a1ant_ips_notify(struct btc_coexist *btcoexist, u8
>> type);
>> -void ex_halbtc8821a1ant_lps_notify(struct btc_coexist *btcoexist, u8
>> type);
>> -void ex_halbtc8821a1ant_scan_notify(struct btc_coexist *btcoexist, u8
>> type);
>> -void ex_halbtc8821a1ant_connect_notify(struct btc_coexist *btcoexist, u8
>> type);
>> void ex_halbtc8821a1ant_media_status_notify(struct btc_coexist
>> *btcoexist,
>> u8 type);
>> -void ex_halbtc8821a1ant_special_packet_notify(struct btc_coexist
>> *btcoexist,
>> - u8 type);
>> -void ex_halbtc8821a1ant_bt_info_notify(struct btc_coexist *btcoexist,
>> - u8 *tmpbuf, u8 length);
>> -void ex_halbtc8821a1ant_halt_notify(struct btc_coexist *btcoexist);
>> -void ex_halbtc8821a1ant_pnp_notify(struct btc_coexist *btcoexist, u8
>> pnpstate);
>> -void ex_halbtc8821a1ant_periodical(struct btc_coexist *btcoexist);
>> -void ex_halbtc8821a1ant_display_coex_info(struct btc_coexist *btcoexist);
>> void ex_halbtc8821a1ant_dbg_control(struct btc_coexist *btcoexist, u8
>> op_code,
>> u8 op_len, u8 *data);
>>
Hi
I was absolutely sure that was in the brcm80211/brcmsmac part for some
reason, but I should of course double-checked it!
Apologies, I will not send any more patches.
Kind regards
Rickard Strandqvist
^ permalink raw reply
* [net-next PATCH 1/1] net: sched: Introduce connmark action
From: Jamal Hadi Salim @ 2015-01-11 12:53 UTC (permalink / raw)
To: davem; +Cc: nbd, pablo, fw, netdev, Jamal Hadi Salim
From: Felix Fietkau <nbd@openwrt.org>
This tc action allows you to retrieve the connection tracking mark
There are known limitations currently:
doesn't work for inital packets, since we only query the ct table.
Fine given use case is for returning packets
no implicit defrag.
frags should be rare so fix later and what is a frag between friends
won't work for more complex tasks, e.g. lookup of other extensions
since we have no means to store results
we still have a 2nd lookup later on via normal conntrack path.
This shouldn't break anything though since skb->nfct isn't altered.
Signed-off-by: Felix Fietkau <nbd@openwrt.org>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
include/net/tc_act/tc_connmark.h | 14 ++
include/uapi/linux/tc_act/tc_connmark.h | 22 ++++
net/sched/Kconfig | 11 ++
net/sched/Makefile | 1 +
net/sched/act_connmark.c | 211 +++++++++++++++++++++++++++++++
5 files changed, 259 insertions(+)
create mode 100644 include/net/tc_act/tc_connmark.h
create mode 100644 include/uapi/linux/tc_act/tc_connmark.h
create mode 100644 net/sched/act_connmark.c
diff --git a/include/net/tc_act/tc_connmark.h b/include/net/tc_act/tc_connmark.h
new file mode 100644
index 0000000..5c1104c
--- /dev/null
+++ b/include/net/tc_act/tc_connmark.h
@@ -0,0 +1,14 @@
+#ifndef __NET_TC_CONNMARK_H
+#define __NET_TC_CONNMARK_H
+
+#include <net/act_api.h>
+
+struct tcf_connmark_info {
+ struct tcf_common common;
+ u16 zone;
+};
+
+#define to_connmark(a) \
+ container_of(a->priv, struct tcf_connmark_info, common)
+
+#endif /* __NET_TC_CONNMARK_H */
diff --git a/include/uapi/linux/tc_act/tc_connmark.h b/include/uapi/linux/tc_act/tc_connmark.h
new file mode 100644
index 0000000..82eda46
--- /dev/null
+++ b/include/uapi/linux/tc_act/tc_connmark.h
@@ -0,0 +1,22 @@
+#ifndef __UAPI_TC_CONNMARK_H
+#define __UAPI_TC_CONNMARK_H
+
+#include <linux/types.h>
+#include <linux/pkt_cls.h>
+
+#define TCA_ACT_CONNMARK 20
+
+struct tc_connmark {
+ tc_gen;
+ __u16 zone;
+};
+
+enum {
+ TCA_CONNMARK_UNSPEC,
+ TCA_CONNMARK_PARMS,
+ TCA_CONNMARK_TM,
+ __TCA_CONNMARK_MAX
+};
+#define TCA_CONNMARK_MAX (__TCA_CONNMARK_MAX - 1)
+
+#endif
diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index c54c9d9..db20cae 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -698,6 +698,17 @@ config NET_ACT_VLAN
To compile this code as a module, choose M here: the
module will be called act_vlan.
+config NET_ACT_CONNMARK
+ tristate "Netfilter Connection Mark Retriever"
+ depends on NET_CLS_ACT && NETFILTER && IP_NF_IPTABLES
+ ---help---
+ Say Y here to allow retrieving of conn mark
+
+ If unsure, say N.
+
+ To compile this code as a module, choose M here: the
+ module will be called act_connmark.
+
config NET_CLS_IND
bool "Incoming device classification"
depends on NET_CLS_U32 || NET_CLS_FW
diff --git a/net/sched/Makefile b/net/sched/Makefile
index 679f24a..47304cd 100644
--- a/net/sched/Makefile
+++ b/net/sched/Makefile
@@ -17,6 +17,7 @@ obj-$(CONFIG_NET_ACT_SIMP) += act_simple.o
obj-$(CONFIG_NET_ACT_SKBEDIT) += act_skbedit.o
obj-$(CONFIG_NET_ACT_CSUM) += act_csum.o
obj-$(CONFIG_NET_ACT_VLAN) += act_vlan.o
+obj-$(CONFIG_NET_ACT_CONNMARK) += act_connmark.o
obj-$(CONFIG_NET_SCH_FIFO) += sch_fifo.o
obj-$(CONFIG_NET_SCH_CBQ) += sch_cbq.o
obj-$(CONFIG_NET_SCH_HTB) += sch_htb.o
diff --git a/net/sched/act_connmark.c b/net/sched/act_connmark.c
new file mode 100644
index 0000000..f936434
--- /dev/null
+++ b/net/sched/act_connmark.c
@@ -0,0 +1,211 @@
+/*
+ * net/sched/act_connmark.c netfilter connmark retriever action
+ * skb mark is over-written
+ *
+ * Copyright (c) 2011 Felix Fietkau <nbd@openwrt.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program; if not, write to the Free Software Foundation, Inc., 59 Temple
+ * Place - Suite 330, Boston, MA 02111-1307 USA.
+ *
+ *
+*/
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/skbuff.h>
+#include <linux/rtnetlink.h>
+#include <linux/pkt_cls.h>
+#include <linux/ip.h>
+#include <linux/ipv6.h>
+#include <net/netlink.h>
+#include <net/pkt_sched.h>
+#include <net/act_api.h>
+#include <uapi/linux/tc_act/tc_connmark.h>
+#include <net/tc_act/tc_connmark.h>
+
+#include <net/netfilter/nf_conntrack.h>
+#include <net/netfilter/nf_conntrack_core.h>
+#include <net/netfilter/nf_conntrack_zones.h>
+
+#define CONNMARK_TAB_MASK 3
+
+static struct tcf_hashinfo connmark_hash_info;
+
+static int tcf_connmark(struct sk_buff *skb, const struct tc_action *a,
+ struct tcf_result *res)
+{
+ const struct nf_conntrack_tuple_hash *thash;
+ struct nf_conntrack_tuple tuple;
+ enum ip_conntrack_info ctinfo;
+ struct tcf_connmark_info *ca = a->priv;
+ struct nf_conn *c;
+ int proto;
+
+ spin_lock(&ca->tcf_lock);
+ ca->tcf_tm.lastuse = jiffies;
+ bstats_update(&ca->tcf_bstats, skb);
+
+ if (skb->protocol == htons(ETH_P_IP)) {
+ if (skb->len < sizeof(struct iphdr)) {
+ goto out;
+ }
+ proto = NFPROTO_IPV4;
+ } else if (skb->protocol == htons(ETH_P_IPV6)) {
+ if (skb->len < sizeof(struct ipv6hdr)) {
+ goto out;
+ }
+ proto = NFPROTO_IPV6;
+ } else {
+ goto out;
+ }
+
+ c = nf_ct_get(skb, &ctinfo);
+ if (c != NULL) {
+ skb->mark = c->mark;
+ /* using overlimits stats to count how many packets marked */
+ ca->tcf_qstats.overlimits++;
+ nf_ct_put(c);
+ goto out;
+ }
+
+ if (!nf_ct_get_tuplepr(skb, skb_network_offset(skb),
+ proto, &tuple))
+ goto out;
+
+ thash = nf_conntrack_find_get(dev_net(skb->dev), ca->zone, &tuple);
+ if (!thash)
+ goto out;
+
+ c = nf_ct_tuplehash_to_ctrack(thash);
+ /* using overlimits stats to count how many packets marked */
+ ca->tcf_qstats.overlimits++;
+ skb->mark = c->mark;
+ nf_ct_put(c);
+
+out:
+ skb->nfct = NULL;
+ spin_unlock(&ca->tcf_lock);
+ return ca->tcf_action;
+}
+
+static const struct nla_policy connmark_policy[TCA_CONNMARK_MAX + 1] = {
+ [TCA_CONNMARK_PARMS] = { .len = sizeof(struct tc_connmark) },
+};
+
+static int tcf_connmark_init(struct net *net, struct nlattr *nla,
+ struct nlattr *est, struct tc_action *a,
+ int ovr, int bind)
+{
+ struct nlattr *tb[TCA_CONNMARK_MAX + 1];
+ struct tcf_connmark_info *ci;
+ struct tc_connmark *parm;
+ int ret = 0;
+
+ if (nla == NULL)
+ return -EINVAL;
+
+ ret = nla_parse_nested(tb, TCA_CONNMARK_MAX, nla, connmark_policy);
+ if (ret < 0)
+ return ret;
+
+ parm = nla_data(tb[TCA_CONNMARK_PARMS]);
+
+ if (!tcf_hash_check(parm->index, a, bind)) {
+ ret = tcf_hash_create(parm->index, est, a, sizeof(*ci), bind);
+ if (ret)
+ return ret;
+
+ ci = to_connmark(a);
+ ci->tcf_action = parm->action;
+ ci->zone = parm->zone;
+
+ tcf_hash_insert(a);
+ ret = ACT_P_CREATED;
+ } else {
+ ci = to_connmark(a);
+ if (bind)
+ return 0;
+ tcf_hash_release(a, bind);
+ if (!ovr)
+ return -EEXIST;
+ /* replacing action and zone */
+ ci->tcf_action = parm->action;
+ ci->zone = parm->zone;
+ }
+
+ return ret;
+}
+
+static inline int tcf_connmark_dump(struct sk_buff *skb, struct tc_action *a,
+ int bind, int ref)
+{
+ unsigned char *b = skb_tail_pointer(skb);
+ struct tcf_connmark_info *ci = a->priv;
+
+ struct tc_connmark opt = {
+ .index = ci->tcf_index,
+ .refcnt = ci->tcf_refcnt - ref,
+ .bindcnt = ci->tcf_bindcnt - bind,
+ .action = ci->tcf_action,
+ .zone = ci->zone,
+ };
+ struct tcf_t t;
+
+ if (nla_put(skb, TCA_CONNMARK_PARMS, sizeof(opt), &opt))
+ goto nla_put_failure;
+
+ t.install = jiffies_to_clock_t(jiffies - ci->tcf_tm.install);
+ t.lastuse = jiffies_to_clock_t(jiffies - ci->tcf_tm.lastuse);
+ t.expires = jiffies_to_clock_t(ci->tcf_tm.expires);
+ if (nla_put(skb, TCA_CONNMARK_TM, sizeof(t), &t))
+ goto nla_put_failure;
+
+ return skb->len;
+nla_put_failure:
+ nlmsg_trim(skb, b);
+ return -1;
+}
+
+static struct tc_action_ops act_connmark_ops = {
+ .kind = "connmark",
+ .hinfo = &connmark_hash_info,
+ .type = TCA_ACT_CONNMARK,
+ .owner = THIS_MODULE,
+ .act = tcf_connmark,
+ .dump = tcf_connmark_dump,
+ .init = tcf_connmark_init,
+};
+
+MODULE_AUTHOR("Felix Fietkau <nbd@openwrt.org>");
+MODULE_DESCRIPTION("Connection tracking mark restoring");
+MODULE_LICENSE("GPL");
+
+static int __init connmark_init_module(void)
+{
+ int ret;
+
+ ret = tcf_hashinfo_init(&connmark_hash_info, CONNMARK_TAB_MASK);
+ if (ret)
+ return ret;
+
+ return tcf_register_action(&act_connmark_ops, CONNMARK_TAB_MASK);
+}
+
+static void __exit connmark_cleanup_module(void)
+{
+ tcf_unregister_action(&act_connmark_ops);
+}
+
+module_init(connmark_init_module);
+module_exit(connmark_cleanup_module);
--
1.7.9.5
^ permalink raw reply related
* [PATCH] net: xfrm: xfrm_algo: Remove unused function
From: Rickard Strandqvist @ 2015-01-11 13:03 UTC (permalink / raw)
To: Steffen Klassert, Herbert Xu
Cc: Rickard Strandqvist, David S. Miller, netdev, linux-kernel
Remove the function aead_entries() that is not used anywhere.
This was partially found by using a static code analysis program called cppcheck.
Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
---
net/xfrm/xfrm_algo.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/net/xfrm/xfrm_algo.c b/net/xfrm/xfrm_algo.c
index debe733..12e82a5 100644
--- a/net/xfrm/xfrm_algo.c
+++ b/net/xfrm/xfrm_algo.c
@@ -561,11 +561,6 @@ static struct xfrm_algo_desc calg_list[] = {
},
};
-static inline int aead_entries(void)
-{
- return ARRAY_SIZE(aead_list);
-}
-
static inline int aalg_entries(void)
{
return ARRAY_SIZE(aalg_list);
--
1.7.10.4
^ permalink raw reply related
* Re: /proc/net/dev regression
From: Carlos R. Mafra @ 2015-01-11 13:40 UTC (permalink / raw)
To: Al Viro; +Cc: LKML, Hauke Mehrtens, John W. Linville, netdev
In-Reply-To: <20150111013913.GE22149@ZenIV.linux.org.uk>
On Sun, 11 Jan 2015 at 1:39:13 +0000, Al Viro wrote:
> On Sun, Jan 11, 2015 at 01:33:35AM +0000, Carlos R. Mafra wrote:
>
> > I think the problem with wmnet is not that it was expecting the fields
> > to be aligned because it never had problems before (when definitely more
> > than 10 megabytes were received, wmnet is crappy but not _that_ crappy).
> >
> > I think the problem really was here,
> >
> > totalbytes_in = strtoul(&buffer[7], NULL, 10);
> >
> > After the patch the device name is 8 characters long and &buffer[7]
> > overlaps with the name instead of reading the bytes. Before the
> > patch is was fine because the call to strtoul() seems correct in the
> > sense that it would read everything until the NULL. So more than 10
> > megabytes was still ok.
> >
> > So I guess I was wrong when suggesting that the problem was the
> > alignment.
>
> Several lines below there's this:
> totalpackets_out = strtoul(&buffer[74], NULL, 10);
> if (totalpackets_out != lastpackets_out) {
> totalbytes_out = strtoul(&buffer[66], NULL, 10);
> diffpackets_out += totalpackets_out - lastpackets_out;
> diffbytes_out += totalbytes_out - lastbytes_out;
> lastpackets_out = totalpackets_out;
> lastbytes_out = totalbytes_out;
> tx = True;
> }
>
> So I'm afraid it *is* that crappy. This function really should use scanf();
> note that updateStats_ipchains() in the same file does just that (well,
> fgets()+sscanf() for fsck knows what reason). And I'd be careful with all
> those %d, actually - it's not _that_ hard to get more than 4Gb sent.
> scanf formats really ought to match the kernel-side (seq_)printf one...
Ok, I fixed wmnet using Al's suggestion.
As far as I'm concerned, my regression complaint can be dismissed. It's
all working fine again.
Thanks!
^ permalink raw reply
* [PATCH] net: sched: sch_teql: Remove unused function
From: Rickard Strandqvist @ 2015-01-11 14:08 UTC (permalink / raw)
To: Jamal Hadi Salim, David S. Miller
Cc: Rickard Strandqvist, netdev, linux-kernel
Remove the function teql_neigh_release() that is not used anywhere.
This was partially found by using a static code analysis program called cppcheck.
Signed-off-by: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
---
net/sched/sch_teql.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
index 6ada423..4899d4a 100644
--- a/net/sched/sch_teql.c
+++ b/net/sched/sch_teql.c
@@ -122,13 +122,6 @@ teql_peek(struct Qdisc *sch)
return NULL;
}
-static inline void
-teql_neigh_release(struct neighbour *n)
-{
- if (n)
- neigh_release(n);
-}
-
static void
teql_reset(struct Qdisc *sch)
{
--
1.7.10.4
^ permalink raw reply related
* [iproute2 PATCH 1/1] actions: Get vlan action to work in pipeline
From: Jamal Hadi Salim @ 2015-01-11 14:31 UTC (permalink / raw)
To: stephen; +Cc: jiri, netdev, Jamal Hadi Salim
From: Jamal Hadi Salim <jhs@mojatatu.com>
When specified in a graph such as:
action vlan ... action foobar
the vlan action chewed more than it can swallow
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
tc/m_vlan.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/tc/m_vlan.c b/tc/m_vlan.c
index 171d268..32db5ed 100644
--- a/tc/m_vlan.c
+++ b/tc/m_vlan.c
@@ -103,20 +103,25 @@ static int parse_vlan(struct action_util *a, int *argc_p, char ***argv_p,
if (argc) {
if (matches(*argv, "reclassify") == 0) {
parm.action = TC_ACT_RECLASSIFY;
- NEXT_ARG();
+ argc--;
+ argv++;
} else if (matches(*argv, "pipe") == 0) {
parm.action = TC_ACT_PIPE;
- NEXT_ARG();
+ argc--;
+ argv++;
} else if (matches(*argv, "drop") == 0 ||
matches(*argv, "shot") == 0) {
parm.action = TC_ACT_SHOT;
- NEXT_ARG();
+ argc--;
+ argv++;
} else if (matches(*argv, "continue") == 0) {
parm.action = TC_ACT_UNSPEC;
- NEXT_ARG();
+ argc--;
+ argv++;
} else if (matches(*argv, "pass") == 0) {
parm.action = TC_ACT_OK;
- NEXT_ARG();
+ argc--;
+ argv++;
}
}
@@ -198,6 +203,7 @@ static int print_vlan(struct action_util *au, FILE *f, struct rtattr *arg)
}
break;
}
+ fprintf(f, " %s", action_n2a(parm->action, b1, sizeof (b1)));
fprintf(f, "\n\t index %d ref %d bind %d", parm->index, parm->refcnt,
parm->bindcnt);
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH v4] can: Convert to runtime_pm
From: Marc Kleine-Budde @ 2015-01-11 15:41 UTC (permalink / raw)
To: Appana Durga Kedareswara Rao
Cc: linux-can@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Soren Brinkmann,
grant.likely@linaro.org, wg@grandegger.com
In-Reply-To: <12e7202e137e4d5680185748ebf0cf2d@BY2FFO11FD060.protection.gbl>
[-- Attachment #1: Type: text/plain, Size: 5561 bytes --]
On 01/11/2015 06:34 AM, Appana Durga Kedareswara Rao wrote:
[...]
>>> return ret;
>>> }
>>>
>>> priv->write_reg(priv, XCAN_MSR_OFFSET, 0);
>>> priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_CEN_MASK);
>>>
>>> if (netif_running(ndev)) {
>>> priv->can.state = CAN_STATE_ERROR_ACTIVE;
>>
>> What happens if the device was not in ACTIVE state prior to the
>> runtime_suspend?
>>
>
> I am not sure about the state of CAN at this point of time.
> I just followed what other drivers are following for run time suspend :).
Please check the state of the hardware if you go with bus off into
suspend and then resume.
>>> netif_device_attach(ndev);
>>> netif_start_queue(ndev);
>>> }
>>>
>>> return 0;
>>> }
>>
>>
>>>
>>> @@ -1020,9 +1035,9 @@ static int __maybe_unused xcan_resume(struct
>>> device *dev)
>>>
>>> priv->write_reg(priv, XCAN_MSR_OFFSET, 0);
>>> priv->write_reg(priv, XCAN_SRR_OFFSET, XCAN_SRR_CEN_MASK);
>>> - priv->can.state = CAN_STATE_ERROR_ACTIVE;
>>>
>>> if (netif_running(ndev)) {
>>> + priv->can.state = CAN_STATE_ERROR_ACTIVE;
>>> netif_device_attach(ndev);
>>> netif_start_queue(ndev);
>>> }
>>> @@ -1030,7 +1045,10 @@ static int __maybe_unused xcan_resume(struct
>> device *dev)
>>> return 0;
>>> }
>>>
>>> -static SIMPLE_DEV_PM_OPS(xcan_dev_pm_ops, xcan_suspend,
>> xcan_resume);
>>> +static const struct dev_pm_ops xcan_dev_pm_ops = {
>>> + SET_SYSTEM_SLEEP_PM_OPS(xcan_suspend, xcan_resume)
>>> + SET_PM_RUNTIME_PM_OPS(xcan_runtime_suspend,
>> xcan_runtime_resume,
>>> +NULL) };
>>>
>>> /**
>>> * xcan_probe - Platform registration call @@ -1071,7 +1089,7 @@
>>> static int xcan_probe(struct platform_device *pdev)
>>> return -ENOMEM;
>>>
>>> priv = netdev_priv(ndev);
>>> - priv->dev = ndev;
>>> + priv->dev = &pdev->dev;
>>> priv->can.bittiming_const = &xcan_bittiming_const;
>>> priv->can.do_set_mode = xcan_do_set_mode;
>>> priv->can.do_get_berr_counter = xcan_get_berr_counter; @@ -
>> 1137,15
>>> +1155,22 @@ static int xcan_probe(struct platform_device *pdev)
>>>
>>> netif_napi_add(ndev, &priv->napi, xcan_rx_poll, rx_max);
>>>
>>> + pm_runtime_set_active(&pdev->dev);
>>> + pm_runtime_irq_safe(&pdev->dev);
>>> + pm_runtime_enable(&pdev->dev);
>>> + pm_runtime_get_sync(&pdev->dev);
>> Check error values?
>
> Ok
>>> +
>>> ret = register_candev(ndev);
>>> if (ret) {
>>> dev_err(&pdev->dev, "fail to register failed (err=%d)\n",
>> ret);
>>> + pm_runtime_put(priv->dev);
>>
>> Please move the pm_runtime_put into the common error exit path.
>>
>
> Ok
>
>>> goto err_unprepare_disable_busclk;
>>> }
>>>
>>> devm_can_led_init(ndev);
>>> - clk_disable_unprepare(priv->bus_clk);
>>> - clk_disable_unprepare(priv->can_clk);
>>> +
>>> + pm_runtime_put(&pdev->dev);
>>> +
>>> netdev_dbg(ndev, "reg_base=0x%p irq=%d clock=%d, tx fifo
>> depth:%d\n",
>>> priv->reg_base, ndev->irq, priv->can.clock.freq,
>>> priv->tx_max);
>>>
>>
>> I think you have to convert the _remove() function, too. Have a look at the
>> gpio-zynq.c driver:
>>
>>> static int zynq_gpio_remove(struct platform_device *pdev) {
>>> struct zynq_gpio *gpio = platform_get_drvdata(pdev);
>>>
>>> pm_runtime_get_sync(&pdev->dev);
>>
>> However I don't understand why the get_sync() is here. Maybe Sören can
>> help?
>
> I converted the remove function to use the run-time PM and .
> Below is the remove code snippet.
>
> ret = pm_runtime_get_sync(&pdev->dev);
> if (ret < 0) {
> netdev_err(ndev, "%s: pm_runtime_get failed(%d)\n",
> __func__, ret);
> return ret;
> }
>
> if (set_reset_mode(ndev) < 0)
> netdev_err(ndev, "mode resetting failed!\n");
>
> unregister_candev(ndev);
> netif_napi_del(&priv->napi);
> free_candev(ndev);
> pm_runtime_disable(&pdev->dev);
Can this make a call to xcan_runtime_*()? I'm asking since the ndev has
been unregistered and already free()ed. Better move this directly after
the set_reset_mode(). This way you are symmetric to the probe() function.
> Observed the below things in the /sys/kernel/debug/clk/clk_summary.
>
> Modprobe ifconfig up ifconfig down rmmod Modprobe ifconfig up ifconfig down rmmod Modprobe ifconfig up ifconfig down rmmod
> Clk_prepare count: 1 1 1 1 2 2 2 2 3 3 3 3
> Clk_enable count: 0 1 0 1 2 2 2 2 3 3 3 3
As the numbers are increasing you have a clk_prepare_enable() leak. Your
remove() function is missing the clk_disable_unprepare() for the can and
bus clock (as you have clk_prepare_enable in probe()).
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Industrial Linux Solutions | Phone: +49-231-2826-924 |
Vertretung West/Dortmund | Fax: +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de |
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH net] packet: bail out of packet_snd() if L2 header creation fails
From: Christoph Jaeger @ 2015-01-11 18:01 UTC (permalink / raw)
To: davem; +Cc: willemb, edumazet, dborkman, netdev, linux-kernel,
Christoph Jaeger
Due to a misplaced parenthesis, the expression
(unlikely(offset) < 0),
which expands to
(__builtin_expect(!!(offset), 0) < 0),
never evaluates to true. Therefore, when sending packets with
PF_PACKET/SOCK_DGRAM, packet_snd() does not abort as intended
if the creation of the layer 2 header fails.
Spotted by Coverity - CID 1259975 ("Operands don't affect result").
Fixes: 9c7077622dd9 ("packet: make packet_snd fail on len smaller than l2 header")
Signed-off-by: Christoph Jaeger <cj@linux.com>
---
net/packet/af_packet.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c
index 6880f34..9cfe2e1 100644
--- a/net/packet/af_packet.c
+++ b/net/packet/af_packet.c
@@ -2517,7 +2517,7 @@ static int packet_snd(struct socket *sock, struct msghdr *msg, size_t len)
err = -EINVAL;
if (sock->type == SOCK_DGRAM) {
offset = dev_hard_header(skb, dev, ntohs(proto), addr, NULL, len);
- if (unlikely(offset) < 0)
+ if (unlikely(offset < 0))
goto out_free;
} else {
if (ll_header_truncated(dev, len))
--
2.1.0
^ permalink raw reply related
* [PATCH net] alx: fix alx_poll()
From: Eric Dumazet @ 2015-01-11 18:32 UTC (permalink / raw)
To: Oded Gabbay
Cc: Johannes Berg, David S. Miller, Eric Dumazet,
netdev@vger.kernel.org, Willem de Bruijn, Bridgman, John,
Elifaz, Dana
In-Reply-To: <1420926614.5947.89.camel@edumazet-glaptop2.roam.corp.google.com>
From: Eric Dumazet <edumazet@google.com>
Commit d75b1ade567f ("net: less interrupt masking in NAPI") uncovered
wrong alx_poll() behavior.
A NAPI poll() handler is supposed to return exactly the budget when/if
napi_complete() has not been called.
It is also supposed to return number of frames that were received, so
that netdev_budget can have a meaning.
Also, in case of TX pressure, we still have to dequeue received
packets : alx_clean_rx_irq() has to be called even if
alx_clean_tx_irq(alx) returns false, otherwise device is half duplex.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Fixes: d75b1ade567f ("net: less interrupt masking in NAPI")
Reported-by: Oded Gabbay <oded.gabbay@amd.com>
Bisected-by: Oded Gabbay <oded.gabbay@amd.com>
Tested-by: Oded Gabbay <oded.gabbay@amd.com>
---
drivers/net/ethernet/atheros/alx/main.c | 24 +++++++++++-----------
1 file changed, 13 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
index e398eda07298..c8af3ce3ea38 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -184,15 +184,16 @@ static void alx_schedule_reset(struct alx_priv *alx)
schedule_work(&alx->reset_wk);
}
-static bool alx_clean_rx_irq(struct alx_priv *alx, int budget)
+static int alx_clean_rx_irq(struct alx_priv *alx, int budget)
{
struct alx_rx_queue *rxq = &alx->rxq;
struct alx_rrd *rrd;
struct alx_buffer *rxb;
struct sk_buff *skb;
u16 length, rfd_cleaned = 0;
+ int work = 0;
- while (budget > 0) {
+ while (work < budget) {
rrd = &rxq->rrd[rxq->rrd_read_idx];
if (!(rrd->word3 & cpu_to_le32(1 << RRD_UPDATED_SHIFT)))
break;
@@ -203,7 +204,7 @@ static bool alx_clean_rx_irq(struct alx_priv *alx, int budget)
ALX_GET_FIELD(le32_to_cpu(rrd->word0),
RRD_NOR) != 1) {
alx_schedule_reset(alx);
- return 0;
+ return work;
}
rxb = &rxq->bufs[rxq->read_idx];
@@ -243,7 +244,7 @@ static bool alx_clean_rx_irq(struct alx_priv *alx, int budget)
}
napi_gro_receive(&alx->napi, skb);
- budget--;
+ work++;
next_pkt:
if (++rxq->read_idx == alx->rx_ringsz)
@@ -258,21 +259,22 @@ next_pkt:
if (rfd_cleaned)
alx_refill_rx_ring(alx, GFP_ATOMIC);
- return budget > 0;
+ return work;
}
static int alx_poll(struct napi_struct *napi, int budget)
{
struct alx_priv *alx = container_of(napi, struct alx_priv, napi);
struct alx_hw *hw = &alx->hw;
- bool complete = true;
unsigned long flags;
+ bool tx_complete;
+ int work;
- complete = alx_clean_tx_irq(alx) &&
- alx_clean_rx_irq(alx, budget);
+ tx_complete = alx_clean_tx_irq(alx);
+ work = alx_clean_rx_irq(alx, budget);
- if (!complete)
- return 1;
+ if (!tx_complete || work == budget)
+ return budget;
napi_complete(&alx->napi);
@@ -284,7 +286,7 @@ static int alx_poll(struct napi_struct *napi, int budget)
alx_post_write(hw);
- return 0;
+ return work;
}
static irqreturn_t alx_intr_handle(struct alx_priv *alx, u32 intr)
^ permalink raw reply related
* Re: [PATCH net] packet: bail out of packet_snd() if L2 header creation fails
From: Eric Dumazet @ 2015-01-11 18:35 UTC (permalink / raw)
To: Christoph Jaeger; +Cc: davem, willemb, edumazet, dborkman, netdev, linux-kernel
In-Reply-To: <1420999276-28225-1-git-send-email-cj@linux.com>
On Sun, 2015-01-11 at 13:01 -0500, Christoph Jaeger wrote:
> Due to a misplaced parenthesis, the expression
>
> (unlikely(offset) < 0),
>
> which expands to
>
> (__builtin_expect(!!(offset), 0) < 0),
>
> never evaluates to true. Therefore, when sending packets with
> PF_PACKET/SOCK_DGRAM, packet_snd() does not abort as intended
> if the creation of the layer 2 header fails.
>
> Spotted by Coverity - CID 1259975 ("Operands don't affect result").
>
> Fixes: 9c7077622dd9 ("packet: make packet_snd fail on len smaller than l2 header")
> Signed-off-by: Christoph Jaeger <cj@linux.com>
> ---
Nice catch !
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply
* Re: [PATCH net] packet: bail out of packet_snd() if L2 header creation fails
From: Willem de Bruijn @ 2015-01-11 18:49 UTC (permalink / raw)
To: Eric Dumazet
Cc: Christoph Jaeger, David Miller, Eric Dumazet, Daniel Borkmann,
Network Development, linux-kernel
In-Reply-To: <1421001331.5947.101.camel@edumazet-glaptop2.roam.corp.google.com>
On Sun, Jan 11, 2015 at 1:35 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Sun, 2015-01-11 at 13:01 -0500, Christoph Jaeger wrote:
>> Due to a misplaced parenthesis, the expression
>>
>> (unlikely(offset) < 0),
>>
>> which expands to
>>
>> (__builtin_expect(!!(offset), 0) < 0),
>>
>> never evaluates to true. Therefore, when sending packets with
>> PF_PACKET/SOCK_DGRAM, packet_snd() does not abort as intended
>> if the creation of the layer 2 header fails.
>>
>> Spotted by Coverity - CID 1259975 ("Operands don't affect result").
>>
>> Fixes: 9c7077622dd9 ("packet: make packet_snd fail on len smaller than l2 header")
>> Signed-off-by: Christoph Jaeger <cj@linux.com>
>> ---
>
> Nice catch !
>
> Acked-by: Eric Dumazet <edumazet@google.com>
>
Indeed. I'm responsible for that typo. Thanks a lot for catching it!
Acked-by: Willem de Bruijn <willemb@google.com>
^ permalink raw reply
* Re: [PATCH net] packet: bail out of packet_snd() if L2 header creation fails
From: Joe Perches @ 2015-01-11 18:52 UTC (permalink / raw)
To: Christoph Jaeger, Alan
Cc: davem, willemb, edumazet, dborkman, netdev, linux-kernel
In-Reply-To: <1420999276-28225-1-git-send-email-cj@linux.com>
On Sun, 2015-01-11 at 13:01 -0500, Christoph Jaeger wrote:
> Due to a misplaced parenthesis, the expression
>
> (unlikely(offset) < 0),
>
> which expands to
>
> (__builtin_expect(!!(offset), 0) < 0),
Here's another one:
drivers/platform/goldfish/goldfish_pipe.c:285: if (unlikely(bufflen) == 0)
^ 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