All of lore.kernel.org
 help / color / mirror / Atom feed
From: Stephen Boyd <sboyd@codeaurora.org>
To: Ohad Ben-Cohen <ohad@wizery.com>
Cc: linux-kernel@vger.kernel.org, linux-omap@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] remoteproc: block premature rproc booting
Date: Mon, 04 Jun 2012 14:23:54 -0700	[thread overview]
Message-ID: <4FCD276A.80505@codeaurora.org> (raw)
In-Reply-To: <CAK=WgbaP5qSso_Eu=Oup4eqLDcorZ-qf=mysMKiEvOS=m7yLmg@mail.gmail.com>

On 05/24/12 13:11, Ohad Ben-Cohen wrote:
>
>> Would it suffice to
>> have some new rproc->state like RPROC_UNKNOWN that we set in
>> rproc_register() before adding it to the list of rprocs? If we find the
>> firmware then we set it to RPROC_READY or RPROC_REGISTERED? Then we
>> wait_for_completion() and check the state, failing if it's still in the
>> unknown state.
> That makes me think - what if we'll add the rproc to the list only
> after we find the firmware? This way we avoid this race completely.

I thought we wanted to allow both calls to proceed in parallel? If we
don't care about that then "announcing" it once the firmware is found
the first time sounds correct.

>
> Speaking of which, I was wondering whether you guys have some free
> cycles to try remoteproc out.

I want to try it out but I've not had enough free time to do so.

>
> The main reason we kept the get/put interface was to make it easier
> for you guys to adopt it, but I've been re-thinking lately whether we
> really want that interface. It's a problematic interface with
> non-negligible maintenance burden, and the code will be greatly
> simplified without it.

If nobody in the kernel is using it why keep it? If MSM needs we can add
it back when we move to rproc.

>
> Even if you guys won't be adopting virtio (of any other
> virtio-over-smd variant) - do you think you'll be able to adopt a
> similar model with which remoteproc registers, according to the fw
> capabilities, the relevant devices which then get bound to the
> relevant higher-level drivers (virtio drivers, smd drivers, etc..) ?
> That way these devices can point to the rproc context and we don't
> need any get_by_name interface.

I think we'll need to have some way to describe the resources in the
kernel when we register the rproc. We aren't going to be able to change
our firmware to have the resource section. It would probably just be one
device (the equivalent of the rpmsg device but for smd channels).
Everything else would build on top of the smd virtio device.

Getting to that point would require changing smd code to be more linux
device model friendly. We're exploring using virtio over smd (basically
virtqueues would replace smd APIs while we replace the vrings with smd
wire protocol). If that works out then we'll be able to attach the smd
virtio device to the remote proc via some in kernel resource list. Then
I imagine when the rproc registers we'll add the device directly. We
need to figure out some way to delay loading of the firmware at that
point. I guess it means we'll need to move both firmware requests to be
async and the first firmware request to get the resource table will be
skipped on MSM.

One sticking point has been the desire to shut down processors when
they're not in use and reclaim the memory. Also we would like to upgrade
the firmware without rebooting the phone. Do you have any plans for
that? It looks like the current design boots anything that is registered
and has a matching rpmsg driver. I suppose one solution is to use
modules but that looks like it disrupts other processors (I don't want
to rmmod all of rpmsg). We probably need some sort of shutdown/boot
mechanism that isn't driven entirely by the client drivers.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.


WARNING: multiple messages have this Message-ID (diff)
From: sboyd@codeaurora.org (Stephen Boyd)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] remoteproc: block premature rproc booting
Date: Mon, 04 Jun 2012 14:23:54 -0700	[thread overview]
Message-ID: <4FCD276A.80505@codeaurora.org> (raw)
In-Reply-To: <CAK=WgbaP5qSso_Eu=Oup4eqLDcorZ-qf=mysMKiEvOS=m7yLmg@mail.gmail.com>

On 05/24/12 13:11, Ohad Ben-Cohen wrote:
>
>> Would it suffice to
>> have some new rproc->state like RPROC_UNKNOWN that we set in
>> rproc_register() before adding it to the list of rprocs? If we find the
>> firmware then we set it to RPROC_READY or RPROC_REGISTERED? Then we
>> wait_for_completion() and check the state, failing if it's still in the
>> unknown state.
> That makes me think - what if we'll add the rproc to the list only
> after we find the firmware? This way we avoid this race completely.

I thought we wanted to allow both calls to proceed in parallel? If we
don't care about that then "announcing" it once the firmware is found
the first time sounds correct.

>
> Speaking of which, I was wondering whether you guys have some free
> cycles to try remoteproc out.

I want to try it out but I've not had enough free time to do so.

>
> The main reason we kept the get/put interface was to make it easier
> for you guys to adopt it, but I've been re-thinking lately whether we
> really want that interface. It's a problematic interface with
> non-negligible maintenance burden, and the code will be greatly
> simplified without it.

If nobody in the kernel is using it why keep it? If MSM needs we can add
it back when we move to rproc.

>
> Even if you guys won't be adopting virtio (of any other
> virtio-over-smd variant) - do you think you'll be able to adopt a
> similar model with which remoteproc registers, according to the fw
> capabilities, the relevant devices which then get bound to the
> relevant higher-level drivers (virtio drivers, smd drivers, etc..) ?
> That way these devices can point to the rproc context and we don't
> need any get_by_name interface.

I think we'll need to have some way to describe the resources in the
kernel when we register the rproc. We aren't going to be able to change
our firmware to have the resource section. It would probably just be one
device (the equivalent of the rpmsg device but for smd channels).
Everything else would build on top of the smd virtio device.

Getting to that point would require changing smd code to be more linux
device model friendly. We're exploring using virtio over smd (basically
virtqueues would replace smd APIs while we replace the vrings with smd
wire protocol). If that works out then we'll be able to attach the smd
virtio device to the remote proc via some in kernel resource list. Then
I imagine when the rproc registers we'll add the device directly. We
need to figure out some way to delay loading of the firmware at that
point. I guess it means we'll need to move both firmware requests to be
async and the first firmware request to get the resource table will be
skipped on MSM.

One sticking point has been the desire to shut down processors when
they're not in use and reclaim the memory. Also we would like to upgrade
the firmware without rebooting the phone. Do you have any plans for
that? It looks like the current design boots anything that is registered
and has a matching rpmsg driver. I suppose one solution is to use
modules but that looks like it disrupts other processors (I don't want
to rmmod all of rpmsg). We probably need some sort of shutdown/boot
mechanism that isn't driven entirely by the client drivers.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.

  reply	other threads:[~2012-06-04 21:23 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-22 11:51 [PATCH] remoteproc: block premature rproc booting Ohad Ben-Cohen
2012-05-22 11:51 ` Ohad Ben-Cohen
2012-05-22 11:51 ` Ohad Ben-Cohen
2012-05-24  9:15 ` Stephen Boyd
2012-05-24  9:15   ` Stephen Boyd
2012-05-24 20:11   ` Ohad Ben-Cohen
2012-05-24 20:11     ` Ohad Ben-Cohen
2012-06-04 21:23     ` Stephen Boyd [this message]
2012-06-04 21:23       ` Stephen Boyd
2012-06-05 10:57       ` Ohad Ben-Cohen
2012-06-05 10:57         ` Ohad Ben-Cohen
2012-06-06  3:25         ` Stephen Boyd
2012-06-06  3:25           ` Stephen Boyd
2012-06-06  5:44           ` Ohad Ben-Cohen
2012-06-06  5:44             ` Ohad Ben-Cohen
2012-06-06  5:44             ` Ohad Ben-Cohen
2012-07-02 12:25         ` Ohad Ben-Cohen
2012-07-02 12:25           ` Ohad Ben-Cohen
2012-07-02 15:15           ` Sjur BRENDELAND
2012-07-02 15:15             ` Sjur BRENDELAND
2012-07-02 15:15             ` Sjur BRENDELAND
2012-07-02 15:19             ` Ohad Ben-Cohen
2012-07-02 15:19               ` Ohad Ben-Cohen
2012-07-02 15:19               ` Ohad Ben-Cohen
     [not found] <81C3A93C17462B4BBD7E272753C10579232F86B623@EXDCVYMBSTM005.EQ1STM.local>
2012-06-29 14:56 ` Ohad Ben-Cohen
2012-07-02  6:08   ` Preetham-rao K
2012-07-02 21:10     ` Guzman Lugo, Fernando

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=4FCD276A.80505@codeaurora.org \
    --to=sboyd@codeaurora.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=ohad@wizery.com \
    /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.