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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Wed, 21 Feb 2024 13:00:38 +0100
From: Philipp Zabel <p.zabel@...gutronix.de>
To: Yang Xiwen <forbidden405@...look.com>, Christophe JAILLET
	 <christophe.jaillet@...adoo.fr>
Cc: conor+dt@...nel.org, devicetree@...r.kernel.org, 
	krzysztof.kozlowski+dt@...aro.org, linux-kernel@...r.kernel.org, 
	linux-watchdog@...r.kernel.org, linux@...ck-us.net, robh+dt@...nel.org, 
	vireshk@...nel.org, wim@...ux-watchdog.org
Subject: Re: [PATCH v2 1/2] watchdog: sp805_wdt: deassert the reset if
 available

On Di, 2024-02-20 at 03:00 +0800, Yang Xiwen wrote:
> On 2/20/2024 2:22 AM, Christophe JAILLET wrote:
> > Le 19/02/2024 à 19:14, Yang Xiwen via B4 Relay a écrit :
> > > From: Yang Xiwen <forbidden405-1ViLX0X+lBJBDgjK7y7TUQ@...lic.gmane.org>
> > > 
> > > According to the datasheet, the core has an WDOGRESn input signal that
> > > needs to be deasserted before being operational. Implement it in the
> > > driver.
> > > 
> > > Signed-off-by: Yang Xiwen 
> > > <forbidden405-1ViLX0X+lBJBDgjK7y7TUQ@...lic.gmane.org>
> > > ---
> > >   drivers/watchdog/sp805_wdt.c | 9 +++++++++
> > >   1 file changed, 9 insertions(+)
> > > 
> > > diff --git a/drivers/watchdog/sp805_wdt.c b/drivers/watchdog/sp805_wdt.c
> > > index 2756ed54ca3d..b4bcfdeb39e6 100644
> > > --- a/drivers/watchdog/sp805_wdt.c
> > > +++ b/drivers/watchdog/sp805_wdt.c
> > > @@ -25,6 +25,7 @@
> > >   #include <linux/moduleparam.h>
> > >   #include <linux/pm.h>
> > >   #include <linux/property.h>
> > > +#include <linux/reset.h>
> > >   #include <linux/slab.h>
> > >   #include <linux/spinlock.h>
> > >   #include <linux/types.h>
> > > @@ -59,6 +60,7 @@
> > >    * @lock: spin lock protecting dev structure and io access
> > >    * @base: base address of wdt
> > >    * @clk: (optional) clock structure of wdt
> > > + * @rst: (optional) reset control signal of wdt
> > >    * @rate: (optional) clock rate when provided via properties
> > >    * @adev: amba device structure of wdt
> > >    * @status: current status of wdt
> > > @@ -69,6 +71,7 @@ struct sp805_wdt {
> > >       spinlock_t            lock;
> > >       void __iomem            *base;
> > >       struct clk            *clk;
> > > +    struct reset_control        *rst;
> > >       u64                rate;
> > >       struct amba_device        *adev;
> > >       unsigned int            load_val;
> > > @@ -264,6 +267,12 @@ sp805_wdt_probe(struct amba_device *adev, const 
> > > struct amba_id *id)
> > >           return -ENODEV;
> > >       }
> > >   +    wdt->rst = devm_reset_control_get_optional(&adev->dev, NULL);
> > > +    if (IS_ERR(wdt->rst))
> > > +        return dev_err_probe(&adev->dev, PTR_ERR(wdt->rst), "Can not 
> > > get reset\n");
> > > +
> > > +    reset_control_deassert(wdt->rst);
> > > +
> > 
> > Hi,
> > 
> > Is a corresponding reset_control_assert() needed in the remove function?
> 
> I don't think it makes much sense. Many drivers in kernel does not 
> reassert the resets in their driver's remove callback too.

In general, balancing deassert/assert is only required for shared reset
controls.

For exclusive reset controls, it depends on whether the driver has any
expectations that the reset was asserted at some point before deassert
is called, possibly to reset registers or some internal state.
E.g., if this driver is unbound and then bound again, the deassert
during second probe is a no-op.

regards
Philipp

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ