On Sun, 30 Jun 2013, Aleksandr Rybalko wrote:

Log:
 Decrypt magic numbers - define names for fields of Generic Timer's CNTKCTL reg.

 Submitted by:  Ruslan Bukin <b...@bsdpad.com>

Modified:
 head/sys/arm/arm/generic_timer.c

Modified: head/sys/arm/arm/generic_timer.c
==============================================================================
--- head/sys/arm/arm/generic_timer.c    Sun Jun 30 19:36:17 2013        
(r252424)
+++ head/sys/arm/arm/generic_timer.c    Sun Jun 30 19:52:41 2013        
(r252425)
@@ -66,7 +66,22 @@ __FBSDID("$FreeBSD$");
#define GENERIC_TIMER_REG_CTRL          0
#define GENERIC_TIMER_REG_TVAL          1

-#define        CNTPSIRQ        29
+#define        GENERIC_TIMER_CNTKCTL_PL0PTEN   (1 << 9) /* Physical timer 
registers
+                                                   access from PL0 */
+#define        GENERIC_TIMER_CNTKCTL_PL0VTEN   (1 << 8) /* Virtual timer 
registers

With names like these, the magic numbers are better.  The prefix name
GENERIC_TIMER is especially bad.  GT would be good.

...
+#define        GENERIC_TIMER_CNTPSIRQ  29

Here the interesting part CNTPSIRQ is fairly abbreviated, but its prefix
is not.


struct arm_tmr_softc {
        struct resource         *irq_res;
@@ -167,7 +182,11 @@ disable_user_access(void)
        uint32_t cntkctl;

        __asm volatile("mrc p15, 0, %0, c14, c1, 0" : "=r" (cntkctl));
-       cntkctl &= ~((3 << 8) | (7 << 0));
+       cntkctl &= ~(GENERIC_TIMER_CNTKCTL_PL0PTEN |
+               GENERIC_TIMER_CNTKCTL_PL0VTEN |
+               GENERIC_TIMER_CNTKCTL_EVNTEN |
+               GENERIC_TIMER_CNTKCTL_PL0VCTEN |
+               GENERIC_TIMER_CNTKCTL_PL0PCTEN);
        __asm volatile("mcr p15, 0, %0, c14, c1, 0" : : "r" (cntkctl));
        isb();
}

Using these verbose names takes about 15 times as much code as
before (the statement is only 5 times longer, but the definitions
macros are about 10 times longer), and looks like even more becayse
the statement is misformatted with non-KNF indentation which then
requires extra line spitting with only 1 name per line.

@@ -270,7 +289,8 @@ arm_tmr_attach(device_t dev)

        rid = 0;
        sc->irq_res = bus_alloc_resource(dev, SYS_RES_IRQ, &rid,
-           CNTPSIRQ, CNTPSIRQ, 1, RF_SHAREABLE | RF_ACTIVE);
+           GENERIC_TIMER_CNTPSIRQ, GENERIC_TIMER_CNTPSIRQ,
+           1, RF_SHAREABLE | RF_ACTIVE);

        arm_tmr_sc = sc;

For full uglyness, expand all the prefixes and names:
- SYS -> SYSTEM
- RES -> RESOURCE
- IRQ -> INTERRUPT_REQUEST_NUMBER
- SYS_RES_IRQ -> SYSTEM_RESOURCE_INTERRUPT_REQUEST_NUMBER
- RF -> RESOURCE_FLAG
- RF_SHAREABLE_RESOURCE_FLAG_SHAREABLE - RF_ACTIVE -> RESOURCE_FLAG_ACTIVE
- CNTPSIRQ -> COUNT_PRIVATELY_SOURCED_INTERRUPT_REQUEST_NUMBER
  (just guessing what PS means):

        sc->irq_res = bus_alloc_resource(dev, SYS_RES_INTERRUPT_REQUEST_NUMBER,
            &rid,
            GENERIC_TIMER_COUNT_PRIVATELY_SOURCED_INTERRUPT_REQUEST_NUMBER,
            GENERIC_TIMER_COUNT_PRIVATELY_SOURCED_INTERRUPT_REQUEST_NUMBER,
            1, RESOURCE_FLAG_SHAREABLE | RESOURCE_FLAG_ACTIVE);

Next, use non-KNF indentation:

        sc->irq_res = bus_alloc_resource(dev, SYS_RES_INTERRUPT_REQUEST_NUMBER,
                &rid,
                GENERIC_TIMER_COUNT_PRIVATELY_SOURCED_INTERRUPT_REQUEST_NUMBER,
                GENERIC_TIMER_COUNT_PRIVATELY_SOURCED_INTERRUPT_REQUEST_NUMBER,
                1, RESOURCE_FLAG_SHAREABLE | RESOURCE_FLAG_ACTIVE);

The names aren't even 80 characters long, so they actually fit on 1 line.

Bruce
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to