| Message ID | 20180501214555.11476-1-martin.blumenstingl@googlemail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Tue, May 1, 2018 at 11:45 PM, Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote: > Until commit 05f814402d6174 ("clk: meson: add fdiv clock gates") we > relied on the bootloader to enable the fclk_div clock gates. It turns > out that our clock tree is incomplete at least on Meson8b (tested with > an Odroid-C1, which uses an RGMII PHY) because after the mentioned > commit Ethernet is not working anymore (no RX/TX activity can be seen). > At the same time Ethernet was still working on Meson8m2 with a RMII PHY. > > It is currently not clear which of the fclk_div gates is required for > (RGMII) Ethernet operation on Meson8b and why. Mark the fclk_div gates > as CLK_IS_CRITICAL so the common clock framework keeps these gates > enabled for us until we know which clock is required for Ethernet on > Meson8b and which driver has to claim it. > > Fixes: 05f814402d6174 ("clk: meson: add fdiv clock gates") > Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> Tested-by: Kevin Hilman <khilman@baylibre.com> quote from the discussion on IRC: <khilman> xdarklight: is ethernet on meson8b-odroidc1 working well for you on my v4.18/integ branch? <khilman> for me, the kernel boots, and can even DHCP when I add "ip=dhcp" to the cmdline. <khilman> but when I get to a shell (ramdisk), I can't ping anything. 100% packet loss <xdarklight> that sounds familiar and https://patchwork.kernel.org/patch/10374537/ "fixes" it for me <khilman> [...] NFSroot working now on odroidc1 <khilman> I added that to my "depends" branch (included in integ) until it gets merged. > --- > drivers/clk/meson/meson8b.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c > index d0524ec71aad..f3ac099ee22c 100644 > --- a/drivers/clk/meson/meson8b.c > +++ b/drivers/clk/meson/meson8b.c > @@ -246,6 +246,7 @@ static struct clk_regmap meson8b_fclk_div2 = { > .ops = &clk_regmap_gate_ops, > .parent_names = (const char *[]){ "fclk_div2_div" }, > .num_parents = 1, > + .flags = CLK_IS_CRITICAL, > }, > }; > > @@ -270,6 +271,7 @@ static struct clk_regmap meson8b_fclk_div3 = { > .ops = &clk_regmap_gate_ops, > .parent_names = (const char *[]){ "fclk_div3_div" }, > .num_parents = 1, > + .flags = CLK_IS_CRITICAL, > }, > }; > > @@ -294,6 +296,7 @@ static struct clk_regmap meson8b_fclk_div4 = { > .ops = &clk_regmap_gate_ops, > .parent_names = (const char *[]){ "fclk_div4_div" }, > .num_parents = 1, > + .flags = CLK_IS_CRITICAL, > }, > }; > > @@ -318,6 +321,7 @@ static struct clk_regmap meson8b_fclk_div5 = { > .ops = &clk_regmap_gate_ops, > .parent_names = (const char *[]){ "fclk_div5_div" }, > .num_parents = 1, > + .flags = CLK_IS_CRITICAL, > }, > }; > > @@ -342,6 +346,7 @@ static struct clk_regmap meson8b_fclk_div7 = { > .ops = &clk_regmap_gate_ops, > .parent_names = (const char *[]){ "fclk_div7_div" }, > .num_parents = 1, > + .flags = CLK_IS_CRITICAL, > }, > }; > > -- > 2.17.0 >
On Tue, 2018-05-01 at 23:55 +0200, Martin Blumenstingl wrote: > Hello Yixun, > > On Tue, May 1, 2018 at 11:45 PM, Martin Blumenstingl > <martin.blumenstingl@googlemail.com> wrote: > > Until commit 05f814402d6174 ("clk: meson: add fdiv clock gates") we > > relied on the bootloader to enable the fclk_div clock gates. It turns > > out that our clock tree is incomplete at least on Meson8b (tested with > > an Odroid-C1, which uses an RGMII PHY) because after the mentioned > > commit Ethernet is not working anymore (no RX/TX activity can be seen). > > At the same time Ethernet was still working on Meson8m2 with a RMII PHY. > > > > It is currently not clear which of the fclk_div gates is required for > > (RGMII) Ethernet operation on Meson8b and why. Mark the fclk_div gates > > as CLK_IS_CRITICAL so the common clock framework keeps these gates > > enabled for us until we know which clock is required for Ethernet on > > Meson8b and which driver has to claim it. > > based on the public datasheet for Meson8b (S805) I cannot see which > part of the SoC uses fclk_divX for (RGMII) Ethernet. > I did some further tests and it seems that Ethernet is not working if > fclk_div2 is disabled (the other fclk_divX don't seem to affect > Ethernet operation) > > could you please check your internal datasheets and let us know how > fclk_div2 is routed internally and how it can affect Ethernet data > transfers? Hi Martin, As usual, thanks for your thorough work ! Now that you know which of the fdiv is actually required, I would prefer if we could mark only the required clock as critical. Also, could you please add a short FIXME comment in the code explaining why this is necessary. This should a be temporary fix while we get the ethernet driver sorted out. If the ethernet of the meson8b requires the fdiv2 it should claim it (through other clock if necessary) > > (my current speculation is that it may be related to the "Ethernet > Memory Power Domain" in CBUS HHI_MEM_PD_REG0, but I have no proof for > that)
On Mon, May 14, 2018 at 12:03 PM, Jerome Brunet <jbrunet@baylibre.com> wrote: > On Tue, 2018-05-01 at 23:55 +0200, Martin Blumenstingl wrote: >> Hello Yixun, >> >> On Tue, May 1, 2018 at 11:45 PM, Martin Blumenstingl >> <martin.blumenstingl@googlemail.com> wrote: >> > Until commit 05f814402d6174 ("clk: meson: add fdiv clock gates") we >> > relied on the bootloader to enable the fclk_div clock gates. It turns >> > out that our clock tree is incomplete at least on Meson8b (tested with >> > an Odroid-C1, which uses an RGMII PHY) because after the mentioned >> > commit Ethernet is not working anymore (no RX/TX activity can be seen). >> > At the same time Ethernet was still working on Meson8m2 with a RMII PHY. >> > >> > It is currently not clear which of the fclk_div gates is required for >> > (RGMII) Ethernet operation on Meson8b and why. Mark the fclk_div gates >> > as CLK_IS_CRITICAL so the common clock framework keeps these gates >> > enabled for us until we know which clock is required for Ethernet on >> > Meson8b and which driver has to claim it. >> >> based on the public datasheet for Meson8b (S805) I cannot see which >> part of the SoC uses fclk_divX for (RGMII) Ethernet. >> I did some further tests and it seems that Ethernet is not working if >> fclk_div2 is disabled (the other fclk_divX don't seem to affect >> Ethernet operation) >> >> could you please check your internal datasheets and let us know how >> fclk_div2 is routed internally and how it can affect Ethernet data >> transfers? > > Hi Martin, > > As usual, thanks for your thorough work ! > > Now that you know which of the fdiv is actually required, I would prefer if we > could mark only the required clock as critical. OK, I'll re-send the patch this weekend > Also, could you please add a short FIXME comment in the code explaining why this > is necessary. This should a be temporary fix while we get the ethernet driver > sorted out. If the ethernet of the meson8b requires the fdiv2 it should claim it > (through other clock if necessary) I'll include the "FIXME" comment and I totally agree that this should only be a temporary workaround Regards Martin
diff --git a/drivers/clk/meson/meson8b.c b/drivers/clk/meson/meson8b.c index d0524ec71aad..f3ac099ee22c 100644 --- a/drivers/clk/meson/meson8b.c +++ b/drivers/clk/meson/meson8b.c @@ -246,6 +246,7 @@ static struct clk_regmap meson8b_fclk_div2 = { .ops = &clk_regmap_gate_ops, .parent_names = (const char *[]){ "fclk_div2_div" }, .num_parents = 1, + .flags = CLK_IS_CRITICAL, }, }; @@ -270,6 +271,7 @@ static struct clk_regmap meson8b_fclk_div3 = { .ops = &clk_regmap_gate_ops, .parent_names = (const char *[]){ "fclk_div3_div" }, .num_parents = 1, + .flags = CLK_IS_CRITICAL, }, }; @@ -294,6 +296,7 @@ static struct clk_regmap meson8b_fclk_div4 = { .ops = &clk_regmap_gate_ops, .parent_names = (const char *[]){ "fclk_div4_div" }, .num_parents = 1, + .flags = CLK_IS_CRITICAL, }, }; @@ -318,6 +321,7 @@ static struct clk_regmap meson8b_fclk_div5 = { .ops = &clk_regmap_gate_ops, .parent_names = (const char *[]){ "fclk_div5_div" }, .num_parents = 1, + .flags = CLK_IS_CRITICAL, }, }; @@ -342,6 +346,7 @@ static struct clk_regmap meson8b_fclk_div7 = { .ops = &clk_regmap_gate_ops, .parent_names = (const char *[]){ "fclk_div7_div" }, .num_parents = 1, + .flags = CLK_IS_CRITICAL, }, };
Until commit 05f814402d6174 ("clk: meson: add fdiv clock gates") we relied on the bootloader to enable the fclk_div clock gates. It turns out that our clock tree is incomplete at least on Meson8b (tested with an Odroid-C1, which uses an RGMII PHY) because after the mentioned commit Ethernet is not working anymore (no RX/TX activity can be seen). At the same time Ethernet was still working on Meson8m2 with a RMII PHY. It is currently not clear which of the fclk_div gates is required for (RGMII) Ethernet operation on Meson8b and why. Mark the fclk_div gates as CLK_IS_CRITICAL so the common clock framework keeps these gates enabled for us until we know which clock is required for Ethernet on Meson8b and which driver has to claim it. Fixes: 05f814402d6174 ("clk: meson: add fdiv clock gates") Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com> --- drivers/clk/meson/meson8b.c | 5 +++++ 1 file changed, 5 insertions(+)