HP6xx : weird hwclock behavior, bug?

HP6xx : weird hwclock behavior, bug?

[Kristoffer Ericson]

Greetings,

Is this a bug, or am I just confusing stuff?

hwclock --show = works
hwclock --systohc = freezes system

Its hard to say wether it ever worked since we haven't used the rtc much.

Re: HP6xx : weird hwclock behavior, bug?

[Paul Mundt]

On Thu, Feb 07, 2008 at 12:28:25AM +0100, Kristoffer Ericson wrote:
> hwclock --show = works
> hwclock --systohc = freezes system
> 
> Its hard to say wether it ever worked since we haven't used the rtc much.
> 
It works fine on all of the SH-4 systems, but I don't know if anyone has
done any SH-3 testing with rtc-sh.

Re: HP6xx : weird hwclock behavior, bug?

[Kristoffer Ericson]

On Thu, 7 Feb 2008 11:16:07 +0900
Paul Mundt <lethal@linux-sh.org> wrote:

> On Thu, Feb 07, 2008 at 12:28:25AM +0100, Kristoffer Ericson wrote:
> > hwclock --show = works
> > hwclock --systohc = freezes system
> > 
> > Its hard to say wether it ever worked since we haven't used the rtc much.
> > 
> It works fine on all of the SH-4 systems, but I don't know if anyone has
> done any SH-3 testing with rtc-sh.

Oki, any good way to test this? fill the rtc driver with printk's?

Re: HP6xx : weird hwclock behavior, bug?

[Rafael Ignacio Zurita]

--- Paul Mundt <lethal@linux-sh.org> wrote:

> On Thu, Feb 07, 2008 at 12:28:25AM +0100, Kristoffer
> Ericson wrote:
> > hwclock --show = works
> > hwclock --systohc = freezes system
> > 
> > Its hard to say wether it ever worked since we
> haven't used the rtc much.
> > 
> It works fine on all of the SH-4 systems, but I
> don't know if anyone has
> done any SH-3 testing with rtc-sh.

I have been testing rtc-sh today (on sh3) and it seems
that "hwclock --systohc" does not freeze the system but it keeps
looping calling this function:  
static irqreturn_t sh_rtc_alarm(int irq, void *dev_id)

I've added printks and these show me that in sh_rtc_alarm() the
code into the next "if" never is executed:

        /*
         * If AF is set then the alarm has triggered. If we clear AF while
         * the alarm time still matches the RTC time then AF will
         * immediately be set again, and if AIE is enabled then the alarm
         * interrupt will immediately be retrigger. So we clear AIE here
         * and use rtc->rearm_aie so that the carry interrupt will keep
         * trying to clear AF and once it stays cleared it'll re-enable
         * AIE.
         */
        if (tmp & RCR1_AF) {
                events |= RTC_AF | RTC_IRQF;

                tmp &= ~(RCR1_AF|RCR1_AIE);

                writeb(tmp, rtc->regbase + RCR1);

                rtc->rearm_aie = 1;

                rtc_update_irq(rtc->rtc_dev, 1, events);
        }

Perhaps it should not execute the code inside such if in this case, but 
the system keeps looping calling to sh_rtc_alarm and it looks like a
frozen system.

Some idea about how to debug a bit more?
Regards.

Rafael 

Re: HP6xx : weird hwclock behavior, bug?

[Paul Mundt]

On Fri, Feb 08, 2008 at 05:29:23PM -0300, Rafael Ignacio Zurita wrote:
> --- Paul Mundt <lethal@linux-sh.org> wrote:
> > It works fine on all of the SH-4 systems, but I don't know if anyone
> > has done any SH-3 testing with rtc-sh.
> 
> I have been testing rtc-sh today (on sh3) and it seems
> that "hwclock --systohc" does not freeze the system but it keeps
> looping calling this function:  
> static irqreturn_t sh_rtc_alarm(int irq, void *dev_id)
> 
> I've added printks and these show me that in sh_rtc_alarm() the
> code into the next "if" never is executed:
> 
>         /*
>          * If AF is set then the alarm has triggered. If we clear AF while
>          * the alarm time still matches the RTC time then AF will
>          * immediately be set again, and if AIE is enabled then the alarm
>          * interrupt will immediately be retrigger. So we clear AIE here
>          * and use rtc->rearm_aie so that the carry interrupt will keep
>          * trying to clear AF and once it stays cleared it'll re-enable
>          * AIE.
>          */
>         if (tmp & RCR1_AF) {
>                 events |= RTC_AF | RTC_IRQF;
> 
>                 tmp &= ~(RCR1_AF|RCR1_AIE);
> 
>                 writeb(tmp, rtc->regbase + RCR1);
> 
>                 rtc->rearm_aie = 1;
> 
>                 rtc_update_irq(rtc->rtc_dev, 1, events);
>         }
> 
> Perhaps it should not execute the code inside such if in this case, but 
> the system keeps looping calling to sh_rtc_alarm and it looks like a
> frozen system.
> 
> Some idea about how to debug a bit more?

The alarm code was written and tested on SH-4, it is possible that the
SH-3 has a different format or simply doesn't support the alarm at all.
The first thing to do is to look at the RCR1 definition and see how the
alarm bits are set there. If RCR1_AF isn't asserted, then either the IRQ
is spurious, or we're simply checking the wrong condition for the SH-3
case (in which case the ack'ing case won't be right either.

It's fairly trivial to conditionalize the alarm support, but up until now
the SH-3 folks have been pretty silent on the issue (especially with
regards to what works and what doesn't work). You may want to just
comment out the alarm bits for now and ensure that the rest of the code
is functional first, and then see about what's causing the problem on the
alarm side. I expect it's just a matter of bit or register definitions
being different.

Re: HP6xx : weird hwclock behavior, bug?

[Rafael Ignacio Zurita]

On Sat, Feb 09, 2008 at 06:07:10PM +0900, Paul Mundt wrote:
> On Fri, Feb 08, 2008 at 05:29:23PM -0300, Rafael Ignacio Zurita wrote:
> > --- Paul Mundt <lethal@linux-sh.org> wrote:
> > > It works fine on all of the SH-4 systems, but I don't know if anyone
> > > has done any SH-3 testing with rtc-sh.
> > 
> > I have been testing rtc-sh today (on sh3) and it seems
> > that "hwclock --systohc" does not freeze the system but it keeps
> > looping calling this function:  
> > static irqreturn_t sh_rtc_alarm(int irq, void *dev_id)
> >...
> >         if (tmp & RCR1_AF) {
> >... 
> > Perhaps it should not execute the code inside such if in this case, but 
> > the system keeps looping calling to sh_rtc_alarm and it looks like a
> > frozen system.
> > 
> > Some idea about how to debug a bit more?
> 
> The alarm code was written and tested on SH-4, it is possible that the
> SH-3 has a different format or simply doesn't support the alarm at all.

SH-3 supports alarm (from the manual).


> The first thing to do is to look at the RCR1 definition and see how the
> alarm bits are set there. If RCR1_AF isn't asserted, then either the IRQ
> is spurious, or we're simply checking the wrong condition for the SH-3
> case (in which case the ack'ing case won't be right either.

The RCR1 definition and the manner that the alarm bits are set is ok (from
the manual too). But RCR1_AF is not asserted and the condition check for
SH-3 is well. I have checked the manual and the definition and i have
not found problems there. In fact, i have also checked the sh4 manual
and both (rtc) looks quite similar (the only difference is for the year
register).

 
> You may want to just
> comment out the alarm bits for now and ensure that the rest of the code
> is functional first, and then see about what's causing the problem on the
> alarm side. I expect it's just a matter of bit or register definitions
> being different.

Yep, after some re checks it seems that the problem is the sh7709
rtc_resources definition (sh770x) which we use. It says 
(arch/sh/kernel/cpu/sh3/setup-sh770x.c):

static struct resource rtc_resources[] = {
        [0] =   {
                .start  = 0xfffffec0,
                .end    = 0xfffffec0 + 0x1e,
                .flags  = IORESOURCE_IO,
        },
        [1] =   {
                .start  = 20,
                .flags  = IORESOURCE_IRQ,
        },
        [2] =   {
                .start  = 21,
                .flags  = IORESOURCE_IRQ,
        },
        [3] =   {
                .start  = 22,
                .flags  = IORESOURCE_IRQ,
        },
};

From the manual the rtc order for the interrupt sources are:
ATI: Alarm interrupt
PRI: Periodic interrupt
CUI: Carry-up interrupt

When i saw the setup for arch/sh/kernel/cpu/sh4/setup-sh7750.c it has
commets for that resource struct:
static struct resource rtc_resources[] = {
        [0] = {
                .start  = 0xffc80000,
                .end    = 0xffc80000 + 0x58 - 1,
                .flags  = IORESOURCE_IO,
        },
        [1] = {
                /* Period IRQ */
                .start  = 21,
                .flags  = IORESOURCE_IRQ,
        },
        [2] = {
                /* Carry IRQ */
                .start  = 22,
                .flags  = IORESOURCE_IRQ,
        },
        [3] = {
                /* Alarm IRQ */
                .start  = 20,
                .flags  = IORESOURCE_IRQ,
        },
};

So i modified arch/sh/kernel/cpu/sh3/setup-sh770x.c to reflect the order
and after that rtc works well. This patch reflects the changes:

--- /mnt/local/opt/sh3/kernel/linux/jlime-stable.bkp2/arch/sh/kernel/cpu/sh3/setup-sh770x.c     2008-02-06 17:25:47.000000000 -0300
+++ setup-sh770x.c      2008-02-12 20:49:46.000000000 -0300
@@ -130,15 +130,15 @@ static struct resource rtc_resources[]                 .flags  = IORESOURCE_IO,
        },
        [1] =   {
-               .start  = 20,
+               .start  = 21,
                .flags  = IORESOURCE_IRQ,
        },
        [2] =   {
-               .start  = 21,
+               .start  = 22,
                .flags  = IORESOURCE_IRQ,
        },
        [3] =   {
-               .start  = 22,
+               .start  = 20,
                .flags  = IORESOURCE_IRQ,
        },
 };

Because I don't know well the rtc code I would appreciate if someone could
say me if it would be the problem. Moreover, I don't know how to test the alarm
code. Does somebody know?

Regards.

Rafael

Re: HP6xx : weird hwclock behavior, bug?

[Kristoffer Ericson]

On Wed, 13 Feb 2008 10:18:51 -0300
Rafael Ignacio Zurita <rizurita@yahoo.com> wrote:

> 
> On Sat, Feb 09, 2008 at 06:07:10PM +0900, Paul Mundt wrote:
> > On Fri, Feb 08, 2008 at 05:29:23PM -0300, Rafael Ignacio Zurita wrote:
> > > --- Paul Mundt <lethal@linux-sh.org> wrote:
> > > > It works fine on all of the SH-4 systems, but I don't know if anyone
> > > > has done any SH-3 testing with rtc-sh.
> > > 
> > > I have been testing rtc-sh today (on sh3) and it seems
> > > that "hwclock --systohc" does not freeze the system but it keeps
> > > looping calling this function:  
> > > static irqreturn_t sh_rtc_alarm(int irq, void *dev_id)
> > >...
> > >         if (tmp & RCR1_AF) {
> > >... 
> > > Perhaps it should not execute the code inside such if in this case, but 
> > > the system keeps looping calling to sh_rtc_alarm and it looks like a
> > > frozen system.
> > > 
> > > Some idea about how to debug a bit more?
> > 
> > The alarm code was written and tested on SH-4, it is possible that the
> > SH-3 has a different format or simply doesn't support the alarm at all.
> 
> SH-3 supports alarm (from the manual).
> 
> 
> > The first thing to do is to look at the RCR1 definition and see how the
> > alarm bits are set there. If RCR1_AF isn't asserted, then either the IRQ
> > is spurious, or we're simply checking the wrong condition for the SH-3
> > case (in which case the ack'ing case won't be right either.
> 
> The RCR1 definition and the manner that the alarm bits are set is ok (from
> the manual too). But RCR1_AF is not asserted and the condition check for
> SH-3 is well. I have checked the manual and the definition and i have
> not found problems there. In fact, i have also checked the sh4 manual
> and both (rtc) looks quite similar (the only difference is for the year
> register).
> 
>  
> > You may want to just
> > comment out the alarm bits for now and ensure that the rest of the code
> > is functional first, and then see about what's causing the problem on the
> > alarm side. I expect it's just a matter of bit or register definitions
> > being different.
> 
> Yep, after some re checks it seems that the problem is the sh7709
> rtc_resources definition (sh770x) which we use. It says 
> (arch/sh/kernel/cpu/sh3/setup-sh770x.c):
> 
> static struct resource rtc_resources[] = {
>         [0] =   {
>                 .start  = 0xfffffec0,
>                 .end    = 0xfffffec0 + 0x1e,
>                 .flags  = IORESOURCE_IO,
>         },
>         [1] =   {
>                 .start  = 20,
>                 .flags  = IORESOURCE_IRQ,
>         },
>         [2] =   {
>                 .start  = 21,
>                 .flags  = IORESOURCE_IRQ,
>         },
>         [3] =   {
>                 .start  = 22,
>                 .flags  = IORESOURCE_IRQ,
>         },
> };
> 
> From the manual the rtc order for the interrupt sources are:
> ATI: Alarm interrupt
> PRI: Periodic interrupt
> CUI: Carry-up interrupt
> 
> When i saw the setup for arch/sh/kernel/cpu/sh4/setup-sh7750.c it has
> commets for that resource struct:
> static struct resource rtc_resources[] = {
>         [0] = {
>                 .start  = 0xffc80000,
>                 .end    = 0xffc80000 + 0x58 - 1,
>                 .flags  = IORESOURCE_IO,
>         },
>         [1] = {
>                 /* Period IRQ */
>                 .start  = 21,
>                 .flags  = IORESOURCE_IRQ,
>         },
>         [2] = {
>                 /* Carry IRQ */
>                 .start  = 22,
>                 .flags  = IORESOURCE_IRQ,
>         },
>         [3] = {
>                 /* Alarm IRQ */
>                 .start  = 20,
>                 .flags  = IORESOURCE_IRQ,
>         },
> };
> 
> So i modified arch/sh/kernel/cpu/sh3/setup-sh770x.c to reflect the order
> and after that rtc works well. This patch reflects the changes:
> 
> --- /mnt/local/opt/sh3/kernel/linux/jlime-stable.bkp2/arch/sh/kernel/cpu/sh3/setup-sh770x.c     2008-02-06 17:25:47.000000000 -0300
> +++ setup-sh770x.c      2008-02-12 20:49:46.000000000 -0300
> @@ -130,15 +130,15 @@ static struct resource rtc_resources[] >                 .flags  = IORESOURCE_IO,
>         },
>         [1] =   {
> -               .start  = 20,
> +               .start  = 21,
>                 .flags  = IORESOURCE_IRQ,
>         },
>         [2] =   {
> -               .start  = 21,
> +               .start  = 22,
>                 .flags  = IORESOURCE_IRQ,
>         },
>         [3] =   {
> -               .start  = 22,
> +               .start  = 20,
>                 .flags  = IORESOURCE_IRQ,
>         },
>  };
> 
> Because I don't know well the rtc code I would appreciate if someone could
> say me if it would be the problem. Moreover, I don't know how to test the alarm
> code. Does somebody know?
Rafael,

It sounds like a good catch to me. Perhaps paul has some opinions about it but it would explain the bug while also proving the rtc code correct. Please send patch against paul's tree (inserted not attached) with proper subject title. Include log and Signed-off-by: tag.
If there's any issues then Im sure paul will give you feedback. Nice to know that rtc works.

> 
> Regards.
> 
> Rafael
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-sh" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html