RE: [PATCH V5 1/1] NTB: Add support for AMD PCI-Express Non-Transparent Bridge

[Yu, Xiangliang]

> From: Yu, Xiangliang [mailto:Xiangliang.Yu@xxxxxxx]
> > > > Signed-off-by: Jon Mason <jdmason@xxxxxxxx>
> > > > Signed-off-by: Allen Hubbe <Allen.Hubbe@xxxxxxx>
> > >
> > > NO.
> >
> > Ok, I'll change it if you doesn't want to change it.
> 
> Nah, just remember it for next time...
> 
> I'm satisfied with this v5.
> 
> Reviewed-by: Allen Hubbe <Allen.Hubbe@xxxxxxx>

Ok, I'll change it and resend V5

> > I donât think so. In here, the i/o memory is only happened when
> > pci_iomap return Success, so the register can't be accessed through IO
> > port way. And
> > ioread* will
> > Check if the memory type is mmio type or IO port type (please see the
> > definition).
> >  I donât think we need to check It, so I use read* because It can make
> > more efficient.
> > I think we need to think about actual usage, not only follow book.
> > And, I have said it in previous version, I donât like explain it
> > again, and again.
> > If you have any concern, please tell me after my comment.
> 
> It's not more efficient, on this platform it's the same.
> 
> If it were my driver I would change it... but you can keep it this way.

Because my previous SATA experience, I'd like to use this way.

> > > This is different from v4.  It used to be:
> >
> > Because peer_sta is change to 0, so amd_link_is_up will return 0
> > (offline)
> > And will not check hardware link status. So It maybe make it offline
> > forever
> 
> It fixed a bug?  Great!
> 
> > > I'm nervous about ndev->peer_sta, the behavior of link_is_up,
> > > timers...
> >
> > Actually, the code is designed according to Atom NTB, except for the
> > peer_sta.
> 
> Except for peer_sta, and that's a pretty critical design change.  I'm still
> nervous, but I'll trust that you have been able to test this behavior
> thourougly.

Yes, the part of code will be changed in future because hardware design is
Being changed  too.

> 
> > I'll add the explaination when having changes.