| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAFBinCDxrxuow-e3aofu6k0ShQdCehVbAJH_u3b26RKN6D0Hug@mail.gmail.com>
Date: Tue, 16 Jan 2018 12:20:27 +0100
From: Martin Blumenstingl <martin.blumenstingl@...glemail.com>
To: netdev@...r.kernel.org, ingrassia@...genesys.com
Cc: linus.luessing@...3.blue, khilman@...libre.com,
linux-amlogic@...ts.infradead.org, jbrunet@...libre.com,
Neil Armstrong <narmstrong@...libre.com>,
peppe.cavallaro@...com, alexandre.torgue@...com,
Martin Blumenstingl <martin.blumenstingl@...glemail.com>
Subject: Re: [RFT net-next v4 3/5] net: stmmac: dwmac-meson8b: fix internal
RGMII clock configuration
On Sun, Jan 14, 2018 at 10:48 PM, Martin Blumenstingl
<martin.blumenstingl@...glemail.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@...genesys.com>
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@...glemail.com>
NACK-ed by: Yixun Lan <yixun.lan@...ogic.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
Powered by blists - more mailing lists