mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Sascha Hauer <s.hauer@pengutronix.de>
To: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Barebox List <barebox@lists.infradead.org>
Subject: Re: [PATCH 03/10] ARM: i.MX6: Record reset reason as a part of startup
Date: Wed, 18 Apr 2018 10:23:21 +0200	[thread overview]
Message-ID: <20180418082321.gvm5mhznphwqgl7y@pengutronix.de> (raw)
In-Reply-To: <CAHQ1cqE+jzfOoFNkXpUX_eqjN=whM9EGYFA1OYo59gFbDiSYWQ@mail.gmail.com>

On Tue, Apr 17, 2018 at 08:35:12AM -0700, Andrey Smirnov wrote:
> On Mon, Apr 16, 2018 at 11:49 PM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > On Mon, Apr 16, 2018 at 06:28:11AM -0700, Andrey Smirnov wrote:
> >> On Mon, Apr 16, 2018 at 12:13 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >> >> @@ -151,6 +152,7 @@ int imx6_init(void)
> >> >>  {
> >> >>       const char *cputypestr;
> >> >>       u32 mx6_silicon_revision;
> >> >> +     void __iomem *src = IOMEM(MX6_SRC_BASE_ADDR);
> >> >>
> >> >>       imx6_init_lowlevel();
> >> >>
> >> >> @@ -195,7 +197,7 @@ int imx6_init(void)
> >> >>       }
> >> >>
> >> >>       imx_set_silicon_revision(cputypestr, mx6_silicon_revision);
> >> >> -
> >> >> +     imx_set_reset_reason(src + IMX6_SRC_SRSR);
> >> >
> >> > This will get overwritten by the watchdog driver if enabled.
> >> >
> >>
> >> I am not sure I see how. Imx_watchdog_detect_reset_source() reports
> >> reset sources with the same priority as this code, so
> >> reset_source_set_priority() should bail out early without changing
> >> anything in that case.
> >
> > I wasn't aware there is a priority mechanism involved. Indeed the
> > behaviour seems to be correct. Maybe we should even higher the priority
> > of imx_set_reset_reason()? This information seems the more accurate one,
> > so it should be used, and we shouldn't depend on the order of execution.
> >
> 
> I think if I add support for "reset-source-priority" to imxwd.c and
> set it up to use the value less then RESET_SOURCE_DEFAULT_PRIORITY by
> default we can get both what you describe and allow users to override
> the behavior and prioritize reset source provided by watchdog driver.
> 
> Let me know if this sounds like a bad idea.

It doesn't sound like a bad idea, I'm just not convinced it's a good
idea ;)

Reading the SRC registers seems to give us more accurate informations
than reading the watchdog. Provided this is true I don't see a reason to
make the priority configurable. If it was only for i.MX6/7 I would say,
we could remove the reset source detection from the watchdog driver
entirely, but we need the informations from the driver on older SoCs.

Anyway, before making this configurable I think it's better to wait
until somebody to show up who really needs this (and thus can explain
the reasons), rather than to just add it in case it might be useful.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

  reply	other threads:[~2018-04-18  8:23 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-14 17:50 [PATCH 00/10] i.MX reset reason detection support Andrey Smirnov
2018-04-14 17:50 ` [PATCH 01/10] common: reset_source: Add the notion of "reset source instance" Andrey Smirnov
2018-04-14 17:50 ` [PATCH 02/10] ARM: i.MX: Add infrastructure to record SoC reset reason Andrey Smirnov
2018-04-18 15:23   ` Philipp Zabel
2018-04-19 22:19     ` Andrey Smirnov
2018-04-14 17:50 ` [PATCH 03/10] ARM: i.MX6: Record reset reason as a part of startup Andrey Smirnov
2018-04-16  7:13   ` Sascha Hauer
2018-04-16 13:28     ` Andrey Smirnov
2018-04-17  6:49       ` Sascha Hauer
2018-04-17 15:35         ` Andrey Smirnov
2018-04-18  8:23           ` Sascha Hauer [this message]
2018-04-14 17:50 ` [PATCH 04/10] ARM: i.MX7: " Andrey Smirnov
2018-04-16  7:13   ` Sascha Hauer
2018-04-14 17:50 ` [PATCH 05/10] common: reset_source: Introduce reset_source_name() Andrey Smirnov
2018-04-14 17:50 ` [PATCH 06/10] ARM: i.MX: Log detected reset reason Andrey Smirnov
2018-04-14 17:50 ` [PATCH 07/10] ARM: i.MX: Print "revision unknown" if that is the case Andrey Smirnov
2018-04-14 17:50 ` [PATCH 08/10] ARM: VFxxx: Add code to detect reset reason Andrey Smirnov
2018-04-16  7:19   ` Sascha Hauer
2018-04-16 13:12     ` Andrey Smirnov
2018-04-14 17:50 ` [PATCH 09/10] ARM: VFxxx: Add code to detect cpu variant Andrey Smirnov
2018-04-14 17:50 ` [PATCH 10/10] ARM: VFxxx: Detect cpu variant and reset source on start Andrey Smirnov

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=20180418082321.gvm5mhznphwqgl7y@pengutronix.de \
    --to=s.hauer@pengutronix.de \
    --cc=andrew.smirnov@gmail.com \
    --cc=barebox@lists.infradead.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox