lkml.org 
[lkml]   [2011]   [Dec]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRE: [PATCH 0/9] XSHM: Shared Memory Driver for ST-E Thor M7400 LTE modem
Hi Arnd,

> I've tried to do a review of your code, but I really need to understand
> more of the background first, to be able to tell if it's using the
> right high-level approach. There are a lot of details I could
> comment on, but let's first look at the overall design.

Ok great, looking forward to hearing your feedback.

> > Introduction:
> > ~~~~~~~~~~~~~
> > This patch-set introduces the Shared Memory Driver for ST-Ericsson's
> > Thor M7400 LTE modem.
> > The shared memory model is implemented for Chip-to-chip and
> > uses a reserved memory area and a number of bi-directional
> > channels. Each channel has it's own designated data area where
> payload
> > data is copied into.
>
> Can you explain what the scope of this framework is? What I don't
> understand from the code is whether this is meant to be very
> broadly applicable to a lot of devices and competing with e.g.
> rpmsg from Ohad Ben-Cohen, or whether this is really speciific
> to one hardware implementation for caif.

This is primarily designed for supporting the ST-Ericsson's M7400 product
and future modem platforms supporting shared memory.
There are some parts with a rather tight coupling to the modem such as
synchronization of the startup phases "ipc_ready" and "caif_ready" and
how we store the 1st stage boot images in shared memory.
However there are other concepts and parts of the implementation
that probably is possible to reuse.

> Also, to what degree is the protocol design flexible? Is the modem
> side fixed, so you have to live with any shortcomings of the interface,
> or are you at this point able to improve both sides when something
> is found to be lacking?

As I see it, this interface to the modem is pretty much set, and isn't
going to change a lot for the M7400 product.

However for the long term perspective: we expect this interface to evolve
for future products, so suggestions and input for improvements is welcome.
rpmsg or at least the use of virtio-ring combined with a true end-to-end
zero copy is something we definitely are interested to look into for the
future.

> > Two different channel types are defined, one stream channel which is
> > implemented as a traditional ring-buffer, and a packet channel which
> > is a ring-buffer of fix sized buffers where each buffer contains
> > an array of CAIF frames.
> >
> > The notification of read and write index updates are handled in a
> > separate driver called c2c_genio. This driver will be contributed
> separately,
> > but the API is included in this patch-set.
> >
> > The channel configuration is stored in shared memory, each channel
> > has a designated area in shared memory for configuration data,
> > read and write indexes and a data area.
>
> How many channels will there be on of each type in a typical system,
> and respectively in the maximum you can realistically expect over
> the next 10 years?

Currently we have defined the following channels:

Ch# Type Used for RX size TX size
------------------------------------------------------------------
1 Stream Modem Crash dump 64 kB 4 kB
2 Stream Flash-less Boot Protocol 4 kB 256 kB
3 Stream Boot Logging 8 kB 4kB
4 Packet CAIF high speed 8*16 kB 8*16kB
5 Packet CAIF low latency 4*1kB 4*1kB

The modem is doing a multi stage boot, where it initially reads
boot images stored in shared memory. This will boot-strap the
2nd boot stage where the Flash-less Boot Protocol is used.
In this phase channel #2 and #3 is used. At this stage only a
very basic OS-less runtime system is available, so CAIF protocol
cannot be running on the modem.

When the 2nd boot stage is complete, channel #4 and #5 will be used.
CAIF is a multiplexing protocol allowing multiple connections,
so the reason for two channels is to be able to separate high
throughput traffic such as IP + Logging from traffic with low-latency
requirements such as control (e.g. AT commands) and audio.

In case of modem crash-dump the modem is running in OS-less mode, and
transfers dump over channel #1.

Initially I have implemented separate memory areas for
the different channels and the initial boot images, however
the channel specification and netlink interface supports
overlapping channels. This is a possible future improvement.

> > Stream data
> > ~~~~~~~~~~~
> > The driver for the stream channel is implemented as a character device
> > interface to user space. The character device implements non-blocking open
> > and non-blocking IO in general. The character device is implementing
> > a traditional circular buffer directly in the shared memory region for
> > the channel.
>
> It seems unusual to have both a socket interface and a character device
> interface. What is the motivation behind this? Why can't you use the
> socket for non-blocking I/O?

As mentioned above, the modem is in different run modes, stream device
is only used when modem runs in OS-less mode, during boot or crash-dump.
Except for boot and crash-dump the CAIF stack is used.
A similar approach is used for HSI, where we use a bare
HSI channel in OS-less mode, and CAIF over HSI otherwise.

> > Driver model
> > ~~~~~~~~~~~~~~
> > Current implementation is using the platform bus for XSHM devices.
> > The packet channels are named "xshmp", and stream channel "xshms".
> >
> > /sys/devices:
> > |-- platform
> > | |-- uevent
> > | `-- xshm
> > | |-- bootimg
> > | |-- caif_ready
> > | |-- ipc_ready
> > | |-- subsystem -> ../../../bus/platform
> > | |-- xshmp.1
> > | | |-- driver -> ../../../../bus/platform/drivers/xshmp
> > | | |-- subsystem -> ../../../../bus/platform
> > | `-- xshms.0
> > | |-- driver -> ../../../../bus/platform/drivers/xshms
> > | |-- misc
> > | | `-- xshm0
> > | | |-- device -> ../../../xshms.0
> > | | |-- subsystem -> ../../../../../../class/misc
> > `-- virtual
> > |-- net
> > | |-- cfshm0
>
> Why are the channels /platform/ devices? With all the infrastructure
> you have in the xshm driver, I would think that you should be
> able to probe the available channels yourself instead of relying the
> board to define them.

OK point taken. Using the platform bus, was just share laziness as it
provides a simple way of creating device and drivers. I will look into
creating a separate bus.

Regards,
Sjur

\
 
 \ /
  Last update: 2011-12-07 16:47    [W:0.100 / U:0.236 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site