[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240827204549.GA4545@yjiang5-mobl.amr.corp.intel.com>
Date: Tue, 27 Aug 2024 13:45:49 -0700
From: Yunhong Jiang <yunhong.jiang@...ux.intel.com>
To: Krzysztof Kozlowski <krzk@...nel.org>
Cc: tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com,
robh@...nel.org, krzk+dt@...nel.org, conor+dt@...nel.org,
kys@...rosoft.com, haiyangz@...rosoft.com, wei.liu@...nel.org,
decui@...rosoft.com, rafael@...nel.org, lenb@...nel.org,
kirill.shutemov@...ux.intel.com, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org, linux-hyperv@...r.kernel.org,
linux-acpi@...r.kernel.org
Subject: Re: [PATCH v2 2/9] dt-bindings: x86: Add a binding for x86 wakeup
mailbox
On Sun, Aug 25, 2024 at 09:10:01AM +0200, Krzysztof Kozlowski wrote:
> On Fri, Aug 23, 2024 at 04:23:20PM -0700, Yunhong Jiang wrote:
> > Add the binding to use mailbox wakeup mechanism to bringup APs.
> >
> > Signed-off-by: Yunhong Jiang <yunhong.jiang@...ux.intel.com>
> > ---
> > .../devicetree/bindings/x86/wakeup.yaml | 64 +++++++++++++++++++
> > 1 file changed, 64 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/x86/wakeup.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/x86/wakeup.yaml b/Documentation/devicetree/bindings/x86/wakeup.yaml
> > new file mode 100644
> > index 000000000000..cb84e2756bca
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/x86/wakeup.yaml
> > @@ -0,0 +1,64 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# Copyright (C) 2024 Intel Corporation
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/x86/wakeup.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: x86 mailbox wakeup
> > +maintainers:
> > + - Yunhong Jiang <yunhong.jiang@...ux.intel.com>
> > +
> > +description: |
> > + The x86 mailbox wakeup mechanism defines a mechanism to let the bootstrap
> > + processor (BSP) to wake up application processors (APs) through a wakeup
> > + mailbox.
> > +
> > + The "wakeup-mailbox-addr" property specifies the wakeup mailbox address. The
> > + wakeup mailbox is a 4K-aligned 4K-size memory block allocated in the reserved
> > + memory.
> > +
> > + The wakeup mailbox structure is defined as follows.
> > +
> > + uint16_t command;
> > + uint16_t reserved;
> > + uint32_t apic_id;
> > + uint64_t wakeup_vector;
> > + uint8_t reservedForOs[2032];
> > +
> > + The memory after reservedForOs field is reserved and OS should not touch it.
> > +
> > + To wakes up a AP, the BSP prepares the wakeup routine, fills the wakeup
> > + routine's address into the wakeup_vector field, fill the apic_id field with
> > + the target AP's APIC_ID, and write 1 to the command field. After receiving the
> > + wakeup command, the target AP will jump to the wakeup routine.
> > +
> > + For each AP, the mailbox can be used only once for the wakeup command. After
> > + the AP jumps to the wakeup routine, the mailbox will no longer be checked by
> > + this AP.
> > +
> > + The wakeup mailbox structure and the wakeup process is the same as
> > + the Multiprocessor Wakeup Mailbox Structure defined in ACPI spec version 6.5,
> > + section 5.2.12.19 [1].
> > +
> > + References:
> > +
> > + [1] https://uefi.org/specs/ACPI/6.5/05_ACPI_Software_Programming_Model.html
> > +
> > +select: false
>
> This schema is still a no-op because of this false.
>
> What is the point of defining one property if it is not placed anywhere?
> Every device node can have it? Seems wrong...
>
> You need to come with proper schema. Lack of an example is another thing
> - this cannot be even validated by the tools.
>
> Best regards,
> Krzysztof
Thank you for the feedback. Will update the schema file on next round
submission.
Thanks
--jyh
>
Powered by blists - more mailing lists