clk: qoriq: Move chip-specific knowledge into driver

The device tree should describe the chips (or chip-like subblocks) in the system, but it generally does not describe individual registers -- it should identify, rather than describe, a programming interface.

This has not been the case with the QorIQ clockgen nodes. The knowledge of what each bit setting of CLKCnCSR means is encoded in three places (binding, pll node, and mux node), and the last also needs to know which options are valid on a particular chip. All three of these locations are considered stable ABI, making it difficult to fix mistakes (of which I have found several), much less refactor the abstraction to be able to address problems, limitations, or new chips.

Under the current binding, a pll clock specifier of 2 means that the PLL is divided by 4 -- and the driver implements this, unless there happen to be four clock-output-names rather than 3, in which case it interprets it as PLL divided by 3. This does not appear in the binding documentation at all. That hack is now considered stable ABI.

The current device tree nodes contain errors, such as saying that T1040 can set a core clock to PLL/4 when only PLL and PLL/2 are options. The current binding also ignores some restrictions on clock selection, such as p5020's requirement that if a core uses the "wrong" PLL, that PLL must be clocked lower than the "correct" PLL and be at most 80% of the rated CPU frequency.

Possibly because of the lack of the ability to express such nuance in the binding, some valid options are omitted from the device trees, such as the ability on p4080 to run cores 0-3 from PLL3 and cores 4-7 from PLL1 (again, only if they are at most 80% of rated CPU frequency). This omission, combined with excessive caution in the cpufreq driver (addressed in a subsequent patch), means that currently on a 1500 MHz p4080 with typical PLL configuration, cpufreq can lower the frequency to 1200 MHz on half the CPUs and do nothing on the others. With this patchset, all CPUs can be lowered to 1200 MHz on a rev2 p4080, and on a rev3 p4080 half can be lowered to 750 MHz and the other half to 600 MHz.

The current binding only deals with CPU clocks. To describe FMan in the device tree, we need to describe its clock. Some chips have additional muxes that work like the CPU muxes, but are not described in the device tree. Others require inspecting the Reset Control Word to determine which PLL is used. Rather than continue to extend this mess, replace it. Have the driver bind to the chip-specific clockgen compatible, and keep the detailed description of quirky chip variations in the driver, where it can be easily fixed, refactored, and extended.

Older device trees will continue to work (including a workaround for old ls1021a device trees that are missing compatible and reg in the clockgen node, which even the old binding required). The pll/mux details in old device trees will be ignored, but "clocks" properties pointing at the old nodes will still work, and be directed at the corresponding new clock.

Signed-off-by Scott Wood <[email protected]>

---

LS2080A is the first implementation of the chassis 3 clockgen, which has a different register layout than previous chips. It is also little endian, unlike previous chips.

Signed-off-by Scott Wood <[email protected]>

---

Commit fc4a05d4b0eb ("clk: Remove unused provider APIs") removed __clk_get_num_parents() and clk_hw_get_parent_by_index(), leaving only true provider API versions that operate on struct clk_hw.

qoriq-cpufreq needs these functions in order to determine the options it has for calling clk_set_parent() and thus populate the cpufreq table, so revive them as legitimate consumer APIs.

Signed-off-by Scott Wood <[email protected]>

---
v3: new patch

 drivers/clk/clk.c   | 19 +++++++++++++++++++
 include/linux/clk.h | 31 +++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

drivers/clk/clk.c +19 -0

index 43e2c3a..9436356 100644
@@ -290,6 +290,12 @@ struct clk_hw *__clk_get_hw(struct clk *clk)
 }
 EXPORT_SYMBOL_GPL(__clk_get_hw);
 
+unsigned int clk_get_num_parents(struct clk *clk)
+{
+     return !clk ? 0 : clk->core->num_parents;
+}
+EXPORT_SYMBOL_GPL(clk_get_num_parents);
+
 unsigned int clk_hw_get_num_parents(const struct clk_hw *hw)
 {
      return hw->core->num_parents;

@@ -359,6 +365,19 @@ static struct clk_core 
 *clk_core_get_parent_by_index(struct clk_core *core,
              return core->parents[index];
 }
 
+struct clk *clk_get_parent_by_index(struct clk *clk, unsigned int index)
+{
+     struct clk_core *parent;
+
+     if (!clk)
+             return NULL;
+
+     parent = clk_core_get_parent_by_index(clk->core, index);
+
+     return !parent ? NULL : parent->hw->clk;
+}
+EXPORT_SYMBOL_GPL(clk_get_parent_by_index);
+
 struct clk_hw *
 clk_hw_get_parent_by_index(const struct clk_hw *hw, unsigned int index)

{

include/linux/clk.h +20 -0

index 0df4a51..937de0e 100644
@@ -392,6 +392,26 @@ int clk_set_parent(struct clk *clk, struct clk 
 *parent);
 struct clk *clk_get_parent(struct clk *clk);
 
 /**
+ * clk_get_parent_by_index - get a possible parent clock by index
+ * @clk: clock source
+ * @index: index into the array of possible parents of this clock
+ *
+ * Returns struct clk corresponding to the requested possible
+ * parent clock source, or NULL.
+ */
+struct clk *clk_get_parent_by_index(struct clk *clk,
+                                 unsigned int index);
+
+/**
+ * clk_get_num_parents - get number of possible parents
+ * @clk: clock source
+ *
+ * Returns the number of possible parents of this clock,
+ * which can then be enumerated using clk_get_parent_by_index().
+ */
+unsigned int clk_get_num_parents(struct clk *clk);
+
+/**
  * clk_get_sys - get a clock based upon the device name
  * @dev_id: device name

* @con_id: connection ID

include/linux/clk.h +11 -0

@@ -461,6 +481,17 @@ static inline struct clk *clk_get_parent(struct clk 
 *clk)
      return NULL;
 }
 
+struct clk *clk_get_parent_by_index(struct clk *clk,
+                                 unsigned int index)
+{
+     return NULL;
+}
+
+unsigned int clk_get_num_parents(struct clk *clk)
+{
+     return 0;
+}
+
 #endif
 

/* clk_prepare_enable helps cases using clk_enable in non-atomic context.
*/

Quoted 12 lines · Scott Wood

Commit fc4a05d4b0eb ("clk: Remove unused provider APIs") removed __clk_get_num_parents() and clk_hw_get_parent_by_index(), leaving only true provider API versions that operate on struct clk_hw.

qoriq-cpufreq needs these functions in order to determine the options it has for calling clk_set_parent() and thus populate the cpufreq table, so revive them as legitimate consumer APIs.

Signed-off-by Scott Wood <[email protected]>

---
v3: new patch

Quoted 3 lines

 drivers/clk/clk.c   | 19 +++++++++++++++++++
 include/linux/clk.h | 31 +++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

Russell, could you ACK this if there are no objections?

Thanks,
Scott

Quoted 91 lines

drivers/clk/clk.c +19 -0

index 43e2c3a..9436356 100644
@@ -290,6 +290,12 @@ struct clk_hw *__clk_get_hw(struct clk *clk)
 }
 EXPORT_SYMBOL_GPL(__clk_get_hw);
 
+unsigned int clk_get_num_parents(struct clk *clk)
+{
+     return !clk ? 0 : clk->core->num_parents;
+}
+EXPORT_SYMBOL_GPL(clk_get_num_parents);
+
 unsigned int clk_hw_get_num_parents(const struct clk_hw *hw)
 {
      return hw->core->num_parents;

@@ -359,6 +365,19 @@ static struct clk_core 
 *clk_core_get_parent_by_index(struct clk_core *core,
              return core->parents[index];
 }
 
+struct clk *clk_get_parent_by_index(struct clk *clk, unsigned int index)
+{
+     struct clk_core *parent;
+
+     if (!clk)
+             return NULL;
+
+     parent = clk_core_get_parent_by_index(clk->core, index);
+
+     return !parent ? NULL : parent->hw->clk;
+}
+EXPORT_SYMBOL_GPL(clk_get_parent_by_index);
+
 struct clk_hw *
 clk_hw_get_parent_by_index(const struct clk_hw *hw, unsigned int index)

{

include/linux/clk.h +20 -0

index 0df4a51..937de0e 100644
@@ -392,6 +392,26 @@ int clk_set_parent(struct clk *clk, struct clk 
 *parent);
 struct clk *clk_get_parent(struct clk *clk);
 
 /**
+ * clk_get_parent_by_index - get a possible parent clock by index
+ * @clk: clock source
+ * @index: index into the array of possible parents of this clock
+ *
+ * Returns struct clk corresponding to the requested possible
+ * parent clock source, or NULL.
+ */
+struct clk *clk_get_parent_by_index(struct clk *clk,
+                                 unsigned int index);
+
+/**
+ * clk_get_num_parents - get number of possible parents
+ * @clk: clock source
+ *
+ * Returns the number of possible parents of this clock,
+ * which can then be enumerated using clk_get_parent_by_index().
+ */
+unsigned int clk_get_num_parents(struct clk *clk);
+
+/**
  * clk_get_sys - get a clock based upon the device name
  * @dev_id: device name

* @con_id: connection ID

include/linux/clk.h +11 -0

@@ -461,6 +481,17 @@ static inline struct clk *clk_get_parent(struct clk 
 *clk)
      return NULL;
 }
 
+struct clk *clk_get_parent_by_index(struct clk *clk,
+                                 unsigned int index)
+{
+     return NULL;
+}
+
+unsigned int clk_get_num_parents(struct clk *clk)
+{
+     return 0;
+}
+
 #endif
 

/* clk_prepare_enable helps cases using clk_enable in non-atomic context.

Get the CPU clock's potential parent clocks from the clock interface itself, rather than manually parsing the clocks property to find a phandle, looking at the clock-names property of that, and assuming that those are valid parent clocks for the cpu clock.

This is necessary now that the clocks are generated based on the clock driver's knowledge of the chip rather than a fragile device-tree description of the mux options.

We can now rely on the clock driver to ensure that the mux only exposes options that are valid. The cpufreq driver was currently being overly conservative in some cases -- for example, the "min_cpufreq = get_bus_freq()" restriction only applies to chips with erratum A-004510, and whether the freq_mask used on p5020 is needed depends on the actual frequencies of the PLLs (FWIW, p5040 has a similar limitation but its .freq_mask was zero) -- and the frequency mask mechanism made assumptions about particular parent clock indices that are no longer valid.

Signed-off-by Scott Wood <[email protected]>

---
v3: was patch 1/5 and patch 4/5, plus blacklist e6500 and changes to clk api usage

 drivers/cpufreq/qoriq-cpufreq.c | 137 ++++++++++++----------------------------
 1 file changed, 40 insertions(+), 97 deletions(-)

Quoted 24 lines · Scott Wood

Get the CPU clock's potential parent clocks from the clock interface itself, rather than manually parsing the clocks property to find a phandle, looking at the clock-names property of that, and assuming that those are valid parent clocks for the cpu clock.

This is necessary now that the clocks are generated based on the clock driver's knowledge of the chip rather than a fragile device-tree description of the mux options.

We can now rely on the clock driver to ensure that the mux only exposes options that are valid. The cpufreq driver was currently being overly conservative in some cases -- for example, the "min_cpufreq = get_bus_freq()" restriction only applies to chips with erratum A-004510, and whether the freq_mask used on p5020 is needed depends on the actual frequencies of the PLLs (FWIW, p5040 has a similar limitation but its .freq_mask was zero) -- and the frequency mask mechanism made assumptions about particular parent clock indices that are no longer valid.

Signed-off-by Scott Wood <[email protected]>

---
v3: was patch 1/5 and patch 4/5, plus blacklist e6500 and changes to clk api usage

Quoted 2 lines

 drivers/cpufreq/qoriq-cpufreq.c | 137 ++++++++++++----------------------------
 1 file changed, 40 insertions(+), 97 deletions(-)

Acked-by Viresh Kumar <[email protected]>

Quoted 25 lines · Viresh Kumar

Quoted 24 lines · Scott Wood

Get the CPU clock's potential parent clocks from the clock interface itself, rather than manually parsing the clocks property to find a phandle, looking at the clock-names property of that, and assuming that those are valid parent clocks for the cpu clock.

This is necessary now that the clocks are generated based on the clock driver's knowledge of the chip rather than a fragile device-tree description of the mux options.

We can now rely on the clock driver to ensure that the mux only exposes options that are valid. The cpufreq driver was currently being overly conservative in some cases -- for example, the "min_cpufreq = get_bus_freq()" restriction only applies to chips with erratum A-004510, and whether the freq_mask used on p5020 is needed depends on the actual frequencies of the PLLs (FWIW, p5040 has a similar limitation but its .freq_mask was zero) -- and the frequency mask mechanism made assumptions about particular parent clock indices that are no longer valid.

Signed-off-by Scott Wood <[email protected]>

---
v3: was patch 1/5 and patch 4/5, plus blacklist e6500 and changes to clk api usage

Quoted 5 lines

Quoted 3 lines

 drivers/cpufreq/qoriq-cpufreq.c | 137 ++++++++++++---------------------
-------
 1 file changed, 40 insertions(+), 97 deletions(-)

Acked-by Viresh Kumar <[email protected]>

I'm wondering who's supposed to be merging this set?

Quoted 30 lines · Rafael J. Wysocki

Quoted 29 lines · Viresh Kumar

Quoted 27 lines · Scott Wood

Get the CPU clock's potential parent clocks from the clock interface itself, rather than manually parsing the clocks property to find a phandle, looking at the clock-names property of that, and assuming that those are valid parent clocks for the cpu clock.

This is necessary now that the clocks are generated based on the clock driver's knowledge of the chip rather than a fragile device-tree description of the mux options.

We can now rely on the clock driver to ensure that the mux only exposes options that are valid. The cpufreq driver was currently being overly conservative in some cases -- for example, the "min_cpufreq = get_bus_freq()" restriction only applies to chips with erratum A-004510, and whether the freq_mask used on p5020 is needed depends on the actual frequencies of the PLLs (FWIW, p5040 has a similar limitation but its .freq_mask was zero) -- and the frequency mask mechanism made assumptions about particular parent clock indices that are no longer valid.

Signed-off-by Scott Wood <[email protected]>

---
v3: was patch 1/5 and patch 4/5, plus blacklist e6500 and changes to clk api usage

 drivers/cpufreq/qoriq-cpufreq.c | 137 ++++++++++++---------------------
-------
 1 file changed, 40 insertions(+), 97 deletions(-)

Quoted 3 lines

Quoted 1 line

Acked-by Viresh Kumar <[email protected]>

I'm wondering who's supposed to be merging this set?

As I noted in the cover letter, I'm looking for acks so that I can apply these to a topic branch which can be pulled through the PPC and ARM trees, each of which will have patches that depend on it.

Quoted 33 lines · Scott Wood

Quoted 32 lines · Rafael J. Wysocki

Quoted 30 lines · Viresh Kumar

Quoted 19 lines · Scott Wood

Get the CPU clock's potential parent clocks from the clock interface itself, rather than manually parsing the clocks property to find a phandle, looking at the clock-names property of that, and assuming that those are valid parent clocks for the cpu clock.

This is necessary now that the clocks are generated based on the clock driver's knowledge of the chip rather than a fragile device-tree description of the mux options.

We can now rely on the clock driver to ensure that the mux only exposes options that are valid. The cpufreq driver was currently being overly conservative in some cases -- for example, the "min_cpufreq = get_bus_freq()" restriction only applies to chips with erratum A-004510, and whether the freq_mask used on p5020 is needed depends on the actual frequencies of the PLLs (FWIW, p5040 has a similar limitation but its .freq_mask was zero) -- and the frequency mask mechanism made assumptions about particular parent clock indices that are no longer valid.

Quoted 8 lines

Signed-off-by Scott Wood <[email protected]>

---
v3: was patch 1/5 and patch 4/5, plus blacklist e6500 and changes to clk api usage

 drivers/cpufreq/qoriq-cpufreq.c | 137 ++++++++++++---------------------
-------
 1 file changed, 40 insertions(+), 97 deletions(-)

Acked-by Viresh Kumar <[email protected]>

Quoted 5 lines

Quoted 1 line

I'm wondering who's supposed to be merging this set?

As I noted in the cover letter, I'm looking for acks so that I can apply these to a topic branch which can be pulled through the PPC and ARM trees, each of which will have patches that depend on it.

OK, so no objections from the cpufreq side and you have the ACK from Viresh.

Quoted 49 lines · Rafael J. Wysocki

Quoted 47 lines · Scott Wood

Quoted 33 lines · Rafael J. Wysocki

Quoted 32 lines · Viresh Kumar

Quoted 31 lines · Scott Wood

Get the CPU clock's potential parent clocks from the clock interface
itself, rather than manually parsing the clocks property to find a phandle, looking at the clock-names property of that, and assuming that
those are valid parent clocks for the cpu clock.

This is necessary now that the clocks are generated based on the clock
driver's knowledge of the chip rather than a fragile device-tree description of the mux options.

We can now rely on the clock driver to ensure that the mux only exposes
options that are valid. The cpufreq driver was currently being overly
conservative in some cases -- for example, the "min_cpufreq = get_bus_freq()" restriction only applies to chips with erratum A-004510, and whether the freq_mask used on p5020 is needed depends on
the actual frequencies of the PLLs (FWIW, p5040 has a similar limitation but its .freq_mask was zero) -- and the frequency mask mechanism made assumptions about particular parent clock indices that
are no longer valid.

Signed-off-by Scott Wood <[email protected]>

---
v3: was patch 1/5 and patch 4/5, plus blacklist e6500 and changes to clk api usage

Quoted 8 lines

Quoted 6 lines

Quoted 4 lines

 drivers/cpufreq/qoriq-cpufreq.c | 137 ++++++++++++---------------
------
-------
 1 file changed, 40 insertions(+), 97 deletions(-)

Acked-by Viresh Kumar <[email protected]>

I'm wondering who's supposed to be merging this set?

As I noted in the cover letter, I'm looking for acks so that I can apply these to a topic branch which can be pulled through the PPC and ARM trees,
each of which will have patches that depend on it.

Quoted 2 lines

OK, so no objections from the cpufreq side and you have the ACK from Viresh.

Hi Scott,

Did you drop this patch later? I can not find it in 4.5-rc still.

Quoted 54 lines · Li Yang

Quoted 51 lines · Rafael J. Wysocki

Quoted 39 lines · Scott Wood

Quoted 38 lines · Rafael J. Wysocki

Quoted 37 lines · Viresh Kumar

Quoted 35 lines · Scott Wood

Get the CPU clock's potential parent clocks from the clock interface
itself, rather than manually parsing the clocks property to find a phandle, looking at the clock-names property of that, and assuming that
those are valid parent clocks for the cpu clock.

This is necessary now that the clocks are generated based on the clock
driver's knowledge of the chip rather than a fragile device-tree description of the mux options.

We can now rely on the clock driver to ensure that the mux only exposes
options that are valid. The cpufreq driver was currently being overly
conservative in some cases -- for example, the "min_cpufreq = get_bus_freq()" restriction only applies to chips with erratum A-004510, and whether the freq_mask used on p5020 is needed depends on
the actual frequencies of the PLLs (FWIW, p5040 has a similar limitation but its .freq_mask was zero) -- and the frequency mask mechanism made assumptions about particular parent clock indices that
are no longer valid.

Signed-off-by Scott Wood <[email protected]>

---
v3: was patch 1/5 and patch 4/5, plus blacklist e6500 and changes to clk api usage

 drivers/cpufreq/qoriq-cpufreq.c | 137 ++++++++++++---------------
------
-------
 1 file changed, 40 insertions(+), 97 deletions(-)

Quoted 8 lines

Quoted 3 lines

Quoted 1 line

Acked-by Viresh Kumar <[email protected]>

I'm wondering who's supposed to be merging this set?

As I noted in the cover letter, I'm looking for acks so that I can apply these to a topic branch which can be pulled through the PPC and ARM trees,
each of which will have patches that depend on it.

OK, so no objections from the cpufreq side and you have the ACK from Viresh.

Quoted 3 lines

Hi Scott,

Did you drop this patch later? I can not find it in 4.5-rc still.

I was asked to get an ack from Russell King patch 4/5, which this patch requires. Despite repeated pings, I could not get a reply from Russell King.

Quoted 68 lines · Scott Wood

Quoted 67 lines · Li Yang

Quoted 54 lines · Rafael J. Wysocki

Quoted 53 lines · Scott Wood

Quoted 51 lines · Rafael J. Wysocki

Quoted 41 lines · Viresh Kumar

Quoted 40 lines · Scott Wood

Get the CPU clock's potential parent clocks from the clock interface
itself, rather than manually parsing the clocks property to find a
phandle, looking at the clock-names property of that, and assuming
that
those are valid parent clocks for the cpu clock.

This is necessary now that the clocks are generated based on the
clock
driver's knowledge of the chip rather than a fragile device -tree
description of the mux options.

We can now rely on the clock driver to ensure that the mux only
exposes
options that are valid. The cpufreq driver was currently being
overly
conservative in some cases -- for example, the "min_cpufreq = get_bus_freq()" restriction only applies to chips with erratum A-004510, and whether the freq_mask used on p5020 is needed depends on
the actual frequencies of the PLLs (FWIW, p5040 has a similar limitation but its .freq_mask was zero) -- and the frequency mask
mechanism made assumptions about particular parent clock indices
that
are no longer valid.

Signed-off-by Scott Wood <[email protected]>

---
v3: was patch 1/5 and patch 4/5, plus blacklist e6500 and changes
to clk api usage

Quoted 7 lines

Quoted 5 lines

 drivers/cpufreq/qoriq-cpufreq.c | 137 ++++++++++++-----------
----
------
-------
 1 file changed, 40 insertions(+), 97 deletions(-)

Acked-by Viresh Kumar <[email protected]>

I'm wondering who's supposed to be merging this set?

Quoted 8 lines

Quoted 5 lines

As I noted in the cover letter, I'm looking for acks so that I can apply
these to a topic branch which can be pulled through the PPC and ARM trees,
each of which will have patches that depend on it.

OK, so no objections from the cpufreq side and you have the ACK from Viresh.

Hi Scott,

Quoted 5 lines

Quoted 1 line

Did you drop this patch later? I can not find it in 4.5-rc still.

I was asked to get an ack from Russell King patch 4/5, which this patch requires. Despite repeated pings, I could not get a reply from Russell King.

This patch? I think you should try to get ACK from clock maintainers
instead of Russell.

The existing device tree bindings are error-prone and inflexible. Correct the mistake by moving the knowledge into the driver, which has more flexibility in describing the quirks of each chip. This leaves the device tree to its proper role of identifying a programming interface rather than describing its individual registers.

For more detail, see the commit message of patch 2.

As there are both ARM and PPC patches that depend on this patchset, I'm requesting ACKs from clk and cpufreq in order to put this into a topic branch that both ARM and PPC can pull.

Scott Wood (5):
powerpc/fsl: Move fsl_guts.h out of arch/powerpc
clk: qoriq: Move chip-specific knowledge into driver
clk: qoriq: Add ls2080a support.
clk: Add consumer APIs for discovering possible parent clocks
cpufreq: qoriq: Don't look at clock implementation details