ATH10K Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Norris <briannorris@chromium.org>
To: Govind Singh <govinds@codeaurora.org>
Cc: devicetree@vger.kernel.org, robh@kernel.org,
	linux-wireless@vger.kernel.org, ath10k@lists.infradead.org,
	bjorn.andersson@linaro.org, david.brown@linaro.org,
	niklas.cassel@linaro.org, andy.gross@linaro.org
Subject: Re: [PATCH v4 6/6] ath10k: Add QMI message handshake for wcn3990 client
Date: Mon, 13 Aug 2018 13:06:35 -0700	[thread overview]
Message-ID: <20180813200633.GA38014@ban.mtv.corp.google.com> (raw)
In-Reply-To: <20180723123428.12993-7-govinds@codeaurora.org>

Hi Govind,

On Mon, Jul 23, 2018 at 06:04:28PM +0530, Govind Singh wrote:
> Add WCN3990 QMI client handshakes for Q6 integrated WLAN connectivity
> subsystem. This layer is responsible for communicating qmi control
> messages to wifi fw QMI service using QMI messaging protocol.
> 
> Qualcomm MSM Interface(QMI) is a messaging format used to communicate
> between components running between remote processors with underlying
> transport layer based on integrated chipset(shared memory) or
> discrete chipset(PCI/USB/SDIO/UART).
> 
> Signed-off-by: Govind Singh <govinds@codeaurora.org>
> ---
>  drivers/net/wireless/ath/ath10k/Kconfig  |    1 +
>  drivers/net/wireless/ath/ath10k/Makefile |    4 +-
>  drivers/net/wireless/ath/ath10k/core.c   |    6 +-
>  drivers/net/wireless/ath/ath10k/core.h   |    2 +
>  drivers/net/wireless/ath/ath10k/qmi.c    | 1019 ++++++++++++++++++++++
>  drivers/net/wireless/ath/ath10k/qmi.h    |  129 +++
>  drivers/net/wireless/ath/ath10k/snoc.c   |  262 +++++-
>  drivers/net/wireless/ath/ath10k/snoc.h   |    4 +
>  8 files changed, 1417 insertions(+), 10 deletions(-)
>  create mode 100644 drivers/net/wireless/ath/ath10k/qmi.c
>  create mode 100644 drivers/net/wireless/ath/ath10k/qmi.h
> 

> diff --git a/drivers/net/wireless/ath/ath10k/qmi.c b/drivers/net/wireless/ath/ath10k/qmi.c
> new file mode 100644
> index 000000000000..d91499a7354c
> --- /dev/null
> +++ b/drivers/net/wireless/ath/ath10k/qmi.c
> @@ -0,0 +1,1019 @@

...

> +int ath10k_qmi_deinit(struct ath10k *ar)
> +{
> +	struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
> +	struct ath10k_qmi *qmi = ar_snoc->qmi;
> +
> +	cancel_work_sync(&qmi->event_work);
> +	destroy_workqueue(qmi->event_wq);

I believe this is incorrect ordering. You should not be destroying the
work queue that handles QMI event responses before you release the QMI
handle. As it stands, this means you segfault during 'rmmod'.

> +	qmi_handle_release(&qmi->qmi_hdl);

I think this should be:

	qmi_handle_release(&qmi->qmi_hdl);
	cancel_work_sync(&qmi->event_work);
	destroy_workqueue(qmi->event_wq);

> +	qmi = NULL;

This assignment is pointless, as 'qmi' is a local variable. Maybe you
meant to clear the struct member, as a precaution? So this would be:

	ar_snoc->qmi = NULL;

Brian

> +
> +	return 0;
> +} 

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

  reply	other threads:[~2018-08-13 20:06 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-23 12:34 [PATCH v4 0/6] *** Add support for wifi QMI client handshakes *** Govind Singh
2018-07-23 12:34 ` [PATCH v4 1/6] ath10k: Add qmi service helpers for wcn3990 qmi client Govind Singh
2018-07-23 12:34 ` [PATCH v4 2/6] ath10k: Add support to create boardname for non-bmi target Govind Singh
2018-07-23 12:34 ` [PATCH v4 3/6] dt: bindings: add bindings for msa memory region Govind Singh
2018-07-24 23:28   ` Rob Herring
2018-07-23 12:34 ` [PATCH v4 4/6] firmware: qcom: scm: Add WLAN VMID for Qualcomm SCM interface Govind Singh
2018-07-23 12:34 ` [PATCH v4 5/6] ath10k: Add debug mask for QMI layer Govind Singh
2018-07-23 12:34 ` [PATCH v4 6/6] ath10k: Add QMI message handshake for wcn3990 client Govind Singh
2018-08-13 20:06   ` Brian Norris [this message]
2018-08-14 11:15     ` Govind Singh
2018-07-24 20:23 ` [PATCH v4 0/6] *** Add support for wifi QMI client handshakes *** Niklas Cassel

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=20180813200633.GA38014@ban.mtv.corp.google.com \
    --to=briannorris@chromium.org \
    --cc=andy.gross@linaro.org \
    --cc=ath10k@lists.infradead.org \
    --cc=bjorn.andersson@linaro.org \
    --cc=david.brown@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=govinds@codeaurora.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=niklas.cassel@linaro.org \
    --cc=robh@kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox