On Tue, Aug 06 2024 at 15:12, Yunhong Jiang wrote:
>
> -static int __init acpi_mp_setup_reset(u64 reset_vector)
> +static int __init __maybe_unused acpi_mp_setup_reset(u64 reset_vector)
> {
> struct x86_mapping_info info = {
> .alloc_pgt_page = alloc_pgt_page,
> @@ -226,7 +228,7 @@ static int acpi_wakeup_cpu(u32 apicid, unsigned long start_ip)
> return 0;
> }
>
> -static void acpi_mp_disable_offlining(struct acpi_madt_multiproc_wakeup *mp_wake)
> +static void __maybe_unused acpi_mp_disable_offlining(struct
> acpi_madt_multiproc_wakeup *mp_wake)
Please move those functions into the #ifdef CONFIG_ACPI
> {
> cpu_hotplug_disable_offlining();
>
> @@ -248,6 +250,7 @@ static void acpi_mp_disable_offlining(struct acpi_madt_multiproc_wakeup *mp_wake
> mp_wake->mailbox_address = 0;
> }
>
> +#ifdef CONFIG_ACPI
> int __init acpi_parse_mp_wake(union acpi_subtable_headers *header,
> +
> +#ifdef CONFIG_OF
> +int __init dtb_parse_mp_wake(u64 *wake_mailbox_paddr)
Why not returning the mailbox physical address and 0 on failure instead
of that pointer and a integer return value which is ignored at the call
site?
> +{
> + struct device_node *node;
> + u64 mailaddr;
> + int ret = 0;
> +
> + node = of_find_node_by_path("/cpus");
> + if (!node)
> + return -ENODEV;
> +
> + if (of_property_match_string(node, "enable-method",
> + "acpi-wakeup-mailbox") < 0) {
Please use the 100 characters line width and spare the line break.
> + pr_err("No acpi wakeup mailbox enable-method\n");
> + ret = -ENODEV;
> + goto done;
> + }
> +
> + /*
> + * No support to the MADT reset vector yet.
s/to/for/
Also a single line comment is sufficient for this.
> + */
> + cpu_hotplug_disable_offlining();
> +
> + if (of_property_read_u64(node, "wakeup-mailbox-addr", &mailaddr)) {
> + pr_err("Invalid wakeup mailbox addr\n");
> + ret = -EINVAL;
> + goto done;
> + }
> + acpi_mp_wake_mailbox_paddr = mailaddr;
> + if (wake_mailbox_paddr)
> + *wake_mailbox_paddr = mailaddr;
> + pr_info("dt wakeup-mailbox: addr 0x%llx\n", mailaddr);
> + apic_update_callback(wakeup_secondary_cpu_64, acpi_wakeup_cpu);
> +done:
newline before the label please. Up there you waste 3 lines for a
trivial comment and here you make the code unreadable...
Thanks,
tglx
