RE: [PATCH 03/16] ntb: Enable link training for RP mode in the driver probe

[Hubbe, Allen]

From: Bjorn Helgaas [mailto:bhelgaas@xxxxxxxxxx]
> On Wed, May 20, 2015 at 4:46 PM, Hubbe, Allen <Allen.Hubbe@xxxxxxx>
> wrote:
> > From: Bjorn Helgaas [mailto:bhelgaas@xxxxxxxxxx]
> >> Please run "git log --oneline drivers/ntb" and make your subject
> lines
> >> consistent.  The convention I use in PCI (and what was used in NTB
> >> until recently) is
> >>
> >>   - Acronyms and initialisms are capitalized
> >>   - Subject line is a subsystem prefix ("PCI:", "NTB:", etc.)
> followed
> >> by a complete sentence starting with a capitalized verb, with no
> >> period at the end
> >
> > These are the new subject lines that will be in the v2 of this set.
> 
> That helps a lot, thanks.
> 
> > NTB: Fix small code format issues in transport
> > NTB: Default to CPU memcpy for performance
> > NTB: Improve performance with write combining
> > NTB: Use NUMA aware memory allocation
> > NTB: Use NUMA memory and DMA chan in transport
> > NTB: Reset transport QP link stats on down
> > NTB: Do not advance transport RX on link down
> > NTB: Differentiate transport link down messages
> > NTB: Rate limit ntb_qp_link_work
> > NTB: Add tool client
> > NTB: Add ping pong client
> 
> I think these are test cases, not part of the kernel itself.  It'd be
> nice to know that from the summary.  I don't know what the precedent
> is for that (if there is one).

Would you consider this as precedent:

4a776f0 dmatest: Simple DMA memcpy test client

> 
> > NTB: Intel NTB params for SNB B2B addresses
> 
> This sentence no verb :)  Not sure what you're doing with those params.

NTB: Add parameters for Intel SNB B2B addresses

> 
> > NTB: Check the DID for workaround error flags
> 
> I think there's room to spell out "device ID".

NTB: Check the device ID to set errata flags

Also, add this prose description to this commit:

Set errata flags for the specific device IDs to which they apply,
instead of the whole xeon hardware class.

> 
> > NTB: Enable link training for RP mode in probe
> 
> Is that Root Port?

yes

> 
> > NTB: Add NTB hardware abstraction layer
> > NTB: Move files in preparation for NTB abstraction
> 
> Of course, none of this has anything to do with the patches
> themselves, which I don't intend to review because they're out of my
> area :)
> 

I appreciate your comments :)

> >>
> >> On Wed, May 20, 2015 at 10:41 AM, Allen Hubbe <Allen.Hubbe@xxxxxxx>
> >> wrote:
> >> > From: Dave Jiang <dave.jiang@xxxxxxxxx>
> >> >
> >> > Link training for RP should be enabled in the driver probe. We
> should
> >> > not have to wait for transport loaded for this to hapen. Otherwise
> the
> >>
> >> s/hapen/happen/
> >>
> >> > device will not show up on the transparent bridge side.
> >> >
> >> > Signed-off-by: Dave Jiang <dave.jiang@xxxxxxxxx>
> >> > ---
> >> >  drivers/ntb/hw/intel/ntb_hw_intel.c | 10 ++++++++++
> >> >  1 file changed, 10 insertions(+)
> >> >
> >> > diff --git a/drivers/ntb/hw/intel/ntb_hw_intel.c
> >> b/drivers/ntb/hw/intel/ntb_hw_intel.c
> >> > index 05c4b77..d162f22 100644
> >> > --- a/drivers/ntb/hw/intel/ntb_hw_intel.c
> >> > +++ b/drivers/ntb/hw/intel/ntb_hw_intel.c
> >> > @@ -1333,6 +1333,9 @@ static int snb_poll_link(struct intel_ntb_dev
> >> *ndev)
> >> >
> >> >  static int snb_link_is_up(struct intel_ntb_dev *ndev)
> >> >  {
> >> > +       if (ndev->ntb.topo == NTB_TOPO_SEC)
> >> > +               return 1;
> >> > +
> >> >         return NTB_LNK_STA_ACTIVE(ndev->lnk_sta);
> >> >  }
> >> >
> >> > @@ -1642,6 +1645,7 @@ static int snb_setup_b2b_mw(struct
> intel_ntb_dev
> >> *ndev,
> >> >  static int snb_init_ntb(struct intel_ntb_dev *ndev)
> >> >  {
> >> >         int rc;
> >> > +       u32 ntb_ctl;
> >> >
> >> >         if (ndev->bar4_split)
> >> >                 ndev->mw_count = HSX_SPLIT_BAR_MW_COUNT;
> >> > @@ -1658,6 +1662,12 @@ static int snb_init_ntb(struct intel_ntb_dev
> >> *ndev)
> >> >                         dev_err(ndev_dev(ndev), "NTB Primary config
> >> disabled\n");
> >> >                         return -EINVAL;
> >> >                 }
> >> > +
> >> > +               /* enable link to allow secondary side device to
> >> appear */
> >> > +               ntb_ctl = ioread32(ndev->self_mmio + ndev->reg-
> >> >ntb_ctl);
> >> > +               ntb_ctl &= ~NTB_CTL_DISABLE;
> >> > +               iowrite32(ntb_ctl, ndev->self_mmio + ndev->reg-
> >> >ntb_ctl);
> >> > +
> >> >                 /* use half the spads for the peer */
> >> >                 ndev->spad_count >>= 1;
> >> >                 ndev->self_reg = &snb_pri_reg;
> >> > --
> >> > 2.4.0.rc0.43.gcf8a8c6
> >> >
> >> > --
> >> > To unsubscribe from this list: send the line "unsubscribe linux-
> pci"
> >> in
> >> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> >> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> You received this message because you are subscribed to the Google
> Groups "linux-ntb" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to linux-ntb+unsubscribe@xxxxxxxxxxxxxxxxx
> To post to this group, send email to linux-ntb@xxxxxxxxxxxxxxxxx
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/linux-
> ntb/CAErSpo5WzQ86t5F2pfhHYipcUH3h2vv1DN1-
> Dpx_VPsOnbpu%3Dw%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.
N‹§²æ¸›yú²X¬¶ÇvØ–)Þ{.nlj·¥Š{±‘êX§¶›¡Ü}©ž²ÆzÚj:+v‰¨¾«‘êZ+€Êzf£¢·hšˆ§~†­†Ûÿû®w¥¢¸?™¨è&¢)ßf”ùy§m…á«a¶Úÿ0¶ìå