svn commit: r348355 - head/sys/dev/iicbus
Niclas Zeising
zeising at freebsd.org
Mon Jun 3 11:16:37 UTC 2019
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
More information about the svn-src-all
mailing list