| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAGb2v65VojZVEijdjXYGkMud7dMFzMzagfN=_4S3msZygu3YPQ@mail.gmail.com>
Date: Wed, 5 Nov 2014 18:02:35 +0800
From: Chen-Yu Tsai <wens@...e.org>
To: Maxime Ripard <maxime.ripard@...e-electrons.com>
Cc: Kishon Vijay Abraham I <kishon@...com>,
Mike Turquette <mturquette@...aro.org>,
Grant Likely <grant.likely@...aro.org>,
Rob Herring <robh+dt@...nel.org>,
Hans de Goede <hdegoede@...hat.com>,
linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>,
linux-kernel <linux-kernel@...r.kernel.org>,
linux-sunxi <linux-sunxi@...glegroups.com>
Subject: Re: [PATCH 1/6] clk: sunxi: Add support for sun9i a80 usb clocks and resets
Hi,
On Wed, Nov 5, 2014 at 12:57 AM, Maxime Ripard
<maxime.ripard@...e-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@...e.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@...pez.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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists