Re: [RFC 0/8] Introducing a generic AMP/IPC framework

2011-06-28 Thread Brian Swetland
On Tue, Jun 28, 2011 at 4:26 AM, Ohad Ben-Cohen  wrote:
> On Tue, Jun 28, 2011 at 12:22 AM, Grosen, Mark  wrote:
>> 2. We added a special section (resource table) that is interpreted as part
>> of the loading process. The tool that generates our simple format just
>> recognizes a named section (".resource_table"), so perhaps that could be
>> done with the PIL ELF loader.
>
> I hope that DT will completely replace that "resource table" section
> eventually. We still need to try it out (as soon as DT materializes on
> ARM/pandaboard) and see how it all fits together, but in general it
> looks like DT could provide all the necessary static resource
> configuration, and then we just need to expose it to the remote
> processor.

I'm not sure I see the benefit to pulling the resource information out
of the firmware image.  The resource information is a description of
what resources the firmware requires to work properly (it needs
certain amounts of working memory, timers, peripheral interfaces like
i2c to control camera hw, etc), which will be specific to a given
firmware build.  Pulling that information out into another place adds
new ways for things to break as the firmware and its resource
requirements are now separate and could get out of sync, etc.

Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 0/8] Introducing a generic AMP/IPC framework

2011-06-28 Thread Ohad Ben-Cohen
On Tue, Jun 28, 2011 at 12:22 AM, Grosen, Mark  wrote:
> 2. We added a special section (resource table) that is interpreted as part
> of the loading process. The tool that generates our simple format just
> recognizes a named section (".resource_table"), so perhaps that could be
> done with the PIL ELF loader.

I hope that DT will completely replace that "resource table" section
eventually. We still need to try it out (as soon as DT materializes on
ARM/pandaboard) and see how it all fits together, but in general it
looks like DT could provide all the necessary static resource
configuration, and then we just need to expose it to the remote
processor.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [RFC 0/8] Introducing a generic AMP/IPC framework

2011-06-27 Thread Grosen, Mark
> From: Ohad Ben-Cohen
> Sent: Saturday, June 25, 2011 6:12 PM
> 
> Hi Stephen,
> 
> On Fri, Jun 24, 2011 at 11:16 PM, Stephen Boyd  wrote:
> > Instead of devising a new firmware format, we decided
> > to just stick with elf and parse the headers in the kernel because we
> > needed them for authentication anyway. Is this reason enough to move to
> > an ELF format instead?
> 
> I think that consolidation of code is enough reason to make an effort.
> I know that our firmware format was chosen for simplicity, but I'm not
> sure if we have the tools yet to build standard ELF files for the
> remote processors (IIRC it's in the works though). I'll let Mark
> comment this one.

Yes, we are converting from "standard" ELF to the simple format. I've used the
GNU binutils to work with our ELF files. There were a few motivations:

1. Concern about complexity of parsing ELF files in kernel; however, the PIL
implementation looks pretty clean to me.

2. We added a special section (resource table) that is interpreted as part
of the loading process. The tool that generates our simple format just
recognizes a named section (".resource_table"), so perhaps that could be
done with the PIL ELF loader.

3. Smaller firmware file sizes. Our ELF files are large relative to the
payload, but this might be addressed by a better ELF "strip" utility.

> > Another difference is inter-processor dependencies. For example, on
> > msm8660 the modem can't boot until the dsp has been booted. I suppose we
> > could hide this detail in the platform specific get() implementation by
> > calling rproc_get() on the dependent processor (hopefully no locking
> > issues arise). I'd rather have it built into the core though as it isn't
> > really specific to the hardware.
> 
> No problems, I'm sure we can solve this one easily.
> 
> > If we can resolve these differences I think we can easily support remote
> > processor boot on MSM via remoteproc.
> 
> That'd be very cool, I sure do hope we can work together.

Yes, I hope we can merge our efforts on PIL and remoteproc since they seem
quite close in function and design.

Mark
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 0/8] Introducing a generic AMP/IPC framework

2011-06-25 Thread Brian Swetland
On Sat, Jun 25, 2011 at 6:11 PM, Ohad Ben-Cohen  wrote:
> Hi Stephen,
>
> On Fri, Jun 24, 2011 at 11:16 PM, Stephen Boyd  wrote:
>> This sounds a lot like SMD (shared memory driver) on MSM. The main
>> difference I see is that SMD uses the platform bus instead of the virtio
>> bus and it has its own protocol for channel allocation.
>
> Yeah, virtio is a key factor in this work; it was suggested to us by
> Arnd at the AMP plumbers discussions last year, where it was apparent
> that many vendors have their own IPC drivers/buses/channels over
> shared memory with some vendor-ish binary protocol. I must say we
> really liked virtio: it considerably simplified the code (we're not
> adding any new binary protocol), it's very nicely optimized and
> flexible, and it comes with a set of virtio drivers (e.g. network,
> console, block) so we don't have to write our own.
>
> We also cared about adding this functionality as an IPC bus, so the
> driver core will help matching drivers to channels - it simplified the
> code (in both setup and tear down of channels) and kept it flexible.
> It will also facilitate error recovery (on remote crash, we just
> remove the virtio device, and then the driver core will in turn start
> ->remove()ing the rpmsg drivers) and power management (via runtime
> PM).
>
> About SMD: I'm not familiar with it too much, but Brian naturally is
> (just for the sake of everyone who are not reading headers - Brian
> Swetland wrote the Linux SMD driver, and is also an author of this
> Google+TI joint work).

rpmsg definitely shares some design features with SMD (given that I
wrote the linux SMD driver and was involved in the design of rpmsg,
this is maybe unsurprising), but whereas in SMD we had to be
compatible with existing AMSS modems (to a degree), we had more
flexibility in the "wire protocol" on rpmsg and virtio looked like a
really nice fit that already was in the kernel.

Ohad had a number of great ideas for making the dynamic discovery,
publishing, and binding of remote services very clean and
straightforward -- I wish I had a chance to pick his brain about this
stuff back when building the SMD interfaces, which started out fairly
static, but then evolved into publishing platform devices, etc.  Of
course some of this is the benefit of hindsight.  It's easier to get
it right (er?) the second or third time around.

The TI SYS/BIOS folks were quite helpful and patient as we redesigned
the wire protocol and publishing/resource model several times along
the way here.

Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 0/8] Introducing a generic AMP/IPC framework

2011-06-25 Thread Ohad Ben-Cohen
Hi Stephen,

On Fri, Jun 24, 2011 at 11:16 PM, Stephen Boyd  wrote:
> This sounds a lot like SMD (shared memory driver) on MSM. The main
> difference I see is that SMD uses the platform bus instead of the virtio
> bus and it has its own protocol for channel allocation.

Yeah, virtio is a key factor in this work; it was suggested to us by
Arnd at the AMP plumbers discussions last year, where it was apparent
that many vendors have their own IPC drivers/buses/channels over
shared memory with some vendor-ish binary protocol. I must say we
really liked virtio: it considerably simplified the code (we're not
adding any new binary protocol), it's very nicely optimized and
flexible, and it comes with a set of virtio drivers (e.g. network,
console, block) so we don't have to write our own.

We also cared about adding this functionality as an IPC bus, so the
driver core will help matching drivers to channels - it simplified the
code (in both setup and tear down of channels) and kept it flexible.
It will also facilitate error recovery (on remote crash, we just
remove the virtio device, and then the driver core will in turn start
->remove()ing the rpmsg drivers) and power management (via runtime
PM).

About SMD: I'm not familiar with it too much, but Brian naturally is
(just for the sake of everyone who are not reading headers - Brian
Swetland wrote the Linux SMD driver, and is also an author of this
Google+TI joint work).

Btw, I'm sure SMD is conceptually not MSM-specific, and have wondered
whether you guys would like to use rpmsg/virtio (I know you have
several drivers like network/tty/etc over SMD, somewhat similarly to
virtio). Probably the biggest reason why not to is the pain in
changing the binary protocol with the modem/dsp side. If you ever do
think about it, I'd be happy to work with you to make it happen.

> This remote proc code is eerily similar to PIL (peripheral image loader,
> yes we love our acronyms) which I posted a few months back[1]. Was it
> inspiration for this patch series?

No, we weren't (or at least I wasn't) aware of PIL.

> In terms of API, s/pil/rproc/ and it would be 95% identical. There are
> some low-level differences though (see below).

Indeed, eerily similar :O

I just guess the API is so simple that probably most kernel hackers
would use refcounting get/put semantics here.

> This is an important difference between remote proc and PIL. In PIL, we
> do image authentication in addition to processor boot.

Yes, we have this too (secure code will need to authenticate the image
in some use cases) - but that's not ready yet for submission and we
decided to start off with the basics first and then evolve.

> Instead of devising a new firmware format, we decided
> to just stick with elf and parse the headers in the kernel because we
> needed them for authentication anyway. Is this reason enough to move to
> an ELF format instead?

I think that consolidation of code is enough reason to make an effort.
I know that our firmware format was chosen for simplicity, but I'm not
sure if we have the tools yet to build standard ELF files for the
remote processors (IIRC it's in the works though). I'll let Mark
comment this one.

> Another difference is inter-processor dependencies. For example, on
> msm8660 the modem can't boot until the dsp has been booted. I suppose we
> could hide this detail in the platform specific get() implementation by
> calling rproc_get() on the dependent processor (hopefully no locking
> issues arise). I'd rather have it built into the core though as it isn't
> really specific to the hardware.

No problems, I'm sure we can solve this one easily.

> If we can resolve these differences I think we can easily support remote
> processor boot on MSM via remoteproc.

That'd be very cool, I sure do hope we can work together.

Thanks for your comments !
Ohad.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 0/8] Introducing a generic AMP/IPC framework

2011-06-24 Thread Stephen Boyd
On 06/21/2011 12:18 AM, Ohad Ben-Cohen wrote:
> * Rpmsg: a virtio-based messaging bus that allows kernel drivers to
> communicate with remote processors available on the system. In turn,
> drivers could then expose appropriate user space interfaces, if needed
> (tasks running on remote processors often have direct access to sensitive
> resources like the system's physical memory, gpios, i2c buses, dma
> controllers, etc..  so one normally wouldn't want to allow userland to
> send everything/everywhere it wants).
>
> Every rpmsg device is a communication channel with a service running on a
> remote processor (thus rpmsg devices are called channels). Channels are
> identified by a textual name (which is used to match drivers to devices)
> and have a local ("source") rpmsg address, and remote ("destination") rpmsg
> address. When a driver starts listening on a channel (most commonly when it
> is probed), the bus assigns the driver a unique rpmsg src address (a 32 bit
> integer) and binds it with the driver's rx callback handler. This way
> when inbound messages arrive to this src address, the rpmsg core dispatches
> them to that driver, by invoking the driver's rx handler with the payload
> of the incoming message.
>
[snip]
>
> In addition to dynamic creation of rpmsg channels, the rpmsg bus also
> supports creation of static channels. This is needed in two cases:
> - when a certain remote processor doesn't support sending those "name
>   service" announcements. In that case, a static table of remote rpmsg
>   services must be used to create the rpmsg channels.
> - to support rpmsg server drivers, which aren't bound to a specific remote
>   rpmsg address. Instead, they just listen on a local address, waiting for
>   incoming messages. To send a message, those server drivers need to use
>   the rpmsg_sendto() API, so they can explicitly indicate the dst address
>   every time.
>

This sounds a lot like SMD (shared memory driver) on MSM. The main
difference I see is that SMD uses the platform bus instead of the virtio
bus and it has its own protocol for channel allocation.

>
> * Remoteproc: a generic driver that maintains the state of the remote
> processor(s). Simple rproc_get() and rproc_put() API is exposed, which
> drivers can use when needed (first driver to call get() will load a firmware,
> configure an iommu if needed, and boot the remote processor, while last
> driver to call put() will power it down).
>
> Hardware differences are abstracted as usual: a platform-specific driver
> registers its own start/stop handlers in the generic remoteproc driver,
> and those are invoked when its time to power up/down the processor. As a
> reference, this patch set include remoteproc support for both OMAP4's
> cortex-M3 and Davinci's DSP, tested on the pandaboard and hawkboard,
> respectively.

This remote proc code is eerily similar to PIL (peripheral image loader,
yes we love our acronyms) which I posted a few months back[1]. Was it
inspiration for this patch series?

In terms of API, s/pil/rproc/ and it would be 95% identical. There are
some low-level differences though (see below).

>
> The gory part of remoteproc is the firmware handling. We tried to come up
> with a simple binary format that will require minimum kernel code to handle,
> but at the same time be generic enough in the hopes that it would prove
> useful to others as well. We're not at all hang onto the binary format
> we picked: if there's any technical reason to change it to support other
> platforms, please let us know. We do realize that a single binary firmware
> structure might eventually not work for everyone. it did prove useful for
> us though; we adopted both the OMAP and Davinci platforms (and their
> completely different remote processor devices) to this simple binary
> structure, so we don't have to duplicate the firmware handling code.

This is an important difference between remote proc and PIL. In PIL, we
do image authentication in addition to processor boot. In this case, we
need to give the authentication engine the elf header and program
headers for the firmware so that it knows what parts of memory need to
be authenticated. Instead of devising a new firmware format, we decided
to just stick with elf and parse the headers in the kernel because we
needed them for authentication anyway. Is this reason enough to move to
an ELF format instead?

Another difference is inter-processor dependencies. For example, on
msm8660 the modem can't boot until the dsp has been booted. I suppose we
could hide this detail in the platform specific get() implementation by
calling rproc_get() on the dependent processor (hopefully no locking
issues arise). I'd rather have it built into the core though as it isn't
really specific to the hardware.

If we can resolve these differences I think we can easily support remote
processor boot on MSM via remoteproc.

[1] https://lkml.org/lkml/2011/3/9/490

-- 
Sent by an employee of the Qualcomm Innovation Center,

RE: [RFC 0/8] Introducing a generic AMP/IPC framework

2011-06-23 Thread Grosen, Mark
> From: Arnd Bergmann
> Sent: Thursday, June 23, 2011 11:47 AM
> Subject: Re: [RFC 0/8] Introducing a generic AMP/IPC framework
> 
> On Thursday 23 June 2011 18:27:10 Grosen, Mark wrote:
> > First, we are not abandoning DSPLINK. We have many users of this, even
> > though it is out-of-tree, and we will continue to support it. That said, we
> > do intend to make this new design the basis for DSPLINK-like
> > functionality. It's designed to be done "the right way" for Linux (and we
> > are looking for feedback to make it better).  It is also designed to be more
> > scalable and extensible in userspace. With a solid kernel foundation, we can
> > provide lots of functionality in userspace, or users can implement their own
> > custom solutions. One of the key things to do is map our existing DSPLINK
> > APIs, like MessageQ, to the new rpmsg transport.
> 
> Sounds all good. What about the PRUSS code? Does that fit into the new
> model as well?
> 
>   Arnd

Arnd,

Yes, I have been following some of the PRUSS discussion. I think the
remoteproc driver could be used to manage the basic load/start/stop of the
PRUSS processor. I am not sure if the virio/rpmsg part would be a good
fit. The PRUSS processor is pretty limited, so the generality of virtio
might be too much to fit and too much overhead. However, one of the good
things about remoteproc currently is that it is standalone, so other
transports could use it via the rproc_get/put methods.

Mark
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 0/8] Introducing a generic AMP/IPC framework

2011-06-23 Thread Arnd Bergmann
On Thursday 23 June 2011 18:27:10 Grosen, Mark wrote:
> First, we are not abandoning DSPLINK. We have many users of this, even
> though it is out-of-tree, and we will continue to support it. That said, we
> do intend to make this new design the basis for DSPLINK-like
> functionality. It's designed to be done "the right way" for Linux (and we
> are looking for feedback to make it better).  It is also designed to be more
> scalable and extensible in userspace. With a solid kernel foundation, we can
> provide lots of functionality in userspace, or users can implement their own
> custom solutions. One of the key things to do is map our existing DSPLINK
> APIs, like MessageQ, to the new rpmsg transport.

Sounds all good. What about the PRUSS code? Does that fit into the new model
as well?

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [RFC 0/8] Introducing a generic AMP/IPC framework

2011-06-23 Thread Grosen, Mark
> From: Michael Williamson
> Sent: Thursday, June 23, 2011 5:23 AM
...
> I'd like to kick the tires on this with a da850 based platform (MityDSP-L138).
> Any chance you might be able to share the stuff you did on the remote side
> (DSP/BIOS adaptations for rpmsg,  utils for ELF or COFF conversion to
> firmware format, etc.) for the DSP side of your tests done with the
> hawkboard?

We have only implemented the remoteproc (load, start, stop) part for
OMAPL138 so far, i.e., not the rpmsg part (communications). I am not sure
when we will get to the rpmsg part.

We will be publishing a Git tree soon with the "remote processor" code under
a BSD license. This code works with our TI RTOS, SYS/BIOS (also BSD), on
both M3 and DSP CPUs. This will include the utility to generate the RPRC
firmware format from an ELF file. Note that the Linux side does not have any
explicit binding or dependency on the runtime environment on the remote
processor, so alternate RTOS or bare-metal implementations could also be
done. We will post a follow-up to the list with a URL soon.

> It looks like, at least for the da850, this subsumes or obsoletes
> DSPLINK in order to drive a more general purpose architecture
> (which looks great, so far, BTW).
> Is that the intent?

First, we are not abandoning DSPLINK. We have many users of this, even
though it is out-of-tree, and we will continue to support it. That said, we
do intend to make this new design the basis for DSPLINK-like
functionality. It's designed to be done "the right way" for Linux (and we
are looking for feedback to make it better).  It is also designed to be more
scalable and extensible in userspace. With a solid kernel foundation, we can
provide lots of functionality in userspace, or users can implement their own
custom solutions. One of the key things to do is map our existing DSPLINK
APIs, like MessageQ, to the new rpmsg transport.

Mark
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 0/8] Introducing a generic AMP/IPC framework

2011-06-23 Thread Michael Williamson
On 6/21/2011 3:18 AM, Ohad Ben-Cohen wrote:
> Modern SoCs typically employ a central symmetric multiprocessing (SMP)
> application processor running Linux, with several other asymmetric
> multiprocessing (AMP) heterogeneous processors running different instances
> of operating system, whether Linux or any other flavor of real-time OS.
>
> OMAP4, for example, has dual Cortex-A9, dual Cortex-M3 and a C64x+ DSP.
> Typically, the dual cortex-A9 is running Linux in a SMP configuration, and
> each of the other three cores (two M3 cores and a DSP) is running its own
> instance of RTOS in an AMP configuration.
>
> AMP remote processors typically employ dedicated DSP codecs and multimedia
> hardware accelerators, and therefore are often used to offload cpu-intensive
> multimedia tasks from the main application processor. They could also be
> used to control latency-sensitive sensors, drive "random" hardware blocks,
> or just perform background tasks while the main CPU is idling.
>
> Users of those remote processors can either be userland apps (e.g.
> multimedia frameworks talking with remote OMX components) or kernel drivers
> (controlling hardware accessible only by the remote processor, reserving
> kernel-controlled resources on behalf of the remote processor, etc..).
>
> This patch set adds a generic AMP/IPC framework which makes it possible to
> control (power on, boot, power off) and communicate (simply send and receive
> messages) with those remote processors.
>
> Specifically, we're adding:
>
> * Rpmsg: a virtio-based messaging bus that allows kernel drivers to
> communicate with remote processors available on the system. In turn,
> drivers could then expose appropriate user space interfaces, if needed
> (tasks running on remote processors often have direct access to sensitive
> resources like the system's physical memory, gpios, i2c buses, dma
> controllers, etc..  so one normally wouldn't want to allow userland to
> send everything/everywhere it wants).
>
> Every rpmsg device is a communication channel with a service running on a
> remote processor (thus rpmsg devices are called channels). Channels are
> identified by a textual name (which is used to match drivers to devices)
> and have a local ("source") rpmsg address, and remote ("destination") rpmsg
> address. When a driver starts listening on a channel (most commonly when it
> is probed), the bus assigns the driver a unique rpmsg src address (a 32 bit
> integer) and binds it with the driver's rx callback handler. This way
> when inbound messages arrive to this src address, the rpmsg core dispatches
> them to that driver, by invoking the driver's rx handler with the payload
> of the incoming message.
>
> Once probed, rpmsg drivers can also immediately start sending messages to the
> remote rpmsg service by using simple sending API; no need even to specify
> a destination address, since that's part of the rpmsg channel, and the rpmsg
> bus uses the channel's dst address when it constructs the message (for
> more demanding use cases, there's also an extended API, which does allow
> full control of both the src and dst addresses).
>
> The rpmsg bus is using virtio to send and receive messages: every pair
> of processors share two vrings, which are used to send and receive the
> messages over shared memory (one vring is used for tx, and the other one
> for rx). Kicking the remote processor (i.e. letting it know it has a pending
> message on its vring) is accomplished by means available on the platform we
> run on (e.g. OMAP is using its mailbox to both interrupt the remote processor
> and tell it which vring is kicked at the same time). The header of every
> message sent on the rpmsg bus contains src and dst addresses, which make it
> possible to multiplex several rpmsg channels on the same vring.
>
> One nice property of the rpmsg bus is that device creation is completely
> dynamic: remote processors can announce the existence of remote rpmsg
> services by sending a "name service" messages (which contain the name and
> rpmsg addr of the remote service). Those messages are picked by the rpmsg
> bus, which in turn dynamically creates and registers the rpmsg channels
> (i.e devices) which represents the remote services. If/when a relevant rpmsg
> driver is registered, it will be immediately probed by the bus, and can then
> start "talking" to the remote service.
>
> Similarly, we can use this technique to dynamically create virtio devices
> (and new vrings) which would then represent e.g. remote network, console
> and block devices that will be driven by the existing virtio drivers
> (this is still not implemented though; it requires some RTOS work as we're
> not booting Linux on OMAP's remote processors). Creating new vrings might
> also be desired by users who just don't want to use the shared rpmsg vrings
> (for performance or any other functionality reasons).
>
> In addition to dynamic creation of rpmsg channels, the rpmsg bus also
> supports creation of 

Re: [RFC 0/8] Introducing a generic AMP/IPC framework

2011-06-22 Thread Ohad Ben-Cohen
On Wed, Jun 22, 2011 at 4:05 PM, Arnd Bergmann  wrote:
> Ok, I see. In that case I agree that using debugfs is fine, but I would
> recommend trying to use fewer macros and just open-coding the file
> operations for better readability.

Sure thing. It didn't end up saving much code eventually, too, so I'll
just remove it.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 0/8] Introducing a generic AMP/IPC framework

2011-06-22 Thread Arnd Bergmann
On Wednesday 22 June 2011, Ohad Ben-Cohen wrote:
> > One point I noticed is the use of debugfs, which you should probably
> > replace at some point with a stable API, e.g. your own debugfs-like
> > file system, but there is no hurry to do that now.
> 
> debugfs is only being used to expose debugging info (namely the power
> state of the remote processor and its trace log messages), which is
> mostly useful for developers trying to understand what went wrong.
> 
> It seems like debugfs fits quite nicely here (e.g. it's perfectly fine
> if this is completely compiled out on production systems), but sure,
> we can always revisit this later too.

Ok, I see. In that case I agree that using debugfs is fine, but I would
recommend trying to use fewer macros and just open-coding the file
operations for better readability.

> > Unfortunately require __packed. It would be better to sort the members
> > differently so that each member is naturally aligned in order to
> > avoid the need for padding or __packed attributes
> 
> Definitely. __packed is being used just to be on the safe side; we
> didn't intend to introduce unnatural alignment intentionally. will be
> fixed.

Ok.

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 0/8] Introducing a generic AMP/IPC framework

2011-06-22 Thread Ohad Ben-Cohen
Hi Arnd,

On Tue, Jun 21, 2011 at 5:20 PM, Arnd Bergmann  wrote:
> This looks really nice overall, but I don't currently have time for a
> more in-depth review. My feeling is that you are definitely on the right
> track here, and the plans you list as TODO to continue are all good.

Thanks !

> One point I noticed is the use of debugfs, which you should probably
> replace at some point with a stable API, e.g. your own debugfs-like
> file system, but there is no hurry to do that now.

debugfs is only being used to expose debugging info (namely the power
state of the remote processor and its trace log messages), which is
mostly useful for developers trying to understand what went wrong.

It seems like debugfs fits quite nicely here (e.g. it's perfectly fine
if this is completely compiled out on production systems), but sure,
we can always revisit this later too.

> Regarding the firmware, I think it would be nice to have the option
> to stick the firmware blob into the flattened device tree as a bininclude,
> so you can build systems using the external processors without special
> user space support.

That's interesting. we'll definitely explore this - thanks !

I was also hoping that DT will simplify remote resource allocations as
well (just like any other device, remote processors usually needs some
hw resources) and planned to try it out soon to see how it all works
out together. It might completely eliminate the preliminary "resource
section" thingy we have in the firmware structure currently.

> The binary format looks reasonable, but if you need anything more complex,
> you should probably go straight for ELF files ;-)

Hopefully we won't need to :)

> +struct fw_section {
> +       u32 type;
> +       u64 da;
> +       u32 len;
> +       char content[0];
> +} __packed;
>
> Unfortunately require __packed. It would be better to sort the members
> differently so that each member is naturally aligned in order to
> avoid the need for padding or __packed attributes

Definitely. __packed is being used just to be on the safe side; we
didn't intend to introduce unnatural alignment intentionally. will be
fixed.

Thanks!
Ohad.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 0/8] Introducing a generic AMP/IPC framework

2011-06-21 Thread Grant Likely
On Tue, Jun 21, 2011 at 8:20 AM, Arnd Bergmann  wrote:
> Hi Ohad,
>
> On Tuesday 21 June 2011, Ohad Ben-Cohen wrote:
>
>> This patch set adds a generic AMP/IPC framework which makes it possible to
>> control (power on, boot, power off) and communicate (simply send and receive
>> messages) with those remote processors.
>
> This looks really nice overall, but I don't currently have time for a
> more in-depth review. My feeling is that you are definitely on the right
> track here, and the plans you list as TODO to continue are all good.

Second that.  I'm happy to see the shift over to virtio, I think that
is the right direction overall.  I'll try to carve out some time for a
detailed review.

g.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 0/8] Introducing a generic AMP/IPC framework

2011-06-21 Thread Arnd Bergmann
Hi Ohad,

On Tuesday 21 June 2011, Ohad Ben-Cohen wrote:

> This patch set adds a generic AMP/IPC framework which makes it possible to
> control (power on, boot, power off) and communicate (simply send and receive
> messages) with those remote processors.

This looks really nice overall, but I don't currently have time for a
more in-depth review. My feeling is that you are definitely on the right
track here, and the plans you list as TODO to continue are all good.

One point I noticed is the use of debugfs, which you should probably
replace at some point with a stable API, e.g. your own debugfs-like
file system, but there is no hurry to do that now.

> The gory part of remoteproc is the firmware handling. We tried to come up
> with a simple binary format that will require minimum kernel code to handle,
> but at the same time be generic enough in the hopes that it would prove
> useful to others as well. We're not at all hang onto the binary format
> we picked: if there's any technical reason to change it to support other
> platforms, please let us know. We do realize that a single binary firmware
> structure might eventually not work for everyone. it did prove useful for
> us though; we adopted both the OMAP and Davinci platforms (and their
> completely different remote processor devices) to this simple binary
> structure, so we don't have to duplicate the firmware handling code.

Regarding the firmware, I think it would be nice to have the option
to stick the firmware blob into the flattened device tree as a bininclude,
so you can build systems using the external processors without special
user space support.

The binary format looks reasonable, but if you need anything more complex,
you should probably go straight for ELF files ;-)

The structures you use like

+struct fw_section {
+   u32 type;
+   u64 da;
+   u32 len;
+   char content[0];
+} __packed;

Unfortunately require __packed. It would be better to sort the members
differently so that each member is naturally aligned in order to
avoid the need for padding or __packed attributes, like:

struct fw_section {
   u32 type;
   u32 len;
   u64 da;
   char content[0];
};

Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC 0/8] Introducing a generic AMP/IPC framework

2011-06-21 Thread Ohad Ben-Cohen
On Tue, Jun 21, 2011 at 10:50 AM, Ohad Ben-Cohen  wrote:
> root@omap4430-panda:~# cat /debug/remoteproc/omap-rproc.1/state
> running (2)

At this point, the two remote M3 cores also start dumping trace logs to
shared memory buffers, which are exposed by debugfs entries:

root@omap4430-panda:~# cat /debug/remoteproc/omap-rproc.1/trace0
CORE0 starting..
...

root@omap4430-panda:~# cat /debug/remoteproc/omap-rproc.1/trace1
CORE1 starting..
...
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html