Messages in this thread | | | From | xt.hu[胡先韬] <> | Subject | RE: [PATCH v2 1/2] watchdog: Add watchdog driver for Sunplus SP7021 | Date | Thu, 25 Nov 2021 02:42:33 +0000 |
| |
Hi Thanks for your review. I explain this in detail below the comment.
Best Regards, Xiantao > -----Original Message----- > From: Guenter Roeck [mailto:groeck7@gmail.com] On Behalf Of Guenter Roeck > Sent: Wednesday, November 24, 2021 10:25 PM > To: xt.hu[胡先韬] <xt.hu@cqplus1.com> > Cc: wim@linux-watchdog.org; p.zabel@pengutronix.de; linux-kernel@vger.kernel.org; > linux-watchdog@vger.kernel.org; robh+dt@kernel.org; devicetree@vger.kernel.org; Wells Lu 呂芳騰 > <wells.lu@sunplus.com>; qinjian[覃健] <qinjian@cqplus1.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@cqplus1.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
| |