Re: [edk2-devel] Additional configuration options on Armada/Cn913x

[greg@unrelenting.technology]

On June 12, 2020 3:07:21 PM UTC, Marcin Wojtas <mw@semihalf.com> wrote:
>Hi,
>
>I see Greg was dropped in the meantime.
>
>pt., 12 cze 2020 o 10:45 Ard Biesheuvel <ard.biesheuvel@arm.com> napisał(a):
>>
>> On 6/12/20 12:43 AM, Mark Kettenis via Groups.Io wrote:
>> > On Thu, Jun 11, 2020 at 04:17 PM, Ard Biesheuvel wrote:
>> >
>> >     On 6/11/20 4:07 PM, greg@unrelenting.technology wrote:
>> >
>> >         June 11, 2020 4:19 PM, "Ard Biesheuvel" <ard.biesheuvel@arm.com>
>> >         wrote:
>> >
>> >             On 6/5/20 5:19 PM, Marcin Wojtas via groups.io wrote:
>> >
>> >                 Hi,
>> >                 I'd like to ask for comments before I develop the actual
>> >                 code - > currently we have 2 workarounds
>> >                 done specifically for Linux:
>> >                 a. ECAM shift in PCIE
>> >                 b. SPCR address space definition
>> >
>> >             What does this mean?
>> >
>> >         The SPCR in upstream edk2 is set up to work around some Linux
>> >         weirdness (?) and I have to do this:
>> >
>> >         https://github.com/myfreeweb/edk2-platforms/commit/74ec98a6498e78d2ae6c861db88487bf75f2e1a1
>> >
>> >         to make it work on FreeBSD.
>> >
>> >     Surely, they can't both be correct. Marcin?
>> >
>> > Assuming the serial port on Armada/Cn911x is the same as on Armada8k,
>> > the following DT properties would be applicable:
>> >
>> > reg-shift = <2>;
>> > reg-io-width = <1>
>> >
>> > which means the registers are spaced 32-bits apart but have to be
>> > accessed using 8-bit load/store instructions.  I'd say that
>> > means that the ACPI Generic Address Space should have RegisterBitWidth
>> > set to 32 and AccessSize set to BYTE.
>> > In other words, I think that the current:
>> >
>> > #define MV_UART_AS32(Address) { EFI_ACPI_5_0_SYSTEM_MEMORY, 32, 0,
>> > EFI_ACPI_5_0_BYTE, Address }
>> >
>> > is correct.  That certainly is what works for OpenBSD.
>>
>> Thanks Mark
>>
>> The struct type is defined as
>>
>> typedef struct {
>>    UINT8   AddressSpaceId;
>>    UINT8   RegisterBitWidth;
>>    UINT8   RegisterBitOffset;
>>    UINT8   AccessSize;
>>    UINT64  Address;
>> } EFI_ACPI_5_0_GENERIC_ADDRESS_STRUCTURE;
>>
>> so I agree that the current definition matches a UART that requires byte
>> accesses on registers that are 32 bits apart.
>>
>> So why does FreeBSD deviate from this?
>
>Greg, can you please explain your concerns here?

Yeah, looking at our code again, looks like we did screw it up, seems like we are using register width as access width and vice versa :/