Re: [PATCH] ARM: dts: imx6sx: document SION necessity of ENET1_REF_CLK1

[Fabio Estevam <festevam@xxxxxxxxx>]

Hi Christoph,

On Wed, Aug 17, 2016 at 6:25 AM, Christoph Fritz
<chf.fritz@xxxxxxxxxxxxxx> wrote:

> +/*
> + * SION bit is necessary for ENET1_REF_CLK1 (ENET2_REF_CLK2 untested) if it is
> + * used as clock output of IMX6SX_CLK_ENET_REF (ENET1_TX_CLK) to e.g. supply a
> + * PHY in RMII mode. This configuration is valid if:
> + *  - bit 1 in field IMX6SX_GPR1_FEC_CLOCK_PAD_DIR_MASK is set
> + *  - bit 1 in field IMX6SX_GPR1_FEC_CLOCK_MUX_SEL_MASK unset

So in your case the imx6sx_enet_clk_sel() does not do what you need:

static void __init imx6sx_enet_clk_sel(void)
{
    struct regmap *gpr;

    gpr = syscon_regmap_lookup_by_compatible("fsl,imx6sx-iomuxc-gpr");
    if (!IS_ERR(gpr)) {
        regmap_update_bits(gpr, IOMUXC_GPR1,
                   IMX6SX_GPR1_FEC_CLOCK_MUX_SEL_MASK, 0);
        regmap_update_bits(gpr, IOMUXC_GPR1,
                   IMX6SX_GPR1_FEC_CLOCK_PAD_DIR_MASK, 0);
    } else {
        pr_err("failed to find fsl,imx6sx-iomux-gpr regmap\n");
    }
}

It seems that it is not a good idea to have imx6sx_enet_clk_sel() in
common code as the GPR1 setting can change from board to board.

I think we need the fec driver to be in charge of configuring the GPR1 register.

This is unrelated to your patch though :-)
--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html