All of lore.kernel.org
 help / color / mirror / Atom feed
From: Aleksander Morgado <aleksander@aleksander.es>
To: Stephan Gerhold <stephan@gerhold.net>
Cc: "David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Loic Poulain <loic.poulain@linaro.org>,
	Bjorn Andersson <bjorn.andersson@linaro.org>,
	Sergey Ryazanov <ryazanov.s.a@gmail.com>,
	Johannes Berg <johannes.berg@intel.com>,
	Leon Romanovsky <leon@kernel.org>,
	M Chetan Kumar <m.chetan.kumar@intel.com>,
	linuxwwan@intel.com, Ohad Ben-Cohen <ohad@wizery.com>,
	Mathieu Poirier <mathieu.poirier@linaro.org>,
	Network Development <netdev@vger.kernel.org>,
	linux-remoteproc@vger.kernel.org,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	phone-devel@vger.kernel.org,
	open list <linux-kernel@vger.kernel.org>,
	~postmarketos/upstreaming@lists.sr.ht
Subject: Re: [PATCH net-next v2 2/3] net: wwan: Add RPMSG WWAN CTRL driver
Date: Fri, 18 Jun 2021 14:24:17 +0200	[thread overview]
Message-ID: <CAAP7uc+ckHLdMis0WQpuJSCJ0Ln7zBddEa-w3itRGykXsUiF2Q@mail.gmail.com> (raw)
In-Reply-To: <YMxx0XimZAEHmeUx@gerhold.net>

Hey,

> > So, does this mean we're limiting the amount of channels exported to
> > only one QMI control port and one AT control port?
>
> Yep, but I think:
>   - It's easy to extend this with additional ports later
>     if someone has a real use case for that.
>   - It's still possible to export via rpmsgexport.
>

Ah, that's good then, if we can have the rpmsgexport fallback, there
shouldn't be any issue.

> > Not saying that's wrong, but maybe it makes sense to add a comment
> > somewhere specifying that explicitly.
>
> Given that these channels were only found through reverse engineering,
> saying that DATA*_CNTL/DATA* are fully equivalent QMI/AT ports is just
> a theory, I have no proof for this. Generally these channels had some
> fixed use case on the original Android system, for example DATA1 (AT)
> seems to have been often used for Bluetooth Dial-Up Networking (DUN)
> while DATA4 was often more general purpose.
>
> Perhaps DATA* are all fully equivalent, independent AT channels at the
> end, or perhaps DATA1/DATA4 behave slightly differently because there
> were some special requirements for Bluetooth DUN. I have no way to tell.
> And it can vary from device to device since we're stuck with
> device-specific (and usually signed) firmware.
>
> Another example: I have seen DATA11 on some devices, but it does not
> seem to work as AT port for some reason, there is no reply at all
> from the modem on that channel. Perhaps it needs to be activated
> somehow, perhaps it's not an AT channel at all, I have no way to tell.
>
> My point is: Here I'm only enabling what is proven to work on all
> devices (used in postmarketOS for more than a year). I have insufficient
> data to vouch for the reliability of any other channel. I cannot say if
> the channels are really independent, or influence each other somehow.
>

Fair enough; I think your approach is the correct one, just enable
what's known to work.

> As far as I understand, we currently do not have any use case for having
> multiple QMI/AT ports exposed for ModemManager, right? And if someone
> does have a use case, perhaps exposing them through the WWAN subsystem
> is not even what they want, perhaps they want to forward them through
> USB or something.
>

There is no such usecase in MM; having one single QMI port in MM is
more than enough, and having the extra AT port gives some
functionalities that we don't yet support in QMI (e.g. voice call
management). We don't gain anything extra by having more QMI or more
AT ports at this moment.

> > Also, would it make sense to have some way to trigger the export of
> > additional channels somehow via userspace? e.g. something like
> > rpmsgexport but using the wwan subsystem. I'm not sure if that's a
> > true need anywhere or just over-engineering the solution, truth be
> > told.
>
> So personally I think we should keep this simple and limited to existing
> use cases. If someone shows up with different requirements we can
> investigate this further.
>

Yes, I think I'm on that same boat.

> If I send a v3 I will check if I can clarify this in the commit
> message somewhat. I actually had something related in there but removed
> it shortly before submitting the patch because I thought it's mostly
> just speculation and the message was already quite long. Oh well :)
>

Heh :) Thanks!

-- 
Aleksander
https://aleksander.es

  reply	other threads:[~2021-06-18 12:24 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-18  7:52 [PATCH net-next v2 0/3] net: wwan: Add RPMSG WWAN CTRL driver Stephan Gerhold
2021-06-18  7:52 ` [PATCH net-next v2 1/3] rpmsg: core: Add driver_data for rpmsg_device_id Stephan Gerhold
2021-06-18  7:52 ` [PATCH net-next v2 2/3] net: wwan: Add RPMSG WWAN CTRL driver Stephan Gerhold
2021-06-18  8:21   ` Aleksander Morgado
2021-06-18 10:13     ` Stephan Gerhold
2021-06-18 12:24       ` Aleksander Morgado [this message]
2021-06-18  7:52 ` [PATCH net-next v2 3/3] net: wwan: Allow WWAN drivers to provide blocking tx and poll function Stephan Gerhold
2021-06-18 12:45   ` kernel test robot
2021-06-18 12:45     ` kernel test robot

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=CAAP7uc+ckHLdMis0WQpuJSCJ0Ln7zBddEa-w3itRGykXsUiF2Q@mail.gmail.com \
    --to=aleksander@aleksander.es \
    --cc=bjorn.andersson@linaro.org \
    --cc=davem@davemloft.net \
    --cc=johannes.berg@intel.com \
    --cc=kuba@kernel.org \
    --cc=leon@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-remoteproc@vger.kernel.org \
    --cc=linuxwwan@intel.com \
    --cc=loic.poulain@linaro.org \
    --cc=m.chetan.kumar@intel.com \
    --cc=mathieu.poirier@linaro.org \
    --cc=netdev@vger.kernel.org \
    --cc=ohad@wizery.com \
    --cc=phone-devel@vger.kernel.org \
    --cc=ryazanov.s.a@gmail.com \
    --cc=stephan@gerhold.net \
    --cc=~postmarketos/upstreaming@lists.sr.ht \
    /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.