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

From: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
To: Jakub Kicinski <kuba@kernel.org>
Cc: 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: Thu, 4 Feb 2021 09:37:46 +0530	[thread overview]
Message-ID: <20210204040746.GA8235@thinkpad> (raw)
In-Reply-To: <20210203104028.62d41962@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>

On Wed, Feb 03, 2021 at 10:40:28AM -0800, Jakub Kicinski 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.

Sorry, I do not want this to go towards rant... But I don't think this is anyway
near a backdoor interface. There are userspace tools available to evaluate the
chardev node and whatever this driver supports and going to support in the
future is part of the Qualcomm modems. The fact that we can't add a separate
driver for MHI QMI is due to the code duplication as the underlying interface is
same it is just the channel which differs.

And I got your point in doing everything in the chardev'ish way here. But we
don't have any standard mechanism (QMI, MBIM, firmware/crash dump download).
And lot of people argued that we are too far away from creating a WWAN
subsystem. And the usecase we are dealing here is specific to Qualcomm. So IMO
we should go ahead with the current interface this driver offers.

Thanks,
Mani