On Mon, Jul 22, 2019 at 08:56:35PM +0900, Masahisa Kojima wrote: > NETSEC hardware requires stable RXCLK input upon initialization > triggered with DISCORE = 0. > However, RXCLK input could be unstable depending on phy chipset > and deployed network environment, which could cause NETSEC to > hang up during initialization. > > We solve this platform/environment dependent issue by temporarily > putting phy in loopback mode, then we can expect the stable RXCLK input. > > Signed-off-by: Masahisa Kojima <masahisa.koj...@linaro.org> > Signed-off-by: Satoru Okamoto <okamoto.sat...@socionext.com> > --- > .../netsec_sdk/src/ogma_misc.c | 72 ++++++++++++++++++- > .../netsec_for_uefi/netsec_sdk/src/ogma_reg.h | 4 ++ > 2 files changed, 75 insertions(+), 1 deletion(-) > > diff --git > a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_misc.c > > b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_misc.c > index 7481d2da2d24..07308d38a5c2 100644 > --- > a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_misc.c > +++ > b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_misc.c > @@ -35,7 +35,6 @@ static const ogma_uint32 > desc_ring_config_reg_addr[OGMA_DESC_RING_ID_MAX + 1] = > > }; > > -
Please, no unrelated whitespace changes. > /* Internal function definition*/ > #ifndef OGMA_CONFIG_DISABLE_CLK_CTRL > STATIC void ogma_set_clk_en_reg ( > @@ -327,6 +326,60 @@ STATIC ogma_uint32 ogma_calc_pkt_ctrl_reg_param ( > return param; > } > > +STATIC > +void > +ogma_pre_init_microengine ( > + ogma_handle_t ogma_handle > + ) > +{ > + UINT16 Data; > + > + /* Remove dormant settings */ > + Data = ogma_get_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL) & > + ~((1U << OGMA_PHY_CONTROL_REG_POWER_DOWN) | > + (1U << OGMA_PHY_CONTROL_REG_ISOLATE)); I am unsure of how much we should try to adhere to TianoCore coding style in this imported module, but the above does not look correct regardless. Should not the second/third lines should be aligned relative to the ogma_get_phy_reg call rather than the variable name... > + > + ogma_set_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL, Data); > + > + while ((ogma_get_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL) & > + ((1U << OGMA_PHY_CONTROL_REG_POWER_DOWN) | > + (1U << OGMA_PHY_CONTROL_REG_ISOLATE))) != 0); ...like it is here. > + > + /* Put phy in loopback mode to guarantee RXCLK input */ > + Data |= (1U << OGMA_PHY_CONTROL_REG_LOOPBACK); > + > + ogma_set_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL, Data); > + > + while ((ogma_get_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL) & > + (1U << OGMA_PHY_CONTROL_REG_LOOPBACK)) == 0); > +} > + > +STATIC > +void > +ogma_post_init_microengine ( > + IN ogma_handle_t ogma_handle > + ) > +{ > + UINT16 Data; > + > + /* Get phy back to normal operation */ > + Data = ogma_get_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL) & > + ~(1U << OGMA_PHY_CONTROL_REG_LOOPBACK); Same indentation comment as above. > + > + ogma_set_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL, Data); > + > + while ((ogma_get_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL) & > + (1U << OGMA_PHY_CONTROL_REG_LOOPBACK)) != 0); > + > + Data |= (1U << OGMA_PHY_CONTROL_REG_RESET); > + > + /* Apply software reset */ > + ogma_set_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL, Data); > + > + while ((ogma_get_phy_reg (ogma_handle, OGMA_PHY_REG_ADDR_CONTROL) & > + (1U << OGMA_PHY_CONTROL_REG_RESET)) != 0); > +} > + > ogma_err_t ogma_init ( > void *base_addr, > pfdep_dev_handle_t dev_handle, > @@ -551,6 +604,16 @@ ogma_err_t ogma_init ( > ogma_write_reg( ctrl_p, OGMA_REG_ADDR_DMA_TMR_CTRL, > ( ogma_uint32)( ( OGMA_CONFIG_CLK_HZ / OGMA_CLK_MHZ) - > 1) ); > > + /* > + * Do pre-initialization tasks for microengine > + * > + * In particular, we put phy in loopback mode > + * in order to make sure RXCLK keeps provided to mac > + * irrespective of phy link status, > + * which is required for microengine intialization. > + */ Good use of comment here, and below. *But*, if the next thing was not the bit taking the PHY back out of loopback mode, I would have had to spend time searching for whether that was done. Could you add a line saying "this will be disabled once initialization complete"? / Leif > + ogma_pre_init_microengine (ctrl_p); > + > /* start microengines */ > ogma_write_reg( ctrl_p, OGMA_REG_ADDR_DIS_CORE, 0); > > @@ -573,6 +636,13 @@ ogma_err_t ogma_init ( > goto err; > } > > + /* > + * Do post-initialization tasks for microengine > + * > + * We put phy in normal mode and apply reset. > + */ > + ogma_post_init_microengine (ctrl_p); > + > /* clear microcode load end status */ > ogma_write_reg( ctrl_p, OGMA_REG_ADDR_TOP_STATUS, > OGMA_TOP_IRQ_REG_ME_START); > diff --git > a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_reg.h > > b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_reg.h > index 30c716352b37..ca769084cb31 100644 > --- > a/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_reg.h > +++ > b/Silicon/Socionext/SynQuacer/Drivers/Net/NetsecDxe/netsec_for_uefi/netsec_sdk/src/ogma_reg.h > @@ -138,8 +138,12 @@ > /* bit fields for PHY CONTROL Register */ > #define OGMA_PHY_CONTROL_REG_SPEED_SELECTION_MSB (6) > #define OGMA_PHY_CONTROL_REG_DUPLEX_MODE (8) > +#define OGMA_PHY_CONTROL_REG_ISOLATE (10) > +#define OGMA_PHY_CONTROL_REG_POWER_DOWN (11) > #define OGMA_PHY_CONTROL_REG_AUTO_NEGO_ENABLE (12) > #define OGMA_PHY_CONTROL_REG_SPEED_SELECTION_LSB (13) > +#define OGMA_PHY_CONTROL_REG_LOOPBACK (14) > +#define OGMA_PHY_CONTROL_REG_RESET (15) > > /* bit fields for PHY STATUS Register */ > #define OGMA_PHY_STATUS_REG_LINK_STATUS (2) > -- > 2.17.1 > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#44262): https://edk2.groups.io/g/devel/message/44262 Mute This Topic: https://groups.io/mt/32557832/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-