From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 55757C4320A for ; Thu, 12 Aug 2021 14:21:45 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id F20CD60EFE for ; Thu, 12 Aug 2021 14:21:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org F20CD60EFE Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 7852A82DC3; Thu, 12 Aug 2021 16:21:41 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1628778101; bh=eKmY2dNWyJIvkLikb2/9/BQii/Ge6ipcN5+6Ns7j+xo=; h=To:cc:From:Subject:In-reply-to:References:Date:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=zSF+2pO4eAtqCrSUMsEb9ZOUeBL1PjEOIXW/5V7jus3DVdyoZRBeHCgqleGTJtzxs 8pX+20flHiDrh8++Yi7WJpbPhZgnjogBkww26T8Kn22aHWAvxQ16JKsWtJ9nKGo+mO cR6zARFvg++dQ+qdelwKp7psruiDSm6hKSpJU2tp3ZkDTctnPri1Bfei2jXrX/ygTf KIz3VVatONckX4y87gL9YBcwAp57G80Bwx6qbvzrf/VCIOk8iV/X9afBRzvVXrph3p umZhNCfvR3ydbR2IzxG/8P18u9c077648YSTCNRgKmwGNgr+3N8UOJSvPNiCGDv+C6 hy17Wk1TdwSaA== Received: from janitor.denx.de (unknown [62.91.23.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: noc@denx.de) by phobos.denx.de (Postfix) with ESMTPSA id 956B482DB3 for ; Thu, 12 Aug 2021 16:21:40 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1628778100; bh=eKmY2dNWyJIvkLikb2/9/BQii/Ge6ipcN5+6Ns7j+xo=; h=To:cc:From:Subject:In-reply-to:References:Date:From; b=Xt9V8htN94fUuNLSxJrUsOclNF7MELvrF18mfOpqBfFn7P9Ejk0DA3rS2fK1txgY0 nFXAttfTgYqDRyC9luKyrq1HW6F1tzY0P8IfYWTeLPUYtYUUwtShGKUN0oCQ8QqK7k pmPsIyvNulZKyCuQAZ7xQQRT5LE+sGeh6XSqUJGc0ej0pyGFMbLGPAM1zJoRlQXoqS szd4kbWAKXqnFxEWfvFUoHqxZjNMztWT3nCYyFjT97QlKLm3kM+CR43CBNxG9qnWHG 5yOUvVhXKedHxAQxa/fkBemKBdmGJUxOKpAIGioYBBsy137kurwigo2SR7BqUWSAf+ k+ud4JY+lmeXA== Received: by janitor.denx.de (Postfix, from userid 108) id 29390A0162; Thu, 12 Aug 2021 16:21:40 +0200 (CEST) Received: from gemini.denx.de (gemini.denx.de [10.4.0.2]) by janitor.denx.de (Postfix) with ESMTPS id 40B21A0036; Thu, 12 Aug 2021 16:21:30 +0200 (CEST) Received: from gemini.denx.de (localhost [IPv6:::1]) by gemini.denx.de (Postfix) with ESMTP id D99AD1E2304; Thu, 12 Aug 2021 16:21:29 +0200 (CEST) To: Tom Rini cc: Rasmus Villemoes , Stefan Roese , u-boot@lists.denx.de, Simon Glass From: Wolfgang Denk Subject: Re: [PATCH v4 07/10] watchdog: wdt-uclass.c: handle all DM watchdogs in watchdog_reset() MIME-Version: 1.0 Content-type: text/plain; charset=UTF-8 Content-transfer-encoding: 8bit In-reply-to: <20210812134833.GU858@bill-the-cat> References: <20210802150016.588750-1-rasmus.villemoes@prevas.dk> <20210802150016.588750-8-rasmus.villemoes@prevas.dk> <024904e9-2205-37d9-e480-04de75c8cb5c@denx.de> <3d48015a-07d3-e296-b9ba-a1edd455ce9e@prevas.dk> <538195.1628684952@gemini.denx.de> <20210811124318.GT858@bill-the-cat> <575632.1628750421@gemini.denx.de> <20210812134833.GU858@bill-the-cat> Comments: In-reply-to Tom Rini message dated "Thu, 12 Aug 2021 09:48:33 -0400." Date: Thu, 12 Aug 2021 16:21:29 +0200 Message-ID: <601067.1628778089@gemini.denx.de> X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean Dear Tom, In message <20210812134833.GU858@bill-the-cat> you wrote: > > Alright, lets take a look at what kind of area of the code we're talking > about. uclass_get is a pretty fundamental thing. If that fails, your > system is on fire. Things are massively corrupt. Full agreement here. > So yes, return codes need to be checked and passed. But no, not every > single error path needs to print to the user along every part of an > error path either. So if "the system is on fire" is one of the cases where an error message should be omitted to save maybe 50 or 100 bytes of image size? This sounds wrong to me. When a system crashes or hangs, it is extremely helpful to gen an indication of what happened. Printing messages only with debug enabled is pretty worthless, as in the real world the failures will happen in the field when running the production image with debug not enabled. And as soon as you do enable debug, you will have a different image, which may or may not show the problem. We have all been there before. > That would be the next patch in the series where the BSP author isn't > currently checking the return value, and this series doesn't change > that. Perhaps it should, and CC the maintainer. Indeed. > But I think has been > said a few times over the course of this series, what exactly is one > going to do about the failure? Getting specific for a moment, if you're > in the case of "shutdown the watchdog" and the watchdog doesn't shutdown > like you want it to, do you hang and hope the watchdog is alive to kick > things still? hang and lock the system? Figure the system is on fire > anyhow but add another message to the failure spew? Adding a message about the _cause_ of the failure, i. e. at least information about the place where it was first detected, is what will be helpful to the engineer who is supposed to analyze the problem. And yes, if such a fundamental operation fails, it is wise to abort the operation of this device - either by hard resetting it or by hanging it, depending of what the chances are that a reboot might fix the problem. IMO it is better to fail a broken device in a reliable mode instead of letting it run and having more spectacular crashes (likely with more serious consequences) happen later. > Again, I think the change that's needed to this patch is to make it > return the error code to the caller. Let the caller decide. And make > sure to CC the board maintainer on the next go-round so they can chime > in about that. If we agree that in the disputed case "the system is on fire", then there is actually very little to decide. There should be only one possible action: stop here, before more damage happens. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de ... not meant for the weak-at-heart: /^(?=.*one)(?=.*two)/ If you can follow that, you can use it. - Randal L. Schwartz in