diff mbox

[v2,1/6] ath10k: use inline ce_state structure

Message ID 1377601683-12072-2-git-send-email-michal.kazior@tieto.com (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Michal Kazior Aug. 27, 2013, 11:07 a.m. UTC
Simplifies memory managament of ce_state.

Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 drivers/net/wireless/ath/ath10k/ce.c  |   37 ++++++++++++---------------------
 drivers/net/wireless/ath/ath10k/pci.h |    5 ++++-
 2 files changed, 17 insertions(+), 25 deletions(-)

Comments

Kalle Valo Aug. 28, 2013, 4:12 a.m. UTC | #1
Michal Kazior <michal.kazior@tieto.com> writes:

> Simplifies memory managament of ce_state.
>
> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>

Sorry, I noticed this only in my second review round:

> --- a/drivers/net/wireless/ath/ath10k/pci.h
> +++ b/drivers/net/wireless/ath/ath10k/pci.h
> @@ -233,7 +233,10 @@ struct ath10k_pci {
>  	spinlock_t ce_lock;
>  
>  	/* Map CE id to ce_state */
> -	struct ce_state *ce_id_to_state[CE_COUNT_MAX];
> +	struct ce_state ce_states[CE_COUNT_MAX];
> +
> +	/* makes sure that dummy reads are atomic */
> +	spinlock_t hw_v1_workaround_lock;
>  };

That lock doesn't look right. Is it a leftover from a rebase?
Michal Kazior Aug. 28, 2013, 5:32 a.m. UTC | #2
On 28 August 2013 06:12, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> Michal Kazior <michal.kazior@tieto.com> writes:
>
>> Simplifies memory managament of ce_state.
>>
>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>
> Sorry, I noticed this only in my second review round:
>
>> --- a/drivers/net/wireless/ath/ath10k/pci.h
>> +++ b/drivers/net/wireless/ath/ath10k/pci.h
>> @@ -233,7 +233,10 @@ struct ath10k_pci {
>>       spinlock_t ce_lock;
>>
>>       /* Map CE id to ce_state */
>> -     struct ce_state *ce_id_to_state[CE_COUNT_MAX];
>> +     struct ce_state ce_states[CE_COUNT_MAX];
>> +
>> +     /* makes sure that dummy reads are atomic */
>> +     spinlock_t hw_v1_workaround_lock;
>>  };
>
> That lock doesn't look right. Is it a leftover from a rebase?

Oh, good catch. I wonder how it got there..


Micha?.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Kalle Valo Sept. 1, 2013, 6:34 a.m. UTC | #3
Michal Kazior <michal.kazior@tieto.com> writes:

> On 28 August 2013 06:12, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>> Michal Kazior <michal.kazior@tieto.com> writes:
>>
>>> Simplifies memory managament of ce_state.
>>>
>>> Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
>>
>> Sorry, I noticed this only in my second review round:
>>
>>> --- a/drivers/net/wireless/ath/ath10k/pci.h
>>> +++ b/drivers/net/wireless/ath/ath10k/pci.h
>>> @@ -233,7 +233,10 @@ struct ath10k_pci {
>>>       spinlock_t ce_lock;
>>>
>>>       /* Map CE id to ce_state */
>>> -     struct ce_state *ce_id_to_state[CE_COUNT_MAX];
>>> +     struct ce_state ce_states[CE_COUNT_MAX];
>>> +
>>> +     /* makes sure that dummy reads are atomic */
>>> +     spinlock_t hw_v1_workaround_lock;
>>>  };
>>
>> That lock doesn't look right. Is it a leftover from a rebase?
>
> Oh, good catch. I wonder how it got there..

I had patches pending on this patchset, so I removed the lock from this
patch and applied it.
diff mbox

Patch

diff --git a/drivers/net/wireless/ath/ath10k/ce.c b/drivers/net/wireless/ath/ath10k/ce.c
index 1a702e1..c8b7d21 100644
--- a/drivers/net/wireless/ath/ath10k/ce.c
+++ b/drivers/net/wireless/ath/ath10k/ce.c
@@ -727,7 +727,7 @@  int ath10k_ce_completed_send_next(struct ce_state *ce_state,
 void ath10k_ce_per_engine_service(struct ath10k *ar, unsigned int ce_id)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	struct ce_state *ce_state = ar_pci->ce_id_to_state[ce_id];
+	struct ce_state *ce_state = &ar_pci->ce_states[ce_id];
 	u32 ctrl_addr = ce_state->ctrl_addr;
 	void *transfer_context;
 	u32 buf;
@@ -846,7 +846,7 @@  void ath10k_ce_disable_interrupts(struct ath10k *ar)
 
 	ath10k_pci_wake(ar);
 	for (ce_id = 0; ce_id < ar_pci->ce_count; ce_id++) {
-		struct ce_state *ce_state = ar_pci->ce_id_to_state[ce_id];
+		struct ce_state *ce_state = &ar_pci->ce_states[ce_id];
 		u32 ctrl_addr = ce_state->ctrl_addr;
 
 		ath10k_ce_copy_complete_intr_disable(ar, ctrl_addr);
@@ -1081,27 +1081,17 @@  static struct ce_state *ath10k_ce_init_state(struct ath10k *ar,
 					     const struct ce_attr *attr)
 {
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
-	struct ce_state *ce_state = NULL;
+	struct ce_state *ce_state = &ar_pci->ce_states[ce_id];
 	u32 ctrl_addr = ath10k_ce_base_address(ce_id);
 
 	spin_lock_bh(&ar_pci->ce_lock);
 
-	if (!ar_pci->ce_id_to_state[ce_id]) {
-		ce_state = kzalloc(sizeof(*ce_state), GFP_ATOMIC);
-		if (ce_state == NULL) {
-			spin_unlock_bh(&ar_pci->ce_lock);
-			return NULL;
-		}
-
-		ar_pci->ce_id_to_state[ce_id] = ce_state;
-		ce_state->ar = ar;
-		ce_state->id = ce_id;
-		ce_state->ctrl_addr = ctrl_addr;
-		ce_state->state = CE_RUNNING;
-		/* Save attribute flags */
-		ce_state->attr_flags = attr->flags;
-		ce_state->src_sz_max = attr->src_sz_max;
-	}
+	ce_state->ar = ar;
+	ce_state->id = ce_id;
+	ce_state->ctrl_addr = ctrl_addr;
+	ce_state->state = CE_RUNNING;
+	ce_state->attr_flags = attr->flags;
+	ce_state->src_sz_max = attr->src_sz_max;
 
 	spin_unlock_bh(&ar_pci->ce_lock);
 
@@ -1159,13 +1149,9 @@  struct ce_state *ath10k_ce_init(struct ath10k *ar,
 
 void ath10k_ce_deinit(struct ce_state *ce_state)
 {
-	unsigned int ce_id = ce_state->id;
 	struct ath10k *ar = ce_state->ar;
 	struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
 
-	ce_state->state = CE_UNUSED;
-	ar_pci->ce_id_to_state[ce_id] = NULL;
-
 	if (ce_state->src_ring) {
 		kfree(ce_state->src_ring->shadow_base_unaligned);
 		pci_free_consistent(ar_pci->pdev,
@@ -1186,5 +1172,8 @@  void ath10k_ce_deinit(struct ce_state *ce_state)
 				    ce_state->dest_ring->base_addr_ce_space);
 		kfree(ce_state->dest_ring);
 	}
-	kfree(ce_state);
+
+	ce_state->state = CE_UNUSED;
+	ce_state->src_ring = NULL;
+	ce_state->dest_ring = NULL;
 }
diff --git a/drivers/net/wireless/ath/ath10k/pci.h b/drivers/net/wireless/ath/ath10k/pci.h
index 153ae28..64fcef5 100644
--- a/drivers/net/wireless/ath/ath10k/pci.h
+++ b/drivers/net/wireless/ath/ath10k/pci.h
@@ -233,7 +233,10 @@  struct ath10k_pci {
 	spinlock_t ce_lock;
 
 	/* Map CE id to ce_state */
-	struct ce_state *ce_id_to_state[CE_COUNT_MAX];
+	struct ce_state ce_states[CE_COUNT_MAX];
+
+	/* makes sure that dummy reads are atomic */
+	spinlock_t hw_v1_workaround_lock;
 };
 
 static inline struct ath10k_pci *ath10k_pci_priv(struct ath10k *ar)