| Message ID | 1503029477-31179-1-git-send-email-alistair@popple.id.au |
|---|---|
| State | Accepted |
| Headers | show |
On Fri, Aug 18, 2017 at 02:11:17PM +1000, Alistair Popple wrote: >Newer versions of Hostboot will have various clocks powered down by default >to save power. Therefore we need to power them up before accessing the OBUS >PHY. > >Signed-off-by: Alistair Popple <alistair@popple.id.au> >--- > >Stewart, > >Reza wants to test this before we merge it so best to wait for his >reviewed/tested-by first. Thanks! I got confirmation from the lab that this patch eliminates the workaround they were using, but I was curious about one thing: >@@ -252,6 +257,12 @@ static uint32_t phy_reset(struct npu2_dev *ndev) > { > int lane; > >+ /* Power on clocks */ >+ phy_write(ndev, &NPU2_PHY_RX_CLKDIST_PDWN, 0); >+ phy_write(ndev, &NPU2_PHY_RX_IREF_PDWN, 1); Shouldn't this one be 0 like the rest? >+ phy_write(ndev, &NPU2_PHY_TX_CLKDIST_PDWN, 0); >+ phy_write(ndev, &NPU2_PHY_RX_CTL_DATASM_CLKDIST_PDWN, 0); >+ > FOR_EACH_LANE(ndev, lane) > phy_write_lane(ndev, &NPU2_PHY_RX_RUN_LANE, lane, 0); >
On Mon, 21 Aug 2017 10:05:57 AM Reza Arbab wrote: > On Fri, Aug 18, 2017 at 02:11:17PM +1000, Alistair Popple wrote: > >Newer versions of Hostboot will have various clocks powered down by default > >to save power. Therefore we need to power them up before accessing the OBUS > >PHY. > > > >Signed-off-by: Alistair Popple <alistair@popple.id.au> > >--- > > > >Stewart, > > > >Reza wants to test this before we merge it so best to wait for his > >reviewed/tested-by first. Thanks! > > I got confirmation from the lab that this patch eliminates the > workaround they were using, but I was curious about one thing: > > >@@ -252,6 +257,12 @@ static uint32_t phy_reset(struct npu2_dev *ndev) > > { > > int lane; > > > >+ /* Power on clocks */ > >+ phy_write(ndev, &NPU2_PHY_RX_CLKDIST_PDWN, 0); > >+ phy_write(ndev, &NPU2_PHY_RX_IREF_PDWN, 1); > > Shouldn't this one be 0 like the rest? Not sure, we will have to confirm with Chris. According to what he gave me though it should be 0x1: rx_clkdist_pdwn :: indirect_reg(0x102) start_bit(48) width(3) = 0x0 rx_iref_pdwn_b :: indirect_reg(0x118) start_bit(54) width(1) = 0x1 tx_clkdist_pdwn :: indirect_reg(0x182) start_bit(48) width(3) = 0x0 rx_ctl_datasm_clkdist_pdwn :: indirect_reg(0x170) start_bit(60) width(1) = 0x0 > >+ phy_write(ndev, &NPU2_PHY_TX_CLKDIST_PDWN, 0); > >+ phy_write(ndev, &NPU2_PHY_RX_CTL_DATASM_CLKDIST_PDWN, 0); > >+ > > FOR_EACH_LANE(ndev, lane) > > phy_write_lane(ndev, &NPU2_PHY_RX_RUN_LANE, lane, 0); > > > >
On Tue, Aug 22, 2017 at 05:08:14PM +1000, Alistair Popple wrote: >On Mon, 21 Aug 2017 10:05:57 AM Reza Arbab wrote: >> On Fri, Aug 18, 2017 at 02:11:17PM +1000, Alistair Popple wrote: >> >@@ -252,6 +257,12 @@ static uint32_t phy_reset(struct npu2_dev *ndev) >> > { >> > int lane; >> > >> >+ /* Power on clocks */ >> >+ phy_write(ndev, &NPU2_PHY_RX_CLKDIST_PDWN, 0); >> >+ phy_write(ndev, &NPU2_PHY_RX_IREF_PDWN, 1); >> >> Shouldn't this one be 0 like the rest? > >Not sure, we will have to confirm with Chris. According to what he gave me >though it should be 0x1: > >rx_clkdist_pdwn :: indirect_reg(0x102) start_bit(48) width(3) = 0x0 >rx_iref_pdwn_b :: indirect_reg(0x118) start_bit(54) width(1) = 0x1 >tx_clkdist_pdwn :: indirect_reg(0x182) start_bit(48) width(3) = 0x0 >rx_ctl_datasm_clkdist_pdwn :: indirect_reg(0x170) start_bit(60) width(1) = 0x0 Okay, just checking. And since it's been verified to work, that's good enough for me. Reviewed-by: Reza Arbab <arbab@linux.vnet.ibm.com>
Alistair Popple <alistair@popple.id.au> writes: > Newer versions of Hostboot will have various clocks powered down by default > to save power. Therefore we need to power them up before accessing the OBUS > PHY. > > Signed-off-by: Alistair Popple <alistair@popple.id.au> > --- > > Stewart, > > Reza wants to test this before we merge it so best to wait for his > reviewed/tested-by first. Thanks! Thanks (to both, testing is valuable). Merged to master as of 2d4dd7b19ede9aa62c88ead20f97f3e51eb18984
diff --git a/hw/npu2-hw-procedures.c b/hw/npu2-hw-procedures.c index de70009..c9e7673 100644 --- a/hw/npu2-hw-procedures.c +++ b/hw/npu2-hw-procedures.c @@ -87,6 +87,11 @@ struct npu2_phy_reg NPU2_PHY_RX_HIST_MIN_EYE_WIDTH = {0x24e, 54, 8}; struct npu2_phy_reg NPU2_PHY_RX_HIST_MIN_EYE_WIDTH_LANE = {0x24e, 49, 5}; struct npu2_phy_reg NPU2_PHY_RX_HIST_MIN_EYE_WIDTH_VALID= {0x24e, 48, 1}; +struct npu2_phy_reg NPU2_PHY_RX_CLKDIST_PDWN = {0x204, 48, 3}; +struct npu2_phy_reg NPU2_PHY_RX_IREF_PDWN = {0x230, 54, 1}; +struct npu2_phy_reg NPU2_PHY_TX_CLKDIST_PDWN = {0x305, 48, 3}; +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) @@ -252,6 +257,12 @@ static uint32_t phy_reset(struct npu2_dev *ndev) { int lane; + /* Power on clocks */ + phy_write(ndev, &NPU2_PHY_RX_CLKDIST_PDWN, 0); + phy_write(ndev, &NPU2_PHY_RX_IREF_PDWN, 1); + phy_write(ndev, &NPU2_PHY_TX_CLKDIST_PDWN, 0); + phy_write(ndev, &NPU2_PHY_RX_CTL_DATASM_CLKDIST_PDWN, 0); + FOR_EACH_LANE(ndev, lane) phy_write_lane(ndev, &NPU2_PHY_RX_RUN_LANE, lane, 0);
Newer versions of Hostboot will have various clocks powered down by default to save power. Therefore we need to power them up before accessing the OBUS PHY. Signed-off-by: Alistair Popple <alistair@popple.id.au> --- Stewart, Reza wants to test this before we merge it so best to wait for his reviewed/tested-by first. Thanks! - Alistair hw/npu2-hw-procedures.c | 11 +++++++++++ 1 file changed, 11 insertions(+)