Re: [PATCH 01/12] ath10k: Add qmi service for wlan qmi client

[Kalle Valo]

Bjorn Andersson  writes:

>> new file mode 100644
>> index 000..5857c0c
>> --- /dev/null
>> +++ b/drivers/net/wireless/ath/ath10k/qmi_svc_v01.c
>> @@ -0,0 +1,2323 @@
>> +/*
>> + * Copyright (c) 2018 The Linux Foundation. All rights reserved.
>> + *
>> + * Permission to use, copy, modify, and/or distribute this software for any
>> + * purpose with or without fee is hereby granted, provided that the above
>> + * copyright notice and this permission notice appear in all copies.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
>> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
>> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
>> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
>> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
>> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
>> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
>> + */
>
> Please use SPDX license header.

As ath10k does not use SPDX yet so for consistency I would like to use
the old style license text still and later switch the whole ath10k to
use SPDX in one go.

-- 
Kalle Valo

Re: [PATCH 01/12] ath10k: Add qmi service for wlan qmi client

[Bjorn Andersson]

On Sun 25 Mar 22:38 PDT 2018, Govind Singh wrote:

> WLAN qmi server running in Q6 dsp exposes host
> to target cold boot qmi handshake.
> Add WLAN QMI service helpers for WLAN serivice.
> 
> Signed-off-by: Govind Singh 
> ---
>  drivers/net/wireless/ath/ath10k/qmi_svc_v01.c | 2323 
> +
>  drivers/net/wireless/ath/ath10k/qmi_svc_v01.h |  685 
>  2 files changed, 3008 insertions(+)
>  create mode 100644 drivers/net/wireless/ath/ath10k/qmi_svc_v01.c
>  create mode 100644 drivers/net/wireless/ath/ath10k/qmi_svc_v01.h
> 
> diff --git a/drivers/net/wireless/ath/ath10k/qmi_svc_v01.c 
> b/drivers/net/wireless/ath/ath10k/qmi_svc_v01.c

This is not the one and only "QMI service version 1", so perhaps name
the file qmi_wlfw_v01.c or something like that?

> new file mode 100644
> index 000..5857c0c
> --- /dev/null
> +++ b/drivers/net/wireless/ath/ath10k/qmi_svc_v01.c
> @@ -0,0 +1,2323 @@
> +/*
> + * Copyright (c) 2018 The Linux Foundation. All rights reserved.
> + *
> + * Permission to use, copy, modify, and/or distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */

Please use SPDX license header.

> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include "qmi_svc_v01.h"

At least completion, mutex and idr are unused.

> +
> +static struct qmi_elem_info wlfw_ce_tgt_pipe_cfg_s_v01_ei[] = {
> + {
> + .data_type  = QMI_UNSIGNED_4_BYTE,
> + .elem_len   = 1,
> + .elem_size  = sizeof(u32),
> + .array_type = NO_ARRAY,
> + .tlv_type   = 0,
> + .offset = offsetof(struct wlfw_ce_tgt_pipe_cfg_s_v01,
> +pipe_num),
> + },
[..]
> + {
> + .data_type  = QMI_EOTI,
> + .array_type = NO_ARRAY,
> + .tlv_type   = QMI_COMMON_TLV_TYPE,
> + },

I changed the value of QMI_EOTI so this last entry would better be
written as {}. But I presume this requires your code generator to
change.

> +};
[..]
> diff --git a/drivers/net/wireless/ath/ath10k/qmi_svc_v01.h 
> b/drivers/net/wireless/ath/ath10k/qmi_svc_v01.h
> new file mode 100644
> index 000..5e00bfe
> --- /dev/null
> +++ b/drivers/net/wireless/ath/ath10k/qmi_svc_v01.h
> @@ -0,0 +1,685 @@
> +/*
> + * Copyright (c) 2018 The Linux Foundation. All rights reserved.
> + *
> + * Permission to use, copy, modify, and/or distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */

SPDX license header please.

[..]
> +struct wlfw_bdf_download_req_msg_v01 {
> + u8 valid;
> + u8 file_id_valid;
> + enum wlfw_cal_temp_id_enum_v01 file_id;

I would prefer that we replace these enums with a fixed size data type,
and remove the INT_MIN/INT_MAX hack that tricks the compiler into giving
the enum a particular size.

> + u8 total_size_valid;
> + u32 total_size;
> + u8 seg_id_valid;
> + u32 seg_id;
> + u8 data_valid;
> + u32 data_len;
> + u8 data[QMI_WLFW_MAX_DATA_SIZE_V01];
> + u8 end_valid;
> + u8 end;
> + u8 bdf_type_valid;
> + u8 bdf_type;
> +};

Regards,
Bjorn

[PATCH 01/12] ath10k: Add qmi service for wlan qmi client

[Govind Singh]

WLAN qmi server running in Q6 dsp exposes host
to target cold boot qmi handshake.
Add WLAN QMI service helpers for WLAN serivice.

Signed-off-by: Govind Singh 
---
 drivers/net/wireless/ath/ath10k/qmi_svc_v01.c | 2323 +
 drivers/net/wireless/ath/ath10k/qmi_svc_v01.h |  685 
 2 files changed, 3008 insertions(+)
 create mode 100644 drivers/net/wireless/ath/ath10k/qmi_svc_v01.c
 create mode 100644 drivers/net/wireless/ath/ath10k/qmi_svc_v01.h

diff --git a/drivers/net/wireless/ath/ath10k/qmi_svc_v01.c 
b/drivers/net/wireless/ath/ath10k/qmi_svc_v01.c
new file mode 100644
index 000..5857c0c
--- /dev/null
+++ b/drivers/net/wireless/ath/ath10k/qmi_svc_v01.c
@@ -0,0 +1,2323 @@
+/*
+ * Copyright (c) 2018 The Linux Foundation. All rights reserved.
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "qmi_svc_v01.h"
+
+static struct qmi_elem_info wlfw_ce_tgt_pipe_cfg_s_v01_ei[] = {
+   {
+   .data_type  = QMI_UNSIGNED_4_BYTE,
+   .elem_len   = 1,
+   .elem_size  = sizeof(u32),
+   .array_type = NO_ARRAY,
+   .tlv_type   = 0,
+   .offset = offsetof(struct wlfw_ce_tgt_pipe_cfg_s_v01,
+  pipe_num),
+   },
+   {
+   .data_type  = QMI_SIGNED_4_BYTE_ENUM,
+   .elem_len   = 1,
+   .elem_size  = sizeof(enum wlfw_pipedir_enum_v01),
+   .array_type = NO_ARRAY,
+   .tlv_type   = 0,
+   .offset = offsetof(struct wlfw_ce_tgt_pipe_cfg_s_v01,
+  pipe_dir),
+   },
+   {
+   .data_type  = QMI_UNSIGNED_4_BYTE,
+   .elem_len   = 1,
+   .elem_size  = sizeof(u32),
+   .array_type = NO_ARRAY,
+   .tlv_type   = 0,
+   .offset = offsetof(struct wlfw_ce_tgt_pipe_cfg_s_v01,
+  nentries),
+   },
+   {
+   .data_type  = QMI_UNSIGNED_4_BYTE,
+   .elem_len   = 1,
+   .elem_size  = sizeof(u32),
+   .array_type = NO_ARRAY,
+   .tlv_type   = 0,
+   .offset = offsetof(struct wlfw_ce_tgt_pipe_cfg_s_v01,
+  nbytes_max),
+   },
+   {
+   .data_type  = QMI_UNSIGNED_4_BYTE,
+   .elem_len   = 1,
+   .elem_size  = sizeof(u32),
+   .array_type = NO_ARRAY,
+   .tlv_type   = 0,
+   .offset = offsetof(struct wlfw_ce_tgt_pipe_cfg_s_v01,
+  flags),
+   },
+   {
+   .data_type  = QMI_EOTI,
+   .array_type = NO_ARRAY,
+   .tlv_type   = QMI_COMMON_TLV_TYPE,
+   },
+};
+
+static struct qmi_elem_info wlfw_ce_svc_pipe_cfg_s_v01_ei[] = {
+   {
+   .data_type  = QMI_UNSIGNED_4_BYTE,
+   .elem_len   = 1,
+   .elem_size  = sizeof(u32),
+   .array_type = NO_ARRAY,
+   .tlv_type   = 0,
+   .offset = offsetof(struct wlfw_ce_svc_pipe_cfg_s_v01,
+  service_id),
+   },
+   {
+   .data_type  = QMI_SIGNED_4_BYTE_ENUM,
+   .elem_len   = 1,
+   .elem_size  = sizeof(enum wlfw_pipedir_enum_v01),
+   .array_type = NO_ARRAY,
+   .tlv_type   = 0,
+   .offset = offsetof(struct wlfw_ce_svc_pipe_cfg_s_v01,
+  pipe_dir),
+   },
+   {
+   .data_type  = QMI_UNSIGNED_4_BYTE,
+   .elem_len   = 1,
+   .elem_size  = sizeof(u32),
+   .array_type = NO_ARRAY,
+   .tlv_type   = 0,
+   .offset = offsetof(struct wlfw_ce_svc_pipe_cfg_s_v01,
+  pipe_num),
+   },
+   {
+