[RFT net-next v4 3/5] net: stmmac: dwmac-meson8b: fix internal RGMII clock configuration

Martin Blumenstingl martin.blumenstingl at googlemail.com
Tue Jan 16 03:20:27 PST 2018


On Sun, Jan 14, 2018 at 10:48 PM, Martin Blumenstingl
<martin.blumenstingl at googlemail.com> wrote:
> Tests (using an oscilloscope and an Odroid-C1 board with a RTL8211F
> RGMII PHY) have shown that the PRG_ETH0 register behaves as follows:
> - bit 4 is a mux to choose between two parent clocks. according to the
>   public S805 datasheet the only supported parent clock is MPLL2 (this
>   was not verified using the oscilloscope).
>   The public S805/S905 datasheet claims that this bit is reserved.
> - bits 9:7 control a one-based divider (register value 1 means "divide
>   by 1", etc.) for the input clock. we call this clock the "m250_div"
>   clock because it's value is always supposed to be (close to) 250MHz
>   (see below for an explanation).
>   The description in the public S805/S905 datasheet is a bit cryptic,
>   but it comes down to "input clock = 250MHz * value" (which could also
>   be expressed as "250MHz = input clock / value")
> - there seems to be an internal fixed divide-by-2 clock which takes the
>   output from the m250_div and divides it by 2. This is not unusual on
>   Amlogic SoCs, since the SDIO (MMC) driver also uses an internal fixed
>   divide-by-2 clock.
>   This is not documented in the public S805/S905 datasheet
> - bit 10 controls a gate clock which enables or disables the RGMII TX
>   clock (which is an output on the MAC/SoC and an input in the PHY). we
>   call this the "rgmii_tx_en" clock. if this bit is set to "0" the RGMII
>   TX clock output is close to 0
>   The description for this bit in the public S805/S905 datasheet is
>   "Generate 25MHz clock for PHY". Based on these tests it's believed
>   that this is wrong, and should probably read "Generate the 125MHz
>   RGMII TX clock for the PHY"
> - the RGMII TX clock has to be set to 125MHz - the IP block adjusts the
>   output (automatically) depending on the line speed (RGMII specifies
>   that Gbit connections use a 125MHz clock, 100Mbit/s connections use a
>   25MHz clock and 10Mbit/s connections use a 2.5MHz clock. only Gbit and
>   100Mbit/s were tested with an oscilloscope). Due to the requirement
>   that this clock always has to be set to 125MHz and due to the fixed
>   divide-by-2 parent clock this means that m250_div will always end up
>   with a rate of (close to) 250MHz.
> - bits 6:5 are the TX delay, which is also named "clock phase" in some
>   of Amlogic's older GPL kernel sources.
>
> The PHY also has an XTAL_IN pin where a 25MHz clock has to be provided.
> Tests with the oscilloscope have shown that this is routed to a crystal
> right next to the RTL8211F PHY. The same seems to be true on the Khadas
> VIM2 (which uses a GXM SoC) board - however the 25MHz crystal is on the
> other side of the PCB there.
>
> This updates the clocks in the dwmac-meson8b driver by replacing the
> "m25_div" with the "rgmii_tx_en" clock and additionally introducing a
> fixed divide-by-2 clock between "m250_div" and "rgmii_tx_en".
> Now we also need to set a frequency of 125MHz on the RGMII clock
> (opposed to the 25MHz we set before, with that non-existing
> divide-by-5-or-10 divider).
>
> Special thanks go to Linus Lüssing for testing the various bits and
> checking the results with an oscilloscope on his Odroid-C1!
>
> Fixes: 566e8251625304 ("net: stmmac: add a glue driver for the Amlogic Meson 8b / GXBB DWMAC")
> Reported-by: Emiliano Ingrassia <ingrassia at epigenesys.com>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl at googlemail.com>
NACK-ed by: Yixun Lan <yixun.lan at amlogic.com>

as it breaks the AXG SoCs (maybe not even directly related to this
patch, but we're currently not sure if m250_mux is defined correctly)
see: [0]

> ---
>  .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c    | 79 +++++++++++++---------
>  1 file changed, 47 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
> index 670f344f7168..e9fec9e0425c 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
> @@ -40,9 +40,7 @@
>  #define PRG_ETH0_CLK_M250_DIV_SHIFT    7
>  #define PRG_ETH0_CLK_M250_DIV_WIDTH    3
>
> -/* divides the result of m25_sel by either 5 (bit unset) or 10 (bit set) */
> -#define PRG_ETH0_CLK_M25_DIV_SHIFT     10
> -#define PRG_ETH0_CLK_M25_DIV_WIDTH     1
> +#define PRG_ETH0_RGMII_TX_CLK_EN       10
>
>  #define PRG_ETH0_INVERTED_RMII_CLK     BIT(11)
>  #define PRG_ETH0_TX_AND_PHY_REF_CLK    BIT(12)
> @@ -63,8 +61,11 @@ struct meson8b_dwmac {
>         struct clk_divider      m250_div;
>         struct clk              *m250_div_clk;
>
> -       struct clk_divider      m25_div;
> -       struct clk              *m25_div_clk;
> +       struct clk_fixed_factor fixed_div2;
> +       struct clk              *fixed_div2_clk;
> +
> +       struct clk_gate         rgmii_tx_en;
> +       struct clk              *rgmii_tx_en_clk;
>
>         u32                     tx_delay_ns;
>  };
> @@ -88,11 +89,6 @@ static int meson8b_init_rgmii_tx_clk(struct meson8b_dwmac *dwmac)
>         struct device *dev = &dwmac->pdev->dev;
>         const char *clk_div_parents[1];
>         const char *mux_parent_names[MUX_CLK_NUM_PARENTS];
> -       static const struct clk_div_table clk_25m_div_table[] = {
> -               { .val = 0, .div = 5 },
> -               { .val = 1, .div = 10 },
> -               { /* sentinel */ },
> -       };
>
>         /* get the mux parents from DT */
>         for (i = 0; i < MUX_CLK_NUM_PARENTS; i++) {
> @@ -149,25 +145,40 @@ static int meson8b_init_rgmii_tx_clk(struct meson8b_dwmac *dwmac)
>         if (WARN_ON(IS_ERR(dwmac->m250_div_clk)))
>                 return PTR_ERR(dwmac->m250_div_clk);
>
> -       /* create the m25_div */
> -       init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#m25_div",
> +       /* create the fixed_div2 */
> +       init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#fixed_div2",
>                                    dev_name(dev));
> -       init.ops = &clk_divider_ops;
> -       init.flags = CLK_IS_BASIC | CLK_SET_RATE_PARENT;
> +       init.ops = &clk_fixed_factor_ops;
> +       init.flags = CLK_SET_RATE_PARENT;
>         clk_div_parents[0] = __clk_get_name(dwmac->m250_div_clk);
>         init.parent_names = clk_div_parents;
>         init.num_parents = ARRAY_SIZE(clk_div_parents);
>
> -       dwmac->m25_div.reg = dwmac->regs + PRG_ETH0;
> -       dwmac->m25_div.shift = PRG_ETH0_CLK_M25_DIV_SHIFT;
> -       dwmac->m25_div.width = PRG_ETH0_CLK_M25_DIV_WIDTH;
> -       dwmac->m25_div.table = clk_25m_div_table;
> -       dwmac->m25_div.hw.init = &init;
> -       dwmac->m25_div.flags = CLK_DIVIDER_ALLOW_ZERO;
> +       dwmac->fixed_div2.mult = 1;
> +       dwmac->fixed_div2.div = 2;
> +       dwmac->fixed_div2.hw.init = &init;
> +
> +       dwmac->fixed_div2_clk = devm_clk_register(dev, &dwmac->fixed_div2.hw);
> +       if (WARN_ON(IS_ERR(dwmac->fixed_div2_clk)))
> +               return PTR_ERR(dwmac->fixed_div2_clk);
> +
> +       /* create the rgmii_tx_en */
> +       init.name = devm_kasprintf(dev, GFP_KERNEL, "%s#rgmii_tx_en",
> +                                  dev_name(dev));
> +       init.ops = &clk_gate_ops;
> +       init.flags = CLK_SET_RATE_PARENT;
> +       clk_div_parents[0] = __clk_get_name(dwmac->fixed_div2_clk);
> +       init.parent_names = clk_div_parents;
> +       init.num_parents = ARRAY_SIZE(clk_div_parents);
> +
> +       dwmac->rgmii_tx_en.reg = dwmac->regs + PRG_ETH0;
> +       dwmac->rgmii_tx_en.bit_idx = PRG_ETH0_RGMII_TX_CLK_EN;
> +       dwmac->rgmii_tx_en.hw.init = &init;
>
> -       dwmac->m25_div_clk = devm_clk_register(dev, &dwmac->m25_div.hw);
> -       if (WARN_ON(IS_ERR(dwmac->m25_div_clk)))
> -               return PTR_ERR(dwmac->m25_div_clk);
> +       dwmac->rgmii_tx_en_clk = devm_clk_register(dev,
> +                                                  &dwmac->rgmii_tx_en.hw);
> +       if (WARN_ON(IS_ERR(dwmac->rgmii_tx_en_clk)))
> +               return PTR_ERR(dwmac->rgmii_tx_en_clk);
>
>         return 0;
>  }
> @@ -200,18 +211,22 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
>                 meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TXDLY_MASK,
>                                         tx_dly_val << PRG_ETH0_TXDLY_SHIFT);
>
> -               ret = clk_prepare_enable(dwmac->m25_div_clk);
> +               /* Configure the 125MHz RGMII TX clock, the IP block changes
> +                * the output automatically (= without us having to configure
> +                * a register) based on the line-speed (125MHz for Gbit speeds,
> +                * 25MHz for 100Mbit/s and 2.5MHz for 10Mbit/s).
> +                */
> +               ret = clk_set_rate(dwmac->rgmii_tx_en_clk, 125 * 1000 * 1000);
>                 if (ret) {
> -                       dev_err(&dwmac->pdev->dev, "failed to enable the PHY clock\n");
> +                       dev_err(&dwmac->pdev->dev,
> +                               "failed to set RGMII TX clock\n");
>                         return ret;
>                 }
>
> -               /* Generate the 25MHz RGMII clock for the PHY */
> -               ret = clk_set_rate(dwmac->m25_div_clk, 25 * 1000 * 1000);
> +               ret = clk_prepare_enable(dwmac->rgmii_tx_en_clk);
>                 if (ret) {
> -                       clk_disable_unprepare(dwmac->m25_div_clk);
> -
> -                       dev_err(&dwmac->pdev->dev, "failed to set PHY clock\n");
> +                       dev_err(&dwmac->pdev->dev,
> +                               "failed to enable the RGMII TX clock\n");
>                         return ret;
>                 }
>                 break;
> @@ -305,7 +320,7 @@ static int meson8b_dwmac_probe(struct platform_device *pdev)
>
>  err_clk_disable:
>         if (phy_interface_mode_is_rgmii(dwmac->phy_mode))
> -               clk_disable_unprepare(dwmac->m25_div_clk);
> +               clk_disable_unprepare(dwmac->rgmii_tx_en_clk);
>  err_remove_config_dt:
>         stmmac_remove_config_dt(pdev, plat_dat);
>
> @@ -317,7 +332,7 @@ static int meson8b_dwmac_remove(struct platform_device *pdev)
>         struct meson8b_dwmac *dwmac = get_stmmac_bsp_priv(&pdev->dev);
>
>         if (phy_interface_mode_is_rgmii(dwmac->phy_mode))
> -               clk_disable_unprepare(dwmac->m25_div_clk);
> +               clk_disable_unprepare(dwmac->rgmii_tx_en_clk);
>
>         return stmmac_pltfr_remove(pdev);
>  }
> --
> 2.15.1
>

[0] http://lists.infradead.org/pipermail/linux-amlogic/2018-January/006153.html



More information about the linux-amlogic mailing list