Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1609370 > unrolled thread
| Started by | Priit Laes <plaes@plaes.org> |
|---|---|
| First post | 2017-03-26 19:40 +0200 |
| Last post | 2017-04-07 15:50 +0200 |
| Articles | 8 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH v2 0/6] ARM: sunxi: Convert sun4i/sun7i series SoCs to sunxi-ng Priit Laes <plaes@plaes.org> - 2017-03-26 19:40 +0200
[PATCH v2 4/6] dt-bindings: List devicetree binding for the CCU of Allwinner A20 Priit Laes <plaes@plaes.org> - 2017-03-26 20:00 +0200
[PATCH v2 6/6] clk: sunxi-ng: Display index when clock registration fails Priit Laes <plaes@plaes.org> - 2017-03-26 20:10 +0200
[PATCH v2 5/6] dt-bindings: List devicetree binding for the CCU of Allwinner A10 Priit Laes <plaes@plaes.org> - 2017-03-26 20:40 +0200
Re: [PATCH v2 5/6] dt-bindings: List devicetree binding for the CCU of Allwinner A10 Rob Herring <robh@kernel.org> - 2017-03-31 01:30 +0200
Re: [PATCH v2 1/6] clk: sunxi-ng: Add sun4i/sun7i CCU driver Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-03-27 11:00 +0200
Re: [linux-sunxi] Re: [PATCH v2 1/6] clk: sunxi-ng: Add sun4i/sun7i CCU driver Priit Laes <plaes@plaes.org> - 2017-04-04 22:20 +0200
Re: [linux-sunxi] Re: [PATCH v2 1/6] clk: sunxi-ng: Add sun4i/sun7i CCU driver Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-04-07 15:50 +0200
| From | Priit Laes <plaes@plaes.org> |
|---|---|
| Date | 2017-03-26 19:40 +0200 |
| Subject | [PATCH v2 0/6] ARM: sunxi: Convert sun4i/sun7i series SoCs to sunxi-ng |
| Message-ID | <tpk9k-37T-5@gated-at.bofh.it> |
Hi, This serie brings A10 (sun4i) and A20 (sun7i) SoCs into the sunxi-ng world. As mentioned in sun5i conversion, this is pretty much standard stuff as all the required clocks were already implemented in the sunxi-ng framework. Unfortunately there's an issue with LVDS reset control that causes issues with LVDS displays unless 'clk_ignore_unused' option is used. Need help with that :( Changes from v1: - Drop useless comments - Add support for A10 / sun4i. - Rename driver to sunxi-a10-a20. - Add previously unimplemented clocks. - Document the audio pll hardcoded post-divider - Add Acked-by: Rob Herring <robh at kernel.org> on patch 4 Priit Laes (6): clk: sunxi-ng: Add sun4i/sun7i CCU driver ARM: sun7i: Convert to CCU ARM: sun4i: Convert to CCU dt-bindings: List devicetree binding for the CCU of Allwinner A20 dt-bindings: List devicetree binding for the CCU of Allwinner A10 clk: sunxi-ng: Display index when clock registration fails Documentation/devicetree/bindings/clock/sunxi-ccu.txt | 2 +- arch/arm/boot/dts/sun4i-a10.dtsi | 636 +---- arch/arm/boot/dts/sun7i-a20.dtsi | 717 +----- drivers/clk/sunxi-ng/Kconfig | 13 +- drivers/clk/sunxi-ng/Makefile | 1 +- drivers/clk/sunxi-ng/ccu-sunxi-a10-a20.c | 1532 ++++++++++- drivers/clk/sunxi-ng/ccu-sunxi-a10-a20.h | 59 +- drivers/clk/sunxi-ng/ccu_common.c | 4 +- include/dt-bindings/clock/sunxi-a10-a20-ccu.h | 208 +- include/dt-bindings/reset/sunxi-a10-a20-ccu.h | 66 +- 10 files changed, 2050 insertions(+), 1188 deletions(-) create mode 100644 drivers/clk/sunxi-ng/ccu-sunxi-a10-a20.c create mode 100644 drivers/clk/sunxi-ng/ccu-sunxi-a10-a20.h create mode 100644 include/dt-bindings/clock/sunxi-a10-a20-ccu.h create mode 100644 include/dt-bindings/reset/sunxi-a10-a20-ccu.h base-commit: 8e19fb843be8934d48b31fafbb32a4176f7feb65 -- git-series 0.9.1
[toc] | [next] | [standalone]
| From | Priit Laes <plaes@plaes.org> |
|---|---|
| Date | 2017-03-26 20:00 +0200 |
| Subject | [PATCH v2 4/6] dt-bindings: List devicetree binding for the CCU of Allwinner A20 |
| Message-ID | <tpkCl-3iY-3@gated-at.bofh.it> |
| In reply to | #1609370 |
Allwinner A20 is now driven by sunxi-ng CCU driver. Add devicetree binding for it. Acked-by: Rob Herring <robh@kernel.org> Signed-off-by: Priit Laes <plaes@plaes.org> --- Documentation/devicetree/bindings/clock/sunxi-ccu.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/clock/sunxi-ccu.txt b/Documentation/devicetree/bindings/clock/sunxi-ccu.txt index 68512aa..de90988 100644 --- a/Documentation/devicetree/bindings/clock/sunxi-ccu.txt +++ b/Documentation/devicetree/bindings/clock/sunxi-ccu.txt @@ -4,6 +4,7 @@ Allwinner Clock Control Unit Binding Required properties : - compatible: must contain one of the following compatibles: - "allwinner,sun6i-a31-ccu" + - "allwinner,sun7i-a20-ccu" - "allwinner,sun8i-a23-ccu" - "allwinner,sun8i-a33-ccu" - "allwinner,sun8i-h3-ccu" -- git-series 0.9.1
[toc] | [prev] | [next] | [standalone]
| From | Priit Laes <plaes@plaes.org> |
|---|---|
| Date | 2017-03-26 20:10 +0200 |
| Subject | [PATCH v2 6/6] clk: sunxi-ng: Display index when clock registration fails |
| Message-ID | <tpkM2-3F0-7@gated-at.bofh.it> |
| In reply to | #1609370 |
Add clock index to clock registration failure message.
Signed-off-by: Priit Laes <plaes@plaes.org>
---
drivers/clk/sunxi-ng/ccu_common.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/sunxi-ng/ccu_common.c b/drivers/clk/sunxi-ng/ccu_common.c
index 8a47baf..188fa50 100644
--- a/drivers/clk/sunxi-ng/ccu_common.c
+++ b/drivers/clk/sunxi-ng/ccu_common.c
@@ -63,8 +63,8 @@ int sunxi_ccu_probe(struct device_node *node, void __iomem *reg,
ret = clk_hw_register(NULL, hw);
if (ret) {
- pr_err("Couldn't register clock %s\n",
- clk_hw_get_name(hw));
+ pr_err("Couldn't register clock %d - %s\n",
+ i, clk_hw_get_name(hw));
goto err_clk_unreg;
}
}
--
git-series 0.9.1
[toc] | [prev] | [next] | [standalone]
| From | Priit Laes <plaes@plaes.org> |
|---|---|
| Date | 2017-03-26 20:40 +0200 |
| Subject | [PATCH v2 5/6] dt-bindings: List devicetree binding for the CCU of Allwinner A10 |
| Message-ID | <tplf3-3Pl-7@gated-at.bofh.it> |
| In reply to | #1609370 |
Allwinner A10 is now driven by sunxi-ng CCU driver. Add devicetree binding for it. Signed-off-by: Priit Laes <plaes@plaes.org> --- Documentation/devicetree/bindings/clock/sunxi-ccu.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/clock/sunxi-ccu.txt b/Documentation/devicetree/bindings/clock/sunxi-ccu.txt index de90988..db49cbc 100644 --- a/Documentation/devicetree/bindings/clock/sunxi-ccu.txt +++ b/Documentation/devicetree/bindings/clock/sunxi-ccu.txt @@ -3,6 +3,7 @@ Allwinner Clock Control Unit Binding Required properties : - compatible: must contain one of the following compatibles: + - "allwinner,sun4i-a10-ccu" - "allwinner,sun6i-a31-ccu" - "allwinner,sun7i-a20-ccu" - "allwinner,sun8i-a23-ccu" -- git-series 0.9.1
[toc] | [prev] | [next] | [standalone]
| From | Rob Herring <robh@kernel.org> |
|---|---|
| Date | 2017-03-31 01:30 +0200 |
| Subject | Re: [PATCH v2 5/6] dt-bindings: List devicetree binding for the CCU of Allwinner A10 |
| Message-ID | <tqRFT-55I-7@gated-at.bofh.it> |
| In reply to | #1609380 |
On Sun, Mar 26, 2017 at 08:20:20PM +0300, Priit Laes wrote: > Allwinner A10 is now driven by sunxi-ng CCU driver. > > Add devicetree binding for it. > > Signed-off-by: Priit Laes <plaes@plaes.org> > --- > Documentation/devicetree/bindings/clock/sunxi-ccu.txt | 1 + > 1 file changed, 1 insertion(+) Acked-by: Rob Herring <robh@kernel.org>
[toc] | [prev] | [next] | [standalone]
| From | Maxime Ripard <maxime.ripard@free-electrons.com> |
|---|---|
| Date | 2017-03-27 11:00 +0200 |
| Subject | Re: [PATCH v2 1/6] clk: sunxi-ng: Add sun4i/sun7i CCU driver |
| Message-ID | <tpyFk-5la-17@gated-at.bofh.it> |
| In reply to | #1609370 |
[Multipart message — attachments visible in raw view] — view raw
Hi,
Thanks a lot for working on this.
On Sun, Mar 26, 2017 at 08:20:16PM +0300, Priit Laes wrote:
> Introduce a clock controller driver for sun4i A10 and sun7i A20
> series SoCs.
>
> Signed-off-by: Priit Laes <plaes@plaes.org>
> ---
> drivers/clk/sunxi-ng/Kconfig | 13 +-
> drivers/clk/sunxi-ng/Makefile | 1 +-
> drivers/clk/sunxi-ng/ccu-sunxi-a10-a20.c | 1532 ++++++++++++++++++-
> drivers/clk/sunxi-ng/ccu-sunxi-a10-a20.h | 59 +-
> include/dt-bindings/clock/sunxi-a10-a20-ccu.h | 208 ++-
> include/dt-bindings/reset/sunxi-a10-a20-ccu.h | 66 +-
I'm not too fond of those sunxi-<all the SoCs supported>. We're not
doing that for any other driver, I don't really know why this has
became a trend lately.
You can call them ccu-sun4i-a10.h, and it will work just fine.
> +/* Not documented on A10 */
> +static SUNXI_CCU_GATE(pll_periph_sata_clk, "pll-periph-sata", "pll-periph",
> + 0x028, BIT(14), 0);
The rate doesn't come from pll-periph directly, does it?
> +#define SUN4I_AHB_REG 0x054
> +static struct ccu_mux cpu_clk = {
> + .mux = {
> + .shift = 16,
> + .width = 2,
> + .fixed_predivs = cpu_predivs,
> + .n_predivs = ARRAY_SIZE(cpu_predivs),
> + },
> + .common = {
> + .reg = 0x054,
Why did you define this one, even though you don't seem to be using it
anywhere?
> +static const char *const ahb_parents[] = { "axi", "pll-periph",
> + "pll-periph-2x" };
> +static const struct ccu_mux_fixed_prediv ahb_predivs[] = {
> + { .index = 2, .div = 2, },
> +};
This seems to be only true for the A20, and not the A10.
Are you sure here? The pll-periph-2x seem to be only used in the MBUS
clock in our current code.
And then, using pll-periph-2x, and then dividing it by 2 just gives us
pll-periph, which is also our previous parent :)
> +/* Undocumented on A10 */
> +static SUNXI_CCU_PHASE(mmc0_output_clk, "mmc0_output", "mmc0",
> + 0x088, 8, 3, 0);
> +/* Undocumented on A10 */
> +static SUNXI_CCU_PHASE(mmc0_sample_clk, "mmc0_sample", "mmc0",
> + 0x088, 20, 3, 0);
The A10 doesn't have them.
> +/* TODO: Check whether A10 actually supports osc32k as 4th parent? */
> +static const char *const ir_parents_sun4i[] = { "hosc", "pll-periph",
> + "pll-ddr-other" };
What does the BSP say about this?
> +/* Undocumented on A10 */
> +static SUNXI_CCU_MUX_WITH_GATE(spdif_clk, "spdif", audio_parents,
> + 0x0c0, 16, 2, BIT(31), CLK_SET_RATE_PARENT);
This doesn't seem to exist at all on the A10
> +/*
> + * TODO: SATA clock also supports external clock as parent via BIT(24)
> + * The external clock is probably an optional crystal or oscillator
> + * that can be connected to the SATA-CLKM / SATA-CLKP pins.
> + */
> +static SUNXI_CCU_GATE(sata_clk, "sata", "pll-periph-sata",
> + 0x0c8, BIT(31), 0);
The rate won't be good here either. This is supposed to be 100MHz.
> +static const char *const csi_isp_parents[] = { "pll-video0", "pll-ve",
> + "pll-ddr-other", "pll-sata" };
> +
> +static SUNXI_CCU_M_WITH_MUX_GATE(csi_isp_clk, "csi-isp",
> + csi_isp_parents,
> + 0x120, 0, 4, 24, 2, BIT(31), 0);
We've been calling it sclk in the other SoC iirc. Any particular
reason to call it differently?
> +static const char *const out_parents[] = { "hosc", "osc32k", "hosc" };
> +static SUNXI_CCU_MP_WITH_MUX_GATE(out_a_clk, "out-a", out_parents,
> + 0x1f0, 8, 5, 20, 2, 24, 2, BIT(31), 0);
> +static SUNXI_CCU_MP_WITH_MUX_GATE(out_b_clk, "out-b", out_parents,
> + 0x1f4, 8, 5, 20, 2, 24, 2, BIT(31), 0);
There's a fixed pre-divider on the first hosc of 750.
> +static void init_clocks(void __iomem *reg)
> +{
> + u32 val;
> +
> + /* Force the PLL-Audio-1x divider to 4 */
> + val = readl(reg + SUN4I_PLL_AUDIO_REG);
> + val &= ~GENMASK(19, 16);
> + writel(val | (3 << 16), reg + SUN4I_PLL_AUDIO_REG);
> +
> + /* Use PLL6 as parent for AHB */
> + val = readl(reg + SUN4I_AHB_REG);
> + val &= ~GENMASK(7, 6);
> + writel(val | (2 << 6), reg + SUN4I_AHB_REG);
Keeping some kind of comment similar to what was in the DT would be
great, otherwise we lose *why* we need to do so.
> +}
> +
> +static void __init sun4i_a10_ccu_setup(struct device_node *node)
> +{
> + void __iomem *reg;
> +
> + reg = of_io_request_and_map(node, 0, of_node_full_name(node));
> + if (IS_ERR(reg)) {
> + pr_err("%s: Could not map the clock registers\n",
> + of_node_full_name(node));
> + return;
> + }
> +
> + init_clocks(reg);
> +
> + sunxi_ccu_probe(node, reg, &sun4i_a10_ccu_desc);
Can't you move the request_and_map / probe in the common function?
> +#ifndef _DT_BINDINGS_CLK_SUNXI_A10_A20_H_
> +#define _DT_BINDINGS_CLK_SUNXI_A10_A20_H_
> +
> +#define CLK_HOSC 1
> +#define CLK_PLL_PERIPH_SATA 16
That one looks suspicious. I don't see why we would need the PLL,
while we have a perfectly functional SATA clock below. Have you tried
gating the bit31 of the register 0xc8 to see if it has any impact?
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[toc] | [prev] | [next] | [standalone]
| From | Priit Laes <plaes@plaes.org> |
|---|---|
| Date | 2017-04-04 22:20 +0200 |
| Subject | Re: [linux-sunxi] Re: [PATCH v2 1/6] clk: sunxi-ng: Add sun4i/sun7i CCU driver |
| Message-ID | <tsD5M-1gm-15@gated-at.bofh.it> |
| In reply to | #1609588 |
On Mon, Mar 27, 2017 at 09:54:38AM +0200, Maxime Ripard wrote:
> Hi,
>
> Thanks a lot for working on this.
>
> On Sun, Mar 26, 2017 at 08:20:16PM +0300, Priit Laes wrote:
> > Introduce a clock controller driver for sun4i A10 and sun7i A20
> > series SoCs.
> >
> > Signed-off-by: Priit Laes <plaes@plaes.org>
> > ---
> > drivers/clk/sunxi-ng/Kconfig | 13 +-
> > drivers/clk/sunxi-ng/Makefile | 1 +-
> > drivers/clk/sunxi-ng/ccu-sunxi-a10-a20.c | 1532 ++++++++++++++++++-
> > drivers/clk/sunxi-ng/ccu-sunxi-a10-a20.h | 59 +-
> > include/dt-bindings/clock/sunxi-a10-a20-ccu.h | 208 ++-
> > include/dt-bindings/reset/sunxi-a10-a20-ccu.h | 66 +-
>
> I'm not too fond of those sunxi-<all the SoCs supported>. We're not
> doing that for any other driver, I don't really know why this has
> became a trend lately.
>
> You can call them ccu-sun4i-a10.h, and it will work just fine.
OK, will do!
>
> > +/* Not documented on A10 */
> > +static SUNXI_CCU_GATE(pll_periph_sata_clk, "pll-periph-sata", "pll-periph",
> > + 0x028, BIT(14), 0);
>
> The rate doesn't come from pll-periph directly, does it?
So it uses hosc (24MHz parent clock) instead of pll-periph?
>
> > +#define SUN4I_AHB_REG 0x054
> > +static struct ccu_mux cpu_clk = {
> > + .mux = {
> > + .shift = 16,
> > + .width = 2,
> > + .fixed_predivs = cpu_predivs,
> > + .n_predivs = ARRAY_SIZE(cpu_predivs),
> > + },
> > + .common = {
> > + .reg = 0x054,
>
> Why did you define this one, even though you don't seem to be using it
> anywhere?
Leftover from when I also included A10 support.
>
> > +static const char *const ahb_parents[] = { "axi", "pll-periph",
> > + "pll-periph-2x" };
> > +static const struct ccu_mux_fixed_prediv ahb_predivs[] = {
> > + { .index = 2, .div = 2, },
> > +};
>
> This seems to be only true for the A20, and not the A10.
>
> Are you sure here? The pll-periph-2x seem to be only used in the MBUS
> clock in our current code.
Nope...
>
> And then, using pll-periph-2x, and then dividing it by 2 just gives us
> pll-periph, which is also our previous parent :)
...will investigate.
>
> > +/* Undocumented on A10 */
> > +static SUNXI_CCU_PHASE(mmc0_output_clk, "mmc0_output", "mmc0",
> > + 0x088, 8, 3, 0);
> > +/* Undocumented on A10 */
> > +static SUNXI_CCU_PHASE(mmc0_sample_clk, "mmc0_sample", "mmc0",
> > + 0x088, 20, 3, 0);
>
> The A10 doesn't have them.
Are you sure? Although, they weren't listed in datasheet, they are defined
in the sun4i-a10.dtsi:
mmc0_clk: clk@01c20088 {
#clock-cells = <1>;
compatible = "allwinner,sun4i-a10-mmc-clk";
reg = <0x01c20088 0x4>;
clocks = <&osc24M>, <&pll6 1>, <&pll5 1>;
clock-output-names = "mmc0",
"mmc0_output",
"mmc0_sample";
};
> > +/* TODO: Check whether A10 actually supports osc32k as 4th parent? */
> > +static const char *const ir_parents_sun4i[] = { "hosc", "pll-periph",
> > + "pll-ddr-other" };
>
> What does the BSP say about this?
sun7i datasheet mentions osc32k, but BSP code for sun4i, sun5i and sun7i
is identical and supports only 3 first parents without osc32k.
> > +/* Undocumented on A10 */
> > +static SUNXI_CCU_MUX_WITH_GATE(spdif_clk, "spdif", audio_parents,
> > + 0x0c0, 16, 2, BIT(31), CLK_SET_RATE_PARENT);
>
> This doesn't seem to exist at all on the A10
Wasn't listed in datasheet, but it's in BSP and also in sun4i-a10.dtsi:
spdif_clk: clk@01c200c0 {
#clock-cells = <0>;
compatible = "allwinner,sun4i-a10-mod1-clk";
reg = <0x01c200c0 0x4>;
clocks = <&pll2 SUN4I_A10_PLL2_8X>,
<&pll2 SUN4I_A10_PLL2_4X>,
<&pll2 SUN4I_A10_PLL2_2X>,
<&pll2 SUN4I_A10_PLL2_1X>;
clock-output-names = "spdif";
};
>
> > +/*
> > + * TODO: SATA clock also supports external clock as parent via BIT(24)
> > + * The external clock is probably an optional crystal or oscillator
> > + * that can be connected to the SATA-CLKM / SATA-CLKP pins.
> > + */
> > +static SUNXI_CCU_GATE(sata_clk, "sata", "pll-periph-sata",
> > + 0x0c8, BIT(31), 0);
>
> The rate won't be good here either. This is supposed to be 100MHz.
Hmm.. I tested SATA with Cubietruck. Or what do you mean?
> > +static const char *const csi_isp_parents[] = { "pll-video0", "pll-ve",
> > + "pll-ddr-other", "pll-sata" };
> > +
> > +static SUNXI_CCU_M_WITH_MUX_GATE(csi_isp_clk, "csi-isp",
> > + csi_isp_parents,
> > + 0x120, 0, 4, 24, 2, BIT(31), 0);
>
> We've been calling it sclk in the other SoC iirc. Any particular
> reason to call it differently?
It's called ISP in BSP and A10 manual.
In A20 it's indeed Special Clock Register (SCLK).
> > +static const char *const out_parents[] = { "hosc", "osc32k", "hosc" };
> > +static SUNXI_CCU_MP_WITH_MUX_GATE(out_a_clk, "out-a", out_parents,
> > + 0x1f0, 8, 5, 20, 2, 24, 2, BIT(31), 0);
> > +static SUNXI_CCU_MP_WITH_MUX_GATE(out_b_clk, "out-b", out_parents,
> > + 0x1f4, 8, 5, 20, 2, 24, 2, BIT(31), 0);
>
> There's a fixed pre-divider on the first hosc of 750.
Nice catch.
So it should be something like this:
[snip]
static const char *const out_parents[] = { "osc24M", "osc32k", "osc24M" };
static const struct ccu_mux_fixed_prediv out_prediv = {
.index = 0, .div = 750
};
static struct ccu_mp out_a_clk = {
.enable = BIT(31),
.m = _SUNXI_CCU_DIV(8, 5),
.p = _SUNXI_CCU_DIV(20, 2),
.mux = {
.shift = 24,
.width = 2,
.fixed_predivs = &out_prediv,
.n_predivs = ARRAY_SIZE(out_prediv),
},
.common = {
.reg = 0x1f0,
.features = CCU_FEATURE_FIXED_PREDIV,
.hw.init = CLK_HW_INIT_PARENTS("out-a",
out_parents,
&ccu_mp_ops,
0),
},
};
[/snip]
> > +static void init_clocks(void __iomem *reg)
> > +{
> > + u32 val;
> > +
> > + /* Force the PLL-Audio-1x divider to 4 */
> > + val = readl(reg + SUN4I_PLL_AUDIO_REG);
> > + val &= ~GENMASK(19, 16);
> > + writel(val | (3 << 16), reg + SUN4I_PLL_AUDIO_REG);
> > +
> > + /* Use PLL6 as parent for AHB */
> > + val = readl(reg + SUN4I_AHB_REG);
> > + val &= ~GENMASK(7, 6);
> > + writel(val | (2 << 6), reg + SUN4I_AHB_REG);
>
> Keeping some kind of comment similar to what was in the DT would be
> great, otherwise we lose *why* we need to do so.
OK
> > +}
> > +
> > +static void __init sun4i_a10_ccu_setup(struct device_node *node)
> > +{
> > + void __iomem *reg;
> > +
> > + reg = of_io_request_and_map(node, 0, of_node_full_name(node));
> > + if (IS_ERR(reg)) {
> > + pr_err("%s: Could not map the clock registers\n",
> > + of_node_full_name(node));
> > + return;
> > + }
> > +
> > + init_clocks(reg);
> > +
> > + sunxi_ccu_probe(node, reg, &sun4i_a10_ccu_desc);
>
> Can't you move the request_and_map / probe in the common function?
Will do.
> > +#ifndef _DT_BINDINGS_CLK_SUNXI_A10_A20_H_
> > +#define _DT_BINDINGS_CLK_SUNXI_A10_A20_H_
> > +
> > +#define CLK_HOSC 1
> > +#define CLK_PLL_PERIPH_SATA 16
>
> That one looks suspicious. I don't see why we would need the PLL,
> while we have a perfectly functional SATA clock below. Have you tried
> gating the bit31 of the register 0xc8 to see if it has any impact?
Will try it...
> Thanks!
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
>
> --
> You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
[toc] | [prev] | [next] | [standalone]
| From | Maxime Ripard <maxime.ripard@free-electrons.com> |
|---|---|
| Date | 2017-04-07 15:50 +0200 |
| Subject | Re: [linux-sunxi] Re: [PATCH v2 1/6] clk: sunxi-ng: Add sun4i/sun7i CCU driver |
| Message-ID | <ttCr0-7Ki-15@gated-at.bofh.it> |
| In reply to | #1616370 |
[Multipart message — attachments visible in raw view] — view raw
Hi Priit,
On Tue, Apr 04, 2017 at 08:09:19PM +0000, Priit Laes wrote:
> > > +/* Not documented on A10 */
> > > +static SUNXI_CCU_GATE(pll_periph_sata_clk, "pll-periph-sata", "pll-periph",
> > > + 0x028, BIT(14), 0);
> >
> > The rate doesn't come from pll-periph directly, does it?
>
> So it uses hosc (24MHz parent clock) instead of pll-periph?
I never looked too much at this, but it looks more like the input is
pll-periph-sata itself.
> > > +/* Undocumented on A10 */
> > > +static SUNXI_CCU_PHASE(mmc0_output_clk, "mmc0_output", "mmc0",
> > > + 0x088, 8, 3, 0);
> > > +/* Undocumented on A10 */
> > > +static SUNXI_CCU_PHASE(mmc0_sample_clk, "mmc0_sample", "mmc0",
> > > + 0x088, 20, 3, 0);
> >
> > The A10 doesn't have them.
>
> Are you sure? Although, they weren't listed in datasheet, they are defined
> in the sun4i-a10.dtsi:
>
> mmc0_clk: clk@01c20088 {
> #clock-cells = <1>;
> compatible = "allwinner,sun4i-a10-mmc-clk";
> reg = <0x01c20088 0x4>;
> clocks = <&osc24M>, <&pll6 1>, <&pll5 1>;
> clock-output-names = "mmc0",
> "mmc0_output",
> "mmc0_sample";
> };
Yes, those clocks have been introduced in the A20, but we didn't find
out until much later, which is why there's still left overs in the
DT. We're not using them in the driver for the A10 either (but we do
for the A20, obviously).
> > > +/* TODO: Check whether A10 actually supports osc32k as 4th parent? */
> > > +static const char *const ir_parents_sun4i[] = { "hosc", "pll-periph",
> > > + "pll-ddr-other" };
> >
> > What does the BSP say about this?
>
> sun7i datasheet mentions osc32k, but BSP code for sun4i, sun5i and sun7i
> is identical and supports only 3 first parents without osc32k.
Ok. Leave the TODO for now, we'll fix it if relevant.
> > > +/* Undocumented on A10 */
> > > +static SUNXI_CCU_MUX_WITH_GATE(spdif_clk, "spdif", audio_parents,
> > > + 0x0c0, 16, 2, BIT(31), CLK_SET_RATE_PARENT);
> >
> > This doesn't seem to exist at all on the A10
>
> Wasn't listed in datasheet, but it's in BSP and also in sun4i-a10.dtsi:
>
> spdif_clk: clk@01c200c0 {
> #clock-cells = <0>;
> compatible = "allwinner,sun4i-a10-mod1-clk";
> reg = <0x01c200c0 0x4>;
> clocks = <&pll2 SUN4I_A10_PLL2_8X>,
> <&pll2 SUN4I_A10_PLL2_4X>,
> <&pll2 SUN4I_A10_PLL2_2X>,
> <&pll2 SUN4I_A10_PLL2_1X>;
> clock-output-names = "spdif";
> };
Ack.
> > > +/*
> > > + * TODO: SATA clock also supports external clock as parent via BIT(24)
> > > + * The external clock is probably an optional crystal or oscillator
> > > + * that can be connected to the SATA-CLKM / SATA-CLKP pins.
> > > + */
> > > +static SUNXI_CCU_GATE(sata_clk, "sata", "pll-periph-sata",
> > > + 0x0c8, BIT(31), 0);
> >
> > The rate won't be good here either. This is supposed to be 100MHz.
>
> Hmm.. I tested SATA with Cubietruck. Or what do you mean?
As long as you don't have any dependency on the rate itself, as long
as the gate is opened, I expect it to work. But the rate itself will
be reported wrong.
> > > +static const char *const csi_isp_parents[] = { "pll-video0", "pll-ve",
> > > + "pll-ddr-other", "pll-sata" };
> > > +
> > > +static SUNXI_CCU_M_WITH_MUX_GATE(csi_isp_clk, "csi-isp",
> > > + csi_isp_parents,
> > > + 0x120, 0, 4, 24, 2, BIT(31), 0);
> >
> > We've been calling it sclk in the other SoC iirc. Any particular
> > reason to call it differently?
>
> It's called ISP in BSP and A10 manual.
> In A20 it's indeed Special Clock Register (SCLK).
Let's call it SCLK too then, for consistency.
> > > +static const char *const out_parents[] = { "hosc", "osc32k", "hosc" };
> > > +static SUNXI_CCU_MP_WITH_MUX_GATE(out_a_clk, "out-a", out_parents,
> > > + 0x1f0, 8, 5, 20, 2, 24, 2, BIT(31), 0);
> > > +static SUNXI_CCU_MP_WITH_MUX_GATE(out_b_clk, "out-b", out_parents,
> > > + 0x1f4, 8, 5, 20, 2, 24, 2, BIT(31), 0);
> >
> > There's a fixed pre-divider on the first hosc of 750.
>
> Nice catch.
>
> So it should be something like this:
>
> [snip]
> static const char *const out_parents[] = { "osc24M", "osc32k", "osc24M" };
> static const struct ccu_mux_fixed_prediv out_prediv = {
> .index = 0, .div = 750
> };
I think it shoud still be hosc (or at least, the name that you used
for the gate controlling the 24MHz oscillator input).
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web