From patchwork Mon May 8 21:32:55 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Loc Ho X-Patchwork-Id: 9716737 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 88FCC6035D for ; Mon, 8 May 2017 21:33:00 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7760B20564 for ; Mon, 8 May 2017 21:33:00 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6A0E1265B9; Mon, 8 May 2017 21:33:00 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A0E6220564 for ; Mon, 8 May 2017 21:32:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751421AbdEHVc6 (ORCPT ); Mon, 8 May 2017 17:32:58 -0400 Received: from mail-it0-f48.google.com ([209.85.214.48]:33562 "EHLO mail-it0-f48.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750839AbdEHVc5 (ORCPT ); Mon, 8 May 2017 17:32:57 -0400 Received: by mail-it0-f48.google.com with SMTP id w68so10598460itc.0 for ; Mon, 08 May 2017 14:32:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=apm.com; s=apm; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=Iv9502ORSpXoutuKr5aC/BRkoiR1kXHGhXhf7K+WtsY=; b=pSeykIfSuukKXhtZA5RlHVOzM3kG9YLIFOyF+TGLUeM99adkPypS4em17gHUWbwucI DycCSbvJTy1MP8V/cX4nBRgXgmLf6SHSTYAAAHr2ZEXPWJANngWSi3/RA232zsMMf/3Y SAbCgllxTA3gPVJpfoCR+yHqwte5NEh4Db5Jw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=Iv9502ORSpXoutuKr5aC/BRkoiR1kXHGhXhf7K+WtsY=; b=C6zyewt0AXklNEHVTCi/qju+coOoSnH/d6Q/CCBIwRzlnIHSeX7F36WdmqnksZoSC8 7LvMUiPGLU+XW0nYIaPoCBB65jrlKDFFhecuVpwYuzWRSPIRpofc/+kMZdNsduFSO3r/ hF6w2+wrbTp91v8YEFbYLKpTHRQV2LX8zwRAS4KOymm3my9o3lS4w8q7jRT9qVflV/ol /CwhNluVjJ0NH39+JOv9vRkGaqBFS0ejwsW9sYm0B9VmbB5ukuST3ioMj2/X2OEUAsZ7 dq5tgi15WXpisnD/vnjNSlKLm+AeiC+Mhchlid2kbwRMIbn7wVHQDapbviNZJGChBPK+ yMHw== X-Gm-Message-State: AN3rC/7JKjxBXHlA2azw9v9CsShQKaKPAjrtBs1gRcTp77MUzuhVb5AR WUpvBL0Zr8zVlQJ6cB6GPMYndykGG0hz X-Received: by 10.36.40.9 with SMTP id h9mr23747615ith.13.1494279176450; Mon, 08 May 2017 14:32:56 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.19.203 with HTTP; Mon, 8 May 2017 14:32:55 -0700 (PDT) In-Reply-To: <20170508212347.GA3601@broadcom.com> References: <1493910330-17913-1-git-send-email-jon.mason@broadcom.com> <4e11bb2b-7cec-6a7b-ca2b-88be31a98b7d@redhat.com> <20170508212347.GA3601@broadcom.com> From: Loc Ho Date: Mon, 8 May 2017 14:32:55 -0700 Message-ID: Subject: Re: [PATCH] ACPI: SPCR: Use access width to determine mmio usage To: Jon Mason Cc: Jon Masters , Rafael Wysocki , Len Brown , Robert Moore , Lv Zheng , linux-acpi@vger.kernel.org, Linux Kernel Mailing List , devel@acpica.org, BCM Kernel Feedback Sender: linux-acpi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Jon, >> >>>>>> The current SPCR code does not check the access width of the mmio, and >> >>>>>> uses a default of 8bit register accesses. This prevents devices that >> >>>>>> only do 16 or 32bit register accesses from working. By simply checking >> >>>>>> this field and setting the mmio string appropriately, this issue can be >> >>>>>> corrected. To prevent any legacy issues, the code will default to 8bit >> >>>>>> accesses if the value is anything but 16 or 32. >> >>>>> >> >>>>> Thanks for this. Just as an FYI I've a running discussion with Microsoft >> >>>>> about defining additional UART subtypes in the DBG2 for special case >> >>>>> UARTs. Specifically, I want to address AppliedMicro's special 8250 dw IP >> >>>>> that also has a non-standard clock. At this time, there is general >> >>>>> agreement to use the access width for some cases rather than defining >> >>>>> yet more subtypes - so your patch is good. >> >>>>> >> >>>>> Loc/Applied: please track this thread, incorporate feedback, and also >> >>>>> track the other general recent discussions of 8250 dw from this week. >> >>>> >> >>>> Thanks for forward me this patch. This patch does not work with X-Gene >> >>>> v1 and v2 SoC's. As BIOS SPCR encodes these info as: >> >>>> >> >>>> Bit Width: 32 >> >>>> Bit Offset: 0 >> >>>> Encoded Access Width: 01 (Byte Access) >> >>>> >> >>>> With this patch, it would use the "mmio" instead the "mmio32" as with >> >>>> this patch - https://patchwork.kernel.org/patch/9460959 >> >>> >> >>> I think this is why we need the DBG2 subtype for Applied X-Gene1. I'm >> >>> hoping the update to the SPCR/DBG2 spec is done soon. >> >> >> >> We can't rely on the BIOS change to support this new subtype as we >> >> have system that is already in production deployment. When these >> >> system upgrade to new version of the OS (stock, RHELSA, or whatever), >> >> they will break. We need the patch from >> >> https://patchwork.kernel.org/patch/9460959/ rolled upstream. >> > >> > There is no reason why the patch you reference cannot co-exist with >> > the one I am submitting here. In this case, my patch would set it to >> > mmio, then the patch you link above would reset it to mmio32. >> > Personally, I would recommend a big, fat comment on why this extra >> > step is necessary, but it should work as desired. Alternatively, we >> > could add some kind of quirk library (similar to >> > qdf2400_erratum_44_present) where the OEM/OEM Table ID is referenced >> > and workaround applied. Thoughts? >> >> That's was my first version but after seeing both versions, I think >> they are better solution as it works for more SoC's than just our. As >> you had suggested, we should apply your patch and >> https://patchwork.kernel.org/patch/9460959. The third patch - >> https://patchwork.kernel.org/patch/9462183/ - conflicts with your. >> >> Summary: >> 1. Applied your - https://lkml.org/lkml/2017/5/4/450 >> 2. Applied this one - https://patchwork.kernel.org/patch/9460959/ >> >> -Loc > > What if we simply applied the following (100% untested) patch to add > the quirk framework I was suggesting? It can be applied on top of the > patch I submitted previously. It is a bit more complex that this simple patch. How about this one (my original version). As for Jon Master question on McDivitt, not sure what they use for the ACPI table for SPCR. If they used our reference, then this might work for them too. This version would limit to just the existent firmware or until the SPCR table gets changed. tty: 8250: Workaround for APM X-Gene 8250 UART 32-alignment errata APM X-Gene verion 1 and 2 have an 8250 UART with its register aligned to 32-bit. The SPCR always assumes fully compatible 8250. This causes no console with ACPI boot as the console will not match X-Gene UART port due to the lack of mmio32 option. Signed-off-by: Loc Ho --- drivers/acpi/spcr.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) * @@ -115,6 +134,8 @@ int __init parse_spcr(bool earlycon) if (qdf2400_erratum_44_present(&table->header)) uart = "qdf2400_e44"; + if (xgene_8250_erratum_present(table)) + iotype = "mmio32"; snprintf(opts, sizeof(opts), "%s,%s,0x%llx,%d", uart, iotype, table->serial_port.address, baud_rate); -Loc -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c index 3afa8c1..77b45a0 100644 --- a/drivers/acpi/spcr.c +++ b/drivers/acpi/spcr.c @@ -36,6 +36,25 @@ static bool qdf2400_erratum_44_present(struct acpi_table_header *h) return false; } +/* + * APM X-Gene v1 and v2 UART hardware is an 16550 like device but has its + * register aligned to 32-bit. This function detects this errata condition. + */ +static bool xgene_8250_erratum_present(struct acpi_table_spcr *tb) +{ + if (tb->interface_type != ACPI_DBG2_16550_COMPATIBLE) + return false; + + if (memcmp(tb->header.oem_id, "APMC0D", ACPI_OEM_ID_SIZE)) + return false; + + if (!memcmp(tb->header.oem_table_id, "XGENESPC", + ACPI_OEM_TABLE_ID_SIZE) && tb->header.oem_revision == 0) + return true; + + return false; +} + /** * parse_spcr() - parse ACPI SPCR table and add preferred console