Re: [PATCH 3/4] ath6kl: Add uAPSD support in rx path.

[Kalle Valo <kvalo@qca.qualcomm.com>]

On 01/09/2012 09:45 AM, Thirumalai wrote:
> If uAPSD trigger is received, send out the packets in uAPSD
> queue. Set more data bit if the queue is not empty else
> update the uAPSD bitmap for the station.
> 
> Signed-off-by: Thirumalai <tpachamu@qca.qualcomm.com>

[...]

> +	/*
> +	 * If the APSD q for this STA is not empty, dequeue and
> +	 * send a pkt from the head of the q. Also update the
> +	 * More data bit in the WMI_DATA_HDR if there are
> +	 * more pkts for this STA in the APSD q.
> +	 * If there are no more pkts for this STA,
> +	 * update the APSD bitmap for this STA.
> +	 */
> +
> +	num_frames_to_deliver = (conn->apsd_info >> 4) & 0xF;

Defines would be nice for values 4 and 0xF.

> +
> +	/*
> +	 * Number of frames to send in a service period is
> +	 * indicated by the station
> +	 * in the QOS_INFO of the association request
> +	 * If it is zero, send all frames
> +	 */
> +	if (!num_frames_to_deliver)
> +		num_frames_to_deliver = 0xFFFF;

And for 0xFFFF.

> +
> +	spin_lock_bh(&conn->psq_lock);
> +	is_apsdq_empty  = skb_queue_empty(&conn->apsdq);

Extra space after is_apsdq_empty.

> -				while ((skbuff = skb_dequeue(&conn->psq))
> -				       != NULL) {
> +				skbuff = skb_dequeue(&conn->psq);
> +				while (skbuff != NULL) {
> +					spin_unlock_bh(&conn->psq_lock);
> +					ath6kl_data_tx(skbuff, vif->ndev);
> +					spin_lock_bh(&conn->psq_lock);
> +					skbuff = skb_dequeue(&conn->psq);
> +				}

I would say the original style is better:

while ((skbuff = skb_dequeue(&conn->psq))

> +
> +				is_apsdq_empty = skb_queue_empty(&conn->apsdq);
> +				skbuff = skb_dequeue(&conn->apsdq);
> +				while (skbuff != NULL) {
>  					spin_unlock_bh(&conn->psq_lock);
>  					ath6kl_data_tx(skbuff, vif->ndev);
>  					spin_lock_bh(&conn->psq_lock);
> +					skbuff = skb_dequeue(&conn->apsdq);
>  				}

Same here.

Kalle