All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ben Greear <greearb@candelatech.com>
To: Peter Oh <poh@qca.qualcomm.com>, ath10k@lists.infradead.org
Cc: linux-wireless@vger.kernel.org
Subject: Re: [PATCH 4/4] ath10k: add abstraction layer for vdev subtype
Date: Wed, 27 Jan 2016 11:11:01 -0800	[thread overview]
Message-ID: <56A91645.9040602@candelatech.com> (raw)
In-Reply-To: <1453920957-9908-5-git-send-email-poh@qca.qualcomm.com>

On 01/27/2016 10:55 AM, Peter Oh wrote:
> Abstraction layer for vdev subtype is added to solve
> subtype mismatch and to give flexible compatibility
> among different firmware revisions.
>
> For instance, 10.2 and 10.4 firmware has different
> definition of their vdev subtypes for Mesh.
> 10.4 defined subtype 6 for 802.11s Mesh while 10.2 uses 5.
> Hence use the abstraction API to get right subtype to use.
>
> Signed-off-by: Peter Oh <poh@qca.qualcomm.com>
> ---
>   drivers/net/wireless/ath/ath10k/mac.c     | 15 ++++---
>   drivers/net/wireless/ath/ath10k/wmi-ops.h | 11 +++++
>   drivers/net/wireless/ath/ath10k/wmi-tlv.c |  1 +
>   drivers/net/wireless/ath/ath10k/wmi.c     | 70 +++++++++++++++++++++++++++++++
>   drivers/net/wireless/ath/ath10k/wmi.h     | 42 ++++++++++++++++---
>   5 files changed, 128 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
> index 2940b00..c9a39ab 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -4345,25 +4345,29 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
>   		   bit, ar->free_vdev_map);
>
>   	arvif->vdev_id = bit;
> -	arvif->vdev_subtype = WMI_VDEV_SUBTYPE_NONE;
> +	arvif->vdev_subtype =
> +		ath10k_wmi_get_vdev_subtype(ar, WMI_VDEV_SUBTYPE_NONE);
>
>   	switch (vif->type) {
>   	case NL80211_IFTYPE_P2P_DEVICE:
>   		arvif->vdev_type = WMI_VDEV_TYPE_STA;
> -		arvif->vdev_subtype = WMI_VDEV_SUBTYPE_P2P_DEVICE;
> +		arvif->vdev_subtype = ath10k_wmi_get_vdev_subtype
> +					(ar, WMI_VDEV_SUBTYPE_P2P_DEVICE);

Would it maybe be simpler code to just assign these types to the ar struct at
firmware init time.  And then do something like:

arvif->vdev_subtype = ar->p2p_subtype;

Or even maybe:

arvif->vdev_subtype = ar->subtype_for_viftype[vif->type];

I'm not sure how much it matters, but in general I find the abstraction in
ath10k makes it hard to read through the code.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

WARNING: multiple messages have this Message-ID (diff)
From: Ben Greear <greearb@candelatech.com>
To: Peter Oh <poh@qca.qualcomm.com>, ath10k@lists.infradead.org
Cc: linux-wireless@vger.kernel.org
Subject: Re: [PATCH 4/4] ath10k: add abstraction layer for vdev subtype
Date: Wed, 27 Jan 2016 11:11:01 -0800	[thread overview]
Message-ID: <56A91645.9040602@candelatech.com> (raw)
In-Reply-To: <1453920957-9908-5-git-send-email-poh@qca.qualcomm.com>

On 01/27/2016 10:55 AM, Peter Oh wrote:
> Abstraction layer for vdev subtype is added to solve
> subtype mismatch and to give flexible compatibility
> among different firmware revisions.
>
> For instance, 10.2 and 10.4 firmware has different
> definition of their vdev subtypes for Mesh.
> 10.4 defined subtype 6 for 802.11s Mesh while 10.2 uses 5.
> Hence use the abstraction API to get right subtype to use.
>
> Signed-off-by: Peter Oh <poh@qca.qualcomm.com>
> ---
>   drivers/net/wireless/ath/ath10k/mac.c     | 15 ++++---
>   drivers/net/wireless/ath/ath10k/wmi-ops.h | 11 +++++
>   drivers/net/wireless/ath/ath10k/wmi-tlv.c |  1 +
>   drivers/net/wireless/ath/ath10k/wmi.c     | 70 +++++++++++++++++++++++++++++++
>   drivers/net/wireless/ath/ath10k/wmi.h     | 42 ++++++++++++++++---
>   5 files changed, 128 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
> index 2940b00..c9a39ab 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -4345,25 +4345,29 @@ static int ath10k_add_interface(struct ieee80211_hw *hw,
>   		   bit, ar->free_vdev_map);
>
>   	arvif->vdev_id = bit;
> -	arvif->vdev_subtype = WMI_VDEV_SUBTYPE_NONE;
> +	arvif->vdev_subtype =
> +		ath10k_wmi_get_vdev_subtype(ar, WMI_VDEV_SUBTYPE_NONE);
>
>   	switch (vif->type) {
>   	case NL80211_IFTYPE_P2P_DEVICE:
>   		arvif->vdev_type = WMI_VDEV_TYPE_STA;
> -		arvif->vdev_subtype = WMI_VDEV_SUBTYPE_P2P_DEVICE;
> +		arvif->vdev_subtype = ath10k_wmi_get_vdev_subtype
> +					(ar, WMI_VDEV_SUBTYPE_P2P_DEVICE);

Would it maybe be simpler code to just assign these types to the ar struct at
firmware init time.  And then do something like:

arvif->vdev_subtype = ar->p2p_subtype;

Or even maybe:

arvif->vdev_subtype = ar->subtype_for_viftype[vif->type];

I'm not sure how much it matters, but in general I find the abstraction in
ath10k makes it hard to read through the code.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


  reply	other threads:[~2016-01-27 19:11 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-27 18:55 [PATCH 0/4] ath10k: rename mesh and add abstraction layer Peter Oh
2016-01-27 18:55 ` Peter Oh
2016-01-27 18:55 ` [PATCH 1/4] ath10k: rename Mesh related service names Peter Oh
2016-01-27 18:55   ` Peter Oh
2016-01-27 18:55 ` [PATCH 2/4] ath10k: update 10.4 WMI service map Peter Oh
2016-01-27 18:55   ` Peter Oh
2016-01-27 18:55 ` [PATCH 3/4] ath10k: use vif->type and vif->p2p for P2P_GO check Peter Oh
2016-01-27 18:55   ` Peter Oh
2016-01-27 18:55 ` [PATCH 4/4] ath10k: add abstraction layer for vdev subtype Peter Oh
2016-01-27 18:55   ` Peter Oh
2016-01-27 19:11   ` Ben Greear [this message]
2016-01-27 19:11     ` Ben Greear
2016-01-27 21:44     ` Peter Oh
2016-01-27 21:44       ` Peter Oh
2016-01-27 19:39 ` [PATCH 0/4] ath10k: rename mesh and add abstraction layer Bob Copeland
2016-01-27 19:39   ` Bob Copeland
2016-01-27 21:53   ` Peter Oh
2016-01-27 21:53     ` Peter Oh
2016-01-28  2:04     ` Yeoh Chun-Yeow
2016-01-28  2:04       ` Yeoh Chun-Yeow
2016-01-28  5:12       ` Peter Oh
2016-01-28  5:12         ` Peter Oh
2016-01-28  8:51 ` Kalle Valo
2016-01-28  8:51   ` Kalle Valo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=56A91645.9040602@candelatech.com \
    --to=greearb@candelatech.com \
    --cc=ath10k@lists.infradead.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=poh@qca.qualcomm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.