Date: Fri, 24 Apr 2020 09:11:52 -0600 From: Warner Losh <imp@bsdimp.com> To: Andriy Gapon <avg@freebsd.org>, John Baldwin <jhb@freebsd.org> Cc: src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>, svn-src-head <svn-src-head@freebsd.org> Subject: Re: svn commit: r360241 - head/sys/dev/ichiic Message-ID: <CANCZdfqJaBm=Jx43jU5%2BiB1KVx3f1aoSUTDB3U6z9QnPV_q6uQ@mail.gmail.com> In-Reply-To: <32586115-2c82-2497-ba4c-b3471887518d@FreeBSD.org> References: <202004240749.03O7nMSc066344@repo.freebsd.org> <CANCZdfok8rf3SWEtKA_ZHVAgPWHCzOM-95w2pP9UTTLn-9995w@mail.gmail.com> <32586115-2c82-2497-ba4c-b3471887518d@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Apr 24, 2020 at 8:47 AM Andriy Gapon <avg@freebsd.org> wrote: > On 24/04/2020 17:29, Warner Losh wrote: > > > > > > On Fri, Apr 24, 2020 at 1:49 AM Andriy Gapon <avg@freebsd.org > > <mailto:avg@freebsd.org>> wrote: > > > > Author: avg > > Date: Fri Apr 24 07:49:21 2020 > > New Revision: 360241 > > URL: https://svnweb.freebsd.org/changeset/base/360241 > > > > Log: > > ig4: ensure that drivers always attach in correct order > > > > Use DRIVER_MODULE_ORDERED(SI_ORDER_ANY) so that ig4's ACPI > attachment > > happens after iicbus and acpi_iicbus drivers are registered. > > > > I have seen a problem where iicbus attached under ig4 instead of > > acpi_iicbus when ig4.ko was loaded with kldload. I believe that > that > > happened because ig4 driver was a first driver to register, it > attached > > and created an iicbus child. Then iicbus driver was registered > and, > > since it was the only driver that could attach to the iicbus child > > device, it did exactly that. After that acpi_iicbus driver was > > registered. It would be able to attach to the iicbus device, but > it was > > already attached, so nothing happened. > > > > > > Can you post more details of which devices are affected? From the > description > > and the patch, I don't see how this could fix things. > > I think I listed them all: ig4iic with acpi attachment, iicbus and > acpi_iicbus. > acpi > \--- ig4iic > \---- iicbus vs acpi_iicbus > > I tried to explain the problem and the fix in the commit message. If you > want > to discuss any specifics, let's continue with specifics. If there is > anything > unclear in my explanation, I can clarify, but I need to understand what > needs to > be clarified. > That won't fix the stated problem. If changing the module order fixes something, it's the wrong fix. Which is why I asked to make sure I understood the issue (it was unclear if it was at the level indicated, or for the children of the iicbus). iicbus returns BUS_PROBE_GENERIC (-100) from its probe routine, while acpi_iicbus returns BUS_PROBE_DEFAULT (-20). This means that acpi_iicbus should always win. So something else is going on. We don't specify order on other devices except for some weird, special needs drivers (this is not such a driver). Unless I'm mistaken, the real problem is that the following line is missing instead. MODULE_DEPEND(ig4iic, acpi_iicbus, 1, 1, 1); which makes the module dependencies explicit. Can you add that to ig4_acpi.c, revert your change and see if that works? It will ensure that the acpi_iicbus driver is loaded first. If things break because it isn't, it sounds like a hard dependency for ig4. Since we're already pulling in acpi, that doesn't seem unreasonable to me. Warner > > > MFC after: 2 weeks > > > > Modified: > > head/sys/dev/ichiic/ig4_acpi.c > > > > Modified: head/sys/dev/ichiic/ig4_acpi.c > > > ============================================================================== > > --- head/sys/dev/ichiic/ig4_acpi.c Fri Apr 24 05:05:58 2020 > > > (r360240) > > +++ head/sys/dev/ichiic/ig4_acpi.c Fri Apr 24 07:49:21 2020 > > > (r360241) > > @@ -192,5 +192,6 @@ static driver_t ig4iic_acpi_driver = { > > sizeof(struct ig4iic_softc), > > }; > > > > -DRIVER_MODULE(ig4iic, acpi, ig4iic_acpi_driver, ig4iic_devclass, 0, > 0); > > +DRIVER_MODULE_ORDERED(ig4iic, acpi, ig4iic_acpi_driver, > ig4iic_devclass, 0, 0, > > + SI_ORDER_ANY); > > MODULE_DEPEND(ig4iic, acpi, 1, 1, 1); > > > > > -- > Andriy Gapon >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfqJaBm=Jx43jU5%2BiB1KVx3f1aoSUTDB3U6z9QnPV_q6uQ>