[PATCH 0/7] net: wireless: wcn36xx: add basic wcn3620 support

[PATCH 0/7] net: wireless: wcn36xx: add basic wcn3620 support

[Andy Green]

The following series adds some basic wcn3620 support to the
mainline wcn36xx driver.

 - modify the chip detection to be told which chip from
   platform ops, which can get it from Device Tree... the
   existing detection is just assume it's a wcn3680 if it
   has AC mode which is not enough

 - Adapt the DXE register selection to use 3680 mode for
   3620 as well

 - Add a couple of async "indication" messages which can
   appear down smd on 3620, just accept and ignore

 - Accept a modified form of trigger_ba response that is
   sent by the wcn3620 firmware, if device is a 3620

 - Disable powersaving mode if 3620

With these the 3620 can associate and work for a while
before needing to be forcibly reassociated again.

It's tested on an msm8916-QRD "phone" dev platform that
includes wcn3620.

---

Andy Green (7):
      net: wireless: wcn36xx: add wcn3620 chip type definition
      net: wireless: wcn36xx: get chip type from platform ops
      net: wireless: wcn36xx: use 3680 dxe regs for 3620
      net: wireless: wcn36xx: introduce WCN36XX_HAL_AVOID_FREQ_RANGE_IND
      net: wireless: wcn36xx: swallow two wcn3620 IND messages
      net: wireless: wcn36xx: remove powersaving for wcn3620
      net: wireless: wcn36xx: handle new trigger_ba format


 drivers/net/wireless/ath/wcn36xx/dxe.c     |    2 +-
 drivers/net/wireless/ath/wcn36xx/hal.h     |    2 ++
 drivers/net/wireless/ath/wcn36xx/main.c    |   22 +++++++-----------
 drivers/net/wireless/ath/wcn36xx/smd.c     |   34 ++++++++++++++++++++++++++--
 drivers/net/wireless/ath/wcn36xx/smd.h     |    9 +++++++
 drivers/net/wireless/ath/wcn36xx/wcn36xx.h |   13 +++++++----
 6 files changed, 62 insertions(+), 20 deletions(-)

--

[PATCH 0/7] net: wireless: wcn36xx: add basic wcn3620 support

[Andy Green]

The following series adds some basic wcn3620 support to the
mainline wcn36xx driver.

 - modify the chip detection to be told which chip from
   platform ops, which can get it from Device Tree... the
   existing detection is just assume it's a wcn3680 if it
   has AC mode which is not enough

 - Adapt the DXE register selection to use 3680 mode for
   3620 as well

 - Add a couple of async "indication" messages which can
   appear down smd on 3620, just accept and ignore

 - Accept a modified form of trigger_ba response that is
   sent by the wcn3620 firmware, if device is a 3620

 - Disable powersaving mode if 3620

With these the 3620 can associate and work for a while
before needing to be forcibly reassociated again.

It's tested on an msm8916-QRD "phone" dev platform that
includes wcn3620.

---

Andy Green (7):
      net: wireless: wcn36xx: add wcn3620 chip type definition
      net: wireless: wcn36xx: get chip type from platform ops
      net: wireless: wcn36xx: use 3680 dxe regs for 3620
      net: wireless: wcn36xx: introduce WCN36XX_HAL_AVOID_FREQ_RANGE_IND
      net: wireless: wcn36xx: swallow two wcn3620 IND messages
      net: wireless: wcn36xx: remove powersaving for wcn3620
      net: wireless: wcn36xx: handle new trigger_ba format


 drivers/net/wireless/ath/wcn36xx/dxe.c     |    2 +-
 drivers/net/wireless/ath/wcn36xx/hal.h     |    2 ++
 drivers/net/wireless/ath/wcn36xx/main.c    |   22 +++++++-----------
 drivers/net/wireless/ath/wcn36xx/smd.c     |   34 ++++++++++++++++++++++++++--
 drivers/net/wireless/ath/wcn36xx/smd.h     |    9 +++++++
 drivers/net/wireless/ath/wcn36xx/wcn36xx.h |   13 +++++++----
 6 files changed, 62 insertions(+), 20 deletions(-)

--
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/7] net: wireless: wcn36xx: add wcn3620 chip type definition

[Andy Green]

Convert the list of chip types to an enum, add the default
UNKNOWN type and a type for WCN3620 chip

Signed-off-by: Andy Green <andy.green@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/wcn36xx.h |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
index f0fb81d..a5366b6 100644
--- a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
+++ b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
@@ -93,6 +93,13 @@ struct nv_data {
 	u8	table;
 };
 
+enum wcn36xx_chip_type {
+	WCN36XX_CHIP_UNKNOWN,
+	WCN36XX_CHIP_3660,
+	WCN36XX_CHIP_3680,
+	WCN36XX_CHIP_3620,
+};
+
 /* Interface for platform control path
  *
  * @open: hook must be called when wcn36xx wants to open control channel.
@@ -179,7 +186,7 @@ struct wcn36xx {
 	u8			fw_minor;
 	u8			fw_major;
 	u32			fw_feat_caps[WCN36XX_HAL_CAPS_SIZE];
-	u32			chip_version;
+	enum wcn36xx_chip_type	chip_version;
 
 	/* extra byte for the NULL termination */
 	u8			crm_version[WCN36XX_HAL_VERSION_LENGTH + 1];
@@ -227,9 +234,6 @@ struct wcn36xx {
 
 };
 
-#define WCN36XX_CHIP_3660	0
-#define WCN36XX_CHIP_3680	1
-
 static inline bool wcn36xx_is_fw_version(struct wcn36xx *wcn,
 					 u8 major,
 					 u8 minor,

[PATCH 1/7] net: wireless: wcn36xx: add wcn3620 chip type definition

[Andy Green]

Convert the list of chip types to an enum, add the default
UNKNOWN type and a type for WCN3620 chip

Signed-off-by: Andy Green <andy.green-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/net/wireless/ath/wcn36xx/wcn36xx.h |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
index f0fb81d..a5366b6 100644
--- a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
+++ b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
@@ -93,6 +93,13 @@ struct nv_data {
 	u8	table;
 };
 
+enum wcn36xx_chip_type {
+	WCN36XX_CHIP_UNKNOWN,
+	WCN36XX_CHIP_3660,
+	WCN36XX_CHIP_3680,
+	WCN36XX_CHIP_3620,
+};
+
 /* Interface for platform control path
  *
  * @open: hook must be called when wcn36xx wants to open control channel.
@@ -179,7 +186,7 @@ struct wcn36xx {
 	u8			fw_minor;
 	u8			fw_major;
 	u32			fw_feat_caps[WCN36XX_HAL_CAPS_SIZE];
-	u32			chip_version;
+	enum wcn36xx_chip_type	chip_version;
 
 	/* extra byte for the NULL termination */
 	u8			crm_version[WCN36XX_HAL_VERSION_LENGTH + 1];
@@ -227,9 +234,6 @@ struct wcn36xx {
 
 };
 
-#define WCN36XX_CHIP_3660	0
-#define WCN36XX_CHIP_3680	1

[PATCH 2/7] net: wireless: wcn36xx: get chip type from platform ops

[Andy Green]

Autodetecting the chip type does not work well.
Stop attempting to do it and require a platform op
that tells us what the chip is.

Signed-off-by: Andy Green <andy.green@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/main.c    |   18 +++++-------------
 drivers/net/wireless/ath/wcn36xx/wcn36xx.h |    1 +
 2 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index 7dd8873..c4178c7 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -221,17 +221,6 @@ static void wcn36xx_feat_caps_info(struct wcn36xx *wcn)
 	}
 }
 
-static void wcn36xx_detect_chip_version(struct wcn36xx *wcn)
-{
-	if (get_feat_caps(wcn->fw_feat_caps, DOT11AC)) {
-		wcn36xx_info("Chip is 3680\n");
-		wcn->chip_version = WCN36XX_CHIP_3680;
-	} else {
-		wcn36xx_info("Chip is 3660\n");
-		wcn->chip_version = WCN36XX_CHIP_3660;
-	}
-}
-
 static int wcn36xx_start(struct ieee80211_hw *hw)
 {
 	struct wcn36xx *wcn = hw->priv;
@@ -286,8 +275,6 @@ static int wcn36xx_start(struct ieee80211_hw *hw)
 			wcn36xx_feat_caps_info(wcn);
 	}
 
-	wcn36xx_detect_chip_version(wcn);
-
 	/* DMA channel initialization */
 	ret = wcn36xx_dxe_init(wcn);
 	if (ret) {
@@ -1023,6 +1010,11 @@ static int wcn36xx_probe(struct platform_device *pdev)
 	wcn->hw = hw;
 	wcn->dev = &pdev->dev;
 	wcn->ctrl_ops = pdev->dev.platform_data;
+	if (!wcn->ctrl_ops->get_chip_type) {
+		dev_err(&pdev->dev, "Missing ops->get_chip_type\n");
+		return -EINVAL;
+	}
+	wcn->chip_version = wcn->ctrl_ops->get_chip_type();
 
 	mutex_init(&wcn->hal_mutex);
 
diff --git a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
index a5366b6..04793c6 100644
--- a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
+++ b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
@@ -110,6 +110,7 @@ struct wcn36xx_platform_ctrl_ops {
 	void (*close)(void);
 	int (*tx)(char *buf, size_t len);
 	int (*get_hw_mac)(u8 *addr);
+	int (*get_chip_type)(void);
 	int (*smsm_change_state)(u32 clear_mask, u32 set_mask);
 };
 

[PATCH 3/7] net: wireless: wcn36xx: use 3680 dxe regs for 3620

[Andy Green]

Between 3620, 3660 and 3680, only 3660 has a different dxe register

Signed-off-by: Andy Green <andy.green@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/dxe.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/dxe.c b/drivers/net/wireless/ath/wcn36xx/dxe.c
index 73f12f1..334f265 100644
--- a/drivers/net/wireless/ath/wcn36xx/dxe.c
+++ b/drivers/net/wireless/ath/wcn36xx/dxe.c
@@ -46,7 +46,7 @@ static void wcn36xx_dxe_write_register(struct wcn36xx *wcn, int addr, int data)
 
 #define wcn36xx_dxe_write_register_x(wcn, reg, reg_data)		 \
 do {									 \
-	if (wcn->chip_version == WCN36XX_CHIP_3680)			 \
+	if (wcn->chip_version != WCN36XX_CHIP_3660)			 \
 		wcn36xx_dxe_write_register(wcn, reg ## _3680, reg_data); \
 	else								 \
 		wcn36xx_dxe_write_register(wcn, reg ## _3660, reg_data); \

[PATCH 4/7] net: wireless: wcn36xx: introduce WCN36XX_HAL_AVOID_FREQ_RANGE_IND

[Andy Green]

WCN3620 firmware introduces a new async indication, we need to
add it as a known message type so we can accept it

Signed-off-by: Andy Green <andy.green@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/hal.h |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/hal.h b/drivers/net/wireless/ath/wcn36xx/hal.h
index a1f1127..b947de0 100644
--- a/drivers/net/wireless/ath/wcn36xx/hal.h
+++ b/drivers/net/wireless/ath/wcn36xx/hal.h
@@ -345,6 +345,8 @@ enum wcn36xx_hal_host_msg_type {
 	WCN36XX_HAL_DHCP_START_IND = 189,
 	WCN36XX_HAL_DHCP_STOP_IND = 190,
 
+	WCN36XX_HAL_AVOID_FREQ_RANGE_IND = 233,
+
 	WCN36XX_HAL_MSG_MAX = WCN36XX_HAL_MSG_TYPE_MAX_ENUM_SIZE
 };
 

[PATCH 5/7] net: wireless: wcn36xx: swallow two wcn3620 IND messages

[Andy Green]

WCN3620 can asynchronously send two new kinds of indication message,
since we can't handle them just accept them quietly.

Signed-off-by: Andy Green <andy.green@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/smd.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index 6398693..819741c 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -2061,6 +2061,8 @@ static void wcn36xx_smd_rsp_process(struct wcn36xx *wcn, void *buf, size_t len)
 		complete(&wcn->hal_rsp_compl);
 		break;
 
+	case WCN36XX_HAL_COEX_IND:
+	case WCN36XX_HAL_AVOID_FREQ_RANGE_IND:
 	case WCN36XX_HAL_OTA_TX_COMPL_IND:
 	case WCN36XX_HAL_MISSED_BEACON_IND:
 	case WCN36XX_HAL_DELETE_STA_CONTEXT_IND:
@@ -2107,6 +2109,10 @@ static void wcn36xx_ind_smd_work(struct work_struct *work)
 	msg_header = (struct wcn36xx_hal_msg_header *)hal_ind_msg->msg;
 
 	switch (msg_header->msg_type) {
+	case WCN36XX_HAL_COEX_IND:
+		break;
+	case WCN36XX_HAL_AVOID_FREQ_RANGE_IND:
+		break;
 	case WCN36XX_HAL_OTA_TX_COMPL_IND:
 		wcn36xx_smd_tx_compl_ind(wcn,
 					 hal_ind_msg->msg,

[PATCH 6/7] net: wireless: wcn36xx: remove powersaving for wcn3620

[Andy Green]

WCN3620 powersaving mode is not stable.  Disable it if we're
on a wcn3620 chip type.

Signed-off-by: Andy Green <andy.green@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/main.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
index c4178c7..569d45b 100644
--- a/drivers/net/wireless/ath/wcn36xx/main.c
+++ b/drivers/net/wireless/ath/wcn36xx/main.c
@@ -924,6 +924,10 @@ static int wcn36xx_init_ieee80211(struct wcn36xx *wcn)
 		IEEE80211_HW_AMPDU_AGGREGATION |
 		IEEE80211_HW_TIMING_BEACON_ONLY;
 
+	/* 3620 powersaving currently unstable */
+	if (wcn->chip_version == WCN36XX_CHIP_3620)
+		wcn->hw->flags &= ~IEEE80211_HW_SUPPORTS_PS;
+
 	wcn->hw->wiphy->interface_modes = BIT(NL80211_IFTYPE_STATION) |
 		BIT(NL80211_IFTYPE_AP) |
 		BIT(NL80211_IFTYPE_ADHOC) |

[PATCH 7/7] net: wireless: wcn36xx: handle new trigger_ba format

[Andy Green]

wcn3620 has a new message structure for the reply to trigger_ba
We don't know what to do with the candidate list he sends back,
but we can at least accept and ignore it nicely instead of dying.

Signed-off-by: Andy Green <andy.green@linaro.org>
---
 drivers/net/wireless/ath/wcn36xx/smd.c |   28 ++++++++++++++++++++++++++--
 drivers/net/wireless/ath/wcn36xx/smd.h |    9 +++++++++
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
index 819741c..dc24e1b 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.c
+++ b/drivers/net/wireless/ath/wcn36xx/smd.c
@@ -243,8 +243,31 @@ static int wcn36xx_smd_rsp_status_check(void *buf, size_t len)
 	rsp = (struct wcn36xx_fw_msg_status_rsp *)
 		(buf + sizeof(struct wcn36xx_hal_msg_header));
 
-	if (WCN36XX_FW_MSG_RESULT_SUCCESS != rsp->status)
+	if (WCN36XX_FW_MSG_RESULT_SUCCESS != rsp->status) {
+		pr_err("%s: bad status, len = %d\n", __func__, len);
+		return rsp->status;
+	}
+
+	return 0;
+}
+
+static int wcn36xx_smd_rsp_status_check_bav2(struct wcn36xx *wcn, void *buf,
+					     size_t len)
+{
+	struct wcn36xx_fw_msg_status_rspv2 *rsp;
+
+	if (wcn->chip_version != WCN36XX_CHIP_3620)
+		return wcn36xx_smd_rsp_status_check(buf, len);
+
+	if (len < sizeof(struct wcn36xx_hal_msg_header) + sizeof(*rsp))
+		return -EIO;
+
+	rsp = buf + sizeof(struct wcn36xx_hal_msg_header);
+
+	if (WCN36XX_FW_MSG_RESULT_SUCCESS != rsp->status) {
+		pr_err("%s: bad status, len = %d\n", __func__, len);
 		return rsp->status;
+	}
 
 	return 0;
 }
@@ -1884,7 +1907,8 @@ int wcn36xx_smd_trigger_ba(struct wcn36xx *wcn, u8 sta_index)
 		wcn36xx_err("Sending hal_trigger_ba failed\n");
 		goto out;
 	}
-	ret = wcn36xx_smd_rsp_status_check(wcn->hal_buf, wcn->hal_rsp_len);
+	ret = wcn36xx_smd_rsp_status_check_bav2(wcn, wcn->hal_buf,
+						wcn->hal_rsp_len);
 	if (ret) {
 		wcn36xx_err("hal_trigger_ba response failed err=%d\n", ret);
 		goto out;
diff --git a/drivers/net/wireless/ath/wcn36xx/smd.h b/drivers/net/wireless/ath/wcn36xx/smd.h
index 008d034..432d3b8 100644
--- a/drivers/net/wireless/ath/wcn36xx/smd.h
+++ b/drivers/net/wireless/ath/wcn36xx/smd.h
@@ -44,6 +44,15 @@ struct wcn36xx_fw_msg_status_rsp {
 	u32	status;
 } __packed;
 
+/* wcn3620 returns this for tigger_ba */
+
+struct wcn36xx_fw_msg_status_rspv2 {
+	u8	bss_id[6];
+	u32	status __packed;
+	u16	count_following_candidates __packed;
+	/* candidate list follows */
+};
+
 struct wcn36xx_hal_ind_msg {
 	struct list_head list;
 	u8 *msg;

Re: [PATCH 2/7] net: wireless: wcn36xx: get chip type from platform ops

[Pat Erley]

On 01/17/2015 11:10 PM, Andy Green wrote:
> Autodetecting the chip type does not work well.
> Stop attempting to do it and require a platform op
> that tells us what the chip is.
>
> Signed-off-by: Andy Green <andy.green@linaro.org>
> ---
>   drivers/net/wireless/ath/wcn36xx/main.c    |   18 +++++-------------
>   drivers/net/wireless/ath/wcn36xx/wcn36xx.h |    1 +
>   2 files changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
> index 7dd8873..c4178c7 100644
> --- a/drivers/net/wireless/ath/wcn36xx/main.c
> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
> @@ -221,17 +221,6 @@ static void wcn36xx_feat_caps_info(struct wcn36xx *wcn)
>   	}
>   }
>
> -static void wcn36xx_detect_chip_version(struct wcn36xx *wcn)
> -{
> -	if (get_feat_caps(wcn->fw_feat_caps, DOT11AC)) {
> -		wcn36xx_info("Chip is 3680\n");
> -		wcn->chip_version = WCN36XX_CHIP_3680;
> -	} else {
> -		wcn36xx_info("Chip is 3660\n");
> -		wcn->chip_version = WCN36XX_CHIP_3660;
> -	}
> -}
> -
>   static int wcn36xx_start(struct ieee80211_hw *hw)
>   {
>   	struct wcn36xx *wcn = hw->priv;
> @@ -286,8 +275,6 @@ static int wcn36xx_start(struct ieee80211_hw *hw)
>   			wcn36xx_feat_caps_info(wcn);
>   	}
>
> -	wcn36xx_detect_chip_version(wcn);
> -
>   	/* DMA channel initialization */
>   	ret = wcn36xx_dxe_init(wcn);
>   	if (ret) {
> @@ -1023,6 +1010,11 @@ static int wcn36xx_probe(struct platform_device *pdev)
>   	wcn->hw = hw;
>   	wcn->dev = &pdev->dev;
>   	wcn->ctrl_ops = pdev->dev.platform_data;
> +	if (!wcn->ctrl_ops->get_chip_type) {
> +		dev_err(&pdev->dev, "Missing ops->get_chip_type\n");
> +		return -EINVAL;
> +	}
> +	wcn->chip_version = wcn->ctrl_ops->get_chip_type();
>
>   	mutex_init(&wcn->hal_mutex);
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
> index a5366b6..04793c6 100644
> --- a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
> +++ b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
> @@ -110,6 +110,7 @@ struct wcn36xx_platform_ctrl_ops {
>   	void (*close)(void);
>   	int (*tx)(char *buf, size_t len);
>   	int (*get_hw_mac)(u8 *addr);
> +	int (*get_chip_type)(void);
>   	int (*smsm_change_state)(u32 clear_mask, u32 set_mask);
>   };
>

(This all assumes this driver is currently actually being used)

Doesn't this change break any current users of wcn36xx?  Couldn't you
just take the old wcn36xx_detect_chip_version and either add the check
for get_chip_type to the beginning and make it use it and return, or
fall back to the old 'broken' way?

Another alternative would be to update wcn36xx_detect_chip_version to
behave like you expect get_chip_type to, and make it the default and let
platforms override it.

Re: [PATCH 2/7] net: wireless: wcn36xx: get chip type from platform ops

[Pat Erley]

On 01/17/2015 11:10 PM, Andy Green wrote:
> Autodetecting the chip type does not work well.
> Stop attempting to do it and require a platform op
> that tells us what the chip is.
>
> Signed-off-by: Andy Green <andy.green-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>   drivers/net/wireless/ath/wcn36xx/main.c    |   18 +++++-------------
>   drivers/net/wireless/ath/wcn36xx/wcn36xx.h |    1 +
>   2 files changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
> index 7dd8873..c4178c7 100644
> --- a/drivers/net/wireless/ath/wcn36xx/main.c
> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
> @@ -221,17 +221,6 @@ static void wcn36xx_feat_caps_info(struct wcn36xx *wcn)
>   	}
>   }
>
> -static void wcn36xx_detect_chip_version(struct wcn36xx *wcn)
> -{
> -	if (get_feat_caps(wcn->fw_feat_caps, DOT11AC)) {
> -		wcn36xx_info("Chip is 3680\n");
> -		wcn->chip_version = WCN36XX_CHIP_3680;
> -	} else {
> -		wcn36xx_info("Chip is 3660\n");
> -		wcn->chip_version = WCN36XX_CHIP_3660;
> -	}
> -}
> -
>   static int wcn36xx_start(struct ieee80211_hw *hw)
>   {
>   	struct wcn36xx *wcn = hw->priv;
> @@ -286,8 +275,6 @@ static int wcn36xx_start(struct ieee80211_hw *hw)
>   			wcn36xx_feat_caps_info(wcn);
>   	}
>
> -	wcn36xx_detect_chip_version(wcn);
> -
>   	/* DMA channel initialization */
>   	ret = wcn36xx_dxe_init(wcn);
>   	if (ret) {
> @@ -1023,6 +1010,11 @@ static int wcn36xx_probe(struct platform_device *pdev)
>   	wcn->hw = hw;
>   	wcn->dev = &pdev->dev;
>   	wcn->ctrl_ops = pdev->dev.platform_data;
> +	if (!wcn->ctrl_ops->get_chip_type) {
> +		dev_err(&pdev->dev, "Missing ops->get_chip_type\n");
> +		return -EINVAL;
> +	}
> +	wcn->chip_version = wcn->ctrl_ops->get_chip_type();
>
>   	mutex_init(&wcn->hal_mutex);
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
> index a5366b6..04793c6 100644
> --- a/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
> +++ b/drivers/net/wireless/ath/wcn36xx/wcn36xx.h
> @@ -110,6 +110,7 @@ struct wcn36xx_platform_ctrl_ops {
>   	void (*close)(void);
>   	int (*tx)(char *buf, size_t len);
>   	int (*get_hw_mac)(u8 *addr);
> +	int (*get_chip_type)(void);
>   	int (*smsm_change_state)(u32 clear_mask, u32 set_mask);
>   };
>

(This all assumes this driver is currently actually being used)

Doesn't this change break any current users of wcn36xx?  Couldn't you
just take the old wcn36xx_detect_chip_version and either add the check
for get_chip_type to the beginning and make it use it and return, or
fall back to the old 'broken' way?

Another alternative would be to update wcn36xx_detect_chip_version to
behave like you expect get_chip_type to, and make it the default and let
platforms override it.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/7] net: wireless: wcn36xx: swallow two wcn3620 IND messages

[Sergei Shtylyov]

Hello.

On 1/18/2015 8:11 AM, Andy Green wrote:

> WCN3620 can asynchronously send two new kinds of indication message,
> since we can't handle them just accept them quietly.

> Signed-off-by: Andy Green <andy.green@linaro.org>
> ---
>   drivers/net/wireless/ath/wcn36xx/smd.c |    6 ++++++
>   1 file changed, 6 insertions(+)

> diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
> index 6398693..819741c 100644
> --- a/drivers/net/wireless/ath/wcn36xx/smd.c
> +++ b/drivers/net/wireless/ath/wcn36xx/smd.c
[...]
> @@ -2107,6 +2109,10 @@ static void wcn36xx_ind_smd_work(struct work_struct *work)
>   	msg_header = (struct wcn36xx_hal_msg_header *)hal_ind_msg->msg;
>
>   	switch (msg_header->msg_type) {
> +	case WCN36XX_HAL_COEX_IND:
> +		break;
> +	case WCN36XX_HAL_AVOID_FREQ_RANGE_IND:
> +		break;

    Can't you merge these 2 cases, so that there's only one *break*?

[...]

WBR, Sergei

Re: [PATCH 5/7] net: wireless: wcn36xx: swallow two wcn3620 IND messages

[Sergei Shtylyov]

Hello.

On 1/18/2015 8:11 AM, Andy Green wrote:

> WCN3620 can asynchronously send two new kinds of indication message,
> since we can't handle them just accept them quietly.

> Signed-off-by: Andy Green <andy.green-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>   drivers/net/wireless/ath/wcn36xx/smd.c |    6 ++++++
>   1 file changed, 6 insertions(+)

> diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
> index 6398693..819741c 100644
> --- a/drivers/net/wireless/ath/wcn36xx/smd.c
> +++ b/drivers/net/wireless/ath/wcn36xx/smd.c
[...]
> @@ -2107,6 +2109,10 @@ static void wcn36xx_ind_smd_work(struct work_struct *work)
>   	msg_header = (struct wcn36xx_hal_msg_header *)hal_ind_msg->msg;
>
>   	switch (msg_header->msg_type) {
> +	case WCN36XX_HAL_COEX_IND:
> +		break;
> +	case WCN36XX_HAL_AVOID_FREQ_RANGE_IND:
> +		break;

    Can't you merge these 2 cases, so that there's only one *break*?

[...]

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 5/7] net: wireless: wcn36xx: swallow two wcn3620 IND messages

[Andy Green]

On 18 January 2015 at 19:47, Sergei Shtylyov
<sergei.shtylyov@cogentembedded.com> wrote:
> Hello.
>
> On 1/18/2015 8:11 AM, Andy Green wrote:
>
>> WCN3620 can asynchronously send two new kinds of indication message,
>> since we can't handle them just accept them quietly.
>
>
>> Signed-off-by: Andy Green <andy.green@linaro.org>
>> ---
>>   drivers/net/wireless/ath/wcn36xx/smd.c |    6 ++++++
>>   1 file changed, 6 insertions(+)
>
>
>> diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c
>> b/drivers/net/wireless/ath/wcn36xx/smd.c
>> index 6398693..819741c 100644
>> --- a/drivers/net/wireless/ath/wcn36xx/smd.c
>> +++ b/drivers/net/wireless/ath/wcn36xx/smd.c
>
> [...]
>>
>> @@ -2107,6 +2109,10 @@ static void wcn36xx_ind_smd_work(struct work_struct
>> *work)
>>         msg_header = (struct wcn36xx_hal_msg_header *)hal_ind_msg->msg;
>>
>>         switch (msg_header->msg_type) {
>> +       case WCN36XX_HAL_COEX_IND:
>> +               break;
>> +       case WCN36XX_HAL_AVOID_FREQ_RANGE_IND:
>> +               break;
>
>
>    Can't you merge these 2 cases, so that there's only one *break*?

Yes will do.

-Andy

> [...]
>
> WBR, Sergei
>

Re: [PATCH 1/7] net: wireless: wcn36xx: add wcn3620 chip type definition

[Kalle Valo]

Andy Green <andy.green@linaro.org> writes:

> Convert the list of chip types to an enum, add the default
> UNKNOWN type and a type for WCN3620 chip
>
> Signed-off-by: Andy Green <andy.green@linaro.org>

Please just use "wcn36xx: ", you should drop "net: wireless: " entirely.

-- 
Kalle Valo

Re: [PATCH 1/7] net: wireless: wcn36xx: add wcn3620 chip type definition

[Andy Green]

On 18 January 2015 at 22:17, Kalle Valo <kvalo@codeaurora.org> wrote:
> Andy Green <andy.green@linaro.org> writes:
>
>> Convert the list of chip types to an enum, add the default
>> UNKNOWN type and a type for WCN3620 chip
>>
>> Signed-off-by: Andy Green <andy.green@linaro.org>
>
> Please just use "wcn36xx: ", you should drop "net: wireless: " entirely.

OK.

Can you help me understand what you'd like to see happen with the chip
variant detection stuff?

There's a comment sent to one list only saying it might be preferable
to keep the old detection code as the default.  But there are no
in-tree users of wcn36xx (mainly due to PIL not being in mainline, I
guess).

The old test's equivalence that AC == 3680 seems kind of weak to me
and establishing the type must be passed in from platform code
reflects the situation that there's no public way to detect the chip
type from Qualcomm.  In the second not-for-upstream series I use that
to pass it in from DT, which is how it'd be normally used.

-Andy

> --
> Kalle Valo

Re: [PATCH 1/7] net: wireless: wcn36xx: add wcn3620 chip type definition

[Kalle Valo]

Andy Green <andy.green@linaro.org> writes:

> On 18 January 2015 at 22:17, Kalle Valo <kvalo@codeaurora.org> wrote:
>> Andy Green <andy.green@linaro.org> writes:
n>>
>>> Convert the list of chip types to an enum, add the default
>>> UNKNOWN type and a type for WCN3620 chip
>>>
>>> Signed-off-by: Andy Green <andy.green@linaro.org>
>>
>> Please just use "wcn36xx: ", you should drop "net: wireless: " entirely.
>
> OK.
>
> Can you help me understand what you'd like to see happen with the chip
> variant detection stuff?

I haven't looked at wcn36xx for a long time so I'm not really the right
person to answer. I'm more like a desk jockey now ;)

> There's a comment sent to one list only saying it might be preferable
> to keep the old detection code as the default.  But there are no
> in-tree users of wcn36xx (mainly due to PIL not being in mainline, I
> guess).
>
> The old test's equivalence that AC == 3680 seems kind of weak to me
> and establishing the type must be passed in from platform code
> reflects the situation that there's no public way to detect the chip
> type from Qualcomm.  In the second not-for-upstream series I use that
> to pass it in from DT, which is how it'd be normally used.

Please remember that the DT bindings document has to be acked by the
device-tree maintainers.

-- 
Kalle Valo

Re: [PATCH 1/7] net: wireless: wcn36xx: add wcn3620 chip type definition

[Kalle Valo]

Andy Green <andy.green-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> writes:

> On 18 January 2015 at 22:17, Kalle Valo <kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org> wrote:
>> Andy Green <andy.green-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> writes:
n>>
>>> Convert the list of chip types to an enum, add the default
>>> UNKNOWN type and a type for WCN3620 chip
>>>
>>> Signed-off-by: Andy Green <andy.green-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>
>> Please just use "wcn36xx: ", you should drop "net: wireless: " entirely.
>
> OK.
>
> Can you help me understand what you'd like to see happen with the chip
> variant detection stuff?

I haven't looked at wcn36xx for a long time so I'm not really the right
person to answer. I'm more like a desk jockey now ;)

> There's a comment sent to one list only saying it might be preferable
> to keep the old detection code as the default.  But there are no
> in-tree users of wcn36xx (mainly due to PIL not being in mainline, I
> guess).
>
> The old test's equivalence that AC == 3680 seems kind of weak to me
> and establishing the type must be passed in from platform code
> reflects the situation that there's no public way to detect the chip
> type from Qualcomm.  In the second not-for-upstream series I use that
> to pass it in from DT, which is how it'd be normally used.

Please remember that the DT bindings document has to be acked by the
device-tree maintainers.

-- 
Kalle Valo
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 7/7] net: wireless: wcn36xx: handle new trigger_ba format

[Eugene Krasnikov]

arg... it looks like the code is starting to have to many
if(chip_version) cases. Mabe we should concider separate files for
chip specific logic.

2015-01-18 5:11 GMT+00:00 Andy Green <andy.green@linaro.org>:
> wcn3620 has a new message structure for the reply to trigger_ba
> We don't know what to do with the candidate list he sends back,
> but we can at least accept and ignore it nicely instead of dying.
>
> Signed-off-by: Andy Green <andy.green@linaro.org>
> ---
>  drivers/net/wireless/ath/wcn36xx/smd.c |   28 ++++++++++++++++++++++++++--
>  drivers/net/wireless/ath/wcn36xx/smd.h |    9 +++++++++
>  2 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/smd.c b/drivers/net/wireless/ath/wcn36xx/smd.c
> index 819741c..dc24e1b 100644
> --- a/drivers/net/wireless/ath/wcn36xx/smd.c
> +++ b/drivers/net/wireless/ath/wcn36xx/smd.c
> @@ -243,8 +243,31 @@ static int wcn36xx_smd_rsp_status_check(void *buf, size_t len)
>         rsp = (struct wcn36xx_fw_msg_status_rsp *)
>                 (buf + sizeof(struct wcn36xx_hal_msg_header));
>
> -       if (WCN36XX_FW_MSG_RESULT_SUCCESS != rsp->status)
> +       if (WCN36XX_FW_MSG_RESULT_SUCCESS != rsp->status) {
> +               pr_err("%s: bad status, len = %d\n", __func__, len);
> +               return rsp->status;
> +       }
> +
> +       return 0;
> +}
> +
> +static int wcn36xx_smd_rsp_status_check_bav2(struct wcn36xx *wcn, void *buf,
> +                                            size_t len)
> +{
> +       struct wcn36xx_fw_msg_status_rspv2 *rsp;
> +
> +       if (wcn->chip_version != WCN36XX_CHIP_3620)
> +               return wcn36xx_smd_rsp_status_check(buf, len);
> +
> +       if (len < sizeof(struct wcn36xx_hal_msg_header) + sizeof(*rsp))
> +               return -EIO;
> +
> +       rsp = buf + sizeof(struct wcn36xx_hal_msg_header);
> +
> +       if (WCN36XX_FW_MSG_RESULT_SUCCESS != rsp->status) {
> +               pr_err("%s: bad status, len = %d\n", __func__, len);
>                 return rsp->status;
> +       }
>
>         return 0;
>  }
> @@ -1884,7 +1907,8 @@ int wcn36xx_smd_trigger_ba(struct wcn36xx *wcn, u8 sta_index)
>                 wcn36xx_err("Sending hal_trigger_ba failed\n");
>                 goto out;
>         }
> -       ret = wcn36xx_smd_rsp_status_check(wcn->hal_buf, wcn->hal_rsp_len);
> +       ret = wcn36xx_smd_rsp_status_check_bav2(wcn, wcn->hal_buf,
> +                                               wcn->hal_rsp_len);
>         if (ret) {
>                 wcn36xx_err("hal_trigger_ba response failed err=%d\n", ret);
>                 goto out;
> diff --git a/drivers/net/wireless/ath/wcn36xx/smd.h b/drivers/net/wireless/ath/wcn36xx/smd.h
> index 008d034..432d3b8 100644
> --- a/drivers/net/wireless/ath/wcn36xx/smd.h
> +++ b/drivers/net/wireless/ath/wcn36xx/smd.h
> @@ -44,6 +44,15 @@ struct wcn36xx_fw_msg_status_rsp {
>         u32     status;
>  } __packed;
>
> +/* wcn3620 returns this for tigger_ba */
> +
> +struct wcn36xx_fw_msg_status_rspv2 {
> +       u8      bss_id[6];
> +       u32     status __packed;
> +       u16     count_following_candidates __packed;
> +       /* candidate list follows */
> +};
> +
>  struct wcn36xx_hal_ind_msg {
>         struct list_head list;
>         u8 *msg;
>



-- 
Best regards,
Eugene

Re: [PATCH 4/7] net: wireless: wcn36xx: introduce WCN36XX_HAL_AVOID_FREQ_RANGE_IND

[Eugene Krasnikov]

Do you know when is this message used? sounds important.

2015-01-18 5:11 GMT+00:00 Andy Green <andy.green@linaro.org>:
> WCN3620 firmware introduces a new async indication, we need to
> add it as a known message type so we can accept it
>
> Signed-off-by: Andy Green <andy.green@linaro.org>
> ---
>  drivers/net/wireless/ath/wcn36xx/hal.h |    2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/hal.h b/drivers/net/wireless/ath/wcn36xx/hal.h
> index a1f1127..b947de0 100644
> --- a/drivers/net/wireless/ath/wcn36xx/hal.h
> +++ b/drivers/net/wireless/ath/wcn36xx/hal.h
> @@ -345,6 +345,8 @@ enum wcn36xx_hal_host_msg_type {
>         WCN36XX_HAL_DHCP_START_IND = 189,
>         WCN36XX_HAL_DHCP_STOP_IND = 190,
>
> +       WCN36XX_HAL_AVOID_FREQ_RANGE_IND = 233,
> +
>         WCN36XX_HAL_MSG_MAX = WCN36XX_HAL_MSG_TYPE_MAX_ENUM_SIZE
>  };
>
>



-- 
Best regards,
Eugene

Re: [PATCH 4/7] net: wireless: wcn36xx: introduce WCN36XX_HAL_AVOID_FREQ_RANGE_IND

[Andy Green]

On 28 January 2015 at 04:01, Eugene Krasnikov <k.eugene.e@gmail.com> wrote:
> Do you know when is this message used? sounds important.

It's related to BT coexistance or radar... prima expects this payload with it

#define WLAN_HAL_MAX_AVOID_FREQ_RANGE           4

typedef PACKED_PRE struct PACKED_POST
{
   tANI_U32     startFreq;
   tANI_U32     endFreq;
}  tHalFreqRange, *tpHalFreqRange;

typedef PACKED_PRE struct PACKED_POST
{
   tANI_U32         avoidCnt;
   tHalFreqRange    avoidRange[WLAN_HAL_MAX_AVOID_FREQ_RANGE];

Basically wcn firmware can propose up to 4 frequency ranges to not use
for whatever reason... prima looks like it tries to disable channels
accordingly.

-Andy

> 2015-01-18 5:11 GMT+00:00 Andy Green <andy.green@linaro.org>:
>> WCN3620 firmware introduces a new async indication, we need to
>> add it as a known message type so we can accept it
>>
>> Signed-off-by: Andy Green <andy.green@linaro.org>
>> ---
>>  drivers/net/wireless/ath/wcn36xx/hal.h |    2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/wireless/ath/wcn36xx/hal.h b/drivers/net/wireless/ath/wcn36xx/hal.h
>> index a1f1127..b947de0 100644
>> --- a/drivers/net/wireless/ath/wcn36xx/hal.h
>> +++ b/drivers/net/wireless/ath/wcn36xx/hal.h
>> @@ -345,6 +345,8 @@ enum wcn36xx_hal_host_msg_type {
>>         WCN36XX_HAL_DHCP_START_IND = 189,
>>         WCN36XX_HAL_DHCP_STOP_IND = 190,
>>
>> +       WCN36XX_HAL_AVOID_FREQ_RANGE_IND = 233,
>> +
>>         WCN36XX_HAL_MSG_MAX = WCN36XX_HAL_MSG_TYPE_MAX_ENUM_SIZE
>>  };
>>
>>
>
>
>
> --
> Best regards,
> Eugene

Re: [PATCH 6/7] net: wireless: wcn36xx: remove powersaving for wcn3620

[Bjorn Andersson]

On Sat, Jan 17, 2015 at 9:11 PM, Andy Green <andy.green@linaro.org> wrote:
> WCN3620 powersaving mode is not stable.  Disable it if we're
> on a wcn3620 chip type.
>
> Signed-off-by: Andy Green <andy.green@linaro.org>
> ---
>  drivers/net/wireless/ath/wcn36xx/main.c |    4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
> index c4178c7..569d45b 100644
> --- a/drivers/net/wireless/ath/wcn36xx/main.c
> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
> @@ -924,6 +924,10 @@ static int wcn36xx_init_ieee80211(struct wcn36xx *wcn)
>                 IEEE80211_HW_AMPDU_AGGREGATION |
>                 IEEE80211_HW_TIMING_BEACON_ONLY;
>
> +       /* 3620 powersaving currently unstable */
> +       if (wcn->chip_version == WCN36XX_CHIP_3620)
> +               wcn->hw->flags &= ~IEEE80211_HW_SUPPORTS_PS;
> +

Hi Andy,

I have the same problem (Data Abort Exception in wcnss) on 3680, this
with my wip smd code and a hacked up remoteproc-pil. I haven't spent
any effort on debugging this, but it looks like it's something related
to what we have ported to mainline (or lack thereof) rather than a
3620 specific issue.

Regards,
Bjorn

Re: [PATCH 6/7] net: wireless: wcn36xx: remove powersaving for wcn3620

[Bjorn Andersson]

On Sat, Jan 17, 2015 at 9:11 PM, Andy Green <andy.green-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> WCN3620 powersaving mode is not stable.  Disable it if we're
> on a wcn3620 chip type.
>
> Signed-off-by: Andy Green <andy.green-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  drivers/net/wireless/ath/wcn36xx/main.c |    4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
> index c4178c7..569d45b 100644
> --- a/drivers/net/wireless/ath/wcn36xx/main.c
> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
> @@ -924,6 +924,10 @@ static int wcn36xx_init_ieee80211(struct wcn36xx *wcn)
>                 IEEE80211_HW_AMPDU_AGGREGATION |
>                 IEEE80211_HW_TIMING_BEACON_ONLY;
>
> +       /* 3620 powersaving currently unstable */
> +       if (wcn->chip_version == WCN36XX_CHIP_3620)
> +               wcn->hw->flags &= ~IEEE80211_HW_SUPPORTS_PS;
> +

Hi Andy,

I have the same problem (Data Abort Exception in wcnss) on 3680, this
with my wip smd code and a hacked up remoteproc-pil. I haven't spent
any effort on debugging this, but it looks like it's something related
to what we have ported to mainline (or lack thereof) rather than a
3620 specific issue.

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/7] net: wireless: wcn36xx: remove powersaving for wcn3620

[Andy Green]

On 10 February 2015 at 01:54, Bjorn Andersson <bjorn@kryo.se> wrote:
> On Sat, Jan 17, 2015 at 9:11 PM, Andy Green <andy.green@linaro.org> wrote:
>> WCN3620 powersaving mode is not stable.  Disable it if we're
>> on a wcn3620 chip type.
>>
>> Signed-off-by: Andy Green <andy.green@linaro.org>
>> ---
>>  drivers/net/wireless/ath/wcn36xx/main.c |    4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
>> index c4178c7..569d45b 100644
>> --- a/drivers/net/wireless/ath/wcn36xx/main.c
>> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
>> @@ -924,6 +924,10 @@ static int wcn36xx_init_ieee80211(struct wcn36xx *wcn)
>>                 IEEE80211_HW_AMPDU_AGGREGATION |
>>                 IEEE80211_HW_TIMING_BEACON_ONLY;
>>
>> +       /* 3620 powersaving currently unstable */
>> +       if (wcn->chip_version == WCN36XX_CHIP_3620)
>> +               wcn->hw->flags &= ~IEEE80211_HW_SUPPORTS_PS;
>> +
>
> Hi Andy,
>
> I have the same problem (Data Abort Exception in wcnss) on 3680, this
> with my wip smd code and a hacked up remoteproc-pil. I haven't spent
> any effort on debugging this, but it looks like it's something related
> to what we have ported to mainline (or lack thereof) rather than a
> 3620 specific issue.

Makes sense since I only have 3620, I assumed it's related to that.

However he can work for a while with powersaving on, but ping
latencies are +600-800ms and he's shaky.

Later I found something mac80211 generic scan preparation or post-scan
code (for scan initiated by wpa_supplicant) is able to stop wlan
traffic after a few goes even if the actual scan mode smd is not sent.

At that point I think a nice solution would be a donation of time from
guys who specialize in wcn for a living to come and hand out a pony or
two...

-Andy

> Regards,
> Bjorn

Re: [PATCH 6/7] net: wireless: wcn36xx: remove powersaving for wcn3620

[Andy Green]

On 10 February 2015 at 01:54, Bjorn Andersson <bjorn-UYDU3/A3LUY@public.gmane.org> wrote:
> On Sat, Jan 17, 2015 at 9:11 PM, Andy Green <andy.green-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> WCN3620 powersaving mode is not stable.  Disable it if we're
>> on a wcn3620 chip type.
>>
>> Signed-off-by: Andy Green <andy.green-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>  drivers/net/wireless/ath/wcn36xx/main.c |    4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c b/drivers/net/wireless/ath/wcn36xx/main.c
>> index c4178c7..569d45b 100644
>> --- a/drivers/net/wireless/ath/wcn36xx/main.c
>> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
>> @@ -924,6 +924,10 @@ static int wcn36xx_init_ieee80211(struct wcn36xx *wcn)
>>                 IEEE80211_HW_AMPDU_AGGREGATION |
>>                 IEEE80211_HW_TIMING_BEACON_ONLY;
>>
>> +       /* 3620 powersaving currently unstable */
>> +       if (wcn->chip_version == WCN36XX_CHIP_3620)
>> +               wcn->hw->flags &= ~IEEE80211_HW_SUPPORTS_PS;
>> +
>
> Hi Andy,
>
> I have the same problem (Data Abort Exception in wcnss) on 3680, this
> with my wip smd code and a hacked up remoteproc-pil. I haven't spent
> any effort on debugging this, but it looks like it's something related
> to what we have ported to mainline (or lack thereof) rather than a
> 3620 specific issue.

Makes sense since I only have 3620, I assumed it's related to that.

However he can work for a while with powersaving on, but ping
latencies are +600-800ms and he's shaky.

Later I found something mac80211 generic scan preparation or post-scan
code (for scan initiated by wpa_supplicant) is able to stop wlan
traffic after a few goes even if the actual scan mode smd is not sent.

At that point I think a nice solution would be a donation of time from
guys who specialize in wcn for a living to come and hand out a pony or
two...

-Andy

> Regards,
> Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/7] net: wireless: wcn36xx: remove powersaving for wcn3620

[Andy Green]

On 10 February 2015 at 05:11, Bjorn Andersson <bjorn@kryo.se> wrote:
> On Feb 9, 2015 1:07 PM, "Andy Green" <andy.green@linaro.org> wrote:
>>
>> On 10 February 2015 at 01:54, Bjorn Andersson <bjorn@kryo.se> wrote:
>> > On Sat, Jan 17, 2015 at 9:11 PM, Andy Green <andy.green@linaro.org>
>> > wrote:
>> >> WCN3620 powersaving mode is not stable.  Disable it if we're
>> >> on a wcn3620 chip type.
>> >>
>> >> Signed-off-by: Andy Green <andy.green@linaro.org>
>> >> ---
>> >>  drivers/net/wireless/ath/wcn36xx/main.c |    4 ++++
>> >>  1 file changed, 4 insertions(+)
>> >>
>> >> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c
>> >> b/drivers/net/wireless/ath/wcn36xx/main.c
>> >> index c4178c7..569d45b 100644
>> >> --- a/drivers/net/wireless/ath/wcn36xx/main.c
>> >> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
>> >> @@ -924,6 +924,10 @@ static int wcn36xx_init_ieee80211(struct wcn36xx
>> >> *wcn)
>> >>                 IEEE80211_HW_AMPDU_AGGREGATION |
>> >>                 IEEE80211_HW_TIMING_BEACON_ONLY;
>> >>
>> >> +       /* 3620 powersaving currently unstable */
>> >> +       if (wcn->chip_version == WCN36XX_CHIP_3620)
>> >> +               wcn->hw->flags &= ~IEEE80211_HW_SUPPORTS_PS;
>> >> +
>> >
>> > Hi Andy,
>> >
>> > I have the same problem (Data Abort Exception in wcnss) on 3680, this
>> > with my wip smd code and a hacked up remoteproc-pil. I haven't spent
>> > any effort on debugging this, but it looks like it's something related
>> > to what we have ported to mainline (or lack thereof) rather than a
>> > 3620 specific issue.
>>
>> Makes sense since I only have 3620, I assumed it's related to that.
>>
>> However he can work for a while with powersaving on, but ping
>> latencies are +600-800ms and he's shaky.
>>
>> Later I found something mac80211 generic scan preparation or post-scan
>> code (for scan initiated by wpa_supplicant) is able to stop wlan
>> traffic after a few goes even if the actual scan mode smd is not sent.
>>
>> At that point I think a nice solution would be a donation of time from
>> guys who specialize in wcn for a living to come and hand out a pony or
>> two...
>>
>
> I agree, my goal is that we get this running in mainline (smd, smsm, smp2p
> and remoteproc-tz) so that people with the domain knowledge can go in and
> make it work well.

Sounds great thanks.

I also tried hostapd which is fully workable without encryption, plus
or minus the instabilities mentioned.  But for wpa, he chokes trying
to inject a packet.  So it's another pony needed from somewhere.

Can I ask if smdtty will also appear?  I uplevelled and hacked smdtty
a bit from a 3.10 reference tree for 8916-qrd, and I was able to get
wcn3620 BT working stably for BT keyboard + mouse and even ad2p.

However the hack bound together smdtty ch2 + 3 in smdtty driver and
made it understand about the missing hci protocol byte... this is far
from reasonable for upstream, but it works like the 3.10 except needs
no special bluez / userland treatment.  So I'm curious if no smdtty
how the split smd hci / acl link in firmware will appear coherently to
userspace as a normal uart.

-Andy

> Regards,
> Bjorn

Re: [PATCH 6/7] net: wireless: wcn36xx: remove powersaving for wcn3620

[Andy Green]

On 10 February 2015 at 05:11, Bjorn Andersson <bjorn-UYDU3/A3LUY@public.gmane.org> wrote:
> On Feb 9, 2015 1:07 PM, "Andy Green" <andy.green-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>
>> On 10 February 2015 at 01:54, Bjorn Andersson <bjorn-UYDU3/A3LUY@public.gmane.org> wrote:
>> > On Sat, Jan 17, 2015 at 9:11 PM, Andy Green <andy.green-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> > wrote:
>> >> WCN3620 powersaving mode is not stable.  Disable it if we're
>> >> on a wcn3620 chip type.
>> >>
>> >> Signed-off-by: Andy Green <andy.green-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> >> ---
>> >>  drivers/net/wireless/ath/wcn36xx/main.c |    4 ++++
>> >>  1 file changed, 4 insertions(+)
>> >>
>> >> diff --git a/drivers/net/wireless/ath/wcn36xx/main.c
>> >> b/drivers/net/wireless/ath/wcn36xx/main.c
>> >> index c4178c7..569d45b 100644
>> >> --- a/drivers/net/wireless/ath/wcn36xx/main.c
>> >> +++ b/drivers/net/wireless/ath/wcn36xx/main.c
>> >> @@ -924,6 +924,10 @@ static int wcn36xx_init_ieee80211(struct wcn36xx
>> >> *wcn)
>> >>                 IEEE80211_HW_AMPDU_AGGREGATION |
>> >>                 IEEE80211_HW_TIMING_BEACON_ONLY;
>> >>
>> >> +       /* 3620 powersaving currently unstable */
>> >> +       if (wcn->chip_version == WCN36XX_CHIP_3620)
>> >> +               wcn->hw->flags &= ~IEEE80211_HW_SUPPORTS_PS;
>> >> +
>> >
>> > Hi Andy,
>> >
>> > I have the same problem (Data Abort Exception in wcnss) on 3680, this
>> > with my wip smd code and a hacked up remoteproc-pil. I haven't spent
>> > any effort on debugging this, but it looks like it's something related
>> > to what we have ported to mainline (or lack thereof) rather than a
>> > 3620 specific issue.
>>
>> Makes sense since I only have 3620, I assumed it's related to that.
>>
>> However he can work for a while with powersaving on, but ping
>> latencies are +600-800ms and he's shaky.
>>
>> Later I found something mac80211 generic scan preparation or post-scan
>> code (for scan initiated by wpa_supplicant) is able to stop wlan
>> traffic after a few goes even if the actual scan mode smd is not sent.
>>
>> At that point I think a nice solution would be a donation of time from
>> guys who specialize in wcn for a living to come and hand out a pony or
>> two...
>>
>
> I agree, my goal is that we get this running in mainline (smd, smsm, smp2p
> and remoteproc-tz) so that people with the domain knowledge can go in and
> make it work well.

Sounds great thanks.

I also tried hostapd which is fully workable without encryption, plus
or minus the instabilities mentioned.  But for wpa, he chokes trying
to inject a packet.  So it's another pony needed from somewhere.

Can I ask if smdtty will also appear?  I uplevelled and hacked smdtty
a bit from a 3.10 reference tree for 8916-qrd, and I was able to get
wcn3620 BT working stably for BT keyboard + mouse and even ad2p.

However the hack bound together smdtty ch2 + 3 in smdtty driver and
made it understand about the missing hci protocol byte... this is far
from reasonable for upstream, but it works like the 3.10 except needs
no special bluez / userland treatment.  So I'm curious if no smdtty
how the split smd hci / acl link in firmware will appear coherently to
userspace as a normal uart.

-Andy

> Regards,
> Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/7] net: wireless: wcn36xx: remove powersaving for wcn3620

[Bjorn Andersson]

On Mon, Feb 9, 2015 at 1:28 PM, Andy Green <andy.green@linaro.org> wrote:
> On 10 February 2015 at 05:11, Bjorn Andersson <bjorn@kryo.se> wrote:
>> On Feb 9, 2015 1:07 PM, "Andy Green" <andy.green@linaro.org> wrote:
>>>
>>> On 10 February 2015 at 01:54, Bjorn Andersson <bjorn@kryo.se> wrote:
>>> > On Sat, Jan 17, 2015 at 9:11 PM, Andy Green <andy.green@linaro.org>
[..]
>>> At that point I think a nice solution would be a donation of time from
>>> guys who specialize in wcn for a living to come and hand out a pony or
>>> two...
>>>
>>
>> I agree, my goal is that we get this running in mainline (smd, smsm, smp2p
>> and remoteproc-tz) so that people with the domain knowledge can go in and
>> make it work well.
>
[..]
>
> Can I ask if smdtty will also appear?  I uplevelled and hacked smdtty
> a bit from a 3.10 reference tree for 8916-qrd, and I was able to get
> wcn3620 BT working stably for BT keyboard + mouse and even ad2p.
>
> However the hack bound together smdtty ch2 + 3 in smdtty driver and
> made it understand about the missing hci protocol byte... this is far
> from reasonable for upstream, but it works like the 3.10 except needs
> no special bluez / userland treatment.  So I'm curious if no smdtty
> how the split smd hci / acl link in firmware will appear coherently to
> userspace as a normal uart.
>

I'm not entirely sure how we're to proceed with this one.

In msm-3.4 with BlueZ the kernel has a driver named hci_smd, that
consumes the two channels and register with the hci core.

In msm-3.10 Qualcomm have dropped this, because they are running
bluedroid which instead consumes the two smd channels in userspace
(through the smd_pkt driver).

We need smd_pkt for modem related matters anyways, so on that we can
run bluedroid and we could bring in hci_smd for use with BlueZ. But
then we would have to configure the kernel based on what stack we want
to run on top.

So we should probably look into userspace and try to consolidate
things there before deciding where to take this.

Regards,
Bjorn

Re: [PATCH 6/7] net: wireless: wcn36xx: remove powersaving for wcn3620

[Andy Green]

On 10 February 2015 at 05:40, Bjorn Andersson <bjorn@kryo.se> wrote:
> On Mon, Feb 9, 2015 at 1:28 PM, Andy Green <andy.green@linaro.org> wrote:
>> On 10 February 2015 at 05:11, Bjorn Andersson <bjorn@kryo.se> wrote:
>>> On Feb 9, 2015 1:07 PM, "Andy Green" <andy.green@linaro.org> wrote:
>>>>
>>>> On 10 February 2015 at 01:54, Bjorn Andersson <bjorn@kryo.se> wrote:
>>>> > On Sat, Jan 17, 2015 at 9:11 PM, Andy Green <andy.green@linaro.org>
> [..]
>>>> At that point I think a nice solution would be a donation of time from
>>>> guys who specialize in wcn for a living to come and hand out a pony or
>>>> two...
>>>>
>>>
>>> I agree, my goal is that we get this running in mainline (smd, smsm, smp2p
>>> and remoteproc-tz) so that people with the domain knowledge can go in and
>>> make it work well.
>>
> [..]
>>
>> Can I ask if smdtty will also appear?  I uplevelled and hacked smdtty
>> a bit from a 3.10 reference tree for 8916-qrd, and I was able to get
>> wcn3620 BT working stably for BT keyboard + mouse and even ad2p.
>>
>> However the hack bound together smdtty ch2 + 3 in smdtty driver and
>> made it understand about the missing hci protocol byte... this is far
>> from reasonable for upstream, but it works like the 3.10 except needs
>> no special bluez / userland treatment.  So I'm curious if no smdtty
>> how the split smd hci / acl link in firmware will appear coherently to
>> userspace as a normal uart.
>>
>
> I'm not entirely sure how we're to proceed with this one.
>
> In msm-3.4 with BlueZ the kernel has a driver named hci_smd, that
> consumes the two channels and register with the hci core.
>
> In msm-3.10 Qualcomm have dropped this, because they are running
> bluedroid which instead consumes the two smd channels in userspace
> (through the smd_pkt driver).
>
> We need smd_pkt for modem related matters anyways, so on that we can
> run bluedroid and we could bring in hci_smd for use with BlueZ. But
> then we would have to configure the kernel based on what stack we want
> to run on top.
>
> So we should probably look into userspace and try to consolidate
> things there before deciding where to take this.

I understand, although it's a shame to make a whole new protocol in
bluez when it's just the normal protocol with a byte trimmed at the
start of a block.  It can work with stock distro bluez out of the box
if the kernel binds the channels.

The hci_smd thing sounds okay, maybe he can be a module and Android /
bluedroid just doesn't have the module in the rootfs, traditional
Linux gets the module and udev inserts him.

-Andy

> Regards,
> Bjorn