Date: Mon, 3 Jun 2019 13:16:33 +0200 From: Niclas Zeising <zeising@freebsd.org> To: Ian Lepore <ian@freebsd.org>, Andriy Gapon <avg@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Cc: jmd@freebsd.org Subject: Re: svn commit: r348355 - head/sys/dev/iicbus Message-ID: <ac9ae4b6-4b89-1e5f-9116-dcf20fee7e85@freebsd.org> In-Reply-To: <d1128088420c6e52721fb5df2280ca73096bf5c0.camel@freebsd.org> References: <201905290908.x4T98L89066643@repo.freebsd.org> <c3f1c60b-24b2-6098-501a-8cb81ef66d57@freebsd.org> <def030c0-80a5-84ca-bb48-7009aa34e69c@FreeBSD.org> <d1128088420c6e52721fb5df2280ca73096bf5c0.camel@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 2019-05-29 22:52, Ian Lepore wrote: > On Wed, 2019-05-29 at 15:12 +0300, Andriy Gapon wrote: >> On 29/05/2019 14:54, Niclas Zeising wrote: >>> On 2019-05-29 11:08, Andriy Gapon wrote: >>>> Author: avg >>>> Date: Wed May 29 09:08:20 2019 >>>> New Revision: 348355 >>>> URL: https://svnweb.freebsd.org/changeset/base/348355 >>>> >>>> Log: >>>> revert r273728 and parts of r306589, iicbus no-stop by default feature >>>> Since drm2 removal, there has not been any consumer of the feature in the >>>> tree. I am also unaware of any out-of-tree consumer. >>>> More importantly, the feature has been broken from the very start, both >>>> before and after r306589, because the ivar was set on a device that does >>>> not support it and it was read from another device that also does not >>>> support it. >>>> A bus-wide no-stop flag cannot be implemented as an ivar as iicbus >>>> attaches as a child of various drivers. Implementing the ivar in each >>>> and every I2C driver is just impractical. >>>> If we ever want to implement this feature properly, then probably the >>>> easiest way to do it would be via a flag in the softc of iicbus. >>>> In fact, we might have to do that in the stable branches if we want to >>>> fix the code for them. >>>> Reported by: ian (long time ago) >>>> MFC after: 1 month (maybe) >>>> X-MFC-note: cannot just merge the change, must keep drm2 happy >>>> >>> >>> Hi! >>> Just a note, be aware that drm2 lives on in ports as drm-legacy-kmod. I haven't >>> tested, but, from the description above I worry that it will affect the port. >>> What do you think? >> >> Oh, I forgot about that one... >> I think that it could be affected if it still uses FreeBSD iic code. >> I guess I might have to revert the change. >> >> > > I don't think so, because I don't think this change ever worked. I'm > not sure how anybody convinced themselves that it did. It attempts to > retrieve ivars from a device that doesn't have them, so the net effect > is that the nostop variable is initialized from stack garbage. Maybe > whoever wrote and tested it was lucky enough to have that accidentally > be consistently zero or non-zero, so their testing appeared to work. > > Looking at the drm2 code that is the only user of this, it appears that > the nostop value is only used in the case where the driver falls back > to using the builtin intel_iicbb_driver. That driver relies on > iicbus_transfer_gen() which is where the nostop kludge was added. > That's the fundamental problem in all of this: the right thing to do, > IMO, would have been to implement the iicbus_transfer method directly > in the intel_iicbb_driver (probably by just cut-and-pasting the code > from iicconf.c then doing whatever is necessary to ignore stops). And > we can still do that, pretty trivially, if necessary. > > -- Ian > 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 Regards -- Niclas Zeising
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?ac9ae4b6-4b89-1e5f-9116-dcf20fee7e85>