linux-kernel - RE: [PATCH v2 1/2] watchdog: Add watchdog driver for Sunplus SP7021

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <0024d27919c04c84a3f13ecba86a3c70@cqplus1.com>
Date:   Thu, 25 Nov 2021 02:42:33 +0000
From:   xt.hu[胡先韬] <xt.hu@...lus1.com>
To:     Guenter Roeck <linux@...ck-us.net>
CC:     "wim@...ux-watchdog.org" <wim@...ux-watchdog.org>,
        "p.zabel@...gutronix.de" <p.zabel@...gutronix.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-watchdog@...r.kernel.org" <linux-watchdog@...r.kernel.org>,
        "robh+dt@...nel.org" <robh+dt@...nel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        Wells Lu 呂芳騰 <wells.lu@...plus.com>,
        qinjian[覃健] <qinjian@...lus1.com>
Subject: RE: [PATCH v2 1/2] watchdog: Add watchdog driver for Sunplus SP7021

Hi
	Thanks for your review. I explain this in detail below the comment.

Best Regards,
Xiantao
> -----Original Message-----
> From: Guenter Roeck [mailto:groeck7@...il.com] On Behalf Of Guenter Roeck
> Sent: Wednesday, November 24, 2021 10:25 PM
> To: xt.hu[胡先韬] <xt.hu@...lus1.com>
> Cc: wim@...ux-watchdog.org; p.zabel@...gutronix.de; linux-kernel@...r.kernel.org;
> linux-watchdog@...r.kernel.org; robh+dt@...nel.org; devicetree@...r.kernel.org; Wells Lu 呂芳騰
> <wells.lu@...plus.com>; qinjian[覃健] <qinjian@...lus1.com>
> Subject: Re: [PATCH v2 1/2] watchdog: Add watchdog driver for Sunplus SP7021
> 
> On Wed, Nov 24, 2021 at 06:41:48PM +0800, Xiantao Hu wrote:
> > Sunplus SP7021 requires watchdog timer support.
> > Add watchdog driver to enable this.
> >
> > Signed-off-by: Xiantao Hu <xt.hu@...lus1.com>
> > ---
> > +
> > +	priv->base = devm_platform_ioremap_resource(pdev, 0);
> > +	if (IS_ERR(priv->base))
> > +		return PTR_ERR(priv->base);
> > +
> > +	/* The registers accessed here shared by multiple drivers. */
> > +	wdt_res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> 
> This is unusual. Why would other drivers access WDT_CTRL and WDT_CNT registers, and how is it
> ensured that the other drivers do not interfer with the accesses by this driver ?
> 
> Normally such a resource would be shared through a parent driver with appropriate access functions to
> ensure that accesses are synchronized.
> 

The register used by this driver consists of two parts. The first part which contains WDT_CTRL and WDT_CNT 
registers is exclusive by watchdog. 
In specially, the second part is belong to a multifunctional register group which control IP and bus. Refer to 
register manual below:
-------------------------------------------------------------------------------------------------------------------------------------------------
MO1_STC_WDG_RST_EN	4	RW		STC Watchdog Timeout Trigger System Reset Enable
									0: STC watchdog 2 timeout will not trigger system reset(default)
									1: STC watchdog 2 timeout will trigger system reset
MO1_RI_WDG_RST_EN		1	RW		RBUS Watchdog Timeout Trigger System Reset Enable
									0: RBUS watchdog timeout will not trigger system reset(default)
									1: RBUS watchdog timeout will trigger system reset
MO1_TIMER_STAND_BY_EN	0	RW		Timer Standby Mode Enable
									0: Disable (default)
									1: Enable Active high to enter timer standby mode, 
											default not in standby mode
-------------------------------------------------------------------------------------------------------------------------------------------------
You can see that in addition to the bits for watchdog there are bit fields for other modules.
I use this register bit4 and bit1. Default value is 0 that watchdog internal interrupt signal can't trigger system 
and RBUS reset. I need set 1 when watchdog probe. Early I implement the operation in 
arch/arm/mach-sunplus/sp7021.c and configure by macro. But in arch/arm64, directory mach-XXX is removed.
So I solve in this way. Any better way?

> Thanks,
> Guenter