[Skiboot] [PATCH] hw/npu2: support creset of npu2 devices

[Alistair Popple]

Acked-by: Alistair Popple <alistair at popple.id.au>

On Wednesday, 17 January 2018 10:19:32 AM AEDT Balbir Singh wrote:
> On Sun, Dec 17, 2017 at 3:56 PM, Balbir Singh <bsingharora at gmail.com> wrote:
> > On Fri, Dec 15, 2017 at 2:38 PM, Alistair Popple <alistair at popple.id.au> wrote:
> >> On Friday, 15 December 2017 1:59:42 PM AEDT Balbir Singh wrote:
> >>> creset calls in the hw procedure that resets the PHY, we don't
> >>> take them out of reset, just put them in reset.
> >>>
> >>> Signed-off-by: Balbir Singh <bsingharora at gmail.com>
> >>> ---
> >>>  hw/npu2-hw-procedures.c |  2 +-
> >>>  hw/npu2.c               | 21 ++++++++++++++++++++-
> >>>  include/npu2.h          |  1 +
> >>>  3 files changed, 22 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/hw/npu2-hw-procedures.c b/hw/npu2-hw-procedures.c
> >>> index 1318e867..dfff4a2a 100644
> >>> --- a/hw/npu2-hw-procedures.c
> >>> +++ b/hw/npu2-hw-procedures.c
> >>> @@ -233,7 +233,7 @@ static bool poll_fence_status(struct npu2_dev *ndev, uint64_t val)
> >>>  }
> >>>
> >>>  /* Procedure 1.2.1 - Reset NPU/NDL */
> >>> -static uint32_t reset_ntl(struct npu2_dev *ndev)
> >>> +uint32_t reset_ntl(struct npu2_dev *ndev)
> >>>  {
> >>>       uint64_t val;
> >>>
> >>> diff --git a/hw/npu2.c b/hw/npu2.c
> >>> index 7ffb0941..ee75b6fd 100644
> >>> --- a/hw/npu2.c
> >>> +++ b/hw/npu2.c
> >>> @@ -1170,6 +1170,25 @@ static int64_t npu2_freset(struct pci_slot *slot __unused)
> >>>       return OPAL_SUCCESS;
> >>>  }
> >>>
> >>> +static int64_t npu2_creset(struct pci_slot *slot)
> >>> +{
> >>> +     struct npu2 *p;
> >>> +     int i;
> >>> +     struct npu2_dev *ndev;
> >>> +
> >>> +     p = phb_to_npu2(slot->phb);
> >>> +     NPU2INF(p, "Creset PHB state\n");
> >>> +
> >>> +     for (i = 0; i < p->total_devices; i++) {
> >>
> >> Wont this reset every link on the NPU rather than just the given pci_slot? We
> >> need to make sure this resets just the specified device.
> >
> > We have a slot for each of the struct npu2 and total_devices are the
> > total number of devices on that phb. In the tests on a 4 GPU system, I
> > did not find anything missing or extra. I'll double check
> >
> >>
> >>> +             ndev = &p->devices[i];
> >>> +             if (ndev) {
> >>> +                     NPU2DEVINF(ndev, "Resetting device\n");
> >>> +                     reset_ntl(ndev);
> >>
> >> What is the motivation for implementing this? I suspect we should also reset the
> >> DL and PHY.
> >>
> >
> > A creset should set the links to reset state, I'm just following the
> > procedure I am aware of. I do not want to bring these links out of
> > reset
> >
> >
> >> Regards,
> >>
> >> Alistair
> 
> We discussed this offline as well, did you want to see any more changes?
> 
> Balbir
>