linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Shuah Khan <skhan@linuxfoundation.org>
To: "Hongren Zheng (Zenithal)" <i@zenithal.me>,
	"Valentina Manea" <valentina.manea.m@gmail.com>,
	"Shuah Khan" <shuah@kernel.org>,
	"Greg Kroah-Hartman" <gregkh@linuxfoundation.org>,
	"Márton Németh" <nm127@freemail.hu>,
	"Jonathan Corbet" <corbet@lwn.net>,
	linux-doc@vger.kernel.org
Cc: Alexandre Demers <alexandre.f.demers@gmail.com>,
	linux-usb@vger.kernel.org, usbip-devel@lists.sourceforge.net,
	Randy Dunlap <rdunlap@infradead.org>,
	Shuah Khan <skhan@linuxfoundation.org>
Subject: Re: [PATCH v5] docs: usbip: Fix major fields and descriptions in protocol
Date: Tue, 16 Mar 2021 09:57:49 -0600	[thread overview]
Message-ID: <98a9c2c3-c041-4823-f37e-cd9a214a34d0@linuxfoundation.org> (raw)
In-Reply-To: <YFAXGBSxaZJ8Dy3/@Sun>

On 3/15/21 8:25 PM, Hongren Zheng (Zenithal) wrote:
> The old document for usbip protocol is misleading and hard to read:
>    * Some fields in header are incorrect
>    * Explanation of some fields are unclear or even wrong
>    * Padding of header (namely all headers have the same length) is
>      not explicitly pointed out, which is crucial for stream protocol
>      like TCP
> 
> Major changes:
>    * Document the correct field as described in the codebase.
>    * Document the padding in usbip headers. This is crucial for TCP
>      stream hence these padding should be explicitly point out.
>      In code these padding are implemented by a union of all headers.
>    * Fix two FIXME related to usbip unlink and Document the behavior
>      of unlink in different situation.
>    * Clarify some field with more accurate explanation, like those
>      fields associated with URB. Some constraints are extracted from
>      code.
>    * Delete specific transfer_flag doc in usbip as it should be
>      documented by the URB part.
>    * Add data captured from wire as example
> 
> Co-developed-by: Alexandre Demers <alexandre.f.demers@gmail.com>
> Reviewed-by: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Hongren Zheng <i@zenithal.me>
> ---
>   Documentation/usb/usbip_protocol.rst | 320 ++++++++++++++-------------
>   1 file changed, 171 insertions(+), 149 deletions(-)
> 
> PATCH v2:
> Some changes suggested by a previous patch in
> https://lore.kernel.org/linux-usb
> /20180128071514.9107-1-alexandre.f.demers@gmail.com/
> is adopted in this patch.
>    * Fix Typo: duplicated 'the' in 'the following 4 field'
>    * Fix incorrect field: in OP_REQ_DEVLIST, the second dev starts with
>      field 'path', not 'busid'
> 
> PATCH v3:
> Suggested by
> https://lore.kernel.org/linux-doc/YE8Oan2BmSuKR4%2Fp@kroah.com/
>    * Remove date and changelog in doc as these are tracked in git history
>    * Remove 'mistake alert' as all data fields are documented properly
>      now. However, docs on possible values for some field shall be added
>      in the future
> 
> PATCH v4:
> Suggested by https://lore.kernel.org/linux-doc
> /40351ed6-2907-3966-e69a-a564173b3682@infradead.org/
>    * Add punctuations for readability
>    * Move patch changelog after the marker line
>    * Remove nickname in signed-off-by line
> 
> PATCH v5:
>    * Instead of co-developed-by, use reviewed-by
>      for Randy Dunlap
> 

Hi Hongren Zheng,

Thanks for the patch. The document updates are very much needed.
I will review and give you comments.

Please wait for a couple of days before sending another version.
Makes it easier for you an your reviewers.

I started reviewing patch v3 and I will switch to v5 now
and send you comments.

thanks,
-- Shuah

  reply	other threads:[~2021-03-16 15:58 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-15  1:57 [PATCH] docs: usbip: Fix major fields and descriptions in protocol Hongren Zheng (Zenithal)
2021-03-15  6:18 ` [PATCH v2] " Hongren Zheng (Zenithal)
2021-03-15  7:36   ` Greg Kroah-Hartman
2021-03-15  8:40     ` [PATCH v3] " Hongren Zheng (Zenithal)
2021-03-15 18:25       ` Randy Dunlap
2021-03-15 21:15       ` [PATCH v4] " Hongren Zheng (Zenithal)
2021-03-16  1:54         ` Randy Dunlap
2021-03-16  2:25         ` [PATCH v5] " Hongren Zheng (Zenithal)
2021-03-16 15:57           ` Shuah Khan [this message]
2021-03-29 20:06           ` Shuah Khan
2021-03-30 12:35             ` Hongren Zheng (Zenithal)
2021-03-30 14:57               ` Shuah Khan
2021-03-30 17:00           ` [PATCH v6] " Hongren Zheng (Zenithal)
2021-04-08 15:18             ` Shuah Khan

Reply instructions:

You may reply publicly 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=98a9c2c3-c041-4823-f37e-cd9a214a34d0@linuxfoundation.org \
    --to=skhan@linuxfoundation.org \
    --cc=alexandre.f.demers@gmail.com \
    --cc=corbet@lwn.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=i@zenithal.me \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=nm127@freemail.hu \
    --cc=rdunlap@infradead.org \
    --cc=shuah@kernel.org \
    --cc=usbip-devel@lists.sourceforge.net \
    --cc=valentina.manea.m@gmail.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).