From owner-freebsd-current@FreeBSD.ORG Tue Jul 12 19:50:21 2005 Return-Path: X-Original-To: freebsd-current@freebsd.org Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 6B60716A41C for ; Tue, 12 Jul 2005 19:50:21 +0000 (GMT) (envelope-from jhb@FreeBSD.org) Received: from mv.twc.weather.com (mv.twc.weather.com [65.212.71.225]) by mx1.FreeBSD.org (Postfix) with ESMTP id 0A12643D45 for ; Tue, 12 Jul 2005 19:50:20 +0000 (GMT) (envelope-from jhb@FreeBSD.org) Received: from [10.50.40.201] (Not Verified[65.202.103.25]) by mv.twc.weather.com with NetIQ MailMarshal (v6, 0, 3, 8) id ; Tue, 12 Jul 2005 16:04:26 -0400 From: John Baldwin To: freebsd-current@freebsd.org Date: Tue, 12 Jul 2005 15:50:30 -0400 User-Agent: KMail/1.8 References: <4.3.2.7.2.20050711160009.01f96420@mail.qconline.com> <4.3.2.7.2.20050712134818.0202b0d0@mail.qconline.com> In-Reply-To: <4.3.2.7.2.20050712134818.0202b0d0@mail.qconline.com> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200507121550.31852.jhb@FreeBSD.org> Cc: Harry Coin , Nate Lawson Subject: Re: mss.c pcm fix to ' attach returned 6 ' load failure for v5.x acpi and up X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 12 Jul 2005 19:50:21 -0000 On Tuesday 12 July 2005 03:19 pm, Harry Coin wrote: > At 10:44 AM 7/12/2005 -0400, John Baldwin wrote: > >Now, when ACPI is enabled, the > >pnpmss driver doesn't probe the device IIRC? > > Both the mss_probe and pnpmss_probe routine are called in the ACPI case and > in the ACPI disabled case. Let me help by providing proof from some > edited short form commented dmesg excerpts from upstream postings using > unmodified-except-for-debug-writes mss.c: I understand that. My last patch (which you apparently didn't test), makes the non-pnp mss probe not be used by ACPI ever. > The question I can't get out of my head is this: When I change the mss.c > code to use the routine the manual says non-PNP devices are supposed to > use: ISA_PNP_PROBE -- all bootup operations are correct in both the ACPI > and non ACPI case. > > Why doesn't that count as 'fixed, update mss.c and lets move on?'. Because there are 20-30 _other_ drivers that use isa_get_logicalid(), and if there's a bug in isa_get_logicalid() then it's going to break _all_ those drivers, and I'd rather fix ACPI than have to wade through a whole bunch of different drivers. Also, having to use an empty PNP_PROBE list is a rather lame solution. > Clearly the present code uses another function that doesn't work in the > ACPI case. Shouldn't the investigation focus on what ISA_PNP_PROBE knows > that the other call doesn't? ISA_PNP_PROBE in mss_probe works and matches > the docs, the isa_get_whatziz doesn't work and is not accepted as correct > practice in the docs. Maybe the doc writer has the answer. At any > rate I don't get why this further investigation when an apparently > 'canonical' practice works. That, or the architecture manual needs > changing to mention this other way to avoid wrongful probing by non-PNP isa > drivers when called by ACPI or isa0. The problem is that ISA_PNP_PROBE() just uses isa_get_logicalid() (basically). Here's what the two functions look like in ACPI: /* Probe _HID and _CID for compatible ISA PNP ids. */ static uint32_t acpi_isa_get_logicalid(device_t dev) { ACPI_DEVICE_INFO *devinfo; ACPI_BUFFER buf; ACPI_HANDLE h; ACPI_STATUS error; u_int32_t pnpid; int i; ACPI_FUNCTION_TRACE((char *)(uintptr_t)__func__); pnpid = 0; buf.Pointer = NULL; buf.Length = ACPI_ALLOCATE_BUFFER; /* Fetch and validate the HID. */ if ((h = acpi_get_handle(dev)) == NULL) goto out; error = AcpiGetObjectInfo(h, &buf); if (ACPI_FAILURE(error)) goto out; devinfo = (ACPI_DEVICE_INFO *)buf.Pointer; if ((devinfo->Valid & ACPI_VALID_HID) != 0) pnpid = PNP_EISAID(devinfo->HardwareId.Value); out: if (buf.Pointer != NULL) AcpiOsFree(buf.Pointer); return_VALUE (pnpid); } static int acpi_isa_pnp_probe(device_t bus, device_t child, struct isa_pnp_id *ids) { int result, cid_count, i; uint32_t lid, cids[8]; ACPI_FUNCTION_TRACE((char *)(uintptr_t)__func__); /* * ISA-style drivers attached to ACPI may persist and * probe manually if we return ENOENT. We never want * that to happen, so don't ever return it. */ result = ENXIO; /* Scan the supplied IDs for a match */ lid = acpi_isa_get_logicalid(child); cid_count = acpi_isa_get_compatid(child, cids, 8); while (ids && ids->ip_id) { if (lid == ids->ip_id) { result = 0; goto out; } for (i = 0; i < cid_count; i++) { if (cids[i] == ids->ip_id) { result = 0; goto out; } } ids++; } out: if (result == 0 && ids->ip_desc) device_set_desc(child, ids->ip_desc); return_VALUE (result); } The other thing to keep in mind here is that your soundcard isn't an ACPI device that this code would know about, it's an ISA PnP device which is completely different (hung off of isa0 for one), and that what is happening is that ACPI devices like acpi_tz0 (thermal zone) are being probed by mss_probe() and it's not rejecting them because it thinks logicalid() is zero. Hmm, I bet I know what it is actually. ACPI has HIDs that are strings like ACPI003 for things like thermal zones and batteries rather than an ESIA encoded PNPxxx string. So even though there is a valid HID, PNP_EISAID() ends up returning 0. *sigh* The other thing is that the non-pnp mss probe simply does not need to be hooked up to the ACPI bus at all since ACPI will only enumerate PnP devices. I would appreciate it if you would try the simple patch I posted previously to remove the one line from mss.c that hooks the non-pnp mss driver up to acpi. -- John Baldwin <>< http://www.FreeBSD.org/~jhb/ "Power Users Use the Power to Serve" = http://www.FreeBSD.org