Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1690542

Re: [PATCH v2 00/16] Switchtec NTB Support

Show all headers | View raw

On 18/07/17 11:51 AM, Allen Hubbe wrote:
> Acked-by: Allen Hubbe <Allen.Hubbe@dell.com>

Thanks.

> Should the order of 6 and 7 be swapped?

Hmm, yes. I'll make the change for a v3 after more feedback comes.

> 6 - I think just the comment is best.  Rather than prohibit the use of functionality for hardware that does support the calls, in my opinion it should be left to specific hardware drivers to return an error.

As stated, I disagree. You can't allow clients to misuse the API when
testing with drivers that don't care. Otherwise, I'd have to manually
police the mailing-list for changes that would break when used with the
switchtec code. Better to make this automatic.

> 14 - I would prefer that a non-hardware-supported implementation of spads via a memory window should be common code, not reinvented in each specific hardware driver that lacks hardware spads.  You pointed out that the spads implemented here use the shared_mw construct, which is specific to this driver.  I am concerned that spads in the shared_mw (really anything in shared_mw, including this driver's indication of the peer link state) limits the applicability of this driver to just configurations of two ntb ports.  You stated this limitation upfront.

I think it should wait until other drivers actually do something similar
to common up the code. It's impossible to know exactly how it should be
made common until we know what other drivers actually need. When that
does happen I'm happy to work with anyone who wants to make it common.

The code, as is, is limited to two hosts largely because I have no way
to test multi-host setups (as no clients are ready for it) and
multi-host is also not a priority for us at the moment. The shared_mw
concept on its own will _not_ limit the number of hosts. The switchtec
hardware has special fixed size LUT memory windows (the number of these
is configurable but should have easily enough to provide one window per
port as the default is 32 and the maximum is 128). I'm not sure if other
devices provide similar functionality but it's fairly necessary for
emulating spads and link status with memory windows.

Logan

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[PATCH v2 00/16] Switchtec NTB Support Logan Gunthorpe <logang@deltatee.com> - 2017-07-18 18:10 +0200
  [PATCH v2 12/16] switchtec_ntb: add link management Logan Gunthorpe <logang@deltatee.com> - 2017-07-18 18:10 +0200
  [PATCH v2 04/16] switchtec: add link event notifier callback Logan Gunthorpe <logang@deltatee.com> - 2017-07-18 18:10 +0200
  [PATCH v2 11/16] switchtec_ntb: add skeleton NTB driver Logan Gunthorpe <logang@deltatee.com> - 2017-07-18 18:10 +0200
  [PATCH v2 15/16] switchtec_ntb: add memory window support Logan Gunthorpe <logang@deltatee.com> - 2017-07-18 18:10 +0200
  [PATCH v2 13/16] switchtec_ntb: implement doorbell registers Logan Gunthorpe <logang@deltatee.com> - 2017-07-18 18:10 +0200
  [PATCH v2 07/16] ntb: ntb_test: ensure the link is up before trying to configure the mws Logan Gunthorpe <logang@deltatee.com> - 2017-07-18 18:10 +0200
  [PATCH v2 02/16] switchtec: export class symbol for use in upper layer driver Logan Gunthorpe <logang@deltatee.com> - 2017-07-18 18:10 +0200
  [PATCH v2 03/16] switchtec: add NTB hardware register definitions Logan Gunthorpe <logang@deltatee.com> - 2017-07-18 18:10 +0200
  [PATCH v2 16/16] switchtec_ntb: update switchtec documentation with notes for NTB Logan Gunthorpe <logang@deltatee.com> - 2017-07-18 18:10 +0200
  [PATCH v2 01/16] switchtec: move structure definitions into a common header Logan Gunthorpe <logang@deltatee.com> - 2017-07-18 18:10 +0200
  RE: [PATCH v2 00/16] Switchtec NTB Support "Allen Hubbe" <Allen.Hubbe@dell.com> - 2017-07-18 20:00 +0200
    Re: [PATCH v2 00/16] Switchtec NTB Support Logan Gunthorpe <logang@deltatee.com> - 2017-07-18 20:10 +0200

csiph-web