All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com>
To: "linux@roeck-us.net" <linux@roeck-us.net>
Cc: "mazziesaccount@gmail.com" <mazziesaccount@gmail.com>,
	"wim@linux-watchdog.org" <wim@linux-watchdog.org>,
	linux-power <linux-power@fi.rohmeurope.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-watchdog@vger.kernel.org" <linux-watchdog@vger.kernel.org>
Subject: Re: [PATCH 1/2] watchdog: bd70528: don't crash if WDG is confiured with BD71828
Date: Fri, 8 Jan 2021 06:10:48 +0000	[thread overview]
Message-ID: <9d3c8f47716dd0b2ff27ffa0192b4d4d214f7339.camel@fi.rohmeurope.com> (raw)
In-Reply-To: <35ba5bcd48d4e027c0e5c839a856751519ee4dd1.camel@fi.rohmeurope.com>


On Thu, 2021-01-07 at 20:15 +0200, Matti Vaittinen wrote:
> Thanks a lot for taking a careful look at this Guenter!
> 
> On Thu, 2021-01-07 at 07:12 -0800, Guenter Roeck wrote:
> > On Thu, Jan 07, 2021 at 08:37:03AM +0200, Matti Vaittinen wrote:
> > > If config for BD70528 watchdog is enabled when BD71828 or BD71815
> > > are used the RTC module will issue call to BD70528 watchdog with
> > > NULL data. Ignore this call and don't crash.
> > > 
> > > Signed-off-by: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com
> > > >
> > 
> > I really think this should be handled in the calling code.
> > Also, I am curious how this is supposed to work.
> > 
> > The code is called with
> > 
> > 	ret = bd70528_wdt_set(r->parent, new_state &
> > BD70528_WDT_STATE_BIT,
> >                               old_state);
> 
> My brainfart.
> The bd70528_wdt_set is not called as it is protected in RTC by
> has_rtc_timers flag.
> 
> I inserted this check in wrong function. The bd70528_wdt_lock()
> is where we may hit the problem as it is not protected.

Actually, after a fresh look - it seems the bd70528_wdt_lock() is also
just fine. The RTC is not grabbing the lock on other PMICs but the
BD70528. I'm not really sure what I have been thinking of. @_@ I
should've been more careful. Thanks for spotting this in the review!

Best Regards
	Matti

      reply	other threads:[~2021-01-08  6:11 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-07  6:37 [PATCH 1/2] watchdog: bd70528: don't crash if WDG is confiured with BD71828 Matti Vaittinen
2021-01-07  6:37 ` [PATCH 2/2] watchdog: BD70528: conditionally allow BD70528 module Matti Vaittinen
2021-01-07 15:12   ` Guenter Roeck
2021-01-08  6:34     ` Matti Vaittinen
2021-01-07 15:12 ` [PATCH 1/2] watchdog: bd70528: don't crash if WDG is confiured with BD71828 Guenter Roeck
2021-01-07 18:15   ` Vaittinen, Matti
2021-01-08  6:10     ` Vaittinen, Matti [this message]

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=9d3c8f47716dd0b2ff27ffa0192b4d4d214f7339.camel@fi.rohmeurope.com \
    --to=matti.vaittinen@fi.rohmeurope.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-power@fi.rohmeurope.com \
    --cc=linux-watchdog@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=mazziesaccount@gmail.com \
    --cc=wim@linux-watchdog.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.