Re: [PATCH 3/8] NTB: Fix the default port and peer numbers for legacy drivers

[Logan Gunthorpe]

On 12/06/18 09:59 AM, Jon Mason wrote:
>> Patches for ntb_pingpong and ntb_perf follow (which are broken
>> otherwise) to support hardware that doesn't have port numbers. This is
>> important not only to not break support with existing drivers but for
>> the cross link topology which, due to its perfect symmetry, cannot
>> assign unique port numbers to each side.
> 
> This is a very long way of saying "no clients are checking the error
> codes, so removing them". :)

Well, clients not checking the error code made this harder to debug for
sure, but removing the error code is a side effect and not what is
happening here (in fact someone should probably still go back and add
error checking because these functions can still return errors but
that's not really something I have time to do). After the next couple
patches, the clients will use this change to detect that there are no
port numbers and handle things similarly to the way they did before they
were broken by the multiport changes.

> I think the history and references to follow-on patches are not
> necessary in the commit message and belong more in a 0/X.

This is the opposite of what I've ever heard before. Having a commit
message that explains what led up to this commit is a good thing and
allows people debugging in the future to better understand the decisions
made. People debugging commits will never find the 0/X cover letter
which is just intended to introduce the series to reviewers and describe
changes if the series is posted multiple times.

> This is more of a feature than a bug fix.  Can you break this (and the
> pingpong and perf changes caused by this) off into a separate series,
> as I'll want to apply this to the ntb-next and not bugfixes branch?

No this is not a feature request. This is fixing a regression that broke
previously working code in the only sensible way I can come up with. If
you have a better way to fix this, I'd be glad to hear it. But this
should *not* be treated as a feature request.

Logan