diff mbox

[01/19] clk: sunxi: Add display clock

Message ID 1446214865-3972-2-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 A10 SoCs and its relatives has a special clock controller to drive the
display engines (both frontend and backend).

Add a driver for it.

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

Comments

Stephen Boyd Oct. 30, 2015, 9:29 p.m. UTC | #1 
On 10/30, Maxime Ripard wrote:
> diff --git a/drivers/clk/sunxi/clk-sun4i-display.c b/drivers/clk/sunxi/clk-sun4i-display.c
> new file mode 100644
> index 000000000000..f13b095c6d7a
> --- /dev/null
> +++ b/drivers/clk/sunxi/clk-sun4i-display.c
> @@ -0,0 +1,199 @@
> +/*
> + * 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/kernel.h> for container_of?

> +#include <linux/clk-provider.h>
> +#include <linux/of_address.h>
> +#include <linux/reset-controller.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +
> +#define SUN4I_A10_DISPLAY_PARENTS	3
> +
> +#define SUN4I_A10_DISPLAY_GATE_BIT	31
> +#define SUN4I_A10_DISPLAY_RESET_BIT	30
> +#define SUN4I_A10_DISPLAY_MUX_MASK	3
> +#define SUN4I_A10_DISPLAY_MUX_SHIFT	24
> +#define SUN4I_A10_DISPLAY_DIV_WIDTH	4
> +#define SUN4I_A10_DISPLAY_DIV_SHIFT	0
> +
> +struct reset_data {
> +	void __iomem			*reg;
> +	spinlock_t			*lock;
> +	struct reset_controller_dev	rcdev;
> +};
> +
> +static DEFINE_SPINLOCK(sun4i_a10_display_lock);
> +
> +static int sun4i_a10_display_assert(struct reset_controller_dev *rcdev,
> +				    unsigned long id)
> +{
> +	struct reset_data *data = container_of(rcdev,
> +					       struct reset_data,
> +					       rcdev);

Can this be a macro rcdev_to_reset_data() or something?

> +	unsigned long flags;
[..]
> +
> +static int sun4i_a10_display_status(struct reset_controller_dev *rcdev,
> +				    unsigned long id)
> +{
> +	struct reset_data *data = container_of(rcdev,
> +					       struct reset_data,
> +					       rcdev);
> +
> +	return !(readl(data->reg) & BIT(SUN4I_A10_DISPLAY_RESET_BIT));
> +}
> +
> +static struct reset_control_ops sun4i_a10_display_reset_ops = {

Someone should make it so this can be const...

> +	.assert		= sun4i_a10_display_assert,
> +	.deassert	= sun4i_a10_display_deassert,
> +	.status		= sun4i_a10_display_status,
> +};
> +
> +static int sun4i_a10_display_reset_xlate(struct reset_controller_dev *rcdev,
> +					 const struct of_phandle_args *spec)
> +{
> +	if (WARN_ON(spec->args_count != rcdev->of_reset_n_cells))
> +		return -EINVAL;

Do we really need this check? Seems like something the reset core
should handle.

> +
> +	/* We only have a single reset signal */
> +	return 0;
> +}
> +
> +static void __init sun4i_a10_display_setup(struct device_node *node)
> +{
> +	const char *parents[SUN4I_A10_DISPLAY_PARENTS];
> +	const char *clk_name = node->name;
> +	struct reset_data *reset_data;
> +	struct clk_divider *div;
> +	struct clk_gate *gate;
> +	struct clk_mux *mux;
> +	void __iomem *reg;
> +	struct clk *clk;
> +	int i;
> +
> +	of_property_read_string(node, "clock-output-names", &clk_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", clk_name);
> +		return;
> +	}
> +
> +	for (i = 0; i < SUN4I_A10_DISPLAY_PARENTS; i++)
> +		parents[i] = of_clk_get_parent_name(node, i);

of_clk_parent_fill()?

> +
> +	mux = kzalloc(sizeof(*mux), GFP_KERNEL);
> +	if (!mux)
[..]
> +		goto free_reset;
> +	}
> +
> +	return;
> +
> +free_reset:
> +	kfree(reset_data);
> +free_clk:
> +	clk_unregister(clk);

We really ought to have a clk_composite_unregister() API.
Chen-Yu Tsai Oct. 31, 2015, 10:28 a.m. UTC | #2 
Hi,

On Fri, Oct 30, 2015 at 10:20 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> The A10 SoCs and its relatives has a special clock controller to drive the
> display engines (both frontend and backend).
>
> Add a driver for it.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---
>  drivers/clk/sunxi/Makefile            |   1 +
>  drivers/clk/sunxi/clk-sun4i-display.c | 199 ++++++++++++++++++++++++++++++++++
>  2 files changed, 200 insertions(+)
>  create mode 100644 drivers/clk/sunxi/clk-sun4i-display.c
>
> diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
> index cb4c299214ce..a9e1a5885846 100644
> --- a/drivers/clk/sunxi/Makefile
> +++ b/drivers/clk/sunxi/Makefile
> @@ -9,6 +9,7 @@ obj-y += clk-a10-mod1.o
>  obj-y += clk-a10-pll2.o
>  obj-y += clk-a20-gmac.o
>  obj-y += clk-mod0.o
> +obj-y += clk-sun4i-display.o
>  obj-y += clk-simple-gates.o
>  obj-y += clk-sun8i-mbus.o
>  obj-y += clk-sun9i-core.o
> diff --git a/drivers/clk/sunxi/clk-sun4i-display.c b/drivers/clk/sunxi/clk-sun4i-display.c
> new file mode 100644
> index 000000000000..f13b095c6d7a
> --- /dev/null
> +++ b/drivers/clk/sunxi/clk-sun4i-display.c
> @@ -0,0 +1,199 @@
> +/*
> + * 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/reset-controller.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
> +
> +#define SUN4I_A10_DISPLAY_PARENTS      3

Can we change this to 4, so we can reuse this for TCON clocks on sun4i/sun7i?

Thanks

ChenYu

> +#define SUN4I_A10_DISPLAY_GATE_BIT     31
> +#define SUN4I_A10_DISPLAY_RESET_BIT    30
> +#define SUN4I_A10_DISPLAY_MUX_MASK     3
> +#define SUN4I_A10_DISPLAY_MUX_SHIFT    24
> +#define SUN4I_A10_DISPLAY_DIV_WIDTH    4
> +#define SUN4I_A10_DISPLAY_DIV_SHIFT    0
Maxime Ripard Nov. 6, 2015, 7:42 p.m. UTC | #3 
Hi,

On Sat, Oct 31, 2015 at 06:28:16PM +0800, Chen-Yu Tsai wrote:
> Hi,
> 
> On Fri, Oct 30, 2015 at 10:20 PM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > The A10 SoCs and its relatives has a special clock controller to drive the
> > display engines (both frontend and backend).
> >
> > Add a driver for it.
> >
> > Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> > ---
> >  drivers/clk/sunxi/Makefile            |   1 +
> >  drivers/clk/sunxi/clk-sun4i-display.c | 199 ++++++++++++++++++++++++++++++++++
> >  2 files changed, 200 insertions(+)
> >  create mode 100644 drivers/clk/sunxi/clk-sun4i-display.c
> >
> > diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
> > index cb4c299214ce..a9e1a5885846 100644
> > --- a/drivers/clk/sunxi/Makefile
> > +++ b/drivers/clk/sunxi/Makefile
> > @@ -9,6 +9,7 @@ obj-y += clk-a10-mod1.o
> >  obj-y += clk-a10-pll2.o
> >  obj-y += clk-a20-gmac.o
> >  obj-y += clk-mod0.o
> > +obj-y += clk-sun4i-display.o
> >  obj-y += clk-simple-gates.o
> >  obj-y += clk-sun8i-mbus.o
> >  obj-y += clk-sun9i-core.o
> > diff --git a/drivers/clk/sunxi/clk-sun4i-display.c b/drivers/clk/sunxi/clk-sun4i-display.c
> > new file mode 100644
> > index 000000000000..f13b095c6d7a
> > --- /dev/null
> > +++ b/drivers/clk/sunxi/clk-sun4i-display.c
> > @@ -0,0 +1,199 @@
> > +/*
> > + * 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/reset-controller.h>
> > +#include <linux/slab.h>
> > +#include <linux/spinlock.h>
> > +
> > +#define SUN4I_A10_DISPLAY_PARENTS      3
> 
> Can we change this to 4, so we can reuse this for TCON clocks on
> sun4i/sun7i?

Good point, I'll change this.

Thanks!
Maxime
Maxime Ripard Nov. 6, 2015, 11:39 p.m. UTC | #4 
Hi Stephen,

Thanks for your feedback!

On Fri, Oct 30, 2015 at 02:29:02PM -0700, Stephen Boyd wrote:
> > +
> > +	mux = kzalloc(sizeof(*mux), GFP_KERNEL);
> > +	if (!mux)
> [..]
> > +		goto free_reset;
> > +	}
> > +
> > +	return;
> > +
> > +free_reset:
> > +	kfree(reset_data);
> > +free_clk:
> > +	clk_unregister(clk);
> 
> We really ought to have a clk_composite_unregister() API.

Can we?

We can always unregister the clock itself, but do we have a way to
retrieve the structure that has been allocated in there?

(note that it also applies to the generic clocks registration: muxes,
dividers, and so on).

Maxime
Stephen Boyd Nov. 12, 2015, 8:31 p.m. UTC | #5 
On 11/06, Maxime Ripard wrote:
> Hi Stephen,
> 
> Thanks for your feedback!
> 
> On Fri, Oct 30, 2015 at 02:29:02PM -0700, Stephen Boyd wrote:
> > > +
> > > +	mux = kzalloc(sizeof(*mux), GFP_KERNEL);
> > > +	if (!mux)
> > [..]
> > > +		goto free_reset;
> > > +	}
> > > +
> > > +	return;
> > > +
> > > +free_reset:
> > > +	kfree(reset_data);
> > > +free_clk:
> > > +	clk_unregister(clk);
> > 
> > We really ought to have a clk_composite_unregister() API.
> 
> Can we?
> 
> We can always unregister the clock itself, but do we have a way to
> retrieve the structure that has been allocated in there?
> 
> (note that it also applies to the generic clocks registration: muxes,
> dividers, and so on).

Yes we have a way. We've done the same sort of design for other
generic clocks. Do the __clk_get_hw() thing like in clk-divider.c
Maxime Ripard Nov. 19, 2015, 3:42 p.m. UTC | #6 
On Thu, Nov 12, 2015 at 12:31:54PM -0800, Stephen Boyd wrote:
> On 11/06, Maxime Ripard wrote:
> > Hi Stephen,
> > 
> > Thanks for your feedback!
> > 
> > On Fri, Oct 30, 2015 at 02:29:02PM -0700, Stephen Boyd wrote:
> > > > +
> > > > +	mux = kzalloc(sizeof(*mux), GFP_KERNEL);
> > > > +	if (!mux)
> > > [..]
> > > > +		goto free_reset;
> > > > +	}
> > > > +
> > > > +	return;
> > > > +
> > > > +free_reset:
> > > > +	kfree(reset_data);
> > > > +free_clk:
> > > > +	clk_unregister(clk);
> > > 
> > > We really ought to have a clk_composite_unregister() API.
> > 
> > Can we?
> > 
> > We can always unregister the clock itself, but do we have a way to
> > retrieve the structure that has been allocated in there?
> > 
> > (note that it also applies to the generic clocks registration: muxes,
> > dividers, and so on).
> 
> Yes we have a way. We've done the same sort of design for other
> generic clocks. Do the __clk_get_hw() thing like in clk-divider.c

Ack. I'll cook up a patch for this then.

Thanks!
Maxime
diff mbox

Patch

diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile
index cb4c299214ce..a9e1a5885846 100644
--- a/drivers/clk/sunxi/Makefile
+++ b/drivers/clk/sunxi/Makefile
@@ -9,6 +9,7 @@  obj-y += clk-a10-mod1.o
 obj-y += clk-a10-pll2.o
 obj-y += clk-a20-gmac.o
 obj-y += clk-mod0.o
+obj-y += clk-sun4i-display.o
 obj-y += clk-simple-gates.o
 obj-y += clk-sun8i-mbus.o
 obj-y += clk-sun9i-core.o
diff --git a/drivers/clk/sunxi/clk-sun4i-display.c b/drivers/clk/sunxi/clk-sun4i-display.c
new file mode 100644
index 000000000000..f13b095c6d7a
--- /dev/null
+++ b/drivers/clk/sunxi/clk-sun4i-display.c
@@ -0,0 +1,199 @@ 
+/*
+ * 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/reset-controller.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+#define SUN4I_A10_DISPLAY_PARENTS	3
+
+#define SUN4I_A10_DISPLAY_GATE_BIT	31
+#define SUN4I_A10_DISPLAY_RESET_BIT	30
+#define SUN4I_A10_DISPLAY_MUX_MASK	3
+#define SUN4I_A10_DISPLAY_MUX_SHIFT	24
+#define SUN4I_A10_DISPLAY_DIV_WIDTH	4
+#define SUN4I_A10_DISPLAY_DIV_SHIFT	0
+
+struct reset_data {
+	void __iomem			*reg;
+	spinlock_t			*lock;
+	struct reset_controller_dev	rcdev;
+};
+
+static DEFINE_SPINLOCK(sun4i_a10_display_lock);
+
+static int sun4i_a10_display_assert(struct reset_controller_dev *rcdev,
+				    unsigned long id)
+{
+	struct reset_data *data = container_of(rcdev,
+					       struct reset_data,
+					       rcdev);
+	unsigned long flags;
+	u32 reg;
+
+	spin_lock_irqsave(data->lock, flags);
+
+	reg = readl(data->reg);
+	writel(reg & ~BIT(SUN4I_A10_DISPLAY_RESET_BIT), data->reg);
+
+	spin_unlock_irqrestore(data->lock, flags);
+
+	return 0;
+}
+
+static int sun4i_a10_display_deassert(struct reset_controller_dev *rcdev,
+				      unsigned long id)
+{
+	struct reset_data *data = container_of(rcdev,
+						   struct reset_data,
+						   rcdev);
+	unsigned long flags;
+	u32 reg;
+
+	spin_lock_irqsave(data->lock, flags);
+
+	reg = readl(data->reg);
+	writel(reg | BIT(SUN4I_A10_DISPLAY_RESET_BIT), data->reg);
+
+	spin_unlock_irqrestore(data->lock, flags);
+
+	return 0;
+}
+
+static int sun4i_a10_display_status(struct reset_controller_dev *rcdev,
+				    unsigned long id)
+{
+	struct reset_data *data = container_of(rcdev,
+					       struct reset_data,
+					       rcdev);
+
+	return !(readl(data->reg) & BIT(SUN4I_A10_DISPLAY_RESET_BIT));
+}
+
+static struct reset_control_ops sun4i_a10_display_reset_ops = {
+	.assert		= sun4i_a10_display_assert,
+	.deassert	= sun4i_a10_display_deassert,
+	.status		= sun4i_a10_display_status,
+};
+
+static int sun4i_a10_display_reset_xlate(struct reset_controller_dev *rcdev,
+					 const struct of_phandle_args *spec)
+{
+	if (WARN_ON(spec->args_count != rcdev->of_reset_n_cells))
+		return -EINVAL;
+
+	/* We only have a single reset signal */
+	return 0;
+}
+
+static void __init sun4i_a10_display_setup(struct device_node *node)
+{
+	const char *parents[SUN4I_A10_DISPLAY_PARENTS];
+	const char *clk_name = node->name;
+	struct reset_data *reset_data;
+	struct clk_divider *div;
+	struct clk_gate *gate;
+	struct clk_mux *mux;
+	void __iomem *reg;
+	struct clk *clk;
+	int i;
+
+	of_property_read_string(node, "clock-output-names", &clk_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", clk_name);
+		return;
+	}
+
+	for (i = 0; i < SUN4I_A10_DISPLAY_PARENTS; i++)
+		parents[i] = of_clk_get_parent_name(node, i);
+
+	mux = kzalloc(sizeof(*mux), GFP_KERNEL);
+	if (!mux)
+		return;
+
+	mux->reg = reg;
+	mux->shift = SUN4I_A10_DISPLAY_MUX_SHIFT;
+	mux->mask = SUN4I_A10_DISPLAY_MUX_MASK;
+	mux->lock = &sun4i_a10_display_lock;
+
+	gate = kzalloc(sizeof(*gate), GFP_KERNEL);
+	if (!gate)
+		goto free_mux;
+
+	gate->reg = reg;
+	gate->bit_idx = SUN4I_A10_DISPLAY_GATE_BIT;
+	gate->lock = &sun4i_a10_display_lock;
+
+	div = kzalloc(sizeof(*div), GFP_KERNEL);
+	if (!div)
+		goto free_gate;
+
+	div->reg = reg;
+	div->shift = SUN4I_A10_DISPLAY_DIV_SHIFT;
+	div->width = SUN4I_A10_DISPLAY_DIV_WIDTH;
+	div->lock = &sun4i_a10_display_lock;
+
+	clk = clk_register_composite(NULL, clk_name,
+				     parents, SUN4I_A10_DISPLAY_PARENTS,
+				     &mux->hw, &clk_mux_ops,
+				     &div->hw, &clk_divider_ops,
+				     &gate->hw, &clk_gate_ops,
+				     0);
+	if (IS_ERR(clk)) {
+		pr_err("%s: Couldn't register the clock\n", clk_name);
+		goto free_div;
+	}
+
+	of_clk_add_provider(node, of_clk_src_simple_get, clk);
+
+	reset_data = kzalloc(sizeof(*reset_data), GFP_KERNEL);
+	if (!reset_data)
+		goto free_clk;
+
+	reset_data->reg = reg;
+	reset_data->lock = &sun4i_a10_display_lock;
+	reset_data->rcdev.nr_resets = 1;
+	reset_data->rcdev.ops = &sun4i_a10_display_reset_ops;
+	reset_data->rcdev.of_node = node;
+	reset_data->rcdev.of_reset_n_cells = 0;
+	reset_data->rcdev.of_xlate = &sun4i_a10_display_reset_xlate;
+
+	if (reset_controller_register(&reset_data->rcdev)) {
+		pr_err("%s: Couldn't register the reset controller\n",
+		       clk_name);
+		goto free_reset;
+	}
+
+	return;
+
+free_reset:
+	kfree(reset_data);
+free_clk:
+	clk_unregister(clk);
+free_div:
+	kfree(div);
+free_gate:
+	kfree(gate);
+free_mux:
+	kfree(mux);
+}
+
+CLK_OF_DECLARE(sun4i_a10_display, "allwinner,sun4i-a10-display-clk",
+	       sun4i_a10_display_setup);