Hi Greg,

pt., 12 cze 2020 o 19:09 Marcin Wojtas <mw@semihalf.com> napisał(a):

> pt., 12 cze 2020 o 18:32 myfreeweb <greg@unrelenting.technology>
> napisał(a):
> >
> >
> >
> > 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 :/
>
> Ok, so I'll prepare only a switch to toggle the ECAM hack enablement.
>
> Any chance you submit a fix to phabricator?
>
>
>
I pushed a patch to review: Please review and test:
https://reviews.freebsd.org/D25373

Best regards,
Marcin