linux-rtc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* question about firmware RTC design (for rtcwake)
@ 2023-05-22 14:46 Marek Behún
  2023-05-22 19:40 ` Alexandre Belloni
  0 siblings, 1 reply; 6+ messages in thread
From: Marek Behún @ 2023-05-22 14:46 UTC (permalink / raw)
  To: linux-rtc; +Cc: Alessandro Zummo, Alexandre Belloni

Hello RTC guys,

this is a request for suggestions / question about how to implement a
RTC driver so that it will be accepted into upstream Linux.

Let me start with the description of the problem.

The MCU on the Turris Omnia router can disable / enable power
regulators basically for the whole board, giving capability for
platform specific poweroff and for powering the system on at a
specified time.

I want to add support for this to the MCU firmware and write a driver
for Linux. Since I know only of one utility by which users can request
the system to power on at a specified time - the rtcwake utility - I
want to write this as a RTC driver.

Because the MCU is not a true RTC in the sense that it does not have
battery to count time when not under power, I am wondering how exactly
to design this. One problem is that the RTC driver must implement the
read function, but the MCU does not know the real time unless it is
told beforehand.

Note that the board also has a true RTC with battery, handled by the
rtc-armada38x driver. We can make sure that this true RTC is used for
hctosys.

Proposal 1:
- the MCU will implement only one command, poweroff, with optional
  argument wakeup_timeout, the number of seconds after which to power
  the board up again
- the corresponding driver will register a system off handler and a RTC
  device
- the RTC will implement methods as:
    .read_time   will use ktime_get(), so it will return monotonic clock
    .set_alarm   will store the requested wakeup time in an internal
                 structure
    .alarm_irq_enable   will just update the internal structure
    .read_alarm  will return information from the stored structure
- the system off handler will send the poweroff command and it will
  take the requested wakeup time from the internal structure and
  subtract current time (ktime_get()) to get the wakeup_timeout
  argument for the MCU poweroff command

- advantages:
  - MCU needs only to implement one command
- disadvantages:
  - the new RTC device will not behave at all like a rtc device
    regarding timekeeping, since .read_time returns system's monotonic
    clock. This may confuse users who do not know about the fact that
    this rtc device is only meant for rtcwake. We can print this info
    into dmesg, but...
  - removing the driver and loading it back loses information about set
    wakeup time

Proposal 2:
- the MCU will implement:
  - command for getting and setting clock that the MCU will count
    The clock will be considered invalid unless it was set at least
    once.
  - command for getting and setting wakeup alarm
  - command for power off
- the corresponding driver will again register a system off handler and
  a RTC device
- the system off handler will just send the poweroff command
- the RTC device can now behave like a true RTC device, but without a
  battery, the RTC_VL_BACKUP_EMPTY bit will be set.
  The userspace can set time from the true RTC, so that rtcwake may be
  used

Overall proposal 1 is easier to implement, but the resulting /dev/rtc
device will not be a true RTC, which may confuse users. Proposal 2
gives more complexity to MCU firmware.

Personally I prefer proposal one.

Do you guys have suggestions? Which kind of driver would you be willing
to accept?

Note that the driver will probably live under drivers/mfd or
drivers/platform, since the MCU implements other features, as well (a
GPIO controller, for example).

Marek

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: question about firmware RTC design (for rtcwake)
  2023-05-22 14:46 question about firmware RTC design (for rtcwake) Marek Behún
@ 2023-05-22 19:40 ` Alexandre Belloni
  2023-05-22 21:14   ` Marek Behún
  0 siblings, 1 reply; 6+ messages in thread
From: Alexandre Belloni @ 2023-05-22 19:40 UTC (permalink / raw)
  To: Marek Behún; +Cc: linux-rtc, Alessandro Zummo

Hello,

On 22/05/2023 16:46:38+0200, Marek Behún wrote:
> Hello RTC guys,
> 
> this is a request for suggestions / question about how to implement a
> RTC driver so that it will be accepted into upstream Linux.
> 
> Let me start with the description of the problem.
> 
> The MCU on the Turris Omnia router can disable / enable power
> regulators basically for the whole board, giving capability for
> platform specific poweroff and for powering the system on at a
> specified time.
> 
> I want to add support for this to the MCU firmware and write a driver
> for Linux. Since I know only of one utility by which users can request
> the system to power on at a specified time - the rtcwake utility - I
> want to write this as a RTC driver.
> 
> Because the MCU is not a true RTC in the sense that it does not have
> battery to count time when not under power, I am wondering how exactly
> to design this. One problem is that the RTC driver must implement the
> read function, but the MCU does not know the real time unless it is
> told beforehand.
> 
> Note that the board also has a true RTC with battery, handled by the
> rtc-armada38x driver. We can make sure that this true RTC is used for
> hctosys.
> 
> Proposal 1:
> - the MCU will implement only one command, poweroff, with optional
>   argument wakeup_timeout, the number of seconds after which to power
>   the board up again
> - the corresponding driver will register a system off handler and a RTC
>   device
> - the RTC will implement methods as:
>     .read_time   will use ktime_get(), so it will return monotonic clock
>     .set_alarm   will store the requested wakeup time in an internal
>                  structure
>     .alarm_irq_enable   will just update the internal structure
>     .read_alarm  will return information from the stored structure
> - the system off handler will send the poweroff command and it will
>   take the requested wakeup time from the internal structure and
>   subtract current time (ktime_get()) to get the wakeup_timeout
>   argument for the MCU poweroff command
> 
> - advantages:
>   - MCU needs only to implement one command
> - disadvantages:
>   - the new RTC device will not behave at all like a rtc device
>     regarding timekeeping, since .read_time returns system's monotonic
>     clock. This may confuse users who do not know about the fact that
>     this rtc device is only meant for rtcwake. We can print this info
>     into dmesg, but...
>   - removing the driver and loading it back loses information about set
>     wakeup time
> 
> Proposal 2:
> - the MCU will implement:
>   - command for getting and setting clock that the MCU will count
>     The clock will be considered invalid unless it was set at least
>     once.
>   - command for getting and setting wakeup alarm
>   - command for power off
> - the corresponding driver will again register a system off handler and
>   a RTC device
> - the system off handler will just send the poweroff command
> - the RTC device can now behave like a true RTC device, but without a
>   battery, the RTC_VL_BACKUP_EMPTY bit will be set.
>   The userspace can set time from the true RTC, so that rtcwake may be
>   used
> 
> Overall proposal 1 is easier to implement, but the resulting /dev/rtc
> device will not be a true RTC, which may confuse users. Proposal 2
> gives more complexity to MCU firmware.
> 
> Personally I prefer proposal one.
> 
> Do you guys have suggestions? Which kind of driver would you be willing
> to accept?
> 

You probably need to look at rtc-meson-vrtc.c, rtc-fsl-ftm-alarm.c and
rtc-brcmstb-waketimer.c which implement something similar.

Honestly, I would go for an in-between proposal where you would store
the requested alarm time (or more likely countdown) on
set_alarm/alarm_irq_enable so you would get .read_alarm working.

However, my main concern is that this is yet another custom protocol. We
can't possibly have a driver for everyone implementing a timer in their
FPGA/CPLD/cortexM.

How will you communicate with the MCU, can't you use an already existing
driver?

Regards,

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: question about firmware RTC design (for rtcwake)
  2023-05-22 19:40 ` Alexandre Belloni
@ 2023-05-22 21:14   ` Marek Behún
  2023-06-07  8:28     ` Alexandre Belloni
  0 siblings, 1 reply; 6+ messages in thread
From: Marek Behún @ 2023-05-22 21:14 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: linux-rtc, Alessandro Zummo

On Mon, 22 May 2023 21:40:09 +0200
Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:

> Hello,
> 
> On 22/05/2023 16:46:38+0200, Marek Behún wrote:
> > Hello RTC guys,
> > 
> > this is a request for suggestions / question about how to implement a
> > RTC driver so that it will be accepted into upstream Linux.
> > 
> > Let me start with the description of the problem.
> > 
> > The MCU on the Turris Omnia router can disable / enable power
> > regulators basically for the whole board, giving capability for
> > platform specific poweroff and for powering the system on at a
> > specified time.
> > 
> > I want to add support for this to the MCU firmware and write a driver
> > for Linux. Since I know only of one utility by which users can request
> > the system to power on at a specified time - the rtcwake utility - I
> > want to write this as a RTC driver.
> > 
> > Because the MCU is not a true RTC in the sense that it does not have
> > battery to count time when not under power, I am wondering how exactly
> > to design this. One problem is that the RTC driver must implement the
> > read function, but the MCU does not know the real time unless it is
> > told beforehand.
> > 
> > Note that the board also has a true RTC with battery, handled by the
> > rtc-armada38x driver. We can make sure that this true RTC is used for
> > hctosys.
> > 
> > Proposal 1:
> > - the MCU will implement only one command, poweroff, with optional
> >   argument wakeup_timeout, the number of seconds after which to power
> >   the board up again
> > - the corresponding driver will register a system off handler and a RTC
> >   device
> > - the RTC will implement methods as:
> >     .read_time   will use ktime_get(), so it will return monotonic clock
> >     .set_alarm   will store the requested wakeup time in an internal
> >                  structure
> >     .alarm_irq_enable   will just update the internal structure
> >     .read_alarm  will return information from the stored structure
> > - the system off handler will send the poweroff command and it will
> >   take the requested wakeup time from the internal structure and
> >   subtract current time (ktime_get()) to get the wakeup_timeout
> >   argument for the MCU poweroff command
> > 
> > - advantages:
> >   - MCU needs only to implement one command
> > - disadvantages:
> >   - the new RTC device will not behave at all like a rtc device
> >     regarding timekeeping, since .read_time returns system's monotonic
> >     clock. This may confuse users who do not know about the fact that
> >     this rtc device is only meant for rtcwake. We can print this info
> >     into dmesg, but...
> >   - removing the driver and loading it back loses information about set
> >     wakeup time
> > 
> > Proposal 2:
> > - the MCU will implement:
> >   - command for getting and setting clock that the MCU will count
> >     The clock will be considered invalid unless it was set at least
> >     once.
> >   - command for getting and setting wakeup alarm
> >   - command for power off
> > - the corresponding driver will again register a system off handler and
> >   a RTC device
> > - the system off handler will just send the poweroff command
> > - the RTC device can now behave like a true RTC device, but without a
> >   battery, the RTC_VL_BACKUP_EMPTY bit will be set.
> >   The userspace can set time from the true RTC, so that rtcwake may be
> >   used
> > 
> > Overall proposal 1 is easier to implement, but the resulting /dev/rtc
> > device will not be a true RTC, which may confuse users. Proposal 2
> > gives more complexity to MCU firmware.
> > 
> > Personally I prefer proposal one.
> > 
> > Do you guys have suggestions? Which kind of driver would you be willing
> > to accept?
> >   
> 
> You probably need to look at rtc-meson-vrtc.c, rtc-fsl-ftm-alarm.c and
> rtc-brcmstb-waketimer.c which implement something similar.
> 
> Honestly, I would go for an in-between proposal where you would store
> the requested alarm time (or more likely countdown) on
> set_alarm/alarm_irq_enable so you would get .read_alarm working.
> 
> However, my main concern is that this is yet another custom protocol. We
> can't possibly have a driver for everyone implementing a timer in their
> FPGA/CPLD/cortexM.
> 
> How will you communicate with the MCU, can't you use an already existing
> driver?

The MCU exposes a command interface over I2C. There already are
existing commands, which needs to stay for backwards compatibility.

It is theoretically possible to simulate an existing RTC device on
another I2C address, but I would need to study them, because the boards
are shipped with three different MCUs (STM32, GD32, NXP's MKL81) and
they sometimes have a little different I2C slave behavior.

But I will need to create a platform/mfd driver anyway for the system
off handler and GPIO controller. If I am going to create a new driver
anyway, why not add the RTC functionality as well?

Marek

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: question about firmware RTC design (for rtcwake)
  2023-05-22 21:14   ` Marek Behún
@ 2023-06-07  8:28     ` Alexandre Belloni
  2023-06-08  9:05       ` Marek Behún
  0 siblings, 1 reply; 6+ messages in thread
From: Alexandre Belloni @ 2023-06-07  8:28 UTC (permalink / raw)
  To: Marek Behún; +Cc: linux-rtc, Alessandro Zummo

On 22/05/2023 23:14:54+0200, Marek Behún wrote:
> > You probably need to look at rtc-meson-vrtc.c, rtc-fsl-ftm-alarm.c and
> > rtc-brcmstb-waketimer.c which implement something similar.
> > 
> > Honestly, I would go for an in-between proposal where you would store
> > the requested alarm time (or more likely countdown) on
> > set_alarm/alarm_irq_enable so you would get .read_alarm working.
> > 
> > However, my main concern is that this is yet another custom protocol. We
> > can't possibly have a driver for everyone implementing a timer in their
> > FPGA/CPLD/cortexM.
> > 
> > How will you communicate with the MCU, can't you use an already existing
> > driver?
> 
> The MCU exposes a command interface over I2C. There already are
> existing commands, which needs to stay for backwards compatibility.
> 
> It is theoretically possible to simulate an existing RTC device on
> another I2C address, but I would need to study them, because the boards
> are shipped with three different MCUs (STM32, GD32, NXP's MKL81) and
> they sometimes have a little different I2C slave behavior.
> 
> But I will need to create a platform/mfd driver anyway for the system
> off handler and GPIO controller. If I am going to create a new driver
> anyway, why not add the RTC functionality as well?

No, this is not how MFD is working, you will be writing a separate RTC
driver or reusing an existing one. Have a look at the recent isl1208
series:

https://lore.kernel.org/linux-rtc/OS0PR01MB5922DAC377266672ADA9FC28864DA@OS0PR01MB5922.jpnprd01.prod.outlook.com/T/#mab0a75187abf7d8aada2c3517ebfdf7241f4bc7a

This patch adds supports for the isl1208 on board of a PMIC, as you can
see, this is a very small change versus a full blown RTC driver.


-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: question about firmware RTC design (for rtcwake)
  2023-06-07  8:28     ` Alexandre Belloni
@ 2023-06-08  9:05       ` Marek Behún
  2023-06-08  9:25         ` Alexandre Belloni
  0 siblings, 1 reply; 6+ messages in thread
From: Marek Behún @ 2023-06-08  9:05 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: linux-rtc, Alessandro Zummo

On Wed, 7 Jun 2023 10:28:28 +0200
Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:

> On 22/05/2023 23:14:54+0200, Marek Behún wrote:
> > > You probably need to look at rtc-meson-vrtc.c, rtc-fsl-ftm-alarm.c and
> > > rtc-brcmstb-waketimer.c which implement something similar.
> > > 
> > > Honestly, I would go for an in-between proposal where you would store
> > > the requested alarm time (or more likely countdown) on
> > > set_alarm/alarm_irq_enable so you would get .read_alarm working.
> > > 
> > > However, my main concern is that this is yet another custom protocol. We
> > > can't possibly have a driver for everyone implementing a timer in their
> > > FPGA/CPLD/cortexM.
> > > 
> > > How will you communicate with the MCU, can't you use an already existing
> > > driver?  
> > 
> > The MCU exposes a command interface over I2C. There already are
> > existing commands, which needs to stay for backwards compatibility.
> > 
> > It is theoretically possible to simulate an existing RTC device on
> > another I2C address, but I would need to study them, because the boards
> > are shipped with three different MCUs (STM32, GD32, NXP's MKL81) and
> > they sometimes have a little different I2C slave behavior.
> > 
> > But I will need to create a platform/mfd driver anyway for the system
> > off handler and GPIO controller. If I am going to create a new driver
> > anyway, why not add the RTC functionality as well?  
> 
> No, this is not how MFD is working, you will be writing a separate RTC
> driver or reusing an existing one. Have a look at the recent isl1208

THX for the reply.
I have one I2C client through which I need to implement RTC, GPIO and
system power off. I thought such drivers live in drivers/mfd... Am I
wrong?

Marek


> series:
> 
> https://lore.kernel.org/linux-rtc/OS0PR01MB5922DAC377266672ADA9FC28864DA@OS0PR01MB5922.jpnprd01.prod.outlook.com/T/#mab0a75187abf7d8aada2c3517ebfdf7241f4bc7a
> 
> This patch adds supports for the isl1208 on board of a PMIC, as you can
> see, this is a very small change versus a full blown RTC driver.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: question about firmware RTC design (for rtcwake)
  2023-06-08  9:05       ` Marek Behún
@ 2023-06-08  9:25         ` Alexandre Belloni
  0 siblings, 0 replies; 6+ messages in thread
From: Alexandre Belloni @ 2023-06-08  9:25 UTC (permalink / raw)
  To: Marek Behún; +Cc: linux-rtc, Alessandro Zummo

On 08/06/2023 11:05:57+0200, Marek Behún wrote:
> On Wed, 7 Jun 2023 10:28:28 +0200
> Alexandre Belloni <alexandre.belloni@bootlin.com> wrote:
> 
> > On 22/05/2023 23:14:54+0200, Marek Behún wrote:
> > > > You probably need to look at rtc-meson-vrtc.c, rtc-fsl-ftm-alarm.c and
> > > > rtc-brcmstb-waketimer.c which implement something similar.
> > > > 
> > > > Honestly, I would go for an in-between proposal where you would store
> > > > the requested alarm time (or more likely countdown) on
> > > > set_alarm/alarm_irq_enable so you would get .read_alarm working.
> > > > 
> > > > However, my main concern is that this is yet another custom protocol. We
> > > > can't possibly have a driver for everyone implementing a timer in their
> > > > FPGA/CPLD/cortexM.
> > > > 
> > > > How will you communicate with the MCU, can't you use an already existing
> > > > driver?  
> > > 
> > > The MCU exposes a command interface over I2C. There already are
> > > existing commands, which needs to stay for backwards compatibility.
> > > 
> > > It is theoretically possible to simulate an existing RTC device on
> > > another I2C address, but I would need to study them, because the boards
> > > are shipped with three different MCUs (STM32, GD32, NXP's MKL81) and
> > > they sometimes have a little different I2C slave behavior.
> > > 
> > > But I will need to create a platform/mfd driver anyway for the system
> > > off handler and GPIO controller. If I am going to create a new driver
> > > anyway, why not add the RTC functionality as well?  
> > 
> > No, this is not how MFD is working, you will be writing a separate RTC
> > driver or reusing an existing one. Have a look at the recent isl1208
> 
> THX for the reply.
> I have one I2C client through which I need to implement RTC, GPIO and
> system power off. I thought such drivers live in drivers/mfd... Am I
> wrong?

You are not wrong but the code doesn't necessarily live in drivers/mfd
but is in the subsystems instead.

> 
> Marek
> 
> 
> > series:
> > 
> > https://lore.kernel.org/linux-rtc/OS0PR01MB5922DAC377266672ADA9FC28864DA@OS0PR01MB5922.jpnprd01.prod.outlook.com/T/#mab0a75187abf7d8aada2c3517ebfdf7241f4bc7a
> > 
> > This patch adds supports for the isl1208 on board of a PMIC, as you can
> > see, this is a very small change versus a full blown RTC driver.
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-06-08  9:26 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-22 14:46 question about firmware RTC design (for rtcwake) Marek Behún
2023-05-22 19:40 ` Alexandre Belloni
2023-05-22 21:14   ` Marek Behún
2023-06-07  8:28     ` Alexandre Belloni
2023-06-08  9:05       ` Marek Behún
2023-06-08  9:25         ` Alexandre Belloni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).