All of lore.kernel.org
 help / color / mirror / Atom feed
From: Zumeng Chen <zumeng.chen@windriver.com>
To: Shubhrajyoti <shubhrajyoti@ti.com>
Cc: "zumeng.chen" <zchen@windriver.com>, <wim@iguana.be>,
	<linux-watchdog@vger.kernel.org>,
	<linux-arm-kernel@lists.infradead.org>,
	<linux-omap@vger.kernel.org>
Subject: Re: [PATCH 1/1] Watchdog: OMAP3: fix wrong boot status from wdt reboot
Date: Thu, 5 Jul 2012 14:00:34 +0800	[thread overview]
Message-ID: <4FF52D82.6030107@windriver.com> (raw)
In-Reply-To: <4FF52893.30406@ti.com>

于 2012年07月05日 13:39, Shubhrajyoti 写道:
> Hello,
> On Wednesday 04 July 2012 09:27 PM, zumeng.chen wrote:
>> To: Shubhrajyoti
>> On 2012年07月04日 23:54, Zumeng Chen wrote:
>>> Does the following fix make sense?
> yes , thanks for the patch.
> IIRC Rajendra had a similar one.
> Some comments below.
>>> WDIOC_GETBOOTSTATUS always return 0 even if the machine
>>> comes from omap-wdt reboot. Because WKUP_MOD is not right
>>> for OMAP3,so give the right addr 0xA00 of PRM_RSTST for
>>> get_reset_sources, which inputs the signal from omap-wdt
>>> reboot, and return 1 when coming from omap-wdt reboot for
>>> WDIOC_GETBOOTSTATUS.
>>>
>>> Signed-off-by: Zumeng Chen<zumeng.chen@windriver.com>
>>> ---
>>>    arch/arm/mach-omap2/prcm.c  |    4 +++-
>>>    drivers/watchdog/omap_wdt.c |    3 +++
>>>    drivers/watchdog/omap_wdt.h |    3 +++
>>>    3 files changed, 9 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/prcm.c b/arch/arm/mach-omap2/prcm.c
>>> index 480f40a..43f3feb 100644
>>> --- a/arch/arm/mach-omap2/prcm.c
>>> +++ b/arch/arm/mach-omap2/prcm.c
>>> @@ -49,8 +49,10 @@ void __iomem *prcm_mpu_base;
>>>    u32 omap_prcm_get_reset_sources(void)
>>>    {
>>>        /* XXX This presumably needs modification for 34XX */
>>> -    if (cpu_is_omap24xx() || cpu_is_omap34xx())
>>> +    if (cpu_is_omap24xx())
>>>            return omap2_prm_read_mod_reg(WKUP_MOD, OMAP2_RM_RSTST)&
>>> 0x7f;
>>> +    if (cpu_is_omap34xx())
>>> +        return omap2_prm_read_mod_reg(0xA00, OMAP2_RM_RSTST)&   0x7f;
>>>        if (cpu_is_omap44xx())
>>>            return omap2_prm_read_mod_reg(WKUP_MOD, OMAP4_RM_RSTST)&
>>> 0x7f;
>>>
>>> diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
>>> index 8285d65..ea57078 100644
>>> --- a/drivers/watchdog/omap_wdt.c
>>> +++ b/drivers/watchdog/omap_wdt.c
>>> @@ -234,6 +234,9 @@ static long omap_wdt_ioctl(struct file *file,
>>> unsigned int cmd,
>>>            if (cpu_is_omap24xx())
>>>                return put_user(omap_prcm_get_reset_sources(),
>>>                        (int __user *)arg);
>>> +        if (cpu_is_omap34xx())
>>> +            return put_user(omap_prcm_get_reset_sources()&   0x10>>
>>> +                OMAP3_PRM_RSTST_BIT, (int __user *)arg);
> Actually instead of returning yes/no.
> The correct  expectation is to return WDIOF_* flags as defined in
> include/linux/watchdog.h.
Yes, return WDIOF_CARDRESET is more general, thanks.
>
> (BTW I agree that was something even current code is not
> following).Since you are at it may be that’s
> something you can consider.
Yes, I just find them, I'll try it to get it more following, like 
get_status.

Regards,
Zumeng
>
>>>            return put_user(0, (int __user *)arg);
>>>        case WDIOC_KEEPALIVE:
>>>            pm_runtime_get_sync(wdev->dev);
>>> diff --git a/drivers/watchdog/omap_wdt.h b/drivers/watchdog/omap_wdt.h
>>> index 09b774c..d8d5daa 100644
>>> --- a/drivers/watchdog/omap_wdt.h
>>> +++ b/drivers/watchdog/omap_wdt.h
>>> @@ -40,6 +40,9 @@
>>>    #define OMAP_WATCHDOG_WPS        (0x34)
>>>    #define OMAP_WATCHDOG_SPR        (0x48)
>>>
>>> +/* PRM_RSTST MPU_WD_RST bit */
>>> +#define OMAP3_PRM_RSTST_BIT        4
>>> +
>>>    /* Using the prescaler, the OMAP watchdog could go for many
>>>     * months before firing.  These limits work without scaling,
>>>     * with the 60 second default assumed by most tools and docs.

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

WARNING: multiple messages have this Message-ID (diff)
From: Zumeng Chen <zumeng.chen@windriver.com>
To: Shubhrajyoti <shubhrajyoti@ti.com>
Cc: "zumeng.chen" <zchen@windriver.com>,
	wim@iguana.be, linux-watchdog@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, linux-omap@vger.kernel.org
Subject: Re: [PATCH 1/1] Watchdog: OMAP3: fix wrong boot status from wdt reboot
Date: Thu, 5 Jul 2012 14:00:34 +0800	[thread overview]
Message-ID: <4FF52D82.6030107@windriver.com> (raw)
In-Reply-To: <4FF52893.30406@ti.com>

于 2012年07月05日 13:39, Shubhrajyoti 写道:
> Hello,
> On Wednesday 04 July 2012 09:27 PM, zumeng.chen wrote:
>> To: Shubhrajyoti
>> On 2012年07月04日 23:54, Zumeng Chen wrote:
>>> Does the following fix make sense?
> yes , thanks for the patch.
> IIRC Rajendra had a similar one.
> Some comments below.
>>> WDIOC_GETBOOTSTATUS always return 0 even if the machine
>>> comes from omap-wdt reboot. Because WKUP_MOD is not right
>>> for OMAP3,so give the right addr 0xA00 of PRM_RSTST for
>>> get_reset_sources, which inputs the signal from omap-wdt
>>> reboot, and return 1 when coming from omap-wdt reboot for
>>> WDIOC_GETBOOTSTATUS.
>>>
>>> Signed-off-by: Zumeng Chen<zumeng.chen@windriver.com>
>>> ---
>>>    arch/arm/mach-omap2/prcm.c  |    4 +++-
>>>    drivers/watchdog/omap_wdt.c |    3 +++
>>>    drivers/watchdog/omap_wdt.h |    3 +++
>>>    3 files changed, 9 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/prcm.c b/arch/arm/mach-omap2/prcm.c
>>> index 480f40a..43f3feb 100644
>>> --- a/arch/arm/mach-omap2/prcm.c
>>> +++ b/arch/arm/mach-omap2/prcm.c
>>> @@ -49,8 +49,10 @@ void __iomem *prcm_mpu_base;
>>>    u32 omap_prcm_get_reset_sources(void)
>>>    {
>>>        /* XXX This presumably needs modification for 34XX */
>>> -    if (cpu_is_omap24xx() || cpu_is_omap34xx())
>>> +    if (cpu_is_omap24xx())
>>>            return omap2_prm_read_mod_reg(WKUP_MOD, OMAP2_RM_RSTST)&
>>> 0x7f;
>>> +    if (cpu_is_omap34xx())
>>> +        return omap2_prm_read_mod_reg(0xA00, OMAP2_RM_RSTST)&   0x7f;
>>>        if (cpu_is_omap44xx())
>>>            return omap2_prm_read_mod_reg(WKUP_MOD, OMAP4_RM_RSTST)&
>>> 0x7f;
>>>
>>> diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
>>> index 8285d65..ea57078 100644
>>> --- a/drivers/watchdog/omap_wdt.c
>>> +++ b/drivers/watchdog/omap_wdt.c
>>> @@ -234,6 +234,9 @@ static long omap_wdt_ioctl(struct file *file,
>>> unsigned int cmd,
>>>            if (cpu_is_omap24xx())
>>>                return put_user(omap_prcm_get_reset_sources(),
>>>                        (int __user *)arg);
>>> +        if (cpu_is_omap34xx())
>>> +            return put_user(omap_prcm_get_reset_sources()&   0x10>>
>>> +                OMAP3_PRM_RSTST_BIT, (int __user *)arg);
> Actually instead of returning yes/no.
> The correct  expectation is to return WDIOF_* flags as defined in
> include/linux/watchdog.h.
Yes, return WDIOF_CARDRESET is more general, thanks.
>
> (BTW I agree that was something even current code is not
> following).Since you are at it may be that’s
> something you can consider.
Yes, I just find them, I'll try it to get it more following, like 
get_status.

Regards,
Zumeng
>
>>>            return put_user(0, (int __user *)arg);
>>>        case WDIOC_KEEPALIVE:
>>>            pm_runtime_get_sync(wdev->dev);
>>> diff --git a/drivers/watchdog/omap_wdt.h b/drivers/watchdog/omap_wdt.h
>>> index 09b774c..d8d5daa 100644
>>> --- a/drivers/watchdog/omap_wdt.h
>>> +++ b/drivers/watchdog/omap_wdt.h
>>> @@ -40,6 +40,9 @@
>>>    #define OMAP_WATCHDOG_WPS        (0x34)
>>>    #define OMAP_WATCHDOG_SPR        (0x48)
>>>
>>> +/* PRM_RSTST MPU_WD_RST bit */
>>> +#define OMAP3_PRM_RSTST_BIT        4
>>> +
>>>    /* Using the prescaler, the OMAP watchdog could go for many
>>>     * months before firing.  These limits work without scaling,
>>>     * with the 60 second default assumed by most tools and docs.

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

WARNING: multiple messages have this Message-ID (diff)
From: zumeng.chen@windriver.com (Zumeng Chen)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/1] Watchdog: OMAP3: fix wrong boot status from wdt reboot
Date: Thu, 5 Jul 2012 14:00:34 +0800	[thread overview]
Message-ID: <4FF52D82.6030107@windriver.com> (raw)
In-Reply-To: <4FF52893.30406@ti.com>

? 2012?07?05? 13:39, Shubhrajyoti ??:
> Hello,
> On Wednesday 04 July 2012 09:27 PM, zumeng.chen wrote:
>> To: Shubhrajyoti
>> On 2012?07?04? 23:54, Zumeng Chen wrote:
>>> Does the following fix make sense?
> yes , thanks for the patch.
> IIRC Rajendra had a similar one.
> Some comments below.
>>> WDIOC_GETBOOTSTATUS always return 0 even if the machine
>>> comes from omap-wdt reboot. Because WKUP_MOD is not right
>>> for OMAP3,so give the right addr 0xA00 of PRM_RSTST for
>>> get_reset_sources, which inputs the signal from omap-wdt
>>> reboot, and return 1 when coming from omap-wdt reboot for
>>> WDIOC_GETBOOTSTATUS.
>>>
>>> Signed-off-by: Zumeng Chen<zumeng.chen@windriver.com>
>>> ---
>>>    arch/arm/mach-omap2/prcm.c  |    4 +++-
>>>    drivers/watchdog/omap_wdt.c |    3 +++
>>>    drivers/watchdog/omap_wdt.h |    3 +++
>>>    3 files changed, 9 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/prcm.c b/arch/arm/mach-omap2/prcm.c
>>> index 480f40a..43f3feb 100644
>>> --- a/arch/arm/mach-omap2/prcm.c
>>> +++ b/arch/arm/mach-omap2/prcm.c
>>> @@ -49,8 +49,10 @@ void __iomem *prcm_mpu_base;
>>>    u32 omap_prcm_get_reset_sources(void)
>>>    {
>>>        /* XXX This presumably needs modification for 34XX */
>>> -    if (cpu_is_omap24xx() || cpu_is_omap34xx())
>>> +    if (cpu_is_omap24xx())
>>>            return omap2_prm_read_mod_reg(WKUP_MOD, OMAP2_RM_RSTST)&
>>> 0x7f;
>>> +    if (cpu_is_omap34xx())
>>> +        return omap2_prm_read_mod_reg(0xA00, OMAP2_RM_RSTST)&   0x7f;
>>>        if (cpu_is_omap44xx())
>>>            return omap2_prm_read_mod_reg(WKUP_MOD, OMAP4_RM_RSTST)&
>>> 0x7f;
>>>
>>> diff --git a/drivers/watchdog/omap_wdt.c b/drivers/watchdog/omap_wdt.c
>>> index 8285d65..ea57078 100644
>>> --- a/drivers/watchdog/omap_wdt.c
>>> +++ b/drivers/watchdog/omap_wdt.c
>>> @@ -234,6 +234,9 @@ static long omap_wdt_ioctl(struct file *file,
>>> unsigned int cmd,
>>>            if (cpu_is_omap24xx())
>>>                return put_user(omap_prcm_get_reset_sources(),
>>>                        (int __user *)arg);
>>> +        if (cpu_is_omap34xx())
>>> +            return put_user(omap_prcm_get_reset_sources()&   0x10>>
>>> +                OMAP3_PRM_RSTST_BIT, (int __user *)arg);
> Actually instead of returning yes/no.
> The correct  expectation is to return WDIOF_* flags as defined in
> include/linux/watchdog.h.
Yes, return WDIOF_CARDRESET is more general, thanks.
>
> (BTW I agree that was something even current code is not
> following).Since you are at it may be that?s
> something you can consider.
Yes, I just find them, I'll try it to get it more following, like 
get_status.

Regards,
Zumeng
>
>>>            return put_user(0, (int __user *)arg);
>>>        case WDIOC_KEEPALIVE:
>>>            pm_runtime_get_sync(wdev->dev);
>>> diff --git a/drivers/watchdog/omap_wdt.h b/drivers/watchdog/omap_wdt.h
>>> index 09b774c..d8d5daa 100644
>>> --- a/drivers/watchdog/omap_wdt.h
>>> +++ b/drivers/watchdog/omap_wdt.h
>>> @@ -40,6 +40,9 @@
>>>    #define OMAP_WATCHDOG_WPS        (0x34)
>>>    #define OMAP_WATCHDOG_SPR        (0x48)
>>>
>>> +/* PRM_RSTST MPU_WD_RST bit */
>>> +#define OMAP3_PRM_RSTST_BIT        4
>>> +
>>>    /* Using the prescaler, the OMAP watchdog could go for many
>>>     * months before firing.  These limits work without scaling,
>>>     * with the 60 second default assumed by most tools and docs.

  reply	other threads:[~2012-07-05  6:03 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-07-04 15:54 [PATCH 1/1] Watchdog: OMAP3: fix wrong boot status from wdt reboot Zumeng Chen
2012-07-04 15:54 ` Zumeng Chen
2012-07-04 15:54 ` Zumeng Chen
2012-07-04 15:57 ` zumeng.chen
2012-07-04 15:57   ` zumeng.chen
2012-07-04 15:57   ` zumeng.chen
2012-07-05  5:39   ` Shubhrajyoti
2012-07-05  5:39     ` Shubhrajyoti
2012-07-05  5:39     ` Shubhrajyoti
2012-07-05  6:00     ` Zumeng Chen [this message]
2012-07-05  6:00       ` Zumeng Chen
2012-07-05  6:00       ` Zumeng Chen
2012-07-05 13:05 ` Bedia, Vaibhav
2012-07-05 13:05   ` Bedia, Vaibhav
2012-07-05 13:05   ` Bedia, Vaibhav
2012-07-05 15:03   ` zumeng.chen
2012-07-05 15:03     ` zumeng.chen
2012-07-05 15:03     ` zumeng.chen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4FF52D82.6030107@windriver.com \
    --to=zumeng.chen@windriver.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=shubhrajyoti@ti.com \
    --cc=wim@iguana.be \
    --cc=zchen@windriver.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.