From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755461AbcFHGy4 (ORCPT ); Wed, 8 Jun 2016 02:54:56 -0400 Received: from sauhun.de ([89.238.76.85]:56233 "EHLO pokefinder.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753516AbcFHGyy (ORCPT ); Wed, 8 Jun 2016 02:54:54 -0400 Date: Wed, 8 Jun 2016 08:54:45 +0200 From: Wolfram Sang To: Vladimir Zapolskiy Cc: Wim Van Sebroeck , Guenter Roeck , Robin Gong , linux-watchdog@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v3 4/6] watchdog: add watchdog pretimeout framework Message-ID: <20160608065445.GB1521@katana> References: <1465321127-19522-1-git-send-email-vladimir_zapolskiy@mentor.com> <1465321127-19522-5-git-send-email-vladimir_zapolskiy@mentor.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="3lcZGd9BuhuYXNfi" Content-Disposition: inline In-Reply-To: <1465321127-19522-5-git-send-email-vladimir_zapolskiy@mentor.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --3lcZGd9BuhuYXNfi Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Jun 07, 2016 at 08:38:45PM +0300, Vladimir Zapolskiy wrote: > The change adds a simple watchdog pretimeout framework infrastructure, > its purpose is to allow users to select a desired handling of watchdog > pretimeout events, which may be generated by some watchdog devices. >=20 > A user selects a default watchdog pretimeout governor during > compilation stage. >=20 > Watchdogs with WDIOF_PRETIMEOUT capability now have two device > attributes in sysfs: pretimeout to display currently set pretimeout > value and pretimeout_governor attribute to display the selected > watchdog pretimeout governor. >=20 > Watchdogs with no WDIOF_PRETIMEOUT capability has no changes in > sysfs, and such watchdog devices do not require the framework. >=20 > Signed-off-by: Vladimir Zapolskiy > --- > Changes from v2 to v3: > * essentially simplified the implementation due to removal of runtime > dynamic selection of watchdog pretimeout governors by a user, this > feature is supposed to be added later on Hmm, your call, but I'm not sure this will make the reviewing process easier... > * removed support of sleepable watchdog pretimeout governors This does. > * moved sysfs device attributes to watchdog_dev.c, this required to > add exported watchdog_pretimeout_governor_name() interface Why this move? Before, all the pretimeout stuff was nicely encapsulated in its own file which could be compiled out. Now things are mixing. What was wrong with the approach I took?` > @@ -244,6 +245,13 @@ static int __watchdog_register_device(struct watchdo= g_device *wdd) > } > } > =20 > + ret =3D watchdog_register_pretimeout(wdd); > + if (ret) { > + watchdog_dev_unregister(wdd); > + ida_simple_remove(&watchdog_ida, wdd->id); > + return ret; > + } > + What is the advantage of adding it here instead of adding it in watchdog_dev.c? I mean the files to control govenors are tied to the watchdog_device anyhow, so I'd think it's cleaner to move all that action to watchdog_dev instead of having this stray one in the core. --3lcZGd9BuhuYXNfi Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBAgAGBQJXV8E1AAoJEBQN5MwUoCm2U5cP/i2wJjK7xJ9Cpt/fSE4BjRFu jFcCkkgCMNBq9B+O+H+yoaOLXN3PMb8Q8mx2+jU89WEZAaklam3EVH2Z+dWjxIf6 aAV3UWLIk+wHV+o9jd2oHrCrsVMFXjp4fCtr5Cg0MI+0StWGTsAnvqrJHQfaqqgg 1UNUZot0njGVYQSPRaNEZ9PWOLJb/syYVwzOnTxKDiC3APOINMcX4l/AzRCDszsj u9uWNqZhAKQ7PlCKFSckCq2+xx5aq32/8PaJJ8ar4zN8CO8PjAoVzlrIsTsHn2Qr ZRuyA+fKU+3AQ0hJjifCTrvRBRnBL/UvNPeWpIz7CSw6l/Lmbh8Xoj+vRZItLhqq moyLCU6HOmZnSqEQYSY4BilFfiZbO5gJt1M4iseqiqZr6hgQPw9PolX85AbNC9VW 1bsb2AOrAInnAq0Uxd70yy1blM3L7Ecs503tZRO9GQuKSmMadxU5rBIuYC7dqiHl 5j5XeJRQqbs0shGOOdZV+ENTJ5jOdNkSlHFSSTwLDhK5eTq2CCt4biBx1PIdG6YP 2+eahS0Kh2JocmNmqBeR0SqwyDO0OQnogacsUuuhPoDNsuOSVuY0MoNU/thfUs4l qqlxQnvKh0g7a9ZlM/fvaWluYuuNFnGxsRNrQ8ItQnzkYdl5EMfo4GgLIl4+r/DC nSTKWdAzk454RfaPDapA =bZsC -----END PGP SIGNATURE----- --3lcZGd9BuhuYXNfi--