The Amlogic G12A SoCs embeds a dedicated PLL to feed the USB3+PCIE
Combo PHY. This PLL needs a very specific and strict register sequence
in order to correcly enable it and deliver the 100MHz reference clock
to the Analog PHY.
After lot of trials and errors, and since this PLL will ever feed 100MHz
with a static configuration, it is simpler to setup a dedicated ops
structure with a custom _enable() op applying the init register sequence.
The rate calculation ops are kept in order to keep the nominal read
ops as-in, but set_rate is removed.
With this setup, the PLL can be enabled and disable safely and always have
the recommended PLL setup to feed the USB3+PCIE Combo PHY.
Neil Armstrong (3):
clk: meson-pll: add reduced specific clk_ops for G12A PCIe PLL
dt-bindings: clk: g12a-clkc: add PCIE PLL clock ID
clk: meson-g12a: add PCIE PLL clocks
drivers/clk/meson/clk-pll.c | 26 ++++++
drivers/clk/meson/clk-pll.h | 1 +
drivers/clk/meson/g12a.c | 118 ++++++++++++++++++++++++++
drivers/clk/meson/g12a.h | 5 +-
include/dt-bindings/clock/g12a-clkc.h | 1 +
5 files changed, 150 insertions(+), 1 deletion(-)
--
2.20.1
The Meson G12A PCIE PLL is fined tuned to deliver a very precise
100MHz reference clock for the PCIe Analog PHY, and thus requires
a strict register sequence to enable the PLL.
To simplify, use the _init() op to enable the PLL and keep
the other ops except set_rate since the rate is fixed.
Signed-off-by: Neil Armstrong <[email protected]>
---
drivers/clk/meson/clk-pll.c | 26 ++++++++++++++++++++++++++
drivers/clk/meson/clk-pll.h | 1 +
2 files changed, 27 insertions(+)
diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c
index 41e16dd7272a..6a88dd75ccf0 100644
--- a/drivers/clk/meson/clk-pll.c
+++ b/drivers/clk/meson/clk-pll.c
@@ -303,6 +303,16 @@ static int meson_clk_pll_is_enabled(struct clk_hw *hw)
return 1;
}
+static int meson_clk_pcie_pll_enable(struct clk_hw *hw)
+{
+ meson_clk_pll_init(hw);
+
+ if (meson_clk_pll_wait_lock(hw))
+ return -EIO;
+
+ return 0;
+}
+
static int meson_clk_pll_enable(struct clk_hw *hw)
{
struct clk_regmap *clk = to_clk_regmap(hw);
@@ -387,6 +397,22 @@ static int meson_clk_pll_set_rate(struct clk_hw *hw, unsigned long rate,
return 0;
}
+/*
+ * The Meson G12A PCIE PLL is fined tuned to deliver a very precise
+ * 100MHz reference clock for the PCIe Analog PHY, and thus requires
+ * a strict register sequence to enable the PLL.
+ * To simplify, re-use the _init() op to enable the PLL and keep
+ * the other ops except set_rate since the rate is fixed.
+ */
+const struct clk_ops meson_clk_pcie_pll_ops = {
+ .recalc_rate = meson_clk_pll_recalc_rate,
+ .round_rate = meson_clk_pll_round_rate,
+ .is_enabled = meson_clk_pll_is_enabled,
+ .enable = meson_clk_pcie_pll_enable,
+ .disable = meson_clk_pll_disable
+};
+EXPORT_SYMBOL_GPL(meson_clk_pcie_pll_ops);
+
const struct clk_ops meson_clk_pll_ops = {
.init = meson_clk_pll_init,
.recalc_rate = meson_clk_pll_recalc_rate,
diff --git a/drivers/clk/meson/clk-pll.h b/drivers/clk/meson/clk-pll.h
index 55af2e285b1b..367efd0f6410 100644
--- a/drivers/clk/meson/clk-pll.h
+++ b/drivers/clk/meson/clk-pll.h
@@ -45,5 +45,6 @@ struct meson_clk_pll_data {
extern const struct clk_ops meson_clk_pll_ro_ops;
extern const struct clk_ops meson_clk_pll_ops;
+extern const struct clk_ops meson_clk_pcie_pll_ops;
#endif /* __MESON_CLK_PLL_H */
--
2.20.1
Add the PCIe reference clock feeding the USB3 + PCIE combo PHY.
This PLL needs a very precise register sequence to permit to be locked,
thus using the specific clk-pll pcie ops.
The PLL is then followed by :
- a fixed /2 divider
- a 5-bit 1-based divider
- a final /2 divider
This reference clock is fixed to 100MHz, thus only a single PLL setup
is added.
Signed-off-by: Neil Armstrong <[email protected]>
---
drivers/clk/meson/g12a.c | 118 +++++++++++++++++++++++++++++++++++++++
drivers/clk/meson/g12a.h | 5 +-
2 files changed, 122 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
index 80a7172df2a6..d382c21a29e5 100644
--- a/drivers/clk/meson/g12a.c
+++ b/drivers/clk/meson/g12a.c
@@ -614,6 +614,118 @@ static struct clk_regmap g12a_hifi_pll = {
},
};
+/*
+ * The Meson G12A PCIE PLL is fined tuned to deliver a very precise
+ * 100MHz reference clock for the PCIe Analog PHY, and thus requires
+ * a strict register sequence to enable the PLL.
+ */
+static const struct reg_sequence g12a_pcie_pll_init_regs[] = {
+ { .reg = HHI_PCIE_PLL_CNTL0, .def = 0x20090496 },
+ { .reg = HHI_PCIE_PLL_CNTL0, .def = 0x30090496 },
+ { .reg = HHI_PCIE_PLL_CNTL1, .def = 0x00000000 },
+ { .reg = HHI_PCIE_PLL_CNTL2, .def = 0x00001100 },
+ { .reg = HHI_PCIE_PLL_CNTL3, .def = 0x10058e00 },
+ { .reg = HHI_PCIE_PLL_CNTL4, .def = 0x000100c0 },
+ { .reg = HHI_PCIE_PLL_CNTL5, .def = 0x68000048 },
+ { .reg = HHI_PCIE_PLL_CNTL5, .def = 0x68000068, .delay_us = 20 },
+ { .reg = HHI_PCIE_PLL_CNTL4, .def = 0x008100c0, .delay_us = 10 },
+ { .reg = HHI_PCIE_PLL_CNTL0, .def = 0x34090496 },
+ { .reg = HHI_PCIE_PLL_CNTL0, .def = 0x14090496, .delay_us = 10 },
+ { .reg = HHI_PCIE_PLL_CNTL2, .def = 0x00001000 },
+};
+
+/* Keep a single entry table for recalc/round_rate() ops */
+static const struct pll_params_table g12a_pcie_pll_table[] = {
+ PLL_PARAMS(150, 1),
+ {0, 0},
+};
+
+static struct clk_regmap g12a_pcie_pll_dco = {
+ .data = &(struct meson_clk_pll_data){
+ .en = {
+ .reg_off = HHI_PCIE_PLL_CNTL0,
+ .shift = 28,
+ .width = 1,
+ },
+ .m = {
+ .reg_off = HHI_PCIE_PLL_CNTL0,
+ .shift = 0,
+ .width = 8,
+ },
+ .n = {
+ .reg_off = HHI_PCIE_PLL_CNTL0,
+ .shift = 10,
+ .width = 5,
+ },
+ .frac = {
+ .reg_off = HHI_PCIE_PLL_CNTL1,
+ .shift = 0,
+ .width = 12,
+ },
+ .l = {
+ .reg_off = HHI_PCIE_PLL_CNTL0,
+ .shift = 31,
+ .width = 1,
+ },
+ .rst = {
+ .reg_off = HHI_PCIE_PLL_CNTL0,
+ .shift = 29,
+ .width = 1,
+ },
+ .table = g12a_pcie_pll_table,
+ .init_regs = g12a_pcie_pll_init_regs,
+ .init_count = ARRAY_SIZE(g12a_pcie_pll_init_regs),
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "pcie_pll_dco",
+ .ops = &meson_clk_pcie_pll_ops,
+ .parent_names = (const char *[]){ IN_PREFIX "xtal" },
+ .num_parents = 1,
+ },
+};
+
+static struct clk_fixed_factor g12a_pcie_pll_dco_div2 = {
+ .mult = 1,
+ .div = 2,
+ .hw.init = &(struct clk_init_data){
+ .name = "pcie_pll_dco_div2",
+ .ops = &clk_fixed_factor_ops,
+ .parent_names = (const char *[]){ "pcie_pll_dco" },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ },
+};
+
+static struct clk_regmap g12a_pcie_pll_od = {
+ .data = &(struct clk_regmap_div_data){
+ .offset = HHI_PCIE_PLL_CNTL0,
+ .shift = 16,
+ .width = 5,
+ .flags = CLK_DIVIDER_ROUND_CLOSEST |
+ CLK_DIVIDER_ONE_BASED |
+ CLK_DIVIDER_ALLOW_ZERO,
+ },
+ .hw.init = &(struct clk_init_data){
+ .name = "pcie_pll_od",
+ .ops = &clk_regmap_divider_ops,
+ .parent_names = (const char *[]){ "pcie_pll_dco_div2" },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ },
+};
+
+static struct clk_fixed_factor g12a_pcie_pll = {
+ .mult = 1,
+ .div = 2,
+ .hw.init = &(struct clk_init_data){
+ .name = "pcie_pll_pll",
+ .ops = &clk_fixed_factor_ops,
+ .parent_names = (const char *[]){ "pcie_pll_od" },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ },
+};
+
static struct clk_regmap g12a_hdmi_pll_dco = {
.data = &(struct meson_clk_pll_data){
.en = {
@@ -2499,6 +2611,10 @@ static struct clk_hw_onecell_data g12a_hw_onecell_data = {
[CLKID_CPU_CLK_AXI] = &g12a_cpu_clk_axi.hw,
[CLKID_CPU_CLK_TRACE_DIV] = &g12a_cpu_clk_trace_div.hw,
[CLKID_CPU_CLK_TRACE] = &g12a_cpu_clk_trace.hw,
+ [CLKID_PCIE_PLL_DCO] = &g12a_pcie_pll_dco.hw,
+ [CLKID_PCIE_PLL_DCO_DIV2] = &g12a_pcie_pll_dco_div2.hw,
+ [CLKID_PCIE_PLL_OD] = &g12a_pcie_pll_od.hw,
+ [CLKID_PCIE_PLL] = &g12a_pcie_pll.hw,
[NR_CLKS] = NULL,
},
.num = NR_CLKS,
@@ -2685,6 +2801,8 @@ static struct clk_regmap *const g12a_clk_regmaps[] = {
&g12a_cpu_clk_axi,
&g12a_cpu_clk_trace_div,
&g12a_cpu_clk_trace,
+ &g12a_pcie_pll_od,
+ &g12a_pcie_pll_dco,
};
static const struct meson_eeclkc_data g12a_clkc_data = {
diff --git a/drivers/clk/meson/g12a.h b/drivers/clk/meson/g12a.h
index 70aa469ca1cf..1393a09730a6 100644
--- a/drivers/clk/meson/g12a.h
+++ b/drivers/clk/meson/g12a.h
@@ -186,8 +186,11 @@
#define CLKID_CPU_CLK_AXI 195
#define CLKID_CPU_CLK_TRACE_DIV 196
#define CLKID_CPU_CLK_TRACE 197
+#define CLKID_PCIE_PLL_DCO 198
+#define CLKID_PCIE_PLL_DCO_DIV2 199
+#define CLKID_PCIE_PLL_OD 200
-#define NR_CLKS 198
+#define NR_CLKS 202
/* include the CLKIDs that have been made part of the DT binding */
#include <dt-bindings/clock/g12a-clkc.h>
--
2.20.1
Add a clock ID for the reference clock feeding the USB3+PCIe Combo PHY.
Signed-off-by: Neil Armstrong <[email protected]>
---
include/dt-bindings/clock/g12a-clkc.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/dt-bindings/clock/g12a-clkc.h b/include/dt-bindings/clock/g12a-clkc.h
index d7bf0830c87d..30303728fe09 100644
--- a/include/dt-bindings/clock/g12a-clkc.h
+++ b/include/dt-bindings/clock/g12a-clkc.h
@@ -132,5 +132,6 @@
#define CLKID_MALI 175
#define CLKID_MPLL_5OM 177
#define CLKID_CPU_CLK 187
+#define CLKID_PCIE_PLL 201
#endif /* __G12A_CLKC_H */
--
2.20.1
On Thu, 2019-03-07 at 15:14 +0100, Neil Armstrong wrote:
> Add the PCIe reference clock feeding the USB3 + PCIE combo PHY.
>
> This PLL needs a very precise register sequence to permit to be locked,
> thus using the specific clk-pll pcie ops.
>
> The PLL is then followed by :
> - a fixed /2 divider
> - a 5-bit 1-based divider
> - a final /2 divider
>
> This reference clock is fixed to 100MHz, thus only a single PLL setup
> is added.
>
> Signed-off-by: Neil Armstrong <[email protected]>
> ---
> drivers/clk/meson/g12a.c | 118 +++++++++++++++++++++++++++++++++++++++
> drivers/clk/meson/g12a.h | 5 +-
> 2 files changed, 122 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
> index 80a7172df2a6..d382c21a29e5 100644
> --- a/drivers/clk/meson/g12a.c
> +++ b/drivers/clk/meson/g12a.c
> @@ -614,6 +614,118 @@ static struct clk_regmap g12a_hifi_pll = {
> },
> };
>
> +/*
> + * The Meson G12A PCIE PLL is fined tuned to deliver a very precise
> + * 100MHz reference clock for the PCIe Analog PHY, and thus requires
> + * a strict register sequence to enable the PLL.
> + */
> +static const struct reg_sequence g12a_pcie_pll_init_regs[] = {
> + { .reg = HHI_PCIE_PLL_CNTL0, .def = 0x20090496 },
> + { .reg = HHI_PCIE_PLL_CNTL0, .def = 0x30090496 },
> + { .reg = HHI_PCIE_PLL_CNTL1, .def = 0x00000000 },
> + { .reg = HHI_PCIE_PLL_CNTL2, .def = 0x00001100 },
> + { .reg = HHI_PCIE_PLL_CNTL3, .def = 0x10058e00 },
> + { .reg = HHI_PCIE_PLL_CNTL4, .def = 0x000100c0 },
> + { .reg = HHI_PCIE_PLL_CNTL5, .def = 0x68000048 },
> + { .reg = HHI_PCIE_PLL_CNTL5, .def = 0x68000068, .delay_us = 20 },
> + { .reg = HHI_PCIE_PLL_CNTL4, .def = 0x008100c0, .delay_us = 10 },
> + { .reg = HHI_PCIE_PLL_CNTL0, .def = 0x34090496 },
> + { .reg = HHI_PCIE_PLL_CNTL0, .def = 0x14090496, .delay_us = 10 },
> + { .reg = HHI_PCIE_PLL_CNTL2, .def = 0x00001000 },
> +};
> +
> +/* Keep a single entry table for recalc/round_rate() ops */
> +static const struct pll_params_table g12a_pcie_pll_table[] = {
> + PLL_PARAMS(150, 1),
> + {0, 0},
> +};
> +
> +static struct clk_regmap g12a_pcie_pll_dco = {
> + .data = &(struct meson_clk_pll_data){
> + .en = {
> + .reg_off = HHI_PCIE_PLL_CNTL0,
> + .shift = 28,
> + .width = 1,
> + },
> + .m = {
> + .reg_off = HHI_PCIE_PLL_CNTL0,
> + .shift = 0,
> + .width = 8,
> + },
> + .n = {
> + .reg_off = HHI_PCIE_PLL_CNTL0,
> + .shift = 10,
> + .width = 5,
> + },
> + .frac = {
> + .reg_off = HHI_PCIE_PLL_CNTL1,
> + .shift = 0,
> + .width = 12,
> + },
> + .l = {
> + .reg_off = HHI_PCIE_PLL_CNTL0,
> + .shift = 31,
> + .width = 1,
> + },
> + .rst = {
> + .reg_off = HHI_PCIE_PLL_CNTL0,
> + .shift = 29,
> + .width = 1,
> + },
> + .table = g12a_pcie_pll_table,
> + .init_regs = g12a_pcie_pll_init_regs,
> + .init_count = ARRAY_SIZE(g12a_pcie_pll_init_regs),
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "pcie_pll_dco",
> + .ops = &meson_clk_pcie_pll_ops,
> + .parent_names = (const char *[]){ IN_PREFIX "xtal" },
> + .num_parents = 1,
> + },
> +};
> +
> +static struct clk_fixed_factor g12a_pcie_pll_dco_div2 = {
> + .mult = 1,
> + .div = 2,
> + .hw.init = &(struct clk_init_data){
> + .name = "pcie_pll_dco_div2",
> + .ops = &clk_fixed_factor_ops,
> + .parent_names = (const char *[]){ "pcie_pll_dco" },
> + .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT,
> + },
> +};
> +
> +static struct clk_regmap g12a_pcie_pll_od = {
> + .data = &(struct clk_regmap_div_data){
> + .offset = HHI_PCIE_PLL_CNTL0,
> + .shift = 16,
> + .width = 5,
> + .flags = CLK_DIVIDER_ROUND_CLOSEST |
> + CLK_DIVIDER_ONE_BASED |
> + CLK_DIVIDER_ALLOW_ZERO,
That's unusual !
Ods have always been power of two divider so far. I suppose you have noticed
and checked this as well ?
That being said, the doc seems to imply that the global post divider will
divide by 36, which seems difficult to reach with powers of two.
> + },
> + .hw.init = &(struct clk_init_data){
> + .name = "pcie_pll_od",
> + .ops = &clk_regmap_divider_ops,
> + .parent_names = (const char *[]){ "pcie_pll_dco_div2" },
> + .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT,
> + },
> +};
> +
> +static struct clk_fixed_factor g12a_pcie_pll = {
> + .mult = 1,
> + .div = 2,
> + .hw.init = &(struct clk_init_data){
> + .name = "pcie_pll_pll",
> + .ops = &clk_fixed_factor_ops,
> + .parent_names = (const char *[]){ "pcie_pll_od" },
> + .num_parents = 1,
> + .flags = CLK_SET_RATE_PARENT,
> + },
> +};
> +
> static struct clk_regmap g12a_hdmi_pll_dco = {
> .data = &(struct meson_clk_pll_data){
> .en = {
> @@ -2499,6 +2611,10 @@ static struct clk_hw_onecell_data g12a_hw_onecell_data = {
> [CLKID_CPU_CLK_AXI] = &g12a_cpu_clk_axi.hw,
> [CLKID_CPU_CLK_TRACE_DIV] = &g12a_cpu_clk_trace_div.hw,
> [CLKID_CPU_CLK_TRACE] = &g12a_cpu_clk_trace.hw,
> + [CLKID_PCIE_PLL_DCO] = &g12a_pcie_pll_dco.hw,
> + [CLKID_PCIE_PLL_DCO_DIV2] = &g12a_pcie_pll_dco_div2.hw,
> + [CLKID_PCIE_PLL_OD] = &g12a_pcie_pll_od.hw,
> + [CLKID_PCIE_PLL] = &g12a_pcie_pll.hw,
> [NR_CLKS] = NULL,
> },
> .num = NR_CLKS,
> @@ -2685,6 +2801,8 @@ static struct clk_regmap *const g12a_clk_regmaps[] = {
> &g12a_cpu_clk_axi,
> &g12a_cpu_clk_trace_div,
> &g12a_cpu_clk_trace,
> + &g12a_pcie_pll_od,
> + &g12a_pcie_pll_dco,
> };
>
> static const struct meson_eeclkc_data g12a_clkc_data = {
> diff --git a/drivers/clk/meson/g12a.h b/drivers/clk/meson/g12a.h
> index 70aa469ca1cf..1393a09730a6 100644
> --- a/drivers/clk/meson/g12a.h
> +++ b/drivers/clk/meson/g12a.h
> @@ -186,8 +186,11 @@
> #define CLKID_CPU_CLK_AXI 195
> #define CLKID_CPU_CLK_TRACE_DIV 196
> #define CLKID_CPU_CLK_TRACE 197
> +#define CLKID_PCIE_PLL_DCO 198
> +#define CLKID_PCIE_PLL_DCO_DIV2 199
> +#define CLKID_PCIE_PLL_OD 200
>
> -#define NR_CLKS 198
> +#define NR_CLKS 202
>
> /* include the CLKIDs that have been made part of the DT binding */
> #include <dt-bindings/clock/g12a-clkc.h>
On 19/03/2019 10:55, Jerome Brunet wrote:
> On Thu, 2019-03-07 at 15:14 +0100, Neil Armstrong wrote:
>> Add the PCIe reference clock feeding the USB3 + PCIE combo PHY.
>>
>> This PLL needs a very precise register sequence to permit to be locked,
>> thus using the specific clk-pll pcie ops.
>>
>> The PLL is then followed by :
>> - a fixed /2 divider
>> - a 5-bit 1-based divider
>> - a final /2 divider
>>
>> This reference clock is fixed to 100MHz, thus only a single PLL setup
>> is added.
>>
>> Signed-off-by: Neil Armstrong <[email protected]>
>> ---
>> drivers/clk/meson/g12a.c | 118 +++++++++++++++++++++++++++++++++++++++
>> drivers/clk/meson/g12a.h | 5 +-
>> 2 files changed, 122 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c
>> index 80a7172df2a6..d382c21a29e5 100644
>> --- a/drivers/clk/meson/g12a.c
>> +++ b/drivers/clk/meson/g12a.c
>> @@ -614,6 +614,118 @@ static struct clk_regmap g12a_hifi_pll = {
>> },
>> };
>>
>> +/*
>> + * The Meson G12A PCIE PLL is fined tuned to deliver a very precise
>> + * 100MHz reference clock for the PCIe Analog PHY, and thus requires
>> + * a strict register sequence to enable the PLL.
>> + */
>> +static const struct reg_sequence g12a_pcie_pll_init_regs[] = {
>> + { .reg = HHI_PCIE_PLL_CNTL0, .def = 0x20090496 },
>> + { .reg = HHI_PCIE_PLL_CNTL0, .def = 0x30090496 },
>> + { .reg = HHI_PCIE_PLL_CNTL1, .def = 0x00000000 },
>> + { .reg = HHI_PCIE_PLL_CNTL2, .def = 0x00001100 },
>> + { .reg = HHI_PCIE_PLL_CNTL3, .def = 0x10058e00 },
>> + { .reg = HHI_PCIE_PLL_CNTL4, .def = 0x000100c0 },
>> + { .reg = HHI_PCIE_PLL_CNTL5, .def = 0x68000048 },
>> + { .reg = HHI_PCIE_PLL_CNTL5, .def = 0x68000068, .delay_us = 20 },
>> + { .reg = HHI_PCIE_PLL_CNTL4, .def = 0x008100c0, .delay_us = 10 },
>> + { .reg = HHI_PCIE_PLL_CNTL0, .def = 0x34090496 },
>> + { .reg = HHI_PCIE_PLL_CNTL0, .def = 0x14090496, .delay_us = 10 },
>> + { .reg = HHI_PCIE_PLL_CNTL2, .def = 0x00001000 },
>> +};
>> +
>> +/* Keep a single entry table for recalc/round_rate() ops */
>> +static const struct pll_params_table g12a_pcie_pll_table[] = {
>> + PLL_PARAMS(150, 1),
>> + {0, 0},
>> +};
>> +
>> +static struct clk_regmap g12a_pcie_pll_dco = {
>> + .data = &(struct meson_clk_pll_data){
>> + .en = {
>> + .reg_off = HHI_PCIE_PLL_CNTL0,
>> + .shift = 28,
>> + .width = 1,
>> + },
>> + .m = {
>> + .reg_off = HHI_PCIE_PLL_CNTL0,
>> + .shift = 0,
>> + .width = 8,
>> + },
>> + .n = {
>> + .reg_off = HHI_PCIE_PLL_CNTL0,
>> + .shift = 10,
>> + .width = 5,
>> + },
>> + .frac = {
>> + .reg_off = HHI_PCIE_PLL_CNTL1,
>> + .shift = 0,
>> + .width = 12,
>> + },
>> + .l = {
>> + .reg_off = HHI_PCIE_PLL_CNTL0,
>> + .shift = 31,
>> + .width = 1,
>> + },
>> + .rst = {
>> + .reg_off = HHI_PCIE_PLL_CNTL0,
>> + .shift = 29,
>> + .width = 1,
>> + },
>> + .table = g12a_pcie_pll_table,
>> + .init_regs = g12a_pcie_pll_init_regs,
>> + .init_count = ARRAY_SIZE(g12a_pcie_pll_init_regs),
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "pcie_pll_dco",
>> + .ops = &meson_clk_pcie_pll_ops,
>> + .parent_names = (const char *[]){ IN_PREFIX "xtal" },
>> + .num_parents = 1,
>> + },
>> +};
>> +
>> +static struct clk_fixed_factor g12a_pcie_pll_dco_div2 = {
>> + .mult = 1,
>> + .div = 2,
>> + .hw.init = &(struct clk_init_data){
>> + .name = "pcie_pll_dco_div2",
>> + .ops = &clk_fixed_factor_ops,
>> + .parent_names = (const char *[]){ "pcie_pll_dco" },
>> + .num_parents = 1,
>> + .flags = CLK_SET_RATE_PARENT,
>> + },
>> +};
>> +
>> +static struct clk_regmap g12a_pcie_pll_od = {
>> + .data = &(struct clk_regmap_div_data){
>> + .offset = HHI_PCIE_PLL_CNTL0,
>> + .shift = 16,
>> + .width = 5,
>> + .flags = CLK_DIVIDER_ROUND_CLOSEST |
>> + CLK_DIVIDER_ONE_BASED |
>> + CLK_DIVIDER_ALLOW_ZERO,
>
> That's unusual !
> Ods have always been power of two divider so far. I suppose you have noticed
> and checked this as well ?
>
Yes, I've checked and validated it. Without CLK_DIVIDER_ONE_BASED,
the divider is off by 1. And CLK_DIVIDER_ALLOW_ZERO since at reset,
0 is the default register value.
> That being said, the doc seems to imply that the global post divider will
> divide by 36, which seems difficult to reach with powers of two.
>
>> + },
>> + .hw.init = &(struct clk_init_data){
>> + .name = "pcie_pll_od",
>> + .ops = &clk_regmap_divider_ops,
>> + .parent_names = (const char *[]){ "pcie_pll_dco_div2" },
>> + .num_parents = 1,
>> + .flags = CLK_SET_RATE_PARENT,
>> + },
>> +};
>> +
>> +static struct clk_fixed_factor g12a_pcie_pll = {
>> + .mult = 1,
>> + .div = 2,
>> + .hw.init = &(struct clk_init_data){
>> + .name = "pcie_pll_pll",
>> + .ops = &clk_fixed_factor_ops,
>> + .parent_names = (const char *[]){ "pcie_pll_od" },
>> + .num_parents = 1,
>> + .flags = CLK_SET_RATE_PARENT,
>> + },
>> +};
>> +
>> static struct clk_regmap g12a_hdmi_pll_dco = {
>> .data = &(struct meson_clk_pll_data){
>> .en = {
>> @@ -2499,6 +2611,10 @@ static struct clk_hw_onecell_data g12a_hw_onecell_data = {
>> [CLKID_CPU_CLK_AXI] = &g12a_cpu_clk_axi.hw,
>> [CLKID_CPU_CLK_TRACE_DIV] = &g12a_cpu_clk_trace_div.hw,
>> [CLKID_CPU_CLK_TRACE] = &g12a_cpu_clk_trace.hw,
>> + [CLKID_PCIE_PLL_DCO] = &g12a_pcie_pll_dco.hw,
>> + [CLKID_PCIE_PLL_DCO_DIV2] = &g12a_pcie_pll_dco_div2.hw,
>> + [CLKID_PCIE_PLL_OD] = &g12a_pcie_pll_od.hw,
>> + [CLKID_PCIE_PLL] = &g12a_pcie_pll.hw,
>> [NR_CLKS] = NULL,
>> },
>> .num = NR_CLKS,
>> @@ -2685,6 +2801,8 @@ static struct clk_regmap *const g12a_clk_regmaps[] = {
>> &g12a_cpu_clk_axi,
>> &g12a_cpu_clk_trace_div,
>> &g12a_cpu_clk_trace,
>> + &g12a_pcie_pll_od,
>> + &g12a_pcie_pll_dco,
>> };
>>
>> static const struct meson_eeclkc_data g12a_clkc_data = {
>> diff --git a/drivers/clk/meson/g12a.h b/drivers/clk/meson/g12a.h
>> index 70aa469ca1cf..1393a09730a6 100644
>> --- a/drivers/clk/meson/g12a.h
>> +++ b/drivers/clk/meson/g12a.h
>> @@ -186,8 +186,11 @@
>> #define CLKID_CPU_CLK_AXI 195
>> #define CLKID_CPU_CLK_TRACE_DIV 196
>> #define CLKID_CPU_CLK_TRACE 197
>> +#define CLKID_PCIE_PLL_DCO 198
>> +#define CLKID_PCIE_PLL_DCO_DIV2 199
>> +#define CLKID_PCIE_PLL_OD 200
>>
>> -#define NR_CLKS 198
>> +#define NR_CLKS 202
>>
>> /* include the CLKIDs that have been made part of the DT binding */
>> #include <dt-bindings/clock/g12a-clkc.h>
>
>
On Thu, 2019-03-07 at 15:14 +0100, Neil Armstrong wrote:
> The Amlogic G12A SoCs embeds a dedicated PLL to feed the USB3+PCIE
> Combo PHY. This PLL needs a very specific and strict register sequence
> in order to correcly enable it and deliver the 100MHz reference clock
> to the Analog PHY.
>
> After lot of trials and errors, and since this PLL will ever feed 100MHz
> with a static configuration, it is simpler to setup a dedicated ops
> structure with a custom _enable() op applying the init register sequence.
>
> The rate calculation ops are kept in order to keep the nominal read
> ops as-in, but set_rate is removed.
>
> With this setup, the PLL can be enabled and disable safely and always have
> the recommended PLL setup to feed the USB3+PCIE Combo PHY.
>
> Neil Armstrong (3):
> clk: meson-pll: add reduced specific clk_ops for G12A PCIe PLL
> dt-bindings: clk: g12a-clkc: add PCIE PLL clock ID
> clk: meson-g12a: add PCIE PLL clocks
>
> drivers/clk/meson/clk-pll.c | 26 ++++++
> drivers/clk/meson/clk-pll.h | 1 +
> drivers/clk/meson/g12a.c | 118 ++++++++++++++++++++++++++
> drivers/clk/meson/g12a.h | 5 +-
> include/dt-bindings/clock/g12a-clkc.h | 1 +
> 5 files changed, 150 insertions(+), 1 deletion(-)
Well this PLL is indeed a nasty one ;)
Acked-by: Jerome Brunet <[email protected]>
On 19/03/2019 11:01, Jerome Brunet wrote:
> On Thu, 2019-03-07 at 15:14 +0100, Neil Armstrong wrote:
>> The Amlogic G12A SoCs embeds a dedicated PLL to feed the USB3+PCIE
>> Combo PHY. This PLL needs a very specific and strict register sequence
>> in order to correcly enable it and deliver the 100MHz reference clock
>> to the Analog PHY.
>>
>> After lot of trials and errors, and since this PLL will ever feed 100MHz
>> with a static configuration, it is simpler to setup a dedicated ops
>> structure with a custom _enable() op applying the init register sequence.
>>
>> The rate calculation ops are kept in order to keep the nominal read
>> ops as-in, but set_rate is removed.
>>
>> With this setup, the PLL can be enabled and disable safely and always have
>> the recommended PLL setup to feed the USB3+PCIE Combo PHY.
>>
>> Neil Armstrong (3):
>> clk: meson-pll: add reduced specific clk_ops for G12A PCIe PLL
>> dt-bindings: clk: g12a-clkc: add PCIE PLL clock ID
>> clk: meson-g12a: add PCIE PLL clocks
>>
>> drivers/clk/meson/clk-pll.c | 26 ++++++
>> drivers/clk/meson/clk-pll.h | 1 +
>> drivers/clk/meson/g12a.c | 118 ++++++++++++++++++++++++++
>> drivers/clk/meson/g12a.h | 5 +-
>> include/dt-bindings/clock/g12a-clkc.h | 1 +
>> 5 files changed, 150 insertions(+), 1 deletion(-)
>
> Well this PLL is indeed a nasty one ;)
>
> Acked-by: Jerome Brunet <[email protected]>
>
Applied patch 2 on next/headers, patches 1 & 3 on next/drivers
Neil
On Thu, 7 Mar 2019 15:14:54 +0100, Neil Armstrong wrote:
> Add a clock ID for the reference clock feeding the USB3+PCIe Combo PHY.
>
> Signed-off-by: Neil Armstrong <[email protected]>
> ---
> include/dt-bindings/clock/g12a-clkc.h | 1 +
> 1 file changed, 1 insertion(+)
>
Reviewed-by: Rob Herring <[email protected]>