[QUESTION] what's the problem with CBUS ?

[QUESTION] what's the problem with CBUS ?

[Felipe Balbi]

Hi Tony,

What was the problem you saw with N810 which caused the need for
commit fc8c2f1dba400e9a0d1d82756e0e2f52dfe2f4b6, quoted below ?

I mean, looking into the code, all retu_rtc_do_reset() does is setup a
few default values on a few registers and that'll only be used if the
calibration register has a value of 'zero', so I'm suspecting something
(NOLO ?) toggles the gpio reset line on retu before giving control back
to kernel ?!?

Also, the rtc->reset_occured flag seems to be quite useless as it's
never read, only written.

When you figured you needed to disable retu_rtc_do_reset() did you get
any Kernel Oops or N810 simply got stuck ? If it got stuck, it could be
that nothing is calling complete(&rtc->sync), which would mean no RTC
IRQ is happening after a reset.

> Author: Tony Lindgren <tony@atomide.com>
> Date:   Wed Nov 24 17:21:37 2010 -0800
> 
>     cbus: Disable retu_rtc_do_reset for now
>     
>     Otherwise interrupts get enabled too early.
>     
>     This is only a temporary fix to boot N810. For
>     some reason N800 does not need this.
>     
>     Signed-off-by: Tony Lindgren <tony@atomide.com>
> 
> diff --git a/drivers/cbus/retu-rtc.c b/drivers/cbus/retu-rtc.c
> index c0fe481..34fb689 100644
> --- a/drivers/cbus/retu-rtc.c
> +++ b/drivers/cbus/retu-rtc.c
> @@ -309,9 +309,10 @@ static int __init retu_rtc_probe(struct platform_device *pdev)
>          * power */
>         if (retu_read_reg(RETU_REG_RTCCALR) & 0x00ff)
>                 rtc->reset_occurred = 0;
> +#if 0
>         else
>                 retu_rtc_do_reset(rtc);
> -
> +#endif
>  
>         rtc->rtc = rtc_device_register(pdev->name, &pdev->dev, &
>                         retu_rtc_ops, THIS_MODULE);


-- 
balbi

Re: [QUESTION] what's the problem with CBUS ?

[Felipe Balbi]

Adding Tony,

forgot initially

On Mon, Dec 27, 2010 at 07:13:09PM +0200, Felipe Balbi wrote:
>Hi Tony,
>
>What was the problem you saw with N810 which caused the need for
>commit fc8c2f1dba400e9a0d1d82756e0e2f52dfe2f4b6, quoted below ?
>
>I mean, looking into the code, all retu_rtc_do_reset() does is setup a
>few default values on a few registers and that'll only be used if the
>calibration register has a value of 'zero', so I'm suspecting something
>(NOLO ?) toggles the gpio reset line on retu before giving control back
>to kernel ?!?
>
>Also, the rtc->reset_occured flag seems to be quite useless as it's
>never read, only written.
>
>When you figured you needed to disable retu_rtc_do_reset() did you get
>any Kernel Oops or N810 simply got stuck ? If it got stuck, it could be
>that nothing is calling complete(&rtc->sync), which would mean no RTC
>IRQ is happening after a reset.
>
>>Author: Tony Lindgren <tony@atomide.com>
>>Date:   Wed Nov 24 17:21:37 2010 -0800
>>
>>    cbus: Disable retu_rtc_do_reset for now
>>    Otherwise interrupts get enabled too early.
>>    This is only a temporary fix to boot N810. For
>>    some reason N800 does not need this.
>>    Signed-off-by: Tony Lindgren <tony@atomide.com>
>>
>>diff --git a/drivers/cbus/retu-rtc.c b/drivers/cbus/retu-rtc.c
>>index c0fe481..34fb689 100644
>>--- a/drivers/cbus/retu-rtc.c
>>+++ b/drivers/cbus/retu-rtc.c
>>@@ -309,9 +309,10 @@ static int __init retu_rtc_probe(struct platform_device *pdev)
>>         * power */
>>        if (retu_read_reg(RETU_REG_RTCCALR) & 0x00ff)
>>                rtc->reset_occurred = 0;
>>+#if 0
>>        else
>>                retu_rtc_do_reset(rtc);
>>-
>>+#endif
>>        rtc->rtc = rtc_device_register(pdev->name, &pdev->dev, &
>>                        retu_rtc_ops, THIS_MODULE);
>
>
>-- 
>balbi

-- 
balbi

Re: [QUESTION] what's the problem with CBUS ?

[Tony Lindgren]

Hi,

Sorry for the delay in responding, mostly in read-only mode this
week :)

* Felipe Balbi <balbi@ti.com> [101227 09:17]:
> >
> >What was the problem you saw with N810 which caused the need for
> >commit fc8c2f1dba400e9a0d1d82756e0e2f52dfe2f4b6, quoted below ?

Heh found it after some debugging, we need to call init_completion :)

> >Also, the rtc->reset_occured flag seems to be quite useless as it's
> >never read, only written.

Yeah looks like that can be removed.

Reverted the earlier if 0 hack and pushed the following patch
to the cbus branch.

Regards,

Tony


From: Tony Lindgren <tony@atomide.com>
Date: Thu, 30 Dec 2010 12:47:00 -0800
Subject: [PATCH] cbus: Fix retu_rtc_do_reset

We need to also call init_completion before using it.

Signed-off-by: Tony Lindgren <tony@atomide.com>

--- a/drivers/cbus/retu-rtc.c
+++ b/drivers/cbus/retu-rtc.c
@@ -289,6 +289,7 @@ static int __init retu_rtc_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, rtc);
 	INIT_WORK(&rtc->work, retu_rtca_expired);
 	mutex_init(&rtc->mutex);
+	init_completion(&rtc->sync);
 
 	r = retu_get_status();
 	if (!r) {

Re: [QUESTION] what's the problem with CBUS ?

[Felipe Balbi]

On Thu, Dec 30, 2010 at 12:53:31PM -0800, Tony Lindgren wrote:
>Hi,
>
>Sorry for the delay in responding, mostly in read-only mode this
>week :)
>
>* Felipe Balbi <balbi@ti.com> [101227 09:17]:
>> >
>> >What was the problem you saw with N810 which caused the need for
>> >commit fc8c2f1dba400e9a0d1d82756e0e2f52dfe2f4b6, quoted below ?
>
>Heh found it after some debugging, we need to call init_completion :)

Cool, good one. I'll start working on moving to genirq, already have a
few patches on my cbus branch.

-- 
balbi