svn commit: r348355 - head/sys/dev/iicbus

[Ian Lepore]

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