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]
-=-=-=-=-=-=-=-=-=-=-=-
