Messages in this thread | | | Subject | Re: [PATCH v2 1/4] ACPI / watchdog: Add support for WDAT hardware watchdog | From | Guenter Roeck <> | Date | Tue, 20 Sep 2016 07:48:53 -0700 |
| |
On 09/20/2016 06:50 AM, Mika Westerberg wrote: > On Tue, Sep 20, 2016 at 06:37:23AM -0700, Guenter Roeck wrote: >>> + for (i = 0; i < tbl->entries; i++) { >>> + const struct acpi_generic_address *gas; >>> + struct wdat_instruction *instr; >>> + struct list_head *instructions; >>> + unsigned int action; >>> + struct resource r; >>> + int j; >>> + >>> + action = entries[i].action; >>> + if (action >= MAX_WDAT_ACTIONS) { >>> + dev_dbg(&pdev->dev, "Skipping unknown action: %u\n", >>> + action); >>> + continue; >>> + } >>> + >>> + instr = devm_kzalloc(&pdev->dev, sizeof(*instr), GFP_KERNEL); >>> + if (!instr) >>> + return -ENOMEM; >>> + >>> + INIT_LIST_HEAD(&instr->node); >>> + instr->entry = entries[i]; >>> + >>> + gas = &entries[i].register_region; >>> + >>> + memset(&r, 0, sizeof(r)); >>> + r.start = gas->address; >>> + r.end = r.start + gas->access_width; >>> + if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) { >>> + r.flags = IORESOURCE_MEM; >>> + } else if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_IO) { >>> + r.flags = IORESOURCE_IO; >>> + } else { >>> + dev_dbg(&pdev->dev, "Unsupported address space: %d\n", >>> + gas->space_id); >>> + continue; >>> + } >>> + >>> + /* Find the matching resource */ >>> + for (j = 0; j < pdev->num_resources; j++) { >>> + res = &pdev->resource[j]; >>> + if (resource_contains(res, &r)) { >>> + instr->reg = regs[j] + r.start - res->start; >>> + break; >>> + } >>> + } >>> + >>> + if (!instr->reg) { >>> + dev_err(&pdev->dev, "I/O resource not found\n"); >>> + return -EINVAL; >>> + } >>> + >> I must be missing something here. What is happening with instr ? > > Instr contains the actual instruction from WDAT table. There can be > many. > >>> + instructions = wdat->instructions[action]; > > This will be NULL first time so... > >>> + if (!instructions) { >>> + instructions = devm_kzalloc(&pdev->dev, >>> + sizeof(*instructions), GFP_KERNEL); >>> + if (!instructions) >>> + return -ENOMEM; >>> + >>> + INIT_LIST_HEAD(instructions); >>> + wdat->instructions[action] = instructions; >> >> ... and I don't really see how wdat->instructions[action] contains anything but >> a zero filled array. Confused :-(. > > ...we allocate entry for that here. Next time it will not be NULL for > that action. > >> >>> + } >>> + >>> + list_add_tail(&instr->node, instructions); > > Then we append each instr to the list for this action. > > Hope this clarifies :) > Yes, I somehow didn't see that last line. Too early in the morning, not enough coffee.
Thanks, Guenter
| |