Re: [PATCH v2 1/1] watchdog: realtek-otto: Change to use regmap API

[Sander Vanheule]

Hi Rustam,

On Tue, 2026-05-19 at 23:23 +0500, Rustam Adilov wrote:
[...]
> @@ -74,24 +75,17 @@ struct otto_wdt_ctrl {
>  static int otto_wdt_start(struct watchdog_device *wdev)
>  {
>  	struct otto_wdt_ctrl *ctrl = watchdog_get_drvdata(wdev);
> -	u32 v;
> -
> -	v = ioread32(ctrl->base + OTTO_WDT_REG_CTRL);
> -	v |= OTTO_WDT_CTRL_ENABLE;
> -	iowrite32(v, ctrl->base + OTTO_WDT_REG_CTRL);
>  
> +	regmap_set_bits(ctrl->regmap, OTTO_WDT_REG_CTRL,
> OTTO_WDT_CTRL_ENABLE);
>  	return 0;

iowrite32() doesn't return anything, but regmap_set_bits() does.

You can turn this into
    return regmap_set_bits(...);

>  }
>  
>  static int otto_wdt_stop(struct watchdog_device *wdev)
>  {
>  	struct otto_wdt_ctrl *ctrl = watchdog_get_drvdata(wdev);
> -	u32 v;
> -
> -	v = ioread32(ctrl->base + OTTO_WDT_REG_CTRL);
> -	v &= ~OTTO_WDT_CTRL_ENABLE;
> -	iowrite32(v, ctrl->base + OTTO_WDT_REG_CTRL);
>  
> +	regmap_clear_bits(ctrl->regmap, OTTO_WDT_REG_CTRL,
> +			  OTTO_WDT_CTRL_ENABLE);
>  	return 0;

Same as above.
This is probably short enough to keep is on one line? (< 100 characters)

>  }
>  
> @@ -99,8 +93,7 @@ static int otto_wdt_ping(struct watchdog_device *wdev)
>  {
>  	struct otto_wdt_ctrl *ctrl = watchdog_get_drvdata(wdev);
>  
> -	iowrite32(OTTO_WDT_CNTR_PING, ctrl->base + OTTO_WDT_REG_CNTR);
> -
> +	regmap_write(ctrl->regmap, OTTO_WDT_REG_CNTR, OTTO_WDT_CNTR_PING);
>  	return 0;

Same as above.

>  }
>  
> @@ -126,7 +119,7 @@ static int otto_wdt_determine_timeouts(struct
> watchdog_device *wdev, unsigned in
>  	unsigned int total_ticks;
>  	unsigned int prescale;
>  	unsigned int tick_ms;
> -	u32 v;
> +	u32 mask, val;
>  
>  	do {
>  		prescale = prescale_next;
> @@ -142,14 +135,11 @@ static int otto_wdt_determine_timeouts(struct
> watchdog_device *wdev, unsigned in
>  	} while (phase1_ticks > OTTO_WDT_PHASE_TICKS_MAX
>  		|| phase2_ticks > OTTO_WDT_PHASE_TICKS_MAX);
>  
> -	v = ioread32(ctrl->base + OTTO_WDT_REG_CTRL);
> -
> -	v &= ~(OTTO_WDT_CTRL_PRESCALE | OTTO_WDT_CTRL_PHASE1 |
> OTTO_WDT_CTRL_PHASE2);
> -	v |= FIELD_PREP(OTTO_WDT_CTRL_PHASE1, phase1_ticks - 1);
> -	v |= FIELD_PREP(OTTO_WDT_CTRL_PHASE2, phase2_ticks - 1);
> -	v |= FIELD_PREP(OTTO_WDT_CTRL_PRESCALE, prescale);
> -
> -	iowrite32(v, ctrl->base + OTTO_WDT_REG_CTRL);
> +	mask = OTTO_WDT_CTRL_PRESCALE | OTTO_WDT_CTRL_PHASE1 |
> OTTO_WDT_CTRL_PHASE2;
> +	val = FIELD_PREP(OTTO_WDT_CTRL_PHASE1, phase1_ticks - 1);
> +	val |= FIELD_PREP(OTTO_WDT_CTRL_PHASE2, phase2_ticks - 1);
> +	val |= FIELD_PREP(OTTO_WDT_CTRL_PRESCALE, prescale);
> +	regmap_update_bits(ctrl->regmap, OTTO_WDT_REG_CTRL, mask, val);

To be consistent, you would also need to propagate the result of
regmap_update_bits() if it's an error. Same for the other regmap_*() calls.


Best,
Sander