Hi Jon,

On Mon, Dec 5, 2016 at 3:27 PM, Jon Masters [off-list ref] wrote:

Duc, Aleksey, all,

I have a question about this...

On 12/05/2016 01:51 PM, Duc Dang wrote:

quoted

On Mon, Dec 5, 2016 at 5:05 AM, Aleksey Makarov
[off-list ref] wrote:

quoted

Check the 'Register Bit Width' field of the ACPI Generic Address
Structure that specifies the address of the UART registers to
decide if the driver should use "mmio32" access instead of "mmio".

If the driver is other than 16550 the access with is defined
by the Interface Type field of the SPCR table.

I have two questions about this:

1). Why is this not a full 16550 (ACPI_DBG2_16550_COMPATIBLE)?

2). Why is it a ACPI_DBG2_16550_SUBSET you are assuming here?

The patch is actually applied for both ACPI_DBG2_16550_COMPATIBLE and
ACPI_DBG2_16500_SUBSET. Or I misunderstood your question? The end
result after applying the patch on linux-next is like this:
        switch (table->interface_type) {
        case ACPI_DBG2_ARM_SBSA_32BIT:
                iotype = "mmio32";
                /* fall through */
        case ACPI_DBG2_ARM_PL011:
        case ACPI_DBG2_ARM_SBSA_GENERIC:
        case ACPI_DBG2_BCM2835:
                uart = "pl011";
                break;
        case ACPI_DBG2_16550_COMPATIBLE:
        case ACPI_DBG2_16550_SUBSET:
                if (table->serial_port.space_id ==
                        ACPI_ADR_SPACE_SYSTEM_MEMORY &&
                    table->serial_port.bit_width == 32)
                        iotype = "mmio32";
                uart = "uart";
                break;
        default:
                err = -ENOENT;
                goto done;
        }

The SPCR and DBG2 spec clearly state that the _SUBSET is intended
to represent a UART compatible with the earlier DGBP specification,
not that a UART is a "subset" of a full 16550 (which seems to be
the assumption in this patch). It's important we get this right.

I built a test kernel with this patch and updated ACPI tables earlier,
but it didn't boot with a console because I had left it a subtype 0,
but just changed the width to 32 bit, which is what I expected.

On Mustang 3.06.25 firmware, DBG2 table has 'Port Type = 0x8000',
'Port subtype = 0x0001'

But I am still curious why setting subtype to '0' does not work on
your board. Are you using Mustang or m400?

Further, I've heard back from Microsoft and they're looking at
adding a specific subtype for this. If they do, I'm inclined to
address existing designs with your patch (but I would favor this
check because against the full 16550) and then switch newer APM
based designs to the new subtype.

Yes, we will look out for the new subtype information.

Jon.

--
Computer Architect | Sent from my Fedora powered laptop

Regards,
Duc Dang.