| Message ID | 1415074039-16590-2-git-send-email-wens@csie.org |
|---|---|
| State | New |
| Headers | show |
Hi, On Tue, Nov 04, 2014 at 12:07:14PM +0800, Chen-Yu Tsai wrote: > The USB controller/phy clocks and reset controls are in a separate > address block, unlike previous SoCs where they were in the clock > controller. > > This patch copies the original gates clk functions used for usb > clocks into a separate file, and renames them to *_usb_*. Also > add a per-gate parent index, so we can set different parents for > each gate. > > In time we may move the other usb clock drivers to this file. > > Signed-off-by: Chen-Yu Tsai <wens@csie.org> > --- > Documentation/devicetree/bindings/clock/sunxi.txt | 5 + > drivers/clk/sunxi/Makefile | 1 + > drivers/clk/sunxi/clk-usb.c | 192 ++++++++++++++++++++++ > 3 files changed, 198 insertions(+) > create mode 100644 drivers/clk/sunxi/clk-usb.c > > diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt > index 0455cb9..b953fe5 100644 > --- a/Documentation/devicetree/bindings/clock/sunxi.txt > +++ b/Documentation/devicetree/bindings/clock/sunxi.txt > @@ -66,6 +66,8 @@ Required properties: > "allwinner,sun4i-a10-usb-clk" - for usb gates + resets on A10 / A20 > "allwinner,sun5i-a13-usb-clk" - for usb gates + resets on A13 > "allwinner,sun6i-a31-usb-clk" - for usb gates + resets on A31 > + "allwinner,sun9i-a80-usb-mod-clk" - for usb gates + resets on A80 > + "allwinner,sun9i-a80-usb-phy-clk" - for usb phy gates + resets on A80 > > Required properties for all clocks: > - reg : shall be the control register address for the clock. > @@ -82,6 +84,9 @@ Required properties for all clocks: > And "allwinner,*-usb-clk" clocks also require: > - reset-cells : shall be set to 1 > > +"allwinner,sun9i-a80-usb-*-clk" clocks require: > +- clocks : shall be the usb hci ahb1 gate and peripheral pll clocks > + In this particular order, I assume? > For "allwinner,sun7i-a20-gmac-clk", the parent clocks shall be fixed rate > dummy clocks at 25 MHz and 125 MHz, respectively. See example. > > diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile > index a66953c..f19ce54 100644 > --- a/drivers/clk/sunxi/Makefile > +++ b/drivers/clk/sunxi/Makefile > @@ -8,6 +8,7 @@ obj-y += clk-a20-gmac.o > obj-y += clk-mod0.o > obj-y += clk-sun8i-mbus.o > obj-y += clk-sun9i-core.o > +obj-y += clk-usb.o > > obj-$(CONFIG_MFD_SUN6I_PRCM) += \ > clk-sun6i-ar100.o clk-sun6i-apb0.o clk-sun6i-apb0-gates.o \ > diff --git a/drivers/clk/sunxi/clk-usb.c b/drivers/clk/sunxi/clk-usb.c > new file mode 100644 > index 0000000..d92ee36 > --- /dev/null > +++ b/drivers/clk/sunxi/clk-usb.c > @@ -0,0 +1,192 @@ > +/* > + * Copyright 2013 Emilio López > + * > + * Emilio López <emilio@elopez.com.ar> > + * > + * 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/clkdev.h> > +#include <linux/of.h> > +#include <linux/of_address.h> > +#include <linux/reset-controller.h> > +#include <linux/spinlock.h> > + > + > +/** > + * sunxi_usb_reset... - reset bits in usb clk registers handling > + */ > + > +struct usb_reset_data { > + void __iomem *reg; > + spinlock_t *lock; > + struct reset_controller_dev rcdev; > +}; > + > +static int sunxi_usb_reset_assert(struct reset_controller_dev *rcdev, > + unsigned long id) > +{ > + struct usb_reset_data *data = container_of(rcdev, > + struct usb_reset_data, > + rcdev); > + unsigned long flags; > + u32 reg; > + > + spin_lock_irqsave(data->lock, flags); > + > + reg = readl(data->reg); > + writel(reg & ~BIT(id), data->reg); > + > + spin_unlock_irqrestore(data->lock, flags); > + > + return 0; > +} > + > +static int sunxi_usb_reset_deassert(struct reset_controller_dev *rcdev, > + unsigned long id) > +{ > + struct usb_reset_data *data = container_of(rcdev, > + struct usb_reset_data, > + rcdev); > + unsigned long flags; > + u32 reg; > + > + spin_lock_irqsave(data->lock, flags); > + > + reg = readl(data->reg); > + writel(reg | BIT(id), data->reg); > + > + spin_unlock_irqrestore(data->lock, flags); > + > + return 0; > +} > + > +static struct reset_control_ops sunxi_usb_reset_ops = { > + .assert = sunxi_usb_reset_assert, > + .deassert = sunxi_usb_reset_deassert, > +}; > + > +/** > + * sunxi_usb_clk_setup() - Setup function for usb gate clocks > + */ > + > +#define SUNXI_USB_MAX_SIZE 32 > + > +struct usb_clk_data { > + u32 clk_mask; > + u32 reset_mask; > + /* which parent to use, should match clock-output-names */ > + char parents[SUNXI_USB_MAX_SIZE]; > +}; > + > +static void __init sunxi_usb_clk_setup(struct device_node *node, > + const struct usb_clk_data *data, > + spinlock_t *lock) > +{ > + struct clk_onecell_data *clk_data; > + struct usb_reset_data *reset_data; > + const char *clk_parent; > + const char *clk_name; > + void __iomem *reg; > + int qty; > + int i = 0; > + int j = 0; > + > + reg = of_iomap(node, 0); of_io_request_and_map? > + > + /* Worst-case size approximation and memory allocation */ > + qty = find_last_bit((unsigned long *)&data->clk_mask, > + SUNXI_USB_MAX_SIZE); > + clk_data = kmalloc(sizeof(struct clk_onecell_data), GFP_KERNEL); > + if (!clk_data) > + return; > + clk_data->clks = kzalloc((qty+1) * sizeof(struct clk *), GFP_KERNEL); > + if (!clk_data->clks) { > + kfree(clk_data); > + return; > + } > + > + for_each_set_bit(i, (unsigned long *)&data->clk_mask, > + SUNXI_USB_MAX_SIZE) { > + of_property_read_string_index(node, "clock-output-names", > + j, &clk_name); > + clk_parent = of_clk_get_parent_name(node, data->parents[j]); > + > + clk_data->clks[i] = clk_register_gate(NULL, clk_name, > + clk_parent, 0, > + reg, i, 0, lock); > + WARN_ON(IS_ERR(clk_data->clks[i])); > + clk_register_clkdev(clk_data->clks[i], clk_name, NULL); > + > + j++; > + } > + > + /* Adjust to the real max */ > + clk_data->clk_num = i; > + > + of_clk_add_provider(node, of_clk_src_onecell_get, clk_data); > + > + /* Register a reset controller for usb with reset bits */ > + if (data->reset_mask == 0) > + return; > + > + reset_data = kzalloc(sizeof(*reset_data), GFP_KERNEL); > + if (!reset_data) > + return; > + > + reset_data->reg = reg; > + reset_data->lock = lock; > + reset_data->rcdev.nr_resets = __fls(data->reset_mask) + 1; > + reset_data->rcdev.ops = &sunxi_usb_reset_ops; > + reset_data->rcdev.of_node = node; > + reset_controller_register(&reset_data->rcdev); > +} > + > +static const struct usb_clk_data sun9i_a80_usb_mod_data __initconst = { > + .clk_mask = BIT(6) | BIT(5) | BIT(4) | BIT(3) | BIT(2) | BIT(1), > + .reset_mask = BIT(19) | BIT(18) | BIT(17), > + .parents = {0, 1, 0, 1, 0, 1}, > +}; > + > +static DEFINE_SPINLOCK(a80_usb_mod_lock); > + > +static void __init sun9i_a80_usb_mod_setup(struct device_node *node) > +{ > + /* AHB1 gate must be enabled to access registers */ > + struct clk *ahb = of_clk_get(node, 0); > + > + WARN_ON(IS_ERR(ahb)); > + clk_prepare_enable(ahb); Hmmmm. That look off. Why do you need the clock to be enabled all the time? Isn't the CCF already taking care of enabling the parent clock whenever it needs to access any register? Thanks, Maxime
On Tue, Nov 04, 2014 at 12:07:14PM +0800, Chen-Yu Tsai wrote: > + spin_lock_irqsave(data->lock, flags); > + > + reg = readl(data->reg); > + writel(reg & ~BIT(id), data->reg); > + > + spin_unlock_irqrestore(data->lock, flags); Don't we have generic support for atomic modification of register values? Hmm, we have it for ARM only - atomic_io_modify() and atomic_io_modify_relaxed(). I guess we should push for those to become cross-arch if we end up with generic drivers shared between other architectures.
Hi Russell, On Tue, Nov 04, 2014 at 06:12:19PM +0000, Russell King - ARM Linux wrote: > On Tue, Nov 04, 2014 at 12:07:14PM +0800, Chen-Yu Tsai wrote: > > + spin_lock_irqsave(data->lock, flags); > > + > > + reg = readl(data->reg); > > + writel(reg & ~BIT(id), data->reg); > > + > > + spin_unlock_irqrestore(data->lock, flags); > > Don't we have generic support for atomic modification of register > values? Hmm, we have it for ARM only - atomic_io_modify() and > atomic_io_modify_relaxed(). > > I guess we should push for those to become cross-arch if we end up > with generic drivers shared between other architectures. IIRC, the atomic MMIO accessors are doing exactly that, but with a global lock for all MMIO accesses, while here we protect a single register. I'm not really sure that sharing this spinlock across the whole system is worth it and scales that well. Maxime
Hi, On Wed, Nov 5, 2014 at 12:57 AM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > Hi, > > On Tue, Nov 04, 2014 at 12:07:14PM +0800, Chen-Yu Tsai wrote: >> The USB controller/phy clocks and reset controls are in a separate >> address block, unlike previous SoCs where they were in the clock >> controller. >> >> This patch copies the original gates clk functions used for usb >> clocks into a separate file, and renames them to *_usb_*. Also >> add a per-gate parent index, so we can set different parents for >> each gate. >> >> In time we may move the other usb clock drivers to this file. >> >> Signed-off-by: Chen-Yu Tsai <wens@csie.org> >> --- >> Documentation/devicetree/bindings/clock/sunxi.txt | 5 + >> drivers/clk/sunxi/Makefile | 1 + >> drivers/clk/sunxi/clk-usb.c | 192 ++++++++++++++++++++++ >> 3 files changed, 198 insertions(+) >> create mode 100644 drivers/clk/sunxi/clk-usb.c >> >> diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt >> index 0455cb9..b953fe5 100644 >> --- a/Documentation/devicetree/bindings/clock/sunxi.txt >> +++ b/Documentation/devicetree/bindings/clock/sunxi.txt >> @@ -66,6 +66,8 @@ Required properties: >> "allwinner,sun4i-a10-usb-clk" - for usb gates + resets on A10 / A20 >> "allwinner,sun5i-a13-usb-clk" - for usb gates + resets on A13 >> "allwinner,sun6i-a31-usb-clk" - for usb gates + resets on A31 >> + "allwinner,sun9i-a80-usb-mod-clk" - for usb gates + resets on A80 >> + "allwinner,sun9i-a80-usb-phy-clk" - for usb phy gates + resets on A80 >> >> Required properties for all clocks: >> - reg : shall be the control register address for the clock. >> @@ -82,6 +84,9 @@ Required properties for all clocks: >> And "allwinner,*-usb-clk" clocks also require: >> - reset-cells : shall be set to 1 >> >> +"allwinner,sun9i-a80-usb-*-clk" clocks require: >> +- clocks : shall be the usb hci ahb1 gate and peripheral pll clocks >> + > > In this particular order, I assume? Yes, I will make it clear. >> For "allwinner,sun7i-a20-gmac-clk", the parent clocks shall be fixed rate >> dummy clocks at 25 MHz and 125 MHz, respectively. See example. >> >> diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile >> index a66953c..f19ce54 100644 >> --- a/drivers/clk/sunxi/Makefile >> +++ b/drivers/clk/sunxi/Makefile >> @@ -8,6 +8,7 @@ obj-y += clk-a20-gmac.o >> obj-y += clk-mod0.o >> obj-y += clk-sun8i-mbus.o >> obj-y += clk-sun9i-core.o >> +obj-y += clk-usb.o >> >> obj-$(CONFIG_MFD_SUN6I_PRCM) += \ >> clk-sun6i-ar100.o clk-sun6i-apb0.o clk-sun6i-apb0-gates.o \ >> diff --git a/drivers/clk/sunxi/clk-usb.c b/drivers/clk/sunxi/clk-usb.c >> new file mode 100644 >> index 0000000..d92ee36 >> --- /dev/null >> +++ b/drivers/clk/sunxi/clk-usb.c >> @@ -0,0 +1,192 @@ >> +/* >> + * Copyright 2013 Emilio López >> + * >> + * Emilio López <emilio@elopez.com.ar> >> + * >> + * 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/clkdev.h> >> +#include <linux/of.h> >> +#include <linux/of_address.h> >> +#include <linux/reset-controller.h> >> +#include <linux/spinlock.h> >> + >> + >> +/** >> + * sunxi_usb_reset... - reset bits in usb clk registers handling >> + */ >> + >> +struct usb_reset_data { >> + void __iomem *reg; >> + spinlock_t *lock; >> + struct reset_controller_dev rcdev; >> +}; >> + >> +static int sunxi_usb_reset_assert(struct reset_controller_dev *rcdev, >> + unsigned long id) >> +{ >> + struct usb_reset_data *data = container_of(rcdev, >> + struct usb_reset_data, >> + rcdev); >> + unsigned long flags; >> + u32 reg; >> + >> + spin_lock_irqsave(data->lock, flags); >> + >> + reg = readl(data->reg); >> + writel(reg & ~BIT(id), data->reg); >> + >> + spin_unlock_irqrestore(data->lock, flags); >> + >> + return 0; >> +} >> + >> +static int sunxi_usb_reset_deassert(struct reset_controller_dev *rcdev, >> + unsigned long id) >> +{ >> + struct usb_reset_data *data = container_of(rcdev, >> + struct usb_reset_data, >> + rcdev); >> + unsigned long flags; >> + u32 reg; >> + >> + spin_lock_irqsave(data->lock, flags); >> + >> + reg = readl(data->reg); >> + writel(reg | BIT(id), data->reg); >> + >> + spin_unlock_irqrestore(data->lock, flags); >> + >> + return 0; >> +} >> + >> +static struct reset_control_ops sunxi_usb_reset_ops = { >> + .assert = sunxi_usb_reset_assert, >> + .deassert = sunxi_usb_reset_deassert, >> +}; >> + >> +/** >> + * sunxi_usb_clk_setup() - Setup function for usb gate clocks >> + */ >> + >> +#define SUNXI_USB_MAX_SIZE 32 >> + >> +struct usb_clk_data { >> + u32 clk_mask; >> + u32 reset_mask; >> + /* which parent to use, should match clock-output-names */ >> + char parents[SUNXI_USB_MAX_SIZE]; >> +}; >> + >> +static void __init sunxi_usb_clk_setup(struct device_node *node, >> + const struct usb_clk_data *data, >> + spinlock_t *lock) >> +{ >> + struct clk_onecell_data *clk_data; >> + struct usb_reset_data *reset_data; >> + const char *clk_parent; >> + const char *clk_name; >> + void __iomem *reg; >> + int qty; >> + int i = 0; >> + int j = 0; >> + >> + reg = of_iomap(node, 0); > > of_io_request_and_map? OK. About that, any recommended naming style for the 3rd argument? Maybe the driver name "clk_sun9i_usb"? Or just a generic name like "usb_clk"? I'm asking now as we'll likely be changing the existing drivers to use it as well. >> + >> + /* Worst-case size approximation and memory allocation */ >> + qty = find_last_bit((unsigned long *)&data->clk_mask, >> + SUNXI_USB_MAX_SIZE); >> + clk_data = kmalloc(sizeof(struct clk_onecell_data), GFP_KERNEL); >> + if (!clk_data) >> + return; >> + clk_data->clks = kzalloc((qty+1) * sizeof(struct clk *), GFP_KERNEL); >> + if (!clk_data->clks) { >> + kfree(clk_data); >> + return; >> + } >> + >> + for_each_set_bit(i, (unsigned long *)&data->clk_mask, >> + SUNXI_USB_MAX_SIZE) { >> + of_property_read_string_index(node, "clock-output-names", >> + j, &clk_name); >> + clk_parent = of_clk_get_parent_name(node, data->parents[j]); >> + >> + clk_data->clks[i] = clk_register_gate(NULL, clk_name, >> + clk_parent, 0, >> + reg, i, 0, lock); >> + WARN_ON(IS_ERR(clk_data->clks[i])); >> + clk_register_clkdev(clk_data->clks[i], clk_name, NULL); >> + >> + j++; >> + } >> + >> + /* Adjust to the real max */ >> + clk_data->clk_num = i; >> + >> + of_clk_add_provider(node, of_clk_src_onecell_get, clk_data); >> + >> + /* Register a reset controller for usb with reset bits */ >> + if (data->reset_mask == 0) >> + return; >> + >> + reset_data = kzalloc(sizeof(*reset_data), GFP_KERNEL); >> + if (!reset_data) >> + return; >> + >> + reset_data->reg = reg; >> + reset_data->lock = lock; >> + reset_data->rcdev.nr_resets = __fls(data->reset_mask) + 1; >> + reset_data->rcdev.ops = &sunxi_usb_reset_ops; >> + reset_data->rcdev.of_node = node; >> + reset_controller_register(&reset_data->rcdev); >> +} >> + >> +static const struct usb_clk_data sun9i_a80_usb_mod_data __initconst = { >> + .clk_mask = BIT(6) | BIT(5) | BIT(4) | BIT(3) | BIT(2) | BIT(1), >> + .reset_mask = BIT(19) | BIT(18) | BIT(17), >> + .parents = {0, 1, 0, 1, 0, 1}, >> +}; >> + >> +static DEFINE_SPINLOCK(a80_usb_mod_lock); >> + >> +static void __init sun9i_a80_usb_mod_setup(struct device_node *node) >> +{ >> + /* AHB1 gate must be enabled to access registers */ >> + struct clk *ahb = of_clk_get(node, 0); >> + >> + WARN_ON(IS_ERR(ahb)); >> + clk_prepare_enable(ahb); > > Hmmmm. That look off. > > Why do you need the clock to be enabled all the time? Isn't the CCF > already taking care of enabling the parent clock whenever it needs to > access any register? There are also resets in the same block. That and I couldn't get it working without enabling the clock beforehand. ChenYu
On Wed, Nov 05, 2014 at 06:02:35PM +0800, Chen-Yu Tsai wrote: > >> +static void __init sunxi_usb_clk_setup(struct device_node *node, > >> + const struct usb_clk_data *data, > >> + spinlock_t *lock) > >> +{ > >> + struct clk_onecell_data *clk_data; > >> + struct usb_reset_data *reset_data; > >> + const char *clk_parent; > >> + const char *clk_name; > >> + void __iomem *reg; > >> + int qty; > >> + int i = 0; > >> + int j = 0; > >> + > >> + reg = of_iomap(node, 0); > > > > of_io_request_and_map? > > OK. About that, any recommended naming style for the 3rd argument? > Maybe the driver name "clk_sun9i_usb"? Or just a generic name like > "usb_clk"? > > I'm asking now as we'll likely be changing the existing drivers to > use it as well. I don't really have a preference. Maybe the DT node name would be both the easier and better solution. [...] > >> +static void __init sun9i_a80_usb_mod_setup(struct device_node *node) > >> +{ > >> + /* AHB1 gate must be enabled to access registers */ > >> + struct clk *ahb = of_clk_get(node, 0); > >> + > >> + WARN_ON(IS_ERR(ahb)); > >> + clk_prepare_enable(ahb); > > > > Hmmmm. That look off. > > > > Why do you need the clock to be enabled all the time? Isn't the CCF > > already taking care of enabling the parent clock whenever it needs to > > access any register? > > There are also resets in the same block. That and I couldn't get it > working without enabling the clock beforehand. Ah, right. What happens if you just enable and disable the clocks in the reset_assert and reset_deassert right before and after accessing the registers? Maxime
On Wed, Nov 5, 2014 at 6:09 PM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > On Wed, Nov 05, 2014 at 06:02:35PM +0800, Chen-Yu Tsai wrote: >> >> +static void __init sunxi_usb_clk_setup(struct device_node *node, >> >> + const struct usb_clk_data *data, >> >> + spinlock_t *lock) >> >> +{ >> >> + struct clk_onecell_data *clk_data; >> >> + struct usb_reset_data *reset_data; >> >> + const char *clk_parent; >> >> + const char *clk_name; >> >> + void __iomem *reg; >> >> + int qty; >> >> + int i = 0; >> >> + int j = 0; >> >> + >> >> + reg = of_iomap(node, 0); >> > >> > of_io_request_and_map? >> >> OK. About that, any recommended naming style for the 3rd argument? >> Maybe the driver name "clk_sun9i_usb"? Or just a generic name like >> "usb_clk"? >> >> I'm asking now as we'll likely be changing the existing drivers to >> use it as well. > > I don't really have a preference. Maybe the DT node name would be both > the easier and better solution. Using of_node_full_name() then. > [...] > >> >> +static void __init sun9i_a80_usb_mod_setup(struct device_node *node) >> >> +{ >> >> + /* AHB1 gate must be enabled to access registers */ >> >> + struct clk *ahb = of_clk_get(node, 0); >> >> + >> >> + WARN_ON(IS_ERR(ahb)); >> >> + clk_prepare_enable(ahb); >> > >> > Hmmmm. That look off. >> > >> > Why do you need the clock to be enabled all the time? Isn't the CCF >> > already taking care of enabling the parent clock whenever it needs to >> > access any register? >> >> There are also resets in the same block. That and I couldn't get it >> working without enabling the clock beforehand. > > Ah, right. > > What happens if you just enable and disable the clocks in the > reset_assert and reset_deassert right before and after accessing the > registers? That doesn't work either. I forgot to mention that most of the clock gates have the peripheral pll as their parent, not the ahb clock gate. Since most of the clocks are special clocks @ 12M, 48M, or 480M, which on previous SoCs were driven by PLL6 or another special PLL, it seems to make sense to assume the same on A80 and set them this way. ChenYu
On Thu, Nov 06, 2014 at 10:09:27AM +0800, Chen-Yu Tsai wrote: > >> >> +static void __init sun9i_a80_usb_mod_setup(struct device_node *node) > >> >> +{ > >> >> + /* AHB1 gate must be enabled to access registers */ > >> >> + struct clk *ahb = of_clk_get(node, 0); > >> >> + > >> >> + WARN_ON(IS_ERR(ahb)); > >> >> + clk_prepare_enable(ahb); > >> > > >> > Hmmmm. That look off. > >> > > >> > Why do you need the clock to be enabled all the time? Isn't the CCF > >> > already taking care of enabling the parent clock whenever it needs to > >> > access any register? > >> > >> There are also resets in the same block. That and I couldn't get it > >> working without enabling the clock beforehand. > > > > Ah, right. > > > > What happens if you just enable and disable the clocks in the > > reset_assert and reset_deassert right before and after accessing the > > registers? > > That doesn't work either. I forgot to mention that most of the clock > gates have the peripheral pll as their parent, not the ahb clock gate. Why it doesn't work? The clock needs more time to stabilize? The reset line is set back in reset if the clocks are disabled? Maxime
On Thu, Nov 6, 2014 at 4:54 PM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > On Thu, Nov 06, 2014 at 10:09:27AM +0800, Chen-Yu Tsai wrote: >> >> >> +static void __init sun9i_a80_usb_mod_setup(struct device_node *node) >> >> >> +{ >> >> >> + /* AHB1 gate must be enabled to access registers */ >> >> >> + struct clk *ahb = of_clk_get(node, 0); >> >> >> + >> >> >> + WARN_ON(IS_ERR(ahb)); >> >> >> + clk_prepare_enable(ahb); >> >> > >> >> > Hmmmm. That look off. >> >> > >> >> > Why do you need the clock to be enabled all the time? Isn't the CCF >> >> > already taking care of enabling the parent clock whenever it needs to >> >> > access any register? >> >> >> >> There are also resets in the same block. That and I couldn't get it >> >> working without enabling the clock beforehand. >> > >> > Ah, right. >> > >> > What happens if you just enable and disable the clocks in the >> > reset_assert and reset_deassert right before and after accessing the >> > registers? >> >> That doesn't work either. I forgot to mention that most of the clock >> gates have the peripheral pll as their parent, not the ahb clock gate. > > Why it doesn't work? The clock needs more time to stabilize? The reset > line is set back in reset if the clocks are disabled? Let me clarify, what you proposed will work for the resets. However the clock gates won't work if we use the generic clk-gate driver. The problem is most of the gates don't have the ahb gate as their parent, but pll4 (peripheral pll). When we enable the clock, the ahb gate isn't its parent, and doesn't get enabled as a result. This is especially true for the usb phy clocks: all of them use pll4 as their parent. I think this is a better representation of the hardware, but without documents this is really just a guess. As a whole, I think enabling the clock gate at the beginning is simpler. ChenYu
Hi, Sorry for the belated answer. On Thu, Nov 06, 2014 at 05:19:24PM +0800, Chen-Yu Tsai wrote: > On Thu, Nov 6, 2014 at 4:54 PM, Maxime Ripard > <maxime.ripard@free-electrons.com> wrote: > > On Thu, Nov 06, 2014 at 10:09:27AM +0800, Chen-Yu Tsai wrote: > >> >> >> +static void __init sun9i_a80_usb_mod_setup(struct device_node *node) > >> >> >> +{ > >> >> >> + /* AHB1 gate must be enabled to access registers */ > >> >> >> + struct clk *ahb = of_clk_get(node, 0); > >> >> >> + > >> >> >> + WARN_ON(IS_ERR(ahb)); > >> >> >> + clk_prepare_enable(ahb); > >> >> > > >> >> > Hmmmm. That look off. > >> >> > > >> >> > Why do you need the clock to be enabled all the time? Isn't the CCF > >> >> > already taking care of enabling the parent clock whenever it needs to > >> >> > access any register? > >> >> > >> >> There are also resets in the same block. That and I couldn't get it > >> >> working without enabling the clock beforehand. > >> > > >> > Ah, right. > >> > > >> > What happens if you just enable and disable the clocks in the > >> > reset_assert and reset_deassert right before and after accessing the > >> > registers? > >> > >> That doesn't work either. I forgot to mention that most of the clock > >> gates have the peripheral pll as their parent, not the ahb clock gate. > > > > Why it doesn't work? The clock needs more time to stabilize? The reset > > line is set back in reset if the clocks are disabled? > > Let me clarify, what you proposed will work for the resets. > > However the clock gates won't work if we use the generic clk-gate driver. > The problem is most of the gates don't have the ahb gate as their parent, > but pll4 (peripheral pll). When we enable the clock, the ahb gate isn't > its parent, and doesn't get enabled as a result. This is especially true > for the usb phy clocks: all of them use pll4 as their parent. I'm not sure I get this right. You mean that this USB clock needs *both* pll4 and its AHB gates to be enabled in order to run properly? Or that the PHY needs its AHB gate to be enabled? Both ways, I still don't think it's the right thing to do. Maxime
Hi, On Fri, Nov 14, 2014 at 4:39 PM, Maxime Ripard <maxime.ripard@free-electrons.com> wrote: > Hi, > > Sorry for the belated answer. > > On Thu, Nov 06, 2014 at 05:19:24PM +0800, Chen-Yu Tsai wrote: >> On Thu, Nov 6, 2014 at 4:54 PM, Maxime Ripard >> <maxime.ripard@free-electrons.com> wrote: >> > On Thu, Nov 06, 2014 at 10:09:27AM +0800, Chen-Yu Tsai wrote: >> >> >> >> +static void __init sun9i_a80_usb_mod_setup(struct device_node *node) >> >> >> >> +{ >> >> >> >> + /* AHB1 gate must be enabled to access registers */ >> >> >> >> + struct clk *ahb = of_clk_get(node, 0); >> >> >> >> + >> >> >> >> + WARN_ON(IS_ERR(ahb)); >> >> >> >> + clk_prepare_enable(ahb); >> >> >> > >> >> >> > Hmmmm. That look off. >> >> >> > >> >> >> > Why do you need the clock to be enabled all the time? Isn't the CCF >> >> >> > already taking care of enabling the parent clock whenever it needs to >> >> >> > access any register? >> >> >> >> >> >> There are also resets in the same block. That and I couldn't get it >> >> >> working without enabling the clock beforehand. >> >> > >> >> > Ah, right. >> >> > >> >> > What happens if you just enable and disable the clocks in the >> >> > reset_assert and reset_deassert right before and after accessing the >> >> > registers? >> >> >> >> That doesn't work either. I forgot to mention that most of the clock >> >> gates have the peripheral pll as their parent, not the ahb clock gate. >> > >> > Why it doesn't work? The clock needs more time to stabilize? The reset >> > line is set back in reset if the clocks are disabled? >> >> Let me clarify, what you proposed will work for the resets. >> >> However the clock gates won't work if we use the generic clk-gate driver. >> The problem is most of the gates don't have the ahb gate as their parent, >> but pll4 (peripheral pll). When we enable the clock, the ahb gate isn't >> its parent, and doesn't get enabled as a result. This is especially true >> for the usb phy clocks: all of them use pll4 as their parent. > > I'm not sure I get this right. You mean that this USB clock needs > *both* pll4 and its AHB gates to be enabled in order to run properly? > > Or that the PHY needs its AHB gate to be enabled? What I meant was the USB clocks should have some backing PLL. But let's just use the AHB gate for all the parents and keep it simple for now. We can model it accurately after we get documentation. > Both ways, I still don't think it's the right thing to do. As said above, I'll just use ahb gate as the parent, and do what you said earlier about the reset controls (toggle the gate before and after touching the register). ChenYu
diff --git a/Documentation/devicetree/bindings/clock/sunxi.txt b/Documentation/devicetree/bindings/clock/sunxi.txt index 0455cb9..b953fe5 100644 --- a/Documentation/devicetree/bindings/clock/sunxi.txt +++ b/Documentation/devicetree/bindings/clock/sunxi.txt @@ -66,6 +66,8 @@ Required properties: "allwinner,sun4i-a10-usb-clk" - for usb gates + resets on A10 / A20 "allwinner,sun5i-a13-usb-clk" - for usb gates + resets on A13 "allwinner,sun6i-a31-usb-clk" - for usb gates + resets on A31 + "allwinner,sun9i-a80-usb-mod-clk" - for usb gates + resets on A80 + "allwinner,sun9i-a80-usb-phy-clk" - for usb phy gates + resets on A80 Required properties for all clocks: - reg : shall be the control register address for the clock. @@ -82,6 +84,9 @@ Required properties for all clocks: And "allwinner,*-usb-clk" clocks also require: - reset-cells : shall be set to 1 +"allwinner,sun9i-a80-usb-*-clk" clocks require: +- clocks : shall be the usb hci ahb1 gate and peripheral pll clocks + For "allwinner,sun7i-a20-gmac-clk", the parent clocks shall be fixed rate dummy clocks at 25 MHz and 125 MHz, respectively. See example. diff --git a/drivers/clk/sunxi/Makefile b/drivers/clk/sunxi/Makefile index a66953c..f19ce54 100644 --- a/drivers/clk/sunxi/Makefile +++ b/drivers/clk/sunxi/Makefile @@ -8,6 +8,7 @@ obj-y += clk-a20-gmac.o obj-y += clk-mod0.o obj-y += clk-sun8i-mbus.o obj-y += clk-sun9i-core.o +obj-y += clk-usb.o obj-$(CONFIG_MFD_SUN6I_PRCM) += \ clk-sun6i-ar100.o clk-sun6i-apb0.o clk-sun6i-apb0-gates.o \ diff --git a/drivers/clk/sunxi/clk-usb.c b/drivers/clk/sunxi/clk-usb.c new file mode 100644 index 0000000..d92ee36 --- /dev/null +++ b/drivers/clk/sunxi/clk-usb.c @@ -0,0 +1,192 @@ +/* + * Copyright 2013 Emilio López + * + * Emilio López <emilio@elopez.com.ar> + * + * 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/clkdev.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/reset-controller.h> +#include <linux/spinlock.h> + + +/** + * sunxi_usb_reset... - reset bits in usb clk registers handling + */ + +struct usb_reset_data { + void __iomem *reg; + spinlock_t *lock; + struct reset_controller_dev rcdev; +}; + +static int sunxi_usb_reset_assert(struct reset_controller_dev *rcdev, + unsigned long id) +{ + struct usb_reset_data *data = container_of(rcdev, + struct usb_reset_data, + rcdev); + unsigned long flags; + u32 reg; + + spin_lock_irqsave(data->lock, flags); + + reg = readl(data->reg); + writel(reg & ~BIT(id), data->reg); + + spin_unlock_irqrestore(data->lock, flags); + + return 0; +} + +static int sunxi_usb_reset_deassert(struct reset_controller_dev *rcdev, + unsigned long id) +{ + struct usb_reset_data *data = container_of(rcdev, + struct usb_reset_data, + rcdev); + unsigned long flags; + u32 reg; + + spin_lock_irqsave(data->lock, flags); + + reg = readl(data->reg); + writel(reg | BIT(id), data->reg); + + spin_unlock_irqrestore(data->lock, flags); + + return 0; +} + +static struct reset_control_ops sunxi_usb_reset_ops = { + .assert = sunxi_usb_reset_assert, + .deassert = sunxi_usb_reset_deassert, +}; + +/** + * sunxi_usb_clk_setup() - Setup function for usb gate clocks + */ + +#define SUNXI_USB_MAX_SIZE 32 + +struct usb_clk_data { + u32 clk_mask; + u32 reset_mask; + /* which parent to use, should match clock-output-names */ + char parents[SUNXI_USB_MAX_SIZE]; +}; + +static void __init sunxi_usb_clk_setup(struct device_node *node, + const struct usb_clk_data *data, + spinlock_t *lock) +{ + struct clk_onecell_data *clk_data; + struct usb_reset_data *reset_data; + const char *clk_parent; + const char *clk_name; + void __iomem *reg; + int qty; + int i = 0; + int j = 0; + + reg = of_iomap(node, 0); + + /* Worst-case size approximation and memory allocation */ + qty = find_last_bit((unsigned long *)&data->clk_mask, + SUNXI_USB_MAX_SIZE); + clk_data = kmalloc(sizeof(struct clk_onecell_data), GFP_KERNEL); + if (!clk_data) + return; + clk_data->clks = kzalloc((qty+1) * sizeof(struct clk *), GFP_KERNEL); + if (!clk_data->clks) { + kfree(clk_data); + return; + } + + for_each_set_bit(i, (unsigned long *)&data->clk_mask, + SUNXI_USB_MAX_SIZE) { + of_property_read_string_index(node, "clock-output-names", + j, &clk_name); + clk_parent = of_clk_get_parent_name(node, data->parents[j]); + + clk_data->clks[i] = clk_register_gate(NULL, clk_name, + clk_parent, 0, + reg, i, 0, lock); + WARN_ON(IS_ERR(clk_data->clks[i])); + clk_register_clkdev(clk_data->clks[i], clk_name, NULL); + + j++; + } + + /* Adjust to the real max */ + clk_data->clk_num = i; + + of_clk_add_provider(node, of_clk_src_onecell_get, clk_data); + + /* Register a reset controller for usb with reset bits */ + if (data->reset_mask == 0) + return; + + reset_data = kzalloc(sizeof(*reset_data), GFP_KERNEL); + if (!reset_data) + return; + + reset_data->reg = reg; + reset_data->lock = lock; + reset_data->rcdev.nr_resets = __fls(data->reset_mask) + 1; + reset_data->rcdev.ops = &sunxi_usb_reset_ops; + reset_data->rcdev.of_node = node; + reset_controller_register(&reset_data->rcdev); +} + +static const struct usb_clk_data sun9i_a80_usb_mod_data __initconst = { + .clk_mask = BIT(6) | BIT(5) | BIT(4) | BIT(3) | BIT(2) | BIT(1), + .reset_mask = BIT(19) | BIT(18) | BIT(17), + .parents = {0, 1, 0, 1, 0, 1}, +}; + +static DEFINE_SPINLOCK(a80_usb_mod_lock); + +static void __init sun9i_a80_usb_mod_setup(struct device_node *node) +{ + /* AHB1 gate must be enabled to access registers */ + struct clk *ahb = of_clk_get(node, 0); + + WARN_ON(IS_ERR(ahb)); + clk_prepare_enable(ahb); + + sunxi_usb_clk_setup(node, &sun9i_a80_usb_mod_data, &a80_usb_mod_lock); +} +CLK_OF_DECLARE(sun9i_a80_usb_mod, "allwinner,sun9i-a80-usb-mod-clk", sun9i_a80_usb_mod_setup); + +static const struct usb_clk_data sun9i_a80_usb_phy_data __initconst = { + .clk_mask = BIT(10) | BIT(5) | BIT(4) | BIT(3) | BIT(2) | BIT(1), + .reset_mask = BIT(21) | BIT(20) | BIT(19) | BIT(18) | BIT(17), + .parents = {1, 1, 1, 1, 1, 1}, +}; + +static DEFINE_SPINLOCK(a80_usb_phy_lock); + +static void __init sun9i_a80_usb_phy_setup(struct device_node *node) +{ + /* AHB1 gate must be enabled to access registers */ + struct clk *ahb = of_clk_get(node, 0); + + WARN_ON(IS_ERR(ahb)); + clk_prepare_enable(ahb); + + sunxi_usb_clk_setup(node, &sun9i_a80_usb_phy_data, &a80_usb_phy_lock); +} +CLK_OF_DECLARE(sun9i_a80_usb_phy, "allwinner,sun9i-a80-usb-phy-clk", sun9i_a80_usb_phy_setup);
The USB controller/phy clocks and reset controls are in a separate address block, unlike previous SoCs where they were in the clock controller. This patch copies the original gates clk functions used for usb clocks into a separate file, and renames them to *_usb_*. Also add a per-gate parent index, so we can set different parents for each gate. In time we may move the other usb clock drivers to this file. Signed-off-by: Chen-Yu Tsai <wens@csie.org> --- Documentation/devicetree/bindings/clock/sunxi.txt | 5 + drivers/clk/sunxi/Makefile | 1 + drivers/clk/sunxi/clk-usb.c | 192 ++++++++++++++++++++++ 3 files changed, 198 insertions(+) create mode 100644 drivers/clk/sunxi/clk-usb.c