LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Rob Herring <robh@kernel.org>
To: Sujeev Dias <sdias@codeaurora.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Arnd Bergmann <arnd@arndb.de>,
	linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	devicetree@vger.kernel.org, Tony Truong <truong@codeaurora.org>,
	Siddartha Mohanadoss <smohanad@codeaurora.org>
Subject: Re: [PATCH v2 1/7] mhi_bus: core: initial checkin for modem host interface bus driver
Date: Wed, 11 Jul 2018 13:30:11 -0600
Message-ID: <20180711193011.GA8659@rob-hp-laptop> (raw)
In-Reply-To: <1531166894-30984-2-git-send-email-sdias@codeaurora.org>

On Mon, Jul 09, 2018 at 01:08:08PM -0700, Sujeev Dias wrote:
> This is the initial skeleton driver for mhi bus stack. MHI Host
> Interface is a communication protocol to be used by the host to
> control and communcate with modem over a high speed peripheral bus.
> This module will allow host to communicate with external devices that
> support MHI protocol.
> 
> Signed-off-by: Sujeev Dias <sdias@codeaurora.org>
> Reviewed-by: Tony Truong <truong@codeaurora.org>
> Signed-off-by: Siddartha Mohanadoss <smohanad@codeaurora.org>
> ---
>  Documentation/00-INDEX                        |   2 +
>  Documentation/devicetree/bindings/bus/mhi.txt | 258 ++++++++++++

As Greg said, separate patch.

>  Documentation/mhi.txt                         | 235 +++++++++++
>  drivers/bus/Kconfig                           |   8 +
>  drivers/bus/Makefile                          |   1 +
>  drivers/bus/mhi/Makefile                      |   6 +
>  drivers/bus/mhi/core/Makefile                 |   1 +
>  drivers/bus/mhi/core/mhi_init.c               | 538 ++++++++++++++++++++++++++
>  drivers/bus/mhi/core/mhi_internal.h           | 238 ++++++++++++
>  drivers/bus/mhi/core/mhi_main.c               | 122 ++++++
>  include/linux/mhi.h                           | 341 ++++++++++++++++
>  include/linux/mod_devicetable.h               |  12 +
>  12 files changed, 1762 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/bus/mhi.txt
>  create mode 100644 Documentation/mhi.txt
>  create mode 100644 drivers/bus/mhi/Makefile
>  create mode 100644 drivers/bus/mhi/core/Makefile
>  create mode 100644 drivers/bus/mhi/core/mhi_init.c
>  create mode 100644 drivers/bus/mhi/core/mhi_internal.h
>  create mode 100644 drivers/bus/mhi/core/mhi_main.c
>  create mode 100644 include/linux/mhi.h

> diff --git a/Documentation/devicetree/bindings/bus/mhi.txt b/Documentation/devicetree/bindings/bus/mhi.txt
> new file mode 100644
> index 0000000..19deb84
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/bus/mhi.txt
> @@ -0,0 +1,258 @@
> +MHI Host Interface
> +
> +MHI used by the host to control and communicate with modem over
> +high speed peripheral bus.
> +
> +==============
> +Node Structure
> +==============
> +
> +Main node properties:
> +
> +- mhi,max-channels

mhi is not a vendor. mhi-max-channels. Same for rest. Or maybe just drop 
'mhi' completely.

> +  Usage: required
> +  Value type: <u32>
> +  Definition: Maximum number of channels supported by this controller
> +
> +- mhi,timeout
> +  Usage: optional
> +  Value type: <u32>
> +  Definition: Maximum timeout in ms wait for state and cmd completion

Needs a unit suffix.

> +
> +- mhi,use-bb
> +  Usage: optional
> +  Value type: <bool>
> +  Definition: Set true, if PCIe controller does not have full access to host
> +	DDR, and we're using a dedicated memory pool like cma, or
> +	carveout pool. Pool must support atomic allocation.

How is this related to PCI?

"atomic allocation" has nothing to do with bindings.

> +
> +- mhi,buffer-len
> +  Usage: optional
> +  Value type: <bool>
> +  Definition: MHI automatically pre-allocate buffers for some channel.
> +	Set the length of buffer size to allocate. If not default
> +	size MHI_MAX_MTU will be used.
> +
> +============================
> +mhi channel node properties:
> +============================
> +
> +- reg
> +  Usage: required
> +  Value type: <u32>
> +  Definition: physical channel number
> +
> +- label
> +  Usage: required
> +  Value type: <string>
> +  Definition: given name for the channel

Why does the user need this?

> +
> +- mhi,num-elements
> +  Usage: optional
> +  Value type: <u32>
> +  Definition: Number of elements transfer ring support
> +
> +- mhi,event-ring
> +  Usage: required
> +  Value type: <u32>
> +  Definition: Event ring index associated with this channel
> +
> +- mhi,chan-dir
> +  Usage: required
> +  Value type: <u32>
> +  Definition: Channel direction as defined by enum dma_data_direction
> +	0 = Bidirectional data transfer
> +	1 = UL data transfer
> +	2 = DL data transfer
> +	3 = No direction, not a regular data transfer channel
> +
> +- mhi,ee
> +  Usage: required
> +  Value type: <u32>
> +  Definition: Channel execution enviornment as defined by enum MHI_EE
> +	1 = Bootloader stage
> +	2 = AMSS mode
> +
> +- mhi,pollcfg
> +  Usage: optional
> +  Value type: <u32>
> +  Definition: MHI poll configuration, valid only when burst mode is enabled
> +	0 = Use default (device specific) polling configuration
> +	For UL channels, value specifies the timer to poll MHI context in
> +	milliseconds.
> +	For DL channels, the threshold to poll the MHI context in multiple of
> +	eight ring element.
> +
> +- mhi,data-type
> +  Usage: required
> +  Value type: <u32>
> +  Definition: Data transfer type accepted as defined by enum MHI_XFER_TYPE
> +	0 = accept cpu address for buffer
> +	1 = accept skb
> +	2 = accept scatterlist
> +	3 = offload channel, does not accept any transfer type
> +
> +- mhi,doorbell-mode
> +  Usage: required
> +  Value type: <u32>
> +  Definition: Channel doorbell mode configuration as defined by enum
> +	MHI_BRSTMODE
> +	2 = burst mode disabled
> +	3 = burst mode enabled
> +
> +- mhi,lpm-notify
> +  Usage: optional
> +  Value type: <bool>
> +  Definition: This channel master require low power mode enter and exit
> +  notifications from mhi bus master.
> +
> +- mhi,offload-chan
> +  Usage: optional
> +  Value type: <bool>
> +  Definition: Client managed channel, MHI host only involved in setting up
> +	the data path, not involved in active data path.
> +
> +- mhi,db-mode-switch
> +  Usage: optional
> +  Value type: <bool>
> +  Definition: Must switch to doorbell mode whenever MHI M0 state transition
> +	happens.
> +
> +- mhi,auto-queue
> +  Usage: optional
> +  Value type: <bool>
> +  Definition: MHI bus driver will pre-allocate buffers for this channel and
> +	queue to hardware. If set, client not allowed to queue buffers. Valid
> +	only for downlink direction.
> +
> +- mhi,auto-start
> +  Usage: optional
> +  Value type: <bool>
> +  Definition: MHI host driver to automatically start channels once mhi device
> +	driver probe is complete. This should be only set true if initial
> +	handshake iniaitead by external modem.

typo

> +
> +==========================
> +mhi event node properties:
> +==========================
> +
> +- mhi,num-elements
> +  Usage: required
> +  Value type: <u32>
> +  Definition: Number of elements event ring support
> +
> +- mhi,intmod
> +  Usage: required
> +  Value type: <u32>
> +  Definition: interrupt moderation time in ms
> +
> +- mhi,msi
> +  Usage: required
> +  Value type: <u32>
> +  Definition: MSI associated with this event ring
> +
> +- mhi,chan
> +  Usage: optional
> +  Value type: <u32>
> +  Definition: Dedicated channel number, if it's a dedicated event ring
> +
> +- mhi,priority
> +  Usage: required
> +  Value type: <u32>
> +  Definition: Event ring priority, set to 1 for now
> +
> +- mhi,brstmode
> +  Usage: required
> +  Value type: <u32>
> +  Definition: Event doorbell mode configuration as defined by
> +	enum MHI_BRSTMODE
> +		2 = burst mode disabled
> +		3 = burst mode enabled
> +
> +- mhi,data-type
> +  Usage: optional
> +  Value type: <u32>
> +  Definition: Type of data this event ring will process as defined
> +	by enum mhi_er_data_type
> +		0 = process data packets (default)
> +		1 = process mhi control packets
> +
> +- mhi,hw-ev
> +  Usage: optional
> +  Value type: <bool>
> +  Definition: Event ring associated with hardware channels
> +
> +- mhi,client-manage
> +  Usage: optional
> +  Value type: <bool>
> +  Definition: Client manages the event ring (use by napi_poll)
> +
> +- mhi,offload
> +  Usage: optional
> +  Value type: <bool>
> +  Definition: Event ring associated with offload channel

This is a lot of properties. I suspect that many of these should be 
implied by the modem device or be driver settings. If the modems are PCI 
devices, then you should be able to determine a lot from the PCI 
VID/PIDs.

I'm not sure that a bus is appropriate here either. This looks like the 
next layer up. How's it different than a programming model for some 
ethernet NIC?

Rob

  parent reply index

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-27  2:23 MHI initial design review Sujeev Dias
2018-04-27  2:23 ` [PATCH v1 1/4] mhi_bus: core: Add support for MHI host interface Sujeev Dias
2018-04-27  7:22   ` Greg Kroah-Hartman
2018-04-28 14:28     ` Sujeev Dias
2018-04-28 15:50       ` Greg Kroah-Hartman
2018-04-27  7:23   ` Greg Kroah-Hartman
2018-04-27 12:18   ` Arnd Bergmann
2018-04-28 16:08     ` Sujeev Dias
2018-04-28  0:28   ` kbuild test robot
2018-04-28  2:52   ` kbuild test robot
2018-05-03 19:21   ` Pavel Machek
2018-05-04  3:05     ` Sujeev Dias
2018-06-22 23:03   ` Randy Dunlap
2018-04-27  2:23 ` [PATCH v1 2/4] mhi_bus: controller: MHI support for QCOM modems Sujeev Dias
2018-04-27 11:32   ` Arnd Bergmann
2018-04-28 15:40     ` Sujeev Dias
2018-04-28  3:05   ` kbuild test robot
2018-04-28  3:12   ` kbuild test robot
2018-04-27  2:23 ` [PATCH v1 3/4] mhi_bus: dev: netdev: add network interface driver Sujeev Dias
2018-04-27 11:19   ` Arnd Bergmann
2018-04-28 15:25     ` Sujeev Dias
2018-04-27  2:23 ` [PATCH v1 4/4] mhi_bus: dev: uci: add user space " Sujeev Dias
2018-04-27 11:36   ` Arnd Bergmann
2018-04-28  1:03   ` kbuild test robot
2018-04-28  5:16   ` [PATCH] mhi_bus: dev: uci: fix semicolon.cocci warnings kbuild test robot
2018-04-28  5:16   ` [PATCH v1 4/4] mhi_bus: dev: uci: add user space interface driver kbuild test robot
2018-07-09 20:08 ` MHI code review Sujeev Dias
2018-07-09 20:08   ` [PATCH v2 1/7] mhi_bus: core: initial checkin for modem host interface bus driver Sujeev Dias
2018-07-09 20:50     ` Greg Kroah-Hartman
2018-07-09 20:52     ` Greg Kroah-Hartman
2018-07-10  6:36     ` Greg Kroah-Hartman
2018-07-11 19:30     ` Rob Herring [this message]
2018-08-09 18:39     ` Randy Dunlap
2018-07-09 20:08   ` [PATCH v2 2/7] mhi_bus: core: add power management support Sujeev Dias
2018-07-09 20:08   ` [PATCH v2 3/7] mhi_bus: core: add support for data transfer Sujeev Dias
2018-07-10  6:29     ` Greg Kroah-Hartman
2018-07-09 20:08   ` [PATCH v2 4/7] mhi_bus: core: add support for handling ioctl cmds Sujeev Dias
2018-07-09 20:08   ` [PATCH v2 5/7] mhi_bus: core: add support to get external modem time Sujeev Dias
2018-07-11 19:32     ` Rob Herring
2018-08-09 20:17     ` Randy Dunlap
2018-07-09 20:08   ` [PATCH v2 6/7] mhi_bus: controller: MHI support for QCOM modems Sujeev Dias
2018-07-11 19:36     ` Rob Herring
2018-07-09 20:08   ` [PATCH v2 7/7] mhi_bus: dev: uci: add user space interface driver Sujeev Dias
2019-04-30 15:10   ` MHI code review Daniele Palmas
2019-06-12 17:54     ` Sujeev Dias
2019-06-12 20:58       ` Daniele Palmas
2019-06-12 18:00     ` Sujeev Dias

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180711193011.GA8659@rob-hp-laptop \
    --to=robh@kernel.org \
    --cc=arnd@arndb.de \
    --cc=devicetree@vger.kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sdias@codeaurora.org \
    --cc=smohanad@codeaurora.org \
    --cc=truong@codeaurora.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox