Re: Porting amdgpio driver from OpenBSD

To: Bruno Melo <bmelo%protonmail.com@localhost>
Subject: Re: Porting amdgpio driver from OpenBSD
From: Bruno Melo <bmelo%protonmail.com@localhost>
Date: Mon, 12 Dec 2022 02:57:27 +0000
No progress yet.

I can see 2 things:
First one, I put the following code inside acpi_res_irq function to check how setted is the data passed to SIMPLEQ_FOREACH:

if (&res->ar_irq != NULL)
{
	printf(" &res->ar_irq != NULL\n");
}
else
{
	printf(" &res->ar_irq == NULL\n");
}
	
if ((&res->ar_irq)->sqh_first != NULL)
{
	printf(" (&res->ar_irq)->sqh_first != NULL\n");
}
else
{
	printf(" (&res->ar_irq)->sqh_first == NULL\n");
}

and what I see is during boot many devices print the messages &res->ar_irq != NULL *and* (&res->ar_irq)->sqh_first != NULL, but ihidev device print printf(" &res->ar_irq != NULL *and* (&res->ar_irq)->sqh_first == NULL.

Second one, i put a print placed inside if (ops->fini) of acpi_resource_parse function, and ihidev never runs that print, but runs the print inside the if (ops->init).

Any guidance on this?

Sent from ProtonMail, Swiss-based encrypted email.


------- Original Message -------
On sexta-feira, 9 de dezembro de 2022 às 11:39 AM, Bruno Melo <bmelo%protonmail.com@localhost> wrote:


> I'm putting printf everywhere and I identified the struct acpi_resources res variable is not valid condition to SIMPLEQ_FOREACH(ar, &res->ar_irq, ar_list) in acpi_res_irq(). Trying to identify why.
> 
> 
> In acpi_resource_parse() called by ihidev_intr_init() I see the if condition in if (ops->init) is valid and I can see my printf being printed inside this if, so the res should be allocated correctly, but when res is passed in acpi_res_irq() my printf is not printed inside that SIMPLEQ_FOREACH. Stucked here. I put printf everywhere else and got things printed correctly, except in SIMPLEQ_FOREACH when ihidev code calls acpi_res_irq().
> 
> 
> When I compare the attach functions of the new amdgpio_acpi driver and other *_acpi code I can't find any relevant difference for this be happening. Any idea?
> 
> 
> 
> Sent from ProtonMail, Swiss-based encrypted email.
> 
> 
> 
> 
> ------- Original Message -------
> On terça-feira, 6 de dezembro de 2022 às 00:35, Bruno Melo bmelo%protonmail.com@localhost wrote:
> 
> 
> 
> > I finished the port of OpenBSD driver and now I have these lines at boot time:
> > 
> > MEM (PNP0C01) at acpi0 not configured
> > VPC0 (VPC2004) at acpi0 not configured
> > ITSD (IDEA2004) at acpi0 not configured
> > HKDV (LKH2019) at acpi0 not configured
> > acpiwmibus at acpiwmi1 not configured
> > 
> > What should I do with them? How to fix them? Anyway to make them configurable?
> > 
> > Sent from ProtonMail, Swiss-based encrypted email.
> > 
> > ------- Original Message -------
> > On segunda-feira, 28 de novembro de 2022 às 11:11 PM, Bruno Melo bmelo%protonmail.com@localhost wrote:
> > 
> > > Hi folks,
> > > 
> > > I'm trying to port amdgpio.c from OpenBSD (/sys/dev/acpi) to NetBSD then I can use my trackpad hopefully.
> > > I made some progress. The code is building but not crashing when the system boots (as expected). But now I can see new devices as 'not configured' message they did not exist before.
> > > 
> > > MEM (PNP0C01) at acpi0 not configured
> > > VPC0 (VPC2004) at acpi0 not configured
> > > ITSD (IDEA2004) at acpi0 not configured
> > > HKDV (LKH2019) at acpi0 not configured
> > > acpiwmibus at acpiwmi1 not configured
> > > 
> > > and then the crash happens in my bus_space_unmap(). Please, do you have any idea what could be the problem or how i can investigate it? Picture attached and code below:
> > > 
> > > #include <sys/param.h>
> > > 
> > > #include <sys/systm.h>
> > > 
> > > #include <sys/malloc.h>
> > > 
> > > #include <sys/device.h>
> > > 
> > > #include <sys/device_impl.h>
> > > 
> > > #include <sys/gpio.h>
> > > 
> > > #include <sys/kmem.h>
> > > 
> > > #include <dev/acpi/acpireg.h>
> > > 
> > > #include <dev/acpi/acpivar.h>
> > > 
> > > #include <dev/acpi/acpi_intr.h>
> > > 
> > > #include <dev/acpi/acpi_event.h>
> > > 
> > > #include <dev/gpio/gpiovar.h>
> > > 
> > > #define LR_GPIO_POLARITY (3L << 1)
> > > #define LR_GPIO_ACTHI (0L << 1)
> > > #define LR_GPIO_ACTLO (1L << 1)
> > > #define LR_GPIO_ACTBOTH (2L << 1)
> > > #define LR_GPIO_MODE (1L << 0)
> > > #define LR_GPIO_LEVEL (0L << 0)
> > > 
> > > #define AMDGPIO_CONF_LEVEL 0x00000100
> > > #define AMDGPIO_CONF_ACTLO 0x00000200
> > > #define AMDGPIO_CONF_ACTBOTH 0x00000400
> > > #define AMDGPIO_CONF_MASK 0x00000600
> > > #define AMDGPIO_CONF_INT_EN 0x00000800
> > > #define AMDGPIO_CONF_INT_MASK 0x00001000
> > > #define AMDGPIO_CONF_RXSTATE 0x00010000
> > > #define AMDGPIO_CONF_TXSTATE 0x00400000
> > > #define AMDGPIO_CONF_TXSTATE_EN 0x00800000
> > > #define AMDGPIO_CONF_INT_STS 0x10000000
> > > #define AMDGPIO_IRQ_MASTER_EOI 0x20000000
> > > #define AMDGPIO_IRQ_BITS 46
> > > #define AMDGPIO_IRQ_PINS 4
> > > 
> > > #define AMDGPIO_IRQ_MASTER 0xfc
> > > #define AMDGPIO_IRQ_STS 0x2f8
> > > 
> > > struct amdgpio_intrhand {
> > > int (*ih_func)(void *);
> > > void ih_arg;
> > > };
> > > 
> > > struct amdgpio_pincfg {
> > > / Modeled after pchgpio but we only have one value to
> > > save/restore */
> > > uint32_t pin_cfg;
> > > };
> > > 
> > > struct amdgpio_softc {
> > > device_t sc_dev;
> > > struct acpi_softc *sc_acpi;
> > > ACPI_HANDLE sc_handle;
> > > 
> > > bus_space_tag_t sc_memt;
> > > bus_space_handle_t sc_memh;
> > > bus_size_t sc_size;
> > > void *sc_ih;
> > > 
> > > int sc_npins;
> > > int sc_pmf;
> > > struct amdgpio_pincfg *sc_pin_cfg;
> > > struct amdgpio_intrhand *sc_pin_ih;
> > > 
> > > struct gpio_chipset_tag sc_gpio;
> > > };
> > > 
> > > int amdgpio_match(device_t, cfdata_t, void *);
> > > void amdgpio_attach(device_t, device_t, void *);
> > > int amdgpio_activate(device_t, int);
> > > 
> > > CFATTACH_DECL_NEW(amdgpio_acpi, sizeof(struct amdgpio_softc), amdgpio_match, amdgpio_attach, NULL, NULL);
> > > 
> > > static const struct device_compatible_entry compat_data[] = {
> > > { .compat = "AMDI0030" },
> > > { .compat = "AMD0030" },
> > > DEVICE_COMPAT_EOL
> > > };
> > > 
> > > int amdgpio_read_pin(void *, int);
> > > void amdgpio_write_pin(void *, int, int);
> > > void * amdgpio_intr_establish(void , int, int, int, int ()(void *), void *);
> > > void amdgpio_intr_enable(void *, int);
> > > void amdgpio_intr_disable(void *, void *);
> > > int amdgpio_pin_intr(struct amdgpio_softc *, int);
> > > int amdgpio_intr(void *);
> > > void amdgpio_save_pin(struct amdgpio_softc *, int pin);
> > > bool amdgpio_save(device_t self, const pmf_qual_t *qual);
> > > void amdgpio_restore_pin(struct amdgpio_softc *, int pin);
> > > bool amdgpio_restore(device_t self, const pmf_qual_t *qual);
> > > 
> > > int
> > > amdgpio_match(device_t parent, cfdata_t cf, void *aux)
> > > {
> > > struct acpi_attach_args *aa = aux;
> > > 
> > > return acpi_compatible_match(aa, compat_data);
> > > }
> > > 
> > > void
> > > amdgpio_attach(device_t parent, device_t self, void *aux)
> > > {
> > > struct acpi_attach_args *aa = aux;
> > > struct amdgpio_softc *sc = device_private(self);
> > > struct gpiobus_attach_args gba;
> > > struct acpi_resources res;
> > > struct acpi_mem *mem;
> > > struct acpi_irq *irq;
> > > ACPI_STATUS rv;
> > > int64_t uid;
> > > 
> > > sc->sc_dev = self;
> > > 
> > > sc->sc_acpi = (struct acpi_softc *)parent;
> > > 
> > > //sc->sc_node = aa->aa_node;
> > > 
> > > if (acpi_eval_integer(aa->aa_node->ad_handle, "_UID", &uid)) {
> > > 
> > > printf(": can't find uid\n");
> > > return;
> > > }
> > > 
> > > printf(" uid %ld", uid);
> > > 
> > > switch (uid) {
> > > case 0:
> > > sc->sc_npins = 184;
> > > 
> > > break;
> > > default:
> > > printf("\n");
> > > return;
> > > }
> > > 
> > > //sc->sc_memt = aa->aaa_bst[0];
> > > 
> > > rv = acpi_resource_parse(self, aa->aa_node->ad_handle, "_CRS",
> > > 
> > > &res, &acpi_resource_parse_ops_default);
> > > if (ACPI_FAILURE(rv))
> > > return;
> > > 
> > > mem = acpi_res_mem(&res, 0);
> > > if (bus_space_map(aa->aa_memt, mem->ar_base, mem->ar_length,
> > > 
> > > 0, &sc->sc_memh)) {
> > > 
> > > aprint_error_dev(self, ": can't map registers\n");
> > > return;
> > > }
> > > 
> > > sc->sc_size = mem->ar_length;
> > > 
> > > irq = acpi_res_irq(&res, 0);
> > > if (irq == NULL) {
> > > aprint_error_dev(self, "couldn't find irq resource\n");
> > > goto done;
> > > }
> > > 
> > > sc->sc_pin_cfg = kmem_zalloc(sc->sc_npins * sizeof(sc->sc_pin_cfg[0]), KM_SLEEP);
> > > 
> > > sc->sc_pin_ih = kmem_zalloc(sc->sc_npins * sizeof(sc->sc_pin_ih[0]), KM_SLEEP);
> > > 
> > > sc->sc_ih = acpi_intr_establish(self,
> > > 
> > > (uint64_t)(uintptr_t)aa->aa_node->ad_handle,
> > > 
> > > IPL_BIO, false, amdgpio_intr, sc, sc->sc_dev->dv_xname);
> > > 
> > > if (sc->sc_ih == NULL) {
> > > 
> > > printf(": can't establish interrupt\n");
> > > goto unmap;
> > > }
> > > 
> > > sc->sc_gpio.gp_cookie = sc;
> > > 
> > > sc->sc_gpio.gp_pin_read = amdgpio_read_pin;
> > > 
> > > sc->sc_gpio.gp_pin_write = amdgpio_write_pin;
> > > 
> > > sc->sc_gpio.gp_intr_establish = amdgpio_intr_establish;
> > > 
> > > //sc->sc_gpio.gp_intr_enable = amdgpio_intr_enable;
> > > 
> > > sc->sc_gpio.gp_intr_disestablish = amdgpio_intr_disable;
> > > 
> > > //sc->sc_node->gpio = &sc->sc_gpio;
> > > 
> > > memset(&gba, 0, sizeof(gba));
> > > gba.gba_npins = sc->sc_npins;
> > > 
> > > printf(", %d pins\n", sc->sc_npins);
> > > 
> > > #if NGPIO > 0
> > > 
> > > config_found(sc->sc_dev, &gba, gpiobus_print, CFARGS_NONE);
> > > 
> > > #endif
> > > 
> > > unmap:
> > > kmem_free(sc->sc_pin_ih, sc->sc_npins * sizeof(*sc->sc_pin_ih));
> > > 
> > > bus_space_unmap(sc->sc_memt, sc->sc_memh, sc->sc_size);
> > > 
> > > sc->sc_size = 0;
> > > 
> > > printf(" DOIS");
> > > 
> > > done:
> > > acpi_resource_cleanup(&res);
> > > (void)pmf_device_register(self, amdgpio_save, amdgpio_restore);
> > > sc->sc_pmf = 1;
> > > 
> > > printf(" TRES");
> > > 
> > > kmem_free(sc->sc_pin_cfg, sc->sc_npins * sizeof(sc->sc_pin_cfg[0]));
> > > 
> > > kmem_free(sc->sc_pin_ih, sc->sc_npins * sizeof(sc->sc_pin_ih[0]));
> > > 
> > > printf(" QUATRO");
> > > }
> > > 
> > > void
> > > amdgpio_save_pin(struct amdgpio_softc *sc, int pin)
> > > {
> > > sc->sc_pin_cfg[pin].pin_cfg = bus_space_read_4(sc->sc_memt, sc->sc_memh,
> > > 
> > > pin * 4);
> > > }
> > > 
> > > bool
> > > amdgpio_save(device_t self, const pmf_qual_t *qual)
> > > {
> > > struct amdgpio_softc *sc = device_private(self);
> > > int pin;
> > > 
> > > for (pin = 0; pin < sc->sc_npins; pin++)
> > > 
> > > amdgpio_save_pin(sc, pin);
> > > 
> > > return true;
> > > }
> > > 
> > > void
> > > amdgpio_restore_pin(struct amdgpio_softc *sc, int pin)
> > > {
> > > if (!sc->sc_pin_ih[pin].ih_func)
> > > 
> > > return;
> > > 
> > > bus_space_write_4(sc->sc_memt, sc->sc_memh, pin * 4,
> > > 
> > > sc->sc_pin_cfg[pin].pin_cfg);
> > > 
> > > }
> > > 
> > > bool
> > > amdgpio_restore(device_t self, const pmf_qual_t *qual)
> > > {
> > > int pin;
> > > struct amdgpio_softc *sc = device_private(self);
> > > for (pin = 0; pin < sc->sc_npins; pin++)
> > > 
> > > amdgpio_restore_pin(sc, pin);
> > > 
> > > return true;
> > > }
> > > 
> > > int
> > > amdgpio_read_pin(void *cookie, int pin)
> > > {
> > > struct amdgpio_softc *sc = cookie;
> > > uint32_t reg;
> > > 
> > > reg = bus_space_read_4(sc->sc_memt, sc->sc_memh, pin * 4);
> > > 
> > > return !!(reg & AMDGPIO_CONF_RXSTATE);
> > > }
> > > 
> > > void
> > > amdgpio_write_pin(void *cookie, int pin, int value)
> > > {
> > > struct amdgpio_softc *sc = cookie;
> > > uint32_t reg;
> > > 
> > > reg = bus_space_read_4(sc->sc_memt, sc->sc_memh, pin * 4);
> > > 
> > > reg |= AMDGPIO_CONF_TXSTATE_EN;
> > > if (value)
> > > reg |= AMDGPIO_CONF_TXSTATE;
> > > else
> > > reg &= ~AMDGPIO_CONF_TXSTATE;
> > > bus_space_write_4(sc->sc_memt, sc->sc_memh, pin * 4, reg);
> > > 
> > > }
> > > 
> > > void *
> > > amdgpio_intr_establish(void *cookie, int pin, int flags,
> > > int irqmode, int (*func)(void *), void *arg)
> > > {
> > > struct amdgpio_softc *sc = cookie;
> > > uint32_t reg;
> > > 
> > > KASSERT(pin >= 0 && pin != 63 && pin < sc->sc_npins);
> > > 
> > > sc->sc_pin_ih[pin].ih_func = func;
> > > 
> > > sc->sc_pin_ih[pin].ih_arg = arg;
> > > 
> > > reg = bus_space_read_4(sc->sc_memt, sc->sc_memh, pin * 4);
> > > 
> > > reg &= ~(AMDGPIO_CONF_MASK | AMDGPIO_CONF_LEVEL |
> > > AMDGPIO_CONF_TXSTATE_EN);
> > > if ((flags & LR_GPIO_MODE) == 0)
> > > reg |= AMDGPIO_CONF_LEVEL;
> > > if ((flags & LR_GPIO_POLARITY) == LR_GPIO_ACTLO)
> > > reg |= AMDGPIO_CONF_ACTLO;
> > > if ((flags & LR_GPIO_POLARITY) == LR_GPIO_ACTBOTH)
> > > reg |= AMDGPIO_CONF_ACTBOTH;
> > > reg |= (AMDGPIO_CONF_INT_MASK | AMDGPIO_CONF_INT_EN);
> > > bus_space_write_4(sc->sc_memt, sc->sc_memh, pin * 4, reg);
> > > 
> > > return NULL;
> > > }
> > > 
> > > void
> > > amdgpio_intr_disable(void *cookie, void *pin)
> > > {
> > > struct amdgpio_softc *sc = cookie;
> > > uint32_t reg;
> > > int *pin_aux = pin;
> > > 
> > > KASSERT(*pin_aux >= 0 && *pin_aux != 63 && *pin_aux < sc->sc_npins);
> > > 
> > > reg = bus_space_read_4(sc->sc_memt, sc->sc_memh, *pin_aux * 4);
> > > 
> > > reg &= ~(AMDGPIO_CONF_INT_MASK | AMDGPIO_CONF_INT_EN);
> > > bus_space_write_4(sc->sc_memt, sc->sc_memh, (int) *pin_aux * 4, reg);
> > > 
> > > }
> > > 
> > > int
> > > amdgpio_pin_intr(struct amdgpio_softc *sc, int pin)
> > > {
> > > uint32_t reg;
> > > int rc = 0;
> > > 
> > > reg = bus_space_read_4(sc->sc_memt, sc->sc_memh, pin * 4);
> > > 
> > > if (reg & AMDGPIO_CONF_INT_STS) {
> > > if (sc->sc_pin_ih[pin].ih_func) {
> > > 
> > > sc->sc_pin_ih[pin].ih_func(sc->sc_pin_ih[pin].ih_arg);
> > > 
> > > /* Clear interrupt */
> > > reg = bus_space_read_4(sc->sc_memt,
> > > 
> > > sc->sc_memh,
> > > 
> > > pin * 4);
> > > bus_space_write_4(sc->sc_memt, sc->sc_memh,
> > > 
> > > pin * 4, reg);
> > > rc = 1;
> > > } else {
> > > /* Mask unhandled interrupt */
> > > reg &= ~(AMDGPIO_CONF_INT_MASK |
> > > AMDGPIO_CONF_INT_EN);
> > > bus_space_write_4(sc->sc_memt, sc->sc_memh,
> > > 
> > > pin * 4, reg);
> > > }
> > > }
> > > 
> > > return rc;
> > > }
> > > 
> > > int
> > > amdgpio_intr(void *arg)
> > > {
> > > struct amdgpio_softc *sc = arg;
> > > uint64_t status;
> > > uint32_t reg;
> > > int rc = 0, pin = 0;
> > > int i, j;
> > > 
> > > status = bus_space_read_4(sc->sc_memt, sc->sc_memh,
> > > 
> > > AMDGPIO_IRQ_STS + 4);
> > > status <<= 32;
> > > status |= bus_space_read_4(sc->sc_memt, sc->sc_memh,
> > > 
> > > AMDGPIO_IRQ_STS);
> > > 
> > > /* One status bit for every four pins /
> > > for (i = 0; i < AMDGPIO_IRQ_BITS; i++, pin += 4) {
> > > if (status & (1ULL << i)) {
> > > for (j = 0; j < AMDGPIO_IRQ_PINS; j++) {
> > > if (amdgpio_pin_intr(sc, pin + j))
> > > rc = 1;
> > > }
> > > }
> > > }
> > > 
> > > / Signal end of interrupt */
> > > reg = bus_space_read_4(sc->sc_memt, sc->sc_memh,
> > > 
> > > AMDGPIO_IRQ_MASTER);
> > > reg |= AMDGPIO_IRQ_MASTER_EOI;
> > > bus_space_write_4(sc->sc_memt, sc->sc_memh,
> > > 
> > > AMDGPIO_IRQ_MASTER, reg);
> > > 
> > > return rc;
> > > }
> > > 
> > > Sent from ProtonMail, Swiss-based encrypted email.
References:
- Porting amdgpio driver from OpenBSD
  - From: Bruno Melo
- Re: Porting amdgpio driver from OpenBSD
  - From: Bruno Melo
- Re: Porting amdgpio driver from OpenBSD
  - From: Bruno Melo
Prev by Date: Re: ATA TRIM?
Next by Date: re: ATA TRIM?
Previous by Thread: Re: Porting amdgpio driver from OpenBSD
Next by Thread: Re: Porting amdgpio driver from OpenBSD
Indexes:
Home | Main Index | Thread Index | Old Index