Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1360406 > unrolled thread

[RESEND PATCH v4 1/5] clocksource: move some enums and marcos to header file for arm_arch_timer

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [RESEND PATCH v4 1/5] clocksource: move some enums and marcos to header file for arm_arch_timer fu.wei@linaro.org - 2016-03-18 09:10 +0100
    Re: [RESEND PATCH v4 1/5] clocksource: move some enums and marcos  to header file for arm_arch_timer Thomas Gleixner <tglx@linutronix.de> - 2016-03-18 10:30 +0100
      Re: [RESEND PATCH v4 1/5] clocksource: move some enums and marcos to  header file for arm_arch_timer Fu Wei <fu.wei@linaro.org> - 2016-03-29 11:40 +0200

#1360406 — [RESEND PATCH v4 1/5] clocksource: move some enums and marcos to header file for arm_arch_timer

From: Fu Wei <fu.wei@linaro.org>

The patch sorts out the code for arm_arch_timer:
    (1)move enum ppi_nr to the header file
    (2)move "ARCH_*_TIMER" marcos to the header file
    (3)add enum spi_nr in the header file, and use it in the driver
    (4)add ARCH_WD_TIMER and ARCH_TIMER_MEM_MAX_FRAME marcos

Signed-off-by: Fu Wei <fu.wei@linaro.org>
---
 drivers/clocksource/arm_arch_timer.c | 15 ++-------------
 include/clocksource/arm_arch_timer.h | 20 ++++++++++++++++++++
 2 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 5152b38..11744c0 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -48,8 +48,6 @@
 #define CNTV_TVAL	0x38
 #define CNTV_CTL	0x3c
 
-#define ARCH_CP15_TIMER	BIT(0)
-#define ARCH_MEM_TIMER	BIT(1)
 static unsigned arch_timers_present __initdata;
 
 static void __iomem *arch_counter_base;
@@ -62,15 +60,6 @@ struct arch_timer {
 #define to_arch_timer(e) container_of(e, struct arch_timer, evt)
 
 static u32 arch_timer_rate;
-
-enum ppi_nr {
-	PHYS_SECURE_PPI,
-	PHYS_NONSECURE_PPI,
-	VIRT_PPI,
-	HYP_PPI,
-	MAX_TIMER_PPI
-};
-
 static int arch_timer_ppi[MAX_TIMER_PPI];
 
 static struct clock_event_device __percpu *arch_timer_evt;
@@ -834,9 +823,9 @@ static void __init arch_timer_mem_init(struct device_node *np)
 	}
 
 	if (arch_timer_mem_use_virtual)
-		irq = irq_of_parse_and_map(best_frame, 1);
+		irq = irq_of_parse_and_map(best_frame, VIRT_SPI);
 	else
-		irq = irq_of_parse_and_map(best_frame, 0);
+		irq = irq_of_parse_and_map(best_frame, PHYS_SPI);
 
 	if (!irq) {
 		pr_err("arch_timer: Frame missing %s irq",
diff --git a/include/clocksource/arm_arch_timer.h b/include/clocksource/arm_arch_timer.h
index 25d0914..3e060fc 100644
--- a/include/clocksource/arm_arch_timer.h
+++ b/include/clocksource/arm_arch_timer.h
@@ -19,6 +19,10 @@
 #include <linux/timecounter.h>
 #include <linux/types.h>
 
+#define ARCH_CP15_TIMER			BIT(0)
+#define ARCH_MEM_TIMER			BIT(1)
+#define ARCH_WD_TIMER			BIT(2)
+
 #define ARCH_TIMER_CTRL_ENABLE		(1 << 0)
 #define ARCH_TIMER_CTRL_IT_MASK		(1 << 1)
 #define ARCH_TIMER_CTRL_IT_STAT		(1 << 2)
@@ -34,11 +38,27 @@ enum arch_timer_reg {
 	ARCH_TIMER_REG_TVAL,
 };
 
+enum ppi_nr {
+	PHYS_SECURE_PPI,
+	PHYS_NONSECURE_PPI,
+	VIRT_PPI,
+	HYP_PPI,
+	MAX_TIMER_PPI
+};
+
+enum spi_nr {
+	PHYS_SPI,
+	VIRT_SPI,
+	MAX_TIMER_SPI
+};
+
 #define ARCH_TIMER_PHYS_ACCESS		0
 #define ARCH_TIMER_VIRT_ACCESS		1
 #define ARCH_TIMER_MEM_PHYS_ACCESS	2
 #define ARCH_TIMER_MEM_VIRT_ACCESS	3
 
+#define ARCH_TIMER_MEM_MAX_FRAME	8
+
 #define ARCH_TIMER_USR_PCT_ACCESS_EN	(1 << 0) /* physical counter */
 #define ARCH_TIMER_USR_VCT_ACCESS_EN	(1 << 1) /* virtual counter */
 #define ARCH_TIMER_VIRT_EVT_EN		(1 << 2)
-- 
2.5.0

[toc] | [next] | [standalone]

#1360459 — Re: [RESEND PATCH v4 1/5] clocksource: move some enums and marcos to header file for arm_arch_timer

On Fri, 18 Mar 2016, fu.wei@linaro.org wrote:

Subject: clocksource: move some enums and marcos to header file for arm_arch_timer

That sucks.

1) The prefix is crap. It suggests this is about the generic clocksource code

   clocksource/drivers/arm_arch_timer:

   Is the proper prefix

2) Sentences start with upper case letters

3) What exactly are 'marcos' ?

So a useful subject line would be;

clocksource/drivers/arm_arch_timer: Move enums and defines to header file
   
> From: Fu Wei <fu.wei@linaro.org>
> 
> The patch sorts out the code for arm_arch_timer:

What does it sort out? Nothing, AFAICT.

You miss to explain WHY you are doing this. The list below is completely
useless because one can see that from the patch.

So this wants to be something like this:

To support the arm_arch_timer via ACPI we need to share defines and enums
between the driver and the ACPI parser code.

Split out the relevant defines and enums into arm_arch_timer.h. No functional
change.

>     (1)move enum ppi_nr to the header file
>     (2)move "ARCH_*_TIMER" marcos to the header file
>     (3)add enum spi_nr in the header file, and use it in the driver
>     (4)add ARCH_WD_TIMER and ARCH_TIMER_MEM_MAX_FRAME marcos

So this does 3 things at once.

   1) Move existing defines and enums

   2) Add a new enum and convert the driver to use it

   3) Add new defines without using them

So this needs to be seperate

   #1) This patch

   #2) Seperate patch which adds the new enum and converts the places in the
       driver

   #3) This needs to go into the patch which actually uses the defines.

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]

#1366073 — Re: [RESEND PATCH v4 1/5] clocksource: move some enums and marcos to header file for arm_arch_timer

Hi Thomas,

On 18 March 2016 at 17:27, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Fri, 18 Mar 2016, fu.wei@linaro.org wrote:
>
> Subject: clocksource: move some enums and marcos to header file for arm_arch_timer
>
> That sucks.
>
> 1) The prefix is crap. It suggests this is about the generic clocksource code
>
>    clocksource/drivers/arm_arch_timer:
>
>    Is the proper prefix
>
> 2) Sentences start with upper case letters
>
> 3) What exactly are 'marcos' ?
>
> So a useful subject line would be;
>
> clocksource/drivers/arm_arch_timer: Move enums and defines to header file
>
>> From: Fu Wei <fu.wei@linaro.org>
>>
>> The patch sorts out the code for arm_arch_timer:
>
> What does it sort out? Nothing, AFAICT.
>
> You miss to explain WHY you are doing this. The list below is completely
> useless because one can see that from the patch.
>
> So this wants to be something like this:
>
> To support the arm_arch_timer via ACPI we need to share defines and enums
> between the driver and the ACPI parser code.
>
> Split out the relevant defines and enums into arm_arch_timer.h. No functional
> change.
>
>>     (1)move enum ppi_nr to the header file
>>     (2)move "ARCH_*_TIMER" marcos to the header file
>>     (3)add enum spi_nr in the header file, and use it in the driver
>>     (4)add ARCH_WD_TIMER and ARCH_TIMER_MEM_MAX_FRAME marcos
>
> So this does 3 things at once.
>
>    1) Move existing defines and enums
>
>    2) Add a new enum and convert the driver to use it
>
>    3) Add new defines without using them
>
> So this needs to be seperate
>
>    #1) This patch
>
>    #2) Seperate patch which adds the new enum and converts the places in the
>        driver
>
>    #3) This needs to go into the patch which actually uses the defines.

Great thanks , will fix these following your suggestion :-)

>
> Thanks,
>
>         tglx



-- 
Best regards,

Fu Wei
Software Engineer
Red Hat Software (Beijing) Co.,Ltd.Shanghai Branch
Ph: +86 21 61221326(direct)
Ph: +86 186 2020 4684 (mobile)
Room 1512, Regus One Corporate Avenue,Level 15,
One Corporate Avenue,222 Hubin Road,Huangpu District,
Shanghai,China 200021

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web