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@xxxxxxxxxxx>
Thanks,
Miquèl
