ath10k.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Abhishek Kumar <kuabhs@chromium.org>
To: Stephen Boyd <swboyd@chromium.org>
Cc: Matthias Kaehlcke <mka@chromium.org>,
	Kalle Valo <kvalo@codeaurora.org>,
	 LKML <linux-kernel@vger.kernel.org>,
	ath10k <ath10k@lists.infradead.org>,
	 linux-wireless <linux-wireless@vger.kernel.org>,
	linux-arm-msm@vger.kernel.org,  netdev <netdev@vger.kernel.org>,
	Govind Singh <govinds@codeaurora.org>,
	 Youghandhar Chintala <youghand@codeaurora.org>,
	Rakesh Pillai <pillair@codeaurora.org>
Subject: Re: [PATCH] ath10k: Don't always treat modem stop events as crashes
Date: Wed, 8 Sep 2021 15:37:07 -0700	[thread overview]
Message-ID: <CACTWRwsRLrKHRWVoHHyrU2DEc_VkhqSi66tdD2OBWs_y8J2LPw@mail.gmail.com> (raw)
In-Reply-To: <CAE-0n52d_GBh70pSDXTrVkD5S6akP4O9YcE4tVRKZcvLtLZSmg@mail.gmail.com>

The way I see this, this issue is happening because of a much bigger
design constraint. On integrated(modem+wifi) solutions, if remote proc
service is shutdown, it has a trickle down effect. It deinitializes
the modem processor (which controls wifi as its subsystem) and the
wifi FW. As the fw de-initializes, we see the Delete server generated
by the wifi FW. The FW generates delete server qmi event and there is
no way on the host wifi driver to differentiate between delete event
generated from a FW crash and de-initialization due to remoteproc and
so we see the FW crash logs even during shutdown.

I would like to know what are Qualcomm's views on the design
constraint and any plans to reduce the rigidity. Also, will the FW
broadcast other qmi events(QRTR_TYPE_BYE for e.g.) during shutdown
(different from crash) ? If so then we can utilize that event and then
we don't even have to add dependency on remoteproc.

Overall this change should fix the issue, additionally I have one
comment below and would like other reviewers views.

>  #include <linux/regulator/consumer.h>
> +#include <linux/remoteproc/qcom_rproc.h>
>  #include <linux/of_address.h>
We are adding an external dependency here but since this is added in
snoc.c (which is for integrated solution only), I can expect if SNOC
is enabled, remote proc will be enabled as well, so it should be fine.

Reviewed-by: Abhishek Kumar <kuabhs@chromium.org>

On Tue, Sep 7, 2021 at 12:48 PM Stephen Boyd <swboyd@chromium.org> wrote:
>
> Quoting Matthias Kaehlcke (2021-09-07 12:32:59)
> > On Sun, Sep 05, 2021 at 02:04:00PM -0700, Stephen Boyd wrote:
> > > @@ -1740,10 +1805,19 @@ static int ath10k_snoc_probe(struct platform_device *pdev)
> > >               goto err_fw_deinit;
> > >       }
> > >
> > > +     ret = ath10k_modem_init(ar);
> > > +     if (ret) {
> > > +             ath10k_err(ar, "failed to initialize modem notifier: %d\n", ret);
> >
> > nit: ath10k_modem_init() encapsulates/hides the setup of the notifier,
> > the error message should be inside the function, as for _deinit()
>
> Sure. I can fix it. I was also wondering if I should drop the debug
> prints for the cases that don't matter in the switch statement but I'll
> just leave that alone unless someone complains about it.

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

  reply	other threads:[~2021-09-08 22:38 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-05 21:04 Stephen Boyd
2021-09-06  0:43 ` Steev Klimaszewski
2021-09-07 19:32 ` Matthias Kaehlcke
2021-09-07 19:48   ` Stephen Boyd
2021-09-08 22:37     ` Abhishek Kumar [this message]
2021-09-09  0:21       ` Stephen Boyd
     [not found] <002501d7af73$ae0a7620$0a1f6260$@codeaurora.org>
2021-09-22 22:20 ` Stephen Boyd
2021-09-24  7:59   ` Kalle Valo
2021-09-24  8:07     ` pillair

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=CACTWRwsRLrKHRWVoHHyrU2DEc_VkhqSi66tdD2OBWs_y8J2LPw@mail.gmail.com \
    --to=kuabhs@chromium.org \
    --cc=ath10k@lists.infradead.org \
    --cc=govinds@codeaurora.org \
    --cc=kvalo@codeaurora.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=mka@chromium.org \
    --cc=netdev@vger.kernel.org \
    --cc=pillair@codeaurora.org \
    --cc=swboyd@chromium.org \
    --cc=youghand@codeaurora.org \
    --subject='Re: [PATCH] ath10k: Don'\''t always treat modem stop events as crashes' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).