From: maxime.ripard@free-electrons.com (Maxime Ripard)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/6] clk: sunxi: Add support for sun9i a80 usb clocks and resets
Date: Fri, 14 Nov 2014 09:39:17 +0100 [thread overview]
Message-ID: <20141114083917.GT20972@lukather> (raw)
In-Reply-To: <CAGb2v66Riu43PEzR2vLbg9r_WvZDLvxWtwAc3z43FkF57O2ksg@mail.gmail.com>
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
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20141114/4934bbf4/attachment.sig>
WARNING: multiple messages have this Message-ID (diff)
From: Maxime Ripard <maxime.ripard@free-electrons.com>
To: Chen-Yu Tsai <wens@csie.org>
Cc: Kishon Vijay Abraham I <kishon@ti.com>,
Mike Turquette <mturquette@linaro.org>,
Grant Likely <grant.likely@linaro.org>,
Rob Herring <robh+dt@kernel.org>,
Hans de Goede <hdegoede@redhat.com>,
linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
linux-kernel <linux-kernel@vger.kernel.org>,
linux-sunxi <linux-sunxi@googlegroups.com>
Subject: Re: [PATCH 1/6] clk: sunxi: Add support for sun9i a80 usb clocks and resets
Date: Fri, 14 Nov 2014 09:39:17 +0100 [thread overview]
Message-ID: <20141114083917.GT20972@lukather> (raw)
In-Reply-To: <CAGb2v66Riu43PEzR2vLbg9r_WvZDLvxWtwAc3z43FkF57O2ksg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2292 bytes --]
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
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
next prev parent reply other threads:[~2014-11-14 8:39 UTC|newest]
Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-04 4:07 [PATCH 0/6] ARM: sun9i: Add USB host controller support for A80 Chen-Yu Tsai
2014-11-04 4:07 ` Chen-Yu Tsai
2014-11-04 4:07 ` [PATCH 1/6] clk: sunxi: Add support for sun9i a80 usb clocks and resets Chen-Yu Tsai
2014-11-04 4:07 ` Chen-Yu Tsai
2014-11-04 16:57 ` Maxime Ripard
2014-11-04 16:57 ` Maxime Ripard
2014-11-05 10:02 ` Chen-Yu Tsai
2014-11-05 10:02 ` Chen-Yu Tsai
2014-11-05 10:09 ` Maxime Ripard
2014-11-05 10:09 ` Maxime Ripard
2014-11-06 2:09 ` Chen-Yu Tsai
2014-11-06 2:09 ` Chen-Yu Tsai
2014-11-06 8:54 ` Maxime Ripard
2014-11-06 8:54 ` Maxime Ripard
2014-11-06 9:19 ` Chen-Yu Tsai
2014-11-06 9:19 ` Chen-Yu Tsai
2014-11-14 8:39 ` Maxime Ripard [this message]
2014-11-14 8:39 ` Maxime Ripard
2014-11-14 19:58 ` [linux-sunxi] " Chen-Yu Tsai
2014-11-14 19:58 ` Chen-Yu Tsai
2014-11-04 18:12 ` Russell King - ARM Linux
2014-11-04 18:12 ` Russell King - ARM Linux
2014-11-05 9:41 ` Maxime Ripard
2014-11-05 9:41 ` Maxime Ripard
2014-11-04 4:07 ` [PATCH 2/6] ARM: dts: sun9i: Add usb clock nodes to a80 dtsi Chen-Yu Tsai
2014-11-04 4:07 ` Chen-Yu Tsai
2014-11-04 4:07 ` [PATCH 3/6] phy: Add driver to support individual USB PHYs on sun9i Chen-Yu Tsai
2014-11-04 4:07 ` Chen-Yu Tsai
2014-11-04 6:16 ` [linux-sunxi] " Priit Laes
2014-11-04 6:16 ` Priit Laes
2014-11-05 9:35 ` Chen-Yu Tsai
2014-11-05 9:35 ` Chen-Yu Tsai
2014-11-05 9:45 ` Maxime Ripard
2014-11-05 9:45 ` Maxime Ripard
[not found] ` <jwvr3xhu6p3.fsf-monnier+gmane.comp.hardware.netbook.arm.sunxi@gnu.org>
2014-11-05 13:40 ` [linux-sunxi] " Maxime Ripard
2014-11-05 13:40 ` Maxime Ripard
2014-11-04 17:03 ` Maxime Ripard
2014-11-04 17:03 ` Maxime Ripard
2014-11-05 9:31 ` Chen-Yu Tsai
2014-11-05 9:31 ` Chen-Yu Tsai
2014-11-04 4:07 ` [PATCH 4/6] ARM: dts: sun9i: Add usb phy nodes to a80 dtsi Chen-Yu Tsai
2014-11-04 4:07 ` Chen-Yu Tsai
2014-11-04 4:07 ` [PATCH 5/6] ARM: dts: sun9i: Add USB host controller " Chen-Yu Tsai
2014-11-04 4:07 ` Chen-Yu Tsai
2014-11-04 4:07 ` [PATCH 6/6] ARM: dts: sun9i: Enable USB support on A80 Optimus board Chen-Yu Tsai
2014-11-04 4:07 ` Chen-Yu Tsai
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20141114083917.GT20972@lukather \
--to=maxime.ripard@free-electrons.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.