SA1100/PXA RTC clean-up

A git branch is here[4]. I've tested sa1100-rtc on PXA1928. I need help testing on PXA27x/3xx.

Quoted 2 lines · Rob Herring

A git branch is here[4]. I've tested sa1100-rtc on PXA1928. I need help testing on PXA27x/3xx.

Hi Rob,

I tested on PXA27x. It works for the boards which used pxa-rtc. For the others, their defconfig become broken, because the pxa boards with sa1100-rtc do not work anymore. And of course no .config manipulation will fix this, because PXA architecture is denied the sa1100-rtc driver.

Quoted 10 lines · Robert Jarzmik

Quoted 2 lines · Rob Herring

A git branch is here[4]. I've tested sa1100-rtc on PXA1928. I need help testing on PXA27x/3xx.

Hi Rob,

I tested on PXA27x. It works for the boards which used pxa-rtc. For the others, their defconfig become broken, because the pxa boards with sa1100-rtc do not work anymore. And of course no .config manipulation will fix this, because PXA architecture is denied the sa1100-rtc driver.

Ah, I didn't realize there is a mixture of use.

I could just do something like this instead of removing sa1100_device_rtc registration:

#ifndef CONFIG_RTC_DRV_PXA
       &sa1100_device_rtc,

#else
&pxa_device_rtc,
#endif

Quoted 14 lines · Rob Herring

Quoted 10 lines · Robert Jarzmik

Quoted 2 lines · Rob Herring

A git branch is here[4]. I've tested sa1100-rtc on PXA1928. I need help testing on PXA27x/3xx.

Hi Rob,

I tested on PXA27x. It works for the boards which used pxa-rtc. For the others, their defconfig become broken, because the pxa boards with sa1100-rtc do not work anymore. And of course no .config manipulation will fix this, because PXA architecture is denied the sa1100-rtc driver.

Ah, I didn't realize there is a mixture of use.

Quoted 8 lines

I could just do something like this instead of removing sa1100_device_rtc registration:

#ifndef CONFIG_RTC_DRV_PXA
       &sa1100_device_rtc,

#else
&pxa_device_rtc,
#endif

I ... don't like that much, especially when you consider both rtc might be modules for example. I rather like that patch in pxa as they are.

I'm rather in a mood for a more aggressive approach :

- you fire an incremental patch to patch the 10 defconfigs on PXA architecture (pxa27x and pxa3xx)

- in this patch, you replace 's/CONFIG_RTC_DRV_SA1100/CONFIG_RTC_DRV_PXA/'
- you send this patch to the maintainers and me
- you explain in the commit message that from a userland perspective, nothing changes, except that the RTC IP will change, and any dependency on a

bootloader fidling with RTC should be considered as a source of regression.

Moreover the first reboot will have the wrong date, until it is set.

Quoted 4 lines

I'm rather in a mood for a more aggressive approach :

- you fire an incremental patch to patch the 10 defconfigs on PXA architecture (pxa27x and pxa3xx)

Here's the breakdown of configs which enable SA1100 RTC only. Only the last 6 need to change by my count:

pxa25x
arch/arm/configs/h5000_defconfig
arch/arm/configs/xcep_defconfig
arch/arm/configs/viper_defconfig

pxa255/270
arch/arm/configs/cm_x2xx_defconfig

pxa270
arch/arm/configs/em_x270_defconfig
arch/arm/configs/magician_defconfig
arch/arm/configs/palmz72_defconfig
arch/arm/configs/pcm027_defconfig
arch/arm/configs/trizeps4_defconfig

Quoted 4 lines

- in this patch, you replace 's/CONFIG_RTC_DRV_SA1100/CONFIG_RTC_DRV_PXA/'
- you send this patch to the maintainers and me
- you explain in the commit message that from a userland perspective, nothing changes, except that the RTC IP will change, and any dependency on a

The IP does not change here. rtc0 is still going to be the SA1100 RTC being registered first. The only change will be the addition of rtc1.

Quoted 1 line

bootloader fidling with RTC should be considered as a source of regression.

I'm not sure that I follow.

Quoted 1 line

Moreover the first reboot will have the wrong date, until it is set.

Only for rtc1, right?

Quoted 16 lines · Rob Herring

Quoted 4 lines

I'm rather in a mood for a more aggressive approach :

- you fire an incremental patch to patch the 10 defconfigs on PXA architecture (pxa27x and pxa3xx)

Here's the breakdown of configs which enable SA1100 RTC only. Only the last 6 need to change by my count:

pxa25x
arch/arm/configs/h5000_defconfig
arch/arm/configs/xcep_defconfig
arch/arm/configs/viper_defconfig

pxa255/270
arch/arm/configs/cm_x2xx_defconfig

Quoted 6 lines

pxa270
arch/arm/configs/em_x270_defconfig
arch/arm/configs/magician_defconfig
arch/arm/configs/palmz72_defconfig
arch/arm/configs/pcm027_defconfig
arch/arm/configs/trizeps4_defconfig

And arch/arm/configs/jornada720_defconfig I counted wrongly as it's sa1100. So I agree with your list of last 6 defconfigs.

Quoted 5 lines

Quoted 4 lines

- in this patch, you replace 's/CONFIG_RTC_DRV_SA1100/CONFIG_RTC_DRV_PXA/'
- you send this patch to the maintainers and me
- you explain in the commit message that from a userland perspective, nothing changes, except that the RTC IP will change, and any dependency on a

Quoted 2 lines

The IP does not change here. rtc0 is still going to be the SA1100 RTC being registered first. The only change will be the addition of rtc1.

For boards which were only using rtc-pxa.c (as mioa701 for example), they relied on the fact that rtc0 == pxa_rtc. Their time is stored in PXA IP. Therefore, each of their hwclock will end up on sa1100-rtc instead of pxa-rtc.

So for these boards, ie. for all boards where only rtc-pxa.c was used, the IP addressed changes from a casual userspace perspective.

Quoted 4 lines

Quoted 2 lines

bootloader fidling with RTC should be considered as a source of regression.

I'm not sure that I follow.

Let's talk about how a double boot windows + linux box works. The bootloader ensures that :
- sa1100-rtc holds the number of seconds since the OS start (think jiffies)
- pxa-rtc holds the wall clock time

Upon each reboot, sa1100-rtc is checked to see how much time has passed. If an "oustanding number" is detected, for example 10 years, the firmware resets the data partition.

Now think what will happen when this change will be commited, upon the first reboot after the linux kernel has change sa1100-rtc time.

Quoted 3 lines

Quoted 2 lines

Moreover the first reboot will have the wrong date, until it is set.

Only for rtc1, right?

Yes, right, unless the bootloader logic is ... over-engineered.

All of this to say maintainers should be forwarned at least. After that, up to them to react.

Cheers.

--
Robert

Quoted 9 lines

Quoted 5 lines

Quoted 2 lines

- you explain in the commit message that from a userland perspective, nothing changes, except that the RTC IP will change, and any dependency on a

The IP does not change here. rtc0 is still going to be the SA1100 RTC being registered first. The only change will be the addition of rtc1.

For boards which were only using rtc-pxa.c (as mioa701 for example), they relied on the fact that rtc0 == pxa_rtc. Their time is stored in PXA IP. Therefore, each of their hwclock will end up on sa1100-rtc instead of pxa-rtc.

Quoted 2 lines

So for these boards, ie. for all boards where only rtc-pxa.c was used, the IP addressed changes from a casual userspace perspective.

Okay, so this is the case where the time will be wrong.

I could remove the select of the sa1100-rtc and do an empty function for sa1100_rtc_init. This would preserve current behavior.

Quoted 12 lines

Quoted 3 lines

Quoted 1 line

bootloader fidling with RTC should be considered as a source of regression.

I'm not sure that I follow.

Let's talk about how a double boot windows + linux box works. The bootloader ensures that :
- sa1100-rtc holds the number of seconds since the OS start (think jiffies)
- pxa-rtc holds the wall clock time

Upon each reboot, sa1100-rtc is checked to see how much time has passed. If an "oustanding number" is detected, for example 10 years, the firmware resets the data partition.

Quoted 2 lines

Now think what will happen when this change will be commited, upon the first reboot after the linux kernel has change sa1100-rtc time.

That would be bad. But on these platforms, the kernel has been using both RTCs right? Presumably on platforms only using 1 of the RTCs, the bootloader does not touch the RTCs.

Quoted 2 lines

All of this to say maintainers should be forwarned at least. After that, up to them to react.

Quoted 9 lines · Rob Herring

Quoted 9 lines

Quoted 5 lines

Quoted 2 lines

- you explain in the commit message that from a userland perspective, nothing changes, except that the RTC IP will change, and any dependency on a

The IP does not change here. rtc0 is still going to be the SA1100 RTC being registered first. The only change will be the addition of rtc1.

For boards which were only using rtc-pxa.c (as mioa701 for example), they relied on the fact that rtc0 == pxa_rtc. Their time is stored in PXA IP. Therefore, each of their hwclock will end up on sa1100-rtc instead of pxa-rtc.

Quoted 7 lines

Quoted 2 lines

So for these boards, ie. for all boards where only rtc-pxa.c was used, the IP addressed changes from a casual userspace perspective.

Okay, so this is the case where the time will be wrong.

I could remove the select of the sa1100-rtc and do an empty function for sa1100_rtc_init. This would preserve current behavior.

Please don't. I only ask for a sentence in the commit message.

Quoted 12 lines

Quoted 12 lines

Quoted 3 lines

Quoted 1 line

bootloader fidling with RTC should be considered as a source of regression.

I'm not sure that I follow.

Let's talk about how a double boot windows + linux box works. The bootloader ensures that :
- sa1100-rtc holds the number of seconds since the OS start (think jiffies)
- pxa-rtc holds the wall clock time

Upon each reboot, sa1100-rtc is checked to see how much time has passed. If an "oustanding number" is detected, for example 10 years, the firmware resets the data partition.

Quoted 6 lines

Quoted 2 lines

Now think what will happen when this change will be commited, upon the first reboot after the linux kernel has change sa1100-rtc time.

That would be bad. But on these platforms, the kernel has been using both RTCs right? Presumably on platforms only using 1 of the RTCs, the bootloader does not touch the RTCs.

No, the kernel has been using only pxa-rtc. The sa1100-rtc is not used, and yet it accounts the passed time in it.

On platforms where all the OSes use only 1 RTC, the bootloader only touches one RTC, that's correct. Or said differently : the bootloader touches the union of all the RTCs the OSes it launches do touch.

From e43db14b27486faef8d8e74e4aa555aba7f31a0e Mon Sep 17 00:00:00 2001

Quoted 3 lines

On platforms where all the OSes use only 1 RTC, the bootloader only touches one RTC, that's correct. Or said differently : the bootloader touches the union of all the RTCs the OSes it launches do touch.

Sorry, I'm being dense here, but I'm not any less confused as to which scenarios you think have an issue. Rather than go around again on this, perhaps you can just provide the warning for the commit text. Here's what I have.

8<------------------------------------------------------------------- From e43db14b27486faef8d8e74e4aa555aba7f31a0e Mon Sep 17 00:00:00 2001
From: Rob Herring <[email protected]>
Date: Mon, 1 Jun 2015 07:53:01 -0500
Subject: [PATCH] ARM: config: Switch PXA27x platforms to use PXA RTC driver

With the SA1100 and PXA RTC drivers be mutually exclusive and no longer sharing hardware, PXA27x/PXA3xx platforms must use the PXA RTC driver as the SA1100 platform device is no longer registered.

This change should be transparent to userspace, but the first reboot after the switch will have the wrong time.