Re: [PATCH v3] bus: mhi: core: Sanity check values from remote device before use

2021-04-07 Thread Hemant Kumar




On 3/10/21 1:30 PM, Jeffrey Hugo wrote:

When parsing the structures in the shared memory, there are values which
come from the remote device.  For example, a transfer completion event
will have a pointer to the tre in the relevant channel's transfer ring.
As another example, event ring elements may specify a channel in which
the event occurred, however the specified channel value may not be valid
as no channel is defined at that index even though the index may be less
than the maximum allowed index.  Such values should be considered to be
untrusted, and validated before use.  If we blindly use such values, we
may access invalid data or crash if the values are corrupted.

If validation fails, drop the relevant event.

Signed-off-by: Jeffrey Hugo 


Reviewed-by: Hemant Kumar 
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [RESEND PATCH] bus: mhi: core: Remove pre_init flag used for power purposes

2021-04-02 Thread Hemant Kumar




On 4/1/21 2:41 PM, Bhaumik Bhatt wrote:

Some controllers can choose to skip preparation for power up.
In that case, device context is initialized based on the pre_init
flag not being set during mhi_prepare_for_power_up(). There is no
reason MHI host driver should maintain and provide controllers
with two separate paths for preparing MHI.

Going forward, all controllers will be required to call the
mhi_prepare_for_power_up() API followed by their choice of sync
or async power up. This allows MHI host driver to get rid of the
pre_init flag and sets up a common way for all controllers to use
MHI. This also helps controllers fail early on during preparation
phase in some failure cases.

Signed-off-by: Bhaumik Bhatt 
---
This patch was tested on arm64 architecture.


Change looks good as current MHI controllers ath11k and pci generic are 
not using pre_init.


Reviewed-by: Hemant Kumar 

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v2 RESEND] bus: mhi: core: Wait for ready state after reset

2021-03-10 Thread Hemant Kumar




On 3/10/21 12:41 PM, Jeffrey Hugo wrote:

After the device has signaled the end of reset by clearing the reset bit,
it will automatically reinit MHI and the internal device structures.  Once
That is done, the device will signal it has entered the ready state.

Signaling the ready state involves sending an interrupt (MSI) to the host
which might cause IOMMU faults if it occurs at the wrong time.

If the controller is being powered down, and possibly removed, then the
reset flow would only wait for the end of reset.  At which point, the host
and device would start a race.  The host may complete its reset work, and
remove the interrupt handler, which would cause the interrupt to be
disabled in the IOMMU.  If that occurs before the device signals the ready
state, then the IOMMU will fault since it blocked an interrupt.  While
harmless, the fault would appear like a serious issue has occurred so let's
silence it by making sure the device hits the ready state before the host
completes its reset processing.

Signed-off-by: Jeffrey Hugo 


Reviewed-by: Hemant Kumar 

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v6 1/4] bus: mhi: core: Destroy SBL devices when moving to mission mode

2021-02-26 Thread Hemant Kumar




On 2/24/21 3:23 PM, Bhaumik Bhatt wrote:

Currently, client devices are created in SBL or AMSS (mission
mode) and only destroyed after power down or SYS ERROR. When
moving between certain execution environments, such as from SBL
to AMSS, no clean-up is required. This presents an issue where
SBL-specific channels are left open and client drivers now run in
an execution environment where they cannot operate. Fix this by
expanding the mhi_destroy_device() to do an execution environment
specific clean-up if one is requested. Close the gap and destroy
devices in such scenarios that allow SBL client drivers to clean
up once device enters mission mode.

Signed-off-by: Bhaumik Bhatt 

It make sense to clean up previous execution env related resources.

Reviewed-by: Hemant Kumar 

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v3] bus: mhi: core: Return EAGAIN if MHI ring is full

2021-02-17 Thread Hemant Kumar




On 2/17/21 8:26 AM, Jeffrey Hugo wrote:

From: Fan Wu 

Currently ENOMEM is returned when MHI ring is full. This error code is
very misleading. Change to EAGAIN instead.

Signed-off-by: Fan Wu 
Signed-off-by: Jeffrey Hugo 
---

Reviewed-by: Hemant Kumar 
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v6 8/8] bus: mhi: core: Do not clear channel context more than once

2021-02-05 Thread Hemant Kumar




On 2/4/21 12:28 PM, Bhaumik Bhatt wrote:

Clearing a channel context can happen twice if the client driver
unprepares and reset the channels from the remove() callback from
a controller requested MHI power down sequence. If there are
multiple attempts at calling the mhi_free_coherent() API, we see
kernel warnings such as "trying to free invalid coherent area".
Example for one such client is the QRTR MHI driver. Avoid these
warnings by skipping mhi_deinit_chan_ctxt() API call and prevent
extra work from MHI as the channels are already disabled.

Signed-off-by: Bhaumik Bhatt 


Reviewed-by: Hemant Kumar 
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v2 3/3] bus: mhi: core: Process execution environment changes serially

2021-01-15 Thread Hemant Kumar




On 1/14/21 11:16 AM, Bhaumik Bhatt wrote:

In current design, whenever the BHI interrupt is fired, the execution
environment is updated. This can cause race conditions and impede any
ongoing power up/down processing. For example, if a power down is in
progress and the host has updated the execution environment to a
local "disabled" state, any BHI interrupt firing later could replace
it with the value from the BHI EE register.
Can we add what is the real issue observed when mhi_cntrl->ee changed in 
above scenario?

 Another example would be

that the device can enter mission mode while device creation for SBL
is still going on, leading to multiple attempts at opening the same
channel.
Even for this scenario, can we add the real issue that was observed e.g. 
same device was attempting to get created twice and caused xyz issue?


Ensure that EE changes are handled only from appropriate places and
occur one after another and handle only PBL or RDDM EE changes as
critical events directly from the interrupt handler. This also makes
sure that we use the correct execution environment to notify the
controller driver when the device resets to one of the PBL execution
environments.


[..]

Thanks,
Hemant
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v2 2/3] bus: mhi: core: Download AMSS image from appropriate function

2021-01-15 Thread Hemant Kumar

Hi Bhaumik,

On 1/14/21 11:16 AM, Bhaumik Bhatt wrote:

During full boot chain firmware download, the PM state worker
downloads the AMSS image after waiting for the SBL execution
environment change in PBL mode itself. Since getting rid of the
firmware load worker thread, this design needs to change and MHI
Can we reword this as current driver does not have firmware load worker 
thread. Basically change is to avoid blocking st worker thread with a 
timeout to get SBL EE before starting AMSS image download. Instead 
trigger AMSS image download directly from the st worker thread when

DEV_ST_TRANSITION_SBL is queued.


host must download the AMSS image from the SBL mode of PM state
worker thread instead. Since the full boot chain firmware
download is associated with a synchronous power up and has MHI
host waiting for a transition to mission mode with a timeout, we
can skip creating any devices (or probing any client drivers) in
SBL mode transition and proceed directly with the AMSS image
download.

This means that if MHI host driver is not responsible for the
AMSS image download or the controller plans to have client
drivers opening any SBL channels, for example, to download images
or monitor debug logs for memory allocations or power management,
the device can be powered up asynchronously.


[..]

Thanks,
Hemant

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v2 1/3] bus: mhi: core: Clear devices when moving execution environments

2021-01-15 Thread Hemant Kumar




On 1/14/21 11:16 AM, Bhaumik Bhatt wrote:

When moving from SBL to mission mode execution environment, there
is no remove callback notification to MHI client drivers which
operate on SBL mode only. Client driver devices are being created
in SBL or AMSS(mission mode) and only destroyed after power down
or SYS_ERROR. If there exist any SBL-specific channels, those are
left open and client drivers are thus unaware of the new execution
environment where those channels cannot operate. Close the gap and
issue remove callbacks to SBL-specific client drivers once device
enters mission mode.

Signed-off-by: Bhaumik Bhatt 


Reviewed-by: Hemant Kumar 

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v5 9/9] bus: mhi: core: Do not clear channel context more than once

2021-01-08 Thread Hemant Kumar




On 1/8/21 12:54 PM, Bhaumik Bhatt wrote:

When clearing the channel context, calling mhi_free_coherent()
more than once can result in kernel warnings such as "trying to
free invalid coherent area". Prevent extra work by adding a check
to skip calling mhi_deinit_chan_ctxt() if the client driver has
already disabled the channels.

Signed-off-by: Bhaumik Bhatt 
---
  drivers/bus/mhi/core/init.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
index 30eef19..272f350 100644
--- a/drivers/bus/mhi/core/init.c
+++ b/drivers/bus/mhi/core/init.c
@@ -1314,6 +1314,7 @@ static int mhi_driver_remove(struct device *dev)
  
  		if ((ch_state[dir] == MHI_CH_STATE_ENABLED ||

 ch_state[dir] == MHI_CH_STATE_STOP) &&
+   mhi_chan->ch_state != MHI_CH_STATE_DISABLED &&
!mhi_chan->offload_ch)
mhi_deinit_chan_ctxt(mhi_cntrl, mhi_chan);
  


Reviewed-by: Hemant Kumar 
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


[RESEND PATCH v18 3/3] bus: mhi: Add userspace client interface driver

2021-01-06 Thread Hemant Kumar
This MHI client driver allows userspace clients to transfer
raw data between MHI device and host using standard file operations.
Driver instantiates UCI device object which is associated to device
file node. UCI device object instantiates UCI channel object when device
file node is opened. UCI channel object is used to manage MHI channels
by calling MHI core APIs for read and write operations. MHI channels
are started as part of device open(). MHI channels remain in start
state until last release() is called on UCI device file node. Device
file node is created with format

/dev/

This interface allows exposing modem control channel(s) such as QMI, MBIM,
or AT commands to userspace which can be used to configure the modem using
tools such as libqmi, ModemManager, minicom (for AT), etc over MHI. This is
required as there are no kernel APIs to access modem control path for device
configuration. Data path transporting the network payload (IP), however, is
routed to the Linux network via the mhi-net driver. Currently driver supports
QMI channel. libqmi is userspace MHI client which communicates to a QMI
service using QMI channel. Please refer to
https://www.freedesktop.org/wiki/Software/libqmi/ for additional information
on libqmi.

Signed-off-by: Hemant Kumar 
Reviewed-by: Manivannan Sadhasivam 
Reviewed-by: Jeffrey Hugo 
Tested-by: Loic Poulain 
---
 drivers/bus/mhi/Kconfig  |  13 +
 drivers/bus/mhi/Makefile |   3 +
 drivers/bus/mhi/uci.c| 664 +++
 3 files changed, 680 insertions(+)
 create mode 100644 drivers/bus/mhi/uci.c

diff --git a/drivers/bus/mhi/Kconfig b/drivers/bus/mhi/Kconfig
index da5cd0c..5194e8e 100644
--- a/drivers/bus/mhi/Kconfig
+++ b/drivers/bus/mhi/Kconfig
@@ -29,3 +29,16 @@ config MHI_BUS_PCI_GENERIC
  This driver provides MHI PCI controller driver for devices such as
  Qualcomm SDX55 based PCIe modems.
 
+config MHI_UCI
+   tristate "MHI UCI"
+   depends on MHI_BUS
+   help
+ MHI based Userspace Client Interface (UCI) driver is used for
+ transferring raw data between host and device using standard file
+ operations from userspace. Open, read, write, poll and close
+ operations are supported by this driver. Please check
+ mhi_uci_match_table for all supported channels that are exposed to
+ userspace.
+
+ To compile this driver as a module, choose M here: the module will be
+ called mhi_uci.
diff --git a/drivers/bus/mhi/Makefile b/drivers/bus/mhi/Makefile
index 0a2d778..69f2111 100644
--- a/drivers/bus/mhi/Makefile
+++ b/drivers/bus/mhi/Makefile
@@ -4,3 +4,6 @@ obj-y += core/
 obj-$(CONFIG_MHI_BUS_PCI_GENERIC) += mhi_pci_generic.o
 mhi_pci_generic-y += pci_generic.o
 
+# MHI client
+mhi_uci-y := uci.o
+obj-$(CONFIG_MHI_UCI) += mhi_uci.o
diff --git a/drivers/bus/mhi/uci.c b/drivers/bus/mhi/uci.c
new file mode 100644
index 000..1df2377
--- /dev/null
+++ b/drivers/bus/mhi/uci.c
@@ -0,0 +1,664 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2018-2020, The Linux Foundation. All rights reserved.*/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define MHI_UCI_DRIVER_NAME "mhi_uci"
+#define MHI_MAX_UCI_MINORS 128
+
+static DEFINE_IDR(uci_idr);
+static DEFINE_MUTEX(uci_drv_mutex);
+static struct class *uci_dev_class;
+static int uci_dev_major;
+
+/**
+ * struct uci_chan - MHI channel for a UCI device
+ * @udev: associated UCI device object
+ * @ul_wq: wait queue for writer
+ * @write_lock: mutex write lock for ul channel
+ * @dl_wq: wait queue for reader
+ * @read_lock: mutex read lock for dl channel
+ * @dl_pending_lock: spin lock for dl_pending list
+ * @dl_pending: list of dl buffers userspace is waiting to read
+ * @cur_buf: current buffer userspace is reading
+ * @dl_size: size of the current dl buffer userspace is reading
+ * @ref_count: uci_chan reference count
+ */
+struct uci_chan {
+   struct uci_dev *udev;
+   wait_queue_head_t ul_wq;
+
+   /* ul channel lock to synchronize multiple writes */
+   struct mutex write_lock;
+
+   wait_queue_head_t dl_wq;
+
+   /* dl channel lock to synchronize multiple reads */
+   struct mutex read_lock;
+
+   /*
+* protects pending list in bh context, channel release, read and
+* poll
+*/
+   spinlock_t dl_pending_lock;
+
+   struct list_head dl_pending;
+   struct uci_buf *cur_buf;
+   size_t dl_size;
+   struct kref ref_count;
+};
+
+/**
+ * struct uci_buf - UCI buffer
+ * @data: data buffer
+ * @len: length of data buffer
+ * @node: list node of the UCI buffer
+ */
+struct uci_buf {
+   void *data;
+   size_t len;
+   struct list_head node;
+};
+
+/**
+ * struct uci_dev - MHI UCI device
+ * @minor: UCI device node minor number
+ * @mhi_dev: associated mhi device object
+ * @uchan: UCI uplink and downlink channel object
+ * @mtu: max TRE buffer length
+ * @enabled: Fla

[RESEND PATCH v18 2/3] docs: Add documentation for userspace client interface

2021-01-06 Thread Hemant Kumar
MHI userspace client driver is creating device file node
for user application to perform file operations. File
operations are handled by MHI core driver. Currently
QMI MHI channel is supported by this driver.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
---
 Documentation/mhi/index.rst |  1 +
 Documentation/mhi/uci.rst   | 95 +
 2 files changed, 96 insertions(+)
 create mode 100644 Documentation/mhi/uci.rst

diff --git a/Documentation/mhi/index.rst b/Documentation/mhi/index.rst
index 1d8dec3..c75a371 100644
--- a/Documentation/mhi/index.rst
+++ b/Documentation/mhi/index.rst
@@ -9,6 +9,7 @@ MHI
 
mhi
topology
+   uci
 
 .. only::  subproject and html
 
diff --git a/Documentation/mhi/uci.rst b/Documentation/mhi/uci.rst
new file mode 100644
index 000..1e0a015
--- /dev/null
+++ b/Documentation/mhi/uci.rst
@@ -0,0 +1,95 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=
+Userspace Client Interface (UCI)
+=
+
+UCI driver enables userspace clients to communicate to external MHI devices
+like modem. UCI driver probe creates standard character device file nodes for
+userspace clients to perform open, read, write, poll and release file
+operations. UCI device object represents UCI device file node which gets
+instantiated as part of MHI UCI driver probe. UCI channel object represents
+MHI uplink or downlink channel.
+
+Operations
+==
+
+open
+
+
+Instantiates UCI channel object and starts MHI channels to move it to running
+state. Inbound buffers are queued to downlink channel transfer ring. Every
+subsequent open() increments UCI device reference count as well as UCI channel
+reference count.
+
+read
+
+
+When data transfer is completed on downlink channel, transfer ring element
+buffer is copied to pending list. Reader is unblocked and data is copied to
+userspace buffer. Transfer ring element buffer is queued back to downlink
+channel transfer ring.
+
+write
+-
+
+Write buffer is queued to uplink channel transfer ring if ring is not full. 
Upon
+uplink transfer completion buffer is freed.
+
+poll
+
+
+Returns EPOLLIN | EPOLLRDNORM mask if pending list has buffers to be read by
+userspace. Returns EPOLLOUT | EPOLLWRNORM mask if MHI uplink channel transfer
+ring is not empty.  When the uplink channel transfer ring is non-empty, more
+data may be sent to the device. Returns EPOLLERR when UCI driver is removed.
+
+release
+---
+
+Decrements UCI device reference count and UCI channel reference count. Upon
+last release() UCI channel clean up is performed. MHI channel moves to disable
+state and inbound buffers are freed.
+
+Usage
+=
+
+Device file node is created with format:-
+
+/dev/
+
+mhi_device_name includes mhi controller name and the name of the MHI channel
+being used by MHI client in userspace to send or receive data using MHI
+protocol.
+
+There is a separate character device file node created for each channel
+specified in MHI device id table. MHI channels are statically defined by MHI
+specification. The list of supported channels is in the channel list variable
+of mhi_device_id table in UCI driver.
+
+Qualcomm MSM Interface(QMI) Channel
+---
+
+Qualcomm MSM Interface(QMI) is a modem control messaging protocol used to
+communicate between software components in the modem and other peripheral
+subsystems. QMI communication is of request/response type or an unsolicited
+event type. libqmi is userspace MHI client which communicates to a QMI service
+using UCI device. It sends a QMI request to a QMI service using MHI channel 14
+or 16. QMI response is received using MHI channel 15 or 17 respectively. libqmi
+is a glib-based library for talking to WWAN modems and devices which speaks QMI
+protocol. For more information about libqmi please refer
+https://www.freedesktop.org/wiki/Software/libqmi/
+
+Usage Example
+~
+
+QMI command to retrieve device mode
+$ sudo qmicli -d /dev/mhi0_QMI --dms-get-model
+[/dev/mhi0_QMI] Device model retrieved:
+Model: 'FN980m'
+
+Other Use Cases
+---
+
+Getting MHI device specific diagnostics information to userspace MHI diagnostic
+client using DIAG channel 4 (Host to device) and 5 (Device to Host).
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[RESEND PATCH v18 1/3] bus: mhi: core: Move MHI_MAX_MTU to external header file

2021-01-06 Thread Hemant Kumar
Currently this macro is defined in internal MHI header as
a TRE length mask. Moving it to external header allows MHI
client drivers to set this upper bound for the transmit
buffer size.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
Reviewed-by: Manivannan Sadhasivam 
---
 drivers/bus/mhi/core/internal.h | 1 -
 include/linux/mhi.h | 3 +++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
index 6f80ec3..2b9c063 100644
--- a/drivers/bus/mhi/core/internal.h
+++ b/drivers/bus/mhi/core/internal.h
@@ -453,7 +453,6 @@ enum mhi_pm_state {
 #define CMD_EL_PER_RING128
 #define PRIMARY_CMD_RING   0
 #define MHI_DEV_WAKE_DB127
-#define MHI_MAX_MTU0x
 #define MHI_RANDOM_U32_NONZERO(bmsk)   (prandom_u32_max(bmsk) + 1)
 
 enum mhi_er_type {
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index e36d575..f072605 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -15,6 +15,9 @@
 #include 
 #include 
 
+/* MHI client drivers to set this upper bound for tx buffer */
+#define MHI_MAX_MTU 0x
+
 #define MHI_MAX_OEM_PK_HASH_SEGMENTS 16
 
 struct mhi_chan;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



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

2021-01-06 Thread Hemant Kumar
ation describing uci client driver use cases.
- Fixed uci ref counting in mhi_uci_open for error case.
- Addressed style related review comments.

V3: Added documentation for MHI UCI driver.

V2:
- Added mutex lock to prevent multiple readers to access same
- mhi buffer which can result into use after free.

Hemant Kumar (3):
  bus: mhi: core: Move MHI_MAX_MTU to external header file
  docs: Add documentation for userspace client interface
  bus: mhi: Add userspace client interface driver

 Documentation/mhi/index.rst |   1 +
 Documentation/mhi/uci.rst   |  95 ++
 drivers/bus/mhi/Kconfig |  13 +
 drivers/bus/mhi/Makefile|   3 +
 drivers/bus/mhi/core/internal.h |   1 -
 drivers/bus/mhi/uci.c   | 664 
 include/linux/mhi.h |   3 +
 7 files changed, 779 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/mhi/uci.rst
 create mode 100644 drivers/bus/mhi/uci.c

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



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

2020-12-11 Thread Hemant Kumar
ation describing uci client driver use cases.
- Fixed uci ref counting in mhi_uci_open for error case.
- Addressed style related review comments.

V3: Added documentation for MHI UCI driver.

V2:
- Added mutex lock to prevent multiple readers to access same
- mhi buffer which can result into use after free.



Hemant Kumar (3):
  bus: mhi: core: Move MHI_MAX_MTU to external header file
  docs: Add documentation for userspace client interface
  bus: mhi: Add userspace client interface driver

 Documentation/mhi/index.rst |   1 +
 Documentation/mhi/uci.rst   |  95 ++
 drivers/bus/mhi/Kconfig |  13 +
 drivers/bus/mhi/Makefile|   3 +
 drivers/bus/mhi/core/internal.h |   1 -
 drivers/bus/mhi/uci.c   | 664 
 include/linux/mhi.h |   3 +
 7 files changed, 779 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/mhi/uci.rst
 create mode 100644 drivers/bus/mhi/uci.c

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v18 1/3] bus: mhi: core: Move MHI_MAX_MTU to external header file

2020-12-11 Thread Hemant Kumar
Currently this macro is defined in internal MHI header as
a TRE length mask. Moving it to external header allows MHI
client drivers to set this upper bound for the transmit
buffer size.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
Reviewed-by: Manivannan Sadhasivam 
---
 drivers/bus/mhi/core/internal.h | 1 -
 include/linux/mhi.h | 3 +++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
index 6f80ec3..2b9c063 100644
--- a/drivers/bus/mhi/core/internal.h
+++ b/drivers/bus/mhi/core/internal.h
@@ -453,7 +453,6 @@ enum mhi_pm_state {
 #define CMD_EL_PER_RING128
 #define PRIMARY_CMD_RING   0
 #define MHI_DEV_WAKE_DB127
-#define MHI_MAX_MTU0x
 #define MHI_RANDOM_U32_NONZERO(bmsk)   (prandom_u32_max(bmsk) + 1)
 
 enum mhi_er_type {
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index e36d575..f072605 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -15,6 +15,9 @@
 #include 
 #include 
 
+/* MHI client drivers to set this upper bound for tx buffer */
+#define MHI_MAX_MTU 0x
+
 #define MHI_MAX_OEM_PK_HASH_SEGMENTS 16
 
 struct mhi_chan;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v18 2/3] docs: Add documentation for userspace client interface

2020-12-11 Thread Hemant Kumar
MHI userspace client driver is creating device file node
for user application to perform file operations. File
operations are handled by MHI core driver. Currently
QMI MHI channel is supported by this driver.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
---
 Documentation/mhi/index.rst |  1 +
 Documentation/mhi/uci.rst   | 95 +
 2 files changed, 96 insertions(+)
 create mode 100644 Documentation/mhi/uci.rst

diff --git a/Documentation/mhi/index.rst b/Documentation/mhi/index.rst
index 1d8dec3..c75a371 100644
--- a/Documentation/mhi/index.rst
+++ b/Documentation/mhi/index.rst
@@ -9,6 +9,7 @@ MHI
 
mhi
topology
+   uci
 
 .. only::  subproject and html
 
diff --git a/Documentation/mhi/uci.rst b/Documentation/mhi/uci.rst
new file mode 100644
index 000..1e0a015
--- /dev/null
+++ b/Documentation/mhi/uci.rst
@@ -0,0 +1,95 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=
+Userspace Client Interface (UCI)
+=
+
+UCI driver enables userspace clients to communicate to external MHI devices
+like modem. UCI driver probe creates standard character device file nodes for
+userspace clients to perform open, read, write, poll and release file
+operations. UCI device object represents UCI device file node which gets
+instantiated as part of MHI UCI driver probe. UCI channel object represents
+MHI uplink or downlink channel.
+
+Operations
+==
+
+open
+
+
+Instantiates UCI channel object and starts MHI channels to move it to running
+state. Inbound buffers are queued to downlink channel transfer ring. Every
+subsequent open() increments UCI device reference count as well as UCI channel
+reference count.
+
+read
+
+
+When data transfer is completed on downlink channel, transfer ring element
+buffer is copied to pending list. Reader is unblocked and data is copied to
+userspace buffer. Transfer ring element buffer is queued back to downlink
+channel transfer ring.
+
+write
+-
+
+Write buffer is queued to uplink channel transfer ring if ring is not full. 
Upon
+uplink transfer completion buffer is freed.
+
+poll
+
+
+Returns EPOLLIN | EPOLLRDNORM mask if pending list has buffers to be read by
+userspace. Returns EPOLLOUT | EPOLLWRNORM mask if MHI uplink channel transfer
+ring is not empty.  When the uplink channel transfer ring is non-empty, more
+data may be sent to the device. Returns EPOLLERR when UCI driver is removed.
+
+release
+---
+
+Decrements UCI device reference count and UCI channel reference count. Upon
+last release() UCI channel clean up is performed. MHI channel moves to disable
+state and inbound buffers are freed.
+
+Usage
+=
+
+Device file node is created with format:-
+
+/dev/
+
+mhi_device_name includes mhi controller name and the name of the MHI channel
+being used by MHI client in userspace to send or receive data using MHI
+protocol.
+
+There is a separate character device file node created for each channel
+specified in MHI device id table. MHI channels are statically defined by MHI
+specification. The list of supported channels is in the channel list variable
+of mhi_device_id table in UCI driver.
+
+Qualcomm MSM Interface(QMI) Channel
+---
+
+Qualcomm MSM Interface(QMI) is a modem control messaging protocol used to
+communicate between software components in the modem and other peripheral
+subsystems. QMI communication is of request/response type or an unsolicited
+event type. libqmi is userspace MHI client which communicates to a QMI service
+using UCI device. It sends a QMI request to a QMI service using MHI channel 14
+or 16. QMI response is received using MHI channel 15 or 17 respectively. libqmi
+is a glib-based library for talking to WWAN modems and devices which speaks QMI
+protocol. For more information about libqmi please refer
+https://www.freedesktop.org/wiki/Software/libqmi/
+
+Usage Example
+~
+
+QMI command to retrieve device mode
+$ sudo qmicli -d /dev/mhi0_QMI --dms-get-model
+[/dev/mhi0_QMI] Device model retrieved:
+Model: 'FN980m'
+
+Other Use Cases
+---
+
+Getting MHI device specific diagnostics information to userspace MHI diagnostic
+client using DIAG channel 4 (Host to device) and 5 (Device to Host).
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v18 3/3] bus: mhi: Add userspace client interface driver

2020-12-11 Thread Hemant Kumar
This MHI client driver allows userspace clients to transfer
raw data between MHI device and host using standard file operations.
Driver instantiates UCI device object which is associated to device
file node. UCI device object instantiates UCI channel object when device
file node is opened. UCI channel object is used to manage MHI channels
by calling MHI core APIs for read and write operations. MHI channels
are started as part of device open(). MHI channels remain in start
state until last release() is called on UCI device file node. Device
file node is created with format

/dev/

This interface allows exposing modem control channel(s) such as QMI, MBIM,
or AT commands to userspace which can be used to configure the modem using
tools such as libqmi, ModemManager, minicom (for AT), etc over MHI. This is
required as there are no kernel APIs to access modem control path for device
configuration. Data path transporting the network payload (IP), however, is
routed to the Linux network via the mhi-net driver. Currently driver supports
QMI channel. libqmi is userspace MHI client which communicates to a QMI
service using QMI channel. Please refer to
https://www.freedesktop.org/wiki/Software/libqmi/ for additional information
on libqmi.

Signed-off-by: Hemant Kumar 
Reviewed-by: Manivannan Sadhasivam 
Reviewed-by: Jeffrey Hugo 
Tested-by: Loic Poulain 
---
 drivers/bus/mhi/Kconfig  |  13 +
 drivers/bus/mhi/Makefile |   3 +
 drivers/bus/mhi/uci.c| 664 +++
 3 files changed, 680 insertions(+)
 create mode 100644 drivers/bus/mhi/uci.c

diff --git a/drivers/bus/mhi/Kconfig b/drivers/bus/mhi/Kconfig
index da5cd0c..5194e8e 100644
--- a/drivers/bus/mhi/Kconfig
+++ b/drivers/bus/mhi/Kconfig
@@ -29,3 +29,16 @@ config MHI_BUS_PCI_GENERIC
  This driver provides MHI PCI controller driver for devices such as
  Qualcomm SDX55 based PCIe modems.
 
+config MHI_UCI
+   tristate "MHI UCI"
+   depends on MHI_BUS
+   help
+ MHI based Userspace Client Interface (UCI) driver is used for
+ transferring raw data between host and device using standard file
+ operations from userspace. Open, read, write, poll and close
+ operations are supported by this driver. Please check
+ mhi_uci_match_table for all supported channels that are exposed to
+ userspace.
+
+ To compile this driver as a module, choose M here: the module will be
+ called mhi_uci.
diff --git a/drivers/bus/mhi/Makefile b/drivers/bus/mhi/Makefile
index 0a2d778..69f2111 100644
--- a/drivers/bus/mhi/Makefile
+++ b/drivers/bus/mhi/Makefile
@@ -4,3 +4,6 @@ obj-y += core/
 obj-$(CONFIG_MHI_BUS_PCI_GENERIC) += mhi_pci_generic.o
 mhi_pci_generic-y += pci_generic.o
 
+# MHI client
+mhi_uci-y := uci.o
+obj-$(CONFIG_MHI_UCI) += mhi_uci.o
diff --git a/drivers/bus/mhi/uci.c b/drivers/bus/mhi/uci.c
new file mode 100644
index 000..1df2377
--- /dev/null
+++ b/drivers/bus/mhi/uci.c
@@ -0,0 +1,664 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2018-2020, The Linux Foundation. All rights reserved.*/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define MHI_UCI_DRIVER_NAME "mhi_uci"
+#define MHI_MAX_UCI_MINORS 128
+
+static DEFINE_IDR(uci_idr);
+static DEFINE_MUTEX(uci_drv_mutex);
+static struct class *uci_dev_class;
+static int uci_dev_major;
+
+/**
+ * struct uci_chan - MHI channel for a UCI device
+ * @udev: associated UCI device object
+ * @ul_wq: wait queue for writer
+ * @write_lock: mutex write lock for ul channel
+ * @dl_wq: wait queue for reader
+ * @read_lock: mutex read lock for dl channel
+ * @dl_pending_lock: spin lock for dl_pending list
+ * @dl_pending: list of dl buffers userspace is waiting to read
+ * @cur_buf: current buffer userspace is reading
+ * @dl_size: size of the current dl buffer userspace is reading
+ * @ref_count: uci_chan reference count
+ */
+struct uci_chan {
+   struct uci_dev *udev;
+   wait_queue_head_t ul_wq;
+
+   /* ul channel lock to synchronize multiple writes */
+   struct mutex write_lock;
+
+   wait_queue_head_t dl_wq;
+
+   /* dl channel lock to synchronize multiple reads */
+   struct mutex read_lock;
+
+   /*
+* protects pending list in bh context, channel release, read and
+* poll
+*/
+   spinlock_t dl_pending_lock;
+
+   struct list_head dl_pending;
+   struct uci_buf *cur_buf;
+   size_t dl_size;
+   struct kref ref_count;
+};
+
+/**
+ * struct uci_buf - UCI buffer
+ * @data: data buffer
+ * @len: length of data buffer
+ * @node: list node of the UCI buffer
+ */
+struct uci_buf {
+   void *data;
+   size_t len;
+   struct list_head node;
+};
+
+/**
+ * struct uci_dev - MHI UCI device
+ * @minor: UCI device node minor number
+ * @mhi_dev: associated mhi device object
+ * @uchan: UCI uplink and downlink channel object
+ * @mtu: max TRE buffer length
+ * @enabled: Fla

[PATCH v17 0/3] userspace MHI client interface driver

2020-12-10 Thread Hemant Kumar
This patch series adds support for UCI driver. UCI driver enables userspace
clients to communicate to external MHI devices like modem. UCI driver probe
creates standard character device file nodes for userspace clients to
perform open, read, write, poll and release file operations. These file
operations call MHI core layer APIs to perform data transfer using MHI bus
to communicate with MHI device. Currently driver supports QMI channel. libqmi
is userspace MHI client which communicates to a QMI service using QMI channel.
libqmi is a glib-based library for talking to WWAN modems and devices which
speaks QMI protocol. For more information about libqmi please refer
https://www.freedesktop.org/wiki/Software/libqmi/. Patch is tested using arm64
and x86 based platform.

v17:
- Updated commit text for UCI driver by mentioning about libqmi open-source
  userspace program that will be talking to this UCI kernel driver.
- UCI driver depends upon patch "bus: mhi: core: Add helper API to return number
  of free TREs".

v16:
- Removed reference of WLAN as an external MHI device in documentation and
  cover letter.

v15:
- Updated documentation related to poll and release operations.

V14:
- Fixed device file node format to /dev/ instead of
  /dev/mhi_ because "mhi" is already part of mhi device name.
  For example old format: /dev/mhi_mhi0_QMI new format: /dev/mhi0_QMI.
- Updated MHI documentation to reflect index mhi controller name in
  QMI usage example.

V13:
- Removed LOOPBACK channel from mhi_device_id table from this patch series.
  Pushing a new patch series to add support for LOOPBACK channel and the user
  space test application. Also removed the description from kernel 
documentation.
- Added QMI channel to mhi_device_id table. QMI channel has existing libqmi
  support from user space.
- Updated kernel Documentation for QMI channel and provided external reference
  for libqmi.
- Updated device file node name by appending mhi device name only, which already
  includes mhi controller device name.

V12:
- Added loopback test driver under selftest/drivers/mhi. Updated kernel
  documentation for the usage of the loopback test application.
- Addressed review comments for renaming variable names, updated inline
  comments and removed two redundant dev_dbg.

V11:
- Fixed review comments for UCI documentation by expanding TLAs and rewording
  some sentences.

V10:
- Replaced mutex_lock with mutex_lock_interruptible in read() and write() file
  ops call back.

V9:
- Renamed dl_lock to dl_pending _lock and pending list to dl_pending for
  clarity.
- Used read lock to protect cur_buf.
- Change transfer status check logic and only consider 0 and -EOVERFLOW as
  only success.
- Added __int to module init function.
- Print channel name instead of minor number upon successful probe.

V8:
- Fixed kernel test robot compilation error by changing %lu to %zu for
  size_t.
- Replaced uci with UCI in Kconfig, commit text, and comments in driver
  code.
- Fixed minor style related comments.

V7:
- Decoupled uci device and uci channel objects. uci device is
  associated with device file node. uci channel is associated
  with MHI channels. uci device refers to uci channel to perform
  MHI channel operations for device file operations like read()
  and write(). uci device increments its reference count for
  every open(). uci device calls mhi_uci_dev_start_chan() to start
  the MHI channel. uci channel object is tracking number of times
  MHI channel is referred. This allows to keep the MHI channel in
  start state until last release() is called. After that uci channel
  reference count goes to 0 and uci channel clean up is performed
  which stops the MHI channel. After the last call to release() if
  driver is removed uci reference count becomes 0 and uci object is
  cleaned up.
- Use separate uci channel read and write lock to fine grain locking
  between reader and writer.
- Use uci device lock to synchronize open, release and driver remove.
- Optimize for downlink only or uplink only UCI device.

V6:
- Moved uci.c to mhi directory.
- Updated Kconfig to add module information.
- Updated Makefile to rename uci object file name as mhi_uci
- Removed kref for open count

V5:
- Removed mhi_uci_drv structure.
- Used idr instead of creating global list of uci devices.
- Used kref instead of local ref counting for uci device and
  open count.
- Removed unlikely macro.

V4:
- Fix locking to protect proper struct members.
- Updated documentation describing uci client driver use cases.
- Fixed uci ref counting in mhi_uci_open for error case.
- Addressed style related review comments.

V3: Added documentation for MHI UCI driver.

V2:
- Added mutex lock to prevent multiple readers to access same
- mhi buffer which can result into use after free.

Hemant Kumar (3):
  bus: mhi: core: Move MHI_MAX_MTU to external header file
  docs: Add documentation for userspace client interface
  bus: mhi: Add userspace client interface dr

[PATCH v17 1/3] bus: mhi: core: Move MHI_MAX_MTU to external header file

2020-12-10 Thread Hemant Kumar
Currently this macro is defined in internal MHI header as
a TRE length mask. Moving it to external header allows MHI
client drivers to set this upper bound for the transmit
buffer size.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
Reviewed-by: Manivannan Sadhasivam 
---
 drivers/bus/mhi/core/internal.h | 1 -
 include/linux/mhi.h | 3 +++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
index 6f80ec3..2b9c063 100644
--- a/drivers/bus/mhi/core/internal.h
+++ b/drivers/bus/mhi/core/internal.h
@@ -453,7 +453,6 @@ enum mhi_pm_state {
 #define CMD_EL_PER_RING128
 #define PRIMARY_CMD_RING   0
 #define MHI_DEV_WAKE_DB127
-#define MHI_MAX_MTU0x
 #define MHI_RANDOM_U32_NONZERO(bmsk)   (prandom_u32_max(bmsk) + 1)
 
 enum mhi_er_type {
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index e36d575..f072605 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -15,6 +15,9 @@
 #include 
 #include 
 
+/* MHI client drivers to set this upper bound for tx buffer */
+#define MHI_MAX_MTU 0x
+
 #define MHI_MAX_OEM_PK_HASH_SEGMENTS 16
 
 struct mhi_chan;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v17 2/3] docs: Add documentation for userspace client interface

2020-12-10 Thread Hemant Kumar
MHI userspace client driver is creating device file node
for user application to perform file operations. File
operations are handled by MHI core driver. Currently
QMI MHI channel is supported by this driver.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
---
 Documentation/mhi/index.rst |  1 +
 Documentation/mhi/uci.rst   | 95 +
 2 files changed, 96 insertions(+)
 create mode 100644 Documentation/mhi/uci.rst

diff --git a/Documentation/mhi/index.rst b/Documentation/mhi/index.rst
index 1d8dec3..c75a371 100644
--- a/Documentation/mhi/index.rst
+++ b/Documentation/mhi/index.rst
@@ -9,6 +9,7 @@ MHI
 
mhi
topology
+   uci
 
 .. only::  subproject and html
 
diff --git a/Documentation/mhi/uci.rst b/Documentation/mhi/uci.rst
new file mode 100644
index 000..1e0a015
--- /dev/null
+++ b/Documentation/mhi/uci.rst
@@ -0,0 +1,95 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=
+Userspace Client Interface (UCI)
+=
+
+UCI driver enables userspace clients to communicate to external MHI devices
+like modem. UCI driver probe creates standard character device file nodes for
+userspace clients to perform open, read, write, poll and release file
+operations. UCI device object represents UCI device file node which gets
+instantiated as part of MHI UCI driver probe. UCI channel object represents
+MHI uplink or downlink channel.
+
+Operations
+==
+
+open
+
+
+Instantiates UCI channel object and starts MHI channels to move it to running
+state. Inbound buffers are queued to downlink channel transfer ring. Every
+subsequent open() increments UCI device reference count as well as UCI channel
+reference count.
+
+read
+
+
+When data transfer is completed on downlink channel, transfer ring element
+buffer is copied to pending list. Reader is unblocked and data is copied to
+userspace buffer. Transfer ring element buffer is queued back to downlink
+channel transfer ring.
+
+write
+-
+
+Write buffer is queued to uplink channel transfer ring if ring is not full. 
Upon
+uplink transfer completion buffer is freed.
+
+poll
+
+
+Returns EPOLLIN | EPOLLRDNORM mask if pending list has buffers to be read by
+userspace. Returns EPOLLOUT | EPOLLWRNORM mask if MHI uplink channel transfer
+ring is not empty.  When the uplink channel transfer ring is non-empty, more
+data may be sent to the device. Returns EPOLLERR when UCI driver is removed.
+
+release
+---
+
+Decrements UCI device reference count and UCI channel reference count. Upon
+last release() UCI channel clean up is performed. MHI channel moves to disable
+state and inbound buffers are freed.
+
+Usage
+=
+
+Device file node is created with format:-
+
+/dev/
+
+mhi_device_name includes mhi controller name and the name of the MHI channel
+being used by MHI client in userspace to send or receive data using MHI
+protocol.
+
+There is a separate character device file node created for each channel
+specified in MHI device id table. MHI channels are statically defined by MHI
+specification. The list of supported channels is in the channel list variable
+of mhi_device_id table in UCI driver.
+
+Qualcomm MSM Interface(QMI) Channel
+---
+
+Qualcomm MSM Interface(QMI) is a modem control messaging protocol used to
+communicate between software components in the modem and other peripheral
+subsystems. QMI communication is of request/response type or an unsolicited
+event type. libqmi is userspace MHI client which communicates to a QMI service
+using UCI device. It sends a QMI request to a QMI service using MHI channel 14
+or 16. QMI response is received using MHI channel 15 or 17 respectively. libqmi
+is a glib-based library for talking to WWAN modems and devices which speaks QMI
+protocol. For more information about libqmi please refer
+https://www.freedesktop.org/wiki/Software/libqmi/
+
+Usage Example
+~
+
+QMI command to retrieve device mode
+$ sudo qmicli -d /dev/mhi0_QMI --dms-get-model
+[/dev/mhi0_QMI] Device model retrieved:
+Model: 'FN980m'
+
+Other Use Cases
+---
+
+Getting MHI device specific diagnostics information to userspace MHI diagnostic
+client using DIAG channel 4 (Host to device) and 5 (Device to Host).
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v17 3/3] bus: mhi: Add userspace client interface driver

2020-12-10 Thread Hemant Kumar
This MHI client driver allows userspace clients to transfer
raw data between MHI device and host using standard file operations.
Driver instantiates UCI device object which is associated to device
file node. UCI device object instantiates UCI channel object when device
file node is opened. UCI channel object is used to manage MHI channels
by calling MHI core APIs for read and write operations. MHI channels
are started as part of device open(). MHI channels remain in start
state until last release() is called on UCI device file node. Device
file node is created with format

/dev/

Currently it supports QMI channel. libqmi is userspace MHI client which
communicates to a QMI service using QMI channel. libqmi is a glib-based
library for talking to WWAN modems and devices which speaks QMI protocol.
For more information about libqmi please refer
https://www.freedesktop.org/wiki/Software/libqmi/

Signed-off-by: Hemant Kumar 
Reviewed-by: Manivannan Sadhasivam 
Reviewed-by: Jeffrey Hugo 
Tested-by: Loic Poulain 
---
 drivers/bus/mhi/Kconfig  |  13 +
 drivers/bus/mhi/Makefile |   3 +
 drivers/bus/mhi/uci.c| 664 +++
 3 files changed, 680 insertions(+)
 create mode 100644 drivers/bus/mhi/uci.c

diff --git a/drivers/bus/mhi/Kconfig b/drivers/bus/mhi/Kconfig
index da5cd0c..5194e8e 100644
--- a/drivers/bus/mhi/Kconfig
+++ b/drivers/bus/mhi/Kconfig
@@ -29,3 +29,16 @@ config MHI_BUS_PCI_GENERIC
  This driver provides MHI PCI controller driver for devices such as
  Qualcomm SDX55 based PCIe modems.
 
+config MHI_UCI
+   tristate "MHI UCI"
+   depends on MHI_BUS
+   help
+ MHI based Userspace Client Interface (UCI) driver is used for
+ transferring raw data between host and device using standard file
+ operations from userspace. Open, read, write, poll and close
+ operations are supported by this driver. Please check
+ mhi_uci_match_table for all supported channels that are exposed to
+ userspace.
+
+ To compile this driver as a module, choose M here: the module will be
+ called mhi_uci.
diff --git a/drivers/bus/mhi/Makefile b/drivers/bus/mhi/Makefile
index 0a2d778..69f2111 100644
--- a/drivers/bus/mhi/Makefile
+++ b/drivers/bus/mhi/Makefile
@@ -4,3 +4,6 @@ obj-y += core/
 obj-$(CONFIG_MHI_BUS_PCI_GENERIC) += mhi_pci_generic.o
 mhi_pci_generic-y += pci_generic.o
 
+# MHI client
+mhi_uci-y := uci.o
+obj-$(CONFIG_MHI_UCI) += mhi_uci.o
diff --git a/drivers/bus/mhi/uci.c b/drivers/bus/mhi/uci.c
new file mode 100644
index 000..1df2377
--- /dev/null
+++ b/drivers/bus/mhi/uci.c
@@ -0,0 +1,664 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2018-2020, The Linux Foundation. All rights reserved.*/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define MHI_UCI_DRIVER_NAME "mhi_uci"
+#define MHI_MAX_UCI_MINORS 128
+
+static DEFINE_IDR(uci_idr);
+static DEFINE_MUTEX(uci_drv_mutex);
+static struct class *uci_dev_class;
+static int uci_dev_major;
+
+/**
+ * struct uci_chan - MHI channel for a UCI device
+ * @udev: associated UCI device object
+ * @ul_wq: wait queue for writer
+ * @write_lock: mutex write lock for ul channel
+ * @dl_wq: wait queue for reader
+ * @read_lock: mutex read lock for dl channel
+ * @dl_pending_lock: spin lock for dl_pending list
+ * @dl_pending: list of dl buffers userspace is waiting to read
+ * @cur_buf: current buffer userspace is reading
+ * @dl_size: size of the current dl buffer userspace is reading
+ * @ref_count: uci_chan reference count
+ */
+struct uci_chan {
+   struct uci_dev *udev;
+   wait_queue_head_t ul_wq;
+
+   /* ul channel lock to synchronize multiple writes */
+   struct mutex write_lock;
+
+   wait_queue_head_t dl_wq;
+
+   /* dl channel lock to synchronize multiple reads */
+   struct mutex read_lock;
+
+   /*
+* protects pending list in bh context, channel release, read and
+* poll
+*/
+   spinlock_t dl_pending_lock;
+
+   struct list_head dl_pending;
+   struct uci_buf *cur_buf;
+   size_t dl_size;
+   struct kref ref_count;
+};
+
+/**
+ * struct uci_buf - UCI buffer
+ * @data: data buffer
+ * @len: length of data buffer
+ * @node: list node of the UCI buffer
+ */
+struct uci_buf {
+   void *data;
+   size_t len;
+   struct list_head node;
+};
+
+/**
+ * struct uci_dev - MHI UCI device
+ * @minor: UCI device node minor number
+ * @mhi_dev: associated mhi device object
+ * @uchan: UCI uplink and downlink channel object
+ * @mtu: max TRE buffer length
+ * @enabled: Flag to track the state of the UCI device
+ * @lock: mutex lock to manage uchan object
+ * @ref_count: uci_dev reference count
+ */
+struct uci_dev {
+   unsigned int minor;
+   struct mhi_device *mhi_dev;
+   struct uci_chan *uchan;
+   size_t mtu;
+   bool enabled;
+
+   /* synchronize open, release and driver remove */
+

[PATCH v16 0/4] userspace MHI client interface driver

2020-12-09 Thread Hemant Kumar
This patch series adds support for UCI driver. UCI driver enables userspace
clients to communicate to external MHI devices like modem. UCI driver probe
creates standard character device file nodes for userspace clients to
perform open, read, write, poll and release file operations. These file
operations call MHI core layer APIs to perform data transfer using MHI bus
to communicate with MHI device. Patch is tested using arm64 and x86 based
platform.

v16:
- Removed reference of WLAN as an external MHI device in documentation and
  cover letter.

v15:
- Updated documentation related to poll and release operations.

V14:
- Fixed device file node format to /dev/ instead of
  /dev/mhi_ because "mhi" is already part of mhi device name.
  For example old format: /dev/mhi_mhi0_QMI new format: /dev/mhi0_QMI.
- Updated MHI documentation to reflect index mhi controller name in
  QMI usage example.

V13:
- Removed LOOPBACK channel from mhi_device_id table from this patch series.
  Pushing a new patch series to add support for LOOPBACK channel and the user
  space test application. Also removed the description from kernel 
documentation.
- Added QMI channel to mhi_device_id table. QMI channel has existing libqmi
  support from user space.
- Updated kernel Documentation for QMI channel and provided external reference
  for libqmi.
- Updated device file node name by appending mhi device name only, which already
  includes mhi controller device name.

V12:
- Added loopback test driver under selftest/drivers/mhi. Updated kernel
  documentation for the usage of the loopback test application.
- Addressed review comments for renaming variable names, updated inline
  comments and removed two redundant dev_dbg.

V11:
- Fixed review comments for UCI documentation by expanding TLAs and rewording
  some sentences.

V10:
- Replaced mutex_lock with mutex_lock_interruptible in read() and write() file
  ops call back.

V9:
- Renamed dl_lock to dl_pending _lock and pending list to dl_pending for
  clarity.
- Used read lock to protect cur_buf.
- Change transfer status check logic and only consider 0 and -EOVERFLOW as
  only success.
- Added __int to module init function.
- Print channel name instead of minor number upon successful probe.

V8:
- Fixed kernel test robot compilation error by changing %lu to %zu for
  size_t.
- Replaced uci with UCI in Kconfig, commit text, and comments in driver
  code.
- Fixed minor style related comments.

V7:
- Decoupled uci device and uci channel objects. uci device is
  associated with device file node. uci channel is associated
  with MHI channels. uci device refers to uci channel to perform
  MHI channel operations for device file operations like read()
  and write(). uci device increments its reference count for
  every open(). uci device calls mhi_uci_dev_start_chan() to start
  the MHI channel. uci channel object is tracking number of times
  MHI channel is referred. This allows to keep the MHI channel in
  start state until last release() is called. After that uci channel
  reference count goes to 0 and uci channel clean up is performed
  which stops the MHI channel. After the last call to release() if
  driver is removed uci reference count becomes 0 and uci object is
  cleaned up.
- Use separate uci channel read and write lock to fine grain locking
  between reader and writer.
- Use uci device lock to synchronize open, release and driver remove.
- Optimize for downlink only or uplink only UCI device.

V6:
- Moved uci.c to mhi directory.
- Updated Kconfig to add module information.
- Updated Makefile to rename uci object file name as mhi_uci
- Removed kref for open count

V5:
- Removed mhi_uci_drv structure.
- Used idr instead of creating global list of uci devices.
- Used kref instead of local ref counting for uci device and
  open count.
- Removed unlikely macro.

V4:
- Fix locking to protect proper struct members.
- Updated documentation describing uci client driver use cases.
- Fixed uci ref counting in mhi_uci_open for error case.
- Addressed style related review comments.

V3: Added documentation for MHI UCI driver.

V2:
- Added mutex lock to prevent multiple readers to access same
- mhi buffer which can result into use after free.

Hemant Kumar (4):
  bus: mhi: core: Add helper API to return number of free TREs
  bus: mhi: core: Move MHI_MAX_MTU to external header file
  docs: Add documentation for userspace client interface
  bus: mhi: Add userspace client interface driver

 Documentation/mhi/index.rst |   1 +
 Documentation/mhi/uci.rst   |  95 ++
 drivers/bus/mhi/Kconfig |  13 +
 drivers/bus/mhi/Makefile|   3 +
 drivers/bus/mhi/core/internal.h |   1 -
 drivers/bus/mhi/core/main.c |  12 +
 drivers/bus/mhi/uci.c   | 664 
 include/linux/mhi.h |  12 +
 8 files changed, 800 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/mhi/uci.rst
 create mode 100644 drivers/bus

[PATCH v16 3/4] docs: Add documentation for userspace client interface

2020-12-09 Thread Hemant Kumar
MHI userspace client driver is creating device file node
for user application to perform file operations. File
operations are handled by MHI core driver. Currently
QMI MHI channel is supported by this driver.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
---
 Documentation/mhi/index.rst |  1 +
 Documentation/mhi/uci.rst   | 95 +
 2 files changed, 96 insertions(+)
 create mode 100644 Documentation/mhi/uci.rst

diff --git a/Documentation/mhi/index.rst b/Documentation/mhi/index.rst
index 1d8dec3..c75a371 100644
--- a/Documentation/mhi/index.rst
+++ b/Documentation/mhi/index.rst
@@ -9,6 +9,7 @@ MHI
 
mhi
topology
+   uci
 
 .. only::  subproject and html
 
diff --git a/Documentation/mhi/uci.rst b/Documentation/mhi/uci.rst
new file mode 100644
index 000..1e0a015
--- /dev/null
+++ b/Documentation/mhi/uci.rst
@@ -0,0 +1,95 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=
+Userspace Client Interface (UCI)
+=
+
+UCI driver enables userspace clients to communicate to external MHI devices
+like modem. UCI driver probe creates standard character device file nodes for
+userspace clients to perform open, read, write, poll and release file
+operations. UCI device object represents UCI device file node which gets
+instantiated as part of MHI UCI driver probe. UCI channel object represents
+MHI uplink or downlink channel.
+
+Operations
+==
+
+open
+
+
+Instantiates UCI channel object and starts MHI channels to move it to running
+state. Inbound buffers are queued to downlink channel transfer ring. Every
+subsequent open() increments UCI device reference count as well as UCI channel
+reference count.
+
+read
+
+
+When data transfer is completed on downlink channel, transfer ring element
+buffer is copied to pending list. Reader is unblocked and data is copied to
+userspace buffer. Transfer ring element buffer is queued back to downlink
+channel transfer ring.
+
+write
+-
+
+Write buffer is queued to uplink channel transfer ring if ring is not full. 
Upon
+uplink transfer completion buffer is freed.
+
+poll
+
+
+Returns EPOLLIN | EPOLLRDNORM mask if pending list has buffers to be read by
+userspace. Returns EPOLLOUT | EPOLLWRNORM mask if MHI uplink channel transfer
+ring is not empty.  When the uplink channel transfer ring is non-empty, more
+data may be sent to the device. Returns EPOLLERR when UCI driver is removed.
+
+release
+---
+
+Decrements UCI device reference count and UCI channel reference count. Upon
+last release() UCI channel clean up is performed. MHI channel moves to disable
+state and inbound buffers are freed.
+
+Usage
+=
+
+Device file node is created with format:-
+
+/dev/
+
+mhi_device_name includes mhi controller name and the name of the MHI channel
+being used by MHI client in userspace to send or receive data using MHI
+protocol.
+
+There is a separate character device file node created for each channel
+specified in MHI device id table. MHI channels are statically defined by MHI
+specification. The list of supported channels is in the channel list variable
+of mhi_device_id table in UCI driver.
+
+Qualcomm MSM Interface(QMI) Channel
+---
+
+Qualcomm MSM Interface(QMI) is a modem control messaging protocol used to
+communicate between software components in the modem and other peripheral
+subsystems. QMI communication is of request/response type or an unsolicited
+event type. libqmi is userspace MHI client which communicates to a QMI service
+using UCI device. It sends a QMI request to a QMI service using MHI channel 14
+or 16. QMI response is received using MHI channel 15 or 17 respectively. libqmi
+is a glib-based library for talking to WWAN modems and devices which speaks QMI
+protocol. For more information about libqmi please refer
+https://www.freedesktop.org/wiki/Software/libqmi/
+
+Usage Example
+~
+
+QMI command to retrieve device mode
+$ sudo qmicli -d /dev/mhi0_QMI --dms-get-model
+[/dev/mhi0_QMI] Device model retrieved:
+Model: 'FN980m'
+
+Other Use Cases
+---
+
+Getting MHI device specific diagnostics information to userspace MHI diagnostic
+client using DIAG channel 4 (Host to device) and 5 (Device to Host).
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v16 4/4] bus: mhi: Add userspace client interface driver

2020-12-09 Thread Hemant Kumar
This MHI client driver allows userspace clients to transfer
raw data between MHI device and host using standard file operations.
Driver instantiates UCI device object which is associated to device
file node. UCI device object instantiates UCI channel object when device
file node is opened. UCI channel object is used to manage MHI channels
by calling MHI core APIs for read and write operations. MHI channels
are started as part of device open(). MHI channels remain in start
state until last release() is called on UCI device file node. Device
file node is created with format

/dev/

Currently it supports QMI channel.

Signed-off-by: Hemant Kumar 
Reviewed-by: Manivannan Sadhasivam 
Reviewed-by: Jeffrey Hugo 
Tested-by: Loic Poulain 
---
 drivers/bus/mhi/Kconfig  |  13 +
 drivers/bus/mhi/Makefile |   3 +
 drivers/bus/mhi/uci.c| 664 +++
 3 files changed, 680 insertions(+)
 create mode 100644 drivers/bus/mhi/uci.c

diff --git a/drivers/bus/mhi/Kconfig b/drivers/bus/mhi/Kconfig
index da5cd0c..5194e8e 100644
--- a/drivers/bus/mhi/Kconfig
+++ b/drivers/bus/mhi/Kconfig
@@ -29,3 +29,16 @@ config MHI_BUS_PCI_GENERIC
  This driver provides MHI PCI controller driver for devices such as
  Qualcomm SDX55 based PCIe modems.
 
+config MHI_UCI
+   tristate "MHI UCI"
+   depends on MHI_BUS
+   help
+ MHI based Userspace Client Interface (UCI) driver is used for
+ transferring raw data between host and device using standard file
+ operations from userspace. Open, read, write, poll and close
+ operations are supported by this driver. Please check
+ mhi_uci_match_table for all supported channels that are exposed to
+ userspace.
+
+ To compile this driver as a module, choose M here: the module will be
+ called mhi_uci.
diff --git a/drivers/bus/mhi/Makefile b/drivers/bus/mhi/Makefile
index 0a2d778..69f2111 100644
--- a/drivers/bus/mhi/Makefile
+++ b/drivers/bus/mhi/Makefile
@@ -4,3 +4,6 @@ obj-y += core/
 obj-$(CONFIG_MHI_BUS_PCI_GENERIC) += mhi_pci_generic.o
 mhi_pci_generic-y += pci_generic.o
 
+# MHI client
+mhi_uci-y := uci.o
+obj-$(CONFIG_MHI_UCI) += mhi_uci.o
diff --git a/drivers/bus/mhi/uci.c b/drivers/bus/mhi/uci.c
new file mode 100644
index 000..1df2377
--- /dev/null
+++ b/drivers/bus/mhi/uci.c
@@ -0,0 +1,664 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2018-2020, The Linux Foundation. All rights reserved.*/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define MHI_UCI_DRIVER_NAME "mhi_uci"
+#define MHI_MAX_UCI_MINORS 128
+
+static DEFINE_IDR(uci_idr);
+static DEFINE_MUTEX(uci_drv_mutex);
+static struct class *uci_dev_class;
+static int uci_dev_major;
+
+/**
+ * struct uci_chan - MHI channel for a UCI device
+ * @udev: associated UCI device object
+ * @ul_wq: wait queue for writer
+ * @write_lock: mutex write lock for ul channel
+ * @dl_wq: wait queue for reader
+ * @read_lock: mutex read lock for dl channel
+ * @dl_pending_lock: spin lock for dl_pending list
+ * @dl_pending: list of dl buffers userspace is waiting to read
+ * @cur_buf: current buffer userspace is reading
+ * @dl_size: size of the current dl buffer userspace is reading
+ * @ref_count: uci_chan reference count
+ */
+struct uci_chan {
+   struct uci_dev *udev;
+   wait_queue_head_t ul_wq;
+
+   /* ul channel lock to synchronize multiple writes */
+   struct mutex write_lock;
+
+   wait_queue_head_t dl_wq;
+
+   /* dl channel lock to synchronize multiple reads */
+   struct mutex read_lock;
+
+   /*
+* protects pending list in bh context, channel release, read and
+* poll
+*/
+   spinlock_t dl_pending_lock;
+
+   struct list_head dl_pending;
+   struct uci_buf *cur_buf;
+   size_t dl_size;
+   struct kref ref_count;
+};
+
+/**
+ * struct uci_buf - UCI buffer
+ * @data: data buffer
+ * @len: length of data buffer
+ * @node: list node of the UCI buffer
+ */
+struct uci_buf {
+   void *data;
+   size_t len;
+   struct list_head node;
+};
+
+/**
+ * struct uci_dev - MHI UCI device
+ * @minor: UCI device node minor number
+ * @mhi_dev: associated mhi device object
+ * @uchan: UCI uplink and downlink channel object
+ * @mtu: max TRE buffer length
+ * @enabled: Flag to track the state of the UCI device
+ * @lock: mutex lock to manage uchan object
+ * @ref_count: uci_dev reference count
+ */
+struct uci_dev {
+   unsigned int minor;
+   struct mhi_device *mhi_dev;
+   struct uci_chan *uchan;
+   size_t mtu;
+   bool enabled;
+
+   /* synchronize open, release and driver remove */
+   struct mutex lock;
+   struct kref ref_count;
+};
+
+static void mhi_uci_dev_chan_release(struct kref *ref)
+{
+   struct uci_buf *buf_itr, *tmp;
+   struct uci_chan *uchan =
+   container_of(ref, struct uci_chan, ref_count);
+
+   if (

[PATCH v16 1/4] bus: mhi: core: Add helper API to return number of free TREs

2020-12-09 Thread Hemant Kumar
Introduce mhi_get_free_desc_count() API to return number
of TREs available to queue buffer. MHI clients can use this
API to know before hand if ring is full without calling queue
API.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
Reviewed-by: Manivannan Sadhasivam 
---
 drivers/bus/mhi/core/main.c | 12 
 include/linux/mhi.h |  9 +
 2 files changed, 21 insertions(+)

diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index 702c31b..74a25e7 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -260,6 +260,18 @@ int mhi_destroy_device(struct device *dev, void *data)
return 0;
 }
 
+int mhi_get_free_desc_count(struct mhi_device *mhi_dev,
+   enum dma_data_direction dir)
+{
+   struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
+   struct mhi_chan *mhi_chan = (dir == DMA_TO_DEVICE) ?
+   mhi_dev->ul_chan : mhi_dev->dl_chan;
+   struct mhi_ring *tre_ring = _chan->tre_ring;
+
+   return get_nr_avail_ring_elements(mhi_cntrl, tre_ring);
+}
+EXPORT_SYMBOL_GPL(mhi_get_free_desc_count);
+
 void mhi_notify(struct mhi_device *mhi_dev, enum mhi_callback cb_reason)
 {
struct mhi_driver *mhi_drv;
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index aa9757e..e36d575 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -599,6 +599,15 @@ void mhi_set_mhi_state(struct mhi_controller *mhi_cntrl,
 void mhi_notify(struct mhi_device *mhi_dev, enum mhi_callback cb_reason);
 
 /**
+ * mhi_get_free_desc_count - Get transfer ring length
+ * Get # of TD available to queue buffers
+ * @mhi_dev: Device associated with the channels
+ * @dir: Direction of the channel
+ */
+int mhi_get_free_desc_count(struct mhi_device *mhi_dev,
+   enum dma_data_direction dir);
+
+/**
  * mhi_prepare_for_power_up - Do pre-initialization before power up.
  *This is optional, call this before power up if
  *the controller does not want bus framework to
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v16 2/4] bus: mhi: core: Move MHI_MAX_MTU to external header file

2020-12-09 Thread Hemant Kumar
Currently this macro is defined in internal MHI header as
a TRE length mask. Moving it to external header allows MHI
client drivers to set this upper bound for the transmit
buffer size.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
Reviewed-by: Manivannan Sadhasivam 
---
 drivers/bus/mhi/core/internal.h | 1 -
 include/linux/mhi.h | 3 +++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
index 6f80ec3..2b9c063 100644
--- a/drivers/bus/mhi/core/internal.h
+++ b/drivers/bus/mhi/core/internal.h
@@ -453,7 +453,6 @@ enum mhi_pm_state {
 #define CMD_EL_PER_RING128
 #define PRIMARY_CMD_RING   0
 #define MHI_DEV_WAKE_DB127
-#define MHI_MAX_MTU0x
 #define MHI_RANDOM_U32_NONZERO(bmsk)   (prandom_u32_max(bmsk) + 1)
 
 enum mhi_er_type {
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index e36d575..f072605 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -15,6 +15,9 @@
 #include 
 #include 
 
+/* MHI client drivers to set this upper bound for tx buffer */
+#define MHI_MAX_MTU 0x
+
 #define MHI_MAX_OEM_PK_HASH_SEGMENTS 16
 
 struct mhi_chan;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



Re: [PATCH] mhi: use irq_flags if client driver configures it

2020-12-09 Thread Hemant Kumar




On 12/7/20 7:55 PM, Carl Huang wrote:

If client driver has specified the irq_flags, mhi uses this specified
irq_flags. Otherwise, mhi uses default irq_flags.

The purpose of this change is to support one MSI vector for QCA6390.
MHI will use one same MSI vector too in this scenario.

In case of one MSI vector, IRQ_NO_BALANCING is needed when irq handler
is requested. The reason is if irq migration happens, the msi_data may
change too. However, the msi_data is already programmed to QCA6390
hardware during initialization phase. This msi_data inconsistence will
result in crash in kernel.

Another issue is in case of one MSI vector, IRQF_NO_SUSPEND will trigger
WARNINGS because QCA6390 wants to disable the IRQ during the suspend.

To avoid above two issues, QCA6390 driver specifies the irq_flags in case
of one MSI vector when mhi_register_controller is called.

Signed-off-by: Carl Huang 
---
  drivers/bus/mhi/core/init.c | 9 +++--
  include/linux/mhi.h | 1 +
  2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
index 0ffdebd..5f74e1e 100644
--- a/drivers/bus/mhi/core/init.c
+++ b/drivers/bus/mhi/core/init.c
@@ -148,12 +148,17 @@ int mhi_init_irq_setup(struct mhi_controller *mhi_cntrl)
  {
struct mhi_event *mhi_event = mhi_cntrl->mhi_event;
struct device *dev = _cntrl->mhi_dev->dev;
+   unsigned long irq_flags = IRQF_SHARED | IRQF_NO_SUSPEND;
int i, ret;
  
+	/* if client driver has set irq_flags, use it */

+   if (mhi_cntrl->irq_flags)
+   irq_flags = mhi_cntrl->irq_flags;
Jeff if i remember correctly your use case also have one dedicated irq 
line for all the MSIs, just want to confirm if you are fine with this 
change ? i was wondering if any input check is required for irq_flags 
passed by controller, or responsibility is on controller for any 
undesired behavior. Like passing IRQF_SHARED and IRQF_ONESHOT when one 
irq line is shared among multiple MSIs.

+
/* Setup BHI_INTVEC IRQ */
ret = request_threaded_irq(mhi_cntrl->irq[0], mhi_intvec_handler,
   mhi_intvec_threaded_handler,
-  IRQF_SHARED | IRQF_NO_SUSPEND,
+  irq_flags,
   "bhi", mhi_cntrl);
if (ret)
return ret;
@@ -171,7 +176,7 @@ int mhi_init_irq_setup(struct mhi_controller *mhi_cntrl)
  
  		ret = request_irq(mhi_cntrl->irq[mhi_event->irq],

  mhi_irq_handler,
- IRQF_SHARED | IRQF_NO_SUSPEND,
+ irq_flags,
  "mhi", mhi_event);
if (ret) {
dev_err(dev, "Error requesting irq:%d for ev:%d\n",
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index d4841e5..f039e58 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -442,6 +442,7 @@ struct mhi_controller {
bool fbc_download;
bool pre_init;
bool wake_set;
+   unsigned long irq_flags;
  };
  
  /**




Thanks,
Hemant

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v4 5/8] bus: mhi: core: Add support to stop or start channel data transfers

2020-12-03 Thread Hemant Kumar




On 12/3/20 5:23 PM, Bhaumik Bhatt wrote:

Some MHI client drivers may want to request a pause or halt of
data transfer activity on their channels. Support for this does
not exist and must be introduced, wherein the channel context is
not reset or cleared but only the STOP channel command is issued.
This would need to be paired with an API that allows resuming the
data transfer activity on channels by use of the START channel
command. Enable this using two new APIs, mhi_start_transfer() and
mhi_stop_transfer().

Signed-off-by: Bhaumik Bhatt


Reviewed-by: Hemant Kumar 
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v4 4/8] bus: mhi: core: Clear configuration from channel context during reset

2020-12-03 Thread Hemant Kumar




On 12/3/20 5:23 PM, Bhaumik Bhatt wrote:

When clearing up the channel context after client drivers are
done using channels, the configuration is currently not being
reset entirely. Ensure this is done to appropriately handle
issues where clients unaware of the context state end up calling
functions which expect a context.

Signed-off-by: Bhaumik Bhatt 
---


Reviewed-by: Hemant Kumar 

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v4 1/8] bus: mhi: core: Allow sending the STOP channel command

2020-12-03 Thread Hemant Kumar




On 12/3/20 5:23 PM, Bhaumik Bhatt wrote:

Add support to allow sending the STOP channel command. If a
client driver would like to STOP a channel and have the device
retain the channel context instead of issuing a RESET to it and
clearing the context, this would provide support for it after
the ability to send this command is exposed to clients.

Signed-off-by: Bhaumik Bhatt 
---


Reviewed-by: Hemant Kumar 
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


[RESEND v15 4/4] bus: mhi: Add userspace client interface driver

2020-12-03 Thread Hemant Kumar
This MHI client driver allows userspace clients to transfer
raw data between MHI device and host using standard file operations.
Driver instantiates UCI device object which is associated to device
file node. UCI device object instantiates UCI channel object when device
file node is opened. UCI channel object is used to manage MHI channels
by calling MHI core APIs for read and write operations. MHI channels
are started as part of device open(). MHI channels remain in start
state until last release() is called on UCI device file node. Device
file node is created with format

/dev/

Currently it supports QMI channel.

Signed-off-by: Hemant Kumar 
Reviewed-by: Manivannan Sadhasivam 
Reviewed-by: Jeffrey Hugo 
Tested-by: Loic Poulain 
---
 drivers/bus/mhi/Kconfig  |  13 +
 drivers/bus/mhi/Makefile |   3 +
 drivers/bus/mhi/uci.c| 664 +++
 3 files changed, 680 insertions(+)
 create mode 100644 drivers/bus/mhi/uci.c

diff --git a/drivers/bus/mhi/Kconfig b/drivers/bus/mhi/Kconfig
index da5cd0c..5194e8e 100644
--- a/drivers/bus/mhi/Kconfig
+++ b/drivers/bus/mhi/Kconfig
@@ -29,3 +29,16 @@ config MHI_BUS_PCI_GENERIC
  This driver provides MHI PCI controller driver for devices such as
  Qualcomm SDX55 based PCIe modems.
 
+config MHI_UCI
+   tristate "MHI UCI"
+   depends on MHI_BUS
+   help
+ MHI based Userspace Client Interface (UCI) driver is used for
+ transferring raw data between host and device using standard file
+ operations from userspace. Open, read, write, poll and close
+ operations are supported by this driver. Please check
+ mhi_uci_match_table for all supported channels that are exposed to
+ userspace.
+
+ To compile this driver as a module, choose M here: the module will be
+ called mhi_uci.
diff --git a/drivers/bus/mhi/Makefile b/drivers/bus/mhi/Makefile
index 0a2d778..69f2111 100644
--- a/drivers/bus/mhi/Makefile
+++ b/drivers/bus/mhi/Makefile
@@ -4,3 +4,6 @@ obj-y += core/
 obj-$(CONFIG_MHI_BUS_PCI_GENERIC) += mhi_pci_generic.o
 mhi_pci_generic-y += pci_generic.o
 
+# MHI client
+mhi_uci-y := uci.o
+obj-$(CONFIG_MHI_UCI) += mhi_uci.o
diff --git a/drivers/bus/mhi/uci.c b/drivers/bus/mhi/uci.c
new file mode 100644
index 000..1df2377
--- /dev/null
+++ b/drivers/bus/mhi/uci.c
@@ -0,0 +1,664 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2018-2020, The Linux Foundation. All rights reserved.*/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define MHI_UCI_DRIVER_NAME "mhi_uci"
+#define MHI_MAX_UCI_MINORS 128
+
+static DEFINE_IDR(uci_idr);
+static DEFINE_MUTEX(uci_drv_mutex);
+static struct class *uci_dev_class;
+static int uci_dev_major;
+
+/**
+ * struct uci_chan - MHI channel for a UCI device
+ * @udev: associated UCI device object
+ * @ul_wq: wait queue for writer
+ * @write_lock: mutex write lock for ul channel
+ * @dl_wq: wait queue for reader
+ * @read_lock: mutex read lock for dl channel
+ * @dl_pending_lock: spin lock for dl_pending list
+ * @dl_pending: list of dl buffers userspace is waiting to read
+ * @cur_buf: current buffer userspace is reading
+ * @dl_size: size of the current dl buffer userspace is reading
+ * @ref_count: uci_chan reference count
+ */
+struct uci_chan {
+   struct uci_dev *udev;
+   wait_queue_head_t ul_wq;
+
+   /* ul channel lock to synchronize multiple writes */
+   struct mutex write_lock;
+
+   wait_queue_head_t dl_wq;
+
+   /* dl channel lock to synchronize multiple reads */
+   struct mutex read_lock;
+
+   /*
+* protects pending list in bh context, channel release, read and
+* poll
+*/
+   spinlock_t dl_pending_lock;
+
+   struct list_head dl_pending;
+   struct uci_buf *cur_buf;
+   size_t dl_size;
+   struct kref ref_count;
+};
+
+/**
+ * struct uci_buf - UCI buffer
+ * @data: data buffer
+ * @len: length of data buffer
+ * @node: list node of the UCI buffer
+ */
+struct uci_buf {
+   void *data;
+   size_t len;
+   struct list_head node;
+};
+
+/**
+ * struct uci_dev - MHI UCI device
+ * @minor: UCI device node minor number
+ * @mhi_dev: associated mhi device object
+ * @uchan: UCI uplink and downlink channel object
+ * @mtu: max TRE buffer length
+ * @enabled: Flag to track the state of the UCI device
+ * @lock: mutex lock to manage uchan object
+ * @ref_count: uci_dev reference count
+ */
+struct uci_dev {
+   unsigned int minor;
+   struct mhi_device *mhi_dev;
+   struct uci_chan *uchan;
+   size_t mtu;
+   bool enabled;
+
+   /* synchronize open, release and driver remove */
+   struct mutex lock;
+   struct kref ref_count;
+};
+
+static void mhi_uci_dev_chan_release(struct kref *ref)
+{
+   struct uci_buf *buf_itr, *tmp;
+   struct uci_chan *uchan =
+   container_of(ref, struct uci_chan, ref_count);
+
+   if (

[RESEND v15 2/4] bus: mhi: core: Move MHI_MAX_MTU to external header file

2020-12-03 Thread Hemant Kumar
Currently this macro is defined in internal MHI header as
a TRE length mask. Moving it to external header allows MHI
client drivers to set this upper bound for the transmit
buffer size.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
Reviewed-by: Manivannan Sadhasivam 
---
 drivers/bus/mhi/core/internal.h | 1 -
 include/linux/mhi.h | 3 +++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
index 6f80ec3..2b9c063 100644
--- a/drivers/bus/mhi/core/internal.h
+++ b/drivers/bus/mhi/core/internal.h
@@ -453,7 +453,6 @@ enum mhi_pm_state {
 #define CMD_EL_PER_RING128
 #define PRIMARY_CMD_RING   0
 #define MHI_DEV_WAKE_DB127
-#define MHI_MAX_MTU0x
 #define MHI_RANDOM_U32_NONZERO(bmsk)   (prandom_u32_max(bmsk) + 1)
 
 enum mhi_er_type {
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index e36d575..f072605 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -15,6 +15,9 @@
 #include 
 #include 
 
+/* MHI client drivers to set this upper bound for tx buffer */
+#define MHI_MAX_MTU 0x
+
 #define MHI_MAX_OEM_PK_HASH_SEGMENTS 16
 
 struct mhi_chan;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[RESEND v15 3/4] docs: Add documentation for userspace client interface

2020-12-03 Thread Hemant Kumar
MHI userspace client driver is creating device file node
for user application to perform file operations. File
operations are handled by MHI core driver. Currently
QMI MHI channel is supported by this driver.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
---
 Documentation/mhi/index.rst |  1 +
 Documentation/mhi/uci.rst   | 95 +
 2 files changed, 96 insertions(+)
 create mode 100644 Documentation/mhi/uci.rst

diff --git a/Documentation/mhi/index.rst b/Documentation/mhi/index.rst
index 1d8dec3..c75a371 100644
--- a/Documentation/mhi/index.rst
+++ b/Documentation/mhi/index.rst
@@ -9,6 +9,7 @@ MHI
 
mhi
topology
+   uci
 
 .. only::  subproject and html
 
diff --git a/Documentation/mhi/uci.rst b/Documentation/mhi/uci.rst
new file mode 100644
index 000..df1f0c4
--- /dev/null
+++ b/Documentation/mhi/uci.rst
@@ -0,0 +1,95 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=
+Userspace Client Interface (UCI)
+=
+
+UCI driver enables userspace clients to communicate to external MHI devices
+like modem and WLAN. UCI driver probe creates standard character device file
+nodes for userspace clients to perform open, read, write, poll and release file
+operations. UCI device object represents UCI device file node which gets
+instantiated as part of MHI UCI driver probe. UCI channel object represents
+MHI uplink or downlink channel.
+
+Operations
+==
+
+open
+
+
+Instantiates UCI channel object and starts MHI channels to move it to running
+state. Inbound buffers are queued to downlink channel transfer ring. Every
+subsequent open() increments UCI device reference count as well as UCI channel
+reference count.
+
+read
+
+
+When data transfer is completed on downlink channel, transfer ring element
+buffer is copied to pending list. Reader is unblocked and data is copied to
+userspace buffer. Transfer ring element buffer is queued back to downlink
+channel transfer ring.
+
+write
+-
+
+Write buffer is queued to uplink channel transfer ring if ring is not full. 
Upon
+uplink transfer completion buffer is freed.
+
+poll
+
+
+Returns EPOLLIN | EPOLLRDNORM mask if pending list has buffers to be read by
+userspace. Returns EPOLLOUT | EPOLLWRNORM mask if MHI uplink channel transfer
+ring is not empty.  When the uplink channel transfer ring is non-empty, more
+data may be sent to the device. Returns EPOLLERR when UCI driver is removed.
+
+release
+---
+
+Decrements UCI device reference count and UCI channel reference count. Upon
+last release() UCI channel clean up is performed. MHI channel moves to disable
+state and inbound buffers are freed.
+
+Usage
+=
+
+Device file node is created with format:-
+
+/dev/
+
+mhi_device_name includes mhi controller name and the name of the MHI channel
+being used by MHI client in userspace to send or receive data using MHI
+protocol.
+
+There is a separate character device file node created for each channel
+specified in MHI device id table. MHI channels are statically defined by MHI
+specification. The list of supported channels is in the channel list variable
+of mhi_device_id table in UCI driver.
+
+Qualcomm MSM Interface(QMI) Channel
+---
+
+Qualcomm MSM Interface(QMI) is a modem control messaging protocol used to
+communicate between software components in the modem and other peripheral
+subsystems. QMI communication is of request/response type or an unsolicited
+event type. libqmi is userspace MHI client which communicates to a QMI service
+using UCI device. It sends a QMI request to a QMI service using MHI channel 14
+or 16. QMI response is received using MHI channel 15 or 17 respectively. libqmi
+is a glib-based library for talking to WWAN modems and devices which speaks QMI
+protocol. For more information about libqmi please refer
+https://www.freedesktop.org/wiki/Software/libqmi/
+
+Usage Example
+~
+
+QMI command to retrieve device mode
+$ sudo qmicli -d /dev/mhi0_QMI --dms-get-model
+[/dev/mhi0_QMI] Device model retrieved:
+Model: 'FN980m'
+
+Other Use Cases
+---
+
+Getting MHI device specific diagnostics information to userspace MHI diagnostic
+client using DIAG channel 4 (Host to device) and 5 (Device to Host).
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[RESEND v15 1/4] bus: mhi: core: Add helper API to return number of free TREs

2020-12-03 Thread Hemant Kumar
Introduce mhi_get_free_desc_count() API to return number
of TREs available to queue buffer. MHI clients can use this
API to know before hand if ring is full without calling queue
API.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
Reviewed-by: Manivannan Sadhasivam 
---
 drivers/bus/mhi/core/main.c | 12 
 include/linux/mhi.h |  9 +
 2 files changed, 21 insertions(+)

diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index 702c31b..74a25e7 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -260,6 +260,18 @@ int mhi_destroy_device(struct device *dev, void *data)
return 0;
 }
 
+int mhi_get_free_desc_count(struct mhi_device *mhi_dev,
+   enum dma_data_direction dir)
+{
+   struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
+   struct mhi_chan *mhi_chan = (dir == DMA_TO_DEVICE) ?
+   mhi_dev->ul_chan : mhi_dev->dl_chan;
+   struct mhi_ring *tre_ring = _chan->tre_ring;
+
+   return get_nr_avail_ring_elements(mhi_cntrl, tre_ring);
+}
+EXPORT_SYMBOL_GPL(mhi_get_free_desc_count);
+
 void mhi_notify(struct mhi_device *mhi_dev, enum mhi_callback cb_reason)
 {
struct mhi_driver *mhi_drv;
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index aa9757e..e36d575 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -599,6 +599,15 @@ void mhi_set_mhi_state(struct mhi_controller *mhi_cntrl,
 void mhi_notify(struct mhi_device *mhi_dev, enum mhi_callback cb_reason);
 
 /**
+ * mhi_get_free_desc_count - Get transfer ring length
+ * Get # of TD available to queue buffers
+ * @mhi_dev: Device associated with the channels
+ * @dir: Direction of the channel
+ */
+int mhi_get_free_desc_count(struct mhi_device *mhi_dev,
+   enum dma_data_direction dir);
+
+/**
  * mhi_prepare_for_power_up - Do pre-initialization before power up.
  *This is optional, call this before power up if
  *the controller does not want bus framework to
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[RESEND v15 0/4] userspace MHI client interface driver

2020-12-03 Thread Hemant Kumar
This patch series adds support for UCI driver. UCI driver enables userspace
clients to communicate to external MHI devices like modem and WLAN. UCI driver
probe creates standard character device file nodes for userspace clients to
perform open, read, write, poll and release file operations. These file
operations call MHI core layer APIs to perform data transfer using MHI bus
to communicate with MHI device. Patch is tested using arm64 and x86 based
platform.

v15:
- Updated documentation related to poll and release operations.

V14:
- Fixed device file node format to /dev/ instead of
  /dev/mhi_ because "mhi" is already part of mhi device name.
  For example old format: /dev/mhi_mhi0_QMI new format: /dev/mhi0_QMI.
- Updated MHI documentation to reflect index mhi controller name in
  QMI usage example.

V13:
- Removed LOOPBACK channel from mhi_device_id table from this patch series.
  Pushing a new patch series to add support for LOOPBACK channel and the user
  space test application. Also removed the description from kernel 
documentation.
- Added QMI channel to mhi_device_id table. QMI channel has existing libqmi
  support from user space.
- Updated kernel Documentation for QMI channel and provided external reference
  for libqmi.
- Updated device file node name by appending mhi device name only, which already
  includes mhi controller device name.

V12:
- Added loopback test driver under selftest/drivers/mhi. Updated kernel
  documentation for the usage of the loopback test application.
- Addressed review comments for renaming variable names, updated inline
  comments and removed two redundant dev_dbg.

V11:
- Fixed review comments for UCI documentation by expanding TLAs and rewording
  some sentences.

V10:
- Replaced mutex_lock with mutex_lock_interruptible in read() and write() file
  ops call back.

V9:
- Renamed dl_lock to dl_pending _lock and pending list to dl_pending for
  clarity.
- Used read lock to protect cur_buf.
- Change transfer status check logic and only consider 0 and -EOVERFLOW as
  only success.
- Added __int to module init function.
- Print channel name instead of minor number upon successful probe.

V8:
- Fixed kernel test robot compilation error by changing %lu to %zu for
  size_t.
- Replaced uci with UCI in Kconfig, commit text, and comments in driver
  code.
- Fixed minor style related comments.

V7:
- Decoupled uci device and uci channel objects. uci device is
  associated with device file node. uci channel is associated
  with MHI channels. uci device refers to uci channel to perform
  MHI channel operations for device file operations like read()
  and write(). uci device increments its reference count for
  every open(). uci device calls mhi_uci_dev_start_chan() to start
  the MHI channel. uci channel object is tracking number of times
  MHI channel is referred. This allows to keep the MHI channel in
  start state until last release() is called. After that uci channel
  reference count goes to 0 and uci channel clean up is performed
  which stops the MHI channel. After the last call to release() if
  driver is removed uci reference count becomes 0 and uci object is
  cleaned up.
- Use separate uci channel read and write lock to fine grain locking
  between reader and writer.
- Use uci device lock to synchronize open, release and driver remove.
- Optimize for downlink only or uplink only UCI device.

V6:
- Moved uci.c to mhi directory.
- Updated Kconfig to add module information.
- Updated Makefile to rename uci object file name as mhi_uci
- Removed kref for open count

V5:
- Removed mhi_uci_drv structure.
- Used idr instead of creating global list of uci devices.
- Used kref instead of local ref counting for uci device and
  open count.
- Removed unlikely macro.

V4:
- Fix locking to protect proper struct members.
- Updated documentation describing uci client driver use cases.
- Fixed uci ref counting in mhi_uci_open for error case.
- Addressed style related review comments.

V3: Added documentation for MHI UCI driver.

V2:
- Added mutex lock to prevent multiple readers to access same
- mhi buffer which can result into use after free.
Hemant Kumar (4):
  bus: mhi: core: Add helper API to return number of free TREs
  bus: mhi: core: Move MHI_MAX_MTU to external header file
  docs: Add documentation for userspace client interface
  bus: mhi: Add userspace client interface driver

 Documentation/mhi/index.rst |   1 +
 Documentation/mhi/uci.rst   |  95 ++
 drivers/bus/mhi/Kconfig |  13 +
 drivers/bus/mhi/Makefile|   3 +
 drivers/bus/mhi/core/internal.h |   1 -
 drivers/bus/mhi/core/main.c |  12 +
 drivers/bus/mhi/uci.c   | 664 
 include/linux/mhi.h |  12 +
 8 files changed, 800 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/mhi/uci.rst
 create mode 100644 drivers/bus/mhi/uci.c

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux 

Re: [PATCH v15 4/4] bus: mhi: Add userspace client interface driver

2020-12-03 Thread Hemant Kumar



On 12/3/20 3:45 PM, Jeffrey Hugo wrote:

On 12/3/2020 3:45 PM, Hemant Kumar wrote:

This MHI client driver allows userspace clients to transfer
raw data between MHI device and host using standard file operations.
Driver instantiates UCI device object which is associated to device
file node. UCI device object instantiates UCI channel object when device
file node is opened. UCI channel object is used to manage MHI channels
by calling MHI core APIs for read and write operations. MHI channels
are started as part of device open(). MHI channels remain in start
state until last release() is called on UCI device file node. Device
file node is created with format

/dev/

Currently it supports QMI channel.

Signed-off-by: Hemant Kumar 
Reviewed-by: Manivannan Sadhasivam 
Reviewed-by: Jeffrey Hugo 


You dropped Loic's tested by.  Was that a mistake, or did something 
actually change which would invalidate his testing?



Thanks for pointing this out, it was my bad. Re-sending the patch.
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


[PATCH v15 3/4] docs: Add documentation for userspace client interface

2020-12-03 Thread Hemant Kumar
MHI userspace client driver is creating device file node
for user application to perform file operations. File
operations are handled by MHI core driver. Currently
QMI MHI channel is supported by this driver.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
---
 Documentation/mhi/index.rst |  1 +
 Documentation/mhi/uci.rst   | 95 +
 2 files changed, 96 insertions(+)
 create mode 100644 Documentation/mhi/uci.rst

diff --git a/Documentation/mhi/index.rst b/Documentation/mhi/index.rst
index 1d8dec3..c75a371 100644
--- a/Documentation/mhi/index.rst
+++ b/Documentation/mhi/index.rst
@@ -9,6 +9,7 @@ MHI
 
mhi
topology
+   uci
 
 .. only::  subproject and html
 
diff --git a/Documentation/mhi/uci.rst b/Documentation/mhi/uci.rst
new file mode 100644
index 000..df1f0c4
--- /dev/null
+++ b/Documentation/mhi/uci.rst
@@ -0,0 +1,95 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=
+Userspace Client Interface (UCI)
+=
+
+UCI driver enables userspace clients to communicate to external MHI devices
+like modem and WLAN. UCI driver probe creates standard character device file
+nodes for userspace clients to perform open, read, write, poll and release file
+operations. UCI device object represents UCI device file node which gets
+instantiated as part of MHI UCI driver probe. UCI channel object represents
+MHI uplink or downlink channel.
+
+Operations
+==
+
+open
+
+
+Instantiates UCI channel object and starts MHI channels to move it to running
+state. Inbound buffers are queued to downlink channel transfer ring. Every
+subsequent open() increments UCI device reference count as well as UCI channel
+reference count.
+
+read
+
+
+When data transfer is completed on downlink channel, transfer ring element
+buffer is copied to pending list. Reader is unblocked and data is copied to
+userspace buffer. Transfer ring element buffer is queued back to downlink
+channel transfer ring.
+
+write
+-
+
+Write buffer is queued to uplink channel transfer ring if ring is not full. 
Upon
+uplink transfer completion buffer is freed.
+
+poll
+
+
+Returns EPOLLIN | EPOLLRDNORM mask if pending list has buffers to be read by
+userspace. Returns EPOLLOUT | EPOLLWRNORM mask if MHI uplink channel transfer
+ring is not empty.  When the uplink channel transfer ring is non-empty, more
+data may be sent to the device. Returns EPOLLERR when UCI driver is removed.
+
+release
+---
+
+Decrements UCI device reference count and UCI channel reference count. Upon
+last release() UCI channel clean up is performed. MHI channel moves to disable
+state and inbound buffers are freed.
+
+Usage
+=
+
+Device file node is created with format:-
+
+/dev/
+
+mhi_device_name includes mhi controller name and the name of the MHI channel
+being used by MHI client in userspace to send or receive data using MHI
+protocol.
+
+There is a separate character device file node created for each channel
+specified in MHI device id table. MHI channels are statically defined by MHI
+specification. The list of supported channels is in the channel list variable
+of mhi_device_id table in UCI driver.
+
+Qualcomm MSM Interface(QMI) Channel
+---
+
+Qualcomm MSM Interface(QMI) is a modem control messaging protocol used to
+communicate between software components in the modem and other peripheral
+subsystems. QMI communication is of request/response type or an unsolicited
+event type. libqmi is userspace MHI client which communicates to a QMI service
+using UCI device. It sends a QMI request to a QMI service using MHI channel 14
+or 16. QMI response is received using MHI channel 15 or 17 respectively. libqmi
+is a glib-based library for talking to WWAN modems and devices which speaks QMI
+protocol. For more information about libqmi please refer
+https://www.freedesktop.org/wiki/Software/libqmi/
+
+Usage Example
+~
+
+QMI command to retrieve device mode
+$ sudo qmicli -d /dev/mhi0_QMI --dms-get-model
+[/dev/mhi0_QMI] Device model retrieved:
+Model: 'FN980m'
+
+Other Use Cases
+---
+
+Getting MHI device specific diagnostics information to userspace MHI diagnostic
+client using DIAG channel 4 (Host to device) and 5 (Device to Host).
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v15 1/4] bus: mhi: core: Add helper API to return number of free TREs

2020-12-03 Thread Hemant Kumar
Introduce mhi_get_free_desc_count() API to return number
of TREs available to queue buffer. MHI clients can use this
API to know before hand if ring is full without calling queue
API.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
Reviewed-by: Manivannan Sadhasivam 
---
 drivers/bus/mhi/core/main.c | 12 
 include/linux/mhi.h |  9 +
 2 files changed, 21 insertions(+)

diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index 702c31b..74a25e7 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -260,6 +260,18 @@ int mhi_destroy_device(struct device *dev, void *data)
return 0;
 }
 
+int mhi_get_free_desc_count(struct mhi_device *mhi_dev,
+   enum dma_data_direction dir)
+{
+   struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
+   struct mhi_chan *mhi_chan = (dir == DMA_TO_DEVICE) ?
+   mhi_dev->ul_chan : mhi_dev->dl_chan;
+   struct mhi_ring *tre_ring = _chan->tre_ring;
+
+   return get_nr_avail_ring_elements(mhi_cntrl, tre_ring);
+}
+EXPORT_SYMBOL_GPL(mhi_get_free_desc_count);
+
 void mhi_notify(struct mhi_device *mhi_dev, enum mhi_callback cb_reason)
 {
struct mhi_driver *mhi_drv;
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index aa9757e..e36d575 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -599,6 +599,15 @@ void mhi_set_mhi_state(struct mhi_controller *mhi_cntrl,
 void mhi_notify(struct mhi_device *mhi_dev, enum mhi_callback cb_reason);
 
 /**
+ * mhi_get_free_desc_count - Get transfer ring length
+ * Get # of TD available to queue buffers
+ * @mhi_dev: Device associated with the channels
+ * @dir: Direction of the channel
+ */
+int mhi_get_free_desc_count(struct mhi_device *mhi_dev,
+   enum dma_data_direction dir);
+
+/**
  * mhi_prepare_for_power_up - Do pre-initialization before power up.
  *This is optional, call this before power up if
  *the controller does not want bus framework to
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v15 2/4] bus: mhi: core: Move MHI_MAX_MTU to external header file

2020-12-03 Thread Hemant Kumar
Currently this macro is defined in internal MHI header as
a TRE length mask. Moving it to external header allows MHI
client drivers to set this upper bound for the transmit
buffer size.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
Reviewed-by: Manivannan Sadhasivam 
---
 drivers/bus/mhi/core/internal.h | 1 -
 include/linux/mhi.h | 3 +++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
index 6f80ec3..2b9c063 100644
--- a/drivers/bus/mhi/core/internal.h
+++ b/drivers/bus/mhi/core/internal.h
@@ -453,7 +453,6 @@ enum mhi_pm_state {
 #define CMD_EL_PER_RING128
 #define PRIMARY_CMD_RING   0
 #define MHI_DEV_WAKE_DB127
-#define MHI_MAX_MTU0x
 #define MHI_RANDOM_U32_NONZERO(bmsk)   (prandom_u32_max(bmsk) + 1)
 
 enum mhi_er_type {
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index e36d575..f072605 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -15,6 +15,9 @@
 #include 
 #include 
 
+/* MHI client drivers to set this upper bound for tx buffer */
+#define MHI_MAX_MTU 0x
+
 #define MHI_MAX_OEM_PK_HASH_SEGMENTS 16
 
 struct mhi_chan;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v15 4/4] bus: mhi: Add userspace client interface driver

2020-12-03 Thread Hemant Kumar
This MHI client driver allows userspace clients to transfer
raw data between MHI device and host using standard file operations.
Driver instantiates UCI device object which is associated to device
file node. UCI device object instantiates UCI channel object when device
file node is opened. UCI channel object is used to manage MHI channels
by calling MHI core APIs for read and write operations. MHI channels
are started as part of device open(). MHI channels remain in start
state until last release() is called on UCI device file node. Device
file node is created with format

/dev/

Currently it supports QMI channel.

Signed-off-by: Hemant Kumar 
Reviewed-by: Manivannan Sadhasivam 
Reviewed-by: Jeffrey Hugo 
---
 drivers/bus/mhi/Kconfig  |  13 +
 drivers/bus/mhi/Makefile |   3 +
 drivers/bus/mhi/uci.c| 664 +++
 3 files changed, 680 insertions(+)
 create mode 100644 drivers/bus/mhi/uci.c

diff --git a/drivers/bus/mhi/Kconfig b/drivers/bus/mhi/Kconfig
index da5cd0c..5194e8e 100644
--- a/drivers/bus/mhi/Kconfig
+++ b/drivers/bus/mhi/Kconfig
@@ -29,3 +29,16 @@ config MHI_BUS_PCI_GENERIC
  This driver provides MHI PCI controller driver for devices such as
  Qualcomm SDX55 based PCIe modems.
 
+config MHI_UCI
+   tristate "MHI UCI"
+   depends on MHI_BUS
+   help
+ MHI based Userspace Client Interface (UCI) driver is used for
+ transferring raw data between host and device using standard file
+ operations from userspace. Open, read, write, poll and close
+ operations are supported by this driver. Please check
+ mhi_uci_match_table for all supported channels that are exposed to
+ userspace.
+
+ To compile this driver as a module, choose M here: the module will be
+ called mhi_uci.
diff --git a/drivers/bus/mhi/Makefile b/drivers/bus/mhi/Makefile
index 0a2d778..69f2111 100644
--- a/drivers/bus/mhi/Makefile
+++ b/drivers/bus/mhi/Makefile
@@ -4,3 +4,6 @@ obj-y += core/
 obj-$(CONFIG_MHI_BUS_PCI_GENERIC) += mhi_pci_generic.o
 mhi_pci_generic-y += pci_generic.o
 
+# MHI client
+mhi_uci-y := uci.o
+obj-$(CONFIG_MHI_UCI) += mhi_uci.o
diff --git a/drivers/bus/mhi/uci.c b/drivers/bus/mhi/uci.c
new file mode 100644
index 000..1df2377
--- /dev/null
+++ b/drivers/bus/mhi/uci.c
@@ -0,0 +1,664 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2018-2020, The Linux Foundation. All rights reserved.*/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define MHI_UCI_DRIVER_NAME "mhi_uci"
+#define MHI_MAX_UCI_MINORS 128
+
+static DEFINE_IDR(uci_idr);
+static DEFINE_MUTEX(uci_drv_mutex);
+static struct class *uci_dev_class;
+static int uci_dev_major;
+
+/**
+ * struct uci_chan - MHI channel for a UCI device
+ * @udev: associated UCI device object
+ * @ul_wq: wait queue for writer
+ * @write_lock: mutex write lock for ul channel
+ * @dl_wq: wait queue for reader
+ * @read_lock: mutex read lock for dl channel
+ * @dl_pending_lock: spin lock for dl_pending list
+ * @dl_pending: list of dl buffers userspace is waiting to read
+ * @cur_buf: current buffer userspace is reading
+ * @dl_size: size of the current dl buffer userspace is reading
+ * @ref_count: uci_chan reference count
+ */
+struct uci_chan {
+   struct uci_dev *udev;
+   wait_queue_head_t ul_wq;
+
+   /* ul channel lock to synchronize multiple writes */
+   struct mutex write_lock;
+
+   wait_queue_head_t dl_wq;
+
+   /* dl channel lock to synchronize multiple reads */
+   struct mutex read_lock;
+
+   /*
+* protects pending list in bh context, channel release, read and
+* poll
+*/
+   spinlock_t dl_pending_lock;
+
+   struct list_head dl_pending;
+   struct uci_buf *cur_buf;
+   size_t dl_size;
+   struct kref ref_count;
+};
+
+/**
+ * struct uci_buf - UCI buffer
+ * @data: data buffer
+ * @len: length of data buffer
+ * @node: list node of the UCI buffer
+ */
+struct uci_buf {
+   void *data;
+   size_t len;
+   struct list_head node;
+};
+
+/**
+ * struct uci_dev - MHI UCI device
+ * @minor: UCI device node minor number
+ * @mhi_dev: associated mhi device object
+ * @uchan: UCI uplink and downlink channel object
+ * @mtu: max TRE buffer length
+ * @enabled: Flag to track the state of the UCI device
+ * @lock: mutex lock to manage uchan object
+ * @ref_count: uci_dev reference count
+ */
+struct uci_dev {
+   unsigned int minor;
+   struct mhi_device *mhi_dev;
+   struct uci_chan *uchan;
+   size_t mtu;
+   bool enabled;
+
+   /* synchronize open, release and driver remove */
+   struct mutex lock;
+   struct kref ref_count;
+};
+
+static void mhi_uci_dev_chan_release(struct kref *ref)
+{
+   struct uci_buf *buf_itr, *tmp;
+   struct uci_chan *uchan =
+   container_of(ref, struct uci_chan, ref_count);
+
+   if (

[PATCH v15 0/4] userspace MHI client interface driver

2020-12-03 Thread Hemant Kumar
This patch series adds support for UCI driver. UCI driver enables userspace
clients to communicate to external MHI devices like modem and WLAN. UCI driver
probe creates standard character device file nodes for userspace clients to
perform open, read, write, poll and release file operations. These file
operations call MHI core layer APIs to perform data transfer using MHI bus
to communicate with MHI device. Patch is tested using arm64 and x86 based
platform.

v15:
- Updated documentation related to poll and release operations.

V14:
- Fixed device file node format to /dev/ instead of
  /dev/mhi_ because "mhi" is already part of mhi device name.
  For example old format: /dev/mhi_mhi0_QMI new format: /dev/mhi0_QMI.
- Updated MHI documentation to reflect index mhi controller name in
  QMI usage example.

V13:
- Removed LOOPBACK channel from mhi_device_id table from this patch series.
  Pushing a new patch series to add support for LOOPBACK channel and the user
  space test application. Also removed the description from kernel 
documentation.
- Added QMI channel to mhi_device_id table. QMI channel has existing libqmi
  support from user space.
- Updated kernel Documentation for QMI channel and provided external reference
  for libqmi.
- Updated device file node name by appending mhi device name only, which already
  includes mhi controller device name.

V12:
- Added loopback test driver under selftest/drivers/mhi. Updated kernel
  documentation for the usage of the loopback test application.
- Addressed review comments for renaming variable names, updated inline
  comments and removed two redundant dev_dbg.

V11:
- Fixed review comments for UCI documentation by expanding TLAs and rewording
  some sentences.

V10:
- Replaced mutex_lock with mutex_lock_interruptible in read() and write() file
  ops call back.

V9:
- Renamed dl_lock to dl_pending _lock and pending list to dl_pending for
  clarity.
- Used read lock to protect cur_buf.
- Change transfer status check logic and only consider 0 and -EOVERFLOW as
  only success.
- Added __int to module init function.
- Print channel name instead of minor number upon successful probe.

V8:
- Fixed kernel test robot compilation error by changing %lu to %zu for
  size_t.
- Replaced uci with UCI in Kconfig, commit text, and comments in driver
  code.
- Fixed minor style related comments.

V7:
- Decoupled uci device and uci channel objects. uci device is
  associated with device file node. uci channel is associated
  with MHI channels. uci device refers to uci channel to perform
  MHI channel operations for device file operations like read()
  and write(). uci device increments its reference count for
  every open(). uci device calls mhi_uci_dev_start_chan() to start
  the MHI channel. uci channel object is tracking number of times
  MHI channel is referred. This allows to keep the MHI channel in
  start state until last release() is called. After that uci channel
  reference count goes to 0 and uci channel clean up is performed
  which stops the MHI channel. After the last call to release() if
  driver is removed uci reference count becomes 0 and uci object is
  cleaned up.
- Use separate uci channel read and write lock to fine grain locking
  between reader and writer.
- Use uci device lock to synchronize open, release and driver remove.
- Optimize for downlink only or uplink only UCI device.

V6:
- Moved uci.c to mhi directory.
- Updated Kconfig to add module information.
- Updated Makefile to rename uci object file name as mhi_uci
- Removed kref for open count

V5:
- Removed mhi_uci_drv structure.
- Used idr instead of creating global list of uci devices.
- Used kref instead of local ref counting for uci device and
  open count.
- Removed unlikely macro.

V4:
- Fix locking to protect proper struct members.
- Updated documentation describing uci client driver use cases.
- Fixed uci ref counting in mhi_uci_open for error case.
- Addressed style related review comments.

V3: Added documentation for MHI UCI driver.

V2:
- Added mutex lock to prevent multiple readers to access same
- mhi buffer which can result into use after free.
Hemant Kumar (4):
  bus: mhi: core: Add helper API to return number of free TREs
  bus: mhi: core: Move MHI_MAX_MTU to external header file
  docs: Add documentation for userspace client interface
  bus: mhi: Add userspace client interface driver

 Documentation/mhi/index.rst |   1 +
 Documentation/mhi/uci.rst   |  95 ++
 drivers/bus/mhi/Kconfig |  13 +
 drivers/bus/mhi/Makefile|   3 +
 drivers/bus/mhi/core/internal.h |   1 -
 drivers/bus/mhi/core/main.c |  12 +
 drivers/bus/mhi/uci.c   | 664 
 include/linux/mhi.h |  12 +
 8 files changed, 800 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/mhi/uci.rst
 create mode 100644 drivers/bus/mhi/uci.c

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux 

Re: [PATCH v14 3/4] docs: Add documentation for userspace client interface

2020-12-03 Thread Hemant Kumar

Hi Jeff

On 12/3/20 12:38 PM, jh...@codeaurora.org wrote:

On 2020-12-01 19:59, Hemant Kumar wrote:

MHI userspace client driver is creating device file node
for user application to perform file operations. File
operations are handled by MHI core driver. Currently
QMI MHI channel is supported by this driver.

Signed-off-by: Hemant Kumar 


Two minor nits below.  With those -
Reviewed-by: Jeffrey Hugo 


---
 Documentation/mhi/index.rst |  1 +
 Documentation/mhi/uci.rst   | 94 
+

 2 files changed, 95 insertions(+)
 create mode 100644 Documentation/mhi/uci.rst

diff --git a/Documentation/mhi/index.rst b/Documentation/mhi/index.rst
index 1d8dec3..c75a371 100644
--- a/Documentation/mhi/index.rst
+++ b/Documentation/mhi/index.rst
@@ -9,6 +9,7 @@ MHI

    mhi
    topology
+   uci

 .. only::  subproject and html

diff --git a/Documentation/mhi/uci.rst b/Documentation/mhi/uci.rst
new file mode 100644
index 000..9603f92
--- /dev/null
+++ b/Documentation/mhi/uci.rst
@@ -0,0 +1,94 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=
+Userspace Client Interface (UCI)
+=
+
+UCI driver enables userspace clients to communicate to external MHI 
devices
+like modem and WLAN. UCI driver probe creates standard character 
device file
+nodes for userspace clients to perform open, read, write, poll and 
release file

+operations. UCI device object represents UCI device file node which gets
+instantiated as part of MHI UCI driver probe. UCI channel object 
represents

+MHI uplink or downlink channel.
+
+Operations
+==
+
+open
+
+
+Instantiates UCI channel object and starts MHI channels to move it to 
running
+state. Inbound buffers are queued to downlink channel transfer ring. 
Every
+subsequent open() increments UCI device reference count as well as 
UCI channel

+reference count.
+
+read
+
+
+When data transfer is completed on downlink channel, transfer ring 
element
+buffer is copied to pending list. Reader is unblocked and data is 
copied to
+userspace buffer. Transfer ring element buffer is queued back to 
downlink

+channel transfer ring.
+
+write
+-
+
+Write buffer is queued to uplink channel transfer ring if ring is not
full. Upon
+uplink transfer completion buffer is freed.
+
+poll
+
+
+Returns EPOLLIN | EPOLLRDNORM mask if pending list has buffers to be 
read by
+userspace. Returns EPOLLOUT | EPOLLWRNORM mask if MHI uplink channel 
transfer

+ring is not empty. Returns EPOLLERR when UCI driver is removed.


ring is not empty.  When the uplink channel transfer ring is non-empty, 
more
data may be sent to the device. Returns EPOLLERR when UCI driver is 
removed.

Done



+
+release
+---
+
+Decrements UCI device reference count and UCI channel reference count 
upon last
+release(). UCI channel clean up is performed. MHI channel moves to 
disable

+state and inbound buffers are freed.


Decrements UCI device reference count and UCI channel reference count. 
Upon last

release() UCI channel clean up is performed. MHI channel moves to disable
state and inbound buffers are freed.

Done.



+
+Usage
+=
+
+Device file node is created with format:-
+
+/dev/
+
+mhi_device_name includes mhi controller name and the name of the MHI 
channel

+being used by MHI client in userspace to send or receive data using MHI
+protocol.
+
+There is a separate character device file node created for each channel
+specified in MHI device id table. MHI channels are statically defined 
by MHI
+specification. The list of supported channels is in the channel list 
variable

+of mhi_device_id table in UCI driver.
+
+Qualcomm MSM Interface(QMI) Channel
+---
+
+Qualcomm MSM Interface(QMI) is a modem control messaging protocol 
used to
+communicate between software components in the modem and other 
peripheral
+subsystems. QMI communication is of request/response type or an 
unsolicited
+event type. libqmi is userspace MHI client which communicates to a 
QMI service
+using UCI device. It sends a QMI request to a QMI service using MHI 
channel 14
+or 16. QMI response is received using MHI channel 15 or 17 
respectively. libqmi
+is a glib-based library for talking to WWAN modems and devices which 
speaks QMI

+protocol. For more information about libqmi please refer
+https://www.freedesktop.org/wiki/Software/libqmi/
+
+Usage Example
+~
+
+QMI command to retrieve device mode
+$ sudo qmicli -d /dev/mhi0_QMI --dms-get-model
+[/dev/mhi0_QMI] Device model retrieved:
+    Model: 'FN980m'
+
+Other Use Cases
+---
+
+Getting MHI device specific diagnostics information to userspace MHI 
diagnostic

+client using DIAG channel 4 (Host to device) and 5 (Device to Host).


Thanks,
Hemant
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v3 7/7] bus: mhi: Improve documentation on channel transfer setup APIs

2020-12-02 Thread Hemant Kumar




On 12/2/20 3:40 PM, Bhaumik Bhatt wrote:

The mhi_prepare_for_transfer() and mhi_unprepare_from_transfer()
APIs could use better explanation, especially with the addition
of two new APIs to start and stop the transfers on channels. Add
better set of information for those APIs.

Signed-off-by: Bhaumik Bhatt 
---

Reviewed-by: Hemant Kumar 

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v3 5/7] bus: mhi: core: Check execution environment for channel before issuing reset

2020-12-02 Thread Hemant Kumar




On 12/2/20 3:40 PM, Bhaumik Bhatt wrote:

A client can attempt to unprepare certain channels for transfer even
after the execution environment they are supposed to run in has changed.
In the event that happens, the device need not be notified of the reset
and the host can proceed with clean up for the channel context and
memory allocated for it on the host.

Signed-off-by: Bhaumik Bhatt 


Reviewed-by: Hemant Kumar 
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v3 3/7] bus: mhi: core: Improvements to the channel handling state machine

2020-12-02 Thread Hemant Kumar




On 12/2/20 3:40 PM, Bhaumik Bhatt wrote:

Add support to enable sending the stop channel command and
improve the channel handling state machine such that all commands
go through a common function. This can help ensure that the state
machine is not violated in any way and adheres to the MHI
specification.

Signed-off-by: Bhaumik Bhatt 
---

Reviewed-by: Hemant Kumar 
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v3 4/7] bus: mhi: core: Add support to stop or start channel data transfers

2020-12-02 Thread Hemant Kumar

Hi Bhaumik,

On 12/2/20 3:40 PM, Bhaumik Bhatt wrote:

Some MHI client drivers may want to request a pause or halt of
data transfer activity on their channels. Support for this does
not exist and must be introduced, wherein the channel context is
not reset or cleared but only the STOP channel command is issued.
This would need to be paired with an API that allows resuming the
data transfer activity on channels by use of the START channel
command. This API assumes that the context information is already


is it a better option to make sure channel context is setup as this is 
an exported API. Hence check for channel context bail out in case 
channel context is not setup with an err msg ?



setup. Enable this using two new APIs, mhi_start_transfer() and
mhi_stop_transfer().

Signed-off-by: Bhaumik Bhatt 
---
  drivers/bus/mhi/core/main.c | 41 +
  include/linux/mhi.h | 19 +++
  2 files changed, 60 insertions(+)

diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index 4cc5ced..2e4b34a 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -1552,6 +1552,47 @@ void mhi_unprepare_from_transfer(struct mhi_device 
*mhi_dev)
  }
  EXPORT_SYMBOL_GPL(mhi_unprepare_from_transfer);
  
+static int mhi_update_transfer_state(struct mhi_device *mhi_dev,

+enum mhi_ch_state_type to_state)
+{
+   struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
+   struct mhi_chan *mhi_chan;
+   int dir, ret;
+
+   for (dir = 0; dir < 2; dir++) {
+   mhi_chan = dir ? mhi_dev->ul_chan : mhi_dev->dl_chan;
+
+   if (!mhi_chan)
+   continue;
+
+   /*
+* Bail out if one of the channels fail as client will reset
+* both upon failure
+*/
+   mutex_lock(_chan->mutex);
+   ret = mhi_update_channel_state(mhi_cntrl, mhi_chan, to_state);
+   if (ret) {
+   mutex_unlock(_chan->mutex);
+   return ret;
+   }
+   mutex_unlock(_chan->mutex);
+   }
+
+   return 0;
+}
+
+int mhi_stop_transfer(struct mhi_device *mhi_dev)
+{
+   return mhi_update_transfer_state(mhi_dev, MHI_CH_STATE_TYPE_STOP);
+}
+EXPORT_SYMBOL_GPL(mhi_stop_transfer);
+
+int mhi_start_transfer(struct mhi_device *mhi_dev)
+{
+   return mhi_update_transfer_state(mhi_dev, MHI_CH_STATE_TYPE_START);
+}
+EXPORT_SYMBOL_GPL(mhi_start_transfer);
+
  int mhi_poll(struct mhi_device *mhi_dev, u32 budget)
  {
struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index aa9757e..35779a0 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -704,6 +704,25 @@ int mhi_prepare_for_transfer(struct mhi_device *mhi_dev);
  void mhi_unprepare_from_transfer(struct mhi_device *mhi_dev);
  
  /**

+ * mhi_stop_transfer - Pauses ongoing channel activity by issuing the STOP
+ * channel command to both UL and DL channels. This command
+ * does not reset the channel context and the client 
drivers
+ * can issue mhi_start_transfer to resume activity.
+ * @mhi_dev: Device associated with the channels
+ */
+int mhi_stop_transfer(struct mhi_device *mhi_dev);
+
+/**
+ * mhi_start_transfer - Resumes channel activity by issuing the START channel
+ *  command to both UL and DL channels. This command 
assumes
+ *  the channel context is already setup and the client
+ *  drivers can issue mhi_stop_transfer to pause activity 
if
+ *  required.
+ * @mhi_dev: Device associated with the channels
+ */
+int mhi_start_transfer(struct mhi_device *mhi_dev);
+
+/**
   * mhi_poll - Poll for any available data in DL direction
   * @mhi_dev: Device associated with the channels
   * @budget: # of events to process


Overall change looks good.

Thanks,
Hemant
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v3 2/7] bus: mhi: core: Allow channel to be disabled from stopped state

2020-12-02 Thread Hemant Kumar




On 12/2/20 3:40 PM, Bhaumik Bhatt wrote:

If a channel was explicitly stopped but not reset, allow it to
move to a disabled state so the channel context can be cleaned
up. As the channel remained in a stopped state, its context was
not reset and cleared, which needs to be done if a client driver
module is unloaded or a device crash occurs.

Signed-off-by: Bhaumik Bhatt 
---


Reviewed-by: Hemant Kumar 
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v3 1/7] bus: mhi: core: Allow receiving a STOP channel command response

2020-12-02 Thread Hemant Kumar

Hi Bhaumik,

On 12/2/20 3:40 PM, Bhaumik Bhatt wrote:

Add support to receive the response to a STOP channel command to the

How about saying "Add support to send the STOP channel command ?

MHI bus. If a client would like to STOP a channel instead of issuing
a RESET to it, this would provide support for it.

Signed-off-by: Bhaumik Bhatt 
---
  drivers/bus/mhi/core/main.c | 5 +
  1 file changed, 5 insertions(+)

diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index 702c31b..a7bb8a7 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -1193,6 +1193,11 @@ int mhi_send_cmd(struct mhi_controller *mhi_cntrl,
cmd_tre->dword[0] = MHI_TRE_CMD_RESET_DWORD0;
cmd_tre->dword[1] = MHI_TRE_CMD_RESET_DWORD1(chan);
break;
+   case MHI_CMD_STOP_CHAN:
+   cmd_tre->ptr = MHI_TRE_CMD_STOP_PTR;
+   cmd_tre->dword[0] = MHI_TRE_CMD_STOP_DWORD0;
+   cmd_tre->dword[1] = MHI_TRE_CMD_STOP_DWORD1(chan);
+   break;
case MHI_CMD_START_CHAN:
cmd_tre->ptr = MHI_TRE_CMD_START_PTR;
cmd_tre->dword[0] = MHI_TRE_CMD_START_DWORD0;



Thanks,
Hemant
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v3 6/7] bus: mhi: core: Remove __ prefix for MHI channel unprepare function

2020-12-02 Thread Hemant Kumar




On 12/2/20 3:40 PM, Bhaumik Bhatt wrote:

The __mhi_unprepare_channel() API does not require the __ prefix.
Get rid of it and make the internal function consistent with the
other function names.

Signed-off-by: Bhaumik Bhatt 
Reviewed-by: Manivannan Sadhasivam 

Reviewed-by: Hemant Kumar 

The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


[PATCH v14 4/4] bus: mhi: Add userspace client interface driver

2020-12-01 Thread Hemant Kumar
This MHI client driver allows userspace clients to transfer
raw data between MHI device and host using standard file operations.
Driver instantiates UCI device object which is associated to device
file node. UCI device object instantiates UCI channel object when device
file node is opened. UCI channel object is used to manage MHI channels
by calling MHI core APIs for read and write operations. MHI channels
are started as part of device open(). MHI channels remain in start
state until last release() is called on UCI device file node. Device
file node is created with format

/dev/

Currently it supports QMI channel.

Signed-off-by: Hemant Kumar 
Reviewed-by: Manivannan Sadhasivam 
Tested-by: Loic Poulain 
---
 drivers/bus/mhi/Kconfig  |  13 +
 drivers/bus/mhi/Makefile |   3 +
 drivers/bus/mhi/uci.c| 664 +++
 3 files changed, 680 insertions(+)
 create mode 100644 drivers/bus/mhi/uci.c

diff --git a/drivers/bus/mhi/Kconfig b/drivers/bus/mhi/Kconfig
index da5cd0c..5194e8e 100644
--- a/drivers/bus/mhi/Kconfig
+++ b/drivers/bus/mhi/Kconfig
@@ -29,3 +29,16 @@ config MHI_BUS_PCI_GENERIC
  This driver provides MHI PCI controller driver for devices such as
  Qualcomm SDX55 based PCIe modems.
 
+config MHI_UCI
+   tristate "MHI UCI"
+   depends on MHI_BUS
+   help
+ MHI based Userspace Client Interface (UCI) driver is used for
+ transferring raw data between host and device using standard file
+ operations from userspace. Open, read, write, poll and close
+ operations are supported by this driver. Please check
+ mhi_uci_match_table for all supported channels that are exposed to
+ userspace.
+
+ To compile this driver as a module, choose M here: the module will be
+ called mhi_uci.
diff --git a/drivers/bus/mhi/Makefile b/drivers/bus/mhi/Makefile
index 0a2d778..69f2111 100644
--- a/drivers/bus/mhi/Makefile
+++ b/drivers/bus/mhi/Makefile
@@ -4,3 +4,6 @@ obj-y += core/
 obj-$(CONFIG_MHI_BUS_PCI_GENERIC) += mhi_pci_generic.o
 mhi_pci_generic-y += pci_generic.o
 
+# MHI client
+mhi_uci-y := uci.o
+obj-$(CONFIG_MHI_UCI) += mhi_uci.o
diff --git a/drivers/bus/mhi/uci.c b/drivers/bus/mhi/uci.c
new file mode 100644
index 000..1df2377
--- /dev/null
+++ b/drivers/bus/mhi/uci.c
@@ -0,0 +1,664 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2018-2020, The Linux Foundation. All rights reserved.*/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define MHI_UCI_DRIVER_NAME "mhi_uci"
+#define MHI_MAX_UCI_MINORS 128
+
+static DEFINE_IDR(uci_idr);
+static DEFINE_MUTEX(uci_drv_mutex);
+static struct class *uci_dev_class;
+static int uci_dev_major;
+
+/**
+ * struct uci_chan - MHI channel for a UCI device
+ * @udev: associated UCI device object
+ * @ul_wq: wait queue for writer
+ * @write_lock: mutex write lock for ul channel
+ * @dl_wq: wait queue for reader
+ * @read_lock: mutex read lock for dl channel
+ * @dl_pending_lock: spin lock for dl_pending list
+ * @dl_pending: list of dl buffers userspace is waiting to read
+ * @cur_buf: current buffer userspace is reading
+ * @dl_size: size of the current dl buffer userspace is reading
+ * @ref_count: uci_chan reference count
+ */
+struct uci_chan {
+   struct uci_dev *udev;
+   wait_queue_head_t ul_wq;
+
+   /* ul channel lock to synchronize multiple writes */
+   struct mutex write_lock;
+
+   wait_queue_head_t dl_wq;
+
+   /* dl channel lock to synchronize multiple reads */
+   struct mutex read_lock;
+
+   /*
+* protects pending list in bh context, channel release, read and
+* poll
+*/
+   spinlock_t dl_pending_lock;
+
+   struct list_head dl_pending;
+   struct uci_buf *cur_buf;
+   size_t dl_size;
+   struct kref ref_count;
+};
+
+/**
+ * struct uci_buf - UCI buffer
+ * @data: data buffer
+ * @len: length of data buffer
+ * @node: list node of the UCI buffer
+ */
+struct uci_buf {
+   void *data;
+   size_t len;
+   struct list_head node;
+};
+
+/**
+ * struct uci_dev - MHI UCI device
+ * @minor: UCI device node minor number
+ * @mhi_dev: associated mhi device object
+ * @uchan: UCI uplink and downlink channel object
+ * @mtu: max TRE buffer length
+ * @enabled: Flag to track the state of the UCI device
+ * @lock: mutex lock to manage uchan object
+ * @ref_count: uci_dev reference count
+ */
+struct uci_dev {
+   unsigned int minor;
+   struct mhi_device *mhi_dev;
+   struct uci_chan *uchan;
+   size_t mtu;
+   bool enabled;
+
+   /* synchronize open, release and driver remove */
+   struct mutex lock;
+   struct kref ref_count;
+};
+
+static void mhi_uci_dev_chan_release(struct kref *ref)
+{
+   struct uci_buf *buf_itr, *tmp;
+   struct uci_chan *uchan =
+   container_of(ref, struct uci_chan, ref_count);
+
+   if (

[PATCH v14 3/4] docs: Add documentation for userspace client interface

2020-12-01 Thread Hemant Kumar
MHI userspace client driver is creating device file node
for user application to perform file operations. File
operations are handled by MHI core driver. Currently
QMI MHI channel is supported by this driver.

Signed-off-by: Hemant Kumar 
---
 Documentation/mhi/index.rst |  1 +
 Documentation/mhi/uci.rst   | 94 +
 2 files changed, 95 insertions(+)
 create mode 100644 Documentation/mhi/uci.rst

diff --git a/Documentation/mhi/index.rst b/Documentation/mhi/index.rst
index 1d8dec3..c75a371 100644
--- a/Documentation/mhi/index.rst
+++ b/Documentation/mhi/index.rst
@@ -9,6 +9,7 @@ MHI
 
mhi
topology
+   uci
 
 .. only::  subproject and html
 
diff --git a/Documentation/mhi/uci.rst b/Documentation/mhi/uci.rst
new file mode 100644
index 000..9603f92
--- /dev/null
+++ b/Documentation/mhi/uci.rst
@@ -0,0 +1,94 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=
+Userspace Client Interface (UCI)
+=
+
+UCI driver enables userspace clients to communicate to external MHI devices
+like modem and WLAN. UCI driver probe creates standard character device file
+nodes for userspace clients to perform open, read, write, poll and release file
+operations. UCI device object represents UCI device file node which gets
+instantiated as part of MHI UCI driver probe. UCI channel object represents
+MHI uplink or downlink channel.
+
+Operations
+==
+
+open
+
+
+Instantiates UCI channel object and starts MHI channels to move it to running
+state. Inbound buffers are queued to downlink channel transfer ring. Every
+subsequent open() increments UCI device reference count as well as UCI channel
+reference count.
+
+read
+
+
+When data transfer is completed on downlink channel, transfer ring element
+buffer is copied to pending list. Reader is unblocked and data is copied to
+userspace buffer. Transfer ring element buffer is queued back to downlink
+channel transfer ring.
+
+write
+-
+
+Write buffer is queued to uplink channel transfer ring if ring is not full. 
Upon
+uplink transfer completion buffer is freed.
+
+poll
+
+
+Returns EPOLLIN | EPOLLRDNORM mask if pending list has buffers to be read by
+userspace. Returns EPOLLOUT | EPOLLWRNORM mask if MHI uplink channel transfer
+ring is not empty. Returns EPOLLERR when UCI driver is removed.
+
+release
+---
+
+Decrements UCI device reference count and UCI channel reference count upon last
+release(). UCI channel clean up is performed. MHI channel moves to disable
+state and inbound buffers are freed.
+
+Usage
+=
+
+Device file node is created with format:-
+
+/dev/
+
+mhi_device_name includes mhi controller name and the name of the MHI channel
+being used by MHI client in userspace to send or receive data using MHI
+protocol.
+
+There is a separate character device file node created for each channel
+specified in MHI device id table. MHI channels are statically defined by MHI
+specification. The list of supported channels is in the channel list variable
+of mhi_device_id table in UCI driver.
+
+Qualcomm MSM Interface(QMI) Channel
+---
+
+Qualcomm MSM Interface(QMI) is a modem control messaging protocol used to
+communicate between software components in the modem and other peripheral
+subsystems. QMI communication is of request/response type or an unsolicited
+event type. libqmi is userspace MHI client which communicates to a QMI service
+using UCI device. It sends a QMI request to a QMI service using MHI channel 14
+or 16. QMI response is received using MHI channel 15 or 17 respectively. libqmi
+is a glib-based library for talking to WWAN modems and devices which speaks QMI
+protocol. For more information about libqmi please refer
+https://www.freedesktop.org/wiki/Software/libqmi/
+
+Usage Example
+~
+
+QMI command to retrieve device mode
+$ sudo qmicli -d /dev/mhi0_QMI --dms-get-model
+[/dev/mhi0_QMI] Device model retrieved:
+Model: 'FN980m'
+
+Other Use Cases
+---
+
+Getting MHI device specific diagnostics information to userspace MHI diagnostic
+client using DIAG channel 4 (Host to device) and 5 (Device to Host).
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v14 0/4] userspace MHI client interface driver

2020-12-01 Thread Hemant Kumar
This patch series adds support for UCI driver. UCI driver enables userspace
clients to communicate to external MHI devices like modem and WLAN. UCI driver
probe creates standard character device file nodes for userspace clients to
perform open, read, write, poll and release file operations. These file
operations call MHI core layer APIs to perform data transfer using MHI bus
to communicate with MHI device. Patch is tested using arm64 and x86 based
platform.

V14:
- Fixed device file node format to /dev/ instead of
  /dev/mhi_ because "mhi" is already part of mhi device name.
  For example old format: /dev/mhi_mhi0_QMI new format: /dev/mhi0_QMI.
- Updated MHI documentation to reflect index mhi controller name in
  QMI usage example.

V13:
- Removed LOOPBACK channel from mhi_device_id table from this patch series.
  Pushing a new patch series to add support for LOOPBACK channel and the user
  space test application. Also removed the description from kernel 
documentation.
- Added QMI channel to mhi_device_id table. QMI channel has existing libqmi
  support from user space.
- Updated kernel Documentation for QMI channel and provided external reference
  for libqmi.
- Updated device file node name by appending mhi device name only, which already
  includes mhi controller device name.

V12:
- Added loopback test driver under selftest/drivers/mhi. Updated kernel
  documentation for the usage of the loopback test application.
- Addressed review comments for renaming variable names, updated inline
  comments and removed two redundant dev_dbg.

V11:
- Fixed review comments for UCI documentation by expanding TLAs and rewording
  some sentences.

V10:
- Replaced mutex_lock with mutex_lock_interruptible in read() and write() file
  ops call back.

V9:
- Renamed dl_lock to dl_pending _lock and pending list to dl_pending for
  clarity.
- Used read lock to protect cur_buf.
- Change transfer status check logic and only consider 0 and -EOVERFLOW as
  only success.
- Added __int to module init function.
- Print channel name instead of minor number upon successful probe.

V8:
- Fixed kernel test robot compilation error by changing %lu to %zu for
  size_t.
- Replaced uci with UCI in Kconfig, commit text, and comments in driver
  code.
- Fixed minor style related comments.

V7:
- Decoupled uci device and uci channel objects. uci device is
  associated with device file node. uci channel is associated
  with MHI channels. uci device refers to uci channel to perform
  MHI channel operations for device file operations like read()
  and write(). uci device increments its reference count for
  every open(). uci device calls mhi_uci_dev_start_chan() to start
  the MHI channel. uci channel object is tracking number of times
  MHI channel is referred. This allows to keep the MHI channel in
  start state until last release() is called. After that uci channel
  reference count goes to 0 and uci channel clean up is performed
  which stops the MHI channel. After the last call to release() if
  driver is removed uci reference count becomes 0 and uci object is
  cleaned up.
- Use separate uci channel read and write lock to fine grain locking
  between reader and writer.
- Use uci device lock to synchronize open, release and driver remove.
- Optimize for downlink only or uplink only UCI device.

V6:
- Moved uci.c to mhi directory.
- Updated Kconfig to add module information.
- Updated Makefile to rename uci object file name as mhi_uci
- Removed kref for open count

V5:
- Removed mhi_uci_drv structure.
- Used idr instead of creating global list of uci devices.
- Used kref instead of local ref counting for uci device and
  open count.
- Removed unlikely macro.

V4:
- Fix locking to protect proper struct members.
- Updated documentation describing uci client driver use cases.
- Fixed uci ref counting in mhi_uci_open for error case.
- Addressed style related review comments.

V3: Added documentation for MHI UCI driver.

V2:
- Added mutex lock to prevent multiple readers to access same
- mhi buffer which can result into use after free.

Hemant Kumar (4):
  bus: mhi: core: Add helper API to return number of free TREs
  bus: mhi: core: Move MHI_MAX_MTU to external header file
  docs: Add documentation for userspace client interface
  bus: mhi: Add userspace client interface driver

 Documentation/mhi/index.rst |   1 +
 Documentation/mhi/uci.rst   |  94 ++
 drivers/bus/mhi/Kconfig |  13 +
 drivers/bus/mhi/Makefile|   3 +
 drivers/bus/mhi/core/internal.h |   1 -
 drivers/bus/mhi/core/main.c |  12 +
 drivers/bus/mhi/uci.c   | 664 
 include/linux/mhi.h |  12 +
 8 files changed, 799 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/mhi/uci.rst
 create mode 100644 drivers/bus/mhi/uci.c

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v14 1/4] bus: mhi: core: Add helper API to return number of free TREs

2020-12-01 Thread Hemant Kumar
Introduce mhi_get_free_desc_count() API to return number
of TREs available to queue buffer. MHI clients can use this
API to know before hand if ring is full without calling queue
API.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
Reviewed-by: Manivannan Sadhasivam 
---
 drivers/bus/mhi/core/main.c | 12 
 include/linux/mhi.h |  9 +
 2 files changed, 21 insertions(+)

diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index 702c31b..74a25e7 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -260,6 +260,18 @@ int mhi_destroy_device(struct device *dev, void *data)
return 0;
 }
 
+int mhi_get_free_desc_count(struct mhi_device *mhi_dev,
+   enum dma_data_direction dir)
+{
+   struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
+   struct mhi_chan *mhi_chan = (dir == DMA_TO_DEVICE) ?
+   mhi_dev->ul_chan : mhi_dev->dl_chan;
+   struct mhi_ring *tre_ring = _chan->tre_ring;
+
+   return get_nr_avail_ring_elements(mhi_cntrl, tre_ring);
+}
+EXPORT_SYMBOL_GPL(mhi_get_free_desc_count);
+
 void mhi_notify(struct mhi_device *mhi_dev, enum mhi_callback cb_reason)
 {
struct mhi_driver *mhi_drv;
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index aa9757e..e36d575 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -599,6 +599,15 @@ void mhi_set_mhi_state(struct mhi_controller *mhi_cntrl,
 void mhi_notify(struct mhi_device *mhi_dev, enum mhi_callback cb_reason);
 
 /**
+ * mhi_get_free_desc_count - Get transfer ring length
+ * Get # of TD available to queue buffers
+ * @mhi_dev: Device associated with the channels
+ * @dir: Direction of the channel
+ */
+int mhi_get_free_desc_count(struct mhi_device *mhi_dev,
+   enum dma_data_direction dir);
+
+/**
  * mhi_prepare_for_power_up - Do pre-initialization before power up.
  *This is optional, call this before power up if
  *the controller does not want bus framework to
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v14 2/4] bus: mhi: core: Move MHI_MAX_MTU to external header file

2020-12-01 Thread Hemant Kumar
Currently this macro is defined in internal MHI header as
a TRE length mask. Moving it to external header allows MHI
client drivers to set this upper bound for the transmit
buffer size.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
Reviewed-by: Manivannan Sadhasivam 
---
 drivers/bus/mhi/core/internal.h | 1 -
 include/linux/mhi.h | 3 +++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
index 6f80ec3..2b9c063 100644
--- a/drivers/bus/mhi/core/internal.h
+++ b/drivers/bus/mhi/core/internal.h
@@ -453,7 +453,6 @@ enum mhi_pm_state {
 #define CMD_EL_PER_RING128
 #define PRIMARY_CMD_RING   0
 #define MHI_DEV_WAKE_DB127
-#define MHI_MAX_MTU0x
 #define MHI_RANDOM_U32_NONZERO(bmsk)   (prandom_u32_max(bmsk) + 1)
 
 enum mhi_er_type {
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index e36d575..f072605 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -15,6 +15,9 @@
 #include 
 #include 
 
+/* MHI client drivers to set this upper bound for tx buffer */
+#define MHI_MAX_MTU 0x
+
 #define MHI_MAX_OEM_PK_HASH_SEGMENTS 16
 
 struct mhi_chan;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



Re: [PATCH v13 4/4] bus: mhi: Add userspace client interface driver

2020-12-01 Thread Hemant Kumar

Hi Loic,

On 12/1/20 10:04 AM, Jeffrey Hugo wrote:

On 12/1/2020 11:05 AM, Loic Poulain wrote:

On Tue, 1 Dec 2020 at 18:51, Jeffrey Hugo  wrote:


On 12/1/2020 10:52 AM, Loic Poulain wrote:

On Tue, 1 Dec 2020 at 18:37, Jeffrey Hugo  wrote:


On 12/1/2020 10:36 AM, Loic Poulain wrote:
On Tue, 1 Dec 2020 at 02:16, Hemant Kumar  
wrote:


Hi Loic,

On 11/30/20 10:22 AM, Loic Poulain wrote:
On Sat, 28 Nov 2020 at 04:26, Hemant Kumar 
 wrote:


This MHI client driver allows userspace clients to transfer
raw data between MHI device and host using standard file 
operations.
Driver instantiates UCI device object which is associated to 
device
file node. UCI device object instantiates UCI channel object 
when device
file node is opened. UCI channel object is used to manage MHI 
channels
by calling MHI core APIs for read and write operations. MHI 
channels

are started as part of device open(). MHI channels remain in start
state until last release() is called on UCI device file node. 
Device

file node is created with format


[...]


+struct uci_chan {
+   struct uci_dev *udev;
+   wait_queue_head_t ul_wq;
+
+   /* ul channel lock to synchronize multiple writes */
+   struct mutex write_lock;
+
+   wait_queue_head_t dl_wq;
+
+   /* dl channel lock to synchronize multiple reads */
+   struct mutex read_lock;
+
+   /*
+    * protects pending list in bh context, channel 
release, read and

+    * poll
+    */
+   spinlock_t dl_pending_lock;
+
+   struct list_head dl_pending;
+   struct uci_buf *cur_buf;
+   size_t dl_size;
+   struct kref ref_count;
+};


[...]


+ * struct uci_dev - MHI UCI device
+ * @minor: UCI device node minor number
+ * @mhi_dev: associated mhi device object
+ * @uchan: UCI uplink and downlink channel object
+ * @mtu: max TRE buffer length
+ * @enabled: Flag to track the state of the UCI device
+ * @lock: mutex lock to manage uchan object
+ * @ref_count: uci_dev reference count
+ */
+struct uci_dev {
+   unsigned int minor;
+   struct mhi_device *mhi_dev;
+   struct uci_chan *uchan;


Why a pointer to uci_chan and not just plainly integrating the
structure here, AFAIU uci_chan describes the channels and is just a
subpart of uci_dev. That would reduce the number of dynamic
allocations you manage and the extra kref. do you even need a 
separate

structure for this?


This goes back to one of my patch versions i tried to address 
concern

from Greg. Since we need to ref count the channel as well as the uci
device i decoupled the two objects and used two reference counts 
for two

different objects.


What Greg complained about is the two kref in the same structure and
that you were using kref as an open() counter. But splitting your
struct in two in order to keep the two kref does not make the much
code better (and simpler). I'm still a bit puzzled about the driver
complexity, it's supposed to be just a passthrough interface to MHI
after all.

I would suggest several changes, that IMHO would simplify reviewing:
- Use only one structure representing the 'uci' context (uci_dev)
- Keep the read path simple (mhi_uci_read), do no use an intermediate
cur_buf pointer, only dequeue the buffer when it is fully consumed.
- As I commented before, take care of the dl_pending list access
concurrency, even in wait_event.
- You don't need to count the number of open() calls, AFAIK,
mhi_prepare_for_transfer() simply fails if channels are already
started...


Unless I missed something, you seem to have ignored the root issue 
that

Hemant needs to solve, which is when to call
mhi_unprepare_for_transfer().  You can't just call that when 
close() is

called because there might be multiple users, and each one is going to
trigger a close(), so you need to know how many close() instances to
expect, and only call mhi_unprepare_for_transfer() for the last one.


That one part of his problem, yes, but if you unconditionally call
mhi_prepare_for_transfer in open(), it should fail for subsequent
users, and so only one user will successfully open the device.


I'm pretty sure that falls under "trying to prevent users from opening a
device" which Greg gave a pretty strong NACK to.  So, that's not a 
solution.


That would deserve clarification since other drivers like
virtio_console clearly prevent that (guest_connected).


Quoting Greg from the source - https://lkml.org/lkml/2020/9/17/873

"
I told you before, do not try to keep a device node from being opened
multiple times, as it will always fail (think about passing file handles
around between programs...)

If userspace wants to do this, it will do it.  If your driver can't
handle that, that's fine, userspace will learn not to do that.  But the
kernel can not prevent this from happening.
"





So, the complete problem is -

N users need to be able to use the device (and by proxy, the channel)
concurrently, but prepare needs to be called on the first user coming
into the pi

Re: [PATCH v13 4/4] bus: mhi: Add userspace client interface driver

2020-11-30 Thread Hemant Kumar

Hi Loic,

On 11/30/20 10:22 AM, Loic Poulain wrote:

On Sat, 28 Nov 2020 at 04:26, Hemant Kumar  wrote:


This MHI client driver allows userspace clients to transfer
raw data between MHI device and host using standard file operations.
Driver instantiates UCI device object which is associated to device
file node. UCI device object instantiates UCI channel object when device
file node is opened. UCI channel object is used to manage MHI channels
by calling MHI core APIs for read and write operations. MHI channels
are started as part of device open(). MHI channels remain in start
state until last release() is called on UCI device file node. Device
file node is created with format


[...]


+struct uci_chan {
+   struct uci_dev *udev;
+   wait_queue_head_t ul_wq;
+
+   /* ul channel lock to synchronize multiple writes */
+   struct mutex write_lock;
+
+   wait_queue_head_t dl_wq;
+
+   /* dl channel lock to synchronize multiple reads */
+   struct mutex read_lock;
+
+   /*
+* protects pending list in bh context, channel release, read and
+* poll
+*/
+   spinlock_t dl_pending_lock;
+
+   struct list_head dl_pending;
+   struct uci_buf *cur_buf;
+   size_t dl_size;
+   struct kref ref_count;
+};


[...]


+ * struct uci_dev - MHI UCI device
+ * @minor: UCI device node minor number
+ * @mhi_dev: associated mhi device object
+ * @uchan: UCI uplink and downlink channel object
+ * @mtu: max TRE buffer length
+ * @enabled: Flag to track the state of the UCI device
+ * @lock: mutex lock to manage uchan object
+ * @ref_count: uci_dev reference count
+ */
+struct uci_dev {
+   unsigned int minor;
+   struct mhi_device *mhi_dev;
+   struct uci_chan *uchan;


Why a pointer to uci_chan and not just plainly integrating the
structure here, AFAIU uci_chan describes the channels and is just a
subpart of uci_dev. That would reduce the number of dynamic
allocations you manage and the extra kref. do you even need a separate
structure for this?


This goes back to one of my patch versions i tried to address concern 
from Greg. Since we need to ref count the channel as well as the uci 
device i decoupled the two objects and used two reference counts for two 
different objects.


Copying from V7 patch history

V7:
- Decoupled uci device and uci channel objects. uci device is
  associated with device file node. uci channel is associated
  with MHI channels. uci device refers to uci channel to perform
  MHI channel operations for device file operations like read()
  and write(). uci device increments its reference count for
  every open(). uci device calls mhi_uci_dev_start_chan() to start
  the MHI channel. uci channel object is tracking number of times
  MHI channel is referred. This allows to keep the MHI channel in
  start state until last release() is called. After that uci channel
  reference count goes to 0 and uci channel clean up is performed
  which stops the MHI channel. After the last call to release() if
  driver is removed uci reference count becomes 0 and uci object is
  cleaned up.



[...]


+static int mhi_uci_dev_start_chan(struct uci_dev *udev)
+{
+   int ret = 0;
+   struct uci_chan *uchan;
+
+   mutex_lock(>lock);
+   if (!udev->uchan || !kref_get_unless_zero(>uchan->ref_count)) {


This test is suspicious,  kref_get_unless_zero should be enough to test, right?
kref_get_unless_zero is de-referencing uchan->ref_count for the first 
open uchan is set to NULL, for that NULL check is added for uchan.


if (kref_get_unless_zero(>ref))
 goto skip_init;


+   uchan = kzalloc(sizeof(*uchan), GFP_KERNEL);
+   if (!uchan) {
+   ret = -ENOMEM;
+   goto error_chan_start;
+   }
+
+   udev->uchan = uchan;
+   uchan->udev = udev;
+   init_waitqueue_head(>ul_wq);
+   init_waitqueue_head(>dl_wq);
+   mutex_init(>write_lock);
+   mutex_init(>read_lock);
+   spin_lock_init(>dl_pending_lock);
+   INIT_LIST_HEAD(>dl_pending);
+
+   ret = mhi_prepare_for_transfer(udev->mhi_dev);
+   if (ret) {
+   dev_err(>mhi_dev->dev, "Error starting transfer 
channels\n");
+   goto error_chan_cleanup;
+   }
+
+   ret = mhi_queue_inbound(udev);
+   if (ret)
+   goto error_chan_cleanup;
+
+   kref_init(>ref_count);
+   }
+
+   mutex_unlock(>lock);
+
+   return 0;
+
+error_chan_cleanup:
+   mhi_uci_dev_chan_release(>ref_count);
+error_chan_start:
+   mutex_unlock(>lock);
+   return ret;
+}


Regards,
Loic



Thanks,
Hemant
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v13 4/4] bus: mhi: Add userspace client interface driver

2020-11-30 Thread Hemant Kumar

Hi Mani,

On 11/27/20 10:11 PM, Manivannan Sadhasivam wrote:

Hi Hemant,

On Fri, Nov 27, 2020 at 07:26:06PM -0800, Hemant Kumar wrote:

This MHI client driver allows userspace clients to transfer
raw data between MHI device and host using standard file operations.
Driver instantiates UCI device object which is associated to device
file node. UCI device object instantiates UCI channel object when device
file node is opened. UCI channel object is used to manage MHI channels
by calling MHI core APIs for read and write operations. MHI channels
are started as part of device open(). MHI channels remain in start
state until last release() is called on UCI device file node. Device
file node is created with format

/dev/mhi_

Currently it supports QMI channel.



Thanks for the update. This patch looks good to me. But as I'm going to
apply Loic's "bus: mhi: core: Indexed MHI controller name" patch, you
need to update the documentation accordingly.


This is what i added in documentation on v13


Device file node is created with format:-

/dev/mhi_

mhi_device_name includes mhi controller name and the name of the MHI
channel being used by MHI client in userspace to send or receive data
using MHI protocol.

​
Loic's patch is going to update the controller name as indexed 
controller name, which goes fine with or without his change going first.


For example:   With Loic's change name of device node would be 
/dev/mhi_mhi0_QMI


Without Loic's change it would be

/dev/mhi_:00:01.2_QMI

Please let me know if i am missing something.

Thanks,
Hemant




Signed-off-by: Hemant Kumar 


Reviewed-by: Manivannan Sadhasivam 

Thanks,
Mani


[..]
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


[PATCH v13 2/4] bus: mhi: core: Move MHI_MAX_MTU to external header file

2020-11-28 Thread Hemant Kumar
Currently this macro is defined in internal MHI header as
a TRE length mask. Moving it to external header allows MHI
client drivers to set this upper bound for the transmit
buffer size.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
Reviewed-by: Manivannan Sadhasivam 
---
 drivers/bus/mhi/core/internal.h | 1 -
 include/linux/mhi.h | 3 +++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
index 6f80ec3..2b9c063 100644
--- a/drivers/bus/mhi/core/internal.h
+++ b/drivers/bus/mhi/core/internal.h
@@ -453,7 +453,6 @@ enum mhi_pm_state {
 #define CMD_EL_PER_RING128
 #define PRIMARY_CMD_RING   0
 #define MHI_DEV_WAKE_DB127
-#define MHI_MAX_MTU0x
 #define MHI_RANDOM_U32_NONZERO(bmsk)   (prandom_u32_max(bmsk) + 1)
 
 enum mhi_er_type {
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index b3bc966..fc903b2 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -15,6 +15,9 @@
 #include 
 #include 
 
+/* MHI client drivers to set this upper bound for tx buffer */
+#define MHI_MAX_MTU 0x
+
 #define MHI_MAX_OEM_PK_HASH_SEGMENTS 16
 
 struct mhi_chan;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v13 1/4] bus: mhi: core: Add helper API to return number of free TREs

2020-11-28 Thread Hemant Kumar
Introduce mhi_get_free_desc_count() API to return number
of TREs available to queue buffer. MHI clients can use this
API to know before hand if ring is full without calling queue
API.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
Reviewed-by: Manivannan Sadhasivam 
---
 drivers/bus/mhi/core/main.c | 12 
 include/linux/mhi.h |  9 +
 2 files changed, 21 insertions(+)

diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index 4eb93d8..defd579a 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -260,6 +260,18 @@ int mhi_destroy_device(struct device *dev, void *data)
return 0;
 }
 
+int mhi_get_free_desc_count(struct mhi_device *mhi_dev,
+   enum dma_data_direction dir)
+{
+   struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
+   struct mhi_chan *mhi_chan = (dir == DMA_TO_DEVICE) ?
+   mhi_dev->ul_chan : mhi_dev->dl_chan;
+   struct mhi_ring *tre_ring = _chan->tre_ring;
+
+   return get_nr_avail_ring_elements(mhi_cntrl, tre_ring);
+}
+EXPORT_SYMBOL_GPL(mhi_get_free_desc_count);
+
 void mhi_notify(struct mhi_device *mhi_dev, enum mhi_callback cb_reason)
 {
struct mhi_driver *mhi_drv;
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index d31efcf..b3bc966 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -597,6 +597,15 @@ void mhi_set_mhi_state(struct mhi_controller *mhi_cntrl,
 void mhi_notify(struct mhi_device *mhi_dev, enum mhi_callback cb_reason);
 
 /**
+ * mhi_get_free_desc_count - Get transfer ring length
+ * Get # of TD available to queue buffers
+ * @mhi_dev: Device associated with the channels
+ * @dir: Direction of the channel
+ */
+int mhi_get_free_desc_count(struct mhi_device *mhi_dev,
+   enum dma_data_direction dir);
+
+/**
  * mhi_prepare_for_power_up - Do pre-initialization before power up.
  *This is optional, call this before power up if
  *the controller does not want bus framework to
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v13 0/4] userspace MHI client interface driver

2020-11-27 Thread Hemant Kumar
This patch series adds support for UCI driver. UCI driver enables userspace
clients to communicate to external MHI devices like modem and WLAN. UCI driver
probe creates standard character device file nodes for userspace clients to
perform open, read, write, poll and release file operations. These file
operations call MHI core layer APIs to perform data transfer using MHI bus
to communicate with MHI device. Patch is tested using arm64 based platform.

V13:
- Removed LOOPBACK channel from mhi_device_id table from this patch series.
Pushing a new patch series to add support for LOOPBACK channel and the user
space test application. Also removed the description from kernel documentation.
- Added QMI channel to mhi_device_id table. QMI channel has existing libqmi
support from user space.
- Updated kernel Documentation for QMI channel and provided external reference
for libqmi.
- Updated device file node name by appending mhi device name only, which already
includes mhi controller device name.

V12:
- Added loopback test driver under selftest/drivers/mhi. Updated kernel
  documentation for the usage of the loopback test application.
- Addressed review comments for renaming variable names, updated inline
  comments and removed two redundant dev_dbg.

V11:
- Fixed review comments for UCI documentation by expanding TLAs and rewording
  some sentences.

V10:
- Replaced mutex_lock with mutex_lock_interruptible in read() and write() file
  ops call back.

V9:
- Renamed dl_lock to dl_pending _lock and pending list to dl_pending for
  clarity.
- Used read lock to protect cur_buf.
- Change transfer status check logic and only consider 0 and -EOVERFLOW as
  only success.
- Added __int to module init function.
- Print channel name instead of minor number upon successful probe.

V8:
- Fixed kernel test robot compilation error by changing %lu to %zu for
  size_t.
- Replaced uci with UCI in Kconfig, commit text, and comments in driver
  code.
- Fixed minor style related comments.

V7:
- Decoupled uci device and uci channel objects. uci device is
  associated with device file node. uci channel is associated
  with MHI channels. uci device refers to uci channel to perform
  MHI channel operations for device file operations like read()
  and write(). uci device increments its reference count for
  every open(). uci device calls mhi_uci_dev_start_chan() to start
  the MHI channel. uci channel object is tracking number of times
  MHI channel is referred. This allows to keep the MHI channel in
  start state until last release() is called. After that uci channel
  reference count goes to 0 and uci channel clean up is performed
  which stops the MHI channel. After the last call to release() if
  driver is removed uci reference count becomes 0 and uci object is
  cleaned up.
- Use separate uci channel read and write lock to fine grain locking
  between reader and writer.
- Use uci device lock to synchronize open, release and driver remove.
- Optimize for downlink only or uplink only UCI device.

V6:
- Moved uci.c to mhi directory.
- Updated Kconfig to add module information.
- Updated Makefile to rename uci object file name as mhi_uci
- Removed kref for open count

V5:
- Removed mhi_uci_drv structure.
- Used idr instead of creating global list of uci devices.
- Used kref instead of local ref counting for uci device and
  open count.
- Removed unlikely macro.

V4:
- Fix locking to protect proper struct members.
- Updated documentation describing uci client driver use cases.
- Fixed uci ref counting in mhi_uci_open for error case.
- Addressed style related review comments.

V3: Added documentation for MHI UCI driver.

V2:
- Added mutex lock to prevent multiple readers to access same
- mhi buffer which can result into use after free.

Hemant Kumar (4):
  bus: mhi: core: Add helper API to return number of free TREs
  bus: mhi: core: Move MHI_MAX_MTU to external header file
  docs: Add documentation for userspace client interface
  bus: mhi: Add userspace client interface driver

 Documentation/mhi/index.rst |   1 +
 Documentation/mhi/uci.rst   |  94 ++
 drivers/bus/mhi/Kconfig |  13 +
 drivers/bus/mhi/Makefile|   3 +
 drivers/bus/mhi/core/internal.h |   1 -
 drivers/bus/mhi/core/main.c |  12 +
 drivers/bus/mhi/uci.c   | 665 
 include/linux/mhi.h |  12 +
 8 files changed, 800 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/mhi/uci.rst
 create mode 100644 drivers/bus/mhi/uci.c

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v13 4/4] bus: mhi: Add userspace client interface driver

2020-11-27 Thread Hemant Kumar
This MHI client driver allows userspace clients to transfer
raw data between MHI device and host using standard file operations.
Driver instantiates UCI device object which is associated to device
file node. UCI device object instantiates UCI channel object when device
file node is opened. UCI channel object is used to manage MHI channels
by calling MHI core APIs for read and write operations. MHI channels
are started as part of device open(). MHI channels remain in start
state until last release() is called on UCI device file node. Device
file node is created with format

/dev/mhi_

Currently it supports QMI channel.

Signed-off-by: Hemant Kumar 
---
 drivers/bus/mhi/Kconfig  |  13 +
 drivers/bus/mhi/Makefile |   3 +
 drivers/bus/mhi/uci.c| 665 +++
 3 files changed, 681 insertions(+)
 create mode 100644 drivers/bus/mhi/uci.c

diff --git a/drivers/bus/mhi/Kconfig b/drivers/bus/mhi/Kconfig
index da5cd0c..5194e8e 100644
--- a/drivers/bus/mhi/Kconfig
+++ b/drivers/bus/mhi/Kconfig
@@ -29,3 +29,16 @@ config MHI_BUS_PCI_GENERIC
  This driver provides MHI PCI controller driver for devices such as
  Qualcomm SDX55 based PCIe modems.
 
+config MHI_UCI
+   tristate "MHI UCI"
+   depends on MHI_BUS
+   help
+ MHI based Userspace Client Interface (UCI) driver is used for
+ transferring raw data between host and device using standard file
+ operations from userspace. Open, read, write, poll and close
+ operations are supported by this driver. Please check
+ mhi_uci_match_table for all supported channels that are exposed to
+ userspace.
+
+ To compile this driver as a module, choose M here: the module will be
+ called mhi_uci.
diff --git a/drivers/bus/mhi/Makefile b/drivers/bus/mhi/Makefile
index 0a2d778..69f2111 100644
--- a/drivers/bus/mhi/Makefile
+++ b/drivers/bus/mhi/Makefile
@@ -4,3 +4,6 @@ obj-y += core/
 obj-$(CONFIG_MHI_BUS_PCI_GENERIC) += mhi_pci_generic.o
 mhi_pci_generic-y += pci_generic.o
 
+# MHI client
+mhi_uci-y := uci.o
+obj-$(CONFIG_MHI_UCI) += mhi_uci.o
diff --git a/drivers/bus/mhi/uci.c b/drivers/bus/mhi/uci.c
new file mode 100644
index 000..fb9c183
--- /dev/null
+++ b/drivers/bus/mhi/uci.c
@@ -0,0 +1,665 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2018-2020, The Linux Foundation. All rights reserved.*/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define MHI_DEVICE_NAME "mhi"
+#define MHI_UCI_DRIVER_NAME "mhi_uci"
+#define MHI_MAX_UCI_MINORS 128
+
+static DEFINE_IDR(uci_idr);
+static DEFINE_MUTEX(uci_drv_mutex);
+static struct class *uci_dev_class;
+static int uci_dev_major;
+
+/**
+ * struct uci_chan - MHI channel for a UCI device
+ * @udev: associated UCI device object
+ * @ul_wq: wait queue for writer
+ * @write_lock: mutex write lock for ul channel
+ * @dl_wq: wait queue for reader
+ * @read_lock: mutex read lock for dl channel
+ * @dl_pending_lock: spin lock for dl_pending list
+ * @dl_pending: list of dl buffers userspace is waiting to read
+ * @cur_buf: current buffer userspace is reading
+ * @dl_size: size of the current dl buffer userspace is reading
+ * @ref_count: uci_chan reference count
+ */
+struct uci_chan {
+   struct uci_dev *udev;
+   wait_queue_head_t ul_wq;
+
+   /* ul channel lock to synchronize multiple writes */
+   struct mutex write_lock;
+
+   wait_queue_head_t dl_wq;
+
+   /* dl channel lock to synchronize multiple reads */
+   struct mutex read_lock;
+
+   /*
+* protects pending list in bh context, channel release, read and
+* poll
+*/
+   spinlock_t dl_pending_lock;
+
+   struct list_head dl_pending;
+   struct uci_buf *cur_buf;
+   size_t dl_size;
+   struct kref ref_count;
+};
+
+/**
+ * struct uci_buf - UCI buffer
+ * @data: data buffer
+ * @len: length of data buffer
+ * @node: list node of the UCI buffer
+ */
+struct uci_buf {
+   void *data;
+   size_t len;
+   struct list_head node;
+};
+
+/**
+ * struct uci_dev - MHI UCI device
+ * @minor: UCI device node minor number
+ * @mhi_dev: associated mhi device object
+ * @uchan: UCI uplink and downlink channel object
+ * @mtu: max TRE buffer length
+ * @enabled: Flag to track the state of the UCI device
+ * @lock: mutex lock to manage uchan object
+ * @ref_count: uci_dev reference count
+ */
+struct uci_dev {
+   unsigned int minor;
+   struct mhi_device *mhi_dev;
+   struct uci_chan *uchan;
+   size_t mtu;
+   bool enabled;
+
+   /* synchronize open, release and driver remove */
+   struct mutex lock;
+   struct kref ref_count;
+};
+
+static void mhi_uci_dev_chan_release(struct kref *ref)
+{
+   struct uci_buf *buf_itr, *tmp;
+   struct uci_chan *uchan =
+   container_of(ref, struct uci_chan, ref_count);
+
+   if (uchan->udev->enabled)
+   mhi_unprepar

[PATCH v13 3/4] docs: Add documentation for userspace client interface

2020-11-27 Thread Hemant Kumar
MHI userspace client driver is creating device file node
for user application to perform file operations. File
operations are handled by MHI core driver. Currently
QMI MHI channel is supported by this driver.

Signed-off-by: Hemant Kumar 
---
 Documentation/mhi/index.rst |  1 +
 Documentation/mhi/uci.rst   | 94 +
 2 files changed, 95 insertions(+)
 create mode 100644 Documentation/mhi/uci.rst

diff --git a/Documentation/mhi/index.rst b/Documentation/mhi/index.rst
index 1d8dec3..c75a371 100644
--- a/Documentation/mhi/index.rst
+++ b/Documentation/mhi/index.rst
@@ -9,6 +9,7 @@ MHI
 
mhi
topology
+   uci
 
 .. only::  subproject and html
 
diff --git a/Documentation/mhi/uci.rst b/Documentation/mhi/uci.rst
new file mode 100644
index 000..ad22650
--- /dev/null
+++ b/Documentation/mhi/uci.rst
@@ -0,0 +1,94 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=
+Userspace Client Interface (UCI)
+=
+
+UCI driver enables userspace clients to communicate to external MHI devices
+like modem and WLAN. UCI driver probe creates standard character device file
+nodes for userspace clients to perform open, read, write, poll and release file
+operations. UCI device object represents UCI device file node which gets
+instantiated as part of MHI UCI driver probe. UCI channel object represents
+MHI uplink or downlink channel.
+
+Operations
+==
+
+open
+
+
+Instantiates UCI channel object and starts MHI channels to move it to running
+state. Inbound buffers are queued to downlink channel transfer ring. Every
+subsequent open() increments UCI device reference count as well as UCI channel
+reference count.
+
+read
+
+
+When data transfer is completed on downlink channel, transfer ring element
+buffer is copied to pending list. Reader is unblocked and data is copied to
+userspace buffer. Transfer ring element buffer is queued back to downlink
+channel transfer ring.
+
+write
+-
+
+Write buffer is queued to uplink channel transfer ring if ring is not full. 
Upon
+uplink transfer completion buffer is freed.
+
+poll
+
+
+Returns EPOLLIN | EPOLLRDNORM mask if pending list has buffers to be read by
+userspace. Returns EPOLLOUT | EPOLLWRNORM mask if MHI uplink channel transfer
+ring is not empty. Returns EPOLLERR when UCI driver is removed.
+
+release
+---
+
+Decrements UCI device reference count and UCI channel reference count upon last
+release(). UCI channel clean up is performed. MHI channel moves to disable
+state and inbound buffers are freed.
+
+Usage
+=
+
+Device file node is created with format:-
+
+/dev/mhi_
+
+mhi_device_name includes mhi controller name and the name of the MHI channel
+being used by MHI client in userspace to send or receive data using MHI
+protocol.
+
+There is a separate character device file node created for each channel
+specified in MHI device id table. MHI channels are statically defined by MHI
+specification. The list of supported channels is in the channel list variable
+of mhi_device_id table in UCI driver.
+
+Qualcomm MSM Interface(QMI) Channel
+---
+
+Qualcomm MSM Interface(QMI) is a modem control messaging protocol used to
+communicate between software components in the modem and other peripheral
+subsystems. QMI communication is of request/response type or an unsolicited
+event type. libqmi is userspace MHI client which communicates to a QMI service
+using UCI device. It sends a QMI request to a QMI service using MHI channel 14
+or 16. QMI response is received using MHI channel 15 or 17 respectively. libqmi
+is a glib-based library for talking to WWAN modems and devices which speaks QMI
+protocol. For more information about libqmi please refer
+https://www.freedesktop.org/wiki/Software/libqmi/
+
+Usage Example
+~
+
+QMI command to retrieve device mode
+$ sudo qmicli -d /dev/mhi_\:02\:00.0_QMI --dms-get-model
+[/dev/mhi_:02:00.0_QMI] Device model retrieved:
+Model: 'FN980m'
+
+Other Use Cases
+---
+
+Getting MHI device specific diagnostics information to userspace MHI diagnostic
+client using DIAG channel 4 (Host to device) and 5 (Device to Host).
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



Re: [PATCH v12 5/5] selftest: mhi: Add support to test MHI LOOPBACK channel

2020-11-20 Thread Hemant Kumar

Hi Shuah,

On 11/20/20 7:26 AM, Shuah Khan wrote:

On 11/16/20 3:46 PM, Hemant Kumar wrote:

Loopback test opens the MHI device file node and writes
a data buffer to it. MHI UCI kernel space driver copies
the data and sends it to MHI uplink (Tx) LOOPBACK channel.
MHI device loops back the same data to MHI downlink (Rx)
LOOPBACK channel. This data is read by test application
and compared against the data sent. Test passes if data
buffer matches between Tx and Rx. Test application performs
open(), poll(), write(), read() and close() file operations.

Signed-off-by: Hemant Kumar 
---
  Documentation/mhi/uci.rst  |  32 +
  tools/testing/selftests/Makefile   |   1 +
  tools/testing/selftests/drivers/.gitignore |   1 +
  tools/testing/selftests/drivers/mhi/Makefile   |   8 +
  tools/testing/selftests/drivers/mhi/config |   2 +
  .../testing/selftests/drivers/mhi/loopback_test.c  | 802 
+

  6 files changed, 846 insertions(+)
  create mode 100644 tools/testing/selftests/drivers/mhi/Makefile
  create mode 100644 tools/testing/selftests/drivers/mhi/config
  create mode 100644 tools/testing/selftests/drivers/mhi/loopback_test.c

diff --git a/Documentation/mhi/uci.rst b/Documentation/mhi/uci.rst
index ce8740e..0a04afe 100644
--- a/Documentation/mhi/uci.rst
+++ b/Documentation/mhi/uci.rst
@@ -79,6 +79,38 @@ MHI client driver performs read operation, same 
data gets looped back to MHI
  host using LOOPBACK channel 1. LOOPBACK channel is used to verify 
data path

  and data integrity between MHI Host and MHI device.


Nice.

[..]

+
+enum debug_level {
+    DBG_LVL_VERBOSE,
+    DBG_LVL_INFO,
+    DBG_LVL_ERROR,
+};
+
+enum test_status {
+    TEST_STATUS_SUCCESS,
+    TEST_STATUS_ERROR,
+    TEST_STATUS_NO_DEV,
+};
+


Since you are running this test as part of kselftest, please use
ksft errors nd returns.

Are you suggesting to use following macros instead of test_status enum ?
#define KSFT_PASS  0
#define KSFT_FAIL  1




+struct lb_test_ctx {
+    char dev_node[256];
+    unsigned char *tx_buff;
+    unsigned char *rx_buff;
+    unsigned int rx_pkt_count;
+    unsigned int tx_pkt_count;
+    int iterations;
+    bool iter_complete;
+    bool comp_complete;
+    bool test_complete;
+    bool all_complete;
+    unsigned long buff_size;
+    long byte_recvd;
+    long byte_sent;
+};
+
+bool force_exit;
+char write_data = 'a';
+int completed_iterations;
+
+struct lb_test_ctx test_ctxt;
+enum debug_level msg_lvl;
+struct pollfd read_fd;
+struct pollfd write_fd;
+enum test_status mhi_test_return_value;
+enum test_status tx_status;
+enum test_status rx_status;
+enum test_status cmp_rxtx_status;
+
+#define test_log(test_msg_lvl, format, ...) do { \
+    if (test_msg_lvl >= msg_lvl) \
+    fprintf(stderr, format, ##__VA_ARGS__); \
+} while (0)
+
+static void loopback_test_sleep_ms(int ms)
+{
+    usleep(1000 * ms);
+}
+


Have you run this as part of "make kselftest" run. How does this
sleep work in that env.?
Looks like kselftest runs this test application by directly executing 
the binary, but this test application requires a valid mhi device file 
node as a required parameter. So considering that requirement, is this 
test application qualifies to run using kselftest ? Without a valid 
device file node test would fail. Is there an option to run this test as 
standalone test which can only be run when a mhi device file node is 
present ? Having said that i tested this driver by

directly executing it using the test binary which is compiled using
make loopback_test under mhi dir.


Are there any cases where this test can't run and have to - those
cases need to be skips.
Yes, as this test application can not run by itself it needs a valid mhi 
device file node to write and test reads the same device node to get the 
data back.
So test can not be run without having a MHI device connected over a 
transport (in my testing MHI device is connected over PCIe). Could you 
please suggest an option to use this test application as a standalone 
test instead of being part of kselftest?


thanks,
-- Shuah


Thanks,
Hemant
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v12 5/5] selftest: mhi: Add support to test MHI LOOPBACK channel

2020-11-20 Thread Hemant Kumar



Hi Mani,
On 11/19/20 10:10 PM, Manivannan Sadhasivam wrote:

On Mon, Nov 16, 2020 at 02:46:22PM -0800, Hemant Kumar wrote:

Loopback test opens the MHI device file node and writes
a data buffer to it. MHI UCI kernel space driver copies
the data and sends it to MHI uplink (Tx) LOOPBACK channel.
MHI device loops back the same data to MHI downlink (Rx)
LOOPBACK channel. This data is read by test application
and compared against the data sent. Test passes if data
buffer matches between Tx and Rx. Test application performs
open(), poll(), write(), read() and close() file operations.

Signed-off-by: Hemant Kumar 


One nitpick below, with that addressed:

Reviewed-by: Manivannan Sadhasivam 

[..]


Effectively this functions does parse and run, so this should be called
as, "loopback_test_parse_run" or pthread creation should be moved here.

Done.

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


[PATCH v12 2/5] bus: mhi: core: Move MHI_MAX_MTU to external header file

2020-11-16 Thread Hemant Kumar
Currently this macro is defined in internal MHI header as
a TRE length mask. Moving it to external header allows MHI
client drivers to set this upper bound for the transmit
buffer size.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
Reviewed-by: Manivannan Sadhasivam 
---
 drivers/bus/mhi/core/internal.h | 1 -
 include/linux/mhi.h | 3 +++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
index 2df8de5..7d1f8bc 100644
--- a/drivers/bus/mhi/core/internal.h
+++ b/drivers/bus/mhi/core/internal.h
@@ -453,7 +453,6 @@ enum mhi_pm_state {
 #define CMD_EL_PER_RING128
 #define PRIMARY_CMD_RING   0
 #define MHI_DEV_WAKE_DB127
-#define MHI_MAX_MTU0x
 #define MHI_RANDOM_U32_NONZERO(bmsk)   (prandom_u32_max(bmsk) + 1)
 
 enum mhi_er_type {
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index a4d0f48..53bb41d 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -15,6 +15,9 @@
 #include 
 #include 
 
+/* MHI client drivers to set this upper bound for tx buffer */
+#define MHI_MAX_MTU 0x
+
 #define MHI_MAX_OEM_PK_HASH_SEGMENTS 16
 
 struct mhi_chan;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v12 5/5] selftest: mhi: Add support to test MHI LOOPBACK channel

2020-11-16 Thread Hemant Kumar
Loopback test opens the MHI device file node and writes
a data buffer to it. MHI UCI kernel space driver copies
the data and sends it to MHI uplink (Tx) LOOPBACK channel.
MHI device loops back the same data to MHI downlink (Rx)
LOOPBACK channel. This data is read by test application
and compared against the data sent. Test passes if data
buffer matches between Tx and Rx. Test application performs
open(), poll(), write(), read() and close() file operations.

Signed-off-by: Hemant Kumar 
---
 Documentation/mhi/uci.rst  |  32 +
 tools/testing/selftests/Makefile   |   1 +
 tools/testing/selftests/drivers/.gitignore |   1 +
 tools/testing/selftests/drivers/mhi/Makefile   |   8 +
 tools/testing/selftests/drivers/mhi/config |   2 +
 .../testing/selftests/drivers/mhi/loopback_test.c  | 802 +
 6 files changed, 846 insertions(+)
 create mode 100644 tools/testing/selftests/drivers/mhi/Makefile
 create mode 100644 tools/testing/selftests/drivers/mhi/config
 create mode 100644 tools/testing/selftests/drivers/mhi/loopback_test.c

diff --git a/Documentation/mhi/uci.rst b/Documentation/mhi/uci.rst
index ce8740e..0a04afe 100644
--- a/Documentation/mhi/uci.rst
+++ b/Documentation/mhi/uci.rst
@@ -79,6 +79,38 @@ MHI client driver performs read operation, same data gets 
looped back to MHI
 host using LOOPBACK channel 1. LOOPBACK channel is used to verify data path
 and data integrity between MHI Host and MHI device.
 
+Loopback Test
+~
+
+Loopback test application is used to verify data integrity between MHI host and
+MHI device over LOOPBACK channel. This also confirms that basic MHI data path
+is working properly. Test performs write() to send tx buffer to MHI device file
+node for LOOPBACK uplink channel. MHI LOOPBACK downlink channel loops back
+transmit data to MHI Host. Test application receives data in receive buffer as
+part of read(). It verifies if tx buffer matches rx buffer. Test application
+performs poll() before making write() and read() system calls. Test passes if
+match is found.
+
+Test is present under tools/testing/selftests/drivers/mhi. It is compiled using
+following command in same dir:-
+
+make loopback_test
+
+Test is run using following command arguments:-
+
+loopback_test -c  -b  -l  -i
+
+
+Required argument:
+-c : loopback chardev node
+
+Optional argument:
+-b : transmit buffer size. If not present 1024 bytes size transmit buffer
+ is sent.
+-i : Number of iterations to perform, If not present only one transmit buffer
+ is sent.
+-l : Log level. If not present defaults to DBG_LVL_INFO.
+
 Other Use Cases
 ---
 
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index d9c2835..084bc1e 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -10,6 +10,7 @@ TARGETS += core
 TARGETS += cpufreq
 TARGETS += cpu-hotplug
 TARGETS += drivers/dma-buf
+TARGETS += drivers/mhi
 TARGETS += efivarfs
 TARGETS += exec
 TARGETS += filesystems
diff --git a/tools/testing/selftests/drivers/.gitignore 
b/tools/testing/selftests/drivers/.gitignore
index ca74f2e..e4806d5 100644
--- a/tools/testing/selftests/drivers/.gitignore
+++ b/tools/testing/selftests/drivers/.gitignore
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
 /dma-buf/udmabuf
+/mhi/loopback_test
diff --git a/tools/testing/selftests/drivers/mhi/Makefile 
b/tools/testing/selftests/drivers/mhi/Makefile
new file mode 100644
index 000..c06c925
--- /dev/null
+++ b/tools/testing/selftests/drivers/mhi/Makefile
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0-only
+CFLAGS += -I../../../../../usr/include/ -g -Wall
+
+LDLIBS = -lpthread
+TEST_GEN_PROGS := loopback_test
+
+include ../../lib.mk
+
diff --git a/tools/testing/selftests/drivers/mhi/config 
b/tools/testing/selftests/drivers/mhi/config
new file mode 100644
index 000..471dc92
--- /dev/null
+++ b/tools/testing/selftests/drivers/mhi/config
@@ -0,0 +1,2 @@
+CONFIG_MHI_BUS=y
+CONFIG_MHi_UCI=y
diff --git a/tools/testing/selftests/drivers/mhi/loopback_test.c 
b/tools/testing/selftests/drivers/mhi/loopback_test.c
new file mode 100644
index 000..99b7712
--- /dev/null
+++ b/tools/testing/selftests/drivers/mhi/loopback_test.c
@@ -0,0 +1,802 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Loopback test application performs write() to send tx buffer to MHI device
+ * file node for LOOPBACK uplink channel. MHI LOOPBACK downlink channel loops
+ * back transmit data to MHI Host. Test application receives data in receive
+ * buffer as part of read(). It verifies if tx buffer matches rx buffer. Test
+ * application performs poll() before making write() and read() system
+ * calls. Test passes if match is found.
+ *
+ * Test is compiled using following command:-
+ *
+ * make loopback_test
+ *
+ * Test is run using following command arguments:-
+ *
+ * loopback_test -c  -b  -l  -i
+ * 
+ *
+ * Required argument:
+ * -c : loopback

[PATCH v12 3/5] docs: Add documentation for userspace client interface

2020-11-16 Thread Hemant Kumar
MHI userspace client driver is creating device file node
for user application to perform file operations. File
operations are handled by MHI core driver. Currently
Loopback MHI channel is supported by this driver.

Signed-off-by: Hemant Kumar 
---
 Documentation/mhi/index.rst |  1 +
 Documentation/mhi/uci.rst   | 86 +
 2 files changed, 87 insertions(+)
 create mode 100644 Documentation/mhi/uci.rst

diff --git a/Documentation/mhi/index.rst b/Documentation/mhi/index.rst
index 1d8dec3..c75a371 100644
--- a/Documentation/mhi/index.rst
+++ b/Documentation/mhi/index.rst
@@ -9,6 +9,7 @@ MHI
 
mhi
topology
+   uci
 
 .. only::  subproject and html
 
diff --git a/Documentation/mhi/uci.rst b/Documentation/mhi/uci.rst
new file mode 100644
index 000..ce8740e
--- /dev/null
+++ b/Documentation/mhi/uci.rst
@@ -0,0 +1,86 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=
+Userspace Client Interface (UCI)
+=
+
+UCI driver enables userspace clients to communicate to external MHI devices
+like modem and WLAN. UCI driver probe creates standard character device file
+nodes for userspace clients to perform open, read, write, poll and release file
+operations. UCI device object represents UCI device file node which gets
+instantiated as part of MHI UCI driver probe. UCI channel object represents
+MHI uplink or downlink channel.
+
+Operations
+==
+
+open
+
+
+Instantiates UCI channel object and starts MHI channels to move it to running
+state. Inbound buffers are queued to downlink channel transfer ring. Every
+subsequent open() increments UCI device reference count as well as UCI channel
+reference count.
+
+read
+
+
+When data transfer is completed on downlink channel, transfer ring element
+buffer is copied to pending list. Reader is unblocked and data is copied to
+userspace buffer. Transfer ring element buffer is queued back to downlink
+channel transfer ring.
+
+write
+-
+
+Write buffer is queued to uplink channel transfer ring if ring is not full. 
Upon
+uplink transfer completion buffer is freed.
+
+poll
+
+
+Returns EPOLLIN | EPOLLRDNORM mask if pending list has buffers to be read by
+userspace. Returns EPOLLOUT | EPOLLWRNORM mask if MHI uplink channel transfer
+ring is not empty. Returns EPOLLERR when UCI driver is removed.
+
+release
+---
+
+Decrements UCI device reference count and UCI channel reference count upon last
+release(). UCI channel clean up is performed. MHI channel moves to disable
+state and inbound buffers are freed.
+
+Usage
+=
+
+Device file node is created with format:-
+
+/dev/mhi__
+
+controller_name is the name of underlying bus used to transfer data. mhi_device
+name is the name of the MHI channel being used by MHI client in userspace to
+send or receive data using MHI protocol.
+
+There is a separate character device file node created for each channel
+specified in MHI device id table. MHI channels are statically defined by MHI
+specification. The list of supported channels is in the channel list variable
+of mhi_device_id table in UCI driver.
+
+LOOPBACK Channel
+
+
+Userspace MHI client using LOOPBACK channel opens device file node. As part of
+open operation transfer ring elements are queued to transfer ring of LOOPBACK
+channel 1 and channel doorbell is rung. When userspace MHI client performs 
write
+operation on device node, data buffer gets queued as part of transfer ring
+element to transfer ring of LOOPBACK channel 0. MHI Core driver rings the
+channel doorbell for MHI device to move data over underlying bus. When 
userspace
+MHI client driver performs read operation, same data gets looped back to MHI
+host using LOOPBACK channel 1. LOOPBACK channel is used to verify data path
+and data integrity between MHI Host and MHI device.
+
+Other Use Cases
+---
+
+Getting MHI device specific diagnostics information to userspace MHI diagnostic
+client using DIAG channel 4 (Host to device) and 5 (Device to Host).
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v12 1/5] bus: mhi: core: Add helper API to return number of free TREs

2020-11-16 Thread Hemant Kumar
Introduce mhi_get_free_desc_count() API to return number
of TREs available to queue buffer. MHI clients can use this
API to know before hand if ring is full without calling queue
API.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
Reviewed-by: Manivannan Sadhasivam 
---
 drivers/bus/mhi/core/main.c | 12 
 include/linux/mhi.h |  9 +
 2 files changed, 21 insertions(+)

diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index f953e2a..6158720 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -260,6 +260,18 @@ int mhi_destroy_device(struct device *dev, void *data)
return 0;
 }
 
+int mhi_get_free_desc_count(struct mhi_device *mhi_dev,
+   enum dma_data_direction dir)
+{
+   struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
+   struct mhi_chan *mhi_chan = (dir == DMA_TO_DEVICE) ?
+   mhi_dev->ul_chan : mhi_dev->dl_chan;
+   struct mhi_ring *tre_ring = _chan->tre_ring;
+
+   return get_nr_avail_ring_elements(mhi_cntrl, tre_ring);
+}
+EXPORT_SYMBOL_GPL(mhi_get_free_desc_count);
+
 void mhi_notify(struct mhi_device *mhi_dev, enum mhi_callback cb_reason)
 {
struct mhi_driver *mhi_drv;
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index 52b3c60..a4d0f48 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -597,6 +597,15 @@ void mhi_set_mhi_state(struct mhi_controller *mhi_cntrl,
 void mhi_notify(struct mhi_device *mhi_dev, enum mhi_callback cb_reason);
 
 /**
+ * mhi_get_free_desc_count - Get transfer ring length
+ * Get # of TD available to queue buffers
+ * @mhi_dev: Device associated with the channels
+ * @dir: Direction of the channel
+ */
+int mhi_get_free_desc_count(struct mhi_device *mhi_dev,
+   enum dma_data_direction dir);
+
+/**
  * mhi_prepare_for_power_up - Do pre-initialization before power up.
  *This is optional, call this before power up if
  *the controller does not want bus framework to
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v12 0/5] userspace MHI client interface driver

2020-11-16 Thread Hemant Kumar
This patch series adds support for UCI driver. UCI driver enables userspace
clients to communicate to external MHI devices like modem and WLAN. UCI driver
probe creates standard character device file nodes for userspace clients to
perform open, read, write, poll and release file operations. These file
operations call MHI core layer APIs to perform data transfer using MHI bus
to communicate with MHI device. It also adds a loopback test application to
verify the UCI LOOPBACK channel. Patch is tested using arm64 based platform.

V12:
- Added loopback test driver under selftest/drivers/mhi. Updated kernel
  documentation for the usage of the loopback test application.
- Addressed review comments for renaming variable names, updated inline
  comments and removed two redundant dev_dbg.

V11:
- Fixed review comments for UCI documentation by expanding TLAs and rewording
  some sentences.

V10:
- Replaced mutex_lock with mutex_lock_interruptible in read() and write() file
  ops call back.

V9:
- Renamed dl_lock to dl_pending _lock and pending list to dl_pending for
  clarity.
- Used read lock to protect cur_buf.
- Change transfer status check logic and only consider 0 and -EOVERFLOW as
  only success.
- Added __int to module init function.
- Print channel name instead of minor number upon successful probe.

V8:
- Fixed kernel test robot compilation error by changing %lu to %zu for
  size_t.
- Replaced uci with UCI in Kconfig, commit text, and comments in driver
  code.
- Fixed minor style related comments.

V7:
- Decoupled uci device and uci channel objects. uci device is
  associated with device file node. uci channel is associated
  with MHI channels. uci device refers to uci channel to perform
  MHI channel operations for device file operations like read()
  and write(). uci device increments its reference count for
  every open(). uci device calls mhi_uci_dev_start_chan() to start
  the MHI channel. uci channel object is tracking number of times
  MHI channel is referred. This allows to keep the MHI channel in
  start state until last release() is called. After that uci channel
  reference count goes to 0 and uci channel clean up is performed
  which stops the MHI channel. After the last call to release() if
  driver is removed uci reference count becomes 0 and uci object is
  cleaned up.
- Use separate uci channel read and write lock to fine grain locking
  between reader and writer.
- Use uci device lock to synchronize open, release and driver remove.
- Optimize for downlink only or uplink only UCI device.

V6:
- Moved uci.c to mhi directory.
- Updated Kconfig to add module information.
- Updated Makefile to rename uci object file name as mhi_uci
- Removed kref for open count

V5:
- Removed mhi_uci_drv structure.
- Used idr instead of creating global list of uci devices.
- Used kref instead of local ref counting for uci device and
  open count.
- Removed unlikely macro.

V4:
- Fix locking to protect proper struct members.
- Updated documentation describing uci client driver use cases.
- Fixed uci ref counting in mhi_uci_open for error case.
- Addressed style related review comments.

V3: Added documentation for MHI UCI driver.

V2:
- Added mutex lock to prevent multiple readers to access same
- mhi buffer which can result into use after free.

Hemant Kumar (5):
  bus: mhi: core: Add helper API to return number of free TREs
  bus: mhi: core: Move MHI_MAX_MTU to external header file
  docs: Add documentation for userspace client interface
  bus: mhi: Add userspace client interface driver
  selftest: mhi: Add support to test MHI LOOPBACK channel

 Documentation/mhi/index.rst|   1 +
 Documentation/mhi/uci.rst  | 109 +++
 drivers/bus/mhi/Kconfig|  13 +
 drivers/bus/mhi/Makefile   |   3 +
 drivers/bus/mhi/core/internal.h|   1 -
 drivers/bus/mhi/core/main.c|  12 +
 drivers/bus/mhi/uci.c  | 667 +
 include/linux/mhi.h|  12 +
 tools/testing/selftests/Makefile   |   1 +
 tools/testing/selftests/drivers/.gitignore |   1 +
 tools/testing/selftests/drivers/mhi/Makefile   |   8 +
 tools/testing/selftests/drivers/mhi/config |   2 +
 .../testing/selftests/drivers/mhi/loopback_test.c  | 789 +
 13 files changed, 1618 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/mhi/uci.rst
 create mode 100644 drivers/bus/mhi/uci.c
 create mode 100644 tools/testing/selftests/drivers/mhi/Makefile
 create mode 100644 tools/testing/selftests/drivers/mhi/config
 create mode 100644 tools/testing/selftests/drivers/mhi/loopback_test.c

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v12 4/5] bus: mhi: Add userspace client interface driver

2020-11-16 Thread Hemant Kumar
This MHI client driver allows userspace clients to transfer
raw data between MHI device and host using standard file operations.
Driver instantiates UCI device object which is associated to device
file node. UCI device object instantiates UCI channel object when device
file node is opened. UCI channel object is used to manage MHI channels
by calling MHI core APIs for read and write operations. MHI channels
are started as part of device open(). MHI channels remain in start
state until last release() is called on UCI device file node. Device
file node is created with format

/dev/mhi__

Currently it supports LOOPBACK channel.

Signed-off-by: Hemant Kumar 
---
 drivers/bus/mhi/Kconfig  |  13 +
 drivers/bus/mhi/Makefile |   3 +
 drivers/bus/mhi/uci.c| 667 +++
 3 files changed, 683 insertions(+)
 create mode 100644 drivers/bus/mhi/uci.c

diff --git a/drivers/bus/mhi/Kconfig b/drivers/bus/mhi/Kconfig
index da5cd0c..5194e8e 100644
--- a/drivers/bus/mhi/Kconfig
+++ b/drivers/bus/mhi/Kconfig
@@ -29,3 +29,16 @@ config MHI_BUS_PCI_GENERIC
  This driver provides MHI PCI controller driver for devices such as
  Qualcomm SDX55 based PCIe modems.
 
+config MHI_UCI
+   tristate "MHI UCI"
+   depends on MHI_BUS
+   help
+ MHI based Userspace Client Interface (UCI) driver is used for
+ transferring raw data between host and device using standard file
+ operations from userspace. Open, read, write, poll and close
+ operations are supported by this driver. Please check
+ mhi_uci_match_table for all supported channels that are exposed to
+ userspace.
+
+ To compile this driver as a module, choose M here: the module will be
+ called mhi_uci.
diff --git a/drivers/bus/mhi/Makefile b/drivers/bus/mhi/Makefile
index 0a2d778..69f2111 100644
--- a/drivers/bus/mhi/Makefile
+++ b/drivers/bus/mhi/Makefile
@@ -4,3 +4,6 @@ obj-y += core/
 obj-$(CONFIG_MHI_BUS_PCI_GENERIC) += mhi_pci_generic.o
 mhi_pci_generic-y += pci_generic.o
 
+# MHI client
+mhi_uci-y := uci.o
+obj-$(CONFIG_MHI_UCI) += mhi_uci.o
diff --git a/drivers/bus/mhi/uci.c b/drivers/bus/mhi/uci.c
new file mode 100644
index 000..75b0831
--- /dev/null
+++ b/drivers/bus/mhi/uci.c
@@ -0,0 +1,667 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2018-2020, The Linux Foundation. All rights reserved.*/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define MHI_DEVICE_NAME "mhi"
+#define MHI_UCI_DRIVER_NAME "mhi_uci"
+#define MHI_MAX_UCI_MINORS 128
+
+static DEFINE_IDR(uci_idr);
+static DEFINE_MUTEX(uci_drv_mutex);
+static struct class *uci_dev_class;
+static int uci_dev_major;
+
+/**
+ * struct uci_chan - MHI channel for a UCI device
+ * @udev: associated UCI device object
+ * @ul_wq: wait queue for writer
+ * @write_lock: mutex write lock for ul channel
+ * @dl_wq: wait queue for reader
+ * @read_lock: mutex read lock for dl channel
+ * @dl_pending_lock: spin lock for dl_pending list
+ * @dl_pending: list of dl buffers userspace is waiting to read
+ * @cur_buf: current buffer userspace is reading
+ * @dl_size: size of the current dl buffer userspace is reading
+ * @ref_count: uci_chan reference count
+ */
+struct uci_chan {
+   struct uci_dev *udev;
+   wait_queue_head_t ul_wq;
+
+   /* ul channel lock to synchronize multiple writes */
+   struct mutex write_lock;
+
+   wait_queue_head_t dl_wq;
+
+   /* dl channel lock to synchronize multiple reads */
+   struct mutex read_lock;
+
+   /*
+* protects pending list in bh context, channel release, read and
+* poll
+*/
+   spinlock_t dl_pending_lock;
+
+   struct list_head dl_pending;
+   struct uci_buf *cur_buf;
+   size_t dl_size;
+   struct kref ref_count;
+};
+
+/**
+ * struct uci_buf - UCI buffer
+ * @data: data buffer
+ * @len: length of data buffer
+ * @node: list node of the UCI buffer
+ */
+struct uci_buf {
+   void *data;
+   size_t len;
+   struct list_head node;
+};
+
+/**
+ * struct uci_dev - MHI UCI device
+ * @minor: UCI device node minor number
+ * @mhi_dev: associated mhi device object
+ * @uchan: UCI uplink and downlink channel object
+ * @mtu: max TRE buffer length
+ * @enabled: Flag to track the state of the UCI device
+ * @lock: mutex lock to manage uchan object
+ * @ref_count: uci_dev reference count
+ */
+struct uci_dev {
+   unsigned int minor;
+   struct mhi_device *mhi_dev;
+   struct uci_chan *uchan;
+   size_t mtu;
+   bool enabled;
+
+   /* synchronize open, release and driver remove */
+   struct mutex lock;
+   struct kref ref_count;
+};
+
+static void mhi_uci_dev_chan_release(struct kref *ref)
+{
+   struct uci_buf *buf_itr, *tmp;
+   struct uci_chan *uchan =
+   container_of(ref, struct uci_chan, ref_count);
+
+   if (uchan->udev->enabled)
+   mhi

Re: [PATCH v2] bus: mhi: core: Fix null pointer access when parsing MHI configuration

2020-11-03 Thread Hemant Kumar




On 11/2/20 4:27 AM, carl@quectel.com wrote:

From: "carl.yin" 

Functions parse_ev_cfg() and parse_ch_cfg() access mhi_cntrl->mhi_dev
before it is set in function mhi_register_controller(),
use cntrl_dev instead of mhi_dev.

Fixes: 0cbf260820fa ("bus: mhi: core: Add support for registering MHI 
controllers")
Signed-off-by: carl.yin 
Reviewed-by: Bhaumik Bhatt 


Reviewed-by: Hemant Kumar 
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v11 4/4] bus: mhi: Add userspace client interface driver

2020-10-30 Thread Hemant Kumar

Hi Mani,

On 10/30/20 3:34 AM, Manivannan Sadhasivam wrote:

Hi Hemant,

On Thu, Oct 29, 2020 at 07:45:46PM -0700, Hemant Kumar wrote:

This MHI client driver allows userspace clients to transfer
raw data between MHI device and host using standard file operations.
Driver instantiates UCI device object which is associated to device
file node. UCI device object instantiates UCI channel object when device
file node is opened. UCI channel object is used to manage MHI channels
by calling MHI core APIs for read and write operations. MHI channels
are started as part of device open(). MHI channels remain in start
state until last release() is called on UCI device file node. Device
file node is created with format

/dev/mhi__

Currently it supports LOOPBACK channel.

Signed-off-by: Hemant Kumar 


Thanks for continuously updating the series based on reviews, now the locking
part looks a _lot_ cleaner than it used to be. I just have one query (inline)
regarding the usage of refcount for uci_chan and uci_dev. Once you fix that,
I think this is good to go in.

Thanks for reviewing my changes.

[..]


+#define DEVICE_NAME "mhi"
+#define MHI_UCI_DRIVER_NAME "mhi_uci"
+#define MAX_UCI_MINORS 128


Prefix MHI for these.

Done.




+
+static DEFINE_IDR(uci_idr);
+static DEFINE_MUTEX(uci_drv_mutex);
+static struct class *uci_dev_class;
+static int uci_dev_major;
+
+/**
+ * struct uci_chan - MHI channel for a UCI device
+ * @udev: associated UCI device object
+ * @ul_wq: wait queue for writer
+ * @write_lock: mutex write lock for ul channel
+ * @dl_wq: wait queue for reader
+ * @read_lock: mutex read lock for dl channel
+ * @dl_pending_lock: spin lock for dl_pending list
+ * @dl_pending: list of dl buffers userspace is waiting to read
+ * @cur_buf: current buffer userspace is reading
+ * @dl_size: size of the current dl buffer userspace is reading
+ * @ref_count: uci_chan reference count
+ */
+struct uci_chan {
+   struct uci_dev *udev;
+   wait_queue_head_t ul_wq;
+
+   /* ul channel lock to synchronize multiple writes */


I asked you to move these comments to Kdoc in previous iteration.
There are multiple revisions of UCI pushed after i responded on this 
one. On V7 i responded to your comment  :)


"This was added because checkpatch --strict required to add a comment 
when lock is added to struct, after adding inline comment, checkpatch 
error was gone."


i was sticking to --strict option. Considering it is best to address 
what --strict is complaining for.



+   struct mutex write_lock;
+
+   wait_queue_head_t dl_wq;
+
+   /* dl channel lock to synchronize multiple reads */
+   struct mutex read_lock;
+
+   /*
+* protects pending list in bh context, channel release, read and
+* poll
+*/
+   spinlock_t dl_pending_lock;
+
+   struct list_head dl_pending;
+   struct uci_buf *cur_buf;
+   size_t dl_size;
+   struct kref ref_count;


I'm now thinking that instead of having two reference counts for uci_chan and
uci_dev, why can't you club them together and just use uci_dev's refcount to
handle the channel management also.

For instance in uci_open, you are incrementing the refcount for uci_dev before
starting the channel and then doing the same for uci_chan in
mhi_uci_dev_start_chan(). So why can't you just use a single refcount once the
mhi_uci_dev_start_chan() succeeds? The UCI device is useless without a channel,
isn't it?
Main idea is to have the uci driver probed (uci device object is 
instantiated) but it is possible that device node is not opened or if it 
was opened before and release() was called after that. So UCI channel is 
not active but device node would continue to exist. Which can be opened 
again and channel would move to start state. So we dont want to couple 
mhi driver probe with starting of channels. We start channels only when 
it is really needed. This would allow MHI device to go to lower power 
state when channels are disabled.


[..]


+
+static int mhi_queue_inbound(struct uci_dev *udev)
+{
+   struct mhi_device *mhi_dev = udev->mhi_dev;
+   struct device *dev = _dev->dev;
+   int nr_trbs, i, ret = -EIO;


s/nr_trbs/nr_desc

Done.



+   size_t dl_buf_size;
+   void *buf;
+   struct uci_buf *ubuf;
+
+   /* dont queue if dl channel is not supported */
+   if (!udev->mhi_dev->dl_chan)
+   return 0;


Not returning an error?
Here we dont need to return error because when open is called it would 
call this function and if dl_chan is not supported we still want to 
return success for a uci device which only supports UL channel.
Keeping this check inside function looks clean so i am not adding this 
check in open().


[..]


+static __poll_t mhi_uci_poll(struct file *file, poll_table *wait)
+{
+   struct uci_dev *udev = file->private_data;
+   struct mhi_device *mhi_dev = udev->mhi_dev;
+   struct device *dev = _dev-&g

Re: [PATCH v11 4/4] bus: mhi: Add userspace client interface driver

2020-10-30 Thread Hemant Kumar

Hi Randy,

On 10/29/20 10:48 PM, Randy Dunlap wrote:

On 10/29/20 7:45 PM, Hemant Kumar wrote:

diff --git a/drivers/bus/mhi/Kconfig b/drivers/bus/mhi/Kconfig
index e841c10..476cc55 100644
--- a/drivers/bus/mhi/Kconfig
+++ b/drivers/bus/mhi/Kconfig
@@ -20,3 +20,16 @@ config MHI_BUS_DEBUG
  Enable debugfs support for use with the MHI transport. Allows
  reading and/or modifying some values within the MHI controller
  for debug and test purposes.
+
+config MHI_UCI
+   tristate "MHI UCI"
+   depends on MHI_BUS
+   help
+ MHI based Userspace Client Interface (UCI) driver is used for


  MHI-based


+ transferring raw data between host and device using standard file
+ operations from userspace. Open, read, write, and close operations
+ are supported by this driver. Please check mhi_uci_match_table for


also poll according to the documentation.

good catch, will add in next patch set.



+ all supported channels that are exposed to userspace.
+
+ To compile this driver as a module, choose M here: the module will be
+ called mhi_uci.





Thanks,
Hemant
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


[PATCH v11 3/4] docs: Add documentation for userspace client interface

2020-10-29 Thread Hemant Kumar
MHI userspace client driver is creating device file node
for user application to perform file operations. File
operations are handled by MHI core driver. Currently
Loopback MHI channel is supported by this driver.

Signed-off-by: Hemant Kumar 
---
 Documentation/mhi/index.rst |  1 +
 Documentation/mhi/uci.rst   | 84 +
 2 files changed, 85 insertions(+)
 create mode 100644 Documentation/mhi/uci.rst

diff --git a/Documentation/mhi/index.rst b/Documentation/mhi/index.rst
index 1d8dec3..c75a371 100644
--- a/Documentation/mhi/index.rst
+++ b/Documentation/mhi/index.rst
@@ -9,6 +9,7 @@ MHI
 
mhi
topology
+   uci
 
 .. only::  subproject and html
 
diff --git a/Documentation/mhi/uci.rst b/Documentation/mhi/uci.rst
new file mode 100644
index 000..4f706cb
--- /dev/null
+++ b/Documentation/mhi/uci.rst
@@ -0,0 +1,84 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=
+Userspace Client Interface (UCI)
+=
+
+UCI driver enables userspace clients to communicate to external MHI devices
+like modem and WLAN. UCI driver probe creates standard character device file
+nodes for userspace clients to perform open, read, write, poll and release file
+operations.
+
+Operations
+==
+
+open
+
+
+Instantiates UCI channel object and starts MHI channels to move it to running
+state. Inbound buffers are queued to downlink channel transfer ring. Every
+subsequent open() increments UCI device reference count as well as UCI channel
+reference count.
+
+read
+
+
+When data transfer is completed on downlink channel, transfer ring element
+buffer is copied to pending list. Reader is unblocked and data is copied to
+userspace buffer. Transfer ring element buffer is queued back to downlink
+channel transfer ring.
+
+write
+-
+
+Write buffer is queued to uplink channel transfer ring if ring is not full. 
Upon
+uplink transfer completion buffer is freed.
+
+poll
+
+
+Returns EPOLLIN | EPOLLRDNORM mask if pending list has buffers to be read by
+userspace. Returns EPOLLOUT | EPOLLWRNORM mask if MHI uplink channel transfer
+ring is not empty. Returns EPOLLERR when UCI driver is removed.
+
+release
+---
+
+Decrements UCI device reference count and UCI channel reference count upon last
+release(). UCI channel clean up is performed. MHI channel moves to disable
+state and inbound buffers are freed.
+
+Usage
+=
+
+Device file node is created with format:-
+
+/dev/mhi__
+
+controller_name is the name of underlying bus used to transfer data. mhi_device
+name is the name of the MHI channel being used by MHI client in userspace to
+send or receive data using MHI protocol.
+
+There is a separate character device file node created for each channel
+specified in MHI device id table. MHI channels are statically defined by MHI
+specification. The list of supported channels is in the channel list variable
+of mhi_device_id table in UCI driver.
+
+LOOPBACK Channel
+
+
+Userspace MHI client using LOOPBACK channel opens device file node. As part of
+open operation transfer ring elements are queued to transfer ring of LOOPBACK
+channel 1 and channel doorbell is rung. When userspace MHI client performs 
write
+operation on device node, data buffer gets queued as part of transfer ring
+element to transfer ring of LOOPBACK channel 0. MHI Core driver rings the
+channel doorbell for MHI device to move data over underlying bus. When 
userspace
+MHI client driver performs read operation, same data gets looped back to MHI
+host using LOOPBACK channel 1. LOOPBACK channel is used to verify data path
+and data integrity between MHI Host and MHI device.
+
+Other Use Cases
+---
+
+Getting MHI device specific diagnostics information to userspace MHI diagnostic
+client using DIAG channel 4 (Host to device) and 5 (Device to Host).
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v11 4/4] bus: mhi: Add userspace client interface driver

2020-10-29 Thread Hemant Kumar
This MHI client driver allows userspace clients to transfer
raw data between MHI device and host using standard file operations.
Driver instantiates UCI device object which is associated to device
file node. UCI device object instantiates UCI channel object when device
file node is opened. UCI channel object is used to manage MHI channels
by calling MHI core APIs for read and write operations. MHI channels
are started as part of device open(). MHI channels remain in start
state until last release() is called on UCI device file node. Device
file node is created with format

/dev/mhi__

Currently it supports LOOPBACK channel.

Signed-off-by: Hemant Kumar 
---
 drivers/bus/mhi/Kconfig  |  13 +
 drivers/bus/mhi/Makefile |   4 +
 drivers/bus/mhi/uci.c| 662 +++
 3 files changed, 679 insertions(+)
 create mode 100644 drivers/bus/mhi/uci.c

diff --git a/drivers/bus/mhi/Kconfig b/drivers/bus/mhi/Kconfig
index e841c10..476cc55 100644
--- a/drivers/bus/mhi/Kconfig
+++ b/drivers/bus/mhi/Kconfig
@@ -20,3 +20,16 @@ config MHI_BUS_DEBUG
  Enable debugfs support for use with the MHI transport. Allows
  reading and/or modifying some values within the MHI controller
  for debug and test purposes.
+
+config MHI_UCI
+   tristate "MHI UCI"
+   depends on MHI_BUS
+   help
+ MHI based Userspace Client Interface (UCI) driver is used for
+ transferring raw data between host and device using standard file
+ operations from userspace. Open, read, write, and close operations
+ are supported by this driver. Please check mhi_uci_match_table for
+ all supported channels that are exposed to userspace.
+
+ To compile this driver as a module, choose M here: the module will be
+ called mhi_uci.
diff --git a/drivers/bus/mhi/Makefile b/drivers/bus/mhi/Makefile
index 19e6443..80feefb 100644
--- a/drivers/bus/mhi/Makefile
+++ b/drivers/bus/mhi/Makefile
@@ -1,2 +1,6 @@
 # core layer
 obj-y += core/
+
+# MHI client
+mhi_uci-y := uci.o
+obj-$(CONFIG_MHI_UCI) += mhi_uci.o
diff --git a/drivers/bus/mhi/uci.c b/drivers/bus/mhi/uci.c
new file mode 100644
index 000..ea73c08
--- /dev/null
+++ b/drivers/bus/mhi/uci.c
@@ -0,0 +1,662 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2018-2020, The Linux Foundation. All rights reserved.*/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DEVICE_NAME "mhi"
+#define MHI_UCI_DRIVER_NAME "mhi_uci"
+#define MAX_UCI_MINORS 128
+
+static DEFINE_IDR(uci_idr);
+static DEFINE_MUTEX(uci_drv_mutex);
+static struct class *uci_dev_class;
+static int uci_dev_major;
+
+/**
+ * struct uci_chan - MHI channel for a UCI device
+ * @udev: associated UCI device object
+ * @ul_wq: wait queue for writer
+ * @write_lock: mutex write lock for ul channel
+ * @dl_wq: wait queue for reader
+ * @read_lock: mutex read lock for dl channel
+ * @dl_pending_lock: spin lock for dl_pending list
+ * @dl_pending: list of dl buffers userspace is waiting to read
+ * @cur_buf: current buffer userspace is reading
+ * @dl_size: size of the current dl buffer userspace is reading
+ * @ref_count: uci_chan reference count
+ */
+struct uci_chan {
+   struct uci_dev *udev;
+   wait_queue_head_t ul_wq;
+
+   /* ul channel lock to synchronize multiple writes */
+   struct mutex write_lock;
+
+   wait_queue_head_t dl_wq;
+
+   /* dl channel lock to synchronize multiple reads */
+   struct mutex read_lock;
+
+   /*
+* protects pending list in bh context, channel release, read and
+* poll
+*/
+   spinlock_t dl_pending_lock;
+
+   struct list_head dl_pending;
+   struct uci_buf *cur_buf;
+   size_t dl_size;
+   struct kref ref_count;
+};
+
+/**
+ * struct uci_buf - UCI buffer
+ * @data: data buffer
+ * @len: length of data buffer
+ * @node: list node of the UCI buffer
+ */
+struct uci_buf {
+   void *data;
+   size_t len;
+   struct list_head node;
+};
+
+/**
+ * struct uci_dev - MHI UCI device
+ * @minor: UCI device node minor number
+ * @mhi_dev: associated mhi device object
+ * @uchan: UCI uplink and downlink channel object
+ * @mtu: max TRE buffer length
+ * @enabled: Flag to track the state of the UCI device
+ * @lock: mutex lock to manage uchan object
+ * @ref_count: uci_dev reference count
+ */
+struct uci_dev {
+   unsigned int minor;
+   struct mhi_device *mhi_dev;
+   struct uci_chan *uchan;
+   size_t mtu;
+   bool enabled;
+
+   /* synchronize open, release and driver remove */
+   struct mutex lock;
+   struct kref ref_count;
+};
+
+static void mhi_uci_dev_chan_release(struct kref *ref)
+{
+   struct uci_buf *buf_itr, *tmp;
+   struct uci_chan *uchan =
+   container_of(ref, struct uci_chan, ref_count);
+
+   if (uchan->udev->enabled)
+   mhi_unprepare_from_transfer(uchan->

[PATCH v11 0/4] userspace MHI client interface driver

2020-10-29 Thread Hemant Kumar
This patch series adds support for UCI driver. UCI driver enables userspace
clients to communicate to external MHI devices like modem and WLAN. UCI driver
probe creates standard character device file nodes for userspace clients to
perform open, read, write, poll and release file operations. These file
operations call MHI core layer APIs to perform data transfer using MHI bus
to communicate with MHI device. Patch is tested using arm64 based platform.

V11:
- Fixed review comments for UCI documentation by expanding TLAs and rewording
  some sentences.

V10:
- Replaced mutex_lock with mutex_lock_interruptible in read() and write() file
  ops call back.

V9:
- Renamed dl_lock to dl_pending _lock and pending list to dl_pending for
  clarity.
- Used read lock to protect cur_buf.
- Change transfer status check logic and only consider 0 and -EOVERFLOW as
  only success.
- Added __int to module init function.
- Print channel name instead of minor number upon successful probe.

V8:
- Fixed kernel test robot compilation error by changing %lu to %zu for
  size_t.
- Replaced uci with UCI in Kconfig, commit text, and comments in driver
  code.
- Fixed minor style related comments.

V7:
- Decoupled uci device and uci channel objects. uci device is
  associated with device file node. uci channel is associated
  with MHI channels. uci device refers to uci channel to perform
  MHI channel operations for device file operations like read()
  and write(). uci device increments its reference count for
  every open(). uci device calls mhi_uci_dev_start_chan() to start
  the MHI channel. uci channel object is tracking number of times
  MHI channel is referred. This allows to keep the MHI channel in
  start state until last release() is called. After that uci channel
  reference count goes to 0 and uci channel clean up is performed
  which stops the MHI channel. After the last call to release() if
  driver is removed uci reference count becomes 0 and uci object is
  cleaned up.
- Use separate uci channel read and write lock to fine grain locking
  between reader and writer.
- Use uci device lock to synchronize open, release and driver remove.
- Optimize for downlink only or uplink only UCI device.

V6:
- Moved uci.c to mhi directory.
- Updated Kconfig to add module information.
- Updated Makefile to rename uci object file name as mhi_uci
- Removed kref for open count

V5:
- Removed mhi_uci_drv structure.
- Used idr instead of creating global list of uci devices.
- Used kref instead of local ref counting for uci device and
  open count.
- Removed unlikely macro.

V4:
- Fix locking to protect proper struct members.
- Updated documentation describing uci client driver use cases.
- Fixed uci ref counting in mhi_uci_open for error case.
- Addressed style related review comments.

V3: Added documentation for MHI UCI driver.

V2:
- Added mutex lock to prevent multiple readers to access same
- mhi buffer which can result into use after free.

Hemant Kumar (4):
  bus: mhi: core: Add helper API to return number of free TREs
  bus: mhi: core: Move MHI_MAX_MTU to external header file
  docs: Add documentation for userspace client interface
  bus: mhi: Add userspace client interface driver

 Documentation/mhi/index.rst |   1 +
 Documentation/mhi/uci.rst   |  84 +
 drivers/bus/mhi/Kconfig |  13 +
 drivers/bus/mhi/Makefile|   4 +
 drivers/bus/mhi/core/internal.h |   1 -
 drivers/bus/mhi/core/main.c |  12 +
 drivers/bus/mhi/uci.c   | 662 
 include/linux/mhi.h |  12 +
 8 files changed, 788 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/mhi/uci.rst
 create mode 100644 drivers/bus/mhi/uci.c

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v11 2/4] bus: mhi: core: Move MHI_MAX_MTU to external header file

2020-10-29 Thread Hemant Kumar
Currently this macro is defined in internal MHI header as
a TRE length mask. Moving it to external header allows MHI
client drivers to set this upper bound for the transmit
buffer size.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
Reviewed-by: Manivannan Sadhasivam 
---
 drivers/bus/mhi/core/internal.h | 1 -
 include/linux/mhi.h | 3 +++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
index 7989269..4abf0cf 100644
--- a/drivers/bus/mhi/core/internal.h
+++ b/drivers/bus/mhi/core/internal.h
@@ -453,7 +453,6 @@ enum mhi_pm_state {
 #define CMD_EL_PER_RING128
 #define PRIMARY_CMD_RING   0
 #define MHI_DEV_WAKE_DB127
-#define MHI_MAX_MTU0x
 #define MHI_RANDOM_U32_NONZERO(bmsk)   (prandom_u32_max(bmsk) + 1)
 
 enum mhi_er_type {
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index 7829b1d..6e1122c 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -15,6 +15,9 @@
 #include 
 #include 
 
+/* MHI client drivers to set this upper bound for tx buffer */
+#define MHI_MAX_MTU 0x
+
 #define MHI_MAX_OEM_PK_HASH_SEGMENTS 16
 
 struct mhi_chan;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v11 1/4] bus: mhi: core: Add helper API to return number of free TREs

2020-10-29 Thread Hemant Kumar
Introduce mhi_get_free_desc_count() API to return number
of TREs available to queue buffer. MHI clients can use this
API to know before hand if ring is full without calling queue
API.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
Reviewed-by: Manivannan Sadhasivam 
---
 drivers/bus/mhi/core/main.c | 12 
 include/linux/mhi.h |  9 +
 2 files changed, 21 insertions(+)

diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index 2cff5dd..3950792 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -258,6 +258,18 @@ int mhi_destroy_device(struct device *dev, void *data)
return 0;
 }
 
+int mhi_get_free_desc_count(struct mhi_device *mhi_dev,
+   enum dma_data_direction dir)
+{
+   struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
+   struct mhi_chan *mhi_chan = (dir == DMA_TO_DEVICE) ?
+   mhi_dev->ul_chan : mhi_dev->dl_chan;
+   struct mhi_ring *tre_ring = _chan->tre_ring;
+
+   return get_nr_avail_ring_elements(mhi_cntrl, tre_ring);
+}
+EXPORT_SYMBOL_GPL(mhi_get_free_desc_count);
+
 void mhi_notify(struct mhi_device *mhi_dev, enum mhi_callback cb_reason)
 {
struct mhi_driver *mhi_drv;
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index d4841e5..7829b1d 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -597,6 +597,15 @@ void mhi_set_mhi_state(struct mhi_controller *mhi_cntrl,
 void mhi_notify(struct mhi_device *mhi_dev, enum mhi_callback cb_reason);
 
 /**
+ * mhi_get_free_desc_count - Get transfer ring length
+ * Get # of TD available to queue buffers
+ * @mhi_dev: Device associated with the channels
+ * @dir: Direction of the channel
+ */
+int mhi_get_free_desc_count(struct mhi_device *mhi_dev,
+   enum dma_data_direction dir);
+
+/**
  * mhi_prepare_for_power_up - Do pre-initialization before power up.
  *This is optional, call this before power up if
  *the controller does not want bus framework to
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



Re: [PATCH v10 3/4] docs: Add documentation for userspace client interface

2020-10-29 Thread Hemant Kumar

Hi Randy,

On 10/29/20 2:51 PM, Randy Dunlap wrote:

Hi,

On 10/29/20 2:40 PM, Hemant Kumar wrote:

MHI userspace client driver is creating device file node
for user application to perform file operations. File
operations are handled by MHI core driver. Currently
Loopback MHI channel is supported by this driver.

Signed-off-by: Hemant Kumar 
---
  Documentation/mhi/index.rst |  1 +
  Documentation/mhi/uci.rst   | 83 +
  2 files changed, 84 insertions(+)
  create mode 100644 Documentation/mhi/uci.rst




diff --git a/Documentation/mhi/uci.rst b/Documentation/mhi/uci.rst
new file mode 100644
index 000..fe901c4
--- /dev/null
+++ b/Documentation/mhi/uci.rst
@@ -0,0 +1,83 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=
+Userspace Client Interface (UCI)
+=
+



Lots of TLAs.


+
+read
+
+
+When data transfer is completed on downlink channel, TRE buffer is copied to
+pending list. Reader is unblocked and data is copied to userspace buffer. TRE
+buffer is queued back to downlink channel transfer ring.


What is TRE?

Transfer Ring Element
i will add that in small bracket inline.



+
+Usage
+=
+
+Device file node is created with format:-
+
+/dev/mhi__
+
+controller_name is the name of underlying bus used to transfer data. mhi_device
+name is the name of the MHI channel being used by MHI client in userspace to
+send or receive data using MHI protocol.
+
+There is a separate character device file node created for each channel
+specified in mhi device id table. MHI channels are statically defined by MHI


 MHI
unless it is a variable name, like below: mhi_device_id

Done.



+specification. The list of supported channels is in the channel list variable
+of mhi_device_id table in UCI driver.
+



+Other Use Cases
+---
+
+Getting MHI device specific diagnostics information to userspace MHI diag 
client


 
diagnostic client

Done.



+using DIAG channel 4 (Host to device) and 5 (Device to Host).



thanks.



Thanks for reviewing it. Let me fix it and re-upload.

Thanks,
Hemant

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


[PATCH v10 1/4] bus: mhi: core: Add helper API to return number of free TREs

2020-10-29 Thread Hemant Kumar
Introduce mhi_get_free_desc_count() API to return number
of TREs available to queue buffer. MHI clients can use this
API to know before hand if ring is full without calling queue
API.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
Reviewed-by: Manivannan Sadhasivam 
---
 drivers/bus/mhi/core/main.c | 12 
 include/linux/mhi.h |  9 +
 2 files changed, 21 insertions(+)

diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index 2cff5dd..3950792 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -258,6 +258,18 @@ int mhi_destroy_device(struct device *dev, void *data)
return 0;
 }
 
+int mhi_get_free_desc_count(struct mhi_device *mhi_dev,
+   enum dma_data_direction dir)
+{
+   struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
+   struct mhi_chan *mhi_chan = (dir == DMA_TO_DEVICE) ?
+   mhi_dev->ul_chan : mhi_dev->dl_chan;
+   struct mhi_ring *tre_ring = _chan->tre_ring;
+
+   return get_nr_avail_ring_elements(mhi_cntrl, tre_ring);
+}
+EXPORT_SYMBOL_GPL(mhi_get_free_desc_count);
+
 void mhi_notify(struct mhi_device *mhi_dev, enum mhi_callback cb_reason)
 {
struct mhi_driver *mhi_drv;
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index d4841e5..7829b1d 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -597,6 +597,15 @@ void mhi_set_mhi_state(struct mhi_controller *mhi_cntrl,
 void mhi_notify(struct mhi_device *mhi_dev, enum mhi_callback cb_reason);
 
 /**
+ * mhi_get_free_desc_count - Get transfer ring length
+ * Get # of TD available to queue buffers
+ * @mhi_dev: Device associated with the channels
+ * @dir: Direction of the channel
+ */
+int mhi_get_free_desc_count(struct mhi_device *mhi_dev,
+   enum dma_data_direction dir);
+
+/**
  * mhi_prepare_for_power_up - Do pre-initialization before power up.
  *This is optional, call this before power up if
  *the controller does not want bus framework to
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v10 3/4] docs: Add documentation for userspace client interface

2020-10-29 Thread Hemant Kumar
MHI userspace client driver is creating device file node
for user application to perform file operations. File
operations are handled by MHI core driver. Currently
Loopback MHI channel is supported by this driver.

Signed-off-by: Hemant Kumar 
---
 Documentation/mhi/index.rst |  1 +
 Documentation/mhi/uci.rst   | 83 +
 2 files changed, 84 insertions(+)
 create mode 100644 Documentation/mhi/uci.rst

diff --git a/Documentation/mhi/index.rst b/Documentation/mhi/index.rst
index 1d8dec3..c75a371 100644
--- a/Documentation/mhi/index.rst
+++ b/Documentation/mhi/index.rst
@@ -9,6 +9,7 @@ MHI
 
mhi
topology
+   uci
 
 .. only::  subproject and html
 
diff --git a/Documentation/mhi/uci.rst b/Documentation/mhi/uci.rst
new file mode 100644
index 000..fe901c4
--- /dev/null
+++ b/Documentation/mhi/uci.rst
@@ -0,0 +1,83 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=
+Userspace Client Interface (UCI)
+=
+
+UCI driver enables userspace clients to communicate to external MHI devices
+like modem and WLAN. UCI driver probe creates standard character device file
+nodes for userspace clients to perform open, read, write, poll and release file
+operations.
+
+Operations
+==
+
+open
+
+
+Instantiates UCI channel object and starts MHI channels to move it to running
+state. Inbound buffers are queued to downlink channel transfer ring. Every
+subsequent open() increments UCI device reference count as well as UCI channel
+reference count.
+
+read
+
+
+When data transfer is completed on downlink channel, TRE buffer is copied to
+pending list. Reader is unblocked and data is copied to userspace buffer. TRE
+buffer is queued back to downlink channel transfer ring.
+
+write
+-
+
+Write buffer is queued to uplink channel transfer ring if ring is not full. 
Upon
+uplink transfer completion buffer is freed.
+
+poll
+
+
+Returns EPOLLIN | EPOLLRDNORM mask if pending list has buffers to be read by
+userspace. Returns EPOLLOUT | EPOLLWRNORM mask if MHI uplink channel transfer
+ring is not empty. Returns EPOLLERR when UCI driver is removed. MHI channels
+move to disabled state upon driver remove.
+
+release
+---
+
+Decrements UCI device reference count and UCI channel reference count upon last
+release(). UCI channel clean up is performed. MHI channel moves to disabled
+state and inbound buffers are freed.
+
+Usage
+=
+
+Device file node is created with format:-
+
+/dev/mhi__
+
+controller_name is the name of underlying bus used to transfer data. mhi_device
+name is the name of the MHI channel being used by MHI client in userspace to
+send or receive data using MHI protocol.
+
+There is a separate character device file node created for each channel
+specified in mhi device id table. MHI channels are statically defined by MHI
+specification. The list of supported channels is in the channel list variable
+of mhi_device_id table in UCI driver.
+
+LOOPBACK Channel
+
+
+Userspace MHI client using LOOPBACK channel opens device file node. As part of
+open operation TREs to transfer ring of LOOPBACK channel 1 gets queued and 
channel
+doorbell is rung. When userspace MHI client performs write operation on device 
node,
+data buffer gets queued as a TRE to transfer ring of LOOPBACK channel 0. MHI 
Core
+driver rings the channel doorbell for MHI device to move data over underlying 
bus.
+When userspace MHI client driver performs read operation, same data gets 
looped back
+to MHI host using LOOPBACK channel 1. LOOPBACK channel is used to verify data 
path
+and data integrity between MHI Host and MHI device.
+
+Other Use Cases
+---
+
+Getting MHI device specific diagnostics information to userspace MHI diag 
client
+using DIAG channel 4 (Host to device) and 5 (Device to Host).
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v10 2/4] bus: mhi: core: Move MHI_MAX_MTU to external header file

2020-10-29 Thread Hemant Kumar
Currently this macro is defined in internal MHI header as
a TRE length mask. Moving it to external header allows MHI
client drivers to set this upper bound for the transmit
buffer size.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
Reviewed-by: Manivannan Sadhasivam 
---
 drivers/bus/mhi/core/internal.h | 1 -
 include/linux/mhi.h | 3 +++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
index 7989269..4abf0cf 100644
--- a/drivers/bus/mhi/core/internal.h
+++ b/drivers/bus/mhi/core/internal.h
@@ -453,7 +453,6 @@ enum mhi_pm_state {
 #define CMD_EL_PER_RING128
 #define PRIMARY_CMD_RING   0
 #define MHI_DEV_WAKE_DB127
-#define MHI_MAX_MTU0x
 #define MHI_RANDOM_U32_NONZERO(bmsk)   (prandom_u32_max(bmsk) + 1)
 
 enum mhi_er_type {
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index 7829b1d..6e1122c 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -15,6 +15,9 @@
 #include 
 #include 
 
+/* MHI client drivers to set this upper bound for tx buffer */
+#define MHI_MAX_MTU 0x
+
 #define MHI_MAX_OEM_PK_HASH_SEGMENTS 16
 
 struct mhi_chan;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v10 4/4] bus: mhi: Add userspace client interface driver

2020-10-29 Thread Hemant Kumar
This MHI client driver allows userspace clients to transfer
raw data between MHI device and host using standard file operations.
Driver instantiates UCI device object which is associated to device
file node. UCI device object instantiates UCI channel object when device
file node is opened. UCI channel object is used to manage MHI channels
by calling MHI core APIs for read and write operations. MHI channels
are started as part of device open(). MHI channels remain in start
state until last release() is called on UCI device file node. Device
file node is created with format

/dev/mhi__

Currently it supports LOOPBACK channel.

Signed-off-by: Hemant Kumar 
---
 drivers/bus/mhi/Kconfig  |  13 +
 drivers/bus/mhi/Makefile |   4 +
 drivers/bus/mhi/uci.c| 662 +++
 3 files changed, 679 insertions(+)
 create mode 100644 drivers/bus/mhi/uci.c

diff --git a/drivers/bus/mhi/Kconfig b/drivers/bus/mhi/Kconfig
index e841c10..476cc55 100644
--- a/drivers/bus/mhi/Kconfig
+++ b/drivers/bus/mhi/Kconfig
@@ -20,3 +20,16 @@ config MHI_BUS_DEBUG
  Enable debugfs support for use with the MHI transport. Allows
  reading and/or modifying some values within the MHI controller
  for debug and test purposes.
+
+config MHI_UCI
+   tristate "MHI UCI"
+   depends on MHI_BUS
+   help
+ MHI based Userspace Client Interface (UCI) driver is used for
+ transferring raw data between host and device using standard file
+ operations from userspace. Open, read, write, and close operations
+ are supported by this driver. Please check mhi_uci_match_table for
+ all supported channels that are exposed to userspace.
+
+ To compile this driver as a module, choose M here: the module will be
+ called mhi_uci.
diff --git a/drivers/bus/mhi/Makefile b/drivers/bus/mhi/Makefile
index 19e6443..80feefb 100644
--- a/drivers/bus/mhi/Makefile
+++ b/drivers/bus/mhi/Makefile
@@ -1,2 +1,6 @@
 # core layer
 obj-y += core/
+
+# MHI client
+mhi_uci-y := uci.o
+obj-$(CONFIG_MHI_UCI) += mhi_uci.o
diff --git a/drivers/bus/mhi/uci.c b/drivers/bus/mhi/uci.c
new file mode 100644
index 000..ea73c08
--- /dev/null
+++ b/drivers/bus/mhi/uci.c
@@ -0,0 +1,662 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2018-2020, The Linux Foundation. All rights reserved.*/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DEVICE_NAME "mhi"
+#define MHI_UCI_DRIVER_NAME "mhi_uci"
+#define MAX_UCI_MINORS 128
+
+static DEFINE_IDR(uci_idr);
+static DEFINE_MUTEX(uci_drv_mutex);
+static struct class *uci_dev_class;
+static int uci_dev_major;
+
+/**
+ * struct uci_chan - MHI channel for a UCI device
+ * @udev: associated UCI device object
+ * @ul_wq: wait queue for writer
+ * @write_lock: mutex write lock for ul channel
+ * @dl_wq: wait queue for reader
+ * @read_lock: mutex read lock for dl channel
+ * @dl_pending_lock: spin lock for dl_pending list
+ * @dl_pending: list of dl buffers userspace is waiting to read
+ * @cur_buf: current buffer userspace is reading
+ * @dl_size: size of the current dl buffer userspace is reading
+ * @ref_count: uci_chan reference count
+ */
+struct uci_chan {
+   struct uci_dev *udev;
+   wait_queue_head_t ul_wq;
+
+   /* ul channel lock to synchronize multiple writes */
+   struct mutex write_lock;
+
+   wait_queue_head_t dl_wq;
+
+   /* dl channel lock to synchronize multiple reads */
+   struct mutex read_lock;
+
+   /*
+* protects pending list in bh context, channel release, read and
+* poll
+*/
+   spinlock_t dl_pending_lock;
+
+   struct list_head dl_pending;
+   struct uci_buf *cur_buf;
+   size_t dl_size;
+   struct kref ref_count;
+};
+
+/**
+ * struct uci_buf - UCI buffer
+ * @data: data buffer
+ * @len: length of data buffer
+ * @node: list node of the UCI buffer
+ */
+struct uci_buf {
+   void *data;
+   size_t len;
+   struct list_head node;
+};
+
+/**
+ * struct uci_dev - MHI UCI device
+ * @minor: UCI device node minor number
+ * @mhi_dev: associated mhi device object
+ * @uchan: UCI uplink and downlink channel object
+ * @mtu: max TRE buffer length
+ * @enabled: Flag to track the state of the UCI device
+ * @lock: mutex lock to manage uchan object
+ * @ref_count: uci_dev reference count
+ */
+struct uci_dev {
+   unsigned int minor;
+   struct mhi_device *mhi_dev;
+   struct uci_chan *uchan;
+   size_t mtu;
+   bool enabled;
+
+   /* synchronize open, release and driver remove */
+   struct mutex lock;
+   struct kref ref_count;
+};
+
+static void mhi_uci_dev_chan_release(struct kref *ref)
+{
+   struct uci_buf *buf_itr, *tmp;
+   struct uci_chan *uchan =
+   container_of(ref, struct uci_chan, ref_count);
+
+   if (uchan->udev->enabled)
+   mhi_unprepare_from_transfer(uchan->

[PATCH v10 0/4] userspace MHI client interface driver

2020-10-29 Thread Hemant Kumar
This patch series adds support for UCI driver. UCI driver enables userspace
clients to communicate to external MHI devices like modem and WLAN. UCI driver
probe creates standard character device file nodes for userspace clients to
perform open, read, write, poll and release file operations. These file
operations call MHI core layer APIs to perform data transfer using MHI bus
to communicate with MHI device. Patch is tested using arm64 based platform.

V10:
- Replaced mutex_lock with mutex_lock_interruptible in read() and write() file
  ops call back.

V9:
- Renamed dl_lock to dl_pending _lock and pending list to dl_pending for
  clarity.
- Used read lock to protect cur_buf.
- Change transfer status check logic and only consider 0 and -EOVERFLOW as
  only success.
- Added __int to module init function.
- Print channel name instead of minor number upon successful probe.

V8:
- Fixed kernel test robot compilation error by changing %lu to %zu for
  size_t.
- Replaced uci with UCI in Kconfig, commit text, and comments in driver
  code.
- Fixed minor style related comments.

V7:
- Decoupled uci device and uci channel objects. uci device is
  associated with device file node. uci channel is associated
  with MHI channels. uci device refers to uci channel to perform
  MHI channel operations for device file operations like read()
  and write(). uci device increments its reference count for
  every open(). uci device calls mhi_uci_dev_start_chan() to start
  the MHI channel. uci channel object is tracking number of times
  MHI channel is referred. This allows to keep the MHI channel in
  start state until last release() is called. After that uci channel
  reference count goes to 0 and uci channel clean up is performed
  which stops the MHI channel. After the last call to release() if
  driver is removed uci reference count becomes 0 and uci object is
  cleaned up.
- Use separate uci channel read and write lock to fine grain locking
  between reader and writer.
- Use uci device lock to synchronize open, release and driver remove.
- Optimize for downlink only or uplink only UCI device.

V6:
- Moved uci.c to mhi directory.
- Updated Kconfig to add module information.
- Updated Makefile to rename uci object file name as mhi_uci
- Removed kref for open count

V5:
- Removed mhi_uci_drv structure.
- Used idr instead of creating global list of uci devices.
- Used kref instead of local ref counting for uci device and
  open count.
- Removed unlikely macro.

V4:
- Fix locking to protect proper struct members.
- Updated documentation describing uci client driver use cases.
- Fixed uci ref counting in mhi_uci_open for error case.
- Addressed style related review comments.

V3: Added documentation for MHI UCI driver.

V2:
- Added mutex lock to prevent multiple readers to access same
- mhi buffer which can result into use after free.

Hemant Kumar (4):
  bus: mhi: core: Add helper API to return number of free TREs
  bus: mhi: core: Move MHI_MAX_MTU to external header file
  docs: Add documentation for userspace client interface
  bus: mhi: Add userspace client interface driver

 Documentation/mhi/index.rst |   1 +
 Documentation/mhi/uci.rst   |  83 +
 drivers/bus/mhi/Kconfig |  13 +
 drivers/bus/mhi/Makefile|   4 +
 drivers/bus/mhi/core/internal.h |   1 -
 drivers/bus/mhi/core/main.c |  12 +
 drivers/bus/mhi/uci.c   | 662 
 include/linux/mhi.h |  12 +
 8 files changed, 787 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/mhi/uci.rst
 create mode 100644 drivers/bus/mhi/uci.c

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



Re: [PATCH] bus: mhi: core: Add support MHI EE FP for download firmware

2020-10-27 Thread Hemant Kumar

Hi Jeff,

On 10/27/20 8:11 AM, Jeffrey Hugo wrote:

On 10/27/2020 3:43 AM, carl@quectel.com wrote:

From: "carl.yin" 

MHI wwan modems support download firmware to nand or emmc
by firehose protocol, process as next:
1. wwan modem normal bootup and enter EE AMSS, create mhi DIAG chan 
device

2. send EDL cmd via DIAG chan, then modem enter EE EDL
3. boot.c download 'firehose/prog_firehose_sdx55.mbn' via BHI interface
4. modem enter EE FP, and create mhi EDL chan device
5. user space tool download FW to modem via EDL chan by firehose protocol

Signed-off-by: carl.yin 
---
  drivers/bus/mhi/core/boot.c |  4 +++-
  drivers/bus/mhi/core/init.c |  2 ++
  drivers/bus/mhi/core/internal.h |  1 +
  drivers/bus/mhi/core/main.c |  3 +++
  drivers/bus/mhi/core/pm.c   | 16 +++-
  include/linux/mhi.h |  4 +++-
  6 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/bus/mhi/core/boot.c b/drivers/bus/mhi/core/boot.c
index 24422f5..ab39ad6 100644
--- a/drivers/bus/mhi/core/boot.c
+++ b/drivers/bus/mhi/core/boot.c
@@ -460,8 +460,10 @@ void mhi_fw_load_handler(struct mhi_controller 
*mhi_cntrl)

  return;
  }
-    if (mhi_cntrl->ee == MHI_EE_EDL)
+    if (mhi_cntrl->ee == MHI_EE_EDL) {
+    mhi_ready_state_transition(mhi_cntrl);
  return;
+    }
  write_lock_irq(_cntrl->pm_lock);
  mhi_cntrl->dev_state = MHI_STATE_RESET;
diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
index ac4aa5c..9c2c2f3 100644
--- a/drivers/bus/mhi/core/init.c
+++ b/drivers/bus/mhi/core/init.c
@@ -26,6 +26,7 @@ const char * const mhi_ee_str[MHI_EE_MAX] = {
  [MHI_EE_WFW] = "WFW",
  [MHI_EE_PTHRU] = "PASS THRU",
  [MHI_EE_EDL] = "EDL",
+    [MHI_EE_FP] = "FP",
  [MHI_EE_DISABLE_TRANSITION] = "DISABLE",
  [MHI_EE_NOT_SUPPORTED] = "NOT SUPPORTED",
  };
@@ -35,6 +36,7 @@ const char * const 
dev_state_tran_str[DEV_ST_TRANSITION_MAX] = {

  [DEV_ST_TRANSITION_READY] = "READY",
  [DEV_ST_TRANSITION_SBL] = "SBL",
  [DEV_ST_TRANSITION_MISSION_MODE] = "MISSION_MODE",
+    [DEV_ST_TRANSITION_FP] = "FP",
  [DEV_ST_TRANSITION_SYS_ERR] = "SYS_ERR",
  [DEV_ST_TRANSITION_DISABLE] = "DISABLE",
  };
diff --git a/drivers/bus/mhi/core/internal.h 
b/drivers/bus/mhi/core/internal.h

index 4abf0cf..6ae897a 100644
--- a/drivers/bus/mhi/core/internal.h
+++ b/drivers/bus/mhi/core/internal.h
@@ -386,6 +386,7 @@ enum dev_st_transition {
  DEV_ST_TRANSITION_READY,
  DEV_ST_TRANSITION_SBL,
  DEV_ST_TRANSITION_MISSION_MODE,
+    DEV_ST_TRANSITION_FP,
  DEV_ST_TRANSITION_SYS_ERR,
  DEV_ST_TRANSITION_DISABLE,
  DEV_ST_TRANSITION_MAX,
diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index 3950792..e307b58 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -782,6 +782,9 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller 
*mhi_cntrl,

  case MHI_EE_SBL:
  st = DEV_ST_TRANSITION_SBL;
  break;
+    case MHI_EE_FP:
+    st = DEV_ST_TRANSITION_FP;
+    break;
  case MHI_EE_WFW:
  case MHI_EE_AMSS:
  st = DEV_ST_TRANSITION_MISSION_MODE;
diff --git a/drivers/bus/mhi/core/pm.c b/drivers/bus/mhi/core/pm.c
index 3de7b16..3c95a5d 100644
--- a/drivers/bus/mhi/core/pm.c
+++ b/drivers/bus/mhi/core/pm.c
@@ -563,7 +563,15 @@ static void mhi_pm_disable_transition(struct 
mhi_controller *mhi_cntrl,

  }
  if (cur_state == MHI_PM_SYS_ERR_PROCESS) {
-    mhi_ready_state_transition(mhi_cntrl);
+    if (mhi_get_exec_env(mhi_cntrl) == MHI_EE_EDL
+    && mhi_get_mhi_state(mhi_cntrl) == MHI_STATE_RESET) {
+    write_lock_irq(_cntrl->pm_lock);
+    cur_state = mhi_tryset_pm_state(mhi_cntrl, MHI_PM_POR);
+    write_unlock_irq(_cntrl->pm_lock);
+    mhi_queue_state_transition(mhi_cntrl, 
DEV_ST_TRANSITION_PBL);

+    } else {
+    mhi_ready_state_transition(mhi_cntrl);
+    }
  } else {
  /* Move to disable state */
  write_lock_irq(_cntrl->pm_lock);
@@ -658,6 +666,12 @@ void mhi_pm_st_worker(struct work_struct *work)
  case DEV_ST_TRANSITION_MISSION_MODE:
  mhi_pm_mission_mode_transition(mhi_cntrl);
  break;
+    case DEV_ST_TRANSITION_FP:
+    write_lock_irq(_cntrl->pm_lock);
+    mhi_cntrl->ee = MHI_EE_FP;
+    write_unlock_irq(_cntrl->pm_lock);
+    mhi_create_devices(mhi_cntrl);
+    break;
  case DEV_ST_TRANSITION_READY:
  mhi_ready_state_transition(mhi_cntrl);
  break;
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index 6e1122c..4620af8 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -120,6 +120,7 @@ struct mhi_link_info {
   * @MHI_EE_WFW: WLAN firmware mode
   * @MHI_EE_PTHRU: Passthrough
   * @MHI_EE_EDL: Embedded downloader
+ * @MHI_EE_FP, 

Re: [PATCH] bus: mhi: core: Introduce sysfs ul chan id for mhi chan device

2020-10-27 Thread Hemant Kumar

Hi Carl,

On 10/27/20 8:06 AM, Jeffrey Hugo wrote:

On 10/27/2020 3:43 AM, carl@quectel.com wrote:

From: "carl.yin" 

User space software like ModemManager can identify the function
of the mhi chan device by ul_chan_id.

Signed-off-by: carl.yin 
---
  Documentation/ABI/stable/sysfs-bus-mhi | 10 ++
  drivers/bus/mhi/core/init.c    | 15 +++
  2 files changed, 25 insertions(+)

diff --git a/Documentation/ABI/stable/sysfs-bus-mhi 
b/Documentation/ABI/stable/sysfs-bus-mhi

index ecfe766..6d52768 100644
--- a/Documentation/ABI/stable/sysfs-bus-mhi
+++ b/Documentation/ABI/stable/sysfs-bus-mhi
@@ -19,3 +19,13 @@ Description:    The file holds the OEM PK Hash 
value of the endpoint device

  read without having the device power on at least once, the file
  will read all 0's.
  Users:    Any userspace application or clients interested in 
device info.

+
+What:    /sys/bus/mhi/devices/.../ul_chan_id
+Date:    November 2020
+KernelVersion:    5.10
+Contact:    Carl Yin 
+Description:    The file holds the uplink chan id of the mhi chan 
device.
+    User space software like ModemManager can identify the 
function of

+    the mhi chan device. If the mhi device is not a chan device,
+    eg mhi controller device, the file read -1.
+Users:    Any userspace application or clients interested in 
device info.

diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
index c6b43e9..ac4aa5c 100644
--- a/drivers/bus/mhi/core/init.c
+++ b/drivers/bus/mhi/core/init.c
@@ -105,9 +105,24 @@ static ssize_t oem_pk_hash_show(struct device *dev,
  }
  static DEVICE_ATTR_RO(oem_pk_hash);
+static ssize_t ul_chan_id_show(struct device *dev,
+    struct device_attribute *attr,
+    char *buf)
+{
+    struct mhi_device *mhi_dev = to_mhi_device(dev);
+    int ul_chan_id = -1;
+
+    if (mhi_dev->ul_chan)
+    ul_chan_id = mhi_dev->ul_chan_id;
+
+    return snprintf(buf, PAGE_SIZE, "%d\n", ul_chan_id);
+}
+static DEVICE_ATTR_RO(ul_chan_id);
+
  static struct attribute *mhi_dev_attrs[] = {
  _attr_serial_number.attr,
  _attr_oem_pk_hash.attr,
+    _attr_ul_chan_id.attr,
  NULL,
  };
  ATTRIBUTE_GROUPS(mhi_dev);



NACK

Channel ID is a device specific detail.  Userspace should be basing 
decisions on the channel name.


I agree with Jeff, why do you need to know the channel id, if you need 
to poll for any device node to get created you can try to open the 
device node from user space and wait until the device gets opened.

Are you trying to wait for EDL channels to get started using UCI ?

Thanks,
Hemant
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v9 4/4] bus: mhi: Add userspace client interface driver

2020-10-26 Thread Hemant Kumar

Hi Loic,

On 10/26/20 10:34 AM, Loic Poulain wrote:

Hi Hemant,

That looks better IMHO, just small comments on locking.


[..]

+static ssize_t mhi_uci_write(struct file *file,
+                            const char __user *buf,
+                            size_t count,
+                            loff_t *offp)
+{
+       struct uci_dev *udev = file->private_data;
+       struct mhi_device *mhi_dev = udev->mhi_dev;
+       struct device *dev = _dev->dev;
+       struct uci_chan *uchan = udev->uchan;
+       size_t bytes_xfered = 0;
+       int ret, nr_avail = 0;
+
+       /* if ul channel is not supported return error */
+       if (!buf || !count || !mhi_dev->ul_chan)
+               return -EINVAL;
+
+       dev_dbg(dev, "%s: to xfer: %zu bytes\n", __func__, count);
+
+       mutex_lock(>write_lock);


Maybe mutex_lock_interruptible is more appropriate here (same in read fops).

i agree, will return -EINTR if mutex_lock_interruptible returns < 0.



[..]

+static ssize_t mhi_uci_read(struct file *file,
+                           char __user *buf,
+                           size_t count,
+                           loff_t *ppos)
+{
+       struct uci_dev *udev = file->private_data;
+       struct mhi_device *mhi_dev = udev->mhi_dev;
+       struct uci_chan *uchan = udev->uchan;
+       struct device *dev = _dev->dev;
+       struct uci_buf *ubuf;
+       size_t rx_buf_size;
+       char *ptr;
+       size_t to_copy;
+       int ret = 0;
+
+       /* if dl channel is not supported return error */
+       if (!buf || !mhi_dev->dl_chan)
+               return -EINVAL;
+
+       mutex_lock(>read_lock);
+       spin_lock_bh(>dl_pending_lock);
+       /* No data available to read, wait */
+       if (!uchan->cur_buf && list_empty(>dl_pending)) {
+               dev_dbg(dev, "No data available to read, waiting\n");
+
+               spin_unlock_bh(>dl_pending_lock);
+               ret = wait_event_interruptible(uchan->dl_wq,
+                                              (!udev->enabled ||
+   
  !list_empty(>dl_pending)));



If you need to protect dl_pending list against concurent access, you 
need to do it in wait_event as well. I would suggest to lookg at 
`wait_event_interruptible_lock_irq` function, that allows to pass a 
locked spinlock as parameter. That would be safer and prevent this 
lock/unlock dance.
When using this API difference is, first we take spin_lock_bh() and then 
wait API is using spin_unlock_irq()/spin_lock_irq(). I am getting
"BUG: scheduling while atomic" when i use this way. When i changed 
spin_lock_bh to spin_lock_irq then we got rid of the kernel BUG.


Thanks,
Hemant

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v9 3/4] docs: Add documentation for userspace client interface

2020-10-26 Thread Hemant Kumar

Hi Jakub,

On 10/26/20 3:56 PM, Jakub Kicinski wrote:

On Mon, 26 Oct 2020 07:38:46 -0600 Jeffrey Hugo wrote:

On 10/25/2020 3:46 PM, Jakub Kicinski wrote:

On Fri, 23 Oct 2020 16:17:54 -0700 Hemant Kumar wrote:

+UCI driver enables userspace clients to communicate to external MHI devices
+like modem and WLAN. UCI driver probe creates standard character device file
+nodes for userspace clients to perform open, read, write, poll and release file
+operations.


What's the user space that talks to this?


Multiple.

Each channel has a different purpose.  There it is expected that a
different userspace application would be using it.

Hemant implemented the loopback channel, which is a simple channel that
just sends you back anything you send it.  Typically this is consumed by
a test application.

Diag is a typical channel to be consumed by userspace.  This is consumed
by various applications that talk to the remote device for diagnostic
information (logs and such).

Sahara is another common channel that is usually used for the multistage
firmware loading process.


Thanks for the info, are there any open source tests based on the
loopback channel (perhaps even in tree?)

Since that's the only channel enabled in this set its the only one
we can comment on.

i am not aware of any open source tests based on loopback channel. My 
testing includes multiple sessions of echo, cat etc using adb to confirm 
what is sent is received back. Loic is using UCI driver for his use case 
too.


Loic, in case you have any use case (which is part of open source) which 
can use UCI driver, pls share that info ?


I think as soon as UCI becomes part of the tree, more and more channels 
would get added to the driver having open source code for that from 
other folks in community (Loic would be one of them i guess).


Thanks,
Hemant
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v9 3/4] docs: Add documentation for userspace client interface

2020-10-26 Thread Hemant Kumar




On 10/26/20 6:56 AM, Jeffrey Hugo wrote:

On 10/26/2020 7:46 AM, Dan Williams wrote:

On Mon, 2020-10-26 at 07:38 -0600, Jeffrey Hugo wrote:

On 10/25/2020 3:46 PM, Jakub Kicinski wrote:

On Fri, 23 Oct 2020 16:17:54 -0700 Hemant Kumar wrote:

+UCI driver enables userspace clients to communicate to external
MHI devices
+like modem and WLAN. UCI driver probe creates standard character
device file
+nodes for userspace clients to perform open, read, write, poll
and release file
+operations.


What's the user space that talks to this?



Multiple.

Each channel has a different purpose.  There it is expected that a
different userspace application would be using it.

Hemant implemented the loopback channel, which is a simple channel
that
just sends you back anything you send it.  Typically this is consumed
by
a test application.

Diag is a typical channel to be consumed by userspace.  This is
consumed
by various applications that talk to the remote device for
diagnostic
information (logs and such).


QMI too?
Dan


Interesting question.  My product doesn't use QMI.  I would expect that 
all QMI runs through Router these days, but I am seeing some QMI 
channels in the downstream source.


Hemant, Do you know what is the usecase for the QMI0/QMI1 channels?

QMI0/QMI1 is used to send QMI message (control path) to bring the qmi 
rmnet data call.


Thanks,
Hemant
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


[PATCH v1 1/2] bus: mhi: core: Count number of HW channels supported by controller

2020-10-23 Thread Hemant Kumar
Device provides the total number of HW channels it supports using MHI
configuration register. Host supported HW channels shall not exceed
that value. In order to make this check, a counter is needed to store
total number of HW channels required by host.

Signed-off-by: Hemant Kumar 
---
 drivers/bus/mhi/core/init.c | 2 ++
 drivers/bus/mhi/core/internal.h | 1 +
 include/linux/mhi.h | 1 +
 3 files changed, 4 insertions(+)

diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
index 0ffdebd..70fd6af 100644
--- a/drivers/bus/mhi/core/init.c
+++ b/drivers/bus/mhi/core/init.c
@@ -725,6 +725,8 @@ static int parse_ch_cfg(struct mhi_controller *mhi_cntrl,
mhi_chan = _cntrl->mhi_chan[chan];
mhi_chan->name = ch_cfg->name;
mhi_chan->chan = chan;
+   if (chan >= MHI_HW_CHAN_START_IDX)
+   mhi_cntrl->hw_chan++;
 
mhi_chan->tre_ring.elements = ch_cfg->num_elements;
if (!mhi_chan->tre_ring.elements)
diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
index 7989269..3d8e480 100644
--- a/drivers/bus/mhi/core/internal.h
+++ b/drivers/bus/mhi/core/internal.h
@@ -454,6 +454,7 @@ enum mhi_pm_state {
 #define PRIMARY_CMD_RING   0
 #define MHI_DEV_WAKE_DB127
 #define MHI_MAX_MTU0x
+#define MHI_HW_CHAN_START_IDX  100
 #define MHI_RANDOM_U32_NONZERO(bmsk)   (prandom_u32_max(bmsk) + 1)
 
 enum mhi_er_type {
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index d4841e5..ea441d2 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -389,6 +389,7 @@ struct mhi_controller {
struct list_head lpm_chans;
int *irq;
u32 max_chan;
+   u32 hw_chan;
u32 total_ev_rings;
u32 hw_ev_rings;
u32 sw_ev_rings;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v1 2/2] bus: mhi: core: Check for device supported event rings and channels

2020-10-23 Thread Hemant Kumar
It is possible that the device does not support the number of event
rings and channels that the controller would like to use. Read the
MHICFG to determine device-side support and if the controller requests
more than the device supports, bailout without configuring device MMIO
registers.

Signed-off-by: Hemant Kumar 
---
 drivers/bus/mhi/core/init.c | 31 +++
 drivers/bus/mhi/core/internal.h |  4 
 2 files changed, 35 insertions(+)

diff --git a/drivers/bus/mhi/core/init.c b/drivers/bus/mhi/core/init.c
index 70fd6af..35a6b1d 100644
--- a/drivers/bus/mhi/core/init.c
+++ b/drivers/bus/mhi/core/init.c
@@ -488,6 +488,37 @@ int mhi_init_mmio(struct mhi_controller *mhi_cntrl)
{ 0, 0, 0 }
};
 
+   /* range check b/w host and device supported ev rings and channels */
+   ret = mhi_read_reg(mhi_cntrl, base, MHICFG, );
+   if (ret) {
+   dev_err(dev, "Unable to read MHICFG register\n");
+   return -EIO;
+   }
+
+   if (MHICFG_NHWER(val) < mhi_cntrl->hw_ev_rings) {
+   dev_err(dev, "#HWEV ring: host requires %d dev supports %d\n",
+   mhi_cntrl->hw_ev_rings, MHICFG_NHWER(val));
+   return -EIO;
+   }
+
+   if (MHICFG_NER(val) < mhi_cntrl->total_ev_rings) {
+   dev_err(dev, "#EV ring: host requires %d dev supports %d\n",
+   mhi_cntrl->total_ev_rings, MHICFG_NER(val));
+   return -EIO;
+   }
+
+   if (MHICFG_NHWCH(val) < mhi_cntrl->hw_chan) {
+   dev_err(dev, "#HWCH: host requires %d dev supports %d\n",
+   mhi_cntrl->hw_chan, MHICFG_NHWCH(val));
+   return -EIO;
+   }
+
+   if (MHICFG_NCH(val) < mhi_cntrl->max_chan) {
+   dev_err(dev, "#CH: host requires %d dev supports %d\n",
+   mhi_cntrl->max_chan, MHICFG_NCH(val));
+   return -EIO;
+   }
+
dev_dbg(dev, "Initializing MHI registers\n");
 
/* Read channel db offset */
diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
index 3d8e480..9cbfa71 100644
--- a/drivers/bus/mhi/core/internal.h
+++ b/drivers/bus/mhi/core/internal.h
@@ -28,6 +28,10 @@ extern struct bus_type mhi_bus_type;
 #define MHICFG_NHWCH_SHIFT (8)
 #define MHICFG_NCH_MASK (0xFF)
 #define MHICFG_NCH_SHIFT (0)
+#define MHICFG_NHWER(n) (((n) & MHICFG_NHWER_MASK) >> MHICFG_NHWER_SHIFT)
+#define MHICFG_NER(n) (((n) & MHICFG_NER_MASK) >> MHICFG_NER_SHIFT)
+#define MHICFG_NHWCH(n) (((n) & MHICFG_NHWCH_MASK) >> MHICFG_NHWCH_SHIFT)
+#define MHICFG_NCH(n) (((n) & MHICFG_NCH_MASK) >> MHICFG_NCH_SHIFT)
 
 #define CHDBOFF (0x18)
 #define CHDBOFF_CHDBOFF_MASK (0x)
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v1 0/2] Check for device supported event rings and channels

2020-10-23 Thread Hemant Kumar
This change is introduced to make sure device supported hardware event ring,
hardware channels, total number of event rings and total number of channels
match with MHI host controller. In case of a mismatch, driver bails out and
does not move MHI device to M0 from Ready state.

Hemant Kumar (2):
  bus: mhi: core: Count number of HW channels supported by controller
  bus: mhi: core: Check for device supported event rings and channels

 drivers/bus/mhi/core/init.c | 33 +
 drivers/bus/mhi/core/internal.h |  5 +
 include/linux/mhi.h |  1 +
 3 files changed, 39 insertions(+)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v9 1/4] bus: mhi: core: Add helper API to return number of free TREs

2020-10-23 Thread Hemant Kumar
Introduce mhi_get_free_desc_count() API to return number
of TREs available to queue buffer. MHI clients can use this
API to know before hand if ring is full without calling queue
API.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
Reviewed-by: Manivannan Sadhasivam 
---
 drivers/bus/mhi/core/main.c | 12 
 include/linux/mhi.h |  9 +
 2 files changed, 21 insertions(+)

diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index 2cff5dd..3950792 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -258,6 +258,18 @@ int mhi_destroy_device(struct device *dev, void *data)
return 0;
 }
 
+int mhi_get_free_desc_count(struct mhi_device *mhi_dev,
+   enum dma_data_direction dir)
+{
+   struct mhi_controller *mhi_cntrl = mhi_dev->mhi_cntrl;
+   struct mhi_chan *mhi_chan = (dir == DMA_TO_DEVICE) ?
+   mhi_dev->ul_chan : mhi_dev->dl_chan;
+   struct mhi_ring *tre_ring = _chan->tre_ring;
+
+   return get_nr_avail_ring_elements(mhi_cntrl, tre_ring);
+}
+EXPORT_SYMBOL_GPL(mhi_get_free_desc_count);
+
 void mhi_notify(struct mhi_device *mhi_dev, enum mhi_callback cb_reason)
 {
struct mhi_driver *mhi_drv;
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index d4841e5..7829b1d 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -597,6 +597,15 @@ void mhi_set_mhi_state(struct mhi_controller *mhi_cntrl,
 void mhi_notify(struct mhi_device *mhi_dev, enum mhi_callback cb_reason);
 
 /**
+ * mhi_get_free_desc_count - Get transfer ring length
+ * Get # of TD available to queue buffers
+ * @mhi_dev: Device associated with the channels
+ * @dir: Direction of the channel
+ */
+int mhi_get_free_desc_count(struct mhi_device *mhi_dev,
+   enum dma_data_direction dir);
+
+/**
  * mhi_prepare_for_power_up - Do pre-initialization before power up.
  *This is optional, call this before power up if
  *the controller does not want bus framework to
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v9 2/4] bus: mhi: core: Move MHI_MAX_MTU to external header file

2020-10-23 Thread Hemant Kumar
Currently this macro is defined in internal MHI header as
a TRE length mask. Moving it to external header allows MHI
client drivers to set this upper bound for the transmit
buffer size.

Signed-off-by: Hemant Kumar 
Reviewed-by: Jeffrey Hugo 
Reviewed-by: Manivannan Sadhasivam 
---
 drivers/bus/mhi/core/internal.h | 1 -
 include/linux/mhi.h | 3 +++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
index 7989269..4abf0cf 100644
--- a/drivers/bus/mhi/core/internal.h
+++ b/drivers/bus/mhi/core/internal.h
@@ -453,7 +453,6 @@ enum mhi_pm_state {
 #define CMD_EL_PER_RING128
 #define PRIMARY_CMD_RING   0
 #define MHI_DEV_WAKE_DB127
-#define MHI_MAX_MTU0x
 #define MHI_RANDOM_U32_NONZERO(bmsk)   (prandom_u32_max(bmsk) + 1)
 
 enum mhi_er_type {
diff --git a/include/linux/mhi.h b/include/linux/mhi.h
index 7829b1d..6e1122c 100644
--- a/include/linux/mhi.h
+++ b/include/linux/mhi.h
@@ -15,6 +15,9 @@
 #include 
 #include 
 
+/* MHI client drivers to set this upper bound for tx buffer */
+#define MHI_MAX_MTU 0x
+
 #define MHI_MAX_OEM_PK_HASH_SEGMENTS 16
 
 struct mhi_chan;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v9 4/4] bus: mhi: Add userspace client interface driver

2020-10-23 Thread Hemant Kumar
This MHI client driver allows userspace clients to transfer
raw data between MHI device and host using standard file operations.
Driver instantiates UCI device object which is associated to device
file node. UCI device object instantiates UCI channel object when device
file node is opened. UCI channel object is used to manage MHI channels
by calling MHI core APIs for read and write operations. MHI channels
are started as part of device open(). MHI channels remain in start
state until last release() is called on UCI device file node. Device
file node is created with format

/dev/mhi__

Currently it supports LOOPBACK channel.

Signed-off-by: Hemant Kumar 
---
 drivers/bus/mhi/Kconfig  |  13 +
 drivers/bus/mhi/Makefile |   4 +
 drivers/bus/mhi/uci.c| 658 +++
 3 files changed, 675 insertions(+)
 create mode 100644 drivers/bus/mhi/uci.c

diff --git a/drivers/bus/mhi/Kconfig b/drivers/bus/mhi/Kconfig
index e841c10..476cc55 100644
--- a/drivers/bus/mhi/Kconfig
+++ b/drivers/bus/mhi/Kconfig
@@ -20,3 +20,16 @@ config MHI_BUS_DEBUG
  Enable debugfs support for use with the MHI transport. Allows
  reading and/or modifying some values within the MHI controller
  for debug and test purposes.
+
+config MHI_UCI
+   tristate "MHI UCI"
+   depends on MHI_BUS
+   help
+ MHI based Userspace Client Interface (UCI) driver is used for
+ transferring raw data between host and device using standard file
+ operations from userspace. Open, read, write, and close operations
+ are supported by this driver. Please check mhi_uci_match_table for
+ all supported channels that are exposed to userspace.
+
+ To compile this driver as a module, choose M here: the module will be
+ called mhi_uci.
diff --git a/drivers/bus/mhi/Makefile b/drivers/bus/mhi/Makefile
index 19e6443..80feefb 100644
--- a/drivers/bus/mhi/Makefile
+++ b/drivers/bus/mhi/Makefile
@@ -1,2 +1,6 @@
 # core layer
 obj-y += core/
+
+# MHI client
+mhi_uci-y := uci.o
+obj-$(CONFIG_MHI_UCI) += mhi_uci.o
diff --git a/drivers/bus/mhi/uci.c b/drivers/bus/mhi/uci.c
new file mode 100644
index 000..6c64356
--- /dev/null
+++ b/drivers/bus/mhi/uci.c
@@ -0,0 +1,658 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2018-2020, The Linux Foundation. All rights reserved.*/
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DEVICE_NAME "mhi"
+#define MHI_UCI_DRIVER_NAME "mhi_uci"
+#define MAX_UCI_MINORS 128
+
+static DEFINE_IDR(uci_idr);
+static DEFINE_MUTEX(uci_drv_mutex);
+static struct class *uci_dev_class;
+static int uci_dev_major;
+
+/**
+ * struct uci_chan - MHI channel for a UCI device
+ * @udev: associated UCI device object
+ * @ul_wq: wait queue for writer
+ * @write_lock: mutex write lock for ul channel
+ * @dl_wq: wait queue for reader
+ * @read_lock: mutex read lock for dl channel
+ * @dl_pending_lock: spin lock for dl_pending list
+ * @dl_pending: list of dl buffers userspace is waiting to read
+ * @cur_buf: current buffer userspace is reading
+ * @dl_size: size of the current dl buffer userspace is reading
+ * @ref_count: uci_chan reference count
+ */
+struct uci_chan {
+   struct uci_dev *udev;
+   wait_queue_head_t ul_wq;
+
+   /* ul channel lock to synchronize multiple writes */
+   struct mutex write_lock;
+
+   wait_queue_head_t dl_wq;
+
+   /* dl channel lock to synchronize multiple reads */
+   struct mutex read_lock;
+
+   /*
+* protects pending list in bh context, channel release, read and
+* poll
+*/
+   spinlock_t dl_pending_lock;
+
+   struct list_head dl_pending;
+   struct uci_buf *cur_buf;
+   size_t dl_size;
+   struct kref ref_count;
+};
+
+/**
+ * struct uci_buf - UCI buffer
+ * @data: data buffer
+ * @len: length of data buffer
+ * @node: list node of the UCI buffer
+ */
+struct uci_buf {
+   void *data;
+   size_t len;
+   struct list_head node;
+};
+
+/**
+ * struct uci_dev - MHI UCI device
+ * @minor: UCI device node minor number
+ * @mhi_dev: associated mhi device object
+ * @uchan: UCI uplink and downlink channel object
+ * @mtu: max TRE buffer length
+ * @enabled: Flag to track the state of the UCI device
+ * @lock: mutex lock to manage uchan object
+ * @ref_count: uci_dev reference count
+ */
+struct uci_dev {
+   unsigned int minor;
+   struct mhi_device *mhi_dev;
+   struct uci_chan *uchan;
+   size_t mtu;
+   bool enabled;
+
+   /* synchronize open, release and driver remove */
+   struct mutex lock;
+   struct kref ref_count;
+};
+
+static void mhi_uci_dev_chan_release(struct kref *ref)
+{
+   struct uci_buf *buf_itr, *tmp;
+   struct uci_chan *uchan =
+   container_of(ref, struct uci_chan, ref_count);
+
+   if (uchan->udev->enabled)
+   mhi_unprepare_from_transfer(uchan->

[PATCH v9 3/4] docs: Add documentation for userspace client interface

2020-10-23 Thread Hemant Kumar
MHI userspace client driver is creating device file node
for user application to perform file operations. File
operations are handled by MHI core driver. Currently
Loopback MHI channel is supported by this driver.

Signed-off-by: Hemant Kumar 
---
 Documentation/mhi/index.rst |  1 +
 Documentation/mhi/uci.rst   | 83 +
 2 files changed, 84 insertions(+)
 create mode 100644 Documentation/mhi/uci.rst

diff --git a/Documentation/mhi/index.rst b/Documentation/mhi/index.rst
index 1d8dec3..c75a371 100644
--- a/Documentation/mhi/index.rst
+++ b/Documentation/mhi/index.rst
@@ -9,6 +9,7 @@ MHI
 
mhi
topology
+   uci
 
 .. only::  subproject and html
 
diff --git a/Documentation/mhi/uci.rst b/Documentation/mhi/uci.rst
new file mode 100644
index 000..fe901c4
--- /dev/null
+++ b/Documentation/mhi/uci.rst
@@ -0,0 +1,83 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=
+Userspace Client Interface (UCI)
+=
+
+UCI driver enables userspace clients to communicate to external MHI devices
+like modem and WLAN. UCI driver probe creates standard character device file
+nodes for userspace clients to perform open, read, write, poll and release file
+operations.
+
+Operations
+==
+
+open
+
+
+Instantiates UCI channel object and starts MHI channels to move it to running
+state. Inbound buffers are queued to downlink channel transfer ring. Every
+subsequent open() increments UCI device reference count as well as UCI channel
+reference count.
+
+read
+
+
+When data transfer is completed on downlink channel, TRE buffer is copied to
+pending list. Reader is unblocked and data is copied to userspace buffer. TRE
+buffer is queued back to downlink channel transfer ring.
+
+write
+-
+
+Write buffer is queued to uplink channel transfer ring if ring is not full. 
Upon
+uplink transfer completion buffer is freed.
+
+poll
+
+
+Returns EPOLLIN | EPOLLRDNORM mask if pending list has buffers to be read by
+userspace. Returns EPOLLOUT | EPOLLWRNORM mask if MHI uplink channel transfer
+ring is not empty. Returns EPOLLERR when UCI driver is removed. MHI channels
+move to disabled state upon driver remove.
+
+release
+---
+
+Decrements UCI device reference count and UCI channel reference count upon last
+release(). UCI channel clean up is performed. MHI channel moves to disabled
+state and inbound buffers are freed.
+
+Usage
+=
+
+Device file node is created with format:-
+
+/dev/mhi__
+
+controller_name is the name of underlying bus used to transfer data. mhi_device
+name is the name of the MHI channel being used by MHI client in userspace to
+send or receive data using MHI protocol.
+
+There is a separate character device file node created for each channel
+specified in mhi device id table. MHI channels are statically defined by MHI
+specification. The list of supported channels is in the channel list variable
+of mhi_device_id table in UCI driver.
+
+LOOPBACK Channel
+
+
+Userspace MHI client using LOOPBACK channel opens device file node. As part of
+open operation TREs to transfer ring of LOOPBACK channel 1 gets queued and 
channel
+doorbell is rung. When userspace MHI client performs write operation on device 
node,
+data buffer gets queued as a TRE to transfer ring of LOOPBACK channel 0. MHI 
Core
+driver rings the channel doorbell for MHI device to move data over underlying 
bus.
+When userspace MHI client driver performs read operation, same data gets 
looped back
+to MHI host using LOOPBACK channel 1. LOOPBACK channel is used to verify data 
path
+and data integrity between MHI Host and MHI device.
+
+Other Use Cases
+---
+
+Getting MHI device specific diagnostics information to userspace MHI diag 
client
+using DIAG channel 4 (Host to device) and 5 (Device to Host).
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



[PATCH v9 0/4] userspace MHI client interface driver

2020-10-23 Thread Hemant Kumar
This patch series adds support for UCI driver. UCI driver enables userspace
clients to communicate to external MHI devices like modem and WLAN. UCI driver
probe creates standard character device file nodes for userspace clients to
perform open, read, write, poll and release file operations. These file
operations call MHI core layer APIs to perform data transfer using MHI bus
to communicate with MHI device. Patch is tested using arm64 based platform.

V9:
- Renamed dl_lock to dl_pending _lock and pending list to dl_pending for
  clarity.
- Used read lock to protect cur_buf.
- Change transfer status check logic and only consider 0 and -EOVERFLOW as
  only success.
- Added __int to module init function.
- Print channel name instead of minor number upon successful probe.

V8:
- Fixed kernel test robot compilation error by changing %lu to %zu for
  size_t.
- Replaced uci with UCI in Kconfig, commit text, and comments in driver
  code.
- Fixed minor style related comments.

V7:
- Decoupled uci device and uci channel objects. uci device is
  associated with device file node. uci channel is associated
  with MHI channels. uci device refers to uci channel to perform
  MHI channel operations for device file operations like read()
  and write(). uci device increments its reference count for
  every open(). uci device calls mhi_uci_dev_start_chan() to start
  the MHI channel. uci channel object is tracking number of times
  MHI channel is referred. This allows to keep the MHI channel in
  start state until last release() is called. After that uci channel
  reference count goes to 0 and uci channel clean up is performed
  which stops the MHI channel. After the last call to release() if
  driver is removed uci reference count becomes 0 and uci object is
  cleaned up.
- Use separate uci channel read and write lock to fine grain locking
  between reader and writer.
- Use uci device lock to synchronize open, release and driver remove.
- Optimize for downlink only or uplink only UCI device.

V6:
- Moved uci.c to mhi directory.
- Updated Kconfig to add module information.
- Updated Makefile to rename uci object file name as mhi_uci
- Removed kref for open count

V5:
- Removed mhi_uci_drv structure.
- Used idr instead of creating global list of uci devices.
- Used kref instead of local ref counting for uci device and
  open count.
- Removed unlikely macro.

V4:
- Fix locking to protect proper struct members.
- Updated documentation describing uci client driver use cases.
- Fixed uci ref counting in mhi_uci_open for error case.
- Addressed style related review comments.

V3: Added documentation for MHI UCI driver.

V2: Added mutex lock to prevent multiple readers to access same
mhi buffer which can result into use after free.

Hemant Kumar (4):
  bus: mhi: core: Add helper API to return number of free TREs
  bus: mhi: core: Move MHI_MAX_MTU to external header file
  docs: Add documentation for userspace client interface
  bus: mhi: Add userspace client interface driver

 Documentation/mhi/index.rst |   1 +
 Documentation/mhi/uci.rst   |  83 +
 drivers/bus/mhi/Kconfig |  13 +
 drivers/bus/mhi/Makefile|   4 +
 drivers/bus/mhi/core/internal.h |   1 -
 drivers/bus/mhi/core/main.c |  12 +
 drivers/bus/mhi/uci.c   | 658 
 include/linux/mhi.h |  12 +
 8 files changed, 783 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/mhi/uci.rst
 create mode 100644 drivers/bus/mhi/uci.c

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project



Re: [PATCH v8 0/4] userspace MHI client interface driver

2020-10-23 Thread Hemant Kumar

Hi Jakub,

On 10/23/20 9:37 AM, Jakub Kicinski wrote:

On Thu, 22 Oct 2020 01:22:34 -0700 Hemant Kumar wrote:

This patch series adds support for UCI driver. UCI driver enables userspace
clients to communicate to external MHI devices like modem and WLAN. UCI driver
probe creates standard character device file nodes for userspace clients to
perform open, read, write, poll and release file operations. These file
operations call MHI core layer APIs to perform data transfer using MHI bus
to communicate with MHI device. Patch is tested using arm64 based platform.


Until you CC netdev on this (as suggested [1]), here's my:

Nacked-by: Jakub Kicinski 

[1]
https://lore.kernel.org/netdev/20201016183759.7fa7c...@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com/



UCI driver patch series includes following patches

1)   bus: mhi: core: Add helper API to return number of free TREs
2)   bus: mhi: core: Move MHI_MAX_MTU to external header file
3)   docs: Add documentation for userspace client interface
4)   bus: mhi: Add userspace client interface driver

mhi net driver can use  #1 and #2 and these two are part of MHI core 
driver change. These are helper API/macro which can be used by any mhi 
clients like UCI or mhi net dev.


#3 and #4 are MHI UCI character driver completely unrelated to netdev.

How about splitting the patch series between #1,#2 and #3, #4 and add 
netdev to #1, #2?


#1,#2 were already reviewed by two folks in community so it would be 
much easier for netdev folks to refer that in context of mhi net device 
driver.


Please let me know what do you think about this idea.

Thanks,
Hemant

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


Re: [PATCH v8 4/4] bus: mhi: Add userspace client interface driver

2020-10-22 Thread Hemant Kumar

Hi Loic,

On 10/22/20 3:20 AM, Loic Poulain wrote:

Hi Hemant,

A few comments inline.

On Thu, 22 Oct 2020 at 10:22, Hemant Kumar  wrote:


This MHI client driver allows userspace clients to transfer
raw data between MHI device and host using standard file operations.
Driver instantiates UCI device object which is associated to device
file node. UCI device object instantiates UCI channel object when device
file node is opened. UCI channel object is used to manage MHI channels
by calling MHI core APIs for read and write operations. MHI channels
are started as part of device open(). MHI channels remain in start
state until last release() is called on UCI device file node. Device
file node is created with format


[..]

+
+/**
+ * struct uci_chan - MHI channel for a UCI device
+ * @udev: associated UCI device object
+ * @ul_wq: wait queue for writer
+ * @write_lock: mutex write lock for ul channel
+ * @dl_wq: wait queue for reader
+ * @read_lock: mutex read lock for dl channel
+ * @dl_lock: spin lock
+ * @pending: list of dl buffers userspace is waiting to read
+ * @cur_buf: current buffer userspace is reading
+ * @dl_size: size of the current dl buffer userspace is reading
+ * @ref_count: uci_chan reference count
+ */
+struct uci_chan {
+   struct uci_dev *udev;
+   wait_queue_head_t ul_wq;
+
+   /* ul channel lock to synchronize multiple writes */
+   struct mutex write_lock;
+
+   wait_queue_head_t dl_wq;
+
+   /* dl channel lock to synchronize multiple reads */
+   struct mutex read_lock;
+
+   /*
+* protects pending and cur_buf members in bh context, channel release,
+* read and poll
+*/
+   spinlock_t dl_lock;


Maybe I'm wrong, but I think it would be clearer and simpler for
dl_lock to be only a lock for the pending RX list (e.g. pending_lock),
for protecting against concurrent access in chardev read ops
(consumer) and MHI download callback (producer). cur_buf is the
currently read buffer, and so could be simply protected by the
read_lock mutex (never accessed from bh/irq callback context).
You have a good point. I can protect pending list related operations 
using spin lock and call it pending_lock which would be used in dl_xfer 
call back, channel release, read and poll. Use read lock for cur_buf in 
read().



+
+   struct list_head pending;
+   struct uci_buf *cur_buf;
+   size_t dl_size;
+   struct kref ref_count;
+};
+
+/**

[..]

+static void mhi_dl_xfer_cb(struct mhi_device *mhi_dev,
+  struct mhi_result *mhi_result)
+{
+   struct uci_dev *udev = dev_get_drvdata(_dev->dev);
+   struct uci_chan *uchan = udev->uchan;
+   struct device *dev = _dev->dev;
+   struct uci_buf *ubuf;
+   size_t dl_buf_size = udev->mtu - sizeof(*ubuf);
+
+   dev_dbg(dev, "status: %d receive_len: %zu\n",
+   mhi_result->transaction_status, mhi_result->bytes_xferd);
+
+   if (mhi_result->transaction_status == -ENOTCONN) {
+   kfree(mhi_result->buf_addr);
+   return;
+   }


It would be more robust to test only transaction_status values that
allow you to consider the buffer as valid, AFAIU 0 and -EOVERFLOW.
That would prevent breaking that code if new transaction_status errors
are returned in the futur (e.g. -EIO...).

I agree, will use this instead
if (mhi_result->transaction_status &&
mhi_result->transaction_status != -EOVERFLOW)




+
+   ubuf = mhi_result->buf_addr + dl_buf_size;
+   ubuf->data = mhi_result->buf_addr;
+   ubuf->len = mhi_result->bytes_xferd;
+   spin_lock_bh(>dl_lock);
+   list_add_tail(>node, >pending);
+   spin_unlock_bh(>dl_lock);
+
+   wake_up(>dl_wq);
+}
+


Thanks,
Hemant
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project


  1   2   3   4   5   6   7   >