| Message ID | 20180124034458.12027-2-alistair@popple.id.au |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [1/2] npu2-hw-procedures.c: Power up lanes during ntl reset | expand |
Le 24/01/2018 à 04:44, Alistair Popple a écrit : > Each nvlink device is associated with a particular group of OBUS lanes via > a lane mask which is read from HDAT via the device-tree. However Skiboot's > interpretation of lane mask was different to what is exported from the > HDAT. > > Specifically the lane mask bits in the HDAT are encoded in IBM bit ordering > for a 24-bit wide value. So for example in normal bit ordering lane-0 is > represented by having lane-mask bit 23 set and lane-23 is represented by > lane-mask bit 0. This patch alters the Skiboot interpretation to match what > is passed from HDAT. > > Signed-off-by: Alistair Popple <alistair@popple.id.au> > --- Thanks for the heads up! Reviewed-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com> Andrew: please confirm you're seeing this. It obviously will have an impact when you merge it with your patchset (and I can see troubles ahead for our custom device tree on ZZ). Fred > hw/npu2-hw-procedures.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/hw/npu2-hw-procedures.c b/hw/npu2-hw-procedures.c > index a43952d7..b21c399d 100644 > --- a/hw/npu2-hw-procedures.c > +++ b/hw/npu2-hw-procedures.c > @@ -102,12 +102,12 @@ struct npu2_phy_reg NPU2_PHY_RX_CTL_DATASM_CLKDIST_PDWN = {0x2e0, 60, 1}; > #define NPU2_PHY_REG(scom_base, reg, lane) \ > SETFIELD(PPC_BITMASK(27, 31), ((reg)->offset << 42) | scom_base, lane) > > -#define NPU2_MAX_PHY_LANES 24 > +#define NPU2_MAX_PHY_LANE 23 > > /* This is a bit of a gross hack but it does the job */ > #define FOR_EACH_LANE(ndev, lane) \ > - for (lane = 0; lane < NPU2_MAX_PHY_LANES; lane++) \ > - if (!(ndev->lane_mask & (1 << lane))) \ > + for (lane = 0; lane <= NPU2_MAX_PHY_LANE; lane++) \ > + if (!(ndev->lane_mask & (1 << (NPU2_MAX_PHY_LANE - lane)))) \ > continue; \ > else >
On Wed, Jan 24, 2018 at 02:44:58PM +1100, Alistair Popple wrote: >Each nvlink device is associated with a particular group of OBUS lanes via >a lane mask which is read from HDAT via the device-tree. However Skiboot's >interpretation of lane mask was different to what is exported from the >HDAT. > >Specifically the lane mask bits in the HDAT are encoded in IBM bit ordering >for a 24-bit wide value. So for example in normal bit ordering lane-0 is >represented by having lane-mask bit 23 set and lane-23 is represented by >lane-mask bit 0. This patch alters the Skiboot interpretation to match what >is passed from HDAT. Reviewed-by: Reza Arbab <arbab@linux.vnet.ibm.com>
On 25/01/18 04:18, Frederic Barrat wrote: > > > Le 24/01/2018 à 04:44, Alistair Popple a écrit : >> Each nvlink device is associated with a particular group of OBUS lanes >> via >> a lane mask which is read from HDAT via the device-tree. However >> Skiboot's >> interpretation of lane mask was different to what is exported from the >> HDAT. >> >> Specifically the lane mask bits in the HDAT are encoded in IBM bit >> ordering >> for a 24-bit wide value. So for example in normal bit ordering lane-0 is >> represented by having lane-mask bit 23 set and lane-23 is represented by >> lane-mask bit 0. This patch alters the Skiboot interpretation to match >> what >> is passed from HDAT. >> >> Signed-off-by: Alistair Popple <alistair@popple.id.au> >> --- > > Thanks for the heads up! > > Reviewed-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com> > > Andrew: please confirm you're seeing this. It obviously will have an > impact when you merge it with your patchset (and I can see troubles > ahead for our custom device tree on ZZ). Reviewed-by: Andrew Donnellan <andrew.donnellan@au1.ibm.com> I'll apply this on my opencapi branch and fix things up accordingly. > > Fred > >> hw/npu2-hw-procedures.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/hw/npu2-hw-procedures.c b/hw/npu2-hw-procedures.c >> index a43952d7..b21c399d 100644 >> --- a/hw/npu2-hw-procedures.c >> +++ b/hw/npu2-hw-procedures.c >> @@ -102,12 +102,12 @@ struct npu2_phy_reg >> NPU2_PHY_RX_CTL_DATASM_CLKDIST_PDWN = {0x2e0, 60, 1}; >> #define NPU2_PHY_REG(scom_base, reg, lane) \ >> SETFIELD(PPC_BITMASK(27, 31), ((reg)->offset << 42) | scom_base, >> lane) >> >> -#define NPU2_MAX_PHY_LANES 24 >> +#define NPU2_MAX_PHY_LANE 23 >> >> /* This is a bit of a gross hack but it does the job */ >> #define FOR_EACH_LANE(ndev, lane) \ >> - for (lane = 0; lane < NPU2_MAX_PHY_LANES; lane++) \ >> - if (!(ndev->lane_mask & (1 << lane))) \ >> + for (lane = 0; lane <= NPU2_MAX_PHY_LANE; lane++) \ >> + if (!(ndev->lane_mask & (1 << (NPU2_MAX_PHY_LANE - lane)))) \ >> continue; \ >> else >>
diff --git a/hw/npu2-hw-procedures.c b/hw/npu2-hw-procedures.c index a43952d7..b21c399d 100644 --- a/hw/npu2-hw-procedures.c +++ b/hw/npu2-hw-procedures.c @@ -102,12 +102,12 @@ struct npu2_phy_reg NPU2_PHY_RX_CTL_DATASM_CLKDIST_PDWN = {0x2e0, 60, 1}; #define NPU2_PHY_REG(scom_base, reg, lane) \ SETFIELD(PPC_BITMASK(27, 31), ((reg)->offset << 42) | scom_base, lane) -#define NPU2_MAX_PHY_LANES 24 +#define NPU2_MAX_PHY_LANE 23 /* This is a bit of a gross hack but it does the job */ #define FOR_EACH_LANE(ndev, lane) \ - for (lane = 0; lane < NPU2_MAX_PHY_LANES; lane++) \ - if (!(ndev->lane_mask & (1 << lane))) \ + for (lane = 0; lane <= NPU2_MAX_PHY_LANE; lane++) \ + if (!(ndev->lane_mask & (1 << (NPU2_MAX_PHY_LANE - lane)))) \ continue; \ else
Each nvlink device is associated with a particular group of OBUS lanes via a lane mask which is read from HDAT via the device-tree. However Skiboot's interpretation of lane mask was different to what is exported from the HDAT. Specifically the lane mask bits in the HDAT are encoded in IBM bit ordering for a 24-bit wide value. So for example in normal bit ordering lane-0 is represented by having lane-mask bit 23 set and lane-23 is represented by lane-mask bit 0. This patch alters the Skiboot interpretation to match what is passed from HDAT. Signed-off-by: Alistair Popple <alistair@popple.id.au> --- hw/npu2-hw-procedures.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)