[ddk] simplify get_auxdata

* pass a single string as the argument to get_auxdata
* introduce a fixed-size message between the pci and pci proxy
* use a more/cookie mechanism instead of get-nth to get
  multiple children data

Change-Id: I505bb97ad82a5e7f71bc8fdc2953575a32926639
diff --git a/system/dev/bus/acpi/bus-acpi.c b/system/dev/bus/acpi/bus-acpi.c
index c4f2f9e..3edf9a5 100644
--- a/system/dev/bus/acpi/bus-acpi.c
+++ b/system/dev/bus/acpi/bus-acpi.c
@@ -82,12 +82,17 @@
 } publish_acpi_device_ctx_t;
 
 typedef struct {
-    uint8_t n;
+    uint8_t max;
     uint8_t i;
     zx_status_t st;
+    uint32_t* more;
+    uint32_t* cookie;
     auxdata_i2c_device_t* data;
 } pci_child_auxdata_ctx_t;
 
+typedef struct {
+} acpi_auxdata_ctx_t;
+
 zx_handle_t root_resource_handle;
 
 static zx_device_t* publish_device(zx_device_t* parent, ACPI_HANDLE handle,
@@ -156,13 +161,12 @@
     return AE_CTRL_TERMINATE;
 }
 
-static ACPI_STATUS pci_child_data_callback(ACPI_HANDLE object, uint32_t nesting_level,
-                                       void* context, void** out_value) {
+static ACPI_STATUS pci_child_data_callback(ACPI_HANDLE object,
+                                           uint32_t nesting_level,
+                                           void* context, void** out_value) {
     pci_child_auxdata_ctx_t* ctx = (pci_child_auxdata_ctx_t*)context;
-    uint32_t i = ctx->i++;
-    if (i < ctx->n) {
-        return AE_OK;
-    } else if (i > ctx->n) {
+    if (ctx->i++ > ctx->max) {
+        *ctx->more = 1;
         return AE_CTRL_TERMINATE;
     }
 
@@ -190,25 +194,27 @@
     return AE_CTRL_TERMINATE;
 }
 
-static zx_status_t pciroot_op_get_auxdata(void* context, auxdata_type_t type,
-                                          void* _args, size_t args_len,
-                                          void* out_data, size_t out_len) {
+static zx_status_t pciroot_op_get_auxdata(void* context, const char* args,
+                                          void* data, uint32_t bytes,
+                                          uint32_t* actual) {
     acpi_device_t* dev = (acpi_device_t*)context;
-    if (type != AUXDATA_PCI_CHILD_NTH_DEVICE) {
-        return ZX_ERR_NOT_SUPPORTED;
-    }
 
-    auxdata_args_pci_child_nth_device_t* args = _args;
-    if (args->child_type != AUXDATA_DEVICE_I2C) {
-        return ZX_ERR_NOT_SUPPORTED;
-    }
-
-    if (out_len != sizeof(auxdata_i2c_device_t)) {
+    char type[16];
+    uint32_t bus_id, dev_id, func_id;
+    if (sscanf(args, "%s,%02x:%02x:%02x", type, bus_id, dev_id, func_id) < 0) {
         return ZX_ERR_INVALID_ARGS;
     }
 
+    if (strcmp(type, "i2c-child")) {
+        return ZX_ERR_NOT_SUPPORTED;
+    }
+
+    if (bytes < (2 * sizeof(uint32_t))) {
+        return ZX_ERR_BUFFER_TOO_SMALL;
+    }
+
     ACPI_HANDLE pci_node = NULL;
-    uint32_t addr = (args->dev_id << 16) | args->func_id;
+    uint32_t addr = (dev_id << 16) | func_id;
 
     // Look for the child node with this device and function id
     ACPI_STATUS acpi_status = AcpiWalkNamespace(ACPI_TYPE_DEVICE, dev->ns_node, 1,
@@ -221,16 +227,18 @@
         return ZX_ERR_NOT_FOUND;
     }
 
-    // Look for the nth child for this pci node and fill in data
+    // Look for as many children as can fit in the provided buffer
     pci_child_auxdata_ctx_t ctx = {
-        .n = args->n,
+        .max = (bytes - 2 * sizeof(uint32_t)) / sizeof(auxdata_i2c_device_t),
         .i = 0,
         .st = ZX_ERR_NOT_FOUND,
-        .data = out_data,
+        .more = data,
+        .cookie = data + sizeof(uint32_t),
+        .data = data + 2 * sizeof(uint32_t),
     };
 
-    acpi_status = AcpiWalkNamespace(ACPI_TYPE_DEVICE, pci_node, 1, pci_child_data_callback,
-                                    NULL, &ctx, NULL);
+    acpi_status = AcpiWalkNamespace(ACPI_TYPE_DEVICE, pci_node, 1,
+                                    pci_child_data_callback, NULL, &ctx, NULL);
     if ((acpi_status != AE_OK) && (acpi_status != AE_CTRL_TERMINATE)) {
         return acpi_to_zx_status(acpi_status);
     }
diff --git a/system/dev/bus/pci/kpci-private.h b/system/dev/bus/pci/kpci-private.h
index 2443dea..46692dc 100644
--- a/system/dev/bus/pci/kpci-private.h
+++ b/system/dev/bus/pci/kpci-private.h
@@ -33,26 +33,14 @@
 
 extern _Atomic zx_txid_t pci_global_txid;
 
-typedef struct {
-    zx_txid_t txid;
-    uint32_t len;
-    uint32_t op;
-} pci_req_hdr_t;
+#define PCI_MAX_DATA 4096
 
 typedef struct {
-    pci_req_hdr_t hdr;
-    auxdata_type_t auxdata_type;
-    auxdata_args_nth_device_t args;
-} pci_req_auxdata_nth_device_t;
+    zx_txid_t txid;     // FIDL2 message header
+    uint32_t reserved0;
+    uint32_t flags;
+    uint32_t ordinal;
 
-typedef struct {
-    zx_txid_t txid;
-    uint32_t len;
-    uint32_t op;
-    zx_status_t status;
-} pci_resp_hdr_t;
-
-typedef struct {
-    pci_resp_hdr_t hdr;
-    auxdata_i2c_device_t device;
-} pci_resp_auxdata_i2c_nth_device_t;
+    uint32_t datalen;
+    uint8_t data[PCI_MAX_DATA];
+} pci_msg_t;
diff --git a/system/dev/bus/pci/kpci.c b/system/dev/bus/pci/kpci.c
index af7ec10..ae89011 100644
--- a/system/dev/bus/pci/kpci.c
+++ b/system/dev/bus/pci/kpci.c
@@ -36,56 +36,38 @@
 #if PROXY_DEVICE
     return ZX_ERR_NOT_SUPPORTED;
 #else
-    uint32_t actual;
-    zx_status_t st = zx_channel_read(h, 0, NULL, NULL, 0, 0, &actual, NULL);
-    if ((st != ZX_OK) && (st != ZX_ERR_BUFFER_TOO_SMALL)) {
-        return st;
-    }
-
-    pci_req_auxdata_nth_device_t req;
-    if (actual > sizeof(req)) {
-        return ZX_ERR_NOT_SUPPORTED;
-    }
-
-    st = zx_channel_read(h, 0, &req, NULL, sizeof(req), 0, &actual, NULL);
+    pci_msg_t req;
+    st = zx_channel_read(h, 0, &req, NULL, sizeof(req), 0, NULL, NULL);
     if (st != ZX_OK) {
         return st;
     }
 
-    if ((req.hdr.op != PCI_OP_GET_AUXDATA) ||
-        (req.auxdata_type != AUXDATA_NTH_DEVICE) ||
-        (req.args.child_type != AUXDATA_DEVICE_I2C)) {
-        return ZX_ERR_NOT_SUPPORTED;
+    pci_msg_t resp = {
+        .txid = msg.txid,
+        .reserved0 = 0,
+        .flags = 0,
+        .datalen = 0,
+    };
+    if (req.ordinal != PCI_OP_GET_AUXDATA) {
+        resp.ordinal = ZX_ERR_NOT_SUPPORTED;
+        goto out;
     }
 
-    kpci_device_t* device = ctx;
-    if (device->pciroot.ctx == NULL) {
-        return ZX_ERR_NOT_SUPPORTED;
-    }
+    char args[32];
+    snprintf(args, sizeof(args), "%s,%02x:%02x:%02x", req.data,
+            device->info.bus_id, device->info.dev_id, device->info.func_id);
 
-    auxdata_args_pci_child_nth_device_t args = {
-        .bus_id = device->info.bus_id,
-        .dev_id = device->info.dev_id,
-        .func_id = device->info.func_id,
-        .n = req.args.n,
-        .child_type = req.args.child_type,
-    };
-
-    pci_resp_auxdata_i2c_nth_device_t resp = {
-        .hdr = {
-            .txid = req.hdr.txid,
-            .len = sizeof(resp),
-            .op = req.hdr.op,
-            .status = ZX_OK,
-        },
-    };
-    st = pciroot_get_auxdata(&device->pciroot, AUXDATA_PCI_CHILD_NTH_DEVICE,
-                             &args, sizeof(args),
-                             &resp.device, sizeof(resp.device));
+    size_t actual;
+    st = pciroot_get_auxdata(&device->pciroot, args, resp.data, PCI_MAX_DATA,
+                             &resp.device, &actual);
     if (st != ZX_OK) {
-        resp.hdr.status = st;
+        resp.ordinal = st;
+    } else {
+        resp.ordinal = ZX_OK;
+        resp.datalen = actual;
     }
 
+out:
     return zx_channel_write(h, 0, &resp, sizeof(resp), NULL, 0);
 #endif
 }
diff --git a/system/dev/bus/pci/protocol.c b/system/dev/bus/pci/protocol.c
index 056d516..d9cda5d 100644
--- a/system/dev/bus/pci/protocol.c
+++ b/system/dev/bus/pci/protocol.c
@@ -231,44 +231,28 @@
     return ZX_OK;
 }
 
-static zx_status_t kpci_get_auxdata(void* ctx, auxdata_type_t type,
-                                    void* _args, size_t args_len,
-                                    void* out_data, size_t out_len) {
+static zx_status_t kpci_get_auxdata(void* ctx, const char* args, void* data,
+                                    uint32_t bytes, uint32_t* actual) {
 #if PROXY_DEVICE
-    if (type != AUXDATA_NTH_DEVICE) {
-        return ZX_ERR_NOT_SUPPORTED;
-    }
-
-    if (args_len != sizeof(auxdata_args_nth_device_t)) {
-        return ZX_ERR_INVALID_ARGS;
-    }
-
-    auxdata_args_nth_device_t* args = _args;
-    if (args->child_type != AUXDATA_DEVICE_I2C) {
-        return ZX_ERR_NOT_SUPPORTED;
-    }
-    if (out_len != sizeof(auxdata_i2c_device_t)) {
-        return ZX_ERR_INVALID_ARGS;
-    }
-
-    kpci_device_t* dev = ctx;
     if (dev->pciroot_rpcch == ZX_HANDLE_INVALID) {
         return ZX_ERR_NOT_SUPPORTED;
     }
 
-    pci_req_auxdata_nth_device_t req = {
-        .hdr = {
-            .txid = atomic_fetch_add(&pci_global_txid, 1),
-            .op = PCI_OP_GET_AUXDATA,
-            .len = sizeof(req),
-        },
-        .auxdata_type = type,
-        .args = {
-            .child_type = args->child_type,
-            .n = args->n,
-        },
+    size_t arglen = strlen(args);
+    if (arglen > PCI_MAX_DATA) {
+        return ZX_ERR_INVALID_ARGS;
+    }
+
+    pci_msg_t resp;
+    pci_msg_t req = {
+        .txid = atomic_fetch_add(&pci_global_txid, 1),
+        .reserved0 = 0,
+        .flags = 0,
+        .ordinal = PCI_OP_GET_AUXDATA,
+        .datalen = arglen,
     };
-    pci_resp_auxdata_i2c_nth_device_t resp;
+    memcpy(data, args, arglen);
+
     zx_channel_call_args_t cc_args = {
         .wr_bytes = &req,
         .rd_bytes = &resp,
@@ -281,19 +265,26 @@
     };
     uint32_t actual_bytes;
     uint32_t actual_handles;
-    zx_status_t st = zx_channel_call(dev->pciroot_rpcch, 0, ZX_TIME_INFINITE, &cc_args,
-                                     &actual_bytes, &actual_handles, NULL);
+    zx_status_t st = zx_channel_call(dev->pciroot_rpcch, 0, ZX_TIME_INFINITE,
+                                     &cc_args, &actual_bytes,
+                                     &actual_handles, NULL);
     if (st != ZX_OK) {
         return st;
-    } else if (actual_bytes != sizeof(resp)) {
+    }
+    if (actual_bytes != sizeof(resp)) {
         return ZX_ERR_INTERNAL;
     }
-
-    if (resp.hdr.status == ZX_OK) {
-        memcpy(out_data, &resp.device, sizeof(resp.device));
+    if (resp.ordinal != ZX_OK) {
+        return resp.ordinal;
     }
-
-    return resp.hdr.status;
+    if (resp.datalen > bytes) {
+        return ZX_ERR_BUFFER_TOO_SMALL;
+    }
+    memcpy(data, resp.data, resp.datalen);
+    if (actual) {
+        *actual = resp.datalen;
+    }
+    return ZX_OK;
 #else
     return ZX_ERR_NOT_SUPPORTED;
 #endif
diff --git a/system/dev/serial/intel-serialio/i2c/controller.c b/system/dev/serial/intel-serialio/i2c/controller.c
index deb564d..a1f5d85 100644
--- a/system/dev/serial/intel-serialio/i2c/controller.c
+++ b/system/dev/serial/intel-serialio/i2c/controller.c
@@ -718,6 +718,52 @@
     return ZX_ERR_NOT_SUPPORTED;
 }
 
+void zx_status_t intel_serialio_add_devices(zx_device_t* parent,
+                                            pci_protocol_t* pci) {
+    // get child info from aux data
+    // space for 1 device plus header (more + cookie)
+    uint8_t childdata[sizeof(auxdata_i2c_device_t) + sizeof(uint32_t) * 2];
+    size_t actual;
+    char args[16];
+    uint32_t more;
+    uint32_t cookie = 0;
+    auxdata_i2c_device_t* child;
+    uint32_t bus_speed = 0;
+    // TODO: this seems nonstandard to device model
+    for (;;) {
+        memset(childdata, 0, sizeof(childdata));
+        status = pci_get_auxdata(pci, "i2c-child", childdata, sizeof(childdata),
+                                 &actual);
+        if (status != ZX_OK) {
+            break;
+        }
+        if (actual != sizeof(childdata)) {
+            break;
+        }
+        more = *(uint32_t*)(childdata);
+        cookie = *(uint32_t*)(childdata + sizeof(uint32_t));
+        child = (auxdata_i2c_device_t*)(childdata + 2 * sizeof(uint32_t));
+        if (child->protocol_id == ZX_PROTOCOL_I2C_HID) {
+            if (bus_speed && bus_speed != child->bus_speed) {
+                zxlogf(ERROR, "i2c: cannot add devices with different bus speeds (%u, %u)\n",
+                        bus_speed, child->bus_speed);
+                break;
+            }
+            if (!bus_speed) {
+                intel_serialio_i2c_set_bus_frequency(parent, child->bus_speed);
+                bus_speed = child->bus_speed;
+            }
+            intel_serialio_i2c_add_slave(parent,
+                    child->ten_bit ? I2C_10BIT_ADDRESS : I2C_7BIT_ADDRESS,
+                    child->address);
+        }
+        if (!more) {
+            break;
+        }
+    }
+
+}
+
 zx_status_t intel_serialio_bind_i2c(zx_device_t* dev) {
     pci_protocol_t pci;
     if (device_get_protocol(dev, ZX_PROTOCOL_PCI, &pci))
@@ -830,36 +876,7 @@
         "reg=%p regsize=%ld\n",
         device->regs, device->regs_size);
 
-    // get child info from aux data
-    auxdata_args_nth_device_t auxdata_args = {
-        .child_type = AUXDATA_DEVICE_I2C,
-        .n = 0,
-    };
-    auxdata_i2c_device_t child;
-    uint32_t bus_speed = 0;
-    // TODO: this seems nonstandard to device model
-    do {
-        memset(&child, 0, sizeof(child));
-        status = pci_get_auxdata(&pci, AUXDATA_NTH_DEVICE, &auxdata_args, sizeof(auxdata_args),
-                                 &child, sizeof(child));
-        if (status == ZX_OK) {
-            if (child.protocol_id == ZX_PROTOCOL_I2C_HID) {
-                if (bus_speed && bus_speed != child.bus_speed) {
-                    zxlogf(ERROR, "i2c: cannot add devices with different bus speeds (%u, %u)\n",
-                            bus_speed, child.bus_speed);
-                    break;
-                }
-                if (!bus_speed) {
-                    intel_serialio_i2c_set_bus_frequency(device, child.bus_speed);
-                    bus_speed = child.bus_speed;
-                }
-                intel_serialio_i2c_add_slave(device,
-                        child.ten_bit ? I2C_10BIT_ADDRESS : I2C_7BIT_ADDRESS,
-                        child.address);
-            }
-        }
-    } while ((status == ZX_OK) && auxdata_args.n++);
-
+    intel_serialio_add_devices(device, &pci);
 
     zx_handle_close(config_handle);
     return ZX_OK;
diff --git a/system/ulib/ddk/include/ddk/protocol/auxdata.h b/system/ulib/ddk/include/ddk/protocol/auxdata.h
index e23ce25..9450c59 100644
--- a/system/ulib/ddk/include/ddk/protocol/auxdata.h
+++ b/system/ulib/ddk/include/ddk/protocol/auxdata.h
@@ -9,58 +9,6 @@
 
 __BEGIN_CDECLS;
 
-typedef enum {
-    // returns the nth child of this device
-    //  in: auxdata_args_nth_device_t
-    // out: [depends on auxdata_args_nth_device_t.child_type]
-    AUXDATA_NTH_DEVICE,
-
-    // returns the nth child of a pci device
-    //  in: auxdata_args_pci_child_nth_device_t
-    // out: [depends on auxdata_args_pci_child_nth_device_t.child_type]
-    AUXDATA_PCI_CHILD_NTH_DEVICE,
-
-    // TODO: not implemented
-    // returns the timing parameters of the i2c bus
-    //  in: auxdata_i2c_timing_type_t timing
-    // out: auxdata_i2c_timing_t i2c bus timing info
-    AUXDATA_I2C_TIMING,
-
-    AUXDATA_TYPE_MAX,
-} auxdata_type_t;
-
-// AUXDATA_NTH_DEVICE, AUXDATA_PCI_CHILD_NTH_DEVICE
-
-typedef enum {
-    // auxdata_i2c_device_t
-    AUXDATA_DEVICE_I2C,
-    AUXDATA_DEVICE_MAX,
-} auxdata_device_type_t;
-
-// args: AUXDATA_NTH_DEVICE
-
-typedef struct {
-    // type of the expected device
-    auxdata_device_type_t child_type;
-    // device index
-    uint8_t n;
-} auxdata_args_nth_device_t;
-
-// args: AUXDATA_PCI_CHILD_NTH_DEVICE
-
-typedef struct {
-    // PCI device info
-    uint8_t bus_id;
-    uint8_t dev_id;
-    uint8_t func_id;
-    // device index
-    uint8_t n;
-    // type of the expected device
-    auxdata_device_type_t child_type;
-} auxdata_args_pci_child_nth_device_t;
-
-// out: AUXDATA_NTH_DEVICE, AUXDATA_PCI_CHILD_NTH_DEVICE
-
 typedef struct {
     // i2c bus config
     uint8_t bus_master;
@@ -71,21 +19,4 @@
     uint32_t protocol_id;
 } auxdata_i2c_device_t;
 
-// args: AUXDATA_I2C_TIMING
-
-typedef enum {
-    AUXDATA_I2C_TIMING_SS,
-    AUXDATA_I2C_TIMING_FP,
-    AUXDATA_I2C_TIMING_HS,
-    AUXDATA_I2C_TIMING_FM,
-} auxdata_i2c_timing_type_t;
-
-// out: AUXDATA_I2C_TIMING
-
-typedef struct {
-    uint16_t hcnt;
-    uint16_t lcnt;
-    uint32_t sda_hold_time;
-} auxdata_i2c_timing_t;
-
 __END_CDECLS;
diff --git a/system/ulib/ddk/include/ddk/protocol/pci.h b/system/ulib/ddk/include/ddk/protocol/pci.h
index 4937b03..cc20343 100644
--- a/system/ulib/ddk/include/ddk/protocol/pci.h
+++ b/system/ulib/ddk/include/ddk/protocol/pci.h
@@ -77,8 +77,8 @@
     zx_status_t (*get_device_info)(void* ctx, zx_pcie_device_info_t* out_info);
     uint32_t    (*config_read)(void* ctx, uint8_t offset, size_t width);
     uint8_t     (*get_next_capability)(void* ctx, uint8_t type, uint8_t offset);
-    zx_status_t (*get_auxdata)(void* ctx, auxdata_type_t type, void* args, size_t args_len,
-                               void* out_data, size_t out_len);
+    zx_status_t (*get_auxdata)(void* ctx, const char* args,
+                               void* data, uint32_t bytes, uint32_t* actual);
 } pci_protocol_ops_t;
 typedef struct pci_protocol {
     pci_protocol_ops_t* ops;
@@ -151,10 +151,10 @@
     return pci_get_next_capability(pci, kPciCfgCapabilitiesPtr - 1u, type);
 }
 
-static inline zx_status_t pci_get_auxdata(pci_protocol_t* pci, auxdata_type_t type,
-                                          void* args, size_t args_len,
-                                          void* out_data, size_t out_len) {
-    return pci->ops->get_auxdata(pci->ctx, type, args, args_len, out_data, out_len);
+static inline zx_status_t pci_get_auxdata(pci_protocol_t* pci,
+                                          const char* args, void* data,
+                                          uint32_t bytes, uint32_t* actual) {
+    return pci->ops->get_auxdata(pci->ctx, args, data, bytes, actual);
 }
 
 __END_CDECLS;
diff --git a/system/ulib/ddk/include/ddk/protocol/pciroot.h b/system/ulib/ddk/include/ddk/protocol/pciroot.h
index 194e911..ac13f63 100644
--- a/system/ulib/ddk/include/ddk/protocol/pciroot.h
+++ b/system/ulib/ddk/include/ddk/protocol/pciroot.h
@@ -11,9 +11,8 @@
 __BEGIN_CDECLS;
 
 typedef struct pciroot_protocol_ops {
-    zx_status_t (*get_auxdata)(void* ctx, auxdata_type_t type,
-                               void* args, size_t args_len,
-                               void* out_data, size_t out_len);
+    zx_status_t (*get_auxdata)(void* ctx, const char* args, void* data,
+                               uint32_t bytes, uint32_t* actual);
 } pciroot_protocol_ops_t;
 
 typedef struct pciroot_protocol {
@@ -22,10 +21,9 @@
 } pciroot_protocol_t;
 
 static inline zx_status_t pciroot_get_auxdata(pciroot_protocol_t* pciroot,
-                                              auxdata_type_t type,
-                                              void* args, size_t args_len,
-                                              void* out_data, size_t out_len) {
-    return pciroot->ops->get_auxdata(pciroot->ctx, type, args, args_len, out_data, out_len);
+                                              const char* args, void* data,
+                                              uint32_t bytes, uint32_t* actual) {
+    return pciroot->ops->get_auxdata(pciroot->ctx, args, data, bytes, actual);
 }
 
 __END_CDECLS;