davinci: dm646x: remove the macros from the header to move to c file

PATCH New Manjunath Hadli 4 participants 7 messages May 20, 2011 – May 24, 2011

Subsystems (3)

Arm (32-bit) Arm sub-architectures TI DaVinci machine support

lore

Share & tools

lore

Manjunath Hadli

author

2011-05-20 14:12 UTC #0 lore raw

PATCH 3/4 davinci: dm646x: remove the macros from the header to move to c file

move the register base addresses and offsets used only by dm646x platform file from platform header dm646x.h to dm646x.c as they are used only in the c file.

Signed-off-by Manjunath Hadli <[email protected]>

---
 arch/arm/mach-davinci/dm646x.c              |   34 +++++++++++++++-----------
 arch/arm/mach-davinci/include/mach/dm646x.h |    7 -----
 2 files changed, 20 insertions(+), 21 deletions(-)

arch/arm/mach-davinci/dm646x.c +20 ~~-14~~

index 1e0f809..9efa380 100644
@@ -30,20 +30,6 @@
 #include "clock.h"
 #include "mux.h"
 
-#define DAVINCI_VPIF_BASE       (0x01C12000)
-#define VDD3P3V_PWDN_OFFSET	(0x48)
-#define VSCLKDIS_OFFSET		(0x6C)
-
-#define VDD3P3V_VID_MASK	(BIT_MASK(3) | BIT_MASK(2) | BIT_MASK(1) |\
-					BIT_MASK(0))
-#define VSCLKDIS_MASK		(BIT_MASK(11) | BIT_MASK(10) | BIT_MASK(9) |\
-					BIT_MASK(8))
-
-/*
- * Device specific clocks
- */
-#define DM646X_AUX_FREQ		24000000
-
 static struct pll_data pll1_data = {
 	.num       = 1,
 	.phys_base = DAVINCI_PLL1_BASE,

@@ -58,6 +44,8 @@ static struct clk ref_clk = {
 	.name = "ref_clk",
 };
 
+#define DM646X_AUX_FREQ		24000000
+
 static struct clk aux_clkin = {
 	.name = "aux_clkin",
 	.rate = DM646X_AUX_FREQ,

@@ -354,6 +342,11 @@ static struct clk_lookup dm646x_clks[] = {
 	CLK(NULL, NULL, NULL),
 };
 
+#define DM646X_EMAC_CNTRL_OFFSET	0x0000
+#define DM646X_EMAC_CNTRL_MOD_OFFSET	0x1000
+#define DM646X_EMAC_CNTRL_RAM_OFFSET	0x2000
+#define DM646X_EMAC_CNTRL_RAM_SIZE	0x2000
+
 static struct emac_platform_data dm646x_emac_pdata = {
 	.ctrl_reg_offset	= DM646X_EMAC_CNTRL_OFFSET,
 	.ctrl_mod_reg_offset	= DM646X_EMAC_CNTRL_MOD_OFFSET,

@@ -362,6 +355,8 @@ static struct emac_platform_data dm646x_emac_pdata = {
 	.version		= EMAC_VERSION_2,
 };
 
+#define DM646X_EMAC_BASE	0x01C80000
+
 static struct resource dm646x_emac_resources[] = {
 	{
 		.start	= DM646X_EMAC_BASE,

@@ -400,6 +395,8 @@ static struct platform_device dm646x_emac_device = {
 	.resource	= dm646x_emac_resources,
 };
 
+#define DM646X_EMAC_MDIO_BASE	(DM646X_EMAC_BASE + 0x4000)
+
 static struct resource dm646x_mdio_resources[] = {
 	{
 		.start	= DM646X_EMAC_MDIO_BASE,

@@ -671,6 +668,8 @@ static struct platform_device dm646x_dit_device = {
 
 static u64 vpif_dma_mask = DMA_BIT_MASK(32);
 
+#define DAVINCI_VPIF_BASE	0x01C12000
+
 static struct resource vpif_resource[] = {
 	{
 		.start	= DAVINCI_VPIF_BASE,

@@ -866,6 +865,13 @@ void __init dm646x_init_mcasp1(struct snd_platform_data *pdata)
 	platform_device_register(&dm646x_dit_device);
 }
 
+#define VDD3P3V_PWDN_OFFSET	0x48
+#define VSCLKDIS_OFFSET		0x6C
+#define VDD3P3V_VID_MASK	(BIT_MASK(3) | BIT_MASK(2) | BIT_MASK(1) |\
+					BIT_MASK(0))
+#define VSCLKDIS_MASK		(BIT_MASK(11) | BIT_MASK(10) | BIT_MASK(9) |\
+					BIT_MASK(8))
+
 void dm646x_setup_vpif(struct vpif_display_config *display_config,
 		       struct vpif_capture_config *capture_config)
 {

arch/arm/mach-davinci/include/mach/dm646x.h +0 -7

index 245a1c0..e36ab28 100644
@@ -19,13 +19,6 @@
 #include <linux/davinci_emac.h>
 #include <media/davinci/vpif.h>
 
-#define DM646X_EMAC_BASE		(0x01C80000)
-#define DM646X_EMAC_MDIO_BASE		(DM646X_EMAC_BASE + 0x4000)
-#define DM646X_EMAC_CNTRL_OFFSET	(0x0000)
-#define DM646X_EMAC_CNTRL_MOD_OFFSET	(0x1000)
-#define DM646X_EMAC_CNTRL_RAM_OFFSET	(0x2000)
-#define DM646X_EMAC_CNTRL_RAM_SIZE	(0x2000)
-
 #define DM646X_ASYNC_EMIF_CONTROL_BASE	0x20008000
 #define DM646X_ASYNC_EMIF_CS2_SPACE_BASE 0x42000000

Signature · 1 line

1.6.2.4

Jean-Christophe PLAGNIOL-VILLARD

2011-05-20 14:24 UTC #1 lore raw

PATCH 3/4 davinci: dm646x: remove the macros from the header to move to c file

Quoted 5 lines · Manjunath Hadli

move the register base addresses and offsets used only by dm646x platform file from platform header dm646x.h to dm646x.c as they are used only in the c file.
Signed-off-by Manjunath Hadli <[email protected]>

I do not think it's a good idea to put the register mapping in the c file evenif they are used only today in dm646x

Best Regards,
J.

Nori, Sekhar

2011-05-23 13:34 UTC #2 lore raw

PATCH 3/4 davinci: dm646x: remove the macros from the header to move to c file

Hi Jean-Christophe,

Quoted 9 lines · Jean-Christophe PLAGNIOL-VILLARD

Quoted 5 lines · Manjunath Hadli
move the register base addresses and offsets used only by dm646x platform file from platform header dm646x.h to dm646x.c as they are used only in the c file.
Signed-off-by Manjunath Hadli <[email protected]>
I do not think it's a good idea to put the register mapping in the c file evenif they are used only today in dm646x

Unless a definition is used across C files, it would be better to keep it local to the C file. Why treat base addresses and register offsets differently?

Thanks,
Sekhar

Jean-Christophe PLAGNIOL-VILLARD

2011-05-24 05:32 UTC #3 lore raw

PATCH 3/4 davinci: dm646x: remove the macros from the header to move to c file

Quoted 9 lines · Sekhar

Hi Jean-Christophe,
Quoted 5 lines · Jean-Christophe PLAGNIOL-VILLARD
Quoted 4 lines · Manjunath Hadli
move the register base addresses and offsets used only by dm646x platform file from platform header dm646x.h to dm646x.c as they are used only in the c file.

Quoted 8 lines

Quoted 4 lines
Quoted 1 line
Signed-off-by Manjunath Hadli <[email protected]>
I do not think it's a good idea to put the register mapping in the c file evenif they are used only today in dm646x
Unless a definition is used across C files, it would be better to keep it local to the C file. Why treat base addresses and register offsets differently?

no difference the macro are nearly every time put in header not in C code

ditto of ifdef

Best Regards,
J.

Nori, Sekhar

2011-05-24 10:54 UTC #4 lore raw

PATCH 3/4 davinci: dm646x: remove the macros from the header to move to c file

Quoted 11 lines · Jean-Christophe PLAGNIOL-VILLARD

Quoted 10 lines · Sekhar
Hi Jean-Christophe,
Quoted 6 lines · Jean-Christophe PLAGNIOL-VILLARD
Quoted 5 lines · Manjunath Hadli
move the register base addresses and offsets used only by dm646x platform file from platform header dm646x.h to dm646x.c as they are used only in the c file.
Signed-off-by Manjunath Hadli <[email protected]>

Quoted 8 lines

Quoted 7 lines
Quoted 3 lines
I do not think it's a good idea to put the register mapping in the c file evenif they are used only today in dm646x
Unless a definition is used across C files, it would be better to keep it local to the C file. Why treat base addresses and register offsets differently?
no difference the macro are nearly every time put in header not in C code

I don't think there is a preference of this sort. There are numerous instances in kernel where macros are defined in C files.

Thanks,
Sekhar

Nori, Sekhar

2011-05-23 14:17 UTC #6 lore raw

PATCH 3/4 davinci: dm646x: remove the macros from the header to move to c file

Quoted 6 lines · Sergei Shtylyov

Quoted 6 lines

arch/arm/mach-davinci/dm646x.c +0 -0

@@ -866,6 +865,13 @@ void __init dm646x_init_mcasp1(struct snd_platform_data *pdata)
 	platform_device_register(&dm646x_dit_device);
 }

Quoted 9 lines

arch/arm/mach-davinci/dm646x.c void __init dm646x_init_mcasp1(struct snd_platform_data *pdata) · line 873
Quoted 6 lines
arch/arm/mach-davinci/dm646x.c +6 -0
+#define VDD3P3V_PWDN_OFFSET	0x48
+#define VSCLKDIS_OFFSET		0x6C
+#define VDD3P3V_VID_MASK	(BIT_MASK(3) | BIT_MASK(2) | BIT_MASK(1) |\
+					BIT_MASK(0))
+#define VSCLKDIS_MASK		(BIT_MASK(11) | BIT_MASK(10) | BIT_MASK(9) |\
+					BIT_MASK(8))
I'd prefer that these remain on their places too. Let's wait for Sekhar's
word though...

I think there is some personal coding style involved here and some folks prefer to keep the definitions near the actual usage so that you don't have to scroll two or three pages up just to see what the macro is defined to.

However, consistency is important too and almost all across mach-davinci I see the defines (including base address definitions) grouped at the top of the file. So, I will go with what Sergei is saying here.

Thanks,
Sekhar