Re: [PATCH] bus: mhi: host: Add userspace character interface

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5/31/2023 9:04 AM, Jeffrey Hugo wrote:
On 5/31/2023 8:35 AM, Greg KH wrote:
On Wed, May 31, 2023 at 07:58:03PM +0530, Manivannan Sadhasivam wrote:
+ Jakub (who NACKed the previous submission of UCI driver)
Link to previous submission: https://lore.kernel.org/all/1606533966-22821-1-git-send-email-hemantk@xxxxxxxxxxxxxx/

On Mon, May 22, 2023 at 01:04:59PM -0600, Jeffrey Hugo wrote:
From: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@xxxxxxxxxxx>

I2C, USB, and PCIe are examples of buses which have a mechanism to give
userspace direct access to a device on those buses. The MHI userspace
character interface (uci) is the MHI bus analogue.

The MHI bus devices are MHI channels which ferry blocks of data from one
end to the other. With this simple purpose, we can define a simple
interface to userspace - a character device that supports open/close/read/ write/poll operations. Since bus devices can only have a single consumer
we encode a whitelist of MHI channels to be exported to userspace so as
to avoid conflicts.

We also make this mechanism open to any device that implements MHI.
Today this includes WLAN (Wi-Fi), WWAN (4G/5G cellular), and ML/AI
devices. More devices are expected in the future.

In addition to implementing the framework for uci, we include an initial
usecase - the QAIC Sahara device.

Sahara is a file transfer protocol that is commonly used for two purposes when interacting with a device - transferring firmware to the device and
transferring crashdumps from the device. The Sahara protocol puts the
receiver of the data in control of the transfer. A firmware transfer
operation would have the device requesting the specific firmware images
that the device wants, and the host satisfying those requests.

In most cases, including for AIC100, Sahara is used as part of a two stage loading process. The device will boot a very limited bootloader that does the base minimum initialization and jump to the next stage. A simple, one-
shot protocol like BHI is used to send the next stage bootloader to the
device. This second stage bootloader contains more functionality and
implements the Sahara protocol. The second stage determines from various inputs what set of runtime firmware is required to boot the device into an operational status, and requests those pieces from the host.  With those
images transferred over, the device can funnly initialize.

Each AIC100 instance (currently, up to 16) in a system will create a
MHI device for QAIC_SAHARA. MHI_uci will consume each of these and create
a unique chardev which will be found as
/dev/<mhi instance>_QAIC_SAHARA
For example - /dev/mhi0_QAIC_SAHARA

An open userspace application that can consume these devices for firmware
transfers is located at https://github.com/andersson/qdl

Signed-off-by: Pranjal Ramajor Asha Kanojiya <quic_pkanojiy@xxxxxxxxxxx>
[jhugo: Rename to uci, plumb to mhi, rewrite commit text]
Signed-off-by: Jeffrey Hugo <quic_jhugo@xxxxxxxxxxx>

The previous attempt on adding UCI driver was NACKed by Jakub. For merging this
patch, I need an ACK from Jakub.

Given that this fails the kernel robot tests, why would anyone ack it
as-is?

I think Mani I looking for some "guidance" on the "architecture", and frankly so am I.  An official Ack from Jakub might not be quite the right thing at this stage, but at-least Jakub could come in and say he isn't planning on NACKing this right off the bat, in particular because this functionality can be used by WWAN devices which seems to be what caused the mess the last time around.

We've gone full circle here.  This functionality was proposed as part of the bus.  Jakub came in an NACKed that, which resulted in the WWAN subsystem and the guidance that this functionally belongs with the devices.  I tried to put it with the AIC100/QAIC device based on that, and that got NACKed by Daniel (GPU) saying that this belongs with the bus.  You (Greg) seemed to agree with Daniel on that.

Fixing kernel robot tests is one thing (I haven't seen any reports on this iteration), but if there is no agreement on where this lives, isn't it DOA?

I went hunting for a report and found it. Not sure why it hasn't hit my inbox. The issue looks trivial and really doesn't seem to prevent discussions on this IMO.


In summary, if you don't like this, please give some clear guidance. Greg, you've told me in the past that you don't discuss "architecture" without seeing the code.  Here is some code.  I don't claim it is perfect (you mentioned the QAIC version had some issues you were going to help with), but I would like to see some input.

-Jeff





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux