From owner-svn-src-all@freebsd.org Mon Oct 14 16:45:29 2019 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id D11D6130351; Mon, 14 Oct 2019 16:45:29 +0000 (UTC) (envelope-from manu@bidouilliste.com) Received: from mail.blih.net (mail.blih.net [212.83.177.182]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mail.blih.net", Issuer "mail.blih.net" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id 46sPZ04S1Bz4ZJc; Mon, 14 Oct 2019 16:45:28 +0000 (UTC) (envelope-from manu@bidouilliste.com) Received: from mail.blih.net (mail.blih.net [212.83.177.182]) by mail.blih.net (OpenSMTPD) with ESMTP id 3e7095a9; Mon, 14 Oct 2019 18:45:26 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=bidouilliste.com; h=date :from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-type:content-transfer-encoding; s=mail; bh=iGGZD919yCZ3EAJs3ifLlk2isaQ=; b=izF4fEPdnS1r9OI9BwT08lv2LtNo 4l0VhmGs5HyOy98ZrlBu1bp2U5NLixrThwHDG3tpb3HDZWlfT6AapWVWZbZQHexD ddWPZT1aIYB5meD+SeK/PogECNYfi2Y6XZIXvVfwGva4vaMI3EP7WNH+VCI6144V Y+iMX2y82tU+05c= DomainKey-Signature: a=rsa-sha1; c=nofws; d=bidouilliste.com; h=date :from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-type:content-transfer-encoding; q=dns; s= mail; b=IGQBqLEhopbYENfXeINc9m3v9A3uskXRtOxjSVn2RyzghQEATBdj1W4D 4aEwKkUZasiIOHe/6eTUpwJEpHRbTTikSWp4F/Fq0FgvbpwaKXCw+ic4gz7CjcHm t/3dHauByFt3A2Hz6hlnt8EEfH1k7RJSZmNVs5VrjZ7Xtg5tlz0= Received: from skull.home.blih.net (ip-9.net-89-3-105.rev.numericable.fr [89.3.105.9]) by mail.blih.net (OpenSMTPD) with ESMTPSA id b8d2786e TLS version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO; Mon, 14 Oct 2019 18:45:26 +0200 (CEST) Date: Mon, 14 Oct 2019 18:45:26 +0200 From: Emmanuel Vadot To: Ruslan Bukin Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r353493 - head/sys/dev/mmc/host Message-Id: <20191014184526.e5f3798fd4d4f9c744f4a491@bidouilliste.com> In-Reply-To: <20191014162751.GA30496@bsdpad.com> References: <201910141553.x9EFr0Zb010167@repo.freebsd.org> <20191014181051.bd8c7a3dbb7b07a636d81ed9@bidouilliste.com> <20191014162751.GA30496@bsdpad.com> X-Mailer: Sylpheed 3.7.0 (GTK+ 2.24.32; amd64-portbld-freebsd13.0) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 46sPZ04S1Bz4ZJc X-Spamd-Bar: / Authentication-Results: mx1.freebsd.org; dkim=pass header.d=bidouilliste.com header.s=mail header.b=izF4fEPd; dmarc=none; spf=pass (mx1.freebsd.org: domain of manu@bidouilliste.com designates 212.83.177.182 as permitted sender) smtp.mailfrom=manu@bidouilliste.com X-Spamd-Result: default: False [-0.54 / 15.00]; ARC_NA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; R_DKIM_ALLOW(-0.20)[bidouilliste.com:s=mail]; NEURAL_HAM_MEDIUM(-0.15)[-0.149,0]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_THREE(0.00)[4]; R_SPF_ALLOW(-0.20)[+ip4:212.83.177.182/32]; TO_MATCH_ENVRCPT_ALL(0.00)[]; MIME_GOOD(-0.10)[text/plain]; MV_CASE(0.50)[]; DMARC_NA(0.00)[bidouilliste.com]; TO_DN_SOME(0.00)[]; NEURAL_HAM_LONG(-0.86)[-0.864,0]; DKIM_TRACE(0.00)[bidouilliste.com:+]; RCVD_NO_TLS_LAST(0.10)[]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; IP_SCORE(0.38)[ip: (-0.70), ipnet: 212.83.160.0/19(2.47), asn: 12876(0.12), country: FR(-0.00)]; ASN(0.00)[asn:12876, ipnet:212.83.160.0/19, country:FR]; MID_RHS_MATCH_FROM(0.00)[]; RCVD_COUNT_TWO(0.00)[2] X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 14 Oct 2019 16:45:29 -0000 On Mon, 14 Oct 2019 17:27:51 +0100 Ruslan Bukin wrote: > On Mon, Oct 14, 2019 at 06:10:51PM +0200, Emmanuel Vadot wrote: > > > > On Mon, 14 Oct 2019 15:53:00 +0000 (UTC) > > Ruslan Bukin wrote: > > > > > Author: br > > > Date: Mon Oct 14 15:52:59 2019 > > > New Revision: 353493 > > > URL: https://svnweb.freebsd.org/changeset/base/353493 > > > > > > Log: > > > Fix the driver attachment in cases when the external resource devices > > > (resets, regulators, clocks) are not available. > > > > > > Rely on a system initialization done by a bootloader in that cases. > > > > > > This fixes operation on Terasic DE10-Pro (an Intel Stratix 10 > > > development kit). > > > > > > Sponsored by: DARPA, AFRL > > > > > > Modified: > > > head/sys/dev/mmc/host/dwmmc.c > > > > > > Modified: head/sys/dev/mmc/host/dwmmc.c > > > ============================================================================== > > > --- head/sys/dev/mmc/host/dwmmc.c Mon Oct 14 15:33:53 2019 (r353492) > > > +++ head/sys/dev/mmc/host/dwmmc.c Mon Oct 14 15:52:59 2019 (r353493) > > > @@ -1,5 +1,5 @@ > > > /*- > > > - * Copyright (c) 2014 Ruslan Bukin > > > + * Copyright (c) 2014-2019 Ruslan Bukin > > > * All rights reserved. > > > * > > > * This software was developed by SRI International and the University of > > > @@ -457,26 +457,20 @@ parse_fdt(struct dwmmc_softc *sc) > > > > > > /* IP block reset is optional */ > > > error = hwreset_get_by_ofw_name(sc->dev, 0, "reset", &sc->hwreset); > > > - if (error != 0 && error != ENOENT) { > > > + if (error != 0 && error != ENOENT) > > > device_printf(sc->dev, "Cannot get reset\n"); > > > - goto fail; > > > - } > > > > This is not correct, on a system without reset/clock/regulator support > > you will get ENODEV as the phandle is present but no device is > > associated with it. This is the case that you want to test. Currently > > this hide all errors. > > The change means that the driver will be attached regardless of the return value from ext resources. Yes and this is a problem. > Why it is not correct? Because if a reset/clock/regulator is present but we failed to parse the node (bad #clock-cell for example) this just assume that the resource isn't present which is not correct. Or worse the underlying gpio cannot be mapped, you now have a non functional driver because you cannot switch the regulator. > It does not hide all errors, the printf will be called and a user will be warned. If the ressources are present in the DTB and the system support them but they cannot be used the driver should fail. Which is why I suggest you to make an extra case on ENODEV even if I don't like that because it's hacky even for a platform that is, to my knowledge, not really supported (no u-boot port, dtb aren't build, not part of GENERIC). > Ruslan -- Emmanuel Vadot