Re: [RESEND PATCH v18 0/3] userspace MHI client interface driver

From: Aleksander Morgado <aleksander@aleksander.es>
To: Jakub Kicinski <kuba@kernel.org>
Cc: Loic Poulain <loic.poulain@linaro.org>,
	Manivannan Sadhasivam <manivannan.sadhasivam@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: Tue, 9 Feb 2021 10:20:30 +0100	[thread overview]
Message-ID: <CAAP7ucLZ5jKbKriSp39OtDLotbv72eBWKFCfqCbAF854kCBU8w@mail.gmail.com> (raw)
In-Reply-To: <20210203104028.62d41962@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>

Hey Jakub

On Wed, Feb 3, 2021 at 7:41 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed, 3 Feb 2021 19:28:28 +0100 Loic Poulain wrote:
> > On Wed, 3 Feb 2021 at 19:05, Jakub Kicinski <kuba@kernel.org> wrote:
> > > On Wed, 03 Feb 2021 09:45:06 +0530 Manivannan Sadhasivam wrote:
> > > > The current patchset only supports QMI channel so I'd request you to
> > > > review the chardev node created for it. The QMI chardev node created
> > > > will be unique for the MHI bus and the number of nodes depends on the
> > > > MHI controllers in the system (typically 1 but not limited).
> > >
> > > If you want to add a MHI QMI driver, please write a QMI-only driver.
> > > This generic "userspace client interface" driver is a no go. Nobody will
> > > have the time and attention to police what you throw in there later.
> >
> > Think it should be seen as filtered userspace access to MHI bus
> > (filtered because not all channels are exposed), again it's not
> > specific to MHI, any bus in Linux offers that (i2c, spi, usb, serial,
> > etc...). It will not be specific to QMI, since we will also need it
> > for MBIM (modem control path), AT commands, and GPS (NMEA frames), all
> > these protocols are usually handled by userspace tools and not linked
> > to any internal Linux framework, so it would be better not having a
> > dedicated chardev for each of them.
>
> The more people argue for this backdoor interface the more distrustful
> of it we'll become. Keep going at your own peril.

Are your worries that this driver will end up being used for many more
things than the initial wwan control port management being suggested
here? If so, what would be the suggested alternative for this
integration? Just a different way to access those control ports
instead of a chardev? A per port type specific driver?

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?

-- 
Aleksander