Date:   Thu, 27 Sep 2018 10:38:27 -0600
From:   Logan Gunthorpe <logang@...tatee.com>
To:     Wesley.Sheng@...rochip.com, kurt.schwemmer@...rosemi.com,
        bhelgaas@...gle.com, corbet@....net
Cc:     linux-pci@...r.kernel.org, linux-doc@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-ntb@...glegroups.com
Subject: Re: [PATCH v2] NTB: switchtec_ntb: Update switchtec documentation
 with pre-requisites for NTB



On 2018-09-27 12:36 a.m., Wesley.Sheng@...rochip.com wrote:
> Update switchtec documentation
> 1.add pre-requisites for NTB driver
> 2.misc updates
>  change NTB hardware driver name to ntb_hw_switchtec
>  update the description of 2 partitions limit
>  add reference to Linux Documentation/ntb.txt
>  add use of term 'Command ID'
>  add use of term 'Command Return Value'
>  add a missing ')'

You need a better commit message. It needs to describe, in plain
English, why the maintainers should merge this, not itemize the
individual changes (that's what the patch body tells us).

>  * A write must consist of at least 4 bytes and no more than 1028 bytes.
> -  The first four bytes will be interpreted as the command to run and
> -  the remainder will be used as the input data. A write will send the
> +  The first 4 bytes will be interpreted as the Command ID and the  
> + remainder will be used as the input data. A write will send the
>    command to the firmware to begin processing.

You still have some broken white space in this patch. Notice the spacing
before "remainder" compared to the previous line.

>  * Each write must be followed by exactly one read. Any double write will
>  @@ -45,9 +45,9 @@ The char device has the following semantics:
>    produce an error.
>  
>  * A read will block until the firmware completes the command and return
> -  the four bytes of status plus up to 1024 bytes of output data. (The
> -  length will be specified by the size parameter of the read call --
> -  reading less than 4 bytes will produce an error.
> +  the 4-byte Command Return Value plus up to 1024 bytes of output  
> + data. (The length will be specified by the size parameter of the read  
> + call -- reading less than 4 bytes will produce an error.)

Here too.

>  * The poll call will also be supported for userspace applications that
>    need to do other things while waiting for the command to complete.
> @@ -83,10 +83,20 @@ The following IOCTLs are also supported by the device:
>  Non-Transparent Bridge (NTB) Driver
>  ===================================
>  
> -An NTB driver is provided for the switchtec hardware in switchtec_ntb.
> -Currently, it only supports switches configured with exactly 2 
> -partitions. It also requires the following configuration settings:
> +An NTB hardware driver is provided for the Switchtec hardware in 
> +ntb_hw_switchtec. Currently, it only supports switches configured with 
> +exactly 2 NT partitions and zero or more non-NT partitions. It also 
> +requires the following configuration settings:
>  
> -* Both partitions must be able to access each other's GAS spaces.
> +* Both NT partitions must be able to access each other's GAS spaces.
>    Thus, the bits in the GAS Access Vector under Management Settings
>    must be set to support this.
> +* Kernel configuration MUST include support for NTB (CONFIG_NTB needs
> +  to be set)
> +
> +NT EP BAR 2 will be dynamically configured as a Direct Window, and the 
> +configuration file does not need to configure it explicitly.
> +
> +Please refer to Documentation/ntb.txt in Linux source tree for an 
> +overall understanding of the Linux NTB stack. ntb_hw_switchtec works as 
> +an NTB Hardware Driver in this stack.
> 

Powered by blists - more mailing lists