Received-SPF: pass (google.com: domain of linux-kernel+bounces-160184-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Precedence: bulk
MIME-Version: 1.0
References: <20240425090727.3787160-1-peteryin.openbmc@gmail.com>
 <20240425090727.3787160-2-peteryin.openbmc@gmail.com> <5ed8bfd6ce559a7d00ec89f1bd7ae1aea90e3ae7.camel@codeconstruct.com.au>
In-Reply-To: <5ed8bfd6ce559a7d00ec89f1bd7ae1aea90e3ae7.camel@codeconstruct.com.au>
From: Chia Hsing Yin <peteryin.openbmc@gmail.com>
Date: Fri, 26 Apr 2024 22:45:32 +0800
Message-ID: <CAPSyxFTa6k3FzUmDoZ6HMpAfwZGK_yPrQKGSvSdtj_JqsnMVrA@mail.gmail.com>
Subject: Re: [PATCH v7 1/1] drivers: watchdog: revise watchdog bootstatus
To: Andrew Jeffery <andrew@codeconstruct.com.au>
Cc: patrick@stwcx.xyz, Wim Van Sebroeck <wim@linux-watchdog.org>, 
	Guenter Roeck <linux@roeck-us.net>, Joel Stanley <joel@jms.id.au>, linux-watchdog@vger.kernel.org, 
	linux-arm-kernel@lists.infradead.org, linux-aspeed@lists.ozlabs.org, 
	linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

I can include reset condition in struct maybe like this

static const struct aspeed_wdt_config ast2600_config =3D {
ext_pulse_width_mask =3D 0xfffff,
irq_shift =3D 0,
irq_mask =3D GENMASK(31, 10),
compatible =3D "aspeed,ast2600-scu",
reset_event =3D AST2600_SYSTEM_RESET_EVENT,
watchdog_reset_flag =3D AST2600_WATCHDOG_RESET_FLAG,
extern_reset_flag =3D EXTERN_RESET_FLAG,
reset_flag_clear =3D AST2600_RESET_FLAG_CLEAR,
};

in probe( ) we  just call

scu_base =3D syscon_regmap_lookup_by_compatible(wdt->cfg->compatible);
if (IS_ERR(scu_base))
    return PTR_ERR(scu_base);

ret =3D regmap_read(scu_base, wdt->cfg->reset_event, &status);
if (ret)
    return ret;

if ((status & POWERON_RESET_FLAG) =3D=3D 0 &&
     status & wdt->cfg->watchdog_reset_flag)
    wdt->wdd.bootstatus =3D (status & wdt->cfg->extern_reset_flag) ?
    WDIOF_EXTERN1 : WDIOF_CARDRESET;

status =3D wdt->cfg->watchdog_reset_flag | POWERON_RESET_FLAG |
wdt->cfg->extern_reset_flag;

ret =3D regmap_write(scu_base, wdt->cfg->reset_event, status);

Does this meet your expectations?

On Fri, Apr 26, 2024 at 8:42=E2=80=AFAM Andrew Jeffery
<andrew@codeconstruct.com.au> wrote:
>
> On Thu, 2024-04-25 at 17:07 +0800, Peter Yin wrote:
> > Regarding the AST2600 specification, the WDTn Timeout Status Register
> > (WDT10) has bit 1 reserved. Bit 1 of the status register indicates
> > on ast2500 if the boot was from the second boot source.
> > It does not indicate that the most recent reset was triggered by
> > the watchdog. The code should just be changed to set WDIOF_CARDRESET
> > if bit 0 of the status register is set. However, this bit can be clear =
when
> > watchdog register 0x0c bit1(Reset System after timeout) is enabled.
> > Thereforce include SCU register to veriy WDIOF_EXTERN1 and WDIOF_CARDRE=
SET
> > in ast2600 SCU74 or ast2400/ast2500 SCU3C.
> >
> > Signed-off-by: Peter Yin <peteryin.openbmc@gmail.com>
> > ---
> >  drivers/watchdog/aspeed_wdt.c | 109 ++++++++++++++++++++++++++++++++--
> >  1 file changed, 103 insertions(+), 6 deletions(-)
>
> After this patch the probe() implementation is ~250loc with a whole
> bunch of conditional behaviours based on the SoC version. Maybe it's
> time to break it up into version-specific functions that are called
> from the probe() implementation?
>
> Andrew