linux-kernel - [PATCH] Revert "clk: fix __clk_init_parent() for single parent clocks"

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Tue,  4 Dec 2018 17:32:57 +0100
From:   Jerome Brunet <jbrunet@...libre.com>
To:     Stephen Boyd <sboyd@...nel.org>,
        Michael Turquette <mturquette@...libre.com>
Cc:     Jerome Brunet <jbrunet@...libre.com>, linux-clk@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Masahiro Yamada <yamada.masahiro@...ionext.com>
Subject: [PATCH] Revert "clk: fix __clk_init_parent() for single parent clocks"

This reverts commit 2430a94d1e719b7b4af2a65b781a4c036eb22e64.

>From the original commit message:
  It turned out a problem because there are some single-parent clocks
  that implement .get_parent() callback and return non-zero index.
  The SOCFPGA clock is the case; the commit broke the SOCFPGA boards.

It is wrong for a clock to return an index >= num_parents. CCF checks
for this condition in clk_core_get_parent_by_index(). This function sets
the parent to NULL if index is incoherent, which seems like the only
sane choice.

commit 2430a94d1e71 ("clk: fix __clk_init_parent() for single parent clocks")
appears to be a work around installed in the core framework for a problem
that is platform specific, and should probably be fixed in the platform code.

Further more, it introduces a problem in a corner case of the mux clock.
Take mux with multiple parents, but only one is known, the rest being
undocumented. The register reset has one of unknown parent set.

Before commit 2430a94d1e71 ("clk: fix __clk_init_parent() for single parent clocks"):
 * get_parent() is called, register is read and give an unknown index.
   -> the mux is orphaned.
 * a call to set_rate() will reparent the mux to the only known parent.

With commit 2430a94d1e71 ("clk: fix __clk_init_parent() for single parent clocks"):
 * the register is never read.
 * parent is wrongly assumed to be the only known one.
   As a consequence, all the calculation deriving from the mux will be
   wrong
 * Since we believe the only know parent to be set, set_parent() won't
   ever be called and the register won't be set with the correct value.

Signed-off-by: Jerome Brunet <jbrunet@...libre.com>
---
 drivers/clk/clk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index af011974d4ec..1b33ef3c37f4 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2246,7 +2246,7 @@ static struct clk_core *__clk_init_parent(struct clk_core *core)
 {
 	u8 index = 0;

-	if (core->num_parents > 1 && core->ops->get_parent)
+	if (core->ops->get_parent)
 		index = core->ops->get_parent(core->hw);

 	return clk_core_get_parent_by_index(core, index);
-- 
2.19.1