Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754226AbaKEKDD (ORCPT ); Wed, 5 Nov 2014 05:03:03 -0500 Received: from mail-ig0-f176.google.com ([209.85.213.176]:35383 "EHLO mail-ig0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751302AbaKEKC5 convert rfc822-to-8bit (ORCPT ); Wed, 5 Nov 2014 05:02:57 -0500 MIME-Version: 1.0 In-Reply-To: <20141104165733.GI26729@lukather> References: <1415074039-16590-1-git-send-email-wens@csie.org> <1415074039-16590-2-git-send-email-wens@csie.org> <20141104165733.GI26729@lukather> From: Chen-Yu Tsai Date: Wed, 5 Nov 2014 18:02:35 +0800 X-Google-Sender-Auth: M_nGUbdrwixtyfb4VJEySz3U4V4 Message-ID: Subject: Re: [PATCH 1/6] clk: sunxi: Add support for sun9i a80 usb clocks and resets To: Maxime Ripard Cc: Kishon Vijay Abraham I , Mike Turquette , Grant Likely , Rob Herring , Hans de Goede , linux-arm-kernel , linux-kernel , linux-sunxi Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On Wed, Nov 5, 2014 at 12:57 AM, Maxime Ripard 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 >> --- >> 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 >> + * >> + * 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 >> +#include >> + >> + >> +/** >> + * 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/