diff mbox

[04/19] clk: sunxi: Add TCON channel1 clock

Message ID 1446214865-3972-5-git-send-email-maxime.ripard@free-electrons.com (mailing list archive)
State New, archived
Headers show

Commit Message

Maxime Ripard Oct. 30, 2015, 2:20 p.m. UTC
The TCON is a controller generating the timings to output videos signals,
acting like both a CRTC and an encoder.

It has two channels depending on the output, each channel being driven by
its own clock (and own clock controller).

Add a driver for the channel 1 clock.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clk/sunxi/Makefile             |   1 +
 drivers/clk/sunxi/clk-sun4i-tcon-ch1.c | 167 +++++++++++++++++++++++++++++++++
 2 files changed, 168 insertions(+)
 create mode 100644 drivers/clk/sunxi/clk-sun4i-tcon-ch1.c

Comments

Stephen Boyd Oct. 30, 2015, 9:37 p.m. UTC | #1
On 10/30, Maxime Ripard wrote:
> The TCON is a controller generating the timings to output videos signals,
> acting like both a CRTC and an encoder.
> 
> It has two channels depending on the output, each channel being driven by
> its own clock (and own clock controller).
> 
> Add a driver for the channel 1 clock.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Similar comments apply to patches 3 and 4. Was the same code
copy/pasted two more times and then changed to have different
values? Looks like we should consolidate all that stuff into
something more generic so that we don't have the same problems 3
times.
Chen-Yu Tsai Oct. 31, 2015, 9:53 a.m. UTC | #2
On Fri, Oct 30, 2015 at 10:20 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> The TCON is a controller generating the timings to output videos signals,
> acting like both a CRTC and an encoder.
>
> It has two channels depending on the output, each channel being driven by
> its own clock (and own clock controller).
>
> Add a driver for the channel 1 clock.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  drivers/clk/sunxi/Makefile             |   1 +
>  drivers/clk/sunxi/clk-sun4i-tcon-ch1.c | 167 +++++++++++++++++++++++++++++++++
>  2 files changed, 168 insertions(+)
>  create mode 100644 drivers/clk/sunxi/clk-sun4i-tcon-ch1.c

According to the documents I have, this variant of the TCON clock
is specific to sun5i. On sun4i/sun7i, TCON CH1 clock has the same
layout as TCON CH0 and the other display clocks.

>
> diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
> index 7821b2b63d58..ed59ee4b3a85 100644
> --- a/drivers/clk/sunxi/Makefile
> +++ b/drivers/clk/sunxi/Makefile
> @@ -13,6 +13,7 @@ obj-y += clk-simple-gates.o
>  obj-y += clk-sun4i-display.o
>  obj-y += clk-sun4i-pll3.o
>  obj-y += clk-sun4i-tcon-ch0.o
> +obj-y += clk-sun4i-tcon-ch1.o
>  obj-y += clk-sun8i-mbus.o
>  obj-y += clk-sun9i-core.o
>  obj-y += clk-sun9i-mmc.o
> diff --git a/drivers/clk/sunxi/clk-sun4i-tcon-ch1.c b/drivers/clk/sunxi/clk-sun4i-tcon-ch1.c
> new file mode 100644
> index 000000000000..9f2ab52b0bf9
> --- /dev/null
> +++ b/drivers/clk/sunxi/clk-sun4i-tcon-ch1.c
> @@ -0,0 +1,167 @@
> +/*
> + * Copyright 2015 Maxime Ripard
> + *
> + * Maxime Ripard <maxime.ripard@free-electrons.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/clk-provider.h>
> +#include <linux/of_address.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +
> +#define SUN4I_TCON_CH1_SCLK_NAME_LEN   32
> +
> +#define SUN4I_A10_TCON_CH1_SCLK1_PARENTS       2
> +#define SUN4I_A10_TCON_CH1_SCLK2_PARENTS       4
> +
> +#define SUN4I_A10_TCON_CH1_SCLK2_GATE_BIT      31
> +#define SUN4I_A10_TCON_CH1_SCLK2_MUX_MASK      3
> +#define SUN4I_A10_TCON_CH1_SCLK2_MUX_SHIFT     24
> +#define SUN4I_A10_TCON_CH1_SCLK2_DIV_WIDTH     4
> +#define SUN4I_A10_TCON_CH1_SCLK2_DIV_SHIFT     0
> +
> +#define SUN4I_A10_TCON_CH1_SCLK1_GATE_BIT      15
> +#define SUN4I_A10_TCON_CH1_SCLK1_MUX_MASK      1
> +#define SUN4I_A10_TCON_CH1_SCLK1_MUX_SHIFT     11
> +
> +static DEFINE_SPINLOCK(sun4i_a10_tcon_ch1_lock);
> +
> +static void __init sun4i_a10_tcon_ch1_setup(struct device_node *node)
> +{
> +       const char *sclk1_parents[SUN4I_A10_TCON_CH1_SCLK1_PARENTS];
> +       const char *sclk2_parents[SUN4I_A10_TCON_CH1_SCLK2_PARENTS];
> +       const char *sclk1_name = node->name;
> +       char sclk2_name[SUN4I_TCON_CH1_SCLK_NAME_LEN];
> +       char sclk2d2_name[SUN4I_TCON_CH1_SCLK_NAME_LEN];
> +       struct clk_gate *sclk1_gate, *sclk2_gate;
> +       struct clk_mux *sclk1_mux, *sclk2_mux;
> +       struct clk *sclk1, *sclk2, *sclk2d2;
> +       struct clk_divider *sclk2_div;
> +       void __iomem *reg;
> +       int i;
> +
> +       of_property_read_string(node, "clock-output-names",
> +                               &sclk1_name);
> +
> +       snprintf(sclk2_name, SUN4I_TCON_CH1_SCLK_NAME_LEN,
> +                "%s2", sclk1_name);
> +
> +       snprintf(sclk2d2_name, SUN4I_TCON_CH1_SCLK_NAME_LEN,
> +                "%s2d2", sclk1_name);
> +
> +       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", sclk2_name);
> +               return;
> +       }
> +
> +       for (i = 0; i < SUN4I_A10_TCON_CH1_SCLK2_PARENTS; i++)
> +               sclk2_parents[i] = of_clk_get_parent_name(node, i);
> +
> +       sclk2_mux = kzalloc(sizeof(*sclk2_mux), GFP_KERNEL);
> +       if (!sclk2_mux)
> +               return;
> +
> +       sclk2_mux->reg = reg;
> +       sclk2_mux->shift = SUN4I_A10_TCON_CH1_SCLK2_MUX_SHIFT;
> +       sclk2_mux->mask = SUN4I_A10_TCON_CH1_SCLK2_MUX_MASK;
> +       sclk2_mux->lock = &sun4i_a10_tcon_ch1_lock;
> +
> +       sclk2_gate = kzalloc(sizeof(*sclk2_gate), GFP_KERNEL);
> +       if (!sclk2_gate)
> +               goto free_sclk2_mux;
> +
> +       sclk2_gate->reg = reg;
> +       sclk2_gate->bit_idx = SUN4I_A10_TCON_CH1_SCLK2_GATE_BIT;
> +       sclk2_gate->lock = &sun4i_a10_tcon_ch1_lock;
> +
> +       sclk2_div = kzalloc(sizeof(*sclk2_div), GFP_KERNEL);
> +       if (!sclk2_div)
> +               goto free_sclk2_gate;
> +
> +       sclk2_div->reg = reg;
> +       sclk2_div->shift = SUN4I_A10_TCON_CH1_SCLK2_DIV_SHIFT;
> +       sclk2_div->width = SUN4I_A10_TCON_CH1_SCLK2_DIV_WIDTH;
> +       sclk2_div->lock = &sun4i_a10_tcon_ch1_lock;
> +
> +       sclk2 = clk_register_composite(NULL, sclk2_name, sclk2_parents,
> +                                      SUN4I_A10_TCON_CH1_SCLK2_PARENTS,
> +                                      &sclk2_mux->hw, &clk_mux_ops,
> +                                      &sclk2_div->hw, &clk_divider_ops,
> +                                      &sclk2_gate->hw, &clk_gate_ops,
> +                                      0);
> +       if (IS_ERR(sclk2)) {
> +               pr_err("%s: Couldn't register the clock\n", sclk2_name);
> +               goto free_sclk2_div;
> +       }
> +
> +       sclk2d2 = clk_register_fixed_factor(NULL, sclk2d2_name, sclk2_name, 0,
> +                                           1, 2);
> +       if (IS_ERR(sclk2d2)) {
> +               pr_err("%s: Couldn't register the clock\n", sclk2d2_name);
> +               goto free_sclk2;
> +       }
> +
> +       sclk1_parents[0] = sclk2_name;
> +       sclk1_parents[1] = sclk2d2_name;

Is there any need to expose these 2 clocks via DT using of_clk_add_provider?

Note that these complex clock trees within a clock node breaks the
assigned-clock-parents mechanism, as you can no longer specify the output
clock's direct parents.


Regards
ChenYu

> +
> +       sclk1_mux = kzalloc(sizeof(*sclk1_mux), GFP_KERNEL);
> +       if (!sclk1_mux)
> +               goto free_sclk2d2;
> +
> +       sclk1_mux->reg = reg;
> +       sclk1_mux->shift = SUN4I_A10_TCON_CH1_SCLK1_MUX_SHIFT;
> +       sclk1_mux->mask = SUN4I_A10_TCON_CH1_SCLK1_MUX_MASK;
> +       sclk1_mux->lock = &sun4i_a10_tcon_ch1_lock;
> +
> +       sclk1_gate = kzalloc(sizeof(*sclk1_gate), GFP_KERNEL);
> +       if (!sclk1_gate)
> +               goto free_sclk1_mux;
> +
> +       sclk1_gate->reg = reg;
> +       sclk1_gate->bit_idx = SUN4I_A10_TCON_CH1_SCLK1_GATE_BIT;
> +       sclk1_gate->lock = &sun4i_a10_tcon_ch1_lock;
> +
> +       sclk1 = clk_register_composite(NULL, sclk1_name, sclk1_parents,
> +                                      SUN4I_A10_TCON_CH1_SCLK1_PARENTS,
> +                                      &sclk1_mux->hw, &clk_mux_ops,
> +                                      NULL, NULL,
> +                                      &sclk1_gate->hw, &clk_gate_ops,
> +                                      0);
> +       if (IS_ERR(sclk1)) {
> +               pr_err("%s: Couldn't register the clock\n", sclk1_name);
> +               goto free_sclk1_gate;
> +       }
> +
> +       of_clk_add_provider(node, of_clk_src_simple_get, sclk1);
> +
> +       return;
> +
> +free_sclk1_gate:
> +       kfree(sclk1_gate);
> +free_sclk1_mux:
> +       kfree(sclk1_mux);
> +free_sclk2d2:
> +       clk_unregister(sclk2d2);
> +free_sclk2:
> +       clk_unregister(sclk2);
> +free_sclk2_div:
> +       kfree(sclk2_div);
> +free_sclk2_gate:
> +       kfree(sclk2_gate);
> +free_sclk2_mux:
> +       kfree(sclk2_mux);
> +}
> +
> +CLK_OF_DECLARE(sun4i_a10_tcon_ch1, "allwinner,sun4i-a10-tcon-ch1-clk",
> +              sun4i_a10_tcon_ch1_setup);
> --
> 2.6.2
>
Maxime Ripard Nov. 7, 2015, 12:01 a.m. UTC | #3
Hi,

On Sat, Oct 31, 2015 at 05:53:26PM +0800, Chen-Yu Tsai wrote:
> On Fri, Oct 30, 2015 at 10:20 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > The TCON is a controller generating the timings to output videos signals,
> > acting like both a CRTC and an encoder.
> >
> > It has two channels depending on the output, each channel being driven by
> > its own clock (and own clock controller).
> >
> > Add a driver for the channel 1 clock.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> >  drivers/clk/sunxi/Makefile             |   1 +
> >  drivers/clk/sunxi/clk-sun4i-tcon-ch1.c | 167 +++++++++++++++++++++++++++++++++
> >  2 files changed, 168 insertions(+)
> >  create mode 100644 drivers/clk/sunxi/clk-sun4i-tcon-ch1.c
> 
> According to the documents I have, this variant of the TCON clock
> is specific to sun5i. On sun4i/sun7i, TCON CH1 clock has the same
> layout as TCON CH0 and the other display clocks.

At least for the A20, it's not true.

Make sure you do not confuse LCD1 CH0 (p79, which is a channel 0
clock), with LCD0 CH1 (p81, which is a channel 1 clock).

> > +       sclk1_parents[0] = sclk2_name;
> > +       sclk1_parents[1] = sclk2d2_name;
> 
> Is there any need to expose these 2 clocks via DT using of_clk_add_provider?

No, as far as I'm aware, there's no user external to this clock
driver.

> Note that these complex clock trees within a clock node breaks the
> assigned-clock-parents mechanism, as you can no longer specify the output
> clock's direct parents.

There's no point of changing the parent either. Hardware blocks are
always connected to the leaf clock (sclk1). We could also model it as
an extra 1-bit divider, which would simplify a bit the logic though.

Maxime
Maxime Ripard Nov. 7, 2015, 12:11 a.m. UTC | #4
Hi,

On Fri, Oct 30, 2015 at 02:37:34PM -0700, Stephen Boyd wrote:
> On 10/30, Maxime Ripard wrote:
> > The TCON is a controller generating the timings to output videos signals,
> > acting like both a CRTC and an encoder.
> > 
> > It has two channels depending on the output, each channel being driven by
> > its own clock (and own clock controller).
> > 
> > Add a driver for the channel 1 clock.
> > 
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> 
> Similar comments apply to patches 3 and 4. Was the same code
> copy/pasted two more times and then changed to have different
> values?

I don't really recall, but I probably used the same skeleton yeah.

> Looks like we should consolidate all that stuff into something more
> generic so that we don't have the same problems 3 times.

Does it?

They're both pretty different actually. One is a gate + mux + reset
(patch 3), the other one is actually a combination of two clocks, one
that is the parent of the other, the former being a gate + mux, the
latter a gate + div.

At least at the hardware level, they're very different, and adding
more code to deal with both case would complicate quite a lot the
probe code, for no real reasons.

Maxime
Chen-Yu Tsai Nov. 9, 2015, 3:36 a.m. UTC | #5
On Sat, Nov 7, 2015 at 8:01 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi,
>
> On Sat, Oct 31, 2015 at 05:53:26PM +0800, Chen-Yu Tsai wrote:
>> On Fri, Oct 30, 2015 at 10:20 PM, Maxime Ripard
>> <maxime.ripard@free-electrons.com> wrote:
>> > The TCON is a controller generating the timings to output videos signals,
>> > acting like both a CRTC and an encoder.
>> >
>> > It has two channels depending on the output, each channel being driven by
>> > its own clock (and own clock controller).
>> >
>> > Add a driver for the channel 1 clock.
>> >
>> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>> > ---
>> >  drivers/clk/sunxi/Makefile             |   1 +
>> >  drivers/clk/sunxi/clk-sun4i-tcon-ch1.c | 167 +++++++++++++++++++++++++++++++++
>> >  2 files changed, 168 insertions(+)
>> >  create mode 100644 drivers/clk/sunxi/clk-sun4i-tcon-ch1.c
>>
>> According to the documents I have, this variant of the TCON clock
>> is specific to sun5i. On sun4i/sun7i, TCON CH1 clock has the same
>> layout as TCON CH0 and the other display clocks.
>
> At least for the A20, it's not true.
>
> Make sure you do not confuse LCD1 CH0 (p79, which is a channel 0
> clock), with LCD0 CH1 (p81, which is a channel 1 clock).

Right. The names are great for confusing the reader. :(

>> > +       sclk1_parents[0] = sclk2_name;
>> > +       sclk1_parents[1] = sclk2d2_name;
>>
>> Is there any need to expose these 2 clocks via DT using of_clk_add_provider?
>
> No, as far as I'm aware, there's no user external to this clock
> driver.
>
>> Note that these complex clock trees within a clock node breaks the
>> assigned-clock-parents mechanism, as you can no longer specify the output
>> clock's direct parents.
>
> There's no point of changing the parent either. Hardware blocks are
> always connected to the leaf clock (sclk1). We could also model it as
> an extra 1-bit divider, which would simplify a bit the logic though.

Probably not. You still have a gate to handle. It's just moving the
divider from 1 clock to the other. I think the current approach of
modeling it like the hardware is better.

About reparenting, what I meant was if sclk2 is not exposed through
of_clk_add_provider, then we can't do assigned-clocks stuff on it,
like setting a default parent or making each channel use a different
source pll.

What I'm saying is if it is not expected to work with another core
binding, we should probably note it somewhere.


Regards
ChenYu
Maxime Ripard Nov. 19, 2015, 3:35 p.m. UTC | #6
On Mon, Nov 09, 2015 at 11:36:15AM +0800, Chen-Yu Tsai wrote:
> >> > +       sclk1_parents[0] = sclk2_name;
> >> > +       sclk1_parents[1] = sclk2d2_name;
> >>
> >> Is there any need to expose these 2 clocks via DT using of_clk_add_provider?
> >
> > No, as far as I'm aware, there's no user external to this clock
> > driver.
> >
> >> Note that these complex clock trees within a clock node breaks the
> >> assigned-clock-parents mechanism, as you can no longer specify the output
> >> clock's direct parents.
> >
> > There's no point of changing the parent either. Hardware blocks are
> > always connected to the leaf clock (sclk1). We could also model it as
> > an extra 1-bit divider, which would simplify a bit the logic though.
> 
> Probably not. You still have a gate to handle. It's just moving the
> divider from 1 clock to the other. I think the current approach of
> modeling it like the hardware is better.

Not really if you model it using sclk2 being a mux + gate, and sclk1
being a divider + gate. It works great using the composite clocks.

> About reparenting, what I meant was if sclk2 is not exposed through
> of_clk_add_provider, then we can't do assigned-clocks stuff on it,
> like setting a default parent or making each channel use a different
> source pll.

And we don't really want to. Using the divider allow us to simply set
the rate of sclk1, and the mux / divider will do the rest. Since only
sclk1 is exposed to the rest of the system, we do not really care
about the rate of sclk2 anyway.

> What I'm saying is if it is not expected to work with another core
> binding, we should probably note it somewhere.

Indeed.

Maxime
diff mbox

Patch

diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
index 7821b2b63d58..ed59ee4b3a85 100644
--- a/drivers/clk/sunxi/Makefile
+++ b/drivers/clk/sunxi/Makefile
@@ -13,6 +13,7 @@  obj-y += clk-simple-gates.o
 obj-y += clk-sun4i-display.o
 obj-y += clk-sun4i-pll3.o
 obj-y += clk-sun4i-tcon-ch0.o
+obj-y += clk-sun4i-tcon-ch1.o
 obj-y += clk-sun8i-mbus.o
 obj-y += clk-sun9i-core.o
 obj-y += clk-sun9i-mmc.o
diff --git a/drivers/clk/sunxi/clk-sun4i-tcon-ch1.c b/drivers/clk/sunxi/clk-sun4i-tcon-ch1.c
new file mode 100644
index 000000000000..9f2ab52b0bf9
--- /dev/null
+++ b/drivers/clk/sunxi/clk-sun4i-tcon-ch1.c
@@ -0,0 +1,167 @@ 
+/*
+ * Copyright 2015 Maxime Ripard
+ *
+ * Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/of_address.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#define SUN4I_TCON_CH1_SCLK_NAME_LEN	32
+
+#define SUN4I_A10_TCON_CH1_SCLK1_PARENTS	2
+#define SUN4I_A10_TCON_CH1_SCLK2_PARENTS	4
+
+#define SUN4I_A10_TCON_CH1_SCLK2_GATE_BIT	31
+#define SUN4I_A10_TCON_CH1_SCLK2_MUX_MASK	3
+#define SUN4I_A10_TCON_CH1_SCLK2_MUX_SHIFT	24
+#define SUN4I_A10_TCON_CH1_SCLK2_DIV_WIDTH	4
+#define SUN4I_A10_TCON_CH1_SCLK2_DIV_SHIFT	0
+
+#define SUN4I_A10_TCON_CH1_SCLK1_GATE_BIT	15
+#define SUN4I_A10_TCON_CH1_SCLK1_MUX_MASK	1
+#define SUN4I_A10_TCON_CH1_SCLK1_MUX_SHIFT	11
+
+static DEFINE_SPINLOCK(sun4i_a10_tcon_ch1_lock);
+
+static void __init sun4i_a10_tcon_ch1_setup(struct device_node *node)
+{
+	const char *sclk1_parents[SUN4I_A10_TCON_CH1_SCLK1_PARENTS];
+	const char *sclk2_parents[SUN4I_A10_TCON_CH1_SCLK2_PARENTS];
+	const char *sclk1_name = node->name;
+	char sclk2_name[SUN4I_TCON_CH1_SCLK_NAME_LEN];
+	char sclk2d2_name[SUN4I_TCON_CH1_SCLK_NAME_LEN];
+	struct clk_gate *sclk1_gate, *sclk2_gate;
+	struct clk_mux *sclk1_mux, *sclk2_mux;
+	struct clk *sclk1, *sclk2, *sclk2d2;
+	struct clk_divider *sclk2_div;
+	void __iomem *reg;
+	int i;
+
+	of_property_read_string(node, "clock-output-names",
+				&sclk1_name);
+
+	snprintf(sclk2_name, SUN4I_TCON_CH1_SCLK_NAME_LEN,
+		 "%s2", sclk1_name);
+
+	snprintf(sclk2d2_name, SUN4I_TCON_CH1_SCLK_NAME_LEN,
+		 "%s2d2", sclk1_name);
+
+	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", sclk2_name);
+		return;
+	}
+
+	for (i = 0; i < SUN4I_A10_TCON_CH1_SCLK2_PARENTS; i++)
+		sclk2_parents[i] = of_clk_get_parent_name(node, i);
+
+	sclk2_mux = kzalloc(sizeof(*sclk2_mux), GFP_KERNEL);
+	if (!sclk2_mux)
+		return;
+
+	sclk2_mux->reg = reg;
+	sclk2_mux->shift = SUN4I_A10_TCON_CH1_SCLK2_MUX_SHIFT;
+	sclk2_mux->mask = SUN4I_A10_TCON_CH1_SCLK2_MUX_MASK;
+	sclk2_mux->lock = &sun4i_a10_tcon_ch1_lock;
+
+	sclk2_gate = kzalloc(sizeof(*sclk2_gate), GFP_KERNEL);
+	if (!sclk2_gate)
+		goto free_sclk2_mux;
+
+	sclk2_gate->reg = reg;
+	sclk2_gate->bit_idx = SUN4I_A10_TCON_CH1_SCLK2_GATE_BIT;
+	sclk2_gate->lock = &sun4i_a10_tcon_ch1_lock;
+
+	sclk2_div = kzalloc(sizeof(*sclk2_div), GFP_KERNEL);
+	if (!sclk2_div)
+		goto free_sclk2_gate;
+
+	sclk2_div->reg = reg;
+	sclk2_div->shift = SUN4I_A10_TCON_CH1_SCLK2_DIV_SHIFT;
+	sclk2_div->width = SUN4I_A10_TCON_CH1_SCLK2_DIV_WIDTH;
+	sclk2_div->lock = &sun4i_a10_tcon_ch1_lock;
+
+	sclk2 = clk_register_composite(NULL, sclk2_name, sclk2_parents,
+				       SUN4I_A10_TCON_CH1_SCLK2_PARENTS,
+				       &sclk2_mux->hw, &clk_mux_ops,
+				       &sclk2_div->hw, &clk_divider_ops,
+				       &sclk2_gate->hw, &clk_gate_ops,
+				       0);
+	if (IS_ERR(sclk2)) {
+		pr_err("%s: Couldn't register the clock\n", sclk2_name);
+		goto free_sclk2_div;
+	}
+
+	sclk2d2 = clk_register_fixed_factor(NULL, sclk2d2_name, sclk2_name, 0,
+					    1, 2);
+	if (IS_ERR(sclk2d2)) {
+		pr_err("%s: Couldn't register the clock\n", sclk2d2_name);
+		goto free_sclk2;
+	}
+
+	sclk1_parents[0] = sclk2_name;
+	sclk1_parents[1] = sclk2d2_name;
+
+	sclk1_mux = kzalloc(sizeof(*sclk1_mux), GFP_KERNEL);
+	if (!sclk1_mux)
+		goto free_sclk2d2;
+
+	sclk1_mux->reg = reg;
+	sclk1_mux->shift = SUN4I_A10_TCON_CH1_SCLK1_MUX_SHIFT;
+	sclk1_mux->mask = SUN4I_A10_TCON_CH1_SCLK1_MUX_MASK;
+	sclk1_mux->lock = &sun4i_a10_tcon_ch1_lock;
+
+	sclk1_gate = kzalloc(sizeof(*sclk1_gate), GFP_KERNEL);
+	if (!sclk1_gate)
+		goto free_sclk1_mux;
+
+	sclk1_gate->reg = reg;
+	sclk1_gate->bit_idx = SUN4I_A10_TCON_CH1_SCLK1_GATE_BIT;
+	sclk1_gate->lock = &sun4i_a10_tcon_ch1_lock;
+
+	sclk1 = clk_register_composite(NULL, sclk1_name, sclk1_parents,
+				       SUN4I_A10_TCON_CH1_SCLK1_PARENTS,
+				       &sclk1_mux->hw, &clk_mux_ops,
+				       NULL, NULL,
+				       &sclk1_gate->hw, &clk_gate_ops,
+				       0);
+	if (IS_ERR(sclk1)) {
+		pr_err("%s: Couldn't register the clock\n", sclk1_name);
+		goto free_sclk1_gate;
+	}
+
+	of_clk_add_provider(node, of_clk_src_simple_get, sclk1);
+
+	return;
+
+free_sclk1_gate:
+	kfree(sclk1_gate);
+free_sclk1_mux:
+	kfree(sclk1_mux);
+free_sclk2d2:
+	clk_unregister(sclk2d2);
+free_sclk2:
+	clk_unregister(sclk2);
+free_sclk2_div:
+	kfree(sclk2_div);
+free_sclk2_gate:
+	kfree(sclk2_gate);
+free_sclk2_mux:
+	kfree(sclk2_mux);
+}
+
+CLK_OF_DECLARE(sun4i_a10_tcon_ch1, "allwinner,sun4i-a10-tcon-ch1-clk",
+	       sun4i_a10_tcon_ch1_setup);