NetBSD Problem Report #53171
From www@NetBSD.org Tue Apr 10 04:32:22 2018
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
by mollari.NetBSD.org (Postfix) with ESMTPS id 9FE277A1DC
for <gnats-bugs@gnats.NetBSD.org>; Tue, 10 Apr 2018 04:32:22 +0000 (UTC)
Message-Id: <20180410043221.77ADD7A1EE@mollari.NetBSD.org>
Date: Tue, 10 Apr 2018 04:32:21 +0000 (UTC)
From: thorpej@me.com
Reply-To: thorpej@me.com
To: gnats-bugs@NetBSD.org
Subject: Broadcom (and other ARM SoC i2c drivers) mis-handle indirect configuration of devices
X-Send-Pr-Version: www-1.0
>Number: 53171
>Category: port-arm
>Synopsis: Broadcom (and other ARM SoC i2c drivers) mis-handle indirect configuration of devices
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: thorpej
>State: closed
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Tue Apr 10 04:35:00 +0000 2018
>Closed-Date: Wed May 09 02:54:22 +0000 2018
>Last-Modified: Wed May 09 02:55:00 +0000 2018
>Originator: Jason Thorpe
>Release: NetBSD 8.99.14 Mon Apr 9 21:30:42 PDT 2018
>Organization:
>Environment:
NetBSD nixie-dev 8.99.14 NetBSD 8.99.14 (thorpej-RPI-NixieClock) #9: Mon Apr 9 21:09:00 PDT 2018 thorpej@BigMac.local:/Volumes/Data0/Users/thorpej/hack/NetBSD/current/src/sys/arch/evbarm/compile/thorpej-RPI-NixieClock evbarm
>Description:
The “iic” driver supports both direct configuration of devices (for board configs with a known set of devices on the i2c), as well as indirect config, which searches for devices specified in the kernel config file.
It looks like this code has changed a bit since I wrote it 15 years ago. If the parent driver (bsciic, in this case) provides a prop_array_t as iba->iba_child_devices, then indirect configuration is bypassed. If it does not, then there’s a dance to check for i2c-indirect-config == false or if the parent has i2c-child-devices as a property on itself, in which case, again, indirect config is skipped.
The problem in the bsciic driver is that if there is not i2c-child-devices property, then an empty array is assigned to iba->iba_child_devices. This makes the check against NULL fail, and thus no indirect config devices will be found.
>How-To-Repeat:
Attempt to enable indirect configuration of non-standard i2c devices on a Raspberry Pi.
>Fix:
Fix for Raspberry Pi attached. Other i2c host controller drivers for ARM SoCs have a similar bug.
Index: bcm2835_bsc.c
===================================================================
RCS file: /cvsroot/src/sys/arch/arm/broadcom/bcm2835_bsc.c,v
retrieving revision 1.10
diff -u -p -r1.10 bcm2835_bsc.c
--- bcm2835_bsc.c 3 Mar 2018 16:03:38 -0000 1.10
+++ bcm2835_bsc.c 10 Apr 2018 04:22:17 -0000
@@ -184,7 +184,7 @@ bsciic_attach(device_t parent, device_t
if (iba.iba_child_devices)
prop_object_retain(iba.iba_child_devices);
else
- iba.iba_child_devices = prop_array_create();
+ iba.iba_child_devices = NULL;
prop_object_release(devs);
config_found_ia(self, "i2cbus", &iba, iicbus_print);
>Release-Note:
>Audit-Trail:
From: Jason Thorpe <thorpej@me.com>
To: gnats-bugs@NetBSD.org
Cc: port-arm-maintainer@netbsd.org, gnats-admin@netbsd.org,
netbsd-bugs@netbsd.org
Subject: Re: port-arm/53171: Broadcom (and other ARM SoC i2c drivers)
mis-handle indirect configuration of devices
Date: Thu, 12 Apr 2018 20:42:41 -0700
Here is a more complete patch. That NULL assignment is redundant, of =
course, and this catches the rest of the ARM SoC drivers that get it =
wrong.
Index: broadcom/bcm2835_bsc.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvsroot/src/sys/arch/arm/broadcom/bcm2835_bsc.c,v
retrieving revision 1.10
diff -u -p -r1.10 bcm2835_bsc.c
--- broadcom/bcm2835_bsc.c 3 Mar 2018 16:03:38 -0000 1.10
+++ broadcom/bcm2835_bsc.c 13 Apr 2018 03:35:18 -0000
@@ -183,8 +183,6 @@ bsciic_attach(device_t parent, device_t=20
iba.iba_child_devices =3D prop_dictionary_get(devs, =
"i2c-child-devices");
if (iba.iba_child_devices)
prop_object_retain(iba.iba_child_devices);
- else
- iba.iba_child_devices =3D prop_array_create();
prop_object_release(devs);
=20
config_found_ia(self, "i2cbus", &iba, iicbus_print);
Index: nvidia/tegra_i2c.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvsroot/src/sys/arch/arm/nvidia/tegra_i2c.c,v
retrieving revision 1.16
diff -u -p -r1.16 tegra_i2c.c
--- nvidia/tegra_i2c.c 25 May 2017 23:43:49 -0000 1.16
+++ nvidia/tegra_i2c.c 13 Apr 2018 03:35:18 -0000
@@ -199,11 +199,8 @@ tegra_i2c_attach(device_t parent, device
memset(&iba, 0, sizeof(iba));
iba.iba_tag =3D &sc->sc_ic;
iba.iba_child_devices =3D prop_dictionary_get(devs, =
"i2c-child-devices");
- if (iba.iba_child_devices !=3D NULL) {
+ if (iba.iba_child_devices !=3D NULL)
prop_object_retain(iba.iba_child_devices);
- } else {
- iba.iba_child_devices =3D prop_array_create();
- }
prop_object_release(devs);
=20
sc->sc_i2cdev =3D config_found_ia(self, "i2cbus", &iba, =
iicbus_print);
Index: samsung/exynos_i2c.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvsroot/src/sys/arch/arm/samsung/exynos_i2c.c,v
retrieving revision 1.13
diff -u -p -r1.13 exynos_i2c.c
--- samsung/exynos_i2c.c 2 Jul 2017 18:27:45 -0000 1.13
+++ samsung/exynos_i2c.c 13 Apr 2018 03:35:18 -0000
@@ -198,8 +198,6 @@ exynos_i2c_attach(device_t parent, devic
iba.iba_child_devices =3D prop_dictionary_get(devs, =
"i2c-child-devices");
if (iba.iba_child_devices !=3D NULL)
prop_object_retain(iba.iba_child_devices);
- else
- iba.iba_child_devices =3D prop_array_create();
prop_object_release(devs);
=20
sc->sc_i2cdev =3D config_found_ia(self, "i2cbus", &iba, =
iicbus_print);
Index: sunxi/sunxi_rsb.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvsroot/src/sys/arch/arm/sunxi/sunxi_rsb.c,v
retrieving revision 1.1
diff -u -p -r1.1 sunxi_rsb.c
--- sunxi/sunxi_rsb.c 2 Jul 2017 18:06:45 -0000 1.1
+++ sunxi/sunxi_rsb.c 13 Apr 2018 03:35:18 -0000
@@ -220,8 +220,6 @@ sunxi_rsb_attach(device_t parent, device
iba.iba_child_devices =3D prop_dictionary_get(devs, =
"i2c-child-devices");
if (iba.iba_child_devices)
prop_object_retain(iba.iba_child_devices);
- else
- iba.iba_child_devices =3D prop_array_create();
prop_object_release(devs);
=20
sc->sc_i2cdev =3D config_found_ia(self, "i2cbus", &iba, =
iicbus_print);
Index: sunxi/sunxi_twi.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvsroot/src/sys/arch/arm/sunxi/sunxi_twi.c,v
retrieving revision 1.8
diff -u -p -r1.8 sunxi_twi.c
--- sunxi/sunxi_twi.c 2 Dec 2017 18:56:18 -0000 1.8
+++ sunxi/sunxi_twi.c 13 Apr 2018 03:35:18 -0000
@@ -236,8 +236,6 @@ sunxi_twi_attach(device_t parent, device
iba.iba_child_devices =3D prop_dictionary_get(devs, =
"i2c-child-devices");
if (iba.iba_child_devices)
prop_object_retain(iba.iba_child_devices);
- else
- iba.iba_child_devices =3D prop_array_create();
prop_object_release(devs);
=20
config_found_ia(self, "i2cbus", &iba, iicbus_print);
Responsible-Changed-From-To: port-arm-maintainer->thorpej
Responsible-Changed-By: thorpej@NetBSD.org
Responsible-Changed-When: Mon, 30 Apr 2018 17:16:02 +0000
Responsible-Changed-Why:
I'll guide this through.
From: Jared McNeill <jmcneill@invisible.ca>
To: gnats-bugs@NetBSD.org
Cc: thorpej@NetBSD.org, port-arm-maintainer@netbsd.org, netbsd-bugs@netbsd.org,
gnats-admin@netbsd.org, thorpej@NetBSD.org, thorpej@me.com
Subject: Re: port-arm/53171 (Broadcom (and other ARM SoC i2c drivers) mis-handle
indirect configuration of devices)
Date: Mon, 30 Apr 2018 14:33:32 -0300 (ADT)
Can this change be done in a way where we require explicit locators for
attaching indirect child devices? I worry about driver probe functions
sending crap over the i2c bus on boards where we've otherwise strictly
defined devices in the DT.
From: Martin Husemann <martin@duskware.de>
To: Jared McNeill <jmcneill@invisible.ca>
Cc: gnats-bugs@NetBSD.org, thorpej@NetBSD.org
Subject: Re: port-arm/53171 (Broadcom (and other ARM SoC i2c drivers)
mis-handle indirect configuration of devices)
Date: Mon, 30 Apr 2018 19:48:51 +0200
On Mon, Apr 30, 2018 at 02:33:32PM -0300, Jared McNeill wrote:
> Can this change be done in a way where we require explicit locators for
> attaching indirect child devices? I worry about driver probe functions
> sending crap over the i2c bus on boards where we've otherwise strictly
> defined devices in the DT.
This sounds similar to some sparc machines where we do strict direct
configuration for devices that OF knows about, but on some i2c buses
use indirect config (but I haven't been paying close attention so far).
Martin
From: Jason Thorpe <thorpej@me.com>
To: Jared McNeill <jmcneill@invisible.ca>
Cc: gnats-bugs@NetBSD.org, Jason Thorpe <thorpej@NetBSD.org>,
port-arm-maintainer@netbsd.org, netbsd-bugs@netbsd.org, gnats-admin@netbsd.org
Subject: Re: port-arm/53171 (Broadcom (and other ARM SoC i2c drivers)
mis-handle indirect configuration of devices)
Date: Mon, 30 Apr 2018 12:51:19 -0700
> On Apr 30, 2018, at 10:33 AM, Jared McNeill <jmcneill@invisible.ca> =
wrote:
>=20
> Can this change be done in a way where we require explicit locators =
for attaching indirect child devices? I worry about driver probe =
functions sending crap over the i2c bus on boards where we've otherwise =
strictly defined devices in the DT.
Not sure what you mean =E2=80=94do you mean =E2=80=9Cdisallow wildcard =
locators for indirect config=E2=80=9D? That should be trivial. I think =
just about all of the i2c drivers already specifically filter their own =
=E2=80=9Cvalid addresses for this device=E2=80=9D before even doing =
anything, though.
That said, the current behavior, is obviously a bug =E2=80=A6 probably =
introduced when FDT-ization of evbarm happened in the first place (I =
guess no one had devices that were commented out in the kernel config =
files to try?)
Anyway, if you want to prevent random devices from even trying =
known-to-DT-i2c-locations, we=E2=80=99ll have to separately record which =
i2c locations are reserved for direct-config-only, and only process =
indirect after we=E2=80=99ve finished enumeration of direct-config =
devices from the DT.
I=E2=80=99m not sure what all of this really buys you, though, because =
userland has access to i2c as well =E2=80=A6 so to keep that avenue from =
tripping things up, you want to separate filter =E2=80=9Cany address =
that is in use by the kernel for some reason=E2=80=9D. But that filter =
would add expense to each userland i2c command, since the address is =
specified in the i2c exec argument. Seems like an orthogonal problem =
that warrants a separate change.
DT is nifty, but it=E2=80=99s leading to an annoying bifurcation of =
device configuration information. The kernel config file was the =
traditional way to specify this stuff=E2=80=A6 but if we=E2=80=99re =
going to say =E2=80=9Chey, the right thing to do on FDT platforms is to =
provide your own DT snippets for whatever hardware you=E2=80=99re adding =
to your prototyping platform=E2=80=9D, then we need a way to make that =
easy (or discoverable, or maybe both).
-- thorpej
From: Jared McNeill <jmcneill@invisible.ca>
To: Jason Thorpe <thorpej@me.com>
Cc: gnats-bugs@NetBSD.org,
Jason Thorpe <thorpej@NetBSD.org>,
port-arm-maintainer@netbsd.org,
netbsd-bugs@netbsd.org,
gnats-admin@netbsd.org
Subject: Re: port-arm/53171 (Broadcom (and other ARM SoC i2c drivers)
mis-handle indirect configuration of devices)
Date: Mon, 30 Apr 2018 17:16:16 -0300
On Apr 30, 2018, at 4:51 PM, Jason Thorpe <thorpej@me.com> wrote:
> Not sure what you mean =E2=80=94do you mean =E2=80=9Cdisallow =
wildcard locators for indirect config=E2=80=9D? That should be trivial. =
I think just about all of the i2c drivers already specifically filter =
their own =E2=80=9Cvalid addresses for this device=E2=80=9D before even =
doing anything, though.
I thought we had some drivers that relied on this but when I went to =
look I couldn=E2=80=99t find any..
> That said, the current behavior, is obviously a bug =E2=80=A6 probably =
introduced when FDT-ization of evbarm happened in the first place (I =
guess no one had devices that were commented out in the kernel config =
files to try?)
Completely agree that it=E2=80=99s a bug. Thanks for working on this.
> Anyway, if you want to prevent random devices from even trying =
known-to-DT-i2c-locations, we=E2=80=99ll have to separately record which =
i2c locations are reserved for direct-config-only, and only process =
indirect after we=E2=80=99ve finished enumeration of direct-config =
devices from the DT.
>=20
> I=E2=80=99m not sure what all of this really buys you, though, because =
userland has access to i2c as well =E2=80=A6 so to keep that avenue from =
tripping things up, you want to separate filter =E2=80=9Cany address =
that is in use by the kernel for some reason=E2=80=9D. But that filter =
would add expense to each userland i2c command, since the address is =
specified in the i2c exec argument. Seems like an orthogonal problem =
that warrants a separate change.
I=E2=80=99m not really concerned with preventing access to the I2C bus. =
I just want to make sure that we don=E2=80=99t accidentally break things =
for GENERIC and GENERIC-like kernel configs. As long as we disallow =
wildcard locators for indirect config on these busses, I think we will =
be ok. Maybe I=E2=80=99m just being paranoid.
> DT is nifty, but it=E2=80=99s leading to an annoying bifurcation of =
device configuration information. The kernel config file was the =
traditional way to specify this stuff=E2=80=A6 but if we=E2=80=99re =
going to say =E2=80=9Chey, the right thing to do on FDT platforms is to =
provide your own DT snippets for whatever hardware you=E2=80=99re adding =
to your prototyping platform=E2=80=9D, then we need a way to make that =
easy (or discoverable, or maybe both).
Cheers,
Jared=
From: Jared McNeill <jmcneill@invisible.ca>
To: Jason Thorpe <thorpej@me.com>
Cc: gnats-bugs@NetBSD.org,
Jason Thorpe <thorpej@NetBSD.org>,
port-arm-maintainer@netbsd.org,
netbsd-bugs@netbsd.org,
gnats-admin@netbsd.org
Subject: Re: port-arm/53171 (Broadcom (and other ARM SoC i2c drivers)
mis-handle indirect configuration of devices)
Date: Mon, 30 Apr 2018 17:21:10 -0300
--Apple-Mail=_F5DC7B54-F600-400B-9B47-490068413EB3
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
charset=utf-8
> On Apr 30, 2018, at 5:16 PM, Jared McNeill <jmcneill@invisible.ca> =
wrote:
>=20
> I thought we had some drivers that relied on this but when I went to =
look I couldn=E2=80=99t find any..
Ok I found one! It=E2=80=99s clearly broken..
https://nxr.netbsd.org/xref/src/sys/dev/i2c/ibmhawk.c#96 =
<https://nxr.netbsd.org/xref/src/sys/dev/i2c/ibmhawk.c#96>
--Apple-Mail=_F5DC7B54-F600-400B-9B47-490068413EB3
Content-Transfer-Encoding: quoted-printable
Content-Type: text/html;
charset=utf-8
<html><head><meta http-equiv=3D"Content-Type" content=3D"text/html; =
charset=3Dutf-8"></head><body style=3D"word-wrap: break-word; =
-webkit-nbsp-mode: space; line-break: after-white-space;" class=3D""><br =
class=3D""><div><blockquote type=3D"cite" class=3D""><div class=3D"">On =
Apr 30, 2018, at 5:16 PM, Jared McNeill <<a =
href=3D"mailto:jmcneill@invisible.ca" =
class=3D"">jmcneill@invisible.ca</a>> wrote:</div><br =
class=3D"Apple-interchange-newline"><div class=3D""><span =
style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: =
12px; font-style: normal; font-variant-caps: normal; font-weight: =
normal; letter-spacing: normal; text-align: start; text-indent: 0px; =
text-transform: none; white-space: normal; word-spacing: 0px; =
-webkit-text-stroke-width: 0px; text-decoration: none; float: none; =
display: inline !important;" class=3D"">I thought we had some drivers =
that relied on this but when I went to look I couldn=E2=80=99t find =
any..</span><br style=3D"caret-color: rgb(0, 0, 0); font-family: =
Helvetica; font-size: 12px; font-style: normal; font-variant-caps: =
normal; font-weight: normal; letter-spacing: normal; text-align: start; =
text-indent: 0px; text-transform: none; white-space: normal; =
word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: =
none;" class=3D""></div></blockquote></div><br class=3D""><div =
class=3D"">Ok I found one! It=E2=80=99s clearly broken..</div><div =
class=3D""><br class=3D""></div><div class=3D""><a =
href=3D"https://nxr.netbsd.org/xref/src/sys/dev/i2c/ibmhawk.c#96" =
class=3D"">https://nxr.netbsd.org/xref/src/sys/dev/i2c/ibmhawk.c#96</a></d=
iv><div class=3D""><br class=3D""></div><div class=3D""><br =
class=3D""></div></body></html>=
--Apple-Mail=_F5DC7B54-F600-400B-9B47-490068413EB3--
From: Jared McNeill <jmcneill@invisible.ca>
To: Jason Thorpe <thorpej@me.com>
Cc: gnats-bugs@NetBSD.org,
Jason Thorpe <thorpej@NetBSD.org>,
port-arm-maintainer@netbsd.org,
netbsd-bugs@netbsd.org,
gnats-admin@netbsd.org
Subject: Re: port-arm/53171 (Broadcom (and other ARM SoC i2c drivers)
mis-handle indirect configuration of devices)
Date: Mon, 30 Apr 2018 17:38:33 -0300
--Apple-Mail=_4FA40B50-3E1E-4320-993F-8BB7A77FD40B
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
charset=utf-8
On Apr 30, 2018, at 5:21 PM, Jared McNeill <jmcneill@invisible.ca> =
wrote:
>=20
>=20
>> On Apr 30, 2018, at 5:16 PM, Jared McNeill <jmcneill@invisible.ca =
<mailto:jmcneill@invisible.ca>> wrote:
>>=20
>> I thought we had some drivers that relied on this but when I went to =
look I couldn=E2=80=99t find any..
>=20
> Ok I found one! It=E2=80=99s clearly broken..
>=20
> https://nxr.netbsd.org/xref/src/sys/dev/i2c/ibmhawk.c#96 =
<https://nxr.netbsd.org/xref/src/sys/dev/i2c/ibmhawk.c#96>
I found three more, and apparently I=E2=80=99m the idiot responsible for =
all three of them. Checked in fixes for titemp, tcakp, and act8846pm.
--Apple-Mail=_4FA40B50-3E1E-4320-993F-8BB7A77FD40B
Content-Transfer-Encoding: quoted-printable
Content-Type: text/html;
charset=utf-8
<html><head><meta http-equiv=3D"Content-Type" content=3D"text/html; =
charset=3Dutf-8"></head><body style=3D"word-wrap: break-word; =
-webkit-nbsp-mode: space; line-break: after-white-space;" class=3D"">On =
Apr 30, 2018, at 5:21 PM, Jared McNeill <<a =
href=3D"mailto:jmcneill@invisible.ca" =
class=3D"">jmcneill@invisible.ca</a>> wrote:<br =
class=3D""><div><blockquote type=3D"cite" class=3D""><br =
class=3D"Apple-interchange-newline"><div class=3D""><meta =
http-equiv=3D"Content-Type" content=3D"text/html; charset=3Dutf-8" =
class=3D""><div style=3D"word-wrap: break-word; -webkit-nbsp-mode: =
space; line-break: after-white-space;" class=3D""><br class=3D""><div =
class=3D""><blockquote type=3D"cite" class=3D""><div class=3D"">On Apr =
30, 2018, at 5:16 PM, Jared McNeill <<a =
href=3D"mailto:jmcneill@invisible.ca" =
class=3D"">jmcneill@invisible.ca</a>> wrote:</div><br =
class=3D"Apple-interchange-newline"><div class=3D""><span =
style=3D"caret-color: rgb(0, 0, 0); font-family: Helvetica; font-size: =
12px; font-style: normal; font-variant-caps: normal; font-weight: =
normal; letter-spacing: normal; text-align: start; text-indent: 0px; =
text-transform: none; white-space: normal; word-spacing: 0px; =
-webkit-text-stroke-width: 0px; text-decoration: none; float: none; =
display: inline !important;" class=3D"">I thought we had some drivers =
that relied on this but when I went to look I couldn=E2=80=99t find =
any..</span><br style=3D"caret-color: rgb(0, 0, 0); font-family: =
Helvetica; font-size: 12px; font-style: normal; font-variant-caps: =
normal; font-weight: normal; letter-spacing: normal; text-align: start; =
text-indent: 0px; text-transform: none; white-space: normal; =
word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: =
none;" class=3D""></div></blockquote></div><br class=3D""><div =
class=3D"">Ok I found one! It=E2=80=99s clearly broken..</div><div =
class=3D""><br class=3D""></div><div class=3D""><a =
href=3D"https://nxr.netbsd.org/xref/src/sys/dev/i2c/ibmhawk.c#96" =
class=3D"">https://nxr.netbsd.org/xref/src/sys/dev/i2c/ibmhawk.c#96</a></d=
iv></div></div></blockquote><br class=3D""></div><div>I found three =
more, and apparently I=E2=80=99m the idiot responsible for all three of =
them. Checked in fixes for titemp, tcakp, and <span =
style=3D"white-space: pre-wrap; background-color: rgb(255, 255, 255);" =
class=3D"">act8846pm.</span></div><div class=3D""><br class=3D""></div><br=
class=3D""></body></html>=
--Apple-Mail=_4FA40B50-3E1E-4320-993F-8BB7A77FD40B--
From: Jason Thorpe <thorpej@me.com>
To: Jared McNeill <jmcneill@invisible.ca>
Cc: gnats-bugs@NetBSD.org, Jason Thorpe <thorpej@NetBSD.org>,
port-arm-maintainer@netbsd.org, netbsd-bugs@netbsd.org, gnats-admin@netbsd.org
Subject: Re: port-arm/53171 (Broadcom (and other ARM SoC i2c drivers)
mis-handle indirect configuration of devices)
Date: Mon, 30 Apr 2018 19:12:31 -0700
> On Apr 30, 2018, at 1:38 PM, Jared McNeill <jmcneill@invisible.ca> =
wrote:
>=20
> On Apr 30, 2018, at 5:21 PM, Jared McNeill <jmcneill@invisible.ca> =
wrote:
>>=20
>>=20
>>> On Apr 30, 2018, at 5:16 PM, Jared McNeill <jmcneill@invisible.ca> =
wrote:
>>>=20
>>> I thought we had some drivers that relied on this but when I went to =
look I couldn=E2=80=99t find any..
>>=20
>> Ok I found one! It=E2=80=99s clearly broken..
>>=20
>> https://nxr.netbsd.org/xref/src/sys/dev/i2c/ibmhawk.c#96
>=20
> I found three more, and apparently I=E2=80=99m the idiot responsible =
for all three of them. Checked in fixes for titemp, tcakp, and =
act8846pm.
Hey man, I wasn=E2=80=99t going to say anything=E2=80=A6 :-)
Anyway, I double-checked, and iic_search() already disallows =
IICCF_ADDR_DEFAULT, so that=E2=80=99s good. =E2=80=9Cibmhawk=E2=80=9D =
is particularly egregious. It looks like it=E2=80=99s expected at 0x37, =
so we can filter it out there, I suppose. It would be even better if =
there were an ACPI node for it, or something.
(Speaking of ACPI =E2=80=94 geez, that thing is just crying out to be =
converted to FDT at run-time :-) )
>=20
>=20
-- thorpej
State-Changed-From-To: open->closed
State-Changed-By: thorpej@NetBSD.org
State-Changed-When: Wed, 09 May 2018 02:54:22 +0000
State-Changed-Why:
Patch committed.
From: "Jason R Thorpe" <thorpej@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/53171 CVS commit: src/sys/arch/arm
Date: Wed, 9 May 2018 02:53:00 +0000
Module Name: src
Committed By: thorpej
Date: Wed May 9 02:53:00 UTC 2018
Modified Files:
src/sys/arch/arm/broadcom: bcm2835_bsc.c
src/sys/arch/arm/nvidia: tegra_i2c.c
src/sys/arch/arm/samsung: exynos_i2c.c
src/sys/arch/arm/sunxi: sunxi_rsb.c sunxi_twi.c
Log Message:
If we don't get informed (via device properties) of child I2C devices,
don't assign an empty array to iba.iba_child_devices, as it will prevent
indirect configuration of the I2C bus from occurring.
Tested on Raspberry Pi (bcm2835), identical logical fix replicated
(and compile-tested) elsewhere.
PR port-arm/53171
To generate a diff of this commit:
cvs rdiff -u -r1.10 -r1.11 src/sys/arch/arm/broadcom/bcm2835_bsc.c
cvs rdiff -u -r1.16 -r1.17 src/sys/arch/arm/nvidia/tegra_i2c.c
cvs rdiff -u -r1.13 -r1.14 src/sys/arch/arm/samsung/exynos_i2c.c
cvs rdiff -u -r1.1 -r1.2 src/sys/arch/arm/sunxi/sunxi_rsb.c
cvs rdiff -u -r1.8 -r1.9 src/sys/arch/arm/sunxi/sunxi_twi.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
>Unformatted:
(Contact us)
$NetBSD: query-full-pr,v 1.43 2018/01/16 07:36:43 maya Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2017
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.
Home