Re: [PATCH v3 1/3] watchdog: sama5d4_wdt: cleanup the bit definitions

[Eugen.Hristev]

On 12.11.2019 15:47, Guenter Roeck wrote:

> 
> On 11/11/19 4:13 AM, Eugen.Hristev@xxxxxxxxxxxxx wrote:
>> From: Eugen Hristev <eugen.hristev@xxxxxxxxxxxxx>
>>
>> Cleanup the macro definitions to use BIT and align with two spaces.
>>
>> Signed-off-by: Eugen Hristev <eugen.hristev@xxxxxxxxxxxxx>
>> ---
>> Changes in v3:
>> - new patch as requested from review on ML
>>
>> Â drivers/watchdog/at91sam9_wdt.h | 30 +++++++++++++++---------------
>> Â 1 file changed, 15 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/watchdog/at91sam9_wdt.h 
>> b/drivers/watchdog/at91sam9_wdt.h
>> index 390941c..2ca5fc5 100644
>> --- a/drivers/watchdog/at91sam9_wdt.h
>> +++ b/drivers/watchdog/at91sam9_wdt.h
>> @@ -14,23 +14,23 @@
>> Â #define AT91_WDT_H
>> Â #define AT91_WDT_CRÂÂÂÂÂÂÂ 0x00ÂÂÂÂÂÂÂÂÂÂÂ /* Watchdog Control 
>> Register */
>> -#defineÂÂÂÂÂÂÂ AT91_WDT_WDRSTTÂÂÂÂÂÂÂ (1ÂÂÂ << 0)ÂÂÂÂÂÂÂ /* Restart */
>> -#defineÂÂÂÂÂÂÂ AT91_WDT_KEYÂÂÂÂÂÂÂ (0xa5 << 24)ÂÂÂÂÂÂÂ /* KEY 
>> Password */
>> +#define AT91_WDT_WDRSTT BIT(0) /* Restart */
> 
> Using BIT() requires including linux/bits.h.


Hi Guenter,

The C files include/will include the bits.h as the drivers use this 
definition header, or, you have something else in mind ?

Thanks,
Eugen

> 
>> +#define AT91_WDT_KEY (0xa5 << 24) /* KEY Password */
>> Â #define AT91_WDT_MRÂÂÂÂÂÂÂ 0x04ÂÂÂÂÂÂÂÂÂÂÂ /* Watchdog Mode Register */
>> -#defineÂÂÂÂÂÂÂ AT91_WDT_WDVÂÂÂÂÂÂÂ (0xfff << 0)ÂÂÂÂÂÂÂ /* Counter 
>> Value */
>> -#defineÂÂÂÂÂÂÂÂÂÂÂ AT91_WDT_SET_WDV(x)ÂÂÂ ((x) & AT91_WDT_WDV)
>> -#defineÂÂÂÂÂÂÂ AT91_WDT_WDFIENÂÂÂÂÂÂÂ (1ÂÂÂÂ << 12)ÂÂÂÂÂÂÂ /* Fault 
>> Interrupt Enable */
>> -#defineÂÂÂÂÂÂÂ AT91_WDT_WDRSTENÂÂÂ (1ÂÂÂÂ << 13)ÂÂÂÂÂÂÂ /* Reset 
>> Processor */
>> -#defineÂÂÂÂÂÂÂ AT91_WDT_WDRPROCÂÂÂ (1ÂÂÂÂ << 14)ÂÂÂÂÂÂÂ /* Timer 
>> Restart */
>> -#defineÂÂÂÂÂÂÂ AT91_WDT_WDDISÂÂÂÂÂÂÂ (1ÂÂÂÂ << 15)ÂÂÂÂÂÂÂ /* Watchdog 
>> Disable */
>> -#defineÂÂÂÂÂÂÂ AT91_WDT_WDDÂÂÂÂÂÂÂ (0xfff << 16)ÂÂÂÂÂÂÂ /* Delta 
>> Value */
>> -#defineÂÂÂÂÂÂÂÂÂÂÂ AT91_WDT_SET_WDD(x)ÂÂÂ (((x) << 16) & AT91_WDT_WDD)
>> -#defineÂÂÂÂÂÂÂ AT91_WDT_WDDBGHLTÂÂÂ (1ÂÂÂÂ << 28)ÂÂÂÂÂÂÂ /* Debug 
>> Halt */
>> -#defineÂÂÂÂÂÂÂ AT91_WDT_WDIDLEHLTÂÂÂ (1ÂÂÂÂ << 29)ÂÂÂÂÂÂÂ /* Idle 
>> Halt */
>> +#define AT91_WDT_WDV (0xfff << 0) /* Counter Value */
>> +#define AT91_WDT_SET_WDV(x) ((x) & AT91_WDT_WDV)
>> +#define AT91_WDT_WDFIEN BIT(12) /* Fault Interrupt Enable */
>> +#define AT91_WDT_WDRSTEN BIT(13) /* Reset Processor */
>> +#define AT91_WDT_WDRPROC BIT(14) /* Timer Restart */
>> +#define AT91_WDT_WDDIS BIT(15) /* Watchdog Disable */
>> +#define AT91_WDT_WDD (0xfff << 16) /* Delta Value */
>> +#define AT91_WDT_SET_WDD(x) (((x) << 16) & AT91_WDT_WDD)
>> +#define AT91_WDT_WDDBGHLT BIT(28) /* Debug Halt */
>> +#define AT91_WDT_WDIDLEHLT BIT(29) /* Idle Halt */
>> -#define AT91_WDT_SRÂÂÂÂÂÂÂ 0x08ÂÂÂÂÂÂÂÂÂÂÂ /* Watchdog Status 
>> Register */
>> -#defineÂÂÂÂÂÂÂ AT91_WDT_WDUNFÂÂÂÂÂÂÂ (1 << 0)ÂÂÂÂÂÂÂ /* Watchdog 
>> Underflow */
>> -#defineÂÂÂÂÂÂÂ AT91_WDT_WDERRÂÂÂÂÂÂÂ (1 << 1)ÂÂÂÂÂÂÂ /* Watchdog 
>> Error */
>> +#define AT91_WDT_SRÂÂÂÂÂÂÂ 0x08ÂÂÂÂÂÂÂ /* Watchdog Status Register */
>> +#define AT91_WDT_WDUNF BIT(0) /* Watchdog Underflow */
>> +#define AT91_WDT_WDERR BIT(1) /* Watchdog Error */
>> Â #endif
>>
>