Re: [PATCH] android: binder: no outgoing transaction when thread todo has transaction

2018-08-13 Thread Martijn Coenen
Sherry, this was found by syzkaller, right? In that case, can you add
the  tag so the issue gets closed automatically when this
gets merged?


On Tue, Aug 14, 2018 at 2:28 AM, Sherry Yang  wrote:
> When a process dies, failed reply is sent to the sender of any transaction
> queued on a dead thread's todo list. The sender asserts that the
> received failed reply corresponds to the head of the transaction stack.
> This assert can fail if the dead thread is allowed to send outgoing
> transactions when there is already a transaction on its todo list,
> because this new transaction can end up on the transaction stack of the
> original sender. The following steps illustrate how this assertion can
> fail.
>
> 1. Thread1 sends txn19 to Thread2
>(T1->transaction_stack=txn19, T2->todo+=txn19)
> 2. Without processing todo list, Thread2 sends txn20 to Thread1
>(T1->todo+=txn20, T2->transaction_stack=txn20)
> 3. T1 processes txn20 on its todo list
>(T1->transaction_stack=txn20->txn19, T1->todo=)
> 4. T2 dies, T2->todo cleanup attempts to send failed reply for txn19, but
>T1->transaction_stack points to txn20 -- assertion failes
>
> Step 2. is the incorrect behavior. When there is a transaction on a
> thread's todo list, this thread should not be able to send any outgoing
> synchronous transactions. Only the head of the todo list needs to be
> checked because only threads that are waiting for proc work can directly
> receive work from another thread, and no work is allowed to be queued
> on such a thread without waking up the thread. This patch also enforces
> that a thread is not waiting for proc work when a work is directly
> enqueued to its todo list.
>
> Acked-by: Arve Hjønnevåg 
> Signed-off-by: Sherry Yang 

Reviewed-by: Martijn Coenen 

Thanks,
Martijn

> ---
>  drivers/android/binder.c | 44 +---
>  1 file changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index d58763b6b009..f2081934eb8b 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -822,6 +822,7 @@ static void
>  binder_enqueue_deferred_thread_work_ilocked(struct binder_thread *thread,
> struct binder_work *work)
>  {
> +   WARN_ON(!list_empty(&thread->waiting_thread_node));
> binder_enqueue_work_ilocked(work, &thread->todo);
>  }
>
> @@ -839,6 +840,7 @@ static void
>  binder_enqueue_thread_work_ilocked(struct binder_thread *thread,
>struct binder_work *work)
>  {
> +   WARN_ON(!list_empty(&thread->waiting_thread_node));
> binder_enqueue_work_ilocked(work, &thread->todo);
> thread->process_todo = true;
>  }
> @@ -1270,19 +1272,12 @@ static int binder_inc_node_nilocked(struct 
> binder_node *node, int strong,
> } else
> node->local_strong_refs++;
> if (!node->has_strong_ref && target_list) {
> +   struct binder_thread *thread = 
> container_of(target_list,
> +   struct binder_thread, 
> todo);
> binder_dequeue_work_ilocked(&node->work);
> -   /*
> -* Note: this function is the only place where we 
> queue
> -* directly to a thread->todo without using the
> -* corresponding binder_enqueue_thread_work() helper
> -* functions; in this case it's ok to not set the
> -* process_todo flag, since we know this node work 
> will
> -* always be followed by other work that starts queue
> -* processing: in case of synchronous transactions, a
> -* BR_REPLY or BR_ERROR; in case of oneway
> -* transactions, a BR_TRANSACTION_COMPLETE.
> -*/
> -   binder_enqueue_work_ilocked(&node->work, target_list);
> +   BUG_ON(&thread->todo != target_list);
> +   binder_enqueue_deferred_thread_work_ilocked(thread,
> +  
> &node->work);
> }
> } else {
> if (!internal)
> @@ -2723,6 +2718,7 @@ static void binder_transaction(struct binder_proc *proc,
>  {
> int ret;
> struct binder_transaction *t;
> +   struct binder_work *w;
> struct binder_work *tcomplete;
> binder_size_t *offp, *off_end, *off_start;
> binder_size_t off_min;
> @@ -2864,6 +2860,29 @@ static void binder_transaction(struct binder_proc 
> *proc,
> goto err_invalid_target_handle;
> }
> binder_inner_proc_lock(proc);
> +
> +   w = list_first_entry_or_null(&thread->todo,
> + 

[PATCH 16/24] staging: wilc1000: refactor tcp_process() to avoid extra leading tabs

2018-08-13 Thread Ajay Singh
Refactor tcp_process() to avoid unnecessary leading tabs in the
function.

Signed-off-by: Ajay Singh 
---
 drivers/staging/wilc1000/wilc_wlan.c | 52 +++-
 1 file changed, 28 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_wlan.c 
b/drivers/staging/wilc1000/wilc_wlan.c
index 6787b6e..bf45b4c 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -162,42 +162,46 @@ static inline void tcp_process(struct net_device *dev, 
struct txq_entry_t *tqe)
unsigned long flags;
struct wilc_vif *vif = netdev_priv(dev);
struct wilc *wilc = vif->wilc;
+   const struct iphdr *ip_hdr_ptr;
+   const struct tcphdr *tcp_hdr_ptr;
+   u32 IHL, total_length, data_offset;
 
spin_lock_irqsave(&wilc->txq_spinlock, flags);
 
-   if (eth_hdr_ptr->h_proto == htons(ETH_P_IP)) {
-   const struct iphdr *ip_hdr_ptr = buffer + ETH_HLEN;
+   if (eth_hdr_ptr->h_proto != htons(ETH_P_IP))
+   goto out;
 
-   if (ip_hdr_ptr->protocol == IPPROTO_TCP) {
-   const struct tcphdr *tcp_hdr_ptr;
-   u32 IHL, total_length, data_offset;
+   ip_hdr_ptr = buffer + ETH_HLEN;
 
-   IHL = ip_hdr_ptr->ihl << 2;
-   tcp_hdr_ptr = buffer + ETH_HLEN + IHL;
-   total_length = ntohs(ip_hdr_ptr->tot_len);
+   if (ip_hdr_ptr->protocol != IPPROTO_TCP)
+   goto out;
 
-   data_offset = tcp_hdr_ptr->doff << 2;
-   if (total_length == (IHL + data_offset)) {
-   u32 seq_no, ack_no;
+   IHL = ip_hdr_ptr->ihl << 2;
+   tcp_hdr_ptr = buffer + ETH_HLEN + IHL;
+   total_length = ntohs(ip_hdr_ptr->tot_len);
 
-   seq_no = ntohl(tcp_hdr_ptr->seq);
-   ack_no = ntohl(tcp_hdr_ptr->ack_seq);
-   for (i = 0; i < tcp_session; i++) {
-   u32 j = ack_session_info[i].seq_num;
+   data_offset = tcp_hdr_ptr->doff << 2;
+   if (total_length == (IHL + data_offset)) {
+   u32 seq_no, ack_no;
 
-   if (i < 2 * MAX_TCP_SESSION &&
-   j == seq_no) {
-   update_tcp_session(i, ack_no);
-   break;
-   }
-   }
-   if (i == tcp_session)
-   add_tcp_session(0, 0, seq_no);
+   seq_no = ntohl(tcp_hdr_ptr->seq);
+   ack_no = ntohl(tcp_hdr_ptr->ack_seq);
+   for (i = 0; i < tcp_session; i++) {
+   u32 j = ack_session_info[i].seq_num;
 
-   add_tcp_pending_ack(ack_no, i, tqe);
+   if (i < 2 * MAX_TCP_SESSION &&
+   j == seq_no) {
+   update_tcp_session(i, ack_no);
+   break;
}
}
+   if (i == tcp_session)
+   add_tcp_session(0, 0, seq_no);
+
+   add_tcp_pending_ack(ack_no, i, tqe);
}
+
+out:
spin_unlock_irqrestore(&wilc->txq_spinlock, flags);
 }
 
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 12/24] staging: wilc1000: move static variable 'terminated_handle' to wilc_vif struct

2018-08-13 Thread Ajay Singh
Remove the use of static variable 'terminated_handle' and instead move
in wilc_vif struct.
After moving this variable to wilc_vif struct its not required to keep
'terminated_handle', so changed it to boolean type.

Signed-off-by: Ajay Singh 
---
 drivers/staging/wilc1000/host_interface.c | 11 +--
 drivers/staging/wilc1000/wilc_wfi_netdevice.h |  1 +
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index d5d81843..f71f451f 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -185,7 +185,6 @@ struct join_bss_param {
u8 start_time[4];
 };
 
-static struct host_if_drv *terminated_handle;
 static u8 p2p_listen_state;
 static struct timer_list periodic_rssi;
 static struct wilc_vif *periodic_rssi_vif;
@@ -3505,7 +3504,7 @@ int wilc_deinit(struct wilc_vif *vif)
 
mutex_lock(&vif->wilc->hif_deinit_lock);
 
-   terminated_handle = hif_drv;
+   vif->is_termination_progress = true;
 
del_timer_sync(&hif_drv->scan_timer);
del_timer_sync(&hif_drv->connect_timer);
@@ -3543,7 +3542,7 @@ int wilc_deinit(struct wilc_vif *vif)
kfree(hif_drv);
 
vif->wilc->clients_count--;
-   terminated_handle = NULL;
+   vif->is_termination_progress = false;
mutex_unlock(&vif->wilc->hif_deinit_lock);
return result;
 }
@@ -3565,7 +3564,7 @@ void wilc_network_info_received(struct wilc *wilc, u8 
*buffer, u32 length)
return;
hif_drv = vif->hif_drv;
 
-   if (!hif_drv || hif_drv == terminated_handle) {
+   if (!hif_drv || vif->is_termination_progress) {
netdev_err(vif->ndev, "driver not init[%p]\n", hif_drv);
return;
}
@@ -3611,7 +3610,7 @@ void wilc_gnrl_async_info_received(struct wilc *wilc, u8 
*buffer, u32 length)
 
hif_drv = vif->hif_drv;
 
-   if (!hif_drv || hif_drv == terminated_handle) {
+   if (!hif_drv || vif->is_termination_progress) {
mutex_unlock(&wilc->hif_deinit_lock);
return;
}
@@ -3662,7 +3661,7 @@ void wilc_scan_complete_received(struct wilc *wilc, u8 
*buffer, u32 length)
return;
hif_drv = vif->hif_drv;
 
-   if (!hif_drv || hif_drv == terminated_handle)
+   if (!hif_drv || vif->is_termination_progress)
return;
 
if (hif_drv->usr_scan_req.scan_result) {
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h 
b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index eb00e42..ba606d0 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -121,6 +121,7 @@ struct wilc_vif {
struct timer_list during_ip_timer;
bool obtaining_ip;
u8 mc_mac_addr_list[WILC_MULTICAST_TABLE_SIZE][ETH_ALEN];
+   bool is_termination_progress;
 };
 
 struct wilc {
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 20/24] staging: wilc1000: avoid line over 80 chars in tcp_process()

2018-08-13 Thread Ajay Singh
Cleanup patch to avoid line over 80 chars issue reported by
checkpatch.pl script.

Signed-off-by: Ajay Singh 
---
 drivers/staging/wilc1000/wilc_wlan.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/wilc1000/wilc_wlan.c 
b/drivers/staging/wilc1000/wilc_wlan.c
index 041c9dd..f0743d9 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -137,6 +137,11 @@ static inline int add_tcp_pending_ack(struct wilc_vif 
*vif, u32 ack,
return 0;
 }
 
+static inline void clear_tcp_session_txq(struct wilc_vif *vif, int index)
+{
+   vif->ack_filter.pending_acks_info[index].txqe = NULL;
+}
+
 static inline void tcp_process(struct net_device *dev, struct txq_entry_t *tqe)
 {
void *buffer = tqe->buffer;
@@ -670,7 +675,7 @@ int wilc_wlan_handle_txq(struct net_device *dev, u32 
*txq_count)
tqe->tx_complete_func(tqe->priv, tqe->status);
if (tqe->tcp_pending_ack_idx != NOT_TCP_ACK &&
tqe->tcp_pending_ack_idx < MAX_PENDING_ACKS)
-   
vif->ack_filter.pending_acks_info[tqe->tcp_pending_ack_idx].txqe = NULL;
+   clear_tcp_session_txq(vif, tqe->tcp_pending_ack_idx);
kfree(tqe);
} while (--entries);
 
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 14/24] staging: wilc1000: rename 'dummy_statistics' variable to 'periodic_stat'

2018-08-13 Thread Ajay Singh
Cleanup patch to use appropriate variable name to fetch the periodic
statistics.

Signed-off-by: Ajay Singh 
---
 drivers/staging/wilc1000/host_interface.c | 2 +-
 drivers/staging/wilc1000/wilc_wfi_netdevice.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index a440f84..98098fb 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -3425,7 +3425,7 @@ static void get_periodic_rssi(struct timer_list *t)
}
 
if (vif->hif_drv->hif_state == HOST_IF_CONNECTED)
-   wilc_get_statistics(vif, &vif->dummy_statistics, false);
+   wilc_get_statistics(vif, &vif->periodic_stat, false);
 
mod_timer(&vif->periodic_rssi, jiffies + msecs_to_jiffies(5000));
 }
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h 
b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index cdb90c3..c53d38f 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -123,7 +123,7 @@ struct wilc_vif {
u8 mc_mac_addr_list[WILC_MULTICAST_TABLE_SIZE][ETH_ALEN];
bool is_termination_progress;
struct timer_list periodic_rssi;
-   struct rf_info dummy_statistics;
+   struct rf_info periodic_stat;
 };
 
 struct wilc {
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 15/24] staging: wilc1000: move 'rcv_assoc_resp' as part of hif_drv

2018-08-13 Thread Ajay Singh
Avoid use of static variable and move 'rcv_assoc_resp' as part of
'hif_drv' struct. Rename from 'rcv_assoc_resp' to 'assoc_resp'.

Signed-off-by: Ajay Singh 
---
 drivers/staging/wilc1000/host_interface.c | 8 +++-
 drivers/staging/wilc1000/host_interface.h | 1 +
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index 98098fb..4c148bc 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -187,8 +187,6 @@ struct join_bss_param {
 
 static u8 p2p_listen_state;
 
-static u8 rcv_assoc_resp[MAX_ASSOC_RESP_FRAME_SIZE];
-
 static u8 set_ip[2][4];
 static u8 get_ip[2][4];
 
@@ -1498,16 +1496,16 @@ static inline void 
host_int_parse_assoc_resp_info(struct wilc_vif *vif,
if (mac_status == MAC_STATUS_CONNECTED) {
u32 assoc_resp_info_len;
 
-   memset(rcv_assoc_resp, 0, MAX_ASSOC_RESP_FRAME_SIZE);
+   memset(hif_drv->assoc_resp, 0, MAX_ASSOC_RESP_FRAME_SIZE);
 
-   host_int_get_assoc_res_info(vif, rcv_assoc_resp,
+   host_int_get_assoc_res_info(vif, hif_drv->assoc_resp,
MAX_ASSOC_RESP_FRAME_SIZE,
&assoc_resp_info_len);
 
if (assoc_resp_info_len != 0) {
s32 err = 0;
 
-   err = wilc_parse_assoc_resp_info(rcv_assoc_resp,
+   err = wilc_parse_assoc_resp_info(hif_drv->assoc_resp,
 assoc_resp_info_len,
 &conn_info);
if (err)
diff --git a/drivers/staging/wilc1000/host_interface.h 
b/drivers/staging/wilc1000/host_interface.h
index 4a84dd2..80cb298 100644
--- a/drivers/staging/wilc1000/host_interface.h
+++ b/drivers/staging/wilc1000/host_interface.h
@@ -284,6 +284,7 @@ struct host_if_drv {
 
bool ifc_up;
int driver_handler_id;
+   u8 assoc_resp[MAX_ASSOC_RESP_FRAME_SIZE];
 };
 
 struct add_sta_param {
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 23/24] staging: wilc1000: move 'wilc_connecting' static variable to 'wilc_vif' struct

2018-08-13 Thread Ajay Singh
Move static variable 'wilc_connecting' as part of 'wilc_vif' private
struct. Remove "wilc_" prefix from name as its already part of wilc_vif
struct.

Signed-off-by: Ajay Singh 
---
 drivers/staging/wilc1000/host_interface.c |  4 ++--
 drivers/staging/wilc1000/host_interface.h |  2 --
 drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 18 --
 drivers/staging/wilc1000/wilc_wfi_netdevice.h |  1 +
 4 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index f37ba64..d8cc08b 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -720,7 +720,7 @@ static void handle_scan(struct work_struct *work)
goto error;
}
 
-   if (vif->obtaining_ip || wilc_connecting) {
+   if (vif->obtaining_ip || vif->connecting) {
netdev_err(vif->ndev, "Don't do obss scan\n");
result = -EBUSY;
goto error;
@@ -2326,7 +2326,7 @@ static int handle_remain_on_chan(struct wilc_vif *vif,
goto error;
}
 
-   if (vif->obtaining_ip || wilc_connecting) {
+   if (vif->obtaining_ip || vif->connecting) {
result = -EBUSY;
goto error;
}
diff --git a/drivers/staging/wilc1000/host_interface.h 
b/drivers/staging/wilc1000/host_interface.h
index 0f0d509..4048eab 100644
--- a/drivers/staging/wilc1000/host_interface.h
+++ b/drivers/staging/wilc1000/host_interface.h
@@ -361,6 +361,4 @@ int wilc_get_tx_power(struct wilc_vif *vif, u8 *tx_power);
 
 extern u8 wilc_connected_ssid[6];
 
-extern int wilc_connecting;
-
 #endif
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c 
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index 35a83d4..cc44640 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -453,8 +453,6 @@ static inline bool wilc_cfg_scan_time_expired(struct 
wilc_priv *priv, int i)
return false;
 }
 
-int wilc_connecting;
-
 static void cfg_connect_result(enum conn_event conn_disconn_evt,
   struct connect_info *conn_info,
   u8 mac_status,
@@ -468,7 +466,7 @@ static void cfg_connect_result(enum conn_event 
conn_disconn_evt,
struct host_if_drv *wfi_drv = priv->hif_drv;
u8 null_bssid[ETH_ALEN] = {0};
 
-   wilc_connecting = 0;
+   vif->connecting = 0;
 
if (conn_disconn_evt == CONN_DISCONN_EVENT_CONN_RESP) {
u16 connect_status;
@@ -666,7 +664,7 @@ static int connect(struct wiphy *wiphy, struct net_device 
*dev,
enum authtype auth_type = ANY;
u32 cipher_group;
 
-   wilc_connecting = 1;
+   vif->connecting = 1;
 
if (!(strncmp(sme->ssid, "DIRECT-", 7)))
wfi_drv->p2p_connect = 1;
@@ -698,7 +696,7 @@ static int connect(struct wiphy *wiphy, struct net_device 
*dev,
nw_info = &priv->scanned_shadow[sel_bssi_idx];
} else {
ret = -ENOENT;
-   wilc_connecting = 0;
+   vif->connecting = 0;
return ret;
}
 
@@ -741,7 +739,7 @@ static int connect(struct wiphy *wiphy, struct net_device 
*dev,
ret = -ENOTSUPP;
netdev_err(dev, "%s: Unsupported cipher\n",
   __func__);
-   wilc_connecting = 0;
+   vif->connecting = 0;
return ret;
}
}
@@ -792,7 +790,7 @@ static int connect(struct wiphy *wiphy, struct net_device 
*dev,
if (ret != 0) {
netdev_err(dev, "wilc_set_join_req(): Error\n");
ret = -ENOENT;
-   wilc_connecting = 0;
+   vif->connecting = 0;
return ret;
}
 
@@ -809,7 +807,7 @@ static int disconnect(struct wiphy *wiphy, struct 
net_device *dev,
int ret;
u8 null_bssid[ETH_ALEN] = {0};
 
-   wilc_connecting = 0;
+   vif->connecting = 0;
 
if (!wilc)
return -EIO;
@@ -1747,7 +1745,7 @@ static int change_virtual_intf(struct wiphy *wiphy, 
struct net_device *dev,
 
switch (type) {
case NL80211_IFTYPE_STATION:
-   wilc_connecting = 0;
+   vif->connecting = 0;
dev->ieee80211_ptr->iftype = type;
priv->wdev->iftype = type;
vif->monitor_flag = 0;
@@ -1762,7 +1760,7 @@ static int change_virtual_intf(struct wiphy *wiphy, 
struct net_device *dev,
break;
 
case NL80211_IFTYPE_P2P_CLIENT:
-   wilc_connecting = 0;
+   vif->connecting = 0;
dev->ieee80211_ptr->iftype = type;
priv->wdev->iftype = type;
vif->monitor_flag = 0;
diff --git a/drivers/st

[PATCH 18/24] staging: wilc1000: move tcp_ack_filter algo related variables to 'wilc_vif' struct

2018-08-13 Thread Ajay Singh
Avoid use of static variables and move them as part of wilc_vif struct.
Move all the parameters related to tcp_ack_filter algo to wilc_vif
struct.

Signed-off-by: Ajay Singh 
---
 drivers/staging/wilc1000/host_interface.c |   4 +-
 drivers/staging/wilc1000/wilc_wfi_cfgoperations.c |   4 +-
 drivers/staging/wilc1000/wilc_wfi_netdevice.h |  27 ++
 drivers/staging/wilc1000/wilc_wlan.c  | 107 +-
 drivers/staging/wilc1000/wilc_wlan.h  |   2 +-
 5 files changed, 77 insertions(+), 67 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index 4c148bc..bffe0c8 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -2068,9 +2068,9 @@ static void handle_get_statistics(struct work_struct 
*work)
 
if (stats->link_speed > TCP_ACK_FILTER_LINK_SPEED_THRESH &&
stats->link_speed != DEFAULT_LINK_SPEED)
-   wilc_enable_tcp_ack_filter(true);
+   wilc_enable_tcp_ack_filter(vif, true);
else if (stats->link_speed != DEFAULT_LINK_SPEED)
-   wilc_enable_tcp_ack_filter(false);
+   wilc_enable_tcp_ack_filter(vif, false);
 
/* free 'msg' for async command, for sync caller will free it */
if (msg->is_sync)
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c 
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index 3418d2d..35a83d4 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -1125,9 +1125,9 @@ static int get_station(struct wiphy *wiphy, struct 
net_device *dev,
 
if (stats.link_speed > TCP_ACK_FILTER_LINK_SPEED_THRESH &&
stats.link_speed != DEFAULT_LINK_SPEED)
-   wilc_enable_tcp_ack_filter(true);
+   wilc_enable_tcp_ack_filter(vif, true);
else if (stats.link_speed != DEFAULT_LINK_SPEED)
-   wilc_enable_tcp_ack_filter(false);
+   wilc_enable_tcp_ack_filter(vif, false);
}
return 0;
 }
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h 
b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index c53d38f..54ce1ff 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -104,6 +104,32 @@ struct frame_reg {
bool reg;
 };
 
+#define MAX_TCP_SESSION25
+#define MAX_PENDING_ACKS   256
+
+struct ack_session_info {
+   u32 seq_num;
+   u32 bigger_ack_num;
+   u16 src_port;
+   u16 dst_port;
+   u16 status;
+};
+
+struct pending_acks_info {
+   u32 ack_num;
+   u32 session_index;
+   struct txq_entry_t  *txqe;
+};
+
+struct tcp_ack_filter {
+   struct ack_session_info ack_session_info[2 * MAX_TCP_SESSION];
+   struct pending_acks_info pending_acks_info[MAX_PENDING_ACKS];
+   u32 pending_base;
+   u32 tcp_session;
+   u32 pending_acks;
+   bool enabled;
+};
+
 struct wilc_vif {
u8 idx;
u8 iftype;
@@ -124,6 +150,7 @@ struct wilc_vif {
bool is_termination_progress;
struct timer_list periodic_rssi;
struct rf_info periodic_stat;
+   struct tcp_ack_filter ack_filter;
 };
 
 struct wilc {
diff --git a/drivers/staging/wilc1000/wilc_wlan.c 
b/drivers/staging/wilc1000/wilc_wlan.c
index d397c27..52402c3 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -93,63 +93,46 @@ static int wilc_wlan_txq_add_to_head(struct wilc_vif *vif,
return 0;
 }
 
-struct ack_session_info;
-struct ack_session_info {
-   u32 seq_num;
-   u32 bigger_ack_num;
-   u16 src_port;
-   u16 dst_port;
-   u16 status;
-};
-
-struct pending_acks_info {
-   u32 ack_num;
-   u32 session_index;
-   struct txq_entry_t  *txqe;
-};
-
 #define NOT_TCP_ACK(-1)
 
-#define MAX_TCP_SESSION25
-#define MAX_PENDING_ACKS   256
-static struct ack_session_info ack_session_info[2 * MAX_TCP_SESSION];
-static struct pending_acks_info pending_acks_info[MAX_PENDING_ACKS];
-
-static u32 pending_base;
-static u32 tcp_session;
-static u32 pending_acks;
-
-static inline int add_tcp_session(u32 src_prt, u32 dst_prt, u32 seq)
+static inline int add_tcp_session(struct wilc_vif *vif, u32 src_prt,
+ u32 dst_prt, u32 seq)
 {
-   if (tcp_session < 2 * MAX_TCP_SESSION) {
-   ack_session_info[tcp_session].seq_num = seq;
-   ack_session_info[tcp_session].bigger_ack_num = 0;
-   ack_session_info[tcp_session].src_port = src_prt;
-   ack_session_info[tcp_session].dst_port = dst_prt;
-   tcp_session++;
+   struct tcp_ack_filter *f = &vif->ack_filter;
+
+   if (f->tcp_session < 2 * MAX_TCP_SESSION) {
+  

[PATCH 11/24] staging: wilc1000: move hif specific static variables to 'wilc' structure

2018-08-13 Thread Ajay Singh
Avoid use of static variable and move it in 'wilc' structure related to
hif and added NULL before accessing hif_workqueue in wilc_enqueue_work().

Below variables are moved to 'wilc' struct:
 struct workqueue_struct *hif_workqueue;
 struct mutex hif_deinit_lock;
 struct completion hif_driver_comp;

Signed-off-by: Ajay Singh 
---
 drivers/staging/wilc1000/host_interface.c | 43 ++-
 drivers/staging/wilc1000/wilc_wfi_netdevice.h |  4 +++
 2 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index 642c314..d5d81843 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -187,9 +187,6 @@ struct join_bss_param {
 
 static struct host_if_drv *terminated_handle;
 static u8 p2p_listen_state;
-static struct workqueue_struct *hif_workqueue;
-static struct completion hif_driver_comp;
-static struct mutex hif_deinit_lock;
 static struct timer_list periodic_rssi;
 static struct wilc_vif *periodic_rssi_vif;
 
@@ -225,7 +222,11 @@ wilc_alloc_work(struct wilc_vif *vif, void 
(*work_fun)(struct work_struct *),
 static int wilc_enqueue_work(struct host_if_msg *msg)
 {
INIT_WORK(&msg->work, msg->fn);
-   if (!hif_workqueue || !queue_work(hif_workqueue, &msg->work))
+
+   if (!msg->vif || !msg->vif->wilc || !msg->vif->wilc->hif_workqueue)
+   return -EINVAL;
+
+   if (!queue_work(msg->vif->wilc->hif_workqueue, &msg->work))
return -EINVAL;
 
return 0;
@@ -316,7 +317,7 @@ static void handle_set_wfi_drv_handler(struct work_struct 
*work)
if (ret)
netdev_err(vif->ndev, "Failed to set driver handler\n");
 
-   complete(&hif_driver_comp);
+   complete(&vif->wilc->hif_driver_comp);
kfree(buffer);
 
 free_msg:
@@ -340,7 +341,7 @@ static void handle_set_operation_mode(struct work_struct 
*work)
   wilc_get_vif_idx(vif));
 
if (hif_op_mode->mode == IDLE_MODE)
-   complete(&hif_driver_comp);
+   complete(&vif->wilc->hif_driver_comp);
 
if (ret)
netdev_err(vif->ndev, "Failed to set operation mode\n");
@@ -3454,11 +3455,11 @@ int wilc_init(struct net_device *dev, struct 
host_if_drv **hif_drv_handler)
vif->obtaining_ip = false;
 
if (wilc->clients_count == 0) {
-   init_completion(&hif_driver_comp);
-   mutex_init(&hif_deinit_lock);
+   init_completion(&wilc->hif_driver_comp);
+   mutex_init(&wilc->hif_deinit_lock);
 
-   hif_workqueue = create_singlethread_workqueue("WILC_wq");
-   if (!hif_workqueue) {
+   wilc->hif_workqueue = create_singlethread_workqueue("WILC_wq");
+   if (!wilc->hif_workqueue) {
netdev_err(vif->ndev, "Failed to create workqueue\n");
kfree(hif_drv);
return -ENOMEM;
@@ -3502,7 +3503,7 @@ int wilc_deinit(struct wilc_vif *vif)
return -EFAULT;
}
 
-   mutex_lock(&hif_deinit_lock);
+   mutex_lock(&vif->wilc->hif_deinit_lock);
 
terminated_handle = hif_drv;
 
@@ -3512,7 +3513,7 @@ int wilc_deinit(struct wilc_vif *vif)
del_timer_sync(&hif_drv->remain_on_ch_timer);
 
wilc_set_wfi_drv_handler(vif, 0, 0, 0);
-   wait_for_completion(&hif_driver_comp);
+   wait_for_completion(&vif->wilc->hif_driver_comp);
 
if (hif_drv->usr_scan_req.scan_result) {
hif_drv->usr_scan_req.scan_result(SCAN_EVENT_ABORTED, NULL,
@@ -3536,14 +3537,14 @@ int wilc_deinit(struct wilc_vif *vif)
wait_for_completion(&msg->work_comp);
kfree(msg);
}
-   destroy_workqueue(hif_workqueue);
+   destroy_workqueue(vif->wilc->hif_workqueue);
}
 
kfree(hif_drv);
 
vif->wilc->clients_count--;
terminated_handle = NULL;
-   mutex_unlock(&hif_deinit_lock);
+   mutex_unlock(&vif->wilc->hif_deinit_lock);
return result;
 }
 
@@ -3596,7 +3597,7 @@ void wilc_gnrl_async_info_received(struct wilc *wilc, u8 
*buffer, u32 length)
struct host_if_drv *hif_drv;
struct wilc_vif *vif;
 
-   mutex_lock(&hif_deinit_lock);
+   mutex_lock(&wilc->hif_deinit_lock);
 
id = buffer[length - 4];
id |= (buffer[length - 3] << 8);
@@ -3604,26 +3605,26 @@ void wilc_gnrl_async_info_received(struct wilc *wilc, 
u8 *buffer, u32 length)
id |= (buffer[length - 1] << 24);
vif = wilc_get_vif_from_idx(wilc, id);
if (!vif) {
-   mutex_unlock(&hif_deinit_lock);
+   mutex_unlock(&wilc->hif_deinit_lock);
return;
}
 
hif_drv = vif->hif_drv;
 
if (!hif_drv || hif_drv == terminated_handle) {
-   mutex_unlock(&hif_d

[PATCH 17/24] staging: wilc1000: use lowercase for get_BSSID() and HIL variable

2018-08-13 Thread Ajay Singh
Cleanup patch to use lowercase name for get_BSSID() and HIL variable.

Signed-off-by: Ajay Singh 
---
 drivers/staging/wilc1000/coreconfigurator.c | 4 ++--
 drivers/staging/wilc1000/wilc_wlan.c| 8 
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/wilc1000/coreconfigurator.c 
b/drivers/staging/wilc1000/coreconfigurator.c
index e542067..d6d3a97 100644
--- a/drivers/staging/wilc1000/coreconfigurator.c
+++ b/drivers/staging/wilc1000/coreconfigurator.c
@@ -116,7 +116,7 @@ static inline void get_address3(u8 *msa, u8 *addr)
memcpy(addr, msa + 16, 6);
 }
 
-static inline void get_BSSID(u8 *data, u8 *bssid)
+static inline void get_bssid(u8 *data, u8 *bssid)
 {
if (get_from_ds(data) == 1)
get_address2(data, bssid);
@@ -233,7 +233,7 @@ s32 wilc_parse_network_info(u8 *msg_buffer,
network_info->tsf_hi = tsf_lo | ((u64)tsf_hi << 32);
 
get_ssid(msa, network_info->ssid, &network_info->ssid_len);
-   get_BSSID(msa, network_info->bssid);
+   get_bssid(msa, network_info->bssid);
 
network_info->ch = get_current_channel_802_11n(msa, rx_len
   + FCS_LEN);
diff --git a/drivers/staging/wilc1000/wilc_wlan.c 
b/drivers/staging/wilc1000/wilc_wlan.c
index bf45b4c..d397c27 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -164,7 +164,7 @@ static inline void tcp_process(struct net_device *dev, 
struct txq_entry_t *tqe)
struct wilc *wilc = vif->wilc;
const struct iphdr *ip_hdr_ptr;
const struct tcphdr *tcp_hdr_ptr;
-   u32 IHL, total_length, data_offset;
+   u32 ihl, total_length, data_offset;
 
spin_lock_irqsave(&wilc->txq_spinlock, flags);
 
@@ -176,12 +176,12 @@ static inline void tcp_process(struct net_device *dev, 
struct txq_entry_t *tqe)
if (ip_hdr_ptr->protocol != IPPROTO_TCP)
goto out;
 
-   IHL = ip_hdr_ptr->ihl << 2;
-   tcp_hdr_ptr = buffer + ETH_HLEN + IHL;
+   ihl = ip_hdr_ptr->ihl << 2;
+   tcp_hdr_ptr = buffer + ETH_HLEN + ihl;
total_length = ntohs(ip_hdr_ptr->tot_len);
 
data_offset = tcp_hdr_ptr->doff << 2;
-   if (total_length == (IHL + data_offset)) {
+   if (total_length == (ihl + data_offset)) {
u32 seq_no, ack_no;
 
seq_no = ntohl(tcp_hdr_ptr->seq);
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 22/24] staging: wilc1000: move 'chip_ps_state' static variable as part of 'wilc' struct

2018-08-13 Thread Ajay Singh
Move the static variable as part of 'wilc' priv struct.

Signed-off-by: Ajay Singh 
---
 drivers/staging/wilc1000/linux_wlan.c |  1 +
 drivers/staging/wilc1000/wilc_wfi_netdevice.h |  2 ++
 drivers/staging/wilc1000/wilc_wlan.c  | 10 --
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/wilc1000/linux_wlan.c 
b/drivers/staging/wilc1000/linux_wlan.c
index 74f8166..4b99de2 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -1062,6 +1062,7 @@ int wilc_netdev_init(struct wilc **wilc, struct device 
*dev, int io_type,
wl->io_type = io_type;
wl->hif_func = ops;
wl->enable_ps = true;
+   wl->chip_ps_state = CHIP_WAKEDUP;
INIT_LIST_HEAD(&wl->txq_head.list);
INIT_LIST_HEAD(&wl->rxq_head.list);
 
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h 
b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index 54ce1ff..9d57adb 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -206,6 +206,8 @@ struct wilc {
/* deinitialization lock */
struct mutex hif_deinit_lock;
struct completion hif_driver_comp;
+
+   enum chip_ps_states chip_ps_state;
 };
 
 struct wilc_wfi_mon_priv {
diff --git a/drivers/staging/wilc1000/wilc_wlan.c 
b/drivers/staging/wilc1000/wilc_wlan.c
index f0743d9..fb8a1e4 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -9,8 +9,6 @@
 #include "wilc_wfi_netdevice.h"
 #include "wilc_wlan_cfg.h"
 
-static enum chip_ps_states chip_ps_state = CHIP_WAKEDUP;
-
 static inline bool is_wilc1000(u32 id)
 {
return ((id & 0xf000) == 0x10 ? true : false);
@@ -449,7 +447,7 @@ void chip_wakeup(struct wilc *wilc)
} while ((clk_status_reg & 0x1) == 0);
}
 
-   if (chip_ps_state == CHIP_SLEEPING_MANUAL) {
+   if (wilc->chip_ps_state == CHIP_SLEEPING_MANUAL) {
if (wilc_get_chipid(wilc, false) < 0x1002b0) {
u32 val32;
 
@@ -462,19 +460,19 @@ void chip_wakeup(struct wilc *wilc)
wilc->hif_func->hif_write_reg(wilc, 0x1e9c, val32);
}
}
-   chip_ps_state = CHIP_WAKEDUP;
+   wilc->chip_ps_state = CHIP_WAKEDUP;
 }
 
 void wilc_chip_sleep_manually(struct wilc *wilc)
 {
-   if (chip_ps_state != CHIP_WAKEDUP)
+   if (wilc->chip_ps_state != CHIP_WAKEDUP)
return;
acquire_bus(wilc, ACQUIRE_ONLY);
 
chip_allow_sleep(wilc);
wilc->hif_func->hif_write_reg(wilc, 0x10a8, 1);
 
-   chip_ps_state = CHIP_SLEEPING_MANUAL;
+   wilc->chip_ps_state = CHIP_SLEEPING_MANUAL;
release_bus(wilc, RELEASE_ONLY);
 }
 
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 24/24] staging: wilc1000: remove unnecessary static variable 'p2p_listen_state'

2018-08-13 Thread Ajay Singh
Remove the use of unnecessary static variable 'p2p_listen_state'.
Already 'p2p_listen_state' is present in 'wilc_priv' struct. So making
use of that variable as its getting set in channel ready and
remain on channel expired callback.

Signed-off-by: Ajay Singh 
---
 drivers/staging/wilc1000/host_interface.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index d8cc08b..cf7ead5 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -185,8 +185,6 @@ struct join_bss_param {
u8 start_time[4];
 };
 
-static u8 p2p_listen_state;
-
 /* 'msg' should be free by the caller for syc */
 static struct host_if_msg*
 wilc_alloc_work(struct wilc_vif *vif, void (*work_fun)(struct work_struct *),
@@ -2351,7 +2349,6 @@ static int handle_remain_on_chan(struct wilc_vif *vif,
netdev_err(vif->ndev, "Failed to set remain on channel\n");
 
 error:
-   p2p_listen_state = 1;
hif_drv->remain_on_ch_timer_vif = vif;
mod_timer(&hif_drv->remain_on_ch_timer,
  jiffies + msecs_to_jiffies(hif_remain_ch->duration));
@@ -2407,8 +2404,9 @@ static void handle_listen_state_expired(struct 
work_struct *work)
struct wid wid;
int result;
struct host_if_drv *hif_drv = vif->hif_drv;
+   struct wilc_priv *priv = wdev_priv(vif->ndev->ieee80211_ptr);
 
-   if (p2p_listen_state) {
+   if (priv->p2p_listen_state) {
remain_on_chan_flag = false;
wid.id = WID_REMAIN_ON_CHAN;
wid.type = WID_STR;
@@ -2433,7 +2431,6 @@ static void handle_listen_state_expired(struct 
work_struct *work)
hif_drv->remain_on_ch.expired(hif_drv->remain_on_ch.arg,
  hif_remain_ch->id);
}
-   p2p_listen_state = 0;
} else {
netdev_dbg(vif->ndev, "Not in listen state\n");
}
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 21/24] staging: wilc1000: remove unused code to set and get IP address

2018-08-13 Thread Ajay Singh
Cleanup code to remove the variables related to setting and getting IP
address as this case was not handled from firmware side.

Signed-off-by: Ajay Singh 
---
 drivers/staging/wilc1000/host_interface.c | 105 --
 drivers/staging/wilc1000/host_interface.h |   3 -
 drivers/staging/wilc1000/linux_wlan.c |   3 -
 3 files changed, 111 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index bffe0c8..f37ba64 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -187,11 +187,6 @@ struct join_bss_param {
 
 static u8 p2p_listen_state;
 
-static u8 set_ip[2][4];
-static u8 get_ip[2][4];
-
-static int host_int_get_ipaddress(struct wilc_vif *vif, u8 *ip_addr, u8 idx);
-
 /* 'msg' should be free by the caller for syc */
 static struct host_if_msg*
 wilc_alloc_work(struct wilc_vif *vif, void (*work_fun)(struct work_struct *),
@@ -344,64 +339,6 @@ static void handle_set_operation_mode(struct work_struct 
*work)
kfree(msg);
 }
 
-static void handle_set_ip_address(struct work_struct *work)
-{
-   struct host_if_msg *msg = container_of(work, struct host_if_msg, work);
-   struct wilc_vif *vif = msg->vif;
-   u8 *ip_addr = msg->body.ip_info.ip_addr;
-   u8 idx = msg->body.ip_info.idx;
-   int ret;
-   struct wid wid;
-   char firmware_ip_addr[4] = {0};
-
-   if (ip_addr[0] < 192)
-   ip_addr[0] = 0;
-
-   memcpy(set_ip[idx], ip_addr, IP_ALEN);
-
-   wid.id = WID_IP_ADDRESS;
-   wid.type = WID_STR;
-   wid.val = ip_addr;
-   wid.size = IP_ALEN;
-
-   ret = wilc_send_config_pkt(vif, SET_CFG, &wid, 1,
-  wilc_get_vif_idx(vif));
-
-   host_int_get_ipaddress(vif, firmware_ip_addr, idx);
-
-   if (ret)
-   netdev_err(vif->ndev, "Failed to set IP address\n");
-   kfree(msg);
-}
-
-static void handle_get_ip_address(struct work_struct *work)
-{
-   struct host_if_msg *msg = container_of(work, struct host_if_msg, work);
-   struct wilc_vif *vif = msg->vif;
-   u8 idx = msg->body.ip_info.idx;
-   int ret;
-   struct wid wid;
-
-   wid.id = WID_IP_ADDRESS;
-   wid.type = WID_STR;
-   wid.val = kmalloc(IP_ALEN, GFP_KERNEL);
-   wid.size = IP_ALEN;
-
-   ret = wilc_send_config_pkt(vif, GET_CFG, &wid, 1,
-  wilc_get_vif_idx(vif));
-
-   memcpy(get_ip[idx], wid.val, IP_ALEN);
-
-   kfree(wid.val);
-
-   if (memcmp(get_ip[idx], set_ip[idx], IP_ALEN) != 0)
-   wilc_setup_ipaddress(vif, set_ip[idx], idx);
-
-   if (ret)
-   netdev_err(vif->ndev, "Failed to get IP address\n");
-   kfree(msg);
-}
-
 static void handle_get_mac_address(struct work_struct *work)
 {
struct host_if_msg *msg = container_of(work, struct host_if_msg, work);
@@ -4002,48 +3939,6 @@ int wilc_setup_multicast_filter(struct wilc_vif *vif, 
bool enabled,
return result;
 }
 
-int wilc_setup_ipaddress(struct wilc_vif *vif, u8 *ip_addr, u8 idx)
-{
-   int result;
-   struct host_if_msg *msg;
-
-   msg = wilc_alloc_work(vif, handle_set_ip_address, false);
-   if (IS_ERR(msg))
-   return PTR_ERR(msg);
-
-   msg->body.ip_info.ip_addr = ip_addr;
-   msg->body.ip_info.idx = idx;
-
-   result = wilc_enqueue_work(msg);
-   if (result) {
-   netdev_err(vif->ndev, "%s: enqueue work failed\n", __func__);
-   kfree(msg);
-   }
-
-   return result;
-}
-
-static int host_int_get_ipaddress(struct wilc_vif *vif, u8 *ip_addr, u8 idx)
-{
-   int result;
-   struct host_if_msg *msg;
-
-   msg = wilc_alloc_work(vif, handle_get_ip_address, false);
-   if (IS_ERR(msg))
-   return PTR_ERR(msg);
-
-   msg->body.ip_info.ip_addr = ip_addr;
-   msg->body.ip_info.idx = idx;
-
-   result = wilc_enqueue_work(msg);
-   if (result) {
-   netdev_err(vif->ndev, "%s: enqueue work failed\n", __func__);
-   kfree(msg);
-   }
-
-   return result;
-}
-
 int wilc_set_tx_power(struct wilc_vif *vif, u8 tx_power)
 {
int ret;
diff --git a/drivers/staging/wilc1000/host_interface.h 
b/drivers/staging/wilc1000/host_interface.h
index 80cb298..0f0d509 100644
--- a/drivers/staging/wilc1000/host_interface.h
+++ b/drivers/staging/wilc1000/host_interface.h
@@ -9,8 +9,6 @@
 #include 
 #include "coreconfigurator.h"
 
-#define IP_ALEN  4
-
 #define IDLE_MODE  0x00
 #define AP_MODE0x01
 #define STATION_MODE   0x02
@@ -344,7 +342,6 @@ int wilc_edit_station(struct wilc_vif *vif,
 int wilc_set_power_mgmt(struct wilc_vif *vif, bool enabled, u32 timeout);
 int wilc_setup_multicast_filter(struct wilc_vif *vif, bool enabled,
u32 count);
-int wilc_setup_ipaddress(struct wilc_vif *vif, u8 *ip_addr, u8 idx);
 int wilc_remain_on_chan

[PATCH 09/24] staging: wilc1000: move static variable clients_count to 'wilc' structure

2018-08-13 Thread Ajay Singh
Avoid use of static variable 'clients_count' and move it part of 'wilc'
structure.

Signed-off-by: Ajay Singh 
---
 drivers/staging/wilc1000/host_interface.c | 9 -
 drivers/staging/wilc1000/wilc_wfi_netdevice.h | 1 +
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index 6225e67..d930f06 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -199,7 +199,6 @@ static u8 rcv_assoc_resp[MAX_ASSOC_RESP_FRAME_SIZE];
 
 static u8 set_ip[2][4];
 static u8 get_ip[2][4];
-static u32 clients_count;
 
 static int host_int_get_ipaddress(struct wilc_vif *vif, u8 *ip_addr, u8 idx);
 
@@ -3456,7 +3455,7 @@ int wilc_init(struct net_device *dev, struct host_if_drv 
**hif_drv_handler)
 
vif->obtaining_ip = false;
 
-   if (clients_count == 0) {
+   if (wilc->clients_count == 0) {
init_completion(&hif_driver_comp);
mutex_init(&hif_deinit_lock);
 
@@ -3490,7 +3489,7 @@ int wilc_init(struct net_device *dev, struct host_if_drv 
**hif_drv_handler)
 
mutex_unlock(&hif_drv->cfg_values_lock);
 
-   clients_count++;
+   wilc->clients_count++;
 
return 0;
 }
@@ -3526,7 +3525,7 @@ int wilc_deinit(struct wilc_vif *vif)
 
hif_drv->hif_state = HOST_IF_IDLE;
 
-   if (clients_count == 1) {
+   if (vif->wilc->clients_count == 1) {
struct host_if_msg *msg;
 
msg = wilc_alloc_work(vif, handle_hif_exit_work, true);
@@ -3544,7 +3543,7 @@ int wilc_deinit(struct wilc_vif *vif)
 
kfree(hif_drv);
 
-   clients_count--;
+   vif->wilc->clients_count--;
terminated_handle = NULL;
mutex_unlock(&hif_deinit_lock);
return result;
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h 
b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index 8e56a28..8cccbbc 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -171,6 +171,7 @@ struct wilc {
 
struct rf_info dummy_statistics;
bool enable_ps;
+   int clients_count;
 };
 
 struct wilc_wfi_mon_priv {
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 19/24] staging: wilc1000: avoid line over 80 chars in wilc_wlan_txq_filter_dup_tcp_ack()

2018-08-13 Thread Ajay Singh
Cleanup patch to avoid line over 80 chars checkpatch issue introduced in
previous code refactor commit.

Signed-off-by: Ajay Singh 
---
 drivers/staging/wilc1000/wilc_wlan.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_wlan.c 
b/drivers/staging/wilc1000/wilc_wlan.c
index 52402c3..041c9dd 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -199,19 +199,20 @@ static int wilc_wlan_txq_filter_dup_tcp_ack(struct 
net_device *dev)
unsigned long flags;
 
spin_lock_irqsave(&wilc->txq_spinlock, flags);
-   for (i = f->pending_base; i < (f->pending_base + f->pending_acks); i++) 
{
-   u32 session_index;
+   i = f->pending_base;
+   for (; i < (f->pending_base + f->pending_acks); i++) {
+   u32 index;
u32 bigger_ack_num;
 
if (i >= MAX_PENDING_ACKS)
break;
 
-   session_index = f->pending_acks_info[i].session_index;
+   index = f->pending_acks_info[i].session_index;
 
-   if (session_index >= 2 * MAX_TCP_SESSION)
+   if (index >= 2 * MAX_TCP_SESSION)
break;
 
-   bigger_ack_num = 
f->ack_session_info[session_index].bigger_ack_num;
+   bigger_ack_num = f->ack_session_info[index].bigger_ack_num;
 
if (f->pending_acks_info[i].ack_num < bigger_ack_num) {
struct txq_entry_t *tqe;
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 02/24] staging: wilc1000: move 'aging_timer' static variable to wilc_priv struct

2018-08-13 Thread Ajay Singh
Moved 'aging_timer' to wilc_priv struct instead of having it as static
variable.
As 'aging_timer' is maintained for each interfaces so 'op_ifcs' check is
not required before the timer_setup() and del_timer_sync() call.

Signed-off-by: Ajay Singh 
---
 drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 25 ---
 drivers/staging/wilc1000/wilc_wfi_netdevice.h |  2 +-
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c 
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index 4727a8a..d853508 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -85,7 +85,6 @@ static const struct wiphy_wowlan_support wowlan_support = {
 static struct network_info 
last_scanned_shadow[MAX_NUM_SCANNED_NETWORKS_SHADOW];
 static u32 last_scanned_cnt;
 struct timer_list wilc_during_ip_timer;
-static struct timer_list aging_timer;
 static u8 op_ifcs;
 
 #define CHAN2G(_channel, _freq, _flags) {   \
@@ -165,8 +164,6 @@ static void clear_shadow_scan(void)
if (op_ifcs != 0)
return;
 
-   del_timer_sync(&aging_timer);
-
for (i = 0; i < last_scanned_cnt; i++) {
if (last_scanned_shadow[last_scanned_cnt].ies) {
kfree(last_scanned_shadow[i].ies);
@@ -245,8 +242,9 @@ static void update_scan_time(void)
last_scanned_shadow[i].time_scan = jiffies;
 }
 
-static void remove_network_from_shadow(struct timer_list *unused)
+static void remove_network_from_shadow(struct timer_list *t)
 {
+   struct wilc_priv *priv = from_timer(priv, t, aging_timer);
unsigned long now = jiffies;
int i, j;
 
@@ -266,7 +264,8 @@ static void remove_network_from_shadow(struct timer_list 
*unused)
}
 
if (last_scanned_cnt != 0)
-   mod_timer(&aging_timer, jiffies + msecs_to_jiffies(AGING_TIME));
+   mod_timer(&priv->aging_timer,
+ jiffies + msecs_to_jiffies(AGING_TIME));
 }
 
 static void clear_during_ip(struct timer_list *unused)
@@ -274,13 +273,15 @@ static void clear_during_ip(struct timer_list *unused)
wilc_optaining_ip = false;
 }
 
-static int is_network_in_shadow(struct network_info *nw_info, void *user_void)
+static int is_network_in_shadow(struct network_info *nw_info,
+   struct wilc_priv *priv)
 {
int state = -1;
int i;
 
if (last_scanned_cnt == 0) {
-   mod_timer(&aging_timer, jiffies + msecs_to_jiffies(AGING_TIME));
+   mod_timer(&priv->aging_timer,
+ jiffies + msecs_to_jiffies(AGING_TIME));
state = -1;
} else {
for (i = 0; i < last_scanned_cnt; i++) {
@@ -295,9 +296,9 @@ static int is_network_in_shadow(struct network_info 
*nw_info, void *user_void)
 }
 
 static void add_network_to_shadow(struct network_info *nw_info,
- void *user_void, void *join_params)
+ struct wilc_priv *priv, void *join_params)
 {
-   int ap_found = is_network_in_shadow(nw_info, user_void);
+   int ap_found = is_network_in_shadow(nw_info, priv);
u32 ap_index = 0;
u8 rssi_index = 0;
struct network_info *shadow_nw_info;
@@ -2166,10 +2167,9 @@ int wilc_init_host_int(struct net_device *net)
int ret;
struct wilc_priv *priv = wdev_priv(net->ieee80211_ptr);
 
-   if (op_ifcs == 0) {
-   timer_setup(&aging_timer, remove_network_from_shadow, 0);
+   timer_setup(&priv->aging_timer, remove_network_from_shadow, 0);
+   if (op_ifcs == 0)
timer_setup(&wilc_during_ip_timer, clear_during_ip, 0);
-   }
op_ifcs++;
 
priv->p2p_listen_state = false;
@@ -2195,6 +2195,7 @@ int wilc_deinit_host_int(struct net_device *net)
mutex_destroy(&priv->scan_req_lock);
ret = wilc_deinit(vif);
 
+   del_timer_sync(&priv->aging_timer);
clear_shadow_scan();
if (op_ifcs == 0)
del_timer_sync(&wilc_during_ip_timer);
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h 
b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index 8b74d61..a76b68c 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -94,7 +94,7 @@ struct wilc_priv {
/* mutexes */
struct mutex scan_req_lock;
bool p2p_listen_state;
-
+   struct timer_list aging_timer;
 };
 
 struct frame_reg {
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 00/24] staging: wilc1000: avoid use of static and global variable

2018-08-13 Thread Ajay Singh
This patch set mainly contains changes to avoid the use of static
and global variables. Also contains few patch to avoid the checkpatch
warning arise due to code refactor.

Ajay Singh (24):
  staging: wilc1000: move 'wilc_enable_ps' global variable into 'wilc'
struct
  staging: wilc1000: move 'aging_timer' static variable to wilc_priv
struct
  staging: wilc1000: fix to use correct index to free scanned info in
clear_shadow_scan()
  staging: wilc1000: remove unnecessary NULL check in
clear_shadow_scan()
  staging: wilc1000: moved last_scanned_shadow & last_scanned_cnt to
wilc_priv struct
  staging: wilc1000: move during_ip_timer & wilc_optaining_ip to
'wilc_vif' struct
  staging: wilc1000: remove unused variable 'op_ifcs'
  staging: wilc1000: avoid use of extra 'if' condition in wilc_init()
  staging: wilc1000: move static variable clients_count to 'wilc'
structure
  staging: wilc1000: move wilc_multicast_mac_addr_list to 'wilc_vif'
struct
  staging: wilc1000: move hif specific static variables to 'wilc'
structure
  staging: wilc1000: move static variable 'terminated_handle' to
wilc_vif struct
  staging: wilc1000: move 'periodic_rssi' as part of 'wilc_vif' struct
  staging: wilc1000: rename 'dummy_statistics' variable to
'periodic_stat'
  staging: wilc1000: move 'rcv_assoc_resp' as part of hif_drv
  staging: wilc1000: refactor tcp_process() to avoid extra leading tabs
  staging: wilc1000: use lowercase for get_BSSID() and HIL variable
  staging: wilc1000: move tcp_ack_filter algo related variables to
'wilc_vif' struct
  staging: wilc1000: avoid line over 80 chars in
wilc_wlan_txq_filter_dup_tcp_ack()
  staging: wilc1000: avoid line over 80 chars in tcp_process()
  staging: wilc1000: remove unused code to set and get IP address
  staging: wilc1000: move 'chip_ps_state' static variable as part of
'wilc' struct
  staging: wilc1000: move 'wilc_connecting' static variable to
'wilc_vif' struct
  staging: wilc1000: remove unnecessary static variable
'p2p_listen_state'

 drivers/staging/wilc1000/coreconfigurator.c   |   4 +-
 drivers/staging/wilc1000/host_interface.c | 227 +-
 drivers/staging/wilc1000/host_interface.h |   9 +-
 drivers/staging/wilc1000/linux_wlan.c |  29 ++-
 drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 191 +-
 drivers/staging/wilc1000/wilc_wfi_netdevice.h |  47 -
 drivers/staging/wilc1000/wilc_wlan.c  | 167 
 drivers/staging/wilc1000/wilc_wlan.h  |   3 +-
 8 files changed, 284 insertions(+), 393 deletions(-)

-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 08/24] staging: wilc1000: avoid use of extra 'if' condition in wilc_init()

2018-08-13 Thread Ajay Singh
Cleanup patch to avoid the avoid extra 'if' condition and clubbed the
same condition in single 'if' block.

Signed-off-by: Ajay Singh 
---
 drivers/staging/wilc1000/host_interface.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index 11eb632..6225e67 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -3459,9 +3459,7 @@ int wilc_init(struct net_device *dev, struct host_if_drv 
**hif_drv_handler)
if (clients_count == 0) {
init_completion(&hif_driver_comp);
mutex_init(&hif_deinit_lock);
-   }
 
-   if (clients_count == 0) {
hif_workqueue = create_singlethread_workqueue("WILC_wq");
if (!hif_workqueue) {
netdev_err(vif->ndev, "Failed to create workqueue\n");
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 07/24] staging: wilc1000: remove unused variable 'op_ifcs'

2018-08-13 Thread Ajay Singh
After code refactor in previous commit now 'op_ifcs' is not require any
more, so remove it.

Signed-off-by: Ajay Singh 
---
 drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c 
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index 1690890..3418d2d 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -82,8 +82,6 @@ static const struct wiphy_wowlan_support wowlan_support = {
.flags = WIPHY_WOWLAN_ANY
 };
 
-static u8 op_ifcs;
-
 #define CHAN2G(_channel, _freq, _flags) {   \
.band = NL80211_BAND_2GHZ, \
.center_freq  = (_freq), \
@@ -2164,7 +2162,6 @@ int wilc_init_host_int(struct net_device *net)
 
timer_setup(&priv->aging_timer, remove_network_from_shadow, 0);
timer_setup(&vif->during_ip_timer, clear_during_ip, 0);
-   op_ifcs++;
 
priv->p2p_listen_state = false;
 
@@ -2184,8 +2181,6 @@ int wilc_deinit_host_int(struct net_device *net)
 
priv->p2p_listen_state = false;
 
-   op_ifcs--;
-
mutex_destroy(&priv->scan_req_lock);
ret = wilc_deinit(vif);
 
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 06/24] staging: wilc1000: move during_ip_timer & wilc_optaining_ip to 'wilc_vif' struct

2018-08-13 Thread Ajay Singh
Move global variable 'wilc_during_ip_timer' and 'wilc_optaining_ip' to
'wilc_vif' structure.

Rename these variables like below

wilc_during_ip_timer -> during_ip_timer
wilc_optaining_ip -> obtaining_ip.

Signed-off-by: Ajay Singh 
---
 drivers/staging/wilc1000/host_interface.c | 15 +++---
 drivers/staging/wilc1000/host_interface.h |  2 --
 drivers/staging/wilc1000/linux_wlan.c |  6 +++---
 drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 24 +++
 drivers/staging/wilc1000/wilc_wfi_netdevice.h |  2 ++
 5 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index 42d8acc..11eb632 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -186,7 +186,6 @@ struct join_bss_param {
 };
 
 static struct host_if_drv *terminated_handle;
-bool wilc_optaining_ip;
 static u8 p2p_listen_state;
 static struct workqueue_struct *hif_workqueue;
 static struct completion hif_driver_comp;
@@ -791,7 +790,7 @@ static void handle_scan(struct work_struct *work)
goto error;
}
 
-   if (wilc_optaining_ip || wilc_connecting) {
+   if (vif->obtaining_ip || wilc_connecting) {
netdev_err(vif->ndev, "Don't do obss scan\n");
result = -EBUSY;
goto error;
@@ -1562,8 +1561,8 @@ static inline void host_int_parse_assoc_resp_info(struct 
wilc_vif *vif,
 
hif_drv->hif_state = HOST_IF_CONNECTED;
 
-   wilc_optaining_ip = true;
-   mod_timer(&wilc_during_ip_timer,
+   vif->obtaining_ip = true;
+   mod_timer(&vif->during_ip_timer,
  jiffies + msecs_to_jiffies(1));
} else {
hif_drv->hif_state = HOST_IF_IDLE;
@@ -1595,7 +1594,7 @@ static inline void host_int_handle_disconnect(struct 
wilc_vif *vif)
disconn_info.ie_len = 0;
 
if (conn_result) {
-   wilc_optaining_ip = false;
+   vif->obtaining_ip = false;
wilc_set_power_mgmt(vif, 0, 0);
 
conn_result(CONN_DISCONN_EVENT_DISCONN_NOTIF, NULL, 0,
@@ -1942,7 +1941,7 @@ static void handle_disconnect(struct work_struct *work)
wid.val = (s8 *)&dummy_reason_code;
wid.size = sizeof(char);
 
-   wilc_optaining_ip = false;
+   vif->obtaining_ip = false;
wilc_set_power_mgmt(vif, 0, 0);
 
eth_zero_addr(wilc_connected_ssid);
@@ -2397,7 +2396,7 @@ static int handle_remain_on_chan(struct wilc_vif *vif,
goto error;
}
 
-   if (wilc_optaining_ip || wilc_connecting) {
+   if (vif->obtaining_ip || wilc_connecting) {
result = -EBUSY;
goto error;
}
@@ -3455,7 +3454,7 @@ int wilc_init(struct net_device *dev, struct host_if_drv 
**hif_drv_handler)
break;
}
 
-   wilc_optaining_ip = false;
+   vif->obtaining_ip = false;
 
if (clients_count == 0) {
init_completion(&hif_driver_comp);
diff --git a/drivers/staging/wilc1000/host_interface.h 
b/drivers/staging/wilc1000/host_interface.h
index 84866a6..d026f44 100644
--- a/drivers/staging/wilc1000/host_interface.h
+++ b/drivers/staging/wilc1000/host_interface.h
@@ -361,11 +361,9 @@ int wilc_get_vif_idx(struct wilc_vif *vif);
 int wilc_set_tx_power(struct wilc_vif *vif, u8 tx_power);
 int wilc_get_tx_power(struct wilc_vif *vif, u8 *tx_power);
 
-extern bool wilc_optaining_ip;
 extern u8 wilc_connected_ssid[6];
 extern u8 wilc_multicast_mac_addr_list[WILC_MULTICAST_TABLE_SIZE][ETH_ALEN];
 
 extern int wilc_connecting;
-extern struct timer_list wilc_during_ip_timer;
 
 #endif
diff --git a/drivers/staging/wilc1000/linux_wlan.c 
b/drivers/staging/wilc1000/linux_wlan.c
index 57e3176..283bb74 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -48,8 +48,8 @@ static int dev_state_ev_handler(struct notifier_block *this,
case NETDEV_UP:
if (vif->iftype == STATION_MODE || vif->iftype == CLIENT_MODE) {
hif_drv->ifc_up = 1;
-   wilc_optaining_ip = false;
-   del_timer(&wilc_during_ip_timer);
+   vif->obtaining_ip = false;
+   del_timer(&vif->during_ip_timer);
}
 
if (vif->wilc->enable_ps)
@@ -68,7 +68,7 @@ static int dev_state_ev_handler(struct notifier_block *this,
case NETDEV_DOWN:
if (vif->iftype == STATION_MODE || vif->iftype == CLIENT_MODE) {
hif_drv->ifc_up = 0;
-   wilc_optaining_ip = false;
+   vif->obtaining_ip = false;
}
 
if (memcmp(dev_iface->ifa_label, wlan_dev_name, 5) == 0)
diff --git a/drivers/staging/wilc1000/wilc

[PATCH 10/24] staging: wilc1000: move wilc_multicast_mac_addr_list to 'wilc_vif' struct

2018-08-13 Thread Ajay Singh
Instead of using 'wilc_multicast_mac_addr_list' as global variable move
it part of wilc_vif struct. Rename 'wilc_multicast_mac_addr_list'
variable to 'mc_mac_addr_list' as its now part of 'wilc_vif' struct.

Signed-off-by: Ajay Singh 
---
 drivers/staging/wilc1000/host_interface.c |  4 +---
 drivers/staging/wilc1000/host_interface.h |  1 -
 drivers/staging/wilc1000/linux_wlan.c | 14 +++---
 drivers/staging/wilc1000/wilc_wfi_netdevice.h |  1 +
 4 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index d930f06..642c314 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -193,8 +193,6 @@ static struct mutex hif_deinit_lock;
 static struct timer_list periodic_rssi;
 static struct wilc_vif *periodic_rssi_vif;
 
-u8 wilc_multicast_mac_addr_list[WILC_MULTICAST_TABLE_SIZE][ETH_ALEN];
-
 static u8 rcv_assoc_resp[MAX_ASSOC_RESP_FRAME_SIZE];
 
 static u8 set_ip[2][4];
@@ -2588,7 +2586,7 @@ static void handle_set_mcast_filter(struct work_struct 
*work)
*cur_byte++ = ((hif_set_mc->cnt >> 24) & 0xFF);
 
if (hif_set_mc->cnt > 0)
-   memcpy(cur_byte, wilc_multicast_mac_addr_list,
+   memcpy(cur_byte, vif->mc_mac_addr_list,
   ((hif_set_mc->cnt) * ETH_ALEN));
 
result = wilc_send_config_pkt(vif, SET_CFG, &wid, 1,
diff --git a/drivers/staging/wilc1000/host_interface.h 
b/drivers/staging/wilc1000/host_interface.h
index d026f44..4a84dd2 100644
--- a/drivers/staging/wilc1000/host_interface.h
+++ b/drivers/staging/wilc1000/host_interface.h
@@ -362,7 +362,6 @@ int wilc_set_tx_power(struct wilc_vif *vif, u8 tx_power);
 int wilc_get_tx_power(struct wilc_vif *vif, u8 *tx_power);
 
 extern u8 wilc_connected_ssid[6];
-extern u8 wilc_multicast_mac_addr_list[WILC_MULTICAST_TABLE_SIZE][ETH_ALEN];
 
 extern int wilc_connecting;
 
diff --git a/drivers/staging/wilc1000/linux_wlan.c 
b/drivers/staging/wilc1000/linux_wlan.c
index 283bb74..bbaa653 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -822,14 +822,14 @@ static void wilc_set_multicast_list(struct net_device 
*dev)
}
 
netdev_for_each_mc_addr(ha, dev) {
-   memcpy(wilc_multicast_mac_addr_list[i], ha->addr, ETH_ALEN);
+   memcpy(vif->mc_mac_addr_list[i], ha->addr, ETH_ALEN);
netdev_dbg(dev, "Entry[%d]: %x:%x:%x:%x:%x:%x\n", i,
-  wilc_multicast_mac_addr_list[i][0],
-  wilc_multicast_mac_addr_list[i][1],
-  wilc_multicast_mac_addr_list[i][2],
-  wilc_multicast_mac_addr_list[i][3],
-  wilc_multicast_mac_addr_list[i][4],
-  wilc_multicast_mac_addr_list[i][5]);
+  vif->mc_mac_addr_list[i][0],
+  vif->mc_mac_addr_list[i][1],
+  vif->mc_mac_addr_list[i][2],
+  vif->mc_mac_addr_list[i][3],
+  vif->mc_mac_addr_list[i][4],
+  vif->mc_mac_addr_list[i][5]);
i++;
}
 
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h 
b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index 8cccbbc..ee8eda7 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -120,6 +120,7 @@ struct wilc_vif {
u8 ifc_id;
struct timer_list during_ip_timer;
bool obtaining_ip;
+   u8 mc_mac_addr_list[WILC_MULTICAST_TABLE_SIZE][ETH_ALEN];
 };
 
 struct wilc {
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 01/24] staging: wilc1000: move 'wilc_enable_ps' global variable into 'wilc' struct

2018-08-13 Thread Ajay Singh
Instead of having 'wilc_enable_ps' as global variable moved it to 'wilc'
structure. Rename 'wilc_enable_ps' to 'enable_ps' as its already part of
'wilc' structure

Signed-off-by: Ajay Singh 
---
 drivers/staging/wilc1000/linux_wlan.c |  5 ++---
 drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 10 +-
 drivers/staging/wilc1000/wilc_wfi_netdevice.h |  1 +
 drivers/staging/wilc1000/wilc_wlan.h  |  1 -
 4 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/wilc1000/linux_wlan.c 
b/drivers/staging/wilc1000/linux_wlan.c
index 01cf4bd..57e3176 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -12,8 +12,6 @@
 
 #include "wilc_wfi_cfgoperations.h"
 
-bool wilc_enable_ps = true;
-
 static int dev_state_ev_handler(struct notifier_block *this,
unsigned long event, void *ptr)
 {
@@ -54,7 +52,7 @@ static int dev_state_ev_handler(struct notifier_block *this,
del_timer(&wilc_during_ip_timer);
}
 
-   if (wilc_enable_ps)
+   if (vif->wilc->enable_ps)
wilc_set_power_mgmt(vif, 1, 0);
 
netdev_dbg(dev, "[%s] Up IP\n", dev_iface->ifa_label);
@@ -1066,6 +1064,7 @@ int wilc_netdev_init(struct wilc **wilc, struct device 
*dev, int io_type,
*wilc = wl;
wl->io_type = io_type;
wl->hif_func = ops;
+   wl->enable_ps = true;
INIT_LIST_HEAD(&wl->txq_head.list);
INIT_LIST_HEAD(&wl->rxq_head.list);
 
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c 
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index 7cd0330..4727a8a 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -1732,7 +1732,7 @@ static int set_power_mgmt(struct wiphy *wiphy, struct 
net_device *dev,
if (!priv->hif_drv)
return -EIO;
 
-   if (wilc_enable_ps)
+   if (vif->wilc->enable_ps)
wilc_set_power_mgmt(vif, enabled, timeout);
 
return 0;
@@ -1764,7 +1764,7 @@ static int change_virtual_intf(struct wiphy *wiphy, 
struct net_device *dev,
memset(priv->assoc_stainfo.sta_associated_bss, 0,
   MAX_NUM_STA * ETH_ALEN);
 
-   wilc_enable_ps = true;
+   wl->enable_ps = true;
wilc_set_power_mgmt(vif, 1, 0);
break;
 
@@ -1776,12 +1776,12 @@ static int change_virtual_intf(struct wiphy *wiphy, 
struct net_device *dev,
vif->iftype = CLIENT_MODE;
wilc_set_operation_mode(vif, STATION_MODE);
 
-   wilc_enable_ps = false;
+   wl->enable_ps = false;
wilc_set_power_mgmt(vif, 0, 0);
break;
 
case NL80211_IFTYPE_AP:
-   wilc_enable_ps = false;
+   wl->enable_ps = false;
dev->ieee80211_ptr->iftype = type;
priv->wdev->iftype = type;
vif->iftype = AP_MODE;
@@ -1803,7 +1803,7 @@ static int change_virtual_intf(struct wiphy *wiphy, 
struct net_device *dev,
priv->wdev->iftype = type;
vif->iftype = GO_MODE;
 
-   wilc_enable_ps = false;
+   wl->enable_ps = false;
wilc_set_power_mgmt(vif, 0, 0);
break;
 
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h 
b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index b7eee77..8b74d61 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -166,6 +166,7 @@ struct wilc {
bool suspend_event;
 
struct rf_info dummy_statistics;
+   bool enable_ps;
 };
 
 struct wilc_wfi_mon_priv {
diff --git a/drivers/staging/wilc1000/wilc_wlan.h 
b/drivers/staging/wilc1000/wilc_wlan.h
index 7467188..1f874d1 100644
--- a/drivers/staging/wilc1000/wilc_wlan.h
+++ b/drivers/staging/wilc1000/wilc_wlan.h
@@ -289,7 +289,6 @@ netdev_tx_t wilc_mac_xmit(struct sk_buff *skb, struct 
net_device *dev);
 void wilc_wfi_p2p_rx(struct net_device *dev, u8 *buff, u32 size);
 void host_wakeup_notify(struct wilc *wilc);
 void host_sleep_notify(struct wilc *wilc);
-extern bool wilc_enable_ps;
 void chip_allow_sleep(struct wilc *wilc);
 void chip_wakeup(struct wilc *wilc);
 int wilc_send_config_pkt(struct wilc_vif *vif, u8 mode, struct wid *wids,
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 04/24] staging: wilc1000: remove unnecessary NULL check in clear_shadow_scan()

2018-08-13 Thread Ajay Singh
Cleanup patch to remove the unnecessary NULL check before freeing up ies
information in clear_shadow_scan().

Signed-off-by: Ajay Singh 
---
 drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 6 ++
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c 
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index ede9134..00a167b 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -165,10 +165,8 @@ static void clear_shadow_scan(void)
return;
 
for (i = 0; i < last_scanned_cnt; i++) {
-   if (last_scanned_shadow[i].ies) {
-   kfree(last_scanned_shadow[i].ies);
-   last_scanned_shadow[i].ies = NULL;
-   }
+   kfree(last_scanned_shadow[i].ies);
+   last_scanned_shadow[i].ies = NULL;
 
kfree(last_scanned_shadow[i].join_params);
last_scanned_shadow[i].join_params = NULL;
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 03/24] staging: wilc1000: fix to use correct index to free scanned info in clear_shadow_scan()

2018-08-13 Thread Ajay Singh
Fixes to use correct index to free the allocated memory for ies
information. The check was done using 'last_scanned_cnt' index and its
not correct, so use the correct index ('i') to check for before freeing
the allocated memory.

Signed-off-by: Ajay Singh 
---
 drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c 
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index d853508..ede9134 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -165,9 +165,9 @@ static void clear_shadow_scan(void)
return;
 
for (i = 0; i < last_scanned_cnt; i++) {
-   if (last_scanned_shadow[last_scanned_cnt].ies) {
+   if (last_scanned_shadow[i].ies) {
kfree(last_scanned_shadow[i].ies);
-   last_scanned_shadow[last_scanned_cnt].ies = NULL;
+   last_scanned_shadow[i].ies = NULL;
}
 
kfree(last_scanned_shadow[i].join_params);
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 13/24] staging: wilc1000: move 'periodic_rssi' as part of 'wilc_vif' struct

2018-08-13 Thread Ajay Singh
Refactor code to move 'periodic_rssi' as part of wilc_vif struct.Move
'dummy_statistics' from 'wilc' struct to 'wilc_vif' to maintain for
each interface separatly.

Signed-off-by: Ajay Singh 
---
 drivers/staging/wilc1000/host_interface.c | 19 ---
 drivers/staging/wilc1000/wilc_wfi_netdevice.h |  3 ++-
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/wilc1000/host_interface.c 
b/drivers/staging/wilc1000/host_interface.c
index f71f451f..a440f84 100644
--- a/drivers/staging/wilc1000/host_interface.c
+++ b/drivers/staging/wilc1000/host_interface.c
@@ -186,8 +186,6 @@ struct join_bss_param {
 };
 
 static u8 p2p_listen_state;
-static struct timer_list periodic_rssi;
-static struct wilc_vif *periodic_rssi_vif;
 
 static u8 rcv_assoc_resp[MAX_ASSOC_RESP_FRAME_SIZE];
 
@@ -3417,9 +3415,9 @@ int wilc_hif_set_cfg(struct wilc_vif *vif,
return result;
 }
 
-static void get_periodic_rssi(struct timer_list *unused)
+static void get_periodic_rssi(struct timer_list *t)
 {
-   struct wilc_vif *vif = periodic_rssi_vif;
+   struct wilc_vif *vif = from_timer(vif, t, periodic_rssi);
 
if (!vif->hif_drv) {
netdev_err(vif->ndev, "%s: hif driver is NULL", __func__);
@@ -3427,9 +3425,9 @@ static void get_periodic_rssi(struct timer_list *unused)
}
 
if (vif->hif_drv->hif_state == HOST_IF_CONNECTED)
-   wilc_get_statistics(vif, &vif->wilc->dummy_statistics, false);
+   wilc_get_statistics(vif, &vif->dummy_statistics, false);
 
-   mod_timer(&periodic_rssi, jiffies + msecs_to_jiffies(5000));
+   mod_timer(&vif->periodic_rssi, jiffies + msecs_to_jiffies(5000));
 }
 
 int wilc_init(struct net_device *dev, struct host_if_drv **hif_drv_handler)
@@ -3463,12 +3461,11 @@ int wilc_init(struct net_device *dev, struct 
host_if_drv **hif_drv_handler)
kfree(hif_drv);
return -ENOMEM;
}
-
-   periodic_rssi_vif = vif;
-   timer_setup(&periodic_rssi, get_periodic_rssi, 0);
-   mod_timer(&periodic_rssi, jiffies + msecs_to_jiffies(5000));
}
 
+   timer_setup(&vif->periodic_rssi, get_periodic_rssi, 0);
+   mod_timer(&vif->periodic_rssi, jiffies + msecs_to_jiffies(5000));
+
timer_setup(&hif_drv->scan_timer, timer_scan_cb, 0);
timer_setup(&hif_drv->connect_timer, timer_connect_cb, 0);
timer_setup(&hif_drv->remain_on_ch_timer, listen_timer_cb, 0);
@@ -3508,7 +3505,7 @@ int wilc_deinit(struct wilc_vif *vif)
 
del_timer_sync(&hif_drv->scan_timer);
del_timer_sync(&hif_drv->connect_timer);
-   del_timer_sync(&periodic_rssi);
+   del_timer_sync(&vif->periodic_rssi);
del_timer_sync(&hif_drv->remain_on_ch_timer);
 
wilc_set_wfi_drv_handler(vif, 0, 0, 0);
diff --git a/drivers/staging/wilc1000/wilc_wfi_netdevice.h 
b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
index ba606d0..cdb90c3 100644
--- a/drivers/staging/wilc1000/wilc_wfi_netdevice.h
+++ b/drivers/staging/wilc1000/wilc_wfi_netdevice.h
@@ -122,6 +122,8 @@ struct wilc_vif {
bool obtaining_ip;
u8 mc_mac_addr_list[WILC_MULTICAST_TABLE_SIZE][ETH_ALEN];
bool is_termination_progress;
+   struct timer_list periodic_rssi;
+   struct rf_info dummy_statistics;
 };
 
 struct wilc {
@@ -171,7 +173,6 @@ struct wilc {
struct device *dev;
bool suspend_event;
 
-   struct rf_info dummy_statistics;
bool enable_ps;
int clients_count;
struct workqueue_struct *hif_workqueue;
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 05/24] staging: wilc1000: moved last_scanned_shadow & last_scanned_cnt to wilc_priv struct

2018-08-13 Thread Ajay Singh
Avoid use of static variables and moved the varibles as part of private
data. last_scanned_shadow & last_scanned_cnt variable is moved to
'wilc_priv' to maintain for each interface. After moving static
variable, clear_shadow_scan() doesn't require check 'op_ifcs'
count as now for each interface the againg timer is initiated.

Signed-off-by: Ajay Singh 
---
 drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 105 +++---
 drivers/staging/wilc1000/wilc_wfi_netdevice.h |   2 +
 2 files changed, 52 insertions(+), 55 deletions(-)

diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c 
b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index 00a167b..1eac244 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -82,8 +82,6 @@ static const struct wiphy_wowlan_support wowlan_support = {
.flags = WIPHY_WOWLAN_ANY
 };
 
-static struct network_info 
last_scanned_shadow[MAX_NUM_SCANNED_NETWORKS_SHADOW];
-static u32 last_scanned_cnt;
 struct timer_list wilc_during_ip_timer;
 static u8 op_ifcs;
 
@@ -157,21 +155,18 @@ static struct ieee80211_supported_band wilc_band_2ghz = {
 #define AGING_TIME (9 * 1000)
 #define DURING_IP_TIME_OUT 15000
 
-static void clear_shadow_scan(void)
+static void clear_shadow_scan(struct wilc_priv *priv)
 {
int i;
 
-   if (op_ifcs != 0)
-   return;
-
-   for (i = 0; i < last_scanned_cnt; i++) {
-   kfree(last_scanned_shadow[i].ies);
-   last_scanned_shadow[i].ies = NULL;
+   for (i = 0; i < priv->scanned_cnt; i++) {
+   kfree(priv->scanned_shadow[i].ies);
+   priv->scanned_shadow[i].ies = NULL;
 
-   kfree(last_scanned_shadow[i].join_params);
-   last_scanned_shadow[i].join_params = NULL;
+   kfree(priv->scanned_shadow[i].join_params);
+   priv->scanned_shadow[i].join_params = NULL;
}
-   last_scanned_cnt = 0;
+   priv->scanned_cnt = 0;
 }
 
 static u32 get_rssi_avg(struct network_info *network_info)
@@ -193,14 +188,14 @@ static void refresh_scan(struct wilc_priv *priv, bool 
direct_scan)
struct wiphy *wiphy = priv->dev->ieee80211_ptr->wiphy;
int i;
 
-   for (i = 0; i < last_scanned_cnt; i++) {
+   for (i = 0; i < priv->scanned_cnt; i++) {
struct network_info *network_info;
s32 freq;
struct ieee80211_channel *channel;
int rssi;
struct cfg80211_bss *bss;
 
-   network_info = &last_scanned_shadow[i];
+   network_info = &priv->scanned_shadow[i];
 
if (!memcmp("DIRECT-", network_info->ssid, 7) && !direct_scan)
continue;
@@ -224,20 +219,20 @@ static void refresh_scan(struct wilc_priv *priv, bool 
direct_scan)
}
 }
 
-static void reset_shadow_found(void)
+static void reset_shadow_found(struct wilc_priv *priv)
 {
int i;
 
-   for (i = 0; i < last_scanned_cnt; i++)
-   last_scanned_shadow[i].found = 0;
+   for (i = 0; i < priv->scanned_cnt; i++)
+   priv->scanned_shadow[i].found = 0;
 }
 
-static void update_scan_time(void)
+static void update_scan_time(struct wilc_priv *priv)
 {
int i;
 
-   for (i = 0; i < last_scanned_cnt; i++)
-   last_scanned_shadow[i].time_scan = jiffies;
+   for (i = 0; i < priv->scanned_cnt; i++)
+   priv->scanned_shadow[i].time_scan = jiffies;
 }
 
 static void remove_network_from_shadow(struct timer_list *t)
@@ -246,22 +241,22 @@ static void remove_network_from_shadow(struct timer_list 
*t)
unsigned long now = jiffies;
int i, j;
 
-   for (i = 0; i < last_scanned_cnt; i++) {
-   if (!time_after(now, last_scanned_shadow[i].time_scan +
+   for (i = 0; i < priv->scanned_cnt; i++) {
+   if (!time_after(now, priv->scanned_shadow[i].time_scan +
(unsigned long)(SCAN_RESULT_EXPIRE)))
continue;
-   kfree(last_scanned_shadow[i].ies);
-   last_scanned_shadow[i].ies = NULL;
+   kfree(priv->scanned_shadow[i].ies);
+   priv->scanned_shadow[i].ies = NULL;
 
-   kfree(last_scanned_shadow[i].join_params);
+   kfree(priv->scanned_shadow[i].join_params);
 
-   for (j = i; (j < last_scanned_cnt - 1); j++)
-   last_scanned_shadow[j] = last_scanned_shadow[j + 1];
+   for (j = i; (j < priv->scanned_cnt - 1); j++)
+   priv->scanned_shadow[j] = priv->scanned_shadow[j + 1];
 
-   last_scanned_cnt--;
+   priv->scanned_cnt--;
}
 
-   if (last_scanned_cnt != 0)
+   if (priv->scanned_cnt != 0)
mod_timer(&priv->aging_timer,
  jiffies + msecs_to_jiffies(AGING_TIME));
 }
@@ -277,13 +272,1

Re: [PATCH] staging: wilc1000: revert "fix TODO to compile spi and sdio components in single module"

2018-08-13 Thread Ajay Singh
Hi Arnd,

On Mon, 13 Aug 2018 23:20:33 +0200
Arnd Bergmann  wrote:

> The TODO item named "make spi and sdio components coexist in one
> build" was apparently addressed a long time ago, but never removed
> from the TODO file. However, the new patch that tries to address it
> actually makes it worse again by duplicating the common parts of the
> driver into two separate modules rather than sharing them. This also
> introduces a build regression when one of the two is built-in while
> the other is a loadable module:

Thanks for sharing your inputs and submitting patch.
I have also submitted a patch to address the compilation error[1].
We can ignore my patch and proceed with your changes.

[1].https://patchwork.kernel.org/patch/10563873/

In my understanding, even when the common part is compiled separately,
the wilc1000-sdio/wilc1000-spi module still expects separate instance of
common part for each of them because of use of static variables.

Shouldn't it be correct to compile the components required for SDIO and
SPI separately and then get rid of use of global variables to support
running of SDIO/SPI module at same time?

> 
> drivers/staging/wilc1000/wilc_debugfs.o:(.data+0x10): undefined
> reference to `__this_module'
> 
> Reverting the patch makes it build again. I'm leaving the TODO file
> modification though, as there is nothing left to do for this item.
> 
> A related problem however still seems to exist: one still cannot have
> multiple concurrent instances of wilc1000 devices present in the
> system, as there are lots of shared global variables such as
> 
> host_interface.c:static struct wilc_vif *periodic_rssi_vif;
> wilc_sdio.c:static struct wilc_sdio g_sdio;
> wilc_wlan.c:static enum chip_ps_states chip_ps_state = CHIP_WAKEDUP;
> wilc_wlan.c:static u32 pending_acks;
> wilc_wfi_cfgoperations.c:int wilc_connecting;
> 
> In order to have multiple instances working (sdio, spi, or mixed),
> all such variables need to be dynamically allocated per instance and
> stored in 'struct wilc' or one of the structures referenced by it.
> 

Actually, I have been locally working on this patch for a while now
and I will submit a patch to avoid use of most of global and static
variable soon. 

> Fixes: 9abc44ba4e2f ("staging: wilc1000: fix TODO to compile spi and
> sdio components in single module") Signed-off-by: Arnd Bergmann
>  ---
>  drivers/staging/wilc1000/Makefile   | 3 +--
>  drivers/staging/wilc1000/linux_wlan.c   | 6 --
>  drivers/staging/wilc1000/wilc_debugfs.c | 7 +--
>  drivers/staging/wilc1000/wilc_wlan.c| 6 ++
>  drivers/staging/wilc1000/wilc_wlan_if.h | 2 --
>  5 files changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/staging/wilc1000/Makefile
> b/drivers/staging/wilc1000/Makefile index f7b07c0b5ce2..ee7e26b886a5
> 100644 --- a/drivers/staging/wilc1000/Makefile
> +++ b/drivers/staging/wilc1000/Makefile
> @@ -1,4 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_WILC1000) += wilc1000.o
>  
>  ccflags-y += -DFIRMWARE_1002=\"atmel/wilc1002_firmware.bin\" \
>   -DFIRMWARE_1003=\"atmel/wilc1003_firmware.bin\"
> @@ -11,9 +12,7 @@ wilc1000-objs := wilc_wfi_cfgoperations.o
> linux_wlan.o linux_mon.o \ wilc_wlan.o
>  
>  obj-$(CONFIG_WILC1000_SDIO) += wilc1000-sdio.o
> -wilc1000-sdio-objs += $(wilc1000-objs)
>  wilc1000-sdio-objs += wilc_sdio.o
>  
>  obj-$(CONFIG_WILC1000_SPI) += wilc1000-spi.o
> -wilc1000-spi-objs += $(wilc1000-objs)
>  wilc1000-spi-objs += wilc_spi.o
> diff --git a/drivers/staging/wilc1000/linux_wlan.c
> b/drivers/staging/wilc1000/linux_wlan.c index
> 01cf4bd2e192..3b8d237decbf 100644 ---
> a/drivers/staging/wilc1000/linux_wlan.c +++
> b/drivers/staging/wilc1000/linux_wlan.c @@ -1038,8 +1038,8 @@ void
> wilc_netdev_cleanup(struct wilc *wilc) }
>  
>   kfree(wilc);
> - wilc_debugfs_remove();
>  }
> +EXPORT_SYMBOL_GPL(wilc_netdev_cleanup);
>  
>  static const struct net_device_ops wilc_netdev_ops = {
>   .ndo_init = mac_init_fn,
> @@ -1062,7 +1062,6 @@ int wilc_netdev_init(struct wilc **wilc, struct
> device *dev, int io_type, if (!wl)
>   return -ENOMEM;
>  
> - wilc_debugfs_init();
>   *wilc = wl;
>   wl->io_type = io_type;
>   wl->hif_func = ops;
> @@ -1124,3 +1123,6 @@ int wilc_netdev_init(struct wilc **wilc, struct
> device *dev, int io_type, 
>   return 0;
>  }
> +EXPORT_SYMBOL_GPL(wilc_netdev_init);
> +
> +MODULE_LICENSE("GPL");
> diff --git a/drivers/staging/wilc1000/wilc_debugfs.c
> b/drivers/staging/wilc1000/wilc_debugfs.c index
> edc72876458d..8001df66b8c2 100644 ---
> a/drivers/staging/wilc1000/wilc_debugfs.c +++
> b/drivers/staging/wilc1000/wilc_debugfs.c @@ -19,6 +19,7 @@ static
> struct dentry *wilc_dir; 
>  #define DBG_LEVEL_ALL(DEBUG | INFO | WRN | ERR)
>  static atomic_t WILC_DEBUG_LEVEL = ATOMIC_INIT(ERR);
> +EXPORT_SYMBOL_GPL(WILC_DEBUG_LEVEL);
>  
>  static ssize_t wilc_debug_level_read(struct file *file, char __user
> *userbuf, size_t count, loff_t *ppos)

[PATCH] android: binder: no outgoing transaction when thread todo has transaction

2018-08-13 Thread Sherry Yang
When a process dies, failed reply is sent to the sender of any transaction
queued on a dead thread's todo list. The sender asserts that the
received failed reply corresponds to the head of the transaction stack.
This assert can fail if the dead thread is allowed to send outgoing
transactions when there is already a transaction on its todo list,
because this new transaction can end up on the transaction stack of the
original sender. The following steps illustrate how this assertion can
fail.

1. Thread1 sends txn19 to Thread2
   (T1->transaction_stack=txn19, T2->todo+=txn19)
2. Without processing todo list, Thread2 sends txn20 to Thread1
   (T1->todo+=txn20, T2->transaction_stack=txn20)
3. T1 processes txn20 on its todo list
   (T1->transaction_stack=txn20->txn19, T1->todo=)
4. T2 dies, T2->todo cleanup attempts to send failed reply for txn19, but
   T1->transaction_stack points to txn20 -- assertion failes

Step 2. is the incorrect behavior. When there is a transaction on a
thread's todo list, this thread should not be able to send any outgoing
synchronous transactions. Only the head of the todo list needs to be
checked because only threads that are waiting for proc work can directly
receive work from another thread, and no work is allowed to be queued
on such a thread without waking up the thread. This patch also enforces
that a thread is not waiting for proc work when a work is directly
enqueued to its todo list.

Acked-by: Arve Hjønnevåg 
Signed-off-by: Sherry Yang 
---
 drivers/android/binder.c | 44 +---
 1 file changed, 32 insertions(+), 12 deletions(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index d58763b6b009..f2081934eb8b 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -822,6 +822,7 @@ static void
 binder_enqueue_deferred_thread_work_ilocked(struct binder_thread *thread,
struct binder_work *work)
 {
+   WARN_ON(!list_empty(&thread->waiting_thread_node));
binder_enqueue_work_ilocked(work, &thread->todo);
 }
 
@@ -839,6 +840,7 @@ static void
 binder_enqueue_thread_work_ilocked(struct binder_thread *thread,
   struct binder_work *work)
 {
+   WARN_ON(!list_empty(&thread->waiting_thread_node));
binder_enqueue_work_ilocked(work, &thread->todo);
thread->process_todo = true;
 }
@@ -1270,19 +1272,12 @@ static int binder_inc_node_nilocked(struct binder_node 
*node, int strong,
} else
node->local_strong_refs++;
if (!node->has_strong_ref && target_list) {
+   struct binder_thread *thread = container_of(target_list,
+   struct binder_thread, todo);
binder_dequeue_work_ilocked(&node->work);
-   /*
-* Note: this function is the only place where we queue
-* directly to a thread->todo without using the
-* corresponding binder_enqueue_thread_work() helper
-* functions; in this case it's ok to not set the
-* process_todo flag, since we know this node work will
-* always be followed by other work that starts queue
-* processing: in case of synchronous transactions, a
-* BR_REPLY or BR_ERROR; in case of oneway
-* transactions, a BR_TRANSACTION_COMPLETE.
-*/
-   binder_enqueue_work_ilocked(&node->work, target_list);
+   BUG_ON(&thread->todo != target_list);
+   binder_enqueue_deferred_thread_work_ilocked(thread,
+  &node->work);
}
} else {
if (!internal)
@@ -2723,6 +2718,7 @@ static void binder_transaction(struct binder_proc *proc,
 {
int ret;
struct binder_transaction *t;
+   struct binder_work *w;
struct binder_work *tcomplete;
binder_size_t *offp, *off_end, *off_start;
binder_size_t off_min;
@@ -2864,6 +2860,29 @@ static void binder_transaction(struct binder_proc *proc,
goto err_invalid_target_handle;
}
binder_inner_proc_lock(proc);
+
+   w = list_first_entry_or_null(&thread->todo,
+struct binder_work, entry);
+   if (!(tr->flags & TF_ONE_WAY) && w &&
+   w->type == BINDER_WORK_TRANSACTION) {
+   /*
+* Do not allow new outgoing transaction from a
+* thread that has a transaction at the head of
+* its todo list. Only need to check the head
+* because binder_select_thread_

[PATCH] media: imx: work around false-positive warning, again

2018-08-13 Thread Arnd Bergmann
A warning that I thought to be solved by a previous patch of mine
has resurfaced with gcc-8:

media/imx/imx-media-csi.c: In function 'csi_link_validate':
media/imx/imx-media-csi.c:1025:20: error: 'upstream_ep' may be used 
uninitialized in this function [-Werror=maybe-uninitialized]
media/imx/imx-media-csi.c:1026:24: error: 'upstream_ep.bus_type' may be used 
uninitialized in this function [-Werror=maybe-uninitialized]
media/imx/imx-media-csi.c:127:19: error: 'upstream_ep.bus.parallel.bus_width' 
may be used uninitialized in this function [-Werror=maybe-uninitialized]
media/imx/imx-media-csi.c: In function 'csi_enum_mbus_code':
media/imx/imx-media-csi.c:132:9: error: '*((void *)&upstream_ep+12)' may be 
used uninitialized in this function [-Werror=maybe-uninitialized]
media/imx/imx-media-csi.c:132:48: error: 'upstream_ep.bus.parallel.bus_width' 
may be used uninitialized in this function [-Werror=maybe-uninitialized]

I spent some more time digging in this time, and think I have a better
fix, bailing out of the function that either initializes or errors
out here, which simplifies the code enough for gcc to figure out
what is going on. The earlier partial workaround can be removed now,
as the new workaround is better.

Fixes: 890f27693f2a ("media: imx: work around false-positive warning")
Signed-off-by: Arnd Bergmann 
---
 drivers/staging/media/imx/imx-media-csi.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/media/imx/imx-media-csi.c 
b/drivers/staging/media/imx/imx-media-csi.c
index cd2c291e1e94..4acdd7ae612b 100644
--- a/drivers/staging/media/imx/imx-media-csi.c
+++ b/drivers/staging/media/imx/imx-media-csi.c
@@ -165,6 +165,9 @@ static int csi_get_upstream_endpoint(struct csi_priv *priv,
struct v4l2_subdev *sd;
struct media_pad *pad;
 
+   if (!IS_ENABLED(CONFIG_OF))
+   return -ENXIO;
+
if (!priv->src_sd)
return -EPIPE;
 
@@ -1050,7 +1053,7 @@ static int csi_link_validate(struct v4l2_subdev *sd,
 struct v4l2_subdev_format *sink_fmt)
 {
struct csi_priv *priv = v4l2_get_subdevdata(sd);
-   struct v4l2_fwnode_endpoint upstream_ep = {};
+   struct v4l2_fwnode_endpoint upstream_ep;
bool is_csi2;
int ret;
 
-- 
2.18.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] staging: wilc1000: revert "fix TODO to compile spi and sdio components in single module"

2018-08-13 Thread Arnd Bergmann
The TODO item named "make spi and sdio components coexist in one build"
was apparently addressed a long time ago, but never removed from the
TODO file. However, the new patch that tries to address it actually
makes it worse again by duplicating the common parts of the driver into
two separate modules rather than sharing them. This also introduces a
build regression when one of the two is built-in while the other is a
loadable module:

drivers/staging/wilc1000/wilc_debugfs.o:(.data+0x10): undefined reference to 
`__this_module'

Reverting the patch makes it build again. I'm leaving the TODO file
modification though, as there is nothing left to do for this item.

A related problem however still seems to exist: one still cannot have
multiple concurrent instances of wilc1000 devices present in the
system, as there are lots of shared global variables such as

host_interface.c:static struct wilc_vif *periodic_rssi_vif;
wilc_sdio.c:static struct wilc_sdio g_sdio;
wilc_wlan.c:static enum chip_ps_states chip_ps_state = CHIP_WAKEDUP;
wilc_wlan.c:static u32 pending_acks;
wilc_wfi_cfgoperations.c:int wilc_connecting;

In order to have multiple instances working (sdio, spi, or mixed),
all such variables need to be dynamically allocated per instance and
stored in 'struct wilc' or one of the structures referenced by it.

Fixes: 9abc44ba4e2f ("staging: wilc1000: fix TODO to compile spi and sdio 
components in single module")
Signed-off-by: Arnd Bergmann 
---
 drivers/staging/wilc1000/Makefile   | 3 +--
 drivers/staging/wilc1000/linux_wlan.c   | 6 --
 drivers/staging/wilc1000/wilc_debugfs.c | 7 +--
 drivers/staging/wilc1000/wilc_wlan.c| 6 ++
 drivers/staging/wilc1000/wilc_wlan_if.h | 2 --
 5 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/wilc1000/Makefile 
b/drivers/staging/wilc1000/Makefile
index f7b07c0b5ce2..ee7e26b886a5 100644
--- a/drivers/staging/wilc1000/Makefile
+++ b/drivers/staging/wilc1000/Makefile
@@ -1,4 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_WILC1000) += wilc1000.o
 
 ccflags-y += -DFIRMWARE_1002=\"atmel/wilc1002_firmware.bin\" \
-DFIRMWARE_1003=\"atmel/wilc1003_firmware.bin\"
@@ -11,9 +12,7 @@ wilc1000-objs := wilc_wfi_cfgoperations.o linux_wlan.o 
linux_mon.o \
wilc_wlan.o
 
 obj-$(CONFIG_WILC1000_SDIO) += wilc1000-sdio.o
-wilc1000-sdio-objs += $(wilc1000-objs)
 wilc1000-sdio-objs += wilc_sdio.o
 
 obj-$(CONFIG_WILC1000_SPI) += wilc1000-spi.o
-wilc1000-spi-objs += $(wilc1000-objs)
 wilc1000-spi-objs += wilc_spi.o
diff --git a/drivers/staging/wilc1000/linux_wlan.c 
b/drivers/staging/wilc1000/linux_wlan.c
index 01cf4bd2e192..3b8d237decbf 100644
--- a/drivers/staging/wilc1000/linux_wlan.c
+++ b/drivers/staging/wilc1000/linux_wlan.c
@@ -1038,8 +1038,8 @@ void wilc_netdev_cleanup(struct wilc *wilc)
}
 
kfree(wilc);
-   wilc_debugfs_remove();
 }
+EXPORT_SYMBOL_GPL(wilc_netdev_cleanup);
 
 static const struct net_device_ops wilc_netdev_ops = {
.ndo_init = mac_init_fn,
@@ -1062,7 +1062,6 @@ int wilc_netdev_init(struct wilc **wilc, struct device 
*dev, int io_type,
if (!wl)
return -ENOMEM;
 
-   wilc_debugfs_init();
*wilc = wl;
wl->io_type = io_type;
wl->hif_func = ops;
@@ -1124,3 +1123,6 @@ int wilc_netdev_init(struct wilc **wilc, struct device 
*dev, int io_type,
 
return 0;
 }
+EXPORT_SYMBOL_GPL(wilc_netdev_init);
+
+MODULE_LICENSE("GPL");
diff --git a/drivers/staging/wilc1000/wilc_debugfs.c 
b/drivers/staging/wilc1000/wilc_debugfs.c
index edc72876458d..8001df66b8c2 100644
--- a/drivers/staging/wilc1000/wilc_debugfs.c
+++ b/drivers/staging/wilc1000/wilc_debugfs.c
@@ -19,6 +19,7 @@ static struct dentry *wilc_dir;
 
 #define DBG_LEVEL_ALL  (DEBUG | INFO | WRN | ERR)
 static atomic_t WILC_DEBUG_LEVEL = ATOMIC_INIT(ERR);
+EXPORT_SYMBOL_GPL(WILC_DEBUG_LEVEL);
 
 static ssize_t wilc_debug_level_read(struct file *file, char __user *userbuf,
 size_t count, loff_t *ppos)
@@ -87,7 +88,7 @@ static struct wilc_debugfs_info_t debugfs_info[] = {
},
 };
 
-int wilc_debugfs_init(void)
+static int __init wilc_debugfs_init(void)
 {
int i;
struct wilc_debugfs_info_t *info;
@@ -103,10 +104,12 @@ int wilc_debugfs_init(void)
}
return 0;
 }
+module_init(wilc_debugfs_init);
 
-void wilc_debugfs_remove(void)
+static void __exit wilc_debugfs_remove(void)
 {
debugfs_remove_recursive(wilc_dir);
 }
+module_exit(wilc_debugfs_remove);
 
 #endif
diff --git a/drivers/staging/wilc1000/wilc_wlan.c 
b/drivers/staging/wilc1000/wilc_wlan.c
index 6787b6e9f124..8b184aa30d25 100644
--- a/drivers/staging/wilc1000/wilc_wlan.c
+++ b/drivers/staging/wilc1000/wilc_wlan.c
@@ -417,6 +417,7 @@ void chip_allow_sleep(struct wilc *wilc)
wilc->hif_func->hif_write_reg(wilc, 0xf0, reg & ~BIT(0));
wilc->hif_func->hif_write_reg(wilc, 0xfa, 0);
 }
+EXPORT_SYMBOL_GPL(chip_allow_

[PATCH] Staging: rtlwifi: efuse: fixed a line length coding style issue

2018-08-13 Thread Tom Todd
Fixed a code style issue. Line length over 80 characters.

Signed-off-by: Tom Todd 
---
 drivers/staging/rtlwifi/efuse.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rtlwifi/efuse.c b/drivers/staging/rtlwifi/efuse.c
index 1dc71455f270..49ec9728fb04 100644
--- a/drivers/staging/rtlwifi/efuse.c
+++ b/drivers/staging/rtlwifi/efuse.c
@@ -245,7 +245,8 @@ void read_efuse(struct ieee80211_hw *hw, u16 _offset, u16 
_size_byte, u8 *pbuf)
if (!efuse_word)
goto out;
for (i = 0; i < EFUSE_MAX_WORD_UNIT; i++) {
-   efuse_word[i] = kcalloc(efuse_max_section, sizeof(u16), 
GFP_ATOMIC);
+   efuse_word[i] =
+   kcalloc(efuse_max_section, sizeof(u16), GFP_ATOMIC);
if (!efuse_word[i])
goto done;
}
-- 
2.18.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 2/5] vmbus: add driver_override support

2018-08-13 Thread Stephen Hemminger
On Mon, 13 Aug 2018 19:30:50 +
"Michael Kelley (EOSG)"  wrote:

> From: k...@linuxonhyperv.com   Sent: Friday, August 
> 10, 2018 4:06 PM
> 
> > From: Stephen Hemminger 
> > 
> > Add support for overriding the default driver for a VMBus device
> > in the same way that it can be done for PCI devices. This patch
> > adds the /sys/bus/vmbus/devices/.../driver_override file
> > and the logic for matching.
> > 
> > This is used by driverctl tool to do driver override.
> > https://gitlab.com/driverctl/driverctl
> > 
> > Signed-off-by: Stephen Hemminger 
> > Signed-off-by: K. Y. Srinivasan 
> > ---
> > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> > index b1b548a21f91..e6d8fdac6d8b 100644
> > --- a/drivers/hv/vmbus_drv.c
> > +++ b/drivers/hv/vmbus_drv.c
> > @@ -498,6 +498,54 @@ static ssize_t device_show(struct device *dev,
> >  }
> >  static DEVICE_ATTR_RO(device);
> > 
> > +static ssize_t driver_override_store(struct device *dev,
> > +struct device_attribute *attr,
> > +const char *buf, size_t count)
> > +{
> > +   struct hv_device *hv_dev = device_to_hv_device(dev);
> > +   char *driver_override, *old, *cp;
> > +
> > +   /* We need to keep extra room for a newline */
> > +   if (count >= (PAGE_SIZE - 1))
> > +   return -EINVAL;  
> 
> Does 'count' actually have a relationship to PAGE_SIZE, or
> is PAGE_SIZE just used as an arbitrary size limit?  I'm
> wondering what happens on ARM64 with a 64K page size,
> for example.  If it's just arbitrary, coding such a constant
> would be better.

This comes from original code how sysfs works.
Sysfs uses PAGE_SIZE for string buffers on store. This code
snippet was cloned from PCI version of same thing.

> > +/*
> > + * Return a matching hv_vmbus_device_id pointer.
> > + * If there is no match, return NULL.
> > + */
> > +static const struct hv_vmbus_device_id *hv_vmbus_get_id(struct hv_driver 
> > *drv,
> > +   struct hv_device *dev)
> > +{
> > +   const uuid_le *guid = &dev->dev_type;
> > +   const struct hv_vmbus_device_id *id;
> > 
> > -   return NULL;
> > +   /* When driver_override is set, only bind to the matching driver */
> > +   if (dev->driver_override && strcmp(dev->driver_override, drv->name))
> > +   return NULL;  
> 
> This function needs to be covered by the device lock, so that
> dev->driver_override can't be set to NULL and the memory freed
> during the above 'if' statement.  When called from vmbus_probe(),
> the device lock is held, so it's good. But when called from
> vmbus_match(), the device lock may not be held: consider the path
> __driver_attach() -> driver_match_device() -> vmbus_match().

The patch clones existing locking model from PCI.
So either both are broken, or somehow vmbus is behaving differently.
I will investigate.





___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 2/5] vmbus: add driver_override support

2018-08-13 Thread gre...@linuxfoundation.org
On Mon, Aug 13, 2018 at 07:30:50PM +, Michael Kelley (EOSG) wrote:
> From: k...@linuxonhyperv.com   Sent: Friday, August 
> 10, 2018 4:06 PM
> 
> > From: Stephen Hemminger 
> > 
> > Add support for overriding the default driver for a VMBus device
> > in the same way that it can be done for PCI devices. This patch
> > adds the /sys/bus/vmbus/devices/.../driver_override file
> > and the logic for matching.
> > 
> > This is used by driverctl tool to do driver override.
> > https://gitlab.com/driverctl/driverctl
> > 
> > Signed-off-by: Stephen Hemminger 
> > Signed-off-by: K. Y. Srinivasan 
> > ---
> > diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> > index b1b548a21f91..e6d8fdac6d8b 100644
> > --- a/drivers/hv/vmbus_drv.c
> > +++ b/drivers/hv/vmbus_drv.c
> > @@ -498,6 +498,54 @@ static ssize_t device_show(struct device *dev,
> >  }
> >  static DEVICE_ATTR_RO(device);
> > 
> > +static ssize_t driver_override_store(struct device *dev,
> > +struct device_attribute *attr,
> > +const char *buf, size_t count)
> > +{
> > +   struct hv_device *hv_dev = device_to_hv_device(dev);
> > +   char *driver_override, *old, *cp;
> > +
> > +   /* We need to keep extra room for a newline */
> > +   if (count >= (PAGE_SIZE - 1))
> > +   return -EINVAL;
> 
> Does 'count' actually have a relationship to PAGE_SIZE, or
> is PAGE_SIZE just used as an arbitrary size limit?  I'm
> wondering what happens on ARM64 with a 64K page size,
> for example.  If it's just arbitrary, coding such a constant
> would be better.

sysfs buffers are PAGE_SIZE big.

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [PATCH 2/5] vmbus: add driver_override support

2018-08-13 Thread Michael Kelley (EOSG)
From: k...@linuxonhyperv.com   Sent: Friday, August 10, 
2018 4:06 PM

> From: Stephen Hemminger 
> 
> Add support for overriding the default driver for a VMBus device
> in the same way that it can be done for PCI devices. This patch
> adds the /sys/bus/vmbus/devices/.../driver_override file
> and the logic for matching.
> 
> This is used by driverctl tool to do driver override.
> https://gitlab.com/driverctl/driverctl
> 
> Signed-off-by: Stephen Hemminger 
> Signed-off-by: K. Y. Srinivasan 
> ---
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index b1b548a21f91..e6d8fdac6d8b 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -498,6 +498,54 @@ static ssize_t device_show(struct device *dev,
>  }
>  static DEVICE_ATTR_RO(device);
> 
> +static ssize_t driver_override_store(struct device *dev,
> +  struct device_attribute *attr,
> +  const char *buf, size_t count)
> +{
> + struct hv_device *hv_dev = device_to_hv_device(dev);
> + char *driver_override, *old, *cp;
> +
> + /* We need to keep extra room for a newline */
> + if (count >= (PAGE_SIZE - 1))
> + return -EINVAL;

Does 'count' actually have a relationship to PAGE_SIZE, or
is PAGE_SIZE just used as an arbitrary size limit?  I'm
wondering what happens on ARM64 with a 64K page size,
for example.  If it's just arbitrary, coding such a constant
would be better.

> +/*
> + * Return a matching hv_vmbus_device_id pointer.
> + * If there is no match, return NULL.
> + */
> +static const struct hv_vmbus_device_id *hv_vmbus_get_id(struct hv_driver 
> *drv,
> + struct hv_device *dev)
> +{
> + const uuid_le *guid = &dev->dev_type;
> + const struct hv_vmbus_device_id *id;
> 
> - return NULL;
> + /* When driver_override is set, only bind to the matching driver */
> + if (dev->driver_override && strcmp(dev->driver_override, drv->name))
> + return NULL;

This function needs to be covered by the device lock, so that
dev->driver_override can't be set to NULL and the memory freed
during the above 'if' statement.  When called from vmbus_probe(),
the device lock is held, so it's good. But when called from
vmbus_match(), the device lock may not be held: consider the path
__driver_attach() -> driver_match_device() -> vmbus_match().

Michael
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] vmbus: don't return values for uninitalized channels

2018-08-13 Thread Vitaly Kuznetsov
Stephen Hemminger  writes:

> For unsupported device types, the vmbus channel ringbuffer is never
> initialized, and therefore reading the sysfs files will return garbage
> or cause a kernel OOPS.
>
> Fixes: c2e5df616e1a ("vmbus: add per-channel sysfs info")
> Signed-off-by: Stephen Hemminger 
> ---
>  drivers/hv/vmbus_drv.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index b10fe26c4891..c9a466be7709 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -1178,6 +1178,9 @@ static ssize_t vmbus_chan_attr_show(struct kobject 
> *kobj,
>   if (!attribute->show)
>   return -EIO;
>
> + if (chan->state != CHANNEL_OPENED_STATE)
> + return -EINVAL;
> +
>   return attribute->show(chan, buf);
>  }

Tested-by: Vitaly Kuznetsov 

-- 
  Vitaly
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] vmbus: don't return values for uninitalized channels

2018-08-13 Thread Stephen Hemminger
For unsupported device types, the vmbus channel ringbuffer is never
initialized, and therefore reading the sysfs files will return garbage
or cause a kernel OOPS.

Fixes: c2e5df616e1a ("vmbus: add per-channel sysfs info")
Signed-off-by: Stephen Hemminger 
---
 drivers/hv/vmbus_drv.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index b10fe26c4891..c9a466be7709 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1178,6 +1178,9 @@ static ssize_t vmbus_chan_attr_show(struct kobject *kobj,
if (!attribute->show)
return -EIO;
 
+   if (chan->state != CHANNEL_OPENED_STATE)
+   return -EINVAL;
+
return attribute->show(chan, buf);
 }
 
-- 
2.18.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


RE: [PATCH 1/5] Tools: hv: Fix a bug in the key delete code

2018-08-13 Thread Michael Kelley (EOSG)
From: k...@linuxonhyperv.com  Sent: Friday, August 10, 
2018 4:06 PM
> 
> Fix a bug in the key delete code - the num_records range
> from 0 to num_records-1.
> 
> Signed-off-by: K. Y. Srinivasan 
> Reported-by: David Binderman 
> Cc: 
> ---

Reviewed-by: Michael Kelley 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] Staging: octeon-usb: Replaces CVMX_WAIT_FOR_FIELD32 macro with a function

2018-08-13 Thread Georgios Tsotsos
Replaces CVMX_WAIT_FOR_FIELD32 macro with equivalent function.

Signed-off-by: Georgios Tsotsos 
---
 drivers/staging/octeon-usb/octeon-hcd.c | 56 +
 1 file changed, 29 insertions(+), 27 deletions(-)

diff --git a/drivers/staging/octeon-usb/octeon-hcd.c 
b/drivers/staging/octeon-usb/octeon-hcd.c
index cff5e790b196..f188e19c6fc1 100644
--- a/drivers/staging/octeon-usb/octeon-hcd.c
+++ b/drivers/staging/octeon-usb/octeon-hcd.c
@@ -377,29 +377,6 @@ struct octeon_hcd {
struct cvmx_usb_tx_fifo nonperiodic;
 };
 
-/* This macro spins on a register waiting for it to reach a condition. */
-#define CVMX_WAIT_FOR_FIELD32(address, _union, cond, timeout_usec) \
-   ({int result;   \
-   do {\
-   u64 done = cvmx_get_cycle() + (u64)timeout_usec *   \
-  octeon_get_clock_rate() / 100;   \
-   union _union c; \
-   \
-   while (1) { \
-   c.u32 = cvmx_usb_read_csr32(usb, address);  \
-   \
-   if (cond) { \
-   result = 0; \
-   break;  \
-   } else if (cvmx_get_cycle() > done) {   \
-   result = -1;\
-   break;  \
-   } else  \
-   __delay(100);   \
-   }   \
-   } while (0);\
-   result; })
-
 /*
  * This macro logically sets a single field in a CSR. It does the sequence
  * read, modify, and write
@@ -593,6 +570,33 @@ static inline int cvmx_usb_get_data_pid(struct 
cvmx_usb_pipe *pipe)
return 0; /* Data0 */
 }
 
+/* Loops through register until txfflsh or rxfflsh become zero.*/
+static int cvmx_wait_tx_rx(struct octeon_hcd *usb, int fflsh_type)
+{
+   int result;
+   u64 address = CVMX_USBCX_GRSTCTL(usb->index);
+   u64 done = cvmx_get_cycle() + 100 *
+  (u64)octeon_get_clock_rate / 100;
+   union cvmx_usbcx_grstctl c;
+
+   while (1) {
+   c.u32 = cvmx_usb_read_csr32(usb, address);
+   if (fflsh_type == 0 && c.s.txfflsh == 0) {
+   result = 0;
+   break;
+   } else if (fflsh_type == 1 && c.s.rxfflsh == 0) {
+   result = 0;
+   break;
+   } else if (cvmx_get_cycle() > done) {
+   result = -1;
+   break;
+   }
+
+   __delay(100);
+   }
+   return result;
+}
+
 static void cvmx_fifo_setup(struct octeon_hcd *usb)
 {
union cvmx_usbcx_ghwcfg3 usbcx_ghwcfg3;
@@ -634,12 +638,10 @@ static void cvmx_fifo_setup(struct octeon_hcd *usb)
cvmx_usbcx_grstctl, txfnum, 0x10);
USB_SET_FIELD32(CVMX_USBCX_GRSTCTL(usb->index),
cvmx_usbcx_grstctl, txfflsh, 1);
-   CVMX_WAIT_FOR_FIELD32(CVMX_USBCX_GRSTCTL(usb->index),
- cvmx_usbcx_grstctl, c.s.txfflsh == 0, 100);
+   cvmx_wait_tx_rx(usb, 0);
USB_SET_FIELD32(CVMX_USBCX_GRSTCTL(usb->index),
cvmx_usbcx_grstctl, rxfflsh, 1);
-   CVMX_WAIT_FOR_FIELD32(CVMX_USBCX_GRSTCTL(usb->index),
- cvmx_usbcx_grstctl, c.s.rxfflsh == 0, 100);
+   cvmx_wait_tx_rx(usb, 1);
 }
 
 /**
-- 
2.16.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: erofs: change 'unsigned' to 'unsigned int'

2018-08-13 Thread Gao Xiang
Hi Leon,

On 2018/8/13 23:20, Leon Imhof wrote:
> Fix coding style issue "Prefer 'unsigned int' to bare use of 'unsigned'"
> detected by checkpatch.pl
> 
> Signed-off-by: Leon Imhof 
> ---

Thanks for the patch, could be better if use `[PATCH v2]' to indicate
that you send a new version of `[PATCH] staging: erofs: add int to usigned'.

Reviewed-by: Gao Xiang 

Thanks,
Gao Xiang

>  drivers/staging/erofs/data.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/erofs/data.c b/drivers/staging/erofs/data.c
> index ac263a180253..2a401216007a 100644
> --- a/drivers/staging/erofs/data.c
> +++ b/drivers/staging/erofs/data.c
> @@ -337,7 +337,7 @@ static int erofs_raw_access_readpage(struct file *file, 
> struct page *page)
>  
>  static int erofs_raw_access_readpages(struct file *filp,
>   struct address_space *mapping,
> - struct list_head *pages, unsigned nr_pages)
> + struct list_head *pages, unsigned int nr_pages)
>  {
>   erofs_off_t last_block;
>   struct bio *bio = NULL;
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] staging: erofs: change 'unsigned' to 'unsigned int'

2018-08-13 Thread Leon Imhof
Fix coding style issue "Prefer 'unsigned int' to bare use of 'unsigned'"
detected by checkpatch.pl

Signed-off-by: Leon Imhof 
---
 drivers/staging/erofs/data.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/erofs/data.c b/drivers/staging/erofs/data.c
index ac263a180253..2a401216007a 100644
--- a/drivers/staging/erofs/data.c
+++ b/drivers/staging/erofs/data.c
@@ -337,7 +337,7 @@ static int erofs_raw_access_readpage(struct file *file, 
struct page *page)
 
 static int erofs_raw_access_readpages(struct file *filp,
struct address_space *mapping,
-   struct list_head *pages, unsigned nr_pages)
+   struct list_head *pages, unsigned int nr_pages)
 {
erofs_off_t last_block;
struct bio *bio = NULL;
-- 
2.18.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 01/14] staging: media: tegra-vde: Support BSEV clock and reset

2018-08-13 Thread Dmitry Osipenko
On Monday, 13 August 2018 17:50:14 MSK Thierry Reding wrote:
> From: Thierry Reding 
> 
> The BSEV clock has a separate gate bit and can not be assumed to be
> always enabled. Add explicit handling for the BSEV clock and reset.
> 
> This fixes an issue on Tegra124 where the BSEV clock is not enabled
> by default and therefore accessing the BSEV registers will hang the
> CPU if the BSEV clock is not enabled and the reset not deasserted.
> 
> Signed-off-by: Thierry Reding 
> ---

Are you sure that BSEV clock is really needed for T20/30? I've tried already 
to disable the clock explicitly and everything kept working, though I'll try 
again.

The device-tree changes should be reflected in the binding documentation.



___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 09/14] staging: media: tegra-vde: Add IOMMU support

2018-08-13 Thread Thierry Reding
From: Thierry Reding 

Implement support for using an IOMMU to map physically discontiguous
buffers into contiguous I/O virtual mappings that the VDE can use. This
allows importing arbitrary DMA-BUFs for use by the VDE.

While at it, make sure that the device is detached from any DMA/IOMMU
mapping that it might have automatically been attached to at boot. If
using the IOMMU API explicitly, detaching from any existing mapping is
required to avoid double mapping of buffers.

Signed-off-by: Thierry Reding 
---
 drivers/staging/media/tegra-vde/tegra-vde.c | 171 +---
 1 file changed, 153 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c 
b/drivers/staging/media/tegra-vde/tegra-vde.c
index 2496a03fd158..3bc0bfcfe34e 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -13,7 +13,9 @@
 #include 
 #include 
 #include 
+#include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -22,6 +24,10 @@
 #include 
 #include 
 
+#if IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)
+#include 
+#endif
+
 #include 
 
 #include 
@@ -61,6 +67,11 @@ struct video_frame {
u32 frame_num;
u32 flags;
u64 modifier;
+
+   struct iova *y_iova;
+   struct iova *cb_iova;
+   struct iova *cr_iova;
+   struct iova *aux_iova;
 };
 
 struct tegra_vde_soc {
@@ -93,6 +104,12 @@ struct tegra_vde {
struct clk *clk_bsev;
dma_addr_t iram_lists_addr;
u32 *iram;
+
+   struct iommu_domain *domain;
+   struct iommu_group *group;
+   struct iova_domain iova;
+   unsigned long limit;
+   unsigned int shift;
 };
 
 static void tegra_vde_set_bits(struct tegra_vde *vde,
@@ -634,12 +651,22 @@ static void tegra_vde_decode_frame(struct tegra_vde *vde,
VDE_WR(0x2000 | (macroblocks_nb - 1), vde->sxe + 0x00);
 }
 
-static void tegra_vde_detach_and_put_dmabuf(struct dma_buf_attachment *a,
+static void tegra_vde_detach_and_put_dmabuf(struct tegra_vde *vde,
+   struct dma_buf_attachment *a,
struct sg_table *sgt,
+   struct iova *iova,
enum dma_data_direction dma_dir)
 {
struct dma_buf *dmabuf = a->dmabuf;
 
+   if (vde->domain) {
+   unsigned long size = iova_size(iova) << vde->shift;
+   dma_addr_t addr = iova_dma_addr(&vde->iova, iova);
+
+   iommu_unmap(vde->domain, addr, size);
+   __free_iova(&vde->iova, iova);
+   }
+
dma_buf_unmap_attachment(a, sgt, dma_dir);
dma_buf_detach(dmabuf, a);
dma_buf_put(dmabuf);
@@ -651,14 +678,16 @@ static int tegra_vde_attach_dmabuf(struct tegra_vde *vde,
   size_t min_size,
   size_t align_size,
   struct dma_buf_attachment **a,
-  dma_addr_t *addr,
+  dma_addr_t *addrp,
   struct sg_table **s,
-  size_t *size,
+  struct iova **iovap,
+  size_t *sizep,
   enum dma_data_direction dma_dir)
 {
struct dma_buf_attachment *attachment;
struct dma_buf *dmabuf;
struct sg_table *sgt;
+   size_t size;
int err;
 
dmabuf = dma_buf_get(fd);
@@ -695,18 +724,47 @@ static int tegra_vde_attach_dmabuf(struct tegra_vde *vde,
goto err_detach;
}
 
-   if (sgt->nents != 1) {
+   if (sgt->nents > 1 && !vde->domain) {
dev_err(vde->dev, "Sparse DMA region is unsupported\n");
err = -EINVAL;
goto err_unmap;
}
 
-   *addr = sg_dma_address(sgt->sgl) + offset;
+   if (vde->domain) {
+   int prot = IOMMU_READ | IOMMU_WRITE;
+   struct iova *iova;
+   dma_addr_t addr;
+
+   size = (dmabuf->size - offset) >> vde->shift;
+
+   iova = alloc_iova(&vde->iova, size, vde->limit - 1, true);
+   if (!iova) {
+   err = -ENOMEM;
+   goto err_unmap;
+   }
+
+   addr = iova_dma_addr(&vde->iova, iova);
+
+   size = iommu_map_sg(vde->domain, addr, sgt->sgl, sgt->nents,
+   prot);
+   if (!size) {
+   __free_iova(&vde->iova, iova);
+   err = -ENXIO;
+   goto err_unmap;
+   }
+
+   *addrp = addr;
+   *iovap = iova;
+   } else {
+   *addrp = sg_dma_address(sgt->sgl) + offset;
+   size = dmabuf->size - offset;
+   }
+
*a = attachment;
*s = sgt;
 
-   

[PATCH 02/14] staging: media: tegra-vde: Support reference picture marking

2018-08-13 Thread Thierry Reding
From: Thierry Reding 

Tegra114 and Tegra124 support reference picture marking, which will
cause BSEV to write picture marking data to SDRAM. Make sure there is
a valid destination address for that data to avoid error messages from
the memory controller.

Signed-off-by: Thierry Reding 
---
 drivers/staging/media/tegra-vde/tegra-vde.c | 54 -
 drivers/staging/media/tegra-vde/uapi.h  |  3 ++
 2 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c 
b/drivers/staging/media/tegra-vde/tegra-vde.c
index 9d8f833744db..3027b11b11ae 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -60,7 +60,12 @@ struct video_frame {
u32 flags;
 };
 
+struct tegra_vde_soc {
+   bool supports_ref_pic_marking;
+};
+
 struct tegra_vde {
+   const struct tegra_vde_soc *soc;
void __iomem *sxe;
void __iomem *bsev;
void __iomem *mbe;
@@ -330,6 +335,7 @@ static int tegra_vde_setup_hw_context(struct tegra_vde *vde,
  struct video_frame *dpb_frames,
  dma_addr_t bitstream_data_addr,
  size_t bitstream_data_size,
+ dma_addr_t secure_addr,
  unsigned int macroblocks_nb)
 {
struct device *dev = vde->miscdev.parent;
@@ -454,6 +460,9 @@ static int tegra_vde_setup_hw_context(struct tegra_vde *vde,
 
VDE_WR(bitstream_data_addr, vde->sxe + 0x6C);
 
+   if (vde->soc->supports_ref_pic_marking)
+   VDE_WR(secure_addr, vde->sxe + 0x7c);
+
value = 0x1005;
value |= ctx->pic_width_in_mbs << 11;
value |= ctx->pic_height_in_mbs << 3;
@@ -772,12 +781,15 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde 
*vde,
struct tegra_vde_h264_frame __user *frames_user;
struct video_frame *dpb_frames;
struct dma_buf_attachment *bitstream_data_dmabuf_attachment;
-   struct sg_table *bitstream_sgt;
+   struct dma_buf_attachment *secure_attachment = NULL;
+   struct sg_table *bitstream_sgt, *secure_sgt;
enum dma_data_direction dma_dir;
dma_addr_t bitstream_data_addr;
+   dma_addr_t secure_addr;
dma_addr_t bsev_ptr;
size_t lsize, csize;
size_t bitstream_data_size;
+   size_t secure_size;
unsigned int macroblocks_nb;
unsigned int read_bytes;
unsigned int cstride;
@@ -803,6 +815,18 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde 
*vde,
if (ret)
return ret;
 
+   if (vde->soc->supports_ref_pic_marking) {
+   ret = tegra_vde_attach_dmabuf(dev, ctx.secure_fd,
+ ctx.secure_offset, 0, SZ_256,
+ &secure_attachment,
+ &secure_addr,
+ &secure_sgt,
+ &secure_size,
+ DMA_TO_DEVICE);
+   if (ret)
+   goto release_bitstream_dmabuf;
+   }
+
dpb_frames = kcalloc(ctx.dpb_frames_nb, sizeof(*dpb_frames),
 GFP_KERNEL);
if (!dpb_frames) {
@@ -876,6 +900,7 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde 
*vde,
ret = tegra_vde_setup_hw_context(vde, &ctx, dpb_frames,
 bitstream_data_addr,
 bitstream_data_size,
+secure_addr,
 macroblocks_nb);
if (ret)
goto put_runtime_pm;
@@ -929,6 +954,10 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde 
*vde,
kfree(dpb_frames);
 
 release_bitstream_dmabuf:
+   if (secure_attachment)
+   tegra_vde_detach_and_put_dmabuf(secure_attachment, secure_sgt,
+   DMA_TO_DEVICE);
+
tegra_vde_detach_and_put_dmabuf(bitstream_data_dmabuf_attachment,
bitstream_sgt, DMA_TO_DEVICE);
 
@@ -1029,6 +1058,8 @@ static int tegra_vde_probe(struct platform_device *pdev)
 
platform_set_drvdata(pdev, vde);
 
+   vde->soc = of_device_get_match_data(&pdev->dev);
+
regs = platform_get_resource_byname(pdev, IORESOURCE_MEM, "sxe");
if (!regs)
return -ENODEV;
@@ -1258,8 +1289,27 @@ static const struct dev_pm_ops tegra_vde_pm_ops = {
tegra_vde_pm_resume)
 };
 
+static const struct tegra_vde_soc tegra20_vde_soc = {
+   .supports_ref_pic_marking = false,
+};
+
+static const struct tegra_vde_soc tegra30_vde_soc = {
+   .supports_ref_pic_marking = false,
+};
+
+static const struct tegra_v

[PATCH 03/14] staging: media: tegra-vde: Prepare for interlacing support

2018-08-13 Thread Thierry Reding
From: Thierry Reding 

The number of frames doubles when decoding interlaced content and the
structures describing the frames double in size. Take that into account
to prepare for interlacing support.

Signed-off-by: Thierry Reding 
---
 drivers/staging/media/tegra-vde/tegra-vde.c | 73 -
 1 file changed, 58 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c 
b/drivers/staging/media/tegra-vde/tegra-vde.c
index 3027b11b11ae..1a40f6dff7c8 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -61,7 +61,9 @@ struct video_frame {
 };
 
 struct tegra_vde_soc {
+   unsigned int num_ref_pics;
bool supports_ref_pic_marking;
+   bool supports_interlacing;
 };
 
 struct tegra_vde {
@@ -205,8 +207,12 @@ static void tegra_vde_setup_frameid(struct tegra_vde *vde,
u32 cr_addr = frame ? frame->cr_addr : 0x6CDEAD00;
u32 value1 = frame ? ((mbs_width << 16) | mbs_height) : 0;
u32 value2 = frame ? mbs_width + 1) >> 1) << 6) | 1) : 0;
+   u32 value = y_addr >> 8;
 
-   VDE_WR(y_addr  >> 8, vde->frameid + 0x000 + frameid * 4);
+   if (vde->soc->supports_interlacing)
+   value |= BIT(31);
+
+   VDE_WR(value,vde->frameid + 0x000 + frameid * 4);
VDE_WR(cb_addr >> 8, vde->frameid + 0x100 + frameid * 4);
VDE_WR(cr_addr >> 8, vde->frameid + 0x180 + frameid * 4);
VDE_WR(value1,   vde->frameid + 0x080 + frameid * 4);
@@ -229,20 +235,23 @@ static void tegra_setup_frameidx(struct tegra_vde *vde,
 }
 
 static void tegra_vde_setup_iram_entry(struct tegra_vde *vde,
+  unsigned int num_ref_pics,
   unsigned int table,
   unsigned int row,
   u32 value1, u32 value2)
 {
+   unsigned int entries = num_ref_pics * 2;
u32 *iram_tables = vde->iram;
 
dev_dbg(vde->miscdev.parent, "IRAM table %u: row %u: 0x%08X 0x%08X\n",
table, row, value1, value2);
 
-   iram_tables[0x20 * table + row * 2] = value1;
-   iram_tables[0x20 * table + row * 2 + 1] = value2;
+   iram_tables[entries * table + row * 2] = value1;
+   iram_tables[entries * table + row * 2 + 1] = value2;
 }
 
 static void tegra_vde_setup_iram_tables(struct tegra_vde *vde,
+   unsigned int num_ref_pics,
struct video_frame *dpb_frames,
unsigned int ref_frames_nb,
unsigned int with_earlier_poc_nb)
@@ -251,13 +260,17 @@ static void tegra_vde_setup_iram_tables(struct tegra_vde 
*vde,
u32 value, aux_addr;
int with_later_poc_nb;
unsigned int i, k;
+   size_t size;
+
+   size = num_ref_pics * 4 * 8;
+   memset(vde->iram, 0, size);
 
dev_dbg(vde->miscdev.parent, "DPB: Frame 0: frame_num = %d\n",
dpb_frames[0].frame_num);
 
dev_dbg(vde->miscdev.parent, "REF L0:\n");
 
-   for (i = 0; i < 16; i++) {
+   for (i = 0; i < num_ref_pics; i++) {
if (i < ref_frames_nb) {
frame = &dpb_frames[i + 1];
 
@@ -277,10 +290,14 @@ static void tegra_vde_setup_iram_tables(struct tegra_vde 
*vde,
value = 0;
}
 
-   tegra_vde_setup_iram_entry(vde, 0, i, value, aux_addr);
-   tegra_vde_setup_iram_entry(vde, 1, i, value, aux_addr);
-   tegra_vde_setup_iram_entry(vde, 2, i, value, aux_addr);
-   tegra_vde_setup_iram_entry(vde, 3, i, value, aux_addr);
+   tegra_vde_setup_iram_entry(vde, num_ref_pics, 0, i, value,
+  aux_addr);
+   tegra_vde_setup_iram_entry(vde, num_ref_pics, 1, i, value,
+  aux_addr);
+   tegra_vde_setup_iram_entry(vde, num_ref_pics, 2, i, value,
+  aux_addr);
+   tegra_vde_setup_iram_entry(vde, num_ref_pics, 3, i, value,
+  aux_addr);
}
 
if (!(dpb_frames[0].flags & FLAG_B_FRAME))
@@ -309,7 +326,8 @@ static void tegra_vde_setup_iram_tables(struct tegra_vde 
*vde,
"\tFrame %d: frame_num = %d\n",
k + 1, frame->frame_num);
 
-   tegra_vde_setup_iram_entry(vde, 2, i, value, aux_addr);
+   tegra_vde_setup_iram_entry(vde, num_ref_pics, 2, i, value,
+  aux_addr);
}
 
for (k = 0; i < ref_frames_nb; i++, k++) {
@@ -326,7 +344,8 @@ static void tegra_vde_setup_iram_tables(struct tegra_vde 
*vde,
"\tFrame %d: frame_num = %d\n",
k + 1, frame->frame_num);
 
- 

[PATCH 00/14] staging: media: tegra-vdea: Add Tegra124 support

2018-08-13 Thread Thierry Reding
From: Thierry Reding 

Hi,

this set of patches perform a bit of cleanup and extend support to the
VDE implementation found on Tegra114 and Tegra124. This requires adding
handling for a clock and a reset for the BSEV block that is separate
from the main VDE block. The new VDE revision also supports reference
picture marking, which requires that the BSEV writes out some related
data to a memory location. Since the supported tiling layouts have been
changed in Tegra124, which supports only block-linear and no pitch-
linear layouts, a new way is added to request a specific layout for the
decoded frames. Both of the above changes require breaking the ABI to
accomodate for the new data in the custom IOCTL.

Finally this set also adds support for dealing with an IOMMU, which
makes it more convenient to deal with imported buffers since they no
longer need to be physically contiguous.

Userspace changes for the updated ABI are available here:

https://cgit.freedesktop.org/~tagr/libvdpau-tegra/commit/

Mauro, I'm sending the device tree changes as part of the series for
completeness, but I expect to pick those up into the Tegra tree once
this has been reviewed and you've applied the driver changes.

Thanks,
Thierry

Thierry Reding (14):
  staging: media: tegra-vde: Support BSEV clock and reset
  staging: media: tegra-vde: Support reference picture marking
  staging: media: tegra-vde: Prepare for interlacing support
  staging: media: tegra-vde: Use DRM/KMS framebuffer modifiers
  staging: media: tegra-vde: Properly mark invalid entries
  staging: media: tegra-vde: Print out invalid FD
  staging: media: tegra-vde: Add some clarifying comments
  staging: media: tegra-vde: Track struct device *
  staging: media: tegra-vde: Add IOMMU support
  staging: media: tegra-vde: Keep VDE in reset when unused
  ARM: tegra: Enable VDE on Tegra124
  ARM: tegra: Add BSEV clock and reset for VDE on Tegra20
  ARM: tegra: Add BSEV clock and reset for VDE on Tegra30
  ARM: tegra: Enable SMMU for VDE on Tegra124

 arch/arm/boot/dts/tegra124.dtsi |  42 ++
 arch/arm/boot/dts/tegra20.dtsi  |  10 +-
 arch/arm/boot/dts/tegra30.dtsi  |  10 +-
 drivers/staging/media/tegra-vde/tegra-vde.c | 528 +---
 drivers/staging/media/tegra-vde/uapi.h  |   6 +-
 5 files changed, 511 insertions(+), 85 deletions(-)

-- 
2.17.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 11/14] ARM: tegra: Enable VDE on Tegra124

2018-08-13 Thread Thierry Reding
From: Thierry Reding 

Signed-off-by: Thierry Reding 
---
 arch/arm/boot/dts/tegra124.dtsi | 40 +
 1 file changed, 40 insertions(+)

diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi
index b113e47b2b2a..8fdca4723205 100644
--- a/arch/arm/boot/dts/tegra124.dtsi
+++ b/arch/arm/boot/dts/tegra124.dtsi
@@ -83,6 +83,19 @@
};
};
 
+   iram@4000 {
+   compatible = "mmio-sram";
+   reg = <0x0 0x4000 0x0 0x4>;
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges = <0 0x0 0x4000 0x4>;
+
+   vde_pool: pool@400 {
+   reg = <0x400 0x3fc00>;
+   pool;
+   };
+   };
+
host1x@5000 {
compatible = "nvidia,tegra124-host1x", "simple-bus";
reg = <0x0 0x5000 0x0 0x00034000>;
@@ -283,6 +296,33 @@
*/
};
 
+   vde@6003 {
+   compatible = "nvidia,tegra124-vde", "nvidia,tegra30-vde",
+"nvidia,tegra20-vde";
+   reg = <0x0 0x6003 0x0 0x1000   /* Syntax Engine */
+  0x0 0x60031000 0x0 0x1000   /* Video Bitstream Engine */
+  0x0 0x60032000 0x0 0x0100   /* Macroblock Engine */
+  0x0 0x60032200 0x0 0x0100   /* Post-processing Engine */
+  0x0 0x60032400 0x0 0x0100   /* Motion Compensation 
Engine */
+  0x0 0x60032600 0x0 0x0100   /* Transform Engine */
+  0x0 0x60032800 0x0 0x0100   /* Pixel prediction block */
+  0x0 0x60032a00 0x0 0x0100   /* Video DMA */
+  0x0 0x60033800 0x0 0x0400>; /* Video frame controls */
+   reg-names = "sxe", "bsev", "mbe", "ppe", "mce",
+   "tfe", "ppb", "vdma", "frameid";
+   iram = <&vde_pool>; /* IRAM region */
+   interrupts = , /* Sync token 
interrupt */
+, /* BSE-V 
interrupt */
+; /* SXE interrupt 
*/
+   interrupt-names = "sync-token", "bsev", "sxe";
+   clocks = <&tegra_car TEGRA124_CLK_VDE>,
+<&tegra_car TEGRA124_CLK_BSEV>;
+   clock-names = "vde", "bsev";
+   resets = <&tegra_car 61>,
+<&tegra_car 63>;
+   reset-names = "vde", "bsev";
+   };
+
apbdma: dma@6002 {
compatible = "nvidia,tegra124-apbdma", "nvidia,tegra148-apbdma";
reg = <0x0 0x6002 0x0 0x1400>;
-- 
2.17.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 14/14] ARM: tegra: Enable SMMU for VDE on Tegra124

2018-08-13 Thread Thierry Reding
From: Thierry Reding 

The video decode engine can use the SMMU to use buffers that are not
physically contiguous in memory. This allows better memory usage for
video decoding, since fragmentation may cause contiguous allocations
to fail.

Signed-off-by: Thierry Reding 
---
 arch/arm/boot/dts/tegra124.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm/boot/dts/tegra124.dtsi b/arch/arm/boot/dts/tegra124.dtsi
index 8fdca4723205..0713e0ed5fef 100644
--- a/arch/arm/boot/dts/tegra124.dtsi
+++ b/arch/arm/boot/dts/tegra124.dtsi
@@ -321,6 +321,8 @@
resets = <&tegra_car 61>,
 <&tegra_car 63>;
reset-names = "vde", "bsev";
+
+   iommus = <&mc TEGRA_SWGROUP_VDE>;
};
 
apbdma: dma@6002 {
-- 
2.17.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 06/14] staging: media: tegra-vde: Print out invalid FD

2018-08-13 Thread Thierry Reding
From: Thierry Reding 

Include the invalid file descriptor when reporting an error message to
help diagnosing why importing the buffer failed.

Signed-off-by: Thierry Reding 
---
 drivers/staging/media/tegra-vde/tegra-vde.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c 
b/drivers/staging/media/tegra-vde/tegra-vde.c
index 0ce30c7ccb75..0adc603fa437 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -643,7 +643,7 @@ static int tegra_vde_attach_dmabuf(struct device *dev,
 
dmabuf = dma_buf_get(fd);
if (IS_ERR(dmabuf)) {
-   dev_err(dev, "Invalid dmabuf FD\n");
+   dev_err(dev, "Invalid dmabuf FD: %d\n", fd);
return PTR_ERR(dmabuf);
}
 
-- 
2.17.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 12/14] ARM: tegra: Add BSEV clock and reset for VDE on Tegra20

2018-08-13 Thread Thierry Reding
From: Thierry Reding 

Signed-off-by: Thierry Reding 
---
 arch/arm/boot/dts/tegra20.dtsi | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 15b73bd377f0..abb5738a0705 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -287,9 +287,13 @@
 , /* BSE-V 
interrupt */
 ; /* SXE interrupt 
*/
interrupt-names = "sync-token", "bsev", "sxe";
-   clocks = <&tegra_car TEGRA20_CLK_VDE>;
-   reset-names = "vde", "mc";
-   resets = <&tegra_car 61>, <&mc TEGRA20_MC_RESET_VDE>;
+   clocks = <&tegra_car TEGRA20_CLK_VDE>,
+<&tegra_car TEGRA20_CLK_BSEV>;
+   clock-names = "vde", "bsev";
+   resets = <&tegra_car 61>,
+<&tegra_car 63>,
+<&mc TEGRA20_MC_RESET_VDE>;
+   reset-names = "vde", "bsev", "mc";
};
 
apbmisc@7800 {
-- 
2.17.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 07/14] staging: media: tegra-vde: Add some clarifying comments

2018-08-13 Thread Thierry Reding
From: Thierry Reding 

Add some comments specifying what tables are being set up in VRAM.

Signed-off-by: Thierry Reding 
---
 drivers/staging/media/tegra-vde/tegra-vde.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c 
b/drivers/staging/media/tegra-vde/tegra-vde.c
index 0adc603fa437..41cf86dc5dbd 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -271,6 +271,7 @@ static void tegra_vde_setup_iram_tables(struct tegra_vde 
*vde,
unsigned int i, k;
size_t size;
 
+   /* clear H256RefPicList */
size = num_ref_pics * 4 * 8;
memset(vde->iram, 0, size);
 
@@ -453,6 +454,7 @@ static int tegra_vde_setup_hw_context(struct tegra_vde *vde,
VDE_WR(0x, vde->bsev + 0x98);
VDE_WR(0x0060, vde->bsev + 0x9C);
 
+   /* clear H264MB2SliceGroupMap, assuming no FMO */
memset(vde->iram + 1024, 0, macroblocks_nb / 2);
 
tegra_setup_frameidx(vde, dpb_frames, ctx->dpb_frames_nb,
@@ -480,6 +482,8 @@ static int tegra_vde_setup_hw_context(struct tegra_vde *vde,
if (err)
return err;
 
+   /* upload H264MB2SliceGroupMap */
+   /* XXX don't hardcode map size? */
value = (0x20 << 26) | (0 << 25) | ((4096 >> 2) & 0x1fff);
err = tegra_vde_push_to_bsev_icmdqueue(vde, value, false);
if (err)
@@ -492,6 +496,7 @@ static int tegra_vde_setup_hw_context(struct tegra_vde *vde,
if (err)
return err;
 
+   /* clear H264MBInfo XXX don't hardcode size */
value = (0x21 << 26) | ((240 & 0x1fff) << 12) | (0x54c & 0xfff);
err = tegra_vde_push_to_bsev_icmdqueue(vde, 0x840F054C, false);
if (err)
@@ -499,6 +504,16 @@ static int tegra_vde_setup_hw_context(struct tegra_vde 
*vde,
 
size = num_ref_pics * 4 * 8;
 
+   /* clear H264RefPicList */
+   /*
+   value = (0x21 << 26) | (((size >> 2) & 0x1fff) << 12) | 0xE34;
+
+   err = tegra_vde_push_to_bsev_icmdqueue(vde, value, false);
+   if (err)
+   return err;
+   */
+
+   /* upload H264RefPicList */
value = (0x20 << 26) | (0x0 << 25) | ((size >> 2) & 0x1fff);
err = tegra_vde_push_to_bsev_icmdqueue(vde, value, false);
if (err)
@@ -584,7 +599,11 @@ static int tegra_vde_setup_hw_context(struct tegra_vde 
*vde,
 
tegra_vde_mbe_set_0xa_reg(vde, 0, 0x09FC);
tegra_vde_mbe_set_0xa_reg(vde, 2, 0x61DEAD00);
+#if 0
+   tegra_vde_mbe_set_0xa_reg(vde, 4, dpb_frames[0].aux_addr); /* 
0x62DEAD00 */
+#else
tegra_vde_mbe_set_0xa_reg(vde, 4, 0x62DEAD00);
+#endif
tegra_vde_mbe_set_0xa_reg(vde, 6, 0x63DEAD00);
tegra_vde_mbe_set_0xa_reg(vde, 8, dpb_frames[0].aux_addr);
 
-- 
2.17.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 10/14] staging: media: tegra-vde: Keep VDE in reset when unused

2018-08-13 Thread Thierry Reding
From: Thierry Reding 

There is no point in keeping the VDE module out of reset when it is not
in use. Reset it on runtime suspend.

Signed-off-by: Thierry Reding 
---
 drivers/staging/media/tegra-vde/tegra-vde.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c 
b/drivers/staging/media/tegra-vde/tegra-vde.c
index 3bc0bfcfe34e..4b3c6ab3c77e 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -1226,6 +1226,7 @@ static int tegra_vde_runtime_suspend(struct device *dev)
}
 
reset_control_assert(vde->rst_bsev);
+   reset_control_assert(vde->rst);
 
usleep_range(2000, 4000);
 
-- 
2.17.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 04/14] staging: media: tegra-vde: Use DRM/KMS framebuffer modifiers

2018-08-13 Thread Thierry Reding
From: Thierry Reding 

VDE on Tegra20 through Tegra114 supports reading and writing frames in
16x16 tiled layout. Similarily, the various block-linear layouts that
are supported by the GPU on Tegra124 can also be read from and written
to by the Tegra124 VDE.

Enable userspace to specify the desired layout using the existing DRM
framebuffer modifiers.

Signed-off-by: Thierry Reding 
---
 drivers/staging/media/tegra-vde/tegra-vde.c | 112 +---
 drivers/staging/media/tegra-vde/uapi.h  |   3 +-
 2 files changed, 100 insertions(+), 15 deletions(-)

diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c 
b/drivers/staging/media/tegra-vde/tegra-vde.c
index 1a40f6dff7c8..275884e745df 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -24,6 +24,8 @@
 
 #include 
 
+#include 
+
 #include "uapi.h"
 
 #define ICMDQUE_WR 0x00
@@ -58,12 +60,14 @@ struct video_frame {
dma_addr_t aux_addr;
u32 frame_num;
u32 flags;
+   u64 modifier;
 };
 
 struct tegra_vde_soc {
unsigned int num_ref_pics;
bool supports_ref_pic_marking;
bool supports_interlacing;
+   bool supports_block_linear;
 };
 
 struct tegra_vde {
@@ -202,6 +206,7 @@ static void tegra_vde_setup_frameid(struct tegra_vde *vde,
unsigned int frameid,
u32 mbs_width, u32 mbs_height)
 {
+   u64 modifier = frame ? frame->modifier : DRM_FORMAT_MOD_LINEAR;
u32 y_addr  = frame ? frame->y_addr  : 0x6CDEAD00;
u32 cb_addr = frame ? frame->cb_addr : 0x6CDEAD00;
u32 cr_addr = frame ? frame->cr_addr : 0x6CDEAD00;
@@ -209,8 +214,12 @@ static void tegra_vde_setup_frameid(struct tegra_vde *vde,
u32 value2 = frame ? mbs_width + 1) >> 1) << 6) | 1) : 0;
u32 value = y_addr >> 8;
 
-   if (vde->soc->supports_interlacing)
+   if (!vde->soc->supports_interlacing) {
+   if (modifier == DRM_FORMAT_MOD_NVIDIA_TEGRA_TILED)
+   value |= BIT(31);
+   } else {
value |= BIT(31);
+   }
 
VDE_WR(value,vde->frameid + 0x000 + frameid * 4);
VDE_WR(cb_addr >> 8, vde->frameid + 0x100 + frameid * 4);
@@ -349,6 +358,37 @@ static void tegra_vde_setup_iram_tables(struct tegra_vde 
*vde,
}
 }
 
+static int tegra_vde_get_block_height(u64 modifier, unsigned int *block_height)
+{
+   switch (modifier) {
+   case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_ONE_GOB:
+   *block_height = 0;
+   return 0;
+
+   case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_TWO_GOB:
+   *block_height = 1;
+   return 0;
+
+   case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_FOUR_GOB:
+   *block_height = 2;
+   return 0;
+
+   case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_EIGHT_GOB:
+   *block_height = 3;
+   return 0;
+
+   case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_SIXTEEN_GOB:
+   *block_height = 4;
+   return 0;
+
+   case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_THIRTYTWO_GOB:
+   *block_height = 5;
+   return 0;
+   }
+
+   return -EINVAL;
+}
+
 static int tegra_vde_setup_hw_context(struct tegra_vde *vde,
  struct tegra_vde_h264_decoder_ctx *ctx,
  struct video_frame *dpb_frames,
@@ -383,7 +423,21 @@ static int tegra_vde_setup_hw_context(struct tegra_vde 
*vde,
tegra_vde_set_bits(vde, 0x0005, vde->vdma + 0x04);
 
VDE_WR(0x, vde->vdma + 0x1C);
-   VDE_WR(0x, vde->vdma + 0x00);
+
+   value = 0x;
+
+   if (vde->soc->supports_block_linear) {
+   unsigned int block_height;
+
+   err = tegra_vde_get_block_height(dpb_frames[0].modifier,
+&block_height);
+   if (err < 0)
+   return err;
+
+   value |= block_height << 10;
+   }
+
+   VDE_WR(value, vde->vdma + 0x00);
VDE_WR(0x0007, vde->vdma + 0x04);
VDE_WR(0x0007, vde->frameid + 0x200);
VDE_WR(0x0005, vde->tfe + 0x04);
@@ -730,11 +784,37 @@ static void tegra_vde_release_frame_dmabufs(struct 
video_frame *frame,
 static int tegra_vde_validate_frame(struct device *dev,
struct tegra_vde_h264_frame *frame)
 {
+   struct tegra_vde *vde = dev_get_drvdata(dev);
+
if (frame->frame_num > 0x7F) {
dev_err(dev, "Bad frame_num %u\n", frame->frame_num);
return -EINVAL;
}
 
+   if (vde->soc->supports_block_linear) {
+   switch (frame->modifier) {
+   case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_ONE_GOB:
+   case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_TWO_GOB:
+   case DRM_FORMAT_MOD_NVIDIA_16BX2_BLOCK_FOUR_GO

[PATCH 01/14] staging: media: tegra-vde: Support BSEV clock and reset

2018-08-13 Thread Thierry Reding
From: Thierry Reding 

The BSEV clock has a separate gate bit and can not be assumed to be
always enabled. Add explicit handling for the BSEV clock and reset.

This fixes an issue on Tegra124 where the BSEV clock is not enabled
by default and therefore accessing the BSEV registers will hang the
CPU if the BSEV clock is not enabled and the reset not deasserted.

Signed-off-by: Thierry Reding 
---
 drivers/staging/media/tegra-vde/tegra-vde.c | 35 +++--
 1 file changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c 
b/drivers/staging/media/tegra-vde/tegra-vde.c
index 6f06061a40d9..9d8f833744db 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -74,9 +74,11 @@ struct tegra_vde {
struct miscdevice miscdev;
struct reset_control *rst;
struct reset_control *rst_mc;
+   struct reset_control *rst_bsev;
struct gen_pool *iram_pool;
struct completion decode_completion;
struct clk *clk;
+   struct clk *clk_bsev;
dma_addr_t iram_lists_addr;
u32 *iram;
 };
@@ -979,6 +981,11 @@ static int tegra_vde_runtime_suspend(struct device *dev)
return err;
}
 
+   reset_control_assert(vde->rst_bsev);
+
+   usleep_range(2000, 4000);
+
+   clk_disable_unprepare(vde->clk_bsev);
clk_disable_unprepare(vde->clk);
 
return 0;
@@ -996,6 +1003,16 @@ static int tegra_vde_runtime_resume(struct device *dev)
return err;
}
 
+   err = clk_prepare_enable(vde->clk_bsev);
+   if (err < 0)
+   return err;
+
+   err = reset_control_deassert(vde->rst_bsev);
+   if (err < 0)
+   return err;
+
+   usleep_range(2000, 4000);
+
return 0;
 }
 
@@ -1084,14 +1101,21 @@ static int tegra_vde_probe(struct platform_device *pdev)
if (IS_ERR(vde->frameid))
return PTR_ERR(vde->frameid);
 
-   vde->clk = devm_clk_get(dev, NULL);
+   vde->clk = devm_clk_get(dev, "vde");
if (IS_ERR(vde->clk)) {
err = PTR_ERR(vde->clk);
dev_err(dev, "Could not get VDE clk %d\n", err);
return err;
}
 
-   vde->rst = devm_reset_control_get(dev, NULL);
+   vde->clk_bsev = devm_clk_get(dev, "bsev");
+   if (IS_ERR(vde->clk_bsev)) {
+   err = PTR_ERR(vde->clk_bsev);
+   dev_err(dev, "failed to get BSEV clock: %d\n", err);
+   return err;
+   }
+
+   vde->rst = devm_reset_control_get(dev, "vde");
if (IS_ERR(vde->rst)) {
err = PTR_ERR(vde->rst);
dev_err(dev, "Could not get VDE reset %d\n", err);
@@ -1105,6 +1129,13 @@ static int tegra_vde_probe(struct platform_device *pdev)
return err;
}
 
+   vde->rst_bsev = devm_reset_control_get(dev, "bsev");
+   if (IS_ERR(vde->rst_bsev)) {
+   err = PTR_ERR(vde->rst_bsev);
+   dev_err(dev, "failed to get BSEV reset: %d\n", err);
+   return err;
+   }
+
irq = platform_get_irq_byname(pdev, "sync-token");
if (irq < 0)
return irq;
-- 
2.17.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 13/14] ARM: tegra: Add BSEV clock and reset for VDE on Tegra30

2018-08-13 Thread Thierry Reding
From: Thierry Reding 

Signed-off-by: Thierry Reding 
---
 arch/arm/boot/dts/tegra30.dtsi | 10 +++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index a6781f653310..492917d61bab 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -408,9 +408,13 @@
 , /* BSE-V 
interrupt */
 ; /* SXE interrupt 
*/
interrupt-names = "sync-token", "bsev", "sxe";
-   clocks = <&tegra_car TEGRA30_CLK_VDE>;
-   reset-names = "vde", "mc";
-   resets = <&tegra_car 61>, <&mc TEGRA30_MC_RESET_VDE>;
+   clocks = <&tegra_car TEGRA30_CLK_VDE>,
+<&tegra_car TEGRA30_CLK_BSEV>;
+   clock-names = "vde", "bsev";
+   resets = <&tegra_car 61>,
+<&tegra_car 63>,
+<&mc TEGRA30_MC_RESET_VDE>;
+   reset-names = "vde", "bsev", "mc";
};
 
apbmisc@7800 {
-- 
2.17.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 05/14] staging: media: tegra-vde: Properly mark invalid entries

2018-08-13 Thread Thierry Reding
From: Thierry Reding 

Entries in the reference picture list are marked as invalid by setting
the frame ID to 0x3f.

Signed-off-by: Thierry Reding 
---
 drivers/staging/media/tegra-vde/tegra-vde.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c 
b/drivers/staging/media/tegra-vde/tegra-vde.c
index 275884e745df..0ce30c7ccb75 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -296,7 +296,7 @@ static void tegra_vde_setup_iram_tables(struct tegra_vde 
*vde,
(frame->flags & FLAG_B_FRAME));
} else {
aux_addr = 0x6ADEAD00;
-   value = 0;
+   value = 0x3f;
}
 
tegra_vde_setup_iram_entry(vde, num_ref_pics, 0, i, value,
-- 
2.17.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 08/14] staging: media: tegra-vde: Track struct device *

2018-08-13 Thread Thierry Reding
From: Thierry Reding 

The pointer to the struct device is frequently used, so store it in
struct tegra_vde. Also, pass around a pointer to a struct tegra_vde
instead of struct device in some cases to prepare for subsequent
patches referencing additional data from that structure.

Signed-off-by: Thierry Reding 
---
 drivers/staging/media/tegra-vde/tegra-vde.c | 63 -
 1 file changed, 36 insertions(+), 27 deletions(-)

diff --git a/drivers/staging/media/tegra-vde/tegra-vde.c 
b/drivers/staging/media/tegra-vde/tegra-vde.c
index 41cf86dc5dbd..2496a03fd158 100644
--- a/drivers/staging/media/tegra-vde/tegra-vde.c
+++ b/drivers/staging/media/tegra-vde/tegra-vde.c
@@ -71,6 +71,7 @@ struct tegra_vde_soc {
 };
 
 struct tegra_vde {
+   struct device *dev;
const struct tegra_vde_soc *soc;
void __iomem *sxe;
void __iomem *bsev;
@@ -644,7 +645,7 @@ static void tegra_vde_detach_and_put_dmabuf(struct 
dma_buf_attachment *a,
dma_buf_put(dmabuf);
 }
 
-static int tegra_vde_attach_dmabuf(struct device *dev,
+static int tegra_vde_attach_dmabuf(struct tegra_vde *vde,
   int fd,
   unsigned long offset,
   size_t min_size,
@@ -662,38 +663,40 @@ static int tegra_vde_attach_dmabuf(struct device *dev,
 
dmabuf = dma_buf_get(fd);
if (IS_ERR(dmabuf)) {
-   dev_err(dev, "Invalid dmabuf FD: %d\n", fd);
+   dev_err(vde->dev, "Invalid dmabuf FD: %d\n", fd);
return PTR_ERR(dmabuf);
}
 
if (dmabuf->size & (align_size - 1)) {
-   dev_err(dev, "Unaligned dmabuf 0x%zX, should be aligned to 
0x%zX\n",
+   dev_err(vde->dev,
+   "Unaligned dmabuf 0x%zX, should be aligned to 0x%zX\n",
dmabuf->size, align_size);
return -EINVAL;
}
 
if ((u64)offset + min_size > dmabuf->size) {
-   dev_err(dev, "Too small dmabuf size %zu @0x%lX, should be at 
least %zu\n",
+   dev_err(vde->dev,
+   "Too small dmabuf size %zu @0x%lX, should be at least 
%zu\n",
dmabuf->size, offset, min_size);
return -EINVAL;
}
 
-   attachment = dma_buf_attach(dmabuf, dev);
+   attachment = dma_buf_attach(dmabuf, vde->dev);
if (IS_ERR(attachment)) {
-   dev_err(dev, "Failed to attach dmabuf\n");
+   dev_err(vde->dev, "Failed to attach dmabuf\n");
err = PTR_ERR(attachment);
goto err_put;
}
 
sgt = dma_buf_map_attachment(attachment, dma_dir);
if (IS_ERR(sgt)) {
-   dev_err(dev, "Failed to get dmabufs sg_table\n");
+   dev_err(vde->dev, "Failed to get dmabufs sg_table\n");
err = PTR_ERR(sgt);
goto err_detach;
}
 
if (sgt->nents != 1) {
-   dev_err(dev, "Sparse DMA region is unsupported\n");
+   dev_err(vde->dev, "Sparse DMA region is unsupported\n");
err = -EINVAL;
goto err_unmap;
}
@@ -717,7 +720,7 @@ static int tegra_vde_attach_dmabuf(struct device *dev,
return err;
 }
 
-static int tegra_vde_attach_dmabufs_to_frame(struct device *dev,
+static int tegra_vde_attach_dmabufs_to_frame(struct tegra_vde *vde,
 struct video_frame *frame,
 struct tegra_vde_h264_frame *src,
 enum dma_data_direction dma_dir,
@@ -726,7 +729,7 @@ static int tegra_vde_attach_dmabufs_to_frame(struct device 
*dev,
 {
int err;
 
-   err = tegra_vde_attach_dmabuf(dev, src->y_fd,
+   err = tegra_vde_attach_dmabuf(vde, src->y_fd,
  src->y_offset, lsize, SZ_256,
  &frame->y_dmabuf_attachment,
  &frame->y_addr,
@@ -735,7 +738,7 @@ static int tegra_vde_attach_dmabufs_to_frame(struct device 
*dev,
if (err)
return err;
 
-   err = tegra_vde_attach_dmabuf(dev, src->cb_fd,
+   err = tegra_vde_attach_dmabuf(vde, src->cb_fd,
  src->cb_offset, csize, SZ_256,
  &frame->cb_dmabuf_attachment,
  &frame->cb_addr,
@@ -744,7 +747,7 @@ static int tegra_vde_attach_dmabufs_to_frame(struct device 
*dev,
if (err)
goto err_release_y;
 
-   err = tegra_vde_attach_dmabuf(dev, src->cr_fd,
+   err = tegra_vde_attach_dmabuf(vde, src->cr_fd,
  src->cr_offset, csize, SZ_256,
  &frame->cr_dmabuf_attachment,
  &frame->cr_addr,
@@ -758,7 +761,7 @@ static int tegra_vde_attach_dmabu

дюгд

2018-08-13 Thread жрп
взр тднй ттос


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


еюда

2018-08-13 Thread тпх
фцф йиаэ егея


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 3/8] staging: erofs: add error handling for xattr submodule

2018-08-13 Thread Dan Carpenter
> But it is better to fix them in an independent patch. :)

Yah.  Of course.  This was completely unrelated.

regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 3/8] staging: erofs: add error handling for xattr submodule

2018-08-13 Thread Gao Xiang
Hi Dan,

On 2018/8/13 20:25, Dan Carpenter wrote:
> On Mon, Aug 13, 2018 at 08:17:27PM +0800, Gao Xiang wrote:
>>> /* we assume that ofs is aligned with 4 bytes */
>>> it->ofs = EROFS_XATTR_ALIGN(it->ofs);
>>> return err;
>>>
> This might be cleaner if we wrote:
> 
>   return (err < 0) ? error : 0;
> 
> The callers all treate zero and one the same so there isn't any reason
> to propogate the 1 back.
> 

Thanks, I will recheck all callers and fix as you suggested.
But it is better to fix them in an independent patch. :) I will send a new 
patch later.

Thanks,
Gao Xiang

> regards,
> dan carpenter
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 4/8] staging: erofs: cleanup z_erofs_vle_work_{lookup, register}

2018-08-13 Thread Gao Xiang
Hi Dan,

On 2018/8/13 21:05, Dan Carpenter wrote:
> Yeah.  You'd have to remove the const.
> 
> Anyway, on looking at it more, I guess this patch is fine for now.  We
> will probably end up changing z_erofs_vle_work_lookup() and
> z_erofs_vle_work_register() some more in the future.
> 

Thanks for review, I personally tend to leave this patch unmodified for now :( 
.

The struct is wrote in order to read-only or assign and read at once, and let 
compilers notice that
(all modifications are local so compilers can handle them safely)...

Thanks,
Gao Xiang

> regards,
> dan carpenter
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 1/2] staging: most: register channel device after init of struct members

2018-08-13 Thread Christian Gromm
This patch moves the call to device_register to the end of the channel
initialization section that the devcie belongs to. It is needed to
avoid NULL pointer dereferences once the device is registered with
sysfs.

Signed-off-by: Christian Gromm 
---
 drivers/staging/most/core.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/most/core.c b/drivers/staging/most/core.c
index f4c4646..b59e471 100644
--- a/drivers/staging/most/core.c
+++ b/drivers/staging/most/core.c
@@ -1439,10 +1439,6 @@ int most_register_interface(struct most_interface *iface)
c->dev.parent = &iface->dev;
c->dev.groups = channel_attr_groups;
c->dev.release = release_channel;
-   if (device_register(&c->dev)) {
-   pr_err("registering c->dev failed\n");
-   goto free_instance_nodev;
-   }
iface->p->channel[i] = c;
c->is_starving = 0;
c->iface = iface;
@@ -1465,6 +1461,10 @@ int most_register_interface(struct most_interface *iface)
mutex_init(&c->start_mutex);
mutex_init(&c->nq_mutex);
list_add_tail(&c->list, &iface->p->channel_list);
+   if (device_register(&c->dev)) {
+   pr_err("registering c->dev failed\n");
+   goto free_instance_nodev;
+   }
}
pr_info("registered new device mdev%d (%s)\n",
id, iface->description);
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH] staging: erofs: add int to usigned

2018-08-13 Thread Gao Xiang
Hi Leon,

On 2018/8/13 20:09, Leon Imhof wrote:
> Fix coding style issue "Prefer 'unsigned int' to bare use of 'unsigned'"
> detected by checkpatch.pl
> 
> Signed-off-by: Leon Imhof 

Looks fine,

Reviewed-by: Gao Xiang 

(BTW, could you fix the title of this commit? Thanks.)

> ---
>  drivers/staging/erofs/data.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/erofs/data.c b/drivers/staging/erofs/data.c
> index ac263a180253..2a401216007a 100644
> --- a/drivers/staging/erofs/data.c
> +++ b/drivers/staging/erofs/data.c
> @@ -337,7 +337,7 @@ static int erofs_raw_access_readpage(struct file *file, 
> struct page *page)
>  
>  static int erofs_raw_access_readpages(struct file *filp,
>   struct address_space *mapping,
> - struct list_head *pages, unsigned nr_pages)
> + struct list_head *pages, unsigned int nr_pages)
>  {
>   erofs_off_t last_block;
>   struct bio *bio = NULL;
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 4/8] staging: erofs: cleanup z_erofs_vle_work_{lookup, register}

2018-08-13 Thread Dan Carpenter
Yeah.  You'd have to remove the const.

Anyway, on looking at it more, I guess this patch is fine for now.  We
will probably end up changing z_erofs_vle_work_lookup() and
z_erofs_vle_work_register() some more in the future.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 2/2] staging: most: do not make interface dependent attrs default for all channels

2018-08-13 Thread Christian Gromm
The channel attribute dbr_size is only relevant for the DIM2 interface. so
is the packets_per_xact for USB. This patch cleans up the driver's ABI by
not showing all attributes by default for each channel, but only on those
they belong to.

Signed-off-by: Christian Gromm 
---
 drivers/staging/most/core.c | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/drivers/staging/most/core.c b/drivers/staging/most/core.c
index b59e471..dabe180 100644
--- a/drivers/staging/most/core.c
+++ b/drivers/staging/most/core.c
@@ -442,6 +442,24 @@ static ssize_t set_dbr_size_store(struct device *dev,
return count;
 }
 
+#define to_dev_attr(a) container_of(a, struct device_attribute, attr)
+static umode_t channel_attr_is_visible(struct kobject *kobj,
+  struct attribute *attr, int index)
+{
+   struct device_attribute *dev_attr = to_dev_attr(attr);
+   struct device *dev = kobj_to_dev(kobj);
+   struct most_channel *c = to_channel(dev);
+
+   if (!strcmp(dev_attr->attr.name, "set_dbr_size") &&
+   (c->iface->interface != ITYPE_MEDIALB_DIM2))
+   return 0;
+   if (!strcmp(dev_attr->attr.name, "set_packets_per_xact") &&
+   (c->iface->interface != ITYPE_USB))
+   return 0;
+
+   return attr->mode;
+}
+
 #define DEV_ATTR(_name)  (&dev_attr_##_name.attr)
 
 static DEVICE_ATTR_RO(available_directions);
@@ -479,6 +497,7 @@ static struct attribute *channel_attrs[] = {
 
 static struct attribute_group channel_attr_group = {
.attrs = channel_attrs,
+   .is_visible = channel_attr_is_visible,
 };
 
 static const struct attribute_group *channel_attr_groups[] = {
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH 0/2] staging: most: clean up sysfs interface

2018-08-13 Thread Christian Gromm
This patch set cleans up the driver's ABI. It takes care of proper
initialization sequences and prevents interface dependent channel
attributes from being displayed by default on all interfaces.


Christian Gromm (2):
  staging: most: register channel device after init of struct members
  staging: most: do not make interface dependent attrs default for all
channels

 drivers/staging/most/core.c | 27 +++
 1 file changed, 23 insertions(+), 4 deletions(-)

-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 6/8] staging: erofs: fix vle_decompressed_index_clusterofs

2018-08-13 Thread Gao Xiang
Hi Dan,

On 2018/8/13 20:03, Dan Carpenter wrote:
>> -static inline unsigned
>> -vle_compressed_index_clusterofs(unsigned clustersize,
>> -struct z_erofs_vle_decompressed_index *di)
>> +static inline int
>> +vle_decompressed_index_clusterofs(unsigned int *clusterofs,
> 
> Not related to your patch, but don't make functions inline.  Leave it to
> the compiler to decide.

OK, thanks for your suggestion.

Thanks,
Gao Xiang

> 
> regards,
> dan carpenter
> 
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 3/8] staging: erofs: add error handling for xattr submodule

2018-08-13 Thread Gao Xiang
Hi Dan,

On 2018/8/13 20:40, Dan Carpenter wrote:
> On Mon, Aug 13, 2018 at 08:17:27PM +0800, Gao Xiang wrote:
 @@ -294,8 +322,11 @@ static int inline_getxattr(struct inode *inode, 
 struct getxattr_iter *it)
ret = xattr_foreach(&it->it, &find_xattr_handlers, &remaining);
if (ret >= 0)
break;
 +
 +  if (unlikely(ret != -ENOATTR))  /* -ENOMEM, -EIO, etc. */
>>> I have held off commenting on all the likely/unlikely annotations we
>>> are adding because I don't know what the fast paths are in this code.
>>> However, this is clearly an error path here, not on a fast path.
>>>
>>> Generally the rule on likely/unlikely is that they hurt readability so
>>> we should only add them if it makes a difference in benchmarking.
>>>
>> In my opinion, return values other than 0 and ENOATTR(ENODATA) rarely 
>> happens,
>> it should be in the slow path...
>>
> What I'm trying to say is please stop adding so many likely/unlikely
> annotations.  You should only add them if you have the benchmark data to
> show the it really is required.
> 
> 
OK, I'll follow your suggestion.

I could say it is my personal code tendency(style).
If you don't like it/think them useless, I will remove them all. 

Thanks for your suggestion.

Thanks,
Gao Xiang

> regards,
> dan carpenter
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 3/8] staging: erofs: add error handling for xattr submodule

2018-08-13 Thread Chao Yu
On 2018/8/13 20:17, Gao Xiang wrote:
>> Generally the rule on likely/unlikely is that they hurt readability so
>> we should only add them if it makes a difference in benchmarking.
>>
> 
> In my opinion, return values other than 0 and ENOATTR(ENODATA) rarely happens,
> it should be in the slow path...

Hi Dan, thanks for the comments.

IMO, we should check and clean up all likely/unlikely in erofs, to make sure
they are used in the right place.

Thanks,
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 3/8] staging: erofs: add error handling for xattr submodule

2018-08-13 Thread Dan Carpenter
On Mon, Aug 13, 2018 at 08:17:27PM +0800, Gao Xiang wrote:
> >> @@ -294,8 +322,11 @@ static int inline_getxattr(struct inode *inode, 
> >> struct getxattr_iter *it)
> >>ret = xattr_foreach(&it->it, &find_xattr_handlers, &remaining);
> >>if (ret >= 0)
> >>break;
> >> +
> >> +  if (unlikely(ret != -ENOATTR))  /* -ENOMEM, -EIO, etc. */
> > 
> > I have held off commenting on all the likely/unlikely annotations we
> > are adding because I don't know what the fast paths are in this code.
> > However, this is clearly an error path here, not on a fast path.
> > 
> > Generally the rule on likely/unlikely is that they hurt readability so
> > we should only add them if it makes a difference in benchmarking.
> > 
> 
> In my opinion, return values other than 0 and ENOATTR(ENODATA) rarely happens,
> it should be in the slow path...
> 

What I'm trying to say is please stop adding so many likely/unlikely
annotations.  You should only add them if you have the benchmark data to
show the it really is required.


regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 4/8] staging: erofs: cleanup z_erofs_vle_work_{lookup, register}

2018-08-13 Thread Gao Xiang
Hi Dan,

On 2018/8/13 20:00, Dan Carpenter wrote:
> On Sun, Aug 12, 2018 at 10:01:46PM +0800, Chao Yu wrote:
>> From: Gao Xiang 
>>
>> This patch introduces 'struct z_erofs_vle_work_finder' to clean up
>> arguments of z_erofs_vle_work_lookup and z_erofs_vle_work_register.
>>
>> Signed-off-by: Gao Xiang 
>> Reviewed-by: Chao Yu 
>> Signed-off-by: Chao Yu 
>> ---
>>  drivers/staging/erofs/unzip_vle.c | 89 ---
>>  1 file changed, 47 insertions(+), 42 deletions(-)
>>
>> diff --git a/drivers/staging/erofs/unzip_vle.c 
>> b/drivers/staging/erofs/unzip_vle.c
>> index b2e05e2b4116..5032b3b05de1 100644
>> --- a/drivers/staging/erofs/unzip_vle.c
>> +++ b/drivers/staging/erofs/unzip_vle.c
>> @@ -271,36 +271,42 @@ static inline bool try_to_claim_workgroup(
>>  return true;/* lucky, I am the followee :) */
>>  }
>>  
>> +struct z_erofs_vle_work_finder {
>> +struct super_block *sb;
>> +pgoff_t idx;
>> +unsigned pageofs;
>> +
>> +struct z_erofs_vle_workgroup **grp_ret;
>> +enum z_erofs_vle_work_role *role;
>> +z_erofs_vle_owned_workgrp_t *owned_head;
>> +bool *hosted;
>> +};
>> +
>>  static struct z_erofs_vle_work *
>> -z_erofs_vle_work_lookup(struct super_block *sb,
>> -pgoff_t idx, unsigned pageofs,
>> -struct z_erofs_vle_workgroup **grp_ret,
>> -enum z_erofs_vle_work_role *role,
>> -z_erofs_vle_owned_workgrp_t *owned_head,
>> -bool *hosted)
>> +z_erofs_vle_work_lookup(const struct z_erofs_vle_work_finder *f)
>>  {
>>  bool tag, primary;
>>  struct erofs_workgroup *egrp;
>>  struct z_erofs_vle_workgroup *grp;
>>  struct z_erofs_vle_work *work;
>>  
>> -egrp = erofs_find_workgroup(sb, idx, &tag);
>> +egrp = erofs_find_workgroup(f->sb, f->idx, &tag);
>>  if (egrp == NULL) {
>> -*grp_ret = NULL;
>> +*f->grp_ret = NULL;
> 
> All these pointers to pointer seem a bit messy.  Just do this:
> 
>   struct z_erofs_vle_workgroup *grp;
> 
> Then replace "grp" in z_erofs_vle_work_iter_begin() with finder.grp;
> 

I wrote this because I am not sure of all compiler behaviors.

Notice that the struct `struct z_erofs_vle_work_finder' has been all marked as 
const.

If I use `struct z_erofs_vle_workgroup *grp;' and drop the `const' decorator,
compilers could do some re-read operations bacause its value could potentially 
change by its caller at the same time.

Thanks,
Gao Xiang

> regards,
> dan carpenter
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 2/8] staging: erofs: separate erofs_get_meta_page

2018-08-13 Thread Chao Yu
On 2018/8/13 19:04, Dan Carpenter wrote:
> On Sun, Aug 12, 2018 at 10:01:44PM +0800, Chao Yu wrote:
>> --- a/drivers/staging/erofs/data.c
>> +++ b/drivers/staging/erofs/data.c
>> @@ -39,31 +39,44 @@ static inline void read_endio(struct bio *bio)
>>  }
>>  
>>  /* prio -- true is used for dir */
>> -struct page *erofs_get_meta_page(struct super_block *sb,
>> -erofs_blk_t blkaddr, bool prio)
>> +struct page *__erofs_get_meta_page(struct super_block *sb,
>> +erofs_blk_t blkaddr, bool prio, bool nofail)
>>  {
>> -struct inode *bd_inode = sb->s_bdev->bd_inode;
>> -struct address_space *mapping = bd_inode->i_mapping;
>> +struct inode *const bd_inode = sb->s_bdev->bd_inode;
>> +struct address_space *const mapping = bd_inode->i_mapping;
>> +/* prefer retrying in the allocator to blindly looping below */
>> +const gfp_t gfp = mapping_gfp_constraint(mapping, ~__GFP_FS) |
>> +(nofail ? __GFP_NOFAIL : 0);
>> +unsigned int io_retries = nofail ? EROFS_IO_MAX_RETRIES_NOFAIL : 0;
>>  struct page *page;
>>  
>>  repeat:
>> -page = find_or_create_page(mapping, blkaddr,
>> -/*
>> - * Prefer looping in the allocator rather than here,
>> - * at least that code knows what it's doing.
>> - */
>> -mapping_gfp_constraint(mapping, ~__GFP_FS) | __GFP_NOFAIL);
>> -
>> -BUG_ON(!page || !PageLocked(page));
>> +page = find_or_create_page(mapping, blkaddr, gfp);
>> +if (unlikely(page == NULL)) {
>> +DBG_BUGON(nofail);
>> +return ERR_PTR(-ENOMEM);
>> +}
>> +DBG_BUGON(!PageLocked(page));
>>  
>>  if (!PageUptodate(page)) {
>>  struct bio *bio;
>>  int err;
>>  
>> -bio = erofs_grab_bio(sb, blkaddr, 1, read_endio, true);
>> +bio = erofs_grab_bio(sb, blkaddr, 1, read_endio, nofail);
>> +if (unlikely(bio == NULL)) {
>> +DBG_BUGON(nofail);
>> +err = -ENOMEM;
>> +err_out:
>> +unlock_page(page);
>> +put_page(page);
>> +return ERR_PTR(err);
> 
> 
> Put this err_out stuff at the bottom of the function so that we don't
> have to do backward hops to get to it.

Agreed, we can move error path at the bottom of this function.

> 
>> +}
>>  
>>  err = bio_add_page(bio, page, PAGE_SIZE, 0);
>> -BUG_ON(err != PAGE_SIZE);
>> +if (unlikely(err != PAGE_SIZE)) {
>> +err = -EFAULT;
>> +goto err_out;
> 
> Like this.  Generally avoid backwards hops if you can.
> 
>> +}
>>  
>>  __submit_bio(bio, REQ_OP_READ,
>>  REQ_META | (prio ? REQ_PRIO : 0));
>> @@ -72,6 +85,7 @@ struct page *erofs_get_meta_page(struct super_block *sb,
>>  
>>  /* the page has been truncated by others? */
>>  if (unlikely(page->mapping != mapping)) {
>> +unlock_repeat:
> 
> The question mark in the "truncated by others?" is a little concerning.

Yup, we can remove the question mark.

> It's slightly weird that we don't check "io_retries" on this path.

We don't need to cover this path since io_retries is used for accounting retry
time only when IO occurs.

Thanks,

> 
>>  unlock_page(page);
>>  put_page(page);
>>  goto repeat;
>> @@ -79,10 +93,12 @@ struct page *erofs_get_meta_page(struct super_block *sb,
>>  
>>  /* more likely a read error */
>>  if (unlikely(!PageUptodate(page))) {
>> -unlock_page(page);
>> -put_page(page);
>> -
>> -page = ERR_PTR(-EIO);
>> +if (io_retries) {
>> +--io_retries;
>> +goto unlock_repeat;
>> +}
>> +err = -EIO;
>> +goto err_out;
> 
> regards,
> dan carpenter
> 
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 3/8] staging: erofs: add error handling for xattr submodule

2018-08-13 Thread Dan Carpenter
On Mon, Aug 13, 2018 at 08:17:27PM +0800, Gao Xiang wrote:
> > /* we assume that ofs is aligned with 4 bytes */
> > it->ofs = EROFS_XATTR_ALIGN(it->ofs);
> > return err;
> > 

This might be cleaner if we wrote:

return (err < 0) ? error : 0;

The callers all treate zero and one the same so there isn't any reason
to propogate the 1 back.

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 3/8] staging: erofs: add error handling for xattr submodule

2018-08-13 Thread Gao Xiang
Hi Dan,

On 2018/8/13 19:47, Dan Carpenter wrote:
>> -static void xattr_iter_fixup(struct xattr_iter *it)
>> +static inline int xattr_iter_fixup(struct xattr_iter *it)
>>  {
>> -if (unlikely(it->ofs >= EROFS_BLKSIZ)) {
>> -xattr_iter_end(it, true);
>> +if (likely(it->ofs < EROFS_BLKSIZ))
>> +return 0;
>>  
>> -it->blkaddr += erofs_blknr(it->ofs);
>> -it->page = erofs_get_meta_page_nofail(it->sb,
>> -it->blkaddr, false);
>> -BUG_ON(IS_ERR(it->page));
>> +xattr_iter_end(it, true);
>>  
>> -it->kaddr = kmap_atomic(it->page);
>> -it->ofs = erofs_blkoff(it->ofs);
>> +it->blkaddr += erofs_blknr(it->ofs);
>> +
>> +it->page = erofs_get_meta_page(it->sb, it->blkaddr, false);
>> +if (IS_ERR(it->page)) {
>> +it->page = NULL;
>> +return PTR_ERR(it->page);
> 
> This is returning PTR_ERR(NULL) which is success.  There is a smatch
> test for this and maybe other static checkers as well so it would have
> been caught very quickly.
> 

I am sorry about this. It has already fixed in patch v2.

>>  }
>> +
>> +it->kaddr = kmap_atomic(it->page);
>> +it->ofs = erofs_blkoff(it->ofs);
>> +return 0;
>>  }
>>  
>>  static int inline_xattr_iter_begin(struct xattr_iter *it,
>> @@ -134,22 +154,25 @@ static int inline_xattr_iter_begin(struct xattr_iter 
>> *it,
>>  it->blkaddr = erofs_blknr(iloc(sbi, vi->nid) + inline_xattr_ofs);
>>  it->ofs = erofs_blkoff(iloc(sbi, vi->nid) + inline_xattr_ofs);
>>  
>> -it->page = erofs_get_inline_page_nofail(inode, it->blkaddr);
>> -BUG_ON(IS_ERR(it->page));
>> -it->kaddr = kmap_atomic(it->page);
>> +it->page = erofs_get_inline_page(inode, it->blkaddr);
>> +if (IS_ERR(it->page))
>> +return PTR_ERR(it->page);
>>  
>> +it->kaddr = kmap_atomic(it->page);
>>  return vi->xattr_isize - xattr_header_sz;
>>  }
>>  
>>  static int xattr_foreach(struct xattr_iter *it,
>> -struct xattr_iter_handlers *op, unsigned *tlimit)
>> +const struct xattr_iter_handlers *op, unsigned *tlimit)
>>  {
>>  struct erofs_xattr_entry entry;
>>  unsigned value_sz, processed, slice;
>>  int err;
>>  
>>  /* 0. fixup blkaddr, ofs, ipage */
>> -xattr_iter_fixup(it);
>> +err = xattr_iter_fixup(it);
>> +if (unlikely(err))
>> +return err;
>>  
>>  /*
>>   * 1. read xattr entry to the memory,
>> @@ -181,7 +204,9 @@ static int xattr_foreach(struct xattr_iter *it,
>>  if (it->ofs >= EROFS_BLKSIZ) {
>>  BUG_ON(it->ofs > EROFS_BLKSIZ);
>>  
>> -xattr_iter_fixup(it);
>> +err = xattr_iter_fixup(it);
>> +if (unlikely(err))
>> +goto out;
>>  it->ofs = 0;
>>  }
>>  
>> @@ -213,7 +238,10 @@ static int xattr_foreach(struct xattr_iter *it,
>>  while (processed < value_sz) {
>>  if (it->ofs >= EROFS_BLKSIZ) {
>>  BUG_ON(it->ofs > EROFS_BLKSIZ);
>> -xattr_iter_fixup(it);
>> +
>> +err = xattr_iter_fixup(it);
>> +if (unlikely(err))
>> +goto out;
>>  it->ofs = 0;
>>  }
>>  
>> @@ -273,7 +301,7 @@ static void xattr_copyvalue(struct xattr_iter *_it,
>>  memcpy(it->buffer + processed, buf, len);
>>  }
>>  
>> -static struct xattr_iter_handlers find_xattr_handlers = {
>> +static const struct xattr_iter_handlers find_xattr_handlers = {
>>  .entry = xattr_entrymatch,
>>  .name = xattr_namematch,
>>  .alloc_buffer = xattr_checkbuffer,
>> @@ -294,8 +322,11 @@ static int inline_getxattr(struct inode *inode, struct 
>> getxattr_iter *it)
>>  ret = xattr_foreach(&it->it, &find_xattr_handlers, &remaining);
>>  if (ret >= 0)
>>  break;
>> +
>> +if (unlikely(ret != -ENOATTR))  /* -ENOMEM, -EIO, etc. */
> 
> I have held off commenting on all the likely/unlikely annotations we
> are adding because I don't know what the fast paths are in this code.
> However, this is clearly an error path here, not on a fast path.
> 
> Generally the rule on likely/unlikely is that they hurt readability so
> we should only add them if it makes a difference in benchmarking.
> 

In my opinion, return values other than 0 and ENOATTR(ENODATA) rarely happens,
it should be in the slow path...

>> +break;
>>  }
>> -xattr_iter_end(&it->it, true);
>> +xattr_iter_end_final(&it->it);
>>  
>>  return ret < 0 ? ret : it->buffer_size;
>>  }
>> @@ -318,9 +349,10 @@ static int shared_getxattr(struct inode *inode, struct 
>> getxattr_iter *it)
>>  if (i)
>>  xattr_iter_end(&it->it, true);
>>  
>> -it->it.page = erofs_get_meta_page_nofail(sb,
>> -  

[PATCH] staging: erofs: add int to usigned

2018-08-13 Thread Leon Imhof
Fix coding style issue "Prefer 'unsigned int' to bare use of 'unsigned'"
detected by checkpatch.pl

Signed-off-by: Leon Imhof 
---
 drivers/staging/erofs/data.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/erofs/data.c b/drivers/staging/erofs/data.c
index ac263a180253..2a401216007a 100644
--- a/drivers/staging/erofs/data.c
+++ b/drivers/staging/erofs/data.c
@@ -337,7 +337,7 @@ static int erofs_raw_access_readpage(struct file *file, 
struct page *page)
 
 static int erofs_raw_access_readpages(struct file *filp,
struct address_space *mapping,
-   struct list_head *pages, unsigned nr_pages)
+   struct list_head *pages, unsigned int nr_pages)
 {
erofs_off_t last_block;
struct bio *bio = NULL;
-- 
2.18.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 6/8] staging: erofs: fix vle_decompressed_index_clusterofs

2018-08-13 Thread Dan Carpenter
> -static inline unsigned
> -vle_compressed_index_clusterofs(unsigned clustersize,
> - struct z_erofs_vle_decompressed_index *di)
> +static inline int
> +vle_decompressed_index_clusterofs(unsigned int *clusterofs,

Not related to your patch, but don't make functions inline.  Leave it to
the compiler to decide.

regards,
dan carpenter


___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 4/8] staging: erofs: cleanup z_erofs_vle_work_{lookup, register}

2018-08-13 Thread Dan Carpenter
On Sun, Aug 12, 2018 at 10:01:46PM +0800, Chao Yu wrote:
> From: Gao Xiang 
> 
> This patch introduces 'struct z_erofs_vle_work_finder' to clean up
> arguments of z_erofs_vle_work_lookup and z_erofs_vle_work_register.
> 
> Signed-off-by: Gao Xiang 
> Reviewed-by: Chao Yu 
> Signed-off-by: Chao Yu 
> ---
>  drivers/staging/erofs/unzip_vle.c | 89 ---
>  1 file changed, 47 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/staging/erofs/unzip_vle.c 
> b/drivers/staging/erofs/unzip_vle.c
> index b2e05e2b4116..5032b3b05de1 100644
> --- a/drivers/staging/erofs/unzip_vle.c
> +++ b/drivers/staging/erofs/unzip_vle.c
> @@ -271,36 +271,42 @@ static inline bool try_to_claim_workgroup(
>   return true;/* lucky, I am the followee :) */
>  }
>  
> +struct z_erofs_vle_work_finder {
> + struct super_block *sb;
> + pgoff_t idx;
> + unsigned pageofs;
> +
> + struct z_erofs_vle_workgroup **grp_ret;
> + enum z_erofs_vle_work_role *role;
> + z_erofs_vle_owned_workgrp_t *owned_head;
> + bool *hosted;
> +};
> +
>  static struct z_erofs_vle_work *
> -z_erofs_vle_work_lookup(struct super_block *sb,
> - pgoff_t idx, unsigned pageofs,
> - struct z_erofs_vle_workgroup **grp_ret,
> - enum z_erofs_vle_work_role *role,
> - z_erofs_vle_owned_workgrp_t *owned_head,
> - bool *hosted)
> +z_erofs_vle_work_lookup(const struct z_erofs_vle_work_finder *f)
>  {
>   bool tag, primary;
>   struct erofs_workgroup *egrp;
>   struct z_erofs_vle_workgroup *grp;
>   struct z_erofs_vle_work *work;
>  
> - egrp = erofs_find_workgroup(sb, idx, &tag);
> + egrp = erofs_find_workgroup(f->sb, f->idx, &tag);
>   if (egrp == NULL) {
> - *grp_ret = NULL;
> + *f->grp_ret = NULL;

All these pointers to pointer seem a bit messy.  Just do this:

struct z_erofs_vle_workgroup *grp;

Then replace "grp" in z_erofs_vle_work_iter_begin() with finder.grp;

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 3/8] staging: erofs: add error handling for xattr submodule

2018-08-13 Thread Dan Carpenter
> -static void xattr_iter_fixup(struct xattr_iter *it)
> +static inline int xattr_iter_fixup(struct xattr_iter *it)
>  {
> - if (unlikely(it->ofs >= EROFS_BLKSIZ)) {
> - xattr_iter_end(it, true);
> + if (likely(it->ofs < EROFS_BLKSIZ))
> + return 0;
>  
> - it->blkaddr += erofs_blknr(it->ofs);
> - it->page = erofs_get_meta_page_nofail(it->sb,
> - it->blkaddr, false);
> - BUG_ON(IS_ERR(it->page));
> + xattr_iter_end(it, true);
>  
> - it->kaddr = kmap_atomic(it->page);
> - it->ofs = erofs_blkoff(it->ofs);
> + it->blkaddr += erofs_blknr(it->ofs);
> +
> + it->page = erofs_get_meta_page(it->sb, it->blkaddr, false);
> + if (IS_ERR(it->page)) {
> + it->page = NULL;
> + return PTR_ERR(it->page);

This is returning PTR_ERR(NULL) which is success.  There is a smatch
test for this and maybe other static checkers as well so it would have
been caught very quickly.

>   }
> +
> + it->kaddr = kmap_atomic(it->page);
> + it->ofs = erofs_blkoff(it->ofs);
> + return 0;
>  }
>  
>  static int inline_xattr_iter_begin(struct xattr_iter *it,
> @@ -134,22 +154,25 @@ static int inline_xattr_iter_begin(struct xattr_iter 
> *it,
>   it->blkaddr = erofs_blknr(iloc(sbi, vi->nid) + inline_xattr_ofs);
>   it->ofs = erofs_blkoff(iloc(sbi, vi->nid) + inline_xattr_ofs);
>  
> - it->page = erofs_get_inline_page_nofail(inode, it->blkaddr);
> - BUG_ON(IS_ERR(it->page));
> - it->kaddr = kmap_atomic(it->page);
> + it->page = erofs_get_inline_page(inode, it->blkaddr);
> + if (IS_ERR(it->page))
> + return PTR_ERR(it->page);
>  
> + it->kaddr = kmap_atomic(it->page);
>   return vi->xattr_isize - xattr_header_sz;
>  }
>  
>  static int xattr_foreach(struct xattr_iter *it,
> - struct xattr_iter_handlers *op, unsigned *tlimit)
> + const struct xattr_iter_handlers *op, unsigned *tlimit)
>  {
>   struct erofs_xattr_entry entry;
>   unsigned value_sz, processed, slice;
>   int err;
>  
>   /* 0. fixup blkaddr, ofs, ipage */
> - xattr_iter_fixup(it);
> + err = xattr_iter_fixup(it);
> + if (unlikely(err))
> + return err;
>  
>   /*
>* 1. read xattr entry to the memory,
> @@ -181,7 +204,9 @@ static int xattr_foreach(struct xattr_iter *it,
>   if (it->ofs >= EROFS_BLKSIZ) {
>   BUG_ON(it->ofs > EROFS_BLKSIZ);
>  
> - xattr_iter_fixup(it);
> + err = xattr_iter_fixup(it);
> + if (unlikely(err))
> + goto out;
>   it->ofs = 0;
>   }
>  
> @@ -213,7 +238,10 @@ static int xattr_foreach(struct xattr_iter *it,
>   while (processed < value_sz) {
>   if (it->ofs >= EROFS_BLKSIZ) {
>   BUG_ON(it->ofs > EROFS_BLKSIZ);
> - xattr_iter_fixup(it);
> +
> + err = xattr_iter_fixup(it);
> + if (unlikely(err))
> + goto out;
>   it->ofs = 0;
>   }
>  
> @@ -273,7 +301,7 @@ static void xattr_copyvalue(struct xattr_iter *_it,
>   memcpy(it->buffer + processed, buf, len);
>  }
>  
> -static struct xattr_iter_handlers find_xattr_handlers = {
> +static const struct xattr_iter_handlers find_xattr_handlers = {
>   .entry = xattr_entrymatch,
>   .name = xattr_namematch,
>   .alloc_buffer = xattr_checkbuffer,
> @@ -294,8 +322,11 @@ static int inline_getxattr(struct inode *inode, struct 
> getxattr_iter *it)
>   ret = xattr_foreach(&it->it, &find_xattr_handlers, &remaining);
>   if (ret >= 0)
>   break;
> +
> + if (unlikely(ret != -ENOATTR))  /* -ENOMEM, -EIO, etc. */

I have held off commenting on all the likely/unlikely annotations we
are adding because I don't know what the fast paths are in this code.
However, this is clearly an error path here, not on a fast path.

Generally the rule on likely/unlikely is that they hurt readability so
we should only add them if it makes a difference in benchmarking.

> + break;
>   }
> - xattr_iter_end(&it->it, true);
> + xattr_iter_end_final(&it->it);
>  
>   return ret < 0 ? ret : it->buffer_size;
>  }
> @@ -318,9 +349,10 @@ static int shared_getxattr(struct inode *inode, struct 
> getxattr_iter *it)
>   if (i)
>   xattr_iter_end(&it->it, true);
>  
> - it->it.page = erofs_get_meta_page_nofail(sb,
> - blkaddr, false);
> - BUG_ON(IS_ERR(it->it.page));
> + it->it.page = erofs_get_meta_page(sb, blkaddr, false);
> + if (IS_ERR(it->it.page))
> + return PTR_ERR(it->it.page);
> +
>  

Re: [PATCH 2/8] staging: erofs: separate erofs_get_meta_page

2018-08-13 Thread Gao Xiang
Hi Dan,

On 2018/8/13 19:04, Dan Carpenter wrote:
> On Sun, Aug 12, 2018 at 10:01:44PM +0800, Chao Yu wrote:
>> --- a/drivers/staging/erofs/data.c
>> +++ b/drivers/staging/erofs/data.c
>> @@ -39,31 +39,44 @@ static inline void read_endio(struct bio *bio)
>>  }
>>  
>>  /* prio -- true is used for dir */
>> -struct page *erofs_get_meta_page(struct super_block *sb,
>> -erofs_blk_t blkaddr, bool prio)
>> +struct page *__erofs_get_meta_page(struct super_block *sb,
>> +erofs_blk_t blkaddr, bool prio, bool nofail)
>>  {
>> -struct inode *bd_inode = sb->s_bdev->bd_inode;
>> -struct address_space *mapping = bd_inode->i_mapping;
>> +struct inode *const bd_inode = sb->s_bdev->bd_inode;
>> +struct address_space *const mapping = bd_inode->i_mapping;
>> +/* prefer retrying in the allocator to blindly looping below */
>> +const gfp_t gfp = mapping_gfp_constraint(mapping, ~__GFP_FS) |
>> +(nofail ? __GFP_NOFAIL : 0);
>> +unsigned int io_retries = nofail ? EROFS_IO_MAX_RETRIES_NOFAIL : 0;
>>  struct page *page;
>>  
>>  repeat:
>> -page = find_or_create_page(mapping, blkaddr,
>> -/*
>> - * Prefer looping in the allocator rather than here,
>> - * at least that code knows what it's doing.
>> - */
>> -mapping_gfp_constraint(mapping, ~__GFP_FS) | __GFP_NOFAIL);
>> -
>> -BUG_ON(!page || !PageLocked(page));
>> +page = find_or_create_page(mapping, blkaddr, gfp);
>> +if (unlikely(page == NULL)) {
>> +DBG_BUGON(nofail);
>> +return ERR_PTR(-ENOMEM);
>> +}
>> +DBG_BUGON(!PageLocked(page));
>>  
>>  if (!PageUptodate(page)) {
>>  struct bio *bio;
>>  int err;
>>  
>> -bio = erofs_grab_bio(sb, blkaddr, 1, read_endio, true);
>> +bio = erofs_grab_bio(sb, blkaddr, 1, read_endio, nofail);
>> +if (unlikely(bio == NULL)) {
>> +DBG_BUGON(nofail);
>> +err = -ENOMEM;
>> +err_out:
>> +unlock_page(page);
>> +put_page(page);
>> +return ERR_PTR(err);
> 
> Put this err_out stuff at the bottom of the function so that we don't
> have to do backward hops to get to it.
> 
>> +}
>>  
>>  err = bio_add_page(bio, page, PAGE_SIZE, 0);
>> -BUG_ON(err != PAGE_SIZE);
>> +if (unlikely(err != PAGE_SIZE)) {
>> +err = -EFAULT;
>> +goto err_out;
> 
> Like this.  Generally avoid backwards hops if you can.
> 
>> +}
>>  
>>  __submit_bio(bio, REQ_OP_READ,
>>  REQ_META | (prio ? REQ_PRIO : 0));
>> @@ -72,6 +85,7 @@ struct page *erofs_get_meta_page(struct super_block *sb,
>>  
>>  /* the page has been truncated by others? */
>>  if (unlikely(page->mapping != mapping)) {
>> +unlock_repeat:
> The question mark in the "truncated by others?" is a little concerning.
> It's slightly weird that we don't check "io_retries" on this path.
>
Thanks for your reply.

erofs use the shared block device bd_inode->mapping for its meta use, which is 
visible
to other code rather than erofs internally only, so I think it needs to get rid 
of
potentially truncating by others, and such comments can also be found at

- pagecache_get_page
1557 /* Has the page been truncated? */
1558 if (unlikely(page->mapping != mapping)) {
1559 unlock_page(page);
1560 put_page(page);
1561 goto repeat;
1562 }

- filemap_fault
2535 /* Did it get truncated? */
2536 if (unlikely(page->mapping != mapping)) {
2537 unlock_page(page);
2538 put_page(page);
2539 goto retry_find;
2540 }

I think it is somewhat necessary, and some suggestion about this? thanks in 
advance.

"It's slightly weird that we don't check "io_retries" on this path"
I think you are right, let me rethink about it and send the next patch...

Thanks,
Gao Xiang

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 2/8] staging: erofs: separate erofs_get_meta_page

2018-08-13 Thread Dan Carpenter
On Sun, Aug 12, 2018 at 10:01:44PM +0800, Chao Yu wrote:
> --- a/drivers/staging/erofs/data.c
> +++ b/drivers/staging/erofs/data.c
> @@ -39,31 +39,44 @@ static inline void read_endio(struct bio *bio)
>  }
>  
>  /* prio -- true is used for dir */
> -struct page *erofs_get_meta_page(struct super_block *sb,
> - erofs_blk_t blkaddr, bool prio)
> +struct page *__erofs_get_meta_page(struct super_block *sb,
> + erofs_blk_t blkaddr, bool prio, bool nofail)
>  {
> - struct inode *bd_inode = sb->s_bdev->bd_inode;
> - struct address_space *mapping = bd_inode->i_mapping;
> + struct inode *const bd_inode = sb->s_bdev->bd_inode;
> + struct address_space *const mapping = bd_inode->i_mapping;
> + /* prefer retrying in the allocator to blindly looping below */
> + const gfp_t gfp = mapping_gfp_constraint(mapping, ~__GFP_FS) |
> + (nofail ? __GFP_NOFAIL : 0);
> + unsigned int io_retries = nofail ? EROFS_IO_MAX_RETRIES_NOFAIL : 0;
>   struct page *page;
>  
>  repeat:
> - page = find_or_create_page(mapping, blkaddr,
> - /*
> -  * Prefer looping in the allocator rather than here,
> -  * at least that code knows what it's doing.
> -  */
> - mapping_gfp_constraint(mapping, ~__GFP_FS) | __GFP_NOFAIL);
> -
> - BUG_ON(!page || !PageLocked(page));
> + page = find_or_create_page(mapping, blkaddr, gfp);
> + if (unlikely(page == NULL)) {
> + DBG_BUGON(nofail);
> + return ERR_PTR(-ENOMEM);
> + }
> + DBG_BUGON(!PageLocked(page));
>  
>   if (!PageUptodate(page)) {
>   struct bio *bio;
>   int err;
>  
> - bio = erofs_grab_bio(sb, blkaddr, 1, read_endio, true);
> + bio = erofs_grab_bio(sb, blkaddr, 1, read_endio, nofail);
> + if (unlikely(bio == NULL)) {
> + DBG_BUGON(nofail);
> + err = -ENOMEM;
> +err_out:
> + unlock_page(page);
> + put_page(page);
> + return ERR_PTR(err);


Put this err_out stuff at the bottom of the function so that we don't
have to do backward hops to get to it.

> + }
>  
>   err = bio_add_page(bio, page, PAGE_SIZE, 0);
> - BUG_ON(err != PAGE_SIZE);
> + if (unlikely(err != PAGE_SIZE)) {
> + err = -EFAULT;
> + goto err_out;

Like this.  Generally avoid backwards hops if you can.

> + }
>  
>   __submit_bio(bio, REQ_OP_READ,
>   REQ_META | (prio ? REQ_PRIO : 0));
> @@ -72,6 +85,7 @@ struct page *erofs_get_meta_page(struct super_block *sb,
>  
>   /* the page has been truncated by others? */
>   if (unlikely(page->mapping != mapping)) {
> +unlock_repeat:

The question mark in the "truncated by others?" is a little concerning.
It's slightly weird that we don't check "io_retries" on this path.

>   unlock_page(page);
>   put_page(page);
>   goto repeat;
> @@ -79,10 +93,12 @@ struct page *erofs_get_meta_page(struct super_block *sb,
>  
>   /* more likely a read error */
>   if (unlikely(!PageUptodate(page))) {
> - unlock_page(page);
> - put_page(page);
> -
> - page = ERR_PTR(-EIO);
> + if (io_retries) {
> + --io_retries;
> + goto unlock_repeat;
> + }
> + err = -EIO;
> + goto err_out;

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


mobile apps for you

2018-08-13 Thread Jack Dike

Do you need mobile apps development? We can do it for you.

We are an India base company. Here are the details about us:
Years in business: 8
Staffs: 125
App developed: 350

We work on Android, iOS, Ionic, and PhoneGap platforms, we have clients
across different kind of industries.
Such like retail, media and entertainment, BFSI, hospitality, social media,
eCommerce, food and beverages, etc.

We do below:
Mobile Apps
Mobile App UI/UX designing
App Maintenance and Support
Website or ecommerce portal development

Please reply back if you are interested in what we do.
We will share our portfolios to you.

Regards,
Jack

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 3/8] staging: erofs: add error handling for xattr submodule

2018-08-13 Thread Chao Yu
Hi Xiang,

On 2018/8/13 10:36, Gao Xiang wrote:
> Hi Chao,
> 
> On 2018/8/13 10:00, Chao Yu wrote:
>> On 2018/8/12 22:01, Chao Yu wrote:
>>> From: Gao Xiang 
>>>
>>> This patch enhances the missing error handling code for
>>> xattr submodule, which improves the stability for the rare cases.
>>>
>>> Signed-off-by: Gao Xiang 
>>> Reviewed-by: Chao Yu 
>>> Signed-off-by: Chao Yu 
>>> ---
>>>  drivers/staging/erofs/internal.h |   6 +-
>>>  drivers/staging/erofs/xattr.c| 120 ---
>>>  2 files changed, 83 insertions(+), 43 deletions(-)
>>>
>>> diff --git a/drivers/staging/erofs/internal.h 
>>> b/drivers/staging/erofs/internal.h
>>> index a756abeff7e9..8951e01216e3 100644
>>> --- a/drivers/staging/erofs/internal.h
>>> +++ b/drivers/staging/erofs/internal.h
>>> @@ -485,10 +485,10 @@ struct erofs_map_blocks_iter {
>>>  
>>>  
>>>  static inline struct page *
>>> -erofs_get_inline_page_nofail(struct inode *inode,
>>> -erofs_blk_t blkaddr)
>>> +erofs_get_inline_page(struct inode *inode,
>>> + erofs_blk_t blkaddr)
>>>  {
>>> -   return erofs_get_meta_page_nofail(inode->i_sb,
>>> +   return erofs_get_meta_page(inode->i_sb,
>>> blkaddr, S_ISDIR(inode->i_mode));
>>>  }
>>>  
>>> diff --git a/drivers/staging/erofs/xattr.c b/drivers/staging/erofs/xattr.c
>>> index 2593c856b079..1b5815fc70db 100644
>>> --- a/drivers/staging/erofs/xattr.c
>>> +++ b/drivers/staging/erofs/xattr.c
>>> @@ -24,16 +24,25 @@ struct xattr_iter {
>>>  
>>>  static inline void xattr_iter_end(struct xattr_iter *it, bool atomic)
>>>  {
>>> -   /* only init_inode_xattrs use non-atomic once */
>>> +   /* the only user of kunmap() is 'init_inode_xattrs' */
>>> if (unlikely(!atomic))
>>> kunmap(it->page);
>>> else
>>> kunmap_atomic(it->kaddr);
>>> +
>>> unlock_page(it->page);
>>> put_page(it->page);
>>>  }
>>>  
>>> -static void init_inode_xattrs(struct inode *inode)
>>> +static inline void xattr_iter_end_final(struct xattr_iter *it)
>>> +{
>>> +   if (unlikely(it->page == NULL))
>>> +   return;
>>> +
>>> +   xattr_iter_end(it, true);
>>> +}
>>> +
>>> +static int init_inode_xattrs(struct inode *inode)
>>>  {
>>> struct xattr_iter it;
>>> unsigned i;
>>> @@ -44,7 +53,7 @@ static void init_inode_xattrs(struct inode *inode)
>>> bool atomic_map;
>>>  
>>> if (likely(inode_has_inited_xattr(inode)))
>>> -   return;
>>> +   return 0;
>>>  
>>> vi = EROFS_V(inode);
>>> BUG_ON(!vi->xattr_isize);
>>> @@ -54,8 +63,9 @@ static void init_inode_xattrs(struct inode *inode)
>>> it.blkaddr = erofs_blknr(iloc(sbi, vi->nid) + vi->inode_isize);
>>> it.ofs = erofs_blkoff(iloc(sbi, vi->nid) + vi->inode_isize);
>>>  
>>> -   it.page = erofs_get_inline_page_nofail(inode, it.blkaddr);
>>> -   BUG_ON(IS_ERR(it.page));
>>> +   it.page = erofs_get_inline_page(inode, it.blkaddr);
>>> +   if (IS_ERR(it.page))
>>> +   return PTR_ERR(it.page);
>>>  
>>> /* read in shared xattr array (non-atomic, see kmalloc below) */
>>> it.kaddr = kmap(it.page);
>>> @@ -64,9 +74,12 @@ static void init_inode_xattrs(struct inode *inode)
>>> ih = (struct erofs_xattr_ibody_header *)(it.kaddr + it.ofs);
>>>  
>>> vi->xattr_shared_count = ih->h_shared_count;
>>> -   vi->xattr_shared_xattrs = (unsigned *)kmalloc_array(
>>> -   vi->xattr_shared_count, sizeof(unsigned),
>>> -   GFP_KERNEL | __GFP_NOFAIL);
>>> +   vi->xattr_shared_xattrs = kmalloc_array(vi->xattr_shared_count,
>>> +   sizeof(unsigned), GFP_KERNEL);
>>> +   if (unlikely(vi->xattr_shared_xattrs == NULL)) {
>>> +   xattr_iter_end(&it, atomic_map);
>>> +   return -ENOMEM;
>>> +   }
>>>  
>>> /* let's skip ibody header */
>>> it.ofs += sizeof(struct erofs_xattr_ibody_header);
>>> @@ -77,9 +90,10 @@ static void init_inode_xattrs(struct inode *inode)
>>> BUG_ON(it.ofs != EROFS_BLKSIZ);
>>> xattr_iter_end(&it, atomic_map);
>>>  
>>> -   it.page = erofs_get_meta_page_nofail(sb,
>>> +   it.page = erofs_get_meta_page(sb,
>>> ++it.blkaddr, S_ISDIR(inode->i_mode));
>>> -   BUG_ON(IS_ERR(it.page));
>>> +   if (IS_ERR(it.page))
>>> +   return PTR_ERR(it.page);
>>>  
>>> it.kaddr = kmap_atomic(it.page);
>>> atomic_map = true;
>>> @@ -92,6 +106,7 @@ static void init_inode_xattrs(struct inode *inode)
>>> xattr_iter_end(&it, atomic_map);
>>>  
>>> inode_set_inited_xattr(inode);
>>> +   return 0;
>>>  }
>>>  
>>>  struct xattr_iter_handlers {
>>> @@ -101,19 +116,24 @@ struct xattr_iter_handlers {
>>> void (*value)(struct xattr_iter *, unsigned, char *, unsigned);
>>>  };
>>>  
>>> -static void xattr_iter_fixup(struct xattr_iter *it)
>>> +static inline int xattr_iter_fixup(struct xattr_

the mobile apps

2018-08-13 Thread Jack Dike

Do you need mobile apps development? We can do it for you.

We are an India base company. Here are the details about us:
Years in business: 8
Staffs: 125
App developed: 350

We work on Android, iOS, Ionic, and PhoneGap platforms, we have clients
across different kind of industries.
Such like retail, media and entertainment, BFSI, hospitality, social media,
eCommerce, food and beverages, etc.

We do below:
Mobile Apps
Mobile App UI/UX designing
App Maintenance and Support
Website or ecommerce portal development

Please reply back if you are interested in what we do.
We will share our portfolios to you.

Regards,
Jack

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 00/16] staging: gasket: return of the son of cleanups

2018-08-13 Thread Christoph Hellwig
Btw, can someone explain what 'gasket' even is?  A quick look through
the directory didn't provide any answers.  And please don't just send
that anser to me, put it in the source tree (Kconfig, and if needed
another documentation file).
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel