All of lore.kernel.org
 help / color / mirror / Atom feed
From: mturquette@linaro.org (Mike Turquette)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3 2/6] clk: Add clock driver for AXM55xx SoC
Date: Wed, 14 May 2014 15:36:57 -0700	[thread overview]
Message-ID: <20140514223657.19795.79741@quantum> (raw)
In-Reply-To: <CAE0=X_3WA5ZzEE1NespeScmnMkr9oxNMc1=+X86i5NpSffiUnA@mail.gmail.com>

Quoting Anders Berg (2014-05-14 15:22:16)
> On Wed, May 14, 2014 at 10:08 PM, Mike Turquette <mturquette@linaro.org> wrote:
> > Quoting Anders Berg (2014-05-14 11:37:57)
> >> +Example:
> >> +
> >> +       clk_ref0: clk_ref0 {
> >> +               compatible = "fixed-clock";
> >> +               #clock-cells = <0>;
> >> +               clock-frequency = <125000000>;
> >> +       };
> >
> > Hi Anders,
> >
> > The driver looks good. As for the DT binding, I am starting to request
> > that bindings for new hardware move away from the one-clock-per-node
> > method. I am not forcing anyone with stable bindings to migrate that
> > way, but it tends to make maintenance easier in the long run (e.g.
> > setting per-clock flags, etc).
> >
> > Your clk_ref0 example looks good, assuming that it is an off-chip clock
> > that feeds into the rest of the clock generator.
> >
> >> +
> >> +       clk_cpu_pll: clk_cpu_pll at 2010022000 {
> >> +               compatible = "lsi,axxia-pll-clock";
> >> +               #clock-cells = <0>;
> >> +               clocks = <&clk_ref0>;
> >> +               clock-output-names = "clk_cpu_pll";
> >> +               reg = <0x20 0x10022000 0 0x2c>;
> >> +       };
> >
> > I assume the rest of your clocks are part of a clock generator IP block
> > inside of your chip. Have you looked at the QCOM binding? It is my
> > favorite binding these days. Here are some highlights:
> >
> [...]
> >
> > Using this type of binding you only need to declare your clock generator
> > IP node in dts, and then define a mapping in the DT include chroot. Then
> > you can define your per-clock data inside of your clock driver instead
> > of putting all of the details inside of DT.
> >
> > If you have a strong reason to do it the way that you originally posted
> > then let me know.
> >
> 
> No strong reason... I just happened to pick the keystone-clocks.dtsi
> as an example when I wrote this. But I can rework this according to
> your suggestions. I'll post it as a separate patch (thus dropping the
> clk patch from this series). Ok?

Sounds great. Thanks for reworking it.

Regards,
Mike

> 
> Thanks,
> Anders

WARNING: multiple messages have this Message-ID (diff)
From: Mike Turquette <mturquette@linaro.org>
To: Anders Berg <anders.berg@avagotech.com>
Cc: Mark Rutland <mark.rutland@arm.com>,
	devicetree <devicetree@vger.kernel.org>,
	Russell King <linux@arm.linux.org.uk>,
	Arnd Bergmann <arnd@arndb.de>, Anders Berg <anders.berg@lsi.com>,
	Linus Walleij <linus.walleij@linaro.org>,
	linux-kernel@vger.kernel.org, Olof Johansson <olof@lixom.net>,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v3 2/6] clk: Add clock driver for AXM55xx SoC
Date: Wed, 14 May 2014 15:36:57 -0700	[thread overview]
Message-ID: <20140514223657.19795.79741@quantum> (raw)
In-Reply-To: <CAE0=X_3WA5ZzEE1NespeScmnMkr9oxNMc1=+X86i5NpSffiUnA@mail.gmail.com>

Quoting Anders Berg (2014-05-14 15:22:16)
> On Wed, May 14, 2014 at 10:08 PM, Mike Turquette <mturquette@linaro.org> wrote:
> > Quoting Anders Berg (2014-05-14 11:37:57)
> >> +Example:
> >> +
> >> +       clk_ref0: clk_ref0 {
> >> +               compatible = "fixed-clock";
> >> +               #clock-cells = <0>;
> >> +               clock-frequency = <125000000>;
> >> +       };
> >
> > Hi Anders,
> >
> > The driver looks good. As for the DT binding, I am starting to request
> > that bindings for new hardware move away from the one-clock-per-node
> > method. I am not forcing anyone with stable bindings to migrate that
> > way, but it tends to make maintenance easier in the long run (e.g.
> > setting per-clock flags, etc).
> >
> > Your clk_ref0 example looks good, assuming that it is an off-chip clock
> > that feeds into the rest of the clock generator.
> >
> >> +
> >> +       clk_cpu_pll: clk_cpu_pll@2010022000 {
> >> +               compatible = "lsi,axxia-pll-clock";
> >> +               #clock-cells = <0>;
> >> +               clocks = <&clk_ref0>;
> >> +               clock-output-names = "clk_cpu_pll";
> >> +               reg = <0x20 0x10022000 0 0x2c>;
> >> +       };
> >
> > I assume the rest of your clocks are part of a clock generator IP block
> > inside of your chip. Have you looked at the QCOM binding? It is my
> > favorite binding these days. Here are some highlights:
> >
> [...]
> >
> > Using this type of binding you only need to declare your clock generator
> > IP node in dts, and then define a mapping in the DT include chroot. Then
> > you can define your per-clock data inside of your clock driver instead
> > of putting all of the details inside of DT.
> >
> > If you have a strong reason to do it the way that you originally posted
> > then let me know.
> >
> 
> No strong reason... I just happened to pick the keystone-clocks.dtsi
> as an example when I wrote this. But I can rework this according to
> your suggestions. I'll post it as a separate patch (thus dropping the
> clk patch from this series). Ok?

Sounds great. Thanks for reworking it.

Regards,
Mike

> 
> Thanks,
> Anders

  reply	other threads:[~2014-05-14 22:36 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-14 18:37 [PATCH v3 0/6] Add platform support for LSI AXM55xx Anders Berg
2014-05-14 18:37 ` Anders Berg
2014-05-14 18:37 ` [PATCH v3 1/6] ARM: Add platform support for LSI AXM55xx SoC Anders Berg
2014-05-14 18:37   ` Anders Berg
2014-05-14 18:37 ` [PATCH v3 2/6] clk: Add clock driver for " Anders Berg
2014-05-14 18:37   ` Anders Berg
2014-05-14 20:08   ` Mike Turquette
2014-05-14 20:08     ` Mike Turquette
2014-05-14 22:22     ` Anders Berg
2014-05-14 22:22       ` Anders Berg
2014-05-14 22:22       ` Anders Berg
2014-05-14 22:36       ` Mike Turquette [this message]
2014-05-14 22:36         ` Mike Turquette
2014-05-14 18:37 ` [PATCH v3 3/6] ARM: dts: Device tree for AXM55xx Anders Berg
2014-05-14 18:37   ` Anders Berg
2014-05-14 18:37   ` Anders Berg
2014-05-14 18:37 ` [PATCH v3 4/6] ARM: axxia: Adding defconfig " Anders Berg
2014-05-14 18:37   ` Anders Berg
2014-05-14 18:38 ` [PATCH v3 5/6] power: reset: Add Axxia system reset driver Anders Berg
2014-05-14 18:38   ` Anders Berg
2014-05-14 18:38 ` [PATCH v3 6/6] ARM: dts: axxia: Add reset controller Anders Berg
2014-05-14 18:38   ` Anders Berg
2014-05-14 18:56 ` [PATCH v3 0/6] Add platform support for LSI AXM55xx Arnd Bergmann
2014-05-14 18:56   ` Arnd Bergmann
2014-05-14 19:21   ` Anders Berg
2014-05-14 19:21     ` Anders Berg
2014-05-15 12:37   ` Anders Berg
2014-05-15 12:37     ` Anders Berg
2014-05-15 12:37     ` Anders Berg
2014-05-15 12:39     ` Arnd Bergmann
2014-05-15 12:39       ` Arnd Bergmann
2014-05-15 12:39       ` Arnd Bergmann
2014-05-15 12:59       ` Anders Berg
2014-05-15 12:59         ` Anders Berg
2014-05-15 12:59         ` Anders Berg

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=20140514223657.19795.79741@quantum \
    --to=mturquette@linaro.org \
    --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.