linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: tomasz.figa@gmail.com (Tomasz Figa)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCHv2 2/5] clk: samsung: exynos5410: Organize register offset constants
Date: Thu, 31 Jul 2014 14:49:09 +0200	[thread overview]
Message-ID: <53DA3B45.4080501@gmail.com> (raw)
In-Reply-To: <1406805732-17372-3-git-send-email-hsnaves@gmail.com>

Hi Humberto,

Please see my comments inline.

On 31.07.2014 13:22, Humberto Silva Naves wrote:
> The different register groups (SRC, DIV, PLL, GATE, etc) are
> now separated by a blank line, and within the same group, the
> definitions are ordered by address. This is done to reduce the
> chances of potential conflicts when adding new entries, and
> to improve the readability of code. While at it, replaced some
> spaces with tabs to keep consistency.

I'm not sure whether this change really improves anything.

It might seem plausible to have the registers grouped by their purpose,
however remaining drivers have them directly listed in order of their
addresses to match the order they are mentioned in documentation. For
consistency, I'd prefer only one convention to be used across all
Samsung clock drivers, so they would have to be changed as well. But
IMHO this is a material for a separate clean-up series, while this one
should be limited to functional improvements.

In fact, this driver is kind of exception as it has PLL register
definitions separated, which I probably missed in review.

Best regards,
Tomasz

  reply	other threads:[~2014-07-31 12:49 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-31 11:22 [PATCHv2 0/5] clk: samsung: exynos5410: Implementation of the PLL clocks Humberto Silva Naves
2014-07-31 11:22 ` [PATCHv2 1/5] clk: samsung: exynos5410: Add NULL pointer checks in clock init Humberto Silva Naves
2014-07-31 12:34   ` Tomasz Figa
2014-07-31 13:13     ` Humberto Naves
2014-07-31 13:20       ` Tomasz Figa
2014-07-31 11:22 ` [PATCHv2 2/5] clk: samsung: exynos5410: Organize register offset constants Humberto Silva Naves
2014-07-31 12:49   ` Tomasz Figa [this message]
2014-07-31 11:22 ` [PATCHv2 3/5] clk: samsung: exynos5410: Add suspend/resume handling Humberto Silva Naves
2014-07-31 13:09   ` Tomasz Figa
2014-07-31 11:22 ` [PATCHv2 4/5] clk: samsung: exynos5410: Add fixed rate clocks Humberto Silva Naves
2014-07-31 11:45   ` Sylwester Nawrocki
2014-07-31 21:01     ` Humberto Naves
2014-07-31 21:08       ` Tomasz Figa
2014-07-31 12:53   ` Tomasz Figa
2014-07-31 13:23     ` Humberto Naves
2014-07-31 13:37       ` Tomasz Figa
2014-07-31 11:22 ` [PATCHv2 5/5] clk: samsung: exynos5410: Added clocks DPLL, EPLL, IPLL, and VPLL Humberto Silva Naves
2014-07-31 13:07   ` Tomasz Figa
2014-07-31 13:37     ` Humberto Naves
2014-07-31 15:19       ` Tomasz Figa
2014-07-31 21:19         ` Humberto Naves
2014-07-31 22:17           ` Tomasz Figa
2014-07-31 22:51             ` Mike Turquette

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=53DA3B45.4080501@gmail.com \
    --to=tomasz.figa@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).