Niklas Cassel <niklas.cassel@xxxxxxxxxx> writes: > On Tue, Jun 05, 2018 at 06:03:40PM +0530, Govind Singh wrote: >> WLAN qmi server running in Q6 exposes host to target >> cold boot qmi handshakes. Add WLAN QMI service helpers >> for ath10k wcn3990 qmi client. >> >> Signed-off-by: Govind Singh <govinds@xxxxxxxxxxxxxx> >> --- >> .../net/wireless/ath/ath10k/qmi_wlfw_v01.c | 2072 +++++++++++++++++ >> .../net/wireless/ath/ath10k/qmi_wlfw_v01.h | 677 ++++++ >> 2 files changed, 2749 insertions(+) >> create mode 100644 drivers/net/wireless/ath/ath10k/qmi_wlfw_v01.c >> create mode 100644 drivers/net/wireless/ath/ath10k/qmi_wlfw_v01.h >> >> diff --git a/drivers/net/wireless/ath/ath10k/qmi_wlfw_v01.c >> b/drivers/net/wireless/ath/ath10k/qmi_wlfw_v01.c >> new file mode 100644 >> index 000000000000..ba79c2e4aed6 >> --- /dev/null >> +++ b/drivers/net/wireless/ath/ath10k/qmi_wlfw_v01.c >> @@ -0,0 +1,2072 @@ > > Hello Govind, > > Please run checkpatch on this patch (and all other patches in the series). As not all checkpatch warnings make sense I wrote a wrapper called ath10k-check which disables the warnings we don't care about. I always run that on patches on my pending branch. https://wireless.wiki.kernel.org/en/users/drivers/ath10k/codingstyle#checking_code > WARNING: Missing or malformed SPDX-License-Identifier tag in line 1 > #32: FILE: drivers/net/wireless/ath/ath10k/qmi_wlfw_v01.c:1: > +/* > > WARNING: Missing or malformed SPDX-License-Identifier tag in line 1 > #2110: FILE: drivers/net/wireless/ath/ath10k/qmi_wlfw_v01.h:1: > +/* > > If I'm not mistaken you are using the ISC license, and > the proper SPDX-License-Identifier tag would then be: > > /* SPDX-License-Identifier: ISC */ I would prefer to switch ath10k to SPDX in one go, but I cannot do that yet as ISC is not in LICENSES directory yet. So please ignore that warning for now. >> +/* >> + * 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 <linux/soc/qcom/qmi.h> >> +#include <linux/types.h> >> +#include "qmi_wlfw_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), >> + }, > > There's a lot of lines that are over 80 characters. > > WARNING: line over 80 characters > #580: FILE: drivers/net/wireless/ath/ath10k/qmi_wlfw_v01.c:549: > + .offset = offsetof(struct wlfw_ind_register_resp_msg_v01, > > WARNING: line over 80 characters > #2402: FILE: drivers/net/wireless/ath/ath10k/qmi_wlfw_v01.h:293: > + struct wlfw_shadow_reg_cfg_s_v01 shadow_reg[QMI_WLFW_MAX_NUM_SHADOW_REG_V01]; > > Perhaps all these spaces to keep the same alignment isn't needed. In ath10k-check the max line length is 90 chars, in some cases it just did not make sense to keep the 80 char limit. -- Kalle Valo