From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: Stephan Gerhold <stephan@gerhold.net>,
Loic Poulain <loic.poulain@linaro.org>
Cc: Bjorn Andersson <bjorn.andersson@linaro.org>,
Aleksander Morgado <aleksander@aleksander.es>,
Network Development <netdev@vger.kernel.org>,
linux-remoteproc@vger.kernel.org,
linux-arm-msm <linux-arm-msm@vger.kernel.org>,
"David S. Miller" <davem@davemloft.net>,
Jakub Kicinski <kuba@kernel.org>,
Ohad Ben-Cohen <ohad@wizery.com>,
Mathieu Poirier <mathieu.poirier@linaro.org>
Subject: Re: [RFC] Integrate RPMSG/SMD into WWAN subsystem
Date: Mon, 07 Jun 2021 13:23:18 +0200 [thread overview]
Message-ID: <87sg1tvryx.fsf@toke.dk> (raw)
In-Reply-To: <YL364+xK3mE2FU8a@gerhold.net>
Stephan Gerhold <stephan@gerhold.net> writes:
> Hi Loic,
>
> On Mon, Jun 07, 2021 at 11:27:07AM +0200, Loic Poulain wrote:
>> On Sat, 5 Jun 2021 at 11:25, Stephan Gerhold <stephan@gerhold.net> wrote:
>> > On Fri, Jun 04, 2021 at 11:11:45PM +0200, Loic Poulain wrote:
>> > > On Wed, 2 Jun 2021 at 20:20, Stephan Gerhold <stephan@gerhold.net> wrote:
>> > > > I've been thinking about creating some sort of "RPMSG" driver for the
>> > > > new WWAN subsystem; this would be used as a QMI/AT channel to the
>> > > > integrated modem on some older Qualcomm SoCs such as MSM8916 and MSM8974.
>> > > >
>> > > > It's easy to confuse all the different approaches that Qualcomm has to
>> > > > talk to their modems, so I will first try to briefly give an overview
>> > > > about those that I'm familiar with:
>> > > >
>> > > > ---
>> > > > There is USB and MHI that are mainly used to talk to "external" modems.
>> > > >
>> > > > For the integrated modems in many Qualcomm SoCs there is typically
>> > > > a separate control and data path. They are not really related to each
>> > > > other (e.g. currently no common parent device in sysfs).
>> > > >
>> > > > For the data path (network interface) there is "IPA" (drivers/net/ipa)
>> > > > on newer SoCs or "BAM-DMUX" on some older SoCs (e.g. MSM8916/MSM8974).
>> > > > I have a driver for BAM-DMUX that I hope to finish up and submit soon.
>> > > >
>> > > > The connection is set up via QMI. The messages are either sent via
>> > > > a shared RPMSG channel (net/qrtr sockets in Linux) or via standalone
>> > > > SMD/RPMSG channels (e.g. "DATA5_CNTL" for QMI and "DATA1" for AT).
>> > > >
>> > > > This gives a lot of possible combinations like BAM-DMUX+RPMSG
>> > > > (MSM8916, MSM8974), or IPA+QRTR (SDM845) but also other funny
>> > > > combinations like IPA+RPMSG (MSM8994) or BAM-DMUX+QRTR (MSM8937).
>> > > >
>> > > > Simply put, supporting all these in userspace like ModemManager
>> > > > is a mess (Aleksander can probably confirm).
>> > > > It would be nice if this could be simplified through the WWAN subsystem.
>> > > >
>> > > > It's not clear to me if or how well QRTR sockets can be mapped to a char
>> > > > device for the WWAN subsystem, so for now I'm trying to focus on the
>> > > > standalone RPMSG approach (for MSM8916, MSM8974, ...).
>> > > > ---
>> > > >
>> > > > Currently ModemManager uses the RPMSG channels via the rpmsg-chardev
>> > > > (drivers/rpmsg/rpmsg_char.c). It wasn't my idea to use it like this,
>> > > > I just took that over from someone else. Realistically speaking, the
>> > > > current approach isn't too different from the UCI "backdoor interface"
>> > > > approach that was rejected for MHI...
>> > > >
>> > > > I kind of expected that I can just trivially copy some code from
>> > > > rpmsg_char.c into a WWAN driver since they both end up as a simple char
>> > > > device. But it looks like the abstractions in wwan_core are kind of
>> > > > getting in the way here... As far as I can tell, they don't really fit
>> > > > together with the RPMSG interface.
>> > > >
>> > > > For example there is rpmsg_send(...) (blocking) and rpmsg_trysend(...)
>> > > > (non-blocking) and even a rpmsg_poll(...) [1] but I don't see a way to
>> > > > get notified when the TX queue is full or no longer full so I can call
>> > > > wwan_port_txon/off().
>> > > >
>> > > > Any suggestions or other thoughts?
>> > >
>> > > It would be indeed nice to get this in the WWAN framework.
>> > > I don't know much about rpmsg but I think it is straightforward for
>> > > the RX path, the ept_cb can simply forward the buffers to
>> > > wwan_port_rx.
>> >
>> > Right, that part should be straightforward.
>> >
>> > > For tx, simply call rpmsg_trysend() in the wwan tx
>> > > callback and don't use the txon/off helpers. In short, keep it simple
>> > > and check if you observe any issues.
>> > >
>> >
>> > I'm not sure that's a good idea. This sounds like exactly the kind of
>> > thing that might explode later just because I don't manage to get the
>> > TX queue full in my tests. In that case, writing to the WWAN char dev
>> > would not block, even if O_NONBLOCK is not set.
>>
>> Right, if you think it could be a problem, you can always implement a
>> more complex solution like calling rpmsg_send from a
>> workqueue/kthread, and only re-enable tx once rpmsg_send returns.
>>
>
> I did run into trouble when I tried to stream lots of data into the WWAN
> char device (e.g. using dd). However, in practice (with ModemManager)
> I did not manage to cause such issues yet. Personally, I think it's
> something we should get right, just to avoid trouble later
> (like "modem suddenly stops working").
>
> Right now I extended the WWAN port ops a bit so I tells me if the write
> should be non-blocking or blocking and so I can call rpmsg_poll(...).
>
> But having some sort of workqueue also sounds like it could work quite
> well, thanks for the suggestion! Will think about it some more, or
> I might post what I have right now so you can take a look.
How big are those hardware TXQs? Just pushing packets to the hardware
until it overflows sounds like a recipe for absolutely terrible
bufferbloat... That would be bad!
-Toke
next prev parent reply other threads:[~2021-06-07 11:23 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-02 18:20 [RFC] Integrate RPMSG/SMD into WWAN subsystem Stephan Gerhold
2021-06-04 21:11 ` Loic Poulain
2021-06-05 9:25 ` Stephan Gerhold
2021-06-07 9:27 ` Loic Poulain
2021-06-07 10:54 ` Stephan Gerhold
2021-06-07 11:23 ` Toke Høiland-Jørgensen [this message]
2021-06-07 11:44 ` Stephan Gerhold
2021-06-07 12:16 ` Loic Poulain
2021-06-07 12:48 ` Stephan Gerhold
2021-06-15 9:03 ` Loic Poulain
2021-06-08 11:30 ` Toke Høiland-Jørgensen
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=87sg1tvryx.fsf@toke.dk \
--to=toke@redhat.com \
--cc=aleksander@aleksander.es \
--cc=bjorn.andersson@linaro.org \
--cc=davem@davemloft.net \
--cc=kuba@kernel.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-remoteproc@vger.kernel.org \
--cc=loic.poulain@linaro.org \
--cc=mathieu.poirier@linaro.org \
--cc=netdev@vger.kernel.org \
--cc=ohad@wizery.com \
--cc=stephan@gerhold.net \
/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 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).