Re: [PATCH 01/19] clk: sunxi: Add display clock
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 -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH 01/19] clk: sunxi: Add display clock
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 -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/19] clk: sunxi: Add display clock
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 -- Maxime Ripard, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH 01/19] clk: sunxi: Add display clock
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 > 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 > > --- > > 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 ..f13b095c6d7a > > --- /dev/null > > +++ b/drivers/clk/sunxi/clk-sun4i-display.c > > @@ -0,0 +1,199 @@ > > +/* > > + * Copyright 2015 Maxime Ripard > > + * > > + * Maxime Ripard > > + * > > + * 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 > > +#include > > +#include > > +#include > > +#include > > + > > +#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, Free Electrons Embedded Linux, Kernel and Android engineering http://free-electrons.com signature.asc Description: Digital signature
Re: [PATCH 01/19] clk: sunxi: Add display clock
Hi, On Fri, Oct 30, 2015 at 10:20 PM, Maxime Ripard 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 > --- > 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 ..f13b095c6d7a > --- /dev/null > +++ b/drivers/clk/sunxi/clk-sun4i-display.c > @@ -0,0 +1,199 @@ > +/* > + * Copyright 2015 Maxime Ripard > + * > + * Maxime Ripard > + * > + * 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 > +#include > +#include > +#include > +#include > + > +#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_BIT30 > +#define SUN4I_A10_DISPLAY_MUX_MASK 3 > +#define SUN4I_A10_DISPLAY_MUX_SHIFT24 > +#define SUN4I_A10_DISPLAY_DIV_WIDTH4 > +#define SUN4I_A10_DISPLAY_DIV_SHIFT0 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 01/19] clk: sunxi: Add display clock
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 ..f13b095c6d7a
> --- /dev/null
> +++ b/drivers/clk/sunxi/clk-sun4i-display.c
> @@ -0,0 +1,199 @@
> +/*
> + * Copyright 2015 Maxime Ripard
> + *
> + * Maxime Ripard
> + *
> + * 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 for container_of?
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define SUN4I_A10_DISPLAY_PARENTS3
> +
> +#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.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 01/19] clk: sunxi: Add display clock
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
---
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 ..f13b095c6d7a
--- /dev/null
+++ b/drivers/clk/sunxi/clk-sun4i-display.c
@@ -0,0 +1,199 @@
+/*
+ * Copyright 2015 Maxime Ripard
+ *
+ * Maxime Ripard
+ *
+ * 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
+#include
+#include
+#include
+#include
+
+#define SUN4I_A10_DISPLAY_PARENTS 3
+
+#define SUN4I_A10_DISPLAY_GATE_BIT 31
+#define SUN4I_A10_DISPLAY_RESET_BIT30
+#define SUN4I_A10_DISPLAY_MUX_MASK 3
+#define SUN4I_A10_DISPLAY_MUX_SHIFT24
+#define SUN4I_A10_DISPLAY_DIV_WIDTH4
+#define SUN4I_A10_DISPLAY_DIV_SHIFT0
+
+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+
