From owner-svn-src-head@freebsd.org Mon Jun 3 14:52:58 2019 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id EE36515B3926 for ; Mon, 3 Jun 2019 14:52:57 +0000 (UTC) (envelope-from ian@freebsd.org) Received: from outbound1.eu.mailhop.org (outbound1.eu.mailhop.org [52.28.251.132]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 4DBA48434A for ; Mon, 3 Jun 2019 14:52:57 +0000 (UTC) (envelope-from ian@freebsd.org) ARC-Seal: i=1; a=rsa-sha256; t=1559573566; cv=none; d=outbound.mailhop.org; s=arc-outbound20181012; b=D3Jb2TwUPRUt+gf+LhuLdom8U7PRP099wmK3T7nRlAg9Jao7kjNJBcNoFZR1noQ95ZJz9uLcp0XM0 H/tOaANnNaNmYAUqmOCQY0m7RkXDHtLNU+ASly+FEgNT3YQ+DiuwdCPuAdyDneVsdS8lvAAcC4plmI lqjjKXSpFM+FsOZkit3/L7j3WPuF5D9L4NdarOtfLOgbLo/L/nJQZSpOqdpVEi0YERzdjnkgUhfPUu Bz7JQY6kpv0qJou3tm5a3Nhdip1Msd0vcTbqMNU/mjir7cYOT9E7muFM2Z4X2SvQqkocvVFSNdWaZp A5ZTlN2lGjXo0Hmuu/z4fdO+Nst1pLA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=outbound.mailhop.org; s=arc-outbound20181012; h=content-transfer-encoding:mime-version:content-type:references:in-reply-to: date:cc:to:from:subject:message-id:dkim-signature:from; bh=qyenNvNTH52a0KXewTd7CkhxTQemCgKs7MHhWX6XmDA=; b=c4OJugBmfNnocCNvgk0Y5xAHl2d4ZfXzyUs6XXQ1tfHsleAjdbRJhmMn5II8msDuw5blh5XSj/lJJ URDDSfrACpDwwlSHXrW1roQ/Dga8CaUyCSQj0Blnn4Bu5a9yn3fMYXNd/mCXoJjB6EgrNhHYDWVk6M Lp92OpkHvCnourYvdzWBnZBADSj2chnNASCmIQBJiPauL3tnwq5c+A4vtk9kB5LVG0HH2ZXDBMDuvb dvJbxy3YWaq24DtzF2jyEc+234WkgDt/niAhdeD6dq5hACnCTHQZ0huM7kB2R2G9YJ5IOz3qQeN1T8 I9+9XTERZrAjpwHYTcNOdnbmblWlLyw== ARC-Authentication-Results: i=1; outbound3.eu.mailhop.org; spf=softfail smtp.mailfrom=freebsd.org smtp.remote-ip=67.177.211.60; dmarc=none header.from=freebsd.org; arc=none header.oldest-pass=0; DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=outbound.mailhop.org; s=dkim-high; h=content-transfer-encoding:mime-version:content-type:references:in-reply-to: date:cc:to:from:subject:message-id:from; bh=qyenNvNTH52a0KXewTd7CkhxTQemCgKs7MHhWX6XmDA=; b=EMPz3a2/iyJinV2nLkL/gTJ/lU6lbiaVkNlhmVgUlgobNzbEP2wapRy3vUcGQoloiHHNcbbNvvcwh //sBEw8ZED1cOVwqikzC4YAUNfrzG0RFlUtuHIRi94CLp7mnGT4EKzeT3r83IycTIqL+PHDiBtQdpc 5TW200qKhY88s6F0NbhZI7sgtKvVWGLMFhIIiaPTL+97SsC9ft6OHoF1WwOS/bUMi65h2yYZ3vKgQV 92GqACaoiWXTGoAX2sp43QllYauXn+aFw6Mt5hMZGB23oDQCIxqzqeFjfl9OFJMuxJiGJZGbTSPN2f D7RpocfgOjTYZT4QO/cCK2Kr1QkNpIg== X-MHO-RoutePath: aGlwcGll X-MHO-User: 3d1aa7f2-860f-11e9-91aa-b56e4e6b5865 X-Report-Abuse-To: https://support.duocircle.com/support/solutions/articles/5000540958-duocircle-standard-smtp-abuse-information X-Originating-IP: 67.177.211.60 X-Mail-Handler: DuoCircle Outbound SMTP Received: from ilsoft.org (unknown [67.177.211.60]) by outbound3.eu.mailhop.org (Halon) with ESMTPSA id 3d1aa7f2-860f-11e9-91aa-b56e4e6b5865; Mon, 03 Jun 2019 14:52:43 +0000 (UTC) Received: from rev (rev [172.22.42.240]) by ilsoft.org (8.15.2/8.15.2) with ESMTP id x53EqfIv055818; Mon, 3 Jun 2019 08:52:41 -0600 (MDT) (envelope-from ian@freebsd.org) Message-ID: <75cec0b83709f48bbd52e2444d7af17569093f60.camel@freebsd.org> Subject: Re: svn commit: r348355 - head/sys/dev/iicbus From: Ian Lepore To: Andriy Gapon , Niclas Zeising , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Cc: jmd@freebsd.org Date: Mon, 03 Jun 2019 08:52:41 -0600 In-Reply-To: References: <201905290908.x4T98L89066643@repo.freebsd.org> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.28.5 FreeBSD GNOME Team Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: 4DBA48434A X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.99 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_SHORT(-0.99)[-0.987,0]; REPLY(-4.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000,0] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 03 Jun 2019 14:52:58 -0000 On Mon, 2019-06-03 at 15:08 +0300, Andriy Gapon wrote: > On 03/06/2019 14:16, Niclas Zeising wrote: > > Hi! > > It seems like things broke after all, latest pkg build (on head- > > amd64) reports > > this: > > > > > > /wrkdirs/usr/ports/graphics/drm-legacy-kmod/work/drm-legacy- > > 12bd551/src/dev/drm2/i915/intel_iic.c:570:2: > > error: implicit declaration of function 'iicbus_set_nostop' is > > invalid in C99 > > [-Werror,-Wimplicit-function-declaration] > > iicbus_set_nostop(idev, true); > > ^ > > /wrkdirs/usr/ports/graphics/drm-legacy-kmod/work/drm-legacy- > > 12bd551/src/dev/drm2/i915/intel_iic.c:570:2: > > error: this function declaration is not a prototype [-Werror,- > > Wstrict-prototypes] > > 2 errors generated. > > > > Full log: > > > > http://beefy12.nyi.freebsd.org/data/head-amd64-default/p503023_s348376/logs/drm-legacy-kmod-g20190523.log > > Hi! Thank you for the report. > I am going to restore iicbus_set_nostop, but this time as a function > that > modifies iicbus softc (instead of an ivar accessor for the bus). > I am including a patch that I would like to commit. > > However, for the drm code to request the nostop mode correctly it > needs to be > fixed as well. > My proposed patch is here: > https://github.com/FreeBSDDesktop/drm-legacy/pull/9 > > Index: sys/dev/iicbus/iicbus.h > =================================================================== > --- sys/dev/iicbus/iicbus.h (revision 348529) > +++ sys/dev/iicbus/iicbus.h (working copy) > @@ -46,6 +46,8 @@ struct iicbus_softc > * 0 if no start condition succeeded */ > u_char strict; /* deny operations that violate the > * I2C protocol */ > + bool nostop; /* iicbus_transfer defaults to > repeated > + * start between messages */ > struct mtx lock; > u_int bus_freq; /* Configured bus Hz. */ > }; > @@ -77,6 +79,7 @@ IICBUS_ACCESSOR(addr, ADDR, > uint32_t) > > int iicbus_generic_intr(device_t dev, int event, char *buf); > void iicbus_init_frequency(device_t dev, u_int bus_freq); > +void iicbus_set_nostop(device_t dev, bool val); > > extern driver_t iicbus_driver; > extern devclass_t iicbus_devclass; > Index: sys/dev/iicbus/iiconf.c > =================================================================== > --- sys/dev/iicbus/iiconf.c (revision 348529) > +++ sys/dev/iicbus/iiconf.c (working copy) > @@ -383,6 +383,14 @@ iicbus_block_read(device_t bus, u_char slave, > char > return (error); > } > > +void > +iicbus_set_nostop(device_t bus, bool val) > +{ > + struct iicbus_softc *sc = device_get_softc(bus); > + > + sc->nostop = val; > +} > + > /* > * iicbus_transfer() > * > @@ -427,7 +435,8 @@ iicbus_transfer_gen(device_t dev, struct iic_msg > * > { > int i, error, lenread, lenwrote, nkid, rpstart, addr; > device_t *children, bus; > - bool started; > + struct iicbus_softc *sc; > + bool nostop, started; > > if ((error = device_get_children(dev, &children, &nkid)) != 0) > return (IIC_ERESOURCE); > @@ -438,6 +447,8 @@ iicbus_transfer_gen(device_t dev, struct iic_msg > * > bus = children[0]; > rpstart = 0; > free(children, M_TEMP); > + sc = device_get_softc(bus); > + nostop = sc->nostop; > started = false; > for (i = 0, error = 0; i < nmsgs && error == 0; i++) { > addr = msgs[i].slave; > @@ -465,11 +476,12 @@ iicbus_transfer_gen(device_t dev, struct > iic_msg * > if (error != 0) > break; > > - if (!(msgs[i].flags & IIC_M_NOSTOP)) { > + if ((msgs[i].flags & IIC_M_NOSTOP) != 0 || > + (nostop && i + 1 < nmsgs)) { > + rpstart = 1; /* Next message gets repeated > start */ > + } else { > rpstart = 0; > iicbus_stop(bus); > - } else { > - rpstart = 1; /* Next message gets repeated > start */ > } > } > if (error != 0 && started) > > Please don't. We still have a situation where nobody has shown a runtime failure at all. This build failure could be fixed by simply defining a do-nothing iicbus_set_nostop() function if a quick fix is needed. Putting this nostop concept into code that is shared by many drivers is an abomination. We have exactly one driver that needs this functionality, so the right fix is to implement it wholly within that one driver. I'll put together a diff for that. -- Ian