diff mbox

[PATCHv2,2/5] clk: samsung: exynos5410: Organize register offset constants

Message ID 1406805732-17372-3-git-send-email-hsnaves@gmail.com (mailing list archive)
State Superseded
Headers show

Commit Message

Humberto Naves July 31, 2014, 11:22 a.m. UTC
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.

Signed-off-by: Humberto Silva Naves <hsnaves@gmail.com>
---
 drivers/clk/samsung/clk-exynos5410.c |   42 +++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 19 deletions(-)

Comments

Tomasz Figa July 31, 2014, 12:49 p.m. UTC | #1
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
--
To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/clk/samsung/clk-exynos5410.c b/drivers/clk/samsung/clk-exynos5410.c
index bf57c80..92c56b7 100644
--- a/drivers/clk/samsung/clk-exynos5410.c
+++ b/drivers/clk/samsung/clk-exynos5410.c
@@ -19,39 +19,43 @@ 
 
 #include "clk.h"
 
-#define APLL_LOCK               0x0
-#define APLL_CON0               0x100
-#define CPLL_LOCK               0x10020
-#define CPLL_CON0               0x10120
-#define MPLL_LOCK               0x4000
-#define MPLL_CON0               0x4100
-#define BPLL_LOCK               0x20010
-#define BPLL_CON0               0x20110
-#define KPLL_LOCK               0x28000
-#define KPLL_CON0               0x28100
+#define APLL_LOCK		0x0
+#define APLL_CON0		0x100
+#define MPLL_LOCK		0x4000
+#define MPLL_CON0		0x4100
+#define CPLL_LOCK		0x10020
+#define CPLL_CON0		0x10120
+#define BPLL_LOCK		0x20010
+#define BPLL_CON0		0x20110
+#define KPLL_LOCK		0x28000
+#define KPLL_CON0		0x28100
 
 #define SRC_CPU			0x200
-#define DIV_CPU0		0x500
 #define SRC_CPERI1		0x4204
-#define DIV_TOP0		0x10510
-#define DIV_TOP1		0x10514
-#define DIV_FSYS1		0x1054c
-#define DIV_FSYS2		0x10550
-#define DIV_PERIC0		0x10558
 #define SRC_TOP0		0x10210
 #define SRC_TOP1		0x10214
 #define SRC_TOP2		0x10218
 #define SRC_FSYS		0x10244
 #define SRC_PERIC0		0x10250
+#define SRC_CDREX		0x20200
+#define SRC_KFC			0x28200
+
 #define SRC_MASK_FSYS		0x10340
 #define SRC_MASK_PERIC0		0x10350
+
+#define DIV_CPU0		0x500
+#define DIV_TOP0		0x10510
+#define DIV_TOP1		0x10514
+#define DIV_FSYS1		0x1054c
+#define DIV_FSYS2		0x10550
+#define DIV_PERIC0		0x10558
+#define DIV_KFC0		0x28500
+
 #define GATE_BUS_FSYS0		0x10740
+
 #define GATE_IP_FSYS		0x10944
 #define GATE_IP_PERIC		0x10950
 #define GATE_IP_PERIS		0x10960
-#define SRC_CDREX		0x20200
-#define SRC_KFC			0x28200
-#define DIV_KFC0		0x28500
 
 /* list of PLLs */
 enum exynos5410_plls {