Re: port-arm/53171 (Broadcom (and other ARM SoC i2c drivers) mis-handle indirect configuration of devices)

[Jared McNeill <jmcneill%invisible.ca@localhost>]

The following reply was made to PR port-arm/53171; it has been noted by GNATS.

From: Jared McNeill <jmcneill%invisible.ca@localhost>
To: Jason Thorpe <thorpej%me.com@localhost>
Cc: gnats-bugs%NetBSD.org@localhost,
 Jason Thorpe <thorpej%NetBSD.org@localhost>,
 port-arm-maintainer%netbsd.org@localhost,
 netbsd-bugs%netbsd.org@localhost,
 gnats-admin%netbsd.org@localhost
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@localhost> 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=