> > > There is some concept of "split bar" in this function, and I want to > > be sure to > > > understand it correctly. > > > > > > BAR0 - configuration? > > > BAR1 - 32bit memory window? i.e. "split" bar? > > > BAR2+3 - 64bit memory window? > > > BAR4+5 - 64bit memory window? > > > > Yes > > Ok. > > > > Note that "split" in the intel driver refers to BAR4+5, which is > > normally a 64bit > > > memory window, split into independent 32bit windows BAR4 and BAR5 > by > > > bios configuration. Calling it "split" there makes sense. Here, > > calling it "split" > > > is confusing, but as long as the code is correct, I think it's ok. > > > > AMD NTB has similar design. > > If by similar design, you mean that BAR2+3 or BAR4+5 can be split, those > configurations are not currently supported by this driver. If needed, support > for those configurations can be added later, as a patch. > > Ok with the current implementation. > > > > I see a lot of readl/writel and ioread/iowrite in the same file, > > > even > > in the > > > same function as there is here. Pick one variant of the functions, > > preferably > > > the ioread/iowrite variant, and be consistent in its usage > > > throughout > > the file. > > > > Ioread/iowrite is only for 64bit read/write, I don't think it has any > > confusion. > > It's not just for 64bit: there are ioread8,16,32. In fact, ioread64 is the > one > that's not provided by io.h: not all hardware can do a true 64bit read or > write. > We don't need a true 64bit operation, so ioread64 is provided locally, for > convenience within this driver. > > > Actually, Intel NTB driver has same behavior. > > Actually, Intel NTB driver does not use readl/writel. > > Would you please fix this?
I'll change ioread64/iowrite64 to read64/write64, keep consistent witch readl/readw > > > It's also interesting that if it hits the last branch, > > > ndev->peer_sta > > is assigned > > > zero, but the function returns zero, not LNK_STA. If the function > > were > > > immediately called again, it would return LNK_STA. Can you please > > explain > > > the logic here? > > > > Force to read link status register again because the opposite side > > maybe still In resuming progress, the status variable can't show the > > right state, have to Read register directly. > > There's no indication in this function that it interacts with the timer. With > your suggestion, I'll take a closer look at the interactions between this > function and the timer. I'll add a comment.
