netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Bjorn Andersson <bjorn.andersson@linaro.org>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
	Aleksander Morgado <aleksander@aleksander.es>,
	Loic Poulain <loic.poulain@linaro.org>,
	Greg KH <gregkh@linuxfoundation.org>,
	David Miller <davem@davemloft.net>,
	linux-arm-msm <linux-arm-msm@vger.kernel.org>,
	open list <linux-kernel@vger.kernel.org>,
	Jeffrey Hugo <jhugo@codeaurora.org>,
	Bhaumik Bhatt <bbhatt@codeaurora.org>,
	Network Development <netdev@vger.kernel.org>
Subject: Re: [RESEND PATCH v18 0/3] userspace MHI client interface driver
Date: Wed, 10 Feb 2021 16:08:39 -0600	[thread overview]
Message-ID: <YCRZZyHO/QkCT9sa@builder.lan> (raw)
In-Reply-To: <20210210104128.2166e506@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>

On Wed 10 Feb 12:41 CST 2021, Jakub Kicinski wrote:

> On Wed, 10 Feb 2021 11:55:31 +0530 Manivannan Sadhasivam wrote:
> > On Tue, Feb 09, 2021 at 08:17:44AM -0800, Jakub Kicinski wrote:
> > > On Tue, 9 Feb 2021 10:20:30 +0100 Aleksander Morgado wrote:  
> > > > This may be a stupid suggestion, but would the integration look less a
> > > > backdoor if it would have been named "mhi_wwan" and it exposed already
> > > > all the AT+DIAG+QMI+MBIM+NMEA possible channels as chardevs, not just
> > > > QMI?  
> > > 
> > > What's DIAG? Who's going to remember that this is a backdoor driver 
> > > a year from now when Qualcomm sends a one liner patches which just 
> > > adds a single ID to open another channel?  
> > 
> > I really appreciate your feedback on this driver eventhough I'm not
> > inclined with you calling this driver a "backdoor interface". But can
> > you please propose a solution on how to make this driver a good one as
> > per your thoughts?
> > 
> > I really don't know what bothers you even if the userspace tools making
> > use of these chardevs are available openly (you can do the audit and see
> > if anything wrong we are doing).
> 
> What bothers me is maintaining shim drivers which just shuttle opaque
> messages between user space and firmware. One of which definitely is,
> and the other may well be, proprietary. This is an open source project,
> users are supposed to be able to meaningfully change the behavior of
> the system.
> 

You're absolutely right in that we in general don't like shim drivers
and there are several examples of proper MHI drivers - for e.g.
networking, WiFi

Technically we could fork/reimplement
https://github.com/freedesktop/libqmi, https://github.com/andersson/diag
and https://github.com/andersson/qdl in the kernel as "proper drivers" -
each one exposing their own userspace ABI.

But to leave these in userspace and rely on something that looks exactly
like USBDEVFS seems like a much better strategy.

> What bothers me is that we have 3 WWAN vendors all doing their own
> thing and no common Linux API for WWAN. It may have been fine 10 years
> ago, but WWAN is increasingly complex and important.
> 

We had a deep discussion and a few prototypes for a WWAN framework going
around 1-1.5 years ago. Unfortunately, what did fit Intel's view of what
a WWAN device is didn't fit at all with what's run and exposed by the
"modem" DSP in a Qualcomm platform. After trying to find various
contrived ways to model this we gave up.

> > And exposing the raw access to the
> > hardware is not a new thing in kernel. There are several existing
> > subsystems/drivers does this as pointed out by Bjorn. Moreover we don't
> > have in-kernel APIs for the functionalities exposed by this driver and
> > creating one is not feasible as explained by many.
> > 
> > So please let us know the path forward on this series. We are open to
> > any suggestions but you haven't provided one till now.
> 
> Well. You sure know how to aggravate people. I said clearly that you
> can move forward on purpose build drivers (e.g. for WWAN). There is no
> way forward on this common shim driver as far as I'm concerned.

But what is a WWAN device? What features does it have? What kind of APIs
does it expose?


Note that in this sense "QMI" really is a "binary equivalent" of AT
commands, the data flows over a DMA engine, which is not part of the
"WWAN device" and other services, such as GPS, already has specific
transports available upstream.

Regards,
Bjorn

  parent reply	other threads:[~2021-02-10 22:09 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-06 18:44 [RESEND PATCH v18 0/3] userspace MHI client interface driver Hemant Kumar
2021-01-06 18:44 ` [RESEND PATCH v18 1/3] bus: mhi: core: Move MHI_MAX_MTU to external header file Hemant Kumar
2021-01-06 18:44 ` [RESEND PATCH v18 2/3] docs: Add documentation for userspace client interface Hemant Kumar
2021-01-06 18:44 ` [RESEND PATCH v18 3/3] bus: mhi: Add userspace client interface driver Hemant Kumar
2021-01-13 15:26 ` [RESEND PATCH v18 0/3] userspace MHI " Manivannan Sadhasivam
2021-01-19  9:42   ` Manivannan Sadhasivam
2021-01-19 10:28     ` Greg KH
2021-01-27 15:15   ` Greg KH
2021-01-27 16:24     ` Bjorn Andersson
2021-02-01 10:55     ` Manivannan Sadhasivam
2021-02-01 11:15       ` Greg KH
2021-02-01 12:13         ` Manivannan Sadhasivam
2021-02-02  4:22           ` Manivannan Sadhasivam
2021-02-03  4:10             ` Jakub Kicinski
2021-02-03  4:15               ` Manivannan Sadhasivam
2021-02-03 18:05                 ` Jakub Kicinski
2021-02-03 18:28                   ` Loic Poulain
2021-02-03 18:40                     ` Jakub Kicinski
2021-02-04  4:07                       ` Manivannan Sadhasivam
2021-02-04  5:53                       ` Bjorn Andersson
2021-02-09  9:20                       ` Aleksander Morgado
2021-02-09 16:17                         ` Jakub Kicinski
2021-02-09 16:49                           ` Aleksander Morgado
2021-02-10  6:25                           ` Manivannan Sadhasivam
2021-02-10 18:41                             ` Jakub Kicinski
2021-02-10 19:18                               ` Jeffrey Hugo
2021-02-10 22:08                               ` Bjorn Andersson [this message]
2021-02-11  9:26                               ` Aleksander Morgado
2021-02-28 14:12                             ` Aleksander Morgado
2021-02-28 15:52                               ` Manivannan Sadhasivam
2021-02-03 18:34                   ` Bjorn Andersson

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=YCRZZyHO/QkCT9sa@builder.lan \
    --to=bjorn.andersson@linaro.org \
    --cc=aleksander@aleksander.es \
    --cc=bbhatt@codeaurora.org \
    --cc=davem@davemloft.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=jhugo@codeaurora.org \
    --cc=kuba@kernel.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=loic.poulain@linaro.org \
    --cc=manivannan.sadhasivam@linaro.org \
    --cc=netdev@vger.kernel.org \
    /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).