> Date: Mon, 6 Dec 2021 21:45:03 +0100
> From: Anton Lindqvist <[email protected]>
>
> On Mon, Dec 06, 2021 at 09:23:45PM +0100, Mark Kettenis wrote:
> > > Date: Mon, 6 Dec 2021 21:08:04 +0100
> > > From: Patrick Wildt <[email protected]>
> > >
> > > Hi,
> > >
> > > On one machine I had the pleasure of having to try and use the
> > > Serial-over-LAN feature which shows up as just another com(4)
> > > device. Instead of having to manually add a com(4) at isa(4)
> > > I figured it would be nicer to have them attach via ACPI. At
> > > least on that machine, the SOL definitely shows up in the DSDT.
> > >
> > > Since I don't want to break any legacy machines, I figured I'd
> > > keep ignoring the isa(4) addresses specified in amd64's GENERIC.
> > >
> > > Right now this diff is more about putting it out there, not about
> > > asking for OKs, as amd64 isn't really my strong suit. If people
> > > are interested, I can definitely put in all the feedback there is.
> > >
> > > Patrick
> >
> > anton@ has a better diff he's working on
>
> Here's the diff in its current state. There's one thing I haven't had
> the time to figure out yet: in order to get interrupts working on my
> apu2 I had to explicit pass LR_EXTIRQ_MODE (aaa_irq_flags is zero) to
> acpi_intr_establish() which causes IST_EDGE to be passed to
> intr_establish(). Worth noting is that this matches what com at isa
> already does. This was however not necessary on my amd64 build machine
> where aaa_irq_flags also is zero.
Actually, it seems we're parsing the ACPI interrupt resource
descriptor wrong. The decompiled DSDTs I have here seem to use
IRQNoFlags() for the interrupts, which apparently implies the
interrupt is edge-triggered.
Let me see if I can cook a diff.
>
> Example attachment:
>
> com0 at acpi0 UAR2 addr 0x3f8/0x8 irq 3: ns16550a, 16 byte fifo
> com0: console
>
> diff --git share/man/man4/com.4 share/man/man4/com.4
> index 73b421f2ca7..e54255fe0d6 100644
> --- share/man/man4/com.4
> +++ share/man/man4/com.4
> @@ -61,11 +61,13 @@
> .Cd "com0 at isa? port 0x3f8 irq 4"
> .Cd "com1 at isa? port 0x2f8 irq 3"
> .Pp
> +.Cd "# arm64 and amd64"
> +.Cd "com* at acpi?"
> +.Pp
> .Cd "# armv7"
> .Cd "com* at fdt?"
> .Pp
> .Cd "# arm64"
> -.Cd "com* at acpi?"
> .Cd "com* at fdt?"
> .Pp
> .Cd "# hppa"
> diff --git sys/arch/amd64/conf/GENERIC sys/arch/amd64/conf/GENERIC
> index ecccd1323d9..87fcaced306 100644
> --- sys/arch/amd64/conf/GENERIC
> +++ sys/arch/amd64/conf/GENERIC
> @@ -65,6 +65,11 @@ amdgpio* at acpi?
> aplgpio* at acpi?
> bytgpio* at acpi?
> chvgpio* at acpi?
> +com0 at acpi?
> +com1 at acpi?
> +com2 at acpi?
> +com3 at acpi?
> +com* at acpi?
> glkgpio* at acpi?
> pchgpio* at acpi?
> sdhc* at acpi?
> diff --git sys/arch/amd64/conf/RAMDISK sys/arch/amd64/conf/RAMDISK
> index 6041293b287..a5f10b357dd 100644
> --- sys/arch/amd64/conf/RAMDISK
> +++ sys/arch/amd64/conf/RAMDISK
> @@ -34,6 +34,9 @@ acpipci* at acpi?
> acpiprt* at acpi?
> acpimadt0 at acpi?
> #acpitz* at acpi?
> +com0 at acpi?
> +com1 at acpi?
> +com* at acpi?
>
> mpbios0 at bios0
>
> diff --git sys/arch/amd64/conf/RAMDISK_CD sys/arch/amd64/conf/RAMDISK_CD
> index 8bd646b4ea3..d007c0102d0 100644
> --- sys/arch/amd64/conf/RAMDISK_CD
> +++ sys/arch/amd64/conf/RAMDISK_CD
> @@ -51,6 +51,10 @@ bytgpio* at acpi?
> sdhc* at acpi?
> acpihve* at acpi?
> chvgpio* at acpi?
> +com0 at acpi?
> +com1 at acpi?
> +com2 at acpi?
> +com* at acpi?
> glkgpio* at acpi?
>
> mpbios0 at bios0
> diff --git sys/dev/acpi/acpi.c sys/dev/acpi/acpi.c
> index 7577424e8a2..e89869aedbd 100644
> --- sys/dev/acpi/acpi.c
> +++ sys/dev/acpi/acpi.c
> @@ -3140,7 +3140,6 @@ const char *acpi_isa_hids[] = {
> "PNP0303", /* IBM Enhanced Keyboard (101/102-key, PS/2 Mouse) */
> "PNP0400", /* Standard LPT Parallel Port */
> "PNP0401", /* ECP Parallel Port */
> - "PNP0501", /* 16550A-compatible COM Serial Port */
> "PNP0700", /* PC-class Floppy Disk Controller */
> "PNP0F03", /* Microsoft PS/2-style Mouse */
> "PNP0F13", /* PS/2 Mouse */
> diff --git sys/dev/acpi/com_acpi.c sys/dev/acpi/com_acpi.c
> index 852be6c71b3..be78e34a016 100644
> --- sys/dev/acpi/com_acpi.c
> +++ sys/dev/acpi/com_acpi.c
> @@ -49,10 +49,12 @@ struct cfattach com_acpi_ca = {
>
> const char *com_hids[] = {
> "HISI0031",
> + "PNP0501",
> NULL
> };
>
> int com_acpi_is_console(struct com_acpi_softc *);
> +int com_acpi_is_designware(const char *);
> int com_acpi_intr_designware(void *);
>
> int
> @@ -69,7 +71,7 @@ com_acpi_attach(struct device *parent, struct device *self,
> void *aux)
> {
> struct com_acpi_softc *sc = (struct com_acpi_softc *)self;
> struct acpi_attach_args *aaa = aux;
> - uint32_t freq;
> + int (*intrfn)(void *) = comintr;
>
> sc->sc_acpi = (struct acpi_softc *)parent;
> sc->sc_node = aaa->aaa_node;
> @@ -85,34 +87,44 @@ com_acpi_attach(struct device *parent, struct device
> *self, void *aux)
> return;
> }
>
> - printf(" addr 0x%llx/0x%llx", aaa->aaa_addr[0], aaa->aaa_size[0]);
> + printf(" addr 0x%llx/%llu", aaa->aaa_addr[0], aaa->aaa_size[0]);
> printf(" irq %d", aaa->aaa_irq[0]);
>
> - freq = acpi_getpropint(sc->sc_node, "clock-frequency", 0);
> -
> sc->sc.sc_iot = aaa->aaa_bst[0];
> sc->sc.sc_iobase = aaa->aaa_addr[0];
> - sc->sc.sc_uarttype = COM_UART_16550;
> - sc->sc.sc_frequency = freq ? freq : COM_FREQ;
> -
> - sc->sc.sc_reg_width = acpi_getpropint(sc->sc_node, "reg-io-width", 4);
> - sc->sc.sc_reg_shift = acpi_getpropint(sc->sc_node, "reg-shift", 2);
> -
> - if (com_acpi_is_console(sc)) {
> - SET(sc->sc.sc_hwflags, COM_HW_CONSOLE);
> - SET(sc->sc.sc_swflags, COM_SW_SOFTCAR);
> - comconsfreq = sc->sc.sc_frequency;
> - comconsrate = B115200;
> + sc->sc.sc_frequency = COM_FREQ;
> +
> + if (com_acpi_is_designware(aaa->aaa_dev)) {
> + intrfn = com_acpi_intr_designware;
> + sc->sc.sc_uarttype = COM_UART_16550;
> + sc->sc.sc_frequency = acpi_getpropint(sc->sc_node,
> + "clock-frequency", COM_FREQ);
> + sc->sc.sc_reg_width = acpi_getpropint(sc->sc_node,
> + "reg-io-width", 4);
> + sc->sc.sc_reg_shift = acpi_getpropint(sc->sc_node,
> + "reg-shift", 2);
> +
> + if (com_acpi_is_console(sc)) {
> + SET(sc->sc.sc_hwflags, COM_HW_CONSOLE);
> + SET(sc->sc.sc_swflags, COM_SW_SOFTCAR);
> + comconsfreq = sc->sc.sc_frequency;
> + comconsrate = B115200;
> + }
> }
>
> - if (bus_space_map(sc->sc.sc_iot, aaa->aaa_addr[0], aaa->aaa_size[0],
> - 0, &sc->sc.sc_ioh)) {
> - printf(": can't map registers\n");
> - return;
> + if (sc->sc.sc_iobase == comconsaddr) {
> + sc->sc.sc_ioh = comconsioh;
> + } else {
> + if (bus_space_map(sc->sc.sc_iot,
> + aaa->aaa_addr[0], aaa->aaa_size[0], 0, &sc->sc.sc_ioh)) {
> + printf(": can't map registers\n");
> + return;
> + }
> }
>
> - sc->sc_ih = acpi_intr_establish(aaa->aaa_irq[0], aaa->aaa_irq_flags[0],
> - IPL_TTY, com_acpi_intr_designware, sc, sc->sc.sc_dev.dv_xname);
> + sc->sc_ih = acpi_intr_establish(aaa->aaa_irq[0],
> + aaa->aaa_irq_flags[0] | LR_EXTIRQ_MODE, IPL_TTY, intrfn,
> + sc, sc->sc.sc_dev.dv_xname);
> if (sc->sc_ih == NULL) {
> printf(": can't establish interrupt\n");
> return;
> @@ -144,6 +156,12 @@ com_acpi_is_console(struct com_acpi_softc *sc)
> return 0;
> }
>
> +int
> +com_acpi_is_designware(const char *hid)
> +{
> + return strcmp("HISI0031", hid) == 0;
> +}
> +
> int
> com_acpi_intr_designware(void *cookie)
> {
>
