[PATCH V2 0/3] Change notes of V2

[PATCH V2 0/3] Change notes of V2

[Xiangliang Yu]

Main changes in V2:
1. Fixed compiler warning;
2. Add marcro argument of ndev in NTB_READ_REG/NTB_WRITE_REG;
3. Add notes for flush and wakeup interfaces;

Xiangliang Yu (3):
  NTB: Add AMD PCI-Express NTB driver
  NTB: Add AMD NTB support in Kconfig and Makefile
  NTB: Add flush_req and wakeup interface

 drivers/ntb/hw/Kconfig          |    1 +
 drivers/ntb/hw/Makefile         |    1 +
 drivers/ntb/hw/amd/Kconfig      |    7 +
 drivers/ntb/hw/amd/Makefile     |    1 +
 drivers/ntb/hw/amd/ntb_hw_amd.c | 1265 +++++++++++++++++++++++++++++++++++++++
 drivers/ntb/hw/amd/ntb_hw_amd.h |  265 ++++++++
 include/linux/ntb.h             |   35 ++
 7 files changed, 1575 insertions(+)
 create mode 100644 drivers/ntb/hw/amd/Kconfig
 create mode 100644 drivers/ntb/hw/amd/Makefile
 create mode 100644 drivers/ntb/hw/amd/ntb_hw_amd.c
 create mode 100644 drivers/ntb/hw/amd/ntb_hw_amd.h

-- 
1.9.1

RE: [PATCH V2 0/3] Change notes of V2

[Allen Hubbe]

From: Xiangliang Yu <Xiangliang.Yu@amd.com>:
> Main changes in V2:
> 1. Fixed compiler warning;
> 2. Add marcro argument of ndev in NTB_READ_REG/NTB_WRITE_REG;
> 3. Add notes for flush and wakeup interfaces;
> 
> Xiangliang Yu (3):
>   NTB: Add AMD PCI-Express NTB driver
>   NTB: Add AMD NTB support in Kconfig and Makefile
>   NTB: Add flush_req and wakeup interface

Could you re-spin these patches as:

Patch 1:
	Only the parts of the driver that fit the current NTB APIs.

Patch 2:
	The new flush_req API, and the related code in the AMD driver.

Patch 3:
	The new wakeup API, and related code in the AMD driver.

In particular, I think we need feedback on #3 from PCI and power management maintainers.

When making #1, please make sure that the patch stands on its own, without any of the code related to #2 or #3 in #1.  Do put the makefile and kconfig changes in #1.

Thanks,
Allen

RE: [PATCH V2 0/3] Change notes of V2

[Yu, Xiangliang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1402 bytes --]

> From: Xiangliang Yu <Xiangliang.Yu@amd.com>:
> > Main changes in V2:
> > 1. Fixed compiler warning;
> > 2. Add marcro argument of ndev in NTB_READ_REG/NTB_WRITE_REG; 3.
> Add
> > notes for flush and wakeup interfaces;
> >
> > Xiangliang Yu (3):
> >   NTB: Add AMD PCI-Express NTB driver
> >   NTB: Add AMD NTB support in Kconfig and Makefile
> >   NTB: Add flush_req and wakeup interface
> 
> Could you re-spin these patches as:
> 
> Patch 1:
> 	Only the parts of the driver that fit the current NTB APIs.
> 
> Patch 2:
> 	The new flush_req API, and the related code in the AMD driver.
> 
> Patch 3:
> 	The new wakeup API, and related code in the AMD driver.
> 
Ok.

> In particular, I think we need feedback on #3 from PCI and power
> management maintainers.
I don't get your concern.
I think we can add device attribute file to let application to trigger wakeup function, then NTB hardware will do the rest. NTB driver just need to implement suspend/resume interface of PCI PM.

Add one more thing, do you think NTB should support runtime power management?

> 
> When making #1, please make sure that the patch stands on its own, without
> any of the code related to #2 or #3 in #1.  Do put the makefile and kconfig
> changes in #1.
Ok. 
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH V2 0/3] Change notes of V2

[Allen Hubbe]

> > In particular, I think we need feedback on #3 from PCI and power
> > management maintainers.
>
> I don't get your concern.
> I think we can add device attribute file to let application to trigger
> wakeup function, then NTB hardware will do the rest. NTB driver just
> need to implement suspend/resume interface of PCI PM.
> 
> Add one more thing, do you think NTB should support runtime power
> management?
>

I think it is good to make the power management functionality available.  In other words, yes, to your last question.

My concern is that I would like some degree of certainty that it is done right, in harmony with the rest of the kernel.  I don't know what "done right" means in this case, which is why I would like someone else to review it.  A smaller patch with only (and all of) the power management code will have a better chance of being reviewed.

I'm also concerned about the waiting behavior in #2 and #3.  I'm not saying it's wrong.  At least now that behavior is noted in the api documentation; thanks for that.  If a PCI or power management expert has no objection to the waiting behavior in #3, then I would be comfortable with that behavior in #2 as well.

Allen

RE: [PATCH V2 0/3] Change notes of V2

[Yu, Xiangliang]

Hi ,




> > > In particular, I think we need feedback on #3 from PCI and power
> > > management maintainers.
> >
> > I don't get your concern.
> > I think we can add device attribute file to let application to trigger
> > wakeup function, then NTB hardware will do the rest. NTB driver just
> > need to implement suspend/resume interface of PCI PM.
> >
> > Add one more thing, do you think NTB should support runtime power
> > management?
> >
> 
> I think it is good to make the power management functionality available.  In
> other words, yes, to your last question.

Got it.

> My concern is that I would like some degree of certainty that it is done right,
> in harmony with the rest of the kernel.  I don't know what "done right"
> means in this case, which is why I would like someone else to review it.  A
> smaller patch with only (and all of) the power management code will have a
> better chance of being reviewed.

I think it is ok if following the PM interface and test pass. This version I'll remove 
the PM part and will submit all related PM patch when runtime code is ready.

> I'm also concerned about the waiting behavior in #2 and #3.  I'm not saying
> it's wrong.  At least now that behavior is noted in the api documentation;
> thanks for that.  If a PCI or power management expert has no objection to
> the waiting behavior in #3, then I would be comfortable with that behavior in
> #2 as well.

I also don't like the waiting behavior, but I can't find the asynchronous method to
Let application know the result. And I think #2 is different from #3 because it isn't
related to PM or PCI. Please let me know if you have better choice.