Re: [PATCH v8 01/13] Documentation: Add SLIMbus summary

["Jonathan Neuschäfer" <j.neuschaefer@gmx.net>]

[-- Attachment #1: Type: text/plain, Size: 5663 bytes --]

Hi, some small nits below.

On Thu, Nov 30, 2017 at 05:41:48PM +0000, srinivas.kandagatla@linaro.org wrote:
> From: Sagar Dharia <sdharia@codeaurora.org>
> 
> SLIMbus (Serial Low Power Interchip Media Bus) is a specification
> developed by MIPI (Mobile Industry Processor Interface) alliance.
> SLIMbus is a 2-wire implementation, which is used to communicate with
> peripheral components like audio-codec.
> 
> The summary of SLIMbus and API is documented in the 'summary' file.
> 
> Signed-off-by: Sagar Dharia <sdharia@codeaurora.org>
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
> ---
>  Documentation/driver-api/slimbus/index.rst   |  15 ++++
>  Documentation/driver-api/slimbus/summary.rst | 108 +++++++++++++++++++++++++++
>  2 files changed, 123 insertions(+)
>  create mode 100644 Documentation/driver-api/slimbus/index.rst
>  create mode 100644 Documentation/driver-api/slimbus/summary.rst

It would probably make sense to Cc the documentation maintainers/mailing
list on this patch (AFAICS, you didn't do that).

When do you plan to add slimbus to Documentation/driver-api/index.rst?

> diff --git a/Documentation/driver-api/slimbus/index.rst b/Documentation/driver-api/slimbus/index.rst
> new file mode 100644
> index 000000000000..586f979659e6
> --- /dev/null
> +++ b/Documentation/driver-api/slimbus/index.rst
> @@ -0,0 +1,15 @@
> +=====================
> +SLIMbus Documentation
> +=====================
> +
> +.. toctree::
> +   :maxdepth: 1
> +
> +   summary
> +
> +.. only::  subproject
> +
> +   Indices
> +   =======
> +
> +   * :ref:`genindex`
> diff --git a/Documentation/driver-api/slimbus/summary.rst b/Documentation/driver-api/slimbus/summary.rst
> new file mode 100644
> index 000000000000..ba165611725a
> --- /dev/null
> +++ b/Documentation/driver-api/slimbus/summary.rst
> @@ -0,0 +1,108 @@
> +============================
> +Linux kernel SLIMbus support
> +============================
> +
> +Overview
> +========
> +
> +What is SLIMbus?
> +----------------
> +SLIMbus (Serial Low Power Interchip Media Bus) is a specification developed by
> +MIPI (Mobile Industry Processor Interface) alliance. The bus uses master/slave
> +configuration, and is a 2-wire multi-drop implementation (clock, and data).
> +
> +Currently, SLIMbus is used to interface between application processors of SoCs
> +(System-on-Chip) and peripheral components (typically codec).SLIMbus uses
> +Time-Division-Multiplexing to accommodate multiple data channels, and
> +a control channel.
> +
> +The control channel is used for various control functions such as bus
> +management, configuration and status updates.These messages can be unicast (e.g.
                                               ^^ space after period, please

> +reading/writing device specific values), or multicast (e.g. data channel
> +reconfiguration sequence is a broadcast message announced to all devices)
> +
> +A data channel is used for data-transfer between 2 SLIMbus devices. Data
> +channel uses dedicated ports on the device.
> +
[...]
> +
> +Device notifications to the driver:
> +-----------------------------------
> +Since SLIMbus devices have mechanisms for reporting their presence, the
> +framework allows drivers to bind when corresponding devices report their
> +presence on the bus.
> +However, it is possible that the driver needs to be probed
> +first so that it can enable corresponding SLIMbus device (e.g. power it up and/or
> +take it out of reset). To support that behavior, the framework allows drivers
> +to probe first as well  (e.g. using standard DeviceTree compatbility field).

Typo: s/compatbility/compatibility/

> +This creates the necessity for the driver to know when the device is functional
> +(i.e. reported present). device_up callback is used for that reason when the
> +device reports present and is assigned a logical address by the controller.
> +
> +Similarly, SLIMbus devices 'report absent' when they go down. A 'device_down'
> +callback notifies the driver when the device reports absent and its logical
> +address assignment is invalidated by the controller.
> +
> +Another notification "boot_device" is used to notify the slim_driver when
> +controller resets the bus. This notification allows the driver to take necessary
> +steps to boot the device so that it's functional after the bus has been reset.
> +
> +Clock-pause:
> +------------
> +SLIMbus mandates that a reconfiguration sequence (known as clock-pause) be
> +broadcast to all active devices on the bus before the bus can enter low-power
> +mode. Controller uses this sequence when it decides to enter low-power mode so
> +that corresponding clocks and/or power-rails can be turned off to save power.
> +Clock-pause is exited by waking up framer device (if controller driver initiates
> +exiting low power mode), or by toggling the data line (if a slave device wants
> +to initiate it).
> +
> +Messaging APIs:
> +---------------
> +The framework supports APIs to exchange control-information with a SLIMbus
> +device. APIs can be synchronous or asynchronous.
> +From controller's perspective, multiple buffers can be queued to/from
> +hardware for sending/receiving data using slim_ctrl_buf circular buffer.
> +The header file <linux/slimbus.h> has more documentation about messaging APIs.

Once the kerneldoc documentation (i.e. the /** ... */ comments in the
source) is included somewhere, I think it would make sense to make
slim_ctrl_buf a clickable link to the struct's documentation.


Thanks,
Jonathan Neuschäfer

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]