Re: ACPI aml_rwgsb() fix

2021-05-22 Thread Theo Buehler 
On Thu, May 20, 2021 at 12:19:37AM +0200, Mark Kettenis wrote:
> My last change to dsdt.c broke one or two of my cheap little Intel
> "Atom" laptops.  Seems my interpretation of the ACPI standard wasn't
> quite right.  I went back to the original bug report and I think I
> understand a bit better what the AML in that report is trying to do.
> So here is a diff that fixes things.
> 
> Theo, can you try this on that Dell Precision 3640?
> 
> Tests on other hardware are welcome, especially on laptops.

ok tb

Re: ACPI aml_rwgsb() fix

2021-05-21 Thread Cesare Gargano 
On Fri, May 21, 2021 at 06:03:49AM +0200, Theo Buehler wrote:
> On Thu, May 20, 2021 at 12:19:37AM +0200, Mark Kettenis wrote:
> > My last change to dsdt.c broke one or two of my cheap little Intel
> > "Atom" laptops.  Seems my interpretation of the ACPI standard wasn't
> > quite right.  I went back to the original bug report and I think I
> > understand a bit better what the AML in that report is trying to do.
> > So here is a diff that fixes things.
> > 
> > Theo, can you try this on that Dell Precision 3640?
> > 
> > Tests on other hardware are welcome, especially on laptops.
> 
> That Dell Precision boots happily with this, as does my x280.
> 

With Mark's patch Asus E200HA boots OK (disabled acpitz and inteldrm for
other problems), attached e200ha-dmesg-patched.txt.

Without Mark's patch, -current panics, attached handwritten
e200ha-dmesg-current.txt.
OpenBSD 6.9-current (GENERIC.MP) #28: Fri May 21 06:24:33 CEST 2021
?? ?? gar...@zizania.plusiagamma.org:/sys/arch/amd64/compile/GENERIC.MP
real mem = 2020298752 (1926MB)
avail mem = 1935659008 (1845MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 3.0 @ 0x7b19e000 (19 entries)
bios0: vendor American Megatrends Inc. version "E200HA.303" date 12/21/2016
bios0: ASUSTeK COMPUTER INC. E200HA
acpi0 at bios0: ACPI 5.0
acpi0: sleep states S0 S4 S5
acpi0: tables DSDT FACP APIC FPDT FIDT MCFG SSDT SSDT SSDT UEFI HPET SSDT SSDT 
SSDT SSDT TPM2 LPIT BCFG PRAM BGRT CSRT WDAT MSDM
acpi0: wakeup devices
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Atom(TM) x5-Z8300 CPU @ 1.44GHz, 1440.32 MHz, 06-4c-03
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,SSE4.1,SSE4.2,MOVBE,POPCNT,DEADLINE,AES,RDRAND,NXE,RDTSCP,LONG,LAHF,3DNOWP,PERF,ITSC,TSC_ADJUST,SMEP,ERMS,MD_CLEAR,IBRS,IBPB,STIBP,SENSOR,ARAT,MELTDOWN
cpu0: 1MB 64b/line 16-way L2 cache
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
cpu0: apic clock running at 79MHz
cpu0: mwait min=64, max=64, C-substates=0.2.0.0.0.0.3.3, IBE
cpu1 at mainbus0: apid 2 (application processor)
cpu1: Intel(R) Atom(TM) x5-Z8300 CPU @ 1.44GHz, 1439.97 MHz, 06-4c-03
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,SSE4.1,SSE4.2,MOVBE,POPCNT,DEADLINE,AES,RDRAND,NXE,RDTSCP,LONG,LAHF,3DNOWP,PERF,ITSC,TSC_ADJUST,SMEP,ERMS,MD_CLEAR,IBRS,IBPB,STIBP,SENSOR,ARAT,MELTDOWN
cpu1: 1MB 64b/line 16-way L2 cache
cpu1: smt 0, core 1, package 0
cpu2 at mainbus0: apid 4 (application processor)
cpu2: Intel(R) Atom(TM) x5-Z8300 CPU @ 1.44GHz, 1439.98 MHz, 06-4c-03
cpu2: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,SSE4.1,SSE4.2,MOVBE,POPCNT,DEADLINE,AES,RDRAND,NXE,RDTSCP,LONG,LAHF,3DNOWP,PERF,ITSC,TSC_ADJUST,SMEP,ERMS,MD_CLEAR,IBRS,IBPB,STIBP,SENSOR,ARAT,MELTDOWN
cpu2: 1MB 64b/line 16-way L2 cache
cpu2: smt 0, core 2, package 0
cpu3 at mainbus0: apid 6 (application processor)
cpu3: Intel(R) Atom(TM) x5-Z8300 CPU @ 1.44GHz, 1439.97 MHz, 06-4c-03
cpu3: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,CX16,xTPR,PDCM,SSE4.1,SSE4.2,MOVBE,POPCNT,DEADLINE,AES,RDRAND,NXE,RDTSCP,LONG,LAHF,3DNOWP,PERF,ITSC,TSC_ADJUST,SMEP,ERMS,MD_CLEAR,IBRS,IBPB,STIBP,SENSOR,ARAT,MELTDOWN
cpu3: 1MB 64b/line 16-way L2 cache
cpu3: smt 0, core 3, package 0
ioapic0 at mainbus0: apid 1 pa 0xfec0, version 20, 115 pins
acpimcfg0 at acpi0
acpimcfg0: addr 0xe000, bus 0-255
acpihpet0 at acpi0: 14318179 Hz
acpiprt0 at acpi0: bus 0 (PCI0)
acpiprt1 at acpi0: bus -1 (RP01)
acpiprt2 at acpi0: bus -1 (RP02)
acpiprt3 at acpi0: bus -1 (RP03)
acpiprt4 at acpi0: bus -1 (RP04)
acpiec0 at acpi0: not present
"INT33A4" at acpi0 not configured
dwiic0 at acpi0 I2C1 addr 0x91432000/0x1000 irq 32
iic0 at dwiic0
chvgpio0 at acpi0 GPO0 uid 1 addr 0xfed8/0x8000 irq 49, 56 pins
ihidev0 at iic0 addr 0x68 gpio 93, vendor 0xb05 product 0x8585, PDEC3393
ihidev0: 9 report ids
ikbd0 at ihidev0 reportid 1: 8 variable keys, 6 key codes
wskbd0 at ikbd0 mux 1
hid at ihidev0 reportid 3 not configured
hid at ihidev0 reportid 6 not configured
hid at ihidev0 reportid 9 not configured
dwiic1 at acpi0 I2C3 addr 0x9142e000/0x1000 irq 34
iic1 at dwiic1
chvgpio1 at acpi0 GPO1 uid 2 addr 0xfed88000/0x8000 irq 48, 59 pins
ihidev1 at iic1 addr 0x15 gpio 17, vendor 0xb05 product 0x101, FTE1000
ihidev1: 93 report ids
ims0 at ihidev1 reportid 1: 2 buttons, Z dir
wsmouse0 at

Re: ACPI aml_rwgsb() fix

2021-05-20 Thread Theo Buehler 
On Thu, May 20, 2021 at 12:19:37AM +0200, Mark Kettenis wrote:
> My last change to dsdt.c broke one or two of my cheap little Intel
> "Atom" laptops.  Seems my interpretation of the ACPI standard wasn't
> quite right.  I went back to the original bug report and I think I
> understand a bit better what the AML in that report is trying to do.
> So here is a diff that fixes things.
> 
> Theo, can you try this on that Dell Precision 3640?
> 
> Tests on other hardware are welcome, especially on laptops.

That Dell Precision boots happily with this, as does my x280.

ACPI aml_rwgsb() fix

2021-05-19 Thread Mark Kettenis 
My last change to dsdt.c broke one or two of my cheap little Intel
"Atom" laptops.  Seems my interpretation of the ACPI standard wasn't
quite right.  I went back to the original bug report and I think I
understand a bit better what the AML in that report is trying to do.
So here is a diff that fixes things.

Theo, can you try this on that Dell Precision 3640?

Tests on other hardware are welcome, especially on laptops.


Index: dev/acpi/dsdt.c
===
RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
retrieving revision 1.262
diff -u -p -r1.262 dsdt.c
--- dev/acpi/dsdt.c 30 Mar 2021 16:49:58 -  1.262
+++ dev/acpi/dsdt.c 19 May 2021 22:14:46 -
@@ -2527,7 +2527,7 @@ aml_rwgpio(struct aml_value *conn, int b
 #ifndef SMALL_KERNEL
 
 void
-aml_rwgsb(struct aml_value *conn, int alen, int bpos, int blen,
+aml_rwgsb(struct aml_value *conn, int len, int bpos, int blen,
 struct aml_value *val, int mode, int flag)
 {
union acpi_resource *crs = (union acpi_resource *)conn->v_buffer;
@@ -2535,17 +2535,17 @@ aml_rwgsb(struct aml_value *conn, int al
i2c_tag_t tag;
i2c_op_t op;
i2c_addr_t addr;
-   int cmdlen, buflen, acclen;
-   uint8_t cmd;
+   int cmdlen, buflen;
+   uint8_t cmd[2];
uint8_t *buf;
-   int pos, err;
+   int err;
 
if (conn->type != AML_OBJTYPE_BUFFER || conn->length < 5 ||
AML_CRSTYPE(crs) != LR_SERBUS || AML_CRSLEN(crs) > conn->length ||
crs->lr_i2cbus.revid != 1 || crs->lr_i2cbus.type != LR_SERBUS_I2C)
aml_die("Invalid GenericSerialBus");
if (AML_FIELD_ACCESS(flag) != AML_FIELD_BUFFERACC ||
-   bpos & 0x3 || (blen % 8) != 0)
+   bpos & 0x3 || (blen % 8) != 0 || blen > 16)
aml_die("Invalid GenericSerialBus access");
 
node = aml_searchname(conn->node,
@@ -2556,32 +2556,27 @@ aml_rwgsb(struct aml_value *conn, int al
switch (AML_FIELD_ATTR(flag)) {
case 0x02:  /* AttribQuick */
cmdlen = 0;
-   buflen = acclen = 0;
+   buflen = 0;
break;
case 0x04:  /* AttribSendReceive */
cmdlen = 0;
-   acclen = 1;
-   buflen = blen / 8;
+   buflen = 1;
break;
case 0x06:  /* AttribByte */
-   cmdlen = 1;
-   acclen = 1;
-   buflen = blen / 8;
+   cmdlen = blen / 8;
+   buflen = 1;
break;
case 0x08:  /* AttribWord */
-   cmdlen = 1;
-   acclen = 2;
-   buflen = blen / 8;
+   cmdlen = blen / 8;
+   buflen = 2;
break;
case 0x0b:  /* AttribBytes */
-   cmdlen = 1;
-   acclen = alen;
-   buflen = blen / 8;
+   cmdlen = blen / 8;
+   buflen = len;
break;
case 0x0e:  /* AttribRawBytes */
cmdlen = 0;
-   acclen = alen;
-   buflen = blen / 8;
+   buflen = len;
break;
default:
aml_die("unsupported access type 0x%x", flag);
@@ -2589,12 +2584,12 @@ aml_rwgsb(struct aml_value *conn, int al
}
break;
case 1: /* AttribBytes */
-   cmdlen = 1;
-   acclen = buflen = AML_FIELD_ATTR(flag);
+   cmdlen = blen / 8;
+   buflen = AML_FIELD_ATTR(flag);
break;
case 2: /* AttribRawBytes */
cmdlen = 0;
-   acclen = buflen = AML_FIELD_ATTR(flag);
+   buflen = AML_FIELD_ATTR(flag);
break;
default:
aml_die("unsupported access type 0x%x", flag);
@@ -2621,16 +2616,11 @@ aml_rwgsb(struct aml_value *conn, int al
 
tag = node->i2c;
addr = crs->lr_i2cbus._adr;
-   cmd = bpos >> 3;
+   cmd[0] = bpos >> 3;
+   cmd[1] = bpos >> 11;
 
iic_acquire_bus(tag, 0);
-   for (pos = 0; pos < buflen; pos += acclen) {
-   err = iic_exec(tag, op, addr, &cmd, cmdlen,
-   &buf[pos + 2], acclen, 0);
-   if (err)
-   break;
-   cmd++;
-   }
+   err = iic_exec(tag, op, addr, &cmd, cmdlen, &buf[2], buflen, 0);
iic_release_bus(tag, 0);
 
/*
@@ -2650,14 +2640,14 @@ aml_rwgsb(struct aml_value *conn, int al
  */
 
 void
-aml_rwgsb(struct aml_value *conn, int alen,