[PATCHV2]Remove the PNO verion bit from the update scan param response to get the correcly response status

[Kalle Valo]

YanBo <dreamfly281 at gmail.com> writes:

> The update scan param status always report error cause it contain the PNO mark,
> remove it to get the correctly response status

As I don't see this on my old mako firmware I take it that this happens
only on newer firmware versions, right? It would be good to mention in
the commit log the firmware version. I suspect that in the future we
have quite a lot of work to do to handle different versions of firmware
interfaces :/

> +int wcn36xx_smd_rsp_update_scan_param_status_check(void *buf, size_t len)
> +{
> +       struct wcn36xx_fw_msg_status_rsp * rsp;
> +       if (len < sizeof(struct wcn36xx_hal_msg_header) +
> +               sizeof(struct wcn36xx_fw_msg_status_rsp))
> +               return -EIO;
> +       rsp = (struct wcn36xx_fw_msg_status_rsp *)
> +               (buf + sizeof(struct wcn36xx_hal_msg_header));
> +
> +       /* Remove the PNO version bit */
> +       rsp->status &= (~(WCN36XX_FW_MSG_PNO_VERSION_MASK));
> +
> +       if (WCN36XX_FW_MSG_RESULT_SUCCESS != rsp->status) {
> +               return -EIO;
> +       }
> +       return 0;
> +}

Like Eugene I don't understand why we need the new function.

>  int wcn36xx_smd_load_nv(struct wcn36xx *wcn)
>  {
>         struct nv_data *nv_d;
> @@ -298,18 +316,6 @@ int wcn36xx_smd_update_scan_params(struct wcn36xx *wcn){
>
>         return wcn36xx_smd_send_and_wait(wcn, msg_body.header.len);
>  }
> -static int wcn36xx_smd_update_scan_params_rsp(void *buf, size_t len)
> -{
> -       struct  wcn36xx_hal_update_scan_params_resp * rsp;
> -
> -       rsp = (struct wcn36xx_hal_update_scan_params_resp *)buf;
> -
> -       wcn36xx_dbg(WCN36XX_DBG_HAL,
> -                   "hal rsp update scan params status %d",
> -                   rsp->status);
> -
> -       return 0;
> -}

You could do the same just in this function.

> @@ -901,7 +907,10 @@ static void wcn36xx_smd_rsp_process(struct
> wcn36xx *wcn, void *buf, si
>                                  ze_t len)
>                 wcn36xx_smd_join_rsp(buf, len);
>                 break;
>         case WCN36XX_HAL_UPDATE_SCAN_PARAM_RSP:
> -               wcn36xx_smd_update_scan_params_rsp(buf, len);
> +               if(wcn36xx_smd_rsp_update_scan_param_status_check(buf, len)) {
> +                       wcn36xx_warn("error response from hal request %d",
> +                                    msg_header->msg_type);
> +               }
>                 break;

That way we can keep this part simple.

>         case WCN36XX_HAL_CH_SWITCH_RSP:
>                 wcn36xx_smd_switch_channel_rsp(buf,len);
> diff --git a/smd.h b/smd.h
> index 47a6645..9703e19 100644
> --- a/smd.h
> +++ b/smd.h
> @@ -27,6 +27,9 @@
>  #define SMD_MSG_TIMEOUT 200
>  #define WCN36XX_SMSM_WLAN_TX_ENABLE                    0x00000400
>  #define WCN36XX_SMSM_WLAN_TX_RINGS_EMPTY               0x00000200
> +/* The PNO version info be contained in the rsp msg */
> +#define WCN36XX_FW_MSG_PNO_VERSION_MASK                        0X8000

"0x8000" (lower case)

> +
>
>  enum wcn36xx_fw_msg_result {
>         WCN36XX_FW_MSG_RESULT_SUCCESS                   = 0,
> @@ -54,7 +57,6 @@ int wcn36xx_smd_init_scan(struct wcn36xx *wcn);
>  int wcn36xx_smd_start_scan(struct wcn36xx *wcn, int ch);
>  int wcn36xx_smd_end_scan(struct wcn36xx *wcn, int ch);
>  int wcn36xx_smd_finish_scan(struct wcn36xx *wcn);
> -int wcn36xx_smd_update_scan_params(struct wcn36xx *wcn);

Why this?

-- 
Kalle Valo