Date: Wed, 16 Oct 2019 16:06:34 +0100 From: Ruslan Bukin <br@freebsd.org> To: Emmanuel Vadot <manu@bidouilliste.com> 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: <20191016150634.GA17444@bsdpad.com> In-Reply-To: <20191014184526.e5f3798fd4d4f9c744f4a491@bidouilliste.com> References: <201910141553.x9EFr0Zb010167@repo.freebsd.org> <20191014181051.bd8c7a3dbb7b07a636d81ed9@bidouilliste.com> <20191014162751.GA30496@bsdpad.com> <20191014184526.e5f3798fd4d4f9c744f4a491@bidouilliste.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Oct 14, 2019 at 06:45:26PM +0200, Emmanuel Vadot wrote: > On Mon, 14 Oct 2019 17:27:51 +0100 > Ruslan Bukin <br@freebsd.org> 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 <br@FreeBSD.org> 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 <br@bsdpad.com> > > > > + * Copyright (c) 2014-2019 Ruslan Bukin <br@bsdpad.com> > > > > * 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. > Could an error in DTS convert a device from being optional to become mandatory? It is still not clear if any support present on a stage of parsing. Since the knowledge of registered support is at extres, then it should be mandatory from a driver perspective (if enabled in kernel config). And then the framework should make a decision based on any support present. What do you think ? Ruslan
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20191016150634.GA17444>