Date: Mon, 03 Jun 2019 10:30:58 -0600 From: Ian Lepore <ian@freebsd.org> To: Andriy Gapon <avg@FreeBSD.org>, Niclas Zeising <zeising@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: <68e39f5f8b18569a406ec6bafe8d60fbe9e6d87f.camel@freebsd.org> In-Reply-To: <f4291764-09e3-2870-9e33-362ae1423c4b@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> <ac9ae4b6-4b89-1e5f-9116-dcf20fee7e85@freebsd.org> <cda9c032-f0ab-7bf5-9d6f-c6167b1cf9ff@FreeBSD.org> <75cec0b83709f48bbd52e2444d7af17569093f60.camel@freebsd.org> <f4291764-09e3-2870-9e33-362ae1423c4b@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 2019-06-03 at 18:19 +0300, Andriy Gapon wrote: > On 03/06/2019 17:52, Ian Lepore wrote: > > 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. > > Well, I am quite certain that the run-time failure will follow after > the build > time failure is fixed. > > > 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. > > That's true that we have just one such driver. > At the same time, the "no stop" (or rather, repeated start) behavior > makes more > sense. If stop+start between transfers are needed then that can be > done with > multiple calls to iicbus_transfer. If multiple messages are given to > iicbus_transfer, then it's reasonable to assume that a repeated > started is > wanted between them. But it would be a big change to review and, if > needed, fix > or tidy up all code that uses iicbus_transfer. So, > iicbus_set_nostop() could be > just a small step towards the bigger goal. > > But I really don't have a strong opinion. > Fixing drm2 directly is just as good for me as iicbus_set_nostop. > I agree with you that the freebsd iic stuff is a Big Mess, poorly designed. But it is what it is, we can't change things now without breaking every out-of-tree driver in the world (and I know for sure they exist, because we have some at $work). So we're stuck with explicitly setting NOSTOP flags in iic_msg descriptors even though it would be sensible to assume NOSTOP on all but the last one. Anyway, the bad news is I mis-remembered how the iicbb interface and driver are structured. The hook point I wanted isn't there, making it hard to put the fix I was picturing in the intel_iic code. I'd still like to think of a better fix, but it's going to take more than a few minutes. So I guess we'd better go with your fix to get things working right away. If I can think up something more elegant we can always switch to it later. -- Ian
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?68e39f5f8b18569a406ec6bafe8d60fbe9e6d87f.camel>