Re: [PATCH v2 3/3] rtc: rzn1: support input frequencies other than 32768Hz

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hello Wolfram,

...

> @@ -357,22 +359,21 @@ static int rzn1_rtc_set_offset(struct device *dev, long offset)
>  	return 0;
>  }
>  
> -static const struct rtc_class_ops rzn1_rtc_ops = {
> +static struct rtc_class_ops rzn1_rtc_ops = {
>  	.read_time = rzn1_rtc_read_time,
>  	.set_time = rzn1_rtc_set_time,
>  	.read_alarm = rzn1_rtc_read_alarm,
>  	.set_alarm = rzn1_rtc_set_alarm,
>  	.alarm_irq_enable = rzn1_rtc_alarm_irq_enable,
> -	.read_offset = rzn1_rtc_read_offset,
> -	.set_offset = rzn1_rtc_set_offset,
>  };

...

> -	writel(RZN1_RTC_CTL0_CE | RZN1_RTC_CTL0_AMPM | RZN1_RTC_CTL0_SLSB_SUBU,
> -	       rtc->base + RZN1_RTC_CTL0);
> +	/* Set desired modes while leaving the controller disabled */
> +	writel(RZN1_RTC_CTL0_AMPM | scmp_val, rtc->base + RZN1_RTC_CTL0);
> +
> +	if (scmp_val) {
> +		writel(rate - 1, rtc->base + RZN1_RTC_SCMP);
> +	} else {
> +		rzn1_rtc_ops.read_offset = rzn1_rtc_read_offset;
> +		rzn1_rtc_ops.set_offset = rzn1_rtc_set_offset;

I guess this situation is not possible but let me ask. Do we care
about the very unlikely case where two RTCs are available and configured
differently? Because we are overwriting a static entry here. Maybe we
could have two static structures and pick the correct one during probe
instead.

> +	}
> +
> +	/* Enable controller finally */
> +	writel(RZN1_RTC_CTL0_CE | RZN1_RTC_CTL0_AMPM | scmp_val, rtc->base + RZN1_RTC_CTL0);
>  
>  	/* Disable all interrupts */
>  	writel(0, rtc->base + RZN1_RTC_CTL1);

Anyhow, I don't see this comment as a no-go so,

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl