Hi Thirumalai,
On 01/12/2012 02:51 PM, Thirumalai Pachamuthu wrote:
> * A new APSD power save queue is added in the station structure.
> * When a station has APSD capability and goes to power save, the frame
> designated to the station will be buffered in APSD queue.
> * When the host receives a frame which the firmware marked as trigger,
> host delivers the buffered frame from the APSD power save queue.
> Number of frames to deliver is decided by MAX SP length.
> * When a station moves from sleep to awake state, all frames buffered
> in APSD power save queue are sent to the firmware.
> * When a station is disconnected, all frames bufferes in APSD power save
> queue are dropped.
> * When the host queues the first frame to the APSD queue or removes the
> last frame from the APSD queue, it is indicated to the firmware using
> WMI_AP_APSD_BUFFERED_TRAFFIC_CMD.
>
> Signed-off-by: Thirumalai Pachamuthu <tpachamu@xxxxxxxxxxxxxxxx>
Thanks, I applied your patch with these changes below. Please check that
I didn't break anything.
> +static bool ath6kl_process_uapsdq(struct ath6kl_sta *conn,
> + struct ath6kl_vif *vif,
> + struct sk_buff *skb,
> + u32 *flags)
> +{
> + struct ath6kl *ar = vif->ar;
> + bool is_apsdq_empty = false;
> + struct ethhdr *datap = (struct ethhdr *) skb->data;
> + u8 up = 0;
This is initilised in an else branch I added below.
> + u8 traffic_class;
> + u16 ether_type;
> + u8 *ip_hdr;
> + struct ath6kl_llc_snap_hdr *llc_hdr;
I combined the same type variable declarations here.
> +
> + if (conn->sta_flags & STA_PS_APSD_TRIGGER) {
> + /*
> + * This tx is because of a uAPSD trigger, determine
> + * more and EOSP bit. Set EOSP if queue is empty
> + * or sufficient frames are delivered for this trigger.
> + */
> + spin_lock_bh(&conn->psq_lock);
> + if (!skb_queue_empty(&conn->apsdq))
> + *flags |= WMI_DATA_HDR_FLAGS_MORE;
> + else if (conn->sta_flags & STA_PS_APSD_EOSP)
> + *flags |= WMI_DATA_HDR_FLAGS_EOSP;
> + *flags |= WMI_DATA_HDR_FLAGS_UAPSD;
> + spin_unlock_bh(&conn->psq_lock);
> + return false;
> + } else if (!conn->apsd_info)
> + return false;
> +
> + if (test_bit(WMM_ENABLED, &vif->flags)) {
> + ether_type = be16_to_cpu(datap->h_proto);
> + if (is_ethertype(ether_type)) {
> + /* packet is in DIX format */
> + ip_hdr = (u8 *)(datap + 1);
> + } else {
> + /* packet is in 802.3 format */
> + llc_hdr = (struct ath6kl_llc_snap_hdr *)
> + (datap + 1);
> + ether_type = be16_to_cpu(llc_hdr->eth_type);
> + ip_hdr = (u8 *)(llc_hdr + 1);
> + }
> +
> + if (ether_type == IP_ETHERTYPE)
> + up = ath6kl_wmi_determine_user_priority(
> + ip_hdr, 0);
> + }
I added the else branch here.
> +static bool ath6kl_process_psq(struct ath6kl_sta *conn,
> + struct ath6kl_vif *vif,
> + struct sk_buff *skb,
> + u32 *flags)
> +{
> + bool is_psq_empty = false;
> + struct ath6kl *ar = vif->ar;
> +
> + if (conn->sta_flags & STA_PS_POLLED) {
> + spin_lock_bh(&conn->psq_lock);
> + if (!skb_queue_empty(&conn->psq))
> + *flags |= WMI_DATA_HDR_FLAGS_MORE;
> + spin_unlock_bh(&conn->psq_lock);
> + return false;
> + } else {
> + /* Queue the frames if the STA is sleeping */
The else block is not needed as there's a return in the if block above.
So the code below can be taken out of the else block.
> +static void ath6kl_uapsd_trigger_frame_rx(struct ath6kl_vif *vif,
> + struct ath6kl_sta *conn)
> +{
> + struct ath6kl *ar = vif->ar;
> + bool is_apsdq_empty;
> + bool is_apsdq_empty_at_start;
> + u32 num_frames_to_deliver;
> + struct sk_buff *skb = NULL;
> + u32 flags;
Combined variable declarations.
> spin_lock_bh(&conn->psq_lock);
> - while ((skbuff = skb_dequeue(&conn->psq))
> - != NULL) {
> + while ((skbuff = skb_dequeue(&conn->psq))) {
> + spin_unlock_bh(&conn->psq_lock);
> + ath6kl_data_tx(skbuff, vif->ndev);
> + spin_lock_bh(&conn->psq_lock);
> + skbuff = skb_dequeue(&conn->psq);
> + }
This was buggy now. After the first skb you will take two skbs in one round.
> +
> + is_apsdq_empty = skb_queue_empty(&conn->apsdq);
> + while ((skbuff = skb_dequeue(&conn->apsdq))) {
> spin_unlock_bh(&conn->psq_lock);
> ath6kl_data_tx(skbuff, vif->ndev);
> spin_lock_bh(&conn->psq_lock);
> + skbuff = skb_dequeue(&conn->apsdq);
> }
And this had the same bug.
Kalle
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
