Re: [PATCH 5/5] watchdog: imx7ulp: Use definitions instead of magic values

[Guenter Roeck <linux@roeck-us.net>]

On Tue, Oct 29, 2019 at 02:40:37PM -0300, Fabio Estevam wrote:
> Use definitions instead of magic values in order to improve readability.
> 
> Since the CLK field of the WDOG CS register is composed of two bits
> to select the watchdog clock source, use a shift representation
> instead of BIT().
> 
> Signed-off-by: Fabio Estevam <festevam@gmail.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  drivers/watchdog/imx7ulp_wdt.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c
> index bcef3b6a9782..e3c1382dac52 100644
> --- a/drivers/watchdog/imx7ulp_wdt.c
> +++ b/drivers/watchdog/imx7ulp_wdt.c
> @@ -17,6 +17,9 @@
>  #define WDOG_CS_CMD32EN		BIT(13)
>  #define WDOG_CS_ULK		BIT(11)
>  #define WDOG_CS_RCS		BIT(10)
> +#define LPO_CLK			0x1
> +#define LPO_CLK_SHIFT		8
> +#define WDOG_CS_CLK		(LPO_CLK << LPO_CLK_SHIFT)
>  #define WDOG_CS_EN		BIT(7)
>  #define WDOG_CS_UPDATE		BIT(5)
>  
> @@ -143,7 +146,7 @@ static void imx7ulp_wdt_init(void __iomem *base, unsigned int timeout)
>  	/* set an initial timeout value in TOVAL */
>  	writel(timeout, base + WDOG_TOVAL);
>  	/* enable 32bit command sequence and reconfigure */
> -	val = BIT(13) | BIT(8) | BIT(5);
> +	val = WDOG_CS_CMD32EN | WDOG_CS_CLK | WDOG_CS_UPDATE;
>  	writel(val, base + WDOG_CS);
>  }
>