iochannel.h is using Linux/dma-direction

[wordforthis]

See KanBoard-1562.

The IO channel is a shared resource between the linux guest and an IO Service Partition. Currently it is using an enum dma_data_direction from "linux/dma-direction" for a field contained inside the channel. If the community ever decided to change the size of the enum our partition would break.

We should only be using fields created by us in the iochannel to maintain compatibility with the rest of s-Par.

[selltc]

I see you have a patch for this on the githubissue-78-upstream-next branch, as commit bf463ef7000d8c281b53a1327ccc65e2dd5dd507. I assume this is what you were asking about at our meeting earlier. I'll attempt to guide you thru our process...

[selltc]

I created links in both the KanBoard task description (KanBoard-1562) and above github description to link the KanBoard task and github issue together. That just makes things more convenient to navigate.

[selltc]

Comments regarding "patch mechanics" of commit bf463ef7000d8c281b53a1327ccc65e2dd5dd507

• The first line of the commit log comment should be a headline <= 72 characters long, essentially identifying the patch very concisely. For your patch, I would recommend:

  staging: unisys: fix improper use of dma_data_direction

• All comment decorations like Signed-off-by: should go at the end of the log comment.

FYI, David K is in the process of developing a script that will automatically append "githubissue: #<nnn>" at the end of the log comment (on a line by itself) as part of pushing patches to github. Until we all start using the script, it wouldn't hurt to manually append that to the end of all your commit comments. What that buys us is that github will see the issue number reference, and automatically include a note (with a link to the commit) as a github issue comment, and this will appear in the list of comments on the screen you are reading right now. Look at issue #71 as an example of what I mean.

When David K pushes stuff to Greg, his script will automatically remove all of these "githubissue:" decorations, because the upstream folks won't want to see that.

I'll look at the content of your patch next.

[selltc]

The patch looks good. My only comment involves your kerneldoc.

Re:

*      dma_data_dir_linux_to_spar
*      @d: dma direction value to convert
*
*      Converts standard dma direction value to a Unisys-specific one

As-per kerneldoc conventions (see Documentation/kernel-doc-nano-HOWTO.txt), you should probably have something like this:

*      dma_data_dir_linux_to_spar() - convert dma_data_direction value to
*                                     Unisys-specific equivalent
*      @d: dma direction value to convert
*
*      Return: the Unisys-specific dma direction value corresponding to @d

[davidker]

I agree with Tim's comments, but I would also suggest a describing more of the reason why we are doing this in the code commit comment. I'm sure greg and company will just say why not use dma_direction from the kernel. We need to make sure they know this is a shared data structure across multiple OS/language instances.

[selltc]

Agreed David; very good point.

[selltc]

I forgot one thing Steve. You should "assign yourself" to this issue by clicking the "assign yourself" link in the column to the right of this web page (under "Assignees").

[wordforthis]

Thanks guys. I updated the branch based on your comments. For the kerneldoc format, the documentation you pointed me to suggested their format was optional for static functions, so I tried harder to match the style in the surrounding code than the style in that documentation.

[selltc]

Ahh... forgot about that. We've been fairly lax about kerneldoc until we recently received comments from the upstream; that's primarily why it was on my mind.