All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Suman Anna <s-anna@ti.com>
Cc: Sarangdhar Joshi <spjoshi@codeaurora.org>,
	Ohad Ben-Cohen <ohad@wizery.com>,
	Santosh Shilimkar <ssantosh@kernel.org>,
	linux-remoteproc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-arm-msm@vger.kernel.org,
	Stephen Boyd <sboyd@codeaurora.org>,
	Trilok Soni <tsoni@codeaurora.org>,
	Dave Gerlach <d-gerlach@ti.com>
Subject: Re: [PATCH 1/2] soc: ti: Use remoteproc auto_boot feature
Date: Thu, 22 Dec 2016 05:02:46 -0800	[thread overview]
Message-ID: <20161222130246.GA8359@builder> (raw)
In-Reply-To: <1f5b2631-f744-a5c1-55c1-82eb27d5cbd7@ti.com>

On Wed 21 Dec 19:16 PST 2016, Suman Anna wrote:

> Hi Sarang,
> 
> On 12/15/2016 06:03 PM, Sarangdhar Joshi wrote:
> > The function wkup_m3_rproc_boot_thread waits for asynchronous
> > firmware loading to complete successfully before calling
> > rproc_boot(). The same can be achieved by just setting
> > rproc->auto_boot flag. Change this. As a result this change
> > removes wkup_m3_rproc_boot_thread and moves m3_ipc->sync_complete
> > initialization to the wkup_m3_ipc_probe().
> > 
> > Other than the current usage, the firmware_loading_complete is
> > only used in rproc_del() where it's no longer needed.  This
> > change is in preparation for removing firmware_loading_complete
> > completely.
> 
> Based on the comments so far, I am assuming that you are dropping this
> series.
> 

Following up on those comments only revealed that we have several other
similar race conditions, so I'm hoping that Sarangdhar will continue to
work on fixing those - and in this process get rid of this completion.

> In any case, this series did break our PM stack. We definitely don't
> want to auto-boot the wkup_m3_rproc device, that responsibility will
> need to stay with the wkup_m3_ipc driver.
> 

Reviewing the wkup_m3 situation again I see that as we have moved the
resource table parsing to the rproc_boot() path there's no longer any
need for the wkup_m3_ipc driver to wait for the remoteproc-core-internal
completion.

If rproc_get_by_phandle() returns non-NULL it is initialized. We still
don't want to call rproc_boot() from wkup_m3_ipc_probe(), so let's keep
the wkup_m3_rproc_boot_thread().

Sarangdhar, could you update the wkup_m3_ipc patch to just drop the
wait_for_completion() call?

Regards,
Bjorn

WARNING: multiple messages have this Message-ID (diff)
From: bjorn.andersson@linaro.org (Bjorn Andersson)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/2] soc: ti: Use remoteproc auto_boot feature
Date: Thu, 22 Dec 2016 05:02:46 -0800	[thread overview]
Message-ID: <20161222130246.GA8359@builder> (raw)
In-Reply-To: <1f5b2631-f744-a5c1-55c1-82eb27d5cbd7@ti.com>

On Wed 21 Dec 19:16 PST 2016, Suman Anna wrote:

> Hi Sarang,
> 
> On 12/15/2016 06:03 PM, Sarangdhar Joshi wrote:
> > The function wkup_m3_rproc_boot_thread waits for asynchronous
> > firmware loading to complete successfully before calling
> > rproc_boot(). The same can be achieved by just setting
> > rproc->auto_boot flag. Change this. As a result this change
> > removes wkup_m3_rproc_boot_thread and moves m3_ipc->sync_complete
> > initialization to the wkup_m3_ipc_probe().
> > 
> > Other than the current usage, the firmware_loading_complete is
> > only used in rproc_del() where it's no longer needed.  This
> > change is in preparation for removing firmware_loading_complete
> > completely.
> 
> Based on the comments so far, I am assuming that you are dropping this
> series.
> 

Following up on those comments only revealed that we have several other
similar race conditions, so I'm hoping that Sarangdhar will continue to
work on fixing those - and in this process get rid of this completion.

> In any case, this series did break our PM stack. We definitely don't
> want to auto-boot the wkup_m3_rproc device, that responsibility will
> need to stay with the wkup_m3_ipc driver.
> 

Reviewing the wkup_m3 situation again I see that as we have moved the
resource table parsing to the rproc_boot() path there's no longer any
need for the wkup_m3_ipc driver to wait for the remoteproc-core-internal
completion.

If rproc_get_by_phandle() returns non-NULL it is initialized. We still
don't want to call rproc_boot() from wkup_m3_ipc_probe(), so let's keep
the wkup_m3_rproc_boot_thread().

Sarangdhar, could you update the wkup_m3_ipc patch to just drop the
wait_for_completion() call?

Regards,
Bjorn

  reply	other threads:[~2016-12-22 21:02 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-16  0:03 [PATCH 1/2] soc: ti: Use remoteproc auto_boot feature Sarangdhar Joshi
2016-12-16  0:03 ` Sarangdhar Joshi
2016-12-16  0:03 ` [PATCH 2/2] remoteproc: Remove firmware_loading_complete Sarangdhar Joshi
2016-12-16  0:03   ` Sarangdhar Joshi
2016-12-16  8:26   ` loic pallardy
2016-12-16  8:26     ` loic pallardy
2016-12-16  8:26     ` loic pallardy
2016-12-16 19:28     ` Bjorn Andersson
2016-12-16 19:28       ` Bjorn Andersson
2016-12-17  2:41       ` Sarangdhar Joshi
2016-12-17  2:41         ` Sarangdhar Joshi
2016-12-17  2:41         ` Sarangdhar Joshi
2016-12-22  3:16 ` [PATCH 1/2] soc: ti: Use remoteproc auto_boot feature Suman Anna
2016-12-22  3:16   ` Suman Anna
2016-12-22  3:16   ` Suman Anna
2016-12-22 13:02   ` Bjorn Andersson [this message]
2016-12-22 13:02     ` Bjorn Andersson
2016-12-23  0:07     ` Sarangdhar Joshi
2016-12-23  0:07       ` Sarangdhar Joshi
2016-12-23 16:55       ` Suman Anna
2016-12-23 16:55         ` Suman Anna
2016-12-23 16:55         ` Suman Anna
2016-12-23  0:01   ` Sarangdhar Joshi
2016-12-23  0:01     ` Sarangdhar Joshi
2016-12-23 17:05     ` Suman Anna
2016-12-23 17:05       ` Suman Anna
2016-12-23 17:05       ` Suman Anna
2016-12-23 23:57       ` Suman Anna
2016-12-23 23:57         ` Suman Anna
2016-12-23 23:57         ` Suman Anna
2017-01-03 23:52         ` Sarangdhar Joshi
2017-01-03 23:52           ` Sarangdhar Joshi

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=20161222130246.GA8359@builder \
    --to=bjorn.andersson@linaro.org \
    --cc=d-gerlach@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-remoteproc@vger.kernel.org \
    --cc=ohad@wizery.com \
    --cc=s-anna@ti.com \
    --cc=sboyd@codeaurora.org \
    --cc=spjoshi@codeaurora.org \
    --cc=ssantosh@kernel.org \
    --cc=tsoni@codeaurora.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.