| Message ID | 1493910330-17913-1-git-send-email-jon.mason@broadcom.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
Jon, You might want to take a look at using the acpi_read and acpi_write ACPICA interfaces that accept a GAS structure and handle the access width (etc.) automatically. Bob > -----Original Message----- > From: Jon Mason [mailto:jon.mason@broadcom.com] > Sent: Thursday, May 4, 2017 8:06 AM > To: Rafael Wysocki <rjw@rjwysocki.net>; Len Brown <lenb@kernel.org>; > Moore, Robert <robert.moore@intel.com>; Zheng, Lv <lv.zheng@intel.com> > Cc: linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org; > devel@acpica.org; bcm-kernel-feedback-list@broadcom.com > Subject: [PATCH] ACPI: SPCR: Use access width to determine mmio usage > > 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. > > Signed-off-by: Jon Mason <jon.mason@broadcom.com> > --- > drivers/acpi/spcr.c | 18 ++++++++++++++++-- > include/acpi/acrestyp.h | 7 +++++++ > 2 files changed, 23 insertions(+), 2 deletions(-) > > diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c index > 01c9466..11233f6 100644 > --- a/drivers/acpi/spcr.c > +++ b/drivers/acpi/spcr.c > @@ -74,8 +74,22 @@ int __init parse_spcr(bool earlycon) > goto done; > } > > - iotype = table->serial_port.space_id == > ACPI_ADR_SPACE_SYSTEM_MEMORY ? > - "mmio" : "io"; > + if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) { > + switch (table->serial_port.access_width) { > + default: > + pr_err("Unexpected SPCR Access Width. Defaulting to > byte size\n"); > + case ACPI_ACCESS_SIZE_BYTE: > + iotype = "mmio"; > + break; > + case ACPI_ACCESS_SIZE_WORD: > + iotype = "mmio16"; > + break; > + case ACPI_ACCESS_SIZE_DWORD: > + iotype = "mmio32"; > + break; > + } > + } else > + iotype = "io"; > > switch (table->interface_type) { > case ACPI_DBG2_ARM_SBSA_32BIT: > diff --git a/include/acpi/acrestyp.h b/include/acpi/acrestyp.h index > f0f7403..781cb55 100644 > --- a/include/acpi/acrestyp.h > +++ b/include/acpi/acrestyp.h > @@ -372,6 +372,13 @@ struct acpi_resource_generic_register { > u64 address; > }; > > +/* Generic Address Space Access Sizes */ > +#define ACPI_ACCESS_SIZE_UNDEFINED 0 > +#define ACPI_ACCESS_SIZE_BYTE 1 > +#define ACPI_ACCESS_SIZE_WORD 2 > +#define ACPI_ACCESS_SIZE_DWORD 3 > +#define ACPI_ACCESS_SIZE_QWORD 4 > + > struct acpi_resource_gpio { > u8 revision_id; > u8 connection_type; > -- > 2.7.4 -- 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
On Thu, May 4, 2017 at 12:28 PM, Moore, Robert <robert.moore@intel.com> wrote: > Jon, > > You might want to take a look at using the acpi_read and acpi_write ACPICA interfaces that accept a GAS structure and handle the access width (etc.) automatically. > > Bob This function in SPCR is using common code for the early console. So the acpi_read/write would not be usable in that common code. Thanks, Jon > > >> -----Original Message----- >> From: Jon Mason [mailto:jon.mason@broadcom.com] >> Sent: Thursday, May 4, 2017 8:06 AM >> To: Rafael Wysocki <rjw@rjwysocki.net>; Len Brown <lenb@kernel.org>; >> Moore, Robert <robert.moore@intel.com>; Zheng, Lv <lv.zheng@intel.com> >> Cc: linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org; >> devel@acpica.org; bcm-kernel-feedback-list@broadcom.com >> Subject: [PATCH] ACPI: SPCR: Use access width to determine mmio usage >> >> 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. >> >> Signed-off-by: Jon Mason <jon.mason@broadcom.com> >> --- >> drivers/acpi/spcr.c | 18 ++++++++++++++++-- >> include/acpi/acrestyp.h | 7 +++++++ >> 2 files changed, 23 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c index >> 01c9466..11233f6 100644 >> --- a/drivers/acpi/spcr.c >> +++ b/drivers/acpi/spcr.c >> @@ -74,8 +74,22 @@ int __init parse_spcr(bool earlycon) >> goto done; >> } >> >> - iotype = table->serial_port.space_id == >> ACPI_ADR_SPACE_SYSTEM_MEMORY ? >> - "mmio" : "io"; >> + if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) { >> + switch (table->serial_port.access_width) { >> + default: >> + pr_err("Unexpected SPCR Access Width. Defaulting to >> byte size\n"); >> + case ACPI_ACCESS_SIZE_BYTE: >> + iotype = "mmio"; >> + break; >> + case ACPI_ACCESS_SIZE_WORD: >> + iotype = "mmio16"; >> + break; >> + case ACPI_ACCESS_SIZE_DWORD: >> + iotype = "mmio32"; >> + break; >> + } >> + } else >> + iotype = "io"; >> >> switch (table->interface_type) { >> case ACPI_DBG2_ARM_SBSA_32BIT: >> diff --git a/include/acpi/acrestyp.h b/include/acpi/acrestyp.h index >> f0f7403..781cb55 100644 >> --- a/include/acpi/acrestyp.h >> +++ b/include/acpi/acrestyp.h >> @@ -372,6 +372,13 @@ struct acpi_resource_generic_register { >> u64 address; >> }; >> >> +/* Generic Address Space Access Sizes */ >> +#define ACPI_ACCESS_SIZE_UNDEFINED 0 >> +#define ACPI_ACCESS_SIZE_BYTE 1 >> +#define ACPI_ACCESS_SIZE_WORD 2 >> +#define ACPI_ACCESS_SIZE_DWORD 3 >> +#define ACPI_ACCESS_SIZE_QWORD 4 >> + >> struct acpi_resource_gpio { >> u8 revision_id; >> u8 connection_type; >> -- >> 2.7.4 > -- 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
Hi, > From: Jon Mason [mailto:jon.mason@broadcom.com] > Sent: Thursday, May 4, 2017 11:06 PM > Subject: [PATCH] ACPI: SPCR: Use access width to determine mmio usage > > 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. > > Signed-off-by: Jon Mason <jon.mason@broadcom.com> > --- > drivers/acpi/spcr.c | 18 ++++++++++++++++-- > include/acpi/acrestyp.h | 7 +++++++ > 2 files changed, 23 insertions(+), 2 deletions(-) > > diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c > index 01c9466..11233f6 100644 > --- a/drivers/acpi/spcr.c > +++ b/drivers/acpi/spcr.c > @@ -74,8 +74,22 @@ int __init parse_spcr(bool earlycon) > goto done; > } > > - iotype = table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY ? > - "mmio" : "io"; > + if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) { > + switch (table->serial_port.access_width) { > + default: > + pr_err("Unexpected SPCR Access Width. Defaulting to byte size\n"); > + case ACPI_ACCESS_SIZE_BYTE: > + iotype = "mmio"; > + break; > + case ACPI_ACCESS_SIZE_WORD: > + iotype = "mmio16"; > + break; > + case ACPI_ACCESS_SIZE_DWORD: > + iotype = "mmio32"; > + break; > + } > + } else > + iotype = "io"; > > switch (table->interface_type) { > case ACPI_DBG2_ARM_SBSA_32BIT: > diff --git a/include/acpi/acrestyp.h b/include/acpi/acrestyp.h > index f0f7403..781cb55 100644 > --- a/include/acpi/acrestyp.h > +++ b/include/acpi/acrestyp.h > @@ -372,6 +372,13 @@ struct acpi_resource_generic_register { > u64 address; > }; > > +/* Generic Address Space Access Sizes */ > +#define ACPI_ACCESS_SIZE_UNDEFINED 0 > +#define ACPI_ACCESS_SIZE_BYTE 1 > +#define ACPI_ACCESS_SIZE_WORD 2 > +#define ACPI_ACCESS_SIZE_DWORD 3 > +#define ACPI_ACCESS_SIZE_QWORD 4 > + > struct acpi_resource_gpio { > u8 revision_id; > u8 connection_type; Please don't define this. It's possible to calculate 8/16/32/64 from the access width value. Try: 1 << (access_width + 2） Thanks Lv
On Thu, May 4, 2017 at 11:09 PM, Zheng, Lv <lv.zheng@intel.com> wrote: > Hi, > >> From: Jon Mason [mailto:jon.mason@broadcom.com] >> Sent: Thursday, May 4, 2017 11:06 PM >> Subject: [PATCH] ACPI: SPCR: Use access width to determine mmio usage >> >> 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. >> >> Signed-off-by: Jon Mason <jon.mason@broadcom.com> >> --- >> drivers/acpi/spcr.c | 18 ++++++++++++++++-- >> include/acpi/acrestyp.h | 7 +++++++ >> 2 files changed, 23 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/acpi/spcr.c b/drivers/acpi/spcr.c >> index 01c9466..11233f6 100644 >> --- a/drivers/acpi/spcr.c >> +++ b/drivers/acpi/spcr.c >> @@ -74,8 +74,22 @@ int __init parse_spcr(bool earlycon) >> goto done; >> } >> >> - iotype = table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY ? >> - "mmio" : "io"; >> + if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) { >> + switch (table->serial_port.access_width) { >> + default: >> + pr_err("Unexpected SPCR Access Width. Defaulting to byte size\n"); >> + case ACPI_ACCESS_SIZE_BYTE: >> + iotype = "mmio"; >> + break; >> + case ACPI_ACCESS_SIZE_WORD: >> + iotype = "mmio16"; >> + break; >> + case ACPI_ACCESS_SIZE_DWORD: >> + iotype = "mmio32"; >> + break; >> + } >> + } else >> + iotype = "io"; >> >> switch (table->interface_type) { >> case ACPI_DBG2_ARM_SBSA_32BIT: >> diff --git a/include/acpi/acrestyp.h b/include/acpi/acrestyp.h >> index f0f7403..781cb55 100644 >> --- a/include/acpi/acrestyp.h >> +++ b/include/acpi/acrestyp.h >> @@ -372,6 +372,13 @@ struct acpi_resource_generic_register { >> u64 address; >> }; >> >> +/* Generic Address Space Access Sizes */ >> +#define ACPI_ACCESS_SIZE_UNDEFINED 0 >> +#define ACPI_ACCESS_SIZE_BYTE 1 >> +#define ACPI_ACCESS_SIZE_WORD 2 >> +#define ACPI_ACCESS_SIZE_DWORD 3 >> +#define ACPI_ACCESS_SIZE_QWORD 4 >> + >> struct acpi_resource_gpio { >> u8 revision_id; >> u8 connection_type; > > Please don't define this. > It's possible to calculate 8/16/32/64 from the access width value. > Try: > 1 << (access_width + 2） Thanks, I'll send out v2 of the patch with this change. > Thanks > Lv > -- 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
On 05/04/2017 11:05 AM, Jon Mason wrote: > 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. Jon.
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 -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
On 05/08/2017 03:11 PM, Loc Ho wrote: > 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. Jon.
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. -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
On Mon, May 8, 2017 at 3:57 PM, Loc Ho <lho@apm.com> wrote: > 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? Thanks, Jon Mas(on) > > -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
Hi Jon, On Mon, May 8, 2017 at 1:34 PM, Jon Mason <jon.mason@broadcom.com> wrote: > On Mon, May 8, 2017 at 3:57 PM, Loc Ho <lho@apm.com> wrote: >> 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 -- 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 01c9466..11233f6 100644 --- a/drivers/acpi/spcr.c +++ b/drivers/acpi/spcr.c @@ -74,8 +74,22 @@ int __init parse_spcr(bool earlycon) goto done; } - iotype = table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY ? - "mmio" : "io"; + if (table->serial_port.space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) { + switch (table->serial_port.access_width) { + default: + pr_err("Unexpected SPCR Access Width. Defaulting to byte size\n"); + case ACPI_ACCESS_SIZE_BYTE: + iotype = "mmio"; + break; + case ACPI_ACCESS_SIZE_WORD: + iotype = "mmio16"; + break; + case ACPI_ACCESS_SIZE_DWORD: + iotype = "mmio32"; + break; + } + } else + iotype = "io"; switch (table->interface_type) { case ACPI_DBG2_ARM_SBSA_32BIT: diff --git a/include/acpi/acrestyp.h b/include/acpi/acrestyp.h index f0f7403..781cb55 100644 --- a/include/acpi/acrestyp.h +++ b/include/acpi/acrestyp.h @@ -372,6 +372,13 @@ struct acpi_resource_generic_register { u64 address; }; +/* Generic Address Space Access Sizes */ +#define ACPI_ACCESS_SIZE_UNDEFINED 0 +#define ACPI_ACCESS_SIZE_BYTE 1 +#define ACPI_ACCESS_SIZE_WORD 2 +#define ACPI_ACCESS_SIZE_DWORD 3 +#define ACPI_ACCESS_SIZE_QWORD 4 + struct acpi_resource_gpio { u8 revision_id; u8 connection_type;
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. Signed-off-by: Jon Mason <jon.mason@broadcom.com> --- drivers/acpi/spcr.c | 18 ++++++++++++++++-- include/acpi/acrestyp.h | 7 +++++++ 2 files changed, 23 insertions(+), 2 deletions(-)